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/server/src/api/controllers/row/external.ts b/packages/server/src/api/controllers/row/external.ts index 4fcd612ec3..082d07283b 100644 --- a/packages/server/src/api/controllers/row/external.ts +++ b/packages/server/src/api/controllers/row/external.ts @@ -28,6 +28,7 @@ import { import { cloneDeep } from "lodash" import { generateIdForRow } from "./utils" import { helpers } from "@budibase/shared-core" +import { HTTPError } from "@budibase/backend-core" export async function handleRequest( operation: T, @@ -102,6 +103,11 @@ export async function patch(ctx: UserCtx) { export async function destroy(ctx: UserCtx) { const source = await utils.getSource(ctx) + + if (sdk.views.isView(source) && helpers.views.isCalculationView(source)) { + throw new HTTPError("Cannot delete rows through a calculation view", 400) + } + const _id = ctx.request.body._id const { row } = await handleRequest(Operation.DELETE, source, { id: breakRowIdField(_id), diff --git a/packages/server/src/api/controllers/row/internal.ts b/packages/server/src/api/controllers/row/internal.ts index 53b24d517c..e193f2e968 100644 --- a/packages/server/src/api/controllers/row/internal.ts +++ b/packages/server/src/api/controllers/row/internal.ts @@ -8,7 +8,7 @@ import { } from "../../../utilities/rowProcessor" import * as utils from "./utils" import { cloneDeep } from "lodash/fp" -import { context } from "@budibase/backend-core" +import { context, HTTPError } from "@budibase/backend-core" import { finaliseRow, updateRelatedFormula } from "./staticFormula" import { FieldType, @@ -16,6 +16,7 @@ import { PatchRowRequest, PatchRowResponse, Row, + Table, UserCtx, } from "@budibase/types" import sdk from "../../../sdk" @@ -97,15 +98,26 @@ export async function patch(ctx: UserCtx) { export async function destroy(ctx: UserCtx) { const db = context.getAppDB() - const { tableId } = utils.getSourceId(ctx) + const source = await utils.getSource(ctx) + + if (sdk.views.isView(source) && helpers.views.isCalculationView(source)) { + throw new HTTPError("Cannot delete rows through a calculation view", 400) + } + + let table: Table + if (sdk.views.isView(source)) { + table = await sdk.views.getTable(source.id) + } else { + table = source + } + const { _id } = ctx.request.body let row = await db.get(_id) let _rev = ctx.request.body._rev || row._rev - if (row.tableId !== tableId) { + if (row.tableId !== table._id) { throw "Supplied tableId doesn't match the row's tableId" } - const table = await sdk.tables.getTable(tableId) // update the row to include full relationships before deleting them row = await outputProcessing(table, row, { squash: false, @@ -115,7 +127,7 @@ export async function destroy(ctx: UserCtx) { await linkRows.updateLinks({ eventType: linkRows.EventType.ROW_DELETE, row, - tableId, + tableId: table._id!, }) // remove any attachments that were on the row from object storage await AttachmentCleanup.rowDelete(table, [row]) @@ -123,7 +135,7 @@ export async function destroy(ctx: UserCtx) { await updateRelatedFormula(table, row) let response - if (tableId === InternalTables.USER_METADATA) { + if (table._id === InternalTables.USER_METADATA) { ctx.params = { id: _id, } 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..0b4237406f 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", () => { @@ -1904,6 +1932,59 @@ describe.each([ }) }) }) + + describe("calculation views", () => { + it("should not remove calculation columns when modifying table schema", async () => { + let table = await config.api.table.save( + saveTableRequest({ + schema: { + name: { + name: "name", + type: FieldType.STRING, + }, + age: { + name: "age", + type: FieldType.NUMBER, + }, + }, + }) + ) + + let view = await config.api.viewV2.create({ + tableId: table._id!, + name: generator.guid(), + type: ViewV2Type.CALCULATION, + schema: { + sum: { + visible: true, + calculationType: CalculationType.SUM, + field: "age", + }, + }, + }) + + table = await config.api.table.get(table._id!) + await config.api.table.save({ + ...table, + schema: { + ...table.schema, + name: { + name: "name", + type: FieldType.STRING, + constraints: { presence: true }, + }, + }, + }) + + view = await config.api.viewV2.get(view.id) + expect(Object.keys(view.schema!).sort()).toEqual([ + "age", + "id", + "name", + "sum", + ]) + }) + }) }) describe("row operations", () => { @@ -2148,6 +2229,32 @@ describe.each([ }) await config.api.row.get(table._id!, rows[1]._id!, { status: 200 }) }) + + it("should not be possible to delete a row in a calculation view", async () => { + const row = await config.api.row.save(table._id!, {}) + + const view = await config.api.viewV2.create({ + tableId: table._id!, + name: generator.guid(), + type: ViewV2Type.CALCULATION, + schema: { + id: { visible: true }, + one: { visible: true }, + }, + }) + + await config.api.row.delete( + view.id, + { _id: row._id! }, + { + status: 400, + body: { + message: "Cannot delete rows through a calculation view", + status: 400, + }, + } + ) + }) }) describe("read", () => { @@ -3157,6 +3264,123 @@ describe.each([ } ) }) + + it("should be able to filter rows on the view itself", async () => { + const table = await config.api.table.save( + saveTableRequest({ + schema: { + quantity: { + type: FieldType.NUMBER, + name: "quantity", + }, + price: { + type: FieldType.NUMBER, + name: "price", + }, + }, + }) + ) + + const view = await config.api.viewV2.create({ + tableId: table._id!, + name: generator.guid(), + type: ViewV2Type.CALCULATION, + query: { + equal: { + quantity: 1, + }, + }, + schema: { + sum: { + visible: true, + calculationType: CalculationType.SUM, + field: "price", + }, + }, + }) + + await config.api.row.bulkImport(table._id!, { + rows: [ + { + quantity: 1, + price: 1, + }, + { + quantity: 1, + price: 2, + }, + { + quantity: 2, + price: 10, + }, + ], + }) + + const { rows } = await config.api.viewV2.search(view.id, { + query: {}, + }) + expect(rows).toHaveLength(1) + expect(rows[0].sum).toEqual(3) + }) + + it("should be able to filter on group by fields", async () => { + const table = await config.api.table.save( + saveTableRequest({ + schema: { + quantity: { + type: FieldType.NUMBER, + name: "quantity", + }, + price: { + type: FieldType.NUMBER, + name: "price", + }, + }, + }) + ) + + const view = await config.api.viewV2.create({ + tableId: table._id!, + name: generator.guid(), + type: ViewV2Type.CALCULATION, + schema: { + quantity: { visible: true }, + sum: { + visible: true, + calculationType: CalculationType.SUM, + field: "price", + }, + }, + }) + + await config.api.row.bulkImport(table._id!, { + rows: [ + { + quantity: 1, + price: 1, + }, + { + quantity: 1, + price: 2, + }, + { + quantity: 2, + price: 10, + }, + ], + }) + + const { rows } = await config.api.viewV2.search(view.id, { + query: { + equal: { + quantity: 1, + }, + }, + }) + + expect(rows).toHaveLength(1) + expect(rows[0].sum).toEqual(3) + }) }) !isLucene && diff --git a/packages/server/src/api/routes/utils/validators.ts b/packages/server/src/api/routes/utils/validators.ts index 7b13beae9f..978243c55a 100644 --- a/packages/server/src/api/routes/utils/validators.ts +++ b/packages/server/src/api/routes/utils/validators.ts @@ -20,29 +20,28 @@ const OPTIONAL_NUMBER = Joi.number().optional().allow(null) const OPTIONAL_BOOLEAN = Joi.boolean().optional().allow(null) const APP_NAME_REGEX = /^[\w\s]+$/ -const validateViewSchemas: CustomValidator = (table, helpers) => { - if (table.views && Object.entries(table.views).length) { - const requiredFields = Object.entries(table.schema) - .filter(([_, v]) => isRequired(v.constraints)) +const validateViewSchemas: CustomValidator
= (table, joiHelpers) => { + if (!table.views || Object.keys(table.views).length === 0) { + return table + } + const required = Object.keys(table.schema).filter(key => + isRequired(table.schema[key].constraints) + ) + if (required.length === 0) { + return table + } + for (const view of Object.values(table.views)) { + if (!sdk.views.isV2(view) || helpers.views.isCalculationView(view)) { + continue + } + const editable = Object.entries(view.schema || {}) + .filter(([_, f]) => f.visible && !f.readonly) .map(([key]) => key) - if (requiredFields.length) { - for (const view of Object.values(table.views)) { - if (!sdk.views.isV2(view)) { - continue - } - - const editableViewFields = Object.entries(view.schema || {}) - .filter(([_, f]) => f.visible && !f.readonly) - .map(([key]) => key) - const missingField = requiredFields.find( - f => !editableViewFields.includes(f) - ) - if (missingField) { - return helpers.message({ - custom: `To make field "${missingField}" required, this field must be present and writable in views: ${view.name}.`, - }) - } - } + const missingField = required.find(f => !editable.includes(f)) + if (missingField) { + return joiHelpers.message({ + custom: `To make field "${missingField}" required, this field must be present and writable in views: ${view.name}.`, + }) } } return table 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/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 } diff --git a/packages/server/src/sdk/app/views/index.ts b/packages/server/src/sdk/app/views/index.ts index 27b3af87f1..c9c9fd780b 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, @@ -94,6 +95,13 @@ async function guardCalculationViewSchema( continue } + if (!schema.field) { + throw new HTTPError( + `Calculation field "${name}" is missing a "field" property`, + 400 + ) + } + const targetSchema = table.schema[schema.field] if (!targetSchema) { throw new HTTPError( @@ -241,12 +249,17 @@ export async function create( await guardViewSchema(tableId, viewRequest) 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 } @@ -368,7 +381,8 @@ export function syncSchema( if (view.schema) { for (const fieldName of Object.keys(view.schema)) { - if (!schema[fieldName]) { + const viewSchema = view.schema[fieldName] + if (!helpers.views.isCalculationField(viewSchema) && !schema[fieldName]) { delete view.schema[fieldName] } } 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)) + }) } 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 {