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.36k stars 2.79k forks source link

[HOLD for Payment 2024-08-22][$250] Search - Expense RHP closes after clicking on the From subtitle link #46518

Closed lanitochka17 closed 1 month ago

lanitochka17 commented 2 months 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.14-2 Reproducible in staging?: Y Reproducible in production?: Y If this was caught during regression testing, add the test name, ID and link from TestRail: N/A Issue reported by: Applause - Internal Team

Action Performed:

  1. Go to staging.new.expensify.com
  2. Go to any chat
  3. Submit an expense
  4. Go to Search
  5. Click on the expense submitted in Step 3
  6. Click on the RHP header
  7. Click on the From subtitle link

Expected Result:

App will open the report from the subtitle link

Actual Result:

The expense RHP closes after clicking on the From subtitle link

Workaround:

Unknown

Platforms:

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

Screenshots/Videos

Add any screenshot/video evidence

https://github.com/user-attachments/assets/719753a0-3af0-46ec-9187-cf3fca3ada25

View all open jobs on GitHub

Upwork Automation - Do Not Edit
  • Upwork Job URL: https://www.upwork.com/jobs/~01d7addf8985df968d
  • Upwork Job ID: 1818576628459044491
  • Last Price Increase: 2024-07-31
  • Automatic offers:
    • nkdengineer | Contributor | 103461487
Issue OwnerCurrent Issue Owner: @fedirjh
melvin-bot[bot] commented 2 months ago

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

lanitochka17 commented 2 months ago

@jliexpensify FYI I haven't added the External label as I wasn't 100% sure about this issue. Please take a look and add the label if you agree it's a bug and can be handled by external contributors

nkdengineer commented 2 months ago

Proposal

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

The expense RHP closes after clicking on the From subtitle link

What is the root cause of that problem?

When we open the report detail page from RHP Report Screen and click on the parent subtitle, we use goBack to navigate back to the report screen but it doesn't work as expected.

https://github.com/Expensify/App/blob/8e79c59615feb7de82e4aaa42d8b2297542f5b3b/src/components/ParentNavigationSubtitle.tsx#L44-L47

We only navigate back to the fallbackRoute if isFirstRouteInNavigator is true or we're in the central panel. In this case, the RHP stack has two screens RHP ReportScreen --> Report Detail Page so isFirstRouteInNavigator is false at both times we call goBack (we call goBack two times immediately so navigationRef.current.getState() is the same) and then we only go back to the previous screen two times Report Detail Page --> RHP ReportScreen --> Search Page. https://github.com/Expensify/App/blob/8e79c59615feb7de82e4aaa42d8b2297542f5b3b/src/libs/Navigation/Navigation.ts#L211

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

We should pass shouldEnforceFallback param as true here to make sure we will use the fallbackRoute when we call goBack the second time.

https://github.com/Expensify/App/blob/8e79c59615feb7de82e4aaa42d8b2297542f5b3b/src/components/ParentNavigationSubtitle.tsx#L47

What alternative solutions did you explore? (Optional)

We can use Navigation.isNavigationReady() to wait until the first goBack is complete before calling goBack the second time. With this at the second time the RHP stack has only RHP ReportScreen and then isFirstRouteInNavigator is true that leads we navigate to the fallbackRoute

// Pop the thread report screen before navigating to the chat report.
Navigation.goBack(ROUTES.REPORT_WITH_ID.getRoute(parentReportID));
if (isVisibleAction && !isOffline) {
    // Pop the chat report screen before navigating to the linked report action.
    Navigation.isNavigationReady().then(() => {
        Navigation.goBack(ROUTES.REPORT_WITH_ID.getRoute(parentReportID, parentReportActionID));
    });
}

https://github.com/Expensify/App/blob/8e79c59615feb7de82e4aaa42d8b2297542f5b3b/src/components/ParentNavigationSubtitle.tsx#L44-L47

melvin-bot[bot] commented 2 months ago

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

jliexpensify commented 2 months ago

I'm able to repro this

melvin-bot[bot] commented 2 months ago

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

fedirjh commented 2 months ago

I am unable to replicate in the latest main

https://github.com/user-attachments/assets/d5626510-2a5e-4170-8a04-969ee6937715

nkdengineer commented 2 months ago

@fedirjh I'm still able to reproduce this on the latest main bug by disabling the strict mode. Correct me if I missed something.

https://github.com/user-attachments/assets/a08c8aeb-c6f7-4408-b94e-6ec59964c6b0

melvin-bot[bot] commented 1 month ago

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

fedirjh commented 1 month ago

Thank you, @nkdengineer, for the suggestion. I managed to reproduce the issue on the dev by disabling strict mode. The proposed solution also seems satisfactory to me, so let's move forward with implementing the main solution.

Let's proceed with @nkdengineer's proposal;

πŸŽ€ πŸ‘€ πŸŽ€ C+ reviewed

melvin-bot[bot] commented 1 month ago

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

nkdengineer commented 1 month ago

@NikkiWines Please help to take a look when you have a chance, thanks.

NikkiWines commented 1 month ago

Sorry for the delay, agreed that @nkdengineer's proposal looks good πŸ‘

melvin-bot[bot] commented 1 month ago

πŸ“£ @nkdengineer πŸŽ‰ 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 πŸ“–

fedirjh commented 1 month ago

BugZero Checklist:


Regression Test Proposal

Submit an expense
Go to Search
Click on the expense submitted in Step 3
Click on the RHP header
Click on the From subtitle link
Verify that: App will open the report from the subtitle link
jliexpensify commented 1 month ago

Paying now, apologies for the delay!

Upwork job

jliexpensify commented 1 month ago

Paid and job remvoed

garrettmknight commented 3 weeks ago

$250 approved for @fedirjh