Commit d1718afb authored by Jan Provaznik's avatar Jan Provaznik Committed by Douglas Barbosa Alexandre

Move scoped_label into label presenter

When rendering a label we want to check 'scoped_label' feature
availability on a project/group where label is being used. For
this reason a label presenter is used in UI and information about
context project/group is passed to this presenter.
parent 24c9c458
......@@ -31,7 +31,7 @@ module MilestoneActions
format.html { redirect_to milestone_redirect_path }
format.json do
render json: tabs_json("shared/milestones/_labels_tab", {
labels: @milestone.labels # rubocop:disable Gitlab/ModuleWithInstanceVariables
labels: @milestone.labels.map { |label| label.present(issuable_subject: @milestone.parent) } # rubocop:disable Gitlab/ModuleWithInstanceVariables
})
end
end
......
......@@ -13,9 +13,7 @@ module LabelsHelper
# Link to a Label
#
# label - Label object to link to
# subject - Project/Group object which will be used as the context for the
# label's link. If omitted, defaults to the label's own group/project.
# label - LabelPresenter object to link to
# type - The type of item the link will point to (:issue or
# :merge_request). If omitted, defaults to :issue.
# block - An optional block that will be passed to `link_to`, forming the
......@@ -40,8 +38,8 @@ module LabelsHelper
# link_to_label(label) { "My Custom Label Text" }
#
# Returns a String
def link_to_label(label, subject: nil, type: :issue, tooltip: true, css_class: nil, &block)
link = label_filter_path(subject || label.subject, label, type: type)
def link_to_label(label, type: :issue, tooltip: true, css_class: nil, &block)
link = label.filter_path(type: type)
if block_given?
link_to link, class: css_class, &block
......@@ -50,34 +48,6 @@ module LabelsHelper
end
end
def label_filter_path(subject, label, type: :issue)
case subject
when Group
send("#{type.to_s.pluralize}_group_path", # rubocop:disable GitlabSecurity/PublicSend
subject,
label_name: [label.name])
when Project
send("namespace_project_#{type.to_s.pluralize}_path", # rubocop:disable GitlabSecurity/PublicSend
subject.namespace,
subject,
label_name: [label.name])
end
end
def edit_label_path(label)
case label
when GroupLabel then edit_group_label_path(label.group, label)
when ProjectLabel then edit_project_label_path(label.project, label)
end
end
def destroy_label_path(label)
case label
when GroupLabel then group_label_path(label.group, label)
when ProjectLabel then project_label_path(label.project, label)
end
end
def render_label(label, tooltip: true, link: nil, css: nil)
# if scoped label is used then EE wraps label tag with scoped label
# doc link
......@@ -168,10 +138,6 @@ module LabelsHelper
end
end
def can_subscribe_to_label_in_different_levels?(label)
defined?(@project) && label.is_a?(GroupLabel)
end
def label_subscription_status(label, project)
return 'group-level' if label.subscribed?(current_user)
return 'project-level' if label.subscribed?(current_user, project)
......@@ -241,8 +207,8 @@ module LabelsHelper
"#{action} at #{level} level"
end
def labels_sorted_by_title(labels)
labels.sort_by(&:title)
def presented_labels_sorted_by_title(labels, subject)
labels.sort_by(&:title).map { |label| label.present(issuable_subject: subject) }
end
def label_dropdown_data(project, opts = {})
......
# frozen_string_literal: true
class GlobalLabel
include Presentable
attr_accessor :title, :labels
alias_attribute :name, :title
......@@ -23,4 +25,8 @@ class GlobalLabel
@labels = labels
@first_label = labels.find { |lbl| lbl.description.present? } || labels.first
end
def present(attributes)
super(attributes.merge(presenter_class: ::LabelPresenter))
end
end
......@@ -9,7 +9,9 @@ class GlobalMilestone
attr_reader :milestone
alias_attribute :name, :title
delegate :title, :state, :due_date, :start_date, :participants, :project, :group, :expires_at, :closed?, :iid, :group_milestone?, :safe_title, :milestoneish_id, to: :milestone
delegate :title, :state, :due_date, :start_date, :participants, :project,
:group, :expires_at, :closed?, :iid, :group_milestone?, :safe_title,
:milestoneish_id, :parent, to: :milestone
def to_hash
{
......
......@@ -8,6 +8,7 @@ class Label < ApplicationRecord
include OptionallySearch
include Sortable
include FromUnion
include Presentable
cache_markdown_field :description, pipeline: :single_line
......@@ -233,6 +234,10 @@ class Label < ApplicationRecord
attributes
end
def present(attributes)
super(attributes.merge(presenter_class: ::LabelPresenter))
end
private
def issues_count(user, params = {})
......
# frozen_string_literal: true
class LabelPresenter < Gitlab::View::Presenter::Delegated
presents :label
def edit_path
case label
when GroupLabel then edit_group_label_path(label.group, label)
when ProjectLabel then edit_project_label_path(label.project, label)
end
end
def destroy_path
case label
when GroupLabel then group_label_path(label.group, label)
when ProjectLabel then project_label_path(label.project, label)
end
end
def filter_path(type: :issue)
case context_subject
when Group
send("#{type.to_s.pluralize}_group_path", # rubocop:disable GitlabSecurity/PublicSend
context_subject,
label_name: [label.name])
when Project
send("namespace_project_#{type.to_s.pluralize}_path", # rubocop:disable GitlabSecurity/PublicSend
context_subject.namespace,
context_subject,
label_name: [label.name])
end
end
def can_subscribe_to_label_in_different_levels?
issuable_subject.is_a?(Project) && label.is_a?(GroupLabel)
end
private
def context_subject
issuable_subject || label.try(:subject)
end
end
LabelPresenter.prepend(EE::LabelPresenter)
%li.label-list-item{ id: dom_id(label) }
= render "shared/label_row", label: label
= render "shared/label_row", label: label.present(issuable_subject: nil)
.label-actions-list
= link_to edit_admin_label_path(label), class: 'btn btn-transparent label-action has-tooltip', title: _('Edit'), data: { placement: 'bottom' }, aria_label: _('Edit') do
= sprite_icon('pencil')
......
......@@ -17,7 +17,7 @@
.other-labels
%h5= _('Labels')
%ul.content-list.manage-labels-list.js-other-labels
= render partial: 'shared/label', subject: @group, collection: @labels, as: :label, locals: { use_label_priority: false }
= render partial: 'shared/label', collection: @labels, as: :label, locals: { use_label_priority: false, subject: @group }
= paginate @labels, theme: 'gitlab'
- elsif search.present?
.nothing-here-block
......
......@@ -36,8 +36,8 @@
= issue.due_date.to_s(:medium)
- if issue.labels.any?
&nbsp;
- labels_sorted_by_title(issue.labels).each do |label|
= link_to_label(label, subject: issue.project, css_class: 'label-link')
- presented_labels_sorted_by_title(issue.labels, issue.project).each do |label|
= link_to_label(label, css_class: 'label-link')
- if issue.weight
%span.issuable-weight.d-none.d-sm-inline-block.has-tooltip{ data: { container: 'body' }, title: _('Weight') }
......
......@@ -25,7 +25,7 @@
#js-priority-labels-empty-state.priority-labels-empty-state{ class: "#{'hidden' unless @prioritized_labels.empty? && search.blank?}" }
= render 'shared/empty_states/priority_labels'
- if @prioritized_labels.present?
= render partial: 'shared/label', subject: @project, collection: @prioritized_labels, as: :label, locals: { force_priority: true }
= render partial: 'shared/label', collection: @prioritized_labels, as: :label, locals: { force_priority: true, subject: @project }
- elsif search.present?
.nothing-here-block
= _('No prioritised labels with such name or description')
......@@ -34,7 +34,7 @@
.other-labels
%h5{ class: ('hide' if hide) }= _('Other Labels')
.content-list.manage-labels-list.js-other-labels
= render partial: 'shared/label', subject: @project, collection: @labels, as: :label
= render partial: 'shared/label', collection: @labels, as: :label, locals: { subject: @project }
= paginate @labels, theme: 'gitlab'
- elsif search.present?
.other-labels
......
......@@ -34,8 +34,8 @@
= merge_request.target_branch
- if merge_request.labels.any?
&nbsp;
- labels_sorted_by_title(merge_request.labels).each do |label|
= link_to_label(label, subject: merge_request.project, type: :merge_request, css_class: 'label-link')
- presented_labels_sorted_by_title(merge_request.labels, merge_request.project).each do |label|
= link_to_label(label, type: :merge_request, css_class: 'label-link')
.issuable-meta
%ul.controls
......
......@@ -9,13 +9,13 @@
.modal-body
%p
%strong= label.name
%span will be permanently deleted from #{label.is_a?(ProjectLabel)? label.project.name : label.group.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
= link_to 'Delete label',
destroy_label_path(label),
label.destroy_path,
title: 'Delete',
method: :delete,
class: 'btn btn-remove'
- label = label.present(issuable_subject: local_assigns[:subject])
- label_css_id = dom_id(label)
- status = label_subscription_status(label, @project).inquiry if current_user
- subject = local_assigns[:subject]
- use_label_priority = local_assigns.fetch(:use_label_priority, false)
- force_priority = local_assigns.fetch(:force_priority, use_label_priority ? label.priority.present? : false)
- toggle_subscription_path = toggle_subscription_label_path(label, @project) if current_user
- tooltip_title = label_status_tooltip(label, status) if status
%li.label-list-item{ id: label_css_id, data: { id: label.id } }
= render "shared/label_row", label: label, subject: subject, force_priority: force_priority
= render "shared/label_row", label: label, force_priority: force_priority
%ul.label-actions-list
- if @project
%li.inline
......@@ -21,7 +21,7 @@
= sprite_icon('star')
- if can?(current_user, :admin_label, label)
%li.inline
= link_to edit_label_path(label), class: 'btn btn-transparent label-action edit has-tooltip', title: _('Edit'), data: { placement: 'bottom' }, aria_label: _('Edit') do
= link_to label.edit_path, class: 'btn btn-transparent label-action edit has-tooltip', title: _('Edit'), data: { placement: 'bottom' }, aria_label: _('Edit') do
= sprite_icon('pencil')
- if can?(current_user, :admin_label, label)
%li.inline
......@@ -48,7 +48,7 @@
%button.text-danger.remove-row{ type: 'button' }= _('Delete')
- if current_user
%li.inline.label-subscription
- if can_subscribe_to_label_in_different_levels?(label)
- if label.can_subscribe_to_label_in_different_levels?
%button.js-unsubscribe-button.label-subscribe-button.btn.btn-default{ class: ('hidden' if status.unsubscribed?), data: { url: toggle_subscription_path, toggle: 'tooltip' }, title: tooltip_title }
%span= _('Unsubscribe')
.dropdown.dropdown-group-label{ class: ('hidden' unless status.unsubscribed?) }
......
- subject = local_assigns[:subject]
- force_priority = local_assigns.fetch(:force_priority, false)
- subject_or_group_defined = defined?(@project) || defined?(@group)
- show_label_issues_link = subject_or_group_defined && show_label_issuables_link?(label, :issues, project: @project)
......@@ -14,11 +13,11 @@
%ul.label-links
- if show_label_issues_link
%li.label-link-item.inline
= link_to_label(label, subject: subject) { 'Issues' }
= link_to_label(label) { 'Issues' }
- if show_label_merge_requests_link
&middot;
%li.label-link-item.inline
= link_to_label(label, subject: subject, type: :merge_request) { _('Merge requests') }
= link_to_label(label, type: :merge_request) { _('Merge requests') }
- if force_priority
&middot;
%li.label-link-item.priority-badge.js-priority-badge.inline.prepend-left-10
......
......@@ -106,7 +106,7 @@
.value.issuable-show-labels.dont-hide.hide-collapsed.qa-labels-block{ class: ("has-labels" if selected_labels.any?) }
- if selected_labels.any?
- selected_labels.each do |label_hash|
= render_label(label_from_hash(label_hash), link: sidebar_label_filter_path(issuable_sidebar[:project_issuables_path], label_hash[:title]))
= render_label(label_from_hash(label_hash).present(issuable_subject: nil), link: sidebar_label_filter_path(issuable_sidebar[:project_issuables_path], label_hash[:title]))
- else
%span.no-value
= _('None')
......
......@@ -21,7 +21,7 @@
%span.issuable-number= issuable.to_reference
- labels.each do |label|
= render_label(label, link: polymorphic_path(issuable_type_args, { milestone_title: @milestone.title, label_name: label.title, state: 'all' }))
= render_label(label.present(issuable_subject: project), link: polymorphic_path(issuable_type_args, { milestone_title: @milestone.title, label_name: label.title, state: 'all' }))
%span.assignee-icon
- assignees.each do |assignee|
......
......@@ -7,7 +7,7 @@ module EE
SCOPED_LABEL_PATTERN = /^.*::/.freeze
def scoped_label?
SCOPED_LABEL_PATTERN.match?(name) && respond_to?(:subject) && subject.feature_available?(:scoped_labels)
SCOPED_LABEL_PATTERN.match?(name)
end
def scoped_label_key
......
# frozen_string_literal: true
module EE
module LabelPresenter
def scoped_label?
label.scoped_label? && context_subject && context_subject.feature_available?(:scoped_labels)
end
end
end
......@@ -15,4 +15,4 @@
- if epic.labels.any?
&nbsp;
- epic.labels.each do |label|
= render_label(label, tooltip: true, link: group_epics_path(@group, label_name:[label.name]), css: 'label-link')
= render_label(label.present(issuable_subject: @group), tooltip: true, link: group_epics_path(@group, label_name:[label.name]), css: 'label-link')
......@@ -9,7 +9,12 @@ module EE
override :wrap_link
def wrap_link(link, label)
content = super
content = ::EE::LabelsHelper.scoped_label_wrapper(content, label) if label.scoped_label?
parent = project || group
if label.scoped_label? && parent && parent.feature_available?(:scoped_labels)
presenter = label.present(issuable_parent: parent)
content = ::EE::LabelsHelper.scoped_label_wrapper(content, presenter)
end
content
end
......
......@@ -3,9 +3,9 @@
require 'spec_helper'
describe LabelsHelper do
set(:project) { create(:project) }
set(:label) { create(:label, project: project) }
set(:scoped_label) { create(:label, name: 'key::value', project: project) }
let(:project) { create(:project) }
let(:label) { build_stubbed(:label, project: project).present(issuable_subject: nil) }
let(:scoped_label) { build_stubbed(:label, name: 'key::value', project: project).present(issuable_subject: nil) }
describe '#render_label' do
context 'with scoped labels enabled' do
......
......@@ -16,22 +16,6 @@ describe Label do
it 'returns true for scoped labels' do
expect(build(:label, title: 'key::some name').scoped_label?).to be_truthy
end
it 'returns false for scoped labels without subject' do
label = described_class.new(title: 'key::some name')
expect(label.scoped_label?).to be_falsey
end
end
context 'with scoped_labels not available' do
before do
stub_licensed_features(scoped_labels: false)
end
it 'returns false for scoped labels' do
expect(build(:label, title: 'key::some name').scoped_label?).to be_falsey
end
end
end
......
# frozen_string_literal: true
require 'spec_helper'
describe LabelPresenter do
let(:project) { create(:project) }
describe '#scoped_label?' do
subject { label.scoped_label? }
context 'with scoped_labels enabled' do
before do
stub_licensed_features(scoped_labels: true)
end
context 'with project label with context subject set' do
let(:label) { build_stubbed(:label, title: 'key::val', project: project).present(issuable_subject: project) }
it { is_expected.to be_truthy }
end
context 'with project label without context subject' do
let(:label) { build_stubbed(:label, title: 'key::val', project: project).present(issuable_subject: nil) }
it { is_expected.to be_truthy }
end
end
context 'with scoped_labels disabled' do
before do
stub_licensed_features(scoped_labels: false)
end
context 'with project label with context subject set' do
let(:label) { create(:label, title: 'key::val', project: project).present(issuable_subject: project) }
it { is_expected.to be_falsey }
end
end
end
end
......@@ -82,16 +82,18 @@ module Banzai
def object_link_text(object, matches)
label_suffix = ''
parent = project || group
if project || full_path_ref?(matches)
project_path = full_project_path(matches[:namespace], matches[:project])
parent_from_ref = from_ref_cached(project_path)
reference = parent_from_ref.to_human_reference(project || group)
reference = parent_from_ref.to_human_reference(parent)
label_suffix = " <i>in #{reference}</i>" if reference.present?
end
LabelsHelper.render_colored_label(object, label_suffix: label_suffix, title: tooltip_title(object))
presenter = object.present(issuable_subject: parent)
LabelsHelper.render_colored_label(presenter, label_suffix: label_suffix, title: tooltip_title(presenter))
end
def tooltip_title(label)
......
......@@ -67,27 +67,29 @@ describe LabelsHelper do
describe 'link_to_label' do
let(:project) { create(:project) }
let(:label) { create(:label, project: project) }
let(:subject) { nil }
let(:label_presenter) { label.present(issuable_subject: subject) }
context 'without subject' do
it "uses the label's project" do
expect(link_to_label(label)).to match %r{<a href="/#{label.project.full_path}/issues\?label_name%5B%5D=#{label.name}">.*</a>}
expect(link_to_label(label_presenter)).to match %r{<a href="/#{label.project.full_path}/issues\?label_name%5B%5D=#{label.name}">.*</a>}
end
end
context 'with a project as subject' do
let(:namespace) { build(:namespace, name: 'foo3') }
let(:another_project) { build(:project, namespace: namespace, name: 'bar3') }
let(:subject) { build(:project, namespace: namespace, name: 'bar3') }
it 'links to project issues page' do
expect(link_to_label(label, subject: another_project)).to match %r{<a href="/foo3/bar3/issues\?label_name%5B%5D=#{label.name}">.*</a>}
expect(link_to_label(label_presenter)).to match %r{<a href="/foo3/bar3/issues\?label_name%5B%5D=#{label.name}">.*</a>}
end
end
context 'with a group as subject' do
let(:group) { build(:group, name: 'bar') }
let(:subject) { build(:group, name: 'bar') }
it 'links to group issues page' do
expect(link_to_label(label, subject: group)).to match %r{<a href="/groups/bar/-/issues\?label_name%5B%5D=#{label.name}">.*</a>}
expect(link_to_label(label_presenter)).to match %r{<a href="/groups/bar/-/issues\?label_name%5B%5D=#{label.name}">.*</a>}
end
end
......@@ -95,7 +97,7 @@ describe LabelsHelper do
['issue', :issue, 'merge_request', :merge_request].each do |type|
context "set to #{type}" do
it 'links to correct page' do
expect(link_to_label(label, type: type)).to match %r{<a href="/#{label.project.full_path}/#{type.to_s.pluralize}\?label_name%5B%5D=#{label.name}">.*</a>}
expect(link_to_label(label_presenter, type: type)).to match %r{<a href="/#{label.project.full_path}/#{type.to_s.pluralize}\?label_name%5B%5D=#{label.name}">.*</a>}
end
end
end
......@@ -104,14 +106,14 @@ describe LabelsHelper do
context 'with a tooltip argument' do
context 'set to false' do
it 'does not include the has-tooltip class' do
expect(link_to_label(label, tooltip: false)).not_to match /has-tooltip/
expect(link_to_label(label_presenter, tooltip: false)).not_to match /has-tooltip/
end
end
end
context 'with block' do
it 'passes the block to link_to' do
link = link_to_label(label) { 'Foo' }
link = link_to_label(label_presenter) { 'Foo' }
expect(link).to match('Foo')
end
end
......@@ -119,8 +121,8 @@ describe LabelsHelper do
context 'without block' do
it 'uses render_colored_label as the link content' do
expect(self).to receive(:render_colored_label)
.with(label, tooltip: true).and_return('Foo')
expect(link_to_label(label)).to match('Foo')
.with(label_presenter, tooltip: true).and_return('Foo')
expect(link_to_label(label_presenter)).to match('Foo')
end
end
end
......@@ -237,16 +239,24 @@ describe LabelsHelper do
end
end
describe 'labels_sorted_by_title' do
describe 'presented_labels_sorted_by_title' do
let(:labels) do
[build(:label, title: 'a'),
build(:label, title: 'B'),
build(:label, title: 'c'),
build(:label, title: 'D')]
end
it 'sorts labels alphabetically' do
label1 = double(:label, title: 'a')
label2 = double(:label, title: 'B')
label3 = double(:label, title: 'c')
label4 = double(:label, title: 'D')
labels = [label1, label2, label3, label4]
expect(labels_sorted_by_title(labels))
.to match_array([label2, label4, label1, label3])
sorted_ids = presented_labels_sorted_by_title(labels, nil).map(&:id)
expect(sorted_ids)
.to match_array([labels[1].id, labels[3].id, labels[0].id, labels[2].id])
end
it 'returns an array of label presenters' do
expect(presented_labels_sorted_by_title(labels, nil))
.to all(be_a(LabelPresenter))
end
end
......
# frozen_string_literal: true
require 'spec_helper'
describe LabelPresenter do
include Gitlab::Routing.url_helpers
set(:group) { create(:group) }
set(:project) { create(:project, group: group) }
let(:label) { build_stubbed(:label, project: project).present(issuable_subject: project) }
let(:group_label) { build_stubbed(:group_label, group: group).present(issuable_subject: project) }
describe '#edit_path' do
context 'with group label' do
subject { group_label.edit_path }
it { is_expected.to eq(edit_group_label_path(group, group_label)) }
end
context 'with project label' do
subject { label.edit_path }
it { is_expected.to eq(edit_project_label_path(project, label)) }
end
end
describe '#destroy_path' do
context 'with group label' do
subject { group_label.destroy_path }
it { is_expected.to eq(group_label_path(group, group_label)) }
end
context 'with project label' do
subject { label.destroy_path }
it { is_expected.to eq(project_label_path(project, label)) }
end
end
describe '#filter_path' do
context 'with group as context subject' do
let(:label_in_group) { build_stubbed(:label, project: project).present(issuable_subject: group) }
subject { label_in_group.filter_path }
it { is_expected.to eq(issues_group_path(group, label_name: [label_in_group.title])) }
end
context 'with project as context subject' do
subject { label.filter_path }
it { is_expected.to eq(namespace_project_issues_path(group, project, label_name: [label.title])) }
end
end
describe '#can_subscribe_to_label_in_different_levels?' do
it 'returns true for group labels in project context' do
expect(group_label.can_subscribe_to_label_in_different_levels?).to be_truthy
end
it 'returns false for project labels in project context' do
expect(label.can_subscribe_to_label_in_different_levels?).to be_falsey
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