[HOLD for payment 2024-11-20] Search - App navigates to Inbox page when submitting an expense via QAB #50157

lanitochka17 · 2024-10-03T15:01:20Z

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.44-0
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
Email or phone of affected tester (no customers): [email protected]
Issue reported by: Applause - Internal Team

Issue found when executing PR #50028

Action Performed:

Login to any account
Switch to the Search page tab
Click the global create button (FAB) > Submit expense > Manual > Enter amount > Choose recipient and submit
Notice that you remain on the search page
Click on the FAB button again > Quick Action Button > Enter amount and submit

Expected Result:

The user remains on the search page and the newly added expense appears in the table and the row is briefly highlighted

Actual Result:

The user is navigated to inbox page, there is an inconsistency when submitting an expense via FAB and QAB

Workaround:

Unknown

Platforms:

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

Screenshots/Videos

Add any screenshot/video evidence

Bug6623276_1727964658628.2024-10-03_17_07_56.mp4

View all open jobs on GitHub

Issue Owner

Current Issue Owner: @trjExpensify

melvin-bot · 2024-10-03T15:01:31Z

Triggered auto assignment to @slafortune (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-10-03T15:02:17Z

@slafortune 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-10-03T15:02:35Z

We think that this bug might be related to #wave-control

trjExpensify · 2024-10-03T15:27:04Z

Yep, like the create options, we'd want someone using the QAB to remain on the search page if they created from the search page as well.

@mananjadhav can you take this?

mananjadhav · 2024-10-03T15:27:51Z

Will take this up.

mananjadhav · 2024-10-03T19:37:44Z

I have a solution that works. This definitely seems to be related to Navigation so I was verifying if it would break something. I also feel we should get an opinion from the expert agency as it relates to Navigation.

Proposal

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

When we click on the "Quick action" from search or other pages on FAB, it navigates to "Inbox" page.

What is the root cause of that problem?

We're calling IOU.startMoneyRequest with parameters IOU.startMoneyRequest(CONST.IOU.TYPE.SUBMIT, quickActionReportID, CONST.IOU.REQUEST_TYPE.MANUAL, true).
When this is called, the navigation path doesn't directly called the /create/submit/start/{reportId}/manual. Instead this calls /create/submit/{reportId}?initial=true&screen=scan&params=object&path=/create/submit/start/.... I think this is related to the linkTo implementation.

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

For all quick action cases in FloatingActionButtonAndPopover.tsx, we can get rid of the third parameter requestType and set it as undefined.

App/src/pages/home/sidebar/SidebarScreen/FloatingActionButtonAndPopover.tsx

Lines 249 to 282 in b027baa

    
           switch (quickAction?.action) { 
        
               case CONST.QUICK_ACTIONS.REQUEST_MANUAL: 
        
                   selectOption(() => IOU.startMoneyRequest(CONST.IOU.TYPE.SUBMIT, quickActionReportID, CONST.IOU.REQUEST_TYPE.MANUAL, true), true); 
        
                   return; 
        
               case CONST.QUICK_ACTIONS.REQUEST_SCAN: 
        
                   selectOption(() => IOU.startMoneyRequest(CONST.IOU.TYPE.SUBMIT, quickActionReportID, CONST.IOU.REQUEST_TYPE.SCAN, true), true); 
        
                   return; 
        
               case CONST.QUICK_ACTIONS.REQUEST_DISTANCE: 
        
                   selectOption(() => IOU.startMoneyRequest(CONST.IOU.TYPE.SUBMIT, quickActionReportID, CONST.IOU.REQUEST_TYPE.DISTANCE, true), true); 
        
                   return; 
        
               case CONST.QUICK_ACTIONS.SPLIT_MANUAL: 
        
                   selectOption(() => IOU.startMoneyRequest(CONST.IOU.TYPE.SPLIT, quickActionReportID, CONST.IOU.REQUEST_TYPE.MANUAL, true), true); 
        
                   return; 
        
               case CONST.QUICK_ACTIONS.SPLIT_SCAN: 
        
                   selectOption(() => IOU.startMoneyRequest(CONST.IOU.TYPE.SPLIT, quickActionReportID, CONST.IOU.REQUEST_TYPE.SCAN, true), true); 
        
                   return; 
        
               case CONST.QUICK_ACTIONS.SPLIT_DISTANCE: 
        
                   selectOption(() => IOU.startMoneyRequest(CONST.IOU.TYPE.SPLIT, quickActionReportID, CONST.IOU.REQUEST_TYPE.DISTANCE, false), true); 
        
                   return; 
        
               case CONST.QUICK_ACTIONS.SEND_MONEY: 
        
                   selectOption(() => IOU.startMoneyRequest(CONST.IOU.TYPE.PAY, quickActionReportID, CONST.IOU.REQUEST_TYPE.MANUAL, true), false); 
        
                   return; 
        
               case CONST.QUICK_ACTIONS.ASSIGN_TASK: 
        
                   selectOption(() => Task.startOutCreateTaskQuickAction(isValidReport ? quickActionReportID : '', quickAction.targetAccountID ?? -1), false); 
        
                   break; 
        
               case CONST.QUICK_ACTIONS.TRACK_MANUAL: 
        
                   selectOption(() => IOU.startMoneyRequest(CONST.IOU.TYPE.TRACK, quickActionReportID, CONST.IOU.REQUEST_TYPE.MANUAL, true), false); 
        
                   break; 
        
               case CONST.QUICK_ACTIONS.TRACK_SCAN: 
        
                   selectOption(() => IOU.startMoneyRequest(CONST.IOU.TYPE.TRACK, quickActionReportID, CONST.IOU.REQUEST_TYPE.SCAN, true), false); 
        
                   break; 
        
               case CONST.QUICK_ACTIONS.TRACK_DISTANCE: 
        
                   selectOption(() => IOU.startMoneyRequest(CONST.IOU.TYPE.TRACK, quickActionReportID, CONST.IOU.REQUEST_TYPE.DISTANCE, true), false); 
        
                   break;

The reason I suggested this is because we the function startMoneyRequest has four parameters (iouType: ValueOf<typeof CONST.IOU.TYPE>, reportID: string, requestType?: IOURequestType, skipConfirmation = false) but across the application we never use the requestType argument.

Here's a quick video of the implementation.

Screen.Recording.2024-10-04.at.12.25.03.AM.mov

What alternative solutions did you explore? (Optional)

Fix this at the navigation level.

c3024 · 2024-10-04T07:24:10Z

Proposal

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

BottomTab and CentralPane change when we open RHP with QAB in Search Central Pane.

What is the root cause of that problem?

When we start the flow with QAB the report in the URL param is a valid report and it finds a matching route for RHP route here.

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

Lines 162 to 164 in 66aefc3

    
           if (ReportConnection.getAllReports()?.[`${ONYXKEYS.COLLECTION.REPORT}${reportID}`]?.reportID) { 
        
               return {name: SCREENS.REPORT, params: {reportID}}; 
        
           }

and this is pushed to routes with metaInfo as true here.

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

Lines 215 to 225 in 66aefc3

    
                   if (matchingRootRoute && (!isNarrowLayout || !isRHPScreenOpenedFromLHN)) { 
        
                       routes.push(matchingRootRoute); 
        
                   } 
        
               } 
        
               routes.push(rhpNavigator); 
        
               return { 
        
                   adaptedState: getRoutesWithIndex(routes), 
        
                   metainfo, 
        
               }; 
        
           }

