From ed256242c8f1df3a042f8ab37f301cda58d0a62d Mon Sep 17 00:00:00 2001 From: Adria Navarro Date: Mon, 31 Jul 2023 09:54:20 +0200 Subject: [PATCH 01/35] 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/35] 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/35] 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/35] 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/35] 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/35] 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/35] 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/35] 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/35] 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/35] 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/35] 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/35] 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/35] 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/35] 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/35] 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/35] 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/35] 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/35] 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/35] 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 }) + }) }) }) }) From 878e09cfb0953b9fb173ad394516ec599bbddac7 Mon Sep 17 00:00:00 2001 From: Adria Navarro Date: Mon, 31 Jul 2023 17:04:32 +0200 Subject: [PATCH 20/35] Change record --- packages/types/src/documents/app/view.ts | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/packages/types/src/documents/app/view.ts b/packages/types/src/documents/app/view.ts index aeef95ed5b..10fcac2805 100644 --- a/packages/types/src/documents/app/view.ts +++ b/packages/types/src/documents/app/view.ts @@ -25,7 +25,8 @@ export interface ViewV2 { order?: SortOrder type?: SortType } - columns?: Record + columns?: string[] + schemaUI?: Record } export type ViewSchema = ViewCountOrSumSchema | ViewStatisticsSchema From da6136a1085c7e1e2d70e5b1d3c9b1ff5954a962 Mon Sep 17 00:00:00 2001 From: Adria Navarro Date: Mon, 31 Jul 2023 17:15:51 +0200 Subject: [PATCH 21/35] Enrich schema using the new data --- packages/server/src/sdk/app/views/index.ts | 65 ++++++++++--------- .../src/sdk/app/views/tests/views.spec.ts | 46 +++++++++---- 2 files changed, 66 insertions(+), 45 deletions(-) diff --git a/packages/server/src/sdk/app/views/index.ts b/packages/server/src/sdk/app/views/index.ts index 244ac899c1..2cda79b470 100644 --- a/packages/server/src/sdk/app/views/index.ts +++ b/packages/server/src/sdk/app/views/index.ts @@ -1,5 +1,11 @@ import { HTTPError, context } from "@budibase/backend-core" -import { TableSchema, UIFieldMetadata, View, ViewV2 } from "@budibase/types" +import { + FieldSchema, + TableSchema, + UIFieldMetadata, + View, + ViewV2, +} from "@budibase/types" import sdk from "../../../sdk" import * as utils from "../../../db/utils" @@ -73,37 +79,34 @@ export function enrichSchema(view: View | ViewV2, tableSchema: TableSchema) { return view } + let schema = { ...tableSchema } + if (view.schemaUI) { + const viewOverridesEntries = Object.entries(view.schemaUI) + const viewSetsOrder = viewOverridesEntries.some(([_, v]) => v.order) + for (const [fieldName, schemaUI] of viewOverridesEntries) { + schema[fieldName] = { + ...schema[fieldName], + ...schemaUI, + order: viewSetsOrder + ? schemaUI.order || undefined + : schema[fieldName].order, + } + } + } + + if (view?.columns?.length) { + const pickedSchema: Record = {} + for (const fieldName of view.columns) { + if (!schema[fieldName]) { + continue + } + pickedSchema[fieldName] = { ...schema[fieldName] } + } + schema = pickedSchema + } + return { ...view, - schema: - !view?.columns || !Object.entries(view?.columns).length - ? tableSchema - : enrichViewV2Schema(tableSchema, view.columns), + schema: schema, } } - -function enrichViewV2Schema( - tableSchema: TableSchema, - viewOverrides: Record -) { - const result: TableSchema = {} - const viewOverridesEntries = Object.entries(viewOverrides) - const viewSetsOrder = viewOverridesEntries.some(([_, v]) => v.order) - for (const [columnName, columnUIMetadata] of viewOverridesEntries) { - if (!columnUIMetadata.visible) { - continue - } - - if (!tableSchema[columnName]) { - continue - } - - const tableFieldSchema = tableSchema[columnName] - if (viewSetsOrder) { - delete tableFieldSchema.order - } - - result[columnName] = merge(tableFieldSchema, columnUIMetadata) - } - return result -} diff --git a/packages/server/src/sdk/app/views/tests/views.spec.ts b/packages/server/src/sdk/app/views/tests/views.spec.ts index cbcd98eb91..3b1cb84a42 100644 --- a/packages/server/src/sdk/app/views/tests/views.spec.ts +++ b/packages/server/src/sdk/app/views/tests/views.spec.ts @@ -102,18 +102,14 @@ describe("table sdk", () => { }) }) - it("if view schema only defines visiblility, should only fetch the selected fields", async () => { + it("if view schema only defines columns, should only fetch the selected fields", async () => { const tableId = basicTable._id! const view: ViewV2 = { version: 2, id: generator.guid(), name: generator.guid(), tableId, - columns: { - name: { visible: true }, - id: { visible: true }, - description: { visible: false }, - }, + columns: ["name", "id"], } const res = enrichSchema(view, basicTable.schema) @@ -151,7 +147,7 @@ describe("table sdk", () => { id: generator.guid(), name: generator.guid(), tableId, - columns: { unnexisting: { visible: true }, name: { visible: true } }, + columns: ["unnexisting", "name"], } const res = enrichSchema(view, basicTable.schema) @@ -175,16 +171,17 @@ describe("table sdk", () => { ) }) - it("if view schema only defines visiblility, should only fetch the selected fields", async () => { + it("if the view schema overrides the schema UI, the table schema should be overridden", async () => { const tableId = basicTable._id! const view: ViewV2 = { version: 2, id: generator.guid(), name: generator.guid(), tableId, - columns: { - name: { visible: true }, - id: { visible: true }, + columns: ["name", "id", "description"], + schemaUI: { + name: { visible: true, width: 100 }, + id: { visible: true, width: 20 }, description: { visible: false }, }, } @@ -200,7 +197,7 @@ describe("table sdk", () => { name: "name", order: 2, visible: true, - width: 80, + width: 100, constraints: { type: "string", }, @@ -210,23 +207,34 @@ describe("table sdk", () => { name: "id", order: 1, visible: true, + width: 20, constraints: { type: "number", }, }, + description: { + type: "string", + name: "description", + visible: false, + width: 200, + constraints: { + type: "string", + }, + }, }, }) ) }) - it("if view defines order, the table schema order should be ignored", async () => { + it("if the view defines order, the table schema order should be ignored", async () => { const tableId = basicTable._id! const view: ViewV2 = { version: 2, id: generator.guid(), name: generator.guid(), tableId, - columns: { + columns: ["name", "id", "description"], + schemaUI: { name: { visible: true, order: 1 }, id: { visible: true }, description: { visible: false, order: 2 }, @@ -257,6 +265,16 @@ describe("table sdk", () => { type: "number", }, }, + description: { + type: "string", + name: "description", + order: 2, + visible: false, + width: 200, + constraints: { + type: "string", + }, + }, }, }) ) From 1b2ce184d9af1c253273c8d52ee2a3a3dc9c8b8d Mon Sep 17 00:00:00 2001 From: Adria Navarro Date: Mon, 31 Jul 2023 17:22:10 +0200 Subject: [PATCH 22/35] Fix types --- packages/server/src/api/routes/tests/viewV2.spec.ts | 3 ++- packages/server/src/sdk/app/views/index.ts | 8 +------- 2 files changed, 3 insertions(+), 8 deletions(-) diff --git a/packages/server/src/api/routes/tests/viewV2.spec.ts b/packages/server/src/api/routes/tests/viewV2.spec.ts index e728af3e40..2660864948 100644 --- a/packages/server/src/api/routes/tests/viewV2.spec.ts +++ b/packages/server/src/api/routes/tests/viewV2.spec.ts @@ -40,7 +40,8 @@ describe("/v2/views", () => { order: SortOrder.DESCENDING, type: SortType.STRING, }, - columns: { + columns: ["name"], + schemaUI: { name: { visible: true, }, diff --git a/packages/server/src/sdk/app/views/index.ts b/packages/server/src/sdk/app/views/index.ts index 2cda79b470..3dfa82df0d 100644 --- a/packages/server/src/sdk/app/views/index.ts +++ b/packages/server/src/sdk/app/views/index.ts @@ -1,11 +1,5 @@ import { HTTPError, context } from "@budibase/backend-core" -import { - FieldSchema, - TableSchema, - UIFieldMetadata, - View, - ViewV2, -} from "@budibase/types" +import { FieldSchema, TableSchema, View, ViewV2 } from "@budibase/types" import sdk from "../../../sdk" import * as utils from "../../../db/utils" From 73ded07484b44a3bbd0bf7d3ffb1b91c060c3f12 Mon Sep 17 00:00:00 2001 From: Adria Navarro Date: Mon, 31 Jul 2023 17:23:34 +0200 Subject: [PATCH 23/35] Fix tests --- packages/server/src/api/routes/tests/row.spec.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/server/src/api/routes/tests/row.spec.ts b/packages/server/src/api/routes/tests/row.spec.ts index dbc417a5b5..baec8ddb85 100644 --- a/packages/server/src/api/routes/tests/row.spec.ts +++ b/packages/server/src/api/routes/tests/row.spec.ts @@ -983,7 +983,7 @@ describe("/rows", () => { } const view = await config.api.viewV2.create({ - columns: { name: { visible: true } }, + columns: ["name"], }) const response = await config.api.viewV2.search(view.id) From ebb32f8d966a6d602f572abeca53dd3335ec26ab Mon Sep 17 00:00:00 2001 From: Jonny McCullagh Date: Tue, 1 Aug 2023 09:01:04 +0100 Subject: [PATCH 24/35] add startupProbe;Probes into values.yaml (#11404) --- .../templates/app-service-deployment.yaml | 33 +++--- .../templates/proxy-service-deployment.yaml | 32 +++--- .../templates/worker-service-deployment.yaml | 32 +++--- charts/budibase/values.yaml | 104 ++++++++++++++---- 4 files changed, 133 insertions(+), 68 deletions(-) diff --git a/charts/budibase/templates/app-service-deployment.yaml b/charts/budibase/templates/app-service-deployment.yaml index 2b2589406a..e47dc0bb58 100644 --- a/charts/budibase/templates/app-service-deployment.yaml +++ b/charts/budibase/templates/app-service-deployment.yaml @@ -201,25 +201,24 @@ spec: image: budibase/apps:{{ .Values.globals.appVersion | default .Chart.AppVersion }} imagePullPolicy: Always + {{- if .Values.services.apps.startupProbe }} + {{- with .Values.services.apps.startupProbe }} + startupProbe: + {{- toYaml . | nindent 10 }} + {{- end }} + {{- end }} + {{- if .Values.services.apps.livenessProbe }} + {{- with .Values.services.apps.livenessProbe }} livenessProbe: - httpGet: - path: /health - port: {{ .Values.services.apps.port }} - initialDelaySeconds: 10 - periodSeconds: 5 - successThreshold: 1 - failureThreshold: 3 - timeoutSeconds: 3 + {{- toYaml . | nindent 10 }} + {{- end }} + {{- end }} + {{- if .Values.services.apps.readinessProbe }} + {{- with .Values.services.apps.readinessProbe }} readinessProbe: - httpGet: - path: /health - port: {{ .Values.services.apps.port }} - initialDelaySeconds: 5 - periodSeconds: 5 - successThreshold: 1 - failureThreshold: 3 - timeoutSeconds: 3 - + {{- toYaml . | nindent 10 }} + {{- end }} + {{- end }} name: bbapps ports: - containerPort: {{ .Values.services.apps.port }} diff --git a/charts/budibase/templates/proxy-service-deployment.yaml b/charts/budibase/templates/proxy-service-deployment.yaml index c087627100..53bba6232d 100644 --- a/charts/budibase/templates/proxy-service-deployment.yaml +++ b/charts/budibase/templates/proxy-service-deployment.yaml @@ -40,24 +40,24 @@ spec: - image: budibase/proxy:{{ .Values.globals.appVersion | default .Chart.AppVersion }} imagePullPolicy: Always name: proxy-service + {{- if .Values.services.proxy.startupProbe }} + {{- with .Values.services.proxy.startupProbe }} + startupProbe: + {{- toYaml . | nindent 10 }} + {{- end }} + {{- end }} + {{- if .Values.services.proxy.livenessProbe }} + {{- with .Values.services.proxy.livenessProbe }} livenessProbe: - httpGet: - path: /health - port: {{ .Values.services.proxy.port }} - initialDelaySeconds: 0 - periodSeconds: 5 - successThreshold: 1 - failureThreshold: 2 - timeoutSeconds: 3 + {{- toYaml . | nindent 10 }} + {{- end }} + {{- end }} + {{- if .Values.services.proxy.readinessProbe }} + {{- with .Values.services.proxy.readinessProbe }} readinessProbe: - httpGet: - path: /health - port: {{ .Values.services.proxy.port }} - initialDelaySeconds: 0 - periodSeconds: 5 - successThreshold: 1 - failureThreshold: 2 - timeoutSeconds: 3 + {{- toYaml . | nindent 10 }} + {{- end }} + {{- end }} ports: - containerPort: {{ .Values.services.proxy.port }} env: diff --git a/charts/budibase/templates/worker-service-deployment.yaml b/charts/budibase/templates/worker-service-deployment.yaml index 5fed80b355..124c667807 100644 --- a/charts/budibase/templates/worker-service-deployment.yaml +++ b/charts/budibase/templates/worker-service-deployment.yaml @@ -190,24 +190,24 @@ spec: {{ end }} image: budibase/worker:{{ .Values.globals.appVersion | default .Chart.AppVersion }} imagePullPolicy: Always + {{- if .Values.services.worker.startupProbe }} + {{- with .Values.services.worker.startupProbe }} + startupProbe: + {{- toYaml . | nindent 10 }} + {{- end }} + {{- end }} + {{- if .Values.services.worker.livenessProbe }} + {{- with .Values.services.worker.livenessProbe }} livenessProbe: - httpGet: - path: /health - port: {{ .Values.services.worker.port }} - initialDelaySeconds: 10 - periodSeconds: 5 - successThreshold: 1 - failureThreshold: 3 - timeoutSeconds: 3 + {{- toYaml . | nindent 10 }} + {{- end }} + {{- end }} + {{- if .Values.services.worker.readinessProbe }} + {{- with .Values.services.worker.readinessProbe }} readinessProbe: - httpGet: - path: /health - port: {{ .Values.services.worker.port }} - initialDelaySeconds: 5 - periodSeconds: 5 - successThreshold: 1 - failureThreshold: 3 - timeoutSeconds: 3 + {{- toYaml . | nindent 10 }} + {{- end }} + {{- end }} name: bbworker ports: - containerPort: {{ .Values.services.worker.port }} diff --git a/charts/budibase/values.yaml b/charts/budibase/values.yaml index 74e4c52654..12e21a8e9c 100644 --- a/charts/budibase/values.yaml +++ b/charts/budibase/values.yaml @@ -119,15 +119,37 @@ services: port: 10000 replicaCount: 1 upstreams: - apps: 'http://app-service.{{ .Release.Namespace }}.svc.{{ .Values.services.dns }}:{{ .Values.services.apps.port }}' - worker: 'http://worker-service.{{ .Release.Namespace }}.svc.{{ .Values.services.dns }}:{{ .Values.services.worker.port }}' - minio: 'http://minio-service.{{ .Release.Namespace }}.svc.{{ .Values.services.dns }}:{{ .Values.services.objectStore.port }}' - couchdb: 'http://{{ .Release.Name }}-svc-couchdb:{{ .Values.services.couchdb.port }}' + apps: "http://app-service.{{ .Release.Namespace }}.svc.{{ .Values.services.dns }}:{{ .Values.services.apps.port }}" + worker: "http://worker-service.{{ .Release.Namespace }}.svc.{{ .Values.services.dns }}:{{ .Values.services.worker.port }}" + minio: "http://minio-service.{{ .Release.Namespace }}.svc.{{ .Values.services.dns }}:{{ .Values.services.objectStore.port }}" + couchdb: "http://{{ .Release.Name }}-svc-couchdb:{{ .Values.services.couchdb.port }}" resources: {} -# annotations: -# co.elastic.logs/module: nginx -# co.elastic.logs/fileset.stdout: access -# co.elastic.logs/fileset.stderr: error + startupProbe: + httpGet: + path: /health + port: 10000 + scheme: HTTP + failureThreshold: 30 + periodSeconds: 3 + readinessProbe: + httpGet: + path: /health + port: 10000 + scheme: HTTP + enabled: true + periodSeconds: 3 + failureThreshold: 1 + livenessProbe: + httpGet: + path: /health + port: 10000 + scheme: HTTP + failureThreshold: 3 + periodSeconds: 5 + # annotations: + # co.elastic.logs/module: nginx + # co.elastic.logs/fileset.stdout: access + # co.elastic.logs/fileset.stderr: error apps: port: 4002 @@ -135,23 +157,67 @@ services: logLevel: info httpLogging: 1 resources: {} -# nodeDebug: "" # set the value of NODE_DEBUG -# annotations: -# co.elastic.logs/multiline.type: pattern -# co.elastic.logs/multiline.pattern: '^[[:space:]]' -# co.elastic.logs/multiline.negate: false -# co.elastic.logs/multiline.match: after + startupProbe: + httpGet: + path: /health + port: 4002 + scheme: HTTP + failureThreshold: 30 + periodSeconds: 3 + readinessProbe: + httpGet: + path: /health + port: 4002 + scheme: HTTP + enabled: true + periodSeconds: 3 + failureThreshold: 1 + livenessProbe: + httpGet: + path: /health + port: 4002 + scheme: HTTP + failureThreshold: 3 + periodSeconds: 5 + # nodeDebug: "" # set the value of NODE_DEBUG + # annotations: + # co.elastic.logs/multiline.type: pattern + # co.elastic.logs/multiline.pattern: '^[[:space:]]' + # co.elastic.logs/multiline.negate: false + # co.elastic.logs/multiline.match: after worker: port: 4003 replicaCount: 1 logLevel: info httpLogging: 1 resources: {} -# annotations: -# co.elastic.logs/multiline.type: pattern -# co.elastic.logs/multiline.pattern: '^[[:space:]]' -# co.elastic.logs/multiline.negate: false -# co.elastic.logs/multiline.match: after + startupProbe: + httpGet: + path: /health + port: 4003 + scheme: HTTP + failureThreshold: 30 + periodSeconds: 3 + readinessProbe: + httpGet: + path: /health + port: 4003 + scheme: HTTP + enabled: true + periodSeconds: 3 + failureThreshold: 1 + livenessProbe: + httpGet: + path: /health + port: 4003 + scheme: HTTP + failureThreshold: 3 + periodSeconds: 5 + # annotations: + # co.elastic.logs/multiline.type: pattern + # co.elastic.logs/multiline.pattern: '^[[:space:]]' + # co.elastic.logs/multiline.negate: false + # co.elastic.logs/multiline.match: after couchdb: enabled: true From f0b4f7db26d42222dcdb63a20abfb8d260a70f01 Mon Sep 17 00:00:00 2001 From: Budibase Staging Release Bot <> Date: Tue, 1 Aug 2023 08:01:23 +0000 Subject: [PATCH 25/35] Bump version to 2.8.29-alpha.7 --- lerna.json | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lerna.json b/lerna.json index f41e29c815..457fef141c 100644 --- a/lerna.json +++ b/lerna.json @@ -1,5 +1,5 @@ { - "version": "2.8.29-alpha.6", + "version": "2.8.29-alpha.7", "npmClient": "yarn", "packages": [ "packages/*" From b528257bbee0a0981c65f0b88560a5028853232b Mon Sep 17 00:00:00 2001 From: Adria Navarro Date: Tue, 1 Aug 2023 10:45:00 +0200 Subject: [PATCH 26/35] Change viewrequest to accept schema --- .../src/api/controllers/view/viewsV2.ts | 11 +++- .../src/api/routes/tests/viewV2.spec.ts | 53 +++++++++++++++++-- .../server/src/tests/utilities/api/viewV2.ts | 7 +++ packages/types/src/api/web/app/view.ts | 7 ++- 4 files changed, 71 insertions(+), 7 deletions(-) diff --git a/packages/server/src/api/controllers/view/viewsV2.ts b/packages/server/src/api/controllers/view/viewsV2.ts index 9a4949bb43..b2a9e2f223 100644 --- a/packages/server/src/api/controllers/view/viewsV2.ts +++ b/packages/server/src/api/controllers/view/viewsV2.ts @@ -4,13 +4,22 @@ import { Ctx, UpdateViewRequest, ViewResponse, + ViewV2, } from "@budibase/types" export async function create(ctx: Ctx) { const view = ctx.request.body const { tableId } = view - const result = await sdk.views.create(tableId, view) + const parsedView: Omit = { + name: view.name, + tableId: view.tableId, + query: view.query, + sort: view.sort, + columns: view.schema && Object.keys(view.schema), + schemaUI: view.schema, + } + const result = await sdk.views.create(tableId, parsedView) ctx.status = 201 ctx.body = { data: result, diff --git a/packages/server/src/api/routes/tests/viewV2.spec.ts b/packages/server/src/api/routes/tests/viewV2.spec.ts index 2660864948..e2b81f4cf0 100644 --- a/packages/server/src/api/routes/tests/viewV2.spec.ts +++ b/packages/server/src/api/routes/tests/viewV2.spec.ts @@ -40,9 +40,10 @@ describe("/v2/views", () => { order: SortOrder.DESCENDING, type: SortType.STRING, }, - columns: ["name"], - schemaUI: { + schema: { name: { + name: "name", + type: FieldType.STRING, visible: true, }, }, @@ -74,17 +75,61 @@ describe("/v2/views", () => { const newView: CreateViewRequest = { name: generator.name(), tableId: config.table!._id!, - ...viewFilters, + query: viewFilters.query, + sort: viewFilters.sort, } + delete newView.schema const res = await config.api.viewV2.create(newView) expect(res).toEqual({ ...newView, - ...viewFilters, + query: viewFilters.query, + sort: viewFilters.sort, id: expect.any(String), version: 2, }) }) + + it("persist schema overrides", async () => { + const newView: CreateViewRequest = { + name: generator.name(), + tableId: config.table!._id!, + schema: { + name: { + name: "name", + type: FieldType.STRING, + visible: true, + }, + lastname: { + name: "lastname", + type: FieldType.STRING, + visible: false, + }, + }, + } + + const createdView = await config.api.viewV2.create(newView) + + expect(await config.api.viewV2.get(createdView.id)).toEqual({ + ...newView, + schema: undefined, + columns: ["name", "lastname"], + schemaUI: { + name: { + name: "name", + type: FieldType.STRING, + visible: true, + }, + lastname: { + name: "lastname", + type: FieldType.STRING, + visible: false, + }, + }, + id: createdView.id, + version: 2, + }) + }) }) describe("update", () => { diff --git a/packages/server/src/tests/utilities/api/viewV2.ts b/packages/server/src/tests/utilities/api/viewV2.ts index fae0850f79..1f7fe6e2bb 100644 --- a/packages/server/src/tests/utilities/api/viewV2.ts +++ b/packages/server/src/tests/utilities/api/viewV2.ts @@ -3,6 +3,7 @@ import TestConfiguration from "../TestConfiguration" import { TestAPI } from "./base" import { generator } from "@budibase/backend-core/tests" import { Response } from "superagent" +import sdk from "../../../sdk" export class ViewV2API extends TestAPI { constructor(config: TestConfiguration) { @@ -62,6 +63,12 @@ export class ViewV2API extends TestAPI { .expect(expectStatus) } + get = async (viewId: string) => { + return await this.config.doInContext(this.config.appId, () => + sdk.views.get(viewId) + ) + } + search = async ( viewId: string, options?: { diff --git a/packages/types/src/api/web/app/view.ts b/packages/types/src/api/web/app/view.ts index 6b516c0314..b694521678 100644 --- a/packages/types/src/api/web/app/view.ts +++ b/packages/types/src/api/web/app/view.ts @@ -1,9 +1,12 @@ -import { TableSchema, ViewV2 } from "../../../documents" +import { ViewV2, FieldSchema } from "../../../documents" export interface ViewResponse { data: ViewV2 } -export type CreateViewRequest = Omit +export interface CreateViewRequest + extends Omit { + schema?: Record +} export type UpdateViewRequest = ViewV2 From bce75c91a6a83d29c9c826c7992e1b99f9d400f1 Mon Sep 17 00:00:00 2001 From: Adria Navarro Date: Tue, 1 Aug 2023 10:57:03 +0200 Subject: [PATCH 27/35] Persist only UI schema overrides --- .../server/src/api/controllers/view/viewsV2.ts | 16 +++++++++++++++- .../server/src/api/routes/tests/viewV2.spec.ts | 12 +++++++----- packages/types/src/index.ts | 1 + packages/types/src/shared/typeUtils.ts | 4 ++++ 4 files changed, 27 insertions(+), 6 deletions(-) diff --git a/packages/server/src/api/controllers/view/viewsV2.ts b/packages/server/src/api/controllers/view/viewsV2.ts index b2a9e2f223..c872abba39 100644 --- a/packages/server/src/api/controllers/view/viewsV2.ts +++ b/packages/server/src/api/controllers/view/viewsV2.ts @@ -2,22 +2,36 @@ import sdk from "../../../sdk" import { CreateViewRequest, Ctx, + UIFieldMetadata, UpdateViewRequest, ViewResponse, ViewV2, + RequiredKeys, } from "@budibase/types" export async function create(ctx: Ctx) { const view = ctx.request.body const { tableId } = view + const schemaUI = + view.schema && + Object.entries(view.schema).reduce((p, [fieldName, schemaValue]) => { + p[fieldName] = { + order: schemaValue.order, + width: schemaValue.width, + visible: schemaValue.visible, + icon: schemaValue.icon, + } + return p + }, {} as Record>) + const parsedView: Omit = { name: view.name, tableId: view.tableId, query: view.query, sort: view.sort, columns: view.schema && Object.keys(view.schema), - schemaUI: view.schema, + schemaUI, } const result = await sdk.views.create(tableId, parsedView) ctx.status = 201 diff --git a/packages/server/src/api/routes/tests/viewV2.spec.ts b/packages/server/src/api/routes/tests/viewV2.spec.ts index e2b81f4cf0..14fe90b93e 100644 --- a/packages/server/src/api/routes/tests/viewV2.spec.ts +++ b/packages/server/src/api/routes/tests/viewV2.spec.ts @@ -90,7 +90,7 @@ describe("/v2/views", () => { }) }) - it("persist schema overrides", async () => { + it("persist only UI schema overrides", async () => { const newView: CreateViewRequest = { name: generator.name(), tableId: config.table!._id!, @@ -99,11 +99,14 @@ describe("/v2/views", () => { name: "name", type: FieldType.STRING, visible: true, + order: 1, + width: 100, }, lastname: { name: "lastname", type: FieldType.STRING, visible: false, + icon: "ic", }, }, } @@ -116,14 +119,13 @@ describe("/v2/views", () => { columns: ["name", "lastname"], schemaUI: { name: { - name: "name", - type: FieldType.STRING, visible: true, + order: 1, + width: 100, }, lastname: { - name: "lastname", - type: FieldType.STRING, visible: false, + icon: "ic", }, }, id: createdView.id, diff --git a/packages/types/src/index.ts b/packages/types/src/index.ts index 4adb2fda97..ad1688ef52 100644 --- a/packages/types/src/index.ts +++ b/packages/types/src/index.ts @@ -1,3 +1,4 @@ export * from "./documents" export * from "./sdk" export * from "./api" +export * from "./shared" diff --git a/packages/types/src/shared/typeUtils.ts b/packages/types/src/shared/typeUtils.ts index fbe215fdb9..df0e049455 100644 --- a/packages/types/src/shared/typeUtils.ts +++ b/packages/types/src/shared/typeUtils.ts @@ -3,3 +3,7 @@ export type DeepPartial = { } export type ISO8601 = string + +export type RequiredKeys = { + [K in keyof Required]: T[K] +} From d440291ebc5ddb9b6c439af44db3d6b38c69ef22 Mon Sep 17 00:00:00 2001 From: Adria Navarro Date: Tue, 1 Aug 2023 11:31:58 +0200 Subject: [PATCH 28/35] Throw exception when updating non ui fields --- .../src/api/controllers/view/viewsV2.ts | 45 ++++++++++++- .../src/api/routes/tests/viewV2.spec.ts | 64 ++++++++++++++++--- 2 files changed, 98 insertions(+), 11 deletions(-) diff --git a/packages/server/src/api/controllers/view/viewsV2.ts b/packages/server/src/api/controllers/view/viewsV2.ts index c872abba39..f403d14180 100644 --- a/packages/server/src/api/controllers/view/viewsV2.ts +++ b/packages/server/src/api/controllers/view/viewsV2.ts @@ -9,9 +9,40 @@ import { RequiredKeys, } from "@budibase/types" -export async function create(ctx: Ctx) { - const view = ctx.request.body - const { tableId } = view +async function parseSchemaUI(ctx: Ctx, view: CreateViewRequest) { + if (!view.schema) { + return + } + + function hasOverrides( + newObj: Record, + existingObj: Record + ) { + for (const [key, value] of Object.entries(newObj)) { + if (typeof value === "object") { + if (hasOverrides(value, existingObj[key] || {})) { + return true + } + } else if (value !== existingObj[key]) { + return true + } + } + return false + } + + const table = await sdk.tables.getTable(view.tableId) + for (const [ + fieldName, + { order, width, visible, icon, ...schemaNonUI }, + ] of Object.entries(view.schema)) { + const overrides = hasOverrides(schemaNonUI, table.schema[fieldName]) + if (overrides) { + ctx.throw( + 400, + "This endpoint does not support overriding non UI fields in the schema" + ) + } + } const schemaUI = view.schema && @@ -24,6 +55,14 @@ export async function create(ctx: Ctx) { } return p }, {} as Record>) + return schemaUI +} + +export async function create(ctx: Ctx) { + const view = ctx.request.body + const { tableId } = view + + const schemaUI = await parseSchemaUI(ctx, view) const parsedView: Omit = { name: view.name, diff --git a/packages/server/src/api/routes/tests/viewV2.spec.ts b/packages/server/src/api/routes/tests/viewV2.spec.ts index 14fe90b93e..e60e9a5126 100644 --- a/packages/server/src/api/routes/tests/viewV2.spec.ts +++ b/packages/server/src/api/routes/tests/viewV2.spec.ts @@ -95,15 +95,15 @@ describe("/v2/views", () => { name: generator.name(), tableId: config.table!._id!, schema: { - name: { - name: "name", - type: FieldType.STRING, + Price: { + name: "Price", + type: FieldType.NUMBER, visible: true, order: 1, width: 100, }, - lastname: { - name: "lastname", + Category: { + name: "Category", type: FieldType.STRING, visible: false, icon: "ic", @@ -116,14 +116,14 @@ describe("/v2/views", () => { expect(await config.api.viewV2.get(createdView.id)).toEqual({ ...newView, schema: undefined, - columns: ["name", "lastname"], + columns: ["Price", "Category"], schemaUI: { - name: { + Price: { visible: true, order: 1, width: 100, }, - lastname: { + Category: { visible: false, icon: "ic", }, @@ -132,6 +132,54 @@ describe("/v2/views", () => { version: 2, }) }) + + it("throw an exception if the schema overrides a non UI field", async () => { + const newView: CreateViewRequest = { + name: generator.name(), + tableId: config.table!._id!, + schema: { + Price: { + name: "Price", + type: FieldType.NUMBER, + visible: true, + }, + Category: { + name: "Category", + type: FieldType.STRING, + constraints: { + type: "string", + presence: true, + }, + }, + }, + } + + await config.api.viewV2.create(newView, { + expectStatus: 400, + }) + }) + + it("will not throw an exception if the schema is 'deleting' non UI fields", async () => { + const newView: CreateViewRequest = { + name: generator.name(), + tableId: config.table!._id!, + schema: { + Price: { + name: "Price", + type: FieldType.NUMBER, + visible: true, + }, + Category: { + name: "Category", + type: FieldType.STRING, + }, + }, + } + + await config.api.viewV2.create(newView, { + expectStatus: 201, + }) + }) }) describe("update", () => { From c425194a8561cfb853880f692ab10ae6a38935ac Mon Sep 17 00:00:00 2001 From: Adria Navarro Date: Tue, 1 Aug 2023 11:38:36 +0200 Subject: [PATCH 29/35] Check schemas on patch --- .../src/api/controllers/view/viewsV2.ts | 14 ++- .../src/api/routes/tests/viewV2.spec.ts | 88 +++++++++++++++++++ .../server/src/tests/utilities/api/viewV2.ts | 10 ++- packages/types/src/api/web/app/view.ts | 5 +- 4 files changed, 113 insertions(+), 4 deletions(-) diff --git a/packages/server/src/api/controllers/view/viewsV2.ts b/packages/server/src/api/controllers/view/viewsV2.ts index f403d14180..804de14ea8 100644 --- a/packages/server/src/api/controllers/view/viewsV2.ts +++ b/packages/server/src/api/controllers/view/viewsV2.ts @@ -92,7 +92,19 @@ export async function update(ctx: Ctx) { const { tableId } = view - const result = await sdk.views.update(tableId, view) + const schemaUI = await parseSchemaUI(ctx, view) + const parsedView: ViewV2 = { + id: view.id, + name: view.name, + version: view.version, + tableId: view.tableId, + query: view.query, + sort: view.sort, + columns: view.schema && Object.keys(view.schema), + schemaUI, + } + + const result = await sdk.views.update(tableId, parsedView) ctx.body = { data: result, } diff --git a/packages/server/src/api/routes/tests/viewV2.spec.ts b/packages/server/src/api/routes/tests/viewV2.spec.ts index e60e9a5126..b665b261e6 100644 --- a/packages/server/src/api/routes/tests/viewV2.spec.ts +++ b/packages/server/src/api/routes/tests/viewV2.spec.ts @@ -298,6 +298,94 @@ describe("/v2/views", () => { status: 400, }) }) + + it("updates only UI schema overrides", async () => { + await config.api.viewV2.update({ + ...view, + schema: { + Price: { + name: "Price", + type: FieldType.NUMBER, + visible: true, + order: 1, + width: 100, + }, + Category: { + name: "Category", + type: FieldType.STRING, + visible: false, + icon: "ic", + }, + }, + }) + + expect(await config.api.viewV2.get(view.id)).toEqual({ + ...view, + schema: undefined, + columns: ["Price", "Category"], + schemaUI: { + Price: { + visible: true, + order: 1, + width: 100, + }, + Category: { + visible: false, + icon: "ic", + }, + }, + id: view.id, + version: 2, + }) + }) + + it("throw an exception if the schema overrides a non UI field", async () => { + await config.api.viewV2.update( + { + ...view, + schema: { + Price: { + name: "Price", + type: FieldType.NUMBER, + visible: true, + }, + Category: { + name: "Category", + type: FieldType.STRING, + constraints: { + type: "string", + presence: true, + }, + }, + }, + }, + { + expectStatus: 400, + } + ) + }) + + it("will not throw an exception if the schema is 'deleting' non UI fields", async () => { + await config.api.viewV2.update( + { + ...view, + schema: { + Price: { + name: "Price", + type: FieldType.NUMBER, + visible: true, + }, + Category: { + name: "Category", + type: FieldType.STRING, + }, + }, + }, + { + expectStatus: 200, + } + ) + }) }) describe("delete", () => { diff --git a/packages/server/src/tests/utilities/api/viewV2.ts b/packages/server/src/tests/utilities/api/viewV2.ts index 1f7fe6e2bb..b404904a28 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, + SortOrder, + SortType, + UpdateViewRequest, + ViewV2, +} from "@budibase/types" import TestConfiguration from "../TestConfiguration" import { TestAPI } from "./base" import { generator } from "@budibase/backend-core/tests" @@ -34,7 +40,7 @@ export class ViewV2API extends TestAPI { } update = async ( - view: ViewV2, + view: UpdateViewRequest, { expectStatus, handleResponse, diff --git a/packages/types/src/api/web/app/view.ts b/packages/types/src/api/web/app/view.ts index b694521678..411bbef44e 100644 --- a/packages/types/src/api/web/app/view.ts +++ b/packages/types/src/api/web/app/view.ts @@ -9,4 +9,7 @@ export interface CreateViewRequest schema?: Record } -export type UpdateViewRequest = ViewV2 +export interface UpdateViewRequest + extends Omit { + schema?: Record +} From 0db0e2f74e6ba1a668bd3eefe4626403f7418b0c Mon Sep 17 00:00:00 2001 From: Budibase Staging Release Bot <> Date: Tue, 1 Aug 2023 09:52:05 +0000 Subject: [PATCH 30/35] Bump version to 2.8.29-alpha.8 --- lerna.json | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lerna.json b/lerna.json index 457fef141c..2295b6a975 100644 --- a/lerna.json +++ b/lerna.json @@ -1,5 +1,5 @@ { - "version": "2.8.29-alpha.7", + "version": "2.8.29-alpha.8", "npmClient": "yarn", "packages": [ "packages/*" From 76cacfaff074a487a658484a88df34b2ae362385 Mon Sep 17 00:00:00 2001 From: Adria Navarro Date: Tue, 1 Aug 2023 12:03:43 +0200 Subject: [PATCH 31/35] Clean code --- .../server/src/api/controllers/view/viewsV2.ts | 18 ++++++++++-------- 1 file changed, 10 insertions(+), 8 deletions(-) diff --git a/packages/server/src/api/controllers/view/viewsV2.ts b/packages/server/src/api/controllers/view/viewsV2.ts index 804de14ea8..cd28a1b55f 100644 --- a/packages/server/src/api/controllers/view/viewsV2.ts +++ b/packages/server/src/api/controllers/view/viewsV2.ts @@ -18,16 +18,18 @@ async function parseSchemaUI(ctx: Ctx, view: CreateViewRequest) { newObj: Record, existingObj: Record ) { - for (const [key, value] of Object.entries(newObj)) { - if (typeof value === "object") { - if (hasOverrides(value, existingObj[key] || {})) { - return true - } - } else if (value !== existingObj[key]) { + const result = Object.entries(newObj).some(([key, value]) => { + const isObject = typeof value === "object" + const existing = existingObj[key] + if (isObject && hasOverrides(value, existing || {})) { return true } - } - return false + if (!isObject && value !== existing) { + return true + } + }) + + return result } const table = await sdk.tables.getTable(view.tableId) From 2cc924f9e717f1aa26969c68459d6fe239546c3e Mon Sep 17 00:00:00 2001 From: Adria Navarro Date: Tue, 1 Aug 2023 12:08:40 +0200 Subject: [PATCH 32/35] Fix tests --- packages/server/src/api/routes/tests/row.spec.ts | 10 +++++----- .../server/src/api/routes/tests/viewV2.spec.ts | 15 +++++++-------- packages/types/src/api/web/app/view.ts | 6 +++--- 3 files changed, 15 insertions(+), 16 deletions(-) diff --git a/packages/server/src/api/routes/tests/row.spec.ts b/packages/server/src/api/routes/tests/row.spec.ts index 479f27d679..b986ffb945 100644 --- a/packages/server/src/api/routes/tests/row.spec.ts +++ b/packages/server/src/api/routes/tests/row.spec.ts @@ -1033,7 +1033,7 @@ describe("/rows", () => { } const view = await config.api.viewV2.create({ - columns: ["name"], + schema: { name: {} }, }) const response = await config.api.viewV2.search(view.id) @@ -1102,7 +1102,7 @@ describe("/rows", () => { const table = await config.createTable(userTable()) const view = await config.api.viewV2.create({ tableId: table._id!, - columns: { + schema: { name: { visible: true }, surname: { visible: true }, address: { visible: true }, @@ -1150,7 +1150,7 @@ describe("/rows", () => { const tableId = table._id! const view = await config.api.viewV2.create({ tableId, - columns: { + schema: { name: { visible: true }, address: { visible: true }, }, @@ -1203,7 +1203,7 @@ describe("/rows", () => { const tableId = table._id! const view = await config.api.viewV2.create({ tableId, - columns: { + schema: { name: { visible: true }, address: { visible: true }, }, @@ -1231,7 +1231,7 @@ describe("/rows", () => { const tableId = table._id! const view = await config.api.viewV2.create({ tableId, - columns: { + schema: { name: { visible: true }, address: { visible: true }, }, diff --git a/packages/server/src/api/routes/tests/viewV2.spec.ts b/packages/server/src/api/routes/tests/viewV2.spec.ts index b665b261e6..452d15bde0 100644 --- a/packages/server/src/api/routes/tests/viewV2.spec.ts +++ b/packages/server/src/api/routes/tests/viewV2.spec.ts @@ -1,6 +1,7 @@ import * as setup from "./utilities" import { CreateViewRequest, + FieldSchema, FieldType, SortOrder, SortType, @@ -42,8 +43,6 @@ describe("/v2/views", () => { }, schema: { name: { - name: "name", - type: FieldType.STRING, visible: true, }, }, @@ -108,7 +107,7 @@ describe("/v2/views", () => { visible: false, icon: "ic", }, - }, + } as Record, } const createdView = await config.api.viewV2.create(newView) @@ -151,7 +150,7 @@ describe("/v2/views", () => { presence: true, }, }, - }, + } as Record, } await config.api.viewV2.create(newView, { @@ -173,7 +172,7 @@ describe("/v2/views", () => { name: "Category", type: FieldType.STRING, }, - }, + } as Record, } await config.api.viewV2.create(newView, { @@ -316,7 +315,7 @@ describe("/v2/views", () => { visible: false, icon: "ic", }, - }, + } as Record, }) expect(await config.api.viewV2.get(view.id)).toEqual({ @@ -357,7 +356,7 @@ describe("/v2/views", () => { presence: true, }, }, - }, + } as Record, }, { expectStatus: 400, @@ -379,7 +378,7 @@ describe("/v2/views", () => { name: "Category", type: FieldType.STRING, }, - }, + } as Record, }, { expectStatus: 200, diff --git a/packages/types/src/api/web/app/view.ts b/packages/types/src/api/web/app/view.ts index 411bbef44e..60b5a01b15 100644 --- a/packages/types/src/api/web/app/view.ts +++ b/packages/types/src/api/web/app/view.ts @@ -1,4 +1,4 @@ -import { ViewV2, FieldSchema } from "../../../documents" +import { ViewV2, UIFieldMetadata } from "../../../documents" export interface ViewResponse { data: ViewV2 @@ -6,10 +6,10 @@ export interface ViewResponse { export interface CreateViewRequest extends Omit { - schema?: Record + schema?: Record } export interface UpdateViewRequest extends Omit { - schema?: Record + schema?: Record } From 44e028b574431d3f128a1563d1386c9d520cbf78 Mon Sep 17 00:00:00 2001 From: Adria Navarro Date: Tue, 1 Aug 2023 12:26:37 +0200 Subject: [PATCH 33/35] Fix test --- packages/server/src/middleware/tests/trimViewRowInfo.spec.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/server/src/middleware/tests/trimViewRowInfo.spec.ts b/packages/server/src/middleware/tests/trimViewRowInfo.spec.ts index 89b831b647..427ac9a608 100644 --- a/packages/server/src/middleware/tests/trimViewRowInfo.spec.ts +++ b/packages/server/src/middleware/tests/trimViewRowInfo.spec.ts @@ -129,7 +129,7 @@ describe("trimViewRowInfo middleware", () => { id: viewId, name: generator.guid(), tableId: table._id!, - columns: { name: { visible: true }, address: { visible: true } }, + columns: ["name", "address"], }) const data = getRandomData() From 67baa56cbf5d4b8344a5dc29468021ef648a2c3f Mon Sep 17 00:00:00 2001 From: Adria Navarro Date: Tue, 1 Aug 2023 14:40:47 +0200 Subject: [PATCH 34/35] Skip flaky test --- packages/server/src/sdk/app/rows/search/tests/external.spec.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/server/src/sdk/app/rows/search/tests/external.spec.ts b/packages/server/src/sdk/app/rows/search/tests/external.spec.ts index e2b71ac81e..b24557fd4f 100644 --- a/packages/server/src/sdk/app/rows/search/tests/external.spec.ts +++ b/packages/server/src/sdk/app/rows/search/tests/external.spec.ts @@ -13,7 +13,7 @@ jest.unmock("mysql2/promise") jest.setTimeout(30000) -describe("external", () => { +describe.skip("external", () => { const config = new TestConfiguration() let externalDatasource: Datasource From f8cbf3c29895896a58f1dcf15906678b953d6f0b Mon Sep 17 00:00:00 2001 From: Budibase Staging Release Bot <> Date: Tue, 1 Aug 2023 12:57:56 +0000 Subject: [PATCH 35/35] Bump version to 2.8.29-alpha.9 --- lerna.json | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lerna.json b/lerna.json index 2295b6a975..2b5b60e7a9 100644 --- a/lerna.json +++ b/lerna.json @@ -1,5 +1,5 @@ { - "version": "2.8.29-alpha.8", + "version": "2.8.29-alpha.9", "npmClient": "yarn", "packages": [ "packages/*"