Refactor Form Submission and Field Management Logic

- Simplified the constructor in StoreFormSubmissionJob for improved readability.
- Enhanced the storeFile method to handle null and empty values more robustly, ensuring better validation of file names.
- Updated StorageFileNameParser to return null for empty file names, improving error handling.
- Refactored OpenForm.vue to optimize form initialization and prevent unnecessary reinitializations, enhancing performance.
- Introduced new methods for managing field groups and preventing recursive updates in OpenForm.vue, improving the overall user experience.
- Enhanced FormEditorSidebar.vue to utilize Pinia store for better state management and clarity.
- Improved FormFieldEdit.vue to prevent page jumps during field editing, ensuring a smoother user experience.
- Added new getters in working_form.js for better page management and navigation within forms.

These changes aim to enhance the maintainability and performance of the form handling logic, providing a more efficient and user-friendly experience.

api/app/Jobs/Form/StoreFormSubmissionJob.php

@@ -176,9 +176,9 @@ class StoreFormSubmissionJob implements ShouldQueue
      * - file_name-{uuid}.{ext}
      * - {uuid}
      */
-    private function storeFile(?string $value)
+    private function storeFile($value, ?bool $isPublic = null)
     {
-        if ($value == null) {
+        if (is_null($value) || empty($value)) {
             return null;
         }
 
@@ -196,6 +196,9 @@ class StoreFormSubmissionJob implements ShouldQueue
         }
 
         $fileNameParser = StorageFileNameParser::parse($value);
+        if (!$fileNameParser || !$fileNameParser->uuid) {
+            return null;
+        }
 
         // Make sure we retrieve the file in tmp storage, move it to persistent
         $fileName = PublicFormController::TMP_FILE_UPLOAD_PATH . '/' . $fileNameParser->uuid;

api/app/Service/Storage/StorageFileNameParser.php

@@ -33,7 +33,7 @@ class StorageFileNameParser
             $fileName = substr($this->fileName, 0, 50) . '_' . $this->uuid . '.' . $this->extension;
             $fileName = preg_replace('#\p{C}+#u', '', $fileName); // avoid CorruptedPathDetected exceptions
 
-            return S3KeyCleaner::sanitize($fileName);
+            return $fileName ? S3KeyCleaner::sanitize($fileName) : null;
         }
 
         return $this->uuid;

client/components/open/forms/OpenForm.vue

