[HOLD for payment 2023-10-09] [HOLD for payment 2023-09-04] [HOLD 25758][$2000] App displays strong HTML tag in reply in thread first message if we keep it in edit mode and edit message in main report

[kavimuru]

If you haven’t already, check out our contributing guidelines for onboarding and email [email protected] to request to join our Slack channel!

---

Action Performed:

1. Open the app
2. Open any report
3. Send any message with ** around it to make it bold eg: hello
4. Hover and click on reply in thread
5. Edit the first message of new thread of reply in thread and keep it in edit mode
6. Open main report and edit the message on which we had clicked reply in thread
7. Again hover and click on reply in thread and observe that message now has HTML tag around it

Expected Result:

App should not display HTML tags around bold messages

Actual Result:

App display's strong HTML tag around message in reply in thread if the message is kept in edit and we edit that message in main thread

Workaround:

Can the user still use Expensify without this being fixed? Have you informed them of the workaround?

Platforms:

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

• Android / native
• Android / Chrome
• iOS / native
• iOS / Safari
• MacOS / Chrome / Safari
• MacOS / Desktop

Version Number: 1.3.32-5
Reproducible in staging?: y
Reproducible in production?: y
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
Notes/Photos/Videos: Any additional supporting documentation

strong.HTML.reply.in.thread.mp4

Desktop.2023.06.24.-.16.25.16.12.mp4

Expensify/Expensify Issue URL:
Issue reported by: @dhanashree-sawant
Slack conversation: https://expensify.slack.com/archives/C049HHMV9SM/p1687259310889789

View all open jobs on GitHub

Upwork Automation - Do Not Edit

• Upwork Job URL: https://www.upwork.com/jobs/~01ee1f1adfd50b483f
• Upwork Job ID: 1675932442345897984
• Last Price Increase: 2023-07-20
• Automatic offers:
• situchan | Reviewer | 26113077
• eh2077 | Contributor | 26113080
• dhanashree-sawant | Reporter | 26113082

[melvin-bot]

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

[melvin-bot]

Bug0 Triage Checklist (Main S/O)

