From 2e0c77506e7831e22907833fb163a7cd8c31c243 Mon Sep 17 00:00:00 2001 From: Alex Sanford Date: Fri, 24 Feb 2017 08:33:44 -0400 Subject: [PATCH 1/4] Add condition for printing merge request urls By default, the URL to view/create a merge request will be displayed on push. If, however, the GitLab API specifies that `disable_printing_merge_request_link` is true, the link will not be printed. --- lib/gitlab_net.rb | 5 ++ lib/gitlab_post_receive.rb | 9 ++- spec/gitlab_net_spec.rb | 11 ++++ spec/gitlab_post_receive_spec.rb | 76 ++++++++++++++++++------- spec/vcr_cassettes/project-settings.yml | 46 +++++++++++++++ 5 files changed, 122 insertions(+), 25 deletions(-) create mode 100644 spec/vcr_cassettes/project-settings.yml diff --git a/lib/gitlab_net.rb b/lib/gitlab_net.rb index 438c626b..7a48d10a 100644 --- a/lib/gitlab_net.rb +++ b/lib/gitlab_net.rb @@ -73,6 +73,11 @@ class GitlabNet JSON.parse(resp.body) rescue [] end + def project_settings(repo_path) + resp = get("#{host_v3}/project_settings?project=#{URI.escape(repo_path)}") + JSON.parse(resp.body) rescue {} + end + def check get("#{host_v3}/check", read_timeout: CHECK_TIMEOUT) end diff --git a/lib/gitlab_post_receive.rb b/lib/gitlab_post_receive.rb index 8383135a..5f998d54 100644 --- a/lib/gitlab_post_receive.rb +++ b/lib/gitlab_post_receive.rb @@ -31,10 +31,13 @@ class GitlabPostReceive print_broadcast_message(broadcast_message["message"]) end - merge_request_urls = GitlabMetrics.measure("merge-request-urls") do - api.merge_request_urls(@repo_path, @changes) + project_settings = api.project_settings(@repo_path) + unless project_settings['disable_printing_merge_request_link'] + merge_request_urls = GitlabMetrics.measure("merge-request-urls") do + api.merge_request_urls(@repo_path, @changes) + end + print_merge_request_links(merge_request_urls) end - print_merge_request_links(merge_request_urls) api.notify_post_receive(repo_path) rescue GitlabNet::ApiUnreachableError diff --git a/spec/gitlab_net_spec.rb b/spec/gitlab_net_spec.rb index 1b64567a..4651195f 100644 --- a/spec/gitlab_net_spec.rb +++ b/spec/gitlab_net_spec.rb @@ -91,6 +91,17 @@ describe GitlabNet, vcr: true do end end + describe :project_settings do + let(:repo_path) { 'gitlab-org/gitlab-ce.git' } + + it 'should return project settings hash' do + VCR.use_cassette('project-settings') do + result = gitlab_net.project_settings(repo_path) + result['disable_printing_merge_request_link'].should be(true) + end + end + end + describe :authorized_key do let (:ssh_key) { "AAAAB3NzaC1yc2EAAAADAQABAAACAQDPKPqqnqQ9PDFw65cO7iHXrKw6ucSZg8Bd2CZ150Yy1YRDPJOWeRNCnddS+M/Lk" } diff --git a/spec/gitlab_post_receive_spec.rb b/spec/gitlab_post_receive_spec.rb index c63c2673..48884690 100644 --- a/spec/gitlab_post_receive_spec.rb +++ b/spec/gitlab_post_receive_spec.rb @@ -12,12 +12,14 @@ describe GitlabPostReceive do let(:repo_path) { File.join(repository_path, repo_name) + ".git" } let(:gitlab_post_receive) { GitlabPostReceive.new(repo_path, actor, wrongly_encoded_changes) } let(:message) { "test " * 10 + "message " * 10 } + let(:project_settings) { { 'disable_printing_merge_request_link' => false } } let(:redis_client) { double('redis_client') } let(:enqueued_at) { Time.new(2016, 6, 23, 6, 59) } before do GitlabConfig.any_instance.stub(repos_path: repository_path) GitlabNet.any_instance.stub(broadcast_message: { }) + GitlabNet.any_instance.stub(:project_settings).with(repo_path) { project_settings } GitlabNet.any_instance.stub(:merge_request_urls).with(repo_path, wrongly_encoded_changes) { [] } GitlabNet.any_instance.stub(notify_post_receive: true) expect(Time).to receive(:now).and_return(enqueued_at) @@ -45,19 +47,34 @@ describe GitlabPostReceive do end end - it "prints the new merge request url" do - expect(redis_client).to receive(:rpush) + context 'when printing merge request urls is not disabled' do + it "prints the new merge request url" do + expect(redis_client).to receive(:rpush) - expect(gitlab_post_receive).to receive(:puts).ordered - expect(gitlab_post_receive).to receive(:puts).with( - "To create a merge request for new_branch, visit:" - ).ordered - expect(gitlab_post_receive).to receive(:puts).with( - " http://localhost/dzaporozhets/gitlab-ci/merge_requests/new?merge_request%5Bsource_branch%5D=new_branch" - ).ordered - expect(gitlab_post_receive).to receive(:puts).ordered + expect(gitlab_post_receive).to receive(:puts).ordered + expect(gitlab_post_receive).to receive(:puts).with( + "To create a merge request for new_branch, visit:" + ).ordered + expect(gitlab_post_receive).to receive(:puts).with( + " http://localhost/dzaporozhets/gitlab-ci/merge_requests/new?merge_request%5Bsource_branch%5D=new_branch" + ).ordered + expect(gitlab_post_receive).to receive(:puts).ordered - gitlab_post_receive.exec + gitlab_post_receive.exec + end + end + + context 'when printing merge request urls is disabled' do + before do + GitlabNet.any_instance.stub(:project_settings).and_return({ + 'disable_printing_merge_request_link' => true + }) + end + + it "does not print the new merge request url" do + expect(gitlab_post_receive).not_to receive(:puts) + gitlab_post_receive.exec + end end end @@ -72,19 +89,34 @@ describe GitlabPostReceive do end end - it "prints the view merge request url" do - expect(redis_client).to receive(:rpush) + context 'when printing merge request urls is not disabled' do + it "prints the view merge request url" do + expect(redis_client).to receive(:rpush) - expect(gitlab_post_receive).to receive(:puts).ordered - expect(gitlab_post_receive).to receive(:puts).with( - "View merge request for feature_branch:" - ).ordered - expect(gitlab_post_receive).to receive(:puts).with( - " http://localhost/dzaporozhets/gitlab-ci/merge_requests/1" - ).ordered - expect(gitlab_post_receive).to receive(:puts).ordered + expect(gitlab_post_receive).to receive(:puts).ordered + expect(gitlab_post_receive).to receive(:puts).with( + "View merge request for feature_branch:" + ).ordered + expect(gitlab_post_receive).to receive(:puts).with( + " http://localhost/dzaporozhets/gitlab-ci/merge_requests/1" + ).ordered + expect(gitlab_post_receive).to receive(:puts).ordered - gitlab_post_receive.exec + gitlab_post_receive.exec + end + end + + context 'when printing merge request urls is disabled' do + before do + GitlabNet.any_instance.stub(:project_settings).and_return({ + 'disable_printing_merge_request_link' => true + }) + end + + it "does not print the new merge request url" do + expect(gitlab_post_receive).not_to receive(:puts) + gitlab_post_receive.exec + end end end end diff --git a/spec/vcr_cassettes/project-settings.yml b/spec/vcr_cassettes/project-settings.yml new file mode 100644 index 00000000..9cfba005 --- /dev/null +++ b/spec/vcr_cassettes/project-settings.yml @@ -0,0 +1,46 @@ +--- +http_interactions: +- request: + method: get + uri: https://dev.gitlab.org/api/v3/internal/project_settings?project=gitlab-org/gitlab-ce.git + body: + encoding: US-ASCII + string: secret_token=a123 + headers: + Accept-Encoding: + - gzip;q=1.0,deflate;q=0.6,identity;q=0.3 + Accept: + - "*/*" + User-Agent: + - Ruby + Content-Type: + - application/x-www-form-urlencoded + response: + status: + code: 200 + message: OK + headers: + Server: + - nginx + Date: + - Thu, 02 Mar 2017 00:58:26 GMT + Content-Type: + - application/json + Content-Length: + - '44' + Connection: + - keep-alive + Cache-Control: + - no-cache + Vary: + - Origin + X-Request-Id: + - 1d1c20cc-04e4-4b1b-bc84-e4cf54ea7e49 + X-Runtime: + - '0.005841' + body: + encoding: UTF-8 + string: '{"disable_printing_merge_request_link":true}' + http_version: + recorded_at: Thu, 02 Mar 2017 00:58:26 GMT +recorded_with: VCR 2.4.0 -- GitLab From 3bc05582d5199c3fb8228c898b6d182277361fd0 Mon Sep 17 00:00:00 2001 From: Alex Sanford Date: Sat, 4 Mar 2017 09:57:28 -0400 Subject: [PATCH 2/4] Tweak specs --- spec/gitlab_net_spec.rb | 8 ++++++++ spec/gitlab_post_receive_spec.rb | 10 ++++------ 2 files changed, 12 insertions(+), 6 deletions(-) diff --git a/spec/gitlab_net_spec.rb b/spec/gitlab_net_spec.rb index 4651195f..25462045 100644 --- a/spec/gitlab_net_spec.rb +++ b/spec/gitlab_net_spec.rb @@ -100,6 +100,14 @@ describe GitlabNet, vcr: true do result['disable_printing_merge_request_link'].should be(true) end end + + it 'should return empty hash on error' do + gitlab_net.stub(:get).and_return('Error!') + + result = gitlab_net.project_settings(repo_path) + + result.should eq({}) + end end describe :authorized_key do diff --git a/spec/gitlab_post_receive_spec.rb b/spec/gitlab_post_receive_spec.rb index 48884690..acb3e40f 100644 --- a/spec/gitlab_post_receive_spec.rb +++ b/spec/gitlab_post_receive_spec.rb @@ -65,14 +65,13 @@ describe GitlabPostReceive do end context 'when printing merge request urls is disabled' do - before do + it "does not print the new merge request url" do GitlabNet.any_instance.stub(:project_settings).and_return({ 'disable_printing_merge_request_link' => true }) - end - it "does not print the new merge request url" do expect(gitlab_post_receive).not_to receive(:puts) + gitlab_post_receive.exec end end @@ -107,14 +106,13 @@ describe GitlabPostReceive do end context 'when printing merge request urls is disabled' do - before do + it "does not print the new merge request url" do GitlabNet.any_instance.stub(:project_settings).and_return({ 'disable_printing_merge_request_link' => true }) - end - it "does not print the new merge request url" do expect(gitlab_post_receive).not_to receive(:puts) + gitlab_post_receive.exec end end -- GitLab From 231479da79a2f76cec4c13b0df00f3f6523fc0c5 Mon Sep 17 00:00:00 2001 From: Alex Sanford Date: Wed, 8 Mar 2017 20:23:36 -0400 Subject: [PATCH 3/4] Use printing_merge_request_link_enabled from project_settings --- lib/gitlab_post_receive.rb | 2 +- spec/gitlab_post_receive_spec.rb | 24 +++++++++++++++--------- 2 files changed, 16 insertions(+), 10 deletions(-) diff --git a/lib/gitlab_post_receive.rb b/lib/gitlab_post_receive.rb index 5f998d54..aa294879 100644 --- a/lib/gitlab_post_receive.rb +++ b/lib/gitlab_post_receive.rb @@ -32,7 +32,7 @@ class GitlabPostReceive end project_settings = api.project_settings(@repo_path) - unless project_settings['disable_printing_merge_request_link'] + if project_settings['printing_merge_request_link_enabled'] merge_request_urls = GitlabMetrics.measure("merge-request-urls") do api.merge_request_urls(@repo_path, @changes) end diff --git a/spec/gitlab_post_receive_spec.rb b/spec/gitlab_post_receive_spec.rb index acb3e40f..c4388d96 100644 --- a/spec/gitlab_post_receive_spec.rb +++ b/spec/gitlab_post_receive_spec.rb @@ -12,7 +12,7 @@ describe GitlabPostReceive do let(:repo_path) { File.join(repository_path, repo_name) + ".git" } let(:gitlab_post_receive) { GitlabPostReceive.new(repo_path, actor, wrongly_encoded_changes) } let(:message) { "test " * 10 + "message " * 10 } - let(:project_settings) { { 'disable_printing_merge_request_link' => false } } + let(:project_settings) { { 'printing_merge_request_link_enabled' => true } } let(:redis_client) { double('redis_client') } let(:enqueued_at) { Time.new(2016, 6, 23, 6, 59) } @@ -47,10 +47,13 @@ describe GitlabPostReceive do end end - context 'when printing merge request urls is not disabled' do + context 'when printing merge request urls is enabled' do it "prints the new merge request url" do - expect(redis_client).to receive(:rpush) + GitlabNet.any_instance.stub(:project_settings).and_return({ + 'printing_merge_request_link_enabled' => true + }) + expect(redis_client).to receive(:rpush) expect(gitlab_post_receive).to receive(:puts).ordered expect(gitlab_post_receive).to receive(:puts).with( "To create a merge request for new_branch, visit:" @@ -64,10 +67,10 @@ describe GitlabPostReceive do end end - context 'when printing merge request urls is disabled' do + context 'when printing merge request urls is not enabled' do it "does not print the new merge request url" do GitlabNet.any_instance.stub(:project_settings).and_return({ - 'disable_printing_merge_request_link' => true + 'printing_merge_request_link_enabled' => false }) expect(gitlab_post_receive).not_to receive(:puts) @@ -88,10 +91,13 @@ describe GitlabPostReceive do end end - context 'when printing merge request urls is not disabled' do + context 'when printing merge request urls is enabled' do it "prints the view merge request url" do - expect(redis_client).to receive(:rpush) + GitlabNet.any_instance.stub(:project_settings).and_return({ + 'printing_merge_request_link_enabled' => true + }) + expect(redis_client).to receive(:rpush) expect(gitlab_post_receive).to receive(:puts).ordered expect(gitlab_post_receive).to receive(:puts).with( "View merge request for feature_branch:" @@ -105,10 +111,10 @@ describe GitlabPostReceive do end end - context 'when printing merge request urls is disabled' do + context 'when printing merge request urls is not enabled' do it "does not print the new merge request url" do GitlabNet.any_instance.stub(:project_settings).and_return({ - 'disable_printing_merge_request_link' => true + 'printing_merge_request_link_enabled' => false }) expect(gitlab_post_receive).not_to receive(:puts) -- GitLab From 6adb05ea851c2db15058da72a8ec6dc7af91cf3f Mon Sep 17 00:00:00 2001 From: Alex Sanford Date: Wed, 8 Mar 2017 20:28:39 -0400 Subject: [PATCH 4/4] Fix VCR casette for project_settings spec --- spec/gitlab_net_spec.rb | 2 +- spec/vcr_cassettes/project-settings.yml | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/spec/gitlab_net_spec.rb b/spec/gitlab_net_spec.rb index 25462045..c6ffd8ce 100644 --- a/spec/gitlab_net_spec.rb +++ b/spec/gitlab_net_spec.rb @@ -97,7 +97,7 @@ describe GitlabNet, vcr: true do it 'should return project settings hash' do VCR.use_cassette('project-settings') do result = gitlab_net.project_settings(repo_path) - result['disable_printing_merge_request_link'].should be(true) + result['printing_merge_request_link_enabled'].should be(true) end end diff --git a/spec/vcr_cassettes/project-settings.yml b/spec/vcr_cassettes/project-settings.yml index 9cfba005..c4ac3b6d 100644 --- a/spec/vcr_cassettes/project-settings.yml +++ b/spec/vcr_cassettes/project-settings.yml @@ -40,7 +40,7 @@ http_interactions: - '0.005841' body: encoding: UTF-8 - string: '{"disable_printing_merge_request_link":true}' + string: '{"printing_merge_request_link_enabled":true}' http_version: recorded_at: Thu, 02 Mar 2017 00:58:26 GMT recorded_with: VCR 2.4.0 -- GitLab