Skip to content

feat(purchase-order): cover purchase_order_number payloads - #95

Open
tiagolupepic wants to merge 5 commits into
mainfrom
pr-554-purchase-order-number
Open

feat(purchase-order): cover purchase_order_number payloads#95
tiagolupepic wants to merge 5 commits into
mainfrom
pr-554-purchase-order-number

Conversation

@tiagolupepic

Copy link
Copy Markdown
Contributor

Add purchase order number support

  • Subscription
  • Wallets and Wallet Recurring rules
  • Wallet Transactions

Comment thread dev_deps.ts Outdated

@ancorcruz ancorcruz left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Direction looks right, and the mock_fetch removal is worth merging on its own — deno task test is currently broken on main.

Three things before merge, details inline:

  1. The satisfies X & { purchase_order_number: string } intersections aren't needed — the current spec already types purchase_order_number?: string | null on every payload touched here — and they make the fixtures unable to fail, which defeats the point of adding them.
  2. tests/wallet.test.ts:61-62 adds two new type errors (credits are string in WalletRecurringTransactionRule). Measured against a freshly generated client: 77 errors on main → 79 here.
  3. Worth being explicit that none of this runs in CI: .github/workflows/ci.yml runs deno task build + deno task typecheck, and typecheck only covers mod.ts, webhook_types.ts and tests/webhook_types.test.ts. deno task test isn't wired in, and the suite fails type-check with 77 pre-existing errors (stale names WalletInputWalletCreateInput, WalletsWalletsPaginated, SubscriptionsSubscriptionsPaginated, plus number-vs-string amounts). So as merged these assertions are documentation, not a safety net. This PR clears the biggest blocker — a follow-up that fixes the fixture drift and adds deno task test to CI is where the value actually lands.

Two coverage gaps vs the description, if you want them in scope:

  • WalletUpdateInput.wallet.recurring_transaction_rules[].purchase_order_number exists in the spec but isn't covered (only the top-level field is in walletUpdateInput).
  • Nothing exercises purchase_order_number: null, which the spec allows.

Comment thread tests/subscription.test.ts Outdated
Comment thread tests/wallet.test.ts Outdated
Comment thread tests/wallet.test.ts Outdated
Comment thread tests/wallet.test.ts Outdated
Comment thread tests/wallet.test.ts Outdated
Comment thread tests/wallet_transaction.test.ts Outdated
Comment thread tests/utils.ts Outdated
Comment thread tests/utils.ts Outdated
Comment thread tests/utils.ts Outdated
Comment thread tests/utils.ts
tiagolupepic and others added 3 commits August 26, 2026 13:08
- Drop `satisfies X & { ... }` intersections that made purchase_order_number
  checks unable to fail if the field disappeared
- Fix stale generated type names (WalletInput, Wallets, WalletTransactionInput)
- Fix numeric fixtures that should be strings (paid_credits, granted_credits,
  amount, credit_amount) and fill in required recurring-rule response fields
- Build walletsResponse from the response fixture instead of the input fixture
- Restore expectedBody support in lagoTest with an undefined check instead of
  truthiness, and assert after the client call returns so failures surface as
  normal assertion errors instead of being swallowed by the error path
- Dedupe the hand-rolled mock fetch in rate_limit.test.ts to reuse
  tests/utils.ts's createMockFetch

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
The error path awaited assertRequest but the 200 path didn't, so a body
mismatch there would surface as an unhandled promise rejection instead
of a normal assertion failure.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
@tiagolupepic
tiagolupepic requested a review from ancorcruz August 27, 2026 09:15
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