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.31k stars 2.74k forks source link

[$250] Android - Room - Whisper displays 1 mention when 2 mentions are send in specific way #46299

Open lanitochka17 opened 1 month ago

lanitochka17 commented 1 month 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.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:

  1. Launch app
  2. Tap on a room chat
  3. Enter #4856₹&_#7553 and sent the message
  4. Note whisper for 2 mentions are shown
  5. Enter #gh4h_+&&&-#ehhh & send the message
  6. 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?

Screenshots/Videos

Add any screenshot/video evidence

https://github.com/user-attachments/assets/30d757d7-8725-47c1-9b9e-6119fba632f2

View all open jobs on GitHub

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 OwnerCurrent Issue Owner: @rojiphil
melvin-bot[bot] commented 1 month ago

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.

lanitochka17 commented 1 month ago

We think that this bug might be related to #vip-vsp

lanitochka17 commented 1 month ago

@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

melvin-bot[bot] commented 1 month ago

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

melvin-bot[bot] commented 1 month ago

@kevinksullivan Eep! 4 days overdue now. Issues have feelings too...

kevinksullivan commented 1 month ago

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

melvin-bot[bot] commented 1 month ago

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

melvin-bot[bot] commented 1 month ago

@kevinksullivan, @lanitochka17 Huh... This is 4 days overdue. Who can take care of this?

melvin-bot[bot] commented 1 month ago

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

melvin-bot[bot] commented 1 month ago

@kevinksullivan, @lanitochka17 Still overdue 6 days?! Let's take care of this!

kevinksullivan commented 1 month ago

Bump @lanitochka17

lanitochka17 commented 3 weeks ago

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

lanitochka17 commented 3 weeks ago

Issue is still reproducible

https://github.com/user-attachments/assets/4c461993-9e30-4063-a30d-ed1090525887

melvin-bot[bot] commented 2 weeks ago

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

melvin-bot[bot] commented 2 weeks ago

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

brad-isbell commented 1 week ago

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:

image

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:

image

these are matched as separate mentions: #gh4h_+&&& and #ehhh.

melvin-bot[bot] commented 1 week ago

📣 @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:

  1. Make sure you've read and understood the contributing guidelines.
  2. 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.
  3. 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.
  4. Copy the format below and paste it in a comment on this issue. Replace the placeholder text with your actual details. Screen Shot 2022-11-16 at 4 42 54 PM Format:
    Contributor details
    Your Expensify account email: <REPLACE EMAIL HERE>
    Upwork Profile Link: <REPLACE LINK HERE>
brad-isbell commented 1 week ago

Contributor details Your Expensify account email: morteprhimi@gmail.com Upwork Profile Link: https://www.upwork.com/freelancers/~01e11da7d5ef514a1c

melvin-bot[bot] commented 1 week ago

⚠️ Invalid email. Please make sure to create an Expensify account with this email first here.

NJ-2020 commented 1 week ago

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 Screenshot 2024-08-27 at 00 20 57

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)

brad-isbell commented 1 week ago

Proposal

Updated

brad-isbell commented 1 week ago

Contributor details Your Expensify account email: morteprhimi@gmail.com Upwork Profile Link: https://www.upwork.com/freelancers/~01e11da7d5ef514a1c

melvin-bot[bot] commented 1 week ago

⚠️ Missing/invalid email or upwork profile link. Please make sure you add both your Expensify email and Upwork profile link in the format specified.

brad-isbell commented 1 week ago

Contributor details Your Expensify account email: mohamediyedrhimita@gmail.com Upwork Profile Link: https://www.upwork.com/freelancers/~01e11da7d5ef514a1c

melvin-bot[bot] commented 1 week ago

✅ Contributor details stored successfully. Thank you for contributing to Expensify!

melvin-bot[bot] commented 1 week ago

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

rojiphil commented 1 week ago

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₹&#7553? 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

NJ-2020 commented 1 week ago

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

melvin-bot[bot] commented 1 week ago

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

melvin-bot[bot] commented 6 days ago

@rojiphil, @kevinksullivan, @lanitochka17 Whoops! This issue is 2 days overdue. Let's get this updated quick!

rojiphil commented 6 days ago

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₹&#7553?

@kevinksullivan I just noticed that @rlinoz is OOO this week. Meanwhile, what do you think of the expected behavior for the above-mentioned comment?

melvin-bot[bot] commented 2 days ago

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

melvin-bot[bot] commented 1 day ago

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