Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
14 changes: 7 additions & 7 deletions PLAN.md
Original file line number Diff line number Diff line change
Expand Up @@ -25,13 +25,13 @@ ledger.

## Where we are

| Milestone | Status |
| --------------------------------------- | ----------------------------------------------------------------------------------------------------------------------------------------------------------------------------- |
| M1 safety behavior (was PR 1) | Merged as #2020. Also landed the M2 pure modules: `src/shared/payment/money.ts`, `resource-id.ts`, `refund-state.ts`, and `validated-session.ts`. |
| M2 money/resource vocabulary (was PR 2) | Core modules merged inside #2020. Any provider parsing still off those schemas rides with M3 or M4. |
| M3 provider ownership (was PR 3) | In flight. Merged slices so far: #2048 (payment processing core), #2050 (bounded registration delivery). The observation boundary + SumUp callback wiring slice is in review. |
| M11 verifier slice (was PR 13) | Started early in #2056 — the verifier is read-only and parallelizable. |
| M4–M10 and M12–M13 | Not started. |
| Milestone | Status |
| --------------------------------------- | -------------------------------------------------------------------------------------------------------------------------------------------------- |
| M1 safety behavior (was PR 1) | Merged as #2020. Also landed the M2 pure modules: `src/shared/payment/money.ts`, `resource-id.ts`, `refund-state.ts`, and `validated-session.ts`. |
| M2 money/resource vocabulary (was PR 2) | Core modules merged inside #2020. Any provider parsing still off those schemas rides with M3 or M4. |
| M3 provider ownership (was PR 3) | Complete: #2048 (payment processing core), #2050 (bounded registration delivery), #2060 (observation boundary + SumUp callback wiring; F2 closed). |
| M11 verifier slice (was PR 13) | Started early in #2056 — the verifier is read-only and parallelizable. |
| M4–M10 and M12–M13 | Not started. |

