feat: ai translation#6543
Conversation
Co-authored-by: Tanner Fleming <tanner.fleming@gmail.com> Co-authored-by: autofix-ci[bot] <114827586+autofix-ci[bot]@users.noreply.github.com>
Co-authored-by: Tanner Fleming <tanner.fleming@gmail.com> Co-authored-by: autofix-ci[bot] <114827586+autofix-ci[bot]@users.noreply.github.com>
Co-authored-by: autofix-ci[bot] <114827586+autofix-ci[bot]@users.noreply.github.com>
- Updated the `wrangler.toml` to change the `ENDPOINT_ARCLIGHT_WEIGHT` to "0". - Added new localization files for various languages including Japanese, Korean, and multiple others. - Introduced new components in the `journeys-ui` for better internationalization support. - Enhanced the worker scripts for improved functionality and maintenance.
WalkthroughThis update introduces a comprehensive journey translation feature to the admin and UI components. It adds dialog interfaces for translating journeys, supports duplication with optional translation, and enhances related menus and dialogs. Supporting hooks, localization keys, and tests are included. Several components are refactored to streamline dialog state management and improve translation workflows. Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant JourneyCardMenu
participant DefaultMenu
participant TranslateJourneyDialog
participant useJourneyDuplicateAndTranslate
participant useJourneyAiTranslateMutation
User->>JourneyCardMenu: Opens menu
JourneyCardMenu->>DefaultMenu: Renders menu items
User->>DefaultMenu: Clicks "Translate"
DefaultMenu->>JourneyCardMenu: setOpenTranslateDialog(true)
JourneyCardMenu->>TranslateJourneyDialog: Renders dialog (open)
User->>TranslateJourneyDialog: Selects language, clicks "Create"
TranslateJourneyDialog->>useJourneyDuplicateAndTranslate: duplicateAndTranslate(teamId, language, true)
useJourneyDuplicateAndTranslate->>useJourneyAiTranslateMutation: If translation requested, call mutation
useJourneyAiTranslateMutation-->>useJourneyDuplicateAndTranslate: Returns translated journey
useJourneyDuplicateAndTranslate-->>TranslateJourneyDialog: Success callback, close dialog
TranslateJourneyDialog-->>User: Shows success message
Possibly related PRs
Suggested reviewers
Warning There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure. 🔧 ESLint
npm warn config production Use 📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (1)
🧰 Additional context used🪛 Biome (1.9.4)libs/journeys/ui/src/components/CopyToTeamDialog/CopyToTeamDialog.tsx[error] 130-130: Do not add then to an object. (lint/suspicious/noThenProperty) ⏰ Context from checks skipped due to timeout of 90000ms (21)
🔇 Additional comments (6)
✨ Finishing Touches
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. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
Co-authored-by: Tanner Fleming <tanner.fleming@gmail.com> Co-authored-by: autofix-ci[bot] <114827586+autofix-ci[bot]@users.noreply.github.com>
- Added the Language type to the journey schema, marking it as shareable. - Updated the journey resolver to include the language field, ensuring it resolves correctly. - Introduced a new language module with external reference implementation. - Adjusted imports in the schema to include the new language definitions.
- Updated the Language type in the GraphQL schema to mark the ID field as an external reference. - Adjusted the implementation in the language module to reflect that no additional fields are needed.
…/25-04-MA-feat-ai-translation
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (9)
apis/api-journeys-modern/src/lib/auth/ability.spec.ts (2)
19-19: Minor style note regarding leading semicolon.The leading semicolon is used to prevent issues with automatic semicolon insertion. While functional, consider using a consistent approach across the codebase for readability.
11-12: Consider strengthening type definitions.The tests use
as anytype assertions which bypass TypeScript's type checking. While common in tests, consider creating proper mock types that match the expected interface structure for better type safety and documentation.- const mockUser = { id: 'user1' } as any - const mockJourney = { id: 'journey1' } as any + // Create interfaces or use existing ones + interface User { + id: string; + // other required properties + } + + interface Journey { + id: string; + // other required properties + } + + const mockUser = { id: 'user1' } as User + const mockJourney = { id: 'journey1' } as Journeyapis/api-journeys-modern/src/schema/journeyAiTranslate/getCardBlocksContent/getBlockContent.spec.ts (2)
9-21: Mock helpers asjest.fn().mockResolvedValue()for clearer intentTwo helpers are asynchronous (
getImageBlockContent,getVideoBlockContent) while the others are synchronous. Usingjest.fn(() => Promise.resolve(...))works, but the more idiomatic and readable form is:jest.fn().mockResolvedValue('image-content')(analogous
mockReturnValuefor the sync helpers).
This also prevents accidental wrapping of a non-Promise value if the helper later becomes async.
30-74: Reduce duplication withdescribe.eachThe 6 test cases that check delegation all share the same arrange-act-assert structure. Parameterising them will shrink the file and make it easier to add more block types:
describe.each` type | factory | calledWith ${'ImageBlock'} | ${() => ({ __typename: 'ImageBlock' })} | ${{ block: expect.any(Object) }} ${'VideoBlock'} | ${() => ({ __typename: 'VideoBlock' })} | ${{ blocks: expect.any(Array), block: expect.any(Object) }} ... `('delegates $type', async ({ factory, calledWith }) => { const result = await getBlockContent({ blocks, block: factory() }) expect(mockedHelper).toHaveBeenCalledWith(calledWith) expect(result).toBe(expected) })apis/api-journeys-modern/src/schema/journeyAiTranslate/getCardBlocksContent/getCardBlocksContent.spec.ts (3)
7-12: Strongly type the mocked helpersCasting with
as jest.Mocklater in the file leaksany-typed functions and defeats TypeScript’s safety. Provide the full generic signature when mocking:import type { getBlockContent as realGetBlockContent } from './getBlockContent' jest.mock('./getBlockContent', () => ({ getBlockContent: jest.fn<ReturnType<typeof realGetBlockContent>, Parameters<typeof realGetBlockContent>>() }))This preserves type inference for the
mockImplementationthat follows.
44-50: Accidental ASI hazard – keep the semicolon with IIFE-style castsThe leading
;before(getBlockContent as jest.Mock)…prevents JavaScript’s automatic semicolon insertion from concatenating with the previous statement when the file is minified or transformed. Good catch! Add a comment explaining why it exists so future refactors don’t remove it inadvertently:// Ensure previous statement is terminated before the cast ;(getBlockContent as jest.Mock)...
52-69: Use regex or split assertions instead ofindexOfordering
indexOfon strings can give false positives when substrings repeat. Prefer splitting the result into lines and asserting on the relative order:const lines = result[0].split('\n') const idxChild2 = lines.indexOf('block:child2') const idxChild1 = lines.indexOf('block:child1') expect(idxChild2).toBeLessThan(idxChild1)or use a single regex:
expect(result[0]).toMatch(/block:child2[\s\S]*block:child1/)apis/api-journeys-modern/src/schema/journeyAiTranslate/journeyAiTranslate.spec.ts (2)
40-54: Async iterator stub lacksreturn/throw– may leave open handles
for-awaitloops callreturn()orthrow()on the iterator when they finish early or on error. Omitting these optional methods isn’t spec-breaking, but Node will emit an “iterator not closed” warning in future versions. Adding no-op implementations prevents that:return { [Symbol.asyncIterator]() { let index = 0 return { next: () => Promise.resolve( index < items.length ? { done: false, value: items[index++] } : { done: true, value: undefined } ), return: () => Promise.resolve({ done: true, value: undefined }), throw : (e) => Promise.reject(e) } } }
191-231: Use a fixedcreatedAtdate for deterministic snapshots
new Date()introduces time-dependent behaviour that can break snapshot or CI comparisons when the string representation differs across environments/time-zones. Replace withnew Date('2023-01-01T00:00:00Z')or a constant timestamp.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (13)
apis/api-journeys-modern/schema.graphql(3 hunks)apis/api-journeys-modern/src/lib/auth/ability.spec.ts(1 hunks)apis/api-journeys-modern/src/lib/auth/ability.ts(1 hunks)apis/api-journeys-modern/src/schema/journeyAiTranslate/getCardBlocksContent/getBlockContent.spec.ts(1 hunks)apis/api-journeys-modern/src/schema/journeyAiTranslate/getCardBlocksContent/getBlockContent.ts(1 hunks)apis/api-journeys-modern/src/schema/journeyAiTranslate/getCardBlocksContent/getButtonBlockContent.spec.ts(1 hunks)apis/api-journeys-modern/src/schema/journeyAiTranslate/getCardBlocksContent/getCardBlocksContent.spec.ts(1 hunks)apis/api-journeys-modern/src/schema/journeyAiTranslate/getCardBlocksContent/getCoverBlockContent.spec.ts(1 hunks)apis/api-journeys-modern/src/schema/journeyAiTranslate/getCardBlocksContent/getImageBlockContent.spec.ts(1 hunks)apis/api-journeys-modern/src/schema/journeyAiTranslate/getCardBlocksContent/getRadioQuestionBlockContent.spec.ts(1 hunks)apis/api-journeys-modern/src/schema/journeyAiTranslate/getCardBlocksContent/getVideoBlockContent.spec.ts(1 hunks)apis/api-journeys-modern/src/schema/journeyAiTranslate/journeyAiTranslate.spec.ts(1 hunks)apis/api-journeys-modern/src/schema/language/language.ts(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (4)
- apis/api-journeys-modern/src/schema/language/language.ts
- apis/api-journeys-modern/src/schema/journeyAiTranslate/getCardBlocksContent/getBlockContent.ts
- apis/api-journeys-modern/src/lib/auth/ability.ts
- apis/api-journeys-modern/schema.graphql
🧰 Additional context used
🧬 Code Graph Analysis (4)
apis/api-journeys-modern/src/schema/journeyAiTranslate/getCardBlocksContent/getButtonBlockContent.spec.ts (1)
apis/api-journeys-modern/src/schema/journeyAiTranslate/getCardBlocksContent/getButtonBlockContent.ts (1)
getButtonBlockContent(3-9)
apis/api-journeys-modern/src/schema/journeyAiTranslate/getCardBlocksContent/getImageBlockContent.spec.ts (1)
apis/api-journeys-modern/src/schema/journeyAiTranslate/getCardBlocksContent/getImageBlockContent.ts (1)
getImageBlockContent(4-31)
apis/api-journeys-modern/src/schema/journeyAiTranslate/getCardBlocksContent/getRadioQuestionBlockContent.spec.ts (1)
apis/api-journeys-modern/src/schema/journeyAiTranslate/getCardBlocksContent/getRadioQuestionBlockContent.ts (1)
getRadioQuestionBlockContent(3-23)
apis/api-journeys-modern/src/schema/journeyAiTranslate/getCardBlocksContent/getVideoBlockContent.spec.ts (2)
libs/journeys/ui/src/components/Card/Card.mock.ts (1)
videoBlock(193-244)apis/api-journeys-modern/src/schema/journeyAiTranslate/getCardBlocksContent/getVideoBlockContent.ts (1)
getVideoBlockContent(4-44)
⏰ Context from checks skipped due to timeout of 90000ms (17)
- GitHub Check: Deploy Preview and Test (journeys-admin, 6543/merge, pull_request, 22)
- GitHub Check: Deploy Preview and Test (videos-admin, 6543/merge, pull_request, 22)
- GitHub Check: Deploy Preview and Test (watch, 6543/merge, pull_request, 22)
- GitHub Check: Deploy Preview and Test (short-links, 6543/merge, pull_request, 22)
- GitHub Check: Deploy Preview and Test (journeys, 6543/merge, pull_request, 22)
- GitHub Check: Deploy Preview and Test (watch, feature/25-04-MA-feat-ai-translation, push, 22)
- GitHub Check: Deploy Preview and Test (journeys, feature/25-04-MA-feat-ai-translation, push, 22)
- GitHub Check: Deploy Preview and Test (journeys-admin, feature/25-04-MA-feat-ai-translation, push, 22)
- GitHub Check: Deploy Preview and Test (short-links, feature/25-04-MA-feat-ai-translation, push, 22)
- GitHub Check: Deploy Preview and Test (arclight, feature/25-04-MA-feat-ai-translation, push, 22)
- GitHub Check: Deploy Preview and Test (videos-admin, feature/25-04-MA-feat-ai-translation, push, 22)
- GitHub Check: lint (22)
- GitHub Check: test (22, 3/3)
- GitHub Check: build (22)
- GitHub Check: test (22, 2/3)
- GitHub Check: test (22, 1/3)
- GitHub Check: Analyze (javascript)
🔇 Additional comments (19)
apis/api-journeys-modern/src/lib/auth/ability.spec.ts (6)
1-8: Well-structured test setup with appropriate mocking.The import statements and mock setup are clear and follow best practices. Mocking the
journeyAclfunction appropriately isolates the ability function under test.
10-16: Good test preparation with proper test isolation.The test suite uses appropriate mock objects and implements
beforeEachto clear mocks between tests, ensuring test cases don't affect each other.
18-31: Well-implemented positive test case.This test correctly verifies that the
abilityfunction returns true when the underlyingjourneyAclfunction returns true, and confirms the correct parameters are passed.
33-46: Well-implemented negative test case.This test properly verifies that the
abilityfunction returns false when the underlyingjourneyAclfunction returns false, maintaining consistency with the positive test case structure.
48-60: Good edge case handling for unknown subjects.The test correctly verifies that the
abilityfunction returns false for unknown subject types without callingjourneyAcl, demonstrating robust error handling.
62-67: Appropriate handling of undefined subject objects.The test uses
@ts-expect-errorappropriately to test the edge case of undefined subject objects, ensuring the function handles this gracefully without crashing.apis/api-journeys-modern/src/schema/journeyAiTranslate/getCardBlocksContent/getButtonBlockContent.spec.ts (1)
1-30: Comprehensive button block content extraction tests!This test suite thoroughly verifies the
getButtonBlockContentfunction for all critical scenarios:
- Successfully formats output with block ID and label
- Properly handles null labels
- Correctly handles undefined labels
The tests ensure the function works reliably for the AI translation feature.
apis/api-journeys-modern/src/schema/journeyAiTranslate/getCardBlocksContent/getRadioQuestionBlockContent.spec.ts (2)
5-20: Well-structured test fixturesThe test data setup is clear and reusable, with appropriate typing using
Partial<Block>. The different test cases can effectively reuse these fixtures.
22-64: Complete test coverage for radio question content extractionThis test suite thoroughly covers all relevant scenarios:
- Includes block ID and label in output
- Lists all radio question block labels
- Properly filters out non-radio blocks
- Handles single radio question block case
- Handles empty blocks array gracefully
The tests ensure the function reliably extracts and formats radio question content for AI translation.
apis/api-journeys-modern/src/schema/journeyAiTranslate/getCardBlocksContent/getImageBlockContent.spec.ts (3)
6-8: Proper mocking approachGood use of Jest's mocking capabilities to isolate the component under test.
13-15: Good test hygieneUsing
beforeEachto clear mocks between tests prevents test pollution and ensures test isolation.
17-60: Comprehensive image block content test suiteThe test cases effectively cover:
- Successfully retrieving and using image descriptions
- Handling missing descriptions
- Handling missing image sources
- Correctly using different labels based on the block type
This thorough coverage ensures reliable image content extraction for AI translation.
apis/api-journeys-modern/src/schema/journeyAiTranslate/getCardBlocksContent/getVideoBlockContent.spec.ts (3)
10-21: Well-structured test fixturesGood approach to setting up reusable test data that models the relationship between video blocks and their poster blocks.
48-56: Excellent error handling testGood test for verifying graceful error handling when
getImageDescriptionthrows an exception. This ensures the translation process won't fail if image description generation encounters issues.
27-102: Thorough video block content test coverageThis test suite comprehensively covers all scenarios:
- Successfully retrieving video descriptions from poster images
- Handling missing descriptions
- Handling API errors
- Handling missing poster blocks
- Handling poster blocks with no source
- Correctly using different labels based on block type
The tests ensure reliable and resilient video content extraction for the AI translation feature.
apis/api-journeys-modern/src/schema/journeyAiTranslate/getCardBlocksContent/getBlockContent.spec.ts (1)
24-29: Emptyblocksarray may mask integration issues
const blocks: Block[] = []keeps the tests isolated, butgetBlockContentdelegates to other helpers that can rely on parent/child relations contained in theblocksparameter (e.g.getVideoBlockContentmay need to resolve aVideoBlockSource). Passing an empty array hides those expectations.Provide a minimal but realistic fixture (at least the block under test) or add a separate test that asserts
blocksis forwarded correctly.apis/api-journeys-modern/src/schema/journeyAiTranslate/getCardBlocksContent/getCardBlocksContent.spec.ts (1)
85-101: Duplicate card ids make the assertion brittleBoth
cardBlockandcardNoCoverreuse the idcard1. In the multiple-card test you introducecard2, but the originalcardBlockis present twice (inblocks2and in the imported outer scope). This can mask bugs in deduplication logic insidegetCardBlocksContent.Assign distinct ids to each fixture to mirror production data.
apis/api-journeys-modern/src/schema/journeyAiTranslate/journeyAiTranslate.spec.ts (1)
250-263: Permission check mock ignoressubjectparameterThe production
abilityhelper usually takes(action, subject, user); the call here passesundefinedas subject. Stubbingabilityto always returntruemeans the test never fails, but an incomplete call signature can hide integration bugs. Mock it so the test still verifies the correct subject is provided:expect(mockAbility).toHaveBeenCalledWith(Action.Update, expect.any(Object), mockUser)apis/api-journeys-modern/src/schema/journeyAiTranslate/getCardBlocksContent/getCoverBlockContent.spec.ts (1)
1-119: Well-structured and comprehensive test suite.This test suite thoroughly covers all the expected scenarios for the
getCoverBlockContentfunction, including:
- Image cover block handling
- Video cover block with poster image handling
- Error handling and fallback cases
- Unsupported block types
The tests are well-organized with clear descriptions, proper mocking, and specific assertions. I particularly appreciate the attention to error handling in lines 64-91, covering cases where image descriptions might be missing or fail to load.
The test fixtures are reused efficiently and the mocks are properly cleared between tests, ensuring test independence.
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
libs/journeys/ui/src/components/TemplateView/CreateJourneyButton/CreateJourneyButton.spec.tsx (1)
89-89: Clean up unused mock variable.The
setOpenTeamDialogMockvariable is no longer used since the component now manages dialog state internally. Consider removing it to keep the test file clean.-const setOpenTeamDialogMock = jest.fn()
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (15)
libs/journeys/ui/src/components/TemplateView/CreateJourneyButton/CreateJourneyButton.spec.tsx(5 hunks)libs/journeys/ui/src/components/TemplateView/CreateJourneyButton/CreateJourneyButton.tsx(5 hunks)libs/journeys/ui/src/components/TemplateView/TemplateFooter/TemplateFooter.spec.tsx(2 hunks)libs/journeys/ui/src/components/TemplateView/TemplateFooter/TemplateFooter.stories.tsx(1 hunks)libs/journeys/ui/src/components/TemplateView/TemplateFooter/TemplateFooter.tsx(2 hunks)libs/journeys/ui/src/components/TemplateView/TemplatePreviewTabs/TemplateCardPreview/TemplateCardPreview.spec.tsx(4 hunks)libs/journeys/ui/src/components/TemplateView/TemplatePreviewTabs/TemplateCardPreview/TemplateCardPreview.stories.tsx(1 hunks)libs/journeys/ui/src/components/TemplateView/TemplatePreviewTabs/TemplateCardPreview/TemplateCardPreview.tsx(3 hunks)libs/journeys/ui/src/components/TemplateView/TemplatePreviewTabs/TemplatePreviewTabs.spec.tsx(2 hunks)libs/journeys/ui/src/components/TemplateView/TemplatePreviewTabs/TemplatePreviewTabs.stories.tsx(1 hunks)libs/journeys/ui/src/components/TemplateView/TemplatePreviewTabs/TemplatePreviewTabs.tsx(3 hunks)libs/journeys/ui/src/components/TemplateView/TemplateView.tsx(2 hunks)libs/journeys/ui/src/components/TemplateView/TemplateViewHeader/TemplateViewHeader.spec.tsx(4 hunks)libs/journeys/ui/src/components/TemplateView/TemplateViewHeader/TemplateViewHeader.stories.tsx(0 hunks)libs/journeys/ui/src/components/TemplateView/TemplateViewHeader/TemplateViewHeader.tsx(4 hunks)
💤 Files with no reviewable changes (1)
- libs/journeys/ui/src/components/TemplateView/TemplateViewHeader/TemplateViewHeader.stories.tsx
✅ Files skipped from review due to trivial changes (5)
- libs/journeys/ui/src/components/TemplateView/TemplatePreviewTabs/TemplatePreviewTabs.spec.tsx
- libs/journeys/ui/src/components/TemplateView/TemplatePreviewTabs/TemplatePreviewTabs.stories.tsx
- libs/journeys/ui/src/components/TemplateView/TemplatePreviewTabs/TemplateCardPreview/TemplateCardPreview.stories.tsx
- libs/journeys/ui/src/components/TemplateView/TemplateViewHeader/TemplateViewHeader.spec.tsx
- libs/journeys/ui/src/components/TemplateView/TemplatePreviewTabs/TemplateCardPreview/TemplateCardPreview.spec.tsx
🚧 Files skipped from review as they are similar to previous changes (1)
- libs/journeys/ui/src/components/TemplateView/CreateJourneyButton/CreateJourneyButton.tsx
🧰 Additional context used
🧬 Code Graph Analysis (4)
libs/journeys/ui/src/components/TemplateView/TemplateFooter/TemplateFooter.spec.tsx (1)
libs/journeys/ui/src/components/TemplateView/TemplateFooter/TemplateFooter.tsx (1)
TemplateFooter(12-45)
libs/journeys/ui/src/components/TemplateView/TemplateFooter/TemplateFooter.stories.tsx (1)
libs/journeys/ui/src/components/TemplateView/TemplateFooter/TemplateFooter.tsx (1)
TemplateFooter(12-45)
libs/journeys/ui/src/components/TemplateView/CreateJourneyButton/CreateJourneyButton.spec.tsx (1)
libs/journeys/ui/src/components/TemplateView/CreateJourneyButton/CreateJourneyButton.tsx (1)
CreateJourneyButton(30-172)
libs/journeys/ui/src/components/TemplateView/TemplateViewHeader/TemplateViewHeader.tsx (1)
libs/journeys/ui/src/components/TemplateView/CreateJourneyButton/CreateJourneyButton.tsx (1)
CreateJourneyButton(30-172)
⏰ Context from checks skipped due to timeout of 90000ms (17)
- GitHub Check: Deploy Preview and Test (docs, feature/25-04-MA-feat-ai-translation, push, 22)
- GitHub Check: Deploy Preview and Test (journeys-admin, feature/25-04-MA-feat-ai-translation, push, 22)
- GitHub Check: Deploy Preview and Test (journeys, feature/25-04-MA-feat-ai-translation, push, 22)
- GitHub Check: Deploy Preview and Test (videos-admin, feature/25-04-MA-feat-ai-translation, push, 22)
- GitHub Check: Deploy Preview and Test (videos-admin, 6543/merge, pull_request, 22)
- GitHub Check: Deploy Preview and Test (journeys-admin, 6543/merge, pull_request, 22)
- GitHub Check: Deploy Preview and Test (arclight, 6543/merge, pull_request, 22)
- GitHub Check: test (22, 1/3)
- GitHub Check: lint (22)
- GitHub Check: test (22, 3/3)
- GitHub Check: test (22, 2/3)
- GitHub Check: build (22)
- GitHub Check: Analyze (javascript)
- GitHub Check: test (22, 3/3)
- GitHub Check: test (22, 1/3)
- GitHub Check: test (22, 2/3)
- GitHub Check: build (22)
🔇 Additional comments (15)
libs/journeys/ui/src/components/TemplateView/TemplateFooter/TemplateFooter.spec.tsx (1)
19-19: LGTM! Test simplification aligns with component interface changes.The removal of unused props from the test cases correctly reflects the simplified
TemplateFootercomponent interface. The component now only requires thesignedInprop (when needed) and manages dialog state internally.Also applies to: 36-36
libs/journeys/ui/src/components/TemplateView/TemplateView.tsx (1)
106-106: Excellent refactoring! Removes unnecessary prop drilling.This change eliminates the need to pass dialog state through multiple component layers. Each component now manages its own dialog state internally, which:
- Reduces coupling between components
- Simplifies the component interfaces
- Makes the code more maintainable
- Follows the principle of keeping state close to where it's used
Also applies to: 108-108, 153-153
libs/journeys/ui/src/components/TemplateView/TemplateFooter/TemplateFooter.stories.tsx (1)
26-26: Story correctly updated to match simplified component interface.The removal of props from the story aligns with the
TemplateFootercomponent's refactored interface. The story now accurately represents how the component is used in the application.libs/journeys/ui/src/components/TemplateView/CreateJourneyButton/CreateJourneyButton.spec.tsx (2)
2-2: Excellent test improvements! Testing behavior instead of implementation.The updated tests are much better because they:
- Test actual user-visible behavior (dialog presence) rather than implementation details
- Use
screen.getByTestId('CopyToTeamDialog')to verify the dialog is rendered- Align with the component's new internal state management
This follows testing best practices by focusing on what users experience rather than internal function calls.
Also applies to: 142-144, 171-173
103-103: Component interface correctly simplified.The removal of
openTeamDialogandsetOpenTeamDialogprops from all test instances correctly reflects the component's new interface where dialog state is managed internally viauseState.Also applies to: 138-138, 164-164, 392-392
libs/journeys/ui/src/components/TemplateView/TemplateFooter/TemplateFooter.tsx (2)
8-14: LGTM! Clean interface simplification.The removal of
openTeamDialogandsetOpenTeamDialogprops simplifies the component interface. This aligns with the refactor whereCreateJourneyButtonnow manages dialog state internally, improving separation of concerns.
42-42: Consistent prop passing after refactor.The simplified prop passing to
CreateJourneyButtonis consistent with the component's new interface that only requires thesignedInprop.libs/journeys/ui/src/components/TemplateView/TemplateViewHeader/TemplateViewHeader.tsx (2)
17-25: Interface simplification improves component architecture.Good refactor removing the dialog state props from the component interface. This follows the pattern of centralizing dialog state management in
CreateJourneyButton.
152-152: Correct prop derivation and consistent usage.The
signedInprop is correctly derived fromauthUser?.id != nullfor both desktop and mobile button instances, maintaining the same authentication logic.Also applies to: 161-161
libs/journeys/ui/src/components/TemplateView/TemplatePreviewTabs/TemplatePreviewTabs.tsx (3)
7-7: Clean import optimization.Good cleanup removing unused
DispatchandSetStateActionimports that are no longer needed after removing dialog state props.
23-29: Simplified component interface.The interface and props destructuring have been properly updated to reflect the removal of dialog state management, maintaining only the necessary
authUserprop.
99-99: Consistent prop passing in component hierarchy.The
TemplateCardPreviewcomponent usage correctly reflects the simplified interface, passing only the required props.libs/journeys/ui/src/components/TemplateView/TemplatePreviewTabs/TemplateCardPreview/TemplateCardPreview.tsx (3)
9-9: Proper import cleanup.Good removal of unused React type imports that are no longer needed after the dialog state refactor.
30-33: Interface and props simplified correctly.The component interface and props destructuring have been updated appropriately to reflect the centralized dialog state management approach.
Also applies to: 112-115
201-201: Correct authentication prop derivation.The
signedInprop is properly derived fromauthUser?.id != null, maintaining the authentication logic while using the simplified component interface.
Co-authored-by: autofix-ci[bot] <114827586+autofix-ci[bot]@users.noreply.github.com>
There was a problem hiding this comment.
Actionable comments posted: 0
♻️ Duplicate comments (4)
apps/journeys-admin/src/components/JourneyList/JourneyCard/JourneyCardMenu/TranslateJourneyDialog/TranslateJourneyDialog.spec.tsx (1)
20-207: Test coverage expansion still needed.The test suite covers basic functionality but is missing critical test scenarios as previously identified.
libs/journeys/ui/src/components/CopyToTeamDialog/CopyToTeamDialog.tsx (2)
96-100: PasslanguageSelectonly when translation is enabled.The
submitActionreceivesvalues.languageSelecteven whenshowTranslationisfalse, requiring downstream consumers to defensively check the flag.
85-90: Add error handling to the language query.The
useLanguagesQuerycall lacks error handling, which means users won't receive feedback if the language query fails.apps/journeys-admin/src/components/Team/CopyToTeamMenuItem/CopyToTeamMenuItem.spec.tsx (1)
210-234: Fix type casting and avoid journey object duplication.The journey object type casting and duplication issues identified in previous reviews still need to be addressed.
🧹 Nitpick comments (2)
libs/journeys/ui/src/components/TranslationDialogWrapper/TranslationDialogWrapper.tsx (1)
78-102: Remove unnecessary Fragment wrapper.The Fragment wrapper is redundant here since it only contains conditional JSX elements.
Apply this diff to simplify the JSX structure:
- <> {!loading && ( - <> + <React.Fragment> <Button variant="outlined" color="secondary" onClick={onClose} disabled={loading} sx={{ mr: 3 }} > {t('Cancel')} </Button> <LoadingButton variant="contained" onClick={onTranslate} loading={loading} sx={{ backgroundColor: 'secondary.dark' }} > {submitLabel ?? t('Create')} </LoadingButton> - </> + </React.Fragment> )} - </>🧰 Tools
🪛 Biome (1.9.4)
[error] 78-102: Avoid using unnecessary Fragment.
A fragment is redundant if it contains only one child, or if it is the child of a html element, and is not a keyed fragment.
Unsafe fix: Remove the Fragment(lint/complexity/noUselessFragments)
apps/journeys-admin/src/components/Team/CopyToTeamMenuItem/CopyToTeamMenuItem.spec.tsx (1)
26-26: Remove useless rename of import.The import renames
JourneyFieldsto itself, which is unnecessary.Apply this diff to clean up the import:
-import { JourneyFields as JourneyFields } from '../../../../__generated__/JourneyFields' +import { JourneyFields } from '../../../../__generated__/JourneyFields'🧰 Tools
🪛 Biome (1.9.4)
[error] 26-26: Useless rename.
Safe fix: Remove the renaming.
(lint/complexity/noUselessRename)
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (12)
apps/journeys-admin/src/components/JourneyList/JourneyCard/JourneyCardMenu/DefaultMenu/DefaultMenu.tsx(3 hunks)apps/journeys-admin/src/components/JourneyList/JourneyCard/JourneyCardMenu/JourneyCardMenu.tsx(7 hunks)apps/journeys-admin/src/components/JourneyList/JourneyCard/JourneyCardMenu/TranslateJourneyDialog/TranslateJourneyDialog.spec.tsx(1 hunks)apps/journeys-admin/src/components/JourneyList/JourneyCard/JourneyCardMenu/TranslateJourneyDialog/TranslateJourneyDialog.tsx(1 hunks)apps/journeys-admin/src/components/Team/CopyToTeamMenuItem/CopyToTeamMenuItem.spec.tsx(3 hunks)apps/journeys-admin/src/components/Team/CopyToTeamMenuItem/CopyToTeamMenuItem.tsx(2 hunks)libs/journeys/ui/src/components/CopyToTeamDialog/CopyToTeamDialog.tsx(1 hunks)libs/journeys/ui/src/components/TranslationDialogWrapper/TranslationDialogWrapper.spec.tsx(1 hunks)libs/journeys/ui/src/components/TranslationDialogWrapper/TranslationDialogWrapper.stories.tsx(1 hunks)libs/journeys/ui/src/components/TranslationDialogWrapper/TranslationDialogWrapper.tsx(1 hunks)libs/journeys/ui/src/libs/useJourneyAiTranslateMutation/index.ts(1 hunks)libs/shared/ui/src/components/LanguageAutocomplete/LanguageAutocomplete.tsx(5 hunks)
✅ Files skipped from review due to trivial changes (1)
- libs/journeys/ui/src/components/TranslationDialogWrapper/TranslationDialogWrapper.stories.tsx
🚧 Files skipped from review as they are similar to previous changes (7)
- libs/journeys/ui/src/libs/useJourneyAiTranslateMutation/index.ts
- libs/shared/ui/src/components/LanguageAutocomplete/LanguageAutocomplete.tsx
- apps/journeys-admin/src/components/JourneyList/JourneyCard/JourneyCardMenu/DefaultMenu/DefaultMenu.tsx
- apps/journeys-admin/src/components/Team/CopyToTeamMenuItem/CopyToTeamMenuItem.tsx
- apps/journeys-admin/src/components/JourneyList/JourneyCard/JourneyCardMenu/JourneyCardMenu.tsx
- libs/journeys/ui/src/components/TranslationDialogWrapper/TranslationDialogWrapper.spec.tsx
- apps/journeys-admin/src/components/JourneyList/JourneyCard/JourneyCardMenu/TranslateJourneyDialog/TranslateJourneyDialog.tsx
🧰 Additional context used
🪛 Biome (1.9.4)
apps/journeys-admin/src/components/Team/CopyToTeamMenuItem/CopyToTeamMenuItem.spec.tsx
[error] 26-26: Useless rename.
Safe fix: Remove the renaming.
(lint/complexity/noUselessRename)
libs/journeys/ui/src/components/CopyToTeamDialog/CopyToTeamDialog.tsx
[error] 130-130: Do not add then to an object.
(lint/suspicious/noThenProperty)
libs/journeys/ui/src/components/TranslationDialogWrapper/TranslationDialogWrapper.tsx
[error] 78-102: Avoid using unnecessary Fragment.
A fragment is redundant if it contains only one child, or if it is the child of a html element, and is not a keyed fragment.
Unsafe fix: Remove the Fragment
(lint/complexity/noUselessFragments)
⏰ Context from checks skipped due to timeout of 90000ms (2)
- GitHub Check: Deploy Preview and Test (journeys-admin, feature/25-04-MA-feat-ai-translation, push, 22)
- GitHub Check: Deploy Preview and Test (watch, feature/25-04-MA-feat-ai-translation, push, 22)
🔇 Additional comments (3)
libs/journeys/ui/src/components/TranslationDialogWrapper/TranslationDialogWrapper.tsx (1)
11-118: Well-structured reusable dialog component.The component provides excellent encapsulation of translation dialog concerns with:
- Comprehensive TypeScript interfaces
- Detailed JSDoc documentation
- Proper loading state handling
- Flexible customization options
🧰 Tools
🪛 Biome (1.9.4)
[error] 78-102: Avoid using unnecessary Fragment.
A fragment is redundant if it contains only one child, or if it is the child of a html element, and is not a keyed fragment.
Unsafe fix: Remove the Fragment(lint/complexity/noUselessFragments)
libs/journeys/ui/src/components/CopyToTeamDialog/CopyToTeamDialog.tsx (1)
123-136: Yup conditional validation schema is correctly implemented.The validation schema properly uses Yup's conditional validation to require language selection only when translation is enabled. The static analysis hint about the "then" property is a false positive - this is valid Yup syntax.
🧰 Tools
🪛 Biome (1.9.4)
[error] 130-130: Do not add then to an object.
(lint/suspicious/noThenProperty)
apps/journeys-admin/src/components/Team/CopyToTeamMenuItem/CopyToTeamMenuItem.spec.tsx (1)
194-371: Excellent test coverage for translation workflow.The tests comprehensively cover both the basic duplication and the new AI translation flows, including proper GraphQL mock setup, user interactions, and assertion of expected mutations.
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (1)
apps/journeys-admin/src/components/JourneyList/JourneyCard/JourneyCardMenu/DefaultMenu/DefaultMenu.spec.tsx (1)
21-21: Consider removing ThemeProvider import if no longer needed.The
ThemeProviderimport is present but doesn't appear to be used in any of the tests after the changes. This might be leftover from previous test setup that was simplified.If
ThemeProvideris no longer used, remove the import:- import { ThemeProvider } from '../../../../ThemeProvider'Also applies to: 192-938
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
apps/journeys-admin/src/components/JourneyList/JourneyCard/JourneyCardMenu/DefaultMenu/DefaultMenu.spec.tsx(16 hunks)apps/journeys-admin/src/components/JourneyList/JourneyCard/JourneyCardMenu/DefaultMenu/DefaultMenu.tsx(3 hunks)apps/journeys-admin/src/components/JourneyList/JourneyCard/JourneyCardMenu/JourneyCardMenu.spec.tsx(2 hunks)libs/locales/en/apps-journeys-admin.json(2 hunks)
✅ Files skipped from review due to trivial changes (1)
- libs/locales/en/apps-journeys-admin.json
🚧 Files skipped from review as they are similar to previous changes (2)
- apps/journeys-admin/src/components/JourneyList/JourneyCard/JourneyCardMenu/JourneyCardMenu.spec.tsx
- apps/journeys-admin/src/components/JourneyList/JourneyCard/JourneyCardMenu/DefaultMenu/DefaultMenu.tsx
🧰 Additional context used
🧬 Code Graph Analysis (1)
apps/journeys-admin/src/components/JourneyList/JourneyCard/JourneyCardMenu/DefaultMenu/DefaultMenu.spec.tsx (1)
apps/journeys-admin/src/components/JourneyList/JourneyCard/JourneyCardMenu/DefaultMenu/DefaultMenu.tsx (1)
DefaultMenu(95-259)
🪛 GitHub Check: test (22, 3/3)
apps/journeys-admin/src/components/JourneyList/JourneyCard/JourneyCardMenu/DefaultMenu/DefaultMenu.spec.tsx
[failure] 212-212: DefaultMenu › should render menu for journey
TestingLibraryElementError: Unable to find an accessible element with the role "menuitem" and name "Duplicate"
Here are the accessible roles:
menuitem:
Name "Edit Details":
Name "Access":
Name "Share":
Name "Copy to ...":
separator:
Name "":
Name "":
Name "":
Ignored nodes: comments, script, style
[warning] 212-212: RETRY 3: DefaultMenu › should render menu for journey
TestingLibraryElementError: Unable to find an accessible element with the role "menuitem" and name "Duplicate"
Here are the accessible roles:
menuitem:
Name "Edit Details":
Name "Access":
Name "Share":
Name "Copy to ...":
separator:
Name "":
Name "":
Name "":
Ignored nodes: comments, script, style
[warning] 212-212: RETRY 2: DefaultMenu › should render menu for journey
TestingLibraryElementError: Unable to find an accessible element with the role "menuitem" and name "Duplicate"
Here are the accessible roles:
menuitem:
Name "Edit Details":
Name "Access":
Name "Share":
Name "Copy to ...":
separator:
Name "":
Name "":
Name "":
Ignored nodes: comments, script, style
[warning] 212-212: RETRY 1: DefaultMenu › should render menu for journey
TestingLibraryElementError: Unable to find an accessible element with the role "menuitem" and name "Duplicate"
Here are the accessible roles:
menuitem:
Name "Edit Details":
Name "Access":
Name "Share":
Name "Copy to ...":
separator:
Name "":
Name "":
Name "":
Ignored nodes: comments, script, style
[failure] 935-935: DefaultMenu › should call correct functions on Translate click
TestingLibraryElementError: Unable to find an accessible element with the role "menuitem" and name "Translate"
Here are the accessible roles:
menuitem:
Name "Edit Details":
Name "Access":
Name "Share":
Name "Copy to ...":
separator:
Name "":
Name "":
Name "":
Ignored nodes: comments, script, style
[warning] 935-935: RETRY 3: DefaultMenu › should call correct functions on Translate click
TestingLibraryElementError: Unable to find an accessible element with the role "menuitem" and name "Translate"
Here are the accessible roles:
menuitem:
Name "Edit Details":
Name "Access":
Name "Share":
Name "Copy to ...":
separator:
Name "":
Name "":
Name "":
Ignored nodes: comments, script, style
[warning] 935-935: RETRY 2: DefaultMenu › should call correct functions on Translate click
TestingLibraryElementError: Unable to find an accessible element with the role "menuitem" and name "Translate"
Here are the accessible roles:
menuitem:
Name "Edit Details":
Name "Access":
Name "Share":
Name "Copy to ...":
separator:
Name "":
Name "":
Name "":
Ignored nodes: comments, script, style
[warning] 935-935: RETRY 1: DefaultMenu › should call correct functions on Translate click
TestingLibraryElementError: Unable to find an accessible element with the role "menuitem" and name "Translate"
Here are the accessible roles:
menuitem:
Name "Edit Details":
Name "Access":
Name "Share":
Name "Copy to ...":
separator:
Name "":
Name "":
Name "":
Ignored nodes: comments, script, style
⏰ Context from checks skipped due to timeout of 90000ms (23)
- GitHub Check: Deploy Preview and Test (short-links, feature/25-04-MA-feat-ai-translation, push, 22)
- GitHub Check: Deploy Preview and Test (journeys, feature/25-04-MA-feat-ai-translation, push, 22)
- GitHub Check: Deploy Preview and Test (watch, feature/25-04-MA-feat-ai-translation, push, 22)
- GitHub Check: Deploy Preview and Test (videos-admin, feature/25-04-MA-feat-ai-translation, push, 22)
- GitHub Check: Deploy Preview and Test (docs, feature/25-04-MA-feat-ai-translation, push, 22)
- GitHub Check: Deploy Preview and Test (arclight, feature/25-04-MA-feat-ai-translation, push, 22)
- GitHub Check: Deploy Preview and Test (journeys-admin, feature/25-04-MA-feat-ai-translation, push, 22)
- GitHub Check: Deploy Preview and Test (videos-admin, 6543/merge, pull_request, 22)
- GitHub Check: Deploy Preview and Test (short-links, 6543/merge, pull_request, 22)
- GitHub Check: Deploy Preview and Test (docs, 6543/merge, pull_request, 22)
- GitHub Check: Deploy Preview and Test (watch, 6543/merge, pull_request, 22)
- GitHub Check: Deploy Preview and Test (journeys, 6543/merge, pull_request, 22)
- GitHub Check: Deploy Preview and Test (journeys-admin, 6543/merge, pull_request, 22)
- GitHub Check: Deploy Preview and Test (arclight, 6543/merge, pull_request, 22)
- GitHub Check: test (22, 3/3)
- GitHub Check: test (22, 2/3)
- GitHub Check: build (22)
- GitHub Check: test (22, 1/3)
- GitHub Check: lint (22)
- GitHub Check: Analyze (javascript)
- GitHub Check: test (22, 2/3)
- GitHub Check: test (22, 1/3)
- GitHub Check: build (22)
🔇 Additional comments (2)
apps/journeys-admin/src/components/JourneyList/JourneyCard/JourneyCardMenu/DefaultMenu/DefaultMenu.spec.tsx (2)
203-203: Consistent prop addition across all test instances.The
setOpenTranslateDialog={noop}prop has been systematically added to allDefaultMenucomponent instances throughout the test file. This ensures consistency and prevents prop validation errors.Also applies to: 237-237, 279-279, 307-307, 346-346, 385-385, 447-447, 510-510, 573-573, 638-638, 700-700, 759-759, 820-820, 890-890, 928-928
252-257: Correct test expectations for template journeys.The test correctly verifies that "Duplicate" and "Translate" menu items are not present for template journeys, which aligns with the business logic that these features should only be available for regular journeys.
…n up journeyEventsExportLog tests
4 participants
Summary by CodeRabbit
New Features
Improvements
Bug Fixes
Tests
Documentation