From 5aa44c3b167b69f5159898a4ce016ad695406b87 Mon Sep 17 00:00:00 2001 From: Quang Tran <16215255+trmquang93@users.noreply.github.com> Date: Mon, 27 Apr 2026 07:24:00 +0700 Subject: [PATCH] fix: keep hotspot conditional branches coherent across UI and persistence MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Conditional connections carry two indicators — `connectionPath` (renderer) and `conditionGroupId` (modal/grouping). They could drift apart in three flows, producing branches that rendered as conditional but reopened as plain navigate (or vice versa): - saveHotspot wrote per-branch connections without a shared conditionGroupId - deleteConnection on a hotspot branch left the owning hotspot.conditions stale - convertToConditionalGroup / addToConditionalGroup updated connections only, not the hotspot, so HotspotModal opened with a stale single target - legacy v1–v9 .drawd files could load with mismatched indicators Fixes: - New canonical `isConditionalConnection` predicate in utils/connectionHelpers.js - saveHotspot now stamps a single shared conditionGroupId across each branch and its api-success/api-error sub-branches - deleteConnection rebuilds hotspot.conditions from surviving branches, collapsing back to plain navigate when only one remains - convert/addToConditionalGroup mirror the new branch onto the hotspot - importFlow heals legacy mismatches in two idempotent passes --- src/components/ConnectionEditModal.jsx | 3 +- src/hooks/useInteractionCallbacks.js | 7 +- src/hooks/useScreenManager.js | 173 ++++++++++++++++++++ src/hooks/useScreenManager.test.js | 218 +++++++++++++++++++++++++ src/utils/connectionHelpers.js | 13 ++ src/utils/importFlow.js | 52 ++++++ src/utils/importFlow.test.js | 138 ++++++++++++++++ 7 files changed, 600 insertions(+), 4 deletions(-) create mode 100644 src/utils/connectionHelpers.js diff --git a/src/components/ConnectionEditModal.jsx b/src/components/ConnectionEditModal.jsx index dd8e73c..08d1384 100644 --- a/src/components/ConnectionEditModal.jsx +++ b/src/components/ConnectionEditModal.jsx @@ -1,11 +1,12 @@ import { useState } from "react"; import { COLORS, styles } from "../styles/theme"; import { generateId } from "../utils/generateId"; +import { isConditionalConnection } from "../utils/connectionHelpers"; import { DataFlowEditor } from "./DataFlowEditor"; import { TRANSITION_TYPES } from "../constants"; export function ConnectionEditModal({ connection, groupConnections, screens, fromScreen, onSave, onDelete, onClose }) { - const isConditional = groupConnections.length > 1 || !!connection.conditionGroupId; + const isConditional = isConditionalConnection(connection) || groupConnections.length > 1; const [mode, setMode] = useState(isConditional ? "conditional" : "navigate"); const [label, setLabel] = useState(connection.label || ""); diff --git a/src/hooks/useInteractionCallbacks.js b/src/hooks/useInteractionCallbacks.js index 95fcca3..5503664 100644 --- a/src/hooks/useInteractionCallbacks.js +++ b/src/hooks/useInteractionCallbacks.js @@ -1,5 +1,6 @@ import { useCallback, useEffect } from "react"; import { HEADER_HEIGHT, DEFAULT_SCREEN_WIDTH, DEFAULT_SCREEN_HEIGHT } from "../constants"; +import { isConditionalConnection } from "../utils/connectionHelpers"; export function useInteractionCallbacks({ screens, connections, stickyNotes, @@ -30,7 +31,7 @@ export function useInteractionCallbacks({ const hotspot = screen.hotspots.find((h) => h.id === conn.hotspotId); if (hotspot) setHotspotModal({ screen, hotspot, connection: conn }); } else { - const groupConns = conn.conditionGroupId + const groupConns = isConditionalConnection(conn) ? connections.filter((c) => c.conditionGroupId === conn.conditionGroupId) : [conn]; setConnectionEditModal({ connection: conn, groupConnections: groupConns, fromScreen: screen }); @@ -48,7 +49,7 @@ export function useInteractionCallbacks({ ); // Case 1: Already a conditional group — add a new branch - const existingGroup = existingHotspotConns.find((c) => c.conditionGroupId); + const existingGroup = existingHotspotConns.find(isConditionalConnection); if (existingGroup) { const isDuplicate = existingHotspotConns.some((c) => c.toScreenId === targetScreenId); if (!isDuplicate) { @@ -83,7 +84,7 @@ export function useInteractionCallbacks({ const existingPlain = connections.filter((c) => c.fromScreenId === fromId && !c.hotspotId); - const existingGroup = existingPlain.find((c) => c.conditionGroupId); + const existingGroup = existingPlain.find(isConditionalConnection); if (existingGroup) { addToConditionalGroup(fromId, targetScreenId, existingGroup.conditionGroupId); setEditingConditionGroup(existingGroup.conditionGroupId); diff --git a/src/hooks/useScreenManager.js b/src/hooks/useScreenManager.js index cba9c8b..830f212 100644 --- a/src/hooks/useScreenManager.js +++ b/src/hooks/useScreenManager.js @@ -602,6 +602,11 @@ export function useScreenManager(pan, zoom, canvasRef, commentCallbacks = {}) { if (hotspot.action === "conditional") { setConnections((prev) => { let updated = prev.filter((c) => c.hotspotId !== hotspot.id); + // Single shared conditionGroupId for every branch (and its api-success/ + // api-error sub-branches) belonging to this hotspot. Keeps the canonical + // `isConditionalConnection` predicate true for these connections, matching + // what convertToConditionalGroup / addToConditionalGroup write at drag time. + const groupId = generateId(); (hotspot.conditions || []).forEach((cond, i) => { const branchAction = cond.action || "navigate"; @@ -618,6 +623,7 @@ export function useScreenManager(pan, zoom, canvasRef, commentCallbacks = {}) { action: branchAction, connectionPath: `condition-${i}`, condition: cond.label || "", + conditionGroupId: groupId, transitionType: null, transitionLabel: "", dataFlow: cond.dataFlow || [], @@ -637,6 +643,7 @@ export function useScreenManager(pan, zoom, canvasRef, commentCallbacks = {}) { action: successAction, connectionPath: `condition-${i}-api-success`, condition: cond.label || "", + conditionGroupId: groupId, transitionType: null, transitionLabel: "", dataFlow: cond.onSuccessDataFlow || [], @@ -653,6 +660,7 @@ export function useScreenManager(pan, zoom, canvasRef, commentCallbacks = {}) { action: errorAction, connectionPath: `condition-${i}-api-error`, condition: cond.label || "", + conditionGroupId: groupId, transitionType: null, transitionLabel: "", dataFlow: cond.onErrorDataFlow || [], @@ -818,7 +826,86 @@ export function useScreenManager(pan, zoom, canvasRef, commentCallbacks = {}) { const deleteConnection = useCallback((connectionId) => { commentCallbacksRef.current.onDeleteCommentsForConnection?.(connectionId); pushHistory(screens, connections, documents); + + // Snapshot the connection being deleted so we can sync the owning hotspot + // (if any) below. Captured from `connections` because setConnections runs + // asynchronously and we need this value during setScreens scheduling. + const deletedConn = connections.find((c) => c.id === connectionId); + const isHotspotConditionalBranch = + deletedConn?.hotspotId && /^condition-\d+$/.test(deletedConn.connectionPath || ""); + setConnections((prev) => prev.filter((c) => c.id !== connectionId)); + + // If the deleted connection was one branch of a hotspot's conditional group, + // shrink (or revert) the hotspot's `conditions[]` so HotspotModal stays + // coherent with the canvas. Without this, on next saveHotspot the deleted + // branch would resurrect from the stale hotspot.conditions array. + if (isHotspotConditionalBranch) { + // Remaining main-branch conditional connections for this hotspot, in their + // existing on-canvas order (we ignore api-success/api-error sub-branches + // here — they're tied to a parent condition, not a branch on their own). + const remaining = connections.filter( + (c) => + c.id !== connectionId && + c.hotspotId === deletedConn.hotspotId && + /^condition-\d+$/.test(c.connectionPath || "") + ); + + setScreens((prev) => + prev.map((s) => { + if (s.id !== deletedConn.fromScreenId) return s; + return { + ...s, + hotspots: s.hotspots.map((h) => { + if (h.id !== deletedConn.hotspotId) return h; + + // Empty: revert hotspot to a default navigate-with-no-target state. + if (remaining.length === 0) { + return { ...h, action: "navigate", targetScreenId: null, conditions: [] }; + } + // One branch left: collapse back to plain navigate. The remaining + // connection is left intact (renderer still colors it by its + // existing condition-N path until next saveHotspot normalizes). + if (remaining.length === 1) { + return { + ...h, + action: "navigate", + targetScreenId: remaining[0].toScreenId || null, + conditions: [], + }; + } + // Two or more left: keep conditional. Rebuild conditions from the + // surviving connections, preserving any rich per-condition data + // (api/custom/dataFlow) by matching label + target against the + // previous `hotspot.conditions` snapshot. + const previousConditions = h.conditions || []; + const newConditions = remaining.map((c) => { + const match = previousConditions.find( + (pc) => + (pc.label || "") === (c.condition || "") && + (pc.targetScreenId || "") === (c.toScreenId || "") + ); + return ( + match || { + id: generateId(), + label: c.condition || c.label || "", + targetScreenId: c.toScreenId || "", + action: "navigate", + dataFlow: c.dataFlow || [], + } + ); + }); + return { + ...h, + action: "conditional", + targetScreenId: null, + conditions: newConditions, + }; + }), + }; + }) + ); + } }, [screens, connections, documents, pushHistory]); const saveConnectionGroup = useCallback((originalConnId, payload) => { @@ -889,6 +976,7 @@ export function useScreenManager(pan, zoom, canvasRef, commentCallbacks = {}) { const convertToConditionalGroup = useCallback((existingConnId, fromScreenId, toScreenId, hotspotId = null) => { pushHistory(screens, connections, documents); const groupId = generateId(); + const existingConn = connections.find((c) => c.id === existingConnId) || null; setConnections((prev) => { const updated = prev.map((c) => c.id === existingConnId @@ -910,6 +998,45 @@ export function useScreenManager(pan, zoom, canvasRef, commentCallbacks = {}) { dataFlow: [], }]; }); + // When converting a hotspot-backed connection into a conditional group, + // sync the hotspot itself so it becomes the source of truth (saveHotspot + // and HotspotModal both read from `hotspot.action` / `hotspot.conditions`). + // Without this, double-clicking the new conditional branch would open + // HotspotModal in stale "navigate" mode pointing at the old single target. + if (hotspotId) { + setScreens((prev) => + prev.map((s) => { + if (s.id !== fromScreenId) return s; + return { + ...s, + hotspots: s.hotspots.map((h) => { + if (h.id !== hotspotId) return h; + return { + ...h, + action: "conditional", + targetScreenId: null, + conditions: [ + { + id: generateId(), + label: existingConn?.label || "", + targetScreenId: existingConn?.toScreenId || "", + action: "navigate", + dataFlow: existingConn?.dataFlow || [], + }, + { + id: generateId(), + label: "", + targetScreenId: toScreenId, + action: "navigate", + dataFlow: [], + }, + ], + }; + }), + }; + }) + ); + } return groupId; }, [screens, connections, documents, pushHistory]); @@ -936,6 +1063,49 @@ export function useScreenManager(pan, zoom, canvasRef, commentCallbacks = {}) { dataFlow: [], }]; }); + // When adding to a hotspot-backed conditional group, mirror the new branch + // onto the hotspot so HotspotModal opens with the full set of conditions. + // If a previous code path (legacy state, or a save round-trip) left the + // hotspot's `conditions` empty but the connection group populated, we + // reconstruct the conditions from the existing group connections first. + if (hotspotId) { + const existingGroupConns = connections.filter((c) => c.conditionGroupId === conditionGroupId); + setScreens((prev) => + prev.map((s) => { + if (s.id !== fromScreenId) return s; + return { + ...s, + hotspots: s.hotspots.map((h) => { + if (h.id !== hotspotId) return h; + const baseConditions = (h.conditions && h.conditions.length > 0) + ? h.conditions + : existingGroupConns.map((c) => ({ + id: generateId(), + label: c.label || "", + targetScreenId: c.toScreenId || "", + action: "navigate", + dataFlow: c.dataFlow || [], + })); + return { + ...h, + action: "conditional", + targetScreenId: null, + conditions: [ + ...baseConditions, + { + id: generateId(), + label: "", + targetScreenId: toScreenId, + action: "navigate", + dataFlow: [], + }, + ], + }; + }), + }; + }) + ); + } }, [screens, connections, documents, pushHistory]); const addConnection = useCallback((fromScreenId, toScreenId) => { @@ -952,6 +1122,9 @@ export function useScreenManager(pan, zoom, canvasRef, commentCallbacks = {}) { hotspotId: null, label: "", action: "navigate", + connectionPath: "default", + condition: "", + conditionGroupId: null, transitionType: null, transitionLabel: "", dataFlow: [], diff --git a/src/hooks/useScreenManager.test.js b/src/hooks/useScreenManager.test.js index 7c6399f..24624d8 100644 --- a/src/hooks/useScreenManager.test.js +++ b/src/hooks/useScreenManager.test.js @@ -365,6 +365,36 @@ describe("saveHotspot connection management", () => { expect(result.current.connections[1].connectionPath).toBe("condition-1"); }); + it("conditional hotspot connections share a single non-null conditionGroupId", () => { + const { result } = setup(); + act(() => result.current.addScreen(null, "A")); + act(() => result.current.addScreen(null, "B")); + act(() => result.current.addScreen(null, "C")); + const idA = result.current.screens[0].id; + const idB = result.current.screens[1].id; + const idC = result.current.screens[2].id; + + const hotspot = { + id: "hs1", label: "Check", x: 10, y: 10, w: 20, h: 20, + action: "conditional", + conditions: [ + { id: "c1", label: "logged in", targetScreenId: idB }, + { id: "c2", label: "guest", targetScreenId: idC }, + ], + }; + act(() => result.current.saveHotspot(idA, hotspot)); + + const groupId = result.current.connections[0].conditionGroupId; + expect(groupId).toBeTruthy(); + expect(result.current.connections[1].conditionGroupId).toBe(groupId); + // Re-saving generates a fresh groupId (connections are wiped + recreated), + // but every branch must still share whatever groupId is in effect. + act(() => result.current.saveHotspot(idA, hotspot)); + const newGroupId = result.current.connections[0].conditionGroupId; + expect(newGroupId).toBeTruthy(); + expect(result.current.connections[1].conditionGroupId).toBe(newGroupId); + }); + it("saving hotspot with action='back' does NOT create a connection", () => { const { result } = setup(); act(() => result.current.addScreen(null, "A")); @@ -734,6 +764,36 @@ describe("convertToConditionalGroup", () => { expect(groupId).toBeTruthy(); expect(result.current.connections[1].conditionGroupId).toBe(groupId); }); + + it("hotspot-backed conversion syncs hotspot.action='conditional' with conditions", () => { + const { result } = setup(); + act(() => result.current.addScreen(null, "A")); + act(() => result.current.addScreen(null, "B")); + act(() => result.current.addScreen(null, "C")); + const idA = result.current.screens[0].id; + const idB = result.current.screens[1].id; + const idC = result.current.screens[2].id; + + // Seed a hotspot in a "navigate" state pointing at B (mirrors what + // quickConnectHotspot leaves behind after the first drag from a hotspot). + const hotspot = { + id: "hs1", label: "Tap", x: 10, y: 10, w: 20, h: 20, + action: "navigate", targetScreenId: idB, + }; + act(() => result.current.saveHotspot(idA, hotspot)); + expect(result.current.connections).toHaveLength(1); + const connId = result.current.connections[0].id; + + // Second drag from the same hotspot to C → convertToConditionalGroup with hotspotId. + act(() => result.current.convertToConditionalGroup(connId, idA, idC, "hs1")); + + const updatedHotspot = result.current.screens[0].hotspots.find((h) => h.id === "hs1"); + expect(updatedHotspot.action).toBe("conditional"); + expect(updatedHotspot.targetScreenId).toBeNull(); + expect(updatedHotspot.conditions).toHaveLength(2); + expect(updatedHotspot.conditions[0].targetScreenId).toBe(idB); + expect(updatedHotspot.conditions[1].targetScreenId).toBe(idC); + }); }); describe("addToConditionalGroup", () => { @@ -781,6 +841,164 @@ describe("addToConditionalGroup", () => { act(() => result.current.addToConditionalGroup(idA, idD, groupId)); expect(result.current.connections[2].conditionGroupId).toBe(groupId); }); + + it("hotspot-backed addTo appends to hotspot.conditions", () => { + const { result } = setup(); + act(() => result.current.addScreen(null, "A")); + act(() => result.current.addScreen(null, "B")); + act(() => result.current.addScreen(null, "C")); + act(() => result.current.addScreen(null, "D")); + const idA = result.current.screens[0].id; + const idB = result.current.screens[1].id; + const idC = result.current.screens[2].id; + const idD = result.current.screens[3].id; + + // Seed: hotspot navigating to B (quickConnectHotspot output). + act(() => result.current.saveHotspot(idA, { + id: "hs1", label: "Tap", x: 10, y: 10, w: 20, h: 20, + action: "navigate", targetScreenId: idB, + })); + const connId = result.current.connections[0].id; + + // Drag #2: convert to a conditional group (B + C). + let groupId; + act(() => { groupId = result.current.convertToConditionalGroup(connId, idA, idC, "hs1"); }); + + // Drag #3: add D to the same hotspot's conditional group. + act(() => result.current.addToConditionalGroup(idA, idD, groupId, "hs1")); + + const updatedHotspot = result.current.screens[0].hotspots.find((h) => h.id === "hs1"); + expect(updatedHotspot.action).toBe("conditional"); + expect(updatedHotspot.conditions).toHaveLength(3); + expect(updatedHotspot.conditions.map((c) => c.targetScreenId)).toEqual([idB, idC, idD]); + }); +}); + +describe("deleteConnection hotspot sync", () => { + it("deleting one of two hotspot conditional branches reverts hotspot to navigate", () => { + const { result } = setup(); + act(() => result.current.addScreen(null, "A")); + act(() => result.current.addScreen(null, "B")); + act(() => result.current.addScreen(null, "C")); + const idA = result.current.screens[0].id; + const idB = result.current.screens[1].id; + const idC = result.current.screens[2].id; + + // Set up a hotspot-backed conditional group via the drag flow. + act(() => result.current.saveHotspot(idA, { + id: "hs1", label: "Tap", x: 10, y: 10, w: 20, h: 20, + action: "navigate", targetScreenId: idB, + })); + const firstConnId = result.current.connections[0].id; + act(() => result.current.convertToConditionalGroup(firstConnId, idA, idC, "hs1")); + expect(result.current.connections).toHaveLength(2); + + // Delete the second branch (the C-bound one). + const cConn = result.current.connections.find((c) => c.toScreenId === idC); + act(() => result.current.deleteConnection(cConn.id)); + + expect(result.current.connections).toHaveLength(1); + const hotspot = result.current.screens[0].hotspots.find((h) => h.id === "hs1"); + expect(hotspot.action).toBe("navigate"); + expect(hotspot.targetScreenId).toBe(idB); + expect(hotspot.conditions).toEqual([]); + }); + + it("deleting one of three hotspot conditional branches keeps conditional with two", () => { + const { result } = setup(); + act(() => result.current.addScreen(null, "A")); + act(() => result.current.addScreen(null, "B")); + act(() => result.current.addScreen(null, "C")); + act(() => result.current.addScreen(null, "D")); + const idA = result.current.screens[0].id; + const idB = result.current.screens[1].id; + const idC = result.current.screens[2].id; + const idD = result.current.screens[3].id; + + act(() => result.current.saveHotspot(idA, { + id: "hs1", label: "Tap", x: 10, y: 10, w: 20, h: 20, + action: "navigate", targetScreenId: idB, + })); + const firstConnId = result.current.connections[0].id; + let groupId; + act(() => { groupId = result.current.convertToConditionalGroup(firstConnId, idA, idC, "hs1"); }); + act(() => result.current.addToConditionalGroup(idA, idD, groupId, "hs1")); + expect(result.current.connections).toHaveLength(3); + + // Delete the middle branch (the C-bound one). + const cConn = result.current.connections.find((c) => c.toScreenId === idC); + act(() => result.current.deleteConnection(cConn.id)); + + expect(result.current.connections).toHaveLength(2); + const hotspot = result.current.screens[0].hotspots.find((h) => h.id === "hs1"); + expect(hotspot.action).toBe("conditional"); + expect(hotspot.conditions).toHaveLength(2); + expect(hotspot.conditions.map((c) => c.targetScreenId).sort()).toEqual([idB, idD].sort()); + }); + + it("deleting a non-hotspot connection does not mutate any hotspot", () => { + const { result } = setup(); + act(() => result.current.addScreen(null, "A")); + act(() => result.current.addScreen(null, "B")); + const idA = result.current.screens[0].id; + const idB = result.current.screens[1].id; + + // A hotspot in conditional state, untouched by the deletion under test. + act(() => result.current.saveHotspot(idA, { + id: "hs1", label: "Check", x: 10, y: 10, w: 20, h: 20, + action: "conditional", + conditions: [ + { id: "c1", label: "x", targetScreenId: idB }, + { id: "c2", label: "y", targetScreenId: idB }, + ], + })); + const hotspotBefore = result.current.screens[0].hotspots.find((h) => h.id === "hs1"); + + // Add a plain (non-hotspot) connection and delete it. + act(() => result.current.addConnection(idA, idB)); + const plainConn = result.current.connections.find((c) => !c.hotspotId); + act(() => result.current.deleteConnection(plainConn.id)); + + const hotspotAfter = result.current.screens[0].hotspots.find((h) => h.id === "hs1"); + expect(hotspotAfter).toEqual(hotspotBefore); + }); + + it("preserves rich per-condition data when shrinking from 3 → 2 branches", () => { + const { result } = setup(); + act(() => result.current.addScreen(null, "A")); + act(() => result.current.addScreen(null, "B")); + act(() => result.current.addScreen(null, "C")); + act(() => result.current.addScreen(null, "D")); + const idA = result.current.screens[0].id; + const idB = result.current.screens[1].id; + const idC = result.current.screens[2].id; + const idD = result.current.screens[3].id; + + // Three rich conditions (each with a distinct label so label+target match + // is unambiguous when we rebuild after the deletion). + act(() => result.current.saveHotspot(idA, { + id: "hs1", label: "Check", x: 10, y: 10, w: 20, h: 20, + action: "conditional", + conditions: [ + { id: "c1", label: "alpha", targetScreenId: idB, action: "navigate", dataFlow: [{ key: "u" }] }, + { id: "c2", label: "beta", targetScreenId: idC, action: "navigate", dataFlow: [{ key: "v" }] }, + { id: "c3", label: "gamma", targetScreenId: idD, action: "navigate", dataFlow: [{ key: "w" }] }, + ], + })); + expect(result.current.connections).toHaveLength(3); + + // Delete the C-bound branch. + const cConn = result.current.connections.find((c) => c.toScreenId === idC); + act(() => result.current.deleteConnection(cConn.id)); + + const hotspot = result.current.screens[0].hotspots.find((h) => h.id === "hs1"); + expect(hotspot.conditions).toHaveLength(2); + const labels = hotspot.conditions.map((c) => c.label); + expect(labels).toEqual(["alpha", "gamma"]); + // Original dataFlow objects should be carried over via the label+target match. + expect(hotspot.conditions[0].dataFlow).toEqual([{ key: "u" }]); + expect(hotspot.conditions[1].dataFlow).toEqual([{ key: "w" }]); + }); }); describe("pasteHotspots", () => { diff --git a/src/utils/connectionHelpers.js b/src/utils/connectionHelpers.js new file mode 100644 index 0000000..70a3aea --- /dev/null +++ b/src/utils/connectionHelpers.js @@ -0,0 +1,13 @@ +/** + * Canonical predicate for "is this connection part of a conditional branch group?" + * + * A connection has historically carried two redundant indicators of "conditional": + * - `connectionPath` starting with "condition-" (used by the renderer for color) + * - `conditionGroupId` (used by the modal and double-click handler to gather siblings) + * + * `conditionGroupId` is canonical because only it identifies which other connections + * belong to the same branch group. `importFlow.js` backfills both fields so the two + * agree on every connection coming out of the loader; this helper centralizes the + * predicate so call sites can't drift again. + */ +export const isConditionalConnection = (conn) => !!conn?.conditionGroupId; diff --git a/src/utils/importFlow.js b/src/utils/importFlow.js index 1622fbb..b18dadc 100644 --- a/src/utils/importFlow.js +++ b/src/utils/importFlow.js @@ -127,6 +127,58 @@ export function importFlow(fileText) { if (!Array.isArray(conn.dataFlow)) conn.dataFlow = []; } + // Backward compat: heal mismatched conditional indicators. + // + // Legacy files (v1–v9) may carry connections where `connectionPath` says "condition-N" + // but `conditionGroupId` is null, or vice versa. The renderer keys on `connectionPath` + // while the modal/double-click keys on `conditionGroupId`, so a mismatch produces a + // connection that *renders* conditional but *opens* in plain navigate mode. + // + // Two passes, both idempotent on already-coherent files: + // Pass A — synthesize missing `conditionGroupId` for connections with a "condition-*" + // path, grouped by (fromScreenId, hotspotId || "__none__"). + // Pass B — synthesize a missing/non-conditional `connectionPath` for connections that + // have a `conditionGroupId`, choosing the next free "condition-N" index in + // the group. + { + // Pass A: connectionPath → conditionGroupId + const groupIdByKey = new Map(); + for (const conn of data.connections) { + const isCondPath = typeof conn.connectionPath === "string" && conn.connectionPath.startsWith("condition-"); + if (!isCondPath || conn.conditionGroupId) continue; + const key = `${conn.fromScreenId}__${conn.hotspotId || "__none__"}`; + if (!groupIdByKey.has(key)) groupIdByKey.set(key, generateId()); + conn.conditionGroupId = groupIdByKey.get(key); + } + + // Pass B: conditionGroupId → connectionPath + const groupMembers = new Map(); + for (const conn of data.connections) { + if (!conn.conditionGroupId) continue; + if (!groupMembers.has(conn.conditionGroupId)) groupMembers.set(conn.conditionGroupId, []); + groupMembers.get(conn.conditionGroupId).push(conn); + } + for (const members of groupMembers.values()) { + const usedIndices = new Set(); + for (const m of members) { + const match = m.connectionPath?.match(/^condition-(\d+)$/); + if (match) usedIndices.add(parseInt(match[1], 10)); + } + let next = 0; + const nextFreeIndex = () => { + while (usedIndices.has(next)) next++; + const v = next; + usedIndices.add(v); + next++; + return v; + }; + for (const m of members) { + const isCondPath = typeof m.connectionPath === "string" && m.connectionPath.startsWith("condition-"); + if (!isCondPath) m.connectionPath = `condition-${nextFreeIndex()}`; + } + } + } + // Backward compat: featureBrief if (!data.metadata) data.metadata = {}; if (!data.metadata.featureBrief) data.metadata.featureBrief = ""; diff --git a/src/utils/importFlow.test.js b/src/utils/importFlow.test.js index 6d154bf..5103a5a 100644 --- a/src/utils/importFlow.test.js +++ b/src/utils/importFlow.test.js @@ -313,4 +313,142 @@ describe("importFlow", () => { expect(result.version).toBe(7); expect(result.screens[0].notes).toBe("Some implementation notes"); }); + + // --- Conditional connector backfill (Backlog 8.1) --- + // See src/utils/connectionHelpers.js — `conditionGroupId` is the canonical predicate. + // These cases verify that legacy files with a mismatch between `connectionPath` and + // `conditionGroupId` get healed during import so the renderer and the modal agree. + + describe("conditional connector backfill", () => { + const makeConn = (overrides) => ({ + id: overrides.id, + fromScreenId: "src", + toScreenId: "dst", + hotspotId: null, + label: "", + action: "navigate", + connectionPath: "default", + condition: "", + conditionGroupId: null, + transitionType: null, + transitionLabel: "", + dataFlow: [], + ...overrides, + }); + + it("synthesizes a shared conditionGroupId for siblings sharing fromScreen+hotspot", () => { + const file = makeValidFile({ + version: 5, + connections: [ + makeConn({ id: "c0", connectionPath: "condition-0", toScreenId: "a" }), + makeConn({ id: "c1", connectionPath: "condition-1", toScreenId: "b" }), + ], + }); + const result = importFlow(file); + const [c0, c1] = result.connections; + expect(c0.conditionGroupId).toBeTruthy(); + expect(c1.conditionGroupId).toBe(c0.conditionGroupId); + }); + + it("uses distinct synthesized group ids for connections from different sources", () => { + const file = makeValidFile({ + version: 5, + connections: [ + makeConn({ id: "c0", fromScreenId: "src1", connectionPath: "condition-0", toScreenId: "a" }), + makeConn({ id: "c1", fromScreenId: "src2", connectionPath: "condition-0", toScreenId: "b" }), + ], + }); + const result = importFlow(file); + const [c0, c1] = result.connections; + expect(c0.conditionGroupId).toBeTruthy(); + expect(c1.conditionGroupId).toBeTruthy(); + expect(c0.conditionGroupId).not.toBe(c1.conditionGroupId); + }); + + it("keeps a coherent conditional connection unchanged", () => { + const file = makeValidFile({ + version: 5, + connections: [ + makeConn({ + id: "c0", + connectionPath: "condition-0", + conditionGroupId: "g-existing", + toScreenId: "a", + }), + ], + }); + const result = importFlow(file); + expect(result.connections[0].connectionPath).toBe("condition-0"); + expect(result.connections[0].conditionGroupId).toBe("g-existing"); + }); + + it("synthesizes a connectionPath when a connection has only a conditionGroupId", () => { + const file = makeValidFile({ + version: 5, + connections: [ + makeConn({ + id: "c0", + connectionPath: "default", + conditionGroupId: "g1", + toScreenId: "a", + }), + ], + }); + const result = importFlow(file); + expect(result.connections[0].connectionPath).toBe("condition-0"); + expect(result.connections[0].conditionGroupId).toBe("g1"); + }); + + it("picks a non-colliding condition index when synthesizing within an existing group", () => { + const file = makeValidFile({ + version: 5, + connections: [ + makeConn({ + id: "c0", + connectionPath: "condition-0", + conditionGroupId: "g1", + toScreenId: "a", + }), + makeConn({ + id: "c1", + connectionPath: "default", + conditionGroupId: "g1", + toScreenId: "b", + }), + ], + }); + const result = importFlow(file); + const paths = result.connections.map((c) => c.connectionPath).sort(); + expect(paths).toEqual(["condition-0", "condition-1"]); + }); + + it("does not merge a hotspot conditional with a plain conditional from the same screen", () => { + const file = makeValidFile({ + version: 5, + connections: [ + makeConn({ id: "c0", hotspotId: "h1", connectionPath: "condition-0", toScreenId: "a" }), + makeConn({ id: "c1", hotspotId: null, connectionPath: "condition-0", toScreenId: "b" }), + ], + }); + const result = importFlow(file); + const [c0, c1] = result.connections; + expect(c0.conditionGroupId).toBeTruthy(); + expect(c1.conditionGroupId).toBeTruthy(); + expect(c0.conditionGroupId).not.toBe(c1.conditionGroupId); + }); + + it("is idempotent on a healed file (re-import produces no further changes)", () => { + const initialFile = makeValidFile({ + version: 5, + connections: [ + makeConn({ id: "c0", connectionPath: "condition-0", toScreenId: "a" }), + makeConn({ id: "c1", connectionPath: "condition-1", toScreenId: "b" }), + ], + }); + const firstPass = importFlow(initialFile); + // Re-stringify and re-import — should be a fixed point. + const secondPass = importFlow(JSON.stringify({ ...firstPass, version: 5 })); + expect(secondPass.connections).toEqual(firstPass.connections); + }); + }); });