Open
Conversation
…tps://github.com/JesusFilm/core into 25-00-NC-feat-ai-translate-quick-start-templates
- Added .cursorignore to exclude environment files. - Updated .gitignore to include new personal Claude rules and Strapi CMS config files. - Modified .prettierignore to include Kubernetes manifests. - Updated package.json with new dependencies and version upgrades. - Added new rules and guidelines for backend, frontend, and infrastructure in .claude directory. - Updated workflows to improve dependency installation and notifications. - Adjusted TypeScript configuration for better module resolution.
…put structure - Replaced instances of generateObject with generateText in translation logic. - Adjusted mock implementations and test cases to reflect the new function usage. - Updated return values to align with the new Output structure in the translation schema. - Enhanced error handling in LanguageScreen for guest and signed-in users during journey duplication.
mikeallisonJS
requested review from
kiran-redhat,
sharon-elsa-mathew1995 and
tataihono
as code owners
Contributor
WalkthroughThis PR extends the journey AI translation pipeline to include customization field and description translation. It introduces a new translation module for customizing field values and descriptions, adds a GraphQL mutation for translating customization descriptions, updates the AI translate input schema to accept optional user language preferences, and integrates translation flows into multiple UI components for language selection and journey duplication. Changes
Sequence DiagramsequenceDiagram
participant User
participant UI as LanguageScreen UI
participant Apollo as Apollo Cache
participant API as API GraphQL
participant AIModel as AI Service (Gemini)
participant DB as Database
User->>UI: Select translation language
UI->>API: Start JourneyAiTranslateCreateSubscription<br/>(with userLanguageId, userLanguageName)
API->>DB: Fetch journey with customization fields
API->>AIModel: translateCardBlocks(blocks, language)
AIModel-->>API: Translated block updates
API->>AIModel: translateCustomizationFields(fields, language)
AIModel-->>API: Translated field values & description
API->>DB: Update blocks with translations
API->>DB: Update journeyCustomizationFields
API->>DB: Update journeyCustomizationDescription
API-->>UI: Progress: 100% (subscription update)
UI->>Apollo: updateCacheWithTranslatedJourney()
Apollo->>Apollo: Write journeyCustomizationDescription<br/>journeyCustomizationFields to cache
UI->>User: Show translation complete, navigate
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
🚥 Pre-merge checks | ✅ 1 | ❌ 2❌ Failed checks (1 warning, 1 inconclusive)
✅ Passed checks (1 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 |
💰 Infracost reportMonthly estimate increased by $155 📈
*Usage costs can be estimated by updating Infracost Cloud settings, see docs for other options. Estimate details (includes details of unsupported resources) |
|
Ran Plan for dir: Show OutputTerraform used the selected providers to generate the following execution
plan. Resource actions are indicated with the following symbols:
~ update in-place
- destroy
+/- create replacement and then destroy
Terraform will perform the following actions:
# module.prod.module.api-analytics.module.ecs-task.aws_ecs_service.ecs_service will be updated in-place
~ resource "aws_ecs_service" "ecs_service" {
id = "arn:aws:ecs:us-east-2:410965620680:service/jfp-ecs-cluster-prod/api-analytics-prod-service"
name = "api-analytics-prod-service"
tags = {}
~ task_definition = "arn:aws:ecs:us-east-2:410965620680:task-definition/jfp-api-analytics-prod:9" -> (known after apply)
# (18 unchanged attributes hidden)
# (4 unchanged blocks hidden)
}
# module.prod.module.api-analytics.module.ecs-task.aws_ecs_task_definition.ecs_task_definition must be replaced
+/- resource "aws_ecs_task_definition" "ecs_task_definition" {
~ arn = "arn:aws:ecs:us-east-2:410965620680:task-definition/jfp-api-analytics-prod:9" -> (known after apply)
~ arn_without_revision = "arn:aws:ecs:us-east-2:410965620680:task-definition/jfp-api-analytics-prod" -> (known after apply)
~ container_definitions = jsonencode(
~ [
~ {
name = "jfp-api-analytics-prod-app"
~ secrets = [
+ {
+ name = "DD_API_KEY"
+ valueFrom = "arn:aws:ssm:us-east-2:410965620680:parameter/terraform/prd/DATADOG_API_KEY"
},
{
name = "GATEWAY_HMAC_SECRET"
valueFrom = "arn:aws:ssm:us-east-2:410965620680:parameter/ecs/api-analytics/prod/GATEWAY_HMAC_SECRET"
},
# (1 unchanged element hidden)
{
name = "PLAUSIBLE_SECRET_KEY_BASE"
valueFrom = "arn:aws:ssm:us-east-2:410965620680:parameter/ecs/api-analytics/prod/PLAUSIBLE_SECRET_KEY_BASE"
},
- {
- name = "PRISMA_LOCATION_ANALYTICS"
- valueFrom = "arn:aws:ssm:us-east-2:410965620680:parameter/ecs/api-analytics/prod/PRISMA_LOCATION_ANALYTICS"
},
- {
- name = "DD_API_KEY"
- valueFrom = "arn:aws:ssm:us-east-2:410965620680:parameter/terraform/prd/DATADOG_API_KEY"
},
]
- systemControls = []
# (9 unchanged attributes hidden)
},
~ {
- cpu = 0
name = "jfp-api-analytics-prod-datadog-agent"
- systemControls = []
# (9 unchanged attributes hidden)
},
~ {
- cpu = 0
name = "jfp-api-analytics-prod-log-router"
- systemControls = []
# (10 unchanged attributes hidden)
},
] # forces replacement
)
~ enable_fault_injection = false -> (known after apply)
~ id = "jfp-api-analytics-prod" -> (known after apply)
~ revision = 9 -> (known after apply)
- tags = {} -> null
~ tags_all = {} -> (known after apply)
# (12 unchanged attributes hidden)
}
# module.prod.module.api-analytics.module.ecs-task.aws_ssm_parameter.parameters["PRISMA_LOCATION_ANALYTICS"] will be destroyed
# (because key ["PRISMA_LOCATION_ANALYTICS"] is not in for_each map)
- resource "aws_ssm_parameter" "parameters" {
- arn = "arn:aws:ssm:us-east-2:410965620680:parameter/ecs/api-analytics/prod/PRISMA_LOCATION_ANALYTICS" -> null
- data_type = "text" -> null
- id = "/ecs/api-analytics/prod/PRISMA_LOCATION_ANALYTICS" -> null
- key_id = "alias/aws/ssm" -> null
- name = "/ecs/api-analytics/prod/PRISMA_LOCATION_ANALYTICS" -> null
- overwrite = true -> null
- region = "us-east-2" -> null
- tags = {
- "name" = "PRISMA_LOCATION_ANALYTICS"
} -> null
- tags_all = {
- "name" = "PRISMA_LOCATION_ANALYTICS"
} -> null
- tier = "Standard" -> null
- type = "SecureString" -> null
- value = (sensitive value) -> null
- value_wo = (write-only attribute) -> null
- version = 3 -> null
# (2 unchanged attributes hidden)
}
# module.prod.module.api-media.module.ecs-task.aws_appautoscaling_target.service_autoscaling will be updated in-place
~ resource "aws_appautoscaling_target" "service_autoscaling" {
id = "service/jfp-ecs-cluster-prod/api-media-prod-service"
~ min_capacity = 2 -> 1
tags = {}
# (8 unchanged attributes hidden)
# (1 unchanged block hidden)
}
# module.prod.module.api-media.module.ecs-task.aws_ecs_service.ecs_service will be updated in-place
~ resource "aws_ecs_service" "ecs_service" {
~ desired_count = 2 -> 1
id = "arn:aws:ecs:us-east-2:410965620680:service/jfp-ecs-cluster-prod/api-media-prod-service"
name = "api-media-prod-service"
tags = {}
# (18 unchanged attributes hidden)
# (5 unchanged blocks hidden)
}
# module.prod.module.arclight.module.ecs-task.aws_ecs_service.ecs_service will be updated in-place
~ resource "aws_ecs_service" "ecs_service" {
~ desired_count = 1 -> 2
id = "arn:aws:ecs:us-east-2:410965620680:service/jfp-ecs-cluster-prod/arclight-prod-service"
name = "arclight-prod-service"
tags = {}
# (18 unchanged attributes hidden)
# (4 unchanged blocks hidden)
}
# module.stage.module.api-users.module.ecs-task.aws_ecs_service.ecs_service will be updated in-place
~ resource "aws_ecs_service" "ecs_service" {
id = "arn:aws:ecs:us-east-2:410965620680:service/jfp-ecs-cluster-stage/api-users-stage-service"
name = "api-users-stage-service"
tags = {}
~ task_definition = "arn:aws:ecs:us-east-2:410965620680:task-definition/jfp-api-users-stage:44" -> (known after apply)
# (18 unchanged attributes hidden)
# (4 unchanged blocks hidden)
}
# module.stage.module.api-users.module.ecs-task.aws_ecs_task_definition.ecs_task_definition must be replaced
+/- resource "aws_ecs_task_definition" "ecs_task_definition" {
~ arn = "arn:aws:ecs:us-east-2:410965620680:task-definition/jfp-api-users-stage:44" -> (known after apply)
~ arn_without_revision = "arn:aws:ecs:us-east-2:410965620680:task-definition/jfp-api-users-stage" -> (known after apply)
~ container_definitions = jsonencode(
~ [
~ {
name = "jfp-api-users-stage-app"
~ secrets = [
# (4 unchanged elements hidden)
{
name = "GATEWAY_HMAC_SECRET"
valueFrom = "arn:aws:ssm:us-east-2:410965620680:parameter/ecs/api-users/stage/GATEWAY_HMAC_SECRET"
},
- {
- name = "GATEWAY_URL"
- valueFrom = "arn:aws:ssm:us-east-2:410965620680:parameter/ecs/api-users/stage/GATEWAY_URL"
},
{
name = "GOOGLE_APPLICATION_JSON"
valueFrom = "arn:aws:ssm:us-east-2:410965620680:parameter/ecs/api-users/stage/GOOGLE_APPLICATION_JSON"
},
# (8 unchanged elements hidden)
]
- systemControls = []
# (9 unchanged attributes hidden)
},
~ {
name = "jfp-api-users-stage-datadog-agent"
- systemControls = []
# (9 unchanged attributes hidden)
},
~ {
name = "jfp-api-users-stage-log-router"
- systemControls = []
# (10 unchanged attributes hidden)
},
] # forces replacement
)
~ enable_fault_injection = false -> (known after apply)
~ id = "jfp-api-users-stage" -> (known after apply)
~ revision = 44 -> (known after apply)
- tags = {} -> null
~ tags_all = {} -> (known after apply)
# (12 unchanged attributes hidden)
}
# module.stage.module.api-users.module.ecs-task.aws_ssm_parameter.parameters["GATEWAY_URL"] will be destroyed
# (because key ["GATEWAY_URL"] is not in for_each map)
- resource "aws_ssm_parameter" "parameters" {
- arn = "arn:aws:ssm:us-east-2:410965620680:parameter/ecs/api-users/stage/GATEWAY_URL" -> null
- data_type = "text" -> null
- has_value_wo = false -> null
- id = "/ecs/api-users/stage/GATEWAY_URL" -> null
- key_id = "alias/aws/ssm" -> null
- name = "/ecs/api-users/stage/GATEWAY_URL" -> null
- overwrite = true -> null
- region = "us-east-2" -> null
- tags = {
- "name" = "GATEWAY_URL"
} -> null
- tags_all = {
- "name" = "GATEWAY_URL"
} -> null
- tier = "Standard" -> null
- type = "SecureString" -> null
- value = (sensitive value) -> null
- value_wo = (write-only attribute) -> null
- version = 3 -> null
# (2 unchanged attributes hidden)
}
Plan: 2 to add, 5 to change, 4 to destroy.
╷
│ Warning: Deprecated Resource
│
│ with module.datadog.datadog_integration_aws.sandbox,
│ on modules/aws/datadog/main.tf line 118, in resource "datadog_integration_aws" "sandbox":
│ 118: resource "datadog_integration_aws" "sandbox" {
│
│ **This resource is deprecated - use the `datadog_integration_aws_account`
│ resource instead**:
│ https://registry.terraform.io/providers/DataDog/datadog/latest/docs/resources/integration_aws_account
╵
╷
│ Warning: Deprecated attribute
│
│ on .terraform/modules/datadog.datadog_log_forwarder/modules/log_forwarder/main.tf line 2, in locals:
│ 2: bucket_name = var.bucket_name != "" ? var.bucket_name : "datadog-forwarder-${data.aws_caller_identity.current.account_id}-${data.aws_region.current.name}"
│
│ The attribute "name" is deprecated. Refer to the provider documentation for
│ details.
│
│ (and 2 more similar warnings elsewhere)
╵
Plan: 2 to add, 5 to change, 4 to destroy.
|
… screens - Added loading states to the Next and Done buttons in various screens (DoneScreen, LanguageScreen, LinksScreen, MediaScreen, SocialScreen, TextScreen) to enhance user experience during asynchronous operations. - Updated button components to reflect loading states and prevent multiple submissions. - Ensured that loading states are properly managed in the component state for better UI feedback.
- Removed dotenv and prisma from package.json as they are no longer needed in the project.
|
| Command | Status | Duration | Result |
|---|---|---|---|
nx run journeys-e2e:e2e |
❌ Failed | 3m 26s | View ↗ |
nx run resources-e2e:e2e |
✅ Succeeded | 23s | View ↗ |
nx run watch-e2e:e2e |
✅ Succeeded | 26s | View ↗ |
nx run journeys-admin-e2e:e2e |
✅ Succeeded | 33s | View ↗ |
nx run player-e2e:e2e |
✅ Succeeded | 3s | View ↗ |
nx run watch-modern-e2e:e2e |
✅ Succeeded | 3s | View ↗ |
nx run short-links-e2e:e2e |
✅ Succeeded | 3s | View ↗ |
nx run videos-admin-e2e:e2e |
✅ Succeeded | 4s | View ↗ |
Additional runs (24) |
✅ Succeeded | ... | View ↗ |
☁️ Nx Cloud last updated this comment at 2026-04-02 23:11:15 UTC
|
View your CI Pipeline Execution ↗ for commit c052613
☁️ Nx Cloud last updated this comment at |
Contributor
This comment has been minimized.
This comment has been minimized.
|
I see you added the "on stage" label, I'll get this merged to the stage branch! |
|
Merge conflict attempting to merge this into stage. Please fix manually. |
This was referenced Mar 30, 2026
Closed
…ation handling This commit modifies the quickStartTemplate function to accept a reset action, allowing for improved customization of journey templates. It enhances the customization description with additional fields and updates the creation of customization fields to use createMany for efficiency. Additionally, it refines the handling of curly braces in translation processes to ensure accurate preservation of template variables. These changes streamline the journey creation process and improve localization capabilities.
This update modifies the `resolveJourneyCustomizationString` function to support both "{{ key }}" and "{{ key: value }}" syntaxes. It allows for inline values to be resolved when no matching field exists, improving the flexibility of customization templates. Additionally, test cases have been updated to reflect these changes, ensuring accurate resolution of both quoted and unquoted inline values.
|
Merge conflict attempting to merge this into stage. Please fix manually. |
This update clarifies the translation instructions by removing redundant examples and emphasizing the importance of preserving template variables within curly braces. The changes aim to enhance the understanding of translation processes while maintaining cultural appropriateness and conciseness in UI contexts.
|
Merge conflict attempting to merge this into stage. Please fix manually. |
This update introduces the `translateValue` function to handle the translation of customization field values alongside the journey customization description. It modifies the `journeyCustomizationDescriptionTranslate` mutation to include logic for translating field values when a description is absent or empty. Additionally, the schema has been updated to include `journeyCustomizationFields` in relevant queries, improving the overall translation capabilities for journey customizations.
…ation This update modifies the `videoPublishChildren` mutation to include additional parameters: `mode` and `dryRun`, allowing for more flexible publishing options. The result type has been updated to reflect changes in published video IDs and counts, and a new type for unpublished videos has been introduced to handle validation failures. Additionally, the `videoPublishChildrenAndLanguages` mutation has been removed to streamline the API. Tests have been updated accordingly to ensure proper functionality.
… videoPublishChildren types This update removes the `videoPublishChildrenAndLanguages` mutation to simplify the API. The `videoPublishChildren` mutation has been enhanced with new parameters: `mode` and `dryRun`, and the result type has been updated to include additional fields for published videos and validation failures. A new type for unpublished videos has also been introduced, improving error handling during the publishing process.
|
Merge conflict attempting to merge this into stage. Please fix manually. |
|
Merge conflict attempting to merge this into stage. Please fix manually. |
This comment has been minimized.
This comment has been minimized.
- Introduced batch translation for customization fields and descriptions to improve efficiency. - Updated translation prompts to ensure cultural appropriateness and preserve specific formats. - Refactored translation logic to handle multiple values in a single AI call, reducing the number of requests. - Added new schemas for batch translation responses and improved error handling in translation processes.
Collaborator
Author
|
@coderabbitai resume |
Contributor
✅ Actions performedReviews resumed. |
Collaborator
Author
|
@coderabbitai review |
Contributor
✅ Actions performedReview triggered.
|
![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: 6
♻️ Duplicate comments (1)
apps/journeys-admin/src/components/TemplateCustomization/MultiStepForm/Screens/LanguageScreen/LanguageScreen.tsx (1)
362-410:⚠️ Potential issue | 🟠 MajorReject guest submissions when the selected language needs AI translation.
When
needsTranslationis true, only signed-in users enter the translation path. Guests still duplicate the current journey and continue, so they can pick a language that has no template variant and still receive a draft in the source language.🛠️ Minimal safe fix
- if (needsTranslation && isSignedIn) { + if (needsTranslation) { + if (!isSignedIn) { + enqueueSnackbar( + t('Sign in to translate templates into a new language.'), + { variant: 'error' } + ) + return + } const sourceLanguageName = journey.language.name.find((name) => !name.primary)?.value ?? ''Also applies to: 413-446
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/journeys-admin/src/components/TemplateCustomization/MultiStepForm/Screens/LanguageScreen/LanguageScreen.tsx` around lines 362 - 410, The code currently allows guests to duplicate and proceed even when needsTranslation is true; change the flow so guests are rejected when translation is required: after computing needsTranslation (use the existing selectedLanguageId, needsTranslation, journeyId), add a guard that if needsTranslation && !isSignedIn you abort progressing (do not call handleJourneyDuplication) and surface an error to the user by setting the appropriate form error/state (e.g., setTranslationCompleted(false) and set a validation error on languageSelect or a general form error) and return; ensure the same guard is applied in the later similar branch around the handleJourneyDuplication/translation setup so guests cannot enter the translation path in either location.
🧹 Nitpick comments (1)
apis/api-journeys-modern/src/schema/journeyAiTranslate/journeyAiTranslate.spec.ts (1)
960-969: These subscription tests are vacuous.Both cases only inspect local mocks/fixtures, so they stay green even if the subscription never calls
translateCustomizationFields. Please drive the subscription path through the test client and assert either the mock invocation or the resulting Prisma writes.Also applies to: 971-994
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apis/api-journeys-modern/src/schema/journeyAiTranslate/journeyAiTranslate.spec.ts` around lines 960 - 969, The tests currently only assert the presence of the mock (mockTranslateCustomizationFields) instead of actually driving the subscription flow; update the spec so the test triggers the real subscription path via the test client/handler used in other tests (e.g., call the subscription endpoint or invoke the subscription handler used in the suite), await completion, and then assert the mock was invoked (mockTranslateCustomizationFields.mock.calls.length > 0) or assert the expected Prisma writes occurred using the test Prisma/mock client (e.g., check prismaMock.create/update calls). Ensure you do not just inspect local fixtures—reset/clear mocks before the test, perform the actual subscription interaction, then assert on mockTranslateCustomizationFields and/or the Prisma mock to validate side effects; apply the same change to the other similar test block that currently only inspects the mock.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In
`@apis/api-journeys-modern/src/schema/journeyAiTranslate/journeyCustomizationDescriptionTranslate.mutation.ts`:
- Around line 91-104: The loop currently always translates field.defaultValue
and writes it into field.value, overwriting any existing customized values;
update the logic in the journey.journeyCustomizationFields handling so you only
translate and write defaults when there is no existing customization (i.e., when
field.value is null/undefined). Constrain the .filter() or add a guard before
calling translateValue to require field.defaultValue != null && (field.value ==
null), then call translateValue(...) and use
prisma.journeyCustomizationField.update(...) only for those fields; keep
function names journey.journeyCustomizationFields, translateValue, and
prisma.journeyCustomizationField.update unchanged.
In
`@apis/api-journeys-modern/src/schema/journeyAiTranslate/translateCustomizationFields/translateCustomizationFields.ts`:
- Around line 10-16: The prompt in translateCustomizationFields forbids
translating "times (time formats, day names, month names)" which prevents
localizing user-facing dates; update the customization prompt used by
translateCustomizationFields to remove or relax that rule so month and weekday
names are allowed to be localized (e.g., change the line "DO NOT translate times
(time formats, day names, month names)" to either remove it or replace with
"Preserve numeric time formats but localize month and weekday names as
appropriate"), ensuring the change is made where the
translateCustomizationFields prompt string/constant is defined so event date
strings like "January 15, 2024" will be translated/localized for target locales.
In `@apis/api-journeys/db/seed.ts`:
- Line 25: The seed entry point is calling quickStartTemplate('reset') which
triggers a destructive delete on every db seed run; remove the hardcoded 'reset'
argument from the seed invocation in seed.ts so quickStartTemplate runs
non-destructively by default, or gate the destructive branch behind an explicit
one-off switch (e.g., check an environment variable like SEED_RESET === 'true'
or a CLI flag and only pass 'reset' to quickStartTemplate when that switch is
present), and ensure any documentation/comments note the required flag for
destructive reset.
In `@apis/api-journeys/db/seeds/quickStartTemplate.ts`:
- Around line 57-82: The createMany call to
prisma.journeyCustomizationField.createMany currently only seeds five keys
(name, church_name, feedback_label, website_label, email_label) but the template
uses additional placeholders (video_title, video_description, response_question,
option_1, option_2, option_3, feedback_hint, connect_title, signup_label) which
must be seeded so they are editable via journeyCustomizationFields; update the
prisma.journeyCustomizationField.createMany(...) invocation(s) in this file (and
the other similar blocks referenced by the reviewer) to include rows for each of
those keys with sensible defaultValue strings, or alternatively change the
template strings back to plain text if they are not intended to be
customizable—ensure you edit the prisma.journeyCustomizationField.createMany
blocks (and any other place that builds journeyCustomizationFields) so the new
placeholder keys are present.
In `@apps/journeys-admin/middleware.ts`:
- Around line 28-29: The locale-to-languageID mapping currently assigns the same
ID ('584') to both keys 'ms' and 'pt', causing Malay and Portuguese to collide;
update the mapping so 'ms' and 'pt' have distinct language IDs (replace one of
the '584' values with the correct ID for Malay or Portuguese) in the locale map
object where 'ms' and 'pt' are defined, and verify the chosen IDs match the
canonical language table used elsewhere (so translation/quick-start lookups
resolve correctly).
In
`@apps/journeys-admin/src/components/JourneyList/JourneyCard/JourneyCardMenu/TranslateJourneyDialog/TranslateJourneyDialog.tsx`:
- Around line 158-170: currentLanguageName is computed by looking only for a
non-primary label and can be empty for languages that expose only a primary
label; update the logic used when calling setTranslationVariables so
userLanguageName falls back to the primary label if currentLanguageName is empty
(e.g., compute primaryLabel = journeyData.language.name.find(({ primary }) =>
primary)?.value and set userLanguageName to currentLanguageName || primaryLabel
|| ''), ensuring this change is applied where setTranslationVariables is invoked
to preserve a usable source-language name.
---
Duplicate comments:
In
`@apps/journeys-admin/src/components/TemplateCustomization/MultiStepForm/Screens/LanguageScreen/LanguageScreen.tsx`:
- Around line 362-410: The code currently allows guests to duplicate and proceed
even when needsTranslation is true; change the flow so guests are rejected when
translation is required: after computing needsTranslation (use the existing
selectedLanguageId, needsTranslation, journeyId), add a guard that if
needsTranslation && !isSignedIn you abort progressing (do not call
handleJourneyDuplication) and surface an error to the user by setting the
appropriate form error/state (e.g., setTranslationCompleted(false) and set a
validation error on languageSelect or a general form error) and return; ensure
the same guard is applied in the later similar branch around the
handleJourneyDuplication/translation setup so guests cannot enter the
translation path in either location.
---
Nitpick comments:
In
`@apis/api-journeys-modern/src/schema/journeyAiTranslate/journeyAiTranslate.spec.ts`:
- Around line 960-969: The tests currently only assert the presence of the mock
(mockTranslateCustomizationFields) instead of actually driving the subscription
flow; update the spec so the test triggers the real subscription path via the
test client/handler used in other tests (e.g., call the subscription endpoint or
invoke the subscription handler used in the suite), await completion, and then
assert the mock was invoked (mockTranslateCustomizationFields.mock.calls.length
> 0) or assert the expected Prisma writes occurred using the test Prisma/mock
client (e.g., check prismaMock.create/update calls). Ensure you do not just
inspect local fixtures—reset/clear mocks before the test, perform the actual
subscription interaction, then assert on mockTranslateCustomizationFields and/or
the Prisma mock to validate side effects; apply the same change to the other
similar test block that currently only inspects the mock.
🪄 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: 9e96cfd5-daa9-47c8-99ec-153726dcbb3e
⛔ Files ignored due to path filters (14)
apis/api-journeys/src/__generated__/graphql.tsis excluded by!**/__generated__/**apps/journeys-admin/__generated__/JourneyAiTranslateCreateSubscription.tsis excluded by!**/__generated__/**apps/journeys-admin/__generated__/JourneyCustomizationDescriptionTranslate.tsis excluded by!**/__generated__/**apps/journeys-admin/__generated__/globalTypes.tsis excluded by!**/__generated__/**apps/journeys/__generated__/JourneyAiTranslateCreateSubscription.tsis excluded by!**/__generated__/**apps/journeys/__generated__/JourneyCustomizationDescriptionTranslate.tsis excluded by!**/__generated__/**apps/journeys/__generated__/globalTypes.tsis excluded by!**/__generated__/**apps/resources/__generated__/JourneyAiTranslateCreateSubscription.tsis excluded by!**/__generated__/**apps/resources/__generated__/JourneyCustomizationDescriptionTranslate.tsis excluded by!**/__generated__/**apps/resources/__generated__/globalTypes.tsis excluded by!**/__generated__/**apps/watch/__generated__/JourneyAiTranslateCreateSubscription.tsis excluded by!**/__generated__/**apps/watch/__generated__/JourneyCustomizationDescriptionTranslate.tsis excluded by!**/__generated__/**apps/watch/__generated__/globalTypes.tsis excluded by!**/__generated__/**libs/journeys/ui/__generated__/globalTypes.tsis excluded by!**/__generated__/**
📒 Files selected for processing (19)
apis/api-gateway/schema.graphqlapis/api-journeys-modern/schema.graphqlapis/api-journeys-modern/src/schema/journeyAiTranslate/index.tsapis/api-journeys-modern/src/schema/journeyAiTranslate/journeyAiTranslate.spec.tsapis/api-journeys-modern/src/schema/journeyAiTranslate/journeyAiTranslate.tsapis/api-journeys-modern/src/schema/journeyAiTranslate/journeyCustomizationDescriptionTranslate.mutation.spec.tsapis/api-journeys-modern/src/schema/journeyAiTranslate/journeyCustomizationDescriptionTranslate.mutation.tsapis/api-journeys-modern/src/schema/journeyAiTranslate/translateCustomizationFields/index.tsapis/api-journeys-modern/src/schema/journeyAiTranslate/translateCustomizationFields/translateCustomizationFields.spec.tsapis/api-journeys-modern/src/schema/journeyAiTranslate/translateCustomizationFields/translateCustomizationFields.tsapis/api-journeys/db/seed.tsapis/api-journeys/db/seeds/quickStartTemplate.tsapps/journeys-admin/middleware.tsapps/journeys-admin/src/components/JourneyList/JourneyCard/JourneyCardMenu/TranslateJourneyDialog/TranslateJourneyDialog.spec.tsxapps/journeys-admin/src/components/JourneyList/JourneyCard/JourneyCardMenu/TranslateJourneyDialog/TranslateJourneyDialog.tsxapps/journeys-admin/src/components/Team/CopyToTeamMenuItem/CopyToTeamMenuItem.spec.tsxapps/journeys-admin/src/components/Team/CopyToTeamMenuItem/CopyToTeamMenuItem.tsxapps/journeys-admin/src/components/TemplateCustomization/MultiStepForm/Screens/LanguageScreen/LanguageScreen.spec.tsxapps/journeys-admin/src/components/TemplateCustomization/MultiStepForm/Screens/LanguageScreen/LanguageScreen.tsx
✅ Files skipped from review due to trivial changes (2)
- apis/api-journeys-modern/src/schema/journeyAiTranslate/index.ts
- apis/api-journeys-modern/src/schema/journeyAiTranslate/journeyCustomizationDescriptionTranslate.mutation.spec.ts
🚧 Files skipped from review as they are similar to previous changes (4)
- apis/api-journeys-modern/src/schema/journeyAiTranslate/translateCustomizationFields/index.ts
- apps/journeys-admin/src/components/Team/CopyToTeamMenuItem/CopyToTeamMenuItem.tsx
- apis/api-gateway/schema.graphql
- apis/api-journeys-modern/src/schema/journeyAiTranslate/journeyAiTranslate.ts
Comment on lines
+91
to
+104
| if (hasFields) { | ||
| await Promise.all( | ||
| journey.journeyCustomizationFields | ||
| .filter((field) => field.defaultValue != null) | ||
| .map(async (field) => { | ||
| const translatedFieldValue = await translateValue({ | ||
| value: field.defaultValue!, | ||
| sourceLanguageName: input.sourceLanguageName, | ||
| targetLanguageName: input.targetLanguageName | ||
| }) | ||
| await prisma.journeyCustomizationField.update({ | ||
| where: { id: field.id }, | ||
| data: { value: translatedFieldValue } | ||
| }) |
Contributor
There was a problem hiding this comment.
Don't overwrite existing customization values with translated defaults.
This loop always translates field.defaultValue and writes it into value. If a journey already has customized values, invoking this mutation will replace them with translated defaults.
🛠️ Minimal safe fix
if (hasFields) {
await Promise.all(
journey.journeyCustomizationFields
- .filter((field) => field.defaultValue != null)
+ .filter(
+ (field) =>
+ field.defaultValue != null && field.value == null
+ )
.map(async (field) => {
const translatedFieldValue = await translateValue({
value: field.defaultValue!,🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In
`@apis/api-journeys-modern/src/schema/journeyAiTranslate/journeyCustomizationDescriptionTranslate.mutation.ts`
around lines 91 - 104, The loop currently always translates field.defaultValue
and writes it into field.value, overwriting any existing customized values;
update the logic in the journey.journeyCustomizationFields handling so you only
translate and write defaults when there is no existing customization (i.e., when
field.value is null/undefined). Constrain the .filter() or add a guard before
calling translateValue to require field.defaultValue != null && (field.value ==
null), then call translateValue(...) and use
prisma.journeyCustomizationField.update(...) only for those fields; keep
function names journey.journeyCustomizationFields, translateValue, and
prisma.journeyCustomizationField.update unchanged.
Comment on lines
+10
to
+16
| You are a professional translation engine. | ||
| - Translate text accurately while preserving meaning and cultural appropriateness | ||
| - DO NOT translate addresses (street addresses, city names, postal codes, country names) | ||
| - DO NOT translate times (time formats, day names, month names) | ||
| - DO NOT translate locations (place names, venue names, building names) | ||
| - DO NOT translate proper nouns (names of people, organizations, brands) | ||
| - Maintain the original format and structure` |
Contributor
There was a problem hiding this comment.
Don't tell the model to keep month/day names in the source language.
This prompt explicitly forbids translating month and day names, but the new customization-field flow needs exactly that for user-facing values like event dates. With this instruction, strings such as January 15, 2024 will tend to stay in the source language instead of localizing for the target audience.
🛠️ Prompt tweak
- - DO NOT translate times (time formats, day names, month names)
+ - Preserve literal clock times and time-zone identifiers when they must remain exact
+ - Localize user-facing dates, including month/day names, when translating copy📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| You are a professional translation engine. | |
| - Translate text accurately while preserving meaning and cultural appropriateness | |
| - DO NOT translate addresses (street addresses, city names, postal codes, country names) | |
| - DO NOT translate times (time formats, day names, month names) | |
| - DO NOT translate locations (place names, venue names, building names) | |
| - DO NOT translate proper nouns (names of people, organizations, brands) | |
| - Maintain the original format and structure` | |
| You are a professional translation engine. | |
| - Translate text accurately while preserving meaning and cultural appropriateness | |
| - DO NOT translate addresses (street addresses, city names, postal codes, country names) | |
| - Preserve literal clock times and time-zone identifiers when they must remain exact | |
| - Localize user-facing dates, including month/day names, when translating copy | |
| - DO NOT translate locations (place names, venue names, building names) | |
| - DO NOT translate proper nouns (names of people, organizations, brands) | |
| - Maintain the original format and structure` |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In
`@apis/api-journeys-modern/src/schema/journeyAiTranslate/translateCustomizationFields/translateCustomizationFields.ts`
around lines 10 - 16, The prompt in translateCustomizationFields forbids
translating "times (time formats, day names, month names)" which prevents
localizing user-facing dates; update the customization prompt used by
translateCustomizationFields to remove or relax that rule so month and weekday
names are allowed to be localized (e.g., change the line "DO NOT translate times
(time formats, day names, month names)" to either remove it or replace with
"Preserve numeric time formats but localize month and weekday names as
appropriate"), ensuring the change is made where the
translateCustomizationFields prompt string/constant is defined so event date
strings like "January 15, 2024" will be translated/localized for target locales.
Comment on lines
+57
to
82
| // Customization fields parsed from the description | ||
| await prisma.journeyCustomizationField.createMany({ | ||
| data: [ | ||
| { journeyId: journey.id, key: 'name', defaultValue: 'Friend' }, | ||
| { | ||
| journeyId: journey.id, | ||
| key: 'church_name', | ||
| defaultValue: 'Our Church' | ||
| }, | ||
| { | ||
| journeyId: journey.id, | ||
| key: 'feedback_label', | ||
| defaultValue: 'share your thoughts' | ||
| }, | ||
| { | ||
| journeyId: journey.id, | ||
| key: 'website_label', | ||
| defaultValue: 'our website' | ||
| }, | ||
| { | ||
| journeyId: journey.id, | ||
| key: 'email_label', | ||
| defaultValue: 'email us' | ||
| } | ||
| ] | ||
| }) |
Contributor
There was a problem hiding this comment.
Seed every new quick-start placeholder key.
The template now uses additional {{ key: default }} placeholders like video_title, video_description, response_question, option_1/2/3, feedback_hint, connect_title, and signup_label, but journeyCustomizationField.createMany() still only creates five keys. The new quick-start field translation path in this PR only reads journeyCustomizationFields, so those extra placeholders won't be editable or translatable through that flow. Either add rows for each new key or switch those strings back to plain text if they are not meant to be customizable.
Also applies to: 221-245, 296-357, 419-438
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@apis/api-journeys/db/seeds/quickStartTemplate.ts` around lines 57 - 82, The
createMany call to prisma.journeyCustomizationField.createMany currently only
seeds five keys (name, church_name, feedback_label, website_label, email_label)
but the template uses additional placeholders (video_title, video_description,
response_question, option_1, option_2, option_3, feedback_hint, connect_title,
signup_label) which must be seeded so they are editable via
journeyCustomizationFields; update the
prisma.journeyCustomizationField.createMany(...) invocation(s) in this file (and
the other similar blocks referenced by the reviewer) to include rows for each of
those keys with sensible defaultValue strings, or alternatively change the
template strings back to plain text if they are not intended to be
customizable—ensure you edit the prisma.journeyCustomizationField.createMany
blocks (and any other place that builds journeyCustomizationFields) so the new
placeholder keys are present.
Comment on lines
+28
to
+29
| ms: '584', // Malay | ||
| pt: '584' // Portuguese |
Contributor
There was a problem hiding this comment.
Give ms and pt distinct language IDs.
These two locales now resolve to the same language ID (584). Unless Malay and Portuguese intentionally share a single language record, one of these locales will map to the wrong language anywhere this table drives quick-start or locale-based translation.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@apps/journeys-admin/middleware.ts` around lines 28 - 29, The
locale-to-languageID mapping currently assigns the same ID ('584') to both keys
'ms' and 'pt', causing Malay and Portuguese to collide; update the mapping so
'ms' and 'pt' have distinct language IDs (replace one of the '584' values with
the correct ID for Malay or Portuguese) in the locale map object where 'ms' and
'pt' are defined, and verify the chosen IDs match the canonical language table
used elsewhere (so translation/quick-start lookups resolve correctly).
Comment on lines
+158
to
+170
| const currentLanguageName = | ||
| journeyData.language.name.find(({ primary }) => !primary)?.value ?? '' | ||
|
|
||
| // Start the translation subscription | ||
| setTranslationVariables({ | ||
| journeyId: duplicateData.journeyDuplicate.id, | ||
| name: `${journeyData.title}`, | ||
| journeyLanguageName: | ||
| journeyData.language.name.find(({ primary }) => !primary)?.value ?? | ||
| '', | ||
| journeyLanguageName: currentLanguageName, | ||
| textLanguageId: selectedLanguage.id, | ||
| textLanguageName: | ||
| selectedLanguage.nativeName ?? selectedLanguage.localName ?? '' | ||
| selectedLanguage.nativeName ?? selectedLanguage.localName ?? '', | ||
| userLanguageId: journeyData.language.id, | ||
| userLanguageName: currentLanguageName |
Contributor
There was a problem hiding this comment.
Fall back to the primary language name before setting userLanguageName.
This lookup only considers non-primary names. For languages that only expose a primary label, currentLanguageName becomes '', so the new userLanguageName loses the source-language hint even though a usable name exists.
💡 Suggested change
- const currentLanguageName =
- journeyData.language.name.find(({ primary }) => !primary)?.value ?? ''
+ const currentLanguageName =
+ journeyData.language.name.find(({ primary }) => !primary)?.value ??
+ journeyData.language.name.find(({ primary }) => primary)?.value
// Start the translation subscription
setTranslationVariables({
journeyId: duplicateData.journeyDuplicate.id,
name: `${journeyData.title}`,
- journeyLanguageName: currentLanguageName,
+ journeyLanguageName: currentLanguageName ?? '',
textLanguageId: selectedLanguage.id,
textLanguageName:
selectedLanguage.nativeName ?? selectedLanguage.localName ?? '',
userLanguageId: journeyData.language.id,
- userLanguageName: currentLanguageName
+ ...(currentLanguageName != null
+ ? { userLanguageName: currentLanguageName }
+ : {})
})🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In
`@apps/journeys-admin/src/components/JourneyList/JourneyCard/JourneyCardMenu/TranslateJourneyDialog/TranslateJourneyDialog.tsx`
around lines 158 - 170, currentLanguageName is computed by looking only for a
non-primary label and can be empty for languages that expose only a primary
label; update the logic used when calling setTranslationVariables so
userLanguageName falls back to the primary label if currentLanguageName is empty
(e.g., compute primaryLabel = journeyData.language.name.find(({ primary }) =>
primary)?.value and set userLanguageName to currentLanguageName || primaryLabel
|| ''), ensuring this change is applied where setTranslationVariables is invoked
to preserve a usable source-language name.
Contributor
|
Found 1 test failure on Blacksmith runners: Failure
|
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
3 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 by CodeRabbit
Release Notes