[MKT-846]:feat/new text for renewal#1958
Conversation
Deploying drive-web with
|
| Latest commit: |
89ce672
|
| Status: | ✅ Deploy successful! |
| Preview URL: | https://7153dae2.drive-web.pages.dev |
| Branch Preview URL: | https://feat-update-texts-for-annual.drive-web.pages.dev |
jaaaaavier
requested review from
a team,
CandelR,
larryrider and
xabg2
as code owners
| describe('Calculating final price of a product', () => { | ||
| it('When there is no coupon, returns base amount multiplied by users', () => { | ||
| expect(getProductAmount(10, 2)).toBe('20'); | ||
| expect(getProductAmount(10, 2)).toBe('20.00'); |
There was a problem hiding this comment.
Do we want to introduce decimals when they are 00? Is this approved from product? I recall last time I had to remove them if they were .00.
| export const formatPrice = (price: number) => { | ||
| const formattedAmount = Number(price.toFixed(2)); | ||
| return Number.isInteger(formattedAmount) ? formattedAmount.toString() : price.toFixed(2); | ||
| const truncated = Math.floor(Number(price.toFixed(8)) * 100) / 100; |
There was a problem hiding this comment.
Why are we using toFixed(8) here? It looks arbitrary and makes the intent unclear.
If the goal is simply truncating to 2 decimals, Math.floor(price * 100) / 100 should be enough.
If this is a workaround for floating-point precision issues, it would be good to document it explicitly or use a more deterministic decimal handling approach.
There was a problem hiding this comment.
Math.floor(price * 100) / 100 alone is unsafe. If price is the result of floating-point arithmetic, you can get values like 0.2999999999999 where * 100 gives 29.9999, and Math.floor then truncates to 29 instead of 30,a wrong result for what we want right now. The toFixed(8) neutralizes that noise by rounding to 8 decimal places first, resolving the imprecision before the floor. The alternative Math.floor(price * 100) / 100 would reproduce that exact bug for computed prices.
|
3 participants
Description
This PR introduces a change to the text displayed in annual plans regarding the renewal rate. Since we now offer a discount on monthly plans, we should display the current price and specify that it renews at price X.
Related Issues
Related Pull Requests
Checklist
Testing Process
Additional Notes