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
This commit is contained in:
Adam Niedzielski 2017-04-13 17:09:30 +02:00
parent 9daf4f646b
commit 551d0a3c03
2 changed files with 34 additions and 13 deletions

View file

@ -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

View file

@ -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 <strong>else</strong>'.html_safe,
issue: issue.id,
reference_type: 'issue'
)
doc = filter(link, current_user: user)
expect(doc.css('a').last.inner_html).
to eq('something <strong>else</strong> [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]')