Merged r13785 (#18665).

git-svn-id: http://svn.redmine.org/redmine/branches/2.6-stable@13802 e93f8b46-1217-0410-a6f0-8f06a7374b81

diff --git a/app/controllers/application_controller.rb b/app/controllers/application_controller.rb
index fbebfd5bdc59737d266d39590274c2598c26319b..1c22ff73d137d5571dafb208b7200bc1197946ae 100644 (file)
--- 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 555fc61df80960b9dcb112c266ef6211fc531355..e1aa04f421e908e8ca3bd6c68aed4b2a3eb58bbb 100644 (file)
--- a/app/controllers/groups_controller.rb
+++ b/app/controllers/groups_controller.rb
@@ -95,12 +95,18 @@ class GroupsController < ApplicationController
   end
 
   def add_users
-    @users = User.where(:id => (params[:user_id] || params[:user_ids])).all
-    @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 550eb50e50f19b50ca89cbf805ecb58b49187f80..e7cc2b2425e0ddac73120617de02565759e94ae8 100644 (file)
--- a/test/integration/api_test/groups_test.rb
+++ b/test/integration/api_test/groups_test.rb
@@ -189,6 +189,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')