feat(NES-1637): add "Copy to collection" action to template cards in Collections#9237
Open
csiyang wants to merge 14 commits into
Open
feat(NES-1637): add "Copy to collection" action to template cards in Collections#9237csiyang wants to merge 14 commits into
csiyang wants to merge 14 commits into
Conversation
…oCollectionMenuItem
Contributor
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Repository UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (9)
✅ Files skipped from review due to trivial changes (3)
WalkthroughAdds a "Copy to collection…" menu item gated by the ChangesCopy to collection feature
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Suggested reviewers
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Contributor
|
View your CI Pipeline Execution ↗ for commit 4419323
☁️ Nx Cloud last updated this comment at |
|
View your CI Pipeline Execution ↗ for commit 02214be
☁️ Nx Cloud last updated this comment at |
Contributor
|
The latest updates on your projects.
|
…emplates; drop InCollectionContext
…item Removing the gate in 2f6b4dd 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.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 4
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In
`@apps/journeys-admin/src/components/JourneyList/JourneyCard/JourneyCardMenu/DefaultMenu/DefaultMenu.spec.tsx`:
- Around line 1094-1194: Extend the existing test suite around
renderWithFlagAndTemplate/DefaultMenu to cover the collection-context gate by
adding test cases for both "in collection" and "not in collection" states (in
addition to the current flag × template combinations); simulate a journey with
and without a team (or explicitly set journey.team to the active team id vs
undefined) so the isLocalTemplate/collection context branch is exercised, and
assert presence/absence of the JourneysAdminMenuItemCopyToCollection test id and
the 'Copy to ...' menuitem accordingly to ensure visibility is tested for all
three dimensions: flag, template, and collection context.
In
`@apps/journeys-admin/src/components/JourneyList/JourneyCard/JourneyCardMenu/DefaultMenu/DefaultMenu.tsx`:
- Around line 306-314: The condition rendering CopyToCollectionMenuItem is
missing the collection-context predicate: currently it only checks
teamTemplateCollection and template, allowing the menu to appear outside
Collections; update the JSX conditional that renders CopyToCollectionMenuItem to
include the collection-context boolean (the same prop or state used elsewhere to
indicate "inside collection" — e.g., collectionContext, isInCollectionView, or
the existing collection prop) so the component only renders when
teamTemplateCollection === true && template === true && <collection-context> ===
true, leaving the props (id, journey, handleCloseMenu, handleKeepMounted,
setHasOpenDialog) unchanged.
In
`@apps/journeys-admin/src/components/TemplateGalleryPageList/CopyToCollectionMenuItem/CopyToCollectionMenuItem.spec.tsx`:
- Around line 317-319: The test helper declares resolveDuplicate as an empty
arrow function which violates the no-empty-function rule; replace its
initializer with a non-empty stub (for example change the default from () => {}
to () => undefined or to jest.fn()) and do the same for the other similar test
helpers referenced in this file (the other empty arrow initializers around the
CopyToCollectionMenuItem.spec.tsx tests). Keep the type signature ({ data?: {
journeyDuplicate?: { id: string } | null } }) => void and only change the
initializer to a benign no-op or jest.fn() so ESLint no-longer flags it.
In `@docs/plans/2026-05-21-001-feat-nes-1637-copy-to-collection-action-plan.md`:
- Around line 258-290: The fenced code block containing the pseudocode for
handleSubmit, runAssign and the onComplete/onError flows is missing a language
tag (MD040); update the opening fence to include a language identifier (for
example "text" or "txt") so the block becomes ```text and leave the closing
fence as is; this will satisfy markdownlint without changing any of the
pseudocode references such as handleSubmit, runAssign,
useJourneyAiTranslateSubscription, onComplete, onError or guardedClose.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
Run ID: 47996ec1-ea5c-4cf1-a2ce-c63019cb256e
📒 Files selected for processing (9)
apps/journeys-admin/src/components/JourneyList/JourneyCard/JourneyCardMenu/DefaultMenu/DefaultMenu.spec.tsxapps/journeys-admin/src/components/JourneyList/JourneyCard/JourneyCardMenu/DefaultMenu/DefaultMenu.tsxapps/journeys-admin/src/components/TemplateGalleryPageList/CopyToCollectionDialog/CopyToCollectionDialog.spec.tsxapps/journeys-admin/src/components/TemplateGalleryPageList/CopyToCollectionDialog/CopyToCollectionDialog.tsxapps/journeys-admin/src/components/TemplateGalleryPageList/CopyToCollectionDialog/index.tsapps/journeys-admin/src/components/TemplateGalleryPageList/CopyToCollectionMenuItem/CopyToCollectionMenuItem.spec.tsxapps/journeys-admin/src/components/TemplateGalleryPageList/CopyToCollectionMenuItem/CopyToCollectionMenuItem.tsxapps/journeys-admin/src/components/TemplateGalleryPageList/CopyToCollectionMenuItem/index.tsdocs/plans/2026-05-21-001-feat-nes-1637-copy-to-collection-action-plan.md
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
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.
edmonday
reviewed
May 21, 2026
Contributor
edmonday
left a comment
edmonday
left a comment
There was a problem hiding this comment.
Review summary
Concern: 1 — translation-failure copy doesn't tell the user the duplicate already exists in All Templates (the duplicate has been created and refetched by this point; only the translation/assign chain failed). Inconsistent with the assign-failure copy right next to it. User may retry and create a second duplicate.
Nit: 1 — minor cleanup hygiene on mount.
Otherwise solid: refs pattern handles subscription/unmount races cleanly, StrictMode-aware mountedRef, single-flight guard, aria-live status region, and 29 specs covering the full error matrix. CI green.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Adds a
Copy to collection...action to template journey cards rendered inside a Collection on the gallery page. Submitting the dialog runsjourneyDuplicate→ optionaluseJourneyAiTranslateSubscription→templateGalleryPageAssignJourney, with a single dialog-internal spinner and a Done state on success. Gated by theteamTemplateCollectionLaunchDarkly flag and anInCollectionContextso the action only appears inside Collections.Linear: NES-1637
What's in the PR
Test plan
Automated (passing locally, 67/67 across all relevant specs):
Manual QA (with `teamTemplateCollection` flag ON):
🤖 Generated with Claude Code
Summary by CodeRabbit
New Features
Tests
Documentation