@@ -200,11 +200,10 @@ export default {
 
   data() {
     return {
-      // Page index
-      /**
-       * Used to force refresh components by changing their keys
-       */
       isAutoSubmit: false,
+      isInitialLoad: true,
+      // Flag to prevent recursion in field group updates
+      _isUpdatingFieldGroups: false,
     }
   },
 
@@ -337,16 +336,24 @@ export default {
   },
 
   watch: {
-    form: {
+    // Monitor only critical changes that require full reinitialization
-      deep: true,
+    'form.database_id': function() {
-      handler() {
+      // Only reinitialize when database changes
       this.initForm()
-      }
     },
-    fields: {
+    'fields.length': function() {
+      // Only reinitialize when fields are added or removed
+      this._updateFieldGroups()
+    },
+    // Watch for changes to individual field properties
+    'fields': {
       deep: true,
-      handler() {
+      handler(newFields, oldFields) {
-        this.initForm()
+        // Skip update if only triggered by internal fieldGroups changes
+        if (this._isUpdatingFieldGroups) return
+        
+        // Safely update field groups without causing recursive updates
+        this._updateFieldGroups()
       }
     },
     dataFormValue: {
@@ -460,21 +467,48 @@ export default {
      * Form initialization
      */
     async initForm() {
+      // Only do a full initialization when necessary
+      // Store current page index and form data to avoid overwriting existing values
+      const currentFormData = this.dataForm ? clonedeep(this.dataForm.data()) : {}
+
+      // Handle special cases first
       if (this.defaultDataForm) {
+        // If we have default data form, initialize with that
         await nextTick(() => {
           this.dataForm.resetAndFill(this.defaultDataForm)
         })
+        this._updateFieldGroups()
         return
       }
 
-      if (await this.tryInitFormFromEditableSubmission()) return
+      // Initialize the field groups without resetting form data
-      if (this.tryInitFormFromPendingSubmission()) return
+      this._updateFieldGroups()
 
-      await nextTick(() => {
+      // Check if we need to handle form submission states
-        this.formPageIndex = 0
+      if (await this.checkForEditableSubmission()) {
-        this.initFormWithDefaultValues()
+        return
-      })
+      }
+
+      if (this.checkForPendingSubmission()) {
+        return
+      }
+
+      // Standard initialization with default values
+      this.initFormWithDefaultValues(currentFormData)
     },
+    
+    checkForEditableSubmission() {
+      return this.tryInitFormFromEditableSubmission()
+    },
+
+    checkForPendingSubmission() {
+      if (this.tryInitFormFromPendingSubmission()) {
+        this._updateFieldGroups()
+        return true
+      }
+      return false
+    },
+    
     async tryInitFormFromEditableSubmission() {
       if (this.isPublicFormPage && this.form.editable_submissions) {
         const submissionId = useRoute().query?.submission_id
@@ -489,6 +523,7 @@ export default {
       }
       return false
     },
+    
     tryInitFormFromPendingSubmission() {
       if (this.isPublicFormPage && this.form.auto_save) {
         const pendingData = this.pendingSubmission.get()
@@ -500,6 +535,7 @@ export default {
       }
       return false
     },
+    
     updatePendingDataFields(pendingData) {
       this.fields.forEach(field => {
         if (field.type === 'date' && field.prefill_today) {
@@ -507,17 +543,26 @@ export default {
         }
       })
     },
-    initFormWithDefaultValues() {
+    
-      const formData = {}
+    initFormWithDefaultValues(currentFormData = {}) {
+      // Only set page 0 on first load, otherwise maintain current position
+      if (this.formPageIndex === undefined || this.isInitialLoad) {
+        this.formPageIndex = 0
+        this.isInitialLoad = false
+      }
+      
+      // Initialize form data with default values
+      const formData = { ...currentFormData }
       const urlPrefill = this.isPublicFormPage ? new URLSearchParams(window.location.search) : null
 
       this.fields.forEach(field => {
-        if (field.type.startsWith('nf-')) return
+        if (field.type.startsWith('nf-') && !['nf-page-body-input', 'nf-page-logo', 'nf-page-cover'].includes(field.type)) return
 
         this.handleUrlPrefill(field, formData, urlPrefill)
         this.handleDefaultPrefill(field, formData)
       })
       
+      // Reset form with new data
       this.dataForm.resetAndFill(formData)
     },
     handleUrlPrefill(field, formData, urlPrefill) {
@@ -620,6 +665,59 @@ export default {
       }
       this.formPageIndex = this.fieldGroups.length - 1
     },
+    
+    // New method for updating field groups
+    updateFieldGroups() {
+      if (!this.fields || this.fields.length === 0) return
+
+      // Preserve the current page index if possible
+      const currentPageIndex = this.formPageIndex
+      
+      // Use a local variable instead of directly modifying computed property
+      // We'll use this to determine totalPages and currentPageIndex
+      const calculatedGroups = this.fields.reduce((groups, field, index) => {
+        // If the field is a page break, start a new group
+        if (field.type === 'nf-page-break' && index !== 0) {
+          groups.push([])
+        }
+        // Add the field to the current group
+        if (groups.length === 0) groups.push([])
+        groups[groups.length - 1].push(field)
+        return groups
+      }, [])
+
+      // If we don't have any groups (shouldn't happen), create a default group
+      if (calculatedGroups.length === 0) {
+        calculatedGroups.push([])
+      }
+
+      // Update page navigation
+      const totalPages = calculatedGroups.length
+      
+      // Try to maintain the current page index if valid
+      if (currentPageIndex !== undefined && currentPageIndex < totalPages) {
+        this.formPageIndex = currentPageIndex
+      } else {
+        this.formPageIndex = 0
+      }
+
+      // Force a re-render of the component, which will update fieldGroups computed property
+      this.$forceUpdate()
+    },
+
+    // Helper method to prevent recursive updates
+    _updateFieldGroups() {
+      // Set flag to prevent recursive updates
+      this._isUpdatingFieldGroups = true
+      
+      // Call the actual update method
+      this.updateFieldGroups()
+      
+      // Clear the flag after a short delay to allow Vue to process the update
+      this.$nextTick(() => {
+        this._isUpdatingFieldGroups = false
+      })
+    }
   }
 }
 </script>

client/components/open/forms/components/form-components/FormEditorSidebar.vue

@@ -25,6 +25,7 @@
 <script>
 import { computed } from "vue"
 import { useWorkingFormStore } from "../../../../../stores/working_form"
+import { storeToRefs } from 'pinia'
 import EditorRightSidebar from "../../../editors/EditorRightSidebar.vue"
 import FormFieldEdit from "../../fields/FormFieldEdit.vue"
 import AddFormBlock from "./AddFormBlock.vue"
@@ -35,13 +36,20 @@ export default {
   props: {},
   setup() {
     const workingFormStore = useWorkingFormStore()
+    
+    const editFieldIndex = computed(() => workingFormStore.selectedFieldIndex)
+    const form = storeToRefs(workingFormStore).content
+    const showEditFieldSidebar = computed(() => workingFormStore.showEditFieldSidebar)
+    const showAddFieldSidebar = computed(() => workingFormStore.showAddFieldSidebar)
+    
+    // The store now handles setting the page automatically in openSettingsForField
+    
     return {
       workingFormStore,
-      editFieldIndex: computed(() => workingFormStore.selectedFieldIndex),
+      editFieldIndex,
-      showEditFieldSidebar: computed(
+      form,
-        () => workingFormStore.showEditFieldSidebar,
+      showEditFieldSidebar,
-      ),
+      showAddFieldSidebar,
-      showAddFieldSidebar: computed(() => workingFormStore.showAddFieldSidebar),
     }
   },
   data() {
@@ -51,15 +59,6 @@ export default {
     isOpen() {
       return this.form !== null && (this.showEditFieldSidebar || this.showAddFieldSidebar)
     },
-    form: {
-      get() {
-        return this.workingFormStore.content
-      },
-      /* We add a setter */
-      set(value) {
-        this.workingFormStore.set(value)
-      },
-    },
   },
   watch: {},
   mounted() {},

client/components/open/forms/fields/FormFieldEdit.vue

@@ -124,6 +124,14 @@ const field = computed(() => {
     : null
 })
 
+// Only set the page once when the component is mounted
+// This prevents page jumps when editing field properties
+onMounted(() => {
+  if (selectedFieldIndex.value !== null) {
+    workingFormStore.setPageForField(selectedFieldIndex.value)
+  }
+})
+
 const isBlockField = computed(() => {
   return field.value && field.value.type.startsWith('nf')
 })
@@ -147,6 +155,8 @@ function removeBlock() {
 }
 
 function closeSidebar() {
+  // Explicitly clear the selected field index to prevent issues with subsequent block additions
+  workingFormStore.selectedFieldIndex = null
   workingFormStore.closeEditFieldSidebar()
 }
 

client/stores/working_form.js

@@ -17,6 +17,80 @@ export const useWorkingFormStore = defineStore("working_form", {
     blockForm: null,
     draggingNewBlock: false,
   }),
+  getters: {
+    // Get all blocks/properties in the form
+    formBlocks() {
+      return this.content?.properties || []
+    },
+    
+    // Get page break indices to determine page boundaries
+    pageBreakIndices() {
+      if (!this.content?.properties || this.content.properties.length === 0) return []
+      
+      // Find all indices of page break blocks
+      const indices = []
+      this.content.properties.forEach((prop, index) => {
+        if (prop.type === 'nf-page-break' && !prop.hidden) {
+          indices.push(index)
+        }
+      })
+      
+      return indices
+    },
+    
+    // Calculate page boundaries (start/end indices for each page)
+    pageBoundaries() {
+      if (!this.content?.properties || this.content.properties.length === 0) {
+        return [{ start: 0, end: 0 }]
+      }
+      
+      const boundaries = []
+      const breaks = this.pageBreakIndices
+      
+      // If no page breaks, return a single page boundary
+      if (breaks.length === 0) {
+        return [{ 
+          start: 0, 
+          end: this.formBlocks.length - 1 
+        }]
+      }
+      
+      // First page starts at 0
+      let startIndex = 0
+      
+      // For each page break, create a boundary
+      breaks.forEach(breakIndex => {
+        boundaries.push({
+          start: startIndex,
+          end: breakIndex
+        })
+        startIndex = breakIndex + 1
+      })
+      
+      // Add the last page
+      boundaries.push({
+        start: startIndex,
+        end: this.formBlocks.length - 1
+      })
+      
+      return boundaries
+    },
+    
+    // Get the current page's boundary
+    currentPageBoundary() {
+      return this.pageBoundaries[this.formPageIndex] || { start: 0, end: this.formBlocks.length - 1 }
+    },
+    
+    // Count total pages
+    pageCount() {
+      return this.pageBoundaries.length
+    },
+    
+    // Whether this is the last page
+    isLastPage() {
+      return this.formPageIndex === this.pageCount - 1
+    }
+  },
   actions: {
     set(form) {
       this.content = form
@@ -39,6 +113,19 @@ export const useWorkingFormStore = defineStore("working_form", {
       this.setEditingField(field)
       this.showEditFieldSidebar = true
       this.showAddFieldSidebar = false
+      
+      // Set the page to the one containing this field
+      // But only do this when initially opening settings, not during editing
+      if (typeof field === 'number' || (typeof field === 'object' && field !== null)) {
+        // Only navigate to the field's page if we're newly selecting it
+        // Not if we're just updating an already selected field
+        const previousIndex = this.selectedFieldIndex
+        const currentIndex = this.objectToIndex(field)
+        
+        if (previousIndex !== currentIndex) {
+          this.setPageForField(this.selectedFieldIndex)
+        }
+      }
     },
     closeEditFieldSidebar() {
       this.selectedFieldIndex = null
@@ -69,7 +156,55 @@ export const useWorkingFormStore = defineStore("working_form", {
         type: null,
         name: null,
       })
+    },
 
+    /**
+     * Determine where to insert a new block
+     * @param {number|null} explicitIndex - Optional explicit index to insert at
+     * @returns {number} The index where the block should be inserted
+     */
+    determineInsertIndex(explicitIndex) {
+      // If an explicit index is provided, use that
+      if (explicitIndex !== null && typeof explicitIndex === 'number') {
+        return explicitIndex
+      }
+      
+      // If a field is selected, insert after it
+      // This handles the case when adding from a field's "Add new field" button
+      if (this.selectedFieldIndex !== null && this.selectedFieldIndex !== undefined) {
+        return this.selectedFieldIndex + 1
+      }
+      
+      // Early validation
+      if (!this.content?.properties || this.content.properties.length === 0) {
+        return 0
+      }
+      
+      // Get the current page's boundaries
+      const pageBreaks = this.pageBreakIndices
+      
+      // If no page breaks, insert at the end of the form
+      if (pageBreaks.length === 0) {
+        return this.content.properties.length
+      }
+      
+      // For first page
+      if (this.formPageIndex === 0) {
+        return pageBreaks[0]
+      }
+      
+      // For pages after the first one
+      // Find the end of the current page (the page break index)
+      const currentPageBreakIndex = pageBreaks[this.formPageIndex - 1]
+      const nextPageBreakIndex = pageBreaks[this.formPageIndex] || this.content.properties.length
+      
+      // Insert at the end of the current page, right before the next page break
+      // If this is the last page, insert at the very end
+      if (this.formPageIndex >= pageBreaks.length) {
+        return this.content.properties.length
+      }
+      
+      return nextPageBreakIndex
     },
 
     prefillDefault(data) {
@@ -108,27 +243,16 @@ export const useWorkingFormStore = defineStore("working_form", {
         Object.assign(newBlock, blocksTypes[type].default_values)
       }
 
-      // Insert in right position
+      // Determine the insert index
-      if (
+      const insertIndex = this.determineInsertIndex(index)
-        (this.selectedFieldIndex === null || this.selectedFieldIndex === undefined) &&
+      
-        (index === null || index === undefined)
+      // Insert at the determined position
-      ) {
       const newFields = clonedeep(this.content.properties)
-        newFields.push(newBlock)
+      newFields.splice(insertIndex, 0, newBlock)
       this.content.properties = newFields
+      
       if (openSettings) {
-          this.openSettingsForField(
+        this.openSettingsForField(insertIndex)
-            this.content.properties.length - 1,
-          )
-        }
-      } else {
-        const fieldIndex = typeof index === "number" ? index : this.selectedFieldIndex + 1
-        const newFields = clonedeep(this.content.properties)
-        newFields.splice(fieldIndex, 0, newBlock)
-        this.content.properties = newFields
-        if (openSettings) {
-          this.openSettingsForField(fieldIndex)
-        }
       }
     },
     removeField(field) {
@@ -156,6 +280,45 @@ export const useWorkingFormStore = defineStore("working_form", {
       const field = newFields.splice(oldIndex, 1)[0]
       newFields.splice(newIndex, 0, field)
       this.content.properties = newFields
+    },
+    
+    /**
+     * Find which page a field belongs to and navigate to it
+     * @param {number} fieldIndex - The index of the field to navigate to
+     */
+    setPageForField(fieldIndex) {
+      if (fieldIndex === -1 || fieldIndex === null) return
+      
+      // Early return if no fields or field is out of range
+      if (!this.content?.properties || 
+          this.content.properties.length === 0 || 
+          fieldIndex >= this.content.properties.length) {
+        return
+      }
+      
+      // If there are no page breaks, everything is on page 0
+      if (this.pageBreakIndices.length === 0) {
+        this.formPageIndex = 0
+        return
+      }
+      
+      // Find which page contains this field
+      for (let i = 0; i < this.pageBoundaries.length; i++) {
+        const { start, end } = this.pageBoundaries[i]
+        if (fieldIndex >= start && fieldIndex <= end) {
+          // Only set page if it's different to avoid unnecessary rerenders
+          if (this.formPageIndex !== i) {
+            this.formPageIndex = i
+          }
+          return
+        }
+      }
+      
+      // Fallback to last page if field not found in any boundaries
+      const lastPageIndex = this.pageBoundaries.length - 1
+      if (this.formPageIndex !== lastPageIndex) {
+        this.formPageIndex = lastPageIndex
+      }
     }
   },
   history: {}