diff --git a/app/controllers/groups_controller.rb b/app/controllers/groups_controller.rb index ff14238113e..54906c2d611 100644 --- a/app/controllers/groups_controller.rb +++ b/app/controllers/groups_controller.rb @@ -240,20 +240,20 @@ class GroupsController < ApplicationController group = Group.find(params[:id]) group.public_admission ? ensure_logged_in : guardian.ensure_can_edit!(group) - users = - if params[:usernames].present? - User.where(username: params[:usernames].split(",")) - elsif params[:user_ids].present? - User.find(params[:user_ids].split(",")) - elsif params[:user_emails].present? - User.with_email(params[:user_emails].split(",")) - else - raise Discourse::InvalidParameters.new( - 'user_ids or usernames or user_emails must be present' - ) - end - - raise Discourse::NotFound if users.blank? + if params[:usernames].present? + users = User.where(username: params[:usernames].split(",")) + raise Discourse::InvalidParameters.new(:usernames) if users.blank? + elsif params[:user_ids].present? + users = User.where(id: params[:user_ids].split(",")) + raise Discourse::InvalidParameters.new(:user_ids) if users.blank? + elsif params[:user_emails].present? + users = User.with_email(params[:user_emails].split(",")) + raise Discourse::InvalidParameters.new(:user_emails) if users.blank? + else + raise Discourse::InvalidParameters.new( + 'user_ids or usernames or user_emails must be present' + ) + end if group.public_admission if !guardian.can_log_group_changes?(group) && current_user != users.first diff --git a/spec/requests/groups_controller_spec.rb b/spec/requests/groups_controller_spec.rb index d1279fe23fd..8a25aea5fd1 100644 --- a/spec/requests/groups_controller_spec.rb +++ b/spec/requests/groups_controller_spec.rb @@ -864,10 +864,20 @@ describe GroupsController do )) end - it "returns 404 if member is not found" do - put "/groups/#{group.id}/members.json", params: { usernames: 'some donkey' } + it "returns 400 if member is not found" do + [ + { usernames: "some thing" }, + { user_ids: "-5,-6" }, + { user_emails: "some@test.org" } + ].each do |params| + put "/groups/#{group.id}/members.json", params: params - expect(response.status).to eq(404) + expect(response.status).to eq(400) + + body = JSON.parse(response.body) + + expect(body["error_type"]).to eq("invalid_parameters") + end end context 'public group' do