FIX: Fix race condition when resolving tag and category hashtags (#10153)

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

diff --git a/app/assets/javascripts/discourse/app/lib/category-hashtags.js b/app/assets/javascripts/discourse/app/lib/category-hashtags.js
index 28042ff..6b6fc5f 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 7179f23..f074c69 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 5aa3f6d..1d689fe 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 e5bbdf3..080ff31 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 4a28ca1..37be13e 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>'
+    );
+  }
+);

GitHub sha: 556f7dc9

This commit appears in #10153 which was approved by ZogStriP, ZogStriP, and ZogStriP. It was merged by SamSaffron.