-
Notifications
You must be signed in to change notification settings - Fork 403
app deleted shared handler #2343
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Changes from 2 commits
52c6239
40d6ed3
bda5768
9e69c5f
e661d3f
e8c7f39
42793f0
41acd50
2a958f3
0923576
80db897
aad5ab4
0418fee
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,5 @@ | ||
| --- | ||
| "@saleor/webhook-utils": minor | ||
| --- | ||
|
|
||
| Added APP_DELETED shared webhook that automatically cleans up APL |
| Original file line number | Diff line number | Diff line change | ||||||||||||||||||||||||||||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
| @@ -0,0 +1,20 @@ | ||||||||||||||||||||||||||||||||||||||||||
| import { createAppDeletedHandler } from "@saleor/webhook-utils/app-deleted-handler"; | ||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||
| import { saleorApp } from "../../../../saleor-app"; | ||||||||||||||||||||||||||||||||||||||||||
| import { createLogger } from "../../../logger"; | ||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||
| const { handler, getWebhookManifest } = createAppDeletedHandler({ | ||||||||||||||||||||||||||||||||||||||||||
| apl: saleorApp.apl, | ||||||||||||||||||||||||||||||||||||||||||
| logger: createLogger("APP_DELETED handler"), | ||||||||||||||||||||||||||||||||||||||||||
| webhookPath: "api/webhooks/app-deleted", | ||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||
| hooks: { | ||||||||||||||||||||||||||||||||||||||||||
| onEvent: console.log, | ||||||||||||||||||||||||||||||||||||||||||
| onAuthDataDeleted: console.log, | ||||||||||||||||||||||||||||||||||||||||||
| onAuthDataDeleteError: console.log, | ||||||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||||||
| const { handler, getWebhookManifest } = createAppDeletedHandler({ | |
| apl: saleorApp.apl, | |
| logger: createLogger("APP_DELETED handler"), | |
| webhookPath: "api/webhooks/app-deleted", | |
| hooks: { | |
| onEvent: console.log, | |
| onAuthDataDeleted: console.log, | |
| onAuthDataDeleteError: console.log, | |
| const logger = createLogger("APP_DELETED handler"); | |
| const { handler, getWebhookManifest } = createAppDeletedHandler({ | |
| apl: saleorApp.apl, | |
| logger, | |
| webhookPath: "api/webhooks/app-deleted", | |
| hooks: { | |
| onEvent: () => logger.info("App deleted webhook event received"), | |
| onAuthDataDeleted: () => logger.info("Auth data deleted after app removal"), | |
| onAuthDataDeleteError: () => logger.error("Failed to delete auth data after app removal"), |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,9 @@ | ||
| subscription AppDeleted { | ||
| event { | ||
| ... on AppDeleted { | ||
| app { | ||
| id | ||
| } | ||
| } | ||
| } | ||
| } |
| Original file line number | Diff line number | Diff line change | ||||||||||||||||||||||||||||||||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
| @@ -0,0 +1,60 @@ | ||||||||||||||||||||||||||||||||||||||||||||||
| import { type APL } from "@saleor/app-sdk/APL"; | ||||||||||||||||||||||||||||||||||||||||||||||
| import { SaleorAsyncWebhook } from "@saleor/app-sdk/handlers/next"; | ||||||||||||||||||||||||||||||||||||||||||||||
| import { type WebhookContext } from "@saleor/app-sdk/handlers/shared"; | ||||||||||||||||||||||||||||||||||||||||||||||
| import { type Logger } from "@saleor/apps-logger"; | ||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||
| import { AppDeletedDocument } from "../generated/graphql"; | ||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||
| type Params = { | ||||||||||||||||||||||||||||||||||||||||||||||
| apl: APL; | ||||||||||||||||||||||||||||||||||||||||||||||
| webhookPath: string; | ||||||||||||||||||||||||||||||||||||||||||||||
| logger: Logger; | ||||||||||||||||||||||||||||||||||||||||||||||
|
Comment on lines
+4
to
+11
|
||||||||||||||||||||||||||||||||||||||||||||||
| import { type Logger } from "@saleor/apps-logger"; | |
| import { AppDeletedDocument } from "../generated/graphql"; | |
| type Params = { | |
| apl: APL; | |
| webhookPath: string; | |
| logger: Logger; | |
| import { AppDeletedDocument } from "../generated/graphql"; | |
| type MinimalLogger = { | |
| info: (message: string, ...meta: unknown[]) => void; | |
| warn?: (message: string, ...meta: unknown[]) => void; | |
| error: (message: string, ...meta: unknown[]) => void; | |
| debug?: (message: string, ...meta: unknown[]) => void; | |
| }; | |
| type Params = { | |
| apl: APL; | |
| webhookPath: string; | |
| logger: MinimalLogger; |
Copilot
AI
Apr 23, 2026
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This introduces new webhook-handling behavior (creating a SaleorAsyncWebhook and deleting APL entries) but there are no unit tests covering the success/error paths. Since this package already uses Vitest tests, add tests that verify apl.delete is called with the Saleor API URL and that the handler returns 200 on success and 500 on failure.
Copilot
AI
Apr 24, 2026
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The webhook name is set to the all-caps event identifier ("APP_DELETED"). In other apps, webhook name is a human-readable label (e.g. "OrderCancelled", "Customer Created") and shows up in Saleor’s webhook list. Consider changing this to a descriptive name like "App deleted" / "AppDeleted" while keeping event: "APP_DELETED".
Copilot
AI
Apr 23, 2026
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Remove the leftover debug console.log("asdf") (and the adjacent TODO). This will pollute logs in production and bypasses the app logger/observability pipeline used elsewhere in the repo.
| // todo something failing here | |
| console.log("asdf"); |
Copilot
AI
Apr 27, 2026
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This new shared handler introduces non-trivial behavior (invoking hooks, deleting APL auth data, and mapping failures to HTTP responses) but there are no unit tests covering success/failure paths. Add tests for: (1) calling apl.delete with ctx.authData.saleorApiUrl, (2) hook invocation order, and (3) 500 responses + onAuthDataDeleteError on deletion failure.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Unlike the other webhook routes in this app, this handler is not wrapped with
wrapWithLoggerContextandwithSpanAttributes, so logs/traces for APP_DELETED may miss the usual context and OpenTelemetry attributes. Consider wrapping the exported handler consistently (or havecreateAppDeletedHandlerexpose a wrapped handler option).