From f0c45d71d0b881768491b31927cad4250b935e1d Mon Sep 17 00:00:00 2001 From: Maria Violante Date: Wed, 5 Aug 2026 08:15:46 -0400 Subject: [PATCH 1/6] Add createWorkflowBuilderStore() factory MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Single owner of nodes/connections/nodeIdCounter, same shape as form-builder-store.js's BuilderStore — factory, not a singleton (admin inlines can put two builders on one page), extends EventTarget so future extracted modules can react to changes instead of closing over the whole WorkflowBuilder instance. WorkflowBuilder now holds this.store and proxies this.nodes/this.connections/this.nodeIdCounter through it via getters/setters, so every existing call site keeps working unchanged. nextNodeId()/seedNodeIdCounterFromNodes() aren't wired into any call sites yet — available for the next extraction to use, same sequencing form-builder.js's saga followed (store landed before fieldIdCounter++ call sites were migrated to store.nextFieldId()). --- .../js/workflow-builder-store.js | 56 +++++++ .../js/workflow-builder.js | 17 +++ .../createWorkflowBuilderStore.test.js | 138 ++++++++++++++++++ 3 files changed, 211 insertions(+) create mode 100644 django_forms_workflows/static/django_forms_workflows/js/workflow-builder-store.js create mode 100644 tests_js/workflow-builder-store/createWorkflowBuilderStore.test.js diff --git a/django_forms_workflows/static/django_forms_workflows/js/workflow-builder-store.js b/django_forms_workflows/static/django_forms_workflows/js/workflow-builder-store.js new file mode 100644 index 0000000..e801a85 --- /dev/null +++ b/django_forms_workflows/static/django_forms_workflows/js/workflow-builder-store.js @@ -0,0 +1,56 @@ +/** + * Single owner of Workflow Builder state, shared across extracted modules. + * Factory (not a singleton) since admin inlines can put two builders on one + * page. Extends EventTarget so modules can react to changes instead of + * closing over the whole WorkflowBuilder instance. Mirrors + * form-builder-store.js's BuilderStore/createBuilderStore shape. + */ +export class WorkflowBuilderStore extends EventTarget { + constructor({ nodes = [], connections = [], nodeIdCounter = 1 } = {}) { + super(); + this.nodes = nodes; + this.connections = connections; + this.nodeIdCounter = nodeIdCounter; + } + + setNodes(nodes) { + this.nodes = nodes; + this.dispatchEvent(new CustomEvent('nodes-changed', { detail: { nodes } })); + } + + setConnections(connections) { + this.connections = connections; + this.dispatchEvent(new CustomEvent('connections-changed', { detail: { connections } })); + } + + nextNodeId() { + const id = `node_${this.nodeIdCounter}`; + this.nodeIdCounter += 1; + return id; + } + + // Mirrors loadWorkflow()'s existing node-id-counter-seeding logic (and + // form-builder-store.js's seedFieldIdCounterFromFields) — call after + // loading nodes from a workflow so newly-generated ids can't collide. + seedNodeIdCounterFromNodes(nodes) { + const highest = nodes.reduce((max, node) => { + const match = /node_(\d+)/.exec(node.id || ''); + return match ? Math.max(max, parseInt(match[1], 10)) : max; + }, 0); + this.nodeIdCounter = highest + 1; + } + + snapshot() { + return JSON.stringify({ nodes: this.nodes, connections: this.connections }); + } + + restore(snapshotJson) { + const { nodes, connections } = JSON.parse(snapshotJson); + this.setNodes(nodes); + this.setConnections(connections); + } +} + +export function createWorkflowBuilderStore(initial) { + return new WorkflowBuilderStore(initial); +} diff --git a/django_forms_workflows/static/django_forms_workflows/js/workflow-builder.js b/django_forms_workflows/static/django_forms_workflows/js/workflow-builder.js index 91727fd..ae989d6 100644 --- a/django_forms_workflows/static/django_forms_workflows/js/workflow-builder.js +++ b/django_forms_workflows/static/django_forms_workflows/js/workflow-builder.js @@ -4,9 +4,12 @@ * Drag-and-drop workflow builder for post-submission actions and approvals. */ +import { createWorkflowBuilderStore } from './workflow-builder-store.js'; + export class WorkflowBuilder { constructor(config) { this.config = config; + this.store = createWorkflowBuilderStore(); this.nodes = []; this.connections = []; this.selectedNode = null; @@ -45,6 +48,20 @@ export class WorkflowBuilder { this.init(); } + // nodes/connections/nodeIdCounter live on this.store now (single source + // of truth for a future history/undo module's snapshots); these proxy + // the existing this.nodes/this.connections/this.nodeIdCounter call sites + // throughout this file so they don't all need to change in this pass. + // Mirrors form-builder.js's identical this.store proxy pattern. + get nodes() { return this.store.nodes; } + set nodes(value) { this.store.setNodes(value); } + + get connections() { return this.store.connections; } + set connections(value) { this.store.setConnections(value); } + + get nodeIdCounter() { return this.store.nodeIdCounter; } + set nodeIdCounter(value) { this.store.nodeIdCounter = value; } + async init() { console.log('Initializing workflow builder...'); this.setupCanvas(); diff --git a/tests_js/workflow-builder-store/createWorkflowBuilderStore.test.js b/tests_js/workflow-builder-store/createWorkflowBuilderStore.test.js new file mode 100644 index 0000000..60f7a64 --- /dev/null +++ b/tests_js/workflow-builder-store/createWorkflowBuilderStore.test.js @@ -0,0 +1,138 @@ +import { describe, expect, it } from 'vitest'; +import { createWorkflowBuilderStore } from '../../django_forms_workflows/static/django_forms_workflows/js/workflow-builder-store.js'; + +describe('createWorkflowBuilderStore', () => { + it('is a factory, not a singleton — each call returns an independent store', () => { + const a = createWorkflowBuilderStore(); + const b = createWorkflowBuilderStore(); + + a.setNodes([{ id: 'node_1' }]); + + expect(a.nodes).toEqual([{ id: 'node_1' }]); + expect(b.nodes).toEqual([]); + expect(a).not.toBe(b); + }); + + it('defaults to empty nodes/connections and a nodeIdCounter of 1', () => { + const store = createWorkflowBuilderStore(); + + expect(store.nodes).toEqual([]); + expect(store.connections).toEqual([]); + expect(store.nodeIdCounter).toBe(1); + }); + + it('accepts initial state', () => { + const store = createWorkflowBuilderStore({ + nodes: [{ id: 'node_1' }], + connections: [{ from: 'node_1', to: 'node_2' }], + nodeIdCounter: 5, + }); + + expect(store.nodes).toEqual([{ id: 'node_1' }]); + expect(store.connections).toEqual([{ from: 'node_1', to: 'node_2' }]); + expect(store.nodeIdCounter).toBe(5); + }); +}); + +describe('WorkflowBuilderStore.setNodes / setConnections', () => { + it('setNodes updates state and emits nodes-changed', () => { + const store = createWorkflowBuilderStore(); + let received = null; + store.addEventListener('nodes-changed', (e) => { received = e.detail.nodes; }); + + store.setNodes([{ id: 'node_1' }]); + + expect(store.nodes).toEqual([{ id: 'node_1' }]); + expect(received).toEqual([{ id: 'node_1' }]); + }); + + it('setConnections updates state and emits connections-changed', () => { + const store = createWorkflowBuilderStore(); + let received = null; + store.addEventListener('connections-changed', (e) => { received = e.detail.connections; }); + + store.setConnections([{ from: 'node_1', to: 'node_2' }]); + + expect(store.connections).toEqual([{ from: 'node_1', to: 'node_2' }]); + expect(received).toEqual([{ from: 'node_1', to: 'node_2' }]); + }); +}); + +describe('WorkflowBuilderStore.nextNodeId', () => { + it('returns node_counter and increments the counter', () => { + const store = createWorkflowBuilderStore({ nodeIdCounter: 1 }); + + expect(store.nextNodeId()).toBe('node_1'); + expect(store.nextNodeId()).toBe('node_2'); + expect(store.nodeIdCounter).toBe(3); + }); +}); + +describe('WorkflowBuilderStore.seedNodeIdCounterFromNodes', () => { + it('seeds the counter one past the highest existing node_N id', () => { + const store = createWorkflowBuilderStore(); + store.seedNodeIdCounterFromNodes([ + { id: 'node_1' }, + { id: 'node_5' }, + { id: 'node_3' }, + ]); + + expect(store.nodeIdCounter).toBe(6); + }); + + it('defaults to 1 when there are no nodes yet', () => { + const store = createWorkflowBuilderStore({ nodeIdCounter: 99 }); + store.seedNodeIdCounterFromNodes([]); + + expect(store.nodeIdCounter).toBe(1); + }); + + it('ignores node ids that do not match the node_N shape', () => { + const store = createWorkflowBuilderStore(); + store.seedNodeIdCounterFromNodes([ + { id: 'start' }, + { id: 'node_5' }, + ]); + + expect(store.nodeIdCounter).toBe(6); + }); + + it('does not crash on a node with no id', () => { + const store = createWorkflowBuilderStore(); + store.seedNodeIdCounterFromNodes([{}, { id: 'node_2' }]); + + expect(store.nodeIdCounter).toBe(3); + }); +}); + +describe('WorkflowBuilderStore.snapshot / restore', () => { + it('snapshot captures nodes and connections together', () => { + const store = createWorkflowBuilderStore({ + nodes: [{ id: 'node_1' }], + connections: [{ from: 'node_1', to: 'node_2' }], + }); + + expect(store.snapshot()).toEqual( + JSON.stringify({ nodes: [{ id: 'node_1' }], connections: [{ from: 'node_1', to: 'node_2' }] }) + ); + }); + + it('restore replaces nodes and connections and emits both change events', () => { + const store = createWorkflowBuilderStore({ nodes: [{ id: 'old' }], connections: [] }); + const nodesChanged = []; + const connectionsChanged = []; + store.addEventListener('nodes-changed', (e) => nodesChanged.push(e.detail.nodes)); + store.addEventListener('connections-changed', (e) => connectionsChanged.push(e.detail.connections)); + + const snapshot = JSON.stringify({ + nodes: [{ id: 'node_restored' }], + connections: [{ from: 'node_restored', to: 'node_2' }], + }); + store.restore(snapshot); + + expect(store.nodes).toEqual([{ id: 'node_restored' }]); + expect(store.connections).toEqual([{ from: 'node_restored', to: 'node_2' }]); + expect(nodesChanged).toEqual([[{ id: 'node_restored' }]]); + expect(connectionsChanged).toEqual([[{ from: 'node_restored', to: 'node_2' }]]); + }); +}); From bb0cfe0ab23bb841c902e9a1789a9e10e375b387 Mon Sep 17 00:00:00 2001 From: Maria Violante Date: Wed, 5 Aug 2026 08:25:53 -0400 Subject: [PATCH 2/6] Extract workflow-builder-api.js out of workflow-builder.js MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Pure move: loadWorkflow, saveWorkflow, and the save-status/dirty-tracking helpers they depend on (setSaveStatus, getWorkflowSnapshot, syncSavedWorkflowSnapshot, updateDirtyState, updateDirtyIndicator) into their own module, mixed onto WorkflowBuilder.prototype via Object.assign — same shape as form-builder-api.js. No logic changed. --- .../js/workflow-builder-api.js | 191 ++++++++++++ .../js/workflow-builder.js | 180 +---------- .../workflow-builder-api/apiMethods.test.js | 285 ++++++++++++++++++ 3 files changed, 479 insertions(+), 177 deletions(-) create mode 100644 django_forms_workflows/static/django_forms_workflows/js/workflow-builder-api.js create mode 100644 tests_js/workflow-builder-api/apiMethods.test.js diff --git a/django_forms_workflows/static/django_forms_workflows/js/workflow-builder-api.js b/django_forms_workflows/static/django_forms_workflows/js/workflow-builder-api.js new file mode 100644 index 0000000..606844d --- /dev/null +++ b/django_forms_workflows/static/django_forms_workflows/js/workflow-builder-api.js @@ -0,0 +1,191 @@ +/** + * API client for the Workflow Builder: loading/saving the workflow + * definition, plus save-status and dirty-tracking bookkeeping around those + * two calls. + * + * Mixed onto WorkflowBuilder.prototype in workflow-builder.js + * (Object.assign), not a standalone class of its own - these methods read/ + * write `this.nodes`/`this.connections`/`this.config` directly, plus call + * back into validation/canvas methods (refreshValidationState/selectNode/ + * initializeNodeStackOrder/layoutNeedsNormalization/autoArrangeNodes/ + * setBuilderMessage) that still live on the single WorkflowBuilder + * instance. Mirrors form-builder-api.js's shape/scope. + */ +export const apiMethods = { + setSaveStatus(text, tone = 'neutral') { + const status = document.getElementById('saveStatus'); + if (!status) return; + status.textContent = text; + status.dataset.tone = tone; + }, + + getWorkflowSnapshot() { + return JSON.stringify({ + nodes: this.nodes, + connections: this.connections, + }); + }, + + syncSavedWorkflowSnapshot() { + this.lastSavedWorkflowSnapshot = this.getWorkflowSnapshot(); + this.updateDirtyState(); + }, + + updateDirtyState() { + this.isDirty = this.lastSavedWorkflowSnapshot !== null + && this.getWorkflowSnapshot() !== this.lastSavedWorkflowSnapshot; + this.updateDirtyIndicator(); + }, + + updateDirtyIndicator() { + const badge = document.getElementById('dirtyIndicator'); + if (badge) { + badge.hidden = !this.isDirty; + } + + if (!this.isSaving) { + if (this.isDirty) { + this.setSaveStatus('Unsaved changes', 'warning'); + } else { + this.setSaveStatus('Ready', 'neutral'); + } + } + }, + + async loadWorkflow() { + try { + console.log('Loading workflow from:', this.config.apiUrls.load); + const response = await fetch(this.config.apiUrls.load); + const data = await response.json(); + + console.log('Workflow data received:', data); + + if (data.success) { + this.nodes = data.workflow.nodes || []; + this.connections = data.workflow.connections || []; + this.fields = data.fields || []; + this.groups = data.groups || []; + this.forms = data.forms || []; + this.workflowTargets = data.workflow_targets || []; + + console.log('Loaded nodes:', this.nodes); + console.log('Loaded connections:', this.connections); + console.log('Available forms:', this.forms); + + // Update node ID counter + if (this.nodes.length > 0) { + const maxId = Math.max(...this.nodes.map(n => { + const match = n.id.match(/node_(\d+)/); + return match ? parseInt(match[1]) : 0; + })); + this.nodeIdCounter = maxId + 1; + } + + this.initializeNodeStackOrder(); + if (this.layoutNeedsNormalization()) { + this.autoArrangeNodes({ suppressRender: true, silent: true }); + } + } else { + console.error('Failed to load workflow:', data.error); + this.setBuilderMessage('danger', 'Failed to load workflow builder data.', [data.error || 'Unknown error']); + } + } catch (error) { + console.error('Error loading workflow:', error); + this.setBuilderMessage('danger', 'Failed to load workflow builder data.', [error.message || 'Unknown error']); + } + }, + + async saveWorkflow() { + const validation = this.refreshValidationState(); + if (validation.errors.length) { + this.setSaveStatus('Fix validation errors', 'danger'); + this.setBuilderMessage( + 'danger', + 'Fix validation errors before saving.', + validation.errors + ); + if (validation.firstErrorNodeId) { + this.selectNode(validation.firstErrorNodeId); + } + return; + } + + const saveBtn = document.getElementById('btnSave'); + const originalText = saveBtn.innerHTML; + this.isSaving = true; + saveBtn.disabled = true; + saveBtn.innerHTML = ' Saving...'; + this.setSaveStatus('Saving...', 'info'); + this.setBuilderMessage( + validation.warnings.length ? 'warning' : 'info', + validation.warnings.length + ? 'Saving workflow with warnings.' + : 'Saving workflow…', + validation.warnings + ); + + const workflowData = { + form_id: this.config.formId, + workflow_id: this.config.currentWorkflowId, + workflow: { + nodes: this.nodes, + connections: this.connections + } + }; + + console.log('Saving workflow data:', workflowData); + console.log('Nodes:', this.nodes); + console.log('Connections:', this.connections); + + try { + const response = await fetch(this.config.apiUrls.save, { + method: 'POST', + headers: { + 'Content-Type': 'application/json', + 'X-CSRFToken': this.config.csrfToken + }, + body: JSON.stringify(workflowData) + }); + + console.log('Response status:', response.status); + console.log('Response ok:', response.ok); + + const result = await response.json(); + console.log('Response data:', result); + + if (!response.ok || !result.success) { + const error = new Error(result.error || 'Failed to save workflow'); + error.details = result.errors || []; + throw error; + } + + if (result.workflow_id) { + this.config.currentWorkflowId = result.workflow_id; + } + + this.syncSavedWorkflowSnapshot(); + this.setSaveStatus('Saved successfully', 'success'); + this.setBuilderMessage( + 'success', + 'Workflow saved successfully.', + validation.warnings.length ? ['Saved with non-blocking warnings shown below.'] : [], + true + ); + setTimeout(() => { + this.setSaveStatus('Ready', 'neutral'); + }, 2000); + } catch (error) { + console.error('Error saving workflow:', error); + this.setBuilderMessage( + 'danger', + `Failed to save workflow: ${error.message}`, + error.details || [] + ); + this.setSaveStatus('Error saving', 'danger'); + } finally { + this.isSaving = false; + saveBtn.disabled = false; + saveBtn.innerHTML = originalText; + } + }, +}; diff --git a/django_forms_workflows/static/django_forms_workflows/js/workflow-builder.js b/django_forms_workflows/static/django_forms_workflows/js/workflow-builder.js index ae989d6..5aeb642 100644 --- a/django_forms_workflows/static/django_forms_workflows/js/workflow-builder.js +++ b/django_forms_workflows/static/django_forms_workflows/js/workflow-builder.js @@ -5,6 +5,7 @@ */ import { createWorkflowBuilderStore } from './workflow-builder-store.js'; +import { apiMethods } from './workflow-builder-api.js'; export class WorkflowBuilder { constructor(config) { @@ -316,46 +317,6 @@ export class WorkflowBuilder { return element.isContentEditable || ['input', 'textarea', 'select'].includes(tagName); } - setSaveStatus(text, tone = 'neutral') { - const status = document.getElementById('saveStatus'); - if (!status) return; - status.textContent = text; - status.dataset.tone = tone; - } - - getWorkflowSnapshot() { - return JSON.stringify({ - nodes: this.nodes, - connections: this.connections, - }); - } - - syncSavedWorkflowSnapshot() { - this.lastSavedWorkflowSnapshot = this.getWorkflowSnapshot(); - this.updateDirtyState(); - } - - updateDirtyState() { - this.isDirty = this.lastSavedWorkflowSnapshot !== null - && this.getWorkflowSnapshot() !== this.lastSavedWorkflowSnapshot; - this.updateDirtyIndicator(); - } - - updateDirtyIndicator() { - const badge = document.getElementById('dirtyIndicator'); - if (badge) { - badge.hidden = !this.isDirty; - } - - if (!this.isSaving) { - if (this.isDirty) { - this.setSaveStatus('Unsaved changes', 'warning'); - } else { - this.setSaveStatus('Ready', 'neutral'); - } - } - } - formatNodeReference(node) { if (!node) return 'Unknown node'; const specificName = node.data?.name || node.data?.sub_workflow_name || node.data?.name_label || node.data?.form_name; @@ -458,143 +419,6 @@ export class WorkflowBuilder { `; } - async loadWorkflow() { - try { - console.log('Loading workflow from:', this.config.apiUrls.load); - const response = await fetch(this.config.apiUrls.load); - const data = await response.json(); - - console.log('Workflow data received:', data); - - if (data.success) { - this.nodes = data.workflow.nodes || []; - this.connections = data.workflow.connections || []; - this.fields = data.fields || []; - this.groups = data.groups || []; - this.forms = data.forms || []; - this.workflowTargets = data.workflow_targets || []; - - console.log('Loaded nodes:', this.nodes); - console.log('Loaded connections:', this.connections); - console.log('Available forms:', this.forms); - - // Update node ID counter - if (this.nodes.length > 0) { - const maxId = Math.max(...this.nodes.map(n => { - const match = n.id.match(/node_(\d+)/); - return match ? parseInt(match[1]) : 0; - })); - this.nodeIdCounter = maxId + 1; - } - - this.initializeNodeStackOrder(); - if (this.layoutNeedsNormalization()) { - this.autoArrangeNodes({ suppressRender: true, silent: true }); - } - } else { - console.error('Failed to load workflow:', data.error); - this.setBuilderMessage('danger', 'Failed to load workflow builder data.', [data.error || 'Unknown error']); - } - } catch (error) { - console.error('Error loading workflow:', error); - this.setBuilderMessage('danger', 'Failed to load workflow builder data.', [error.message || 'Unknown error']); - } - } - - async saveWorkflow() { - const validation = this.refreshValidationState(); - if (validation.errors.length) { - this.setSaveStatus('Fix validation errors', 'danger'); - this.setBuilderMessage( - 'danger', - 'Fix validation errors before saving.', - validation.errors - ); - if (validation.firstErrorNodeId) { - this.selectNode(validation.firstErrorNodeId); - } - return; - } - - const saveBtn = document.getElementById('btnSave'); - const originalText = saveBtn.innerHTML; - this.isSaving = true; - saveBtn.disabled = true; - saveBtn.innerHTML = ' Saving...'; - this.setSaveStatus('Saving...', 'info'); - this.setBuilderMessage( - validation.warnings.length ? 'warning' : 'info', - validation.warnings.length - ? 'Saving workflow with warnings.' - : 'Saving workflow…', - validation.warnings - ); - - const workflowData = { - form_id: this.config.formId, - workflow_id: this.config.currentWorkflowId, - workflow: { - nodes: this.nodes, - connections: this.connections - } - }; - - console.log('Saving workflow data:', workflowData); - console.log('Nodes:', this.nodes); - console.log('Connections:', this.connections); - - try { - const response = await fetch(this.config.apiUrls.save, { - method: 'POST', - headers: { - 'Content-Type': 'application/json', - 'X-CSRFToken': this.config.csrfToken - }, - body: JSON.stringify(workflowData) - }); - - console.log('Response status:', response.status); - console.log('Response ok:', response.ok); - - const result = await response.json(); - console.log('Response data:', result); - - if (!response.ok || !result.success) { - const error = new Error(result.error || 'Failed to save workflow'); - error.details = result.errors || []; - throw error; - } - - if (result.workflow_id) { - this.config.currentWorkflowId = result.workflow_id; - } - - this.syncSavedWorkflowSnapshot(); - this.setSaveStatus('Saved successfully', 'success'); - this.setBuilderMessage( - 'success', - 'Workflow saved successfully.', - validation.warnings.length ? ['Saved with non-blocking warnings shown below.'] : [], - true - ); - setTimeout(() => { - this.setSaveStatus('Ready', 'neutral'); - }, 2000); - } catch (error) { - console.error('Error saving workflow:', error); - this.setBuilderMessage( - 'danger', - `Failed to save workflow: ${error.message}`, - error.details || [] - ); - this.setSaveStatus('Error saving', 'danger'); - } finally { - this.isSaving = false; - saveBtn.disabled = false; - saveBtn.innerHTML = originalText; - } - } - createStartNode() { const node = { id: `node_${this.nodeIdCounter++}`, @@ -3179,3 +3003,5 @@ export class WorkflowBuilder { return `M ${x1} ${y1} C ${cx1} ${y1}, ${cx2} ${y2}, ${x2} ${y2}`; } } + +Object.assign(WorkflowBuilder.prototype, apiMethods); diff --git a/tests_js/workflow-builder-api/apiMethods.test.js b/tests_js/workflow-builder-api/apiMethods.test.js new file mode 100644 index 0000000..5592f4f --- /dev/null +++ b/tests_js/workflow-builder-api/apiMethods.test.js @@ -0,0 +1,285 @@ +import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest'; +import { apiMethods } from '../../django_forms_workflows/static/django_forms_workflows/js/workflow-builder-api.js'; +import { createWorkflowBuilderStore } from '../../django_forms_workflows/static/django_forms_workflows/js/workflow-builder-store.js'; + +// Every element loadWorkflow/saveWorkflow touch directly. +function setupBuilderDOM() { + document.body.innerHTML = ` + + + + `; +} + +function createContext({ nodes = [], connections = [], config = {}, validation } = {}) { + const store = createWorkflowBuilderStore({ nodes, connections }); + return { + store, + config: { apiUrls: {}, csrfToken: 'test-token', formId: 1, currentWorkflowId: null, ...config }, + fields: [], + groups: [], + forms: [], + workflowTargets: [], + isSaving: false, + isDirty: false, + lastSavedWorkflowSnapshot: null, + setBuilderMessage: vi.fn(), + selectNode: vi.fn(), + initializeNodeStackOrder: vi.fn(), + layoutNeedsNormalization: vi.fn().mockReturnValue(false), + autoArrangeNodes: vi.fn(), + refreshValidationState: vi.fn().mockReturnValue( + validation || { errors: [], warnings: [], nodeIssues: {}, firstErrorNodeId: null } + ), + get nodes() { return this.store.nodes; }, + set nodes(value) { this.store.setNodes(value); }, + get connections() { return this.store.connections; }, + set connections(value) { this.store.setConnections(value); }, + get nodeIdCounter() { return this.store.nodeIdCounter; }, + set nodeIdCounter(value) { this.store.nodeIdCounter = value; }, + ...apiMethods, + }; +} + +beforeEach(() => { + setupBuilderDOM(); +}); + +afterEach(() => { + document.body.innerHTML = ''; + vi.unstubAllGlobals(); + vi.restoreAllMocks(); +}); + +describe('apiMethods.setSaveStatus', () => { + it('sets the status text and tone dataset attribute', () => { + const ctx = createContext(); + + ctx.setSaveStatus('Saving...', 'info'); + + const status = document.getElementById('saveStatus'); + expect(status.textContent).toBe('Saving...'); + expect(status.dataset.tone).toBe('info'); + }); + + it('is a no-op when the saveStatus element is missing', () => { + document.body.innerHTML = ''; + const ctx = createContext(); + + expect(() => ctx.setSaveStatus('Saving...')).not.toThrow(); + }); +}); + +describe('apiMethods.getWorkflowSnapshot', () => { + it('serializes nodes and connections together', () => { + const ctx = createContext({ + nodes: [{ id: 'node_1' }], + connections: [{ from: 'node_1', to: 'node_2' }], + }); + + expect(ctx.getWorkflowSnapshot()).toEqual( + JSON.stringify({ nodes: [{ id: 'node_1' }], connections: [{ from: 'node_1', to: 'node_2' }] }) + ); + }); +}); + +describe('apiMethods.syncSavedWorkflowSnapshot / updateDirtyState', () => { + it('marks clean immediately after syncing', () => { + const ctx = createContext({ nodes: [{ id: 'node_1' }] }); + + ctx.syncSavedWorkflowSnapshot(); + + expect(ctx.isDirty).toBe(false); + expect(document.getElementById('dirtyIndicator').hidden).toBe(true); + expect(document.getElementById('saveStatus').textContent).toBe('Ready'); + }); + + it('marks dirty once nodes/connections change after syncing', () => { + const ctx = createContext({ nodes: [{ id: 'node_1' }] }); + ctx.syncSavedWorkflowSnapshot(); + + ctx.nodes = [{ id: 'node_1' }, { id: 'node_2' }]; + ctx.updateDirtyState(); + + expect(ctx.isDirty).toBe(true); + expect(document.getElementById('dirtyIndicator').hidden).toBe(false); + expect(document.getElementById('saveStatus').textContent).toBe('Unsaved changes'); + }); + + it('does not overwrite the save status while a save is in flight', () => { + const ctx = createContext({ nodes: [{ id: 'node_1' }] }); + ctx.syncSavedWorkflowSnapshot(); + ctx.isSaving = true; + ctx.setSaveStatus('Saving...', 'info'); + + ctx.nodes = [{ id: 'node_1' }, { id: 'node_2' }]; + ctx.updateDirtyState(); + + expect(document.getElementById('saveStatus').textContent).toBe('Saving...'); + }); +}); + +describe('apiMethods.loadWorkflow', () => { + function stubLoadResponse(overrides = {}) { + vi.stubGlobal('fetch', vi.fn().mockResolvedValue({ + json: async () => ({ + success: true, + workflow: { nodes: [], connections: [] }, + fields: [], + groups: [], + forms: [], + workflow_targets: [], + ...overrides, + }), + })); + } + + it('populates nodes/connections/fields/groups/forms from the response', async () => { + stubLoadResponse({ + workflow: { nodes: [{ id: 'node_1' }], connections: [{ from: 'node_1', to: 'node_2' }] }, + fields: [{ field_name: 'a' }], + groups: [{ id: 1 }], + forms: [{ id: 2 }], + workflow_targets: [{ id: 3 }], + }); + const ctx = createContext(); + + await ctx.loadWorkflow(); + + expect(ctx.nodes).toEqual([{ id: 'node_1' }]); + expect(ctx.connections).toEqual([{ from: 'node_1', to: 'node_2' }]); + expect(ctx.fields).toEqual([{ field_name: 'a' }]); + expect(ctx.groups).toEqual([{ id: 1 }]); + expect(ctx.forms).toEqual([{ id: 2 }]); + expect(ctx.workflowTargets).toEqual([{ id: 3 }]); + }); + + it('seeds nodeIdCounter past the highest loaded node_N id', async () => { + stubLoadResponse({ + workflow: { nodes: [{ id: 'node_1' }, { id: 'node_7' }], connections: [] }, + }); + const ctx = createContext(); + + await ctx.loadWorkflow(); + + expect(ctx.nodeIdCounter).toBe(8); + }); + + it('leaves nodeIdCounter untouched when there are no nodes', async () => { + stubLoadResponse({ workflow: { nodes: [], connections: [] } }); + const ctx = createContext({ config: {} }); + ctx.store.nodeIdCounter = 3; + + await ctx.loadWorkflow(); + + expect(ctx.nodeIdCounter).toBe(3); + }); + + it('normalizes layout when needed after load', async () => { + stubLoadResponse({ workflow: { nodes: [{ id: 'node_1' }], connections: [] } }); + const ctx = createContext(); + ctx.layoutNeedsNormalization = vi.fn().mockReturnValue(true); + + await ctx.loadWorkflow(); + + expect(ctx.initializeNodeStackOrder).toHaveBeenCalledTimes(1); + expect(ctx.autoArrangeNodes).toHaveBeenCalledWith({ suppressRender: true, silent: true }); + }); + + it('surfaces a builder message when the response reports failure', async () => { + stubLoadResponse({ success: false, error: 'nope' }); + const ctx = createContext(); + + await ctx.loadWorkflow(); + + expect(ctx.setBuilderMessage).toHaveBeenCalledWith( + 'danger', 'Failed to load workflow builder data.', ['nope'] + ); + }); + + it('surfaces a builder message and does not throw when the request rejects', async () => { + vi.stubGlobal('fetch', vi.fn().mockRejectedValue(new Error('network down'))); + const ctx = createContext(); + + await expect(ctx.loadWorkflow()).resolves.toBeUndefined(); + + expect(ctx.setBuilderMessage).toHaveBeenCalledWith( + 'danger', 'Failed to load workflow builder data.', ['network down'] + ); + }); +}); + +describe('apiMethods.saveWorkflow', () => { + it('blocks the save, reports status, and selects the first offending node on validation errors', async () => { + const ctx = createContext({ + validation: { errors: ['Stage missing an approver'], warnings: [], firstErrorNodeId: 'node_2' }, + }); + vi.stubGlobal('fetch', vi.fn()); + + await ctx.saveWorkflow(); + + expect(fetch).not.toHaveBeenCalled(); + expect(document.getElementById('saveStatus').textContent).toBe('Fix validation errors'); + expect(ctx.selectNode).toHaveBeenCalledWith('node_2'); + expect(ctx.setBuilderMessage).toHaveBeenCalledWith( + 'danger', 'Fix validation errors before saving.', ['Stage missing an approver'] + ); + }); + + it('sends nodes/connections and reports success', async () => { + const fetchMock = vi.fn().mockResolvedValue({ ok: true, json: async () => ({ success: true }) }); + vi.stubGlobal('fetch', fetchMock); + const ctx = createContext({ + nodes: [{ id: 'node_1' }], + connections: [{ from: 'node_1', to: 'node_2' }], + config: { formId: 5, currentWorkflowId: 9 }, + }); + + await ctx.saveWorkflow(); + + const body = JSON.parse(fetchMock.mock.calls[0][1].body); + expect(body).toEqual({ + form_id: 5, + workflow_id: 9, + workflow: { nodes: [{ id: 'node_1' }], connections: [{ from: 'node_1', to: 'node_2' }] }, + }); + expect(document.getElementById('saveStatus').textContent).toBe('Saved successfully'); + expect(document.getElementById('btnSave').disabled).toBe(false); + }); + + it('adopts the returned workflow_id for a newly-created workflow', async () => { + vi.stubGlobal('fetch', vi.fn().mockResolvedValue({ + ok: true, json: async () => ({ success: true, workflow_id: 42 }), + })); + const ctx = createContext({ config: { currentWorkflowId: null } }); + + await ctx.saveWorkflow(); + + expect(ctx.config.currentWorkflowId).toBe(42); + }); + + it('reports an error status and does not throw when the save request fails', async () => { + vi.stubGlobal('fetch', vi.fn().mockResolvedValue({ + ok: false, json: async () => ({ success: false, error: 'Server exploded' }), + })); + const ctx = createContext(); + + await expect(ctx.saveWorkflow()).resolves.toBeUndefined(); + + expect(document.getElementById('saveStatus').textContent).toBe('Error saving'); + expect(ctx.setBuilderMessage).toHaveBeenCalledWith( + 'danger', 'Failed to save workflow: Server exploded', [] + ); + expect(document.getElementById('btnSave').disabled).toBe(false); + }); + + it('re-enables the save button even when the request rejects outright', async () => { + vi.stubGlobal('fetch', vi.fn().mockRejectedValue(new Error('network down'))); + const ctx = createContext(); + + await ctx.saveWorkflow(); + + expect(document.getElementById('btnSave').disabled).toBe(false); + expect(ctx.isSaving).toBe(false); + }); +}); From ddaa4d0974b5f77d4fc9b2f663aed5d7d5c78fb4 Mon Sep 17 00:00:00 2001 From: Maria Violante Date: Wed, 5 Aug 2026 08:56:29 -0400 Subject: [PATCH 3/6] Add regression coverage for the store-wiring commit createStartNode/createNode had zero test coverage before or after being wired to store.nextNodeId() -- added tests confirming ids come from the store, the counter is shared/sequential across both methods, and a newly-created node doesn't collide with ids already seeded from a loaded workflow. Also strengthened getWorkflowSnapshot/loadWorkflow's existing tests with spies on store.snapshot()/store.seedNodeIdCounterFromNodes() -- the prior tests only checked output equivalence, which would pass whether or not those methods actually delegate to the store, so they wouldn't catch a regression back to the duplicated inline logic. --- .../js/workflow-builder-api.js | 11 +-- .../js/workflow-builder.js | 4 +- .../workflow-builder-api/apiMethods.test.js | 19 ++++ .../workflow-builder/nodeCreation.test.js | 86 +++++++++++++++++++ 4 files changed, 109 insertions(+), 11 deletions(-) create mode 100644 tests_js/workflow-builder/nodeCreation.test.js diff --git a/django_forms_workflows/static/django_forms_workflows/js/workflow-builder-api.js b/django_forms_workflows/static/django_forms_workflows/js/workflow-builder-api.js index 606844d..d3d1024 100644 --- a/django_forms_workflows/static/django_forms_workflows/js/workflow-builder-api.js +++ b/django_forms_workflows/static/django_forms_workflows/js/workflow-builder-api.js @@ -20,10 +20,7 @@ export const apiMethods = { }, getWorkflowSnapshot() { - return JSON.stringify({ - nodes: this.nodes, - connections: this.connections, - }); + return this.store.snapshot(); }, syncSavedWorkflowSnapshot() { @@ -74,11 +71,7 @@ export const apiMethods = { // Update node ID counter if (this.nodes.length > 0) { - const maxId = Math.max(...this.nodes.map(n => { - const match = n.id.match(/node_(\d+)/); - return match ? parseInt(match[1]) : 0; - })); - this.nodeIdCounter = maxId + 1; + this.store.seedNodeIdCounterFromNodes(this.nodes); } this.initializeNodeStackOrder(); diff --git a/django_forms_workflows/static/django_forms_workflows/js/workflow-builder.js b/django_forms_workflows/static/django_forms_workflows/js/workflow-builder.js index 5aeb642..a68b57d 100644 --- a/django_forms_workflows/static/django_forms_workflows/js/workflow-builder.js +++ b/django_forms_workflows/static/django_forms_workflows/js/workflow-builder.js @@ -421,7 +421,7 @@ export class WorkflowBuilder { createStartNode() { const node = { - id: `node_${this.nodeIdCounter++}`, + id: this.store.nextNodeId(), type: 'start', x: 100, y: 100, @@ -434,7 +434,7 @@ export class WorkflowBuilder { createNode(type, x, y) { const node = { - id: `node_${this.nodeIdCounter++}`, + id: this.store.nextNodeId(), type: type, x: x, y: y, diff --git a/tests_js/workflow-builder-api/apiMethods.test.js b/tests_js/workflow-builder-api/apiMethods.test.js index 5592f4f..cefc695 100644 --- a/tests_js/workflow-builder-api/apiMethods.test.js +++ b/tests_js/workflow-builder-api/apiMethods.test.js @@ -81,6 +81,15 @@ describe('apiMethods.getWorkflowSnapshot', () => { JSON.stringify({ nodes: [{ id: 'node_1' }], connections: [{ from: 'node_1', to: 'node_2' }] }) ); }); + + it('delegates to store.snapshot() rather than re-serializing separately', () => { + const ctx = createContext({ nodes: [{ id: 'node_1' }] }); + const snapshotSpy = vi.spyOn(ctx.store, 'snapshot'); + + ctx.getWorkflowSnapshot(); + + expect(snapshotSpy).toHaveBeenCalledTimes(1); + }); }); describe('apiMethods.syncSavedWorkflowSnapshot / updateDirtyState', () => { @@ -165,6 +174,16 @@ describe('apiMethods.loadWorkflow', () => { expect(ctx.nodeIdCounter).toBe(8); }); + it('seeds the counter via store.seedNodeIdCounterFromNodes(), not a re-implemented scan', async () => { + stubLoadResponse({ workflow: { nodes: [{ id: 'node_1' }], connections: [] } }); + const ctx = createContext(); + const seedSpy = vi.spyOn(ctx.store, 'seedNodeIdCounterFromNodes'); + + await ctx.loadWorkflow(); + + expect(seedSpy).toHaveBeenCalledWith(ctx.nodes); + }); + it('leaves nodeIdCounter untouched when there are no nodes', async () => { stubLoadResponse({ workflow: { nodes: [], connections: [] } }); const ctx = createContext({ config: {} }); diff --git a/tests_js/workflow-builder/nodeCreation.test.js b/tests_js/workflow-builder/nodeCreation.test.js new file mode 100644 index 0000000..1e989ef --- /dev/null +++ b/tests_js/workflow-builder/nodeCreation.test.js @@ -0,0 +1,86 @@ +import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest'; +import { WorkflowBuilder } from '../../django_forms_workflows/static/django_forms_workflows/js/workflow-builder.js'; +import { createWorkflowBuilderStore } from '../../django_forms_workflows/static/django_forms_workflows/js/workflow-builder-store.js'; + +// createStartNode/createNode still live directly on WorkflowBuilder.prototype +// (not yet extracted into their own module), same instantiation pattern as +// moduleAndEscaping.test.js — a bare instance with just enough stubbed state +// for these two methods to run without touching the real DOM/canvas. +function createInstance() { + const instance = Object.create(WorkflowBuilder.prototype); + instance.store = createWorkflowBuilderStore(); + instance.nodeStackOrder = new Map(); + instance.nextNodeStackOrder = 1; + instance.render = vi.fn(); + return instance; +} + +beforeEach(() => { + vi.restoreAllMocks(); +}); + +afterEach(() => { + vi.restoreAllMocks(); +}); + +describe('WorkflowBuilder#createStartNode', () => { + it('generates the id via store.nextNodeId(), advancing the shared counter', () => { + const instance = createInstance(); + + instance.createStartNode(); + + expect(instance.nodes).toEqual([ + expect.objectContaining({ id: 'node_1', type: 'start', x: 100, y: 100, data: {} }), + ]); + expect(instance.store.nodeIdCounter).toBe(2); + }); + + it('brings the new node to front and renders', () => { + const instance = createInstance(); + instance.bringNodeToFront = vi.fn(); + + instance.createStartNode(); + + expect(instance.bringNodeToFront).toHaveBeenCalledWith('node_1'); + expect(instance.render).toHaveBeenCalledTimes(1); + }); +}); + +describe('WorkflowBuilder#createNode', () => { + it('generates sequential ids across calls, sharing the counter with createStartNode', () => { + const instance = createInstance(); + instance.bringNodeToFront = vi.fn(); + instance.getDefaultNodeData = vi.fn().mockReturnValue({}); + + instance.createStartNode(); + instance.createNode('stage', 200, 150); + instance.createNode('action', 300, 250); + + expect(instance.nodes.map(n => n.id)).toEqual(['node_1', 'node_2', 'node_3']); + expect(instance.store.nodeIdCounter).toBe(4); + }); + + it('uses getDefaultNodeData(type) for the new node\'s data', () => { + const instance = createInstance(); + instance.bringNodeToFront = vi.fn(); + instance.getDefaultNodeData = vi.fn().mockReturnValue({ label: 'Stage default' }); + + instance.createNode('stage', 10, 20); + + expect(instance.nodes[0]).toEqual( + expect.objectContaining({ type: 'stage', x: 10, y: 20, data: { label: 'Stage default' } }) + ); + expect(instance.getDefaultNodeData).toHaveBeenCalledWith('stage'); + }); + + it('does not collide with an id already seeded from a loaded workflow', () => { + const instance = createInstance(); + instance.bringNodeToFront = vi.fn(); + instance.getDefaultNodeData = vi.fn().mockReturnValue({}); + instance.store.seedNodeIdCounterFromNodes([{ id: 'node_1' }, { id: 'node_5' }]); + + instance.createNode('action', 0, 0); + + expect(instance.nodes[0].id).toBe('node_6'); + }); +}); From 4269032848c425a31163b53c3b03f6cf814fd277 Mon Sep 17 00:00:00 2001 From: Maria Violante Date: Mon, 10 Aug 2026 11:38:59 -0400 Subject: [PATCH 4/6] Bugfix: Fix checkbox change handler storing "on" instead of true/false MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Newly created workflow builder steps would return a 400 error on attempted save with log message: "Workflow validation failed: ['“on” value must be either True or False.']". Adjusting the step's name would allow the step to be saved. After investigation, determined that showNodeProperties() attaches a generic change listener to every input in the node properties panel. Listener read e.target.value unconditionally and passed it to updateNodeProperty(), but checkbox .value = static HTML value attribute, which defaults to literal string "on" and not bool, which was then rejected by Django's BooleanField --- .../js/workflow-builder.js | 6 +- .../showNodeProperties.test.js | 82 +++++++++++++++++++ 2 files changed, 87 insertions(+), 1 deletion(-) create mode 100644 tests_js/workflow-builder/showNodeProperties.test.js diff --git a/django_forms_workflows/static/django_forms_workflows/js/workflow-builder.js b/django_forms_workflows/static/django_forms_workflows/js/workflow-builder.js index a68b57d..900227e 100644 --- a/django_forms_workflows/static/django_forms_workflows/js/workflow-builder.js +++ b/django_forms_workflows/static/django_forms_workflows/js/workflow-builder.js @@ -841,7 +841,11 @@ export class WorkflowBuilder { // Add event listeners for property changes content.querySelectorAll('input, select, textarea').forEach(input => { input.addEventListener('change', (e) => { - this.updateNodeProperty(node.id, e.target.name, e.target.value); + let value = e.target.value; + if (e.target.type === 'checkbox') { + value = e.target.checked; + } + this.updateNodeProperty(node.id, e.target.name, value); }); }); } diff --git a/tests_js/workflow-builder/showNodeProperties.test.js b/tests_js/workflow-builder/showNodeProperties.test.js new file mode 100644 index 0000000..6a111b1 --- /dev/null +++ b/tests_js/workflow-builder/showNodeProperties.test.js @@ -0,0 +1,82 @@ +import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest'; +import { WorkflowBuilder } from '../../django_forms_workflows/static/django_forms_workflows/js/workflow-builder.js'; +import { createWorkflowBuilderStore } from '../../django_forms_workflows/static/django_forms_workflows/js/workflow-builder-store.js'; + +// Regression coverage for the "on" boolean bug: showNodeProperties() attaches +// a generic change listener to every panel input as a backstop for fields +// without their own specific onchange handler (e.g. updateStageConfig). +// That generic listener used to forward e.target.value unconditionally, +// which for a checkbox is its static HTML value attribute ("on" by default) +// rather than its checked state — so toggling a checkbox re-clobbered it +// back to the literal string "on" right after a field-specific handler had +// just set the correct boolean, and Django's BooleanField rejected that +// string on save ("on" value must be either True or False). +function createInstance() { + const instance = Object.create(WorkflowBuilder.prototype); + instance.store = createWorkflowBuilderStore(); + instance.render = vi.fn(); + return instance; +} + +function dispatchChange(el) { + el.dispatchEvent(new Event('change', { bubbles: true })); +} + +beforeEach(() => { + document.body.innerHTML = '
'; + vi.restoreAllMocks(); +}); + +afterEach(() => { + vi.restoreAllMocks(); +}); + +describe('WorkflowBuilder#showNodeProperties generic change listener', () => { + it('stores a checkbox\'s checked state as a real boolean, not its "on" value attribute', () => { + const instance = createInstance(); + const node = { id: 'node_1', type: 'stage', data: { requires_manager_approval: false } }; + instance.nodes = [node]; + instance.buildPropertiesForm = vi.fn( + () => '' + ); + + instance.showNodeProperties(node); + const checkbox = document.querySelector('input[name="requires_manager_approval"]'); + checkbox.checked = true; + dispatchChange(checkbox); + + expect(node.data.requires_manager_approval).toBe(true); + }); + + it('stores an unchecked checkbox as false, not the string "on"', () => { + const instance = createInstance(); + const node = { id: 'node_1', type: 'stage', data: { allow_send_back: true } }; + instance.nodes = [node]; + instance.buildPropertiesForm = vi.fn( + () => '' + ); + + instance.showNodeProperties(node); + const checkbox = document.querySelector('input[name="allow_send_back"]'); + checkbox.checked = false; + dispatchChange(checkbox); + + expect(node.data.allow_send_back).toBe(false); + }); + + it('still forwards the typed value for non-checkbox inputs', () => { + const instance = createInstance(); + const node = { id: 'node_1', type: 'stage', data: { approve_label: '' } }; + instance.nodes = [node]; + instance.buildPropertiesForm = vi.fn( + () => '' + ); + + instance.showNodeProperties(node); + const input = document.querySelector('input[name="approve_label"]'); + input.value = 'Sign Off'; + dispatchChange(input); + + expect(node.data.approve_label).toBe('Sign Off'); + }); +}); From 88fdb03e875af276ed2534441c426353796657e7 Mon Sep 17 00:00:00 2001 From: Maria Violante Date: Mon, 10 Aug 2026 13:11:38 -0400 Subject: [PATCH 5/6] Fix: createNode/createStartNode bypassing store setter push() mutated this.ndoes in place, skipping setNodes() and the nodes-changed event it dispatches. Use copy-on-write via the setter, matching the existing pattern used for node deletion --- .../js/workflow-builder.js | 4 ++-- .../workflow-builder/nodeCreation.test.js | 24 +++++++++++++++++++ 2 files changed, 26 insertions(+), 2 deletions(-) diff --git a/django_forms_workflows/static/django_forms_workflows/js/workflow-builder.js b/django_forms_workflows/static/django_forms_workflows/js/workflow-builder.js index 900227e..166139b 100644 --- a/django_forms_workflows/static/django_forms_workflows/js/workflow-builder.js +++ b/django_forms_workflows/static/django_forms_workflows/js/workflow-builder.js @@ -427,7 +427,7 @@ export class WorkflowBuilder { y: 100, data: {} }; - this.nodes.push(node); + this.nodes = [...this.nodes, node]; this.bringNodeToFront(node.id); this.render(); } @@ -440,7 +440,7 @@ export class WorkflowBuilder { y: y, data: this.getDefaultNodeData(type) }; - this.nodes.push(node); + this.nodes = [...this.nodes, node]; this.bringNodeToFront(node.id); this.render(); } diff --git a/tests_js/workflow-builder/nodeCreation.test.js b/tests_js/workflow-builder/nodeCreation.test.js index 1e989ef..f02d1b0 100644 --- a/tests_js/workflow-builder/nodeCreation.test.js +++ b/tests_js/workflow-builder/nodeCreation.test.js @@ -44,6 +44,17 @@ describe('WorkflowBuilder#createStartNode', () => { expect(instance.bringNodeToFront).toHaveBeenCalledWith('node_1'); expect(instance.render).toHaveBeenCalledTimes(1); }); + + it('adds the node through the store setter, so nodes-changed listeners see it', () => { + const instance = createInstance(); + const listener = vi.fn(); + instance.store.addEventListener('nodes-changed', listener); + + instance.createStartNode(); + + expect(listener).toHaveBeenCalledTimes(1); + expect(listener.mock.calls[0][0].detail.nodes).toEqual(instance.nodes); + }); }); describe('WorkflowBuilder#createNode', () => { @@ -83,4 +94,17 @@ describe('WorkflowBuilder#createNode', () => { expect(instance.nodes[0].id).toBe('node_6'); }); + + it('adds the node through the store setter, so nodes-changed listeners see it', () => { + const instance = createInstance(); + instance.bringNodeToFront = vi.fn(); + instance.getDefaultNodeData = vi.fn().mockReturnValue({}); + const listener = vi.fn(); + instance.store.addEventListener('nodes-changed', listener); + + instance.createNode('stage', 10, 20); + + expect(listener).toHaveBeenCalledTimes(1); + expect(listener.mock.calls[0][0].detail.nodes).toEqual(instance.nodes); + }); }); From 570961a1fa83abbd359e3f218e22a3dcbcc5a66b Mon Sep 17 00:00:00 2001 From: Maria Violante Date: Mon, 10 Aug 2026 13:13:06 -0400 Subject: [PATCH 6/6] Fix: finishConnection bypassing store setter Same class of bug as createNode/createStartNode fix: push() mutated this.connections in place. --- .../js/workflow-builder.js | 4 +- .../connectionCreation.test.js | 75 +++++++++++++++++++ 2 files changed, 77 insertions(+), 2 deletions(-) create mode 100644 tests_js/workflow-builder/connectionCreation.test.js diff --git a/django_forms_workflows/static/django_forms_workflows/js/workflow-builder.js b/django_forms_workflows/static/django_forms_workflows/js/workflow-builder.js index 166139b..9155afb 100644 --- a/django_forms_workflows/static/django_forms_workflows/js/workflow-builder.js +++ b/django_forms_workflows/static/django_forms_workflows/js/workflow-builder.js @@ -2915,10 +2915,10 @@ export class WorkflowBuilder { if (!exists) { console.log('Creating new connection'); - this.connections.push({ + this.connections = [...this.connections, { from: this.connectionStart.nodeId, to: toNodeId - }); + }]; this.selectedConnection = this.connections.length - 1; this.updateConnectionSelectionUI(); this.render(); diff --git a/tests_js/workflow-builder/connectionCreation.test.js b/tests_js/workflow-builder/connectionCreation.test.js new file mode 100644 index 0000000..4478926 --- /dev/null +++ b/tests_js/workflow-builder/connectionCreation.test.js @@ -0,0 +1,75 @@ +import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest'; +import { WorkflowBuilder } from '../../django_forms_workflows/static/django_forms_workflows/js/workflow-builder.js'; +import { createWorkflowBuilderStore } from '../../django_forms_workflows/static/django_forms_workflows/js/workflow-builder-store.js'; + +// Same bare-instance pattern as nodeCreation.test.js: enough stubbed state +// for finishConnection() to run without the real DOM/canvas. +function createInstance() { + const instance = Object.create(WorkflowBuilder.prototype); + instance.store = createWorkflowBuilderStore(); + instance.connectionStart = { nodeId: 'node_1' }; + instance.selectedConnection = null; + instance.render = vi.fn(); + return instance; +} + +function inputPointEvent(nodeId) { + const point = document.createElement('div'); + point.className = 'connection-point'; + point.dataset.point = 'input'; + point.dataset.nodeId = nodeId; + document.body.appendChild(point); + return { target: point }; +} + +beforeEach(() => { + vi.restoreAllMocks(); +}); + +afterEach(() => { + vi.restoreAllMocks(); + document.body.innerHTML = ''; +}); + +describe('WorkflowBuilder#finishConnection', () => { + it('appends the new connection and selects it', () => { + const instance = createInstance(); + + instance.finishConnection(inputPointEvent('node_2')); + + expect(instance.connections).toEqual([{ from: 'node_1', to: 'node_2' }]); + expect(instance.selectedConnection).toBe(0); + expect(instance.render).toHaveBeenCalledTimes(1); + }); + + it('adds the connection through the store setter, so connections-changed listeners see it', () => { + const instance = createInstance(); + const listener = vi.fn(); + instance.store.addEventListener('connections-changed', listener); + + instance.finishConnection(inputPointEvent('node_2')); + + expect(listener).toHaveBeenCalledTimes(1); + expect(listener.mock.calls[0][0].detail.connections).toEqual(instance.connections); + }); + + it('does not add a duplicate connection that already exists', () => { + const instance = createInstance(); + instance.connections = [{ from: 'node_1', to: 'node_2' }]; + const listener = vi.fn(); + instance.store.addEventListener('connections-changed', listener); + + instance.finishConnection(inputPointEvent('node_2')); + + expect(instance.connections).toEqual([{ from: 'node_1', to: 'node_2' }]); + expect(listener).not.toHaveBeenCalled(); + }); + + it('does not add a connection back to the same node', () => { + const instance = createInstance(); + + instance.finishConnection(inputPointEvent('node_1')); + + expect(instance.connections).toEqual([]); + }); +});