diff --git a/packages/backend-core/src/middleware/joi-validator.ts b/packages/backend-core/src/middleware/joi-validator.ts index ac8064a512..5047cdbbc1 100644 --- a/packages/backend-core/src/middleware/joi-validator.ts +++ b/packages/backend-core/src/middleware/joi-validator.ts @@ -3,7 +3,8 @@ import { Ctx } from "@budibase/types" function validate( schema: Joi.ObjectSchema | Joi.ArraySchema, - property: string + property: string, + opts: { errorPrefix: string } = { errorPrefix: `Invalid ${property}` } ) { // Return a Koa middleware function return (ctx: Ctx, next: any) => { @@ -29,16 +30,26 @@ function validate( const { error } = schema.validate(params) if (error) { - ctx.throw(400, `Invalid ${property} - ${error.message}`) + let message = error.message + if (opts.errorPrefix) { + message = `Invalid ${property} - ${message}` + } + ctx.throw(400, message) } return next() } } -export function body(schema: Joi.ObjectSchema | Joi.ArraySchema) { - return validate(schema, "body") +export function body( + schema: Joi.ObjectSchema | Joi.ArraySchema, + opts?: { errorPrefix: string } +) { + return validate(schema, "body", opts) } -export function params(schema: Joi.ObjectSchema | Joi.ArraySchema) { - return validate(schema, "params") +export function params( + schema: Joi.ObjectSchema | Joi.ArraySchema, + opts?: { errorPrefix: string } +) { + return validate(schema, "params", opts) } diff --git a/packages/builder/src/components/backend/DataTable/ViewV2DataTable.svelte b/packages/builder/src/components/backend/DataTable/ViewV2DataTable.svelte index 3b628c7b53..646b764a2c 100644 --- a/packages/builder/src/components/backend/DataTable/ViewV2DataTable.svelte +++ b/packages/builder/src/components/backend/DataTable/ViewV2DataTable.svelte @@ -1,6 +1,6 @@ @@ -54,7 +103,7 @@ quiet size="M" on:click={() => (open = !open)} - selected={open || anyHidden} + selected={open || anyRestricted} disabled={!$columns.length} > {text} @@ -64,19 +113,7 @@
- {#if $stickyColumn} -
- - {$stickyColumn.label} -
- - - {/if} - {#each $columns as column} + {#each displayColumns as column}
{column.label} @@ -84,7 +121,7 @@ toggleColumn(column, e.detail)} value={columnToPermissionOptions(column)} - {options} + options={column.options} /> {/each}
diff --git a/packages/frontend-core/src/components/grid/controls/ToggleActionButtonGroup.svelte b/packages/frontend-core/src/components/grid/controls/ToggleActionButtonGroup.svelte index e705b5016d..2e62c593d1 100644 --- a/packages/frontend-core/src/components/grid/controls/ToggleActionButtonGroup.svelte +++ b/packages/frontend-core/src/components/grid/controls/ToggleActionButtonGroup.svelte @@ -7,7 +7,6 @@ export let value export let options - export let disabled
@@ -15,7 +14,7 @@ dispatch("click", option.value)} - {disabled} + disabled={option.disabled} size="S" icon={option.icon} quiet diff --git a/packages/frontend-core/src/components/grid/layout/Grid.svelte b/packages/frontend-core/src/components/grid/layout/Grid.svelte index 0a3075a61f..8a82209162 100644 --- a/packages/frontend-core/src/components/grid/layout/Grid.svelte +++ b/packages/frontend-core/src/components/grid/layout/Grid.svelte @@ -57,6 +57,7 @@ export let buttons = null export let darkMode export let isCloud = null + export let allowViewReadonlyColumns = false // Unique identifier for DOM nodes inside this instance const gridID = `grid-${Math.random().toString().slice(2)}` @@ -153,7 +154,7 @@
- +
diff --git a/packages/frontend-core/src/components/grid/stores/columns.js b/packages/frontend-core/src/components/grid/stores/columns.js index a3281be936..b76dcbfe0e 100644 --- a/packages/frontend-core/src/components/grid/stores/columns.js +++ b/packages/frontend-core/src/components/grid/stores/columns.js @@ -146,6 +146,7 @@ export const initialise = context => { schema: fieldSchema, width: fieldSchema.width || oldColumn?.width || DefaultColumnWidth, visible: fieldSchema.visible ?? true, + readonly: fieldSchema.readonly, order: fieldSchema.order ?? oldColumn?.order, primaryDisplay: field === primaryDisplay, } diff --git a/packages/frontend-core/src/components/grid/stores/datasource.js b/packages/frontend-core/src/components/grid/stores/datasource.js index 1fc973f171..09b8be4868 100644 --- a/packages/frontend-core/src/components/grid/stores/datasource.js +++ b/packages/frontend-core/src/components/grid/stores/datasource.js @@ -204,6 +204,10 @@ export const createActions = context => { ...$definition, schema: newSchema, }) + resetSchemaMutations() + } + + const resetSchemaMutations = () => { schemaMutations.set({}) } @@ -253,6 +257,7 @@ export const createActions = context => { addSchemaMutation, addSchemaMutations, saveSchemaMutations, + resetSchemaMutations, }, }, } diff --git a/packages/server/src/api/routes/tests/viewV2.spec.ts b/packages/server/src/api/routes/tests/viewV2.spec.ts index 8069fadf10..375c4c7c77 100644 --- a/packages/server/src/api/routes/tests/viewV2.spec.ts +++ b/packages/server/src/api/routes/tests/viewV2.spec.ts @@ -22,10 +22,7 @@ import { generator, mocks } from "@budibase/backend-core/tests" 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") +import { db, roles } from "@budibase/backend-core" describe.each([ ["internal", undefined], @@ -120,6 +117,9 @@ describe.each([ const newView: CreateViewRequest = { name: generator.name(), tableId: table._id!, + schema: { + id: { visible: true }, + }, } const res = await config.api.viewV2.create(newView) @@ -134,7 +134,7 @@ describe.each([ const newView: Required = { name: generator.name(), tableId: table._id!, - primaryDisplay: generator.word(), + primaryDisplay: "id", query: [ { operator: SearchFilterOperator.EQUAL, @@ -148,6 +148,7 @@ describe.each([ type: SortType.STRING, }, schema: { + id: { visible: true }, Price: { visible: true, }, @@ -158,6 +159,7 @@ describe.each([ expect(res).toEqual({ ...newView, schema: { + id: { visible: true }, Price: { visible: true, }, @@ -172,6 +174,11 @@ describe.each([ name: generator.name(), tableId: table._id!, schema: { + id: { + name: "id", + type: FieldType.NUMBER, + visible: true, + }, Price: { name: "Price", type: FieldType.NUMBER, @@ -193,6 +200,7 @@ describe.each([ expect(createdView).toEqual({ ...newView, schema: { + id: { visible: true }, Price: { visible: true, order: 1, @@ -209,6 +217,12 @@ describe.each([ name: generator.name(), tableId: table._id!, schema: { + id: { + name: "id", + type: FieldType.AUTO, + autocolumn: true, + visible: true, + }, Price: { name: "Price", type: FieldType.NUMBER, @@ -230,8 +244,9 @@ describe.each([ const newView: CreateViewRequest = { name: generator.name(), tableId: table._id!, - primaryDisplay: generator.word(), + primaryDisplay: "id", schema: { + id: { visible: true }, Price: { visible: true }, Category: { visible: false }, }, @@ -241,6 +256,7 @@ describe.each([ expect(res).toEqual({ ...newView, schema: { + id: { visible: true }, Price: { visible: true, }, @@ -255,6 +271,7 @@ describe.each([ name: generator.name(), tableId: table._id!, schema: { + id: { visible: true }, nonExisting: { visible: true, }, @@ -293,6 +310,7 @@ describe.each([ name: generator.name(), tableId: table._id!, schema: { + id: { visible: true }, name: { visible: true, readonly: true, @@ -306,6 +324,7 @@ describe.each([ const res = await config.api.viewV2.create(newView) expect(res.schema).toEqual({ + id: { visible: true }, name: { visible: true, readonly: true, @@ -318,15 +337,13 @@ describe.each([ }) 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, + constraints: { presence: true }, }, description: { name: "description", @@ -340,7 +357,9 @@ describe.each([ name: generator.name(), tableId: table._id!, schema: { + id: { visible: true }, name: { + visible: true, readonly: true, }, }, @@ -350,7 +369,7 @@ describe.each([ status: 400, body: { message: - 'Field "name" cannot be readonly as it is a required field', + 'You can\'t make "name" readonly because it is a required field.', status: 400, }, }) @@ -376,6 +395,7 @@ describe.each([ name: generator.name(), tableId: table._id!, schema: { + id: { visible: true }, name: { visible: false, readonly: true, @@ -414,6 +434,7 @@ describe.each([ name: generator.name(), tableId: table._id!, schema: { + id: { visible: true }, name: { visible: true, readonly: true, @@ -424,12 +445,84 @@ describe.each([ await config.api.viewV2.create(newView, { status: 400, body: { - message: "Readonly fields are not enabled for your tenant", + message: "Readonly fields are not enabled", status: 400, }, }) }) }) + + it("display 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!, + primaryDisplay: "name", + schema: { + id: { visible: true }, + name: { + visible: false, + }, + }, + } + + await config.api.viewV2.create(newView, { + status: 400, + body: { + message: 'You can\'t hide "name" because it is the display column.', + status: 400, + }, + }) + }) + + it("display fields can be readonly", async () => { + mocks.licenses.useViewReadonlyColumns() + 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!, + primaryDisplay: "name", + schema: { + id: { visible: true }, + name: { + visible: true, + readonly: true, + }, + }, + } + + await config.api.viewV2.create(newView, { + status: 201, + }) + }) }) describe("update", () => { @@ -441,6 +534,9 @@ describe.each([ view = await config.api.viewV2.create({ tableId: table._id!, name: generator.guid(), + schema: { + id: { visible: true }, + }, }) }) @@ -475,7 +571,7 @@ describe.each([ id: view.id, tableId, name: view.name, - primaryDisplay: generator.word(), + primaryDisplay: "Price", query: [ { operator: SearchFilterOperator.EQUAL, @@ -489,6 +585,7 @@ describe.each([ type: SortType.STRING, }, schema: { + id: { visible: true }, Category: { visible: false, }, @@ -506,7 +603,7 @@ describe.each([ schema: { ...table.schema, id: expect.objectContaining({ - visible: false, + visible: true, }), Category: expect.objectContaining({ visible: false, @@ -603,6 +700,9 @@ describe.each([ const anotherView = await config.api.viewV2.create({ tableId: table._id!, name: generator.guid(), + schema: { + id: { visible: true }, + }, }) const result = await config .request!.put(`/api/v2/views/${anotherView.id}`) @@ -621,6 +721,7 @@ describe.each([ const updatedView = await config.api.viewV2.update({ ...view, schema: { + ...view.schema, Price: { name: "Price", type: FieldType.NUMBER, @@ -640,6 +741,7 @@ describe.each([ expect(updatedView).toEqual({ ...view, schema: { + id: { visible: true }, Price: { visible: true, order: 1, @@ -656,6 +758,7 @@ describe.each([ { ...view, schema: { + ...view.schema, Price: { name: "Price", type: FieldType.NUMBER, @@ -679,6 +782,7 @@ describe.each([ view = await config.api.viewV2.update({ ...view, schema: { + id: { visible: true }, Price: { visible: true, readonly: true, @@ -690,7 +794,7 @@ describe.each([ await config.api.viewV2.update(view, { status: 400, body: { - message: "Readonly fields are not enabled for your tenant", + message: "Readonly fields are not enabled", }, }) }) @@ -701,6 +805,7 @@ describe.each([ view = await config.api.viewV2.update({ ...view, schema: { + id: { visible: true }, Price: { visible: true, readonly: true, @@ -715,6 +820,7 @@ describe.each([ const res = await config.api.viewV2.update({ ...view, schema: { + id: { visible: true }, Price: { visible: true, readonly: false, @@ -725,6 +831,7 @@ describe.each([ expect.objectContaining({ ...view, schema: { + id: { visible: true }, Price: { visible: true, readonly: false, @@ -733,6 +840,53 @@ describe.each([ }) ) }) + + isInternal && + it("updating schema will only validate modified field", async () => { + let view = await config.api.viewV2.create({ + tableId: table._id!, + name: generator.guid(), + schema: { + id: { visible: true }, + Price: { + visible: true, + }, + Category: { visible: true }, + }, + }) + + // Update the view to an invalid state + const tableToUpdate = await config.api.table.get(table._id!) + ;(tableToUpdate.views![view.name] as ViewV2).schema!.id.visible = false + await db.getDB(config.appId!).put(tableToUpdate) + + view = await config.api.viewV2.get(view.id) + await config.api.viewV2.update({ + ...view, + schema: { + ...view.schema, + Price: { + visible: false, + }, + }, + }) + + expect(await config.api.viewV2.get(view.id)).toEqual( + expect.objectContaining({ + schema: { + id: expect.objectContaining({ + visible: false, + }), + Price: expect.objectContaining({ + visible: false, + }), + Category: expect.objectContaining({ + visible: true, + }), + }, + }) + ) + }) }) describe("delete", () => { @@ -742,6 +896,9 @@ describe.each([ view = await config.api.viewV2.create({ tableId: table._id!, name: generator.guid(), + schema: { + id: { visible: true }, + }, }) }) @@ -764,6 +921,7 @@ describe.each([ name: generator.name(), tableId: table._id!, schema: { + id: { visible: true }, Price: { visible: false }, Category: { visible: true }, }, @@ -786,6 +944,7 @@ describe.each([ name: generator.name(), tableId: table._id!, schema: { + id: { visible: true }, Price: { visible: true, readonly: true }, }, }) @@ -821,6 +980,7 @@ describe.each([ tableId: table._id!, name: generator.guid(), schema: { + id: { visible: true }, Country: { visible: true, }, @@ -855,6 +1015,7 @@ describe.each([ tableId: table._id!, name: generator.guid(), schema: { + id: { visible: true }, two: { visible: true }, }, }) @@ -880,6 +1041,7 @@ describe.each([ tableId: table._id!, name: generator.guid(), schema: { + id: { visible: true }, one: { visible: true, readonly: true }, two: { visible: true }, }, @@ -921,6 +1083,7 @@ describe.each([ tableId: table._id!, name: generator.guid(), schema: { + id: { visible: true }, one: { visible: true, readonly: true }, two: { visible: true }, }, @@ -988,6 +1151,7 @@ describe.each([ rows.map(r => ({ _viewId: view.id, tableId: table._id, + id: r.id, _id: r._id, _rev: r._rev, ...(isInternal @@ -1028,6 +1192,7 @@ describe.each([ }, ], schema: { + id: { visible: true }, two: { visible: true }, }, }) @@ -1039,6 +1204,7 @@ describe.each([ { _viewId: view.id, tableId: table._id, + id: two.id, two: two.two, _id: two._id, _rev: two._rev, @@ -1192,7 +1358,11 @@ describe.each([ describe("sorting", () => { let table: Table - const viewSchema = { age: { visible: true }, name: { visible: true } } + const viewSchema = { + id: { visible: true }, + age: { visible: true }, + name: { visible: true }, + } beforeAll(async () => { table = await config.api.table.save( @@ -1348,4 +1518,123 @@ describe.each([ }) }) }) + + describe("updating table schema", () => { + describe("existing columns changed to required", () => { + beforeEach(async () => { + table = await config.api.table.save( + saveTableRequest({ + schema: { + id: { + name: "id", + type: FieldType.AUTO, + autocolumn: true, + }, + name: { + name: "name", + type: FieldType.STRING, + }, + }, + }) + ) + }) + + it("allows updating when no views constrains the field", async () => { + await config.api.viewV2.create({ + name: "view a", + tableId: table._id!, + schema: { + id: { visible: true }, + name: { visible: true }, + }, + }) + + 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: { allowEmpty: false } }, + }, + }, + }, + { status: 200 } + ) + }) + + it("rejects if field is readonly in any view", async () => { + mocks.licenses.useViewReadonlyColumns() + + await config.api.viewV2.create({ + name: "view a", + tableId: table._id!, + schema: { + id: { visible: true }, + name: { + visible: true, + readonly: true, + }, + }, + }) + + 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 }, + }, + }, + }, + { + status: 400, + body: { + status: 400, + message: + 'To make field "name" required, this field must be present and writable in views: view a.', + }, + } + ) + }) + + it("rejects if field is hidden in any view", async () => { + await config.api.viewV2.create({ + name: "view a", + tableId: table._id!, + schema: { id: { visible: true } }, + }) + + 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 }, + }, + }, + }, + { + status: 400, + body: { + status: 400, + message: + 'To make field "name" required, this field must be present and writable in views: view a.', + }, + } + ) + }) + }) + }) }) diff --git a/packages/server/src/api/routes/utils/validators.ts b/packages/server/src/api/routes/utils/validators.ts index 424d0d6c79..a63b29fe5a 100644 --- a/packages/server/src/api/routes/utils/validators.ts +++ b/packages/server/src/api/routes/utils/validators.ts @@ -1,51 +1,89 @@ import { auth, permissions } from "@budibase/backend-core" import { DataSourceOperation } from "../../../constants" -import { WebhookActionType } from "@budibase/types" -import Joi from "joi" -import { ValidSnippetNameRegex } from "@budibase/shared-core" +import { Table, WebhookActionType } from "@budibase/types" +import Joi, { CustomValidator } from "joi" +import { ValidSnippetNameRegex, helpers } from "@budibase/shared-core" +import sdk from "../../../sdk" + +const { isRequired } = helpers.schema const OPTIONAL_STRING = Joi.string().optional().allow(null).allow("") 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)) + .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}.`, + }) + } + } + } + } + return table +} + export function tableValidator() { - // prettier-ignore - return auth.joiValidator.body(Joi.object({ - _id: OPTIONAL_STRING, - _rev: OPTIONAL_STRING, - type: OPTIONAL_STRING.valid("table", "internal", "external"), - primaryDisplay: OPTIONAL_STRING, - schema: Joi.object().required(), - name: Joi.string().required(), - views: Joi.object(), - rows: Joi.array(), - }).unknown(true)) + return auth.joiValidator.body( + Joi.object({ + _id: OPTIONAL_STRING, + _rev: OPTIONAL_STRING, + type: OPTIONAL_STRING.valid("table", "internal", "external"), + primaryDisplay: OPTIONAL_STRING, + schema: Joi.object().required(), + name: Joi.string().required(), + views: Joi.object(), + rows: Joi.array(), + }) + .custom(validateViewSchemas) + .unknown(true), + { errorPrefix: "" } + ) } export function nameValidator() { - // prettier-ignore - return auth.joiValidator.body(Joi.object({ - name: OPTIONAL_STRING, - })) + return auth.joiValidator.body( + Joi.object({ + name: OPTIONAL_STRING, + }) + ) } export function datasourceValidator() { - // prettier-ignore - return auth.joiValidator.body(Joi.object({ - _id: Joi.string(), - _rev: Joi.string(), - type: OPTIONAL_STRING.allow("datasource_plus"), - relationships: Joi.array().items(Joi.object({ - from: Joi.string().required(), - to: Joi.string().required(), - cardinality: Joi.valid("1:N", "1:1", "N:N").required() - })), - }).unknown(true)) + return auth.joiValidator.body( + Joi.object({ + _id: Joi.string(), + _rev: Joi.string(), + type: OPTIONAL_STRING.allow("datasource_plus"), + relationships: Joi.array().items( + Joi.object({ + from: Joi.string().required(), + to: Joi.string().required(), + cardinality: Joi.valid("1:N", "1:1", "N:N").required(), + }) + ), + }).unknown(true) + ) } function filterObject() { - // prettier-ignore return Joi.object({ string: Joi.object().optional(), fuzzy: Joi.object().optional(), @@ -62,17 +100,20 @@ function filterObject() { } export function internalSearchValidator() { - // prettier-ignore - return auth.joiValidator.body(Joi.object({ - tableId: OPTIONAL_STRING, - query: filterObject(), - limit: OPTIONAL_NUMBER, - sort: OPTIONAL_STRING, - sortOrder: OPTIONAL_STRING, - sortType: OPTIONAL_STRING, - paginate: Joi.boolean(), - bookmark: Joi.alternatives().try(OPTIONAL_STRING, OPTIONAL_NUMBER).optional(), - })) + return auth.joiValidator.body( + Joi.object({ + tableId: OPTIONAL_STRING, + query: filterObject(), + limit: OPTIONAL_NUMBER, + sort: OPTIONAL_STRING, + sortOrder: OPTIONAL_STRING, + sortType: OPTIONAL_STRING, + paginate: Joi.boolean(), + bookmark: Joi.alternatives() + .try(OPTIONAL_STRING, OPTIONAL_NUMBER) + .optional(), + }) + ) } export function externalSearchValidator() { @@ -94,92 +135,110 @@ export function externalSearchValidator() { } export function datasourceQueryValidator() { - // prettier-ignore - return auth.joiValidator.body(Joi.object({ - endpoint: Joi.object({ - datasourceId: Joi.string().required(), - operation: Joi.string().required().valid(...Object.values(DataSourceOperation)), - entityId: Joi.string().required(), - }).required(), - resource: Joi.object({ - fields: Joi.array().items(Joi.string()).optional(), - }).optional(), - body: Joi.object().optional(), - sort: Joi.object().optional(), - filters: filterObject().optional(), - paginate: Joi.object({ - page: Joi.string().alphanum().optional(), - limit: Joi.number().optional(), - }).optional(), - })) + return auth.joiValidator.body( + Joi.object({ + endpoint: Joi.object({ + datasourceId: Joi.string().required(), + operation: Joi.string() + .required() + .valid(...Object.values(DataSourceOperation)), + entityId: Joi.string().required(), + }).required(), + resource: Joi.object({ + fields: Joi.array().items(Joi.string()).optional(), + }).optional(), + body: Joi.object().optional(), + sort: Joi.object().optional(), + filters: filterObject().optional(), + paginate: Joi.object({ + page: Joi.string().alphanum().optional(), + limit: Joi.number().optional(), + }).optional(), + }) + ) } export function webhookValidator() { - // prettier-ignore - return auth.joiValidator.body(Joi.object({ - live: Joi.bool(), - _id: OPTIONAL_STRING, - _rev: OPTIONAL_STRING, - name: Joi.string().required(), - bodySchema: Joi.object().optional(), - action: Joi.object({ - type: Joi.string().required().valid(WebhookActionType.AUTOMATION), - target: Joi.string().required(), - }).required(), - }).unknown(true)) + return auth.joiValidator.body( + Joi.object({ + live: Joi.bool(), + _id: OPTIONAL_STRING, + _rev: OPTIONAL_STRING, + name: Joi.string().required(), + bodySchema: Joi.object().optional(), + action: Joi.object({ + type: Joi.string().required().valid(WebhookActionType.AUTOMATION), + target: Joi.string().required(), + }).required(), + }).unknown(true) + ) } export function roleValidator() { const permLevelArray = Object.values(permissions.PermissionLevel) - // prettier-ignore - return auth.joiValidator.body(Joi.object({ - _id: OPTIONAL_STRING, - _rev: OPTIONAL_STRING, - name: Joi.string().regex(/^[a-zA-Z0-9_]*$/).required(), - // this is the base permission ID (for now a built in) - permissionId: Joi.string().valid(...Object.values(permissions.BuiltinPermissionID)).required(), - permissions: Joi.object() - .pattern(/.*/, [Joi.string().valid(...permLevelArray)]) - .optional(), - inherits: OPTIONAL_STRING, - }).unknown(true)) + + return auth.joiValidator.body( + Joi.object({ + _id: OPTIONAL_STRING, + _rev: OPTIONAL_STRING, + name: Joi.string() + .regex(/^[a-zA-Z0-9_]*$/) + .required(), + // this is the base permission ID (for now a built in) + permissionId: Joi.string() + .valid(...Object.values(permissions.BuiltinPermissionID)) + .required(), + permissions: Joi.object() + .pattern(/.*/, [Joi.string().valid(...permLevelArray)]) + .optional(), + inherits: OPTIONAL_STRING, + }).unknown(true) + ) } export function permissionValidator() { const permLevelArray = Object.values(permissions.PermissionLevel) - // prettier-ignore - return auth.joiValidator.params(Joi.object({ - level: Joi.string().valid(...permLevelArray).required(), - resourceId: Joi.string(), - roleId: Joi.string(), - }).unknown(true)) + + return auth.joiValidator.params( + Joi.object({ + level: Joi.string() + .valid(...permLevelArray) + .required(), + resourceId: Joi.string(), + roleId: Joi.string(), + }).unknown(true) + ) } export function screenValidator() { - // prettier-ignore - return auth.joiValidator.body(Joi.object({ - name: Joi.string().required(), - showNavigation: OPTIONAL_BOOLEAN, - width: OPTIONAL_STRING, - routing: Joi.object({ - route: Joi.string().required(), - roleId: Joi.string().required().allow(""), - homeScreen: OPTIONAL_BOOLEAN, - }).required().unknown(true), - props: Joi.object({ - _id: Joi.string().required(), - _component: Joi.string().required(), - _children: Joi.array().required(), - _styles: Joi.object().required(), - type: OPTIONAL_STRING, - table: OPTIONAL_STRING, - layoutId: OPTIONAL_STRING, - }).required().unknown(true), - }).unknown(true)) + return auth.joiValidator.body( + Joi.object({ + name: Joi.string().required(), + showNavigation: OPTIONAL_BOOLEAN, + width: OPTIONAL_STRING, + routing: Joi.object({ + route: Joi.string().required(), + roleId: Joi.string().required().allow(""), + homeScreen: OPTIONAL_BOOLEAN, + }) + .required() + .unknown(true), + props: Joi.object({ + _id: Joi.string().required(), + _component: Joi.string().required(), + _children: Joi.array().required(), + _styles: Joi.object().required(), + type: OPTIONAL_STRING, + table: OPTIONAL_STRING, + layoutId: OPTIONAL_STRING, + }) + .required() + .unknown(true), + }).unknown(true) + ) } function generateStepSchema(allowStepTypes: string[]) { - // prettier-ignore return Joi.object({ stepId: Joi.string().required(), id: Joi.string().required(), @@ -189,33 +248,39 @@ function generateStepSchema(allowStepTypes: string[]) { icon: Joi.string().required(), params: Joi.object(), args: Joi.object(), - type: Joi.string().required().valid(...allowStepTypes), + type: Joi.string() + .required() + .valid(...allowStepTypes), }).unknown(true) } export function automationValidator(existing = false) { - // prettier-ignore - return auth.joiValidator.body(Joi.object({ - _id: existing ? Joi.string().required() : OPTIONAL_STRING, - _rev: existing ? Joi.string().required() : OPTIONAL_STRING, - name: Joi.string().required(), - type: Joi.string().valid("automation").required(), - definition: Joi.object({ - steps: Joi.array().required().items(generateStepSchema(["ACTION", "LOGIC"])), - trigger: generateStepSchema(["TRIGGER"]).allow(null), - }).required().unknown(true), - }).unknown(true)) + return auth.joiValidator.body( + Joi.object({ + _id: existing ? Joi.string().required() : OPTIONAL_STRING, + _rev: existing ? Joi.string().required() : OPTIONAL_STRING, + name: Joi.string().required(), + type: Joi.string().valid("automation").required(), + definition: Joi.object({ + steps: Joi.array() + .required() + .items(generateStepSchema(["ACTION", "LOGIC"])), + trigger: generateStepSchema(["TRIGGER"]).allow(null), + }) + .required() + .unknown(true), + }).unknown(true) + ) } export function applicationValidator(opts = { isCreate: true }) { - // prettier-ignore const base: any = { _id: OPTIONAL_STRING, _rev: OPTIONAL_STRING, url: OPTIONAL_STRING, template: Joi.object({ templateString: OPTIONAL_STRING, - }) + }), } const appNameValidator = Joi.string() diff --git a/packages/server/src/sdk/app/views/index.ts b/packages/server/src/sdk/app/views/index.ts index ea05ecf512..b6ac7b6f6b 100644 --- a/packages/server/src/sdk/app/views/index.ts +++ b/packages/server/src/sdk/app/views/index.ts @@ -8,7 +8,8 @@ import { } from "@budibase/types" import { HTTPError, db as dbCore } from "@budibase/backend-core" import { features } from "@budibase/pro" -import { cloneDeep } from "lodash" +import { helpers } from "@budibase/shared-core" +import { cloneDeep } from "lodash/fp" import * as utils from "../../../db/utils" import { isExternalTableID } from "../../../integrations/utils" @@ -16,7 +17,6 @@ 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)) { @@ -37,11 +37,9 @@ export async function getEnriched(viewId: string): Promise { async function guardViewSchema( tableId: string, - viewSchema?: Record + view: Omit ) { - if (!viewSchema || !Object.keys(viewSchema).length) { - return - } + const viewSchema = view.schema || {} const table = await sdk.tables.getTable(tableId) for (const field of Object.keys(viewSchema)) { @@ -54,18 +52,11 @@ async function guardViewSchema( } 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 ( + !(await features.isViewReadonlyColumnsEnabled()) && + !(tableSchemaField as ViewUIFieldMetadata).readonly + ) { + throw new HTTPError(`Readonly fields are not enabled`, 400) } if (!viewSchema[field].visible) { @@ -76,19 +67,61 @@ async function guardViewSchema( } } } + + const existingView = + table?.views && (table.views[view.name] as ViewV2 | undefined) + + for (const field of Object.values(table.schema)) { + if (!helpers.schema.isRequired(field.constraints)) { + continue + } + + const viewSchemaField = viewSchema[field.name] + const existingViewSchema = + existingView?.schema && existingView.schema[field.name] + if (!viewSchemaField && !existingViewSchema?.visible) { + // Supporting existing configs with required columns but hidden in views + continue + } + + if (!viewSchemaField?.visible) { + throw new HTTPError( + `You can't hide "${field.name}" because it is a required field.`, + 400 + ) + } + + if (viewSchemaField.readonly) { + throw new HTTPError( + `You can't make "${field.name}" readonly because it is a required field.`, + 400 + ) + } + } + + if (view.primaryDisplay) { + const viewSchemaField = viewSchema[view.primaryDisplay] + + if (!viewSchemaField?.visible) { + throw new HTTPError( + `You can't hide "${view.primaryDisplay}" because it is the display column.`, + 400 + ) + } + } } export async function create( tableId: string, viewRequest: Omit ): Promise { - await guardViewSchema(tableId, viewRequest.schema) + await guardViewSchema(tableId, viewRequest) return pickApi(tableId).create(tableId, viewRequest) } export async function update(tableId: string, view: ViewV2): Promise { - await guardViewSchema(tableId, view.schema) + await guardViewSchema(tableId, view) return pickApi(tableId).update(tableId, view) } diff --git a/packages/server/src/utilities/schema.ts b/packages/server/src/utilities/schema.ts index 7fea417d5a..8e6cd34c7c 100644 --- a/packages/server/src/utilities/schema.ts +++ b/packages/server/src/utilities/schema.ts @@ -4,9 +4,8 @@ import { TableSchema, FieldSchema, Row, - FieldConstraints, } from "@budibase/types" -import { ValidColumnNameRegex, utils } from "@budibase/shared-core" +import { ValidColumnNameRegex, helpers, utils } from "@budibase/shared-core" import { db } from "@budibase/backend-core" import { parseCsvExport } from "../api/controllers/view/exporters" @@ -41,15 +40,6 @@ 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: {}, @@ -109,7 +99,7 @@ export function validate(rows: Rows, schema: TableSchema): ValidationResults { columnData, columnType, columnSubtype, - isRequired(constraints) + helpers.schema.isRequired(constraints) ) ) { results.schemaValidation[columnName] = false diff --git a/packages/shared-core/src/helpers/schema.ts b/packages/shared-core/src/helpers/schema.ts index ad4c237247..caf562a8cb 100644 --- a/packages/shared-core/src/helpers/schema.ts +++ b/packages/shared-core/src/helpers/schema.ts @@ -1,5 +1,6 @@ import { BBReferenceFieldSubType, + FieldConstraints, FieldSchema, FieldType, } from "@budibase/types" @@ -16,3 +17,12 @@ export function isDeprecatedSingleUserColumn( schema.constraints?.type !== "array" return result } + +export function isRequired(constraints: FieldConstraints | undefined) { + const isRequired = + !!constraints && + ((typeof constraints.presence !== "boolean" && + constraints.presence?.allowEmpty === false) || + constraints.presence === true) + return isRequired +} diff --git a/packages/server/src/utilities/tests/schema.spec.ts b/packages/shared-core/src/helpers/tests/schema.spec.ts similarity index 100% rename from packages/server/src/utilities/tests/schema.spec.ts rename to packages/shared-core/src/helpers/tests/schema.spec.ts