Since metaInfo fields are both true, we find if there are diff actions that need to be dispatched here and we dispatch them.

App/src/libs/Navigation/linkTo/index.ts

Lines 136 to 143 in 66aefc3

    
           const {adaptedState, metainfo} = getAdaptedStateFromPath(path, linkingConfig.config); 
        
           if (adaptedState && (metainfo.isCentralPaneAndBottomTabMandatory || metainfo.isFullScreenNavigatorMandatory)) { 
        
               const diff = getPartialStateDiff(rootState, adaptedState as State<RootStackParamList>, metainfo); 
        
               const diffActions = getActionsFromPartialDiff(diff); 
        
               for (const diffAction of diffActions) { 
        
                   root.dispatch(diffAction); 
        
               } 
        
           }

In this case, the bottom tab navigators and central panes are different (Search_Bottom_Tab/Home and Search_Central_Pane and Report respectively). So, these diffs are dispatched which cause the QAB report and Home to be added for the Central Pane and Bottom Tab respectively.

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

When on Search_Central_Pane I have not found any cases where we need to change the bottom tab or central pane when opening RHP. So we can dispatch the diffs only when when do not open the RHP when on Search Central Pane.
So, we can wrap this in a check

App/src/libs/Navigation/linkTo/index.ts

Lines 135 to 143 in 66aefc3

    
           // If this RHP has mandatory central pane and bottom tab screens defined we need to push them. 
        
           const {adaptedState, metainfo} = getAdaptedStateFromPath(path, linkingConfig.config); 
        
           if (adaptedState && (metainfo.isCentralPaneAndBottomTabMandatory || metainfo.isFullScreenNavigatorMandatory)) { 
        
               const diff = getPartialStateDiff(rootState, adaptedState as State<RootStackParamList>, metainfo); 
        
               const diffActions = getActionsFromPartialDiff(diff); 
        
               for (const diffAction of diffActions) { 
        
                   root.dispatch(diffAction); 
        
               } 
        
           }

 if (!(isSideModalNavigator(action.payload.name) && lastRoute?.name === "Search_Central_Pane")) {
            const {adaptedState, metainfo} = getAdaptedStateFromPath(path, linkingConfig.config);
            if (adaptedState && (metainfo.isCentralPaneAndBottomTabMandatory || metainfo.isFullScreenNavigatorMandatory)) {
                const diff = getPartialStateDiff(rootState, adaptedState as State<RootStackParamList>, metainfo);
                const diffActions = getActionsFromPartialDiff(diff);
                for (const diffAction of diffActions) {
                    root.dispatch(diffAction);
                }
            }
}

