Conversation
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 (2)
🚧 Files skipped from review as they are similar to previous changes (2)
WalkthroughFetch of template language IDs is dynamic: a new GET_JOURNEY_TEMPLATE_LANGUAGE_IDS query is introduced and used by the admin page (getStaticProps) and the HeaderAndLanguageFilter component to drive a subsequent GET_LANGUAGES call instead of a hardcoded ID list. Tests, stories, mocks, and docs were updated accordingly. Changes
Sequence Diagram(s)sequenceDiagram
participant Admin as Admin Page (getStaticProps)
participant Apollo as Apollo Client
participant API as api-journeys GraphQL
participant DB as Database
Admin->>Apollo: query GET_JOURNEY_TEMPLATE_LANGUAGE_IDS
Apollo->>API: GET_JOURNEY_TEMPLATE_LANGUAGE_IDS
API->>DB: findMany(distinct: ['languageId'], filter: templates & published)
DB-->>API: languageId list
API-->>Apollo: { journeyTemplateLanguageIds }
Apollo-->>Admin: { journeyTemplateLanguageIds }
Admin->>Apollo: query GET_LANGUAGES(where.ids: templateLanguageIds)
Apollo->>API: GET_LANGUAGES
API->>DB: fetch languages by ids
DB-->>API: languages
API-->>Apollo: { languages }
Apollo-->>Admin: { languages }
sequenceDiagram
participant Client as HeaderAndLanguageFilter (Client)
participant Apollo as Apollo Client
participant API as api-journeys GraphQL
participant DB as Database
Client->>Apollo: query GET_JOURNEY_TEMPLATE_LANGUAGE_IDS
Apollo->>API: GET_JOURNEY_TEMPLATE_LANGUAGE_IDS
API->>DB: findMany(...)
DB-->>API: languageId list
API-->>Apollo: { journeyTemplateLanguageIds }
Apollo-->>Client: { journeyTemplateLanguageIds }
alt templateLanguageIds present
Client->>Apollo: query GET_LANGUAGES(where.ids: templateLanguageIds)
Apollo->>API: GET_LANGUAGES
API->>DB: fetch languages
DB-->>API: languages
API-->>Apollo: { languages }
Apollo-->>Client: { languages }
else loading/error
Client-->>Client: show loading / handle error
end
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes 🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 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 |
|
| Command | Status | Duration | Result |
|---|---|---|---|
nx run journeys-admin-e2e:e2e |
❌ Failed | 6m 17s | View ↗ |
nx run journeys-e2e:e2e |
❌ Failed | 3m 19s | View ↗ |
nx run resources-e2e:e2e |
✅ Succeeded | 8s | View ↗ |
nx run videos-admin-e2e:e2e |
✅ Succeeded | 4s | View ↗ |
nx run watch-e2e:e2e |
✅ Succeeded | 17s | View ↗ |
nx run-many --target=vercel-alias --projects=jo... |
✅ Succeeded | 2s | View ↗ |
nx run-many --target=upload-sourcemaps --projec... |
✅ Succeeded | 9s | View ↗ |
nx run-many --target=deploy --projects=journeys... |
✅ Succeeded | 2m 28s | View ↗ |
Additional runs (12) |
✅ Succeeded | ... | View ↗ |
☁️ Nx Cloud last updated this comment at 2026-04-10 04:22:11 UTC
Contributor
|
The latest updates on your projects.
|
Contributor
|
The latest updates on your projects.
|
Contributor
|
The latest updates on your projects.
|
Contributor
|
The latest updates on your projects.
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
🧹 Nitpick comments (3)
apps/journeys-admin/pages/templates/index.tsx (1)
112-129: LGTM! Sequential-then-parallel fetch pattern is correct.The implementation correctly fetches language IDs first (as the result is needed for the
GET_LANGUAGESfilter), then runs the remaining queries in parallel. The null-coalescing fallback to[]handles edge cases gracefully.Consider using a generated type instead of the inline type annotation for better type safety and maintainability:
♻️ Optional: Use generated type
If a type is generated for
GET_JOURNEY_TEMPLATE_LANGUAGE_IDS, you could import and use it:+import { GetJourneyTemplateLanguageIds } from '../../__generated__/GetJourneyTemplateLanguageIds' - const { data: languageIdsData } = await apolloClient.query<{ - journeyTemplateLanguageIds: string[] - }>({ + const { data: languageIdsData } = await apolloClient.query<GetJourneyTemplateLanguageIds>({ query: GET_JOURNEY_TEMPLATE_LANGUAGE_IDS })🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/journeys-admin/pages/templates/index.tsx` around lines 112 - 129, Replace the inline response type used in the apolloClient.query for GET_JOURNEY_TEMPLATE_LANGUAGE_IDS with the generated GraphQL type for that query (e.g., the generated GetJourneyTemplateLanguageIds or similar); locate the query call that assigns languageIdsData and the constant templateLanguageIds, import the corresponding generated type from your GraphQL types file, and use it as the generic type parameter to apolloClient.query to improve type safety and maintainability.apis/api-journeys/src/app/modules/journey/journey.resolver.ts (1)
212-224: LGTM! Clean resolver implementation.The query correctly uses
distinctfor deduplication andselectfor minimal data transfer. SincelanguageIdis non-nullable in the schema, the mapping is type-safe.One optional consideration: the magic string
'jfp-team'is duplicated here and in the seed file. A shared constant could improve maintainability, but this matches the existing codebase pattern.🔧 Optional: Extract magic string to a shared constant
// In a shared constants file (e.g., src/app/lib/constants.ts) export const JFP_TEAM_ID = 'jfp-team'Then use it in both the resolver and seed file.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apis/api-journeys/src/app/modules/journey/journey.resolver.ts` around lines 212 - 224, Replace the duplicated magic string 'jfp-team' in the journeyTemplateLanguageIds resolver with a shared constant: create or reuse a exported constant (e.g., JFP_TEAM_ID) and import it into journey.resolver.ts, then use JFP_TEAM_ID in the where.teamId clause of the journeyTemplateLanguageIds method so both resolver and seed file reference the same value.libs/journeys/ui/src/components/TemplateGallery/HeaderAndLanguageFilter/HeaderAndLanguageFilter.tsx (1)
183-202: Consider adding error handling for query failures.The two-step query flow is well-implemented with the
skippattern preventing transient unfiltered requests. However, neither query has error handling. IfGET_JOURNEY_TEMPLATE_LANGUAGE_IDSfails, users will see an indefinite loading state with no feedback.♻️ Proposed enhancement for error visibility
- const { data: languageIdsData, loading: languageIdsLoading } = useQuery<{ + const { data: languageIdsData, loading: languageIdsLoading, error: languageIdsError } = useQuery<{ journeyTemplateLanguageIds: string[] }>(GET_JOURNEY_TEMPLATE_LANGUAGE_IDS) const templateLanguageIds = languageIdsData?.journeyTemplateLanguageIds - const { data, loading: languagesLoading } = useQuery< + const { data, loading: languagesLoading, error: languagesError } = useQuery< GetLanguages, GetLanguagesVariables >(GET_LANGUAGES, { variables: { languageId: '529', where: { ids: templateLanguageIds } }, skip: templateLanguageIds == null }) const loading = languageIdsLoading || languagesLoading + const error = languageIdsError ?? languagesError + + // Log errors for monitoring (per PR objectives: "log query errors for journeyTemplateLanguageIds") + useEffect(() => { + if (error != null) { + console.error('Language filter query error:', error) + } + }, [error])🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@libs/journeys/ui/src/components/TemplateGallery/HeaderAndLanguageFilter/HeaderAndLanguageFilter.tsx` around lines 183 - 202, The component currently omits error handling for the two GraphQL queries (GET_JOURNEY_TEMPLATE_LANGUAGE_IDS and GET_LANGUAGES) causing indefinite loading on failure; update the useQuery destructurings to capture errors (e.g., languageIdsError and languagesError), incorporate those into the combined loading/ready logic (so loading becomes false when an error exists), and render or surface an error state (a user-facing message or fallback UI) when either languageIdsError or languagesError is set; also consider logging the errors for diagnostics and keep the existing skip logic around templateLanguageIds.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@apis/api-journeys/src/app/modules/journey/journey.resolver.ts`:
- Around line 212-224: Replace the duplicated magic string 'jfp-team' in the
journeyTemplateLanguageIds resolver with a shared constant: create or reuse a
exported constant (e.g., JFP_TEAM_ID) and import it into journey.resolver.ts,
then use JFP_TEAM_ID in the where.teamId clause of the
journeyTemplateLanguageIds method so both resolver and seed file reference the
same value.
In `@apps/journeys-admin/pages/templates/index.tsx`:
- Around line 112-129: Replace the inline response type used in the
apolloClient.query for GET_JOURNEY_TEMPLATE_LANGUAGE_IDS with the generated
GraphQL type for that query (e.g., the generated GetJourneyTemplateLanguageIds
or similar); locate the query call that assigns languageIdsData and the constant
templateLanguageIds, import the corresponding generated type from your GraphQL
types file, and use it as the generic type parameter to apolloClient.query to
improve type safety and maintainability.
In
`@libs/journeys/ui/src/components/TemplateGallery/HeaderAndLanguageFilter/HeaderAndLanguageFilter.tsx`:
- Around line 183-202: The component currently omits error handling for the two
GraphQL queries (GET_JOURNEY_TEMPLATE_LANGUAGE_IDS and GET_LANGUAGES) causing
indefinite loading on failure; update the useQuery destructurings to capture
errors (e.g., languageIdsError and languagesError), incorporate those into the
combined loading/ready logic (so loading becomes false when an error exists),
and render or surface an error state (a user-facing message or fallback UI) when
either languageIdsError or languagesError is set; also consider logging the
errors for diagnostics and keep the existing skip logic around
templateLanguageIds.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
Run ID: c02d5d1e-3303-4d94-8ee9-8daba62ba943
📒 Files selected for processing (11)
apis/api-journeys/src/app/modules/journey/journey.graphqlapis/api-journeys/src/app/modules/journey/journey.resolver.spec.tsapis/api-journeys/src/app/modules/journey/journey.resolver.tsapps/journeys-admin/pages/templates/index.tsxlibs/journeys/ui/src/components/TemplateGallery/HeaderAndLanguageFilter/HeaderAndLanguageFilter.spec.tsxlibs/journeys/ui/src/components/TemplateGallery/HeaderAndLanguageFilter/HeaderAndLanguageFilter.stories.tsxlibs/journeys/ui/src/components/TemplateGallery/HeaderAndLanguageFilter/HeaderAndLanguageFilter.tsxlibs/journeys/ui/src/components/TemplateGallery/HeaderAndLanguageFilter/index.tslibs/journeys/ui/src/components/TemplateGallery/TemplateGallery.spec.tsxlibs/journeys/ui/src/components/TemplateGallery/TemplateGallery.stories.tsxlibs/journeys/ui/src/components/TemplateGallery/data.ts
2 tasks
jianwei1
force-pushed
the
26-03-JC-feat-templates-language-dropdown
branch
from
18618a5 to
e4272a0
Compare
Surface warnings/errors when the language IDs query returns empty, null, or errors — aids debugging during ISR revalidation and client-side rendering without changing the user-facing behavior. Made-with: Cursor
Contributor
Author
Review feedback addressed (f0d6554)Fixed:
Challenged:
|
csiyang
approved these changes
Apr 8, 2026
Contributor
|
The latest updates on your projects.
|
Contributor
tanflem
pushed a commit
that referenced
this pull request
Apr 13, 2026
…te languages (#8973) Co-authored-by: Claude Opus 4.6 (1M context) <noreply@anthropic.com> Co-authored-by: autofix-ci[bot] <114827586+autofix-ci[bot]@users.noreply.github.com>
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
HeaderAndLanguageFilterwith dynamic two-step fetch using the newjourneyTemplateLanguageIdsqueryuseQuerywithskipto prevent transient unfiltered fetch of all ~5000 languages while language IDs are loadinggetStaticPropsin templates page to fetch language IDs sequentially, then languages/tags/journeys in parallelNES-1537
Test plan
HeaderAndLanguageFiltercomponent tests pass (2 tests with updated mocks)TemplateGalleryintegration tests pass (4 tests with updated mocks)Post-Deploy Monitoring & Validation
🤖 Generated with Claude Code
Summary by CodeRabbit
New Features
Documentation