[HOLD for payment 2024-07-24] [$250] Empty Entry in LHN with RBR and "Hmm.. It's not there message when clicked

[m-natarajan]

If you haven’t already, check out our contributing guidelines for onboarding and email contributors@expensify.com to request to join our Slack channel!

---

Version Number:
Reproducible in staging?: Needs reproduction
Reproducible in production?: Needs reproduction
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: @iwiznia
Slack conversation: https://expensify.slack.com/archives/C049HHMV9SM/p1720455609916209

Action Performed:

1. Go to staging.new.expensify.com
2. Observe LHN

Expected Result:

All reports or IOU displayed with username and message

Actual Result:

One empty entry observed in LHN with RBR and when clicked displays "Hmm... Its not here" 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

ReportID Is 1915196065459787

Logs: {
"reportID": "3916529629983126",
"preexistingReportID": "65121290",
"lastReadTime": "2024-07-02 18:08:57.011",
"errorFields": {
"notFound": {
"1719943737548592": "Report not found",
"1720459088807020": "Report not found",
"1720459290281777": "Report not found"
}
},
"participants": {},
"pendingFields": {},
"isOptimisticReport": false
}

Got preexisting report from Auth::openReport, new report was not created ~~ optimisticReportID: '3916529629983126' preexistingReportID: '65121290'
Add any screenshot/video evidence

View all open jobs on GitHub

Upwork Automation - Do Not Edit

• Upwork Job URL: https://www.upwork.com/jobs/~01ae00b986e1d955ec
• Upwork Job ID: 1810728317314423962
• Last Price Increase: 2024-07-09
• Automatic offers:
• brunovjk | Reviewer | 103066501
• dominictb | Contributor | 103066502

Issue Owner

Current Issue Owner: @abekkala / @abekkala

[MelvinBot]

This has been labelled "Needs Reproduction". Follow the steps here: https://stackoverflowteams.com/c/expensify/questions/16989

[melvin-bot]

Current assignee @iwiznia is eligible for the AutoAssignerNewDotQuality assigner, not assigning anyone new.

[melvin-bot]

Triggered auto assignment to @abekkala (Bug), see https://stackoverflow.com/c/expensify/questions/14418 for more details. Please add this bug to a GH project, as outlined in the SO.

[melvin-bot]

Triggered auto assignment to @jasperhuangg (AutoAssignerNewDotQuality)

[iwiznia]

