[Hold for Payment June 19][$250] Workspace - Inconsistency in Block Quote Usage Between Workspace Description and Chat

[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.63-7
Reproducible in staging?: Y
Reproducible in production?: Y
If this was caught during regression testing, add the test name, ID and link from TestRail: N/A
Email or phone of affected tester (no customers): abebemiherat@gmail.com
Issue reported by: Applause - Internal Team

Action Performed:

1. Navigate to profile > workspace
2. Go to Description, erase the default statement, and add a block quote markdown (e.g., ">>abebe"). Notice it displays correctly
3. Click on Description again, and notice that one quote is reduced
4. Go to any chat, write the same block quote as in the description, click on edit, and then save it. Notice that it doesn't change

Expected Result:

The block quote should be displayed correctly in the workspace description section, like in chat, ensuring consistency

Actual Result:

When writing a block code example (">>abebe") in the workspace description, clicking on it and saving it without changes reduces one quote. However, this inconsistency does not occur in chat

Workaround:

Unknown

Platforms:

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

• Android: Native
• Android: mWeb Chrome
• iOS: Native
• iOS: mWeb Safari
• MacOS: Chrome / Safari
• MacOS: Desktop

Screenshots/Videos

Add any screenshot/video evidence

Bug6454831_1713528210858.Screen_Recording_2024-04-19_at_4.59.16_AM.mp4

View all open jobs on GitHub

Upwork Automation - Do Not Edit

• Upwork Job URL: https://www.upwork.com/jobs/~015e52952078f428b3
• Upwork Job ID: 1781372065227497472
• Last Price Increase: 2024-04-19
• Automatic offers:
• jjcoffee | Reviewer | 0

Issue Owner

Current Issue Owner: @jjcoffee

[melvin-bot]

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

@isabelastisser 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]

Job added to Upwork: https://www.upwork.com/jobs/~015e52952078f428b3

[melvin-bot]

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

[skyweb331]

Proposal

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

Inconsistency in Block Quote Usage in text

What is the root cause of that problem?

App/src/pages/workspace/WorkspaceProfileDescriptionPage.tsx

Lines 29 to 40 in c1827d8

	const [description, setDescription] = useState(() =>
	parser.htmlToMarkdown(
	// policy?.description can be an empty string
	// eslint-disable-next-line @typescript-eslint/prefer-nullish-coalescing
	policy?.description ||
	parser.replace(
	translate('workspace.common.welcomeNote', {
	workspaceName: policy?.name ?? '',
	}),
	),
	),
	);

parser.htmlToMarkdown does not parse html correctly.

For example
parser.htmlToMarkdown('<blockquote><blockquote>abc</blockquote></blockquote>') => >abc which should be >>abc

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

Need to fix ExpensiMark module.

https://github.com/Expensify/expensify-common/blob/c0f7f3b6558fbeda0527c80d68460d418afef219/lib/ExpensiMark.js#L408-L424

FYI, this problem also exists on chat, but if message is saved without changes, it does not call API. That's why this problem does not detected by @lanitochka17. If user edits a blockquote message on chat, it eats blockquotes too.

What alternative solutions did you explore? (Optional)

N/A

[saifelance]

@skyweb331

The root cause of the problem is that the parser.htmlToMarkdown function does not correctly parse HTML. Specifically, when encountering nested

elements, it produces incorrect Markdown output.

To solve this issue, you'll need to fix the ExpensiMark module, which is responsible for parsing HTML into Markdown. The problematic behavior is identified in the ExpensiMark file, particularly in the section that handles blockquote elements.

You can address the issue by modifying the logic in the ExpensiMark.js file to correctly handle nested blockquote elements and ensure that the Markdown output reflects the correct nesting level.

[jjcoffee]

@skyweb331 Thanks for your proposal! We look for detailed solutions, so you would need to propose what changes you would make to ExpensiMark in order to fix this issue.

[skyweb331]

@jjcoffee I am going to use fast-xml-parser or xml2js to parse HTML string to javascript object and then will make markdown string from that object.
Regex is really difficult to handle complicated html structures.

[skyweb331]

If these modules are too big, I can just copy ideas from those modules for parsing html.

[dragnoir]

Proposal

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

Inconsistency in Block Quote Usage Between Workspace Description and Chat

What is the root cause of that problem?

then the page WorkspaceProfileDescriptionPage opens, the description value called from policy?.name pass throw parser.htmlToMarkdown for transformation from HTML to Markdown

Currently, if we pass <blockquote><blockquote>abebe</blockquote></blockquote> the returned value from parser.htmlToMarkdown is >abebe and not >>abebe

We need to fix this part here from expensify-common lib

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

Here's an adjusted version of the code snippet that will handle the nested <blockquote> elements correctly:

{
    name: 'quote',
    regex: /(<blockquote(?:[^>]*?)>)([\s\S]*?)(<\/blockquote>)/gi,
    replacement: function replaceBlockquotes(match, openTag, content, closeTag, offset, string) {
        // Recursively process nested blockquotes
        content = content.replace(/(<blockquote(?:[^>]*?)>)([\s\S]*?)(<\/blockquote>)/gi, replaceBlockquotes);
        
        // Split content into lines, trim and add quote markers
        content = content
            .split('\n')
            .map(line => line.trim() ? `> ${line}` : '')
            .join('\n');

        // Add an extra '>' to each line to account for the current blockquote level
        content = content
            .split('\n')
            .map(line => line ? `>${line}` : '> ')
            .join('\n');

        return content;
    }
},

