diff --git a/packages/backend-core/tests/core/utilities/mocks/licenses.ts b/packages/backend-core/tests/core/utilities/mocks/licenses.ts index 1cbc282575..ec7d2af794 100644 --- a/packages/backend-core/tests/core/utilities/mocks/licenses.ts +++ b/packages/backend-core/tests/core/utilities/mocks/licenses.ts @@ -106,6 +106,10 @@ export const useViewPermissions = () => { return useFeature(Feature.VIEW_PERMISSIONS) } +export const useViewReadonlyColumns = () => { + return useFeature(Feature.VIEW_READONLY_COLUMNS) +} + // QUOTAS export const setAutomationLogsQuota = (value: number) => { diff --git a/packages/pro b/packages/pro index 1879d8686b..5189b83bea 160000 --- a/packages/pro +++ b/packages/pro @@ -1 +1 @@ -Subproject commit 1879d8686b1d9392707595a02cdd4981923e7f99 +Subproject commit 5189b83bea1868574ff7f4c51fe5db38a11badb8 diff --git a/packages/server/src/api/controllers/view/viewsV2.ts b/packages/server/src/api/controllers/view/viewsV2.ts index eb28883e15..76807b796a 100644 --- a/packages/server/src/api/controllers/view/viewsV2.ts +++ b/packages/server/src/api/controllers/view/viewsV2.ts @@ -3,7 +3,7 @@ import { CreateViewRequest, Ctx, RequiredKeys, - UIFieldMetadata, + ViewUIFieldMetadata, UpdateViewRequest, ViewResponse, ViewResponseEnriched, @@ -18,22 +18,23 @@ async function parseSchema(view: CreateViewRequest) { const finalViewSchema = view.schema && Object.entries(view.schema).reduce((p, [fieldName, schemaValue]) => { - const fieldSchema: RequiredKeys = { + const fieldSchema: RequiredKeys = { order: schemaValue.order, width: schemaValue.width, visible: schemaValue.visible, + readonly: schemaValue.readonly, icon: schemaValue.icon, } Object.entries(fieldSchema) .filter(([, val]) => val === undefined) .forEach(([key]) => { - delete fieldSchema[key as keyof UIFieldMetadata] + delete fieldSchema[key as keyof ViewUIFieldMetadata] }) p[fieldName] = fieldSchema return p - }, {} as Record>) + }, {} as Record>) for (let [key, column] of Object.entries(finalViewSchema)) { - if (!column.visible) { + if (!column.visible && !column.readonly) { delete finalViewSchema[key] } } diff --git a/packages/server/src/api/routes/tests/viewV2.spec.ts b/packages/server/src/api/routes/tests/viewV2.spec.ts index 2ad02a8082..8069fadf10 100644 --- a/packages/server/src/api/routes/tests/viewV2.spec.ts +++ b/packages/server/src/api/routes/tests/viewV2.spec.ts @@ -14,8 +14,8 @@ import { StaticQuotaName, Table, TableSourceType, - UIFieldMetadata, UpdateViewRequest, + ViewUIFieldMetadata, ViewV2, } from "@budibase/types" import { generator, mocks } from "@budibase/backend-core/tests" @@ -23,6 +23,9 @@ import { DatabaseName, getDatasource } from "../../../integrations/tests/utils" import merge from "lodash/merge" import { quotas } from "@budibase/pro" import { roles } from "@budibase/backend-core" +import * as schemaUtils from "../../../utilities/schema" + +jest.mock("../../../utilities/schema") describe.each([ ["internal", undefined], @@ -96,6 +99,10 @@ describe.each([ setup.afterAll() }) + beforeEach(() => { + mocks.licenses.useCloudFree() + }) + const getRowUsage = async () => { const { total } = await config.doInContext(undefined, () => quotas.getCurrentUsageValues(QuotaUsageType.STATIC, StaticQuotaName.ROWS) @@ -141,7 +148,7 @@ describe.each([ type: SortType.STRING, }, schema: { - name: { + Price: { visible: true, }, }, @@ -150,7 +157,11 @@ describe.each([ expect(res).toEqual({ ...newView, - schema: newView.schema, + schema: { + Price: { + visible: true, + }, + }, id: expect.any(String), version: 2, }) @@ -214,6 +225,211 @@ describe.each([ status: 201, }) }) + + it("does not persist non-visible fields", async () => { + const newView: CreateViewRequest = { + name: generator.name(), + tableId: table._id!, + primaryDisplay: generator.word(), + schema: { + Price: { visible: true }, + Category: { visible: false }, + }, + } + const res = await config.api.viewV2.create(newView) + + expect(res).toEqual({ + ...newView, + schema: { + Price: { + visible: true, + }, + }, + id: expect.any(String), + version: 2, + }) + }) + + it("throws bad request when the schema fields are not valid", async () => { + const newView: CreateViewRequest = { + name: generator.name(), + tableId: table._id!, + schema: { + nonExisting: { + visible: true, + }, + }, + } + await config.api.viewV2.create(newView, { + status: 400, + body: { + message: 'Field "nonExisting" is not valid for the requested table', + }, + }) + }) + + describe("readonly fields", () => { + beforeEach(() => { + mocks.licenses.useViewReadonlyColumns() + }) + + it("readonly fields are persisted", async () => { + const table = await config.api.table.save( + saveTableRequest({ + schema: { + name: { + name: "name", + type: FieldType.STRING, + }, + description: { + name: "description", + type: FieldType.STRING, + }, + }, + }) + ) + + const newView: CreateViewRequest = { + name: generator.name(), + tableId: table._id!, + schema: { + name: { + visible: true, + readonly: true, + }, + description: { + visible: true, + readonly: true, + }, + }, + } + + const res = await config.api.viewV2.create(newView) + expect(res.schema).toEqual({ + name: { + visible: true, + readonly: true, + }, + description: { + visible: true, + readonly: true, + }, + }) + }) + + it("required fields cannot be marked as readonly", async () => { + const isRequiredSpy = jest.spyOn(schemaUtils, "isRequired") + isRequiredSpy.mockReturnValueOnce(true) + + const table = await config.api.table.save( + saveTableRequest({ + schema: { + name: { + name: "name", + type: FieldType.STRING, + }, + description: { + name: "description", + type: FieldType.STRING, + }, + }, + }) + ) + + const newView: CreateViewRequest = { + name: generator.name(), + tableId: table._id!, + schema: { + name: { + readonly: true, + }, + }, + } + + await config.api.viewV2.create(newView, { + status: 400, + body: { + message: + 'Field "name" cannot be readonly as it is a required field', + status: 400, + }, + }) + }) + + it("readonly fields must be visible", async () => { + const table = await config.api.table.save( + saveTableRequest({ + schema: { + name: { + name: "name", + type: FieldType.STRING, + }, + description: { + name: "description", + type: FieldType.STRING, + }, + }, + }) + ) + + const newView: CreateViewRequest = { + name: generator.name(), + tableId: table._id!, + schema: { + name: { + visible: false, + readonly: true, + }, + }, + } + + await config.api.viewV2.create(newView, { + status: 400, + body: { + message: + 'Field "name" must be visible if you want to make it readonly', + status: 400, + }, + }) + }) + + it("readonly fields cannot be used on free license", async () => { + mocks.licenses.useCloudFree() + const table = await config.api.table.save( + saveTableRequest({ + schema: { + name: { + name: "name", + type: FieldType.STRING, + }, + description: { + name: "description", + type: FieldType.STRING, + }, + }, + }) + ) + + const newView: CreateViewRequest = { + name: generator.name(), + tableId: table._id!, + schema: { + name: { + visible: true, + readonly: true, + }, + }, + } + + await config.api.viewV2.create(newView, { + status: 400, + body: { + message: "Readonly fields are not enabled for your tenant", + status: 400, + }, + }) + }) + }) }) describe("update", () => { @@ -251,6 +467,7 @@ describe.each([ }) it("can update all fields", async () => { + mocks.licenses.useViewReadonlyColumns() const tableId = table._id! const updatedData: Required = { @@ -275,6 +492,10 @@ describe.each([ Category: { visible: false, }, + Price: { + visible: true, + readonly: true, + }, }, } await config.api.viewV2.update(updatedData) @@ -291,7 +512,8 @@ describe.each([ visible: false, }), Price: expect.objectContaining({ - visible: false, + visible: true, + readonly: true, }), }, }, @@ -450,6 +672,67 @@ describe.each([ } ) }) + + it("cannot update views with readonly on on free license", async () => { + mocks.licenses.useViewReadonlyColumns() + + view = await config.api.viewV2.update({ + ...view, + schema: { + Price: { + visible: true, + readonly: true, + }, + }, + }) + + mocks.licenses.useCloudFree() + await config.api.viewV2.update(view, { + status: 400, + body: { + message: "Readonly fields are not enabled for your tenant", + }, + }) + }) + + it("can remove readonly config after license downgrade", async () => { + mocks.licenses.useViewReadonlyColumns() + + view = await config.api.viewV2.update({ + ...view, + schema: { + Price: { + visible: true, + readonly: true, + }, + Category: { + visible: true, + readonly: true, + }, + }, + }) + mocks.licenses.useCloudFree() + const res = await config.api.viewV2.update({ + ...view, + schema: { + Price: { + visible: true, + readonly: false, + }, + }, + }) + expect(res).toEqual( + expect.objectContaining({ + ...view, + schema: { + Price: { + visible: true, + readonly: false, + }, + }, + }) + ) + }) }) describe("delete", () => { @@ -491,15 +774,35 @@ describe.each([ const updatedTable = await config.api.table.get(table._id!) const viewSchema = updatedTable.views![view!.name!].schema as Record< string, - UIFieldMetadata + ViewUIFieldMetadata > expect(viewSchema.Price?.visible).toEqual(false) + expect(viewSchema.Category?.visible).toEqual(true) + }) + + it("should be able to fetch readonly config after downgrades", async () => { + mocks.licenses.useViewReadonlyColumns() + const res = await config.api.viewV2.create({ + name: generator.name(), + tableId: table._id!, + schema: { + Price: { visible: true, readonly: true }, + }, + }) + + mocks.licenses.useCloudFree() + const view = await config.api.viewV2.get(res.id) + expect(view.schema?.Price).toEqual( + expect.objectContaining({ visible: true, readonly: true }) + ) }) }) describe("read", () => { - it("views have extra data trimmed", async () => { - const table = await config.api.table.save( + let view: ViewV2 + + beforeAll(async () => { + table = await config.api.table.save( saveTableRequest({ schema: { Country: { @@ -514,7 +817,7 @@ describe.each([ }) ) - const view = await config.api.viewV2.create({ + view = await config.api.viewV2.create({ tableId: table._id!, name: generator.guid(), schema: { @@ -523,7 +826,9 @@ describe.each([ }, }, }) + }) + it("views have extra data trimmed", async () => { let row = await config.api.row.save(view.id, { Country: "Aussy", Story: "aaaaa", @@ -568,6 +873,27 @@ describe.each([ expect(row.one).toBeUndefined() expect(row.two).toEqual("bar") }) + + it("can't persist readonly columns", async () => { + mocks.licenses.useViewReadonlyColumns() + const view = await config.api.viewV2.create({ + tableId: table._id!, + name: generator.guid(), + schema: { + one: { visible: true, readonly: true }, + two: { visible: true }, + }, + }) + const row = await config.api.row.save(view.id, { + tableId: table!._id, + _viewId: view.id, + one: "foo", + two: "bar", + }) + + expect(row.one).toBeUndefined() + expect(row.two).toEqual("bar") + }) }) describe("patch", () => { @@ -588,6 +914,33 @@ describe.each([ expect(row.one).toEqual("foo") expect(row.two).toEqual("newBar") }) + + it("can't update readonly columns", async () => { + mocks.licenses.useViewReadonlyColumns() + const view = await config.api.viewV2.create({ + tableId: table._id!, + name: generator.guid(), + schema: { + one: { visible: true, readonly: true }, + two: { visible: true }, + }, + }) + const newRow = await config.api.row.save(table._id!, { + one: "foo", + two: "bar", + }) + await config.api.row.patch(view.id, { + tableId: table._id!, + _id: newRow._id!, + _rev: newRow._rev!, + one: "newFoo", + two: "newBar", + }) + + const row = await config.api.row.get(table._id!, newRow._id!) + expect(row.one).toEqual("foo") + expect(row.two).toEqual("newBar") + }) }) describe("destroy", () => { diff --git a/packages/server/src/middleware/tests/trimViewRowInfo.spec.ts b/packages/server/src/middleware/tests/trimViewRowInfo.spec.ts index 243f9a9329..29d80759c7 100644 --- a/packages/server/src/middleware/tests/trimViewRowInfo.spec.ts +++ b/packages/server/src/middleware/tests/trimViewRowInfo.spec.ts @@ -144,8 +144,12 @@ describe("trimViewRowInfo middleware", () => { name: generator.guid(), tableId: table._id!, schema: { - name: {}, - address: {}, + name: { + visible: true, + }, + address: { + visible: true, + }, }, }) diff --git a/packages/server/src/sdk/app/views/index.ts b/packages/server/src/sdk/app/views/index.ts index 2edfd900c4..ea05ecf512 100644 --- a/packages/server/src/sdk/app/views/index.ts +++ b/packages/server/src/sdk/app/views/index.ts @@ -2,10 +2,12 @@ import { RenameColumn, TableSchema, View, + ViewUIFieldMetadata, ViewV2, ViewV2Enriched, } from "@budibase/types" -import { db as dbCore } from "@budibase/backend-core" +import { HTTPError, db as dbCore } from "@budibase/backend-core" +import { features } from "@budibase/pro" import { cloneDeep } from "lodash" import * as utils from "../../../db/utils" @@ -13,6 +15,8 @@ import { isExternalTableID } from "../../../integrations/utils" import * as internal from "./internal" import * as external from "./external" +import sdk from "../../../sdk" +import { isRequired } from "../../../utilities/schema" function pickApi(tableId: any) { if (isExternalTableID(tableId)) { @@ -31,14 +35,61 @@ export async function getEnriched(viewId: string): Promise { return pickApi(tableId).getEnriched(viewId) } +async function guardViewSchema( + tableId: string, + viewSchema?: Record +) { + if (!viewSchema || !Object.keys(viewSchema).length) { + return + } + const table = await sdk.tables.getTable(tableId) + + for (const field of Object.keys(viewSchema)) { + const tableSchemaField = table.schema[field] + if (!tableSchemaField) { + throw new HTTPError( + `Field "${field}" is not valid for the requested table`, + 400 + ) + } + + if (viewSchema[field].readonly) { + if (!(await features.isViewReadonlyColumnsEnabled())) { + throw new HTTPError( + `Readonly fields are not enabled for your tenant`, + 400 + ) + } + + if (isRequired(tableSchemaField.constraints)) { + throw new HTTPError( + `Field "${field}" cannot be readonly as it is a required field`, + 400 + ) + } + + if (!viewSchema[field].visible) { + throw new HTTPError( + `Field "${field}" must be visible if you want to make it readonly`, + 400 + ) + } + } + } +} + export async function create( tableId: string, viewRequest: Omit ): Promise { + await guardViewSchema(tableId, viewRequest.schema) + return pickApi(tableId).create(tableId, viewRequest) } export async function update(tableId: string, view: ViewV2): Promise { + await guardViewSchema(tableId, view.schema) + return pickApi(tableId).update(tableId, view) } @@ -53,7 +104,13 @@ export async function remove(viewId: string): Promise { export function allowedFields(view: View | ViewV2) { return [ - ...Object.keys(view?.schema || {}), + ...Object.keys(view?.schema || {}).filter(key => { + if (!isV2(view)) { + return true + } + const fieldSchema = view.schema![key] + return fieldSchema.visible && !fieldSchema.readonly + }), ...dbCore.CONSTANT_EXTERNAL_ROW_COLS, ...dbCore.CONSTANT_INTERNAL_ROW_COLS, ] diff --git a/packages/server/src/utilities/schema.ts b/packages/server/src/utilities/schema.ts index f73701fdfd..7fea417d5a 100644 --- a/packages/server/src/utilities/schema.ts +++ b/packages/server/src/utilities/schema.ts @@ -4,6 +4,7 @@ import { TableSchema, FieldSchema, Row, + FieldConstraints, } from "@budibase/types" import { ValidColumnNameRegex, utils } from "@budibase/shared-core" import { db } from "@budibase/backend-core" @@ -40,6 +41,15 @@ export function isRows(rows: any): rows is Rows { return Array.isArray(rows) && rows.every(row => typeof row === "object") } +export function isRequired(constraints: FieldConstraints | undefined) { + const isRequired = + !!constraints && + ((typeof constraints.presence !== "boolean" && + constraints.presence?.allowEmpty === false) || + constraints.presence === true) + return isRequired +} + export function validate(rows: Rows, schema: TableSchema): ValidationResults { const results: ValidationResults = { schemaValidation: {}, @@ -62,12 +72,6 @@ export function validate(rows: Rows, schema: TableSchema): ValidationResults { return } - const isRequired = - !!constraints && - ((typeof constraints.presence !== "boolean" && - !constraints.presence?.allowEmpty) || - constraints.presence === true) - // If the columnType is not a string, then it's not present in the schema, and should be added to the invalid columns array if (typeof columnType !== "string") { results.invalidColumns.push(columnName) @@ -101,7 +105,12 @@ export function validate(rows: Rows, schema: TableSchema): ValidationResults { } else if ( (columnType === FieldType.BB_REFERENCE || columnType === FieldType.BB_REFERENCE_SINGLE) && - !isValidBBReference(columnData, columnType, columnSubtype, isRequired) + !isValidBBReference( + columnData, + columnType, + columnSubtype, + isRequired(constraints) + ) ) { results.schemaValidation[columnName] = false } else { diff --git a/packages/server/src/utilities/tests/schema.spec.ts b/packages/server/src/utilities/tests/schema.spec.ts new file mode 100644 index 0000000000..664e95a42a --- /dev/null +++ b/packages/server/src/utilities/tests/schema.spec.ts @@ -0,0 +1,35 @@ +import { isRequired } from "../schema" + +describe("schema utilities", () => { + describe("isRequired", () => { + it("not required by default", () => { + const result = isRequired(undefined) + expect(result).toBe(false) + }) + + it("required when presence is true", () => { + const result = isRequired({ presence: true }) + expect(result).toBe(true) + }) + + it("not required when presence is false", () => { + const result = isRequired({ presence: false }) + expect(result).toBe(false) + }) + + it("not required when presence is an empty object", () => { + const result = isRequired({ presence: {} }) + expect(result).toBe(false) + }) + + it("not required when allowEmpty is true", () => { + const result = isRequired({ presence: { allowEmpty: true } }) + expect(result).toBe(false) + }) + + it("required when allowEmpty is false", () => { + const result = isRequired({ presence: { allowEmpty: false } }) + expect(result).toBe(true) + }) + }) +}) diff --git a/packages/types/src/api/web/app/table.ts b/packages/types/src/api/web/app/table.ts index c9d63feaab..fc382643f1 100644 --- a/packages/types/src/api/web/app/table.ts +++ b/packages/types/src/api/web/app/table.ts @@ -1,10 +1,5 @@ -import { - Row, - Table, - TableRequest, - View, - ViewV2Enriched, -} from "../../../documents" +import { Row, Table, TableRequest, View } from "../../../documents" +import { ViewV2Enriched } from "../../../sdk" export type TableViewsResponse = { [key: string]: View | ViewV2Enriched } diff --git a/packages/types/src/api/web/app/view.ts b/packages/types/src/api/web/app/view.ts index c00bc0e468..a6be5e2986 100644 --- a/packages/types/src/api/web/app/view.ts +++ b/packages/types/src/api/web/app/view.ts @@ -1,4 +1,5 @@ -import { ViewV2, ViewV2Enriched } from "../../../documents" +import { ViewV2 } from "../../../documents" +import { ViewV2Enriched } from "../../../sdk/view" export interface ViewResponse { data: ViewV2 diff --git a/packages/types/src/documents/app/view.ts b/packages/types/src/documents/app/view.ts index 8a36b96b4e..2572ddba5d 100644 --- a/packages/types/src/documents/app/view.ts +++ b/packages/types/src/documents/app/view.ts @@ -1,5 +1,5 @@ import { SearchFilter, SortOrder, SortType } from "../../api" -import { TableSchema, UIFieldMetadata } from "./table" +import { UIFieldMetadata } from "./table" import { Document } from "../document" import { DBView } from "../../sdk" @@ -33,6 +33,10 @@ export interface View { groupBy?: string } +export type ViewUIFieldMetadata = UIFieldMetadata & { + readonly?: boolean +} + export interface ViewV2 { version: 2 id: string @@ -45,11 +49,7 @@ export interface ViewV2 { order?: SortOrder type?: SortType } - schema?: Record -} - -export interface ViewV2Enriched extends ViewV2 { - schema?: TableSchema + schema?: Record } export type ViewSchema = ViewCountOrSumSchema | ViewStatisticsSchema diff --git a/packages/types/src/sdk/index.ts b/packages/types/src/sdk/index.ts index 36faaae9c3..d87ec58b0c 100644 --- a/packages/types/src/sdk/index.ts +++ b/packages/types/src/sdk/index.ts @@ -21,3 +21,4 @@ export * from "./websocket" export * from "./permissions" export * from "./row" export * from "./vm" +export * from "./view" diff --git a/packages/types/src/sdk/licensing/feature.ts b/packages/types/src/sdk/licensing/feature.ts index a3f149d6da..0d8db8258e 100644 --- a/packages/types/src/sdk/licensing/feature.ts +++ b/packages/types/src/sdk/licensing/feature.ts @@ -14,6 +14,7 @@ export enum Feature { OFFLINE = "offline", EXPANDED_PUBLIC_API = "expandedPublicApi", VIEW_PERMISSIONS = "viewPermissions", + VIEW_READONLY_COLUMNS = "viewReadonlyColumns", } export type PlanFeatures = { [key in PlanType]: Feature[] | undefined } diff --git a/packages/types/src/sdk/view.ts b/packages/types/src/sdk/view.ts new file mode 100644 index 0000000000..cb551dada9 --- /dev/null +++ b/packages/types/src/sdk/view.ts @@ -0,0 +1,5 @@ +import { TableSchema, ViewV2 } from "../documents" + +export interface ViewV2Enriched extends ViewV2 { + schema?: TableSchema +}