From a1d85a831c7118c62918722d3923fc4255945f59 Mon Sep 17 00:00:00 2001 From: mike12345567 Date: Fri, 4 Aug 2023 16:58:40 +0100 Subject: [PATCH 1/7] Some quick modifications to allow the views to go through the standard row CRUD, the view search is still separate for now however this may change. --- packages/server/src/api/routes/row.ts | 91 +------------------ .../server/src/api/routes/tests/row.spec.ts | 17 ---- packages/server/src/db/utils.ts | 12 ++- packages/server/src/middleware/noViewData.ts | 9 -- .../src/middleware/tests/noViewData.spec.ts | 83 ----------------- .../server/src/middleware/trimViewRowInfo.ts | 14 +-- packages/server/src/sdk/app/views/index.ts | 7 +- packages/types/src/documents/document.ts | 1 + packages/types/src/sdk/permissions.ts | 1 - 9 files changed, 25 insertions(+), 210 deletions(-) delete mode 100644 packages/server/src/middleware/noViewData.ts delete mode 100644 packages/server/src/middleware/tests/noViewData.spec.ts diff --git a/packages/server/src/api/routes/row.ts b/packages/server/src/api/routes/row.ts index ac0cd2b4a4..3227df98ed 100644 --- a/packages/server/src/api/routes/row.ts +++ b/packages/server/src/api/routes/row.ts @@ -4,9 +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 noViewData from "../../middleware/noViewData" import trimViewRowInfo from "../../middleware/trimViewRowInfo" -import * as utils from "../../db/utils" const { PermissionType, PermissionLevel } = permissions const router: Router = new Router() @@ -177,7 +175,7 @@ router "/api/:tableId/rows", paramResource("tableId"), authorized(PermissionType.TABLE, PermissionLevel.WRITE), - noViewData, + trimViewRowInfo, rowController.save ) /** @@ -192,7 +190,7 @@ router "/api/:tableId/rows", paramResource("tableId"), authorized(PermissionType.TABLE, PermissionLevel.WRITE), - noViewData, + trimViewRowInfo, rowController.patch ) /** @@ -245,6 +243,7 @@ router "/api/:tableId/rows", paramResource("tableId"), authorized(PermissionType.TABLE, PermissionLevel.WRITE), + trimViewRowInfo, rowController.destroy ) @@ -267,92 +266,10 @@ router authorized(PermissionType.TABLE, PermissionLevel.WRITE), rowController.exportRows ) - -router .get( "/api/v2/views/:viewId/search", - authorized(PermissionType.VIEW, PermissionLevel.READ), + authorized(PermissionType.TABLE, 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. - */ - .post( - "/api/v2/views/:viewId/rows", - paramResource("viewId"), - authorized(PermissionType.VIEW, PermissionLevel.WRITE), - 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"), - authorized(PermissionType.VIEW, PermissionLevel.WRITE), - 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 - return 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 b986ffb945..86c41b8503 100644 --- a/packages/server/src/api/routes/tests/row.spec.ts +++ b/packages/server/src/api/routes/tests/row.spec.ts @@ -24,7 +24,6 @@ import { structures, } from "@budibase/backend-core/tests" import trimViewRowInfoMiddleware from "../../../middleware/trimViewRowInfo" -import noViewDataMiddleware from "../../../middleware/noViewData" import router from "../row" describe("/rows", () => { @@ -406,14 +405,6 @@ 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", () => { @@ -482,14 +473,6 @@ 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", () => { diff --git a/packages/server/src/db/utils.ts b/packages/server/src/db/utils.ts index dda14a9187..e8dc3b3f6c 100644 --- a/packages/server/src/db/utils.ts +++ b/packages/server/src/db/utils.ts @@ -284,10 +284,20 @@ export function getMultiIDParams(ids: string[]) { * @returns {string} The new view ID which the view doc can be stored under. */ export function generateViewID(tableId: string) { - return `${tableId}${SEPARATOR}${newid()}` + return `${DocumentType.VIEW}${SEPARATOR}${tableId}${SEPARATOR}${newid()}` +} + +export function isViewID(viewId: string) { + return viewId?.split(SEPARATOR)[0] === DocumentType.VIEW } export function extractViewInfoFromID(viewId: string) { + if (!isViewID(viewId)) { + throw new Error("Unable to extract table ID, is not a view ID") + } + const split = viewId.split(SEPARATOR) + split.shift() + viewId = split.join(SEPARATOR) const regex = new RegExp(`^(?.+)${SEPARATOR}([^${SEPARATOR}]+)$`) const res = regex.exec(viewId) return { diff --git a/packages/server/src/middleware/noViewData.ts b/packages/server/src/middleware/noViewData.ts deleted file mode 100644 index 809424b6bf..0000000000 --- a/packages/server/src/middleware/noViewData.ts +++ /dev/null @@ -1,9 +0,0 @@ -import { Ctx, Row } from "@budibase/types" - -export default async (ctx: Ctx, next: any) => { - if (ctx.request.body._viewId) { - 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 deleted file mode 100644 index 54b0ca8ff8..0000000000 --- a/packages/server/src/middleware/tests/noViewData.spec.ts +++ /dev/null @@ -1,83 +0,0 @@ -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/trimViewRowInfo.ts b/packages/server/src/middleware/trimViewRowInfo.ts index 2e3ded27f5..763552c3d7 100644 --- a/packages/server/src/middleware/trimViewRowInfo.ts +++ b/packages/server/src/middleware/trimViewRowInfo.ts @@ -7,15 +7,15 @@ import { Next } from "koa" export default async (ctx: Ctx, next: Next) => { const { body } = ctx.request const { _viewId: viewId } = body - if (!viewId) { - return ctx.throw(400, "_viewId is required") + + const possibleViewId = ctx.params.tableId + + // nothing to do, it is not a view (just a table ID) + if (!viewId || !utils.isViewID(possibleViewId)) { + return next() } - if (!ctx.params.viewId) { - return ctx.throw(400, "viewId path is required") - } - - const { tableId } = utils.extractViewInfoFromID(ctx.params.viewId) + const { tableId } = utils.extractViewInfoFromID(possibleViewId) const { _viewId, ...trimmedView } = await trimViewFields( viewId, tableId, diff --git a/packages/server/src/sdk/app/views/index.ts b/packages/server/src/sdk/app/views/index.ts index 7e75f22060..637caa06ee 100644 --- a/packages/server/src/sdk/app/views/index.ts +++ b/packages/server/src/sdk/app/views/index.ts @@ -1,17 +1,14 @@ -import { HTTPError, context } from "@budibase/backend-core" +import { context, HTTPError } from "@budibase/backend-core" import { FieldSchema, TableSchema, View, ViewV2 } from "@budibase/types" import sdk from "../../../sdk" import * as utils from "../../../db/utils" -import merge from "lodash/merge" export async function get(viewId: string): Promise { const { tableId } = utils.extractViewInfoFromID(viewId) const table = await sdk.tables.getTable(tableId) const views = Object.values(table.views!) - const view = views.find(v => isV2(v) && v.id === viewId) as ViewV2 | undefined - - return view + return views.find(v => isV2(v) && v.id === viewId) as ViewV2 | undefined } export async function create( diff --git a/packages/types/src/documents/document.ts b/packages/types/src/documents/document.ts index 6ba318269b..164aa79ac9 100644 --- a/packages/types/src/documents/document.ts +++ b/packages/types/src/documents/document.ts @@ -37,6 +37,7 @@ export enum DocumentType { USER_FLAG = "flag", AUTOMATION_METADATA = "meta_au", AUDIT_LOG = "al", + VIEW = "view", } export interface Document { diff --git a/packages/types/src/sdk/permissions.ts b/packages/types/src/sdk/permissions.ts index 9e51e4c7b4..9fe1970e44 100644 --- a/packages/types/src/sdk/permissions.ts +++ b/packages/types/src/sdk/permissions.ts @@ -14,6 +14,5 @@ export enum PermissionType { WEBHOOK = "webhook", BUILDER = "builder", GLOBAL_BUILDER = "globalBuilder", - VIEW = "view", QUERY = "query", } From 41a904126817d1f72014dc44c88e8c093ab927af Mon Sep 17 00:00:00 2001 From: mike12345567 Date: Mon, 7 Aug 2023 16:36:28 +0100 Subject: [PATCH 2/7] PR fixes. --- packages/backend-core/src/security/permissions.ts | 4 ---- packages/server/src/api/routes/row.ts | 11 +++++------ 2 files changed, 5 insertions(+), 10 deletions(-) diff --git a/packages/backend-core/src/security/permissions.ts b/packages/backend-core/src/security/permissions.ts index 70dae57ae6..aa0b20a30c 100644 --- a/packages/backend-core/src/security/permissions.ts +++ b/packages/backend-core/src/security/permissions.ts @@ -78,7 +78,6 @@ export const BUILTIN_PERMISSIONS = { permissions: [ new Permission(PermissionType.QUERY, PermissionLevel.READ), new Permission(PermissionType.TABLE, PermissionLevel.READ), - new Permission(PermissionType.VIEW, PermissionLevel.READ), ], }, WRITE: { @@ -87,7 +86,6 @@ export const BUILTIN_PERMISSIONS = { permissions: [ new Permission(PermissionType.QUERY, PermissionLevel.WRITE), new Permission(PermissionType.TABLE, PermissionLevel.WRITE), - new Permission(PermissionType.VIEW, PermissionLevel.READ), new Permission(PermissionType.AUTOMATION, PermissionLevel.EXECUTE), ], }, @@ -98,7 +96,6 @@ export const BUILTIN_PERMISSIONS = { new Permission(PermissionType.TABLE, PermissionLevel.WRITE), new Permission(PermissionType.USER, PermissionLevel.READ), new Permission(PermissionType.AUTOMATION, PermissionLevel.EXECUTE), - new Permission(PermissionType.VIEW, PermissionLevel.READ), new Permission(PermissionType.WEBHOOK, PermissionLevel.READ), ], }, @@ -109,7 +106,6 @@ export const BUILTIN_PERMISSIONS = { new Permission(PermissionType.TABLE, PermissionLevel.ADMIN), new Permission(PermissionType.USER, PermissionLevel.ADMIN), new Permission(PermissionType.AUTOMATION, PermissionLevel.ADMIN), - new Permission(PermissionType.VIEW, PermissionLevel.ADMIN), new Permission(PermissionType.WEBHOOK, PermissionLevel.READ), new Permission(PermissionType.QUERY, PermissionLevel.ADMIN), ], diff --git a/packages/server/src/api/routes/row.ts b/packages/server/src/api/routes/row.ts index 55906c2ffe..1fd45e0e92 100644 --- a/packages/server/src/api/routes/row.ts +++ b/packages/server/src/api/routes/row.ts @@ -267,11 +267,10 @@ router rowController.exportRows ) -router - .post( - "/api/v2/views/:viewId/search", - authorized(PermissionType.TABLE, PermissionLevel.READ), - rowController.views.searchView - ) +router.post( + "/api/v2/views/:viewId/search", + authorized(PermissionType.TABLE, PermissionLevel.READ), + rowController.views.searchView +) export default router From 4c2e3a54893db23f7c8f0524e079004e7cbf4415 Mon Sep 17 00:00:00 2001 From: mike12345567 Date: Mon, 7 Aug 2023 16:49:13 +0100 Subject: [PATCH 3/7] Updating last remaining view perms to table perms. --- packages/server/src/api/routes/view.ts | 2 +- packages/server/src/utilities/security.ts | 4 +--- 2 files changed, 2 insertions(+), 4 deletions(-) diff --git a/packages/server/src/api/routes/view.ts b/packages/server/src/api/routes/view.ts index 18c58134b4..07e31fc701 100644 --- a/packages/server/src/api/routes/view.ts +++ b/packages/server/src/api/routes/view.ts @@ -34,7 +34,7 @@ router "/api/views/:viewName", paramResource("viewName"), authorized( - permissions.PermissionType.VIEW, + permissions.PermissionType.TABLE, permissions.PermissionLevel.READ ), rowController.fetchView diff --git a/packages/server/src/utilities/security.ts b/packages/server/src/utilities/security.ts index 694dff4360..54e19007f1 100644 --- a/packages/server/src/utilities/security.ts +++ b/packages/server/src/utilities/security.ts @@ -14,6 +14,7 @@ export function getPermissionType(resourceId: string) { switch (docType) { case DocumentType.TABLE: case DocumentType.ROW: + case DocumentType.VIEW: return permissions.PermissionType.TABLE case DocumentType.AUTOMATION: return permissions.PermissionType.AUTOMATION @@ -22,9 +23,6 @@ export function getPermissionType(resourceId: string) { case DocumentType.QUERY: case DocumentType.DATASOURCE: return permissions.PermissionType.QUERY - default: - // views don't have an ID, will end up here - return permissions.PermissionType.VIEW } } From 2011e1693e35b4102bdbb131dfd23d2f9abd3632 Mon Sep 17 00:00:00 2001 From: mike12345567 Date: Tue, 8 Aug 2023 12:06:25 +0100 Subject: [PATCH 4/7] PR comments. --- .../server/src/api/controllers/row/utils.ts | 9 ++- packages/server/src/api/routes/row.ts | 72 +++++++++---------- packages/server/src/db/utils.ts | 9 ++- packages/server/src/utilities/security.ts | 6 +- packages/types/src/documents/document.ts | 6 ++ 5 files changed, 57 insertions(+), 45 deletions(-) diff --git a/packages/server/src/api/controllers/row/utils.ts b/packages/server/src/api/controllers/row/utils.ts index 6ff90f2b25..6cc342b8a0 100644 --- a/packages/server/src/api/controllers/row/utils.ts +++ b/packages/server/src/api/controllers/row/utils.ts @@ -45,13 +45,16 @@ export async function findRow(ctx: UserCtx, tableId: string, rowId: string) { } export function getTableId(ctx: Ctx) { - if (ctx.request.body && ctx.request.body.tableId) { + if (ctx.request.body?.tableId) { return ctx.request.body.tableId } - if (ctx.params && ctx.params.tableId) { + if (ctx.params?.sourceId) { + return ctx.params.sourceId + } + if (ctx.params?.tableId) { return ctx.params.tableId } - if (ctx.params && ctx.params.viewName) { + if (ctx.params?.viewName) { return ctx.params.viewName } } diff --git a/packages/server/src/api/routes/row.ts b/packages/server/src/api/routes/row.ts index 1fd45e0e92..a4ac8aa3ee 100644 --- a/packages/server/src/api/routes/row.ts +++ b/packages/server/src/api/routes/row.ts @@ -11,7 +11,7 @@ const router: Router = new Router() router /** - * @api {get} /api/:tableId/:rowId/enrich Get an enriched row + * @api {get} /api/:sourceId/:rowId/enrich Get an enriched row * @apiName Get an enriched row * @apiGroup rows * @apiPermission table read access @@ -25,13 +25,13 @@ router * @apiSuccess {object} row The response body will be the enriched row. */ .get( - "/api/:tableId/:rowId/enrich", - paramSubResource("tableId", "rowId"), + "/api/:sourceId/:rowId/enrich", + paramSubResource("sourceId", "rowId"), authorized(PermissionType.TABLE, PermissionLevel.READ), rowController.fetchEnrichedRow ) /** - * @api {get} /api/:tableId/rows Get all rows in a table + * @api {get} /api/:sourceId/rows Get all rows in a table * @apiName Get all rows in a table * @apiGroup rows * @apiPermission table read access @@ -40,37 +40,37 @@ router * due to its lack of support for pagination. With SQL tables this will retrieve up to a limit and then * will simply stop. * - * @apiParam {string} tableId The ID of the table to retrieve all rows within. + * @apiParam {string} sourceId The ID of the table to retrieve all rows within. * * @apiSuccess {object[]} rows The response body will be an array of all rows found. */ .get( - "/api/:tableId/rows", - paramResource("tableId"), + "/api/:sourceId/rows", + paramResource("sourceId"), authorized(PermissionType.TABLE, PermissionLevel.READ), rowController.fetch ) /** - * @api {get} /api/:tableId/rows/:rowId Retrieve a single row + * @api {get} /api/:sourceId/rows/:rowId Retrieve a single row * @apiName Retrieve a single row * @apiGroup rows * @apiPermission table read access * @apiDescription This endpoint retrieves only the specified row. If you wish to retrieve * a row by anything other than its _id field, use the search endpoint. * - * @apiParam {string} tableId The ID of the table to retrieve a row from. + * @apiParam {string} sourceId The ID of the table to retrieve a row from. * @apiParam {string} rowId The ID of the row to retrieve. * * @apiSuccess {object} body The response body will be the row that was found. */ .get( - "/api/:tableId/rows/:rowId", - paramSubResource("tableId", "rowId"), + "/api/:sourceId/rows/:rowId", + paramSubResource("sourceId", "rowId"), authorized(PermissionType.TABLE, PermissionLevel.READ), rowController.find ) /** - * @api {post} /api/:tableId/search Search for rows in a table + * @api {post} /api/:sourceId/search Search for rows in a table * @apiName Search for rows in a table * @apiGroup rows * @apiPermission table read access @@ -78,7 +78,7 @@ router * and data UI in the builder are built atop this. All filtering, sorting and pagination is * handled through this, for internal and external (datasource plus, e.g. SQL) tables. * - * @apiParam {string} tableId The ID of the table to retrieve rows from. + * @apiParam {string} sourceId The ID of the table to retrieve rows from. * * @apiParam (Body) {boolean} [paginate] If pagination is required then this should be set to true, * defaults to false. @@ -133,22 +133,22 @@ router * page. */ .post( - "/api/:tableId/search", + "/api/:sourceId/search", internalSearchValidator(), - paramResource("tableId"), + paramResource("sourceId"), authorized(PermissionType.TABLE, PermissionLevel.READ), rowController.search ) // DEPRECATED - this is an old API, but for backwards compat it needs to be // supported still .post( - "/api/search/:tableId/rows", - paramResource("tableId"), + "/api/search/:sourceId/rows", + paramResource("sourceId"), authorized(PermissionType.TABLE, PermissionLevel.READ), rowController.search ) /** - * @api {post} /api/:tableId/rows Creates a new row + * @api {post} /api/:sourceId/rows Creates a new row * @apiName Creates a new row * @apiGroup rows * @apiPermission table write access @@ -157,7 +157,7 @@ router * 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 {string} sourceId 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 @@ -172,14 +172,14 @@ router * @apiSuccess {object} body The contents of the row that was saved will be returned as well. */ .post( - "/api/:tableId/rows", - paramResource("tableId"), + "/api/:sourceId/rows", + paramResource("sourceId"), authorized(PermissionType.TABLE, PermissionLevel.WRITE), trimViewRowInfo, rowController.save ) /** - * @api {patch} /api/:tableId/rows Updates a row + * @api {patch} /api/:sourceId/rows Updates a row * @apiName Update a row * @apiGroup rows * @apiPermission table write access @@ -187,14 +187,14 @@ router * error if an _id isn't provided, it will only function for existing rows. */ .patch( - "/api/:tableId/rows", - paramResource("tableId"), + "/api/:sourceId/rows", + paramResource("sourceId"), authorized(PermissionType.TABLE, PermissionLevel.WRITE), trimViewRowInfo, rowController.patch ) /** - * @api {post} /api/:tableId/rows/validate Validate inputs for a row + * @api {post} /api/:sourceId/rows/validate Validate inputs for a row * @apiName Validate inputs for a row * @apiGroup rows * @apiPermission table write access @@ -202,7 +202,7 @@ router * given the table schema, this will iterate through all the constraints on the table and * check if the request body is valid. * - * @apiParam {string} tableId The ID of the table the row is to be validated for. + * @apiParam {string} sourceId The ID of the table the row is to be validated for. * * @apiParam (Body) {any} [any] Any fields provided in the request body will be tested * against the table schema and constraints. @@ -214,20 +214,20 @@ router * the schema. */ .post( - "/api/:tableId/rows/validate", - paramResource("tableId"), + "/api/:sourceId/rows/validate", + paramResource("sourceId"), authorized(PermissionType.TABLE, PermissionLevel.WRITE), rowController.validate ) /** - * @api {delete} /api/:tableId/rows Delete rows + * @api {delete} /api/:sourceId/rows Delete rows * @apiName Delete rows * @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 {string} sourceId 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. @@ -240,29 +240,29 @@ router * is the deleted row. */ .delete( - "/api/:tableId/rows", - paramResource("tableId"), + "/api/:sourceId/rows", + paramResource("sourceId"), authorized(PermissionType.TABLE, PermissionLevel.WRITE), trimViewRowInfo, rowController.destroy ) /** - * @api {post} /api/:tableId/rows/exportRows Export Rows + * @api {post} /api/:sourceId/rows/exportRows Export Rows * @apiName Export rows * @apiGroup rows * @apiPermission table write access * @apiDescription This API can export a number of provided rows * - * @apiParam {string} tableId The ID of the table the row is to be deleted from. + * @apiParam {string} sourceId The ID of the table the row is to be deleted from. * * @apiParam (Body) {object[]} [rows] The row IDs which are to be exported * * @apiSuccess {object[]|object} */ .post( - "/api/:tableId/rows/exportRows", - paramResource("tableId"), + "/api/:sourceId/rows/exportRows", + paramResource("sourceId"), authorized(PermissionType.TABLE, PermissionLevel.WRITE), rowController.exportRows ) diff --git a/packages/server/src/db/utils.ts b/packages/server/src/db/utils.ts index e8dc3b3f6c..abea725707 100644 --- a/packages/server/src/db/utils.ts +++ b/packages/server/src/db/utils.ts @@ -1,5 +1,7 @@ import newid from "./newid" import { db as dbCore } from "@budibase/backend-core" +import { DocumentType, VirtualDocumentType } from "@budibase/types" +export { DocumentType, VirtualDocumentType } from "@budibase/types" type Optional = string | null @@ -19,7 +21,6 @@ export const BudibaseInternalDB = { export const SEPARATOR = dbCore.SEPARATOR export const StaticDatabases = dbCore.StaticDatabases -export const DocumentType = dbCore.DocumentType export const APP_PREFIX = dbCore.APP_PREFIX export const APP_DEV_PREFIX = dbCore.APP_DEV_PREFIX export const isDevAppID = dbCore.isDevAppID @@ -284,11 +285,13 @@ export function getMultiIDParams(ids: string[]) { * @returns {string} The new view ID which the view doc can be stored under. */ export function generateViewID(tableId: string) { - return `${DocumentType.VIEW}${SEPARATOR}${tableId}${SEPARATOR}${newid()}` + return `${ + VirtualDocumentType.VIEW + }${SEPARATOR}${tableId}${SEPARATOR}${newid()}` } export function isViewID(viewId: string) { - return viewId?.split(SEPARATOR)[0] === DocumentType.VIEW + return viewId?.split(SEPARATOR)[0] === VirtualDocumentType.VIEW } export function extractViewInfoFromID(viewId: string) { diff --git a/packages/server/src/utilities/security.ts b/packages/server/src/utilities/security.ts index 54e19007f1..0da7621773 100644 --- a/packages/server/src/utilities/security.ts +++ b/packages/server/src/utilities/security.ts @@ -1,5 +1,5 @@ import { permissions, roles } from "@budibase/backend-core" -import { DocumentType } from "../db/utils" +import { DocumentType, VirtualDocumentType } from "../db/utils" export const CURRENTLY_SUPPORTED_LEVELS: string[] = [ permissions.PermissionLevel.WRITE, @@ -11,10 +11,10 @@ export function getPermissionType(resourceId: string) { const docType = Object.values(DocumentType).filter(docType => resourceId.startsWith(docType) )[0] - switch (docType) { + switch (docType as DocumentType | VirtualDocumentType) { case DocumentType.TABLE: case DocumentType.ROW: - case DocumentType.VIEW: + case VirtualDocumentType.VIEW: return permissions.PermissionType.TABLE case DocumentType.AUTOMATION: return permissions.PermissionType.AUTOMATION diff --git a/packages/types/src/documents/document.ts b/packages/types/src/documents/document.ts index 164aa79ac9..75f55e1367 100644 --- a/packages/types/src/documents/document.ts +++ b/packages/types/src/documents/document.ts @@ -37,6 +37,12 @@ export enum DocumentType { USER_FLAG = "flag", AUTOMATION_METADATA = "meta_au", AUDIT_LOG = "al", + VIEW = "awd", +} + +// these documents don't really exist, they are part of other +// documents or enriched into existence as part of get requests +export enum VirtualDocumentType { VIEW = "view", } From 0abd1deb3492f80b98b0d34a6709254f393020a6 Mon Sep 17 00:00:00 2001 From: mike12345567 Date: Tue, 8 Aug 2023 13:19:22 +0100 Subject: [PATCH 5/7] Updating test cases, fixes based on running through view/row API. --- .../src/api/controllers/row/external.ts | 13 ++-- .../src/api/controllers/row/internal.ts | 23 +++--- .../server/src/api/controllers/row/utils.ts | 10 ++- .../server/src/api/routes/tests/row.spec.ts | 71 ++----------------- .../server/src/middleware/trimViewRowInfo.ts | 31 +++++--- .../server/src/tests/utilities/api/row.ts | 37 ++++++++-- .../server/src/tests/utilities/api/viewV2.ts | 46 ------------ packages/types/src/api/web/app/rows.ts | 2 + 8 files changed, 87 insertions(+), 146 deletions(-) diff --git a/packages/server/src/api/controllers/row/external.ts b/packages/server/src/api/controllers/row/external.ts index 3ee4ca6edd..bc4edbd661 100644 --- a/packages/server/src/api/controllers/row/external.ts +++ b/packages/server/src/api/controllers/row/external.ts @@ -15,6 +15,7 @@ import { UserCtx, } from "@budibase/types" import sdk from "../../../sdk" +import * as utils from "./utils" export async function handleRequest( operation: Operation, @@ -43,7 +44,7 @@ export async function handleRequest( } export async function patch(ctx: UserCtx) { - const tableId = ctx.params.tableId + const tableId = utils.getTableId(ctx) const { _id, ...rowData } = ctx.request.body const validateResult = await sdk.rows.utils.validate({ @@ -70,7 +71,7 @@ export async function patch(ctx: UserCtx) { export async function save(ctx: UserCtx) { const inputs = ctx.request.body - const tableId = ctx.params.tableId + const tableId = utils.getTableId(ctx) const validateResult = await sdk.rows.utils.validate({ row: inputs, tableId, @@ -98,12 +99,12 @@ export async function save(ctx: UserCtx) { export async function find(ctx: UserCtx) { const id = ctx.params.rowId - const tableId = ctx.params.tableId + const tableId = utils.getTableId(ctx) return sdk.rows.external.getRow(tableId, id) } export async function destroy(ctx: UserCtx) { - const tableId = ctx.params.tableId + const tableId = utils.getTableId(ctx) const _id = ctx.request.body._id const { row } = (await handleRequest(Operation.DELETE, tableId, { id: breakRowIdField(_id), @@ -114,7 +115,7 @@ export async function destroy(ctx: UserCtx) { export async function bulkDestroy(ctx: UserCtx) { const { rows } = ctx.request.body - const tableId = ctx.params.tableId + const tableId = utils.getTableId(ctx) let promises: Promise[] = [] for (let row of rows) { promises.push( @@ -130,7 +131,7 @@ export async function bulkDestroy(ctx: UserCtx) { export async function fetchEnrichedRow(ctx: UserCtx) { const id = ctx.params.rowId - const tableId = ctx.params.tableId + const tableId = utils.getTableId(ctx) const { datasourceId, tableName } = breakExternalTableId(tableId) const datasource: Datasource = await sdk.datasources.get(datasourceId!) if (!tableName) { diff --git a/packages/server/src/api/controllers/row/internal.ts b/packages/server/src/api/controllers/row/internal.ts index 2ff1df0933..3432ec80f3 100644 --- a/packages/server/src/api/controllers/row/internal.ts +++ b/packages/server/src/api/controllers/row/internal.ts @@ -13,7 +13,7 @@ import { import { FieldTypes } from "../../../constants" import * as utils from "./utils" import { cloneDeep } from "lodash/fp" -import { context, db as dbCore } from "@budibase/backend-core" +import { context } from "@budibase/backend-core" import { finaliseRow, updateRelatedFormula } from "./staticFormula" import { UserCtx, @@ -26,8 +26,8 @@ import { import sdk from "../../../sdk" export async function patch(ctx: UserCtx) { + const tableId = utils.getTableId(ctx) const inputs = ctx.request.body - const tableId = inputs.tableId const isUserTable = tableId === InternalTables.USER_METADATA let oldRow const dbTable = await sdk.tables.getTable(tableId) @@ -94,7 +94,8 @@ export async function patch(ctx: UserCtx) { export async function save(ctx: UserCtx) { let inputs = ctx.request.body - inputs.tableId = ctx.params.tableId + const tableId = utils.getTableId(ctx) + inputs.tableId = tableId if (!inputs._rev && !inputs._id) { inputs._id = generateRowID(inputs.tableId) @@ -132,20 +133,22 @@ export async function save(ctx: UserCtx) { } export async function find(ctx: UserCtx) { - const db = dbCore.getDB(ctx.appId) - const table = await sdk.tables.getTable(ctx.params.tableId) - let row = await utils.findRow(ctx, ctx.params.tableId, ctx.params.rowId) + const tableId = utils.getTableId(ctx), + rowId = ctx.params.rowId + const table = await sdk.tables.getTable(tableId) + let row = await utils.findRow(ctx, tableId, rowId) row = await outputProcessing(table, row) return row } export async function destroy(ctx: UserCtx) { const db = context.getAppDB() + const tableId = utils.getTableId(ctx) const { _id } = ctx.request.body let row = await db.get(_id) let _rev = ctx.request.body._rev || row._rev - if (row.tableId !== ctx.params.tableId) { + if (row.tableId !== tableId) { throw "Supplied tableId doesn't match the row's tableId" } const table = await sdk.tables.getTable(row.tableId) @@ -163,7 +166,7 @@ export async function destroy(ctx: UserCtx) { await updateRelatedFormula(table, row) let response - if (ctx.params.tableId === InternalTables.USER_METADATA) { + if (tableId === InternalTables.USER_METADATA) { ctx.params = { id: _id, } @@ -176,7 +179,7 @@ export async function destroy(ctx: UserCtx) { } export async function bulkDestroy(ctx: UserCtx) { - const tableId = ctx.params.tableId + const tableId = utils.getTableId(ctx) const table = await sdk.tables.getTable(tableId) let { rows } = ctx.request.body @@ -216,7 +219,7 @@ export async function bulkDestroy(ctx: UserCtx) { export async function fetchEnrichedRow(ctx: UserCtx) { const db = context.getAppDB() - const tableId = ctx.params.tableId + const tableId = utils.getTableId(ctx) const rowId = ctx.params.rowId // need table to work out where links go in row let [table, row] = await Promise.all([ diff --git a/packages/server/src/api/controllers/row/utils.ts b/packages/server/src/api/controllers/row/utils.ts index 6cc342b8a0..157f18e231 100644 --- a/packages/server/src/api/controllers/row/utils.ts +++ b/packages/server/src/api/controllers/row/utils.ts @@ -45,15 +45,19 @@ export async function findRow(ctx: UserCtx, tableId: string, rowId: string) { } export function getTableId(ctx: Ctx) { - if (ctx.request.body?.tableId) { - return ctx.request.body.tableId - } + // top priority, use the URL first if (ctx.params?.sourceId) { return ctx.params.sourceId } + // check body for a table ID + if (ctx.request.body?.tableId) { + return ctx.request.body.tableId + } + // now check for old way of specifying table ID if (ctx.params?.tableId) { return ctx.params.tableId } + // now check if a specific view name if (ctx.params?.viewName) { return ctx.params.viewName } diff --git a/packages/server/src/api/routes/tests/row.spec.ts b/packages/server/src/api/routes/tests/row.spec.ts index 5e1616340f..8d4c9a91fd 100644 --- a/packages/server/src/api/routes/tests/row.spec.ts +++ b/packages/server/src/api/routes/tests/row.spec.ts @@ -16,15 +16,12 @@ import { FieldType, SortType, SortOrder, - DeleteRow, } from "@budibase/types" import { expectAnyInternalColsAttributes, generator, structures, } from "@budibase/backend-core/tests" -import trimViewRowInfoMiddleware from "../../../middleware/trimViewRowInfo" -import router from "../row" describe("/rows", () => { let request = setup.getRequest() @@ -393,18 +390,6 @@ describe("/rows", () => { expect(saved.arrayFieldArrayStrKnown).toEqual(["One"]) expect(saved.optsFieldStrKnown).toEqual("Alpha") }) - - 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() }) - .set(config.defaultHeaders()) - .expect("Content-Type", /json/) - .expect(400) - expect(res.body.message).toEqual( - "Table row endpoints cannot contain view info" - ) - }) }) describe("patch", () => { @@ -454,25 +439,6 @@ 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", () => { @@ -741,7 +707,7 @@ describe("/rows", () => { }) // the environment needs configured for this await setup.switchToSelfHosted(async () => { - context.doInAppContext(config.getAppId(), async () => { + return context.doInAppContext(config.getAppId(), async () => { const enriched = await outputProcessing(table, [row]) expect((enriched as Row[])[0].attachment[0].url).toBe( `/files/signed/prod-budi-app-assets/${config.getProdAppId()}/attachments/${attachmentId}` @@ -847,7 +813,7 @@ describe("/rows", () => { }) const data = randomRowData() - const newRow = await config.api.viewV2.row.create(view.id, { + const newRow = await config.api.row.save(view.id, { tableId: config.table!._id, _viewId: view.id, ...data, @@ -869,16 +835,6 @@ 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) - }) }) describe("patch", () => { @@ -893,13 +849,13 @@ describe("/rows", () => { }, }) - const newRow = await config.api.viewV2.row.create(view.id, { + const newRow = await config.api.row.save(view.id, { tableId, _viewId: view.id, ...randomRowData(), }) const newData = randomRowData() - await config.api.viewV2.row.update(view.id, newRow._id!, { + await config.api.row.patch(view.id, { tableId, _viewId: view.id, _id: newRow._id!, @@ -922,16 +878,6 @@ 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("PATCH") && - r.path === "/api/v2/views/:viewId/rows/:rowId" - ) - expect(route).toBeDefined() - expect(route?.stack).toContainEqual(trimViewRowInfoMiddleware) - }) }) describe("destroy", () => { @@ -950,10 +896,7 @@ describe("/rows", () => { const rowUsage = await getRowUsage() const queryUsage = await getQueryUsage() - const body: DeleteRow = { - _id: createdRow._id!, - } - await config.api.viewV2.row.delete(view.id, body) + await config.api.row.delete(view.id, [createdRow]) await assertRowUsage(rowUsage - 1) await assertQueryUsage(queryUsage + 1) @@ -982,9 +925,7 @@ describe("/rows", () => { const rowUsage = await getRowUsage() const queryUsage = await getQueryUsage() - await config.api.viewV2.row.delete(view.id, { - rows: [rows[0], rows[2]], - }) + await config.api.row.delete(view.id, [rows[0], rows[2]]) await assertRowUsage(rowUsage - 2) await assertQueryUsage(queryUsage + 1) diff --git a/packages/server/src/middleware/trimViewRowInfo.ts b/packages/server/src/middleware/trimViewRowInfo.ts index 763552c3d7..5a207936b2 100644 --- a/packages/server/src/middleware/trimViewRowInfo.ts +++ b/packages/server/src/middleware/trimViewRowInfo.ts @@ -3,26 +3,35 @@ import * as utils from "../db/utils" import sdk from "../sdk" import { db } from "@budibase/backend-core" import { Next } from "koa" +import { getTableId } from "../api/controllers/row/utils" export default async (ctx: Ctx, next: Next) => { const { body } = ctx.request - const { _viewId: viewId } = body + let { _viewId: viewId } = body - const possibleViewId = ctx.params.tableId + const possibleViewId = getTableId(ctx) + if (utils.isViewID(possibleViewId)) { + viewId = possibleViewId + } // nothing to do, it is not a view (just a table ID) - if (!viewId || !utils.isViewID(possibleViewId)) { + if (!viewId) { return next() } - const { tableId } = utils.extractViewInfoFromID(possibleViewId) - const { _viewId, ...trimmedView } = await trimViewFields( - viewId, - tableId, - body - ) - ctx.request.body = trimmedView - ctx.params.tableId = tableId + const { tableId } = utils.extractViewInfoFromID(viewId) + + // don't need to trim delete requests + if (ctx.method.toLowerCase() !== "delete") { + const { _viewId, ...trimmedView } = await trimViewFields( + viewId, + tableId, + body + ) + ctx.request.body = trimmedView + } + + ctx.params.sourceId = tableId return next() } diff --git a/packages/server/src/tests/utilities/api/row.ts b/packages/server/src/tests/utilities/api/row.ts index c7c72368f5..c6ef4606d2 100644 --- a/packages/server/src/tests/utilities/api/row.ts +++ b/packages/server/src/tests/utilities/api/row.ts @@ -1,4 +1,4 @@ -import { PatchRowRequest } from "@budibase/types" +import { PatchRowRequest, SaveRowRequest, Row } from "@budibase/types" import TestConfiguration from "../TestConfiguration" import { TestAPI } from "./base" @@ -8,12 +8,12 @@ export class RowAPI extends TestAPI { } get = async ( - tableId: string, + sourceId: string, rowId: string, { expectStatus } = { expectStatus: 200 } ) => { const request = this.request - .get(`/api/${tableId}/rows/${rowId}`) + .get(`/api/${sourceId}/rows/${rowId}`) .set(this.config.defaultHeaders()) .expect(expectStatus) if (expectStatus !== 404) { @@ -22,16 +22,43 @@ export class RowAPI extends TestAPI { return request } + save = async ( + sourceId: string, + row: SaveRowRequest, + { expectStatus } = { expectStatus: 200 } + ): Promise => { + const resp = await this.request + .post(`/api/${sourceId}/rows`) + .send(row) + .set(this.config.defaultHeaders()) + .expect("Content-Type", /json/) + .expect(expectStatus) + return resp.body as Row + } + patch = async ( - tableId: string, + sourceId: string, row: PatchRowRequest, { expectStatus } = { expectStatus: 200 } ) => { return this.request - .patch(`/api/${tableId}/rows`) + .patch(`/api/${sourceId}/rows`) .send(row) .set(this.config.defaultHeaders()) .expect("Content-Type", /json/) .expect(expectStatus) } + + delete = async ( + sourceId: string, + rows: Row[], + { expectStatus } = { expectStatus: 200 } + ) => { + return this.request + .delete(`/api/${sourceId}/rows`) + .send({ rows }) + .set(this.config.defaultHeaders()) + .expect("Content-Type", /json/) + .expect(expectStatus) + } } diff --git a/packages/server/src/tests/utilities/api/viewV2.ts b/packages/server/src/tests/utilities/api/viewV2.ts index 813d2ebfd1..1520154641 100644 --- a/packages/server/src/tests/utilities/api/viewV2.ts +++ b/packages/server/src/tests/utilities/api/viewV2.ts @@ -1,10 +1,6 @@ import { CreateViewRequest, UpdateViewRequest, - DeleteRowRequest, - PatchRowRequest, - PatchRowResponse, - Row, ViewV2, SearchViewRowRequest, } from "@budibase/types" @@ -90,46 +86,4 @@ 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 - }, - 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 - }, - 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 - }, - } } diff --git a/packages/types/src/api/web/app/rows.ts b/packages/types/src/api/web/app/rows.ts index f1890ef777..2b51c7b203 100644 --- a/packages/types/src/api/web/app/rows.ts +++ b/packages/types/src/api/web/app/rows.ts @@ -1,6 +1,8 @@ import { SearchParams } from "../../../sdk" import { Row } from "../../../documents" +export interface SaveRowRequest extends Row {} + export interface PatchRowRequest extends Row { _id: string _rev: string From c18459d84d7c8979e5402226b6b189033dd386fc Mon Sep 17 00:00:00 2001 From: mike12345567 Date: Tue, 8 Aug 2023 13:43:13 +0100 Subject: [PATCH 6/7] Updating trim view info test case. --- .../middleware/tests/trimViewRowInfo.spec.ts | 27 ++----------------- .../server/src/middleware/trimViewRowInfo.ts | 2 +- 2 files changed, 3 insertions(+), 26 deletions(-) diff --git a/packages/server/src/middleware/tests/trimViewRowInfo.spec.ts b/packages/server/src/middleware/tests/trimViewRowInfo.spec.ts index 427ac9a608..69d1272df9 100644 --- a/packages/server/src/middleware/tests/trimViewRowInfo.spec.ts +++ b/packages/server/src/middleware/tests/trimViewRowInfo.spec.ts @@ -117,7 +117,7 @@ describe("trimViewRowInfo middleware", () => { }) expect(config.request?.body).toEqual(data) - expect(config.params.tableId).toEqual(table._id) + expect(config.params.sourceId).toEqual(table._id) expect(config.next).toBeCalledTimes(1) expect(config.throw).not.toBeCalled() @@ -143,32 +143,9 @@ describe("trimViewRowInfo middleware", () => { name: data.name, address: data.address, }) - expect(config.params.tableId).toEqual(table._id) + expect(config.params.sourceId).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") - expect(config.next).not.toBeCalled() - }) - - 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") - expect(config.next).not.toBeCalled() - }) }) diff --git a/packages/server/src/middleware/trimViewRowInfo.ts b/packages/server/src/middleware/trimViewRowInfo.ts index 5a207936b2..cff9dabd37 100644 --- a/packages/server/src/middleware/trimViewRowInfo.ts +++ b/packages/server/src/middleware/trimViewRowInfo.ts @@ -22,7 +22,7 @@ export default async (ctx: Ctx, next: Next) => { const { tableId } = utils.extractViewInfoFromID(viewId) // don't need to trim delete requests - if (ctx.method.toLowerCase() !== "delete") { + if (ctx?.method?.toLowerCase() !== "delete") { const { _viewId, ...trimmedView } = await trimViewFields( viewId, tableId, From d6da826c9fdbdc4c65f86e54406fe622dfd634dc Mon Sep 17 00:00:00 2001 From: mike12345567 Date: Thu, 10 Aug 2023 11:07:25 +0100 Subject: [PATCH 7/7] Further PR comments. --- packages/server/src/api/controllers/row/utils.ts | 8 ++++---- packages/types/src/documents/document.ts | 1 - 2 files changed, 4 insertions(+), 5 deletions(-) diff --git a/packages/server/src/api/controllers/row/utils.ts b/packages/server/src/api/controllers/row/utils.ts index 157f18e231..e85ec4553c 100644 --- a/packages/server/src/api/controllers/row/utils.ts +++ b/packages/server/src/api/controllers/row/utils.ts @@ -49,14 +49,14 @@ export function getTableId(ctx: Ctx) { if (ctx.params?.sourceId) { return ctx.params.sourceId } - // check body for a table ID - if (ctx.request.body?.tableId) { - return ctx.request.body.tableId - } // now check for old way of specifying table ID if (ctx.params?.tableId) { return ctx.params.tableId } + // check body for a table ID + if (ctx.request.body?.tableId) { + return ctx.request.body.tableId + } // now check if a specific view name if (ctx.params?.viewName) { return ctx.params.viewName diff --git a/packages/types/src/documents/document.ts b/packages/types/src/documents/document.ts index 75f55e1367..03e01907b8 100644 --- a/packages/types/src/documents/document.ts +++ b/packages/types/src/documents/document.ts @@ -37,7 +37,6 @@ export enum DocumentType { USER_FLAG = "flag", AUTOMATION_METADATA = "meta_au", AUDIT_LOG = "al", - VIEW = "awd", } // these documents don't really exist, they are part of other