fix: use stable public ids for card webhook (#463)
Signed-off-by: Vikram Vaswani <2571660+vvaswani@users.noreply.github.com>
This commit is contained in:
@@ -189,7 +189,7 @@ export const cardRouter = createTRPCRouter({
|
|||||||
title: input.title,
|
title: input.title,
|
||||||
description: input.description,
|
description: input.description,
|
||||||
dueDate: input.dueDate ?? null,
|
dueDate: input.dueDate ?? null,
|
||||||
listId: String(newCard.listId),
|
listId: list.publicId,
|
||||||
},
|
},
|
||||||
{
|
{
|
||||||
boardId: list.boardPublicId,
|
boardId: list.boardPublicId,
|
||||||
@@ -888,9 +888,18 @@ export const cardRouter = createTRPCRouter({
|
|||||||
);
|
);
|
||||||
|
|
||||||
let newListId: number | undefined;
|
let newListId: number | undefined;
|
||||||
|
let newList:
|
||||||
|
| {
|
||||||
|
id: number;
|
||||||
|
publicId: string;
|
||||||
|
name: string;
|
||||||
|
boardId: number;
|
||||||
|
index: number;
|
||||||
|
}
|
||||||
|
| undefined;
|
||||||
|
|
||||||
if (input.listPublicId) {
|
if (input.listPublicId) {
|
||||||
const newList = await listRepo.getByPublicId(
|
newList = await listRepo.getByPublicId(
|
||||||
ctx.db,
|
ctx.db,
|
||||||
input.listPublicId,
|
input.listPublicId,
|
||||||
);
|
);
|
||||||
@@ -1037,8 +1046,19 @@ export const cardRouter = createTRPCRouter({
|
|||||||
) {
|
) {
|
||||||
webhookChanges.dueDate = { from: previousDueDate, to: input.dueDate };
|
webhookChanges.dueDate = { from: previousDueDate, to: input.dueDate };
|
||||||
}
|
}
|
||||||
if (newListId && existingCard.listId !== newListId) {
|
const movedToNewList = Boolean(newListId && existingCard.listId !== newListId);
|
||||||
webhookChanges.listId = { from: existingCard.listId, to: 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)
|
// Fire webhooks (non-blocking)
|
||||||
@@ -1046,20 +1066,18 @@ export const cardRouter = createTRPCRouter({
|
|||||||
ctx.db,
|
ctx.db,
|
||||||
card.workspaceId,
|
card.workspaceId,
|
||||||
createCardWebhookPayload(
|
createCardWebhookPayload(
|
||||||
newListId && existingCard.listId !== newListId
|
movedToNewList ? "card.moved" : "card.updated",
|
||||||
? "card.moved"
|
|
||||||
: "card.updated",
|
|
||||||
{
|
{
|
||||||
id: String(result.id),
|
id: String(result.id),
|
||||||
title: result.title,
|
title: result.title,
|
||||||
description: result.description,
|
description: result.description,
|
||||||
dueDate: result.dueDate,
|
dueDate: result.dueDate,
|
||||||
listId: String(newListId ?? existingCard.listId),
|
listId: currentWebhookListPublicId,
|
||||||
},
|
},
|
||||||
{
|
{
|
||||||
boardId: card.boardPublicId,
|
boardId: card.boardPublicId,
|
||||||
boardName: card.boardName,
|
boardName: card.boardName,
|
||||||
listName: card.listName,
|
listName: currentWebhookListName,
|
||||||
user: ctx.user
|
user: ctx.user
|
||||||
? { id: ctx.user.id, name: ctx.user.name }
|
? { id: ctx.user.id, name: ctx.user.name }
|
||||||
: undefined,
|
: undefined,
|
||||||
@@ -1149,12 +1167,12 @@ export const cardRouter = createTRPCRouter({
|
|||||||
title: fullCard.title,
|
title: fullCard.title,
|
||||||
description: fullCard.description,
|
description: fullCard.description,
|
||||||
dueDate: fullCard.dueDate,
|
dueDate: fullCard.dueDate,
|
||||||
listId: String(fullCard.listId),
|
listId: fullCard.list.publicId,
|
||||||
},
|
},
|
||||||
{
|
{
|
||||||
boardId: card.boardPublicId,
|
boardId: card.boardPublicId,
|
||||||
boardName: card.boardName,
|
boardName: card.boardName,
|
||||||
listName: card.listName,
|
listName: fullCard.list.name,
|
||||||
user: ctx.user
|
user: ctx.user
|
||||||
? { id: ctx.user.id, name: ctx.user.name }
|
? { id: ctx.user.id, name: ctx.user.name }
|
||||||
: undefined,
|
: undefined,
|
||||||
|
|||||||
@@ -1,9 +1,20 @@
|
|||||||
import { describe, it, expect, vi, beforeEach, afterEach } from "vitest";
|
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", () => ({
|
vi.mock("@kan/db/repository/webhook.repo", () => ({
|
||||||
getActiveByWorkspaceId: vi.fn(),
|
getActiveByWorkspaceId: vi.fn(),
|
||||||
}));
|
}));
|
||||||
|
|
||||||
|
vi.mock("@kan/logger", () => ({
|
||||||
|
createLogger: vi.fn(() => mockLogger),
|
||||||
|
}));
|
||||||
|
|
||||||
import * as webhookRepo from "@kan/db/repository/webhook.repo";
|
import * as webhookRepo from "@kan/db/repository/webhook.repo";
|
||||||
import {
|
import {
|
||||||
sendWebhookToUrl,
|
sendWebhookToUrl,
|
||||||
@@ -155,6 +166,33 @@ describe("webhook utilities", () => {
|
|||||||
title: { from: "Old Title", to: "Updated Title" },
|
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", () => {
|
describe("sendWebhookToUrl", () => {
|
||||||
@@ -434,8 +472,6 @@ describe("webhook utilities", () => {
|
|||||||
});
|
});
|
||||||
|
|
||||||
it("continues sending to other webhooks when one fails", async () => {
|
it("continues sending to other webhooks when one fails", async () => {
|
||||||
const consoleSpy = vi.spyOn(console, "error").mockImplementation(() => {});
|
|
||||||
|
|
||||||
mockGetActiveByWorkspaceId.mockResolvedValueOnce([
|
mockGetActiveByWorkspaceId.mockResolvedValueOnce([
|
||||||
{
|
{
|
||||||
id: 1,
|
id: 1,
|
||||||
@@ -462,11 +498,15 @@ describe("webhook utilities", () => {
|
|||||||
await sendWebhooksForWorkspace(mockDb, 1, mockPayload);
|
await sendWebhooksForWorkspace(mockDb, 1, mockPayload);
|
||||||
|
|
||||||
expect(global.fetch).toHaveBeenCalledTimes(2);
|
expect(global.fetch).toHaveBeenCalledTimes(2);
|
||||||
expect(consoleSpy).toHaveBeenCalledWith(
|
expect(mockLogger.error).toHaveBeenCalledWith(
|
||||||
expect.stringContaining("Webhook delivery failed"),
|
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 () => {
|
it("handles empty webhook list", async () => {
|
||||||
@@ -478,9 +518,6 @@ describe("webhook utilities", () => {
|
|||||||
});
|
});
|
||||||
|
|
||||||
it("catches and logs DB errors without throwing", async () => {
|
it("catches and logs DB errors without throwing", async () => {
|
||||||
const consoleSpy = vi
|
|
||||||
.spyOn(console, "error")
|
|
||||||
.mockImplementation(() => {});
|
|
||||||
mockGetActiveByWorkspaceId.mockRejectedValueOnce(
|
mockGetActiveByWorkspaceId.mockRejectedValueOnce(
|
||||||
new Error("DB connection failed"),
|
new Error("DB connection failed"),
|
||||||
);
|
);
|
||||||
@@ -490,12 +527,13 @@ describe("webhook utilities", () => {
|
|||||||
sendWebhooksForWorkspace(mockDb, 1, mockPayload),
|
sendWebhooksForWorkspace(mockDb, 1, mockPayload),
|
||||||
).resolves.toBeUndefined();
|
).resolves.toBeUndefined();
|
||||||
|
|
||||||
expect(consoleSpy).toHaveBeenCalledWith(
|
expect(mockLogger.error).toHaveBeenCalledWith(
|
||||||
"Failed to send webhooks for workspace:",
|
expect.objectContaining({
|
||||||
expect.any(Error),
|
err: expect.any(Error),
|
||||||
|
workspaceId: 1,
|
||||||
|
}),
|
||||||
|
"Failed to send webhooks for workspace",
|
||||||
);
|
);
|
||||||
|
|
||||||
consoleSpy.mockRestore();
|
|
||||||
});
|
});
|
||||||
});
|
});
|
||||||
|
|
||||||
|
|||||||
@@ -241,6 +241,14 @@ export const getByPublicId = (db: dbClient, cardPublicId: string) => {
|
|||||||
listId: true,
|
listId: true,
|
||||||
dueDate: true,
|
dueDate: true,
|
||||||
},
|
},
|
||||||
|
with: {
|
||||||
|
list: {
|
||||||
|
columns: {
|
||||||
|
publicId: true,
|
||||||
|
name: true,
|
||||||
|
},
|
||||||
|
},
|
||||||
|
},
|
||||||
where: eq(cards.publicId, cardPublicId),
|
where: eq(cards.publicId, cardPublicId),
|
||||||
});
|
});
|
||||||
};
|
};
|
||||||
@@ -945,7 +953,7 @@ export const getWorkspaceAndCardIdByCardPublicId = async (
|
|||||||
where: and(eq(cards.publicId, cardPublicId), isNull(cards.deletedAt)),
|
where: and(eq(cards.publicId, cardPublicId), isNull(cards.deletedAt)),
|
||||||
with: {
|
with: {
|
||||||
list: {
|
list: {
|
||||||
columns: { name: true },
|
columns: { name: true, publicId: true },
|
||||||
with: {
|
with: {
|
||||||
board: {
|
board: {
|
||||||
columns: {
|
columns: {
|
||||||
@@ -966,6 +974,7 @@ export const getWorkspaceAndCardIdByCardPublicId = async (
|
|||||||
createdBy: result.createdBy,
|
createdBy: result.createdBy,
|
||||||
workspaceId: result.list.board.workspaceId,
|
workspaceId: result.list.board.workspaceId,
|
||||||
workspaceVisibility: result.list.board.visibility,
|
workspaceVisibility: result.list.board.visibility,
|
||||||
|
listPublicId: result.list.publicId,
|
||||||
listName: result.list.name,
|
listName: result.list.name,
|
||||||
boardPublicId: result.list.board.publicId,
|
boardPublicId: result.list.board.publicId,
|
||||||
boardName: result.list.board.name,
|
boardName: result.list.board.name,
|
||||||
|
|||||||
@@ -209,10 +209,12 @@ export const getByPublicId = async (db: dbClient, listPublicId: string) => {
|
|||||||
return db.query.lists.findFirst({
|
return db.query.lists.findFirst({
|
||||||
columns: {
|
columns: {
|
||||||
id: true,
|
id: true,
|
||||||
|
publicId: true,
|
||||||
|
name: true,
|
||||||
boardId: true,
|
boardId: true,
|
||||||
index: 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
|
return result
|
||||||
? {
|
? {
|
||||||
id: result.id,
|
id: result.id,
|
||||||
|
publicId: listPublicId,
|
||||||
name: result.name,
|
name: result.name,
|
||||||
createdBy: result.createdBy,
|
createdBy: result.createdBy,
|
||||||
workspaceId: result.board.workspaceId,
|
workspaceId: result.board.workspaceId,
|
||||||
|
|||||||
Reference in New Issue
Block a user