From d2c493aa7b6268e63f2dc8ac0f8c3e8b4c14efef Mon Sep 17 00:00:00 2001
From: Anbarasu <anbarasun123@gmail.com>
Date: Tue, 25 Jun 2024 11:00:36 +0530
Subject: [PATCH] Nc fix/api call (#8841)

* fix: aggregate not loading

* fix: dulpicate call

* fix: scroll issue

* fix: scroll issue

* fix: random fix

* fix: test

* fix: remove console.log

* fix: set Timeout

* fix: enable none aggregation for fk

* fix: test debug

* fix: lookup attachment issue

* fix: skip system LTAR columns from aggregation

* fix: revert debug changes

* fix: if display value missing treat the first column as display value

* fix: disable system col aggregation if hidden

* fix: missing commit

* fix: exclude system column

* fix: rollup grouping/aggregation fix

---------

Co-authored-by: Pranav C <pranavxc@gmail.com>
---
 .../nc-gui/components/nc/PaginationV2.vue     |  2 +-
 .../smartsheet/grid/PaginationV2.vue          |  8 +---
 .../components/smartsheet/grid/Table.vue      |  4 +-
 .../components/smartsheet/grid/index.vue      |  4 +-
 .../nc-gui/composables/useViewAggregate.ts    | 45 ++++++++++---------
 .../nocodb-sdk/src/lib/aggregationHelper.ts   |  2 +-
 packages/nocodb/src/db/BaseModelSqlv2.ts      | 16 +++++--
 packages/nocodb/src/db/aggregation.ts         |  1 +
 .../src/db/generateLookupSelectQuery.ts       | 23 ++++++----
 .../nocodb/src/services/data-table.service.ts |  2 +-
 .../src/services/public-datas.service.ts      |  2 +-
 11 files changed, 64 insertions(+), 45 deletions(-)

diff --git a/packages/nc-gui/components/nc/PaginationV2.vue b/packages/nc-gui/components/nc/PaginationV2.vue
index 714fc023c6..b1c493b0b9 100644
--- a/packages/nc-gui/components/nc/PaginationV2.vue
+++ b/packages/nc-gui/components/nc/PaginationV2.vue
@@ -143,7 +143,7 @@ const pageSizeOptions = [
           </NcButton>
 
           <template #overlay>
-            <NcMenu class="nc-scrollbar-thin nc-pagination-menu max-h-54 overflow-y-auto">
+            <NcMenu class="nc-scrollbar-md nc-pagination-menu max-h-54">
               <NcSubMenu :key="`${localPageSize}page`" class="bg-gray-100 z-20 top-0 !sticky">
                 <template #title>
                   <div class="rounded-lg text-[13px] font-medium w-full">{{ localPageSize }} / page</div>
diff --git a/packages/nc-gui/components/smartsheet/grid/PaginationV2.vue b/packages/nc-gui/components/smartsheet/grid/PaginationV2.vue
index fbb409ec27..8f2cf66a2b 100644
--- a/packages/nc-gui/components/smartsheet/grid/PaginationV2.vue
+++ b/packages/nc-gui/components/smartsheet/grid/PaginationV2.vue
@@ -84,10 +84,6 @@ const size = computed({
 const renderAltOrOptlKey = () => {
   return isMac() ? '⌥' : 'ALT'
 }
-
-onMounted(() => {
-  loadViewAggregate()
-})
 </script>
 
 <template>
@@ -95,7 +91,7 @@ onMounted(() => {
     <div class="sticky flex items-center bg-gray-50 left-0">
       <NcDropdown
         :disabled="[UITypes.SpecificDBType, UITypes.ForeignKey].includes(displayFieldComputed.column?.uidt!) || isLocked"
-        overlay-class-name="max-h-96 relative scroll-container nc-scrollbar-thin overflow-auto"
+        overlay-class-name="max-h-96 relative scroll-container nc-scrollbar-md overflow-auto"
       >
         <div
           v-if="displayFieldComputed.field && displayFieldComputed.column?.id"
@@ -194,7 +190,7 @@ onMounted(() => {
       <NcDropdown
         v-if="field && column?.id"
         :disabled="[UITypes.SpecificDBType, UITypes.ForeignKey].includes(column?.uidt!) || isLocked"
-        overlay-class-name="max-h-96 relative scroll-container nc-scrollbar-thin overflow-auto"
+        overlay-class-name="max-h-96 relative scroll-container nc-scrollbar-md overflow-auto"
       >
         <div
           class="flex items-center overflow-x-hidden justify-end group hover:bg-gray-100 cursor-pointer text-gray-500 transition-all transition-linear px-3 py-2"
diff --git a/packages/nc-gui/components/smartsheet/grid/Table.vue b/packages/nc-gui/components/smartsheet/grid/Table.vue
index b50b93bbef..3b63279e41 100644
--- a/packages/nc-gui/components/smartsheet/grid/Table.vue
+++ b/packages/nc-gui/components/smartsheet/grid/Table.vue
@@ -147,6 +147,8 @@ const { paste } = usePaste()
 
 const { addLTARRef, syncLTARRefs, clearLTARCell, cleaMMCell } = useSmartsheetLtarHelpersOrThrow()
 
+const { loadViewAggregate } = useViewAggregateOrThrow()
+
 // #Refs
 
 const smartTable = ref(null)
@@ -1522,7 +1524,7 @@ watch(
         }
         isViewDataLoading.value = true
         try {
-          await loadData?.()
+          await Promise.allSettled([loadData?.(), loadViewAggregate()])
           calculateSlices()
         } catch (e) {
           if (!axios.isCancel(e)) {
diff --git a/packages/nc-gui/components/smartsheet/grid/index.vue b/packages/nc-gui/components/smartsheet/grid/index.vue
index c069a6f1d6..156c33cca6 100644
--- a/packages/nc-gui/components/smartsheet/grid/index.vue
+++ b/packages/nc-gui/components/smartsheet/grid/index.vue
@@ -31,6 +31,8 @@ const expandedFormRowState = ref<Record<string, any>>()
 
 const tableRef = ref<typeof Table>()
 
+useProvideViewAggregate(view, meta, xWhere)
+
 const {
   loadData,
   paginationData,
@@ -79,8 +81,6 @@ provide(IsCalendarInj, ref(false))
 
 provide(RowHeightInj, rowHeight)
 
-useProvideViewAggregate(view, meta, xWhere)
-
 const isPublic = inject(IsPublicInj, ref(false))
 
 // reload table data reload hook as fallback to rowdatareload
diff --git a/packages/nc-gui/composables/useViewAggregate.ts b/packages/nc-gui/composables/useViewAggregate.ts
index 2d5a4a963d..6357fd555e 100644
--- a/packages/nc-gui/composables/useViewAggregate.ts
+++ b/packages/nc-gui/composables/useViewAggregate.ts
@@ -71,26 +71,31 @@ const [useProvideViewAggregate, useViewAggregate] = useInjectionState(
         type: string
       }>,
     ) => {
-      if (!meta.value?.id || !view.value?.id) return
-
-      try {
-        const data = !isPublic.value
-          ? await api.dbDataTableAggregate.dbDataTableAggregate(meta.value.id, {
-              viewId: view.value.id,
-              where: where?.value,
-              ...(fields ? { aggregation: fields } : {}),
-            })
-          : await fetchAggregatedData({
-              where: where?.value,
-              filtersArr: nestedFilters.value,
-              ...(fields ? { aggregation: fields } : {}),
-            })
-
-        Object.assign(aggregations.value, data)
-      } catch (error) {
-        console.log(error)
-        message.error(await extractSdkResponseErrorMsgv2(error as any))
-      }
+      // Wait for meta to be defined https://vueuse.org/shared/until/
+      await until(meta)
+        .toBeTruthy((c) => !!c, {
+          timeout: 10000,
+        })
+        .then(async () => {
+          try {
+            const data = !isPublic.value
+              ? await api.dbDataTableAggregate.dbDataTableAggregate(meta.value.id, {
+                  viewId: view.value.id,
+                  where: where?.value,
+                  ...(fields ? { aggregation: fields } : {}),
+                })
+              : await fetchAggregatedData({
+                  where: where?.value,
+                  filtersArr: nestedFilters.value,
+                  ...(fields ? { aggregation: fields } : {}),
+                })
+
+            Object.assign(aggregations.value, data)
+          } catch (error) {
+            console.log(error)
+            message.error(await extractSdkResponseErrorMsgv2(error as any))
+          }
+        })
     }
 
     const updateAggregate = async (fieldId: string, agg: string) => {
diff --git a/packages/nocodb-sdk/src/lib/aggregationHelper.ts b/packages/nocodb-sdk/src/lib/aggregationHelper.ts
index a9fd58de62..0577913ba5 100644
--- a/packages/nocodb-sdk/src/lib/aggregationHelper.ts
+++ b/packages/nocodb-sdk/src/lib/aggregationHelper.ts
@@ -115,7 +115,7 @@ const getAvailableAggregations = (type: string, parsed_tree?): string[] => {
       break;
     case UITypes.SpecificDBType:
     case UITypes.ForeignKey:
-      return [];
+      returnAggregations = [CommonAggregations.None];
   }
 
   if (!returnAggregations.length) {
diff --git a/packages/nocodb/src/db/BaseModelSqlv2.ts b/packages/nocodb/src/db/BaseModelSqlv2.ts
index a55a29f9a7..c614230251 100644
--- a/packages/nocodb/src/db/BaseModelSqlv2.ts
+++ b/packages/nocodb/src/db/BaseModelSqlv2.ts
@@ -697,13 +697,18 @@ class BaseModelSqlv2 {
     return await this.execAndParse(qb);
   }
 
-  async aggregate(args: { filterArr?: Filter[]; where?: string }) {
+  async aggregate(args: { filterArr?: Filter[]; where?: string }, view: View) {
     try {
       const { where, aggregation } = this._getListArgs(args as any);
 
+      const columns = await this.model.getColumns(this.context);
+
       let viewColumns = (
         await GridViewColumn.list(this.context, this.viewId)
-      ).filter((c) => c.show);
+      ).filter((c) => {
+        const col = columns.find((col) => col.id === c.fk_column_id);
+        return c.show && (view.show_system_fields || !isSystemColumn(col));
+      });
 
       // By default, the aggregation is done based on the columns configured in the view
       // If the aggregation parameter is provided, only the columns mentioned in the aggregation parameter are considered
@@ -721,8 +726,6 @@ class BaseModelSqlv2 {
           .filter((c) => c.show);
       }
 
-      const columns = await this.model.getColumns(this.context);
-
       const aliasColObjMap = await this.model.getAliasColObjMap(
         this.context,
         columns,
@@ -768,6 +771,11 @@ class BaseModelSqlv2 {
           const col = columns.find((c) => c.id === viewColumn.fk_column_id);
           if (!col) return null;
 
+          if (!viewColumn.aggregation) return;
+
+          // Skip system LTAR columns
+          if (isLinksOrLTAR(col) && col.system) return;
+
           const aggSql = await applyAggregation({
             baseModelSqlv2: this,
             aggregation: viewColumn.aggregation,
diff --git a/packages/nocodb/src/db/aggregation.ts b/packages/nocodb/src/db/aggregation.ts
index 73cd7d4a26..82ff3b9113 100644
--- a/packages/nocodb/src/db/aggregation.ts
+++ b/packages/nocodb/src/db/aggregation.ts
@@ -162,6 +162,7 @@ export default async function applyAggregation({
           column: column,
           alias: null,
           model,
+          isAggregation: true,
         })
       ).builder;
       break;
diff --git a/packages/nocodb/src/db/generateLookupSelectQuery.ts b/packages/nocodb/src/db/generateLookupSelectQuery.ts
index e06de66cb8..025ab085f6 100644
--- a/packages/nocodb/src/db/generateLookupSelectQuery.ts
+++ b/packages/nocodb/src/db/generateLookupSelectQuery.ts
@@ -1,4 +1,4 @@
-import { RelationTypes, UITypes } from 'nocodb-sdk';
+import { isVirtualCol, RelationTypes, UITypes } from 'nocodb-sdk';
 import type LookupColumn from '../models/LookupColumn';
 import type { BaseModelSqlv2 } from '~/db/BaseModelSqlv2';
 import type {
@@ -27,7 +27,7 @@ export async function getDisplayValueOfRefTable(
     .getColOptions(context)
     .then((colOpt) => colOpt.getRelatedTable(context))
     .then((model) => model.getColumns(context))
-    .then((cols) => cols.find((col) => col.pv));
+    .then((cols) => cols.find((col) => col.pv) || cols[0]);
 }
 
 // this function will generate the query for lookup column
@@ -41,12 +41,14 @@ export default async function generateLookupSelectQuery({
   alias,
   model: _model,
   getAlias = getAliasGenerator('__lk_slt_'),
+  isAggregation = false,
 }: {
   column: Column;
   baseModelSqlv2: BaseModelSqlv2;
   alias: string;
   model: Model;
   getAlias?: ReturnType<typeof getAliasGenerator>;
+  isAggregation?: boolean;
 }): Promise<any> {
   const knex = baseModelSqlv2.dbDriver;
 
@@ -298,11 +300,6 @@ 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:
           {
@@ -316,7 +313,9 @@ export default async function generateLookupSelectQuery({
                 alias: prevAlias,
               })
             ).builder;
-            selectQb.select(knex.raw(builder).wrap('(', ')'));
+            selectQb.select({
+              [lookupColumn.id]: knex.raw(builder).wrap('(', ')'),
+            });
           }
           break;
         case UITypes.Formula:
@@ -349,6 +348,14 @@ export default async function generateLookupSelectQuery({
             });
           }
           break;
+        case UITypes.Attachment:
+          if (!isAggregation) {
+            NcError.badRequest(
+              'Group by using attachment column is not supported',
+            );
+            break;
+          }
+        // eslint-disable-next-line no-fallthrough
         default:
           {
             selectQb.select(
diff --git a/packages/nocodb/src/services/data-table.service.ts b/packages/nocodb/src/services/data-table.service.ts
index 28155539a4..715f7d4d77 100644
--- a/packages/nocodb/src/services/data-table.service.ts
+++ b/packages/nocodb/src/services/data-table.service.ts
@@ -98,7 +98,7 @@ export class DataTableService {
       listArgs.aggregation = JSON.parse(listArgs.aggregation);
     } catch (e) {}
 
-    const data = await baseModel.aggregate(listArgs);
+    const data = await baseModel.aggregate(listArgs, view);
 
     return data;
   }
diff --git a/packages/nocodb/src/services/public-datas.service.ts b/packages/nocodb/src/services/public-datas.service.ts
index 5c58aeb6db..c3efc70675 100644
--- a/packages/nocodb/src/services/public-datas.service.ts
+++ b/packages/nocodb/src/services/public-datas.service.ts
@@ -138,7 +138,7 @@ export class PublicDatasService {
       listArgs.aggregation = JSON.parse(listArgs.aggregation);
     } catch (e) {}
 
-    return await baseModel.aggregate(listArgs);
+    return await baseModel.aggregate(listArgs, view);
   }
 
   // todo: Handle the error case where view doesnt belong to model