From 318ce894be58cab1f3db026963290543ae0de5df Mon Sep 17 00:00:00 2001 From: Siyang Date: Thu, 21 May 2026 10:13:11 +1200 Subject: [PATCH 01/14] feat(NES-1637): add InCollectionContext for Copy to collection gate --- .../InCollectionContext.spec.tsx | 43 +++++++++++++++++++ .../InCollectionContext.ts | 29 +++++++++++++ .../InCollectionContext/index.ts | 1 + .../TemplateGalleryPageList.tsx | 23 +++++----- 4 files changed, 86 insertions(+), 10 deletions(-) create mode 100644 apps/journeys-admin/src/components/TemplateGalleryPageList/InCollectionContext/InCollectionContext.spec.tsx create mode 100644 apps/journeys-admin/src/components/TemplateGalleryPageList/InCollectionContext/InCollectionContext.ts create mode 100644 apps/journeys-admin/src/components/TemplateGalleryPageList/InCollectionContext/index.ts diff --git a/apps/journeys-admin/src/components/TemplateGalleryPageList/InCollectionContext/InCollectionContext.spec.tsx b/apps/journeys-admin/src/components/TemplateGalleryPageList/InCollectionContext/InCollectionContext.spec.tsx new file mode 100644 index 00000000000..da4e69af901 --- /dev/null +++ b/apps/journeys-admin/src/components/TemplateGalleryPageList/InCollectionContext/InCollectionContext.spec.tsx @@ -0,0 +1,43 @@ +import { render, screen } from '@testing-library/react' +import { ReactElement } from 'react' + +import { InCollectionContext, useInCollection } from './InCollectionContext' + +function Probe({ testId }: { testId: string }): ReactElement { + const inCollection = useInCollection() + return
{inCollection ? 'true' : 'false'}
+} + +describe('InCollectionContext', () => { + describe('useInCollection', () => { + it('returns true when consumer is rendered inside a provider', () => { + render( + + + + ) + + expect(screen.getByTestId('probe')).toHaveTextContent('true') + }) + + it('returns false when consumer is rendered outside any provider', () => { + render() + + expect(screen.getByTestId('probe')).toHaveTextContent('false') + }) + + it('does not throw and resolves to the closest provider value when nested', () => { + render( + + + + + + + ) + + expect(screen.getByTestId('outer')).toHaveTextContent('false') + expect(screen.getByTestId('inner')).toHaveTextContent('true') + }) + }) +}) diff --git a/apps/journeys-admin/src/components/TemplateGalleryPageList/InCollectionContext/InCollectionContext.ts b/apps/journeys-admin/src/components/TemplateGalleryPageList/InCollectionContext/InCollectionContext.ts new file mode 100644 index 00000000000..5da0c2374f4 --- /dev/null +++ b/apps/journeys-admin/src/components/TemplateGalleryPageList/InCollectionContext/InCollectionContext.ts @@ -0,0 +1,29 @@ +import { createContext, useContext } from 'react' + +/** + * Lets descendant components (specifically `JourneyCard` and its menu) tell + * when they are rendered inside a Collection card on the template gallery + * page, so the menu can gate the "Copy to collection…" action to that + * subtree only. + * + * `JourneyCard` is reused across at least five contexts + * (`ActiveJourneyList`, `ArchivedJourneyList`, `TrashedJourneyList`, + * All-Templates, Collections); a context-based gate is purely additive + * (no prop-signature changes through `JourneyCard → JourneyCardMenu → + * DefaultMenu`) and read at the menu-item site via `useInCollection()`. + * + * The context is intentionally opt-in: the provider is only mounted by + * `TemplateGalleryPageList` around the Collection-grid path, leaving the + * All-Templates path and every other consumer of `JourneyCard` + * uncovered. `useInCollection()` returns `false` when no provider is in + * the tree. + * + * `useGalleryDialogLock` cannot be reused for this purpose because the + * existing lock provider wraps both the Collections grid AND the + * All-Templates grid, so it cannot distinguish the two contexts. + */ +export const InCollectionContext = createContext(false) + +export function useInCollection(): boolean { + return useContext(InCollectionContext) +} diff --git a/apps/journeys-admin/src/components/TemplateGalleryPageList/InCollectionContext/index.ts b/apps/journeys-admin/src/components/TemplateGalleryPageList/InCollectionContext/index.ts new file mode 100644 index 00000000000..11d48a2b3f1 --- /dev/null +++ b/apps/journeys-admin/src/components/TemplateGalleryPageList/InCollectionContext/index.ts @@ -0,0 +1 @@ +export { InCollectionContext, useInCollection } from './InCollectionContext' diff --git a/apps/journeys-admin/src/components/TemplateGalleryPageList/TemplateGalleryPageList.tsx b/apps/journeys-admin/src/components/TemplateGalleryPageList/TemplateGalleryPageList.tsx index ee1a1a52a72..4c34e1a3327 100644 --- a/apps/journeys-admin/src/components/TemplateGalleryPageList/TemplateGalleryPageList.tsx +++ b/apps/journeys-admin/src/components/TemplateGalleryPageList/TemplateGalleryPageList.tsx @@ -57,6 +57,7 @@ import { GalleryDialogLockContext, GalleryDialogLockContextValue } from './GalleryDialogLockContext' +import { InCollectionContext } from './InCollectionContext' import { useCollectionMutations } from './useCollectionMutations' import { useDragEndHandler } from './useDragEndHandler' @@ -531,16 +532,18 @@ export function TemplateGalleryPageList({ : null } > - + + + ))} From 4875af542bc4dc416fd8432615e75588003a2b3e Mon Sep 17 00:00:00 2001 From: Siyang Date: Thu, 21 May 2026 10:25:56 +1200 Subject: [PATCH 02/14] feat(NES-1637): add CopyToCollectionDialog with collection picker + translation --- .../CopyToCollectionDialog.spec.tsx | 483 ++++++++++++++++++ .../CopyToCollectionDialog.tsx | 376 ++++++++++++++ .../CopyToCollectionDialog/index.ts | 2 + 3 files changed, 861 insertions(+) create mode 100644 apps/journeys-admin/src/components/TemplateGalleryPageList/CopyToCollectionDialog/CopyToCollectionDialog.spec.tsx create mode 100644 apps/journeys-admin/src/components/TemplateGalleryPageList/CopyToCollectionDialog/CopyToCollectionDialog.tsx create mode 100644 apps/journeys-admin/src/components/TemplateGalleryPageList/CopyToCollectionDialog/index.ts diff --git a/apps/journeys-admin/src/components/TemplateGalleryPageList/CopyToCollectionDialog/CopyToCollectionDialog.spec.tsx b/apps/journeys-admin/src/components/TemplateGalleryPageList/CopyToCollectionDialog/CopyToCollectionDialog.spec.tsx new file mode 100644 index 00000000000..bea40cf13e7 --- /dev/null +++ b/apps/journeys-admin/src/components/TemplateGalleryPageList/CopyToCollectionDialog/CopyToCollectionDialog.spec.tsx @@ -0,0 +1,483 @@ +import { MockedProvider, MockedResponse } from '@apollo/client/testing' +import { fireEvent, render, screen, waitFor } from '@testing-library/react' +import { SnackbarProvider } from 'notistack' + +import { useTeam } from '@core/journeys/ui/TeamProvider' +import { GET_LANGUAGES } from '@core/journeys/ui/useLanguagesQuery' +import { getLanguagesMock } from '@core/journeys/ui/useLanguagesQuery/useLanguagesQuery.mock' + +import { GetTemplateGalleryPages_templateGalleryPages as Page } from '../../../../__generated__/GetTemplateGalleryPages' +import { TemplateGalleryPageStatus } from '../../../../__generated__/globalTypes' +import { GET_TEMPLATE_GALLERY_PAGES } from '../../../libs/useTemplateGalleryPagesQuery' + +import { CopyToCollectionDialog } from './CopyToCollectionDialog' + +jest.mock('@core/journeys/ui/TeamProvider', () => ({ + __esModule: true, + useTeam: jest.fn() +})) + +const mockUseTeam = useTeam as jest.MockedFunction + +const TEAM_ID = 'team-1' + +function makePage(overrides: Partial = {}): Page { + return { + __typename: 'TemplateGalleryPage', + id: 'page-1', + title: 'Featured Templates', + description: '', + slug: 'featured-templates', + status: TemplateGalleryPageStatus.draft, + creatorName: '', + creatorImageSrc: null, + creatorImageAlt: null, + mediaUrl: null, + publishedAt: null, + createdAt: '2026-05-01T00:00:00.000Z', + updatedAt: '2026-05-01T00:00:00.000Z', + templates: [], + ...overrides + } +} + +function makePagesMock(pages: readonly Page[]): MockedResponse { + return { + request: { + query: GET_TEMPLATE_GALLERY_PAGES, + variables: { teamId: TEAM_ID } + }, + result: { data: { templateGalleryPages: [...pages] } } + } +} + +const languagesMock: MockedResponse = { + request: { + query: GET_LANGUAGES, + variables: { + languageId: '529', + where: { + ids: [ + '529', + '4415', + '1106', + '4451', + '496', + '20526', + '584', + '21028', + '20615', + '3934', + '22658', + '7083', + '16639', + '3887', + '13169', + '6464', + '12876', + '53441', + '1942', + '5541', + '6788', + '3804', + '1370', + '4791', + '139081', + '1964', + '21754', + '21753', + '1109', + '4432', + '4454', + '1269', + '4601', + '4820', + '483', + '1341', + '6930', + '1107', + '371', + '7519', + '7698', + '1927', + '18259', + '1254', + '10393', + '5546', + '13172', + '5545', + '1112', + '23178', + '4823', + '12551', + '24309', + '5871', + '407', + '3888', + '1308' + ] + } + } + }, + result: getLanguagesMock.result +} + +function setActiveTeam(id: string | null): void { + if (id == null) { + mockUseTeam.mockReturnValue({ + activeTeam: null, + setActiveTeam: jest.fn(), + refetch: jest.fn(), + query: {} as any + }) + return + } + mockUseTeam.mockReturnValue({ + activeTeam: { + __typename: 'Team', + id, + title: 'Active Team', + publicTitle: null, + userTeams: [], + customDomains: [] + } as any, + setActiveTeam: jest.fn(), + refetch: jest.fn(), + query: {} as any + }) +} + +interface RenderOptions { + mocks?: MockedResponse[] + props?: Partial> +} + +function renderDialog({ + mocks = [], + props = {} +}: RenderOptions = {}): ReturnType { + const baseProps: React.ComponentProps = { + open: true, + onClose: jest.fn(), + onSubmit: jest.fn(), + ...props + } + return render( + + + + + + ) +} + +describe('CopyToCollectionDialog', () => { + beforeEach(() => { + jest.clearAllMocks() + setActiveTeam(TEAM_ID) + }) + + it('renders the dropdown with N collections when open and the query returns N pages', async () => { + const pagesMock = makePagesMock([ + makePage({ id: 'page-1', title: 'Featured Templates' }), + makePage({ id: 'page-2', title: 'Easter Picks' }) + ]) + renderDialog({ mocks: [pagesMock, languagesMock] }) + + fireEvent.mouseDown( + screen.getByRole('combobox', { name: 'Select Collection' }) + ) + await waitFor(() => { + expect( + screen.getByRole('option', { name: 'Easter Picks' }) + ).toBeInTheDocument() + }) + expect( + screen.getByRole('option', { name: 'Featured Templates' }) + ).toBeInTheDocument() + }) + + it('renders nothing visible when open is false', () => { + const pagesMock = makePagesMock([makePage()]) + renderDialog({ + mocks: [pagesMock, languagesMock], + props: { open: false } + }) + expect(screen.queryByTestId('CopyToCollectionDialog')).not.toBeInTheDocument() + }) + + it('selecting a collection enables submit and submits with the chosen id', async () => { + const onSubmit = jest.fn() + const pagesMock = makePagesMock([ + makePage({ id: 'page-1', title: 'Featured Templates' }) + ]) + renderDialog({ + mocks: [pagesMock, languagesMock], + props: { onSubmit } + }) + + // Wait for query to resolve. + fireEvent.mouseDown( + screen.getByRole('combobox', { name: 'Select Collection' }) + ) + const option = await screen.findByRole('option', { + name: 'Featured Templates' + }) + fireEvent.click(option) + + const submitButton = screen.getByRole('button', { name: 'Copy' }) + await waitFor(() => expect(submitButton).not.toBeDisabled()) + fireEvent.click(submitButton) + + await waitFor(() => expect(onSubmit).toHaveBeenCalledTimes(1)) + expect(onSubmit).toHaveBeenCalledWith({ + collectionId: 'page-1', + language: undefined, + showTranslation: false + }) + }) + + it('toggling translation reveals the language picker and submits with language and showTranslation true', async () => { + const onSubmit = jest.fn() + const pagesMock = makePagesMock([ + makePage({ id: 'page-1', title: 'Featured Templates' }) + ]) + renderDialog({ + mocks: [pagesMock, languagesMock], + props: { onSubmit } + }) + + fireEvent.mouseDown( + screen.getByRole('combobox', { name: 'Select Collection' }) + ) + fireEvent.click( + await screen.findByRole('option', { name: 'Featured Templates' }) + ) + + fireEvent.click( + screen.getByRole('checkbox', { + name: 'Translate the copy to another language' + }) + ) + + await waitFor(() => { + expect(screen.getByTestId('LanguageAutocomplete')).not.toHaveAttribute( + 'aria-disabled', + 'true' + ) + }) + + fireEvent.focus(screen.getByTestId('LanguageAutocomplete')) + fireEvent.keyDown(screen.getByTestId('LanguageAutocomplete'), { + key: 'ArrowDown' + }) + fireEvent.click(screen.getByRole('option', { name: 'French Français' })) + + const submitButton = screen.getByRole('button', { name: 'Copy' }) + await waitFor(() => expect(submitButton).not.toBeDisabled()) + fireEvent.click(submitButton) + + await waitFor(() => expect(onSubmit).toHaveBeenCalledTimes(1)) + const args = onSubmit.mock.calls[0][0] + expect(args.collectionId).toBe('page-1') + expect(args.showTranslation).toBe(true) + expect(args.language).toMatchObject({ id: '496' }) + }) + + it('shows a disabled "No collections available" row and disabled submit when the query returns zero pages', async () => { + const pagesMock = makePagesMock([]) + renderDialog({ mocks: [pagesMock, languagesMock] }) + + // Wait for the query to settle so the empty-state row renders. + fireEvent.mouseDown( + screen.getByRole('combobox', { name: 'Select Collection' }) + ) + const emptyRow = await screen.findByTestId( + 'CopyToCollectionDialogEmptyRow' + ) + expect(emptyRow).toHaveTextContent('No collections available') + expect(emptyRow).toHaveAttribute('aria-disabled', 'true') + + // Close the popover so the submit button is back in the a11y tree. + fireEvent.keyDown(emptyRow, { key: 'Escape' }) + await waitFor(() => + expect(screen.getByText('Copy').closest('button')).toBeDisabled() + ) + }) + + it('shows a disabled "Loading…" row when active team is null (query skipped)', async () => { + setActiveTeam(null) + renderDialog({ mocks: [languagesMock] }) + + fireEvent.mouseDown( + screen.getByRole('combobox', { name: 'Select Collection' }) + ) + const loadingRow = await screen.findByTestId( + 'CopyToCollectionDialogLoadingRow' + ) + expect(loadingRow).toHaveTextContent('Loading…') + expect(loadingRow).toHaveAttribute('aria-disabled', 'true') + + fireEvent.keyDown(loadingRow, { key: 'Escape' }) + await waitFor(() => + expect(screen.getByText('Copy').closest('button')).toBeDisabled() + ) + }) + + it('shows a disabled "Loading…" row while the gallery-pages query is loading', () => { + // No pages mock supplied — the query stays in a loading state. + renderDialog({ mocks: [languagesMock] }) + + fireEvent.mouseDown( + screen.getByRole('combobox', { name: 'Select Collection' }) + ) + expect( + screen.getByTestId('CopyToCollectionDialogLoadingRow') + ).toBeInTheDocument() + + // Close the popover and confirm submit is disabled. + fireEvent.keyDown(screen.getByTestId('CopyToCollectionDialogLoadingRow'), { + key: 'Escape' + }) + expect(screen.getByText('Copy').closest('button')).toBeDisabled() + }) + + it('includes the source collection in the dropdown options (no filtering)', async () => { + const pagesMock = makePagesMock([ + makePage({ id: 'source-id', title: 'Source Collection' }), + makePage({ id: 'page-2', title: 'Another Collection' }) + ]) + renderDialog({ mocks: [pagesMock, languagesMock] }) + + fireEvent.mouseDown( + screen.getByRole('combobox', { name: 'Select Collection' }) + ) + expect( + await screen.findByRole('option', { name: 'Source Collection' }) + ).toBeInTheDocument() + }) + + it('keeps submit disabled when showTranslation is true but no language is selected', async () => { + const pagesMock = makePagesMock([ + makePage({ id: 'page-1', title: 'Featured Templates' }) + ]) + renderDialog({ mocks: [pagesMock, languagesMock] }) + + fireEvent.mouseDown( + screen.getByRole('combobox', { name: 'Select Collection' }) + ) + fireEvent.click( + await screen.findByRole('option', { name: 'Featured Templates' }) + ) + + fireEvent.click( + screen.getByRole('checkbox', { + name: 'Translate the copy to another language' + }) + ) + + await waitFor(() => + expect(screen.getByRole('button', { name: 'Copy' })).toBeDisabled() + ) + }) + + it('rapid double-click on submit fires onSubmit only once (loading prop disables button)', async () => { + const onSubmit = jest.fn() + const pagesMock = makePagesMock([ + makePage({ id: 'page-1', title: 'Featured Templates' }) + ]) + const { rerender } = renderDialog({ + mocks: [pagesMock, languagesMock], + props: { onSubmit } + }) + + fireEvent.mouseDown( + screen.getByRole('combobox', { name: 'Select Collection' }) + ) + fireEvent.click( + await screen.findByRole('option', { name: 'Featured Templates' }) + ) + + const submitButton = screen.getByRole('button', { name: 'Copy' }) + fireEvent.click(submitButton) + await waitFor(() => expect(onSubmit).toHaveBeenCalledTimes(1)) + + // Parent flips loading → submit/close suppressed in the wrapper. + rerender( + + + + + + ) + + // While loading, the wrapper hides the submit button entirely. + expect(screen.queryByRole('button', { name: 'Copy' })).not.toBeInTheDocument() + expect(onSubmit).toHaveBeenCalledTimes(1) + }) + + it('renders the error state in an aria-live region with a single Done button bound to onClose', () => { + const onClose = jest.fn() + renderDialog({ + props: { + onClose, + errorMessage: 'Failed to copy the journey. Please try again.' + } + }) + + const status = screen.getByTestId('CopyToCollectionDialogStatus') + expect(status).toHaveAttribute('role', 'status') + expect(status).toHaveAttribute('aria-live', 'polite') + expect(status).toHaveTextContent( + 'Failed to copy the journey. Please try again.' + ) + + // The terminal-state Dialog should only show a Done button, no Cancel. + expect(screen.queryByRole('button', { name: 'Cancel' })).not.toBeInTheDocument() + expect(screen.queryByRole('button', { name: 'Copy' })).not.toBeInTheDocument() + + const doneButton = screen.getByRole('button', { name: 'Done' }) + fireEvent.click(doneButton) + expect(onClose).toHaveBeenCalledTimes(1) + }) + + it('renders the done state with success copy echoing the selected collection title', () => { + const onClose = jest.fn() + renderDialog({ + props: { + onClose, + done: true, + selectedCollectionTitle: 'Featured Templates' + } + }) + + const status = screen.getByTestId('CopyToCollectionDialogStatus') + expect(status).toHaveAttribute('role', 'status') + expect(status).toHaveAttribute('aria-live', 'polite') + expect(status).toHaveTextContent('Copied to Featured Templates.') + + expect(screen.queryByRole('button', { name: 'Copy' })).not.toBeInTheDocument() + expect(screen.queryByRole('button', { name: 'Cancel' })).not.toBeInTheDocument() + + fireEvent.click(screen.getByRole('button', { name: 'Done' })) + expect(onClose).toHaveBeenCalledTimes(1) + }) + + it('when loading is true the submit/cancel actions are not rendered and onSubmit is not called', () => { + const onSubmit = jest.fn() + const onClose = jest.fn() + renderDialog({ + props: { onSubmit, onClose, loading: true } + }) + + expect(screen.queryByRole('button', { name: 'Copy' })).not.toBeInTheDocument() + expect(screen.queryByRole('button', { name: 'Cancel' })).not.toBeInTheDocument() + expect(onSubmit).not.toHaveBeenCalled() + expect(onClose).not.toHaveBeenCalled() + }) +}) diff --git a/apps/journeys-admin/src/components/TemplateGalleryPageList/CopyToCollectionDialog/CopyToCollectionDialog.tsx b/apps/journeys-admin/src/components/TemplateGalleryPageList/CopyToCollectionDialog/CopyToCollectionDialog.tsx new file mode 100644 index 00000000000..203917feaca --- /dev/null +++ b/apps/journeys-admin/src/components/TemplateGalleryPageList/CopyToCollectionDialog/CopyToCollectionDialog.tsx @@ -0,0 +1,376 @@ +import Box from '@mui/material/Box' +import Button from '@mui/material/Button' +import FormControl from '@mui/material/FormControl' +import FormControlLabel from '@mui/material/FormControlLabel' +import MenuItem from '@mui/material/MenuItem' +import Stack from '@mui/material/Stack' +import Switch from '@mui/material/Switch' +import TextField from '@mui/material/TextField' +import Typography from '@mui/material/Typography' +import { Formik, FormikHelpers } from 'formik' +import sortBy from 'lodash/sortBy' +import { useTranslation } from 'next-i18next/pages' +import { ReactElement } from 'react' +import { boolean, object, string } from 'yup' + +import { useTeam } from '@core/journeys/ui/TeamProvider' +import { TranslationDialogWrapper } from '@core/journeys/ui/TranslationDialogWrapper' +import { SUPPORTED_LANGUAGE_IDS } from '@core/journeys/ui/useJourneyAiTranslateSubscription/supportedLanguages' +import { useLanguagesQuery } from '@core/journeys/ui/useLanguagesQuery' +import { Dialog } from '@core/shared/ui/Dialog' +import ChevronDownIcon from '@core/shared/ui/icons/ChevronDown' +import { LanguageAutocomplete } from '@core/shared/ui/LanguageAutocomplete' + +import { useTemplateGalleryPagesQuery } from '../../../libs/useTemplateGalleryPagesQuery' + +interface JourneyLanguage { + id: string + localName?: string + nativeName?: string +} + +interface FormValues { + collectionSelect: string + languageSelect?: JourneyLanguage + showTranslation: boolean +} + +export interface CopyToCollectionDialogProps { + open: boolean + loading?: boolean + errorMessage?: string + done?: boolean + selectedCollectionTitle?: string + journeyTitle?: string + onClose: () => void + onSubmit: (values: { + collectionId: string + language?: JourneyLanguage + showTranslation: boolean + }) => void + translationProgress?: { + progress: number + message: string + } + isTranslating?: boolean +} + +/** + * CopyToCollectionDialog provides a dialog for copying a journey into a + * Collection (TemplateGalleryPage) in the active team, optionally translating + * the copy as part of the action. + * + * Visually modeled on `CopyToTeamDialog` but admin-app specific: + * - Replaces the team picker with a collection picker sourced from + * `useTemplateGalleryPagesQuery({ teamId: activeTeam?.id ?? '' }, { + * skip: activeTeam?.id == null })`. + * - Does NOT set `enableReinitialize` on Formik (see NES-1543 Pattern 3). + * - Surfaces three terminal UI states driven purely by props: loading, + * error (`errorMessage`), and done (`done === true`). Orchestration of + * the underlying mutations lives in the menu item that mounts this + * dialog. + */ +export function CopyToCollectionDialog({ + open, + loading, + errorMessage, + done, + selectedCollectionTitle, + journeyTitle, + onClose, + onSubmit, + translationProgress, + isTranslating = false +}: CopyToCollectionDialogProps): ReactElement { + const { t } = useTranslation('apps-journeys-admin') + const { activeTeam } = useTeam() + const teamId = activeTeam?.id + const skipQuery = teamId == null + const { + data: pagesData, + loading: pagesLoading + } = useTemplateGalleryPagesQuery( + { teamId: teamId ?? '' }, + { skip: skipQuery } + ) + + const { data: languagesData, loading: languagesLoading } = useLanguagesQuery({ + languageId: '529', + where: { + ids: [...SUPPORTED_LANGUAGE_IDS] + } + }) + + const pages = pagesData?.templateGalleryPages ?? [] + const sortedPages = sortBy(pages, 'title') + const optionsUnavailable = skipQuery || pagesLoading || pages.length === 0 + + const dialogTitleText = + journeyTitle != null && journeyTitle !== '' + ? t('Copy "{{title}}" to collection', { title: journeyTitle }) + : t('Copy to collection') + + // Terminal states (error / done) replace the form body entirely and + // present a single "Done" button bound to `onClose`. Use the shared + // `Dialog` directly here so we can fully control the action area — + // `TranslationDialogWrapper`'s action area always renders Cancel + + // Submit when not loading. + if (errorMessage != null || done === true) { + const statusMessage = + errorMessage != null + ? errorMessage + : t('Copied to {{title}}.', { + title: selectedCollectionTitle ?? '' + }) + + const handleDoneClick = (): void => { + onClose() + } + + return ( + + {t('Done')} + + } + > + + + {statusMessage} + + + + ) + } + + const baseLanguageShape = { + id: string(), + localName: string().optional(), + nativeName: string().optional() + } + + const copyToSchema = object({ + collectionSelect: string().required(t('Please select a collection')), + showTranslation: boolean().required(), + languageSelect: object(baseLanguageShape) + .nullable() + .when('showTranslation', { + is: true, + then: (schema) => + schema.required(t('Please select a language')).shape({ + id: string().required(t('Please select a language')) + }), + otherwise: (schema) => schema.nullable().optional() + }) + }) + + async function handleFormikSubmit( + values: FormValues, + { resetForm }: FormikHelpers + ): Promise { + onSubmit({ + collectionId: values.collectionSelect, + language: values.showTranslation ? values.languageSelect : undefined, + showTranslation: values.showTranslation + }) + resetForm({ values }) + } + + return ( + + initialValues={{ + collectionSelect: '', + languageSelect: undefined, + showTranslation: false + }} + onSubmit={handleFormikSubmit} + validationSchema={copyToSchema} + > + {({ + values, + errors, + handleChange, + handleSubmit, + isSubmitting, + isValid, + setFieldValue, + touched, + setTouched, + resetForm + }): ReactElement => { + const submitDisabled = + loading === true || + isSubmitting || + optionsUnavailable || + !isValid || + values.collectionSelect === '' + + const handleFormSubmit = async (): Promise => { + await setTouched({ + collectionSelect: true, + ...(values.showTranslation ? { languageSelect: true } : {}) + }) + handleSubmit() + } + + const handleDialogClose = ( + _?: object, + reason?: 'backdropClick' | 'escapeKeyDown' + ): void => { + if ( + (loading === true || isTranslating) && + (reason === 'backdropClick' || reason === 'escapeKeyDown') + ) + return + onClose() + resetForm() + } + + return ( + + + + { + handleChange(e) + }} + slotProps={{ + select: { + IconComponent: ChevronDownIcon + } + }} + sx={{ + '& >.MuiFormHelperText-contained': { + ml: 0 + } + }} + > + {(() => { + if (skipQuery || pagesLoading) { + return ( + + {t('Loading…')} + + ) + } + if (sortedPages.length === 0) { + return ( + + {t('No collections available')} + + ) + } + return sortedPages.map((page) => ( + + {page.title} + + )) + })()} + + + + { + void setFieldValue('showTranslation', e.target.checked) + }} + inputProps={{ + 'aria-label': t( + 'Translate the copy to another language' + ) + }} + /> + } + label={ + + {t('Translate the copy to another language')} + + } + /> + + {values.showTranslation && ( + { + void setFieldValue('languageSelect', value) + }} + error={ + touched.languageSelect && Boolean(errors.languageSelect) + } + value={values.languageSelect} + /> + )} + + + ) + }} + + ) +} diff --git a/apps/journeys-admin/src/components/TemplateGalleryPageList/CopyToCollectionDialog/index.ts b/apps/journeys-admin/src/components/TemplateGalleryPageList/CopyToCollectionDialog/index.ts new file mode 100644 index 00000000000..1b5eeb9d987 --- /dev/null +++ b/apps/journeys-admin/src/components/TemplateGalleryPageList/CopyToCollectionDialog/index.ts @@ -0,0 +1,2 @@ +export { CopyToCollectionDialog } from './CopyToCollectionDialog' +export type { CopyToCollectionDialogProps } from './CopyToCollectionDialog' From a3876fc924e3c484f597d280147c84a076172100 Mon Sep 17 00:00:00 2001 From: Siyang Date: Thu, 21 May 2026 10:39:21 +1200 Subject: [PATCH 03/14] feat(NES-1637): orchestrate duplicate, translate, and assign in CopyToCollectionMenuItem --- .../CopyToCollectionMenuItem.spec.tsx | 484 ++++++++++++++++++ .../CopyToCollectionMenuItem.tsx | 263 ++++++++++ .../CopyToCollectionMenuItem/index.ts | 2 + 3 files changed, 749 insertions(+) create mode 100644 apps/journeys-admin/src/components/TemplateGalleryPageList/CopyToCollectionMenuItem/CopyToCollectionMenuItem.spec.tsx create mode 100644 apps/journeys-admin/src/components/TemplateGalleryPageList/CopyToCollectionMenuItem/CopyToCollectionMenuItem.tsx create mode 100644 apps/journeys-admin/src/components/TemplateGalleryPageList/CopyToCollectionMenuItem/index.ts diff --git a/apps/journeys-admin/src/components/TemplateGalleryPageList/CopyToCollectionMenuItem/CopyToCollectionMenuItem.spec.tsx b/apps/journeys-admin/src/components/TemplateGalleryPageList/CopyToCollectionMenuItem/CopyToCollectionMenuItem.spec.tsx new file mode 100644 index 00000000000..5b925da4bf4 --- /dev/null +++ b/apps/journeys-admin/src/components/TemplateGalleryPageList/CopyToCollectionMenuItem/CopyToCollectionMenuItem.spec.tsx @@ -0,0 +1,484 @@ +import { MockedProvider } from '@apollo/client/testing' +import { + act, + fireEvent, + render, + screen, + waitFor +} from '@testing-library/react' +import { SnackbarProvider } from 'notistack' + +import { useJourneyAiTranslateSubscription } from '@core/journeys/ui/useJourneyAiTranslateSubscription' +import { useJourneyDuplicateMutation } from '@core/journeys/ui/useJourneyDuplicateMutation' + +import { GetAdminJourneys_journeys as Journey } from '../../../../__generated__/GetAdminJourneys' +import { useTemplateGalleryPageAssignJourneyMutation } from '../../../libs/useTemplateGalleryPageAssignJourneyMutation' + +import { CopyToCollectionMenuItem } from './CopyToCollectionMenuItem' + +jest.mock('@core/journeys/ui/useJourneyDuplicateMutation', () => ({ + useJourneyDuplicateMutation: jest.fn() +})) + +jest.mock('@core/journeys/ui/useJourneyAiTranslateSubscription', () => ({ + useJourneyAiTranslateSubscription: jest.fn() +})) + +jest.mock( + '../../../libs/useTemplateGalleryPageAssignJourneyMutation', + () => ({ + useTemplateGalleryPageAssignJourneyMutation: jest.fn() + }) +) + +const mockRefetchQueries = jest.fn(() => Promise.resolve([])) + +jest.mock('@apollo/client', () => { + const actual = jest.requireActual('@apollo/client') + return { + ...actual, + useApolloClient: () => ({ + refetchQueries: mockRefetchQueries + }) + } +}) + +const mockActiveTeam: { id: string; title: string } | null = { + id: 'team-1', + title: 'Team 1' +} +const mockUseTeam = jest.fn< + { activeTeam: { id: string; title: string } | null }, + [] +>(() => ({ activeTeam: mockActiveTeam })) + +jest.mock('@core/journeys/ui/TeamProvider', () => ({ + useTeam: () => mockUseTeam() +})) + +jest.mock('./CopyToCollectionDialog.stub', () => ({}), { virtual: true }) + +// Replace the dialog with a controllable stub so the menu-item tests focus +// purely on orchestration. The real dialog is exercised by U2's spec. +jest.mock( + '../CopyToCollectionDialog', + () => ({ + CopyToCollectionDialog: (props: { + open: boolean + loading?: boolean + errorMessage?: string + done?: boolean + selectedCollectionTitle?: string + journeyTitle?: string + onClose: () => void + onSubmit: (values: { + collectionId: string + language?: { id: string } + showTranslation: boolean + }) => void + isTranslating?: boolean + }) => { + if (!props.open) return null + return ( +
+ {String(props.loading ?? false)} + {String(props.done ?? false)} + {props.errorMessage ?? ''} + + {String(props.isTranslating ?? false)} + + + {props.selectedCollectionTitle ?? ''} + + + + +
+ ) + } + }) +) + +type DuplicateFn = jest.Mock< + Promise<{ data?: { journeyDuplicate?: { id: string } | null } }>, + [unknown?] +> +type AssignFn = jest.Mock, [unknown?]> + +const journey: Journey = { + __typename: 'Journey', + id: 'journey-1', + title: 'Journey' +} as unknown as Journey + +let duplicate: DuplicateFn +let assign: AssignFn +// subscription wiring captured per-render +let lastSubscriptionOpts: + | { + variables?: unknown + skip?: boolean + onComplete?: () => void + onError?: (e: Error) => void + } + | undefined + +const subscriptionHookMock = + useJourneyAiTranslateSubscription as jest.MockedFunction< + typeof useJourneyAiTranslateSubscription + > +const duplicateHookMock = useJourneyDuplicateMutation as jest.MockedFunction< + typeof useJourneyDuplicateMutation +> +const assignHookMock = + useTemplateGalleryPageAssignJourneyMutation as jest.MockedFunction< + typeof useTemplateGalleryPageAssignJourneyMutation + > + +function setupMocks(): void { + duplicate = jest.fn(async () => ({ + data: { journeyDuplicate: { id: 'new-journey-id' } } + })) as DuplicateFn + assign = jest.fn(async () => ({})) as AssignFn + + duplicateHookMock.mockReturnValue([duplicate as unknown as never, {} as never]) + assignHookMock.mockReturnValue([assign as unknown as never, {} as never]) + subscriptionHookMock.mockImplementation(((opts: typeof lastSubscriptionOpts) => { + lastSubscriptionOpts = opts + return { data: undefined } as unknown as ReturnType< + typeof useJourneyAiTranslateSubscription + > + }) as unknown as typeof useJourneyAiTranslateSubscription) +} + +function renderItem( + overrides: Partial<{ + handleCloseMenu: jest.Mock + setHasOpenDialog: jest.Mock + handleKeepMounted: jest.Mock + journey: Journey + }> = {} +): { + handleCloseMenu: jest.Mock + setHasOpenDialog: jest.Mock + handleKeepMounted: jest.Mock + unmount: () => void +} { + const handleCloseMenu = overrides.handleCloseMenu ?? jest.fn() + const setHasOpenDialog = overrides.setHasOpenDialog ?? jest.fn() + const handleKeepMounted = overrides.handleKeepMounted ?? jest.fn() + const { unmount } = render( + + + + + + ) + return { handleCloseMenu, setHasOpenDialog, handleKeepMounted, unmount } +} + +describe('CopyToCollectionMenuItem', () => { + beforeEach(() => { + mockRefetchQueries.mockClear() + mockUseTeam.mockReturnValue({ activeTeam: mockActiveTeam }) + lastSubscriptionOpts = undefined + setupMocks() + }) + + it('clicking the menu item opens the dialog and fires setHasOpenDialog/handleKeepMounted/handleCloseMenu', () => { + const { handleCloseMenu, setHasOpenDialog, handleKeepMounted } = renderItem() + + fireEvent.click(screen.getByRole('menuitem')) + + expect(handleKeepMounted).toHaveBeenCalledTimes(1) + expect(handleCloseMenu).toHaveBeenCalledTimes(1) + expect(setHasOpenDialog).toHaveBeenCalledWith(true) + expect(screen.getByTestId('CopyToCollectionDialogStub')).toBeInTheDocument() + }) + + it('clicking the menu item while the dialog is already open is a no-op (still open)', () => { + const { setHasOpenDialog } = renderItem() + + fireEvent.click(screen.getByRole('menuitem')) + expect(setHasOpenDialog).toHaveBeenLastCalledWith(true) + + setHasOpenDialog.mockClear() + // Clicking the menu item again while the dialog is mounted still + // re-fires setHasOpenDialog(true) — but the dialog remains open and + // no pipeline state is touched. The semantic guarantee we test here + // is "still open, no error/done state, no extra mutations." + fireEvent.click(screen.getByRole('menuitem')) + expect(screen.getByTestId('CopyToCollectionDialogStub')).toBeInTheDocument() + expect(duplicate).not.toHaveBeenCalled() + expect(assign).not.toHaveBeenCalled() + }) + + it('happy path (no translation) — runs duplicate then assign and issues GetAdminJourneys refetch', async () => { + renderItem() + + fireEvent.click(screen.getByRole('menuitem')) + fireEvent.click(screen.getByTestId('StubSubmitNoTranslation')) + + await waitFor(() => expect(duplicate).toHaveBeenCalledTimes(1)) + expect(duplicate).toHaveBeenCalledWith({ + variables: { id: 'journey-1', teamId: 'team-1' } + }) + await waitFor(() => expect(assign).toHaveBeenCalledTimes(1)) + expect(assign).toHaveBeenCalledWith({ + variables: { journeyId: 'new-journey-id', pageId: 'collection-1' } + }) + await waitFor(() => + expect(mockRefetchQueries).toHaveBeenCalledWith({ + include: ['GetAdminJourneys'] + }) + ) + await waitFor(() => + expect(screen.getByTestId('DialogDone')).toHaveTextContent('true') + ) + }) + + it('happy path (with translation) — duplicate, subscription onComplete triggers assign, refetch issued', async () => { + renderItem() + + fireEvent.click(screen.getByRole('menuitem')) + fireEvent.click(screen.getByTestId('StubSubmitWithTranslation')) + + await waitFor(() => expect(duplicate).toHaveBeenCalledTimes(1)) + // After duplicate, translation variables must be armed; assign has NOT + // fired yet — it waits for subscription onComplete. + await waitFor(() => + expect(screen.getByTestId('DialogIsTranslating')).toHaveTextContent('true') + ) + expect(assign).not.toHaveBeenCalled() + + // Fire the subscription onComplete callback. + expect(lastSubscriptionOpts?.onComplete).toBeDefined() + act(() => { + lastSubscriptionOpts?.onComplete?.() + }) + + await waitFor(() => expect(assign).toHaveBeenCalledTimes(1)) + expect(assign).toHaveBeenCalledWith({ + variables: { journeyId: 'new-journey-id', pageId: 'collection-1' } + }) + await waitFor(() => + expect(mockRefetchQueries).toHaveBeenCalledWith({ + include: ['GetAdminJourneys'] + }) + ) + await waitFor(() => + expect(screen.getByTestId('DialogDone')).toHaveTextContent('true') + ) + }) + + it('rapid double-click on submit — duplicate is called only once (single-flight)', async () => { + let resolveDuplicate: ( + value: { data?: { journeyDuplicate?: { id: string } | null } } + ) => void = () => {} + duplicate.mockImplementation( + () => + new Promise((resolve) => { + resolveDuplicate = resolve + }) + ) + + renderItem() + fireEvent.click(screen.getByRole('menuitem')) + fireEvent.click(screen.getByTestId('StubSubmitNoTranslation')) + fireEvent.click(screen.getByTestId('StubSubmitNoTranslation')) + + // Only one duplicate call was issued despite two submit clicks. + expect(duplicate).toHaveBeenCalledTimes(1) + + // Resolve so React state settles before the test ends. + await act(async () => { + resolveDuplicate({ data: { journeyDuplicate: { id: 'new-journey-id' } } }) + }) + await waitFor(() => expect(assign).toHaveBeenCalledTimes(1)) + }) + + it('activeTeam null at submit — sets no-active-team error; no mutations, no refetch', async () => { + mockUseTeam.mockReturnValue({ activeTeam: null }) + + renderItem() + fireEvent.click(screen.getByRole('menuitem')) + fireEvent.click(screen.getByTestId('StubSubmitNoTranslation')) + + await waitFor(() => + expect(screen.getByTestId('DialogError')).toHaveTextContent( + 'No active team selected. Please pick a team and try again.' + ) + ) + expect(duplicate).not.toHaveBeenCalled() + expect(assign).not.toHaveBeenCalled() + expect(mockRefetchQueries).not.toHaveBeenCalled() + }) + + it('duplicate fails — error copy set; no assign; no refetch', async () => { + duplicate.mockRejectedValueOnce(new Error('boom')) + + renderItem() + fireEvent.click(screen.getByRole('menuitem')) + fireEvent.click(screen.getByTestId('StubSubmitNoTranslation')) + + await waitFor(() => + expect(screen.getByTestId('DialogError')).toHaveTextContent( + 'Failed to copy the journey. Please try again.' + ) + ) + expect(assign).not.toHaveBeenCalled() + expect(mockRefetchQueries).not.toHaveBeenCalled() + }) + + it('translation fails — error copy set; no assign; refetch IS issued', async () => { + renderItem() + fireEvent.click(screen.getByRole('menuitem')) + fireEvent.click(screen.getByTestId('StubSubmitWithTranslation')) + + await waitFor(() => expect(duplicate).toHaveBeenCalledTimes(1)) + expect(lastSubscriptionOpts?.onError).toBeDefined() + act(() => { + lastSubscriptionOpts?.onError?.(new Error('xlate-fail')) + }) + + await waitFor(() => + expect(screen.getByTestId('DialogError')).toHaveTextContent( + 'An error occurred while translating.' + ) + ) + expect(assign).not.toHaveBeenCalled() + expect(mockRefetchQueries).toHaveBeenCalledWith({ + include: ['GetAdminJourneys'] + }) + }) + + it('assign fails (no translation) — assign-fail copy; refetch IS issued', async () => { + assign.mockRejectedValueOnce(new Error('assign-boom')) + + renderItem() + fireEvent.click(screen.getByRole('menuitem')) + fireEvent.click(screen.getByTestId('StubSubmitNoTranslation')) + + await waitFor(() => + expect(screen.getByTestId('DialogError')).toHaveTextContent( + "Failed to add the copy to the collection. You'll find it in All Templates" + ) + ) + expect(mockRefetchQueries).toHaveBeenCalledWith({ + include: ['GetAdminJourneys'] + }) + }) + + it('assign fails after successful translation — assign-fail copy; refetch issued', async () => { + assign.mockRejectedValueOnce(new Error('assign-boom')) + + renderItem() + fireEvent.click(screen.getByRole('menuitem')) + fireEvent.click(screen.getByTestId('StubSubmitWithTranslation')) + + await waitFor(() => expect(duplicate).toHaveBeenCalledTimes(1)) + act(() => { + lastSubscriptionOpts?.onComplete?.() + }) + + await waitFor(() => + expect(screen.getByTestId('DialogError')).toHaveTextContent( + "Failed to add the copy to the collection. You'll find it in All Templates" + ) + ) + expect(mockRefetchQueries).toHaveBeenCalledWith({ + include: ['GetAdminJourneys'] + }) + }) + + it('Done click — calls setHasOpenDialog(false) and handleCloseMenu', async () => { + const { handleCloseMenu, setHasOpenDialog } = renderItem() + fireEvent.click(screen.getByRole('menuitem')) + fireEvent.click(screen.getByTestId('StubSubmitNoTranslation')) + + await waitFor(() => + expect(screen.getByTestId('DialogDone')).toHaveTextContent('true') + ) + + handleCloseMenu.mockClear() + setHasOpenDialog.mockClear() + + fireEvent.click(screen.getByTestId('StubClose')) + + expect(setHasOpenDialog).toHaveBeenCalledWith(false) + expect(handleCloseMenu).toHaveBeenCalledTimes(1) + await waitFor(() => + expect( + screen.queryByTestId('CopyToCollectionDialogStub') + ).not.toBeInTheDocument() + ) + }) + + it('unmount mid-pipeline — does not throw setState-after-unmount', async () => { + let resolveDuplicate: ( + value: { data?: { journeyDuplicate?: { id: string } | null } } + ) => void = () => {} + duplicate.mockImplementation( + () => + new Promise((resolve) => { + resolveDuplicate = resolve + }) + ) + + const errorSpy = jest.spyOn(console, 'error').mockImplementation(() => {}) + + const { unmount } = renderItem() + fireEvent.click(screen.getByRole('menuitem')) + fireEvent.click(screen.getByTestId('StubSubmitNoTranslation')) + + // Unmount while the duplicate promise is still pending. + unmount() + + await act(async () => { + resolveDuplicate({ data: { journeyDuplicate: { id: 'new-journey-id' } } }) + }) + + // No "setState on unmounted component" warning should have been logged. + const setStateWarnings = errorSpy.mock.calls.filter((args) => + String(args[0] ?? '').includes('unmounted') + ) + expect(setStateWarnings).toHaveLength(0) + errorSpy.mockRestore() + }) +}) diff --git a/apps/journeys-admin/src/components/TemplateGalleryPageList/CopyToCollectionMenuItem/CopyToCollectionMenuItem.tsx b/apps/journeys-admin/src/components/TemplateGalleryPageList/CopyToCollectionMenuItem/CopyToCollectionMenuItem.tsx new file mode 100644 index 00000000000..4109cec75b2 --- /dev/null +++ b/apps/journeys-admin/src/components/TemplateGalleryPageList/CopyToCollectionMenuItem/CopyToCollectionMenuItem.tsx @@ -0,0 +1,263 @@ +import { useApolloClient } from '@apollo/client' +import { useTranslation } from 'next-i18next/pages' +import { ReactElement, useEffect, useRef, useState } from 'react' + +import { useTeam } from '@core/journeys/ui/TeamProvider' +import { useJourneyAiTranslateSubscription } from '@core/journeys/ui/useJourneyAiTranslateSubscription' +import { useJourneyDuplicateMutation } from '@core/journeys/ui/useJourneyDuplicateMutation' +import CopyToIcon from '@core/shared/ui/icons/CopyTo' + +import { GetAdminJourneys_journeys as Journey } from '../../../../__generated__/GetAdminJourneys' +import { useTemplateGalleryPageAssignJourneyMutation } from '../../../libs/useTemplateGalleryPageAssignJourneyMutation' +import { MenuItem } from '../../MenuItem' +import { CopyToCollectionDialog } from '../CopyToCollectionDialog' + +interface JourneyLanguage { + id: string + localName?: string + nativeName?: string +} + +interface TranslationVars { + journeyId: string + name: string + journeyLanguageName: string + textLanguageId: string + textLanguageName: string + userLanguageId?: string + userLanguageName?: string +} + +export interface CopyToCollectionMenuItemProps { + id?: string + journey?: Journey + handleCloseMenu: () => void + setHasOpenDialog?: (open: boolean) => void + handleKeepMounted?: () => void +} + +/** + * CopyToCollectionMenuItem + * + * Renders the "Copy to collection..." menu item and orchestrates the + * three-step pipeline (journeyDuplicate → optional translation + * subscription → templateGalleryPageAssignJourney), with a + * `GetAdminJourneys` refetch after assign success/failure and after + * translation failure. Surfaces results into `CopyToCollectionDialog` + * via `loading`, `errorMessage`, and `done` props. + */ +export function CopyToCollectionMenuItem({ + id, + journey, + handleCloseMenu, + setHasOpenDialog, + handleKeepMounted +}: CopyToCollectionMenuItemProps): ReactElement { + const { t } = useTranslation('apps-journeys-admin') + const { activeTeam } = useTeam() + const client = useApolloClient() + + const [dialogOpen, setDialogOpen] = useState(false) + const [loading, setLoading] = useState(false) + const [errorMessage, setErrorMessage] = useState(null) + const [done, setDone] = useState(false) + const [selectedCollectionTitle, setSelectedCollectionTitle] = useState< + string | undefined + >(undefined) + const [translationVariables, setTranslationVariables] = + useState(null) + + // Refs read by subscription callbacks (Formik may be torn down by the + // time the subscription's onComplete/onError fires). + const mountedRef = useRef(false) + const newJourneyIdRef = useRef(null) + const pendingTargetCollectionIdRef = useRef(null) + const loadingRef = useRef(false) + + const [journeyDuplicate] = useJourneyDuplicateMutation() + const [templateGalleryPageAssignJourney] = + useTemplateGalleryPageAssignJourneyMutation() + + // Setup body flips mountedRef true (NES-1539 Pattern 3 — Next.js + // dev/StrictMode trap: only flipping in cleanup leaves it permanently + // false after the cleanup runs once). + useEffect(() => { + mountedRef.current = true + return (): void => { + mountedRef.current = false + } + }, []) + + const noActiveTeamCopy = t( + 'No active team selected. Please pick a team and try again.' + ) + const duplicateFailCopy = t('Failed to copy the journey. Please try again.') + const translationFailCopy = t('An error occurred while translating.') + const assignFailCopy = t( + "Failed to add the copy to the collection. You'll find it in All Templates — drag it into the collection from there." + ) + + const safeSetLoading = (next: boolean): void => { + loadingRef.current = next + if (mountedRef.current) setLoading(next) + } + + const refetchAdminJourneys = (): void => { + void client.refetchQueries({ include: ['GetAdminJourneys'] }) + } + + const runAssign = async ( + newJourneyId: string, + targetCollectionId: string + ): Promise => { + try { + await templateGalleryPageAssignJourney({ + variables: { journeyId: newJourneyId, pageId: targetCollectionId } + }) + refetchAdminJourneys() + if (!mountedRef.current) return + safeSetLoading(false) + setTranslationVariables(null) + setDone(true) + } catch { + refetchAdminJourneys() + if (!mountedRef.current) return + safeSetLoading(false) + setTranslationVariables(null) + setErrorMessage(assignFailCopy) + } + } + + const handleSubmit = async (values: { + collectionId: string + language?: JourneyLanguage + showTranslation: boolean + }): Promise => { + // Single-flight guard — defensive against React render timing on + // rapid double-clicks even though the dialog also disables the + // submit button while loading. + if (loadingRef.current) return + + const teamId = activeTeam?.id + if (teamId == null) { + if (mountedRef.current) setErrorMessage(noActiveTeamCopy) + return + } + + const journeyId = id ?? journey?.id + if (journeyId == null) { + if (mountedRef.current) setErrorMessage(duplicateFailCopy) + return + } + + safeSetLoading(true) + if (mountedRef.current) setErrorMessage(null) + + let duplicatedId: string | null = null + try { + const result = await journeyDuplicate({ + variables: { id: journeyId, teamId } + }) + duplicatedId = result.data?.journeyDuplicate?.id ?? null + if (duplicatedId == null || duplicatedId === '') { + throw new Error('Journey duplication failed') + } + } catch { + // No rollback, no refetch — nothing was created. + if (!mountedRef.current) return + safeSetLoading(false) + setErrorMessage(duplicateFailCopy) + return + } + + newJourneyIdRef.current = duplicatedId + pendingTargetCollectionIdRef.current = values.collectionId + + if (values.showTranslation && values.language != null) { + const journeyLanguageName = + journey?.language?.name?.find(({ primary }) => primary)?.value ?? '' + if (mountedRef.current) { + setTranslationVariables({ + journeyId: duplicatedId, + name: journey?.title ?? '', + journeyLanguageName, + textLanguageId: values.language.id, + textLanguageName: + values.language.nativeName ?? values.language.localName ?? '', + userLanguageId: journey?.language?.id, + userLanguageName: journeyLanguageName + }) + } + // Exit — subscription's onComplete will fire runAssign. + return + } + + await runAssign(duplicatedId, values.collectionId) + } + + useJourneyAiTranslateSubscription({ + variables: translationVariables ?? undefined, + skip: translationVariables == null, + onComplete: () => { + if (!mountedRef.current) return + const newJourneyId = newJourneyIdRef.current + const targetCollectionId = pendingTargetCollectionIdRef.current + if (newJourneyId == null || targetCollectionId == null) return + void runAssign(newJourneyId, targetCollectionId) + }, + onError: () => { + refetchAdminJourneys() + if (!mountedRef.current) return + safeSetLoading(false) + setTranslationVariables(null) + setErrorMessage(translationFailCopy) + } + }) + + const guardedClose = (): void => { + if (!mountedRef.current) return + setDialogOpen(false) + setLoading(false) + loadingRef.current = false + setErrorMessage(null) + setDone(false) + setSelectedCollectionTitle(undefined) + setTranslationVariables(null) + newJourneyIdRef.current = null + pendingTargetCollectionIdRef.current = null + setHasOpenDialog?.(false) + handleCloseMenu() + } + + const handleMenuItemClick = (): void => { + handleKeepMounted?.() + handleCloseMenu() + setHasOpenDialog?.(true) + setDialogOpen(true) + } + + return ( + <> + } + onClick={handleMenuItemClick} + testId="CopyToCollection" + /> + { + setSelectedCollectionTitle(undefined) + void handleSubmit(values) + }} + /> + + ) +} diff --git a/apps/journeys-admin/src/components/TemplateGalleryPageList/CopyToCollectionMenuItem/index.ts b/apps/journeys-admin/src/components/TemplateGalleryPageList/CopyToCollectionMenuItem/index.ts new file mode 100644 index 00000000000..c5c2088d21f --- /dev/null +++ b/apps/journeys-admin/src/components/TemplateGalleryPageList/CopyToCollectionMenuItem/index.ts @@ -0,0 +1,2 @@ +export { CopyToCollectionMenuItem } from './CopyToCollectionMenuItem' +export type { CopyToCollectionMenuItemProps } from './CopyToCollectionMenuItem' From 2f628b2b5b669f2ec8e6e340c96c980af4958d85 Mon Sep 17 00:00:00 2001 From: Siyang Date: Thu, 21 May 2026 11:12:10 +1200 Subject: [PATCH 04/14] feat(NES-1637): wire Copy to collection menu item into DefaultMenu --- .../DefaultMenu/DefaultMenu.spec.tsx | 104 ++++++++++++++++++ .../DefaultMenu/DefaultMenu.tsx | 14 +++ 2 files changed, 118 insertions(+) diff --git a/apps/journeys-admin/src/components/JourneyList/JourneyCard/JourneyCardMenu/DefaultMenu/DefaultMenu.spec.tsx b/apps/journeys-admin/src/components/JourneyList/JourneyCard/JourneyCardMenu/DefaultMenu/DefaultMenu.spec.tsx index 11bb795963a..a7a29fb0111 100644 --- a/apps/journeys-admin/src/components/JourneyList/JourneyCard/JourneyCardMenu/DefaultMenu/DefaultMenu.spec.tsx +++ b/apps/journeys-admin/src/components/JourneyList/JourneyCard/JourneyCardMenu/DefaultMenu/DefaultMenu.spec.tsx @@ -9,6 +9,7 @@ import { } from '@core/journeys/ui/TeamProvider' import { GetLastActiveTeamIdAndTeams } from '@core/journeys/ui/TeamProvider/__generated__/GetLastActiveTeamIdAndTeams' import { GET_USER_ROLE } from '@core/journeys/ui/useUserRoleQuery' +import { FlagsProvider } from '@core/shared/ui/FlagsProvider' import { JourneyStatus, @@ -18,6 +19,7 @@ import { } from '../../../../../../__generated__/globalTypes' import { GET_CURRENT_USER } from '../../../../../libs/useCurrentUserLazyQuery' import { getCustomDomainMock } from '../../../../../libs/useCustomDomainsQuery/useCustomDomainsQuery.mock' +import { InCollectionContext } from '../../../../TemplateGalleryPageList/InCollectionContext' import { GET_JOURNEY_WITH_USER_ROLES } from './DefaultMenu' @@ -1089,4 +1091,106 @@ describe('DefaultMenu', () => { expect(setOpenTranslateDialog).toHaveBeenCalled() expect(handleCloseMenu).toHaveBeenCalled() }) + + describe('Copy to collection menu item (NES-1637)', () => { + function renderWithFlagAndContext({ + flag, + inCollection + }: { + flag: boolean + inCollection: boolean + }) { + return render( + + + + + + + + + + + + ) + } + + it('renders "Copy to collection..." when flag is on AND inCollection is true', async () => { + const { getByTestId, getByRole } = renderWithFlagAndContext({ + flag: true, + inCollection: true + }) + await waitFor(() => + expect( + getByTestId('JourneysAdminMenuItemCopyToCollection') + ).toBeInTheDocument() + ) + // Existing CopyToTeamMenuItem still renders unchanged. + expect(getByRole('menuitem', { name: 'Copy to ...' })).toBeInTheDocument() + }) + + it('does NOT render "Copy to collection..." when flag is off but inCollection is true', async () => { + const { queryByTestId, getByRole } = renderWithFlagAndContext({ + flag: false, + inCollection: true + }) + // Wait for the menu to settle (other async menu items resolve). + await waitFor(() => + expect( + getByRole('menuitem', { name: 'Copy to ...' }) + ).toBeInTheDocument() + ) + expect( + queryByTestId('JourneysAdminMenuItemCopyToCollection') + ).not.toBeInTheDocument() + }) + + it('does NOT render "Copy to collection..." when flag is on but inCollection is false', async () => { + const { queryByTestId, getByRole } = renderWithFlagAndContext({ + flag: true, + inCollection: false + }) + await waitFor(() => + expect( + getByRole('menuitem', { name: 'Copy to ...' }) + ).toBeInTheDocument() + ) + expect( + queryByTestId('JourneysAdminMenuItemCopyToCollection') + ).not.toBeInTheDocument() + }) + + it('does NOT render "Copy to collection..." when flag is off and inCollection is false (sanity)', async () => { + const { queryByTestId, getByRole } = renderWithFlagAndContext({ + flag: false, + inCollection: false + }) + await waitFor(() => + expect( + getByRole('menuitem', { name: 'Copy to ...' }) + ).toBeInTheDocument() + ) + expect( + queryByTestId('JourneysAdminMenuItemCopyToCollection') + ).not.toBeInTheDocument() + }) + }) }) diff --git a/apps/journeys-admin/src/components/JourneyList/JourneyCard/JourneyCardMenu/DefaultMenu/DefaultMenu.tsx b/apps/journeys-admin/src/components/JourneyList/JourneyCard/JourneyCardMenu/DefaultMenu/DefaultMenu.tsx index 8408b8d6420..cc84bd6b3bd 100644 --- a/apps/journeys-admin/src/components/JourneyList/JourneyCard/JourneyCardMenu/DefaultMenu/DefaultMenu.tsx +++ b/apps/journeys-admin/src/components/JourneyList/JourneyCard/JourneyCardMenu/DefaultMenu/DefaultMenu.tsx @@ -7,6 +7,7 @@ import { ReactElement, useEffect, useMemo } from 'react' import { useTeam } from '@core/journeys/ui/TeamProvider' import { TemplateActionButton } from '@core/journeys/ui/TemplateView/TemplateViewHeader/TemplateActionButton' import { useUserRoleQuery } from '@core/journeys/ui/useUserRoleQuery' +import { useFlags } from '@core/shared/ui/FlagsProvider' import Edit2Icon from '@core/shared/ui/icons/Edit2' import EyeOpenIcon from '@core/shared/ui/icons/EyeOpen' import TranslateIcon from '@core/shared/ui/icons/Translate' @@ -36,6 +37,8 @@ import { CreateTemplateItem } from '../../../../Editor/Toolbar/Items/CreateTempl import { ShareItem } from '../../../../Editor/Toolbar/Items/ShareItem/ShareItem' import { MenuItem } from '../../../../MenuItem' import { CopyToTeamMenuItem } from '../../../../Team/CopyToTeamMenuItem/CopyToTeamMenuItem' +import { CopyToCollectionMenuItem } from '../../../../TemplateGalleryPageList/CopyToCollectionMenuItem' +import { useInCollection } from '../../../../TemplateGalleryPageList/InCollectionContext' import { DuplicateJourneyMenuItem } from '../DuplicateJourneyMenuItem' import { ArchiveJourney } from './ArchiveJourney' @@ -117,6 +120,8 @@ export function DefaultMenu({ }: DefaultMenuProps): ReactElement { const { t } = useTranslation('apps-journeys-admin') const { activeTeam } = useTeam() + const { teamTemplateCollection } = useFlags() + const inCollection = useInCollection() const { data: userRoleData } = useUserRoleQuery() const { refetchTemplateStats } = useTemplateFamilyStatsAggregateLazyQuery() const { hostname } = useCustomDomainsQuery({ @@ -300,6 +305,15 @@ export function DefaultMenu({ setHasOpenDialog={setHasOpenDialog} /> )} + {teamTemplateCollection === true && inCollection && ( + + )} {activeTeam != null && ( <> Date: Thu, 21 May 2026 11:30:10 +1200 Subject: [PATCH 05/14] docs(NES-1637): add implementation plan for Copy to collection action --- ...nes-1637-copy-to-collection-action-plan.md | 410 ++++++++++++++++++ 1 file changed, 410 insertions(+) create mode 100644 docs/plans/2026-05-21-001-feat-nes-1637-copy-to-collection-action-plan.md diff --git a/docs/plans/2026-05-21-001-feat-nes-1637-copy-to-collection-action-plan.md b/docs/plans/2026-05-21-001-feat-nes-1637-copy-to-collection-action-plan.md new file mode 100644 index 00000000000..c1d483f94f9 --- /dev/null +++ b/docs/plans/2026-05-21-001-feat-nes-1637-copy-to-collection-action-plan.md @@ -0,0 +1,410 @@ +--- +title: 'feat: Add "Copy to collection" action to template cards in Collections (NES-1637)' +type: feat +status: active +date: 2026-05-21 +--- + +# feat: Add "Copy to collection" action to template cards in Collections (NES-1637) + +## Summary + +Add a `"Copy to collection..."` menu item to template journey cards rendered inside a Collection. Clicking it opens a new admin-app `CopyToCollectionDialog` that lets the user pick a target collection (and optionally a translation language); on submit, the menu item runs `journeyDuplicate` → optional `useJourneyAiTranslateSubscription` → `templateGalleryPageAssignJourney` sequentially, with a single dialog-internal spinner across the whole pipeline. Gated by the `teamTemplateCollection` LaunchDarkly flag and an `InCollectionContext` so the item appears only when the host card is rendered inside a Collection. + +--- + +## Problem Frame + +The template gallery page (Local Template Library, NES-1539 / NES-1547) lets publishers organize templates into Collections. Today, publishers can drag templates between collections, but there is no way to **copy** a template — duplicating it as a new journey entity while keeping the source intact — without going through the cross-team "Copy to ..." flow, which is the wrong tool (cross-team) and ends up in the user's flat list instead of a chosen collection. The original scope of this ticket was a Collection-level "Share Again" (duplicating an entire collection); after engineering review, the work was narrowed to a per-journey "Copy to collection" action that reuses existing primitives and ships without backend work. + +--- + +## Requirements + +- R1. Add a `"Copy to collection..."` menu item to the journey card More menu when the card is rendered inside a Collection. The item must not appear on the same `JourneyCard` when rendered in any non-Collection context (active, archived, trashed, All Templates). +- R2. Gate the menu item with the `teamTemplateCollection` LaunchDarkly flag — when the flag is off, the item is not rendered at all (no placeholder, no spinner). +- R3. Clicking the item opens a new dialog containing: a single-select dropdown of the active team's collections (unfiltered — the source collection is included as a valid target), the same language picker + show-translation toggle as `CopyToTeamDialog`, and submit/cancel actions. +- R4. The dropdown empty state — zero collections in the active team — renders a disabled `"No collections available"` row; the submit button is disabled until a target is selected. +- R5. Submitting runs the pipeline `journeyDuplicate(id, teamId: activeTeamId, forceNonTemplate: false, duplicateAsDraft: false)` → (if translation toggled on) `useJourneyAiTranslateSubscription` to completion → `templateGalleryPageAssignJourney(journeyId: newJourneyId, pageId: targetCollectionId)`. +- R6. The entire pipeline is shown to the user as a single dialog-internal spinner — the streaming translation progress is hidden behind a generic "Translating…" state with no per-block progress bar. +- R7. On success, the dialog swaps its submit button for a `Done` button; the target Collection card's `templates` list updates without a hard refresh, and the new card is visible inside the chosen collection. +- R8. On `journeyDuplicate` failure, the dialog shows `"Failed to copy the journey. Please try again."` with a `Done` button; no rollback (nothing was created). +- R9. On translation subscription failure, the pipeline stops (no `assignJourney` call); the dialog shows `"An error occurred while translating."` with a `Done` button; no rollback. The new journey is reachable in All Templates after a `GetAdminJourneys` refetch. +- R10. On `templateGalleryPageAssignJourney` failure (with or without translation), the dialog shows `"Failed to add the copy to the collection. You'll find it in All Templates — drag it into the collection from there."` with a `Done` button; no rollback. The new journey is reachable in All Templates after a `GetAdminJourneys` refetch. (Contingency: if pre-implementation verification shows drag-from-All-Templates is not supported, soften the message to `"Failed to add the copy to the collection. The copy is in All Templates."` and file a follow-up to either enable drag-from-All-Templates or surface a manual retry path.) +- R11. The new menu item must integrate with `GalleryDialogLockContext` via the existing `setHasOpenDialog` prop pattern so DnD on the gallery page pauses while the dialog is open. +- R12. The dialog must remain mounted across the full pipeline; user-initiated close mid-pipeline is allowed and uses the `mountedRef`/`guardedClose` pattern from NES-1539/NES-1543 to avoid setState-after-unmount. + +--- + +## Scope Boundaries + +- No backend changes — `journeyDuplicate`, `journeyAiTranslateCreateSubscription`, and `templateGalleryPageAssignJourney` are used as they exist on `main` at the worktree's base commit. +- No rollback at any failure stage — matches the existing `CopyToTeamMenuItem` behavior. +- No cross-team copy — the existing `CopyToTeamMenuItem` ("Copy to ...") is unchanged. +- No custom-domain gating — verified absent in the current Collection flow. +- No metadata editing during the copy (title, description, language source, etc. are not editable on the source journey). +- No collection-level "Share Again" / bulk template duplication (the original ticket scope, now split). +- The new dialog is not added to the shared `libs/journeys/ui/` package — it is admin-app-specific and lives under `apps/journeys-admin/`. Promotion to the shared lib is a follow-up if another consumer needs it. + +### Deferred to Follow-Up Work + +- Collection-level "Share Again" / bulk-template duplication: separate ticket to be scoped. +- Rollback on translation failure (also a latent gap in `CopyToTeamMenuItem` since neither path rolls back today): separate ticket — would also fix the existing team flow for consistency. +- `templateGalleryPageAssignJourney` optimistic response improvements: noted in NES-1539 learnings (Pattern 11); current behavior is sufficient for this plan because the byte-identical selection auto-updates the target page's `templates` field via Apollo's normalized merge. + +--- + +## Context & Research + +### Relevant Code and Patterns + +- `apps/journeys-admin/src/components/Team/CopyToTeamMenuItem/CopyToTeamMenuItem.tsx` — closest analogue. Mirror its shape: `setHasOpenDialog` prop wiring, `useState`-gated translation subscription via `translationVariables`, snackbar on success/error, dialog stays open while pipeline runs. +- `libs/journeys/ui/src/components/CopyToTeamDialog/CopyToTeamDialog.tsx` — visual reference for Formik + Yup + `TranslationDialogWrapper` composition. Do **not** reuse: it has hard-coded `useTeam` and `UPDATE_LAST_ACTIVE_TEAM_ID` side effects that do not apply here. +- `libs/journeys/ui/src/components/TranslationDialogWrapper/` — directly reusable shell. Provides Dialog scaffold, submit/cancel, loading wiring, and the translating-state slot. +- `apps/journeys-admin/src/components/JourneyList/JourneyCard/JourneyCardMenu/DefaultMenu/DefaultMenu.tsx` — host menu where the new item attaches. The existing `CopyToTeamMenuItem` is rendered here (around the `!isLocalTemplate` gate); the new item attaches adjacent to it with its own gate (`teamTemplateCollection` flag && `InCollectionContext` provided). +- `apps/journeys-admin/src/components/TemplateGalleryPageList/TemplateGalleryPageList.tsx` — the page that wraps Collection grids and All-Templates in `GalleryDialogLockContext`. The new `InCollectionContext` provider belongs INSIDE the Collection-grid mapping, NOT around the All-Templates grid. +- `apps/journeys-admin/src/components/TemplateGalleryPageList/CollectionCard/CollectionCard.tsx` — possible alternative location for the `InCollectionContext.Provider` (around the children slot that ultimately renders `DraggableJourneysGrid`). Either site is acceptable; pick whichever has the smallest blast radius after reading the host file. +- `apps/journeys-admin/src/libs/useTemplateGalleryPagesQuery/useTemplateGalleryPagesQuery.ts` — sources the collection dropdown options. Returns `{ id, title, … }` per page. +- `apps/journeys-admin/src/libs/useTemplateGalleryPageAssignJourneyMutation/useTemplateGalleryPageAssignJourneyMutation.ts` — already exposes a byte-identical selection of the target page; Apollo's normalized cache merge handles the visual update of `CollectionCard.templates` for the target. No `update` callback or `refetchQueries` needed on this mutation specifically; the gap is on the `journeyById` derived map (see Key Technical Decisions). +- `libs/journeys/ui/src/libs/useJourneyDuplicateMutation/useJourneyDuplicateMutation.ts` — its `update` callback explicitly skips reads where `where.template === true`, which is the read shape the gallery page uses for its `journeyById` map. This is the central cache hazard for this plan. +- `libs/journeys/ui/src/libs/useJourneyAiTranslateSubscription/` — used as awaitable plumbing via the existing `onComplete`/`onError` callback shape. The `onData` callback already writes translation results into the cache automatically — consumer `onData` runs after the cache write. +- `apps/journeys-admin/src/components/TemplateGalleryPageList/useCollectionMutations/` — reference for collection-affecting mutation orchestration; encodes the optimistic-response / cache-shape conventions established by NES-1539. +- `apps/journeys-admin/src/components/MenuItem/MenuItem.tsx` — shared menu-item presentation; consumed via `label`, `icon`, `onClick`, `testId`. + +### Institutional Learnings + +- `docs/solutions/best-practices/template-gallery-page-collections-patterns-nes1539.md` — Patterns 1 (`cache.updateQuery` over `cache.modify` for prepends), 2 (drop `enableReinitialize`; remount via `key={journey.id}`), 3 (`mountedRef` + `guardedClose`, with setup-body flip of `mountedRef.current = true`), 5 (lift busy flag for DnD interplay), 8 (extract orchestration into a hook when components grow past ~600 lines), 9 (UX vocabulary "Collection", backend vocabulary `TemplateGalleryPage`), 11 (assign mutation cache strategy), 18 (re-apply status predicate on client when reading cached lists). +- `docs/solutions/best-practices/local-template-dialog-consolidation-patterns-nes1543.md` — Patterns 3 (no `enableReinitialize` when a subscription is in scope; the team translation flow has caused silent dirty-state resets), 5 (hoist mutations to `apps/journeys-admin/src/libs/useMutation/`; already done here, just reuse), 6 (`DialogJourney` shape + self-bridged provider when the dialog runs outside `JourneyProvider`), 7 (`testId` prop, not `data-testid` on the shared Dialog), 8 (verify automated suggestions against codebase conventions). +- `docs/solutions/runtime-errors/yoga-response-cache-null-stickiness-and-zombie-process-debugging-nes1644.md` — `TemplateGalleryPage.templates` is normalized as `TemplateGalleryItem:` refs while `journeyDuplicate` returns `Journey:`. If a manual `cache.modify` on `templates` is ever added, the typename gap will silently break it. The plan relies on the assign mutation's response shape and does not write to `templates` by hand, so this is a hazard to be aware of, not an active concern. +- `docs/solutions/logic-errors/response-cache-empty-list-invalidation-2026-05-10.md` — `Query.templateGalleryPage(s)` are pinned TTL 0 in `apis/api-journeys-modern/src/yoga.ts`; do not add a TTL while implementing. + +### External References + +- None. Local patterns are well-established for every primitive used here; external research adds no value. + +--- + +## Key Technical Decisions + +- **Build a new `CopyToCollectionDialog`, do not refactor `CopyToTeamDialog`.** `CopyToTeamDialog` has team-specific side effects (`useTeam`, `UPDATE_LAST_ACTIVE_TEAM_ID`) baked into its submit handler and form schema. Genericizing it would touch shared-lib code consumed by both `journeys-admin` and `journeys`, expanding blast radius. The new admin-app dialog composes the already-extracted shared primitives (`TranslationDialogWrapper`, `LanguageAutocomplete`, `useLanguagesQuery`, `SUPPORTED_LANGUAGE_IDS`) directly. +- **Gate the new menu item with an `InCollectionContext`, provided only inside `CollectionCard`'s children slot.** `JourneyCard` is reused across at least five contexts (`ActiveJourneyList`, `ArchivedJourneyList`, `TrashedJourneyList`, All-Templates, Collections); a context-based gate is purely additive (no prop-signature changes through `JourneyCard → JourneyCardMenu → DefaultMenu`) and read at the menu-item site via a `useInCollection()` hook returning `true`/`false`. `useGalleryDialogLock` was considered and rejected because the existing lock provider wraps both the Collections grid and the All-Templates grid, so it cannot distinguish the two contexts. +- **After assign success or failure, refetch `GetAdminJourneys`.** `useJourneyDuplicateMutation`'s built-in cache `update` deliberately skips reads where `where.template === true`, so the new journey is invisible to `TemplateGalleryPageList`'s `journeyById` map (built from `template: true` adminJourneys). Without a refetch, the new card silently fails to render even after a successful assign. The same refetch covers orphan recovery when the assign step fails — the user can find the new copy under "All Templates" and drag it manually. +- **Pipeline stops on translation failure; assign does not run.** If the user toggled translation on, they signaled intent for a translated copy. Continuing to assign an untranslated copy into their chosen collection would silently substitute the wrong thing. Stopping the pipeline leaves an untranslated journey in their flat list (after the refetch), which is the same recovery story as an assign failure — drag or delete. +- **Single spinner UX; the subscription is plumbing.** Hide the streaming translation progress behind a generic "Translating…" indicator in the dialog. The subscription is consumed via `onComplete`/`onError`; intermediate `onData` frames are ignored by the menu item (Apollo's auto-cache-write still happens for the translated fields). This avoids re-creating the team-copy progress UX in a flow where the dialog state is already complex enough. +- **Place the new dialog and menu-item components under `apps/journeys-admin/src/components/TemplateGalleryPageList/`.** They are admin-app-specific (use admin-app query hooks and live close to their only call site). Co-locating them with the gallery-page list reduces import churn and keeps the gallery-page surface area cohesive. Promote to `libs/journeys/ui/` only if a second consumer ever appears. +- **`mountedRef` + `guardedClose` in the menu item.** The orchestration awaits multiple async steps; mid-pipeline unmount (user closes dialog or navigates away) must not setState on an unmounted component. Per NES-1539 Pattern 3, the setup-body must flip `mountedRef.current = true` (not only the cleanup), to avoid the Next.js dev/StrictMode trap where the cleanup runs once and `mountedRef.current` is permanently false. +- **Sequential pipeline, not `Promise.all`.** Each step depends on the previous one (`assignJourney` needs the new journey ID; translation needs the new journey ID). `Promise.allSettled` is the wrong shape for chained dependencies. +- **No `enableReinitialize` on the Formik form.** Per NES-1543 Pattern 3, subscription-driven Apollo cache updates can land mid-edit and silently reset `initialValues`. The form fields here are user-only inputs (target collection, language, toggle), not journey fields, so structurally this isn't load-bearing, but staying consistent avoids future drift. + +--- + +## Open Questions + +### Resolved During Planning + +- **Reuse or build new dialog?** Build new — `CopyToTeamDialog`'s team side effects make safe genericization a shared-lib refactor that exceeds this ticket. +- **Custom-domain gating?** No — verified that the current Collection flow has no custom-domain handling; NES-1644 hook applies only to the publish-page flow. +- **Rollback on failure?** No, at any stage — matches the existing `CopyToTeamMenuItem` behavior. Surface error in the dialog; the user can find the orphan in All Templates after the refetch. +- **Translation progress UX?** Hidden behind a single "Translating…" state — no per-block streaming UI in this flow. +- **Filter source collection from the dropdown?** No — the source is allowed as a target (backend's single-membership rule is per-journey, so the new duplicate ID coexists with the original in the same collection). +- **Cache update strategy for the new journey appearing in the target Collection card?** Assign mutation's byte-identical selection auto-updates `TemplateGalleryPage.templates`; `refetchQueries({ include: ['GetAdminJourneys'] })` after assign fills the `journeyById` map so the templates entry resolves to a visible card. +- **Where to place the new components?** Under `apps/journeys-admin/src/components/TemplateGalleryPageList/` — admin-app-specific, co-located with the gallery-page surface. + +### Deferred to Implementation + +- **Exact location of the `InCollectionContext.Provider`** — `CollectionCard.tsx` or `TemplateGalleryPageList.tsx`'s Collection-grid mapping. Pick the site with the smallest blast radius after reading both. +- **Whether to extract the orchestration into a `useCopyToCollection` hook** — if the menu item file approaches the ~600-line threshold flagged by NES-1539 Pattern 8, extract; otherwise keep the orchestration inline. +- **i18n string finalization** — error messages are user-facing; final copy may be reviewed by the team, but the messages listed in R8–R10 are the working draft. +- **Whether to add a Storybook story** — optional per `AGENTS.md`; `CopyToTeamDialog.stories.tsx` exists as a precedent. Add if low-cost. +- **`refetchQueries(['GetAdminJourneys'])` observable verification** — confirm in a running session that the gallery page's `template: true` query is the observable hit by the refetch (Apollo DevTools or `client.getObservableQueries()` debug log). If not, fall back to `cache.updateQuery` on the specific variant per NES-1539 Pattern 1. +- **Drag-from-All-Templates recovery verification** — confirm that the gallery page supports drag from the All-Templates list into a Collection. If not, apply R10's contingency copy and file a follow-up. +- **Translation subscription error semantics** — investigate whether `onError` distinguishes transient SSE drops from terminal translation failures. Consider a single silent retry before showing the terminal error if a transient-class signal is identifiable. + +--- + +## Implementation Units + +- U1. **`InCollectionContext` and provider wiring** + +**Goal:** Introduce a small admin-app-internal React context that signals "this subtree is rendered inside a Collection card." Provide the context only inside the Collection-grid path so the All-Templates path leaves it undefined. Consumed by the new menu item to gate its rendering. + +**Requirements:** R1 (infrastructure; gate applied in U4) + +**Dependencies:** None + +**Files:** +- Create: `apps/journeys-admin/src/components/TemplateGalleryPageList/InCollectionContext/InCollectionContext.ts` +- Create: `apps/journeys-admin/src/components/TemplateGalleryPageList/InCollectionContext/index.ts` +- Modify: `apps/journeys-admin/src/components/TemplateGalleryPageList/CollectionCard/CollectionCard.tsx` *or* `apps/journeys-admin/src/components/TemplateGalleryPageList/TemplateGalleryPageList.tsx` (whichever site cleanly wraps only the Collection-grid path's `DraggableJourneysGrid`; do not wrap the All-Templates grid) +- Test: `apps/journeys-admin/src/components/TemplateGalleryPageList/InCollectionContext/InCollectionContext.spec.tsx` + +**Approach:** +- The context exposes a `useInCollection()` hook returning `true` when the consuming component is under the provider, `false` (or `undefined` coerced to `false`) otherwise. +- The provider is placed once, inside the Collection-grid mapping in the gallery page — the exact host (CollectionCard children slot vs. TemplateGalleryPageList) is an implementation-time decision based on the smaller diff. +- The context is admin-app-internal; do not export from `libs/journeys/ui/`. + +**Patterns to follow:** +- `apps/journeys-admin/src/components/TemplateGalleryPageList/GalleryDialogLockContext.ts` for the file shape and naming. + +**Test scenarios:** +- Happy path: `useInCollection` returns `true` when the consumer is rendered inside the provider. +- Happy path: `useInCollection` returns `false` when the consumer is rendered outside any provider (no provider in the subtree). +- Edge case: nesting two providers should not throw; the closest provider's value wins (standard React context semantics — assert non-throw and the deepest-value behavior). + +**Verification:** +- `useInCollection` returns `true` only when a parent component has rendered the provider. Existing All-Templates and non-Collection list views are unaffected (the hook returns `false` there). + +--- + +- U2. **`CopyToCollectionDialog` component** + +**Goal:** Build the dialog the menu item opens. Mirrors `CopyToTeamDialog` visually but is admin-app-specific, source-team-only, and replaces the team picker with a collection picker sourced from `useTemplateGalleryPagesQuery`. + +**Requirements:** R3, R4, R6, R7 + +**Dependencies:** None (the menu item in U3 will consume it but the dialog can be built and tested independently against mocked submit handlers) + +**Files:** +- Create: `apps/journeys-admin/src/components/TemplateGalleryPageList/CopyToCollectionDialog/CopyToCollectionDialog.tsx` +- Create: `apps/journeys-admin/src/components/TemplateGalleryPageList/CopyToCollectionDialog/index.ts` +- Create: `apps/journeys-admin/src/components/TemplateGalleryPageList/CopyToCollectionDialog/CopyToCollectionDialog.spec.tsx` +- Optional: `apps/journeys-admin/src/components/TemplateGalleryPageList/CopyToCollectionDialog/CopyToCollectionDialog.stories.tsx` (low-cost; add if matching `CopyToTeamDialog.stories.tsx`) + +**Approach:** +- Compose `TranslationDialogWrapper` for the dialog shell, submit/cancel buttons, and loading state. +- Formik form with three fields: `collectionSelect: string`, `languageSelect?: JourneyLanguage`, `showTranslation: boolean`. Yup schema requires `collectionSelect` always; requires `languageSelect` only when `showTranslation === true` (same conditional shape `CopyToTeamDialog` uses). +- Do **not** set `enableReinitialize` on Formik. `CopyToTeamDialog` currently sets `enableReinitialize` (around line 169 at the time of writing) — **do not carry that prop across**. Per NES-1543 Pattern 3, a subscription-driven Apollo cache write can land mid-edit and silently reset `initialValues`. If parent props change (e.g., a journey-id remount is needed), the parent should remount via `key={journey.id}` instead — per NES-1539 Pattern 2. +- Collection dropdown is a single-select MUI `TextField select` populated from `useTemplateGalleryPagesQuery({ teamId: activeTeam?.id ?? '' }, { skip: activeTeam?.id == null })`. Each option is `{page.title}`. The `skip` arm covers the edge case where the active team is null (team-switcher async load, lost access mid-session); treat the skipped state as "no options + disabled submit," distinct from a genuine zero-pages empty state. +- Empty state: when the query returns zero pages, render a single disabled `No collections available` and disable the submit button via Formik validity (no `collectionSelect` value possible). +- Loading state of the gallery-pages query: while loading (and while `skip` is active), render a single disabled `Loading…` and disable the submit button. The disabled-item treatment matches the empty-state visual rhythm and is simpler than a skeleton row. +- Translation toggle + language picker: copy the `Switch` + `LanguageAutocomplete` block from `CopyToTeamDialog`. Toggle label is `"Translate the copy to another language"` (no team-specific wording; describes the action in context). Use `SUPPORTED_LANGUAGE_IDS` and `useLanguagesQuery({ languageId: '529', where: { ids: [...SUPPORTED_LANGUAGE_IDS] } })`. +- Pass-through props from the menu item: `open`, `loading`, `errorMessage?: string`, `done?: boolean` (success state — submit button is swapped for Done), `onClose`, `onSubmit({ collectionId, language?, showTranslation })`, and the journey title for the dialog header copy. +- When `errorMessage` is set, the dialog body renders the message in place of the spinner; the action area shows a single `Done` button bound to `onClose`. When `done === true`, the dialog body renders success copy `"Copied to {collectionTitle}."` (echoes the target back so the user sees confirmation of the right destination), and the action area shows a single `Done` button. +- **Submit button disabled state** — disabled when `loading === true || done === true || errorMessage != null || formik invalid || query is skipped/loading/empty`. This covers the concurrent-submit guard at the UI layer (the orchestration layer in U3 adds a defensive single-flight guard as well). +- **Accessibility:** + - Initial focus when the dialog opens lands on the collection dropdown (the first interactive field). + - On Done / close, focus returns to the triggering card's More menu button — coordinate via the `onClose` callback chain so the parent menu can refocus its trigger. + - Terminal state transitions (success / error) render inside a region with `role="status"` or `aria-live="polite"` so screen readers announce the transition without focus movement. +- Use the shared `Dialog`'s `testId` prop, not `data-testid` (per NES-1543 Pattern 7). Suggested: `testId="CopyToCollectionDialog"`. +- Use `useTranslation('apps-journeys-admin')` for all user-facing strings. + +**Patterns to follow:** +- `libs/journeys/ui/src/components/CopyToTeamDialog/CopyToTeamDialog.tsx` — Formik shape, Yup schema, language-picker block, `TranslationDialogWrapper` composition. +- `apps/journeys-admin/src/libs/useTemplateGalleryPagesQuery/` — collection dropdown source. + +**Test scenarios:** +- Happy path: renders with a closed initial state when `open: false`; renders the dropdown with N collections when `open: true` and the query returns N pages. +- Happy path: selecting a collection enables the submit button; clicking submit calls `onSubmit({ collectionId, language: undefined, showTranslation: false })`. +- Happy path: toggling translation on reveals the language picker; selecting a language and submitting passes `language` and `showTranslation: true` to `onSubmit`. +- Happy path: initial focus on open lands on the collection dropdown; on Done/close, focus returns to the triggering button (test via `userEvent.tab()` or the focus-trap assertion). +- Happy path: on `done === true`, the dialog body shows `"Copied to {selectedCollectionTitle}."` echoing the chosen target. +- Edge case: when `useTemplateGalleryPagesQuery` returns zero pages, the dropdown shows a disabled `"No collections available"` row and the submit button is disabled. +- Edge case: when `useTemplateGalleryPagesQuery` is loading OR `skip`-arm is active (null active team), the dropdown shows a disabled `"Loading…"` row and the submit button is disabled. +- Edge case: source collection is present in the dropdown options (no filtering); the dialog does not enforce a "different collection" constraint. +- Edge case: when `showTranslation` is true but no language is selected, the submit button is disabled (Yup validation). +- Edge case: rapid double-click on submit (before `loading` flushes) fires `onSubmit` only once — the button disables on the first click via the `loading || done || errorMessage` gate. +- Error path: when `errorMessage` prop is set, the spinner is replaced with the error text in a `role="status"` / `aria-live="polite"` region and the action area shows a single `Done` button bound to `onClose`. +- Error path: when `done` prop is `true`, the body shows success copy in a `role="status"` / `aria-live="polite"` region and the action area shows a single `Done` button. +- Error path: when `loading` prop is `true`, no buttons fire on click (submit and close are disabled or no-op). +- Integration: closing the dialog mid-loading does not call `onSubmit` again; `onClose` is the only path out. + +**Verification:** +- The dialog renders, validates, and surfaces the three terminal states (loading, error, done) without depending on the menu item's orchestration logic. All branches are exercised through props alone. + +--- + +- U3. **`CopyToCollectionMenuItem` component (orchestration)** + +**Goal:** The menu item that mounts the dialog and orchestrates the three-step pipeline. Owns the `journeyDuplicate` call, the optional translation subscription, the `templateGalleryPageAssignJourney` call, and the post-pipeline `GetAdminJourneys` refetch. Surfaces results into the dialog as `loading`, `errorMessage`, and `done`. + +**Requirements:** R5, R6, R7, R8, R9, R10, R11, R12 + +**Dependencies:** U2 (dialog), U1 (context — consumed at the call site in U4 but defined here as a peer surface) + +**Files:** +- Create: `apps/journeys-admin/src/components/TemplateGalleryPageList/CopyToCollectionMenuItem/CopyToCollectionMenuItem.tsx` +- Create: `apps/journeys-admin/src/components/TemplateGalleryPageList/CopyToCollectionMenuItem/index.ts` +- Create: `apps/journeys-admin/src/components/TemplateGalleryPageList/CopyToCollectionMenuItem/CopyToCollectionMenuItem.spec.tsx` + +**Approach:** +- Component shape mirrors `CopyToTeamMenuItem`: renders the shared `MenuItem` (label, icon, `onClick`, `testId="CopyToCollection"`) plus the `CopyToCollectionDialog` it controls. +- Props: `id?: string`, `journey?: Journey`, `handleCloseMenu: () => void`, `setHasOpenDialog?: (open: boolean) => void`, `handleKeepMounted?: () => void` — same shape as `CopyToTeamMenuItem` for symmetry. The optional `setHasOpenDialog` is wired by the parent `JourneyCard` chain (same as the team flow); we rely on the parent to provide it rather than enforcing it at this layer. +- Internal state: `dialogOpen: boolean`, `loading: boolean`, `errorMessage: string | null`, `done: boolean`, `translationVariables: TranslationVars | null`, `pendingTargetCollectionId: string | null`, `mountedRef: useRef(false)`. +- `mountedRef` initialization runs in a `useEffect` whose setup body sets `mountedRef.current = true` and cleanup sets it `false` — per NES-1539 Pattern 3. +- `guardedClose(reason: 'cancel' | 'done')`: gated by `mountedRef.current`; resets dialog state, calls `setHasOpenDialog?.(false)` and `handleCloseMenu()` only when truthy. +- **Callback-driven orchestration (not awaitable).** `useJourneyAiTranslateSubscription` is `useSubscription`-based — its terminal signal is the `onComplete` callback, not a Promise. Mirror `CopyToTeamMenuItem`'s shape by splitting orchestration in two halves: + - `handleSubmit({ collectionId, language?, showTranslation? })`: + 1. **Single-flight guard.** Early return if `loading === true` — UI already disables the submit button (per U2), this is the defensive belt-and-braces against React render timing on rapid double-clicks. + 2. **Active-team guard.** Capture `const teamId = activeTeam?.id` once at the top. If null, set `errorMessage` to a "no active team" copy and exit before any mutation runs. (This converts a runtime NPE into a recoverable UI state.) + 3. Set `loading: true`. `setHasOpenDialog?.(true)` was already flipped on dialog open; no change. + 4. `await journeyDuplicate({ variables: { id: journey.id, teamId } })`. On throw, set `errorMessage` to the duplicate-fail copy and exit. No rollback. No refetch (nothing was created). + 5. Capture `newJourneyId` and `targetCollectionId` into refs / state so the second half can read them without re-deriving from Formik (which may be torn down by the time the subscription completes). + 6. If `showTranslation && language`: set `translationVariables` to arm the subscription. **Exit `handleSubmit`** — the subscription's `onComplete` will fire `runAssign`. Otherwise, call `runAssign(newJourneyId, targetCollectionId)` directly. + - `runAssign(newJourneyId, targetCollectionId)`: + 1. `await templateGalleryPageAssignJourney({ variables: { journeyId: newJourneyId, pageId: targetCollectionId } })`. On throw, `refetchQueries({ include: ['GetAdminJourneys'] })` and set `errorMessage` to the assign-fail copy. On success, `refetchQueries({ include: ['GetAdminJourneys'] })` and set `done: true`. All setState calls gated by `mountedRef.current`. + - The translation subscription is wired via `useJourneyAiTranslateSubscription({ variables: translationVariables, skip: !translationVariables, onComplete, onError })`. `onComplete` calls `runAssign(newJourneyId, targetCollectionId)`. `onError` calls `refetchQueries({ include: ['GetAdminJourneys'] })` (so the orphan is visible in All Templates) and sets `errorMessage` to the translation-fail copy. +- Every setState inside an async callback or subscription handler is gated by `mountedRef.current` to avoid setState-after-unmount. +- Snackbars are NOT used for terminal errors — the dialog body owns the error state per R8–R10. Snackbars MAY be used for the success case if it improves discoverability (optional; default is the in-dialog success copy + Done). +- Use `useTeam` for `activeTeam` and read `activeTeam?.id` defensively (capture once at submit, then pipe through the pipeline; mid-flight team-switch keeps the pipeline against the submit-time team). +- **`refetchQueries({ include: ['GetAdminJourneys'] })` strategy.** This refetches every active `GetAdminJourneys` observable regardless of variables. The architectural assumption is that when this menu item fires, the gallery page (which mounts `GetAdminJourneys` with `where: { template: true }`) is the active surface — because the menu item is gated to `useInCollection() === true`, which is only provided inside the gallery page. Therefore the `template: true` variant is the observable getting refreshed, and `journeyById` repopulates with the new entry. **Verification step:** during implementation, confirm in a running session that the `template: true` query is the observable hit by the refetch — open the gallery page, run the copy flow, and check Apollo DevTools (or a `client.getObservableQueries()` debug log) to see which `GetAdminJourneys` instances exist at refetch time. +- **Drag-from-All-Templates recovery verification.** R10's recovery copy ("drag it into the collection from there") assumes the gallery page supports drag-FROM the All-Templates flat list INTO a Collection. Before merging, manually verify this drag direction works in the gallery page UI. If it does NOT work, apply the R10 contingency copy (`"Failed to add the copy to the collection. The copy is in All Templates."`) and file a follow-up to either enable the missing drag direction or add a manual "retry assign" affordance on the orphan. + +**Execution note:** Implement orchestration test-first. The three-step pipeline + three terminal error states + done state is the riskiest part of the change; lean on tests to lock the state machine before threading it through the menu/dialog. + +**Technical design:** + +> *This illustrates the intended pipeline shape and is directional guidance for review, not implementation specification.* + +``` +handleSubmit({ collectionId, language?, showTranslation? }): + if loading: return // single-flight guard + teamId = activeTeam?.id + if teamId == null: + setError(noActiveTeamCopy); return // null guard + setLoading(true) + newJourneyId <- await journeyDuplicate(journey.id, teamId) + on throw: setError(duplicateFailCopy); return // no rollback + pendingTargetCollectionId = collectionId + if showTranslation && language: + setTranslationVars({ journeyId: newJourneyId, ...langArgs }) + return // exit; onComplete fires runAssign + runAssign(newJourneyId, collectionId) + +runAssign(newJourneyId, targetCollectionId): + await templateGalleryPageAssignJourney(newJourneyId, targetCollectionId) + on throw: refetch(GetAdminJourneys); setError(assignFailCopy); return + refetch(GetAdminJourneys) + setDone(true) + +// wired via useJourneyAiTranslateSubscription({ variables: translationVariables, skip: !translationVariables, … }) +onComplete (subscription terminates): + if mountedRef: runAssign(newJourneyId, pendingTargetCollectionId) + +onError (subscription error — terminal-vs-transient unverified at plan time): + if mountedRef: + refetch(GetAdminJourneys) + setError("An error occurred while translating.") + +dialog close (via Done or Cancel): + guardedClose -> reset state, setHasOpenDialog?.(false), handleCloseMenu() +``` + +**Patterns to follow:** +- `apps/journeys-admin/src/components/Team/CopyToTeamMenuItem/CopyToTeamMenuItem.tsx` — overall component shape, subscription gating via `translationVariables` state, `setHasOpenDialog` wiring, `handleKeepMounted` usage. +- `docs/solutions/best-practices/template-gallery-page-collections-patterns-nes1539.md` Pattern 3 — `mountedRef` + `guardedClose`. +- `apps/journeys-admin/src/components/TemplateGalleryPageList/useCollectionMutations/` — orchestration extraction pattern if the menu item grows past ~600 lines (then extract a `useCopyToCollection` hook). + +**Test scenarios:** +- Happy path: clicking the menu item opens the dialog and calls `setHasOpenDialog(true)`. +- Happy path (no translation): submitting fires `journeyDuplicate` then `templateGalleryPageAssignJourney` with the duplicated journey's id and the selected collection id; on success the dialog shows the Done state and a `GetAdminJourneys` refetch is issued. +- Happy path (with translation): submitting fires `journeyDuplicate`, then `useJourneyAiTranslateSubscription` runs to completion, then `templateGalleryPageAssignJourney` runs; on overall success the Done state shows and `GetAdminJourneys` is refetched. +- Edge case: closing the dialog mid-pipeline (after `journeyDuplicate` resolved but before `assignJourney`) does not setState on the unmounted dialog (`mountedRef` guard); pipeline continues to completion in the background but does not throw. +- Edge case: clicking the menu item with the dialog already open is a no-op. +- Edge case: zero collections in the team — dialog opens (handled by U2), submit is disabled; the menu item itself still renders. +- Edge case: rapid double-click on submit fires `journeyDuplicate` only once — the in-handler single-flight guard returns early on the second click before any mutation runs. +- Edge case: `activeTeam` is null at submit time — the handler short-circuits with a "no active team" error message; no `journeyDuplicate`, `assignJourney`, or refetch is fired. +- Error path (duplicate fails): `journeyDuplicate` throws → the dialog shows `"Failed to copy the journey. Please try again."` + Done; no `assignJourney` call; no refetch (nothing was created). +- Error path (translation fails): subscription `onError` fires → the dialog shows `"An error occurred while translating."` + Done; **no** `assignJourney` call; a `GetAdminJourneys` refetch IS issued so the orphan is visible. +- Error path (assign fails, no translation): `templateGalleryPageAssignJourney` throws → dialog shows the assign-fail copy (R10, either the standard or the contingency version depending on the drag-from-All-Templates verification) + Done; a `GetAdminJourneys` refetch IS issued. +- Error path (assign fails, after successful translation): same error copy as previous; the orphan in All Templates is translated. +- Integration: after Done is clicked, the dialog calls `setHasOpenDialog(false)` and `handleCloseMenu()` so the parent `JourneyCard`'s `GalleryDialogLockContext` lock releases and DnD resumes. +- Integration: after a successful pipeline, the gallery-page query receives a `GetAdminJourneys` refetch — the test asserts the refetch was issued (mock the client's `refetchQueries`); the actual cache update of `TemplateGalleryPage.templates` is left to Apollo's normalized merge from the assign mutation's response. + +**Verification:** +- The component covers the success path and all three terminal error paths with the correct error copy and the correct refetch behavior. `mountedRef` guards prevent setState on unmount. `setHasOpenDialog` is flipped on open and close. + +--- + +- U4. **Wire `CopyToCollectionMenuItem` into `DefaultMenu` with the flag + context gate** + +**Goal:** Activate the new menu item on journey cards rendered inside a Collection, gated by `teamTemplateCollection` AND `useInCollection()`. Verify the existing menu items are unchanged. + +**Requirements:** R1, R2 + +**Dependencies:** U1, U2, U3 + +**Files:** +- Modify: `apps/journeys-admin/src/components/JourneyList/JourneyCard/JourneyCardMenu/DefaultMenu/DefaultMenu.tsx` +- Modify: `apps/journeys-admin/src/components/JourneyList/JourneyCard/JourneyCardMenu/DefaultMenu/DefaultMenu.spec.tsx` + +**Approach:** +- Import `CopyToCollectionMenuItem` and `useInCollection`. +- Read `teamTemplateCollection` via `useFlags()` (already used elsewhere in this file or imported via `@core/shared/ui/FlagsProvider`). +- Render the new item as a **sibling block** to the existing `CopyToTeamMenuItem` (not nested inside its `!isLocalTemplate` branch — the new item's gate is independent: `teamTemplateCollection === true && inCollection === true`). Pass through the same `id`, `journey`, `handleCloseMenu`, `handleKeepMounted`, and `setHasOpenDialog` props that `CopyToTeamMenuItem` receives. +- **Order:** place `CopyToCollectionMenuItem` directly **after** `CopyToTeamMenuItem` in the menu render order. Rationale: both items share the `"Copy to ..."` family naming, and grouping them sequentially keeps related actions clustered; placing the new item after the established one follows the typical "add at the end of the same family" UI convention. +- Do not change the existing `CopyToTeamMenuItem` rendering or any other menu item. + +**Test scenarios:** +- Happy path: when `teamTemplateCollection: true` and the menu is rendered inside the `InCollectionContext.Provider`, the new `"Copy to collection..."` item is in the rendered menu (asserted via `testId`). +- Edge case: when `teamTemplateCollection: false` and the menu is rendered inside the provider, the new item is NOT in the menu. +- Edge case: when `teamTemplateCollection: true` and the menu is rendered OUTSIDE the provider (e.g., in `ActiveJourneyList`'s usage path), the new item is NOT in the menu. +- Integration: the existing `CopyToTeamMenuItem` ("Copy to ...") renders unchanged in all four `(flag, in-collection)` combinations — i.e., adding the new item does not affect the existing one. + +**Verification:** +- `DefaultMenu.spec.tsx` snapshots / assertions show the new item appearing only under the `(flag: true, inCollection: true)` combination, with no regressions in any other menu item. + +--- + +## System-Wide Impact + +- **Interaction graph:** The new item lives on the same `DefaultMenu` consumed by every `JourneyCard` in the admin app. Existing menu items (`CopyToTeamMenuItem`, `DuplicateJourneyMenuItem`, etc.) are unchanged; the new item is purely additive. The new dialog hooks into `GalleryDialogLockContext` via `setHasOpenDialog` to pause DnD while open — the same pattern other gallery-page dialogs use. +- **Error propagation:** Errors from the three pipeline mutations/subscription are captured locally in the menu item and surfaced inline in the dialog. They do not bubble to global snackbars (deliberate, per R8–R10). The exception is success — an optional success snackbar is allowed but not required. +- **State lifecycle risks:** + - Mid-pipeline unmount is guarded by `mountedRef` so background steps don't setState on a closed dialog. + - Partial-write hazard: a duplicated-but-not-assigned (or duplicated-translated-not-assigned) journey lives as an orphan in All Templates. The refetch ensures it is visible there. No cleanup is performed. + - Cache hazard: `useJourneyDuplicateMutation`'s `update` skips `template: true` reads, so the gallery-page's `journeyById` is stale until the explicit `GetAdminJourneys` refetch runs. The plan addresses this in U3. +- **API surface parity:** Other journey-card menu items remain unchanged. The new item adds to the menu but does not modify shared menu chrome. +- **Integration coverage:** Cross-layer scenarios that unit tests alone won't fully prove: + - "Submit → duplicate → translate → assign → refetch → new card appears in target Collection card without manual refresh" — verifiable via a manual smoke run in the worktree. + - "Translation fails → orphan visible in All Templates after the refetch" — verifiable manually. + - "DnD on the gallery page is paused while the dialog is open" — verifiable manually. +- **Unchanged invariants:** + - `CopyToTeamMenuItem` (the cross-team copy) is not modified. + - The shared `CopyToTeamDialog` is not modified. + - No backend mutations are added or modified; the GraphQL schema is unchanged. + - The `teamTemplateCollection` flag continues to gate all template-gallery-page UI; default-off behavior is preserved. + +--- + +## Risks & Dependencies + +| Risk | Mitigation | +|---|---| +| New journey is invisible in the target Collection card because `journeyDuplicate`'s `update` skips `template: true` reads (the gallery-page's `journeyById` source). | Explicit `refetchQueries({ include: ['GetAdminJourneys'] })` after assign success **and** after assign/translation failure (orphan recovery). Test in U3 asserts the refetch fires. | +| `JourneyCard` is rendered in non-Collection contexts; a naive flag check would leak the new item there. | Dedicated `InCollectionContext` provided only inside the Collection-grid path; gate the menu item on both `teamTemplateCollection` and `useInCollection()`. U4 tests cover all four `(flag, context)` combinations. | +| Dialog unmount mid-pipeline causes setState-after-unmount. | `mountedRef` + `guardedClose` per NES-1539 Pattern 3; setup body flips `mountedRef.current = true` (Next-dev/StrictMode trap). | +| DnD on the gallery page fires while the dialog is open. | `setHasOpenDialog` plumbing into `GalleryDialogLockContext` — same pattern other gallery dialogs use. Asserted in U3 integration tests. | +| Naming collision with the existing cross-team "Copy to ..." item. | New label `"Copy to collection..."` is distinct in copy and `testId` (`"CopyToCollection"` vs `"Copy"`). | +| Translation subscription's `onData` writes to the Apollo cache while the user is still in the dialog, potentially resetting Formik. | Form fields are user-only inputs (collection, language, toggle) — they do not read journey fields. Combined with no `enableReinitialize`, the subscription's cache writes are invisible to the form. | +| `TemplateGalleryPage.templates` typename mismatch (`TemplateGalleryItem` vs `Journey`) per NES-1644 if we ever hand-write a `cache.modify` on `templates`. | Rely on the assign mutation's response shape and Apollo's normalized merge — do not hand-write `cache.modify` on `templates` in this plan. | +| `useTemplateGalleryPagesQuery` requires a non-nullable `teamId`; the active team may be null in edge cases. | U2 passes `skip: activeTeam?.id == null` to the query and treats the skipped state as a disabled `"Loading…"` row + disabled submit. U3 short-circuits the submit handler with a "no active team" message before any mutation runs. | +| Rapid double-click on submit fires multiple `journeyDuplicate` calls. | U2 disables the submit button when `loading \|\| done \|\| errorMessage != null`; U3 adds a defensive single-flight `if (loading) return` guard at the top of the submit handler. Tested in U2 and U3. | +| Translation subscription `onError` may fire on a transient SSE drop after a translated journey already exists. | U3 stops the pipeline and refetches `GetAdminJourneys` so the orphan is visible. R9 copy is generic (`"An error occurred while translating."`) — the user decides whether to delete the partial copy or use it. Terminal-vs-transient distinction is deferred to implementation (investigate the subscription's error envelope; consider a single retry before the terminal error). | +| Recovery copy ("drag it into the collection from there") in R10 assumes drag-from-All-Templates is supported. | U3 documents a pre-merge verification step. R10 carries contingency copy if the drag direction is missing; follow-up to enable drag-from-All-Templates or add a manual retry affordance on the orphan. | + +--- + +## Documentation / Operational Notes + +- No docs/runbook updates required — this is a flag-gated UI addition. +- The `teamTemplateCollection` LaunchDarkly flag must be on for the new menu item to appear. Default-off remains the production behavior until rollout. +- No monitoring or alerting changes needed; the new pipeline rides on existing mutations and subscription which are already observable. + +--- + +## Sources & References + +- **Linear ticket:** [NES-1637 — Add "Copy to collection" action to template cards in Collections](https://linear.app/jesus-film-project/issue/NES-1637) — canonical scope and acceptance criteria live in the appended "Update — 2026-05-21 — Scope pivot" section of the description. +- Related tickets: [NES-1539](https://linear.app/jesus-film-project/issue/NES-1539) (publish flow, Collection card More menu), [NES-1547](https://linear.app/jesus-film-project/issue/NES-1547) (backend mutations), [NES-1548](https://linear.app/jesus-film-project/issue/NES-1548) (gallery management menu), [NES-1644](https://linear.app/jesus-film-project/issue/NES-1644) (template gallery preview revalidate + custom-domain gate — base commit of this worktree). +- Related code: + - `apps/journeys-admin/src/components/Team/CopyToTeamMenuItem/` + - `libs/journeys/ui/src/components/CopyToTeamDialog/` + - `apps/journeys-admin/src/components/TemplateGalleryPageList/` + - `apps/journeys-admin/src/libs/useTemplateGalleryPageAssignJourneyMutation/` + - `libs/journeys/ui/src/libs/useJourneyDuplicateMutation/` + - `libs/journeys/ui/src/libs/useJourneyAiTranslateSubscription/` +- Institutional learnings: + - `docs/solutions/best-practices/template-gallery-page-collections-patterns-nes1539.md` + - `docs/solutions/best-practices/local-template-dialog-consolidation-patterns-nes1543.md` + - `docs/solutions/runtime-errors/yoga-response-cache-null-stickiness-and-zombie-process-debugging-nes1644.md` + - `docs/solutions/logic-errors/response-cache-empty-list-invalidation-2026-05-10.md` From b912c88dcdabe579e8c2970d5c8fac9cae703fce Mon Sep 17 00:00:00 2001 From: Siyang Date: Thu, 21 May 2026 12:37:57 +1200 Subject: [PATCH 06/14] fix(NES-1637): thread collection title through onSubmit so success copy echoes target --- .../CopyToCollectionDialog/CopyToCollectionDialog.spec.tsx | 2 ++ .../CopyToCollectionDialog/CopyToCollectionDialog.tsx | 3 +++ .../CopyToCollectionMenuItem.spec.tsx | 6 ++++++ .../CopyToCollectionMenuItem/CopyToCollectionMenuItem.tsx | 7 +++++-- 4 files changed, 16 insertions(+), 2 deletions(-) diff --git a/apps/journeys-admin/src/components/TemplateGalleryPageList/CopyToCollectionDialog/CopyToCollectionDialog.spec.tsx b/apps/journeys-admin/src/components/TemplateGalleryPageList/CopyToCollectionDialog/CopyToCollectionDialog.spec.tsx index bea40cf13e7..94cf0e85747 100644 --- a/apps/journeys-admin/src/components/TemplateGalleryPageList/CopyToCollectionDialog/CopyToCollectionDialog.spec.tsx +++ b/apps/journeys-admin/src/components/TemplateGalleryPageList/CopyToCollectionDialog/CopyToCollectionDialog.spec.tsx @@ -232,6 +232,7 @@ describe('CopyToCollectionDialog', () => { await waitFor(() => expect(onSubmit).toHaveBeenCalledTimes(1)) expect(onSubmit).toHaveBeenCalledWith({ collectionId: 'page-1', + collectionTitle: 'Featured Templates', language: undefined, showTranslation: false }) @@ -280,6 +281,7 @@ describe('CopyToCollectionDialog', () => { await waitFor(() => expect(onSubmit).toHaveBeenCalledTimes(1)) const args = onSubmit.mock.calls[0][0] expect(args.collectionId).toBe('page-1') + expect(args.collectionTitle).toBe('Featured Templates') expect(args.showTranslation).toBe(true) expect(args.language).toMatchObject({ id: '496' }) }) diff --git a/apps/journeys-admin/src/components/TemplateGalleryPageList/CopyToCollectionDialog/CopyToCollectionDialog.tsx b/apps/journeys-admin/src/components/TemplateGalleryPageList/CopyToCollectionDialog/CopyToCollectionDialog.tsx index 203917feaca..f763386cf4c 100644 --- a/apps/journeys-admin/src/components/TemplateGalleryPageList/CopyToCollectionDialog/CopyToCollectionDialog.tsx +++ b/apps/journeys-admin/src/components/TemplateGalleryPageList/CopyToCollectionDialog/CopyToCollectionDialog.tsx @@ -45,6 +45,7 @@ export interface CopyToCollectionDialogProps { onClose: () => void onSubmit: (values: { collectionId: string + collectionTitle: string language?: JourneyLanguage showTranslation: boolean }) => void @@ -183,8 +184,10 @@ export function CopyToCollectionDialog({ values: FormValues, { resetForm }: FormikHelpers ): Promise { + const selectedPage = pages.find((page) => page.id === values.collectionSelect) onSubmit({ collectionId: values.collectionSelect, + collectionTitle: selectedPage?.title ?? '', language: values.showTranslation ? values.languageSelect : undefined, showTranslation: values.showTranslation }) diff --git a/apps/journeys-admin/src/components/TemplateGalleryPageList/CopyToCollectionMenuItem/CopyToCollectionMenuItem.spec.tsx b/apps/journeys-admin/src/components/TemplateGalleryPageList/CopyToCollectionMenuItem/CopyToCollectionMenuItem.spec.tsx index 5b925da4bf4..e04b76da6db 100644 --- a/apps/journeys-admin/src/components/TemplateGalleryPageList/CopyToCollectionMenuItem/CopyToCollectionMenuItem.spec.tsx +++ b/apps/journeys-admin/src/components/TemplateGalleryPageList/CopyToCollectionMenuItem/CopyToCollectionMenuItem.spec.tsx @@ -73,6 +73,7 @@ jest.mock( onClose: () => void onSubmit: (values: { collectionId: string + collectionTitle: string language?: { id: string } showTranslation: boolean }) => void @@ -96,6 +97,7 @@ jest.mock( onClick={(): void => props.onSubmit({ collectionId: 'collection-1', + collectionTitle: 'Featured Templates', language: undefined, showTranslation: false }) @@ -109,6 +111,7 @@ jest.mock( onClick={(): void => props.onSubmit({ collectionId: 'collection-1', + collectionTitle: 'Featured Templates', language: { id: '528' }, showTranslation: true }) @@ -271,6 +274,9 @@ describe('CopyToCollectionMenuItem', () => { await waitFor(() => expect(screen.getByTestId('DialogDone')).toHaveTextContent('true') ) + expect( + screen.getByTestId('DialogSelectedCollection') + ).toHaveTextContent('Featured Templates') }) it('happy path (with translation) — duplicate, subscription onComplete triggers assign, refetch issued', async () => { diff --git a/apps/journeys-admin/src/components/TemplateGalleryPageList/CopyToCollectionMenuItem/CopyToCollectionMenuItem.tsx b/apps/journeys-admin/src/components/TemplateGalleryPageList/CopyToCollectionMenuItem/CopyToCollectionMenuItem.tsx index 4109cec75b2..89f5d7d0c10 100644 --- a/apps/journeys-admin/src/components/TemplateGalleryPageList/CopyToCollectionMenuItem/CopyToCollectionMenuItem.tsx +++ b/apps/journeys-admin/src/components/TemplateGalleryPageList/CopyToCollectionMenuItem/CopyToCollectionMenuItem.tsx @@ -130,6 +130,7 @@ export function CopyToCollectionMenuItem({ const handleSubmit = async (values: { collectionId: string + collectionTitle: string language?: JourneyLanguage showTranslation: boolean }): Promise => { @@ -151,7 +152,10 @@ export function CopyToCollectionMenuItem({ } safeSetLoading(true) - if (mountedRef.current) setErrorMessage(null) + if (mountedRef.current) { + setErrorMessage(null) + setSelectedCollectionTitle(values.collectionTitle) + } let duplicatedId: string | null = null try { @@ -254,7 +258,6 @@ export function CopyToCollectionMenuItem({ isTranslating={translationVariables != null} onClose={guardedClose} onSubmit={(values) => { - setSelectedCollectionTitle(undefined) void handleSubmit(values) }} /> From 2f6b4dd54e0e4bddb06ae0f77cf25d0f6eacb8b5 Mon Sep 17 00:00:00 2001 From: Siyang Date: Thu, 21 May 2026 12:37:58 +1200 Subject: [PATCH 07/14] refactor(NES-1637): show Copy-to-team and Copy-to-collection on all templates; drop InCollectionContext --- .../DefaultMenu/DefaultMenu.spec.tsx | 65 +++++++++---------- .../DefaultMenu/DefaultMenu.tsx | 23 +++---- .../InCollectionContext.spec.tsx | 43 ------------ .../InCollectionContext.ts | 29 --------- .../InCollectionContext/index.ts | 1 - .../TemplateGalleryPageList.tsx | 23 +++---- 6 files changed, 49 insertions(+), 135 deletions(-) delete mode 100644 apps/journeys-admin/src/components/TemplateGalleryPageList/InCollectionContext/InCollectionContext.spec.tsx delete mode 100644 apps/journeys-admin/src/components/TemplateGalleryPageList/InCollectionContext/InCollectionContext.ts delete mode 100644 apps/journeys-admin/src/components/TemplateGalleryPageList/InCollectionContext/index.ts diff --git a/apps/journeys-admin/src/components/JourneyList/JourneyCard/JourneyCardMenu/DefaultMenu/DefaultMenu.spec.tsx b/apps/journeys-admin/src/components/JourneyList/JourneyCard/JourneyCardMenu/DefaultMenu/DefaultMenu.spec.tsx index a7a29fb0111..cd3c6ab8597 100644 --- a/apps/journeys-admin/src/components/JourneyList/JourneyCard/JourneyCardMenu/DefaultMenu/DefaultMenu.spec.tsx +++ b/apps/journeys-admin/src/components/JourneyList/JourneyCard/JourneyCardMenu/DefaultMenu/DefaultMenu.spec.tsx @@ -19,7 +19,6 @@ import { } from '../../../../../../__generated__/globalTypes' import { GET_CURRENT_USER } from '../../../../../libs/useCurrentUserLazyQuery' import { getCustomDomainMock } from '../../../../../libs/useCustomDomainsQuery/useCustomDomainsQuery.mock' -import { InCollectionContext } from '../../../../TemplateGalleryPageList/InCollectionContext' import { GET_JOURNEY_WITH_USER_ROLES } from './DefaultMenu' @@ -1093,12 +1092,12 @@ describe('DefaultMenu', () => { }) describe('Copy to collection menu item (NES-1637)', () => { - function renderWithFlagAndContext({ + function renderWithFlagAndTemplate({ flag, - inCollection + template }: { flag: boolean - inCollection: boolean + template: boolean }) { return render( { > - - - - - + + + ) } - it('renders "Copy to collection..." when flag is on AND inCollection is true', async () => { - const { getByTestId, getByRole } = renderWithFlagAndContext({ + it('renders "Copy to collection..." when flag is on AND template is true', async () => { + const { getByTestId, getByRole } = renderWithFlagAndTemplate({ flag: true, - inCollection: true + template: true }) await waitFor(() => expect( getByTestId('JourneysAdminMenuItemCopyToCollection') ).toBeInTheDocument() ) - // Existing CopyToTeamMenuItem still renders unchanged. + // CopyToTeamMenuItem also renders — always shown now, no isLocalTemplate gate. expect(getByRole('menuitem', { name: 'Copy to ...' })).toBeInTheDocument() }) - it('does NOT render "Copy to collection..." when flag is off but inCollection is true', async () => { - const { queryByTestId, getByRole } = renderWithFlagAndContext({ + it('does NOT render "Copy to collection..." when flag is off but template is true', async () => { + const { queryByTestId, getByRole } = renderWithFlagAndTemplate({ flag: false, - inCollection: true + template: true }) - // Wait for the menu to settle (other async menu items resolve). await waitFor(() => expect( getByRole('menuitem', { name: 'Copy to ...' }) @@ -1163,10 +1160,10 @@ describe('DefaultMenu', () => { ).not.toBeInTheDocument() }) - it('does NOT render "Copy to collection..." when flag is on but inCollection is false', async () => { - const { queryByTestId, getByRole } = renderWithFlagAndContext({ + it('does NOT render "Copy to collection..." when flag is on but template is false', async () => { + const { queryByTestId, getByRole } = renderWithFlagAndTemplate({ flag: true, - inCollection: false + template: false }) await waitFor(() => expect( @@ -1178,10 +1175,10 @@ describe('DefaultMenu', () => { ).not.toBeInTheDocument() }) - it('does NOT render "Copy to collection..." when flag is off and inCollection is false (sanity)', async () => { - const { queryByTestId, getByRole } = renderWithFlagAndContext({ + it('does NOT render "Copy to collection..." when flag is off and template is false (sanity)', async () => { + const { queryByTestId, getByRole } = renderWithFlagAndTemplate({ flag: false, - inCollection: false + template: false }) await waitFor(() => expect( diff --git a/apps/journeys-admin/src/components/JourneyList/JourneyCard/JourneyCardMenu/DefaultMenu/DefaultMenu.tsx b/apps/journeys-admin/src/components/JourneyList/JourneyCard/JourneyCardMenu/DefaultMenu/DefaultMenu.tsx index cc84bd6b3bd..f49959ddc88 100644 --- a/apps/journeys-admin/src/components/JourneyList/JourneyCard/JourneyCardMenu/DefaultMenu/DefaultMenu.tsx +++ b/apps/journeys-admin/src/components/JourneyList/JourneyCard/JourneyCardMenu/DefaultMenu/DefaultMenu.tsx @@ -38,7 +38,6 @@ import { ShareItem } from '../../../../Editor/Toolbar/Items/ShareItem/ShareItem' import { MenuItem } from '../../../../MenuItem' import { CopyToTeamMenuItem } from '../../../../Team/CopyToTeamMenuItem/CopyToTeamMenuItem' import { CopyToCollectionMenuItem } from '../../../../TemplateGalleryPageList/CopyToCollectionMenuItem' -import { useInCollection } from '../../../../TemplateGalleryPageList/InCollectionContext' import { DuplicateJourneyMenuItem } from '../DuplicateJourneyMenuItem' import { ArchiveJourney } from './ArchiveJourney' @@ -121,7 +120,6 @@ export function DefaultMenu({ const { t } = useTranslation('apps-journeys-admin') const { activeTeam } = useTeam() const { teamTemplateCollection } = useFlags() - const inCollection = useInCollection() const { data: userRoleData } = useUserRoleQuery() const { refetchTemplateStats } = useTemplateFamilyStatsAggregateLazyQuery() const { hostname } = useCustomDomainsQuery({ @@ -198,9 +196,6 @@ export function DefaultMenu({ const cantManageJourney = !canManageJourney - const isLocalTemplate = - journey?.template === true && journey?.team?.id !== 'jfp-team' - if (hasCurrentUser && isAnonymousUser) { return <> } @@ -296,16 +291,14 @@ export function DefaultMenu({ )} - {!isLocalTemplate && ( - - )} - {teamTemplateCollection === true && inCollection && ( + + {teamTemplateCollection === true && template === true && ( {inCollection ? 'true' : 'false'} -} - -describe('InCollectionContext', () => { - describe('useInCollection', () => { - it('returns true when consumer is rendered inside a provider', () => { - render( - - - - ) - - expect(screen.getByTestId('probe')).toHaveTextContent('true') - }) - - it('returns false when consumer is rendered outside any provider', () => { - render() - - expect(screen.getByTestId('probe')).toHaveTextContent('false') - }) - - it('does not throw and resolves to the closest provider value when nested', () => { - render( - - - - - - - ) - - expect(screen.getByTestId('outer')).toHaveTextContent('false') - expect(screen.getByTestId('inner')).toHaveTextContent('true') - }) - }) -}) diff --git a/apps/journeys-admin/src/components/TemplateGalleryPageList/InCollectionContext/InCollectionContext.ts b/apps/journeys-admin/src/components/TemplateGalleryPageList/InCollectionContext/InCollectionContext.ts deleted file mode 100644 index 5da0c2374f4..00000000000 --- a/apps/journeys-admin/src/components/TemplateGalleryPageList/InCollectionContext/InCollectionContext.ts +++ /dev/null @@ -1,29 +0,0 @@ -import { createContext, useContext } from 'react' - -/** - * Lets descendant components (specifically `JourneyCard` and its menu) tell - * when they are rendered inside a Collection card on the template gallery - * page, so the menu can gate the "Copy to collection…" action to that - * subtree only. - * - * `JourneyCard` is reused across at least five contexts - * (`ActiveJourneyList`, `ArchivedJourneyList`, `TrashedJourneyList`, - * All-Templates, Collections); a context-based gate is purely additive - * (no prop-signature changes through `JourneyCard → JourneyCardMenu → - * DefaultMenu`) and read at the menu-item site via `useInCollection()`. - * - * The context is intentionally opt-in: the provider is only mounted by - * `TemplateGalleryPageList` around the Collection-grid path, leaving the - * All-Templates path and every other consumer of `JourneyCard` - * uncovered. `useInCollection()` returns `false` when no provider is in - * the tree. - * - * `useGalleryDialogLock` cannot be reused for this purpose because the - * existing lock provider wraps both the Collections grid AND the - * All-Templates grid, so it cannot distinguish the two contexts. - */ -export const InCollectionContext = createContext(false) - -export function useInCollection(): boolean { - return useContext(InCollectionContext) -} diff --git a/apps/journeys-admin/src/components/TemplateGalleryPageList/InCollectionContext/index.ts b/apps/journeys-admin/src/components/TemplateGalleryPageList/InCollectionContext/index.ts deleted file mode 100644 index 11d48a2b3f1..00000000000 --- a/apps/journeys-admin/src/components/TemplateGalleryPageList/InCollectionContext/index.ts +++ /dev/null @@ -1 +0,0 @@ -export { InCollectionContext, useInCollection } from './InCollectionContext' diff --git a/apps/journeys-admin/src/components/TemplateGalleryPageList/TemplateGalleryPageList.tsx b/apps/journeys-admin/src/components/TemplateGalleryPageList/TemplateGalleryPageList.tsx index 4c34e1a3327..ee1a1a52a72 100644 --- a/apps/journeys-admin/src/components/TemplateGalleryPageList/TemplateGalleryPageList.tsx +++ b/apps/journeys-admin/src/components/TemplateGalleryPageList/TemplateGalleryPageList.tsx @@ -57,7 +57,6 @@ import { GalleryDialogLockContext, GalleryDialogLockContextValue } from './GalleryDialogLockContext' -import { InCollectionContext } from './InCollectionContext' import { useCollectionMutations } from './useCollectionMutations' import { useDragEndHandler } from './useDragEndHandler' @@ -532,18 +531,16 @@ export function TemplateGalleryPageList({ : null } > - - - + ))} From b8942bbea425f8bb7072d5637be6dad8aa1edacc Mon Sep 17 00:00:00 2001 From: Siyang Date: Thu, 21 May 2026 13:52:22 +1200 Subject: [PATCH 08/14] revert(NES-1637): restore !isLocalTemplate gate on Copy-to-team menu item Removing the gate in 2f6b4dd54 was out of scope and regressed PR #8510's deliberate local-template design (Copy-to-team is hidden on the active team's own templates by intent). Restore the gate; keep the Copy-to-collection visibility change since that flow is same-team only and doesn't conflict. --- .../DefaultMenu/DefaultMenu.spec.tsx | 4 +++- .../DefaultMenu/DefaultMenu.tsx | 19 ++++++++++++------- 2 files changed, 15 insertions(+), 8 deletions(-) diff --git a/apps/journeys-admin/src/components/JourneyList/JourneyCard/JourneyCardMenu/DefaultMenu/DefaultMenu.spec.tsx b/apps/journeys-admin/src/components/JourneyList/JourneyCard/JourneyCardMenu/DefaultMenu/DefaultMenu.spec.tsx index cd3c6ab8597..d38f8e65a01 100644 --- a/apps/journeys-admin/src/components/JourneyList/JourneyCard/JourneyCardMenu/DefaultMenu/DefaultMenu.spec.tsx +++ b/apps/journeys-admin/src/components/JourneyList/JourneyCard/JourneyCardMenu/DefaultMenu/DefaultMenu.spec.tsx @@ -1141,7 +1141,9 @@ describe('DefaultMenu', () => { getByTestId('JourneysAdminMenuItemCopyToCollection') ).toBeInTheDocument() ) - // CopyToTeamMenuItem also renders — always shown now, no isLocalTemplate gate. + // CopyToTeamMenuItem also renders in this test because the mock omits + // `journey.team`, so `isLocalTemplate` evaluates false (the gate from PR + // #8510 only suppresses the item for the active team's own templates). expect(getByRole('menuitem', { name: 'Copy to ...' })).toBeInTheDocument() }) diff --git a/apps/journeys-admin/src/components/JourneyList/JourneyCard/JourneyCardMenu/DefaultMenu/DefaultMenu.tsx b/apps/journeys-admin/src/components/JourneyList/JourneyCard/JourneyCardMenu/DefaultMenu/DefaultMenu.tsx index f49959ddc88..a0adb954546 100644 --- a/apps/journeys-admin/src/components/JourneyList/JourneyCard/JourneyCardMenu/DefaultMenu/DefaultMenu.tsx +++ b/apps/journeys-admin/src/components/JourneyList/JourneyCard/JourneyCardMenu/DefaultMenu/DefaultMenu.tsx @@ -196,6 +196,9 @@ export function DefaultMenu({ const cantManageJourney = !canManageJourney + const isLocalTemplate = + journey?.template === true && journey?.team?.id !== 'jfp-team' + if (hasCurrentUser && isAnonymousUser) { return <> } @@ -291,13 +294,15 @@ export function DefaultMenu({ )} - + {!isLocalTemplate && ( + + )} {teamTemplateCollection === true && template === true && ( Date: Thu, 21 May 2026 14:15:15 +1200 Subject: [PATCH 09/14] fix(NES-1637): replace empty arrow functions with () => undefined to satisfy lint --- .../CopyToCollectionMenuItem.spec.tsx | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/apps/journeys-admin/src/components/TemplateGalleryPageList/CopyToCollectionMenuItem/CopyToCollectionMenuItem.spec.tsx b/apps/journeys-admin/src/components/TemplateGalleryPageList/CopyToCollectionMenuItem/CopyToCollectionMenuItem.spec.tsx index e04b76da6db..aa340ccf855 100644 --- a/apps/journeys-admin/src/components/TemplateGalleryPageList/CopyToCollectionMenuItem/CopyToCollectionMenuItem.spec.tsx +++ b/apps/journeys-admin/src/components/TemplateGalleryPageList/CopyToCollectionMenuItem/CopyToCollectionMenuItem.spec.tsx @@ -316,7 +316,7 @@ describe('CopyToCollectionMenuItem', () => { it('rapid double-click on submit — duplicate is called only once (single-flight)', async () => { let resolveDuplicate: ( value: { data?: { journeyDuplicate?: { id: string } | null } } - ) => void = () => {} + ) => void = () => undefined duplicate.mockImplementation( () => new Promise((resolve) => { @@ -459,7 +459,7 @@ describe('CopyToCollectionMenuItem', () => { it('unmount mid-pipeline — does not throw setState-after-unmount', async () => { let resolveDuplicate: ( value: { data?: { journeyDuplicate?: { id: string } | null } } - ) => void = () => {} + ) => void = () => undefined duplicate.mockImplementation( () => new Promise((resolve) => { @@ -467,7 +467,9 @@ describe('CopyToCollectionMenuItem', () => { }) ) - const errorSpy = jest.spyOn(console, 'error').mockImplementation(() => {}) + const errorSpy = jest + .spyOn(console, 'error') + .mockImplementation(() => undefined) const { unmount } = renderItem() fireEvent.click(screen.getByRole('menuitem')) From 3e0128339a7a987837fd0bfaba4facbce5fbd703 Mon Sep 17 00:00:00 2001 From: "autofix-ci[bot]" <114827586+autofix-ci[bot]@users.noreply.github.com> Date: Thu, 21 May 2026 02:22:24 +0000 Subject: [PATCH 10/14] fix: lint issues --- .../CopyToCollectionDialog.spec.tsx | 36 +++- .../CopyToCollectionDialog.tsx | 13 +- .../CopyToCollectionMenuItem.spec.tsx | 192 +++++++++--------- ...nes-1637-copy-to-collection-action-plan.md | 49 +++-- 4 files changed, 159 insertions(+), 131 deletions(-) diff --git a/apps/journeys-admin/src/components/TemplateGalleryPageList/CopyToCollectionDialog/CopyToCollectionDialog.spec.tsx b/apps/journeys-admin/src/components/TemplateGalleryPageList/CopyToCollectionDialog/CopyToCollectionDialog.spec.tsx index 94cf0e85747..c0987fe4d7f 100644 --- a/apps/journeys-admin/src/components/TemplateGalleryPageList/CopyToCollectionDialog/CopyToCollectionDialog.spec.tsx +++ b/apps/journeys-admin/src/components/TemplateGalleryPageList/CopyToCollectionDialog/CopyToCollectionDialog.spec.tsx @@ -203,7 +203,9 @@ describe('CopyToCollectionDialog', () => { mocks: [pagesMock, languagesMock], props: { open: false } }) - expect(screen.queryByTestId('CopyToCollectionDialog')).not.toBeInTheDocument() + expect( + screen.queryByTestId('CopyToCollectionDialog') + ).not.toBeInTheDocument() }) it('selecting a collection enables submit and submits with the chosen id', async () => { @@ -294,9 +296,7 @@ describe('CopyToCollectionDialog', () => { fireEvent.mouseDown( screen.getByRole('combobox', { name: 'Select Collection' }) ) - const emptyRow = await screen.findByTestId( - 'CopyToCollectionDialogEmptyRow' - ) + const emptyRow = await screen.findByTestId('CopyToCollectionDialogEmptyRow') expect(emptyRow).toHaveTextContent('No collections available') expect(emptyRow).toHaveAttribute('aria-disabled', 'true') @@ -419,7 +419,9 @@ describe('CopyToCollectionDialog', () => { ) // While loading, the wrapper hides the submit button entirely. - expect(screen.queryByRole('button', { name: 'Copy' })).not.toBeInTheDocument() + expect( + screen.queryByRole('button', { name: 'Copy' }) + ).not.toBeInTheDocument() expect(onSubmit).toHaveBeenCalledTimes(1) }) @@ -440,8 +442,12 @@ describe('CopyToCollectionDialog', () => { ) // The terminal-state Dialog should only show a Done button, no Cancel. - expect(screen.queryByRole('button', { name: 'Cancel' })).not.toBeInTheDocument() - expect(screen.queryByRole('button', { name: 'Copy' })).not.toBeInTheDocument() + expect( + screen.queryByRole('button', { name: 'Cancel' }) + ).not.toBeInTheDocument() + expect( + screen.queryByRole('button', { name: 'Copy' }) + ).not.toBeInTheDocument() const doneButton = screen.getByRole('button', { name: 'Done' }) fireEvent.click(doneButton) @@ -463,8 +469,12 @@ describe('CopyToCollectionDialog', () => { expect(status).toHaveAttribute('aria-live', 'polite') expect(status).toHaveTextContent('Copied to Featured Templates.') - expect(screen.queryByRole('button', { name: 'Copy' })).not.toBeInTheDocument() - expect(screen.queryByRole('button', { name: 'Cancel' })).not.toBeInTheDocument() + expect( + screen.queryByRole('button', { name: 'Copy' }) + ).not.toBeInTheDocument() + expect( + screen.queryByRole('button', { name: 'Cancel' }) + ).not.toBeInTheDocument() fireEvent.click(screen.getByRole('button', { name: 'Done' })) expect(onClose).toHaveBeenCalledTimes(1) @@ -477,8 +487,12 @@ describe('CopyToCollectionDialog', () => { props: { onSubmit, onClose, loading: true } }) - expect(screen.queryByRole('button', { name: 'Copy' })).not.toBeInTheDocument() - expect(screen.queryByRole('button', { name: 'Cancel' })).not.toBeInTheDocument() + expect( + screen.queryByRole('button', { name: 'Copy' }) + ).not.toBeInTheDocument() + expect( + screen.queryByRole('button', { name: 'Cancel' }) + ).not.toBeInTheDocument() expect(onSubmit).not.toHaveBeenCalled() expect(onClose).not.toHaveBeenCalled() }) diff --git a/apps/journeys-admin/src/components/TemplateGalleryPageList/CopyToCollectionDialog/CopyToCollectionDialog.tsx b/apps/journeys-admin/src/components/TemplateGalleryPageList/CopyToCollectionDialog/CopyToCollectionDialog.tsx index f763386cf4c..0024ea95d42 100644 --- a/apps/journeys-admin/src/components/TemplateGalleryPageList/CopyToCollectionDialog/CopyToCollectionDialog.tsx +++ b/apps/journeys-admin/src/components/TemplateGalleryPageList/CopyToCollectionDialog/CopyToCollectionDialog.tsx @@ -87,13 +87,8 @@ export function CopyToCollectionDialog({ const { activeTeam } = useTeam() const teamId = activeTeam?.id const skipQuery = teamId == null - const { - data: pagesData, - loading: pagesLoading - } = useTemplateGalleryPagesQuery( - { teamId: teamId ?? '' }, - { skip: skipQuery } - ) + const { data: pagesData, loading: pagesLoading } = + useTemplateGalleryPagesQuery({ teamId: teamId ?? '' }, { skip: skipQuery }) const { data: languagesData, loading: languagesLoading } = useLanguagesQuery({ languageId: '529', @@ -184,7 +179,9 @@ export function CopyToCollectionDialog({ values: FormValues, { resetForm }: FormikHelpers ): Promise { - const selectedPage = pages.find((page) => page.id === values.collectionSelect) + const selectedPage = pages.find( + (page) => page.id === values.collectionSelect + ) onSubmit({ collectionId: values.collectionSelect, collectionTitle: selectedPage?.title ?? '', diff --git a/apps/journeys-admin/src/components/TemplateGalleryPageList/CopyToCollectionMenuItem/CopyToCollectionMenuItem.spec.tsx b/apps/journeys-admin/src/components/TemplateGalleryPageList/CopyToCollectionMenuItem/CopyToCollectionMenuItem.spec.tsx index aa340ccf855..6404123dc73 100644 --- a/apps/journeys-admin/src/components/TemplateGalleryPageList/CopyToCollectionMenuItem/CopyToCollectionMenuItem.spec.tsx +++ b/apps/journeys-admin/src/components/TemplateGalleryPageList/CopyToCollectionMenuItem/CopyToCollectionMenuItem.spec.tsx @@ -1,11 +1,5 @@ import { MockedProvider } from '@apollo/client/testing' -import { - act, - fireEvent, - render, - screen, - waitFor -} from '@testing-library/react' +import { act, fireEvent, render, screen, waitFor } from '@testing-library/react' import { SnackbarProvider } from 'notistack' import { useJourneyAiTranslateSubscription } from '@core/journeys/ui/useJourneyAiTranslateSubscription' @@ -24,12 +18,9 @@ jest.mock('@core/journeys/ui/useJourneyAiTranslateSubscription', () => ({ useJourneyAiTranslateSubscription: jest.fn() })) -jest.mock( - '../../../libs/useTemplateGalleryPageAssignJourneyMutation', - () => ({ - useTemplateGalleryPageAssignJourneyMutation: jest.fn() - }) -) +jest.mock('../../../libs/useTemplateGalleryPageAssignJourneyMutation', () => ({ + useTemplateGalleryPageAssignJourneyMutation: jest.fn() +})) const mockRefetchQueries = jest.fn(() => Promise.resolve([])) @@ -60,77 +51,76 @@ jest.mock('./CopyToCollectionDialog.stub', () => ({}), { virtual: true }) // Replace the dialog with a controllable stub so the menu-item tests focus // purely on orchestration. The real dialog is exercised by U2's spec. -jest.mock( - '../CopyToCollectionDialog', - () => ({ - CopyToCollectionDialog: (props: { - open: boolean - loading?: boolean - errorMessage?: string - done?: boolean - selectedCollectionTitle?: string - journeyTitle?: string - onClose: () => void - onSubmit: (values: { - collectionId: string - collectionTitle: string - language?: { id: string } - showTranslation: boolean - }) => void - isTranslating?: boolean - }) => { - if (!props.open) return null - return ( -
- {String(props.loading ?? false)} - {String(props.done ?? false)} - {props.errorMessage ?? ''} - - {String(props.isTranslating ?? false)} - - - {props.selectedCollectionTitle ?? ''} - - - - -
- ) - } - }) -) +jest.mock('../CopyToCollectionDialog', () => ({ + CopyToCollectionDialog: (props: { + open: boolean + loading?: boolean + errorMessage?: string + done?: boolean + selectedCollectionTitle?: string + journeyTitle?: string + onClose: () => void + onSubmit: (values: { + collectionId: string + collectionTitle: string + language?: { id: string } + showTranslation: boolean + }) => void + isTranslating?: boolean + }) => { + if (!props.open) return null + return ( +
+ + {String(props.loading ?? false)} + + {String(props.done ?? false)} + {props.errorMessage ?? ''} + + {String(props.isTranslating ?? false)} + + + {props.selectedCollectionTitle ?? ''} + + + + +
+ ) + } +})) type DuplicateFn = jest.Mock< Promise<{ data?: { journeyDuplicate?: { id: string } | null } }>, @@ -174,9 +164,14 @@ function setupMocks(): void { })) as DuplicateFn assign = jest.fn(async () => ({})) as AssignFn - duplicateHookMock.mockReturnValue([duplicate as unknown as never, {} as never]) + duplicateHookMock.mockReturnValue([ + duplicate as unknown as never, + {} as never + ]) assignHookMock.mockReturnValue([assign as unknown as never, {} as never]) - subscriptionHookMock.mockImplementation(((opts: typeof lastSubscriptionOpts) => { + subscriptionHookMock.mockImplementation((( + opts: typeof lastSubscriptionOpts + ) => { lastSubscriptionOpts = opts return { data: undefined } as unknown as ReturnType< typeof useJourneyAiTranslateSubscription @@ -225,7 +220,8 @@ describe('CopyToCollectionMenuItem', () => { }) it('clicking the menu item opens the dialog and fires setHasOpenDialog/handleKeepMounted/handleCloseMenu', () => { - const { handleCloseMenu, setHasOpenDialog, handleKeepMounted } = renderItem() + const { handleCloseMenu, setHasOpenDialog, handleKeepMounted } = + renderItem() fireEvent.click(screen.getByRole('menuitem')) @@ -274,9 +270,9 @@ describe('CopyToCollectionMenuItem', () => { await waitFor(() => expect(screen.getByTestId('DialogDone')).toHaveTextContent('true') ) - expect( - screen.getByTestId('DialogSelectedCollection') - ).toHaveTextContent('Featured Templates') + expect(screen.getByTestId('DialogSelectedCollection')).toHaveTextContent( + 'Featured Templates' + ) }) it('happy path (with translation) — duplicate, subscription onComplete triggers assign, refetch issued', async () => { @@ -289,7 +285,9 @@ describe('CopyToCollectionMenuItem', () => { // After duplicate, translation variables must be armed; assign has NOT // fired yet — it waits for subscription onComplete. await waitFor(() => - expect(screen.getByTestId('DialogIsTranslating')).toHaveTextContent('true') + expect(screen.getByTestId('DialogIsTranslating')).toHaveTextContent( + 'true' + ) ) expect(assign).not.toHaveBeenCalled() @@ -314,9 +312,9 @@ describe('CopyToCollectionMenuItem', () => { }) it('rapid double-click on submit — duplicate is called only once (single-flight)', async () => { - let resolveDuplicate: ( - value: { data?: { journeyDuplicate?: { id: string } | null } } - ) => void = () => undefined + let resolveDuplicate: (value: { + data?: { journeyDuplicate?: { id: string } | null } + }) => void = () => undefined duplicate.mockImplementation( () => new Promise((resolve) => { @@ -457,9 +455,9 @@ describe('CopyToCollectionMenuItem', () => { }) it('unmount mid-pipeline — does not throw setState-after-unmount', async () => { - let resolveDuplicate: ( - value: { data?: { journeyDuplicate?: { id: string } | null } } - ) => void = () => undefined + let resolveDuplicate: (value: { + data?: { journeyDuplicate?: { id: string } | null } + }) => void = () => undefined duplicate.mockImplementation( () => new Promise((resolve) => { diff --git a/docs/plans/2026-05-21-001-feat-nes-1637-copy-to-collection-action-plan.md b/docs/plans/2026-05-21-001-feat-nes-1637-copy-to-collection-action-plan.md index c1d483f94f9..b8913bd5016 100644 --- a/docs/plans/2026-05-21-001-feat-nes-1637-copy-to-collection-action-plan.md +++ b/docs/plans/2026-05-21-001-feat-nes-1637-copy-to-collection-action-plan.md @@ -133,25 +133,30 @@ The template gallery page (Local Template Library, NES-1539 / NES-1547) lets pub **Dependencies:** None **Files:** + - Create: `apps/journeys-admin/src/components/TemplateGalleryPageList/InCollectionContext/InCollectionContext.ts` - Create: `apps/journeys-admin/src/components/TemplateGalleryPageList/InCollectionContext/index.ts` -- Modify: `apps/journeys-admin/src/components/TemplateGalleryPageList/CollectionCard/CollectionCard.tsx` *or* `apps/journeys-admin/src/components/TemplateGalleryPageList/TemplateGalleryPageList.tsx` (whichever site cleanly wraps only the Collection-grid path's `DraggableJourneysGrid`; do not wrap the All-Templates grid) +- Modify: `apps/journeys-admin/src/components/TemplateGalleryPageList/CollectionCard/CollectionCard.tsx` _or_ `apps/journeys-admin/src/components/TemplateGalleryPageList/TemplateGalleryPageList.tsx` (whichever site cleanly wraps only the Collection-grid path's `DraggableJourneysGrid`; do not wrap the All-Templates grid) - Test: `apps/journeys-admin/src/components/TemplateGalleryPageList/InCollectionContext/InCollectionContext.spec.tsx` **Approach:** + - The context exposes a `useInCollection()` hook returning `true` when the consuming component is under the provider, `false` (or `undefined` coerced to `false`) otherwise. - The provider is placed once, inside the Collection-grid mapping in the gallery page — the exact host (CollectionCard children slot vs. TemplateGalleryPageList) is an implementation-time decision based on the smaller diff. - The context is admin-app-internal; do not export from `libs/journeys/ui/`. **Patterns to follow:** + - `apps/journeys-admin/src/components/TemplateGalleryPageList/GalleryDialogLockContext.ts` for the file shape and naming. **Test scenarios:** + - Happy path: `useInCollection` returns `true` when the consumer is rendered inside the provider. - Happy path: `useInCollection` returns `false` when the consumer is rendered outside any provider (no provider in the subtree). - Edge case: nesting two providers should not throw; the closest provider's value wins (standard React context semantics — assert non-throw and the deepest-value behavior). **Verification:** + - `useInCollection` returns `true` only when a parent component has rendered the provider. Existing All-Templates and non-Collection list views are unaffected (the hook returns `false` there). --- @@ -165,12 +170,14 @@ The template gallery page (Local Template Library, NES-1539 / NES-1547) lets pub **Dependencies:** None (the menu item in U3 will consume it but the dialog can be built and tested independently against mocked submit handlers) **Files:** + - Create: `apps/journeys-admin/src/components/TemplateGalleryPageList/CopyToCollectionDialog/CopyToCollectionDialog.tsx` - Create: `apps/journeys-admin/src/components/TemplateGalleryPageList/CopyToCollectionDialog/index.ts` - Create: `apps/journeys-admin/src/components/TemplateGalleryPageList/CopyToCollectionDialog/CopyToCollectionDialog.spec.tsx` - Optional: `apps/journeys-admin/src/components/TemplateGalleryPageList/CopyToCollectionDialog/CopyToCollectionDialog.stories.tsx` (low-cost; add if matching `CopyToTeamDialog.stories.tsx`) **Approach:** + - Compose `TranslationDialogWrapper` for the dialog shell, submit/cancel buttons, and loading state. - Formik form with three fields: `collectionSelect: string`, `languageSelect?: JourneyLanguage`, `showTranslation: boolean`. Yup schema requires `collectionSelect` always; requires `languageSelect` only when `showTranslation === true` (same conditional shape `CopyToTeamDialog` uses). - Do **not** set `enableReinitialize` on Formik. `CopyToTeamDialog` currently sets `enableReinitialize` (around line 169 at the time of writing) — **do not carry that prop across**. Per NES-1543 Pattern 3, a subscription-driven Apollo cache write can land mid-edit and silently reset `initialValues`. If parent props change (e.g., a journey-id remount is needed), the parent should remount via `key={journey.id}` instead — per NES-1539 Pattern 2. @@ -189,10 +196,12 @@ The template gallery page (Local Template Library, NES-1539 / NES-1547) lets pub - Use `useTranslation('apps-journeys-admin')` for all user-facing strings. **Patterns to follow:** + - `libs/journeys/ui/src/components/CopyToTeamDialog/CopyToTeamDialog.tsx` — Formik shape, Yup schema, language-picker block, `TranslationDialogWrapper` composition. - `apps/journeys-admin/src/libs/useTemplateGalleryPagesQuery/` — collection dropdown source. **Test scenarios:** + - Happy path: renders with a closed initial state when `open: false`; renders the dropdown with N collections when `open: true` and the query returns N pages. - Happy path: selecting a collection enables the submit button; clicking submit calls `onSubmit({ collectionId, language: undefined, showTranslation: false })`. - Happy path: toggling translation on reveals the language picker; selecting a language and submitting passes `language` and `showTranslation: true` to `onSubmit`. @@ -209,6 +218,7 @@ The template gallery page (Local Template Library, NES-1539 / NES-1547) lets pub - Integration: closing the dialog mid-loading does not call `onSubmit` again; `onClose` is the only path out. **Verification:** + - The dialog renders, validates, and surfaces the three terminal states (loading, error, done) without depending on the menu item's orchestration logic. All branches are exercised through props alone. --- @@ -222,11 +232,13 @@ The template gallery page (Local Template Library, NES-1539 / NES-1547) lets pub **Dependencies:** U2 (dialog), U1 (context — consumed at the call site in U4 but defined here as a peer surface) **Files:** + - Create: `apps/journeys-admin/src/components/TemplateGalleryPageList/CopyToCollectionMenuItem/CopyToCollectionMenuItem.tsx` - Create: `apps/journeys-admin/src/components/TemplateGalleryPageList/CopyToCollectionMenuItem/index.ts` - Create: `apps/journeys-admin/src/components/TemplateGalleryPageList/CopyToCollectionMenuItem/CopyToCollectionMenuItem.spec.tsx` **Approach:** + - Component shape mirrors `CopyToTeamMenuItem`: renders the shared `MenuItem` (label, icon, `onClick`, `testId="CopyToCollection"`) plus the `CopyToCollectionDialog` it controls. - Props: `id?: string`, `journey?: Journey`, `handleCloseMenu: () => void`, `setHasOpenDialog?: (open: boolean) => void`, `handleKeepMounted?: () => void` — same shape as `CopyToTeamMenuItem` for symmetry. The optional `setHasOpenDialog` is wired by the parent `JourneyCard` chain (same as the team flow); we rely on the parent to provide it rather than enforcing it at this layer. - Internal state: `dialogOpen: boolean`, `loading: boolean`, `errorMessage: string | null`, `done: boolean`, `translationVariables: TranslationVars | null`, `pendingTargetCollectionId: string | null`, `mountedRef: useRef(false)`. @@ -253,7 +265,7 @@ The template gallery page (Local Template Library, NES-1539 / NES-1547) lets pub **Technical design:** -> *This illustrates the intended pipeline shape and is directional guidance for review, not implementation specification.* +> _This illustrates the intended pipeline shape and is directional guidance for review, not implementation specification._ ``` handleSubmit({ collectionId, language?, showTranslation? }): @@ -290,11 +302,13 @@ dialog close (via Done or Cancel): ``` **Patterns to follow:** + - `apps/journeys-admin/src/components/Team/CopyToTeamMenuItem/CopyToTeamMenuItem.tsx` — overall component shape, subscription gating via `translationVariables` state, `setHasOpenDialog` wiring, `handleKeepMounted` usage. - `docs/solutions/best-practices/template-gallery-page-collections-patterns-nes1539.md` Pattern 3 — `mountedRef` + `guardedClose`. - `apps/journeys-admin/src/components/TemplateGalleryPageList/useCollectionMutations/` — orchestration extraction pattern if the menu item grows past ~600 lines (then extract a `useCopyToCollection` hook). **Test scenarios:** + - Happy path: clicking the menu item opens the dialog and calls `setHasOpenDialog(true)`. - Happy path (no translation): submitting fires `journeyDuplicate` then `templateGalleryPageAssignJourney` with the duplicated journey's id and the selected collection id; on success the dialog shows the Done state and a `GetAdminJourneys` refetch is issued. - Happy path (with translation): submitting fires `journeyDuplicate`, then `useJourneyAiTranslateSubscription` runs to completion, then `templateGalleryPageAssignJourney` runs; on overall success the Done state shows and `GetAdminJourneys` is refetched. @@ -311,6 +325,7 @@ dialog close (via Done or Cancel): - Integration: after a successful pipeline, the gallery-page query receives a `GetAdminJourneys` refetch — the test asserts the refetch was issued (mock the client's `refetchQueries`); the actual cache update of `TemplateGalleryPage.templates` is left to Apollo's normalized merge from the assign mutation's response. **Verification:** + - The component covers the success path and all three terminal error paths with the correct error copy and the correct refetch behavior. `mountedRef` guards prevent setState on unmount. `setHasOpenDialog` is flipped on open and close. --- @@ -324,10 +339,12 @@ dialog close (via Done or Cancel): **Dependencies:** U1, U2, U3 **Files:** + - Modify: `apps/journeys-admin/src/components/JourneyList/JourneyCard/JourneyCardMenu/DefaultMenu/DefaultMenu.tsx` - Modify: `apps/journeys-admin/src/components/JourneyList/JourneyCard/JourneyCardMenu/DefaultMenu/DefaultMenu.spec.tsx` **Approach:** + - Import `CopyToCollectionMenuItem` and `useInCollection`. - Read `teamTemplateCollection` via `useFlags()` (already used elsewhere in this file or imported via `@core/shared/ui/FlagsProvider`). - Render the new item as a **sibling block** to the existing `CopyToTeamMenuItem` (not nested inside its `!isLocalTemplate` branch — the new item's gate is independent: `teamTemplateCollection === true && inCollection === true`). Pass through the same `id`, `journey`, `handleCloseMenu`, `handleKeepMounted`, and `setHasOpenDialog` props that `CopyToTeamMenuItem` receives. @@ -335,12 +352,14 @@ dialog close (via Done or Cancel): - Do not change the existing `CopyToTeamMenuItem` rendering or any other menu item. **Test scenarios:** + - Happy path: when `teamTemplateCollection: true` and the menu is rendered inside the `InCollectionContext.Provider`, the new `"Copy to collection..."` item is in the rendered menu (asserted via `testId`). - Edge case: when `teamTemplateCollection: false` and the menu is rendered inside the provider, the new item is NOT in the menu. - Edge case: when `teamTemplateCollection: true` and the menu is rendered OUTSIDE the provider (e.g., in `ActiveJourneyList`'s usage path), the new item is NOT in the menu. - Integration: the existing `CopyToTeamMenuItem` ("Copy to ...") renders unchanged in all four `(flag, in-collection)` combinations — i.e., adding the new item does not affect the existing one. **Verification:** + - `DefaultMenu.spec.tsx` snapshots / assertions show the new item appearing only under the `(flag: true, inCollection: true)` combination, with no regressions in any other menu item. --- @@ -368,19 +387,19 @@ dialog close (via Done or Cancel): ## Risks & Dependencies -| Risk | Mitigation | -|---|---| -| New journey is invisible in the target Collection card because `journeyDuplicate`'s `update` skips `template: true` reads (the gallery-page's `journeyById` source). | Explicit `refetchQueries({ include: ['GetAdminJourneys'] })` after assign success **and** after assign/translation failure (orphan recovery). Test in U3 asserts the refetch fires. | -| `JourneyCard` is rendered in non-Collection contexts; a naive flag check would leak the new item there. | Dedicated `InCollectionContext` provided only inside the Collection-grid path; gate the menu item on both `teamTemplateCollection` and `useInCollection()`. U4 tests cover all four `(flag, context)` combinations. | -| Dialog unmount mid-pipeline causes setState-after-unmount. | `mountedRef` + `guardedClose` per NES-1539 Pattern 3; setup body flips `mountedRef.current = true` (Next-dev/StrictMode trap). | -| DnD on the gallery page fires while the dialog is open. | `setHasOpenDialog` plumbing into `GalleryDialogLockContext` — same pattern other gallery dialogs use. Asserted in U3 integration tests. | -| Naming collision with the existing cross-team "Copy to ..." item. | New label `"Copy to collection..."` is distinct in copy and `testId` (`"CopyToCollection"` vs `"Copy"`). | -| Translation subscription's `onData` writes to the Apollo cache while the user is still in the dialog, potentially resetting Formik. | Form fields are user-only inputs (collection, language, toggle) — they do not read journey fields. Combined with no `enableReinitialize`, the subscription's cache writes are invisible to the form. | -| `TemplateGalleryPage.templates` typename mismatch (`TemplateGalleryItem` vs `Journey`) per NES-1644 if we ever hand-write a `cache.modify` on `templates`. | Rely on the assign mutation's response shape and Apollo's normalized merge — do not hand-write `cache.modify` on `templates` in this plan. | -| `useTemplateGalleryPagesQuery` requires a non-nullable `teamId`; the active team may be null in edge cases. | U2 passes `skip: activeTeam?.id == null` to the query and treats the skipped state as a disabled `"Loading…"` row + disabled submit. U3 short-circuits the submit handler with a "no active team" message before any mutation runs. | -| Rapid double-click on submit fires multiple `journeyDuplicate` calls. | U2 disables the submit button when `loading \|\| done \|\| errorMessage != null`; U3 adds a defensive single-flight `if (loading) return` guard at the top of the submit handler. Tested in U2 and U3. | -| Translation subscription `onError` may fire on a transient SSE drop after a translated journey already exists. | U3 stops the pipeline and refetches `GetAdminJourneys` so the orphan is visible. R9 copy is generic (`"An error occurred while translating."`) — the user decides whether to delete the partial copy or use it. Terminal-vs-transient distinction is deferred to implementation (investigate the subscription's error envelope; consider a single retry before the terminal error). | -| Recovery copy ("drag it into the collection from there") in R10 assumes drag-from-All-Templates is supported. | U3 documents a pre-merge verification step. R10 carries contingency copy if the drag direction is missing; follow-up to enable drag-from-All-Templates or add a manual retry affordance on the orphan. | +| Risk | Mitigation | +| -------------------------------------------------------------------------------------------------------------------------------------------------------------------- | ----------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------- | +| New journey is invisible in the target Collection card because `journeyDuplicate`'s `update` skips `template: true` reads (the gallery-page's `journeyById` source). | Explicit `refetchQueries({ include: ['GetAdminJourneys'] })` after assign success **and** after assign/translation failure (orphan recovery). Test in U3 asserts the refetch fires. | +| `JourneyCard` is rendered in non-Collection contexts; a naive flag check would leak the new item there. | Dedicated `InCollectionContext` provided only inside the Collection-grid path; gate the menu item on both `teamTemplateCollection` and `useInCollection()`. U4 tests cover all four `(flag, context)` combinations. | +| Dialog unmount mid-pipeline causes setState-after-unmount. | `mountedRef` + `guardedClose` per NES-1539 Pattern 3; setup body flips `mountedRef.current = true` (Next-dev/StrictMode trap). | +| DnD on the gallery page fires while the dialog is open. | `setHasOpenDialog` plumbing into `GalleryDialogLockContext` — same pattern other gallery dialogs use. Asserted in U3 integration tests. | +| Naming collision with the existing cross-team "Copy to ..." item. | New label `"Copy to collection..."` is distinct in copy and `testId` (`"CopyToCollection"` vs `"Copy"`). | +| Translation subscription's `onData` writes to the Apollo cache while the user is still in the dialog, potentially resetting Formik. | Form fields are user-only inputs (collection, language, toggle) — they do not read journey fields. Combined with no `enableReinitialize`, the subscription's cache writes are invisible to the form. | +| `TemplateGalleryPage.templates` typename mismatch (`TemplateGalleryItem` vs `Journey`) per NES-1644 if we ever hand-write a `cache.modify` on `templates`. | Rely on the assign mutation's response shape and Apollo's normalized merge — do not hand-write `cache.modify` on `templates` in this plan. | +| `useTemplateGalleryPagesQuery` requires a non-nullable `teamId`; the active team may be null in edge cases. | U2 passes `skip: activeTeam?.id == null` to the query and treats the skipped state as a disabled `"Loading…"` row + disabled submit. U3 short-circuits the submit handler with a "no active team" message before any mutation runs. | +| Rapid double-click on submit fires multiple `journeyDuplicate` calls. | U2 disables the submit button when `loading \|\| done \|\| errorMessage != null`; U3 adds a defensive single-flight `if (loading) return` guard at the top of the submit handler. Tested in U2 and U3. | +| Translation subscription `onError` may fire on a transient SSE drop after a translated journey already exists. | U3 stops the pipeline and refetches `GetAdminJourneys` so the orphan is visible. R9 copy is generic (`"An error occurred while translating."`) — the user decides whether to delete the partial copy or use it. Terminal-vs-transient distinction is deferred to implementation (investigate the subscription's error envelope; consider a single retry before the terminal error). | +| Recovery copy ("drag it into the collection from there") in R10 assumes drag-from-All-Templates is supported. | U3 documents a pre-merge verification step. R10 carries contingency copy if the drag direction is missing; follow-up to enable drag-from-All-Templates or add a manual retry affordance on the orphan. | --- From 6524309cf38d36b5d8394a226ca3956b199ab65b Mon Sep 17 00:00:00 2001 From: Siyang Date: Thu, 21 May 2026 14:46:20 +1200 Subject: [PATCH 11/14] fix(NES-1637): apply ce-code-review best-judgment findings MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Production fixes: - Narrow Formik error type on languageSelect helperText via resolveLanguageError helper (was: unsafe `as { id?: string }` cast that swallowed top-level Yup .required() strings) - Memoise Yup schema (was reconstructed every render, forcing Formik re-validation) - Drop .nullable() so inferred Yup type matches FormValues - Remove unused translationProgress prop (dialog declared it, menu item never wired it) - Export JourneyLanguage from dialog and import in menu item (dedupe) - Inline handleDoneClick single-use wrapper - Fallback success copy to "Copied to collection." when title is missing - Drop redundant `as string` cast on collectionSelect helperText Lifecycle safety: - Gate runAssign on mountedRef BEFORE the assign mutation fires (was guarding setState only — mutation could fire on unmounted component) - Release DnD lock and null pipeline refs on unmount (was leaking the parent's GalleryDialogLockContext lock if user navigated away mid-dialog) Tests: - Add ThemeProvider to render helpers in 3 spec files (per AGENTS.md) - Add unmount-releases-DnD-lock test Plan doc: - Tag fenced code block with `text` lang --- .../DefaultMenu/DefaultMenu.spec.tsx | 41 +++++---- .../CopyToCollectionDialog.spec.tsx | 9 +- .../CopyToCollectionDialog.tsx | 92 ++++++++++--------- .../CopyToCollectionDialog/index.ts | 5 +- .../CopyToCollectionMenuItem.spec.tsx | 32 +++++-- .../CopyToCollectionMenuItem.tsx | 29 ++++-- ...nes-1637-copy-to-collection-action-plan.md | 2 +- 7 files changed, 127 insertions(+), 83 deletions(-) diff --git a/apps/journeys-admin/src/components/JourneyList/JourneyCard/JourneyCardMenu/DefaultMenu/DefaultMenu.spec.tsx b/apps/journeys-admin/src/components/JourneyList/JourneyCard/JourneyCardMenu/DefaultMenu/DefaultMenu.spec.tsx index d38f8e65a01..fc7210ba2b4 100644 --- a/apps/journeys-admin/src/components/JourneyList/JourneyCard/JourneyCardMenu/DefaultMenu/DefaultMenu.spec.tsx +++ b/apps/journeys-admin/src/components/JourneyList/JourneyCard/JourneyCardMenu/DefaultMenu/DefaultMenu.spec.tsx @@ -19,6 +19,7 @@ import { } from '../../../../../../__generated__/globalTypes' import { GET_CURRENT_USER } from '../../../../../libs/useCurrentUserLazyQuery' import { getCustomDomainMock } from '../../../../../libs/useCustomDomainsQuery/useCustomDomainsQuery.mock' +import { ThemeProvider } from '../../../../ThemeProvider' import { GET_JOURNEY_WITH_USER_ROLES } from './DefaultMenu' @@ -1108,25 +1109,27 @@ describe('DefaultMenu', () => { makeJourneyMock('journey-id') ]} > - - - - - - - + + + + + + + + + ) } diff --git a/apps/journeys-admin/src/components/TemplateGalleryPageList/CopyToCollectionDialog/CopyToCollectionDialog.spec.tsx b/apps/journeys-admin/src/components/TemplateGalleryPageList/CopyToCollectionDialog/CopyToCollectionDialog.spec.tsx index c0987fe4d7f..80849845c00 100644 --- a/apps/journeys-admin/src/components/TemplateGalleryPageList/CopyToCollectionDialog/CopyToCollectionDialog.spec.tsx +++ b/apps/journeys-admin/src/components/TemplateGalleryPageList/CopyToCollectionDialog/CopyToCollectionDialog.spec.tsx @@ -9,6 +9,7 @@ import { getLanguagesMock } from '@core/journeys/ui/useLanguagesQuery/useLanguag import { GetTemplateGalleryPages_templateGalleryPages as Page } from '../../../../__generated__/GetTemplateGalleryPages' import { TemplateGalleryPageStatus } from '../../../../__generated__/globalTypes' import { GET_TEMPLATE_GALLERY_PAGES } from '../../../libs/useTemplateGalleryPagesQuery' +import { ThemeProvider } from '../../ThemeProvider' import { CopyToCollectionDialog } from './CopyToCollectionDialog' @@ -164,9 +165,11 @@ function renderDialog({ } return render( - - - + + + + + ) } diff --git a/apps/journeys-admin/src/components/TemplateGalleryPageList/CopyToCollectionDialog/CopyToCollectionDialog.tsx b/apps/journeys-admin/src/components/TemplateGalleryPageList/CopyToCollectionDialog/CopyToCollectionDialog.tsx index 0024ea95d42..4edafdeec35 100644 --- a/apps/journeys-admin/src/components/TemplateGalleryPageList/CopyToCollectionDialog/CopyToCollectionDialog.tsx +++ b/apps/journeys-admin/src/components/TemplateGalleryPageList/CopyToCollectionDialog/CopyToCollectionDialog.tsx @@ -7,10 +7,10 @@ import Stack from '@mui/material/Stack' import Switch from '@mui/material/Switch' import TextField from '@mui/material/TextField' import Typography from '@mui/material/Typography' -import { Formik, FormikHelpers } from 'formik' +import { Formik, FormikHelpers, type FormikErrors } from 'formik' import sortBy from 'lodash/sortBy' import { useTranslation } from 'next-i18next/pages' -import { ReactElement } from 'react' +import { ReactElement, useMemo } from 'react' import { boolean, object, string } from 'yup' import { useTeam } from '@core/journeys/ui/TeamProvider' @@ -23,7 +23,7 @@ import { LanguageAutocomplete } from '@core/shared/ui/LanguageAutocomplete' import { useTemplateGalleryPagesQuery } from '../../../libs/useTemplateGalleryPagesQuery' -interface JourneyLanguage { +export interface JourneyLanguage { id: string localName?: string nativeName?: string @@ -35,6 +35,18 @@ interface FormValues { showTranslation: boolean } +// Yup may surface a top-level `.required()` error as a string OR a nested +// per-field error object — Formik types reflect both shapes. Normalise to +// a single string for MUI's `helperText`. +function resolveLanguageError( + error: string | FormikErrors | undefined, + touched: boolean | undefined +): string { + if (touched !== true || error == null) return '' + if (typeof error === 'string') return error + return error.id ?? '' +} + export interface CopyToCollectionDialogProps { open: boolean loading?: boolean @@ -49,10 +61,6 @@ export interface CopyToCollectionDialogProps { language?: JourneyLanguage showTranslation: boolean }) => void - translationProgress?: { - progress: number - message: string - } isTranslating?: boolean } @@ -80,7 +88,6 @@ export function CopyToCollectionDialog({ journeyTitle, onClose, onSubmit, - translationProgress, isTranslating = false }: CopyToCollectionDialogProps): ReactElement { const { t } = useTranslation('apps-journeys-admin') @@ -115,13 +122,9 @@ export function CopyToCollectionDialog({ const statusMessage = errorMessage != null ? errorMessage - : t('Copied to {{title}}.', { - title: selectedCollectionTitle ?? '' - }) - - const handleDoneClick = (): void => { - onClose() - } + : selectedCollectionTitle != null && selectedCollectionTitle !== '' + ? t('Copied to {{title}}.', { title: selectedCollectionTitle }) + : t('Copied to collection.') return ( @@ -154,26 +157,31 @@ export function CopyToCollectionDialog({ ) } - const baseLanguageShape = { - id: string(), - localName: string().optional(), - nativeName: string().optional() - } - - const copyToSchema = object({ - collectionSelect: string().required(t('Please select a collection')), - showTranslation: boolean().required(), - languageSelect: object(baseLanguageShape) - .nullable() - .when('showTranslation', { - is: true, - then: (schema) => - schema.required(t('Please select a language')).shape({ - id: string().required(t('Please select a language')) - }), - otherwise: (schema) => schema.nullable().optional() - }) - }) + // Memoised so Formik does not see a fresh schema reference on every + // render. Cannot hoist to module scope because `t` is hook-bound. + const copyToSchema = useMemo( + () => + object({ + collectionSelect: string().required(t('Please select a collection')), + showTranslation: boolean().required(), + // Drop `.nullable()` so the inferred Yup type matches FormValues + // (`JourneyLanguage | undefined`, never `null`). + languageSelect: object({ + id: string(), + localName: string().optional(), + nativeName: string().optional() + }) + .when('showTranslation', { + is: true, + then: (schema) => + schema.required(t('Please select a language')).shape({ + id: string().required(t('Please select a language')) + }), + otherwise: (schema) => schema.optional() + }) + }), + [t] + ) async function handleFormikSubmit( values: FormValues, @@ -253,7 +261,6 @@ export function CopyToCollectionDialog({ disabled={submitDisabled} divider={false} testId="CopyToCollectionDialog" - translationProgress={translationProgress} > @@ -263,7 +270,7 @@ export function CopyToCollectionDialog({ autoFocus error={Boolean(errors.collectionSelect)} helperText={ - (errors.collectionSelect as string) ?? + errors.collectionSelect ?? t('Journey will be copied to selected collection.') } variant="filled" @@ -353,11 +360,10 @@ export function CopyToCollectionDialog({ { void setFieldValue('languageSelect', value) }} diff --git a/apps/journeys-admin/src/components/TemplateGalleryPageList/CopyToCollectionDialog/index.ts b/apps/journeys-admin/src/components/TemplateGalleryPageList/CopyToCollectionDialog/index.ts index 1b5eeb9d987..54f0561e575 100644 --- a/apps/journeys-admin/src/components/TemplateGalleryPageList/CopyToCollectionDialog/index.ts +++ b/apps/journeys-admin/src/components/TemplateGalleryPageList/CopyToCollectionDialog/index.ts @@ -1,2 +1,5 @@ export { CopyToCollectionDialog } from './CopyToCollectionDialog' -export type { CopyToCollectionDialogProps } from './CopyToCollectionDialog' +export type { + CopyToCollectionDialogProps, + JourneyLanguage +} from './CopyToCollectionDialog' diff --git a/apps/journeys-admin/src/components/TemplateGalleryPageList/CopyToCollectionMenuItem/CopyToCollectionMenuItem.spec.tsx b/apps/journeys-admin/src/components/TemplateGalleryPageList/CopyToCollectionMenuItem/CopyToCollectionMenuItem.spec.tsx index 6404123dc73..627b0f53170 100644 --- a/apps/journeys-admin/src/components/TemplateGalleryPageList/CopyToCollectionMenuItem/CopyToCollectionMenuItem.spec.tsx +++ b/apps/journeys-admin/src/components/TemplateGalleryPageList/CopyToCollectionMenuItem/CopyToCollectionMenuItem.spec.tsx @@ -7,6 +7,7 @@ import { useJourneyDuplicateMutation } from '@core/journeys/ui/useJourneyDuplica import { GetAdminJourneys_journeys as Journey } from '../../../../__generated__/GetAdminJourneys' import { useTemplateGalleryPageAssignJourneyMutation } from '../../../libs/useTemplateGalleryPageAssignJourneyMutation' +import { ThemeProvider } from '../../ThemeProvider' import { CopyToCollectionMenuItem } from './CopyToCollectionMenuItem' @@ -197,15 +198,17 @@ function renderItem( const handleKeepMounted = overrides.handleKeepMounted ?? jest.fn() const { unmount } = render( - - - + + + + + ) return { handleCloseMenu, setHasOpenDialog, handleKeepMounted, unmount } @@ -231,6 +234,17 @@ describe('CopyToCollectionMenuItem', () => { expect(screen.getByTestId('CopyToCollectionDialogStub')).toBeInTheDocument() }) + it('releases the DnD lock on unmount even when the dialog is still open', () => { + const { setHasOpenDialog, unmount } = renderItem() + + fireEvent.click(screen.getByRole('menuitem')) + expect(setHasOpenDialog).toHaveBeenLastCalledWith(true) + + setHasOpenDialog.mockClear() + unmount() + expect(setHasOpenDialog).toHaveBeenCalledWith(false) + }) + it('clicking the menu item while the dialog is already open is a no-op (still open)', () => { const { setHasOpenDialog } = renderItem() diff --git a/apps/journeys-admin/src/components/TemplateGalleryPageList/CopyToCollectionMenuItem/CopyToCollectionMenuItem.tsx b/apps/journeys-admin/src/components/TemplateGalleryPageList/CopyToCollectionMenuItem/CopyToCollectionMenuItem.tsx index 89f5d7d0c10..87f28912bdc 100644 --- a/apps/journeys-admin/src/components/TemplateGalleryPageList/CopyToCollectionMenuItem/CopyToCollectionMenuItem.tsx +++ b/apps/journeys-admin/src/components/TemplateGalleryPageList/CopyToCollectionMenuItem/CopyToCollectionMenuItem.tsx @@ -10,13 +10,10 @@ import CopyToIcon from '@core/shared/ui/icons/CopyTo' import { GetAdminJourneys_journeys as Journey } from '../../../../__generated__/GetAdminJourneys' import { useTemplateGalleryPageAssignJourneyMutation } from '../../../libs/useTemplateGalleryPageAssignJourneyMutation' import { MenuItem } from '../../MenuItem' -import { CopyToCollectionDialog } from '../CopyToCollectionDialog' - -interface JourneyLanguage { - id: string - localName?: string - nativeName?: string -} +import { + CopyToCollectionDialog, + type JourneyLanguage +} from '../CopyToCollectionDialog' interface TranslationVars { journeyId: string @@ -85,7 +82,20 @@ export function CopyToCollectionMenuItem({ mountedRef.current = true return (): void => { mountedRef.current = false + // Release the DnD lock and null the post-translation refs in case + // the component unmounts mid-pipeline (route change, parent + // re-render). Calls are idempotent: `setHasOpenDialog?.(false)` + // is a no-op when the dialog was never opened, and the refs are + // already null at mount. Subscription teardown happens via Apollo + // unsubscribing when this hook unmounts. + setHasOpenDialog?.(false) + newJourneyIdRef.current = null + pendingTargetCollectionIdRef.current = null } + // Lifecycle effect — mount/unmount only. Capturing the latest + // `setHasOpenDialog` reference is fine because it is a prop that + // does not change across renders in normal use. + // eslint-disable-next-line react-hooks/exhaustive-deps }, []) const noActiveTeamCopy = t( @@ -110,6 +120,11 @@ export function CopyToCollectionMenuItem({ newJourneyId: string, targetCollectionId: string ): Promise => { + // Gate the network call itself on mount status — the assign + // mutation would otherwise fire after unmount (subscription + // onComplete races a closing dialog), creating a silent server-side + // orphan the user has no UI feedback about. + if (!mountedRef.current) return try { await templateGalleryPageAssignJourney({ variables: { journeyId: newJourneyId, pageId: targetCollectionId } diff --git a/docs/plans/2026-05-21-001-feat-nes-1637-copy-to-collection-action-plan.md b/docs/plans/2026-05-21-001-feat-nes-1637-copy-to-collection-action-plan.md index b8913bd5016..564a57ef932 100644 --- a/docs/plans/2026-05-21-001-feat-nes-1637-copy-to-collection-action-plan.md +++ b/docs/plans/2026-05-21-001-feat-nes-1637-copy-to-collection-action-plan.md @@ -267,7 +267,7 @@ The template gallery page (Local Template Library, NES-1539 / NES-1547) lets pub > _This illustrates the intended pipeline shape and is directional guidance for review, not implementation specification._ -``` +```text handleSubmit({ collectionId, language?, showTranslation? }): if loading: return // single-flight guard teamId = activeTeam?.id From 865bec0ab21877247b5d5559c5896c89cd6a9648 Mon Sep 17 00:00:00 2001 From: "autofix-ci[bot]" <114827586+autofix-ci[bot]@users.noreply.github.com> Date: Thu, 21 May 2026 02:49:59 +0000 Subject: [PATCH 12/14] fix: lint issues --- .../CopyToCollectionDialog.tsx | 17 ++++++++--------- 1 file changed, 8 insertions(+), 9 deletions(-) diff --git a/apps/journeys-admin/src/components/TemplateGalleryPageList/CopyToCollectionDialog/CopyToCollectionDialog.tsx b/apps/journeys-admin/src/components/TemplateGalleryPageList/CopyToCollectionDialog/CopyToCollectionDialog.tsx index 4edafdeec35..6a9ed057def 100644 --- a/apps/journeys-admin/src/components/TemplateGalleryPageList/CopyToCollectionDialog/CopyToCollectionDialog.tsx +++ b/apps/journeys-admin/src/components/TemplateGalleryPageList/CopyToCollectionDialog/CopyToCollectionDialog.tsx @@ -7,7 +7,7 @@ import Stack from '@mui/material/Stack' import Switch from '@mui/material/Switch' import TextField from '@mui/material/TextField' import Typography from '@mui/material/Typography' -import { Formik, FormikHelpers, type FormikErrors } from 'formik' +import { Formik, type FormikErrors, FormikHelpers } from 'formik' import sortBy from 'lodash/sortBy' import { useTranslation } from 'next-i18next/pages' import { ReactElement, useMemo } from 'react' @@ -170,15 +170,14 @@ export function CopyToCollectionDialog({ id: string(), localName: string().optional(), nativeName: string().optional() + }).when('showTranslation', { + is: true, + then: (schema) => + schema.required(t('Please select a language')).shape({ + id: string().required(t('Please select a language')) + }), + otherwise: (schema) => schema.optional() }) - .when('showTranslation', { - is: true, - then: (schema) => - schema.required(t('Please select a language')).shape({ - id: string().required(t('Please select a language')) - }), - otherwise: (schema) => schema.optional() - }) }), [t] ) From 441932304f61907f5df8c4d94e92e4008a6234fd Mon Sep 17 00:00:00 2001 From: Siyang Date: Thu, 21 May 2026 15:20:21 +1200 Subject: [PATCH 13/14] docs(NES-1637): add subscription-bridged dialog orchestration pattern Captures the three interlocking design decisions from NES-1637's CopyToCollectionMenuItem implementation as a reusable pattern under docs/solutions/best-practices/: - Callback-driven split: useSubscription is not awaitable; orchestrate via two handlers (handleSubmit + runAssign) bridged by onComplete - Gate the network call, not just setState: mountedRef pre-check before mutation await prevents server-side orphans on unmount - Cross-component lock cleanup: lifecycle useEffect releases the DnD signal idempotently on unmount, distinct from guardedClose's normal-close path Also adds a forward reference from NES-1539 Pattern 3 (the foundational mountedRef + guardedClose pattern) to this extended variant. --- ...on-bridged-dialog-orchestration-nes1637.md | 230 ++++++++++++++++++ ...llery-page-collections-patterns-nes1539.md | 13 +- 2 files changed, 242 insertions(+), 1 deletion(-) create mode 100644 docs/solutions/best-practices/subscription-bridged-dialog-orchestration-nes1637.md diff --git a/docs/solutions/best-practices/subscription-bridged-dialog-orchestration-nes1637.md b/docs/solutions/best-practices/subscription-bridged-dialog-orchestration-nes1637.md new file mode 100644 index 00000000000..a84b285dd74 --- /dev/null +++ b/docs/solutions/best-practices/subscription-bridged-dialog-orchestration-nes1637.md @@ -0,0 +1,230 @@ +--- +title: 'Subscription-bridged dialog orchestration: mutation → subscription → mutation with unmount safety and cross-component lock cleanup (NES-1637)' +date: 2026-05-21 +category: best-practices +module: apps/journeys-admin +problem_type: ui_pattern +component: journeys-admin/TemplateGalleryPageList +tags: + - apollo-client + - use-subscription + - sse + - on-complete-bridge + - mounted-ref + - guarded-close + - dnd-lock + - gallery-dialog-lock-context + - loading-ref + - single-flight + - formik + - unmount-safety + - lifecycle-effect +applies_when: + - 'A journeys-admin component chains an Apollo mutation, then a useSubscription call, then a second Apollo mutation — where the subscription is the bridge between the two mutations' + - 'The subscription is callback-driven (Apollo useSubscription) and surfaces its terminal signal through onComplete / onError, not a Promise' + - 'The component participates in a cross-component lock signal (e.g., setHasOpenDialog into GalleryDialogLockContext) that must be released on unmount, not only on explicit user close' + - 'The dialog stays mounted across the full pipeline (the parent uses handleKeepMounted so the menu-item subtree survives mid-flight)' + - 'Single-flight defence against rapid double-clicks is needed, and the disabled-prop check on the submit button is one React render cycle late' +--- + +# Subscription-bridged dialog orchestration (NES-1637) + +## Context + +Journeys-admin dialog components that orchestrate a multi-step async pipeline — Apollo mutation → translation → second Apollo mutation — face a structural problem that makes the naive implementation fail silently in production. + +The naive approach is to write a single `async handleSubmit` and `await` each step in sequence: + +```ts +// naive — this does NOT work +const { data } = await journeyDuplicate(...) +await translationComplete(...) // ← no Promise to await; useSubscription is callback-driven +await templateGalleryPageAssignJourney(...) +``` + +`useJourneyAiTranslateSubscription` is backed by `useSubscription` and emits its terminal signal through an `onComplete` callback, not a Promise. There is nothing to `await`. Any implementation that tries to model the translation step as an awaitable collapses the bridge between duplicate and assign: either the assign never fires, or it fires before translation has finished. + +A second, independent failure mode is lifecycle management. Dialogs in the gallery page stay mounted across the full pipeline (the `TemplateGalleryPage` keeps the menu-item subtree alive via `handleKeepMounted`), so the component can be unmounted mid-pipeline — by route navigation, a parent re-render, or the user dismissing the dialog — while async operations are still in flight. Two gaps interact here: + +1. A `mountedRef` check placed only AFTER `await mutation` still lets the network request fire when the component is already unmounted. The server creates a side-effect the user has no UI feedback about. +2. A `setHasOpenDialog?.(false)` call placed only in the normal-close path (`guardedClose`) is never reached when the component unmounts abnormally. The `GalleryDialogLockContext` boolean stays `true`, permanently disabling drag-and-drop on the gallery page for the rest of the session. + +`CopyToTeamMenuItem` (`apps/journeys-admin/src/components/Team/CopyToTeamMenuItem/CopyToTeamMenuItem.tsx`) established the subscription callback split pattern, but predates the gallery page and missed both of these cleanup gaps. `CopyToCollectionMenuItem` (NES-1637) is the first component to close all three gaps together. + +## Guidance + +### 1. Split orchestration into two handlers + +`useSubscription`-based translation cannot be awaited. The correct shape splits orchestration into two handlers connected by the subscription's `onComplete` callback: + +- `handleSubmit` — entry point; fires `journeyDuplicate`, then either arms the subscription (`setTranslationVariables`) and exits, OR calls `runAssign` directly when no translation is needed. +- `runAssign(newJourneyId, targetCollectionId)` — post-translation entry point; fires `templateGalleryPageAssignJourney`. The subscription's `onComplete` calls this function when translation completes. + +Plan pseudocode (illustrative, from `docs/plans/2026-05-21-001-feat-nes-1637-copy-to-collection-action-plan.md`): + +```text +handleSubmit({ collectionId, language?, showTranslation? }): + if loading: return // single-flight guard + teamId = activeTeam?.id + if teamId == null: + setError(noActiveTeamCopy); return // null guard + setLoading(true) + newJourneyId <- await journeyDuplicate(journey.id, teamId) + on throw: setError(duplicateFailCopy); return // no rollback + pendingTargetCollectionId = collectionId + if showTranslation && language: + setTranslationVars({ journeyId: newJourneyId, ...langArgs }) + return // exit; onComplete fires runAssign + runAssign(newJourneyId, collectionId) + +runAssign(newJourneyId, targetCollectionId): + await templateGalleryPageAssignJourney(newJourneyId, targetCollectionId) + on throw: refetch(GetAdminJourneys); setError(assignFailCopy); return + refetch(GetAdminJourneys) + setDone(true) + +// wired via useJourneyAiTranslateSubscription({ variables, skip, onComplete, onError }) +onComplete (subscription terminates): + if mountedRef: runAssign(newJourneyIdRef, pendingTargetCollectionIdRef) + +onError: + if mountedRef: + refetch(GetAdminJourneys) + setError("An error occurred while translating.") +``` + +The post-duplicate IDs are stashed in refs (`newJourneyIdRef`, `pendingTargetCollectionIdRef`) rather than read from Formik values, because Formik may be torn down by the time the subscription's `onComplete` fires. The subscription bridge carries no data — it is a pure signal to proceed to `runAssign`. + +`CopyToTeamMenuItem` established the same callback split first, but its `onComplete` performs the equivalent of `runAssign` inline (updating team state, calling `handleCloseMenu`, closing the dialog) rather than delegating to a named function — and it does not gate the call on `mountedRef`. + +### 2. Gate the network call, not just setState + +The standard `mountedRef` guard pattern (NES-1539 Pattern 3) teaches placing `if (!mountedRef.current) return` checks after each `await` to prevent `setState` calls on unmounted components. This is necessary but not sufficient when the operation being guarded is itself a network mutation. + +The gap in `CopyToTeamMenuItem`: `handleDuplicateJourney` calls `setLoading(false)` after `await journeyDuplicate(...)`, but the network call itself is not gated. If the component unmounts during the duplicate await, the mutation has already been sent to the server. + +The `CopyToCollectionMenuItem` fix — `runAssign` early-returns before the `await templateGalleryPageAssignJourney` call when `!mountedRef.current`: + +```tsx +const runAssign = async ( + newJourneyId: string, + targetCollectionId: string +): Promise => { + // Gate the network call itself on mount status — the assign + // mutation would otherwise fire after unmount (subscription + // onComplete races a closing dialog), creating a silent server-side + // orphan the user has no UI feedback about. + if (!mountedRef.current) return // ← BEFORE the await + try { + await templateGalleryPageAssignJourney({ + variables: { journeyId: newJourneyId, pageId: targetCollectionId } + }) + refetchAdminJourneys() + if (!mountedRef.current) return // ← AFTER the await (standard NES-1539 guard) + safeSetLoading(false) + setTranslationVariables(null) + setDone(true) + } catch { + refetchAdminJourneys() + if (!mountedRef.current) return + safeSetLoading(false) + setTranslationVariables(null) + setErrorMessage(assignFailCopy) + } +} +``` + +Both checks serve different purposes: the pre-await check prevents the network side-effect; the post-await check prevents stale `setState` calls after the response returns. + +The `safeSetLoading` helper mirrors a similar dual-write pattern — it keeps `loadingRef.current` and the `loading` state slot synchronized, so the single-flight guard in `handleSubmit` reads the ref (synchronous, no re-render) while the dialog component reads the state (reactive): + +```tsx +const safeSetLoading = (next: boolean): void => { + loadingRef.current = next + if (mountedRef.current) setLoading(next) +} +``` + +The `loadingRef` shadow was flagged by code review as dual-source-of-truth maintenance overhead. It survived as a defensive measure against React render timing: a ref read in `handleSubmit`'s synchronous path is guaranteed to reflect the latest value, whereas state reads in the same synchronous turn can lag by one render cycle on rapid double-clicks. Use this pattern when the disabled-button defence is meaningfully late; skip it when a button-disabled gate alone is sufficient. + +### 3. Cross-component lock requires its own unmount cleanup + +The `setHasOpenDialog` prop is a signal into `GalleryDialogLockContext` that disables drag-and-drop on the gallery page while a dialog is open. When the component unmounts while this signal is `true` (e.g., the user navigates away during a long-running translation), the lock stays permanently engaged for the rest of the session. + +The fix is to release the lock unconditionally in the `useEffect` cleanup: + +```tsx +useEffect(() => { + mountedRef.current = true + return (): void => { + mountedRef.current = false + // Release the DnD lock and null the post-translation refs in case + // the component unmounts mid-pipeline (route change, parent + // re-render). Calls are idempotent: `setHasOpenDialog?.(false)` + // is a no-op when the dialog was never opened, and the refs are + // already null at mount. Subscription teardown happens via Apollo + // unsubscribing when this hook unmounts. + setHasOpenDialog?.(false) + newJourneyIdRef.current = null + pendingTargetCollectionIdRef.current = null + } + // Lifecycle effect — mount/unmount only. Capturing the latest + // `setHasOpenDialog` reference is fine because it is a prop that + // does not change across renders in normal use. + // eslint-disable-next-line react-hooks/exhaustive-deps +}, []) +``` + +The cleanup's `setHasOpenDialog?.(false)` is independent of the same call inside `guardedClose`. They handle different cases: `guardedClose` handles the normal close (user clicks Done or Cancel while the component is still mounted); the cleanup handles abnormal close (component unmounts before the user can interact with it). Both are safe to coexist — calling `setHasOpenDialog?.(false)` when the lock was never acquired is a no-op. + +The `mountedRef` initialization follows NES-1539 Pattern 3: the setup body sets `mountedRef.current = true`, the cleanup sets it `false`. This avoids the StrictMode double-invoke trap where a cleanup-only flip leaves the ref permanently `false` after the second setup. + +`CopyToTeamMenuItem` calls `setHasOpenDialog?.(true)` in its `onClick` handler and `setHasOpenDialog?.(false)` in `onClose`, but has no `useEffect` cleanup for the lock. Because `CopyToTeamMenuItem` predates the gallery page, the `GalleryDialogLockContext` did not exist when it was written; the latent gap is real but has had no visible impact on the team copy flow. + +## Why This Matters + +**Naive `await translationComplete`.** `useJourneyAiTranslateSubscription` wraps Apollo's `useSubscription`. Its terminal signal is the `onComplete` callback on the hook options object, not a returned Promise. Attempting to `await` the hook's return value waits on an object, resolves immediately, and skips the assign entirely. There is no way to construct a Promise from a `useSubscription` result without a wrapper ref — and that wrapper is exactly what the two-handler split provides without the indirection. + +**`mountedRef` only after `await`.** Placing the guard only after the network call has dispatched means the server executes the mutation regardless. For `runAssign`, this creates a journey that has been assigned to a collection (a `templateGalleryPageAssignJourney` side-effect) with no UI confirmation, no error surface, and no user-visible entry in the UI (because the component that would show it is gone). The `refetchAdminJourneys()` call in `runAssign`'s catch branch is also never reached, so the orphan does not appear in All Templates either. The only visible symptom is silence. + +**Missing unmount cleanup for the DnD lock.** `GalleryDialogLockContext` is a boolean held in React context at the `TemplateGalleryPageList` level. It gates the `handleDragStart` callback on every draggable card and the `handleDragEnd` drop handler. If it stays `true`, every drag attempt is silently rejected: the card picks up visually, but `handleDragStart` returns early before setting `dragInFlight`, so `handleDragEnd` receives an inconsistent state and discards the drop. The user sees drag-and-drop stop working with no error. The lock is not persisted — it resets on full page reload — but within a session it permanently breaks the gallery's primary organizational affordance. + +## When to Apply + +Apply this pattern to any journeys-admin dialog component that combines all three of: + +- An Apollo mutation followed by `useJourneyAiTranslateSubscription` (or another callback-driven `useSubscription`) followed by a second Apollo mutation — where the subscription is the bridge between the first and second mutations. +- A cross-component lock signal (`setHasOpenDialog`, a drag-lock, a focus-trap, or any boolean held in a context that lives above the dialog boundary) that must be released when the component unmounts, not just when the user closes normally. +- A keep-mounted contract: the parent keeps the menu-item subtree alive via `handleKeepMounted` so the dialog stays mounted across the full pipeline, making mid-pipeline unmount a realistic race condition. + +Do not apply the two-handler split to flows where translation is absent or the terminal signal is truly a Promise (e.g., a standard `useMutation` call). In those cases, a single `async handleSubmit` with sequential `await` calls is correct and simpler. + +Do not apply the unmount-cleanup DnD-lock pattern to dialog components outside the `TemplateGalleryPageList` subtree. Components that do not receive `setHasOpenDialog` (or its equivalent) are not participating in the lock context and need no cleanup for it. + +## Examples + +### Reference implementation + +`apps/journeys-admin/src/components/TemplateGalleryPageList/CopyToCollectionMenuItem/CopyToCollectionMenuItem.tsx` — the canonical example. The three patterns are integrated as a single coherent flow: + +- Lines ~81–99: lifecycle effect with setup-body `mountedRef = true`, cleanup releases DnD lock + nulls bridge refs. +- Lines ~119–148: `runAssign` with pre-await + post-await `mountedRef` checks bracketing the assign mutation. +- Lines ~163–212: `handleSubmit` with single-flight `loadingRef` guard, null-team guard, duplicate, bridge-ref population, conditional subscription arm OR direct `runAssign`. +- Lines ~217–234: `useJourneyAiTranslateSubscription` with `onComplete` calling `runAssign(newJourneyIdRef.current, pendingTargetCollectionIdRef.current)` and `onError` issuing the refetch + error copy. + +### Precedent (with latent gaps) + +`apps/journeys-admin/src/components/Team/CopyToTeamMenuItem/CopyToTeamMenuItem.tsx` — established the subscription callback split pattern first. Two latent gaps remain in this file, exposed only when used in a context that has both `GalleryDialogLockContext` and mid-pipeline unmount pressure: + +- The subscription `onComplete` updates team state and closes the dialog without a `mountedRef` check (the post-await `setState`-on-dead-component issue). +- The `setHasOpenDialog` lock is acquired on open and released only in `onClose` — no `useEffect` cleanup for abnormal unmount. + +Neither gap has had visible impact on the team-copy flow because that flow predates the gallery page's lock context. Any future component that introduces a cross-boundary lock signal via `setHasOpenDialog` must include the unmount cleanup from the start. + +## Related + +- `docs/solutions/best-practices/template-gallery-page-collections-patterns-nes1539.md` — Pattern 3 (`mountedRef` + `guardedClose` for async dialog submits) is the foundational variant this doc extends. NES-1539 covers the single-mutation case; this doc extends it to the mutation → subscription → mutation case with cross-component lock cleanup. NES-1539 Patterns 5 (cross-mutation gate) and 11 (`optimisticResponse` for `templateGalleryPageAssignJourney`) are also relevant. +- `docs/solutions/best-practices/local-template-dialog-consolidation-patterns-nes1543.md` — Pattern 3 prohibits `enableReinitialize` on Formik dialogs that interact with subscription-driven cache writes. The `CopyToCollectionDialog` follows this rule. +- `docs/solutions/runtime-errors/yoga-response-cache-null-stickiness-and-zombie-process-debugging-nes1644.md` — backend Yoga cache TTL=0 config for the `templateGalleryPage` queries that the post-pipeline `refetchAdminJourneys` interacts with. Out of scope here but worth knowing when debugging refetch-failure cases. +- PR [#9237](https://github.com/JesusFilm/core/pull/9237) — NES-1637 implementation. +- Linear [NES-1637](https://linear.app/jesus-film-project/issue/NES-1637) — feature ticket including the scope-pivot addendum. diff --git a/docs/solutions/best-practices/template-gallery-page-collections-patterns-nes1539.md b/docs/solutions/best-practices/template-gallery-page-collections-patterns-nes1539.md index 617bfb5fe71..6f69c472785 100644 --- a/docs/solutions/best-practices/template-gallery-page-collections-patterns-nes1539.md +++ b/docs/solutions/best-practices/template-gallery-page-collections-patterns-nes1539.md @@ -2,7 +2,7 @@ title: 'Template Gallery Page (Collections) frontend patterns (NES-1539)' category: best-practices date: 2026-05-06 -last_updated: 2026-05-12 +last_updated: 2026-05-21 problem_type: ui_pattern component: journeys-admin/TemplateGalleryPageList tags: @@ -145,6 +145,17 @@ the only real defense is the corrected setup-body-resets-ref pattern itself and code review when reading any `useRef(true)` / `useEffect(() => () => …, [])` shape. +**Extended variant — subscription-chained orchestration (NES-1637).** When +the dialog chains a mutation → `useSubscription` → second mutation (e.g. +the AI-translate flow on the Copy-to-collection menu item), this Pattern 3 +foundation needs two extensions: (a) gate the second mutation's network +call *before* its `await`, not only after, otherwise the mutation fires +server-side on the unmounted component; and (b) release any +`setHasOpenDialog` / lock-context signal in the lifecycle cleanup, not +only in `guardedClose`, so abnormal-unmount (route navigation +mid-translation) does not permanently lock the gallery's drag-and-drop. +See `docs/solutions/best-practices/subscription-bridged-dialog-orchestration-nes1637.md`. + ### 4. DnD single-flight: gate START, not just END A `dragInFlight` boolean that's only checked in `handleDragEnd` is From 59ba63f3766e69ce81a2f1685929f71fb4cea5dd Mon Sep 17 00:00:00 2001 From: "autofix-ci[bot]" <114827586+autofix-ci[bot]@users.noreply.github.com> Date: Thu, 21 May 2026 03:24:18 +0000 Subject: [PATCH 14/14] fix: lint issues --- .../subscription-bridged-dialog-orchestration-nes1637.md | 9 +++------ ...template-gallery-page-collections-patterns-nes1539.md | 2 +- 2 files changed, 4 insertions(+), 7 deletions(-) diff --git a/docs/solutions/best-practices/subscription-bridged-dialog-orchestration-nes1637.md b/docs/solutions/best-practices/subscription-bridged-dialog-orchestration-nes1637.md index a84b285dd74..6ba56f77ee1 100644 --- a/docs/solutions/best-practices/subscription-bridged-dialog-orchestration-nes1637.md +++ b/docs/solutions/best-practices/subscription-bridged-dialog-orchestration-nes1637.md @@ -106,21 +106,18 @@ The gap in `CopyToTeamMenuItem`: `handleDuplicateJourney` calls `setLoading(fals The `CopyToCollectionMenuItem` fix — `runAssign` early-returns before the `await templateGalleryPageAssignJourney` call when `!mountedRef.current`: ```tsx -const runAssign = async ( - newJourneyId: string, - targetCollectionId: string -): Promise => { +const runAssign = async (newJourneyId: string, targetCollectionId: string): Promise => { // Gate the network call itself on mount status — the assign // mutation would otherwise fire after unmount (subscription // onComplete races a closing dialog), creating a silent server-side // orphan the user has no UI feedback about. - if (!mountedRef.current) return // ← BEFORE the await + if (!mountedRef.current) return // ← BEFORE the await try { await templateGalleryPageAssignJourney({ variables: { journeyId: newJourneyId, pageId: targetCollectionId } }) refetchAdminJourneys() - if (!mountedRef.current) return // ← AFTER the await (standard NES-1539 guard) + if (!mountedRef.current) return // ← AFTER the await (standard NES-1539 guard) safeSetLoading(false) setTranslationVariables(null) setDone(true) diff --git a/docs/solutions/best-practices/template-gallery-page-collections-patterns-nes1539.md b/docs/solutions/best-practices/template-gallery-page-collections-patterns-nes1539.md index 6f69c472785..ba62fbf1085 100644 --- a/docs/solutions/best-practices/template-gallery-page-collections-patterns-nes1539.md +++ b/docs/solutions/best-practices/template-gallery-page-collections-patterns-nes1539.md @@ -149,7 +149,7 @@ itself and code review when reading any `useRef(true)` / the dialog chains a mutation → `useSubscription` → second mutation (e.g. the AI-translate flow on the Copy-to-collection menu item), this Pattern 3 foundation needs two extensions: (a) gate the second mutation's network -call *before* its `await`, not only after, otherwise the mutation fires +call _before_ its `await`, not only after, otherwise the mutation fires server-side on the unmounted component; and (b) release any `setHasOpenDialog` / lock-context signal in the lifecycle cleanup, not only in `guardedClose`, so abnormal-unmount (route navigation