From ed256242c8f1df3a042f8ab37f301cda58d0a62d Mon Sep 17 00:00:00 2001 From: Adria Navarro Date: Mon, 31 Jul 2023 09:54:20 +0200 Subject: [PATCH 01/19] Don't allow saving _viewId on row.create --- packages/server/src/api/controllers/row/index.ts | 7 ++++++- packages/server/src/api/routes/tests/row.spec.ts | 12 ++++++++++++ 2 files changed, 18 insertions(+), 1 deletion(-) diff --git a/packages/server/src/api/controllers/row/index.ts b/packages/server/src/api/controllers/row/index.ts index 4cbf17d844..32f892c4a5 100644 --- a/packages/server/src/api/controllers/row/index.ts +++ b/packages/server/src/api/controllers/row/index.ts @@ -62,10 +62,15 @@ export async function patch( } } -export const save = async (ctx: any) => { +export const save = async (ctx: UserCtx) => { const appId = ctx.appId const tableId = utils.getTableId(ctx) const body = ctx.request.body + + if (body._viewId) { + ctx.throw(400, "Table row endpoints cannot contain view info") + } + // if it has an ID already then its a patch if (body && body._id) { return patch(ctx) diff --git a/packages/server/src/api/routes/tests/row.spec.ts b/packages/server/src/api/routes/tests/row.spec.ts index dbc417a5b5..ed0fb19343 100644 --- a/packages/server/src/api/routes/tests/row.spec.ts +++ b/packages/server/src/api/routes/tests/row.spec.ts @@ -391,6 +391,18 @@ describe("/rows", () => { expect(saved.arrayFieldArrayStrKnown).toEqual(["One"]) expect(saved.optsFieldStrKnown).toEqual("Alpha") }) + + it("should not allow creating a table row with view id data", async () => { + const res = await request + .post(`/api/${row.tableId}/rows`) + .send({ ...row, _viewId: generator.guid() }) + .set(config.defaultHeaders()) + .expect("Content-Type", /json/) + .expect(400) + expect(res.body.message).toEqual( + "Table row endpoints cannot contain view info" + ) + }) }) describe("patch", () => { From 752e901b3dfec9aad3106a49787cf325690b755a Mon Sep 17 00:00:00 2001 From: Adria Navarro Date: Mon, 31 Jul 2023 09:58:49 +0200 Subject: [PATCH 02/19] Don't allow saving _viewId on row.patch --- .../server/src/api/controllers/row/index.ts | 9 ++++++-- .../server/src/api/routes/tests/row.spec.ts | 21 ++++++++++++++++++- 2 files changed, 27 insertions(+), 3 deletions(-) diff --git a/packages/server/src/api/controllers/row/index.ts b/packages/server/src/api/controllers/row/index.ts index 32f892c4a5..41f5f1ca89 100644 --- a/packages/server/src/api/controllers/row/index.ts +++ b/packages/server/src/api/controllers/row/index.ts @@ -37,6 +37,11 @@ export async function patch( const appId = ctx.appId const tableId = utils.getTableId(ctx) const body = ctx.request.body + + if (body._viewId) { + ctx.throw(400, "Table row endpoints cannot contain view info") + } + // if it doesn't have an _id then its save if (body && !body._id) { return save(ctx) @@ -62,7 +67,7 @@ export async function patch( } } -export const save = async (ctx: UserCtx) => { +export const save = async (ctx: UserCtx) => { const appId = ctx.appId const tableId = utils.getTableId(ctx) const body = ctx.request.body @@ -73,7 +78,7 @@ export const save = async (ctx: UserCtx) => { // if it has an ID already then its a patch if (body && body._id) { - return patch(ctx) + return patch(ctx as UserCtx) } const { row, table, squashed } = await quotas.addRow(() => quotas.addQuery(() => pickApi(tableId).save(ctx), { diff --git a/packages/server/src/api/routes/tests/row.spec.ts b/packages/server/src/api/routes/tests/row.spec.ts index ed0fb19343..8f2e64db7a 100644 --- a/packages/server/src/api/routes/tests/row.spec.ts +++ b/packages/server/src/api/routes/tests/row.spec.ts @@ -392,7 +392,7 @@ describe("/rows", () => { expect(saved.optsFieldStrKnown).toEqual("Alpha") }) - it("should not allow creating a table row with view id data", async () => { + it("should throw an error when creating a table row with view id data", async () => { const res = await request .post(`/api/${row.tableId}/rows`) .send({ ...row, _viewId: generator.guid() }) @@ -452,6 +452,25 @@ describe("/rows", () => { await assertRowUsage(rowUsage) await assertQueryUsage(queryUsage) }) + + it("should throw an error when creating a table row with view id data", async () => { + const existing = await config.createRow() + + const res = await config.api.row.patch( + table._id!, + { + ...existing, + _id: existing._id!, + _rev: existing._rev!, + tableId: table._id!, + _viewId: generator.guid(), + }, + { expectStatus: 400 } + ) + expect(res.body.message).toEqual( + "Table row endpoints cannot contain view info" + ) + }) }) describe("destroy", () => { From 5cfddabac76b50e8698e39d1403edc9a12f0c06e Mon Sep 17 00:00:00 2001 From: Adria Navarro Date: Mon, 31 Jul 2023 10:21:44 +0200 Subject: [PATCH 03/19] Move row view controllers in their own file --- .../server/src/api/controllers/row/index.ts | 82 +----------------- .../server/src/api/controllers/row/views.ts | 86 +++++++++++++++++++ packages/server/src/api/routes/row.ts | 36 ++++++-- 3 files changed, 118 insertions(+), 86 deletions(-) create mode 100644 packages/server/src/api/controllers/row/views.ts diff --git a/packages/server/src/api/controllers/row/index.ts b/packages/server/src/api/controllers/row/index.ts index 41f5f1ca89..7c8af846b8 100644 --- a/packages/server/src/api/controllers/row/index.ts +++ b/packages/server/src/api/controllers/row/index.ts @@ -11,10 +11,6 @@ import { Row, PatchRowRequest, PatchRowResponse, - SearchResponse, - SortOrder, - SortType, - ViewV2, } from "@budibase/types" import * as utils from "./utils" import { gridSocket } from "../../../websockets" @@ -23,6 +19,7 @@ import { fixRow } from "../public/rows" import sdk from "../../../sdk" import * as exporters from "../view/exporters" import { apiFileReturn } from "../../../utilities/fileSystem" +export * as views from "./views" function pickApi(tableId: any) { if (isExternalTable(tableId)) { @@ -222,83 +219,6 @@ export async function search(ctx: any) { }) } -function getSortOptions( - ctx: Ctx, - view: ViewV2 -): - | { - sort: string - sortOrder?: SortOrder - sortType?: SortType - } - | undefined { - const { sort_column, sort_order, sort_type } = ctx.query - if (Array.isArray(sort_column)) { - ctx.throw(400, "sort_column cannot be an array") - } - if (Array.isArray(sort_order)) { - ctx.throw(400, "sort_order cannot be an array") - } - if (Array.isArray(sort_type)) { - ctx.throw(400, "sort_type cannot be an array") - } - - if (sort_column) { - return { - sort: sort_column, - sortOrder: sort_order as SortOrder, - sortType: sort_type as SortType, - } - } - if (view.sort) { - return { - sort: view.sort.field, - sortOrder: view.sort.order, - sortType: view.sort.type, - } - } - - return -} - -export async function searchView(ctx: Ctx) { - const { viewId } = ctx.params - - const view = await sdk.views.get(viewId) - if (!view) { - ctx.throw(404, `View ${viewId} not found`) - } - - if (view.version !== 2) { - ctx.throw(400, `This method only supports viewsV2`) - } - - const table = await sdk.tables.getTable(view?.tableId) - - const viewFields = - (view.columns && - Object.entries(view.columns).length && - Object.keys(sdk.views.enrichSchema(view, table.schema).schema)) || - undefined - - ctx.status = 200 - const result = await quotas.addQuery( - () => - sdk.rows.search({ - tableId: view.tableId, - query: view.query || {}, - fields: viewFields, - ...getSortOptions(ctx, view), - }), - { - datasourceId: view.tableId, - } - ) - - result.rows.forEach(r => (r._viewId = view.id)) - ctx.body = result -} - export async function validate(ctx: Ctx) { const tableId = utils.getTableId(ctx) // external tables are hard to validate currently diff --git a/packages/server/src/api/controllers/row/views.ts b/packages/server/src/api/controllers/row/views.ts new file mode 100644 index 0000000000..2560002ead --- /dev/null +++ b/packages/server/src/api/controllers/row/views.ts @@ -0,0 +1,86 @@ +import { quotas } from "@budibase/pro" +import { + Ctx, + SearchResponse, + SortOrder, + SortType, + ViewV2, +} from "@budibase/types" +import sdk from "../../../sdk" + +export async function searchView(ctx: Ctx) { + const { viewId } = ctx.params + + const view = await sdk.views.get(viewId) + if (!view) { + ctx.throw(404, `View ${viewId} not found`) + } + + if (view.version !== 2) { + ctx.throw(400, `This method only supports viewsV2`) + } + + const table = await sdk.tables.getTable(view?.tableId) + + const viewFields = + (view.columns && + Object.entries(view.columns).length && + Object.keys(sdk.views.enrichSchema(view, table.schema).schema)) || + undefined + + ctx.status = 200 + const result = await quotas.addQuery( + () => + sdk.rows.search({ + tableId: view.tableId, + query: view.query || {}, + fields: viewFields, + ...getSortOptions(ctx, view), + }), + { + datasourceId: view.tableId, + } + ) + + result.rows.forEach(r => (r._viewId = view.id)) + ctx.body = result +} + +function getSortOptions( + ctx: Ctx, + view: ViewV2 +): + | { + sort: string + sortOrder?: SortOrder + sortType?: SortType + } + | undefined { + const { sort_column, sort_order, sort_type } = ctx.query + if (Array.isArray(sort_column)) { + ctx.throw(400, "sort_column cannot be an array") + } + if (Array.isArray(sort_order)) { + ctx.throw(400, "sort_order cannot be an array") + } + if (Array.isArray(sort_type)) { + ctx.throw(400, "sort_type cannot be an array") + } + + if (sort_column) { + return { + sort: sort_column, + sortOrder: sort_order as SortOrder, + sortType: sort_type as SortType, + } + } + if (view.sort) { + return { + sort: view.sort.field, + sortOrder: view.sort.order, + sortType: view.sort.type, + } + } + + return +} diff --git a/packages/server/src/api/routes/row.ts b/packages/server/src/api/routes/row.ts index 5fdc02b7a7..a68a9065c7 100644 --- a/packages/server/src/api/routes/row.ts +++ b/packages/server/src/api/routes/row.ts @@ -146,11 +146,6 @@ router authorized(PermissionType.TABLE, PermissionLevel.READ), rowController.search ) - .get( - "/api/v2/views/:viewId/search", - authorized(PermissionType.VIEW, PermissionLevel.READ), - rowController.searchView - ) /** * @api {post} /api/:tableId/rows Creates a new row * @apiName Creates a new row @@ -268,4 +263,35 @@ router rowController.exportRows ) +router + .get( + "/api/v2/views/:viewId/search", + authorized(PermissionType.VIEW, PermissionLevel.READ), + rowController.views.searchView + ) + /** + * @api {post} /api/:tableId/rows Creates a new row + * @apiName Creates a new row + * @apiGroup rows + * @apiPermission table write access + * @apiDescription This API will create a new row based on the supplied body. If the + * body includes an "_id" field then it will update an existing row if the field + * links to one. Please note that "_id", "_rev" and "tableId" are fields that are + * already used by Budibase tables and cannot be used for columns. + * + * @apiParam {string} tableId The ID of the table to save a row to. + * + * @apiParam (Body) {string} [_id] If the row exists already then an ID for the row must be provided. + * @apiParam (Body) {string} [_rev] If working with an existing row for an internal table its revision + * must also be provided. + * @apiParam (Body) {string} _viewId The ID of the view should be specified in the row body itself. + * @apiParam (Body) {string} tableId The ID of the table should also be specified in the row body itself. + * @apiParam (Body) {any} [any] Any field supplied in the body will be assessed to see if it matches + * a column in the specified table. All other fields will be dropped and not stored. + * + * @apiSuccess {string} _id The ID of the row that was just saved, if it was just created this + * is the rows new ID. + * @apiSuccess {string} [_rev] If saving to an internal table a revision will also be returned. + * @apiSuccess {object} body The contents of the row that was saved will be returned as well. + */ export default router From 7a9a997d73e1b4d7f6b891fefe0fce3472db498c Mon Sep 17 00:00:00 2001 From: Adria Navarro Date: Mon, 31 Jul 2023 10:38:31 +0200 Subject: [PATCH 04/19] Use middleware for checks --- packages/server/src/api/controllers/row/index.ts | 8 -------- packages/server/src/api/routes/row.ts | 10 ++++++++++ packages/server/src/middleware/guardViewRowInfo.ts | 12 ++++++++++++ 3 files changed, 22 insertions(+), 8 deletions(-) create mode 100644 packages/server/src/middleware/guardViewRowInfo.ts diff --git a/packages/server/src/api/controllers/row/index.ts b/packages/server/src/api/controllers/row/index.ts index 7c8af846b8..1bb9569c49 100644 --- a/packages/server/src/api/controllers/row/index.ts +++ b/packages/server/src/api/controllers/row/index.ts @@ -35,10 +35,6 @@ export async function patch( const tableId = utils.getTableId(ctx) const body = ctx.request.body - if (body._viewId) { - ctx.throw(400, "Table row endpoints cannot contain view info") - } - // if it doesn't have an _id then its save if (body && !body._id) { return save(ctx) @@ -69,10 +65,6 @@ export const save = async (ctx: UserCtx) => { const tableId = utils.getTableId(ctx) const body = ctx.request.body - if (body._viewId) { - ctx.throw(400, "Table row endpoints cannot contain view info") - } - // if it has an ID already then its a patch if (body && body._id) { return patch(ctx as UserCtx) diff --git a/packages/server/src/api/routes/row.ts b/packages/server/src/api/routes/row.ts index a68a9065c7..179082d8de 100644 --- a/packages/server/src/api/routes/row.ts +++ b/packages/server/src/api/routes/row.ts @@ -4,6 +4,7 @@ import authorized from "../../middleware/authorized" import { paramResource, paramSubResource } from "../../middleware/resourceId" import { permissions } from "@budibase/backend-core" import { internalSearchValidator } from "./utils/validators" +import guardViewRowInfo from "../../middleware/guardViewRowInfo" const { PermissionType, PermissionLevel } = permissions const router: Router = new Router() @@ -174,6 +175,7 @@ router "/api/:tableId/rows", paramResource("tableId"), authorized(PermissionType.TABLE, PermissionLevel.WRITE), + guardViewRowInfo(), rowController.save ) /** @@ -188,6 +190,7 @@ router "/api/:tableId/rows", paramResource("tableId"), authorized(PermissionType.TABLE, PermissionLevel.WRITE), + guardViewRowInfo(), rowController.patch ) /** @@ -294,4 +297,11 @@ router * @apiSuccess {string} [_rev] If saving to an internal table a revision will also be returned. * @apiSuccess {object} body The contents of the row that was saved will be returned as well. */ + .post( + "/api/v2/views/:viewId/rows", + paramResource("viewId"), + authorized(PermissionType.VIEW, PermissionLevel.WRITE), + rowController.views.save + ) + export default router diff --git a/packages/server/src/middleware/guardViewRowInfo.ts b/packages/server/src/middleware/guardViewRowInfo.ts new file mode 100644 index 0000000000..7a7413b760 --- /dev/null +++ b/packages/server/src/middleware/guardViewRowInfo.ts @@ -0,0 +1,12 @@ +import { Ctx, Row } from "@budibase/types" + +const checkNoViewData = async (ctx: Ctx) => { + if (ctx.request.body._viewId) { + ctx.throw(400, "Table row endpoints cannot contain view info") + } +} + +export default () => async (ctx: any, next: any) => { + await checkNoViewData(ctx) + return next() +} From 0f540e669c877830c18789b07112ffe3d9a90dc5 Mon Sep 17 00:00:00 2001 From: Adria Navarro Date: Mon, 31 Jul 2023 10:39:12 +0200 Subject: [PATCH 05/19] Types --- packages/server/src/api/controllers/row/views.ts | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/packages/server/src/api/controllers/row/views.ts b/packages/server/src/api/controllers/row/views.ts index 2560002ead..c38b7fe56e 100644 --- a/packages/server/src/api/controllers/row/views.ts +++ b/packages/server/src/api/controllers/row/views.ts @@ -1,6 +1,6 @@ import { quotas } from "@budibase/pro" import { - Ctx, + UserCtx, SearchResponse, SortOrder, SortType, @@ -8,7 +8,7 @@ import { } from "@budibase/types" import sdk from "../../../sdk" -export async function searchView(ctx: Ctx) { +export async function searchView(ctx: UserCtx) { const { viewId } = ctx.params const view = await sdk.views.get(viewId) @@ -47,7 +47,7 @@ export async function searchView(ctx: Ctx) { } function getSortOptions( - ctx: Ctx, + ctx: UserCtx, view: ViewV2 ): | { From 4c11a6593c38c65a897d1e476734cf78ab6f942e Mon Sep 17 00:00:00 2001 From: Adria Navarro Date: Mon, 31 Jul 2023 10:43:45 +0200 Subject: [PATCH 06/19] Add sdk trim view fields --- .../src/sdk/app/rows/tests/utils.spec.ts | 90 +++++++++++++++++++ packages/server/src/sdk/app/rows/utils.ts | 19 ++++ 2 files changed, 109 insertions(+) create mode 100644 packages/server/src/sdk/app/rows/tests/utils.spec.ts diff --git a/packages/server/src/sdk/app/rows/tests/utils.spec.ts b/packages/server/src/sdk/app/rows/tests/utils.spec.ts new file mode 100644 index 0000000000..18b169ed4f --- /dev/null +++ b/packages/server/src/sdk/app/rows/tests/utils.spec.ts @@ -0,0 +1,90 @@ +import { generator } from "@budibase/backend-core/tests" +import { FieldType, Table } from "@budibase/types" +jest.mock("../../../../sdk", () => ({ + views: { + ...jest.requireActual("../../../../sdk/app/views"), + get: jest.fn(), + }, +})) + +import sdk from "../../../../sdk" +import { trimViewFields } from "../utils" + +const mockGetView = sdk.views.get as jest.MockedFunction + +describe("utils", () => { + const table: Table = { + name: generator.word(), + type: "table", + schema: { + name: { + name: "name", + type: FieldType.STRING, + }, + age: { + name: "age", + type: FieldType.NUMBER, + }, + address: { + name: "address", + type: FieldType.STRING, + }, + }, + } + + beforeEach(() => { + jest.resetAllMocks() + }) + + describe("trimViewFields", () => { + it("when no columns are defined, same data is returned", async () => { + mockGetView.mockResolvedValue({ + version: 2, + id: generator.guid(), + name: generator.guid(), + tableId: generator.guid(), + }) + + const viewId = generator.guid() + const data = { + _id: generator.guid(), + name: generator.name(), + age: generator.age(), + address: generator.address(), + } + + const result = await trimViewFields(viewId, table, data) + + expect(result).toBe(data) + }) + + it("when columns are defined, trim data is returned", async () => { + mockGetView.mockResolvedValue({ + version: 2, + id: generator.guid(), + name: generator.guid(), + tableId: generator.guid(), + columns: { + name: { visible: true }, + address: { visible: true }, + age: { visible: false }, + }, + }) + + const viewId = generator.guid() + const data = { + _id: generator.guid(), + name: generator.name(), + age: generator.age(), + address: generator.address(), + } + + const result = await trimViewFields(viewId, table, data) + + expect(result).toEqual({ + name: data.name, + address: data.address, + }) + }) + }) +}) diff --git a/packages/server/src/sdk/app/rows/utils.ts b/packages/server/src/sdk/app/rows/utils.ts index 51e418c324..17add1f934 100644 --- a/packages/server/src/sdk/app/rows/utils.ts +++ b/packages/server/src/sdk/app/rows/utils.ts @@ -135,3 +135,22 @@ export async function validate({ } return { valid: Object.keys(errors).length === 0, errors } } + +export async function trimViewFields( + viewId: string, + table: Table, + data: T +): Promise { + const view = await sdk.views.get(viewId) + if (!view?.columns || !Object.keys(view.columns).length) { + return data + } + + const { schema } = sdk.views.enrichSchema(view!, table.schema) + const result: Record = {} + for (const key of Object.keys(schema)) { + result[key] = data[key] !== null ? data[key] : undefined + } + + return result as T +} From 97f7629345af35f103a7572256c89e4da9070bc8 Mon Sep 17 00:00:00 2001 From: Adria Navarro Date: Mon, 31 Jul 2023 10:50:33 +0200 Subject: [PATCH 07/19] Renames --- packages/server/src/api/routes/row.ts | 6 +++--- .../src/middleware/{guardViewRowInfo.ts => noViewData.ts} | 5 +---- 2 files changed, 4 insertions(+), 7 deletions(-) rename packages/server/src/middleware/{guardViewRowInfo.ts => noViewData.ts} (55%) diff --git a/packages/server/src/api/routes/row.ts b/packages/server/src/api/routes/row.ts index 179082d8de..25224623c6 100644 --- a/packages/server/src/api/routes/row.ts +++ b/packages/server/src/api/routes/row.ts @@ -4,7 +4,7 @@ import authorized from "../../middleware/authorized" import { paramResource, paramSubResource } from "../../middleware/resourceId" import { permissions } from "@budibase/backend-core" import { internalSearchValidator } from "./utils/validators" -import guardViewRowInfo from "../../middleware/guardViewRowInfo" +import noViewData from "../../middleware/noViewData" const { PermissionType, PermissionLevel } = permissions const router: Router = new Router() @@ -175,7 +175,7 @@ router "/api/:tableId/rows", paramResource("tableId"), authorized(PermissionType.TABLE, PermissionLevel.WRITE), - guardViewRowInfo(), + noViewData(), rowController.save ) /** @@ -190,7 +190,7 @@ router "/api/:tableId/rows", paramResource("tableId"), authorized(PermissionType.TABLE, PermissionLevel.WRITE), - guardViewRowInfo(), + noViewData(), rowController.patch ) /** diff --git a/packages/server/src/middleware/guardViewRowInfo.ts b/packages/server/src/middleware/noViewData.ts similarity index 55% rename from packages/server/src/middleware/guardViewRowInfo.ts rename to packages/server/src/middleware/noViewData.ts index 7a7413b760..5a2bb45012 100644 --- a/packages/server/src/middleware/guardViewRowInfo.ts +++ b/packages/server/src/middleware/noViewData.ts @@ -1,12 +1,9 @@ import { Ctx, Row } from "@budibase/types" -const checkNoViewData = async (ctx: Ctx) => { +export default () => async (ctx: Ctx, next: any) => { if (ctx.request.body._viewId) { ctx.throw(400, "Table row endpoints cannot contain view info") } -} -export default () => async (ctx: any, next: any) => { - await checkNoViewData(ctx) return next() } From 5052f2cd6894339e09bd8ea01135ea3c913c02c1 Mon Sep 17 00:00:00 2001 From: Adria Navarro Date: Mon, 31 Jul 2023 11:22:12 +0200 Subject: [PATCH 08/19] Fix --- packages/server/src/api/routes/row.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/server/src/api/routes/row.ts b/packages/server/src/api/routes/row.ts index 25224623c6..379e44b789 100644 --- a/packages/server/src/api/routes/row.ts +++ b/packages/server/src/api/routes/row.ts @@ -301,7 +301,7 @@ router "/api/v2/views/:viewId/rows", paramResource("viewId"), authorized(PermissionType.VIEW, PermissionLevel.WRITE), - rowController.views.save + rowController.save ) export default router From eaa7d9bf815b1af26d06737183d9fe2f61b10adc Mon Sep 17 00:00:00 2001 From: Adria Navarro Date: Mon, 31 Jul 2023 12:03:03 +0200 Subject: [PATCH 09/19] trimViewRowInfo middleware and api test --- .../src/api/controllers/row/internal.ts | 3 +- packages/server/src/api/routes/row.ts | 2 + .../server/src/api/routes/tests/row.spec.ts | 76 ++++++++++++++++ .../server/src/middleware/trimViewRowInfo.ts | 47 ++++++++++ .../src/sdk/app/rows/tests/utils.spec.ts | 90 ------------------- packages/server/src/sdk/app/rows/utils.ts | 19 ---- .../server/src/tests/utilities/api/row.ts | 8 ++ .../server/src/tests/utilities/api/viewV2.ts | 24 ++++- 8 files changed, 157 insertions(+), 112 deletions(-) create mode 100644 packages/server/src/middleware/trimViewRowInfo.ts delete mode 100644 packages/server/src/sdk/app/rows/tests/utils.spec.ts diff --git a/packages/server/src/api/controllers/row/internal.ts b/packages/server/src/api/controllers/row/internal.ts index 1153461b89..2ff1df0933 100644 --- a/packages/server/src/api/controllers/row/internal.ts +++ b/packages/server/src/api/controllers/row/internal.ts @@ -93,7 +93,6 @@ export async function patch(ctx: UserCtx) { } export async function save(ctx: UserCtx) { - const db = context.getAppDB() let inputs = ctx.request.body inputs.tableId = ctx.params.tableId @@ -177,7 +176,6 @@ export async function destroy(ctx: UserCtx) { } export async function bulkDestroy(ctx: UserCtx) { - const db = context.getAppDB() const tableId = ctx.params.tableId const table = await sdk.tables.getTable(tableId) let { rows } = ctx.request.body @@ -206,6 +204,7 @@ export async function bulkDestroy(ctx: UserCtx) { }) ) } else { + const db = context.getAppDB() await db.bulkDocs(processedRows.map(row => ({ ...row, _deleted: true }))) } // remove any attachments that were on the rows from object storage diff --git a/packages/server/src/api/routes/row.ts b/packages/server/src/api/routes/row.ts index 379e44b789..5d9a2d1df8 100644 --- a/packages/server/src/api/routes/row.ts +++ b/packages/server/src/api/routes/row.ts @@ -5,6 +5,7 @@ import { paramResource, paramSubResource } from "../../middleware/resourceId" import { permissions } from "@budibase/backend-core" import { internalSearchValidator } from "./utils/validators" import noViewData from "../../middleware/noViewData" +import trimViewRowInfo from "../../middleware/trimViewRowInfo" const { PermissionType, PermissionLevel } = permissions const router: Router = new Router() @@ -301,6 +302,7 @@ router "/api/v2/views/:viewId/rows", paramResource("viewId"), authorized(PermissionType.VIEW, PermissionLevel.WRITE), + trimViewRowInfo(), rowController.save ) diff --git a/packages/server/src/api/routes/tests/row.spec.ts b/packages/server/src/api/routes/tests/row.spec.ts index 8f2e64db7a..ad521967b0 100644 --- a/packages/server/src/api/routes/tests/row.spec.ts +++ b/packages/server/src/api/routes/tests/row.spec.ts @@ -1039,4 +1039,80 @@ describe("/rows", () => { expect(response.body.rows).toHaveLength(0) }) }) + + describe("view 2.0", () => { + function userTable(): Table { + return { + name: "user", + type: "user", + schema: { + name: { + type: FieldType.STRING, + name: "name", + }, + surname: { + type: FieldType.STRING, + name: "name", + }, + age: { + type: FieldType.NUMBER, + name: "age", + }, + address: { + type: FieldType.STRING, + name: "address", + }, + jobTitle: { + type: FieldType.STRING, + name: "jobTitle", + }, + }, + } + } + + const randomRowData = () => ({ + name: generator.first(), + surname: generator.last(), + age: generator.age(), + address: generator.address(), + jobTitle: generator.word(), + }) + + describe("create", () => { + it("should persist a new row with only the provided view fields", async () => { + const table = await config.createTable(userTable()) + const view = await config.api.viewV2.create({ + tableId: table._id!, + columns: { + name: { visible: true }, + surname: { visible: true }, + address: { visible: true }, + }, + }) + + const data = randomRowData() + const newRow = await config.api.viewV2.row.create(view.id, { + tableId: config.table!._id, + _viewId: view.id, + ...data, + }) + + const row = await config.api.row.get(table._id!, newRow._id!) + expect(row.body).toEqual({ + name: data.name, + surname: data.surname, + address: data.address, + tableId: config.table!._id, + type: "row", + _id: expect.any(String), + _rev: expect.any(String), + createdAt: expect.any(String), + updatedAt: expect.any(String), + }) + expect(row.body._viewId).toBeUndefined() + expect(row.body.age).toBeUndefined() + expect(row.body.jobTitle).toBeUndefined() + }) + }) + }) }) diff --git a/packages/server/src/middleware/trimViewRowInfo.ts b/packages/server/src/middleware/trimViewRowInfo.ts new file mode 100644 index 0000000000..66d6d32ed6 --- /dev/null +++ b/packages/server/src/middleware/trimViewRowInfo.ts @@ -0,0 +1,47 @@ +import { Ctx, Row } from "@budibase/types" +import * as utils from "../db/utils" +import sdk from "../sdk" +import { db } from "@budibase/backend-core" + +export default () => async (ctx: Ctx, next: any) => { + const { body } = ctx.request + const { _viewId: viewId } = body + if (!viewId) { + ctx.throw(400, "_viewId is required") + } + + const { tableId } = utils.extractViewInfoFromID(viewId) + const { _viewId, ...trimmedView } = await trimViewFields( + viewId, + tableId, + body + ) + ctx.request.body = trimmedView + ctx.params.tableId = body.tableId + + return next() +} + +export async function trimViewFields( + viewId: string, + tableId: string, + data: T +): Promise { + const view = await sdk.views.get(viewId) + if (!view?.columns || !Object.keys(view.columns).length) { + return data + } + + const table = await sdk.tables.getTable(tableId) + const { schema } = sdk.views.enrichSchema(view!, table.schema) + const result: Record = {} + for (const key of [ + ...Object.keys(schema), + ...db.CONSTANT_EXTERNAL_ROW_COLS, + ...db.CONSTANT_INTERNAL_ROW_COLS, + ]) { + result[key] = data[key] !== null ? data[key] : undefined + } + + return result as T +} diff --git a/packages/server/src/sdk/app/rows/tests/utils.spec.ts b/packages/server/src/sdk/app/rows/tests/utils.spec.ts deleted file mode 100644 index 18b169ed4f..0000000000 --- a/packages/server/src/sdk/app/rows/tests/utils.spec.ts +++ /dev/null @@ -1,90 +0,0 @@ -import { generator } from "@budibase/backend-core/tests" -import { FieldType, Table } from "@budibase/types" -jest.mock("../../../../sdk", () => ({ - views: { - ...jest.requireActual("../../../../sdk/app/views"), - get: jest.fn(), - }, -})) - -import sdk from "../../../../sdk" -import { trimViewFields } from "../utils" - -const mockGetView = sdk.views.get as jest.MockedFunction - -describe("utils", () => { - const table: Table = { - name: generator.word(), - type: "table", - schema: { - name: { - name: "name", - type: FieldType.STRING, - }, - age: { - name: "age", - type: FieldType.NUMBER, - }, - address: { - name: "address", - type: FieldType.STRING, - }, - }, - } - - beforeEach(() => { - jest.resetAllMocks() - }) - - describe("trimViewFields", () => { - it("when no columns are defined, same data is returned", async () => { - mockGetView.mockResolvedValue({ - version: 2, - id: generator.guid(), - name: generator.guid(), - tableId: generator.guid(), - }) - - const viewId = generator.guid() - const data = { - _id: generator.guid(), - name: generator.name(), - age: generator.age(), - address: generator.address(), - } - - const result = await trimViewFields(viewId, table, data) - - expect(result).toBe(data) - }) - - it("when columns are defined, trim data is returned", async () => { - mockGetView.mockResolvedValue({ - version: 2, - id: generator.guid(), - name: generator.guid(), - tableId: generator.guid(), - columns: { - name: { visible: true }, - address: { visible: true }, - age: { visible: false }, - }, - }) - - const viewId = generator.guid() - const data = { - _id: generator.guid(), - name: generator.name(), - age: generator.age(), - address: generator.address(), - } - - const result = await trimViewFields(viewId, table, data) - - expect(result).toEqual({ - name: data.name, - address: data.address, - }) - }) - }) -}) diff --git a/packages/server/src/sdk/app/rows/utils.ts b/packages/server/src/sdk/app/rows/utils.ts index 17add1f934..51e418c324 100644 --- a/packages/server/src/sdk/app/rows/utils.ts +++ b/packages/server/src/sdk/app/rows/utils.ts @@ -135,22 +135,3 @@ export async function validate({ } return { valid: Object.keys(errors).length === 0, errors } } - -export async function trimViewFields( - viewId: string, - table: Table, - data: T -): Promise { - const view = await sdk.views.get(viewId) - if (!view?.columns || !Object.keys(view.columns).length) { - return data - } - - const { schema } = sdk.views.enrichSchema(view!, table.schema) - const result: Record = {} - for (const key of Object.keys(schema)) { - result[key] = data[key] !== null ? data[key] : undefined - } - - return result as T -} diff --git a/packages/server/src/tests/utilities/api/row.ts b/packages/server/src/tests/utilities/api/row.ts index 9c7e33278d..f4c80fa4c2 100644 --- a/packages/server/src/tests/utilities/api/row.ts +++ b/packages/server/src/tests/utilities/api/row.ts @@ -7,6 +7,14 @@ export class RowAPI extends TestAPI { super(config) } + get = async (tableId: string, rowId: string) => { + return await this.request + .get(`/api/${tableId}/rows/${rowId}`) + .set(this.config.defaultHeaders()) + .expect("Content-Type", /json/) + .expect(200) + } + patch = async ( tableId: string, row: PatchRowRequest, diff --git a/packages/server/src/tests/utilities/api/viewV2.ts b/packages/server/src/tests/utilities/api/viewV2.ts index fae0850f79..c2b5812fc3 100644 --- a/packages/server/src/tests/utilities/api/viewV2.ts +++ b/packages/server/src/tests/utilities/api/viewV2.ts @@ -1,4 +1,10 @@ -import { CreateViewRequest, SortOrder, SortType, ViewV2 } from "@budibase/types" +import { + CreateViewRequest, + Row, + SortOrder, + SortType, + ViewV2, +} from "@budibase/types" import TestConfiguration from "../TestConfiguration" import { TestAPI } from "./base" import { generator } from "@budibase/backend-core/tests" @@ -93,4 +99,20 @@ export class ViewV2API extends TestAPI { .expect("Content-Type", /json/) .expect(expectStatus) } + + row = { + create: async ( + viewId: string, + row: Row, + { expectStatus } = { expectStatus: 200 } + ): Promise => { + const result = await this.request + .post(`/api/v2/views/${viewId}/rows`) + .send(row) + .set(this.config.defaultHeaders()) + .expect("Content-Type", /json/) + .expect(expectStatus) + return result.body as Row + }, + } } From 30c3c6e5adc6aab7ebc3dca756fa615be7d48a5d Mon Sep 17 00:00:00 2001 From: Adria Navarro Date: Mon, 31 Jul 2023 13:17:54 +0200 Subject: [PATCH 10/19] Test middlewate --- .../middleware/tests/trimViewRowInfo.spec.ts | 122 ++++++++++++++++++ .../server/src/middleware/trimViewRowInfo.ts | 7 +- 2 files changed, 126 insertions(+), 3 deletions(-) create mode 100644 packages/server/src/middleware/tests/trimViewRowInfo.spec.ts diff --git a/packages/server/src/middleware/tests/trimViewRowInfo.spec.ts b/packages/server/src/middleware/tests/trimViewRowInfo.spec.ts new file mode 100644 index 0000000000..beeaab3608 --- /dev/null +++ b/packages/server/src/middleware/tests/trimViewRowInfo.spec.ts @@ -0,0 +1,122 @@ +import { generator } from "@budibase/backend-core/tests" +import { BBRequest, FieldType, Row, Table } from "@budibase/types" +import * as utils from "../../db/utils" +import trimViewRowInfoMiddleware from "../trimViewRowInfo" + +jest.mock("../../sdk", () => ({ + views: { + ...jest.requireActual("../../sdk/app/views"), + get: jest.fn(), + }, + tables: { + getTable: jest.fn(), + }, +})) + +import sdk from "../../sdk" +import { BaseContext, Next } from "koa" + +const mockGetView = sdk.views.get as jest.MockedFunction +const mockGetTable = sdk.tables.getTable as jest.MockedFunction< + typeof sdk.tables.getTable +> + +class TestConfiguration { + next: Next + throw: (status: number, message: string) => never + middleware: typeof trimViewRowInfoMiddleware + params: Record + request?: Pick, "body"> + + constructor() { + this.next = jest.fn() + this.throw = (_status: any, message: any) => { + throw new Error(message) + } + this.params = {} + + this.middleware = trimViewRowInfoMiddleware + } + + executeMiddleware(viewId: string, ctxRequestBody: Row) { + this.request = { + body: ctxRequestBody, + } + this.params.viewId = viewId + return this.middleware( + { + request: this.request as any, + next: this.next, + throw: this.throw as any, + params: this.params, + } as any, + this.next + ) + } + + afterEach() { + jest.clearAllMocks() + } +} + +describe("trimViewRowInfo middleware", () => { + let config: TestConfiguration + + beforeEach(() => { + config = new TestConfiguration() + }) + + afterEach(() => { + config.afterEach() + }) + + const table: Table = { + _id: utils.generateTableID(), + name: generator.word(), + type: "table", + schema: { + name: { + name: "name", + type: FieldType.STRING, + }, + age: { + name: "age", + type: FieldType.NUMBER, + }, + address: { + name: "address", + type: FieldType.STRING, + }, + }, + } + + beforeEach(() => { + jest.resetAllMocks() + }) + + it("when no columns are defined, same data is returned", async () => { + mockGetView.mockResolvedValue({ + version: 2, + id: generator.guid(), + name: generator.guid(), + tableId: generator.guid(), + }) + mockGetTable.mockResolvedValue(table) + + const viewId = utils.generateViewID(table._id!) + const data = { + _id: generator.guid(), + name: generator.name(), + age: generator.age(), + address: generator.address(), + } + + await config.executeMiddleware(viewId, { + _viewId: viewId, + ...data, + }) + + expect(config.request?.body).toEqual(data) + expect(config.params.tableId).toEqual(table._id) + }) +}) diff --git a/packages/server/src/middleware/trimViewRowInfo.ts b/packages/server/src/middleware/trimViewRowInfo.ts index 66d6d32ed6..5dbe7a6da0 100644 --- a/packages/server/src/middleware/trimViewRowInfo.ts +++ b/packages/server/src/middleware/trimViewRowInfo.ts @@ -2,22 +2,23 @@ import { Ctx, Row } from "@budibase/types" import * as utils from "../db/utils" import sdk from "../sdk" import { db } from "@budibase/backend-core" +import { Next } from "koa" -export default () => async (ctx: Ctx, next: any) => { +export default async (ctx: Ctx, next: Next) => { const { body } = ctx.request const { _viewId: viewId } = body if (!viewId) { ctx.throw(400, "_viewId is required") } - const { tableId } = utils.extractViewInfoFromID(viewId) + const { tableId } = utils.extractViewInfoFromID(ctx.params.viewId) const { _viewId, ...trimmedView } = await trimViewFields( viewId, tableId, body ) ctx.request.body = trimmedView - ctx.params.tableId = body.tableId + ctx.params.tableId = tableId return next() } From 70cd44ba2af81e2bb1b96bf935648d148147b5e4 Mon Sep 17 00:00:00 2001 From: Adria Navarro Date: Mon, 31 Jul 2023 13:24:08 +0200 Subject: [PATCH 11/19] More tests --- .../middleware/tests/trimViewRowInfo.spec.ts | 47 ++++++++++++++----- 1 file changed, 36 insertions(+), 11 deletions(-) diff --git a/packages/server/src/middleware/tests/trimViewRowInfo.spec.ts b/packages/server/src/middleware/tests/trimViewRowInfo.spec.ts index beeaab3608..8313052e79 100644 --- a/packages/server/src/middleware/tests/trimViewRowInfo.spec.ts +++ b/packages/server/src/middleware/tests/trimViewRowInfo.spec.ts @@ -92,25 +92,26 @@ describe("trimViewRowInfo middleware", () => { beforeEach(() => { jest.resetAllMocks() + mockGetTable.mockResolvedValue(table) + }) + + const getRandomData = () => ({ + _id: generator.guid(), + name: generator.name(), + age: generator.age(), + address: generator.address(), }) it("when no columns are defined, same data is returned", async () => { + const viewId = utils.generateViewID(table._id!) mockGetView.mockResolvedValue({ version: 2, - id: generator.guid(), + id: viewId, name: generator.guid(), - tableId: generator.guid(), + tableId: table._id!, }) - mockGetTable.mockResolvedValue(table) - - const viewId = utils.generateViewID(table._id!) - const data = { - _id: generator.guid(), - name: generator.name(), - age: generator.age(), - address: generator.address(), - } + const data = getRandomData() await config.executeMiddleware(viewId, { _viewId: viewId, ...data, @@ -119,4 +120,28 @@ describe("trimViewRowInfo middleware", () => { expect(config.request?.body).toEqual(data) expect(config.params.tableId).toEqual(table._id) }) + + it("when columns are defined, trimmed data is returned", async () => { + const viewId = utils.generateViewID(table._id!) + mockGetView.mockResolvedValue({ + version: 2, + id: viewId, + name: generator.guid(), + tableId: table._id!, + columns: { name: { visible: true }, address: { visible: true } }, + }) + + const data = getRandomData() + await config.executeMiddleware(viewId, { + _viewId: viewId, + ...data, + }) + + expect(config.request?.body).toEqual({ + _id: data._id, + name: data.name, + address: data.address, + }) + expect(config.params.tableId).toEqual(table._id) + }) }) From 525052ce7e729d569e1c8915db3a257ee50b0d00 Mon Sep 17 00:00:00 2001 From: Adria Navarro Date: Mon, 31 Jul 2023 13:34:57 +0200 Subject: [PATCH 12/19] Unhappy path tests --- .../middleware/tests/trimViewRowInfo.spec.ts | 26 +++++++++++++++---- 1 file changed, 21 insertions(+), 5 deletions(-) diff --git a/packages/server/src/middleware/tests/trimViewRowInfo.spec.ts b/packages/server/src/middleware/tests/trimViewRowInfo.spec.ts index 8313052e79..888e22ef0a 100644 --- a/packages/server/src/middleware/tests/trimViewRowInfo.spec.ts +++ b/packages/server/src/middleware/tests/trimViewRowInfo.spec.ts @@ -23,16 +23,16 @@ const mockGetTable = sdk.tables.getTable as jest.MockedFunction< class TestConfiguration { next: Next - throw: (status: number, message: string) => never + throw: jest.Mock<(status: number, message: string) => never> middleware: typeof trimViewRowInfoMiddleware params: Record request?: Pick, "body"> constructor() { this.next = jest.fn() - this.throw = (_status: any, message: any) => { + this.throw = jest.fn().mockImplementation((_status: any, message: any) => { throw new Error(message) - } + }) this.params = {} this.middleware = trimViewRowInfoMiddleware @@ -89,10 +89,12 @@ describe("trimViewRowInfo middleware", () => { }, }, } + let viewId: string = undefined! beforeEach(() => { jest.resetAllMocks() mockGetTable.mockResolvedValue(table) + viewId = utils.generateViewID(table._id!) }) const getRandomData = () => ({ @@ -103,7 +105,6 @@ describe("trimViewRowInfo middleware", () => { }) it("when no columns are defined, same data is returned", async () => { - const viewId = utils.generateViewID(table._id!) mockGetView.mockResolvedValue({ version: 2, id: viewId, @@ -119,10 +120,12 @@ describe("trimViewRowInfo middleware", () => { expect(config.request?.body).toEqual(data) expect(config.params.tableId).toEqual(table._id) + + expect(config.next).toBeCalledTimes(1) + expect(config.throw).not.toBeCalled() }) it("when columns are defined, trimmed data is returned", async () => { - const viewId = utils.generateViewID(table._id!) mockGetView.mockResolvedValue({ version: 2, id: viewId, @@ -143,5 +146,18 @@ describe("trimViewRowInfo middleware", () => { address: data.address, }) expect(config.params.tableId).toEqual(table._id) + + expect(config.next).toBeCalledTimes(1) + expect(config.throw).not.toBeCalled() + }) + + it("it should throw an error if no viewid is provided on the body", async () => { + const data = getRandomData() + await config.executeMiddleware(viewId, { + ...data, + }) + + expect(config.throw).toBeCalledTimes(1) + expect(config.throw).toBeCalledWith(400, "_viewId is required") }) }) From d8df917772931e76f72d7b5fd6ee534b078f6d4c Mon Sep 17 00:00:00 2001 From: Adria Navarro Date: Mon, 31 Jul 2023 14:08:31 +0200 Subject: [PATCH 13/19] More tests --- .../src/middleware/tests/trimViewRowInfo.spec.ts | 11 +++++++++++ packages/server/src/middleware/trimViewRowInfo.ts | 6 +++++- 2 files changed, 16 insertions(+), 1 deletion(-) diff --git a/packages/server/src/middleware/tests/trimViewRowInfo.spec.ts b/packages/server/src/middleware/tests/trimViewRowInfo.spec.ts index 888e22ef0a..aba6817e5c 100644 --- a/packages/server/src/middleware/tests/trimViewRowInfo.spec.ts +++ b/packages/server/src/middleware/tests/trimViewRowInfo.spec.ts @@ -160,4 +160,15 @@ describe("trimViewRowInfo middleware", () => { expect(config.throw).toBeCalledTimes(1) expect(config.throw).toBeCalledWith(400, "_viewId is required") }) + + it("it should throw an error if no viewid is provided on the parameters", async () => { + const data = getRandomData() + await config.executeMiddleware(undefined as any, { + _viewId: viewId, + ...data, + }) + + expect(config.throw).toBeCalledTimes(1) + expect(config.throw).toBeCalledWith(400, "viewId path is required") + }) }) diff --git a/packages/server/src/middleware/trimViewRowInfo.ts b/packages/server/src/middleware/trimViewRowInfo.ts index 5dbe7a6da0..2e3ded27f5 100644 --- a/packages/server/src/middleware/trimViewRowInfo.ts +++ b/packages/server/src/middleware/trimViewRowInfo.ts @@ -8,7 +8,11 @@ export default async (ctx: Ctx, next: Next) => { const { body } = ctx.request const { _viewId: viewId } = body if (!viewId) { - ctx.throw(400, "_viewId is required") + return ctx.throw(400, "_viewId is required") + } + + if (!ctx.params.viewId) { + return ctx.throw(400, "viewId path is required") } const { tableId } = utils.extractViewInfoFromID(ctx.params.viewId) From b3b4a6a17735838628d0c549e96aed813c366a0f Mon Sep 17 00:00:00 2001 From: Adria Navarro Date: Mon, 31 Jul 2023 14:14:59 +0200 Subject: [PATCH 14/19] Test noViewDataMiddleware --- packages/server/src/middleware/noViewData.ts | 4 +- .../src/middleware/tests/noViewData.spec.ts | 83 +++++++++++++++++++ .../middleware/tests/trimViewRowInfo.spec.ts | 8 +- 3 files changed, 89 insertions(+), 6 deletions(-) create mode 100644 packages/server/src/middleware/tests/noViewData.spec.ts diff --git a/packages/server/src/middleware/noViewData.ts b/packages/server/src/middleware/noViewData.ts index 5a2bb45012..809424b6bf 100644 --- a/packages/server/src/middleware/noViewData.ts +++ b/packages/server/src/middleware/noViewData.ts @@ -1,8 +1,8 @@ import { Ctx, Row } from "@budibase/types" -export default () => async (ctx: Ctx, next: any) => { +export default async (ctx: Ctx, next: any) => { if (ctx.request.body._viewId) { - ctx.throw(400, "Table row endpoints cannot contain view info") + return ctx.throw(400, "Table row endpoints cannot contain view info") } return next() diff --git a/packages/server/src/middleware/tests/noViewData.spec.ts b/packages/server/src/middleware/tests/noViewData.spec.ts new file mode 100644 index 0000000000..54b0ca8ff8 --- /dev/null +++ b/packages/server/src/middleware/tests/noViewData.spec.ts @@ -0,0 +1,83 @@ +import { generator } from "@budibase/backend-core/tests" +import { BBRequest, FieldType, Row, Table } from "@budibase/types" +import { Next } from "koa" +import * as utils from "../../db/utils" +import noViewDataMiddleware from "../noViewData" + +class TestConfiguration { + next: Next + throw: jest.Mock<(status: number, message: string) => never> + middleware: typeof noViewDataMiddleware + params: Record + request?: Pick, "body"> + + constructor() { + this.next = jest.fn() + this.throw = jest.fn() + this.params = {} + + this.middleware = noViewDataMiddleware + } + + executeMiddleware(ctxRequestBody: Row) { + this.request = { + body: ctxRequestBody, + } + return this.middleware( + { + request: this.request as any, + throw: this.throw as any, + params: this.params, + } as any, + this.next + ) + } + + afterEach() { + jest.clearAllMocks() + } +} + +describe("noViewData middleware", () => { + let config: TestConfiguration + + beforeEach(() => { + config = new TestConfiguration() + }) + + afterEach(() => { + config.afterEach() + }) + + const getRandomData = () => ({ + _id: generator.guid(), + name: generator.name(), + age: generator.age(), + address: generator.address(), + }) + + it("it should pass without view id data", async () => { + const data = getRandomData() + await config.executeMiddleware({ + ...data, + }) + + expect(config.next).toBeCalledTimes(1) + expect(config.throw).not.toBeCalled() + }) + + it("it should throw an error if _viewid is provided", async () => { + const data = getRandomData() + await config.executeMiddleware({ + _viewId: generator.guid(), + ...data, + }) + + expect(config.throw).toBeCalledTimes(1) + expect(config.throw).toBeCalledWith( + 400, + "Table row endpoints cannot contain view info" + ) + expect(config.next).not.toBeCalled() + }) +}) diff --git a/packages/server/src/middleware/tests/trimViewRowInfo.spec.ts b/packages/server/src/middleware/tests/trimViewRowInfo.spec.ts index aba6817e5c..89b831b647 100644 --- a/packages/server/src/middleware/tests/trimViewRowInfo.spec.ts +++ b/packages/server/src/middleware/tests/trimViewRowInfo.spec.ts @@ -14,7 +14,7 @@ jest.mock("../../sdk", () => ({ })) import sdk from "../../sdk" -import { BaseContext, Next } from "koa" +import { Next } from "koa" const mockGetView = sdk.views.get as jest.MockedFunction const mockGetTable = sdk.tables.getTable as jest.MockedFunction< @@ -30,9 +30,7 @@ class TestConfiguration { constructor() { this.next = jest.fn() - this.throw = jest.fn().mockImplementation((_status: any, message: any) => { - throw new Error(message) - }) + this.throw = jest.fn() this.params = {} this.middleware = trimViewRowInfoMiddleware @@ -159,6 +157,7 @@ describe("trimViewRowInfo middleware", () => { expect(config.throw).toBeCalledTimes(1) expect(config.throw).toBeCalledWith(400, "_viewId is required") + expect(config.next).not.toBeCalled() }) it("it should throw an error if no viewid is provided on the parameters", async () => { @@ -170,5 +169,6 @@ describe("trimViewRowInfo middleware", () => { expect(config.throw).toBeCalledTimes(1) expect(config.throw).toBeCalledWith(400, "viewId path is required") + expect(config.next).not.toBeCalled() }) }) From 1d13b49e866a193c241ade358fa10bcb701c07ec Mon Sep 17 00:00:00 2001 From: Adria Navarro Date: Mon, 31 Jul 2023 14:16:38 +0200 Subject: [PATCH 15/19] Fix middleware usage --- packages/server/src/api/routes/row.ts | 6 +++--- packages/server/src/api/routes/tests/row.spec.ts | 1 - 2 files changed, 3 insertions(+), 4 deletions(-) diff --git a/packages/server/src/api/routes/row.ts b/packages/server/src/api/routes/row.ts index 5d9a2d1df8..e530c3165b 100644 --- a/packages/server/src/api/routes/row.ts +++ b/packages/server/src/api/routes/row.ts @@ -176,7 +176,7 @@ router "/api/:tableId/rows", paramResource("tableId"), authorized(PermissionType.TABLE, PermissionLevel.WRITE), - noViewData(), + noViewData, rowController.save ) /** @@ -191,7 +191,7 @@ router "/api/:tableId/rows", paramResource("tableId"), authorized(PermissionType.TABLE, PermissionLevel.WRITE), - noViewData(), + noViewData, rowController.patch ) /** @@ -302,7 +302,7 @@ router "/api/v2/views/:viewId/rows", paramResource("viewId"), authorized(PermissionType.VIEW, PermissionLevel.WRITE), - trimViewRowInfo(), + trimViewRowInfo, rowController.save ) diff --git a/packages/server/src/api/routes/tests/row.spec.ts b/packages/server/src/api/routes/tests/row.spec.ts index ad521967b0..7705e84b47 100644 --- a/packages/server/src/api/routes/tests/row.spec.ts +++ b/packages/server/src/api/routes/tests/row.spec.ts @@ -16,7 +16,6 @@ import { FieldType, SortType, SortOrder, - PatchRowRequest, } from "@budibase/types" import { expectAnyInternalColsAttributes, From b43f921253d355158ff137432c1ad82c2c34fc0f Mon Sep 17 00:00:00 2001 From: Adria Navarro Date: Mon, 31 Jul 2023 15:26:01 +0200 Subject: [PATCH 16/19] Test middlewares --- .../server/src/api/routes/tests/row.spec.ts | 29 +++++++++++++++++++ 1 file changed, 29 insertions(+) diff --git a/packages/server/src/api/routes/tests/row.spec.ts b/packages/server/src/api/routes/tests/row.spec.ts index 7705e84b47..bf2eb0ce80 100644 --- a/packages/server/src/api/routes/tests/row.spec.ts +++ b/packages/server/src/api/routes/tests/row.spec.ts @@ -22,6 +22,9 @@ import { generator, structures, } from "@budibase/backend-core/tests" +import trimViewRowInfoMiddleware from "../../../middleware/trimViewRowInfo" +import noViewDataMiddleware from "../../../middleware/noViewData" +import router from "../row" describe("/rows", () => { let request = setup.getRequest() @@ -402,6 +405,14 @@ describe("/rows", () => { "Table row endpoints cannot contain view info" ) }) + + it("should setup the noViewData middleware", async () => { + const route = router.stack.find( + r => r.methods.includes("POST") && r.path === "/api/:tableId/rows" + ) + expect(route).toBeDefined() + expect(route?.stack).toContainEqual(noViewDataMiddleware) + }) }) describe("patch", () => { @@ -470,6 +481,14 @@ describe("/rows", () => { "Table row endpoints cannot contain view info" ) }) + + it("should setup the noViewData middleware", async () => { + const route = router.stack.find( + r => r.methods.includes("PATCH") && r.path === "/api/:tableId/rows" + ) + expect(route).toBeDefined() + expect(route?.stack).toContainEqual(noViewDataMiddleware) + }) }) describe("destroy", () => { @@ -1112,6 +1131,16 @@ describe("/rows", () => { expect(row.body.age).toBeUndefined() expect(row.body.jobTitle).toBeUndefined() }) + + it("should setup the trimViewRowInfo middleware", async () => { + const route = router.stack.find( + r => + r.methods.includes("POST") && + r.path === "/api/v2/views/:viewId/rows" + ) + expect(route).toBeDefined() + expect(route?.stack).toContainEqual(trimViewRowInfoMiddleware) + }) }) }) }) From d1ad443d181bed2bcd7654f3300914a1a5780610 Mon Sep 17 00:00:00 2001 From: Adria Navarro Date: Mon, 31 Jul 2023 15:41:13 +0200 Subject: [PATCH 17/19] Patch --- packages/server/src/api/routes/row.ts | 7 +++ .../server/src/api/routes/tests/row.spec.ts | 53 +++++++++++++++++++ .../server/src/tests/utilities/api/viewV2.ts | 16 ++++++ 3 files changed, 76 insertions(+) diff --git a/packages/server/src/api/routes/row.ts b/packages/server/src/api/routes/row.ts index e530c3165b..ba3b2404cc 100644 --- a/packages/server/src/api/routes/row.ts +++ b/packages/server/src/api/routes/row.ts @@ -305,5 +305,12 @@ router trimViewRowInfo, rowController.save ) + .patch( + "/api/v2/views/:viewId/rows/:rowId", + paramResource("viewId"), + authorized(PermissionType.VIEW, PermissionLevel.WRITE), + trimViewRowInfo, + rowController.patch + ) export default router diff --git a/packages/server/src/api/routes/tests/row.spec.ts b/packages/server/src/api/routes/tests/row.spec.ts index bf2eb0ce80..125edd668a 100644 --- a/packages/server/src/api/routes/tests/row.spec.ts +++ b/packages/server/src/api/routes/tests/row.spec.ts @@ -1142,5 +1142,58 @@ describe("/rows", () => { expect(route?.stack).toContainEqual(trimViewRowInfoMiddleware) }) }) + + describe("patch", () => { + it("should update only the view fields for a row", async () => { + const table = await config.createTable(userTable()) + const tableId = config.table!._id! + const view = await config.api.viewV2.create({ + tableId, + columns: { + name: { visible: true }, + address: { visible: true }, + }, + }) + + const newRow = await config.api.viewV2.row.create(view.id, { + tableId, + _viewId: view.id, + ...randomRowData(), + }) + const newData = randomRowData() + await config.api.viewV2.row.update(view.id, newRow._id!, { + tableId, + _viewId: view.id, + _id: newRow._id!, + _rev: newRow._rev!, + ...newData, + }) + + const row = await config.api.row.get(tableId, newRow._id!) + expect(row.body).toEqual({ + ...newRow, + type: "row", + name: newData.name, + address: newData.address, + _id: expect.any(String), + _rev: expect.any(String), + createdAt: expect.any(String), + updatedAt: expect.any(String), + }) + expect(row.body._viewId).toBeUndefined() + expect(row.body.age).toBeUndefined() + expect(row.body.jobTitle).toBeUndefined() + }) + + it("should setup the trimViewRowInfo middleware", async () => { + const route = router.stack.find( + r => + r.methods.includes("PATCH") && + r.path === "/api/v2/views/:viewId/rows/:rowId" + ) + expect(route).toBeDefined() + expect(route?.stack).toContainEqual(trimViewRowInfoMiddleware) + }) + }) }) }) diff --git a/packages/server/src/tests/utilities/api/viewV2.ts b/packages/server/src/tests/utilities/api/viewV2.ts index c2b5812fc3..1df056c630 100644 --- a/packages/server/src/tests/utilities/api/viewV2.ts +++ b/packages/server/src/tests/utilities/api/viewV2.ts @@ -1,5 +1,7 @@ import { CreateViewRequest, + PatchRowRequest, + PatchRowResponse, Row, SortOrder, SortType, @@ -114,5 +116,19 @@ export class ViewV2API extends TestAPI { .expect(expectStatus) return result.body as Row }, + update: async ( + viewId: string, + rowId: string, + row: PatchRowRequest, + { expectStatus } = { expectStatus: 200 } + ): Promise => { + const result = await this.request + .patch(`/api/v2/views/${viewId}/rows/${rowId}`) + .send(row) + .set(this.config.defaultHeaders()) + .expect("Content-Type", /json/) + .expect(expectStatus) + return result.body as PatchRowResponse + }, } } From 3335c86a84802fe027becb7557d098317cbb170c Mon Sep 17 00:00:00 2001 From: Adria Navarro Date: Mon, 31 Jul 2023 16:14:14 +0200 Subject: [PATCH 18/19] Delete row from view --- .../server/src/api/controllers/row/index.ts | 4 +- packages/server/src/api/routes/row.ts | 42 +++++++++++++++++++ .../server/src/api/routes/tests/row.spec.ts | 33 ++++++++++++++- .../server/src/tests/utilities/api/row.ts | 15 +++++-- .../server/src/tests/utilities/api/viewV2.ts | 13 ++++++ 5 files changed, 100 insertions(+), 7 deletions(-) diff --git a/packages/server/src/api/controllers/row/index.ts b/packages/server/src/api/controllers/row/index.ts index 1bb9569c49..d177fdc8a9 100644 --- a/packages/server/src/api/controllers/row/index.ts +++ b/packages/server/src/api/controllers/row/index.ts @@ -166,13 +166,13 @@ async function deleteRow(ctx: UserCtx) { const appId = ctx.appId const tableId = utils.getTableId(ctx) - let resp = await quotas.addQuery(() => pickApi(tableId).destroy(ctx), { + const 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) + gridSocket?.emitRowDeletion(ctx, resp.row._id!) return resp } diff --git a/packages/server/src/api/routes/row.ts b/packages/server/src/api/routes/row.ts index ba3b2404cc..b79347a871 100644 --- a/packages/server/src/api/routes/row.ts +++ b/packages/server/src/api/routes/row.ts @@ -6,6 +6,7 @@ import { permissions } from "@budibase/backend-core" import { internalSearchValidator } from "./utils/validators" import noViewData from "../../middleware/noViewData" import trimViewRowInfo from "../../middleware/trimViewRowInfo" +import * as utils from "../../db/utils" const { PermissionType, PermissionLevel } = permissions const router: Router = new Router() @@ -305,6 +306,14 @@ router trimViewRowInfo, rowController.save ) + /** + * @api {patch} /api/v2/views/:viewId/rows/:rowId Updates a row + * @apiName Update a row + * @apiGroup rows + * @apiPermission table write access + * @apiDescription This endpoint is identical to the row creation endpoint but instead it will + * error if an _id isn't provided, it will only function for existing rows. + */ .patch( "/api/v2/views/:viewId/rows/:rowId", paramResource("viewId"), @@ -312,5 +321,38 @@ router trimViewRowInfo, rowController.patch ) + /** + * @api {delete} /api/v2/views/:viewId/rows Delete rows for a view + * @apiName Delete rows for a view + * @apiGroup rows + * @apiPermission table write access + * @apiDescription This endpoint can delete a single row, or delete them in a bulk + * fashion. + * + * @apiParam {string} tableId The ID of the table the row is to be deleted from. + * + * @apiParam (Body) {object[]} [rows] If bulk deletion is desired then provide the rows in this + * key of the request body that are to be deleted. + * @apiParam (Body) {string} [_id] If deleting a single row then provide its ID in this field. + * @apiParam (Body) {string} [_rev] If deleting a single row from an internal table then provide its + * revision here. + * + * @apiSuccess {object[]|object} body If deleting bulk then the response body will be an array + * of the deleted rows, if deleting a single row then the body will contain a "row" property which + * is the deleted row. + */ + .delete( + "/api/v2/views/:viewId/rows", + paramResource("viewId"), + authorized(PermissionType.VIEW, PermissionLevel.WRITE), + // This is required as the implementation relies on the table id + (ctx, next) => { + ctx.params.tableId = utils.extractViewInfoFromID( + ctx.params.viewId + ).tableId + next() + }, + rowController.destroy + ) export default router diff --git a/packages/server/src/api/routes/tests/row.spec.ts b/packages/server/src/api/routes/tests/row.spec.ts index 125edd668a..b7df9777d4 100644 --- a/packages/server/src/api/routes/tests/row.spec.ts +++ b/packages/server/src/api/routes/tests/row.spec.ts @@ -16,6 +16,7 @@ import { FieldType, SortType, SortOrder, + DeleteRow, } from "@budibase/types" import { expectAnyInternalColsAttributes, @@ -1146,7 +1147,7 @@ describe("/rows", () => { describe("patch", () => { it("should update only the view fields for a row", async () => { const table = await config.createTable(userTable()) - const tableId = config.table!._id! + const tableId = table._id! const view = await config.api.viewV2.create({ tableId, columns: { @@ -1195,5 +1196,35 @@ describe("/rows", () => { expect(route?.stack).toContainEqual(trimViewRowInfoMiddleware) }) }) + + describe("destroy", () => { + it("should be able to delete a row", async () => { + const table = await config.createTable(userTable()) + const tableId = table._id! + const view = await config.api.viewV2.create({ + tableId, + columns: { + name: { visible: true }, + address: { visible: true }, + }, + }) + + const createdRow = await config.createRow() + const rowUsage = await getRowUsage() + const queryUsage = await getQueryUsage() + + const body: DeleteRow = { + _id: createdRow._id!, + } + await config.api.viewV2.row.delete(view.id, body) + + await assertRowUsage(rowUsage - 1) + await assertQueryUsage(queryUsage + 1) + + await config.api.row.get(tableId, createdRow._id!, { + expectStatus: 404, + }) + }) + }) }) }) diff --git a/packages/server/src/tests/utilities/api/row.ts b/packages/server/src/tests/utilities/api/row.ts index f4c80fa4c2..c7c72368f5 100644 --- a/packages/server/src/tests/utilities/api/row.ts +++ b/packages/server/src/tests/utilities/api/row.ts @@ -7,12 +7,19 @@ export class RowAPI extends TestAPI { super(config) } - get = async (tableId: string, rowId: string) => { - return await this.request + get = async ( + tableId: string, + rowId: string, + { expectStatus } = { expectStatus: 200 } + ) => { + const request = this.request .get(`/api/${tableId}/rows/${rowId}`) .set(this.config.defaultHeaders()) - .expect("Content-Type", /json/) - .expect(200) + .expect(expectStatus) + if (expectStatus !== 404) { + request.expect("Content-Type", /json/) + } + return request } patch = async ( diff --git a/packages/server/src/tests/utilities/api/viewV2.ts b/packages/server/src/tests/utilities/api/viewV2.ts index 1df056c630..a3e58bc537 100644 --- a/packages/server/src/tests/utilities/api/viewV2.ts +++ b/packages/server/src/tests/utilities/api/viewV2.ts @@ -1,5 +1,6 @@ import { CreateViewRequest, + DeleteRowRequest, PatchRowRequest, PatchRowResponse, Row, @@ -130,5 +131,17 @@ export class ViewV2API extends TestAPI { .expect(expectStatus) return result.body as PatchRowResponse }, + delete: async ( + viewId: string, + body: DeleteRowRequest, + { expectStatus } = { expectStatus: 200 } + ): Promise => { + const result = await this.request + .delete(`/api/v2/views/${viewId}/rows`) + .send(body) + .set(this.config.defaultHeaders()) + .expect(expectStatus) + return result.body + }, } } From 76e836ca555595965115c68c9a9e45932a4c6b44 Mon Sep 17 00:00:00 2001 From: Adria Navarro Date: Mon, 31 Jul 2023 16:37:24 +0200 Subject: [PATCH 19/19] Test deleting multiple rows --- .../server/src/api/controllers/row/index.ts | 2 +- packages/server/src/api/routes/row.ts | 2 +- .../server/src/api/routes/tests/row.spec.ts | 35 +++++++++++++++++++ 3 files changed, 37 insertions(+), 2 deletions(-) diff --git a/packages/server/src/api/controllers/row/index.ts b/packages/server/src/api/controllers/row/index.ts index d177fdc8a9..7f6f494621 100644 --- a/packages/server/src/api/controllers/row/index.ts +++ b/packages/server/src/api/controllers/row/index.ts @@ -146,7 +146,7 @@ async function deleteRows(ctx: UserCtx) { const rowDeletes: Row[] = await processDeleteRowsRequest(ctx) deleteRequest.rows = rowDeletes - let { rows } = await quotas.addQuery( + const { rows } = await quotas.addQuery( () => pickApi(tableId).bulkDestroy(ctx), { datasourceId: tableId, diff --git a/packages/server/src/api/routes/row.ts b/packages/server/src/api/routes/row.ts index b79347a871..ac0cd2b4a4 100644 --- a/packages/server/src/api/routes/row.ts +++ b/packages/server/src/api/routes/row.ts @@ -350,7 +350,7 @@ router ctx.params.tableId = utils.extractViewInfoFromID( ctx.params.viewId ).tableId - next() + return next() }, rowController.destroy ) diff --git a/packages/server/src/api/routes/tests/row.spec.ts b/packages/server/src/api/routes/tests/row.spec.ts index b7df9777d4..38399c81cd 100644 --- a/packages/server/src/api/routes/tests/row.spec.ts +++ b/packages/server/src/api/routes/tests/row.spec.ts @@ -1225,6 +1225,41 @@ describe("/rows", () => { expectStatus: 404, }) }) + + it("should be able to delete multiple rows", async () => { + const table = await config.createTable(userTable()) + const tableId = table._id! + const view = await config.api.viewV2.create({ + tableId, + columns: { + name: { visible: true }, + address: { visible: true }, + }, + }) + + const rows = [ + await config.createRow(), + await config.createRow(), + await config.createRow(), + ] + const rowUsage = await getRowUsage() + const queryUsage = await getQueryUsage() + + await config.api.viewV2.row.delete(view.id, { + rows: [rows[0], rows[2]], + }) + + await assertRowUsage(rowUsage - 2) + await assertQueryUsage(queryUsage + 1) + + await config.api.row.get(tableId, rows[0]._id!, { + expectStatus: 404, + }) + await config.api.row.get(tableId, rows[2]._id!, { + expectStatus: 404, + }) + await config.api.row.get(tableId, rows[1]._id!, { expectStatus: 200 }) + }) }) }) })