diff options
author | Tiago Botelho <tiagonbotelho@hotmail.com> | 2018-09-06 20:02:51 +0300 |
---|---|---|
committer | Nick Thomas <nick@gitlab.com> | 2018-09-06 20:02:51 +0300 |
commit | 342fa0f871e0f716e9cb1c0f80a2e0ab455d65c0 (patch) | |
tree | 21a6936ce1d1bd6a971edaa884149e57a77a6548 | |
parent | a062de4ed4abab3ffc1c6bd620c663c534474bab (diff) |
Resolve "Migrate issue labels and milestone to related merge request"
-rw-r--r-- | app/models/concerns/issuable.rb | 2 | ||||
-rw-r--r-- | app/models/label_link.rb | 2 | ||||
-rw-r--r-- | app/services/issuable_base_service.rb | 13 | ||||
-rw-r--r-- | app/services/merge_requests/build_service.rb | 20 | ||||
-rw-r--r-- | app/services/merge_requests/create_from_issue_service.rb | 5 | ||||
-rw-r--r-- | changelogs/unreleased/50657-migrate-issue-labels-and-milestone-to-related-mr-on-creation.yml | 5 | ||||
-rw-r--r-- | spec/services/merge_requests/build_service_spec.rb | 33 |
7 files changed, 61 insertions, 19 deletions
diff --git a/app/models/concerns/issuable.rb b/app/models/concerns/issuable.rb index f881ce2321c..7f14d78e976 100644 --- a/app/models/concerns/issuable.rb +++ b/app/models/concerns/issuable.rb @@ -49,7 +49,7 @@ module Issuable end end - has_many :label_links, as: :target, dependent: :destroy # rubocop:disable Cop/ActiveRecordDependent + has_many :label_links, as: :target, dependent: :destroy, inverse_of: :target # rubocop:disable Cop/ActiveRecordDependent has_many :labels, through: :label_links has_many :todos, as: :target, dependent: :destroy # rubocop:disable Cop/ActiveRecordDependent diff --git a/app/models/label_link.rb b/app/models/label_link.rb index 779657b25d5..1d93a55e8e9 100644 --- a/app/models/label_link.rb +++ b/app/models/label_link.rb @@ -3,7 +3,7 @@ class LabelLink < ActiveRecord::Base include Importable - belongs_to :target, polymorphic: true # rubocop:disable Cop/PolymorphicAssociations + belongs_to :target, polymorphic: true, inverse_of: :label_links # rubocop:disable Cop/PolymorphicAssociations belongs_to :label validates :target, presence: true, unless: :importing? diff --git a/app/services/issuable_base_service.rb b/app/services/issuable_base_service.rb index 7d60c65bb79..1259c2c2b3d 100644 --- a/app/services/issuable_base_service.rb +++ b/app/services/issuable_base_service.rb @@ -129,28 +129,19 @@ class IssuableBaseService < BaseService params.merge!(command_params) end - def create_issuable(issuable, attributes, label_ids:) - issuable.with_transaction_returning_status do - if issuable.save - issuable.update(label_ids: label_ids) - end - end - end - def create(issuable) handle_quick_actions_on_create(issuable) filter_params(issuable) params.delete(:state_event) params[:author] ||= current_user - - label_ids = process_label_ids(params) + params[:label_ids] = issuable.label_ids.to_a + process_label_ids(params) issuable.assign_attributes(params) before_create(issuable) - if params.present? && create_issuable(issuable, params, label_ids: label_ids) + if issuable.save after_create(issuable) execute_hooks(issuable) invalidate_cache_counts(issuable, users: issuable.assignees) diff --git a/app/services/merge_requests/build_service.rb b/app/services/merge_requests/build_service.rb index 55750269bb4..0e76d2cc3ab 100644 --- a/app/services/merge_requests/build_service.rb +++ b/app/services/merge_requests/build_service.rb @@ -20,6 +20,8 @@ module MergeRequests if merge_request.can_be_created compare_branches assign_title_and_description + assign_labels + assign_milestone end merge_request @@ -135,6 +137,20 @@ module MergeRequests append_closes_description end + def assign_labels + return unless target_project.issues_enabled? && issue + return if merge_request.label_ids&.any? + + merge_request.label_ids = issue.try(:label_ids) + end + + def assign_milestone + return unless target_project.issues_enabled? && issue + return if merge_request.milestone_id.present? + + merge_request.milestone_id = issue.try(:milestone_id) + end + def append_closes_description return unless issue&.to_reference.present? @@ -185,7 +201,9 @@ module MergeRequests end def issue - @issue ||= target_project.get_issue(issue_iid, current_user) + strong_memoize(:issue) do + target_project.get_issue(issue_iid, current_user) + end end end end diff --git a/app/services/merge_requests/create_from_issue_service.rb b/app/services/merge_requests/create_from_issue_service.rb index fd91dc4acd0..d9a29693987 100644 --- a/app/services/merge_requests/create_from_issue_service.rb +++ b/app/services/merge_requests/create_from_issue_service.rb @@ -16,8 +16,6 @@ module MergeRequests def execute return error('Invalid issue iid') unless @issue_iid.present? && issue.present? - params[:label_ids] = issue.label_ids if issue.label_ids.any? - result = CreateBranchService.new(project, current_user).execute(branch_name, ref) return result if result[:status] == :error @@ -58,8 +56,7 @@ module MergeRequests source_project_id: project.id, source_branch: branch_name, target_project_id: project.id, - target_branch: ref, - milestone_id: issue.milestone_id + target_branch: ref } end diff --git a/changelogs/unreleased/50657-migrate-issue-labels-and-milestone-to-related-mr-on-creation.yml b/changelogs/unreleased/50657-migrate-issue-labels-and-milestone-to-related-mr-on-creation.yml new file mode 100644 index 00000000000..65419d3ac77 --- /dev/null +++ b/changelogs/unreleased/50657-migrate-issue-labels-and-milestone-to-related-mr-on-creation.yml @@ -0,0 +1,5 @@ +--- +title: Merge request copies all associated issue labels and milestone on creation +merge_request: 21383 +author: +type: added diff --git a/spec/services/merge_requests/build_service_spec.rb b/spec/services/merge_requests/build_service_spec.rb index 0ced5d1b6d6..9f1da7d9419 100644 --- a/spec/services/merge_requests/build_service_spec.rb +++ b/spec/services/merge_requests/build_service_spec.rb @@ -13,6 +13,8 @@ describe MergeRequests::BuildService do let(:description) { nil } let(:source_branch) { 'feature-branch' } let(:target_branch) { 'master' } + let(:milestone_id) { nil } + let(:label_ids) { [] } let(:merge_request) { service.execute } let(:compare) { double(:compare, commits: commits) } let(:commit_1) { double(:commit_1, sha: 'f00ba7', safe_message: "Initial commit\n\nCreate the app") } @@ -25,7 +27,9 @@ describe MergeRequests::BuildService do source_branch: source_branch, target_branch: target_branch, source_project: source_project, - target_project: target_project) + target_project: target_project, + milestone_id: milestone_id, + label_ids: label_ids) end before do @@ -179,6 +183,33 @@ describe MergeRequests::BuildService do expect(merge_request.description).to eq(expected_description) end end + + context 'when the source branch matches an internal issue' do + let(:label) { create(:label, project: project) } + let(:milestone) { create(:milestone, project: project) } + let(:source_branch) { '123-fix-issue' } + + before do + issue.update!(iid: 123, labels: [label], milestone: milestone) + end + + it 'assigns the issue label and milestone' do + expect(merge_request.milestone).to eq(milestone) + expect(merge_request.labels).to match_array([label]) + end + + context 'when milestone_id and label_ids are shared in the params' do + let(:label2) { create(:label, project: project) } + let(:milestone2) { create(:milestone, project: project) } + let(:label_ids) { [label2.id] } + let(:milestone_id) { milestone2.id } + + it 'assigns milestone_id and label_ids instead of issue labels and milestone' do + expect(merge_request.milestone).to eq(milestone2) + expect(merge_request.labels).to match_array([label2]) + end + end + end end end |