From 2933571c62e9a8f58a24cdb73e5d7eed768b8812 Mon Sep 17 00:00:00 2001 From: Peter Clement Date: Wed, 28 Feb 2024 08:34:41 +0000 Subject: [PATCH 01/17] update runLuceneQuery in client to allow for all filter matching --- packages/shared-core/src/filters.ts | 59 +++++++++++++++++++++-------- 1 file changed, 44 insertions(+), 15 deletions(-) diff --git a/packages/shared-core/src/filters.ts b/packages/shared-core/src/filters.ts index 46d765a7b5..2c4861ed60 100644 --- a/packages/shared-core/src/filters.ts +++ b/packages/shared-core/src/filters.ts @@ -390,23 +390,52 @@ export const runLuceneQuery = (docs: any[], query?: SearchQuery) => { } ) - // Match a document against all criteria const docMatch = (doc: any) => { - return ( - stringMatch(doc) && - fuzzyMatch(doc) && - rangeMatch(doc) && - equalMatch(doc) && - notEqualMatch(doc) && - emptyMatch(doc) && - notEmptyMatch(doc) && - oneOf(doc) && - contains(doc) && - containsAny(doc) && - notContains(doc) - ) - } + // Determine active filters based on query object + const activeFilterKeys = Object.entries(query || {}) + .filter( + ([key, value]) => + !["allOr", "onEmptyFilter"].includes(key) && + Object.keys(value).length > 0 + ) + .map(([key]) => key) + // Apply filters dynamically based on activeFilterKeys + const results = activeFilterKeys.map(filterKey => { + switch (filterKey) { + case "string": + return stringMatch(doc) + case "fuzzy": + return fuzzyMatch(doc) + case "range": + return rangeMatch(doc) + case "equal": + return equalMatch(doc) + case "notEqual": + return notEqualMatch(doc) + case "empty": + return emptyMatch(doc) + case "notEmpty": + return notEmptyMatch(doc) + case "oneOf": + return oneOf(doc) + case "contains": + return contains(doc) + case "containsAny": + return containsAny(doc) + case "notContains": + return notContains(doc) + default: + return true // If the filter type is not recognized, default to true (assuming pass) + } + }) + + if (query!.allOr) { + return results.some(result => result === true) + } else { + return results.every(result => result === true) + } + } // Process all docs return docs.filter(docMatch) } From e3c514e45aee8398c4b65b530c86536f7169dafd Mon Sep 17 00:00:00 2001 From: Peter Clement Date: Mon, 4 Mar 2024 09:48:47 +0000 Subject: [PATCH 02/17] Update test lucene builder and add more tests --- packages/shared-core/src/filters.ts | 57 ++++---- .../shared-core/src/tests/filters.test.ts | 128 +++++++++++++----- 2 files changed, 116 insertions(+), 69 deletions(-) diff --git a/packages/shared-core/src/filters.ts b/packages/shared-core/src/filters.ts index 2c4861ed60..5f975ff541 100644 --- a/packages/shared-core/src/filters.ts +++ b/packages/shared-core/src/filters.ts @@ -391,43 +391,32 @@ export const runLuceneQuery = (docs: any[], query?: SearchQuery) => { ) const docMatch = (doc: any) => { - // Determine active filters based on query object - const activeFilterKeys = Object.entries(query || {}) + const filterFunctions = { + string: stringMatch, + fuzzy: fuzzyMatch, + range: rangeMatch, + equal: equalMatch, + notEqual: notEqualMatch, + empty: emptyMatch, + notEmpty: notEmptyMatch, + oneOf: oneOf, + contains: contains, + containsAny: containsAny, + notContains: notContains, + } + const activeFilterKeys: (keyof typeof filterFunctions)[] = Object.entries( + query + ) .filter( - ([key, value]) => + ([key, value]: [string, any]) => !["allOr", "onEmptyFilter"].includes(key) && - Object.keys(value).length > 0 + Object.keys(value as Record).length > 0 ) - .map(([key]) => key) + .map(([key]) => key as keyof typeof filterFunctions) - // Apply filters dynamically based on activeFilterKeys - const results = activeFilterKeys.map(filterKey => { - switch (filterKey) { - case "string": - return stringMatch(doc) - case "fuzzy": - return fuzzyMatch(doc) - case "range": - return rangeMatch(doc) - case "equal": - return equalMatch(doc) - case "notEqual": - return notEqualMatch(doc) - case "empty": - return emptyMatch(doc) - case "notEmpty": - return notEmptyMatch(doc) - case "oneOf": - return oneOf(doc) - case "contains": - return contains(doc) - case "containsAny": - return containsAny(doc) - case "notContains": - return notContains(doc) - default: - return true // If the filter type is not recognized, default to true (assuming pass) - } + const results: boolean[] = activeFilterKeys.map(filterKey => { + const filterFunction = filterFunctions[filterKey] + return filterFunction ? filterFunction(doc) : true }) if (query!.allOr) { @@ -436,7 +425,7 @@ export const runLuceneQuery = (docs: any[], query?: SearchQuery) => { return results.every(result => result === true) } } - // Process all docs + return docs.filter(docMatch) } diff --git a/packages/shared-core/src/tests/filters.test.ts b/packages/shared-core/src/tests/filters.test.ts index 8586d58777..1e0a68de89 100644 --- a/packages/shared-core/src/tests/filters.test.ts +++ b/packages/shared-core/src/tests/filters.test.ts @@ -47,10 +47,7 @@ describe("runLuceneQuery", () => { }, ] - function buildQuery( - filterKey: string, - value: { [key: string]: any } - ): SearchQuery { + function buildQuery(filters: { [filterKey: string]: any }): SearchQuery { const query: SearchQuery = { string: {}, fuzzy: {}, @@ -63,8 +60,13 @@ describe("runLuceneQuery", () => { notContains: {}, oneOf: {}, containsAny: {}, + allOr: false, } - query[filterKey as SearchQueryOperators] = value + + for (const filterKey in filters) { + query[filterKey as SearchQueryOperators] = filters[filterKey] + } + return query } @@ -73,16 +75,17 @@ describe("runLuceneQuery", () => { }) it("should return matching rows for equal filter", () => { - const query = buildQuery("equal", { - order_status: 4, + const query = buildQuery({ + equal: { order_status: 4 }, }) expect(runLuceneQuery(docs, query).map(row => row.order_id)).toEqual([1, 2]) }) it("should return matching row for notEqual filter", () => { - const query = buildQuery("notEqual", { - order_status: 4, + const query = buildQuery({ + notEqual: { order_status: 4 }, }) + expect(runLuceneQuery(docs, query).map(row => row.order_id)).toEqual([3]) }) @@ -90,48 +93,56 @@ describe("runLuceneQuery", () => { expect( runLuceneQuery( docs, - buildQuery("fuzzy", { - description: "sm", + buildQuery({ + fuzzy: { description: "sm" }, }) ).map(row => row.description) ).toEqual(["Small box"]) expect( runLuceneQuery( docs, - buildQuery("string", { - description: "SM", + buildQuery({ + string: { description: "SM" }, }) ).map(row => row.description) ).toEqual(["Small box"]) }) it("should return rows within a range filter", () => { - const query = buildQuery("range", { - customer_id: { - low: 500, - high: 1000, + const query = buildQuery({ + range: { + customer_id: { + low: 500, + high: 1000, + }, }, }) + expect(runLuceneQuery(docs, query).map(row => row.order_id)).toEqual([3]) }) it("should return rows with numeric strings within a range filter", () => { - const query = buildQuery("range", { - customer_id: { - low: "500", - high: "1000", + const query = buildQuery({ + range: { + customer_id: { + low: "500", + high: "1000", + }, }, }) expect(runLuceneQuery(docs, query).map(row => row.order_id)).toEqual([3]) }) it("should return rows with ISO date strings within a range filter", () => { - const query = buildQuery("range", { - order_date: { - low: "2016-01-04T00:00:00.000Z", - high: "2016-01-11T00:00:00.000Z", + const query = buildQuery({ + range: { + order_date: { + low: "2016-01-04T00:00:00.000Z", + high: "2016-01-11T00:00:00.000Z", + }, }, }) + expect(runLuceneQuery(docs, query).map(row => row.order_id)).toEqual([2]) }) @@ -150,40 +161,87 @@ describe("runLuceneQuery", () => { label: "", }, ] - const query = buildQuery("range", { - order_date: { - low: "2016-01-04T00:00:00.000Z", - high: "2016-01-11T00:00:00.000Z", + + const query = buildQuery({ + range: { + order_date: { + low: "2016-01-04T00:00:00.000Z", + high: "2016-01-11T00:00:00.000Z", + }, }, }) + expect(runLuceneQuery(docs, query)).toEqual(docs) }) it("should return rows with matches on empty filter", () => { - const query = buildQuery("empty", { - label: null, + const query = buildQuery({ + empty: { + label: null, + }, }) + expect(runLuceneQuery(docs, query).map(row => row.order_id)).toEqual([1]) }) it("should return rows with matches on notEmpty filter", () => { - const query = buildQuery("notEmpty", { - label: null, + const query = buildQuery({ + notEmpty: { + label: null, + }, }) + expect(runLuceneQuery(docs, query).map(row => row.order_id)).toEqual([2, 3]) }) test.each([[523, 259], "523,259"])( "should return rows with matches on numeric oneOf filter", input => { - let query = buildQuery("oneOf", { - customer_id: input, + const query = buildQuery({ + oneOf: { + customer_id: input, + }, }) + expect(runLuceneQuery(docs, query).map(row => row.customer_id)).toEqual([ 259, 523, ]) } ) + + it("should return matching results if allOr is true and only one filter matches", () => { + const query = buildQuery({ + allOr: true, + oneOf: { staff_id: [10] }, + contains: { description: ["box"] }, + }) + + expect(runLuceneQuery(docs, query).map(row => row.order_id)).toEqual([ + 1, 2, 3, + ]) + }) + + // what should the name of this test be if it's the same test as above but with different operands + + it("should return matching results if allOr is true and only one filter matches with different operands", () => { + const query = buildQuery({ + allOr: true, + equal: { order_status: 4 }, + oneOf: { label: ["FRAGILE"] }, + }) + + expect(runLuceneQuery(docs, query).map(row => row.order_id)).toEqual([1, 2]) + }) + + it("should return nothing if allOr is false and only one filter matches", () => { + const query = buildQuery({ + allOr: false, + oneOf: { staff_id: [10] }, + contains: { description: ["box"] }, + }) + + expect(runLuceneQuery(docs, query).map(row => row.order_id)).toEqual([]) + }) }) describe("buildLuceneQuery", () => { From 5679acb86811c290dd84faf1d81b19d615680f6b Mon Sep 17 00:00:00 2001 From: Peter Clement Date: Mon, 4 Mar 2024 09:55:28 +0000 Subject: [PATCH 03/17] fix types --- packages/shared-core/src/filters.ts | 34 ++++++++++++++--------------- 1 file changed, 17 insertions(+), 17 deletions(-) diff --git a/packages/shared-core/src/filters.ts b/packages/shared-core/src/filters.ts index 5f975ff541..6d81bbdc62 100644 --- a/packages/shared-core/src/filters.ts +++ b/packages/shared-core/src/filters.ts @@ -391,28 +391,28 @@ export const runLuceneQuery = (docs: any[], query?: SearchQuery) => { ) const docMatch = (doc: any) => { - const filterFunctions = { - string: stringMatch, - fuzzy: fuzzyMatch, - range: rangeMatch, - equal: equalMatch, - notEqual: notEqualMatch, - empty: emptyMatch, - notEmpty: notEmptyMatch, - oneOf: oneOf, - contains: contains, - containsAny: containsAny, - notContains: notContains, - } - const activeFilterKeys: (keyof typeof filterFunctions)[] = Object.entries( - query - ) + const filterFunctions: Record boolean> = + { + string: stringMatch, + fuzzy: fuzzyMatch, + range: rangeMatch, + equal: equalMatch, + notEqual: notEqualMatch, + empty: emptyMatch, + notEmpty: notEmptyMatch, + oneOf: oneOf, + contains: contains, + containsAny: containsAny, + notContains: notContains, + } + + const activeFilterKeys: SearchQueryOperators[] = Object.entries(query) .filter( ([key, value]: [string, any]) => !["allOr", "onEmptyFilter"].includes(key) && Object.keys(value as Record).length > 0 ) - .map(([key]) => key as keyof typeof filterFunctions) + .map(([key]) => key as any) const results: boolean[] = activeFilterKeys.map(filterKey => { const filterFunction = filterFunctions[filterKey] From 3d9a7e5ddf5f76236a304e65239c899e8e865cd7 Mon Sep 17 00:00:00 2001 From: Peter Clement Date: Mon, 4 Mar 2024 10:07:06 +0000 Subject: [PATCH 04/17] fix type --- packages/shared-core/src/filters.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/shared-core/src/filters.ts b/packages/shared-core/src/filters.ts index 6d81bbdc62..0a1673e558 100644 --- a/packages/shared-core/src/filters.ts +++ b/packages/shared-core/src/filters.ts @@ -406,7 +406,7 @@ export const runLuceneQuery = (docs: any[], query?: SearchQuery) => { notContains: notContains, } - const activeFilterKeys: SearchQueryOperators[] = Object.entries(query) + const activeFilterKeys: SearchQueryOperators[] = Object.entries(query || {}) .filter( ([key, value]: [string, any]) => !["allOr", "onEmptyFilter"].includes(key) && From 13563d18dca87872dad9294c61c8018158fa191d Mon Sep 17 00:00:00 2001 From: Sam Rose Date: Tue, 5 Mar 2024 09:20:20 +0000 Subject: [PATCH 05/17] Write a failing test. --- .../src/api/routes/tests/application.spec.ts | 46 ++++++++++++++++++- .../server/src/tests/utilities/api/index.ts | 3 ++ .../server/src/tests/utilities/api/role.ts | 41 +++++++++++++++++ 3 files changed, 89 insertions(+), 1 deletion(-) create mode 100644 packages/server/src/tests/utilities/api/role.ts diff --git a/packages/server/src/api/routes/tests/application.spec.ts b/packages/server/src/api/routes/tests/application.spec.ts index 5a3be462e8..b452e8742f 100644 --- a/packages/server/src/api/routes/tests/application.spec.ts +++ b/packages/server/src/api/routes/tests/application.spec.ts @@ -16,7 +16,13 @@ import * as setup from "./utilities" import { AppStatus } from "../../../db/utils" import { events, utils, context } from "@budibase/backend-core" import env from "../../../environment" -import type { App } from "@budibase/types" +import { + PermissionLevel, + type App, + INTERNAL_TABLE_SOURCE_ID, + TableSourceType, + FieldType, +} from "@budibase/types" import tk from "timekeeper" describe("/applications", () => { @@ -256,10 +262,48 @@ describe("/applications", () => { admin: { global: false }, }) + const table = await config.api.table.save({ + name: "table", + type: "table", + sourceId: INTERNAL_TABLE_SOURCE_ID, + sourceType: TableSourceType.INTERNAL, + schema: { + name: { + type: FieldType.STRING, + name: "name", + }, + }, + }) + await config.withUser(user, async () => { const apps = await config.api.application.fetch() expect(apps).toHaveLength(0) }) + + const role = await config.api.roles.save({ + name: "Test", + inherits: "PUBLIC", + permissionId: "read_only", + version: "name", + }) + + await config.api.user.update({ + ...user, + roles: { + [config.getAppId()]: role._id!, + }, + }) + + await config.api.permission.add({ + resourceId: table._id!, + roleId: role._id!, + level: PermissionLevel.READ, + }) + + await config.withUser(user, async () => { + const apps = await config.api.application.fetch() + expect(apps).toHaveLength(1) + }) }) }) }) diff --git a/packages/server/src/tests/utilities/api/index.ts b/packages/server/src/tests/utilities/api/index.ts index fdcec3098d..d66acd86fd 100644 --- a/packages/server/src/tests/utilities/api/index.ts +++ b/packages/server/src/tests/utilities/api/index.ts @@ -11,6 +11,7 @@ import { BackupAPI } from "./backup" import { AttachmentAPI } from "./attachment" import { UserAPI } from "./user" import { QueryAPI } from "./query" +import { RoleAPI } from "./role" export default class API { table: TableAPI @@ -25,6 +26,7 @@ export default class API { attachment: AttachmentAPI user: UserAPI query: QueryAPI + roles: RoleAPI constructor(config: TestConfiguration) { this.table = new TableAPI(config) @@ -39,5 +41,6 @@ export default class API { this.attachment = new AttachmentAPI(config) this.user = new UserAPI(config) this.query = new QueryAPI(config) + this.roles = new RoleAPI(config) } } diff --git a/packages/server/src/tests/utilities/api/role.ts b/packages/server/src/tests/utilities/api/role.ts new file mode 100644 index 0000000000..4defbc1220 --- /dev/null +++ b/packages/server/src/tests/utilities/api/role.ts @@ -0,0 +1,41 @@ +import { + AccessibleRolesResponse, + FetchRolesResponse, + FindRoleResponse, + SaveRoleRequest, + SaveRoleResponse, +} from "@budibase/types" +import { Expectations, TestAPI } from "./base" + +export class RoleAPI extends TestAPI { + fetch = async (expectations?: Expectations) => { + return await this._get(`/api/roles`, { + expectations, + }) + } + + find = async (roleId: string, expectations?: Expectations) => { + return await this._get(`/api/roles/${roleId}`, { + expectations, + }) + } + + save = async (body: SaveRoleRequest, expectations?: Expectations) => { + return await this._post(`/api/roles`, { + body, + expectations, + }) + } + + destroy = async (roleId: string, expectations?: Expectations) => { + return await this._delete(`/api/roles/${roleId}`, { + expectations, + }) + } + + accesssible = async (expectations?: Expectations) => { + return await this._get(`/api/roles/accessible`, { + expectations, + }) + } +} From aa124524d4bc93e228c5ada844fccb541cb55e6e Mon Sep 17 00:00:00 2001 From: Sam Rose Date: Tue, 5 Mar 2024 10:05:05 +0000 Subject: [PATCH 06/17] Add a simpler test. --- packages/backend-core/src/cache/user.ts | 4 +- packages/server/src/api/controllers/user.ts | 3 +- .../src/api/routes/tests/application.spec.ts | 39 ++++++++++++++++++- .../src/tests/utilities/TestConfiguration.ts | 4 +- 4 files changed, 44 insertions(+), 6 deletions(-) diff --git a/packages/backend-core/src/cache/user.ts b/packages/backend-core/src/cache/user.ts index 313b9a4d4a..ecfa20f99e 100644 --- a/packages/backend-core/src/cache/user.ts +++ b/packages/backend-core/src/cache/user.ts @@ -6,7 +6,7 @@ import env from "../environment" import * as accounts from "../accounts" import { UserDB } from "../users" import { sdk } from "@budibase/shared-core" -import { User } from "@budibase/types" +import { User, UserMetadata } from "@budibase/types" const EXPIRY_SECONDS = 3600 @@ -15,7 +15,7 @@ const EXPIRY_SECONDS = 3600 */ async function populateFromDB(userId: string, tenantId: string) { const db = tenancy.getTenantDB(tenantId) - const user = await db.get(userId) + const user = await db.get(userId) user.budibaseAccess = true if (!env.SELF_HOSTED && !env.DISABLE_ACCOUNT_PORTAL) { const account = await accounts.getAccount(user.email) diff --git a/packages/server/src/api/controllers/user.ts b/packages/server/src/api/controllers/user.ts index 108e29fd3d..d1658f9820 100644 --- a/packages/server/src/api/controllers/user.ts +++ b/packages/server/src/api/controllers/user.ts @@ -1,6 +1,6 @@ import { generateUserFlagID, InternalTables } from "../../db/utils" import { getFullUser } from "../../utilities/users" -import { context } from "@budibase/backend-core" +import { cache, context } from "@budibase/backend-core" import { ContextUserMetadata, Ctx, @@ -42,6 +42,7 @@ export async function updateMetadata( // this isn't applicable to the user delete metadata.roles ctx.body = await db.put(metadata) + await cache.user.invalidateUser(user._id!) } export async function destroyMetadata(ctx: UserCtx) { diff --git a/packages/server/src/api/routes/tests/application.spec.ts b/packages/server/src/api/routes/tests/application.spec.ts index b452e8742f..7424511200 100644 --- a/packages/server/src/api/routes/tests/application.spec.ts +++ b/packages/server/src/api/routes/tests/application.spec.ts @@ -256,7 +256,44 @@ describe("/applications", () => { }) describe("permissions", () => { - it("should only return apps a user has access to", async () => { + it.only("should only return apps a user has access to", async () => { + const user = await config.createUser({ + builder: { global: false }, + admin: { global: false }, + }) + + const table = await config.api.table.save({ + name: "table", + type: "table", + sourceId: INTERNAL_TABLE_SOURCE_ID, + sourceType: TableSourceType.INTERNAL, + schema: { + name: { + type: FieldType.STRING, + name: "name", + }, + }, + }) + + await config.withUser(user, async () => { + const apps = await config.api.application.fetch() + expect(apps).toHaveLength(0) + }) + + await config.api.user.update({ + ...user, + builder: { + [config.getAppId()]: true, + }, + }) + + await config.withUser(user, async () => { + const apps = await config.api.application.fetch() + expect(apps).toHaveLength(1) + }) + }) + + it("should only return apps a user has access to through a custom role on a group", async () => { const user = await config.createUser({ builder: { global: false }, admin: { global: false }, diff --git a/packages/server/src/tests/utilities/TestConfiguration.ts b/packages/server/src/tests/utilities/TestConfiguration.ts index 2127e9d1cd..32af88836e 100644 --- a/packages/server/src/tests/utilities/TestConfiguration.ts +++ b/packages/server/src/tests/utilities/TestConfiguration.ts @@ -299,11 +299,11 @@ export default class TestConfiguration { } } - withUser(user: User, f: () => Promise) { + async withUser(user: User, f: () => Promise) { const oldUser = this.user this.user = user try { - return f() + return await f() } finally { this.user = oldUser } From f1decee0102c0bc6b4687fffa14f5b445e1c3689 Mon Sep 17 00:00:00 2001 From: Sam Rose Date: Tue, 5 Mar 2024 14:37:06 +0000 Subject: [PATCH 07/17] Get test passing. --- .../src/api/routes/tests/application.spec.ts | 8 +++++--- .../src/tests/utilities/TestConfiguration.ts | 18 +++++++++++------- 2 files changed, 16 insertions(+), 10 deletions(-) diff --git a/packages/server/src/api/routes/tests/application.spec.ts b/packages/server/src/api/routes/tests/application.spec.ts index 7424511200..6f948d9977 100644 --- a/packages/server/src/api/routes/tests/application.spec.ts +++ b/packages/server/src/api/routes/tests/application.spec.ts @@ -25,6 +25,8 @@ import { } from "@budibase/types" import tk from "timekeeper" +jest.setTimeout(99999999) + describe("/applications", () => { let config = setup.getConfig() let app: App @@ -257,7 +259,7 @@ describe("/applications", () => { describe("permissions", () => { it.only("should only return apps a user has access to", async () => { - const user = await config.createUser({ + let user = await config.createUser({ builder: { global: false }, admin: { global: false }, }) @@ -280,10 +282,10 @@ describe("/applications", () => { expect(apps).toHaveLength(0) }) - await config.api.user.update({ + user = await config.globalUser({ ...user, builder: { - [config.getAppId()]: true, + apps: [config.getProdAppId()], }, }) diff --git a/packages/server/src/tests/utilities/TestConfiguration.ts b/packages/server/src/tests/utilities/TestConfiguration.ts index 32af88836e..cfe1bf4066 100644 --- a/packages/server/src/tests/utilities/TestConfiguration.ts +++ b/packages/server/src/tests/utilities/TestConfiguration.ts @@ -363,6 +363,7 @@ export default class TestConfiguration { _id, ...existing, ...config, + _rev: existing._rev, email, roles, tenantId, @@ -372,11 +373,12 @@ export default class TestConfiguration { admin, } await sessions.createASession(_id, { - sessionId: "sessionid", + sessionId: this.sessionIdForUser(_id), tenantId: this.getTenantId(), csrfToken: this.csrfToken, }) const resp = await db.put(user) + await cache.user.invalidateUser(_id) return { _rev: resp.rev, ...user, @@ -384,9 +386,7 @@ export default class TestConfiguration { } async createUser(user: Partial = {}): Promise { - const resp = await this.globalUser(user) - await cache.user.invalidateUser(resp._id!) - return resp + return await this.globalUser(user) } async createGroup(roleId: string = roles.BUILTIN_ROLE_IDS.BASIC) { @@ -416,6 +416,10 @@ export default class TestConfiguration { }) } + sessionIdForUser(userId: string): string { + return `sessionid-${userId}` + } + async login({ roleId, userId, @@ -442,13 +446,13 @@ export default class TestConfiguration { }) } await sessions.createASession(userId, { - sessionId: "sessionid", + sessionId: this.sessionIdForUser(userId), tenantId: this.getTenantId(), }) // have to fake this const authObj = { userId, - sessionId: "sessionid", + sessionId: this.sessionIdForUser(userId), tenantId: this.getTenantId(), } const authToken = jwt.sign(authObj, coreEnv.JWT_SECRET as Secret) @@ -470,7 +474,7 @@ export default class TestConfiguration { const user = this.getUser() const authObj: AuthToken = { userId: user._id!, - sessionId: "sessionid", + sessionId: this.sessionIdForUser(user._id!), tenantId, } const authToken = jwt.sign(authObj, coreEnv.JWT_SECRET as Secret) From 182a1df9606f98da9791cb50df8355fc54eb21c2 Mon Sep 17 00:00:00 2001 From: Sam Rose Date: Tue, 5 Mar 2024 17:35:04 +0000 Subject: [PATCH 08/17] Fix the bug, I think. --- packages/backend-core/src/db/Replication.ts | 36 ++++++--- packages/backend-core/src/security/roles.ts | 5 +- packages/server/src/api/controllers/role.ts | 10 +++ .../src/api/routes/tests/application.spec.ts | 81 +++++++++---------- packages/types/src/documents/app/role.ts | 1 + 5 files changed, 72 insertions(+), 61 deletions(-) diff --git a/packages/backend-core/src/db/Replication.ts b/packages/backend-core/src/db/Replication.ts index f91a37ce8f..12c11eb9e2 100644 --- a/packages/backend-core/src/db/Replication.ts +++ b/packages/backend-core/src/db/Replication.ts @@ -1,17 +1,18 @@ +import PouchDB from "pouchdb" import { getPouchDB, closePouchDB } from "./couch" import { DocumentType } from "../constants" class Replication { - source: any - target: any - replication: any + source: PouchDB.Database + target: PouchDB.Database + replication?: Promise /** * * @param source - the DB you want to replicate or rollback to * @param target - the DB you want to replicate to, or rollback from */ - constructor({ source, target }: any) { + constructor({ source, target }: { source: string; target: string }) { this.source = getPouchDB(source) this.target = getPouchDB(target) } @@ -40,7 +41,7 @@ class Replication { * Two way replication operation, intended to be promise based. * @param opts - PouchDB replication options */ - sync(opts = {}) { + sync(opts: PouchDB.Replication.SyncOptions = {}) { this.replication = this.promisify(this.source.sync, opts) return this.replication } @@ -49,18 +50,31 @@ class Replication { * One way replication operation, intended to be promise based. * @param opts - PouchDB replication options */ - replicate(opts = {}) { + replicate(opts: PouchDB.Replication.ReplicateOptions = {}) { this.replication = this.promisify(this.source.replicate.to, opts) return this.replication } - appReplicateOpts() { + appReplicateOpts( + opts: PouchDB.Replication.ReplicateOptions = {} + ): PouchDB.Replication.ReplicateOptions { + if (typeof opts.filter === "string") { + return opts + } + + const filter = opts.filter + delete opts.filter + return { - filter: (doc: any) => { + ...opts, + filter: (doc: any, params: any) => { if (doc._id && doc._id.startsWith(DocumentType.AUTOMATION_LOG)) { return false } - return doc._id !== DocumentType.APP_METADATA + if (doc._id === DocumentType.APP_METADATA) { + return false + } + return filter ? filter(doc, params) : true }, } } @@ -75,10 +89,6 @@ class Replication { // take the opportunity to remove deleted tombstones await this.replicate() } - - cancel() { - this.replication.cancel() - } } export default Replication diff --git a/packages/backend-core/src/security/roles.ts b/packages/backend-core/src/security/roles.ts index 01473ad991..a64be6b319 100644 --- a/packages/backend-core/src/security/roles.ts +++ b/packages/backend-core/src/security/roles.ts @@ -101,10 +101,7 @@ export function getBuiltinRole(roleId: string): Role | undefined { /** * Works through the inheritance ranks to see how far up the builtin stack this ID is. */ -export function builtinRoleToNumber(id?: string) { - if (!id) { - return 0 - } +export function builtinRoleToNumber(id: string) { const builtins = getBuiltinRoles() const MAX = Object.values(builtins).length + 1 if (id === BUILTIN_IDS.ADMIN || id === BUILTIN_IDS.BUILDER) { diff --git a/packages/server/src/api/controllers/role.ts b/packages/server/src/api/controllers/role.ts index b3eb61a255..fff58da86e 100644 --- a/packages/server/src/api/controllers/role.ts +++ b/packages/server/src/api/controllers/role.ts @@ -106,6 +106,16 @@ export async function save(ctx: UserCtx) { ) role._rev = result.rev ctx.body = role + + const replication = new dbCore.Replication({ + source: context.getDevAppDB().name, + target: context.getProdAppDB().name, + }) + await replication.replicate({ + filter: (doc: any, params: any) => { + return doc._id === _id + }, + }) } export async function destroy(ctx: UserCtx) { diff --git a/packages/server/src/api/routes/tests/application.spec.ts b/packages/server/src/api/routes/tests/application.spec.ts index 6f948d9977..63c9fe44b8 100644 --- a/packages/server/src/api/routes/tests/application.spec.ts +++ b/packages/server/src/api/routes/tests/application.spec.ts @@ -16,16 +16,9 @@ import * as setup from "./utilities" import { AppStatus } from "../../../db/utils" import { events, utils, context } from "@budibase/backend-core" import env from "../../../environment" -import { - PermissionLevel, - type App, - INTERNAL_TABLE_SOURCE_ID, - TableSourceType, - FieldType, -} from "@budibase/types" +import { type App } from "@budibase/types" import tk from "timekeeper" - -jest.setTimeout(99999999) +import * as uuid from "uuid" describe("/applications", () => { let config = setup.getConfig() @@ -258,25 +251,12 @@ describe("/applications", () => { }) describe("permissions", () => { - it.only("should only return apps a user has access to", async () => { + it("should only return apps a user has access to", async () => { let user = await config.createUser({ builder: { global: false }, admin: { global: false }, }) - const table = await config.api.table.save({ - name: "table", - type: "table", - sourceId: INTERNAL_TABLE_SOURCE_ID, - sourceType: TableSourceType.INTERNAL, - schema: { - name: { - type: FieldType.STRING, - name: "name", - }, - }, - }) - await config.withUser(user, async () => { const apps = await config.api.application.fetch() expect(apps).toHaveLength(0) @@ -295,25 +275,12 @@ describe("/applications", () => { }) }) - it("should only return apps a user has access to through a custom role on a group", async () => { - const user = await config.createUser({ + it("should only return apps a user has access to through a custom role", async () => { + let user = await config.createUser({ builder: { global: false }, admin: { global: false }, }) - const table = await config.api.table.save({ - name: "table", - type: "table", - sourceId: INTERNAL_TABLE_SOURCE_ID, - sourceType: TableSourceType.INTERNAL, - schema: { - name: { - type: FieldType.STRING, - name: "name", - }, - }, - }) - await config.withUser(user, async () => { const apps = await config.api.application.fetch() expect(apps).toHaveLength(0) @@ -326,17 +293,43 @@ describe("/applications", () => { version: "name", }) - await config.api.user.update({ + user = await config.globalUser({ ...user, roles: { - [config.getAppId()]: role._id!, + [config.getProdAppId()]: role.name, }, }) - await config.api.permission.add({ - resourceId: table._id!, - roleId: role._id!, - level: PermissionLevel.READ, + await config.withUser(user, async () => { + const apps = await config.api.application.fetch() + expect(apps).toHaveLength(1) + }) + }) + + it.only("should only return apps a user has access to through a custom role on a group", async () => { + let user = await config.createUser({ + builder: { global: false }, + admin: { global: false }, + }) + + await config.withUser(user, async () => { + const apps = await config.api.application.fetch() + expect(apps).toHaveLength(0) + }) + + const roleName = uuid.v4().replace(/-/g, "") + const role = await config.api.roles.save({ + name: roleName, + inherits: "PUBLIC", + permissionId: "read_only", + version: "name", + }) + + const group = await config.createGroup(role._id!) + + user = await config.globalUser({ + ...user, + userGroups: [group._id!], }) await config.withUser(user, async () => { diff --git a/packages/types/src/documents/app/role.ts b/packages/types/src/documents/app/role.ts index d126a67b16..f32ba810b0 100644 --- a/packages/types/src/documents/app/role.ts +++ b/packages/types/src/documents/app/role.ts @@ -5,4 +5,5 @@ export interface Role extends Document { inherits?: string permissions: { [key: string]: string[] } version?: string + name: string } From 11704ea983b5ec3d7426b6927afa41d1cdea81a7 Mon Sep 17 00:00:00 2001 From: Sam Rose Date: Tue, 5 Mar 2024 17:40:38 +0000 Subject: [PATCH 09/17] TODO. --- packages/server/src/api/controllers/role.ts | 2 ++ 1 file changed, 2 insertions(+) diff --git a/packages/server/src/api/controllers/role.ts b/packages/server/src/api/controllers/role.ts index fff58da86e..6b62c568e2 100644 --- a/packages/server/src/api/controllers/role.ts +++ b/packages/server/src/api/controllers/role.ts @@ -107,6 +107,8 @@ export async function save(ctx: UserCtx) { role._rev = result.rev ctx.body = role + // TODO: need to check that the prod DB actually exists, I think it won't + // if the app has never been published. const replication = new dbCore.Replication({ source: context.getDevAppDB().name, target: context.getProdAppDB().name, From 2b206f2105681140a1079ba49bcc434df1e7f489 Mon Sep 17 00:00:00 2001 From: Sam Rose Date: Wed, 6 Mar 2024 10:00:02 +0000 Subject: [PATCH 10/17] Fix the TODO I left myself last night. --- packages/backend-core/src/db/Replication.ts | 41 +++++---------------- packages/server/src/api/controllers/role.ts | 25 +++++++------ 2 files changed, 23 insertions(+), 43 deletions(-) diff --git a/packages/backend-core/src/db/Replication.ts b/packages/backend-core/src/db/Replication.ts index 12c11eb9e2..9c960d76dd 100644 --- a/packages/backend-core/src/db/Replication.ts +++ b/packages/backend-core/src/db/Replication.ts @@ -5,56 +5,33 @@ import { DocumentType } from "../constants" class Replication { source: PouchDB.Database target: PouchDB.Database - replication?: Promise - /** - * - * @param source - the DB you want to replicate or rollback to - * @param target - the DB you want to replicate to, or rollback from - */ constructor({ source, target }: { source: string; target: string }) { this.source = getPouchDB(source) this.target = getPouchDB(target) } - close() { - return Promise.all([closePouchDB(this.source), closePouchDB(this.target)]) + async close() { + await Promise.all([closePouchDB(this.source), closePouchDB(this.target)]) } - promisify(operation: any, opts = {}) { - return new Promise(resolve => { - operation(this.target, opts) - .on("denied", function (err: any) { + replicate(opts: PouchDB.Replication.ReplicateOptions = {}) { + return new Promise>(resolve => { + this.source.replicate + .to(this.target, opts) + .on("denied", function (err) { // a document failed to replicate (e.g. due to permissions) throw new Error(`Denied: Document failed to replicate ${err}`) }) - .on("complete", function (info: any) { + .on("complete", function (info) { return resolve(info) }) - .on("error", function (err: any) { + .on("error", function (err) { throw new Error(`Replication Error: ${err}`) }) }) } - /** - * Two way replication operation, intended to be promise based. - * @param opts - PouchDB replication options - */ - sync(opts: PouchDB.Replication.SyncOptions = {}) { - this.replication = this.promisify(this.source.sync, opts) - return this.replication - } - - /** - * One way replication operation, intended to be promise based. - * @param opts - PouchDB replication options - */ - replicate(opts: PouchDB.Replication.ReplicateOptions = {}) { - this.replication = this.promisify(this.source.replicate.to, opts) - return this.replication - } - appReplicateOpts( opts: PouchDB.Replication.ReplicateOptions = {} ): PouchDB.Replication.ReplicateOptions { diff --git a/packages/server/src/api/controllers/role.ts b/packages/server/src/api/controllers/role.ts index 6b62c568e2..84179d8dbc 100644 --- a/packages/server/src/api/controllers/role.ts +++ b/packages/server/src/api/controllers/role.ts @@ -107,17 +107,20 @@ export async function save(ctx: UserCtx) { role._rev = result.rev ctx.body = role - // TODO: need to check that the prod DB actually exists, I think it won't - // if the app has never been published. - const replication = new dbCore.Replication({ - source: context.getDevAppDB().name, - target: context.getProdAppDB().name, - }) - await replication.replicate({ - filter: (doc: any, params: any) => { - return doc._id === _id - }, - }) + const devDb = context.getDevAppDB() + const prodDb = context.getProdAppDB() + + if (await prodDb.exists()) { + const replication = new dbCore.Replication({ + source: devDb.name, + target: prodDb.name, + }) + await replication.replicate({ + filter: (doc: any, params: any) => { + return doc._id === _id + }, + }) + } } export async function destroy(ctx: UserCtx) { From b232371efff95f7925c93960ba92862324cb1a46 Mon Sep 17 00:00:00 2001 From: Peter Clement Date: Wed, 6 Mar 2024 10:01:42 +0000 Subject: [PATCH 11/17] remove uneeded comment --- packages/shared-core/src/tests/filters.test.ts | 2 -- 1 file changed, 2 deletions(-) diff --git a/packages/shared-core/src/tests/filters.test.ts b/packages/shared-core/src/tests/filters.test.ts index 1e0a68de89..0cf7e0e92a 100644 --- a/packages/shared-core/src/tests/filters.test.ts +++ b/packages/shared-core/src/tests/filters.test.ts @@ -221,8 +221,6 @@ describe("runLuceneQuery", () => { ]) }) - // what should the name of this test be if it's the same test as above but with different operands - it("should return matching results if allOr is true and only one filter matches with different operands", () => { const query = buildQuery({ allOr: true, From eb00ce401f9819406acde58c60018945bc95864e Mon Sep 17 00:00:00 2001 From: Peter Clement Date: Wed, 6 Mar 2024 10:10:28 +0000 Subject: [PATCH 12/17] pr comments --- packages/shared-core/src/filters.ts | 7 ++++--- packages/shared-core/src/tests/filters.test.ts | 10 ++++++++++ 2 files changed, 14 insertions(+), 3 deletions(-) diff --git a/packages/shared-core/src/filters.ts b/packages/shared-core/src/filters.ts index 0a1673e558..84b6076d56 100644 --- a/packages/shared-core/src/filters.ts +++ b/packages/shared-core/src/filters.ts @@ -12,6 +12,7 @@ import { import dayjs from "dayjs" import { OperatorOptions, SqlNumberTypeRangeMap } from "./constants" import { deepGet } from "./helpers" +import test from "node:test" const HBS_REGEX = /{{([^{].*?)}}/g @@ -359,6 +360,7 @@ export const runLuceneQuery = (docs: any[], query?: SearchQuery) => { const oneOf = match( SearchQueryOperators.ONE_OF, (docValue: any, testValue: any) => { + console.log(testValue) if (typeof testValue === "string") { testValue = testValue.split(",") if (typeof docValue === "number") { @@ -410,13 +412,13 @@ export const runLuceneQuery = (docs: any[], query?: SearchQuery) => { .filter( ([key, value]: [string, any]) => !["allOr", "onEmptyFilter"].includes(key) && + value && Object.keys(value as Record).length > 0 ) .map(([key]) => key as any) const results: boolean[] = activeFilterKeys.map(filterKey => { - const filterFunction = filterFunctions[filterKey] - return filterFunction ? filterFunction(doc) : true + return filterFunctions[filterKey]?.(doc) ?? false }) if (query!.allOr) { @@ -425,7 +427,6 @@ export const runLuceneQuery = (docs: any[], query?: SearchQuery) => { return results.every(result => result === true) } } - return docs.filter(docMatch) } diff --git a/packages/shared-core/src/tests/filters.test.ts b/packages/shared-core/src/tests/filters.test.ts index 0cf7e0e92a..1f8f534f0d 100644 --- a/packages/shared-core/src/tests/filters.test.ts +++ b/packages/shared-core/src/tests/filters.test.ts @@ -240,6 +240,16 @@ describe("runLuceneQuery", () => { expect(runLuceneQuery(docs, query).map(row => row.order_id)).toEqual([]) }) + + it("should handle when a value is null or undefined", () => { + const query = buildQuery({ + allOr: true, + equal: { order_status: null }, + oneOf: { label: ["FRAGILE"] }, + }) + + expect(runLuceneQuery(docs, query).map(row => row.order_id)).toEqual([2]) + }) }) describe("buildLuceneQuery", () => { From 1f107041a108aeaf677da20659819bfe2d06ec03 Mon Sep 17 00:00:00 2001 From: Peter Clement Date: Wed, 6 Mar 2024 11:57:45 +0000 Subject: [PATCH 13/17] use vitest each --- .../shared-core/src/tests/filters.test.ts | 23 +++++++------------ 1 file changed, 8 insertions(+), 15 deletions(-) diff --git a/packages/shared-core/src/tests/filters.test.ts b/packages/shared-core/src/tests/filters.test.ts index 1f8f534f0d..de969562af 100644 --- a/packages/shared-core/src/tests/filters.test.ts +++ b/packages/shared-core/src/tests/filters.test.ts @@ -209,16 +209,19 @@ describe("runLuceneQuery", () => { } ) - it("should return matching results if allOr is true and only one filter matches", () => { + test.each([ + [false, []], + [true, [1, 2, 3]], + ])("should return %s if allOr is %s ", (allOr, expectedResult) => { const query = buildQuery({ - allOr: true, + allOr, oneOf: { staff_id: [10] }, contains: { description: ["box"] }, }) - expect(runLuceneQuery(docs, query).map(row => row.order_id)).toEqual([ - 1, 2, 3, - ]) + expect(runLuceneQuery(docs, query).map(row => row.order_id)).toEqual( + expectedResult + ) }) it("should return matching results if allOr is true and only one filter matches with different operands", () => { @@ -231,16 +234,6 @@ describe("runLuceneQuery", () => { expect(runLuceneQuery(docs, query).map(row => row.order_id)).toEqual([1, 2]) }) - it("should return nothing if allOr is false and only one filter matches", () => { - const query = buildQuery({ - allOr: false, - oneOf: { staff_id: [10] }, - contains: { description: ["box"] }, - }) - - expect(runLuceneQuery(docs, query).map(row => row.order_id)).toEqual([]) - }) - it("should handle when a value is null or undefined", () => { const query = buildQuery({ allOr: true, From 632b9a26f4313216c28458db46dc9334aea7e909 Mon Sep 17 00:00:00 2001 From: Peter Clement Date: Wed, 6 Mar 2024 14:42:30 +0000 Subject: [PATCH 14/17] remove log --- packages/shared-core/src/filters.ts | 2 -- 1 file changed, 2 deletions(-) diff --git a/packages/shared-core/src/filters.ts b/packages/shared-core/src/filters.ts index 84b6076d56..d9fe533c88 100644 --- a/packages/shared-core/src/filters.ts +++ b/packages/shared-core/src/filters.ts @@ -12,7 +12,6 @@ import { import dayjs from "dayjs" import { OperatorOptions, SqlNumberTypeRangeMap } from "./constants" import { deepGet } from "./helpers" -import test from "node:test" const HBS_REGEX = /{{([^{].*?)}}/g @@ -360,7 +359,6 @@ export const runLuceneQuery = (docs: any[], query?: SearchQuery) => { const oneOf = match( SearchQueryOperators.ONE_OF, (docValue: any, testValue: any) => { - console.log(testValue) if (typeof testValue === "string") { testValue = testValue.split(",") if (typeof docValue === "number") { From 1b387d359c669b9672ab1de10e2811bb72f26811 Mon Sep 17 00:00:00 2001 From: Conor Webb <126772285+ConorWebb96@users.noreply.github.com> Date: Wed, 6 Mar 2024 16:32:00 +0000 Subject: [PATCH 15/17] Added icon to button component, reworked icon display code. (#12624) * Added icons to buttons, removed svg code added icon component code. * Added icon functionality to button group component. * Added gap to button manifest * Added gap to button setitngs. * Added gap setting to ButtonGroup component * Added the ability to clear the selected icon. * Added enter search to icon select * Removed use:styleable as its for the button * Moved non internal props up * Fixed broken DynamicFilter component icon * Updated DynamicFilter icon to a better suited one --------- Co-authored-by: melohagan <101575380+melohagan@users.noreply.github.com> --- .../controls/IconSelect/IconSelect.svelte | 17 ++++++++-- packages/client/manifest.json | 32 +++++++++++++++++++ .../client/src/components/app/Button.svelte | 24 +++++++------- .../src/components/app/ButtonGroup.svelte | 4 ++- .../app/dynamic-filter/DynamicFilter.svelte | 4 +-- 5 files changed, 65 insertions(+), 16 deletions(-) diff --git a/packages/builder/src/components/design/settings/controls/IconSelect/IconSelect.svelte b/packages/builder/src/components/design/settings/controls/IconSelect/IconSelect.svelte index 0c68c3c3e6..a28f5cfb3b 100644 --- a/packages/builder/src/components/design/settings/controls/IconSelect/IconSelect.svelte +++ b/packages/builder/src/components/design/settings/controls/IconSelect/IconSelect.svelte @@ -139,10 +139,22 @@ {/each}
-
- +
+ { + if (event.key === "Enter") { + searchForIcon() + } + }} + thin + placeholder="Search Icon" + />
+ {#if value} + + {/if}
@@ -239,6 +251,7 @@ flex-flow: row nowrap; width: 100%; padding-right: 15px; + gap: 10px; } .input-wrapper { width: 510px; diff --git a/packages/client/manifest.json b/packages/client/manifest.json index 43b75ebe26..10f9c5f412 100644 --- a/packages/client/manifest.json +++ b/packages/client/manifest.json @@ -525,6 +525,38 @@ "barTitle": "Disable button", "key": "disabled" }, + { + "type": "icon", + "label": "Icon", + "key": "icon" + }, + { + "type": "select", + "label": "Gap", + "key": "gap", + "showInBar": true, + "barStyle": "picker", + "dependsOn": "icon", + "options": [ + { + "label": "None", + "value": "N" + }, + { + "label": "Small", + "value": "S" + }, + { + "label": "Medium", + "value": "M" + }, + { + "label": "Large", + "value": "L" + } + ], + "defaultValue": "M" + }, { "type": "event", "label": "On click", diff --git a/packages/client/src/components/app/Button.svelte b/packages/client/src/components/app/Button.svelte index 361e64a983..c43face1bb 100644 --- a/packages/client/src/components/app/Button.svelte +++ b/packages/client/src/components/app/Button.svelte @@ -13,9 +13,10 @@ export let size = "M" export let type = "cta" export let quiet = false + export let icon = null + export let gap = "M" // For internal use only for now - not defined in the manifest - export let icon = null export let active = false const handleOnClick = async () => { @@ -47,7 +48,7 @@ {#key $component.editing} @@ -92,4 +85,13 @@ .active { color: var(--spectrum-global-color-blue-600); } + .gap-S { + gap: 8px; + } + .gap-M { + gap: 16px; + } + .gap-L { + gap: 32px; + } diff --git a/packages/client/src/components/app/ButtonGroup.svelte b/packages/client/src/components/app/ButtonGroup.svelte index 3ee703e253..2cf6b3db7d 100644 --- a/packages/client/src/components/app/ButtonGroup.svelte +++ b/packages/client/src/components/app/ButtonGroup.svelte @@ -20,7 +20,7 @@ wrap: true, }} > - {#each buttons as { text, type, quiet, disabled, onClick, size }} + {#each buttons as { text, type, quiet, disabled, onClick, size, icon, gap }} diff --git a/packages/client/src/components/app/dynamic-filter/DynamicFilter.svelte b/packages/client/src/components/app/dynamic-filter/DynamicFilter.svelte index 199a6122ab..549574e89b 100644 --- a/packages/client/src/components/app/dynamic-filter/DynamicFilter.svelte +++ b/packages/client/src/components/app/dynamic-filter/DynamicFilter.svelte @@ -92,9 +92,9 @@ {#if schemaLoaded}