From 556f7dc9c0f00503171862a1162be9a8b4073fa1 Mon Sep 17 00:00:00 2001
From: Dan Ungureanu <dan@ungureanu.me>
Date: Tue, 7 Jul 2020 03:20:51 +0300
Subject: [PATCH] FIX: Fix race condition when resolving tag and category
 hashtags (#10153)

* FIX: Fix race condition when resolving tag and category hashtags

If the category hashtags were resolved first and then tag hashtags, then
the tags would overwrite the categories. Similarly, if the category
hashtags were resolved last it would overwrite even hashtags which ended
with '::tag'.

* DEV: Add test

* DEV: Fix test
---
 .../discourse/app/lib/category-hashtags.js    |  5 ++--
 .../app/lib/link-category-hashtags.js         | 13 ++++++----
 .../discourse/app/lib/link-tag-hashtag.js     | 16 ++++++++-----
 .../acceptance/category-hashtag-test.js       |  2 +-
 .../acceptance/tag-hashtag-test.js            | 24 ++++++++++++++++---
 5 files changed, 43 insertions(+), 17 deletions(-)

diff --git a/app/assets/javascripts/discourse/app/lib/category-hashtags.js b/app/assets/javascripts/discourse/app/lib/category-hashtags.js
index 28042fff160..6b6fc5f3399 100644
--- a/app/assets/javascripts/discourse/app/lib/category-hashtags.js
+++ b/app/assets/javascripts/discourse/app/lib/category-hashtags.js
@@ -5,9 +5,10 @@ import {
   inCodeBlock
 } from "discourse/lib/utilities";
 
-export function replaceSpan($elem, categorySlug, categoryLink) {
+export function replaceSpan($elem, categorySlug, categoryLink, type) {
+  type = type ? ` data-type="${type}"` : "";
   $elem.replaceWith(
-    `<a href="${categoryLink}" class="hashtag">#<span>${categorySlug}</span></a>`
+    `<a href="${categoryLink}" class="hashtag"${type}>#<span>${categorySlug}</span></a>`
   );
 }
 
diff --git a/app/assets/javascripts/discourse/app/lib/link-category-hashtags.js b/app/assets/javascripts/discourse/app/lib/link-category-hashtags.js
index 7179f234f19..f074c69cc70 100644
--- a/app/assets/javascripts/discourse/app/lib/link-category-hashtags.js
+++ b/app/assets/javascripts/discourse/app/lib/link-category-hashtags.js
@@ -4,8 +4,7 @@ import { replaceSpan } from "discourse/lib/category-hashtags";
 
 const validCategoryHashtags = {};
 const checkedCategoryHashtags = [];
-const testedKey = "tested";
-const testedClass = `hashtag-${testedKey}`;
+const testedClass = `hashtag-category-tested`;
 
 function updateFound($hashtags, categorySlugs) {
   schedule("afterRender", () => {
@@ -15,16 +14,20 @@ function updateFound($hashtags, categorySlugs) {
       const $hashtag = $(hashtag);
 
       if (link) {
-        replaceSpan($hashtag, categorySlug, link);
+        if ($hashtag.data("type") !== "tag") {
+          replaceSpan($hashtag, categorySlug, link, "category");
+        }
       } else if (checkedCategoryHashtags.indexOf(categorySlug) !== -1) {
-        $hashtag.addClass(testedClass);
+        if (hashtag.tagName !== "A") {
+          $hashtag.addClass(testedClass);
+        }
       }
     });
   });
 }
 
 export function linkSeenCategoryHashtags($elem) {
-  const $hashtags = $(`span.hashtag:not(.${testedClass})`, $elem);
+  const $hashtags = $(`span.hashtag:not(.${testedClass}), a.hashtag`, $elem);
   const unseen = [];
 
   if ($hashtags.length) {
diff --git a/app/assets/javascripts/discourse/app/lib/link-tag-hashtag.js b/app/assets/javascripts/discourse/app/lib/link-tag-hashtag.js
index 5aa3f6d6120..1d689fefdde 100644
--- a/app/assets/javascripts/discourse/app/lib/link-tag-hashtag.js
+++ b/app/assets/javascripts/discourse/app/lib/link-tag-hashtag.js
@@ -5,7 +5,7 @@ import { TAG_HASHTAG_POSTFIX } from "discourse/lib/tag-hashtags";
 
 const validTagHashtags = {};
 const checkedTagHashtags = [];
-const testedClass = "tag-hashtag-tested";
+const testedClass = "hashtag-tag-tested";
 
 function updateFound($hashtags, tagValues) {
   schedule("afterRender", () => {
@@ -15,7 +15,9 @@ function updateFound($hashtags, tagValues) {
       const $hashtag = $(hashtag);
 
       if (link) {
-        replaceSpan($hashtag, tagValue, link);
+        if (!$hashtag.data("type") || $hashtag.data("type") === "tag") {
+          replaceSpan($hashtag, tagValue, link, $hashtag.data("type"));
+        }
       } else if (checkedTagHashtags.indexOf(tagValue) !== -1) {
         $hashtag.addClass(testedClass);
       }
@@ -29,10 +31,12 @@ export function linkSeenTagHashtags($elem) {
 
   if ($hashtags.length) {
     const tagValues = $hashtags.map((_, hashtag) => {
-      return $(hashtag)
-        .text()
-        .substr(1)
-        .replace(TAG_HASHTAG_POSTFIX, "");
+      let text = $(hashtag).text();
+      if (text.endsWith(TAG_HASHTAG_POSTFIX)) {
+        text = text.slice(0, -TAG_HASHTAG_POSTFIX.length);
+        $(hashtag).data("type", "tag");
+      }
+      return text.substr(1);
     });
 
     if (tagValues.length) {
diff --git a/test/javascripts/acceptance/category-hashtag-test.js b/test/javascripts/acceptance/category-hashtag-test.js
index e5bbdf3c48d..080ff313bac 100644
--- a/test/javascripts/acceptance/category-hashtag-test.js
+++ b/test/javascripts/acceptance/category-hashtag-test.js
@@ -13,6 +13,6 @@ QUnit.test("category hashtag is cooked properly", async assert => {
     find(".d-editor-preview:visible")
       .html()
       .trim(),
-    '<p>this is a category hashtag <a href="/c/bugs" class="hashtag">#<span>bug</span></a></p>'
+    '<p>this is a category hashtag <a href="/c/bugs" class="hashtag" data-type="category">#<span>bug</span></a></p>'
   );
 });
diff --git a/test/javascripts/acceptance/tag-hashtag-test.js b/test/javascripts/acceptance/tag-hashtag-test.js
index 4a28ca137dd..37be13e7261 100644
--- a/test/javascripts/acceptance/tag-hashtag-test.js
+++ b/test/javascripts/acceptance/tag-hashtag-test.js
@@ -6,7 +6,10 @@ acceptance("Tag Hashtag", {
   pretend(server, helper) {
     server.get("/tags/check", () => {
       return helper.response({
-        valid: [{ value: "monkey", url: "/tag/monkey" }]
+        valid: [
+          { value: "monkey", url: "/tag/monkey" },
+          { value: "bug", url: "/tag/bug" }
+        ]
       });
     });
   }
@@ -16,8 +19,7 @@ QUnit.test("tag is cooked properly", async assert => {
   await visit("/t/internationalization-localization/280");
   await click("#topic-footer-buttons .btn.create");
 
-  await fillIn(".d-editor-input", "this is a tag hashtag #monkey::tag");
-  // TODO: Test that the autocomplete shows
+  await fillIn(".d-editor-input", "this is a tag hashtag #monkey");
   assert.equal(
     find(".d-editor-preview:visible")
       .html()
@@ -25,3 +27,19 @@ QUnit.test("tag is cooked properly", async assert => {
     '<p>this is a tag hashtag <a href="/tag/monkey" class="hashtag">#<span>monkey</span></a></p>'
   );
 });
+
+QUnit.test(
+  "tags and categories with same name are cooked properly",
+  async assert => {
+    await visit("/t/internationalization-localization/280");
+    await click("#topic-footer-buttons .btn.create");
+
+    await fillIn(".d-editor-input", "#bug vs #bug::tag");
+    assert.equal(
+      find(".d-editor-preview:visible")
+        .html()
+        .trim(),
+      '<p><a href="/c/bugs" class="hashtag" data-type="category">#<span>bug</span></a> vs <a href="/tag/bug" class="hashtag" data-type="tag">#<span>bug</span></a></p>'
+    );
+  }
+);