[$1000] Mentioned list is not showing with display name with & in between

[kavimuru]

If you haven’t already, check out our contributing guidelines for onboarding and email contributors@expensify.com to request to join our Slack channel!

---

Action Performed:

Precondition: Please change the display name of user B to anything but should have & in the display name

1. Go to staging dot on web and login with User A
2. Go to any chat and try to mention user B by FULL display name with &
3. Notice that if the display name contains & , the mention suggestion will not be shown

Expected Result:

Mentioned list should be shown with display name having & in between

Actual Result:

Mentioned list is not showing with display name with & in between

Workaround:

Can the user still use Expensify without this being fixed? Have you informed them of the workaround?

Platforms:

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

• Android / native
• Android / Chrome
• iOS / native
• iOS / Safari
• MacOS / Chrome / Safari
• MacOS / Desktop

Version Number: 1.3.37-2
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
Notes/Photos/Videos: Any additional supporting documentation

error-2023-07-05_22.08.23.mp4

Recording.1187.mp4

Expensify/Expensify Issue URL:
Issue reported by: @priya-zha
Slack conversation: https://expensify.slack.com/archives/C049HHMV9SM/p1688574053007549

View all open jobs on GitHub

Upwork Automation - Do Not Edit

• Upwork Job URL: https://www.upwork.com/jobs/~01b5bdd04db4b9c2a1
• Upwork Job ID: 1677163730809729024
• Last Price Increase: 2023-07-21

[melvin-bot]

Triggered auto assignment to @kadiealexander (Bug), see https://stackoverflow.com/c/expensify/questions/14418 for more details.

[melvin-bot]

Bug0 Triage Checklist (Main S/O)

