Fix LabelsHelper#link_to_label to use the subject argument

This commit is contained in:
Douglas Barbosa Alexandre 2016-09-26 19:58:36 -03:00
parent ae88126d13
commit b654229dcd
5 changed files with 34 additions and 29 deletions

View file

@ -31,7 +31,13 @@ module LabelsHelper
# #
# Returns a String # Returns a String
def link_to_label(label, subject: nil, type: :issue, tooltip: true, css_class: nil, &block) def link_to_label(label, subject: nil, type: :issue, tooltip: true, css_class: nil, &block)
link = label_filter_path(label, type: type) subject ||=
case label
when GroupLabel then label.group
when ProjectLabel then label.project
end
link = label_filter_path(subject, label, type: type)
if block_given? if block_given?
link_to link, class: css_class, &block link_to link, class: css_class, &block
@ -40,16 +46,16 @@ module LabelsHelper
end end
end end
def label_filter_path(label, type: issue) def label_filter_path(subject, label, type: issue)
case label case subject
when GroupLabel when Group
send("#{type.to_s.pluralize}_group_path", send("#{type.to_s.pluralize}_group_path",
label.group, subject,
label_name: [label.name]) label_name: [label.name])
else when Project
send("namespace_project_#{type.to_s.pluralize}_path", send("namespace_project_#{type.to_s.pluralize}_path",
label.project.namespace, subject.namespace,
label.project, subject,
label_name: [label.name]) label_name: [label.name])
end end
end end

View file

@ -12,10 +12,10 @@
.dropdown-menu.dropdown-menu-align-right .dropdown-menu.dropdown-menu-align-right
%ul %ul
%li %li
= link_to_label(label, type: :merge_request) do = link_to_label(label, subject: @project, type: :merge_request) do
= pluralize open_merge_requests_count, 'merge request' = pluralize open_merge_requests_count, 'merge request'
%li %li
= link_to_label(label) do = link_to_label(label, subject: @project) do
= pluralize open_issues_count, 'open issue' = pluralize open_issues_count, 'open issue'
- if current_user - if current_user
%li.label-subscription{ data: { url: toggle_subscription_label_path(label) } } %li.label-subscription{ data: { url: toggle_subscription_label_path(label) } }
@ -28,9 +28,9 @@
= link_to 'Delete', destroy_label_path(label), title: 'Delete', method: :delete, remote: true, data: {confirm: 'Remove this label? Are you sure?'} = link_to 'Delete', destroy_label_path(label), title: 'Delete', method: :delete, remote: true, data: {confirm: 'Remove this label? Are you sure?'}
.pull-right.hidden-xs.hidden-sm.hidden-md .pull-right.hidden-xs.hidden-sm.hidden-md
= link_to_label(label, type: :merge_request, css_class: 'btn btn-transparent btn-action') do = link_to_label(label, subject: @project, type: :merge_request, css_class: 'btn btn-transparent btn-action') do
= pluralize open_merge_requests_count, 'merge request' = pluralize open_merge_requests_count, 'merge request'
= link_to_label(label, css_class: 'btn btn-transparent btn-action') do = link_to_label(label, subject: @project, css_class: 'btn btn-transparent btn-action') do
= pluralize open_issues_count, 'open issue' = pluralize open_issues_count, 'open issue'
- if current_user - if current_user

View file

@ -9,7 +9,7 @@
%button.remove-priority.btn.has-tooltip{ title: 'Remove priority', :'data-placement' => 'top' } %button.remove-priority.btn.has-tooltip{ title: 'Remove priority', :'data-placement' => 'top' }
= icon('star') = icon('star')
%span.label-name %span.label-name
= link_to_label(label, tooltip: false) = link_to_label(label, subject: @project, tooltip: false)
- if can?(current_user, :admin_label, @project) - if can?(current_user, :admin_label, @project)
= label_type_icon(label, hidden: label.priority.blank?) = label_type_icon(label, hidden: label.priority.blank?)
- if label.description - if label.description

View file

@ -1,5 +1,5 @@
- labels.each do |label| - labels.each do |label|
%span.label-row.btn-group{ role: "group", aria: { label: label.name }, style: "color: #{text_color_for_bg(label.color)}" } %span.label-row.btn-group{ role: "group", aria: { label: label.name }, style: "color: #{text_color_for_bg(label.color)}" }
= link_to_label(label, css_class: 'btn btn-transparent') = link_to_label(label, subject: @project, css_class: 'btn btn-transparent')
%button.btn.btn-transparent.label-remove.js-label-filter-remove{ type: "button", style: "background-color: #{label.color};", data: { label: label.title } } %button.btn.btn-transparent.label-remove.js-label-filter-remove{ type: "button", style: "background-color: #{label.color};", data: { label: label.title } }
= icon("times") = icon("times")

View file

@ -5,27 +5,26 @@ describe LabelsHelper do
let(:project) { create(:empty_project) } let(:project) { create(:empty_project) }
let(:label) { create(:label, project: project) } let(:label) { create(:label, project: project) }
context 'with @project set' do context 'without subject' do
before do
@project = project
end
it 'uses the instance variable' do
expect(link_to_label(label)).to match %r{<a href="/#{@project.to_reference}/issues\?label_name%5B%5D=#{label.name}"><span class="[\w\s\-]*has-tooltip".*</span></a>}
end
end
context 'without @project set' do
it "uses the label's project" do it "uses the label's project" do
expect(link_to_label(label)).to match %r{<a href="/#{label.project.to_reference}/issues\?label_name%5B%5D=#{label.name}">.*</a>} expect(link_to_label(label)).to match %r{<a href="/#{label.project.to_reference}/issues\?label_name%5B%5D=#{label.name}">.*</a>}
end end
end end
context 'with a project argument' do context 'with a project as subject' do
let(:another_project) { double('project', namespace: 'foo3', to_param: 'bar3') } let(:namespace) { build(:namespace, name: 'foo3') }
let(:another_project) { build(:empty_project, namespace: namespace, name: 'bar3') }
it 'links to merge requests page' do it 'links to project issues page' do
expect(link_to_label(label, project: another_project)).to match %r{<a href="/foo3/bar3/issues\?label_name%5B%5D=#{label.name}">.*</a>} expect(link_to_label(label, subject: another_project)).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') }
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>}
end end
end end