@m-natarajan, you should not assign the reporter to these issues, it will get someone auto assigned if the label AutoAssignerNewDotQuality is added (that's why I removed my assignment and triggered the auto assigner again)

[puneetlath]

The issue here seems to be that the client isn't clearing out the optimistic report for some reason when the back-end returns the pre-existing report. I have four of these reports where they show up in the LHN like this, but when you click on them, you get "report not found".

In all four scenarios, there is a preestingReportID in the Onyx object for the report. Example:

{
    "reportID": "3916529629983126",
    "preexistingReportID": "65121290",
    "lastReadTime": "2024-07-02 18:08:57.011",
    "errorFields": {
        "notFound": {
            "1719943737548592": "Report not found",
            "1720459088807020": "Report not found",
            "1720459290281777": "Report not found"
        }
    },
    "participants": {},
    "pendingFields": {},
    "isOptimisticReport": false
}

This indicates that this was an optimistic report that got created and the back-end responded to OpenReport by setting the preexisting report field. At that point, this code or this code should have kicked in to clear the report out of Onyx. For some reason that didn't happen.

I think what we should potentially do is never show reports that have preexistingReportIDs in the LHN as there is no reason for them to be there.

[iwiznia]

I think what we should potentially do is never show reports that have preexistingReportIDs in the LHN as there is no reason for them to be there.

This makes sense, but I think I first would want to know why the existing code we have is not working, since as you mention, this allegedly should be handled already.

[puneetlath]

Perhaps the problem is that every time it gets navigated to we are adding another not found item to the error fields object

"errorFields": {
        "notFound": {
            "1719943737548592": "Report not found",
            "1720459088807020": "Report not found",
            "1720459290281777": "Report not found"
        }
    },

And that's what makes it so that it stays in the LHN and is never able to be cleared.

[melvin-bot]

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

[melvin-bot]

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

[muttmuure]

@iwiznia I asked @m-natarajan to assign the reporter because they have the most context for the bug and also need to confirm that the bug is fixed.

If you don't want to do that, that's fine, but it would be more helpful to the project if you did.

[brunovjk]

I haven't been able to reproduce it yet.

[dominictb]

Proposal

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

One empty entry observed in LHN with RBR and when clicked displays "Hmm... Its not here" message

What is the root cause of that problem?

The callback to clear the duplicate report is

App/src/libs/actions/Report.ts

Line 1323 in c973e62

Onyx.set(`${ONYXKEYS.COLLECTION.REPORT}${report.reportID}`, null);

It's unfortunately only triggered here

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

Line 356 in c973e62

callback();

if we at the moment is opening that optimistic report (and the composer is rendered). If we're not opening that report, there'll be no listener to the event emitter here and the dup report clearance will never happen.

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

We should not rely on the event emitter's rendering for the dup report clearance.

So we'll use another mechanism to save the draft comment as intended here.

• In

  App/src/libs/actions/Report.ts

  Line 1333 in c973e62

  DeviceEventEmitter.emit(`switchToPreExistingReport_${report.reportID}`, {

  remove the event emitter, and remove the event listener in ComposerWithSuggestions too.
• There, get the report draft comment of reportID, we can create a reportDraftComments local variable to store the draft comments in Onyx that's from reportDraftComment_ collection that we'll connect to, then we can get it by key reportID. Or can use Onyx.get(...).then(...

let allReportDraftComments = {};

Onyx.connect({
    key: ONYXKEYS.COLLECTION.REPORT_DRAFT_COMMENT,
    waitForCollectionCallback: true,
    callback: (value) => {
        allReportDraftComments = value;
    },
});

• If the draft comment doesn't exist, call callback and early return (as we intended here)
• If the draft comment exists, save it to the preexistingReportID like here

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

  Line 359 in c973e62

  Report.saveReportDraftComment(preexistingReportID, commentRef.current, callback);

const draftComment = allReportDraftComments[`${ONYXKEYS.COLLECTION.REPORT_DRAFT_COMMENT}${report.reportID}`];

if (!draftComment) {
    callback();
    return;
}

saveReportDraftComment(report.preexistingReportID, draftComment, callback);

So the difference in approaches here is we don't rely on component rendering and event emitter but we'll update the data in Onyx directly

In the callback we can clear the REPORT_DRAFT_COMMENT of report.reportID too

What alternative solutions did you explore? (Optional)

If the user is on the dupe report route (using this check), use EventEmitter as existing implementation, otherwise use the above direct Onyx get and save implementation.

[dominictb]

Proposal updated to add some example changes

[dominictb]

Could you please share a test branch with your solution? Thank you :D

@brunovjk Of course, here's the test branch main...dominictb:epsf-app:fix/45035

[dominictb]

What will happen after your changes? Will it show in my LHN? And if I click on it, will it get cleared?

@puneetlath It will get cleared the next time you reload the site (no need to click on it), because handleReportChanged will run for every report upon first Onyx data load.

[brunovjk]

@brunovjk Of course, here's the test branch main...dominictb:epsf-app:fix/45035

Thanks @dominictb, I tested it look good!

I think what we should potentially do is never show reports that have preexistingReportIDs in the LHN as there is no reason for them to be there.

@puneetlath, do you think implementing this would be a good safety measure?

[melvin-bot]

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

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

[puneetlath]

Ok let's go with @dominictb. Can you raise a PR today?

@puneetlath, do you think implementing this would be a good safety measure?

I kind of think yes. But maybe it's better to start with these changes and see where they get us. What do you think?

[brunovjk]

Sounds great 😃

[dominictb]

Working on this now!

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

• fix: clear duplicated report data #45295

If no regressions arise, payment will be issued on 2024-07-24. 🎊

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

• @brunovjk requires payment automatic offer (Reviewer)
• @dominictb requires payment automatic offer (Contributor)

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

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

[abekkala]

PAYMENT SUMMARY FOR JULY 24

• Fix: @dominictb [$250, if no regressions] offer
• PR Review: @brunovjk [$250, if no regressions] offer

[abekkala]

@brunovjk please complete checklist above prior to payment date - thanks!

[brunovjk]

• [@brunovjk] The PR that introduced the bug has been identified. Link to the PR: N/A
• [@brunovjk] 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
• [@brunovjk] 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
• [@brunovjk] Determine if we should create a regression test for this bug. No
• [@brunovjk] 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. N/A

[abekkala]

@dominictb and @brunovjk payments sent and contracts ended - thank you! 🎉