From a00a64bb6e5451d2b4b9f93e748b9e25a37cf171 Mon Sep 17 00:00:00 2001 From: Sam Rose Date: Thu, 10 Oct 2024 14:56:38 +0100 Subject: [PATCH] Properly clean up row actions on table deletion. --- .../server/src/api/controllers/table/index.ts | 1 + .../src/api/routes/tests/rowAction.spec.ts | 40 +++++++++++++++++++ packages/server/src/sdk/app/rowActions.ts | 15 +++++++ .../src/tests/utilities/api/automation.ts | 22 +++++++++- 4 files changed, 77 insertions(+), 1 deletion(-) diff --git a/packages/server/src/api/controllers/table/index.ts b/packages/server/src/api/controllers/table/index.ts index 404f82e57c..efe1a88e4a 100644 --- a/packages/server/src/api/controllers/table/index.ts +++ b/packages/server/src/api/controllers/table/index.ts @@ -139,6 +139,7 @@ export async function save(ctx: UserCtx) { export async function destroy(ctx: UserCtx) { const appId = ctx.appId const tableId = ctx.params.tableId + await sdk.rowActions.deleteAll(tableId) const deletedTable = await pickApi({ tableId }).destroy(ctx) await events.table.deleted(deletedTable) ctx.eventEmitter && diff --git a/packages/server/src/api/routes/tests/rowAction.spec.ts b/packages/server/src/api/routes/tests/rowAction.spec.ts index fb59ce73c3..7140f3486e 100644 --- a/packages/server/src/api/routes/tests/rowAction.spec.ts +++ b/packages/server/src/api/routes/tests/rowAction.spec.ts @@ -1041,4 +1041,44 @@ describe("/rowsActions", () => { ) }) }) + + describe("scenarios", () => { + // https://linear.app/budibase/issue/BUDI-8717/ + it("should not brick the app when deleting a table with row actions", async () => { + const view = await config.api.viewV2.create({ + tableId, + name: generator.guid(), + schema: { + name: { visible: true }, + }, + }) + + const rowAction = await config.api.rowAction.save(tableId, { + name: generator.guid(), + }) + + await config.api.rowAction.setViewPermission( + tableId, + view.id, + rowAction.id + ) + + let actionsResp = await config.api.rowAction.find(tableId) + expect(actionsResp.actions[rowAction.id]).toEqual({ + ...rowAction, + allowedSources: [tableId, view.id], + }) + + const table = await config.api.table.get(tableId) + await config.api.table.destroy(table._id!, table._rev!) + + // In the bug reported by Conor, when a delete got deleted its row action + // document was not being cleaned up. This meant there existed code paths + // that would find it and try to reference the tables within it, resulting + // in errors. + await config.api.automation.fetchEnriched({ + status: 200, + }) + }) + }) }) diff --git a/packages/server/src/sdk/app/rowActions.ts b/packages/server/src/sdk/app/rowActions.ts index 418a906c00..958401b8b9 100644 --- a/packages/server/src/sdk/app/rowActions.ts +++ b/packages/server/src/sdk/app/rowActions.ts @@ -1,5 +1,6 @@ import { context, docIds, HTTPError, utils } from "@budibase/backend-core" import { + Automation, AutomationTriggerStepId, SEPARATOR, TableRowActions, @@ -105,6 +106,20 @@ export async function getAll(tableId: string) { return await db.get(rowActionsId) } +export async function deleteAll(tableId: string) { + const db = context.getAppDB() + + const doc = await getAll(tableId) + const automationIds = Object.values(doc.actions).map(a => a.automationId) + const automations = await db.getMultiple(automationIds) + + for (const automation of automations) { + await sdk.automations.remove(automation._id!, automation._rev!) + } + + await db.remove(doc) +} + export async function docExists(tableId: string) { const db = context.getAppDB() const rowActionsId = generateRowActionsID(tableId) diff --git a/packages/server/src/tests/utilities/api/automation.ts b/packages/server/src/tests/utilities/api/automation.ts index 61bd915647..11c041d52b 100644 --- a/packages/server/src/tests/utilities/api/automation.ts +++ b/packages/server/src/tests/utilities/api/automation.ts @@ -1,4 +1,4 @@ -import { Automation } from "@budibase/types" +import { Automation, FetchAutomationResponse } from "@budibase/types" import { Expectations, TestAPI } from "./base" export class AutomationAPI extends TestAPI { @@ -14,6 +14,26 @@ export class AutomationAPI extends TestAPI { ) return result } + + fetch = async ( + expectations?: Expectations + ): Promise => { + return await this._get(`/api/automations`, { + expectations, + }) + } + + fetchEnriched = async ( + expectations?: Expectations + ): Promise => { + return await this._get( + `/api/automations?enrich=true`, + { + expectations, + } + ) + } + post = async ( body: Automation, expectations?: Expectations