DEV: Merge category and tag hashtags code paths (#10216)

DEV: Merge category and tag hashtags code paths (#10216)

Category and tag hashtags used to be handled differently even though most of the code was very similar. This design was the root cause of multiple issues related to hashtags.

This commit reduces the number of requests (just one and debounced better), removes the use of CSS classes which marked resolved hashtags, simplifies a lot of the code as there is a single source of truth and previous race condition fixes are now useless.

It also includes a very minor security fix which let unauthorized users to guess hidden tags.

diff --git a/app/assets/javascripts/discourse/app/components/composer-editor.js b/app/assets/javascripts/discourse/app/components/composer-editor.js
index e33973c..f1520eb 100644
--- a/app/assets/javascripts/discourse/app/components/composer-editor.js
+++ b/app/assets/javascripts/discourse/app/components/composer-editor.js
@@ -12,13 +12,9 @@ import {
   fetchUnseenMentions
 } from "discourse/lib/link-mentions";
 import {
-  linkSeenCategoryHashtags,
-  fetchUnseenCategoryHashtags
-} from "discourse/lib/link-category-hashtags";
-import {
-  linkSeenTagHashtags,
-  fetchUnseenTagHashtags
-} from "discourse/lib/link-tag-hashtag";
+  linkSeenHashtags,
+  fetchUnseenHashtags
+} from "discourse/lib/link-hashtags";
 import Composer from "discourse/models/composer";
 import { load, LOADING_ONEBOX_CSS_CLASS } from "pretty-text/oneboxer";
 import { applyInlineOneboxes } from "pretty-text/inline-oneboxer";
@@ -520,16 +516,13 @@ export default Component.extend({
     });
   },
 
