From 4c4c8532d0710689cca2a3c71b134665d42e839a Mon Sep 17 00:00:00 2001 From: Stan Hu Date: Wed, 21 Nov 2018 07:05:02 -0800 Subject: Handle force_remove_source_branch when creating merge request Creating a merge request with `merge_request[force_remove_source_branch]` parameter would result in an Error 500 since this attribute was passed directly to the merge request. Fix this by properly parsing this attribute into `merge_params`. Closes https://gitlab.com/gitlab-org/gitlab-ce/issues/51220 --- app/services/merge_requests/build_service.rb | 1 + changelogs/unreleased/sh-fix-issue-51220.yml | 5 ++++ spec/services/merge_requests/build_service_spec.rb | 34 +++++++++++++++++----- 3 files changed, 32 insertions(+), 8 deletions(-) create mode 100644 changelogs/unreleased/sh-fix-issue-51220.yml diff --git a/app/services/merge_requests/build_service.rb b/app/services/merge_requests/build_service.rb index 6c69452e2ab..36767621d74 100644 --- a/app/services/merge_requests/build_service.rb +++ b/app/services/merge_requests/build_service.rb @@ -10,6 +10,7 @@ module MergeRequests # TODO: this should handle all quick actions that don't have side effects # https://gitlab.com/gitlab-org/gitlab-ce/issues/53658 merge_quick_actions_into_params!(merge_request, only: [:target_branch]) + merge_request.merge_params['force_remove_source_branch'] = params.delete(:force_remove_source_branch) if params.has_key?(:force_remove_source_branch) merge_request.assign_attributes(params) merge_request.author = current_user diff --git a/changelogs/unreleased/sh-fix-issue-51220.yml b/changelogs/unreleased/sh-fix-issue-51220.yml new file mode 100644 index 00000000000..048f58611cb --- /dev/null +++ b/changelogs/unreleased/sh-fix-issue-51220.yml @@ -0,0 +1,5 @@ +--- +title: Handle force_remove_source_branch when creating merge request +merge_request: 23281 +author: +type: fixed diff --git a/spec/services/merge_requests/build_service_spec.rb b/spec/services/merge_requests/build_service_spec.rb index c9a668994eb..1894d8c8d0e 100644 --- a/spec/services/merge_requests/build_service_spec.rb +++ b/spec/services/merge_requests/build_service_spec.rb @@ -21,15 +21,20 @@ describe MergeRequests::BuildService do let(:commit_2) { double(:commit_2, sha: 'f00ba7', safe_message: 'This is a bad commit message!') } let(:commits) { nil } + let(:params) do + { + description: description, + source_branch: source_branch, + target_branch: target_branch, + source_project: source_project, + target_project: target_project, + milestone_id: milestone_id, + label_ids: label_ids + } + end + let(:service) do - described_class.new(project, user, - description: description, - source_branch: source_branch, - target_branch: target_branch, - source_project: source_project, - target_project: target_project, - milestone_id: milestone_id, - label_ids: label_ids) + described_class.new(project, user, params) end before do @@ -56,6 +61,19 @@ describe MergeRequests::BuildService do merge_request end + it 'does not assign force_remove_source_branch' do + expect(merge_request.force_remove_source_branch?).to be_falsey + end + + context 'with force_remove_source_branch parameter' do + let(:mr_params) { params.merge(force_remove_source_branch: '1') } + let(:merge_request) { described_class.new(project, user, mr_params).execute } + + it 'assigns force_remove_source_branch' do + expect(merge_request.force_remove_source_branch?).to be_truthy + end + end + context 'missing source branch' do let(:source_branch) { '' } -- cgit v1.2.1