From 0f119092217333c36ee880441c84af029dcd2aaa Mon Sep 17 00:00:00 2001 From: Andrew Kingston Date: Fri, 2 May 2025 11:12:23 +0100 Subject: [PATCH 1/5] Ensure primary display column is always required, regardless of validation rules --- .../src/sdk/app/rows/tests/utils.spec.ts | 27 +++++++++++++++++++ packages/server/src/sdk/app/rows/utils.ts | 8 +++++- 2 files changed, 34 insertions(+), 1 deletion(-) diff --git a/packages/server/src/sdk/app/rows/tests/utils.spec.ts b/packages/server/src/sdk/app/rows/tests/utils.spec.ts index 516334d31d..0ae9e0c079 100644 --- a/packages/server/src/sdk/app/rows/tests/utils.spec.ts +++ b/packages/server/src/sdk/app/rows/tests/utils.spec.ts @@ -375,4 +375,31 @@ describe("validate", () => { }) }) }) + + describe("primary display", () => { + const getTable = (): Table => ({ + type: "table", + _id: generateTableID(), + name: "table", + sourceId: INTERNAL_TABLE_SOURCE_ID, + sourceType: TableSourceType.INTERNAL, + primaryDisplay: "foo", + schema: { + foo: { + name: "foo", + type: FieldType.STRING, + }, + }, + }) + + it("should always require primary display column", async () => { + const row = {} + const table = getTable() + const output = await validate({ source: table, row }) + expect(output.valid).toBe(false) + expect(output.errors).toStrictEqual({ + foo: ["can't be blank"], + }) + }) + }) }) diff --git a/packages/server/src/sdk/app/rows/utils.ts b/packages/server/src/sdk/app/rows/utils.ts index c19654d817..cac44aaac9 100644 --- a/packages/server/src/sdk/app/rows/utils.ts +++ b/packages/server/src/sdk/app/rows/utils.ts @@ -206,8 +206,14 @@ export async function validate({ ] for (let fieldName of Object.keys(table.schema)) { const column = table.schema[fieldName] - const constraints = cloneDeep(column.constraints) const type = column.type + let constraints = cloneDeep(column.constraints) + + // Ensure display column is required + if (table.primaryDisplay === fieldName) { + constraints = { ...constraints, presence: true } + } + // foreign keys are likely to be enriched if (isForeignKey(fieldName, table)) { continue From 6b7cee8d10ccb66114e6fa1f5dd2f4ee285446a5 Mon Sep 17 00:00:00 2001 From: Andrew Kingston Date: Fri, 2 May 2025 11:39:48 +0100 Subject: [PATCH 2/5] Fix some broken row tests which were missing primary display columns --- .../server/src/api/routes/tests/row.spec.ts | 51 +++++++++++++++---- 1 file changed, 42 insertions(+), 9 deletions(-) diff --git a/packages/server/src/api/routes/tests/row.spec.ts b/packages/server/src/api/routes/tests/row.spec.ts index 3fb882ff2f..7f319cc2ec 100644 --- a/packages/server/src/api/routes/tests/row.spec.ts +++ b/packages/server/src/api/routes/tests/row.spec.ts @@ -918,7 +918,9 @@ if (descriptions.length) { describe("get", () => { it("reads an existing row successfully", async () => { - const existing = await config.api.row.save(table._id!, {}) + const existing = await config.api.row.save(table._id!, { + name: "foo", + }) const res = await config.api.row.get(table._id!, existing._id!) @@ -930,7 +932,7 @@ if (descriptions.length) { it("returns 404 when row does not exist", async () => { const table = await config.api.table.save(defaultTable()) - await config.api.row.save(table._id!, {}) + await config.api.row.save(table._id!, { name: "foo" }) await config.api.row.get(table._id!, "1234567", { status: 404, }) @@ -975,7 +977,9 @@ if (descriptions.length) { describe("update", () => { it("updates an existing row successfully", async () => { - const existing = await config.api.row.save(table._id!, {}) + const existing = await config.api.row.save(table._id!, { + name: "foo", + }) await expectRowUsage(0, async () => { const res = await config.api.row.save(table._id!, { @@ -1166,7 +1170,9 @@ if (descriptions.length) { }) it("should update only the fields that are supplied", async () => { - const existing = await config.api.row.save(table._id!, {}) + const existing = await config.api.row.save(table._id!, { + name: "foo", + }) await expectRowUsage(0, async () => { const row = await config.api.row.patch(table._id!, { @@ -1186,6 +1192,22 @@ if (descriptions.length) { }) }) + it("should not require the primary display", async () => { + const existing = await config.api.row.save(table._id!, { + name: "foo", + description: "bar", + }) + await expectRowUsage(0, async () => { + const row = await config.api.row.patch(table._id!, { + _id: existing._id!, + _rev: existing._rev!, + tableId: table._id!, + description: "baz", + }) + expect(row.description).toEqual("baz") + }) + }) + it("should update only the fields that are supplied and emit the correct oldRow", async () => { let beforeRow = await config.api.row.save(table._id!, { name: "test", @@ -1213,7 +1235,9 @@ if (descriptions.length) { }) it("should throw an error when given improper types", async () => { - const existing = await config.api.row.save(table._id!, {}) + const existing = await config.api.row.save(table._id!, { + name: "foo", + }) await expectRowUsage(0, async () => { await config.api.row.patch( @@ -1289,6 +1313,7 @@ if (descriptions.length) { description: "test", }) const { _id } = await config.api.row.save(table._id!, { + name: "test", relationship: [{ _id: row._id }, { _id: row2._id }], }) const relatedRow = await config.api.row.get(table._id!, _id!, { @@ -1440,7 +1465,9 @@ if (descriptions.length) { }) it("should be able to delete a row", async () => { - const createdRow = await config.api.row.save(table._id!, {}) + const createdRow = await config.api.row.save(table._id!, { + name: "foo", + }) await expectRowUsage(isInternal ? -1 : 0, async () => { const res = await config.api.row.bulkDelete(table._id!, { @@ -1451,7 +1478,9 @@ if (descriptions.length) { }) it("should be able to delete a row with ID only", async () => { - const createdRow = await config.api.row.save(table._id!, {}) + const createdRow = await config.api.row.save(table._id!, { + name: "foo", + }) await expectRowUsage(isInternal ? -1 : 0, async () => { const res = await config.api.row.bulkDelete(table._id!, { @@ -1463,8 +1492,12 @@ if (descriptions.length) { }) it("should be able to bulk delete rows, including a row that doesn't exist", async () => { - const createdRow = await config.api.row.save(table._id!, {}) - const createdRow2 = await config.api.row.save(table._id!, {}) + const createdRow = await config.api.row.save(table._id!, { + name: "foo", + }) + const createdRow2 = await config.api.row.save(table._id!, { + name: "bar", + }) const res = await config.api.row.bulkDelete(table._id!, { rows: [createdRow, createdRow2, { _id: "9999999" }], From d09ea59c7220fb5714e84c90db16ab449837006f Mon Sep 17 00:00:00 2001 From: Andrew Kingston Date: Fri, 2 May 2025 11:47:34 +0100 Subject: [PATCH 3/5] Fix more tests --- .../server/src/api/routes/tests/row.spec.ts | 32 ++++++++++++------- 1 file changed, 20 insertions(+), 12 deletions(-) diff --git a/packages/server/src/api/routes/tests/row.spec.ts b/packages/server/src/api/routes/tests/row.spec.ts index 7f319cc2ec..b33924c8a9 100644 --- a/packages/server/src/api/routes/tests/row.spec.ts +++ b/packages/server/src/api/routes/tests/row.spec.ts @@ -960,8 +960,8 @@ if (descriptions.length) { it("fetches all rows for given tableId", async () => { const table = await config.api.table.save(defaultTable()) const rows = await Promise.all([ - config.api.row.save(table._id!, {}), - config.api.row.save(table._id!, {}), + config.api.row.save(table._id!, { name: "foo" }), + config.api.row.save(table._id!, { name: "bar" }), ]) const res = await config.api.row.fetch(table._id!) @@ -1614,8 +1614,8 @@ if (descriptions.length) { }) it("should be able to delete a bulk set of rows", async () => { - const row1 = await config.api.row.save(table._id!, {}) - const row2 = await config.api.row.save(table._id!, {}) + const row1 = await config.api.row.save(table._id!, { name: "foo" }) + const row2 = await config.api.row.save(table._id!, { name: "bar" }) await expectRowUsage(isInternal ? -2 : 0, async () => { const res = await config.api.row.bulkDelete(table._id!, { @@ -1629,9 +1629,9 @@ if (descriptions.length) { it("should be able to delete a variety of row set types", async () => { const [row1, row2, row3] = await Promise.all([ - config.api.row.save(table._id!, {}), - config.api.row.save(table._id!, {}), - config.api.row.save(table._id!, {}), + config.api.row.save(table._id!, { name: "foo" }), + config.api.row.save(table._id!, { name: "bar" }), + config.api.row.save(table._id!, { name: "baz" }), ]) await expectRowUsage(isInternal ? -3 : 0, async () => { @@ -1645,7 +1645,7 @@ if (descriptions.length) { }) it("should accept a valid row object and delete the row", async () => { - const row1 = await config.api.row.save(table._id!, {}) + const row1 = await config.api.row.save(table._id!, { name: "foo" }) await expectRowUsage(isInternal ? -1 : 0, async () => { const res = await config.api.row.delete( @@ -2396,7 +2396,9 @@ if (descriptions.length) { }) it("should allow exporting without filtering", async () => { - const existing = await config.api.row.save(table._id!, {}) + const existing = await config.api.row.save(table._id!, { + name: "foo", + }) const res = await config.api.row.exportRows(table._id!) const results = JSON.parse(res) expect(results.length).toEqual(1) @@ -2406,7 +2408,9 @@ if (descriptions.length) { }) it("should allow exporting only certain columns", async () => { - const existing = await config.api.row.save(table._id!, {}) + const existing = await config.api.row.save(table._id!, { + name: "foo", + }) const res = await config.api.row.exportRows(table._id!, { rows: [existing._id!], columns: ["_id"], @@ -2421,7 +2425,9 @@ if (descriptions.length) { }) it("should handle single quotes in row filtering", async () => { - const existing = await config.api.row.save(table._id!, {}) + const existing = await config.api.row.save(table._id!, { + name: "foo", + }) const res = await config.api.row.exportRows(table._id!, { rows: [`['${existing._id!}']`], }) @@ -2432,7 +2438,9 @@ if (descriptions.length) { }) it("should return an error if no table is found", async () => { - const existing = await config.api.row.save(table._id!, {}) + const existing = await config.api.row.save(table._id!, { + name: "foo", + }) await config.api.row.exportRows( "1234567", { rows: [existing._id!] }, From 4aee34a621e6441590aa0434cda0744129031dba Mon Sep 17 00:00:00 2001 From: Andrew Kingston Date: Fri, 2 May 2025 11:59:10 +0100 Subject: [PATCH 4/5] Fix more tests --- packages/server/src/api/routes/tests/row.spec.ts | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/packages/server/src/api/routes/tests/row.spec.ts b/packages/server/src/api/routes/tests/row.spec.ts index b33924c8a9..829b7f56d2 100644 --- a/packages/server/src/api/routes/tests/row.spec.ts +++ b/packages/server/src/api/routes/tests/row.spec.ts @@ -2380,7 +2380,9 @@ if (descriptions.length) { !isInternal && it("should allow exporting all columns", async () => { - const existing = await config.api.row.save(table._id!, {}) + const existing = await config.api.row.save(table._id!, { + name: "foo", + }) const res = await config.api.row.exportRows(table._id!, { rows: [existing._id!], }) From ab5ed57865eb922e5ad65bdd08fee90ffc65a3bb Mon Sep 17 00:00:00 2001 From: Peter Clement Date: Wed, 7 May 2025 08:26:21 +0100 Subject: [PATCH 5/5] only clear user prompt if the user rejects a suggestion --- .../src/components/common/CodeEditor/AIGen.svelte | 10 ++++++---- 1 file changed, 6 insertions(+), 4 deletions(-) diff --git a/packages/builder/src/components/common/CodeEditor/AIGen.svelte b/packages/builder/src/components/common/CodeEditor/AIGen.svelte index 1e2593bb62..7cdb834847 100644 --- a/packages/builder/src/components/common/CodeEditor/AIGen.svelte +++ b/packages/builder/src/components/common/CodeEditor/AIGen.svelte @@ -78,15 +78,17 @@ prompt: promptText, }) dispatch("reject", { code: previousContents }) - reset() + reset(false) } - function reset() { + function reset(clearPrompt: boolean = true) { + if (clearPrompt) { + promptText = "" + inputValue = "" + } suggestedCode = null previousContents = null - promptText = "" expanded = false - inputValue = "" }