Nc fix: prevent commentor/viewer from enabling hidden columns (#8971)

* fix(nc-gui): show proper error msg instead of harcoded text

* fix(nc-gui): disable show field switch is user don't have permission

* fix(nc-gui): hide hidden field for commenter/viewer role

* fix(nc-gui): hide groupby option in local mode

* fix(nc-gui): groupby issue in shared view if grouping field is hidden

* fix(nc-gui): show pagination even if total no pages is 1

* fix(nc-gui): show hidden grouping field in shared view

* chore(nc-gui): type mistake

* fix(nc-gui): show/hide all columns in local mode issue

* fix(nc-gui): pw test fail issue

* fix(nc-gui): hide/show field local mode save issue

packages/nc-gui/components/nc/PaginationV2.vue

@@ -99,7 +99,6 @@ const pageSizeOptions = [
 
 <template>
   <div class="nc-pagination flex flex-row items-center gap-x-0.25">
-    <template v-if="totalPages > 1">
     <component :is="props.firstPageTooltip && mode === 'full' ? NcTooltip : 'div'" v-if="mode === 'full'">
       <template v-if="props.firstPageTooltip" #title>
         {{ props.firstPageTooltip }}
@@ -228,7 +227,7 @@ const pageSizeOptions = [
         <GeneralIcon icon="doubleRightArrow" class="nc-pagination-icon" />
       </NcButton>
     </component>
-    </template>
+
     <div v-if="showSizeChanger && !isMobileMode" class="text-gray-500"></div>
   </div>
 </template>

packages/nc-gui/components/smartsheet/Toolbar.vue

@@ -6,6 +6,8 @@ const { isLeftSidebarOpen } = storeToRefs(useSidebarStore())
 
 const { isViewsLoading } = storeToRefs(useViewsStore())
 
+const { isLocalMode } = useViewColumnsOrThrow()
+
 const containerRef = ref<HTMLElement>()
 
 const { width } = useElementSize(containerRef)
@@ -49,7 +51,7 @@ const isTab = computed(() => {
 
         <LazySmartsheetToolbarColumnFilterMenu v-if="isGrid || isGallery || isKanban || isMap" />
 
-        <LazySmartsheetToolbarGroupByMenu v-if="isGrid" />
+        <LazySmartsheetToolbarGroupByMenu v-if="isGrid && !isLocalMode" />
 
         <LazySmartsheetToolbarSortListMenu v-if="isGrid || isGallery || isKanban" />
       </div>

packages/nc-gui/components/smartsheet/expanded-form/index.vue

@@ -48,6 +48,8 @@ const { copy } = useClipboard()
 
 const { isMobileMode } = useGlobal()
 
+const { fieldsMap, isLocalMode } = useViewColumnsOrThrow()
+
 const { t } = useI18n()
 
 const rowId = toRef(props, 'rowId')
@@ -102,11 +104,19 @@ const fields = computedInject(FieldsInj, (_fields) => {
   return _fields?.value ?? []
 })
 
-const displayField = computed(() => meta.value?.columns?.find((c) => c.pv && fields.value.includes(c)) ?? null)
+const displayField = computed(() => meta.value?.columns?.find((c) => c.pv && fields.value?.includes(c)) ?? null)
 
 const hiddenFields = computed(() => {
   // todo: figure out when meta.value is undefined
-  return (meta.value?.columns ?? []).filter((col) => !fields.value?.includes(col)).filter((col) => !isSystemColumn(col))
+  return (meta.value?.columns ?? [])
+    .filter(
+      (col) =>
+        !fields.value?.includes(col) &&
+        (isLocalMode.value && col?.id && fieldsMap.value[col.id] 
+          ? fieldsMap.value[col.id]?.initialShow
+          : true),
+    )
+    .filter((col) => !isSystemColumn(col))
 })
 
 const showHiddenFields = ref(false)

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

@@ -24,11 +24,20 @@ const localValue = computed({
   set: (val) => emit('update:modelValue', val),
 })
 
-const { showSystemFields, metaColumnById, fieldsMap } = useViewColumnsOrThrow()
+const { showSystemFields, metaColumnById, fieldsMap, isLocalMode } = useViewColumnsOrThrow()
 
 const options = computed<SelectProps['options']>(() =>
   (
     customColumns.value?.filter((c: ColumnType) => {
+      if (
+        isLocalMode.value &&
+        c?.id &&
+        fieldsMap.value[c.id] &&
+        (!fieldsMap.value[c.id]?.initialShow || (!showSystemFields.value && isSystemColumn(metaColumnById?.value?.[c.id!])))
+      ) {
+        return false
+      }
+
       if (isSystemColumn(metaColumnById?.value?.[c.id!])) {
         if (isHiddenCol(c)) {
           /** ignore mm relation column, created by and last modified by system field */
@@ -38,6 +47,15 @@ const options = computed<SelectProps['options']>(() =>
       return true
     }) ||
     meta.value?.columns?.filter((c: ColumnType) => {
+      if (
+        isLocalMode.value &&
+        c?.id &&
+        fieldsMap.value[c.id] &&
+        (!fieldsMap.value[c.id]?.initialShow || (!showSystemFields.value && isSystemColumn(metaColumnById?.value?.[c.id!])))
+      ) {
+        return false
+      }
+
       if (c.uidt === UITypes.Links) {
         return true
       }

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

@@ -15,12 +15,13 @@ const emits = defineEmits<{ selected: [ColumnType] }>()
 
 const { isParentOpen, toolbarMenu, searchInputPlaceholder, selectedOptionId, showSelectedOption } = toRefs(props)
 
-const { fieldsMap } = useViewColumnsOrThrow()
+const { fieldsMap, isLocalMode } = useViewColumnsOrThrow()
 
 const searchQuery = ref('')
 
 const options = computed(() =>
   (props.options || [])
+    .filter((c) => (isLocalMode.value && c?.id && fieldsMap.value[c.id] ? fieldsMap.value[c.id]?.initialShow : true))
     .map((c) => c)
     .sort((field1, field2) => {
       // sort by view column order and keep system columns at the end

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

@@ -29,6 +29,7 @@ const {
   showSystemFields,
   fields,
   filteredFieldList,
+  numberOfHiddenFields,
   filterQuery,
   showAll,
   hideAll,
@@ -37,6 +38,7 @@ const {
   loadViewColumns,
   toggleFieldStyles,
   toggleFieldVisibility,
+  isLocalMode,
 } = useViewColumnsOrThrow()
 
 const { eventBus, isDefaultView } = useSmartsheetStoreOrThrow()
@@ -51,8 +53,6 @@ eventBus.on((event) => {
   }
 })
 
-const numberOfHiddenFields = computed(() => filteredFieldList.value?.filter((field) => !field.show)?.length)
-
 const gridDisplayValueField = computed(() => {
   if (activeView.value?.type !== ViewTypes.GRID && activeView.value?.type !== ViewTypes.CALENDAR) return null
   const pvCol = Object.values(metaColumnById.value)?.find((col) => col?.pv)
@@ -608,7 +608,7 @@ useMenuCloseOnEsc(open)
                   <component :is="iconMap.drag" class="cursor-move !h-3.75 text-gray-600 mr-1" />
                   <div
                     v-e="['a:fields:show-hide']"
-                    class="flex flex-row items-center w-full truncate cursor-pointer ml-1 py-[5px] pr-2"
+                    class="flex flex-row items-center w-full cursor-pointer truncate ml-1 py-[5px] pr-2"
                     @click="
                       () => {
                         field.show = !field.show
@@ -662,7 +662,7 @@ useMenuCloseOnEsc(open)
                       :checked="field.show"
                       :disabled="field.isViewEssentialField"
                       size="xsmall"
-                      @change="$t('a:fields:show-hide')"
+                      @change="$e('a:fields:show-hide')"
                     />
                   </div>
 
@@ -677,7 +677,7 @@ useMenuCloseOnEsc(open)
             {{ showAllColumns ? $t('general.hideAll') : $t('general.showAll') }} {{ $t('objects.fields').toLowerCase() }}
           </NcButton>
           <NcButton
-            v-if="!isPublic"
+            v-if="!isLocalMode"
             class="nc-fields-show-system-fields"
             size="small"
             type="ghost"

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

@@ -72,8 +72,8 @@ const updateRowHeight = async (rh: number, undo = false) => {
       ;(view.value.view as GridType).row_height = rh
 
       open.value = false
-    } catch (e) {
+    } catch (e: any) {
-      message.error('There was an error while updating view!')
+      message.error((await extractSdkResponseErrorMsg(e)) || 'There was an error while updating view!')
     }
   }
 }

packages/nc-gui/composables/useSharedView.ts

@@ -97,7 +97,7 @@ export function useSharedView() {
 
     if (meta.value) {
       meta.value.columns = [...viewMeta.model.columns]
-        .filter((c) => c.show || rangeFields.includes(c.id))
+        .filter((c) => c.show || rangeFields.includes(c.id) || (sharedView.value?.type === ViewTypes.GRID && c?.group_by))
         .map((c) => ({ ...c, order: order++ }))
         .sort((a, b) => a.order - b.order)
     }

packages/nc-gui/composables/useViewColumns.ts

@@ -42,11 +42,9 @@ const [useProvideViewColumns, useViewColumns] = useInjectionState(
 
     const { addUndo, defineViewScope } = useUndoRedo()
 
-    const isLocalMode = computed(
+    const isLocalMode = computed(() => isPublic || !isUIAllowed('viewFieldEdit') || isSharedBase.value)
-      () => isPublic || !isUIAllowed('viewFieldEdit') || !isUIAllowed('viewFieldEdit') || isSharedBase.value,
-    )
 
-    const localChanges = ref<Field[]>([])
+    const localChanges = ref<Record<string, Field>>({})
 
     const isColumnViewEssential = (column: ColumnType) => {
       // TODO: consider at some point ti delegate this via a cleaner design pattern to view specific check logic
@@ -71,6 +69,7 @@ const [useProvideViewColumns, useViewColumns] = useInjectionState(
 
     const loadViewColumns = async () => {
       if (!meta || !view) return
+
       let order = 1
 
       if (view.value?.id) {
@@ -103,15 +102,19 @@ const [useProvideViewColumns, useViewColumns] = useInjectionState(
               aggregation: currentColumnField?.aggregation ?? CommonAggregations.None,
               system: isSystemColumn(metaColumnById?.value?.[currentColumnField.fk_column_id!]),
               isViewEssentialField: isColumnViewEssential(column),
+              initialShow:
+                currentColumnField.show ||
+                isColumnViewEssential(currentColumnField) ||
+                (currentColumnField as GridColumnType)?.group_by,
             }
           })
           .sort((a: Field, b: Field) => a.order - b.order)
 
         if (isLocalMode.value && fields.value) {
-          for (const field of localChanges.value) {
+          for (const key in localChanges.value) {
-            const fieldIndex = fields.value.findIndex((f) => f.fk_column_id === field.fk_column_id)
+            const fieldIndex = fields.value.findIndex((f) => f.fk_column_id === key)
             if (fieldIndex !== undefined && fieldIndex > -1) {
-              fields.value[fieldIndex] = field
+              fields.value[fieldIndex] = localChanges.value[key]
               fields.value = fields.value.sort((a: Field, b: Field) => a.order - b.order)
             }
           }
@@ -133,16 +136,20 @@ const [useProvideViewColumns, useViewColumns] = useInjectionState(
       if (isLocalMode.value) {
         const fieldById = (fields.value || []).reduce<Record<string, any>>((acc, curr) => {
           if (curr.fk_column_id) {
-            curr.show = true
+            curr.show = curr.initialShow ? true : false
             acc[curr.fk_column_id] = curr
           }
           return acc
         }, {})
 
-        fields.value = fields.value?.map((field: Field) => ({
+        fields.value = (fields.value || [])?.map((field: Field) => {
+          const updateField = {
             ...field,
             show: fieldById[field.fk_column_id!]?.show,
-        }))
+          }
+          localChanges.value[field.fk_column_id!] = field
+          return updateField
+        })
 
         meta.value!.columns = meta.value!.columns?.map((column: ColumnType) => {
           if (fieldById[column.id!]) {
@@ -177,16 +184,20 @@ const [useProvideViewColumns, useViewColumns] = useInjectionState(
       if (isLocalMode.value) {
         const fieldById = (fields.value || []).reduce<Record<string, any>>((acc, curr) => {
           if (curr.fk_column_id) {
-            curr.show = !!curr.isViewEssentialField
+            curr.show = !!metaColumnById?.value?.[curr.fk_column_id!]?.pv || !!curr.isViewEssentialField
             acc[curr.fk_column_id] = curr
           }
           return acc
         }, {})
 
-        fields.value = fields.value?.map((field: Field) => ({
+        fields.value = (fields.value || [])?.map((field: Field) => {
+          const updateField = {
             ...field,
             show: fieldById[field.fk_column_id!]?.show,
-        }))
+          }
+          localChanges.value[field.fk_column_id!] = field
+          return updateField
+        })
 
         meta.value!.columns = meta.value!.columns?.map((column: ColumnType) => {
           if (fieldById[column.id!]) {
@@ -237,7 +248,7 @@ const [useProvideViewColumns, useViewColumns] = useInjectionState(
           return column
         })
 
-        localChanges.value.push(field)
+        localChanges.value[field.fk_column_id] = field
       }
 
       if (isUIAllowed('viewFieldEdit')) {
@@ -284,6 +295,10 @@ const [useProvideViewColumns, useViewColumns] = useInjectionState(
     const filteredFieldList = computed(() => {
       return (
         fields.value?.filter((field: Field) => {
+          if (!field.initialShow && isLocalMode.value) {
+            return false
+          }
+
           if (
             metaColumnById?.value?.[field.fk_column_id!]?.pv &&
             (!filterQuery.value || field.title.toLowerCase().includes(filterQuery.value.toLowerCase()))
@@ -305,6 +320,27 @@ const [useProvideViewColumns, useViewColumns] = useInjectionState(
       )
     })
 
+    const numberOfHiddenFields = computed(() => {
+      return (fields.value || [])
+        ?.filter((field: Field) => {
+          if (!field.initialShow && isLocalMode.value) {
+            return false
+          }
+
+          if (metaColumnById?.value?.[field.fk_column_id!]?.pv) {
+            return true
+          }
+
+          // hide system columns if not enabled
+          if (!showSystemFields.value && isSystemColumn(metaColumnById?.value?.[field.fk_column_id!])) {
+            return false
+          }
+
+          return true
+        })
+        .filter((field) => !field.show)?.length
+    })
+
     const sortedAndFilteredFields = computed<ColumnType[]>(() => {
       return (fields?.value
         ?.filter((field: Field) => {
@@ -432,6 +468,7 @@ const [useProvideViewColumns, useViewColumns] = useInjectionState(
       fieldsMap,
       loadViewColumns,
       filteredFieldList,
+      numberOfHiddenFields,
       filterQuery,
       showAll,
       hideAll,
@@ -445,6 +482,7 @@ const [useProvideViewColumns, useViewColumns] = useInjectionState(
       updateGridViewColumn,
       gridViewCols,
       resizingColOldWith,
+      isLocalMode,
     }
   },
   'useViewColumnsOrThrow',

packages/nc-gui/lib/types.ts

@@ -51,6 +51,7 @@ interface Field {
   fk_column_id?: string
   system?: boolean
   isViewEssentialField?: boolean
+  initialShow?: boolean
 }
 
 type Filter = FilterType & {

packages/nocodb/src/services/public-metas.service.ts

@@ -12,7 +12,15 @@ import type {
   LookupColumn,
 } from '~/models';
 import type { NcContext } from '~/interface/config';
-import { Base, BaseUser, Column, Model, Source, View } from '~/models';
+import {
+  Base,
+  BaseUser,
+  Column,
+  GridViewColumn,
+  Model,
+  Source,
+  View,
+} from '~/models';
 import { NcError } from '~/helpers/catchError';
 
 @Injectable()
@@ -71,6 +79,7 @@ export class PublicMetasService {
         if (!column) return false;
 
         return (
+          (c instanceof GridViewColumn && c.group_by) ||
           c.show ||
           (column.rqd && !column.cdf && !column.ai) ||
           column.pk ||