From aaa1603d1b54ce9587727a4930ebc9f0c3545535 Mon Sep 17 00:00:00 2001 From: Andrew Kingston Date: Tue, 25 Feb 2025 15:55:13 +0000 Subject: [PATCH 01/17] Rewrite relationship field --- .../app/forms/RelationshipField.svelte | 443 +++++++++++------- 1 file changed, 276 insertions(+), 167 deletions(-) diff --git a/packages/client/src/components/app/forms/RelationshipField.svelte b/packages/client/src/components/app/forms/RelationshipField.svelte index 54171309de..5da03eba2b 100644 --- a/packages/client/src/components/app/forms/RelationshipField.svelte +++ b/packages/client/src/components/app/forms/RelationshipField.svelte @@ -1,17 +1,23 @@ {#if fieldState} option.primaryDisplay} getOptionValue={option => option._id} + {options} {placeholder} + {autocomplete} bind:searchTerm - loading={$fetch.loading} bind:open + on:change={handleChange} + on:loadMore={() => fetch?.nextPage()} /> {/if} From bf673bff71231a6c6b5d5307a7ccb401dd1910bf Mon Sep 17 00:00:00 2001 From: Andrew Kingston Date: Tue, 25 Feb 2025 16:20:58 +0000 Subject: [PATCH 02/17] Fix filtering options --- .../app/forms/RelationshipField.svelte | 33 ++++++++++--------- 1 file changed, 17 insertions(+), 16 deletions(-) diff --git a/packages/client/src/components/app/forms/RelationshipField.svelte b/packages/client/src/components/app/forms/RelationshipField.svelte index 5da03eba2b..914d552e06 100644 --- a/packages/client/src/components/app/forms/RelationshipField.svelte +++ b/packages/client/src/components/app/forms/RelationshipField.svelte @@ -89,8 +89,8 @@ $: updateOptions(optionsMap) $: !open && sortOptions() - // Fetch new options when search term changes - $: debouncedFetchRows(searchTerm, primaryDisplay) + // Search for new options when search term changes + $: debouncedSearchOptions(searchTerm || "", primaryDisplayField) // Ensure backwards compatibility $: enrichedDefaultValue = enrichDefaultValue(defaultValue) @@ -310,28 +310,29 @@ return val.includes(",") ? val.split(",") : val } - async function fetchRows(searchTerm: any, primaryDisplay: string) { - // Don't request until we have the primary display or default value has been fetched + // Searches for new options matching the given term + async function searchOptions(searchTerm: string, primaryDisplay?: string) { if (!primaryDisplay) { return } // Ensure we match all filters, rather than any - // @ts-expect-error this doesn't fit types, but don't want to change it yet - const baseFilter: any = (filter || []).filter(x => x.operator !== "allOr") + let newFilter: any = filter + if (searchTerm) { + // @ts-expect-error this doesn't fit types, but don't want to change it yet + newFilter = (newFilter || []).filter(x => x.operator !== "allOr") + newFilter.push({ + // Use a big numeric prefix to avoid clashing with an existing filter + field: `999:${primaryDisplay}`, + operator: "string", + value: searchTerm, + }) + } await fetch?.update({ - filter: [ - ...baseFilter, - { - // Use a big numeric prefix to avoid clashing with an existing filter - field: `999:${primaryDisplay}`, - operator: "string", - value: searchTerm, - }, - ], + filter: newFilter, }) } - const debouncedFetchRows = Utils.debounce(fetchRows, 250) + const debouncedSearchOptions = Utils.debounce(searchOptions, 250) // Flattens an array of row-like objects into a simple array of row IDs const flatten = (values: any | any[]): string[] => { From 24fd3e5c860d294e04ea3825db70933149cbc88e Mon Sep 17 00:00:00 2001 From: Andrew Kingston Date: Tue, 25 Feb 2025 16:50:28 +0000 Subject: [PATCH 03/17] Use limit of 1 rather than 0 for readonly fields, as 0 seems to be treated as max page size --- .../client/src/components/app/forms/RelationshipField.svelte | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/client/src/components/app/forms/RelationshipField.svelte b/packages/client/src/components/app/forms/RelationshipField.svelte index 914d552e06..0226e7d7f3 100644 --- a/packages/client/src/components/app/forms/RelationshipField.svelte +++ b/packages/client/src/components/app/forms/RelationshipField.svelte @@ -135,7 +135,7 @@ datasource, options: { filter, - limit: writable ? 100 : 0, + limit: writable ? 100 : 1, }, }) } From 433c20d80cbd1e48b5bebddf387843a2787c41e1 Mon Sep 17 00:00:00 2001 From: Sam Rose Date: Tue, 25 Feb 2025 18:23:29 +0000 Subject: [PATCH 04/17] wip --- .../src/api/routes/public/tests/users.spec.ts | 79 ++++++++++--------- .../src/api/routes/tests/utilities/index.ts | 38 --------- .../src/tests/utilities/TestConfiguration.ts | 32 +++++--- .../server/src/tests/utilities/api/base.ts | 56 +++++++++---- .../server/src/tests/utilities/api/index.ts | 8 ++ .../src/tests/utilities/api/public/user.ts | 19 +++++ 6 files changed, 129 insertions(+), 103 deletions(-) create mode 100644 packages/server/src/tests/utilities/api/public/user.ts diff --git a/packages/server/src/api/routes/public/tests/users.spec.ts b/packages/server/src/api/routes/public/tests/users.spec.ts index 4ca9ff8104..4983761c3a 100644 --- a/packages/server/src/api/routes/public/tests/users.spec.ts +++ b/packages/server/src/api/routes/public/tests/users.spec.ts @@ -1,27 +1,44 @@ import * as setup from "../../tests/utilities" -import { generateMakeRequest, MakeRequestResponse } from "./utils" import { User } from "@budibase/types" import { mocks } from "@budibase/backend-core/tests" +import nock from "nock" +import environment from "../../../../environment" +import TestConfiguration from "../../../../tests/utilities/TestConfiguration" -import * as workerRequests from "../../../../utilities/workerRequests" - -const mockedWorkerReq = jest.mocked(workerRequests) - -let config = setup.getConfig() -let apiKey: string, globalUser: User, makeRequest: MakeRequestResponse +const config = new TestConfiguration() +let globalUser: User beforeAll(async () => { await config.init() - globalUser = await config.globalUser() - apiKey = await config.generateApiKey(globalUser._id) - makeRequest = generateMakeRequest(apiKey) - mockedWorkerReq.readGlobalUser.mockImplementation(() => - Promise.resolve(globalUser) - ) }) afterAll(setup.afterAll) +beforeEach(async () => { + globalUser = await config.globalUser() + + nock.cleanAll() + nock(environment.WORKER_URL!) + .get(`/api/global/users/${globalUser._id}`) + .reply(200, (uri, body) => { + return globalUser + }) + .persist() + + nock(environment.WORKER_URL!) + .post(`/api/global/users`) + .reply(200, (uri, body) => { + const updatedUser = body as User + if (updatedUser._id === globalUser._id) { + globalUser = updatedUser + return globalUser + } else { + throw new Error("User not found") + } + }) + .persist() +}) + function base() { return { tenantId: config.getTenantId(), @@ -30,37 +47,26 @@ function base() { } } -function updateMock() { - mockedWorkerReq.readGlobalUser.mockImplementation(ctx => ctx.request.body) -} - -describe("check user endpoints", () => { +describe.only("check user endpoints", () => { it("should not allow a user to update their own roles", async () => { - const res = await makeRequest("put", `/users/${globalUser._id}`, { - ...globalUser, - roles: { - app_1: "ADMIN", - }, - }) - expect( - mockedWorkerReq.saveGlobalUser.mock.lastCall?.[0].body.data.roles["app_1"] - ).toBeUndefined() - expect(res.status).toBe(200) - expect(res.body.data.roles["app_1"]).toBeUndefined() + await config.withUser(globalUser, () => + config.api.public.user.update({ + ...globalUser, + roles: { app_1: "ADMIN" }, + }) + ) + const updatedUser = await config.api.user.find(globalUser._id!) + expect(updatedUser.roles?.app_1).toBeUndefined() }) it("should not allow a user to delete themselves", async () => { - const res = await makeRequest("delete", `/users/${globalUser._id}`) - expect(res.status).toBe(405) - expect(mockedWorkerReq.deleteGlobalUser.mock.lastCall).toBeUndefined() + await config.withUser(globalUser, () => + config.api.public.user.destroy(globalUser._id!, { status: 405 }) + ) }) }) describe("no user role update in free", () => { - beforeAll(() => { - updateMock() - }) - it("should not allow 'roles' to be updated", async () => { const res = await makeRequest("post", "/users", { ...base(), @@ -94,7 +100,6 @@ describe("no user role update in free", () => { describe("no user role update in business", () => { beforeAll(() => { - updateMock() mocks.licenses.useExpandedPublicApi() }) diff --git a/packages/server/src/api/routes/tests/utilities/index.ts b/packages/server/src/api/routes/tests/utilities/index.ts index dcb8ccd6c0..944a56d7ba 100644 --- a/packages/server/src/api/routes/tests/utilities/index.ts +++ b/packages/server/src/api/routes/tests/utilities/index.ts @@ -3,44 +3,6 @@ import supertest from "supertest" export * as structures from "../../../../tests/utilities/structures" -function user() { - return { - _id: "user", - _rev: "rev", - createdAt: Date.now(), - email: "test@example.com", - roles: {}, - tenantId: "default", - status: "active", - } -} - -jest.mock("../../../../utilities/workerRequests", () => ({ - getGlobalUsers: jest.fn(() => { - return { - _id: "us_uuid1", - } - }), - getGlobalSelf: jest.fn(() => { - return { - _id: "us_uuid1", - } - }), - allGlobalUsers: jest.fn(() => { - return [user()] - }), - readGlobalUser: jest.fn(() => { - return user() - }), - saveGlobalUser: jest.fn(() => { - return { _id: "user", _rev: "rev" } - }), - deleteGlobalUser: jest.fn(() => { - return { message: "deleted user" } - }), - removeAppFromUserRoles: jest.fn(), -})) - export function delay(ms: number) { return new Promise(resolve => setTimeout(resolve, ms)) } diff --git a/packages/server/src/tests/utilities/TestConfiguration.ts b/packages/server/src/tests/utilities/TestConfiguration.ts index edb397169d..219a428f05 100644 --- a/packages/server/src/tests/utilities/TestConfiguration.ts +++ b/packages/server/src/tests/utilities/TestConfiguration.ts @@ -67,6 +67,7 @@ import { View, Webhook, WithRequired, + DevInfo, } from "@budibase/types" import API from "./api" @@ -248,7 +249,7 @@ export default class TestConfiguration { } } - async withUser(user: User, f: () => Promise) { + async withUser(user: User, f: () => Promise): Promise { const oldUser = this.user this.user = user try { @@ -469,7 +470,10 @@ export default class TestConfiguration { } } - defaultHeaders(extras = {}, prodApp = false) { + defaultHeaders( + extras: Record = {}, + prodApp = false + ) { const tenantId = this.getTenantId() const user = this.getUser() const authObj: AuthToken = { @@ -498,10 +502,13 @@ export default class TestConfiguration { } } - publicHeaders({ prodApp = true } = {}) { + publicHeaders({ + prodApp = true, + extras = {}, + }: { prodApp?: boolean; extras?: Record } = {}) { const appId = prodApp ? this.prodAppId : this.appId - const headers: any = { + const headers: Record = { Accept: "application/json", Cookie: "", } @@ -514,6 +521,7 @@ export default class TestConfiguration { return { ...headers, ...this.temporaryHeaders, + ...extras, } } @@ -577,17 +585,17 @@ export default class TestConfiguration { } const db = tenancy.getTenantDB(this.getTenantId()) const id = dbCore.generateDevInfoID(userId) - let devInfo: any - try { - devInfo = await db.get(id) - } catch (err) { - devInfo = { _id: id, userId } + const devInfo = await db.tryGet(id) + if (devInfo && devInfo.apiKey) { + return devInfo.apiKey } - devInfo.apiKey = encryption.encrypt( + + const apiKey = encryption.encrypt( `${this.getTenantId()}${dbCore.SEPARATOR}${newid()}` ) - await db.put(devInfo) - return devInfo.apiKey + const newDevInfo: DevInfo = { _id: id, userId, apiKey } + await db.put(newDevInfo) + return apiKey } // APP diff --git a/packages/server/src/tests/utilities/api/base.ts b/packages/server/src/tests/utilities/api/base.ts index 39ac5cefc0..177c3c3c0b 100644 --- a/packages/server/src/tests/utilities/api/base.ts +++ b/packages/server/src/tests/utilities/api/base.ts @@ -46,6 +46,7 @@ export interface RequestOpts { export abstract class TestAPI { config: TestConfiguration request: SuperTest + prefix = "" constructor(config: TestConfiguration) { this.config = config @@ -53,26 +54,26 @@ export abstract class TestAPI { } protected _get = async (url: string, opts?: RequestOpts): Promise => { - return await this._request("get", url, opts) + return await this._request("get", `${this.prefix}${url}`, opts) } protected _post = async (url: string, opts?: RequestOpts): Promise => { - return await this._request("post", url, opts) + return await this._request("post", `${this.prefix}${url}`, opts) } protected _put = async (url: string, opts?: RequestOpts): Promise => { - return await this._request("put", url, opts) + return await this._request("put", `${this.prefix}${url}`, opts) } protected _patch = async (url: string, opts?: RequestOpts): Promise => { - return await this._request("patch", url, opts) + return await this._request("patch", `${this.prefix}${url}`, opts) } protected _delete = async ( url: string, opts?: RequestOpts ): Promise => { - return await this._request("delete", url, opts) + return await this._request("delete", `${this.prefix}${url}`, opts) } protected _requestRaw = async ( @@ -88,7 +89,6 @@ export abstract class TestAPI { fields = {}, files = {}, expectations, - publicUser = false, } = opts || {} const { status = 200 } = expectations || {} const expectHeaders = expectations?.headers || {} @@ -97,7 +97,7 @@ export abstract class TestAPI { expectHeaders["Content-Type"] = /^application\/json/ } - let queryParams = [] + let queryParams: string[] = [] for (const [key, value] of Object.entries(query)) { if (value) { queryParams.push(`${key}=${value}`) @@ -107,18 +107,10 @@ export abstract class TestAPI { url += `?${queryParams.join("&")}` } - const headersFn = publicUser - ? (_extras = {}) => - this.config.publicHeaders.bind(this.config)({ - prodApp: opts?.useProdApp, - }) - : (extras = {}) => - this.config.defaultHeaders.bind(this.config)(extras, opts?.useProdApp) - const app = getServer() let req = request(app)[method](url) req = req.set( - headersFn({ + await this.getHeaders(opts, { "x-budibase-include-stacktrace": "true", }) ) @@ -167,6 +159,17 @@ export abstract class TestAPI { } } + protected async getHeaders( + opts?: RequestOpts, + extras?: Record + ): Promise> { + if (opts?.publicUser) { + return this.config.publicHeaders({ prodApp: opts?.useProdApp, extras }) + } else { + return this.config.defaultHeaders(extras, opts?.useProdApp) + } + } + protected _checkResponse = ( response: Response, expectations?: Expectations @@ -236,3 +239,24 @@ export abstract class TestAPI { ).body } } + +export abstract class PublicAPI extends TestAPI { + prefix = "/api/public/v1" + + protected async getHeaders( + opts?: RequestOpts, + extras?: Record + ): Promise> { + const apiKey = await this.config.generateApiKey() + + const headers: Record = { + Accept: "application/json", + Host: this.config.tenantHost(), + "x-budibase-api-key": apiKey, + "x-budibase-app-id": this.config.getAppId(), + ...extras, + } + + return headers + } +} diff --git a/packages/server/src/tests/utilities/api/index.ts b/packages/server/src/tests/utilities/api/index.ts index 2fdf726b6c..1252b10e40 100644 --- a/packages/server/src/tests/utilities/api/index.ts +++ b/packages/server/src/tests/utilities/api/index.ts @@ -17,6 +17,7 @@ import { RowActionAPI } from "./rowAction" import { AutomationAPI } from "./automation" import { PluginAPI } from "./plugin" import { WebhookAPI } from "./webhook" +import { UserPublicAPI } from "./public/user" export default class API { application: ApplicationAPI @@ -38,6 +39,10 @@ export default class API { viewV2: ViewV2API webhook: WebhookAPI + public: { + user: UserPublicAPI + } + constructor(config: TestConfiguration) { this.application = new ApplicationAPI(config) this.attachment = new AttachmentAPI(config) @@ -57,5 +62,8 @@ export default class API { this.user = new UserAPI(config) this.viewV2 = new ViewV2API(config) this.webhook = new WebhookAPI(config) + this.public = { + user: new UserPublicAPI(config), + } } } diff --git a/packages/server/src/tests/utilities/api/public/user.ts b/packages/server/src/tests/utilities/api/public/user.ts new file mode 100644 index 0000000000..e33b85ad9a --- /dev/null +++ b/packages/server/src/tests/utilities/api/public/user.ts @@ -0,0 +1,19 @@ +import { User } from "@budibase/types" +import { Expectations, PublicAPI } from "../base" + +export class UserPublicAPI extends PublicAPI { + find = async (id: string, expectations?: Expectations): Promise => { + return await this._get(`/users/${id}`, { expectations }) + } + + update = async (user: User, expectations?: Expectations): Promise => { + return await this._put(`/users/${user._id}`, { + body: user, + expectations, + }) + } + + destroy = async (id: string, expectations?: Expectations): Promise => { + return await this._delete(`/users/${id}`, { expectations }) + } +} From 5b285940f13da0b9fb43f74fcd526eb0acd28a91 Mon Sep 17 00:00:00 2001 From: Sam Rose Date: Wed, 26 Feb 2025 12:02:35 +0000 Subject: [PATCH 05/17] Introduce test error propagation to public API endpoints. --- .../src/api/controllers/public/users.ts | 2 +- .../server/src/api/routes/public/index.ts | 5 +++ .../public/middleware/testErrorHandling.ts | 24 ++++++++++++++ .../src/api/routes/public/tests/users.spec.ts | 32 +++++++++---------- .../src/tests/utilities/api/public/user.ts | 13 +++++++- 5 files changed, 58 insertions(+), 18 deletions(-) create mode 100644 packages/server/src/api/routes/public/middleware/testErrorHandling.ts diff --git a/packages/server/src/api/controllers/public/users.ts b/packages/server/src/api/controllers/public/users.ts index 4265c7ac22..c0cd3248a2 100644 --- a/packages/server/src/api/controllers/public/users.ts +++ b/packages/server/src/api/controllers/public/users.ts @@ -48,7 +48,7 @@ function getUser(ctx: UserCtx, userId?: string) { if (userId) { ctx.params = { userId } } else if (!ctx.params?.userId) { - throw "No user ID provided for getting" + throw new Error("No user ID provided for getting") } return readGlobalUser(ctx) } diff --git a/packages/server/src/api/routes/public/index.ts b/packages/server/src/api/routes/public/index.ts index 531192811c..e4fd9d5ea7 100644 --- a/packages/server/src/api/routes/public/index.ts +++ b/packages/server/src/api/routes/public/index.ts @@ -12,6 +12,7 @@ import { paramResource, paramSubResource } from "../../../middleware/resourceId" import { PermissionLevel, PermissionType } from "@budibase/types" import { CtxFn } from "./utils/Endpoint" import mapperMiddleware from "./middleware/mapper" +import testErrorHandling from "./middleware/testErrorHandling" import env from "../../../environment" import { middleware, redis } from "@budibase/backend-core" import { SelectableDatabase } from "@budibase/backend-core/src/redis/utils" @@ -144,6 +145,10 @@ function applyRoutes( // add the output mapper middleware addMiddleware(endpoints.read, mapperMiddleware, { output: true }) addMiddleware(endpoints.write, mapperMiddleware, { output: true }) + if (env.isTest()) { + addMiddleware(endpoints.read, testErrorHandling) + addMiddleware(endpoints.write, testErrorHandling) + } addToRouter(endpoints.read) addToRouter(endpoints.write) } diff --git a/packages/server/src/api/routes/public/middleware/testErrorHandling.ts b/packages/server/src/api/routes/public/middleware/testErrorHandling.ts new file mode 100644 index 0000000000..82519102d2 --- /dev/null +++ b/packages/server/src/api/routes/public/middleware/testErrorHandling.ts @@ -0,0 +1,24 @@ +import { Ctx } from "@budibase/types" +import environment from "../../../../environment" + +export default async (ctx: Ctx, next: any) => { + try { + await next() + } catch (err: any) { + if ( + !(environment.isTest() && ctx.headers["x-budibase-include-stacktrace"]) + ) { + throw err + } + + const status = err.status || err.statusCode || 500 + + let error = err + while (error.cause) { + error = error.cause + } + + ctx.status = status + ctx.body = { status, message: error.message, stack: error.stack } + } +} diff --git a/packages/server/src/api/routes/public/tests/users.spec.ts b/packages/server/src/api/routes/public/tests/users.spec.ts index 4983761c3a..fbfb7bbd61 100644 --- a/packages/server/src/api/routes/public/tests/users.spec.ts +++ b/packages/server/src/api/routes/public/tests/users.spec.ts @@ -1,12 +1,13 @@ import * as setup from "../../tests/utilities" import { User } from "@budibase/types" -import { mocks } from "@budibase/backend-core/tests" +import { generator, mocks } from "@budibase/backend-core/tests" import nock from "nock" import environment from "../../../../environment" import TestConfiguration from "../../../../tests/utilities/TestConfiguration" const config = new TestConfiguration() let globalUser: User +let users: Record = {} beforeAll(async () => { await config.init() @@ -16,25 +17,26 @@ afterAll(setup.afterAll) beforeEach(async () => { globalUser = await config.globalUser() + users[globalUser._id!] = globalUser nock.cleanAll() nock(environment.WORKER_URL!) - .get(`/api/global/users/${globalUser._id}`) + .get(new RegExp(`/api/global/users/.*`)) .reply(200, (uri, body) => { - return globalUser + const id = uri.split("/").pop() + return users[id!] }) .persist() nock(environment.WORKER_URL!) .post(`/api/global/users`) .reply(200, (uri, body) => { - const updatedUser = body as User - if (updatedUser._id === globalUser._id) { - globalUser = updatedUser - return globalUser - } else { - throw new Error("User not found") + const newUser = body as User + if (!newUser._id) { + newUser._id = `us_${generator.guid()}` } + users[newUser._id!] = newUser + return newUser }) .persist() }) @@ -47,7 +49,7 @@ function base() { } } -describe.only("check user endpoints", () => { +describe("check user endpoints", () => { it("should not allow a user to update their own roles", async () => { await config.withUser(globalUser, () => config.api.public.user.update({ @@ -67,14 +69,12 @@ describe.only("check user endpoints", () => { }) describe("no user role update in free", () => { - it("should not allow 'roles' to be updated", async () => { - const res = await makeRequest("post", "/users", { - ...base(), + it.only("should not allow 'roles' to be updated", async () => { + const newUser = await config.api.public.user.create({ + email: generator.email({ domain: "example.com" }), roles: { app_a: "BASIC" }, }) - expect(res.status).toBe(200) - expect(res.body.data.roles["app_a"]).toBeUndefined() - expect(res.body.message).toBeDefined() + expect(newUser.roles["app_a"]).toBeUndefined() }) it("should not allow 'admin' to be updated", async () => { diff --git a/packages/server/src/tests/utilities/api/public/user.ts b/packages/server/src/tests/utilities/api/public/user.ts index e33b85ad9a..a8a7baed7f 100644 --- a/packages/server/src/tests/utilities/api/public/user.ts +++ b/packages/server/src/tests/utilities/api/public/user.ts @@ -1,4 +1,4 @@ -import { User } from "@budibase/types" +import { UnsavedUser, User } from "@budibase/types" import { Expectations, PublicAPI } from "../base" export class UserPublicAPI extends PublicAPI { @@ -16,4 +16,15 @@ export class UserPublicAPI extends PublicAPI { destroy = async (id: string, expectations?: Expectations): Promise => { return await this._delete(`/users/${id}`, { expectations }) } + + create = async ( + user: UnsavedUser, + expectations?: Expectations + ): Promise => { + const response = await this._post<{ data: User }>("/users", { + body: user, + expectations, + }) + return response.data + } } From d7f3ad8a84787c821d0e298d92d0688dcc5711c0 Mon Sep 17 00:00:00 2001 From: Sam Rose Date: Thu, 27 Feb 2025 15:35:09 +0000 Subject: [PATCH 06/17] Fix users.spec.ts. --- .../src/api/routes/public/tests/users.spec.ts | 60 +++++++------------ 1 file changed, 22 insertions(+), 38 deletions(-) diff --git a/packages/server/src/api/routes/public/tests/users.spec.ts b/packages/server/src/api/routes/public/tests/users.spec.ts index fbfb7bbd61..13630e443d 100644 --- a/packages/server/src/api/routes/public/tests/users.spec.ts +++ b/packages/server/src/api/routes/public/tests/users.spec.ts @@ -41,14 +41,6 @@ beforeEach(async () => { .persist() }) -function base() { - return { - tenantId: config.getTenantId(), - firstName: "Test", - lastName: "Test", - } -} - describe("check user endpoints", () => { it("should not allow a user to update their own roles", async () => { await config.withUser(globalUser, () => @@ -68,8 +60,8 @@ describe("check user endpoints", () => { }) }) -describe("no user role update in free", () => { - it.only("should not allow 'roles' to be updated", async () => { +describe("role updating on free tier", () => { + it("should not allow 'roles' to be updated", async () => { const newUser = await config.api.public.user.create({ email: generator.email({ domain: "example.com" }), roles: { app_a: "BASIC" }, @@ -78,60 +70,52 @@ describe("no user role update in free", () => { }) it("should not allow 'admin' to be updated", async () => { - const res = await makeRequest("post", "/users", { - ...base(), + const newUser = await config.api.public.user.create({ + email: generator.email({ domain: "example.com" }), + roles: {}, admin: { global: true }, }) - expect(res.status).toBe(200) - expect(res.body.data.admin).toBeUndefined() - expect(res.body.message).toBeDefined() + expect(newUser.admin).toBeUndefined() }) it("should not allow 'builder' to be updated", async () => { - const res = await makeRequest("post", "/users", { - ...base(), + const newUser = await config.api.public.user.create({ + email: generator.email({ domain: "example.com" }), + roles: {}, builder: { global: true }, }) - expect(res.status).toBe(200) - expect(res.body.data.builder).toBeUndefined() - expect(res.body.message).toBeDefined() + expect(newUser.builder).toBeUndefined() }) }) -describe("no user role update in business", () => { +describe("role updating on business tier", () => { beforeAll(() => { mocks.licenses.useExpandedPublicApi() }) it("should allow 'roles' to be updated", async () => { - const res = await makeRequest("post", "/users", { - ...base(), + const newUser = await config.api.public.user.create({ + email: generator.email({ domain: "example.com" }), roles: { app_a: "BASIC" }, }) - expect(res.status).toBe(200) - expect(res.body.data.roles["app_a"]).toBe("BASIC") - expect(res.body.message).toBeUndefined() + expect(newUser.roles["app_a"]).toBe("BASIC") }) it("should allow 'admin' to be updated", async () => { - mocks.licenses.useExpandedPublicApi() - const res = await makeRequest("post", "/users", { - ...base(), + const newUser = await config.api.public.user.create({ + email: generator.email({ domain: "example.com" }), + roles: {}, admin: { global: true }, }) - expect(res.status).toBe(200) - expect(res.body.data.admin.global).toBe(true) - expect(res.body.message).toBeUndefined() + expect(newUser.admin?.global).toBe(true) }) it("should allow 'builder' to be updated", async () => { - mocks.licenses.useExpandedPublicApi() - const res = await makeRequest("post", "/users", { - ...base(), + const newUser = await config.api.public.user.create({ + email: generator.email({ domain: "example.com" }), + roles: {}, builder: { global: true }, }) - expect(res.status).toBe(200) - expect(res.body.data.builder.global).toBe(true) - expect(res.body.message).toBeUndefined() + expect(newUser.builder?.global).toBe(true) }) }) From 55783e17f179993cc8fd4a37a362ef0ba387b959 Mon Sep 17 00:00:00 2001 From: Sam Rose Date: Thu, 27 Feb 2025 17:11:50 +0000 Subject: [PATCH 07/17] Fix compare.spec.ts. --- packages/server/specs/resources/error.ts | 21 ++ .../api/routes/public/tests/compare.spec.ts | 308 +++++++++--------- .../src/api/routes/public/tests/users.spec.ts | 230 +++++++------ .../src/api/routes/public/tests/utils.ts | 44 +++ .../server/src/tests/utilities/api/base.ts | 20 +- .../src/tests/utilities/api/public/user.ts | 8 +- 6 files changed, 362 insertions(+), 269 deletions(-) create mode 100644 packages/server/specs/resources/error.ts diff --git a/packages/server/specs/resources/error.ts b/packages/server/specs/resources/error.ts new file mode 100644 index 0000000000..0d42963b73 --- /dev/null +++ b/packages/server/specs/resources/error.ts @@ -0,0 +1,21 @@ +import { object } from "./utils" +import Resource from "./utils/Resource" + +const errorSchema = object({ + status: { + type: "number", + description: "The HTTP status code of the error.", + }, + message: { + type: "string", + description: "A descriptive message about the error.", + }, +}) + +export default new Resource() + .setExamples({ + error: {}, + }) + .setSchemas({ + error: errorSchema, + }) diff --git a/packages/server/src/api/routes/public/tests/compare.spec.ts b/packages/server/src/api/routes/public/tests/compare.spec.ts index ccf248f360..b4f5f20e2c 100644 --- a/packages/server/src/api/routes/public/tests/compare.spec.ts +++ b/packages/server/src/api/routes/public/tests/compare.spec.ts @@ -2,184 +2,174 @@ import jestOpenAPI from "jest-openapi" import { run as generateSchema } from "../../../../../specs/generate" import * as setup from "../../tests/utilities" import { generateMakeRequest } from "./utils" -import { Table, App, Row, User } from "@budibase/types" +import { Table, App, Row } from "@budibase/types" +import nock from "nock" +import environment from "../../../../environment" const yamlPath = generateSchema() jestOpenAPI(yamlPath!) -let config = setup.getConfig() -let apiKey: string, table: Table, app: App, makeRequest: any +describe("compare", () => { + let config = setup.getConfig() + let apiKey: string, table: Table, app: App, makeRequest: any -beforeAll(async () => { - app = await config.init() - table = await config.upsertTable() - apiKey = await config.generateApiKey() - makeRequest = generateMakeRequest(apiKey) -}) - -afterAll(setup.afterAll) - -describe("check the applications endpoints", () => { - it("should allow retrieving applications through search", async () => { - const res = await makeRequest("post", "/applications/search") - expect(res).toSatisfyApiSpec() - }) - - it("should allow creating an application", async () => { - const res = await makeRequest( - "post", - "/applications", - { - name: "new App", - }, - null - ) - expect(res).toSatisfyApiSpec() - }) - - it("should allow updating an application", async () => { - const app = config.getApp() - const appId = config.getAppId() - const res = await makeRequest( - "put", - `/applications/${appId}`, - { - ...app, - name: "updated app name", - }, - appId - ) - expect(res).toSatisfyApiSpec() - }) - - it("should allow retrieving an application", async () => { - const res = await makeRequest("get", `/applications/${config.getAppId()}`) - expect(res).toSatisfyApiSpec() - }) - - it("should allow deleting an application", async () => { - const res = await makeRequest( - "delete", - `/applications/${config.getAppId()}` - ) - expect(res).toSatisfyApiSpec() - }) -}) - -describe("check the tables endpoints", () => { - it("should allow retrieving tables through search", async () => { - await config.createApp("new app 1") + beforeAll(async () => { + app = await config.init() table = await config.upsertTable() - const res = await makeRequest("post", "/tables/search") - expect(res).toSatisfyApiSpec() + apiKey = await config.generateApiKey() + makeRequest = generateMakeRequest(apiKey) }) - it("should allow creating a table", async () => { - const res = await makeRequest("post", "/tables", { - name: "table name", - primaryDisplay: "column1", - schema: { - column1: { - type: "string", - constraints: {}, + afterAll(setup.afterAll) + + beforeEach(() => { + nock.cleanAll() + }) + + describe("check the applications endpoints", () => { + it("should allow retrieving applications through search", async () => { + const res = await makeRequest("post", "/applications/search") + expect(res).toSatisfyApiSpec() + }) + + it("should allow creating an application", async () => { + const res = await makeRequest( + "post", + "/applications", + { + name: "new App", }, - }, + null + ) + expect(res).toSatisfyApiSpec() }) - expect(res).toSatisfyApiSpec() - }) - it("should allow updating a table", async () => { - const updated = { ...table, _rev: undefined, name: "new name" } - const res = await makeRequest("put", `/tables/${table._id}`, updated) - expect(res).toSatisfyApiSpec() - }) - - it("should allow retrieving a table", async () => { - const res = await makeRequest("get", `/tables/${table._id}`) - expect(res).toSatisfyApiSpec() - }) - - it("should allow deleting a table", async () => { - const res = await makeRequest("delete", `/tables/${table._id}`) - expect(res).toSatisfyApiSpec() - }) -}) - -describe("check the rows endpoints", () => { - let row: Row - it("should allow retrieving rows through search", async () => { - table = await config.upsertTable() - const res = await makeRequest("post", `/tables/${table._id}/rows/search`, { - query: {}, + it("should allow updating an application", async () => { + const app = config.getApp() + const appId = config.getAppId() + const res = await makeRequest( + "put", + `/applications/${appId}`, + { + ...app, + name: "updated app name", + }, + appId + ) + expect(res).toSatisfyApiSpec() }) - expect(res).toSatisfyApiSpec() - }) - it("should allow creating a row", async () => { - const res = await makeRequest("post", `/tables/${table._id}/rows`, { - name: "test row", + it("should allow retrieving an application", async () => { + const res = await makeRequest("get", `/applications/${config.getAppId()}`) + expect(res).toSatisfyApiSpec() + }) + + it("should allow deleting an application", async () => { + nock(environment.WORKER_URL!) + .delete(`/api/global/roles/${config.getProdAppId()}`) + .reply(200, {}) + + const res = await makeRequest( + "delete", + `/applications/${config.getAppId()}` + ) + expect(res).toSatisfyApiSpec() }) - expect(res).toSatisfyApiSpec() - row = res.body.data }) - it("should allow updating a row", async () => { - const res = await makeRequest( - "put", - `/tables/${table._id}/rows/${row._id}`, - { - name: "test row updated", - } - ) - expect(res).toSatisfyApiSpec() + describe("check the tables endpoints", () => { + it("should allow retrieving tables through search", async () => { + await config.createApp("new app 1") + table = await config.upsertTable() + const res = await makeRequest("post", "/tables/search") + expect(res).toSatisfyApiSpec() + }) + + it("should allow creating a table", async () => { + const res = await makeRequest("post", "/tables", { + name: "table name", + primaryDisplay: "column1", + schema: { + column1: { + type: "string", + constraints: {}, + }, + }, + }) + expect(res).toSatisfyApiSpec() + }) + + it("should allow updating a table", async () => { + const updated = { ...table, _rev: undefined, name: "new name" } + const res = await makeRequest("put", `/tables/${table._id}`, updated) + expect(res).toSatisfyApiSpec() + }) + + it("should allow retrieving a table", async () => { + const res = await makeRequest("get", `/tables/${table._id}`) + expect(res).toSatisfyApiSpec() + }) + + it("should allow deleting a table", async () => { + const res = await makeRequest("delete", `/tables/${table._id}`) + expect(res).toSatisfyApiSpec() + }) }) - it("should allow retrieving a row", async () => { - const res = await makeRequest("get", `/tables/${table._id}/rows/${row._id}`) - expect(res).toSatisfyApiSpec() + describe("check the rows endpoints", () => { + let row: Row + it("should allow retrieving rows through search", async () => { + table = await config.upsertTable() + const res = await makeRequest( + "post", + `/tables/${table._id}/rows/search`, + { + query: {}, + } + ) + expect(res).toSatisfyApiSpec() + }) + + it("should allow creating a row", async () => { + const res = await makeRequest("post", `/tables/${table._id}/rows`, { + name: "test row", + }) + expect(res).toSatisfyApiSpec() + row = res.body.data + }) + + it("should allow updating a row", async () => { + const res = await makeRequest( + "put", + `/tables/${table._id}/rows/${row._id}`, + { + name: "test row updated", + } + ) + expect(res).toSatisfyApiSpec() + }) + + it("should allow retrieving a row", async () => { + const res = await makeRequest( + "get", + `/tables/${table._id}/rows/${row._id}` + ) + expect(res).toSatisfyApiSpec() + }) + + it("should allow deleting a row", async () => { + const res = await makeRequest( + "delete", + `/tables/${table._id}/rows/${row._id}` + ) + expect(res).toSatisfyApiSpec() + }) }) - it("should allow deleting a row", async () => { - const res = await makeRequest( - "delete", - `/tables/${table._id}/rows/${row._id}` - ) - expect(res).toSatisfyApiSpec() - }) -}) - -describe("check the users endpoints", () => { - let user: User - it("should allow retrieving users through search", async () => { - user = await config.createUser() - const res = await makeRequest("post", "/users/search") - expect(res).toSatisfyApiSpec() - }) - - it("should allow creating a user", async () => { - const res = await makeRequest("post", "/users") - expect(res).toSatisfyApiSpec() - }) - - it("should allow updating a user", async () => { - const res = await makeRequest("put", `/users/${user._id}`) - expect(res).toSatisfyApiSpec() - }) - - it("should allow retrieving a user", async () => { - const res = await makeRequest("get", `/users/${user._id}`) - expect(res).toSatisfyApiSpec() - }) - - it("should allow deleting a user", async () => { - const res = await makeRequest("delete", `/users/${user._id}`) - expect(res).toSatisfyApiSpec() - }) -}) - -describe("check the queries endpoints", () => { - it("should allow retrieving queries through search", async () => { - const res = await makeRequest("post", "/queries/search") - expect(res).toSatisfyApiSpec() + describe("check the queries endpoints", () => { + it("should allow retrieving queries through search", async () => { + const res = await makeRequest("post", "/queries/search") + expect(res).toSatisfyApiSpec() + }) }) }) diff --git a/packages/server/src/api/routes/public/tests/users.spec.ts b/packages/server/src/api/routes/public/tests/users.spec.ts index 13630e443d..bb3e4b6fba 100644 --- a/packages/server/src/api/routes/public/tests/users.spec.ts +++ b/packages/server/src/api/routes/public/tests/users.spec.ts @@ -2,120 +2,142 @@ import * as setup from "../../tests/utilities" import { User } from "@budibase/types" import { generator, mocks } from "@budibase/backend-core/tests" import nock from "nock" -import environment from "../../../../environment" import TestConfiguration from "../../../../tests/utilities/TestConfiguration" +import { mockWorkerUserAPI } from "./utils" -const config = new TestConfiguration() -let globalUser: User -let users: Record = {} +describe("public users API", () => { + const config = new TestConfiguration() + let globalUser: User -beforeAll(async () => { - await config.init() -}) + beforeAll(async () => { + await config.init() + }) -afterAll(setup.afterAll) + afterAll(setup.afterAll) -beforeEach(async () => { - globalUser = await config.globalUser() - users[globalUser._id!] = globalUser + beforeEach(async () => { + globalUser = await config.globalUser() - nock.cleanAll() - nock(environment.WORKER_URL!) - .get(new RegExp(`/api/global/users/.*`)) - .reply(200, (uri, body) => { - const id = uri.split("/").pop() - return users[id!] + nock.cleanAll() + mockWorkerUserAPI(globalUser) + }) + + describe("read", () => { + it("should allow a user to read themselves", async () => { + const user = await config.api.user.find(globalUser._id!) + expect(user._id).toBe(globalUser._id) }) - .persist() - nock(environment.WORKER_URL!) - .post(`/api/global/users`) - .reply(200, (uri, body) => { - const newUser = body as User - if (!newUser._id) { - newUser._id = `us_${generator.guid()}` - } - users[newUser._id!] = newUser - return newUser - }) - .persist() -}) - -describe("check user endpoints", () => { - it("should not allow a user to update their own roles", async () => { - await config.withUser(globalUser, () => - config.api.public.user.update({ - ...globalUser, - roles: { app_1: "ADMIN" }, + it("should allow a user to read another user", async () => { + const otherUser = await config.api.public.user.create({ + email: generator.email({ domain: "example.com" }), + roles: {}, }) - ) - const updatedUser = await config.api.user.find(globalUser._id!) - expect(updatedUser.roles?.app_1).toBeUndefined() + const user = await config.withUser(globalUser, () => + config.api.public.user.find(otherUser._id!) + ) + expect(user._id).toBe(otherUser._id) + }) }) - it("should not allow a user to delete themselves", async () => { - await config.withUser(globalUser, () => - config.api.public.user.destroy(globalUser._id!, { status: 405 }) - ) - }) -}) - -describe("role updating on free tier", () => { - it("should not allow 'roles' to be updated", async () => { - const newUser = await config.api.public.user.create({ - email: generator.email({ domain: "example.com" }), - roles: { app_a: "BASIC" }, - }) - expect(newUser.roles["app_a"]).toBeUndefined() - }) - - it("should not allow 'admin' to be updated", async () => { - const newUser = await config.api.public.user.create({ - email: generator.email({ domain: "example.com" }), - roles: {}, - admin: { global: true }, - }) - expect(newUser.admin).toBeUndefined() - }) - - it("should not allow 'builder' to be updated", async () => { - const newUser = await config.api.public.user.create({ - email: generator.email({ domain: "example.com" }), - roles: {}, - builder: { global: true }, - }) - expect(newUser.builder).toBeUndefined() - }) -}) - -describe("role updating on business tier", () => { - beforeAll(() => { - mocks.licenses.useExpandedPublicApi() - }) - - it("should allow 'roles' to be updated", async () => { - const newUser = await config.api.public.user.create({ - email: generator.email({ domain: "example.com" }), - roles: { app_a: "BASIC" }, - }) - expect(newUser.roles["app_a"]).toBe("BASIC") - }) - - it("should allow 'admin' to be updated", async () => { - const newUser = await config.api.public.user.create({ - email: generator.email({ domain: "example.com" }), - roles: {}, - admin: { global: true }, - }) - expect(newUser.admin?.global).toBe(true) - }) - - it("should allow 'builder' to be updated", async () => { - const newUser = await config.api.public.user.create({ - email: generator.email({ domain: "example.com" }), - roles: {}, - builder: { global: true }, - }) - expect(newUser.builder?.global).toBe(true) + describe("create", () => { + it("can successfully create a new user", async () => { + const email = generator.email({ domain: "example.com" }) + const newUser = await config.api.public.user.create({ + email, + roles: {}, + }) + expect(newUser.email).toBe(email) + expect(newUser._id).toBeDefined() + }) + + describe("role creation on free tier", () => { + it("should not allow 'roles' to be updated", async () => { + const newUser = await config.api.public.user.create({ + email: generator.email({ domain: "example.com" }), + roles: { app_a: "BASIC" }, + }) + expect(newUser.roles["app_a"]).toBeUndefined() + }) + + it("should not allow 'admin' to be updated", async () => { + const newUser = await config.api.public.user.create({ + email: generator.email({ domain: "example.com" }), + roles: {}, + admin: { global: true }, + }) + expect(newUser.admin).toBeUndefined() + }) + + it("should not allow 'builder' to be updated", async () => { + const newUser = await config.api.public.user.create({ + email: generator.email({ domain: "example.com" }), + roles: {}, + builder: { global: true }, + }) + expect(newUser.builder).toBeUndefined() + }) + }) + + describe("role creation on business tier", () => { + beforeAll(() => { + mocks.licenses.useExpandedPublicApi() + }) + + it("should allow 'roles' to be updated", async () => { + const newUser = await config.api.public.user.create({ + email: generator.email({ domain: "example.com" }), + roles: { app_a: "BASIC" }, + }) + expect(newUser.roles["app_a"]).toBe("BASIC") + }) + + it("should allow 'admin' to be updated", async () => { + const newUser = await config.api.public.user.create({ + email: generator.email({ domain: "example.com" }), + roles: {}, + admin: { global: true }, + }) + expect(newUser.admin?.global).toBe(true) + }) + + it("should allow 'builder' to be updated", async () => { + const newUser = await config.api.public.user.create({ + email: generator.email({ domain: "example.com" }), + roles: {}, + builder: { global: true }, + }) + expect(newUser.builder?.global).toBe(true) + }) + }) + }) + + describe("update", () => { + it("can update a user", async () => { + const updatedUser = await config.api.public.user.update({ + ...globalUser, + email: `updated-${globalUser.email}`, + }) + expect(updatedUser.email).toBe(`updated-${globalUser.email}`) + }) + + it("should not allow a user to update their own roles", async () => { + await config.withUser(globalUser, () => + config.api.public.user.update({ + ...globalUser, + roles: { app_1: "ADMIN" }, + }) + ) + const updatedUser = await config.api.user.find(globalUser._id!) + expect(updatedUser.roles?.app_1).toBeUndefined() + }) + }) + + describe("delete", () => { + it("should not allow a user to delete themselves", async () => { + await config.withUser(globalUser, () => + config.api.public.user.destroy(globalUser._id!, { status: 405 }) + ) + }) }) }) diff --git a/packages/server/src/api/routes/public/tests/utils.ts b/packages/server/src/api/routes/public/tests/utils.ts index 4fb048a540..4985c0db58 100644 --- a/packages/server/src/api/routes/public/tests/utils.ts +++ b/packages/server/src/api/routes/public/tests/utils.ts @@ -1,6 +1,10 @@ import * as setup from "../../tests/utilities" import { checkSlashesInUrl } from "../../../../utilities" import supertest from "supertest" +import { User } from "@budibase/types" +import environment from "../../../../environment" +import nock from "nock" +import { generator } from "@budibase/backend-core/tests" export type HttpMethod = "post" | "get" | "put" | "delete" | "patch" @@ -91,3 +95,43 @@ export function generateMakeRequestWithFormData( return res } } + +export function mockWorkerUserAPI(...seedUsers: User[]) { + const users: Record = { + ...seedUsers.reduce((acc, user) => { + acc[user._id!] = user + return acc + }, {} as Record), + } + + nock(environment.WORKER_URL!) + .get(new RegExp(`/api/global/users/.*`)) + .reply(200, (uri, body) => { + const id = uri.split("/").pop() + return users[id!] + }) + .persist() + + nock(environment.WORKER_URL!) + .post(`/api/global/users`) + .reply(200, (uri, body) => { + const newUser = body as User + if (!newUser._id) { + newUser._id = `us_${generator.guid()}` + } + users[newUser._id!] = newUser + return newUser + }) + .persist() + + nock(environment.WORKER_URL!) + .put(new RegExp(`/api/global/users/.*`)) + .reply(200, (uri, body) => { + const id = uri.split("/").pop()! + const updatedUser = body as User + const existingUser = users[id] || {} + users[id] = { ...existingUser, ...updatedUser } + return users[id] + }) + .persist() +} diff --git a/packages/server/src/tests/utilities/api/base.ts b/packages/server/src/tests/utilities/api/base.ts index 177c3c3c0b..2b3e3db44c 100644 --- a/packages/server/src/tests/utilities/api/base.ts +++ b/packages/server/src/tests/utilities/api/base.ts @@ -1,8 +1,13 @@ +import jestOpenAPI from "jest-openapi" +import { run as generateSchema } from "../../../../specs/generate" import TestConfiguration from "../TestConfiguration" import request, { SuperTest, Test, Response } from "supertest" import { ReadStream } from "fs" import { getServer } from "../../../app" +const yamlPath = generateSchema() +jestOpenAPI(yamlPath!) + type Headers = Record type Method = "get" | "post" | "put" | "patch" | "delete" @@ -170,10 +175,7 @@ export abstract class TestAPI { } } - protected _checkResponse = ( - response: Response, - expectations?: Expectations - ) => { + protected _checkResponse(response: Response, expectations?: Expectations) { const { status = 200 } = expectations || {} if (response.status !== status) { @@ -259,4 +261,14 @@ export abstract class PublicAPI extends TestAPI { return headers } + + protected _checkResponse(response: Response, expectations?: Expectations) { + const checked = super._checkResponse(response, expectations) + if (checked.status >= 200 && checked.status < 300) { + // We don't seem to have documented our errors yet, so for the time being + // we'll only do the schema check for successful responses. + expect(checked).toSatisfyApiSpec() + } + return checked + } } diff --git a/packages/server/src/tests/utilities/api/public/user.ts b/packages/server/src/tests/utilities/api/public/user.ts index a8a7baed7f..4f5fccc740 100644 --- a/packages/server/src/tests/utilities/api/public/user.ts +++ b/packages/server/src/tests/utilities/api/public/user.ts @@ -3,14 +3,18 @@ import { Expectations, PublicAPI } from "../base" export class UserPublicAPI extends PublicAPI { find = async (id: string, expectations?: Expectations): Promise => { - return await this._get(`/users/${id}`, { expectations }) + const response = await this._get<{ data: User }>(`/users/${id}`, { + expectations, + }) + return response.data } update = async (user: User, expectations?: Expectations): Promise => { - return await this._put(`/users/${user._id}`, { + const response = await this._put<{ data: User }>(`/users/${user._id}`, { body: user, expectations, }) + return response.data } destroy = async (id: string, expectations?: Expectations): Promise => { From e2e1e9d7d2a9080e3af858820a7cfc85b0603017 Mon Sep 17 00:00:00 2001 From: Sam Rose Date: Mon, 3 Mar 2025 09:55:34 +0000 Subject: [PATCH 08/17] De-mock the environment variable tests. --- .../routes/tests/environmentVariables.spec.ts | 237 +++++++----------- .../src/tests/utilities/api/environment.ts | 52 ++++ .../server/src/tests/utilities/api/index.ts | 3 + .../server/src/tests/utilities/structures.ts | 5 +- 4 files changed, 155 insertions(+), 142 deletions(-) create mode 100644 packages/server/src/tests/utilities/api/environment.ts diff --git a/packages/server/src/api/routes/tests/environmentVariables.spec.ts b/packages/server/src/api/routes/tests/environmentVariables.spec.ts index beb6012c9c..e6df353797 100644 --- a/packages/server/src/api/routes/tests/environmentVariables.spec.ts +++ b/packages/server/src/api/routes/tests/environmentVariables.spec.ts @@ -1,153 +1,108 @@ const pg = require("pg") -jest.mock("pg", () => { - return { - Client: jest.fn().mockImplementation(() => ({ - connect: jest.fn(), - query: jest.fn().mockImplementation(() => ({ rows: [] })), - end: jest.fn().mockImplementation((fn: any) => fn()), - })), - queryMock: jest.fn().mockImplementation(() => {}), - on: jest.fn(), - } -}) -import * as setup from "./utilities" +import { structures } from "./utilities" import { mocks } from "@budibase/backend-core/tests" -import { env, events } from "@budibase/backend-core" -import { QueryPreview } from "@budibase/types" +import { setEnv } from "@budibase/backend-core" +import { Datasource } from "@budibase/types" +import TestConfiguration from "../../../tests/utilities/TestConfiguration" +import { + DatabaseName, + datasourceDescribe, +} from "../../../integrations/tests/utils" -const structures = setup.structures +const describes = datasourceDescribe({ only: [DatabaseName.POSTGRES] }) -env._set("ENCRYPTION_KEY", "budibase") -mocks.licenses.useEnvironmentVariables() +if (describes.length > 0) { + describe.each(describes)("/api/env/variables", ({ dsProvider }) => { + const config = new TestConfiguration() -describe("/api/env/variables", () => { - let request = setup.getRequest() - let config = setup.getConfig() + let rawDatasource: Datasource + let restoreEnv: () => void - afterAll(setup.afterAll) + beforeAll(async () => { + await config.init() + restoreEnv = setEnv({ ENCRYPTION_KEY: "budibase" }) + mocks.licenses.useEnvironmentVariables() - beforeAll(async () => { - await config.init() - }) + const ds = await dsProvider() + rawDatasource = ds.rawDatasource! + }) - it("should be able check the status of env var API", async () => { - const res = await request - .get(`/api/env/variables/status`) - .set(config.defaultHeaders()) - .expect("Content-Type", /json/) - .expect(200) + afterAll(() => { + restoreEnv() + }) - expect(res.body.encryptionKeyAvailable).toEqual(true) - }) - - it("should be able to create an environment variable", async () => { - await request - .post(`/api/env/variables`) - .send(structures.basicEnvironmentVariable("test", "test")) - .set(config.defaultHeaders()) - .expect(200) - }) - - it("should be able to fetch the 'test' variable name", async () => { - const res = await request - .get(`/api/env/variables`) - .set(config.defaultHeaders()) - .expect("Content-Type", /json/) - .expect(200) - expect(res.body.variables.length).toEqual(1) - expect(res.body.variables[0]).toEqual("test") - }) - - it("should be able to update the environment variable 'test'", async () => { - const varName = "test" - await request - .patch(`/api/env/variables/${varName}`) - .send(structures.basicEnvironmentVariable("test", "test1")) - .set(config.defaultHeaders()) - .expect(200) - }) - - it("should be able to delete the environment variable 'test'", async () => { - const varName = "test" - await request - .delete(`/api/env/variables/${varName}`) - .set(config.defaultHeaders()) - .expect(200) - }) - - it("should create a datasource (using the environment variable) and query", async () => { - const datasourceBase = structures.basicDatasource() - await request - .post(`/api/env/variables`) - .send(structures.basicEnvironmentVariable("test", "test")) - .set(config.defaultHeaders()) - - datasourceBase.datasource.config = { - password: "{{ env.test }}", - } - const response = await request - .post(`/api/datasources`) - .send(datasourceBase) - .set(config.defaultHeaders()) - .expect("Content-Type", /json/) - .expect(200) - expect(response.body.datasource._id).toBeDefined() - - const response2 = await request - .post(`/api/queries`) - .send(structures.basicQuery(response.body.datasource._id)) - .set(config.defaultHeaders()) - .expect("Content-Type", /json/) - .expect(200) - expect(response2.body._id).toBeDefined() - }) - - it("should run a query preview and check the mocked results", async () => { - const datasourceBase = structures.basicDatasource() - await request - .post(`/api/env/variables`) - .send(structures.basicEnvironmentVariable("test", "test")) - .set(config.defaultHeaders()) - - datasourceBase.datasource.config = { - password: "{{ env.test }}", - } - const response = await request - .post(`/api/datasources`) - .send(datasourceBase) - .set(config.defaultHeaders()) - .expect("Content-Type", /json/) - .expect(200) - expect(response.body.datasource._id).toBeDefined() - - const queryPreview: QueryPreview = { - datasourceId: response.body.datasource._id, - parameters: [], - fields: {}, - queryVerb: "read", - name: response.body.datasource.name, - transformer: null, - schema: {}, - readable: true, - } - const res = await request - .post(`/api/queries/preview`) - .send(queryPreview) - .set(config.defaultHeaders()) - .expect("Content-Type", /json/) - .expect(200) - expect(res.body.rows.length).toEqual(0) - expect(events.query.previewed).toHaveBeenCalledTimes(1) - // API doesn't include config in response - delete response.body.datasource.config - expect(events.query.previewed).toHaveBeenCalledWith( - response.body.datasource, - { - ...queryPreview, - nullDefaultSupport: true, + beforeEach(async () => { + const { variables } = await config.api.environment.fetch() + for (const variable of variables) { + await config.api.environment.destroy(variable) } - ) - expect(pg.Client).toHaveBeenCalledWith({ password: "test", ssl: undefined }) + + await config.api.environment.create({ + name: "test", + production: rawDatasource.config!.password, + development: rawDatasource.config!.password, + }) + }) + + it("should be able check the status of env var API", async () => { + const { encryptionKeyAvailable } = await config.api.environment.status() + expect(encryptionKeyAvailable).toEqual(true) + }) + + it("should be able to fetch the 'test' variable name", async () => { + const { variables } = await config.api.environment.fetch() + expect(variables.length).toEqual(1) + expect(variables[0]).toEqual("test") + }) + + it("should be able to update the environment variable 'test'", async () => { + await config.api.environment.update("test", { + production: "test1", + development: "test1", + }) + }) + + it("should be able to delete the environment variable 'test'", async () => { + await config.api.environment.destroy("test") + }) + + it("should create a datasource (using the environment variable) and query", async () => { + const datasource = await config.api.datasource.create({ + ...structures.basicDatasource().datasource, + config: { + ...rawDatasource.config, + password: "{{ env.test }}", + }, + }) + + const query = await config.api.query.save({ + ...structures.basicQuery(datasource._id!), + fields: { sql: "SELECT 1" }, + }) + expect(query._id).toBeDefined() + }) + + it("should run a query preview and check the mocked results", async () => { + const datasource = await config.api.datasource.create({ + ...structures.basicDatasource().datasource, + config: { + ...rawDatasource.config, + password: "{{ env.test }}", + }, + }) + + const query = await config.api.query.save({ + ...structures.basicQuery(datasource._id!), + fields: { sql: "SELECT 1 as id" }, + }) + + const { rows } = await config.api.query.preview({ + ...query, + queryId: query._id!, + }) + + expect(rows).toEqual([{ id: 1 }]) + }) }) -}) +} diff --git a/packages/server/src/tests/utilities/api/environment.ts b/packages/server/src/tests/utilities/api/environment.ts new file mode 100644 index 0000000000..2c4e7a751e --- /dev/null +++ b/packages/server/src/tests/utilities/api/environment.ts @@ -0,0 +1,52 @@ +import { Expectations, TestAPI } from "./base" +import { + CreateEnvironmentVariableRequest, + CreateEnvironmentVariableResponse, + GetEnvironmentVariablesResponse, + Row, + StatusEnvironmentVariableResponse, + UpdateEnvironmentVariableRequest, +} from "@budibase/types" + +export class EnvironmentAPI extends TestAPI { + create = async ( + body: CreateEnvironmentVariableRequest, + expectations?: Expectations + ) => { + return await this._post( + `/api/env/variables`, + { body, expectations } + ) + } + + status = async (expectations?: Expectations) => { + return await this._get( + `/api/env/variables/status`, + { expectations } + ) + } + + fetch = async (expectations?: Expectations) => { + return await this._get( + `/api/env/variables`, + { expectations } + ) + } + + update = async ( + varName: string, + body: UpdateEnvironmentVariableRequest, + expectations?: Expectations + ) => { + return await this._patch(`/api/env/variables/${varName}`, { + body, + expectations, + }) + } + + destroy = async (varName: string, expectations?: Expectations) => { + return await this._delete(`/api/env/variables/${varName}`, { + expectations, + }) + } +} diff --git a/packages/server/src/tests/utilities/api/index.ts b/packages/server/src/tests/utilities/api/index.ts index 2fdf726b6c..42afd10647 100644 --- a/packages/server/src/tests/utilities/api/index.ts +++ b/packages/server/src/tests/utilities/api/index.ts @@ -17,6 +17,7 @@ import { RowActionAPI } from "./rowAction" import { AutomationAPI } from "./automation" import { PluginAPI } from "./plugin" import { WebhookAPI } from "./webhook" +import { EnvironmentAPI } from "./environment" export default class API { application: ApplicationAPI @@ -24,6 +25,7 @@ export default class API { automation: AutomationAPI backup: BackupAPI datasource: DatasourceAPI + environment: EnvironmentAPI legacyView: LegacyViewAPI permission: PermissionAPI plugin: PluginAPI @@ -44,6 +46,7 @@ export default class API { this.automation = new AutomationAPI(config) this.backup = new BackupAPI(config) this.datasource = new DatasourceAPI(config) + this.environment = new EnvironmentAPI(config) this.legacyView = new LegacyViewAPI(config) this.permission = new PermissionAPI(config) this.plugin = new PluginAPI(config) diff --git a/packages/server/src/tests/utilities/structures.ts b/packages/server/src/tests/utilities/structures.ts index 38d60e1c11..bcad085e08 100644 --- a/packages/server/src/tests/utilities/structures.ts +++ b/packages/server/src/tests/utilities/structures.ts @@ -37,6 +37,9 @@ import { DeepPartial, FilterCondition, AutomationTriggerResult, + EnvironmentVariablesDoc, + EnvironmentVariableValue, + CreateEnvironmentVariableRequest, } from "@budibase/types" import { LoopInput } from "../../definitions/automations" import { merge } from "lodash" @@ -574,7 +577,7 @@ export function basicEnvironmentVariable( name: string, prod: string, dev?: string -) { +): CreateEnvironmentVariableRequest { return { name, production: prod, From 81bfd81126ac89ea0824648b553377a0a7b2e34c Mon Sep 17 00:00:00 2001 From: Sam Rose Date: Mon, 3 Mar 2025 10:15:47 +0000 Subject: [PATCH 09/17] Fix lint. --- .../server/src/api/routes/tests/environmentVariables.spec.ts | 2 -- packages/server/src/tests/utilities/api/environment.ts | 1 - packages/server/src/tests/utilities/structures.ts | 2 -- 3 files changed, 5 deletions(-) diff --git a/packages/server/src/api/routes/tests/environmentVariables.spec.ts b/packages/server/src/api/routes/tests/environmentVariables.spec.ts index e6df353797..c663e2e26e 100644 --- a/packages/server/src/api/routes/tests/environmentVariables.spec.ts +++ b/packages/server/src/api/routes/tests/environmentVariables.spec.ts @@ -1,5 +1,3 @@ -const pg = require("pg") - import { structures } from "./utilities" import { mocks } from "@budibase/backend-core/tests" import { setEnv } from "@budibase/backend-core" diff --git a/packages/server/src/tests/utilities/api/environment.ts b/packages/server/src/tests/utilities/api/environment.ts index 2c4e7a751e..152336b316 100644 --- a/packages/server/src/tests/utilities/api/environment.ts +++ b/packages/server/src/tests/utilities/api/environment.ts @@ -3,7 +3,6 @@ import { CreateEnvironmentVariableRequest, CreateEnvironmentVariableResponse, GetEnvironmentVariablesResponse, - Row, StatusEnvironmentVariableResponse, UpdateEnvironmentVariableRequest, } from "@budibase/types" diff --git a/packages/server/src/tests/utilities/structures.ts b/packages/server/src/tests/utilities/structures.ts index bcad085e08..1679334cab 100644 --- a/packages/server/src/tests/utilities/structures.ts +++ b/packages/server/src/tests/utilities/structures.ts @@ -37,8 +37,6 @@ import { DeepPartial, FilterCondition, AutomationTriggerResult, - EnvironmentVariablesDoc, - EnvironmentVariableValue, CreateEnvironmentVariableRequest, } from "@budibase/types" import { LoopInput } from "../../definitions/automations" From a7e17712a30acca56d54bcabe5dafdd326e5a99a Mon Sep 17 00:00:00 2001 From: Adria Navarro Date: Mon, 3 Mar 2025 12:23:25 +0100 Subject: [PATCH 10/17] Remove types --- .../app/forms/RelationshipField.svelte | 37 +++++++------------ .../frontend-core/src/fetch/GroupUserFetch.ts | 2 +- packages/frontend-core/src/fetch/UserFetch.ts | 4 +- 3 files changed, 16 insertions(+), 27 deletions(-) diff --git a/packages/client/src/components/app/forms/RelationshipField.svelte b/packages/client/src/components/app/forms/RelationshipField.svelte index 0226e7d7f3..c40041c7d6 100644 --- a/packages/client/src/components/app/forms/RelationshipField.svelte +++ b/packages/client/src/components/app/forms/RelationshipField.svelte @@ -1,12 +1,6 @@