From 9c9b2ff48da936deea204681142bfc4e64dc386c Mon Sep 17 00:00:00 2001 From: Sam Rose Date: Tue, 21 May 2024 14:15:17 +0100 Subject: [PATCH 01/16] Move empty object range tests out of sql.spec.ts. --- .../src/api/routes/tests/search.spec.ts | 14 +++++++ .../server/src/integrations/tests/sql.spec.ts | 38 ------------------- 2 files changed, 14 insertions(+), 38 deletions(-) diff --git a/packages/server/src/api/routes/tests/search.spec.ts b/packages/server/src/api/routes/tests/search.spec.ts index 2ddc6ef1ff..5b5f05bc58 100644 --- a/packages/server/src/api/routes/tests/search.spec.ts +++ b/packages/server/src/api/routes/tests/search.spec.ts @@ -712,6 +712,20 @@ describe.each([ expectQuery({ range: { name: { low: "g", high: "h" } }, }).toFindNothing()) + + !isLucene && + it("ignores low if it's an empty object", () => + expectQuery({ + // @ts-ignore + range: { name: { low: {}, high: "z" } }, + }).toContainExactly([{ name: "foo" }, { name: "bar" }])) + + !isLucene && + it("ignores high if it's an empty object", () => + expectQuery({ + // @ts-ignore + range: { name: { low: "a", high: {} } }, + }).toContainExactly([{ name: "foo" }, { name: "bar" }])) }) describe("empty", () => { diff --git a/packages/server/src/integrations/tests/sql.spec.ts b/packages/server/src/integrations/tests/sql.spec.ts index cf433725ce..377c1d65e0 100644 --- a/packages/server/src/integrations/tests/sql.spec.ts +++ b/packages/server/src/integrations/tests/sql.spec.ts @@ -190,44 +190,6 @@ describe("SQL query builder", () => { }) }) - it("should ignore high range value if it is an empty object", () => { - const query = sql._query( - generateReadJson({ - filters: { - range: { - dob: { - low: "2000-01-01 00:00:00", - high: {}, - }, - }, - }, - }) - ) - expect(query).toEqual({ - bindings: ["2000-01-01 00:00:00", 500], - sql: `select * from (select * from "${TABLE_NAME}" where "${TABLE_NAME}"."dob" >= $1 limit $2) as "${TABLE_NAME}"`, - }) - }) - - it("should ignore low range value if it is an empty object", () => { - const query = sql._query( - generateReadJson({ - filters: { - range: { - dob: { - low: {}, - high: "2010-01-01 00:00:00", - }, - }, - }, - }) - ) - expect(query).toEqual({ - bindings: ["2010-01-01 00:00:00", 500], - sql: `select * from (select * from "${TABLE_NAME}" where "${TABLE_NAME}"."dob" <= $1 limit $2) as "${TABLE_NAME}"`, - }) - }) - it("should lowercase the values for Oracle LIKE statements", () => { let query = new Sql(SqlClient.ORACLE, limit)._query( generateReadJson({ From bc63a119798d03e92e2c39e072483b6f36c9976a Mon Sep 17 00:00:00 2001 From: Sam Rose Date: Tue, 21 May 2024 14:20:05 +0100 Subject: [PATCH 02/16] Move sort stability check to search.spec.ts. --- .../src/api/routes/tests/search.spec.ts | 19 +++++++++++++++++++ .../server/src/integrations/tests/sql.spec.ts | 15 --------------- 2 files changed, 19 insertions(+), 15 deletions(-) diff --git a/packages/server/src/api/routes/tests/search.spec.ts b/packages/server/src/api/routes/tests/search.spec.ts index 5b5f05bc58..876b52c0a6 100644 --- a/packages/server/src/api/routes/tests/search.spec.ts +++ b/packages/server/src/api/routes/tests/search.spec.ts @@ -1309,6 +1309,25 @@ describe.each([ { auto: 2 }, { auto: 1 }, ])) + + // This is important for pagination. The order of results must always + // be stable or pagination will break. We don't want the user to need + // to specify an order for pagination to work. + it("is stable without a sort specified", async () => { + let { rows } = await config.api.row.search(table._id!, { + tableId: table._id!, + query: {}, + }) + + for (let i = 0; i < 10; i++) { + const response = await config.api.row.search(table._id!, { + tableId: table._id!, + limit: 1, + query: {}, + }) + expect(response.rows).toEqual(rows) + } + }) }) // TODO(samwho): fix for SQS diff --git a/packages/server/src/integrations/tests/sql.spec.ts b/packages/server/src/integrations/tests/sql.spec.ts index 377c1d65e0..4fc964b320 100644 --- a/packages/server/src/integrations/tests/sql.spec.ts +++ b/packages/server/src/integrations/tests/sql.spec.ts @@ -235,21 +235,6 @@ describe("SQL query builder", () => { }) }) - it("should sort SQL Server tables by the primary key if no sort data is provided", () => { - let query = new Sql(SqlClient.MS_SQL, limit)._query( - generateReadJson({ - sort: {}, - paginate: { - limit: 10, - }, - }) - ) - expect(query).toEqual({ - bindings: [10], - sql: `select * from (select top (@p0) * from [test] order by [test].[id] asc) as [test]`, - }) - }) - it("should not parse JSON string as Date", () => { let query = new Sql(SqlClient.POSTGRES, limit)._query( generateCreateJson(TABLE_NAME, { From 7f7ed9f0cb25353c9dd28109b364af42525dc12b Mon Sep 17 00:00:00 2001 From: Sam Rose Date: Tue, 21 May 2024 17:38:38 +0100 Subject: [PATCH 03/16] Move more tests out of sql.spec.ts. --- .../server/src/api/routes/tests/row.spec.ts | 19 +++++ .../src/api/routes/tests/search.spec.ts | 69 ++++++++++++++++--- .../server/src/integrations/tests/sql.spec.ts | 53 -------------- .../server/src/tests/utilities/structures.ts | 2 +- .../src/utilities/rowProcessor/index.ts | 6 ++ 5 files changed, 85 insertions(+), 64 deletions(-) diff --git a/packages/server/src/api/routes/tests/row.spec.ts b/packages/server/src/api/routes/tests/row.spec.ts index 934a838e6a..4801ac4c55 100644 --- a/packages/server/src/api/routes/tests/row.spec.ts +++ b/packages/server/src/api/routes/tests/row.spec.ts @@ -485,6 +485,25 @@ describe.each([ ) expect(response.message).toBe("Cannot create new user entry.") }) + + it("should not mis-parse date string out of JSON", async () => { + const table = await config.api.table.save( + saveTableRequest({ + schema: { + name: { + type: FieldType.STRING, + name: "name", + }, + }, + }) + ) + + const row = await config.api.row.save(table._id!, { + name: `{ "foo": "2023-01-26T11:48:57.000Z" }`, + }) + + expect(row.name).toEqual(`{ "foo": "2023-01-26T11:48:57.000Z" }`) + }) }) describe("get", () => { diff --git a/packages/server/src/api/routes/tests/search.spec.ts b/packages/server/src/api/routes/tests/search.spec.ts index 876b52c0a6..d65980a7cb 100644 --- a/packages/server/src/api/routes/tests/search.spec.ts +++ b/packages/server/src/api/routes/tests/search.spec.ts @@ -17,6 +17,7 @@ import { TableSchema, User, Row, + RelationshipType, } from "@budibase/types" import _ from "lodash" import tk from "timekeeper" @@ -73,7 +74,7 @@ describe.each([ }) async function createTable(schema: TableSchema) { - table = await config.api.table.save( + return await config.api.table.save( tableForDatasource(datasource, { schema }) ) } @@ -186,7 +187,7 @@ describe.each([ describe("boolean", () => { beforeAll(async () => { - await createTable({ + table = await createTable({ isTrue: { name: "isTrue", type: FieldType.BOOLEAN }, }) await createRows([{ isTrue: true }, { isTrue: false }]) @@ -316,7 +317,7 @@ describe.each([ }) ) - await createTable({ + table = await createTable({ name: { name: "name", type: FieldType.STRING }, appointment: { name: "appointment", type: FieldType.DATETIME }, single_user: { @@ -592,7 +593,7 @@ describe.each([ describe.each([FieldType.STRING, FieldType.LONGFORM])("%s", () => { beforeAll(async () => { - await createTable({ + table = await createTable({ name: { name: "name", type: FieldType.STRING }, }) await createRows([{ name: "foo" }, { name: "bar" }]) @@ -790,7 +791,7 @@ describe.each([ describe("numbers", () => { beforeAll(async () => { - await createTable({ + table = await createTable({ age: { name: "age", type: FieldType.NUMBER }, }) await createRows([{ age: 1 }, { age: 10 }]) @@ -899,7 +900,7 @@ describe.each([ const JAN_10TH = "2020-01-10T00:00:00.000Z" beforeAll(async () => { - await createTable({ + table = await createTable({ dob: { name: "dob", type: FieldType.DATETIME }, }) @@ -1011,7 +1012,7 @@ describe.each([ describe.each([FieldType.ARRAY, FieldType.OPTIONS])("%s", () => { beforeAll(async () => { - await createTable({ + table = await createTable({ numbers: { name: "numbers", type: FieldType.ARRAY, @@ -1091,7 +1092,7 @@ describe.each([ const BIG = "9223372036854775807" beforeAll(async () => { - await createTable({ + table = await createTable({ num: { name: "num", type: FieldType.BIGINT }, }) await createRows([{ num: SMALL }, { num: MEDIUM }, { num: BIG }]) @@ -1182,7 +1183,7 @@ describe.each([ isInternal && describe("auto", () => { beforeAll(async () => { - await createTable({ + table = await createTable({ auto: { name: "auto", type: FieldType.AUTO, @@ -1366,7 +1367,7 @@ describe.each([ describe("field name 1:name", () => { beforeAll(async () => { - await createTable({ + table = await createTable({ "1:name": { name: "1:name", type: FieldType.STRING }, }) await createRows([{ "1:name": "bar" }, { "1:name": "foo" }]) @@ -1382,4 +1383,52 @@ describe.each([ expectQuery({ equal: { "1:1:name": "none" } }).toFindNothing()) }) }) + + // This will never work for Lucene. + // TODO(samwho): fix for SQS + !isInternal && + describe("relations", () => { + let otherTable: Table + let rows: Row[] + + beforeAll(async () => { + otherTable = await createTable({ + one: { name: "one", type: FieldType.STRING }, + }) + table = await createTable({ + two: { name: "two", type: FieldType.STRING }, + other: { + type: FieldType.LINK, + relationshipType: RelationshipType.ONE_TO_MANY, + name: "other", + fieldName: "other", + tableId: otherTable._id!, + constraints: { + type: "array", + }, + }, + }) + + rows = await Promise.all([ + config.api.row.save(otherTable._id!, { one: "foo" }), + config.api.row.save(otherTable._id!, { one: "bar" }), + ]) + + await Promise.all([ + config.api.row.save(table._id!, { + two: "foo", + other: [rows[0]._id], + }), + config.api.row.save(table._id!, { + two: "bar", + other: [rows[1]._id], + }), + ]) + }) + + it("can search through relations", () => + expectQuery({ + equal: { [`${otherTable.name}.one`]: "foo" }, + }).toContainExactly([{ two: "foo", other: [{ _id: rows[0]._id }] }])) + }) }) diff --git a/packages/server/src/integrations/tests/sql.spec.ts b/packages/server/src/integrations/tests/sql.spec.ts index 4fc964b320..9b84409e92 100644 --- a/packages/server/src/integrations/tests/sql.spec.ts +++ b/packages/server/src/integrations/tests/sql.spec.ts @@ -56,16 +56,6 @@ function generateReadJson({ } } -function generateCreateJson(table = TABLE_NAME, body = {}): QueryJson { - return { - endpoint: endpoint(table, "CREATE"), - meta: { - table: TABLE, - }, - body, - } -} - function generateRelationshipJson(config: { schema?: string } = {}): QueryJson { return { endpoint: { @@ -146,24 +136,6 @@ describe("SQL query builder", () => { sql = new Sql(client, limit) }) - it("should allow filtering on a related field", () => { - const query = sql._query( - generateReadJson({ - filters: { - equal: { - age: 10, - "task.name": "task 1", - }, - }, - }) - ) - // order of bindings changes because relationship filters occur outside inner query - expect(query).toEqual({ - bindings: [10, limit, "task 1"], - sql: `select * from (select * from "${TABLE_NAME}" where "${TABLE_NAME}"."age" = $1 limit $2) as "${TABLE_NAME}" where "task"."name" = $3`, - }) - }) - it("should add the schema to the LEFT JOIN", () => { const query = sql._query(generateRelationshipJson({ schema: "production" })) expect(query).toEqual({ @@ -234,29 +206,4 @@ describe("SQL query builder", () => { sql: `select * from (select * from (select * from "test" where LOWER("test"."name") LIKE :1) where rownum <= :2) "test"`, }) }) - - it("should not parse JSON string as Date", () => { - let query = new Sql(SqlClient.POSTGRES, limit)._query( - generateCreateJson(TABLE_NAME, { - name: '{ "created_at":"2023-09-09T03:21:06.024Z" }', - }) - ) - expect(query).toEqual({ - bindings: ['{ "created_at":"2023-09-09T03:21:06.024Z" }'], - sql: `insert into "test" ("name") values ($1) returning *`, - }) - }) - - it("should parse and trim valid string as Date", () => { - const dateObj = new Date("2023-09-09T03:21:06.024Z") - let query = new Sql(SqlClient.POSTGRES, limit)._query( - generateCreateJson(TABLE_NAME, { - name: " 2023-09-09T03:21:06.024Z ", - }) - ) - expect(query).toEqual({ - bindings: [dateObj], - sql: `insert into "test" ("name") values ($1) returning *`, - }) - }) }) diff --git a/packages/server/src/tests/utilities/structures.ts b/packages/server/src/tests/utilities/structures.ts index 77a6431335..7213cc66f1 100644 --- a/packages/server/src/tests/utilities/structures.ts +++ b/packages/server/src/tests/utilities/structures.ts @@ -37,7 +37,7 @@ export function tableForDatasource( ): Table { return merge( { - name: generator.guid(), + name: generator.guid().substring(0, 10), type: "table", sourceType: datasource ? TableSourceType.EXTERNAL diff --git a/packages/server/src/utilities/rowProcessor/index.ts b/packages/server/src/utilities/rowProcessor/index.ts index 73176af6d8..4d91759be5 100644 --- a/packages/server/src/utilities/rowProcessor/index.ts +++ b/packages/server/src/utilities/rowProcessor/index.ts @@ -150,6 +150,12 @@ export async function inputProcessing( clonedRow[key] = coerce(value, field.type) } + if (field.type === FieldType.DATETIME) { + if (typeof clonedRow[key] === "string") { + clonedRow[key] = clonedRow[key].trim() + } + } + // remove any attachment urls, they are generated on read if (field.type === FieldType.ATTACHMENTS) { const attachments = clonedRow[key] From 0561ca6e1b367729921328af5c6e4a8ad20c4f41 Mon Sep 17 00:00:00 2001 From: Sam Rose Date: Tue, 21 May 2024 17:39:33 +0100 Subject: [PATCH 04/16] Remove date leniency. --- packages/server/src/utilities/rowProcessor/index.ts | 6 ------ 1 file changed, 6 deletions(-) diff --git a/packages/server/src/utilities/rowProcessor/index.ts b/packages/server/src/utilities/rowProcessor/index.ts index 4d91759be5..73176af6d8 100644 --- a/packages/server/src/utilities/rowProcessor/index.ts +++ b/packages/server/src/utilities/rowProcessor/index.ts @@ -150,12 +150,6 @@ export async function inputProcessing( clonedRow[key] = coerce(value, field.type) } - if (field.type === FieldType.DATETIME) { - if (typeof clonedRow[key] === "string") { - clonedRow[key] = clonedRow[key].trim() - } - } - // remove any attachment urls, they are generated on read if (field.type === FieldType.ATTACHMENTS) { const attachments = clonedRow[key] From a5c5e2ffcdfcd9ecc13f15e0df1ef5b986ed662e Mon Sep 17 00:00:00 2001 From: mike12345567 Date: Tue, 21 May 2024 18:39:46 +0100 Subject: [PATCH 05/16] Fixing issue discovered by test with 1: syntax being required for relationship based filters. --- packages/server/src/sdk/app/rows/search/sqs.ts | 16 +++++++--------- 1 file changed, 7 insertions(+), 9 deletions(-) diff --git a/packages/server/src/sdk/app/rows/search/sqs.ts b/packages/server/src/sdk/app/rows/search/sqs.ts index c4dc408cac..ec3ac08c2e 100644 --- a/packages/server/src/sdk/app/rows/search/sqs.ts +++ b/packages/server/src/sdk/app/rows/search/sqs.ts @@ -55,8 +55,8 @@ function buildInternalFieldList( return fieldList } -function tableInFilter(name: string) { - return `:${name}.` +function tableNameInFieldRegex(tableName: string) { + return new RegExp(`^${tableName}.|:${tableName}.`, "g") } function cleanupFilters(filters: SearchFilters, tables: Table[]) { @@ -72,15 +72,13 @@ function cleanupFilters(filters: SearchFilters, tables: Table[]) { // relationship, switch to table ID const tableRelated = tables.find( table => - table.originalName && key.includes(tableInFilter(table.originalName)) + table.originalName && + key.match(tableNameInFieldRegex(table.originalName)) ) if (tableRelated && tableRelated.originalName) { - filter[ - key.replace( - tableInFilter(tableRelated.originalName), - tableInFilter(tableRelated._id!) - ) - ] = filter[key] + // only replace the first, not replaceAll + filter[key.replace(tableRelated.originalName, tableRelated._id!)] = + filter[key] delete filter[key] } } From c726dfbedbd27a4ae71c5f3f9472612315264fcb Mon Sep 17 00:00:00 2001 From: Andrew Kingston Date: Wed, 22 May 2024 09:22:06 +0100 Subject: [PATCH 06/16] Add link in navigation when duplicating screens --- .../_components/ScreenList/DropdownMenu.svelte | 10 +++++++++- 1 file changed, 9 insertions(+), 1 deletion(-) diff --git a/packages/builder/src/pages/builder/app/[application]/design/[screenId]/_components/ScreenList/DropdownMenu.svelte b/packages/builder/src/pages/builder/app/[application]/design/[screenId]/_components/ScreenList/DropdownMenu.svelte index 0d96dcda93..7421bfa68a 100644 --- a/packages/builder/src/pages/builder/app/[application]/design/[screenId]/_components/ScreenList/DropdownMenu.svelte +++ b/packages/builder/src/pages/builder/app/[application]/design/[screenId]/_components/ScreenList/DropdownMenu.svelte @@ -1,5 +1,5 @@