summaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authorJan Provaznik <jprovaznik@gitlab.com>2019-05-23 10:02:00 +0200
committerJan Provaznik <jprovaznik@gitlab.com>2019-05-23 13:36:09 +0200
commitf9a55f93e761f7a9f00aa43327e760af839cc5f8 (patch)
tree6a0e54b5905c2b014da29a63a198504d10c88e12
parente1407df4a1943efefd34e152db3ef1a8281a305d (diff)
downloadgitlab-ce-jp-label-fix.tar.gz
Fix display of promote to group labeljp-label-fix
Since label presenter is used in label index view, label class check doesn't work as expected because the class is now LabelPresenter. Also `label.subject` doesn't work as expected now because Label's model `subject` method is shadowed by Gitlab's presenter's method which uses `subject` for referencing the original object. Instead we use a presenter's method for both checks now. `label_deletion_confirm_text` is not used anywhere so it's removed
-rw-r--r--app/helpers/labels_helper.rb11
-rw-r--r--app/presenters/label_presenter.rb8
-rw-r--r--app/views/shared/_delete_label_modal.html.haml2
-rw-r--r--app/views/shared/_label.html.haml2
-rw-r--r--changelogs/unreleased/jp-label-fix.yml5
-rw-r--r--locale/gitlab.pot6
-rw-r--r--spec/features/projects/labels/user_promotes_label_spec.rb34
-rw-r--r--spec/features/projects/labels/user_removes_labels_spec.rb5
-rw-r--r--spec/helpers/labels_helper_spec.rb19
-rw-r--r--spec/presenters/label_presenter_spec.rb28
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 6209530deab..cbaa8a68303 100644
--- a/locale/gitlab.pot
+++ b/locale/gitlab.pot
@@ -8034,12 +8034,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..f50e635b181
--- /dev/null
+++ b/spec/features/projects/labels/user_promotes_label_spec.rb
@@ -0,0 +1,34 @@
+# frozen_string_literal: true
+
+require 'rails_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..aca3fb09726 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