[HOLD for payment 2024-04-09] [$500] Task - A new line is added to code block when saving unchanged description with code block

[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:
Reproducible in staging?:
Reproducible in production?:
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
Expensify/Expensify Issue URL:
Issue reported by: Applause - Internal team
Slack conversation:

Action Performed:

1. Go to staging.new.expensify.com
2. Create a task without description
3. Go to task report
4. Click description
5. Type a word in code block in one line
6. Save the description
7. Click on the description
8. Save the description
9. Repeat Step 7 and 8 several times

Expected Result:

No extra line will be added each time saving unchanged description with code block
The system message for task descrption will not appear each time saving unchanged description with code block

Actual Result:

An extra line will be added each time saving unchanged description with code block
The system message for task descrption appears each time saving unchanged description with code block

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

Bug6407279_1709922210048.20240309_021530.1.mp4

View all open jobs on GitHub

Upwork Automation - Do Not Edit

• Upwork Job URL: https://www.upwork.com/jobs/~01e1f69c2987288ad8
• Upwork Job ID: 1767765440789991424
• Last Price Increase: 2024-03-13

[melvin-bot]

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

[lanitochka17]

@jliexpensify 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
CC @quinthar

[allgandalf]

Proposal

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

A new line is added to code block when saving unchanged description with code block

What is the root cause of that problem?

This is a issue with expensify-common library, we use replace which leads to this bug:

App/src/pages/tasks/TaskDescriptionPage.tsx

Line 114 in 39bd108

defaultValue={parser.htmlToMarkdown((report && parser.replace(report?.description ?? '')) || '')}

Everytime we use replace it adds a extra line to the start of the markdown

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

We can make use of the filterRules in replace:
https://github.com/Expensify/expensify-common/blob/77d0b150ba6bfbe7a64b3c3e30b65592b2e58c4a/lib/ExpensiMark.d.ts#L26-L27

In the taskDescriptionPage, we need to apply the following markdownInBackticks filter option:

defaultValue={parser.htmlToMarkdown((props.report && parser.replace(props.report.description, {
                                filterRules: ["markdownInBackticks"]
                            })) || '')}

Note: I won't recommend fixing this upstream as we use parser at many places and this is related to only markdown, so we can use the filterRules provided upstream to deal with this bug.

Result:

simplescreenrecorder-2024-03-09_02.03.02.mp4

[bernhardoj]

Proposal

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

Saving an unchanged codefence markdown will add 1 new line.

What is the root cause of that problem?

When we save a codefence markdown with new line,

```
hello
```

EditTask response will return the new line as \r\n. Currently, codefence markdown regex only matches ``` with \n as the opening tag.
https://github.com/Expensify/expensify-common/blob/77d0b150ba6bfbe7a64b3c3e30b65592b2e58c4a/lib/ExpensiMark.js#L33-L34

So, when we convert it to markdown, there will be an extra new line by \r\n in the codefence content/body.

It's know that the sever might returns \r\n as the new line as shown by the comment below.

App/src/pages/tasks/TaskDescriptionPage.tsx

Lines 50 to 51 in 9302ade

	// report.description might contain CRLF from the server
	if (StringUtils.normalizeCRLF(values.description) !== StringUtils.normalizeCRLF(report?.description) && !isEmptyObject(report)) {

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

To solve it case by case, we can also apply StringUtils.normalizeCRLF to the task description before converting it to markdown when showing the task description in both TaskDescriptionPage and TaskView.

What alternative solutions did you explore? (Optional)

Update the codefence regex to match either \r\n or \n.
https://github.com/Expensify/expensify-common/blob/77d0b150ba6bfbe7a64b3c3e30b65592b2e58c4a/lib/ExpensiMark.js#L33-L34

/(```(?:\r\n|\n)?)((?:\s*?(?!(?:\r\n|\n)?```(?!`))[\S])+\s*?)((?=(?:\r\n|\n)?)```)/g

// the difference is instead of using \n, we use (?:\r\n|\n)
// ?: to not capture it as a group as it will broke the replacement logic

[melvin-bot]

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

[melvin-bot]

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

[melvin-bot]

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

[jliexpensify]

Sorry, was sick yesterday. Can repro - what a weird bug.

[tienifr]

Proposal

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

1. An extra line will be added each time saving unchanged description with code block
2. The system message for task description appears each time saving unchanged description with code block

What is the root cause of that problem?

1. When we create or edit a task description, the back-end will return the description with \r\n as the new line, this is also acknowledged by us here
   Then in our parser, we only acknowledge the \n as a possible new line after "```", if it's not there, we'll append new line, this leads to a new line being added each time we edit the task
2. The 1. is one of the root cause for this problem. After 1. is fixed, we'll be left with this

Although we didn't change anything, there's still another `updated the description to` message, with spaces between the backticks and the text inside backticks.

This is because when we save

```hello```

as codeblock, we'll save it as is. Meanwhile, when we display it here, we normalize it by adding extra new lines, this is what we do in the main composer as well.

```
hello
```

Now when we save, it will be saved with the extra new lines, we detect changes and the updated the description to is added. Although to the users, nothing changes at all.

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

1. Update the code fence Regex here to acknowledge \r\n as possible new line character as well

/(```(?:\r\n|\n)?)((?:\s*?(?!(?:\r\n|\n)?```(?!`))[\S])+\s*?)((?=(?:\r\n|\n)?)```)/g

2. Before saving the description here, and also when creating the task with description, we should normalize the description just like when we display it here. So what we save will be consistent with what we displayed. And if the user sees there's no visual change, there's indeed no change.

What alternative solutions did you explore? (Optional)

1. An alternate solution is we can call StringUtils.normalizeCRLF on the report description whenever we try to display it to set it as default value of the input.

[DylanDylann]

Thanks for all your proposals

@GandalfGwaihir's proposal is to filter out the markdownInBackticks rule and it's not correct since we need to add the extra line for description when users open edit task page

@bernhardoj's proposal doesn't fix the second point The system message for task descrption will not appear each time saving unchanged description with code block

@tienifr's solution makes sense since it fixes 2 points in the OP

🎀👀🎀 C+ reviewed

[melvin-bot]

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

[DylanDylann]

Waiting internal engineer give a final approval

[melvin-bot]

❌ There was an error making the offer to @DylanDylann for the Reviewer role. The BZ member will need to manually hire the contributor.

[melvin-bot]

❌ There was an error making the offer to @tienifr for the Contributor role. The BZ member will need to manually hire the contributor.

[melvin-bot]

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

[jliexpensify]

Hi @muttmuure - I am headed OOO from 21st to 31st March. Here's a summary of where we are at:

• We've got @tienifr working on a PR.
• You may need to pay this out

PAYMENT SUMMARY

• C: @tienifr $500
• C+: @DylanDylann $500

UPWORKS JOB

I'll grab this one if when I'm back, thanks!

[DylanDylann]

@tienifr Is there any update here?

[tienifr]

@DylanDylann Expensify/expensify-common#669, #38908 these PRs are ready for review.

[jliexpensify]

Back, removed Matt!

[melvin-bot]

Reviewing label has been removed, please complete the "BugZero Checklist".

[melvin-bot]

The solution for this issue has been 🚀 deployed to production 🚀 in version 1.4.58-8 and is now subject to a 7-day regression period 📆. Here is the list of pull requests that resolve this issue:

• Fix: A new line is added after backticks when saving unchanged task description with code fence #38908

If no regressions arise, payment will be issued on 2024-04-09. 🎊

For reference, here are some details about the assignees on this issue:

• @tienifr requires payment (Needs manual offer from BZ)
• @DylanDylann requires payment (Needs manual offer from BZ)

[melvin-bot]

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:

• [@DylanDylann] The PR that introduced the bug has been identified. Link to the PR:
• [@DylanDylann] 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:
• [@DylanDylann] 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:
• [@DylanDylann] Determine if we should create a regression test for this bug.
• [@DylanDylann] 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.
• [@jliexpensify] Link the GH issue for creating/updating the regression test once above steps have been agreed upon:

[DylanDylann]

BugZero Checklist:

• The PR that introduced the bug has been identified. Link to the PR: N/A
• 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:

1. Create the task without description
2. Edit a task description with code fence without line break

```task description```

3. Save the description
4. Verify the updated description to system message is correct
5. Open task description edit page again
6. Repeat step 2 and 3 several times
7. Verify that no extra line break is added each time saving the description
8. Verify that no new updated the description to system message is generated

Do we 👍 or 👎

[jliexpensify]

PAYMENT SUMMARY

• C: @tienifr $500
• C+: @DylanDylann $500

UPWORKS JOB

[melvin-bot]

Payment Summary

Upwork Job

• ROLE: @tienifr paid $(AMOUNT) via Upwork (LINK)
• ROLE: @DylanDylann paid $(AMOUNT) via Upwork (LINK)

BugZero Checklist (@jliexpensify)

• I have verified the correct assignees and roles are listed above and updated the neccesary manual offers
• I have verified that there are no duplicate or incorrect contracts on Upwork for this job (https://www.upwork.com/ab/applicants/1767765440789991424/hired)
• I have paid out the Upwork contracts or cancelled the ones that are incorrect
• I have verified the payment summary above is correct

[jliexpensify]

Paid and job closed