Skip to content

fix: coerce money fields to integers before API calls - #17

Merged
wzul merged 7 commits into
mainfrom
fix/money-fields-integer-coercion
Aug 31, 2026
Merged

fix: coerce money fields to integers before API calls#17
wzul merged 7 commits into
mainfrom
fix/money-fields-integer-coercion

Conversation

@wzul

@wzul wzul commented Aug 31, 2026

Copy link
Copy Markdown
Collaborator

Summary

CHIP Collect requires money fields (price, discount, total_price_override, capture/refund amounts, totals and overrides) as integers in minor units (sen).

Apps computing sen via floating point (e.g. 0.29 * 10028.999999999999996) were sending fractional JSON numbers, and the API rejects them with:

400 Bad Request
{"purchase":{"products":[{"price":{"message":"A valid integer is required.","code":"invalid"}}]}}

Verified live against the test-mode API:

  • 108 / 108.0 / "108.00" → 201 OK
  • 28.999999999999996 (float noise) → 400 (exactly the merchant-reported error)

This is not a magnitude rule — it is binary-float rounding noise occurring for arbitrary values (any ringgit × 100 whose result is not exactly representable).

Failure modes closed by this PR

SDK version Before After
v1.x-style model path (all 1.x, and v2 model path) fractional float sent verbatim → 400 coerced to nearest integer (within 1e-9) → 201
v2 PurchaseBuilder int $price weak-mode coercion silently truncated sen (28.999…28, wrong amount) rounds within epsilon; genuine fraction (108.5) explodes loudly instead of charging the wrong price

Changes

  • Chip\Support\Money::coerce() — shared helper: numeric within 1e-9 of an integer → that integer (covers float noise, int-valued floats, numeric strings like "108.00"); genuine fractional sen → Chip\Exception\InvalidMoneyValueException; null passes through.
  • Product — coerce price/discount/total_price_override in fromArray() and at jsonSerialize() time (public properties mean v1-style direct assignment still reaches the API clean).
  • PurchaseDetails — coerce total/debt/subtotal_override/total_tax_override/total_discount_override/total_override in both directions.
  • PurchaseBuilder — money params widened to int|float|string, coerced explicitly (no more silent weak-mode truncation).
  • PurchasesResource::capture()/refund() — amount widened + coerced.
  • InvalidMoneyValueException extends ChipApiException so existing catch blocks keep working.
  • Removed pr-summary.yml + scripts/generate_pr_summary.py — the Ollama endpoint now returns 410 Gone and the workflow was overwriting PR bodies with its error output (it corrupted this very PR's description on first push).

Test Plan

  • composer test — 110 tests, 352 assertions OK (15 new in tests/MoneyCoercionTest.php: float noise, numeric strings, genuine fractions, NaN, serialization + builder paths)
  • composer phpstan — level 8, no errors (conditional return type on Money::coerce)
  • composer cs-check — clean
  • Live E2E vs test-mode API: previously-failing manual model path now returns 201, fraction 108.5 rejected with precise message naming the field

wzul added 2 commits August 31, 2026 19:28
CHIP Collect requires money fields (price, discount, total_price_override,
capture/refund amounts, totals and overrides) as integers in minor units.
Apps computing sen via floating point (e.g. 0.29 * 100 = 28.999999999999996)
sent fractional numbers, causing 400 'A valid integer is required.' from the
API. Values within 1e-9 of an integer are now coerced; genuine fractional sen
(e.g. 108.5) throws InvalidMoneyValueException instead of being silently
truncated by PHP's implicit int cast (v2 builder previously sent 28 sen for
28.999...).

- Add Chip\Support\Money::coerce() shared helper with conditional return type
- Add Chip\Exception\InvalidMoneyValueException
- Product: coerce price/discount/total_price_override in fromArray() and at
  jsonSerialize() time (covers direct property assignment, v1-style usage)
- PurchaseDetails: coerce total/debt/overrides in fromArray() and jsonSerialize()
- PurchaseBuilder: accept int|float|string for money params, coerce explicitly
- PurchasesResource::capture()/refund(): accept and coerce amount
- Add MoneyCoercionTest (15 tests): float noise, numeric strings, genuine
  fractions, NaN, serialization and builder paths
@wzul
wzul requested review from amirulazreen and qir4ni August 31, 2026 17:32
wzul added 5 commits August 31, 2026 19:33
The Ollama Cloud API endpoint (api.ollama.com/api/generate) returns 410 Gone,
so the workflow overwrites PR descriptions with its error output instead of a
summary - destroying manually written PR bodies on every push. Remove the
workflow and generator script entirely.
Release workflow extracts notes from the '## [X.Y.Z]' section matching the
tag; prepare 2.1.0 so tagging works right after merge.
Manual process is preferred: the changelog is maintained by hand in the PR
and releases are tagged manually instead of being driven by GitHub Actions.
- actions/checkout v6 -> v7.0.1
- actions/cache v5 -> v6.1.0
- shivammathur/setup-php v2 -> 2.37.2
@wzul
wzul requested a review from LiyanaAmil August 31, 2026 17:51
@wzul
wzul merged commit 434584e into main Aug 31, 2026
7 checks passed
@wzul
wzul deleted the fix/money-fields-integer-coercion branch August 31, 2026 17:52
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants