Expensify / App

Welcome to New Expensify: a complete re-imagination of financial collaboration, centered around chat. Help us build the next generation of Expensify by sharing feedback and contributing to the code.
https://new.expensify.com
MIT License
3.5k stars 2.85k forks source link

[$250] Mention disappears when including `'s` #50724

Open m-natarajan opened 1 week ago

m-natarajan commented 1 week ago

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.48-0 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 Expensify/Expensify Issue URL: Issue reported by: @flodnv Slack conversation: https://expensify.slack.com/archives/C049HHMV9SM/p1728591305161159

Action Performed:

  1. Type something @user in a room (the mention must be the short version without the email part)
  2. This inserts something @user with a trailing space
  3. Remove the trailing space with the backspace key, add a 's

    Expected Result:

    The @mentioned user is highlighted in the sent message as something @user's

    Actual Result:

    The @user part is not marked as a mention and message is sent as plain text

    Workaround:

    Unknown

    Platforms:

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

    • [ ] Android: Native
    • [ ] Android: mWeb Chrome
    • [ ] iOS: Native
    • [ ] iOS: mWeb Safari
    • [x] MacOS: Chrome / Safari
    • [ ] MacOS: Desktop

Screenshots/Videos

https://github.com/user-attachments/assets/a76f0845-7388-4e7b-8ffe-0318f6d85ebc

Add any screenshot/video evidence

View all open jobs on GitHub

Upwork Automation - Do Not Edit
  • Upwork Job URL: https://www.upwork.com/jobs/~021845828693936391795
  • Upwork Job ID: 1845828693936391795
  • Last Price Increase: 2024-10-21
Issue OwnerCurrent Issue Owner: @shubham1206agra
melvin-bot[bot] commented 1 week ago

Triggered auto assignment to @mallenexpensify (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[bot] commented 1 week ago

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

melvin-bot[bot] commented 1 week ago

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

bernhardoj commented 1 week ago

Edited by proposal-police: This proposal was edited at 2024-10-14 14:27:47 UTC.

Proposal

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

Mention doesn't work when the mention has '.

What is the root cause of that problem?

Looks like this only happens on short mentions. When we do a complete mention, for example, @bernhard.josephus@gmail.com's, only @bernhard.josephus@gmail.com is matched as the mention, and 's is treated as a normal text.

But when we do short mentions, for example, @bernhard.josephus's, all text is matched as a mention. That's because the regex accepts '. https://github.com/Expensify/App/blob/0bb5806d60a69303b6d37ebde4d23450714c2ce2/src/CONST.ts#L2754

Based on this SO, ' is a valid character for an email before the @ character.

And because there is no contact with an email that contains ', the full mention can't be built. https://github.com/Expensify/App/blob/0bb5806d60a69303b6d37ebde4d23450714c2ce2/src/libs/ReportUtils.ts#L4176-L4178

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

If we don't want to accept ' as part of the short mention, then we can remove it from the regex. https://github.com/Expensify/App/blob/0bb5806d60a69303b6d37ebde4d23450714c2ce2/src/CONST.ts#L2754

flodnv commented 1 week ago

What if the short mention has an ' in it?

mallenexpensify commented 1 week ago

Was able to reproduce, i wonder if this is a bug or new feature. Meaning... was this ever discussed before, where we decide NOT to tag a user with 's at the end of their handle. (guessing not, just seems like it's worked this way from the start)

bernhardoj commented 1 week ago

@flodnv if the email has ' in it, then it won't be converted to a mention. The user needs to type the full mention. But I just tested that our full user mention regex can't match either if there is ' in it.

image
shoibsq commented 1 week ago

Proposal

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

Mention is not working with possessive forms like 's

What is the root cause of that problem?

Regex is not handling Possessive forms

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

Updating Regex to handle Apostrophes, Email Mentions, and Possessives.

SHORT_MENTION: new RegExp(`@[\\w\\-\\+#']+(?:\\.[\\w\\-\\+#']+)*(?:@[\\w\\-]+\\.[a-z]{2,})?(?![^\`]*\`)(?=\\b|'s)`, 'gim'),

Details on updating regex :

  1. Handling Apostrophes in Mentions: @[\w\-\+#']+ allows apostrophes (') inside mentions.
  2. Handling Dot-separated Parts: (?:\.[\w\-\+#']+)* matches additional parts like @user.name.
  3. Handling Email Mentions: (?:@[\w\-]+\.[a-z]{2,})? allows optional email-style mentions like @bernha'ard.josephus@gmail.com.
  4. Handling Word Boundary and Possessive 's: The lookahead (?=\b|'s) ensures that mentions are valid when followed by a word boundary (space or punctuation) or possessive 's.
melvin-bot[bot] commented 1 week ago

@mallenexpensify, @shubham1206agra Uh oh! This issue is overdue by 2 days. Don't forget to update your issues!

shubham1206agra commented 1 week ago

@mallenexpensify I don't think we can omit ' from the short mention check. We must find another solution to this problem since slack has no problem handling it.

bernhardoj commented 1 week ago

Does Slack have a short mention?

melvin-bot[bot] commented 5 days ago

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

mallenexpensify commented 4 days ago

I'm uncertain of the Slack's logic or exactly how they handle it, check the vid below

https://github.com/user-attachments/assets/7f911179-ff79-4bd3-bcca-f3835fe8443a

melvin-bot[bot] commented 4 days ago

@mallenexpensify, @shubham1206agra Whoops! This issue is 2 days overdue. Let's get this updated quick!

melvin-bot[bot] commented 2 days ago

@mallenexpensify, @shubham1206agra Eep! 4 days overdue now. Issues have feelings too...