From a00a64bb6e5451d2b4b9f93e748b9e25a37cf171 Mon Sep 17 00:00:00 2001 From: Sam Rose Date: Thu, 10 Oct 2024 14:56:38 +0100 Subject: [PATCH 1/9] Properly clean up row actions on table deletion. --- .../server/src/api/controllers/table/index.ts | 1 + .../src/api/routes/tests/rowAction.spec.ts | 40 +++++++++++++++++++ packages/server/src/sdk/app/rowActions.ts | 15 +++++++ .../src/tests/utilities/api/automation.ts | 22 +++++++++- 4 files changed, 77 insertions(+), 1 deletion(-) diff --git a/packages/server/src/api/controllers/table/index.ts b/packages/server/src/api/controllers/table/index.ts index 404f82e57c..efe1a88e4a 100644 --- a/packages/server/src/api/controllers/table/index.ts +++ b/packages/server/src/api/controllers/table/index.ts @@ -139,6 +139,7 @@ export async function save(ctx: UserCtx) { export async function destroy(ctx: UserCtx) { const appId = ctx.appId const tableId = ctx.params.tableId + await sdk.rowActions.deleteAll(tableId) const deletedTable = await pickApi({ tableId }).destroy(ctx) await events.table.deleted(deletedTable) ctx.eventEmitter && diff --git a/packages/server/src/api/routes/tests/rowAction.spec.ts b/packages/server/src/api/routes/tests/rowAction.spec.ts index fb59ce73c3..7140f3486e 100644 --- a/packages/server/src/api/routes/tests/rowAction.spec.ts +++ b/packages/server/src/api/routes/tests/rowAction.spec.ts @@ -1041,4 +1041,44 @@ describe("/rowsActions", () => { ) }) }) + + describe("scenarios", () => { + // https://linear.app/budibase/issue/BUDI-8717/ + it("should not brick the app when deleting a table with row actions", async () => { + const view = await config.api.viewV2.create({ + tableId, + name: generator.guid(), + schema: { + name: { visible: true }, + }, + }) + + const rowAction = await config.api.rowAction.save(tableId, { + name: generator.guid(), + }) + + await config.api.rowAction.setViewPermission( + tableId, + view.id, + rowAction.id + ) + + let actionsResp = await config.api.rowAction.find(tableId) + expect(actionsResp.actions[rowAction.id]).toEqual({ + ...rowAction, + allowedSources: [tableId, view.id], + }) + + const table = await config.api.table.get(tableId) + await config.api.table.destroy(table._id!, table._rev!) + + // In the bug reported by Conor, when a delete got deleted its row action + // document was not being cleaned up. This meant there existed code paths + // that would find it and try to reference the tables within it, resulting + // in errors. + await config.api.automation.fetchEnriched({ + status: 200, + }) + }) + }) }) diff --git a/packages/server/src/sdk/app/rowActions.ts b/packages/server/src/sdk/app/rowActions.ts index 418a906c00..958401b8b9 100644 --- a/packages/server/src/sdk/app/rowActions.ts +++ b/packages/server/src/sdk/app/rowActions.ts @@ -1,5 +1,6 @@ import { context, docIds, HTTPError, utils } from "@budibase/backend-core" import { + Automation, AutomationTriggerStepId, SEPARATOR, TableRowActions, @@ -105,6 +106,20 @@ export async function getAll(tableId: string) { return await db.get(rowActionsId) } +export async function deleteAll(tableId: string) { + const db = context.getAppDB() + + const doc = await getAll(tableId) + const automationIds = Object.values(doc.actions).map(a => a.automationId) + const automations = await db.getMultiple(automationIds) + + for (const automation of automations) { + await sdk.automations.remove(automation._id!, automation._rev!) + } + + await db.remove(doc) +} + export async function docExists(tableId: string) { const db = context.getAppDB() const rowActionsId = generateRowActionsID(tableId) diff --git a/packages/server/src/tests/utilities/api/automation.ts b/packages/server/src/tests/utilities/api/automation.ts index 61bd915647..11c041d52b 100644 --- a/packages/server/src/tests/utilities/api/automation.ts +++ b/packages/server/src/tests/utilities/api/automation.ts @@ -1,4 +1,4 @@ -import { Automation } from "@budibase/types" +import { Automation, FetchAutomationResponse } from "@budibase/types" import { Expectations, TestAPI } from "./base" export class AutomationAPI extends TestAPI { @@ -14,6 +14,26 @@ export class AutomationAPI extends TestAPI { ) return result } + + fetch = async ( + expectations?: Expectations + ): Promise => { + return await this._get(`/api/automations`, { + expectations, + }) + } + + fetchEnriched = async ( + expectations?: Expectations + ): Promise => { + return await this._get( + `/api/automations?enrich=true`, + { + expectations, + } + ) + } + post = async ( body: Automation, expectations?: Expectations From e191c90385803118fa77a9e63cef7760b8f0ee1f Mon Sep 17 00:00:00 2001 From: Adria Navarro Date: Thu, 10 Oct 2024 16:14:36 +0200 Subject: [PATCH 2/9] Simplify enriched column --- packages/server/src/sdk/app/views/index.ts | 15 +++++++++++---- packages/types/src/sdk/view.ts | 13 +++++++++++-- 2 files changed, 22 insertions(+), 6 deletions(-) diff --git a/packages/server/src/sdk/app/views/index.ts b/packages/server/src/sdk/app/views/index.ts index 35f2a84b07..246922678d 100644 --- a/packages/server/src/sdk/app/views/index.ts +++ b/packages/server/src/sdk/app/views/index.ts @@ -1,4 +1,5 @@ import { + BBReferenceFieldSubType, CalculationType, canGroupBy, FeatureFlag, @@ -7,6 +8,7 @@ import { PermissionLevel, RelationSchemaField, RenameColumn, + RequiredKeys, Table, TableSchema, View, @@ -325,13 +327,18 @@ export async function enrichSchema( const viewFieldSchema = viewFields[relTableFieldName] const isVisible = !!viewFieldSchema?.visible const isReadonly = !!viewFieldSchema?.readonly - result[relTableFieldName] = { - ...relTableField, - ...viewFieldSchema, - name: relTableField.name, + const enrichedFieldSchema: RequiredKeys = { visible: isVisible, readonly: isReadonly, + order: viewFieldSchema?.order, + width: viewFieldSchema?.width, + + icon: relTableField.icon, + type: relTableField.type, + subtype: relTableField.subtype, } + } + result[relTableFieldName] = enrichedFieldSchema } return result } diff --git a/packages/types/src/sdk/view.ts b/packages/types/src/sdk/view.ts index 422207197d..4c555fbaa7 100644 --- a/packages/types/src/sdk/view.ts +++ b/packages/types/src/sdk/view.ts @@ -1,4 +1,10 @@ -import { FieldSchema, RelationSchemaField, ViewV2 } from "../documents" +import { + FieldSchema, + FieldSubType, + FieldType, + RelationSchemaField, + ViewV2, +} from "../documents" export interface ViewV2Enriched extends ViewV2 { schema?: { @@ -8,4 +14,7 @@ export interface ViewV2Enriched extends ViewV2 { } } -export type ViewV2ColumnEnriched = RelationSchemaField & FieldSchema +export interface ViewV2ColumnEnriched extends RelationSchemaField { + type: FieldType + subtype?: FieldSubType +} From 41cd0d96d625cc125480cf66b61fccf96edcd147 Mon Sep 17 00:00:00 2001 From: Adria Navarro Date: Thu, 10 Oct 2024 16:14:53 +0200 Subject: [PATCH 3/9] Fix multiple user column icon --- packages/frontend-core/src/components/grid/lib/utils.js | 4 ++++ packages/server/src/sdk/app/views/index.ts | 8 ++++++++ 2 files changed, 12 insertions(+) diff --git a/packages/frontend-core/src/components/grid/lib/utils.js b/packages/frontend-core/src/components/grid/lib/utils.js index 1988b66cc2..fb062cb1fa 100644 --- a/packages/frontend-core/src/components/grid/lib/utils.js +++ b/packages/frontend-core/src/components/grid/lib/utils.js @@ -19,6 +19,10 @@ export const getCellID = (rowId, fieldName) => { } export const getColumnIcon = column => { + if (column.schema.icon) { + return column.schema.icon + } + if (column.schema.autocolumn) { return "MagicWand" } diff --git a/packages/server/src/sdk/app/views/index.ts b/packages/server/src/sdk/app/views/index.ts index 246922678d..45004a861d 100644 --- a/packages/server/src/sdk/app/views/index.ts +++ b/packages/server/src/sdk/app/views/index.ts @@ -337,6 +337,14 @@ export async function enrichSchema( type: relTableField.type, subtype: relTableField.subtype, } + if ( + !enrichedFieldSchema.icon && + relTableField.type === FieldType.BB_REFERENCE && + relTableField.subtype === BBReferenceFieldSubType.USER && + !helpers.schema.isDeprecatedSingleUserColumn(relTableField) + ) { + // Forcing the icon, otherwise we would need to pass the constraints to show the proper icon + enrichedFieldSchema.icon = "UserGroup" } result[relTableFieldName] = enrichedFieldSchema } From 4c4429b88ade15ee8ed98e6233d814d2a5041360 Mon Sep 17 00:00:00 2001 From: Sam Rose Date: Thu, 10 Oct 2024 15:49:00 +0100 Subject: [PATCH 4/9] Fix tests. --- packages/backend-core/src/db/couch/DatabaseImpl.ts | 11 +++++++++++ packages/backend-core/src/db/instrumentation.ts | 7 +++++++ packages/server/src/api/controllers/rowAction/crud.ts | 5 +++-- packages/server/src/sdk/app/automations/utils.ts | 8 ++++++-- packages/server/src/sdk/app/rowActions.ts | 11 +++++++---- packages/types/src/sdk/db.ts | 5 +++++ 6 files changed, 39 insertions(+), 8 deletions(-) diff --git a/packages/backend-core/src/db/couch/DatabaseImpl.ts b/packages/backend-core/src/db/couch/DatabaseImpl.ts index 2b37526dde..8ca20bf8e1 100644 --- a/packages/backend-core/src/db/couch/DatabaseImpl.ts +++ b/packages/backend-core/src/db/couch/DatabaseImpl.ts @@ -211,6 +211,17 @@ export class DatabaseImpl implements Database { }) } + async tryGet(id?: string): Promise { + try { + return await this.get(id) + } catch (err: any) { + if (err.statusCode === 404) { + return undefined + } + throw err + } + } + async getMultiple( ids: string[], opts?: { allowMissing?: boolean; excludeDocs?: boolean } diff --git a/packages/backend-core/src/db/instrumentation.ts b/packages/backend-core/src/db/instrumentation.ts index 7026224564..e08bfc0362 100644 --- a/packages/backend-core/src/db/instrumentation.ts +++ b/packages/backend-core/src/db/instrumentation.ts @@ -42,6 +42,13 @@ export class DDInstrumentedDatabase implements Database { }) } + tryGet(id?: string | undefined): Promise { + return tracer.trace("db.tryGet", span => { + span?.addTags({ db_name: this.name, doc_id: id }) + return this.db.tryGet(id) + }) + } + getMultiple( ids: string[], opts?: { allowMissing?: boolean | undefined } | undefined diff --git a/packages/server/src/api/controllers/rowAction/crud.ts b/packages/server/src/api/controllers/rowAction/crud.ts index 67b6d9383e..ca84b2ea30 100644 --- a/packages/server/src/api/controllers/rowAction/crud.ts +++ b/packages/server/src/api/controllers/rowAction/crud.ts @@ -21,14 +21,15 @@ export async function find(ctx: Ctx) { const table = await getTable(ctx) const tableId = table._id! - if (!(await sdk.rowActions.docExists(tableId))) { + const rowActions = await sdk.rowActions.getAll(tableId) + if (!rowActions) { ctx.body = { actions: {}, } return } - const { actions } = await sdk.rowActions.getAll(tableId) + const { actions } = rowActions const result: RowActionsResponse = { actions: Object.entries(actions).reduce>( (acc, [key, action]) => ({ diff --git a/packages/server/src/sdk/app/automations/utils.ts b/packages/server/src/sdk/app/automations/utils.ts index 2ce0012b6a..f3bd77c012 100644 --- a/packages/server/src/sdk/app/automations/utils.ts +++ b/packages/server/src/sdk/app/automations/utils.ts @@ -26,13 +26,13 @@ export async function getBuilderData( return tableNameCache[tableId] } - const rowActionNameCache: Record = {} + const rowActionNameCache: Record = {} async function getRowActionName(tableId: string, rowActionId: string) { if (!rowActionNameCache[tableId]) { rowActionNameCache[tableId] = await sdk.rowActions.getAll(tableId) } - return rowActionNameCache[tableId].actions[rowActionId]?.name + return rowActionNameCache[tableId]?.actions[rowActionId]?.name } const result: Record = {} @@ -51,6 +51,10 @@ export async function getBuilderData( const tableName = await getTableName(tableId) const rowActionName = await getRowActionName(tableId, rowActionId) + if (!rowActionName) { + throw new Error(`Row action not found: ${rowActionId}`) + } + result[automation._id!] = { displayName: `${tableName}: ${automation.name}`, triggerInfo: { diff --git a/packages/server/src/sdk/app/rowActions.ts b/packages/server/src/sdk/app/rowActions.ts index 958401b8b9..3d8a6fd9be 100644 --- a/packages/server/src/sdk/app/rowActions.ts +++ b/packages/server/src/sdk/app/rowActions.ts @@ -103,13 +103,17 @@ export async function get(tableId: string, rowActionId: string) { export async function getAll(tableId: string) { const db = context.getAppDB() const rowActionsId = generateRowActionsID(tableId) - return await db.get(rowActionsId) + return await db.tryGet(rowActionsId) } export async function deleteAll(tableId: string) { const db = context.getAppDB() const doc = await getAll(tableId) + if (!doc) { + return + } + const automationIds = Object.values(doc.actions).map(a => a.automationId) const automations = await db.getMultiple(automationIds) @@ -238,9 +242,8 @@ export async function run(tableId: any, rowActionId: any, rowId: string) { throw new HTTPError("Table not found", 404) } - const { actions } = await getAll(tableId) - - const rowAction = actions[rowActionId] + const rowActions = await getAll(tableId) + const rowAction = rowActions?.actions[rowActionId] if (!rowAction) { throw new HTTPError("Row action not found", 404) } diff --git a/packages/types/src/sdk/db.ts b/packages/types/src/sdk/db.ts index 49baaa5bb1..b679d6e182 100644 --- a/packages/types/src/sdk/db.ts +++ b/packages/types/src/sdk/db.ts @@ -129,7 +129,12 @@ export interface Database { name: string exists(): Promise + /** + * @deprecated the plan is to get everything using `tryGet` instead, then rename + * `tryGet` to `get`. + */ get(id?: string): Promise + tryGet(id?: string): Promise exists(docId: string): Promise getMultiple( ids: string[], From ca7f3369af2839432da7610cfd1af113d4f70945 Mon Sep 17 00:00:00 2001 From: Adria Navarro Date: Thu, 10 Oct 2024 16:51:08 +0200 Subject: [PATCH 5/9] Fix user icons on picker --- .../src/components/grid/controls/ColumnsSettingContent.svelte | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/frontend-core/src/components/grid/controls/ColumnsSettingContent.svelte b/packages/frontend-core/src/components/grid/controls/ColumnsSettingContent.svelte index bbc3d55f04..dd12af3ff4 100644 --- a/packages/frontend-core/src/components/grid/controls/ColumnsSettingContent.svelte +++ b/packages/frontend-core/src/components/grid/controls/ColumnsSettingContent.svelte @@ -125,7 +125,7 @@ subtype: column.subtype, visible: column.visible, readonly: column.readonly, - constraints: column.constraints, // This is needed to properly display "users" column + icon: column.icon, }, } }) From 18e1bd0b4c58c8f2541767ca932edf195ed8632c Mon Sep 17 00:00:00 2001 From: Adria Navarro Date: Thu, 10 Oct 2024 17:03:46 +0200 Subject: [PATCH 6/9] Fix test --- packages/server/src/sdk/app/views/tests/views.spec.ts | 2 -- 1 file changed, 2 deletions(-) diff --git a/packages/server/src/sdk/app/views/tests/views.spec.ts b/packages/server/src/sdk/app/views/tests/views.spec.ts index 1d7360c5eb..948ffbf096 100644 --- a/packages/server/src/sdk/app/views/tests/views.spec.ts +++ b/packages/server/src/sdk/app/views/tests/views.spec.ts @@ -355,13 +355,11 @@ describe("table sdk", () => { visible: true, columns: { title: { - name: "title", type: "string", visible: true, readonly: true, }, age: { - name: "age", type: "number", visible: false, readonly: false, From 5f4694d4d8699a364c5a44dd41921f3bcbfa42b0 Mon Sep 17 00:00:00 2001 From: mike12345567 Date: Thu, 10 Oct 2024 16:13:32 +0100 Subject: [PATCH 7/9] Fix for an issue found with custom role naming. --- packages/backend-core/src/security/roles.ts | 4 +++- packages/server/src/api/routes/tests/role.spec.js | 6 +++--- 2 files changed, 6 insertions(+), 4 deletions(-) diff --git a/packages/backend-core/src/security/roles.ts b/packages/backend-core/src/security/roles.ts index 65339832cf..fa2d114d7d 100644 --- a/packages/backend-core/src/security/roles.ts +++ b/packages/backend-core/src/security/roles.ts @@ -435,7 +435,9 @@ export function getExternalRoleID(roleId: string, version?: string) { roleId.startsWith(DocumentType.ROLE) && (isBuiltin(roleId) || version === RoleIDVersion.NAME) ) { - return roleId.split(`${DocumentType.ROLE}${SEPARATOR}`)[1] + const parts = roleId.split(SEPARATOR) + parts.shift() + return parts.join(SEPARATOR) } return roleId } diff --git a/packages/server/src/api/routes/tests/role.spec.js b/packages/server/src/api/routes/tests/role.spec.js index 4575f9b213..00025e396a 100644 --- a/packages/server/src/api/routes/tests/role.spec.js +++ b/packages/server/src/api/routes/tests/role.spec.js @@ -161,7 +161,7 @@ describe("/roles", () => { it("should not fetch higher level accessible roles when a custom role header is provided", async () => { await createRole({ - name: `CUSTOM_ROLE`, + name: `custom_role_1`, inherits: roles.BUILTIN_ROLE_IDS.BASIC, permissionId: permissions.BuiltinPermissionID.READ_ONLY, version: "name", @@ -170,11 +170,11 @@ describe("/roles", () => { .get("/api/roles/accessible") .set({ ...config.defaultHeaders(), - "x-budibase-role": "CUSTOM_ROLE", + "x-budibase-role": "custom_role_1", }) .expect(200) expect(res.body.length).toBe(3) - expect(res.body[0]).toBe("CUSTOM_ROLE") + expect(res.body[0]).toBe("custom_role_1") expect(res.body[1]).toBe("BASIC") expect(res.body[2]).toBe("PUBLIC") }) From a1e55a5324fbf2917553a6bb235631b1c8abbfec Mon Sep 17 00:00:00 2001 From: Budibase Staging Release Bot <> Date: Thu, 10 Oct 2024 15:28:36 +0000 Subject: [PATCH 8/9] Bump version to 2.32.16 --- lerna.json | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lerna.json b/lerna.json index 4d36affd04..ba3db109d0 100644 --- a/lerna.json +++ b/lerna.json @@ -1,6 +1,6 @@ { "$schema": "node_modules/lerna/schemas/lerna-schema.json", - "version": "2.32.15", + "version": "2.32.16", "npmClient": "yarn", "packages": [ "packages/*", From 402013bef3c46900a35eff24d8034e1a330dcb2b Mon Sep 17 00:00:00 2001 From: melohagan <101575380+melohagan@users.noreply.github.com> Date: Fri, 11 Oct 2024 07:39:03 +0100 Subject: [PATCH 9/9] UI tweak for days remaining banner of free trial (#14764) --- .../portal/licensing/EnterpriseBasicTrialBanner.svelte | 10 ++++++++-- 1 file changed, 8 insertions(+), 2 deletions(-) diff --git a/packages/builder/src/components/portal/licensing/EnterpriseBasicTrialBanner.svelte b/packages/builder/src/components/portal/licensing/EnterpriseBasicTrialBanner.svelte index 111f0481b9..350ebb0f11 100644 --- a/packages/builder/src/components/portal/licensing/EnterpriseBasicTrialBanner.svelte +++ b/packages/builder/src/components/portal/licensing/EnterpriseBasicTrialBanner.svelte @@ -14,7 +14,13 @@ function daysUntilCancel() { const cancelAt = license?.billing?.subscription?.cancelAt const diffTime = Math.abs(cancelAt - new Date().getTime()) / 1000 - return Math.floor(diffTime / oneDayInSeconds) + const days = Math.floor(diffTime / oneDayInSeconds) + if (days === 1) { + return "tomorrow." + } else if (days === 0) { + return "today." + } + return `in ${days} days.` } @@ -28,7 +34,7 @@ extraLinkAction={$licensing.goToUpgradePage} showCloseButton={false} > - Your free trial will end in {daysUntilCancel()} days. + Your free trial will end {daysUntilCancel()} {/if}