Expensify / App

Welcome to New Expensify: a complete re-imagination of financial collaboration, centered around chat. Help us build the next generation of Expensify by sharing feedback and contributing to the code.
https://new.expensify.com
MIT License
3.52k stars 2.87k forks source link

[$250] Inbox showing GBR when there’s a report with RBR #50927

Open m-natarajan opened 2 weeks ago

m-natarajan commented 2 weeks ago

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.49-0 Reproducible in staging?: y Reproducible in production?: y If this was caught on HybridApp, is this reproducible on New Expensify Standalone?: 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: @pac-guerreiro Slack conversation: https://expensify.slack.com/archives/C049HHMV9SM/p1728665408536049

Action Performed:

  1. Make sure there is at least a report with GBR and another with a RBR
  2. Select a report which hasn’t a RBR indicator
  3. Go to Troubleshoot, then click on Clear cache and restart
  4. Go back to Inbox and check that the Inbox is showing a GBR indicator
  5. Open the report that has the RBR indicator then refresh the page

Expected Result:

Should have same logic that should display RBR in inbox

Actual Result:

The inbox indicator doesn’t show an updated color until the page is refreshed and it shows the wrong color until a report is fully loaded

Workaround:

unknown

Platforms:

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

Screenshots/Videos

https://github.com/user-attachments/assets/287d512e-03f5-4ff2-80fd-8e590c91596e

https://github.com/user-attachments/assets/791a3300-0f7a-49dc-b7f5-f1d004ce0958

Add any screenshot/video evidence

View all open jobs on GitHub

Upwork Automation - Do Not Edit
  • Upwork Job URL: https://www.upwork.com/jobs/~021847128366508796109
  • Upwork Job ID: 1847128366508796109
  • Last Price Increase: 2024-10-18
  • Automatic offers:
    • mkhutornyi | Reviewer | 104590221
    • truph01 | Contributor | 104590223
Issue OwnerCurrent Issue Owner: @mkhutornyi
melvin-bot[bot] commented 2 weeks ago

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

FitseTLT commented 2 weeks ago

Proposal

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

Inbox showing GBR when there’s a report with RBR

What is the root cause of that problem?

We are only recalculating chat tab rbr value for transactionViolation and activeWorkspaceID changes here https://github.com/Expensify/App/blob/4a25c36e50b58ef9e5ba8b63cf32132f95a0b005/src/libs/Navigation/AppNavigator/createCustomBottomTabNavigator/BottomTabBar.tsx#L72-L73 But inside getChatTabBrickRoad we use the status of allReports to calculate the value of rbr. But, allReports will not be available immediately after a user clears caches/onyx data so the rbr value will be calculated without the list of reports which will result wrong value and there is no recalculation held when the reports are loaded in onyx so it will remain green until we reload the page

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

We should subscribe to reports collection and use it in the calculation of rbr value https://github.com/Expensify/App/blob/4a25c36e50b58ef9e5ba8b63cf32132f95a0b005/src/libs/Navigation/AppNavigator/createCustomBottomTabNavigator/BottomTabBar.tsx#L69-L73 https://github.com/Expensify/App/blob/4a25c36e50b58ef9e5ba8b63cf32132f95a0b005/src/libs/Navigation/AppNavigator/createCustomPlatformStackBottomTabNavigator/BottomTabBar.tsx#L76-L79

    const [allReports] = useOnyx(ONYXKEYS.COLLECTION.REPORT);
    const [chatTabBrickRoad, setChatTabBrickRoad] = useState<BrickRoad>(getChatTabBrickRoad(activeWorkspaceID, allReports));

    useEffect(() => {
        setChatTabBrickRoad(getChatTabBrickRoad(activeWorkspaceID,allReports));
    }, [activeWorkspaceID, transactionViolations,allReports]);

https://github.com/Expensify/App/blob/4a25c36e50b58ef9e5ba8b63cf32132f95a0b005/src/libs/WorkspacesSettingsUtils.ts#L122-L123

