From c0800b330c858edbb961e71e54771ad30f9fee7b Mon Sep 17 00:00:00 2001 From: Jean-Philippe Lang Date: Sun, 21 Dec 2014 20:15:24 +0000 Subject: [PATCH] Error when adding user to group where he is already assigned (#18665). git-svn-id: http://svn.redmine.org/redmine/trunk@13785 e93f8b46-1217-0410-a6f0-8f06a7374b81 --- app/controllers/application_controller.rb | 12 +++++++----- app/controllers/groups_controller.rb | 12 +++++++++--- test/integration/api_test/groups_test.rb | 13 +++++++++++++ 3 files changed, 29 insertions(+), 8 deletions(-) diff --git a/app/controllers/application_controller.rb b/app/controllers/application_controller.rb index fbdb1d301..4b4e04cd0 100644 --- a/app/controllers/application_controller.rb +++ b/app/controllers/application_controller.rb @@ -637,12 +637,14 @@ class ApplicationController < ActionController::Base end # Renders API response on validation failure + # for an object or an array of objects def render_validation_errors(objects) - if objects.is_a?(Array) - @error_messages = objects.map {|object| object.errors.full_messages}.flatten - else - @error_messages = objects.errors.full_messages - end + messages = Array.wrap(objects).map {|object| object.errors.full_messages}.flatten + render_api_errors(messages) + end + + def render_api_errors(*messages) + @error_messages = messages.flatten render :template => 'common/error_messages.api', :status => :unprocessable_entity, :layout => nil end diff --git a/app/controllers/groups_controller.rb b/app/controllers/groups_controller.rb index 4a12b9b5b..5d82782ec 100644 --- a/app/controllers/groups_controller.rb +++ b/app/controllers/groups_controller.rb @@ -99,12 +99,18 @@ class GroupsController < ApplicationController end def add_users - @users = User.where(:id => (params[:user_id] || params[:user_ids])).to_a - @group.users << @users if request.post? + @users = User.not_in_group(@group).where(:id => (params[:user_id] || params[:user_ids])).to_a + @group.users << @users respond_to do |format| format.html { redirect_to edit_group_path(@group, :tab => 'users') } format.js - format.api { render_api_ok } + format.api { + if @users.any? + render_api_ok + else + render_api_errors "#{l(:label_user)} #{l('activerecord.errors.messages.invalid')}" + end + } end end diff --git a/test/integration/api_test/groups_test.rb b/test/integration/api_test/groups_test.rb index 2fe8271b1..087041219 100644 --- a/test/integration/api_test/groups_test.rb +++ b/test/integration/api_test/groups_test.rb @@ -185,6 +185,19 @@ class Redmine::ApiTest::GroupsTest < Redmine::ApiTest::Base assert_include User.find(5), Group.find(10).users end + test "POST /groups/:id/users.xml should not add the user if already added" do + Group.find(10).users << User.find(5) + + assert_no_difference 'Group.find(10).users.count' do + post '/groups/10/users.xml', {:user_id => 5}, credentials('admin') + assert_response :unprocessable_entity + end + + assert_select 'errors' do + assert_select 'error', :text => /User is invalid/ + end + end + test "DELETE /groups/:id/users/:user_id.xml should remove user from the group" do assert_difference 'Group.find(10).users.count', -1 do delete '/groups/10/users/8.xml', {}, credentials('admin') -- 2.39.5