From a44493320ee2a7047c98090f306f5fc1ab655630 Mon Sep 17 00:00:00 2001 From: ulferts Date: Wed, 18 Aug 2021 10:10:28 +0200 Subject: [PATCH] don`t include a non hidden closed reference in daily summary --- app/helpers/work_packages_helper.rb | 45 +++++----- .../digest_mailer/work_packages.html.erb | 2 +- .../spec/features/custom_fields_spec.rb | 6 +- .../work_packages/table/context_menu_spec.rb | 2 +- spec/helpers/work_packages_helper_spec.rb | 85 ++++++++++--------- 5 files changed, 72 insertions(+), 68 deletions(-) diff --git a/app/helpers/work_packages_helper.rb b/app/helpers/work_packages_helper.rb index 875e0d359b..04df386440 100644 --- a/app/helpers/work_packages_helper.rb +++ b/app/helpers/work_packages_helper.rb @@ -35,17 +35,16 @@ module WorkPackagesHelper # Displays a link to +work_package+ with its subject. # Examples: # - # link_to_work_package(package) # => Defect #6: This is the subject - # link_to_work_package(package, all_link: true) # => Defect #6: This is the subject (everything within the link) - # link_to_work_package(package, truncate: 9) # => Defect #6: This i... - # link_to_work_package(package, subject: false) # => Defect #6 - # link_to_work_package(package, type: false) # => #6: This is the subject - # link_to_work_package(package, project: true) # => Foo - Defect #6 - # link_to_work_package(package, id_only: true) # => #6 - # link_to_work_package(package, subject_only: true) # => This is the subject (as link) - # link_to_work_package(package, status: true) # => #6 New (if #id => true) + # link_to_work_package(package) # => Defect #6: This is the subject + # link_to_work_package(package, all_link: true) # => Defect #6: This is the subject (everything within the link) + # link_to_work_package(package, truncate: 9) # => Defect #6: This i... + # link_to_work_package(package, subject: false) # => Defect #6 + # link_to_work_package(package, type: false) # => #6: This is the subject + # link_to_work_package(package, project: true) # => Foo - Defect #6 + # link_to_work_package(package, id_only: true) # => #6 + # link_to_work_package(package, subject_only: true) # => This is the subject (as link) + # link_to_work_package(package, status: true) # => #6 New (if #id => true) def link_to_work_package(package, options = {}) - only_path = options.fetch(:only_path) { true } if options[:subject_only] options.merge!(type: false, subject: true, @@ -67,7 +66,7 @@ module WorkPackagesHelper link: [], suffix: [], title: [], - css_class: ['issue'] } + css_class: link_to_work_package_css_classes(package, options) } # Prefix part @@ -85,12 +84,10 @@ module WorkPackagesHelper # Hidden link part - if package.closed? + if package.closed? && !options[:no_hidden] parts[:hidden_link] << content_tag(:span, I18n.t(:label_closed_work_packages), class: 'hidden-for-sighted') - - parts[:css_class] << 'closed' end # Suffix part @@ -117,14 +114,13 @@ module WorkPackagesHelper prefix = parts[:prefix].join(' ') suffix = parts[:suffix].join(' ') link = parts[:link].join(' ').strip - hidden_link = parts[:hidden_link].join('') + hidden_link = parts[:hidden_link].join title = parts[:title].join(' ') css_class = parts[:css_class].join(' ') - css_class << options[:class].to_s # Determine path or url work_package_link = - if only_path + if options.fetch(:only_path, true) work_package_path(package) else work_package_url(package) @@ -133,14 +129,14 @@ module WorkPackagesHelper if options[:all_link] link_text = [prefix, link].reject(&:empty?).join(' - ') link_text = [link_text, suffix].reject(&:empty?).join(': ') - link_text = [hidden_link, link_text].reject(&:empty?).join('') + link_text = [hidden_link, link_text].reject(&:empty?).join link_to(link_text.html_safe, work_package_link, title: title, class: css_class) else - link_text = [hidden_link, link].reject(&:empty?).join('') + link_text = [hidden_link, link].reject(&:empty?).join html_link = link_to(link_text.html_safe, work_package_link, @@ -179,8 +175,7 @@ module WorkPackagesHelper # Returns a string of css classes that apply to the issue def work_package_css_classes(work_package) - # TODO: remove issue once css is cleaned of it - s = 'issue work_package preview-trigger'.html_safe + s = 'work_package preview-trigger'.html_safe s << " status-#{work_package.status.position}" if work_package.status s << " priority-#{work_package.priority.position}" if work_package.priority s << ' closed' if work_package.closed? @@ -258,4 +253,12 @@ module WorkPackagesHelper [responsible, assignee].compact.join('
').html_safe end + + def link_to_work_package_css_classes(package, options) + classes = ['work_package'] + classes << 'closed' if package.closed? + classes << options[:class].to_s + + classes + end end diff --git a/app/views/digest_mailer/work_packages.html.erb b/app/views/digest_mailer/work_packages.html.erb index 8653f6a30c..4eb1cd7b8d 100644 --- a/app/views/digest_mailer/work_packages.html.erb +++ b/app/views/digest_mailer/work_packages.html.erb @@ -8,7 +8,7 @@ <% notifications_by_work_package.each do |work_package, notifications| %>
-

<%= link_to_work_package work_package, status: true, only_path: false %>

+

<%= link_to_work_package work_package, status: true, only_path: false, no_hidden: true %>

<% notifications.sort_by(&:created_at).each do |notification| %> diff --git a/modules/reporting/spec/features/custom_fields_spec.rb b/modules/reporting/spec/features/custom_fields_spec.rb index 1f1ceed431..daf4a39977 100644 --- a/modules/reporting/spec/features/custom_fields_spec.rb +++ b/modules/reporting/spec/features/custom_fields_spec.rb @@ -129,7 +129,7 @@ describe 'Custom fields reporting', type: :feature, js: true do # Expect row of work package within('#result-table') do - expect(page).to have_selector('a.issue', text: "#{work_package.type} ##{work_package.id}") + expect(page).to have_selector('a.work_package', text: "#{work_package.type} ##{work_package.id}") expect(page).to have_selector('th.inner', text: 'First option') expect(page).to have_no_selector('th.inner', text: 'Second option') @@ -190,7 +190,7 @@ describe 'Custom fields reporting', type: :feature, js: true do # Expect row of work package within('#result-table') do - expect(page).to have_selector('a.issue', text: "#{work_package.type} ##{work_package.id}") + expect(page).to have_selector('a.work_package', text: "#{work_package.type} ##{work_package.id}") expect(page).to have_selector('th.inner', text: '1') expect(page).to have_no_selector('th.inner', text: 'invalid!') end @@ -223,7 +223,7 @@ describe 'Custom fields reporting', type: :feature, js: true do # Expect row of work package within('#result-table') do - expect(page).to have_selector('a.issue', text: "#{work_package.type} ##{work_package.id}") + expect(page).to have_selector('a.work_package', text: "#{work_package.type} ##{work_package.id}") expect(page).to have_selector('th.inner', text: 'foo') expect(page).to have_no_selector('th.inner', text: 'None') diff --git a/spec/features/work_packages/table/context_menu_spec.rb b/spec/features/work_packages/table/context_menu_spec.rb index 15e4d283cb..64b538fb50 100644 --- a/spec/features/work_packages/table/context_menu_spec.rb +++ b/spec/features/work_packages/table/context_menu_spec.rb @@ -52,7 +52,7 @@ describe 'Work package table context menu', js: true do goto_context_menu list_view menu.choose('Change project') expect(page).to have_selector('h2', text: I18n.t(:button_move)) - expect(page).to have_selector('a.issue', text: "##{work_package.id}") + expect(page).to have_selector('a.work_package', text: "##{work_package.id}") # Open Copy goto_context_menu list_view diff --git a/spec/helpers/work_packages_helper_spec.rb b/spec/helpers/work_packages_helper_spec.rb index e495ab7801..c3f3447535 100644 --- a/spec/helpers/work_packages_helper_spec.rb +++ b/spec/helpers/work_packages_helper_spec.rb @@ -33,24 +33,23 @@ describe WorkPackagesHelper, type: :helper do let(:stub_project) { FactoryBot.build_stubbed(:project) } let(:stub_type) { FactoryBot.build_stubbed(:type) } let(:stub_user) { FactoryBot.build_stubbed(:user) } + let(:open_status) { FactoryBot.build_stubbed(:status, is_closed: false) } + let(:closed_status) { FactoryBot.build_stubbed(:status, is_closed: true) } describe '#link_to_work_package' do - let(:open_status) { FactoryBot.build_stubbed(:status, is_closed: false) } - let(:closed_status) { FactoryBot.build_stubbed(:status, is_closed: true) } - before do stub_work_package.status = open_status end describe 'without parameters' do - it 'should return a link to the work package with the id as the text' do + it 'returns a link to the work package with the id as the text' do link_text = Regexp.new("^##{stub_work_package.id}$") expect(helper.link_to_work_package(stub_work_package)).to have_selector( "a[href='#{work_package_path(stub_work_package)}']", text: link_text ) end - it 'should return a link to the work package with type and id as the text if type is set' do + it 'returns a link to the work package with type and id as the text if type is set' do stub_work_package.type = stub_type link_text = Regexp.new("^#{stub_type.name} ##{stub_work_package.id}$") @@ -59,20 +58,27 @@ describe WorkPackagesHelper, type: :helper do ) end - it 'should additionally return the subject' do + it 'additionally returns the subject' do text = Regexp.new("#{stub_work_package.subject}$") expect(helper.link_to_work_package(stub_work_package)).to have_text(text) end - it 'should prepend an invisible closed information if the work package is closed' do + it 'prepends an invisible closed information if the work package is closed' do stub_work_package.status = closed_status expect(helper.link_to_work_package(stub_work_package)).to have_selector('a span.hidden-for-sighted', text: 'closed') end + + it 'omits the invisible closed information if told so even though the work package is closed' do + stub_work_package.status = closed_status + + expect(helper.link_to_work_package(stub_work_package, no_hidden: true)) + .not_to have_selector('a span.hidden-for-sighted', text: 'closed') + end end describe 'with the all_link option provided' do - it 'should return a link to the work package with the type, id, and subject as the text' do + it 'returns a link to the work package with the type, id, and subject as the text' do stub_work_package.type = stub_type link_text = Regexp.new("^#{stub_type} ##{stub_work_package.id}: #{stub_work_package.subject}$") @@ -84,14 +90,14 @@ describe WorkPackagesHelper, type: :helper do end describe 'when truncating' do - it 'should truncate the subject if the subject is longer than the specified amount' do + it 'truncates the subject if the subject is longer than the specified amount' do stub_work_package.subject = '12345678' text = Regexp.new('1234...$') expect(helper.link_to_work_package(stub_work_package, truncate: 7)).to have_text(text) end - it 'should not truncate the subject if the subject is shorter than the specified amount' do + it 'does not truncate the subject if the subject is shorter than the specified amount' do stub_work_package.subject = '1234567' text = Regexp.new('1234567$') @@ -100,13 +106,13 @@ describe WorkPackagesHelper, type: :helper do end describe 'when omitting the subject' do - it 'should omit the subject' do + it 'omits the subject' do expect(helper.link_to_work_package(stub_work_package, subject: false)).not_to have_text(stub_work_package.subject) end end describe 'when omitting the type' do - it 'should omit the type' do + it 'omits the type' do stub_work_package.type = stub_type link_text = Regexp.new("^##{stub_work_package.id}$") @@ -123,17 +129,17 @@ describe WorkPackagesHelper, type: :helper do stub_work_package.project = stub_project end - it 'should prepend the project if parameter set to true' do + it 'prepends the project if parameter set to true' do expect(helper.link_to_work_package(stub_work_package, project: true)).to have_text(text) end - it 'should not have the project name if the parameter is missing/false' do + it 'does not include the project name if the parameter is missing/false' do expect(helper.link_to_work_package(stub_work_package)).not_to have_text(text) end end describe 'when only wanting the id' do - it 'should return a link with the id as text only even if the work package has a type' do + it 'returns a link with the id as text only even if the work package has a type' do stub_work_package.type = stub_type link_text = Regexp.new("^##{stub_work_package.id}$") @@ -142,13 +148,13 @@ describe WorkPackagesHelper, type: :helper do text: link_text) end - it 'should not have the subject as text' do + it 'does not have the subject as text' do expect(helper.link_to_work_package(stub_work_package, id_only: true)).not_to have_text(stub_work_package.subject) end end describe 'when only wanting the subject' do - it 'should return a link with the subject as text' do + it 'returns a link with the subject as text' do link_text = Regexp.new("^#{stub_work_package.subject}$") expect(helper.link_to_work_package(stub_work_package, subject_only: true)).to have_selector( @@ -158,7 +164,7 @@ describe WorkPackagesHelper, type: :helper do end describe 'with the status displayed' do - it 'should return a link with the status name contained in the text' do + it 'returns a link with the status name contained in the text' do stub_work_package.type = stub_type link_text = Regexp.new("^#{stub_type.name} ##{stub_work_package.id} #{stub_work_package.status}$") @@ -179,77 +185,72 @@ describe WorkPackagesHelper, type: :helper do priority: priority) end - it 'should always have the work_package class' do + it 'always has the work_package class' do expect(helper.work_package_css_classes(stub_work_package)).to include('work_package') end - it "should return the position of the work_package's status" do - status = double('status', is_closed?: false) - - allow(stub_work_package).to receive(:status).and_return(status) - allow(status).to receive(:position).and_return(5) + it "returns the position of the work_package's status" do + stub_work_package.status = open_status + allow(open_status).to receive(:position).and_return(5) expect(helper.work_package_css_classes(stub_work_package)).to include('status-5') end - it "should return the position of the work_package's priority" do - priority = double('priority') - - allow(stub_work_package).to receive(:priority).and_return(priority) + it "returns the position of the work_package's priority" do allow(priority).to receive(:position).and_return(5) expect(helper.work_package_css_classes(stub_work_package)).to include('priority-5') end - it 'should have a closed class if the work_package is closed' do + it 'has a closed class if the work_package is closed' do allow(stub_work_package).to receive(:closed?).and_return(true) expect(helper.work_package_css_classes(stub_work_package)).to include('closed') end - it 'should not have a closed class if the work_package is not closed' do + it 'has no closed class if the work_package is not closed' do allow(stub_work_package).to receive(:closed?).and_return(false) expect(helper.work_package_css_classes(stub_work_package)).not_to include('closed') end - it 'should have an overdue class if the work_package is overdue' do + it 'has an overdue class if the work_package is overdue' do allow(stub_work_package).to receive(:overdue?).and_return(true) expect(helper.work_package_css_classes(stub_work_package)).to include('overdue') end - it 'should not have an overdue class if the work_package is not overdue' do + it 'has an overdue class if the work_package is not overdue' do allow(stub_work_package).to receive(:overdue?).and_return(false) expect(helper.work_package_css_classes(stub_work_package)).not_to include('overdue') end - it 'should have a child class if the work_package is a child' do + it 'has a child class if the work_package is a child' do allow(stub_work_package).to receive(:child?).and_return(true) expect(helper.work_package_css_classes(stub_work_package)).to include('child') end - it 'should not have a child class if the work_package is not a child' do + it 'has no child class if the work_package is not a child' do allow(stub_work_package).to receive(:child?).and_return(false) expect(helper.work_package_css_classes(stub_work_package)).not_to include('child') end - it 'should have a parent class if the work_package is a parent' do + it 'has a parent class if the work_package is a parent' do allow(stub_work_package).to receive(:leaf?).and_return(false) expect(helper.work_package_css_classes(stub_work_package)).to include('parent') end - it 'should not have a parent class if the work_package is not a parent' do + it 'has no parent class if the work_package is not a parent' do allow(stub_work_package).to receive(:leaf?).and_return(true) expect(helper.work_package_css_classes(stub_work_package)).not_to include('parent') end - it 'should have a created-by-me class if the work_package is a created by the current user' do + it 'has a created-by-me class if the work_package is a created by the current user' do stub_user = double('user', logged?: true, id: 5) allow(User).to receive(:current).and_return(stub_user) allow(stub_work_package).to receive(:author_id).and_return(5) @@ -257,7 +258,7 @@ describe WorkPackagesHelper, type: :helper do expect(helper.work_package_css_classes(stub_work_package)).to include('created-by-me') end - it 'should not have a created-by-me class if the work_package is not created by the current user' do + it 'has no created-by-me class if the work_package is not created by the current user' do stub_user = double('user', logged?: true, id: 5) allow(User).to receive(:current).and_return(stub_user) allow(stub_work_package).to receive(:author_id).and_return(4) @@ -265,11 +266,11 @@ describe WorkPackagesHelper, type: :helper do expect(helper.work_package_css_classes(stub_work_package)).not_to include('created-by-me') end - it 'should not have a created-by-me class if the work_package is the current user is not logged in' do + it 'has a created-by-me class if the work_package is the current user is not logged in' do expect(helper.work_package_css_classes(stub_work_package)).not_to include('created-by-me') end - it 'should have a assigned-to-me class if the work_package is a created by the current user' do + it 'has a assigned-to-me class if the work_package is a created by the current user' do stub_user = double('user', logged?: true, id: 5) allow(User).to receive(:current).and_return(stub_user) allow(stub_work_package).to receive(:assigned_to_id).and_return(5) @@ -277,7 +278,7 @@ describe WorkPackagesHelper, type: :helper do expect(helper.work_package_css_classes(stub_work_package)).to include('assigned-to-me') end - it 'should not have a assigned-to-me class if the work_package is not created by the current user' do + it 'has no assigned-to-me class if the work_package is not created by the current user' do stub_user = double('user', logged?: true, id: 5) allow(User).to receive(:current).and_return(stub_user) allow(stub_work_package).to receive(:assigned_to_id).and_return(4) @@ -285,7 +286,7 @@ describe WorkPackagesHelper, type: :helper do expect(helper.work_package_css_classes(stub_work_package)).not_to include('assigned-to-me') end - it 'should not have a assigned-to-me class if the work_package is the current user is not logged in' do + it 'has no assigned-to-me class if the work_package is the current user is not logged in' do expect(helper.work_package_css_classes(stub_work_package)).not_to include('assigned-to-me') end end