From 68e3fa0e58938152357c6fb5997f1229666b1d5c Mon Sep 17 00:00:00 2001 From: Alex Sanford Date: Fri, 17 Mar 2017 12:35:39 +0000 Subject: Add ability to disable Merge Request URL on push --- app/controllers/projects_controller.rb | 1 + app/services/merge_requests/get_urls_service.rb | 2 ++ .../_merge_request_merge_settings.html.haml | 4 ++++ .../unreleased/21451-allow-disable-mr-link.yml | 4 ++++ ...inting_merge_request_link_enabled_to_project.rb | 18 +++++++++++++++++ db/schema.rb | 1 + .../settings/merge_requests_settings_spec.rb | 23 ++++++++++++++++++++++ spec/requests/api/internal_spec.rb | 11 ++++++++++- .../merge_requests/get_urls_service_spec.rb | 10 ++++++++++ 9 files changed, 73 insertions(+), 1 deletion(-) create mode 100644 changelogs/unreleased/21451-allow-disable-mr-link.yml create mode 100644 db/migrate/20170301125302_add_printing_merge_request_link_enabled_to_project.rb diff --git a/app/controllers/projects_controller.rb b/app/controllers/projects_controller.rb index 395a8bffe92..47f7e0b1b28 100644 --- a/app/controllers/projects_controller.rb +++ b/app/controllers/projects_controller.rb @@ -316,6 +316,7 @@ class ProjectsController < Projects::ApplicationController :namespace_id, :only_allow_merge_if_all_discussions_are_resolved, :only_allow_merge_if_pipeline_succeeds, + :printing_merge_request_link_enabled, :path, :public_builds, :request_access_enabled, diff --git a/app/services/merge_requests/get_urls_service.rb b/app/services/merge_requests/get_urls_service.rb index ae386b53f42..f00a33969a8 100644 --- a/app/services/merge_requests/get_urls_service.rb +++ b/app/services/merge_requests/get_urls_service.rb @@ -7,6 +7,8 @@ module MergeRequests end def execute(changes) + return [] unless project.printing_merge_request_link_enabled + branches = get_branches(changes) merge_requests_map = opened_merge_requests_from_source_branches(branches) branches.map do |branch| diff --git a/app/views/projects/_merge_request_merge_settings.html.haml b/app/views/projects/_merge_request_merge_settings.html.haml index 188198c47d5..61420fd0fb6 100644 --- a/app/views/projects/_merge_request_merge_settings.html.haml +++ b/app/views/projects/_merge_request_merge_settings.html.haml @@ -13,3 +13,7 @@ = form.label :only_allow_merge_if_all_discussions_are_resolved do = form.check_box :only_allow_merge_if_all_discussions_are_resolved %strong Only allow merge requests to be merged if all discussions are resolved + .checkbox + = form.label :printing_merge_request_link_enabled do + = form.check_box :printing_merge_request_link_enabled + %strong Show link to create/view merge request when pushing from the command line diff --git a/changelogs/unreleased/21451-allow-disable-mr-link.yml b/changelogs/unreleased/21451-allow-disable-mr-link.yml new file mode 100644 index 00000000000..ef99970a7a2 --- /dev/null +++ b/changelogs/unreleased/21451-allow-disable-mr-link.yml @@ -0,0 +1,4 @@ +--- +title: Add ability to disable Merge Request URL on push +merge_request: 9663 +author: Alex Sanford diff --git a/db/migrate/20170301125302_add_printing_merge_request_link_enabled_to_project.rb b/db/migrate/20170301125302_add_printing_merge_request_link_enabled_to_project.rb new file mode 100644 index 00000000000..f54608ecceb --- /dev/null +++ b/db/migrate/20170301125302_add_printing_merge_request_link_enabled_to_project.rb @@ -0,0 +1,18 @@ +# See http://doc.gitlab.com/ce/development/migration_style_guide.html +# for more information on how to write migrations for GitLab. + +class AddPrintingMergeRequestLinkEnabledToProject < ActiveRecord::Migration + include Gitlab::Database::MigrationHelpers + disable_ddl_transaction! + + # Set this constant to true if this migration requires downtime. + DOWNTIME = false + + def up + add_column_with_default(:projects, :printing_merge_request_link_enabled, :boolean, default: true) + end + + def down + remove_column(:projects, :printing_merge_request_link_enabled) + end +end diff --git a/db/schema.rb b/db/schema.rb index 3bef910c1d6..6eb3c95de93 100644 --- a/db/schema.rb +++ b/db/schema.rb @@ -1003,6 +1003,7 @@ ActiveRecord::Schema.define(version: 20170315174634) do t.boolean "lfs_enabled" t.text "description_html" t.boolean "only_allow_merge_if_all_discussions_are_resolved" + t.boolean "printing_merge_request_link_enabled", default: true, null: false end add_index "projects", ["ci_id"], name: "index_projects_on_ci_id", using: :btree diff --git a/spec/features/projects/settings/merge_requests_settings_spec.rb b/spec/features/projects/settings/merge_requests_settings_spec.rb index 6815039d5ed..321af416c91 100644 --- a/spec/features/projects/settings/merge_requests_settings_spec.rb +++ b/spec/features/projects/settings/merge_requests_settings_spec.rb @@ -62,4 +62,27 @@ feature 'Project settings > Merge Requests', feature: true, js: true do expect(page).to have_content('Only allow merge requests to be merged if all discussions are resolved') end end + + describe 'Checkbox to enable merge request link' do + before do + visit edit_project_path(project) + end + + scenario 'is initially checked' do + checkbox = find_field('project_printing_merge_request_link_enabled') + expect(checkbox).to be_checked + end + + scenario 'when unchecked sets :printing_merge_request_link_enabled to false' do + uncheck('project_printing_merge_request_link_enabled') + click_on('Save') + + # Wait for save to complete and page to reload + checkbox = find_field('project_printing_merge_request_link_enabled') + expect(checkbox).not_to be_checked + + project.reload + expect(project.printing_merge_request_link_enabled).to be(false) + end + end end diff --git a/spec/requests/api/internal_spec.rb b/spec/requests/api/internal_spec.rb index f18b8e98707..63ec00cdf04 100644 --- a/spec/requests/api/internal_spec.rb +++ b/spec/requests/api/internal_spec.rb @@ -397,16 +397,25 @@ describe API::Internal, api: true do before do project.team << [user, :developer] - get api("/internal/merge_request_urls?project=#{repo_name}&changes=#{changes}"), secret_token: secret_token end it 'returns link to create new merge request' do + get api("/internal/merge_request_urls?project=#{repo_name}&changes=#{changes}"), secret_token: secret_token + expect(json_response).to match [{ "branch_name" => "new_branch", "url" => "http://#{Gitlab.config.gitlab.host}/#{project.namespace.name}/#{project.path}/merge_requests/new?merge_request%5Bsource_branch%5D=new_branch", "new_merge_request" => true }] end + + it 'returns empty array if printing_merge_request_link_enabled is false' do + project.update!(printing_merge_request_link_enabled: false) + + get api("/internal/merge_request_urls?project=#{repo_name}&changes=#{changes}"), secret_token: secret_token + + expect(json_response).to eq([]) + end end describe 'POST /notify_post_receive' do diff --git a/spec/services/merge_requests/get_urls_service_spec.rb b/spec/services/merge_requests/get_urls_service_spec.rb index 08829e4be70..b7a05907208 100644 --- a/spec/services/merge_requests/get_urls_service_spec.rb +++ b/spec/services/merge_requests/get_urls_service_spec.rb @@ -130,5 +130,15 @@ describe MergeRequests::GetUrlsService do }]) end end + + context 'when printing_merge_request_link_enabled is false' do + it 'returns empty array' do + project.update!(printing_merge_request_link_enabled: false) + + result = service.execute(existing_branch_changes) + + expect(result).to eq([]) + end + end end end -- cgit v1.2.3