6 files changed, 46 insertions, 30 deletions
diff --git a/app/controllers/groups_controller.rb b/app/controllers/groups_controller.rb
index e4613b5e3..2e57d179a 100644
--- a/app/controllers/groups_controller.rb
+++ b/app/controllers/groups_controller.rb
@@ -90,16 +90,11 @@ class GroupsController < ApplicationController
   end
 
   def add_users
-    users = User.find_all_by_id(params[:user_id] || params[:user_ids])
-    @group.users << users if request.post?
+    @users = User.find_all_by_id(params[:user_id] || params[:user_ids])
+    @group.users << @users if request.post?
     respond_to do |format|
       format.html { redirect_to :controller => 'groups', :action => 'edit', :id => @group, :tab => 'users' }
-      format.js {
-        render(:update) {|page|
-          page.replace_html "tab-content-users", :partial => 'groups/users'
-          users.each {|user| page.visual_effect(:highlight, "user-#{user.id}") }
-        }
-      }
+      format.js
       format.api { render_api_ok }
     end
   end
@@ -108,7 +103,7 @@ class GroupsController < ApplicationController
     @group.users.delete(User.find(params[:user_id])) if request.delete?
     respond_to do |format|
       format.html { redirect_to :controller => 'groups', :action => 'edit', :id => @group, :tab => 'users' }
-      format.js { render(:update) {|page| page.replace_html "tab-content-users", :partial => 'groups/users'} }
+      format.js
       format.api { render_api_ok }
     end
   end
@@ -122,21 +117,8 @@ class GroupsController < ApplicationController
     @membership = Member.edit_membership(params[:membership_id], params[:membership], @group)
     @membership.save if request.post?
     respond_to do |format|
-      if @membership.valid?
-        format.html { redirect_to :controller => 'groups', :action => 'edit', :id => @group, :tab => 'memberships' }
-        format.js {
-          render(:update) {|page|
-            page.replace_html "tab-content-memberships", :partial => 'groups/memberships'
-            page.visual_effect(:highlight, "member-#{@membership.id}")
-          }
-        }
-      else
-        format.js {
-          render(:update) {|page|
-            page.alert(l(:notice_failed_to_save_members, :errors => @membership.errors.full_messages.join(', ')))
-          }
-        }
-      end
+      format.html { redirect_to :controller => 'groups', :action => 'edit', :id => @group, :tab => 'memberships' }
+      format.js
     end
   end
 
@@ -144,7 +126,7 @@ class GroupsController < ApplicationController
     Member.find(params[:membership_id]).destroy if request.post?
     respond_to do |format|
       format.html { redirect_to :controller => 'groups', :action => 'edit', :id => @group, :tab => 'memberships' }
-      format.js { render(:update) {|page| page.replace_html "tab-content-memberships", :partial => 'groups/memberships'} }
+      format.js
     end
   end
 
diff --git a/app/views/groups/add_users.js.erb b/app/views/groups/add_users.js.erb
new file mode 100644
index 000000000..35d432760
--- /dev/null
+++ b/app/views/groups/add_users.js.erb
@@ -0,0 +1,4 @@
+Element.update('tab-content-users', '<%= escape_javascript(render :partial => 'groups/users') %>');
+<% @users.each do |user| %>
+  new Effect.Highlight('user-<%= user.id %>');
+<% end %>
diff --git a/app/views/groups/destroy_membership.js.erb b/app/views/groups/destroy_membership.js.erb
new file mode 100644
index 000000000..3e03eb817
--- /dev/null
+++ b/app/views/groups/destroy_membership.js.erb
@@ -0,0 +1 @@
+Element.update('tab-content-memberships', '<%= escape_javascript(render :partial => 'groups/memberships') %>');
diff --git a/app/views/groups/edit_membership.js.erb b/app/views/groups/edit_membership.js.erb
new file mode 100644
index 000000000..b693a7f90
--- /dev/null
+++ b/app/views/groups/edit_membership.js.erb
@@ -0,0 +1,6 @@
+<% if @membership.valid? %>
+  Element.update('tab-content-memberships', '<%= escape_javascript(render :partial => 'groups/memberships') %>');
+  new Effect.Highlight('member-<%= @membership.id %>');
+<% else %>
+  alert('<%= escape_javascript(l(:notice_failed_to_save_members, :errors => @membership.errors.full_messages.join(', '))) %>');
+<% end %>
diff --git a/app/views/groups/remove_user.js.erb b/app/views/groups/remove_user.js.erb
new file mode 100644
index 000000000..b383830b2
--- /dev/null
+++ b/app/views/groups/remove_user.js.erb
@@ -0,0 +1 @@
+Element.update('tab-content-users', '<%= escape_javascript(render :partial => 'groups/users') %>');
diff --git a/test/functional/groups_controller_test.rb b/test/functional/groups_controller_test.rb
index 4d5270145..d76a952a5 100644
--- a/test/functional/groups_controller_test.rb
+++ b/test/functional/groups_controller_test.rb
@@ -113,8 +113,11 @@ class GroupsControllerTest < ActionController::TestCase
   def test_xhr_add_users
     assert_difference 'Group.find(10).users.count', 2 do
       xhr :post, :add_users, :id => 10, :user_ids => ['2', '3']
+      assert_response :success
+      assert_template 'add_users'
+      assert_equal 'text/javascript', response.content_type
     end
-    assert_select_rjs :replace_html, 'tab-content-users'
+    assert_match /John Smith/, response.body
   end
 
   def test_remove_user
@@ -126,8 +129,10 @@ class GroupsControllerTest < ActionController::TestCase
   def test_xhr_remove_user
     assert_difference 'Group.find(10).users.count', -1 do
       xhr :delete, :remove_user, :id => 10, :user_id => '8'
+      assert_response :success
+      assert_template 'remove_user'
+      assert_equal 'text/javascript', response.content_type
     end
-    assert_select_rjs :replace_html, 'tab-content-users'
   end
 
   def test_new_membership
@@ -139,15 +144,21 @@ class GroupsControllerTest < ActionController::TestCase
   def test_xhr_new_membership
     assert_difference 'Group.find(10).members.count' do
       xhr :post, :edit_membership, :id => 10, :membership => { :project_id => 2, :role_ids => ['1', '2']}
+      assert_response :success
+      assert_template 'edit_membership'
+      assert_equal 'text/javascript', response.content_type
     end
-    assert_select_rjs :replace_html, 'tab-content-memberships'
+    assert_match /OnlineStore/, response.body
   end
 
   def test_xhr_new_membership_with_failure
     assert_no_difference 'Group.find(10).members.count' do
       xhr :post, :edit_membership, :id => 10, :membership => { :project_id => 999, :role_ids => ['1', '2']}
+      assert_response :success
+      assert_template 'edit_membership'
+      assert_equal 'text/javascript', response.content_type
     end
-    assert @response.body.match(/alert/i), "Alert message not sent"
+    assert_match /alert/, response.body, "Alert message not sent"
   end
 
   def test_edit_membership
@@ -156,6 +167,15 @@ class GroupsControllerTest < ActionController::TestCase
     end
   end
 
+  def test_xhr_edit_membership
+    assert_no_difference 'Group.find(10).members.count' do
+      xhr :post, :edit_membership, :id => 10, :membership_id => 6, :membership => { :role_ids => ['1', '3']}
+      assert_response :success
+      assert_template 'edit_membership'
+      assert_equal 'text/javascript', response.content_type
+    end
+  end
+
   def test_destroy_membership
     assert_difference 'Group.find(10).members.count', -1 do
       post :destroy_membership, :id => 10, :membership_id => 6
@@ -165,8 +185,10 @@ class GroupsControllerTest < ActionController::TestCase
   def test_xhr_destroy_membership
     assert_difference 'Group.find(10).members.count', -1 do
       xhr :post, :destroy_membership, :id => 10, :membership_id => 6
+      assert_response :success
+      assert_template 'destroy_membership'
+      assert_equal 'text/javascript', response.content_type
     end
-    assert_select_rjs :replace_html, 'tab-content-memberships'
   end
 
   def test_autocomplete_for_user