From fb7133e64fe4e6985351f31ea5485ca30f66bf37 Mon Sep 17 00:00:00 2001
From: Sam Rose <hello@samwho.dev>
Date: Fri, 4 Oct 2024 11:09:29 +0100
Subject: [PATCH] Validate that there are no duplicate calculations in
 calculation views.

---
 .../src/api/routes/tests/viewV2.spec.ts       | 103 ++++++++++++++++++
 packages/server/src/sdk/app/views/index.ts    |  20 +++-
 2 files changed, 119 insertions(+), 4 deletions(-)

diff --git a/packages/server/src/api/routes/tests/viewV2.spec.ts b/packages/server/src/api/routes/tests/viewV2.spec.ts
index 7ad1be4c1f..97e82f071b 100644
--- a/packages/server/src/api/routes/tests/viewV2.spec.ts
+++ b/packages/server/src/api/routes/tests/viewV2.spec.ts
@@ -615,6 +615,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
index be7259b057..dcd2c8cc14 100644
--- 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
       )
     }