FIX: Allow anyone who can manage groups to see Automatic tooltip (#30174)

Followup 35ecd0335c

Since we have the moderators_manage_categories_and_groups setting,
more than admins can manage groups, so we need to allow others to
see this Automatic tooltip as well.

Also fixes an inconsistency with canManageGroup between the User
model and Group controller, the latter is correct, allowing management
of automatic groups if can_admin_group permission is true
This commit is contained in:
Martin Brennan 2024-12-09 13:21:24 +10:00 committed by GitHub
parent 25ce1f3399
commit 5e86bc2f43
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
8 changed files with 56 additions and 37 deletions

View File

@ -64,7 +64,7 @@ export default class GroupIndexController extends Controller {
@discourseComputed("model")
canManageGroup(model) {
return this.currentUser && this.currentUser.canManageGroup(model);
return this.currentUser?.canManageGroup(model);
}
@discourseComputed

View File

@ -115,12 +115,8 @@ export default class GroupController extends Controller {
}
@discourseComputed("model", "model.automatic")
canManageGroup(model, automatic) {
return (
this.currentUser &&
(this.currentUser.canManageGroup(model) ||
(model.can_admin_group && automatic))
);
canManageGroup(model) {
return this.currentUser?.canManageGroup(model);
}
@action

View File

@ -1089,9 +1089,7 @@ export default class User extends RestModel.extend(Evented) {
}
canManageGroup(group) {
return group.get("automatic")
? false
: group.get("can_admin_group") || group.get("is_group_owner");
return group.get("can_admin_group") || group.get("is_group_owner");
}
@discourseComputed("groups.@each.title", "badges.[]")

View File

@ -41,8 +41,8 @@
</div>
{{/if}}
{{#if (and this.currentUser.admin this.model.automatic)}}
<DTooltip class="admin-group-automatic-tooltip">
{{#if (and this.canManageGroup this.model.automatic)}}
<DTooltip class="group-automatic-tooltip">
<:trigger>
{{d-icon "gear"}}
{{i18n "admin.groups.manage.membership.automatic"}}

View File

@ -1,11 +1,13 @@
import { click, visit } from "@ember/test-helpers";
import { test } from "qunit";
import Site from "discourse/models/site";
import groupFixtures from "discourse/tests/fixtures/group-fixtures";
import {
acceptance,
updateCurrentUser,
} from "discourse/tests/helpers/qunit-helpers";
import selectKit from "discourse/tests/helpers/select-kit-helper";
import { cloneJSON } from "discourse-common/lib/object";
acceptance("Managing Group Membership", function (needs) {
needs.user();
@ -24,24 +26,6 @@ acceptance("Managing Group Membership", function (needs) {
);
});
test("As an admin on an automatic group", async function (assert) {
await visit("/g/moderators");
assert
.dom(".admin-group-automatic-tooltip")
.exists("displays automatic tooltip");
});
test("As a non-admin user on an automatic group", async function (assert) {
updateCurrentUser({ admin: false });
await visit("/g/moderators");
assert
.dom(".admin-group-automatic-tooltip")
.doesNotExist("does not display automatic tooltip");
});
test("As an admin", async function (assert) {
updateCurrentUser({ can_create_group: true });
@ -182,3 +166,49 @@ acceptance("Managing Group Membership", function (needs) {
.isDisabled("disables group allow_membership_request input");
});
});
acceptance(
"Automatic Group Tooltip - can_admin_group is true",
function (needs) {
needs.user();
needs.pretender((server, helper) => {
server.get("/groups/moderators.json", () => {
const cloned = cloneJSON(groupFixtures["/groups/moderators.json"]);
cloned.group.can_admin_group = true;
cloned.group.is_group_owner = false;
return helper.response(200, cloned);
});
});
test("the current user can see the tooltip because they can manage the group", async function (assert) {
await visit("/g/moderators");
assert
.dom(".group-automatic-tooltip")
.exists("displays automatic tooltip");
});
}
);
acceptance(
"Automatic Group Tooltip - can_admin_group is false",
function (needs) {
needs.user();
needs.pretender((server, helper) => {
server.get("/groups/moderators.json", () => {
const cloned = cloneJSON(groupFixtures["/groups/moderators.json"]);
cloned.group.can_admin_group = false;
cloned.group.is_group_owner = false;
return helper.response(200, cloned);
});
});
test("the current user cannot see the tooltip because they cannot manage the group", async function (assert) {
await visit("/g/moderators");
assert
.dom(".group-automatic-tooltip")
.doesNotExist("does not display automatic tooltip");
});
}
);

View File

@ -29,6 +29,7 @@ export default {
is_group_owner: true,
mentionable: false,
messageable: true,
can_admin_group: false,
},
},
"/groups/discourse.json": {

View File

@ -53,12 +53,6 @@ module("Unit | Model | user", function (hooks) {
const user = store.createRecord("user", { admin: true });
const group = store.createRecord("group", { automatic: true });
assert.false(
user.canManageGroup(group),
"automatic groups cannot be managed."
);
group.set("automatic", false);
group.setProperties({ can_admin_group: true });
assert.true(

View File

@ -180,7 +180,7 @@
}
}
.admin-group-automatic-tooltip {
.group-automatic-tooltip {
align-items: center;
font-size: var(--font-down-2);
padding: 0.2em 1em;