Merge pull request #14704 from Budibase/view-calculation-validation-2

Validate that there are no duplicate calculations in calculation views.
Budibase · Oct 8, 2024 · e2f4c38 · e2f4c38
2 parents be4bc43 + 863ce48
commit e2f4c38
Show file tree

Hide file tree

Showing 2 changed files with 119 additions and 4 deletions.
diff --git a/packages/server/src/api/routes/tests/viewV2.spec.ts b/packages/server/src/api/routes/tests/viewV2.spec.ts
@@ -618,6 +618,109 @@ describe.each([
           }
         )
       })
+
+      it("cannot create a calculation view with duplicate calculations", async () => {
+        await config.api.viewV2.create(
+          {
+            tableId: table._id!,
+            name: generator.guid(),
+            schema: {
+              sum: {
+                visible: true,
+                calculationType: CalculationType.SUM,
+                field: "Price",
+              },
+              sum2: {
+                visible: true,
+                calculationType: CalculationType.SUM,
+                field: "Price",
+              },
+            },
+          },
+          {
+            status: 400,
+            body: {
+              message:
+                'Duplicate calculation on field "Price", calculation type "sum"',
+            },
+          }
+        )
+      })
+
+      it("finds duplicate counts", async () => {
+        await config.api.viewV2.create(
+          {
+            tableId: table._id!,
+            name: generator.guid(),
+            schema: {
+              count: {
+                visible: true,
+                calculationType: CalculationType.COUNT,
+              },
+              count2: {
+                visible: true,
+                calculationType: CalculationType.COUNT,
+              },
+            },
+          },
+          {
+            status: 400,
+            body: {
+              message:
+                'Duplicate calculation on field "*", calculation type "count"',
+            },
+          }
+        )
+      })
+
+      it("finds duplicate count distincts", async () => {
+        await config.api.viewV2.create(
+          {
+            tableId: table._id!,
+            name: generator.guid(),
+            schema: {
+              count: {
+                visible: true,
+                calculationType: CalculationType.COUNT,
+                distinct: true,
+                field: "Price",
+              },
+              count2: {
+                visible: true,
+                calculationType: CalculationType.COUNT,
+                distinct: true,
+                field: "Price",
+              },
+            },
+          },
+          {
+            status: 400,
+            body: {
+              message:
+                'Duplicate calculation on field "Price", calculation type "count"',
+            },
+          }
+        )
+      })
+
+      it("does not confuse counts and count distincts in the duplicate check", async () => {
+        await config.api.viewV2.create({
+          tableId: table._id!,
+          name: generator.guid(),
+          schema: {
+            count: {
+              visible: true,
+              calculationType: CalculationType.COUNT,
+            },
+            count2: {
+              visible: true,
+              calculationType: CalculationType.COUNT,
+              distinct: true,
+              field: "Price",
+            },
+          },
+        })
+      })
     })
 
     describe("update", () => {

diff --git a/packages/server/src/sdk/app/views/index.ts b/packages/server/src/sdk/app/views/index.ts
@@ -72,11 +72,23 @@ async function guardCalculationViewSchema(
     )
   }
 
-  for (const calculationFieldName of Object.keys(calculationFields)) {
-    const schema = calculationFields[calculationFieldName]
+  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) {
@@ -86,14 +98,14 @@ async function guardCalculationViewSchema(
     const targetSchema = table.schema[schema.field]
     if (!targetSchema) {
       throw new HTTPError(
-        `Calculation field "${calculationFieldName}" references field "${schema.field}" which does not exist in the table schema`,
+        `Calculation field "${name}" references field "${schema.field}" which does not exist in the table schema`,
         400
       )
     }
 
     if (!isCount && !helpers.schema.isNumeric(targetSchema)) {
       throw new HTTPError(
-        `Calculation field "${calculationFieldName}" references field "${schema.field}" which is not a numeric field`,
+        `Calculation field "${name}" references field "${schema.field}" which is not a numeric field`,
         400
       )
     }