• This "bug" occurs on a supported platform (ensure Platforms in OP are ✅)
• This bug is not a duplicate report (check E/App issues and #expensify-bugs)
  • If it is, comment with a link to the original report, close the issue and add any novel details to the original issue instead
• This bug is reproducible using the reproduction steps in the OP. S/O
  • If the reproduction steps are clear and you're unable to reproduce the bug, check with the reporter and QA first, then close the issue.
  • If the reproduction steps aren't clear and you determine the correct steps, please update the OP.
• This issue is filled out as thoroughly and clearly as possible
  • Pay special attention to the title, results, platforms where the bug occurs, and if the bug happens on staging/production.
• I have reviewed and subscribed to the linked Slack conversation to ensure Slack/Github stay in sync

[CortneyOfstad]

This seems like a duplicate of #22374

[kadiealexander]

I think they're slightly different, right? Yours is where the display name won't show at all for new users, whereas mine Is related to the & symbol being in the name for any user?

@s77rt could you please take a look and see if these might have the same root cause? Thanks so much!

[c3024]

Proposal

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

If name contains & sign it would not appear when we search like @abc&

What is the root cause of that problem?

This here breaks the words at special characters hence when & comes the last word would be empty string '' and there is no atIndex.
https://github.com/Expensify/App/blob/35e8d4643eb5f1ecd4db78bac6e145585e3d333a/src/pages/home/report/ReportActionCompose.js#L567C9-L573C10

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

We should define a CONST for SPECIAL_CHAR_OR_EMOJI_FOR_MENTIONS excluding the special characters allowed in displayName and use it to split the words.
For example for excluding & the regex is

SPECIAL_CHAR_OR_EMOJI_FOR_MENTION: /[\n\s,/?"{}[\]()^%\\;`$=#<>!*\p{Extended_Pictographic}\u200d\u{1f1e6}-\u{1f1ff}\u{1f3fb}-\u{1f3ff}\u{e0020}-\u{e007f}\u20E3\uFE0F]|[#*0-9]\uFE0F?\u20E3/gu,

and change in split words. As all special characters except , and ; are allowed in displayName we need to change the regex accordingly

Screen.Recording.2023-07-07.at.6.52.18.AM.mov

What other alternative solutions do you propose?

We can restrict users from using some special characters in the name.
https://github.com/Expensify/App/blob/35e8d4643eb5f1ecd4db78bac6e145585e3d333a/src/libs/ValidationUtils.js#L353C1-L355C2
Here we only disallow ',' & ';'
Then we need to remove all special characters except these from the regex specified above for splitting words.

For example, Slack restricts username to include only (){}[];’,./. If we restrict similarly in displayName we can change in the isValidDisplayName to check if displayName includes only these special characters.
In this case we can have a

CONST.VALID_DISPLAY_NAME = /[^a-zA-Z0-9\[\]();',.\/]/

and change the isValidName function to

function isValidDisplayName(name) {
    return !CONST.VALID_DISPLAY_NAME.test(name)
}

[c3024]

Root cause for this is different

[kadiealexander]

Repro:

2023-07-07_15-51-47.mp4

[melvin-bot]

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

[melvin-bot]

Current assignee @kadiealexander is eligible for the External assigner, not assigning anyone new.

[melvin-bot]

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

[s77rt]

@kadiealexander The two issues look different.

[LiaqatSaeed]

Hello
I was looking into this issue not sure if its already fixed or I am doing something wrong. Can anyone guide me on this? Here is the loom video
https://www.loom.com/share/c29e9669184f487088d51f70e2146bfb?sid=e46966dc-4ab4-45dd-9964-c40dc9656c7e

[melvin-bot]

📣 @LiaqatSaeed! 📣
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. 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.
2. 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.
3. 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>

[LiaqatSaeed]

Contributor details
Your Expensify account email: liaqatsaeed007@gmail.com
Upwork Profile Link: https://www.upwork.com/freelancers/liaqatsaeed

[melvin-bot]

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

[melvin-bot]

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

[eVoloshchak]

Not overdue, reviewing a proposal tomorrow

[Victor-Nyagudi]

After investigating this issue, I arrived at a similar conclusion to @c3024 above.

I just thought I'd add that the bug occurs not just when there's an & in the middle of the name - it's also there when the character is a # or some other special characters, so hopefully, whoever gets assigned checks for those too.

If the proposal isn't accepted, then I'm open to taking another crack at this, otherwise, all the best.

[eVoloshchak]

I agree with @c3024 assessment of the root cause

I like the general direction of the proposal, but it didn't resolve the issue for me.
However, I agree with the approach, looks like we were using a similar approach before changing it in #19004
@c3024, could you please verify once more your solution is working?

[c3024]

I checked and found that only part of string till the first special character is replaced with email once we submit the message. To replace the whole string with email, the REPLACE_MENTIONS should also be changed as specified below.
All changes are as follows:
We add

SPECIAL_CHAR_OR_EMOJI_FOR_MENTIONS:
            // eslint-disable-next-line no-misleading-character-class
            /[\n\s,;\p{Extended_Pictographic}\u200d\u{1f1e6}-\u{1f1ff}\u{1f3fb}-\u{1f3ff}\u{e0020}-\u{e007f}\u20E3\uFE0F]|[#*0-9]\uFE0F?\u20E3/gu,

here
and change

MENTION_REPLACER:
            // eslint-disable-next-line no-misleading-character-class
            /^@[^\n\r]*(?=$|[\s,/?"{}[\]()&^%\\;`$=#<>!*\p{Extended_Pictographic}\u200d\u{1f1e6}-\u{1f1ff}\u{1f3fb}-\u{1f3ff}\u{e0020}-\u{e007f}\u20E3\uFE0F]|[#*0-9]\uFE0F?\u20E3)/u,

We are changing the MENTION_REPLACER otherwise it will only replace part of the string till the first character with email (specifically the first '?' is removed from this regex)
at
https://github.com/Expensify/App/blob/35e8d4643eb5f1ecd4db78bac6e145585e3d333a/src/CONST.js#L1158C1-L1158C1
and
change

App/src/pages/home/report/ReportActionCompose.js

Line 556 in 35e8d46

const indexOfFirstWhitespaceCharOrEmojiAfterTheCursor = valueAfterTheCursor.search(CONST.REGEX.SPECIAL_CHAR_OR_EMOJI);

and
https://github.com/Expensify/App/blob/35e8d4643eb5f1ecd4db78bac6e145585e3d333a/src/pages/home/report/ReportActionCompose.js#L567C9-L567C9
to

const indexOfFirstWhitespaceCharOrEmojiAfterTheCursor = valueAfterTheCursor.search(CONST.REGEX.SPECIAL_CHAR_OR_EMOJI_FOR_MENTIONS);

and

const words = leftString.split(CONST.REGEX.SPECIAL_CHAR_OR_EMOJI_FOR_MENTIONS);

Result

Screen.Recording.2023-07-14.at.1.05.52.AM.mov

Could you please check this? @eVoloshchak This should work.

[melvin-bot]

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

[melvin-bot]

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

[eVoloshchak]

Testing this

[eVoloshchak]

@c3024, can confirm it working, thanks
I wonder if we could simplify the logic here. What is the purpose of SPECIAL_CHAR_OR_EMOJI_FOR_MENTIONS in

const words = leftString.split(CONST.REGEX.SPECIAL_CHAR_OR_EMOJI_FOR_MENTIONS);

Can we replace it with const words = leftString.split(' ')?

[c3024]

@eVoloshchak There is a specific case where this difference matters.
Say a username is abc/de.
Now, if I search for @abc/de,@, with the line of code I used, the words get spit at ',' and the last word will be @ so the mentions appear. If we split words with " " (space), then the last word will be "@abc/de,@" and no mentions appear.
Please check the attached video.
I wanted to keep the existing behaviour intact (i.e. the words break at the last special character which is not allowed in name (instead of " ")) because this change might be treated as a regression so I just kept it in line with earlier code.

We can change it to split at ' ' because it also appears logical but there may have been some discussion in which it was decided to split at special characters.

Result

Screen.Recording.2023-07-20.at.2.42.46.AM.mov

[melvin-bot]

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

[eVoloshchak]

@c3024, ah, I see, thanks
I think this is a bit weird, we allow user to use special characters to break the word (i.e. use comma in @abc/de,@ to start a new mention), while simultaneously allowing the same special characters to be used in names
I'll start a discussion on Slack to get more opinions

[c3024]

@eVoloshchak In case I misunderstood, just to clarify we don't allow , and : in name. All other characters are allowed. So, words are split at , or : only with the regex I suggested and not at other special characters.
So, they are not the same special characters that are used in both names and in splitting.

I too think we can just split at ' ' as it is simpler and I think it is similar in other platforms like slack as well. I need to confirm.

[melvin-bot]

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

[melvin-bot]

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

[kadiealexander]

@eVoloshchak can you please link your Slack discussion here?

[eVoloshchak]

Asking here: https://expensify.slack.com/archives/C01GTK53T8Q/p1690397006388229

[kadiealexander]

We agreed not to fix this here, as it's a very unlikely use case we'll wait for a real user to report it.