[HOLD for payment 2023-12-28] [$500] Mentioning an email with double @ sign results in an invalid email mention #28946

m-natarajan · 2023-10-05T18:39:10Z

If you haven’t already, check out our contributing guidelines for onboarding and email [email protected] to request to join our Slack channel!

Action Performed:

Open any chat
Type @ and select any email from suggestions
Move the cursor to the beginning
Add another @ sign
Hit the send button
Click on the mentioned email

Expected Result:

The valid email is mentioned

Actual Result:

Invalid email is mentioned

Workaround:

unknown

Platforms:

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

Version Number: 1.3.78-2
Reproducible in staging?: y
Reproducible in production?: y
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
Notes/Photos/Videos: Any additional supporting documentation

web.mov

desktop.mov

RPReplay_Final1696444877.MP4

RPReplay_Final1696444780.MP4

Screen_Recording_20231004_232047_New.Expensify.MP4

Screen_Recording_20231004_232235_Chrome.MP4

Recording.481.mp4

Expensify/Expensify Issue URL:
Issue reported by: @adeel0202
Slack conversation: https://expensify.slack.com/archives/C049HHMV9SM/p1696445508377469

View all open jobs on GitHub

Upwork Automation - Do Not Edit

Upwork Job URL: https://www.upwork.com/jobs/~016783de9809fcb5bd
Upwork Job ID: 1710001713857413120
Last Price Increase: 2023-12-07
Automatic offers:

Victor-Nyagudi | Contributor | 28037832

melvin-bot · 2023-10-05T18:39:15Z

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

melvin-bot · 2023-10-05T18:39:21Z

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

abdel-h66 · 2023-10-05T19:13:13Z

Proposal

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

Mentions with Double leading @ redirect to the wrong route

What is the root cause of that problem?

To correctly calculate the display name or the login of the mentioned user, we only remove the leading @
using

App/src/components/HTMLEngineProvider/HTMLRenderers/MentionUserRenderer.js

