From 10c09f00a84d8e80be6d5c1e4d6fa1fff52f4c5a Mon Sep 17 00:00:00 2001 From: Dhruwang Date: Tue, 12 May 2026 16:11:45 +0530 Subject: [PATCH] refactor(dashboards): address review on removeWidgetFromDashboard - Drop the prisma.$transaction wrapper; find + delete is two sequential steps, doesn't need a transaction. - Drop the redundant ResourceNotFoundError catch branch; the trailing `throw error` already lets it bubble. - Let action-client infer ctx / parsedInput types. Tests: cover the two catch branches (Prisma -> DatabaseError, unknown rethrow) so the new function is fully line-covered. --- .../modules/ee/analysis/dashboards/actions.ts | 46 ++++++++----------- .../dashboards/lib/dashboards.test.ts | 20 ++++++++ .../ee/analysis/dashboards/lib/dashboards.ts | 21 ++++----- 3 files changed, 46 insertions(+), 41 deletions(-) diff --git a/apps/web/modules/ee/analysis/dashboards/actions.ts b/apps/web/modules/ee/analysis/dashboards/actions.ts index 00185c71ef..2a63ebe1a5 100644 --- a/apps/web/modules/ee/analysis/dashboards/actions.ts +++ b/apps/web/modules/ee/analysis/dashboards/actions.ts @@ -334,34 +334,24 @@ const ZRemoveWidgetFromDashboardAction = z.object({ export const removeWidgetFromDashboardAction = authenticatedActionClient .inputSchema(ZRemoveWidgetFromDashboardAction) .action( - withAuditLogging( - "deleted", - "dashboardWidget", - async ({ - ctx, - parsedInput, - }: { - ctx: AuthenticatedActionClientCtx; - parsedInput: z.infer; - }) => { - const { organizationId, workspaceId } = await checkWorkspaceAccess( - ctx.user.id, - parsedInput.workspaceId, - "readWrite" - ); - await checkDashboardsEnabled(organizationId); + withAuditLogging("deleted", "dashboardWidget", async ({ ctx, parsedInput }) => { + const { organizationId, workspaceId } = await checkWorkspaceAccess( + ctx.user.id, + parsedInput.workspaceId, + "readWrite" + ); + await checkDashboardsEnabled(organizationId); - const widget = await removeWidgetFromDashboard( - parsedInput.dashboardId, - workspaceId, - parsedInput.widgetId - ); + const widget = await removeWidgetFromDashboard( + parsedInput.dashboardId, + workspaceId, + parsedInput.widgetId + ); - ctx.auditLoggingCtx.organizationId = organizationId; - ctx.auditLoggingCtx.workspaceId = workspaceId; - ctx.auditLoggingCtx.dashboardWidgetId = widget.id; - ctx.auditLoggingCtx.oldObject = widget; - return { success: true }; - } - ) + ctx.auditLoggingCtx.organizationId = organizationId; + ctx.auditLoggingCtx.workspaceId = workspaceId; + ctx.auditLoggingCtx.dashboardWidgetId = widget.id; + ctx.auditLoggingCtx.oldObject = widget; + return { success: true }; + }) ); diff --git a/apps/web/modules/ee/analysis/dashboards/lib/dashboards.test.ts b/apps/web/modules/ee/analysis/dashboards/lib/dashboards.test.ts index be1382088e..74e75e238e 100644 --- a/apps/web/modules/ee/analysis/dashboards/lib/dashboards.test.ts +++ b/apps/web/modules/ee/analysis/dashboards/lib/dashboards.test.ts @@ -48,6 +48,7 @@ vi.mock("@formbricks/database", () => { findFirst: vi.fn(), findMany: vi.fn(), }, + dashboardWidget: txWidget, $transaction: vi.fn((cb: any) => cb({ dashboard: txDash, chart: txChart, dashboardWidget: txWidget })), }, }; @@ -704,5 +705,24 @@ describe("Dashboard Service", () => { ).rejects.toMatchObject({ name: "ResourceNotFoundError", resourceType: "DashboardWidget" }); expect(mockTxWidget.delete).not.toHaveBeenCalled(); }); + + test("wraps Prisma errors in DatabaseError", async () => { + mockTxWidget.findFirst.mockRejectedValue(makePrismaError("P9999")); + const { removeWidgetFromDashboard } = await import("./dashboards"); + + await expect( + removeWidgetFromDashboard(mockDashboardId, mockWorkspaceId, mockWidgetId) + ).rejects.toMatchObject({ name: "DatabaseError" }); + }); + + test("rethrows unknown errors", async () => { + const error = new Error("boom"); + mockTxWidget.findFirst.mockRejectedValue(error); + const { removeWidgetFromDashboard } = await import("./dashboards"); + + await expect(removeWidgetFromDashboard(mockDashboardId, mockWorkspaceId, mockWidgetId)).rejects.toBe( + error + ); + }); }); }); diff --git a/apps/web/modules/ee/analysis/dashboards/lib/dashboards.ts b/apps/web/modules/ee/analysis/dashboards/lib/dashboards.ts index 246788c265..040caafbe4 100644 --- a/apps/web/modules/ee/analysis/dashboards/lib/dashboards.ts +++ b/apps/web/modules/ee/analysis/dashboards/lib/dashboards.ts @@ -309,21 +309,16 @@ export const removeWidgetFromDashboard = async ( validateInputs([dashboardId, ZId], [workspaceId, ZId], [widgetId, ZId]); try { - return await prisma.$transaction(async (tx) => { - const widget = await tx.dashboardWidget.findFirst({ - where: { id: widgetId, dashboard: { id: dashboardId, workspaceId } }, - }); - - if (!widget) { - throw new ResourceNotFoundError("DashboardWidget", widgetId); - } - - return tx.dashboardWidget.delete({ where: { id: widgetId } }); + const widget = await prisma.dashboardWidget.findFirst({ + where: { id: widgetId, dashboard: { id: dashboardId, workspaceId } }, }); - } catch (error) { - if (error instanceof ResourceNotFoundError) { - throw error; + + if (!widget) { + throw new ResourceNotFoundError("DashboardWidget", widgetId); } + + return await prisma.dashboardWidget.delete({ where: { id: widgetId } }); + } catch (error) { if (error instanceof Prisma.PrismaClientKnownRequestError) { throw new DatabaseError(error.message); }