[$1000] Chat - SMS domain shown in mentions

[kbecciv]

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:

1. Have some previous chats with phone number
2. Go to any chat
3. Start mentioning phone number

Expected Result:

Phone number alone should be shown

Actual Result:

Phone number is shown with expensify sms domain

Workaround:

Unknown

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

Reproducible in staging?: yes

Reproducible in production?: yes

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

Expensify/Expensify Issue URL:

Issue reported by: @abdulrahuman5196

Slack conversation: https://expensify.slack.com/archives/C049HHMV9SM/p1686239745633599

View all open jobs on GitHub

Upwork Automation - Do Not Edit

• Upwork Job URL: https://www.upwork.com/jobs/~01180378586a3144b1
• Upwork Job ID: 1669613431669092352
• Last Price Increase: 2023-06-16

[melvin-bot]

Triggered auto assignment to @bfitzexpensify (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

[melvin-bot]

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

[melvin-bot]

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

[melvin-bot]

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

[dhairyasenjaliya]

Proposal

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

• Chat - SMS domain shown in mentions

What is the root cause of that problem?

• The root cause is we are not adding any filter to remove domain extensions from the number

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

• We need 2 changes in order to remove the SMS domain as well as filter data as that we omit by removing the sms domain

Change - 1

• 1st change is to remove the SMS domain from the alternateText which can be found here and apply Str.removeSMSDomain

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

Line 465 in 38fe17f

alternateText: detail.login,

Change - 2

• Another change is to filter data as per the Ommited SMS domain which will be found here and apply same Str.removeSMSDomain

• If we don't add filter change and then type @exp then also it will render a number that has an extension of @expensify on it, so it does not make any sense to show that since we already removed the domain from displayed mention.

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

Line 455 in 38fe17f

if (searchValue && !`${detail.displayName} ${detail.login}`.toLowerCase().includes(searchValue.toLowerCase())) {

Result if we don't add changes - 2

Final result

Note : At time of adding this proposal non other proposal suggest to remove domain from search as well

[tienifr]

Proposal

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

SMS domain shown in mentions.

What is the root cause of that problem?

Currently we do not allow phone number mentions without SMS domain. We do not remove the SMS domain when displaying it in both the mention suggestions and the message.

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

1. Remove SMS domain in mention suggestions in alternateText:

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

Line 465 in 38fe17f

alternateText: detail.login,

2. Update regex in expensify-common to allow mentions with phone number only. Otherwise, the phone number will not be parsed with <mention-user> tag and thus not displayed as mention. (I think this is the trickiest part but we'll handle that in the PR if assigned)

3. To allow user to navigate to mentioned account, we need to add the SMS domain back into the phone number in MentionUserRenderer using OptionsListUtils.addSMSDomainIfPhoneNumber. Since this format is recognized as the primary login:

App/src/components/HTMLEngineProvider/HTMLRenderers/MentionUserRenderer.js

Line 34 in bd0e843

const loginWithoutLeadingAt = props.tnode.data.slice(1);

4. (Optional) In case we want to completely eliminate the SMS domain while searching for mentions as in the above proposal, we need to remove SMS domain from detail.login here too:

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

Line 455 in 38fe17f

if (searchValue && !`${detail.displayName} ${detail.login}`.toLowerCase().includes(searchValue.toLowerCase())) {

What alternative solutions did you explore? (Optional)

NA

Results

FIY, without the second step in my solution, the mention will be displayed as plain text in the message:

[bfitzexpensify]

Couple of proposals ready for review @Santhosh-Sellavel

[Santhosh-Sellavel]

@bfitzexpensify I'll post my review comments tomorrow. Need to check this further.

[Santhosh-Sellavel]

@tienifr's Proposal here look good to me. Both proposals are mostly similar, but @tienifr's proposal mentioned the highlighting of the phone number which will be missing, so we should go with them.

(Optional) In case we want to completely eliminate the SMS domain while searching for mentions as in the above proposal, we need to remove SMS domain from detail.login here too:

I think we need to exclude the results while searching with SMS domain

C+ Reviewed
🎀 👀 🎀

[melvin-bot]

Triggered auto assignment to @flodnv, see https://stackoverflow.com/c/expensify/questions/7972 for more details.

[flodnv]

@puneetlath can you check this please? It has to do with mentions (and some of it came from #19221)

[puneetlath]

This isn't really a bug. This is how it will have to be until we implement handles.

[puneetlath]

I'm going to close, but feel free to reopen if you disagree.