From ce6436b98a8a86356ee93e6eaf136d27d7f77b93 Mon Sep 17 00:00:00 2001
From: Felix Gilcher <felix.gilcher@asquera.de>
Date: Fri, 1 Feb 2013 13:53:35 +0000
Subject: [PATCH] Don't crash when removing a user that's not project member

The attempt to revoke project access for a user that was not member of the
project results in a 500 Internal Server error where it actually should
result in a 200 OK since after the operation, the user is not member of
the project. This turns the operation into an idempotent call that can
be repeated with no ill effects.

Updated the spec and changed the code accordingly. However, the result differs
slightly, as we can't return the users project access level if the user was not
member. I'm not aware if anybody relies on the result of this call.

Fixes #2832
---
 lib/api/projects.rb                |  6 +++++-
 spec/requests/api/projects_spec.rb | 11 +++++++++++
 2 files changed, 16 insertions(+), 1 deletion(-)

diff --git a/lib/api/projects.rb b/lib/api/projects.rb
index cbef1ed3..5444ba6a 100644
--- a/lib/api/projects.rb
+++ b/lib/api/projects.rb
@@ -132,7 +132,11 @@ module Gitlab
       delete ":id/members/:user_id" do
         authorize! :admin_project, user_project
         users_project = user_project.users_projects.find_by_user_id params[:user_id]
-        users_project.destroy
+        unless users_project.nil?
+          users_project.destroy
+        else
+          {:message => "Access revoked", :id => params[:user_id].to_i}
+        end
       end
 
       # Get project hooks
diff --git a/spec/requests/api/projects_spec.rb b/spec/requests/api/projects_spec.rb
index c2244210..8351b4bf 100644
--- a/spec/requests/api/projects_spec.rb
+++ b/spec/requests/api/projects_spec.rb
@@ -167,6 +167,17 @@ describe Gitlab::API do
     end
   end
 
+  describe "DELETE /projects/:id/members/:user_id" do
+    it "should return 200 OK when the user was not member" do
+      expect {
+        delete api("/projects/#{project.id}/members/1000000", user)
+      }.to change { UsersProject.count }.by(0)
+      response.status.should == 200
+      json_response['message'].should == "Access revoked"
+      json_response['id'].should == 1000000
+    end
+  end
+
   describe "GET /projects/:id/hooks" do
     it "should return project hooks" do
       get api("/projects/#{project.id}/hooks", user)