feat(purchase-order): cover purchase_order_number payloads by tiagolupepic · Pull Request #95 · getlago/lago-javascript-client

tiagolupepic · 2026-07-31T09:22:19Z

Add purchase order number support

Subscription
Wallets and Wallet Recurring rules
Wallet Transactions

tiagolupepic · 2026-07-31T09:33:49Z

@@ -1,2 +1 @@
 export { assertEquals } from "https://deno.land/std@0.196.0/assert/mod.ts";
-export * as mf from "https://deno.land/x/mock_fetch@0.3.0/mod.ts";


Removed because the mock package is not available anymore

error: Uncaught (in promise) 'Module not found "https://crux.land/router@0.0.5".\n at https://deno.land/x/mock_fetch@0.3.0/mod.ts:1:46'

ancorcruz

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:

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.
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.
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 WalletInput → WalletCreateInput, Wallets → WalletsPaginated, Subscriptions → SubscriptionsPaginated, 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.

ancorcruz · 2026-07-31T11:42:53Z

+} satisfies SubscriptionCreateInput & {
+  subscription: { purchase_order_number: string };
+};


Why is the & { subscription: { purchase_order_number: string } } needed here? Does it quitely removes the check we want here?

I regenerated the client from the current spec — purchase_order_number?: string | null is already on SubscriptionCreateInput, SubscriptionUpdateInput, SubscriptionObject, WalletCreateInput, WalletUpdateInput, WalletObject, WalletRecurringTransactionRule, WalletTransactionCreateInput and WalletTransactionObject. This compiles with no intersection:

const wt = { wallet_transaction: { wallet_id: "1", paid_credits: "100", purchase_order_number: "PO-123" }, } satisfies WalletTransactionCreateInput; // ✅

By widening the target type, the fixture keeps type-checking even if the generated client ever loses the field — which is exactly the regression these fixtures should catch. It also pins the field as required string, where the spec says optional string | null.

Suggest dropping every & { ... } in this PR and going back to plain satisfies X (as const can come back too — it was only dropped because readonly arrays don't satisfy Array<{...}>).

ancorcruz · 2026-07-31T11:43:51Z

+        "paid_credits": 100,
+        "granted_credits": 10,


WalletRecurringTransactionRule declares these as strings, so the response fixture needs "100" /
"10":

Suggested change

"paid_credits": 100,

"granted_credits": 10,

"paid_credits": "100",

"granted_credits": "10",

These two lines are also the only new type errors this PR introduces — I ran deno test ./tests against a freshly generated client on both branches: 77 errors on main, 79 here, and the delta is exactly this.

The rule object is also missing fields the response type marks required: status, threshold_credits, grants_target_top_up, started_at, target_ongoing_balance, created_at, expiration_at, invoice_requires_successful_payment, transaction_name, ignore_paid_top_up_limits.

ancorcruz · 2026-07-31T11:44:29Z

+        "paid_credits": 100,
+        "granted_credits": 10,


Same here — the create payload types these as string | null in the spec (paid_credits / granted_credits, both top-level and inside recurring_transaction_rules), so this should be "100" / "10".

It doesn't error today only because the WalletInput import on line 3 no longer resolves (the type was renamed to WalletCreateInput), which makes the whole satisfies on line 30 a no-op. Once that import is fixed, this line and the existing rate_amount: 2 / paid_credits: 500 above will all surface.

ancorcruz · 2026-07-31T11:44:51Z

+} satisfies WalletInput & {
+  wallet: {
+    purchase_order_number: string;
+    recurring_transaction_rules: Array<{ purchase_order_number: string }>;
+  };
+};


Same as the subscription fixture — the intersection isn't needed (WalletCreateInput already has purchase_order_number, including inside recurring_transaction_rules) and it turns the type assertion into something that can't fail.

Worth noting this satisfies is currently inert anyway: WalletInput isn't exported by the generated client anymore (it's WalletCreateInput now; Wallets → WalletsPaginated), so TS resolves it to an error type and checks nothing.

ancorcruz · 2026-07-31T11:46:06Z