Lines 45 to 50 in 8d42ad3

    
           } else if (!_.isEmpty(props.tnode.data)) { 
        
               // We need to remove the LTR unicode and leading @ from data as it is not part of the login 
        
               displayNameOrLogin = props.tnode.data.replace(CONST.UNICODE.LTR, '').slice(1); 
        
               accountID = _.first(PersonalDetailsUtils.getAccountIDsByLogins([displayNameOrLogin])); 
        
               navigationRoute = ROUTES.DETAILS.getRoute(displayNameOrLogin);

This only remove the first @ and the LTR unicode, but in the case of having the double @ it will keep it and causes the navigation link to contain that extra @

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

We can use the following regex to remove all the leading @ and correctly calculate the navigation link

 displayNameOrLogin = props.tnode.data.replace(/^((CONST\.UNICODE\.LTR)*@)*/, '')

Test 1: Removing multiple leading @ characters

const str = '@@@hello world';
const result = str.replace(/^((CONST\.UNICODE\.LTR)*@)*/, '');
console.log(result); // Output: 'hello world'

Test 2: Removing one leading @ character

const str = '@hello world';
const result = str.replace(/^((CONST\.UNICODE\.LTR)*@)*/, '');
console.log(result); // Output: 'hello world'

What alternative solutions did you explore? (Optional)

N/A

akamefi202 · 2023-10-05T20:07:30Z

Proposal

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

Mentioning an email with double @ sign results in an invalid email mention.

What is the root cause of that problem?

The root cause is replace regex of userMentions in ExpensiMark class of expensify-common repo.
https://github.com/Expensify/expensify-common/blob/bc275547d5e0e36cfd25169bdb6eb0ae3f505bf2/lib/ExpensiMark.js#L111-L120

Currently, it inputs the whole string inside <mention-user> tag if the string is matched with regex.
So even if the string contains multiple @ characters at the first, <mention-user> tag will include all those @ characters too.

@@[email protected] => <mention-user>@@[email protected]</mention-user>

That's why the app shows invalid email with @ characters at first, on the mentioned email.

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

We need to update replace regex of userMentions to input only one @ character inside <mention-user> tag.
Rest of @ characters should stay outside of <mention-user> tag.

name: 'userMentions',
regex: new RegExp(`[a-zA-Z0-9.!$%&+/=?^\`{|}-]?(@*?)(@${CONST.REG_EXP.EMAIL_PART})(?!((?:(?!<a).)+)?<\\/a>|[^<]*(<\\/pre>|<\\/code>))`, 'gm'),
replacement: (match, g1, g2) => {
    if (!Str.isValidMention(match)) {
        return match;
    }
    return `${g1}<mention-user>${g2}</mention-user>`;
},

As a result, the app will show valid email although the user inputs multiple @ characters at first.

@@[email protected] => @<mention-user>@[email protected]</mention-user>

What alternative solutions did you explore? (Optional)

N/A

Victor-Nyagudi · 2023-10-06T08:25:06Z

Proposal

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

Mentioning an email with double @ sign results in an invalid email mention

What is the root cause of that problem?

We're matching multiple @ signs in the regex of user mentions rule in ExpensiMark i.e. the @+ in the @+${CONST.REG_EXP.EMAIL_PART} section matches one or more @ symbols.

regex: new RegExp(`[a-zA-Z0-9.!$%&+/=?^\`{|}-]?@+${CONST.REG_EXP.EMAIL_PART}(?!((?:(?!<a).)+)?<\\/a>|[^<]*(<\\/pre>|<\\/code>))`, 'gm'),

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

I propose removing the + after the @ symbol so only one @ is matched.

regex: new RegExp(`[a-zA-Z0-9.!$%&+/=?^\`{|}-]?@${CONST.REG_EXP.EMAIL_PART}(?!((?:(?!<a).)+)?<\\/a>|[^<]*(<\\/pre>|<\\/code>))`, 'gm'),

This is what the regex will capture whenever there are multiple leading @ symbols after this change.

@@@@[email protected]

This behavior is also consistent with what we currently have when there are multiple leading @ symbols for @here mentions.

Demo after changes

expensify-fix-double-at-symbol-bug-demo.mov

What alternative solutions did you explore? (Optional)

None as of yet.

melvin-bot · 2023-10-09T11:28:29Z

@Santhosh-Sellavel Uh oh! This issue is overdue by 2 days. Don't forget to update your issues!

Santhosh-Sellavel · 2023-10-09T20:15:18Z

Thanks, @akamefi202 @Victor-Nyagudi Both of your proposals work.

I like your @Victor-Nyagudi as its cleaner and does the job. Wondering why + was added in the first place, are we missing something?

Victor-Nyagudi · 2023-10-09T20:56:33Z

I'm not sure either, @Santhosh-Sellavel.

I looked through the merged PRs dealing with mentions, and I didn't find any that added the +. It's possible it may have always been there from the beginning.

EDIT: Specified what the PRs I looked at dealt with.

melvin-bot · 2023-10-10T18:20:51Z

Triggered auto assignment to @abekkala (Bug), see https://stackoverflow.com/c/expensify/questions/14418 for more details.

melvin-bot · 2023-10-10T18:20:54Z

Bug0 Triage Checklist (Main S/O)

This "bug" occurs on a supported platform (ensure Platforms in OP are ✅)
This bug is not a duplicate report (check E/App issues and #expensify-bugs)
- If it is, comment with a link to the original report, close the issue and add any novel details to the original issue instead
This bug is reproducible using the reproduction steps in the OP. S/O
- If the reproduction steps are clear and you're unable to reproduce the bug, check with the reporter and QA first, then close the issue.
- If the reproduction steps aren't clear and you determine the correct steps, please update the OP.
This issue is filled out as thoroughly and clearly as possible
- Pay special attention to the title, results, platforms where the bug occurs, and if the bug happens on staging/production.
I have reviewed and subscribed to the linked Slack conversation to ensure Slack/Github stay in sync

abekkala · 2023-10-12T13:16:29Z

@Santhosh-Sellavel do you have any further input for the proposals that were provided?

melvin-bot · 2023-10-12T16:00:10Z

📣 It's been a week! Do we have any satisfactory proposals yet? Do we need to adjust the bounty for this issue? 💸

Santhosh-Sellavel · 2023-10-12T19:57:14Z

Asked here for clarifications regarding this proposal.

melvin-bot · 2023-10-16T11:55:33Z

@abekkala, @Santhosh-Sellavel Uh oh! This issue is overdue by 2 days. Don't forget to update your issues!

abekkala · 2023-10-16T18:03:45Z

@getusha can you respond to @Santhosh-Sellavel question

Asked here for clarifications regarding this #28946 (comment).

Victor-Nyagudi · 2023-10-18T07:00:05Z

@abekkala They responded.

melvin-bot · 2023-12-21T05:13:13Z

Reviewing label has been removed, please complete the "BugZero Checklist".

melvin-bot · 2023-12-21T05:13:18Z

The solution for this issue has been 🚀 deployed to production 🚀 in version 1.4.14-6 and is now subject to a 7-day regression period 📆. Here is the list of pull requests that resolve this issue:

Update expensify-common commit hash in package.json #33142

If no regressions arise, payment will be issued on 2023-12-28. 🎊

After the hold period is over and BZ checklist items are completed, please complete any of the applicable payments for this issue, and check them off once done.

External issue reporter
Contributor that fixed the issue
Contributor+ that helped on the issue and/or PR

For reference, here are some details about the assignees on this issue:

@Victor-Nyagudi requires payment automatic offer (Contributor)
@Santhosh-Sellavel requires payment through NewDot Manual Requests
@adeel0202 requires payment (Needs manual offer from BZ)

melvin-bot · 2023-12-21T05:13:19Z

BugZero Checklist: The PR fixing this issue has been merged! The following checklist (instructions) will need to be completed before the issue can be closed:

[@Santhosh-Sellavel] The PR that introduced the bug has been identified. Link to the PR:
[@Santhosh-Sellavel] The offending PR has been commented on, pointing out the bug it caused and why, so the author and reviewers can learn from the mistake. Link to comment:
[@Santhosh-Sellavel] A discussion in #expensify-bugs has been started about whether any other steps should be taken (e.g. updating the PR review checklist) in order to catch this type of bug sooner. Link to discussion:
[@Santhosh-Sellavel] Determine if we should create a regression test for this bug.
[@Santhosh-Sellavel] If we decide to create a regression test for the bug, please propose the regression test steps to ensure the same bug will not reach production again.
[@abekkala / @kadiealexander] Link the GH issue for creating/updating the regression test once above steps have been agreed upon:

melvin-bot · 2024-01-01T11:16:25Z

@amyevans, @abekkala, @kadiealexander, @Victor-Nyagudi, @Santhosh-Sellavel Eep! 4 days overdue now. Issues have feelings too...

melvin-bot · 2024-01-01T17:12:47Z

@amyevans, @abekkala, @kadiealexander, @Victor-Nyagudi, @Santhosh-Sellavel Huh... This is 4 days overdue. Who can take care of this?

Santhosh-Sellavel · 2024-01-01T18:56:42Z

BugZero Checklist: The PR fixing this issue has been merged! The following checklist (instructions) will need to be completed before the issue can be closed:

[@Santhosh-Sellavel] The PR that introduced the bug has been identified. Link to the PR: feat: add rules to recognize user and @here mentions expensify-common#524
[@Santhosh-Sellavel] The offending PR has been commented on, pointing out the bug it caused and why, so the author and reviewers can learn from the mistake. Link to comment: https://github.com/Expensify/expensify-common/pull/524/files#r1357339778
[@Santhosh-Sellavel] A discussion in #expensify-bugs has been started about whether any other steps should be taken (e.g. updating the PR review checklist) in order to catch this type of bug sooner. Link to discussion: N/A
[@Santhosh-Sellavel] Determine if we should create a regression test for this bug. Yes
[@Santhosh-Sellavel] If we decide to create a regression test for the bug, please propose the regression test steps to ensure the same bug will not reach production again. [HOLD for payment 2023-12-28] [$500] Mentioning an email with double @ sign results in an invalid email mention #28946 (comment)
[@abekkala / @kadiealexander] Link the GH issue for creating/updating the regression test once above steps have been agreed upon: https://github.com/Expensify/Expensify/issues/356530

Santhosh-Sellavel · 2024-01-01T18:59:53Z

Bug: Mentioning an email with double @ sign results in an invalid email mention

Steps:

Log into an account and open a report
Type '@' in the composer followed by the valid email of someone you've chatted with before. You can also optionally select one from the mentions' suggestions list.
Place the cursor at the beginning of the composer, type another '@' sign, and then send the message.
Verify that only the first '@' sign before the email and the email is highlighted e.g. @@[email protected].
Repeat step 2 and then send the message.
Verify that a regular mention with one '@' sign works as usual.

abekkala · 2024-01-03T14:25:48Z

I'm back from ooo, unassigning @kadiealexander and creating regression test GH and payment today

abekkala · 2024-01-03T17:39:32Z

@Victor-Nyagudi payment sent and contract ended - thank you! 🎉

abekkala · 2024-01-03T17:40:00Z

@adeel0202 payment offer sent in Upwork - once accepted, I can process payment

abekkala · 2024-01-03T17:41:02Z

@Santhosh-Sellavel - [$500] to be made via New Dot

adeel0202 · 2024-01-03T17:58:28Z

@adeel0202 payment offer sent in Upwork - once accepted, I can process payment

Thanks. I've accepted.

abekkala · 2024-01-03T21:45:27Z

@adeel0202 payment sent and contract ended - thank you! 🎉

abekkala · 2024-01-08T14:24:34Z

@Santhosh-Sellavel have you requested/received your payment in NewDot?

Santhosh-Sellavel · 2024-01-10T17:29:09Z

Requested on ND

abekkala · 2024-01-11T20:17:38Z

PAYMENTS:

Muhammad Adeel Nawaz paid ✅
Victor-Nyagudi paid ✅

Santosh - requested payment [$500] in ND

JmillsExpensify · 2024-01-14T08:57:48Z

$500 payment approved for @Santhosh-Sellavel based on this comment.

m-natarajan added the External Added to denote the issue can be worked on by a contributor label Oct 5, 2023

melvin-bot bot changed the title ~~Mentioning an email with double @ sign results in an invalid email mention~~ [$500] Mentioning an email with double @ sign results in an invalid email mention Oct 5, 2023

melvin-bot bot added the Help Wanted Apply this label when an issue is open to proposals by contributors label Oct 5, 2023

melvin-bot bot assigned Santhosh-Sellavel Oct 5, 2023

melvin-bot bot added the Daily KSv2 label Oct 5, 2023

melvin-bot bot added the Overdue label Oct 9, 2023

melvin-bot bot removed the Overdue label Oct 9, 2023

m-natarajan added the Bug Something is broken. Auto assigns a BugZero manager. label Oct 10, 2023

melvin-bot bot assigned abekkala Oct 10, 2023

melvin-bot bot added the Overdue label Oct 12, 2023

melvin-bot bot removed the Overdue label Oct 12, 2023

Santhosh-Sellavel mentioned this issue Oct 12, 2023

feat: add rules to recognize user and @here mentions Expensify/expensify-common#524

Closed

melvin-bot bot added the Overdue label Oct 16, 2023

melvin-bot bot removed the Overdue label Oct 16, 2023

melvin-bot bot added Weekly KSv2 Awaiting Payment Auto-added when associated PR is deployed to production and removed Daily KSv2 labels Dec 21, 2023

melvin-bot bot changed the title ~~[$500] Mentioning an email with double @ sign results in an invalid email mention~~ [HOLD for payment 2023-12-28] [$500] Mentioning an email with double @ sign results in an invalid email mention Dec 21, 2023

melvin-bot bot removed the Reviewing Has a PR in review label Dec 21, 2023

melvin-bot bot added Daily KSv2 Overdue and removed Weekly KSv2 labels Dec 27, 2023

melvin-bot bot removed the Overdue label Jan 1, 2024

abekkala unassigned kadiealexander Jan 3, 2024

melvin-bot bot added the Overdue label Jan 8, 2024

melvin-bot bot removed the Overdue label Jan 8, 2024

abekkala closed this as completed Jan 11, 2024

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[HOLD for payment 2023-12-28] [$500] Mentioning an email with double @ sign results in an invalid email mention #28946

[HOLD for payment 2023-12-28] [$500] Mentioning an email with double @ sign results in an invalid email mention #28946

m-natarajan commented Oct 5, 2023 •

edited by melvin-bot bot

Loading

melvin-bot bot commented Oct 5, 2023

melvin-bot bot commented Oct 5, 2023

abdel-h66 commented Oct 5, 2023 •

edited

Loading

akamefi202 commented Oct 5, 2023 •

edited

Loading

Victor-Nyagudi commented Oct 6, 2023

melvin-bot bot commented Oct 9, 2023

Santhosh-Sellavel commented Oct 9, 2023

Victor-Nyagudi commented Oct 9, 2023 •

edited

Loading

melvin-bot bot commented Oct 10, 2023

melvin-bot bot commented Oct 10, 2023

abekkala commented Oct 12, 2023

melvin-bot bot commented Oct 12, 2023

Santhosh-Sellavel commented Oct 12, 2023

melvin-bot bot commented Oct 16, 2023

abekkala commented Oct 16, 2023

Victor-Nyagudi commented Oct 18, 2023

melvin-bot bot commented Dec 21, 2023

melvin-bot bot commented Dec 21, 2023

melvin-bot bot commented Dec 21, 2023

melvin-bot bot commented Jan 1, 2024

melvin-bot bot commented Jan 1, 2024

Santhosh-Sellavel commented Jan 1, 2024 •

edited by abekkala

Loading

Santhosh-Sellavel commented Jan 1, 2024

abekkala commented Jan 3, 2024

abekkala commented Jan 3, 2024

abekkala commented Jan 3, 2024 •

edited

Loading

abekkala commented Jan 3, 2024

adeel0202 commented Jan 3, 2024

abekkala commented Jan 3, 2024

abekkala commented Jan 8, 2024

Santhosh-Sellavel commented Jan 10, 2024

abekkala commented Jan 11, 2024

JmillsExpensify commented Jan 14, 2024

[HOLD for payment 2023-12-28] [$500] Mentioning an email with double @ sign results in an invalid email mention #28946

[HOLD for payment 2023-12-28] [$500] Mentioning an email with double @ sign results in an invalid email mention #28946

Comments

m-natarajan commented Oct 5, 2023 • edited by melvin-bot bot Loading

Action Performed:

Expected Result:

Actual Result:

Workaround:

Platforms:

melvin-bot bot commented Oct 5, 2023

melvin-bot bot commented Oct 5, 2023

abdel-h66 commented Oct 5, 2023 • edited Loading

Proposal

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

What is the root cause of that problem?

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

What alternative solutions did you explore? (Optional)

akamefi202 commented Oct 5, 2023 • edited Loading

Proposal

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

What is the root cause of that problem?

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

What alternative solutions did you explore? (Optional)

Victor-Nyagudi commented Oct 6, 2023

Proposal

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

What is the root cause of that problem?

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

What alternative solutions did you explore? (Optional)

melvin-bot bot commented Oct 9, 2023

Santhosh-Sellavel commented Oct 9, 2023

Victor-Nyagudi commented Oct 9, 2023 • edited Loading

melvin-bot bot commented Oct 10, 2023

melvin-bot bot commented Oct 10, 2023

Bug0 Triage Checklist (Main S/O)

abekkala commented Oct 12, 2023

melvin-bot bot commented Oct 12, 2023

Santhosh-Sellavel commented Oct 12, 2023

melvin-bot bot commented Oct 16, 2023

abekkala commented Oct 16, 2023

Victor-Nyagudi commented Oct 18, 2023

melvin-bot bot commented Dec 21, 2023

melvin-bot bot commented Dec 21, 2023

melvin-bot bot commented Dec 21, 2023

melvin-bot bot commented Jan 1, 2024

melvin-bot bot commented Jan 1, 2024

Santhosh-Sellavel commented Jan 1, 2024 • edited by abekkala Loading

Santhosh-Sellavel commented Jan 1, 2024

Steps:

abekkala commented Jan 3, 2024

abekkala commented Jan 3, 2024

abekkala commented Jan 3, 2024 • edited Loading

abekkala commented Jan 3, 2024

adeel0202 commented Jan 3, 2024

abekkala commented Jan 3, 2024

abekkala commented Jan 8, 2024

Santhosh-Sellavel commented Jan 10, 2024

abekkala commented Jan 11, 2024

PAYMENTS:

JmillsExpensify commented Jan 14, 2024

m-natarajan commented Oct 5, 2023 •

edited by melvin-bot bot

Loading

abdel-h66 commented Oct 5, 2023 •

edited

Loading

akamefi202 commented Oct 5, 2023 •

edited

Loading

Victor-Nyagudi commented Oct 9, 2023 •

edited

Loading

Santhosh-Sellavel commented Jan 1, 2024 •

edited by abekkala

Loading

abekkala commented Jan 3, 2024 •

edited

Loading