From e6e57c883569a7d040a2e6c064bdba65b665f5e6 Mon Sep 17 00:00:00 2001
From: Mac Siri <krairit.siri@gmail.com>
Date: Wed, 18 Sep 2019 17:15:06 -0400
Subject: [PATCH] Fix mention's inconsistent hyperlink (#4051)

* Upgrade mention parsing logic

* Avoid hardcoded link in spec
---
 app/labor/markdown_parser.rb       | 31 +++++++++++++++---------------
 spec/labor/markdown_parser_spec.rb | 15 +++++++++++++++
 2 files changed, 30 insertions(+), 16 deletions(-)

diff --git a/app/labor/markdown_parser.rb b/app/labor/markdown_parser.rb
index b7b4eb773..677aaab09 100644
--- a/app/labor/markdown_parser.rb
+++ b/app/labor/markdown_parser.rb
@@ -203,23 +203,22 @@ class MarkdownParser
 
   def wrap_mentions_with_links!(html)
     html_doc = Nokogiri::HTML(html)
-    # looks for node that isn't <code>, <a>, and contains "@"
-    html_doc.xpath('//*[not (self::code) and not(self::a) and contains(text(), "@")]').each do |node|
-      # if the target node is a <p>, <ul>, or <li>, it will have more than 1 child
-      # otherwise inner_html can be use when there's only 1 child
-      if node.children.count > 1
-        # only focus on portion of text with "@"
-        node.xpath("text()[contains(.,'@')]").each do |el|
-          el.replace(el.text.gsub(/\B@[a-z0-9_-]+/i) do |text|
-            user_link_if_exists(text)
-          end)
-        end
-      else
-        # with only 1 child, inner_html can be used update the content
-        node.inner_html = node.inner_html.gsub(/\B@[a-z0-9_-]+/i) do |text|
-          user_link_if_exists(text)
-        end
+
+    # looks for nodes that isn't <code>, <a>, and contains "@"
+    targets = html_doc.xpath('//html/body/*[not (self::code) and not(self::a) and contains(., "@")]').to_a
+
+    # A Queue system to look for and replace possible usernames
+    until targets.empty?
+      node = targets.shift
+
+      # only focus on portion of text with "@"
+      node.xpath("text()[contains(.,'@')]").each do |el|
+        el.replace(el.text.gsub(/\B@[a-z0-9_-]+/i) { |text| user_link_if_exists(text) })
       end
+
+      # enqueue children that has @ in it's text
+      children = node.xpath('*[not(self::code) and not(self::a) and contains(., "@")]').to_a
+      targets.concat(children)
     end
 
     if html_doc.at_css("body")
diff --git a/spec/labor/markdown_parser_spec.rb b/spec/labor/markdown_parser_spec.rb
index 168527232..82a09f23d 100644
--- a/spec/labor/markdown_parser_spec.rb
+++ b/spec/labor/markdown_parser_spec.rb
@@ -81,12 +81,27 @@ RSpec.describe MarkdownParser do
       expect(result).to include "<a", "<em"
     end
 
+    it "works in ul/li tag" do
+      mention = <<~DOC
+        `@#{user.username}` one two, @#{user.username} three four:
+          - `@#{user.username}`
+      DOC
+      result = generate_and_parse_markdown(mention)
+      expect(result).to eq("<p><code>@#{user.username}</code> one two, <a class=\"comment-mentioned-user\" href=\"#{ApplicationConfig['APP_PROTOCOL']}#{ApplicationConfig['APP_DOMAIN']}/#{user.username}\">@#{user.username}</a>\n three four:</p>\n\n<ul>\n<li><code>@#{user.username}</code></li>\n</ul>\n\n")
+    end
+
     it "will not work in code tag" do
       mention = "this is a chunk of text `@#{user.username}`"
       result = generate_and_parse_markdown(mention)
       expect(result).to include "<code"
       expect(result).not_to include "<a"
     end
+
+    it "works with markdown heavy contents" do
+      mention = "test **[link?](https://dev.to/ben/)** thread, @#{user.username} talks :"
+      result = generate_and_parse_markdown(mention)
+      expect(result).to include "<a class=\"comment-mentioned-user\""
+    end
   end
 
   it "renders a double backtick codespan with a word wrapped in single backticks properly" do