[HOLD for payment 2024-10-25] [$250] QAB - Destination opens in the background when starting QAB flow #46352

lanitochka17 · 2024-07-27T02:03:13Z

If you haven’t already, check out our contributing guidelines for onboarding and email [email protected] to request to join our Slack channel!

Version Number: 9.0.13-3
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:

Launch New Expensify app
Go to workspace chat
Submit an expense
Return to LHN
Tap FAB
Tap Submit expense under QAB section
Tap app back button

Expected Result:

App will return to LHN after closing QAB flow

Actual Result:

App will return to workspace chat (destination) after closing QAB flow
The destination (in this case, workspace chat) opens in the background when starting QAB flow

Workaround:

Unknown

Platforms:

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

Screenshots/Videos

Add any screenshot/video evidence

Bug6554078_1722025287007.ScreenRecording_07-27-2024_04-17-08_1.mp4

View all open jobs on GitHub

Upwork Automation - Do Not Edit

Upwork Job URL: https://www.upwork.com/jobs/~01234964a7fed98878
Upwork Job ID: 1819145506544874650
Last Price Increase: 2024-09-26
Automatic offers:

tsa321 | Contributor | 104236266

Issue Owner

Current Issue Owner: @aimane-chnaif

melvin-bot · 2024-07-27T02:03:27Z

Triggered auto assignment to @kevinksullivan (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 · 2024-07-27T02:04:14Z

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

lanitochka17 · 2024-07-27T02:04:37Z

We think that this bug might be related to #wave-collect - Release 1

tsa321 · 2024-07-28T16:45:13Z

Edited by proposal-police: This proposal was edited at 2024-08-09 22:16:08 UTC.

Proposal

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

QAB - Destination opens in the background when starting QAB flow

What is the root cause of that problem?

Here:

App/src/libs/Navigation/linkingConfig/getAdaptedStateFromPath.ts

Lines 177 to 196 in 98d8a5a

    
           if (rhpNavigator) { 
        
               // Routes 
        
               // - matching bottom tab 
        
               // - matching root route for rhp 
        
               // - found rhp 
        
               // This one will be defined because rhpNavigator is defined. 
        
               const focusedRHPRoute = findFocusedRoute(state); 
        
               const routes = []; 
        
               if (focusedRHPRoute) { 
        
                   let matchingRootRoute = getMatchingRootRouteForRHPRoute(focusedRHPRoute); 
        
                   const isRHPScreenOpenedFromLHN = focusedRHPRoute?.name && RHP_SCREENS_OPENED_FROM_LHN.includes(focusedRHPRoute?.name as RHPScreenOpenedFromLHN); 
        
                   // This may happen if this RHP doens't have a route that should be under the overlay defined. 
        
                   if (!matchingRootRoute || isRHPScreenOpenedFromLHN) { 
        
                       metainfo.isCentralPaneAndBottomTabMandatory = false; 
        
                       metainfo.isFullScreenNavigatorMandatory = false; 
        
                       // If matchingRootRoute is undefined and it's a narrow layout, don't add a report screen under the RHP. 
        
                       matchingRootRoute = matchingRootRoute ?? (!isNarrowLayout ? {name: SCREENS.REPORT} : undefined); 
        
                   }

When the target navigation is RHP from the bottom tab navigator (LHN), as in the case of creating an IOU request, the isRHPScreenOpenedFromLHN does not include SCREENS.MONEY_REQUEST.CREATE. As a result, the isRHPScreenOpenedFromLHN condition evaluates to false, and since there is a matchingRootRoute, the metainfo values for isCentralPaneAndBottomTabMandatory and isFullScreenNavigatorMandatory remain true. Consequently, the navigation state adapts to include the central pane route (the matching root route, which in this case is the report screen).

If the user clicks on the FAB and selects "Submit Expense," the matchingRootRoute will be undefined because the report ID in the URL/route is generated optimistically. This change will update the metainfo to:
metainfo.isCentralPaneAndBottomTabMandatory = false; metainfo.isFullScreenNavigatorMandatory = false;

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

we should include the iou request create focused route, (in this case "manual") add SCREENS.MONEY_REQUEST.CREATE.MANUAL / CONST.TAB_REQUEST_MANUAL in RHP_SCREENS_OPENED_FROM_LHN, also we must check other related screens that can be opened from LHN, the scan and distance tab_request, etc...

Alternative solution

I think there will be many screens to include as well.

I believe a better fix is:

App/src/libs/Navigation/linkingConfig/getAdaptedStateFromPath.ts

Lines 187 to 191 in 98d8a5a

    
           if (focusedRHPRoute) { 
        
               let matchingRootRoute = getMatchingRootRouteForRHPRoute(focusedRHPRoute); 
        
               const isRHPScreenOpenedFromLHN = focusedRHPRoute?.name && RHP_SCREENS_OPENED_FROM_LHN.includes(focusedRHPRoute?.name as RHPScreenOpenedFromLHN); 
        
               // This may happen if this RHP doens't have a route that should be under the overlay defined. 
        
               if (!matchingRootRoute || isRHPScreenOpenedFromLHN) {

to make the matchingRootRoute has correct value.

The getMatchingRootRouteForRHPRoute will always include the report screen if there is a reportID parameter in the route:

App/src/libs/Navigation/linkingConfig/getAdaptedStateFromPath.ts

Lines 151 to 156 in 98d8a5a

    
           // check for valid reportID in the route params 
        
           // if the reportID is valid, we should navigate back to screen report in CPN 
        
           const reportID = (route.params as Record<string, string | undefined>)?.reportID; 
        
           if (ReportConnection.getAllReports()?.[`${ONYXKEYS.COLLECTION.REPORT}${reportID}`]) { 
        
               return {name: SCREENS.REPORT, params: {reportID}}; 
        
           }

However, the route that must include the report screen is the route that starts with /r/. Other routes do not need to include the report screen. Therefore, the code changes should be:

// sometimes route path start with `/` and sometimes not. just to make sure here.
if (route?.path.indexOf('/r/') === 0 || route?.path.indexOf('r/') === 0) {
        const reportID = (route.params as Record < string, string | undefined > )?.reportID;
        if (!!reportID && ReportConnection.getAllReports()?.[`${ONYXKEYS.COLLECTION.REPORT}${reportID}`]) {
            return {name: SCREENS.REPORT, params: {reportID}};
        }
    }

Other routes which really require report of reportID param to be openened first can be included in exception list.

melvin-bot · 2024-07-30T18:29:15Z

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

melvin-bot · 2024-08-01T18:27:56Z

@kevinksullivan Eep! 4 days overdue now. Issues have feelings too...

kevinksullivan · 2024-08-01T22:57:31Z

Reproduced. I think this is a polish item for collect.

melvin-bot · 2024-08-01T22:57:43Z

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

melvin-bot · 2024-08-01T22:57:47Z

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

goldman727 · 2024-08-02T02:09:19Z

I have seen about this issue, what do you need fix exactly?

melvin-bot · 2024-08-05T18:45:07Z

@kevinksullivan, @aimane-chnaif Whoops! This issue is 2 days overdue. Let's get this updated quick!

aimane-chnaif · 2024-08-05T18:49:14Z

reviewing

aimane-chnaif · 2024-08-06T19:37:11Z

Reproduced. This happens in various flows.

i.e. split expense

split.mp4

@tsa321 can you please share test branch covering all cases?

tsa321 · 2024-08-07T02:18:14Z

@aimane-chnaif, I think there will be many screens to include as well.

I believe a better fix is:

App/src/libs/Navigation/linkingConfig/getAdaptedStateFromPath.ts

Lines 187 to 191 in 98d8a5a

    
           if (focusedRHPRoute) { 
        
               let matchingRootRoute = getMatchingRootRouteForRHPRoute(focusedRHPRoute); 
        
               const isRHPScreenOpenedFromLHN = focusedRHPRoute?.name && RHP_SCREENS_OPENED_FROM_LHN.includes(focusedRHPRoute?.name as RHPScreenOpenedFromLHN); 
        
               // This may happen if this RHP doens't have a route that should be under the overlay defined. 
        
               if (!matchingRootRoute || isRHPScreenOpenedFromLHN) {

to make the matchingRootRoute has correct value.

The getMatchingRootRouteForRHPRoute will always include the report screen if there is a reportID parameter in the route:

App/src/libs/Navigation/linkingConfig/getAdaptedStateFromPath.ts

Lines 151 to 156 in 98d8a5a

    
           // check for valid reportID in the route params 
        
           // if the reportID is valid, we should navigate back to screen report in CPN 
        
           const reportID = (route.params as Record<string, string | undefined>)?.reportID; 
        
           if (ReportConnection.getAllReports()?.[`${ONYXKEYS.COLLECTION.REPORT}${reportID}`]) { 
        
               return {name: SCREENS.REPORT, params: {reportID}}; 
        
           }

However, the route that must include the report screen is the route that starts with /r/. Other routes do not need to include the report screen. Therefore, the code changes should be:

// sometimes route path start with `/` and sometimes not. just to make sure here.
if (route?.path.indexOf('/r/') === 0 || route?.path.indexOf('r/') === 0) {
        const reportID = (route.params as Record < string, string | undefined > )?.reportID;
        if (!!reportID && ReportConnection.getAllReports()?.[`${ONYXKEYS.COLLECTION.REPORT}${reportID}`]) {
            return {name: SCREENS.REPORT, params: {reportID}};
        }
    }

Alternatively, if you think a better solution would be to include the related screens in RHP_SCREENS_OPENED_FROM_LHN, I can list the screens.

aimane-chnaif · 2024-08-07T20:58:57Z

I prefer general solution. If we can fix in navigation level (getAdaptedStateFromPath), that would be great.

tsa321 · 2024-08-08T00:00:38Z

@aimane-chnaif then How about my code change, replace these lines:

App/src/libs/Navigation/linkingConfig/getAdaptedStateFromPath.ts

Lines 151 to 156 in 98d8a5a

    
           // check for valid reportID in the route params 
        
           // if the reportID is valid, we should navigate back to screen report in CPN 
        
           const reportID = (route.params as Record<string, string | undefined>)?.reportID; 
        
           if (ReportConnection.getAllReports()?.[`${ONYXKEYS.COLLECTION.REPORT}${reportID}`]) { 
        
               return {name: SCREENS.REPORT, params: {reportID}}; 
        
           }

with:

// sometimes route path start with `/` and sometimes not. just to make sure here.
if (route?.path.indexOf('/r/') === 0 || route?.path.indexOf('r/') === 0) {
        const reportID = (route.params as Record < string, string | undefined > )?.reportID;
        if (!!reportID && ReportConnection.getAllReports()?.[`${ONYXKEYS.COLLECTION.REPORT}${reportID}`]) {
            return {name: SCREENS.REPORT, params: {reportID}};
        }
    }

This will fix many similar issues.

melvin-bot · 2024-08-08T16:00:18Z

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

melvin-bot · 2024-10-01T20:54:45Z

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

mallenexpensify · 2024-10-01T22:54:39Z

Since I happen to be in this issue for some reason...
At a high level, if a contributor provides a solution on an issue that has the Help Wanted label, they might be eligible for compensation. I think my preference here would be to have @tsa321 raise the PR, since that keeps things easy, where they'd get paid the full amount. @aimane-chnaif is there a reason you wouldn't recommend this? (I realize you found the offending PR, thanks for helping there)

aimane-chnaif · 2024-10-02T04:59:08Z

The bug didn't happen before so I was sure that this was recent regression. So I didn't accept any proposal without finding offending PR, which means the root cause is not accurate. That's only the reason.

dangrous · 2024-10-02T15:23:10Z

Jumping in here too, I think if the solution is correct let's move forward with @tsa321; it's definitely not the ideal that the root cause was incorrect originally, but now that we've determined it and the proposal still solves it in the best way possible, I think that's okay.

I'll wait a bit for any further discussion and then plan to assign @tsa321 - let's just make sure in the future we make sure to find that correct root cause since that will give us the best solution!

melvin-bot · 2024-10-02T18:33:22Z

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

Guccio163 · 2024-10-03T12:41:01Z

I see that @tsa321 will handle this one, feel free to call me for a review etc. when the PR is done though ;)

melvin-bot · 2024-10-18T18:54:19Z

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

melvin-bot · 2024-10-18T18:54:24Z

The solution for this issue has been 🚀 deployed to production 🚀 in version 9.0.50-8 and is now subject to a 7-day regression period 📆. Here is the list of pull requests that resolve this issue:

Fix report screen opened in background when starting QAB flow #50200

If no regressions arise, payment will be issued on 2024-10-25. 🎊

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

@Guccio163 does not require payment (Contractor)
@aimane-chnaif requires payment (Needs manual offer from BZ)
@tsa321 requires payment automatic offer (Contributor)

melvin-bot · 2024-10-18T18:54:25Z

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:

[@aimane-chnaif] The PR that introduced the bug has been identified. Link to the PR:
[@aimane-chnaif] 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:
[@aimane-chnaif] 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:
[@aimane-chnaif] Determine if we should create a regression test for this bug.
[@aimane-chnaif] 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.
[@sonialiap] Link the GH issue for creating/updating the regression test once above steps have been agreed upon: https://github.com/Expensify/Expensify/issues/439489

sonialiap · 2024-10-25T10:47:06Z

Payment summary:

@aimane-chnaif $250 paid in upwork ✔️
@tsa321 $250 paid in upwork ✔️

aimane-chnaif · 2024-10-28T06:45:18Z

The PR that introduced the bug has been identified. Link to the PR: Fix: three not found view v2 #41665
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: [HOLD for payment 2024-10-25] [$250] QAB - Destination opens in the background when starting QAB flow #46352 (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: N/A

Regression Test Proposal

Complete the manual expense submission flow.
Click on FAB, then select QAB to open the expense submission flow.
Click the back button in the header.
For small screens (Android and iOS), verify that the LHN screen opens instead of the report screen. For wide screens, ensure that the submit expense flow functions properly.
Repeat steps 1–4 for the scan and distance expense submission flows.

aimane-chnaif · 2024-10-28T06:45:39Z

@sonialiap I am still using upwork

sonialiap · 2024-10-28T16:34:13Z

@aimane-chnaif oh sorry! I saw that you're whitelisted for ND payments. Offer sent in Upwork

melvin-bot · 2024-10-29T09:42:29Z

@dangrous @sonialiap Be sure to fill out the Contact List!

lanitochka17 added Daily KSv2 Bug Something is broken. Auto assigns a BugZero manager. labels Jul 27, 2024

melvin-bot bot assigned kevinksullivan Jul 27, 2024

melvin-bot bot added the Overdue label Jul 29, 2024

melvin-bot bot removed the Overdue label Aug 1, 2024

kevinksullivan added this to [#whatsnext] #wave-collect Aug 1, 2024

kevinksullivan added the External Added to denote the issue can be worked on by a contributor label Aug 1, 2024

melvin-bot bot changed the title ~~QAB - Destination opens in the background when starting QAB flow~~ [$250] QAB - Destination opens in the background when starting QAB flow Aug 1, 2024

melvin-bot bot added the Help Wanted Apply this label when an issue is open to proposals by contributors label Aug 1, 2024

melvin-bot bot assigned aimane-chnaif Aug 1, 2024

kevinksullivan moved this to Polish in [#whatsnext] #wave-collect Aug 1, 2024

melvin-bot bot added the Overdue label Aug 5, 2024

melvin-bot bot removed the Overdue label Aug 5, 2024

melvin-bot bot assigned dangrous Oct 1, 2024

dangrous assigned tsa321 Oct 2, 2024

tsa321 mentioned this issue Oct 4, 2024

Fix report screen opened in background when starting QAB flow #50200

Merged

50 tasks

melvin-bot bot added Reviewing Has a PR in review Weekly KSv2 and removed Daily KSv2 labels Oct 4, 2024

melvin-bot bot added Weekly KSv2 Awaiting Payment Auto-added when associated PR is deployed to production and removed Weekly KSv2 labels Oct 18, 2024

melvin-bot bot changed the title ~~[$250] QAB - Destination opens in the background when starting QAB flow~~ [HOLD for payment 2024-10-25] [$250] QAB - Destination opens in the background when starting QAB flow Oct 18, 2024

melvin-bot bot removed the Reviewing Has a PR in review label Oct 18, 2024

sonialiap added Daily KSv2 and removed Weekly KSv2 labels Oct 25, 2024

melvin-bot bot added Daily KSv2 and removed Daily KSv2 labels Oct 25, 2024

sonialiap closed this as completed Oct 29, 2024

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[HOLD for payment 2024-10-25] [$250] QAB - Destination opens in the background when starting QAB flow #46352

[HOLD for payment 2024-10-25] [$250] QAB - Destination opens in the background when starting QAB flow #46352

lanitochka17 commented Jul 27, 2024 •

edited by sonialiap

Loading

melvin-bot bot commented Jul 27, 2024

lanitochka17 commented Jul 27, 2024

lanitochka17 commented Jul 27, 2024

tsa321 commented Jul 28, 2024 •

edited by github-actions bot

Loading

melvin-bot bot commented Jul 30, 2024

melvin-bot bot commented Aug 1, 2024

kevinksullivan commented Aug 1, 2024

melvin-bot bot commented Aug 1, 2024

melvin-bot bot commented Aug 1, 2024

goldman727 commented Aug 2, 2024

melvin-bot bot commented Aug 5, 2024

aimane-chnaif commented Aug 5, 2024

aimane-chnaif commented Aug 6, 2024

tsa321 commented Aug 7, 2024 •

edited

Loading

aimane-chnaif commented Aug 7, 2024

tsa321 commented Aug 8, 2024

melvin-bot bot commented Aug 8, 2024

melvin-bot bot commented Oct 1, 2024

mallenexpensify commented Oct 1, 2024 •

edited

Loading

aimane-chnaif commented Oct 2, 2024 •

edited

Loading

dangrous commented Oct 2, 2024

melvin-bot bot commented Oct 2, 2024

Guccio163 commented Oct 3, 2024

melvin-bot bot commented Oct 18, 2024

melvin-bot bot commented Oct 18, 2024

melvin-bot bot commented Oct 18, 2024 •

edited by sonialiap

Loading

sonialiap commented Oct 25, 2024 •

edited

Loading

aimane-chnaif commented Oct 28, 2024 •

edited

Loading

aimane-chnaif commented Oct 28, 2024

sonialiap commented Oct 28, 2024

melvin-bot bot commented Oct 29, 2024

[HOLD for payment 2024-10-25] [$250] QAB - Destination opens in the background when starting QAB flow #46352

[HOLD for payment 2024-10-25] [$250] QAB - Destination opens in the background when starting QAB flow #46352

Comments

lanitochka17 commented Jul 27, 2024 • edited by sonialiap Loading

Action Performed:

Expected Result:

Actual Result:

Workaround:

Platforms:

Screenshots/Videos

melvin-bot bot commented Jul 27, 2024

lanitochka17 commented Jul 27, 2024

lanitochka17 commented Jul 27, 2024

tsa321 commented Jul 28, 2024 • edited by github-actions bot Loading

Proposal

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

What is the root cause of that problem?

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

Alternative solution

melvin-bot bot commented Jul 30, 2024

melvin-bot bot commented Aug 1, 2024

kevinksullivan commented Aug 1, 2024

melvin-bot bot commented Aug 1, 2024

melvin-bot bot commented Aug 1, 2024

goldman727 commented Aug 2, 2024

melvin-bot bot commented Aug 5, 2024

aimane-chnaif commented Aug 5, 2024

aimane-chnaif commented Aug 6, 2024

tsa321 commented Aug 7, 2024 • edited Loading

aimane-chnaif commented Aug 7, 2024

tsa321 commented Aug 8, 2024

melvin-bot bot commented Aug 8, 2024

melvin-bot bot commented Oct 1, 2024

mallenexpensify commented Oct 1, 2024 • edited Loading

aimane-chnaif commented Oct 2, 2024 • edited Loading

dangrous commented Oct 2, 2024

melvin-bot bot commented Oct 2, 2024

Guccio163 commented Oct 3, 2024

melvin-bot bot commented Oct 18, 2024

melvin-bot bot commented Oct 18, 2024

melvin-bot bot commented Oct 18, 2024 • edited by sonialiap Loading

sonialiap commented Oct 25, 2024 • edited Loading

aimane-chnaif commented Oct 28, 2024 • edited Loading

Regression Test Proposal

aimane-chnaif commented Oct 28, 2024

sonialiap commented Oct 28, 2024

melvin-bot bot commented Oct 29, 2024

lanitochka17 commented Jul 27, 2024 •

edited by sonialiap

Loading

tsa321 commented Jul 28, 2024 •

edited by github-actions bot

Loading

tsa321 commented Aug 7, 2024 •

edited

Loading

mallenexpensify commented Oct 1, 2024 •

edited

Loading

aimane-chnaif commented Oct 2, 2024 •

edited

Loading

melvin-bot bot commented Oct 18, 2024 •

edited by sonialiap

Loading

sonialiap commented Oct 25, 2024 •

edited

Loading

aimane-chnaif commented Oct 28, 2024 •

edited

Loading