From 0d34c3731b4df639df760851302ab1809f1fcec5 Mon Sep 17 00:00:00 2001
From: Eric Davis <edavis@littlestreamsoftware.com>
Date: Mon, 24 May 2010 20:21:16 +0000
Subject: [PATCH] Add error messages to MembersController#new so a user is
 informed of missing data.

git-svn-id: svn+ssh://rubyforge.org/var/svn/redmine/trunk@3749 e93f8b46-1217-0410-a6f0-8f06a7374b81
---
 app/controllers/members_controller.rb      | 28 ++++++++++++++----
 config/locales/en.yml                      |  2 ++
 test/functional/members_controller_test.rb | 34 ++++++++++++++++++++++
 3 files changed, 58 insertions(+), 6 deletions(-)

diff --git a/app/controllers/members_controller.rb b/app/controllers/members_controller.rb
index 6de7f82d0..a84cf50db 100644
--- a/app/controllers/members_controller.rb
+++ b/app/controllers/members_controller.rb
@@ -36,13 +36,29 @@ class MembersController < ApplicationController
       @project.members << members
     end
     respond_to do |format|
-      format.html { redirect_to :controller => 'projects', :action => 'settings', :tab => 'members', :id => @project }
-      format.js { 
-        render(:update) {|page| 
-          page.replace_html "tab-content-members", :partial => 'projects/settings/members'
-          members.each {|member| page.visual_effect(:highlight, "member-#{member.id}") }
+      if members.present? && members.all? {|m| m.valid? }
+
+        format.html { redirect_to :controller => 'projects', :action => 'settings', :tab => 'members', :id => @project }
+
+        format.js { 
+          render(:update) {|page| 
+            page.replace_html "tab-content-members", :partial => 'projects/settings/members'
+            members.each {|member| page.visual_effect(:highlight, "member-#{member.id}") }
+          }
         }
-      }
+      else
+
+        format.js {
+          render(:update) {|page|
+            errors = members.collect {|m|
+              m.errors.full_messages
+            }.flatten.uniq
+
+            page.alert(l(:notice_failed_to_save_members, :errors => errors.join(', ')))
+          }
+        }
+        
+      end
     end
   end
   
diff --git a/config/locales/en.yml b/config/locales/en.yml
index 81d744f93..756010987 100644
--- a/config/locales/en.yml
+++ b/config/locales/en.yml
@@ -148,6 +148,7 @@ en:
   notice_feeds_access_key_reseted: Your RSS access key was reset.
   notice_api_access_key_reseted: Your API access key was reset.
   notice_failed_to_save_issues: "Failed to save {{count}} issue(s) on {{total}} selected: {{ids}}."
+  notice_failed_to_save_members: "Failed to save member(s): {{errors}}."
   notice_no_issue_selected: "No issue is selected! Please, check the issues you want to edit."
   notice_account_pending: "Your account was created and is now pending administrator approval."
   notice_default_data_loaded: Default configuration successfully loaded.
@@ -226,6 +227,7 @@ en:
   field_priority: Priority
   field_fixed_version: Target version
   field_user: User
+  field_principal: Principal
   field_role: Role
   field_homepage: Homepage
   field_is_public: Public
diff --git a/test/functional/members_controller_test.rb b/test/functional/members_controller_test.rb
index 612fcdda9..213a01730 100644
--- a/test/functional/members_controller_test.rb
+++ b/test/functional/members_controller_test.rb
@@ -48,6 +48,40 @@ class MembersControllerTest < ActionController::TestCase
     assert_redirected_to '/projects/ecookbook/settings/members'
     assert User.find(7).member_of?(Project.find(1))
   end
+
+  context "post :new in JS format" do
+    context "with successful saves" do
+      should "add membership for each user" do
+        post :new, :format => "js", :id => 1, :member => {:role_ids => [1], :user_ids => [7, 8, 9]}
+
+        assert User.find(7).member_of?(Project.find(1))
+        assert User.find(8).member_of?(Project.find(1))
+        assert User.find(9).member_of?(Project.find(1))
+      end
+      
+      should "replace the tab with RJS" do
+        post :new, :format => "js", :id => 1, :member => {:role_ids => [1], :user_ids => [7, 8, 9]}
+
+        assert_select_rjs :replace_html, 'tab-content-members'
+      end
+      
+    end
+
+    context "with a failed save" do
+      should "not replace the tab with RJS" do
+        post :new, :format => "js", :id => 1, :member => {:role_ids => [], :user_ids => [7, 8, 9]}
+
+        assert_select '#tab-content-members', 0
+      end
+      
+      should "open an error message" do
+        post :new, :format => "js", :id => 1, :member => {:role_ids => [], :user_ids => [7, 8, 9]}
+
+        assert @response.body.match(/alert/i), "Alert message not sent"
+      end
+    end
+
+  end
   
   def test_edit
     assert_no_difference 'Member.count' do