diff --git a/app/controllers/concerns/creates_commit.rb b/app/controllers/concerns/creates_commit.rb index f644702cbdb..b3777fd2b0f 100644 --- a/app/controllers/concerns/creates_commit.rb +++ b/app/controllers/concerns/creates_commit.rb @@ -86,10 +86,10 @@ module CreatesCommit def new_merge_request_path project_new_merge_request_path( @project_to_commit_into, - merge_request_source_branch: @branch_name, merge_request: { source_project_id: @project_to_commit_into.id, target_project_id: @project.id, + source_branch: @branch_name, target_branch: @start_branch } ) diff --git a/app/controllers/projects/merge_requests/creations_controller.rb b/app/controllers/projects/merge_requests/creations_controller.rb index bbf662a63c8..5639402a1e9 100644 --- a/app/controllers/projects/merge_requests/creations_controller.rb +++ b/app/controllers/projects/merge_requests/creations_controller.rb @@ -89,8 +89,6 @@ class Projects::MergeRequests::CreationsController < Projects::MergeRequests::Ap def build_merge_request params[:merge_request] ||= ActionController::Parameters.new(source_project: @project) - params[:merge_request][:source_branch] ||= params[:merge_request_source_branch].presence - @merge_request = ::MergeRequests::BuildService.new(project, current_user, merge_request_params.merge(diff_options: diff_options)).execute end diff --git a/app/helpers/compare_helper.rb b/app/helpers/compare_helper.rb index 57e397f6ca0..9ece8b0bc5b 100644 --- a/app/helpers/compare_helper.rb +++ b/app/helpers/compare_helper.rb @@ -13,8 +13,8 @@ module CompareHelper def create_mr_path(from = params[:from], to = params[:to], project = @project) project_new_merge_request_path( project, - merge_request_source_branch: to, merge_request: { + source_branch: to, target_branch: from } ) diff --git a/app/helpers/merge_requests_helper.rb b/app/helpers/merge_requests_helper.rb index 8f549bfce73..23d7aa427bb 100644 --- a/app/helpers/merge_requests_helper.rb +++ b/app/helpers/merge_requests_helper.rb @@ -11,10 +11,10 @@ module MergeRequestsHelper def new_mr_from_push_event(event, target_project) { - merge_request_source_branch: event.branch_name, merge_request: { source_project_id: event.project.id, target_project_id: target_project.id, + source_branch: event.branch_name, target_branch: target_project.repository.root_ref } } @@ -51,10 +51,10 @@ module MergeRequestsHelper def mr_change_branches_path(merge_request) project_new_merge_request_path( @project, - merge_request_source_branch: merge_request.source_branch, merge_request: { source_project_id: merge_request.source_project_id, target_project_id: merge_request.target_project_id, + source_branch: merge_request.source_branch, target_branch: merge_request.target_branch }, change_branches: true diff --git a/app/services/merge_requests/get_urls_service.rb b/app/services/merge_requests/get_urls_service.rb index 35a22449e34..7c88c9abb41 100644 --- a/app/services/merge_requests/get_urls_service.rb +++ b/app/services/merge_requests/get_urls_service.rb @@ -50,8 +50,8 @@ module MergeRequests end def url_for_new_merge_request(branch_name) - url = Gitlab::Routing.url_helpers.project_new_merge_request_url(project, branch_name) - + merge_request_params = { source_branch: branch_name } + url = Gitlab::Routing.url_helpers.project_new_merge_request_url(project, merge_request: merge_request_params) { branch_name: branch_name, url: url, new_merge_request: true } end diff --git a/changelogs/unreleased/blackst0ne-update-push-new-merge-request-url.yml b/changelogs/unreleased/blackst0ne-update-push-new-merge-request-url.yml deleted file mode 100644 index b89ba754952..00000000000 --- a/changelogs/unreleased/blackst0ne-update-push-new-merge-request-url.yml +++ /dev/null @@ -1,5 +0,0 @@ ---- -title: Make new merge request URL more friendly when pushing code -merge_request: 22526 -author: "@blackst0ne" -type: changed diff --git a/config/routes/project.rb b/config/routes/project.rb index 387d2363552..3f1ad90dfca 100644 --- a/config/routes/project.rb +++ b/config/routes/project.rb @@ -149,7 +149,9 @@ constraints(::Constraints::ProjectUrlConstrainer.new) do scope path: 'merge_requests', controller: 'merge_requests/creations' do post '', action: :create, as: nil - scope path: 'new/(:merge_request_source_branch)', as: :new_merge_request do + scope path: 'new', as: :new_merge_request do + get '', action: :new + scope constraints: { format: nil }, action: :new do get :diffs, defaults: { tab: 'diffs' } get :pipelines, defaults: { tab: 'pipelines' } @@ -163,7 +165,6 @@ constraints(::Constraints::ProjectUrlConstrainer.new) do get :diff_for_path get :branch_from get :branch_to - get '', action: :new end end diff --git a/spec/controllers/projects/blob_controller_spec.rb b/spec/controllers/projects/blob_controller_spec.rb index f58aa25cbdd..5fdf7f1229d 100644 --- a/spec/controllers/projects/blob_controller_spec.rb +++ b/spec/controllers/projects/blob_controller_spec.rb @@ -331,10 +331,10 @@ describe Projects::BlobController do expect(response).to redirect_to( project_new_merge_request_path( forked_project, - merge_request_source_branch: "fork-test-1", merge_request: { source_project_id: forked_project.id, target_project_id: project.id, + source_branch: "fork-test-1", target_branch: "master" } ) diff --git a/spec/features/dashboard/projects_spec.rb b/spec/features/dashboard/projects_spec.rb index 0a24c5e906a..975b7944741 100644 --- a/spec/features/dashboard/projects_spec.rb +++ b/spec/features/dashboard/projects_spec.rb @@ -147,12 +147,10 @@ describe 'Dashboard Projects' do end context 'last push widget', :use_clean_rails_memory_store_caching do - let(:ref) { "feature" } - before do event = create(:push_event, project: project, author: user) - create(:push_event_payload, event: event, ref: ref, action: :created) + create(:push_event_payload, event: event, ref: 'feature', action: :created) Users::LastPushEventService.new(user).cache_last_push_event(event) @@ -167,9 +165,9 @@ describe 'Dashboard Projects' do end expect(page).to have_selector('.merge-request-form') - expect(current_path).to eq project_new_merge_request_path(project, merge_request_source_branch: ref) + expect(current_path).to eq project_new_merge_request_path(project) expect(find('#merge_request_target_project_id', visible: false).value).to eq project.id.to_s - expect(find('input#merge_request_source_branch', visible: false).value).to eq ref + expect(find('input#merge_request_source_branch', visible: false).value).to eq 'feature' expect(find('input#merge_request_target_branch', visible: false).value).to eq 'master' end end diff --git a/spec/features/merge_request/user_allows_commits_from_memebers_who_can_merge_spec.rb b/spec/features/merge_request/user_allows_commits_from_memebers_who_can_merge_spec.rb index a124c99ecc8..0ccab5b2fac 100644 --- a/spec/features/merge_request/user_allows_commits_from_memebers_who_can_merge_spec.rb +++ b/spec/features/merge_request/user_allows_commits_from_memebers_who_can_merge_spec.rb @@ -9,10 +9,10 @@ describe 'create a merge request, allowing commits from members who can merge to def visit_new_merge_request visit project_new_merge_request_path( source_project, - merge_request_source_branch: 'fix', merge_request: { source_project_id: source_project.id, target_project_id: target_project.id, + source_branch: 'fix', target_branch: 'master' }) end diff --git a/spec/features/merge_request/user_sees_merge_widget_spec.rb b/spec/features/merge_request/user_sees_merge_widget_spec.rb index d907ed4198c..582be101399 100644 --- a/spec/features/merge_request/user_sees_merge_widget_spec.rb +++ b/spec/features/merge_request/user_sees_merge_widget_spec.rb @@ -20,10 +20,10 @@ describe 'Merge request > User sees merge widget', :js do before do visit project_new_merge_request_path( project, - merge_request_source_branch: 'feature', merge_request: { source_project_id: project.id, target_project_id: project.id, + source_branch: 'feature', target_branch: 'master' }) end diff --git a/spec/features/merge_request/user_sees_wip_help_message_spec.rb b/spec/features/merge_request/user_sees_wip_help_message_spec.rb index 6dfc819fe8a..92cc73ddf1f 100644 --- a/spec/features/merge_request/user_sees_wip_help_message_spec.rb +++ b/spec/features/merge_request/user_sees_wip_help_message_spec.rb @@ -13,10 +13,10 @@ describe 'Merge request > User sees WIP help message' do it 'shows a specific WIP hint' do visit project_new_merge_request_path( project, - merge_request_source_branch: 'wip', merge_request: { source_project_id: project.id, target_project_id: project.id, + source_branch: 'wip', target_branch: 'master' }) @@ -32,10 +32,10 @@ describe 'Merge request > User sees WIP help message' do it 'shows the regular WIP message' do visit project_new_merge_request_path( project, - merge_request_source_branch: 'fix', merge_request: { source_project_id: project.id, target_project_id: project.id, + source_branch: 'fix', target_branch: 'master' }) diff --git a/spec/features/merge_request/user_selects_branches_for_new_mr_spec.rb b/spec/features/merge_request/user_selects_branches_for_new_mr_spec.rb index 147544740dc..ae41cf90576 100644 --- a/spec/features/merge_request/user_selects_branches_for_new_mr_spec.rb +++ b/spec/features/merge_request/user_selects_branches_for_new_mr_spec.rb @@ -109,13 +109,13 @@ describe 'Merge request > User selects branches for new MR', :js do end it 'populates source branch button' do - visit project_new_merge_request_path(project, change_branches: true, merge_request_source_branch: 'fix', merge_request: { target_branch: 'master' }) + visit project_new_merge_request_path(project, change_branches: true, merge_request: { target_branch: 'master', source_branch: 'fix' }) expect(find('.js-source-branch')).to have_content('fix') end it 'allows to change the diff view' do - visit project_new_merge_request_path(project, merge_request_source_branch: 'fix', merge_request: { target_branch: 'master' }) + visit project_new_merge_request_path(project, merge_request: { target_branch: 'master', source_branch: 'fix' }) click_link 'Changes' @@ -131,7 +131,7 @@ describe 'Merge request > User selects branches for new MR', :js do end it 'does not allow non-existing branches' do - visit project_new_merge_request_path(project, merge_request_source_branch: 'non-exist-source', merge_request: { target_branch: 'non-exist-target' }) + visit project_new_merge_request_path(project, merge_request: { target_branch: 'non-exist-target', source_branch: 'non-exist-source' }) expect(page).to have_content('The form contains the following errors') expect(page).to have_content('Source branch "non-exist-source" does not exist') @@ -140,7 +140,7 @@ describe 'Merge request > User selects branches for new MR', :js do context 'when a branch contains commits that both delete and add the same image' do it 'renders the diff successfully' do - visit project_new_merge_request_path(project, merge_request_source_branch: 'deleted-image-test', merge_request: { target_branch: 'master' }) + visit project_new_merge_request_path(project, merge_request: { target_branch: 'master', source_branch: 'deleted-image-test' }) click_link "Changes" @@ -165,8 +165,7 @@ describe 'Merge request > User selects branches for new MR', :js do it 'shows pipelines for a new merge request' do visit project_new_merge_request_path( project, - merge_request_source_branch: 'fix', - merge_request: { target_branch: 'master' }) + merge_request: { target_branch: 'master', source_branch: 'fix' }) page.within('.merge-request') do click_link 'Pipelines' diff --git a/spec/features/merge_request/user_uses_quick_actions_spec.rb b/spec/features/merge_request/user_uses_quick_actions_spec.rb index 6e681185e1f..b81478a481f 100644 --- a/spec/features/merge_request/user_uses_quick_actions_spec.rb +++ b/spec/features/merge_request/user_uses_quick_actions_spec.rb @@ -144,7 +144,7 @@ describe 'Merge request > User uses quick actions', :js do describe '/target_branch command in merge request' do let(:another_project) { create(:project, :public, :repository) } - let(:new_url_opts) { { merge_request_source_branch: 'feature' } } + let(:new_url_opts) { { merge_request: { source_branch: 'feature' } } } before do another_project.add_maintainer(user) diff --git a/spec/features/merge_requests/user_squashes_merge_request_spec.rb b/spec/features/merge_requests/user_squashes_merge_request_spec.rb index 8ecdec491b8..ec1153b7f7f 100644 --- a/spec/features/merge_requests/user_squashes_merge_request_spec.rb +++ b/spec/features/merge_requests/user_squashes_merge_request_spec.rb @@ -65,7 +65,7 @@ describe 'User squashes a merge request', :js do context 'when squash is enabled on merge request creation' do before do - visit project_new_merge_request_path(project, merge_request_source_branch: source_branch, merge_request: { target_branch: 'master' }) + visit project_new_merge_request_path(project, merge_request: { target_branch: 'master', source_branch: source_branch }) check 'merge_request[squash]' click_on 'Submit merge request' wait_for_requests @@ -95,7 +95,7 @@ describe 'User squashes a merge request', :js do context 'when squash is not enabled on merge request creation' do before do - visit project_new_merge_request_path(project, merge_request_source_branch: source_branch, merge_request: { target_branch: 'master' }) + visit project_new_merge_request_path(project, merge_request: { target_branch: 'master', source_branch: source_branch }) click_on 'Submit merge request' wait_for_requests end diff --git a/spec/features/projects/files/user_creates_directory_spec.rb b/spec/features/projects/files/user_creates_directory_spec.rb index 6f620dff82b..847b5f0860f 100644 --- a/spec/features/projects/files/user_creates_directory_spec.rb +++ b/spec/features/projects/files/user_creates_directory_spec.rb @@ -57,7 +57,7 @@ describe 'Projects > Files > User creates a directory', :js do expect(page).to have_content('From new-feature into master') expect(page).to have_content('Add new directory') - expect(current_path).to eq(project_new_merge_request_path(project, merge_request_source_branch: "new-feature")) + expect(current_path).to eq(project_new_merge_request_path(project)) end end @@ -80,7 +80,8 @@ describe 'Projects > Files > User creates a directory', :js do click_button('Create directory') fork = user.fork_of(project2.reload) - expect(current_path).to eq(project_new_merge_request_path(fork, merge_request_source_branch: "patch-1")) + + expect(current_path).to eq(project_new_merge_request_path(fork)) end end end diff --git a/spec/features/projects/files/user_creates_files_spec.rb b/spec/features/projects/files/user_creates_files_spec.rb index 14b5bd58bd1..a4f94b7a76d 100644 --- a/spec/features/projects/files/user_creates_files_spec.rb +++ b/spec/features/projects/files/user_creates_files_spec.rb @@ -144,7 +144,7 @@ describe 'Projects > Files > User creates files' do fill_in(:branch_name, with: 'new_branch_name', visible: true) click_button('Commit changes') - expect(current_path).to eq(project_new_merge_request_path(project, merge_request_source_branch: "new_branch_name")) + expect(current_path).to eq(project_new_merge_request_path(project)) click_link('Changes') @@ -182,7 +182,7 @@ describe 'Projects > Files > User creates files' do fork = user.fork_of(project2.reload) - expect(current_path).to eq(project_new_merge_request_path(fork, merge_request_source_branch: "patch-1")) + expect(current_path).to eq(project_new_merge_request_path(fork)) expect(page).to have_content('New commit message') end end diff --git a/spec/features/projects/files/user_deletes_files_spec.rb b/spec/features/projects/files/user_deletes_files_spec.rb index faf11ee9dd8..614b11fa5c8 100644 --- a/spec/features/projects/files/user_deletes_files_spec.rb +++ b/spec/features/projects/files/user_deletes_files_spec.rb @@ -63,7 +63,7 @@ describe 'Projects > Files > User deletes files', :js do fork = user.fork_of(project2.reload) - expect(current_path).to eq(project_new_merge_request_path(fork, merge_request_source_branch: "patch-1")) + expect(current_path).to eq(project_new_merge_request_path(fork)) expect(page).to have_content('New commit message') end end diff --git a/spec/features/projects/files/user_edits_files_spec.rb b/spec/features/projects/files/user_edits_files_spec.rb index c6b2aaea906..9eb65ec159c 100644 --- a/spec/features/projects/files/user_edits_files_spec.rb +++ b/spec/features/projects/files/user_edits_files_spec.rb @@ -86,7 +86,7 @@ describe 'Projects > Files > User edits files', :js do fill_in(:branch_name, with: 'new_branch_name', visible: true) click_button('Commit changes') - expect(current_path).to eq(project_new_merge_request_path(project, merge_request_source_branch: "new_branch_name")) + expect(current_path).to eq(project_new_merge_request_path(project)) click_link('Changes') @@ -155,7 +155,7 @@ describe 'Projects > Files > User edits files', :js do fork = user.fork_of(project2.reload) - expect(current_path).to eq(project_new_merge_request_path(fork, merge_request_source_branch: "patch-1")) + expect(current_path).to eq(project_new_merge_request_path(fork)) wait_for_requests @@ -183,7 +183,7 @@ describe 'Projects > Files > User edits files', :js do fork = user.fork_of(project2) - expect(current_path).to eq(project_new_merge_request_path(fork, merge_request_source_branch: "patch-1")) + expect(current_path).to eq(project_new_merge_request_path(fork)) wait_for_requests diff --git a/spec/features/projects/files/user_replaces_files_spec.rb b/spec/features/projects/files/user_replaces_files_spec.rb index 09feb315465..e3da28d73c3 100644 --- a/spec/features/projects/files/user_replaces_files_spec.rb +++ b/spec/features/projects/files/user_replaces_files_spec.rb @@ -78,7 +78,7 @@ describe 'Projects > Files > User replaces files', :js do fork = user.fork_of(project2.reload) - expect(current_path).to eq(project_new_merge_request_path(fork, merge_request_source_branch: "undefined")) + expect(current_path).to eq(project_new_merge_request_path(fork)) click_link('Changes') diff --git a/spec/features/projects/files/user_uploads_files_spec.rb b/spec/features/projects/files/user_uploads_files_spec.rb index 92df8303f46..af3fc528a20 100644 --- a/spec/features/projects/files/user_uploads_files_spec.rb +++ b/spec/features/projects/files/user_uploads_files_spec.rb @@ -36,7 +36,7 @@ describe 'Projects > Files > User uploads files' do click_button('Upload file') expect(page).to have_content('New commit message') - expect(current_path).to eq(project_new_merge_request_path(project, merge_request_source_branch: "new_branch_name")) + expect(current_path).to eq(project_new_merge_request_path(project)) click_link('Changes') find("a[data-action='diffs']", text: 'Changes').click @@ -92,7 +92,7 @@ describe 'Projects > Files > User uploads files' do fork = user.fork_of(project2.reload) - expect(current_path).to eq(project_new_merge_request_path(fork, merge_request_source_branch: "undefined")) + expect(current_path).to eq(project_new_merge_request_path(fork)) find("a[data-action='diffs']", text: 'Changes').click diff --git a/spec/features/projects/merge_request_button_spec.rb b/spec/features/projects/merge_request_button_spec.rb index 4be31511ceb..69561b4d733 100644 --- a/spec/features/projects/merge_request_button_spec.rb +++ b/spec/features/projects/merge_request_button_spec.rb @@ -22,8 +22,8 @@ describe 'Merge Request button' do it 'shows Create merge request button' do href = project_new_merge_request_path(project, - merge_request_source_branch: 'feature', - merge_request: { target_branch: 'master' }) + merge_request: { source_branch: 'feature', + target_branch: 'master' }) visit url @@ -77,8 +77,8 @@ describe 'Merge Request button' do it 'shows Create merge request button' do href = project_new_merge_request_path(forked_project, - merge_request_source_branch: 'feature', - merge_request: { target_branch: 'master' }) + merge_request: { source_branch: 'feature', + target_branch: 'master' }) visit fork_url diff --git a/spec/requests/api/internal_spec.rb b/spec/requests/api/internal_spec.rb index 5ea869796b0..2ebcb787d06 100644 --- a/spec/requests/api/internal_spec.rb +++ b/spec/requests/api/internal_spec.rb @@ -683,7 +683,7 @@ describe API::Internal do expect(json_response).to match [{ "branch_name" => "new_branch", - "url" => "http://#{Gitlab.config.gitlab.host}/#{project.namespace.name}/#{project.path}/merge_requests/new/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 @@ -704,7 +704,7 @@ describe API::Internal do expect(json_response).to match [{ "branch_name" => "new_branch", - "url" => "http://#{Gitlab.config.gitlab.host}/#{project.namespace.name}/#{project.path}/merge_requests/new/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 @@ -837,7 +837,7 @@ describe API::Internal do expect(json_response['merge_request_urls']).to match [{ "branch_name" => "new_branch", - "url" => "http://#{Gitlab.config.gitlab.host}/#{project.namespace.name}/#{project.path}/merge_requests/new/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 diff --git a/spec/services/merge_requests/get_urls_service_spec.rb b/spec/services/merge_requests/get_urls_service_spec.rb index 3e33a165e55..274624aa8bb 100644 --- a/spec/services/merge_requests/get_urls_service_spec.rb +++ b/spec/services/merge_requests/get_urls_service_spec.rb @@ -6,7 +6,7 @@ describe MergeRequests::GetUrlsService do let(:project) { create(:project, :public, :repository) } let(:service) { described_class.new(project) } let(:source_branch) { "merge-test" } - let(:new_merge_request_url) { "http://#{Gitlab.config.gitlab.host}/#{project.namespace.name}/#{project.path}/merge_requests/new/#{source_branch}" } + let(:new_merge_request_url) { "http://#{Gitlab.config.gitlab.host}/#{project.namespace.name}/#{project.path}/merge_requests/new?merge_request%5Bsource_branch%5D=#{source_branch}" } let(:show_merge_request_url) { "http://#{Gitlab.config.gitlab.host}/#{project.namespace.name}/#{project.path}/merge_requests/#{merge_request.iid}" } let(:new_branch_changes) { "#{Gitlab::Git::BLANK_SHA} 570e7b2abdd848b95f2f578043fc23bd6f6fd24d refs/heads/#{source_branch}" } let(:deleted_branch_changes) { "d14d6c0abdd253381df51a723d58691b2ee1ab08 #{Gitlab::Git::BLANK_SHA} refs/heads/#{source_branch}" } @@ -117,7 +117,7 @@ describe MergeRequests::GetUrlsService do let(:new_branch_changes) { "#{Gitlab::Git::BLANK_SHA} 570e7b2abdd848b95f2f578043fc23bd6f6fd24d refs/heads/new_branch" } let(:existing_branch_changes) { "d14d6c0abdd253381df51a723d58691b2ee1ab08 570e7b2abdd848b95f2f578043fc23bd6f6fd24d refs/heads/markdown" } let(:changes) { "#{new_branch_changes}\n#{existing_branch_changes}" } - let(:new_merge_request_url) { "http://#{Gitlab.config.gitlab.host}/#{project.namespace.name}/#{project.path}/merge_requests/new/new_branch" } + let(:new_merge_request_url) { "http://#{Gitlab.config.gitlab.host}/#{project.namespace.name}/#{project.path}/merge_requests/new?merge_request%5Bsource_branch%5D=new_branch" } it 'returns 2 urls for both creating new and showing merge request' do result = service.execute(changes) diff --git a/spec/support/shared_examples/features/creatable_merge_request_shared_examples.rb b/spec/support/shared_examples/features/creatable_merge_request_shared_examples.rb index 9373de5aeab..7038a366144 100644 --- a/spec/support/shared_examples/features/creatable_merge_request_shared_examples.rb +++ b/spec/support/shared_examples/features/creatable_merge_request_shared_examples.rb @@ -17,10 +17,10 @@ RSpec.shared_examples 'a creatable merge request' do sign_in(user) visit project_new_merge_request_path( target_project, - merge_request_source_branch: 'fix', merge_request: { source_project_id: source_project.id, target_project_id: target_project.id, + source_branch: 'fix', target_branch: 'master' }) end