From 964f8222babbdb7867ed1b3df834ac693d687851 Mon Sep 17 00:00:00 2001 From: Sam Rose Date: Thu, 10 Oct 2024 17:10:07 +0100 Subject: [PATCH 1/4] Allow sorting by calculation fields. --- packages/backend-core/src/sql/sql.ts | 24 +++- .../src/api/routes/tests/viewV2.spec.ts | 118 ++++++++++++++++++ .../src/sdk/app/rows/search/internal/sqs.ts | 27 ++-- 3 files changed, 156 insertions(+), 13 deletions(-) diff --git a/packages/backend-core/src/sql/sql.ts b/packages/backend-core/src/sql/sql.ts index f122ad1c41..382eca3f76 100644 --- a/packages/backend-core/src/sql/sql.ts +++ b/packages/backend-core/src/sql/sql.ts @@ -273,6 +273,7 @@ class InternalBuilder { const col = parts.pop()! const schema = this.table.schema[col] let identifier = this.quotedIdentifier(field) + if ( schema.type === FieldType.STRING || schema.type === FieldType.LONGFORM || @@ -957,6 +958,13 @@ class InternalBuilder { return query } + isAggregateField(field: string): boolean { + const found = this.query.resource?.aggregations?.find( + aggregation => aggregation.name === field + ) + return !!found + } + addSorting(query: Knex.QueryBuilder): Knex.QueryBuilder { let { sort, resource } = this.query const primaryKey = this.table.primary @@ -979,13 +987,17 @@ class InternalBuilder { nulls = value.direction === SortOrder.ASCENDING ? "first" : "last" } - let composite = `${aliased}.${key}` - if (this.client === SqlClient.ORACLE) { - query = query.orderByRaw( - `${this.convertClobs(composite)} ${direction} nulls ${nulls}` - ) + if (this.isAggregateField(key)) { + query = query.orderBy(key, direction, nulls) } else { - query = query.orderBy(composite, direction, nulls) + let composite = `${aliased}.${key}` + if (this.client === SqlClient.ORACLE) { + query = query.orderByRaw( + `${this.convertClobs(composite)} ${direction} nulls ${nulls}` + ) + } else { + query = query.orderBy(composite, direction, nulls) + } } } } diff --git a/packages/server/src/api/routes/tests/viewV2.spec.ts b/packages/server/src/api/routes/tests/viewV2.spec.ts index 0b4237406f..eae429d49d 100644 --- a/packages/server/src/api/routes/tests/viewV2.spec.ts +++ b/packages/server/src/api/routes/tests/viewV2.spec.ts @@ -3381,6 +3381,124 @@ describe.each([ expect(rows).toHaveLength(1) expect(rows[0].sum).toEqual(3) }) + + it("should be able to sort by group by field", async () => { + const table = await config.api.table.save( + saveTableRequest({ + schema: { + quantity: { + type: FieldType.NUMBER, + name: "quantity", + }, + price: { + type: FieldType.NUMBER, + name: "price", + }, + }, + }) + ) + + const view = await config.api.viewV2.create({ + tableId: table._id!, + name: generator.guid(), + type: ViewV2Type.CALCULATION, + schema: { + quantity: { visible: true }, + sum: { + visible: true, + calculationType: CalculationType.SUM, + field: "price", + }, + }, + }) + + await config.api.row.bulkImport(table._id!, { + rows: [ + { + quantity: 1, + price: 1, + }, + { + quantity: 1, + price: 2, + }, + { + quantity: 2, + price: 10, + }, + ], + }) + + const { rows } = await config.api.viewV2.search(view.id, { + query: {}, + sort: "quantity", + sortOrder: SortOrder.DESCENDING, + }) + + expect(rows).toEqual([ + expect.objectContaining({ quantity: 2, sum: 10 }), + expect.objectContaining({ quantity: 1, sum: 3 }), + ]) + }) + + it("should be able to sort by a calculation", async () => { + const table = await config.api.table.save( + saveTableRequest({ + schema: { + quantity: { + type: FieldType.NUMBER, + name: "quantity", + }, + price: { + type: FieldType.NUMBER, + name: "price", + }, + }, + }) + ) + + await config.api.row.bulkImport(table._id!, { + rows: [ + { + quantity: 1, + price: 1, + }, + { + quantity: 1, + price: 2, + }, + { + quantity: 2, + price: 10, + }, + ], + }) + + const view = await config.api.viewV2.create({ + tableId: table._id!, + name: generator.guid(), + type: ViewV2Type.CALCULATION, + schema: { + quantity: { visible: true }, + sum: { + visible: true, + calculationType: CalculationType.SUM, + field: "price", + }, + }, + }) + + const { rows } = await config.api.viewV2.search(view.id, { + query: {}, + sort: "sum", + sortOrder: SortOrder.DESCENDING, + }) + + expect(rows).toEqual([ + expect.objectContaining({ quantity: 2, sum: 10 }), + expect.objectContaining({ quantity: 1, sum: 3 }), + ]) + }) }) !isLucene && diff --git a/packages/server/src/sdk/app/rows/search/internal/sqs.ts b/packages/server/src/sdk/app/rows/search/internal/sqs.ts index ad323a8c12..916e20957b 100644 --- a/packages/server/src/sdk/app/rows/search/internal/sqs.ts +++ b/packages/server/src/sdk/app/rows/search/internal/sqs.ts @@ -418,13 +418,26 @@ export async function search( if (params.sort) { const sortField = table.schema[params.sort] - const sortType = - sortField.type === FieldType.NUMBER ? SortType.NUMBER : SortType.STRING - request.sort = { - [mapToUserColumn(sortField.name)]: { - direction: params.sortOrder || SortOrder.ASCENDING, - type: sortType as SortType, - }, + const isAggregateField = aggregations.some(agg => agg.name === params.sort) + + if (isAggregateField) { + request.sort = { + [params.sort]: { + direction: params.sortOrder || SortOrder.ASCENDING, + type: SortType.NUMBER, + }, + } + } else if (sortField) { + const sortType = + sortField.type === FieldType.NUMBER ? SortType.NUMBER : SortType.STRING + request.sort = { + [mapToUserColumn(sortField.name)]: { + direction: params.sortOrder || SortOrder.ASCENDING, + type: sortType as SortType, + }, + } + } else { + throw new Error(`Unable to sort by ${params.sort}`) } } From 1bcd2590d3285f404e4f84c48bae8e8cf6992589 Mon Sep 17 00:00:00 2001 From: Sam Rose Date: Fri, 11 Oct 2024 12:28:20 +0100 Subject: [PATCH 2/4] Allow BB_REFERNCE_SINGLE to have default values. --- .../server/src/api/routes/tests/row.spec.ts | 32 +++++++++++++++++++ packages/shared-core/src/table.ts | 2 +- .../types/src/documents/app/table/schema.ts | 1 + 3 files changed, 34 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 b86ec38d08..61031a79b4 100644 --- a/packages/server/src/api/routes/tests/row.spec.ts +++ b/packages/server/src/api/routes/tests/row.spec.ts @@ -14,6 +14,7 @@ import { InternalTable, tenancy, features, + utils, } from "@budibase/backend-core" import { quotas } from "@budibase/pro" import { @@ -757,6 +758,37 @@ describe.each([ }) }) + describe.only("user column", () => { + beforeAll(async () => { + table = await config.api.table.save( + saveTableRequest({ + schema: { + user: { + name: "user", + type: FieldType.BB_REFERENCE_SINGLE, + subtype: BBReferenceFieldSubType.USER, + default: "{{ [Current User]._id }}", + }, + }, + }) + ) + }) + + it("creates a new row with a default value successfully", async () => { + const row = await config.api.row.save(table._id!, {}) + expect(row.user._id).toEqual(config.getUser()._id) + }) + + it("does not use default value if value specified", async () => { + const id = `us_${utils.newid()}` + await config.createUser({ _id: id }) + const row = await config.api.row.save(table._id!, { + user: id, + }) + expect(row.user._id).toEqual(id) + }) + }) + describe("bindings", () => { describe("string column", () => { beforeAll(async () => { diff --git a/packages/shared-core/src/table.ts b/packages/shared-core/src/table.ts index 57f6854604..677b1e2357 100644 --- a/packages/shared-core/src/table.ts +++ b/packages/shared-core/src/table.ts @@ -67,7 +67,7 @@ const allowDefaultColumnByType: Record = { [FieldType.SIGNATURE_SINGLE]: false, [FieldType.LINK]: false, [FieldType.BB_REFERENCE]: false, - [FieldType.BB_REFERENCE_SINGLE]: false, + [FieldType.BB_REFERENCE_SINGLE]: true, } export function canBeDisplayColumn(type: FieldType): boolean { diff --git a/packages/types/src/documents/app/table/schema.ts b/packages/types/src/documents/app/table/schema.ts index f9d1a4c012..f5bb081fd5 100644 --- a/packages/types/src/documents/app/table/schema.ts +++ b/packages/types/src/documents/app/table/schema.ts @@ -126,6 +126,7 @@ export interface BBReferenceSingleFieldMetadata extends Omit { type: FieldType.BB_REFERENCE_SINGLE subtype: Exclude + default?: string } export interface AttachmentFieldMetadata extends BaseFieldSchema { From 64d4aef87df03fd8fc2575fc56f149da7b006913 Mon Sep 17 00:00:00 2001 From: Adria Navarro Date: Fri, 11 Oct 2024 16:41:51 +0200 Subject: [PATCH 3/4] Fix all or property --- packages/shared-core/src/utils.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/shared-core/src/utils.ts b/packages/shared-core/src/utils.ts index 4847e911e9..bead5961f0 100644 --- a/packages/shared-core/src/utils.ts +++ b/packages/shared-core/src/utils.ts @@ -172,7 +172,7 @@ export const processSearchFilters = ( .sort((a, b) => { return a.localeCompare(b) }) - .filter(key => key in filter) + .filter(key => filter[key]) if (filterPropertyKeys.length == 1) { const key = filterPropertyKeys[0], From 158e0950450fb66742d0819a64460f979f528b9b Mon Sep 17 00:00:00 2001 From: Sam Rose Date: Fri, 11 Oct 2024 16:24:03 +0100 Subject: [PATCH 4/4] Fix lint. --- packages/server/src/api/routes/tests/row.spec.ts | 2 +- 1 file changed, 1 insertion(+), 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 61031a79b4..6490b4770a 100644 --- a/packages/server/src/api/routes/tests/row.spec.ts +++ b/packages/server/src/api/routes/tests/row.spec.ts @@ -758,7 +758,7 @@ describe.each([ }) }) - describe.only("user column", () => { + describe("user column", () => { beforeAll(async () => { table = await config.api.table.save( saveTableRequest({