Extract Property Editor into separate module

tests_js/form-builder/handleFieldDroppedToStep.test.js

@@ -46,4 +46,27 @@ describe('FormBuilder#handleFieldDroppedToStep', () => {
     expect(instance.undoStack).toHaveLength(0);
     expect(instance.fields).toHaveLength(0);
   });
+
+  it('reorders this.fields to match the step position when dropped before an existing field, not just appended', () => {
+    const instance = createInstance();
+    instance.fields = [{ field_name: 'existing' }];
+    instance.formSteps[0].fields = ['existing'];
+
+    // Drop at position 0 -> new field belongs before 'existing' in the step.
+    instance.handleFieldDroppedToStep('text', 0, 0);
+
+    expect(instance.formSteps[0].fields[0]).not.toBe('existing');
+    expect(instance.fields.map(f => f.field_name)).toEqual(instance.formSteps[0].fields);
+  });
+
+  it('opens the property editor for the newly-added field even after this.fields gets reordered', () => {
+    const instance = createInstance();
+    instance.fields = [{ field_name: 'existing' }];
+    instance.formSteps[0].fields = ['existing'];
+
+    instance.handleFieldDroppedToStep('text', 0, 0);
+
+    const newFieldIndex = instance.fields.findIndex(f => f.field_name !== 'existing');
+    expect(instance.editField).toHaveBeenCalledWith(newFieldIndex, true);
+  });
 });

tests_js/form-builder/handleFieldMovedToStep.test.js

@@ -0,0 +1,111 @@
+import { afterEach, describe, expect, it, vi } from 'vitest';
+import { FormBuilder } from '../../django_forms_workflows/static/django_forms_workflows/js/form-builder.js';
+import { createBuilderStore } from '../../django_forms_workflows/static/django_forms_workflows/js/form-builder-store.js';
+
+function createInstance({ fields, formSteps }) {
+  const instance = Object.create(FormBuilder.prototype);
+  instance.store = createBuilderStore({ fields, formSteps });
+  instance.renderSingleStep = vi.fn();
+  instance.updatePreview = vi.fn();
+  instance.undoStack = [];
+  instance.redoStack = [];
+  instance.maxUndoSteps = 50;
+  return instance;
+}
+
+// The drag-drop library has already moved the dragged node into the target
+// step's canvas by the time this handler runs; handleFieldMovedToStep reads
+// its position back out of the DOM.
+function buildFieldElement(fieldIndex) {
+  const el = document.createElement('div');
+  el.className = 'field-item';
+  el.dataset.fieldIndex = String(fieldIndex);
+  return el;
+}
+
+afterEach(() => {
+  document.body.innerHTML = '';
+});
+
+describe('FormBuilder#handleFieldMovedToStep', () => {
+  it('moves the field name out of its source step and into the target step at the dropped DOM position', () => {
+    const instance = createInstance({
+      fields: [{ field_name: 'a' }, { field_name: 'b' }, { field_name: 'c' }],
+      formSteps: [{ title: 'Step 1', fields: ['a', 'b'] }, { title: 'Step 2', fields: ['c'] }],
+    });
+    document.body.innerHTML = '<div id="step-canvas-1"></div>';
+    const canvas = document.getElementById('step-canvas-1');
+    const cEl = buildFieldElement(2);
+    const aEl = buildFieldElement(0);
+    canvas.appendChild(cEl);
+    canvas.appendChild(aEl); // 'a' dropped after 'c' in step 2
+
+    instance.handleFieldMovedToStep(aEl, 1);
+
+    expect(instance.formSteps[0].fields).toEqual(['b']);
+    expect(instance.formSteps[1].fields).toEqual(['c', 'a']);
+  });
+
+  it('reorders this.fields to match the new cross-step order (regression: preview used to keep the stale array order)', () => {
+    const instance = createInstance({
+      fields: [{ field_name: 'a' }, { field_name: 'b' }, { field_name: 'c' }],
+      formSteps: [{ title: 'Step 1', fields: ['a', 'b'] }, { title: 'Step 2', fields: ['c'] }],
+    });
+    document.body.innerHTML = '<div id="step-canvas-1"></div>';
+    const canvas = document.getElementById('step-canvas-1');
+    const cEl = buildFieldElement(2);
+    const aEl = buildFieldElement(0);
+    canvas.appendChild(cEl);
+    canvas.appendChild(aEl);
+
+    instance.handleFieldMovedToStep(aEl, 1);
+
+    expect(instance.fields.map(f => f.field_name)).toEqual(['b', 'c', 'a']);
+  });
+
+  it('does nothing when the dragged element has no matching field', () => {
+    const instance = createInstance({
+      fields: [{ field_name: 'a' }],
+      formSteps: [{ title: 'Step 1', fields: ['a'] }],
+    });
+    const orphanEl = buildFieldElement(99);
+
+    instance.handleFieldMovedToStep(orphanEl, 0);
+
+    expect(instance.formSteps[0].fields).toEqual(['a']);
+    expect(instance.renderSingleStep).not.toHaveBeenCalled();
+  });
+
+  it('pushes an undo snapshot before moving the field, so Ctrl+Z can restore the pre-move step assignment', () => {
+    const instance = createInstance({
+      fields: [{ field_name: 'a' }, { field_name: 'b' }, { field_name: 'c' }],
+      formSteps: [{ title: 'Step 1', fields: ['a', 'b'] }, { title: 'Step 2', fields: ['c'] }],
+    });
+    document.body.innerHTML = '<div id="step-canvas-1"></div>';
+    const canvas = document.getElementById('step-canvas-1');
+    const cEl = buildFieldElement(2);
+    const aEl = buildFieldElement(0);
+    canvas.appendChild(cEl);
+    canvas.appendChild(aEl);
+
+    instance.handleFieldMovedToStep(aEl, 1);
+
+    expect(instance.undoStack).toHaveLength(1);
+    expect(JSON.parse(instance.undoStack[0])).toEqual({
+      fields: [{ field_name: 'a' }, { field_name: 'b' }, { field_name: 'c' }],
+      formSteps: [{ title: 'Step 1', fields: ['a', 'b'] }, { title: 'Step 2', fields: ['c'] }],
+    });
+  });
+
+  it('does not push an undo snapshot when the dragged element has no matching field', () => {
+    const instance = createInstance({
+      fields: [{ field_name: 'a' }],
+      formSteps: [{ title: 'Step 1', fields: ['a'] }],
+    });
+    const orphanEl = buildFieldElement(99);
+
+    instance.handleFieldMovedToStep(orphanEl, 0);
+
+    expect(instance.undoStack).toHaveLength(0);
+  });
+});

tests_js/form-builder/multiStepFieldIndexSync.test.js

@@ -0,0 +1,78 @@
+import { afterEach, describe, expect, it, vi } from 'vitest';
+import { FormBuilder } from '../../django_forms_workflows/static/django_forms_workflows/js/form-builder.js';
+import { createBuilderStore } from '../../django_forms_workflows/static/django_forms_workflows/js/form-builder-store.js';
+
+// Regression coverage for a bug found after the this.fields/formSteps sync
+// fix: reordering this.fields *after* rendering (or only re-rendering the
+// touched step) left other steps' data-field-index attributes pointing at
+// the wrong entries once this.fields' array order changed underneath them.
+// A subsequent drag-reorder that trusted those stale indices corrupted
+// formSteps/this.fields (duplicate/missing field names), which the backend
+// preview endpoint then 500'd on.
+function createInstance({ fields = [], formSteps = [] } = {}) {
+  const instance = Object.create(FormBuilder.prototype);
+  instance.store = createBuilderStore({ fields, formSteps });
+  instance.fieldIdCounter = 1;
+  instance.undoStack = [];
+  instance.redoStack = [];
+  instance.maxUndoSteps = 50;
+  instance.fieldTypes = [{ type: 'text' }];
+  instance.editField = vi.fn(); // don't actually open the property modal
+  instance.updatePreview = vi.fn();
+  return instance;
+}
+
+function setupStepCanvases(stepIndexes) {
+  document.body.innerHTML = stepIndexes.map(i => `<div id="step-canvas-${i}"></div>`).join('');
+}
+
+afterEach(() => {
+  document.body.innerHTML = '';
+});
+
+describe('multi-step field-index sync across steps', () => {
+  it('keeps every rendered field-item across every step pointing at the right field after a cross-step add', () => {
+    const instance = createInstance({
+      fields: [{ field_name: 'a', field_label: 'A', field_type: 'text' }, { field_name: 'b', field_label: 'B', field_type: 'text' }],
+      formSteps: [
+        { title: 'Step 1', fields: ['a'] },
+        { title: 'Step 2', fields: ['b'] },
+      ],
+    });
+    setupStepCanvases([0, 1]);
+
+    // Drop a new field into step 0 before 'a' - this pushes 'a' and 'b' one
+    // slot later in the flattened this.fields array, including 'b' which
+    // lives in a step this handler never explicitly re-renders by name.
+    instance.handleFieldDroppedToStep('text', 0, 0);
+
+    document.querySelectorAll('.field-item').forEach(el => {
+      const idx = parseInt(el.dataset.fieldIndex);
+      const field = instance.fields[idx];
+      expect(field).toBeDefined();
+      expect(el.querySelector('.field-label').textContent).toBe(field.field_label);
+    });
+  });
+
+  it('does not corrupt formSteps/this.fields when a step is reordered right after a cross-step field-index shift', () => {
+    const instance = createInstance({
+      fields: [{ field_name: 'a', field_label: 'A', field_type: 'text' }, { field_name: 'b', field_label: 'B', field_type: 'text' }],
+      formSteps: [
+        { title: 'Step 1', fields: ['a'] },
+        { title: 'Step 2', fields: ['b'] },
+      ],
+    });
+    setupStepCanvases([0, 1]);
+
+    instance.handleFieldDroppedToStep('text', 0, 0); // shifts every later index
+
+    // Simulate the very next user action from the bug report: reordering
+    // within step 0, which reads data-field-index back out of the DOM.
+    instance.updateFieldOrderInStep(0);
+
+    const allNames = instance.fields.map(f => f.field_name);
+    expect(new Set(allNames).size).toBe(allNames.length); // no duplicates
+    expect(allNames).toHaveLength(3); // no fields silently dropped
+    expect(instance.formSteps[1].fields).toEqual(['b']); // step 2 untouched
+  });
+});

tests_js/form-builder/updateFieldOrderInStep.test.js

@@ -5,6 +5,9 @@ import { createBuilderStore } from '../../django_forms_workflows/static/django_f
 function createInstance({ fields, formSteps }) {
   const instance = Object.create(FormBuilder.prototype);
   instance.store = createBuilderStore({ fields, formSteps });
+  instance.undoStack = [];
+  instance.redoStack = [];
+  instance.maxUndoSteps = 50;
   return instance;
 }
 
@@ -39,4 +42,34 @@ describe('FormBuilder#updateFieldOrderInStep', () => {
     expect(instance.formSteps[0].fields).toEqual(['b', 'a']);
     expect(instance.updatePreview).toHaveBeenCalledTimes(1);
   });
+
+  it('reorders this.fields to match the new step order, not just formSteps (regression: preview used to keep the stale array order)', () => {
+    const instance = createInstance({
+      fields: [{ field_name: 'a' }, { field_name: 'b' }],
+      formSteps: [{ title: 'Step 1', fields: ['a', 'b'] }],
+    });
+    instance.updatePreview = vi.fn();
+    setStepCanvasOrder(0, [1, 0]); // dragged 'b' above 'a'
+
+    instance.updateFieldOrderInStep(0);
+
+    expect(instance.fields.map(f => f.field_name)).toEqual(['b', 'a']);
+  });
+
+  it('pushes an undo snapshot before reordering, so Ctrl+Z can restore the pre-drag order', () => {
+    const instance = createInstance({
+      fields: [{ field_name: 'a' }, { field_name: 'b' }],
+      formSteps: [{ title: 'Step 1', fields: ['a', 'b'] }],
+    });
+    instance.updatePreview = vi.fn();
+    setStepCanvasOrder(0, [1, 0]);
+
+    instance.updateFieldOrderInStep(0);
+
+    expect(instance.undoStack).toHaveLength(1);
+    expect(JSON.parse(instance.undoStack[0])).toEqual({
+      fields: [{ field_name: 'a' }, { field_name: 'b' }],
+      formSteps: [{ title: 'Step 1', fields: ['a', 'b'] }],
+    });
+  });
 });