Removing people too that don't belong

Signed-off-by: Nick Quaranto <nick@quaran.to>

features/manage_collaborators.feature

@@ -25,3 +25,16 @@ Feature: Manage collaborators
     Then the GitHub API should have received a request to add a "rmmt" as a collaborator for "shoulda"
     And the GitHub API should have received a request to add a "coreyhaines" as a collaborator for "shoulda"
     And the GitHub API should have received a request to add a "qrush" as a collaborator for "shoulda"
+
+  Scenario: Removing one collaborator from the project
+    Given "qrush" is a collaborator for "shoulda"
+    When I execute the following code
+     """
+     Enforcer "thoughtbot", "deadbeef" do
+       project "shoulda" do
+         collaborators 'coreyhaines'
+       end
+     end
+     """
+    Then the GitHub API should have received a request to add a "coreyhaines" as a collaborator for "shoulda"
+    And the GitHub API should have received a request to remove "qrush" as a collaborator for "shoulda"

features/step_definitions/enforcer_steps.rb

@@ -1,5 +1,8 @@
 Before do
   stub(GitHubApi).add_collaborator(anything, anything, anything, anything)
+  stub(GitHubApi).remove_collaborator(anything, anything, anything, anything)
+  @existing_collaborators = []
+  stub(GitHubApi).list_collaborators(anything, anything) { @existing_collaborators }
 end
 
 
@@ -27,3 +30,10 @@
   assert_received(GitHubApi) { |subject| subject.add_collaborator(@account, @api_key, repo, user) }
 end
 
+Given /^"([^\"]*)" is a collaborator for "([^\"]*)"$/ do |user, repo|
+  @existing_collaborators << user
+end
+
+Then /^the GitHub API should have received a request to remove "([^\"]*)" as a collaborator for "([^\"]*)"$/ do |user, repo|
+  assert_received(GitHubApi) { |subject| subject.remove_collaborator(@account, @api_key, repo, user) }
+end

lib/enforcer.rb

@@ -11,8 +11,17 @@ def project(project_name, &block)
     instance_eval(&block)
     return if @collaborators.nil?
 
-    @collaborators.each do |collaborator|
-      GitHubApi.add_collaborator(@account_name, @api_key, project_name, collaborator)
+    existing_collaborators = GitHubApi.list_collaborators(@account_name, project_name)
+
+    { :add => @collaborators - existing_collaborators,
+      :remove => existing_collaborators - @collaborators}.each_pair do |action, collaborators|
+        modify_collaborators action, project_name, collaborators
+      end
+  end
+
+  def modify_collaborators(action, project_name, these)
+    these.each do |collaborator|
+      GitHubApi.send("#{action}_collaborator", @account_name, @api_key, project_name, collaborator)
     end
   end
 

lib/github_api.rb

@@ -9,4 +9,14 @@ def self.add_collaborator(account, api_key, repo, collaborator)
     response = self.post "/repos/collaborators/#{repo}/add/#{collaborator}", :body => { :login => account, :token => api_key }
     response['collaborators']
   end
+
+  def self.remove_collaborator(account, api_key, repo, collaborator)
+    response = self.post "/repos/collaborators/#{repo}/remove/#{collaborator}", :body => { :login => account, :token => api_key }
+    response['collaborators']
+  end
+
+  def self.list_collaborators(account, repo)
+    response = self.get "/repos/show/#{account}/#{repo}/collaborators"
+    response['collaborators']
+  end
 end

test/enforcer_test.rb

@@ -4,6 +4,9 @@ class EnforcerTest < Test::Unit::TestCase
   context "with an enforcer" do
     setup do
       stub(GitHubApi).add_collaborator(anything, anything, anything, anything)
+      @existing_collaborators = []
+      stub(GitHubApi).list_collaborators(anything, anything) { @existing_collaborators }
+
       @username = "user"
       @api_key = "api key"
       @enforcer = Enforcer.new(@username, @api_key)
@@ -29,6 +32,36 @@ class EnforcerTest < Test::Unit::TestCase
         subject.add_collaborator(@username, @api_key, 'foo', 'qrush')
       end
     end
+
+    context "with an existing user" do
+      setup do
+        @existing_user = "ralph"
+        @repo          = "repo"
+        stub(GitHubApi).remove_collaborator(anything, anything, anything, anything)
+        @existing_collaborators << "ralph"
+      end
+
+      should "not add existing user to the project" do
+        @enforcer.project 'foo' do
+          collaborators 'ralph'
+        end
+
+        assert_received(GitHubApi) do |subject|
+          subject.add_collaborator(@username, @api_key, 'foo', 'ralph').never
+        end
+      end
+
+      should "remove existing user from the project if not in collaborators" do
+        @enforcer.project 'foo' do
+          collaborators 'qrush'
+        end
+
+        assert_received(GitHubApi) do |subject|
+          subject.add_collaborator(@username, @api_key, 'foo', 'qrush')
+          subject.remove_collaborator(@username, @api_key, 'foo', @existing_user)
+        end
+      end
+    end
   end
 
   context "setting up enforcer dsl" do

test/github_api_test.rb

@@ -7,16 +7,27 @@ class GitHubApiTest < Test::Unit::TestCase
       @api_key = "deadbeef"
       @repo    = "repo"
       @user    = "coreyhaines" + rand(10).to_s
-      @collaborators = ["qrush", @user]
-      stub(GitHubApi).post(anything, anything) { {'collaborators' => @collaborators }}
     end
 
     should "set base uri" do
       assert_equal "http://github.com/api/v2/json", GitHubApi.base_uri
     end
 
+    context "listing collaborators" do
+      setup do
+        @collaborators = ["qrush", "coreyhaines"]
+        stub(GitHubApi).get("/repos/show/#{@account}/#{@repo}/collaborators") { { 'collaborators' => @collaborators } }
+      end
+
+      should "return just the usernames" do
+        assert_equal @collaborators, GitHubApi.list_collaborators(@account, @repo)
+      end
+    end
+
     context "adding a collaborator" do
       setup do
+        @collaborators = ["qrush", @user]
+        stub(GitHubApi).post(anything, anything) { {'collaborators' => @collaborators }}
         @collaborators = GitHubApi.add_collaborator(@account, @api_key, @repo, @user)
       end
 
@@ -33,6 +44,23 @@ class GitHubApiTest < Test::Unit::TestCase
       end
     end
   end
+  context "removing a collaborator" do
+    setup do
+      @collaborators = ["qrush"]
+      stub(GitHubApi).post(anything, anything) { {'collaborators' => @collaborators }}
+      @collaborators = GitHubApi.remove_collaborator(@account, @api_key, @repo, @user)
+    end
 
-end
+    should "hit the github api" do
+      assert_received(GitHubApi) do |subject|
+        subject.post("/repos/collaborators/#{@repo}/remove/#{@user}", :body => {
+          :login => @account,
+          :token => @api_key})
+      end
+    end
 
+    should "return the new collaborators" do
+      assert_equal ["qrush"], @collaborators
+    end
+  end
+end

test/integration_test.rb → test/integration.rb

@@ -5,8 +5,8 @@
 
 token = `git config github.token`
 
-Enforcer "qrush", token do
+Enforcer "qrush", token.chomp do
   project "jekyll" do
-    collaborators "coreyhaines"
+    collaborators "qrush"
   end
 end