What alternative solutions did you explore? (Optional)

c3024 · 2024-10-04T07:28:33Z

I can work on this as contributor and @mananjadhav as C+ if this solution above looks good.

mananjadhav · 2024-10-04T07:33:48Z

I was working on a solution to update linkTo. Let me review your proposal.

mananjadhav · 2024-10-04T08:48:24Z

Yeah I think @c3024's solution works.

where we need to change the bottom tab or central pane when opening RHP.

Just need to clarify this. Otherwise we'll need to modify the startMoneyRequest calls as I recommended.

c3024 · 2024-10-04T09:54:38Z

My bad! My solution is not good enough. The issue can happen when we click on QAB from other pages like the settings page with related centre panes.

I think we should keep a global value for tracking if we clicked on QAB and just ignore adding the report here

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

Lines 162 to 164 in 66aefc3

    
           if (ReportConnection.getAllReports()?.[`${ONYXKEYS.COLLECTION.REPORT}${reportID}`]?.reportID) { 
        
               return {name: SCREENS.REPORT, params: {reportID}}; 
        
           }

even if it matches with an existing report like this.

trjExpensify · 2024-10-04T10:08:19Z

How did we approach this for the create flow when actioned from the search page, wouldn't we follow that? I.e if you're on the search page and choose Submit expense or the QAB, the outcome is the same on completion. You remain on the search page, and the expense is highlighted in the table.

c3024 · 2024-10-04T10:16:49Z

We generate the navigation state required based on the URL. When we use QAB, we have a valid report ID in the URL. In normal cases (that is, other than QAB), we do not have a valid report ID in the URL. The report is chosen on the participants' page.

When we deeplink into an RHP flow and the URL has an existing report ID, we show the report as an underlay when we navigate to it. Therefore, we add this report screen to the navigation state when the URL has an existing report ID.

The URL is the same for the QAB flow as well, so our navigation code pushes the existing report to the central pane, and this navigation to a different bottom navigator (HOME) and central pane (REPORT) occurs. When we click on the normal 'Submit Expense' flow, there is no valid report ID in the URL, and no report route is pushed into the state. This is peculiar to QAB alone due to the initial URL having a valid report ID.

c3024 · 2024-10-04T10:26:13Z

The bug occurs in other pages also, not just in the Search Page.

Screen.Recording.2024-10-04.at.3.54.58.PM.mov

mananjadhav · 2024-10-04T12:56:21Z

How did we approach this for the create flow when actioned from the search page, wouldn't we follow that?

@c3024 is right. I think it's just how we've implemented @trjExpensify. Hence we have both the options. Either we go ahead an update the Navigation which @c3024 has recommended or what you asked and that's what my proposal does.

trjExpensify · 2024-10-04T19:16:45Z

I think it's just how we've implemented @trjExpensify.

Sorry to be clear, this "stay on the search page if you Submit expense from the search page" feature is a relatively new feature in the last couple of weeks. So I was wondering how that was implemented, and why we can't do the same for the QAB. CC: @ikevin127 who might have some context on this to chime in as well.

trjExpensify · 2024-10-08T00:26:18Z

Whatcha' think @mananjadhav?

mananjadhav · 2024-10-09T13:17:37Z

I think we need to decide whether we want to update the navigation for this. I feel we should do that.

We have both the proposals and I think we should ask an internal engineer to decide this.

🎀 👀 🎀 C+ reviewed (adding this to assign an internal engineer). We can decide between @c3024 or me to implement + review the PR.

melvin-bot · 2024-10-09T13:17:50Z

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

