Merge branch 'jp-label-fix' into 'master'

Fix display of promote to group label

Closes #62200

See merge request gitlab-org/gitlab-ce!28637

(cherry picked from commit 9c2d0d87581f34567e33de1b384f1f360edcd4b7)

f9a55f93 Fix display of promote to group label
52764ec5 Apply suggestion to spec/helpers/labels_helper_spec.rb
58dc21e7 Apply suggestion to spec/features/projects/labels/user_promotes_label_spec.rb

10 files changed, 101 insertions, 19 deletions

diff --git a/app/helpers/labels_helper.rb b/app/helpers/labels_helper.rb
index 76300e791e6..acc8aeae282 100644
--- a/app/helpers/labels_helper.rb
+++ b/app/helpers/labels_helper.rb
@@ -5,7 +5,7 @@ module LabelsHelper
   include ActionView::Helpers::TagHelper
 
   def show_label_issuables_link?(label, issuables_type, current_user: nil, project: nil)
-    return true if label.is_a?(GroupLabel)
+    return true unless label.project_label?
     return true unless project
 
     project.feature_available?(issuables_type, current_user)
@@ -159,13 +159,6 @@ module LabelsHelper
     label.subscribed?(current_user, project) ? 'Unsubscribe' : 'Subscribe'
   end
 
-  def label_deletion_confirm_text(label)
-    case label
-    when GroupLabel then _('Remove this label? This will affect all projects within the group. Are you sure?')
-    when ProjectLabel then _('Remove this label? Are you sure?')
-    end
-  end
-
   def create_label_title(subject)
     case subject
     when Group
@@ -200,7 +193,7 @@ module LabelsHelper
   end
 
   def label_status_tooltip(label, status)
-    type = label.is_a?(ProjectLabel) ? 'project' : 'group'
+    type = label.project_label? ? 'project' : 'group'
     level = status.unsubscribed? ? type : status.sub('-level', '')
     action = status.unsubscribed? ? 'Subscribe' : 'Unsubscribe'
 
diff --git a/app/presenters/label_presenter.rb b/app/presenters/label_presenter.rb
index 5227ef353c3..1077bf543d9 100644
--- a/app/presenters/label_presenter.rb
+++ b/app/presenters/label_presenter.rb
@@ -35,6 +35,14 @@ class LabelPresenter < Gitlab::View::Presenter::Delegated
     issuable_subject.is_a?(Project) && label.is_a?(GroupLabel)
   end
 
+  def project_label?
+    label.is_a?(ProjectLabel)
+  end
+
+  def subject_name
+    label.subject.name
+  end
+
   private
 
   def context_subject
diff --git a/app/views/shared/_delete_label_modal.html.haml b/app/views/shared/_delete_label_modal.html.haml
index 6bd8cadd7d9..f37dd2cdf02 100644
--- a/app/views/shared/_delete_label_modal.html.haml
+++ b/app/views/shared/_delete_label_modal.html.haml
@@ -9,7 +9,7 @@
       .modal-body
         %p
           %strong= label.name
-          %span will be permanently deleted from #{label.subject.name}. This cannot be undone.
+          %span will be permanently deleted from #{label.subject_name}. This cannot be undone.
 
       .modal-footer
         %a{ href: '#', data: { dismiss: 'modal' }, class: 'btn btn-default' } Cancel
diff --git a/app/views/shared/_label.html.haml b/app/views/shared/_label.html.haml
index 2b4a24a001f..c4b7ef481fd 100644
--- a/app/views/shared/_label.html.haml
+++ b/app/views/shared/_label.html.haml
@@ -30,7 +30,7 @@
             = sprite_icon('ellipsis_v')
           .dropdown-menu.dropdown-open-left
             %ul
