Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[$500] Mention disappears when including 's #50724

Open
1 of 6 tasks
m-natarajan opened this issue Oct 14, 2024 · 105 comments
Open
1 of 6 tasks

[$500] Mention disappears when including 's #50724

m-natarajan opened this issue Oct 14, 2024 · 105 comments
Assignees
Labels
Bug Something is broken. Auto assigns a BugZero manager. External Added to denote the issue can be worked on by a contributor Help Wanted Apply this label when an issue is open to proposals by contributors Weekly KSv2

Comments

@m-natarajan
Copy link

m-natarajan commented Oct 14, 2024

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
  • MacOS: Chrome / Safari
  • MacOS: Desktop

Screenshots/Videos

Recording.654.mp4
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: 2025-03-11
Issue OwnerCurrent Issue Owner: @shubham1206agra
@m-natarajan m-natarajan added Daily KSv2 Bug Something is broken. Auto assigns a BugZero manager. labels Oct 14, 2024
Copy link

melvin-bot bot commented Oct 14, 2024

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.

@flodnv flodnv added the External Added to denote the issue can be worked on by a contributor label Oct 14, 2024
@melvin-bot melvin-bot bot changed the title Mention disappears when including 's [$250] Mention disappears when including 's Oct 14, 2024
Copy link

melvin-bot bot commented Oct 14, 2024

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

@melvin-bot melvin-bot bot added the Help Wanted Apply this label when an issue is open to proposals by contributors label Oct 14, 2024
Copy link

melvin-bot bot commented Oct 14, 2024

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

@bernhardoj
Copy link
Contributor

bernhardoj commented Oct 14, 2024

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 '.

App/src/CONST.ts

Line 2754 in 0bb5806

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

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.

App/src/libs/ReportUtils.ts

Lines 4176 to 4178 in 0bb5806

const mention = match.substring(1);
const mentionWithDomain = addDomainToShortMention(mention);
return mentionWithDomain ? `@${mentionWithDomain}` : match;

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.

App/src/CONST.ts

Line 2754 in 0bb5806

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

@flodnv
Copy link
Contributor

flodnv commented Oct 14, 2024

What if the short mention has an ' in it?

@mallenexpensify
Copy link
Contributor

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
Copy link
Contributor

@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
Copy link

shoibsq commented Oct 15, 2024

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.

Copy link

melvin-bot bot commented Oct 17, 2024

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

@melvin-bot melvin-bot bot added the Overdue label Oct 17, 2024
@shubham1206agra
Copy link
Contributor

shubham1206agra commented Oct 19, 2024

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

@melvin-bot melvin-bot bot removed the Overdue label Oct 19, 2024
@bernhardoj
Copy link
Contributor

Does Slack have a short mention?

Copy link

melvin-bot bot commented Oct 21, 2024

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

@melvin-bot melvin-bot bot added the Overdue label Oct 21, 2024
@mallenexpensify
Copy link
Contributor

mallenexpensify commented Oct 22, 2024

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

2024-10-21_17-32-31.mp4

Copy link

melvin-bot bot commented Oct 22, 2024

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

Copy link

melvin-bot bot commented Oct 24, 2024

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

Copy link

melvin-bot bot commented Oct 28, 2024

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

Copy link

melvin-bot bot commented Oct 28, 2024

@mallenexpensify @shubham1206agra this issue was created 2 weeks ago. Are we close to approving a proposal? If not, what's blocking us from getting this issue assigned? Don't hesitate to create a thread in #expensify-open-source to align faster in real time. Thanks!

Copy link

melvin-bot bot commented Oct 28, 2024

@mallenexpensify, @shubham1206agra Now this issue is 8 days overdue. Are you sure this should be a Daily? Feel free to change it!

@mallenexpensify
Copy link
Contributor

Added to #quality project, assuming that user's would expect this to work (I do think adding 's after a user's handle, ie. @flodnv's , is common (as in Slack and GH). Gonna bump to $500 to get more 👀 and proposals.

Copy link

melvin-bot bot commented Jan 8, 2025

@mallenexpensify, @shubham1206agra 12 days overdue now... This issue's end is nigh!

@mallenexpensify
Copy link
Contributor

@melvin-bot melvin-bot bot added Weekly KSv2 and removed Daily KSv2 labels Jan 13, 2025
Copy link

melvin-bot bot commented Jan 13, 2025

This issue has not been updated in over 14 days. @mallenexpensify, @shubham1206agra eroding to Weekly issue.

@melvin-bot melvin-bot bot removed the Overdue label Jan 13, 2025
Copy link

melvin-bot bot commented Jan 14, 2025

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

1 similar comment
Copy link

melvin-bot bot commented Jan 21, 2025

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

@melvin-bot melvin-bot bot added the Overdue label Jan 21, 2025
Copy link

melvin-bot bot commented Jan 28, 2025

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

@mallenexpensify
Copy link
Contributor

@shubham1206agra did you want to purchase a domain to use for testing and we'll reimburse you for it?

@shubham1206agra
Copy link
Contributor

@mallenexpensify Actually I posted a thread some time back here https://expensify.slack.com/archives/C01GTK53T8Q/p1737029721455129

@melvin-bot melvin-bot bot removed the Overdue label Jan 29, 2025
@mallenexpensify
Copy link
Contributor

d'oh, thanks for sharing the link, def missed it before

I have also discussed this issue with @Kicu in length, and the best solution for now is to get short mention running when ' is present (might point to wrong user, but it is better to point to no one at all, which is the current behavior).

Is that how Slack does it? Appears to be

Image

In GH, once you add the ` then no selection shows.

I think the more import part of the bug is that when `s is added at the end of a tagged name, the handle still shows, vs not.

Copy link

melvin-bot bot commented Feb 4, 2025

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

@NJ-2020
Copy link
Contributor

NJ-2020 commented Feb 11, 2025

Any updates?

cc: @shubham1206agra

@melvin-bot melvin-bot bot added the Overdue label Feb 11, 2025
Copy link

melvin-bot bot commented Feb 11, 2025

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

1 similar comment
Copy link

melvin-bot bot commented Feb 18, 2025

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

@NJ-2020
Copy link
Contributor

NJ-2020 commented Feb 19, 2025

Any updates?

@shubham1206agra
Copy link
Contributor

@NJ-2020 Not yet. We are working on Expensify/expensify-common#824 first. Then, we will circle back here.

@melvin-bot melvin-bot bot removed the Overdue label Feb 19, 2025
@NJ-2020
Copy link
Contributor

NJ-2020 commented Feb 19, 2025

Thanks

Copy link

melvin-bot bot commented Feb 25, 2025

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

1 similar comment
Copy link

melvin-bot bot commented Mar 4, 2025

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

@melvin-bot melvin-bot bot added the Overdue label Mar 4, 2025
@mallenexpensify
Copy link
Contributor

@shubham1206agra
Copy link
Contributor

Sorry, we are waiting for version bump in #54037.

@melvin-bot melvin-bot bot removed the Overdue label Mar 5, 2025
Copy link

melvin-bot bot commented Mar 11, 2025

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Bug Something is broken. Auto assigns a BugZero manager. External Added to denote the issue can be worked on by a contributor Help Wanted Apply this label when an issue is open to proposals by contributors Weekly KSv2
Projects
Status: Bugs and Follow Up Issues
Development

No branches or pull requests