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
This commit is contained in:
Dan Ungureanu 2020-07-07 03:20:51 +03:00 committed by GitHub
parent b9e3db6387
commit 556f7dc9c0
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
5 changed files with 43 additions and 17 deletions

View File

@ -5,9 +5,10 @@ import {
inCodeBlock inCodeBlock
} from "discourse/lib/utilities"; } from "discourse/lib/utilities";
export function replaceSpan($elem, categorySlug, categoryLink) { export function replaceSpan($elem, categorySlug, categoryLink, type) {
type = type ? ` data-type="${type}"` : "";
$elem.replaceWith( $elem.replaceWith(
`<a href="${categoryLink}" class="hashtag">#<span>${categorySlug}</span></a>` `<a href="${categoryLink}" class="hashtag"${type}>#<span>${categorySlug}</span></a>`
); );
} }

View File

@ -4,8 +4,7 @@ import { replaceSpan } from "discourse/lib/category-hashtags";
const validCategoryHashtags = {}; const validCategoryHashtags = {};
const checkedCategoryHashtags = []; const checkedCategoryHashtags = [];
const testedKey = "tested"; const testedClass = `hashtag-category-tested`;
const testedClass = `hashtag-${testedKey}`;
function updateFound($hashtags, categorySlugs) { function updateFound($hashtags, categorySlugs) {
schedule("afterRender", () => { schedule("afterRender", () => {
@ -15,16 +14,20 @@ function updateFound($hashtags, categorySlugs) {
const $hashtag = $(hashtag); const $hashtag = $(hashtag);
if (link) { if (link) {
replaceSpan($hashtag, categorySlug, link); if ($hashtag.data("type") !== "tag") {
replaceSpan($hashtag, categorySlug, link, "category");
}
} else if (checkedCategoryHashtags.indexOf(categorySlug) !== -1) { } else if (checkedCategoryHashtags.indexOf(categorySlug) !== -1) {
$hashtag.addClass(testedClass); if (hashtag.tagName !== "A") {
$hashtag.addClass(testedClass);
}
} }
}); });
}); });
} }
export function linkSeenCategoryHashtags($elem) { export function linkSeenCategoryHashtags($elem) {
const $hashtags = $(`span.hashtag:not(.${testedClass})`, $elem); const $hashtags = $(`span.hashtag:not(.${testedClass}), a.hashtag`, $elem);
const unseen = []; const unseen = [];
if ($hashtags.length) { if ($hashtags.length) {

View File

@ -5,7 +5,7 @@ import { TAG_HASHTAG_POSTFIX } from "discourse/lib/tag-hashtags";
const validTagHashtags = {}; const validTagHashtags = {};
const checkedTagHashtags = []; const checkedTagHashtags = [];
const testedClass = "tag-hashtag-tested"; const testedClass = "hashtag-tag-tested";
function updateFound($hashtags, tagValues) { function updateFound($hashtags, tagValues) {
schedule("afterRender", () => { schedule("afterRender", () => {
@ -15,7 +15,9 @@ function updateFound($hashtags, tagValues) {
const $hashtag = $(hashtag); const $hashtag = $(hashtag);
if (link) { 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) { } else if (checkedTagHashtags.indexOf(tagValue) !== -1) {
$hashtag.addClass(testedClass); $hashtag.addClass(testedClass);
} }
@ -29,10 +31,12 @@ export function linkSeenTagHashtags($elem) {
if ($hashtags.length) { if ($hashtags.length) {
const tagValues = $hashtags.map((_, hashtag) => { const tagValues = $hashtags.map((_, hashtag) => {
return $(hashtag) let text = $(hashtag).text();
.text() if (text.endsWith(TAG_HASHTAG_POSTFIX)) {
.substr(1) text = text.slice(0, -TAG_HASHTAG_POSTFIX.length);
.replace(TAG_HASHTAG_POSTFIX, ""); $(hashtag).data("type", "tag");
}
return text.substr(1);
}); });
if (tagValues.length) { if (tagValues.length) {

View File

@ -13,6 +13,6 @@ QUnit.test("category hashtag is cooked properly", async assert => {
find(".d-editor-preview:visible") find(".d-editor-preview:visible")
.html() .html()
.trim(), .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>'
); );
}); });

View File

@ -6,7 +6,10 @@ acceptance("Tag Hashtag", {
pretend(server, helper) { pretend(server, helper) {
server.get("/tags/check", () => { server.get("/tags/check", () => {
return helper.response({ 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 visit("/t/internationalization-localization/280");
await click("#topic-footer-buttons .btn.create"); await click("#topic-footer-buttons .btn.create");
await fillIn(".d-editor-input", "this is a tag hashtag #monkey::tag"); await fillIn(".d-editor-input", "this is a tag hashtag #monkey");
// TODO: Test that the autocomplete shows
assert.equal( assert.equal(
find(".d-editor-preview:visible") find(".d-editor-preview:visible")
.html() .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>' '<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>'
);
}
);