Skip to content

fix(hitpay): respect zero-decimal currencies when charging and storing - #30039

Open
shafi-VM wants to merge 1 commit into
calcom:mainfrom
shafi-VM:fix/hitpay-zero-decimal-currencies
Open

fix(hitpay): respect zero-decimal currencies when charging and storing#30039
shafi-VM wants to merge 1 commit into
calcom:mainfrom
shafi-VM:fix/hitpay-zero-decimal-currencies

Conversation

@shafi-VM

Copy link
Copy Markdown
Contributor

What does this PR do?

HitPay's payment-requests API takes amount as a decimal string in major units and echoes it
back the same way, so the integration converted in both directions — but unconditionally:

amount: payment.amount / 100,                                  // outbound  (:109)
...
amount: parseFloat(data.amount.replace(/,/g, "")) * 100,       // inbound   (:156)

The price input writes the amount through convertToSmallestCurrencyUnit, which stores
zero-decimal currencies unscaled. HitPay offers three of them — jpy, krw, vnd
(components/constants.ts) — so for those the outbound /100 undercharges by 100×: a ¥10,000
event 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 * 100 turns it into 10000, and
the Payment row 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:

Sent to HitPay Stored Booker sees
Current 100 10000 ¥10,000
Outbound fix only 10000 1000000 ¥1,000,000
This PR (both) 10000 10000 ✅ ¥10,000

Both directions now use the shared helpers in @calcom/lib/currencyConversions — the same module
the 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:

AssertionError: expected { sent: '100',  stored: 10000  } to deeply equal { sent: '10000',  stored: 10000  }
AssertionError: expected { sent: '500',  stored: 50000  } to deeply equal { sent: '50000',  stored: 50000  }
AssertionError: expected { sent: '2000', stored: 200000 } to deeply equal { sent: '200000', stored: 200000 }

 Tests  3 failed | 2 passed (5)

Note stored is correct in both columns — that is the masking effect, captured as a test.

After (this PR):

 ✓ packages/app-store/hitpay/lib/PaymentService.test.ts (5 tests)

 Test Files  1 passed (1)
      Tests  5 passed (5)

Video Demo (if applicable):

N/A — no UI change.

Image Demo (if applicable):

N/A — no UI change.

Mandatory Tasks (DO NOT REMOVE)

  • I have self-reviewed the code (A decent size PR without self-review might be rejected).
  • I have updated the developer docs if this PR makes changes that would require a documentation change. If N/A, write N/A here and check the checkbox. — N/A, internal currency conversion with no documented API change.
  • I confirm automated tests are in place that prove my fix is effective or that my feature works.

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:

TZ=UTC yarn vitest run packages/app-store/hitpay/lib/PaymentService.test.ts

To see it fail without the fix, restore the two unconditional conversions in PaymentService.ts
and re-run.

To verify manually against HitPay sandbox:

  • Minimal test data: an event type with the HitPay app enabled, currency Japanese yen (JPY),
    price 10000.
  • Expected (happy path): the HitPay checkout requests ¥10,000, matching the booking page and
    the Payment row. Before this change it requested ¥100 while both still displayed ¥10,000.
  • Repeat with USD at price 50 to confirm unchanged behaviour: HitPay should request 50.
  • krw and vnd behave like jpy; the other 18 currencies in the picker behave like usd.

Checklist

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.
@github-actions

Copy link
Copy Markdown
Contributor

Welcome to Cal.diy, @shafi-VM! Thanks for opening this pull request.

A few things to keep in mind:

  • This is Cal.diy, not Cal.com. Cal.diy is a community-driven, fully open-source fork of Cal.com licensed under MIT. Your changes here will be part of Cal.diy — they will not be deployed to the Cal.com production app.
  • Please review our Contributing Guidelines if you haven't already.
  • Make sure your PR title follows the Conventional Commits format.

A maintainer will review your PR soon. Thanks for contributing!

@github-actions github-actions Bot added the 🐛 bug Something isn't working label Aug 26, 2026
@coderabbitai

coderabbitai Bot commented Aug 26, 2026

Copy link
Copy Markdown
Contributor

Review Change Stack

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro Plus

Run ID: 483d1611-a8f9-4c12-b1dc-9c3667709a6c

📥 Commits

Reviewing files that changed from the base of the PR and between 176037d and 1b8922f.

📒 Files selected for processing (2)
  • packages/app-store/hitpay/lib/PaymentService.test.ts
  • packages/app-store/hitpay/lib/PaymentService.ts

Included review availability: Your plan provides up to 10 included reviews per hour; 9 remain after this review.


📝 Walkthrough

Walkthrough

HitPay 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 1b892

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)
Check name Status Explanation
Title check ✅ Passed The title clearly identifies the HitPay fix and the zero-decimal currency behavior it changes.
Description check ✅ Passed The description directly explains the HitPay conversion bug, the affected currencies, the implementation, and the tests.
Linked Issues check ✅ Passed The changes satisfy issue #30038. Both conversion directions use the shared currency helpers, zero-decimal currencies remain unscaled, minor-unit currencies retain existing behavior, and tests cover J…
Out of Scope Changes check ✅ Passed The changes are limited to HitPay amount conversion logic and focused tests. No unrelated code changes are present.
Docstring Coverage ✅ Passed 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…
Full details: Linked Issues check

Explanation

The changes satisfy issue #30038. Both conversion directions use the shared currency helpers, zero-decimal currencies remain unscaled, minor-unit currencies retain existing behavior, and tests cover JPY, KRW, VND, USD, and SGD.

Full details: Docstring Coverage

Explanation

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 💡
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@bandhan-majumder bandhan-majumder self-assigned this Aug 26, 2026

@bandhan-majumder bandhan-majumder left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

LGTM

@bandhan-majumder bandhan-majumder added ready-for-e2e run-ci Approve CI to run for external contributors labels Aug 26, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

🐛 bug Something isn't working ready-for-e2e run-ci Approve CI to run for external contributors size/L

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[bug]: HitPay charges JPY, KRW and VND bookings 1/100th of the configured price

2 participants