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

[lanitochka17]

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

1. Launch New Expensify app
2. Go to workspace chat
3. Submit an expense
4. Return to LHN
5. Tap FAB
6. Tap Submit expense under QAB section
7. 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?

• [x] Android: Native
• [x] Android: mWeb Chrome
• [x] iOS: Native
• [ ] iOS: mWeb Safari
• [ ] MacOS: Chrome / Safari
• [ ] MacOS: Desktop

Screenshots/Videos

Add any screenshot/video evidence

https://github.com/user-attachments/assets/28466cc9-9eee-4bb2-9513-8e869723b687

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[bot]]

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]

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

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

[tsa321]

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:

https://github.com/Expensify/App/blob/98d8a5ae4bb52539669f79e007246417eb8c8f1e/src/libs/Navigation/linkingConfig/getAdaptedStateFromPath.ts#L177-L196

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:

https://github.com/Expensify/App/blob/98d8a5ae4bb52539669f79e007246417eb8c8f1e/src/libs/Navigation/linkingConfig/getAdaptedStateFromPath.ts#L187-L191

to make the matchingRootRoute has correct value.

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

https://github.com/Expensify/App/blob/98d8a5ae4bb52539669f79e007246417eb8c8f1e/src/libs/Navigation/linkingConfig/getAdaptedStateFromPath.ts#L151-L156

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[bot]]

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

[melvin-bot[bot]]

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

[kevinksullivan]

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

[melvin-bot[bot]]

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

[melvin-bot[bot]]

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

[goldman727]

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

[melvin-bot[bot]]

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

[aimane-chnaif]

reviewing

[aimane-chnaif]

Reproduced. This happens in various flows.

i.e. split expense

https://github.com/user-attachments/assets/eb7d7396-29d8-49a0-951b-9833b5f4a05a

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

[tsa321]

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

I believe a better fix is:

https://github.com/Expensify/App/blob/98d8a5ae4bb52539669f79e007246417eb8c8f1e/src/libs/Navigation/linkingConfig/getAdaptedStateFromPath.ts#L187-L191

to make the matchingRootRoute has correct value.

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

https://github.com/Expensify/App/blob/98d8a5ae4bb52539669f79e007246417eb8c8f1e/src/libs/Navigation/linkingConfig/getAdaptedStateFromPath.ts#L151-L156

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]

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

[tsa321]

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

https://github.com/Expensify/App/blob/98d8a5ae4bb52539669f79e007246417eb8c8f1e/src/libs/Navigation/linkingConfig/getAdaptedStateFromPath.ts#L151-L156

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[bot]]

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

[aimane-chnaif]

we're still discussing solution

[melvin-bot[bot]]

@kevinksullivan @aimane-chnaif this issue was created 2 weeks ago. Are we close to approving a proposal? If not, what's blocking us from getting this issue assigned? Don't hesitate to create a thread in #expensify-open-source to align faster in real time. Thanks!

[melvin-bot[bot]]

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

[aimane-chnaif]

@tsa321 are you able to find offending PR? Looks like this didn't happen before.

[aimane-chnaif]

Adding this code back fixes this bug.

    if (isNarrowLayout) {
        metainfo.isFullScreenNavigatorMandatory = false;
        metainfo.isCentralPaneAndBottomTabMandatory = false;
    }

@kosmydel what does removal of this code fix? can we safely revert back or any alternative solution?

[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? 💸

[melvin-bot[bot]]

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

[aimane-chnaif]

@kosmydel kind bump https://github.com/Expensify/App/issues/46352#issuecomment-2285208830

[aimane-chnaif]

Adding this code back fixes this bug.

    if (isNarrowLayout) {
        metainfo.isFullScreenNavigatorMandatory = false;
        metainfo.isCentralPaneAndBottomTabMandatory = false;
    }

@kosmydel what does removal of this code fix? can we safely revert back or any alternative solution?

cc: @BrtqKr in case @kosmydel is not available

[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? 💸

[BrtqKr]

@aimane-chnaif I think this part was to avoid overlaying two not found views on each other, because under standard circumstances we've been getting two not found views next to each other, but when narrowing it down the screens were ending up with an overlay

[melvin-bot[bot]]

@kevinksullivan @aimane-chnaif this issue is now 4 weeks old, please consider:

• Finding a contributor to fix the bug
• Closing the issue if BZ has been unable to add the issue to a VIP or Wave project
• If you have any questions, don't hesitate to start a discussion in #expensify-open-source

Thanks!

[kevinksullivan]

Still figuring out the ideal solution.

[kevinksullivan]

@aimane-chnaif anything to add on what we need to do here?

[aimane-chnaif]

@BrtqKr are you interested in fixing this as regression from https://github.com/Expensify/App/pull/41665 which you worked on?

[tsa321]

@aimane-chnaif maybe based on this, we can add the screens to RHP_SCREENS_OPENED_FROM_LHN list?

[BrtqKr]

@aimane-chnaif, sorry for the delay, I got sick on Monday. I could take care of this some time in the next week since I've got a couple of things assigned atm. If it's urgent, please give it to someone else

[aimane-chnaif]

@BrtqKr no problem. Feel better. I think we can wait for next week since it's not critical bug

[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? 💸

[melvin-bot[bot]]

@kevinksullivan, @aimane-chnaif Eep! 4 days overdue now. Issues have feelings too...

[aimane-chnaif]

@BrtqKr will take care of this some time this week

[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? 💸

[aimane-chnaif]

@BrtqKr are you back?

[kevinksullivan]

Looping in another BZ as I'm going OOO. @aimane-chnaif , we may want to open this up if we don't hear back today/tomorrow.

[melvin-bot[bot]]

Triggered auto assignment to @lschurr (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[bot]]

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

[melvin-bot[bot]]

@kevinksullivan, @lschurr, @aimane-chnaif Eep! 4 days overdue now. Issues have feelings too...

[lschurr]

What's next on this one @aimane-chnaif - Are we waiting on proposals?

[aimane-chnaif]

yes. can we float this around SWM team?

[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? 💸

[melvin-bot[bot]]

@kevinksullivan, @lschurr, @aimane-chnaif Eep! 4 days overdue now. Issues have feelings too...

[mallenexpensify]

@lschurr can you share in #expensify-swm once they post their weekly update on their projects and bandwidth? Should be tomorrow/Tues.

[lschurr]

I'm OOO until 10/7 but looking at the cal, I think @kevinksullivan is back today? Are you able to take this one back Kev?