From 3203cc3d72b8a95e4bc5e02bac1df7d1d52ab4c9 Mon Sep 17 00:00:00 2001 From: Sam Rose Date: Wed, 28 Feb 2024 16:27:14 +0000 Subject: [PATCH 01/38] Convert TableAPI. --- .../server/src/api/routes/tests/table.spec.ts | 8 +-- .../server/src/tests/utilities/api/base.ts | 65 +++++++++++++++++- .../server/src/tests/utilities/api/table.ts | 67 ++++--------------- 3 files changed, 81 insertions(+), 59 deletions(-) diff --git a/packages/server/src/api/routes/tests/table.spec.ts b/packages/server/src/api/routes/tests/table.spec.ts index ce119e56f0..4c83237a49 100644 --- a/packages/server/src/api/routes/tests/table.spec.ts +++ b/packages/server/src/api/routes/tests/table.spec.ts @@ -831,7 +831,7 @@ describe("/tables", () => { subtype: FieldSubtype.USERS, }, }, - { expectStatus: 400 } + { status: 400 } ) }) @@ -846,7 +846,7 @@ describe("/tables", () => { subtype: FieldSubtype.USERS, }, }, - { expectStatus: 400 } + { status: 400 } ) }) @@ -861,7 +861,7 @@ describe("/tables", () => { subtype: FieldSubtype.USERS, }, }, - { expectStatus: 400 } + { status: 400 } ) }) @@ -880,7 +880,7 @@ describe("/tables", () => { subtype: FieldSubtype.USERS, }, }, - { expectStatus: 400 } + { status: 400 } ) }) }) diff --git a/packages/server/src/tests/utilities/api/base.ts b/packages/server/src/tests/utilities/api/base.ts index 34120277c0..61f2c64610 100644 --- a/packages/server/src/tests/utilities/api/base.ts +++ b/packages/server/src/tests/utilities/api/base.ts @@ -2,7 +2,7 @@ import TestConfiguration from "../TestConfiguration" import { SuperTest, Test } from "supertest" export interface TestAPIOpts { - headers?: any + headers?: Record status?: number } @@ -14,4 +14,67 @@ export abstract class TestAPI { this.config = config this.request = config.request! } + + protected _get = async ( + url: string, + opts: TestAPIOpts = {} + ): Promise => { + return await this._request("get", url, undefined, opts) + } + + protected _post = async ( + url: string, + body: Record, + opts: TestAPIOpts = {} + ): Promise => { + return await this._request("post", url, body, opts) + } + + protected _put = async ( + url: string, + body: Record, + opts: TestAPIOpts = {} + ): Promise => { + return await this._request("put", url, body, opts) + } + + protected _patch = async ( + url: string, + body: Record, + opts: TestAPIOpts = {} + ): Promise => { + return await this._request("patch", url, body, opts) + } + + protected _delete = async ( + url: string, + body: Record, + opts: TestAPIOpts = {} + ): Promise => { + return await this._request("delete", url, body, opts) + } + + protected _request = async ( + method: "get" | "post" | "put" | "patch" | "delete", + url: string, + body?: Record, + opts: TestAPIOpts = {} + ): Promise => { + const { headers = {}, status = 200 } = opts + const response = await this.request[method](url) + .send(body) + .set(this.config.defaultHeaders()) + .set(headers) + .expect("Content-Type", /json/) + + if (response.status !== status) { + throw new Error( + `Expected status ${status} but got ${ + response.status + } with body ${JSON.stringify(response.body)}` + ) + } + + return response.body as T + } } diff --git a/packages/server/src/tests/utilities/api/table.ts b/packages/server/src/tests/utilities/api/table.ts index 5a9654e3bc..d2f16e0e1b 100644 --- a/packages/server/src/tests/utilities/api/table.ts +++ b/packages/server/src/tests/utilities/api/table.ts @@ -5,74 +5,33 @@ import { SaveTableResponse, Table, } from "@budibase/types" -import TestConfiguration from "../TestConfiguration" -import { TestAPI } from "./base" +import { TestAPI, TestAPIOpts } from "./base" export class TableAPI extends TestAPI { - constructor(config: TestConfiguration) { - super(config) - } - save = async ( data: SaveTableRequest, - { expectStatus } = { expectStatus: 200 } + opts?: TestAPIOpts ): Promise => { - const res = await this.request - .post(`/api/tables`) - .send(data) - .set(this.config.defaultHeaders()) - .expect("Content-Type", /json/) - - if (res.status !== expectStatus) { - throw new Error( - `Expected status ${expectStatus} but got ${ - res.status - } with body ${JSON.stringify(res.body)}` - ) - } - - return res.body + return await this._post("/api/tables", data, opts) } - fetch = async ( - { expectStatus } = { expectStatus: 200 } - ): Promise => { - const res = await this.request - .get(`/api/tables`) - .set(this.config.defaultHeaders()) - .expect("Content-Type", /json/) - .expect(expectStatus) - return res.body + fetch = async (opts?: TestAPIOpts): Promise => { + return await this._get("/api/tables", opts) } - get = async ( - tableId: string, - { expectStatus } = { expectStatus: 200 } - ): Promise => { - const res = await this.request - .get(`/api/tables/${tableId}`) - .set(this.config.defaultHeaders()) - .expect("Content-Type", /json/) - .expect(expectStatus) - return res.body + get = async (tableId: string, opts?: TestAPIOpts): Promise
=> { + return await this._get
(`/api/tables/${tableId}`, opts) } migrate = async ( tableId: string, data: MigrateRequest, - { expectStatus } = { expectStatus: 200 } + opts?: TestAPIOpts ): Promise => { - const res = await this.request - .post(`/api/tables/${tableId}/migrate`) - .send(data) - .set(this.config.defaultHeaders()) - if (res.status !== expectStatus) { - throw new Error( - `Expected status ${expectStatus} but got ${ - res.status - } with body ${JSON.stringify(res.body)}` - ) - } - return res.body + return await this._post( + `/api/tables/${tableId}/migrate`, + data, + opts + ) } } From e876d14b92673ec75fe942eb17195583a067ef25 Mon Sep 17 00:00:00 2001 From: Andrew Kingston Date: Wed, 28 Feb 2024 16:43:41 +0000 Subject: [PATCH 02/38] Ensure unsaved pending changes to rows are applied when changing cell --- .../src/components/grid/stores/rows.js | 51 ++++++++++++++----- .../src/components/grid/stores/ui.js | 10 ++++ .../src/components/grid/stores/validation.js | 22 +++++++- 3 files changed, 70 insertions(+), 13 deletions(-) diff --git a/packages/frontend-core/src/components/grid/stores/rows.js b/packages/frontend-core/src/components/grid/stores/rows.js index 34ba77afa2..f4b0e97942 100644 --- a/packages/frontend-core/src/components/grid/stores/rows.js +++ b/packages/frontend-core/src/components/grid/stores/rows.js @@ -328,25 +328,34 @@ export const createActions = context => { } // Patches a row with some changes - const updateRow = async (rowId, changes, options = { save: true }) => { + const updateRow = async ( + rowId, + changes, + options = { save: true, force: false } + ) => { const $rows = get(rows) const $rowLookupMap = get(rowLookupMap) const index = $rowLookupMap[rowId] const row = $rows[index] - if (index == null || !Object.keys(changes || {}).length) { + if (index == null) { + return + } + if (!options?.force && !Object.keys(changes || {}).length) { return } // Abandon if no changes - let same = true - for (let column of Object.keys(changes)) { - if (row[column] !== changes[column]) { - same = false - break + if (!options?.force) { + let same = true + for (let column of Object.keys(changes)) { + if (row[column] !== changes[column]) { + same = false + break + } + } + if (same) { + return } - } - if (same) { - return } // Immediately update state so that the change is reflected @@ -359,7 +368,7 @@ export const createActions = context => { })) // Stop here if we don't want to persist the change - if (!options?.save) { + if (!options?.save && !options?.force) { return } @@ -508,7 +517,14 @@ export const createActions = context => { } export const initialise = context => { - const { rowChangeCache, inProgressChanges, previousFocusedRowId } = context + const { + rowChangeCache, + inProgressChanges, + previousFocusedRowId, + previousFocusedCellId, + rows, + validation, + } = context // Wipe the row change cache when changing row previousFocusedRowId.subscribe(id => { @@ -519,4 +535,15 @@ export const initialise = context => { }) } }) + + // Ensure any unsaved changes are saved when changing cell + previousFocusedCellId.subscribe(id => { + const rowId = id?.split("-")[0] + const hasErrors = validation.actions.rowHasErrors(rowId) + const hasChanges = Object.keys(get(rowChangeCache)[rowId] || {}).length > 0 + const isSavingChanges = get(inProgressChanges)[rowId] + if (rowId && !hasErrors && hasChanges && !isSavingChanges) { + rows.actions.updateRow(rowId, null, { force: true }) + } + }) } diff --git a/packages/frontend-core/src/components/grid/stores/ui.js b/packages/frontend-core/src/components/grid/stores/ui.js index 129d6614e5..da0558bb5b 100644 --- a/packages/frontend-core/src/components/grid/stores/ui.js +++ b/packages/frontend-core/src/components/grid/stores/ui.js @@ -16,6 +16,7 @@ export const createStores = context => { const hoveredRowId = writable(null) const rowHeight = writable(get(props).fixedRowHeight || DefaultRowHeight) const previousFocusedRowId = writable(null) + const previousFocusedCellId = writable(null) const gridFocused = writable(false) const isDragging = writable(false) const buttonColumnWidth = writable(0) @@ -48,6 +49,7 @@ export const createStores = context => { focusedCellAPI, focusedRowId, previousFocusedRowId, + previousFocusedCellId, hoveredRowId, rowHeight, gridFocused, @@ -129,6 +131,7 @@ export const initialise = context => { const { focusedRowId, previousFocusedRowId, + previousFocusedCellId, rows, focusedCellId, selectedRows, @@ -181,6 +184,13 @@ export const initialise = context => { lastFocusedRowId = id }) + // Remember the last focused cell ID so that we can store the previous one + let lastFocusedCellId = null + focusedCellId.subscribe(id => { + previousFocusedCellId.set(lastFocusedCellId) + lastFocusedCellId = id + }) + // Remove hovered row when a cell is selected focusedCellId.subscribe(cell => { if (cell && get(hoveredRowId)) { diff --git a/packages/frontend-core/src/components/grid/stores/validation.js b/packages/frontend-core/src/components/grid/stores/validation.js index 9c3927f9c9..70db076593 100644 --- a/packages/frontend-core/src/components/grid/stores/validation.js +++ b/packages/frontend-core/src/components/grid/stores/validation.js @@ -1,8 +1,23 @@ -import { writable, get } from "svelte/store" +import { writable, get, derived } from "svelte/store" +// Normally we would break out actions into the explicit "createActions" +// function, but for validation all these actions are pure so can go into +// "createStores" instead to make dependency ordering simpler export const createStores = () => { const validation = writable({}) + // Derive which rows have errors so that we can use that info later + const rowErrorMap = derived(validation, $validation => { + let map = {} + Object.entries($validation).forEach(([key, error]) => { + // Extract row ID from all errored cell IDs + if (error) { + map[key.split("-")[0]] = true + } + }) + return map + }) + const setError = (cellId, error) => { if (!cellId) { return @@ -13,11 +28,16 @@ export const createStores = () => { })) } + const rowHasErrors = rowId => { + return get(rowErrorMap)[rowId] + } + return { validation: { ...validation, actions: { setError, + rowHasErrors, }, }, } From f1ed7af4393bc5cfeaa0d862b3ff591428f79e88 Mon Sep 17 00:00:00 2001 From: Sam Rose Date: Wed, 28 Feb 2024 16:55:45 +0000 Subject: [PATCH 03/38] Rework the API slightly. --- .../server/src/tests/utilities/api/base.ts | 90 ++++++++++++------- .../server/src/tests/utilities/api/table.ts | 28 +++--- 2 files changed, 72 insertions(+), 46 deletions(-) diff --git a/packages/server/src/tests/utilities/api/base.ts b/packages/server/src/tests/utilities/api/base.ts index 61f2c64610..e5818f7e23 100644 --- a/packages/server/src/tests/utilities/api/base.ts +++ b/packages/server/src/tests/utilities/api/base.ts @@ -1,71 +1,95 @@ import TestConfiguration from "../TestConfiguration" import { SuperTest, Test } from "supertest" +type Headers = Record + export interface TestAPIOpts { - headers?: Record + headers?: Headers status?: number } +export interface Expectations { + status?: number + contentType?: string | RegExp +} + +export interface RequestOpts { + headers?: Headers + body?: Record + fields?: Record + files?: Record + expectations?: Expectations +} + export abstract class TestAPI { config: TestConfiguration request: SuperTest - protected constructor(config: TestConfiguration) { + constructor(config: TestConfiguration) { this.config = config this.request = config.request! } protected _get = async ( url: string, - opts: TestAPIOpts = {} + expectations?: Expectations ): Promise => { - return await this._request("get", url, undefined, opts) + return await this._request("get", url, { expectations }) } - protected _post = async ( - url: string, - body: Record, - opts: TestAPIOpts = {} - ): Promise => { - return await this._request("post", url, body, opts) + protected _post = async (url: string, opts?: RequestOpts): Promise => { + return await this._request("post", url, opts) } - protected _put = async ( - url: string, - body: Record, - opts: TestAPIOpts = {} - ): Promise => { - return await this._request("put", url, body, opts) + protected _put = async (url: string, opts?: RequestOpts): Promise => { + return await this._request("put", url, opts) } - protected _patch = async ( - url: string, - body: Record, - opts: TestAPIOpts = {} - ): Promise => { - return await this._request("patch", url, body, opts) + protected _patch = async (url: string, opts?: RequestOpts): Promise => { + return await this._request("patch", url, opts) } protected _delete = async ( url: string, - body: Record, - opts: TestAPIOpts = {} + opts?: RequestOpts ): Promise => { - return await this._request("delete", url, body, opts) + return await this._request("delete", url, opts) } protected _request = async ( method: "get" | "post" | "put" | "patch" | "delete", url: string, - body?: Record, - opts: TestAPIOpts = {} + opts?: RequestOpts ): Promise => { - const { headers = {}, status = 200 } = opts - const response = await this.request[method](url) - .send(body) - .set(this.config.defaultHeaders()) - .set(headers) - .expect("Content-Type", /json/) + const { headers = {}, body, fields, files, expectations } = opts || {} + const { status = 200, contentType = /json/ } = expectations || {} + + let request = this.request[method](url).set(this.config.defaultHeaders()) + if (headers) { + request = request.set(headers) + } + if (body) { + request = request.send(body) + } + if (fields) { + for (const [key, value] of Object.entries(fields)) { + request = request.field(key, value) + } + } + if (files) { + for (const [key, value] of Object.entries(files)) { + request = request.attach(key, value) + } + } + if (contentType) { + if (contentType instanceof RegExp) { + request = request.expect("Content-Type", contentType) + } else { + request = request.expect("Content-Type", contentType) + } + } + + const response = await request if (response.status !== status) { throw new Error( diff --git a/packages/server/src/tests/utilities/api/table.ts b/packages/server/src/tests/utilities/api/table.ts index d2f16e0e1b..90ad0e04fb 100644 --- a/packages/server/src/tests/utilities/api/table.ts +++ b/packages/server/src/tests/utilities/api/table.ts @@ -5,33 +5,35 @@ import { SaveTableResponse, Table, } from "@budibase/types" -import { TestAPI, TestAPIOpts } from "./base" +import { Expectations, TestAPI } from "./base" export class TableAPI extends TestAPI { save = async ( data: SaveTableRequest, - opts?: TestAPIOpts + expectations?: Expectations ): Promise => { - return await this._post("/api/tables", data, opts) + return await this._post("/api/tables", { + body: data, + expectations, + }) } - fetch = async (opts?: TestAPIOpts): Promise => { - return await this._get("/api/tables", opts) + fetch = async (expectations?: Expectations): Promise => { + return await this._get("/api/tables", expectations) } - get = async (tableId: string, opts?: TestAPIOpts): Promise
=> { - return await this._get
(`/api/tables/${tableId}`, opts) + get = async (tableId: string, expectations: Expectations): Promise
=> { + return await this._get
(`/api/tables/${tableId}`, expectations) } migrate = async ( tableId: string, data: MigrateRequest, - opts?: TestAPIOpts + expectations?: Expectations ): Promise => { - return await this._post( - `/api/tables/${tableId}/migrate`, - data, - opts - ) + return await this._post(`/api/tables/${tableId}/migrate`, { + body: data, + expectations, + }) } } From 7a48fd85acfeac57b4a22d2237a9b549c7973b5d Mon Sep 17 00:00:00 2001 From: Sam Rose Date: Wed, 28 Feb 2024 17:27:15 +0000 Subject: [PATCH 04/38] Migrate ApplicationAPI --- .../src/api/routes/tests/application.spec.ts | 2 +- .../appMigrations/tests/migrations.spec.ts | 14 +- .../src/tests/utilities/api/application.ts | 231 +++++++----------- .../server/src/tests/utilities/api/base.ts | 51 +++- .../server/src/tests/utilities/api/table.ts | 4 +- packages/types/src/api/web/application.ts | 6 + 6 files changed, 149 insertions(+), 159 deletions(-) diff --git a/packages/server/src/api/routes/tests/application.spec.ts b/packages/server/src/api/routes/tests/application.spec.ts index dbe4eb51ae..dc235dbd01 100644 --- a/packages/server/src/api/routes/tests/application.spec.ts +++ b/packages/server/src/api/routes/tests/application.spec.ts @@ -184,7 +184,7 @@ describe("/applications", () => { it("app should not sync if production", async () => { const { message } = await config.api.application.sync( app.appId.replace("_dev", ""), - { statusCode: 400 } + { status: 400 } ) expect(message).toEqual( diff --git a/packages/server/src/appMigrations/tests/migrations.spec.ts b/packages/server/src/appMigrations/tests/migrations.spec.ts index 5eb8535695..7af2346934 100644 --- a/packages/server/src/appMigrations/tests/migrations.spec.ts +++ b/packages/server/src/appMigrations/tests/migrations.spec.ts @@ -30,9 +30,9 @@ describe("migrations", () => { const appId = config.getAppId() - const response = await config.api.application.getRaw(appId) - - expect(response.headers[Header.MIGRATING_APP]).toBeUndefined() + await config.api.application.get(appId, { + headersNotPresent: [Header.MIGRATING_APP], + }) }) it("accessing an app that has pending migrations will attach the migrating header", async () => { @@ -46,8 +46,10 @@ describe("migrations", () => { func: async () => {}, }) - const response = await config.api.application.getRaw(appId) - - expect(response.headers[Header.MIGRATING_APP]).toEqual(appId) + await config.api.application.get(appId, { + headers: { + [Header.MIGRATING_APP]: appId, + }, + }) }) }) diff --git a/packages/server/src/tests/utilities/api/application.ts b/packages/server/src/tests/utilities/api/application.ts index 3951bba667..f215a98edb 100644 --- a/packages/server/src/tests/utilities/api/application.ts +++ b/packages/server/src/tests/utilities/api/application.ts @@ -1,12 +1,12 @@ -import { Response } from "supertest" import { App, + PublishResponse, type CreateAppRequest, type FetchAppDefinitionResponse, type FetchAppPackageResponse, } from "@budibase/types" import TestConfiguration from "../TestConfiguration" -import { TestAPI } from "./base" +import { Expectations, TestAPI } from "./base" import { AppStatus } from "../../../db/utils" import { constants } from "@budibase/backend-core" @@ -15,179 +15,124 @@ export class ApplicationAPI extends TestAPI { super(config) } - create = async (app: CreateAppRequest): Promise => { - const request = this.request - .post("/api/applications") - .set(this.config.defaultHeaders()) - .expect("Content-Type", /json/) - - for (const key of Object.keys(app)) { - request.field(key, (app as any)[key]) - } - - if (app.templateFile) { - request.attach("templateFile", app.templateFile) - } - - const result = await request - - if (result.statusCode !== 200) { - throw new Error(JSON.stringify(result.body)) - } - - return result.body as App + create = async ( + app: CreateAppRequest, + expectations?: Expectations + ): Promise => { + const files = app.templateFile ? { templateFile: app.templateFile } : {} + delete app.templateFile + return await this._post("/api/applications", { + fields: app, + files, + expectations, + }) } - delete = async (appId: string): Promise => { - await this.request - .delete(`/api/applications/${appId}`) - .set(this.config.defaultHeaders()) - .expect(200) + delete = async ( + appId: string, + expectations?: Expectations + ): Promise => { + await this._delete(`/api/applications/${appId}`, { expectations }) } - publish = async ( - appId: string - ): Promise<{ _id: string; status: string; appUrl: string }> => { - // While the publish endpoint does take an :appId parameter, it doesn't - // use it. It uses the appId from the context. - let headers = { - ...this.config.defaultHeaders(), - [constants.Header.APP_ID]: appId, - } - const result = await this.request - .post(`/api/applications/${appId}/publish`) - .set(headers) - .expect("Content-Type", /json/) - .expect(200) - return result.body as { _id: string; status: string; appUrl: string } + publish = async (appId: string): Promise => { + return await this._post( + `/api/applications/${appId}/publish`, + { + // While the publish endpoint does take an :appId parameter, it doesn't + // use it. It uses the appId from the context. + headers: { + [constants.Header.APP_ID]: appId, + }, + } + ) } unpublish = async (appId: string): Promise => { - await this.request - .post(`/api/applications/${appId}/unpublish`) - .set(this.config.defaultHeaders()) - .expect(204) + await this._post(`/api/applications/${appId}/unpublish`, { + expectations: { status: 204 }, + }) } sync = async ( appId: string, - { statusCode }: { statusCode: number } = { statusCode: 200 } + expectations?: Expectations ): Promise<{ message: string }> => { - const result = await this.request - .post(`/api/applications/${appId}/sync`) - .set(this.config.defaultHeaders()) - .expect("Content-Type", /json/) - .expect(statusCode) - return result.body + return await this._post<{ message: string }>( + `/api/applications/${appId}/sync`, + { expectations } + ) } - getRaw = async (appId: string): Promise => { - // While the appPackage endpoint does take an :appId parameter, it doesn't - // use it. It uses the appId from the context. - let headers = { - ...this.config.defaultHeaders(), - [constants.Header.APP_ID]: appId, - } - const result = await this.request - .get(`/api/applications/${appId}/appPackage`) - .set(headers) - .expect("Content-Type", /json/) - .expect(200) - return result - } - - get = async (appId: string): Promise => { - const result = await this.getRaw(appId) - return result.body.application as App + get = async (appId: string, expectations?: Expectations): Promise => { + return await this._get(`/api/applications/${appId}`, { + // While the get endpoint does take an :appId parameter, it doesn't use + // it. It uses the appId from the context. + headers: { + [constants.Header.APP_ID]: appId, + }, + expectations, + }) } getDefinition = async ( - appId: string + appId: string, + expectations?: Expectations ): Promise => { - const result = await this.request - .get(`/api/applications/${appId}/definition`) - .set(this.config.defaultHeaders()) - .expect("Content-Type", /json/) - .expect(200) - return result.body as FetchAppDefinitionResponse + return await this._get( + `/api/applications/${appId}/definition`, + { expectations } + ) } - getAppPackage = async (appId: string): Promise => { - const result = await this.request - .get(`/api/applications/${appId}/appPackage`) - .set(this.config.defaultHeaders()) - .expect("Content-Type", /json/) - .expect(200) - return result.body + getAppPackage = async ( + appId: string, + expectations?: Expectations + ): Promise => { + return await this._get( + `/api/applications/${appId}/appPackage`, + { expectations } + ) } update = async ( appId: string, - app: { name?: string; url?: string } + app: { name?: string; url?: string }, + expectations?: Expectations ): Promise => { - const request = this.request - .put(`/api/applications/${appId}`) - .set(this.config.defaultHeaders()) - .expect("Content-Type", /json/) - - for (const key of Object.keys(app)) { - request.field(key, (app as any)[key]) - } - - const result = await request - - if (result.statusCode !== 200) { - throw new Error(JSON.stringify(result.body)) - } - - return result.body as App + return await this._put(`/api/applications/${appId}`, { + fields: app, + expectations, + }) } - updateClient = async (appId: string): Promise => { - // While the updateClient endpoint does take an :appId parameter, it doesn't - // use it. It uses the appId from the context. - let headers = { - ...this.config.defaultHeaders(), - [constants.Header.APP_ID]: appId, - } - const response = await this.request - .post(`/api/applications/${appId}/client/update`) - .set(headers) - .expect("Content-Type", /json/) - - if (response.statusCode !== 200) { - throw new Error(JSON.stringify(response.body)) - } + updateClient = async ( + appId: string, + expectations?: Expectations + ): Promise => { + await this._post(`/api/applications/${appId}/client/update`, { + // While the updateClient endpoint does take an :appId parameter, it doesn't + // use it. It uses the appId from the context. + headers: { + [constants.Header.APP_ID]: appId, + }, + expectations, + }) } revertClient = async (appId: string): Promise => { - // While the revertClient endpoint does take an :appId parameter, it doesn't - // use it. It uses the appId from the context. - let headers = { - ...this.config.defaultHeaders(), - [constants.Header.APP_ID]: appId, - } - const response = await this.request - .post(`/api/applications/${appId}/client/revert`) - .set(headers) - .expect("Content-Type", /json/) - - if (response.statusCode !== 200) { - throw new Error(JSON.stringify(response.body)) - } + await this._post(`/api/applications/${appId}/client/revert`, { + // While the revertClient endpoint does take an :appId parameter, it doesn't + // use it. It uses the appId from the context. + headers: { + [constants.Header.APP_ID]: appId, + }, + }) } fetch = async ({ status }: { status?: AppStatus } = {}): Promise => { - let query = [] - if (status) { - query.push(`status=${status}`) - } - - const result = await this.request - .get(`/api/applications${query.length ? `?${query.join("&")}` : ""}`) - .set(this.config.defaultHeaders()) - .expect("Content-Type", /json/) - .expect(200) - return result.body as App[] + return await this._get("/api/applications", { + query: { status }, + }) } } diff --git a/packages/server/src/tests/utilities/api/base.ts b/packages/server/src/tests/utilities/api/base.ts index e5818f7e23..8325c1caf6 100644 --- a/packages/server/src/tests/utilities/api/base.ts +++ b/packages/server/src/tests/utilities/api/base.ts @@ -1,3 +1,4 @@ +import exp from "constants" import TestConfiguration from "../TestConfiguration" import { SuperTest, Test } from "supertest" @@ -11,10 +12,13 @@ export interface TestAPIOpts { export interface Expectations { status?: number contentType?: string | RegExp + headers?: Record + headersNotPresent?: string[] } export interface RequestOpts { headers?: Headers + query?: Record body?: Record fields?: Record files?: Record @@ -30,11 +34,8 @@ export abstract class TestAPI { this.request = config.request! } - protected _get = async ( - url: string, - expectations?: Expectations - ): Promise => { - return await this._request("get", url, { expectations }) + protected _get = async (url: string, opts?: RequestOpts): Promise => { + return await this._request("get", url, opts) } protected _post = async (url: string, opts?: RequestOpts): Promise => { @@ -61,9 +62,26 @@ export abstract class TestAPI { url: string, opts?: RequestOpts ): Promise => { - const { headers = {}, body, fields, files, expectations } = opts || {} + const { + headers = {}, + query = {}, + body, + fields, + files, + expectations, + } = opts || {} const { status = 200, contentType = /json/ } = expectations || {} + let queryParams = [] + for (const [key, value] of Object.entries(query)) { + if (value) { + queryParams.push(`${key}=${value}`) + } + } + if (queryParams.length) { + url += `?${queryParams.join("&")}` + } + let request = this.request[method](url).set(this.config.defaultHeaders()) if (headers) { request = request.set(headers) @@ -81,13 +99,22 @@ export abstract class TestAPI { request = request.attach(key, value) } } - if (contentType) { + if (contentType && status !== 204) { if (contentType instanceof RegExp) { request = request.expect("Content-Type", contentType) } else { request = request.expect("Content-Type", contentType) } } + if (expectations?.headers) { + for (const [key, value] of Object.entries(expectations.headers)) { + if (value instanceof RegExp) { + request = request.expect(key, value) + } else { + request = request.expect(key, value) + } + } + } const response = await request @@ -99,6 +126,16 @@ export abstract class TestAPI { ) } + if (expectations?.headersNotPresent) { + for (const header of expectations.headersNotPresent) { + if (response.headers[header]) { + throw new Error( + `Expected header ${header} not to be present, found value "${response.headers[header]}"` + ) + } + } + } + return response.body as T } } diff --git a/packages/server/src/tests/utilities/api/table.ts b/packages/server/src/tests/utilities/api/table.ts index 90ad0e04fb..b692e4ed1a 100644 --- a/packages/server/src/tests/utilities/api/table.ts +++ b/packages/server/src/tests/utilities/api/table.ts @@ -19,11 +19,11 @@ export class TableAPI extends TestAPI { } fetch = async (expectations?: Expectations): Promise => { - return await this._get("/api/tables", expectations) + return await this._get("/api/tables", { expectations }) } get = async (tableId: string, expectations: Expectations): Promise
=> { - return await this._get
(`/api/tables/${tableId}`, expectations) + return await this._get
(`/api/tables/${tableId}`, { expectations }) } migrate = async ( diff --git a/packages/types/src/api/web/application.ts b/packages/types/src/api/web/application.ts index 87a0bd6ef9..3d33fce5b1 100644 --- a/packages/types/src/api/web/application.ts +++ b/packages/types/src/api/web/application.ts @@ -27,3 +27,9 @@ export interface FetchAppPackageResponse { clientLibPath: string hasLock: boolean } + +export interface PublishResponse { + _id: string + status: string + appUrl: string +} From d9cffa1878e02fe40ec83a7ef9eaf83a36ee6e15 Mon Sep 17 00:00:00 2001 From: Sam Rose Date: Wed, 28 Feb 2024 17:43:39 +0000 Subject: [PATCH 05/38] Migrate AttachmentAPI. --- .../src/api/routes/tests/attachment.spec.ts | 6 +-- .../src/tests/utilities/api/application.ts | 5 -- .../src/tests/utilities/api/attachment.ts | 33 +++--------- .../server/src/tests/utilities/api/base.ts | 54 +++++++++++-------- 4 files changed, 43 insertions(+), 55 deletions(-) diff --git a/packages/server/src/api/routes/tests/attachment.spec.ts b/packages/server/src/api/routes/tests/attachment.spec.ts index e230b0688a..aa02ea898e 100644 --- a/packages/server/src/api/routes/tests/attachment.spec.ts +++ b/packages/server/src/api/routes/tests/attachment.spec.ts @@ -29,7 +29,7 @@ describe("/api/applications/:appId/sync", () => { let resp = (await config.api.attachment.process( "ohno.exe", Buffer.from([0]), - { expectStatus: 400 } + { status: 400 } )) as unknown as APIError expect(resp.message).toContain("invalid extension") }) @@ -40,7 +40,7 @@ describe("/api/applications/:appId/sync", () => { let resp = (await config.api.attachment.process( "OHNO.EXE", Buffer.from([0]), - { expectStatus: 400 } + { status: 400 } )) as unknown as APIError expect(resp.message).toContain("invalid extension") }) @@ -51,7 +51,7 @@ describe("/api/applications/:appId/sync", () => { undefined as any, undefined as any, { - expectStatus: 400, + status: 400, } )) as unknown as APIError expect(resp.message).toContain("No file provided") diff --git a/packages/server/src/tests/utilities/api/application.ts b/packages/server/src/tests/utilities/api/application.ts index f215a98edb..da3d7cefd8 100644 --- a/packages/server/src/tests/utilities/api/application.ts +++ b/packages/server/src/tests/utilities/api/application.ts @@ -5,16 +5,11 @@ import { type FetchAppDefinitionResponse, type FetchAppPackageResponse, } from "@budibase/types" -import TestConfiguration from "../TestConfiguration" import { Expectations, TestAPI } from "./base" import { AppStatus } from "../../../db/utils" import { constants } from "@budibase/backend-core" export class ApplicationAPI extends TestAPI { - constructor(config: TestConfiguration) { - super(config) - } - create = async ( app: CreateAppRequest, expectations?: Expectations diff --git a/packages/server/src/tests/utilities/api/attachment.ts b/packages/server/src/tests/utilities/api/attachment.ts index a466f1a67e..bb33ef04bb 100644 --- a/packages/server/src/tests/utilities/api/attachment.ts +++ b/packages/server/src/tests/utilities/api/attachment.ts @@ -1,35 +1,16 @@ -import { - APIError, - Datasource, - ProcessAttachmentResponse, -} from "@budibase/types" -import TestConfiguration from "../TestConfiguration" -import { TestAPI } from "./base" +import { ProcessAttachmentResponse } from "@budibase/types" +import { Expectations, TestAPI } from "./base" import fs from "fs" export class AttachmentAPI extends TestAPI { - constructor(config: TestConfiguration) { - super(config) - } - process = async ( name: string, file: Buffer | fs.ReadStream | string, - { expectStatus } = { expectStatus: 200 } + expectations?: Expectations ): Promise => { - const result = await this.request - .post(`/api/attachments/process`) - .attach("file", file, name) - .set(this.config.defaultHeaders()) - - if (result.statusCode !== expectStatus) { - throw new Error( - `Expected status ${expectStatus} but got ${ - result.statusCode - }, body: ${JSON.stringify(result.body)}` - ) - } - - return result.body + return await this._post(`/api/attachments/process`, { + files: { file: { name, file } }, + expectations, + }) } } diff --git a/packages/server/src/tests/utilities/api/base.ts b/packages/server/src/tests/utilities/api/base.ts index 8325c1caf6..9a6ff2e6ca 100644 --- a/packages/server/src/tests/utilities/api/base.ts +++ b/packages/server/src/tests/utilities/api/base.ts @@ -1,6 +1,6 @@ -import exp from "constants" import TestConfiguration from "../TestConfiguration" import { SuperTest, Test } from "supertest" +import { ReadStream } from "fs" type Headers = Record @@ -9,6 +9,22 @@ export interface TestAPIOpts { status?: number } +export interface AttachedFile { + name: string + file: Buffer | ReadStream | string +} + +function isAttachedFile(file: any): file is AttachedFile { + if (file === undefined) { + return false + } + const attachedFile = file as AttachedFile + return ( + Object.hasOwnProperty.call(attachedFile, "file") && + Object.hasOwnProperty.call(attachedFile, "name") + ) +} + export interface Expectations { status?: number contentType?: string | RegExp @@ -21,7 +37,10 @@ export interface RequestOpts { query?: Record body?: Record fields?: Record - files?: Record + files?: Record< + string, + Buffer | ReadStream | string | AttachedFile | undefined + > expectations?: Expectations } @@ -66,8 +85,8 @@ export abstract class TestAPI { headers = {}, query = {}, body, - fields, - files, + fields = {}, + files = {}, expectations, } = opts || {} const { status = 200, contentType = /json/ } = expectations || {} @@ -89,30 +108,23 @@ export abstract class TestAPI { if (body) { request = request.send(body) } - if (fields) { - for (const [key, value] of Object.entries(fields)) { - request = request.field(key, value) - } + for (const [key, value] of Object.entries(fields)) { + request = request.field(key, value) } - if (files) { - for (const [key, value] of Object.entries(files)) { - request = request.attach(key, value) + + for (const [key, value] of Object.entries(files)) { + if (isAttachedFile(value)) { + request = request.attach(key, value.file, value.name) + } else { + request = request.attach(key, value as any) } } if (contentType && status !== 204) { - if (contentType instanceof RegExp) { - request = request.expect("Content-Type", contentType) - } else { - request = request.expect("Content-Type", contentType) - } + request = request.expect("Content-Type", contentType as any) } if (expectations?.headers) { for (const [key, value] of Object.entries(expectations.headers)) { - if (value instanceof RegExp) { - request = request.expect(key, value) - } else { - request = request.expect(key, value) - } + request = request.expect(key, value as any) } } From e309282ff795a4752fa8c09f4a8b714f7c7a9af5 Mon Sep 17 00:00:00 2001 From: Sam Rose Date: Wed, 28 Feb 2024 17:46:16 +0000 Subject: [PATCH 06/38] Fix type checks. --- packages/server/src/tests/utilities/api/base.ts | 5 ----- packages/server/src/tests/utilities/api/table.ts | 5 ++++- 2 files changed, 4 insertions(+), 6 deletions(-) diff --git a/packages/server/src/tests/utilities/api/base.ts b/packages/server/src/tests/utilities/api/base.ts index 9a6ff2e6ca..46fb7eb3b9 100644 --- a/packages/server/src/tests/utilities/api/base.ts +++ b/packages/server/src/tests/utilities/api/base.ts @@ -4,11 +4,6 @@ import { ReadStream } from "fs" type Headers = Record -export interface TestAPIOpts { - headers?: Headers - status?: number -} - export interface AttachedFile { name: string file: Buffer | ReadStream | string diff --git a/packages/server/src/tests/utilities/api/table.ts b/packages/server/src/tests/utilities/api/table.ts index b692e4ed1a..49105a3883 100644 --- a/packages/server/src/tests/utilities/api/table.ts +++ b/packages/server/src/tests/utilities/api/table.ts @@ -22,7 +22,10 @@ export class TableAPI extends TestAPI { return await this._get("/api/tables", { expectations }) } - get = async (tableId: string, expectations: Expectations): Promise
=> { + get = async ( + tableId: string, + expectations?: Expectations + ): Promise
=> { return await this._get
(`/api/tables/${tableId}`, { expectations }) } From 8488ff4144872d6e2dc5a9f4de502e0801627f07 Mon Sep 17 00:00:00 2001 From: Sam Rose Date: Thu, 29 Feb 2024 09:19:14 +0000 Subject: [PATCH 07/38] Print stack traces from inside request handler. --- .../src/middleware/errorHandling.ts | 23 ++++++++++------ .../server/src/tests/utilities/api/base.ts | 26 +++++++++++++++---- 2 files changed, 36 insertions(+), 13 deletions(-) diff --git a/packages/backend-core/src/middleware/errorHandling.ts b/packages/backend-core/src/middleware/errorHandling.ts index ebdd4107e9..ae5373a2e0 100644 --- a/packages/backend-core/src/middleware/errorHandling.ts +++ b/packages/backend-core/src/middleware/errorHandling.ts @@ -1,5 +1,6 @@ import { APIError } from "@budibase/types" import * as errors from "../errors" +import environment from "../environment" export async function errorHandling(ctx: any, next: any) { try { @@ -14,15 +15,21 @@ export async function errorHandling(ctx: any, next: any) { console.error(err) } - const error = errors.getPublicError(err) - const body: APIError = { - message: err.message, - status: status, - validationErrors: err.validation, - error, + if (environment.isTest()) { + ctx.body = { + message: err.message, + status: status, + error: errors.getPublicError(err), + stack: err.stack, + } + } else { + ctx.body = { + message: err.message, + status: status, + validationErrors: err.validation, + error: errors.getPublicError(err), + } } - - ctx.body = body } } diff --git a/packages/server/src/tests/utilities/api/base.ts b/packages/server/src/tests/utilities/api/base.ts index 46fb7eb3b9..42911628fd 100644 --- a/packages/server/src/tests/utilities/api/base.ts +++ b/packages/server/src/tests/utilities/api/base.ts @@ -126,11 +126,27 @@ export abstract class TestAPI { const response = await request if (response.status !== status) { - throw new Error( - `Expected status ${status} but got ${ - response.status - } with body ${JSON.stringify(response.body)}` - ) + let message = `Expected status ${status} but got ${response.status}` + + const stack = response.body.stack + delete response.body.stack + + if (response.body) { + message += `\n\nBody:` + const body = JSON.stringify(response.body, null, 2) + for (const line of body.split("\n")) { + message += `\n⏐ ${line}` + } + } + + if (stack) { + message += `\n\nStack from request handler:` + for (const line of stack.split("\n")) { + message += `\n⏐ ${line}` + } + } + + throw new Error(message) } if (expectations?.headersNotPresent) { From acecea570499d26ddd453157d6abedfc86a43dff Mon Sep 17 00:00:00 2001 From: Andrew Kingston Date: Thu, 29 Feb 2024 10:30:38 +0000 Subject: [PATCH 08/38] Refactor grid row actions to be more explicit and remove extraneous flags --- .../src/components/grid/cells/DataCell.svelte | 4 +- .../grid/overlays/KeyboardManager.svelte | 4 +- .../src/components/grid/stores/rows.js | 100 +++++++++--------- 3 files changed, 54 insertions(+), 54 deletions(-) diff --git a/packages/frontend-core/src/components/grid/cells/DataCell.svelte b/packages/frontend-core/src/components/grid/cells/DataCell.svelte index cdaf28978a..d8cff26b9d 100644 --- a/packages/frontend-core/src/components/grid/cells/DataCell.svelte +++ b/packages/frontend-core/src/components/grid/cells/DataCell.svelte @@ -59,13 +59,13 @@ isReadonly: () => readonly, getType: () => column.schema.type, getValue: () => row[column.name], - setValue: (value, options = { save: true }) => { + setValue: (value, options = { apply: true }) => { validation.actions.setError(cellId, null) updateValue({ rowId: row._id, column: column.name, value, - save: options?.save, + apply: options?.apply, }) }, } diff --git a/packages/frontend-core/src/components/grid/overlays/KeyboardManager.svelte b/packages/frontend-core/src/components/grid/overlays/KeyboardManager.svelte index 8b0a0f0942..5e3a035d89 100644 --- a/packages/frontend-core/src/components/grid/overlays/KeyboardManager.svelte +++ b/packages/frontend-core/src/components/grid/overlays/KeyboardManager.svelte @@ -217,14 +217,14 @@ const type = $focusedCellAPI.getType() if (type === "number" && keyCodeIsNumber(keyCode)) { // Update the value locally but don't save it yet - $focusedCellAPI.setValue(parseInt(key), { save: false }) + $focusedCellAPI.setValue(parseInt(key), { apply: false }) $focusedCellAPI.focus() } else if ( ["string", "barcodeqr", "longform"].includes(type) && (keyCodeIsLetter(keyCode) || keyCodeIsNumber(keyCode)) ) { // Update the value locally but don't save it yet - $focusedCellAPI.setValue(key, { save: false }) + $focusedCellAPI.setValue(key, { apply: false }) $focusedCellAPI.focus() } } diff --git a/packages/frontend-core/src/components/grid/stores/rows.js b/packages/frontend-core/src/components/grid/stores/rows.js index f4b0e97942..c8d27da2e7 100644 --- a/packages/frontend-core/src/components/grid/stores/rows.js +++ b/packages/frontend-core/src/components/grid/stores/rows.js @@ -3,6 +3,7 @@ import { fetchData } from "../../../fetch" import { NewRowID, RowPageSize } from "../lib/constants" import { tick } from "svelte" import { Helpers } from "@budibase/bbui" +import { isValid } from "@budibase/string-templates" export const createStores = () => { const rows = writable([]) @@ -327,38 +328,31 @@ export const createActions = context => { get(fetch)?.getInitialData() } - // Patches a row with some changes - const updateRow = async ( - rowId, - changes, - options = { save: true, force: false } - ) => { + // Checks if a changeset for a row actually mutates the row or not + const changesAreValid = (row, changes) => { + const columns = Object.keys(changes || {}) + if (!row || !columns.length) { + return false + } + + // Ensure there is at least 1 column that creates a difference + return columns.some(column => row[column] !== changes[column]) + } + + // Patches a row with some changes in local state, and returns whether a + // valid pending change was made or not + const stashRowChanges = (rowId, changes) => { const $rows = get(rows) const $rowLookupMap = get(rowLookupMap) const index = $rowLookupMap[rowId] const row = $rows[index] - if (index == null) { - return - } - if (!options?.force && !Object.keys(changes || {}).length) { - return + + // Check this is a valid change + if (!row || !changesAreValid(row, changes)) { + return false } - // Abandon if no changes - if (!options?.force) { - let same = true - for (let column of Object.keys(changes)) { - if (row[column] !== changes[column]) { - same = false - break - } - } - if (same) { - return - } - } - - // Immediately update state so that the change is reflected + // Add change to cache rowChangeCache.update(state => ({ ...state, [rowId]: { @@ -366,26 +360,30 @@ export const createActions = context => { ...changes, }, })) + return true + } - // Stop here if we don't want to persist the change - if (!options?.save && !options?.force) { + // Saves any pending changes to a row + const applyRowChanges = async rowId => { + const $rows = get(rows) + const $rowLookupMap = get(rowLookupMap) + const index = $rowLookupMap[rowId] + const row = $rows[index] + if (row == null) { return } // Save change try { - inProgressChanges.update(state => ({ - ...state, - [rowId]: true, - })) + // Mark as in progress + inProgressChanges.update(state => ({ ...state, [rowId]: true })) // Update row - const saved = await datasource.actions.updateRow({ - ...cleanRow(row), - ...get(rowChangeCache)[rowId], - }) + const changes = get(rowChangeCache)[rowId] + const newRow = { ...cleanRow(row), ...changes } + const saved = await datasource.actions.updateRow(newRow) - // Update state after a successful change + // Update row state after a successful change if (saved?._id) { rows.update(state => { state[index] = saved @@ -395,6 +393,8 @@ export const createActions = context => { // Handle users table edge case await refreshRow(saved.id) } + + // Wipe row change cache now that we've saved the row rowChangeCache.update(state => { delete state[rowId] return state @@ -402,15 +402,17 @@ export const createActions = context => { } catch (error) { handleValidationError(rowId, error) } - inProgressChanges.update(state => ({ - ...state, - [rowId]: false, - })) + + // Mark as completed + inProgressChanges.update(state => ({ ...state, [rowId]: false })) } // Updates a value of a row - const updateValue = async ({ rowId, column, value, save = true }) => { - return await updateRow(rowId, { [column]: value }, { save }) + const updateValue = async ({ rowId, column, value, apply = true }) => { + const success = stashRowChanges(rowId, { [column]: value }) + if (success && apply) { + await applyRowChanges(rowId) + } } // Deletes an array of rows @@ -420,9 +422,7 @@ export const createActions = context => { } // Actually delete rows - rowsToDelete.forEach(row => { - delete row.__idx - }) + rowsToDelete.forEach(row => delete row.__idx) await datasource.actions.deleteRows(rowsToDelete) // Update state @@ -442,7 +442,7 @@ export const createActions = context => { newRow = newRows[i] // Ensure we have a unique _id. - // This means generating one for non DS+, overriting any that may already + // This means generating one for non DS+, overwriting any that may already // exist as we cannot allow duplicates. if (!$isDatasourcePlus) { newRow._id = Helpers.uuid() @@ -503,7 +503,7 @@ export const createActions = context => { duplicateRow, getRow, updateValue, - updateRow, + applyRowChanges, deleteRows, hasRow, loadNextPage, @@ -537,13 +537,13 @@ export const initialise = context => { }) // Ensure any unsaved changes are saved when changing cell - previousFocusedCellId.subscribe(id => { + previousFocusedCellId.subscribe(async id => { const rowId = id?.split("-")[0] const hasErrors = validation.actions.rowHasErrors(rowId) const hasChanges = Object.keys(get(rowChangeCache)[rowId] || {}).length > 0 const isSavingChanges = get(inProgressChanges)[rowId] if (rowId && !hasErrors && hasChanges && !isSavingChanges) { - rows.actions.updateRow(rowId, null, { force: true }) + await rows.actions.applyRowChanges(rowId) } }) } From 6b306266b5e322e5f3728a8392e7c55d5e82cd47 Mon Sep 17 00:00:00 2001 From: Sam Rose Date: Thu, 29 Feb 2024 11:09:39 +0000 Subject: [PATCH 09/38] Only show stack traces if you ask for them. --- .../src/middleware/errorHandling.ts | 26 +++++++++---------- .../server/src/tests/utilities/api/base.ts | 6 ++++- 2 files changed, 17 insertions(+), 15 deletions(-) diff --git a/packages/backend-core/src/middleware/errorHandling.ts b/packages/backend-core/src/middleware/errorHandling.ts index ae5373a2e0..2b8f7195ed 100644 --- a/packages/backend-core/src/middleware/errorHandling.ts +++ b/packages/backend-core/src/middleware/errorHandling.ts @@ -15,21 +15,19 @@ export async function errorHandling(ctx: any, next: any) { console.error(err) } - if (environment.isTest()) { - ctx.body = { - message: err.message, - status: status, - error: errors.getPublicError(err), - stack: err.stack, - } - } else { - ctx.body = { - message: err.message, - status: status, - validationErrors: err.validation, - error: errors.getPublicError(err), - } + let error: APIError = { + message: err.message, + status: status, + validationErrors: err.validation, + error: errors.getPublicError(err), } + + if (environment.isTest() && ctx.headers["x-budibase-include-stacktrace"]) { + // @ts-ignore + error.stack = err.stack + } + + ctx.body = error } } diff --git a/packages/server/src/tests/utilities/api/base.ts b/packages/server/src/tests/utilities/api/base.ts index 42911628fd..19fc7ed1f5 100644 --- a/packages/server/src/tests/utilities/api/base.ts +++ b/packages/server/src/tests/utilities/api/base.ts @@ -96,7 +96,11 @@ export abstract class TestAPI { url += `?${queryParams.join("&")}` } - let request = this.request[method](url).set(this.config.defaultHeaders()) + let request = this.request[method](url).set( + this.config.defaultHeaders({ + "x-budibase-include-stacktrace": "true", + }) + ) if (headers) { request = request.set(headers) } From bc723c7094db65e19eabbaff012f126a5037c571 Mon Sep 17 00:00:00 2001 From: Andrew Kingston Date: Thu, 29 Feb 2024 12:25:21 +0000 Subject: [PATCH 10/38] Lint --- packages/frontend-core/src/components/grid/stores/rows.js | 1 - 1 file changed, 1 deletion(-) diff --git a/packages/frontend-core/src/components/grid/stores/rows.js b/packages/frontend-core/src/components/grid/stores/rows.js index c8d27da2e7..5dc9413ccd 100644 --- a/packages/frontend-core/src/components/grid/stores/rows.js +++ b/packages/frontend-core/src/components/grid/stores/rows.js @@ -3,7 +3,6 @@ import { fetchData } from "../../../fetch" import { NewRowID, RowPageSize } from "../lib/constants" import { tick } from "svelte" import { Helpers } from "@budibase/bbui" -import { isValid } from "@budibase/string-templates" export const createStores = () => { const rows = writable([]) From 5163434b08ce284a3171066d94de7b45ac76e98e Mon Sep 17 00:00:00 2001 From: Sam Rose Date: Thu, 29 Feb 2024 14:33:09 +0000 Subject: [PATCH 11/38] Convert BackupAPI. --- .../src/api/routes/tests/backup.spec.ts | 21 +++---- .../server/src/tests/utilities/api/backup.ts | 61 +++++++++---------- .../server/src/tests/utilities/api/base.ts | 35 ++++++++--- 3 files changed, 62 insertions(+), 55 deletions(-) diff --git a/packages/server/src/api/routes/tests/backup.spec.ts b/packages/server/src/api/routes/tests/backup.spec.ts index becbeb5480..c862106d58 100644 --- a/packages/server/src/api/routes/tests/backup.spec.ts +++ b/packages/server/src/api/routes/tests/backup.spec.ts @@ -19,11 +19,8 @@ describe("/backups", () => { describe("/api/backups/export", () => { it("should be able to export app", async () => { - const { body, headers } = await config.api.backup.exportBasicBackup( - config.getAppId()! - ) + const body = await config.api.backup.exportBasicBackup(config.getAppId()!) expect(body instanceof Buffer).toBe(true) - expect(headers["content-type"]).toEqual("application/gzip") expect(events.app.exported).toBeCalledTimes(1) }) @@ -38,15 +35,13 @@ describe("/backups", () => { it("should infer the app name from the app", async () => { tk.freeze(mocks.date.MOCK_DATE) - const { headers } = await config.api.backup.exportBasicBackup( - config.getAppId()! - ) - - expect(headers["content-disposition"]).toEqual( - `attachment; filename="${ - config.getApp().name - }-export-${mocks.date.MOCK_DATE.getTime()}.tar.gz"` - ) + await config.api.backup.exportBasicBackup(config.getAppId()!, { + headers: { + "content-disposition": `attachment; filename="${ + config.getApp().name + }-export-${mocks.date.MOCK_DATE.getTime()}.tar.gz"`, + }, + }) }) }) diff --git a/packages/server/src/tests/utilities/api/backup.ts b/packages/server/src/tests/utilities/api/backup.ts index 8cd1e58a29..7c01b57108 100644 --- a/packages/server/src/tests/utilities/api/backup.ts +++ b/packages/server/src/tests/utilities/api/backup.ts @@ -2,42 +2,38 @@ import { CreateAppBackupResponse, ImportAppBackupResponse, } from "@budibase/types" -import TestConfiguration from "../TestConfiguration" -import { TestAPI } from "./base" +import { Expectations, TestAPI } from "./base" export class BackupAPI extends TestAPI { - constructor(config: TestConfiguration) { - super(config) - } - - exportBasicBackup = async (appId: string) => { - const result = await this.request - .post(`/api/backups/export?appId=${appId}`) - .set(this.config.defaultHeaders()) - .expect("Content-Type", /application\/gzip/) - .expect(200) - return { - body: result.body as Buffer, - headers: result.headers, + exportBasicBackup = async (appId: string, expectations?: Expectations) => { + const exp = { + ...expectations, + headers: { + ...expectations?.headers, + "Content-Type": "application/gzip", + }, } + return await this._post(`/api/backups/export`, { + query: { appId }, + expectations: exp, + }) } - createBackup = async (appId: string) => { - const result = await this.request - .post(`/api/apps/${appId}/backups`) - .set(this.config.defaultHeaders()) - .expect("Content-Type", /json/) - .expect(200) - return result.body as CreateAppBackupResponse + createBackup = async (appId: string, expectations?: Expectations) => { + return await this._post( + `/api/apps/${appId}/backups`, + { expectations } + ) } waitForBackupToComplete = async (appId: string, backupId: string) => { for (let i = 0; i < 10; i++) { await new Promise(resolve => setTimeout(resolve, 1000)) - const result = await this.request - .get(`/api/apps/${appId}/backups/${backupId}/file`) - .set(this.config.defaultHeaders()) - if (result.status === 200) { + const response = await this._requestRaw( + "get", + `/api/apps/${appId}/backups/${backupId}/file` + ) + if (response.status === 200) { return } } @@ -46,13 +42,12 @@ export class BackupAPI extends TestAPI { importBackup = async ( appId: string, - backupId: string + backupId: string, + expectations?: Expectations ): Promise => { - const result = await this.request - .post(`/api/apps/${appId}/backups/${backupId}/import`) - .set(this.config.defaultHeaders()) - .expect("Content-Type", /json/) - .expect(200) - return result.body as ImportAppBackupResponse + return await this._post( + `/api/apps/${appId}/backups/${backupId}/import`, + { expectations } + ) } } diff --git a/packages/server/src/tests/utilities/api/base.ts b/packages/server/src/tests/utilities/api/base.ts index 19fc7ed1f5..b3e5733bb0 100644 --- a/packages/server/src/tests/utilities/api/base.ts +++ b/packages/server/src/tests/utilities/api/base.ts @@ -1,5 +1,5 @@ import TestConfiguration from "../TestConfiguration" -import { SuperTest, Test } from "supertest" +import { SuperTest, Test, Response } from "supertest" import { ReadStream } from "fs" type Headers = Record @@ -22,7 +22,6 @@ function isAttachedFile(file: any): file is AttachedFile { export interface Expectations { status?: number - contentType?: string | RegExp headers?: Record headersNotPresent?: string[] } @@ -71,11 +70,11 @@ export abstract class TestAPI { return await this._request("delete", url, opts) } - protected _request = async ( + protected _requestRaw = async ( method: "get" | "post" | "put" | "patch" | "delete", url: string, opts?: RequestOpts - ): Promise => { + ): Promise => { const { headers = {}, query = {}, @@ -84,7 +83,12 @@ export abstract class TestAPI { files = {}, expectations, } = opts || {} - const { status = 200, contentType = /json/ } = expectations || {} + const { status = 200 } = expectations || {} + const expectHeaders = expectations?.headers || {} + + if (status !== 204 && !expectHeaders["Content-Type"]) { + expectHeaders["Content-Type"] = "application/json" + } let queryParams = [] for (const [key, value] of Object.entries(query)) { @@ -118,16 +122,29 @@ export abstract class TestAPI { request = request.attach(key, value as any) } } - if (contentType && status !== 204) { - request = request.expect("Content-Type", contentType as any) - } if (expectations?.headers) { for (const [key, value] of Object.entries(expectations.headers)) { + if (value === undefined) { + throw new Error( + `Got an undefined expected value for header "${key}", if you want to check for the absence of a header, use headersNotPresent` + ) + } request = request.expect(key, value as any) } } - const response = await request + return await request + } + + protected _request = async ( + method: "get" | "post" | "put" | "patch" | "delete", + url: string, + opts?: RequestOpts + ): Promise => { + const { expectations } = opts || {} + const { status = 200 } = expectations || {} + + const response = await this._requestRaw(method, url, opts) if (response.status !== status) { let message = `Expected status ${status} but got ${response.status}` From 4fbe03bbda0f62d62d0527d4bd90d66ac777c49e Mon Sep 17 00:00:00 2001 From: Sam Rose Date: Thu, 29 Feb 2024 15:50:18 +0000 Subject: [PATCH 12/38] Migrate DatasourceAPI. --- .../routes/tests/queries/query.seq.spec.ts | 17 +++-- .../src/integration-test/postgres.spec.ts | 41 ++++++---- .../server/src/tests/utilities/api/base.ts | 15 +++- .../src/tests/utilities/api/datasource.ts | 75 ++++++++----------- packages/types/src/api/web/app/datasource.ts | 4 +- 5 files changed, 77 insertions(+), 75 deletions(-) diff --git a/packages/server/src/api/routes/tests/queries/query.seq.spec.ts b/packages/server/src/api/routes/tests/queries/query.seq.spec.ts index 2bbc8366ea..c5cb188cbc 100644 --- a/packages/server/src/api/routes/tests/queries/query.seq.spec.ts +++ b/packages/server/src/api/routes/tests/queries/query.seq.spec.ts @@ -397,15 +397,16 @@ describe("/queries", () => { }) it("should fail with invalid integration type", async () => { - const response = await config.api.datasource.create( - { - ...basicDatasource().datasource, - source: "INVALID_INTEGRATION" as SourceName, + const datasource: Datasource = { + ...basicDatasource().datasource, + source: "INVALID_INTEGRATION" as SourceName, + } + await config.api.datasource.create(datasource, { + status: 500, + body: { + message: "No datasource implementation found.", }, - { expectStatus: 500, rawResponse: true } - ) - - expect(response.body.message).toBe("No datasource implementation found.") + }) }) }) diff --git a/packages/server/src/integration-test/postgres.spec.ts b/packages/server/src/integration-test/postgres.spec.ts index 0031fe1136..ae6b66b0f5 100644 --- a/packages/server/src/integration-test/postgres.spec.ts +++ b/packages/server/src/integration-test/postgres.spec.ts @@ -1040,28 +1040,37 @@ describe("postgres integrations", () => { describe("POST /api/datasources/verify", () => { it("should be able to verify the connection", async () => { - const response = await config.api.datasource.verify({ - datasource: await databaseTestProviders.postgres.datasource(), - }) - expect(response.status).toBe(200) - expect(response.body.connected).toBe(true) + await config.api.datasource.verify( + { + datasource: await databaseTestProviders.postgres.datasource(), + }, + { + body: { + connected: true, + }, + } + ) }) it("should state an invalid datasource cannot connect", async () => { const dbConfig = await databaseTestProviders.postgres.datasource() - const response = await config.api.datasource.verify({ - datasource: { - ...dbConfig, - config: { - ...dbConfig.config, - password: "wrongpassword", + const response = await config.api.datasource.verify( + { + datasource: { + ...dbConfig, + config: { + ...dbConfig.config, + password: "wrongpassword", + }, }, }, - }) - - expect(response.status).toBe(200) - expect(response.body.connected).toBe(false) - expect(response.body.error).toBeDefined() + { + body: { + connected: false, + error: 'password authentication failed for user "postgres"', + }, + } + ) }) }) diff --git a/packages/server/src/tests/utilities/api/base.ts b/packages/server/src/tests/utilities/api/base.ts index b3e5733bb0..c79686980b 100644 --- a/packages/server/src/tests/utilities/api/base.ts +++ b/packages/server/src/tests/utilities/api/base.ts @@ -3,6 +3,10 @@ import { SuperTest, Test, Response } from "supertest" import { ReadStream } from "fs" type Headers = Record +type SuccessStatus = 200 | 201 | 204 +type ErrorStatus = 400 | 401 | 403 | 404 | 500 | 502 | 503 | 504 +type Status = SuccessStatus | ErrorStatus +type Method = "get" | "post" | "put" | "patch" | "delete" export interface AttachedFile { name: string @@ -21,9 +25,10 @@ function isAttachedFile(file: any): file is AttachedFile { } export interface Expectations { - status?: number + status?: Status headers?: Record headersNotPresent?: string[] + body?: Record } export interface RequestOpts { @@ -137,7 +142,7 @@ export abstract class TestAPI { } protected _request = async ( - method: "get" | "post" | "put" | "patch" | "delete", + method: Method, url: string, opts?: RequestOpts ): Promise => { @@ -180,6 +185,10 @@ export abstract class TestAPI { } } - return response.body as T + if (expectations?.body) { + expect(response.body).toMatchObject(expectations.body) + } + + return response.body } } diff --git a/packages/server/src/tests/utilities/api/datasource.ts b/packages/server/src/tests/utilities/api/datasource.ts index bcd7a71089..06aa9b4e1e 100644 --- a/packages/server/src/tests/utilities/api/datasource.ts +++ b/packages/server/src/tests/utilities/api/datasource.ts @@ -1,63 +1,48 @@ import { - CreateDatasourceRequest, Datasource, VerifyDatasourceRequest, + CreateDatasourceResponse, + UpdateDatasourceResponse, + UpdateDatasourceRequest, } from "@budibase/types" -import TestConfiguration from "../TestConfiguration" -import { TestAPI } from "./base" -import supertest from "supertest" +import { Expectations, TestAPI } from "./base" export class DatasourceAPI extends TestAPI { - constructor(config: TestConfiguration) { - super(config) - } - - create = async ( + create = async ( config: Datasource, - { - expectStatus, - rawResponse, - }: { expectStatus?: number; rawResponse?: B } = {} - ): Promise => { - const body: CreateDatasourceRequest = { - datasource: config, - tablesFilter: [], - } - const result = await this.request - .post(`/api/datasources`) - .send(body) - .set(this.config.defaultHeaders()) - .expect("Content-Type", /json/) - .expect(expectStatus || 200) - if (rawResponse) { - return result as any - } - return result.body.datasource + expectations?: Expectations + ): Promise => { + const response = await this._post( + `/api/datasources`, + { + body: { + datasource: config, + tablesFilter: [], + }, + expectations, + } + ) + return response.datasource } update = async ( - datasource: Datasource, - { expectStatus } = { expectStatus: 200 } + datasource: UpdateDatasourceRequest, + expectations?: Expectations ): Promise => { - const result = await this.request - .put(`/api/datasources/${datasource._id}`) - .send(datasource) - .set(this.config.defaultHeaders()) - .expect("Content-Type", /json/) - .expect(expectStatus) - return result.body.datasource as Datasource + const response = await this._put( + `/api/datasources/${datasource._id}`, + { body: datasource, expectations } + ) + return response.datasource } verify = async ( data: VerifyDatasourceRequest, - { expectStatus } = { expectStatus: 200 } + expectations?: Expectations ) => { - const result = await this.request - .post(`/api/datasources/verify`) - .send(data) - .set(this.config.defaultHeaders()) - .expect("Content-Type", /json/) - .expect(expectStatus) - return result + return await this._post(`/api/datasources/verify`, { + body: data, + expectations, + }) } } diff --git a/packages/types/src/api/web/app/datasource.ts b/packages/types/src/api/web/app/datasource.ts index 4a3d07a952..08dcd00c80 100644 --- a/packages/types/src/api/web/app/datasource.ts +++ b/packages/types/src/api/web/app/datasource.ts @@ -32,9 +32,7 @@ export interface FetchDatasourceInfoResponse { tableNames: string[] } -export interface UpdateDatasourceRequest extends Datasource { - datasource: Datasource -} +export type UpdateDatasourceRequest = Datasource export interface BuildSchemaFromSourceRequest { tablesFilter?: string[] From 1a2a77fc914f27ed25ed6b4663416c80ecf49319 Mon Sep 17 00:00:00 2001 From: Sam Rose Date: Thu, 29 Feb 2024 15:59:03 +0000 Subject: [PATCH 13/38] Migrate LegacyViewAPI --- packages/server/src/api/routes/tests/row.spec.ts | 14 +++++++------- .../server/src/integration-test/postgres.spec.ts | 2 +- .../server/src/tests/utilities/api/legacyView.ts | 16 ++++------------ 3 files changed, 12 insertions(+), 20 deletions(-) diff --git a/packages/server/src/api/routes/tests/row.spec.ts b/packages/server/src/api/routes/tests/row.spec.ts index 726e493b2d..2e1b1c47b9 100644 --- a/packages/server/src/api/routes/tests/row.spec.ts +++ b/packages/server/src/api/routes/tests/row.spec.ts @@ -757,16 +757,16 @@ describe.each([ const row = await config.createRow() const rowUsage = await getRowUsage() - const res = await config.api.legacyView.get(table._id!) - expect(res.body.length).toEqual(1) - expect(res.body[0]._id).toEqual(row._id) + const rows = await config.api.legacyView.get(table._id!) + expect(rows.length).toEqual(1) + expect(rows[0]._id).toEqual(row._id) await assertRowUsage(rowUsage) }) it("should throw an error if view doesn't exist", async () => { const rowUsage = await getRowUsage() - await config.api.legacyView.get("derp", { expectStatus: 404 }) + await config.api.legacyView.get("derp", { status: 404 }) await assertRowUsage(rowUsage) }) @@ -781,9 +781,9 @@ describe.each([ const row = await config.createRow() const rowUsage = await getRowUsage() - const res = await config.api.legacyView.get(view.name) - expect(res.body.length).toEqual(1) - expect(res.body[0]._id).toEqual(row._id) + const rows = await config.api.legacyView.get(view.name) + expect(rows.length).toEqual(1) + expect(rows[0]._id).toEqual(row._id) await assertRowUsage(rowUsage) }) diff --git a/packages/server/src/integration-test/postgres.spec.ts b/packages/server/src/integration-test/postgres.spec.ts index ae6b66b0f5..2680d1a11b 100644 --- a/packages/server/src/integration-test/postgres.spec.ts +++ b/packages/server/src/integration-test/postgres.spec.ts @@ -1054,7 +1054,7 @@ describe("postgres integrations", () => { it("should state an invalid datasource cannot connect", async () => { const dbConfig = await databaseTestProviders.postgres.datasource() - const response = await config.api.datasource.verify( + await config.api.datasource.verify( { datasource: { ...dbConfig, diff --git a/packages/server/src/tests/utilities/api/legacyView.ts b/packages/server/src/tests/utilities/api/legacyView.ts index 63981cec5e..38ef70d62a 100644 --- a/packages/server/src/tests/utilities/api/legacyView.ts +++ b/packages/server/src/tests/utilities/api/legacyView.ts @@ -1,16 +1,8 @@ -import TestConfiguration from "../TestConfiguration" -import { TestAPI } from "./base" +import { Expectations, TestAPI } from "./base" +import { Row } from "@budibase/types" export class LegacyViewAPI extends TestAPI { - constructor(config: TestConfiguration) { - super(config) - } - - get = async (id: string, { expectStatus } = { expectStatus: 200 }) => { - return await this.request - .get(`/api/views/${id}`) - .set(this.config.defaultHeaders()) - .expect("Content-Type", /json/) - .expect(expectStatus) + get = async (id: string, expectations?: Expectations) => { + return await this._get(`/api/views/${id}`, { expectations }) } } From 46bec3c515293337a75601d7ed961af2c9b3784a Mon Sep 17 00:00:00 2001 From: Sam Rose Date: Thu, 29 Feb 2024 17:33:36 +0000 Subject: [PATCH 14/38] Migrate PermissionAPI --- .../server/src/api/controllers/permission.ts | 39 ++++++---- .../src/api/routes/tests/permissions.spec.ts | 40 ++++++----- .../server/src/api/routes/tests/row.spec.ts | 8 +-- .../src/tests/utilities/api/permission.ts | 71 ++++++++----------- packages/types/src/api/web/app/permission.ts | 20 +++++- 5 files changed, 99 insertions(+), 79 deletions(-) diff --git a/packages/server/src/api/controllers/permission.ts b/packages/server/src/api/controllers/permission.ts index e2bd6c40e5..e12bf3655d 100644 --- a/packages/server/src/api/controllers/permission.ts +++ b/packages/server/src/api/controllers/permission.ts @@ -7,6 +7,10 @@ import { GetResourcePermsResponse, ResourcePermissionInfo, GetDependantResourcesResponse, + AddPermissionResponse, + AddPermissionRequest, + RemovePermissionRequest, + RemovePermissionResponse, } from "@budibase/types" import { getRoleParams } from "../../db/utils" import { @@ -16,9 +20,9 @@ import { import { removeFromArray } from "../../utilities" import sdk from "../../sdk" -const PermissionUpdateType = { - REMOVE: "remove", - ADD: "add", +enum PermissionUpdateType { + REMOVE = "remove", + ADD = "add", } const SUPPORTED_LEVELS = CURRENTLY_SUPPORTED_LEVELS @@ -39,7 +43,7 @@ async function updatePermissionOnRole( resourceId, level, }: { roleId: string; resourceId: string; level: PermissionLevel }, - updateType: string + updateType: PermissionUpdateType ) { const allowedAction = await sdk.permissions.resourceActionAllowed({ resourceId, @@ -107,11 +111,15 @@ async function updatePermissionOnRole( } const response = await db.bulkDocs(docUpdates) - return response.map((resp: any) => { + return response.map(resp => { const version = docUpdates.find(role => role._id === resp.id)?.version - resp._id = roles.getExternalRoleID(resp.id, version) - delete resp.id - return resp + const _id = roles.getExternalRoleID(resp.id, version) + return { + _id, + rev: resp.rev, + error: resp.error, + reason: resp.reason, + } }) } @@ -189,13 +197,14 @@ export async function getDependantResources( } } -export async function addPermission(ctx: UserCtx) { - ctx.body = await updatePermissionOnRole(ctx.params, PermissionUpdateType.ADD) +export async function addPermission(ctx: UserCtx) { + const params: AddPermissionRequest = ctx.params + ctx.body = await updatePermissionOnRole(params, PermissionUpdateType.ADD) } -export async function removePermission(ctx: UserCtx) { - ctx.body = await updatePermissionOnRole( - ctx.params, - PermissionUpdateType.REMOVE - ) +export async function removePermission( + ctx: UserCtx +) { + const params: RemovePermissionRequest = ctx.params + ctx.body = await updatePermissionOnRole(params, PermissionUpdateType.REMOVE) } diff --git a/packages/server/src/api/routes/tests/permissions.spec.ts b/packages/server/src/api/routes/tests/permissions.spec.ts index 129bc00b44..b9c9ba0359 100644 --- a/packages/server/src/api/routes/tests/permissions.spec.ts +++ b/packages/server/src/api/routes/tests/permissions.spec.ts @@ -45,7 +45,7 @@ describe("/permission", () => { table = (await config.createTable()) as typeof table row = await config.createRow() view = await config.api.viewV2.create({ tableId: table._id }) - perms = await config.api.permission.set({ + perms = await config.api.permission.add({ roleId: STD_ROLE_ID, resourceId: table._id, level: PermissionLevel.READ, @@ -88,13 +88,13 @@ describe("/permission", () => { }) it("should get resource permissions with multiple roles", async () => { - perms = await config.api.permission.set({ + perms = await config.api.permission.add({ roleId: HIGHER_ROLE_ID, resourceId: table._id, level: PermissionLevel.WRITE, }) const res = await config.api.permission.get(table._id) - expect(res.body).toEqual({ + expect(res).toEqual({ permissions: { read: { permissionType: "EXPLICIT", role: STD_ROLE_ID }, write: { permissionType: "EXPLICIT", role: HIGHER_ROLE_ID }, @@ -117,16 +117,19 @@ describe("/permission", () => { level: PermissionLevel.READ, }) - const response = await config.api.permission.set( + await config.api.permission.add( { roleId: STD_ROLE_ID, resourceId: table._id, level: PermissionLevel.EXECUTE, }, - { expectStatus: 403 } - ) - expect(response.message).toEqual( - "You are not allowed to 'read' the resource type 'datasource'" + { + status: 403, + body: { + message: + "You are not allowed to 'read' the resource type 'datasource'", + }, + } ) }) }) @@ -138,9 +141,9 @@ describe("/permission", () => { resourceId: table._id, level: PermissionLevel.READ, }) - expect(res.body[0]._id).toEqual(STD_ROLE_ID) + expect(res[0]._id).toEqual(STD_ROLE_ID) const permsRes = await config.api.permission.get(table._id) - expect(permsRes.body[STD_ROLE_ID]).toBeUndefined() + expect(permsRes.permissions[STD_ROLE_ID]).toBeUndefined() }) it("throw forbidden if the action is not allowed for the resource", async () => { @@ -156,10 +159,13 @@ describe("/permission", () => { resourceId: table._id, level: PermissionLevel.EXECUTE, }, - { expectStatus: 403 } - ) - expect(response.body.message).toEqual( - "You are not allowed to 'read' the resource type 'datasource'" + { + status: 403, + body: { + message: + "You are not allowed to 'read' the resource type 'datasource'", + }, + } ) }) }) @@ -203,7 +209,7 @@ describe("/permission", () => { }) it("should ignore the view permissions if the flag is not on", async () => { - await config.api.permission.set({ + await config.api.permission.add({ roleId: STD_ROLE_ID, resourceId: view.id, level: PermissionLevel.READ, @@ -224,7 +230,7 @@ describe("/permission", () => { it("should use the view permissions if the flag is on", async () => { mocks.licenses.useViewPermissions() - await config.api.permission.set({ + await config.api.permission.add({ roleId: STD_ROLE_ID, resourceId: view.id, level: PermissionLevel.READ, @@ -277,7 +283,7 @@ describe("/permission", () => { const res = await config.api.permission.get(legacyView.name) - expect(res.body).toEqual({ + expect(res).toEqual({ permissions: { read: { permissionType: "BASE", diff --git a/packages/server/src/api/routes/tests/row.spec.ts b/packages/server/src/api/routes/tests/row.spec.ts index 2e1b1c47b9..7423157678 100644 --- a/packages/server/src/api/routes/tests/row.spec.ts +++ b/packages/server/src/api/routes/tests/row.spec.ts @@ -1523,7 +1523,7 @@ describe.each([ }) it("allow public users to fetch when permissions are explicit", async () => { - await config.api.permission.set({ + await config.api.permission.add({ roleId: roles.BUILTIN_ROLE_IDS.PUBLIC, level: PermissionLevel.READ, resourceId: viewId, @@ -1538,7 +1538,7 @@ describe.each([ }) it("allow public users to fetch when permissions are inherited", async () => { - await config.api.permission.set({ + await config.api.permission.add({ roleId: roles.BUILTIN_ROLE_IDS.PUBLIC, level: PermissionLevel.READ, resourceId: tableId, @@ -1553,12 +1553,12 @@ describe.each([ }) it("respects inherited permissions, not allowing not public views from public tables", async () => { - await config.api.permission.set({ + await config.api.permission.add({ roleId: roles.BUILTIN_ROLE_IDS.PUBLIC, level: PermissionLevel.READ, resourceId: tableId, }) - await config.api.permission.set({ + await config.api.permission.add({ roleId: roles.BUILTIN_ROLE_IDS.POWER, level: PermissionLevel.READ, resourceId: viewId, diff --git a/packages/server/src/tests/utilities/api/permission.ts b/packages/server/src/tests/utilities/api/permission.ts index ffa89e88f9..986796d9a1 100644 --- a/packages/server/src/tests/utilities/api/permission.ts +++ b/packages/server/src/tests/utilities/api/permission.ts @@ -1,52 +1,39 @@ -import { AnyDocument, PermissionLevel } from "@budibase/types" -import TestConfiguration from "../TestConfiguration" -import { TestAPI } from "./base" +import { + AddPermissionRequest, + AddPermissionResponse, + GetResourcePermsResponse, + RemovePermissionRequest, + RemovePermissionResponse, +} from "@budibase/types" +import { Expectations, TestAPI } from "./base" export class PermissionAPI extends TestAPI { - constructor(config: TestConfiguration) { - super(config) + get = async (resourceId: string, expectations?: Expectations) => { + return await this._get( + `/api/permission/${resourceId}`, + { expectations } + ) } - get = async ( - resourceId: string, - { expectStatus } = { expectStatus: 200 } - ) => { - return this.request - .get(`/api/permission/${resourceId}`) - .set(this.config.defaultHeaders()) - .expect("Content-Type", /json/) - .expect(expectStatus) - } - - set = async ( - { - roleId, - resourceId, - level, - }: { roleId: string; resourceId: string; level: PermissionLevel }, - { expectStatus } = { expectStatus: 200 } - ): Promise => { - const res = await this.request - .post(`/api/permission/${roleId}/${resourceId}/${level}`) - .set(this.config.defaultHeaders()) - .expect("Content-Type", /json/) - .expect(expectStatus) - return res.body + add = async ( + request: AddPermissionRequest, + expectations?: Expectations + ): Promise => { + const { roleId, resourceId, level } = request + return await this._post( + `/api/permission/${roleId}/${resourceId}/${level}`, + { expectations } + ) } revoke = async ( - { - roleId, - resourceId, - level, - }: { roleId: string; resourceId: string; level: PermissionLevel }, - { expectStatus } = { expectStatus: 200 } + request: RemovePermissionRequest, + expectations?: Expectations ) => { - const res = await this.request - .delete(`/api/permission/${roleId}/${resourceId}/${level}`) - .set(this.config.defaultHeaders()) - .expect("Content-Type", /json/) - .expect(expectStatus) - return res + const { roleId, resourceId, level } = request + return await this._delete( + `/api/permission/${roleId}/${resourceId}/${level}`, + { expectations } + ) } } diff --git a/packages/types/src/api/web/app/permission.ts b/packages/types/src/api/web/app/permission.ts index a8ab0e8084..ebe7a8bea3 100644 --- a/packages/types/src/api/web/app/permission.ts +++ b/packages/types/src/api/web/app/permission.ts @@ -1,4 +1,4 @@ -import { PlanType } from "../../../sdk" +import { PermissionLevel, PlanType } from "../../../sdk" export interface ResourcePermissionInfo { role: string @@ -14,3 +14,21 @@ export interface GetResourcePermsResponse { export interface GetDependantResourcesResponse { resourceByType?: Record } + +export interface AddedPermission { + _id: string + rev?: string + error?: string + reason?: string +} + +export type AddPermissionResponse = AddedPermission[] + +export interface AddPermissionRequest { + roleId: string + resourceId: string + level: PermissionLevel +} + +export interface RemovePermissionRequest extends AddPermissionRequest {} +export interface RemovePermissionResponse extends AddPermissionResponse {} From f5f81a5fb01496b627b7207959fb9044436b298d Mon Sep 17 00:00:00 2001 From: Sam Rose Date: Fri, 1 Mar 2024 13:59:55 +0000 Subject: [PATCH 15/38] Fix tests. --- packages/server/src/api/routes/tests/role.spec.js | 2 +- packages/server/src/tests/utilities/api/base.ts | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/packages/server/src/api/routes/tests/role.spec.js b/packages/server/src/api/routes/tests/role.spec.js index a653b573b2..4575f9b213 100644 --- a/packages/server/src/api/routes/tests/role.spec.js +++ b/packages/server/src/api/routes/tests/role.spec.js @@ -93,7 +93,7 @@ describe("/roles", () => { it("should be able to get the role with a permission added", async () => { const table = await config.createTable() - await config.api.permission.set({ + await config.api.permission.add({ roleId: BUILTIN_ROLE_IDS.POWER, resourceId: table._id, level: PermissionLevel.READ, diff --git a/packages/server/src/tests/utilities/api/base.ts b/packages/server/src/tests/utilities/api/base.ts index c79686980b..df37c62f00 100644 --- a/packages/server/src/tests/utilities/api/base.ts +++ b/packages/server/src/tests/utilities/api/base.ts @@ -92,7 +92,7 @@ export abstract class TestAPI { const expectHeaders = expectations?.headers || {} if (status !== 204 && !expectHeaders["Content-Type"]) { - expectHeaders["Content-Type"] = "application/json" + expectHeaders["Content-Type"] = /^application\/json/ } let queryParams = [] From 16e9c5ff4e8f0a72c76c4246c704de0b1815e572 Mon Sep 17 00:00:00 2001 From: Sam Rose Date: Fri, 1 Mar 2024 14:33:18 +0000 Subject: [PATCH 16/38] Migrate QueryAPI --- .../server/src/api/controllers/query/index.ts | 24 ++++---- .../server/src/tests/utilities/api/query.ts | 55 ++++--------------- packages/types/src/api/web/index.ts | 1 + packages/types/src/api/web/query.ts | 20 +++++++ packages/types/src/documents/app/query.ts | 16 ------ 5 files changed, 46 insertions(+), 70 deletions(-) create mode 100644 packages/types/src/api/web/query.ts diff --git a/packages/server/src/api/controllers/query/index.ts b/packages/server/src/api/controllers/query/index.ts index 973718ba48..725de41c9a 100644 --- a/packages/server/src/api/controllers/query/index.ts +++ b/packages/server/src/api/controllers/query/index.ts @@ -17,10 +17,12 @@ import { QueryPreview, QuerySchema, FieldType, - type ExecuteQueryRequest, - type ExecuteQueryResponse, - type Row, + ExecuteQueryRequest, + ExecuteQueryResponse, + Row, QueryParameter, + PreviewQueryRequest, + PreviewQueryResponse, } from "@budibase/types" import { ValidQueryNameRegex, utils as JsonUtils } from "@budibase/shared-core" @@ -134,14 +136,16 @@ function enrichParameters( return requestParameters } -export async function preview(ctx: UserCtx) { +export async function preview( + ctx: UserCtx +) { const { datasource, envVars } = await sdk.datasources.getWithEnvVars( ctx.request.body.datasourceId ) - const query: QueryPreview = ctx.request.body // preview may not have a queryId as it hasn't been saved, but if it does // this stops dynamic variables from calling the same query - const { fields, parameters, queryVerb, transformer, queryId, schema } = query + const { fields, parameters, queryVerb, transformer, queryId, schema } = + ctx.request.body let existingSchema = schema if (queryId && !existingSchema) { @@ -266,9 +270,7 @@ export async function preview(ctx: UserCtx) { }, } - const { rows, keys, info, extra } = (await Runner.run( - inputs - )) as QueryResponse + const { rows, keys, info, extra } = await Runner.run(inputs) const { previewSchema, nestedSchemaFields } = getSchemaFields(rows, keys) // if existing schema, update to include any previous schema keys @@ -281,7 +283,7 @@ export async function preview(ctx: UserCtx) { } // remove configuration before sending event delete datasource.config - await events.query.previewed(datasource, query) + await events.query.previewed(datasource, ctx.request.body) ctx.body = { rows, nestedSchemaFields, @@ -295,7 +297,7 @@ export async function preview(ctx: UserCtx) { } async function execute( - ctx: UserCtx, + ctx: UserCtx, opts: any = { rowsOnly: false, isAutomation: false } ) { const db = context.getAppDB() diff --git a/packages/server/src/tests/utilities/api/query.ts b/packages/server/src/tests/utilities/api/query.ts index b0eac5c8b7..4b6e99fd6c 100644 --- a/packages/server/src/tests/utilities/api/query.ts +++ b/packages/server/src/tests/utilities/api/query.ts @@ -1,60 +1,29 @@ -import TestConfiguration from "../TestConfiguration" import { Query, - QueryPreview, - type ExecuteQueryRequest, - type ExecuteQueryResponse, + ExecuteQueryRequest, + ExecuteQueryResponse, + PreviewQueryRequest, + PreviewQueryResponse, } from "@budibase/types" import { TestAPI } from "./base" export class QueryAPI extends TestAPI { - constructor(config: TestConfiguration) { - super(config) - } - create = async (body: Query): Promise => { - const res = await this.request - .post(`/api/queries`) - .set(this.config.defaultHeaders()) - .send(body) - .expect("Content-Type", /json/) - - if (res.status !== 200) { - throw new Error(JSON.stringify(res.body)) - } - - return res.body as Query + return await this._post(`/api/queries`, { body }) } execute = async ( queryId: string, body?: ExecuteQueryRequest ): Promise => { - const res = await this.request - .post(`/api/v2/queries/${queryId}`) - .set(this.config.defaultHeaders()) - .send(body) - .expect("Content-Type", /json/) - - if (res.status !== 200) { - throw new Error(JSON.stringify(res.body)) - } - - return res.body + return await this._post(`/api/queries/${queryId}`, { + body, + }) } - previewQuery = async (queryPreview: QueryPreview) => { - const res = await this.request - .post(`/api/queries/preview`) - .send(queryPreview) - .set(this.config.defaultHeaders()) - .expect("Content-Type", /json/) - .expect(200) - - if (res.status !== 200) { - throw new Error(JSON.stringify(res.body)) - } - - return res.body + previewQuery = async (queryPreview: PreviewQueryRequest) => { + return await this._post(`/api/queries/preview`, { + body: queryPreview, + }) } } diff --git a/packages/types/src/api/web/index.ts b/packages/types/src/api/web/index.ts index 62d8ce8280..9a688a17a5 100644 --- a/packages/types/src/api/web/index.ts +++ b/packages/types/src/api/web/index.ts @@ -13,3 +13,4 @@ export * from "./searchFilter" export * from "./cookies" export * from "./automation" export * from "./layout" +export * from "./query" diff --git a/packages/types/src/api/web/query.ts b/packages/types/src/api/web/query.ts new file mode 100644 index 0000000000..40f4426d3e --- /dev/null +++ b/packages/types/src/api/web/query.ts @@ -0,0 +1,20 @@ +import { QueryPreview, QuerySchema } from "../../documents" + +export interface PreviewQueryRequest extends QueryPreview {} + +export interface PreviewQueryResponse { + rows: any[] + nestedSchemaFields: { [key: string]: { [key: string]: string | QuerySchema } } + schema: { [key: string]: string | QuerySchema } + info: any + extra: any +} + +export interface ExecuteQueryRequest { + parameters?: { [key: string]: string } + pagination?: any +} + +export interface ExecuteQueryResponse { + data: any[] +} diff --git a/packages/types/src/documents/app/query.ts b/packages/types/src/documents/app/query.ts index b1b0a1d780..3227666bf3 100644 --- a/packages/types/src/documents/app/query.ts +++ b/packages/types/src/documents/app/query.ts @@ -62,22 +62,6 @@ export interface PaginationValues { limit: number | null } -export interface PreviewQueryRequest extends Omit { - parameters: {} - flags?: { - urlName?: boolean - } -} - -export interface ExecuteQueryRequest { - parameters?: { [key: string]: string } - pagination?: any -} - -export interface ExecuteQueryResponse { - data: Row[] -} - export enum HttpMethod { GET = "GET", POST = "POST", From 5d42804020c3e8ea8b7722ebc689f4ef2c0ec92f Mon Sep 17 00:00:00 2001 From: Sam Rose Date: Fri, 1 Mar 2024 15:20:07 +0000 Subject: [PATCH 17/38] Migrate some of RowAPI, WIP --- .../server/src/api/controllers/row/index.ts | 2 +- .../server/src/api/routes/tests/row.spec.ts | 75 +++++++++---------- .../server/src/api/routes/tests/table.spec.ts | 15 ++-- .../src/automations/tests/createRow.spec.ts | 2 +- .../src/automations/tests/updateRow.spec.ts | 18 ++--- .../src/db/tests/linkController.spec.ts | 2 +- .../src/integration-test/postgres.spec.ts | 2 +- .../src/sdk/app/rows/tests/internal.spec.ts | 10 ++- .../src/tests/utilities/TestConfiguration.ts | 5 -- .../server/src/tests/utilities/api/base.ts | 5 +- .../server/src/tests/utilities/api/query.ts | 9 ++- .../server/src/tests/utilities/api/row.ts | 33 +++----- 12 files changed, 78 insertions(+), 100 deletions(-) diff --git a/packages/server/src/api/controllers/row/index.ts b/packages/server/src/api/controllers/row/index.ts index ec56919d12..54c294c42b 100644 --- a/packages/server/src/api/controllers/row/index.ts +++ b/packages/server/src/api/controllers/row/index.ts @@ -211,7 +211,7 @@ export async function validate(ctx: Ctx) { } } -export async function fetchEnrichedRow(ctx: any) { +export async function fetchEnrichedRow(ctx: UserCtx) { const tableId = utils.getTableId(ctx) ctx.body = await pickApi(tableId).fetchEnrichedRow(ctx) } diff --git a/packages/server/src/api/routes/tests/row.spec.ts b/packages/server/src/api/routes/tests/row.spec.ts index 7423157678..38f28cc138 100644 --- a/packages/server/src/api/routes/tests/row.spec.ts +++ b/packages/server/src/api/routes/tests/row.spec.ts @@ -106,9 +106,6 @@ describe.each([ mocks.licenses.useCloudFree() }) - const loadRow = (id: string, tbl_Id: string, status = 200) => - config.api.row.get(tbl_Id, id, { expectStatus: status }) - const getRowUsage = async () => { const { total } = await config.doInContext(undefined, () => quotas.getCurrentUsageValues(QuotaUsageType.STATIC, StaticQuotaName.ROWS) @@ -235,7 +232,7 @@ describe.each([ const res = await config.api.row.get(tableId, existing._id!) - expect(res.body).toEqual({ + expect(res).toEqual({ ...existing, ...defaultRowFields, }) @@ -265,7 +262,7 @@ describe.each([ await config.createRow() await config.api.row.get(tableId, "1234567", { - expectStatus: 404, + status: 404, }) }) @@ -395,7 +392,7 @@ describe.each([ const createdRow = await config.createRow(row) const id = createdRow._id! - const saved = (await loadRow(id, table._id!)).body + const saved = await config.api.row.get(id, table._id!) expect(saved.stringUndefined).toBe(undefined) expect(saved.stringNull).toBe(null) @@ -476,8 +473,8 @@ describe.each([ ) const row = await config.api.row.get(table._id!, createRowResponse._id!) - expect(row.body.Story).toBeUndefined() - expect(row.body).toEqual({ + expect(row.Story).toBeUndefined() + expect(row).toEqual({ ...defaultRowFields, OrderID: 1111, Country: "Aussy", @@ -524,10 +521,10 @@ describe.each([ expect(row.name).toEqual("Updated Name") expect(row.description).toEqual(existing.description) - const savedRow = await loadRow(row._id!, table._id!) + const savedRow = await config.api.row.get(row._id!, table._id!) - expect(savedRow.body.description).toEqual(existing.description) - expect(savedRow.body.name).toEqual("Updated Name") + expect(savedRow.description).toEqual(existing.description) + expect(savedRow.name).toEqual("Updated Name") await assertRowUsage(rowUsage) }) @@ -582,8 +579,8 @@ describe.each([ }) let getResp = await config.api.row.get(table._id!, row._id!) - expect(getResp.body.user1[0]._id).toEqual(user1._id) - expect(getResp.body.user2[0]._id).toEqual(user2._id) + expect(getResp.user1[0]._id).toEqual(user1._id) + expect(getResp.user2[0]._id).toEqual(user2._id) let patchResp = await config.api.row.patch(table._id!, { _id: row._id!, @@ -595,8 +592,8 @@ describe.each([ expect(patchResp.user2[0]._id).toEqual(user2._id) getResp = await config.api.row.get(table._id!, row._id!) - expect(getResp.body.user1[0]._id).toEqual(user2._id) - expect(getResp.body.user2[0]._id).toEqual(user2._id) + expect(getResp.user1[0]._id).toEqual(user2._id) + expect(getResp.user2[0]._id).toEqual(user2._id) }) it("should be able to update relationships when both columns are same name", async () => { @@ -609,7 +606,7 @@ describe.each([ description: "test", relationship: [row._id], }) - row = (await config.api.row.get(table._id!, row._id!)).body + row = await config.api.row.get(table._id!, row._id!) expect(row.relationship.length).toBe(1) const resp = await config.api.row.patch(table._id!, { _id: row._id!, @@ -685,7 +682,7 @@ describe.each([ const res = await config.api.row.delete(table._id!, [row1, row2]) expect(res.body.length).toEqual(2) - await loadRow(row1._id!, table._id!, 404) + await config.api.row.get(row1._id!, table._id!, { status: 404 }) await assertRowUsage(rowUsage - 2) }) @@ -704,7 +701,7 @@ describe.each([ ]) expect(res.body.length).toEqual(3) - await loadRow(row1._id!, table._id!, 404) + await config.api.row.get(row1._id!, table._id!, { status: 404 }) await assertRowUsage(rowUsage - 3) }) @@ -715,7 +712,7 @@ describe.each([ const res = await config.api.row.delete(table._id!, row1) expect(res.body.id).toEqual(row1._id) - await loadRow(row1._id!, table._id!, 404) + await config.api.row.get(row1._id!, table._id!, { status: 404 }) await assertRowUsage(rowUsage - 1) }) @@ -841,8 +838,8 @@ describe.each([ linkedTable._id!, secondRow._id! ) - expect(resBasic.body.link.length).toBe(1) - expect(resBasic.body.link[0]).toEqual({ + expect(resBasic.link.length).toBe(1) + expect(resBasic.link[0]).toEqual({ _id: firstRow._id, primaryDisplay: firstRow.name, }) @@ -852,10 +849,10 @@ describe.each([ linkedTable._id!, secondRow._id! ) - expect(resEnriched.body.link.length).toBe(1) - expect(resEnriched.body.link[0]._id).toBe(firstRow._id) - expect(resEnriched.body.link[0].name).toBe("Test Contact") - expect(resEnriched.body.link[0].description).toBe("original description") + expect(resEnriched.link.length).toBe(1) + expect(resEnriched.link[0]._id).toBe(firstRow._id) + expect(resEnriched.link[0].name).toBe("Test Contact") + expect(resEnriched.link[0].description).toBe("original description") await assertRowUsage(rowUsage) }) }) @@ -1000,7 +997,7 @@ describe.each([ }) const row = await config.api.row.get(table._id!, newRow._id!) - expect(row.body).toEqual({ + expect(row).toEqual({ name: data.name, surname: data.surname, address: data.address, @@ -1010,9 +1007,9 @@ describe.each([ id: newRow.id, ...defaultRowFields, }) - expect(row.body._viewId).toBeUndefined() - expect(row.body.age).toBeUndefined() - expect(row.body.jobTitle).toBeUndefined() + expect(row._viewId).toBeUndefined() + expect(row.age).toBeUndefined() + expect(row.jobTitle).toBeUndefined() }) }) @@ -1042,7 +1039,7 @@ describe.each([ }) const row = await config.api.row.get(tableId, newRow._id!) - expect(row.body).toEqual({ + expect(row).toEqual({ ...newRow, name: newData.name, address: newData.address, @@ -1051,9 +1048,9 @@ describe.each([ id: newRow.id, ...defaultRowFields, }) - expect(row.body._viewId).toBeUndefined() - expect(row.body.age).toBeUndefined() - expect(row.body.jobTitle).toBeUndefined() + expect(row._viewId).toBeUndefined() + expect(row.age).toBeUndefined() + expect(row.jobTitle).toBeUndefined() }) }) @@ -1076,7 +1073,7 @@ describe.each([ await assertRowUsage(rowUsage - 1) await config.api.row.get(tableId, createdRow._id!, { - expectStatus: 404, + status: 404, }) }) @@ -1102,12 +1099,12 @@ describe.each([ await assertRowUsage(rowUsage - 2) await config.api.row.get(tableId, rows[0]._id!, { - expectStatus: 404, + status: 404, }) await config.api.row.get(tableId, rows[2]._id!, { - expectStatus: 404, + status: 404, }) - await config.api.row.get(tableId, rows[1]._id!, { expectStatus: 200 }) + await config.api.row.get(tableId, rows[1]._id!, { status: 200 }) }) }) @@ -1754,7 +1751,7 @@ describe.each([ } const row = await config.api.row.save(tableId, rowData) - const { body: retrieved } = await config.api.row.get(tableId, row._id!) + const retrieved = await config.api.row.get(tableId, row._id!) expect(retrieved).toEqual({ name: rowData.name, description: rowData.description, @@ -1781,7 +1778,7 @@ describe.each([ } const row = await config.api.row.save(tableId, rowData) - const { body: retrieved } = await config.api.row.get(tableId, row._id!) + const retrieved = await config.api.row.get(tableId, row._id!) expect(retrieved).toEqual({ name: rowData.name, description: rowData.description, diff --git a/packages/server/src/api/routes/tests/table.spec.ts b/packages/server/src/api/routes/tests/table.spec.ts index 4c83237a49..29465145a9 100644 --- a/packages/server/src/api/routes/tests/table.spec.ts +++ b/packages/server/src/api/routes/tests/table.spec.ts @@ -663,8 +663,7 @@ describe("/tables", () => { expect(migratedTable.schema["user column"]).toBeDefined() expect(migratedTable.schema["user relationship"]).not.toBeDefined() - const resp = await config.api.row.get(table._id!, testRow._id!) - const migratedRow = resp.body as Row + const migratedRow = await config.api.row.get(table._id!, testRow._id!) expect(migratedRow["user column"]).toBeDefined() expect(migratedRow["user relationship"]).not.toBeDefined() @@ -716,15 +715,13 @@ describe("/tables", () => { expect(migratedTable.schema["user column"]).toBeDefined() expect(migratedTable.schema["user relationship"]).not.toBeDefined() - const row1Migrated = (await config.api.row.get(table._id!, row1._id!)) - .body as Row + const row1Migrated = await config.api.row.get(table._id!, row1._id!) expect(row1Migrated["user relationship"]).not.toBeDefined() expect(row1Migrated["user column"].map((r: Row) => r._id)).toEqual( expect.arrayContaining([users[0]._id, users[1]._id]) ) - const row2Migrated = (await config.api.row.get(table._id!, row2._id!)) - .body as Row + const row2Migrated = await config.api.row.get(table._id!, row2._id!) expect(row2Migrated["user relationship"]).not.toBeDefined() expect(row2Migrated["user column"].map((r: Row) => r._id)).toEqual( expect.arrayContaining([users[1]._id, users[2]._id]) @@ -773,15 +770,13 @@ describe("/tables", () => { expect(migratedTable.schema["user column"]).toBeDefined() expect(migratedTable.schema["user relationship"]).not.toBeDefined() - const row1Migrated = (await config.api.row.get(table._id!, row1._id!)) - .body as Row + const row1Migrated = await config.api.row.get(table._id!, row1._id!) expect(row1Migrated["user relationship"]).not.toBeDefined() expect(row1Migrated["user column"].map((r: Row) => r._id)).toEqual( expect.arrayContaining([users[0]._id, users[1]._id]) ) - const row2Migrated = (await config.api.row.get(table._id!, row2._id!)) - .body as Row + const row2Migrated = await config.api.row.get(table._id!, row2._id!) expect(row2Migrated["user relationship"]).not.toBeDefined() expect(row2Migrated["user column"].map((r: Row) => r._id)).toEqual([ users[2]._id, diff --git a/packages/server/src/automations/tests/createRow.spec.ts b/packages/server/src/automations/tests/createRow.spec.ts index 0615fcdd97..0098be39a5 100644 --- a/packages/server/src/automations/tests/createRow.spec.ts +++ b/packages/server/src/automations/tests/createRow.spec.ts @@ -24,7 +24,7 @@ describe("test the create row action", () => { expect(res.id).toBeDefined() expect(res.revision).toBeDefined() expect(res.success).toEqual(true) - const gottenRow = await config.getRow(table._id, res.id) + const gottenRow = await config.api.row.get(table._id, res.id) expect(gottenRow.name).toEqual("test") expect(gottenRow.description).toEqual("test") }) diff --git a/packages/server/src/automations/tests/updateRow.spec.ts b/packages/server/src/automations/tests/updateRow.spec.ts index b64c52147d..76823e8a11 100644 --- a/packages/server/src/automations/tests/updateRow.spec.ts +++ b/packages/server/src/automations/tests/updateRow.spec.ts @@ -36,7 +36,7 @@ describe("test the update row action", () => { it("should be able to run the action", async () => { const res = await setup.runStep(setup.actions.UPDATE_ROW.stepId, inputs) expect(res.success).toEqual(true) - const updatedRow = await config.getRow(table._id!, res.id) + const updatedRow = await config.api.row.get(table._id!, res.id) expect(updatedRow.name).toEqual("Updated name") expect(updatedRow.description).not.toEqual("") }) @@ -87,8 +87,8 @@ describe("test the update row action", () => { }) let getResp = await config.api.row.get(table._id!, row._id!) - expect(getResp.body.user1[0]._id).toEqual(user1._id) - expect(getResp.body.user2[0]._id).toEqual(user2._id) + expect(getResp.user1[0]._id).toEqual(user1._id) + expect(getResp.user2[0]._id).toEqual(user2._id) let stepResp = await setup.runStep(setup.actions.UPDATE_ROW.stepId, { rowId: row._id, @@ -103,8 +103,8 @@ describe("test the update row action", () => { expect(stepResp.success).toEqual(true) getResp = await config.api.row.get(table._id!, row._id!) - expect(getResp.body.user1[0]._id).toEqual(user2._id) - expect(getResp.body.user2[0]._id).toEqual(user2._id) + expect(getResp.user1[0]._id).toEqual(user2._id) + expect(getResp.user2[0]._id).toEqual(user2._id) }) it("should overwrite links if those links are not set and we ask it do", async () => { @@ -140,8 +140,8 @@ describe("test the update row action", () => { }) let getResp = await config.api.row.get(table._id!, row._id!) - expect(getResp.body.user1[0]._id).toEqual(user1._id) - expect(getResp.body.user2[0]._id).toEqual(user2._id) + expect(getResp.user1[0]._id).toEqual(user1._id) + expect(getResp.user2[0]._id).toEqual(user2._id) let stepResp = await setup.runStep(setup.actions.UPDATE_ROW.stepId, { rowId: row._id, @@ -163,7 +163,7 @@ describe("test the update row action", () => { expect(stepResp.success).toEqual(true) getResp = await config.api.row.get(table._id!, row._id!) - expect(getResp.body.user1[0]._id).toEqual(user2._id) - expect(getResp.body.user2).toBeUndefined() + expect(getResp.user1[0]._id).toEqual(user2._id) + expect(getResp.user2).toBeUndefined() }) }) diff --git a/packages/server/src/db/tests/linkController.spec.ts b/packages/server/src/db/tests/linkController.spec.ts index ae1922db27..4f41fd3838 100644 --- a/packages/server/src/db/tests/linkController.spec.ts +++ b/packages/server/src/db/tests/linkController.spec.ts @@ -100,7 +100,7 @@ describe("test the link controller", () => { const { _id } = await config.createRow( basicLinkedRow(t1._id!, row._id!, linkField) ) - return config.getRow(t1._id!, _id!) + return config.api.row.get(t1._id!, _id!) } it("should be able to confirm if two table schemas are equal", async () => { diff --git a/packages/server/src/integration-test/postgres.spec.ts b/packages/server/src/integration-test/postgres.spec.ts index 2680d1a11b..7c14bc2b69 100644 --- a/packages/server/src/integration-test/postgres.spec.ts +++ b/packages/server/src/integration-test/postgres.spec.ts @@ -398,7 +398,7 @@ describe("postgres integrations", () => { expect(res.status).toBe(200) expect(res.body).toEqual(updatedRow) - const persistedRow = await config.getRow( + const persistedRow = await config.api.row.get( primaryPostgresTable._id!, row.id ) diff --git a/packages/server/src/sdk/app/rows/tests/internal.spec.ts b/packages/server/src/sdk/app/rows/tests/internal.spec.ts index 3908ef83ed..877bd1e6dc 100644 --- a/packages/server/src/sdk/app/rows/tests/internal.spec.ts +++ b/packages/server/src/sdk/app/rows/tests/internal.spec.ts @@ -98,7 +98,10 @@ describe("sdk >> rows >> internal", () => { }, }) - const persistedRow = await config.getRow(table._id!, response.row._id!) + const persistedRow = await config.api.row.get( + table._id!, + response.row._id! + ) expect(persistedRow).toEqual({ ...row, type: "row", @@ -157,7 +160,10 @@ describe("sdk >> rows >> internal", () => { }, }) - const persistedRow = await config.getRow(table._id!, response.row._id!) + const persistedRow = await config.api.row.get( + table._id!, + response.row._id! + ) expect(persistedRow).toEqual({ ...row, type: "row", diff --git a/packages/server/src/tests/utilities/TestConfiguration.ts b/packages/server/src/tests/utilities/TestConfiguration.ts index 21605b7a5e..35ca2982c0 100644 --- a/packages/server/src/tests/utilities/TestConfiguration.ts +++ b/packages/server/src/tests/utilities/TestConfiguration.ts @@ -712,11 +712,6 @@ export default class TestConfiguration { return this.api.row.save(tableId, config) } - async getRow(tableId: string, rowId: string): Promise { - const res = await this.api.row.get(tableId, rowId) - return res.body - } - async getRows(tableId: string) { if (!tableId && this.table) { tableId = this.table._id! diff --git a/packages/server/src/tests/utilities/api/base.ts b/packages/server/src/tests/utilities/api/base.ts index df37c62f00..d69a254f16 100644 --- a/packages/server/src/tests/utilities/api/base.ts +++ b/packages/server/src/tests/utilities/api/base.ts @@ -3,9 +3,6 @@ import { SuperTest, Test, Response } from "supertest" import { ReadStream } from "fs" type Headers = Record -type SuccessStatus = 200 | 201 | 204 -type ErrorStatus = 400 | 401 | 403 | 404 | 500 | 502 | 503 | 504 -type Status = SuccessStatus | ErrorStatus type Method = "get" | "post" | "put" | "patch" | "delete" export interface AttachedFile { @@ -25,7 +22,7 @@ function isAttachedFile(file: any): file is AttachedFile { } export interface Expectations { - status?: Status + status?: number headers?: Record headersNotPresent?: string[] body?: Record diff --git a/packages/server/src/tests/utilities/api/query.ts b/packages/server/src/tests/utilities/api/query.ts index 4b6e99fd6c..32866314ff 100644 --- a/packages/server/src/tests/utilities/api/query.ts +++ b/packages/server/src/tests/utilities/api/query.ts @@ -16,9 +16,12 @@ export class QueryAPI extends TestAPI { queryId: string, body?: ExecuteQueryRequest ): Promise => { - return await this._post(`/api/queries/${queryId}`, { - body, - }) + return await this._post( + `/api/v2/queries/${queryId}`, + { + body, + } + ) } previewQuery = async (queryPreview: PreviewQueryRequest) => { diff --git a/packages/server/src/tests/utilities/api/row.ts b/packages/server/src/tests/utilities/api/row.ts index 936c906f9f..edcca5fa84 100644 --- a/packages/server/src/tests/utilities/api/row.ts +++ b/packages/server/src/tests/utilities/api/row.ts @@ -9,42 +9,27 @@ import { SearchRowResponse, SearchParams, } from "@budibase/types" -import TestConfiguration from "../TestConfiguration" -import { TestAPI } from "./base" +import { Expectations, TestAPI } from "./base" export class RowAPI extends TestAPI { - constructor(config: TestConfiguration) { - super(config) - } - get = async ( sourceId: string, rowId: string, - { expectStatus } = { expectStatus: 200 } + expectations?: Expectations ) => { - const request = this.request - .get(`/api/${sourceId}/rows/${rowId}`) - .set(this.config.defaultHeaders()) - .expect(expectStatus) - if (expectStatus !== 404) { - request.expect("Content-Type", /json/) - } - return request + return await this._get(`/api/${sourceId}/rows/${rowId}`, { + expectations, + }) } getEnriched = async ( sourceId: string, rowId: string, - { expectStatus } = { expectStatus: 200 } + expectations?: Expectations ) => { - const request = this.request - .get(`/api/${sourceId}/${rowId}/enrich`) - .set(this.config.defaultHeaders()) - .expect(expectStatus) - if (expectStatus !== 404) { - request.expect("Content-Type", /json/) - } - return request + return await this._get(`/api/${sourceId}/${rowId}/enrich`, { + expectations, + }) } save = async ( From d5c6ab86482d983d62199c529c79bb4fbbe82261 Mon Sep 17 00:00:00 2001 From: Sam Rose Date: Fri, 1 Mar 2024 16:03:52 +0000 Subject: [PATCH 18/38] Fix tests. --- packages/server/src/api/routes/tests/row.spec.ts | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/packages/server/src/api/routes/tests/row.spec.ts b/packages/server/src/api/routes/tests/row.spec.ts index 38f28cc138..f1c8eb38a6 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.each([ const createdRow = await config.createRow(row) const id = createdRow._id! - const saved = await config.api.row.get(id, table._id!) + const saved = await config.api.row.get(table._id!, id) expect(saved.stringUndefined).toBe(undefined) expect(saved.stringNull).toBe(null) @@ -521,7 +521,7 @@ describe.each([ expect(row.name).toEqual("Updated Name") expect(row.description).toEqual(existing.description) - const savedRow = await config.api.row.get(row._id!, table._id!) + const savedRow = await config.api.row.get(table._id!, row._id!) expect(savedRow.description).toEqual(existing.description) expect(savedRow.name).toEqual("Updated Name") @@ -682,7 +682,7 @@ describe.each([ const res = await config.api.row.delete(table._id!, [row1, row2]) expect(res.body.length).toEqual(2) - await config.api.row.get(row1._id!, table._id!, { status: 404 }) + await config.api.row.get(table._id!, row1._id!, { status: 404 }) await assertRowUsage(rowUsage - 2) }) @@ -701,7 +701,7 @@ describe.each([ ]) expect(res.body.length).toEqual(3) - await config.api.row.get(row1._id!, table._id!, { status: 404 }) + await config.api.row.get(table._id!, row1._id!, { status: 404 }) await assertRowUsage(rowUsage - 3) }) @@ -712,7 +712,7 @@ describe.each([ const res = await config.api.row.delete(table._id!, row1) expect(res.body.id).toEqual(row1._id) - await config.api.row.get(row1._id!, table._id!, { status: 404 }) + await config.api.row.get(table._id!, row1._id!, { status: 404 }) await assertRowUsage(rowUsage - 1) }) From a639ba91d38dbcf61ef2d546615f8521ede88f1f Mon Sep 17 00:00:00 2001 From: Sam Rose Date: Fri, 1 Mar 2024 16:38:31 +0000 Subject: [PATCH 19/38] Migrate RowAPI.save --- .../server/src/api/routes/tests/row.spec.ts | 2 +- .../server/src/tests/utilities/api/row.ts | 19 +++++-------------- 2 files changed, 6 insertions(+), 15 deletions(-) diff --git a/packages/server/src/api/routes/tests/row.spec.ts b/packages/server/src/api/routes/tests/row.spec.ts index f1c8eb38a6..69e268641a 100644 --- a/packages/server/src/api/routes/tests/row.spec.ts +++ b/packages/server/src/api/routes/tests/row.spec.ts @@ -1486,7 +1486,7 @@ describe.each([ email: "joe@joe.com", roles: {}, }, - { expectStatus: 400 } + { status: 400 } ) expect(response.message).toBe("Cannot create new user entry.") }) diff --git a/packages/server/src/tests/utilities/api/row.ts b/packages/server/src/tests/utilities/api/row.ts index edcca5fa84..7e3a0442f6 100644 --- a/packages/server/src/tests/utilities/api/row.ts +++ b/packages/server/src/tests/utilities/api/row.ts @@ -35,21 +35,12 @@ export class RowAPI extends TestAPI { save = async ( tableId: string, row: SaveRowRequest, - { expectStatus } = { expectStatus: 200 } + expectations?: Expectations ): Promise => { - const resp = await this.request - .post(`/api/${tableId}/rows`) - .send(row) - .set(this.config.defaultHeaders()) - .expect("Content-Type", /json/) - if (resp.status !== expectStatus) { - throw new Error( - `Expected status ${expectStatus} but got ${ - resp.status - }, body: ${JSON.stringify(resp.body)}` - ) - } - return resp.body as Row + return await this._post(`/api/${tableId}/rows`, { + body: row, + expectations, + }) } validate = async ( From 58b1c2bca66f9fad4928dab2e365ce1786b5f6f3 Mon Sep 17 00:00:00 2001 From: Sam Rose Date: Fri, 1 Mar 2024 16:39:57 +0000 Subject: [PATCH 20/38] Migrate RowAPI.validate --- packages/server/src/tests/utilities/api/row.ts | 16 ++++++++-------- 1 file changed, 8 insertions(+), 8 deletions(-) diff --git a/packages/server/src/tests/utilities/api/row.ts b/packages/server/src/tests/utilities/api/row.ts index 7e3a0442f6..941ac986e2 100644 --- a/packages/server/src/tests/utilities/api/row.ts +++ b/packages/server/src/tests/utilities/api/row.ts @@ -46,15 +46,15 @@ export class RowAPI extends TestAPI { validate = async ( sourceId: string, row: SaveRowRequest, - { expectStatus } = { expectStatus: 200 } + expectations?: Expectations ): Promise => { - const resp = await this.request - .post(`/api/${sourceId}/rows/validate`) - .send(row) - .set(this.config.defaultHeaders()) - .expect("Content-Type", /json/) - .expect(expectStatus) - return resp.body as ValidateResponse + return await this._post( + `/api/${sourceId}/rows/validate`, + { + body: row, + expectations, + } + ) } patch = async ( From 4efafaeeaf6229cc20b81ce7c5d49f91e30325eb Mon Sep 17 00:00:00 2001 From: Sam Rose Date: Fri, 1 Mar 2024 16:40:47 +0000 Subject: [PATCH 21/38] Migrate RowAPI.patch --- .../server/src/tests/utilities/api/row.ts | 19 +++++-------------- 1 file changed, 5 insertions(+), 14 deletions(-) diff --git a/packages/server/src/tests/utilities/api/row.ts b/packages/server/src/tests/utilities/api/row.ts index 941ac986e2..b2f283bd56 100644 --- a/packages/server/src/tests/utilities/api/row.ts +++ b/packages/server/src/tests/utilities/api/row.ts @@ -60,21 +60,12 @@ export class RowAPI extends TestAPI { patch = async ( sourceId: string, row: PatchRowRequest, - { expectStatus } = { expectStatus: 200 } + expectations?: Expectations ): Promise => { - let resp = await this.request - .patch(`/api/${sourceId}/rows`) - .send(row) - .set(this.config.defaultHeaders()) - .expect("Content-Type", /json/) - if (resp.status !== expectStatus) { - throw new Error( - `Expected status ${expectStatus} but got ${ - resp.status - }, body: ${JSON.stringify(resp.body)}` - ) - } - return resp.body as Row + return await this._patch(`/api/${sourceId}/rows`, { + body: row, + expectations, + }) } delete = async ( From a98948150dca6bb114eb10283a8b4fa6ba9a6025 Mon Sep 17 00:00:00 2001 From: Sam Rose Date: Fri, 1 Mar 2024 17:03:34 +0000 Subject: [PATCH 22/38] Migrate RowAPI.delete --- .../server/src/api/routes/tests/row.spec.ts | 69 ++++++++++--------- .../server/src/tests/utilities/api/row.ts | 28 +++++--- 2 files changed, 57 insertions(+), 40 deletions(-) diff --git a/packages/server/src/api/routes/tests/row.spec.ts b/packages/server/src/api/routes/tests/row.spec.ts index 69e268641a..e4e338700e 100644 --- a/packages/server/src/api/routes/tests/row.spec.ts +++ b/packages/server/src/api/routes/tests/row.spec.ts @@ -7,6 +7,7 @@ import { context, InternalTable, roles, tenancy } from "@budibase/backend-core" import { quotas } from "@budibase/pro" import { AutoFieldSubType, + DeleteRow, FieldSchema, FieldType, FieldTypeSubtypes, @@ -540,7 +541,7 @@ describe.each([ tableId: table._id!, name: 1, }, - { expectStatus: 400 } + { status: 400 } ) await assertRowUsage(rowUsage) @@ -629,8 +630,10 @@ describe.each([ const createdRow = await config.createRow() const rowUsage = await getRowUsage() - const res = await config.api.row.delete(table._id!, [createdRow]) - expect(res.body[0]._id).toEqual(createdRow._id) + const res = await config.api.row.deleteMany(table._id!, { + rows: [createdRow], + }) + expect(res[0]._id).toEqual(createdRow._id) await assertRowUsage(rowUsage - 1) }) }) @@ -679,9 +682,11 @@ describe.each([ const row2 = await config.createRow() const rowUsage = await getRowUsage() - const res = await config.api.row.delete(table._id!, [row1, row2]) + const res = await config.api.row.deleteMany(table._id!, { + rows: [row1, row2], + }) - expect(res.body.length).toEqual(2) + expect(res.length).toEqual(2) await config.api.row.get(table._id!, row1._id!, { status: 404 }) await assertRowUsage(rowUsage - 2) }) @@ -694,13 +699,11 @@ describe.each([ ]) const rowUsage = await getRowUsage() - const res = await config.api.row.delete(table._id!, [ - row1, - row2._id, - { _id: row3._id }, - ]) + const res = await config.api.row.deleteMany(table._id!, { + rows: [row1, row2._id!, { _id: row3._id }], + }) - expect(res.body.length).toEqual(3) + expect(res.length).toEqual(3) await config.api.row.get(table._id!, row1._id!, { status: 404 }) await assertRowUsage(rowUsage - 3) }) @@ -709,9 +712,9 @@ describe.each([ const row1 = await config.createRow() const rowUsage = await getRowUsage() - const res = await config.api.row.delete(table._id!, row1) + const res = await config.api.row.delete(table._id!, row1 as DeleteRow) - expect(res.body.id).toEqual(row1._id) + expect(res.id).toEqual(row1._id) await config.api.row.get(table._id!, row1._id!, { status: 404 }) await assertRowUsage(rowUsage - 1) }) @@ -719,24 +722,26 @@ describe.each([ it("Should ignore malformed/invalid delete requests", async () => { const rowUsage = await getRowUsage() - const res = await config.api.row.delete( - table._id!, - { not: "valid" }, - { expectStatus: 400 } - ) - expect(res.body.message).toEqual("Invalid delete rows request") - - const res2 = await config.api.row.delete( - table._id!, - { rows: 123 }, - { expectStatus: 400 } - ) - expect(res2.body.message).toEqual("Invalid delete rows request") - - const res3 = await config.api.row.delete(table._id!, "invalid", { - expectStatus: 400, + await config.api.row.delete(table._id!, { not: "valid" } as any, { + status: 400, + body: { + message: "Invalid delete rows request", + }, + }) + + await config.api.row.delete(table._id!, { rows: 123 } as any, { + status: 400, + body: { + message: "Invalid delete rows request", + }, + }) + + await config.api.row.delete(table._id!, "invalid" as any, { + status: 400, + body: { + message: "Invalid delete rows request", + }, }) - expect(res3.body.message).toEqual("Invalid delete rows request") await assertRowUsage(rowUsage) }) @@ -1068,7 +1073,7 @@ describe.each([ const createdRow = await config.createRow() const rowUsage = await getRowUsage() - await config.api.row.delete(view.id, [createdRow]) + await config.api.row.deleteMany(view.id, { rows: [createdRow] }) await assertRowUsage(rowUsage - 1) @@ -1094,7 +1099,7 @@ describe.each([ ]) const rowUsage = await getRowUsage() - await config.api.row.delete(view.id, [rows[0], rows[2]]) + await config.api.row.deleteMany(view.id, { rows: [rows[0], rows[2]] }) await assertRowUsage(rowUsage - 2) diff --git a/packages/server/src/tests/utilities/api/row.ts b/packages/server/src/tests/utilities/api/row.ts index b2f283bd56..2802576908 100644 --- a/packages/server/src/tests/utilities/api/row.ts +++ b/packages/server/src/tests/utilities/api/row.ts @@ -8,6 +8,9 @@ import { BulkImportResponse, SearchRowResponse, SearchParams, + DeleteRowRequest, + DeleteRows, + DeleteRow, } from "@budibase/types" import { Expectations, TestAPI } from "./base" @@ -70,15 +73,24 @@ export class RowAPI extends TestAPI { delete = async ( sourceId: string, - rows: Row | string | (Row | string)[], - { expectStatus } = { expectStatus: 200 } + row: DeleteRow, + expectations?: Expectations ) => { - return this.request - .delete(`/api/${sourceId}/rows`) - .send(Array.isArray(rows) ? { rows } : rows) - .set(this.config.defaultHeaders()) - .expect("Content-Type", /json/) - .expect(expectStatus) + return await this._delete(`/api/${sourceId}/rows`, { + body: row, + expectations, + }) + } + + deleteMany = async ( + sourceId: string, + body: DeleteRows, + expectations?: Expectations + ) => { + return await this._delete(`/api/${sourceId}/rows`, { + body, + expectations, + }) } fetch = async ( From 376bb9c105976d80676af3900b799645f9fad0ce Mon Sep 17 00:00:00 2001 From: Sam Rose Date: Fri, 1 Mar 2024 17:04:35 +0000 Subject: [PATCH 23/38] Migrate RowAPI.fetch --- packages/server/src/tests/utilities/api/row.ts | 11 ++++------- 1 file changed, 4 insertions(+), 7 deletions(-) diff --git a/packages/server/src/tests/utilities/api/row.ts b/packages/server/src/tests/utilities/api/row.ts index 2802576908..0ac05ee653 100644 --- a/packages/server/src/tests/utilities/api/row.ts +++ b/packages/server/src/tests/utilities/api/row.ts @@ -95,14 +95,11 @@ export class RowAPI extends TestAPI { fetch = async ( sourceId: string, - { expectStatus } = { expectStatus: 200 } + expectations?: Expectations ): Promise => { - const request = this.request - .get(`/api/${sourceId}/rows`) - .set(this.config.defaultHeaders()) - .expect(expectStatus) - - return (await request).body + return await this._get(`/api/${sourceId}/rows`, { + expectations, + }) } exportRows = async ( From a4e212c0d8eda3e78e23833f8f0c89bda9b8c079 Mon Sep 17 00:00:00 2001 From: Sam Rose Date: Fri, 1 Mar 2024 17:10:49 +0000 Subject: [PATCH 24/38] Migrate RowAPI.exportRows --- .../server/src/api/routes/tests/row.spec.ts | 4 ++-- .../server/src/tests/utilities/api/row.ts | 20 +++++++++++-------- 2 files changed, 14 insertions(+), 10 deletions(-) diff --git a/packages/server/src/api/routes/tests/row.spec.ts b/packages/server/src/api/routes/tests/row.spec.ts index e4e338700e..091aec5e1f 100644 --- a/packages/server/src/api/routes/tests/row.spec.ts +++ b/packages/server/src/api/routes/tests/row.spec.ts @@ -905,7 +905,7 @@ describe.each([ const res = await config.api.row.exportRows(table._id!, { rows: [existing._id!], }) - const results = JSON.parse(res.text) + const results = JSON.parse(res) expect(results.length).toEqual(1) const row = results[0] @@ -924,7 +924,7 @@ describe.each([ rows: [existing._id!], columns: ["_id"], }) - const results = JSON.parse(res.text) + const results = JSON.parse(res) expect(results.length).toEqual(1) const row = results[0] diff --git a/packages/server/src/tests/utilities/api/row.ts b/packages/server/src/tests/utilities/api/row.ts index 0ac05ee653..7f28e7292a 100644 --- a/packages/server/src/tests/utilities/api/row.ts +++ b/packages/server/src/tests/utilities/api/row.ts @@ -11,6 +11,7 @@ import { DeleteRowRequest, DeleteRows, DeleteRow, + ExportRowsResponse, } from "@budibase/types" import { Expectations, TestAPI } from "./base" @@ -105,15 +106,18 @@ export class RowAPI extends TestAPI { exportRows = async ( tableId: string, body: ExportRowsRequest, - { expectStatus } = { expectStatus: 200 } + expectations?: Expectations ) => { - const request = this.request - .post(`/api/${tableId}/rows/exportRows?format=json`) - .set(this.config.defaultHeaders()) - .send(body) - .expect("Content-Type", /json/) - .expect(expectStatus) - return request + const response = await this._requestRaw( + "post", + `/api/${tableId}/rows/exportRows`, + { + body, + query: { format: "json" }, + expectations, + } + ) + return response.text } bulkImport = async ( From 02ac338c3f19b3852c68e741a536087ddca223a4 Mon Sep 17 00:00:00 2001 From: Sam Rose Date: Fri, 1 Mar 2024 17:11:19 +0000 Subject: [PATCH 25/38] deleteMany -> bulkDelete --- packages/server/src/api/routes/tests/row.spec.ts | 10 +++++----- packages/server/src/tests/utilities/api/row.ts | 2 +- 2 files changed, 6 insertions(+), 6 deletions(-) diff --git a/packages/server/src/api/routes/tests/row.spec.ts b/packages/server/src/api/routes/tests/row.spec.ts index 091aec5e1f..027a7fff58 100644 --- a/packages/server/src/api/routes/tests/row.spec.ts +++ b/packages/server/src/api/routes/tests/row.spec.ts @@ -630,7 +630,7 @@ describe.each([ const createdRow = await config.createRow() const rowUsage = await getRowUsage() - const res = await config.api.row.deleteMany(table._id!, { + const res = await config.api.row.bulkDelete(table._id!, { rows: [createdRow], }) expect(res[0]._id).toEqual(createdRow._id) @@ -682,7 +682,7 @@ describe.each([ const row2 = await config.createRow() const rowUsage = await getRowUsage() - const res = await config.api.row.deleteMany(table._id!, { + const res = await config.api.row.bulkDelete(table._id!, { rows: [row1, row2], }) @@ -699,7 +699,7 @@ describe.each([ ]) const rowUsage = await getRowUsage() - const res = await config.api.row.deleteMany(table._id!, { + const res = await config.api.row.bulkDelete(table._id!, { rows: [row1, row2._id!, { _id: row3._id }], }) @@ -1073,7 +1073,7 @@ describe.each([ const createdRow = await config.createRow() const rowUsage = await getRowUsage() - await config.api.row.deleteMany(view.id, { rows: [createdRow] }) + await config.api.row.bulkDelete(view.id, { rows: [createdRow] }) await assertRowUsage(rowUsage - 1) @@ -1099,7 +1099,7 @@ describe.each([ ]) const rowUsage = await getRowUsage() - await config.api.row.deleteMany(view.id, { rows: [rows[0], rows[2]] }) + await config.api.row.bulkDelete(view.id, { rows: [rows[0], rows[2]] }) await assertRowUsage(rowUsage - 2) diff --git a/packages/server/src/tests/utilities/api/row.ts b/packages/server/src/tests/utilities/api/row.ts index 7f28e7292a..af743e3c9b 100644 --- a/packages/server/src/tests/utilities/api/row.ts +++ b/packages/server/src/tests/utilities/api/row.ts @@ -83,7 +83,7 @@ export class RowAPI extends TestAPI { }) } - deleteMany = async ( + bulkDelete = async ( sourceId: string, body: DeleteRows, expectations?: Expectations From c0907d37ef890a8a117f525198f5baa8ab592aaf Mon Sep 17 00:00:00 2001 From: Sam Rose Date: Fri, 1 Mar 2024 17:12:46 +0000 Subject: [PATCH 26/38] Migrate RowAPI.bulkImport --- packages/server/src/tests/utilities/api/row.ts | 12 +++++------- 1 file changed, 5 insertions(+), 7 deletions(-) diff --git a/packages/server/src/tests/utilities/api/row.ts b/packages/server/src/tests/utilities/api/row.ts index af743e3c9b..558ef100a3 100644 --- a/packages/server/src/tests/utilities/api/row.ts +++ b/packages/server/src/tests/utilities/api/row.ts @@ -123,14 +123,12 @@ export class RowAPI extends TestAPI { bulkImport = async ( tableId: string, body: BulkImportRequest, - { expectStatus } = { expectStatus: 200 } + expectations?: Expectations ): Promise => { - let request = this.request - .post(`/api/tables/${tableId}/import`) - .send(body) - .set(this.config.defaultHeaders()) - .expect(expectStatus) - return (await request).body + return await this._post(`/api/${tableId}/rows/import`, { + body, + expectations, + }) } search = async ( From f91db6d9850e146fe0fdd1170b5eed3c2fd79187 Mon Sep 17 00:00:00 2001 From: Sam Rose Date: Fri, 1 Mar 2024 17:13:46 +0000 Subject: [PATCH 27/38] Migrate RowAPI.search --- packages/server/src/tests/utilities/api/row.ts | 13 +++++-------- 1 file changed, 5 insertions(+), 8 deletions(-) diff --git a/packages/server/src/tests/utilities/api/row.ts b/packages/server/src/tests/utilities/api/row.ts index 558ef100a3..58ddb7c049 100644 --- a/packages/server/src/tests/utilities/api/row.ts +++ b/packages/server/src/tests/utilities/api/row.ts @@ -134,14 +134,11 @@ export class RowAPI extends TestAPI { search = async ( sourceId: string, params?: SearchParams, - { expectStatus } = { expectStatus: 200 } + expectations?: Expectations ): Promise => { - const request = this.request - .post(`/api/${sourceId}/search`) - .send(params) - .set(this.config.defaultHeaders()) - .expect(expectStatus) - - return (await request).body + return await this._post(`/api/${sourceId}/search`, { + body: params, + expectations, + }) } } From 149d2c0b29ab594fdd2b1d3a89ab4f927c0b1a01 Mon Sep 17 00:00:00 2001 From: Sam Rose Date: Fri, 1 Mar 2024 17:14:30 +0000 Subject: [PATCH 28/38] Migrate ScreenAPI --- .../server/src/tests/utilities/api/screen.ts | 16 +++------------- 1 file changed, 3 insertions(+), 13 deletions(-) diff --git a/packages/server/src/tests/utilities/api/screen.ts b/packages/server/src/tests/utilities/api/screen.ts index 9245ffe4ba..c8d3e647d8 100644 --- a/packages/server/src/tests/utilities/api/screen.ts +++ b/packages/server/src/tests/utilities/api/screen.ts @@ -1,18 +1,8 @@ -import TestConfiguration from "../TestConfiguration" import { Screen } from "@budibase/types" -import { TestAPI } from "./base" +import { Expectations, TestAPI } from "./base" export class ScreenAPI extends TestAPI { - constructor(config: TestConfiguration) { - super(config) - } - - list = async (): Promise => { - const res = await this.request - .get(`/api/screens`) - .set(this.config.defaultHeaders()) - .expect("Content-Type", /json/) - .expect(200) - return res.body as Screen[] + list = async (expectations?: Expectations): Promise => { + return await this._get(`/api/screens`, { expectations }) } } From 7fa5dbeec9ab3a39d8f609a263b0484fb6d68096 Mon Sep 17 00:00:00 2001 From: Sam Rose Date: Fri, 1 Mar 2024 17:17:38 +0000 Subject: [PATCH 29/38] Migrate UserAPI --- .../server/src/api/routes/tests/user.spec.ts | 2 +- .../server/src/tests/utilities/api/user.ts | 157 +++++------------- 2 files changed, 42 insertions(+), 117 deletions(-) diff --git a/packages/server/src/api/routes/tests/user.spec.ts b/packages/server/src/api/routes/tests/user.spec.ts index 076ee064dc..ff8c0d54b3 100644 --- a/packages/server/src/api/routes/tests/user.spec.ts +++ b/packages/server/src/api/routes/tests/user.spec.ts @@ -90,7 +90,7 @@ describe("/users", () => { }) await config.api.user.update( { ...user, roleId: roles.BUILTIN_ROLE_IDS.POWER }, - { expectStatus: 409 } + { status: 409 } ) }) }) diff --git a/packages/server/src/tests/utilities/api/user.ts b/packages/server/src/tests/utilities/api/user.ts index 2ed23c0461..bb3eae0542 100644 --- a/packages/server/src/tests/utilities/api/user.ts +++ b/packages/server/src/tests/utilities/api/user.ts @@ -4,154 +4,79 @@ import { Flags, UserMetadata, } from "@budibase/types" -import TestConfiguration from "../TestConfiguration" -import { TestAPI } from "./base" +import { Expectations, TestAPI } from "./base" import { DocumentInsertResponse } from "@budibase/nano" export class UserAPI extends TestAPI { - constructor(config: TestConfiguration) { - super(config) - } - fetch = async ( - { expectStatus } = { expectStatus: 200 } + expectations?: Expectations ): Promise => { - const res = await this.request - .get(`/api/users/metadata`) - .set(this.config.defaultHeaders()) - .expect("Content-Type", /json/) - - if (res.status !== expectStatus) { - throw new Error( - `Expected status ${expectStatus} but got ${ - res.status - } with body ${JSON.stringify(res.body)}` - ) - } - - return res.body + return await this._get("/api/users/metadata", { + expectations, + }) } find = async ( id: string, - { expectStatus } = { expectStatus: 200 } + expectations?: Expectations ): Promise => { - const res = await this.request - .get(`/api/users/metadata/${id}`) - .set(this.config.defaultHeaders()) - .expect("Content-Type", /json/) - - if (res.status !== expectStatus) { - throw new Error( - `Expected status ${expectStatus} but got ${ - res.status - } with body ${JSON.stringify(res.body)}` - ) - } - - return res.body + return await this._get( + `/api/users/metadata/${id}`, + { + expectations, + } + ) } update = async ( user: UserMetadata, - { expectStatus } = { expectStatus: 200 } + expectations?: Expectations ): Promise => { - const res = await this.request - .put(`/api/users/metadata`) - .set(this.config.defaultHeaders()) - .send(user) - .expect("Content-Type", /json/) - - if (res.status !== expectStatus) { - throw new Error( - `Expected status ${expectStatus} but got ${ - res.status - } with body ${JSON.stringify(res.body)}` - ) - } - - return res.body as DocumentInsertResponse + return await this._put("/api/users/metadata", { + body: user, + expectations, + }) } updateSelf = async ( user: UserMetadata, - { expectStatus } = { expectStatus: 200 } + expectations?: Expectations ): Promise => { - const res = await this.request - .post(`/api/users/metadata/self`) - .set(this.config.defaultHeaders()) - .send(user) - .expect("Content-Type", /json/) - - if (res.status !== expectStatus) { - throw new Error( - `Expected status ${expectStatus} but got ${ - res.status - } with body ${JSON.stringify(res.body)}` - ) - } - - return res.body as DocumentInsertResponse + return await this._post( + "/api/users/metadata/self", + { + body: user, + expectations, + } + ) } destroy = async ( id: string, - { expectStatus } = { expectStatus: 200 } + expectations?: Expectations ): Promise<{ message: string }> => { - const res = await this.request - .delete(`/api/users/metadata/${id}`) - .set(this.config.defaultHeaders()) - .expect("Content-Type", /json/) - - if (res.status !== expectStatus) { - throw new Error( - `Expected status ${expectStatus} but got ${ - res.status - } with body ${JSON.stringify(res.body)}` - ) - } - - return res.body as { message: string } + return await this._delete<{ message: string }>( + `/api/users/metadata/${id}`, + { + expectations, + } + ) } setFlag = async ( flag: string, value: any, - { expectStatus } = { expectStatus: 200 } + expectations?: Expectations ): Promise<{ message: string }> => { - const res = await this.request - .post(`/api/users/flags`) - .set(this.config.defaultHeaders()) - .send({ flag, value }) - .expect("Content-Type", /json/) - - if (res.status !== expectStatus) { - throw new Error( - `Expected status ${expectStatus} but got ${ - res.status - } with body ${JSON.stringify(res.body)}` - ) - } - - return res.body as { message: string } + return await this._post<{ message: string }>(`/api/users/flags`, { + body: { flag, value }, + expectations, + }) } - getFlags = async ( - { expectStatus } = { expectStatus: 200 } - ): Promise => { - const res = await this.request - .get(`/api/users/flags`) - .set(this.config.defaultHeaders()) - .expect("Content-Type", /json/) - - if (res.status !== expectStatus) { - throw new Error( - `Expected status ${expectStatus} but got ${ - res.status - } with body ${JSON.stringify(res.body)}` - ) - } - - return res.body as Flags + getFlags = async (expectations?: Expectations): Promise => { + return await this._get(`/api/users/flags`, { + expectations, + }) } } From 37a10857df1d979e15925c09797ed3885f98bd7a Mon Sep 17 00:00:00 2001 From: Sam Rose Date: Fri, 1 Mar 2024 17:35:51 +0000 Subject: [PATCH 30/38] Migrate ViewV2API --- .../src/api/routes/tests/permissions.spec.ts | 18 ++-- .../server/src/api/routes/tests/row.spec.ts | 58 ++++++------ .../src/api/routes/tests/viewV2.spec.ts | 18 ++-- .../server/src/tests/utilities/api/base.ts | 7 +- .../server/src/tests/utilities/api/viewV2.ts | 89 +++++++++---------- packages/types/src/api/web/app/rows.ts | 6 +- 6 files changed, 90 insertions(+), 106 deletions(-) diff --git a/packages/server/src/api/routes/tests/permissions.spec.ts b/packages/server/src/api/routes/tests/permissions.spec.ts index b9c9ba0359..edaa73fd1a 100644 --- a/packages/server/src/api/routes/tests/permissions.spec.ts +++ b/packages/server/src/api/routes/tests/permissions.spec.ts @@ -187,9 +187,7 @@ describe("/permission", () => { // replicate changes before checking permissions await config.publish() - const res = await config.api.viewV2.search(view.id, undefined, { - usePublicUser: true, - }) + const res = await config.api.viewV2.publicSearch(view.id) expect(res.body.rows[0]._id).toEqual(row._id) }) @@ -202,10 +200,7 @@ describe("/permission", () => { // replicate changes before checking permissions await config.publish() - await config.api.viewV2.search(view.id, undefined, { - expectStatus: 403, - usePublicUser: true, - }) + await config.api.viewV2.publicSearch(view.id, undefined, { status: 403 }) }) it("should ignore the view permissions if the flag is not on", async () => { @@ -222,9 +217,8 @@ describe("/permission", () => { // replicate changes before checking permissions await config.publish() - await config.api.viewV2.search(view.id, undefined, { - expectStatus: 403, - usePublicUser: true, + await config.api.viewV2.publicSearch(view.id, undefined, { + status: 403, }) }) @@ -243,9 +237,7 @@ describe("/permission", () => { // replicate changes before checking permissions await config.publish() - const res = await config.api.viewV2.search(view.id, undefined, { - usePublicUser: true, - }) + const res = await config.api.viewV2.publicSearch(view.id) expect(res.body.rows[0]._id).toEqual(row._id) }) diff --git a/packages/server/src/api/routes/tests/row.spec.ts b/packages/server/src/api/routes/tests/row.spec.ts index 027a7fff58..c02159bb42 100644 --- a/packages/server/src/api/routes/tests/row.spec.ts +++ b/packages/server/src/api/routes/tests/row.spec.ts @@ -1156,8 +1156,8 @@ describe.each([ const createViewResponse = await config.createView() const response = await config.api.viewV2.search(createViewResponse.id) - expect(response.body.rows).toHaveLength(10) - expect(response.body).toEqual({ + expect(response.rows).toHaveLength(10) + expect(response).toEqual({ rows: expect.arrayContaining( rows.map(r => ({ _viewId: createViewResponse.id, @@ -1208,8 +1208,8 @@ describe.each([ const response = await config.api.viewV2.search(createViewResponse.id) - expect(response.body.rows).toHaveLength(5) - expect(response.body).toEqual({ + expect(response.rows).toHaveLength(5) + expect(response).toEqual({ rows: expect.arrayContaining( expectedRows.map(r => ({ _viewId: createViewResponse.id, @@ -1330,8 +1330,8 @@ describe.each([ createViewResponse.id ) - expect(response.body.rows).toHaveLength(4) - expect(response.body.rows).toEqual( + expect(response.rows).toHaveLength(4) + expect(response.rows).toEqual( expected.map(name => expect.objectContaining({ name })) ) } @@ -1359,8 +1359,8 @@ describe.each([ } ) - expect(response.body.rows).toHaveLength(4) - expect(response.body.rows).toEqual( + expect(response.rows).toHaveLength(4) + expect(response.rows).toEqual( expected.map(name => expect.objectContaining({ name })) ) } @@ -1384,8 +1384,8 @@ describe.each([ }) const response = await config.api.viewV2.search(view.id) - expect(response.body.rows).toHaveLength(10) - expect(response.body.rows).toEqual( + expect(response.rows).toHaveLength(10) + expect(response.rows).toEqual( expect.arrayContaining( rows.map(r => ({ ...(isInternal @@ -1404,7 +1404,7 @@ describe.each([ const createViewResponse = await config.createView() const response = await config.api.viewV2.search(createViewResponse.id) - expect(response.body.rows).toHaveLength(0) + expect(response.rows).toHaveLength(0) }) it("respects the limit parameter", async () => { @@ -1419,7 +1419,7 @@ describe.each([ query: {}, }) - expect(response.body.rows).toHaveLength(limit) + expect(response.rows).toHaveLength(limit) }) it("can handle pagination", async () => { @@ -1428,7 +1428,7 @@ describe.each([ const createViewResponse = await config.createView() const allRows = (await config.api.viewV2.search(createViewResponse.id)) - .body.rows + .rows const firstPageResponse = await config.api.viewV2.search( createViewResponse.id, @@ -1438,7 +1438,7 @@ describe.each([ query: {}, } ) - expect(firstPageResponse.body).toEqual({ + expect(firstPageResponse).toEqual({ rows: expect.arrayContaining(allRows.slice(0, 4)), totalRows: isInternal ? 10 : undefined, hasNextPage: true, @@ -1450,12 +1450,12 @@ describe.each([ { paginate: true, limit: 4, - bookmark: firstPageResponse.body.bookmark, + bookmark: firstPageResponse.bookmark, query: {}, } ) - expect(secondPageResponse.body).toEqual({ + expect(secondPageResponse).toEqual({ rows: expect.arrayContaining(allRows.slice(4, 8)), totalRows: isInternal ? 10 : undefined, hasNextPage: true, @@ -1467,11 +1467,11 @@ describe.each([ { paginate: true, limit: 4, - bookmark: secondPageResponse.body.bookmark, + bookmark: secondPageResponse.bookmark, query: {}, } ) - expect(lastPageResponse.body).toEqual({ + expect(lastPageResponse).toEqual({ rows: expect.arrayContaining(allRows.slice(8)), totalRows: isInternal ? 10 : undefined, hasNextPage: false, @@ -1518,9 +1518,8 @@ describe.each([ it("does not allow public users to fetch by default", async () => { await config.publish() - await config.api.viewV2.search(viewId, undefined, { - expectStatus: 403, - usePublicUser: true, + await config.api.viewV2.publicSearch(viewId, undefined, { + status: 403, }) }) @@ -1532,11 +1531,9 @@ describe.each([ }) await config.publish() - const response = await config.api.viewV2.search(viewId, undefined, { - usePublicUser: true, - }) + const response = await config.api.viewV2.publicSearch(viewId) - expect(response.body.rows).toHaveLength(10) + expect(response.rows).toHaveLength(10) }) it("allow public users to fetch when permissions are inherited", async () => { @@ -1547,11 +1544,9 @@ describe.each([ }) await config.publish() - const response = await config.api.viewV2.search(viewId, undefined, { - usePublicUser: true, - }) + const response = await config.api.viewV2.publicSearch(viewId) - expect(response.body.rows).toHaveLength(10) + expect(response.rows).toHaveLength(10) }) it("respects inherited permissions, not allowing not public views from public tables", async () => { @@ -1567,9 +1562,8 @@ describe.each([ }) await config.publish() - await config.api.viewV2.search(viewId, undefined, { - usePublicUser: true, - expectStatus: 403, + await config.api.viewV2.publicSearch(viewId, undefined, { + status: 403, }) }) }) diff --git a/packages/server/src/api/routes/tests/viewV2.spec.ts b/packages/server/src/api/routes/tests/viewV2.spec.ts index b03a73ddda..5198e63338 100644 --- a/packages/server/src/api/routes/tests/viewV2.spec.ts +++ b/packages/server/src/api/routes/tests/viewV2.spec.ts @@ -177,7 +177,7 @@ describe.each([ } await config.api.viewV2.create(newView, { - expectStatus: 201, + status: 201, }) }) }) @@ -275,7 +275,7 @@ describe.each([ const tableId = table._id! await config.api.viewV2.update( { ...view, id: generator.guid() }, - { expectStatus: 404 } + { status: 404 } ) expect(await config.api.table.get(tableId)).toEqual( @@ -304,7 +304,7 @@ describe.each([ }, ], }, - { expectStatus: 404 } + { status: 404 } ) expect(await config.api.table.get(tableId)).toEqual( @@ -326,12 +326,10 @@ describe.each([ ...viewV1, }, { - expectStatus: 400, - handleResponse: r => { - expect(r.body).toEqual({ - message: "Only views V2 can be updated", - status: 400, - }) + status: 400, + body: { + message: "Only views V2 can be updated", + status: 400, }, } ) @@ -403,7 +401,7 @@ describe.each([ } as Record, }, { - expectStatus: 200, + status: 200, } ) }) diff --git a/packages/server/src/tests/utilities/api/base.ts b/packages/server/src/tests/utilities/api/base.ts index d69a254f16..3f534fba86 100644 --- a/packages/server/src/tests/utilities/api/base.ts +++ b/packages/server/src/tests/utilities/api/base.ts @@ -38,6 +38,7 @@ export interface RequestOpts { Buffer | ReadStream | string | AttachedFile | undefined > expectations?: Expectations + publicUser?: boolean } export abstract class TestAPI { @@ -84,6 +85,7 @@ export abstract class TestAPI { fields = {}, files = {}, expectations, + publicUser = false, } = opts || {} const { status = 200 } = expectations || {} const expectHeaders = expectations?.headers || {} @@ -102,8 +104,11 @@ export abstract class TestAPI { url += `?${queryParams.join("&")}` } + const headersFn = publicUser + ? this.config.publicHeaders.bind(this.config) + : this.config.defaultHeaders.bind(this.config) let request = this.request[method](url).set( - this.config.defaultHeaders({ + headersFn({ "x-budibase-include-stacktrace": "true", }) ) diff --git a/packages/server/src/tests/utilities/api/viewV2.ts b/packages/server/src/tests/utilities/api/viewV2.ts index 92a6d394bf..dd49360aa8 100644 --- a/packages/server/src/tests/utilities/api/viewV2.ts +++ b/packages/server/src/tests/utilities/api/viewV2.ts @@ -3,21 +3,16 @@ import { UpdateViewRequest, ViewV2, SearchViewRowRequest, + PaginatedSearchRowResponse, } from "@budibase/types" -import TestConfiguration from "../TestConfiguration" -import { TestAPI } from "./base" +import { Expectations, 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) { - super(config) - } - create = async ( viewData?: Partial, - { expectStatus } = { expectStatus: 201 } + expectations?: Expectations ): Promise => { let tableId = viewData?.tableId if (!tableId && !this.config.table) { @@ -30,43 +25,27 @@ export class ViewV2API extends TestAPI { name: generator.guid(), ...viewData, } - const result = await this.request - .post(`/api/v2/views`) - .send(view) - .set(this.config.defaultHeaders()) - .expect("Content-Type", /json/) - .expect(expectStatus) - return result.body.data as ViewV2 + + const resp = await this._post<{ data: ViewV2 }>("/api/v2/views", { + body: view, + expectations, + }) + return resp.data } update = async ( view: UpdateViewRequest, - { - expectStatus, - handleResponse, - }: { - expectStatus: number - handleResponse?: (response: Response) => void - } = { expectStatus: 200 } + expectations?: Expectations ): Promise => { - const result = await this.request - .put(`/api/v2/views/${view.id}`) - .send(view) - .set(this.config.defaultHeaders()) - .expect("Content-Type", /json/) - .expect(expectStatus) - - if (handleResponse) { - handleResponse(result) - } - return result.body.data as ViewV2 + const resp = await this._put<{ data: ViewV2 }>(`/api/v2/views/${view.id}`, { + body: view, + expectations, + }) + return resp.data } - delete = async (viewId: string, { expectStatus } = { expectStatus: 204 }) => { - return this.request - .delete(`/api/v2/views/${viewId}`) - .set(this.config.defaultHeaders()) - .expect(expectStatus) + delete = async (viewId: string, expectations?: Expectations) => { + return await this._delete(`/api/v2/views/${viewId}`, { expectations }) } get = async (viewId: string) => { @@ -78,17 +57,29 @@ export class ViewV2API extends TestAPI { search = async ( viewId: string, params?: SearchViewRowRequest, - { expectStatus = 200, usePublicUser = false } = {} + expectations?: Expectations ) => { - return this.request - .post(`/api/v2/views/${viewId}/search`) - .send(params) - .set( - usePublicUser - ? this.config.publicHeaders() - : this.config.defaultHeaders() - ) - .expect("Content-Type", /json/) - .expect(expectStatus) + return await this._post( + `/api/v2/views/${viewId}/search`, + { + body: params, + expectations, + } + ) + } + + publicSearch = async ( + viewId: string, + params?: SearchViewRowRequest, + expectations?: Expectations + ) => { + return await this._post( + `/api/v2/views/${viewId}/search`, + { + body: params, + expectations, + publicUser: true, + } + ) } } diff --git a/packages/types/src/api/web/app/rows.ts b/packages/types/src/api/web/app/rows.ts index 14e28e4a01..0a43182dfd 100644 --- a/packages/types/src/api/web/app/rows.ts +++ b/packages/types/src/api/web/app/rows.ts @@ -1,6 +1,6 @@ import { SearchFilters, SearchParams } from "../../../sdk" import { Row } from "../../../documents" -import { SortOrder } from "../../../api" +import { PaginationResponse, SortOrder } from "../../../api" import { ReadStream } from "fs" export interface SaveRowRequest extends Row {} @@ -31,6 +31,10 @@ export interface SearchRowResponse { rows: any[] } +export interface PaginatedSearchRowResponse + extends SearchRowResponse, + PaginationResponse {} + export interface ExportRowsRequest { rows: string[] columns?: string[] From 90f981724d4172dcaac552e3912a663405309f5b Mon Sep 17 00:00:00 2001 From: Sam Rose Date: Fri, 1 Mar 2024 17:38:21 +0000 Subject: [PATCH 31/38] Fix typing. --- packages/server/src/api/routes/tests/permissions.spec.ts | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/packages/server/src/api/routes/tests/permissions.spec.ts b/packages/server/src/api/routes/tests/permissions.spec.ts index edaa73fd1a..1eabf6edbb 100644 --- a/packages/server/src/api/routes/tests/permissions.spec.ts +++ b/packages/server/src/api/routes/tests/permissions.spec.ts @@ -188,7 +188,7 @@ describe("/permission", () => { await config.publish() const res = await config.api.viewV2.publicSearch(view.id) - expect(res.body.rows[0]._id).toEqual(row._id) + expect(res.rows[0]._id).toEqual(row._id) }) it("should not be able to access the view data when the table is not public and there are no view permissions overrides", async () => { @@ -238,7 +238,7 @@ describe("/permission", () => { await config.publish() const res = await config.api.viewV2.publicSearch(view.id) - expect(res.body.rows[0]._id).toEqual(row._id) + expect(res.rows[0]._id).toEqual(row._id) }) it("shouldn't allow writing from a public user", async () => { From 594cd5ee56bc8f6f42bff279f2ceab747014ff08 Mon Sep 17 00:00:00 2001 From: Sam Rose Date: Fri, 1 Mar 2024 17:40:10 +0000 Subject: [PATCH 32/38] Fix permissions tests. --- packages/server/src/tests/utilities/api/viewV2.ts | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/packages/server/src/tests/utilities/api/viewV2.ts b/packages/server/src/tests/utilities/api/viewV2.ts index dd49360aa8..835431d9d1 100644 --- a/packages/server/src/tests/utilities/api/viewV2.ts +++ b/packages/server/src/tests/utilities/api/viewV2.ts @@ -26,9 +26,14 @@ export class ViewV2API extends TestAPI { ...viewData, } + const exp: Expectations = { + status: 201, + ...expectations, + } + const resp = await this._post<{ data: ViewV2 }>("/api/v2/views", { body: view, - expectations, + expectations: exp, }) return resp.data } From 89a03af92afd443a3c61d7435c0bd22d7c829ca7 Mon Sep 17 00:00:00 2001 From: Sam Rose Date: Mon, 4 Mar 2024 09:20:32 +0000 Subject: [PATCH 33/38] Fix table test. --- packages/server/src/tests/utilities/api/row.ts | 11 +++++++---- 1 file changed, 7 insertions(+), 4 deletions(-) diff --git a/packages/server/src/tests/utilities/api/row.ts b/packages/server/src/tests/utilities/api/row.ts index 58ddb7c049..86664574cb 100644 --- a/packages/server/src/tests/utilities/api/row.ts +++ b/packages/server/src/tests/utilities/api/row.ts @@ -125,10 +125,13 @@ export class RowAPI extends TestAPI { body: BulkImportRequest, expectations?: Expectations ): Promise => { - return await this._post(`/api/${tableId}/rows/import`, { - body, - expectations, - }) + return await this._post( + `/api/tables/${tableId}/import`, + { + body, + expectations, + } + ) } search = async ( From 143daa153cd56c88b684a48d1db05a40851527a4 Mon Sep 17 00:00:00 2001 From: Sam Rose Date: Mon, 4 Mar 2024 09:38:53 +0000 Subject: [PATCH 34/38] Fix ViewV2 tests. --- packages/server/src/tests/utilities/api/viewV2.ts | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/packages/server/src/tests/utilities/api/viewV2.ts b/packages/server/src/tests/utilities/api/viewV2.ts index 835431d9d1..d4539e00b1 100644 --- a/packages/server/src/tests/utilities/api/viewV2.ts +++ b/packages/server/src/tests/utilities/api/viewV2.ts @@ -50,7 +50,11 @@ export class ViewV2API extends TestAPI { } delete = async (viewId: string, expectations?: Expectations) => { - return await this._delete(`/api/v2/views/${viewId}`, { expectations }) + const exp = { + status: 204, + ...expectations, + } + return await this._delete(`/api/v2/views/${viewId}`, { expectations: exp }) } get = async (viewId: string) => { From c39053bb518514849aa801178c0423277886abbe Mon Sep 17 00:00:00 2001 From: Sam Rose Date: Mon, 4 Mar 2024 11:06:25 +0000 Subject: [PATCH 35/38] Respond to PR feedback. --- packages/server/src/api/controllers/permission.ts | 2 +- packages/types/src/api/web/app/permission.ts | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/packages/server/src/api/controllers/permission.ts b/packages/server/src/api/controllers/permission.ts index e12bf3655d..cdfa6d8b1c 100644 --- a/packages/server/src/api/controllers/permission.ts +++ b/packages/server/src/api/controllers/permission.ts @@ -20,7 +20,7 @@ import { import { removeFromArray } from "../../utilities" import sdk from "../../sdk" -enum PermissionUpdateType { +const enum PermissionUpdateType { REMOVE = "remove", ADD = "add", } diff --git a/packages/types/src/api/web/app/permission.ts b/packages/types/src/api/web/app/permission.ts index ebe7a8bea3..88ff4e9d2f 100644 --- a/packages/types/src/api/web/app/permission.ts +++ b/packages/types/src/api/web/app/permission.ts @@ -16,7 +16,7 @@ export interface GetDependantResourcesResponse { } export interface AddedPermission { - _id: string + _id?: string rev?: string error?: string reason?: string From 5fabe14f6261e2ef7c75129ffa2b34941eae4305 Mon Sep 17 00:00:00 2001 From: melohagan <101575380+melohagan@users.noreply.github.com> Date: Mon, 4 Mar 2024 12:20:27 +0000 Subject: [PATCH 36/38] Revert "Rebuild table schema when adding new column to get externalType (#13165)" (#13184) This reverts commit a59647e1580932f6ca278ed933e93c84582a52e5. --- packages/account-portal | 2 +- packages/builder/src/stores/builder/tables.js | 6 - .../src/api/controllers/table/external.ts | 3 +- .../server/src/integration-test/mysql.spec.ts | 309 ------------------ .../src/sdk/app/tables/external/index.ts | 14 +- .../types/src/documents/app/table/table.ts | 3 +- packages/types/src/sdk/search.ts | 4 - 7 files changed, 5 insertions(+), 336 deletions(-) delete mode 100644 packages/server/src/integration-test/mysql.spec.ts diff --git a/packages/account-portal b/packages/account-portal index 806b6fd5c1..19f7a5829f 160000 --- a/packages/account-portal +++ b/packages/account-portal @@ -1 +1 @@ -Subproject commit 806b6fd5c11c284ebf4a01627d75db939f0f8152 +Subproject commit 19f7a5829f4d23cbc694136e45d94482a59a475a diff --git a/packages/builder/src/stores/builder/tables.js b/packages/builder/src/stores/builder/tables.js index f86b37ab85..51b8416eda 100644 --- a/packages/builder/src/stores/builder/tables.js +++ b/packages/builder/src/stores/builder/tables.js @@ -147,12 +147,6 @@ export function createTablesStore() { if (indexes) { draft.indexes = indexes } - // Add object to indicate if column is being added - if (draft.schema[field.name] === undefined) { - draft._add = { - name: field.name, - } - } draft.schema = { ...draft.schema, [field.name]: cloneDeep(field), diff --git a/packages/server/src/api/controllers/table/external.ts b/packages/server/src/api/controllers/table/external.ts index f3478af83b..f035822068 100644 --- a/packages/server/src/api/controllers/table/external.ts +++ b/packages/server/src/api/controllers/table/external.ts @@ -28,7 +28,6 @@ function getDatasourceId(table: Table) { export async function save(ctx: UserCtx) { const inputs = ctx.request.body const renaming = inputs?._rename - const adding = inputs?._add // can't do this right now delete inputs.rows const tableId = ctx.request.body._id @@ -41,7 +40,7 @@ export async function save(ctx: UserCtx) { const { datasource, table } = await sdk.tables.external.save( datasourceId!, inputs, - { tableId, renaming, adding } + { tableId, renaming } ) builderSocket?.emitDatasourceUpdate(ctx, datasource) return table diff --git a/packages/server/src/integration-test/mysql.spec.ts b/packages/server/src/integration-test/mysql.spec.ts deleted file mode 100644 index f7d0388c62..0000000000 --- a/packages/server/src/integration-test/mysql.spec.ts +++ /dev/null @@ -1,309 +0,0 @@ -import fetch from "node-fetch" -import { - generateMakeRequest, - MakeRequestResponse, -} from "../api/routes/public/tests/utils" -import { v4 as uuidv4 } from "uuid" -import * as setup from "../api/routes/tests/utilities" -import { - Datasource, - FieldType, - Table, - TableRequest, - TableSourceType, -} from "@budibase/types" -import _ from "lodash" -import { databaseTestProviders } from "../integrations/tests/utils" -import mysql from "mysql2/promise" -import { builderSocket } from "../websockets" -// @ts-ignore -fetch.mockSearch() - -const config = setup.getConfig()! - -jest.unmock("mysql2/promise") -jest.mock("../websockets", () => ({ - clientAppSocket: jest.fn(), - gridAppSocket: jest.fn(), - initialise: jest.fn(), - builderSocket: { - emitTableUpdate: jest.fn(), - emitTableDeletion: jest.fn(), - emitDatasourceUpdate: jest.fn(), - emitDatasourceDeletion: jest.fn(), - emitScreenUpdate: jest.fn(), - emitAppMetadataUpdate: jest.fn(), - emitAppPublish: jest.fn(), - }, -})) - -describe("mysql integrations", () => { - let makeRequest: MakeRequestResponse, - mysqlDatasource: Datasource, - primaryMySqlTable: Table - - beforeAll(async () => { - await config.init() - const apiKey = await config.generateApiKey() - - makeRequest = generateMakeRequest(apiKey, true) - - mysqlDatasource = await config.api.datasource.create( - await databaseTestProviders.mysql.datasource() - ) - }) - - afterAll(async () => { - await databaseTestProviders.mysql.stop() - }) - - beforeEach(async () => { - primaryMySqlTable = await config.createTable({ - name: uuidv4(), - type: "table", - primary: ["id"], - schema: { - id: { - name: "id", - type: FieldType.AUTO, - autocolumn: true, - }, - name: { - name: "name", - type: FieldType.STRING, - }, - description: { - name: "description", - type: FieldType.STRING, - }, - value: { - name: "value", - type: FieldType.NUMBER, - }, - }, - sourceId: mysqlDatasource._id, - sourceType: TableSourceType.EXTERNAL, - }) - }) - - afterAll(config.end) - - it("validate table schema", async () => { - const res = await makeRequest( - "get", - `/api/datasources/${mysqlDatasource._id}` - ) - - expect(res.status).toBe(200) - expect(res.body).toEqual({ - config: { - database: "mysql", - host: mysqlDatasource.config!.host, - password: "--secret-value--", - port: mysqlDatasource.config!.port, - user: "root", - }, - plus: true, - source: "MYSQL", - type: "datasource_plus", - _id: expect.any(String), - _rev: expect.any(String), - createdAt: expect.any(String), - updatedAt: expect.any(String), - entities: expect.any(Object), - }) - }) - - describe("POST /api/datasources/verify", () => { - it("should be able to verify the connection", async () => { - const response = await config.api.datasource.verify({ - datasource: await databaseTestProviders.mysql.datasource(), - }) - expect(response.status).toBe(200) - expect(response.body.connected).toBe(true) - }) - - it("should state an invalid datasource cannot connect", async () => { - const dbConfig = await databaseTestProviders.mysql.datasource() - const response = await config.api.datasource.verify({ - datasource: { - ...dbConfig, - config: { - ...dbConfig.config, - password: "wrongpassword", - }, - }, - }) - - expect(response.status).toBe(200) - expect(response.body.connected).toBe(false) - expect(response.body.error).toBeDefined() - }) - }) - - describe("POST /api/datasources/info", () => { - it("should fetch information about mysql datasource", async () => { - const primaryName = primaryMySqlTable.name - const response = await makeRequest("post", "/api/datasources/info", { - datasource: mysqlDatasource, - }) - expect(response.status).toBe(200) - expect(response.body.tableNames).toBeDefined() - expect(response.body.tableNames.indexOf(primaryName)).not.toBe(-1) - }) - }) - - describe("Integration compatibility with mysql search_path", () => { - let client: mysql.Connection, pathDatasource: Datasource - const database = "test1" - const database2 = "test-2" - - beforeAll(async () => { - const dsConfig = await databaseTestProviders.mysql.datasource() - const dbConfig = dsConfig.config! - - client = await mysql.createConnection(dbConfig) - await client.query(`CREATE DATABASE \`${database}\`;`) - await client.query(`CREATE DATABASE \`${database2}\`;`) - - const pathConfig: any = { - ...dsConfig, - config: { - ...dbConfig, - database, - }, - } - pathDatasource = await config.api.datasource.create(pathConfig) - }) - - afterAll(async () => { - await client.query(`DROP DATABASE \`${database}\`;`) - await client.query(`DROP DATABASE \`${database2}\`;`) - await client.end() - }) - - it("discovers tables from any schema in search path", async () => { - await client.query( - `CREATE TABLE \`${database}\`.table1 (id1 SERIAL PRIMARY KEY);` - ) - const response = await makeRequest("post", "/api/datasources/info", { - datasource: pathDatasource, - }) - expect(response.status).toBe(200) - expect(response.body.tableNames).toBeDefined() - expect(response.body.tableNames).toEqual( - expect.arrayContaining(["table1"]) - ) - }) - - it("does not mix columns from different tables", async () => { - const repeated_table_name = "table_same_name" - await client.query( - `CREATE TABLE \`${database}\`.${repeated_table_name} (id SERIAL PRIMARY KEY, val1 TEXT);` - ) - await client.query( - `CREATE TABLE \`${database2}\`.${repeated_table_name} (id2 SERIAL PRIMARY KEY, val2 TEXT);` - ) - const response = await makeRequest( - "post", - `/api/datasources/${pathDatasource._id}/schema`, - { - tablesFilter: [repeated_table_name], - } - ) - expect(response.status).toBe(200) - expect( - response.body.datasource.entities[repeated_table_name].schema - ).toBeDefined() - const schema = - response.body.datasource.entities[repeated_table_name].schema - expect(Object.keys(schema).sort()).toEqual(["id", "val1"]) - }) - }) - - describe("POST /api/tables/", () => { - let client: mysql.Connection - const emitDatasourceUpdateMock = jest.fn() - - beforeEach(async () => { - client = await mysql.createConnection( - ( - await databaseTestProviders.mysql.datasource() - ).config! - ) - }) - - afterEach(async () => { - await client.end() - }) - - it("will emit the datasource entity schema with externalType to the front-end when adding a new column", async () => { - mysqlDatasource = ( - await makeRequest( - "post", - `/api/datasources/${mysqlDatasource._id}/schema` - ) - ).body.datasource - - const addColumnToTable: TableRequest = { - type: "table", - sourceType: TableSourceType.EXTERNAL, - name: "table", - sourceId: mysqlDatasource._id!, - primary: ["id"], - schema: { - id: { - type: FieldType.AUTO, - name: "id", - autocolumn: true, - }, - new_column: { - type: FieldType.NUMBER, - name: "new_column", - }, - }, - _add: { - name: "new_column", - }, - } - - jest - .spyOn(builderSocket!, "emitDatasourceUpdate") - .mockImplementation(emitDatasourceUpdateMock) - - await makeRequest("post", "/api/tables/", addColumnToTable) - - const expectedTable: TableRequest = { - ...addColumnToTable, - schema: { - id: { - type: FieldType.NUMBER, - name: "id", - autocolumn: true, - constraints: { - presence: false, - }, - externalType: "int unsigned", - }, - new_column: { - type: FieldType.NUMBER, - name: "new_column", - autocolumn: false, - constraints: { - presence: false, - }, - externalType: "float(8,2)", - }, - }, - created: true, - _id: `${mysqlDatasource._id}__table`, - } - delete expectedTable._add - - expect(emitDatasourceUpdateMock).toBeCalledTimes(1) - const emittedDatasource: Datasource = - emitDatasourceUpdateMock.mock.calls[0][1] - expect(emittedDatasource.entities!["table"]).toEqual(expectedTable) - }) - }) -}) diff --git a/packages/server/src/sdk/app/tables/external/index.ts b/packages/server/src/sdk/app/tables/external/index.ts index 0ace19d00e..9a2bed0da2 100644 --- a/packages/server/src/sdk/app/tables/external/index.ts +++ b/packages/server/src/sdk/app/tables/external/index.ts @@ -3,7 +3,6 @@ import { Operation, RelationshipType, RenameColumn, - AddColumn, Table, TableRequest, ViewV2, @@ -33,7 +32,7 @@ import * as viewSdk from "../../views" export async function save( datasourceId: string, update: Table, - opts?: { tableId?: string; renaming?: RenameColumn; adding?: AddColumn } + opts?: { tableId?: string; renaming?: RenameColumn } ) { let tableToSave: TableRequest = { ...update, @@ -166,17 +165,8 @@ export async function save( // remove the rename prop delete tableToSave._rename - - // if adding a new column, we need to rebuild the schema for that table to get the 'externalType' of the column - if (opts?.adding) { - datasource.entities[tableToSave.name] = ( - await datasourceSdk.buildFilteredSchema(datasource, [tableToSave.name]) - ).tables[tableToSave.name] - } else { - datasource.entities[tableToSave.name] = tableToSave - } - // store it into couch now for budibase reference + datasource.entities[tableToSave.name] = tableToSave await db.put(populateExternalTableSchemas(datasource)) // Since tables are stored inside datasources, we need to notify clients diff --git a/packages/types/src/documents/app/table/table.ts b/packages/types/src/documents/app/table/table.ts index 3b419dd811..f3b8e6df8d 100644 --- a/packages/types/src/documents/app/table/table.ts +++ b/packages/types/src/documents/app/table/table.ts @@ -1,6 +1,6 @@ import { Document } from "../../document" import { View, ViewV2 } from "../view" -import { AddColumn, RenameColumn } from "../../../sdk" +import { RenameColumn } from "../../../sdk" import { TableSchema } from "./schema" export const INTERNAL_TABLE_SOURCE_ID = "bb_internal" @@ -29,6 +29,5 @@ export interface Table extends Document { export interface TableRequest extends Table { _rename?: RenameColumn - _add?: AddColumn created?: boolean } diff --git a/packages/types/src/sdk/search.ts b/packages/types/src/sdk/search.ts index 7a0ddaed66..67c344d845 100644 --- a/packages/types/src/sdk/search.ts +++ b/packages/types/src/sdk/search.ts @@ -60,10 +60,6 @@ export interface RenameColumn { updated: string } -export interface AddColumn { - name: string -} - export interface RelationshipsJson { through?: string from?: string From 1857383c47428e060b731a4207e61392b5bdde45 Mon Sep 17 00:00:00 2001 From: Sam Rose Date: Mon, 4 Mar 2024 13:37:41 +0000 Subject: [PATCH 37/38] Constrain query execution response slightly based on PR feedback. --- .../server/src/api/controllers/query/index.ts | 18 +++++++++++++----- packages/types/src/api/web/query.ts | 2 +- 2 files changed, 14 insertions(+), 6 deletions(-) diff --git a/packages/server/src/api/controllers/query/index.ts b/packages/server/src/api/controllers/query/index.ts index 725de41c9a..3c21537484 100644 --- a/packages/server/src/api/controllers/query/index.ts +++ b/packages/server/src/api/controllers/query/index.ts @@ -297,7 +297,10 @@ export async function preview( } async function execute( - ctx: UserCtx, + ctx: UserCtx< + ExecuteQueryRequest, + ExecuteQueryResponse | Record[] + >, opts: any = { rowsOnly: false, isAutomation: false } ) { const db = context.getAppDB() @@ -352,18 +355,23 @@ async function execute( } } -export async function executeV1(ctx: UserCtx) { +export async function executeV1( + ctx: UserCtx[]> +) { return execute(ctx, { rowsOnly: true, isAutomation: false }) } export async function executeV2( - ctx: UserCtx, + ctx: UserCtx< + ExecuteQueryRequest, + ExecuteQueryResponse | Record[] + >, { isAutomation }: { isAutomation?: boolean } = {} ) { return execute(ctx, { rowsOnly: false, isAutomation }) } -const removeDynamicVariables = async (queryId: any) => { +const removeDynamicVariables = async (queryId: string) => { const db = context.getAppDB() const query = await db.get(queryId) const datasource = await sdk.datasources.get(query.datasourceId) @@ -386,7 +394,7 @@ const removeDynamicVariables = async (queryId: any) => { export async function destroy(ctx: UserCtx) { const db = context.getAppDB() - const queryId = ctx.params.queryId + const queryId = ctx.params.queryId as string await removeDynamicVariables(queryId) const query = await db.get(queryId) const datasource = await sdk.datasources.get(query.datasourceId) diff --git a/packages/types/src/api/web/query.ts b/packages/types/src/api/web/query.ts index 40f4426d3e..3959cdea19 100644 --- a/packages/types/src/api/web/query.ts +++ b/packages/types/src/api/web/query.ts @@ -16,5 +16,5 @@ export interface ExecuteQueryRequest { } export interface ExecuteQueryResponse { - data: any[] + data: Record[] } From ee0f0abad25d9e1bcfc499eb2e8b7d4ed3d7b38f Mon Sep 17 00:00:00 2001 From: melohagan <101575380+melohagan@users.noreply.github.com> Date: Mon, 4 Mar 2024 15:55:45 +0000 Subject: [PATCH 38/38] Fix/rename mysql column (#13186) * Rebuild table schema when adding new column to get externalType * Added MySQL integration test suite * Add test for emitting datasource on save new column * Update packages/server/src/integration-test/mysql.spec.ts Co-authored-by: Sam Rose * remove duplicate tests * Use UUID * update account portal * Remove _add for internal save * Internal DB add column unit test * rename column test * update modules * fix tests --------- Co-authored-by: Sam Rose --- packages/account-portal | 2 +- packages/builder/src/stores/builder/tables.js | 6 + packages/pro | 2 +- .../src/api/controllers/table/external.ts | 10 +- .../server/src/api/controllers/table/index.ts | 9 +- .../src/api/controllers/table/internal.ts | 14 +- .../server/src/api/routes/tests/table.spec.ts | 30 ++ .../server/src/integration-test/mysql.spec.ts | 363 ++++++++++++++++++ .../src/sdk/app/tables/external/index.ts | 14 +- .../types/src/documents/app/table/table.ts | 3 +- packages/types/src/sdk/search.ts | 4 + 11 files changed, 440 insertions(+), 17 deletions(-) create mode 100644 packages/server/src/integration-test/mysql.spec.ts diff --git a/packages/account-portal b/packages/account-portal index 19f7a5829f..0c050591c2 160000 --- a/packages/account-portal +++ b/packages/account-portal @@ -1 +1 @@ -Subproject commit 19f7a5829f4d23cbc694136e45d94482a59a475a +Subproject commit 0c050591c21d3b67dc0c9225d60cc9e2324c8dac diff --git a/packages/builder/src/stores/builder/tables.js b/packages/builder/src/stores/builder/tables.js index 51b8416eda..f86b37ab85 100644 --- a/packages/builder/src/stores/builder/tables.js +++ b/packages/builder/src/stores/builder/tables.js @@ -147,6 +147,12 @@ export function createTablesStore() { if (indexes) { draft.indexes = indexes } + // Add object to indicate if column is being added + if (draft.schema[field.name] === undefined) { + draft._add = { + name: field.name, + } + } draft.schema = { ...draft.schema, [field.name]: cloneDeep(field), diff --git a/packages/pro b/packages/pro index 183b35d3ac..22a278da72 160000 --- a/packages/pro +++ b/packages/pro @@ -1 +1 @@ -Subproject commit 183b35d3acd42433dcb2d32bcd89a36abe13afec +Subproject commit 22a278da720d92991dabdcd4cb6c96e7abe29781 diff --git a/packages/server/src/api/controllers/table/external.ts b/packages/server/src/api/controllers/table/external.ts index f035822068..c85b46a95c 100644 --- a/packages/server/src/api/controllers/table/external.ts +++ b/packages/server/src/api/controllers/table/external.ts @@ -6,6 +6,7 @@ import { BulkImportRequest, BulkImportResponse, Operation, + RenameColumn, SaveTableRequest, SaveTableResponse, Table, @@ -25,9 +26,12 @@ function getDatasourceId(table: Table) { return breakExternalTableId(table._id).datasourceId } -export async function save(ctx: UserCtx) { +export async function save( + ctx: UserCtx, + renaming?: RenameColumn +) { const inputs = ctx.request.body - const renaming = inputs?._rename + const adding = inputs?._add // can't do this right now delete inputs.rows const tableId = ctx.request.body._id @@ -40,7 +44,7 @@ export async function save(ctx: UserCtx) { const { datasource, table } = await sdk.tables.external.save( datasourceId!, inputs, - { tableId, renaming } + { tableId, renaming, adding } ) builderSocket?.emitDatasourceUpdate(ctx, datasource) return table diff --git a/packages/server/src/api/controllers/table/index.ts b/packages/server/src/api/controllers/table/index.ts index 55a896373f..69305c461e 100644 --- a/packages/server/src/api/controllers/table/index.ts +++ b/packages/server/src/api/controllers/table/index.ts @@ -74,8 +74,15 @@ export async function save(ctx: UserCtx) { const appId = ctx.appId const table = ctx.request.body const isImport = table.rows + const renaming = ctx.request.body._rename - let savedTable = await pickApi({ table }).save(ctx) + const api = pickApi({ table }) + // do not pass _rename or _add if saving to CouchDB + if (api === internal) { + delete ctx.request.body._add + delete ctx.request.body._rename + } + let savedTable = await api.save(ctx, renaming) if (!table._id) { await events.table.created(savedTable) savedTable = sdk.tables.enrichViewSchemas(savedTable) diff --git a/packages/server/src/api/controllers/table/internal.ts b/packages/server/src/api/controllers/table/internal.ts index 8e90007d88..eb5e4b6c41 100644 --- a/packages/server/src/api/controllers/table/internal.ts +++ b/packages/server/src/api/controllers/table/internal.ts @@ -12,11 +12,12 @@ import { } from "@budibase/types" import sdk from "../../../sdk" -export async function save(ctx: UserCtx) { +export async function save( + ctx: UserCtx, + renaming?: RenameColumn +) { const { rows, ...rest } = ctx.request.body - let tableToSave: Table & { - _rename?: RenameColumn - } = { + let tableToSave: Table = { _id: generateTableID(), ...rest, // Ensure these fields are populated, even if not sent in the request @@ -28,15 +29,12 @@ export async function save(ctx: UserCtx) { tableToSave.views = {} } - const renaming = tableToSave._rename - delete tableToSave._rename - try { const { table } = await sdk.tables.internal.save(tableToSave, { user: ctx.user, rowsToImport: rows, tableId: ctx.request.body._id, - renaming: renaming, + renaming, }) return table diff --git a/packages/server/src/api/routes/tests/table.spec.ts b/packages/server/src/api/routes/tests/table.spec.ts index 29465145a9..77704a0408 100644 --- a/packages/server/src/api/routes/tests/table.spec.ts +++ b/packages/server/src/api/routes/tests/table.spec.ts @@ -26,6 +26,7 @@ import { TableToBuild } from "../../../tests/utilities/TestConfiguration" tk.freeze(mocks.date.MOCK_DATE) const { basicTable } = setup.structures +const ISO_REGEX_PATTERN = /^\d{4}-\d{2}-\d{2}T\d{2}:\d{2}:\d{2}\.\d{3}Z$/ describe("/tables", () => { let request = setup.getRequest() @@ -285,6 +286,35 @@ describe("/tables", () => { expect(res.body.schema.roleId).toBeDefined() }) }) + + it("should add a new column for an internal DB table", async () => { + const saveTableRequest: SaveTableRequest = { + _add: { + name: "NEW_COLUMN", + }, + ...basicTable(), + } + + const response = await request + .post(`/api/tables`) + .send(saveTableRequest) + .set(config.defaultHeaders()) + .expect("Content-Type", /json/) + .expect(200) + + const expectedResponse = { + ...saveTableRequest, + _rev: expect.stringMatching(/^\d-.+/), + _id: expect.stringMatching(/^ta_.+/), + createdAt: expect.stringMatching(ISO_REGEX_PATTERN), + updatedAt: expect.stringMatching(ISO_REGEX_PATTERN), + views: {}, + } + delete expectedResponse._add + + expect(response.status).toBe(200) + expect(response.body).toEqual(expectedResponse) + }) }) describe("import", () => { diff --git a/packages/server/src/integration-test/mysql.spec.ts b/packages/server/src/integration-test/mysql.spec.ts new file mode 100644 index 0000000000..fac2bfcfeb --- /dev/null +++ b/packages/server/src/integration-test/mysql.spec.ts @@ -0,0 +1,363 @@ +import fetch from "node-fetch" +import { + generateMakeRequest, + MakeRequestResponse, +} from "../api/routes/public/tests/utils" +import { v4 as uuidv4 } from "uuid" +import * as setup from "../api/routes/tests/utilities" +import { + Datasource, + FieldType, + Table, + TableRequest, + TableSourceType, +} from "@budibase/types" +import _ from "lodash" +import { databaseTestProviders } from "../integrations/tests/utils" +import mysql from "mysql2/promise" +import { builderSocket } from "../websockets" +// @ts-ignore +fetch.mockSearch() + +const config = setup.getConfig()! + +jest.unmock("mysql2/promise") +jest.mock("../websockets", () => ({ + clientAppSocket: jest.fn(), + gridAppSocket: jest.fn(), + initialise: jest.fn(), + builderSocket: { + emitTableUpdate: jest.fn(), + emitTableDeletion: jest.fn(), + emitDatasourceUpdate: jest.fn(), + emitDatasourceDeletion: jest.fn(), + emitScreenUpdate: jest.fn(), + emitAppMetadataUpdate: jest.fn(), + emitAppPublish: jest.fn(), + }, +})) + +describe("mysql integrations", () => { + let makeRequest: MakeRequestResponse, + mysqlDatasource: Datasource, + primaryMySqlTable: Table + + beforeAll(async () => { + await config.init() + const apiKey = await config.generateApiKey() + + makeRequest = generateMakeRequest(apiKey, true) + + mysqlDatasource = await config.api.datasource.create( + await databaseTestProviders.mysql.datasource() + ) + }) + + afterAll(async () => { + await databaseTestProviders.mysql.stop() + }) + + beforeEach(async () => { + primaryMySqlTable = await config.createTable({ + name: uuidv4(), + type: "table", + primary: ["id"], + schema: { + id: { + name: "id", + type: FieldType.AUTO, + autocolumn: true, + }, + name: { + name: "name", + type: FieldType.STRING, + }, + description: { + name: "description", + type: FieldType.STRING, + }, + value: { + name: "value", + type: FieldType.NUMBER, + }, + }, + sourceId: mysqlDatasource._id, + sourceType: TableSourceType.EXTERNAL, + }) + }) + + afterAll(config.end) + + it("validate table schema", async () => { + const res = await makeRequest( + "get", + `/api/datasources/${mysqlDatasource._id}` + ) + + expect(res.status).toBe(200) + expect(res.body).toEqual({ + config: { + database: "mysql", + host: mysqlDatasource.config!.host, + password: "--secret-value--", + port: mysqlDatasource.config!.port, + user: "root", + }, + plus: true, + source: "MYSQL", + type: "datasource_plus", + _id: expect.any(String), + _rev: expect.any(String), + createdAt: expect.any(String), + updatedAt: expect.any(String), + entities: expect.any(Object), + }) + }) + + describe("POST /api/datasources/verify", () => { + it("should be able to verify the connection", async () => { + await config.api.datasource.verify( + { + datasource: await databaseTestProviders.mysql.datasource(), + }, + { + body: { + connected: true, + }, + } + ) + }) + + it("should state an invalid datasource cannot connect", async () => { + const dbConfig = await databaseTestProviders.mysql.datasource() + await config.api.datasource.verify( + { + datasource: { + ...dbConfig, + config: { + ...dbConfig.config, + password: "wrongpassword", + }, + }, + }, + { + body: { + connected: false, + error: + "Access denied for the specified user. User does not have the necessary privileges or the provided credentials are incorrect. Please verify the credentials, and ensure that the user has appropriate permissions.", + }, + } + ) + }) + }) + + describe("POST /api/datasources/info", () => { + it("should fetch information about mysql datasource", async () => { + const primaryName = primaryMySqlTable.name + const response = await makeRequest("post", "/api/datasources/info", { + datasource: mysqlDatasource, + }) + expect(response.status).toBe(200) + expect(response.body.tableNames).toBeDefined() + expect(response.body.tableNames.indexOf(primaryName)).not.toBe(-1) + }) + }) + + describe("Integration compatibility with mysql search_path", () => { + let client: mysql.Connection, pathDatasource: Datasource + const database = "test1" + const database2 = "test-2" + + beforeAll(async () => { + const dsConfig = await databaseTestProviders.mysql.datasource() + const dbConfig = dsConfig.config! + + client = await mysql.createConnection(dbConfig) + await client.query(`CREATE DATABASE \`${database}\`;`) + await client.query(`CREATE DATABASE \`${database2}\`;`) + + const pathConfig: any = { + ...dsConfig, + config: { + ...dbConfig, + database, + }, + } + pathDatasource = await config.api.datasource.create(pathConfig) + }) + + afterAll(async () => { + await client.query(`DROP DATABASE \`${database}\`;`) + await client.query(`DROP DATABASE \`${database2}\`;`) + await client.end() + }) + + it("discovers tables from any schema in search path", async () => { + await client.query( + `CREATE TABLE \`${database}\`.table1 (id1 SERIAL PRIMARY KEY);` + ) + const response = await makeRequest("post", "/api/datasources/info", { + datasource: pathDatasource, + }) + expect(response.status).toBe(200) + expect(response.body.tableNames).toBeDefined() + expect(response.body.tableNames).toEqual( + expect.arrayContaining(["table1"]) + ) + }) + + it("does not mix columns from different tables", async () => { + const repeated_table_name = "table_same_name" + await client.query( + `CREATE TABLE \`${database}\`.${repeated_table_name} (id SERIAL PRIMARY KEY, val1 TEXT);` + ) + await client.query( + `CREATE TABLE \`${database2}\`.${repeated_table_name} (id2 SERIAL PRIMARY KEY, val2 TEXT);` + ) + const response = await makeRequest( + "post", + `/api/datasources/${pathDatasource._id}/schema`, + { + tablesFilter: [repeated_table_name], + } + ) + expect(response.status).toBe(200) + expect( + response.body.datasource.entities[repeated_table_name].schema + ).toBeDefined() + const schema = + response.body.datasource.entities[repeated_table_name].schema + expect(Object.keys(schema).sort()).toEqual(["id", "val1"]) + }) + }) + + describe("POST /api/tables/", () => { + let client: mysql.Connection + const emitDatasourceUpdateMock = jest.fn() + + beforeEach(async () => { + client = await mysql.createConnection( + ( + await databaseTestProviders.mysql.datasource() + ).config! + ) + mysqlDatasource = await config.api.datasource.create( + await databaseTestProviders.mysql.datasource() + ) + }) + + afterEach(async () => { + await client.end() + }) + + it("will emit the datasource entity schema with externalType to the front-end when adding a new column", async () => { + const addColumnToTable: TableRequest = { + type: "table", + sourceType: TableSourceType.EXTERNAL, + name: "table", + sourceId: mysqlDatasource._id!, + primary: ["id"], + schema: { + id: { + type: FieldType.AUTO, + name: "id", + autocolumn: true, + }, + new_column: { + type: FieldType.NUMBER, + name: "new_column", + }, + }, + _add: { + name: "new_column", + }, + } + + jest + .spyOn(builderSocket!, "emitDatasourceUpdate") + .mockImplementation(emitDatasourceUpdateMock) + + await makeRequest("post", "/api/tables/", addColumnToTable) + + const expectedTable: TableRequest = { + ...addColumnToTable, + schema: { + id: { + type: FieldType.NUMBER, + name: "id", + autocolumn: true, + constraints: { + presence: false, + }, + externalType: "int unsigned", + }, + new_column: { + type: FieldType.NUMBER, + name: "new_column", + autocolumn: false, + constraints: { + presence: false, + }, + externalType: "float(8,2)", + }, + }, + created: true, + _id: `${mysqlDatasource._id}__table`, + } + delete expectedTable._add + + expect(emitDatasourceUpdateMock).toBeCalledTimes(1) + const emittedDatasource: Datasource = + emitDatasourceUpdateMock.mock.calls[0][1] + expect(emittedDatasource.entities!["table"]).toEqual(expectedTable) + }) + + it("will rename a column", async () => { + await makeRequest("post", "/api/tables/", primaryMySqlTable) + + let renameColumnOnTable: TableRequest = { + ...primaryMySqlTable, + schema: { + id: { + name: "id", + type: FieldType.AUTO, + autocolumn: true, + externalType: "unsigned integer", + }, + name: { + name: "name", + type: FieldType.STRING, + externalType: "text", + }, + description: { + name: "description", + type: FieldType.STRING, + externalType: "text", + }, + age: { + name: "age", + type: FieldType.NUMBER, + externalType: "float(8,2)", + }, + }, + } + + const response = await makeRequest( + "post", + "/api/tables/", + renameColumnOnTable + ) + mysqlDatasource = ( + await makeRequest( + "post", + `/api/datasources/${mysqlDatasource._id}/schema` + ) + ).body.datasource + + expect(response.status).toEqual(200) + expect( + Object.keys(mysqlDatasource.entities![primaryMySqlTable.name].schema) + ).toEqual(["id", "name", "description", "age"]) + }) + }) +}) diff --git a/packages/server/src/sdk/app/tables/external/index.ts b/packages/server/src/sdk/app/tables/external/index.ts index 9a2bed0da2..0ace19d00e 100644 --- a/packages/server/src/sdk/app/tables/external/index.ts +++ b/packages/server/src/sdk/app/tables/external/index.ts @@ -3,6 +3,7 @@ import { Operation, RelationshipType, RenameColumn, + AddColumn, Table, TableRequest, ViewV2, @@ -32,7 +33,7 @@ import * as viewSdk from "../../views" export async function save( datasourceId: string, update: Table, - opts?: { tableId?: string; renaming?: RenameColumn } + opts?: { tableId?: string; renaming?: RenameColumn; adding?: AddColumn } ) { let tableToSave: TableRequest = { ...update, @@ -165,8 +166,17 @@ export async function save( // remove the rename prop delete tableToSave._rename + + // if adding a new column, we need to rebuild the schema for that table to get the 'externalType' of the column + if (opts?.adding) { + datasource.entities[tableToSave.name] = ( + await datasourceSdk.buildFilteredSchema(datasource, [tableToSave.name]) + ).tables[tableToSave.name] + } else { + datasource.entities[tableToSave.name] = tableToSave + } + // store it into couch now for budibase reference - datasource.entities[tableToSave.name] = tableToSave await db.put(populateExternalTableSchemas(datasource)) // Since tables are stored inside datasources, we need to notify clients diff --git a/packages/types/src/documents/app/table/table.ts b/packages/types/src/documents/app/table/table.ts index f3b8e6df8d..3b419dd811 100644 --- a/packages/types/src/documents/app/table/table.ts +++ b/packages/types/src/documents/app/table/table.ts @@ -1,6 +1,6 @@ import { Document } from "../../document" import { View, ViewV2 } from "../view" -import { RenameColumn } from "../../../sdk" +import { AddColumn, RenameColumn } from "../../../sdk" import { TableSchema } from "./schema" export const INTERNAL_TABLE_SOURCE_ID = "bb_internal" @@ -29,5 +29,6 @@ export interface Table extends Document { export interface TableRequest extends Table { _rename?: RenameColumn + _add?: AddColumn created?: boolean } diff --git a/packages/types/src/sdk/search.ts b/packages/types/src/sdk/search.ts index 67c344d845..7a0ddaed66 100644 --- a/packages/types/src/sdk/search.ts +++ b/packages/types/src/sdk/search.ts @@ -60,6 +60,10 @@ export interface RenameColumn { updated: string } +export interface AddColumn { + name: string +} + export interface RelationshipsJson { through?: string from?: string