From 55be64b3718b9f0b944b00a17e2fb1c33a82aa80 Mon Sep 17 00:00:00 2001 From: Sam Rose Date: Thu, 10 Oct 2024 09:16:43 +0100 Subject: [PATCH 1/4] Only check visible fields when checking group by view calculations. --- .../src/api/controllers/row/utils/sqlUtils.ts | 4 +-- .../src/api/routes/tests/viewV2.spec.ts | 28 +++++++++++++++++++ .../src/sdk/app/rows/search/internal/sqs.ts | 4 +-- packages/shared-core/src/helpers/views.ts | 11 ++++++-- 4 files changed, 39 insertions(+), 8 deletions(-) diff --git a/packages/server/src/api/controllers/row/utils/sqlUtils.ts b/packages/server/src/api/controllers/row/utils/sqlUtils.ts index 607fee7580..89f8007642 100644 --- a/packages/server/src/api/controllers/row/utils/sqlUtils.ts +++ b/packages/server/src/api/controllers/row/utils/sqlUtils.ts @@ -133,9 +133,7 @@ export async function buildSqlFieldList( let fields: string[] = [] if (sdk.views.isView(source)) { - fields = Object.keys(helpers.views.basicFields(source)).filter( - key => source.schema?.[key]?.visible !== false - ) + fields = Object.keys(helpers.views.basicFields(source)) } else { fields = extractRealFields(source) } diff --git a/packages/server/src/api/routes/tests/viewV2.spec.ts b/packages/server/src/api/routes/tests/viewV2.spec.ts index 195481ae10..23955a8302 100644 --- a/packages/server/src/api/routes/tests/viewV2.spec.ts +++ b/packages/server/src/api/routes/tests/viewV2.spec.ts @@ -800,6 +800,34 @@ describe.each([ ) } }) + + isInternal && + it("shouldn't trigger a complex type check on a group by field if field is invisible", async () => { + const table = await config.api.table.save( + saveTableRequest({ + schema: { + field: { + name: "field", + type: FieldType.JSON, + }, + }, + }) + ) + + await config.api.viewV2.create( + { + tableId: table._id!, + name: generator.guid(), + type: ViewV2Type.CALCULATION, + schema: { + field: { visible: false }, + }, + }, + { + status: 201, + } + ) + }) }) describe("update", () => { diff --git a/packages/server/src/sdk/app/rows/search/internal/sqs.ts b/packages/server/src/sdk/app/rows/search/internal/sqs.ts index 3847eb8f31..ad323a8c12 100644 --- a/packages/server/src/sdk/app/rows/search/internal/sqs.ts +++ b/packages/server/src/sdk/app/rows/search/internal/sqs.ts @@ -68,9 +68,7 @@ async function buildInternalFieldList( const { relationships, allowedFields } = opts || {} let schemaFields: string[] = [] if (sdk.views.isView(source)) { - schemaFields = Object.keys(helpers.views.basicFields(source)).filter( - key => source.schema?.[key]?.visible !== false - ) + schemaFields = Object.keys(helpers.views.basicFields(source)) } else { schemaFields = Object.keys(source.schema).filter( key => source.schema[key].visible !== false diff --git a/packages/shared-core/src/helpers/views.ts b/packages/shared-core/src/helpers/views.ts index 0113375adf..f0407eeec9 100644 --- a/packages/shared-core/src/helpers/views.ts +++ b/packages/shared-core/src/helpers/views.ts @@ -33,6 +33,13 @@ export function calculationFields(view: UnsavedViewV2) { return pickBy(view.schema || {}, isCalculationField) } -export function basicFields(view: UnsavedViewV2) { - return pickBy(view.schema || {}, field => !isCalculationField(field)) +export function isVisible(field: ViewFieldMetadata) { + return field.visible !== false +} + +export function basicFields(view: UnsavedViewV2, opts?: { visible?: boolean }) { + const { visible = true } = opts || {} + return pickBy(view.schema || {}, field => { + return !isCalculationField(field) && (!visible || isVisible(field)) + }) } From 3e0408d3b948e34820d21ac53a708842fd7c0f69 Mon Sep 17 00:00:00 2001 From: Adria Navarro Date: Thu, 10 Oct 2024 10:34:35 +0200 Subject: [PATCH 2/4] Add new flag --- packages/backend-core/src/features/features.ts | 1 + packages/types/src/sdk/featureFlag.ts | 1 + 2 files changed, 2 insertions(+) diff --git a/packages/backend-core/src/features/features.ts b/packages/backend-core/src/features/features.ts index efe6495cb5..20b207bb02 100644 --- a/packages/backend-core/src/features/features.ts +++ b/packages/backend-core/src/features/features.ts @@ -272,6 +272,7 @@ export const flags = new FlagSet({ SQS: Flag.boolean(env.isDev()), [FeatureFlag.AI_CUSTOM_CONFIGS]: Flag.boolean(env.isDev()), [FeatureFlag.ENRICHED_RELATIONSHIPS]: Flag.boolean(env.isDev()), + [FeatureFlag.TABLES_DEFAULT_ADMIN]: Flag.boolean(env.isDev()), }) type UnwrapPromise = T extends Promise ? U : T diff --git a/packages/types/src/sdk/featureFlag.ts b/packages/types/src/sdk/featureFlag.ts index f87772233d..e35929efe9 100644 --- a/packages/types/src/sdk/featureFlag.ts +++ b/packages/types/src/sdk/featureFlag.ts @@ -3,6 +3,7 @@ export enum FeatureFlag { PER_CREATOR_PER_USER_PRICE_ALERT = "PER_CREATOR_PER_USER_PRICE_ALERT", AI_CUSTOM_CONFIGS = "AI_CUSTOM_CONFIGS", ENRICHED_RELATIONSHIPS = "ENRICHED_RELATIONSHIPS", + TABLES_DEFAULT_ADMIN = "TABLES_DEFAULT_ADMIN", } export interface TenantFeatureFlags { From a646b86131489295334e59d9d961ae7cc50b197d Mon Sep 17 00:00:00 2001 From: Adria Navarro Date: Thu, 10 Oct 2024 10:34:56 +0200 Subject: [PATCH 3/4] Use flag for table --- packages/server/src/sdk/app/tables/create.ts | 18 ++++++++++++------ 1 file changed, 12 insertions(+), 6 deletions(-) diff --git a/packages/server/src/sdk/app/tables/create.ts b/packages/server/src/sdk/app/tables/create.ts index 0b15cdb15a..fa858522bf 100644 --- a/packages/server/src/sdk/app/tables/create.ts +++ b/packages/server/src/sdk/app/tables/create.ts @@ -1,10 +1,10 @@ -import { Row, Table } from "@budibase/types" +import { FeatureFlag, Row, Table } from "@budibase/types" import * as external from "./external" import * as internal from "./internal" import { isExternal } from "./utils" import { setPermissions } from "../permissions" -import { roles } from "@budibase/backend-core" +import { features, roles } from "@budibase/backend-core" export async function create( table: Omit, @@ -18,10 +18,16 @@ export async function create( createdTable = await internal.create(table, rows, userId) } - await setPermissions(createdTable._id!, { - writeRole: roles.BUILTIN_ROLE_IDS.ADMIN, - readRole: roles.BUILTIN_ROLE_IDS.ADMIN, - }) + const setExplicitPermission = await features.flags.isEnabled( + FeatureFlag.TABLES_DEFAULT_ADMIN + ) + + if (setExplicitPermission) { + await setPermissions(createdTable._id!, { + writeRole: roles.BUILTIN_ROLE_IDS.ADMIN, + readRole: roles.BUILTIN_ROLE_IDS.ADMIN, + }) + } return createdTable } From cda7782c6d64625cb75bb54c97b97b36e9a690a3 Mon Sep 17 00:00:00 2001 From: Adria Navarro Date: Thu, 10 Oct 2024 10:35:39 +0200 Subject: [PATCH 4/4] Use flag for views --- packages/server/src/sdk/app/views/index.ts | 20 +++++++++++++------- 1 file changed, 13 insertions(+), 7 deletions(-) diff --git a/packages/server/src/sdk/app/views/index.ts b/packages/server/src/sdk/app/views/index.ts index 629454fecc..6458c3f1bb 100644 --- a/packages/server/src/sdk/app/views/index.ts +++ b/packages/server/src/sdk/app/views/index.ts @@ -1,6 +1,7 @@ import { CalculationType, canGroupBy, + FeatureFlag, FieldType, isNumeric, PermissionLevel, @@ -13,7 +14,7 @@ import { ViewV2ColumnEnriched, ViewV2Enriched, } from "@budibase/types" -import { context, docIds, HTTPError } from "@budibase/backend-core" +import { context, docIds, features, HTTPError } from "@budibase/backend-core" import { helpers, PROTECTED_EXTERNAL_COLUMNS, @@ -244,12 +245,17 @@ export async function create( const view = await pickApi(tableId).create(tableId, viewRequest) - // Set permissions to be the same as the table - const tablePerms = await sdk.permissions.getResourcePerms(tableId) - await sdk.permissions.setPermissions(view.id, { - writeRole: tablePerms[PermissionLevel.WRITE].role, - readRole: tablePerms[PermissionLevel.READ].role, - }) + const setExplicitPermission = await features.flags.isEnabled( + FeatureFlag.TABLES_DEFAULT_ADMIN + ) + if (setExplicitPermission) { + // Set permissions to be the same as the table + const tablePerms = await sdk.permissions.getResourcePerms(tableId) + await sdk.permissions.setPermissions(view.id, { + writeRole: tablePerms[PermissionLevel.WRITE].role, + readRole: tablePerms[PermissionLevel.READ].role, + }) + } return view }