Budgets below count `src/` lines only. Observed totals run 4–15x the `src/`
figure once tests, stories, and catalog copy are included (#2020: 714 src lines,
Expand Down
20 changes: 0 additions & 20 deletions TODO.md
Original file line number Diff line number Diff line change
Expand Up @@ -2150,23 +2150,3 @@ promising shape is to give `mutation:audit-equivalents` a way to attempt a
distinguishing input for each entry — or, failing that, an explicit re-audit
stamp so an entry has to be re-confirmed after the file it lives in changes
shape, instead of resting on a proof nobody has re-read since it was written.

## Delete the sumupApi and squareApi alias wrapper exports (from PR #2060)

`src/shared/sumup.ts` still exports `createCheckout`, `refundTransaction`,
`getTransactionStatus`, and `testSumupConnection` as one-line wrappers over the
same names on `sumupApi`, and `src/shared/square.ts` has the same pattern
(`getSquareClient`, `resetSquareClient`, `testSquareConnection`,
`createPaymentLink` and siblings around `square.ts:850`). AGENTS.md's "No alias
exports" rule says to expose the shared mechanism itself: callers should import
the api object and call `sumupApi.createCheckout(...)` directly, which
late-binds through test stubs exactly like the wrappers do. PR #2060 deleted the
one wrapper it had added (`readCheckoutById`) and migrated its callers to
`sumupApi.readCheckoutById(...)`; the pre-existing wrappers were left alone
because the sweep spans Square too and belongs in one dedicated pass. Starting
point: `grep -n "^export const" src/shared/sumup.ts src/shared/square.ts`, then
migrate `src/shared/sumup-provider.ts`, `src/shared/square-provider.ts`,
`src/features/admin/settings-sumup.ts`, and the direct test importers
(`test/shared/sumup/*.test.ts`). The only subtlety: a function handed away at
module load (like `makeCreateCheckoutSession("SumUp", createCheckout, ...)`)
must become a lambda over the api member so test stubbing keeps working.
1 change: 0 additions & 1 deletion scripts/mutation/equivalent-mutants/features.txt
Original file line number Diff line number Diff line change
Expand Up @@ -117,7 +117,6 @@ src/shared/runtime.ts::buildRuntimeInfo.os~05o6squ ?? → || # os is string|u
src/shared/runtime.ts::buildRuntimeInfo.typescriptVersion~14e4wx7 ?? → || # typescriptVersion is string|undefined and falls back to "", so both operators agree
src/shared/runtime.ts::buildRuntimeInfo.userAgent~17udaa2 ?? → || # userAgent is string|undefined and falls back to "", so both operators agree
src/shared/square.ts::createSquareClient.checkout.paymentLinks.create.data.pre_populated_data~0evahy5 ?: → consequent only # the buyer_phone_number ternary spreads {} when the phone is absent; the consequent-only mutant always spreads { buyer_phone_number: undefined }, but JSON.stringify omits undefined values, so the serialized request body is identical in both cases
src/shared/square.ts::squareApi.testSquareConnection.locations~10z1q7q ?? → || # locations is an array or undefined; arrays are truthy and undefined takes [] under both operators
src/ui/client/dom.ts::createButton.button%2eclassName~16pvzvs = → += # createElement returns a new button with className "", so both assignments produce the supplied class string

# Paid-payment processing: private defaults are always supplied by their only
Expand Down
6 changes: 4 additions & 2 deletions src/features/admin/settings-square.ts
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@ import {
} from "#routes/admin/settings-helpers.ts";
import { settings } from "#shared/db/settings.ts";
import { isDemoMode } from "#shared/demo/mode.ts";
import { testSquareConnection } from "#shared/square.ts";
import { squareApi } from "#shared/square.ts";
/* jscpd:ignore-end */
import {
validateSquareAccessToken,
Expand Down Expand Up @@ -42,7 +42,9 @@ export const squareRoutes = defineProviderCredentialsRoute<SquareFields>({
secretField: "square_access_token",
secretRequiredError: t("error.square_token_required"),
successMessage: "Square credentials updated",
testFn: testSquareConnection,
// A lambda, not the member itself: the config is built once at module
// load, and resolving the member per call keeps test stubs live.
testFn: () => squareApi.testSquareConnection(),
Comment thread
coderabbitai[bot] marked this conversation as resolved.
validate: ({ locationId }, secret) => {
if (isDemoMode()) return t("error.square_demo_mode");
if (!locationId) return t("error.square_location_required");
Expand Down
6 changes: 4 additions & 2 deletions src/features/admin/settings-sumup.ts
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@ import { defineProviderCredentialsRoute } from "#routes/admin/settings-helpers.t
import { settings } from "#shared/db/settings.ts";
import { isDemoMode } from "#shared/demo/mode.ts";
import { providerCurrencyBlock } from "#shared/payment-providers.ts";
import { testSumupConnection } from "#shared/sumup.ts";
import { sumupApi } from "#shared/sumup.ts";

/* jscpd:ignore-end */

Expand All @@ -30,7 +30,9 @@ export const sumupRoutes = defineProviderCredentialsRoute<SumupFields>({
secretField: "sumup_api_key",
secretRequiredError: "SumUp API Key is required",
successMessage: "SumUp credentials updated",
testFn: testSumupConnection,
// A lambda, not the member itself: the config is built once at module
// load, and resolving the member per call keeps test stubs live.
testFn: () => sumupApi.testSumupConnection(),
validate: ({ merchantCode }) => {
if (isDemoMode()) return "Cannot configure SumUp in demo mode";
const currencyBlock = providerCurrencyBlock("sumup", settings.currency);
Expand Down
18 changes: 6 additions & 12 deletions src/shared/square-provider.ts
Original file line number Diff line number Diff line change
Expand Up @@ -28,27 +28,21 @@ import type {
WebhookSessionResult,
WebhookSetupResult,
} from "#shared/payments.ts";
import {
createPaymentLink,
refundPayment,
retrieveOrder,
retrievePayment,
verifyWebhookSignature,
} from "#shared/square.ts";
import { squareApi, verifyWebhookSignature } from "#shared/square.ts";

/** Square payment provider implementation */
export const squarePaymentProvider: PaymentProvider = {
checkoutCompletedEventType: "payment.updated",

createCheckoutSession(intent: CheckoutIntent, baseUrl: string) {
return withCheckoutError(async () => {
const link = await createPaymentLink(intent, baseUrl);
const link = await squareApi.createPaymentLink(intent, baseUrl);
return toCheckoutResult(link?.orderId, link?.url, "Square");
});
},

async isPaymentRefunded(paymentReference: string): Promise<boolean> {
const payment = await retrievePayment(paymentReference);
const payment = await squareApi.retrievePayment(paymentReference);
if (!payment) return false;
// Fully refunded only: a partial refund leaves the customer still charged,
// so it must not count as refunded (matches Stripe's charge.refunded and
Expand All @@ -59,7 +53,7 @@ export const squarePaymentProvider: PaymentProvider = {
},

refundPayment(paymentReference: string): Promise<boolean> {
return refundPayment(paymentReference);
return squareApi.refundPayment(paymentReference);
},
requiresWebhookSignature: true,

Expand Down Expand Up @@ -112,7 +106,7 @@ export const squarePaymentProvider: PaymentProvider = {
): Promise<RetrieveSessionResult> {
/* jscpd:ignore-end */
// sessionId is the Square order ID
const order = await retrieveOrder(sessionId);
const order = await squareApi.retrieveOrder(sessionId);
if (!order?.id) {
logDebug("Square", `Order ${sessionId} not found`);
return null;
Expand All @@ -130,7 +124,7 @@ export const squarePaymentProvider: PaymentProvider = {
const paymentReference =
paidPaymentId ?? order.tenders?.[0]?.paymentId ?? "";
const payment = paymentReference
? await retrievePayment(paymentReference)
? await squareApi.retrievePayment(paymentReference)
: null;
// The webhook already saw this payment complete, so a read-back that is
// missing or still short of COMPLETED is Square lagging its own signed
Expand Down
10 changes: 0 additions & 10 deletions src/shared/square.ts
Original file line number Diff line number Diff line change
Expand Up @@ -846,16 +846,6 @@ export const squareApi: {
},
};

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P2 Badge Remove the stale resetSquareClient exemption

Because this deletion removes the top-level resetSquareClient export, the ALLOWED_TEST_HOOKS entry shared/square.ts:resetSquareClient in test/integration/code-quality.test.ts no longer matches any export. Leaving that dead exemption misdocuments the API and would silently exempt the same test-only alias if it were reintroduced later, so remove its entry and comment with the wrapper.

AGENTS.md reference: AGENTS.md:L170-L177

Useful? React with 👍 / 👎.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Fixed in 366a499 — the shared/square.ts:resetSquareClient entry and its comment are removed from ALLOWED_TEST_HOOKS. Tests reach the reset through squareApi.resetSquareClient(), and squareApi itself has production callers, so the export scan needs no exemption; the code-quality suite passes with the entry gone (19/19, including the test-only-exports scan). I also checked the rest of the list for entries orphaned by this sweep — constructTestWebhookEvent is still a live top-level export, and no sumup.ts entries exist — so this was the only stale one.


Generated by Claude Code

// Wrapper exports for production code (delegate to squareApi for test mocking)
export const getSquareClient = () => squareApi.getSquareClient();
export const resetSquareClient = () => squareApi.resetSquareClient();
export const testSquareConnection = () => squareApi.testSquareConnection();
export const createPaymentLink = (i: CheckoutIntent, b: string) =>
squareApi.createPaymentLink(i, b);
export const retrieveOrder = (id: string) => squareApi.retrieveOrder(id);
export const retrievePayment = (id: string) => squareApi.retrievePayment(id);
export const refundPayment = (id: string) => squareApi.refundPayment(id);

/** Result of testing the Square connection */
export type SquareConnectionTestResult = {
ok: boolean;
Expand Down
17 changes: 8 additions & 9 deletions src/shared/sumup-provider.ts
Original file line number Diff line number Diff line change
Expand Up @@ -38,12 +38,7 @@ import type {
WebhookSetupResult,
WebhookVerifyResult,
} from "#shared/payments.ts";
import {
createCheckout,
getTransactionStatus,
refundTransaction,
sumupApi,
} from "#shared/sumup.ts";
import { sumupApi } from "#shared/sumup.ts";
import type {
SumupCheckout,
SumupCheckoutStatus,
Expand Down Expand Up @@ -94,7 +89,9 @@ const buildValidatedSession = (
/** SumUp's checkout-session builder (see {@link makeCreateCheckoutSession}). */
const createSumupCheckoutSession = makeCreateCheckoutSession(
"SumUp",
createCheckout,
// A lambda, not the member itself: the checkout builder is captured once
// at module load, and resolving the member per call keeps test stubs live.
(intent, baseUrl) => sumupApi.createCheckout(intent, baseUrl),
(result) => ({ id: result?.reference, url: result?.url }),
);

Expand All @@ -104,11 +101,13 @@ export const sumupPaymentProvider: PaymentProvider = {
createCheckoutSession: createSumupCheckoutSession,

async isPaymentRefunded(paymentReference: string): Promise<boolean> {
return (await getTransactionStatus(paymentReference)) === "REFUNDED";
return (
(await sumupApi.getTransactionStatus(paymentReference)) === "REFUNDED"
);
},

refundPayment(paymentReference: string): Promise<boolean> {
return refundTransaction(paymentReference);
return sumupApi.refundTransaction(paymentReference);
},
requiresWebhookSignature: false,

Expand Down
8 changes: 0 additions & 8 deletions src/shared/sumup.ts
Original file line number Diff line number Diff line change
Expand Up @@ -266,11 +266,3 @@ export const sumupApi: {
return result;
},
};

// Wrapper exports for production code (delegate to sumupApi for test mocking)
export const createCheckout = (i: CheckoutIntent, b: string) =>
sumupApi.createCheckout(i, b);
export const refundTransaction = (id: string) => sumupApi.refundTransaction(id);
export const getTransactionStatus = (id: string) =>
sumupApi.getTransactionStatus(id);
export const testSumupConnection = () => sumupApi.testSumupConnection();
2 changes: 0 additions & 2 deletions test/integration/code-quality.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -207,8 +207,6 @@ const ALLOWED_TEST_HOOKS: string[] = [
"shared/limits.ts:PRUNE_CONTACTS_RETENTION_DAYS",
"shared/limits.ts:ADDRESS_CACHE_DAYS",
"shared/limits.ts:PRUNE_INTERVAL_HOURS",
// Reset cached Square client between tests
"shared/square.ts:resetSquareClient",
// Test helper for creating signed Square webhook payloads
"shared/square.ts:constructTestWebhookEvent",
// Raw attendee fetch for testing encrypted data (production uses batched getListingWithAttendeesRaw)
Expand Down
38 changes: 19 additions & 19 deletions test/shared/square/client.test.ts
Original file line number Diff line number Diff line change
@@ -1,11 +1,7 @@
import { expect } from "@std/expect";
import { describe, it as test } from "@std/testing/bdd";
import { settings } from "#shared/db/settings.ts";
import {
getSquareClient,
resetSquareClient,
testSquareConnection,
} from "#shared/square.ts";
import { squareApi } from "#shared/square.ts";
import {
configureSquare,
oneLocation,
Expand All @@ -30,14 +26,16 @@ describeSquare(() => {

/** Drive one request through the client and return the host it called. */
const hostFor = async (
client: NonNullable<Awaited<ReturnType<typeof getSquareClient>>>,
client: NonNullable<
Awaited<ReturnType<typeof squareApi.getSquareClient>>
>,
): Promise<string> => {
await client.locations.list();
return new URL(calledUrl).host;
};

test("returns null when access token not set", async () => {
const client = await getSquareClient();
const client = await squareApi.getSquareClient();
expect(client).toBeNull();
expect(debugMessages(debugLog())).toEqual([
"[Square] No access token configured, cannot create client",
Expand All @@ -46,7 +44,7 @@ describeSquare(() => {

test("returns client when access token is set in database", async () => {
await settings.update.square.accessToken("EAAAl_test_123");
const client = await getSquareClient();
const client = await squareApi.getSquareClient();
expect(client).not.toBeNull();
expect(debugLog().calls.at(-1)?.args[0]).toBe(
"[Square] Creating new Square client (production)",
Expand All @@ -55,18 +53,18 @@ describeSquare(() => {

test("returns cached client on second call with same token", async () => {
await settings.update.square.accessToken("EAAAl_cache_test");
const client1 = await getSquareClient();
const client1 = await squareApi.getSquareClient();
expect(client1).not.toBeNull();

// Second call with same token returns the very same cached instance.
const client2 = await getSquareClient();
const client2 = await squareApi.getSquareClient();
expect(client2).toBe(client1);
});

test("returns client in sandbox mode when sandbox setting enabled", async () => {
await settings.update.square.accessToken("EAAAl_sandbox_123");
await settings.update.square.sandbox(true);
const client = await getSquareClient();
const client = await squareApi.getSquareClient();
expect(client).not.toBeNull();
// Sandbox mode must route requests to the sandbox host.
using _fetch = trackFetch();
Expand All @@ -79,14 +77,14 @@ describeSquare(() => {
test("recreates client when sandbox setting changes", async () => {
await settings.update.square.accessToken("EAAAl_sandbox_toggle");
await settings.update.square.sandbox(false);
const client1 = await getSquareClient();
const client1 = await squareApi.getSquareClient();
expect(client1).not.toBeNull();
using _fetch = trackFetch();
expect(await hostFor(client1!)).toBe("connect.squareup.com");

// Toggling sandbox creates a new client configured for the sandbox host.
await settings.update.square.sandbox(true);
const client2 = await getSquareClient();
const client2 = await squareApi.getSquareClient();
expect(client2).not.toBe(client1);
expect(await hostFor(client2!)).toBe("connect.squareupsandbox.com");
});
Expand All @@ -95,20 +93,22 @@ describeSquare(() => {
describe("resetSquareClient", () => {
test("resets client state after token removed from db", async () => {
await settings.update.square.accessToken("EAAAl_test_123");
const client1 = await getSquareClient();
const client1 = await squareApi.getSquareClient();
expect(client1).not.toBeNull();

resetSquareClient();
squareApi.resetSquareClient();
resetDb();
await createTestDb();

const client2 = await getSquareClient();
const client2 = await squareApi.getSquareClient();
expect(client2).toBeNull();
});
});

describe("testSquareConnection", () => {
type ConnectionResult = Awaited<ReturnType<typeof testSquareConnection>>;
type ConnectionResult = Awaited<
ReturnType<typeof squareApi.testSquareConnection>
>;

/** Run an assertion only when the test named a value for it. */
const when = <T>(value: T | undefined, assert: (value: T) => void) => {
Expand Down Expand Up @@ -157,12 +157,12 @@ describeSquare(() => {
) => {
await configureSquare(config);
await withSquareClient({ locationsList }, async () => {
assert(await testSquareConnection());
assert(await squareApi.testSquareConnection());
});
};

test("returns error when no access token configured", async () => {
expect(await testSquareConnection()).toEqual({
expect(await squareApi.testSquareConnection()).toEqual({
accessToken: {
error: "No Square access token configured",
valid: false,
Expand Down
Loading