Fix javascript Ajax when adding a user to the team

mlainez · Apr 3, 2011 · eb75945 · eb75945
1 parent 7c9a5a9
commit eb75945
Show file tree

Hide file tree

Showing 4 changed files with 52 additions and 26 deletions.
diff --git a/app/controllers/teams_controller.rb b/app/controllers/teams_controller.rb
@@ -67,8 +67,12 @@ def add_user
   # delete /organizations/:organization_id/teams/:id/users/:user_id
   def remove_user
     @team = @organization.teams.find(params[:id])
-    @team.users.delete(@user)
-    render :json => '', :status => :ok
+    if @team.users.include?(@user)
+      @team.users.delete(@user)
+      render :json => '', :status => :ok
+    else
+      render :json => ['user', 'has already been removed'], :status => :precondition_failed
+    end
   end
 
 private 

diff --git a/public/javascripts/helpers/teams.js b/public/javascripts/helpers/teams.js
@@ -101,7 +101,6 @@ Application.Helpers.Teams = {
   },
 
   addUser: function(user){
-    user.fade({duration: 0.2});
     var organizationId = user.readAttribute('data-organization-id');
     var teamId = user.readAttribute('data-team-id');
     var userId = user.readAttribute('data-user-id');
@@ -112,20 +111,29 @@ Application.Helpers.Teams = {
     // Add the picture
     var avatar = new Element('img', { src : userAvatar});
     var avatarContainer = new Element('div', {'class' : 'avatar'});
-    avatarContainer.insert({bottom: avatar})
-    userContainer.insert({bottom: avatarContainer});
-
-    // Add the nametag
     var nametagContainer = new Element('div', {'class' : 'name'}).update(userNametag);
-    userContainer.insert({bottom: nametagContainer});
-
-    // Add Remove Button
-    userContainer.insert({bottom: new Element('div', {'class': 'remove', 'title' : 'Remove From Team'})})
 
-    userContainer.hide();
-    $('team_users').insert({bottom: userContainer});
-    userContainer.appear();
-    new Ajax.Request('/organizations/'+organizationId+'/teams/'+teamId+'/users/'+userId, {method: 'post'});
+    new Ajax.Request('/organizations/'+organizationId+'/teams/'+teamId+'/users/'+userId, 
+      {method: 'post',
+        onSuccess: function(transport){
+          if(200 == transport.status){
+            user.fade({duration: 0.2});
+            avatarContainer.insert({bottom: avatar})
+            userContainer.insert({bottom: avatarContainer});
+
+            // Add the nametag
+            userContainer.insert({bottom: nametagContainer});
+
+            // Add Remove Button
+            userContainer.insert({bottom: new Element('div', {'class': 'remove', 'title' : 'Remove From Team'})})
+
+            userContainer.hide();
+            $('team_users').insert({bottom: userContainer});
+            userContainer.appear();
+          }
+        }
+      }
+    );
   },
 
   removeUser: function(user){
@@ -140,16 +148,21 @@ Application.Helpers.Teams = {
 
     // Add the name
     var nameContainer = new Element('div', {'class' : 'name'}).update(userName);
-    userContainer.insert({bottom: nameContainer});
-
-    // Add Add Button
-    userContainer.insert({bottom: new Element('div', {'class': 'add'}).update('add to team')})
 
-    userContainer.hide();
-    $('organization_users').insert({bottom: userContainer});
-    userContainer.appear();
-
-    new Ajax.Request('/organizations/'+organizationId+'/teams/'+teamId+'/users/'+userId, {method: 'delete'});
+    new Ajax.Request('/organizations/'+organizationId+'/teams/'+teamId+'/users/'+userId, {
+      method: 'delete',
+      onSuccess: function(transport){
+        if(200 == transport.status){
+          userContainer.insert({bottom: nameContainer});
+
+          // Add Add Button
+          userContainer.insert({bottom: new Element('div', {'class': 'add'}).update('add to team')})
+
+          userContainer.hide();
+          $('organization_users').insert({bottom: userContainer});
+          userContainer.appear();
+        }
+      }});
   },
 
   _setupListeners: function(){

diff --git a/test/functional/teams_controller_test.rb b/test/functional/teams_controller_test.rb
@@ -243,6 +243,15 @@ class TeamsControllerTest < ActionController::TestCase
       should "remove the user from the team" do
         assert !@team.users.include?(@user2)
       end
+      context "and remove it again" do
+        setup do
+          delete :remove_user, :id => @team.to_param, :organization_id => @organization.to_param, :user_id => @user2.to_param
+        end
+        should_respond_with :precondition_failed
+        should "give an error" do
+          assert_match 'has already been removed', @response.body
+        end
+      end
 
     end
 

diff --git a/test/shoulda_macros/should_require_belong_to_project_or_admin_on.rb b/test/shoulda_macros/should_require_belong_to_project_or_admin_on.rb
@@ -26,7 +26,7 @@ def should_require_belong_to_project_or_admin_on(actions = [])
         context "on #{method.to_s.upcase} to :#{action}" do
           setup do
             if need_ids.include?(action)
-              send(method, action, :project_id => @project.to_param, :id => 1)
+              send(method, action, :project_id => @project.to_param, :id => Project.first.id)
             else
               send(method, action, :project_id => @project.to_param)
             end
@@ -59,7 +59,7 @@ def should_require_belong_to_project_or_admin_on(actions = [])
         context "on #{method.to_s.upcase} to :#{action}" do
           setup do
             if need_ids.include?(action)
-              send(method, action, :project_id => @project.to_param, :id => 1)
+              send(method, action, :project_id => @project.to_param, :id => Project.first.id)
             else
               send(method, action, :project_id => @project.to_param)
             end