diff --git a/app/services/labels/transfer_service.rb b/app/services/labels/transfer_service.rb index f91b3724aef..514679ed29d 100644 --- a/app/services/labels/transfer_service.rb +++ b/app/services/labels/transfer_service.rb @@ -4,48 +4,75 @@ # module Labels class TransferService - def initialize(current_user, group, project) + def initialize(current_user, old_group, project) @current_user = current_user - @group = group + @old_group = old_group @project = project end def execute - return unless group.present? + return unless old_group.present? Label.transaction do - labels_to_transfer = Label.where(id: label_links.select(:label_id)) - labels_to_transfer.find_each do |label| new_label_id = find_or_create_label!(label) next if new_label_id == label.id - LabelLink.where(label_id: label.id).update_all(label_id: new_label_id) - LabelPriority.where(project_id: project.id, label_id: label.id).update_all(label_id: new_label_id) + update_label_links(group_labels_applied_to_issues, old_label_id: label.id, new_label_id: new_label_id) + update_label_links(group_labels_applied_to_merge_requests, old_label_id: label.id, new_label_id: new_label_id) + update_label_priorities(old_label_id: label.id, new_label_id: new_label_id) end end end private - attr_reader :current_user, :group, :project + attr_reader :current_user, :old_group, :project - def label_links - label_link_ids = [] - label_link_ids << LabelLink.where(target: project.issues, label: group.labels).select(:id) - label_link_ids << LabelLink.where(target: project.merge_requests, label: group.labels).select(:id) + def labels_to_transfer + label_ids = [] + label_ids << group_labels_applied_to_issues.select(:id) + label_ids << group_labels_applied_to_merge_requests.select(:id) - union = Gitlab::SQL::Union.new(label_link_ids) + union = Gitlab::SQL::Union.new(label_ids) - LabelLink.where("label_links.id IN (#{union.to_sql})") + Label.where("labels.id IN (#{union.to_sql})").reorder(nil).uniq + end + + def group_labels_applied_to_issues + Label.joins(:issues). + where( + issues: { project_id: project.id }, + labels: { type: 'GroupLabel', group_id: old_group.id } + ) + end + + def group_labels_applied_to_merge_requests + Label.joins(:merge_requests). + where( + merge_requests: { target_project_id: project.id }, + labels: { type: 'GroupLabel', group_id: old_group.id } + ) end def find_or_create_label!(label) - params = label.attributes.slice('title', 'description', 'color') + params = label.attributes.slice('title', 'description', 'color') new_label = FindOrCreateService.new(current_user, project, params).execute new_label.id end + + def update_label_links(labels, old_label_id:, new_label_id:) + LabelLink.joins(:label). + merge(labels). + where(label_id: old_label_id). + update_all(label_id: new_label_id) + end + + def update_label_priorities(old_label_id:, new_label_id:) + LabelPriority.where(project_id: project.id, label_id: old_label_id). + update_all(label_id: new_label_id) + end end end diff --git a/spec/services/labels/transfer_service_spec.rb b/spec/services/labels/transfer_service_spec.rb index cb09c16698a..ddf3527dc0f 100644 --- a/spec/services/labels/transfer_service_spec.rb +++ b/spec/services/labels/transfer_service_spec.rb @@ -5,33 +5,38 @@ describe Labels::TransferService, services: true do let(:user) { create(:user) } let(:group_1) { create(:group) } let(:group_2) { create(:group) } - let(:project) { create(:project, namespace: group_2) } + let(:group_3) { create(:group) } + let(:project_1) { create(:project, namespace: group_2) } + let(:project_2) { create(:project, namespace: group_3) } let(:group_label_1) { create(:group_label, group: group_1, name: 'Group Label 1') } let(:group_label_2) { create(:group_label, group: group_1, name: 'Group Label 2') } let(:group_label_3) { create(:group_label, group: group_1, name: 'Group Label 3') } let(:group_label_4) { create(:group_label, group: group_2, name: 'Group Label 4') } - let(:project_label_1) { create(:label, project: project, name: 'Project Label 1') } + let(:group_label_5) { create(:group_label, group: group_3, name: 'Group Label 5') } + let(:project_label_1) { create(:label, project: project_1, name: 'Project Label 1') } - subject(:service) { described_class.new(user, group_1, project) } + subject(:service) { described_class.new(user, group_1, project_1) } before do - create(:labeled_issue, project: project, labels: [group_label_1]) - create(:labeled_issue, project: project, labels: [group_label_4]) - create(:labeled_issue, project: project, labels: [project_label_1]) - create(:labeled_merge_request, source_project: project, labels: [group_label_1, group_label_2]) + create(:labeled_issue, project: project_1, labels: [group_label_1]) + create(:labeled_issue, project: project_1, labels: [group_label_4]) + create(:labeled_issue, project: project_1, labels: [project_label_1]) + create(:labeled_issue, project: project_2, labels: [group_label_5]) + create(:labeled_merge_request, source_project: project_1, labels: [group_label_1, group_label_2]) + create(:labeled_merge_request, source_project: project_2, labels: [group_label_5]) end it 'recreates the missing group labels at project level' do - expect { service.execute }.to change(project.labels, :count).by(2) + expect { service.execute }.to change(project_1.labels, :count).by(2) end it 'recreates label priorities related to the missing group labels' do - create(:label_priority, project: project, label: group_label_1, priority: 1) + create(:label_priority, project: project_1, label: group_label_1, priority: 1) service.execute - new_project_label = project.labels.find_by(title: group_label_1.title) + new_project_label = project_1.labels.find_by(title: group_label_1.title) expect(new_project_label.id).not_to eq group_label_1.id expect(new_project_label.priorities).not_to be_empty end @@ -39,13 +44,13 @@ describe Labels::TransferService, services: true do it 'does not recreate missing group labels that are not applied to issues or merge requests' do service.execute - expect(project.labels.where(title: group_label_3.title)).to be_empty + expect(project_1.labels.where(title: group_label_3.title)).to be_empty end it 'does not recreate missing group labels that already exist in the project group' do service.execute - expect(project.labels.where(title: group_label_4.title)).to be_empty + expect(project_1.labels.where(title: group_label_4.title)).to be_empty end end end