From 6be33b34561aebcd2b5740c774d9d9c0058eeda0 Mon Sep 17 00:00:00 2001 From: Frank Midigo Date: Wed, 10 Dec 2025 09:50:14 +0300 Subject: [PATCH 1/6] Improve version pinning behavior in collaborative editor Changes the version dropdown and read-only logic to use URL parameter presence instead of comparing lock versions. When viewing a run from history, only pins the version if it differs from latest, preventing unnecessary read-only mode when viewing latest version runs. The version dropdown now shows both "latest" and version numbers as separate options, making it clearer when editing the latest version versus viewing a pinned snapshot. --- CHANGELOG.md | 3 + .../components/VersionDropdown.tsx | 81 +++++++++++++------ .../diagram/CollaborativeWorkflowDiagram.tsx | 18 ++++- .../collaborative-editor/hooks/useWorkflow.ts | 48 +++++------ .../components/Header.keyboard.test.tsx | 34 ++++++-- .../components/ReadOnlyWarning.test.tsx | 31 +++++-- .../components/VersionDropdown.test.tsx | 16 ++-- .../hooks/useWorkflowReadOnly.test.tsx | 77 +++++++++++------- .../live/run_live/workorder_component.ex | 8 +- 9 files changed, 214 insertions(+), 102 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index fce59acdec..1128a2404b 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -19,6 +19,9 @@ and this project adheres to ### Changed +- Improve version pinning behavior in collaborative editor + [#4121](https://github.com/OpenFn/lightning/issues/4121) + ### Fixed ## [2.15.0-pre4] - 2025-12-08 diff --git a/assets/js/collaborative-editor/components/VersionDropdown.tsx b/assets/js/collaborative-editor/components/VersionDropdown.tsx index 68d8f63a1b..f7b1101153 100644 --- a/assets/js/collaborative-editor/components/VersionDropdown.tsx +++ b/assets/js/collaborative-editor/components/VersionDropdown.tsx @@ -1,5 +1,7 @@ import { useEffect, useRef, useState } from 'react'; +import { useURLState } from '#/react/lib/use-url-state'; + import { cn } from '../../utils/cn'; import { useRequestVersions, @@ -30,11 +32,16 @@ export function VersionDropdown({ const versionsError = useVersionsError(); const requestVersions = useRequestVersions(); + // Check if version is pinned via URL parameter + const { params } = useURLState(); + const isPinnedVersion = params['v'] !== undefined && params['v'] !== null; + // Show placeholder while loading version information const isLoadingVersion = currentVersion === null || latestVersion === null; - // Determine if viewing latest version (only when we have both values) - const isLatestVersion = !isLoadingVersion && currentVersion === latestVersion; + // Determine if viewing latest version (only when we have both values AND no pinned version) + const isLatestVersion = + !isLoadingVersion && currentVersion === latestVersion && !isPinnedVersion; // Format version display const currentVersionDisplay = isLoadingVersion @@ -95,8 +102,8 @@ export function VersionDropdown({ } }, [versionsError]); - const handleVersionClick = (version: Version) => { - if (version.is_latest) { + const handleVersionClick = (version: Version | 'latest') => { + if (version === 'latest') { onVersionSelect('latest'); } else { onVersionSelect(version.lock_version); @@ -146,44 +153,66 @@ export function VersionDropdown({ No versions available ) : ( - versions.map((version, index) => { - const isSelected = version.is_latest - ? isLatestVersion - : version.lock_version === currentVersion; - - // Show "latest" for the latest version, otherwise show version number - // For the first item (which is latest), show "latest" - // For subsequent items, show version number even if they have is_latest=true - const displayText = - index === 0 && version.is_latest - ? 'latest' - : `v${String(version.lock_version).substring(0, 7)}`; - - return ( + <> + {/* First, show "latest" option that removes version parameter */} + {versions.length > 0 && versions[0].is_latest && ( - ); - }) + )} + + {/* Then show all versions with version numbers (including latest) */} + {versions.map(version => { + const isSelected = + !isLatestVersion && version.lock_version === currentVersion; + + const displayText = `v${String(version.lock_version).substring(0, 7)}`; + + return ( + + ); + })} + )} diff --git a/assets/js/collaborative-editor/components/diagram/CollaborativeWorkflowDiagram.tsx b/assets/js/collaborative-editor/components/diagram/CollaborativeWorkflowDiagram.tsx index 063f416099..143d4df134 100644 --- a/assets/js/collaborative-editor/components/diagram/CollaborativeWorkflowDiagram.tsx +++ b/assets/js/collaborative-editor/components/diagram/CollaborativeWorkflowDiagram.tsx @@ -19,7 +19,10 @@ import { useHistoryLoading, useRunSteps, } from '../../hooks/useHistory'; -import { useIsNewWorkflow } from '../../hooks/useSessionContext'; +import { + useIsNewWorkflow, + useLatestSnapshotLockVersion, +} from '../../hooks/useSessionContext'; import { useVersionMismatch } from '../../hooks/useVersionMismatch'; import { useNodeSelection } from '../../hooks/useWorkflow'; import { useKeyboardShortcut } from '../../keyboard'; @@ -42,6 +45,7 @@ export function CollaborativeWorkflowDiagram({ const isNewWorkflow = useIsNewWorkflow(); const isHistoryChannelConnected = useHistoryChannelConnected(); const { params, updateSearchParams } = useURLState(); + const latestSnapshotLockVersion = useLatestSnapshotLockVersion(); // Get history data and commands const history = useHistory(); @@ -85,14 +89,22 @@ export function CollaborativeWorkflowDiagram({ // Find the workorder that contains this run const workorder = history.find(wo => wo.runs.some(r => r.id === run.id)); + // Only include version parameter if the run's version differs from latest + // This prevents pinning to read-only mode when viewing latest version runs + const runVersion = workorder?.version; + const shouldPinVersion = + runVersion !== null && + runVersion !== undefined && + runVersion !== latestSnapshotLockVersion; + // Single atomic update - both version and run in one call // This prevents race conditions between two separate updateSearchParams calls updateSearchParams({ - v: workorder ? String(workorder.version) : null, + v: shouldPinVersion ? String(runVersion) : null, run: run.id, }); }, - [history, updateSearchParams] + [history, latestSnapshotLockVersion, updateSearchParams] ); // Clear URL parameter when deselecting run diff --git a/assets/js/collaborative-editor/hooks/useWorkflow.ts b/assets/js/collaborative-editor/hooks/useWorkflow.ts index 5efb77ddb4..7d0cba86b9 100644 --- a/assets/js/collaborative-editor/hooks/useWorkflow.ts +++ b/assets/js/collaborative-editor/hooks/useWorkflow.ts @@ -632,35 +632,30 @@ export const useWorkflowActions = () => { * - hasPermission: User has can_edit_workflow permission * - isConnected: Session is synced with backend * - isDeleted: Workflow has been deleted - * - isOldSnapshot: Viewing an old snapshot (not latest version) + * - isPinnedVersion: Viewing a pinned version (any ?v parameter in URL) * * @internal This is shared logic between useCanSave and useCanRun */ const useWorkflowConditions = () => { const { isSynced } = useSession(); const permissions = usePermissions(); - const latestSnapshotLockVersion = useLatestSnapshotLockVersion(); const workflow = useWorkflowState(state => state.workflow); + const { params } = useURLState(); const hasEditPermission = permissions?.can_edit_workflow ?? false; const hasRunPermission = permissions?.can_run_workflow ?? false; const isConnected = isSynced; const isDeleted = workflow !== null && workflow.deleted_at !== null; - // Only consider it an old snapshot if workflow is loaded, latest - // snapshot lock version is available AND different from workflow - // lock version - const isOldSnapshot = - workflow !== null && - latestSnapshotLockVersion !== null && - workflow.lock_version !== latestSnapshotLockVersion; + // Check if version is pinned via URL parameter + const isPinnedVersion = params['v'] !== undefined && params['v'] !== null; return { hasEditPermission, hasRunPermission, isConnected, isDeleted, - isOldSnapshot, + isPinnedVersion, }; }; @@ -674,11 +669,11 @@ const useWorkflowConditions = () => { * Checks: * 1. User permissions (can_edit_workflow) * 2. Connection state (isSynced) - * 3. Lock version (viewing latest snapshot) + * 3. Version pinning (any ?v parameter in URL) * 4. Workflow deletion state (deleted_at) */ export const useCanSave = (): { canSave: boolean; tooltipMessage: string } => { - const { hasEditPermission, isConnected, isDeleted, isOldSnapshot } = + const { hasEditPermission, isConnected, isDeleted, isPinnedVersion } = useWorkflowConditions(); // Determine tooltip message (check in priority order) @@ -694,9 +689,9 @@ export const useCanSave = (): { canSave: boolean; tooltipMessage: string } => { } else if (isDeleted) { canSave = false; tooltipMessage = 'Workflow has been deleted'; - } else if (isOldSnapshot) { + } else if (isPinnedVersion) { canSave = false; - tooltipMessage = 'You cannot edit an old snapshot of a workflow'; + tooltipMessage = 'You are viewing a pinned version of this workflow'; } return { canSave, tooltipMessage }; @@ -712,7 +707,7 @@ export const useCanSave = (): { canSave: boolean; tooltipMessage: string } => { * Checks: * 1. User permissions (can_edit_workflow or can_run_workflow) * 2. Connection state (isSynced) - * 3. Lock version (viewing latest snapshot) + * 3. Version pinning (any ?v parameter in URL) * 4. Workflow deletion state (deleted_at) * 5. Run limits (from session context) */ @@ -722,7 +717,7 @@ export const useCanRun = (): { canRun: boolean; tooltipMessage: string } => { hasRunPermission, isConnected, isDeleted, - isOldSnapshot, + isPinnedVersion, } = useWorkflowConditions(); // Get run limits from session context (defaults to allowed if missing) @@ -745,9 +740,9 @@ export const useCanRun = (): { canRun: boolean; tooltipMessage: string } => { } else if (isDeleted) { canRun = false; tooltipMessage = 'Workflow has been deleted'; - } else if (isOldSnapshot) { + } else if (isPinnedVersion) { canRun = false; - tooltipMessage = 'You cannot run an old snapshot of a workflow'; + tooltipMessage = 'You are viewing a pinned version of this workflow'; } else if (!runLimits.allowed && runLimits.message) { canRun = false; tooltipMessage = runLimits.message; @@ -766,7 +761,7 @@ export const useCanRun = (): { canRun: boolean; tooltipMessage: string } => { * Checks (in priority order): * 1. Workflow deletion state (deleted_at) * 2. User permissions (can_edit_workflow) - * 3. Snapshot version (viewing old snapshot) + * 3. Version pinning (any ?v parameter in URL) * * Note: Connection state does not affect read-only status. Offline editing * is fully supported - Y.Doc buffers transactions locally and syncs when @@ -778,8 +773,11 @@ export const useWorkflowReadOnly = (): { } => { // Get permissions and workflow state const permissions = usePermissions(); - const latestSnapshotLockVersion = useLatestSnapshotLockVersion(); const workflow = useWorkflowState(state => state.workflow); + const { params } = useURLState(); + + // Check if version is pinned via URL parameter + const isPinnedVersion = params['v'] !== undefined && params['v'] !== null; // Don't show read-only state until permissions are loaded // This prevents flickering during initial load @@ -790,12 +788,8 @@ export const useWorkflowReadOnly = (): { // Compute read-only conditions const hasPermission = permissions.can_edit_workflow; const isDeleted = workflow !== null && workflow.deleted_at !== null; - const isOldSnapshot = - workflow !== null && - latestSnapshotLockVersion !== null && - workflow.lock_version !== latestSnapshotLockVersion; - // Priority order: deleted > permissions > snapshot + // Priority order: deleted > permissions > pinned version if (isDeleted) { return { isReadOnly: true, @@ -808,10 +802,10 @@ export const useWorkflowReadOnly = (): { tooltipMessage: 'You do not have permission to edit this workflow', }; } - if (isOldSnapshot) { + if (isPinnedVersion) { return { isReadOnly: true, - tooltipMessage: 'You cannot edit or run an old snapshot of a workflow', + tooltipMessage: 'You are viewing a pinned version of this workflow', }; } diff --git a/assets/test/collaborative-editor/components/Header.keyboard.test.tsx b/assets/test/collaborative-editor/components/Header.keyboard.test.tsx index 15a952ca86..3093c5286a 100644 --- a/assets/test/collaborative-editor/components/Header.keyboard.test.tsx +++ b/assets/test/collaborative-editor/components/Header.keyboard.test.tsx @@ -23,12 +23,23 @@ import { StoreContext } from '../../../js/collaborative-editor/contexts/StorePro import { KeyboardProvider } from '../../../js/collaborative-editor/keyboard'; import type { CreateSessionContextOptions } from '../__helpers__/sessionContextFactory'; import { simulateStoreProviderWithConnection } from '../__helpers__/storeProviderHelpers'; +import { + createMockURLState, + getURLStateMockValue, +} from '../__helpers__/urlStateMocks'; import { createMinimalWorkflowYDoc } from '../__helpers__/workflowStoreHelpers'; // ============================================================================= // TEST MOCKS // ============================================================================= +// Mock useURLState +const urlState = createMockURLState(); + +vi.mock('../../../js/react/lib/use-url-state', () => ({ + useURLState: () => getURLStateMockValue(urlState), +})); + // Mock useAdaptorIcons to prevent async fetch warnings vi.mock('../../../js/workflow-diagram/useAdaptorIcons', () => ({ default: () => ({}), @@ -177,6 +188,9 @@ async function renderAndWaitForReady( // ============================================================================= describe('Header - Save Workflow (Cmd+S / Ctrl+S)', () => { + beforeEach(() => { + urlState.reset(); + }); beforeEach(() => { vi.clearAllMocks(); }); @@ -328,13 +342,15 @@ describe('Header - Save Workflow (Cmd+S / Ctrl+S)', () => { cleanup(); }); - test('Cmd+S does NOT call saveWorkflow when viewing old snapshot', async () => { + test('Cmd+S does NOT call saveWorkflow when viewing pinned version', async () => { const user = userEvent.setup(); + + // Set pinned version in URL + urlState.setParam('v', '1'); + const { wrapper, emitSessionContext, saveWorkflowSpy, cleanup } = await createTestSetup({ permissions: { can_edit_workflow: true, can_run_workflow: true }, - workflowLockVersion: 1, - latestSnapshotLockVersion: 2, }); const { unmount } = await renderAndWaitForReady( @@ -515,6 +531,10 @@ describe('Header - Save Workflow (Cmd+S / Ctrl+S)', () => { // ============================================================================= describe('Header - Save & Sync to GitHub (Cmd+Shift+S / Ctrl+Shift+S)', () => { + beforeEach(() => { + urlState.reset(); + }); + beforeEach(() => { vi.clearAllMocks(); }); @@ -645,14 +665,16 @@ describe('Header - Save & Sync to GitHub (Cmd+Shift+S / Ctrl+Shift+S)', () => { cleanup(); }); - test('Cmd+Shift+S does NOT open modal when viewing old snapshot', async () => { + test('Cmd+Shift+S does NOT open modal when viewing pinned version', async () => { const user = userEvent.setup(); + + // Set pinned version in URL + urlState.setParam('v', '1'); + const { wrapper, emitSessionContext, openGitHubSyncModalSpy, cleanup } = await createTestSetup({ permissions: { can_edit_workflow: true, can_run_workflow: true }, hasGithubConnection: true, - workflowLockVersion: 1, - latestSnapshotLockVersion: 2, }); const { unmount } = await renderAndWaitForReady( diff --git a/assets/test/collaborative-editor/components/ReadOnlyWarning.test.tsx b/assets/test/collaborative-editor/components/ReadOnlyWarning.test.tsx index 06ebba4e83..eac3c0092b 100644 --- a/assets/test/collaborative-editor/components/ReadOnlyWarning.test.tsx +++ b/assets/test/collaborative-editor/components/ReadOnlyWarning.test.tsx @@ -11,7 +11,7 @@ import { act, render, screen, waitFor } from '@testing-library/react'; import type React from 'react'; -import { describe, expect, test } from 'vitest'; +import { beforeEach, describe, expect, test, vi } from 'vitest'; import * as Y from 'yjs'; import { ReadOnlyWarning } from '../../../js/collaborative-editor/components/ReadOnlyWarning'; @@ -23,12 +23,23 @@ import { createSessionStore } from '../../../js/collaborative-editor/stores/crea import { createWorkflowStore } from '../../../js/collaborative-editor/stores/createWorkflowStore'; import type { Session } from '../../../js/collaborative-editor/types/session'; import { createSessionContext } from '../__helpers__/sessionContextFactory'; +import { + createMockURLState, + getURLStateMockValue, +} from '../__helpers__/urlStateMocks'; import { createMockPhoenixChannel, createMockPhoenixChannelProvider, } from '../mocks/phoenixChannel'; import { createMockSocket } from '../mocks/phoenixSocket'; +// Mock useURLState +const urlState = createMockURLState(); + +vi.mock('../../../js/react/lib/use-url-state', () => ({ + useURLState: () => getURLStateMockValue(urlState), +})); + // ============================================================================= // TEST HELPERS // ============================================================================= @@ -298,6 +309,10 @@ describe('ReadOnlyWarning - Styling', () => { // ============================================================================= describe('ReadOnlyWarning - Read-Only States', () => { + beforeEach(() => { + urlState.reset(); + }); + test('renders for deleted workflow', async () => { const { wrapper, emitSessionContext } = createTestSetup({ workflowDeletedAt: new Date().toISOString(), @@ -330,11 +345,11 @@ describe('ReadOnlyWarning - Read-Only States', () => { }); }); - test('renders for old snapshots', async () => { - const { wrapper, emitSessionContext } = createTestSetup({ - latestSnapshotLockVersion: 2, - workflowLockVersion: 1, - }); + test('renders for pinned versions', async () => { + // Set pinned version in URL + urlState.setParam('v', '1'); + + const { wrapper, emitSessionContext } = createTestSetup({}); render(, { wrapper }); @@ -353,6 +368,10 @@ describe('ReadOnlyWarning - Read-Only States', () => { // ============================================================================= describe('ReadOnlyWarning - Dynamic Changes', () => { + beforeEach(() => { + urlState.reset(); + }); + test('appears when workflow becomes deleted', async () => { const { wrapper, emitSessionContext, ydoc } = createTestSetup({ permissions: { can_edit_workflow: true, can_run_workflow: true }, diff --git a/assets/test/collaborative-editor/components/VersionDropdown.test.tsx b/assets/test/collaborative-editor/components/VersionDropdown.test.tsx index 4dd65859fa..0fc85a542d 100644 --- a/assets/test/collaborative-editor/components/VersionDropdown.test.tsx +++ b/assets/test/collaborative-editor/components/VersionDropdown.test.tsx @@ -455,9 +455,10 @@ describe('VersionDropdown', () => { // Open dropdown await user.click(button); - // Should show formatted timestamp + // Should show formatted timestamp (appears twice: once for "latest" and once for "v2") const timestamp = new Date('2024-01-15T10:30:00Z').toLocaleString(); - expect(screen.getByText(timestamp)).toBeInTheDocument(); + const timestampElements = screen.getAllByText(timestamp); + expect(timestampElements.length).toBe(2); }); test('highlights currently selected version', async () => { @@ -540,8 +541,11 @@ describe('VersionDropdown', () => { const versionButtons = screen.getAllByRole('menuitem'); expect(versionButtons[0]).toHaveTextContent('latest'); - // Second item should show version number - expect(versionButtons[1]).toHaveTextContent('v4'); + // Second item should show version number (v5) + expect(versionButtons[1]).toHaveTextContent('v5'); + + // Third item should show version number (v4) + expect(versionButtons[2]).toHaveTextContent('v4'); }); }); @@ -827,9 +831,9 @@ describe('VersionDropdown', () => { // Open dropdown await user.click(button); - // All version items should have menuitem role + // All version items should have menuitem role (1 "latest" + 2 versions) const menuItems = screen.getAllByRole('menuitem'); - expect(menuItems).toHaveLength(2); + expect(menuItems).toHaveLength(3); }); }); }); diff --git a/assets/test/collaborative-editor/hooks/useWorkflowReadOnly.test.tsx b/assets/test/collaborative-editor/hooks/useWorkflowReadOnly.test.tsx index e0343c115a..0c95b68dd4 100644 --- a/assets/test/collaborative-editor/hooks/useWorkflowReadOnly.test.tsx +++ b/assets/test/collaborative-editor/hooks/useWorkflowReadOnly.test.tsx @@ -2,12 +2,12 @@ * useWorkflowReadOnly Hook Tests * * Tests for the read-only state hook that determines if a workflow - * should be editable based on deletion state, permissions, and snapshot version. + * should be editable based on deletion state, permissions, and version pinning. */ import { act, renderHook, waitFor } from '@testing-library/react'; import type React from 'react'; -import { describe, expect, test } from 'vitest'; +import { beforeEach, describe, expect, test, vi } from 'vitest'; import * as Y from 'yjs'; import { SessionContext } from '../../../js/collaborative-editor/contexts/SessionProvider'; @@ -24,12 +24,23 @@ import { createSessionContext, mockPermissions, } from '../__helpers__/sessionContextFactory'; +import { + createMockURLState, + getURLStateMockValue, +} from '../__helpers__/urlStateMocks'; import { createMockPhoenixChannel, createMockPhoenixChannelProvider, } from '../mocks/phoenixChannel'; import { createMockSocket } from '../mocks/phoenixSocket'; +// Mock useURLState +const urlState = createMockURLState(); + +vi.mock('../../../js/react/lib/use-url-state', () => ({ + useURLState: () => getURLStateMockValue(urlState), +})); + // ============================================================================= // TEST HELPERS // ============================================================================= @@ -144,6 +155,14 @@ function createWrapper(options: WrapperOptions = {}): [ ]; } +// ============================================================================= +// SETUP +// ============================================================================= + +beforeEach(() => { + urlState.reset(); +}); + // ============================================================================= // DELETED WORKFLOW TESTS // ============================================================================= @@ -189,11 +208,12 @@ describe('useWorkflowReadOnly - Deleted Workflow', () => { }); }); - test('deleted state takes priority over old snapshot', async () => { + test('deleted state takes priority over pinned version', async () => { + // Set pinned version in URL + urlState.setParam('v', '1'); + const [wrapper, { emitSessionContext }] = createWrapper({ permissions: { can_edit_workflow: true, can_run_workflow: true }, - latestSnapshotLockVersion: 2, - workflowLockVersion: 1, workflowDeletedAt: new Date().toISOString(), }); @@ -237,11 +257,12 @@ describe('useWorkflowReadOnly - Permissions', () => { }); }); - test('permission restriction takes priority over old snapshot', async () => { + test('permission restriction takes priority over pinned version', async () => { + // Set pinned version in URL + urlState.setParam('v', '1'); + const [wrapper, { emitSessionContext }] = createWrapper({ permissions: { can_edit_workflow: false, can_run_workflow: false }, - latestSnapshotLockVersion: 2, - workflowLockVersion: 1, workflowDeletedAt: null, }); @@ -261,15 +282,16 @@ describe('useWorkflowReadOnly - Permissions', () => { }); // ============================================================================= -// SNAPSHOT VERSION TESTS +// VERSION PINNING TESTS // ============================================================================= -describe('useWorkflowReadOnly - Snapshot Version', () => { - test('returns read-only true with snapshot message for old snapshot', async () => { +describe('useWorkflowReadOnly - Version Pinning', () => { + test('returns read-only true with pinned version message when ?v parameter is present', async () => { + // Set pinned version in URL + urlState.setParam('v', '1'); + const [wrapper, { emitSessionContext }] = createWrapper({ permissions: { can_edit_workflow: true, can_run_workflow: true }, - latestSnapshotLockVersion: 2, - workflowLockVersion: 1, workflowDeletedAt: null, }); @@ -282,16 +304,15 @@ describe('useWorkflowReadOnly - Snapshot Version', () => { await waitFor(() => { expect(result.current.isReadOnly).toBe(true); expect(result.current.tooltipMessage).toBe( - 'You cannot edit or run an old snapshot of a workflow' + 'You are viewing a pinned version of this workflow' ); }); }); - test('returns not read-only when viewing latest snapshot', async () => { + test('returns not read-only when no ?v parameter is present', async () => { + // No version parameter in URL (editing latest) const [wrapper, { emitSessionContext }] = createWrapper({ permissions: { can_edit_workflow: true, can_run_workflow: true }, - latestSnapshotLockVersion: 1, - workflowLockVersion: 1, workflowDeletedAt: null, }); @@ -485,12 +506,12 @@ describe('useWorkflowReadOnly - Edge Cases', () => { // ============================================================================= describe('useWorkflowReadOnly - Priority Order', () => { - test('verifies complete priority order: deleted > permissions > snapshot', async () => { + test('verifies complete priority order: deleted > permissions > pinned version', async () => { // Test 1: All three conditions apply - deleted takes priority + urlState.setParam('v', '1'); + const [wrapper1, { emitSessionContext: emit1 }] = createWrapper({ permissions: { can_edit_workflow: false, can_run_workflow: false }, - latestSnapshotLockVersion: 2, - workflowLockVersion: 1, workflowDeletedAt: new Date().toISOString(), }); @@ -508,11 +529,12 @@ describe('useWorkflowReadOnly - Priority Order', () => { ); }); - // Test 2: Permission and snapshot conditions apply - permission takes priority + // Test 2: Permission and pinned version conditions apply - permission takes priority + urlState.reset(); + urlState.setParam('v', '1'); + const [wrapper2, { emitSessionContext: emit2 }] = createWrapper({ permissions: { can_edit_workflow: false, can_run_workflow: false }, - latestSnapshotLockVersion: 2, - workflowLockVersion: 1, workflowDeletedAt: null, }); @@ -530,11 +552,12 @@ describe('useWorkflowReadOnly - Priority Order', () => { ); }); - // Test 3: Only snapshot condition applies + // Test 3: Only pinned version condition applies + urlState.reset(); + urlState.setParam('v', '1'); + const [wrapper3, { emitSessionContext: emit3 }] = createWrapper({ permissions: { can_edit_workflow: true, can_run_workflow: true }, - latestSnapshotLockVersion: 2, - workflowLockVersion: 1, workflowDeletedAt: null, }); @@ -548,7 +571,7 @@ describe('useWorkflowReadOnly - Priority Order', () => { await waitFor(() => { expect(result3.current.tooltipMessage).toBe( - 'You cannot edit or run an old snapshot of a workflow' + 'You are viewing a pinned version of this workflow' ); }); }); diff --git a/lib/lightning_web/live/run_live/workorder_component.ex b/lib/lightning_web/live/run_live/workorder_component.ex index dc41046485..ffe08a4102 100644 --- a/lib/lightning_web/live/run_live/workorder_component.ex +++ b/lib/lightning_web/live/run_live/workorder_component.ex @@ -180,7 +180,13 @@ defmodule LightningWeb.RunLive.WorkOrderComponent do <%= if @last_run do %> <.link navigate={ - ~p"/projects/#{@project}/w/#{@work_order.workflow.id}?a=#{@last_run.id}&v=#{@work_order.snapshot.lock_version}" + # Only include version param if snapshot differs from current workflow version + if @work_order.workflow.lock_version == + @work_order.snapshot.lock_version do + ~p"/projects/#{@project}/w/#{@work_order.workflow.id}?a=#{@last_run.id}" + else + ~p"/projects/#{@project}/w/#{@work_order.workflow.id}?a=#{@last_run.id}&v=#{@work_order.snapshot.lock_version}" + end } class="inline-block" > From 4635260bfbdfb6fde0aec97d0ef99de66cea2e3e Mon Sep 17 00:00:00 2001 From: Frank Midigo Date: Wed, 10 Dec 2025 15:27:23 +0300 Subject: [PATCH 2/6] Move version mismatch banner to MiniHistory panel Relocates the version mismatch banner from the top of the canvas to the bottom of the MiniHistory panel, improving UI organization and making the warning more contextual to the run being viewed. Changes the banner from dismissible to actionable - replaces the X button with a "Go to vN" button that navigates to the correct version. This makes it easier for users to resolve the mismatch rather than just hide it. The banner now supports two display modes: - Collapsed panel: constrained width (150px) wraps text to 3 lines - Expanded panel: full width keeps text and button on 1 line Updates tests to match new behavior and adds coverage for compact mode. --- .../diagram/CollaborativeWorkflowDiagram.tsx | 10 +- .../components/diagram/MiniHistory.tsx | 36 ++++++ .../diagram/VersionMismatchBanner.tsx | 49 ++++---- .../diagram/VersionMismatchBanner.test.tsx | 105 ++++++++++++------ 4 files changed, 131 insertions(+), 69 deletions(-) diff --git a/assets/js/collaborative-editor/components/diagram/CollaborativeWorkflowDiagram.tsx b/assets/js/collaborative-editor/components/diagram/CollaborativeWorkflowDiagram.tsx index 143d4df134..3b5d95f302 100644 --- a/assets/js/collaborative-editor/components/diagram/CollaborativeWorkflowDiagram.tsx +++ b/assets/js/collaborative-editor/components/diagram/CollaborativeWorkflowDiagram.tsx @@ -29,7 +29,6 @@ import { useKeyboardShortcut } from '../../keyboard'; import type { RunSummary } from '../../types/history'; import MiniHistory from './MiniHistory'; -import { VersionMismatchBanner } from './VersionMismatchBanner'; import CollaborativeWorkflowDiagramImpl from './WorkflowDiagram'; interface CollaborativeWorkflowDiagramProps { @@ -167,14 +166,6 @@ export function CollaborativeWorkflowDiagram({ return (
- {versionMismatch && ( - - )} - )} diff --git a/assets/js/collaborative-editor/components/diagram/MiniHistory.tsx b/assets/js/collaborative-editor/components/diagram/MiniHistory.tsx index 21792438b9..f47085a62b 100644 --- a/assets/js/collaborative-editor/components/diagram/MiniHistory.tsx +++ b/assets/js/collaborative-editor/components/diagram/MiniHistory.tsx @@ -30,6 +30,7 @@ import { relativeLocale } from '../../../hooks'; import { duration } from '../../../utils/duration'; import truncateUid from '../../../utils/truncateUID'; import { useProject } from '../../hooks/useSessionContext'; +import { useVersionSelect } from '../../hooks/useVersionSelect'; import { useWorkflowState } from '../../hooks/useWorkflow'; import type { RunSummary, WorkOrder } from '../../types/history'; import { @@ -41,6 +42,8 @@ import { RunBadge } from '../common/RunBadge'; import { ShortcutKeys } from '../ShortcutKeys'; import { Tooltip } from '../Tooltip'; +import { VersionMismatchBanner } from './VersionMismatchBanner'; + // Extended types with selection state for UI type RunWithSelection = RunSummary & { selected?: boolean }; type WorkOrderWithSelection = Omit & { @@ -284,6 +287,11 @@ interface MiniHistoryProps { // New props for panel variant variant?: 'floating' | 'panel'; onBack?: () => void; + // Version mismatch detection + versionMismatch?: { + runVersion: number; + currentVersion: number; + } | null; } export default function MiniHistory({ @@ -298,6 +306,7 @@ export default function MiniHistory({ onRetry, variant = 'floating', onBack, + versionMismatch, }: MiniHistoryProps) { const [expandedWorder, setExpandedWorder] = useState(''); const now = new Date(); @@ -305,6 +314,14 @@ export default function MiniHistory({ // Get project and workflow IDs from state for navigation const project = useProject(); const workflow = useWorkflowState(state => state.workflow); + const handleVersionSelect = useVersionSelect(); + + // Handler to navigate to the run's version + const handleGoToVersion = () => { + if (versionMismatch) { + handleVersionSelect(versionMismatch.runVersion); + } + }; // Clear expanded work order when panel collapses React.useEffect(() => { @@ -532,6 +549,16 @@ export default function MiniHistory({
)} + {/* Version mismatch banner when collapsed */} + {collapsed && versionMismatch && ( + + )} +
)}
+ + {/* Version mismatch banner at bottom of panel */} + {!collapsed && versionMismatch && ( + + )} ); } diff --git a/assets/js/collaborative-editor/components/diagram/VersionMismatchBanner.tsx b/assets/js/collaborative-editor/components/diagram/VersionMismatchBanner.tsx index 4eb89b1bde..c8a3abd6b2 100644 --- a/assets/js/collaborative-editor/components/diagram/VersionMismatchBanner.tsx +++ b/assets/js/collaborative-editor/components/diagram/VersionMismatchBanner.tsx @@ -9,59 +9,52 @@ * as the workflow structure may have changed between versions. * * Features: - * - Compact two-line design with version information - * - Dismissible via X button - * - Positioned at top-center of canvas + * - Compact design with version information + * - Action button to navigate to the correct version + * - Positioned at bottom of MiniHistory panel */ -import { useState } from 'react'; - import { cn } from '#/utils/cn'; interface VersionMismatchBannerProps { runVersion: number; currentVersion: number; + onGoToVersion: () => void; className?: string; + compact?: boolean; } export function VersionMismatchBanner({ runVersion, currentVersion, + onGoToVersion, className, + compact = false, }: VersionMismatchBannerProps) { - const [dismissed, setDismissed] = useState(false); - - if (dismissed) { - return null; - } - return (
-
+