github-mirror/discourse
2
0
Fork 0
mirror of https://github.com/discourse/discourse.git synced 2025-01-20 23:42:01 +08:00
Code Issues Actions 1 Packages Projects Releases Wiki Activity

FEATURE: Allow group owners promote more owners (#19768)

Browse Source 
This change allows group owners (in addition to admins) to promote other members to owners.
This commit is contained in:
Ted Johansson 2023-01-11 16:43:18 +08:00 committed by GitHub
parent 17daf077e2
commit d2e9ea6193
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
15 changed files with 227 additions and 187 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

9
app/assets/javascripts/discourse/app/components/group-member-dropdown.js
View File

@ -43,6 +43,15 @@ export default DropdownSelectBoxComponent.extend({
icon: "shield-alt",
});
}
} else if (this.canEditGroup && !this.member.owner) {
items.push({
id: "makeOwner",
name: I18n.t("groups.members.make_owner"),
description: I18n.t("groups.members.make_owner_description", {
username: this.get("member.username"),
}),
icon: "shield-alt",
});
}
if (this.currentUser.staff) {

6
app/assets/javascripts/discourse/app/controllers/group-index.js
View File

@ -134,17 +134,17 @@ export default Controller.extend({
case "removeMembers":
return ajax(`/groups/${this.model.id}/members.json`, {
type: "DELETE",
data: { user_ids: selection.map((u) => u.id).join(",") },
data: { user_ids: selection.mapBy("id").join(",") },
}).then(() => {
this.model.reloadMembers(this.memberParams, true);
this.set("isBulk", false);
});
case "makeOwners":
return ajax(`/admin/groups/${this.model.id}/owners.json`, {
return ajax(`/groups/${this.model.id}/owners.json`, {
type: "PUT",
data: {
group: { usernames: selection.map((u) => u.username).join(",") },
usernames: selection.mapBy("username").join(","),
},
}).then(() => {
selection.forEach((s) => s.set("owner", true));

4
app/assets/javascripts/discourse/app/models/group.js
View File

@ -148,9 +148,9 @@ const Group = RestModel.extend({
},
async addOwners(usernames, filter, notifyUsers) {
const response = await ajax(`/admin/groups/${this.id}/owners.json`, {
const response = await ajax(`/groups/${this.id}/owners.json`, {
type: "PUT",
data: { group: { usernames, notify_users: notifyUsers } },
data: { usernames, notify_users: notifyUsers },
});
if (filter) {

2
app/assets/javascripts/discourse/app/templates/group-index.hbs
View File

@ -54,6 +54,7 @@
<BulkGroupMemberDropdown
@bulkSelection={{this.bulkSelection}}
@canAdminGroup={{this.model.can_admin_group}}
@canEditGroup={{this.model.can_edit_group}}
@onChange={{action "actOnSelection" this.bulkSelection}}
/>
{{/if}}
@ -148,6 +149,7 @@
<GroupMemberDropdown
@member={{m}}
@canAdminGroup={{this.model.can_admin_group}}
@canEditGroup={{this.model.can_edit_group}}
@onChange={{action "actOnGroup" m}}
/>
{{/if}}

1
app/assets/javascripts/discourse/app/templates/mobile/group-index.hbs
View File

@ -44,6 +44,7 @@
<GroupMemberDropdown
@member={{user}}
@canAdminGroup={{this.model.can_admin_group}}
@canEditGroup={{this.model.can_edit_group}}
@onChange={{action "actOnGroup" user}}
/>
{{/if}}

2
app/assets/javascripts/discourse/tests/acceptance/group-index-test.js
View File

@ -39,7 +39,7 @@ acceptance("Group Members", function (needs) {
needs.user();
needs.pretender((server, helper) => {
server.put("/admin/groups/47/owners.json", () => {
server.put("/groups/47/owners.json", () => {
return helper.response({ success: true });
});
});

29
app/controllers/admin/groups_controller.rb
View File

@ -44,35 +44,6 @@ class Admin::GroupsController < Admin::StaffController
end
end
def add_owners
group = Group.find_by(id: params.require(:id))
raise Discourse::NotFound unless group
return can_not_modify_automatic if group.automatic
guardian.ensure_can_edit_group!(group)
users = User.where(username: group_params[:usernames].split(","))
users.each do |user|
group_action_logger = GroupActionLogger.new(current_user, group)
if !group.users.include?(user)
group.add(user)
group_action_logger.log_add_user_to_group(user)
end
group.group_users.where(user_id: user.id).update_all(owner: true)
group_action_logger.log_make_user_group_owner(user)
if group_params[:notify_users] == "true" || group_params[:notify_users] == true
group.notify_added_to_group(user, owner: true)
end
end
group.restore_user_count!
render json: success_json.merge!(usernames: users.pluck(:username))
end
def remove_owner
group = Group.find_by(id: params.require(:id))
raise Discourse::NotFound unless group

32
app/controllers/groups_controller.rb
View File

@ -385,6 +385,32 @@ class GroupsController < ApplicationController
end
end
def add_owners
group = Group.find_by(id: params.require(:id))
raise Discourse::NotFound unless group
return can_not_modify_automatic if group.automatic
guardian.ensure_can_edit_group!(group)
users = users_from_params
group_action_logger = GroupActionLogger.new(current_user, group)
users.each do |user|
if !group.users.include?(user)
group.add(user)
group_action_logger.log_add_user_to_group(user)
end
group.group_users.where(user_id: user.id).update_all(owner: true)
group_action_logger.log_make_user_group_owner(user)
group.notify_added_to_group(user, owner: true) if params[:notify_users].to_s == "true"
end
group.restore_user_count!
render json: success_json.merge!(usernames: users.pluck(:username))
end
def join
ensure_logged_in
unless current_user.staff?
@ -667,6 +693,12 @@ class GroupsController < ApplicationController
end
end
protected
def can_not_modify_automatic
render_json_error(I18n.t("groups.errors.can_not_modify_automatic"))
end
private
def add_user_to_group(group, user, notify = false)

9
app/serializers/basic_group_serializer.rb
View File

@ -31,6 +31,7 @@ class BasicGroupSerializer < ApplicationSerializer
:members_visibility_level,
:can_see_members,
:can_admin_group,
:can_edit_group,
:publish_read_state
def include_display_name?
@ -73,6 +74,14 @@ class BasicGroupSerializer < ApplicationSerializer
owner_group_ids.present?
end
def can_edit_group
scope.can_edit_group?(object)
end
def include_can_edit_group?
scope.can_edit_group?(object)
end
def can_admin_group
scope.can_admin_group?(object)
end

2
config/routes.rb
View File

@ -112,7 +112,6 @@ Discourse::Application.routes.draw do
resources :groups, only: [:create] do
member do
put "owners" => "groups#add_owners"
delete "owners" => "groups#remove_owner"
put "primary" => "groups#set_primary"
end
@ -1052,6 +1051,7 @@ Discourse::Application.routes.draw do
get "permissions" => "groups#permissions"
put "members" => "groups#add_members"
put "owners" => "groups#add_owners"
put "join" => "groups#join"
delete "members" => "groups#remove_member"
delete "leave" => "groups#leave"

151
spec/requests/admin/groups_controller_spec.rb
View File

@ -146,157 +146,6 @@ RSpec.describe Admin::GroupsController do
end
end
describe "#add_owners" do
context "when logged in as an admin" do
before { sign_in(admin) }
it "should work" do
put "/admin/groups/#{group.id}/owners.json",
params: {
group: {
usernames: [user.username, admin.username].join(","),
},
}
expect(response.status).to eq(200)
response_body = response.parsed_body
expect(response_body["usernames"]).to contain_exactly(user.username, admin.username)
expect(group.group_users.where(owner: true).map(&:user)).to contain_exactly(user, admin)
end
it "returns not-found error when there is no group" do
group.destroy!
put "/admin/groups/#{group.id}/owners.json", params: { group: { usernames: user.username } }
expect(response.status).to eq(404)
end
it "does not allow adding owners to an automatic group" do
group.update!(automatic: true)
expect do
put "/admin/groups/#{group.id}/owners.json",
params: {
group: {
usernames: user.username,
},
}
end.to_not change { group.group_users.count }
expect(response.status).to eq(422)
expect(response.parsed_body["errors"]).to eq(["You cannot modify an automatic group"])
end
it "does not notify users when the param is not present" do
put "/admin/groups/#{group.id}/owners.json", params: { group: { usernames: user.username } }
expect(response.status).to eq(200)
topic =
Topic.find_by(
title:
I18n.t(
"system_messages.user_added_to_group_as_owner.subject_template",
group_name: group.name,
),
archetype: "private_message",
)
expect(topic.nil?).to eq(true)
end
it "notifies users when the param is present" do
put "/admin/groups/#{group.id}/owners.json",
params: {
group: {
usernames: user.username,
notify_users: true,
},
}
expect(response.status).to eq(200)
topic =
Topic.find_by(
title:
I18n.t(
"system_messages.user_added_to_group_as_owner.subject_template",
group_name: group.name,
),
archetype: "private_message",
)
expect(topic.nil?).to eq(false)
expect(topic.topic_users.map(&:user_id)).to include(-1, user.id)
end
end
context "when logged in as a moderator" do
before { sign_in(moderator) }
context "with moderators_manage_categories_and_groups enabled" do
before { SiteSetting.moderators_manage_categories_and_groups = true }
it "adds owners" do
put "/admin/groups/#{group.id}/owners.json",
params: {
group: {
usernames: [user.username, admin.username, moderator.username].join(","),
},
}
response_body = response.parsed_body
expect(response.status).to eq(200)
expect(response_body["usernames"]).to contain_exactly(
user.username,
admin.username,
moderator.username,
)
expect(group.group_users.where(owner: true).map(&:user)).to contain_exactly(
user,
admin,
moderator,
)
end
end
context "with moderators_manage_categories_and_groups disabled" do
before { SiteSetting.moderators_manage_categories_and_groups = false }
it "prevents adding of owners with a 403 response" do
put "/admin/groups/#{group.id}/owners.json",
params: {
group: {
usernames: [user.username, admin.username, moderator.username].join(","),
},
}
expect(response.status).to eq(403)
expect(response.parsed_body["errors"]).to include(I18n.t("invalid_access"))
expect(group.group_users.where(owner: true).map(&:user)).to be_empty
end
end
end
context "when logged in as a non-staff user" do
before { sign_in(user) }
it "prevents adding of owners with a 404 response" do
put "/admin/groups/#{group.id}/owners.json",
params: {
group: {
usernames: [user.username, admin.username].join(","),
},
}
expect(response.status).to eq(404)
expect(response.parsed_body["errors"]).to include(I18n.t("not_found"))
expect(group.group_users.where(owner: true).map(&:user)).to be_empty
end
end
end
describe "#remove_owner" do
let(:user2) { Fabricate(:user) }
let(:user3) { Fabricate(:user) }

3
spec/requests/api/schemas/json/group_create_response.json
View File

@ -119,6 +119,9 @@
"can_admin_group": {
"type": "boolean"
},
"can_edit_group": {
"type": "boolean"
},
"publish_read_state": {
"type": "boolean"
}

3
spec/requests/api/schemas/json/group_response.json
View File

@ -122,6 +122,9 @@
"can_admin_group": {
"type": "boolean"
},
"can_edit_group": {
"type": "boolean"
},
"publish_read_state": {
"type": "boolean"
},

3
spec/requests/api/schemas/json/groups_list_response.json
View File

@ -131,6 +131,9 @@
"can_admin_group": {
"type": "boolean"
},
"can_edit_group": {
"type": "boolean"
},
"publish_read_state": {
"type": "boolean"
}

158
spec/requests/groups_controller_spec.rb
View File

@ -1638,6 +1638,164 @@ RSpec.describe GroupsController do
end
end
describe "#add_owners" do
context "when logged in as an admin" do
before { sign_in(admin) }
it "should work" do
put "/groups/#{group.id}/owners.json",
params: {
usernames: [user.username, admin.username].join(","),
}
expect(response.status).to eq(200)
response_body = response.parsed_body
expect(response_body["usernames"]).to contain_exactly(user.username, admin.username)
expect(group.group_users.where(owner: true).map(&:user)).to contain_exactly(user, admin)
end
it "returns not-found error when there is no group" do
group.destroy!
put "/groups/#{group.id}/owners.json", params: { usernames: user.username }
expect(response.status).to eq(404)
end
it "does not allow adding owners to an automatic group" do
group.update!(automatic: true)
expect do
put "/groups/#{group.id}/owners.json", params: { usernames: user.username }
end.to_not change { group.group_users.count }
expect(response.status).to eq(422)
expect(response.parsed_body["errors"]).to eq(
[I18n.t("groups.errors.can_not_modify_automatic")],
)
end
it "does not notify users when the param is not present" do
put "/groups/#{group.id}/owners.json", params: { usernames: user.username }
expect(response.status).to eq(200)
topic =
Topic.find_by(
title:
I18n.t(
"system_messages.user_added_to_group_as_owner.subject_template",
group_name: group.name,
),
archetype: "private_message",
)
expect(topic.nil?).to eq(true)
end
it "notifies users when the param is present" do
put "/groups/#{group.id}/owners.json",
params: {
usernames: user.username,
notify_users: true,
}
expect(response.status).to eq(200)
topic =
Topic.find_by(
title:
I18n.t(
"system_messages.user_added_to_group_as_owner.subject_template",
group_name: group.name,
),
archetype: "private_message",
)
expect(topic.nil?).to eq(false)
expect(topic.topic_users.map(&:user_id)).to include(-1, user.id)
end
end
context "when logged in as a moderator" do
before { sign_in(moderator) }
context "with moderators_manage_categories_and_groups enabled" do
before { SiteSetting.moderators_manage_categories_and_groups = true }
it "adds owners" do
put "/groups/#{group.id}/owners.json",
params: {
usernames: [user.username, admin.username, moderator.username].join(","),
}
response_body = response.parsed_body
expect(response.status).to eq(200)
expect(response_body["usernames"]).to contain_exactly(
user.username,
admin.username,
moderator.username,
)
expect(group.group_users.where(owner: true).map(&:user)).to contain_exactly(
user,
admin,
moderator,
)
end
end
context "with moderators_manage_categories_and_groups disabled" do
before { SiteSetting.moderators_manage_categories_and_groups = false }
it "prevents adding of owners with a 403 response" do
put "/groups/#{group.id}/owners.json",
params: {
usernames: [user.username, admin.username, moderator.username].join(","),
}
expect(response.status).to eq(403)
expect(response.parsed_body["errors"]).to include(I18n.t("invalid_access"))
expect(group.group_users.where(owner: true).map(&:user)).to be_empty
end
end
end
context "when logged in as a non-owner" do
before { sign_in(user) }
it "prevents adding of owners with a 403 response" do
put "/groups/#{group.id}/owners.json",
params: {
usernames: [user.username, admin.username].join(","),
}
expect(response.status).to eq(403)
expect(response.parsed_body["errors"]).to include(I18n.t("invalid_access"))
expect(group.group_users.where(owner: true).map(&:user)).to be_empty
end
end
context "when logged in as an owner" do
before { sign_in(user) }
it "allows adding new owners" do
group.add_owner(user)
put "/groups/#{group.id}/owners.json",
params: {
usernames: [user.username, admin.username].join(","),
}
expect(response.status).to eq(200)
expect(response.parsed_body["usernames"]).to contain_exactly(
user.username,
admin.username,
)
expect(group.group_users.where(owner: true).map(&:user)).to contain_exactly(user, admin)
end
end
end
describe "#join" do
let(:public_group) { Fabricate(:public_group) }