Skip to content

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

Closed

Description

@kbecciv

If you haven’t already, check out our contributing guidelines for onboarding and email contributors@expensify.com 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

Activity

  1. added
    ExternalAdded to denote the issue can be worked on by a contributor
    on Sep 13, 2023
  2. DylanDylann commented on Sep 13, 2023

    @DylanDylann
    Contributor

    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:
      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
  3. changed the title [-]DEV: Web - Draft icon show even there is not any thing in input field[/-] [+][$500] DEV: Web - Draft icon show even there is not any thing in input field[/+] on Sep 13, 2023
  4. melvin-bot commented on Sep 13, 2023

    @melvin-bot
  5. added
    Help WantedApply this label when an issue is open to proposals by contributors
    on Sep 13, 2023
  6. melvin-bot commented on Sep 13, 2023

    @melvin-bot

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

  7. melvin-bot commented on Sep 13, 2023

    @melvin-bot

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

  8. kadiealexander commented on Sep 19, 2023

    @kadiealexander
    Contributor

    Not overdue, please get your proposals in 📢

  9. needpower commented on Sep 20, 2023

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

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

    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

  10. melvin-bot commented on Sep 20, 2023

    @melvin-bot

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

  11. 29 remaining items

  12. melvin-bot commented on Oct 3, 2023

    @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 🚀

  13. added
    Awaiting PaymentAuto-added when associated PR is deployed to production
    and removed on Oct 5, 2023
  14. changed the title [-][$500] DEV: Web - Draft icon show even there is not any thing in input field[/-] [+][HOLD for payment 2023-10-12] [$500] DEV: Web - Draft icon show even there is not any thing in input field[/+] on Oct 5, 2023
  15. melvin-bot commented on Oct 5, 2023

    @melvin-bot

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

  16. melvin-bot commented on Oct 5, 2023

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

    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:

    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
  17. abdulrahuman5196 commented on Oct 10, 2023

    @abdulrahuman5196
    Contributor

    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

  18. kadiealexander commented on Oct 16, 2023

    @kadiealexander
    Contributor

    Payouts due:

    Eligible for 50% #urgency bonus? Yes

    Upwork job

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Labels

Awaiting PaymentAuto-added when associated PR is deployed to productionDailyKSv2ExternalAdded to denote the issue can be worked on by a contributor

Type

No type

Projects

No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions