Merge pull request #13844 from Budibase/BUDI-8282/dont-treat-display-column-as-required

Frontend validation

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
 
@@ -11,7 +12,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"
 
@@ -40,36 +43,50 @@
     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 isRequired = helpers.schema.isRequired(c.schema.constraints)
+    const isDisplayColumn = $stickyColumn === c
 
-  $: options =
-    $datasource.type === "viewV2"
-      ? [EDIT_OPTION, READONLY_OPTION, HIDDEN_OPTION]
-      : [EDIT_OPTION, HIDDEN_OPTION]
+    const requiredTooltip = isRequired && "Required columns must be writable"
+
+    const options = [
+      {
+        icon: "Edit",
+        value: PERMISSION_OPTIONS.WRITABLE,
+        tooltip: requiredTooltip || "Writable",
+        disabled: isRequired,
+      },
+    ]
+    if ($datasource.type === "viewV2") {
+      options.push({
+        icon: "Visibility",
+        value: PERMISSION_OPTIONS.READONLY,
+        tooltip: allowViewReadonlyColumns
+          ? requiredTooltip || "Read only"
+          : "Read only (premium feature)",
+        disabled: !allowViewReadonlyColumns || isRequired,
+      })
+    }
+
+    options.push({
+      icon: "VisibilityOff",
+      value: PERMISSION_OPTIONS.HIDDEN,
+      disabled: isDisplayColumn || isRequired,
+      tooltip:
+        (isDisplayColumn && "Display column cannot be hidden") ||
+        requiredTooltip ||
+        "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
     }
 
@@ -93,19 +110,7 @@
 <Popover bind:open {anchor} align="left">
   <div class="content">
     <div class="columns">
-      {#if $stickyColumn}
-        <div class="column">
-          <Icon size="S" name={getColumnIcon($stickyColumn)} />
-          {$stickyColumn.label}
-        </div>
-
-        <ToggleActionButtonGroup
-          disabled
-          value={PERMISSION_OPTIONS.WRITABLE}
-          options={options.map(o => ({ ...o, disabled: true }))}
-        />
-      {/if}
-      {#each $columns as column}
+      {#each displayColumns as column}
         <div class="column">
           <Icon size="S" name={getColumnIcon(column)} />
           {column.label}
@@ -113,7 +118,7 @@
         <ToggleActionButtonGroup
           on:click={e => toggleColumn(column, e.detail)}
           value={columnToPermissionOptions(column)}
-          {options}
+          options={column.options}
         />
       {/each}
     </div>

packages/server/src/api/routes/tests/viewV2.spec.ts

@@ -134,7 +134,7 @@ describe.each([
       const newView: Required<CreateViewRequest> = {
         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,

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)

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)) {
@@ -37,9 +37,9 @@ export async function getEnriched(viewId: string): Promise<ViewV2Enriched> {
 
 async function guardViewSchema(
   tableId: string,
-  viewSchema?: Record<string, ViewUIFieldMetadata>
+  view: Omit<ViewV2, "id" | "version">
 ) {
-  viewSchema ??= {}
+  const viewSchema = view.schema || {}
   const table = await sdk.tables.getTable(tableId)
 
   for (const field of Object.keys(viewSchema)) {
@@ -69,7 +69,7 @@ async function guardViewSchema(
   }
 
   for (const field of Object.values(table.schema)) {
-    if (!isRequired(field.constraints)) {
+    if (!helpers.schema.isRequired(field.constraints)) {
       continue
     }
 
@@ -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<ViewV2, "id" | "version">
 ): Promise<ViewV2> {
-  await guardViewSchema(tableId, viewRequest.schema)
+  await guardViewSchema(tableId, viewRequest)
 
   return pickApi(tableId).create(tableId, viewRequest)
 }
 
 export async function update(tableId: string, view: ViewV2): Promise<ViewV2> {
-  await guardViewSchema(tableId, view.schema)
+  await guardViewSchema(tableId, view)
 
   return pickApi(tableId).update(tableId, view)
 }

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

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
+}