From 6b70d683eb270e481683f7d67319518047b45671 Mon Sep 17 00:00:00 2001 From: Ramesh Mane <101566080+rameshmane7218@users.noreply.github.com> Date: Thu, 6 Jun 2024 19:34:20 +0530 Subject: [PATCH] fix(nc-gui): form field validation issue if column title contains `.` (#8657) * fix(nc-gui): form field validation issue if column title contains . * chore(nc-gui): lint * test(nc-gui): add `.` included column name in form validation test * test: remove only from test * fix(nc-gui): pr review changes --- .../nc-gui/components/smartsheet/Form.vue | 12 ++++--- .../column/UITypesOptionsWithSearch.vue | 3 +- .../components/smartsheet/grid/Table.vue | 2 +- .../nc-gui/composables/useFormViewStore.ts | 29 ++++++++++++--- .../composables/useSharedFormViewStore.ts | 35 ++++++++++++++++--- .../[typeOrId]/form/[viewId]/index/index.vue | 16 ++++++--- .../[typeOrId]/form/[viewId]/index/survey.vue | 15 +++++--- packages/nc-gui/utils/formValidations.ts | 15 ++++++++ .../playwright/pages/Dashboard/Form/index.ts | 4 +-- .../tests/db/views/viewForm.spec.ts | 8 ++--- 10 files changed, 107 insertions(+), 32 deletions(-) diff --git a/packages/nc-gui/components/smartsheet/Form.vue b/packages/nc-gui/components/smartsheet/Form.vue index d025838c99..4a634dc796 100644 --- a/packages/nc-gui/components/smartsheet/Form.vue +++ b/packages/nc-gui/components/smartsheet/Form.vue @@ -85,6 +85,7 @@ const { validateInfos, validate, clearValidate, + fieldMappings, } = useProvideFormViewStore(meta, view, formViewData, updateFormView, isEditable) const { preFillFormSearchParams } = storeToRefs(useViewsStore()) @@ -198,9 +199,9 @@ async function submitForm() { } try { - await validate([...Object.keys(formState.value)]) + await validate(Object.keys(formState.value).map((title) => fieldMappings.value[title])) } catch (e: any) { - if (e.errorFields.length) { + if (e?.errorFields?.length) { message.error(t('msg.error.someOfTheRequiredFieldsAreEmpty')) return } @@ -586,7 +587,7 @@ watch( updatePreFillFormSearchParams() try { - await validate([...Object.keys(formState.value)]) + await validate(Object.keys(formState.value).map((title) => fieldMappings.value[title])) } catch {} }, { @@ -1103,9 +1104,10 @@ useEventListener(
-import { UITypes } from 'nocodb-sdk' -import { UITypesName } from 'nocodb-sdk' +import { UITypes, UITypesName } from 'nocodb-sdk' const props = defineProps<{ options: typeof uiTypes diff --git a/packages/nc-gui/components/smartsheet/grid/Table.vue b/packages/nc-gui/components/smartsheet/grid/Table.vue index 37f986c8bb..70bc84c7d2 100644 --- a/packages/nc-gui/components/smartsheet/grid/Table.vue +++ b/packages/nc-gui/components/smartsheet/grid/Table.vue @@ -2772,7 +2772,7 @@ onKeyStroke('ArrowDown', onDown) } .nc-grid-add-new-row { - :deep(.ant-btn.ant-dropdown-trigger.ant-btn-icon-only){ + :deep(.ant-btn.ant-dropdown-trigger.ant-btn-icon-only) { @apply !flex items-center justify-center; } } diff --git a/packages/nc-gui/composables/useFormViewStore.ts b/packages/nc-gui/composables/useFormViewStore.ts index 26b1c03b8b..a9d66ca981 100644 --- a/packages/nc-gui/composables/useFormViewStore.ts +++ b/packages/nc-gui/composables/useFormViewStore.ts @@ -40,10 +40,19 @@ const [useProvideFormViewStore, useFormViewStore] = useInjectionState( return null }) + const fieldMappings = computed(() => { + const uniqueFieldNames: Set = new Set() + + return visibleColumns.value.reduce((acc, c) => { + acc[c.title] = getValidFieldName(c.title, uniqueFieldNames) + return acc + }, {} as Record) + }) + const validators = computed(() => { const rulesObj: Record = {} - if (!visibleColumns.value) return rulesObj + if (!visibleColumns.value || !Object.keys(fieldMappings.value).length) return rulesObj for (const column of visibleColumns.value) { let rules: RuleObject[] = [ @@ -73,19 +82,30 @@ const [useProvideFormViewStore, useFormViewStore] = useInjectionState( rules = [...rules, ...additionalRules] if (rules.length) { - rulesObj[column.title!] = rules + rulesObj[fieldMappings.value[column.title!]] = rules } } return rulesObj }) + const fieldMappingFormState = computed(() => { + if (!Object.keys(fieldMappings.value).length) return {} + + return Object.keys(formState.value).reduce((acc, key) => { + acc[fieldMappings.value[key]] = formState.value[key] + return acc + }, {} as Record) + }) + // Form field validation - const { validate, validateInfos, clearValidate } = useForm(formState, validators) + const { validate, validateInfos, clearValidate } = useForm(fieldMappingFormState, validators) const validateActiveField = async (col: ColumnType) => { + if (!col.title) return + try { - await validate(col.title) + await validate(fieldMappings.value[col.title]) } catch {} } @@ -134,6 +154,7 @@ const [useProvideFormViewStore, useFormViewStore] = useInjectionState( validate, validateInfos, clearValidate, + fieldMappings, } }, 'form-view-store', diff --git a/packages/nc-gui/composables/useSharedFormViewStore.ts b/packages/nc-gui/composables/useSharedFormViewStore.ts index f39b4a04e6..e68be73575 100644 --- a/packages/nc-gui/composables/useSharedFormViewStore.ts +++ b/packages/nc-gui/composables/useSharedFormViewStore.ts @@ -187,10 +187,19 @@ const [useProvideSharedFormStore, useSharedFormStore] = useInjectionState((share } } + const fieldMappings = computed(() => { + const uniqueFieldNames: Set = new Set() + + return formColumns.value.reduce((acc, c) => { + acc[c.title!] = getValidFieldName(c.title!, uniqueFieldNames) + return acc + }, {} as Record) + }) + const validators = computed(() => { const rulesObj: Record = {} - if (!formColumns.value) return rulesObj + if (!formColumns.value || !Object.keys(fieldMappings.value).length) return rulesObj for (const column of formColumns.value) { let rules: RuleObject[] = [ @@ -220,7 +229,7 @@ const [useProvideSharedFormStore, useSharedFormStore] = useInjectionState((share rules = [...rules, ...additionalRules] if (rules.length) { - rulesObj[column.title!] = rules + rulesObj[fieldMappings.value[column.title!]] = rules } } @@ -228,7 +237,19 @@ const [useProvideSharedFormStore, useSharedFormStore] = useInjectionState((share }) const validationFieldState = computed(() => { - return { ...formState.value, ...additionalState.value } + if (!Object.keys(fieldMappings.value).length) return {} + + const fieldMappingFormState = Object.keys(formState.value).reduce((acc, key) => { + acc[fieldMappings.value[key]] = formState.value[key] + return acc + }, {} as Record) + + const fieldMappingAdditionalState = Object.keys(additionalState.value).reduce((acc, key) => { + acc[fieldMappings.value[key]] = additionalState.value[key] + return acc + }, {} as Record) + + return { ...fieldMappingFormState, ...fieldMappingAdditionalState } }) const { validate, validateInfos, clearValidate } = useForm(validationFieldState, validators) @@ -254,7 +275,10 @@ const [useProvideSharedFormStore, useSharedFormStore] = useInjectionState((share handleAddMissingRequiredFieldDefaultState() try { - await validate([...Object.keys(formState.value), ...Object.keys(additionalState.value)]) + await validate([ + ...Object.keys(formState.value).map((title) => fieldMappings.value[title]), + ...Object.keys(additionalState.value).map((title) => fieldMappings.value[title]), + ]) return true } catch (e: any) { if (e.errorFields.length) { @@ -571,7 +595,7 @@ const [useProvideSharedFormStore, useSharedFormStore] = useInjectionState((share additionalState, async () => { try { - await validate(Object.keys(additionalState.value)) + await validate(Object.keys(additionalState.value).map((title) => fieldMappings.value[title])) } catch {} }, { @@ -606,6 +630,7 @@ const [useProvideSharedFormStore, useSharedFormStore] = useInjectionState((share additionalState, isRequired, handleAddMissingRequiredFieldDefaultState, + fieldMappings, } }, 'shared-form-view-store') diff --git a/packages/nc-gui/pages/index/[typeOrId]/form/[viewId]/index/index.vue b/packages/nc-gui/pages/index/[typeOrId]/form/[viewId]/index/index.vue index e5695aab78..f93f44aef3 100644 --- a/packages/nc-gui/pages/index/[typeOrId]/form/[viewId]/index/index.vue +++ b/packages/nc-gui/pages/index/[typeOrId]/form/[viewId]/index/index.vue @@ -17,6 +17,7 @@ const { progress, validateInfos, validate, + fieldMappings, } = useSharedFormStoreOrThrow() const { isMobileMode } = storeToRefs(useConfigStore()) @@ -175,7 +176,12 @@ const onDecode = async (scannedCodeValue: string) => {
-
+
{{ field.label || field.title }} @@ -196,9 +202,10 @@ const onDecode = async (scannedCodeValue: string) => { { :read-only="field?.read_only" @update:model-value=" () => { - validate(field.title) + validate(fieldMappings[field.title]) } " /> @@ -317,6 +324,7 @@ const onDecode = async (scannedCodeValue: string) => { } } } + :deep(.ant-form-item-has-error .ant-select:not(.ant-select-disabled) .ant-select-selector) { border: none !important; } diff --git a/packages/nc-gui/pages/index/[typeOrId]/form/[viewId]/index/survey.vue b/packages/nc-gui/pages/index/[typeOrId]/form/[viewId]/index/survey.vue index 646d728fbd..58497eb2a3 100644 --- a/packages/nc-gui/pages/index/[typeOrId]/form/[viewId]/index/survey.vue +++ b/packages/nc-gui/pages/index/[typeOrId]/form/[viewId]/index/survey.vue @@ -31,6 +31,7 @@ const { clearValidate, isRequired, handleAddMissingRequiredFieldDefaultState, + fieldMappings, } = useSharedFormStoreOrThrow() const { isMobileMode } = storeToRefs(useConfigStore()) @@ -77,7 +78,7 @@ const field = computed(() => formColumns.value?.[index.value]) const fieldHasError = computed(() => { if (field.value?.title) { - return validateInfos[field.value.title].validateStatus === 'error' + return validateInfos[fieldMappings.value[field.value.title]]?.validateStatus === 'error' } return false @@ -111,7 +112,7 @@ function animate(target: AnimationTarget) { const validateField = async (title: string) => { try { - await validate(title) + await validate(fieldMappings.value[title]) return true } catch (_e: any) { @@ -394,8 +395,13 @@ onMounted(() => { - - + + { } } } - :deep(.ant-form-item-has-error .ant-select:not(.ant-select-disabled) .ant-select-selector) { border: none !important; } diff --git a/packages/nc-gui/utils/formValidations.ts b/packages/nc-gui/utils/formValidations.ts index 0e29d3c271..f8331cfa19 100644 --- a/packages/nc-gui/utils/formValidations.ts +++ b/packages/nc-gui/utils/formValidations.ts @@ -140,3 +140,18 @@ export const extractFieldValidator = (_validators: Validation[], element: Column return rules } + + + +export const getValidFieldName = (title: string, uniqueFieldNames: Set) => { + title = title.replace(/\./g, '_') + let counter = 1 + + let newTitle = title + while (uniqueFieldNames.has(newTitle)) { + newTitle = `${title}_${counter}` + counter++ + } + uniqueFieldNames.add(newTitle) + return newTitle +} \ No newline at end of file diff --git a/tests/playwright/pages/Dashboard/Form/index.ts b/tests/playwright/pages/Dashboard/Form/index.ts index 3135243c2c..7bcffffeed 100644 --- a/tests/playwright/pages/Dashboard/Form/index.ts +++ b/tests/playwright/pages/Dashboard/Form/index.ts @@ -221,7 +221,7 @@ export class FormPage extends BasePage { getVisibleField({ title }: { title: string }) { return this.get() - .locator(`.nc-form-drag-${title.replace(' ', '')}`) + .locator(`[data-testid="nc-form-fields"][data-title="${title}"]`) .locator('[data-testid="nc-form-input-label"]'); } @@ -661,7 +661,7 @@ export class FormPage extends BasePage { async getFormFieldErrors({ title }: { title: string }) { // ant-form-item-explain - const field = this.get().locator(`.nc-form-drag-${title.replace(' ', '')}`); + const field = this.get().locator(`[data-testid="nc-form-fields"][data-title="${title}"]`); await field.scrollIntoViewIfNeeded(); const fieldErrorEl = field.locator('.ant-form-item-explain'); diff --git a/tests/playwright/tests/db/views/viewForm.spec.ts b/tests/playwright/tests/db/views/viewForm.spec.ts index e43a59cabc..c6fa42d7f2 100644 --- a/tests/playwright/tests/db/views/viewForm.spec.ts +++ b/tests/playwright/tests/db/views/viewForm.spec.ts @@ -610,8 +610,8 @@ test.describe('Form view: field validation', () => { ] : [ { - column_name: 'SingleLineText', - title: 'SingleLineText', + column_name: 'SingleLine.Text', + title: 'SingleLine.Text', uidt: UITypes.SingleLineText, }, { @@ -672,7 +672,7 @@ test.describe('Form view: field validation', () => { }); // 1. - await form.selectVisibleField({ title: 'SingleLineText' }); + await form.selectVisibleField({ title: 'SingleLine.Text' }); await form.addCustomValidation({ type: StringValidationType.MinLength, value: '2', index: 0 }); await form.addCustomValidation({ type: StringValidationType.MaxLength, value: '4', index: 1 }); @@ -782,7 +782,7 @@ test.describe('Form view: field validation', () => { await form.addCustomValidation({ type: StringValidationType.StartsWith, value: 'https://', index: 0 }); const validatorFillDetails = { - SingleLineText: [ + 'SingleLine.Text': [ { type: UITypes.SingleLineText, fillValue: 's',