From 55be64b3718b9f0b944b00a17e2fb1c33a82aa80 Mon Sep 17 00:00:00 2001
From: Sam Rose <hello@samwho.dev>
Date: Thu, 10 Oct 2024 09:16:43 +0100
Subject: [PATCH] Only check visible fields when checking group by view
 calculations.

---
 .../src/api/controllers/row/utils/sqlUtils.ts |  4 +--
 .../src/api/routes/tests/viewV2.spec.ts       | 28 +++++++++++++++++++
 .../src/sdk/app/rows/search/internal/sqs.ts   |  4 +--
 packages/shared-core/src/helpers/views.ts     | 11 ++++++--
 4 files changed, 39 insertions(+), 8 deletions(-)

diff --git a/packages/server/src/api/controllers/row/utils/sqlUtils.ts b/packages/server/src/api/controllers/row/utils/sqlUtils.ts
index 607fee7580..89f8007642 100644
--- a/packages/server/src/api/controllers/row/utils/sqlUtils.ts
+++ b/packages/server/src/api/controllers/row/utils/sqlUtils.ts
@@ -133,9 +133,7 @@ export async function buildSqlFieldList(
 
   let fields: string[] = []
   if (sdk.views.isView(source)) {
-    fields = Object.keys(helpers.views.basicFields(source)).filter(
-      key => source.schema?.[key]?.visible !== false
-    )
+    fields = Object.keys(helpers.views.basicFields(source))
   } else {
     fields = extractRealFields(source)
   }
diff --git a/packages/server/src/api/routes/tests/viewV2.spec.ts b/packages/server/src/api/routes/tests/viewV2.spec.ts
index 195481ae10..23955a8302 100644
--- a/packages/server/src/api/routes/tests/viewV2.spec.ts
+++ b/packages/server/src/api/routes/tests/viewV2.spec.ts
@@ -800,6 +800,34 @@ describe.each([
             )
           }
         })
+
+      isInternal &&
+        it("shouldn't trigger a complex type check on a group by field if field is invisible", async () => {
+          const table = await config.api.table.save(
+            saveTableRequest({
+              schema: {
+                field: {
+                  name: "field",
+                  type: FieldType.JSON,
+                },
+              },
+            })
+          )
+
+          await config.api.viewV2.create(
+            {
+              tableId: table._id!,
+              name: generator.guid(),
+              type: ViewV2Type.CALCULATION,
+              schema: {
+                field: { visible: false },
+              },
+            },
+            {
+              status: 201,
+            }
+          )
+        })
     })
 
     describe("update", () => {
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 3847eb8f31..ad323a8c12 100644
--- a/packages/server/src/sdk/app/rows/search/internal/sqs.ts
+++ b/packages/server/src/sdk/app/rows/search/internal/sqs.ts
@@ -68,9 +68,7 @@ async function buildInternalFieldList(
   const { relationships, allowedFields } = opts || {}
   let schemaFields: string[] = []
   if (sdk.views.isView(source)) {
-    schemaFields = Object.keys(helpers.views.basicFields(source)).filter(
-      key => source.schema?.[key]?.visible !== false
-    )
+    schemaFields = Object.keys(helpers.views.basicFields(source))
   } else {
     schemaFields = Object.keys(source.schema).filter(
       key => source.schema[key].visible !== false
diff --git a/packages/shared-core/src/helpers/views.ts b/packages/shared-core/src/helpers/views.ts
index 0113375adf..f0407eeec9 100644
--- a/packages/shared-core/src/helpers/views.ts
+++ b/packages/shared-core/src/helpers/views.ts
@@ -33,6 +33,13 @@ export function calculationFields(view: UnsavedViewV2) {
   return pickBy(view.schema || {}, isCalculationField)
 }
 
-export function basicFields(view: UnsavedViewV2) {
-  return pickBy(view.schema || {}, field => !isCalculationField(field))
+export function isVisible(field: ViewFieldMetadata) {
+  return field.visible !== false
+}
+
+export function basicFields(view: UnsavedViewV2, opts?: { visible?: boolean }) {
+  const { visible = true } = opts || {}
+  return pickBy(view.schema || {}, field => {
+    return !isCalculationField(field) && (!visible || isVisible(field))
+  })
 }