From aca310e72124f55a2cf623215b2f861b1bff5022 Mon Sep 17 00:00:00 2001 From: Adria Navarro Date: Tue, 3 Sep 2024 17:21:13 +0200 Subject: [PATCH 01/20] Tidy code --- .../middleware/triggerRowActionAuthorised.ts | 20 +++++++++++++------ 1 file changed, 14 insertions(+), 6 deletions(-) diff --git a/packages/server/src/middleware/triggerRowActionAuthorised.ts b/packages/server/src/middleware/triggerRowActionAuthorised.ts index be5c6a97e1..6a3d1e3a5d 100644 --- a/packages/server/src/middleware/triggerRowActionAuthorised.ts +++ b/packages/server/src/middleware/triggerRowActionAuthorised.ts @@ -1,5 +1,5 @@ import { Next } from "koa" -import { Ctx } from "@budibase/types" +import { UserCtx } from "@budibase/types" import { paramSubResource } from "./resourceId" import { docIds } from "@budibase/backend-core" import * as utils from "../db/utils" @@ -9,9 +9,11 @@ export function triggerRowActionAuthorised( sourcePath: string, actionPath: string ) { - return async (ctx: Ctx, next: Next) => { + function extractResourceIds(ctx: UserCtx) { + ctx = { ...ctx } // Reusing the existing middleware to extract the value paramSubResource(sourcePath, actionPath)(ctx, () => {}) + const { resourceId: sourceId, subResourceId: rowActionId } = ctx const isTableId = docIds.isTableId(sourceId) @@ -23,18 +25,24 @@ export function triggerRowActionAuthorised( const tableId = isTableId ? sourceId : utils.extractViewInfoFromID(sourceId).tableId + const viewId = isTableId ? undefined : sourceId + return { tableId, viewId, rowActionId } + } + + return async (ctx: UserCtx, next: Next) => { + const { tableId, viewId, rowActionId } = extractResourceIds(ctx) const rowAction = await sdk.rowActions.get(tableId, rowActionId) - if (isTableId && !rowAction.permissions.table.runAllowed) { + if (!viewId && !rowAction.permissions.table.runAllowed) { ctx.throw( 403, - `Row action '${rowActionId}' is not enabled for table '${sourceId}'` + `Row action '${rowActionId}' is not enabled for table '${tableId}'` ) - } else if (isViewId && !rowAction.permissions.views[sourceId]?.runAllowed) { + } else if (viewId && !rowAction.permissions.views[viewId]?.runAllowed) { ctx.throw( 403, - `Row action '${rowActionId}' is not enabled for view '${sourceId}'` + `Row action '${rowActionId}' is not enabled for view '${viewId}'` ) } From 623b385d8a60df20993eb02211663cbbbb368c30 Mon Sep 17 00:00:00 2001 From: Adria Navarro Date: Tue, 3 Sep 2024 17:53:25 +0200 Subject: [PATCH 02/20] Promisify middleware --- .../src/middleware/triggerRowActionAuthorised.ts | 14 +++++++++++--- 1 file changed, 11 insertions(+), 3 deletions(-) diff --git a/packages/server/src/middleware/triggerRowActionAuthorised.ts b/packages/server/src/middleware/triggerRowActionAuthorised.ts index 6a3d1e3a5d..2f94e46589 100644 --- a/packages/server/src/middleware/triggerRowActionAuthorised.ts +++ b/packages/server/src/middleware/triggerRowActionAuthorised.ts @@ -5,16 +5,24 @@ import { docIds } from "@budibase/backend-core" import * as utils from "../db/utils" import sdk from "../sdk" +async function executeMiddleware( + ctx: UserCtx, + delegate: (ctx: UserCtx, next: any) => any +) { + await new Promise(r => delegate(ctx, () => r())) +} + export function triggerRowActionAuthorised( sourcePath: string, actionPath: string ) { - function extractResourceIds(ctx: UserCtx) { + async function extractResourceIds(ctx: UserCtx) { ctx = { ...ctx } // Reusing the existing middleware to extract the value - paramSubResource(sourcePath, actionPath)(ctx, () => {}) + await executeMiddleware(ctx, paramSubResource(sourcePath, actionPath)) - const { resourceId: sourceId, subResourceId: rowActionId } = ctx + const sourceId: string = ctx.resourceId + const rowActionId: String = ctx.subResourceId const isTableId = docIds.isTableId(sourceId) const isViewId = utils.isViewID(sourceId) From 00119f9d731f8e0c0f2784af4d468f31d03c25ef Mon Sep 17 00:00:00 2001 From: Adria Navarro Date: Wed, 4 Sep 2024 10:16:59 +0200 Subject: [PATCH 03/20] Guard permission --- .../src/api/routes/tests/rowAction.spec.ts | 32 ++++++++ packages/server/src/middleware/authorized.ts | 2 +- .../middleware/triggerRowActionAuthorised.ts | 74 ++++++++++++------- 3 files changed, 81 insertions(+), 27 deletions(-) diff --git a/packages/server/src/api/routes/tests/rowAction.spec.ts b/packages/server/src/api/routes/tests/rowAction.spec.ts index 40ba2c1811..003488c008 100644 --- a/packages/server/src/api/routes/tests/rowAction.spec.ts +++ b/packages/server/src/api/routes/tests/rowAction.spec.ts @@ -742,5 +742,37 @@ describe("/rowsActions", () => { }), ]) }) + + const { PUBLIC, ...nonPublicRoles } = roles.BUILTIN_ROLE_IDS + + it.each(Object.values(nonPublicRoles))( + "rejects if the user does not have table read permission", + async role => { + await config.api.permission.add({ + level: PermissionLevel.READ, + resourceId: tableId, + roleId: role, + }) + + const normalUser = await config.createUser({ + admin: { global: false }, + builder: {}, + }) + await config.withUser(normalUser, async () => { + await config.publish() + await config.api.rowAction.trigger( + tableId, + rowAction.id, + { + rowId: row._id!, + }, + { status: 403, body: { message: "User does not have permission" } } + ) + + const automationLogs = await getAutomationLogs() + expect(automationLogs).toBeEmpty() + }) + } + ) }) }) diff --git a/packages/server/src/middleware/authorized.ts b/packages/server/src/middleware/authorized.ts index b23a9846b7..3bead7f80d 100644 --- a/packages/server/src/middleware/authorized.ts +++ b/packages/server/src/middleware/authorized.ts @@ -88,7 +88,7 @@ const authorized = opts = { schema: false }, resourcePath?: string ) => - async (ctx: any, next: any) => { + async (ctx: UserCtx, next: any) => { // webhooks don't need authentication, each webhook unique // also internal requests (between services) don't need authorized if (isWebhookEndpoint(ctx) || ctx.internal) { diff --git a/packages/server/src/middleware/triggerRowActionAuthorised.ts b/packages/server/src/middleware/triggerRowActionAuthorised.ts index 2f94e46589..4cee167350 100644 --- a/packages/server/src/middleware/triggerRowActionAuthorised.ts +++ b/packages/server/src/middleware/triggerRowActionAuthorised.ts @@ -1,44 +1,64 @@ import { Next } from "koa" -import { UserCtx } from "@budibase/types" +import { PermissionLevel, PermissionType, UserCtx } from "@budibase/types" import { paramSubResource } from "./resourceId" import { docIds } from "@budibase/backend-core" import * as utils from "../db/utils" import sdk from "../sdk" - -async function executeMiddleware( - ctx: UserCtx, - delegate: (ctx: UserCtx, next: any) => any -) { - await new Promise(r => delegate(ctx, () => r())) -} +import { authorizedResource } from "./authorized" export function triggerRowActionAuthorised( sourcePath: string, actionPath: string ) { - async function extractResourceIds(ctx: UserCtx) { - ctx = { ...ctx } - // Reusing the existing middleware to extract the value - await executeMiddleware(ctx, paramSubResource(sourcePath, actionPath)) + return async (ctx: UserCtx, next: Next) => { + async function getResourceIds() { + // Reusing the existing middleware to extract the value + await paramSubResource(sourcePath, actionPath)(ctx, () => {}) - const sourceId: string = ctx.resourceId - const rowActionId: String = ctx.subResourceId + const sourceId: string = ctx.resourceId + const rowActionId: string = ctx.subResourceId - const isTableId = docIds.isTableId(sourceId) - const isViewId = utils.isViewID(sourceId) - if (!isTableId && !isViewId) { - ctx.throw(400, `'${sourceId}' is not a valid source id`) + const isTableId = docIds.isTableId(sourceId) + const isViewId = utils.isViewID(sourceId) + if (!isTableId && !isViewId) { + ctx.throw(400, `'${sourceId}' is not a valid source id`) + } + + const tableId = isTableId + ? sourceId + : utils.extractViewInfoFromID(sourceId).tableId + const viewId = isTableId ? undefined : sourceId + return { tableId, viewId, rowActionId } } - const tableId = isTableId - ? sourceId - : utils.extractViewInfoFromID(sourceId).tableId - const viewId = isTableId ? undefined : sourceId - return { tableId, viewId, rowActionId } - } + async function guardResourcePermissions( + ctx: UserCtx, + tableId: string, + viewId?: string + ) { + const { params } = ctx + try { + if (!viewId) { + ctx.params = { tableId } + await authorizedResource( + PermissionType.TABLE, + PermissionLevel.READ, + "tableId" + )(ctx, () => {}) + } else { + ctx.params = { viewId } + await authorizedResource( + PermissionType.VIEW, + PermissionLevel.READ, + "__viewId" + )(ctx, () => {}) + } + } finally { + ctx.params = params + } + } - return async (ctx: UserCtx, next: Next) => { - const { tableId, viewId, rowActionId } = extractResourceIds(ctx) + const { tableId, viewId, rowActionId } = await getResourceIds() const rowAction = await sdk.rowActions.get(tableId, rowActionId) @@ -54,6 +74,8 @@ export function triggerRowActionAuthorised( ) } + await guardResourcePermissions(ctx, tableId, viewId) + // Enrich tableId ctx.params.tableId = tableId return next() From 92a0740cef6bd46c36243f44a2dae104afc7f1a2 Mon Sep 17 00:00:00 2001 From: Adria Navarro Date: Wed, 4 Sep 2024 11:10:56 +0200 Subject: [PATCH 04/20] Proper guarding --- .../middleware/triggerRowActionAuthorised.ts | 46 ++++--------------- 1 file changed, 10 insertions(+), 36 deletions(-) diff --git a/packages/server/src/middleware/triggerRowActionAuthorised.ts b/packages/server/src/middleware/triggerRowActionAuthorised.ts index 4cee167350..17f22b7000 100644 --- a/packages/server/src/middleware/triggerRowActionAuthorised.ts +++ b/packages/server/src/middleware/triggerRowActionAuthorised.ts @@ -1,6 +1,5 @@ import { Next } from "koa" import { PermissionLevel, PermissionType, UserCtx } from "@budibase/types" -import { paramSubResource } from "./resourceId" import { docIds } from "@budibase/backend-core" import * as utils from "../db/utils" import sdk from "../sdk" @@ -12,11 +11,8 @@ export function triggerRowActionAuthorised( ) { return async (ctx: UserCtx, next: Next) => { async function getResourceIds() { - // Reusing the existing middleware to extract the value - await paramSubResource(sourcePath, actionPath)(ctx, () => {}) - - const sourceId: string = ctx.resourceId - const rowActionId: string = ctx.subResourceId + const sourceId: string = ctx.params[sourcePath] + const rowActionId: string = ctx.params[actionPath] const isTableId = docIds.isTableId(sourceId) const isViewId = utils.isViewID(sourceId) @@ -31,37 +27,17 @@ export function triggerRowActionAuthorised( return { tableId, viewId, rowActionId } } - async function guardResourcePermissions( - ctx: UserCtx, - tableId: string, - viewId?: string - ) { - const { params } = ctx - try { - if (!viewId) { - ctx.params = { tableId } - await authorizedResource( - PermissionType.TABLE, - PermissionLevel.READ, - "tableId" - )(ctx, () => {}) - } else { - ctx.params = { viewId } - await authorizedResource( - PermissionType.VIEW, - PermissionLevel.READ, - "__viewId" - )(ctx, () => {}) - } - } finally { - ctx.params = params - } - } - const { tableId, viewId, rowActionId } = await getResourceIds() - const rowAction = await sdk.rowActions.get(tableId, rowActionId) + // Check if the user has permissions to the table/view + await authorizedResource( + !viewId ? PermissionType.TABLE : PermissionType.VIEW, + PermissionLevel.READ, + sourcePath + )(ctx, () => {}) + // Check is the row action can run for the given view/table + const rowAction = await sdk.rowActions.get(tableId, rowActionId) if (!viewId && !rowAction.permissions.table.runAllowed) { ctx.throw( 403, @@ -74,8 +50,6 @@ export function triggerRowActionAuthorised( ) } - await guardResourcePermissions(ctx, tableId, viewId) - // Enrich tableId ctx.params.tableId = tableId return next() From 11e8d576e2a11553be011b7b4f357ac6aadd9a44 Mon Sep 17 00:00:00 2001 From: Adria Navarro Date: Wed, 4 Sep 2024 11:11:10 +0200 Subject: [PATCH 05/20] Extra tests --- .../src/api/routes/tests/rowAction.spec.ts | 115 +++++++++++++----- 1 file changed, 85 insertions(+), 30 deletions(-) diff --git a/packages/server/src/api/routes/tests/rowAction.spec.ts b/packages/server/src/api/routes/tests/rowAction.spec.ts index 003488c008..6dad230751 100644 --- a/packages/server/src/api/routes/tests/rowAction.spec.ts +++ b/packages/server/src/api/routes/tests/rowAction.spec.ts @@ -743,36 +743,91 @@ describe("/rowsActions", () => { ]) }) - const { PUBLIC, ...nonPublicRoles } = roles.BUILTIN_ROLE_IDS - - it.each(Object.values(nonPublicRoles))( - "rejects if the user does not have table read permission", - async role => { - await config.api.permission.add({ - level: PermissionLevel.READ, - resourceId: tableId, - roleId: role, - }) - - const normalUser = await config.createUser({ - admin: { global: false }, - builder: {}, - }) - await config.withUser(normalUser, async () => { - await config.publish() - await config.api.rowAction.trigger( - tableId, - rowAction.id, - { - rowId: row._id!, - }, - { status: 403, body: { message: "User does not have permission" } } - ) - - const automationLogs = await getAutomationLogs() - expect(automationLogs).toBeEmpty() - }) + describe("role permission checks", () => { + function createUser(role: string) { + return config.createUser({ + admin: { global: false }, + builder: {}, + roles: { [config.getProdAppId()]: role }, + }) } - ) + + function getRolesHigherThan(role: string) { + const result = Object.values(roles.BUILTIN_ROLE_IDS).filter( + r => r !== role && roles.lowerBuiltinRoleID(r, role) === role + ) + return result + } + function getRolesLowerThan(role: string) { + const result = Object.values(roles.BUILTIN_ROLE_IDS).filter( + r => r !== role && roles.lowerBuiltinRoleID(r, role) === r + ) + return result + } + + const allowedRoleConfig = Object.values(roles.BUILTIN_ROLE_IDS).flatMap( + r => [r, ...getRolesLowerThan(r)].map(p => [r, p]) + ) + + const disallowedRoleConfig = Object.values( + roles.BUILTIN_ROLE_IDS + ).flatMap(r => getRolesHigherThan(r).map(p => [r, p])) + + it.each(allowedRoleConfig)( + "allows triggering if the user has table read permission (user %s, table %s)", + async (userRole, resourcePermission) => { + await config.api.permission.add({ + level: PermissionLevel.READ, + resourceId: tableId, + roleId: resourcePermission, + }) + + const normalUser = await createUser(userRole) + + await config.withUser(normalUser, async () => { + await config.publish() + await config.api.rowAction.trigger( + tableId, + rowAction.id, + { + rowId: row._id!, + }, + { status: 200 } + ) + }) + } + ) + + it.each(disallowedRoleConfig)( + "rejects if the user does not have table read permission (user %s, table %s)", + async (userRole, resourcePermission) => { + await config.api.permission.add({ + level: PermissionLevel.READ, + resourceId: tableId, + roleId: resourcePermission, + }) + + const normalUser = await createUser(userRole) + + await config.withUser(normalUser, async () => { + await config.publish() + await config.api.rowAction.trigger( + tableId, + rowAction.id, + { + rowId: row._id!, + }, + { + status: 403, + body: { message: "User does not have permission" }, + } + ) + + const automationLogs = await getAutomationLogs() + expect(automationLogs).toBeEmpty() + }) + } + ) + }) }) }) From f4f503690d28ac374868bad31dc86a14cc429008 Mon Sep 17 00:00:00 2001 From: Adria Navarro Date: Wed, 4 Sep 2024 12:23:15 +0200 Subject: [PATCH 06/20] Dynamic tests --- .../src/api/routes/tests/rowAction.spec.ts | 83 +++++++++++++------ 1 file changed, 58 insertions(+), 25 deletions(-) diff --git a/packages/server/src/api/routes/tests/rowAction.spec.ts b/packages/server/src/api/routes/tests/rowAction.spec.ts index 6dad230751..b6901dd28f 100644 --- a/packages/server/src/api/routes/tests/rowAction.spec.ts +++ b/packages/server/src/api/routes/tests/rowAction.spec.ts @@ -9,7 +9,7 @@ import { RowActionResponse, } from "@budibase/types" import * as setup from "./utilities" -import { generator } from "@budibase/backend-core/tests" +import { generator, mocks } from "@budibase/backend-core/tests" import { Expectations } from "../../../tests/utilities/api/base" import { roles } from "@budibase/backend-core" import { automations } from "@budibase/pro" @@ -743,7 +743,34 @@ describe("/rowsActions", () => { ]) }) - describe("role permission checks", () => { + describe.each([ + ["table", async () => tableId], + [ + "view", + async () => { + const viewId = ( + await config.api.viewV2.create( + setup.structures.viewV2.createRequest(tableId) + ) + ).id + + await config.api.rowAction.setViewPermission( + tableId, + viewId, + rowAction.id + ) + return viewId + }, + ], + ])("role permission checks (for %s)", (_, getResourceId) => { + beforeAll(() => { + mocks.licenses.useViewPermissions() + }) + + afterAll(() => { + mocks.licenses.useCloudFree() + }) + function createUser(role: string) { return config.createUser({ admin: { global: false }, @@ -752,33 +779,38 @@ describe("/rowsActions", () => { }) } - function getRolesHigherThan(role: string) { - const result = Object.values(roles.BUILTIN_ROLE_IDS).filter( - r => r !== role && roles.lowerBuiltinRoleID(r, role) === role + const allowedRoleConfig = (() => { + function getRolesLowerThan(role: string) { + const result = Object.values(roles.BUILTIN_ROLE_IDS).filter( + r => r !== role && roles.lowerBuiltinRoleID(r, role) === r + ) + return result + } + return Object.values(roles.BUILTIN_ROLE_IDS).flatMap(r => + [r, ...getRolesLowerThan(r)].map(p => [r, p]) ) - return result - } - function getRolesLowerThan(role: string) { - const result = Object.values(roles.BUILTIN_ROLE_IDS).filter( - r => r !== role && roles.lowerBuiltinRoleID(r, role) === r + })() + + const disallowedRoleConfig = (() => { + function getRolesHigherThan(role: string) { + const result = Object.values(roles.BUILTIN_ROLE_IDS).filter( + r => r !== role && roles.lowerBuiltinRoleID(r, role) === role + ) + return result + } + return Object.values(roles.BUILTIN_ROLE_IDS).flatMap(r => + getRolesHigherThan(r).map(p => [r, p]) ) - return result - } - - const allowedRoleConfig = Object.values(roles.BUILTIN_ROLE_IDS).flatMap( - r => [r, ...getRolesLowerThan(r)].map(p => [r, p]) - ) - - const disallowedRoleConfig = Object.values( - roles.BUILTIN_ROLE_IDS - ).flatMap(r => getRolesHigherThan(r).map(p => [r, p])) + })() it.each(allowedRoleConfig)( - "allows triggering if the user has table read permission (user %s, table %s)", + "allows triggering if the user has read permission (user %s, table %s)", async (userRole, resourcePermission) => { + const resourceId = await getResourceId() + await config.api.permission.add({ level: PermissionLevel.READ, - resourceId: tableId, + resourceId, roleId: resourcePermission, }) @@ -787,7 +819,7 @@ describe("/rowsActions", () => { await config.withUser(normalUser, async () => { await config.publish() await config.api.rowAction.trigger( - tableId, + resourceId, rowAction.id, { rowId: row._id!, @@ -801,9 +833,10 @@ describe("/rowsActions", () => { it.each(disallowedRoleConfig)( "rejects if the user does not have table read permission (user %s, table %s)", async (userRole, resourcePermission) => { + const resourceId = await getResourceId() await config.api.permission.add({ level: PermissionLevel.READ, - resourceId: tableId, + resourceId, roleId: resourcePermission, }) @@ -812,7 +845,7 @@ describe("/rowsActions", () => { await config.withUser(normalUser, async () => { await config.publish() await config.api.rowAction.trigger( - tableId, + resourceId, rowAction.id, { rowId: row._id!, From 19963f496f9034c4674f6231336daf71947e5216 Mon Sep 17 00:00:00 2001 From: Adria Navarro Date: Wed, 4 Sep 2024 12:26:12 +0200 Subject: [PATCH 07/20] Add extra tests --- .../src/api/routes/tests/rowAction.spec.ts | 40 ++++++++++++++----- 1 file changed, 30 insertions(+), 10 deletions(-) diff --git a/packages/server/src/api/routes/tests/rowAction.spec.ts b/packages/server/src/api/routes/tests/rowAction.spec.ts index b6901dd28f..e0fbcb2847 100644 --- a/packages/server/src/api/routes/tests/rowAction.spec.ts +++ b/packages/server/src/api/routes/tests/rowAction.spec.ts @@ -744,9 +744,12 @@ describe("/rowsActions", () => { }) describe.each([ - ["table", async () => tableId], [ - "view", + "table", + async () => ({ permissionResource: tableId, triggerResouce: tableId }), + ], + [ + "view (with implicit views)", async () => { const viewId = ( await config.api.viewV2.create( @@ -759,10 +762,27 @@ describe("/rowsActions", () => { viewId, rowAction.id ) - return viewId + return { permissionResource: viewId, triggerResouce: viewId } }, ], - ])("role permission checks (for %s)", (_, getResourceId) => { + [ + "view (without implicit views)", + async () => { + const viewId = ( + await config.api.viewV2.create( + setup.structures.viewV2.createRequest(tableId) + ) + ).id + + await config.api.rowAction.setViewPermission( + tableId, + viewId, + rowAction.id + ) + return { permissionResource: tableId, triggerResouce: viewId } + }, + ], + ])("role permission checks (for %s)", (_, getResources) => { beforeAll(() => { mocks.licenses.useViewPermissions() }) @@ -806,11 +826,11 @@ describe("/rowsActions", () => { it.each(allowedRoleConfig)( "allows triggering if the user has read permission (user %s, table %s)", async (userRole, resourcePermission) => { - const resourceId = await getResourceId() + const { permissionResource, triggerResouce } = await getResources() await config.api.permission.add({ level: PermissionLevel.READ, - resourceId, + resourceId: permissionResource, roleId: resourcePermission, }) @@ -819,7 +839,7 @@ describe("/rowsActions", () => { await config.withUser(normalUser, async () => { await config.publish() await config.api.rowAction.trigger( - resourceId, + triggerResouce, rowAction.id, { rowId: row._id!, @@ -833,10 +853,10 @@ describe("/rowsActions", () => { it.each(disallowedRoleConfig)( "rejects if the user does not have table read permission (user %s, table %s)", async (userRole, resourcePermission) => { - const resourceId = await getResourceId() + const { permissionResource, triggerResouce } = await getResources() await config.api.permission.add({ level: PermissionLevel.READ, - resourceId, + resourceId: permissionResource, roleId: resourcePermission, }) @@ -845,7 +865,7 @@ describe("/rowsActions", () => { await config.withUser(normalUser, async () => { await config.publish() await config.api.rowAction.trigger( - resourceId, + triggerResouce, rowAction.id, { rowId: row._id!, From b27d52a0f34ac82b4a096e154f5ca56cbeb28dc1 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Robert=20Sil=C3=A9n?= Date: Wed, 4 Sep 2024 14:49:30 +0300 Subject: [PATCH 08/20] add MariaDB to README.md add MariaDB to README.md - only fair as its the default running most distros for several years already --- README.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/README.md b/README.md index 64492b97e4..26ad9f80c2 100644 --- a/README.md +++ b/README.md @@ -65,7 +65,7 @@ Budibase is open-source - licensed as GPL v3. This should fill you with confiden

### Load data or start from scratch -Budibase pulls data from multiple sources, including MongoDB, CouchDB, PostgreSQL, MySQL, Airtable, S3, DynamoDB, or a REST API. And unlike other platforms, with Budibase you can start from scratch and create business apps with no data sources. [Request new datasources](https://github.com/Budibase/budibase/discussions?discussions_q=category%3AIdeas). +Budibase pulls data from multiple sources, including MongoDB, CouchDB, PostgreSQL, MariaDB, MySQL, Airtable, S3, DynamoDB, or a REST API. And unlike other platforms, with Budibase you can start from scratch and create business apps with no data sources. [Request new datasources](https://github.com/Budibase/budibase/discussions?discussions_q=category%3AIdeas).

Budibase data From 5e98580b60f64664c10219f7fd07b6780f6677ad Mon Sep 17 00:00:00 2001 From: Andrew Kingston Date: Thu, 5 Sep 2024 14:03:03 +0100 Subject: [PATCH 09/20] Improve new component DND for both types of layout --- packages/client/src/components/Component.svelte | 4 +++- packages/client/src/components/preview/HoverIndicator.svelte | 2 +- packages/client/src/utils/styleable.js | 2 +- 3 files changed, 5 insertions(+), 3 deletions(-) diff --git a/packages/client/src/components/Component.svelte b/packages/client/src/components/Component.svelte index 7b87040c4f..f80cd80c00 100644 --- a/packages/client/src/components/Component.svelte +++ b/packages/client/src/components/Component.svelte @@ -153,7 +153,7 @@ $: builderInteractive = $builderStore.inBuilder && insideScreenslot && !isBlock && !instance.static $: devToolsInteractive = $devToolsStore.allowSelection && !isBlock - $: interactive = !isRoot && (builderInteractive || devToolsInteractive) + $: interactive = builderInteractive || devToolsInteractive $: editing = editable && selected && $builderStore.editMode $: draggable = !inDragPath && @@ -231,6 +231,7 @@ empty: emptyState, selected, interactive, + isRoot, draggable, editable, isBlock, @@ -672,6 +673,7 @@ class:parent={hasChildren} class:block={isBlock} class:error={errorState} + class:root={isRoot} data-id={id} data-name={name} data-icon={icon} diff --git a/packages/client/src/components/preview/HoverIndicator.svelte b/packages/client/src/components/preview/HoverIndicator.svelte index d204d77f49..f1c151ce16 100644 --- a/packages/client/src/components/preview/HoverIndicator.svelte +++ b/packages/client/src/components/preview/HoverIndicator.svelte @@ -19,7 +19,7 @@ newId = e.target.dataset.id } else { // Handle normal components - const element = e.target.closest(".interactive.component") + const element = e.target.closest(".interactive.component:not(.root)") newId = element?.dataset?.id } diff --git a/packages/client/src/utils/styleable.js b/packages/client/src/utils/styleable.js index 0f484a9ab9..884420a1fd 100644 --- a/packages/client/src/utils/styleable.js +++ b/packages/client/src/utils/styleable.js @@ -93,7 +93,7 @@ export const styleable = (node, styles = {}) => { node.addEventListener("mouseout", applyNormalStyles) // Add builder preview click listener - if (newStyles.interactive) { + if (newStyles.interactive && !newStyles.isRoot) { node.addEventListener("click", selectComponent, false) node.addEventListener("dblclick", editComponent, false) } From 6f8e669107941ece93a1930f20c9091a85648b9c Mon Sep 17 00:00:00 2001 From: Andrew Kingston Date: Thu, 5 Sep 2024 14:54:16 +0100 Subject: [PATCH 10/20] Improve new component DND for grids and add mobile support --- .../client/src/components/Component.svelte | 18 +++++---- .../preview/DNDPlaceholderOverlay.svelte | 7 +++- packages/client/src/stores/dnd.js | 12 +++--- packages/client/src/stores/screens.js | 2 + packages/client/src/utils/computed.js | 38 ------------------- packages/client/src/utils/grid.js | 8 +++- 6 files changed, 29 insertions(+), 56 deletions(-) delete mode 100644 packages/client/src/utils/computed.js diff --git a/packages/client/src/components/Component.svelte b/packages/client/src/components/Component.svelte index f80cd80c00..f474b01489 100644 --- a/packages/client/src/components/Component.svelte +++ b/packages/client/src/components/Component.svelte @@ -189,12 +189,6 @@ // Scroll the selected element into view $: selected && scrollIntoView() - // When dragging and dropping, pad components to allow dropping between - // nested layers. Only reset this when dragging stops. - let pad = false - $: pad = pad || (interactive && hasChildren && inDndPath) - $: $dndIsDragging, (pad = false) - // Themes $: currentTheme = $context?.device?.theme $: darkMode = !currentTheme?.includes("light") @@ -206,8 +200,10 @@ } // Metadata to pass into grid action to apply CSS - const insideGrid = - parent?._component.endsWith("/container") && parent?.layout === "grid" + const checkGrid = x => + x?._component.endsWith("/container") && x?.layout === "grid" + $: insideGrid = checkGrid(parent) + $: isGrid = checkGrid(instance) $: gridMetadata = { insideGrid, ignoresLayout: definition?.ignoresLayout === true, @@ -219,6 +215,12 @@ errored: errorState, } + // When dragging and dropping, pad components to allow dropping between + // nested layers. Only reset this when dragging stops. + let pad = false + $: pad = pad || (!isGrid && interactive && hasChildren && inDndPath) + $: $dndIsDragging, (pad = false) + // Update component context $: store.set({ id, diff --git a/packages/client/src/components/preview/DNDPlaceholderOverlay.svelte b/packages/client/src/components/preview/DNDPlaceholderOverlay.svelte index 0ad4280e07..61cecc885b 100644 --- a/packages/client/src/components/preview/DNDPlaceholderOverlay.svelte +++ b/packages/client/src/components/preview/DNDPlaceholderOverlay.svelte @@ -6,8 +6,11 @@ let left, top, height, width const updatePosition = () => { - const node = - document.getElementsByClassName(DNDPlaceholderID)[0]?.childNodes[0] + let node = document.getElementsByClassName(DNDPlaceholderID)[0] + const insideGrid = node?.dataset.insideGrid === "true" + if (!insideGrid) { + node = document.getElementsByClassName(`${DNDPlaceholderID}-dom`)[0] + } if (!node) { height = 0 width = 0 diff --git a/packages/client/src/stores/dnd.js b/packages/client/src/stores/dnd.js index 1bdd510eb7..43d4eeeed8 100644 --- a/packages/client/src/stores/dnd.js +++ b/packages/client/src/stores/dnd.js @@ -1,5 +1,5 @@ import { writable } from "svelte/store" -import { computed } from "../utils/computed.js" +import { derivedMemo } from "@budibase/frontend-core" const createDndStore = () => { const initialState = { @@ -78,11 +78,11 @@ export const dndStore = createDndStore() // performance by deriving any state that needs to be externally observed. // By doing this and using primitives, we can avoid invalidating other stores // or components which depend on DND state unless values actually change. -export const dndParent = computed(dndStore, x => x.drop?.parent) -export const dndIndex = computed(dndStore, x => x.drop?.index) -export const dndBounds = computed(dndStore, x => x.source?.bounds) -export const dndIsDragging = computed(dndStore, x => !!x.source) -export const dndIsNewComponent = computed( +export const dndParent = derivedMemo(dndStore, x => x.drop?.parent) +export const dndIndex = derivedMemo(dndStore, x => x.drop?.index) +export const dndBounds = derivedMemo(dndStore, x => x.source?.bounds) +export const dndIsDragging = derivedMemo(dndStore, x => !!x.source) +export const dndIsNewComponent = derivedMemo( dndStore, x => x.source?.newComponentType != null ) diff --git a/packages/client/src/stores/screens.js b/packages/client/src/stores/screens.js index 3c5ece0a6c..bc87216660 100644 --- a/packages/client/src/stores/screens.js +++ b/packages/client/src/stores/screens.js @@ -92,6 +92,8 @@ const createScreenStore = () => { width: `${$dndBounds?.width || 400}px`, height: `${$dndBounds?.height || 200}px`, opacity: 0, + "--default-width": $dndBounds?.width || 400, + "--default-height": $dndBounds?.height || 200, }, }, static: true, diff --git a/packages/client/src/utils/computed.js b/packages/client/src/utils/computed.js deleted file mode 100644 index aa89e7ad1b..0000000000 --- a/packages/client/src/utils/computed.js +++ /dev/null @@ -1,38 +0,0 @@ -import { writable } from "svelte/store" - -/** - * Extension of Svelte's built in "derived" stores, which the addition of deep - * comparison of non-primitives. Falls back to using shallow comparison for - * primitive types to avoid performance penalties. - * Useful for instances where a deep comparison is cheaper than an additional - * store invalidation. - * @param store the store to observer - * @param deriveValue the derivation function - * @returns {Writable<*>} a derived svelte store containing just the derived value - */ -export const computed = (store, deriveValue) => { - const initialValue = deriveValue(store) - const computedStore = writable(initialValue) - let lastKey = getKey(initialValue) - - store.subscribe(state => { - const value = deriveValue(state) - const key = getKey(value) - if (key !== lastKey) { - lastKey = key - computedStore.set(value) - } - }) - - return computedStore -} - -// Helper function to serialise any value into a primitive which can be cheaply -// and shallowly compared -const getKey = value => { - if (value == null || typeof value !== "object") { - return value - } else { - return JSON.stringify(value) - } -} diff --git a/packages/client/src/utils/grid.js b/packages/client/src/utils/grid.js index 1727b904ca..142a7ed55a 100644 --- a/packages/client/src/utils/grid.js +++ b/packages/client/src/utils/grid.js @@ -92,8 +92,12 @@ export const gridLayout = (node, metadata) => { } // Determine default width and height of component - let width = errored ? 500 : definition?.size?.width || 200 - let height = errored ? 60 : definition?.size?.height || 200 + let width = styles["--default-width"] ?? definition?.size?.width ?? 200 + let height = styles["--default-height"] ?? definition?.size?.height ?? 200 + if (errored) { + width = 500 + height = 60 + } width += 2 * GridSpacing height += 2 * GridSpacing let vars = { From 8860acad732d814c36eb44bf7054bc469e0a1aea Mon Sep 17 00:00:00 2001 From: Andrew Kingston Date: Thu, 5 Sep 2024 15:15:28 +0100 Subject: [PATCH 11/20] Update grid layout to support placeholders, as well as grid screens --- packages/client/src/components/Component.svelte | 1 + .../client/src/components/app/EmptyPlaceholder.svelte | 4 +++- .../client/src/components/app/ScreenPlaceholder.svelte | 2 ++ .../src/components/app/container/GridContainer.svelte | 8 +------- 4 files changed, 7 insertions(+), 8 deletions(-) diff --git a/packages/client/src/components/Component.svelte b/packages/client/src/components/Component.svelte index f474b01489..b65d7b0cdc 100644 --- a/packages/client/src/components/Component.svelte +++ b/packages/client/src/components/Component.svelte @@ -240,6 +240,7 @@ }, empty: emptyState, selected, + isRoot, inSelectedPath, name, editing, diff --git a/packages/client/src/components/app/EmptyPlaceholder.svelte b/packages/client/src/components/app/EmptyPlaceholder.svelte index 2c3596aadf..b93bd01457 100644 --- a/packages/client/src/components/app/EmptyPlaceholder.svelte +++ b/packages/client/src/components/app/EmptyPlaceholder.svelte @@ -18,9 +18,11 @@ display: flex; flex-direction: row; justify-content: flex-start; - align-items: center; + align-items: flex-start; color: var(--spectrum-global-color-gray-600); font-size: var(--font-size-s); gap: var(--spacing-s); + grid-column: 1 / -1; + grid-row: 1 / -1; } diff --git a/packages/client/src/components/app/ScreenPlaceholder.svelte b/packages/client/src/components/app/ScreenPlaceholder.svelte index 7635f55054..27d08c20ff 100644 --- a/packages/client/src/components/app/ScreenPlaceholder.svelte +++ b/packages/client/src/components/app/ScreenPlaceholder.svelte @@ -23,6 +23,8 @@ align-items: center; gap: var(--spacing-s); flex: 1 1 auto; + grid-column: 1 / -1; + grid-row: 1 / -1; } .placeholder :global(.spectrum-Button) { margin-top: var(--spacing-m); diff --git a/packages/client/src/components/app/container/GridContainer.svelte b/packages/client/src/components/app/container/GridContainer.svelte index 1f68926658..8850dbcb6f 100644 --- a/packages/client/src/components/app/container/GridContainer.svelte +++ b/packages/client/src/components/app/container/GridContainer.svelte @@ -27,7 +27,6 @@ $: availableRows = Math.floor(height / GridRowHeight) $: rows = Math.max(requiredRows, availableRows) $: mobile = $context.device.mobile - $: empty = $component.empty $: colSize = width / GridColumns $: styles.set({ ...$component.styles, @@ -40,7 +39,6 @@ "--col-size": colSize, "--row-size": GridRowHeight, }, - empty: false, }) // Calculates the minimum number of rows required to render all child @@ -145,11 +143,7 @@ {/each} {/if} - - - {#if !empty && mounted} - - {/if} +