[$500] mWeb - Chat - @ here highlight is removed if you also @phone number

[lanitochka17]

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.60 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/4479302 Issue reported by: Applause - Internal Team

Action Performed:

1. Go to https://staging.new.expensify.com/
2. Tap on a report
3. Enter @ and select here from suggestion box
4. Note here is highlighted
5. In next line, enter @ and select an email
6. Note here is highlighted after selecting an email id
7. Clear the message
8. Enter @ and select here from suggestion box
9. In next line, enter @ and type +1 and select a phone number from suggestion list

Expected Result:

Here highlighted must not be removed after selecting an phone number

Actual Result:

Here highlighted is removed after selecting an phone number

Workaround:

Unknown

Platforms:

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

• [ ] 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/Expensify/App/assets/78819774/d85b7732-0612-4e68-b682-56199d743e54

View all open jobs on GitHub

Upwork Automation - Do Not Edit

• Upwork Job URL: https://www.upwork.com/jobs/~01875c053a0be9f2e8
• Upwork Job ID: 1776196507726311424
• Last Price Increase: 2024-04-25

[melvin-bot[bot]]

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

[lanitochka17]

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

[lanitochka17]

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

[melvin-bot[bot]]

Job added to Upwork: https://www.upwork.com/jobs/~01875c053a0be9f2e8

[melvin-bot[bot]]

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

[adriancova]

I took a look at this one, not 100% sure if my findings are correct. But I don't think this should be solved in App project since it seems to be caused by internal logic of @expensify/react-native-live-markdown library

specifically the parser is utilizing a version from 2 weeks ago for ExpensiMark (expensify-common) https://github.com/Expensify/react-native-live-markdown/blob/main/parser/package.json

and there's a version from yesterday that included a fix for phone regex and is being used in this project, which could explain why there's a difference between mentions inside the input vs a comment. https://github.com/Expensify/expensify-common/commit/13de5b0606662df33fa1392ad82cc11daadff52e

hope this helps

[laurenreidexpensify]

@mollfpr what do you think ^

[mollfpr]

@adriancova I can't verify your found, but if you're sure then you can write a proposal to update the package in @expensify/react-native-live-markdown using the latest library expensify-common.

[adriancova]

Proposal

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

attempting to tag a user via phone +{number}{number} is breaking the handling of special html elements such as <mention-user> and <mention-here>

What is the root cause of that problem?

After debugging the react-native-live-markdown library I've noticed that the parseExpensiMarkToRanges fuction adds @expensify.sms whenever a phone Regex is found.

And afterwards there's a condition that checks that the original text remains the same after being parsed into ranges. since the condition is not met, we default to returning back an empty array of ranges resulting in vanilla text being shown in the input.

this logic comes from ExpensiMark for userMentions:

{
                name: 'userMentions',
                regex: new RegExp(`(@here|[a-zA-Z0-9.!$%&+=?^\`{|}-]?)(@${Constants.CONST.REG_EXP.EMAIL_PART}|@${Constants.CONST.REG_EXP.PHONE_PART})(?!((?:(?!<a).)+)?<\\/a>|[^<]*(<\\/pre>|<\\/code>))`, 'gim'),
                replacement: (match, g1, g2) => {
                    if (!Str.isValidMention(match)) {
                        return match;
                    }
                    const phoneRegex = new RegExp(`^@${Constants.CONST.REG_EXP.PHONE_PART}$`);
                    return `${g1}<mention-user>${g2}${phoneRegex.test(g2) ? `@${Constants.CONST.SMS.DOMAIN}` : ''}</mention-user>`;
                },
            },

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

We can use patch-package to manually change the ExpensiMark being used inside of react-native-live-markdown library as it has already been done before. We just need to remove the check for phoneRegex that adds the @expensify.sms and instead keep the text as it is.

{
                // ...
                    if (!Str.isValidMention(match)) {
                        return match;
                    }
                    return `${g1}<mention-user>${g2}</mention-user>`;
                },
            },

What alternative solutions did you explore? (Optional)

1- Another alternative could be to apply this change directly in the expensify-commons library, but I'm unsure if it would end up breaking some logic/functionality in another place.

[laurenreidexpensify]

@mollfpr ^^

