Merge pull request #5328 from nocodb/enhancement/form-validation

enhancement: validations

packages/nc-gui/components/account/UsersModal.vue

@@ -4,6 +4,7 @@ import type { OrgUserReqType } from 'nocodb-sdk'
 import {
   Form,
   computed,
+  emailValidator,
   extractSdkResponseErrorMsg,
   message,
   ref,
@@ -11,7 +12,6 @@ import {
   useDashboard,
   useI18n,
   useNuxtApp,
-  validateEmail,
 } from '#imports'
 import type { User } from '~/lib'
 import { Role } from '~/lib'
@@ -44,24 +44,10 @@ const usersData = $ref<Users>({ emails: '', role: Role.OrgLevelViewer, invitatio
 const formRef = ref()
 
 const useForm = Form.useForm
+
 const validators = computed(() => {
   return {
-    emails: [
-      {
-        validator: (rule: any, value: string, callback: (errMsg?: string) => void) => {
-          if (!value || value.length === 0) {
-            callback('Email is required')
-            return
-          }
-          const invalidEmails = (value || '').split(/\s*,\s*/).filter((e: string) => !validateEmail(e))
-          if (invalidEmails.length > 0) {
-            callback(`${invalidEmails.length > 1 ? ' Invalid emails:' : 'Invalid email:'} ${invalidEmails.join(', ')} `)
-          } else {
-            callback()
-          }
-        },
-      },
-    ],
+    emails: [emailValidator],
   }
 })
 

packages/nc-gui/components/cell/Email.vue

@@ -1,28 +1,53 @@
 <script lang="ts" setup>
 import type { VNodeRef } from '@vue/runtime-core'
-import { EditModeInj, computed, inject, useVModel, validateEmail } from '#imports'
+import { EditModeInj, IsSurveyFormInj, computed, inject, useI18n, validateEmail } from '#imports'
 
 interface Props {
   modelValue: string | null | undefined
 }
 
-interface Emits {
-  (event: 'update:modelValue', model: string): void
-}
+const { modelValue: value } = defineProps<Props>()
 
-const props = defineProps<Props>()
+const emit = defineEmits(['update:modelValue'])
 
-const emits = defineEmits<Emits>()
+const { t } = useI18n()
 
 const { showNull } = useGlobal()
 
-const editEnabled = inject(EditModeInj)
+const editEnabled = inject(EditModeInj)!
+
+const column = inject(ColumnInj)!
 
-const vModel = useVModel(props, 'modelValue', emits)
+// Used in the logic of when to display error since we are not storing the email if it's not valid
+const localState = ref(value)
+
+const isSurveyForm = inject(IsSurveyFormInj, ref(false))
+
+const vModel = computed({
+  get: () => value,
+  set: (val) => {
+    localState.value = val
+    if (!parseProp(column.value.meta)?.validate || (val && validateEmail(val)) || !val || isSurveyForm.value) {
+      emit('update:modelValue', val)
+    }
+  },
+})
 
 const validEmail = computed(() => vModel.value && validateEmail(vModel.value))
 
 const focus: VNodeRef = (el) => (el as HTMLInputElement)?.focus()
+
+watch(
+  () => editEnabled.value,
+  () => {
+    if (parseProp(column.value.meta)?.validate && !editEnabled.value && localState.value && !validateEmail(localState.value)) {
+      message.error(t('msg.error.invalidEmail'))
+      localState.value = undefined
+      return
+    }
+    localState.value = value
+  },
+)
 </script>
 
 <template>
@@ -30,7 +55,7 @@ const focus: VNodeRef = (el) => (el as HTMLInputElement)?.focus()
     v-if="editEnabled"
     :ref="focus"
     v-model="vModel"
-    class="outline-none text-sm px-2"
+    class="w-full outline-none text-sm px-2"
     @blur="editEnabled = false"
     @keydown.down.stop
     @keydown.left.stop

packages/nc-gui/components/cell/Url.vue

@@ -4,6 +4,7 @@ import {
   CellUrlDisableOverlayInj,
   ColumnInj,
   EditModeInj,
+  IsSurveyFormInj,
   computed,
   inject,
   isValidURL,
@@ -36,11 +37,13 @@ const disableOverlay = inject(CellUrlDisableOverlayInj, ref(false))
 // Used in the logic of when to display error since we are not storing the url if it's not valid
 const localState = ref(value)
 
+const isSurveyForm = inject(IsSurveyFormInj, ref(false))
+
 const vModel = computed({
   get: () => value,
   set: (val) => {
     localState.value = val
-    if (!parseProp(column.value.meta)?.validate || (val && isValidURL(val)) || !val) {
+    if (!parseProp(column.value.meta)?.validate || (val && isValidURL(val)) || !val || isSurveyForm.value) {
       emit('update:modelValue', val)
     }
   },
@@ -119,7 +122,7 @@ watch(
 
     <div v-if="column.meta?.validate && !isValid && value?.length && !editEnabled" class="mr-1 w-1/10">
       <a-tooltip placement="top">
-        <template #title> Invalid URL </template>
+        <template #title> {{ t('msg.error.invalidURL') }} </template>
         <div class="flex flex-row items-center">
           <MiCircleWarning class="text-red-400 h-4" />
         </div>

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

@@ -8,6 +8,7 @@ import {
   IsFormInj,
   IsLockedInj,
   IsPublicInj,
+  IsSurveyFormInj,
   ReadonlyInj,
   computed,
   inject,
@@ -66,7 +67,7 @@ const column = toRef(props, 'column')
 
 const active = toRef(props, 'active', false)
 
-const readOnly = toRef(props, 'readOnly', undefined)
+const readOnly = toRef(props, 'readOnly', false)
 
 provide(ColumnInj, column)
 
@@ -84,6 +85,8 @@ const isPublic = inject(IsPublicInj, ref(false))
 
 const isLocked = inject(IsLockedInj, ref(false))
 
+const isSurveyForm = inject(IsSurveyFormInj, ref(false))
+
 const { currentRow } = useSmartsheetRowStoreOrThrow()
 
 const { sqlUis } = storeToRefs(useProject())
@@ -158,6 +161,7 @@ const onContextmenu = (e: MouseEvent) => {
       `nc-cell-${(column?.uidt || 'default').toLowerCase()}`,
       { 'text-blue-600': isPrimary(column) && !props.virtual && !isForm },
       { 'nc-grid-numeric-cell': isGrid && !isForm && isNumericField },
+      { 'h-[40px]': !props.editEnabled && isForm && !isSurveyForm },
     ]"
     @keydown.enter.exact="syncAndNavigate(NavigateDir.NEXT, $event)"
     @keydown.shift.enter.exact="syncAndNavigate(NavigateDir.PREV, $event)"

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

@@ -97,6 +97,8 @@ const submitted = ref(false)
 
 const activeRow = ref('')
 
+const editEnabled = ref<boolean[]>([])
+
 const { t } = useI18n()
 
 const { betaFeatureToggleState } = useBetaFeatureToggle()
@@ -283,6 +285,8 @@ function setFormData() {
     .sort((a, b) => a.order - b.order)
     .map((c) => ({ ...c, required: !!c.required }))
 
+  editEnabled.value = new Array(localColumns.value.length).fill(false)
+
   systemFieldsIds.value = getSystemColumns(col).map((c) => c.fk_column_id)
 
   hiddenColumns.value = col.filter(
@@ -727,7 +731,10 @@ watch(view, (nextView) => {
                       :class="`nc-form-input-${element.title.replaceAll(' ', '')}`"
                       :data-testid="`nc-form-input-${element.title.replaceAll(' ', '')}`"
                       :column="element"
-                      :edit-enabled="true"
+                      :edit-enabled="editEnabled[index]"
+                      @click="editEnabled[index] = true"
+                      @cancel="editEnabled[index] = false"
+                      @update:edit-enabled="editEnabled[index] = $event"
                       @click.stop.prevent
                     />
                   </a-form-item>

packages/nc-gui/components/tabs/auth/user-management/UsersModal.vue

@@ -4,6 +4,7 @@ import type { ProjectUserReqType } from 'nocodb-sdk'
 import {
   Form,
   computed,
+  emailValidator,
   extractSdkResponseErrorMsg,
   message,
   onMounted,
@@ -17,7 +18,6 @@ import {
   useI18n,
   useNuxtApp,
   useProject,
-  validateEmail,
 } from '#imports'
 import type { User } from '~/lib'
 import { ProjectRole } from '~/lib'
@@ -54,24 +54,10 @@ let usersData = $ref<Users>({ emails: undefined, role: ProjectRole.Viewer, invit
 const formRef = ref()
 
 const useForm = Form.useForm
+
 const validators = computed(() => {
   return {
-    emails: [
-      {
-        validator: (rule: any, value: string, callback: (errMsg?: string) => void) => {
-          if (!value || value.length === 0) {
-            callback('Email is required')
-            return
-          }
-          const invalidEmails = (value || '').split(/\s*,\s*/).filter((e: string) => !validateEmail(e))
-          if (invalidEmails.length > 0) {
-            callback(`${invalidEmails.length > 1 ? ' Invalid emails:' : 'Invalid email:'} ${invalidEmails.join(', ')} `)
-          } else {
-            callback()
-          }
-        },
-      },
-    ],
+    emails: [emailValidator],
   }
 })
 

packages/nc-gui/context/index.ts

@@ -14,6 +14,7 @@ export const PaginationDataInj: InjectionKey<ReturnType<typeof useViewData>['pag
   Symbol('pagination-data-injection')
 export const ChangePageInj: InjectionKey<ReturnType<typeof useViewData>['changePage']> = Symbol('pagination-data-injection')
 export const IsFormInj: InjectionKey<Ref<boolean>> = Symbol('is-form-injection')
+export const IsSurveyFormInj: InjectionKey<Ref<boolean>> = Symbol('is-survey-form-injection')
 export const IsGridInj: InjectionKey<Ref<boolean>> = Symbol('is-grid-injection')
 export const IsGalleryInj: InjectionKey<Ref<boolean>> = Symbol('is-gallery-injection')
 export const IsKanbanInj: InjectionKey<Ref<boolean>> = Symbol('is-kanban-injection')

packages/nc-gui/lang/en.json

@@ -698,6 +698,7 @@
         "allowedSpecialCharList": "Allowed special character list"
       },
       "invalidURL": "Invalid URL",
+      "invalidEmail": "Invalid Email",
       "internalError": "Some internal error occurred",
       "templateGeneratorNotFound": "Template Generator cannot be found!",
       "fileUploadFailed": "Failed to upload file",

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

@@ -27,6 +27,8 @@ const scannerIsReady = ref(false)
 
 const showCodeScannerOverlay = ref(false)
 
+const editEnabled = ref<boolean[]>([])
+
 const onLoaded = async () => {
   scannerIsReady.value = true
 }
@@ -161,7 +163,10 @@ const onDecode = async (scannedCodeValue: string) => {
                         :data-testid="`nc-form-input-cell-${field.label || field.title}`"
                         :class="`nc-form-input-${field.title?.replaceAll(' ', '')}`"
                         :column="field"
-                        :edit-enabled="true"
+                        :edit-enabled="editEnabled[index]"
+                        @click="editEnabled[index] = true"
+                        @cancel="editEnabled[index] = false"
+                        @update:edit-enabled="editEnabled[index] = $event"
                       />
                       <a-button
                         v-if="field.enable_scanner"

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

@@ -4,15 +4,19 @@ import { RelationTypes, UITypes, isVirtualCol } from 'nocodb-sdk'
 import { SwipeDirection, breakpointsTailwind } from '@vueuse/core'
 import {
   DropZoneRef,
+  IsSurveyFormInj,
   computed,
+  isValidURL,
   onKeyStroke,
   onMounted,
   provide,
   ref,
   useBreakpoints,
+  useI18n,
   usePointerSwipe,
   useSharedFormStoreOrThrow,
   useStepper,
+  validateEmail,
 } from '#imports'
 
 enum TransitionDirection {
@@ -32,6 +36,8 @@ const { md } = useBreakpoints(breakpointsTailwind)
 const { v$, formState, formColumns, submitForm, submitted, secondsRemain, sharedFormView, sharedViewMeta, onReset } =
   useSharedFormStoreOrThrow()
 
+const { t } = useI18n()
+
 const isTransitioning = ref(false)
 
 const transitionName = ref<TransitionDirection>(TransitionDirection.Left)
@@ -40,10 +46,14 @@ const animationTarget = ref<AnimationTarget>(AnimationTarget.ArrowRight)
 
 const isAnimating = ref(false)
 
+const editEnabled = ref<boolean[]>([])
+
 const el = ref<HTMLDivElement>()
 
 provide(DropZoneRef, el)
 
+provide(IsSurveyFormInj, ref(true))
+
 const transitionDuration = computed(() => sharedViewMeta.value.transitionDuration || 50)
 
 const steps = computed(() => {
@@ -64,6 +74,8 @@ const { index, goToPrevious, goToNext, isFirst, isLast, goTo } = useStepper(step
 
 const field = computed(() => formColumns.value?.[index.value])
 
+const columnValidationError = ref(false)
+
 function isRequired(column: ColumnType, required = false) {
   let columnObj = column
   if (
@@ -105,7 +117,29 @@ function animate(target: AnimationTarget) {
   }, transitionDuration.value / 2)
 }
 
+async function validateColumn() {
+  const f = field.value!
+  if (parseProp(f.meta)?.validate && formState.value[f.title!]) {
+    if (f.uidt === UITypes.Email) {
+      if (!validateEmail(formState.value[f.title!])) {
+        columnValidationError.value = true
+        message.error(t('msg.error.invalidEmail'))
+        return false
+      }
+    } else if (f.uidt === UITypes.URL) {
+      if (!isValidURL(formState.value[f.title!])) {
+        columnValidationError.value = true
+        message.error(t('msg.error.invalidURL'))
+        return false
+      }
+    }
+  }
+  return true
+}
+
 async function goNext(animationTarget?: AnimationTarget) {
+  columnValidationError.value = false
+
   if (isLast.value || submitted.value) return
 
   if (!field.value || !field.value.title) return
@@ -117,6 +151,8 @@ async function goNext(animationTarget?: AnimationTarget) {
     if (!isValid) return
   }
 
+  if (!(await validateColumn())) return
+
   animate(animationTarget || AnimationTarget.ArrowRight)
 
   setTimeout(
@@ -159,9 +195,8 @@ function resetForm() {
   goTo(steps.value[0])
 }
 
-function submit() {
-  if (submitted.value) return
-
+async function submit() {
+  if (submitted.value || !(await validateColumn())) return
   submitForm()
 }
 
@@ -177,7 +212,7 @@ onKeyStroke(['Enter', 'Space'], () => {
   if (isLast.value) {
     submit()
   } else {
-    goNext(AnimationTarget.OkButton)
+    goNext(AnimationTarget.OkButton, true)
   }
 })
 
@@ -263,7 +298,10 @@ onMounted(() => {
                 class="nc-input"
                 :data-testid="`nc-survey-form__input-${field.title.replaceAll(' ', '')}`"
                 :column="field"
-                :edit-enabled="true"
+                :edit-enabled="editEnabled[index]"
+                @click="editEnabled[index] = true"
+                @cancel="editEnabled[index] = false"
+                @update:edit-enabled="editEnabled[index] = $event"
               />
 
               <div class="flex flex-col gap-2 text-slate-500 dark:text-slate-300 text-[0.75rem] my-2 px-1">
@@ -315,7 +353,7 @@ onMounted(() => {
                     class="bg-opacity-100 scaling-btn flex items-center gap-1"
                     data-testid="nc-survey-form__btn-next"
                     :class="[
-                      v$.localState[field.title]?.$error ? 'after:!bg-gray-100 after:!ring-red-500' : '',
+                      v$.localState[field.title]?.$error || columnValidationError ? 'after:!bg-gray-100 after:!ring-red-500' : '',
                       animationTarget === AnimationTarget.OkButton && isAnimating
                         ? 'transform translate-y-[2px] translate-x-[2px] after:(!ring !ring-accent !ring-opacity-100)'
                         : '',
@@ -327,7 +365,10 @@ onMounted(() => {
                     </Transition>
 
                     <Transition name="slide-right" mode="out-in">
-                      <MdiCloseCircleOutline v-if="v$.localState[field.title]?.$error" class="text-red-500 md:text-md" />
+                      <MdiCloseCircleOutline
+                        v-if="v$.localState[field.title]?.$error || columnValidationError"
+                        class="text-red-500 md:text-md"
+                      />
                       <MdiCheck v-else class="text-white md:text-md" />
                     </Transition>
                   </button>

packages/nc-gui/utils/validation.ts

@@ -173,3 +173,20 @@ export const extraParameterValidator = {
     })
   },
 }
+
+export const emailValidator = {
+  validator: (_: unknown, value: string) => {
+    return new Promise((resolve, reject) => {
+      if (!value || value.length === 0) {
+        return reject(new Error('Email is required'))
+      }
+      const invalidEmails = (value || '').split(/\s*,\s*/).filter((e: string) => !validateEmail(e))
+      if (invalidEmails.length > 0) {
+        return reject(
+          new Error(`${invalidEmails.length > 1 ? ' Invalid emails:' : 'Invalid email:'} ${invalidEmails.join(', ')} `),
+        )
+      }
+      return resolve(true)
+    })
+  },
+}

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

@@ -64,7 +64,7 @@ export class FormPage extends BasePage {
   }
 
   getFormFieldsRequired() {
-    return this.get().locator('[data-testid="nc-form-input-required"]');
+    return this.get().locator('[data-testid="nc-form-input-required"] + button');
   }
 
   getFormFieldsInputLabel() {
@@ -153,6 +153,9 @@ export class FormPage extends BasePage {
 
   async fillForm(param: { field: string; value: string }[]) {
     for (let i = 0; i < param.length; i++) {
+      await this.get()
+        .locator(`[data-testid="nc-form-input-${param[i].field.replace(' ', '')}"]`)
+        .click();
       await this.get()
         .locator(`[data-testid="nc-form-input-${param[i].field.replace(' ', '')}"] >> input`)
         .fill(param[i].value);
@@ -177,9 +180,7 @@ export class FormPage extends BasePage {
     await this.getFormFieldsInputLabel().fill(label);
     await this.getFormFieldsInputHelpText().fill(helpText);
     if (required) {
-      await this.get()
-        .locator(`.nc-form-drag-${field.replace(' ', '')}`)
-        .click();
+      await this.getFormFieldsRequired().click();
     }
     await this.formHeading.click();
   }

tests/playwright/pages/Dashboard/SurveyForm/index.ts

@@ -66,6 +66,7 @@ export class SurveyFormPage extends BasePage {
       // press enter key
       await this.get().locator(`[data-testid="nc-survey-form__input-${param.fieldLabel}"] >> input`).press('Enter');
     } else if (param.type === 'DateTime') {
+      await this.get().locator(`[data-testid="nc-survey-form__input-${param.fieldLabel}"] >> input`).click();
       const modal = await this.rootPage.locator('.nc-picker-datetime');
       await expect(modal).toBeVisible();
       await modal.locator('.ant-picker-now-btn').click();