• This "bug" occurs on a supported platform (ensure Platforms in OP are ✅)
• This bug is not a duplicate report (check E/App issues and #expensify-bugs)
  • If it is, comment with a link to the original report, close the issue and add any novel details to the original issue instead
• This bug is reproducible using the reproduction steps in the OP. S/O
  • If the reproduction steps are clear and you're unable to reproduce the bug, check with the reporter and QA first, then close the issue.
  • If the reproduction steps aren't clear and you determine the correct steps, please update the OP.
• This issue is filled out as thoroughly and clearly as possible
  • Pay special attention to the title, results, platforms where the bug occurs, and if the bug happens on staging/production.
• I have reviewed and subscribed to the linked Slack conversation to ensure Slack/Github stay in sync

[dylanexpensify]

reviewing today!

[dylanexpensify]

Cannot reproduce, @kavimuru can you confirm this is still happening?

[dylanexpensify]

Closing, @kavimuru feel free to open if you can repro

[eh2077]

@kavimuru @dylanexpensify I can still reproduce it on prod, staging and dev. See below video FYI

21527-dupe-steps-video.mov

Reproduce steps:

1. Click to reply in thread
2. Open the editor of the first message of the thread
3. Go back to the parent report
4. Edit the message from parent report and save it
5. Click to reply in thread again
6. Notice these steps are displayed in HTML

[eh2077]

Proposal

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

App displays strong HTML tag in reply in thread first message if we keep it in edit mode and edit message in main report

What is the root cause of that problem?

The root cause of this issue is that

1. When editing the first message of thread, we save draft message(action.message[0].html) for key determined by reportID of thread and reportActionID of parent report action, see

   App/src/pages/home/report/ContextMenu/ContextMenuActions.js

   Line 258 in 2f6002f

   const editAction = () => Report.saveReportActionDraft(reportID, reportAction.reportActionID, _.isEmpty(draftMessage) ? getActionText(reportAction) : '');

2. After editing and saving the changed message from main report action and going back to thread, the following condition

   App/src/pages/home/report/ReportActionItemMessageEdit.js

   Line 84 in 2f6002f

   if (props.draftMessage === props.action.message[0].html) {

   is false because action.message[0].html has been changed and the props.draftMessage is still same as the html of original report action. So, the draft is

   App/src/pages/home/report/ReportActionItemMessageEdit.js

   Line 90 in 2f6002f

   return Str.htmlDecode(props.draftMessage);

   which displayes html tag if the original props.action.message[0].html is not plain text. Note that props.draftMessage which is saved in Step 1 is read from here through reportID of thread and reportActionID of parent report action.

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

To fix this issue, we should save and read draftMessage for the first message of a thread using reportID of parent report and reportActionID of parent report action. The first message of thread already uses the same report action of parent report. So, we only need to fix the reportID.

To achieve it, we can

1. Change the second parameter of method saveReportActionDraft from reportActionID to reportAction and use to method ReportUtils.getOriginalReportID to get the right reportID. The changed method saveReportActionDraft will be like

   /**
    * Saves the draft for a comment report action. This will put the comment into "edit mode"
    *
    * @param {String} reportID
    * @param {Number} reportAction
    * @param {String} draftMessage
    */
   function saveReportActionDraft(reportID, reportAction, draftMessage) {
       const originalReportID = ReportUtils.getOriginalReportID(reportID, reportAction);
       Onyx.set(`${ONYXKEYS.COLLECTION.REPORT_ACTIONS_DRAFTS}${originalReportID}_${reportAction.reportActionID}`, draftMessage);
   }

2. Change all usages of Report.saveReportActionDraft to pass reportAction instead of reportActionID

3. Read draft message from correct reportID by changing this line

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

   Line 523 in 4bd1848

   const draftKey = `${ONYXKEYS.COLLECTION.REPORT_ACTIONS_DRAFTS}${props.report.reportID}_${props.action.reportActionID}`;

   to

   const originalReportID = ReportUtils.getOriginalReportID(props.report.reportID, props.action);
   const draftKey = `${ONYXKEYS.COLLECTION.REPORT_ACTIONS_DRAFTS}${originalReportID}_${props.action.reportActionID}`;

4. Add an effect to handle case of going back by browser back button and deleting action by saving empty draft. We can add following effect below this line

    useEffect(() => {
        // We want to early return if the action is deleted or it's an initial draft
        if (ReportActionsUtils.isDeletedAction(props.action) || props.draftMessage === props.action.message[0].html) {
            return;
        }
        setDraft(Str.htmlDecode(props.draftMessage));
    }, [props.draftMessage, props.action.message]);

After fixing, the editing state of the first message of a thread and the message from parent report will be synced.

Click to see demo video

Screen.Recording.2023-07-01.at.12.25.15.AM.mov

What alternative solutions did you explore? (Optional)

N/A

[kavimuru]

@dylanexpensify Issue is still reproduced by Dhanashree and Eric

21527-dupe-steps-video.mov

Screen.Recording.2023-07-03.at.8.42.01.PM.mov

[dylanexpensify]

reviewing

[dylanexpensify]

Confirmed!

[melvin-bot]

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

[melvin-bot]

Current assignee @dylanexpensify is eligible for the External assigner, not assigning anyone new.

[melvin-bot]

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

[dylanexpensify]

No proposals, doubling

[dylanexpensify]

@techievivek mind if we get an update today/tomorrow? 🙇‍♂️

[techievivek]

Sorry for the delay here. Looks like we have already created a separate issue for the above regression so we can just focus on original issue here. @eh2077 @situchan

[dylanexpensify]

@situchan mind giving us an update today?

[situchan]

waiting for PR from @eh2077

[dylanexpensify]

@eh2077 mind updating?

[eh2077]

@dylanexpensify Sorry for the delay! I'll submit the PR today.

[eh2077]

@situchan Please kindly help to review the PR #28154 when you get time, thanks!

[melvin-bot]

Based on my calculations, the pull request did not get merged within 3 working days of assignment. Please, check out my computations here:

• when @eh2077 got assigned: 2023-08-14 14:11:43 Z
• when the PR got merged: 2023-09-27 08:14:17 UTC
• days elapsed: 31

On to the next one 🚀

[dylanexpensify]

Payment summary:

• Reporter: @dhanashree-sawant $250
• Contributor: @eh2077 $2,000
• Contributor+: @situchan $2,000

[dylanexpensify]

Payments sent! @situchan can we get BZ checklist done

[situchan]

We never synced editing status of parent message and thread first message from the beginning.

Regression Test Proposal

1. Go to a chat and add a comment, like *hello*
2. Click Reply in thread and click to edit the first message of the thread
3. Verify that the draft of the first message in thread is *hello*
4. Click thread title to go back to parent chat
5. Verify that the parent message is in editing mode and the draft is *hello*
6. Change the draft to *hello* hi, save it and click Reply in thread
7. Verify that the first message of thread is updated same as well and not in editing mode.

[situchan]

Should I propose regression test for #27605 since our PR fixed that bug as well?

[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.3.75-12 and is now subject to a 7-day regression period 📆. Here is the list of pull requests that resolve this issue:

• fix: sync draft and edit status of thread first chat #28154

If no regressions arise, payment will be issued on 2023-10-09. 🎊

After the hold period is over and BZ checklist items are completed, please complete any of the applicable payments for this issue, and check them off once done.

• External issue reporter
• Contributor that fixed the issue
• Contributor+ that helped on the issue and/or PR

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

• @situchan requires payment offer (Reviewer)
• @eh2077 requires payment offer (Contributor)
• @dhanashree-sawant requires payment offer (Reporter)

As a reminder, here are the bonuses/penalties that should be applied for any External issue:

• Merged PR within 3 business days of assignment - 50% bonus
• Merged PR more than 9 business days after assignment - 50% penalty

[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:

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

[dylanexpensify]

yes please @situchan, just in that issue!