Skip to content

[HOLD BE] add optimistic data for addPersonalBankAccount #58973

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
mkzie2 wants to merge 3 commits into from
+30 −0
Closed

[HOLD BE] add optimistic data for addPersonalBankAccount #58973

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
0cc7f26
add opitmistic data for addPersonalBankAccount
mkzie2 Mar 24, 2025
0409a87
convert data to achAccount data
mkzie2 Mar 24, 2025
9b3c85d
Merge branch 'main' of github.com-mkzie2:mkzie2/App into mkzie2-issue…
mkzie2 Apr 28, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
25 changes: 25 additions & 0 deletions src/libs/actions/BankAccounts.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
import Onyx from 'react-native-onyx';
import type {OnyxEntry} from 'react-native-onyx';
import type {FormInputErrors, FormOnyxValues} from '@components/Form/types';
import type {OnfidoDataWithApplicantID} from '@components/Onfido/types';
import * as API from '@libs/API';
Expand All @@ -19,6 +20,7 @@ import {READ_COMMANDS, SIDE_EFFECT_REQUEST_COMMANDS, WRITE_COMMANDS} from '@libs
import {getMicroSecondOnyxErrorWithTranslationKey} from '@libs/ErrorUtils';
import {translateLocal} from '@libs/Localize';
import Navigation from '@libs/Navigation/Navigation';
import {getPolicy} from '@libs/PolicyUtils';
import CONST from '@src/CONST';
import type {Country} from '@src/CONST';
import ONYXKEYS from '@src/ONYXKEYS';
Expand All @@ -28,8 +30,10 @@ import type {InternationalBankAccountForm, PersonalBankAccountForm} from '@src/t
import type {ACHContractStepProps, BeneficialOwnersStepProps, CompanyStepProps, ReimbursementAccountForm, RequestorStepProps} from '@src/types/form/ReimbursementAccountForm';
import type PlaidBankAccount from '@src/types/onyx/PlaidBankAccount';
import type {BankAccountStep, ReimbursementAccountStep, ReimbursementAccountSubStep} from '@src/types/onyx/ReimbursementAccount';
import type ReimbursementAccount from '@src/types/onyx/ReimbursementAccount';
import type {OnyxData} from '@src/types/onyx/Request';
import {setBankAccountSubStep} from './ReimbursementAccount';
import {getCurrentUserEmail} from './Report';

export {
goToWithdrawalAccountSetupStep,
Expand Down Expand Up @@ -357,6 +361,26 @@ function validateBankAccount(bankAccountID: number, validateCode: string, policy
API.write(WRITE_COMMANDS.VALIDATE_BANK_ACCOUNT_WITH_TRANSACTIONS, parameters, onyxData);
}

function convertReimbursementAccountDataToAchAccountData(reimbursementAccount: OnyxEntry<ReimbursementAccount>) {
Copy link
Contributor Author

@mkzie2 mkzie2 Mar 24, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@allgandalf Instead of adding optimistic data we need to convert the achData from reimbursementAccount to the policy data after the bank account is verified completely.

Copy link
Contributor

@allgandalf allgandalf Mar 27, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

why so? whats the problem with adding optimistic data?

Copy link
Contributor

@allgandalf allgandalf Mar 31, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

update please

Copy link
Contributor Author

@mkzie2 mkzie2 Mar 31, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@allgandalf addPersonalBankAccount API is only the first step in the bankAccount flow. We should only add the policy.achAccount after the bank account flow is complete.

Copy link
Contributor

@allgandalf allgandalf Mar 31, 2025 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@mkzie2 please use the format:

  • My original solution was:
  • But we couldn't implement it due to reason(s):
  • Now the new solution used is better that the previous one because:

Please try to be detailed!

Copy link
Contributor

@allgandalf allgandalf Apr 24, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

bump @mkzie2

Copy link
Contributor Author

@mkzie2 mkzie2 Apr 24, 2025 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@stitesExpensify It's the Onyx update. Currently it only returns bankAccountList and reimbursementAccount.

Screenshot 2025-04-25 at 00 15 05

We need to return the policy.achAccount too.

Copy link
Contributor

@allgandalf allgandalf Apr 29, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

bump @stitesExpensify

Copy link
Contributor

@stitesExpensify stitesExpensify Apr 30, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Okay I will update the backend but it's going to be a while FYI. I've been dealing with fires for the last few weeks and am about to go OOO

Copy link
Contributor

@allgandalf allgandalf May 1, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

no worries, @mkzie2 please put this PR on hold/ close it if we no longer need this

const achData = reimbursementAccount?.achData;
const policy = getPolicy(achData?.policyID);
if (!achData || policy?.achAccount?.bankAccountID) {
return;
}

Onyx.merge(`${ONYXKEYS.COLLECTION.POLICY}${achData?.policyID}`, {
achAccount: {
bankAccountID: achData?.bankAccountID,
bankName: achData?.bankName,
reimburser: policy?.achAccount?.reimburser ?? getCurrentUserEmail(),
accountNumber: achData.accountNumber,
routingNumber: achData.routingNumber,
addressName: achData.addressName,
},
reimbursementChoice: CONST.POLICY.REIMBURSEMENT_CHOICES.REIMBURSEMENT_YES,
});
}

function getCorpayBankAccountFields(country: string, currency: string) {
const parameters = {
countryISO: country,
Expand Down Expand Up @@ -922,4 +946,5 @@ export {
clearCorpayBankAccountFields,
finishCorpayBankAccountOnboarding,
clearReimbursementAccountFinishCorpayBankAccountOnboarding,
convertReimbursementAccountDataToAchAccountData,
};
5 changes: 5 additions & 0 deletions src/pages/ReimbursementAccount/ConnectedVerifiedBankAccount.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,9 +10,11 @@ import ScreenWrapper from '@components/ScreenWrapper';
import ScrollView from '@components/ScrollView';
import Section from '@components/Section';
import Text from '@components/Text';
import useBeforeRemove from '@hooks/useBeforeRemove';
import useLocalize from '@hooks/useLocalize';
import useResponsiveLayout from '@hooks/useResponsiveLayout';
import useThemeStyles from '@hooks/useThemeStyles';
import {convertReimbursementAccountDataToAchAccountData} from '@libs/actions/BankAccounts';
import WorkspaceResetBankAccountModal from '@pages/workspace/WorkspaceResetBankAccountModal';
import {requestResetBankAccount, resetReimbursementAccount} from '@userActions/ReimbursementAccount';
import type {ReimbursementAccount} from '@src/types/onyx';
Expand Down Expand Up @@ -60,6 +62,9 @@ function ConnectedVerifiedBankAccount({
const pendingAction = reimbursementAccount?.pendingAction;
const shouldShowResetModal = reimbursementAccount?.shouldShowResetModal ?? false;

useBeforeRemove(() => {
convertReimbursementAccountDataToAchAccountData(reimbursementAccount);
});
return (
<ScreenWrapper
testID={ConnectedVerifiedBankAccount.displayName}
Expand Down
Loading