FIX: Do not consider code-blocks when parsing mentions

[AndrewPrigorshnev]

We have the max_mentions_per_chat_message site settings; when a user tries to mention more users than allowed, no one gets mentioned.

Chat messages may contain code-blocks with strings that look like mentions:

def foo
  @bar + @baz
end

The problem is that the parsing code considers these as real mentions and counts them when checking the limit. This PR fixes the problem.

I also made these simple refactorings along the way:

1. Extract #resetMentionStats
2. Use "#" as a private methods prefix
3. Extract #ensureCookFunctionInitialized and #markdownOptions

[AndrewPrigorshnev]

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.

[AndrewPrigorshnev]

There are only two changes in this method:

1. Now we call currentMessage.cook() before parsing mentions
2. We call parseMentionedUsernames(currentMessage.cooked) instead of this._extractMentions(currentMessage.message)

[jjaffeux]

Requesting changes while you address concerns of Sam

[AndrewPrigorshnev]

Requesting changes while you address concerns of Sam

@jjaffeux I've pushed commits that address those concerns. With this commit, instead of parsing mentions from HTML we use the Markdown parser:

1. We parse markdown into tokens (the parser respects code-blocks, so mention-like expressions in code-blocks don't get parsed)
2. We traverse tokens and extract mentioned usernames and groups

[AndrewPrigorshnev]

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

[jjaffeux]

Looking good 👍

[jjaffeux]

@AndrewPrigorshnev is that expected that this will not consider the first (valid) @bar ?

test("doesn't consider code-blocks when counting mentions", async function (assert) {
  this.siteSettings.max_mentions_per_chat_message = 2;

  await visit(`/chat/c/-/${channelId}`);
  // since @bar is inside a code-block it shouldn't be considered a mention
  const message = `Hey @user1 @user2 @bar
  \`\`\`
    def foo
      @bar = true
    end
  \`\`\`
  `;
  await fillIn(".chat-composer__input", message);

  assert.dom(".chat-mention-warnings").doesNotExist();
});

[AndrewPrigorshnev]

@AndrewPrigorshnev is that expected that this will not consider the first (valid) @bar ?

test("doesn't consider code-blocks when counting mentions", async function (assert) {
  this.siteSettings.max_mentions_per_chat_message = 2;

  await visit(`/chat/c/-/${channelId}`);
  // since @bar is inside a code-block it shouldn't be considered a mention
  const message = `Hey @user1 @user2 @bar
  \`\`\`
    def foo
      @bar = true
    end
  \`\`\`
  `;
  await fillIn(".chat-composer__input", message);

  assert.dom(".chat-mention-warnings").doesNotExist();
});

That should consider it. I've just tried it locally, added @bar outside the code block as you did and the test failed. Do you see a passing test in this case?

In case you tested that not in tests, but in an actual chat channel, you may not see a warning because the default value for max_mentions_per_chat_message is bigger than 2, while I set it to 2 in the test:

this.siteSettings.max_mentions_per_chat_message = 2;

[jjaffeux]

Yes I think it's fine indeed, I think I misunderstood the test at first, all good 👍