From ae7a7049f1bbebcc8be8a1bcfadc3de58407acff Mon Sep 17 00:00:00 2001 From: Agata Kosior Date: Wed, 25 Sep 2024 13:46:52 +0700 Subject: [PATCH 01/12] fix: restast the flow for another policy --- .../actions/ReimbursementAccount/index.ts | 14 +++- .../ReimbursementAccountPage.tsx | 78 ++++++++++--------- 2 files changed, 55 insertions(+), 37 deletions(-) diff --git a/src/libs/actions/ReimbursementAccount/index.ts b/src/libs/actions/ReimbursementAccount/index.ts index 62e5f8454cbd..c7c2d8e237d5 100644 --- a/src/libs/actions/ReimbursementAccount/index.ts +++ b/src/libs/actions/ReimbursementAccount/index.ts @@ -26,6 +26,10 @@ function updateReimbursementAccountDraft(bankAccountData: Partial(fieldNames: T[]): Pick { return { @@ -150,34 +180,6 @@ function ReimbursementAccountPage({ }; } - /** - * Returns selected bank account fields based on field names provided. - */ - function getFieldsForStep(step: TBankAccountStep): InputID[] { - switch (step) { - case CONST.BANK_ACCOUNT.STEP.BANK_ACCOUNT: - return ['routingNumber', 'accountNumber', 'bankName', 'plaidAccountID', 'plaidAccessToken', 'isSavings']; - case CONST.BANK_ACCOUNT.STEP.COMPANY: - return [ - 'companyName', - 'addressStreet', - 'addressZipCode', - 'addressCity', - 'addressState', - 'companyPhone', - 'website', - 'companyTaxID', - 'incorporationType', - 'incorporationDate', - 'incorporationState', - ]; - case CONST.BANK_ACCOUNT.STEP.REQUESTOR: - return ['firstName', 'lastName', 'dob', 'ssnLast4', 'requestorAddressStreet', 'requestorAddressCity', 'requestorAddressState', 'requestorAddressZipCode']; - default: - return []; - } - } - /** * Returns true if a VBBA exists in any state other than OPEN or LOCKED */ @@ -203,8 +205,7 @@ function ReimbursementAccountPage({ the full `reimbursementAccount` data from the server. This logic is handled within the useEffect hook, which acts similarly to `componentDidUpdate` when the `reimbursementAccount` dependency changes. */ - const [hasACHDataBeenLoaded, setHasACHDataBeenLoaded] = useState(reimbursementAccount !== CONST.REIMBURSEMENT_ACCOUNT.DEFAULT_DATA); - + const [hasACHDataBeenLoaded, setHasACHDataBeenLoaded] = useState(reimbursementAccount !== CONST.REIMBURSEMENT_ACCOUNT.DEFAULT_DATA && isPreviousPolicy); const [shouldShowContinueSetupButton, setShouldShowContinueSetupButton] = useState(hasACHDataBeenLoaded ? getShouldShowContinueSetupButtonInitialValue() : false); const [isReimbursementAccountLoading, setIsReimbursementAccountLoading] = useState(() => { // By default return true (loading), if there are already loaded data we can skip the loading state @@ -244,6 +245,10 @@ function ReimbursementAccountPage({ } useEffect(() => { + if (!isPreviousPolicy) { + ReimbursementAccount.clearReimbursementAccountDraft(); + } + if (!isReimbursementAccountLoading) { return; } @@ -263,6 +268,7 @@ function ReimbursementAccountPage({ return; } setIsReimbursementAccountLoading(reimbursementAccount.isLoading); + setHasACHDataBeenLoaded(true); }, [prevIsReimbursementAccountLoading, reimbursementAccount?.isLoading]); useEffect( From d7d04116563eef89f8ccd989f1c08fac782273cc Mon Sep 17 00:00:00 2001 From: Agata Kosior Date: Wed, 25 Sep 2024 20:12:39 +0700 Subject: [PATCH 02/12] fix: replace withOnyx for useOnyx --- .../ReimbursementAccountPage.tsx | 179 ++++++------------ 1 file changed, 56 insertions(+), 123 deletions(-) diff --git a/src/pages/ReimbursementAccount/ReimbursementAccountPage.tsx b/src/pages/ReimbursementAccount/ReimbursementAccountPage.tsx index 9812aafaf96a..af4d425ee849 100644 --- a/src/pages/ReimbursementAccount/ReimbursementAccountPage.tsx +++ b/src/pages/ReimbursementAccount/ReimbursementAccountPage.tsx @@ -4,11 +4,11 @@ import {Str} from 'expensify-common'; import lodashPick from 'lodash/pick'; import React, {useEffect, useRef, useState} from 'react'; import {View} from 'react-native'; -import type {OnyxEntry} from 'react-native-onyx'; -import {withOnyx} from 'react-native-onyx'; +import {useOnyx} from 'react-native-onyx'; import type {ValueOf} from 'type-fest'; import FullPageNotFoundView from '@components/BlockingViews/FullPageNotFoundView'; import HeaderWithBackButton from '@components/HeaderWithBackButton'; +import {useSession} from '@components/OnyxProvider'; import ReimbursementAccountLoadingIndicator from '@components/ReimbursementAccountLoadingIndicator'; import ScreenWrapper from '@components/ScreenWrapper'; import Text from '@components/Text'; @@ -31,7 +31,6 @@ import ONYXKEYS from '@src/ONYXKEYS'; import ROUTES from '@src/ROUTES'; import type SCREENS from '@src/SCREENS'; import type {InputID} from '@src/types/form/ReimbursementAccountForm'; -import type * as OnyxTypes from '@src/types/onyx'; import type {ACHDataReimbursementAccount, BankAccountStep as TBankAccountStep} from '@src/types/onyx/ReimbursementAccount'; import {isEmptyObject} from '@src/types/utils/EmptyObject'; import ACHContractStep from './ACHContractStep'; @@ -43,32 +42,7 @@ import ContinueBankAccountSetup from './ContinueBankAccountSetup'; import EnableBankAccount from './EnableBankAccount/EnableBankAccount'; import RequestorStep from './RequestorStep'; -type ReimbursementAccountOnyxProps = { - /** Plaid SDK token to use to initialize the widget */ - plaidLinkToken: OnyxEntry; - - /** Plaid SDK current event */ - plaidCurrentEvent: OnyxEntry; - - /** Indicated whether the app is loading */ - isLoadingApp: OnyxEntry; - - /** Holds information about the users account that is logging in */ - account: OnyxEntry; - - /** Current session for the user */ - session: OnyxEntry; - - /** ACH data for the withdrawal account actively being set up */ - reimbursementAccount: OnyxEntry; - - /** The token required to initialize the Onfido SDK */ - onfidoToken: OnyxEntry; -}; - -type ReimbursementAccountPageProps = WithPolicyOnyxProps & - ReimbursementAccountOnyxProps & - StackScreenProps; +type ReimbursementAccountPageProps = WithPolicyOnyxProps & StackScreenProps; const ROUTE_NAMES = { COMPANY: 'company', @@ -153,17 +127,14 @@ function getFieldsForStep(step: TBankAccountStep): InputID[] { } } -function ReimbursementAccountPage({ - reimbursementAccount, - route, - onfidoToken = '', - policy, - account, - isLoadingApp = false, - session, - plaidLinkToken = '', - plaidCurrentEvent = '', -}: ReimbursementAccountPageProps) { +function ReimbursementAccountPage({route, policy}: ReimbursementAccountPageProps) { + const session = useSession(); + const [isLoadingApp] = useOnyx(ONYXKEYS.IS_LOADING_APP); + const [account] = useOnyx(ONYXKEYS.ACCOUNT); + const [onfidoToken] = useOnyx(ONYXKEYS.ONFIDO_TOKEN); + const [reimbursementAccount] = useOnyx(ONYXKEYS.REIMBURSEMENT_ACCOUNT); + const [plaidLinkToken] = useOnyx(ONYXKEYS.PLAID_LINK_TOKEN); + const [plaidCurrentEvent] = useOnyx(ONYXKEYS.PLAID_CURRENT_EVENT); /** The SetupWithdrawalAccount flow allows us to continue the flow from various points depending on where the user left off. This view will refer to the achData as the single source of truth to determine which route to @@ -184,8 +155,9 @@ function ReimbursementAccountPage({ * Returns true if a VBBA exists in any state other than OPEN or LOCKED */ function hasInProgressVBBA(): boolean { - return !!achData?.bankAccountID && achData?.state !== BankAccount.STATE.OPEN && achData?.state !== BankAccount.STATE.LOCKED; + return isPreviousPolicy && !!achData?.bankAccountID && achData?.state !== BankAccount.STATE.OPEN && achData?.state !== BankAccount.STATE.LOCKED; } + /* * Calculates the state used to show the "Continue with setup" view. If a bank account setup is already in progress and * no specific further step was passed in the url we'll show the workspace bank account reset modal if the user wishes to start over @@ -206,17 +178,17 @@ function ReimbursementAccountPage({ which acts similarly to `componentDidUpdate` when the `reimbursementAccount` dependency changes. */ const [hasACHDataBeenLoaded, setHasACHDataBeenLoaded] = useState(reimbursementAccount !== CONST.REIMBURSEMENT_ACCOUNT.DEFAULT_DATA && isPreviousPolicy); - const [shouldShowContinueSetupButton, setShouldShowContinueSetupButton] = useState(hasACHDataBeenLoaded ? getShouldShowContinueSetupButtonInitialValue() : false); + const [shouldShowContinueSetupButton, setShouldShowContinueSetupButton] = useState(getShouldShowContinueSetupButtonInitialValue()); const [isReimbursementAccountLoading, setIsReimbursementAccountLoading] = useState(() => { // By default return true (loading), if there are already loaded data we can skip the loading state - if (hasACHDataBeenLoaded && typeof reimbursementAccount?.isLoading === 'boolean' && !reimbursementAccount?.isLoading) { + if (isPreviousPolicy && hasACHDataBeenLoaded && typeof reimbursementAccount?.isLoading === 'boolean' && !reimbursementAccount?.isLoading) { return false; } return true; }); // eslint-disable-next-line @typescript-eslint/prefer-nullish-coalescing - const currentStep = achData?.currentStep || CONST.BANK_ACCOUNT.STEP.BANK_ACCOUNT; + const currentStep = !isPreviousPolicy ? CONST.BANK_ACCOUNT.STEP.BANK_ACCOUNT : achData?.currentStep || CONST.BANK_ACCOUNT.STEP.BANK_ACCOUNT; const policyName = policy?.name ?? ''; const policyIDParam = route.params?.policyID ?? '-1'; const styles = useThemeStyles(); @@ -249,10 +221,6 @@ function ReimbursementAccountPage({ ReimbursementAccount.clearReimbursementAccountDraft(); } - if (!isReimbursementAccountLoading) { - return; - } - // If the step to open is empty, we want to clear the sub step, so the connect option view is shown to the user const isStepToOpenEmpty = getStepToOpenFromRouteParams(route) === ''; if (isStepToOpenEmpty) { @@ -391,7 +359,7 @@ function ReimbursementAccountPage({ } }; - const isLoading = (!!isLoadingApp || !!account?.isLoading || isReimbursementAccountLoading) && (!plaidCurrentEvent || plaidCurrentEvent === CONST.BANK_ACCOUNT.PLAID.EVENTS_NAME.EXIT); + const isLoading = (!!isLoadingApp || !!account?.isLoading) && (!plaidCurrentEvent || plaidCurrentEvent === CONST.BANK_ACCOUNT.PLAID.EVENTS_NAME.EXIT); const shouldShowOfflineLoader = !( isOffline && @@ -407,7 +375,7 @@ function ReimbursementAccountPage({ // 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)) { + if (isLoading && shouldShowOfflineLoader && (shouldReopenOnfido || !requestorStepRef.current)) { return ; } @@ -463,81 +431,46 @@ function ReimbursementAccountPage({ ); } - if (currentStep === CONST.BANK_ACCOUNT.STEP.BANK_ACCOUNT) { - return ( - - ); - } - - if (currentStep === CONST.BANK_ACCOUNT.STEP.COMPANY) { - return ; - } - - if (currentStep === CONST.BANK_ACCOUNT.STEP.REQUESTOR) { - const shouldShowOnfido = onfidoToken && !achData?.isOnfidoSetupComplete; - return ( - - ); - } - - if (currentStep === CONST.BANK_ACCOUNT.STEP.BENEFICIAL_OWNERS) { - return ; - } - - if (currentStep === CONST.BANK_ACCOUNT.STEP.ACH_CONTRACT) { - return ; - } - - if (currentStep === CONST.BANK_ACCOUNT.STEP.VALIDATION) { - return ; - } - - if (currentStep === CONST.BANK_ACCOUNT.STEP.ENABLE) { - return ( - - ); + switch (currentStep) { + case CONST.BANK_ACCOUNT.STEP.BANK_ACCOUNT: + return ( + + ); + case CONST.BANK_ACCOUNT.STEP.REQUESTOR: + return ( + + ); + case CONST.BANK_ACCOUNT.STEP.COMPANY: + return ; + case CONST.BANK_ACCOUNT.STEP.BENEFICIAL_OWNERS: + return ; + case CONST.BANK_ACCOUNT.STEP.ACH_CONTRACT: + return ; + case CONST.BANK_ACCOUNT.STEP.VALIDATION: + return ; + case CONST.BANK_ACCOUNT.STEP.ENABLE: + return ( + + ); + default: + return null; } } ReimbursementAccountPage.displayName = 'ReimbursementAccountPage'; -export default withPolicy( - withOnyx({ - // @ts-expect-error: ONYXKEYS.REIMBURSEMENT_ACCOUNT is conflicting with ONYXKEYS.FORMS.REIMBURSEMENT_ACCOUNT_FORM - reimbursementAccount: { - key: ONYXKEYS.REIMBURSEMENT_ACCOUNT, - }, - session: { - key: ONYXKEYS.SESSION, - }, - plaidLinkToken: { - key: ONYXKEYS.PLAID_LINK_TOKEN, - }, - plaidCurrentEvent: { - key: ONYXKEYS.PLAID_CURRENT_EVENT, - }, - onfidoToken: { - key: ONYXKEYS.ONFIDO_TOKEN, - }, - isLoadingApp: { - key: ONYXKEYS.IS_LOADING_APP, - }, - account: { - key: ONYXKEYS.ACCOUNT, - }, - })(ReimbursementAccountPage), -); +export default withPolicy(ReimbursementAccountPage); From e88d940a1f01dd695edf94917a31b0ab09cee01b Mon Sep 17 00:00:00 2001 From: Agata Kosior Date: Thu, 26 Sep 2024 13:45:33 +0700 Subject: [PATCH 03/12] fix: last changes and cleanup --- .../ReimbursementAccountPage.tsx | 29 +++++++------------ 1 file changed, 10 insertions(+), 19 deletions(-) diff --git a/src/pages/ReimbursementAccount/ReimbursementAccountPage.tsx b/src/pages/ReimbursementAccount/ReimbursementAccountPage.tsx index af4d425ee849..b692b3dd1c5a 100644 --- a/src/pages/ReimbursementAccount/ReimbursementAccountPage.tsx +++ b/src/pages/ReimbursementAccount/ReimbursementAccountPage.tsx @@ -179,13 +179,6 @@ function ReimbursementAccountPage({route, policy}: ReimbursementAccountPageProps */ const [hasACHDataBeenLoaded, setHasACHDataBeenLoaded] = useState(reimbursementAccount !== CONST.REIMBURSEMENT_ACCOUNT.DEFAULT_DATA && isPreviousPolicy); const [shouldShowContinueSetupButton, setShouldShowContinueSetupButton] = useState(getShouldShowContinueSetupButtonInitialValue()); - const [isReimbursementAccountLoading, setIsReimbursementAccountLoading] = useState(() => { - // By default return true (loading), if there are already loaded data we can skip the loading state - if (isPreviousPolicy && hasACHDataBeenLoaded && typeof reimbursementAccount?.isLoading === 'boolean' && !reimbursementAccount?.isLoading) { - return false; - } - return true; - }); // eslint-disable-next-line @typescript-eslint/prefer-nullish-coalescing const currentStep = !isPreviousPolicy ? CONST.BANK_ACCOUNT.STEP.BANK_ACCOUNT : achData?.currentStep || CONST.BANK_ACCOUNT.STEP.BANK_ACCOUNT; @@ -205,9 +198,6 @@ function ReimbursementAccountPage({route, policy}: ReimbursementAccountPageProps * @param ignoreLocalSubStep Pass true if you want the last "updated" view (from db), not the last "viewed" view (from onyx). */ function fetchData(ignoreLocalCurrentStep?: boolean, ignoreLocalSubStep?: boolean) { - // Show loader right away, as optimisticData might be set only later in case multiple calls are in the queue - BankAccounts.setReimbursementAccountLoading(true); - // We can specify a step to navigate to by using route params when the component mounts. // We want to use the same stepToOpen variable when the network state changes because we can be redirected to a different step when the account refreshes. const stepToOpen = getStepToOpenFromRouteParams(route); @@ -217,17 +207,20 @@ function ReimbursementAccountPage({route, policy}: ReimbursementAccountPageProps } useEffect(() => { - if (!isPreviousPolicy) { - ReimbursementAccount.clearReimbursementAccountDraft(); + if (isPreviousPolicy) { + return; } + BankAccounts.setReimbursementAccountLoading(true); + ReimbursementAccount.clearReimbursementAccountDraft(); + // If the step to open is empty, we want to clear the sub step, so the connect option view is shown to the user const isStepToOpenEmpty = getStepToOpenFromRouteParams(route) === ''; if (isStepToOpenEmpty) { BankAccounts.setBankAccountSubStep(null); BankAccounts.setPlaidEvent(null); } - fetchData(false, isStepToOpenEmpty); + fetchData(true, isStepToOpenEmpty); // eslint-disable-next-line react-compiler/react-compiler, react-hooks/exhaustive-deps }, []); // The empty dependency array ensures this runs only once after the component mounts. @@ -235,7 +228,6 @@ function ReimbursementAccountPage({route, policy}: ReimbursementAccountPageProps if (typeof reimbursementAccount?.isLoading !== 'boolean' || reimbursementAccount.isLoading === prevIsReimbursementAccountLoading) { return; } - setIsReimbursementAccountLoading(reimbursementAccount.isLoading); setHasACHDataBeenLoaded(true); }, [prevIsReimbursementAccountLoading, reimbursementAccount?.isLoading]); @@ -247,7 +239,7 @@ function ReimbursementAccountPage({route, policy}: ReimbursementAccountPageProps } if (!hasACHDataBeenLoaded) { - if (reimbursementAccount !== CONST.REIMBURSEMENT_ACCOUNT.DEFAULT_DATA && isReimbursementAccountLoading === false) { + if (reimbursementAccount !== CONST.REIMBURSEMENT_ACCOUNT.DEFAULT_DATA && reimbursementAccount?.isLoading === false) { setShouldShowContinueSetupButton(getShouldShowContinueSetupButtonInitialValue()); setHasACHDataBeenLoaded(true); } @@ -290,10 +282,9 @@ function ReimbursementAccountPage({route, policy}: ReimbursementAccountPageProps BankAccounts.hideBankAccountErrors(); } - const policyID = route.params.policyID; const backTo = route.params.backTo; - Navigation.navigate(ROUTES.BANK_ACCOUNT_WITH_STEP_TO_OPEN.getRoute(getRouteForCurrentStep(currentStep), policyID, backTo)); + Navigation.navigate(ROUTES.BANK_ACCOUNT_WITH_STEP_TO_OPEN.getRoute(getRouteForCurrentStep(currentStep), policyIDParam, backTo)); }, // eslint-disable-next-line react-compiler/react-compiler, react-hooks/exhaustive-deps [isOffline, reimbursementAccount, route, hasACHDataBeenLoaded, shouldShowContinueSetupButton], @@ -359,7 +350,7 @@ function ReimbursementAccountPage({route, policy}: ReimbursementAccountPageProps } }; - const isLoading = (!!isLoadingApp || !!account?.isLoading) && (!plaidCurrentEvent || plaidCurrentEvent === CONST.BANK_ACCOUNT.PLAID.EVENTS_NAME.EXIT); + const isLoading = (!!isLoadingApp || !!account?.isLoading || reimbursementAccount?.isLoading) && (!plaidCurrentEvent || plaidCurrentEvent === CONST.BANK_ACCOUNT.PLAID.EVENTS_NAME.EXIT); const shouldShowOfflineLoader = !( isOffline && @@ -375,7 +366,7 @@ function ReimbursementAccountPage({route, policy}: ReimbursementAccountPageProps // 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 (isLoading && shouldShowOfflineLoader && (shouldReopenOnfido || !requestorStepRef.current)) { + if ((!hasACHDataBeenLoaded || isLoading) && shouldShowOfflineLoader && (shouldReopenOnfido || !requestorStepRef.current)) { return ; } From 98b1256be20fd2e43b4afa1426e885b0628b185f Mon Sep 17 00:00:00 2001 From: Agata Kosior Date: Thu, 26 Sep 2024 13:48:49 +0700 Subject: [PATCH 04/12] fix: cleanup --- .../ReimbursementAccountPage.tsx | 47 ++++++++++--------- 1 file changed, 24 insertions(+), 23 deletions(-) diff --git a/src/pages/ReimbursementAccount/ReimbursementAccountPage.tsx b/src/pages/ReimbursementAccount/ReimbursementAccountPage.tsx index b692b3dd1c5a..aac63db6a29a 100644 --- a/src/pages/ReimbursementAccount/ReimbursementAccountPage.tsx +++ b/src/pages/ReimbursementAccount/ReimbursementAccountPage.tsx @@ -135,6 +135,17 @@ function ReimbursementAccountPage({route, policy}: ReimbursementAccountPageProps const [reimbursementAccount] = useOnyx(ONYXKEYS.REIMBURSEMENT_ACCOUNT); const [plaidLinkToken] = useOnyx(ONYXKEYS.PLAID_LINK_TOKEN); const [plaidCurrentEvent] = useOnyx(ONYXKEYS.PLAID_CURRENT_EVENT); + + const policyName = policy?.name ?? ''; + const policyIDParam = route.params?.policyID ?? '-1'; + const styles = useThemeStyles(); + const {translate} = useLocalize(); + const {isOffline} = useNetwork(); + const requestorStepRef = useRef(null); + const prevIsReimbursementAccountLoading = usePrevious(reimbursementAccount?.isLoading); + const prevReimbursementAccount = usePrevious(reimbursementAccount); + const prevIsOffline = usePrevious(isOffline); + /** The SetupWithdrawalAccount flow allows us to continue the flow from various points depending on where the user left off. This view will refer to the achData as the single source of truth to determine which route to @@ -143,7 +154,19 @@ function ReimbursementAccountPage({route, policy}: ReimbursementAccountPageProps next step. */ const achData = reimbursementAccount?.achData; - const isPreviousPolicy = policy?.id === achData?.policyID; + const isPreviousPolicy = policyIDParam === achData?.policyID; + // eslint-disable-next-line @typescript-eslint/prefer-nullish-coalescing + const currentStep = !isPreviousPolicy ? CONST.BANK_ACCOUNT.STEP.BANK_ACCOUNT : achData?.currentStep || CONST.BANK_ACCOUNT.STEP.BANK_ACCOUNT; + + /** + When this page is first opened, `reimbursementAccount` prop might not yet be fully loaded from Onyx. + Calculating `shouldShowContinueSetupButton` immediately on initial render doesn't make sense as + it relies on incomplete data. Thus, we should wait to calculate it until we have received + the full `reimbursementAccount` data from the server. This logic is handled within the useEffect hook, + which acts similarly to `componentDidUpdate` when the `reimbursementAccount` dependency changes. + */ + const [hasACHDataBeenLoaded, setHasACHDataBeenLoaded] = useState(reimbursementAccount !== CONST.REIMBURSEMENT_ACCOUNT.DEFAULT_DATA && isPreviousPolicy); + const [shouldShowContinueSetupButton, setShouldShowContinueSetupButton] = useState(getShouldShowContinueSetupButtonInitialValue()); function getBankAccountFields(fieldNames: T[]): Pick { return { @@ -170,28 +193,6 @@ function ReimbursementAccountPage({route, policy}: ReimbursementAccountPageProps return achData?.state === BankAccount.STATE.PENDING || [CONST.BANK_ACCOUNT.STEP.BANK_ACCOUNT, ''].includes(getStepToOpenFromRouteParams(route)); } - /** - When this page is first opened, `reimbursementAccount` prop might not yet be fully loaded from Onyx. - Calculating `shouldShowContinueSetupButton` immediately on initial render doesn't make sense as - it relies on incomplete data. Thus, we should wait to calculate it until we have received - the full `reimbursementAccount` data from the server. This logic is handled within the useEffect hook, - which acts similarly to `componentDidUpdate` when the `reimbursementAccount` dependency changes. - */ - const [hasACHDataBeenLoaded, setHasACHDataBeenLoaded] = useState(reimbursementAccount !== CONST.REIMBURSEMENT_ACCOUNT.DEFAULT_DATA && isPreviousPolicy); - const [shouldShowContinueSetupButton, setShouldShowContinueSetupButton] = useState(getShouldShowContinueSetupButtonInitialValue()); - - // eslint-disable-next-line @typescript-eslint/prefer-nullish-coalescing - const currentStep = !isPreviousPolicy ? CONST.BANK_ACCOUNT.STEP.BANK_ACCOUNT : achData?.currentStep || CONST.BANK_ACCOUNT.STEP.BANK_ACCOUNT; - const policyName = policy?.name ?? ''; - const policyIDParam = route.params?.policyID ?? '-1'; - const styles = useThemeStyles(); - const {translate} = useLocalize(); - const {isOffline} = useNetwork(); - const requestorStepRef = useRef(null); - const prevIsReimbursementAccountLoading = usePrevious(reimbursementAccount?.isLoading); - const prevReimbursementAccount = usePrevious(reimbursementAccount); - const prevIsOffline = usePrevious(isOffline); - /** * Retrieve verified business bank account currently being set up. * @param ignoreLocalCurrentStep Pass true if you want the last "updated" view (from db), not the last "viewed" view (from onyx). From 90b145402776ea0d0bd393000e5eda7cbaa07c5d Mon Sep 17 00:00:00 2001 From: Agata Kosior Date: Thu, 26 Sep 2024 19:20:25 +0700 Subject: [PATCH 05/12] fix: final fixes --- .../BankInfo/substeps/Manual.tsx | 4 ++++ .../ReimbursementAccountPage.tsx | 16 ++++++---------- src/types/onyx/ReimbursementAccount.ts | 3 +++ 3 files changed, 13 insertions(+), 10 deletions(-) diff --git a/src/pages/ReimbursementAccount/BankInfo/substeps/Manual.tsx b/src/pages/ReimbursementAccount/BankInfo/substeps/Manual.tsx index 647eb924f437..38aeb8967ab6 100644 --- a/src/pages/ReimbursementAccount/BankInfo/substeps/Manual.tsx +++ b/src/pages/ReimbursementAccount/BankInfo/substeps/Manual.tsx @@ -33,6 +33,8 @@ function Manual({onNext}: ManualProps) { [BANK_INFO_STEP_KEYS.ACCOUNT_NUMBER]: reimbursementAccount?.achData?.[BANK_INFO_STEP_KEYS.ACCOUNT_NUMBER] ?? '', }; + const shouldBeInputsReadonly = reimbursementAccount?.achData?.setupType === CONST.BANK_ACCOUNT.SETUP_TYPE.PLAID; + const validate = useCallback( (values: FormOnyxValues): FormInputErrors => { const errors = ValidationUtils.getFieldRequiredErrors(values, STEP_FIELDS); @@ -86,6 +88,7 @@ function Manual({onNext}: ManualProps) { inputMode={CONST.INPUT_MODE.NUMERIC} shouldSaveDraft shouldUseDefaultValue={hasBankAccountData} + disabled={shouldBeInputsReadonly} /> ); diff --git a/src/pages/ReimbursementAccount/ReimbursementAccountPage.tsx b/src/pages/ReimbursementAccount/ReimbursementAccountPage.tsx index aac63db6a29a..7a9cc2c916fb 100644 --- a/src/pages/ReimbursementAccount/ReimbursementAccountPage.tsx +++ b/src/pages/ReimbursementAccount/ReimbursementAccountPage.tsx @@ -178,7 +178,7 @@ function ReimbursementAccountPage({route, policy}: ReimbursementAccountPageProps * Returns true if a VBBA exists in any state other than OPEN or LOCKED */ function hasInProgressVBBA(): boolean { - return isPreviousPolicy && !!achData?.bankAccountID && achData?.state !== BankAccount.STATE.OPEN && achData?.state !== BankAccount.STATE.LOCKED; + return !!achData?.state && achData?.state !== BankAccount.STATE.OPEN && achData?.state !== BankAccount.STATE.LOCKED; } /* @@ -195,16 +195,14 @@ function ReimbursementAccountPage({route, policy}: ReimbursementAccountPageProps /** * Retrieve verified business bank account currently being set up. - * @param ignoreLocalCurrentStep Pass true if you want the last "updated" view (from db), not the last "viewed" view (from onyx). - * @param ignoreLocalSubStep Pass true if you want the last "updated" view (from db), not the last "viewed" view (from onyx). */ - function fetchData(ignoreLocalCurrentStep?: boolean, ignoreLocalSubStep?: boolean) { + function fetchData() { // We can specify a step to navigate to by using route params when the component mounts. // We want to use the same stepToOpen variable when the network state changes because we can be redirected to a different step when the account refreshes. const stepToOpen = getStepToOpenFromRouteParams(route); - const subStep = achData?.subStep ?? ''; - const localCurrentStep = achData?.currentStep ?? ''; - BankAccounts.openReimbursementAccountPage(stepToOpen, ignoreLocalSubStep ? '' : subStep, ignoreLocalCurrentStep ? '' : localCurrentStep, policyIDParam); + const subStep = isPreviousPolicy ? achData?.subStep ?? '' : ''; + const localCurrentStep = isPreviousPolicy ? achData?.currentStep ?? '' : ''; + BankAccounts.openReimbursementAccountPage(stepToOpen, subStep, localCurrentStep, policyIDParam); } useEffect(() => { @@ -221,7 +219,7 @@ function ReimbursementAccountPage({route, policy}: ReimbursementAccountPageProps BankAccounts.setBankAccountSubStep(null); BankAccounts.setPlaidEvent(null); } - fetchData(true, isStepToOpenEmpty); + fetchData(); // eslint-disable-next-line react-compiler/react-compiler, react-hooks/exhaustive-deps }, []); // The empty dependency array ensures this runs only once after the component mounts. @@ -241,7 +239,6 @@ function ReimbursementAccountPage({route, policy}: ReimbursementAccountPageProps if (!hasACHDataBeenLoaded) { if (reimbursementAccount !== CONST.REIMBURSEMENT_ACCOUNT.DEFAULT_DATA && reimbursementAccount?.isLoading === false) { - setShouldShowContinueSetupButton(getShouldShowContinueSetupButtonInitialValue()); setHasACHDataBeenLoaded(true); } return; @@ -295,7 +292,6 @@ function ReimbursementAccountPage({route, policy}: ReimbursementAccountPageProps BankAccounts.setBankAccountSubStep(CONST.BANK_ACCOUNT.SETUP_TYPE.MANUAL).then(() => { setShouldShowContinueSetupButton(false); }); - fetchData(true); }; const goBack = () => { diff --git a/src/types/onyx/ReimbursementAccount.ts b/src/types/onyx/ReimbursementAccount.ts index 186b3e6ad05b..0d5c8a83b99b 100644 --- a/src/types/onyx/ReimbursementAccount.ts +++ b/src/types/onyx/ReimbursementAccount.ts @@ -47,6 +47,9 @@ type ACHData = Partial Date: Fri, 27 Sep 2024 14:40:21 +0700 Subject: [PATCH 06/12] fix: apply suggestions from code review Co-authored-by: Vit Horacek <36083550+mountiny@users.noreply.github.com> Co-authored-by: DylanDylann <141406735+DylanDylann@users.noreply.github.com> --- src/pages/ReimbursementAccount/BankInfo/substeps/Manual.tsx | 6 +++--- src/pages/ReimbursementAccount/ReimbursementAccountPage.tsx | 2 +- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/src/pages/ReimbursementAccount/BankInfo/substeps/Manual.tsx b/src/pages/ReimbursementAccount/BankInfo/substeps/Manual.tsx index 38aeb8967ab6..1fe21fd0963a 100644 --- a/src/pages/ReimbursementAccount/BankInfo/substeps/Manual.tsx +++ b/src/pages/ReimbursementAccount/BankInfo/substeps/Manual.tsx @@ -33,7 +33,7 @@ function Manual({onNext}: ManualProps) { [BANK_INFO_STEP_KEYS.ACCOUNT_NUMBER]: reimbursementAccount?.achData?.[BANK_INFO_STEP_KEYS.ACCOUNT_NUMBER] ?? '', }; - const shouldBeInputsReadonly = reimbursementAccount?.achData?.setupType === CONST.BANK_ACCOUNT.SETUP_TYPE.PLAID; + const shouldBeReadonlyInput = reimbursementAccount?.achData?.setupType === CONST.BANK_ACCOUNT.SETUP_TYPE.PLAID; const validate = useCallback( (values: FormOnyxValues): FormInputErrors => { @@ -88,7 +88,7 @@ function Manual({onNext}: ManualProps) { inputMode={CONST.INPUT_MODE.NUMERIC} shouldSaveDraft shouldUseDefaultValue={hasBankAccountData} - disabled={shouldBeInputsReadonly} + disabled={shouldBeReadonlyInput} /> ); diff --git a/src/pages/ReimbursementAccount/ReimbursementAccountPage.tsx b/src/pages/ReimbursementAccount/ReimbursementAccountPage.tsx index 7a9cc2c916fb..c29779fa1f44 100644 --- a/src/pages/ReimbursementAccount/ReimbursementAccountPage.tsx +++ b/src/pages/ReimbursementAccount/ReimbursementAccountPage.tsx @@ -156,7 +156,7 @@ function ReimbursementAccountPage({route, policy}: ReimbursementAccountPageProps const achData = reimbursementAccount?.achData; const isPreviousPolicy = policyIDParam === achData?.policyID; // eslint-disable-next-line @typescript-eslint/prefer-nullish-coalescing - const currentStep = !isPreviousPolicy ? CONST.BANK_ACCOUNT.STEP.BANK_ACCOUNT : achData?.currentStep || CONST.BANK_ACCOUNT.STEP.BANK_ACCOUNT; + const currentStep = !isPreviousPolicy ? CONST.BANK_ACCOUNT.STEP.BANK_ACCOUNT : (achData?.currentStep || CONST.BANK_ACCOUNT.STEP.BANK_ACCOUNT); /** When this page is first opened, `reimbursementAccount` prop might not yet be fully loaded from Onyx. From 29623ee69961b6c8249ef490071b845959424cc9 Mon Sep 17 00:00:00 2001 From: Agata Kosior Date: Fri, 27 Sep 2024 14:42:06 +0700 Subject: [PATCH 07/12] fix: add policyIDParam to the dependency array --- src/pages/ReimbursementAccount/ReimbursementAccountPage.tsx | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/pages/ReimbursementAccount/ReimbursementAccountPage.tsx b/src/pages/ReimbursementAccount/ReimbursementAccountPage.tsx index c29779fa1f44..1e0b9758cd15 100644 --- a/src/pages/ReimbursementAccount/ReimbursementAccountPage.tsx +++ b/src/pages/ReimbursementAccount/ReimbursementAccountPage.tsx @@ -285,7 +285,7 @@ function ReimbursementAccountPage({route, policy}: ReimbursementAccountPageProps Navigation.navigate(ROUTES.BANK_ACCOUNT_WITH_STEP_TO_OPEN.getRoute(getRouteForCurrentStep(currentStep), policyIDParam, backTo)); }, // eslint-disable-next-line react-compiler/react-compiler, react-hooks/exhaustive-deps - [isOffline, reimbursementAccount, route, hasACHDataBeenLoaded, shouldShowContinueSetupButton], + [isOffline, reimbursementAccount, route, hasACHDataBeenLoaded, shouldShowContinueSetupButton, policyIDParam], ); const setManualStep = () => { From 67d7d9990726ca6102b843731f6f04e36b726ebe Mon Sep 17 00:00:00 2001 From: Agata Kosior Date: Fri, 27 Sep 2024 15:25:17 +0700 Subject: [PATCH 08/12] fix: prettier --- src/pages/ReimbursementAccount/ReimbursementAccountPage.tsx | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/pages/ReimbursementAccount/ReimbursementAccountPage.tsx b/src/pages/ReimbursementAccount/ReimbursementAccountPage.tsx index 1e0b9758cd15..8bd7498e2692 100644 --- a/src/pages/ReimbursementAccount/ReimbursementAccountPage.tsx +++ b/src/pages/ReimbursementAccount/ReimbursementAccountPage.tsx @@ -156,7 +156,7 @@ function ReimbursementAccountPage({route, policy}: ReimbursementAccountPageProps const achData = reimbursementAccount?.achData; const isPreviousPolicy = policyIDParam === achData?.policyID; // eslint-disable-next-line @typescript-eslint/prefer-nullish-coalescing - const currentStep = !isPreviousPolicy ? CONST.BANK_ACCOUNT.STEP.BANK_ACCOUNT : (achData?.currentStep || CONST.BANK_ACCOUNT.STEP.BANK_ACCOUNT); + const currentStep = !isPreviousPolicy ? CONST.BANK_ACCOUNT.STEP.BANK_ACCOUNT : achData?.currentStep || CONST.BANK_ACCOUNT.STEP.BANK_ACCOUNT; /** When this page is first opened, `reimbursementAccount` prop might not yet be fully loaded from Onyx. From d4ca88863f1115fecfa62242fa6784023aa5ba6b Mon Sep 17 00:00:00 2001 From: Agata Kosior Date: Tue, 1 Oct 2024 15:09:21 +0700 Subject: [PATCH 09/12] fix: revert removing check --- src/pages/ReimbursementAccount/ReimbursementAccountPage.tsx | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/pages/ReimbursementAccount/ReimbursementAccountPage.tsx b/src/pages/ReimbursementAccount/ReimbursementAccountPage.tsx index 523d703dbcc0..8bf101e75825 100644 --- a/src/pages/ReimbursementAccount/ReimbursementAccountPage.tsx +++ b/src/pages/ReimbursementAccount/ReimbursementAccountPage.tsx @@ -177,7 +177,7 @@ function ReimbursementAccountPage({route, policy}: ReimbursementAccountPageProps * Returns true if a VBBA exists in any state other than OPEN or LOCKED */ function hasInProgressVBBA(): boolean { - return !!achData?.state && achData?.state !== BankAccount.STATE.OPEN && achData?.state !== BankAccount.STATE.LOCKED; + return !!achData?.bankAccountID && !!achData?.state && achData?.state !== BankAccount.STATE.OPEN && achData?.state !== BankAccount.STATE.LOCKED; } /* From e461db167ea5dda972ee279b2b2ef3a8e6f82008 Mon Sep 17 00:00:00 2001 From: Agata Kosior Date: Wed, 2 Oct 2024 17:21:30 +0700 Subject: [PATCH 10/12] fix: remove unnecessary dep --- src/pages/ReimbursementAccount/ReimbursementAccountPage.tsx | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/pages/ReimbursementAccount/ReimbursementAccountPage.tsx b/src/pages/ReimbursementAccount/ReimbursementAccountPage.tsx index 8bf101e75825..011e89c4d539 100644 --- a/src/pages/ReimbursementAccount/ReimbursementAccountPage.tsx +++ b/src/pages/ReimbursementAccount/ReimbursementAccountPage.tsx @@ -282,7 +282,7 @@ function ReimbursementAccountPage({route, policy}: ReimbursementAccountPageProps Navigation.setParams({stepToOpen: getRouteForCurrentStep(currentStep)}); }, // eslint-disable-next-line react-compiler/react-compiler, react-hooks/exhaustive-deps - [isOffline, reimbursementAccount, route, hasACHDataBeenLoaded, shouldShowContinueSetupButton, policyIDParam], + [isOffline, reimbursementAccount, route, hasACHDataBeenLoaded, shouldShowContinueSetupButton], ); const setManualStep = () => { From e56d462a7cf9ed1055cfc9b63ee08a5243dc3261 Mon Sep 17 00:00:00 2001 From: Agata Kosior Date: Thu, 3 Oct 2024 18:47:47 +0700 Subject: [PATCH 11/12] fix: remove unnecessary useEffect --- .../ReimbursementAccount/ReimbursementAccountPage.tsx | 8 -------- 1 file changed, 8 deletions(-) diff --git a/src/pages/ReimbursementAccount/ReimbursementAccountPage.tsx b/src/pages/ReimbursementAccount/ReimbursementAccountPage.tsx index 011e89c4d539..11820cd2c471 100644 --- a/src/pages/ReimbursementAccount/ReimbursementAccountPage.tsx +++ b/src/pages/ReimbursementAccount/ReimbursementAccountPage.tsx @@ -141,7 +141,6 @@ function ReimbursementAccountPage({route, policy}: ReimbursementAccountPageProps const {translate} = useLocalize(); const {isOffline} = useNetwork(); const requestorStepRef = useRef(null); - const prevIsReimbursementAccountLoading = usePrevious(reimbursementAccount?.isLoading); const prevReimbursementAccount = usePrevious(reimbursementAccount); const prevIsOffline = usePrevious(isOffline); @@ -222,13 +221,6 @@ function ReimbursementAccountPage({route, policy}: ReimbursementAccountPageProps // eslint-disable-next-line react-compiler/react-compiler, react-hooks/exhaustive-deps }, []); // The empty dependency array ensures this runs only once after the component mounts. - useEffect(() => { - if (typeof reimbursementAccount?.isLoading !== 'boolean' || reimbursementAccount.isLoading === prevIsReimbursementAccountLoading) { - return; - } - setHasACHDataBeenLoaded(true); - }, [prevIsReimbursementAccountLoading, reimbursementAccount?.isLoading]); - useEffect( () => { // Check for network change from offline to online From dfd1ab5c193ca14ba05f6b52d0162b96ba273bd0 Mon Sep 17 00:00:00 2001 From: Agata Kosior Date: Mon, 7 Oct 2024 10:07:42 +0700 Subject: [PATCH 12/12] fix: apply suggestion --- src/pages/ReimbursementAccount/BankInfo/substeps/Manual.tsx | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/pages/ReimbursementAccount/BankInfo/substeps/Manual.tsx b/src/pages/ReimbursementAccount/BankInfo/substeps/Manual.tsx index 1fe21fd0963a..d39f5e0ba096 100644 --- a/src/pages/ReimbursementAccount/BankInfo/substeps/Manual.tsx +++ b/src/pages/ReimbursementAccount/BankInfo/substeps/Manual.tsx @@ -33,7 +33,7 @@ function Manual({onNext}: ManualProps) { [BANK_INFO_STEP_KEYS.ACCOUNT_NUMBER]: reimbursementAccount?.achData?.[BANK_INFO_STEP_KEYS.ACCOUNT_NUMBER] ?? '', }; - const shouldBeReadonlyInput = reimbursementAccount?.achData?.setupType === CONST.BANK_ACCOUNT.SETUP_TYPE.PLAID; + const shouldBeReadOnlyInput = reimbursementAccount?.achData?.setupType === CONST.BANK_ACCOUNT.SETUP_TYPE.PLAID; const validate = useCallback( (values: FormOnyxValues): FormInputErrors => { @@ -88,7 +88,7 @@ function Manual({onNext}: ManualProps) { inputMode={CONST.INPUT_MODE.NUMERIC} shouldSaveDraft shouldUseDefaultValue={hasBankAccountData} - disabled={shouldBeReadonlyInput} + disabled={shouldBeReadOnlyInput} /> );