fix(hitpay): respect zero-decimal currencies when charging and storing - #30039
fix(hitpay): respect zero-decimal currencies when charging and storing#30039shafi-VM wants to merge 1 commit into
Conversation
HitPay's payment-requests API takes `amount` as a decimal string in major units, so the integration divided by 100 on the way out and multiplied by 100 on the way back in. Both conversions were unconditional, but the price input writes the amount through `convertToSmallestCurrencyUnit`, which stores zero-decimal currencies unscaled. For the three zero-decimal currencies HitPay offers — JPY, KRW and VND — a ¥10,000 event therefore asked HitPay to collect ¥100. The two unconditional conversions cancelled out in the database, so the Payment row and the booker's page both still showed ¥10,000 while only 1% was charged, and `refund()` is not implemented for this app. Route both directions through the shared helpers so the amount charged matches the amount stored and displayed. Currencies with a minor unit are unaffected; the added tests pin USD and SGD to their existing behaviour.
|
Welcome to Cal.diy, @shafi-VM! Thanks for opening this pull request. A few things to keep in mind:
A maintainer will review your PR soon. Thanks for contributing! |
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (2)
Included review availability: Your plan provides up to 10 included reviews per hour; 9 remain after this review. 📝 WalkthroughWalkthroughHitPay payment amounts now use shared currency conversion helpers. Requests no longer always divide stored amounts by 100. Persisted amounts no longer always multiply payment amounts by 100. Tests cover unchanged amounts for JPY, KRW, and VND, and minor-unit conversion for USD and SGD. Merge Risk: ⚪ Minimal · up to This localized change makes HitPay charging and stored amounts consistent for zero-decimal currencies and adds focused coverage; no actionable merge-blocking risk remains beyond normal checks. 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
Full details: Linked Issues checkExplanation The changes satisfy issue Full details: Docstring CoverageExplanation No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check. Docstring coverage is scoped to functions touched by this diff. Analyzed 0 functions across 2 files. ✨ Finishing Touches 💡 1🛠️ Fix failing CI checks 💡
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
What does this PR do?
HitPay's payment-requests API takes
amountas a decimal string in major units and echoes itback the same way, so the integration converted in both directions — but unconditionally:
The price input writes the amount through
convertToSmallestCurrencyUnit, which storeszero-decimal currencies unscaled. HitPay offers three of them —
jpy,krw,vnd(
components/constants.ts) — so for those the outbound/100undercharges by 100×: a ¥10,000event asks HitPay to collect ¥100.
Why this went unnoticed: the two unconditional conversions cancel each other out in the
database. HitPay echoes back the 100 it was asked for, the inbound
* 100turns it into 10000, andthe
Paymentrow plus the booker's page both display ¥10,000 — while only 1% was actually charged.refund()throws "Method not implemented" for this app, so there is no in-app correction either.Both lines have to change together. Fixing only the outbound conversion sends the right amount
but then stores
1000000, i.e. swaps a silent undercharge for a visible 100× overstatement:100❌10000✅10000✅Both directions now use the shared helpers in
@calcom/lib/currencyConversions— the same modulethe price input already uses, so storage, charge and display agree by construction.
Same class of bug as #29983, but an independent one in a different provider and file.
Visual Demo (For contributors especially)
No UI surface: the amount is computed server-side in the outgoing HitPay request, and because the
two conversions currently cancel out, the booking page renders the correct price both before and
after. Screenshots would be identical and prove nothing — the meaningful evidence is the outgoing
request body and the persisted amount.
Before (fix reverted, tests in place) — the three zero-decimal currencies fail, the rest pass:
Note
storedis correct in both columns — that is the masking effect, captured as a test.After (this PR):
Video Demo (if applicable):
N/A — no UI change.
Image Demo (if applicable):
N/A — no UI change.
Mandatory Tasks (DO NOT REMOVE)
How should this be tested?
No HitPay credentials or environment variables are needed — the test mocks the HTTP calls and
echoes back whatever amount was requested, the way HitPay does:
To see it fail without the fix, restore the two unconditional conversions in
PaymentService.tsand re-run.
To verify manually against HitPay sandbox:
price 10000.
the
Paymentrow. Before this change it requested ¥100 while both still displayed ¥10,000.50.krwandvndbehave likejpy; the other 18 currencies in the picker behave likeusd.Checklist