[$250] Workspace - Next page is shown briefly before a loading animation is shown in manual BA flow

[lanitochka17]

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.6-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: https://expensify.testrail.io/index.php?/tests/view/4710752&group_by=cases:section_id&group_id=306204&group_order=asc
Issue reported by: Applause - Internal Team

Action Performed:

1. Open the app
2. Log in with a new expensifail account
3. Create a workspace
4. Navigate to Workspace settings - More features
5. Enable "Workflows"
6. Navigate to Workflows - Connect bank account
7. Enter "011401533" as Routing Number
8. Enter "1111222233331111" as Account Number
9. Tap on the "Next" button
10. Tap on the "Confirm" button
11. Enter "Alberta" as first name
12. Enter "Charleson" as last name
13. Tap on the "Next" button
14. Select any date
15. Tap on the "Next" button
16. Enter "3333" as SSN
17. Tap on the "Next" button
18. Select any suggested address
19. Tap on the "Next" button
20. Enter "Alberta Bobbeth Charleson" as Legal company name
21. Tap on the "Next" button
22. Enter "123456789" as Tax ID
23. Tap on the "Next" button

Expected Result:

The loading animation should appear, and the next page should only be shown after it's completed

Actual Result:

The app flickers every time a new page is opened in the manual bank account flow. The next page is shown briefly before a loading animation is shown. The issue isn't affecting the Plaid flow

Workaround:

Unknown

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

Add any screenshot/video evidence

Bug6538652_1720681257451.az_recorder_20240711_084557.mp4

View all open jobs on GitHub

Upwork Automation - Do Not Edit

• Upwork Job URL: https://www.upwork.com/jobs/~01959c19102edf5000
• Upwork Job ID: 1815178759737094306
• Last Price Increase: 2024-08-05

Issue Owner

Current Issue Owner: @neil-marcellini

[melvin-bot]

Triggered auto assignment to @VictoriaExpensify (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]

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

[bernhardoj]

Proposal

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

The next step shown briefly and one moment loader appears every time completing a sub-step of personal info. This happens in company/business info too.

What is the root cause of that problem?

This happens after this PR where we save each sub-step data. Every time we save a substep, we set the loading data to true.

App/src/libs/actions/BankAccounts.ts