-              - if label.is_a?(ProjectLabel) && label.project.group && can?(current_user, :admin_label, label.project.group)
+              - if label.project_label? && label.project.group && can?(current_user, :admin_label, label.project.group)
                 %li
                   %button.js-promote-project-label-button.btn.btn-transparent.btn-action{ disabled: true, type: 'button',
                     data: { url: promote_project_label_path(label.project, label),
diff --git a/changelogs/unreleased/jp-label-fix.yml b/changelogs/unreleased/jp-label-fix.yml
new file mode 100644
index 00000000000..de64286cc1f
--- /dev/null
+++ b/changelogs/unreleased/jp-label-fix.yml
@@ -0,0 +1,5 @@
+---
+title: Fix display of 'Promote to group label' button.
+merge_request:
+author:
+type: fixed
diff --git a/locale/gitlab.pot b/locale/gitlab.pot
index b67818768e3..875543e5385 100644
--- a/locale/gitlab.pot
+++ b/locale/gitlab.pot
@@ -7921,12 +7921,6 @@ msgstr ""
 msgid "Remove spent time"
 msgstr ""
 
-msgid "Remove this label? Are you sure?"
-msgstr ""
-
-msgid "Remove this label? This will affect all projects within the group. Are you sure?"
-msgstr ""
-
 msgid "Remove time estimate"
 msgstr ""
 
diff --git a/spec/features/projects/labels/user_promotes_label_spec.rb b/spec/features/projects/labels/user_promotes_label_spec.rb
new file mode 100644
index 00000000000..fdecafd4c50
--- /dev/null
+++ b/spec/features/projects/labels/user_promotes_label_spec.rb
@@ -0,0 +1,34 @@
+# frozen_string_literal: true
+
+require 'spec_helper'
+
+describe 'User promotes label' do
+  set(:group) { create(:group) }
+  set(:user) { create(:user) }
+  set(:project) { create(:project, namespace: group) }
+  set(:label) { create(:label, project: project) }
+
+  context 'when user can admin group labels' do
+    before do
+      group.add_developer(user)
+      sign_in(user)
+      visit(project_labels_path(project))
+    end
+
+    it "shows label promote button" do
+      expect(page).to have_selector('.js-promote-project-label-button')
+    end
+  end
+
+  context 'when user cannot admin group labels' do
+    before do
+      project.add_developer(user)
+      sign_in(user)
+      visit(project_labels_path(project))
+    end
+
+    it "does not show label promote button" do
+      expect(page).not_to have_selector('.js-promote-project-label-button')
+    end
+  end
+end
diff --git a/spec/features/projects/labels/user_removes_labels_spec.rb b/spec/features/projects/labels/user_removes_labels_spec.rb
index b0ce03a1c31..c231e54decd 100644
--- a/spec/features/projects/labels/user_removes_labels_spec.rb
+++ b/spec/features/projects/labels/user_removes_labels_spec.rb
@@ -21,8 +21,11 @@ describe "User removes labels" do
         page.first(".label-list-item") do
           first('.js-label-options-dropdown').click
           first(".remove-row").click
-          first(:link, "Delete label").click
         end
+
+        expect(page).to have_content("#{label.title} will be permanently deleted from #{project.name}. This cannot be undone.")
+
+        first(:link, "Delete label").click
       end
 
       expect(page).to have_content("Label was removed").and have_no_content(label.title)
diff --git a/spec/helpers/labels_helper_spec.rb b/spec/helpers/labels_helper_spec.rb
index 58eaf991d6e..314305d7a8e 100644
--- a/spec/helpers/labels_helper_spec.rb
+++ b/spec/helpers/labels_helper_spec.rb
@@ -6,7 +6,7 @@ describe LabelsHelper do
       let(:context_project) { project }
 
       context "when asking for a #{issuables_type} link" do
-        subject { show_label_issuables_link?(label, issuables_type, project: context_project) }
+        subject { show_label_issuables_link?(label.present(issuable_subject: nil), issuables_type, project: context_project) }
 
         context "when #{issuables_type} are enabled for the project" do
           let(:project) { create(:project, "#{issuables_type}_access_level": ProjectFeature::ENABLED) }
@@ -279,4 +279,21 @@ describe LabelsHelper do
       expect(label.color).to eq('bar')
     end
   end
+
+  describe '#label_status_tooltip' do
+    let(:status) { 'unsubscribed'.inquiry }
+    subject { label_status_tooltip(label.present(issuable_subject: nil), status) }
+
+    context 'with a project label' do
+      let(:label) { create(:label, title: 'bug') }
+
+      it { is_expected.to eq('Subscribe at project level') }
+    end
+
+    context 'with a group label' do
+      let(:label) { create(:group_label, title: 'bug') }
+
+      it { is_expected.to eq('Subscribe at group level') }
+    end
+  end
 end
diff --git a/spec/presenters/label_presenter_spec.rb b/spec/presenters/label_presenter_spec.rb
index fae8188670f..d566da7c872 100644
--- a/spec/presenters/label_presenter_spec.rb
+++ b/spec/presenters/label_presenter_spec.rb
@@ -62,4 +62,32 @@ describe LabelPresenter do
       expect(label.can_subscribe_to_label_in_different_levels?).to be_falsey
     end
   end
+
+  describe '#project_label?' do
+    context 'with group label' do
+      subject { group_label.project_label? }
+
+      it { is_expected.to be_falsey }
+    end
+
+    context 'with project label' do
+      subject { label.project_label? }
+
+      it { is_expected.to be_truthy }
+    end
+  end
+
+  describe '#subject_name' do
+    context 'with group label' do
+      subject { group_label.subject_name }
+
+      it { is_expected.to eq(group_label.group.name) }
+    end
+
+    context 'with project label' do
+      subject { label.subject_name }
+
+      it { is_expected.to eq(label.project.name) }
+    end
+  end
 end