From 7577fddcb3cd623e860b2a1aab31defe69f350b4 Mon Sep 17 00:00:00 2001 From: Ramesh Mane <101566080+rameshmane7218@users.noreply.github.com> Date: Thu, 11 Jul 2024 17:06:10 +0530 Subject: [PATCH] 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 --- .../nc-gui/components/nc/PaginationV2.vue | 243 +++++++++--------- .../nc-gui/components/smartsheet/Toolbar.vue | 4 +- .../smartsheet/expanded-form/index.vue | 14 +- .../toolbar/FieldListAutoCompleteDropdown.vue | 20 +- .../toolbar/FieldListWithSearch.vue | 3 +- .../smartsheet/toolbar/FieldsMenu.vue | 10 +- .../smartsheet/toolbar/RowHeight.vue | 4 +- packages/nc-gui/composables/useSharedView.ts | 2 +- packages/nc-gui/composables/useViewColumns.ts | 74 ++++-- packages/nc-gui/lib/types.ts | 1 + .../src/services/public-metas.service.ts | 11 +- 11 files changed, 232 insertions(+), 154 deletions(-) diff --git a/packages/nc-gui/components/nc/PaginationV2.vue b/packages/nc-gui/components/nc/PaginationV2.vue index da04355732..11e58ed169 100644 --- a/packages/nc-gui/components/nc/PaginationV2.vue +++ b/packages/nc-gui/components/nc/PaginationV2.vue @@ -99,136 +99,135 @@ const pageSizeOptions = [ diff --git a/packages/nc-gui/components/smartsheet/Toolbar.vue b/packages/nc-gui/components/smartsheet/Toolbar.vue index b550838b5f..6c4300f4b3 100644 --- a/packages/nc-gui/components/smartsheet/Toolbar.vue +++ b/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() const { width } = useElementSize(containerRef) @@ -49,7 +51,7 @@ const isTab = computed(() => { - + diff --git a/packages/nc-gui/components/smartsheet/expanded-form/index.vue b/packages/nc-gui/components/smartsheet/expanded-form/index.vue index f01ed820b8..57ce30e2fd 100644 --- a/packages/nc-gui/components/smartsheet/expanded-form/index.vue +++ b/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) diff --git a/packages/nc-gui/components/smartsheet/toolbar/FieldListAutoCompleteDropdown.vue b/packages/nc-gui/components/smartsheet/toolbar/FieldListAutoCompleteDropdown.vue index 0b6a6cecd8..54dd63e945 100644 --- a/packages/nc-gui/components/smartsheet/toolbar/FieldListAutoCompleteDropdown.vue +++ b/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(() => ( 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(() => 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 } diff --git a/packages/nc-gui/components/smartsheet/toolbar/FieldListWithSearch.vue b/packages/nc-gui/components/smartsheet/toolbar/FieldListWithSearch.vue index b77432fc0a..fa06ba1618 100644 --- a/packages/nc-gui/components/smartsheet/toolbar/FieldListWithSearch.vue +++ b/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 diff --git a/packages/nc-gui/components/smartsheet/toolbar/FieldsMenu.vue b/packages/nc-gui/components/smartsheet/toolbar/FieldsMenu.vue index 0394dfc4c6..18a5d2ebda 100644 --- a/packages/nc-gui/components/smartsheet/toolbar/FieldsMenu.vue +++ b/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)
@@ -677,7 +677,7 @@ useMenuCloseOnEsc(open) {{ showAllColumns ? $t('general.hideAll') : $t('general.showAll') }} {{ $t('objects.fields').toLowerCase() }} { ;(view.value.view as GridType).row_height = rh open.value = false - } catch (e) { - message.error('There was an error while updating view!') + } catch (e: any) { + message.error((await extractSdkResponseErrorMsg(e)) || 'There was an error while updating view!') } } } diff --git a/packages/nc-gui/composables/useSharedView.ts b/packages/nc-gui/composables/useSharedView.ts index c297d70dac..ff1f77da5c 100644 --- a/packages/nc-gui/composables/useSharedView.ts +++ b/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) } diff --git a/packages/nc-gui/composables/useViewColumns.ts b/packages/nc-gui/composables/useViewColumns.ts index 921cde1d3f..c257b1b0e6 100644 --- a/packages/nc-gui/composables/useViewColumns.ts +++ b/packages/nc-gui/composables/useViewColumns.ts @@ -42,11 +42,9 @@ const [useProvideViewColumns, useViewColumns] = useInjectionState( const { addUndo, defineViewScope } = useUndoRedo() - const isLocalMode = computed( - () => isPublic || !isUIAllowed('viewFieldEdit') || !isUIAllowed('viewFieldEdit') || isSharedBase.value, - ) + const isLocalMode = computed(() => isPublic || !isUIAllowed('viewFieldEdit') || isSharedBase.value) - const localChanges = ref([]) + const localChanges = ref>({}) 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) { - const fieldIndex = fields.value.findIndex((f) => f.fk_column_id === field.fk_column_id) + for (const key in localChanges.value) { + 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>((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) => ({ - ...field, - show: fieldById[field.fk_column_id!]?.show, - })) + 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>((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) => ({ - ...field, - show: fieldById[field.fk_column_id!]?.show, - })) + 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(() => { 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', diff --git a/packages/nc-gui/lib/types.ts b/packages/nc-gui/lib/types.ts index ff3bbbe97b..09f2225374 100644 --- a/packages/nc-gui/lib/types.ts +++ b/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 & { diff --git a/packages/nocodb/src/services/public-metas.service.ts b/packages/nocodb/src/services/public-metas.service.ts index 2cb080c65e..c9f83be6a3 100644 --- a/packages/nocodb/src/services/public-metas.service.ts +++ b/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 ||