[HOLD for payment 2023-10-12] [$500] DEV: Web - Draft icon show even there is not any thing in input field

[kbecciv]

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. Go to top most thread in LHN
2. Write something
3. Then go to second most thread in LHN
4. Here also write something
5. Then select all and erase that text
6. Then go to remaining draft message and erase all text from input field

Expected Result:

Draft icon not shows if not have any text in input field

Actual Result:

Draft icon is shows even input field is empty

Workaround:

Unknown

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: Dev 1.3.57-5.
Reproducible in staging?: no
Reproducible in production?: no
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

Screen.Recording.2023-08-28.at.10.40.07.AM.mov

Expensify/Expensify Issue URL:
Issue reported by: @niravkakadiya25
Slack conversation: https://expensify.slack.com/archives/C049HHMV9SM/p1693199645599589

View all open jobs on GitHub

Upwork Automation - Do Not Edit

• Upwork Job URL: https://www.upwork.com/jobs/~01e6da2ca5e140fdb7
• Upwork Job ID: 1701973572762148864
• Last Price Increase: 2023-09-27
• Automatic offers:
• abdulrahuman5196 | Reviewer | 26975471
• DylanDylann | Contributor | 26975480
• niravkakadiya25 | Reporter | 26975482

[DylanDylann]

Proposal

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

• Draft icon show even there is not any thing in input field

What is the root cause of that problem?

• Report.setReportWithDraft(reportID, true) is called but the empty text is saved that leads to the bug "Draft icon show even there is not anything in the input field". But why is empty text saved? It will be shown bellow:
• Currently, we are using the debounce function to save the draft comment:
  

  App/src/pages/home/report/ReportActionCompose/ComposerWithSuggestions.js

  Lines 206 to 210 in 1223e1e

  	if (shouldDebounceSaveComment) {
  	debouncedSaveReportComment(reportID, newComment);
  	} else {
  	Report.saveReportComment(reportID, newComment || '');
  	}

• The debouncedSaveReportComment function is defined as below:

const debouncedSaveReportComment = _.debounce((reportID, comment) => {
    Report.saveReportComment(reportID, comment || '');
}, 1000);

• Because all the reports share the same debounce function above - debouncedSaveReportComment, so when the user opens chat report A and types any message then quickly switches to report B and types any messages, the debouncedSaveReportComment will ignore all the calls made within that 1000ms (contains the call that made in report A). It leads to the empty draft message being saved with report A.

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

• We can create the new debouncedSaveReportComment that is used separately for each chat report and replace the debounce function mentioned in the root cause part by it:

    const saveReportComment = useCallback((reportID, newComment) => {
        Report.saveReportComment(reportID, newComment || '');
    }, []);

    const debounceSaveReportComment = useCallback(
        _.debounce((reportID, newComment) => {
            saveReportComment(reportID, newComment);
        }, 1000),
        [saveReportComment],
    );

What alternative solutions did you explore? (Optional)

• NA

Result

Screencast.from.06-09-2023.16.48.47.webm

[melvin-bot]

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

[melvin-bot]

Triggered auto assignment to @kadiealexander (External), see https://stackoverflow.com/c/expensify/questions/8582 for more details.

[melvin-bot]

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

[kadiealexander]

Not overdue, please get your proposals in 📢

[needpower]

Proposal

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

In LHN, a draft icon shows when there is no text in the input field.

What is the root cause of that problem?

The ComposerWithSuggestions component, which is responsible for input text handling, saves a draft of input text with one second debounce:

App/src/pages/home/report/ReportActionCompose/ComposerWithSuggestions.js

