From 0c80b05fd65bcbfcdec127da62a243471df94fd4 Mon Sep 17 00:00:00 2001 From: Corey Haines Date: Mon, 13 Jul 2009 17:53:41 -0400 Subject: [PATCH] Removing people too that don't belong Signed-off-by: Nick Quaranto --- features/manage_collaborators.feature | 13 ++++++++ features/step_definitions/enforcer_steps.rb | 10 ++++++ lib/enforcer.rb | 13 ++++++-- lib/github_api.rb | 10 ++++++ test/enforcer_test.rb | 33 +++++++++++++++++++ test/github_api_test.rb | 34 ++++++++++++++++++-- test/{integration_test.rb => integration.rb} | 4 +-- 7 files changed, 110 insertions(+), 7 deletions(-) rename test/{integration_test.rb => integration.rb} (68%) diff --git a/features/manage_collaborators.feature b/features/manage_collaborators.feature index e479147..20c511e 100644 --- a/features/manage_collaborators.feature +++ b/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" diff --git a/features/step_definitions/enforcer_steps.rb b/features/step_definitions/enforcer_steps.rb index db42012..d9b35fe 100644 --- a/features/step_definitions/enforcer_steps.rb +++ b/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 diff --git a/lib/enforcer.rb b/lib/enforcer.rb index dc8320a..485d69a 100644 --- a/lib/enforcer.rb +++ b/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 diff --git a/lib/github_api.rb b/lib/github_api.rb index 57926a5..40f4737 100644 --- a/lib/github_api.rb +++ b/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 diff --git a/test/enforcer_test.rb b/test/enforcer_test.rb index a9016d9..223ec75 100644 --- a/test/enforcer_test.rb +++ b/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 diff --git a/test/github_api_test.rb b/test/github_api_test.rb index 4ae9fe0..d114f95 100644 --- a/test/github_api_test.rb +++ b/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 diff --git a/test/integration_test.rb b/test/integration.rb similarity index 68% rename from test/integration_test.rb rename to test/integration.rb index 6b896b7..baa456e 100755 --- a/test/integration_test.rb +++ b/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