[HOLD for payment 2024-06-21] [$250] mWeb - Chat - Quote markdown is not applied

[lanitochka17]

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: 1.4.82-1 Reproducible in staging?: Y Reproducible in production?: N If this was caught during regression testing, add the test name, ID and link from TestRail: N/A Issue reported by: Applause - Internal Team

Action Performed:

1. Go to https://staging.new.expensify.com/home
2. Tap on report
3. Enter >hsjd
4. Send the message

Expected Result:

Quote markdown must be applied

Actual Result:

Quote markdown is not applied

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/53cb9fa1-12a0-43ac-82fa-4cc25eb52589

View all open jobs on GitHub

Upwork Automation - Do Not Edit

• Upwork Job URL: https://www.upwork.com/jobs/~0162a44a8fd308afd5
• Upwork Job ID: 1798839739922152623
• Last Price Increase: 2024-06-06
• Automatic offers:
• ishpaul777 | Contributor | 102639567

Issue Owner

Current Issue Owner: @anmurali

[melvin-bot[bot]]

Triggered auto assignment to @bondydaa (DeployBlockerCash), see https://stackoverflowteams.com/c/expensify/questions/9980/ for more details.

[github-actions[bot]]

:wave: Friendly reminder that deploy blockers are time-sensitive ⏱ issues! Check out the open `StagingDeployCash` deploy checklist to see the list of PRs included in this release, then work quickly to do one of the following:

1. Identify the pull request that introduced this issue and revert it.
2. Find someone who can quickly fix the issue.
3. Fix the issue yourself.

[lanitochka17]

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

[dragnoir]

Not a deploy blocker.

Offended PR https://github.com/Expensify/expensify-common/pull/691

this is a new expected behavior:

This PR changes quote regex to enforce following space after >. not followed by space won't be parsed as quote. In the multiple level quote context spaces between > signs are irrelevent

[roryabraham]

If this is indeed the new expected behavior, then react-native-live-markdown needs to be updated such that it's using the new rule. Because right now it's confusing that the markdown processing in the composer is different than what we show in the chat. I think this is a legitimate blocker.

[roryabraham]

These are our options:

• ignore this inconsistency, let this 🐛 go to prod for the time being.
• revert https://github.com/Expensify/App/pull/42387
• push forward https://github.com/Expensify/react-native-live-markdown/pull/360
• Include https://github.com/Expensify/expensify-common/pull/691 in react-native-live-markdown via a patch

in any event, both PRs need to be included in the same release, or we'll have this inconsistency

[luacmartins]

I think reverting https://github.com/Expensify/App/pull/42387 might not be a good solution since there's at least another PR that depends on it https://github.com/Expensify/App/pull/42504

[luacmartins]

TBH I think we should have a full regression test on https://github.com/Expensify/react-native-live-markdown/pull/360, so I'm not sure that we should push it forward and CP a package bump in App. So that leaves us with adding a patch or demoting this to NAB since we know the root cause and will have a fix out soon

[luacmartins]

Discussing our options here, let's try to go with:

• Include https://github.com/Expensify/expensify-common/pull/691 in react-native-live-markdown via a patch

Making this external for the time being so we can get some contributor eyes to work on the solution above

[melvin-bot[bot]]

Job added to Upwork: https://www.upwork.com/jobs/~0162a44a8fd308afd5

[melvin-bot[bot]]

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

[dominictb]

Proposal

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

Quote markdown is not applied consistently in the live preview and after sent.

What is the root cause of that problem?

This is because of version mismatch of expensify-common in react-native-live-markdown package.

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

Here's the decided solution for now

Include https://github.com/Expensify/expensify-common/pull/691 in react-native-live-markdown via a patch

To do this, we should add the changes of https://github.com/Expensify/expensify-common/pull/691 in node_modules/@expensify/react-native-live-markdown/lib/parser/react-native-live-markdown-parser.js, then create a patch file via npx patch-package @expensify/react-native-live-markdown

Here's the generated patch file @expensify+react-native-live-markdown+0.1.70.patch

It should be added in patches directory.

It's working well after the patch: https://github.com/Expensify/App/assets/165644294/f56b689f-9fa4-4d9c-b307-fa85541452e2

What alternative solutions did you explore? (Optional)

We can build the react-native-live-markdown-parser.js again inside the react-native-live-markdown repo, then copy it over, the patch package process is the same

[tomekzaw]

Hey, fyi we're already bumping expensify-common in react-native-live-markdown to 2.0.10 here:

• https://github.com/Expensify/react-native-live-markdown/pull/360

[tomekzaw]

@BartoszGrajdek will submit a PR bumping react-native-live-markdown to 0.1.82 that is supposed to resolve this bug.

[dominictb]

@tomekzaw According to this, it seems we won't be able to deploy https://github.com/Expensify/react-native-live-markdown/pull/360 to prod any time soon. So it was suggested here to use a patch for the time being.

What do you think?

[tomekzaw]

This will not be straightforward as it's not sufficient to just patch expensify-common in react-native-live-markdown; one also needs to re-bundle react-native-live-markdown-parser.js (pretty large file) and perhaps include it as patch as well (as described in https://github.com/Expensify/App/issues/43180#issuecomment-2154047373).

I recommend bumping react-native-live-markdown to 0.1.82 as this solves multiple problems at once.

[dominictb]

one also needs to re-bundle react-native-live-markdown-parser.js (pretty large file) and perhaps include it as patch as well (as described in https://github.com/Expensify/App/issues/43180#issuecomment-2154047373).

Yeah I've done it, tested and it works well, I don't think it will be a problem because it will be removed once we bump react-native-live-markdown version. Maybe we should just get the patch in to avoid rushing to bump react-native-live-markdown which may cause further problems/regressions.

But @luacmartins will decide 👍

[BartoszGrajdek]

Here's the PR bumping the version of react-native-live-markdown: https://github.com/Expensify/App/pull/43255

[melvin-bot[bot]]

📣 @ishpaul777 🎉 An offer has been automatically sent to your Upwork account for the Contributor role 🎉 Thanks for contributing to the Expensify app!

Offer link Upwork job Please accept the offer and leave a comment on the Github issue letting us know when we can expect a PR to be ready for review 🧑‍💻 Keep in mind: Code of Conduct | Contributing 📖

[mountiny]

Noting from @ShridharGoel this might have not been fixed completely yet in native @kavimuru can you please retest?

[ishpaul777]

@tomekzaw Do we have another PR coming for this issue

[ishpaul777]

oh wait i think this issue is fixed already with that PR since the expected here was this https://github.com/Expensify/App/pull/43255#issuecomment-2155009813

cc @mountiny

[BartoszGrajdek]

Yes this should be fixed now. There was a problem with different versions of expensify-common in react-native-live-markdown and E/App. Can we close this one? cc @mountiny @thienlnam

[luacmartins]

Yea, we can close since this is fixed.

[ishpaul777]

hello @luacmartins, payment for the PR is pending can we please reopen

[ishpaul777]

@luacmartins/ @mountiny Can we please assign a BZ member for payment

[melvin-bot[bot]]

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

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

[melvin-bot[bot]]

@anmurali, @luacmartins, @ishpaul777 6 days overdue. This is scarier than being forced to listen to Vogon poetry!

[anmurali]

Paid.