Lines 131 to 138 in 105dce5

	function getVBBADataForOnyx(currentStep?: BankAccountStep): OnyxData {
	return {
	optimisticData: [
	{
	onyxMethod: Onyx.METHOD.MERGE,
	key: ONYXKEYS.REIMBURSEMENT_ACCOUNT,
	value: {
	isLoading: true,

The loading will then show the one moment loading indicator as shown below. The next step appears briefly because we the step is changed before the onyx data is updated.

App/src/pages/ReimbursementAccount/ReimbursementAccountPage.tsx

Lines 391 to 396 in cbd7d39

	// Show loading indicator when page is first time being opened and props.reimbursementAccount yet to be loaded from the server
	// or when data is being loaded. Don't show the loading indicator if we're offline and restarted the bank account setup process
	// On Android, when we open the app from the background, Onfido activity gets destroyed, so we need to reopen it.
	if ((!hasACHDataBeenLoaded || isLoading) && shouldShowOfflineLoader && (shouldReopenOnfido || !requestorStepRef.current)) {
	return <ReimbursementAccountLoadingIndicator onBackButtonPress={goBack} />;
	}

The reason this only happens in Android is because shouldReopenOnfido is true for Android but false for other platforms. The comment above explains why.

App/src/libs/shouldReopenOnfido/index.android.ts

Line 3 in cbd7d39

const shouldReopenOnfido: ShouldReopenOnfido = true;

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

If we are saving a sub-step data, don't set the loading data to true. The loading is only needed when we are confirming the information. We can pass the isConfirmPage from updatePersonalInformationForBankAccount to getVBBADataForOnyx.

App/src/libs/actions/BankAccounts.ts

Lines 292 to 302 in cbd7d39

	function updatePersonalInformationForBankAccount(bankAccountID: number, params: RequestorStepProps, policyID: string, isConfirmPage: boolean) {
	API.write(
	WRITE_COMMANDS.UPDATE_PERSONAL_INFORMATION_FOR_BANK_ACCOUNT,
	{
	...params,
	bankAccountID,
	policyID,
	confirm: isConfirmPage,
	},
	getVBBADataForOnyx(CONST.BANK_ACCOUNT.STEP.REQUESTOR),
	);

Then, set the loading state to true if isConfirmPage is true.

App/src/libs/actions/BankAccounts.ts

Lines 131 to 138 in cbd7d39

	function getVBBADataForOnyx(currentStep?: BankAccountStep): OnyxData {
	return {
	optimisticData: [
	{
	onyxMethod: Onyx.METHOD.MERGE,
	key: ONYXKEYS.REIMBURSEMENT_ACCOUNT,
	value: {
	isLoading: true,

[VictoriaExpensify]

Will review this in the morning

[VictoriaExpensify]

@lanitochka17 I can see the issue you mentioned in your screen recording, but I am not able to recreate it - mine just briefly shows the "we're taking a look at your information" animation. It doesn't flicker the next screen.

It looks like this may be resolved - are you still able to recreate the issue? Is so, is it on iPhone or android?

[VictoriaExpensify]

Asking the above in Slack - https://expensify.slack.com/archives/C9YU7BX5M/p1721199183154919

[kavimuru]

Tester is still able to reproduce

az_recorder_20240717_140538.mp4

[VictoriaExpensify]

Ohh ok you're right. I'm not sure what I did differently in the last test but I'm getting the flicker now

Uploading screen-20240722-101254.mp4…

Adding the required labels

[melvin-bot]

Job added to Upwork: https://www.upwork.com/jobs/~01959c19102edf5000

[melvin-bot]

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

[melvin-bot]

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

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

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

@mollfpr, @VictoriaExpensify 6 days overdue. This is scarier than being forced to listen to Vogon poetry!

[VictoriaExpensify]

@mollfpr can you please review @bernhardoj's proposal and see what you think?

[mollfpr]

Checking 👀

[neil-marcellini]

@bernhardoj could you please help describe the timeline here for this issue, any related issues and regressions? It's quite confusing. The PR was deployed to prod so the next step is just handling payment which @VictoriaExpensify will do.

[bernhardoj]

@neil-marcellini there is no PR yet for this issue. #47345 is a temp fix for #47227. We'll revert #47345 once this issue is resolved (or we can do it in 1 PR).

Ignore this

The last status is @mollfpr approved my proposal and waiting for your review.

[VictoriaExpensify]

It looks like this is just waiting for your review now @neil-marcellini (#45278 (comment))

[neil-marcellini]

Oh ok thanks, I didn't know. Reviewing now.

[neil-marcellini]

The proposal from @bernhardoj looks good to me!

🎀 👀 🎀 C+ reviewed!

Looks good to me too, hiring

[bernhardoj]

PR is ready

cc: @mollfpr

[VictoriaExpensify]

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:

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

[VictoriaExpensify]

Payment summary:
Contributor: @bernhardoj owed $250 via NewDot

@mollfpr I'll organise your payment once the checklist is complete

[bernhardoj]

Requested in ND.

[JmillsExpensify]

$250 approved for @bernhardoj

[mollfpr]

[@mollfpr] The PR that introduced the bug has been identified. Link to the PR:
[@mollfpr] 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:

#37680

[@mollfpr] 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 regression step should be enough.

[@mollfpr] Determine if we should create a regression test for this bug.
[@mollfpr] 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.

1. Open workspace workflow page
2. Press Connect bank account
3. Complete the first 4 steps of the BA
4. Verify that each time you save a sub-step, no loading animation is shown.
5. Verify that each time you confirm in a confirmation step, a loading animation is shown (except personal info where only loading is shown on the button)
6. 👍 or 👎

[mollfpr]

@VictoriaExpensify Could you give the payment summary? Thank you!

[VictoriaExpensify]

Thanks @mollfpr !

Raised an issue to add the regression test to test rail - https://github.com/Expensify/Expensify/issues/429060

[VictoriaExpensify]

Payment summary:
Contributor+: @mollfpr owed $250 via NewDot

[garrettmknight]

$250 approved for @mollfpr