From 551d0a3c03ebb74b0893252683361c280e86849e Mon Sep 17 00:00:00 2001 From: Adam Niedzielski Date: Thu, 13 Apr 2017 17:09:30 +0200 Subject: [PATCH] Fix appending state to issuable references Closes #30874. Ignore nodes with no children. Append directly to the node instead of the last child of the node to avoid inheriting formatting from the last child --- lib/banzai/filter/issuable_state_filter.rb | 4 +- .../filter/issuable_state_filter_spec.rb | 43 ++++++++++++++----- 2 files changed, 34 insertions(+), 13 deletions(-) diff --git a/lib/banzai/filter/issuable_state_filter.rb b/lib/banzai/filter/issuable_state_filter.rb index 6b78aa795b4..0b2b8bd7f4d 100644 --- a/lib/banzai/filter/issuable_state_filter.rb +++ b/lib/banzai/filter/issuable_state_filter.rb @@ -13,8 +13,8 @@ module Banzai issuables = extractor.extract([doc]) issuables.each do |node, issuable| - if VISIBLE_STATES.include?(issuable.state) - node.children.last.content += " [#{issuable.state}]" + if VISIBLE_STATES.include?(issuable.state) && node.children.present? + node.add_child(Nokogiri::XML::Text.new(" [#{issuable.state}]", doc)) end end diff --git a/spec/lib/banzai/filter/issuable_state_filter_spec.rb b/spec/lib/banzai/filter/issuable_state_filter_spec.rb index 603b79a323c..5cb98163746 100644 --- a/spec/lib/banzai/filter/issuable_state_filter_spec.rb +++ b/spec/lib/banzai/filter/issuable_state_filter_spec.rb @@ -6,8 +6,8 @@ describe Banzai::Filter::IssuableStateFilter, lib: true do let(:user) { create(:user) } - def create_link(data) - link_to('text', '', class: 'gfm has-tooltip', data: data) + def create_link(text, data) + link_to(text, '', class: 'gfm has-tooltip', data: data) end it 'ignores non-GFM links' do @@ -19,16 +19,37 @@ describe Banzai::Filter::IssuableStateFilter, lib: true do it 'ignores non-issuable links' do project = create(:empty_project, :public) - link = create_link(project: project, reference_type: 'issue') + link = create_link('text', project: project, reference_type: 'issue') doc = filter(link, current_user: user) expect(doc.css('a').last.text).to eq('text') end + it 'ignores issuable links with empty content' do + issue = create(:issue, :closed) + link = create_link('', issue: issue.id, reference_type: 'issue') + doc = filter(link, current_user: user) + + expect(doc.css('a').last.text).to eq('') + end + + it 'adds text with standard formatting' do + issue = create(:issue, :closed) + link = create_link( + 'something else'.html_safe, + issue: issue.id, + reference_type: 'issue' + ) + doc = filter(link, current_user: user) + + expect(doc.css('a').last.inner_html). + to eq('something else [closed]') + end + context 'for issue references' do it 'ignores open issue references' do issue = create(:issue) - link = create_link(issue: issue.id, reference_type: 'issue') + link = create_link('text', issue: issue.id, reference_type: 'issue') doc = filter(link, current_user: user) expect(doc.css('a').last.text).to eq('text') @@ -36,7 +57,7 @@ describe Banzai::Filter::IssuableStateFilter, lib: true do it 'ignores reopened issue references' do reopened_issue = create(:issue, :reopened) - link = create_link(issue: reopened_issue.id, reference_type: 'issue') + link = create_link('text', issue: reopened_issue.id, reference_type: 'issue') doc = filter(link, current_user: user) expect(doc.css('a').last.text).to eq('text') @@ -44,7 +65,7 @@ describe Banzai::Filter::IssuableStateFilter, lib: true do it 'appends [closed] to closed issue references' do closed_issue = create(:issue, :closed) - link = create_link(issue: closed_issue.id, reference_type: 'issue') + link = create_link('text', issue: closed_issue.id, reference_type: 'issue') doc = filter(link, current_user: user) expect(doc.css('a').last.text).to eq('text [closed]') @@ -54,7 +75,7 @@ describe Banzai::Filter::IssuableStateFilter, lib: true do context 'for merge request references' do it 'ignores open merge request references' do mr = create(:merge_request) - link = create_link(merge_request: mr.id, reference_type: 'merge_request') + link = create_link('text', merge_request: mr.id, reference_type: 'merge_request') doc = filter(link, current_user: user) expect(doc.css('a').last.text).to eq('text') @@ -62,7 +83,7 @@ describe Banzai::Filter::IssuableStateFilter, lib: true do it 'ignores reopened merge request references' do mr = create(:merge_request, :reopened) - link = create_link(merge_request: mr.id, reference_type: 'merge_request') + link = create_link('text', merge_request: mr.id, reference_type: 'merge_request') doc = filter(link, current_user: user) expect(doc.css('a').last.text).to eq('text') @@ -70,7 +91,7 @@ describe Banzai::Filter::IssuableStateFilter, lib: true do it 'ignores locked merge request references' do mr = create(:merge_request, :locked) - link = create_link(merge_request: mr.id, reference_type: 'merge_request') + link = create_link('text', merge_request: mr.id, reference_type: 'merge_request') doc = filter(link, current_user: user) expect(doc.css('a').last.text).to eq('text') @@ -78,7 +99,7 @@ describe Banzai::Filter::IssuableStateFilter, lib: true do it 'appends [closed] to closed merge request references' do mr = create(:merge_request, :closed) - link = create_link(merge_request: mr.id, reference_type: 'merge_request') + link = create_link('text', merge_request: mr.id, reference_type: 'merge_request') doc = filter(link, current_user: user) expect(doc.css('a').last.text).to eq('text [closed]') @@ -86,7 +107,7 @@ describe Banzai::Filter::IssuableStateFilter, lib: true do it 'appends [merged] to merged merge request references' do mr = create(:merge_request, :merged) - link = create_link(merge_request: mr.id, reference_type: 'merge_request') + link = create_link('text', merge_request: mr.id, reference_type: 'merge_request') doc = filter(link, current_user: user) expect(doc.css('a').last.text).to eq('text [merged]')