From c6a413f3991d660436f0c2b7deb0191bd47df482 Mon Sep 17 00:00:00 2001 From: mike12345567 Date: Wed, 23 Oct 2024 14:47:06 +0100 Subject: [PATCH 01/14] Quick fix for role validation, permissionId is no longer required. --- packages/server/src/api/controllers/role.ts | 4 ++++ packages/server/src/api/routes/tests/role.spec.ts | 13 +++++++++++++ packages/server/src/api/routes/utils/validators.ts | 2 +- packages/types/src/api/web/role.ts | 2 +- 4 files changed, 19 insertions(+), 2 deletions(-) diff --git a/packages/server/src/api/controllers/role.ts b/packages/server/src/api/controllers/role.ts index 1047711983..35f221293f 100644 --- a/packages/server/src/api/controllers/role.ts +++ b/packages/server/src/api/controllers/role.ts @@ -137,6 +137,10 @@ export async function save(ctx: UserCtx) { permissionId = PermissionLevel.WRITE } + if (!permissionId) { + ctx.throw(400, "Role requires permissionId to be specified.") + } + const role = new roles.Role(_id, name, permissionId, { displayName: uiMetadata?.displayName || name, description: uiMetadata?.description || "Custom role", diff --git a/packages/server/src/api/routes/tests/role.spec.ts b/packages/server/src/api/routes/tests/role.spec.ts index adb83ca793..e3dfcb40f1 100644 --- a/packages/server/src/api/routes/tests/role.spec.ts +++ b/packages/server/src/api/routes/tests/role.spec.ts @@ -58,6 +58,19 @@ describe("/roles", () => { }) expect(res.inherits).toEqual([BUILTIN_ROLE_IDS.BASIC]) }) + + it("save role without permissionId", async () => { + const res = await config.api.roles.save( + { + ...basicRole(), + permissionId: undefined, + }, + { + status: 200, + } + ) + expect(res.permissionId).toEqual(PermissionLevel.WRITE) + }) }) describe("update", () => { diff --git a/packages/server/src/api/routes/utils/validators.ts b/packages/server/src/api/routes/utils/validators.ts index 02a1a2d060..962735ded7 100644 --- a/packages/server/src/api/routes/utils/validators.ts +++ b/packages/server/src/api/routes/utils/validators.ts @@ -215,7 +215,7 @@ export function roleValidator() { // this is the base permission ID (for now a built in) permissionId: Joi.string() .valid(...Object.values(permissions.BuiltinPermissionID)) - .required(), + .optional(), permissions: Joi.object() .pattern( /.*/, diff --git a/packages/types/src/api/web/role.ts b/packages/types/src/api/web/role.ts index df439e84e7..aa5dd2f31b 100644 --- a/packages/types/src/api/web/role.ts +++ b/packages/types/src/api/web/role.ts @@ -6,7 +6,7 @@ export interface SaveRoleRequest { _rev?: string name: string inherits?: string | string[] - permissionId: string + permissionId?: string permissions?: Record version?: string uiMetadata?: RoleUIMetadata From 8f21802e6ea5a3cf0cea9b06f8276d9b59250277 Mon Sep 17 00:00:00 2001 From: mike12345567 Date: Wed, 23 Oct 2024 15:08:59 +0100 Subject: [PATCH 02/14] Refactor, correct to the BuiltinPermissionID rather than PermissionLevel, these are different. --- .../backend-core/src/security/permissions.ts | 14 +++---- packages/backend-core/src/security/roles.ts | 7 ++-- .../src/security/tests/permissions.spec.ts | 7 ++-- packages/server/src/api/controllers/role.ts | 5 ++- .../src/api/routes/tests/application.spec.ts | 6 +-- .../src/api/routes/tests/permissions.spec.ts | 15 +++++-- .../server/src/api/routes/tests/role.spec.ts | 41 +++++++++++-------- .../src/api/routes/tests/screen.spec.ts | 8 ++-- .../server/src/api/routes/utils/validators.ts | 3 +- .../server/src/tests/utilities/structures.ts | 5 ++- .../src/helpers/tests/roles.spec.ts | 4 +- packages/types/src/api/web/role.ts | 4 +- packages/types/src/documents/app/role.ts | 4 +- packages/types/src/sdk/permissions.ts | 9 ++++ .../src/api/routes/global/tests/roles.spec.ts | 6 +-- 15 files changed, 83 insertions(+), 55 deletions(-) diff --git a/packages/backend-core/src/security/permissions.ts b/packages/backend-core/src/security/permissions.ts index 4ed2cd3954..929ae92909 100644 --- a/packages/backend-core/src/security/permissions.ts +++ b/packages/backend-core/src/security/permissions.ts @@ -1,4 +1,8 @@ -import { PermissionLevel, PermissionType } from "@budibase/types" +import { + PermissionLevel, + PermissionType, + BuiltinPermissionID, +} from "@budibase/types" import flatten from "lodash/flatten" import cloneDeep from "lodash/fp/cloneDeep" @@ -57,14 +61,6 @@ export function getAllowedLevels(userPermLevel: PermissionLevel): string[] { } } -export enum BuiltinPermissionID { - PUBLIC = "public", - READ_ONLY = "read_only", - WRITE = "write", - ADMIN = "admin", - POWER = "power", -} - export const BUILTIN_PERMISSIONS: { [key in keyof typeof BuiltinPermissionID]: { _id: (typeof BuiltinPermissionID)[key] diff --git a/packages/backend-core/src/security/roles.ts b/packages/backend-core/src/security/roles.ts index 7d9a11db48..df7f41e6ce 100644 --- a/packages/backend-core/src/security/roles.ts +++ b/packages/backend-core/src/security/roles.ts @@ -1,5 +1,4 @@ import semver from "semver" -import { BuiltinPermissionID, PermissionLevel } from "./permissions" import { prefixRoleID, getRoleParams, @@ -14,6 +13,8 @@ import { RoleUIMetadata, Database, App, + BuiltinPermissionID, + PermissionLevel, } from "@budibase/types" import cloneDeep from "lodash/fp/cloneDeep" import { RoleColor, helpers } from "@budibase/shared-core" @@ -50,7 +51,7 @@ export class Role implements RoleDoc { _id: string _rev?: string name: string - permissionId: string + permissionId: BuiltinPermissionID inherits?: string | string[] version?: string permissions: Record = {} @@ -59,7 +60,7 @@ export class Role implements RoleDoc { constructor( id: string, name: string, - permissionId: string, + permissionId: BuiltinPermissionID, uiMetadata?: RoleUIMetadata ) { this._id = id diff --git a/packages/backend-core/src/security/tests/permissions.spec.ts b/packages/backend-core/src/security/tests/permissions.spec.ts index 39348646fb..f98833c7cd 100644 --- a/packages/backend-core/src/security/tests/permissions.spec.ts +++ b/packages/backend-core/src/security/tests/permissions.spec.ts @@ -1,6 +1,7 @@ import cloneDeep from "lodash/cloneDeep" import * as permissions from "../permissions" import { BUILTIN_ROLE_IDS } from "../roles" +import { BuiltinPermissionID } from "@budibase/types" describe("levelToNumber", () => { it("should return 0 for EXECUTE", () => { @@ -77,7 +78,7 @@ describe("doesHaveBasePermission", () => { const rolesHierarchy = [ { roleId: BUILTIN_ROLE_IDS.ADMIN, - permissionId: permissions.BuiltinPermissionID.ADMIN, + permissionId: BuiltinPermissionID.ADMIN, }, ] expect( @@ -91,7 +92,7 @@ describe("doesHaveBasePermission", () => { const rolesHierarchy = [ { roleId: BUILTIN_ROLE_IDS.PUBLIC, - permissionId: permissions.BuiltinPermissionID.PUBLIC, + permissionId: BuiltinPermissionID.PUBLIC, }, ] expect( @@ -129,7 +130,7 @@ describe("getBuiltinPermissions", () => { describe("getBuiltinPermissionByID", () => { it("returns correct permission object for valid ID", () => { const expectedPermission = { - _id: permissions.BuiltinPermissionID.PUBLIC, + _id: BuiltinPermissionID.PUBLIC, name: "Public", permissions: [ new permissions.Permission( diff --git a/packages/server/src/api/controllers/role.ts b/packages/server/src/api/controllers/role.ts index 35f221293f..f8af71fe5c 100644 --- a/packages/server/src/api/controllers/role.ts +++ b/packages/server/src/api/controllers/role.ts @@ -19,6 +19,7 @@ import { UserMetadata, DocumentType, PermissionLevel, + BuiltinPermissionID, } from "@budibase/types" import { RoleColor, sdk as sharedSdk, helpers } from "@budibase/shared-core" import sdk from "../../sdk" @@ -134,7 +135,9 @@ export async function save(ctx: UserCtx) { } // assume write permission level for newly created roles if (isCreate && !permissionId) { - permissionId = PermissionLevel.WRITE + permissionId = BuiltinPermissionID.WRITE + } else if (!permissionId && dbRole?.permissionId) { + permissionId = dbRole.permissionId } if (!permissionId) { diff --git a/packages/server/src/api/routes/tests/application.spec.ts b/packages/server/src/api/routes/tests/application.spec.ts index 47b6776610..729f899379 100644 --- a/packages/server/src/api/routes/tests/application.spec.ts +++ b/packages/server/src/api/routes/tests/application.spec.ts @@ -16,7 +16,7 @@ import * as setup from "./utilities" import { AppStatus } from "../../../db/utils" import { events, utils, context, features } from "@budibase/backend-core" import env from "../../../environment" -import { type App } from "@budibase/types" +import { type App, BuiltinPermissionID } from "@budibase/types" import tk from "timekeeper" import * as uuid from "uuid" import { structures } from "@budibase/backend-core/tests" @@ -80,7 +80,7 @@ describe("/applications", () => { const role = await config.api.roles.save({ name: "Test", inherits: "PUBLIC", - permissionId: "read_only", + permissionId: BuiltinPermissionID.READ_ONLY, version: "name", }) @@ -112,7 +112,7 @@ describe("/applications", () => { const role = await config.api.roles.save({ name: roleName, inherits: "PUBLIC", - permissionId: "read_only", + permissionId: BuiltinPermissionID.READ_ONLY, version: "name", }) diff --git a/packages/server/src/api/routes/tests/permissions.spec.ts b/packages/server/src/api/routes/tests/permissions.spec.ts index 7a9bb2f373..193d018c24 100644 --- a/packages/server/src/api/routes/tests/permissions.spec.ts +++ b/packages/server/src/api/routes/tests/permissions.spec.ts @@ -1,5 +1,12 @@ import { roles } from "@budibase/backend-core" -import { Document, PermissionLevel, Role, Row, Table } from "@budibase/types" +import { + BuiltinPermissionID, + Document, + PermissionLevel, + Role, + Row, + Table, +} from "@budibase/types" import * as setup from "./utilities" import { generator, mocks } from "@budibase/backend-core/tests" @@ -304,7 +311,7 @@ describe("/permission", () => { role1 = await config.api.roles.save( { name: "test_1", - permissionId: PermissionLevel.WRITE, + permissionId: BuiltinPermissionID.WRITE, inherits: BUILTIN_ROLE_IDS.BASIC, }, { status: 200 } @@ -312,7 +319,7 @@ describe("/permission", () => { role2 = await config.api.roles.save( { name: "test_2", - permissionId: PermissionLevel.WRITE, + permissionId: BuiltinPermissionID.WRITE, inherits: BUILTIN_ROLE_IDS.BASIC, }, { status: 200 } @@ -345,7 +352,7 @@ describe("/permission", () => { it("should be able to fetch two tables, with different roles, using multi-inheritance", async () => { const role3 = await config.api.roles.save({ name: "role3", - permissionId: PermissionLevel.WRITE, + permissionId: BuiltinPermissionID.WRITE, inherits: [role1._id!, role2._id!], }) diff --git a/packages/server/src/api/routes/tests/role.spec.ts b/packages/server/src/api/routes/tests/role.spec.ts index e3dfcb40f1..1eb59c5a0c 100644 --- a/packages/server/src/api/routes/tests/role.spec.ts +++ b/packages/server/src/api/routes/tests/role.spec.ts @@ -1,15 +1,9 @@ -import { - roles, - events, - permissions, - db as dbCore, -} from "@budibase/backend-core" +import { roles, events, db as dbCore } from "@budibase/backend-core" import * as setup from "./utilities" -import { PermissionLevel } from "@budibase/types" +import { PermissionLevel, BuiltinPermissionID } from "@budibase/types" const { basicRole } = setup.structures const { BUILTIN_ROLE_IDS } = roles -const { BuiltinPermissionID } = permissions const LOOP_ERROR = "Role inheritance contains a loop, this is not supported" @@ -162,7 +156,7 @@ describe("/roles", () => { _id: id1, name: id1, permissions: {}, - permissionId: "write", + permissionId: BuiltinPermissionID.WRITE, version: "name", inherits: ["POWER"], }) @@ -170,7 +164,7 @@ describe("/roles", () => { _id: id2, permissions: {}, name: id2, - permissionId: "write", + permissionId: BuiltinPermissionID.WRITE, version: "name", inherits: [id1], }) @@ -189,10 +183,25 @@ describe("/roles", () => { inherits: [BUILTIN_ROLE_IDS.ADMIN], }) // remove the roles so that it will default back to DB roles, then save again - delete res.inherits - const updatedRes = await config.api.roles.save(res) + const updatedRes = await config.api.roles.save({ + ...res, + inherits: undefined, + }) expect(updatedRes.inherits).toEqual([BUILTIN_ROLE_IDS.ADMIN]) }) + + it("handle updating a role, without its permissionId", async () => { + const res = await config.api.roles.save({ + ...basicRole(), + permissionId: BuiltinPermissionID.READ_ONLY, + }) + // permission ID can be removed during update + const updatedRes = await config.api.roles.save({ + ...res, + permissionId: undefined, + }) + expect(updatedRes.permissionId).toEqual(PermissionLevel.READ) + }) }) describe("fetch", () => { @@ -329,7 +338,7 @@ describe("/roles", () => { await config.api.roles.save({ name: customRoleName, inherits: roles.BUILTIN_ROLE_IDS.BASIC, - permissionId: permissions.BuiltinPermissionID.READ_ONLY, + permissionId: BuiltinPermissionID.READ_ONLY, version: "name", }) await config.withHeaders( @@ -369,19 +378,19 @@ describe("/roles", () => { const { _id: roleId1 } = await config.api.roles.save({ name: role1, inherits: roles.BUILTIN_ROLE_IDS.BASIC, - permissionId: permissions.BuiltinPermissionID.WRITE, + permissionId: BuiltinPermissionID.WRITE, version: "name", }) const { _id: roleId2 } = await config.api.roles.save({ name: role2, inherits: roles.BUILTIN_ROLE_IDS.POWER, - permissionId: permissions.BuiltinPermissionID.POWER, + permissionId: BuiltinPermissionID.POWER, version: "name", }) await config.api.roles.save({ name: role3, inherits: [roleId1!, roleId2!], - permissionId: permissions.BuiltinPermissionID.READ_ONLY, + permissionId: BuiltinPermissionID.READ_ONLY, version: "name", }) const headers = await config.roleHeaders({ diff --git a/packages/server/src/api/routes/tests/screen.spec.ts b/packages/server/src/api/routes/tests/screen.spec.ts index 894710ca27..82a550f2fd 100644 --- a/packages/server/src/api/routes/tests/screen.spec.ts +++ b/packages/server/src/api/routes/tests/screen.spec.ts @@ -1,7 +1,7 @@ import { checkBuilderEndpoint } from "./utilities/TestFunctions" import * as setup from "./utilities" import { events, roles } from "@budibase/backend-core" -import { Screen, PermissionLevel, Role } from "@budibase/types" +import { Screen, Role, BuiltinPermissionID } from "@budibase/types" const { basicScreen } = setup.structures @@ -40,17 +40,17 @@ describe("/screens", () => { role1 = await config.api.roles.save({ name: "role1", inherits: roles.BUILTIN_ROLE_IDS.BASIC, - permissionId: PermissionLevel.WRITE, + permissionId: BuiltinPermissionID.WRITE, }) role2 = await config.api.roles.save({ name: "role2", inherits: roles.BUILTIN_ROLE_IDS.BASIC, - permissionId: PermissionLevel.WRITE, + permissionId: BuiltinPermissionID.WRITE, }) multiRole = await config.api.roles.save({ name: "multiRole", inherits: [role1._id!, role2._id!], - permissionId: PermissionLevel.WRITE, + permissionId: BuiltinPermissionID.WRITE, }) screen1 = await config.api.screen.save( { diff --git a/packages/server/src/api/routes/utils/validators.ts b/packages/server/src/api/routes/utils/validators.ts index 962735ded7..862d8c30c5 100644 --- a/packages/server/src/api/routes/utils/validators.ts +++ b/packages/server/src/api/routes/utils/validators.ts @@ -8,6 +8,7 @@ import { SearchFilters, Table, WebhookActionType, + BuiltinPermissionID, } from "@budibase/types" import Joi, { CustomValidator } from "joi" import { ValidSnippetNameRegex, helpers } from "@budibase/shared-core" @@ -214,7 +215,7 @@ export function roleValidator() { }).optional(), // this is the base permission ID (for now a built in) permissionId: Joi.string() - .valid(...Object.values(permissions.BuiltinPermissionID)) + .valid(...Object.values(BuiltinPermissionID)) .optional(), permissions: Joi.object() .pattern( diff --git a/packages/server/src/tests/utilities/structures.ts b/packages/server/src/tests/utilities/structures.ts index b63d6d1a78..49046d9eda 100644 --- a/packages/server/src/tests/utilities/structures.ts +++ b/packages/server/src/tests/utilities/structures.ts @@ -1,4 +1,4 @@ -import { permissions, roles, utils } from "@budibase/backend-core" +import { roles, utils } from "@budibase/backend-core" import { createHomeScreen } from "../../constants/screens" import { EMPTY_LAYOUT } from "../../constants/layouts" import { cloneDeep } from "lodash/fp" @@ -33,6 +33,7 @@ import { TableSourceType, Webhook, WebhookActionType, + BuiltinPermissionID, } from "@budibase/types" import { LoopInput } from "../../definitions/automations" import { merge } from "lodash" @@ -515,7 +516,7 @@ export function basicRole(): Role { return { name: `NewRole_${utils.newid()}`, inherits: roles.BUILTIN_ROLE_IDS.BASIC, - permissionId: permissions.BuiltinPermissionID.READ_ONLY, + permissionId: BuiltinPermissionID.WRITE, permissions: {}, version: "name", } diff --git a/packages/shared-core/src/helpers/tests/roles.spec.ts b/packages/shared-core/src/helpers/tests/roles.spec.ts index 051a196a5b..003b37f8f2 100644 --- a/packages/shared-core/src/helpers/tests/roles.spec.ts +++ b/packages/shared-core/src/helpers/tests/roles.spec.ts @@ -1,5 +1,5 @@ import { checkForRoleInheritanceLoops } from "../roles" -import { Role } from "@budibase/types" +import { BuiltinPermissionID, Role } from "@budibase/types" /** * This unit test exists as this utility will be used in the frontend and backend, confirmation @@ -19,7 +19,7 @@ function role(id: string, inherits: string | string[]): TestRole { _id: id, inherits: inherits, name: "ROLE", - permissionId: "PERMISSION", + permissionId: BuiltinPermissionID.WRITE, permissions: {}, // not needed for this test } allRoles.push(role) diff --git a/packages/types/src/api/web/role.ts b/packages/types/src/api/web/role.ts index aa5dd2f31b..f1c573c849 100644 --- a/packages/types/src/api/web/role.ts +++ b/packages/types/src/api/web/role.ts @@ -1,12 +1,12 @@ import { Role, RoleUIMetadata } from "../../documents" -import { PermissionLevel } from "../../sdk" +import { PermissionLevel, BuiltinPermissionID } from "../../sdk" export interface SaveRoleRequest { _id?: string _rev?: string name: string inherits?: string | string[] - permissionId?: string + permissionId?: BuiltinPermissionID permissions?: Record version?: string uiMetadata?: RoleUIMetadata diff --git a/packages/types/src/documents/app/role.ts b/packages/types/src/documents/app/role.ts index 22f4ab9cd3..b624a5c617 100644 --- a/packages/types/src/documents/app/role.ts +++ b/packages/types/src/documents/app/role.ts @@ -1,5 +1,5 @@ import { Document } from "../document" -import { PermissionLevel } from "../../sdk" +import { PermissionLevel, BuiltinPermissionID } from "../../sdk" export interface RoleUIMetadata { displayName?: string @@ -8,7 +8,7 @@ export interface RoleUIMetadata { } export interface Role extends Document { - permissionId: string + permissionId: BuiltinPermissionID inherits?: string | string[] permissions: Record version?: string diff --git a/packages/types/src/sdk/permissions.ts b/packages/types/src/sdk/permissions.ts index 638de97f48..d27876156e 100644 --- a/packages/types/src/sdk/permissions.ts +++ b/packages/types/src/sdk/permissions.ts @@ -5,6 +5,15 @@ export enum PermissionLevel { ADMIN = "admin", } +// used within the role, specifies base permissions +export enum BuiltinPermissionID { + PUBLIC = "public", + READ_ONLY = "read_only", + WRITE = "write", + ADMIN = "admin", + POWER = "power", +} + // these are the global types, that govern the underlying default behaviour export enum PermissionType { APP = "app", diff --git a/packages/worker/src/api/routes/global/tests/roles.spec.ts b/packages/worker/src/api/routes/global/tests/roles.spec.ts index 28977d8521..4e7acb2741 100644 --- a/packages/worker/src/api/routes/global/tests/roles.spec.ts +++ b/packages/worker/src/api/routes/global/tests/roles.spec.ts @@ -1,6 +1,6 @@ import { structures, TestConfiguration } from "../../../../tests" -import { context, db, permissions, roles } from "@budibase/backend-core" -import { App, Database } from "@budibase/types" +import { context, db, roles } from "@budibase/backend-core" +import { App, Database, BuiltinPermissionID } from "@budibase/types" jest.mock("@budibase/backend-core", () => { const core = jest.requireActual("@budibase/backend-core") @@ -44,7 +44,7 @@ describe("/api/global/roles", () => { const role = new roles.Role( db.generateRoleID(ROLE_NAME), ROLE_NAME, - permissions.BuiltinPermissionID.READ_ONLY, + BuiltinPermissionID.READ_ONLY, { displayName: roles.BUILTIN_ROLE_IDS.BASIC } ) From ca0dc030ade5f1c941a66437e2f228bdda73d546 Mon Sep 17 00:00:00 2001 From: Sam Rose Date: Wed, 23 Oct 2024 15:08:59 +0100 Subject: [PATCH 03/14] Make sure processSearchFilters handles an undefined input. --- packages/shared-core/src/utils.ts | 11 +++++++++-- 1 file changed, 9 insertions(+), 2 deletions(-) diff --git a/packages/shared-core/src/utils.ts b/packages/shared-core/src/utils.ts index 5db4ead1dc..8e22c10c15 100644 --- a/packages/shared-core/src/utils.ts +++ b/packages/shared-core/src/utils.ts @@ -138,9 +138,16 @@ export function isSupportedUserSearch(query: SearchFilters) { return true } -export const processSearchFilters = ( +export function processSearchFilters(filterArray: undefined): undefined +export function processSearchFilters( filterArray: LegacyFilter[] -): Required => { +): Required +export function processSearchFilters( + filterArray?: LegacyFilter[] +): Required | undefined { + if (!filterArray) { + return undefined + } const { allOr, onEmptyFilter, filters } = splitFiltersArray(filterArray) return { logicalOperator: UILogicalOperator.ALL, From 3ddd8e10cc4e026710b5f1d6ab2b1272d46f9d79 Mon Sep 17 00:00:00 2001 From: mike12345567 Date: Wed, 23 Oct 2024 15:10:53 +0100 Subject: [PATCH 04/14] Comment and test case. --- packages/server/src/api/routes/tests/role.spec.ts | 2 +- packages/types/src/sdk/permissions.ts | 2 ++ 2 files changed, 3 insertions(+), 1 deletion(-) diff --git a/packages/server/src/api/routes/tests/role.spec.ts b/packages/server/src/api/routes/tests/role.spec.ts index 1eb59c5a0c..a4e672e961 100644 --- a/packages/server/src/api/routes/tests/role.spec.ts +++ b/packages/server/src/api/routes/tests/role.spec.ts @@ -200,7 +200,7 @@ describe("/roles", () => { ...res, permissionId: undefined, }) - expect(updatedRes.permissionId).toEqual(PermissionLevel.READ) + expect(updatedRes.permissionId).toEqual(BuiltinPermissionID.READ_ONLY) }) }) diff --git a/packages/types/src/sdk/permissions.ts b/packages/types/src/sdk/permissions.ts index d27876156e..bb36af4170 100644 --- a/packages/types/src/sdk/permissions.ts +++ b/packages/types/src/sdk/permissions.ts @@ -1,3 +1,5 @@ +// used in resource permissions - permissions can be at one of these levels +// endpoints will set what type of permission they require (e.g. searching requires READ) export enum PermissionLevel { READ = "read", WRITE = "write", From 9ebd890c04902dcb2d810c307e46cef004ef8980 Mon Sep 17 00:00:00 2001 From: Sam Rose Date: Wed, 23 Oct 2024 15:13:58 +0100 Subject: [PATCH 05/14] Handle empty arrays too. --- packages/shared-core/src/utils.ts | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/packages/shared-core/src/utils.ts b/packages/shared-core/src/utils.ts index 8e22c10c15..01afbc2539 100644 --- a/packages/shared-core/src/utils.ts +++ b/packages/shared-core/src/utils.ts @@ -135,17 +135,19 @@ export function isSupportedUserSearch(query: SearchFilters) { return false } } + return true } export function processSearchFilters(filterArray: undefined): undefined +export function processSearchFilters(filterArray: []): undefined export function processSearchFilters( filterArray: LegacyFilter[] ): Required export function processSearchFilters( filterArray?: LegacyFilter[] ): Required | undefined { - if (!filterArray) { + if (!filterArray || filterArray.length === 0) { return undefined } const { allOr, onEmptyFilter, filters } = splitFiltersArray(filterArray) From e28357c45120c01025da3fb2ab0b381f76ad4a1c Mon Sep 17 00:00:00 2001 From: mike12345567 Date: Wed, 23 Oct 2024 15:21:37 +0100 Subject: [PATCH 06/14] Linting. --- packages/server/src/api/controllers/role.ts | 1 - 1 file changed, 1 deletion(-) diff --git a/packages/server/src/api/controllers/role.ts b/packages/server/src/api/controllers/role.ts index f8af71fe5c..7f5ac6b92f 100644 --- a/packages/server/src/api/controllers/role.ts +++ b/packages/server/src/api/controllers/role.ts @@ -18,7 +18,6 @@ import { UserCtx, UserMetadata, DocumentType, - PermissionLevel, BuiltinPermissionID, } from "@budibase/types" import { RoleColor, sdk as sharedSdk, helpers } from "@budibase/shared-core" From f2a844c3b5606e39257f64f86ce37e0e3ed20626 Mon Sep 17 00:00:00 2001 From: Sam Rose Date: Wed, 23 Oct 2024 15:24:32 +0100 Subject: [PATCH 07/14] Fix test. --- 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 e0703321e8..e6cab22911 100644 --- a/packages/shared-core/src/filters.ts +++ b/packages/shared-core/src/filters.ts @@ -467,7 +467,7 @@ export function buildQuery( } if (Array.isArray(filter)) { - filter = processSearchFilters(filter) + filter = processSearchFilters(filter) ?? [] } const operator = logicalOperatorFromUI( From a24728394f7b5e4ce1a2a514c0403eb2b4790df5 Mon Sep 17 00:00:00 2001 From: Adria Navarro Date: Wed, 23 Oct 2024 16:26:14 +0200 Subject: [PATCH 08/14] Trigger info data in the frontend --- .../AutomationBuilder/FlowChart/FlowItem.svelte | 16 +++++++++++----- 1 file changed, 11 insertions(+), 5 deletions(-) diff --git a/packages/builder/src/components/automation/AutomationBuilder/FlowChart/FlowItem.svelte b/packages/builder/src/components/automation/AutomationBuilder/FlowChart/FlowItem.svelte index aca3e950c3..873e769e21 100644 --- a/packages/builder/src/components/automation/AutomationBuilder/FlowChart/FlowItem.svelte +++ b/packages/builder/src/components/automation/AutomationBuilder/FlowChart/FlowItem.svelte @@ -3,7 +3,7 @@ automationStore, selectedAutomation, permissions, - selectedAutomationDisplayData, + tables, } from "stores/builder" import { Icon, @@ -17,6 +17,7 @@ AbsTooltip, InlineAlert, } from "@budibase/bbui" + import { sdk } from "@budibase/shared-core" import AutomationBlockSetup from "../../SetupPanel/AutomationBlockSetup.svelte" import CreateWebhookModal from "components/automation/Shared/CreateWebhookModal.svelte" import ActionModal from "./ActionModal.svelte" @@ -51,7 +52,12 @@ $: isAppAction && setPermissions(role) $: isAppAction && getPermissions(automationId) - $: triggerInfo = $selectedAutomationDisplayData?.triggerInfo + $: triggerInfo = sdk.automations.isRowAction($selectedAutomation) && { + title: "Automation trigger", + tableName: $tables.list.find( + x => x._id === $selectedAutomation.definition.trigger.inputs?.tableId + )?.name, + } async function setPermissions(role) { if (!role || !automationId) { @@ -187,10 +193,10 @@ {block} {webhookModal} /> - {#if isTrigger && triggerInfo} + {#if triggerInfo} {/if} {#if lastStep} From 8ebb02eb7976b0a88a847d28509b32a782888473 Mon Sep 17 00:00:00 2001 From: mike12345567 Date: Wed, 23 Oct 2024 15:30:46 +0100 Subject: [PATCH 09/14] Test fix. --- packages/server/src/api/routes/tests/role.spec.ts | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/packages/server/src/api/routes/tests/role.spec.ts b/packages/server/src/api/routes/tests/role.spec.ts index a4e672e961..ee5ba7840d 100644 --- a/packages/server/src/api/routes/tests/role.spec.ts +++ b/packages/server/src/api/routes/tests/role.spec.ts @@ -232,9 +232,7 @@ describe("/roles", () => { const customRoleFetched = res.find(r => r._id === customRole.name) expect(customRoleFetched).toBeDefined() expect(customRoleFetched!.inherits).toEqual(BUILTIN_ROLE_IDS.BASIC) - expect(customRoleFetched!.permissionId).toEqual( - BuiltinPermissionID.READ_ONLY - ) + expect(customRoleFetched!.permissionId).toEqual(BuiltinPermissionID.WRITE) }) it("should be able to get the role with a permission added", async () => { From f7b84ca7eab2d9be82ff84b9e620fc8318b96aec Mon Sep 17 00:00:00 2001 From: Sam Rose Date: Wed, 23 Oct 2024 15:32:07 +0100 Subject: [PATCH 10/14] Fix some type skullduggery. --- packages/shared-core/src/filters.ts | 13 ++++++------- packages/shared-core/src/utils.ts | 5 ----- 2 files changed, 6 insertions(+), 12 deletions(-) diff --git a/packages/shared-core/src/filters.ts b/packages/shared-core/src/filters.ts index e6cab22911..003e6b5082 100644 --- a/packages/shared-core/src/filters.ts +++ b/packages/shared-core/src/filters.ts @@ -455,19 +455,18 @@ export function splitFiltersArray(filters: LegacyFilter[]) { * Legacy support remains for the old **SearchFilter[]** format. * These will be migrated to an appropriate **SearchFilters** object, if encountered */ -export function buildQuery(filter: undefined): undefined -export function buildQuery( - filter: UISearchFilter | LegacyFilter[] -): SearchFilters export function buildQuery( filter?: UISearchFilter | LegacyFilter[] -): SearchFilters | undefined { +): SearchFilters { if (!filter) { - return + return {} } if (Array.isArray(filter)) { - filter = processSearchFilters(filter) ?? [] + filter = processSearchFilters(filter) + if (!filter) { + return {} + } } const operator = logicalOperatorFromUI( diff --git a/packages/shared-core/src/utils.ts b/packages/shared-core/src/utils.ts index 01afbc2539..0e49db9c7c 100644 --- a/packages/shared-core/src/utils.ts +++ b/packages/shared-core/src/utils.ts @@ -139,11 +139,6 @@ export function isSupportedUserSearch(query: SearchFilters) { return true } -export function processSearchFilters(filterArray: undefined): undefined -export function processSearchFilters(filterArray: []): undefined -export function processSearchFilters( - filterArray: LegacyFilter[] -): Required export function processSearchFilters( filterArray?: LegacyFilter[] ): Required | undefined { From 7901c2d4ac9cad004b812d57652b35d44ea4ae81 Mon Sep 17 00:00:00 2001 From: Adria Navarro Date: Wed, 23 Oct 2024 16:37:46 +0200 Subject: [PATCH 11/14] Remove builderData usage --- .../AutomationPanel/AutomationNavItem.svelte | 2 +- .../AutomationPanel/AutomationPanel.svelte | 10 ++-------- packages/builder/src/stores/builder/automations.js | 14 -------------- packages/builder/src/stores/builder/index.js | 2 -- 4 files changed, 3 insertions(+), 25 deletions(-) diff --git a/packages/builder/src/components/automation/AutomationPanel/AutomationNavItem.svelte b/packages/builder/src/components/automation/AutomationPanel/AutomationNavItem.svelte index 50814897b4..4e9ca5fd53 100644 --- a/packages/builder/src/components/automation/AutomationPanel/AutomationNavItem.svelte +++ b/packages/builder/src/components/automation/AutomationPanel/AutomationNavItem.svelte @@ -112,7 +112,7 @@ iconColor={automation.disabled ? "var(--spectrum-global-color-gray-600)" : "var(--spectrum-global-color-gray-900)"} - text={automation.displayName} + text={automation.name} selected={automation._id === $selectedAutomation?._id} hovering={automation._id === $contextMenuStore.id} on:click={() => automationStore.actions.select(automation._id)} diff --git a/packages/builder/src/components/automation/AutomationPanel/AutomationPanel.svelte b/packages/builder/src/components/automation/AutomationPanel/AutomationPanel.svelte index 6b96c4ebf5..a26efdf243 100644 --- a/packages/builder/src/components/automation/AutomationPanel/AutomationPanel.svelte +++ b/packages/builder/src/components/automation/AutomationPanel/AutomationPanel.svelte @@ -25,15 +25,9 @@ automation.name.toLowerCase().includes(searchString.toLowerCase()) ) }) - .map(automation => ({ - ...automation, - displayName: - $automationStore.automationDisplayData[automation._id]?.displayName || - automation.name, - })) .sort((a, b) => { - const lowerA = a.displayName.toLowerCase() - const lowerB = b.displayName.toLowerCase() + const lowerA = a.name.toLowerCase() + const lowerB = b.name.toLowerCase() return lowerA > lowerB ? 1 : -1 }) diff --git a/packages/builder/src/stores/builder/automations.js b/packages/builder/src/stores/builder/automations.js index b8765ad1e7..bb0f5d2085 100644 --- a/packages/builder/src/stores/builder/automations.js +++ b/packages/builder/src/stores/builder/automations.js @@ -23,7 +23,6 @@ const initialAutomationState = { ACTION: {}, }, selectedAutomationId: null, - automationDisplayData: {}, } // If this functions, remove the actions elements @@ -91,7 +90,6 @@ const automationActions = store => ({ state.automations.sort((a, b) => { return a.name < b.name ? -1 : 1 }) - state.automationDisplayData = automationResponse.builderData state.blockDefinitions = getFinalDefinitions( definitions.trigger, definitions.action @@ -153,8 +151,6 @@ const automationActions = store => ({ state.selectedAutomationId = state.automations[0]?._id || null } - // Clear out automationDisplayData for the automation - delete state.automationDisplayData[automation._id] return state }) }, @@ -432,13 +428,3 @@ export const selectedAutomation = derived(automationStore, $automationStore => { x => x._id === $automationStore.selectedAutomationId ) }) - -export const selectedAutomationDisplayData = derived( - [automationStore, selectedAutomation], - ([$automationStore, $selectedAutomation]) => { - if (!$selectedAutomation?._id) { - return null - } - return $automationStore.automationDisplayData[$selectedAutomation._id] - } -) diff --git a/packages/builder/src/stores/builder/index.js b/packages/builder/src/stores/builder/index.js index dbde739951..158cd29973 100644 --- a/packages/builder/src/stores/builder/index.js +++ b/packages/builder/src/stores/builder/index.js @@ -11,7 +11,6 @@ import { automationStore, selectedAutomation, automationHistoryStore, - selectedAutomationDisplayData, } from "./automations.js" import { userStore, userSelectedResourceMap, isOnlyUser } from "./users.js" import { deploymentStore } from "./deployments.js" @@ -46,7 +45,6 @@ export { previewStore, automationStore, selectedAutomation, - selectedAutomationDisplayData, automationHistoryStore, sortedScreens, userStore, From 11b2e40836a77a8d2f76a3945a35984c37734a4b Mon Sep 17 00:00:00 2001 From: Adria Navarro Date: Wed, 23 Oct 2024 16:37:55 +0200 Subject: [PATCH 12/14] Remove --- .../server/src/api/controllers/automation.ts | 8 -------- packages/types/src/api/web/automation.ts | 16 ---------------- 2 files changed, 24 deletions(-) diff --git a/packages/server/src/api/controllers/automation.ts b/packages/server/src/api/controllers/automation.ts index b19218647b..d8bc9d6b21 100644 --- a/packages/server/src/api/controllers/automation.ts +++ b/packages/server/src/api/controllers/automation.ts @@ -76,16 +76,8 @@ export async function update(ctx: UserCtx) { } export async function fetch(ctx: UserCtx) { - const query: { enrich?: string } = ctx.request.query || {} - const enrich = query.enrich === "true" - const automations = await sdk.automations.fetch() ctx.body = { automations } - if (enrich) { - ctx.body.builderData = await sdk.automations.utils.getBuilderData( - automations - ) - } } export async function find(ctx: UserCtx) { diff --git a/packages/types/src/api/web/automation.ts b/packages/types/src/api/web/automation.ts index 6c810d5e78..06080fc667 100644 --- a/packages/types/src/api/web/automation.ts +++ b/packages/types/src/api/web/automation.ts @@ -3,22 +3,6 @@ import { Automation } from "../../documents" export interface DeleteAutomationResponse extends DocumentDestroyResponse {} -export interface AutomationBuilderData { - displayName: string - triggerInfo?: { - type: string - table: { - id: string - name: string - } - rowAction: { - id: string - name: string - } - } -} - export interface FetchAutomationResponse { automations: Automation[] - builderData?: Record // The key will be the automationId } From e67352d51bf0f65600964393c6625d3e6eeaee25 Mon Sep 17 00:00:00 2001 From: Adria Navarro Date: Wed, 23 Oct 2024 16:38:07 +0200 Subject: [PATCH 13/14] Remove api flag --- packages/builder/src/stores/builder/automations.js | 2 +- packages/frontend-core/src/api/automations.js | 9 ++------- 2 files changed, 3 insertions(+), 8 deletions(-) diff --git a/packages/builder/src/stores/builder/automations.js b/packages/builder/src/stores/builder/automations.js index bb0f5d2085..3ab81ccce6 100644 --- a/packages/builder/src/stores/builder/automations.js +++ b/packages/builder/src/stores/builder/automations.js @@ -82,7 +82,7 @@ const automationActions = store => ({ }, fetch: async () => { const [automationResponse, definitions] = await Promise.all([ - API.getAutomations({ enrich: true }), + API.getAutomations(), API.getAutomationDefinitions(), ]) store.update(state => { diff --git a/packages/frontend-core/src/api/automations.js b/packages/frontend-core/src/api/automations.js index ce5f7d3aba..37a834cf04 100644 --- a/packages/frontend-core/src/api/automations.js +++ b/packages/frontend-core/src/api/automations.js @@ -26,14 +26,9 @@ export const buildAutomationEndpoints = API => ({ /** * Gets a list of all automations. */ - getAutomations: async ({ enrich }) => { - const params = new URLSearchParams() - if (enrich) { - params.set("enrich", true) - } - + getAutomations: async () => { return await API.get({ - url: `/api/automations?${params.toString()}`, + url: "/api/automations", }) }, From 998d0002b8edf8abdcfdad4a9eeaca06665ec871 Mon Sep 17 00:00:00 2001 From: Adria Navarro Date: Wed, 23 Oct 2024 16:48:22 +0200 Subject: [PATCH 14/14] Cleanup dead code --- .../server/src/sdk/app/automations/utils.ts | 51 +------------------ 1 file changed, 1 insertion(+), 50 deletions(-) diff --git a/packages/server/src/sdk/app/automations/utils.ts b/packages/server/src/sdk/app/automations/utils.ts index f5ae8bf948..79e70923f1 100644 --- a/packages/server/src/sdk/app/automations/utils.ts +++ b/packages/server/src/sdk/app/automations/utils.ts @@ -1,56 +1,7 @@ -import { - Automation, - AutomationActionStepId, - AutomationBuilderData, -} from "@budibase/types" -import { sdk as coreSdk } from "@budibase/shared-core" -import sdk from "../../../sdk" +import { Automation, AutomationActionStepId } from "@budibase/types" export function checkForCollectStep(automation: Automation) { return automation.definition.steps.some( (step: any) => step.stepId === AutomationActionStepId.COLLECT ) } - -export async function getBuilderData( - automations: Automation[] -): Promise> { - const tableNameCache: Record = {} - async function getTableName(tableId: string) { - if (!tableNameCache[tableId]) { - const table = await sdk.tables.getTable(tableId) - tableNameCache[tableId] = table.name - } - - return tableNameCache[tableId] - } - - const result: Record = {} - for (const automation of automations) { - const isRowAction = coreSdk.automations.isRowAction(automation) - if (!isRowAction) { - result[automation._id!] = { displayName: automation.name } - continue - } - - const { tableId, rowActionId } = automation.definition.trigger.inputs - if (!tableId || !rowActionId) { - continue - } - - const tableName = await getTableName(tableId) - const rowActionName = automation.name - result[automation._id!] = { - displayName: rowActionName, - triggerInfo: { - type: "Automation trigger", - table: { id: tableId, name: tableName }, - rowAction: { - id: rowActionId, - name: rowActionName, - }, - }, - } - } - return result -}