Browse Source

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
pull/8671/head
Ramesh Mane 6 months ago committed by GitHub
parent
commit
6b70d683eb
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
  1. 12
      packages/nc-gui/components/smartsheet/Form.vue
  2. 3
      packages/nc-gui/components/smartsheet/column/UITypesOptionsWithSearch.vue
  3. 2
      packages/nc-gui/components/smartsheet/grid/Table.vue
  4. 29
      packages/nc-gui/composables/useFormViewStore.ts
  5. 35
      packages/nc-gui/composables/useSharedFormViewStore.ts
  6. 16
      packages/nc-gui/pages/index/[typeOrId]/form/[viewId]/index/index.vue
  7. 15
      packages/nc-gui/pages/index/[typeOrId]/form/[viewId]/index/survey.vue
  8. 15
      packages/nc-gui/utils/formValidations.ts
  9. 4
      tests/playwright/pages/Dashboard/Form/index.ts
  10. 8
      tests/playwright/tests/db/views/viewForm.spec.ts

12
packages/nc-gui/components/smartsheet/Form.vue

@ -85,6 +85,7 @@ const {
validateInfos, validateInfos,
validate, validate,
clearValidate, clearValidate,
fieldMappings,
} = useProvideFormViewStore(meta, view, formViewData, updateFormView, isEditable) } = useProvideFormViewStore(meta, view, formViewData, updateFormView, isEditable)
const { preFillFormSearchParams } = storeToRefs(useViewsStore()) const { preFillFormSearchParams } = storeToRefs(useViewsStore())
@ -198,9 +199,9 @@ async function submitForm() {
} }
try { try {
await validate([...Object.keys(formState.value)]) await validate(Object.keys(formState.value).map((title) => fieldMappings.value[title]))
} catch (e: any) { } catch (e: any) {
if (e.errorFields.length) { if (e?.errorFields?.length) {
message.error(t('msg.error.someOfTheRequiredFieldsAreEmpty')) message.error(t('msg.error.someOfTheRequiredFieldsAreEmpty'))
return return
} }
@ -586,7 +587,7 @@ watch(
updatePreFillFormSearchParams() updatePreFillFormSearchParams()
try { try {
await validate([...Object.keys(formState.value)]) await validate(Object.keys(formState.value).map((title) => fieldMappings.value[title]))
} catch {} } catch {}
}, },
{ {
@ -1103,9 +1104,10 @@ useEventListener(
<div class="nc-form-field-body"> <div class="nc-form-field-body">
<div class="mt-2"> <div class="mt-2">
<a-form-item <a-form-item
:name="element.title" v-if="fieldMappings[element.title]"
:name="fieldMappings[element.title]"
class="!my-0 nc-input-required-error nc-form-input-item" class="!my-0 nc-input-required-error nc-form-input-item"
v-bind="validateInfos[element.title]" v-bind="validateInfos[fieldMappings[element.title]]"
> >
<LazySmartsheetDivDataCell class="relative" @click.stop> <LazySmartsheetDivDataCell class="relative" @click.stop>
<LazySmartsheetVirtualCell <LazySmartsheetVirtualCell

3
packages/nc-gui/components/smartsheet/column/UITypesOptionsWithSearch.vue

@ -1,6 +1,5 @@
<script lang="ts" setup> <script lang="ts" setup>
import { UITypes } from 'nocodb-sdk' import { UITypes, UITypesName } from 'nocodb-sdk'
import { UITypesName } from 'nocodb-sdk'
const props = defineProps<{ const props = defineProps<{
options: typeof uiTypes options: typeof uiTypes

2
packages/nc-gui/components/smartsheet/grid/Table.vue

@ -2772,7 +2772,7 @@ onKeyStroke('ArrowDown', onDown)
} }
.nc-grid-add-new-row { .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; @apply !flex items-center justify-center;
} }
} }

29
packages/nc-gui/composables/useFormViewStore.ts

@ -40,10 +40,19 @@ const [useProvideFormViewStore, useFormViewStore] = useInjectionState(
return null return null
}) })
const fieldMappings = computed(() => {
const uniqueFieldNames: Set<string> = new Set()
return visibleColumns.value.reduce((acc, c) => {
acc[c.title] = getValidFieldName(c.title, uniqueFieldNames)
return acc
}, {} as Record<string, string>)
})
const validators = computed(() => { const validators = computed(() => {
const rulesObj: Record<string, RuleObject[]> = {} const rulesObj: Record<string, RuleObject[]> = {}
if (!visibleColumns.value) return rulesObj if (!visibleColumns.value || !Object.keys(fieldMappings.value).length) return rulesObj
for (const column of visibleColumns.value) { for (const column of visibleColumns.value) {
let rules: RuleObject[] = [ let rules: RuleObject[] = [
@ -73,19 +82,30 @@ const [useProvideFormViewStore, useFormViewStore] = useInjectionState(
rules = [...rules, ...additionalRules] rules = [...rules, ...additionalRules]
if (rules.length) { if (rules.length) {
rulesObj[column.title!] = rules rulesObj[fieldMappings.value[column.title!]] = rules
} }
} }
return rulesObj 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<string, any>)
})
// Form field validation // Form field validation
const { validate, validateInfos, clearValidate } = useForm(formState, validators) const { validate, validateInfos, clearValidate } = useForm(fieldMappingFormState, validators)
const validateActiveField = async (col: ColumnType) => { const validateActiveField = async (col: ColumnType) => {
if (!col.title) return
try { try {
await validate(col.title) await validate(fieldMappings.value[col.title])
} catch {} } catch {}
} }
@ -134,6 +154,7 @@ const [useProvideFormViewStore, useFormViewStore] = useInjectionState(
validate, validate,
validateInfos, validateInfos,
clearValidate, clearValidate,
fieldMappings,
} }
}, },
'form-view-store', 'form-view-store',

35
packages/nc-gui/composables/useSharedFormViewStore.ts

@ -187,10 +187,19 @@ const [useProvideSharedFormStore, useSharedFormStore] = useInjectionState((share
} }
} }
const fieldMappings = computed(() => {
const uniqueFieldNames: Set<string> = new Set()
return formColumns.value.reduce((acc, c) => {
acc[c.title!] = getValidFieldName(c.title!, uniqueFieldNames)
return acc
}, {} as Record<string, string>)
})
const validators = computed(() => { const validators = computed(() => {
const rulesObj: Record<string, RuleObject[]> = {} const rulesObj: Record<string, RuleObject[]> = {}
if (!formColumns.value) return rulesObj if (!formColumns.value || !Object.keys(fieldMappings.value).length) return rulesObj
for (const column of formColumns.value) { for (const column of formColumns.value) {
let rules: RuleObject[] = [ let rules: RuleObject[] = [
@ -220,7 +229,7 @@ const [useProvideSharedFormStore, useSharedFormStore] = useInjectionState((share
rules = [...rules, ...additionalRules] rules = [...rules, ...additionalRules]
if (rules.length) { 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(() => { 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<string, any>)
const fieldMappingAdditionalState = Object.keys(additionalState.value).reduce((acc, key) => {
acc[fieldMappings.value[key]] = additionalState.value[key]
return acc
}, {} as Record<string, any>)
return { ...fieldMappingFormState, ...fieldMappingAdditionalState }
}) })
const { validate, validateInfos, clearValidate } = useForm(validationFieldState, validators) const { validate, validateInfos, clearValidate } = useForm(validationFieldState, validators)
@ -254,7 +275,10 @@ const [useProvideSharedFormStore, useSharedFormStore] = useInjectionState((share
handleAddMissingRequiredFieldDefaultState() handleAddMissingRequiredFieldDefaultState()
try { 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 return true
} catch (e: any) { } catch (e: any) {
if (e.errorFields.length) { if (e.errorFields.length) {
@ -571,7 +595,7 @@ const [useProvideSharedFormStore, useSharedFormStore] = useInjectionState((share
additionalState, additionalState,
async () => { async () => {
try { try {
await validate(Object.keys(additionalState.value)) await validate(Object.keys(additionalState.value).map((title) => fieldMappings.value[title]))
} catch {} } catch {}
}, },
{ {
@ -606,6 +630,7 @@ const [useProvideSharedFormStore, useSharedFormStore] = useInjectionState((share
additionalState, additionalState,
isRequired, isRequired,
handleAddMissingRequiredFieldDefaultState, handleAddMissingRequiredFieldDefaultState,
fieldMappings,
} }
}, 'shared-form-view-store') }, 'shared-form-view-store')

16
packages/nc-gui/pages/index/[typeOrId]/form/[viewId]/index/index.vue

@ -17,6 +17,7 @@ const {
progress, progress,
validateInfos, validateInfos,
validate, validate,
fieldMappings,
} = useSharedFormStoreOrThrow() } = useSharedFormStoreOrThrow()
const { isMobileMode } = storeToRefs(useConfigStore()) const { isMobileMode } = storeToRefs(useConfigStore())
@ -175,7 +176,12 @@ const onDecode = async (scannedCodeValue: string) => {
<a-form :model="formState"> <a-form :model="formState">
<div class="nc-form h-full"> <div class="nc-form h-full">
<div class="flex flex-col gap-3 md:gap-6"> <div class="flex flex-col gap-3 md:gap-6">
<div v-for="(field, index) in formColumns" :key="index" class="flex flex-col gap-2"> <div
v-for="(field, index) in formColumns"
:key="index"
class="flex flex-col gap-2"
:data-testid="`nc-shared-form-item-${field.title?.replace(' ', '')}`"
>
<div class="nc-form-column-label text-sm font-semibold text-gray-800"> <div class="nc-form-column-label text-sm font-semibold text-gray-800">
<span> <span>
{{ field.label || field.title }} {{ field.label || field.title }}
@ -196,9 +202,10 @@ const onDecode = async (scannedCodeValue: string) => {
<NcTooltip :disabled="!field?.read_only"> <NcTooltip :disabled="!field?.read_only">
<template #title> {{ $t('activity.preFilledFields.lockedFieldTooltip') }} </template> <template #title> {{ $t('activity.preFilledFields.lockedFieldTooltip') }} </template>
<a-form-item <a-form-item
:name="field.title" v-if="field.title && fieldMappings[field.title]"
:name="fieldMappings[field.title]"
class="!my-0 nc-input-required-error" class="!my-0 nc-input-required-error"
v-bind="validateInfos[field.title]" v-bind="validateInfos[fieldMappings[field.title]]"
> >
<LazySmartsheetDivDataCell class="flex relative"> <LazySmartsheetDivDataCell class="flex relative">
<LazySmartsheetVirtualCell <LazySmartsheetVirtualCell
@ -225,7 +232,7 @@ const onDecode = async (scannedCodeValue: string) => {
:read-only="field?.read_only" :read-only="field?.read_only"
@update:model-value=" @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) { :deep(.ant-form-item-has-error .ant-select:not(.ant-select-disabled) .ant-select-selector) {
border: none !important; border: none !important;
} }

15
packages/nc-gui/pages/index/[typeOrId]/form/[viewId]/index/survey.vue

@ -31,6 +31,7 @@ const {
clearValidate, clearValidate,
isRequired, isRequired,
handleAddMissingRequiredFieldDefaultState, handleAddMissingRequiredFieldDefaultState,
fieldMappings,
} = useSharedFormStoreOrThrow() } = useSharedFormStoreOrThrow()
const { isMobileMode } = storeToRefs(useConfigStore()) const { isMobileMode } = storeToRefs(useConfigStore())
@ -77,7 +78,7 @@ const field = computed(() => formColumns.value?.[index.value])
const fieldHasError = computed(() => { const fieldHasError = computed(() => {
if (field.value?.title) { if (field.value?.title) {
return validateInfos[field.value.title].validateStatus === 'error' return validateInfos[fieldMappings.value[field.value.title]]?.validateStatus === 'error'
} }
return false return false
@ -111,7 +112,7 @@ function animate(target: AnimationTarget) {
const validateField = async (title: string) => { const validateField = async (title: string) => {
try { try {
await validate(title) await validate(fieldMappings.value[title])
return true return true
} catch (_e: any) { } catch (_e: any) {
@ -394,8 +395,13 @@ onMounted(() => {
<NcTooltip :disabled="!field?.read_only"> <NcTooltip :disabled="!field?.read_only">
<template #title> {{ $t('activity.preFilledFields.lockedFieldTooltip') }} </template> <template #title> {{ $t('activity.preFilledFields.lockedFieldTooltip') }} </template>
<a-form-item :name="field.title" class="!my-0 nc-input-required-error" v-bind="validateInfos[field.title]"> <a-form-item
<SmartsheetDivDataCell v-if="field.title" class="relative nc-form-data-cell" @click.stop="handleFocus"> v-if="field.title && fieldMappings[field.title]"
:name="fieldMappings[field.title]"
class="!my-0 nc-input-required-error"
v-bind="validateInfos[fieldMappings[field.title]]"
>
<SmartsheetDivDataCell class="relative nc-form-data-cell" @click.stop="handleFocus">
<LazySmartsheetVirtualCell <LazySmartsheetVirtualCell
v-if="isVirtualCol(field)" v-if="isVirtualCol(field)"
v-model="formState[field.title]" v-model="formState[field.title]"
@ -564,7 +570,6 @@ onMounted(() => {
} }
} }
} }
:deep(.ant-form-item-has-error .ant-select:not(.ant-select-disabled) .ant-select-selector) { :deep(.ant-form-item-has-error .ant-select:not(.ant-select-disabled) .ant-select-selector) {
border: none !important; border: none !important;
} }

15
packages/nc-gui/utils/formValidations.ts

@ -140,3 +140,18 @@ export const extractFieldValidator = (_validators: Validation[], element: Column
return rules return rules
} }
export const getValidFieldName = (title: string, uniqueFieldNames: Set<string>) => {
title = title.replace(/\./g, '_')
let counter = 1
let newTitle = title
while (uniqueFieldNames.has(newTitle)) {
newTitle = `${title}_${counter}`
counter++
}
uniqueFieldNames.add(newTitle)
return newTitle
}

4
tests/playwright/pages/Dashboard/Form/index.ts

@ -221,7 +221,7 @@ export class FormPage extends BasePage {
getVisibleField({ title }: { title: string }) { getVisibleField({ title }: { title: string }) {
return this.get() 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"]'); .locator('[data-testid="nc-form-input-label"]');
} }
@ -661,7 +661,7 @@ export class FormPage extends BasePage {
async getFormFieldErrors({ title }: { title: string }) { async getFormFieldErrors({ title }: { title: string }) {
// ant-form-item-explain // 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(); await field.scrollIntoViewIfNeeded();
const fieldErrorEl = field.locator('.ant-form-item-explain'); const fieldErrorEl = field.locator('.ant-form-item-explain');

8
tests/playwright/tests/db/views/viewForm.spec.ts

@ -610,8 +610,8 @@ test.describe('Form view: field validation', () => {
] ]
: [ : [
{ {
column_name: 'SingleLineText', column_name: 'SingleLine.Text',
title: 'SingleLineText', title: 'SingleLine.Text',
uidt: UITypes.SingleLineText, uidt: UITypes.SingleLineText,
}, },
{ {
@ -672,7 +672,7 @@ test.describe('Form view: field validation', () => {
}); });
// 1. // 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.MinLength, value: '2', index: 0 });
await form.addCustomValidation({ type: StringValidationType.MaxLength, value: '4', index: 1 }); 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 }); await form.addCustomValidation({ type: StringValidationType.StartsWith, value: 'https://', index: 0 });
const validatorFillDetails = { const validatorFillDetails = {
SingleLineText: [ 'SingleLine.Text': [
{ {
type: UITypes.SingleLineText, type: UITypes.SingleLineText,
fillValue: 's', fillValue: 's',

Loading…
Cancel
Save