Commit 342fa0f8 authored by Tiago Botelho's avatar Tiago Botelho Committed by Nick Thomas

Resolve "Migrate issue labels and milestone to related merge request"

parent a062de4e
...@@ -49,7 +49,7 @@ module Issuable ...@@ -49,7 +49,7 @@ module Issuable
end end
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 :labels, through: :label_links
has_many :todos, as: :target, dependent: :destroy # rubocop:disable Cop/ActiveRecordDependent has_many :todos, as: :target, dependent: :destroy # rubocop:disable Cop/ActiveRecordDependent
......
...@@ -3,7 +3,7 @@ ...@@ -3,7 +3,7 @@
class LabelLink < ActiveRecord::Base class LabelLink < ActiveRecord::Base
include Importable 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 belongs_to :label
validates :target, presence: true, unless: :importing? validates :target, presence: true, unless: :importing?
......
...@@ -129,28 +129,19 @@ class IssuableBaseService < BaseService ...@@ -129,28 +129,19 @@ class IssuableBaseService < BaseService
params.merge!(command_params) params.merge!(command_params)
end 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) def create(issuable)
handle_quick_actions_on_create(issuable) handle_quick_actions_on_create(issuable)
filter_params(issuable) filter_params(issuable)
params.delete(:state_event) params.delete(:state_event)
params[:author] ||= current_user params[:author] ||= current_user
params[:label_ids] = issuable.label_ids.to_a + process_label_ids(params)
label_ids = process_label_ids(params)
issuable.assign_attributes(params) issuable.assign_attributes(params)
before_create(issuable) before_create(issuable)
if params.present? && create_issuable(issuable, params, label_ids: label_ids) if issuable.save
after_create(issuable) after_create(issuable)
execute_hooks(issuable) execute_hooks(issuable)
invalidate_cache_counts(issuable, users: issuable.assignees) invalidate_cache_counts(issuable, users: issuable.assignees)
......
...@@ -20,6 +20,8 @@ module MergeRequests ...@@ -20,6 +20,8 @@ module MergeRequests
if merge_request.can_be_created if merge_request.can_be_created
compare_branches compare_branches
assign_title_and_description assign_title_and_description
assign_labels
assign_milestone
end end
merge_request merge_request
...@@ -135,6 +137,20 @@ module MergeRequests ...@@ -135,6 +137,20 @@ module MergeRequests
append_closes_description append_closes_description
end 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 def append_closes_description
return unless issue&.to_reference.present? return unless issue&.to_reference.present?
...@@ -185,7 +201,9 @@ module MergeRequests ...@@ -185,7 +201,9 @@ module MergeRequests
end end
def issue 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 end
end end
...@@ -16,8 +16,6 @@ module MergeRequests ...@@ -16,8 +16,6 @@ module MergeRequests
def execute def execute
return error('Invalid issue iid') unless @issue_iid.present? && issue.present? 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) result = CreateBranchService.new(project, current_user).execute(branch_name, ref)
return result if result[:status] == :error return result if result[:status] == :error
...@@ -58,8 +56,7 @@ module MergeRequests ...@@ -58,8 +56,7 @@ module MergeRequests
source_project_id: project.id, source_project_id: project.id,
source_branch: branch_name, source_branch: branch_name,
target_project_id: project.id, target_project_id: project.id,
target_branch: ref, target_branch: ref
milestone_id: issue.milestone_id
} }
end end
......
---
title: Merge request copies all associated issue labels and milestone on creation
merge_request: 21383
author:
type: added
...@@ -13,6 +13,8 @@ describe MergeRequests::BuildService do ...@@ -13,6 +13,8 @@ describe MergeRequests::BuildService do
let(:description) { nil } let(:description) { nil }
let(:source_branch) { 'feature-branch' } let(:source_branch) { 'feature-branch' }
let(:target_branch) { 'master' } let(:target_branch) { 'master' }
let(:milestone_id) { nil }
let(:label_ids) { [] }
let(:merge_request) { service.execute } let(:merge_request) { service.execute }
let(:compare) { double(:compare, commits: commits) } let(:compare) { double(:compare, commits: commits) }
let(:commit_1) { double(:commit_1, sha: 'f00ba7', safe_message: "Initial commit\n\nCreate the app") } let(:commit_1) { double(:commit_1, sha: 'f00ba7', safe_message: "Initial commit\n\nCreate the app") }
...@@ -25,7 +27,9 @@ describe MergeRequests::BuildService do ...@@ -25,7 +27,9 @@ describe MergeRequests::BuildService do
source_branch: source_branch, source_branch: source_branch,
target_branch: target_branch, target_branch: target_branch,
source_project: source_project, source_project: source_project,
target_project: target_project) target_project: target_project,
milestone_id: milestone_id,
label_ids: label_ids)
end end
before do before do
...@@ -179,6 +183,33 @@ describe MergeRequests::BuildService do ...@@ -179,6 +183,33 @@ describe MergeRequests::BuildService do
expect(merge_request.description).to eq(expected_description) expect(merge_request.description).to eq(expected_description)
end end
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
end end
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment