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.
WalkthroughAdds AI translation for journey customization descriptions and fields: new translation utilities, a standalone mutation, integration into the existing journey AI translate flow (mutation + subscription), GraphQL and UI changes to pass user language hints, and tests + cache updates to persist translated customization data. Changes
Sequence Diagram(s)sequenceDiagram
participant User as User
participant UI as Admin UI
participant Sub as Translate<br/>Subscription
participant API as Journeys API
participant AI as Google Gemini
participant DB as Database
User->>UI: Request duplicate / translate
UI->>Sub: Start subscription with journeyId, textLanguage, userLanguage?
Sub->>API: journeyAiTranslateCreate subscription event
API->>DB: Fetch journey, blocks, customization fields
API->>AI: Stream translate blocks (per-card)
AI-->>API: Block translations (stream)
API-->>Sub: Stream progress (blocks)
API->>AI: Translate customization description & field values (batched)
AI-->>API: Translated description & field values
API->>DB: Update blocks, journeyCustomizationFields, journeyCustomizationDescription
API-->>Sub: Final translated journey (100%)
Sub-->>UI: Progress 100%, provide translated journey
UI->>User: Navigate / show success
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
🚥 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 |
💰 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.
|
View your CI Pipeline Execution ↗ for commit 7cf697d
☁️ Nx Cloud last updated this comment at |
|
View your CI Pipeline Execution ↗ for commit c052613
☁️ Nx Cloud last updated this comment at |
…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.
|
@coderabbitai resume |
✅ Actions performedReviews resumed. |
|
@coderabbitai review |
✅ Actions performedReview triggered.
|
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
...ys-modern/src/schema/journeyAiTranslate/journeyCustomizationDescriptionTranslate.mutation.ts
Show resolved
Hide resolved
...n/src/schema/journeyAiTranslate/translateCustomizationFields/translateCustomizationFields.ts
Show resolved
Hide resolved
| ms: '584', // Malay | ||
| pt: '584' // Portuguese |
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).
...ts/JourneyList/JourneyCard/JourneyCardMenu/TranslateJourneyDialog/TranslateJourneyDialog.tsx
Show resolved
Hide resolved
This comment has been minimized.
This comment has been minimized.
- Updated `getTextResponseValues` to prefer `defaultValue` over `value` for non-admin variants. - Modified tests to reflect changes in expected output for both default and embed variants. - Introduced `useDefaultValue` option in `resolveJourneyCustomizationString` to support this behavior. - Updated related hooks to ensure consistent resolution logic across different contexts.
There was a problem hiding this comment.
🧹 Nitpick comments (2)
libs/journeys/ui/src/libs/resolveJourneyCustomizationString/resolveJourneyCustomizationString.ts (1)
13-16: Consider extracting the options type for reusability.The inline type
{ useDefaultValue?: boolean }is clear, but extracting it as a named interface would improve discoverability and make it easier to extend in the future. As per coding guidelines, defining a type is preferred.♻️ Proposed refactor
+export interface ResolveJourneyCustomizationStringOptions { + useDefaultValue?: boolean +} + export function resolveJourneyCustomizationString( input: string | null, journeyCustomizationFields: JourneyCustomizationField[], - options?: { useDefaultValue?: boolean } + options?: ResolveJourneyCustomizationStringOptions ): string | null {🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@libs/journeys/ui/src/libs/resolveJourneyCustomizationString/resolveJourneyCustomizationString.ts` around lines 13 - 16, Extract the inline options type in resolveJourneyCustomizationString into a reusable named interface (e.g. ResolveJourneyCustomizationOptions with useDefaultValue?: boolean), export it if it should be consumed elsewhere, and update the function signature to use this new interface (resolveJourneyCustomizationString(input: string | null, journeyCustomizationFields: JourneyCustomizationField[], options?: ResolveJourneyCustomizationOptions)). Replace any other occurrences of the inline type in this module with the new interface so the type is discoverable and easier to extend.libs/journeys/ui/src/libs/useGetValueFromJourneyCustomizationString/useGetValueFromJourneyCustomizationString.ts (1)
18-29: Consider addingjourney?.templateto the dependency array.The memoization depends on
journey?.templatefor the early return condition (line 20), but it's not included in the dependency array. Ifjourney.templatechanges fromtruetofalsewhilevariantremains'admin', the hook won't recompute and could return a stale value.🔧 Proposed fix
- }, [label, variant, journey?.journeyCustomizationFields]) + }, [label, variant, journey?.template, journey?.journeyCustomizationFields])🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@libs/journeys/ui/src/libs/useGetValueFromJourneyCustomizationString/useGetValueFromJourneyCustomizationString.ts` around lines 18 - 29, The memoized return in useGetValueFromJourneyCustomizationString uses journey?.template in the conditional early return inside the arrow in useMemo, but the dependency array only lists [label, variant, journey?.journeyCustomizationFields]; update the dependency array to include journey?.template so the hook recomputes when template changes, i.e., modify the useMemo dependencies referenced in useGetValueFromJourneyCustomizationString to include journey?.template along with the existing dependencies.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In
`@libs/journeys/ui/src/libs/resolveJourneyCustomizationString/resolveJourneyCustomizationString.ts`:
- Around line 13-16: Extract the inline options type in
resolveJourneyCustomizationString into a reusable named interface (e.g.
ResolveJourneyCustomizationOptions with useDefaultValue?: boolean), export it if
it should be consumed elsewhere, and update the function signature to use this
new interface (resolveJourneyCustomizationString(input: string | null,
journeyCustomizationFields: JourneyCustomizationField[], options?:
ResolveJourneyCustomizationOptions)). Replace any other occurrences of the
inline type in this module with the new interface so the type is discoverable
and easier to extend.
In
`@libs/journeys/ui/src/libs/useGetValueFromJourneyCustomizationString/useGetValueFromJourneyCustomizationString.ts`:
- Around line 18-29: The memoized return in
useGetValueFromJourneyCustomizationString uses journey?.template in the
conditional early return inside the arrow in useMemo, but the dependency array
only lists [label, variant, journey?.journeyCustomizationFields]; update the
dependency array to include journey?.template so the hook recomputes when
template changes, i.e., modify the useMemo dependencies referenced in
useGetValueFromJourneyCustomizationString to include journey?.template along
with the existing dependencies.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
Run ID: ddaaabef-3735-4128-b09c-ca5c1ef6d5b7
📒 Files selected for processing (6)
libs/journeys/ui/src/components/TextResponse/utils/getTextResponseValues/getTextResponseValues.spec.tslibs/journeys/ui/src/components/TextResponse/utils/getTextResponseValues/getTextResponseValues.tslibs/journeys/ui/src/libs/resolveJourneyCustomizationString/resolveJourneyCustomizationString.spec.tslibs/journeys/ui/src/libs/resolveJourneyCustomizationString/resolveJourneyCustomizationString.tslibs/journeys/ui/src/libs/useGetValueFromJourneyCustomizationString/useGetValueFromJourneyCustomizationString.spec.tsxlibs/journeys/ui/src/libs/useGetValueFromJourneyCustomizationString/useGetValueFromJourneyCustomizationString.ts
Summary by CodeRabbit
New Features
Bug Fixes
Behavior Changes