Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

FIX: Do not consider code-blocks when parsing mentions #23280

Merged
36 changes: 36 additions & 0 deletions app/assets/javascripts/discourse/app/lib/mentions-parser.js
@@ -0,0 +1,36 @@
export class MentionsParser {
constructor(prettyText) {
this.prettyText = prettyText;
}

parse(markdown) {
const tokens = this.prettyText.parse(markdown);
const mentions = this.#parse(tokens);
return [...new Set(mentions)];
}

#parse(tokens) {
const mentions = [];
let insideMention = false;
for (const token of tokens) {
if (token.children) {
this.#parse(token.children).forEach((mention) =>
mentions.push(mention)
);
} else {
if (token.type === "mention_open") {
insideMention = true;
} else if (insideMention && token.type === "text") {
mentions.push(this.#extractMention(token.content));
insideMention = false;
}
}
}

return mentions;
}

#extractMention(mention) {
return mention.trim().substring(1);
}
}
6 changes: 3 additions & 3 deletions app/assets/javascripts/discourse/app/lib/plugin-api.js
Expand Up @@ -2126,15 +2126,15 @@ class PluginApi {
* Support for setting a Sidebar panel.
*/
setSidebarPanel(name) {
this._lookupContainer("service:sidebar-state").setPanel(name);
this._lookupContainer("service:sidebar-state")?.setPanel(name);
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This and following methods get called during the route's deactivation and at that moment service is null. That lead to errors in tests, so I've added safe navigation here.

}

/**
* EXPERIMENTAL. Do not use.
* Set combined sidebar section mode. In this mode, sections from all panels are displayed together.
*/
setCombinedSidebarMode() {
this._lookupContainer("service:sidebar-state").setCombinedMode();
this._lookupContainer("service:sidebar-state")?.setCombinedMode();
}

/**
Expand All @@ -2158,7 +2158,7 @@ class PluginApi {
* Hide sidebar switch panels buttons in separated mode.
*/
hideSidebarSwitchPanelButtons() {
this._lookupContainer("service:sidebar-state").hideSwitchPanelButtons();
this._lookupContainer("service:sidebar-state")?.hideSwitchPanelButtons();
}

/**
Expand Down
10 changes: 9 additions & 1 deletion app/assets/javascripts/discourse/app/lib/text.js
Expand Up @@ -9,6 +9,7 @@ import { helperContext } from "discourse-common/lib/helpers";
import { htmlSafe } from "@ember/template";
import loadScript from "discourse/lib/load-script";
import { sanitize as textSanitize } from "pretty-text/sanitizer";
import { MentionsParser } from "discourse/lib/mentions-parser";

function getOpts(opts) {
let context = helperContext();
Expand Down Expand Up @@ -71,10 +72,17 @@ export function sanitizeAsync(text, options) {

export function parseAsync(md, options = {}, env = {}) {
return loadMarkdownIt().then(() => {
return createPrettyText(options).opts.engine.parse(md, env);
return createPrettyText(options).parse(md, env);
});
}

export async function parseMentions(markdown, options) {
await loadMarkdownIt();
const prettyText = createPrettyText(options);
const mentionsParser = new MentionsParser(prettyText);
return mentionsParser.parse(markdown);
}

function loadMarkdownIt() {
return new Promise((resolve) => {
let markdownItURL = Session.currentProp("markdownItURL");
Expand Down
40 changes: 39 additions & 1 deletion app/assets/javascripts/discourse/tests/unit/lib/text-test.js
@@ -1,5 +1,10 @@
import { module, test } from "qunit";
import { cookAsync, excerpt, parseAsync } from "discourse/lib/text";
import {
cookAsync,
excerpt,
parseAsync,
parseMentions,
} from "discourse/lib/text";

module("Unit | Utility | text", function () {
test("parseAsync", async function (assert) {
Expand Down Expand Up @@ -35,3 +40,36 @@ module("Unit | Utility | text", function () {
);
});
});

module("Unit | Utility | text | parseMentions", function () {
test("parses mentions from markdown", async function (assert) {
const markdown = "Hey @user1, @user2, @group1, @group2, @here, @all";
const mentions = await parseMentions(markdown);
assert.deepEqual(mentions, [
"user1",
"user2",
"group1",
"group2",
"here",
"all",
]);
});

test("ignores duplicated mentions", async function (assert) {
const markdown = "Hey @john, @john, @john, @john";
const mentions = await parseMentions(markdown);
assert.deepEqual(mentions, ["john"]);
});

test("ignores mentions in codeblocks", async function (assert) {
const markdown = `Hey
\`\`\`
def foo
@bar = true
end
\`\`\`
`;
const mentions = await parseMentions(markdown);
assert.equal(mentions.length, 0);
});
});
4 changes: 4 additions & 0 deletions app/assets/javascripts/pretty-text/addon/pretty-text.js
Expand Up @@ -125,6 +125,10 @@ export default class {
return result ? result : "";
}

parse(markdown, env = {}) {
return this.opts.engine.parse(markdown, env);
}

sanitize(html) {
return this.opts.sanitizer(html).trim();
}
Expand Down
57 changes: 32 additions & 25 deletions plugins/chat/assets/javascripts/discourse/models/chat-message.js
Expand Up @@ -4,7 +4,7 @@ import { TrackedArray, TrackedObject } from "@ember-compat/tracked-built-ins";
import ChatMessageReaction from "discourse/plugins/chat/discourse/models/chat-message-reaction";
import Bookmark from "discourse/models/bookmark";
import I18n from "I18n";
import { generateCookFunction } from "discourse/lib/text";
import { generateCookFunction, parseMentions } from "discourse/lib/text";
import transformAutolinks from "discourse/plugins/chat/discourse/lib/transform-auto-links";
import { getOwner } from "discourse-common/lib/get-owner";
import discourseLater from "discourse-common/lib/later";
Expand Down Expand Up @@ -170,33 +170,11 @@ export default class ChatMessage {
}

async cook() {
const site = getOwner(this).lookup("service:site");

if (this.isDestroyed || this.isDestroying) {
return;
}

const markdownOptions = {
featuresOverride:
site.markdown_additional_options?.chat?.limited_pretty_text_features,
markdownItRules:
site.markdown_additional_options?.chat
?.limited_pretty_text_markdown_rules,
hashtagTypesInPriorityOrder:
site.hashtag_configurations?.["chat-composer"],
hashtagIcons: site.hashtag_icons,
};

if (ChatMessage.cookFunction) {
this.cooked = ChatMessage.cookFunction(this.message);
} else {
const cookFunction = await generateCookFunction(markdownOptions);
ChatMessage.cookFunction = (raw) => {
return transformAutolinks(cookFunction(raw));
};

this.cooked = ChatMessage.cookFunction(this.message);
}
await this.#ensureCookFunctionInitialized();
this.cooked = ChatMessage.cookFunction(this.message);
}

get read() {
Expand Down Expand Up @@ -263,6 +241,10 @@ export default class ChatMessage {
this.version++;
}

async parseMentions() {
return await parseMentions(this.message, this.#markdownOptions);
}

toJSONDraft() {
if (
this.message?.length === 0 &&
Expand Down Expand Up @@ -361,6 +343,31 @@ export default class ChatMessage {
}
}

async #ensureCookFunctionInitialized() {
if (ChatMessage.cookFunction) {
return;
}

const cookFunction = await generateCookFunction(this.#markdownOptions);
ChatMessage.cookFunction = (raw) => {
return transformAutolinks(cookFunction(raw));
};
}

get #markdownOptions() {
const site = getOwner(this).lookup("service:site");
return {
featuresOverride:
site.markdown_additional_options?.chat?.limited_pretty_text_features,
markdownItRules:
site.markdown_additional_options?.chat
?.limited_pretty_text_markdown_rules,
hashtagTypesInPriorityOrder:
site.hashtag_configurations?.["chat-composer"],
hashtagIcons: site.hashtag_icons,
};
}

#initChatMessageReactionModel(reactions = []) {
return reactions.map((reaction) => ChatMessageReaction.create(reaction));
}
Expand Down
Expand Up @@ -2,7 +2,6 @@ import Service, { inject as service } from "@ember/service";
import { ajax } from "discourse/lib/ajax";
import discourseDebounce from "discourse-common/lib/debounce";
import { bind } from "discourse-common/utils/decorators";
import { mentionRegex } from "pretty-text/mentions";
import { cancel } from "@ember/runloop";
import { tracked } from "@glimmer/tracking";

Expand Down Expand Up @@ -34,11 +33,7 @@ export default class ChatComposerWarningsTracker extends Service {

@bind
reset() {
this.unreachableGroupMentions = [];
this.unreachableGroupMentions = [];
this.overMembersLimitGroupMentions = [];
this.tooManyMentions = false;
this.channelWideMentionDisallowed = false;
this.#resetMentionStats();
this.mentionsCount = 0;
cancel(this.mentionsTimer);
}
Expand All @@ -63,61 +58,42 @@ export default class ChatComposerWarningsTracker extends Service {
return;
}

const mentions = this._extractMentions(currentMessage.message);
this.mentionsCount = mentions?.length;

if (this.mentionsCount > 0) {
this.tooManyMentions =
this.mentionsCount > this.siteSettings.max_mentions_per_chat_message;
currentMessage.parseMentions().then((mentions) => {
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

After the last commits, this line is the only significant change to this function, we called this._extractMentions before, now we call currentMessage.parseMentions().

this.mentionsCount = mentions?.length;

if (!this.tooManyMentions) {
const newMentions = mentions.filter(
(mention) => !(mention in this._mentionWarningsSeen)
);
if (this.mentionsCount > 0) {
this.tooManyMentions =
this.mentionsCount > this.siteSettings.max_mentions_per_chat_message;

this.channelWideMentionDisallowed =
!currentMessage.channel.allowChannelWideMentions &&
(mentions.includes("here") || mentions.includes("all"));

if (newMentions?.length > 0) {
this._recordNewWarnings(newMentions, mentions);
} else {
this._rebuildWarnings(mentions);
}
}
} else {
this.tooManyMentions = false;
this.channelWideMentionDisallowed = false;
this.unreachableGroupMentions = [];
this.overMembersLimitGroupMentions = [];
}
}
if (!this.tooManyMentions) {
const newMentions = mentions.filter(
(mention) => !(mention in this._mentionWarningsSeen)
);

_extractMentions(message) {
const regex = mentionRegex(this.siteSettings.unicode_usernames);
const mentions = [];
let mentionsLeft = true;
this.channelWideMentionDisallowed =
!currentMessage.channel.allowChannelWideMentions &&
(mentions.includes("here") || mentions.includes("all"));

while (mentionsLeft) {
const matches = message.match(regex);

if (matches) {
const mention = matches[1] || matches[2];
mentions.push(mention);
message = message.replaceAll(`${mention}`, "");

if (mentions.length > this.siteSettings.max_mentions_per_chat_message) {
mentionsLeft = false;
if (newMentions?.length > 0) {
this.#recordNewWarnings(newMentions, mentions);
} else {
this.#rebuildWarnings(mentions);
}
}
} else {
mentionsLeft = false;
this.#resetMentionStats();
}
}
});
}

return mentions;
#resetMentionStats() {
this.tooManyMentions = false;
this.channelWideMentionDisallowed = false;
this.unreachableGroupMentions = [];
this.overMembersLimitGroupMentions = [];
}

_recordNewWarnings(newMentions, mentions) {
#recordNewWarnings(newMentions, mentions) {
ajax("/chat/api/mentions/groups.json", {
data: { mentions: newMentions },
})
Expand All @@ -135,12 +111,12 @@ export default class ChatComposerWarningsTracker extends Service {
this._mentionWarningsSeen[warning] = MENTION_RESULT["invalid"];
});

this._rebuildWarnings(mentions);
this.#rebuildWarnings(mentions);
})
.catch(this._rebuildWarnings(mentions));
.catch(this.#rebuildWarnings(mentions));
}

_rebuildWarnings(mentions) {
#rebuildWarnings(mentions) {
const newWarnings = mentions.reduce(
(memo, mention) => {
if (
Expand Down