[$250] [Held requests] Markdown isn't applied to the "Hold" message

[kavimuru]

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.43.0
Reproducible in staging?: y
Reproducible in production?: new feature
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. Create an IOU in a chat
2. Put it on hold
3. Format the reason with markdowns

Expected Result:

Markdown should be applied.

Actual Result:

Markdown isn't applied to the "Hold" message.

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

View all open jobs on GitHub

Upwork Automation - Do Not Edit

• Upwork Job URL: https://www.upwork.com/jobs/~011f83ef4d2ad62f49
• Upwork Job ID: 1760293330198360064
• Last Price Increase: 2024-03-25
• Automatic offers:
• hoangzinh | Reviewer | 0

[melvin-bot]

Job added to Upwork: https://www.upwork.com/jobs/~011f83ef4d2ad62f49

[melvin-bot]

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

[melvin-bot]

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

[github-actions]

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

[melvin-bot]

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

[ghost]

Proposal

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

IOU - Markdown isn't applied to the "Hold" message

What is the root cause of that problem?

The root cause of this issue is we are not passing RNMarkdownTextInput in the below-mentioned component.

https://github.com/Expensify/App/blob/20ca041595e76cdd289977e55b2af3fbb9618868/src/pages/iou/HoldReasonPage.tsx

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

We need to pass it and add it to the component so that the Markdowns are added and applied to the "Hold" message.

What alternative solutions did you explore? (Optional)

N/A

[Krishna2323]

Proposal

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

IOU - Markdown isn't applied to the "Hold" message

What is the root cause of that problem?

We are not parsing comment with using const parsedDescription = ReportUtils.getParsedComment(newValue); before making api request to HoldRequest.

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

Use ReportUtils.getParsedComment to parse comment in putOnHold util function.

Result

[flodnv]

@BartoszGrajdek @MonilBhavsar @robertjchen I see you in #33897, can you please have a look here?

[shahinyan11]

Proposal

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

What is the root cause of that problem?

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

1. Update the comment here and here with getParsedComment(comment)
2. Use RenderCommentHTML component instead Text here

<RenderCommentHTML
    source={source}
    html={fragment.text} // Or fragment.html after changes to the backend side mentioned in Step 3
/>

Screen.Recording.2024-02-21.at.19.09.13.mov

3. If you noticed in above video, the markdown disappears after the refresh. it is because there need updates in backbend too . The hold action will not contain html key in messages or originalMessage in response

What alternative solutions did you explore? (Optional)

[dragnoir]

Proposal

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

On hold message not shown as HTML

What is the root cause of that problem?

The message from description for HOLD reason is saved as simple string

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

There's 2 main areas that needs to be fixed.

1 - use ReportUtils.getParsedComment(comment)

We need to save the message as parsed before passed to Onyx here

App/src/libs/actions/IOU.ts

Line 5049 in d8361ee

function putOnHold(transactionID: string, comment: string, reportID: string) {

function putOnHold(transactionID: string, comment: string, reportID: string) {
+	const  parsedComment  =  ReportUtils.getParsedComment(comment);
    const createdReportAction = ReportUtils.buildOptimisticHoldReportAction();
-    const createdReportActionComment = ReportUtils.buildOptimisticHoldReportActionComment(comment);
+	const createdReportActionComment = ReportUtils.buildOptimisticHoldReportActionComment(parsedComment);

   // ...

    API.write(
        'HoldRequest',
        {
            transactionID,
-           comment,
+           parsedComment
            reportActionID: createdReportAction.reportActionID,
            commentReportActionID: createdReportActionComment.reportActionID,
        },
        {optimisticData, successData, failureData},
    );
}

2- Use <RenderHTML> not <Text>

After adding the ReportUtils.getParsedComment(comment) the UI will display the markdown as it it is, something like <em> text </em>, so we need to use RenderHTML component here

App/src/pages/home/report/ReportActionItem.tsx

Line 520 in d8361ee

children = <ReportActionItemBasicMessage message={translate('iou.heldRequest')} />;

We can add a new prop to ReportActionItemBasicMessage like shouldDisplayHTML or use another component tthat render comment as HTML

POC video:

20240326_111057.mp4

[shahinyan11]

Proposal

Updated

[robertjchen]

Good question, I'll update the backend to return HOLDCOMMENT as well for consistency 🤔

[hoangzinh]

Thanks for proposals, everyone.

• @godofoutcasts94 proposal and @Krishna2323 proposal found out root cause but your solutions are not completed yet. We need to use RenderHTML to render markdown as well.
• @shahinyan11 proposal is invalid because it doesn't contain RCA + re-state the problem

---

Based on this comment. I'm inclining to @dragnoir proposal. But I have a suggestion for his solution, I think he can use RenderHTML directly instead of passing a prop to ReportActionItemBasicMessage

Link to selected proposal #37015 (comment)

🎀👀🎀 C+ reviewed

[melvin-bot]

Current assignee @robertjchen is eligible for the choreEngineerContributorManagement assigner, not assigning anyone new.

[robertjchen]

Thanks for the review, the backend change should be going out later today so we can proceed here 👍

[melvin-bot]

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

Offer link
Upwork job

[melvin-bot]

📣 @dragnoir 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 📖

[dragnoir]

Upwork said "This job is no longer available." is it OK to proceed with the PR?

[hoangzinh]

I think just continue with the PR @dragnoir. The BZ team will handle it later.

[dragnoir]

PR ready for review #39452

[abekkala]

Fix: @dragnoir
PR Review: @hoangzinh

PR not merged yet

[melvin-bot]

This issue has not been updated in over 15 days. @robertjchen, @hoangzinh, @abekkala, @dragnoir eroding to Monthly issue.

P.S. Is everyone reading this sure this is really a near-term priority? Be brave: if you disagree, go ahead and close it out. If someone disagrees, they'll reopen it, and if they don't: one less thing to do!

[hoangzinh]

We're still working on the PR #39452

[trjExpensify]

I'm suggesting we close this issue, to reflect where we've landed in the PR. #40408 (comment) - any thoughts, comment there. CC: @JmillsExpensify

[JmillsExpensify]

Yes, agreed. Let's close.

[trjExpensify]

Cool, thanks ya'll!