mananjadhav · 2024-10-22T17:58:16Z

I posted one yesterday on the PR itself here. I'll have it raised by tomorrow.

trjExpensify · 2024-10-25T18:10:03Z

Thanks, any luck?

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

@francoisl, @mananjadhav, @trjExpensify, @c3024 Whoops! This issue is 2 days overdue. Let's get this updated quick!

mananjadhav · 2024-10-25T18:42:52Z

I am left with one last withOnyx update and I need to modify the selector.

melvin-bot · 2024-11-13T00:50:20Z

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

melvin-bot · 2024-11-13T00:50:24Z

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

fix: update navigation for qab #51057

If no regressions arise, payment will be issued on 2024-11-20. 🎊

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

@mananjadhav requires payment through NewDot Manual Requests
@c3024 requires payment (Needs manual offer from BZ)

melvin-bot · 2024-11-13T00:50:25Z

@mananjadhav / @c3024 @trjExpensify @mananjadhav / @c3024 The PR fixing this issue has been merged! The following checklist (instructions) will need to be completed before the issue can be closed. Please copy/paste the BugZero Checklist from here into a new comment on this GH and complete it. If you have the K2 extension, you can simply click: [this button]

melvin-bot · 2024-11-20T09:06:18Z

Payment Summary

Upwork Job

Reviewer: @mananjadhav owed $250 via NewDot
ROLE: @c3024 paid $(AMOUNT) via Upwork (LINK)

BugZero Checklist (@trjExpensify)