+};

 const walletsResponse = {
  wallets: [walletInput.wallet],


walletsResponse is built from the input fixture, so it now carries a recurring rule with no lago_id / status and numeric credits — none of which a real list response would return. Using the response fixture instead would keep it honest:

Suggested change

wallets: [walletInput.wallet],

wallets: [walletResponse.wallet],

ancorcruz · 2026-07-31T11:46:26Z

+} satisfies WalletTransactionInput & {
+  wallet_transaction: { purchase_order_number: string };
+};


Same intersection point as the other two fixtures — WalletTransactionCreateInput already declares purchase_order_number?: string | null, so plain satisfies covers it and actually fails if the field disappears.

(Heads-up while you're here: WalletTransaction / WalletTransactionInput are stale names too — the generated client exports WalletTransactionCreateInput and WalletTransactionObject now.)

ancorcruz · 2026-07-31T11:50:51Z

+      if (expectedBody) {
+        assertEquals(await _req.json(), expectedBody);
+      }


Suggested change

if (expectedBody) {

assertEquals(await _req.json(), expectedBody);

}

if (expectedBody !== undefined) {

assertEquals(await _req.json(), expectedBody);

}

Truthiness skips a null / "" / 0 expectation silently. Also, if a test sets expectedBody for a request that carries no body, _req.json() throws a fairly opaque SyntaxError — a request.body guard (or an explicit "expected a body" assertion) would fail more clearly.

ancorcruz · 2026-07-31T11:51:46Z

+    assertEquals(request.method, method);
+    assertEquals(url.pathname, path);


Asserting from inside customFetch means failures travel back through the client's error path. In testType: "error" tests they land in the catch on line 103 → getLagoError → throw new Error(error), which stringifies the AssertionError and loses the diff output.

Alternative: capture the request in the handler and assert after the client call returns, so mismatches show up as normal assertion failures rather than as request errors.

ancorcruz · 2026-07-31T11:52:14Z

+type MatchHandler = (request: Request) => Response | Promise<Response>;

-  mock(route, handler);
+export function setupMockClient(route: string, handler: MatchHandler) {


route is typed as plain string here while lagoTest has the precise `${"POST"|"GET"|"PUT"|"DELETE"}@/api/v1/${string}` type on line 64. Worth extracting that into a type LagoRoute and using it in both — right now split("@") will happily hand back path === undefined for a malformed route, and every request then fails on a confusing pathname assertion.

ancorcruz · 2026-07-31T11:54:10Z


-export function setupMockClient(route: string, handler: mf.MatchHandler) {
-  const { fetch, mock } = mf.sandbox();
+type MatchHandler = (request: Request) => Response | Promise<Response>;


tests/rate_limit.test.ts:14 already has a local createMockFetch with the comment "replaces broken mock_fetch library". Now there are two hand-rolled mocks in the suite — worth exporting one from here and deleting the other.

tiagolupepic added 2 commits July 31, 2026 11:19

feat(purchase-order): cover purchase_order_number payloads

a694afe

fix(test): replace mock_fetch dependency

2781e4a

tiagolupepic commented Jul 31, 2026

View reviewed changes

ancorcruz reviewed Jul 31, 2026

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

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

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

tiagolupepic commented Jul 31, 2026

Uh oh!

tiagolupepic Jul 31, 2026

Uh oh!

ancorcruz left a comment

Uh oh!

ancorcruz Jul 31, 2026

Uh oh!

ancorcruz Jul 31, 2026

Uh oh!

ancorcruz Jul 31, 2026

Uh oh!

ancorcruz Jul 31, 2026

Uh oh!

ancorcruz Jul 31, 2026

Uh oh!

ancorcruz Jul 31, 2026

Uh oh!

ancorcruz Jul 31, 2026

Uh oh!

ancorcruz Jul 31, 2026

Uh oh!

ancorcruz Jul 31, 2026

Uh oh!

ancorcruz Jul 31, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

		@@ -1,2 +1 @@
		export { assertEquals } from "https://deno.land/std@0.196.0/assert/mod.ts";
		export * as mf from "https://deno.land/x/mock_fetch@0.3.0/mod.ts";

	wallets: [walletInput.wallet],
	wallets: [walletResponse.wallet],

		assertEquals(request.method, method);
		assertEquals(url.pathname, path);

Uh oh!

Conversation

tiagolupepic commented Jul 31, 2026

Uh oh!

Choose a reason for hiding this comment

Uh oh!

ancorcruz left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants