From aca6c23e429211ef949dc2b015d4e80b21705af7 Mon Sep 17 00:00:00 2001 From: "alberto@expensify.com" Date: Tue, 12 Oct 2021 17:06:04 +0200 Subject: [PATCH 1/5] Do not always clear Onyx VBA data when accessing workspace sections --- src/libs/actions/BankAccounts.js | 19 ++++++++++++++++--- .../workspace/WorkspacePageWithSections.js | 4 +++- 2 files changed, 19 insertions(+), 4 deletions(-) diff --git a/src/libs/actions/BankAccounts.js b/src/libs/actions/BankAccounts.js index c9191083f0ce..7a47c2369bce 100644 --- a/src/libs/actions/BankAccounts.js +++ b/src/libs/actions/BankAccounts.js @@ -335,14 +335,20 @@ function fetchUserWallet() { * Fetch the bank account currently being set up by the user for the free plan if it exists. * * @param {String} [stepToOpen] + * @param {Boolean} [hasLocalOpenAccount] */ -function fetchFreePlanVerifiedBankAccount(stepToOpen) { +function fetchFreePlanVerifiedBankAccount(stepToOpen, hasLocalOpenAccount = false) { // Remember which account BankAccountStep subStep the user had before so we can set it later const subStep = lodashGet(reimbursementAccountInSetup, 'subStep', ''); // We are using set here since we will rely on data from the server (not local data) to populate the VBA flow - // and determine which step to navigate to. - Onyx.set(ONYXKEYS.REIMBURSEMENT_ACCOUNT, {loading: true, error: ''}); + // and determine which step to navigate to, unless there is an already open local account, in which case we'll only. + // override the data in ONYX if there isn't one on the server side. + if (hasLocalOpenAccount) { + Onyx.merge(ONYXKEYS.REIMBURSEMENT_ACCOUNT, {loading: true, error: ''}); + } else { + Onyx.set(ONYXKEYS.REIMBURSEMENT_ACCOUNT, {loading: true, error: ''}); + } let bankAccountID; API.Get({ @@ -383,6 +389,13 @@ function fetchFreePlanVerifiedBankAccount(stepToOpen) { // If the user is already setting up a bank account we will continue the flow for them let currentStep = reimbursementAccountInSetup.currentStep; const achData = bankAccount ? bankAccount.toACHData() : {}; + + // If we passed the flag indicating there was an already open account in local data, and there isn't + // one on the server side, let's make sure we use just the server-side data. + if (hasLocalOpenAccount && lodashGet(achData, 'state', '') !== BankAccount.STATE.OPEN) { + Onyx.set(ONYXKEYS.REIMBURSEMENT_ACCOUNT, {loading: false, error: ''}); + reimbursementAccountInSetup = {}; + } if (!stepToOpen && achData.currentStep) { // eslint-disable-next-line no-use-before-define currentStep = getNextStepToComplete(achData); diff --git a/src/pages/workspace/WorkspacePageWithSections.js b/src/pages/workspace/WorkspacePageWithSections.js index 6179eb8b6c65..3ae7e41c7472 100644 --- a/src/pages/workspace/WorkspacePageWithSections.js +++ b/src/pages/workspace/WorkspacePageWithSections.js @@ -46,7 +46,9 @@ const defaultProps = { class WorkspacePageWithSections extends React.Component { componentDidMount() { - fetchFreePlanVerifiedBankAccount(); + const achState = lodashGet(this.props.reimbursementAccount, 'achData.state', ''); + const hasVBA = achState === BankAccount.STATE.OPEN; + fetchFreePlanVerifiedBankAccount('', hasVBA); } render() { From 07001fa08494ec4f7168fe06c3d94311e3d10281 Mon Sep 17 00:00:00 2001 From: "alberto@expensify.com" Date: Tue, 12 Oct 2021 18:15:57 +0200 Subject: [PATCH 2/5] Simplify solution --- src/libs/actions/BankAccounts.js | 18 ++---------------- .../workspace/WorkspacePageWithSections.js | 4 +--- 2 files changed, 3 insertions(+), 19 deletions(-) diff --git a/src/libs/actions/BankAccounts.js b/src/libs/actions/BankAccounts.js index 7a47c2369bce..c703f9255ea6 100644 --- a/src/libs/actions/BankAccounts.js +++ b/src/libs/actions/BankAccounts.js @@ -335,20 +335,12 @@ function fetchUserWallet() { * Fetch the bank account currently being set up by the user for the free plan if it exists. * * @param {String} [stepToOpen] - * @param {Boolean} [hasLocalOpenAccount] */ -function fetchFreePlanVerifiedBankAccount(stepToOpen, hasLocalOpenAccount = false) { +function fetchFreePlanVerifiedBankAccount(stepToOpen) { // Remember which account BankAccountStep subStep the user had before so we can set it later const subStep = lodashGet(reimbursementAccountInSetup, 'subStep', ''); - // We are using set here since we will rely on data from the server (not local data) to populate the VBA flow - // and determine which step to navigate to, unless there is an already open local account, in which case we'll only. - // override the data in ONYX if there isn't one on the server side. - if (hasLocalOpenAccount) { - Onyx.merge(ONYXKEYS.REIMBURSEMENT_ACCOUNT, {loading: true, error: ''}); - } else { - Onyx.set(ONYXKEYS.REIMBURSEMENT_ACCOUNT, {loading: true, error: ''}); - } + Onyx.merge(ONYXKEYS.REIMBURSEMENT_ACCOUNT, {loading: true, error: ''}); let bankAccountID; API.Get({ @@ -390,12 +382,6 @@ function fetchFreePlanVerifiedBankAccount(stepToOpen, hasLocalOpenAccount = fals let currentStep = reimbursementAccountInSetup.currentStep; const achData = bankAccount ? bankAccount.toACHData() : {}; - // If we passed the flag indicating there was an already open account in local data, and there isn't - // one on the server side, let's make sure we use just the server-side data. - if (hasLocalOpenAccount && lodashGet(achData, 'state', '') !== BankAccount.STATE.OPEN) { - Onyx.set(ONYXKEYS.REIMBURSEMENT_ACCOUNT, {loading: false, error: ''}); - reimbursementAccountInSetup = {}; - } if (!stepToOpen && achData.currentStep) { // eslint-disable-next-line no-use-before-define currentStep = getNextStepToComplete(achData); diff --git a/src/pages/workspace/WorkspacePageWithSections.js b/src/pages/workspace/WorkspacePageWithSections.js index 3ae7e41c7472..6179eb8b6c65 100644 --- a/src/pages/workspace/WorkspacePageWithSections.js +++ b/src/pages/workspace/WorkspacePageWithSections.js @@ -46,9 +46,7 @@ const defaultProps = { class WorkspacePageWithSections extends React.Component { componentDidMount() { - const achState = lodashGet(this.props.reimbursementAccount, 'achData.state', ''); - const hasVBA = achState === BankAccount.STATE.OPEN; - fetchFreePlanVerifiedBankAccount('', hasVBA); + fetchFreePlanVerifiedBankAccount(); } render() { From 48345e76032fed1ba21d1249fd2cbe9abd863dbe Mon Sep 17 00:00:00 2001 From: "alberto@expensify.com" Date: Tue, 12 Oct 2021 18:17:24 +0200 Subject: [PATCH 3/5] remove extra line --- src/libs/actions/BankAccounts.js | 1 - 1 file changed, 1 deletion(-) diff --git a/src/libs/actions/BankAccounts.js b/src/libs/actions/BankAccounts.js index c703f9255ea6..bb4ce14a59bb 100644 --- a/src/libs/actions/BankAccounts.js +++ b/src/libs/actions/BankAccounts.js @@ -381,7 +381,6 @@ function fetchFreePlanVerifiedBankAccount(stepToOpen) { // If the user is already setting up a bank account we will continue the flow for them let currentStep = reimbursementAccountInSetup.currentStep; const achData = bankAccount ? bankAccount.toACHData() : {}; - if (!stepToOpen && achData.currentStep) { // eslint-disable-next-line no-use-before-define currentStep = getNextStepToComplete(achData); From fee575ee418de3829dd7fd8b94a19f43dd37ce66 Mon Sep 17 00:00:00 2001 From: "alberto@expensify.com" Date: Wed, 13 Oct 2021 16:26:03 +0200 Subject: [PATCH 4/5] Keep state in onyx --- src/libs/actions/BankAccounts.js | 11 +++++++++-- src/pages/workspace/WorkspacePageWithSections.js | 4 +++- 2 files changed, 12 insertions(+), 3 deletions(-) diff --git a/src/libs/actions/BankAccounts.js b/src/libs/actions/BankAccounts.js index bb4ce14a59bb..a72a8e755291 100644 --- a/src/libs/actions/BankAccounts.js +++ b/src/libs/actions/BankAccounts.js @@ -335,12 +335,19 @@ function fetchUserWallet() { * Fetch the bank account currently being set up by the user for the free plan if it exists. * * @param {String} [stepToOpen] + * @param {Boolean} [hasLocalOpenVBA] */ -function fetchFreePlanVerifiedBankAccount(stepToOpen) { +function fetchFreePlanVerifiedBankAccount(stepToOpen, hasLocalOpenVBA = false) { // Remember which account BankAccountStep subStep the user had before so we can set it later const subStep = lodashGet(reimbursementAccountInSetup, 'subStep', ''); + const initialData = {loading: true, error: ''}; + if (hasLocalOpenVBA) { + initialData.achData = {state: BankAccount.STATE.OPEN}; + } - Onyx.merge(ONYXKEYS.REIMBURSEMENT_ACCOUNT, {loading: true, error: ''}); + // We are using set here since we will rely on data from the server (not local data) to populate the VBA flow + // and determine which step to navigate to. + Onyx.set(ONYXKEYS.REIMBURSEMENT_ACCOUNT, initialData); let bankAccountID; API.Get({ diff --git a/src/pages/workspace/WorkspacePageWithSections.js b/src/pages/workspace/WorkspacePageWithSections.js index 6179eb8b6c65..3ae7e41c7472 100644 --- a/src/pages/workspace/WorkspacePageWithSections.js +++ b/src/pages/workspace/WorkspacePageWithSections.js @@ -46,7 +46,9 @@ const defaultProps = { class WorkspacePageWithSections extends React.Component { componentDidMount() { - fetchFreePlanVerifiedBankAccount(); + const achState = lodashGet(this.props.reimbursementAccount, 'achData.state', ''); + const hasVBA = achState === BankAccount.STATE.OPEN; + fetchFreePlanVerifiedBankAccount('', hasVBA); } render() { From ab5acf460511f97a9bf2daf6e0eabe60cfd5fe8d Mon Sep 17 00:00:00 2001 From: "alberto@expensify.com" Date: Wed, 13 Oct 2021 18:36:38 +0200 Subject: [PATCH 5/5] Pass state instead --- src/libs/actions/BankAccounts.js | 10 ++++++---- src/pages/workspace/WorkspacePageWithSections.js | 3 +-- 2 files changed, 7 insertions(+), 6 deletions(-) diff --git a/src/libs/actions/BankAccounts.js b/src/libs/actions/BankAccounts.js index a72a8e755291..f2697261a6ff 100644 --- a/src/libs/actions/BankAccounts.js +++ b/src/libs/actions/BankAccounts.js @@ -335,14 +335,16 @@ function fetchUserWallet() { * Fetch the bank account currently being set up by the user for the free plan if it exists. * * @param {String} [stepToOpen] - * @param {Boolean} [hasLocalOpenVBA] + * @param {String} [localBankAccountState] */ -function fetchFreePlanVerifiedBankAccount(stepToOpen, hasLocalOpenVBA = false) { +function fetchFreePlanVerifiedBankAccount(stepToOpen, localBankAccountState) { // Remember which account BankAccountStep subStep the user had before so we can set it later const subStep = lodashGet(reimbursementAccountInSetup, 'subStep', ''); const initialData = {loading: true, error: ''}; - if (hasLocalOpenVBA) { - initialData.achData = {state: BankAccount.STATE.OPEN}; + + // Some UI needs to know the bank account state during the loading process, so we are keeping it in Onyx if passed + if (localBankAccountState) { + initialData.achData = {state: localBankAccountState}; } // We are using set here since we will rely on data from the server (not local data) to populate the VBA flow diff --git a/src/pages/workspace/WorkspacePageWithSections.js b/src/pages/workspace/WorkspacePageWithSections.js index 3ae7e41c7472..07ff09ff8b05 100644 --- a/src/pages/workspace/WorkspacePageWithSections.js +++ b/src/pages/workspace/WorkspacePageWithSections.js @@ -47,8 +47,7 @@ const defaultProps = { class WorkspacePageWithSections extends React.Component { componentDidMount() { const achState = lodashGet(this.props.reimbursementAccount, 'achData.state', ''); - const hasVBA = achState === BankAccount.STATE.OPEN; - fetchFreePlanVerifiedBankAccount('', hasVBA); + fetchFreePlanVerifiedBankAccount('', achState); } render() {