From 9db2ef3adddd8760c1faa475ac4d69803ae9cde2 Mon Sep 17 00:00:00 2001 From: Lukasz Ostrowski Date: Wed, 26 Feb 2025 12:16:17 +0100 Subject: [PATCH 1/2] remove built in webhook response builder --- .changeset/new-paws-win.md | 37 +++++++++++++++++++ .../saleor-sync-webhook.test.ts | 24 ++++++------ .../saleor-webhooks/saleor-sync-webhook.ts | 12 ++---- .../saleor-webhooks/saleor-webhook.ts | 15 ++++---- .../saleor-sync-webhook.test.ts | 22 +++++------ .../saleor-webhooks/saleor-sync-webhook.ts | 17 ++------- .../saleor-sync-webhook.test.ts | 10 ++--- .../saleor-webhooks/saleor-sync-webhook.ts | 17 ++------- src/handlers/shared/saleor-webhook.ts | 7 ---- .../shared/sync-webhook-response-builder.ts | 2 +- 10 files changed, 83 insertions(+), 80 deletions(-) create mode 100644 .changeset/new-paws-win.md diff --git a/.changeset/new-paws-win.md b/.changeset/new-paws-win.md new file mode 100644 index 00000000..83bd31b3 --- /dev/null +++ b/.changeset/new-paws-win.md @@ -0,0 +1,37 @@ +--- +"@saleor/app-sdk": major +--- + +Removed `ctx` parameter from SyncWebhookHandler and replace with standalone `buildSyncWebhookResponsePayload` function + +Before + +```typescript + +new SaleorSyncWebhook(...).createHandler( + req, res, ctx +) { + + const typeSafePayload = ctx.buildResponse({ + // this must be valid response + }) +} +``` + +After + +```typescript +import { buildSyncWebhookResponsePayload } from "@saleor/app-sdk/handlers/shared"; + +new SaleorSyncWebhook(...).createHandler( + req, res, ctx +) +{ + + const typeSafePayload = buildSyncWebhookResponsePayload<"ORDER_CALCULATE_TAXES">({ + // this must be valid shape + }) +} +``` + +This change reduces complexity of TypeScript generics and make it easier to build abstractions on top of built-in handlers diff --git a/src/handlers/platforms/aws-lambda/saleor-webhooks/saleor-sync-webhook.test.ts b/src/handlers/platforms/aws-lambda/saleor-webhooks/saleor-sync-webhook.test.ts index 1949c06a..f7fa38dd 100644 --- a/src/handlers/platforms/aws-lambda/saleor-webhooks/saleor-sync-webhook.test.ts +++ b/src/handlers/platforms/aws-lambda/saleor-webhooks/saleor-sync-webhook.test.ts @@ -1,6 +1,6 @@ import { afterEach, describe, expect, it, vi } from "vitest"; -import { FormatWebhookErrorResult } from "@/handlers/shared"; +import { buildSyncWebhookResponsePayload, FormatWebhookErrorResult } from "@/handlers/shared"; import { SaleorWebhookValidator } from "@/handlers/shared/saleor-webhook-validator"; import { MockAPL } from "@/test-utils/mock-apl"; @@ -27,20 +27,20 @@ describe("AWS Lambda SaleorSyncWebhook", () => { it("returns valid URL when baseUrl has Lambda stage", () => { const webhook = new SaleorSyncWebhook(webhookConfig); expect(webhook.getWebhookManifest("https://aws-lambda.com/prod").targetUrl).toBe( - "https://aws-lambda.com/prod/api/webhooks/checkout-calculate-taxes" + "https://aws-lambda.com/prod/api/webhooks/checkout-calculate-taxes", ); expect(webhook.getWebhookManifest("https://aws-lambda.com/prod/").targetUrl).toBe( - "https://aws-lambda.com/prod/api/webhooks/checkout-calculate-taxes" + "https://aws-lambda.com/prod/api/webhooks/checkout-calculate-taxes", ); expect(webhook.getWebhookManifest("https://aws-lambda.com/test").targetUrl).toBe( - "https://aws-lambda.com/test/api/webhooks/checkout-calculate-taxes" + "https://aws-lambda.com/test/api/webhooks/checkout-calculate-taxes", ); }); it("returns valid URL when baseURl doesn't have lambda stage ($default stage)", () => { const webhook = new SaleorSyncWebhook(webhookConfig); expect(webhook.getWebhookManifest("https://aws-lambda.com/").targetUrl).toBe( - "https://aws-lambda.com/api/webhooks/checkout-calculate-taxes" + "https://aws-lambda.com/api/webhooks/checkout-calculate-taxes", ); }); @@ -50,16 +50,16 @@ describe("AWS Lambda SaleorSyncWebhook", () => { webhookPath: `/${webhookConfig.webhookPath}`, }); expect(webhook.getWebhookManifest("https://aws-lambda.com/prod").targetUrl).toBe( - "https://aws-lambda.com/prod/api/webhooks/checkout-calculate-taxes" + "https://aws-lambda.com/prod/api/webhooks/checkout-calculate-taxes", ); expect(webhook.getWebhookManifest("https://aws-lambda.com/prod/").targetUrl).toBe( - "https://aws-lambda.com/prod/api/webhooks/checkout-calculate-taxes" + "https://aws-lambda.com/prod/api/webhooks/checkout-calculate-taxes", ); expect(webhook.getWebhookManifest("https://aws-lambda.com/test").targetUrl).toBe( - "https://aws-lambda.com/test/api/webhooks/checkout-calculate-taxes" + "https://aws-lambda.com/test/api/webhooks/checkout-calculate-taxes", ); expect(webhook.getWebhookManifest("https://aws-lambda.com/").targetUrl).toBe( - "https://aws-lambda.com/api/webhooks/checkout-calculate-taxes" + "https://aws-lambda.com/api/webhooks/checkout-calculate-taxes", ); }); }); @@ -88,15 +88,15 @@ describe("AWS Lambda SaleorSyncWebhook", () => { const handler: AwsLambdaSyncWebhookHandler = vi .fn() - .mockImplementation(async (_event, _context, ctx) => ({ + .mockImplementation(async () => ({ statusCode: 200, body: JSON.stringify( - ctx.buildResponse({ + buildSyncWebhookResponsePayload<"CHECKOUT_CALCULATE_TAXES">({ lines: [{ tax_rate: 8, total_net_amount: 10, total_gross_amount: 10.8 }], shipping_price_gross_amount: 2.16, shipping_tax_rate: 8, shipping_price_net_amount: 2, - }) + }), ), })); const wrappedHandler = saleorSyncWebhook.createHandler(handler); diff --git a/src/handlers/platforms/aws-lambda/saleor-webhooks/saleor-sync-webhook.ts b/src/handlers/platforms/aws-lambda/saleor-webhooks/saleor-sync-webhook.ts index c28d817f..809a2858 100644 --- a/src/handlers/platforms/aws-lambda/saleor-webhooks/saleor-sync-webhook.ts +++ b/src/handlers/platforms/aws-lambda/saleor-webhooks/saleor-sync-webhook.ts @@ -1,5 +1,3 @@ -import { SyncWebhookInjectedContext } from "@/handlers/shared"; -import { buildSyncWebhookResponsePayload } from "@/handlers/shared/sync-webhook-response-builder"; import { SyncWebhookEventType } from "@/types"; import { AWSLambdaHandler } from "../platform-adapter"; @@ -7,20 +5,16 @@ import { AwsLambdaWebhookHandler, SaleorWebApiWebhook, WebhookConfig } from "./s export type AwsLambdaSyncWebhookHandler< TPayload, - TEvent extends SyncWebhookEventType = SyncWebhookEventType, -> = AwsLambdaWebhookHandler>; +> = AwsLambdaWebhookHandler; export class SaleorSyncWebhook< TPayload = unknown, TEvent extends SyncWebhookEventType = SyncWebhookEventType, -> extends SaleorWebApiWebhook> { +> extends SaleorWebApiWebhook { readonly event: TEvent; protected readonly eventType = "sync" as const; - protected extraContext = { - buildResponse: buildSyncWebhookResponsePayload, - }; constructor(configuration: WebhookConfig) { super(configuration); @@ -28,7 +22,7 @@ export class SaleorSyncWebhook< this.event = configuration.event; } - createHandler(handlerFn: AwsLambdaSyncWebhookHandler): AWSLambdaHandler { + createHandler(handlerFn: AwsLambdaSyncWebhookHandler): AWSLambdaHandler { return super.createHandler(handlerFn); } } diff --git a/src/handlers/platforms/aws-lambda/saleor-webhooks/saleor-webhook.ts b/src/handlers/platforms/aws-lambda/saleor-webhooks/saleor-webhook.ts index 0c6d03f2..0b32cd27 100644 --- a/src/handlers/platforms/aws-lambda/saleor-webhooks/saleor-webhook.ts +++ b/src/handlers/platforms/aws-lambda/saleor-webhooks/saleor-webhook.ts @@ -16,21 +16,21 @@ export type WebhookConfig GenericWebhookConfig; /** Function type provided by consumer in `SaleorWebApiWebhook.createHandler` */ -export type AwsLambdaWebhookHandler = ( +export type AwsLambdaWebhookHandler = ( event: AwsLambdaHandlerInput, context: Context, - ctx: WebhookContext & TExtras + ctx: WebhookContext, ) => Promise | APIGatewayProxyStructuredResultV2; -export abstract class SaleorWebApiWebhook< - TPayload = unknown, - TExtras extends Record = {} -> extends GenericSaleorWebhook { +export abstract class SaleorWebApiWebhook extends GenericSaleorWebhook< + AwsLambdaHandlerInput, + TPayload +> { /** * Wraps provided function, to ensure incoming request comes from registered Saleor instance. * Also provides additional `context` object containing typed payload and request properties. */ - createHandler(handlerFn: AwsLambdaWebhookHandler): AWSLambdaHandler { + createHandler(handlerFn: AwsLambdaWebhookHandler): AWSLambdaHandler { return async (event, context) => { const adapter = new AwsLambdaAdapter(event, context); const prepareRequestResult = await super.prepareRequest(adapter); @@ -41,7 +41,6 @@ export abstract class SaleorWebApiWebhook< debug("Incoming request validated. Call handlerFn"); return handlerFn(event, context, { - ...(this.extraContext ?? ({} as TExtras)), ...prepareRequestResult.context, }); }; diff --git a/src/handlers/platforms/fetch-api/saleor-webhooks/saleor-sync-webhook.test.ts b/src/handlers/platforms/fetch-api/saleor-webhooks/saleor-sync-webhook.test.ts index 83dbea60..9cf9c741 100644 --- a/src/handlers/platforms/fetch-api/saleor-webhooks/saleor-sync-webhook.test.ts +++ b/src/handlers/platforms/fetch-api/saleor-webhooks/saleor-sync-webhook.test.ts @@ -1,6 +1,6 @@ import { afterEach, describe, expect, it, vi } from "vitest"; -import { FormatWebhookErrorResult } from "@/handlers/shared"; +import { buildSyncWebhookResponsePayload, FormatWebhookErrorResult } from "@/handlers/shared"; import { SaleorWebhookValidator } from "@/handlers/shared/saleor-webhook-validator"; import { MockAPL } from "@/test-utils/mock-apl"; @@ -41,17 +41,15 @@ describe("Web API SaleorSyncWebhook", () => { }, }); - const handler = vi - .fn>() - .mockImplementation((_request, ctx) => { - const responsePayload = ctx.buildResponse({ - lines: [{ tax_rate: 8, total_net_amount: 10, total_gross_amount: 1.08 }], - shipping_price_gross_amount: 2, - shipping_tax_rate: 8, - shipping_price_net_amount: 1, - }); - return new Response(JSON.stringify(responsePayload), { status: 200 }); + const handler = vi.fn>().mockImplementation(() => { + const responsePayload = buildSyncWebhookResponsePayload<"ORDER_CALCULATE_TAXES">({ + lines: [{ tax_rate: 8, total_net_amount: 10, total_gross_amount: 1.08 }], + shipping_price_gross_amount: 2, + shipping_tax_rate: 8, + shipping_price_net_amount: 1, }); + return new Response(JSON.stringify(responsePayload), { status: 200 }); + }); const saleorSyncWebhook = new SaleorSyncWebhook(webhookConfiguration); @@ -70,7 +68,7 @@ describe("Web API SaleorSyncWebhook", () => { shipping_price_gross_amount: 2, shipping_tax_rate: 8, shipping_price_net_amount: 1, - }) + }), ); }); diff --git a/src/handlers/platforms/fetch-api/saleor-webhooks/saleor-sync-webhook.ts b/src/handlers/platforms/fetch-api/saleor-webhooks/saleor-sync-webhook.ts index af5e8a89..83ec2a57 100644 --- a/src/handlers/platforms/fetch-api/saleor-webhooks/saleor-sync-webhook.ts +++ b/src/handlers/platforms/fetch-api/saleor-webhooks/saleor-sync-webhook.ts @@ -1,34 +1,25 @@ -import { SyncWebhookInjectedContext } from "@/handlers/shared"; -import { buildSyncWebhookResponsePayload } from "@/handlers/shared/sync-webhook-response-builder"; import { SyncWebhookEventType } from "@/types"; import { WebApiHandler } from "../platform-adapter"; import { SaleorWebApiWebhook, WebApiWebhookHandler, WebhookConfig } from "./saleor-webhook"; -export type WebApiSyncWebhookHandler< - TPayload, - TEvent extends SyncWebhookEventType = SyncWebhookEventType -> = WebApiWebhookHandler>; +export type WebApiSyncWebhookHandler = WebApiWebhookHandler; export class SaleorSyncWebhook< TPayload = unknown, - TEvent extends SyncWebhookEventType = SyncWebhookEventType -> extends SaleorWebApiWebhook> { + TEvent extends SyncWebhookEventType = SyncWebhookEventType, +> extends SaleorWebApiWebhook { readonly event: TEvent; protected readonly eventType = "sync" as const; - protected extraContext = { - buildResponse: buildSyncWebhookResponsePayload, - }; - constructor(configuration: WebhookConfig) { super(configuration); this.event = configuration.event; } - createHandler(handlerFn: WebApiSyncWebhookHandler): WebApiHandler { + createHandler(handlerFn: WebApiSyncWebhookHandler): WebApiHandler { return super.createHandler(handlerFn); } } diff --git a/src/handlers/platforms/next/saleor-webhooks/saleor-sync-webhook.test.ts b/src/handlers/platforms/next/saleor-webhooks/saleor-sync-webhook.test.ts index f0c95ea5..7c75f6fa 100644 --- a/src/handlers/platforms/next/saleor-webhooks/saleor-sync-webhook.test.ts +++ b/src/handlers/platforms/next/saleor-webhooks/saleor-sync-webhook.test.ts @@ -1,7 +1,7 @@ import { createMocks } from "node-mocks-http"; import { afterEach, describe, expect, it, vi } from "vitest"; -import { WebhookError } from "@/handlers/shared"; +import { buildSyncWebhookResponsePayload, WebhookError } from "@/handlers/shared"; import { SaleorWebhookValidator } from "@/handlers/shared/saleor-webhook-validator"; import { MockAPL } from "@/test-utils/mock-apl"; @@ -31,7 +31,7 @@ describe("Next.js SaleorSyncWebhook", () => { expect(manifest).toEqual( expect.objectContaining({ targetUrl: `${baseUrl}/${validSyncWebhookConfiguration.webhookPath}`, - }) + }), ); }); @@ -66,8 +66,8 @@ describe("Next.js SaleorSyncWebhook", () => { }); const saleorSyncWebhook = new SaleorSyncWebhook(validSyncWebhookConfiguration); - const testHandler: NextJsWebhookHandler = vi.fn().mockImplementation((_req, res, ctx) => { - const responsePayload = ctx.buildResponse({ + const testHandler: NextJsWebhookHandler = vi.fn().mockImplementation((_req, res) => { + const responsePayload = buildSyncWebhookResponsePayload({ lines: [{ tax_rate: 8, total_net_amount: 10, total_gross_amount: 1.08 }], shipping_price_gross_amount: 2, shipping_tax_rate: 8, @@ -88,7 +88,7 @@ describe("Next.js SaleorSyncWebhook", () => { shipping_price_gross_amount: 2, shipping_tax_rate: 8, shipping_price_net_amount: 1, - }) + }), ); }); diff --git a/src/handlers/platforms/next/saleor-webhooks/saleor-sync-webhook.ts b/src/handlers/platforms/next/saleor-webhooks/saleor-sync-webhook.ts index 74b75710..ee31d125 100644 --- a/src/handlers/platforms/next/saleor-webhooks/saleor-sync-webhook.ts +++ b/src/handlers/platforms/next/saleor-webhooks/saleor-sync-webhook.ts @@ -1,35 +1,26 @@ import { NextApiHandler } from "next"; -import { SyncWebhookInjectedContext } from "@/handlers/shared"; -import { buildSyncWebhookResponsePayload } from "@/handlers/shared/sync-webhook-response-builder"; import { SyncWebhookEventType } from "@/types"; import { NextJsWebhookHandler, SaleorWebhook, WebhookConfig } from "./saleor-webhook"; -export type NextJsSyncWebhookHandler< - TPayload, - TEvent extends SyncWebhookEventType = SyncWebhookEventType -> = NextJsWebhookHandler>; +export type NextJsSyncWebhookHandler = NextJsWebhookHandler; export class SaleorSyncWebhook< TPayload = unknown, - TEvent extends SyncWebhookEventType = SyncWebhookEventType -> extends SaleorWebhook> { + TEvent extends SyncWebhookEventType = SyncWebhookEventType, +> extends SaleorWebhook { readonly event: TEvent; protected readonly eventType = "sync" as const; - protected extraContext = { - buildResponse: buildSyncWebhookResponsePayload, - }; - constructor(configuration: WebhookConfig) { super(configuration); this.event = configuration.event; } - createHandler(handlerFn: NextJsSyncWebhookHandler): NextApiHandler { + createHandler(handlerFn: NextJsSyncWebhookHandler): NextApiHandler { return super.createHandler(handlerFn); } } diff --git a/src/handlers/shared/saleor-webhook.ts b/src/handlers/shared/saleor-webhook.ts index e207daa9..053de609 100644 --- a/src/handlers/shared/saleor-webhook.ts +++ b/src/handlers/shared/saleor-webhook.ts @@ -1,7 +1,4 @@ -import { SyncWebhookEventType } from "@/types"; - import { AuthData } from "../../APL"; -import { buildSyncWebhookResponsePayload } from "./sync-webhook-response-builder"; export const WebhookErrorCodeMap: Record = { OTHER: 500, @@ -58,10 +55,6 @@ export type WebhookContext = { schemaVersion: number | null; }; -export type SyncWebhookInjectedContext = { - buildResponse: typeof buildSyncWebhookResponsePayload; -}; - export type FormatWebhookErrorResult = { code: number; body: string; diff --git a/src/handlers/shared/sync-webhook-response-builder.ts b/src/handlers/shared/sync-webhook-response-builder.ts index 9b690661..a755814a 100644 --- a/src/handlers/shared/sync-webhook-response-builder.ts +++ b/src/handlers/shared/sync-webhook-response-builder.ts @@ -167,5 +167,5 @@ export type SyncWebhookResponsesMap = { * Identity function, but it works on Typescript level to pick right payload based on first param */ export const buildSyncWebhookResponsePayload = ( - payload: SyncWebhookResponsesMap[E] + payload: SyncWebhookResponsesMap[E], ): SyncWebhookResponsesMap[E] => payload; From c04f993d5521c36e3e0da5ceb5e732e42e538f59 Mon Sep 17 00:00:00 2001 From: Lukasz Ostrowski Date: Thu, 27 Feb 2025 09:38:23 +0100 Subject: [PATCH 2/2] iprove changeset --- .changeset/new-paws-win.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.changeset/new-paws-win.md b/.changeset/new-paws-win.md index 83bd31b3..d6098e0b 100644 --- a/.changeset/new-paws-win.md +++ b/.changeset/new-paws-win.md @@ -2,7 +2,7 @@ "@saleor/app-sdk": major --- -Removed `ctx` parameter from SyncWebhookHandler and replace with standalone `buildSyncWebhookResponsePayload` function +Removed `ctx.buildResponse` parameter from SyncWebhookHandler ctx and replace with standalone `buildSyncWebhookResponsePayload` function Before