From b937d95de2a4556fa10deb11a7311b7bd44f0318 Mon Sep 17 00:00:00 2001 From: Adria Navarro Date: Mon, 3 Jun 2024 16:55:02 +0200 Subject: [PATCH 1/7] Move isRequired to shared-core --- packages/server/src/sdk/app/views/index.ts | 4 ++-- packages/server/src/utilities/schema.ts | 10 ---------- packages/shared-core/src/helpers/schema.ts | 10 ++++++++++ .../src/helpers}/tests/schema.spec.ts | 0 4 files changed, 12 insertions(+), 12 deletions(-) rename packages/{server/src/utilities => shared-core/src/helpers}/tests/schema.spec.ts (100%) diff --git a/packages/server/src/sdk/app/views/index.ts b/packages/server/src/sdk/app/views/index.ts index 18ab94be21..127c955dc8 100644 --- a/packages/server/src/sdk/app/views/index.ts +++ b/packages/server/src/sdk/app/views/index.ts @@ -8,6 +8,7 @@ import { } from "@budibase/types" import { HTTPError, db as dbCore } from "@budibase/backend-core" import { features } from "@budibase/pro" +import { helpers } from "@budibase/shared-core" import { cloneDeep } from "lodash" import * as utils from "../../../db/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)) { @@ -58,7 +58,7 @@ async function guardViewSchema( throw new HTTPError(`Readonly fields are not enabled`, 400) } - if (isRequired(tableSchemaField.constraints)) { + if (helpers.schema.isRequired(tableSchemaField.constraints)) { throw new HTTPError( `Field "${field}" cannot be readonly as it is a required field`, 400 diff --git a/packages/server/src/utilities/schema.ts b/packages/server/src/utilities/schema.ts index 7fea417d5a..b4ea1277d6 100644 --- a/packages/server/src/utilities/schema.ts +++ b/packages/server/src/utilities/schema.ts @@ -4,7 +4,6 @@ import { TableSchema, FieldSchema, Row, - FieldConstraints, } from "@budibase/types" import { ValidColumnNameRegex, utils } from "@budibase/shared-core" import { db } from "@budibase/backend-core" @@ -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: {}, 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 From 42d60ad95b549e6fe9e005dbd584425079da5ef2 Mon Sep 17 00:00:00 2001 From: Adria Navarro Date: Mon, 3 Jun 2024 16:56:12 +0200 Subject: [PATCH 2/7] Fix --- packages/server/src/utilities/schema.ts | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/packages/server/src/utilities/schema.ts b/packages/server/src/utilities/schema.ts index b4ea1277d6..8e6cd34c7c 100644 --- a/packages/server/src/utilities/schema.ts +++ b/packages/server/src/utilities/schema.ts @@ -5,7 +5,7 @@ import { FieldSchema, Row, } 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" @@ -99,7 +99,7 @@ export function validate(rows: Rows, schema: TableSchema): ValidationResults { columnData, columnType, columnSubtype, - isRequired(constraints) + helpers.schema.isRequired(constraints) ) ) { results.schemaValidation[columnName] = false From cb2349fdeff75aec3bc7ef71ed1e5d4cf2c442b3 Mon Sep 17 00:00:00 2001 From: Adria Navarro Date: Mon, 3 Jun 2024 17:04:35 +0200 Subject: [PATCH 3/7] Allow edition display --- .../grid/controls/ColumnsSettingButton.svelte | 76 +++++++++---------- 1 file changed, 37 insertions(+), 39 deletions(-) diff --git a/packages/frontend-core/src/components/grid/controls/ColumnsSettingButton.svelte b/packages/frontend-core/src/components/grid/controls/ColumnsSettingButton.svelte index 3f0e2341be..43efda80fa 100644 --- a/packages/frontend-core/src/components/grid/controls/ColumnsSettingButton.svelte +++ b/packages/frontend-core/src/components/grid/controls/ColumnsSettingButton.svelte @@ -11,7 +11,9 @@ let open = false let anchor - $: restrictedColumns = $columns.filter(col => !col.visible || col.readonly) + $: allColumns = $stickyColumn ? [$stickyColumn, ...$columns] : $columns + + $: restrictedColumns = allColumns.filter(col => !col.visible || col.readonly) $: anyRestricted = restrictedColumns.length $: text = anyRestricted ? `Columns (${anyRestricted} restricted)` : "Columns" @@ -34,36 +36,44 @@ HIDDEN: "hidden", } - const EDIT_OPTION = { - icon: "Edit", - value: PERMISSION_OPTIONS.WRITABLE, - tooltip: "Writable", - } - $: READONLY_OPTION = { - icon: "Visibility", - value: PERMISSION_OPTIONS.READONLY, - tooltip: allowViewReadonlyColumns - ? "Read only" - : "Read only (premium feature)", - disabled: !allowViewReadonlyColumns, - } - const HIDDEN_OPTION = { - icon: "VisibilityOff", - value: PERMISSION_OPTIONS.HIDDEN, - tooltip: "Hidden", - } + $: displayColumns = allColumns.map(c => { + const isDisplayColumn = $stickyColumn === c - $: options = - $datasource.type === "viewV2" - ? [EDIT_OPTION, READONLY_OPTION, HIDDEN_OPTION] - : [EDIT_OPTION, HIDDEN_OPTION] + const options = [ + { + icon: "Edit", + value: PERMISSION_OPTIONS.WRITABLE, + tooltip: "Writable", + }, + ] + if ($datasource.type === "viewV2") { + options.push({ + icon: "Visibility", + value: PERMISSION_OPTIONS.READONLY, + tooltip: allowViewReadonlyColumns + ? "Read only" + : "Read only (premium feature)", + disabled: !allowViewReadonlyColumns, + }) + } + + options.push({ + icon: "VisibilityOff", + value: PERMISSION_OPTIONS.HIDDEN, + tooltip: "Hidden", + disabled: isDisplayColumn, + tooltip: isDisplayColumn && "Display column cannot be hidden", + }) + + return { ...c, options } + }) function columnToPermissionOptions(column) { - if (!column.visible) { + if (!column.schema.visible) { return PERMISSION_OPTIONS.HIDDEN } - if (column.readonly) { + if (column.schema.readonly) { return PERMISSION_OPTIONS.READONLY } @@ -87,19 +97,7 @@
- {#if $stickyColumn} -
- - {$stickyColumn.label} -
- - ({ ...o, disabled: true }))} - /> - {/if} - {#each $columns as column} + {#each displayColumns as column}
{column.label} @@ -107,7 +105,7 @@ toggleColumn(column, e.detail)} value={columnToPermissionOptions(column)} - {options} + options={column.options} /> {/each}
From 10f77c83b6ef08522da14181f1aabda05f213cca Mon Sep 17 00:00:00 2001 From: Adria Navarro Date: Mon, 3 Jun 2024 17:24:30 +0200 Subject: [PATCH 4/7] Don't allow selecting required columns --- .../grid/controls/ColumnsSettingButton.svelte | 15 ++++++++++----- 1 file changed, 10 insertions(+), 5 deletions(-) diff --git a/packages/frontend-core/src/components/grid/controls/ColumnsSettingButton.svelte b/packages/frontend-core/src/components/grid/controls/ColumnsSettingButton.svelte index 43efda80fa..d4ae71280b 100644 --- a/packages/frontend-core/src/components/grid/controls/ColumnsSettingButton.svelte +++ b/packages/frontend-core/src/components/grid/controls/ColumnsSettingButton.svelte @@ -3,6 +3,7 @@ import { ActionButton, Popover, Icon, notifications } from "@budibase/bbui" import { getColumnIcon } from "../lib/utils" import ToggleActionButtonGroup from "./ToggleActionButtonGroup.svelte" + import { helpers } from "@budibase/shared-core" export let allowViewReadonlyColumns = false @@ -37,13 +38,17 @@ } $: displayColumns = allColumns.map(c => { + const isRequired = helpers.schema.isRequired(c.schema.constraints) const isDisplayColumn = $stickyColumn === c + const requiredTooltip = isRequired && "Required columns must be writable" + const options = [ { icon: "Edit", value: PERMISSION_OPTIONS.WRITABLE, - tooltip: "Writable", + tooltip: requiredTooltip || "Writable", + disabled: isRequired, }, ] if ($datasource.type === "viewV2") { @@ -51,17 +56,17 @@ icon: "Visibility", value: PERMISSION_OPTIONS.READONLY, tooltip: allowViewReadonlyColumns - ? "Read only" + ? requiredTooltip || "Read only" : "Read only (premium feature)", - disabled: !allowViewReadonlyColumns, + disabled: !allowViewReadonlyColumns || isRequired, }) } options.push({ icon: "VisibilityOff", value: PERMISSION_OPTIONS.HIDDEN, - tooltip: "Hidden", - disabled: isDisplayColumn, + tooltip: requiredTooltip || "Hidden", + disabled: isDisplayColumn || isRequired, tooltip: isDisplayColumn && "Display column cannot be hidden", }) From b65e9cfc80c08deea471b3cad8e33a1d8508f9d6 Mon Sep 17 00:00:00 2001 From: Adria Navarro Date: Tue, 4 Jun 2024 12:20:19 +0200 Subject: [PATCH 5/7] Lint --- .../components/grid/controls/ColumnsSettingButton.svelte | 6 ++++-- packages/server/src/api/routes/utils/validators.ts | 5 +++-- 2 files changed, 7 insertions(+), 4 deletions(-) diff --git a/packages/frontend-core/src/components/grid/controls/ColumnsSettingButton.svelte b/packages/frontend-core/src/components/grid/controls/ColumnsSettingButton.svelte index 7f54dbf582..9f7ced013d 100644 --- a/packages/frontend-core/src/components/grid/controls/ColumnsSettingButton.svelte +++ b/packages/frontend-core/src/components/grid/controls/ColumnsSettingButton.svelte @@ -71,9 +71,11 @@ options.push({ icon: "VisibilityOff", value: PERMISSION_OPTIONS.HIDDEN, - tooltip: requiredTooltip || "Hidden", disabled: isDisplayColumn || isRequired, - tooltip: isDisplayColumn && "Display column cannot be hidden", + tooltip: + (isDisplayColumn && "Display column cannot be hidden") || + requiredTooltip || + "Hidden", }) return { ...c, options } diff --git a/packages/server/src/api/routes/utils/validators.ts b/packages/server/src/api/routes/utils/validators.ts index e2cc463f38..a63b29fe5a 100644 --- a/packages/server/src/api/routes/utils/validators.ts +++ b/packages/server/src/api/routes/utils/validators.ts @@ -2,10 +2,11 @@ import { auth, permissions } from "@budibase/backend-core" import { DataSourceOperation } from "../../../constants" import { Table, WebhookActionType } from "@budibase/types" import Joi, { CustomValidator } from "joi" -import { ValidSnippetNameRegex } from "@budibase/shared-core" -import { isRequired } from "../../../utilities/schema" +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) From e6e67af2c4f3e151cbe2f1dfd1afd5a7e0acc008 Mon Sep 17 00:00:00 2001 From: Adria Navarro Date: Tue, 4 Jun 2024 12:32:33 +0200 Subject: [PATCH 6/7] Guard display name column --- packages/server/src/sdk/app/views/index.ts | 19 +++++++++++++++---- 1 file changed, 15 insertions(+), 4 deletions(-) diff --git a/packages/server/src/sdk/app/views/index.ts b/packages/server/src/sdk/app/views/index.ts index 07c207e334..b088051773 100644 --- a/packages/server/src/sdk/app/views/index.ts +++ b/packages/server/src/sdk/app/views/index.ts @@ -37,9 +37,9 @@ export async function getEnriched(viewId: string): Promise { async function guardViewSchema( tableId: string, - viewSchema?: Record + view: Omit ) { - viewSchema ??= {} + const viewSchema = view.schema || {} const table = await sdk.tables.getTable(tableId) for (const field of Object.keys(viewSchema)) { @@ -89,19 +89,30 @@ async function guardViewSchema( ) } } + + 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) } From edd9ebc38923d40149eb6554360c60893e0e51d9 Mon Sep 17 00:00:00 2001 From: Adria Navarro Date: Tue, 4 Jun 2024 12:33:48 +0200 Subject: [PATCH 7/7] Tests --- .../src/api/routes/tests/viewV2.spec.ts | 78 ++++++++++++++++++- 1 file changed, 75 insertions(+), 3 deletions(-) diff --git a/packages/server/src/api/routes/tests/viewV2.spec.ts b/packages/server/src/api/routes/tests/viewV2.spec.ts index 650c36794b..c8035bd578 100644 --- a/packages/server/src/api/routes/tests/viewV2.spec.ts +++ b/packages/server/src/api/routes/tests/viewV2.spec.ts @@ -134,7 +134,7 @@ describe.each([ const newView: Required = { name: generator.name(), tableId: table._id!, - primaryDisplay: generator.word(), + primaryDisplay: "id", query: [ { operator: SearchFilterOperator.EQUAL, @@ -244,7 +244,7 @@ describe.each([ const newView: CreateViewRequest = { name: generator.name(), tableId: table._id!, - primaryDisplay: generator.word(), + primaryDisplay: "id", schema: { id: { visible: true }, Price: { visible: true }, @@ -451,6 +451,78 @@ describe.each([ }) }) }) + + 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", () => { @@ -499,7 +571,7 @@ describe.each([ id: view.id, tableId, name: view.name, - primaryDisplay: generator.word(), + primaryDisplay: "Price", query: [ { operator: SearchFilterOperator.EQUAL,