Skip to content
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

Use policy achAccount instead of reimbursementAccount in WorkspaceWorkflow #38395

Merged
merged 6 commits into from
Mar 28, 2024
Merged

Use policy achAccount instead of reimbursementAccount in WorkspaceWorkflow #38395

Show file tree
Hide file tree
Changes from 4 commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
9af39f6
Use policy achAccount instead of reimbursementAccount in WorkspaceWor…
nkuoch Mar 15, 2024
bb7f77c
Fix displayName
nkuoch Mar 26, 2024
8dc35ac
Merge branch 'main' into nat-workflow
nkuoch Mar 26, 2024
354a2f6
Add back OnyxData, but with Policy loading key
nkuoch Mar 26, 2024
f597bea
Prevent opening vbba modal if offline
nkuoch Mar 27, 2024
25bb7e5
Fix lint and use disabled
nkuoch Mar 28, 2024
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
9 changes: 3 additions & 6 deletions src/libs/actions/Policy.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2206,8 +2206,7 @@ function openPolicyWorkflowsPage(policyID: string) {
optimisticData: [
{
onyxMethod: Onyx.METHOD.MERGE,
// @ts-expect-error: ONYXKEYS.REIMBURSEMENT_ACCOUNT is conflicting with ONYXKEYS.FORMS.REIMBURSEMENT_ACCOUNT_FORM
key: `${ONYXKEYS.REIMBURSEMENT_ACCOUNT}${policyID}`,
key: `${ONYXKEYS.COLLECTION.POLICY}${policyID}`,
value: {
isLoading: true,
},
Expand All @@ -2216,8 +2215,7 @@ function openPolicyWorkflowsPage(policyID: string) {
successData: [
{
onyxMethod: Onyx.METHOD.MERGE,
// @ts-expect-error: ONYXKEYS.REIMBURSEMENT_ACCOUNT is conflicting with ONYXKEYS.FORMS.REIMBURSEMENT_ACCOUNT_FORM
key: `${ONYXKEYS.REIMBURSEMENT_ACCOUNT}${policyID}`,
key: `${ONYXKEYS.COLLECTION.POLICY}${policyID}`,
value: {
isLoading: false,
},
Expand All @@ -2226,8 +2224,7 @@ function openPolicyWorkflowsPage(policyID: string) {
failureData: [
{
onyxMethod: Onyx.METHOD.MERGE,
// @ts-expect-error: ONYXKEYS.REIMBURSEMENT_ACCOUNT is conflicting with ONYXKEYS.FORMS.REIMBURSEMENT_ACCOUNT_FORM
key: `${ONYXKEYS.REIMBURSEMENT_ACCOUNT}${policyID}`,
key: `${ONYXKEYS.COLLECTION.POLICY}${policyID}`,
value: {
isLoading: false,
},
Expand Down
25 changes: 10 additions & 15 deletions src/pages/workspace/workflows/WorkspaceWorkflowsPage.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,6 @@ import useNetwork from '@hooks/useNetwork';
import useThemeStyles from '@hooks/useThemeStyles';
import useWindowDimensions from '@hooks/useWindowDimensions';
import * as ErrorUtils from '@libs/ErrorUtils';
import BankAccount from '@libs/models/BankAccount';
import Navigation from '@libs/Navigation/Navigation';
import Permissions from '@libs/Permissions';
import * as PersonalDetailsUtils from '@libs/PersonalDetailsUtils';
Expand All @@ -30,7 +29,7 @@ import CONST from '@src/CONST';
import ONYXKEYS from '@src/ONYXKEYS';
import ROUTES from '@src/ROUTES';
import type SCREENS from '@src/SCREENS';
import type {Beta, ReimbursementAccount, Session} from '@src/types/onyx';
import type {Beta, Session} from '@src/types/onyx';
import ToggleSettingOptionRow from './ToggleSettingsOptionRow';
import type {ToggleSettingOptionRowProps} from './ToggleSettingsOptionRow';
import {getAutoReportingFrequencyDisplayNames} from './WorkspaceAutoReportingFrequencyPage';
Expand All @@ -39,14 +38,12 @@ import type {AutoReportingFrequencyKey} from './WorkspaceAutoReportingFrequencyP
type WorkspaceWorkflowsPageOnyxProps = {
/** Beta features list */
betas: OnyxEntry<Beta[]>;
/** Reimbursement account details */
reimbursementAccount: OnyxEntry<ReimbursementAccount>;
/** Policy details */
session: OnyxEntry<Session>;
};
type WorkspaceWorkflowsPageProps = WithPolicyProps & WorkspaceWorkflowsPageOnyxProps & StackScreenProps<WorkspacesCentralPaneNavigatorParamList, typeof SCREENS.WORKSPACE.WORKFLOWS>;

function WorkspaceWorkflowsPage({policy, betas, route, reimbursementAccount, session}: WorkspaceWorkflowsPageProps) {
function WorkspaceWorkflowsPage({policy, betas, route, session}: WorkspaceWorkflowsPageProps) {
const {translate, preferredLocale} = useLocalize();
const styles = useThemeStyles();
const {isSmallScreenWidth} = useWindowDimensions();
Expand Down Expand Up @@ -86,9 +83,12 @@ function WorkspaceWorkflowsPage({policy, betas, route, reimbursementAccount, ses
}, []);

const optionItems: ToggleSettingOptionRowProps[] = useMemo(() => {
const {accountNumber, state, bankName} = reimbursementAccount?.achData ?? {};
const hasVBA = state === BankAccount.STATE.OPEN;
const bankDisplayName = bankName ? `${bankName} ${accountNumber ? `${accountNumber.slice(-5)}` : ''}` : '';
const {accountNumber, addressName, bankName} = policy?.achAccount ?? {};
const hasVBA = !!policy?.achAccount;
Copy link
Contributor

Choose a reason for hiding this comment

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

Hey, this PR caused a bug: Disconnected bank account is still displayed on bank account preview on Workflows page.
It has been handled but just FYI,
Whether the bank name shows up in the bank account preview on the workspace workflows page depends on the policy's achAccount. But when we disconnect the bank account, we forget to set achAccount to null in optimistic data. So, even after disconnecting, the bank account preview still appears until we get response from backend.
More details here - #39439 (comment)

Copy link
Contributor

Choose a reason for hiding this comment

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

This change also caused issue #39947 where we don't yet have a bank account added and because the achAccount: {reimburser: reimburserEmail} is set everytime we toggle "Make or track payments", the UI ends up looking like we have a bank account added because of this check, when we don't actually have a bank account added yet.

let bankDisplayName = bankName ?? addressName;
if (accountNumber && bankDisplayName !== accountNumber) {
bankDisplayName += ` ${accountNumber.slice(-5)}`;
}
const hasReimburserEmailError = !!policy?.errorFields?.reimburserEmail;
const hasApprovalError = !!policy?.errorFields?.approvalMode;
const hasDelayedSubmissionError = !!policy?.errorFields?.autoReporting;
Expand Down Expand Up @@ -187,7 +187,7 @@ function WorkspaceWorkflowsPage({policy, betas, route, reimbursementAccount, ses
? translate('common.bankAccount')
: translate('workflowsPage.connectBankAccount')
}
description={state === BankAccount.STATE.OPEN ? bankDisplayName : undefined}
description={bankDisplayName}
onPress={() => {
if (!Policy.isCurrencySupportedForDirectReimbursement(policy?.outputCurrency ?? '')) {
setIsCurrencyModalOpen(true);
Expand Down Expand Up @@ -238,7 +238,6 @@ function WorkspaceWorkflowsPage({policy, betas, route, reimbursementAccount, ses
onPressAutoReportingFrequency,
preferredLocale,
canUseDelayedSubmission,
reimbursementAccount?.achData,
displayNameForAuthorizedPayer,
session?.accountID,
]);
Expand All @@ -264,7 +263,7 @@ function WorkspaceWorkflowsPage({policy, betas, route, reimbursementAccount, ses

const isPaidGroupPolicy = PolicyUtils.isPaidGroupPolicy(policy);
const isPolicyAdmin = PolicyUtils.isPolicyAdmin(policy);
const isLoading = reimbursementAccount?.isLoading && policy?.reimbursementChoice === undefined;
const isLoading = Boolean(policy?.isLoading && policy?.reimbursementChoice === undefined);

return (
<FeatureEnabledAccessOrNotFoundWrapper
Expand Down Expand Up @@ -317,10 +316,6 @@ export default withPolicy(
betas: {
key: ONYXKEYS.BETAS,
},
reimbursementAccount: {
// @ts-expect-error: ONYXKEYS.REIMBURSEMENT_ACCOUNT is conflicting with ONYXKEYS.FORMS.REIMBURSEMENT_ACCOUNT_FORM
key: ({route}) => `${ONYXKEYS.REIMBURSEMENT_ACCOUNT}${route.params.policyID}`,
},
session: {
key: ONYXKEYS.SESSION,
},
Expand Down
11 changes: 11 additions & 0 deletions src/types/onyx/Policy.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -183,6 +183,14 @@ type Connections = {
quickbooksOnline: Connection<QBOConnectionData, QBOConnectionConfig>;
};

type ACHAccount = {
bankAccountID: number;
accountNumber: string;
routingNumber: string;
addressName: string;
bankName: string;
};

type AutoReportingOffset = number | ValueOf<typeof CONST.POLICY.AUTO_REPORTING_OFFSET>;

type PolicyReportFieldType = 'text' | 'date' | 'dropdown' | 'formula';
Expand Down Expand Up @@ -418,6 +426,9 @@ type Policy = OnyxCommon.OnyxValueWithOfflineFeedback<
/** Whether the Connections feature is enabled */
areConnectionsEnabled?: boolean;

/** The verified bank account linked to the policy */
achAccount?: ACHAccount;

/** Indicates if the Policy is in loading state */
isLoading?: boolean;

Expand Down
Loading