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.

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

[TS migration] Migrate 'PaymentUtils.js' lib to TypeScript #27923

Merged
Merged
Show file tree
Hide file tree
Changes from 4 commits
Commits
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
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
import _ from 'underscore';
import {SvgProps} from 'react-native-svg';
import * as Expensicons from './Expensicons';
import AmericanExpress from '../../../assets/images/bankicons/american-express.svg';
import BankOfAmerica from '../../../assets/images/bankicons/bank-of-america.svg';
Expand All @@ -21,14 +21,16 @@ import USBank from '../../../assets/images/bankicons/us-bank.svg';
import USAA from '../../../assets/images/bankicons/usaa.svg';
import variables from '../../styles/variables';

type BankIcon = {
icon: React.FC<SvgProps>;
iconSize: number;
};

/**
* Returns matching asset icon for bankName
* @param {String} bankName
* @param {Boolean} isCard
* @returns {Object}
*/

function getAssetIcon(bankName, isCard) {
function getAssetIcon(bankName: string, isCard: boolean): React.FC<SvgProps> {
if (bankName.includes('americanexpress')) {
return AmericanExpress;
}
Expand Down Expand Up @@ -106,22 +108,20 @@ function getAssetIcon(bankName, isCard) {

/**
* Returns Bank Icon Object that matches to existing bank icons or default icons
* @param {String} bankName
* @param {Boolean} [isCard = false]
* @returns {Object} Object includes props icon, iconSize only if applicable
*/

export default function getBankIcon(bankName, isCard) {
const bankIcon = {
export default function getBankIcon(bankName: string, isCard = false) {
const bankIcon: BankIcon = {
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
export default function getBankIcon(bankName: string, isCard = false) {
export default function getBankIcon(bankName: string, isCard = false): BankIcon {

icon: isCard ? Expensicons.CreditCard : GenericBank,
iconSize: 0,
};

if (bankName) {
bankIcon.icon = getAssetIcon(bankName.toLowerCase(), isCard);
}

// For default Credit Card icon the icon size should not be set.
if (!_.contains([Expensicons.CreditCard], bankIcon.icon)) {
if (![Expensicons.CreditCard].includes(bankIcon.icon)) {
bankIcon.iconSize = variables.iconSizeExtraLarge;
}

Expand Down
95 changes: 0 additions & 95 deletions src/libs/PaymentUtils.js

This file was deleted.

85 changes: 85 additions & 0 deletions src/libs/PaymentUtils.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,85 @@
import {SvgProps} from 'react-native-svg';
import BankAccountModel from './models/BankAccount';
import getBankIcon from '../components/Icon/BankIcons';
import CONST from '../CONST';
import * as Localize from './Localize';
import Fund from '../types/onyx/Fund';
import BankAccount from '../types/onyx/BankAccount';
Comment on lines +6 to +7
Copy link
Contributor

Choose a reason for hiding this comment

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

Change these types so that BankAccount['accountType'] is of CONST.PAYMENT_METHODS.BANK_ACCOUNT and
Fund['accountType'] is of CONST.PAYMENT_METHODS.DEBIT_CARD


type AccountType = BankAccount['accountType'] | Fund['accountType'];

type PaymentMethod = (BankAccount | Fund) & {
description: string;
icon: React.FC<SvgProps>;
iconSize: number;
};

/**
* Check to see if user has either a debit card or personal bank account added
*/
function hasExpensifyPaymentMethod(fundList: Record<string, Fund>, bankAccountList: Record<string, BankAccount>): boolean {
const validBankAccount = Object.values(bankAccountList).some((bankAccountJSON) => {
const bankAccount = new BankAccountModel(bankAccountJSON);
return bankAccount.isDefaultCredit();
});

// Hide any billing cards that are not P2P debit cards for now because you cannot make them your default method, or delete them
const validDebitCard = Object.values(fundList).some((card) => card?.accountData?.additionalData?.isP2PDebitCard ?? false);

return validBankAccount || validDebitCard;
}

function getPaymentMethodDescription(accountType: AccountType, account: BankAccount['accountData'] | Fund['accountData']): string {
if (account) {
if (accountType === CONST.PAYMENT_METHODS.BANK_ACCOUNT && 'accountNumber' in account) {
return `${Localize.translateLocal('paymentMethodList.accountLastFour')} ${account.accountNumber?.slice(-4)}`;
}
if (accountType === CONST.PAYMENT_METHODS.DEBIT_CARD && 'cardNumber' in account) {
return `${Localize.translateLocal('paymentMethodList.cardLastFour')} ${account.cardNumber?.slice(-4)}`;
}
}
return '';
}

/**
* Get the PaymentMethods list
*/
function formatPaymentMethods(bankAccountList: Record<string, BankAccount>, fundList: Record<string, Fund>): PaymentMethod[] {
const combinedPaymentMethods: PaymentMethod[] = [];

Object.values(bankAccountList).forEach((bankAccount) => {
// Add all bank accounts besides the wallet
if (bankAccount?.accountData?.type === CONST.BANK_ACCOUNT_TYPES.WALLET) {
return;
}

const {icon, iconSize} = getBankIcon(bankAccount?.accountData?.additionalData?.bankName ?? '', false);
combinedPaymentMethods.push({
...bankAccount,
description: getPaymentMethodDescription(bankAccount?.accountType, bankAccount.accountData),
icon,
iconSize,
});
});

Object.values(fundList).forEach((card) => {
const {icon, iconSize} = getBankIcon(card?.accountData?.bank ?? '', true);
combinedPaymentMethods.push({
...card,
description: getPaymentMethodDescription(card?.accountType, card.accountData),
icon,
iconSize,
});
});

return combinedPaymentMethods;
}

function calculateWalletTransferBalanceFee(currentBalance: number, methodType: string): number {
const transferMethodTypeFeeStructure =
methodType === CONST.WALLET.TRANSFER_METHOD_TYPE.INSTANT ? CONST.WALLET.TRANSFER_METHOD_TYPE_FEE.INSTANT : CONST.WALLET.TRANSFER_METHOD_TYPE_FEE.ACH;
const calculateFee = Math.ceil(currentBalance * (transferMethodTypeFeeStructure.RATE / 100));
return Math.max(calculateFee, transferMethodTypeFeeStructure.MINIMUM_FEE);
}

export {hasExpensifyPaymentMethod, getPaymentMethodDescription, formatPaymentMethods, calculateWalletTransferBalanceFee};
4 changes: 3 additions & 1 deletion src/types/onyx/BankAccount.ts
Original file line number Diff line number Diff line change
@@ -1,3 +1,5 @@
import CONST from '../../CONST';

type AdditionalData = {
isP2PDebitCard?: boolean;
beneficialOwners?: string[];
Expand Down Expand Up @@ -49,7 +51,7 @@ type AccountData = {

type BankAccount = {
/** The bank account type */
accountType?: string;
accountType?: typeof CONST.PAYMENT_METHODS.BANK_ACCOUNT;
Copy link
Contributor

Choose a reason for hiding this comment

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

Is this really correct?

Copy link
Contributor

Choose a reason for hiding this comment

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

@Skalakid Please check it once you are back

Copy link
Contributor Author

@Skalakid Skalakid Oct 2, 2023

Choose a reason for hiding this comment

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

I checked the codebase and i can't find any other string that accountType can be. Also I tested the app and all bank accounts are type of CONST.PAYMENT_METHODS.BANK_ACCOUNT. The change was suggested by @blazejkustra here. Błażej do you have any other argument why we shoudn't use string type in accountType ?

Copy link
Contributor

Choose a reason for hiding this comment

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

No, it's just to be precise. Not much benefits out of it 🤷


/** string like 'Account ending in XXXX' */
description?: string;
Expand Down
5 changes: 4 additions & 1 deletion src/types/onyx/Fund.ts
Original file line number Diff line number Diff line change
@@ -1,3 +1,5 @@
import CONST from '../../CONST';

type AdditionalData = {
isBillingCard?: boolean;
isP2PDebitCard?: boolean;
Expand All @@ -18,11 +20,12 @@ type AccountData = {
created?: string;
currency?: string;
fundID?: number;
bank?: string;
};

type Fund = {
accountData?: AccountData;
accountType?: string;
accountType?: typeof CONST.PAYMENT_METHODS.DEBIT_CARD;
Copy link
Contributor

Choose a reason for hiding this comment

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

Is this really correct?

Copy link
Contributor

Choose a reason for hiding this comment

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

@Skalakid Please check it once you are back

description?: string;
key?: string;
methodID?: number;
Expand Down
Loading