From 7c2c8a9c4986b0631514d7721ab89aeb8e8a134b Mon Sep 17 00:00:00 2001 From: Lucy Macartney Date: Fri, 7 Nov 2025 08:58:34 -0500 Subject: [PATCH 1/2] Add version mismatch warning banner to collaborative editor Display warning banner when viewing latest workflow version but selected run was executed on an older version. This prevents user confusion when workflow structure has changed since the run executed (jobs added, removed, or modified). Features: - VersionMismatchBanner component using bg-yellow-100 styling to match other warning elements (VersionDropdown, credential badges) - useVersionMismatch hook encapsulating detection logic - Full-width banner at top of canvas, consistent with LiveView alerts - 8 passing tests covering all edge cases and null safety The banner shows when: - A run is selected from history - Viewing "latest" workflow (not a specific snapshot) - Run's version differs from current workflow version Fix history page 500 error and workflow filtering Fixes two related bugs that prevented the history page from loading when navigating from the collaborative editor: 1. Backend: Fix MatchError crash in humanize_workflow - Handle nil case when Enum.find doesn't find the workflow - Previously crashed with "no match of right hand side value: nil" - Now returns empty string when workflow not found 2. Frontend: Fix workflow ID extraction in MiniHistory - Extract workflow ID from position [1] instead of last element - Previously took "collaborate" instead of the actual workflow UUID - Now works for both /w/:id and /w/:id/collaborate URLs The bug occurred when clicking "View History" from the collaborative editor because the frontend was sending workflow_id="collaborate" which the backend couldn't find, causing a 500 error. Add test coverage for missing workflow in humanize_search_params Adds test case to cover the nil branch when a workflow_id filter references a workflow that doesn't exist in the workflows list. Fix node position warnings and layout jumps on initial render When viewing workflow runs (especially failed runs), nodes would trigger "could not auto-calculate position" warnings during initial render and briefly appear at incorrect positions before jumping to their correct layout. This happened after page refresh when the positions cache starts empty before the layout system runs, which is expected behavior. Changes: - Only log warning if positions map has data but parent still not found - Silently default to {x: 0, y: 0} when positions cache is empty on first render - Eliminates console spam and improves render performance for failed run views Auto-switch workflow version when selecting run from history When clicking a run in the MiniHistory panel, automatically switch the workflow view to display the version that was used when that run executed. This matches the LiveView editor behavior and eliminates confusion about viewing mismatched versions. Changes: - Auto-detect run's workflow version and switch to it on selection - Show version mismatch warning for ANY version mismatch, not just when viewing latest (fixes issue where warning didn't appear when manually switching between historical versions) - URL now includes both run and version parameters (?run=&v=) Fixes the issue where IDE header would show "latest" while viewing a run from an older version, causing unnecessary version mismatch warnings. Update CHANGELOG for PR #3941 --- CHANGELOG.md | 2 + .../diagram/CollaborativeWorkflowDiagram.tsx | 43 +++- .../components/diagram/MiniHistory.tsx | 7 +- .../diagram/VersionMismatchBanner.tsx | 45 ++++ .../hooks/useVersionMismatch.ts | 64 ++++++ .../util/ensure-node-position.ts | 12 +- .../hooks/useVersionMismatch.test.tsx | 211 ++++++++++++++++++ lib/lightning_web/live/run_live/components.ex | 9 +- .../live/run_live/components_test.exs | 28 +++ 9 files changed, 406 insertions(+), 15 deletions(-) create mode 100644 assets/js/collaborative-editor/components/diagram/VersionMismatchBanner.tsx create mode 100644 assets/js/collaborative-editor/hooks/useVersionMismatch.ts create mode 100644 assets/test/collaborative-editor/hooks/useVersionMismatch.test.tsx diff --git a/CHANGELOG.md b/CHANGELOG.md index d5a2806ee0..b771b11b47 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -62,6 +62,8 @@ and this project adheres to ### Fixed +- 500 error when navigating from collaborative editor to full history page + [#3941](https://github.com/OpenFn/lightning/pull/3941) - Duplicate `isReadOnly` declaration in TriggerForm that was blocking asset builds [#3976](https://github.com/OpenFn/lightning/issues/3976) - Run duration and status alignment drift in history view diff --git a/assets/js/collaborative-editor/components/diagram/CollaborativeWorkflowDiagram.tsx b/assets/js/collaborative-editor/components/diagram/CollaborativeWorkflowDiagram.tsx index 7fc8bc94b5..ced33d7cfe 100644 --- a/assets/js/collaborative-editor/components/diagram/CollaborativeWorkflowDiagram.tsx +++ b/assets/js/collaborative-editor/components/diagram/CollaborativeWorkflowDiagram.tsx @@ -18,10 +18,13 @@ import { useRunSteps, } from '../../hooks/useHistory'; import { useIsNewWorkflow } from '../../hooks/useSessionContext'; +import { useVersionMismatch } from '../../hooks/useVersionMismatch'; +import { useVersionSelect } from '../../hooks/useVersionSelect'; import { useNodeSelection } from '../../hooks/useWorkflow'; import type { Run } from '../../types/history'; import MiniHistory from './MiniHistory'; +import { VersionMismatchBanner } from './VersionMismatchBanner'; import CollaborativeWorkflowDiagramImpl from './WorkflowDiagram'; interface CollaborativeWorkflowDiagramProps { @@ -72,14 +75,31 @@ export function CollaborativeWorkflowDiagram({ // Use hook to get run steps with automatic subscription management const currentRunSteps = useRunSteps(selectedRunId); - // Update URL when run selection changes - const handleRunSelect = useCallback((run: Run) => { - setSelectedRunId(run.id); + // Detect version mismatch for warning banner + const versionMismatch = useVersionMismatch(selectedRunId); - const url = new URL(window.location.href); - url.searchParams.set('run', run.id); - window.history.pushState({}, '', url.toString()); - }, []); + // Get version selection handler + const handleVersionSelect = useVersionSelect(); + + // Update URL when run selection changes + const handleRunSelect = useCallback( + (run: Run) => { + setSelectedRunId(run.id); + + // Find the workorder that contains this run + const workorder = history.find(wo => wo.runs.some(r => r.id === run.id)); + + // Switch to the version this run was executed on + if (workorder) { + handleVersionSelect(workorder.version); + } + + const url = new URL(window.location.href); + url.searchParams.set('run', run.id); + window.history.pushState({}, '', url.toString()); + }, + [history, handleVersionSelect] + ); // Clear URL parameter when deselecting run const handleDeselectRun = useCallback(() => { @@ -133,6 +153,15 @@ export function CollaborativeWorkflowDiagram({ return (
+ {/* Version mismatch warning when viewing latest but run used older version */} + {versionMismatch && ( + + )} + +
+ ); +} diff --git a/assets/js/collaborative-editor/hooks/useVersionMismatch.ts b/assets/js/collaborative-editor/hooks/useVersionMismatch.ts new file mode 100644 index 0000000000..ea379a0fa5 --- /dev/null +++ b/assets/js/collaborative-editor/hooks/useVersionMismatch.ts @@ -0,0 +1,64 @@ +/** + * useVersionMismatch - Detects when viewing latest workflow but selected run used older version + * + * Returns version mismatch info when: + * - A run is selected + * - Viewing "latest" workflow (not a specific snapshot) + * - The run was executed on a different version than currently displayed + * + * This prevents confusion when the workflow structure has changed since the run executed. + */ + +import { useMemo } from 'react'; + +import { useHistory } from './useHistory'; +import { useLatestSnapshotLockVersion } from './useSessionContext'; +import { useWorkflowState } from './useWorkflow'; + +interface VersionMismatch { + runVersion: number; + currentVersion: number; +} + +export function useVersionMismatch( + selectedRunId: string | null +): VersionMismatch | null { + const history = useHistory(); + const workflow = useWorkflowState(state => state.workflow); + const latestSnapshotLockVersion = useLatestSnapshotLockVersion(); + + return useMemo(() => { + if ( + !selectedRunId || + !workflow || + !workflow.lock_version || + !latestSnapshotLockVersion + ) { + return null; + } + + const workflowLockVersion = workflow.lock_version; + + // Find the work order that contains the selected run + const selectedWorkOrder = history.find(wo => + wo.runs.some(run => run.id === selectedRunId) + ); + + if (!selectedWorkOrder) { + return null; + } + + // Show warning when viewing a different version than the run used + const runUsedDifferentVersion = + selectedWorkOrder.version !== workflowLockVersion; + + if (runUsedDifferentVersion) { + return { + runVersion: selectedWorkOrder.version, + currentVersion: workflowLockVersion, + }; + } + + return null; + }, [selectedRunId, workflow, latestSnapshotLockVersion, history]); +} diff --git a/assets/js/workflow-diagram/util/ensure-node-position.ts b/assets/js/workflow-diagram/util/ensure-node-position.ts index 3255ce8fed..12513bdc74 100644 --- a/assets/js/workflow-diagram/util/ensure-node-position.ts +++ b/assets/js/workflow-diagram/util/ensure-node-position.ts @@ -1,4 +1,4 @@ -import type { Flow, Positions } from "./types"; +import type { Flow, Positions } from './types'; export const ensureNodePosition = ( model: Flow.Model, @@ -40,7 +40,15 @@ export const ensureNodePosition = ( }; return true; } else { - console.warn("WARNING: could not auto-calculate position for ", node.id); + // Only warn if positions map has data but we still couldn't find a parent + // Empty positions on initial render is expected before layout runs + const hasPositions = Object.keys(positions).length > 0; + if (hasPositions) { + console.warn( + 'WARNING: could not auto-calculate position for ', + node.id + ); + } node.position = { x: 0, y: 0 }; } } diff --git a/assets/test/collaborative-editor/hooks/useVersionMismatch.test.tsx b/assets/test/collaborative-editor/hooks/useVersionMismatch.test.tsx new file mode 100644 index 0000000000..6bb1aecafa --- /dev/null +++ b/assets/test/collaborative-editor/hooks/useVersionMismatch.test.tsx @@ -0,0 +1,211 @@ +/** + * useVersionMismatch Hook Tests + * + * Tests the logic for detecting when a user is viewing the latest workflow + * but a selected run was executed on an older version. + */ + +import { renderHook } from '@testing-library/react'; +import { describe, expect, test, vi } from 'vitest'; + +import { useVersionMismatch } from '../../../js/collaborative-editor/hooks/useVersionMismatch'; + +// Mock the dependency hooks +vi.mock('../../../js/collaborative-editor/hooks/useHistory', () => ({ + useHistory: vi.fn(), +})); + +vi.mock('../../../js/collaborative-editor/hooks/useSessionContext', () => ({ + useLatestSnapshotLockVersion: vi.fn(), +})); + +vi.mock('../../../js/collaborative-editor/hooks/useWorkflow', () => ({ + useWorkflowState: vi.fn(), +})); + +import { useHistory } from '../../../js/collaborative-editor/hooks/useHistory'; +import { useLatestSnapshotLockVersion } from '../../../js/collaborative-editor/hooks/useSessionContext'; +import { useWorkflowState } from '../../../js/collaborative-editor/hooks/useWorkflow'; + +describe('useVersionMismatch', () => { + test('returns mismatch when viewing latest and run used older version', () => { + // Arrange + const mockWorkflow = { id: 'wf-1', lock_version: 28 }; + const mockHistory = [ + { + id: 'wo-1', + version: 22, // Run executed on v22 + runs: [{ id: 'run-1', state: 'success' }], + }, + ]; + + vi.mocked(useWorkflowState).mockReturnValue(mockWorkflow); + vi.mocked(useLatestSnapshotLockVersion).mockReturnValue(28); + vi.mocked(useHistory).mockReturnValue(mockHistory); + + // Act + const { result } = renderHook(() => useVersionMismatch('run-1')); + + // Assert + expect(result.current).toEqual({ + runVersion: 22, + currentVersion: 28, + }); + }); + + test('returns null when viewing snapshot that matches run version', () => { + // Arrange: Viewing v22 snapshot, run also v22 + const mockWorkflow = { id: 'wf-1', lock_version: 22 }; + const mockHistory = [ + { + id: 'wo-1', + version: 22, + runs: [{ id: 'run-1', state: 'success' }], + }, + ]; + + vi.mocked(useWorkflowState).mockReturnValue(mockWorkflow); + vi.mocked(useLatestSnapshotLockVersion).mockReturnValue(28); // Latest is v28 + vi.mocked(useHistory).mockReturnValue(mockHistory); + + // Act + const { result } = renderHook(() => useVersionMismatch('run-1')); + + // Assert: No mismatch because viewing v22 snapshot (not latest) + expect(result.current).toBeNull(); + }); + + test('returns null when no run is selected', () => { + // Arrange + const mockWorkflow = { id: 'wf-1', lock_version: 28 }; + const mockHistory = [ + { + id: 'wo-1', + version: 22, + runs: [{ id: 'run-1', state: 'success' }], + }, + ]; + + vi.mocked(useWorkflowState).mockReturnValue(mockWorkflow); + vi.mocked(useLatestSnapshotLockVersion).mockReturnValue(28); + vi.mocked(useHistory).mockReturnValue(mockHistory); + + // Act: No run selected + const { result } = renderHook(() => useVersionMismatch(null)); + + // Assert + expect(result.current).toBeNull(); + }); + + test('returns null when workflow is null', () => { + // Arrange + const mockHistory = [ + { + id: 'wo-1', + version: 22, + runs: [{ id: 'run-1', state: 'success' }], + }, + ]; + + vi.mocked(useWorkflowState).mockReturnValue(null); + vi.mocked(useLatestSnapshotLockVersion).mockReturnValue(28); + vi.mocked(useHistory).mockReturnValue(mockHistory); + + // Act + const { result } = renderHook(() => useVersionMismatch('run-1')); + + // Assert + expect(result.current).toBeNull(); + }); + + test('returns null when latest snapshot version is null', () => { + // Arrange + const mockWorkflow = { id: 'wf-1', lock_version: 28 }; + const mockHistory = [ + { + id: 'wo-1', + version: 22, + runs: [{ id: 'run-1', state: 'success' }], + }, + ]; + + vi.mocked(useWorkflowState).mockReturnValue(mockWorkflow); + vi.mocked(useLatestSnapshotLockVersion).mockReturnValue(null); + vi.mocked(useHistory).mockReturnValue(mockHistory); + + // Act + const { result } = renderHook(() => useVersionMismatch('run-1')); + + // Assert + expect(result.current).toBeNull(); + }); + + test('returns null when selected run is not found in history', () => { + // Arrange + const mockWorkflow = { id: 'wf-1', lock_version: 28 }; + const mockHistory = [ + { + id: 'wo-1', + version: 22, + runs: [{ id: 'run-1', state: 'success' }], + }, + ]; + + vi.mocked(useWorkflowState).mockReturnValue(mockWorkflow); + vi.mocked(useLatestSnapshotLockVersion).mockReturnValue(28); + vi.mocked(useHistory).mockReturnValue(mockHistory); + + // Act: Looking for non-existent run + const { result } = renderHook(() => useVersionMismatch('run-999')); + + // Assert + expect(result.current).toBeNull(); + }); + + test('returns mismatch when versions differ even by small amount', () => { + // Arrange: v29 vs v30 (still a mismatch) + const mockWorkflow = { id: 'wf-1', lock_version: 30 }; + const mockHistory = [ + { + id: 'wo-1', + version: 29, + runs: [{ id: 'run-1', state: 'success' }], + }, + ]; + + vi.mocked(useWorkflowState).mockReturnValue(mockWorkflow); + vi.mocked(useLatestSnapshotLockVersion).mockReturnValue(30); + vi.mocked(useHistory).mockReturnValue(mockHistory); + + // Act + const { result } = renderHook(() => useVersionMismatch('run-1')); + + // Assert + expect(result.current).toEqual({ + runVersion: 29, + currentVersion: 30, + }); + }); + + test('returns null when viewing latest and run also used latest version', () => { + // Arrange: Both at v28 + const mockWorkflow = { id: 'wf-1', lock_version: 28 }; + const mockHistory = [ + { + id: 'wo-1', + version: 28, // Same version + runs: [{ id: 'run-1', state: 'success' }], + }, + ]; + + vi.mocked(useWorkflowState).mockReturnValue(mockWorkflow); + vi.mocked(useLatestSnapshotLockVersion).mockReturnValue(28); + vi.mocked(useHistory).mockReturnValue(mockHistory); + + // Act + const { result } = renderHook(() => useVersionMismatch('run-1')); + + // Assert: No mismatch when versions match + expect(result.current).toBeNull(); + }); +}); diff --git a/lib/lightning_web/live/run_live/components.ex b/lib/lightning_web/live/run_live/components.ex index 99484a90b3..ea9e8334fe 100644 --- a/lib/lightning_web/live/run_live/components.ex +++ b/lib/lightning_web/live/run_live/components.ex @@ -757,10 +757,13 @@ defmodule LightningWeb.RunLive.Components do defp humanize_workflow(filter, workflows) do case filter do %{workflow_id: workflow_id} -> - {workflow, _id} = - Enum.find(workflows, fn {_name, id} -> id == workflow_id end) + case Enum.find(workflows, fn {_name, id} -> id == workflow_id end) do + {workflow, _id} -> + "for #{workflow} workflow" - "for #{workflow} workflow" + nil -> + "" + end _other -> "" diff --git a/test/lightning_web/live/run_live/components_test.exs b/test/lightning_web/live/run_live/components_test.exs index b5a41b686c..f3dcd4c16b 100644 --- a/test/lightning_web/live/run_live/components_test.exs +++ b/test/lightning_web/live/run_live/components_test.exs @@ -621,4 +621,32 @@ defmodule LightningWeb.RunLive.ComponentsTest do ) |> Enum.any?() end + + describe "bulk_rerun_modal/1" do + test "handles missing workflow in humanize_search_params" do + workflow = insert(:simple_workflow) + workflows = [{"Test Workflow", workflow.id}] + + filters = %Lightning.WorkOrders.SearchParams{ + workflow_id: Ecto.UUID.generate() + } + + html = + render_component(&Components.bulk_rerun_modal/1, + id: "bulk-rerun-modal", + show: true, + all_selected?: true, + selected_count: 5, + page_number: 1, + pages: 2, + total_entries: 10, + filters: filters, + workflows: workflows + ) + + assert html =~ "Run Selected Work Orders" + assert html =~ "You've selected all 5 work orders" + refute html =~ "for #{workflow.name} workflow" + end + end end From a06f02d28357cc19582284f8a2a3315b74cc67a5 Mon Sep 17 00:00:00 2001 From: Stuart Corbishley Date: Thu, 13 Nov 2025 16:48:37 +0200 Subject: [PATCH 2/2] Refactor MiniHistory navigation to use state-based utilities Replace brittle URL parsing with clean navigation utilities that get IDs from state instead of parsing window.location. Uses URLSearchParams for proper query string encoding and provides reusable functions for navigating to history and run pages. - Extract navigation logic to dedicated utility module - Get project/workflow IDs from hooks instead of URL parsing - Add comprehensive test coverage for navigation utilities - Update MiniHistory tests to mock new dependencies --- .../components/diagram/MiniHistory.tsx | 55 ++--- .../collaborative-editor/utils/navigation.ts | 79 +++++++ .../components/diagram/MiniHistory.test.tsx | 31 ++- .../utils/navigation.test.ts | 198 ++++++++++++++++++ 4 files changed, 329 insertions(+), 34 deletions(-) create mode 100644 assets/js/collaborative-editor/utils/navigation.ts create mode 100644 assets/test/collaborative-editor/utils/navigation.test.ts diff --git a/assets/js/collaborative-editor/components/diagram/MiniHistory.tsx b/assets/js/collaborative-editor/components/diagram/MiniHistory.tsx index ee4620376a..9dd81af1eb 100644 --- a/assets/js/collaborative-editor/components/diagram/MiniHistory.tsx +++ b/assets/js/collaborative-editor/components/diagram/MiniHistory.tsx @@ -29,7 +29,14 @@ import React, { useState } from 'react'; import { relativeLocale } from '../../../hooks'; import { duration } from '../../../utils/duration'; import truncateUid from '../../../utils/truncateUID'; +import { useProject } from '../../hooks/useSessionContext'; +import { useWorkflowState } from '../../hooks/useWorkflow'; import type { Run, WorkOrder } from '../../types/history'; +import { + navigateToRun, + navigateToWorkOrderHistory, + navigateToWorkflowHistory, +} from '../../utils/navigation'; // Extended types with selection state for UI type RunWithSelection = Run & { selected?: boolean }; @@ -102,6 +109,10 @@ export default function MiniHistory({ const [expandedWorder, setExpandedWorder] = useState(''); const now = new Date(); + // Get project and workflow IDs from state for navigation + const project = useProject(); + const workflow = useWorkflowState(state => state.workflow); + // Clear expanded work order when panel collapses React.useEffect(() => { if (collapsed) { @@ -131,50 +142,30 @@ export default function MiniHistory({ const gotoHistory = (e: React.MouseEvent | React.KeyboardEvent) => { e.preventDefault(); e.stopPropagation(); - const currentUrl = new URL(window.location.href); - const nextUrl = new URL(currentUrl); - const paths = nextUrl.pathname.split('/'); - const wIdx = paths.indexOf('w'); - const workflowPaths = paths.splice(wIdx, paths.length - wIdx); - nextUrl.pathname = paths.join('/') + `/history`; - // Extract workflow ID: /w/:id or /w/:id/collaborate - // workflowPaths = ["w", workflow_id] or ["w", workflow_id, "collaborate"] - const workflowId = workflowPaths[1]; - nextUrl.search = `?filters[workflow_id]=${workflowId}`; - window.location.assign(nextUrl.toString()); + + if (project?.id && workflow?.id) { + navigateToWorkflowHistory(project.id, workflow.id); + } }; - const navigateToWorkorderHistory = ( + const handleNavigateToWorkorderHistory = ( e: React.MouseEvent, workorderId: string ) => { e.preventDefault(); e.stopPropagation(); - const currentUrl = new URL(window.location.href); - const nextUrl = new URL(currentUrl); - const paths = nextUrl.pathname.split('/'); - const projectIndex = paths.indexOf('projects'); - const projectId = projectIndex !== -1 ? paths[projectIndex + 1] : null; - if (projectId) { - nextUrl.pathname = `/projects/${projectId}/history`; - nextUrl.search = `?filters[workorder_id]=${workorderId}`; - window.location.assign(nextUrl.toString()); + if (project?.id) { + navigateToWorkOrderHistory(project.id, workorderId); } }; - const navigateToRunView = (e: React.MouseEvent, runId: string) => { + const handleNavigateToRunView = (e: React.MouseEvent, runId: string) => { e.preventDefault(); e.stopPropagation(); - const currentUrl = new URL(window.location.href); - const nextUrl = new URL(currentUrl); - const paths = nextUrl.pathname.split('/'); - const projectIndex = paths.indexOf('projects'); - const projectId = projectIndex !== -1 ? paths[projectIndex + 1] : null; - if (projectId) { - nextUrl.pathname = `/projects/${projectId}/runs/${runId}`; - window.location.assign(nextUrl.toString()); + if (project?.id) { + navigateToRun(project.id, runId); } }; @@ -343,7 +334,7 @@ export default function MiniHistory({