[HOLD for payment 2024-08-29] [CVP] [$250] Default Selected Tab is Scan when submitting an expense

[m-natarajan]

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.17-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:
Email or phone of affected tester (no customers):
Logs: https://stackoverflow.com/c/expensify/questions/4856
Expensify/Expensify Issue URL:
Issue reported by: @davidcardoza
Slack conversation: https://expensify.slack.com/archives/C049HHMV9SM/p1722959330691129

Action Performed:

1. Navigate to the app on either the mobile or web platform.
2. Open Global create > Tap submit expense

Expected Result:

The default selected option should be set to "Scan."

• The issue was identified as a regression via this GH PR.
• We changed the default selectedTab to Scan during the Saastr activation.
• It appears that this file no longer exists, likely broken during a TypeScript migration.

Actual Result:

The default selected option is set to "Manual."

Workaround:

Can the user still use Expensify without this being fixed? Have you informed them of the workaround?

Platforms:

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

• Android: Native
• Android: mWeb Chrome
• iOS: Native
• iOS: mWeb Safari
• MacOS: Chrome / Safari
• MacOS: Desktop

Screenshots/Videos

RPReplay_Final1722912795.MP4

• The issue was identified as a regression via this GH PR.
• We changed the default selectedTab to Scan during the Saastr activation.
• It appears that this file no longer exists, likely broken during a TypeScript migration.

View all open jobs on GitHub

Upwork Automation - Do Not Edit

• Upwork Job URL: https://www.upwork.com/jobs/~010e779507128f89c4
• Upwork Job ID: 1821296903042091088
• Last Price Increase: 2024-08-07
• Automatic offers:
• brunovjk | Reviewer | 103493720
• dominictb | Contributor | 103493721

Issue Owner

Current Issue Owner: @trjExpensify

[melvin-bot]

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

[dominictb]

Proposal

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

The default selected option is set to "Manual."

What is the root cause of that problem?

When clicking on "Submit expense" if ${ONYXKEYS.COLLECTION.SELECTED_TAB}${CONST.TAB.IOU_REQUEST_TYPE} is empty, the initialRouteName in navigator:

App/src/libs/Navigation/OnyxTabNavigator.tsx

Line 46 in de89479

initialRouteName={selectedTab}

is undefined, hence the index in:

App/src/libs/Navigation/OnyxTabNavigator.tsx

Line 53 in de89479

const index = state.index;

is 0 intitially, then it call Tab.setSelectedTab("iouRequestType", "manually"):

App/src/libs/Navigation/OnyxTabNavigator.tsx

Line 55 in de89479

Tab.setSelectedTab(id, routeNames[index] as SelectedTabRequest);

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

We should add the fallback value to the initialRouteName:

App/src/libs/Navigation/OnyxTabNavigator.tsx

Line 46 in de89479

initialRouteName={selectedTab}

so that if the selectedTab in undefined, we fallback to scan tab.

The detail changes:

App/src/libs/Navigation/OnyxTabNavigator.tsx

Line 40 in de89479

