[$250] 3 OpenReport API calls for the same report

[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: 9.0.41-6 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 Expensify/Expensify Issue URL: Issue reported by: @iwiznia Slack conversation: https://expensify.slack.com/archives/C049HHMV9SM/p1727446418470879

Action Performed:

1. User A and user B are on a chat
2. User A adds a comment
3. User A creates a thread under their comment
4. User B clicks the thread

Expected Result:

Thread opens and no duplicate OpenReport API calls for same report

Actual Result:

3 OpenReport API calls for the same report

Workaround:

Unknown

Platforms:

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

• [ ] Android: Native
• [ ] Android: mWeb Chrome
• [ ] iOS: Native
• [ ] iOS: mWeb Safari
• [x] 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/~021841330786260257123
• Upwork Job ID: 1841330786260257123
• Last Price Increase: 2024-10-09

Issue Owner

Current Issue Owner: @shubham1206agra

[melvin-bot[bot]]

Triggered auto assignment to @VictoriaExpensify (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.

[bernhardoj]

Proposal

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

There are 2 OpenReport calls when open a thread that we never loaded yet.

What is the root cause of that problem?

The 1st OpenReport is called in this effect. The purpose of this effect is to call the OpenReport on mount, but we only want to call it when the report onyx is already loaded. We did this after migrating from withOnyx to useOnyx here. https://github.com/Expensify/App/blob/69483ff0ef36e39fbf40c15f2caa3471dccbef17/src/pages/home/ReportScreen.tsx#L489-L496

The 2nd and 3rd call is triggered from this effect. https://github.com/Expensify/App/blob/69483ff0ef36e39fbf40c15f2caa3471dccbef17/src/pages/home/ReportScreen.tsx#L577-L588

When we open the thread that we haven't loaded yet, the report will initially be undefined. The 1st OpenReport will optimistically merge the reportName as Chat Report. https://github.com/Expensify/App/blob/69483ff0ef36e39fbf40c15f2caa3471dccbef17/src/libs/actions/Report.ts#L779-L783

This will update the report to {reportName: 'Chat Report'}, and so, the report onyx now contains an empty reportID. https://github.com/Expensify/App/blob/69483ff0ef36e39fbf40c15f2caa3471dccbef17/src/pages/home/ReportScreen.tsx#L169-L173

Because onyxReportID (empty string) is different from prevReport?.reportID (undefined), the 2nd OpenReport is called. After the 1st OpenReport is finished, the onyxReportID is updated to the correct reportID (X). Again, because the onyxReportID (X) is different from prevReport?.reportID (empty string), the 3rd OpenReport is called.

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

There are 2 solutions. First, if we see the comment here, https://github.com/Expensify/App/blob/69483ff0ef36e39fbf40c15f2caa3471dccbef17/src/pages/home/ReportScreen.tsx#L577-L585

it's mentioned that when deeplinking from a report to another report, the report screen is not unmounted and the reportID param is not updated, so the old conditions doesn't work when comparing prev reportID params to the current reportID params.

The report screen is not unmounted part is still true today because if we deeplink to the same screen with a different params, react-navigation will just update the screen params, so this part isn't true anymore.

the reportID in the route also doesn't change.

So, the first solution is to use the old conditions by comparing the prev reportID params and the current reportID params.

if (prevReportIDFromRoute === reportIDFromRoute) {
    return;
}

The second solution is to push a new screen if the reportID is different. To do that, we can override the deeplink here. https://github.com/Expensify/App/blob/69483ff0ef36e39fbf40c15f2caa3471dccbef17/src/libs/Navigation/linkingConfig/subscribe/index.native.ts#L21-L29

if (focusedRoute?.name === SCREENS.REPORT && hasAuthToken()) {
    Navigation.navigate(path as Route);
    return;
}

Then, we can remove the OpenReport call here. https://github.com/Expensify/App/blob/69483ff0ef36e39fbf40c15f2caa3471dccbef17/src/pages/home/ReportScreen.tsx#L577-L585

[melvin-bot[bot]]

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

[melvin-bot[bot]]

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

[VictoriaExpensify]

I think #vip-vsb is the best place for this since it's related to threads

[melvin-bot[bot]]

@VictoriaExpensify, @shubham1206agra Huh... This is 4 days overdue. Who can take care of this?

[melvin-bot[bot]]

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

[iwiznia]

@shubham1206agra can you review @bernhardoj's proposal or should we reassign this?

[shubham1206agra]

@bernhardoj Your RCA looks correct but I don't think your solution is addressing the RCA.

[bernhardoj]

In my root cause, I explained the unnecessary calls are coming from here, https://github.com/Expensify/App/blob/2987fc0562f2c83852b9df19f9cef36ecdf4d621/src/pages/home/ReportScreen.tsx#L582-L587

which I offer 2 solutions to solve it. Do you care to explain why you think the solution doesn't address the RCA?

[shubham1206agra]

@bernhardoj Can you address this?

Because onyxReportID (empty string) is different from prevReport?.reportID (undefined), the 2nd OpenReport is called.

[bernhardoj]

That specifically happens after https://github.com/Expensify/App/pull/49056 where they add a check for the report here. https://github.com/Expensify/App/blob/f0be8a731dc0832827dbd14d23fd49b332ff275e/src/pages/home/ReportScreen.tsx#L169-L173

So, previously, even when the report is undefined, the reportID will still be an empty string, so the 2nd OpenReport isn't called.

Again, because the onyxReportID (X) is different from prevReport?.reportID (empty string), the 3rd OpenReport is called.

But I tried reverting it locally and the 3rd OpenReport is still called when the reportID changes from empty string to the correct reportID. That's why I didn't talk about that since the multiple OpenReport will still happen.