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

build: upgrade and migrate to Dinero.js v2 #1030

Closed
wants to merge 3 commits into from
Closed

build: upgrade and migrate to Dinero.js v2 #1030

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
1d32684
build: upgrade dinero.js to v2
sarahdayan Jul 22, 2021
9f5ef96
build: install Dinero.js currencies
sarahdayan Jul 22, 2021
0c6c8d3
refactor: migrate codebase to Dinero.js v2
sarahdayan Jul 22, 2021
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
20 changes: 10 additions & 10 deletions cypress/tests/ui/new-transaction.spec.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
import Dinero from "dinero.js";
import { dineroUSD, format } from "utils/transactionUtils";
import { User } from "../../../src/models";
import { isMobile } from "../../support/utils";

Expand Down Expand Up @@ -59,9 +59,9 @@ describe("New Transaction", function () {
.should("be.visible")
.and("have.text", "Transaction Submitted!");

const updatedAccountBalance = Dinero({
amount: ctx.user!.balance - parseInt(payment.amount) * 100,
}).toFormat();
const updatedAccountBalance = format(
dineroUSD(ctx.user!.balance - parseInt(payment.amount) * 100)
);

if (isMobile()) {
cy.getBySel("sidenav-toggle").click();
Expand Down Expand Up @@ -179,9 +179,9 @@ describe("New Transaction", function () {

cy.switchUserByXstate(ctx.contact!.username);

const updatedAccountBalance = Dinero({
amount: ctx.contact!.balance + transactionPayload.amount * 100,
}).toFormat();
const updatedAccountBalance = format(
dineroUSD(ctx.contact!.balance + transactionPayload.amount * 100)
);

if (isMobile()) {
cy.getBySel("sidenav-toggle").click();
Expand Down Expand Up @@ -230,9 +230,9 @@ describe("New Transaction", function () {

cy.switchUserByXstate(ctx.user!.username);

const updatedAccountBalance = Dinero({
amount: ctx.user!.balance + transactionPayload.amount * 100,
}).toFormat();
const updatedAccountBalance = format(
dineroUSD(ctx.user!.balance + transactionPayload.amount * 100)
);

if (isMobile()) {
cy.getBySel("sidenav-toggle").click();
Expand Down
15 changes: 4 additions & 11 deletions cypress/tests/ui/transaction-feeds.spec.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,4 +1,3 @@
import Dinero from "dinero.js";
import {
User,
Transaction,
Expand All @@ -8,7 +7,7 @@ import {
TransactionStatus,
} from "../../../src/models";
import { addDays, isWithinInterval, startOfDay } from "date-fns";
import { startOfDayUTC, endOfDayUTC } from "../../../src/utils/transactionUtils";
import { startOfDayUTC, endOfDayUTC, dineroUSD, format } from "../../../src/utils/transactionUtils";
import { isMobile } from "../../support/utils";

const { _ } = Cypress;
Expand Down Expand Up @@ -112,9 +111,7 @@ describe("Transaction Feed", function () {
cy.log("🚩Testing a paid payment transaction item");
cy.contains("[data-test*='transaction-item']", "paid").within(($el) => {
const transaction = getTransactionFromEl($el);
const formattedAmount = Dinero({
amount: transaction.amount,
}).toFormat();
const formattedAmount = format(dineroUSD(transaction.amount));

expect([TransactionStatus.pending, TransactionStatus.complete]).to.include(
transaction.status
Expand All @@ -136,9 +133,7 @@ describe("Transaction Feed", function () {
cy.log("🚩Testing a charged payment transaction item");
cy.contains("[data-test*='transaction-item']", "charged").within(($el) => {
const transaction = getTransactionFromEl($el);
const formattedAmount = Dinero({
amount: transaction.amount,
}).toFormat();
const formattedAmount = format(dineroUSD(transaction.amount));

expect(TransactionStatus.complete).to.equal(transaction.status);

Expand All @@ -152,9 +147,7 @@ describe("Transaction Feed", function () {
cy.log("🚩Testing a requested payment transaction item");
cy.contains("[data-test*='transaction-item']", "requested").within(($el) => {
const transaction = getTransactionFromEl($el);
const formattedAmount = Dinero({
amount: transaction.amount,
}).toFormat();
const formattedAmount = format(dineroUSD(transaction.amount));

expect([TransactionStatus.pending, TransactionStatus.complete]).to.include(
transaction.status
Expand Down
3 changes: 2 additions & 1 deletion package.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@
"dependencies": {
"@auth0/auth0-react": "1.5.0",
"@aws-amplify/ui-react": "1.2.4",
"@dinero.js/currencies": "2.0.0-alpha.6",
"@material-ui/core": "4.11.3",
"@material-ui/icons": "4.11.2",
"@material-ui/lab": "4.0.0-alpha.58",
Expand All @@ -28,7 +29,7 @@
"axios": "0.21.1",
"clsx": "1.1.1",
"date-fns": "2.22.1",
"dinero.js": "1.8.1",
"dinero.js": "2.0.0-alpha.6",
"formik": "2.2.9",
"history": "4.10.1",
"react": "17.0.2",
Expand Down
56 changes: 40 additions & 16 deletions src/utils/transactionUtils.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,8 @@ import {
CommentNotification,
} from "../models";
import faker from "faker";
import Dinero from "dinero.js";
import { dinero, add, subtract, toFormat, toSnapshot, isPositive, Dinero } from "dinero.js";
import { USD } from "@dinero.js/currencies";
import {
flow,
get,
Expand All @@ -30,6 +31,22 @@ import {
drop,
} from "lodash/fp";

const currencySymbols = {
USD: "$",
};

type CurrencySymbolsIndices = keyof typeof currencySymbols;

export const dineroUSD = (amount: number) => dinero({ amount, currency: USD });

export const format = (dineroObject: Dinero<number>, fractionDigits?: number) =>
Copy link
Contributor

Choose a reason for hiding this comment

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

@sarahdayan I see that Dinero v2 is in alpha and I'd like to provide some feedback around this DX.

While the flexibility for this format method is great, I'm wondering why the need to define such a large callback around a given currency? This DX feels a bit unintuitive and for a currency's "default", I would expect the library to handle this for me. Have you've considered including default helpers such as this or simplifying this piece for v2?

Copy link
Author

Choose a reason for hiding this comment

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

Hey @kevinold and thanks for the feedback!

You're right, the flexibility comes at the expense of simplicity here. I'm currently rethinking this function for several other reasons, and providing a default return value is part of the thinking process.

The main reason why picking a default is difficult is that Dinero also handles non-decimal currencies. While it might make sense to return "USD 10.50" by default for dinero({ amount: 1050, currency: USD }), there's no good option when passing non-decimal objects. More context in this issue.

I've reworked things and now should be able to expose enough information to toFormat to make formatting easier.

toFormat(dineroObject, ({ amount, currency }) => {
const { scale } = toSnapshot(dineroObject);
const symbol = currencySymbols[currency.code as CurrencySymbolsIndices] || currency.code;

return `${symbol}${amount.toFixed(fractionDigits ?? scale)}`;
});

export const isRequestTransaction = (transaction: Transaction) =>
flow(get("requestStatus"), negate(isEmpty))(transaction);

Expand All @@ -52,33 +69,40 @@ export const getFakeAmount = (min: number = 1000, max: number = 50000) =>
parseInt(faker.finance.amount(min, max), 10);

/* istanbul ignore next */
export const formatAmount = (amount: number) => Dinero({ amount }).toFormat();
export const formatAmount = (amount: number) => format(dineroUSD(amount));

/* istanbul ignore next */
export const formatAmountSlider = (amount: number) => Dinero({ amount }).toFormat("$0,0");
export const formatAmountSlider = (amount: number) => format(dineroUSD(amount), 0);

export const payAppDifference = curry((sender: User, transaction: Transaction) =>
Dinero({ amount: get("balance", sender) }).subtract(
Dinero({ amount: get("amount", transaction) })
)
subtract(dineroUSD(get("balance", sender)), dineroUSD(get("amount", transaction)))
);

export const payAppAddition = curry((sender: User, transaction: Transaction) =>
Dinero({ amount: get("balance", sender) }).add(Dinero({ amount: get("amount", transaction) }))
add(dineroUSD(get("balance", sender)), dineroUSD(get("amount", transaction)))
);

export const getChargeAmount = (sender: User, transaction: Transaction) =>
Math.abs(payAppDifference(sender, transaction).getAmount());
export const getChargeAmount = (sender: User, transaction: Transaction) => {
const { amount } = toSnapshot(payAppDifference(sender, transaction));

export const getTransferAmount = curry((sender: User, transaction: Transaction) =>
Math.abs(payAppDifference(sender, transaction).getAmount())
);
return Math.abs(amount);
};

export const getTransferAmount = curry((sender: User, transaction: Transaction) => {
const { amount } = toSnapshot(payAppDifference(sender, transaction));

return Math.abs(amount);
});

export const getPayAppCreditedAmount = (receiver: User, transaction: Transaction) =>
Math.abs(payAppAddition(receiver, transaction).getAmount());
export const getPayAppCreditedAmount = (receiver: User, transaction: Transaction) => {
const { amount } = toSnapshot(payAppAddition(receiver, transaction));

export const hasSufficientFunds = (sender: User, transaction: Transaction) =>
payAppDifference(sender, transaction).isPositive();
return Math.abs(amount);
};

export const hasSufficientFunds = (sender: User, transaction: Transaction) => {
return isPositive(payAppDifference(sender, transaction));
};

/* istanbul ignore next */
export const receiverIsCurrentUser = (currentUser: User, transaction: Transaction) =>
Expand Down
31 changes: 27 additions & 4 deletions yarn.lock
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3715,6 +3715,25 @@
enabled "2.0.x"
kuler "^2.0.0"

"@dinero.js/[email protected]":
version "2.0.0-alpha.6"
resolved "https://registry.yarnpkg.com/@dinero.js/calculator-number/-/calculator-number-2.0.0-alpha.6.tgz#9ed3d733a7914b283e41be05005d2d6512145032"
integrity sha512-+nTAemUcZMYvwYQ4s8xn3SlIQPJtz6UD97rs9BMAK4vXTihFIrxlDDiJIXwUgFsSJ/SfNLaVgYAEVNhCratO/w==
dependencies:
"@dinero.js/core" "2.0.0-alpha.6"

"@dinero.js/[email protected]":
version "2.0.0-alpha.6"
resolved "https://registry.yarnpkg.com/@dinero.js/core/-/core-2.0.0-alpha.6.tgz#d8554033125a9bbc72a3ca98a68363c8b339d293"
integrity sha512-AJxNGi9a9+y1Ow2RR4KAGpWxhrrIgtHFkAQOVJOaWT2Js7TcAEWPgyubE4RBKSN0lgS2sMkeB8FfUbaO7Sphdw==
dependencies:
"@dinero.js/currencies" "2.0.0-alpha.6"

"@dinero.js/[email protected]":
version "2.0.0-alpha.6"
resolved "https://registry.yarnpkg.com/@dinero.js/currencies/-/currencies-2.0.0-alpha.6.tgz#4dc9aea3b32e9eab8cc8bb3a0b3102e9ad876a89"
integrity sha512-MLvmLF7B82ntGT3y/qxWga7VTUi/bmfj0e8RxcpphkmzT2qBtmn0fabqf+bSkpLn5Xbnz2yOXo8LAJHul94XpA==

"@emotion/hash@^0.8.0":
version "0.8.0"
resolved "https://registry.yarnpkg.com/@emotion/hash/-/hash-0.8.0.tgz#bbbff68978fefdbe68ccb533bc8cbe1d1afb5413"
Expand Down Expand Up @@ -8732,10 +8751,14 @@ dijkstrajs@^1.0.1:
resolved "https://registry.yarnpkg.com/dijkstrajs/-/dijkstrajs-1.0.1.tgz#d3cd81221e3ea40742cfcde556d4e99e98ddc71b"
integrity sha1-082BIh4+pAdCz83lVtTpnpjdxxs=

[email protected]:
version "1.8.1"
resolved "https://registry.yarnpkg.com/dinero.js/-/dinero.js-1.8.1.tgz#775a647629b4195af9d02f46e9b7fa1fd81e906d"
integrity sha512-AQ09MDKonkGUrhBZZFx4tPTVcVJuHJ0VEA73LvcBoBB2eQSi1DbapeXj4wnUUpx1hVnPdyev1xPNnNMGy/Au0g==
[email protected]:
version "2.0.0-alpha.6"
resolved "https://registry.yarnpkg.com/dinero.js/-/dinero.js-2.0.0-alpha.6.tgz#eb22e2e7a6e8d7796838b9ad60f0d5a23db1add4"
integrity sha512-rKU2w5AATj7SlPCIAErjuC5iGBW+dM/jes1Sl0loOWlBFIEUbBCcAvk9wLM1/Wg87YbktWdEd+sgTdPxLrA0NA==
dependencies:
"@dinero.js/calculator-number" "2.0.0-alpha.6"
"@dinero.js/core" "2.0.0-alpha.6"
"@dinero.js/currencies" "2.0.0-alpha.6"

dir-glob@^3.0.1:
version "3.0.1"
Expand Down