DEV: Show failed to remove members from bulk groups api

Before this commit if you were bulk removing group members and passed in
a user who wasn't currently a member of that group the whole request
would fail. This change will return a 200 response now listing the users
that were removed and those that were skipped.
This commit is contained in:
Blake Erickson 2020-07-22 14:27:43 -06:00
parent 1100b3d185
commit 395d17e2ac
2 changed files with 23 additions and 5 deletions

View File

@ -412,16 +412,25 @@ class GroupsController < ApplicationController
end
end
removed_users = []
skipped_users = []
users.each do |user|
if group.remove(user)
removed_users << user.username
GroupActionLogger.new(current_user, group).log_remove_user_from_group(user)
else
if group.users.exclude? user
skipped_users << user.username
else
raise Discourse::InvalidParameters
end
end
end
render json: success_json.merge!(
usernames: users.map(&:username)
usernames: removed_users,
skipped_usernames: skipped_users
)
end

View File

@ -1191,12 +1191,18 @@ describe GroupsController do
it "raises an error if user to be removed is not found" do
delete "/groups/#{group.id}/members.json", params: { user_id: -10 }
response_body = response.parsed_body
expect(response.status).to eq(400)
end
it "raises an error when removing a valid user but is not a member of that group" do
it "returns skipped_usernames response body when removing a valid user but is not a member of that group" do
delete "/groups/#{group.id}/members.json", params: { user_id: -1 }
expect(response.status).to eq(400)
response_body = response.parsed_body
expect(response.status).to eq(200)
expect(response_body["usernames"]).to eq([])
expect(response_body["skipped_usernames"].first).to eq("system")
end
context "is able to remove a member" do
@ -1324,7 +1330,10 @@ describe GroupsController do
delete "/groups/#{group1.id}/members.json",
params: { usernames: [user.username, user2.username].join(",") }
expect(response.status).to eq(400)
response_body = response.parsed_body
expect(response.status).to eq(200)
expect(response_body["usernames"].first).to eq(user2.username)
expect(response_body["skipped_usernames"].first).to eq(user.username)
end
end
end