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

Jump to bottom

Feat/44305 be integration configure expensify cards for policy #46564

Merged
merged 24 commits into from
Aug 12, 2024
Merged

Feat/44305 be integration configure expensify cards for policy #46564

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
24 commits
Select commit Hold shift + click to select a range
1a19a65
feat: add new write command - ConfigureExpensifyCardForPolicy
koko57 Jul 30, 2024
3abc929
feat: use new api call in WorkspaceExpensifyCardBankAccounts page
koko57 Jul 31, 2024
8cc9256
fix: replace policyID for workspaceAccountID
koko57 Jul 31, 2024
ed8ac0c
fix: lint
koko57 Jul 31, 2024
c822a00
Merge branch 'main' into feat/44305-be-integration-ConfigureExpensify…
koko57 Jul 31, 2024
1e51c2a
fix: resolve conflicts
koko57 Aug 7, 2024
e2b5d8e
fix: remove sharedNvp prefix
koko57 Aug 7, 2024
91f4dfc
Merge branch 'main' into feat/44305-be-integration-ConfigureExpensify…
koko57 Aug 7, 2024
03a18c7
fix: lint
koko57 Aug 7, 2024
abfb9cf
fix: minor fix
koko57 Aug 7, 2024
8c040fb
fix: change workspaceAccountID type
koko57 Aug 7, 2024
3eec68c
fix: resolve conflicts
koko57 Aug 8, 2024
bd89ae2
fix: typecheck
koko57 Aug 8, 2024
e883b68
fix: lint and typecheck
koko57 Aug 8, 2024
cb8a142
fix: minor fix
koko57 Aug 8, 2024
c4bf48d
fix: minor improvements
koko57 Aug 9, 2024
8c66efd
fix: minor fix
koko57 Aug 9, 2024
4620f69
fix: redirect to a proper page when no elligible bank account
koko57 Aug 9, 2024
a3c40c6
fix: resolve conflicts
koko57 Aug 12, 2024
784f1f2
fix: typecheck
koko57 Aug 12, 2024
ee65625
fix: lint
koko57 Aug 12, 2024
b76ab11
fix: minor fix
koko57 Aug 12, 2024
39545cd
fix: rename to private
koko57 Aug 12, 2024
c25fba0
fix: revert naming for reconciliation connection keys
koko57 Aug 12, 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
16 changes: 8 additions & 8 deletions src/ONYXKEYS.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -459,20 +459,20 @@ const ONYXKEYS = {
/** Collection of objects where each object represents the owner of the workspace that is past due billing AND the user is a member of. */
SHARED_NVP_PRIVATE_USER_BILLING_GRACE_PERIOD_END: 'sharedNVP_private_billingGracePeriodEnd_',

/** Expensify cards settings */
SHARED_NVP_PRIVATE_EXPENSIFY_CARD_SETTINGS: 'sharedNVP_private_expensifyCardSettings_',

/**
* Stores the card list for a given fundID and feed in the format: card_<fundID>_<bankName>
* So for example: card_12345_Expensify Card
*/
WORKSPACE_CARDS_LIST: 'card_',

/** Expensify cards settings */
PRIVATE_EXPENSIFY_CARD_SETTINGS: 'private_expensifyCardSettings_',

/** Stores which connection is set up to use Continuous Reconciliation */
SHARED_NVP_EXPENSIFY_CARD_CONTINUOUS_RECONCILIATION_CONNECTION: 'sharedNVP_expensifyCard_continuousReconciliationConnection_',
EXPENSIFY_CARD_CONTINUOUS_RECONCILIATION_CONNECTION: 'expensifyCard_continuousReconciliationConnection_',

/** The value that indicates whether Continuous Reconciliation should be used on the domain */
SHARED_NVP_EXPENSIFY_CARD_USE_CONTINUOUS_RECONCILIATION: 'sharedNVP_expensifyCard_useContinuousReconciliation_',
EXPENSIFY_CARD_USE_CONTINUOUS_RECONCILIATION: 'expensifyCard_useContinuousReconciliation_',
},

/** List of Form ids */
Expand Down Expand Up @@ -737,10 +737,10 @@ type OnyxCollectionValuesMapping = {
[ONYXKEYS.COLLECTION.POLICY_CONNECTION_SYNC_PROGRESS]: OnyxTypes.PolicyConnectionSyncProgress;
[ONYXKEYS.COLLECTION.SNAPSHOT]: OnyxTypes.SearchResults;
[ONYXKEYS.COLLECTION.SHARED_NVP_PRIVATE_USER_BILLING_GRACE_PERIOD_END]: OnyxTypes.BillingGraceEndPeriod;
[ONYXKEYS.COLLECTION.SHARED_NVP_PRIVATE_EXPENSIFY_CARD_SETTINGS]: OnyxTypes.ExpensifyCardSettings;
[ONYXKEYS.COLLECTION.PRIVATE_EXPENSIFY_CARD_SETTINGS]: OnyxTypes.ExpensifyCardSettings;
[ONYXKEYS.COLLECTION.WORKSPACE_CARDS_LIST]: OnyxTypes.WorkspaceCardsList;
[ONYXKEYS.COLLECTION.SHARED_NVP_EXPENSIFY_CARD_CONTINUOUS_RECONCILIATION_CONNECTION]: OnyxTypes.PolicyConnectionName;
[ONYXKEYS.COLLECTION.SHARED_NVP_EXPENSIFY_CARD_USE_CONTINUOUS_RECONCILIATION]: boolean;
[ONYXKEYS.COLLECTION.EXPENSIFY_CARD_CONTINUOUS_RECONCILIATION_CONNECTION]: OnyxTypes.PolicyConnectionName;
[ONYXKEYS.COLLECTION.EXPENSIFY_CARD_USE_CONTINUOUS_RECONCILIATION]: boolean;
};

type OnyxValuesMapping = {
Expand Down
5 changes: 2 additions & 3 deletions src/ROUTES.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -716,12 +716,11 @@ const ROUTES = {
},
WORKSPACE_ACCOUNTING_CARD_RECONCILIATION: {
route: 'settings/workspaces/:policyID/accounting/:connection/card-reconciliation',
getRoute: (policyID: string, connection: ValueOf<typeof CONST.POLICY.CONNECTIONS.NAME>) => `settings/workspaces/${policyID}/accounting/${connection}/card-reconciliation` as const,
getRoute: (policyID: string, connection?: ConnectionName) => `settings/workspaces/${policyID}/accounting/${connection}/card-reconciliation` as const,
},
WORKSPACE_ACCOUNTING_RECONCILIATION_ACCOUNT_SETTINGS: {
route: 'settings/workspaces/:policyID/accounting/:connection/card-reconciliation/account',
getRoute: (policyID: string, connection?: ValueOf<typeof CONST.POLICY.CONNECTIONS.NAME>) =>
`settings/workspaces/${policyID}/accounting/${connection}/card-reconciliation/account` as const,
getRoute: (policyID: string, connection?: ConnectionName) => `settings/workspaces/${policyID}/accounting/${connection}/card-reconciliation/account` as const,
},
WORKSPACE_CATEGORIES: {
route: 'settings/workspaces/:policyID/categories',
Expand Down
6 changes: 6 additions & 0 deletions src/libs/API/parameters/ConfigureExpensifyCardsForPolicyParams.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
type ConfigureExpensifyCardsForPolicyParams = {
policyID: string;
bankAccountID: number;
};

export default ConfigureExpensifyCardsForPolicyParams;
1 change: 1 addition & 0 deletions src/libs/API/parameters/index.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -270,3 +270,4 @@ export type {default as ExportSearchItemsToCSVParams} from './ExportSearchItemsT
export type {default as UpdateExpensifyCardLimitParams} from './UpdateExpensifyCardLimitParams';
export type {CreateWorkspaceApprovalParams, UpdateWorkspaceApprovalParams, RemoveWorkspaceApprovalParams} from './WorkspaceApprovalParams';
export type {default as StartIssueNewCardFlowParams} from './StartIssueNewCardFlowParams';
export type {default as ConfigureExpensifyCardsForPolicyParams} from './ConfigureExpensifyCardsForPolicyParams';
2 changes: 2 additions & 0 deletions src/libs/API/types.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -325,6 +325,7 @@ const WRITE_COMMANDS = {
CREATE_WORKSPACE_APPROVAL: 'CreateWorkspaceApproval',
UPDATE_WORKSPACE_APPROVAL: 'UpdateWorkspaceApproval',
REMOVE_WORKSPACE_APPROVAL: 'RemoveWorkspaceApproval',
CONFIGURE_EXPENSIFY_CARDS_FOR_POLICY: 'ConfigureExpensifyCardsForPolicy',
} as const;

type WriteCommand = ValueOf<typeof WRITE_COMMANDS>;
Expand Down Expand Up @@ -656,6 +657,7 @@ type WriteCommandParameters = {
[WRITE_COMMANDS.CREATE_WORKSPACE_APPROVAL]: Parameters.CreateWorkspaceApprovalParams;
[WRITE_COMMANDS.UPDATE_WORKSPACE_APPROVAL]: Parameters.UpdateWorkspaceApprovalParams;
[WRITE_COMMANDS.REMOVE_WORKSPACE_APPROVAL]: Parameters.RemoveWorkspaceApprovalParams;
[WRITE_COMMANDS.CONFIGURE_EXPENSIFY_CARDS_FOR_POLICY]: Parameters.ConfigureExpensifyCardsForPolicyParams;
};

const READ_COMMANDS = {
Expand Down
10 changes: 10 additions & 0 deletions src/libs/PolicyUtils.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -923,6 +923,15 @@ function getCurrentTaxID(policy: OnyxEntry<Policy>, taxID: string): string | und
return Object.keys(policy?.taxRates?.taxes ?? {}).find((taxIDKey) => policy?.taxRates?.taxes?.[taxIDKey].previousTaxCode === taxID || taxIDKey === taxID);
}

function getWorkspaceAccountID(policyID: string) {
const policy = getPolicy(policyID);

if (!policy) {
return 0;
}
return policy.workspaceAccountID ?? 0;
Copy link
Contributor

Choose a reason for hiding this comment

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

@koko57 I am curious about the difference between policyID and workspaceAccountID. Could you help to explain this?

cc @mountiny

Copy link
Contributor Author

Choose a reason for hiding this comment

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

we use workspaceAccountID instead of policyID for the expensifyCard identifier

Copy link
Contributor

Choose a reason for hiding this comment

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

@koko57 Are these two IDs the same or different? We're using a different key (workspaceAccountID), but the value remains the same, correct?

Copy link
Contributor

Choose a reason for hiding this comment

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

I'm asking this because I noticed that in the functions updateExpensifyCardLimit and updateSettlementFrequency, we define workspaceAccountID, but when calling these functions, we pass policyID.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@DylanDylann They should pass workspaceAccountID, I should have corrected them too - I can do it in this PR or I will do it while integrating with the BE. workspaceAccountID and policyID have different values

Copy link
Contributor

Choose a reason for hiding this comment

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

They are different IDs and are used for different concepts in the backend.

It's a bit complicated, and it's built like that for historical reasons, but the card functionality is tied to "domain." A domain is a row in the accounts table (the same as using an account when they sign up). The domain accounts are specific given the email format they use.

Now to have an expensify card in oldDot you need to have a verified domain (like expensify.com). We are changing that for the workspace feeds, but we still create this fake account in the database that is 1:1 tied to the policy. The workpaceAccountID is the accountID of this "fake" domain account. policyID is just an ID of the policy

}

export {
canEditTaxRate,
extractPolicyIDFromPath,
Expand Down Expand Up @@ -1024,6 +1033,7 @@ export {
getSubmitToEmail,
getForwardsToAccount,
getSubmitToAccountID,
getWorkspaceAccountID,
};

export type {MemberEmailsToAccountIDs};
48 changes: 31 additions & 17 deletions src/libs/actions/Card.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -206,14 +206,14 @@ function revealVirtualCardDetails(cardID: number): Promise<ExpensifyCardDetails>
});
}

function updateSettlementFrequency(policyID: string, frequency: ValueOf<typeof CONST.EXPENSIFY_CARD.FREQUENCY_SETTING>) {
function updateSettlementFrequency(workspaceAccountID: number, frequency: ValueOf<typeof CONST.EXPENSIFY_CARD.FREQUENCY_SETTING>) {
// TODO: remove this code when the API is ready
if (frequency === CONST.EXPENSIFY_CARD.FREQUENCY_SETTING.DAILY) {
Onyx.merge(`${ONYXKEYS.COLLECTION.SHARED_NVP_PRIVATE_EXPENSIFY_CARD_SETTINGS}${policyID}`, {
Onyx.merge(`${ONYXKEYS.COLLECTION.PRIVATE_EXPENSIFY_CARD_SETTINGS}${workspaceAccountID}`, {
monthlySettlementDate: null,
});
} else {
Onyx.merge(`${ONYXKEYS.COLLECTION.SHARED_NVP_PRIVATE_EXPENSIFY_CARD_SETTINGS}${policyID}`, {
Onyx.merge(`${ONYXKEYS.COLLECTION.PRIVATE_EXPENSIFY_CARD_SETTINGS}${workspaceAccountID}`, {
monthlySettlementDate: new Date(),
});
}
Expand All @@ -222,7 +222,7 @@ function updateSettlementFrequency(policyID: string, frequency: ValueOf<typeof C
// const optimisticData: OnyxUpdate[] = [
// {
// onyxMethod: Onyx.METHOD.MERGE,
// key: `${ONYXKEYS.COLLECTION.SHARED_NVP_PRIVATE_EXPENSIFY_CARD_SETTINGS}${policyID}`,
// key: `${ONYXKEYS.COLLECTION.PRIVATE_EXPENSIFY_CARD_SETTINGS}${workspaceAccountID}`,
// value: {
// monthlySettlementDate: '',
// },
Expand All @@ -232,7 +232,7 @@ function updateSettlementFrequency(policyID: string, frequency: ValueOf<typeof C
// const successData: OnyxUpdate[] = [
// {
// onyxMethod: Onyx.METHOD.MERGE,
// key: `${ONYXKEYS.COLLECTION.SHARED_NVP_PRIVATE_EXPENSIFY_CARD_SETTINGS}${policyID}`,
// key: `${ONYXKEYS.COLLECTION.PRIVATE_EXPENSIFY_CARD_SETTINGS}${workspaceAccountID}`,
// value: {
// monthlySettlementDate: '',
// },
Expand All @@ -242,32 +242,32 @@ function updateSettlementFrequency(policyID: string, frequency: ValueOf<typeof C
// const failureData: OnyxUpdate[] = [
// {
// onyxMethod: Onyx.METHOD.MERGE,
// key: `${ONYXKEYS.COLLECTION.SHARED_NVP_PRIVATE_EXPENSIFY_CARD_SETTINGS}${policyID}`,
// key: `${ONYXKEYS.COLLECTION.PRIVATE_EXPENSIFY_CARD_SETTINGS}${workspaceAccountID}`,
// value: {
// monthlySettlementDate: null,
// },
// },
// ];
//
// const parameters = {
// workspaceAccountID: policyID,
// workspaceAccountID,
// settlementFrequency: frequency,
// };
//
// API.write(WRITE_COMMANDS.UPDATE_CARD_SETTLEMENT_FREQUENCY, parameters, {optimisticData, successData, failureData});
}

function updateSettlementAccount(policyID: string, accountID: number) {
function updateSettlementAccount(workspaceAccountID: number, accountID: number) {
// TODO: remove this code when the API is ready
Onyx.merge(`${ONYXKEYS.COLLECTION.SHARED_NVP_PRIVATE_EXPENSIFY_CARD_SETTINGS}${policyID}`, {
Onyx.merge(`${ONYXKEYS.COLLECTION.PRIVATE_EXPENSIFY_CARD_SETTINGS}${workspaceAccountID}`, {
paymentBankAccountID: accountID,
});

// TODO: uncomment this code when the API is ready
// const optimisticData: OnyxUpdate[] = [
// {
// onyxMethod: Onyx.METHOD.MERGE,
// key: `${ONYXKEYS.COLLECTION.SHARED_NVP_PRIVATE_EXPENSIFY_CARD_SETTINGS}${policyID}`,
// key: `${ONYXKEYS.COLLECTION.PRIVATE_EXPENSIFY_CARD_SETTINGS}${workspaceAccountID}`,
// value: {
// paymentBankAccountID: accountID,
// },
Expand All @@ -277,7 +277,7 @@ function updateSettlementAccount(policyID: string, accountID: number) {
// const successData: OnyxUpdate[] = [
// {
// onyxMethod: Onyx.METHOD.MERGE,
// key: `${ONYXKEYS.COLLECTION.SHARED_NVP_PRIVATE_EXPENSIFY_CARD_SETTINGS}${policyID}`,
// key: `${ONYXKEYS.COLLECTION.PRIVATE_EXPENSIFY_CARD_SETTINGS}${workspaceAccountID}`,
// value: {
// paymentBankAccountID: accountID,
// },
Expand All @@ -287,15 +287,15 @@ function updateSettlementAccount(policyID: string, accountID: number) {
// const failureData: OnyxUpdate[] = [
// {
// onyxMethod: Onyx.METHOD.MERGE,
// key: `${ONYXKEYS.COLLECTION.SHARED_NVP_PRIVATE_EXPENSIFY_CARD_SETTINGS}${policyID}`,
// key: `${ONYXKEYS.COLLECTION.PRIVATE_EXPENSIFY_CARD_SETTINGS}${workspaceAccountID}`,
// value: {
// paymentBankAccountID: null,
// },
// },
// ];
//
// const parameters = {
// workspaceAccountID: policyID,
// workspaceAccountID,
// settlementBankAccountID: accountID,
// };
//
Expand All @@ -313,7 +313,7 @@ function clearIssueNewCardFlow() {
});
}

function updateExpensifyCardLimit(policyID: string, cardID: number, newLimit: number, oldLimit?: number) {
function updateExpensifyCardLimit(workspaceAccountID: number, cardID: number, newLimit: number, oldLimit?: number) {
const authToken = NetworkStore.getAuthToken();

if (!authToken) {
Expand All @@ -323,7 +323,7 @@ function updateExpensifyCardLimit(policyID: string, cardID: number, newLimit: nu
const optimisticData: OnyxUpdate[] = [
{
onyxMethod: Onyx.METHOD.MERGE,
key: `${ONYXKEYS.COLLECTION.WORKSPACE_CARDS_LIST}${policyID}_${CONST.EXPENSIFY_CARD.BANK}`,
key: `${ONYXKEYS.COLLECTION.WORKSPACE_CARDS_LIST}${workspaceAccountID}_${CONST.EXPENSIFY_CARD.BANK}`,
value: {
[cardID]: {
nameValuePairs: {
Expand All @@ -339,7 +339,7 @@ function updateExpensifyCardLimit(policyID: string, cardID: number, newLimit: nu
const successData: OnyxUpdate[] = [
{
onyxMethod: Onyx.METHOD.MERGE,
key: `${ONYXKEYS.COLLECTION.WORKSPACE_CARDS_LIST}${policyID}_${CONST.EXPENSIFY_CARD.BANK}`,
key: `${ONYXKEYS.COLLECTION.WORKSPACE_CARDS_LIST}${workspaceAccountID}_${CONST.EXPENSIFY_CARD.BANK}`,
value: {
[cardID]: {
isLoading: false,
Expand All @@ -351,7 +351,7 @@ function updateExpensifyCardLimit(policyID: string, cardID: number, newLimit: nu
const failureData: OnyxUpdate[] = [
{
onyxMethod: Onyx.METHOD.MERGE,
key: `${ONYXKEYS.COLLECTION.WORKSPACE_CARDS_LIST}${policyID}_${CONST.EXPENSIFY_CARD.BANK}`,
key: `${ONYXKEYS.COLLECTION.WORKSPACE_CARDS_LIST}${workspaceAccountID}_${CONST.EXPENSIFY_CARD.BANK}`,
value: {
[cardID]: {
nameValuePairs: {
Expand Down Expand Up @@ -381,6 +381,19 @@ function startIssueNewCardFlow(policyID: string) {
API.read(READ_COMMANDS.START_ISSUE_NEW_CARD_FLOW, parameters);
}

function configureExpensifyCardsForPolicy(policyID: string, bankAccountID?: number) {
if (!bankAccountID) {
return;
}

const parameters = {
policyID,
bankAccountID,
};

API.write(WRITE_COMMANDS.CONFIGURE_EXPENSIFY_CARDS_FOR_POLICY, parameters);
}

export {
requestReplacementExpensifyCard,
activatePhysicalExpensifyCard,
Expand All @@ -393,5 +406,6 @@ export {
updateExpensifyCardLimit,
updateSettlementAccount,
startIssueNewCardFlow,
configureExpensifyCardsForPolicy,
};
export type {ReplacementReason};
5 changes: 2 additions & 3 deletions src/pages/workspace/WorkspaceMoreFeaturesPage.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -69,9 +69,8 @@ function WorkspaceMoreFeaturesPage({policy, route}: WorkspaceMoreFeaturesPagePro
!!policy?.connections?.xero?.config?.importTaxRates ||
!!policy?.connections?.netsuite?.options?.config?.syncOptions?.syncTax;
const policyID = policy?.id;
// @ts-expect-error a new props will be added during feed api implementation
const workspaceAccountID = (policy?.workspaceAccountID as string) ?? '';
const [cardsList] = useOnyx(`${ONYXKEYS.COLLECTION.WORKSPACE_CARDS_LIST}${workspaceAccountID}${CONST.EXPENSIFY_CARD.BANK}`);
const workspaceAccountID = policy?.workspaceAccountID ?? -1;
const [cardsList] = useOnyx(`${ONYXKEYS.COLLECTION.WORKSPACE_CARDS_LIST}${workspaceAccountID.toString()}${CONST.EXPENSIFY_CARD.BANK}`);
const [isOrganizeWarningModalOpen, setIsOrganizeWarningModalOpen] = useState(false);
const [isIntegrateWarningModalOpen, setIsIntegrateWarningModalOpen] = useState(false);
const [isReportFieldsWarningModalOpen, setIsReportFieldsWarningModalOpen] = useState(false);
Expand Down
8 changes: 4 additions & 4 deletions src/pages/workspace/accounting/reconciliation/CardReconciliationPage.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -26,12 +26,12 @@ function CardReconciliationPage({policy, route}: CardReconciliationPageProps) {
const styles = useThemeStyles();
const {translate} = useLocalize();

const [reconciliationConnection] = useOnyx(`${ONYXKEYS.COLLECTION.SHARED_NVP_EXPENSIFY_CARD_CONTINUOUS_RECONCILIATION_CONNECTION}${policy?.id}`);
const [isContinuousReconciliationOn] = useOnyx(`${ONYXKEYS.COLLECTION.SHARED_NVP_EXPENSIFY_CARD_USE_CONTINUOUS_RECONCILIATION}${policy?.id}`);
const [reconciliationConnection] = useOnyx(`${ONYXKEYS.COLLECTION.EXPENSIFY_CARD_CONTINUOUS_RECONCILIATION_CONNECTION}${policy?.workspaceAccountID}`);
const [isContinuousReconciliationOn] = useOnyx(`${ONYXKEYS.COLLECTION.EXPENSIFY_CARD_USE_CONTINUOUS_RECONCILIATION}${policy?.workspaceAccountID}`);
const [cardSettings] = useOnyx(`${ONYXKEYS.COLLECTION.PRIVATE_EXPENSIFY_CARD_SETTINGS}${policy?.workspaceAccountID}`);
const [bankAccountList] = useOnyx(ONYXKEYS.BANK_ACCOUNT_LIST);
const [cardSettings] = useOnyx(`${ONYXKEYS.COLLECTION.SHARED_NVP_PRIVATE_EXPENSIFY_CARD_SETTINGS}${policy?.id}`);

const paymentBankAccountID = cardSettings?.paymentBankAccountID ?? -1;
const paymentBankAccountID = cardSettings?.paymentBankAccountID ?? 0;
Copy link
Contributor

Choose a reason for hiding this comment

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

Why do we need to change this line?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

yes, it caused some problems in another place, we want the value falsy (-1 is truthy)

Copy link
Contributor

Choose a reason for hiding this comment

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

@koko57 Could you detail which problem is in other places?

Because we have a below rule

App/contributingGuides/STYLE.md

Lines 474 to 490 in 9812479

### Default value for inexistent IDs
Use `'-1'` or `-1` when there is a possibility that the ID property of an Onyx value could be `null` or `undefined`.
``` ts
// BAD
const foo = report?.reportID ?? '';
const bar = report?.reportID ?? '0';
report ? report.reportID : '0';
report ? report.reportID : '';
// GOOD
const foo = report?.reportID ?? '-1';
report ? report.reportID : '-1';
```

Copy link
Contributor Author

Choose a reason for hiding this comment

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

but here you have a string, I wonder why we need to change it here as we have a number. And as I said - it caused a problem here #45901 (comment)

Copy link
Contributor

@DylanDylann DylanDylann Aug 10, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

@koko57

1. The current place (in this PR)

    const paymentBankAccountID = cardSettings?.paymentBankAccountID ?? 0;
    const bankAccountTitle = bankAccountList?.[paymentBankAccountID]?.title ?? '';

If we use 0 as a fallback value for paymentBankAccountID, when cardSettings?.paymentBankAccountID is undefined, paymentBankAccountID is 0 and bankAccountTitle is bankAccountList?.[0]

--> It mean that we always use bankAccountList?.[0] as default value for bankAccountTitle

My suggestion:

    const paymentBankAccountID = cardSettings?.paymentBankAccountID ?? -1;   // USING '-1'
    const bankAccountTitle = bankAccountList?.[paymentBankAccountID]?.title ?? '';

--> It mean that if cardSettings?.paymentBankAccountID is undefined, bankAccountTitle is empty string

2. Your case that you mentioned here

Why do we need a fallback value in this case?

App/src/pages/workspace/expensifyCard/WorkspaceExpensifyCardPage.tsx

Line 18 in e6546d6

const paymentBankAccountID = cardSettings?.paymentBankAccountID ?? 0;

My suggestion: : We don't need fallback value

    const paymentBankAccountID = cardSettings?.paymentBankAccountID;

Copy link
Contributor Author

@koko57 koko57 Aug 12, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

@DylanDylann what is the difference here? bankAccountList?.[0] vs bankAccountList?.[-1]? bankAccountList is an object

const bankAccountTitle = bankAccountList?.[paymentBankAccountID]?.title ?? '';

const policyID = policy?.id ?? '-1';
Expand Down
13 changes: 9 additions & 4 deletions src/pages/workspace/accounting/reconciliation/ReconciliationAccountSettingsPage.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,8 @@ import Text from '@components/Text';
import TextLink from '@components/TextLink';
import useLocalize from '@hooks/useLocalize';
import useThemeStyles from '@hooks/useThemeStyles';
import {getLastFourDigits} from '@libs/BankAccountUtils';
import * as PolicyUtils from '@libs/PolicyUtils';
import Navigation from '@navigation/Navigation';
import type {SettingsNavigatorParamList} from '@navigation/types';
import CONST from '@src/CONST';
Expand All @@ -19,16 +21,19 @@ type ReconciliationAccountSettingsPageProps = StackScreenProps<SettingsNavigator

function ReconciliationAccountSettingsPage({route}: ReconciliationAccountSettingsPageProps) {
const {policyID, connection} = route.params;
const settlementAccountEnding = '1234'; // TODO: use correct settlement account ending value https://github.com/Expensify/App/issues/44313

const styles = useThemeStyles();
const {translate} = useLocalize();

const workspaceAccountID = PolicyUtils.getWorkspaceAccountID(policyID);

const [bankAccountList] = useOnyx(ONYXKEYS.BANK_ACCOUNT_LIST);
const [cardSettings] = useOnyx(`${ONYXKEYS.COLLECTION.SHARED_NVP_PRIVATE_EXPENSIFY_CARD_SETTINGS}${policyID}`);
const paymentBankAccountID = cardSettings?.paymentBankAccountID ?? '';
const [cardSettings] = useOnyx(`${ONYXKEYS.COLLECTION.PRIVATE_EXPENSIFY_CARD_SETTINGS}${workspaceAccountID}`);
const paymentBankAccountID = cardSettings?.paymentBankAccountID ?? 0;

const selectedBankAccount = useMemo(() => bankAccountList?.[paymentBankAccountID], [paymentBankAccountID, bankAccountList]);
const selectedBankAccount = useMemo(() => bankAccountList?.[paymentBankAccountID.toString()], [paymentBankAccountID, bankAccountList]);
const bankAccountNumber = useMemo(() => selectedBankAccount?.accountData?.accountNumber ?? '', [selectedBankAccount]);
const settlementAccountEnding = getLastFourDigits(bankAccountNumber);

const sections = useMemo(() => {
const data = Object.values(bankAccountList ?? {}).map((bankAccount) => ({
Expand Down
2 changes: 1 addition & 1 deletion src/pages/workspace/expensifyCard/WorkspaceCardListHeader.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,7 @@ function WorkspaceCardListHeader() {
const isLessThanMediumScreen = isMediumScreenWidth || isSmallScreenWidth;

// TODO: uncomment the code line below to use cardSettings data from Onyx when it's supported
// const [cardSettings] = useOnyx(`${ONYXKEYS.COLLECTION.SHARED_NVP_PRIVATE_EXPENSIFY_CARD_SETTINGS}${policyID}`);
// const [cardSettings] = useOnyx(`${ONYXKEYS.COLLECTION.PRIVATE_EXPENSIFY_CARD_SETTINGS}${workspaceAccountID}`);
const cardSettings = mockedSettings;

return (
Expand Down
Loading
Loading