From b7beec622641ca8729c522c6888e28c2fa70756f Mon Sep 17 00:00:00 2001 From: Adria Navarro Date: Wed, 18 Dec 2024 13:18:13 +0100 Subject: [PATCH 1/4] Add tests --- .../server/src/api/routes/tests/table.spec.ts | 99 +++++++++++++++++++ 1 file changed, 99 insertions(+) diff --git a/packages/server/src/api/routes/tests/table.spec.ts b/packages/server/src/api/routes/tests/table.spec.ts index e47181e21f..2a7f039ff5 100644 --- a/packages/server/src/api/routes/tests/table.spec.ts +++ b/packages/server/src/api/routes/tests/table.spec.ts @@ -185,6 +185,62 @@ if (descriptions.length) { ) } ) + + it("can set primary display", async () => { + const columnName = generator.word() + const table = await config.api.table.save( + tableForDatasource(datasource, { + primaryDisplay: columnName, + schema: { + [columnName]: { + name: columnName, + type: FieldType.STRING, + }, + }, + }) + ) + expect(table.primaryDisplay).toEqual(columnName) + + const res = await config.api.table.get(table._id!) + expect(res.primaryDisplay).toEqual(columnName) + }) + + it("cannot use unexisting columns as primary display", async () => { + const columnName = generator.word() + await config.api.table.save( + tableForDatasource(datasource, { + primaryDisplay: columnName, + }), + { + status: 400, + body: { + message: `Column "${columnName}" cannot be used as a display type.`, + }, + } + ) + }) + + it("cannot use invalid column types as display name", async () => { + const columnName = generator.word() + + await config.api.table.save( + tableForDatasource(datasource, { + primaryDisplay: columnName, + schema: { + [columnName]: { + name: columnName, + type: FieldType.BOOLEAN, + }, + }, + }), + { + status: 400, + body: { + message: `Column "${columnName}" cannot be used as a display type.`, + }, + } + ) + }) }) describe("permissions", () => { @@ -603,6 +659,49 @@ if (descriptions.length) { } expect(response).toEqual(expectedResponse) }) + + it("cannot use unexisting columns as primary display", async () => { + const table = await config.api.table.save( + tableForDatasource(datasource) + ) + + const columnName = generator.word() + const tableRequest = { + ...table, + primaryDisplay: columnName, + } + await config.api.table.save(tableRequest, { + status: 400, + body: { + message: `Column "${columnName}" cannot be used as a display type.`, + }, + }) + }) + + it("cannot use invalid column types as display name", async () => { + const table = await config.api.table.save( + tableForDatasource(datasource) + ) + const columnName = generator.word() + const tableRequest: SaveTableRequest = { + ...table, + primaryDisplay: columnName, + schema: { + ...table.schema, + [columnName]: { + name: columnName, + type: FieldType.BOOLEAN, + }, + }, + } + + await config.api.table.save(tableRequest, { + status: 400, + body: { + message: `Column "${columnName}" cannot be used as a display type.`, + }, + }) + }) }) describe("import", () => { From ad02581e1dd71a9958e81e61c8d31cc02d1af6ed Mon Sep 17 00:00:00 2001 From: Adria Navarro Date: Wed, 18 Dec 2024 13:18:46 +0100 Subject: [PATCH 2/4] Guard primary display --- .../server/src/api/controllers/table/index.ts | 17 ++++++++++++++++- packages/shared-core/src/table.ts | 2 +- 2 files changed, 17 insertions(+), 2 deletions(-) diff --git a/packages/server/src/api/controllers/table/index.ts b/packages/server/src/api/controllers/table/index.ts index 02134f0317..76102b9be4 100644 --- a/packages/server/src/api/controllers/table/index.ts +++ b/packages/server/src/api/controllers/table/index.ts @@ -37,6 +37,7 @@ import { jsonFromCsvString } from "../../../utilities/csv" import { builderSocket } from "../../../websockets" import { cloneDeep } from "lodash" import { + canBeDisplayColumn, helpers, PROTECTED_EXTERNAL_COLUMNS, PROTECTED_INTERNAL_COLUMNS, @@ -67,6 +68,20 @@ function checkDefaultFields(table: Table) { } } +function guardTable(table: Table) { + checkDefaultFields(table) + + if ( + table.primaryDisplay && + !canBeDisplayColumn(table.schema[table.primaryDisplay]?.type) + ) { + throw new HTTPError( + `Column "${table.primaryDisplay}" cannot be used as a display type.`, + 400 + ) + } +} + // covers both internal and external export async function fetch(ctx: UserCtx) { const internal = await sdk.tables.getAllInternalTables() @@ -111,7 +126,7 @@ export async function save(ctx: UserCtx) { const isCreate = !table._id - checkDefaultFields(table) + guardTable(table) let savedTable: Table if (isCreate) { diff --git a/packages/shared-core/src/table.ts b/packages/shared-core/src/table.ts index 5402439d9c..070ee6c760 100644 --- a/packages/shared-core/src/table.ts +++ b/packages/shared-core/src/table.ts @@ -12,8 +12,8 @@ const allowDisplayColumnByType: Record = { [FieldType.AUTO]: true, [FieldType.INTERNAL]: true, [FieldType.BARCODEQR]: true, - [FieldType.BIGINT]: true, + [FieldType.BOOLEAN]: false, [FieldType.ARRAY]: false, [FieldType.ATTACHMENTS]: false, From 59bdae57ccdc5a414c74d62ffdecfe0853f017fe Mon Sep 17 00:00:00 2001 From: Adria Navarro Date: Tue, 17 Dec 2024 16:58:15 +0100 Subject: [PATCH 3/4] Fix test --- packages/server/src/api/routes/tests/search.spec.ts | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/packages/server/src/api/routes/tests/search.spec.ts b/packages/server/src/api/routes/tests/search.spec.ts index e97f48afbe..e94e567b43 100644 --- a/packages/server/src/api/routes/tests/search.spec.ts +++ b/packages/server/src/api/routes/tests/search.spec.ts @@ -3399,7 +3399,7 @@ if (descriptions.length) { type: FieldType.LINK, relationshipType: RelationshipType.MANY_TO_ONE, tableId: toRelateTableId, - fieldName: "link", + fieldName: "main", }, }) @@ -3408,7 +3408,7 @@ if (descriptions.length) { ) await config.api.table.save({ ...toRelateTable, - primaryDisplay: "link", + primaryDisplay: "name", }) const relatedRows = await Promise.all([ config.api.row.save(toRelateTable._id!, { From 3a36289306e65b1e3889b8750adf17e1ca22e478 Mon Sep 17 00:00:00 2001 From: Adria Navarro Date: Wed, 18 Dec 2024 13:32:09 +0100 Subject: [PATCH 4/4] Only throw on creation or primary display edition --- .../server/src/api/controllers/table/index.ts | 19 +++++++++++++------ 1 file changed, 13 insertions(+), 6 deletions(-) diff --git a/packages/server/src/api/controllers/table/index.ts b/packages/server/src/api/controllers/table/index.ts index 76102b9be4..06d44bc41c 100644 --- a/packages/server/src/api/controllers/table/index.ts +++ b/packages/server/src/api/controllers/table/index.ts @@ -68,17 +68,24 @@ function checkDefaultFields(table: Table) { } } -function guardTable(table: Table) { +async function guardTable(table: Table, isCreate: boolean) { checkDefaultFields(table) if ( table.primaryDisplay && !canBeDisplayColumn(table.schema[table.primaryDisplay]?.type) ) { - throw new HTTPError( - `Column "${table.primaryDisplay}" cannot be used as a display type.`, - 400 - ) + // Prevent throwing errors from existing badly configured tables. Only throw for new tables or if this setting is being updated + if ( + isCreate || + (await sdk.tables.getTable(table._id!)).primaryDisplay !== + table.primaryDisplay + ) { + throw new HTTPError( + `Column "${table.primaryDisplay}" cannot be used as a display type.`, + 400 + ) + } } } @@ -126,7 +133,7 @@ export async function save(ctx: UserCtx) { const isCreate = !table._id - guardTable(table) + await guardTable(table, isCreate) let savedTable: Table if (isCreate) {