The replaceBlockquotes function is called recursively whenever another <blockquote> is found within the content. This ensures that each level of nesting is processed, and the correct number of > is prepended.

Each line of the content inside a blockquote is prefixed with > to transform the content into markdown quote format. Lines are trimmed to handle spacing and alignment issues.

This revised method ensures that every nested <blockquote> results in an additional > symbol being added to each line, recursively building up the correct quoting structure for markdown.

As on chat, we can also check on Workspace description if the old value is equal to the new one. If true, we will not update the value in the database

POC;

20240422_183810.mp4

[skyweb331]

@dragnoir I just tested your code against all testing cases on expensify-common but it fails...
your parse logic does not converts all cases correctly.

For example, <blockquote>123<br/><blockquote>abc</blockquote></blockquote> becomes >>123\n>abc by your logic which is wrong.

[dragnoir]

123

abc

what was your input? before HTML

[skyweb331]

<blockquote>123<br/><blockquote>abc</blockquote></blockquote> this should be parsed to >123\n>>abc

[dragnoir]

<blockquote>123<br/><blockquote>abc</blockquote></blockquote> this should be parsed to >123\n>>abc

What input did you texted inside the workspace description? like markdown not HTML

[skyweb331]

>123\n>>abc is what I input and <blockquote>123<br/><blockquote>abc</blockquote></blockquote> what ExpensiMark converted.

[jjcoffee]

Reviewing tomorrow!

[dragnoir]

Proposal update

Proposal updated to fix the issue of multi-lines quote.

[jjcoffee]

Unfortunately, I had higher priority issues to get to today. Tomorrow!

[isabelastisser]

Hey @jjcoffee, can you please review the updated proposal above? Thanks!

[MonilBhavsar]

PR is being reviewed here

[isabelastisser]

Any updates, @dragnoir ?

[jjcoffee]

@isabelastisser App PR is up, but we are waiting to proceed because an earlier PR to upgrade expensify-common recently got reverted so we don't want to upgrade until that's fixed.

[isabelastisser]

Thanks for the update, @jjcoffee !

[jjcoffee]

There's a new PR that will bump expensify-common to a newer version (that will include the changes from this issue). We'll just need to monitor that when it's pushed to production.

[blazejkustra]

@jjcoffee Would you mind testing related changes to this issue on the new PR?

[dragnoir]

I think this is ready for payment!

[MonilBhavsar]

I think it should be 7 days post #42387 is deployed to production

[jjcoffee]

Would you mind testing related changes to this issue on the new PR?

@blazejkustra I see it's already been merged, but I've given it a quick test run anyway and all looks good!

desktop-chrome-retest-2024-06-06_10.25.39.mp4

[mallenexpensify]

Can't figure out what's going on here. @jjcoffee @MonilBhavsar can you provide an update? Is payment due to anyone? Thx

[jjcoffee]

@mallenexpensify #42387 bumped expensify-common to the version that includes our changes. It just hit production yesterday, so payment should be due 19 June (assuming no regressions, of course!).

[MonilBhavsar]

Waiting for payment due here.
@jjcoffee mind doing checklist please when you have a moment

[MonilBhavsar]

BugZero Checklist: The PR fixing this issue has been merged! The following checklist (instructions) will need to be completed before the issue can be closed:

• [@jjcoffee] The PR that introduced the bug has been identified. Link to the PR:
• [@jjcoffee] The offending PR has been commented on, pointing out the bug it caused and why, so the author and reviewers can learn from the mistake. Link to comment:
• [@jjcoffee] A discussion in #expensify-bugs has been started about whether any other steps should be taken (e.g. updating the PR review checklist) in order to catch this type of bug sooner. Link to discussion:
• [@jjcoffee] Determine if we should create a regression test for this bug.
• [@jjcoffee] If we decide to create a regression test for the bug, please propose the regression test steps to ensure the same bug will not reach production again.
• [@isabelastisser ] Link the GH issue for creating/updating the regression test once above steps have been agreed upon:

[jjcoffee]

• The PR that introduced the bug has been identified. Link to the PR: N/A - I think this behaviour has always been there
• The offending PR has been commented on, pointing out the bug it caused and why, so the author and reviewers can learn from the mistake. Link to comment: N/A
• A discussion in #expensify-bugs has been started about whether any other steps should be taken (e.g. updating the PR review checklist) in order to catch this type of bug sooner. Link to discussion: N/A
• Determine if we should create a regression test for this bug. Yes
• If we decide to create a regression test for the bug, please propose the regression test steps to ensure the same bug will not reach production again.

Regression Test Proposal

1. Navigate to Profile > Workspace
2. Go to Description and enter some nested block quote markdown, e.g. >> ABC
3. Verify that the live markdown is rendering the nested blockquotes correctly, i.e.

ABC

4. Save the description
5. Verify that the blockquotes still render correctly
6. Tap on Description to edit it again
7. Verify that the value to edit displays the correct number of arrows to match the nesting level of the blockquote

Do we agree 👍 or 👎

[mallenexpensify]

Contributor: @dragnoir paid $250 via Upwork
Contributor+: @jjcoffee paid $250 via Upwork

TestRail GH

• https://github.com/Expensify/Expensify/issues/405754

Thanks y'all.