Expensify
[$250] Android - Room - Whisper displays 1 mention when 2 mentions are send in specific way
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: 9.0.12 Reproducible in staging?: Y Reproducible in production?: Y If this was caught during regression testing, add the test name, ID and link from TestRail: https://expensify.testrail.io/index.php?/tests/view/4768836 Issue reported by: Applause - Internal Team
Action Performed:
- Launch app
- Tap on a room chat
- Enter #4856₹&_#7553 and sent the message
- Note whisper for 2 mentions are shown
- Enter #gh4h_+&&&-#ehhh & send the message
- Note whisper for only one mention is shown
Expected Result:
Whisper must display 2 mention when 2 mentions are send
Actual Result:
Whisper displays 1 mention when 2 mentions are send in specific way.
Workaround:
Unknown
Platforms:
Which of our officially supported platforms is this issue occurring on?
- [x] Android: Native
- [x] Android: mWeb Chrome
- [ ] iOS: Native
- [ ] iOS: mWeb Safari
- [ ] MacOS: Chrome / Safari
- [ ] MacOS: Desktop
Screenshots/Videos
Add any screenshot/video evidence
https://github.com/user-attachments/assets/30d757d7-8725-47c1-9b9e-6119fba632f2
Upwork Automation - Do Not Edit
- Upwork Job URL: https://www.upwork.com/jobs/~01903b4afb9ff494db
- Upwork Job ID: 1827453197366571253
- Last Price Increase: 2024-09-07
Issue Owner
Current Issue Owner: @rojiphil
Triggered auto assignment to @kevinksullivan (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}
@kevinksullivan FYI I haven't added the External label as I wasn't 100% sure about this issue. Please take a look and add the label if you agree it's a bug and can be handled by external contributors
@kevinksullivan Whoops! This issue is 2 days overdue. Let's get this updated quick!
@kevinksullivan Eep! 4 days overdue now. Issues have feelings too...
@lanitochka17 does this happen with other combinations of room references? I'm sort of confused because both strings are just one line with no spaces, so I am not sure what is supposed to happen here.
@kevinksullivan, @lanitochka17 Uh oh! This issue is overdue by 2 days. Don't forget to update your issues!
@kevinksullivan, @lanitochka17 Huh... This is 4 days overdue. Who can take care of this?
@kevinksullivan @lanitochka17 this issue was created 2 weeks ago. Are we close to a solution? Let's make sure we're treating this as a top priority. Don't hesitate to create a thread in #expensify-open-source to align faster in real time. Thanks!
@kevinksullivan, @lanitochka17 Still overdue 6 days?! Let's take care of this!
@kevinksullivan According steps in TR https://expensify.testrail.io/index.php?/tests/view/4768836 step 5 is fail when create whisper with new room. Expected resault: Verify an actionable whisper asking the user if they want to create the room is triggered.
Issue is still reproducible
https://github.com/user-attachments/assets/4c461993-9e30-4063-a30d-ed1090525887
Job added to Upwork: https://www.upwork.com/jobs/~01903b4afb9ff494db
Triggered auto assignment to Contributor-plus team member for initial proposal review - @rojiphil (External)
Edited by proposal-police: This proposal was edited at 2023-10-05T13:46:00Z.
Proposal
Please re-state the problem that we are trying to solve in this issue.
Whisper displays 1 mention when 2 mentions are send in specific way.
What is the root cause of that problem?
the issue is in this regular expression. It restricts the characters after the # to lowercase letters (\p{Ll}), digits (0-9), and hyphens (-) (eg: this group(#[\p{Ll}0-9-]{1,80})). This causes it to stop matching when it encounters characters like _, + or &.
What changes do you think we should make in order to solve the problem?
i included those characters (_, +, &) into the old regular expression, so it becomes:
(?<![^ \n*~_])(#[\p{Ll}0-9_+\-&]{1,80})(?![^<]*(?:<\/pre>|<\/code>|<\/a>))
Result:
the "#gh4h_+&&&-" is matched.
What alternative solutions did you explore? (Optional)
if you want to match mentions separated using the dash character "-" this regex will serve the need :
(?<![^ \n*~_-])(#[\p{Ll}0-9_+&-]{1,80}[^-#])(?=|[^<]*(?:<\/pre>|<\/code>|<\/a>))
Result:
these are matched as separate mentions: #gh4h_+&&& and #ehhh.
📣 @Mohamed-iyed-ta! 📣 Hey, it seems we don’t have your contributor details yet! You'll only have to do this once, and this is how we'll hire you on Upwork. Please follow these steps:
- Make sure you've read and understood the contributing guidelines.
- Get the email address used to login to your Expensify account. If you don't already have an Expensify account, create one here. If you have multiple accounts (e.g. one for testing), please use your main account email.
- Get the link to your Upwork profile. It's necessary because we only pay via Upwork. You can access it by logging in, and then clicking on your name. It'll look like this. If you don't already have an account, sign up for one here.
- Copy the format below and paste it in a comment on this issue. Replace the placeholder text with your actual details.
Format:
Contributor details
Your Expensify account email: <REPLACE EMAIL HERE>
Upwork Profile Link: <REPLACE LINK HERE>
Contributor details Your Expensify account email: [email protected] Upwork Profile Link: https://www.upwork.com/freelancers/~01e11da7d5ef514a1c
Proposal
Please re-state the problem that we are trying to solve in this issue.
Android - Room - Whisper displays 1 mention when 2 mentions are send in specific way
What is the root cause of that problem?
When we type #gh4h_+&&&-#ehhh we mention 2 user by splitting with minus -, but in our expensify-common we do not include the - minus we only include the underscore _
https://github.com/Expensify/expensify-common/blob/d11466167bc562447d2dfdad624145040153efae/lib/ExpensiMark.ts#L298-L303
So when we add the comment the reportComment params is not correct
What changes do you think we should make in order to solve the problem?
We can add minus - to the regex
From:
/(?<![^ \n*~_])(#[\p{Ll}0-9-]{1,80})(?![^<]*(?:<\/pre>|<\/code>|<\/a>))/gimu
To:
/(?<![^ \n*~_-])(#[\p{Ll}0-9-]{1,80})(?![^<]*(?:<\/pre>|<\/code>|<\/a>))/gimu
What alternative solutions did you explore? (Optional)
Contributor details Your Expensify account email: [email protected] Upwork Profile Link: https://www.upwork.com/freelancers/~01e11da7d5ef514a1c
⚠️ Missing/invalid email or upwork profile link. Please make sure you add both your Expensify email and Upwork profile link in the format specified.
Contributor details Your Expensify account email: [email protected] Upwork Profile Link: https://www.upwork.com/freelancers/~01e11da7d5ef514a1c
✅ Contributor details stored successfully. Thank you for contributing to Expensify!
@rojiphil, @kevinksullivan, @lanitochka17 Uh oh! This issue is overdue by 2 days. Don't forget to update your issues!
Well! This seems tricky to handle. If the expectation from message #gh4h_+&&&-#ehhh is to identify #gh4h and #ehhh, then would it not be fair to expect the same output for another message #4856₹ᶁ?
So, it seems like while the specific issue here can be resolved by adding - to negative lookbehind, maybe we need to find a holistic solution to this problem.
Tagging @rlinoz to weigh-in here on the expected output due to involvement in the mentions-v2 here
Also we don't need to update/fix the react-native-live-markdown because it already synced with the expensify-common, we just need to update to the latest version of the expensify-common inside the react-native-live-markdown and build it again
📣 It's been a week! Do we have any satisfactory proposals yet? Do we need to adjust the bounty for this issue? 💸
@rojiphil, @kevinksullivan, @lanitochka17 Whoops! This issue is 2 days overdue. Let's get this updated quick!