I have verified the correct assignees and roles are listed above and updated the neccesary manual offers
I have verified that there are no duplicate or incorrect contracts on Upwork for this job (https://www.upwork.com/ab/applicants//hired)
I have paid out the Upwork contracts or cancelled the ones that are incorrect
I have verified the payment summary above is correct

trjExpensify · 2024-11-20T14:12:46Z

Checklist time, @mananjadhav!

mananjadhav · 2024-11-20T16:19:06Z

I thought @c3024 would take care of it as the reviewer.

I checked the code and I don't think we have an offending PR for this one. But I do feel we should add a regression test step for this.

Regression Test Proposal

Login to any account
Switch to the Search page tab
Click the global create button (FAB) > Submit expense > Manual > Enter amount > Choose recipient and submit.
Notice that you remain on the search page.
Click on the FAB button again > Quick Action Button > Enter amount and submit.
The user remains on the search page and the newly added expense appears in the table.

wdyt?

c3024 · 2024-11-21T04:38:24Z

I thought @c3024 would take care of it as the reviewer.

Yea, I missed that this was due for payment yesterday.

Thanks for handling it!

trjExpensify · 2024-11-21T22:09:05Z

Oh sorry, I assumed you were the reviewer haha.

Payment summary as follows:

$250 to @mananjadhav for the fix (go ahead and request!)
$250 to @c3024 for the review (offer sent!)

trjExpensify · 2024-11-21T23:29:42Z

Paid, closing!

mananjadhav · 2024-11-22T12:03:36Z

Yea, I missed that this was due for payment yesterday.

No worries at all bro!

thanks @trjExpensify

garrettmknight · 2024-11-26T13:49:08Z

$250 approved for @mananjadhav

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

melvin-bot bot assigned slafortune Oct 3, 2024

trjExpensify assigned trjExpensify and unassigned slafortune Oct 3, 2024

trjExpensify added this to [#whatsnext] #wave-collect Oct 3, 2024

trjExpensify moved this to Release 2.5: SuiteWorld (Sept 9th) in [#whatsnext] #wave-collect Oct 3, 2024

trjExpensify assigned mananjadhav Oct 3, 2024

melvin-bot bot added the Overdue label Oct 7, 2024

melvin-bot bot removed the Overdue label Oct 8, 2024

melvin-bot bot assigned francoisl Oct 9, 2024

melvin-bot bot added the Overdue label Oct 24, 2024

melvin-bot bot removed the Overdue label Oct 25, 2024

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

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

melvin-bot bot changed the title ~~Search - App navigates to Inbox page when submitting an expense via QAB~~ [HOLD for payment 2024-11-20] Search - App navigates to Inbox page when submitting an expense via QAB Nov 13, 2024

melvin-bot bot removed the Reviewing Has a PR in review label Nov 13, 2024

melvin-bot bot added Daily KSv2 Monthly KSv2 and removed Weekly KSv2 labels Nov 19, 2024

trjExpensify closed this as completed Nov 21, 2024

github-project-automation bot moved this from MEDIUM to Done in [#whatsnext] #quality Nov 21, 2024

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[HOLD for payment 2024-11-20] Search - App navigates to Inbox page when submitting an expense via QAB #50157

[HOLD for payment 2024-11-20] Search - App navigates to Inbox page when submitting an expense via QAB #50157

lanitochka17 commented Oct 3, 2024 •

edited by melvin-bot bot

Loading

melvin-bot bot commented Oct 3, 2024

lanitochka17 commented Oct 3, 2024

lanitochka17 commented Oct 3, 2024

trjExpensify commented Oct 3, 2024

mananjadhav commented Oct 3, 2024

mananjadhav commented Oct 3, 2024

c3024 commented Oct 4, 2024 •

edited

Loading

c3024 commented Oct 4, 2024

mananjadhav commented Oct 4, 2024

mananjadhav commented Oct 4, 2024 •

edited

Loading

c3024 commented Oct 4, 2024

trjExpensify commented Oct 4, 2024 •

edited

Loading

c3024 commented Oct 4, 2024 •

edited

Loading

c3024 commented Oct 4, 2024

mananjadhav commented Oct 4, 2024

trjExpensify commented Oct 4, 2024

trjExpensify commented Oct 8, 2024

mananjadhav commented Oct 9, 2024

melvin-bot bot commented Oct 9, 2024

mananjadhav commented Oct 22, 2024

trjExpensify commented Oct 25, 2024

melvin-bot bot commented Oct 25, 2024

mananjadhav commented Oct 25, 2024

melvin-bot bot commented Nov 13, 2024

melvin-bot bot commented Nov 13, 2024

melvin-bot bot commented Nov 13, 2024

melvin-bot bot commented Nov 20, 2024

trjExpensify commented Nov 20, 2024

mananjadhav commented Nov 20, 2024

c3024 commented Nov 21, 2024

trjExpensify commented Nov 21, 2024

trjExpensify commented Nov 21, 2024

mananjadhav commented Nov 22, 2024

garrettmknight commented Nov 26, 2024

[HOLD for payment 2024-11-20] Search - App navigates to Inbox page when submitting an expense via QAB #50157

[HOLD for payment 2024-11-20] Search - App navigates to Inbox page when submitting an expense via QAB #50157

Comments

lanitochka17 commented Oct 3, 2024 • edited by melvin-bot bot Loading

Action Performed:

Expected Result:

Actual Result:

Workaround:

Platforms:

Screenshots/Videos

melvin-bot bot commented Oct 3, 2024

lanitochka17 commented Oct 3, 2024

lanitochka17 commented Oct 3, 2024

trjExpensify commented Oct 3, 2024

mananjadhav commented Oct 3, 2024

mananjadhav commented Oct 3, 2024

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?

What alternative solutions did you explore? (Optional)

c3024 commented Oct 4, 2024 • edited 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?

What alternative solutions did you explore? (Optional)

c3024 commented Oct 4, 2024

mananjadhav commented Oct 4, 2024

mananjadhav commented Oct 4, 2024 • edited Loading

c3024 commented Oct 4, 2024

trjExpensify commented Oct 4, 2024 • edited Loading

c3024 commented Oct 4, 2024 • edited Loading

c3024 commented Oct 4, 2024

mananjadhav commented Oct 4, 2024

trjExpensify commented Oct 4, 2024

trjExpensify commented Oct 8, 2024

mananjadhav commented Oct 9, 2024

melvin-bot bot commented Oct 9, 2024

mananjadhav commented Oct 22, 2024

trjExpensify commented Oct 25, 2024

melvin-bot bot commented Oct 25, 2024

mananjadhav commented Oct 25, 2024

melvin-bot bot commented Nov 13, 2024

melvin-bot bot commented Nov 13, 2024

melvin-bot bot commented Nov 13, 2024

melvin-bot bot commented Nov 20, 2024

Payment Summary

BugZero Checklist (@trjExpensify)

trjExpensify commented Nov 20, 2024

mananjadhav commented Nov 20, 2024

c3024 commented Nov 21, 2024

trjExpensify commented Nov 21, 2024

trjExpensify commented Nov 21, 2024

mananjadhav commented Nov 22, 2024

garrettmknight commented Nov 26, 2024

lanitochka17 commented Oct 3, 2024 •

edited by melvin-bot bot

Loading

c3024 commented Oct 4, 2024 •

edited

Loading

mananjadhav commented Oct 4, 2024 •

edited

Loading

trjExpensify commented Oct 4, 2024 •

edited

Loading

c3024 commented Oct 4, 2024 •

edited

Loading