diff --git a/packages/api/src/routers/card.ts b/packages/api/src/routers/card.ts index 5591120f..a7aa1f6e 100644 --- a/packages/api/src/routers/card.ts +++ b/packages/api/src/routers/card.ts @@ -189,7 +189,7 @@ export const cardRouter = createTRPCRouter({ title: input.title, description: input.description, dueDate: input.dueDate ?? null, - listId: String(newCard.listId), + listId: list.publicId, }, { boardId: list.boardPublicId, @@ -888,9 +888,18 @@ export const cardRouter = createTRPCRouter({ ); let newListId: number | undefined; + let newList: + | { + id: number; + publicId: string; + name: string; + boardId: number; + index: number; + } + | undefined; if (input.listPublicId) { - const newList = await listRepo.getByPublicId( + newList = await listRepo.getByPublicId( ctx.db, input.listPublicId, ); @@ -1037,8 +1046,19 @@ export const cardRouter = createTRPCRouter({ ) { webhookChanges.dueDate = { from: previousDueDate, to: input.dueDate }; } - if (newListId && existingCard.listId !== newListId) { - webhookChanges.listId = { from: existingCard.listId, to: newListId }; + const movedToNewList = Boolean(newListId && existingCard.listId !== newListId); + const currentWebhookListPublicId = movedToNewList + ? input.listPublicId! + : existingCard.list.publicId; + const currentWebhookListName = movedToNewList + ? newList?.name ?? card.listName + : existingCard.list.name; + + if (movedToNewList) { + webhookChanges.listId = { + from: existingCard.list.publicId, + to: input.listPublicId!, + }; } // Fire webhooks (non-blocking) @@ -1046,20 +1066,18 @@ export const cardRouter = createTRPCRouter({ ctx.db, card.workspaceId, createCardWebhookPayload( - newListId && existingCard.listId !== newListId - ? "card.moved" - : "card.updated", + movedToNewList ? "card.moved" : "card.updated", { id: String(result.id), title: result.title, description: result.description, dueDate: result.dueDate, - listId: String(newListId ?? existingCard.listId), + listId: currentWebhookListPublicId, }, { boardId: card.boardPublicId, boardName: card.boardName, - listName: card.listName, + listName: currentWebhookListName, user: ctx.user ? { id: ctx.user.id, name: ctx.user.name } : undefined, @@ -1149,12 +1167,12 @@ export const cardRouter = createTRPCRouter({ title: fullCard.title, description: fullCard.description, dueDate: fullCard.dueDate, - listId: String(fullCard.listId), + listId: fullCard.list.publicId, }, { boardId: card.boardPublicId, boardName: card.boardName, - listName: card.listName, + listName: fullCard.list.name, user: ctx.user ? { id: ctx.user.id, name: ctx.user.name } : undefined, diff --git a/packages/api/src/utils/webhook.test.ts b/packages/api/src/utils/webhook.test.ts index 55574a1b..fee44d04 100644 --- a/packages/api/src/utils/webhook.test.ts +++ b/packages/api/src/utils/webhook.test.ts @@ -1,9 +1,20 @@ import { describe, it, expect, vi, beforeEach, afterEach } from "vitest"; +const { mockLogger } = vi.hoisted(() => ({ + mockLogger: { + error: vi.fn(), + info: vi.fn(), + }, +})); + vi.mock("@kan/db/repository/webhook.repo", () => ({ getActiveByWorkspaceId: vi.fn(), })); +vi.mock("@kan/logger", () => ({ + createLogger: vi.fn(() => mockLogger), +})); + import * as webhookRepo from "@kan/db/repository/webhook.repo"; import { sendWebhookToUrl, @@ -155,6 +166,33 @@ describe("webhook utilities", () => { title: { from: "Old Title", to: "Updated Title" }, }); }); + + it("preserves public list IDs in moved payloads", () => { + const payload = createCardWebhookPayload( + "card.moved", + { + id: "card-123", + title: "Moved Card", + listId: "list-public-done", + }, + { + boardId: "board-789", + listName: "Done", + changes: { + listId: { from: "list-public-backlog", to: "list-public-done" }, + }, + }, + ); + + expect(payload.data.card.listId).toBe("list-public-done"); + expect(payload.data.list).toEqual({ + id: "list-public-done", + name: "Done", + }); + expect(payload.data.changes).toEqual({ + listId: { from: "list-public-backlog", to: "list-public-done" }, + }); + }); }); describe("sendWebhookToUrl", () => { @@ -434,8 +472,6 @@ describe("webhook utilities", () => { }); it("continues sending to other webhooks when one fails", async () => { - const consoleSpy = vi.spyOn(console, "error").mockImplementation(() => {}); - mockGetActiveByWorkspaceId.mockResolvedValueOnce([ { id: 1, @@ -462,11 +498,15 @@ describe("webhook utilities", () => { await sendWebhooksForWorkspace(mockDb, 1, mockPayload); expect(global.fetch).toHaveBeenCalledTimes(2); - expect(consoleSpy).toHaveBeenCalledWith( - expect.stringContaining("Webhook delivery failed"), + expect(mockLogger.error).toHaveBeenCalledWith( + expect.objectContaining({ + url: "https://example.com/webhook1", + event: "card.created", + error: "500 Error", + statusCode: 500, + }), + "Webhook delivery failed", ); - - consoleSpy.mockRestore(); }); it("handles empty webhook list", async () => { @@ -478,9 +518,6 @@ describe("webhook utilities", () => { }); it("catches and logs DB errors without throwing", async () => { - const consoleSpy = vi - .spyOn(console, "error") - .mockImplementation(() => {}); mockGetActiveByWorkspaceId.mockRejectedValueOnce( new Error("DB connection failed"), ); @@ -490,12 +527,13 @@ describe("webhook utilities", () => { sendWebhooksForWorkspace(mockDb, 1, mockPayload), ).resolves.toBeUndefined(); - expect(consoleSpy).toHaveBeenCalledWith( - "Failed to send webhooks for workspace:", - expect.any(Error), + expect(mockLogger.error).toHaveBeenCalledWith( + expect.objectContaining({ + err: expect.any(Error), + workspaceId: 1, + }), + "Failed to send webhooks for workspace", ); - - consoleSpy.mockRestore(); }); }); diff --git a/packages/db/src/repository/card.repo.ts b/packages/db/src/repository/card.repo.ts index 431d00ec..f20d5500 100644 --- a/packages/db/src/repository/card.repo.ts +++ b/packages/db/src/repository/card.repo.ts @@ -241,6 +241,14 @@ export const getByPublicId = (db: dbClient, cardPublicId: string) => { listId: true, dueDate: true, }, + with: { + list: { + columns: { + publicId: true, + name: true, + }, + }, + }, where: eq(cards.publicId, cardPublicId), }); }; @@ -945,7 +953,7 @@ export const getWorkspaceAndCardIdByCardPublicId = async ( where: and(eq(cards.publicId, cardPublicId), isNull(cards.deletedAt)), with: { list: { - columns: { name: true }, + columns: { name: true, publicId: true }, with: { board: { columns: { @@ -966,6 +974,7 @@ export const getWorkspaceAndCardIdByCardPublicId = async ( createdBy: result.createdBy, workspaceId: result.list.board.workspaceId, workspaceVisibility: result.list.board.visibility, + listPublicId: result.list.publicId, listName: result.list.name, boardPublicId: result.list.board.publicId, boardName: result.list.board.name, diff --git a/packages/db/src/repository/list.repo.ts b/packages/db/src/repository/list.repo.ts index 1f4ff3d4..33d7ef30 100644 --- a/packages/db/src/repository/list.repo.ts +++ b/packages/db/src/repository/list.repo.ts @@ -209,10 +209,12 @@ export const getByPublicId = async (db: dbClient, listPublicId: string) => { return db.query.lists.findFirst({ columns: { id: true, + publicId: true, + name: true, boardId: true, index: true, }, - where: eq(lists.publicId, listPublicId), + where: and(eq(lists.publicId, listPublicId), isNull(lists.deletedAt)), }); }; @@ -433,6 +435,7 @@ export const getWorkspaceAndListIdByListPublicId = async ( return result ? { id: result.id, + publicId: listPublicId, name: result.name, createdBy: result.createdBy, workspaceId: result.board.workspaceId,