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


Former-commit-id: 551d0a3c

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

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 <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]')