Lines 206 to 207 in e2a1343

	if (shouldDebounceSaveComment) {
	debouncedSaveReportComment(reportID, newComment);

App/src/libs/ComposerUtils/debouncedSaveReportComment.js

Lines 9 to 11 in 8000576

	const debouncedSaveReportComment = _.debounce((reportID, comment) => {
	Report.saveReportComment(reportID, comment || '');
	}, 1000);

The problem occurs when a user types in a message in one report chat, switches to another report and enters another message within one second. The new debounced function call happens, and the previous call becomes overridden, but a report menu item in LHN displays that it was edited.

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

Move the debounced comment save function on the component level. So, every report will have it's own instance of that function. We need to do it in ReportActionCompose (parent component) because it has some use cases with canceling the debounce:

// ReportActionCompose.js
const debouncedSaveReportComment = useMemo(function () {
  return _.debounce((reportID, comment) => {
    Report.saveReportComment(reportID, comment || '');
      }, 5000);
   }, []);
    ...
 <ComposerWithSuggestions
   ...
   debouncedSaveReportComment={debouncedSaveReportComment} // new property
 />

What alternative solutions did you explore? (Optional)

N/A

[melvin-bot]

📣 It's been a week! Do we have any satisfactory proposals yet? Do we need to adjust the bounty for this issue? 💸

[kadiealexander]

@abdulrahuman5196 any thoughts on the above?

[kadiealexander]

Bumped in Slack.

[tsa321]

Proposal

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

In LHN, a draft icon shows when there is no text in the input field.

What is the root cause of that problem?

App/src/components/LHNOptionsList/OptionRowLHN.js

Line 263 in 09a0118

{optionItem.hasDraftComment && optionItem.isAllowedToComment && (

The draft icon is shown based on hasDraft field of the report:

App/src/libs/SidebarUtils.js

Line 299 in 09a0118

result.hasDraftComment = report.hasDraft;

and immediately set when user type in composer :

App/src/pages/home/report/ReportActionCompose/ComposerWithSuggestions.js

Lines 222 to 224 in 09a0118

	if (commentRef.current.length === 0 && newComment.length !== 0) {
	Report.setReportWithDraft(reportID, true);
	}

When user type in composer and continuously typing without pause within 1 second of the next character input, the draft is not saved for the current opened report, because of the debouncedSaveReportComment in update comment. Until he stop typing for 1 second, the draft will be saved for currently opened report.
If user immediately open other report when typing continuously, the draft is saved for the next report and previously opened report draft will be empty. But the Report.setReportWithDraft(reportID, true); already executed for previous report and draft icon is shown even though the draft is empty.
We need a way to check is there is really a draft for the report in onyx after user leave the report and opens other report.

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

We could check ifReportActionsList.js reportID has been changed, and set hasDraft value based on previousReportID. In this function or making new function insideReportActionsList.js:

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

Lines 138 to 144 in 09a0118

	if (userActiveSince.current && prevReportID && prevReportID !== report.reportID) {
	userActiveSince.current = null;
	} else {
	userActiveSince.current = DateUtils.getDBTime();
	}
	prevReportID = report.reportID;
	}, [report.reportID]);

We could add check on the draft and set hasDraft value accordingly:

const draftComment = getDraftComment(prevReportID);
Report.setReportWithDraft(prevReportID, draftComment && draftComment.length > 0);

What alternative solutions did you explore? (Optional)

If the expected result is to make the current draft text input persist for the previously opened report when user switching/opening other report while typing, we could use previousReportID method in ReportActionList.js in ComposerWithSuggestion. If the current reportID of ComposerWithSuggestion has been changed, then we call the Report.saveReportComment with previousReportID param and current value of textInput to save the current draft for previousReportID. Then call Report.setReportWithDraft based on the current textInput value, then clear the textInput.

Result:

draft_icon_lhn_d.mp4

[abdulrahuman5196]

Reviewing now

[melvin-bot]

📣 It's been a week! Do we have any satisfactory proposals yet? Do we need to adjust the bounty for this issue? 💸

[kadiealexander]

@abdulrahuman5196 did you have any feedback for the proposals?

[abdulrahuman5196]

Checking again.

[abdulrahuman5196]

@DylanDylann @needpower @tsa321

Hi folks, thank you for the proposal. But I am unable to fully understand the root cause.

The display of draft icon is dependent on the hasDraft flag.

App/src/libs/SidebarUtils.js

Line 299 in 09a0118

result.hasDraftComment = report.hasDraft;

But that flag is updated via setReportWithDraft function which is not present inside the debounce function. Then why is it getting affected?

[DylanDylann]

@abdulrahuman5196 setReportWithDraft is just set: whether the report has draft or not. (Based on this value, we display the 'edit' icon in the LHN). But the draft content for each report is set in the debounce function debouncedSaveReportComment. For more detail, please check my proposal

[abdulrahuman5196]

@DylanDylann 's proposal here #27362 (comment) looks good and works well.

🎀 👀 🎀
C+ Reviewed

[melvin-bot]

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

[DylanDylann]

@tsa321 Please follow the steps to reproduce this bug which was reported by reporter. If not, maybe the above is another bug, and you can report it. Thanks. Also, the proposal just contains the RCA and the main idea to resolve the issue. Every related case and edge case will be covered in the PR

[melvin-bot]

📣 @abdulrahuman5196 🎉 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]

📣 @DylanDylann 🎉 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 📖

[melvin-bot]

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

Offer link
Upwork job

[DylanDylann]

@abdulrahuman5196 PR #28574 is ready for review

[melvin-bot]

🎯 ⚡️ Woah @abdulrahuman5196 / @DylanDylann, great job pushing this forwards! ⚡️

The pull request got merged within 3 working days of assignment, so this job is eligible for a 50% #urgency bonus 🎉

• when @DylanDylann got assigned: 2023-10-02 08:17:22 Z
• when the PR got merged: 2023-10-03 13:57:46 UTC

On to the next one 🚀

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

• Fix/27362: Draft icon show even there is not anything #28574

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

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:

• @abdulrahuman5196 requires payment offer (Reviewer)
• @DylanDylann requires payment offer (Contributor)
• @niravkakadiya25 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

[abdulrahuman5196]

The PR that introduced the bug has been identified. Link to the PR:

#25758

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

https://github.com/Expensify/App/pull/25758/files#r1352852646

Determine if we should create a regression test for this bug.
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.

No. Not beneficial for a minor bug

[kadiealexander]

Payouts due:

• Issue Reporter: $50 @niravkakadiya25
• Contributor: $500 + $250 bonus @DylanDylann
• Contributor+: $500 + $250 bonus @abdulrahuman5196

Eligible for 50% #urgency bonus? Yes

Upwork job