diff --git a/assets/js/collaborative-editor/CollaborativeEditor.tsx b/assets/js/collaborative-editor/CollaborativeEditor.tsx index 6b4863071b..8878b00c5d 100644 --- a/assets/js/collaborative-editor/CollaborativeEditor.tsx +++ b/assets/js/collaborative-editor/CollaborativeEditor.tsx @@ -19,10 +19,14 @@ import { VersionDropdown } from './components/VersionDropdown'; import { WorkflowEditor } from './components/WorkflowEditor'; import { YAMLImportModal } from './components/YAMLImportModal'; import { CredentialModalProvider } from './contexts/CredentialModalContext'; -import { LiveViewActionsProvider } from './contexts/LiveViewActionsContext'; +import { + LiveViewActionsProvider, + useLiveViewActions, +} from './contexts/LiveViewActionsContext'; import { MonacoRefProvider } from './contexts/MonacoRefContext'; import { SessionProvider } from './contexts/SessionProvider'; import { StoreProvider } from './contexts/StoreProvider'; +import { useActionLock } from './hooks/useActionLock'; import { useIsNewWorkflow, useLatestSnapshotLockVersion, @@ -36,6 +40,7 @@ import { import { useVersionSelect } from './hooks/useVersionSelect'; import { useWorkflowState } from './hooks/useWorkflow'; import { KeyboardProvider } from './keyboard'; +import { notifications } from './lib/notifications'; export interface CollaborativeEditorDataProps { 'data-workflow-id': string; @@ -190,6 +195,27 @@ function LandingScreenWrapper({ dismissLandingScreen, openAIAssistantPanel, } = useUICommands(); + const { pushEventTo } = useLiveViewActions(); + const { run: runBuildFromScratch, isPending: isBuildingFromScratch } = + useActionLock(async () => { + try { + await new Promise((resolve, reject) => { + const timeout = setTimeout( + () => reject(new Error('build_from_scratch timed out')), + 10_000 + ); + pushEventTo('build_from_scratch', {}, () => { + clearTimeout(timeout); + resolve(); + }); + }); + } catch { + notifications.alert({ + title: 'Failed to create workflow', + description: 'Please check your connection and try again.', + }); + } + }); if (!showLandingScreen) return null; @@ -201,7 +227,8 @@ function LandingScreenWrapper({ dismissLandingScreen(); openAIAssistantPanel(prompt); }} - onBuildFromScratch={() => {}} + onBuildFromScratch={() => void runBuildFromScratch()} + isBuildingFromScratch={isBuildingFromScratch} onBrowseTemplates={openTemplateBrowserModal} onImportYAML={openYAMLImportModal} /> diff --git a/assets/js/collaborative-editor/components/LandingScreen.tsx b/assets/js/collaborative-editor/components/LandingScreen.tsx index 5bf2a5ce61..76e26c5b0d 100644 --- a/assets/js/collaborative-editor/components/LandingScreen.tsx +++ b/assets/js/collaborative-editor/components/LandingScreen.tsx @@ -8,12 +8,14 @@ interface WorkflowOptionCardProps { description: string; onClick: () => void; testId: string; + disabled?: boolean; } interface LandingScreenProps { aiAssistantEnabled: boolean; onBuildWithAI: (prompt: string) => void; onBuildFromScratch: () => void; + isBuildingFromScratch: boolean; onBrowseTemplates: () => void; onImportYAML: () => void; } @@ -22,6 +24,7 @@ export function LandingScreen({ aiAssistantEnabled, onBuildWithAI, onBuildFromScratch, + isBuildingFromScratch, onBrowseTemplates, onImportYAML, }: LandingScreenProps) { @@ -118,6 +121,7 @@ export function LandingScreen({ title="Build from scratch" description="Start with an empty canvas and pick a trigger as your first step." onClick={onBuildFromScratch} + disabled={isBuildingFromScratch} /> diff --git a/assets/js/collaborative-editor/components/inspector/TriggerInspector.tsx b/assets/js/collaborative-editor/components/inspector/TriggerInspector.tsx index e652e51e93..9a01fd249b 100644 --- a/assets/js/collaborative-editor/components/inspector/TriggerInspector.tsx +++ b/assets/js/collaborative-editor/components/inspector/TriggerInspector.tsx @@ -1,5 +1,7 @@ import { useEffect, useState } from 'react'; +import { useURLState } from '#/react/lib/use-url-state'; + import type { Workflow } from '../../types/workflow'; import { @@ -27,21 +29,47 @@ export function TriggerInspector({ onClose, onOpenRunPanel: _onOpenRunPanel, }: TriggerInspectorProps) { + const { params, updateSearchParams } = useURLState(); + + // `?trigger_view=picker` is a one-shot launch signal (set by the + // build-from-scratch redirect), not durable UI state: it only decides what + // this component mounts into on its very first render. Read with a lazy + // initializer so it's captured once, then cleared below (both from the URL + // and from this flag) — otherwise every later "Edit" click within the same + // TriggerInspector instance would keep re-opening the picker instead of the + // normal Choose step. + const [startedOnPicker, setStartedOnPicker] = useState( + () => params['trigger_view'] === 'picker' + ); + // View-state machine. The read-only "show" panel is the resting state for a // typed trigger; "edit" hands off to the unified wizard (Choose → Configure - // over a local draft). - const [view, setView] = useState<'show' | 'edit'>('show'); + // over a local draft). A fresh instance is mounted per trigger id (keyed by + // the caller), so this only ever needs to compute its starting state once. + const [view, setView] = useState<'show' | 'edit'>(() => + startedOnPicker ? 'edit' : 'show' + ); // When the user enters edit via an inline deep link ("Add authentication" / // "Configure default response status"), jump straight to Configure with that // section expanded. `undefined` = the plain Edit button → Choose step. Only // the webhook show panel produces a non-undefined focus. const [editFocus, setEditFocus] = useState(undefined); - // Reset to the resting state whenever a different trigger is selected. useEffect(() => { - setView('show'); - setEditFocus(undefined); - }, [trigger.id]); + if (!startedOnPicker) return; + // Strip the param with `replace: true` so it patches the current history + // entry in place, rather than pushing a new one that would leave a + // phantom Back-button stop pointing right back at the picker. + updateSearchParams({ trigger_view: null }, { replace: true }); + // TriggerEditWizard already captured startOnPicker in its own initial + // state by the time this effect runs, so clearing the flag here doesn't + // affect the picker that's already open — it only prevents a *future* + // Edit click (via the show panel) from reopening the picker again. + setStartedOnPicker(false); + // Run once on mount only: this is consuming the one-shot signal captured + // above, not reacting to subsequent param changes. + // eslint-disable-next-line react-hooks/exhaustive-deps + }, []); // The "edit" view is reachable only via a show panel's Edit button, which is // already disabled when the user can't edit the workflow, so no extra @@ -51,6 +79,7 @@ export function TriggerInspector({ { setView('show'); diff --git a/assets/js/collaborative-editor/components/inspector/trigger/TriggerEditWizard.tsx b/assets/js/collaborative-editor/components/inspector/trigger/TriggerEditWizard.tsx index 092931d1a9..725c7758b0 100644 --- a/assets/js/collaborative-editor/components/inspector/trigger/TriggerEditWizard.tsx +++ b/assets/js/collaborative-editor/components/inspector/trigger/TriggerEditWizard.tsx @@ -18,6 +18,13 @@ interface TriggerEditWizardProps { * start at Choose. */ initialFocus?: 'authentication' | 'response' | undefined; + /** + * Open directly on the Picker step ("What triggers this workflow?") + * instead of Choose. Independent of `initialFocus` — the two are never set + * together, since this is only used for the one-shot build-from-scratch + * entry point, not the in-app deep links that produce `initialFocus`. + */ + startOnPicker?: boolean; /** Close the inspector entirely. */ onClose: () => void; /** @@ -49,6 +56,7 @@ type Step = 'choose' | 'picker' | 'configure'; export function TriggerEditWizard({ trigger, initialFocus, + startOnPicker, onClose, onDone, }: TriggerEditWizardProps) { @@ -82,9 +90,13 @@ export function TriggerEditWizard({ } ); - // Initial step: rest on Choose, or jump straight to Configure on a deep-link. - // (The picker is still reachable mid-flow via the "Change" button.) - const [step, setStep] = useState(initialFocus ? 'configure' : 'choose'); + // Initial step: rest on Choose, jump straight to Configure on a deep-link, + // or jump straight to Picker on the build-from-scratch entry point. Once + // mounted, all navigation (back/pick-type/close) uses the wizard's normal + // step transitions regardless of how this initial step was chosen. + const [step, setStep] = useState( + startOnPicker ? 'picker' : initialFocus ? 'configure' : 'choose' + ); const finish = useCallback(async () => { const result = await commit(); diff --git a/assets/js/react/lib/use-url-state.ts b/assets/js/react/lib/use-url-state.ts index f088d47f53..2c11515c48 100644 --- a/assets/js/react/lib/use-url-state.ts +++ b/assets/js/react/lib/use-url-state.ts @@ -99,18 +99,28 @@ class URLStore { // Skip no-op writes so mount-time normalization doesn't stack duplicate // browser history entries (a no-op pushState never notifies subscribers // anyway, due to the guard in updateParams). Param order is ignored so a - // reorder-only write is also treated as a no-op. - private pushIfChanged = (newURL: URL) => { + // reorder-only write is also treated as a no-op. Pass `replace: true` to + // patch the current history entry in place instead of pushing a new one — + // for one-shot signals that shouldn't leave a Back-button stop pointing + // back at themselves. + private pushIfChanged = (newURL: URL, options?: { replace?: boolean }) => { if (this.urlsAreEquivalent(newURL, this.currentURL())) return; - history.pushState({}, '', newURL); + if (options?.replace) { + history.replaceState({}, '', newURL); + } else { + history.pushState({}, '', newURL); + } }; /** * Update URL search params (merges with existing params). * Accepts strings, numbers, booleans; null removes param. + * Pass `{ replace: true }` to patch the current history entry instead of + * pushing a new one. */ updateSearchParams = ( - updates: Record + updates: Record, + options?: { replace?: boolean } ) => { const newURL = this.currentURL(); @@ -122,15 +132,18 @@ class URLStore { } }); - this.pushIfChanged(newURL); + this.pushIfChanged(newURL, options); }; /** * Replace all URL search params (clears existing params). * Accepts strings, numbers, booleans; null skips param. + * Pass `{ replace: true }` to patch the current history entry instead of + * pushing a new one. */ replaceSearchParams = ( - newParams: Record + newParams: Record, + options?: { replace?: boolean } ) => { const newURL = this.currentURL(); newURL.search = ''; @@ -139,17 +152,19 @@ class URLStore { newURL.searchParams.set(key, String(value)); } }); - this.pushIfChanged(newURL); + this.pushIfChanged(newURL, options); }; /** * Update the URL hash fragment. * Pass null to remove hash. + * Pass `{ replace: true }` to patch the current history entry instead of + * pushing a new one. */ - updateHash = (hash: string | null) => { + updateHash = (hash: string | null, options?: { replace?: boolean }) => { const newURL = this.currentURL(); newURL.hash = hash ? `#${hash}` : ''; - this.pushIfChanged(newURL); + this.pushIfChanged(newURL, options); }; } diff --git a/assets/test/collaborative-editor/components/LandingScreen.test.tsx b/assets/test/collaborative-editor/components/LandingScreen.test.tsx index 2fb053ff5c..1f94f9e29d 100644 --- a/assets/test/collaborative-editor/components/LandingScreen.test.tsx +++ b/assets/test/collaborative-editor/components/LandingScreen.test.tsx @@ -20,6 +20,7 @@ function renderLandingScreen(props: { aiAssistantEnabled?: boolean; onBuildWithAI?: (prompt: string) => void; onBuildFromScratch?: () => void; + isBuildingFromScratch?: boolean; onBrowseTemplates?: () => void; onImportYAML?: () => void; }) { @@ -27,6 +28,7 @@ function renderLandingScreen(props: { aiAssistantEnabled: true, onBuildWithAI: vi.fn(), onBuildFromScratch: vi.fn(), + isBuildingFromScratch: false, onBrowseTemplates: vi.fn(), onImportYAML: vi.fn(), }; @@ -174,4 +176,16 @@ describe('LandingScreen - Card click handlers', () => { await user.click(screen.getByTestId('import-yaml-link')); expect(onImportYAML).toHaveBeenCalledTimes(1); }); + + test('build-from-scratch card is disabled while isBuildingFromScratch and does not call the handler', async () => { + const user = userEvent.setup(); + const onBuildFromScratch = vi.fn(); + renderLandingScreen({ onBuildFromScratch, isBuildingFromScratch: true }); + + const card = screen.getByTestId('build-from-scratch-card'); + expect(card).toBeDisabled(); + + await user.click(card); + expect(onBuildFromScratch).not.toHaveBeenCalled(); + }); }); diff --git a/assets/test/collaborative-editor/components/inspector/TriggerInspector.test.tsx b/assets/test/collaborative-editor/components/inspector/TriggerInspector.test.tsx index fa3702f4c9..96c25f968e 100644 --- a/assets/test/collaborative-editor/components/inspector/TriggerInspector.test.tsx +++ b/assets/test/collaborative-editor/components/inspector/TriggerInspector.test.tsx @@ -8,6 +8,7 @@ */ import { render, screen } from '@testing-library/react'; +import userEvent from '@testing-library/user-event'; import type React from 'react'; import { act } from 'react'; import { beforeEach, describe, expect, test, vi } from 'vitest'; @@ -34,8 +35,18 @@ import { createMockPhoenixChannelProvider, } from '../../__helpers__/channelMocks'; import { createMockSocket } from '../../__helpers__/sessionStoreHelpers'; +import { + createMockURLState, + getURLStateMockValue, +} from '../../__helpers__/urlStateMocks'; import { createWorkflowYDoc } from '../../__helpers__/workflowFactory'; +const urlState = createMockURLState(); + +vi.mock('../../../../js/react/lib/use-url-state', () => ({ + useURLState: () => getURLStateMockValue(urlState), +})); + // Mock useCanRun hook vi.mock('../../../../js/collaborative-editor/hooks/useWorkflow', async () => { const actual = await vi.importActual( @@ -216,3 +227,149 @@ describe('TriggerInspector — show dispatch by type', () => { ).toBeInTheDocument(); }); }); + +// --------------------------------------------------------------------------- +// Build-from-scratch picker entry (?trigger_view=picker) +// --------------------------------------------------------------------------- + +describe('TriggerInspector — build-from-scratch picker entry', () => { + let credentialStore: CredentialStoreInstance; + let adaptorStore: AdaptorStoreInstance; + let awarenessStore: AwarenessStoreInstance; + + function makeWebhookSessionContextStore(): SessionContextStoreInstance { + const sessionContextStore = createSessionContextStore(); + const mockChannel = createMockPhoenixChannel(); + const mockProvider = createMockPhoenixChannelProvider(mockChannel); + sessionContextStore._connectChannel(mockProvider as never); + + act(() => { + ( + mockChannel as never as { + _test: { emit: (e: string, m: unknown) => void }; + } + )._test.emit('session_context', { + user: null, + project: null, + config: { + require_email_verification: false, + kafka_triggers_enabled: false, + }, + permissions: { + can_edit_workflow: true, + can_run_workflow: true, + can_write_webhook_auth_method: true, + }, + latest_snapshot_lock_version: 1, + project_repo_connection: null, + webhook_auth_methods: [], + workflow_template: null, + has_read_ai_disclaimer: false, + }); + }); + + return sessionContextStore; + } + + function renderWebhookTrigger() { + const triggerId = 'trigger-webhook'; + const ydoc = createWorkflowYDoc({ + triggers: { + [triggerId]: { id: triggerId, type: 'webhook', enabled: true }, + }, + }); + ydoc.getMap('workflow').set('lock_version', 1); + + const workflowStore = createConnectedWorkflowStore(ydoc); + const sessionContextStore = makeWebhookSessionContextStore(); + const trigger = workflowStore.getSnapshot().triggers[0]; + + render( + , + { + wrapper: createWrapper( + workflowStore, + credentialStore, + sessionContextStore, + adaptorStore, + awarenessStore + ), + } + ); + } + + beforeEach(() => { + credentialStore = createCredentialStore(); + adaptorStore = createAdaptorStore(); + awarenessStore = createAwarenessStore(); + urlState.reset(); + }); + + test('opens directly on the picker when trigger_view=picker, and strips the param', () => { + urlState.setParams({ trigger: 'trigger-webhook', trigger_view: 'picker' }); + + renderWebhookTrigger(); + + expect( + screen.getByText('What triggers this workflow?') + ).toBeInTheDocument(); + expect( + screen.queryByRole('heading', { name: 'On webhook call' }) + ).not.toBeInTheDocument(); + + // One-shot: the launch signal is consumed and stripped immediately via a + // replace-in-place updateSearchParams (no phantom Back-button entry), + // leaving the rest of the URL (the trigger selection) untouched. + expect(urlState.mockFns.updateSearchParams).toHaveBeenCalledWith( + { trigger_view: null }, + { replace: true } + ); + }); + + test('renders the normal show panel when trigger_view is absent', () => { + urlState.setParams({ trigger: 'trigger-webhook' }); + + renderWebhookTrigger(); + + expect( + screen.getByRole('heading', { name: 'On webhook call' }) + ).toBeInTheDocument(); + expect(urlState.mockFns.updateSearchParams).not.toHaveBeenCalled(); + }); + + test('a later Edit click lands on Choose, not the picker again', async () => { + urlState.setParams({ trigger: 'trigger-webhook', trigger_view: 'picker' }); + + renderWebhookTrigger(); + + expect( + screen.getByText('What triggers this workflow?') + ).toBeInTheDocument(); + + const user = userEvent.setup(); + + // Back out of the picker to Choose, then Cancel out of the wizard + // entirely, returning to the resting show panel. + await user.click(screen.getByRole('button', { name: 'Back' })); + await user.click(screen.getByRole('button', { name: 'Cancel' })); + + expect( + screen.getByRole('heading', { name: 'On webhook call' }) + ).toBeInTheDocument(); + + // Re-entering edit via the show panel's Edit button must land on Choose, + // not re-trigger the one-shot picker entry from the original URL signal. + await user.click(screen.getByRole('button', { name: 'Edit trigger' })); + + expect( + screen.queryByText('What triggers this workflow?') + ).not.toBeInTheDocument(); + expect( + screen.getByRole('heading', { name: 'On webhook call' }) + ).toBeInTheDocument(); + }); +}); diff --git a/assets/test/collaborative-editor/components/inspector/trigger/TriggerEditWizard.test.tsx b/assets/test/collaborative-editor/components/inspector/trigger/TriggerEditWizard.test.tsx index e82103ab10..11dfe7a497 100644 --- a/assets/test/collaborative-editor/components/inspector/trigger/TriggerEditWizard.test.tsx +++ b/assets/test/collaborative-editor/components/inspector/trigger/TriggerEditWizard.test.tsx @@ -140,12 +140,13 @@ function createConnectedWorkflowStore( interface SetupOptions { initialFocus?: 'authentication' | 'response'; + startOnPicker?: boolean; } async function setup( trigger: Workflow.Trigger, workflowStore: WorkflowStoreInstance, - { initialFocus }: SetupOptions = {} + { initialFocus, startOnPicker }: SetupOptions = {} ) { const { wrapper, sessionChannel } = await createTriggerTestHarness({ canEdit: true, @@ -162,6 +163,7 @@ async function setup( , @@ -660,3 +662,46 @@ describe('TriggerEditWizard — type switching via picker', () => { ).toBeInTheDocument(); }); }); + +// =========================================================================== +// startOnPicker (build-from-scratch entry point) +// =========================================================================== + +describe('TriggerEditWizard — startOnPicker', () => { + let ydoc: Y.Doc; + + beforeEach(() => { + mockLiveViewActions.pushEvent.mockClear(); + ydoc = createWorkflowYDoc({ + triggers: { [TRIGGER_ID]: { id: TRIGGER_ID, type: 'webhook' } }, + }); + const workflowMap = ydoc.getMap('workflow'); + workflowMap.set('id', 'workflow-1'); + workflowMap.set('lock_version', 1); + workflowMap.set('deleted_at', null); + }); + + test('opens directly on the picker instead of Choose', async () => { + const workflowStore = createConnectedWorkflowStore(ydoc); + await setup(makeWebhookTrigger(), workflowStore, { startOnPicker: true }); + + expect( + screen.getByRole('heading', { name: 'What triggers this workflow?' }) + ).toBeInTheDocument(); + expect( + screen.queryByRole('heading', { name: 'On webhook call' }) + ).not.toBeInTheDocument(); + }); + + test('back arrow from the picker falls through to the normal Choose step', async () => { + const workflowStore = createConnectedWorkflowStore(ydoc); + await setup(makeWebhookTrigger(), workflowStore, { startOnPicker: true }); + + await userEvent.click(screen.getByRole('button', { name: 'Back' })); + + expect( + screen.getByRole('heading', { name: 'On webhook call' }) + ).toBeInTheDocument(); + expect(screen.getByRole('button', { name: 'Change' })).toBeInTheDocument(); + }); +}); diff --git a/assets/test/collaborative-editor/hooks/useAIWorkflowApplications.globalStep.test.ts b/assets/test/collaborative-editor/hooks/useAIWorkflowApplications.globalStep.test.ts index ff054d7a8f..70627a7dd2 100644 --- a/assets/test/collaborative-editor/hooks/useAIWorkflowApplications.globalStep.test.ts +++ b/assets/test/collaborative-editor/hooks/useAIWorkflowApplications.globalStep.test.ts @@ -48,6 +48,12 @@ describe('useAIWorkflowApplications - global messages', () => { const mockClearDiff = vi.fn(); const mockShowDiff = vi.fn(); + const mockStreamingApplyActions = { + set: vi.fn(), + setSaveFailed: vi.fn(), + clear: vi.fn(), + }; + const mockWorkflowActions = { importWorkflow: mockImportWorkflow, startApplyingWorkflow: mockStartApplyingWorkflow, @@ -136,6 +142,8 @@ describe('useAIWorkflowApplications - global messages', () => { previewingMessageId: null, setApplyingMessageId: mockSetApplyingMessageId, appliedMessageIdsRef: { current: new Set() }, + streamingApply: null, + streamingApplyActions: mockStreamingApplyActions, ...overrides, }) ); diff --git a/assets/test/e2e/specs/workflows/workflow-landing-screen.spec.ts b/assets/test/e2e/specs/workflows/workflow-landing-screen.spec.ts index 9cafdc4300..96b60e4b98 100644 --- a/assets/test/e2e/specs/workflows/workflow-landing-screen.spec.ts +++ b/assets/test/e2e/specs/workflows/workflow-landing-screen.spec.ts @@ -120,6 +120,36 @@ test.describe('landing screen — AI-disabled @landing-screen', () => { await expect(workflowEdit.buildWithAIInput).not.toBeAttached(); await expect(workflowEdit.buildWithAIButton).not.toBeAttached(); }); + + test('Build from scratch lands on the trigger picker, not the webhook show panel', async ({ + page, + }) => { + const workflowEdit = new WorkflowEditPage(page); + await navigateToNewWorkflow(page, projectId); + + await workflowEdit.buildFromScratchCard.click(); + await page.waitForURL(url => !url.pathname.endsWith('/w/new')); + await page.waitForLoadState('networkidle'); + + // The build-from-scratch redirect carries a one-shot ?trigger_view=picker + // signal (#4895) so the wizard opens straight on "What triggers this + // workflow?" instead of the webhook show panel — inviting the user to + // actively choose rather than silently defaulting to webhook. + await expect( + page.getByRole('heading', { name: 'What triggers this workflow?' }) + ).toBeVisible(); + await expect( + page.getByRole('heading', { name: 'On webhook call' }) + ).not.toBeVisible(); + + // The one-shot signal is stripped after being consumed. + expect(page.url()).not.toContain('trigger_view'); + + // Per #4895: not in the pink/new-workflow toolbar state — the workflow + // was already persisted by the redirect, so the normal Save button is + // present (unlike the landing screen, where it's absent entirely). + await expect(page.getByTestId('save-workflow-button')).toBeVisible(); + }); }); // --------------------------------------------------------------------------- diff --git a/assets/test/react/lib/use-url-state.test.ts b/assets/test/react/lib/use-url-state.test.ts index da1ee6a316..ea4fe8e86d 100644 --- a/assets/test/react/lib/use-url-state.test.ts +++ b/assets/test/react/lib/use-url-state.test.ts @@ -236,6 +236,48 @@ describe('useURLState', () => { }); }); + describe('updateSearchParams - replace option', () => { + afterEach(() => { + vi.restoreAllMocks(); + }); + + test('patches the current history entry instead of pushing a new one', () => { + history.replaceState({}, '', '/workflow?trigger=abc&trigger_view=picker'); + + const { result } = renderHook(() => useURLState()); + const pushSpy = vi.spyOn(history, 'pushState'); + const replaceSpy = vi.spyOn(history, 'replaceState'); + + act(() => { + result.current.updateSearchParams( + { trigger_view: null }, + { replace: true } + ); + }); + + expect(pushSpy).not.toHaveBeenCalled(); + expect(replaceSpy).toHaveBeenCalledTimes(1); + expect(result.current.params.trigger).toBe('abc'); + expect(result.current.params.trigger_view).toBeUndefined(); + expect(window.location.search).toBe('?trigger=abc'); + }); + + test('still skips the write entirely for a no-op with replace: true', () => { + history.replaceState({}, '', '/workflow?panel=run'); + + const { result } = renderHook(() => useURLState()); + const pushSpy = vi.spyOn(history, 'pushState'); + const replaceSpy = vi.spyOn(history, 'replaceState'); + + act(() => { + result.current.updateSearchParams({ panel: 'run' }, { replace: true }); + }); + + expect(pushSpy).not.toHaveBeenCalled(); + expect(replaceSpy).not.toHaveBeenCalled(); + }); + }); + describe('replaceSearchParams - no-op writes', () => { afterEach(() => { vi.restoreAllMocks(); diff --git a/lib/lightning/workflows.ex b/lib/lightning/workflows.ex index 4ea221a1a6..832681b67c 100644 --- a/lib/lightning/workflows.ex +++ b/lib/lightning/workflows.ex @@ -19,6 +19,7 @@ defmodule Lightning.Workflows do alias Lightning.Workflows.Trigger alias Lightning.Workflows.Triggers alias Lightning.Workflows.Workflow + alias Lightning.Workflows.WorkflowUsageLimiter alias Lightning.WorkflowVersions defdelegate subscribe(project_id), to: Events @@ -135,8 +136,7 @@ defmodule Lightning.Workflows do keyword() ) :: {:ok, Workflow.t()} - | {:error, Ecto.Changeset.t(Workflow.t())} - | {:error, :workflow_deleted} + | {:error, term()} def save_workflow(changeset_or_attrs, actor, opts \\ []) def save_workflow( @@ -203,6 +203,101 @@ defmodule Lightning.Workflows do |> save_workflow(actor, opts) end + @doc """ + Returns a workflow name that is unique within the given project, derived + from `base_name`. A blank or nil `base_name` defaults to + "Untitled workflow". On collision, appends " 1", " 2", etc. until a free + name is found. + + The check includes soft-deleted rows because the unique index on + `[:name, :project_id]` is not partial. (Delete paths rename workflows to + `_del` via `soft_delete_changeset/1`, so in practice deletion frees + the original name — but any row still occupying a name must be avoided.) + + Note: this is check-then-insert, so two concurrent saves can still compute + the same name and one will lose on the unique constraint. Callers already + handle that `{:error, changeset}`; no retry is attempted here. + """ + @spec unique_workflow_name(String.t() | nil, Ecto.UUID.t()) :: String.t() + def unique_workflow_name(base_name, project_id) do + base_name = + base_name + |> to_string() + |> String.trim() + |> case do + "" -> "Untitled workflow" + name -> name + end + + existing_names = + from(w in Workflow, + where: w.project_id == ^project_id, + select: w.name + ) + |> Repo.all() + |> MapSet.new() + + if MapSet.member?(existing_names, base_name) do + 1 + |> Stream.iterate(&(&1 + 1)) + |> Stream.map(&"#{base_name} #{&1}") + |> Enum.find(&(not MapSet.member?(existing_names, &1))) + else + base_name + end + end + + @doc """ + Creates a new workflow with a webhook trigger, one job, and one always-on edge. + Used by the "Build from Scratch" entry point on the new workflow screen. + Returns `{:ok, workflow, trigger_id}` or `{:error, reason}`, where `reason` + is usually an `Ecto.Changeset`, but see `save_workflow/3` for the other + shapes it can take. + """ + @spec create_webhook_workflow(String.t(), struct()) :: + {:ok, Workflow.t(), String.t()} + | {:error, term()} + def create_webhook_workflow(project_id, actor) do + trigger_id = Ecto.UUID.generate() + job_id = Ecto.UUID.generate() + + trigger_enabled = + case WorkflowUsageLimiter.limit_workflow_creation(project_id) do + :ok -> true + {:error, _, _} -> false + end + + attrs = %{ + name: unique_workflow_name(nil, project_id), + project_id: project_id, + triggers: [%{id: trigger_id, type: :webhook, enabled: trigger_enabled}], + jobs: [ + %{ + id: job_id, + name: "Untitled job", + adaptor: "@openfn/language-common@latest", + body: """ + // Check out the Job Writing Guide for help getting started: + // https://docs.openfn.org/documentation/jobs/job-writing-guide + """ + } + ], + edges: [ + %{ + source_trigger_id: trigger_id, + target_job_id: job_id, + condition_type: :always, + enabled: true + } + ] + } + + case save_workflow(attrs, actor) do + {:ok, workflow} -> {:ok, workflow, trigger_id} + {:error, reason} -> {:error, reason} + end + end + # Builds the Ecto.Multi pipeline for save_workflow. Does NOT call # Repo.transaction — that stays in the try/rescue block of the caller so # rescue wraps only the transaction, not this builder. diff --git a/lib/lightning_web/channels/workflow_channel.ex b/lib/lightning_web/channels/workflow_channel.ex index c1ac23f49b..8142448ad2 100644 --- a/lib/lightning_web/channels/workflow_channel.ex +++ b/lib/lightning_web/channels/workflow_channel.ex @@ -1137,40 +1137,11 @@ defmodule LightningWeb.WorkflowChannel do end defp ensure_unique_name(params, project) do - workflow_name = - params["name"] - |> to_string() - |> String.trim() - |> case do - "" -> "Untitled workflow" - name -> name - end - - existing_workflows = Lightning.Projects.list_workflows(project) - unique_name = generate_unique_name(workflow_name, existing_workflows) - - Map.put(params, "name", unique_name) - end - - defp generate_unique_name(base_name, existing_workflows) do - existing_names = MapSet.new(existing_workflows, & &1.name) - - if MapSet.member?(existing_names, base_name) do - find_available_name(base_name, existing_names) - else - base_name - end - end - - defp find_available_name(base_name, existing_names) do - 1 - |> Stream.iterate(&(&1 + 1)) - |> Stream.map(&"#{base_name} #{&1}") - |> Enum.find(&name_available?(&1, existing_names)) - end - - defp name_available?(name, existing_names) do - not MapSet.member?(existing_names, name) + Map.put( + params, + "name", + Lightning.Workflows.unique_workflow_name(params["name"], project.id) + ) end defp verify_trigger_in_workflow(trigger, workflow_id) do diff --git a/lib/lightning_web/live/workflow_live/collaborate.ex b/lib/lightning_web/live/workflow_live/collaborate.ex index b703f886ad..fdc9335a11 100644 --- a/lib/lightning_web/live/workflow_live/collaborate.ex +++ b/lib/lightning_web/live/workflow_live/collaborate.ex @@ -12,6 +12,7 @@ defmodule LightningWeb.WorkflowLive.Collaborate do alias Lightning.AiAssistant alias Lightning.Policies.Permissions + alias Lightning.Policies.ProjectUsers alias Lightning.Projects alias Lightning.Projects.Project alias Lightning.Workflows @@ -178,6 +179,39 @@ defmodule LightningWeb.WorkflowLive.Collaborate do )} end + def handle_event("build_from_scratch", _params, socket) do + with :ok <- + Permissions.can( + ProjectUsers, + :create_workflow, + socket.assigns.current_user, + socket.assigns.project + ), + {:ok, workflow, trigger_id} <- + Workflows.create_webhook_workflow( + socket.assigns.project.id, + socket.assigns.current_user + ) do + {:noreply, + push_navigate(socket, + to: + ~p"/projects/#{socket.assigns.project}/w/#{workflow}?#{%{trigger: trigger_id, trigger_view: "picker"}}", + replace: true + )} + else + {:error, :unauthorized} -> + {:noreply, + put_flash( + socket, + :error, + "You don't have permission to create workflows" + )} + + {:error, _reason} -> + {:noreply, put_flash(socket, :error, "Failed to create workflow")} + end + end + @impl true def render(assigns) do ~H""" diff --git a/test/lightning/workflows_test.exs b/test/lightning/workflows_test.exs index f83b9fcc97..6613015b28 100644 --- a/test/lightning/workflows_test.exs +++ b/test/lightning/workflows_test.exs @@ -932,6 +932,181 @@ defmodule Lightning.WorkflowsTest do end end + describe "create_webhook_workflow/2" do + test "creates a workflow with a webhook trigger, a job, and an always edge" do + project = insert(:project) + user = insert(:user) + + assert {:ok, workflow, trigger_id} = + Workflows.create_webhook_workflow(project.id, user) + + workflow = + Repo.preload(workflow, [:triggers, :jobs, :edges]) + + assert workflow.name == "Untitled workflow" + assert workflow.project_id == project.id + + assert [%Trigger{id: ^trigger_id, type: :webhook, enabled: true}] = + workflow.triggers + + assert [ + %{ + name: "Untitled job", + adaptor: "@openfn/language-common@latest", + body: body + } + ] = workflow.jobs + + assert body =~ "Job Writing Guide" + + [job] = workflow.jobs + + assert [ + %{ + source_trigger_id: ^trigger_id, + target_job_id: job_id, + condition_type: :always, + enabled: true + } + ] = workflow.edges + + assert job_id == job.id + end + + test "returns {:error, changeset} for an invalid project_id" do + user = insert(:user) + + assert {:error, %Ecto.Changeset{} = changeset} = + Workflows.create_webhook_workflow(Ecto.UUID.generate(), user) + + assert %{project: ["does not exist"]} = errors_on(changeset) + end + + test "suffixes the name when called repeatedly in the same project" do + project = insert(:project) + user = insert(:user) + + assert {:ok, first, _} = + Workflows.create_webhook_workflow(project.id, user) + + assert {:ok, second, _} = + Workflows.create_webhook_workflow(project.id, user) + + assert first.name == "Untitled workflow" + assert second.name == "Untitled workflow 1" + end + + test "creates trigger and edge with enabled: false when at the activation limit" do + project = insert(:project) + user = insert(:user) + + Mox.stub( + Lightning.Extensions.MockUsageLimiter, + :limit_action, + fn _action, _context -> + {:error, :limit_exceeded, + %Lightning.Extensions.Message{ + text: "Workflow activation limit exceeded" + }} + end + ) + + assert {:ok, workflow, trigger_id} = + Workflows.create_webhook_workflow(project.id, user) + + workflow = Repo.preload(workflow, [:triggers, :edges]) + + assert [%{id: ^trigger_id, enabled: false}] = workflow.triggers + + # edges from a trigger are always enabled (Edge.enable_if_source_trigger/1 enforces this) + assert [%{enabled: true}] = workflow.edges + end + + test "succeeds again after the previous workflow is deleted" do + project = insert(:project) + user = insert(:user) + + assert {:ok, first, _} = + Workflows.create_webhook_workflow(project.id, user) + + assert {:ok, _} = Workflows.mark_for_deletion(first, user) + + # Soft deletion renames the workflow to "_del", freeing the + # original name, so the new workflow gets it back unsuffixed. + assert {:ok, second, _} = + Workflows.create_webhook_workflow(project.id, user) + + assert second.name == "Untitled workflow" + end + end + + describe "unique_workflow_name/2" do + test "returns the base name unchanged when it is free" do + project = insert(:project) + + assert Workflows.unique_workflow_name("My Workflow", project.id) == + "My Workflow" + end + + test "defaults nil or blank names to Untitled workflow" do + project = insert(:project) + + assert Workflows.unique_workflow_name(nil, project.id) == + "Untitled workflow" + + assert Workflows.unique_workflow_name("", project.id) == + "Untitled workflow" + + assert Workflows.unique_workflow_name(" ", project.id) == + "Untitled workflow" + end + + test "trims surrounding whitespace" do + project = insert(:project) + + assert Workflows.unique_workflow_name(" My Workflow ", project.id) == + "My Workflow" + end + + test "appends an incrementing suffix on collision" do + project = insert(:project) + insert(:workflow, project: project, name: "My Workflow") + + assert Workflows.unique_workflow_name("My Workflow", project.id) == + "My Workflow 1" + + insert(:workflow, project: project, name: "My Workflow 1") + + assert Workflows.unique_workflow_name("My Workflow", project.id) == + "My Workflow 2" + end + + test "ignores workflows in other projects" do + project = insert(:project) + other_project = insert(:project) + insert(:workflow, project: other_project, name: "My Workflow") + + assert Workflows.unique_workflow_name("My Workflow", project.id) == + "My Workflow" + end + + # Real delete paths rename to "_del" (freeing the name), but the + # unique index is not partial, so any row still occupying a name — even + # a soft-deleted one — must be counted as a collision. + test "includes soft-deleted rows in the collision check" do + project = insert(:project) + + insert(:workflow, + project: project, + name: "My Workflow", + deleted_at: DateTime.utc_now() |> DateTime.truncate(:second) + ) + + assert Workflows.unique_workflow_name("My Workflow", project.id) == + "My Workflow 1" + end + end + describe "save_workflow/3 rescue" do setup do Mimic.copy(Lightning.WorkflowVersions) diff --git a/test/lightning_web/live/workflow_live/collaborate_test.exs b/test/lightning_web/live/workflow_live/collaborate_test.exs index a98816facb..98f3b181f1 100644 --- a/test/lightning_web/live/workflow_live/collaborate_test.exs +++ b/test/lightning_web/live/workflow_live/collaborate_test.exs @@ -1,6 +1,7 @@ defmodule LightningWeb.WorkflowLive.CollaborateTest do use LightningWeb.ConnCase, async: false + import Ecto.Query import Lightning.Factories import Lightning.WorkflowsFixtures import Phoenix.LiveViewTest @@ -2723,4 +2724,190 @@ defmodule LightningWeb.WorkflowLive.CollaborateTest do assert html =~ "collaborative-editor-react" end end + + describe "build_from_scratch event" do + test "creates a webhook workflow and navigates to it with the trigger param", + %{conn: conn} do + user = insert(:user) + + project = + insert(:project, + project_users: [%{user_id: user.id, role: :owner}] + ) + + workflow = workflow_fixture(project_id: project.id) + + conn = log_in_user(conn, user) + + {:ok, view, _html} = + live( + conn, + ~p"/projects/#{project.id}/w/#{workflow.id}" + ) + + view + |> element("#collaborative-editor-react") + |> render_hook("build_from_scratch", %{}) + + {path, _flash} = assert_redirect(view) + + assert %{"trigger" => trigger_id, "trigger_view" => "picker"} = + URI.decode_query(URI.parse(path).query) + + assert path =~ ~r{^/projects/#{project.id}/w/[0-9a-f-]+\?} + + new_workflow_id = + path + |> String.split("/w/") + |> List.last() + |> String.split("?") + |> List.first() + + new_workflow = + Lightning.Workflows.get_workflow!(new_workflow_id) + |> Lightning.Repo.preload(:triggers) + + assert new_workflow.project_id == project.id + assert [%{id: ^trigger_id, type: :webhook}] = new_workflow.triggers + end + + test "user without project access gets a flash error", %{conn: conn} do + user = insert(:user) + + project = + insert(:project, + project_users: [%{user_id: user.id, role: :owner}] + ) + + workflow = workflow_fixture(project_id: project.id) + + conn = log_in_user(conn, user) + + {:ok, view, _html} = + live( + conn, + ~p"/projects/#{project.id}/w/#{workflow.id}" + ) + + # Revoke the user's membership after mount (e.g. removed from the + # project mid-session) so the LiveView's cached `project`/`current_user` + # assigns remain, but Bodyguard's live authorization check now fails. + Lightning.Repo.delete_all( + from(pu in Lightning.Projects.ProjectUser, + where: pu.project_id == ^project.id and pu.user_id == ^user.id + ) + ) + + view + |> element("#collaborative-editor-react") + |> render_hook("build_from_scratch", %{}) + + assert %{"error" => "You don't have permission to create workflows"} = + :sys.get_state(view.pid).socket.assigns.flash + end + + test "viewer gets a flash error and no workflow is created", %{conn: conn} do + user = insert(:user) + + project = + insert(:project, + project_users: [%{user_id: user.id, role: :viewer}] + ) + + workflow = workflow_fixture(project_id: project.id) + + conn = log_in_user(conn, user) + + {:ok, view, _html} = + live( + conn, + ~p"/projects/#{project.id}/w/#{workflow.id}" + ) + + workflow_count_before = + Lightning.Repo.aggregate(Lightning.Workflows.Workflow, :count) + + view + |> element("#collaborative-editor-react") + |> render_hook("build_from_scratch", %{}) + + assert %{"error" => "You don't have permission to create workflows"} = + :sys.get_state(view.pid).socket.assigns.flash + + assert Lightning.Repo.aggregate(Lightning.Workflows.Workflow, :count) == + workflow_count_before + end + + test "suffixes the workflow name when clicked again in the same project", + %{conn: conn} do + user = insert(:user) + + project = + insert(:project, + project_users: [%{user_id: user.id, role: :owner}] + ) + + workflow = workflow_fixture(project_id: project.id) + + conn = log_in_user(conn, user) + + names = + for _ <- 1..2 do + {:ok, view, _html} = + live(conn, ~p"/projects/#{project.id}/w/#{workflow.id}") + + view + |> element("#collaborative-editor-react") + |> render_hook("build_from_scratch", %{}) + + {path, _flash} = assert_redirect(view) + + new_workflow_id = + path + |> String.split("/w/") + |> List.last() + |> String.split("?") + |> List.first() + + Lightning.Workflows.get_workflow!(new_workflow_id).name + end + + assert names == ["Untitled workflow", "Untitled workflow 1"] + end + + test "shows an error flash when workflow creation fails" do + Mimic.copy(Lightning.Workflows) + + Mimic.stub( + Lightning.Workflows, + :create_webhook_workflow, + fn _project_id, _actor -> {:error, %Ecto.Changeset{}} end + ) + + user = insert(:user) + + project = + insert(:project, + project_users: [%{user_id: user.id, role: :owner}] + ) + + socket = %Phoenix.LiveView.Socket{ + assigns: %{ + __changed__: %{}, + flash: %{}, + current_user: user, + project: project + } + } + + assert {:noreply, socket} = + LightningWeb.WorkflowLive.Collaborate.handle_event( + "build_from_scratch", + %{}, + socket + ) + + assert socket.assigns.flash == %{"error" => "Failed to create workflow"} + end + end end