From bb86af6c317240b27a77cf1a55caea3763903753 Mon Sep 17 00:00:00 2001
From: Pranav C <pranavxc@gmail.com>
Date: Thu, 16 Nov 2023 12:24:24 +0000
Subject: [PATCH] fix: block group by using attachment column

---
 .../smartsheet/toolbar/GroupByMenu.vue        | 56 +++++--------------
 packages/nocodb/src/db/BaseModelSqlv2.ts      | 10 ++++
 .../src/db/generateLookupSelectQuery.ts       |  3 +
 3 files changed, 27 insertions(+), 42 deletions(-)

diff --git a/packages/nc-gui/components/smartsheet/toolbar/GroupByMenu.vue b/packages/nc-gui/components/smartsheet/toolbar/GroupByMenu.vue
index 0969f2d125..2f67b75262 100644
--- a/packages/nc-gui/components/smartsheet/toolbar/GroupByMenu.vue
+++ b/packages/nc-gui/components/smartsheet/toolbar/GroupByMenu.vue
@@ -18,29 +18,7 @@ import {
   watch,
 } from '#imports'
 
-const groupingUidt = [
-  UITypes.SingleSelect,
-  UITypes.MultiSelect,
-  UITypes.Checkbox,
-  UITypes.Date,
-  UITypes.SingleLineText,
-  UITypes.Number,
-  UITypes.Rollup,
-  UITypes.Lookup,
-  UITypes.Links,
-  UITypes.Formula,
-  UITypes.Email,
-  UITypes.URL,
-  UITypes.PhoneNumber,
-  UITypes.Decimal,
-  UITypes.Currency,
-  UITypes.Percent,
-  UITypes.Duration,
-  UITypes.Rating,
-  UITypes.Year,
-  UITypes.DateTime,
-  UITypes.Time,
-]
+const excludedGroupingUidt = [UITypes.Attachment]
 
 const meta = inject(MetaInj, ref())
 const view = inject(ActiveViewInj, ref())
@@ -74,17 +52,17 @@ const groupedByColumnIds = computed(() => groupBy.value.map((g) => g.fk_column_i
 const { eventBus } = useSmartsheetStoreOrThrow()
 
 const { isMobileMode } = useGlobal()
-const btLookups = ref([])
+const supportedLookups = ref([])
 
 const fieldsToGroupBy = computed(() => {
   const fields = meta.value?.columns || []
 
   return fields.filter((field) => {
-    // if (!groupingUidt.includes(field.uidt as UITypes)) return false
-    //
-    // if (field.uidt === UITypes.Lookup) {
-    //   return btLookups.value.includes(field.id)
-    // }
+    if (excludedGroupingUidt.includes(field.uidt as UITypes)) return false
+
+    if (field.uidt === UITypes.Lookup) {
+      return supportedLookups.value.includes(field.id)
+    }
 
     return true
   })
@@ -173,23 +151,20 @@ watch(open, () => {
   }
 })
 
-const loadBtLookups = async () => {
+const loadAllowedLookups = async () => {
   const filteredLookupCols = []
   try {
     for (const col of meta.value?.columns || []) {
       if (col.uidt !== UITypes.Lookup) continue
 
       let nextCol = col
-      let btLookup = true
 
-      // check all the relation of nested lookup columns is bt or not
-      // include the column only if all only if all relations are bt
+      // check the lookup column is supported type or not
       while (btLookup && nextCol && nextCol.uidt === UITypes.Lookup) {
         const lookupRelation = (await getMeta(nextCol.fk_model_id))?.columns?.find(
           (c) => c.id === (nextCol.colOptions as LookupType).fk_relation_column_id,
         )
         if ((lookupRelation.colOptions as LinkToAnotherRecordType).type !== RelationTypes.BELONGS_TO) {
-          btLookup = false
           continue
         }
 
@@ -202,26 +177,25 @@ const loadBtLookups = async () => {
         // if next column is same as root lookup column then break the loop
         // since it's going to be a circular loop, and ignore the column
         if (nextCol.id === col.id) {
-          btLookup = false
           break
         }
       }
 
-      if (btLookup) filteredLookupCols.push(col.id)
+      if (nextCol.uidt !== UITypes.Attachment) filteredLookupCols.push(col.id)
     }
 
-    btLookups.value = filteredLookupCols
+    supportedLookups.value = filteredLookupCols
   } catch (e) {
     console.error(e)
   }
 }
 
 onMounted(async () => {
-  await loadBtLookups()
+  await loadAllowedLookups()
 })
 
 watch(meta, async () => {
-  await loadBtLookups()
+  await loadAllowedLookups()
 })
 </script>
 
@@ -258,9 +232,7 @@ watch(meta, async () => {
             <LazySmartsheetToolbarFieldListAutoCompleteDropdown
               v-model="group.fk_column_id"
               class="caption nc-sort-field-select"
-              :columns="
-                fieldsToGroupBy
-              "
+              :columns="fieldsToGroupBy"
               :allow-empty="true"
               @change="saveGroupBy"
               @click.stop
diff --git a/packages/nocodb/src/db/BaseModelSqlv2.ts b/packages/nocodb/src/db/BaseModelSqlv2.ts
index 6cf9d25d21..780056ced5 100644
--- a/packages/nocodb/src/db/BaseModelSqlv2.ts
+++ b/packages/nocodb/src/db/BaseModelSqlv2.ts
@@ -572,6 +572,11 @@ class BaseModelSqlv2 {
         groupByColumns[column.id] = column;
 
         switch (column.uidt) {
+          case UITypes.Attachment:
+            NcError.badRequest(
+              'Group by using attachment column is not supported',
+            );
+            break;
           case UITypes.Links:
           case UITypes.Rollup:
             selectors.push(
@@ -748,6 +753,11 @@ class BaseModelSqlv2 {
             });
 
           switch (column.uidt) {
+            case UITypes.Attachment:
+              NcError.badRequest(
+                'Group by using attachment column is not supported',
+              );
+              break;
             case UITypes.Rollup:
             case UITypes.Links:
               selectors.push(
diff --git a/packages/nocodb/src/db/generateLookupSelectQuery.ts b/packages/nocodb/src/db/generateLookupSelectQuery.ts
index 8e4e7066b7..38f3a4cc01 100644
--- a/packages/nocodb/src/db/generateLookupSelectQuery.ts
+++ b/packages/nocodb/src/db/generateLookupSelectQuery.ts
@@ -258,6 +258,9 @@ export default async function generateLookupSelectQuery({
     }
 
     switch (lookupColumn.uidt) {
+      case UITypes.Attachment:
+        NcError.badRequest('Group by using attachment column is not supported');
+        break;
       case UITypes.Links:
       case UITypes.Rollup:
         {