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

[NoQA] Feat/44313 edit settlement settings be integration #47309

Merged
merged 14 commits into from
Aug 27, 2024
Merged

[NoQA] Feat/44313 edit settlement settings be integration #47309

Show file tree
Hide file tree
Changes from 9 commits
Commits
Show all changes
14 commits
Select commit Hold shift + click to select a range
beb2a92
feat: [WIP] updateSettlementFrequency and updateSettlementAccount
koko57 Aug 9, 2024
df3797c
Merge branch 'main' into feat/44313-edit-settlement-settings-be-integ…
koko57 Aug 12, 2024
a40d4bc
fix: resolve conflicts
koko57 Aug 13, 2024
9c26711
feat: create getDomainNameForPolicy util
koko57 Aug 13, 2024
8a405f0
fix: minor fix
koko57 Aug 13, 2024
cacd33e
fix: typecheck
koko57 Aug 13, 2024
aec2763
fix: resolve conflicts
koko57 Aug 13, 2024
1005b60
fix: minor fix
koko57 Aug 13, 2024
acc9224
fix: apply requested changes
koko57 Aug 13, 2024
4cd7ab1
fix: add a comment
koko57 Aug 14, 2024
9c19ae8
fix: resolve conflicts
koko57 Aug 14, 2024
18668cc
fix: resolve conflicts
koko57 Aug 20, 2024
168c554
Merge branch 'main' into feat/44313-edit-settlement-settings-be-integ…
koko57 Aug 22, 2024
385ab1b
fix: resolve conflicts, apply requested changes
koko57 Aug 27, 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
3 changes: 3 additions & 0 deletions src/CONST.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -632,6 +632,9 @@ const CONST = {
INBOX: 'inbox',
},

EXPENSIFY_POLICY_DOMAIN: 'expensify_policy',
mountiny marked this conversation as resolved.
Show resolved Hide resolved
mountiny marked this conversation as resolved.
Show resolved Hide resolved
Copy link
Contributor

Choose a reason for hiding this comment

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

Can you please change this to the dash format?

Suggested change
EXPENSIFY_POLICY_DOMAIN: 'expensify_policy',
EXPENSIFY_POLICY_DOMAIN: 'expensify-policy',

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@mountiny done!

EXPENSIFY_POLICY_DOMAIN_EXTENSION: '.exfy',

SIGN_IN_FORM_WIDTH: 300,

REQUEST_CODE_DELAY: 30,
Expand Down
6 changes: 6 additions & 0 deletions src/libs/API/parameters/UpdateCardSettlementAccountParams.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
type UpdateCardSettlementAccountParams = {
domainName: string;
settlementBankAccountID: number;
};

export default UpdateCardSettlementAccountParams;
9 changes: 9 additions & 0 deletions src/libs/API/parameters/UpdateCardSettlementFrequencyParams.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
import type {ValueOf} from 'type-fest';
import type CONST from '@src/CONST';

type UpdateCardSettlementFrequencyParams = {
settlementFrequency: ValueOf<typeof CONST.EXPENSIFY_CARD.FREQUENCY_SETTING>;
workspaceAccountID: number;
};

export default UpdateCardSettlementFrequencyParams;
2 changes: 2 additions & 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 @@ -272,3 +272,5 @@ export type {CreateWorkspaceApprovalParams, UpdateWorkspaceApprovalParams, Remov
export type {default as StartIssueNewCardFlowParams} from './StartIssueNewCardFlowParams';
export type {default as ConfigureExpensifyCardsForPolicyParams} from './ConfigureExpensifyCardsForPolicyParams';
export type {default as CreateExpensifyCardParams} from './CreateExpensifyCardParams';
export type {default as UpdateCardSettlementFrequencyParams} from './UpdateCardSettlementFrequencyParams';
export type {default as UpdateCardSettlementAccountParams} from './UpdateCardSettlementAccountParams';
4 changes: 4 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 @@ -328,6 +328,8 @@ const WRITE_COMMANDS = {
CONFIGURE_EXPENSIFY_CARDS_FOR_POLICY: 'ConfigureExpensifyCardsForPolicy',
CREATE_EXPENSIFY_CARD: 'CreateExpensifyCard',
CREATE_ADMIN_ISSUED_VIRTUAL_CARD: 'CreateAdminIssuedVirtualCard',
UPDATE_CARD_SETTLEMENT_FREQUENCY: 'UpdateCardSettlementFrequency',
UPDATE_CARD_SETTLEMENT_ACCOUNT: 'UpdateCardSettlementAccount',
} as const;

type WriteCommand = ValueOf<typeof WRITE_COMMANDS>;
Expand Down Expand Up @@ -662,6 +664,8 @@ type WriteCommandParameters = {
[WRITE_COMMANDS.CONFIGURE_EXPENSIFY_CARDS_FOR_POLICY]: Parameters.ConfigureExpensifyCardsForPolicyParams;
[WRITE_COMMANDS.CREATE_EXPENSIFY_CARD]: Parameters.CreateExpensifyCardParams;
[WRITE_COMMANDS.CREATE_ADMIN_ISSUED_VIRTUAL_CARD]: Omit<Parameters.CreateExpensifyCardParams, 'feedCountry'>;
[WRITE_COMMANDS.UPDATE_CARD_SETTLEMENT_FREQUENCY]: Parameters.UpdateCardSettlementFrequencyParams;
[WRITE_COMMANDS.UPDATE_CARD_SETTLEMENT_ACCOUNT]: Parameters.UpdateCardSettlementAccountParams;
};

const READ_COMMANDS = {
Expand Down
9 changes: 9 additions & 0 deletions src/libs/PolicyUtils.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -969,6 +969,14 @@ function getWorkspaceAccountID(policyID: string) {
return policy.workspaceAccountID ?? 0;
}

function getDomainNameForPolicy(policyID?: string): string {
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
function getDomainNameForPolicy(policyID?: string): string {
function getDomainNameForPolicy(policyID: string): string {

Any reason for keeping this optional ?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

eslint complaining

if (!policyID) {
return '';
}

return `${CONST.EXPENSIFY_POLICY_DOMAIN}${policyID}${CONST.EXPENSIFY_POLICY_DOMAIN_EXTENSION}`;
}

export {
canEditTaxRate,
extractPolicyIDFromPath,
Expand Down Expand Up @@ -1074,6 +1082,7 @@ export {
getWorkspaceAccountID,
getAllTaxRatesNamesAndKeys as getAllTaxRates,
getTagNamesFromTagsLists,
getDomainNameForPolicy,
};

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

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.PRIVATE_EXPENSIFY_CARD_SETTINGS}${workspaceAccountID}`, {
monthlySettlementDate: null,
});
} else {
Onyx.merge(`${ONYXKEYS.COLLECTION.PRIVATE_EXPENSIFY_CARD_SETTINGS}${workspaceAccountID}`, {
monthlySettlementDate: new Date(),
});
}
function updateSettlementFrequency(workspaceAccountID: number, settlementFrequency: ValueOf<typeof CONST.EXPENSIFY_CARD.FREQUENCY_SETTING>, currentFrequency?: Date) {
mountiny marked this conversation as resolved.
Show resolved Hide resolved
const monthlySettlementDate = settlementFrequency === CONST.EXPENSIFY_CARD.FREQUENCY_SETTING.DAILY ? null : new Date();

// TODO: uncomment this code when the API is ready
// const optimisticData: OnyxUpdate[] = [
// {
// onyxMethod: Onyx.METHOD.MERGE,
// key: `${ONYXKEYS.COLLECTION.PRIVATE_EXPENSIFY_CARD_SETTINGS}${workspaceAccountID}`,
// value: {
// monthlySettlementDate: '',
// },
// },
// ];
//
// const successData: OnyxUpdate[] = [
// {
// onyxMethod: Onyx.METHOD.MERGE,
// key: `${ONYXKEYS.COLLECTION.PRIVATE_EXPENSIFY_CARD_SETTINGS}${workspaceAccountID}`,
// value: {
// monthlySettlementDate: '',
// },
// },
// ];
//
// const failureData: OnyxUpdate[] = [
// {
// onyxMethod: Onyx.METHOD.MERGE,
// key: `${ONYXKEYS.COLLECTION.PRIVATE_EXPENSIFY_CARD_SETTINGS}${workspaceAccountID}`,
// value: {
// monthlySettlementDate: null,
// },
// },
// ];
//
// const parameters = {
// workspaceAccountID,
// settlementFrequency: frequency,
// };
//
// API.write(WRITE_COMMANDS.UPDATE_CARD_SETTLEMENT_FREQUENCY, parameters, {optimisticData, successData, failureData});
const optimisticData: OnyxUpdate[] = [
{
onyxMethod: Onyx.METHOD.MERGE,
key: `${ONYXKEYS.COLLECTION.PRIVATE_EXPENSIFY_CARD_SETTINGS}${workspaceAccountID}`,
value: {
monthlySettlementDate,
},
},
];

const successData: OnyxUpdate[] = [
{
onyxMethod: Onyx.METHOD.MERGE,
key: `${ONYXKEYS.COLLECTION.PRIVATE_EXPENSIFY_CARD_SETTINGS}${workspaceAccountID}`,
value: {
monthlySettlementDate,
},
},
];

const failureData: OnyxUpdate[] = [
{
onyxMethod: Onyx.METHOD.MERGE,
key: `${ONYXKEYS.COLLECTION.PRIVATE_EXPENSIFY_CARD_SETTINGS}${workspaceAccountID}`,
value: {
monthlySettlementDate: currentFrequency,
},
},
];

const parameters = {
workspaceAccountID,
settlementFrequency,
};

API.write(WRITE_COMMANDS.UPDATE_CARD_SETTLEMENT_FREQUENCY, parameters, {optimisticData, successData, failureData});
}

function updateSettlementAccount(workspaceAccountID: number, accountID: number) {
// TODO: remove this code when the API is ready
Onyx.merge(`${ONYXKEYS.COLLECTION.PRIVATE_EXPENSIFY_CARD_SETTINGS}${workspaceAccountID}`, {
paymentBankAccountID: accountID,
});
function updateSettlementAccount(workspaceAccountID: number, domainName: string, settlementBankAccountID?: number, currentSettlementBankAccountID?: number) {
mountiny marked this conversation as resolved.
Show resolved Hide resolved
if (!settlementBankAccountID) {
return;
}
Comment on lines +255 to +257
Copy link
Contributor

Choose a reason for hiding this comment

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

Do we throw an error on the FE if this fails? I guess we just :do-nothing: if we return from here

Copy link
Contributor Author

Choose a reason for hiding this comment

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

If it's better to send some invalid value - like -1 and get the error from the BE, I'm ok with that I can change it. But in my other PRs I was requested not to do so, so I also applied it here. But since we don't have this kind of error handling anywhere through the app I think we should rely on the BE errors.
cc @mountiny wdyt?

Copy link
Contributor

Choose a reason for hiding this comment

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

Yeah I think its more common to do something and show error but I guess the problem with these is that its theoretical problem only, right? Like it should not happen that the bankAccountID is missing when we get this far?

Happy to change this to call with invalid value or the least we should do is to add some log


const optimisticData: OnyxUpdate[] = [
{
onyxMethod: Onyx.METHOD.MERGE,
key: `${ONYXKEYS.COLLECTION.PRIVATE_EXPENSIFY_CARD_SETTINGS}${workspaceAccountID}`,
value: {
paymentBankAccountID: settlementBankAccountID,
},
},
];

const successData: OnyxUpdate[] = [
{
onyxMethod: Onyx.METHOD.MERGE,
key: `${ONYXKEYS.COLLECTION.PRIVATE_EXPENSIFY_CARD_SETTINGS}${workspaceAccountID}`,
value: {
paymentBankAccountID: settlementBankAccountID,
},
},
];

const failureData: OnyxUpdate[] = [
{
onyxMethod: Onyx.METHOD.MERGE,
key: `${ONYXKEYS.COLLECTION.PRIVATE_EXPENSIFY_CARD_SETTINGS}${workspaceAccountID}`,
value: {
paymentBankAccountID: currentSettlementBankAccountID,
},
},
];

const parameters = {
domainName,
settlementBankAccountID,
};

// TODO: uncomment this code when the API is ready
// const optimisticData: OnyxUpdate[] = [
// {
// onyxMethod: Onyx.METHOD.MERGE,
// key: `${ONYXKEYS.COLLECTION.PRIVATE_EXPENSIFY_CARD_SETTINGS}${workspaceAccountID}`,
// value: {
// paymentBankAccountID: accountID,
// },
// },
// ];
//
// const successData: OnyxUpdate[] = [
// {
// onyxMethod: Onyx.METHOD.MERGE,
// key: `${ONYXKEYS.COLLECTION.PRIVATE_EXPENSIFY_CARD_SETTINGS}${workspaceAccountID}`,
// value: {
// paymentBankAccountID: accountID,
// },
// },
// ];
//
// const failureData: OnyxUpdate[] = [
// {
// onyxMethod: Onyx.METHOD.MERGE,
// key: `${ONYXKEYS.COLLECTION.PRIVATE_EXPENSIFY_CARD_SETTINGS}${workspaceAccountID}`,
// value: {
// paymentBankAccountID: null,
// },
// },
// ];
//
// const parameters = {
// workspaceAccountID,
// settlementBankAccountID: accountID,
// };
//
// API.write(WRITE_COMMANDS.UPDATE_CARD_SETTLEMENT_ACCOUNT, parameters, {optimisticData, successData, failureData});
API.write(WRITE_COMMANDS.UPDATE_CARD_SETTLEMENT_ACCOUNT, parameters, {optimisticData, successData, failureData});
}

function setIssueNewCardStepAndData({data, isEditing, step}: IssueNewCardFlowData) {
Expand Down
5 changes: 5 additions & 0 deletions src/pages/workspace/expensifyCard/WorkspaceExpensifyCardBankAccounts.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@ import useLocalize from '@hooks/useLocalize';
import useThemeStyles from '@hooks/useThemeStyles';
import {getLastFourDigits} from '@libs/BankAccountUtils';
import * as CardUtils from '@libs/CardUtils';
import * as PolicyUtils from '@libs/PolicyUtils';
import Navigation from '@navigation/Navigation';
import type {SettingsNavigatorParamList} from '@navigation/types';
import * as Card from '@userActions/Card';
Expand All @@ -30,12 +31,16 @@ function WorkspaceExpensifyCardBankAccounts({route}: WorkspaceExpensifyCardBankA

const policyID = route?.params?.policyID ?? '-1';

const workspaceAccountID = PolicyUtils.getWorkspaceAccountID(policyID);

const handleAddBankAccount = () => {
Navigation.navigate(ROUTES.BANK_ACCOUNT_WITH_STEP_TO_OPEN.getRoute('new', policyID, ROUTES.WORKSPACE_EXPENSIFY_CARD.getRoute(policyID)));
};

const handleSelectBankAccount = (value?: number) => {
Card.configureExpensifyCardsForPolicy(policyID, value);
const domainName = PolicyUtils.getDomainNameForPolicy(policyID);
Card.updateSettlementAccount(workspaceAccountID, domainName, value);
Navigation.navigate(ROUTES.WORKSPACE_EXPENSIFY_CARD_ISSUE_NEW.getRoute(policyID));
};

Expand Down
3 changes: 2 additions & 1 deletion src/pages/workspace/expensifyCard/WorkspaceSettlementAccountPage.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -73,7 +73,8 @@ function WorkspaceSettlementAccountPage({route}: WorkspaceSettlementAccountPageP
}, [eligibleBankAccounts, paymentBankAccountID, styles, translate]);

const updateSettlementAccount = (value: number) => {
Card.updateSettlementAccount(workspaceAccountID, value);
const domainName = PolicyUtils.getDomainNameForPolicy(policyID);
Card.updateSettlementAccount(workspaceAccountID, domainName, value, paymentBankAccountID);
Navigation.goBack();
};

Expand Down
5 changes: 2 additions & 3 deletions src/pages/workspace/expensifyCard/WorkspaceSettlementFrequencyPage.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -29,8 +29,7 @@ function WorkspaceSettlementFrequencyPage({route}: WorkspaceSettlementFrequencyP

const [cardSettings] = useOnyx(`${ONYXKEYS.COLLECTION.PRIVATE_EXPENSIFY_CARD_SETTINGS}${workspaceAccountID}`);

// TODO: change true for false after API is ready - true is for testing purposes
const shouldShowMonthlyOption = cardSettings?.isMonthlySettlementAllowed ?? true;
const shouldShowMonthlyOption = cardSettings?.isMonthlySettlementAllowed ?? false;
const selectedFrequency = cardSettings?.monthlySettlementDate ? CONST.EXPENSIFY_CARD.FREQUENCY_SETTING.MONTHLY : CONST.EXPENSIFY_CARD.FREQUENCY_SETTING.DAILY;

const data = useMemo(() => {
Expand All @@ -56,7 +55,7 @@ function WorkspaceSettlementFrequencyPage({route}: WorkspaceSettlementFrequencyP
}, [translate, shouldShowMonthlyOption, selectedFrequency]);

const updateSettlementFrequency = (value: ValueOf<typeof CONST.EXPENSIFY_CARD.FREQUENCY_SETTING>) => {
Card.updateSettlementFrequency(workspaceAccountID, value);
Card.updateSettlementFrequency(workspaceAccountID, value, cardSettings?.monthlySettlementDate);
};

return (
Expand Down
Loading