diff --git a/app/controllers/projects/labels_controller.rb b/app/controllers/projects/labels_controller.rb
index 44715dd6f7a7e22b1ebef64413b6b165460c3984..1593b5c1afbd7b1f2a158ef2baf0d62804fa048a 100644
--- a/app/controllers/projects/labels_controller.rb
+++ b/app/controllers/projects/labels_controller.rb
@@ -2,12 +2,13 @@ class Projects::LabelsController < Projects::ApplicationController
include ToggleSubscriptionAction
before_action :module_enabled
- before_action :label, only: [:edit, :update, :destroy]
+ before_action :label, only: [:edit, :update, :destroy, :promote]
before_action :find_labels, only: [:index, :set_priorities, :remove_priority, :toggle_subscription]
before_action :authorize_read_label!
before_action :authorize_admin_labels!, only: [:new, :create, :edit, :update,
:generate, :destroy, :remove_priority,
:set_priorities]
+ before_action :authorize_admin_group!, only: [:promote]
respond_to :js, :html
@@ -102,6 +103,32 @@ class Projects::LabelsController < Projects::ApplicationController
end
end
+ def promote
+ promote_service = Labels::PromoteService.new(@project, @current_user)
+
+ begin
+ return render_404 unless promote_service.execute(@label)
+ respond_to do |format|
+ format.html do
+ redirect_to(namespace_project_labels_path(@project.namespace, @project),
+ notice: 'Label was promoted to a Group Label')
+ end
+ format.js
+ end
+ rescue ActiveRecord::RecordInvalid => e
+ Gitlab::AppLogger.error "Failed to promote label \"#{@label.title}\" to group label"
+ Gitlab::AppLogger.error e
+
+ respond_to do |format|
+ format.html do
+ redirect_to(namespace_project_labels_path(@project.namespace, @project),
+ notice: 'Failed to promote label due to internal error. Please contact administrators.')
+ end
+ format.js
+ end
+ end
+ end
+
protected
def module_enabled
@@ -129,4 +156,8 @@ class Projects::LabelsController < Projects::ApplicationController
def authorize_admin_labels!
return render_404 unless can?(current_user, :admin_label, @project)
end
+
+ def authorize_admin_group!
+ return render_404 unless can?(current_user, :admin_group, @project.group)
+ end
end
diff --git a/app/services/labels/promote_service.rb b/app/services/labels/promote_service.rb
new file mode 100644
index 0000000000000000000000000000000000000000..76d0ba67b07af6616831f8e64f9369ed210c565f
--- /dev/null
+++ b/app/services/labels/promote_service.rb
@@ -0,0 +1,71 @@
+module Labels
+ class PromoteService < BaseService
+ BATCH_SIZE = 1000
+
+ def execute(label)
+ return unless project.group &&
+ label.is_a?(ProjectLabel)
+
+ Label.transaction do
+ new_label = clone_label_to_group_label(label)
+
+ label_ids_for_merge(new_label).find_in_batches(batch_size: BATCH_SIZE) do |batched_ids|
+ update_issuables(new_label, batched_ids)
+ update_issue_board_lists(new_label, batched_ids)
+ update_priorities(new_label, batched_ids)
+ # Order is important, project labels need to be last
+ update_project_labels(batched_ids)
+ end
+
+ # We skipped validations during creation. Let's run them now, after deleting conflicting labels
+ raise ActiveRecord::RecordInvalid.new(new_label) unless new_label.valid?
+ new_label
+ end
+ end
+
+ private
+
+ def label_ids_for_merge(new_label)
+ LabelsFinder.
+ new(current_user, title: new_label.title, group_id: project.group.id).
+ execute(skip_authorization: true).
+ where.not(id: new_label).
+ select(:id) # Can't use pluck() to avoid object-creation because of the batching
+ end
+
+ def update_issuables(new_label, label_ids)
+ LabelLink.
+ where(label: label_ids).
+ update_all(label_id: new_label)
+ end
+
+ def update_issue_board_lists(new_label, label_ids)
+ List.
+ where(label: label_ids).
+ update_all(label_id: new_label)
+ end
+
+ def update_priorities(new_label, label_ids)
+ LabelPriority.
+ where(label: label_ids).
+ update_all(label_id: new_label)
+ end
+
+ def update_project_labels(label_ids)
+ Label.where(id: label_ids).delete_all
+ end
+
+ def clone_label_to_group_label(label)
+ params = label.attributes.slice('title', 'description', 'color')
+ # Since the title of the new label has to be the same as the previous labels
+ # and we're merging old labels in batches we'll skip validation to omit 2-step
+ # merge process and do it in one batch
+ # We'll be forcing validation at the end of the transaction to ensure everything
+ # was merged correctly
+ new_label = GroupLabel.new(params.merge(group: project.group))
+ new_label.save(validate: false)
+
+ new_label
+ end
+ end
+end
diff --git a/app/views/shared/_label.html.haml b/app/views/shared/_label.html.haml
index 4bfdf94b913bf6d7110744ba59238a4cfd1be874..ead9b84b991c80fce69c88c82b5f4cf79c625143 100644
--- a/app/views/shared/_label.html.haml
+++ b/app/views/shared/_label.html.haml
@@ -66,6 +66,10 @@
%a.js-subscribe-button{ data: { url: toggle_subscription_group_label_path(label.group, label) } }
Group level
+ - if label.is_a?(ProjectLabel) && label.project.group && can?(current_user, :admin_group, label.project.group)
+ = link_to promote_namespace_project_label_path(label.project.namespace, label.project, label), title: "Promote to Group Label", class: 'btn btn-transparent btn-action', data: {confirm: "Promoting this label will make this label available to all projects inside this group. Existing project labels with the same name will be merged. Are you sure?", toggle: "tooltip"}, method: :post do
+ %span.sr-only Promote to Group
+ = icon('level-up')
- if can?(current_user, :admin_label, label)
= link_to edit_label_path(label), title: "Edit", class: 'btn btn-transparent btn-action', data: {toggle: "tooltip"} do
%span.sr-only Edit
diff --git a/changelogs/unreleased/label-promotion.yml b/changelogs/unreleased/label-promotion.yml
new file mode 100644
index 0000000000000000000000000000000000000000..2ab997bf42027079a612850b83cf491fa485fac6
--- /dev/null
+++ b/changelogs/unreleased/label-promotion.yml
@@ -0,0 +1,4 @@
+---
+title: "Project labels can now be promoted to group labels"
+merge_request: 7242
+author: Olaf Tomalka
diff --git a/config/routes/project.rb b/config/routes/project.rb
index 6620b765e026c8a5cb62e9feec15c98f3cfacc28..f36febc6e04386f56f44f296d3741a420714d4cf 100644
--- a/config/routes/project.rb
+++ b/config/routes/project.rb
@@ -220,6 +220,7 @@ constraints(ProjectUrlConstrainer.new) do
end
member do
+ post :promote
post :toggle_subscription
delete :remove_priority
end
diff --git a/spec/controllers/projects/labels_controller_spec.rb b/spec/controllers/projects/labels_controller_spec.rb
index ec6cea5c0f4332e6d0ca2a73f16d3727537d71b9..3e0326dd47d79cfa4d12e6a633cb6d7af5f2b271 100644
--- a/spec/controllers/projects/labels_controller_spec.rb
+++ b/spec/controllers/projects/labels_controller_spec.rb
@@ -112,4 +112,49 @@ describe Projects::LabelsController do
post :toggle_subscription, namespace_id: project.namespace.to_param, project_id: project.to_param, id: label.to_param
end
end
+
+ describe 'POST #promote' do
+ let!(:promoted_label_name) { "Promoted Label" }
+ let!(:label_1) { create(:label, title: promoted_label_name, project: project) }
+
+ context 'not group owner' do
+ it 'denies access' do
+ post :promote, namespace_id: project.namespace.to_param, project_id: project.to_param, id: label_1.to_param
+
+ expect(response).to have_http_status(404)
+ end
+ end
+
+ context 'group owner' do
+ before do
+ GroupMember.add_users_to_group(group, [user], :owner)
+ end
+
+ it 'gives access' do
+ post :promote, namespace_id: project.namespace.to_param, project_id: project.to_param, id: label_1.to_param
+
+ expect(response).to redirect_to(namespace_project_labels_path)
+ end
+
+ it 'promotes the label' do
+ post :promote, namespace_id: project.namespace.to_param, project_id: project.to_param, id: label_1.to_param
+
+ expect(Label.where(id: label_1.id)).to be_empty
+ expect(GroupLabel.find_by(title: promoted_label_name)).not_to be_nil
+ end
+
+ context 'service raising InvalidRecord' do
+ before do
+ expect_any_instance_of(Labels::PromoteService).to receive(:execute) do |label|
+ raise ActiveRecord::RecordInvalid.new(label_1)
+ end
+ end
+
+ it 'returns to label list' do
+ post :promote, namespace_id: project.namespace.to_param, project_id: project.to_param, id: label_1.to_param
+ expect(response).to redirect_to(namespace_project_labels_path)
+ end
+ end
+ end
+ end
end
diff --git a/spec/services/labels/promote_service_spec.rb b/spec/services/labels/promote_service_spec.rb
new file mode 100644
index 0000000000000000000000000000000000000000..4b90ad19640cf07eb95540eb69b4e172fe950f92
--- /dev/null
+++ b/spec/services/labels/promote_service_spec.rb
@@ -0,0 +1,187 @@
+require 'spec_helper'
+
+describe Labels::PromoteService, services: true do
+ describe '#execute' do
+ let!(:user) { create(:user) }
+
+ context 'project without group' do
+ let!(:project_1) { create(:empty_project) }
+
+ let!(:project_label_1_1) { create(:label, project: project_1) }
+
+ subject(:service) { described_class.new(project_1, user) }
+
+ it 'fails on project without group' do
+ expect(service.execute(project_label_1_1)).to be_falsey
+ end
+ end
+
+ context 'project with group' do
+ let!(:promoted_label_name) { "Promoted Label" }
+ let!(:untouched_label_name) { "Untouched Label" }
+ let!(:promoted_description) { "Promoted Description" }
+ let!(:promoted_color) { "#0000FF" }
+ let!(:label_2_1_priority) { 1 }
+ let!(:label_3_1_priority) { 2 }
+
+ let!(:group_1) { create(:group) }
+ let!(:group_2) { create(:group) }
+
+ let!(:project_1) { create(:empty_project, namespace: group_1) }
+ let!(:project_2) { create(:empty_project, namespace: group_1) }
+ let!(:project_3) { create(:empty_project, namespace: group_1) }
+ let!(:project_4) { create(:empty_project, namespace: group_2) }
+
+ # Labels/issues can't be lazily created so we might as well eager initialize
+ # all other objects too since we use them inside
+ let!(:project_label_1_1) { create(:label, project: project_1, name: promoted_label_name, color: promoted_color, description: promoted_description) }
+ let!(:project_label_1_2) { create(:label, project: project_1, name: untouched_label_name) }
+ let!(:project_label_2_1) { create(:label, project: project_2, priority: label_2_1_priority, name: promoted_label_name, color: "#FF0000") }
+ let!(:project_label_3_1) { create(:label, project: project_3, priority: label_3_1_priority, name: promoted_label_name) }
+ let!(:project_label_3_2) { create(:label, project: project_3, priority: 1, name: untouched_label_name) }
+ let!(:project_label_4_1) { create(:label, project: project_4, name: promoted_label_name) }
+
+ let!(:issue_1_1) { create(:labeled_issue, project: project_1, labels: [project_label_1_1, project_label_1_2]) }
+ let!(:issue_1_2) { create(:labeled_issue, project: project_1, labels: [project_label_1_2]) }
+ let!(:issue_2_1) { create(:labeled_issue, project: project_2, labels: [project_label_2_1]) }
+ let!(:issue_4_1) { create(:labeled_issue, project: project_4, labels: [project_label_4_1]) }
+
+ let!(:merge_3_1) { create(:labeled_merge_request, source_project: project_3, target_project: project_3, labels: [project_label_3_1, project_label_3_2]) }
+
+ let!(:issue_board_2_1) { create(:board, project: project_2) }
+ let!(:issue_board_list_2_1) { create(:list, board: issue_board_2_1, label: project_label_2_1) }
+
+ let(:new_label) { group_1.labels.find_by(title: promoted_label_name) }
+
+ subject(:service) { described_class.new(project_1, user) }
+
+ it 'fails on group label' do
+ group_label = create(:group_label, group: group_1)
+
+ expect(service.execute(group_label)).to be_falsey
+ end
+
+ it 'is truthy on success' do
+ expect(service.execute(project_label_1_1)).to be_truthy
+ end
+
+ it 'recreates the label as a group label' do
+ expect { service.execute(project_label_1_1) }.
+ to change(project_1.labels, :count).by(-1).
+ and change(group_1.labels, :count).by(1)
+ expect(new_label).not_to be_nil
+ end
+
+ it 'copies title, description and color' do
+ service.execute(project_label_1_1)
+
+ expect(new_label.title).to eq(promoted_label_name)
+ expect(new_label.description).to eq(promoted_description)
+ expect(new_label.color).to eq(promoted_color)
+ end
+
+ it 'merges labels with the same name in group' do
+ expect { service.execute(project_label_1_1) }.to change(project_2.labels, :count).by(-1).and \
+ change(project_3.labels, :count).by(-1)
+ end
+
+ it 'recreates priorities' do
+ service.execute(project_label_1_1)
+
+ expect(new_label.priority(project_1)).to be_nil
+ expect(new_label.priority(project_2)).to eq(label_2_1_priority)
+ expect(new_label.priority(project_3)).to eq(label_3_1_priority)
+ end
+
+ it 'does not touch project out of promoted group' do
+ service.execute(project_label_1_1)
+ project_4_new_label = project_4.labels.find_by(title: promoted_label_name)
+
+ expect(project_4_new_label).not_to be_nil
+ expect(project_4_new_label.id).to eq(project_label_4_1.id)
+ end
+
+ it 'does not touch out of group priority' do
+ service.execute(project_label_1_1)
+
+ expect(new_label.priority(project_4)).to be_nil
+ end
+
+ it 'relinks issue with the promoted label' do
+ service.execute(project_label_1_1)
+ issue_label = issue_1_1.labels.find_by(title: promoted_label_name)
+
+ expect(issue_label).not_to be_nil
+ expect(issue_label.id).to eq(new_label.id)
+ end
+
+ it 'does not remove untouched labels from issue' do
+ expect { service.execute(project_label_1_1) }.not_to change(issue_1_1.labels, :count)
+ end
+
+ it 'does not relink untouched label in issue' do
+ service.execute(project_label_1_1)
+ issue_label = issue_1_2.labels.find_by(title: untouched_label_name)
+
+ expect(issue_label).not_to be_nil
+ expect(issue_label.id).to eq(project_label_1_2.id)
+ end
+
+ it 'relinks issues with merged labels' do
+ service.execute(project_label_1_1)
+ issue_label = issue_2_1.labels.find_by(title: promoted_label_name)
+
+ expect(issue_label).not_to be_nil
+ expect(issue_label.id).to eq(new_label.id)
+ end
+
+ it 'does not relink issues from other group' do
+ service.execute(project_label_1_1)
+ issue_label = issue_4_1.labels.find_by(title: promoted_label_name)
+
+ expect(issue_label).not_to be_nil
+ expect(issue_label.id).to eq(project_label_4_1.id)
+ end
+
+ it 'updates merge request' do
+ service.execute(project_label_1_1)
+ merge_label = merge_3_1.labels.find_by(title: promoted_label_name)
+
+ expect(merge_label).not_to be_nil
+ expect(merge_label.id).to eq(new_label.id)
+ end
+
+ it 'updates board lists' do
+ service.execute(project_label_1_1)
+ list = issue_board_2_1.lists.find_by(label: new_label)
+
+ expect(list).not_to be_nil
+ end
+
+ # In case someone adds a new relation to Label.rb and forgets to relink it
+ # and the database doesn't have foreign key constraints
+ it 'relinks all relations' do
+ service.execute(project_label_1_1)
+
+ Label.reflect_on_all_associations.each do |association|
+ expect(project_label_1_1.send(association.name).any?).to be_falsey
+ end
+ end
+
+ context 'with invalid group label' do
+ before do
+ allow(service).to receive(:clone_label_to_group_label).and_wrap_original do |m, *args|
+ label = m.call(*args)
+ allow(label).to receive(:valid?).and_return(false)
+
+ label
+ end
+ end
+
+ it 'raises an exception' do
+ expect { service.execute(project_label_1_1) }.to raise_error(ActiveRecord::RecordInvalid)
+ end
+ end
+ end
+ end
+end