function OnyxTabNavigator({id, selectedTab, children, onTabSelected = () => {}, screenListeners, ...rest}: OnyxTabNavigatorProps) {

function OnyxTabNavigator({id, defaultTab, selectedTab, children, onTabSelected = () => {}, screenListeners, ...rest}: OnyxTabNavigatorProps) {

and

App/src/libs/Navigation/OnyxTabNavigator.tsx

Line 46 in de89479

initialRouteName={selectedTab}

            initialRouteName={selectedTab || defaultTab}

and

App/src/pages/iou/request/IOURequestStartPage.tsx

Line 139 in de89479

tabBar={TabSelector}

                                    tabBar={TabSelector}
                                    defaultTab={'scan'}

What alternative solutions did you explore? (Optional)

[melvin-bot]

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

[melvin-bot]

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

[brunovjk]

@trjExpensify, could you clarify one thing please? the 'Scan' tab should be the default across the entire app, or just when submitting an expense? Thanks

[trjExpensify]

What do you mean by "entire app"?

or just when submitting an expense?

also, just to clarify as well, it's only the default for the first time you use Submit expense. Thereafter, your choice is remembered, as it is today. The bug is just that the default for the first time using Submit expense is Manual instead of Scan.

[brunovjk]

@trjExpensify, sorry for any confusion in my poor choose of words hehe Im lucky you understood correctly, and thanks for clarifying. Given this, the proposal RCA seems accurate to me; selectedTab is indeed undefined initially, causing the default to be Manual due to the 0 index. However, I've tested the proposed solution and noticed it briefly displays the Manual tab before switching to Scan.

Screen.Recording.2024-08-09.at.18.59.56.mov

@dominictb Do you have a branch with this change, or perhaps an alternative approach? Thanks!

[trjExpensify]

No worries! If it's helpful, this is the PR where we added this initially before the TypeScript migration: #26984

[dominictb]

@brunovjk The draft branch here.

[BhuvaneshPatil]

Proposal

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

When submitting an expense not opening selected tab but manual everytime.

What is the root cause of that problem?

To get previously selected tab we use onyx data -

App/src/libs/Navigation/OnyxTabNavigator.tsx

Lines 69 to 73 in e6546d6

	export default withOnyx<OnyxTabNavigatorProps, OnyxTabNavigatorOnyxProps>({
	selectedTab: {
	key: ({id}) => `${ONYXKEYS.COLLECTION.SELECTED_TAB}${id}`,
	},
	})(OnyxTabNavigator);

But in case of it's not present in Onyx, the value initialRouteName is undefined. As it's undefined we use 0 index type as request type.

App/src/libs/Navigation/OnyxTabNavigator.tsx

Lines 50 to 57 in e6546d6

	state: (e) => {
	const event = e as unknown as EventMapCore<NavigationState>['state'];
	const state = event.data.state;
	const index = state.index;
	const routeNames = state.routeNames;
	Tab.setSelectedTab(id, routeNames[index] as SelectedTabRequest);
	onTabSelected(routeNames[index] as IOURequestType);
	},

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

I am assuming we want this action is performed from FAB's Submit Expense action and I have tailored my solution for that.

1. When we click on Submit Expense button, we call IOU.startMoneyRequest(). This function also takes conditional parameter for it, requestType -

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

Lines 395 to 407 in e6546d6

	{
	icon: getIconForAction(CONST.IOU.TYPE.REQUEST),
	text: translate('iou.submitExpense'),
	onSelected: () =>
	interceptAnonymousUser(() =>
	IOU.startMoneyRequest(
	CONST.IOU.TYPE.SUBMIT,
	// When starting to create an expense from the global FAB, there is not an existing report yet. A random optimistic reportID is generated and used
	// for all of the routes in the creation flow.
	ReportUtils.generateReportID(),
	),
	),
	},

3. We can leverage this param for this case. We will add selectedRequestTab (or some other name) to onyx prop for FloatingActionButtonAndPopover component. That will give us value for selected tab from onyx. Its key will be -

   selectedRequestTab: {
        key: `${ONYXKEYS.COLLECTION.SELECTED_TAB}${CONST.TAB.IOU_REQUEST_TYPE}`
    }

and based on value of this we will pass new argument to mentioned function -

onSelected: () =>
                            interceptAnonymousUser(() =>
                                IOU.startMoneyRequest(
                                    CONST.IOU.TYPE.SUBMIT,
                                    // When starting to create an expense from the global FAB, there is not an existing report yet. A random optimistic reportID is generated and used
                                    // for all of the routes in the creation flow.
                                    ReportUtils.generateReportID(),
                                    selectedRequestTab ? undefined : 'scan',
                                ),
                            ),

What alternative solutions did you explore? (Optional)

Second option is to make changes OnyxTabNavigator.

We remove selectedTab from props and HOC of onyx and use useOnyx instead.

As this component is used at some other places as well, we can add check of id to be CONST.TAB.IOU_REQUEST_TYPE and conditionally return 'scan'

const [selectedTab] = useOnyx(`${ONYXKEYS.COLLECTION.SELECTED_TAB}${id}`, {
    selector: (value) => {
        if (value) {
            return value;
        }
        return 'scan';
    },
});

This will work as expected without that lag where it shows manual for sometime.

scan.mp4

[dominictb]

@brunovjk I cannot reproduce bug with my solution. Can you give me the test steps?

[dominictb]

@BhuvaneshPatil, I noticed that you re-submitted a new proposal after C+ identified a bug in my solution. You mentioned that your solution will work as expected without the lag where it shows 'manual' for a moment, implying that it addresses the issue present in my solution.

Could you explain why my solution encountered that bug? Thank you!

[BhuvaneshPatil]

@dominictb

About re-submitting the proposal, I mis-understood the problem during submitting that proposal, But after comment that it should open what previously selected, I deleted my proposal.

About lag, that is mentioned in comment I didn't notice that in my solution. I am not implying anything. Let @brunovjk Test my solution as well. Thanks

[dominictb]

@BhuvaneshPatil

You mentioned that your solution will work as expected without the lag where it shows 'manual' for a moment, implying that it addresses the issue present in my solution.

I said that because I refer to your comment:

This will work as expected without that lag where it shows manual for sometime.

[brunovjk]

@brunovjk I cannot reproduce bug with my solution. Can you give me the test steps?

@dominictb Thank you for the branch. I tested it and observed the same result:

1. Log in with a new account.
2. Click on the global FAB > Submit Expense.
3. Verify that the default tab is indeed Scan, but it briefly switches to Manual before returning to Scan.

Screen.Recording.2024-08-10.at.20.10.57.mov

[brunovjk]

Thank you very much for the proposals, @dominictb and @BhuvaneshPatil. However, it seems that our RCA is not entirely accurate. I’ve attempted several approaches to set a default value for selectedTab, but I consistently see manual flash briefly before it settles. I experimented with different types, similar to the approach in the old PR mentioned here, and also considered cleaning up OnyxTabNavigator, where we have two selectedTab type keys there that could potentially use useOnyx,

App/src/libs/Navigation/OnyxTabNavigator.tsx

Lines 16 to 25 in 93dc1c9

	selectedTab: OnyxEntry<SelectedTabRequest>;
	};
	type OnyxTabNavigatorProps = OnyxTabNavigatorOnyxProps &
	ChildrenProps & {
	/** ID of the tab component to be saved in onyx */
	id: string;
	/** Name of the selected tab */
	selectedTab?: SelectedTabRequest;

but unfortunately, this did not resolve the issue. I suggest we further research and investigate the root cause. Let’s Smash this bug! :D

[dominictb]

• Thanks for your test steps. I tested in desktop web and ios native, but I still cannot reproduce the bug:

Screen.Recording.2024-08-11.at.07.45.06.mov

Screen.Recording.2024-08-11.at.07.48.31.mov

[BhuvaneshPatil]

@brunovjk Yes on two pages we use onyx key to set default tab - new chat/room page and submit expense step.

Did you try the approach mentioned in my proposal?

[BhuvaneshPatil]

@dominictb I am not sure about desktop app or IOS, but you can follow the steps similar to attached video in my proposal

[brunovjk]

@brunovjk Yes on two pages we use onyx key to set default tab - new chat/room page and submit expense step.

Did you try the approach mentioned in my proposal?

Yes @BhuvaneshPatil , and I get the same behavior as the @dominictb solution, I'm starting to think it's something on my side. Tomorrow I will come back and test your proposals further, if I continue to have the same problem I will tag an intern to test it. Thank you very much :D

[dominictb]

FYI, we might have encountered a similar blinking issue here. However, I wasn't able to reproduce that issue, so I can't confirm if it's the same as this one or whether the proposed solution there would resolve it.

[brunovjk]

After further investigation, I identified a few issues we should address:

1. Replace withOnyx with useOnyx in src/libs/Navigation/OnyxTabNavigator.tsx and create a second variable to handle the selectedTab from Onyx separately from the "default" selectedTab passed as a component prop.
2. In some point after TypeScript migration, we lost the default tab being passed to src/pages/iou/request/IOURequestStartPage.tsx.

I believe we can proceed with @dominictb's proposal. However, I suggest we tweak the PR slightly, such as using CONST.TAB_REQUEST.SCAN instead of the string "scan". We can fine-tune these details in the there.

Thank you for the proposal @BhuvaneshPatil. Both proposals are very similar, but given that dominictb posted first, I think we can proceed with their solution.

🎀👀🎀 C+ reviewed

[melvin-bot]

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

[mountiny]

+1 to using useOnyx where we can

[dominictb]

PR is ready for review

[melvin-bot]

@trjExpensify, @mountiny, @brunovjk, @dominictb Whoops! This issue is 2 days overdue. Let's get this updated quick!

[brunovjk]

PR under review

[melvin-bot]

@trjExpensify @mountiny @brunovjk @dominictb 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!

[brunovjk]

PR going through deploy checklist: #47760

[melvin-bot]

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

• fix: default the selected tab to scan #47310

If no regressions arise, payment will be issued on 2024-08-29. 🎊

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

• @brunovjk requires payment automatic offer (Reviewer)
• @dominictb requires payment automatic offer (Contributor)

[melvin-bot]

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:

• [@brunovjk] The PR that introduced the bug has been identified. Link to the PR:
• [@brunovjk] 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:
• [@brunovjk] 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:
• [@brunovjk] Determine if we should create a regression test for this bug.
• [@brunovjk] 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.
• [@trjExpensify] Link the GH issue for creating/updating the regression test once above steps have been agreed upon:

[brunovjk]

• [@brunovjk] The PR that introduced the bug has been identified. Link to the PR: [TS migration] Migrate IOURequestStartPage, IOURequestRedirectToStartPage, IOURequestStepDistance and IOURequestStepAmount pages to TypeScript #39910
• [@brunovjk] 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: [TS migration] Migrate IOURequestStartPage, IOURequestRedirectToStartPage, IOURequestStepDistance and IOURequestStepAmount pages to TypeScript #39910 (comment)
• [@brunovjk] 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: - The issue seems to be an isolated incident rather than a systemic problem. The removal of selectedTab was correct, but we missed retaining the default value for cases where Onyx doesn't provide selectedTab. Since this doesn't indicate a broader issue with our review process, a discussion might not be necessary at this time.
• [@brunovjk] Determine if we should create a regression test for this bug. Yes
• [@brunovjk] 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.

Regression Test Proposal

• Login to a new account (Or clear the Onyx state in the troubleshooting settings)
• Navigate to the app on either the mobile or web platform.
• Open Global create > Tap submit expense
• Verify that the default selected tab is Scan
• Do we agree 👍 or 👎

[trjExpensify]

👋 Not to nitpick, but pretty sure there must be a PR that caused this regression because it was working when we introduced it for SaaStr. Equally, "N/A" with no other context isn't overly helpful to convey your thinking on why something in the checklist is "not applicable".

[brunovjk]

👋 Not to nitpick, but pretty sure there must be a PR that caused this regression because it was working when we introduced it for SaaStr.

Not nitpicking at all, I appreciate the feedback and apologize for the carelessness. I will investigate and find the PR

[brunovjk]

Hi @trjExpensify, I've updated the checklist. Could you please take a look and let me know if everything looks good now? Thanks

[melvin-bot]

@trjExpensify, @mountiny, @brunovjk, @dominictb Whoops! This issue is 2 days overdue. Let's get this updated quick!

[trjExpensify]

Excellent, thanks for that!

Payment summary as follows:

• $250 to @brunovjk for the C+ review
• $250 to @dominictb for the fix

Paid you both, closing!