-
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 11 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-app-klaviyo": minor | ||
| --- | ||
|
|
||
| App now will listen on APP_DELETED webhook and clean up it's Auth Data once removed | ||
| 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,3 @@ | ||
| import { handler } from "./webhook-definition"; | ||
|
|
||
| export const POST = handler; |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,41 @@ | ||
| import { createAppDeletedHandler } from "@saleor/webhook-utils/app-deleted-handler"; | ||
|
|
||
| import { env } from "@/env"; | ||
| import { ClientLogDynamoEntityFactory, LogsTable } from "@/modules/client-logs/dynamo-logs-table"; | ||
| import { LogsRepositoryDynamodb } from "@/modules/client-logs/logs-repository"; | ||
| import { createDocumentClient, createDynamoClient } from "@/modules/dynamodb/dynamo-client"; | ||
|
|
||
| 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: { | ||
| async onEvent({ authData }) { | ||
| const logsTable = LogsTable.create({ | ||
| documentClient: createDocumentClient( | ||
| createDynamoClient({ | ||
| connectionTimeout: env.DYNAMODB_CONNECTION_TIMEOUT_MS, | ||
| requestTimeout: env.DYNAMODB_REQUEST_TIMEOUT_MS, | ||
| }), | ||
| ), | ||
| tableName: env.DYNAMODB_LOGS_TABLE_NAME, | ||
| }); | ||
| const logByDateEntity = ClientLogDynamoEntityFactory.createLogByDate(logsTable); | ||
| const logByCheckoutOrOrderId = | ||
| ClientLogDynamoEntityFactory.createLogByCheckoutOrOrderId(logsTable); | ||
|
|
||
| const repo = new LogsRepositoryDynamodb({ | ||
| logsTable, | ||
| logByDateEntity, | ||
| logByCheckoutOrOrderId, | ||
| }); | ||
|
|
||
| await repo.pruneAllLogs({ saleorApiUrl: authData.saleorApiUrl }); | ||
| }, | ||
|
Comment on lines
+30
to
+37
|
||
| }, | ||
| }); | ||
|
|
||
| export { getWebhookManifest, handler }; | ||
| Original file line number | Diff line number | Diff line change | ||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|
| @@ -1,9 +1,11 @@ | ||||||||||||
| import { type BatchWriteCommandOutput } from "@aws-sdk/lib-dynamodb"; | ||||||||||||
| import { | ||||||||||||
| BatchDeleteRequest, | ||||||||||||
| BatchPutRequest, | ||||||||||||
| BatchWriteCommand, | ||||||||||||
| executeBatchWrite, | ||||||||||||
| QueryCommand, | ||||||||||||
| ScanCommand, | ||||||||||||
| } from "dynamodb-toolbox"; | ||||||||||||
| import { err, ok, Result, ResultAsync } from "neverthrow"; | ||||||||||||
| import { ulid } from "ulid"; | ||||||||||||
|
|
@@ -38,6 +40,7 @@ export interface ILogsRepository { | |||||||||||
| saleorApiUrl: string; | ||||||||||||
| appId: string; | ||||||||||||
| }): Promise<Result<undefined, unknown>>; | ||||||||||||
| pruneAllLogs(args: { saleorApiUrl: string }): Promise<Result<undefined, unknown>>; | ||||||||||||
| } | ||||||||||||
|
|
||||||||||||
| /** | ||||||||||||
|
|
@@ -323,6 +326,108 @@ export class LogsRepositoryDynamodb implements ILogsRepository { | |||||||||||
|
|
||||||||||||
| return ok(undefined); | ||||||||||||
| } | ||||||||||||
|
|
||||||||||||
| async pruneAllLogs({ | ||||||||||||
| saleorApiUrl, | ||||||||||||
| }: { | ||||||||||||
| saleorApiUrl: string; | ||||||||||||
| }): Promise< | ||||||||||||
| Result< | ||||||||||||
| undefined, | ||||||||||||
| | InstanceType<typeof LogsRepositoryDynamodb.LogsFetchError> | ||||||||||||
| | InstanceType<typeof LogsRepositoryDynamodb.WriteLogError> | ||||||||||||
| | InstanceType<typeof LogsRepositoryDynamodb.UnprocessedItemsError> | ||||||||||||
| > | ||||||||||||
| > { | ||||||||||||
| this.logger.debug("Starting pruning logs for saleorApiUrl", { saleorApiUrl }); | ||||||||||||
|
|
||||||||||||
| const pkPrefix = `${saleorApiUrl}#`; | ||||||||||||
|
|
||||||||||||
| let lastEvaluatedKey: LastEvaluatedKey; | ||||||||||||
| let deletedCount = 0; | ||||||||||||
|
|
||||||||||||
| do { | ||||||||||||
|
Comment on lines
+330
to
+349
|
||||||||||||
| const scanResult = await ResultAsync.fromPromise( | ||||||||||||
| this.logsTable | ||||||||||||
| .build(ScanCommand) | ||||||||||||
| .entities(this.logByDateEntity, this.logsByCheckoutOrOrderId) | ||||||||||||
| .options({ | ||||||||||||
| exclusiveStartKey: lastEvaluatedKey, | ||||||||||||
| showEntityAttr: true, | ||||||||||||
| filters: { | ||||||||||||
| LOG_BY_DATE: { attr: "PK", beginsWith: pkPrefix }, | ||||||||||||
| LOG_BY_CHECKOUT_OR_ORDER_ID: { attr: "PK", beginsWith: pkPrefix }, | ||||||||||||
| }, | ||||||||||||
| }) | ||||||||||||
| .send(), | ||||||||||||
|
Comment on lines
+349
to
+362
|
||||||||||||
| (error) => | ||||||||||||
| new LogsRepositoryDynamodb.LogsFetchError("Error while scanning logs for pruning", { | ||||||||||||
| cause: error, | ||||||||||||
| }), | ||||||||||||
| ); | ||||||||||||
|
|
||||||||||||
| if (scanResult.isErr()) { | ||||||||||||
| this.logger.error("Error while scanning logs for pruning", { error: scanResult.error }); | ||||||||||||
|
|
||||||||||||
| return err(scanResult.error); | ||||||||||||
| } | ||||||||||||
|
|
||||||||||||
| lastEvaluatedKey = scanResult.value.LastEvaluatedKey; | ||||||||||||
|
|
||||||||||||
| const items = scanResult.value.Items ?? []; | ||||||||||||
|
|
||||||||||||
| for (let i = 0; i < items.length; i += 25) { | ||||||||||||
| const chunk = items.slice(i, i + 25); | ||||||||||||
|
|
||||||||||||
| const requests = chunk.map((item) => | ||||||||||||
| item.entity === "LOG_BY_DATE" | ||||||||||||
| ? this.logByDateEntity | ||||||||||||
| .build(BatchDeleteRequest) | ||||||||||||
| .key({ PK: item.PK, ulid: item.ulid, date: item.date }) | ||||||||||||
| : this.logsByCheckoutOrOrderId.build(BatchDeleteRequest).key({ | ||||||||||||
| PK: item.PK, | ||||||||||||
| ulid: item.ulid, | ||||||||||||
| date: item.date, | ||||||||||||
| checkoutOrOrderId: item.checkoutOrOrderId, | ||||||||||||
| }), | ||||||||||||
|
Comment on lines
+382
to
+392
|
||||||||||||
| ); | ||||||||||||
|
|
||||||||||||
| const cmd = this.logsTable.build(BatchWriteCommand).requests(...requests); | ||||||||||||
|
|
||||||||||||
| const deleteResult = await ResultAsync.fromPromise( | ||||||||||||
| executeBatchWrite({ capacity: "TOTAL", maxAttempts: 3 }, cmd), | ||||||||||||
| (error) => | ||||||||||||
| new LogsRepositoryDynamodb.WriteLogError("Error while deleting logs from DynamoDB", { | ||||||||||||
| cause: error, | ||||||||||||
| }), | ||||||||||||
| ); | ||||||||||||
|
|
||||||||||||
| if (deleteResult.isErr()) { | ||||||||||||
| this.logger.error("Error while batch-deleting logs from DynamoDB", { | ||||||||||||
| error: deleteResult.error, | ||||||||||||
| }); | ||||||||||||
|
|
||||||||||||
| return err(deleteResult.error); | ||||||||||||
| } | ||||||||||||
|
|
||||||||||||
| if (this.hasUnprocessedItems(deleteResult.value.UnprocessedItems)) { | ||||||||||||
| this.logger.warn("Some logs were not deleted from DynamoDB", { | ||||||||||||
| unprocessedItems: deleteResult.value.UnprocessedItems, | ||||||||||||
| }); | ||||||||||||
|
|
||||||||||||
| return err( | ||||||||||||
| new LogsRepositoryDynamodb.UnprocessedItemsError("Some logs were not deleted"), | ||||||||||||
| ); | ||||||||||||
| } | ||||||||||||
|
|
||||||||||||
| deletedCount += chunk.length; | ||||||||||||
| } | ||||||||||||
| } while (lastEvaluatedKey); | ||||||||||||
|
|
||||||||||||
| this.logger.info("Pruned all logs for saleorApiUrl", { saleorApiUrl, deletedCount }); | ||||||||||||
|
|
||||||||||||
| return ok(undefined); | ||||||||||||
| } | ||||||||||||
| } | ||||||||||||
|
|
||||||||||||
| /** | ||||||||||||
|
|
@@ -376,4 +481,18 @@ export class LogsRepositoryMemory implements ILogsRepository { | |||||||||||
| }): Promise<Result<{ clientLogs: ClientLog[]; lastEvaluatedKey: LastEvaluatedKey }, never>> { | ||||||||||||
| return ok({ clientLogs: this.logs, lastEvaluatedKey: undefined }); | ||||||||||||
| } | ||||||||||||
|
|
||||||||||||
| async pruneAllLogs(args: { | ||||||||||||
| saleorApiUrl: string; | ||||||||||||
| appId: string; | ||||||||||||
|
||||||||||||
| appId: string; |
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.
LogsRepositoryMemory.pruneAllLogs() calls LogsTable.decomposePrimaryKey(log.id), but ClientLog.id is an ULID (see LogsTransformer mapping id: entity.ulid). Splitting the ULID by # won’t yield saleorApiUrl, so this prune will never remove any logs.
| const [saleorApiUrl] = LogsTable.decomposePrimaryKey(log.id); | |
| return args.saleorApiUrl !== saleorApiUrl; | |
| return args.saleorApiUrl !== log.saleorApiUrl; |
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.
LogsRepositoryMemory.pruneAllLogs has a signature that doesn't match ILogsRepository (it requires appId, while the interface does not), which will break TypeScript compilation. Also, the pruning logic splits log.id using LogsTable.decomposePrimaryKey, but ClientLog.id is just a ULID (see LogsTransformer mapping), so this filter will never match and logs won't be pruned. Align the method signature with the interface and store enough context in the memory repo (e.g. persist PK / saleorApiUrl alongside the log) to make pruning work.
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,5 +1,6 @@ | ||
| /// <reference types="next" /> | ||
| /// <reference types="next/image-types/global" /> | ||
| /// <reference types="next/navigation-types/compat/navigation" /> | ||
|
|
||
| // NOTE: This file should not be edited | ||
| // see https://nextjs.org/docs/pages/api-reference/config/typescript for more information. | ||
| // see https://nextjs.org/docs/app/api-reference/config/typescript for more information. |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,3 @@ | ||
| import { handler } from "./webhook-definition"; | ||
|
|
||
| export const POST = handler; |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,12 @@ | ||
| 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", | ||
| }); | ||
|
|
||
| export { getWebhookManifest, handler }; |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,8 +1,22 @@ | ||
| { | ||
| "extends": "@saleor/typescript-config-apps/base.json", | ||
| "compilerOptions": { | ||
| "baseUrl": "." | ||
| "baseUrl": ".", | ||
| "plugins": [ | ||
| { | ||
| "name": "next" | ||
| } | ||
| ] | ||
| }, | ||
| "include": ["next-env.d.ts", "**/*.ts", "**/*.tsx", "next.config.ts", "graphql.config.ts"], | ||
| "exclude": ["node_modules"] | ||
| "include": [ | ||
| "**/*.ts", | ||
| "**/*.tsx", | ||
| "graphql.config.ts", | ||
| "next-env.d.ts", | ||
| "next.config.ts", | ||
| ".next/types/**/*.ts" | ||
| ], | ||
| "exclude": [ | ||
| "node_modules" | ||
| ] | ||
| } |
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.
Correct grammar in the changeset text: “clean up it's Auth Data” should use the possessive “its”.