App
App copied to clipboard
Expensify
[$250] Typing `@useraccount` and sending it sends `@[email protected]
If you haven’t already, check out our contributing guidelines for onboarding and email [email protected] to request to join our Slack channel!
Version Number: 1.4.62-5 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: @cead22 Slack conversation: https://expensify.slack.com/archives/C049HHMV9SM/p1712874830762349
Action Performed:
- Open any chat
- Type
@applausetesterand hit send (Make sure to use "`" to make it a codeblock)
Expected Result:
Appears @applausetester after sending
Actual Result:
Appears as @[email protected]
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
Add any screenshot/video evidence
https://github.com/Expensify/App/assets/38435837/b26927cc-37a4-44e0-adb1-86dcf1c170dc
Upwork Automation - Do Not Edit
- Upwork Job URL: https://www.upwork.com/jobs/~0115db0ab43f78ece5
- Upwork Job ID: 1779954988168179712
- Last Price Increase: 2024-04-22
Triggered auto assignment to @MitchExpensify (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] avatar](https://avatars.githubusercontent.com/in/179547?v=4 "Published by a pub.dev verified publisher"){kind=small}
Assigning to VSB as low as this is a chat-centric bug that reveals emails
Job added to Upwork: https://www.upwork.com/jobs/~0115db0ab43f78ece5
Triggered auto assignment to Contributor-plus team member for initial proposal review - @shubham1206agra (External)
I'm not able to reproduce. Any help would be appreciated.
https://github.com/Expensify/App/assets/54620116/ae8e80ee-7ef8-42a5-8045-871b7f174181
@cead22 Can you provide us with a better example, as domains are not the same for everybody?
Proposal
Please re-state the problem that we are trying to solve in this issue.
When we send short hand mention like @applausetester inside backticks, it's appending the private domain into it
Eg. Sending
`@applausetester`
It will appears as @[email protected]
What is the root cause of that problem?
In here, we have a logic where if the current user's private domain is expensify.com, it we tag by short hand, like @tienifr, and there's a [email protected] in the personal details list, we'll convert the @tienifr to @[email protected] so that it will be treated as a mention.
This is to make users in the same organization able to tag each other by just @shubham, @tienifr, not having to type the full email domain.
However, when we're trying to detect short mention here, we're not taking into account cases like if the mention is in code blocks, code fence, links, ... So even the short-hand mention inside codeblock is still appended with the private domain. And subsequently it will not be considered in the mention detection logic in expensify-common, thus leading to this issue.
What changes do you think we should make in order to solve the problem?
If we look at the proper logic to detect mentions in expensify-common here and here, we'll see a mention is only valid if:
- It's non inside links, codeblocks,
pre, ... - It's either proceeded by a space, or start and end with a *, _, ~, ', or "
- And a few minor things
We should apply the same when detecting short mentions, basically the only difference between short mention detection and full mention detection is: short mention doesn't require an email domain, the rest should be the same.
We'll need to:
- Modify the Regex for short mentions (similar to for full mention, but do not that for short mention we need to check markdown signs instead of html elements).
We can update this to
SHORT_MENTION: new RegExp(`@[\\w\\-\\+\\'#@]+(?:\\.[\\w\\-\\'\\+]+)*(?![^\`]*\`)`, 'gim'),
So it will not match if the mention is inside backticks ((?![^\`]*\`) added)
- Add logic to check
isValidMentionsimilar to here
We'll add here
if (!Str.isValidMention(match)) {
return match;
}
So if the matched mention is invalid, we'll not replace anything.
What alternative solutions did you explore? (Optional)
Same solution as above, but we can consider centralize it in expensify-common to be DRY.
@cead22 Can you provide us with a better example, as domains are not the same for everybody?
@shubham1206agra To reproduce this, we need:
- A private domain (if you don't have one, can comment out
'gmail.com',innode_modules/expensify-common/lib/CONST.jsxso that anygmail.comwill become a private domain) - The short mention we're mentioning should be inside
personalDetailsListalready
So even the short-hand mention inside codeblock is still appended with the private domain. And subsequently it will not be considered in the mention detection logic in
expensify-common
@tienifr to clarify, it should not be considered a mention if it's inside backticks, right?
We'll need to:
- Modify the Regex for short mentions (similar to for full mention, but do not that for short mention we need to check markdown signs instead of html elements)
Where/how?
- Add logic to check
isValidMentionsimilar to here
Where/how?
Please re-state the problem that we are trying to solve in this issue.
This looks incomplete
@cead22 @shubham1206agra
@tienifr to clarify, it should not be considered a mention if it's inside backticks, right?
Yes, same as when we put full email mention in the backticks
This looks incomplete
I updated Please re-state the problem that we are trying to solve in this issue. in the proposal to clarify.
For the rest, I will add some details within a day.
📣 It's been a week! Do we have any satisfactory proposals yet? Do we need to adjust the bounty for this issue? 💸
@MitchExpensify, @shubham1206agra Uh oh! This issue is overdue by 2 days. Don't forget to update your issues!
Modify the Regex for short mentions (similar to for full mention, but do not that for short mention we need to check markdown signs instead of html elements).
Add logic to check isValidMention similar to here
@cead22 @shubham1206agra Proposal updated to clarify the 2 points above
I'm about to head OOO for a bit so reassigning and will take back upon my return on the 7th
Triggered auto assignment to @laurenreidexpensify (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.
@MitchExpensify @shubham1206agra @laurenreidexpensify 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!
Triggered auto assignment to @robertjchen, see https://stackoverflow.com/c/expensify/questions/7972 for more details.
📣 @shubham1206agra 🎉 An offer has been automatically sent to your Upwork account for the Reviewer role 🎉 Thanks for contributing to the Expensify app!
📣 @tienifr 🎉 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 📖
This issue has not been updated in over 15 days. @robertjchen, @MitchExpensify, @shubham1206agra, @laurenreidexpensify, @tienifr eroding to Monthly issue.
P.S. Is everyone reading this sure this is really a near-term priority? Be brave: if you disagree, go ahead and close it out. If someone disagrees, they'll reopen it, and if they don't: one less thing to do!
@MitchExpensify looks like the payment job on this failed but it's on prod now so should be good to pay on Thurs. Handing back over to you!