From: Jean-Philippe Lang <jp_lang@yahoo.fr>
Date: Sun, 12 Dec 2010 15:13:34 +0000 (+0000)
Subject: Extracts user groups assignment from controller.
X-Git-Tag: 1.1.0~97
X-Git-Url: https://source.dussan.org/?a=commitdiff_plain;h=0a2ec6ef0472fc0a14fe5793408c0546ebf39e4d;p=redmine.git

Extracts user groups assignment from controller.

git-svn-id: svn+ssh://rubyforge.org/var/svn/redmine/trunk@4499 e93f8b46-1217-0410-a6f0-8f06a7374b81
---

diff --git a/app/controllers/users_controller.rb b/app/controllers/users_controller.rb
index 85749920b..c3c63d736 100644
--- a/app/controllers/users_controller.rb
+++ b/app/controllers/users_controller.rb
@@ -145,7 +145,6 @@ class UsersController < ApplicationController
     if params[:user][:password].present? && (@user.auth_source_id.nil? || params[:user][:auth_source_id].blank?)
       @user.password, @user.password_confirmation = params[:user][:password], params[:user][:password_confirmation]
     end
-    @user.group_ids = params[:user][:group_ids] if params[:user][:group_ids]
     @user.safe_attributes = params[:user]
     # Was the account actived ? (do it before User#save clears the change)
     was_activated = (@user.status_change == [User::STATUS_REGISTERED, User::STATUS_ACTIVE])
diff --git a/app/models/user.rb b/app/models/user.rb
index f69604199..91d6c5fd0 100644
--- a/app/models/user.rb
+++ b/app/models/user.rb
@@ -60,7 +60,7 @@ class User < Principal
   attr_accessor :password, :password_confirmation
   attr_accessor :last_before_login_on
   # Prevents unauthorized assignments
-  attr_protected :login, :admin, :password, :password_confirmation, :hashed_password, :group_ids
+  attr_protected :login, :admin, :password, :password_confirmation, :hashed_password
 	
   validates_presence_of :login, :firstname, :lastname, :mail, :if => Proc.new { |user| !user.is_a?(AnonymousUser) }
   validates_uniqueness_of :login, :if => Proc.new { |user| !user.login.blank? }, :case_sensitive => false
@@ -407,6 +407,9 @@ class User < Principal
     'auth_source_id',
     :if => lambda {|user, current_user| current_user.admin?}
   
+  safe_attributes 'group_ids',
+    :if => lambda {|user, current_user| current_user.admin? && !user.new_record?}
+  
   # Utility method to help check if a user should be notified about an
   # event.
   #
diff --git a/test/functional/my_controller_test.rb b/test/functional/my_controller_test.rb
index 3fefa0675..69a5b3e13 100644
--- a/test/functional/my_controller_test.rb
+++ b/test/functional/my_controller_test.rb
@@ -64,17 +64,24 @@ class MyControllerTest < ActionController::TestCase
   end
 
   def test_update_account
-    post :account, :user => {:firstname => "Joe",
-                             :login => "root",
-                             :admin => 1,
-                             :custom_field_values => {"4" => "0100562500"}}
+    post :account,
+      :user => {
+        :firstname => "Joe",
+        :login => "root",
+        :admin => 1,
+        :group_ids => ['10'],
+        :custom_field_values => {"4" => "0100562500"}
+      }
+    
     assert_redirected_to '/my/account'
     user = User.find(2)
     assert_equal user, assigns(:user)
     assert_equal "Joe", user.firstname
     assert_equal "jsmith", user.login
     assert_equal "0100562500", user.custom_value_for(4).value
+    # ignored
     assert !user.admin?
+    assert user.groups.empty?
   end
   
   def test_change_password
diff --git a/test/functional/users_controller_test.rb b/test/functional/users_controller_test.rb
index 0b3231f93..c9c2c0ee3 100644
--- a/test/functional/users_controller_test.rb
+++ b/test/functional/users_controller_test.rb
@@ -183,6 +183,13 @@ class UsersControllerTest < ActionController::TestCase
     assert ActionMailer::Base.deliveries.empty?
   end
   
+  def test_update_with_group_ids_should_assign_groups
+    put :update, :id => 2, :user => {:group_ids => ['10']}
+    
+    user = User.find(2)
+    assert_equal [10], user.group_ids
+  end
+  
   def test_update_with_activation_should_send_a_notification
     u = User.new(:firstname => 'Foo', :lastname => 'Bar', :mail => 'foo.bar@somenet.foo', :language => 'fr')
     u.login = 'foo'