From 533ed392293a461613621a87b5e73c89825f9b57 Mon Sep 17 00:00:00 2001 From: jianwei1 Date: Tue, 5 May 2026 20:07:33 +0000 Subject: [PATCH 1/5] fix(NES-1636): wire AI translation in copy-to-team duplicate flow CopyToTeamDialog forwarded language and showTranslation to its submitAction, but DuplicateJourneyMenuItem only consumed teamId, so the translation toggle never triggered the journeyAiTranslate subscription. Mirror TranslateJourneyDialog: after journeyDuplicate resolves, start the translation subscription with the user-selected target language and the duplicated journey id, and let the subscription's onComplete close the dialog and refetch template stats. --- .../DuplicateJourneyMenuItem.spec.tsx | 363 +++++++++++++++++- .../DuplicateJourneyMenuItem.tsx | 115 +++++- ...-1636-ai-translate-on-copy-to-team-plan.md | 186 +++++++++ 3 files changed, 651 insertions(+), 13 deletions(-) create mode 100644 docs/plans/2026-05-05-001-fix-nes-1636-ai-translate-on-copy-to-team-plan.md diff --git a/apps/journeys-admin/src/components/JourneyList/JourneyCard/JourneyCardMenu/DuplicateJourneyMenuItem/DuplicateJourneyMenuItem.spec.tsx b/apps/journeys-admin/src/components/JourneyList/JourneyCard/JourneyCardMenu/DuplicateJourneyMenuItem/DuplicateJourneyMenuItem.spec.tsx index 7d74414d374..bb41c0ca91d 100644 --- a/apps/journeys-admin/src/components/JourneyList/JourneyCard/JourneyCardMenu/DuplicateJourneyMenuItem/DuplicateJourneyMenuItem.spec.tsx +++ b/apps/journeys-admin/src/components/JourneyList/JourneyCard/JourneyCardMenu/DuplicateJourneyMenuItem/DuplicateJourneyMenuItem.spec.tsx @@ -1,5 +1,11 @@ import { MockedProvider, MockedResponse } from '@apollo/client/testing' -import { fireEvent, render, waitFor, within } from '@testing-library/react' +import { + fireEvent, + render, + screen, + waitFor, + within +} from '@testing-library/react' import { NextRouter, useRouter } from 'next/router' import { SnackbarProvider } from 'notistack' @@ -8,6 +14,8 @@ import { GET_LAST_ACTIVE_TEAM_ID_AND_TEAMS, TeamProvider } from '@core/journeys/ui/TeamProvider' +import { SUPPORTED_LANGUAGE_IDS } from '@core/journeys/ui/useJourneyAiTranslateSubscription/supportedLanguages' +import { JOURNEY_AI_TRANSLATE_CREATE_SUBSCRIPTION } from '@core/journeys/ui/useJourneyAiTranslateSubscription/useJourneyAiTranslateSubscription' import { JOURNEY_DUPLICATE } from '@core/journeys/ui/useJourneyDuplicateMutation' import { GET_LANGUAGES } from '@core/journeys/ui/useLanguagesQuery' import { UPDATE_LAST_ACTIVE_TEAM_ID } from '@core/journeys/ui/useUpdateLastActiveTeamIdMutation' @@ -811,4 +819,357 @@ describe('DuplicateJourneys', () => { expect(handleCloseMenu).toHaveBeenCalled() expect(getByText('Journey Duplicated')).toBeInTheDocument() }) + + describe('AI translation on copy to team', () => { + const teamsMock = { + request: { + query: GET_LAST_ACTIVE_TEAM_ID_AND_TEAMS + }, + result: jest.fn(() => ({ + data: { + teams: [ + { + id: 'destinationTeamId', + title: 'Destination Team', + __typename: 'Team' + } + ], + getJourneyProfile: { + __typename: 'JourneyProfile', + lastActiveTeamId: null + } + } + })) + } + + const updateLastActiveTeamIdMock: MockedResponse = { + request: { + query: UPDATE_LAST_ACTIVE_TEAM_ID, + variables: { + input: { + lastActiveTeamId: 'destinationTeamId' + } + } + }, + result: jest.fn(() => ({ + data: { + journeyProfileUpdate: { + __typename: 'JourneyProfile', + id: 'destinationTeamId' + } + } + })) + } + + const languagesWithFrenchMock = { + request: { + query: GET_LANGUAGES, + variables: { + languageId: '529', + where: { + ids: [...SUPPORTED_LANGUAGE_IDS] + } + } + }, + result: { + data: { + languages: [ + { + __typename: 'Language', + id: '529', + slug: 'english', + name: [ + { + value: 'English', + primary: true, + __typename: 'LanguageName' + } + ] + }, + { + id: '496', + __typename: 'Language', + slug: 'french', + name: [ + { + value: 'French', + primary: false, + __typename: 'LanguageName' + }, + { + value: 'Français', + primary: true, + __typename: 'LanguageName' + } + ] + } + ] + } + } + } + + const journeyDuplicateForTranslationMock = { + request: { + query: JOURNEY_DUPLICATE, + variables: { + id: 'journey-id', + teamId: 'destinationTeamId' + } + }, + result: jest.fn(() => ({ + data: { + journeyDuplicate: { + id: 'duplicatedJourneyId' + } + } + })) + } + + async function selectDestinationTeam(): Promise { + const muiSelect = screen.getByTestId('team-duplicate-select') + const muiSelectDropDownButton = + await within(muiSelect).getByRole('combobox') + await fireEvent.mouseDown(muiSelectDropDownButton) + const teamOption = await screen.getByRole('option', { + name: 'Destination Team' + }) + fireEvent.click(teamOption) + } + + async function toggleTranslationAndPickFrench(): Promise { + fireEvent.click(screen.getByRole('checkbox', { name: 'Translation' })) + const languageInput = + await screen.findByPlaceholderText('Search Language') + fireEvent.focus(languageInput) + fireEvent.keyDown(languageInput, { key: 'ArrowDown' }) + fireEvent.click( + await screen.findByRole('option', { name: 'French Français' }) + ) + } + + function renderForTranslation( + additionalMocks: MockedResponse[] = [] + ): void { + render( + + + + + + + + ) + } + + it('starts AI translation subscription after duplicate when toggle is on', async () => { + const translateSubscriptionMock = { + request: { + query: JOURNEY_AI_TRANSLATE_CREATE_SUBSCRIPTION, + variables: { + journeyId: 'duplicatedJourneyId', + name: 'Default Journey Heading', + journeyLanguageName: '', + textLanguageId: '496', + textLanguageName: 'Français', + userLanguageId: '529', + userLanguageName: '' + } + }, + result: jest.fn(() => ({ + data: { + journeyAiTranslateCreateSubscription: { + progress: 100, + message: 'Translation completed', + journey: { + __typename: 'Journey', + id: 'duplicatedJourneyId', + title: 'Voyage Traduit', + description: null, + languageId: '496', + createdAt: '2026-04-25T12:34:56Z', + updatedAt: '2026-04-25T12:34:56Z', + journeyCustomizationDescription: null, + journeyCustomizationFields: [], + blocks: [], + language: { + __typename: 'Language', + id: '496', + name: [ + { + __typename: 'LanguageName', + value: 'Français', + primary: true + } + ] + } + }, + __typename: 'JourneyAiTranslateProgress' + } + } + })) + } + + renderForTranslation([translateSubscriptionMock]) + + await waitFor(() => expect(teamsMock.result).toHaveBeenCalled()) + fireEvent.click(screen.getByRole('menuitem', { name: 'Duplicate' })) + expect( + await screen.findByText('Copy to Another Team') + ).toBeInTheDocument() + await selectDestinationTeam() + await toggleTranslationAndPickFrench() + fireEvent.click(screen.getByRole('button', { name: 'Create' })) + + await waitFor(() => + expect(journeyDuplicateForTranslationMock.result).toHaveBeenCalled() + ) + await waitFor(() => + expect(translateSubscriptionMock.result).toHaveBeenCalled() + ) + expect(handleCloseMenu).toHaveBeenCalled() + // updateLastActiveTeamId sets the active team during dialog submit, so by + // the time the subscription completes the active-team-aware snackbar reads + // "Journey Duplicated" rather than "Journey Copied". + expect(await screen.findByText('Journey Duplicated')).toBeInTheDocument() + }) + + it('surfaces a snackbar when the translation subscription errors and keeps the duplicate', async () => { + const translateSubscriptionErrorMock = { + request: { + query: JOURNEY_AI_TRANSLATE_CREATE_SUBSCRIPTION, + variables: { + journeyId: 'duplicatedJourneyId', + name: 'Default Journey Heading', + journeyLanguageName: '', + textLanguageId: '496', + textLanguageName: 'Français', + userLanguageId: '529', + userLanguageName: '' + } + }, + error: new Error('Translation service unavailable') + } + + renderForTranslation([translateSubscriptionErrorMock]) + + await waitFor(() => expect(teamsMock.result).toHaveBeenCalled()) + fireEvent.click(screen.getByRole('menuitem', { name: 'Duplicate' })) + expect( + await screen.findByText('Copy to Another Team') + ).toBeInTheDocument() + await selectDestinationTeam() + await toggleTranslationAndPickFrench() + fireEvent.click(screen.getByRole('button', { name: 'Create' })) + + await waitFor(() => + expect(journeyDuplicateForTranslationMock.result).toHaveBeenCalled() + ) + expect( + await screen.findByText('Translation service unavailable') + ).toBeInTheDocument() + // The duplicate snackbar must NOT show when the translation flow errors. + expect(screen.queryByText('Journey Copied')).not.toBeInTheDocument() + expect(screen.queryByText('Journey Duplicated')).not.toBeInTheDocument() + }) + + it('refetches template stats after a translated copy completes', async () => { + const translateSubscriptionMock = { + request: { + query: JOURNEY_AI_TRANSLATE_CREATE_SUBSCRIPTION, + variables: { + journeyId: 'duplicatedJourneyId', + name: 'Default Journey Heading', + journeyLanguageName: '', + textLanguageId: '496', + textLanguageName: 'Français', + userLanguageId: '529', + userLanguageName: '' + } + }, + result: jest.fn(() => ({ + data: { + journeyAiTranslateCreateSubscription: { + progress: 100, + message: 'Translation completed', + journey: { + __typename: 'Journey', + id: 'duplicatedJourneyId', + title: 'Voyage Traduit', + description: null, + languageId: '496', + createdAt: '2026-04-25T12:34:56Z', + updatedAt: '2026-04-25T12:34:56Z', + journeyCustomizationDescription: null, + journeyCustomizationFields: [], + blocks: [], + language: { + __typename: 'Language', + id: '496', + name: [ + { + __typename: 'LanguageName', + value: 'Français', + primary: true + } + ] + } + }, + __typename: 'JourneyAiTranslateProgress' + } + } + })) + } + + render( + + + + + + + + ) + + await waitFor(() => expect(teamsMock.result).toHaveBeenCalled()) + fireEvent.click(screen.getByRole('menuitem', { name: 'Duplicate' })) + expect( + await screen.findByText('Copy to Another Team') + ).toBeInTheDocument() + await selectDestinationTeam() + await toggleTranslationAndPickFrench() + fireEvent.click(screen.getByRole('button', { name: 'Create' })) + + await waitFor(() => + expect(translateSubscriptionMock.result).toHaveBeenCalled() + ) + await waitFor(() => + expect(refetchTemplateStats).toHaveBeenCalledWith(['templateId123']) + ) + }) + }) }) diff --git a/apps/journeys-admin/src/components/JourneyList/JourneyCard/JourneyCardMenu/DuplicateJourneyMenuItem/DuplicateJourneyMenuItem.tsx b/apps/journeys-admin/src/components/JourneyList/JourneyCard/JourneyCardMenu/DuplicateJourneyMenuItem/DuplicateJourneyMenuItem.tsx index d9289437937..b2454d7b1f3 100644 --- a/apps/journeys-admin/src/components/JourneyList/JourneyCard/JourneyCardMenu/DuplicateJourneyMenuItem/DuplicateJourneyMenuItem.tsx +++ b/apps/journeys-admin/src/components/JourneyList/JourneyCard/JourneyCardMenu/DuplicateJourneyMenuItem/DuplicateJourneyMenuItem.tsx @@ -6,7 +6,9 @@ import { ReactElement, useState } from 'react' import { setBeaconPageViewed } from '@core/journeys/ui/beaconHooks' import { CopyToTeamDialog } from '@core/journeys/ui/CopyToTeamDialog' +import { useJourney } from '@core/journeys/ui/JourneyProvider' import { useTeam } from '@core/journeys/ui/TeamProvider' +import { useJourneyAiTranslateSubscription } from '@core/journeys/ui/useJourneyAiTranslateSubscription' import { useJourneyDuplicateMutation } from '@core/journeys/ui/useJourneyDuplicateMutation' import CopyLeftIcon from '@core/shared/ui/icons/CopyLeft' @@ -14,6 +16,22 @@ import { GetAdminJourneys_journeys as Journey } from '../../../../../../__genera import { useTemplateFamilyStatsAggregateLazyQuery } from '../../../../../libs/useTemplateFamilyStatsAggregateLazyQuery' import { MenuItem } from '../../../../MenuItem' +interface JourneyLanguage { + id: string + localName?: string + nativeName?: string +} + +interface TranslationVariables { + journeyId: string + name: string + journeyLanguageName: string + textLanguageId: string + textLanguageName: string + userLanguageId?: string + userLanguageName?: string +} + interface DuplicateJourneyMenuItemProps { id?: string handleCloseMenu: () => void @@ -30,29 +48,89 @@ export function DuplicateJourneyMenuItem({ const { t } = useTranslation('apps-journeys-admin') const router = useRouter() const { activeTeam } = useTeam() + const { journey: journeyFromContext } = useJourney() + const journeyData = journey ?? journeyFromContext const { enqueueSnackbar } = useSnackbar() const { refetchTemplateStats } = useTemplateFamilyStatsAggregateLazyQuery() const [open, setOpen] = useState(false) const [loading, setLoading] = useState(false) + const [translationVariables, setTranslationVariables] = useState< + TranslationVariables | undefined + >(undefined) const [journeyDuplicate] = useJourneyDuplicateMutation() - async function handleDuplicateJourney(teamId?: string): Promise { + const { data: translationData } = useJourneyAiTranslateSubscription({ + variables: translationVariables, + skip: translationVariables == null, + onError(error) { + enqueueSnackbar(error.message, { + variant: 'error', + preventDuplicate: true + }) + setLoading(false) + setTranslationVariables(undefined) + }, + onComplete() { + if (fromTemplateId != null) { + void refetchTemplateStats([fromTemplateId]) + } + enqueueSnackbar( + activeTeam?.id != null ? t('Journey Duplicated') : t('Journey Copied'), + { + variant: 'success', + preventDuplicate: true + } + ) + setTranslationVariables(undefined) + setLoading(false) + setOpen(false) + handleCloseMenu() + } + }) + + async function handleDuplicateJourney( + teamId?: string, + language?: JourneyLanguage, + showTranslation?: boolean + ): Promise { if (id == null) return setLoading(true) try { - if (teamId != null) { - await journeyDuplicate({ - variables: { - id, - teamId - } - }) - } else if (activeTeam?.id != null) { - await journeyDuplicate({ - variables: { id, teamId: activeTeam.id } + const targetTeamId = teamId ?? activeTeam?.id + if (targetTeamId == null) { + setLoading(false) + return + } + + const { data: duplicateData } = await journeyDuplicate({ + variables: { id, teamId: targetTeamId } + }) + + const duplicatedJourneyId = duplicateData?.journeyDuplicate?.id + + if ( + showTranslation === true && + language?.id != null && + duplicatedJourneyId != null && + journeyData?.language != null + ) { + const sourceLanguageName = + journeyData.language.name.find(({ primary }) => !primary)?.value ?? '' + + setTranslationVariables({ + journeyId: duplicatedJourneyId, + name: journeyData.title ?? '', + journeyLanguageName: sourceLanguageName, + textLanguageId: language.id, + textLanguageName: language.nativeName ?? language.localName ?? '', + userLanguageId: journeyData.language.id, + userLanguageName: sourceLanguageName }) + // Subscription's onComplete / onError handles snackbar, refetch, close, + // and clearing the loading state. Keep `loading` true until then. + return } if (fromTemplateId != null) { @@ -67,12 +145,12 @@ export function DuplicateJourneyMenuItem({ } ) handleCloseMenu() + setLoading(false) } catch (error) { enqueueSnackbar(t('Failed to duplicate journey'), { variant: 'error', preventDuplicate: true }) - } finally { setLoading(false) } } @@ -86,6 +164,16 @@ export function DuplicateJourneyMenuItem({ }) } + const translationProgress = + translationData?.journeyAiTranslateCreateSubscription + ? { + progress: + translationData.journeyAiTranslateCreateSubscription.progress ?? 0, + message: + translationData.journeyAiTranslateCreateSubscription.message ?? '' + } + : undefined + return ( <> { setOpen(false) }} submitAction={handleDuplicateJourney} + isTranslating={translationVariables != null} + translationProgress={translationProgress} journeyIsTemplate={journey?.template ?? false} journeyFromTemplateId={journey?.fromTemplateId} /> diff --git a/docs/plans/2026-05-05-001-fix-nes-1636-ai-translate-on-copy-to-team-plan.md b/docs/plans/2026-05-05-001-fix-nes-1636-ai-translate-on-copy-to-team-plan.md new file mode 100644 index 00000000000..bf0e11163fc --- /dev/null +++ b/docs/plans/2026-05-05-001-fix-nes-1636-ai-translate-on-copy-to-team-plan.md @@ -0,0 +1,186 @@ +--- +title: 'fix: Wire AI translation step in Copy-to-Team duplicate flow (NES-1636)' +type: fix +status: active +date: 2026-05-05 +--- + +# fix: Wire AI translation step in Copy-to-Team duplicate flow (NES-1636) + +## Summary + +When a user copies a journey to another team via the 3-dot menu and enables the AI translation toggle, the journey is duplicated but never translated. The fix wires the existing `useJourneyAiTranslateSubscription` into `DuplicateJourneyMenuItem` so that — after `journeyDuplicate` resolves — the translation subscription is started using the user's selected language, mirroring the working pattern already in `TranslateJourneyDialog`. + +--- + +## Problem Frame + +The `CopyToTeamDialog` (a shared library component) already correctly forwards three values to its `submitAction` prop: target `teamId`, selected `language`, and the `showTranslation` boolean. The `DuplicateJourneyMenuItem` callsite, however, declares `handleDuplicateJourney(teamId?: string)` — accepting only the team id and silently dropping the language and translation flag. TypeScript does not flag this because a function with fewer parameters is structurally assignable to one with more. The dialog also defers its own auto-close to the consumer when translation is enabled (`CopyToTeamDialog.tsx:144`), so a Copy-to-Team flow with translation toggled on never closes via the duplicate path either — instead the user only sees the success snackbar from the duplicate, with no translated copy ever produced. + +This is a regression-shape bug: the dialog API was extended to support translation (PR #6761) but the menu-item consumer was never updated to call the translate subscription. + +--- + +## Requirements + +- R1. Toggling the AI translation switch in the Copy-to-Team dialog and submitting MUST start the journey translation subscription against the duplicated journey. +- R2. Submitting Copy-to-Team WITHOUT the translation toggle must continue to behave exactly as today (duplicate only, success snackbar, dialog closes). +- R3. The same-team direct duplicate path (3-dot menu → Duplicate, no dialog) must remain untouched — translation never runs in that flow. +- R4. Translation progress UI from `TranslationDialogWrapper` must render while the subscription is running (the dialog already supports this via `translationProgress` + `isTranslating` props — they just need to be passed in). +- R5. Translation completion must close the dialog and refetch template stats when applicable, matching `TranslateJourneyDialog`'s `onComplete` behavior. +- R6. Translation failures must surface a snackbar without rolling back the already-duplicated journey, matching `TranslateJourneyDialog`'s `onError` behavior. + +--- + +## Scope Boundaries + +- Not touching `CopyToTeamDialog` — its API and form already support translation correctly. +- Not touching `journeyAiTranslate` schema or backend — backend already works; the `TranslateJourneyDialog` flow proves it. +- Not refactoring `TranslateJourneyDialog` for shared logic. A `useJourneyDuplicateAndTranslate` hook is tempting but premature: only two callsites, and they have different UX (separate Translate dialog vs combined Copy + Translate). Defer until a third callsite appears. +- Not changing the `GetAdminJourneys` GraphQL query — already includes `language.name` and `title`. + +### Deferred to Follow-Up Work + +- Extracting a shared `useJourneyDuplicateAndTranslate` hook between `DuplicateJourneyMenuItem` and `TranslateJourneyDialog`: deferred until a third consumer appears. + +--- + +## Context & Research + +### Relevant Code and Patterns + +- **Bug site:** `apps/journeys-admin/src/components/JourneyList/JourneyCard/JourneyCardMenu/DuplicateJourneyMenuItem/DuplicateJourneyMenuItem.tsx` — `handleDuplicateJourney` accepts only `teamId` (line 41) and is wired as `submitAction` (line 121). +- **Dialog forwarding the dropped args:** `libs/journeys/ui/src/components/CopyToTeamDialog/CopyToTeamDialog.tsx` — `handleSubmit` calls `submitAction(values.teamSelect, values.languageSelect, values.showTranslation)` (lines 130-134) and skips its own auto-close when `showTranslation` is true (line 144). +- **Reference pattern (working flow):** `apps/journeys-admin/src/components/JourneyList/JourneyCard/JourneyCardMenu/TranslateJourneyDialog/TranslateJourneyDialog.tsx` — full duplicate-then-translate sequence at lines 139-186, subscription wiring at lines 113-130, source language name derivation at line 159. +- **Subscription hook:** `libs/journeys/ui/src/libs/useJourneyAiTranslateSubscription/useJourneyAiTranslateSubscription.ts` — variables shape at lines 13-32; the hook also handles cache writes on each progress event. +- **Existing spec to extend:** `apps/journeys-admin/src/components/JourneyList/JourneyCard/JourneyCardMenu/DuplicateJourneyMenuItem/DuplicateJourneyMenuItem.spec.tsx` — already mocks `JOURNEY_DUPLICATE`, `GET_LANGUAGES`, `GET_LAST_ACTIVE_TEAM_ID_AND_TEAMS`, and `UPDATE_LAST_ACTIVE_TEAM_ID`. + +### Institutional Learnings + +- `docs/solutions/best-practices/local-template-dialog-consolidation-patterns-nes1543.md` — covers admin dialogs that fire multiple journey mutations. Key takeaway for this plan: a translate failure after a successful duplicate must keep the duplicate, surface a snackbar, and not roll back. Mirror `TranslateJourneyDialog`'s exact `onError` shape so subscription/cache wiring stays consistent. + +### External References + +- None needed. The fix is a near-mechanical port of the proven `TranslateJourneyDialog` wiring into a callsite that's missing it. + +--- + +## Key Technical Decisions + +- **Mirror `TranslateJourneyDialog`'s subscription wiring rather than introducing a shared hook.** The subscription has six tightly-coupled pieces (state, hook, cache writes, onComplete, onError, progress passthrough). Two callsites do not justify abstracting yet — the callsites also differ structurally (one is a dedicated dialog, one is a menu-item that owns the dialog). Rationale: avoid premature abstraction; a future refactor will be cheaper if there's a third consumer to motivate the shape. +- **Drive `showTranslation` flag and language through the existing `submitAction` signature.** No prop or contract change is needed — the dialog already passes them; the consumer just needs to consume them. +- **Skip translation when journey, language, or activeTeam context is missing.** Match `TranslateJourneyDialog`'s `handleTranslate` early-return (line 140-145). Defensive against form-validation gaps. +- **Source language name derivation: read from `journey?.language.name` on the prop first, then fall back to `useJourney()` context.** The `GetAdminJourneys` query already returns `language { id, name { value, primary } }`, so the prop path covers the JourneyList card flow. Falling back to context covers any future caller that renders this menu item without passing `journey`. + +--- + +## Open Questions + +### Resolved During Planning + +- **Does `GetAdminJourneys` expose journey language name?** Yes — confirmed at `apps/journeys-admin/src/libs/useAdminJourneysQuery/useAdminJourneysQuery.ts:8-50`. No GraphQL change required. +- **Should the same-team direct duplicate (no dialog) ever translate?** No — the in-team auto-duplicate path (lines 105-112) bypasses the dialog entirely; the user never sees a translation toggle, so showTranslation is implicitly false. Maintain the existing zero-arg call shape. + +### Deferred to Implementation + +- **Whether to also pass translation context up to `handleCloseMenu` timing.** The current `handleDuplicateJourney` calls `handleCloseMenu()` as soon as the duplicate succeeds (line 69). For the translate path, closing the menu wrapper before the subscription completes is fine (the dialog stays open via `TranslationDialogWrapper`'s loading state), but verify in implementation that closing the menu doesn't unmount the dialog and abort the subscription. If it does, defer `handleCloseMenu` until the subscription `onComplete` fires. + +--- + +## Implementation Units + +- U1. **Wire AI translation subscription into `DuplicateJourneyMenuItem`** + +**Goal:** Make `handleDuplicateJourney` accept the language and translation flag, and after `journeyDuplicate` succeeds, start the AI translation subscription when the user opted in. Pass progress and translation state back into `CopyToTeamDialog` so the existing progress UI renders. + +**Requirements:** R1, R2, R3, R4, R5, R6 + +**Dependencies:** None. + +**Files:** + +- Modify: `apps/journeys-admin/src/components/JourneyList/JourneyCard/JourneyCardMenu/DuplicateJourneyMenuItem/DuplicateJourneyMenuItem.tsx` +- Test: `apps/journeys-admin/src/components/JourneyList/JourneyCard/JourneyCardMenu/DuplicateJourneyMenuItem/DuplicateJourneyMenuItem.spec.tsx` + +**Approach:** + +- Update `handleDuplicateJourney` signature to `(teamId?: string, language?: JourneyLanguage, showTranslation?: boolean)`. Define `JourneyLanguage` locally (matching the shape used by `CopyToTeamDialog` and `TranslateJourneyDialog`) or import from a shared location if one exists. +- Capture the `journeyDuplicate` result so the new journey id is available: `const { data: duplicateData } = await journeyDuplicate({ ... })`. +- Add `translationVariables` state (the same shape as `TranslateJourneyDialog.tsx:64-75`). +- Add the `useJourneyAiTranslateSubscription` call with `variables: translationVariables`, `skip: translationVariables == null`, and `onError` / `onComplete` handlers that mirror `TranslateJourneyDialog.tsx:114-130`. `onComplete` should refetch template stats (when `fromTemplateId` is set) and call `handleCloseMenu` + close the dialog. +- After the duplicate resolves, branch: + - If `showTranslation && language?.id && duplicateData?.journeyDuplicate?.id && journey?.language`: call `setTranslationVariables({ journeyId: duplicateData.journeyDuplicate.id, name: journey.title, journeyLanguageName: , textLanguageId: language.id, textLanguageName: language.nativeName ?? language.localName ?? '', userLanguageId: journey.language.id, userLanguageName: })`. Do NOT show the "Journey Copied" snackbar yet — let the subscription handle completion. + - Else: keep the existing snackbar + `handleCloseMenu` path unchanged. +- Source-language-name derivation: pull from `journey?.language.name.find(({ primary }) => !primary)?.value ?? ''` (matches `TranslateJourneyDialog.tsx:159`). If `journey` prop is missing, fall back to `useJourney()` context, mirroring `TranslateJourneyDialog`. +- Pass `isTranslating={translationVariables != null}` and `translationProgress={...}` (built from the subscription's `data` per `TranslateJourneyDialog.tsx:200-209`) into the rendered `` so the dialog's existing progress UI displays. +- Preserve the existing `journey?.fromTemplateId` refetch logic — but invoke it only on successful completion (either the non-translate snackbar branch as today, or via the subscription `onComplete` for the translate branch). + +**Execution note:** Verify each behavior change with the existing spec file's pattern before adding new tests. The existing tests must continue to pass unchanged (R2/R3). + +**Patterns to follow:** + +- `apps/journeys-admin/src/components/JourneyList/JourneyCard/JourneyCardMenu/TranslateJourneyDialog/TranslateJourneyDialog.tsx` — full subscription + state + onComplete/onError shape. +- `libs/journeys/ui/src/components/CopyToTeamDialog/CopyToTeamDialog.tsx:32-44` — exact `submitAction` signature and `translationProgress` / `isTranslating` prop shape. + +**Test scenarios:** + +- _Happy path_ — Open Copy-to-Team dialog → select different team → toggle Translation on → select target language → submit. Mock `JOURNEY_DUPLICATE` to return a duplicated journey id, mock `JOURNEY_AI_TRANSLATE_CREATE_SUBSCRIPTION` to emit progress and complete. Assert: duplicate mutation runs once with `{id, teamId}`, subscription is called with `{journeyId: '', textLanguageId: '', name: '', journeyLanguageName: '', textLanguageName: '', userLanguageId, userLanguageName}`, dialog closes after subscription completes, no error snackbar. +- _Happy path — translation off_ — Open Copy-to-Team dialog → select different team → leave Translation off → submit. Assert duplicate mutation runs once, subscription is NOT invoked, "Journey Copied" snackbar shows, dialog closes. (Existing test "should open copy to team dialog when on shared with me" already covers this — keep it green.) +- _Happy path — same-team direct duplicate_ — Click Duplicate menu item with an `activeTeam`. Assert: duplicate mutation runs, subscription is NOT invoked, "Journey Duplicated" snackbar shows, `handleCloseMenu` runs. (Existing test "should duplicate a journey on menu card click" already covers this — keep it green.) +- _Edge case — translation toggled on but no language selected_ — `CopyToTeamDialog`'s Yup schema already prevents submit; assert that the existing form-validation behavior still blocks the call (no `JOURNEY_DUPLICATE` mock should be consumed). +- _Error path — duplicate mutation fails_ — Mock `JOURNEY_DUPLICATE` to error. Assert: subscription is NOT invoked, "Failed to duplicate journey" snackbar shows, dialog stays open or closes per existing behavior, `handleCloseMenu` is NOT called. +- _Error path — translation subscription errors after a successful duplicate_ — Mock `JOURNEY_DUPLICATE` to succeed, mock subscription to call `onError`. Assert: duplicate succeeded (no rollback), error snackbar with the subscription error message shows, `translationVariables` is reset to `undefined`, loading state clears. +- _Integration — `fromTemplateId` refetch on translate completion_ — Render with `fromTemplateId="templateId123"`, run the happy-path translate flow. Assert `refetchTemplateStats` is called with `['templateId123']` after the subscription completes (mirroring the existing same-team test). +- _Integration — translation progress prop renders_ — While the subscription is mid-flight (emitted partial progress, not yet complete), assert that `CopyToTeamDialog` receives `isTranslating=true` and `translationProgress` matching the latest emission. Use `getByTestId('CopyToTeamDialog')` and the progress region surfaced by `TranslationDialogWrapper`. + +**Verification:** + +- `handleDuplicateJourney` accepts and uses the language + translation flag passed from the dialog. +- After `journeyDuplicate` succeeds with `showTranslation=true`, the AI translate subscription starts with the duplicated journey id and the user-selected target language. +- After `journeyDuplicate` succeeds with `showTranslation=false` (or the same-team direct path), no subscription is invoked and behavior is unchanged. +- Subscription completion closes the dialog, calls `handleCloseMenu`, and refetches template stats when applicable. +- Subscription error surfaces a snackbar without rolling back the duplicate. +- All existing tests in `DuplicateJourneyMenuItem.spec.tsx` continue to pass without modification. + +--- + +## System-Wide Impact + +- **Interaction graph:** This change is internal to one component plus its tests. The shared `CopyToTeamDialog` API is unchanged; the existing other consumer of `CopyToTeamDialog` (any publisher-side use, if present) is unaffected. +- **Error propagation:** Subscription errors are surfaced via `enqueueSnackbar` and reset local state. No global error boundary involvement. Cache writes inside `useJourneyAiTranslateSubscription` are best-effort and already wrapped in try/catch (`useJourneyAiTranslateSubscription.ts:154`). +- **State lifecycle risks:** The subscription must NOT start before `journeyDuplicate` resolves with a valid id; the `setTranslationVariables` call is gated on the duplicate response. If the user navigates away mid-translation, the subscription unsubscribes via component unmount — this matches `TranslateJourneyDialog`'s behavior and is acceptable: the duplicated journey is already created and the translation will continue server-side per the existing subscription contract. +- **API surface parity:** The other AI-translate entrypoint (`TranslateJourneyDialog`) uses the exact same hook with the same variables shape. After this fix, both entrypoints behave consistently. +- **Integration coverage:** The progress UI relies on prop-passing through `CopyToTeamDialog → TranslationDialogWrapper`. Pure unit tests with mocked subscription frames are sufficient to prove the wiring. +- **Unchanged invariants:** `useJourneyDuplicateMutation`, `JOURNEY_AI_TRANSLATE_CREATE_SUBSCRIPTION`, `CopyToTeamDialog`, and `TranslateJourneyDialog` are not modified. Same-team direct duplicate, "Journey Duplicated" / "Journey Copied" snackbars, and the `fromTemplateId` refetch behavior are preserved. + +--- + +## Risks & Dependencies + +| Risk | Mitigation | +| ------------------------------------------------------------------------------------------------------------------------------------------------------ | --------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------- | +| Closing the menu (`handleCloseMenu`) before the subscription completes could unmount the component and cancel the in-flight translation client-side. | During implementation, verify by exercising the happy path. If unmounting cancels the subscription, defer `handleCloseMenu` until `onComplete`. If the duplicate journey persists server-side regardless, leave as-is and note the behavior in QA scenarios. | +| Subscribing with an unintended/empty `textLanguageName` would still fire the subscription against the backend. | Mirror `TranslateJourneyDialog.tsx:167-168`'s fallback chain (`nativeName ?? localName ?? ''`) and gate the call on `language?.id` being defined. The `CopyToTeamDialog` Yup schema already requires `languageSelect.id` when `showTranslation` is true. | +| Spec file growth: this component already has 5 long tests; adding 4-6 more could push the file over a maintainability threshold. | Acceptable. Keep helper mock builders DRY, reuse existing patterns. If the file gets unwieldy, extract a `__fixtures__` file in a follow-up — not in this PR. | +| `useJourney()` context fallback may be unavailable in JourneyList card render path, leaving source-language name empty. | The `journey` prop already carries `language.name`; the `useJourney()` fallback is only for a hypothetical caller without a prop. If the prop is present (the JourneyList path), the fallback is never exercised. Mirror `TranslateJourneyDialog`'s fallback shape. | + +--- + +## Documentation / Operational Notes + +- No user-facing docs to update. Internal product documentation already describes "Copy to Team with AI translation" as the intended capability — this change makes the feature actually work. +- No feature flag required: the code path was always intended to translate when the toggle is on. This is a bug fix, not a new feature. +- No migration or data backfill needed. +- Manual QA on Vercel preview should cover the three scenarios in the test plan: translation on (different team), translation off (different team), and same-team direct duplicate (no dialog). + +--- + +## Sources & References + +- Linear ticket: [NES-1636](https://linear.app/jesus-film-project/issue/NES-1636/ai-translation-does-not-work-when-copying-a-journey-to-another-team) +- Parent ticket: [NES-1601](https://linear.app/jesus-film-project/issue/NES-1601/set-dialog-default-to-current-team-when-using-team-templates) (PR #9074, separate fix in review) +- Reference implementation (working translate flow): `apps/journeys-admin/src/components/JourneyList/JourneyCard/JourneyCardMenu/TranslateJourneyDialog/TranslateJourneyDialog.tsx` +- Subscription contract: `libs/journeys/ui/src/libs/useJourneyAiTranslateSubscription/useJourneyAiTranslateSubscription.ts` +- Dialog API: `libs/journeys/ui/src/components/CopyToTeamDialog/CopyToTeamDialog.tsx` +- Related PR (added translation toggle to `CopyToTeamDialog`, but never wired the consumer): #6761 +- Institutional learning: `docs/solutions/best-practices/local-template-dialog-consolidation-patterns-nes1543.md` From 02fc93573507a0b48fc9f01daba87eef2cd06663 Mon Sep 17 00:00:00 2001 From: jianwei1 Date: Tue, 5 May 2026 20:29:20 +0000 Subject: [PATCH 2/5] fix(review): apply autofix feedback --- .../DuplicateJourneyMenuItem.spec.tsx | 79 +++++++++++++++++++ .../DuplicateJourneyMenuItem.tsx | 11 ++- 2 files changed, 86 insertions(+), 4 deletions(-) diff --git a/apps/journeys-admin/src/components/JourneyList/JourneyCard/JourneyCardMenu/DuplicateJourneyMenuItem/DuplicateJourneyMenuItem.spec.tsx b/apps/journeys-admin/src/components/JourneyList/JourneyCard/JourneyCardMenu/DuplicateJourneyMenuItem/DuplicateJourneyMenuItem.spec.tsx index bb41c0ca91d..1911b100f80 100644 --- a/apps/journeys-admin/src/components/JourneyList/JourneyCard/JourneyCardMenu/DuplicateJourneyMenuItem/DuplicateJourneyMenuItem.spec.tsx +++ b/apps/journeys-admin/src/components/JourneyList/JourneyCard/JourneyCardMenu/DuplicateJourneyMenuItem/DuplicateJourneyMenuItem.spec.tsx @@ -1082,6 +1082,85 @@ describe('DuplicateJourneys', () => { // The duplicate snackbar must NOT show when the translation flow errors. expect(screen.queryByText('Journey Copied')).not.toBeInTheDocument() expect(screen.queryByText('Journey Duplicated')).not.toBeInTheDocument() + // The duplicated journey must persist; the menu wrapper must NOT close + // and template stats must NOT be refetched on subscription error. + expect(handleCloseMenu).not.toHaveBeenCalled() + expect(refetchTemplateStats).not.toHaveBeenCalled() + }) + + it('shows the duplicate-failure snackbar and does not start translation when journeyDuplicate errors', async () => { + const journeyDuplicateErrorMock = { + request: { + query: JOURNEY_DUPLICATE, + variables: { + id: 'journey-id', + teamId: 'destinationTeamId' + } + }, + error: new Error('Duplicate failed') + } + + const translateSubscriptionMock = { + request: { + query: JOURNEY_AI_TRANSLATE_CREATE_SUBSCRIPTION, + variables: { + journeyId: 'duplicatedJourneyId', + name: 'Default Journey Heading', + journeyLanguageName: '', + textLanguageId: '496', + textLanguageName: 'Français', + userLanguageId: '529', + userLanguageName: '' + } + }, + result: jest.fn(() => ({ + data: { + journeyAiTranslateCreateSubscription: { + progress: 100, + message: 'Translation completed', + journey: null, + __typename: 'JourneyAiTranslateProgress' + } + } + })) + } + + render( + + + + + + + + ) + + await waitFor(() => expect(teamsMock.result).toHaveBeenCalled()) + fireEvent.click(screen.getByRole('menuitem', { name: 'Duplicate' })) + expect( + await screen.findByText('Copy to Another Team') + ).toBeInTheDocument() + await selectDestinationTeam() + await toggleTranslationAndPickFrench() + fireEvent.click(screen.getByRole('button', { name: 'Create' })) + + expect( + await screen.findByText('Failed to duplicate journey') + ).toBeInTheDocument() + expect(translateSubscriptionMock.result).not.toHaveBeenCalled() + expect(handleCloseMenu).not.toHaveBeenCalled() }) it('refetches template stats after a translated copy completes', async () => { diff --git a/apps/journeys-admin/src/components/JourneyList/JourneyCard/JourneyCardMenu/DuplicateJourneyMenuItem/DuplicateJourneyMenuItem.tsx b/apps/journeys-admin/src/components/JourneyList/JourneyCard/JourneyCardMenu/DuplicateJourneyMenuItem/DuplicateJourneyMenuItem.tsx index b2454d7b1f3..1e03ad12cd2 100644 --- a/apps/journeys-admin/src/components/JourneyList/JourneyCard/JourneyCardMenu/DuplicateJourneyMenuItem/DuplicateJourneyMenuItem.tsx +++ b/apps/journeys-admin/src/components/JourneyList/JourneyCard/JourneyCardMenu/DuplicateJourneyMenuItem/DuplicateJourneyMenuItem.tsx @@ -65,10 +65,13 @@ export function DuplicateJourneyMenuItem({ variables: translationVariables, skip: translationVariables == null, onError(error) { - enqueueSnackbar(error.message, { - variant: 'error', - preventDuplicate: true - }) + enqueueSnackbar( + error.message !== '' ? error.message : t('Failed to translate journey'), + { + variant: 'error', + preventDuplicate: true + } + ) setLoading(false) setTranslationVariables(undefined) }, From aa01f7e22bce768581007eefa05e86e5744b57fd Mon Sep 17 00:00:00 2001 From: jianwei1 Date: Tue, 5 May 2026 20:33:13 +0000 Subject: [PATCH 3/5] docs(review): record residual review findings --- ...s-1636-fix-ai-translate-on-copy-to-team.md | 50 +++++++++++++++++++ 1 file changed, 50 insertions(+) create mode 100644 docs/residual-review-findings/jianweichong-nes-1636-fix-ai-translate-on-copy-to-team.md diff --git a/docs/residual-review-findings/jianweichong-nes-1636-fix-ai-translate-on-copy-to-team.md b/docs/residual-review-findings/jianweichong-nes-1636-fix-ai-translate-on-copy-to-team.md new file mode 100644 index 00000000000..d09c25ff149 --- /dev/null +++ b/docs/residual-review-findings/jianweichong-nes-1636-fix-ai-translate-on-copy-to-team.md @@ -0,0 +1,50 @@ +# Residual review findings — NES-1636 + +**Source:** ce-code-review autofix run `20260505-200835-c078a430` against branch `jianweichong/nes-1636-fix-ai-translate-on-copy-to-team` (head `02fc93573` after autofix; pre-autofix `533ed3922`; base `dbbc1e3e` / origin/main). + +**Run artifact:** `/tmp/compound-engineering/ce-code-review/20260505-200835-c078a430/` + +**Reviewers:** correctness, testing, maintainability, project-standards, agent-native, learnings, reliability, adversarial, kieran-typescript, julik-frontend-races (10 personas). + +**Routing decision:** All 16 residuals routed to `no_sink` (inlined here) rather than filed as individual Linear tickets, on the rationale that small-PR code-review residuals belong in the PR body alongside the change rather than as standalone tracker tickets. Tracker filing remains the correct path for genuine cross-PR work; this set is review feedback for the reviewer to weigh. + +## Residual Review Findings + +### P1 — High + +- **#1** [P1, manual] `apps/journeys-admin/src/components/JourneyList/JourneyCard/JourneyCardMenu/DuplicateJourneyMenuItem/DuplicateJourneyMenuItem.tsx:122` — Race: `GetAdminJourneys` refetch from `updateLastActiveTeamId.onCompleted` can unmount the menu item before the translation subscription emits `onComplete`, leaving an orphan untranslated journey with no UI feedback. *Reviewers: adversarial, reliability — confidence 100.* Suggested fix: host the subscription higher in the tree (top-level provider or JourneyList container) so the lifecycle isn't tied to the source-team card, OR defer `updateTeamState` in `CopyToTeamDialog` until the consumer signals translation is complete. Note: the same hazard exists in `TranslateJourneyDialog` (the reference pattern); this is shared subscription-lifecycle debt. +- **#2** [P1, gated_auto] `DuplicateJourneyMenuItem.tsx:207` — Cancel button mid-translation: dialog `onClose` only sets `open=false` but leaves `translationVariables` set, so the subscription continues and `onComplete` fires snackbar/`handleCloseMenu` on a dismissed dialog. *Reviewer: julik-frontend-races — confidence 75.* Suggested fix: make the local `onClose` handler also call `setTranslationVariables(undefined)` and `setLoading(false)` so cancel cancels the in-flight subscription cleanly. Decide whether the backend translation should also abort or be allowed to complete server-side. + +### P2 — Moderate + +- **#3** [P2, manual] `DuplicateJourneyMenuItem.tsx:75` — Subscription `onComplete` unconditionally fires the success snackbar even if the channel terminates without a final `progress=100`/`journey` frame (server crash, deploy restart). *Reviewer: reliability — confidence 75.* Suggested fix: gate the success path on `translationData?.journeyAiTranslateCreateSubscription?.progress === 100` (or `journey != null`) before showing the success snackbar. +- **#4** [P2, manual] `DuplicateJourneyMenuItem.tsx:67` — Subscription error path leaves an orphan untranslated duplicate in the destination team; resubmit creates a second orphan. *Reviewers: reliability, adversarial — confidence 75.* Suggested fix: on subscription error, either delete the orphan via `journeyDelete` before clearing `translationVariables`, or surface a snackbar that names the destination team and instructs the user to retry from there. +- **#5** [P2, manual] `DuplicateJourneyMenuItem.tsx:64-91` — No client-side timeout on the translation subscription: a hung backend leaves `loading=true` and the dialog blocked indefinitely. *Reviewer: reliability — confidence 75.* Suggested fix: add a `setTimeout`-based timeout (e.g., 90s) that clears `translationVariables` and surfaces a timeout snackbar if the subscription hasn't produced `progress=100` by then. +- **#6** [P2, manual] `DuplicateJourneyMenuItem.tsx:113-134` — Translation silently skipped when `journeyData.language` is missing or `duplicatedJourneyId` is null — user sees the success snackbar with no indication translation was skipped. *Reviewer: correctness — confidence 50.* Suggested fix: mirror `TranslateJourneyDialog.tsx:174-176`: throw `'Journey duplication failed'` (or a more specific error) when `showTranslation=true` and any prerequisite is missing, so the user gets an explicit error rather than a misleading success. +- **#7** [P2, gated_auto] `DuplicateJourneyMenuItem.tsx:158-165` — `router.events.on('routeChangeComplete')` listener accumulates per click without removal — pre-existing leak now exercised more by the translation flow. *Reviewer: julik-frontend-races — confidence 75.* Suggested fix: replace with a one-shot listener that removes itself after firing, or attach via `useEffect` with cleanup. +- **#8** [P2, manual] `DuplicateJourneyMenuItem.tsx:93-156` — Stuck `loading=true` when the user dismisses the dialog mid-mutation in the translation branch. *Reviewer: julik-frontend-races — confidence 75.* Suggested fix: tie loading-state cleanup to the dialog's `onClose` path as well as the subscription terminal events. +- **#9** [P2, manual] `DuplicateJourneyMenuItem.tsx:56-60` — Two booleans plus `translationVariables` cannot represent "cancelled mid-flight" or "errored mid-translate"; `setLoading(false)` appears 5× across distributed cleanup paths. *Reviewer: julik-frontend-races — confidence 75.* Suggested fix: consolidate into a single state machine (`'idle' | 'duplicating' | 'translating' | 'cancelled' | 'errored' | 'done'`) with a transition function. Reduces surface area for the race conditions in #1, #2, #8. +- **#16** [P2, manual] `DuplicateJourneyMenuItem.tsx:113` — Missing tests for the post-Yup translation-guard fall-through branches (`showTranslation=true` with missing `language.id`, `duplicatedJourneyId`, or `journeyData.language`). *Reviewer: testing — confidence 75.* Suggested fix: once #6 is resolved (throw or surface an explicit error), add a test asserting the explicit error path. Until then, the test would pin in incorrect behavior. + +### P3 — Low + +- **#10** [P3, manual] `DuplicateJourneyMenuItem.tsx:80` — Snackbar inconsistency: `'Journey Duplicated'` vs `'Journey Copied'` depends on whether `activeTeam` was set when the snackbar fired (translate-off path fires before `updateTeamState`; translate-on `onComplete` fires after). *Reviewers: correctness, julik-frontend-races — confidence 100.* Suggested fix: capture `initialActiveTeamId` at the start of `handleDuplicateJourney` and reference it from `onComplete`, OR key the wording on whether the user explicitly picked another team (always `'Journey Copied'` when `showTranslation=true`). +- **#11** [P3, gated_auto] `DuplicateJourneyMenuItem.tsx:113-148` — When `journeyDuplicate` returns `{ data: { journeyDuplicate: null } }` (partial GraphQL result), the success snackbar still fires. Codegen marks the field non-null but gateways can return null with errors. *Reviewers: correctness, adversarial — confidence 75.* Suggested fix: throw `'Journey duplication failed'` when `duplicatedJourneyId == null`, mirroring `TranslateJourneyDialog.tsx:174-176`. +- **#12** [P3, manual] `DuplicateJourneyMenuItem.spec.tsx:1041-1046` — Happy-path snackbar text asserts `'Journey Duplicated'`, which depends on `updateTeamState` committing before the subscription's `onComplete` reads `activeTeam` — flips silently on shared-dialog reorder. *Reviewers: testing, adversarial — confidence 100.* Suggested fix: assert on `variant=success` only (decouple from wording), or seed `activeTeam` deterministically so the wording is stable. Linked to #10. +- **#13** [P3, gated_auto] `DuplicateJourneyMenuItem.tsx:25-33` — Local `TranslationVariables` interface duplicates the generated `JourneyAiTranslateCreateSubscriptionVariables` shape — drifts silently on schema regen. *Reviewers: maintainability, kieran-typescript — confidence 100.* Suggested fix: import the generated type from `libs/journeys/ui/src/libs/useJourneyAiTranslateSubscription/__generated__/JourneyAiTranslateCreateSubscription` instead of redefining locally. Apply consistently across the 6 callsites with hand-rolled copies. +- **#14** [P3, manual] `DuplicateJourneyMenuItem.tsx:72,87,103,148,154,157` — `setLoading(false)` cleared in five separate paths after the `finally` block was removed — future early-return on a happy path could leave the spinner stuck. *Reviewers: maintainability, kieran-typescript — confidence 75.* Suggested fix: reintroduce a `finally` block with a flag for the translation-skip branch, or consolidate via the state-machine refactor (#9). +- **#15** [P3, manual] `DuplicateJourneyMenuItem.tsx:78-92` — Visual flicker risk: `onComplete` clears `translationVariables` before `setOpen(false)`, risking an empty-form intermediate render outside React 18 batching. *Reviewer: adversarial — confidence 75.* Suggested fix: reorder so `setOpen(false)` fires first, or wrap setters in `unstable_batchedUpdates`, or derive view state from a single source of truth. Same quirk exists in `TranslateJourneyDialog`. + +## Applied autofixes (committed in `02fc93573`) + +- **#17** [safe_auto] `DuplicateJourneyMenuItem.tsx:67-77` — added `t('Failed to translate journey')` fallback for empty `error.message` in subscription `onError`. +- **#18** [safe_auto] `DuplicateJourneyMenuItem.spec.tsx:1083-1088` — strengthened error-path assertion: `handleCloseMenu` and `refetchTemplateStats` are NOT called when subscription errors. +- **#19** [safe_auto] `DuplicateJourneyMenuItem.spec.tsx:1091-1166` — added test for `journeyDuplicate`-error path (Plan scenario #5): asserts the duplicate-failure snackbar appears, no subscription mock is consumed, `handleCloseMenu` is not called. + +## Coverage notes + +- 4 findings suppressed via mode-aware demotion (testing/maintainability advisory P3 — translation-progress mid-flight test; brittle ordering-dependent snackbar assertion; legacy `GET_LANGUAGES` mocks; deferred shared-hook extraction). +- 6 findings dropped at confidence anchor 50 below the gate. +- No validator drops (autofix mode validates only safe_auto candidates inline). +- `ce-agent-native-reviewer`: PASS — both `journeyDuplicate` and `journeyAiTranslateCreateSubscription` are public GraphQL primitives; no agent-tool registry exists in this repo, so there's no per-PR parity gap. +- `ce-learnings-researcher`: `docs/solutions/best-practices/local-template-dialog-consolidation-patterns-nes1543.md` is directly relevant (Patterns 5 and 6 cover the dual-callsite shape this fix creates). Worth a `/ce-compound` capture once this lands. From b65d9e2bf4f03f9aac4703c1e26ff623760bfb35 Mon Sep 17 00:00:00 2001 From: "autofix-ci[bot]" <114827586+autofix-ci[bot]@users.noreply.github.com> Date: Tue, 5 May 2026 20:43:46 +0000 Subject: [PATCH 4/5] fix: lint issues --- ...-1636-ai-translate-on-copy-to-team-plan.md | 12 +++---- ...s-1636-fix-ai-translate-on-copy-to-team.md | 32 +++++++++---------- 2 files changed, 22 insertions(+), 22 deletions(-) diff --git a/docs/plans/2026-05-05-001-fix-nes-1636-ai-translate-on-copy-to-team-plan.md b/docs/plans/2026-05-05-001-fix-nes-1636-ai-translate-on-copy-to-team-plan.md index bf0e11163fc..04d21a6f6e1 100644 --- a/docs/plans/2026-05-05-001-fix-nes-1636-ai-translate-on-copy-to-team-plan.md +++ b/docs/plans/2026-05-05-001-fix-nes-1636-ai-translate-on-copy-to-team-plan.md @@ -157,12 +157,12 @@ This is a regression-shape bug: the dialog API was extended to support translati ## Risks & Dependencies -| Risk | Mitigation | -| ------------------------------------------------------------------------------------------------------------------------------------------------------ | --------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------- | -| Closing the menu (`handleCloseMenu`) before the subscription completes could unmount the component and cancel the in-flight translation client-side. | During implementation, verify by exercising the happy path. If unmounting cancels the subscription, defer `handleCloseMenu` until `onComplete`. If the duplicate journey persists server-side regardless, leave as-is and note the behavior in QA scenarios. | -| Subscribing with an unintended/empty `textLanguageName` would still fire the subscription against the backend. | Mirror `TranslateJourneyDialog.tsx:167-168`'s fallback chain (`nativeName ?? localName ?? ''`) and gate the call on `language?.id` being defined. The `CopyToTeamDialog` Yup schema already requires `languageSelect.id` when `showTranslation` is true. | -| Spec file growth: this component already has 5 long tests; adding 4-6 more could push the file over a maintainability threshold. | Acceptable. Keep helper mock builders DRY, reuse existing patterns. If the file gets unwieldy, extract a `__fixtures__` file in a follow-up — not in this PR. | -| `useJourney()` context fallback may be unavailable in JourneyList card render path, leaving source-language name empty. | The `journey` prop already carries `language.name`; the `useJourney()` fallback is only for a hypothetical caller without a prop. If the prop is present (the JourneyList path), the fallback is never exercised. Mirror `TranslateJourneyDialog`'s fallback shape. | +| Risk | Mitigation | +| ---------------------------------------------------------------------------------------------------------------------------------------------------- | ------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------- | +| Closing the menu (`handleCloseMenu`) before the subscription completes could unmount the component and cancel the in-flight translation client-side. | During implementation, verify by exercising the happy path. If unmounting cancels the subscription, defer `handleCloseMenu` until `onComplete`. If the duplicate journey persists server-side regardless, leave as-is and note the behavior in QA scenarios. | +| Subscribing with an unintended/empty `textLanguageName` would still fire the subscription against the backend. | Mirror `TranslateJourneyDialog.tsx:167-168`'s fallback chain (`nativeName ?? localName ?? ''`) and gate the call on `language?.id` being defined. The `CopyToTeamDialog` Yup schema already requires `languageSelect.id` when `showTranslation` is true. | +| Spec file growth: this component already has 5 long tests; adding 4-6 more could push the file over a maintainability threshold. | Acceptable. Keep helper mock builders DRY, reuse existing patterns. If the file gets unwieldy, extract a `__fixtures__` file in a follow-up — not in this PR. | +| `useJourney()` context fallback may be unavailable in JourneyList card render path, leaving source-language name empty. | The `journey` prop already carries `language.name`; the `useJourney()` fallback is only for a hypothetical caller without a prop. If the prop is present (the JourneyList path), the fallback is never exercised. Mirror `TranslateJourneyDialog`'s fallback shape. | --- diff --git a/docs/residual-review-findings/jianweichong-nes-1636-fix-ai-translate-on-copy-to-team.md b/docs/residual-review-findings/jianweichong-nes-1636-fix-ai-translate-on-copy-to-team.md index d09c25ff149..ea93fba4986 100644 --- a/docs/residual-review-findings/jianweichong-nes-1636-fix-ai-translate-on-copy-to-team.md +++ b/docs/residual-review-findings/jianweichong-nes-1636-fix-ai-translate-on-copy-to-team.md @@ -12,28 +12,28 @@ ### P1 — High -- **#1** [P1, manual] `apps/journeys-admin/src/components/JourneyList/JourneyCard/JourneyCardMenu/DuplicateJourneyMenuItem/DuplicateJourneyMenuItem.tsx:122` — Race: `GetAdminJourneys` refetch from `updateLastActiveTeamId.onCompleted` can unmount the menu item before the translation subscription emits `onComplete`, leaving an orphan untranslated journey with no UI feedback. *Reviewers: adversarial, reliability — confidence 100.* Suggested fix: host the subscription higher in the tree (top-level provider or JourneyList container) so the lifecycle isn't tied to the source-team card, OR defer `updateTeamState` in `CopyToTeamDialog` until the consumer signals translation is complete. Note: the same hazard exists in `TranslateJourneyDialog` (the reference pattern); this is shared subscription-lifecycle debt. -- **#2** [P1, gated_auto] `DuplicateJourneyMenuItem.tsx:207` — Cancel button mid-translation: dialog `onClose` only sets `open=false` but leaves `translationVariables` set, so the subscription continues and `onComplete` fires snackbar/`handleCloseMenu` on a dismissed dialog. *Reviewer: julik-frontend-races — confidence 75.* Suggested fix: make the local `onClose` handler also call `setTranslationVariables(undefined)` and `setLoading(false)` so cancel cancels the in-flight subscription cleanly. Decide whether the backend translation should also abort or be allowed to complete server-side. +- **#1** [P1, manual] `apps/journeys-admin/src/components/JourneyList/JourneyCard/JourneyCardMenu/DuplicateJourneyMenuItem/DuplicateJourneyMenuItem.tsx:122` — Race: `GetAdminJourneys` refetch from `updateLastActiveTeamId.onCompleted` can unmount the menu item before the translation subscription emits `onComplete`, leaving an orphan untranslated journey with no UI feedback. _Reviewers: adversarial, reliability — confidence 100._ Suggested fix: host the subscription higher in the tree (top-level provider or JourneyList container) so the lifecycle isn't tied to the source-team card, OR defer `updateTeamState` in `CopyToTeamDialog` until the consumer signals translation is complete. Note: the same hazard exists in `TranslateJourneyDialog` (the reference pattern); this is shared subscription-lifecycle debt. +- **#2** [P1, gated_auto] `DuplicateJourneyMenuItem.tsx:207` — Cancel button mid-translation: dialog `onClose` only sets `open=false` but leaves `translationVariables` set, so the subscription continues and `onComplete` fires snackbar/`handleCloseMenu` on a dismissed dialog. _Reviewer: julik-frontend-races — confidence 75._ Suggested fix: make the local `onClose` handler also call `setTranslationVariables(undefined)` and `setLoading(false)` so cancel cancels the in-flight subscription cleanly. Decide whether the backend translation should also abort or be allowed to complete server-side. ### P2 — Moderate -- **#3** [P2, manual] `DuplicateJourneyMenuItem.tsx:75` — Subscription `onComplete` unconditionally fires the success snackbar even if the channel terminates without a final `progress=100`/`journey` frame (server crash, deploy restart). *Reviewer: reliability — confidence 75.* Suggested fix: gate the success path on `translationData?.journeyAiTranslateCreateSubscription?.progress === 100` (or `journey != null`) before showing the success snackbar. -- **#4** [P2, manual] `DuplicateJourneyMenuItem.tsx:67` — Subscription error path leaves an orphan untranslated duplicate in the destination team; resubmit creates a second orphan. *Reviewers: reliability, adversarial — confidence 75.* Suggested fix: on subscription error, either delete the orphan via `journeyDelete` before clearing `translationVariables`, or surface a snackbar that names the destination team and instructs the user to retry from there. -- **#5** [P2, manual] `DuplicateJourneyMenuItem.tsx:64-91` — No client-side timeout on the translation subscription: a hung backend leaves `loading=true` and the dialog blocked indefinitely. *Reviewer: reliability — confidence 75.* Suggested fix: add a `setTimeout`-based timeout (e.g., 90s) that clears `translationVariables` and surfaces a timeout snackbar if the subscription hasn't produced `progress=100` by then. -- **#6** [P2, manual] `DuplicateJourneyMenuItem.tsx:113-134` — Translation silently skipped when `journeyData.language` is missing or `duplicatedJourneyId` is null — user sees the success snackbar with no indication translation was skipped. *Reviewer: correctness — confidence 50.* Suggested fix: mirror `TranslateJourneyDialog.tsx:174-176`: throw `'Journey duplication failed'` (or a more specific error) when `showTranslation=true` and any prerequisite is missing, so the user gets an explicit error rather than a misleading success. -- **#7** [P2, gated_auto] `DuplicateJourneyMenuItem.tsx:158-165` — `router.events.on('routeChangeComplete')` listener accumulates per click without removal — pre-existing leak now exercised more by the translation flow. *Reviewer: julik-frontend-races — confidence 75.* Suggested fix: replace with a one-shot listener that removes itself after firing, or attach via `useEffect` with cleanup. -- **#8** [P2, manual] `DuplicateJourneyMenuItem.tsx:93-156` — Stuck `loading=true` when the user dismisses the dialog mid-mutation in the translation branch. *Reviewer: julik-frontend-races — confidence 75.* Suggested fix: tie loading-state cleanup to the dialog's `onClose` path as well as the subscription terminal events. -- **#9** [P2, manual] `DuplicateJourneyMenuItem.tsx:56-60` — Two booleans plus `translationVariables` cannot represent "cancelled mid-flight" or "errored mid-translate"; `setLoading(false)` appears 5× across distributed cleanup paths. *Reviewer: julik-frontend-races — confidence 75.* Suggested fix: consolidate into a single state machine (`'idle' | 'duplicating' | 'translating' | 'cancelled' | 'errored' | 'done'`) with a transition function. Reduces surface area for the race conditions in #1, #2, #8. -- **#16** [P2, manual] `DuplicateJourneyMenuItem.tsx:113` — Missing tests for the post-Yup translation-guard fall-through branches (`showTranslation=true` with missing `language.id`, `duplicatedJourneyId`, or `journeyData.language`). *Reviewer: testing — confidence 75.* Suggested fix: once #6 is resolved (throw or surface an explicit error), add a test asserting the explicit error path. Until then, the test would pin in incorrect behavior. +- **#3** [P2, manual] `DuplicateJourneyMenuItem.tsx:75` — Subscription `onComplete` unconditionally fires the success snackbar even if the channel terminates without a final `progress=100`/`journey` frame (server crash, deploy restart). _Reviewer: reliability — confidence 75._ Suggested fix: gate the success path on `translationData?.journeyAiTranslateCreateSubscription?.progress === 100` (or `journey != null`) before showing the success snackbar. +- **#4** [P2, manual] `DuplicateJourneyMenuItem.tsx:67` — Subscription error path leaves an orphan untranslated duplicate in the destination team; resubmit creates a second orphan. _Reviewers: reliability, adversarial — confidence 75._ Suggested fix: on subscription error, either delete the orphan via `journeyDelete` before clearing `translationVariables`, or surface a snackbar that names the destination team and instructs the user to retry from there. +- **#5** [P2, manual] `DuplicateJourneyMenuItem.tsx:64-91` — No client-side timeout on the translation subscription: a hung backend leaves `loading=true` and the dialog blocked indefinitely. _Reviewer: reliability — confidence 75._ Suggested fix: add a `setTimeout`-based timeout (e.g., 90s) that clears `translationVariables` and surfaces a timeout snackbar if the subscription hasn't produced `progress=100` by then. +- **#6** [P2, manual] `DuplicateJourneyMenuItem.tsx:113-134` — Translation silently skipped when `journeyData.language` is missing or `duplicatedJourneyId` is null — user sees the success snackbar with no indication translation was skipped. _Reviewer: correctness — confidence 50._ Suggested fix: mirror `TranslateJourneyDialog.tsx:174-176`: throw `'Journey duplication failed'` (or a more specific error) when `showTranslation=true` and any prerequisite is missing, so the user gets an explicit error rather than a misleading success. +- **#7** [P2, gated_auto] `DuplicateJourneyMenuItem.tsx:158-165` — `router.events.on('routeChangeComplete')` listener accumulates per click without removal — pre-existing leak now exercised more by the translation flow. _Reviewer: julik-frontend-races — confidence 75._ Suggested fix: replace with a one-shot listener that removes itself after firing, or attach via `useEffect` with cleanup. +- **#8** [P2, manual] `DuplicateJourneyMenuItem.tsx:93-156` — Stuck `loading=true` when the user dismisses the dialog mid-mutation in the translation branch. _Reviewer: julik-frontend-races — confidence 75._ Suggested fix: tie loading-state cleanup to the dialog's `onClose` path as well as the subscription terminal events. +- **#9** [P2, manual] `DuplicateJourneyMenuItem.tsx:56-60` — Two booleans plus `translationVariables` cannot represent "cancelled mid-flight" or "errored mid-translate"; `setLoading(false)` appears 5× across distributed cleanup paths. _Reviewer: julik-frontend-races — confidence 75._ Suggested fix: consolidate into a single state machine (`'idle' | 'duplicating' | 'translating' | 'cancelled' | 'errored' | 'done'`) with a transition function. Reduces surface area for the race conditions in #1, #2, #8. +- **#16** [P2, manual] `DuplicateJourneyMenuItem.tsx:113` — Missing tests for the post-Yup translation-guard fall-through branches (`showTranslation=true` with missing `language.id`, `duplicatedJourneyId`, or `journeyData.language`). _Reviewer: testing — confidence 75._ Suggested fix: once #6 is resolved (throw or surface an explicit error), add a test asserting the explicit error path. Until then, the test would pin in incorrect behavior. ### P3 — Low -- **#10** [P3, manual] `DuplicateJourneyMenuItem.tsx:80` — Snackbar inconsistency: `'Journey Duplicated'` vs `'Journey Copied'` depends on whether `activeTeam` was set when the snackbar fired (translate-off path fires before `updateTeamState`; translate-on `onComplete` fires after). *Reviewers: correctness, julik-frontend-races — confidence 100.* Suggested fix: capture `initialActiveTeamId` at the start of `handleDuplicateJourney` and reference it from `onComplete`, OR key the wording on whether the user explicitly picked another team (always `'Journey Copied'` when `showTranslation=true`). -- **#11** [P3, gated_auto] `DuplicateJourneyMenuItem.tsx:113-148` — When `journeyDuplicate` returns `{ data: { journeyDuplicate: null } }` (partial GraphQL result), the success snackbar still fires. Codegen marks the field non-null but gateways can return null with errors. *Reviewers: correctness, adversarial — confidence 75.* Suggested fix: throw `'Journey duplication failed'` when `duplicatedJourneyId == null`, mirroring `TranslateJourneyDialog.tsx:174-176`. -- **#12** [P3, manual] `DuplicateJourneyMenuItem.spec.tsx:1041-1046` — Happy-path snackbar text asserts `'Journey Duplicated'`, which depends on `updateTeamState` committing before the subscription's `onComplete` reads `activeTeam` — flips silently on shared-dialog reorder. *Reviewers: testing, adversarial — confidence 100.* Suggested fix: assert on `variant=success` only (decouple from wording), or seed `activeTeam` deterministically so the wording is stable. Linked to #10. -- **#13** [P3, gated_auto] `DuplicateJourneyMenuItem.tsx:25-33` — Local `TranslationVariables` interface duplicates the generated `JourneyAiTranslateCreateSubscriptionVariables` shape — drifts silently on schema regen. *Reviewers: maintainability, kieran-typescript — confidence 100.* Suggested fix: import the generated type from `libs/journeys/ui/src/libs/useJourneyAiTranslateSubscription/__generated__/JourneyAiTranslateCreateSubscription` instead of redefining locally. Apply consistently across the 6 callsites with hand-rolled copies. -- **#14** [P3, manual] `DuplicateJourneyMenuItem.tsx:72,87,103,148,154,157` — `setLoading(false)` cleared in five separate paths after the `finally` block was removed — future early-return on a happy path could leave the spinner stuck. *Reviewers: maintainability, kieran-typescript — confidence 75.* Suggested fix: reintroduce a `finally` block with a flag for the translation-skip branch, or consolidate via the state-machine refactor (#9). -- **#15** [P3, manual] `DuplicateJourneyMenuItem.tsx:78-92` — Visual flicker risk: `onComplete` clears `translationVariables` before `setOpen(false)`, risking an empty-form intermediate render outside React 18 batching. *Reviewer: adversarial — confidence 75.* Suggested fix: reorder so `setOpen(false)` fires first, or wrap setters in `unstable_batchedUpdates`, or derive view state from a single source of truth. Same quirk exists in `TranslateJourneyDialog`. +- **#10** [P3, manual] `DuplicateJourneyMenuItem.tsx:80` — Snackbar inconsistency: `'Journey Duplicated'` vs `'Journey Copied'` depends on whether `activeTeam` was set when the snackbar fired (translate-off path fires before `updateTeamState`; translate-on `onComplete` fires after). _Reviewers: correctness, julik-frontend-races — confidence 100._ Suggested fix: capture `initialActiveTeamId` at the start of `handleDuplicateJourney` and reference it from `onComplete`, OR key the wording on whether the user explicitly picked another team (always `'Journey Copied'` when `showTranslation=true`). +- **#11** [P3, gated_auto] `DuplicateJourneyMenuItem.tsx:113-148` — When `journeyDuplicate` returns `{ data: { journeyDuplicate: null } }` (partial GraphQL result), the success snackbar still fires. Codegen marks the field non-null but gateways can return null with errors. _Reviewers: correctness, adversarial — confidence 75._ Suggested fix: throw `'Journey duplication failed'` when `duplicatedJourneyId == null`, mirroring `TranslateJourneyDialog.tsx:174-176`. +- **#12** [P3, manual] `DuplicateJourneyMenuItem.spec.tsx:1041-1046` — Happy-path snackbar text asserts `'Journey Duplicated'`, which depends on `updateTeamState` committing before the subscription's `onComplete` reads `activeTeam` — flips silently on shared-dialog reorder. _Reviewers: testing, adversarial — confidence 100._ Suggested fix: assert on `variant=success` only (decouple from wording), or seed `activeTeam` deterministically so the wording is stable. Linked to #10. +- **#13** [P3, gated_auto] `DuplicateJourneyMenuItem.tsx:25-33` — Local `TranslationVariables` interface duplicates the generated `JourneyAiTranslateCreateSubscriptionVariables` shape — drifts silently on schema regen. _Reviewers: maintainability, kieran-typescript — confidence 100._ Suggested fix: import the generated type from `libs/journeys/ui/src/libs/useJourneyAiTranslateSubscription/__generated__/JourneyAiTranslateCreateSubscription` instead of redefining locally. Apply consistently across the 6 callsites with hand-rolled copies. +- **#14** [P3, manual] `DuplicateJourneyMenuItem.tsx:72,87,103,148,154,157` — `setLoading(false)` cleared in five separate paths after the `finally` block was removed — future early-return on a happy path could leave the spinner stuck. _Reviewers: maintainability, kieran-typescript — confidence 75._ Suggested fix: reintroduce a `finally` block with a flag for the translation-skip branch, or consolidate via the state-machine refactor (#9). +- **#15** [P3, manual] `DuplicateJourneyMenuItem.tsx:78-92` — Visual flicker risk: `onComplete` clears `translationVariables` before `setOpen(false)`, risking an empty-form intermediate render outside React 18 batching. _Reviewer: adversarial — confidence 75._ Suggested fix: reorder so `setOpen(false)` fires first, or wrap setters in `unstable_batchedUpdates`, or derive view state from a single source of truth. Same quirk exists in `TranslateJourneyDialog`. ## Applied autofixes (committed in `02fc93573`) From 8781f61f881894c4617d1be2263ba8b24a173c52 Mon Sep 17 00:00:00 2001 From: jianwei1 Date: Wed, 6 May 2026 21:57:07 +0000 Subject: [PATCH 5/5] fix(NES-1636): surface explicit error when translation prereqs are missing Address CodeRabbit feedback on PR #9161 (apps/.../DuplicateJourneyMenuItem.tsx line 137): when showTranslation is true but a prerequisite is missing (language.id, duplicatedJourneyId, journeyData.language), surface "Failed to translate journey" instead of falling through to the duplicate-only success snackbar. Recreating the silent-skip failure mode this PR was meant to fix would defeat the point. Yup blocks the common "no language selected" case before submit; this branch is a defensive backstop for partial GraphQL responses (e.g. gateway returning { data: { journeyDuplicate: null } } despite codegen marking the field non-null) and for callers without a journey context. Adds a unit test covering the new defensive branch. Skipped the second CodeRabbit comment (line 215, "Closing the dialog does not cancel the in-flight translation state"). TranslationDialogWrapper hides the Cancel and Create buttons while loading is true, and CopyToTeamDialog blocks backdrop click and Escape during translation, so onClose is not user-reachable while translationVariables is set. --- .../DuplicateJourneyMenuItem.spec.tsx | 82 +++++++++++++++++++ .../DuplicateJourneyMenuItem.tsx | 25 ++++-- 2 files changed, 101 insertions(+), 6 deletions(-) diff --git a/apps/journeys-admin/src/components/JourneyList/JourneyCard/JourneyCardMenu/DuplicateJourneyMenuItem/DuplicateJourneyMenuItem.spec.tsx b/apps/journeys-admin/src/components/JourneyList/JourneyCard/JourneyCardMenu/DuplicateJourneyMenuItem/DuplicateJourneyMenuItem.spec.tsx index 1911b100f80..0deb925f13d 100644 --- a/apps/journeys-admin/src/components/JourneyList/JourneyCard/JourneyCardMenu/DuplicateJourneyMenuItem/DuplicateJourneyMenuItem.spec.tsx +++ b/apps/journeys-admin/src/components/JourneyList/JourneyCard/JourneyCardMenu/DuplicateJourneyMenuItem/DuplicateJourneyMenuItem.spec.tsx @@ -1163,6 +1163,88 @@ describe('DuplicateJourneys', () => { expect(handleCloseMenu).not.toHaveBeenCalled() }) + it('shows an explicit error when translation is requested but the duplicate response has no id', async () => { + const journeyDuplicateNullMock = { + request: { + query: JOURNEY_DUPLICATE, + variables: { + id: 'journey-id', + teamId: 'destinationTeamId' + } + }, + result: jest.fn(() => ({ + data: { + journeyDuplicate: null + } + })) + } + + const translateSubscriptionMock = { + request: { + query: JOURNEY_AI_TRANSLATE_CREATE_SUBSCRIPTION, + variables: { + journeyId: 'duplicatedJourneyId', + name: 'Default Journey Heading', + journeyLanguageName: '', + textLanguageId: '496', + textLanguageName: 'Français', + userLanguageId: '529', + userLanguageName: '' + } + }, + result: jest.fn(() => ({ + data: { + journeyAiTranslateCreateSubscription: { + progress: 100, + message: 'Translation completed', + journey: null, + __typename: 'JourneyAiTranslateProgress' + } + } + })) + } + + render( + + + + + + + + ) + + await waitFor(() => expect(teamsMock.result).toHaveBeenCalled()) + fireEvent.click(screen.getByRole('menuitem', { name: 'Duplicate' })) + expect( + await screen.findByText('Copy to Another Team') + ).toBeInTheDocument() + await selectDestinationTeam() + await toggleTranslationAndPickFrench() + fireEvent.click(screen.getByRole('button', { name: 'Create' })) + + expect( + await screen.findByText('Failed to translate journey') + ).toBeInTheDocument() + // The subscription must NOT start when prerequisites are missing. + expect(translateSubscriptionMock.result).not.toHaveBeenCalled() + // The success snackbar must NOT show. + expect(screen.queryByText('Journey Copied')).not.toBeInTheDocument() + expect(screen.queryByText('Journey Duplicated')).not.toBeInTheDocument() + }) + it('refetches template stats after a translated copy completes', async () => { const translateSubscriptionMock = { request: { diff --git a/apps/journeys-admin/src/components/JourneyList/JourneyCard/JourneyCardMenu/DuplicateJourneyMenuItem/DuplicateJourneyMenuItem.tsx b/apps/journeys-admin/src/components/JourneyList/JourneyCard/JourneyCardMenu/DuplicateJourneyMenuItem/DuplicateJourneyMenuItem.tsx index 1e03ad12cd2..97716005e75 100644 --- a/apps/journeys-admin/src/components/JourneyList/JourneyCard/JourneyCardMenu/DuplicateJourneyMenuItem/DuplicateJourneyMenuItem.tsx +++ b/apps/journeys-admin/src/components/JourneyList/JourneyCard/JourneyCardMenu/DuplicateJourneyMenuItem/DuplicateJourneyMenuItem.tsx @@ -113,12 +113,25 @@ export function DuplicateJourneyMenuItem({ const duplicatedJourneyId = duplicateData?.journeyDuplicate?.id - if ( - showTranslation === true && - language?.id != null && - duplicatedJourneyId != null && - journeyData?.language != null - ) { + if (showTranslation === true) { + if ( + language?.id == null || + duplicatedJourneyId == null || + journeyData?.language == null + ) { + // Translation was explicitly requested but a prerequisite is missing + // (Yup normally blocks this — defensive against partial GraphQL + // responses or callers without a journey context). Surface an + // explicit error rather than falling through to the duplicate-only + // success snackbar. + enqueueSnackbar(t('Failed to translate journey'), { + variant: 'error', + preventDuplicate: true + }) + setLoading(false) + return + } + const sourceLanguageName = journeyData.language.name.find(({ primary }) => !primary)?.value ?? ''