-  _renderUnseenCategoryHashtags($preview, unseen) {
-    fetchUnseenCategoryHashtags(unseen).then(() => {
-      linkSeenCategoryHashtags($preview);
-    });
-  },
-
-  _renderUnseenTagHashtags($preview, unseen) {
-    fetchUnseenTagHashtags(unseen).then(() => {
-      linkSeenTagHashtags($preview);
-    });
+  _renderUnseenHashtags($preview) {
+    const unseen = linkSeenHashtags($preview);
+    if (unseen.length > 0) {
+      fetchUnseenHashtags(unseen).then(() => {
+        linkSeenHashtags($preview);
+      });
+    }
   },
 
   _loadInlineOneboxes(inline) {
@@ -927,30 +920,10 @@ export default Component.extend({
       this._warnMentionedGroups($preview);
       this._warnCannotSeeMention($preview);
 
-      // Paint category hashtags
-      const unseenCategoryHashtags = linkSeenCategoryHashtags($preview);
-      if (unseenCategoryHashtags.length) {
-        debounce(
-          this,
-          this._renderUnseenCategoryHashtags,
-          $preview,
-          unseenCategoryHashtags,
-          450
-        );
-      }
-
-      // Paint tag hashtags
-      if (this.siteSettings.tagging_enabled) {
-        const unseenTagHashtags = linkSeenTagHashtags($preview);
-        if (unseenTagHashtags.length) {
-          debounce(
-            this,
-            this._renderUnseenTagHashtags,
-            $preview,
-            unseenTagHashtags,
-            450
-          );
-        }
+      // Paint category and tag hashtags
+      const unseenHashtags = linkSeenHashtags($preview);
+      if (unseenHashtags.length > 0) {
+        debounce(this, this._renderUnseenHashtags, $preview, 450);
       }
 
       // Paint oneboxes
diff --git a/app/assets/javascripts/discourse/app/lib/link-category-hashtags.js b/app/assets/javascripts/discourse/app/lib/link-category-hashtags.js
deleted file mode 100644
index f074c69..0000000
--- a/app/assets/javascripts/discourse/app/lib/link-category-hashtags.js
+++ /dev/null
@@ -1,61 +0,0 @@
-import { schedule } from "@ember/runloop";
-import { ajax } from "discourse/lib/ajax";
-import { replaceSpan } from "discourse/lib/category-hashtags";
-
-const validCategoryHashtags = {};
-const checkedCategoryHashtags = [];
-const testedClass = `hashtag-category-tested`;
-
-function updateFound($hashtags, categorySlugs) {
-  schedule("afterRender", () => {
-    $hashtags.each((index, hashtag) => {
-      const categorySlug = categorySlugs[index];
-      const link = validCategoryHashtags[categorySlug];
-      const $hashtag = $(hashtag);
-
-      if (link) {
-        if ($hashtag.data("type") !== "tag") {
-          replaceSpan($hashtag, categorySlug, link, "category");
-        }
-      } else if (checkedCategoryHashtags.indexOf(categorySlug) !== -1) {
-        if (hashtag.tagName !== "A") {
-          $hashtag.addClass(testedClass);
-        }
-      }
-    });
-  });
-}
-
-export function linkSeenCategoryHashtags($elem) {
-  const $hashtags = $(`span.hashtag:not(.${testedClass}), a.hashtag`, $elem);
-  const unseen = [];
-
-  if ($hashtags.length) {
-    const categorySlugs = $hashtags.map((_, hashtag) =>
-      $(hashtag)
-        .text()
-        .substr(1)
-    );
-    if (categorySlugs.length) {
-      _.uniq(categorySlugs).forEach(categorySlug => {
-        if (checkedCategoryHashtags.indexOf(categorySlug) === -1) {
-          unseen.push(categorySlug);
-        }
-      });
-    }
-    updateFound($hashtags, categorySlugs);
-  }
-
-  return unseen;
-}
-
-export function fetchUnseenCategoryHashtags(categorySlugs) {
-  return ajax("/category_hashtags/check", {
-    data: { category_slugs: categorySlugs }
-  }).then(response => {
-    response.valid.forEach(category => {
-      validCategoryHashtags[category.slug] = category.url;
-    });
-    checkedCategoryHashtags.push.apply(checkedCategoryHashtags, categorySlugs);
-  });
-}
diff --git a/app/assets/javascripts/discourse/app/lib/link-hashtags.js b/app/assets/javascripts/discourse/app/lib/link-hashtags.js
new file mode 100644
index 0000000..6456238
--- /dev/null
+++ b/app/assets/javascripts/discourse/app/lib/link-hashtags.js
@@ -0,0 +1,51 @@
+import { schedule } from "@ember/runloop";
+import { ajax } from "discourse/lib/ajax";
+import { replaceSpan } from "discourse/lib/category-hashtags";
+import { TAG_HASHTAG_POSTFIX } from "discourse/lib/tag-hashtags";
+
+const categoryHashtags = {};
+const tagHashtags = {};
+const checkedHashtags = new Set();
+
+export function linkSeenHashtags($elem) {
+  const $hashtags = $elem.find("span.hashtag");
+  if ($hashtags.length === 0) {
+    return [];
+  }
+
+  const slugs = [...$hashtags.map((_, hashtag) => hashtag.innerText.substr(1))];
+
+  schedule("afterRender", () => {
+    $hashtags.each((index, hashtag) => {
+      let slug = slugs[index];
+      const hasTagSuffix = slug.endsWith(TAG_HASHTAG_POSTFIX);
+      if (hasTagSuffix) {
+        slug = slug.substr(0, slug.length - TAG_HASHTAG_POSTFIX.length);
+      }
+
+      if (categoryHashtags[slug] && !hasTagSuffix) {
+        replaceSpan($(hashtag), slug, categoryHashtags[slug]);
+      } else if (tagHashtags[slug]) {
+        replaceSpan($(hashtag), slug, tagHashtags[slug]);
+      }
+    });
+  });
+
+  return slugs.uniq().filter(slug => !checkedHashtags.has(slug));
+}
+
+export function fetchUnseenHashtags(slugs) {
+  return ajax("/hashtags", {
+    data: { slugs }
+  }).then(response => {
+    Object.keys(response.categories).forEach(slug => {
+      categoryHashtags[slug] = response.categories[slug];
+    });
+
+    Object.keys(response.tags).forEach(slug => {
+      tagHashtags[slug] = response.tags[slug];
+    });
+
+    slugs.forEach(checkedHashtags.add, checkedHashtags);
+  });
+}
diff --git a/app/assets/javascripts/discourse/app/lib/link-tag-hashtag.js b/app/assets/javascripts/discourse/app/lib/link-tag-hashtag.js
deleted file mode 100644
index 1d689fe..0000000
--- a/app/assets/javascripts/discourse/app/lib/link-tag-hashtag.js
+++ /dev/null
@@ -1,62 +0,0 @@
-import { schedule } from "@ember/runloop";
-import { ajax } from "discourse/lib/ajax";
-import { replaceSpan } from "discourse/lib/category-hashtags";
-import { TAG_HASHTAG_POSTFIX } from "discourse/lib/tag-hashtags";
-
-const validTagHashtags = {};
-const checkedTagHashtags = [];
-const testedClass = "hashtag-tag-tested";
-
-function updateFound($hashtags, tagValues) {
-  schedule("afterRender", () => {
-    $hashtags.each((index, hashtag) => {
-      const tagValue = tagValues[index];
-      const link = validTagHashtags[tagValue];
-      const $hashtag = $(hashtag);
-
-      if (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);
-      }
-    });
-  });
-}
-
-export function linkSeenTagHashtags($elem) {
-  const $hashtags = $(`span.hashtag:not(.${testedClass})`, $elem);
-  const unseen = [];
-
-  if ($hashtags.length) {

[... diff too long, it was truncated ...]

GitHub sha: cf02c518

This commit appears in #10216 which was approved by ZogStriP. It was merged by udan11.

This commit has been mentioned on Discourse Meta. There might be relevant details there:

https://meta.discourse.org/t/custom-wizard-plugin/73345/560