Merge branch 'v3-ui' of github.com:Budibase/budibase into views-openapi

2024-10-31 14:34:12 +00:00 · 2024-10-31 14:34:12 +00:00 · ae82c9bae5
parent 91528ca525 e3b499885f
commit ae82c9bae5
7 changed files with 163 additions and 43 deletions
--- a/packages/backend-core/src/sql/sql.ts
+++ b/packages/backend-core/src/sql/sql.ts
@ -1091,7 +1091,14 @@ class InternalBuilder {
            )
          }
        } else {
-          query = query.count(`* as ${aggregation.name}`)
+          if (this.client === SqlClient.ORACLE) {
+            const field = this.convertClobs(`${tableName}.${aggregation.field}`)
+            query = query.select(
+              this.knex.raw(`COUNT(??) as ??`, [field, aggregation.name])
+            )
+          } else {
+            query = query.count(`${aggregation.field} as ${aggregation.name}`)
+          }
        }
      } else {
        const fieldSchema = this.getFieldSchema(aggregation.field)
--- a/packages/server/src/api/controllers/view/viewsV2.ts
+++ b/packages/server/src/api/controllers/view/viewsV2.ts
@ -13,6 +13,8 @@ import {
  ViewFieldMetadata,
  CalculationType,
  ViewFetchResponseEnriched,
+  CountDistinctCalculationFieldMetadata,
+  CountCalculationFieldMetadata,
 } from "@budibase/types"
 import { builderSocket, gridSocket } from "../../../websockets"
 import { helpers } from "@budibase/shared-core"
@ -23,27 +25,31 @@ function stripUnknownFields(
  if (helpers.views.isCalculationField(field)) {
    if (field.calculationType === CalculationType.COUNT) {
      if ("distinct" in field && field.distinct) {
-        return {
+        const strippedField: RequiredKeys<CountDistinctCalculationFieldMetadata> =
+          {
+            order: field.order,
+            width: field.width,
+            visible: field.visible,
+            readonly: field.readonly,
+            icon: field.icon,
+            distinct: field.distinct,
+            calculationType: field.calculationType,
+            field: field.field,
+            columns: field.columns,
+          }
+        return strippedField
+      } else {
+        const strippedField: RequiredKeys<CountCalculationFieldMetadata> = {
          order: field.order,
          width: field.width,
          visible: field.visible,
          readonly: field.readonly,
          icon: field.icon,
-          distinct: field.distinct,
          calculationType: field.calculationType,
          field: field.field,
          columns: field.columns,
        }
-      } else {
-        return {
-          order: field.order,
-          width: field.width,
-          visible: field.visible,
-          readonly: field.readonly,
-          icon: field.icon,
-          calculationType: field.calculationType,
-          columns: field.columns,
-        }
+        return strippedField
      }
    }
    const strippedField: RequiredKeys<ViewCalculationFieldMetadata> = {
--- a/packages/server/src/api/routes/tests/viewV2.spec.ts
+++ b/packages/server/src/api/routes/tests/viewV2.spec.ts
@ -693,6 +693,12 @@ describe.each([
                calculationType: CalculationType.COUNT,
                field: "Price",
              },
+              countDistinct: {
+                visible: true,
+                calculationType: CalculationType.COUNT,
+                distinct: true,
+                field: "Price",
+              },
              min: {
                visible: true,
                calculationType: CalculationType.MIN,
@ -708,11 +714,6 @@ describe.each([
                calculationType: CalculationType.AVG,
                field: "Price",
              },
-              sum2: {
-                visible: true,
-                calculationType: CalculationType.SUM,
-                field: "Price",
-              },
            },
          },
          {
@ -763,10 +764,12 @@ describe.each([
              count: {
                visible: true,
                calculationType: CalculationType.COUNT,
+                field: "Price",
              },
              count2: {
                visible: true,
                calculationType: CalculationType.COUNT,
+                field: "Price",
              },
            },
          },
@ -774,7 +777,7 @@ describe.each([
            status: 400,
            body: {
              message:
-                'Duplicate calculation on field "*", calculation type "count"',
+                'Duplicate calculation on field "Price", calculation type "count"',
            },
          }
        )
@ -805,7 +808,7 @@ describe.each([
            status: 400,
            body: {
              message:
-                'Duplicate calculation on field "Price", calculation type "count"',
+                'Duplicate calculation on field "Price", calculation type "count distinct"',
            },
          }
        )
@ -820,6 +823,7 @@ describe.each([
            count: {
              visible: true,
              calculationType: CalculationType.COUNT,
+              field: "Price",
            },
            count2: {
              visible: true,
@ -831,6 +835,26 @@ describe.each([
        })
      })

+      it("does not confuse counts on different fields in the duplicate check", async () => {
+        await config.api.viewV2.create({
+          tableId: table._id!,
+          name: generator.guid(),
+          type: ViewV2Type.CALCULATION,
+          schema: {
+            count: {
+              visible: true,
+              calculationType: CalculationType.COUNT,
+              field: "Price",
+            },
+            count2: {
+              visible: true,
+              calculationType: CalculationType.COUNT,
+              field: "Category",
+            },
+          },
+        })
+      })
+
      !isLucene &&
        it("does not get confused when a calculation field shadows a basic one", async () => {
          const table = await config.api.table.save(
@ -1607,6 +1631,7 @@ describe.each([
            view.schema!.count = {
              visible: true,
              calculationType: CalculationType.COUNT,
+              field: "age",
            }
            await config.api.viewV2.update(view)

@ -3793,6 +3818,51 @@ describe.each([
              expect(rows[0].sum).toEqual(55)
            })

+            it("should be able to count non-numeric fields", async () => {
+              const table = await config.api.table.save(
+                saveTableRequest({
+                  schema: {
+                    firstName: {
+                      type: FieldType.STRING,
+                      name: "firstName",
+                    },
+                    lastName: {
+                      type: FieldType.STRING,
+                      name: "lastName",
+                    },
+                  },
+                })
+              )
+
+              const view = await config.api.viewV2.create({
+                tableId: table._id!,
+                name: generator.guid(),
+                type: ViewV2Type.CALCULATION,
+                schema: {
+                  count: {
+                    visible: true,
+                    calculationType: CalculationType.COUNT,
+                    field: "firstName",
+                  },
+                },
+              })
+
+              await config.api.row.bulkImport(table._id!, {
+                rows: [
+                  { firstName: "Jane", lastName: "Smith" },
+                  { firstName: "Jane", lastName: "Doe" },
+                  { firstName: "Alice", lastName: "Smith" },
+                ],
+              })
+
+              const { rows } = await config.api.viewV2.search(view.id, {
+                query: {},
+              })
+
+              expect(rows).toHaveLength(1)
+              expect(rows[0].count).toEqual(3)
+            })
+
            it("should be able to filter rows on the view itself", async () => {
              const table = await config.api.table.save(
                saveTableRequest({
--- a/packages/server/src/sdk/app/rows/search/internal/sqs.ts
+++ b/packages/server/src/sdk/app/rows/search/internal/sqs.ts
@ -351,6 +351,7 @@ export async function search(
          aggregations.push({
            name: key,
            calculationType: field.calculationType,
+            field: mapToUserColumn(field.field),
          })
        }
      } else {
--- a/packages/server/src/sdk/app/views/index.ts
+++ b/packages/server/src/sdk/app/views/index.ts
@ -80,12 +80,68 @@ export function isView(view: any): view is ViewV2 {
  return view.id && docIds.isViewId(view.id) && view.version === 2
 }

+function guardDuplicateCalculationFields(view: Omit<ViewV2, "id" | "version">) {
+  const seen: Record<string, Record<CalculationType, boolean>> = {}
+  const calculationFields = helpers.views.calculationFields(view)
+  for (const name of Object.keys(calculationFields)) {
+    const schema = calculationFields[name]
+    const isCount = schema.calculationType === CalculationType.COUNT
+    const isDistinct = "distinct" in schema
+
+    if (isCount && isDistinct) {
+      // We do these separately.
+      continue
+    }
+
+    if (seen[schema.field]?.[schema.calculationType]) {
+      throw new HTTPError(
+        `Duplicate calculation on field "${schema.field}", calculation type "${schema.calculationType}"`,
+        400
+      )
+    }
+    seen[schema.field] ??= {} as Record<CalculationType, boolean>
+    seen[schema.field][schema.calculationType] = true
+  }
+}
+
+function guardDuplicateCountDistinctFields(
+  view: Omit<ViewV2, "id" | "version">
+) {
+  const seen: Record<string, Record<CalculationType, boolean>> = {}
+  const calculationFields = helpers.views.calculationFields(view)
+  for (const name of Object.keys(calculationFields)) {
+    const schema = calculationFields[name]
+    const isCount = schema.calculationType === CalculationType.COUNT
+    if (!isCount) {
+      continue
+    }
+
+    const isDistinct = "distinct" in schema
+    if (!isDistinct) {
+      // We do these separately.
+      continue
+    }
+
+    if (seen[schema.field]?.[schema.calculationType]) {
+      throw new HTTPError(
+        `Duplicate calculation on field "${schema.field}", calculation type "${schema.calculationType} distinct"`,
+        400
+      )
+    }
+    seen[schema.field] ??= {} as Record<CalculationType, boolean>
+    seen[schema.field][schema.calculationType] = true
+  }
+}
+
 async function guardCalculationViewSchema(
  table: Table,
  view: Omit<ViewV2, "id" | "version">
 ) {
  const calculationFields = helpers.views.calculationFields(view)

+  guardDuplicateCalculationFields(view)
+  guardDuplicateCountDistinctFields(view)
+
  if (Object.keys(calculationFields).length > 5) {
    throw new HTTPError(
      "Calculation views can only have a maximum of 5 fields",
@ -93,29 +149,8 @@ async function guardCalculationViewSchema(
    )
  }

-  const seen: Record<string, Record<CalculationType, boolean>> = {}
-
  for (const name of Object.keys(calculationFields)) {
    const schema = calculationFields[name]
-    const isCount = schema.calculationType === CalculationType.COUNT
-    const isDistinct = isCount && "distinct" in schema && schema.distinct
-
-    const field = isCount && !isDistinct ? "*" : schema.field
-    if (seen[field]?.[schema.calculationType]) {
-      throw new HTTPError(
-        `Duplicate calculation on field "${field}", calculation type "${schema.calculationType}"`,
-        400
-      )
-    }
-    seen[field] ??= {} as Record<CalculationType, boolean>
-    seen[field][schema.calculationType] = true
-
-    // Count fields that aren't distinct don't need to reference another field,
-    // so we don't validate it.
-    if (isCount && !isDistinct) {
-      continue
-    }
-
    if (!schema.field) {
      throw new HTTPError(
        `Calculation field "${name}" is missing a "field" property`,
@ -131,6 +166,7 @@ async function guardCalculationViewSchema(
      )
    }

+    const isCount = schema.calculationType === CalculationType.COUNT
    if (!isCount && !isNumeric(targetSchema.type)) {
      throw new HTTPError(
        `Calculation field "${name}" references field "${schema.field}" which is not a numeric field`,
--- a/packages/types/src/documents/app/view.ts
+++ b/packages/types/src/documents/app/view.ts
@ -54,12 +54,12 @@ export interface NumericCalculationFieldMetadata

 export interface CountCalculationFieldMetadata extends BasicViewFieldMetadata {
  calculationType: CalculationType.COUNT
+  field: string
 }

 export interface CountDistinctCalculationFieldMetadata
  extends CountCalculationFieldMetadata {
  distinct: true
-  field: string
 }

 export type ViewCalculationFieldMetadata =
--- a/packages/types/src/sdk/row.ts
+++ b/packages/types/src/sdk/row.ts
@ -18,11 +18,11 @@ export interface NumericAggregation extends BaseAggregation {

 export interface CountAggregation extends BaseAggregation {
  calculationType: CalculationType.COUNT
+  field: string
 }

 export interface CountDistinctAggregation extends CountAggregation {
  distinct: true
-  field: string
 }

 export type Aggregation =