Repository navigation
fix: send exact decimal amounts to Apple Pay and Google Pay - #32
Conversation
- `PaymentItem.amount` is now `PaymentAmount` (`number | string`). iOS parses amounts with `NSDecimalNumber(string:)` instead of `Decimal(Double)`, so 19.99 reaches PassKit as 19.99, not 19.98999999999999488. Android parses and sums them as `BigDecimal`. Numbers are read through their shortest round-trip form. - New `normalizePaymentRequest`, used by `createPaymentRequest` and the hook's `startPayment`, rejects NaN, malformed strings, more decimals than the currency allows (ISO 4217) and a negative total before any native call, then sends amounts as decimal strings. Negative line items (discounts) stay allowed. - `calculateTotal`, the hook's `total` and `formatAmount` use exact decimal arithmetic; `formatAmount` accepts strings and a currency. - Native code rejects unparseable amounts and a negative total; Android also rejects a third decimal place instead of rounding. - Docs, README, CHANGELOG and the example app use `formatAmount`. Closes #18
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info
📝 Summary
Merge Risk: ⚪ Minimal · up to
|
| Check name | Status | Explanation | Resolution |
|---|---|---|---|
| Linked Issues check |  |
Issue #18 requires exact decimal handling and validation. The reported normal paths satisfy most requirements: iOS uses decimal parsing, Android uses BigDecimal, JavaScript uses exact totals, and Je… |
Ensure every public JavaScript payment entry point validates and normalizes amounts before native invocation, including raw handler access, or document and enforce an equivalent wrapper. Add automated Swift conversion tests for the `Payment… |
| Docstring Coverage | |
Docstring coverage is 36.17% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 47 functions across 15 files. (9 skipped:… | Write docstrings for the functions missing them to satisfy the coverage threshold. |
-
Check name Status Explanation Title check The title uses the required fix: prefix and accurately describes the main change. At 59 characters, it exceeds the preferred 50-character target but remains concise and descriptive. Description check The description is detailed and directly covers exact decimal handling, validation, native changes, tests, documentation, and known limitations. Out of Scope Changes check The changed native code, JavaScript amount utilities, hooks, tests, generated bindings, example updates, and documentation all support Issue #18. No demonstrated unrelated product behavior is present.…Security Check No medium-, high-, or critical-severity vulnerability is evident in the changed code. The diff adds decimal parsing and validation, exact arithmetic, generated type bridges, and payment-request handli… Full details: Out of Scope Changes check
-
Explanation
The changed native code, JavaScript amount utilities, hooks, tests, generated bindings, example updates, and documentation all support Issue
#18. No demonstrated unrelated product behavior is present. The reported Android execution and sandbox-token checks are test-status notes, not out-of-scope changes.
Full details: Security Check
-
Explanation
No medium-, high-, or critical-severity vulnerability is evident in the changed code. The diff adds decimal parsing and validation, exact arithmetic, generated type bridges, and payment-request handling. Inputs reach no new injection, network, deserialization, filesystem, logging, or authorization sink. JavaScript normalization and the native iOS and Android paths reject malformed amounts and negative totals. The diff adds no dependency or secret-handling change. No prior security findings were supplied.
-
Full details: Linked Issues check
-
Explanation
Issue
#18requires exact decimal handling and validation. The reported normal paths satisfy most requirements: iOS uses decimal parsing, Android usesBigDecimal, JavaScript uses exact totals, and Jest plus Kotlin tests cover the main cases. The requirement is not complete. DirectHybridPaymentHandler.startPaymentcalls can bypass JavaScript normalization, and iOS can accept an over-precise1.005on that path. The issue also requests Swift automated conversion tests, but the PR reports only a scratch script because no XCTest target exists. Manual sandbox-token testing is not a coding requirement for this assessment.Resolution
Ensure every public JavaScript payment entry point validates and normalizes amounts before native invocation, including raw handler access, or document and enforce an equivalent wrapper. Add automated Swift conversion tests for the
PaymentAmountcases when the test target is available.
Full details: Docstring Coverage
-
Explanation
Docstring coverage is 36.17% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 47 functions across 15 files. (9 skipped: 9 unsupported.)
Resolution
Write docstrings for the functions missing them to satisfy the coverage threshold.
- Fix all pre-merge checks with AI
- Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts
Warning
Billing warning: we have not been able to collect payment for this subscription for more than 72 hours. Please update the payment method or pay any pending invoices in Billing to avoid service interruption.
Comment @coderabbitai help to get the list of available commands.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
1 participant
Closes #18
Summary
PaymentItem.amountis nownumber | string(PaymentAmount, a Nitro variant). iOS parses amounts withNSDecimalNumber(string:locale:)instead ofDecimal(Double), so19.99reaches PassKit as19.99, not19.98999999999999488. Android parses and sums them asBigDecimal(newPaymentAmounts.kt). Numbers are read through their shortest round-trip form on both platforms.normalizePaymentRequest(also used bycreatePaymentRequestandusePaymentCheckout().startPayment()) rejectsNaN/Infinity, malformed strings, more decimals than the currency allows (static ISO 4217 table: JPY 0, KWD 3, default 2), and a negative total. It then sends every amount as a decimal string scaled to the currency ('19.90','1500'). Negative line items (discounts) stay allowed. The hook setserrorand returnsnullwithout calling native code, the same way it handles an empty cart.calculateTotal(and the hook'stotal) sum exactly with BigInt and still return anumber, sototal.toFixed(2)keeps working.formatAmount(amount, currencyCode?)accepts strings and rounds half away from zero.buildreported. Android rejects a negativetotalPriceand a third decimal place instead of rounding, and sends whole-number strings (whatnormalizePaymentRequestproduces for JPY) as"1500", not"1500.00". On iOS the check runs before any payment state is stored; on Android the error goes through the existingcatch, which releases the in-progress slot, so a rejected request does not block the next one.types.md(newPaymentAmountsection),utils.md,use-payment-checkout.md,hybrid-payment-handler.md,manual-flow.md,dynamic-cart.md,quick-start.md, README, CHANGELOG, example app.Test plan
cd package && bun run typecheck: passescd package && bunx eslint "src/**/*.{ts,tsx}" --max-warnings 0: passescd package && bun run test:ci: 89/89 pass. New:amounts.test.ts(validator cases, includingNaN,-1,1.005USD,0.1 + 0.2, JPY decimals, KWD 3 decimals), exactcalculateTotal,formatAmountwith strings and currencies, and hook tests asserting that invalid amounts never reachstartPayment, that19.99and'19.99'both cross as'19.99'(with and withoutonAuthorize), and that a rejected cart does not block the next payment../gradlew :gmisoftware_react-native-pay:testDebugUnitTestfromexample/android). NewPaymentAmountsTest(4: number and string parsing, exact sums) and 5 new cases inGooglePayRequestBuilderTest(exacttotalPrice, JPY formatting, a negative discount line, rejection of a negative total and of a third decimal place); its existing cases now build items withPaymentAmount.create(...).swiftscript against a copy of thePaymentAmountenum. No XCTest target exists yet (Add native unit tests and native build jobs to CI #22).19.99→19.99(old:19.98999999999999488),4.99 + "0.50"→5.49(old:5.490000000000001024);NaN,"12abc"," 1.00","1,00","1e3"→ rejected.xcodebuild -workspace example.xcworkspace -scheme example -sdk iphonesimulator CODE_SIGNING_ALLOWED=NO build(afterpod install): succeeds, as does theNitroPayscheme alone, with no warnings in the changed Swift files. The app was not launched (expo run:iosnot run).cd example && bunx expo run:android: not run. The Gradle unit test task compiles the module's Kotlin, but not the C++/JNI glue (the local CMake configure step fails for environment reasons unrelated to this change).bun run docs:build: passes, no broken links or anchorstransactionAmount: not run, needs a merchant certificateRisk
PaymentItem.amountis nownumber | string, so consumer code that readsitem.amountas a number (item.amount.toFixed(2)) no longer compiles. UseformatAmount(item.amount). The example and docs are updated.1.1 * 3=3.3000000000000003) or with amounts more precise than the currency now fail with an error instead of being rounded silently.createPaymentRequestnow throws for invalid amounts and returns string amounts.HybridPaymentHandler.startPaymentcalls are not validated in JS unless the caller usesnormalizePaymentRequest/createPaymentRequest. Native still rejects malformed amounts and negative totals, but on iOS a1.005USD amount passes through.getPaymentHandler()(fix: guard concurrent startPayment calls on Android and in the hook #29) returns the raw hybrid object, so validating direct callers automatically needs a JS wrapper; left as a follow-up.VariantPaymentAmount), so a native rebuild (pod install/ Gradle sync) is needed.JPaymentAmount.cpp/.hpp,JPaymentItem.hpp,NitroPay+autolinking.cmake) is unmodified Nitrogen output but was not built here. The shared C++PaymentItem.hppvariant is compiled by the iOS build.Notes
numberamounts (unlike the issue's suggestion): numbers are exact for any literal amount, and the precision check catches float arithmetic noise, so a warning would fire on every docs example without improving correctness.DisplayItem.price, and PassKit allows negative summary items); only a negative total is rejected. A single-1item is therefore still rejected.Errors with descriptive messages. Typed configuration errors are Typed payment status and error codes; stop treating cancellation as an error #15.amountMinor) are not added; decimal strings cover server-provided amounts. Possible follow-up.Need help on this PR? Tag
@codesmith-botwith what you need.