[melvin-bot[bot]]

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

[laurenreidexpensify]

@mollfpr bump for review thanks

[mollfpr]

Sorry for the delay 🙏

@adriancova I'm not too onboard patching the expensify-common inside the package. If we use the latest expensify-common, the issue will arise again.

[melvin-bot[bot]]

Upwork job price has been updated to $500

[laurenreidexpensify]

increased bounty

[adriancova]

hey @mollfpr here's an alternative solution which I think you'll like, let me know if you need to modify my original proposal or this message, first time I'm re-proposing.

What alternative solutions did you explore? (Optional)

2- Let's modify the expensify-commons library to add a rawInputReplacement method to userMentions.

This is the current logic for parseMarkdownToHTML inside of the react-native-live-markdown parser.

function parseMarkdownToHTML(markdown: string): string {
  const parser = ExpensiMark;
  const html = parser.replace(markdown, {
    shouldKeepRawInput: true,
  });
  return html as string;
}

The options object includes shouldKeepRawInput: true which translates to the ExpensiMark rules using the rawInputReplacement over the replacement method (if available).

so I suggest we add this method to the userMentions rule like this:

{
                name: 'userMentions',
                regex: new RegExp(`(@here|[a-zA-Z0-9.!$%&+=?^\`{|}-]?)(@${Constants.CONST.REG_EXP.EMAIL_PART}|@${Constants.CONST.REG_EXP.PHONE_PART})(?!((?:(?!<a).)+)?<\\/a>|[^<]*(<\\/pre>|<\\/code>))`, 'gim'),
                replacement: (match, g1, g2) => {
                    if (!Str.isValidMention(match)) {
                        return match;
                    }
                    const phoneRegex = new RegExp(`^@${Constants.CONST.REG_EXP.PHONE_PART}$`);
                    return `${g1}<mention-user>${g2}${phoneRegex.test(g2) ? `@${Constants.CONST.SMS.DOMAIN}` : ''}</mention-user>`;
                },
                rawInputReplacement: (match, g1, g2) => {
                    if (!Str.isValidMention(match)) {
                        return match;
                    }
                    return `${g1}<mention-user>${g2}</mention-user>`;
                },
            },

This way the sms domain won't be added and the text content before and after parsing will remain the same, resulting on the ranges being passed back successfully (and therefore the mentions properly rendered).

This proposal would require 2 prs. 1st one to add the rawInputReplacement method into the expensify-commons repo and 2nd to update the version of expensify-commons used inside of the react-native-live-markdown repo.

[melvin-bot[bot]]

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

[laurenreidexpensify]

@mollfpr for review ^^

[mollfpr]

@adriancova The alternative proposal makes sense to me, but I need to test it by myself. Could you help me with setting up the react-native-live-markdown locally and make the changes apply to the library?

[adriancova]

@mollfpr of course, here you go: https://github.com/adriancova/react-native-live-markdown

I updated the expensify-common library to now include the ExpensiMark change: https://github.com/adriancova/expensify-common/commit/135816a1abddc69cb52e75ee66e1d1305d91239a

rebuilt the parser, and added a new test for phone number, we should be able to add whatever new tests necessary.

Let me know if I can help with anything else

[melvin-bot[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[bot]]

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

[laurenreidexpensify]

@mollfpr bump thanks

[mollfpr]

Checking the lib now 👀

[laurenreidexpensify]

@mollfpr bump for update thanks

[melvin-bot[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[bot]]

@mollfpr @laurenreidexpensify this issue is now 4 weeks old, please consider:

• Finding a contributor to fix the bug
• Closing the issue if BZ has been unable to add the issue to a VIP or Wave project
• If you have any questions, don't hesitate to start a discussion in #expensify-open-source

Thanks!

[mollfpr]

I think the alternative proposal looks good to me!

🎀 👀 🎀 C+ reviewed!

[melvin-bot[bot]]

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

[melvin-bot[bot]]

📣 @adriancova You have been assigned to this job! Please apply to the Upwork job and leave a comment on the Github issue letting us know when we can expect a PR to be ready for review 🧑‍💻 Once you apply to this job, your Upwork ID will be stored and you will be automatically hired for future jobs! Keep in mind: Code of Conduct | Contributing 📖