From 94fc6d8b0fca5b31e8628628f8ea076b7a48aba8 Mon Sep 17 00:00:00 2001 From: Dean Date: Tue, 11 Jul 2023 12:47:17 +0100 Subject: [PATCH 1/8] Merge commit --- .../ButtonActionEditor/ButtonActionDrawer.svelte | 11 +++++++++-- 1 file changed, 9 insertions(+), 2 deletions(-) diff --git a/packages/builder/src/components/design/settings/controls/ButtonActionEditor/ButtonActionDrawer.svelte b/packages/builder/src/components/design/settings/controls/ButtonActionEditor/ButtonActionDrawer.svelte index aa8e1af950..cd7b40aaa0 100644 --- a/packages/builder/src/components/design/settings/controls/ButtonActionEditor/ButtonActionDrawer.svelte +++ b/packages/builder/src/components/design/settings/controls/ButtonActionEditor/ButtonActionDrawer.svelte @@ -175,6 +175,11 @@ } return allBindings } + + const toDisplay = eventKey => { + const type = actionTypes.find(action => action.name == eventKey) + return type?.displayName || type?.name + } @@ -200,7 +205,9 @@ @@ -231,7 +238,7 @@ >
- {index + 1}. {action[EVENT_TYPE_KEY]} + {index + 1}. {toDisplay(action[EVENT_TYPE_KEY])}
Date: Fri, 14 Jul 2023 09:11:34 +0100 Subject: [PATCH 2/8] Binding selection fixes, delete controller refactor and some fixes --- .../builder/src/builderStore/dataBinding.js | 2 + .../actions/DeleteRow.svelte | 78 +++++++++++-------- .../controls/ButtonActionEditor/manifest.json | 1 + .../src/components/app/table/Table.svelte | 8 ++ packages/client/src/utils/buttonActions.js | 50 ++++++++++-- .../server/src/api/controllers/public/rows.ts | 2 +- .../server/src/api/controllers/row/index.ts | 17 ++++ 7 files changed, 117 insertions(+), 41 deletions(-) diff --git a/packages/builder/src/builderStore/dataBinding.js b/packages/builder/src/builderStore/dataBinding.js index e9c8643bce..bbe116721a 100644 --- a/packages/builder/src/builderStore/dataBinding.js +++ b/packages/builder/src/builderStore/dataBinding.js @@ -491,6 +491,7 @@ const getSelectedRowsBindings = asset => { readableBinding: `${table._instanceName}.Selected rows`, category: "Selected rows", icon: "ViewRow", + display: { name: table._instanceName }, })) ) @@ -506,6 +507,7 @@ const getSelectedRowsBindings = asset => { )}.${makePropSafe("selectedRows")}`, readableBinding: `${block._instanceName}.Selected rows`, category: "Selected rows", + display: { name: block._instanceName }, })) ) } diff --git a/packages/builder/src/components/design/settings/controls/ButtonActionEditor/actions/DeleteRow.svelte b/packages/builder/src/components/design/settings/controls/ButtonActionEditor/actions/DeleteRow.svelte index 109eb9a956..e4a5f171ff 100644 --- a/packages/builder/src/components/design/settings/controls/ButtonActionEditor/actions/DeleteRow.svelte +++ b/packages/builder/src/components/design/settings/controls/ButtonActionEditor/actions/DeleteRow.svelte @@ -1,5 +1,5 @@
- - Please specify one or more rows to delete. +
+ + + {/if} +
diff --git a/packages/builder/src/components/design/settings/controls/ButtonActionEditor/manifest.json b/packages/builder/src/components/design/settings/controls/ButtonActionEditor/manifest.json index 2ec7235c59..6ed545f541 100644 --- a/packages/builder/src/components/design/settings/controls/ButtonActionEditor/manifest.json +++ b/packages/builder/src/components/design/settings/controls/ButtonActionEditor/manifest.json @@ -24,6 +24,7 @@ }, { "name": "Delete Row", + "displayName": "Delete Rows", "type": "data", "component": "DeleteRow" }, diff --git a/packages/client/src/components/app/table/Table.svelte b/packages/client/src/components/app/table/Table.svelte index 248151a7a2..0ed76317db 100644 --- a/packages/client/src/components/app/table/Table.svelte +++ b/packages/client/src/components/app/table/Table.svelte @@ -47,6 +47,14 @@ ) } + // If the data changes, double check that the selected elements are still present. + $: if (data) { + let rowIds = data.map(row => row._id) + if (rowIds.length) { + selectedRows = selectedRows.filter(row => rowIds.includes(row._id)) + } + } + const getFields = (schema, customColumns, showAutoColumns) => { // Check for an invalid column selection let invalid = false diff --git a/packages/client/src/utils/buttonActions.js b/packages/client/src/utils/buttonActions.js index 68f312f0ad..97d0d827bd 100644 --- a/packages/client/src/utils/buttonActions.js +++ b/packages/client/src/utils/buttonActions.js @@ -101,13 +101,47 @@ const fetchRowHandler = async action => { } } -const deleteRowHandler = async action => { - const { tableId, revId, rowId, notificationOverride } = action.parameters - if (tableId && rowId) { +const deleteRowHandler = async (action, context) => { + const { tableId, rowId: rowConfig, notificationOverride } = action.parameters + + if (tableId && rowConfig) { try { - await API.deleteRow({ tableId, rowId, revId }) + let requestConfig + + let parsedRowConfig = [] + if (typeof rowConfig === "string") { + try { + parsedRowConfig = JSON.parse(rowConfig) + } catch (e) { + parsedRowConfig = rowConfig + .split(",") + .map(id => id.trim()) + .filter(id => id) + } + } else { + parsedRowConfig = rowConfig + } + + if ( + typeof parsedRowConfig === "object" && + parsedRowConfig.constructor === Object + ) { + requestConfig = [parsedRowConfig] + } else if (Array.isArray(parsedRowConfig)) { + requestConfig = parsedRowConfig + } + + if (!requestConfig.length) { + notificationStore.actions.warning("No valid rows were supplied") + return false + } + + const resp = await API.deleteRows({ tableId, rows: requestConfig }) + if (!notificationOverride) { - notificationStore.actions.success("Row deleted") + notificationStore.actions.success( + resp?.length == 1 ? "Row deleted" : `${resp.length} Rows deleted` + ) } // Refresh related datasources @@ -115,8 +149,10 @@ const deleteRowHandler = async action => { invalidateRelationships: true, }) } catch (error) { - // Abort next actions - return false + console.error(error) + notificationStore.actions.error( + "An error occurred while executing the query" + ) } } } diff --git a/packages/server/src/api/controllers/public/rows.ts b/packages/server/src/api/controllers/public/rows.ts index df856f1fe0..39cf85a2a3 100644 --- a/packages/server/src/api/controllers/public/rows.ts +++ b/packages/server/src/api/controllers/public/rows.ts @@ -5,7 +5,7 @@ import { convertBookmark } from "../../../utilities" // makes sure that the user doesn't need to pass in the type, tableId or _id params for // the call to be correct -function fixRow(row: Row, params: any) { +export function fixRow(row: Row, params: any) { if (!params || !row) { return row } diff --git a/packages/server/src/api/controllers/row/index.ts b/packages/server/src/api/controllers/row/index.ts index 91270429a4..6a969affaf 100644 --- a/packages/server/src/api/controllers/row/index.ts +++ b/packages/server/src/api/controllers/row/index.ts @@ -5,6 +5,8 @@ import { isExternalTable } from "../../../integrations/utils" import { Ctx } from "@budibase/types" import * as utils from "./utils" import { gridSocket } from "../../../websockets" +import { addRev } from "../public/utils" +import { fixRow } from "../public/rows" function pickApi(tableId: any) { if (isExternalTable(tableId)) { @@ -88,7 +90,22 @@ export async function destroy(ctx: any) { const inputs = ctx.request.body const tableId = utils.getTableId(ctx) let response, row + if (inputs.rows) { + const targetRows = inputs.rows.map( + (row: { [key: string]: string | string }) => { + let processedRow = typeof row == "string" ? { _id: row } : row + return !processedRow._rev + ? addRev(fixRow(processedRow, ctx.params), tableId) + : fixRow(processedRow, ctx.params) + } + ) + + const rowDeletes = await Promise.all(targetRows) + if (rowDeletes) { + ctx.request.body.rows = rowDeletes + } + let { rows } = await quotas.addQuery( () => pickApi(tableId).bulkDestroy(ctx), { From 7f3dfc8bbaab834ad655654d4d366b98070fdda0 Mon Sep 17 00:00:00 2001 From: Dean Date: Fri, 14 Jul 2023 11:42:22 +0100 Subject: [PATCH 3/8] Linting --- packages/client/src/utils/buttonActions.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/client/src/utils/buttonActions.js b/packages/client/src/utils/buttonActions.js index 97d0d827bd..653ed6a98c 100644 --- a/packages/client/src/utils/buttonActions.js +++ b/packages/client/src/utils/buttonActions.js @@ -101,7 +101,7 @@ const fetchRowHandler = async action => { } } -const deleteRowHandler = async (action, context) => { +const deleteRowHandler = async action => { const { tableId, rowId: rowConfig, notificationOverride } = action.parameters if (tableId && rowConfig) { From 3b36970c588fac417dbc79f2982a86c4aeba13a2 Mon Sep 17 00:00:00 2001 From: Dean Date: Mon, 24 Jul 2023 09:08:10 +0100 Subject: [PATCH 4/8] Review updates --- .../server/src/api/controllers/row/index.ts | 27 ++++++------- .../server/src/api/routes/tests/row.spec.ts | 40 +++++++++++++++++++ 2 files changed, 53 insertions(+), 14 deletions(-) diff --git a/packages/server/src/api/controllers/row/index.ts b/packages/server/src/api/controllers/row/index.ts index a1f5664754..7183aa6cd7 100644 --- a/packages/server/src/api/controllers/row/index.ts +++ b/packages/server/src/api/controllers/row/index.ts @@ -2,7 +2,7 @@ import { quotas } from "@budibase/pro" import * as internal from "./internal" import * as external from "./external" import { isExternalTable } from "../../../integrations/utils" -import { Ctx } from "@budibase/types" +import { Ctx, UserCtx, DeleteRowRequest, Row } from "@budibase/types" import * as utils from "./utils" import { gridSocket } from "../../../websockets" import { addRev } from "../public/utils" @@ -100,25 +100,24 @@ export async function find(ctx: any) { }) } -export async function destroy(ctx: any) { +export async function destroy(ctx: UserCtx) { const appId = ctx.appId - const inputs = ctx.request.body + const inputs: DeleteRowRequest = ctx.request.body + const tableId = utils.getTableId(ctx) let response, row - if (inputs.rows) { - const targetRows = inputs.rows.map( - (row: { [key: string]: string | string }) => { - let processedRow = typeof row == "string" ? { _id: row } : row - return !processedRow._rev - ? addRev(fixRow(processedRow, ctx.params), tableId) - : fixRow(processedRow, ctx.params) - } - ) + if ("rows" in inputs && Array.isArray(inputs?.rows)) { + const targetRows = inputs.rows.map(row => { + let processedRow: Row = typeof row == "string" ? { _id: row } : row + return !processedRow._rev + ? addRev(fixRow(processedRow, ctx.params), tableId) + : fixRow(processedRow, ctx.params) + }) - const rowDeletes = await Promise.all(targetRows) + const rowDeletes: Row[] = await Promise.all(targetRows) if (rowDeletes) { - ctx.request.body.rows = rowDeletes + inputs.rows = rowDeletes } let { rows } = await quotas.addQuery( diff --git a/packages/server/src/api/routes/tests/row.spec.ts b/packages/server/src/api/routes/tests/row.spec.ts index 8e99c30246..aef5ac57ac 100644 --- a/packages/server/src/api/routes/tests/row.spec.ts +++ b/packages/server/src/api/routes/tests/row.spec.ts @@ -517,6 +517,46 @@ describe("/rows", () => { await assertRowUsage(rowUsage - 2) await assertQueryUsage(queryUsage + 1) }) + + it("should be able to delete a variety of row set types", async () => { + const row1 = await config.createRow() + const row2 = await config.createRow() + const row3 = await config.createRow() + const rowUsage = await getRowUsage() + const queryUsage = await getQueryUsage() + + const res = await request + .delete(`/api/${table._id}/rows`) + .send({ + rows: [row1, row2._id, { _id: row3._id }], + }) + .set(config.defaultHeaders()) + .expect("Content-Type", /json/) + .expect(200) + + expect(res.body.length).toEqual(3) + await loadRow(row1._id!, table._id!, 404) + await assertRowUsage(rowUsage - 3) + await assertQueryUsage(queryUsage + 1) + }) + + it("should accept a valid row object and delete the row", async () => { + const row1 = await config.createRow() + const rowUsage = await getRowUsage() + const queryUsage = await getQueryUsage() + + const res = await request + .delete(`/api/${table._id}/rows`) + .send(row1) + .set(config.defaultHeaders()) + .expect("Content-Type", /json/) + .expect(200) + + expect(res.body.id).toEqual(row1._id) + await loadRow(row1._id!, table._id!, 404) + await assertRowUsage(rowUsage - 1) + await assertQueryUsage(queryUsage + 1) + }) }) describe("fetchView", () => { From 5691be3c4a2280b81aca664c8fdf6e5058a53496 Mon Sep 17 00:00:00 2001 From: Dean Date: Mon, 24 Jul 2023 09:15:13 +0100 Subject: [PATCH 5/8] Added missing types --- packages/types/src/documents/app/row.ts | 10 ++++++++++ 1 file changed, 10 insertions(+) diff --git a/packages/types/src/documents/app/row.ts b/packages/types/src/documents/app/row.ts index a2295c4a42..3a302589cc 100644 --- a/packages/types/src/documents/app/row.ts +++ b/packages/types/src/documents/app/row.ts @@ -32,3 +32,13 @@ export interface Row extends Document { tableId?: string [key: string]: any } + +export interface DeleteRows { + rows: (Row | string)[] +} + +export interface DeleteRow { + _id: string +} + +export type DeleteRowRequest = DeleteRows | DeleteRow From 4091dff6d3832f9889360972df1278fa9f13be4f Mon Sep 17 00:00:00 2001 From: Dean Date: Mon, 24 Jul 2023 10:30:29 +0100 Subject: [PATCH 6/8] PR Feedback --- packages/server/src/api/controllers/row/index.ts | 2 +- packages/types/src/api/web/app/index.ts | 1 + packages/types/src/api/web/app/row.ts | 11 +++++++++++ packages/types/src/documents/app/row.ts | 10 ---------- 4 files changed, 13 insertions(+), 11 deletions(-) create mode 100644 packages/types/src/api/web/app/row.ts diff --git a/packages/server/src/api/controllers/row/index.ts b/packages/server/src/api/controllers/row/index.ts index 7183aa6cd7..c217f06ad4 100644 --- a/packages/server/src/api/controllers/row/index.ts +++ b/packages/server/src/api/controllers/row/index.ts @@ -102,7 +102,7 @@ export async function find(ctx: any) { export async function destroy(ctx: UserCtx) { const appId = ctx.appId - const inputs: DeleteRowRequest = ctx.request.body + const inputs = ctx.request.body const tableId = utils.getTableId(ctx) let response, row diff --git a/packages/types/src/api/web/app/index.ts b/packages/types/src/api/web/app/index.ts index 9be15ecfe3..0b878f38de 100644 --- a/packages/types/src/api/web/app/index.ts +++ b/packages/types/src/api/web/app/index.ts @@ -1,2 +1,3 @@ export * from "./backup" export * from "./datasource" +export * from "./row" diff --git a/packages/types/src/api/web/app/row.ts b/packages/types/src/api/web/app/row.ts new file mode 100644 index 0000000000..f9623a3daf --- /dev/null +++ b/packages/types/src/api/web/app/row.ts @@ -0,0 +1,11 @@ +import { Row } from "../../../documents/app/row" + +export interface DeleteRows { + rows: (Row | string)[] +} + +export interface DeleteRow { + _id: string +} + +export type DeleteRowRequest = DeleteRows | DeleteRow diff --git a/packages/types/src/documents/app/row.ts b/packages/types/src/documents/app/row.ts index 3a302589cc..a2295c4a42 100644 --- a/packages/types/src/documents/app/row.ts +++ b/packages/types/src/documents/app/row.ts @@ -32,13 +32,3 @@ export interface Row extends Document { tableId?: string [key: string]: any } - -export interface DeleteRows { - rows: (Row | string)[] -} - -export interface DeleteRow { - _id: string -} - -export type DeleteRowRequest = DeleteRows | DeleteRow From 0d8d96b9111b87654bcecdca2e1b9ce925b9804e Mon Sep 17 00:00:00 2001 From: Dean Date: Mon, 24 Jul 2023 15:03:13 +0100 Subject: [PATCH 7/8] PR Feedback --- .../server/src/api/controllers/row/index.ts | 109 ++++++++++++------ .../server/src/api/routes/tests/row.spec.ts | 35 ++++++ 2 files changed, 110 insertions(+), 34 deletions(-) diff --git a/packages/server/src/api/controllers/row/index.ts b/packages/server/src/api/controllers/row/index.ts index c217f06ad4..a3f56654b5 100644 --- a/packages/server/src/api/controllers/row/index.ts +++ b/packages/server/src/api/controllers/row/index.ts @@ -2,7 +2,14 @@ import { quotas } from "@budibase/pro" import * as internal from "./internal" import * as external from "./external" import { isExternalTable } from "../../../integrations/utils" -import { Ctx, UserCtx, DeleteRowRequest, Row } from "@budibase/types" +import { + Ctx, + UserCtx, + DeleteRowRequest, + DeleteRow, + DeleteRows, + Row, +} from "@budibase/types" import * as utils from "./utils" import { gridSocket } from "../../../websockets" import { addRev } from "../public/utils" @@ -100,49 +107,83 @@ export async function find(ctx: any) { }) } -export async function destroy(ctx: UserCtx) { - const appId = ctx.appId - const inputs = ctx.request.body +function isDeleteRows(input: any): input is DeleteRows { + return input.rows !== undefined && Array.isArray(input.rows) +} +function isDeleteRow(input: any): input is DeleteRow { + return input._id !== undefined +} + +async function processDeleteRowRequest(ctx: UserCtx) { + let request = ctx.request.body as DeleteRows const tableId = utils.getTableId(ctx) - let response, row - if ("rows" in inputs && Array.isArray(inputs?.rows)) { - const targetRows = inputs.rows.map(row => { - let processedRow: Row = typeof row == "string" ? { _id: row } : row - return !processedRow._rev - ? addRev(fixRow(processedRow, ctx.params), tableId) - : fixRow(processedRow, ctx.params) - }) + const processedRows = request.rows.map(row => { + let processedRow: Row = typeof row == "string" ? { _id: row } : row + return !processedRow._rev + ? addRev(fixRow(processedRow, ctx.params), tableId) + : fixRow(processedRow, ctx.params) + }) - const rowDeletes: Row[] = await Promise.all(targetRows) - if (rowDeletes) { - inputs.rows = rowDeletes - } + return await Promise.all(processedRows) +} - let { rows } = await quotas.addQuery( - () => pickApi(tableId).bulkDestroy(ctx), - { - datasourceId: tableId, - } - ) - await quotas.removeRows(rows.length) - response = rows - for (let row of rows) { - ctx.eventEmitter && ctx.eventEmitter.emitRow(`row:delete`, appId, row) - gridSocket?.emitRowDeletion(ctx, row._id) - } - } else { - let resp = await quotas.addQuery(() => pickApi(tableId).destroy(ctx), { +async function deleteRows(ctx: UserCtx) { + const tableId = utils.getTableId(ctx) + const appId = ctx.appId + + let deleteRequest = ctx.request.body as DeleteRows + + const rowDeletes: Row[] = await processDeleteRowRequest(ctx) + deleteRequest.rows = rowDeletes + + let { rows } = await quotas.addQuery( + () => pickApi(tableId).bulkDestroy(ctx), + { datasourceId: tableId, - }) - await quotas.removeRow() - response = resp.response - row = resp.row + } + ) + await quotas.removeRows(rows.length) + + for (let row of rows) { ctx.eventEmitter && ctx.eventEmitter.emitRow(`row:delete`, appId, row) gridSocket?.emitRowDeletion(ctx, row._id) } + + return rows +} + +async function deleteRow(ctx: UserCtx) { + const appId = ctx.appId + const tableId = utils.getTableId(ctx) + + let resp = await quotas.addQuery(() => pickApi(tableId).destroy(ctx), { + datasourceId: tableId, + }) + await quotas.removeRow() + + ctx.eventEmitter && ctx.eventEmitter.emitRow(`row:delete`, appId, resp.row) + gridSocket?.emitRowDeletion(ctx, resp.row._id) + + return resp +} + +export async function destroy(ctx: UserCtx) { + let response, row ctx.status = 200 + + if (isDeleteRows(ctx.request.body)) { + response = await deleteRows(ctx) + } else if (isDeleteRow(ctx.request.body)) { + const deleteResp = await deleteRow(ctx) + response = deleteResp.response + row = deleteResp.row + } else { + ctx.status = 400 + response = { message: "Invalid delete rows request" } + } + // for automations include the row that was deleted ctx.row = row || {} ctx.body = response diff --git a/packages/server/src/api/routes/tests/row.spec.ts b/packages/server/src/api/routes/tests/row.spec.ts index aef5ac57ac..fbde7842c9 100644 --- a/packages/server/src/api/routes/tests/row.spec.ts +++ b/packages/server/src/api/routes/tests/row.spec.ts @@ -557,6 +557,41 @@ describe("/rows", () => { await assertRowUsage(rowUsage - 1) await assertQueryUsage(queryUsage + 1) }) + + it("Should ignore malformed/invalid delete requests", async () => { + const rowUsage = await getRowUsage() + const queryUsage = await getQueryUsage() + + const res = await request + .delete(`/api/${table._id}/rows`) + .send({ not: "valid" }) + .set(config.defaultHeaders()) + .expect("Content-Type", /json/) + .expect(400) + + expect(res.body.message).toEqual("Invalid delete rows request") + + const res2 = await request + .delete(`/api/${table._id}/rows`) + .send({ rows: 123 }) + .set(config.defaultHeaders()) + .expect("Content-Type", /json/) + .expect(400) + + expect(res2.body.message).toEqual("Invalid delete rows request") + + const res3 = await request + .delete(`/api/${table._id}/rows`) + .send("invalid") + .set(config.defaultHeaders()) + .expect("Content-Type", /json/) + .expect(400) + + expect(res3.body.message).toEqual("Invalid delete rows request") + + await assertRowUsage(rowUsage) + await assertQueryUsage(queryUsage) + }) }) describe("fetchView", () => { From 85d3980bdc3a910d05d3d4358ac0320607cf4695 Mon Sep 17 00:00:00 2001 From: Dean Date: Tue, 25 Jul 2023 15:47:25 +0100 Subject: [PATCH 8/8] PR Feedback --- packages/server/src/api/controllers/row/index.ts | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/packages/server/src/api/controllers/row/index.ts b/packages/server/src/api/controllers/row/index.ts index f3e7ca0c90..79cd5fbfe0 100644 --- a/packages/server/src/api/controllers/row/index.ts +++ b/packages/server/src/api/controllers/row/index.ts @@ -119,7 +119,7 @@ function isDeleteRow(input: any): input is DeleteRow { return input._id !== undefined } -async function processDeleteRowRequest(ctx: UserCtx) { +async function processDeleteRowsRequest(ctx: UserCtx) { let request = ctx.request.body as DeleteRows const tableId = utils.getTableId(ctx) @@ -139,7 +139,7 @@ async function deleteRows(ctx: UserCtx) { let deleteRequest = ctx.request.body as DeleteRows - const rowDeletes: Row[] = await processDeleteRowRequest(ctx) + const rowDeletes: Row[] = await processDeleteRowsRequest(ctx) deleteRequest.rows = rowDeletes let { rows } = await quotas.addQuery(