-
Notifications
You must be signed in to change notification settings - Fork 2.9k
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.
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
Changes from all commits
9af39f6
bb7f77c
8dc35ac
354a2f6
f597bea
25bb7e5
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -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'; | ||
|
@@ -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'; | ||
|
@@ -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(); | ||
|
@@ -78,17 +75,21 @@ function WorkspaceWorkflowsPage({policy, betas, route, reimbursementAccount, ses | |
navigateToBankAccountRoute(route.params.policyID, ROUTES.WORKSPACE_WORKFLOWS.getRoute(route.params.policyID)); | ||
}, [policy, route.params.policyID]); | ||
|
||
useNetwork({onReconnect: fetchData}); | ||
const {isOffline} = useNetwork({onReconnect: fetchData}); | ||
const isPolicyAdmin = PolicyUtils.isPolicyAdmin(policy); | ||
|
||
useEffect(() => { | ||
fetchData(); | ||
// eslint-disable-next-line react-hooks/exhaustive-deps | ||
}, []); | ||
|
||
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; | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe 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; | ||
|
@@ -187,15 +188,16 @@ function WorkspaceWorkflowsPage({policy, betas, route, reimbursementAccount, ses | |
? translate('common.bankAccount') | ||
: translate('workflowsPage.connectBankAccount') | ||
} | ||
description={state === BankAccount.STATE.OPEN ? bankDisplayName : undefined} | ||
description={bankDisplayName} | ||
disabled={isOffline || !isPolicyAdmin} | ||
onPress={() => { | ||
if (!Policy.isCurrencySupportedForDirectReimbursement(policy?.outputCurrency ?? '')) { | ||
setIsCurrencyModalOpen(true); | ||
return; | ||
} | ||
navigateToBankAccountRoute(route.params.policyID, ROUTES.WORKSPACE_WORKFLOWS.getRoute(route.params.policyID)); | ||
}} | ||
shouldShowRightIcon | ||
shouldShowRightIcon={!isOffline && isPolicyAdmin} | ||
wrapperStyle={containerStyle} | ||
hoverAndPressStyle={[styles.mr0, styles.br2]} | ||
/> | ||
|
@@ -238,9 +240,10 @@ function WorkspaceWorkflowsPage({policy, betas, route, reimbursementAccount, ses | |
onPressAutoReportingFrequency, | ||
preferredLocale, | ||
canUseDelayedSubmission, | ||
reimbursementAccount?.achData, | ||
displayNameForAuthorizedPayer, | ||
session?.accountID, | ||
isOffline, | ||
isPolicyAdmin, | ||
]); | ||
|
||
const renderOptionItem = (item: ToggleSettingOptionRowProps, index: number) => ( | ||
|
@@ -263,8 +266,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 | ||
|
@@ -317,10 +319,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, | ||
}, | ||
|
There was a problem hiding this comment.
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 setachAccount
tonull
in optimistic data. So, even after disconnecting, the bank account preview still appears until we get response from backend.More details here - #39439 (comment)