From 1e492c7a1d50f2097189c37f03571cefb3b86a64 Mon Sep 17 00:00:00 2001 From: Andrew Kingston Date: Tue, 18 Jun 2024 09:12:37 +0100 Subject: [PATCH 1/5] Shrink grid padding to least possible while allowing space for required controls --- .../src/components/grid/layout/Grid.svelte | 4 +- .../src/components/grid/layout/NewRow.svelte | 32 +++++++---- .../src/components/grid/lib/constants.js | 11 ++-- .../src/components/grid/stores/scroll.js | 57 +++++++++++-------- 4 files changed, 61 insertions(+), 43 deletions(-) diff --git a/packages/frontend-core/src/components/grid/layout/Grid.svelte b/packages/frontend-core/src/components/grid/layout/Grid.svelte index 8a82209162..a91f0bc605 100644 --- a/packages/frontend-core/src/components/grid/layout/Grid.svelte +++ b/packages/frontend-core/src/components/grid/layout/Grid.svelte @@ -26,7 +26,7 @@ MaxCellRenderOverflow, GutterWidth, DefaultRowHeight, - Padding, + VPadding, SmallRowHeight, ControlsHeight, ScrollBarSize, @@ -119,7 +119,7 @@ // Derive min height and make available in context const minHeight = derived(rowHeight, $height => { const heightForControls = showControls ? ControlsHeight : 0 - return Padding + SmallRowHeight + $height + heightForControls + return VPadding + SmallRowHeight + $height + heightForControls }) context = { ...context, minHeight } diff --git a/packages/frontend-core/src/components/grid/layout/NewRow.svelte b/packages/frontend-core/src/components/grid/layout/NewRow.svelte index 68ace8a5b2..647f7d3898 100644 --- a/packages/frontend-core/src/components/grid/layout/NewRow.svelte +++ b/packages/frontend-core/src/components/grid/layout/NewRow.svelte @@ -31,6 +31,7 @@ filter, inlineFilters, columnRenderMap, + scrollTop, } = getContext("grid") let visible = false @@ -43,6 +44,21 @@ $: $datasource, (visible = false) $: selectedRowCount = Object.values($selectedRows).length $: hasNoRows = !$rows.length + $: renderedRowCount = $renderedRows.length + $: offset = getOffset($hasNextPage, renderedRowCount, $rowHeight, $scrollTop) + + const getOffset = (hasNextPage, rowCount, rowHeight, scrollTop) => { + // If we have a next page of data then we aren't truly at the bottom, so we + // render the add row component at the top + if (hasNextPage) { + return 0 + } + offset = rowCount * rowHeight - (scrollTop % rowHeight) + if (rowCount !== 0) { + offset -= 1 + } + return offset + } const addRow = async () => { // Blur the active cell and tick to let final value updates propagate @@ -85,12 +101,6 @@ return } - // If we have a next page of data then we aren't truly at the bottom, so we - // render the add row component at the top - if ($hasNextPage) { - offset = 0 - } - // If we don't have a next page then we're at the bottom and can scroll to // the max available offset else { @@ -98,10 +108,6 @@ ...state, top: $maxScrollTop, })) - offset = $renderedRows.length * $rowHeight - ($maxScrollTop % $rowHeight) - if ($renderedRows.length !== 0) { - offset -= 1 - } } // Update state and select initial cell @@ -312,8 +318,10 @@ pointer-events: all; z-index: 3; position: absolute; - top: calc(var(--row-height) + var(--offset) + 24px); - left: 18px; + top: calc( + var(--row-height) + var(--offset) + var(--default-row-height) / 2 + ); + left: calc(var(--default-row-height) / 2); } .button-with-keys { display: flex; diff --git a/packages/frontend-core/src/components/grid/lib/constants.js b/packages/frontend-core/src/components/grid/lib/constants.js index 4b5d04894a..6ea7a98178 100644 --- a/packages/frontend-core/src/components/grid/lib/constants.js +++ b/packages/frontend-core/src/components/grid/lib/constants.js @@ -1,12 +1,13 @@ -export const Padding = 100 -export const ScrollBarSize = 8 -export const GutterWidth = 72 -export const DefaultColumnWidth = 200 -export const MinColumnWidth = 80 export const SmallRowHeight = 36 export const MediumRowHeight = 64 export const LargeRowHeight = 92 export const DefaultRowHeight = SmallRowHeight +export const VPadding = SmallRowHeight * 2 +export const HPadding = 40 +export const ScrollBarSize = 8 +export const GutterWidth = 72 +export const DefaultColumnWidth = 200 +export const MinColumnWidth = 80 export const NewRowID = "new" export const BlankRowID = "blank" export const RowPageSize = 100 diff --git a/packages/frontend-core/src/components/grid/stores/scroll.js b/packages/frontend-core/src/components/grid/stores/scroll.js index e7114cd00c..814d4cdc8c 100644 --- a/packages/frontend-core/src/components/grid/stores/scroll.js +++ b/packages/frontend-core/src/components/grid/stores/scroll.js @@ -1,6 +1,12 @@ import { writable, derived, get } from "svelte/store" import { tick } from "svelte" -import { Padding, GutterWidth, FocusedCellMinOffset } from "../lib/constants" +import { + GutterWidth, + FocusedCellMinOffset, + ScrollBarSize, + HPadding, + VPadding, +} from "../lib/constants" import { parseCellID } from "../lib/utils" export const createStores = () => { @@ -34,28 +40,15 @@ export const deriveStores = context => { // Memoize store primitives const stickyColumnWidth = derived(stickyColumn, $col => $col?.width || 0, 0) - // Derive vertical limits - const contentHeight = derived( - [rows, rowHeight], - ([$rows, $rowHeight]) => ($rows.length + 1) * $rowHeight + Padding, - 0 - ) - const maxScrollTop = derived( - [height, contentHeight], - ([$height, $contentHeight]) => Math.max($contentHeight - $height, 0), - 0 - ) - // Derive horizontal limits const contentWidth = derived( [visibleColumns, stickyColumnWidth, buttonColumnWidth], ([$visibleColumns, $stickyColumnWidth, $buttonColumnWidth]) => { - const space = Math.max(Padding, $buttonColumnWidth - 1) - let width = GutterWidth + space + $stickyColumnWidth + let width = GutterWidth + $buttonColumnWidth + $stickyColumnWidth $visibleColumns.forEach(col => { width += col.width }) - return width + return width + HPadding }, 0 ) @@ -71,14 +64,6 @@ export const deriveStores = context => { }, 0 ) - - // Derive whether to show scrollbars or not - const showVScrollbar = derived( - [contentHeight, height], - ([$contentHeight, $height]) => { - return $contentHeight > $height - } - ) const showHScrollbar = derived( [contentWidth, screenWidth], ([$contentWidth, $screenWidth]) => { @@ -86,6 +71,30 @@ export const deriveStores = context => { } ) + // Derive vertical limits + const contentHeight = derived( + [rows, rowHeight, showHScrollbar], + ([$rows, $rowHeight, $showHScrollbar]) => { + let height = ($rows.length + 1) * $rowHeight + VPadding + if ($showHScrollbar) { + height += ScrollBarSize * 2 + } + return height + }, + 0 + ) + const maxScrollTop = derived( + [height, contentHeight], + ([$height, $contentHeight]) => Math.max($contentHeight - $height, 0), + 0 + ) + const showVScrollbar = derived( + [contentHeight, height], + ([$contentHeight, $height]) => { + return $contentHeight > $height + } + ) + return { contentHeight, contentWidth, From 11e75c21992421bd1331f861f77d39bb6d26ef57 Mon Sep 17 00:00:00 2001 From: Andrew Kingston Date: Tue, 18 Jun 2024 10:27:04 +0100 Subject: [PATCH 2/5] Make quiet table even quieter --- .../grid/layout/ButtonColumn.svelte | 18 ++- .../src/components/grid/layout/Grid.svelte | 9 +- .../components/grid/layout/GridBody.svelte | 29 ++--- .../src/components/grid/layout/NewRow.svelte | 56 ++++----- .../grid/layout/StickyColumn.svelte | 114 ++++++++---------- 5 files changed, 121 insertions(+), 105 deletions(-) diff --git a/packages/frontend-core/src/components/grid/layout/ButtonColumn.svelte b/packages/frontend-core/src/components/grid/layout/ButtonColumn.svelte index 20cfdb1ec5..ead2c67787 100644 --- a/packages/frontend-core/src/components/grid/layout/ButtonColumn.svelte +++ b/packages/frontend-core/src/components/grid/layout/ButtonColumn.svelte @@ -3,6 +3,7 @@ import { Button } from "@budibase/bbui" import GridCell from "../cells/GridCell.svelte" import GridScrollWrapper from "./GridScrollWrapper.svelte" + import { BlankRowID } from "../lib/constants" const { renderedRows, @@ -17,6 +18,7 @@ isDragging, buttonColumnWidth, showVScrollbar, + dispatch, } = getContext("grid") let container @@ -89,6 +91,17 @@ {/each} +
($hoveredRowId = BlankRowID)} + on:mouseleave={$isDragging ? null : () => ($hoveredRowId = null)} + > + dispatch("add-row-inline")} + /> +
@@ -129,8 +142,11 @@ align-items: center; gap: 4px; } + .blank :global(.cell:hover) { + cursor: pointer; + } - /* Add left cell border */ + /* Add left cell border to all cells */ .button-column :global(.cell) { border-left: var(--cell-border); } diff --git a/packages/frontend-core/src/components/grid/layout/Grid.svelte b/packages/frontend-core/src/components/grid/layout/Grid.svelte index a91f0bc605..046a3fcea8 100644 --- a/packages/frontend-core/src/components/grid/layout/Grid.svelte +++ b/packages/frontend-core/src/components/grid/layout/Grid.svelte @@ -354,8 +354,13 @@ transition: none; } - /* Overrides */ - .grid.quiet :global(.grid-data-content .row > .cell:not(:last-child)) { + /* Overrides for quiet */ + .grid.quiet :global(.grid-data-content .row .cell:not(:last-child)), + .grid.quiet :global(.sticky-column .row .cell), + .grid.quiet :global(.new-row .cell:not(:last-child)) { border-right: none; } + .grid.quiet :global(.sticky-column:before) { + display: none; + } diff --git a/packages/frontend-core/src/components/grid/layout/GridBody.svelte b/packages/frontend-core/src/components/grid/layout/GridBody.svelte index 8be56674be..cf93f3004e 100644 --- a/packages/frontend-core/src/components/grid/layout/GridBody.svelte +++ b/packages/frontend-core/src/components/grid/layout/GridBody.svelte @@ -2,6 +2,7 @@ import { getContext, onMount } from "svelte" import GridScrollWrapper from "./GridScrollWrapper.svelte" import GridRow from "./GridRow.svelte" + import GridCell from "../cells/GridCell.svelte" import { BlankRowID } from "../lib/constants" import ButtonColumn from "./ButtonColumn.svelte" @@ -46,7 +47,6 @@ -
{#each $renderedRows as row, idx} @@ -54,13 +54,16 @@ {/each} {#if $config.canAddRows}
($hoveredRowId = BlankRowID)} on:mouseleave={$isDragging ? null : () => ($hoveredRowId = null)} - on:click={() => dispatch("add-row-inline")} - /> + > + dispatch("add-row-inline")} + /> +
{/if} {#if $props.buttons?.length} @@ -76,15 +79,13 @@ overflow: hidden; flex: 1 1 auto; } - .blank { - height: var(--row-height); - background: var(--cell-background); - border-bottom: var(--cell-border); - border-right: var(--cell-border); - position: absolute; + .row { + display: flex; + flex-direction: row; + justify-content: flex-start; + align-items: stretch; } - .blank.highlighted { - background: var(--cell-background-hover); + .blank :global(.cell:hover) { cursor: pointer; } diff --git a/packages/frontend-core/src/components/grid/layout/NewRow.svelte b/packages/frontend-core/src/components/grid/layout/NewRow.svelte index 647f7d3898..1da8f7a63e 100644 --- a/packages/frontend-core/src/components/grid/layout/NewRow.svelte +++ b/packages/frontend-core/src/components/grid/layout/NewRow.svelte @@ -177,39 +177,41 @@ {#if visible}
0} style="--offset:{offset}px; --sticky-width:{width}px;" >
- - - {#if isAdding} -
- {/if} - - {#if $stickyColumn} - {@const cellId = getCellID(NewRowID, $stickyColumn.name)} - - {#if $stickyColumn?.schema?.autocolumn} -
Can't edit auto column
- {/if} +
+ + {#if isAdding}
{/if} - - {/if} + + {#if $stickyColumn} + {@const cellId = getCellID(NewRowID, $stickyColumn.name)} + + {#if $stickyColumn?.schema?.autocolumn} +
Can't edit auto column
+ {/if} + {#if isAdding} +
+ {/if} + + {/if} +
@@ -276,7 +278,7 @@ margin-left: -6px; } - .container { + .new-row { position: absolute; top: var(--default-row-height); left: 0; @@ -286,10 +288,10 @@ flex-direction: row; align-items: stretch; } - .container :global(.cell) { + .new-row :global(.cell) { --cell-background: var(--spectrum-global-color-gray-75) !important; } - .container.floating :global(.cell) { + .new-row.floating :global(.cell) { height: calc(var(--row-height) + 1px); border-top: var(--cell-border); } diff --git a/packages/frontend-core/src/components/grid/layout/StickyColumn.svelte b/packages/frontend-core/src/components/grid/layout/StickyColumn.svelte index b57c89ee4f..85c1eb2897 100644 --- a/packages/frontend-core/src/components/grid/layout/StickyColumn.svelte +++ b/packages/frontend-core/src/components/grid/layout/StickyColumn.svelte @@ -66,62 +66,58 @@ -
- - {#each $renderedRows as row, idx} - {@const rowSelected = !!$selectedRows[row._id]} - {@const rowHovered = $hoveredRowId === row._id} - {@const rowFocused = $focusedRow?._id === row._id} - {@const cellId = getCellID(row._id, $stickyColumn?.name)} -
($hoveredRowId = row._id)} - on:mouseleave={$isDragging ? null : () => ($hoveredRowId = null)} - on:click={() => dispatch("rowclick", rows.actions.cleanRow(row))} - > - - {#if $stickyColumn} - - {/if} -
- {/each} - {#if $config.canAddRows} -
($hoveredRowId = BlankRowID)} - on:mouseleave={$isDragging ? null : () => ($hoveredRowId = null)} - on:click={() => dispatch("add-row-inline")} - > - - - - {#if $stickyColumn} - - - - {/if} -
- {/if} - -
+ + {#each $renderedRows as row, idx} + {@const rowSelected = !!$selectedRows[row._id]} + {@const rowHovered = $hoveredRowId === row._id} + {@const rowFocused = $focusedRow?._id === row._id} + {@const cellId = getCellID(row._id, $stickyColumn?.name)} +
($hoveredRowId = row._id)} + on:mouseleave={$isDragging ? null : () => ($hoveredRowId = null)} + on:click={() => dispatch("rowclick", rows.actions.cleanRow(row))} + > + + {#if $stickyColumn} + + {/if} +
+ {/each} + {#if $config.canAddRows} +
($hoveredRowId = BlankRowID)} + on:mouseleave={$isDragging ? null : () => ($hoveredRowId = null)} + on:click={() => dispatch("add-row-inline")} + > + + + + {#if $stickyColumn} + + + + {/if} +
+ {/if} +
From a74dde5985cfe69b59e41405b1ef025ba93a7c99 Mon Sep 17 00:00:00 2001 From: Andrew Kingston Date: Tue, 18 Jun 2024 10:38:50 +0100 Subject: [PATCH 3/5] Fix small border issue --- .../frontend-core/src/components/grid/layout/Grid.svelte | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/packages/frontend-core/src/components/grid/layout/Grid.svelte b/packages/frontend-core/src/components/grid/layout/Grid.svelte index 046a3fcea8..8ea9e2264d 100644 --- a/packages/frontend-core/src/components/grid/layout/Grid.svelte +++ b/packages/frontend-core/src/components/grid/layout/Grid.svelte @@ -355,9 +355,9 @@ } /* Overrides for quiet */ - .grid.quiet :global(.grid-data-content .row .cell:not(:last-child)), - .grid.quiet :global(.sticky-column .row .cell), - .grid.quiet :global(.new-row .cell:not(:last-child)) { + .grid.quiet :global(.grid-data-content .row > .cell:not(:last-child)), + .grid.quiet :global(.sticky-column .row > .cell), + .grid.quiet :global(.new-row .row > .cell:not(:last-child)) { border-right: none; } .grid.quiet :global(.sticky-column:before) { From cfeab17ed89f5be923397a0aa08c3cedc6b039da Mon Sep 17 00:00:00 2001 From: Michael Drury Date: Wed, 26 Jun 2024 12:40:16 +0100 Subject: [PATCH 4/5] Revert "Revert "Disallow prohibited columns"" --- packages/backend-core/src/db/constants.ts | 19 ++++--------- .../DataTable/modals/CreateEditColumn.svelte | 18 ++++++++---- .../server/src/api/routes/tests/table.spec.ts | 28 +++++++++++++++++++ .../src/sdk/app/tables/internal/index.ts | 12 ++++++++ packages/shared-core/src/constants/index.ts | 1 + packages/shared-core/src/constants/rows.ts | 14 ++++++++++ packages/shared-core/src/table.ts | 22 ++++++++++++++- 7 files changed, 94 insertions(+), 20 deletions(-) create mode 100644 packages/shared-core/src/constants/rows.ts diff --git a/packages/backend-core/src/db/constants.ts b/packages/backend-core/src/db/constants.ts index bfa7595d62..69c98fe569 100644 --- a/packages/backend-core/src/db/constants.ts +++ b/packages/backend-core/src/db/constants.ts @@ -1,14 +1,5 @@ -export const CONSTANT_INTERNAL_ROW_COLS = [ - "_id", - "_rev", - "type", - "createdAt", - "updatedAt", - "tableId", -] as const - -export const CONSTANT_EXTERNAL_ROW_COLS = ["_id", "_rev", "tableId"] as const - -export function isInternalColumnName(name: string): boolean { - return (CONSTANT_INTERNAL_ROW_COLS as readonly string[]).includes(name) -} +export { + CONSTANT_INTERNAL_ROW_COLS, + CONSTANT_EXTERNAL_ROW_COLS, + isInternalColumnName, +} from "@budibase/shared-core" diff --git a/packages/builder/src/components/backend/DataTable/modals/CreateEditColumn.svelte b/packages/builder/src/components/backend/DataTable/modals/CreateEditColumn.svelte index 17ecd8f844..d79eedd194 100644 --- a/packages/builder/src/components/backend/DataTable/modals/CreateEditColumn.svelte +++ b/packages/builder/src/components/backend/DataTable/modals/CreateEditColumn.svelte @@ -17,6 +17,8 @@ SWITCHABLE_TYPES, ValidColumnNameRegex, helpers, + CONSTANT_INTERNAL_ROW_COLS, + CONSTANT_EXTERNAL_ROW_COLS, } from "@budibase/shared-core" import { createEventDispatcher, getContext, onMount } from "svelte" import { cloneDeep } from "lodash/fp" @@ -52,7 +54,6 @@ const DATE_TYPE = FieldType.DATETIME const dispatch = createEventDispatcher() - const PROHIBITED_COLUMN_NAMES = ["type", "_id", "_rev", "tableId"] const { dispatch: gridDispatch, rows } = getContext("grid") export let field @@ -487,20 +488,27 @@ }) } const newError = {} + const prohibited = externalTable + ? CONSTANT_EXTERNAL_ROW_COLS + : CONSTANT_INTERNAL_ROW_COLS if (!externalTable && fieldInfo.name?.startsWith("_")) { newError.name = `Column name cannot start with an underscore.` } else if (fieldInfo.name && !fieldInfo.name.match(ValidColumnNameRegex)) { newError.name = `Illegal character; must be alpha-numeric.` - } else if (PROHIBITED_COLUMN_NAMES.some(name => fieldInfo.name === name)) { - newError.name = `${PROHIBITED_COLUMN_NAMES.join( + } else if ( + prohibited.some( + name => fieldInfo?.name?.toLowerCase() === name.toLowerCase() + ) + ) { + newError.name = `${prohibited.join( ", " - )} are not allowed as column names` + )} are not allowed as column names - case insensitive.` } else if (inUse($tables.selected, fieldInfo.name, originalName)) { newError.name = `Column name already in use.` } if (fieldInfo.type === FieldType.AUTO && !fieldInfo.subtype) { - newError.subtype = `Auto Column requires a type` + newError.subtype = `Auto Column requires a type.` } if (fieldInfo.fieldName && fieldInfo.tableId) { diff --git a/packages/server/src/api/routes/tests/table.spec.ts b/packages/server/src/api/routes/tests/table.spec.ts index f23e0de6db..e75e5e23e7 100644 --- a/packages/server/src/api/routes/tests/table.spec.ts +++ b/packages/server/src/api/routes/tests/table.spec.ts @@ -276,6 +276,34 @@ describe.each([ }) }) + isInternal && + it("shouldn't allow duplicate column names", async () => { + const saveTableRequest: SaveTableRequest = { + ...basicTable(), + } + saveTableRequest.schema["Type"] = { + type: FieldType.STRING, + name: "Type", + } + await config.api.table.save(saveTableRequest, { + status: 400, + body: { + message: + 'Column(s) "type" are duplicated - check for other columns with these name (case in-sensitive)', + }, + }) + saveTableRequest.schema.foo = { type: FieldType.STRING, name: "foo" } + saveTableRequest.schema.FOO = { type: FieldType.STRING, name: "FOO" } + + await config.api.table.save(saveTableRequest, { + status: 400, + body: { + message: + 'Column(s) "type, foo" are duplicated - check for other columns with these name (case in-sensitive)', + }, + }) + }) + it("should add a new column for an internal DB table", async () => { const saveTableRequest: SaveTableRequest = { ...basicTable(), diff --git a/packages/server/src/sdk/app/tables/internal/index.ts b/packages/server/src/sdk/app/tables/internal/index.ts index ea40d2bfe9..fc32708708 100644 --- a/packages/server/src/sdk/app/tables/internal/index.ts +++ b/packages/server/src/sdk/app/tables/internal/index.ts @@ -17,6 +17,7 @@ import { cloneDeep } from "lodash/fp" import isEqual from "lodash/isEqual" import { runStaticFormulaChecks } from "../../../../api/controllers/table/bulkFormula" import { context } from "@budibase/backend-core" +import { findDuplicateInternalColumns } from "@budibase/shared-core" import { getTable } from "../getters" import { checkAutoColumns } from "./utils" import * as viewsSdk from "../../views" @@ -44,6 +45,17 @@ export async function save( if (hasTypeChanged(table, oldTable)) { throw new Error("A column type has changed.") } + + // check for case sensitivity - we don't want to allow duplicated columns + const duplicateColumn = findDuplicateInternalColumns(table) + if (duplicateColumn.length) { + throw new Error( + `Column(s) "${duplicateColumn.join( + ", " + )}" are duplicated - check for other columns with these name (case in-sensitive)` + ) + } + // check that subtypes have been maintained table = checkAutoColumns(table, oldTable) diff --git a/packages/shared-core/src/constants/index.ts b/packages/shared-core/src/constants/index.ts index 82ac0d51c3..c9d1a8fc8f 100644 --- a/packages/shared-core/src/constants/index.ts +++ b/packages/shared-core/src/constants/index.ts @@ -1,5 +1,6 @@ export * from "./api" export * from "./fields" +export * from "./rows" export const OperatorOptions = { Equals: { diff --git a/packages/shared-core/src/constants/rows.ts b/packages/shared-core/src/constants/rows.ts new file mode 100644 index 0000000000..bfa7595d62 --- /dev/null +++ b/packages/shared-core/src/constants/rows.ts @@ -0,0 +1,14 @@ +export const CONSTANT_INTERNAL_ROW_COLS = [ + "_id", + "_rev", + "type", + "createdAt", + "updatedAt", + "tableId", +] as const + +export const CONSTANT_EXTERNAL_ROW_COLS = ["_id", "_rev", "tableId"] as const + +export function isInternalColumnName(name: string): boolean { + return (CONSTANT_INTERNAL_ROW_COLS as readonly string[]).includes(name) +} diff --git a/packages/shared-core/src/table.ts b/packages/shared-core/src/table.ts index 7706b78037..4b578a2aef 100644 --- a/packages/shared-core/src/table.ts +++ b/packages/shared-core/src/table.ts @@ -1,4 +1,5 @@ -import { FieldType } from "@budibase/types" +import { FieldType, Table } from "@budibase/types" +import { CONSTANT_INTERNAL_ROW_COLS } from "./constants" const allowDisplayColumnByType: Record = { [FieldType.STRING]: true, @@ -51,3 +52,22 @@ export function canBeDisplayColumn(type: FieldType): boolean { export function canBeSortColumn(type: FieldType): boolean { return !!allowSortColumnByType[type] } + +export function findDuplicateInternalColumns(table: Table): string[] { + // get the column names + const columnNames = Object.keys(table.schema) + .concat(CONSTANT_INTERNAL_ROW_COLS) + .map(colName => colName.toLowerCase()) + // there are duplicates + const set = new Set(columnNames) + let duplicates: string[] = [] + if (set.size !== columnNames.length) { + for (let key of set.keys()) { + const count = columnNames.filter(name => name === key).length + if (count > 1) { + duplicates.push(key) + } + } + } + return duplicates +} From 41f045d8a60fabd6015da7cdcda55925295829a3 Mon Sep 17 00:00:00 2001 From: mike12345567 Date: Wed, 26 Jun 2024 13:36:20 +0100 Subject: [PATCH 5/5] Allow constant internal columns to be duplicated based on being case sensitive. --- .../server/src/api/routes/tests/table.spec.ts | 9 +++------ packages/shared-core/src/table.ts | 17 +++++++++++------ 2 files changed, 14 insertions(+), 12 deletions(-) diff --git a/packages/server/src/api/routes/tests/table.spec.ts b/packages/server/src/api/routes/tests/table.spec.ts index e75e5e23e7..8102966ad1 100644 --- a/packages/server/src/api/routes/tests/table.spec.ts +++ b/packages/server/src/api/routes/tests/table.spec.ts @@ -285,12 +285,9 @@ describe.each([ type: FieldType.STRING, name: "Type", } + // allow the "Type" column - internal columns aren't case sensitive await config.api.table.save(saveTableRequest, { - status: 400, - body: { - message: - 'Column(s) "type" are duplicated - check for other columns with these name (case in-sensitive)', - }, + status: 200, }) saveTableRequest.schema.foo = { type: FieldType.STRING, name: "foo" } saveTableRequest.schema.FOO = { type: FieldType.STRING, name: "FOO" } @@ -299,7 +296,7 @@ describe.each([ status: 400, body: { message: - 'Column(s) "type, foo" are duplicated - check for other columns with these name (case in-sensitive)', + 'Column(s) "foo" are duplicated - check for other columns with these name (case in-sensitive)', }, }) }) diff --git a/packages/shared-core/src/table.ts b/packages/shared-core/src/table.ts index 4b578a2aef..8fd7909b18 100644 --- a/packages/shared-core/src/table.ts +++ b/packages/shared-core/src/table.ts @@ -54,20 +54,25 @@ export function canBeSortColumn(type: FieldType): boolean { } export function findDuplicateInternalColumns(table: Table): string[] { + // maintains the case of keys + const casedKeys = Object.keys(table.schema) // get the column names - const columnNames = Object.keys(table.schema) - .concat(CONSTANT_INTERNAL_ROW_COLS) - .map(colName => colName.toLowerCase()) + const uncasedKeys = casedKeys.map(colName => colName.toLowerCase()) // there are duplicates - const set = new Set(columnNames) + const set = new Set(uncasedKeys) let duplicates: string[] = [] - if (set.size !== columnNames.length) { + if (set.size !== uncasedKeys.length) { for (let key of set.keys()) { - const count = columnNames.filter(name => name === key).length + const count = uncasedKeys.filter(name => name === key).length if (count > 1) { duplicates.push(key) } } } + for (let internalColumn of CONSTANT_INTERNAL_ROW_COLS) { + if (casedKeys.find(key => key === internalColumn)) { + duplicates.push(internalColumn) + } + } return duplicates }