function getChatTabBrickRoad(policyID?: string, allReportsParam): BrickRoad | undefined {
    const allReports = allReportsParam ?? ReportConnection.getAllReports();

It's not a must to use the useOnyx allreports data in getChatTabBrickRoad to fix the issue; only adding allReports to useEffect's dependency suffices, but I think it is generally accepted approach to use it

What alternative solutions did you explore? (Optional)

pac-guerreiro commented 2 weeks ago

I just want to clarify that there are two problems here:

  1. Inbox indicator is not updated in realtime
  2. Inbox indicator is green when there are reports with RBR until at least one RBR is opened manually by the user

To replicate the issue you can also do a fresh login instead of clearing onyx cache.

I had to fix the 1st problem on one of my debug mode PRs for it to work properly but the PR hasn't been merged yet (link to the fix).

If you agree, this bug could be just focused on the 2nd problem. Otherwise I'll discard the fix I did on my PR.

truph01 commented 2 weeks ago

Edited by proposal-police: This proposal was edited at 2024-10-17 09:47:30 UTC.

Proposal

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

Inbox has no GBR or RBR indicator until the page is refreshed

What is the root cause of that problem?

https://github.com/Expensify/App/blob/77a37a101ee8776f9570842e32329ecb98deb94b/src/libs/WorkspacesSettingsUtils.ts#L66-L78

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

    const parentReportActions = (altReportActions ?? allReportActions)?.[`${ONYXKEYS.COLLECTION.REPORT_ACTIONS}${report?.parentReportID}`];
    const parentReportAction = parentReportActions?.[report?.parentReportActionID ?? '-1'];
    const shouldDisplayViolations = ReportUtils.shouldDisplayTransactionThreadViolations(report, allTransactionViolations, parentReportAction);
    const shouldDisplayReportViolations = ReportUtils.isReportOwner(report) && ReportUtils.hasReportViolations(report.reportID);
    const hasViolations = shouldDisplayViolations || shouldDisplayReportViolations;    
    if (hasViolations) {
        doesReportContainErrors = CONST.BRICK_ROAD_INDICATOR_STATUS.ERROR;
    }

https://github.com/Expensify/App/blob/bcc62638a21ccbe717fb25abb983aac86672a8a2/src/libs/Navigation/AppNavigator/createCustomBottomTabNavigator/BottomTabBar.tsx#L71-L73

    const report = useOnyx(ONYXKEYS.COLLECTION.REPORT);
    const reportActions = useOnyx(ONYXKEYS.COLLECTION.REPORT_ACTIONS);
    const [chatTabBrickRoad, setChatTabBrickRoad] = useState<BrickRoad>(getChatTabBrickRoad(activeWorkspaceID));

    useEffect(() => {
        setChatTabBrickRoad(getChatTabBrickRoad(activeWorkspaceID));
    }, [activeWorkspaceID, transactionViolations, report, reportActions]);

to make sure the reports, reportActions data are updated immediately.

What alternative solutions did you explore? (Optional)

melvin-bot[bot] commented 2 weeks ago

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

melvin-bot[bot] commented 2 weeks ago

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

alexpensify commented 2 weeks ago

@mkhutornyi - can you please identify if one of these proposals will fix this issue? Thanks!

Heads up, I will be offline until Tuesday, October 22, 2024, and will not actively watch over this GitHub during that period.

If this GitHub requires an urgent update, please ask for help in the #expensify-open-source Slack Room. If the inquiry can wait, I'll review it when I return online.

FitseTLT commented 2 weeks ago

The issue is very similar to https://github.com/Expensify/App/issues/50364

truph01 commented 2 weeks ago

As mentioned by @pac-guerreiro, this bug is to fix:

Inbox indicator is green when there are reports with RBR until at least one RBR is opened manually by the user

melvin-bot[bot] commented 2 weeks ago

@alexpensify, @mkhutornyi Uh oh! This issue is overdue by 2 days. Don't forget to update your issues!

mkhutornyi commented 2 weeks ago

reviewing 2 proposals

mkhutornyi commented 1 week ago

Inbox indicator is not updated in realtime

This was already fixed in https://github.com/Expensify/App/pull/49962

Inbox indicator is green when there are reports with RBR until at least one RBR is opened manually by the user

This is bug to fix here.

@truph01 can you please explain why the bug is gone after at least one RBR is opened manually in RCA?

truph01 commented 1 week ago

can you please explain why the bug is gone after at least one RBR is opened manually in RCA?

As you can see, we are only considering the case the report is one transaction report: https://github.com/Expensify/App/blob/77a37a101ee8776f9570842e32329ecb98deb94b/src/libs/WorkspacesSettingsUtils.ts#L66-L78

And because the one transaction report data can only be available after we call OpenReport for the expense report, so the bug is gone after at least one RBR is opened.

mkhutornyi commented 1 week ago

And because the one transaction report data can only be available after we call OpenReport for the expense report, so the bug is gone after at least one RBR is opened.

@truph01 I am confused. Are you saying that this bug happens only when one transaction report has RBR?

truph01 commented 1 week ago

Are you saying that this bug happens only when one transaction report has RBR

No, that's not what I meant. To clarify, the bug occurs in both cases—whether it's a single transaction report or not. However, when it's a single transaction report, the bug disappears after visiting the expense report page, which is what we are discussing.

mkhutornyi commented 1 week ago

ok so if it's not a single transaction report, bug won't disappear even after refresh?

truph01 commented 1 week ago

ok so if it's not a single transaction report, bug won't disappear even after refresh?

Yes. If it's not a single transaction report, bug won't disappear even after refresh or visit any RBR report.

mkhutornyi commented 1 week ago

@truph01's proposal looks good to me. 🎀👀🎀 C+ reviewed

melvin-bot[bot] commented 1 week ago

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

melvin-bot[bot] commented 1 week ago

📣 @mkhutornyi 🎉 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[bot] commented 1 week ago

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

alexpensify commented 1 week ago

@truph01 - any update for the PR here? Thanks!

melvin-bot[bot] commented 1 week ago

@tgolen, @alexpensify, @mkhutornyi, @truph01 Whoops! This issue is 2 days overdue. Let's get this updated quick!