Skip to content

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

Closed

Description

@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!


Action Performed:

  1. Open any chat
  2. Type @ and select any email from suggestions
  3. Move the cursor to the beginning
  4. Add another @ sign
  5. Hit the send button
  6. 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?

  • Android / native
  • Android / Chrome
  • iOS / native
  • iOS / Safari
  • MacOS / Chrome / Safari
  • MacOS / Desktop

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

Activity

  1. added
    ExternalAdded to denote the issue can be worked on by a contributor
    on Oct 5, 2023
  2. 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[/+] on Oct 5, 2023
  3. melvin-bot commented on Oct 5, 2023

    @melvin-bot
  4. added
    Help WantedApply this label when an issue is open to proposals by contributors
    on Oct 5, 2023
  5. melvin-bot commented on Oct 5, 2023

    @melvin-bot

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

  6. abdel-h66 commented on Oct 5, 2023

    @abdel-h66
    Contributor

    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

    } 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

  7. akamefi202 commented on Oct 5, 2023

    @akamefi202
    Contributor

    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.

    @@test@test.com => <mention-user>@@test@test.com</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.

    @@test@test.com => @<mention-user>@test@test.com</mention-user>
    

    What alternative solutions did you explore? (Optional)

    N/A

  8. Victor-Nyagudi commented on Oct 6, 2023

    @Victor-Nyagudi
    Contributor

    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.

    @@@@johnsmith@gmail.com

    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.

  9. melvin-bot commented on Oct 9, 2023

    @melvin-bot

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

  10. Santhosh-Sellavel commented on Oct 9, 2023

    @Santhosh-Sellavel
    Collaborator

    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?

  11. Victor-Nyagudi commented on Oct 9, 2023

    @Victor-Nyagudi
    Contributor

    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.

  12. 116 remaining items

  13. Santhosh-Sellavel commented on Jan 1, 2024

    @Santhosh-Sellavel
    Collaborator

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

    Steps:

    1. Log into an account and open a report
    2. 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.
    3. Place the cursor at the beginning of the composer, type another '@' sign, and then send the message.
    4. Verify that only the first '@' sign before the email and the email is highlighted e.g. @@user@expensify.com.
    5. Repeat step 2 and then send the message.
    6. Verify that a regular mention with one '@' sign works as usual.
  14. abekkala commented on Jan 3, 2024

    @abekkala
    Contributor

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

  15. abekkala commented on Jan 3, 2024

    @abekkala
    Contributor

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

  16. abekkala commented on Jan 3, 2024

    @abekkala
    Contributor

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

  17. abekkala commented on Jan 3, 2024

    @abekkala
    Contributor

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

  18. adeel0202 commented on Jan 3, 2024

    @adeel0202
    Contributor

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

    Thanks. I've accepted.

  19. abekkala commented on Jan 3, 2024

    @abekkala
    Contributor

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

  20. abekkala commented on Jan 8, 2024

    @abekkala
    Contributor

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

  21. Santhosh-Sellavel commented on Jan 10, 2024

    @Santhosh-Sellavel
    Collaborator

    Requested on ND

  22. abekkala commented on Jan 11, 2024

    @abekkala
    Contributor

    PAYMENTS:

    Muhammad Adeel Nawaz paid ✅
    Victor-Nyagudi paid ✅

    Santosh - requested payment [$500] in ND

  23. JmillsExpensify commented on Jan 14, 2024

    @JmillsExpensify
    Contributor

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Labels

Awaiting PaymentAuto-added when associated PR is deployed to productionBugSomething is broken. Auto assigns a BugZero manager.DailyKSv2ExternalAdded to denote the issue can be worked on by a contributor

Type

No type

Projects

No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions