From bfdbfaa69b0e4342b983c69d89b495cd7377e6f8 Mon Sep 17 00:00:00 2001 From: yannickmonney Date: Sun, 4 Oct 2026 14:17:37 +0000 Subject: [PATCH 1/2] fix(platform): keep unsaved trigger and project edits on refresh The automation General tab reloaded its trigger form and its project selection wholesale whenever the stored row changed, so a save from another session erased a draft in progress and disabled Save. A trigger field the author changed now keeps the edit while the fields they left alone take the incoming row, and unsaved project adds and removes are replayed on the incoming set. After its own save the trigger form measures edits against what it sent, so the row the store answers with settles it, also when that row arrives before the save answers. Closes #3620 --- .../project-bindings-section.test.tsx | 93 ++++++++++- .../components/project-bindings-section.tsx | 36 +++- .../components/trigger-editor.test.tsx | 134 ++++++++++++++- .../automations/components/trigger-editor.tsx | 154 ++++++++++++------ 4 files changed, 353 insertions(+), 64 deletions(-) diff --git a/services/platform/app/features/automations/components/project-bindings-section.test.tsx b/services/platform/app/features/automations/components/project-bindings-section.test.tsx index 480972d391..d5c6b4331c 100644 --- a/services/platform/app/features/automations/components/project-bindings-section.test.tsx +++ b/services/platform/app/features/automations/components/project-bindings-section.test.tsx @@ -22,6 +22,7 @@ vi.mock('@/app/features/projects/hooks/queries', () => ({ projects: [ { _id: 'proj_1', name: 'Document desk' }, { _id: 'proj_2', name: 'Getting started' }, + { _id: 'proj_3', name: 'Field service' }, ], isLoading: false, }), @@ -51,6 +52,27 @@ function GeneralTab({ children }: { children: ReactNode }) { const saveButton = () => screen.getByRole('button', { name: 'common.actions.save' }); +/** The selected projects, in chip order — each chip carries a remove button. */ +const selectedProjects = () => + screen + .queryAllByRole('button', { name: /^Remove / }) + .map((button) => + button.getAttribute('aria-label')?.replace(/^Remove /, ''), + ); + +/** An author's section, rendered again as each query answer arrives. */ +function section() { + return ( + + + + ); +} + describe('ProjectBindingsSection', () => { it('shows the hint and no count for an unbound automation, and waits for an edit', () => { boundData = []; @@ -106,15 +128,6 @@ describe('ProjectBindingsSection', () => { it('keeps a selection in progress when a refetch answers the same set', async () => { boundData = ['proj_1']; - const section = () => ( - - - - ); const { user, rerender } = render(section()); await user.click(screen.getByRole('combobox')); @@ -127,6 +140,68 @@ describe('ProjectBindingsSection', () => { expect(saveButton()).toBeEnabled(); }); + // A set another session saved reaches a selection holding an edit (#3620): + // the author's unsaved adds and removes are replayed onto it. + it('keeps an unsaved project when another session changes the bound set', async () => { + boundData = ['proj_1']; + setProjects.mutateAsync.mockResolvedValue(undefined); + const { user, rerender } = render(section()); + + await user.click(screen.getByRole('combobox')); + await user.click(screen.getByRole('option', { name: /Getting started/ })); + await user.keyboard('{Escape}'); + boundData = ['proj_1', 'proj_3']; + rerender(section()); + + expect(selectedProjects()).toEqual([ + 'Document desk', + 'Field service', + 'Getting started', + ]); + expect(saveButton()).toBeEnabled(); + await user.click(saveButton()); + expect(setProjects.mutateAsync).toHaveBeenLastCalledWith( + expect.objectContaining({ projectIds: ['proj_1', 'proj_3', 'proj_2'] }), + ); + }); + + it('keeps an unsaved removal when another session adds a project', async () => { + boundData = ['proj_1', 'proj_2']; + const { user, rerender } = render(section()); + + await user.click( + screen.getByRole('button', { name: 'Remove Document desk' }), + ); + boundData = ['proj_1', 'proj_2', 'proj_3']; + rerender(section()); + + expect(selectedProjects()).toEqual(['Getting started', 'Field service']); + expect(saveButton()).toBeEnabled(); + }); + + it('settles on the set its own save wrote', async () => { + boundData = ['proj_1']; + setProjects.mutateAsync.mockResolvedValue(undefined); + const { user, rerender } = render(section()); + + await user.click(screen.getByRole('combobox')); + await user.click(screen.getByRole('option', { name: /Getting started/ })); + await user.keyboard('{Escape}'); + await user.click(saveButton()); + // The store answers with the saved set, in its own order. + boundData = ['proj_2', 'proj_1']; + rerender(section()); + + expect(selectedProjects()).toEqual(['Getting started', 'Document desk']); + // Nothing is left to save (Save reads "Saved" for a moment). + expect( + screen.getByRole('button', { name: 'common.actions.discard' }), + ).toBeDisabled(); + expect( + screen.getByRole('button', { name: /^common\.actions\.saved?$/ }), + ).toBeDisabled(); + }); + it('discards the selection back to the stored set', async () => { boundData = ['proj_1']; const { user } = render( diff --git a/services/platform/app/features/automations/components/project-bindings-section.tsx b/services/platform/app/features/automations/components/project-bindings-section.tsx index 1244417936..b9058ec3eb 100644 --- a/services/platform/app/features/automations/components/project-bindings-section.tsx +++ b/services/platform/app/features/automations/components/project-bindings-section.tsx @@ -28,6 +28,22 @@ function sameSelection(a: readonly string[], b: readonly string[]): boolean { return a.length === b.length && a.every((value, index) => value === b[index]); } +/** + * `incoming` with the author's unsaved edits replayed on it: the projects + * they added since `loaded` stay picked, the ones they removed stay off. + */ +function keepEdits( + loaded: readonly string[], + current: readonly string[], + incoming: readonly string[], +): string[] { + const removed = new Set(loaded.filter((id) => !current.includes(id))); + const added = current.filter( + (id) => !loaded.includes(id) && !incoming.includes(id), + ); + return [...incoming.filter((id) => !removed.has(id)), ...added]; +} + /** * The automation's project bindings: which projects' task boards see it. * @@ -64,17 +80,25 @@ export function ProjectBindingsSection({ ); const [selection, setSelection] = useState([]); - // The rows are the truth; local state only carries unsaved edits. Keyed on - // the set's content, not on the array, so a refetch that answers the same - // set never wipes a selection in progress; and the functional update - // returns the CURRENT array when the content already matches, so React - // bails out of the re-render. + // The rows are the truth; local state only carries unsaved edits, which a + // set another session saved does not erase: they are replayed on it. Keyed + // on the set's content, not on the array, so a refetch that answers the + // same set loads nothing; and the functional update returns the CURRENT + // array when the content already matches, so React bails out of the + // re-render. const storedRef = useRef(stored); storedRef.current = stored; + // The set the selection last loaded: what tells the author's edits apart. + const loadedRef = useRef([]); const storedKey = stored.join(','); useEffect(() => { const next = storedRef.current; - setSelection((current) => (sameSelection(current, next) ? current : next)); + const loaded = loadedRef.current; + loadedRef.current = next; + setSelection((current) => { + const rebased = keepEdits(loaded, current, next); + return sameSelection(current, rebased) ? current : rebased; + }); }, [storedKey]); const dirty = useMemo(() => { diff --git a/services/platform/app/features/automations/components/trigger-editor.test.tsx b/services/platform/app/features/automations/components/trigger-editor.test.tsx index 9ddb1dd782..eacb329f70 100644 --- a/services/platform/app/features/automations/components/trigger-editor.test.tsx +++ b/services/platform/app/features/automations/components/trigger-editor.test.tsx @@ -4,7 +4,7 @@ import userEvent from '@testing-library/user-event'; import type { AnchorHTMLAttributes, ReactNode } from 'react'; import { beforeEach, describe, expect, it, vi } from 'vitest'; -import { render, screen, waitFor } from '@/tests/utils/render'; +import { act, render, screen, waitFor } from '@/tests/utils/render'; import { AutomationEditorActions } from './automation-editor-actions'; import { TriggerEditor } from './trigger-editor'; @@ -129,6 +129,8 @@ function renderTrigger( const saveButton = () => screen.getByRole('button', { name: 'Save' }); const discardButton = () => screen.getByRole('button', { name: 'Discard' }); +/** Save, which reads "Saved" for a moment after a save went through. */ +const savedButton = () => screen.getByRole('button', { name: /^Saved?$/ }); describe('TriggerEditor', () => { beforeEach(() => { @@ -200,6 +202,136 @@ describe('TriggerEditor', () => { expect(saveButton()).toBeEnabled(); }); + // A row another session saved reaches a form holding an edit (#3620): the + // fields the author changed keep the edit, the ones they left alone follow + // the row — and a save's own row still settles the form. + describe('a row saved while the form holds an edit', () => { + const WEBHOOK_ROW = { + name: 'gmail-triage-inbox', + kind: 'webhook', + hasToken: true, + enabled: true, + }; + + function rerenderWith( + rerender: ReturnType['rerender'], + row: NonNullable[number], + ) { + triggersData = [row]; + rerender( + + + , + ); + } + + async function editCron() { + const cron = screen.getByLabelText('Cron'); + await userEvent.clear(cron); + await userEvent.paste('0 9 * * 1'); + return cron; + } + + /** Edit the cron, make the binding a webhook, and press Save. */ + async function saveCronEditAsWebhook() { + await editCron(); + await userEvent.click( + screen.getByRole('combobox', { name: 'Trigger type' }), + ); + await userEvent.click(screen.getByRole('option', { name: 'Webhook' })); + await userEvent.click(saveButton()); + await waitFor(() => expect(mockSetTrigger).toHaveBeenCalledTimes(1)); + } + + it('keeps an edited cron when another session switches the trigger off', async () => { + const { rerender } = renderTrigger(); + const cron = await editCron(); + rerenderWith(rerender, { ...SCHEDULE_ROW, enabled: false }); + + expect(cron).toHaveValue('0 9 * * 1'); + // The switch the author left alone takes the other session's answer. + expect(screen.getByRole('switch', { name: 'Enabled' })).not.toBeChecked(); + expect(saveButton()).toBeEnabled(); + + await userEvent.click(saveButton()); + expect(mockSetTrigger).toHaveBeenCalledWith({ + organizationId: 'org-1', + name: 'gmail-triage-inbox', + trigger: { + kind: 'schedule', + cron: '0 9 * * 1', + timezone: 'UTC', + enabled: false, + }, + }); + }); + + it('keeps an edited cron when another session saved a different one', async () => { + const { rerender } = renderTrigger(); + const cron = await editCron(); + rerenderWith(rerender, { ...SCHEDULE_ROW, cron: '0 7 * * *' }); + + expect(cron).toHaveValue('0 9 * * 1'); + expect(saveButton()).toBeEnabled(); + }); + + it('keeps the edit when the save is refused while another session’s row arrives', async () => { + let refuse: (error: Error) => void = () => {}; + mockSetTrigger.mockImplementation( + () => + new Promise((_resolve, reject) => { + refuse = reject; + }), + ); + const { rerender } = renderTrigger(); + const cron = await editCron(); + await userEvent.click(saveButton()); + await waitFor(() => expect(mockSetTrigger).toHaveBeenCalledTimes(1)); + rerenderWith(rerender, { ...SCHEDULE_ROW, enabled: false }); + await act(async () => { + refuse(new Error('The store refused the trigger.')); + }); + + expect(cron).toHaveValue('0 9 * * 1'); + expect(screen.getByRole('switch', { name: 'Enabled' })).not.toBeChecked(); + expect(saveButton()).toBeEnabled(); + }); + + it('settles on the row its own save wrote, cron left behind', async () => { + const { rerender } = renderTrigger(); + await saveCronEditAsWebhook(); + // The store holds the webhook now, with no cron. + rerenderWith(rerender, WEBHOOK_ROW); + + // Nothing is left to save: the cluster reads clean, not dirty. + expect(discardButton()).toBeDisabled(); + expect(savedButton()).toBeDisabled(); + }); + + it('settles on its own save when the row lands before the save answers', async () => { + let answer: (value: object) => void = () => {}; + mockSetTrigger.mockImplementation( + () => + new Promise((resolve) => { + answer = resolve; + }), + ); + const { rerender } = renderTrigger(); + await saveCronEditAsWebhook(); + rerenderWith(rerender, WEBHOOK_ROW); + await act(async () => { + answer({}); + }); + + expect(discardButton()).toBeDisabled(); + expect(savedButton()).toBeDisabled(); + }); + }); + it('holds Save while the cron cannot be read', async () => { renderTrigger(); diff --git a/services/platform/app/features/automations/components/trigger-editor.tsx b/services/platform/app/features/automations/components/trigger-editor.tsx index 90524b3848..658af2c69f 100644 --- a/services/platform/app/features/automations/components/trigger-editor.tsx +++ b/services/platform/app/features/automations/components/trigger-editor.tsx @@ -60,6 +60,35 @@ type StoredTrigger = NonNullable< ReturnType['data'] >[number]; +/** The fields the form edits, as a binding holds them. */ +interface TriggerFields { + kind: string; + cron: string; + timezone: string; + event: string; + enabled: boolean; +} + +/** The empty form a new binding starts from. */ +const NO_TRIGGER: TriggerFields = { + kind: 'schedule', + cron: '', + timezone: 'UTC', + event: '', + enabled: false, +}; + +function triggerFields(row: StoredTrigger | undefined): TriggerFields { + if (row === undefined) return NO_TRIGGER; + return { + kind: row.kind, + cron: row.cron ?? '', + timezone: row.timezone ?? 'UTC', + event: row.event ?? '', + enabled: row.enabled, + }; +} + const NO_DIRTY_KEYS: ReadonlySet = new Set(); /** What the General tab's strip lights its unsaved dot for. */ const TRIGGER_DIRTY_KEYS: ReadonlySet = new Set([TRIGGER_DIRTY_KEY]); @@ -144,44 +173,50 @@ export function TriggerEditor({ // empty schedule that looks armed. const [adding, setAdding] = useState(false); - /** Put a binding (none: the empty form) into the fields. */ - const applyStored = useCallback((row: typeof stored) => { - setAdding(false); - if (row === undefined) { - setKind('schedule'); - setCron(''); - setTimezone('UTC'); - setEventName(''); - setEnabled(false); - return; - } - if (isTriggerKind(row.kind)) setKind(row.kind); - setCron(row.cron ?? ''); - setTimezone(row.timezone ?? 'UTC'); - setEventName(row.event ?? ''); - setEnabled(row.enabled); - }, []); + // The fields as the form last loaded them (or saved them): what tells an + // edit from a field the author left alone. + const loadedRef = useRef(NO_TRIGGER); + + /** + * Put a binding (none: the empty form) into the fields. With `keepEdits`, + * a field the author changed since the form last loaded keeps the edit and + * only the fields left alone take the binding's values: a row another + * session saved must not erase a draft in progress. + */ + const applyStored = useCallback( + (row: StoredTrigger | undefined, keepEdits = false) => { + const loaded = loadedRef.current; + const next = triggerFields(row); + loadedRef.current = next; + const take = (current: T, wasLoaded: unknown, incoming: T): T => + keepEdits && current !== wasLoaded ? current : incoming; + setAdding(false); + const nextKind = next.kind; + if (isTriggerKind(nextKind)) { + setKind((current) => take(current, loaded.kind, nextKind)); + } + setCron((current) => take(current, loaded.cron, next.cron)); + setTimezone((current) => take(current, loaded.timezone, next.timezone)); + setEventName((current) => take(current, loaded.event, next.event)); + setEnabled((current) => take(current, loaded.enabled, next.enabled)); + }, + [], + ); // Load the stored binding into the form whenever it changes under us — - // the row is the truth; local state only carries unsaved edits. Keyed on - // the fields the form edits, not on the row object: a refetch that only - // moves `lastFiredAt` (the trigger just fired) must not wipe an edit in - // progress. + // the row is the truth; local state only carries unsaved edits, which + // the load keeps. Keyed on the fields the form edits, not on the row + // object: a refetch that only moves `lastFiredAt` (the trigger just + // fired) loads nothing. const storedRef = useRef(stored); storedRef.current = stored; const storedFields = - stored === undefined - ? null - : JSON.stringify([ - stored.kind, - stored.cron ?? '', - stored.timezone ?? 'UTC', - stored.event ?? '', - stored.enabled, - ]); + stored === undefined ? null : JSON.stringify(triggerFields(stored)); + const storedFieldsRef = useRef(storedFields); + storedFieldsRef.current = storedFields; useEffect(() => { if (storedFields === null) return; - applyStored(storedRef.current); + applyStored(storedRef.current, true); }, [storedFields, applyStored]); const dirty = useMemo(() => { @@ -251,23 +286,46 @@ export function TriggerEditor({ const persist = async (rotateToken?: boolean): Promise => { setRefusal(null); setMintedToken(null); - const result = await setTrigger.mutateAsync({ - organizationId, - name, - trigger: { - kind, - ...(kind === 'schedule' && cron !== '' && { cron }), - ...(kind === 'schedule' && timezone !== '' && { timezone }), - ...(kind === 'event' && eventName !== '' && { event: eventName }), - enabled, - }, - ...(rotateToken === true && { rotateToken: true }), - }); - if (result.token !== undefined) setMintedToken(result.token); - // The server names the live URL this bind stopped answering on — say so, - // since nothing on the page shows the old URL any more. - if (result.revoked === 'webhook') { - toast({ title: t('trigger.revokedToast') }); + const sent: TriggerFields = { + kind, + cron, + timezone, + event: eventName, + enabled, + }; + const storedFieldsBefore = storedFieldsRef.current; + try { + const result = await setTrigger.mutateAsync({ + organizationId, + name, + trigger: { + kind, + ...(kind === 'schedule' && cron !== '' && { cron }), + ...(kind === 'schedule' && timezone !== '' && { timezone }), + ...(kind === 'event' && eventName !== '' && { event: eventName }), + enabled, + }, + ...(rotateToken === true && { rotateToken: true }), + }); + // The store holds the form as sent: a field still as sent takes the + // row it answers with, which drops what the kind leaves out (a + // webhook keeps no cron), while an edit made during the save stays. + loadedRef.current = sent; + if (result.token !== undefined) setMintedToken(result.token); + // The server names the live URL this bind stopped answering on — say + // so, since nothing on the page shows the old URL any more. + if (result.revoked === 'webhook') { + toast({ title: t('trigger.revokedToast') }); + } + } finally { + // A row that arrived while the save was out loaded against the form + // as it stood before; load it again against what the store holds now. + if ( + storedFieldsRef.current !== null && + storedFieldsRef.current !== storedFieldsBefore + ) { + applyStored(storedRef.current, true); + } } }; From 7def0ab5e18ae8b2a02d7ac47113454f21b849ee Mon Sep 17 00:00:00 2001 From: yannickmonney Date: Mon, 5 Oct 2026 01:49:23 +0000 Subject: [PATCH 2/2] fix(platform): keep edits made while a General tab save is out The trigger and project sections stay editable while their save is in flight, but they told the author's edits apart only against the last loaded row. A change made after Save back to the loaded value (an undo of the cron, or dropping the project just added) looked untouched, so the save's own row or set re-applied the saved value over it and Save and Discard both went dark. Each section now keeps what its save in flight sent. A field or project that differs from the last load or from what the save sent stays the author's, whichever of the save's row and its answer arrives first. A field the save left as loaded follows each incoming row in that snapshot too, so a row another session saved during the save cannot leave a phantom edit once the save's own row lands. Refused saves still keep the draft, and a successful save still settles clean. Refs #3620 --- .../project-bindings-section.test.tsx | 79 ++++++++++++++++++- .../components/project-bindings-section.tsx | 37 ++++++--- .../components/trigger-editor.test.tsx | 60 ++++++++++++++ .../automations/components/trigger-editor.tsx | 64 ++++++++++++--- 4 files changed, 219 insertions(+), 21 deletions(-) diff --git a/services/platform/app/features/automations/components/project-bindings-section.test.tsx b/services/platform/app/features/automations/components/project-bindings-section.test.tsx index d5c6b4331c..48e8a1c8c1 100644 --- a/services/platform/app/features/automations/components/project-bindings-section.test.tsx +++ b/services/platform/app/features/automations/components/project-bindings-section.test.tsx @@ -4,7 +4,7 @@ import { ActiveEditorProvider, EditorGroup } from '@tale/ui/editor'; import React, { type ReactNode } from 'react'; import { describe, expect, it, vi } from 'vitest'; -import { render, screen } from '@/tests/utils/render'; +import { act, render, screen, waitFor } from '@/tests/utils/render'; // The section reads the bound set and the org's projects reactively and saves // through the reconcile mutation; the tests stub all three seams. @@ -202,6 +202,83 @@ describe('ProjectBindingsSection', () => { ).toBeDisabled(); }); + /** Pick Getting started and press Save, whose answer waits for `answer`. */ + async function addAndSaveDeferred( + user: ReturnType['user'], + ): Promise<() => void> { + let answer: () => void = () => {}; + setProjects.mutateAsync.mockImplementationOnce( + () => + new Promise((resolve) => { + answer = resolve; + }), + ); + await user.click(screen.getByRole('combobox')); + await user.click(screen.getByRole('option', { name: /Getting started/ })); + await user.keyboard('{Escape}'); + await user.click(saveButton()); + await waitFor(() => + expect(setProjects.mutateAsync).toHaveBeenLastCalledWith( + expect.objectContaining({ projectIds: ['proj_1', 'proj_2'] }), + ), + ); + return () => answer(); + } + + // The picker stays editable while a save is out (#4321 review B2): a + // project dropped after Save stays dropped, whichever of the save's set + // and its answer comes first. + it.each([ + ['before the save answers', true], + ['after the save answers', false], + ])( + 'keeps a project dropped during the save when its set lands %s', + async (_when, setFirst) => { + boundData = ['proj_1']; + const { user, rerender } = render(section()); + const answer = await addAndSaveDeferred(user); + await user.click( + screen.getByRole('button', { name: 'Remove Getting started' }), + ); + if (setFirst) { + boundData = ['proj_1', 'proj_2']; + rerender(section()); + } + await act(async () => { + answer(); + }); + if (!setFirst) { + boundData = ['proj_1', 'proj_2']; + rerender(section()); + } + + expect(selectedProjects()).toEqual(['Document desk']); + expect( + screen.getByRole('button', { name: 'common.actions.discard' }), + ).toBeEnabled(); + }, + ); + + it('settles on its own save when another session’s set lands first', async () => { + boundData = ['proj_1']; + const { user, rerender } = render(section()); + const answer = await addAndSaveDeferred(user); + // Another session unbinds Document desk; then this save, written after + // it, lands with both projects bound. + boundData = []; + rerender(section()); + boundData = ['proj_1', 'proj_2']; + rerender(section()); + await act(async () => { + answer(); + }); + + expect(selectedProjects()).toEqual(['Document desk', 'Getting started']); + expect( + screen.getByRole('button', { name: 'common.actions.discard' }), + ).toBeDisabled(); + }); + it('discards the selection back to the stored set', async () => { boundData = ['proj_1']; const { user } = render( diff --git a/services/platform/app/features/automations/components/project-bindings-section.tsx b/services/platform/app/features/automations/components/project-bindings-section.tsx index b9058ec3eb..3bf85545e9 100644 --- a/services/platform/app/features/automations/components/project-bindings-section.tsx +++ b/services/platform/app/features/automations/components/project-bindings-section.tsx @@ -29,19 +29,23 @@ function sameSelection(a: readonly string[], b: readonly string[]): boolean { } /** - * `incoming` with the author's unsaved edits replayed on it: the projects - * they added since `loaded` stay picked, the ones they removed stay off. + * `incoming` with the author's unsaved edits replayed on it: a project they + * picked or dropped — since `loaded`, or since the save in flight sent + * `sent` — keeps their choice, and every other project follows `incoming`. */ function keepEdits( loaded: readonly string[], current: readonly string[], incoming: readonly string[], + sent: readonly string[] = loaded, ): string[] { - const removed = new Set(loaded.filter((id) => !current.includes(id))); - const added = current.filter( - (id) => !loaded.includes(id) && !incoming.includes(id), - ); - return [...incoming.filter((id) => !removed.has(id)), ...added]; + const edited = (id: string): boolean => + current.includes(id) !== loaded.includes(id) || + current.includes(id) !== sent.includes(id); + return [ + ...incoming.filter((id) => !edited(id) || current.includes(id)), + ...current.filter((id) => edited(id) && !incoming.includes(id)), + ]; } /** @@ -88,15 +92,23 @@ export function ProjectBindingsSection({ // re-render. const storedRef = useRef(stored); storedRef.current = stored; - // The set the selection last loaded: what tells the author's edits apart. + // The set the selection last loaded (or saved), and while a save is out + // the set it sent: what tells the author's edits apart. const loadedRef = useRef([]); + const sentRef = useRef(null); const storedKey = stored.join(','); useEffect(() => { const next = storedRef.current; const loaded = loadedRef.current; + const sent = sentRef.current ?? loaded; loadedRef.current = next; + // A project the save in flight left as loaded follows the set from now + // on, as the selection does. + if (sentRef.current !== null) { + sentRef.current = keepEdits(loaded, sentRef.current, next); + } setSelection((current) => { - const rebased = keepEdits(loaded, current, next); + const rebased = keepEdits(loaded, current, next, sent); return sameSelection(current, rebased) ? current : rebased; }); }, [storedKey]); @@ -114,6 +126,7 @@ export function ProjectBindingsSection({ /** Write the selection as the automation's binding set. */ const persist = async (): Promise => { + sentRef.current = selection; try { await setProjects.mutateAsync({ organizationId, @@ -121,10 +134,16 @@ export function ProjectBindingsSection({ // oxlint-disable-next-line typescript/no-unsafe-type-assertion -- every value came from the projects listing projectIds: selection, }); + // The store holds the selection as sent: a project still as sent + // follows the set it answers with, while one changed during the save + // keeps the change. + loadedRef.current = sentRef.current ?? selection; } catch (error) { // The store's refusal names the problem and the fix; the cluster // raises it as the save's one failure toast. throw new Error(automationErrorMessage(error), { cause: error }); + } finally { + sentRef.current = null; } }; diff --git a/services/platform/app/features/automations/components/trigger-editor.test.tsx b/services/platform/app/features/automations/components/trigger-editor.test.tsx index eacb329f70..0f9cb07195 100644 --- a/services/platform/app/features/automations/components/trigger-editor.test.tsx +++ b/services/platform/app/features/automations/components/trigger-editor.test.tsx @@ -330,6 +330,66 @@ describe('TriggerEditor', () => { expect(discardButton()).toBeDisabled(); expect(savedButton()).toBeDisabled(); }); + + // The fields stay editable while a save is out (#4321 review B1): a + // change made after Save is the author's newest word, even one back to + // the value the form loaded, whichever of the save's row and its answer + // comes first. + it.each([ + ['before the save answers', true], + ['after the save answers', false], + ])( + 'keeps a cron changed back during the save when its row lands %s', + async (_when, rowFirst) => { + let answer: (value: object) => void = () => {}; + mockSetTrigger.mockImplementationOnce( + () => + new Promise((resolve) => { + answer = resolve; + }), + ); + const { rerender } = renderTrigger(); + const cron = await editCron(); + await userEvent.click(saveButton()); + await waitFor(() => expect(mockSetTrigger).toHaveBeenCalledTimes(1)); + await userEvent.clear(cron); + await userEvent.paste('0 */6 * * *'); + const saved = { ...SCHEDULE_ROW, cron: '0 9 * * 1' }; + if (rowFirst) rerenderWith(rerender, saved); + await act(async () => { + answer({}); + }); + if (!rowFirst) rerenderWith(rerender, saved); + + expect(cron).toHaveValue('0 */6 * * *'); + expect(discardButton()).toBeEnabled(); + expect(savedButton()).toBeEnabled(); + }, + ); + + it('settles on its own save when another session’s row lands first', async () => { + let answer: (value: object) => void = () => {}; + mockSetTrigger.mockImplementationOnce( + () => + new Promise((resolve) => { + answer = resolve; + }), + ); + const { rerender } = renderTrigger(); + await editCron(); + await userEvent.click(saveButton()); + await waitFor(() => expect(mockSetTrigger).toHaveBeenCalledTimes(1)); + // Another session switches the trigger off; then this save, written + // after it, lands with the switch on again. + rerenderWith(rerender, { ...SCHEDULE_ROW, enabled: false }); + rerenderWith(rerender, { ...SCHEDULE_ROW, cron: '0 9 * * 1' }); + await act(async () => { + answer({}); + }); + + expect(screen.getByRole('switch', { name: 'Enabled' })).toBeChecked(); + expect(discardButton()).toBeDisabled(); + }); }); it('holds Save while the cron cannot be read', async () => { diff --git a/services/platform/app/features/automations/components/trigger-editor.tsx b/services/platform/app/features/automations/components/trigger-editor.tsx index 658af2c69f..34c34a29c4 100644 --- a/services/platform/app/features/automations/components/trigger-editor.tsx +++ b/services/platform/app/features/automations/components/trigger-editor.tsx @@ -89,6 +89,23 @@ function triggerFields(row: StoredTrigger | undefined): TriggerFields { }; } +/** `sent` with each field it left as `loaded` moved on to `incoming`. */ +function keepSentEdits( + loaded: TriggerFields, + sent: TriggerFields, + incoming: TriggerFields, +): TriggerFields { + const pick = (key: K): TriggerFields[K] => + sent[key] === loaded[key] ? incoming[key] : sent[key]; + return { + kind: pick('kind'), + cron: pick('cron'), + timezone: pick('timezone'), + event: pick('event'), + enabled: pick('enabled'), + }; +} + const NO_DIRTY_KEYS: ReadonlySet = new Set(); /** What the General tab's strip lights its unsaved dot for. */ const TRIGGER_DIRTY_KEYS: ReadonlySet = new Set([TRIGGER_DIRTY_KEY]); @@ -176,29 +193,52 @@ export function TriggerEditor({ // The fields as the form last loaded them (or saved them): what tells an // edit from a field the author left alone. const loadedRef = useRef(NO_TRIGGER); + // While a save is out, the fields it sent: a field changed since then is + // an edit too, even one changed back to its loaded value. + const sentRef = useRef(null); /** * Put a binding (none: the empty form) into the fields. With `keepEdits`, - * a field the author changed since the form last loaded keeps the edit and - * only the fields left alone take the binding's values: a row another - * session saved must not erase a draft in progress. + * a field the author changed — since the form last loaded, or since the + * save in flight sent it — keeps the edit, and only the fields left alone + * take the binding's values: a row another session saved, or a save's + * own row, must not erase a draft in progress. */ const applyStored = useCallback( (row: StoredTrigger | undefined, keepEdits = false) => { const loaded = loadedRef.current; + const sent = sentRef.current ?? loaded; const next = triggerFields(row); loadedRef.current = next; - const take = (current: T, wasLoaded: unknown, incoming: T): T => - keepEdits && current !== wasLoaded ? current : incoming; + // A field the save in flight left as loaded follows the row from now + // on, as the form does. + if (sentRef.current !== null) { + sentRef.current = keepSentEdits(loaded, sentRef.current, next); + } + const take = ( + current: T, + wasLoaded: unknown, + wasSent: unknown, + incoming: T, + ): T => + keepEdits && (current !== wasLoaded || current !== wasSent) + ? current + : incoming; setAdding(false); const nextKind = next.kind; if (isTriggerKind(nextKind)) { - setKind((current) => take(current, loaded.kind, nextKind)); + setKind((current) => take(current, loaded.kind, sent.kind, nextKind)); } - setCron((current) => take(current, loaded.cron, next.cron)); - setTimezone((current) => take(current, loaded.timezone, next.timezone)); - setEventName((current) => take(current, loaded.event, next.event)); - setEnabled((current) => take(current, loaded.enabled, next.enabled)); + setCron((current) => take(current, loaded.cron, sent.cron, next.cron)); + setTimezone((current) => + take(current, loaded.timezone, sent.timezone, next.timezone), + ); + setEventName((current) => + take(current, loaded.event, sent.event, next.event), + ); + setEnabled((current) => + take(current, loaded.enabled, sent.enabled, next.enabled), + ); }, [], ); @@ -294,6 +334,7 @@ export function TriggerEditor({ enabled, }; const storedFieldsBefore = storedFieldsRef.current; + sentRef.current = sent; try { const result = await setTrigger.mutateAsync({ organizationId, @@ -310,7 +351,7 @@ export function TriggerEditor({ // The store holds the form as sent: a field still as sent takes the // row it answers with, which drops what the kind leaves out (a // webhook keeps no cron), while an edit made during the save stays. - loadedRef.current = sent; + loadedRef.current = sentRef.current ?? sent; if (result.token !== undefined) setMintedToken(result.token); // The server names the live URL this bind stopped answering on — say // so, since nothing on the page shows the old URL any more. @@ -318,6 +359,7 @@ export function TriggerEditor({ toast({ title: t('trigger.revokedToast') }); } } finally { + sentRef.current = null; // A row that arrived while the save was out loaded against the form // as it stood before; load it again against what the store holds now. if (