[$250] @-mentions don't work in the same message as an inline code block or a multi-line code block

[m-natarajan]

If you haven’t already, check out our contributing guidelines for onboarding and email contributors@expensify.com to request to join our Slack channel!

---

Version Number: 9.0.59-0
Reproducible in staging?: Y
Reproducible in production?: Y
If this was caught on HybridApp, is this reproducible on New Expensify Standalone?:
If this was caught during regression testing, add the test name, ID and link from TestRail:
Email or phone of affected tester (no customers):
Logs: https://stackoverflow.com/c/expensify/questions/4856
Expensify/Expensify Issue URL:
Issue reported by: @rafecolton
Slack conversation (hyperlinked to channel name): ts_external_expensify_quality

Action Performed:

1. Go to staging.new.expensify.com
2. Open any DM
3. Compose a message with @mention and code message

Expected Result:

@mention highlighted with blue or green along with the inline code block message

Actual Result:

@mention not highlighted properly when composed with a inline code block message

Workaround:

Can the user still use Expensify without this being fixed? Have you informed them of the workaround?

Platforms:

Which of our officially supported platforms is this issue occurring on?

• Android: Standalone
• Android: HybridApp
• Android: mWeb Chrome
• iOS: Standalone
• iOS: HybridApp
• iOS: mWeb Safari
• MacOS: Chrome / Safari
• MacOS: Desktop

Screenshots/Videos

Add any screenshot/video evidence

View all open jobs on GitHub

Upwork Automation - Do Not Edit

• Upwork Job URL: https://www.upwork.com/jobs/~021854839507953800276
• Upwork Job ID: 1854839507953800276
• Last Price Increase: 2024-11-08
• Automatic offers:
• allgandalf | Contributor | 104895678
• FitseTLT | Contributor | 104897607

Issue Owner

Current Issue Owner: @stephanieelliott

[melvin-bot]

Triggered auto assignment to @stephanieelliott (Bug), see https://stackoverflow.com/c/expensify/questions/14418 for more details. Please add this bug to a GH project, as outlined in the SO.

[melvin-bot]

Triggered auto assignment to @MonilBhavsar (AutoAssignerNewDotQuality)

[ra-md]

Edited by proposal-police: This proposal was edited at 2024-11-08 08:59:32 UTC.

Proposal

Please re-state the problem that we are trying to solve in this issue.

@-mentions don't work if backtick or code block appeared anywhere after @-mentions

What is the root cause of that problem?

When submitting a comment, we invoke buildOptimisticAddCommentReportAction in addActions, followed by getParsedComment, and then completeShortMention. In the completeShortMention function, we replace all valid short mentions found in the text with their corresponding full mentions using CONST.REGEX.SHORT_MENTION..

App/src/libs/ReportUtils.ts

Lines 4262 to 4271 in c658375

	function completeShortMention(text: string): string {
	return text.replace(CONST.REGEX.SHORT_MENTION, (match) => {
	if (!Str.isValidMention(match)) {
	return match;
	}
	const mention = match.substring(1);
	const mentionWithDomain = addDomainToShortMention(mention);
	return mentionWithDomain ? `@${mentionWithDomain}` : match;
	});
	}

There is an issue with CONST.REGEX.SHORT_MENTION:

App/src/CONST.ts

Line 2983 in c658375

SHORT_MENTION: new RegExp(`@[\\w\\-\\+\\'#@]+(?:\\.[\\w\\-\\'\\+]+)*(?![^\`]*\`)`, 'gim'),

Apparently, this regex (?![^\`]*`) is used to exclude mention if it appears inside a code block. However, this regex is slightly inaccurate because mention will be excluded even if it’s not inside a code block. This happens because (?![^\`]*`) only checks for the presence of a backtick anywhere later in the string, after mention.

As a result, mention like ''@ra-md `code` or @ra-md `" will prevent the mention from working correctly.

What changes do you think we should make in order to solve the problem?

Modify the regex like this:

SHORT_MENTION: new RegExp("(?<!`)@[\\w\\-\\+\\'#@]+(?:\\.[\\w\\-\\'\\+]+)*(?!`)", 'gim'),

This regex excludes mention only if it’s inside a code block, but it won't exclude the mention if there’s a backtick or code block anywhere later in the string after the mention.

What alternative solutions did you explore? (Optional)

[MonilBhavsar]

This can be worked externally. Exporting...

[melvin-bot]

Job added to Upwork: https://www.upwork.com/jobs/~021854839507953800276

[melvin-bot]

Triggered auto assignment to Contributor-plus team member for initial proposal review - @rayane-djouah (External)

[MonilBhavsar]

@rayane-djouah could you please take a look at this proposal when you have a moment please #52214 (comment)

[rayane-djouah]

Reviewing 👀

[FitseTLT]

Proposal

Please re-state the problem that we are trying to solve in this issue.

@-mentions don't work in the same message as an inline code block or a multi-line code block

What is the root cause of that problem?

Our short mention regex doesn't allow any backtick after the mention

App/src/CONST.ts

Line 2984 in 533aed6

SHORT_MENTION: new RegExp(`@[\\w\\-\\+\\'#@]+(?:\\.[\\w\\-\\'\\+]+)*(?![^\`]*\`)`, 'gim'),

What changes do you think we should make in order to solve the problem?

What we should check for in the regex is whether the short mention is inside a backtick pair. The short mention will only be overridden by the code block if there are odd number of backticks to the left of the mention (considering consecutive backticks as one as the other duplicate backticks will only be inserted into the code block) there shouldn't be any backtick after the mention so we should update the regex accordingly

        SHORT_MENTION: new RegExp(`(?<!^[^\`]*(?:\`+[^\`]+\`+[^\`]+)*\`+[^\`]*)@[\\w\\-\\+\\'#@]+(?:\\.[\\w\\-\\'\\+]+)*|@[\\w\\-\\+\\'#@]+(?:\\.[\\w\\-\\'\\+]+)*(?![^\`]*\`)`, 'gim'),

In short what the regex checks is whether the mention is not preceded by an odd sets of backticks (which would mean there is an opened backtick before it waiting to convert the mention into code block if it gets any backtick after) or there are no one backtick after the mention.

What alternative solutions did you explore? (Optional)

[rayane-djouah]

Asked for a volunteer: https://expensify.slack.com/archives/C02NK2DQWUX/p1731515957295929

[allgandalf]

Taking this one over (slack), please assign me here @MonilBhavsar

[allgandalf]

I was able to reproduce this one, please note that this is reproducible only for private domains (i.e. Short mentions ).

As such I like @ra-md proposal here, their RCA is super clear and solution makes sense to me, though please mind to test the changes thoroughly during PR phase @ra-md we do not want to leave any edge cases, the regex is limited to only short mentions so test scope is limited but highly critical

Lets hold assignment for a bit @MonilBhavsar

🎀👀🎀 C+ reviewed

[melvin-bot]

Current assignee @MonilBhavsar is eligible for the choreEngineerContributorManagement assigner, not assigning anyone new.

[allgandalf]

@FitseTLT sorry, but i found your proposal over-engineered, if you observed something in @ra-md 's proposal which will still keep this bug from happening please mention it as well.

I tried to think of all possible P&C's for short mentions and their proposal fits probably all cases (unless you prove otherwise)!

do let us know if you have a say here, open to feedback, but as such @ra-md 's proposal should fix this issue !

[FitseTLT]

Nope @allgandalf you are choosing the wrong proposal.

The proposal is allowing short mentions if it is not immediately preceded by and followed by a backtick but what we are to achieve is more complex than that I have already stated what is expected and how we are going to handle it so please copy my regex and deep test to check that it works perfect.

Some of the cases to test with

1. `sth @shortmention sth` - in this case the mention is inside a code block so not a short mention but the chosen proposal doesn't account for it.
2. `sth` @shortmention sth` - the mention is not in code block so it is a correct short mention
3. `sth @shortmention `sth` - it is inside a code block
4. sth @shortmention `sth` - it is correct mention

[allgandalf]

@FitseTLT all the cases you mentioned above, all give correct format with @ra-md's proposal

Oh also please mind that this bug is only for private domains and not public domains

[FitseTLT]

You are showing me a bug. On 1, 3 it shouldn't have been a long mention but the short mention regex passed it therefore we concatinated the domain before sending it to the BE but the BE turned it to a code block with the long mention. The right result should be the short mention inside block. Did u get me?

[allgandalf]

Ahh, okay i tested on staging now. @MonilBhavsar please hold off assignment for a bit.

@FitseTLT your regex is still very complex, can you maybe try to update the regex to make it more optimised ?

Updating a regex is a really complex tax in general, there are always chances of breaking other things when trying to fix one

[FitseTLT]

Ahh, okay i tested on staging now. @MonilBhavsar please hold off assignment for a bit.

@FitseTLT your regex is still very complex, can you maybe try to update the regex to make it more optimised ?

Updating a regex is a really complex tax in general, there are always chances of breaking other things when trying to fix one

BTW the core structure of the regex is exactly the same as the original one. My regex have two parts separated by or (|)

        SHORT_MENTION: new RegExp(`(?<!^[^\`]*(?:\`+[^\`]+\`+[^\`]+)*\`+[^\`]*)@[\\w\\-\\+\\'#@]+(?:\\.[\\w\\-\\'\\+]+)*|@[\\w\\-\\+\\'#@]+(?:\\.[\\w\\-\\'\\+]+)*(?![^\`]*\`)`, 'gim'),

Part 1 :

(?<!^[^\`]*(?:\`+[^\`]+\`+[^\`]+)*\`+[^\`]*)@[\\w\\-\\+\\'#@]+(?:\\.[\\w\\-\\'\\+]+)*

Part 2:

@[\\w\\-\\+\\'#@]+(?:\\.[\\w\\-\\'\\+]+)*(?![^\`]*\

The Core part that is used untouched for both parts:

@[\\w\\-\\+\\'#@]+(?:\\.[\\w\\-\\'\\+]+)*

Let me explain the two part of the regex a bit:
What we want to achieve is to ensure that the short mention is not inside a code block (back tick pair) but in REGEX you don't have an option to check if sth is in between a character pair instead to achieve it we have to check that the mention is not preceded (look behind (?<!) or not followed by (look ahead (?!) so that if the mention is in between a code block then both conditions will fail but if the mention is only followed by a backtick and not preceded by a backtick (or vice versa) on of the conditions will be satisfied and we consider it a mention.
The Look Behind part is : (?<!^[^\`](?:`+[^\`]+`+[^\`]+)`+[^\`])@[\w\-\+\'#@]+(?:\.[\w\-\'\+]+)
in the look behind we check if there are no odd pair of back ticks because even number of backticks means the code block is already closed before the mention so we can safely allow the mention to be out of the code block.
The Look ahead part is: @[\w\-\+\'#@]+(?:\.[\w\-\'\+]+)(?![^\`]`)
In the look ahead, it is different, we check if there isn't even one backtick because if there is any open backtick before the mention then any backtick after it will render it to be in a code block.

Finally, Consecutive backticks are considered as one, that's why you see + signs after the backtick characters in my regex.

This is the simplest possible regex that correctly handles all the cases, any simpler approach that deals with this issue only is not going to fix the problem from the root cause and we will end up having similar issues as this forever :) all we can do is a deep deep test 👍

[melvin-bot]

📣 @allgandalf 🎉 An offer has been automatically sent to your Upwork account for the Contributor role 🎉 Thanks for contributing to the Expensify app!

Offer link
Upwork job
Please accept the offer and leave a comment on the Github issue letting us know when we can expect a PR to be ready for review 🧑‍💻
Keep in mind: Code of Conduct | Contributing 📖

[allgandalf]

@MonilBhavsar what do you think of @FitseTLT 's proposal ? IMO, updating to such lengthly regex is not really ideal scenario unless there is no other way to work on this, but interested to hear what your opinion is :))

[MonilBhavsar]

Will take a look

@FitseTLT looks like the * updated the formatting of regex in this message #52214
Could you please update it to wrap them in code block. It would be easier to follow. Thanks!

[FitseTLT]

@MonilBhavsar Updated

[MonilBhavsar]

@allgandalf the proposal makes sense to me. But if we can optimise it, let's do it. I didn't look at the ways to optimise it yet

[FitseTLT]

@allgandalf the proposal makes sense to me. But if we can optimise it, let's do it. I didn't look at the ways to optimise it yet

@MonilBhavsar The complexity is due to the task we are aiming to achieve, I already have broken down what every peace of the regex is doing and if we exclude any we end up with recreating similar issues. I think the way to go here is to deep deep test 👍

[MonilBhavsar]

Okay, that should be fine! @allgandalf what do you think?

Let's add automated tests to test simple and edge cases, and test within app too

[allgandalf]

Yes please, tests would be good to have! let's go with @FitseTLT 's proposal

🎀👀🎀 C+ reviewed

[melvin-bot]

Current assignee @MonilBhavsar is eligible for the choreEngineerContributorManagement assigner, not assigning anyone new.

[melvin-bot]

📣 @FitseTLT 🎉 An offer has been automatically sent to your Upwork account for the Contributor role 🎉 Thanks for contributing to the Expensify app!

Offer link
Upwork job
Please accept the offer and leave a comment on the Github issue letting us know when we can expect a PR to be ready for review 🧑‍💻
Keep in mind: Code of Conduct | Contributing 📖

[MonilBhavsar]

Let's do it! @FitseTLT more automated tests are always good. Please add them as much as you can since we're modifying the main regex 😄