Merge pull request #8354 from nocodb/nc-fix/group-by-calls

Nc fix/group by calls

packages/nc-gui/components/shared-view/Grid.vue

@@ -23,7 +23,7 @@ const { signedIn } = useGlobal()
 
 const { loadProject } = useBase()
 
-const { isLocked } = useProvideSmartsheetStore(sharedView, meta, true, ref([]), nestedFilters)
+const { isLocked, xWhere } = useProvideSmartsheetStore(sharedView, meta, true, ref([]), nestedFilters)
 
 useProvideKanbanViewStore(meta, sharedView)
 useProvideCalendarViewStore(meta, sharedView)
@@ -41,6 +41,7 @@ provide(IsPublicInj, ref(true))
 provide(IsLockedInj, isLocked)
 
 useProvideViewColumns(sharedView, meta, () => reloadEventHook?.trigger(), true)
+useProvideViewGroupBy(sharedView, meta, xWhere, true)
 
 if (signedIn.value) {
   try {

packages/nc-gui/components/smartsheet/grid/GroupBy.vue

@@ -37,6 +37,16 @@ const { isViewDataLoading, isPaginationLoading } = storeToRefs(useViewsStore())
 
 const reloadViewDataHook = inject(ReloadViewDataHookInj, createEventHook())
 
+const _loadGroupData = async (group: Group, force?: boolean, params?: any) => {
+  isViewDataLoading.value = true
+  isPaginationLoading.value = true
+
+  await props.loadGroupData(group, force, params)
+
+  isViewDataLoading.value = false
+  isPaginationLoading.value = false
+}
+
 const _depth = props.depth ?? 0
 
 const wrapper = ref<HTMLElement | undefined>()
@@ -67,12 +77,12 @@ const findAndLoadSubGroup = (key: any) => {
   if (key.length > 0 && vGroup.value.children) {
     if (!oldActiveGroups.value.includes(key[key.length - 1])) {
       const k = key[key.length - 1].replace('group-panel-', '')
-      const grp = vGroup.value.children[k]
+      const grp = vGroup.value.children.find((g) => `${g.key}` === k)
       if (grp) {
         if (grp.nested) {
           if (!grp.children?.length) props.loadGroups({}, grp)
         } else {
-          if (!grp.rows?.length || grp.count !== grp.rows?.length) props.loadGroupData(grp)
+          if (!grp.rows?.length || grp.count !== grp.rows?.length) _loadGroupData(grp)
         }
       }
     }
@@ -84,37 +94,35 @@ const reloadViewDataHandler = (params: void | { shouldShowLoading?: boolean | un
   if (vGroup.value.nested) {
     props.loadGroups({ ...(params?.offset !== undefined ? { offset: params.offset } : {}) }, vGroup.value)
   } else {
-    props.loadGroupData(vGroup.value, true, {
+    _loadGroupData(vGroup.value, true, {
       ...(params?.offset !== undefined ? { offset: params.offset } : {}),
     })
   }
 }
 
+onMounted(async () => {
+  reloadViewDataHook?.on(reloadViewDataHandler)
+})
+
 onBeforeUnmount(async () => {
   reloadViewDataHook?.off(reloadViewDataHandler)
 })
 
-reloadViewDataHook?.on(reloadViewDataHandler)
+watch([() => vGroup.value.key], async (n, o) => {
-
-watch(
-  [() => vGroup.value.key],
-  async (n, o) => {
   if (n !== o) {
-      isViewDataLoading.value = true
+    if (!vGroup.value.nested) {
-      isPaginationLoading.value = true
+      await _loadGroupData(vGroup.value, true)
-
+    } else if (vGroup.value.nested) {
-      if (vGroup.value.nested) {
       await props.loadGroups({}, vGroup.value)
-      } else {
-        await props.loadGroupData(vGroup.value, true)
     }
+  }
+})
 
-      isViewDataLoading.value = false
+onMounted(async () => {
-      isPaginationLoading.value = false
+  if (vGroup.value.root === true) {
+    await props.loadGroups({}, vGroup.value)
   }
-  },
+})
-  { immediate: true },
-)
 
 if (vGroup.value.root === true) provide(ScrollParentInj, wrapper)
 
@@ -231,7 +239,7 @@ const shouldRenderCell = (column) =>
           >
             <a-collapse-panel
               v-for="[i, grp] of Object.entries(vGroup?.children ?? [])"
-              :key="`group-panel-${i}`"
+              :key="`group-panel-${grp.key}`"
               class="!border-1 nc-group rounded-[12px]"
               :class="{ 'mb-4': vGroup.children && +i !== vGroup.children.length - 1 }"
               :style="`background: rgb(${245 - _depth * 10}, ${245 - _depth * 10}, ${245 - _depth * 10})`"
@@ -243,7 +251,7 @@ const shouldRenderCell = (column) =>
                     <span role="img" aria-label="right" class="anticon anticon-right ant-collapse-arrow">
                       <GeneralIcon
                         icon="chevronDown"
-                        :style="`${activeGroups.includes(i) ? 'transform: rotate(360deg)' : 'transform: rotate(270deg)'}`"
+                        :style="`${activeGroups.includes(grp.key) ? 'transform: rotate(360deg)' : 'transform: rotate(270deg)'}`"
                       ></GeneralIcon>
                     </span>
                   </div>
@@ -328,7 +336,7 @@ const shouldRenderCell = (column) =>
                 v-if="!grp.nested && grp.rows"
                 :group="grp"
                 :load-groups="loadGroups"
-                :load-group-data="loadGroupData"
+                :load-group-data="_loadGroupData"
                 :load-group-page="loadGroupPage"
                 :group-wrapper-change-page="groupWrapperChangePage"
                 :row-height="rowHeight"
@@ -345,7 +353,7 @@ const shouldRenderCell = (column) =>
                 v-else
                 :group="grp"
                 :load-groups="loadGroups"
-                :load-group-data="loadGroupData"
+                :load-group-data="_loadGroupData"
                 :load-group-page="loadGroupPage"
                 :group-wrapper-change-page="groupWrapperChangePage"
                 :row-height="rowHeight"

packages/nc-gui/components/smartsheet/grid/index.vue

@@ -20,7 +20,7 @@ import {
   ref,
   useSmartsheetStoreOrThrow,
   useViewData,
-  useViewGroupBy,
+  useViewGroupByOrThrow,
 } from '#imports'
 import type { Row } from '#imports'
 
@@ -166,7 +166,7 @@ const toggleOptimisedQuery = () => {
 }
 
 const { rootGroup, groupBy, isGroupBy, loadGroups, loadGroupData, loadGroupPage, groupWrapperChangePage, redistributeRows } =
-  useViewGroupBy(view, xWhere)
+  useViewGroupByOrThrow()
 
 const coreWrapperRef = ref<HTMLElement>()
 

packages/nc-gui/components/smartsheet/header/Menu.vue

@@ -56,7 +56,7 @@ const showDeleteColumnModal = ref(false)
 
 const { gridViewCols } = useViewColumnsOrThrow()
 
-const { fieldsToGroupBy, groupByLimit } = useViewGroupBy(view)
+const { fieldsToGroupBy, groupByLimit } = useViewGroupByOrThrow(view)
 
 const setAsDisplayValue = async () => {
   try {

packages/nc-gui/components/smartsheet/toolbar/CreateGroupBy.vue

@@ -18,7 +18,7 @@ const meta = inject(MetaInj, ref())
 
 const { showSystemFields, metaColumnById } = useViewColumnsOrThrow()
 
-const { groupBy } = useViewGroupBy(activeView)
+const { groupBy } = useViewGroupByOrThrow()
 
 const options = computed<ColumnType[]>(
   () =>

packages/nc-gui/components/smartsheet/toolbar/GroupByMenu.vue

@@ -22,7 +22,7 @@ const isLocked = inject(IsLockedInj, ref(false))
 
 const { gridViewCols, updateGridViewColumn, metaColumnById, showSystemFields } = useViewColumnsOrThrow()
 
-const { fieldsToGroupBy, groupByLimit } = useViewGroupBy(view)
+const { fieldsToGroupBy, groupByLimit } = useViewGroupByOrThrow()
 
 const { $e } = useNuxtApp()
 

packages/nc-gui/components/tabs/Smartsheet.vue

@@ -54,7 +54,7 @@ const { handleSidebarOpenOnMobileForNonViews } = useConfigStore()
 const { activeTableId } = storeToRefs(useTablesStore())
 
 const { activeView, openedViewsTab, activeViewTitleOrId } = storeToRefs(useViewsStore())
-const { isGallery, isGrid, isForm, isKanban, isLocked, isMap, isCalendar } = useProvideSmartsheetStore(activeView, meta)
+const { isGallery, isGrid, isForm, isKanban, isLocked, isMap, isCalendar, xWhere } = useProvideSmartsheetStore(activeView, meta)
 
 useSqlEditor()
 
@@ -86,6 +86,7 @@ provide(
 )
 
 useProvideViewColumns(activeView, meta, () => reloadViewDataEventHook?.trigger())
+useProvideViewGroupBy(activeView, meta, xWhere)
 
 const grid = ref()
 

packages/nc-gui/composables/useSharedFormViewStore.ts

@@ -217,7 +217,7 @@ const [useProvideSharedFormStore, useSharedFormStore] = useInjectionState((share
         ((column.rqd && !column.cdf) || (column.pk && !(column.ai || column.cdf)) || column.required)
       ) {
         obj.localState[column.title!] = {
-          required: fieldRequired(undefined, column.uidt === UITypes.Checkbox && column.required ? true : false),
+          required: fieldRequired(undefined, !!(column.uidt === UITypes.Checkbox && column.required)),
         }
       } else if (
         isLinksOrLTAR(column) &&

packages/nc-gui/composables/useViewGroupBy.ts

@@ -1,5 +1,5 @@
 import { UITypes } from 'nocodb-sdk'
-import type { ColumnType, LinkToAnotherRecordType, LookupType, SelectOptionsType, ViewType } from 'nocodb-sdk'
+import type { ColumnType, LinkToAnotherRecordType, LookupType, SelectOptionsType, TableType, ViewType } from 'nocodb-sdk'
 import type { Ref } from 'vue'
 import { message } from 'ant-design-vue'
 import { extractSdkResponseErrorMsg } from '../utils'
@@ -17,7 +17,13 @@ import type { Group, GroupNestedIn, Row } from '#imports'
 
 const excludedGroupingUidt = [UITypes.Attachment, UITypes.QrCode, UITypes.Barcode]
 
-export const useViewGroupBy = (view: Ref<ViewType | undefined>, where?: ComputedRef<string | undefined>) => {
+const [useProvideViewGroupBy, useViewGroupBy] = useInjectionState(
+  (
+    view: Ref<ViewType | undefined>,
+    meta: Ref<TableType | undefined> | ComputedRef<TableType | undefined>,
+    where?: ComputedRef<string | undefined>,
+    isPublic = false,
+  ) => {
     const groupByLimit: number = 3
 
     const { api } = useApi()
@@ -28,8 +34,6 @@ export const useViewGroupBy = (view: Ref<ViewType | undefined>, where?: Computed
 
     const { sharedView, fetchSharedViewData } = useSharedView()
 
-  const meta = inject(MetaInj)
-
     const { gridViewCols } = useViewColumnsOrThrow()
 
     const { getMeta } = useMetas()
@@ -60,8 +64,6 @@ export const useViewGroupBy = (view: Ref<ViewType | undefined>, where?: Computed
 
     const { sorts, nestedFilters } = useSmartsheetStoreOrThrow()
 
-  const isPublic = inject(IsPublicInj, ref(false))
-
     const reloadViewDataHook = inject(ReloadViewDataHookInj, createEventHook())
 
     const groupByGroupLimit = computed(() => {
@@ -232,11 +234,11 @@ export const useViewGroupBy = (view: Ref<ViewType | undefined>, where?: Computed
         const nestedWhere = calculateNestedWhere(group.nestedIn, where?.value)
         if (!groupby || !groupby.column.title) return
 
-      if (isPublic.value && !sharedView.value?.uuid) {
+        if (isPublic && !sharedView.value?.uuid) {
           return
         }
 
-      const response = !isPublic.value
+        const response = !isPublic
           ? await api.dbViewRow.groupBy('noco', base.value.id, view.value.fk_model_id, view.value.id, {
               offset: ((group.paginationData.page ?? 0) - 1) * (group.paginationData.pageSize ?? groupByGroupLimit.value),
               limit: group.paginationData.pageSize ?? groupByGroupLimit.value,
@@ -361,7 +363,7 @@ export const useViewGroupBy = (view: Ref<ViewType | undefined>, where?: Computed
           where: `${nestedWhere}`,
         }
 
-      const response = !isPublic.value
+        const response = !isPublic
           ? await api.dbViewRow.list('noco', base.value.id, view.value.fk_model_id, view.value.id, {
               ...query,
               ...params,
@@ -419,12 +421,10 @@ export const useViewGroupBy = (view: Ref<ViewType | undefined>, where?: Computed
         if (groupBy.value.length > 0) {
           rootGroup.value.paginationData = { page: 1, pageSize: groupByGroupLimit.value }
           rootGroup.value.column = {} as any
-        await loadGroups()
           refreshNested()
           nextTick(() => reloadViewDataHook?.trigger())
         }
       },
-    { immediate: true },
     )
 
     const findGroupByNestedIn = (nestedIn: GroupNestedIn[], group?: Group, nestLevel = 0): Group => {
@@ -541,12 +541,11 @@ export const useViewGroupBy = (view: Ref<ViewType | undefined>, where?: Computed
       }
     }
 
-  onMounted(async () => {
+    watch([() => view?.value?.id, () => meta.value?.columns], async ([newViewId]) => {
-    await loadAllowedLookups()
+      // reload only if view belongs to current table
-  })
+      if (newViewId && view.value?.fk_model_id === meta.value?.id) {
-
-  watch(meta, async () => {
         await loadAllowedLookups()
+      }
     })
 
     return {
@@ -561,4 +560,14 @@ export const useViewGroupBy = (view: Ref<ViewType | undefined>, where?: Computed
       groupWrapperChangePage,
       redistributeRows,
     }
+  },
+  'useViewGroupBy',
+)
+
+export { useProvideViewGroupBy }
+
+export function useViewGroupByOrThrow() {
+  const viewColumns = useViewGroupBy()
+  if (viewColumns == null) throw new Error('Please call `useProvideViewGroupBy` on the appropriate parent component')
+  return viewColumns
 }

tests/playwright/tests/db/general/toolbarOperations.spec.ts

@@ -230,7 +230,7 @@ test.describe('Toolbar operations (GRID)', () => {
     });
     await toolbar.clickFilter();
 
-    await dashboard.grid.groupPage.openGroup({ indexMap: [1, 0] });
+    await dashboard.grid.groupPage.openGroup({ indexMap: [2, 0] });
 
     await dashboard.grid.groupPage.verifyGroupHeader({
       indexMap: [1, 0],
@@ -377,16 +377,14 @@ test.describe('Toolbar operations (GRID)', () => {
     });
     await toolbar.clickFilter();
 
-    await dashboard.grid.groupPage.openGroup({ indexMap: [5, 0, 0] });
-
     await dashboard.grid.groupPage.verifyGroupHeader({
-      indexMap: [5, 0, 0],
+      indexMap: [4, 0, 0],
       count: 2,
       title: 'ReleaseYear',
     });
 
     await dashboard.grid.groupPage.verifyGroup({
-      indexMap: [5, 0, 0],
+      indexMap: [4, 0, 0],
       value: '2006',
     });