fix: correct customMessages type definitions to match translator implementation - #428
Open
rax7389 wants to merge 2 commits into
Open
fix: correct customMessages type definitions to match translator implementation#428rax7389 wants to merge 2 commits into
rax7389 wants to merge 2 commits into
Conversation
Contributor
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Contributor
Author
|
@coderabbitai Review the PR changes |
Contributor
|
✅ Action performedReview finished.
|
![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)
packages/react/src/components/auth0/my-organization/shared/domain-management/domain-configure/__tests__/domain-configure-providers-modal.test.tsx (1)
97-103: 🎯 Functional Correctness | 🔵 Trivial | ⚡ Quick winAssert the complete translated output.
getByText(/Custom Title/)can pass even when{domain}interpolation is broken, and this test never verifies the custom description. Assert both rendered strings using the mock domain value.Suggested assertion
- expect(screen.getByText(/Custom Title/)).toBeInTheDocument(); + const domain = props.domain?.domain ?? ''; + expect(screen.getByText(`Custom Title - ${domain}`)).toBeInTheDocument(); + expect(screen.getByText(`Custom description for ${domain}`)).toBeInTheDocument();🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/react/src/components/auth0/my-organization/shared/domain-management/domain-configure/__tests__/domain-configure-providers-modal.test.tsx` around lines 97 - 103, Update the test for DomainConfigureProvidersModal to assert the complete translated title using the mock domain value, ensuring {domain} interpolation is verified. Also add an assertion for the complete custom description rendered with that same domain value, replacing the partial title match.packages/core/src/i18n/custom-messages/my-organization/idp-management/sso-provider/sso-provider-table-types.ts (1)
28-29: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winTooltip field naming inconsistent with the sibling addition in this PR.
enabled_tooltip/disabled_tooltipdeviate from theenable_X_tooltip/disable_X_tooltipconvention used for the equivalent new fields added indomain-configure-types.ts(enable_provider_tooltip/disable_provider_tooltip) and the existingenable_domain_tooltip/disable_domain_tooltippattern insso-domain-tab-types.ts. Consider renaming for consistency, e.g.enable_provider_tooltip/disable_provider_tooltip.♻️ Suggested rename
actions?: { edit_button_text?: string; delete_button_text?: string; remove_button_text?: string; - enabled_tooltip?: string; - disabled_tooltip?: string; + enable_provider_tooltip?: string; + disable_provider_tooltip?: string; };🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/core/src/i18n/custom-messages/my-organization/idp-management/sso-provider/sso-provider-table-types.ts` around lines 28 - 29, Rename the tooltip fields in the SSO provider table types from enabled_tooltip and disabled_tooltip to enable_provider_tooltip and disable_provider_tooltip, matching the naming convention used by domain-configure-types and sso-domain-tab-types. Update all references to these fields consistently.packages/core/src/i18n/custom-messages/my-organization/idp-management/sso-domain/sso-domain-tab-types.ts (1)
7-54: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winReuse the shared domain message interfaces for
DomainTab*Messages.
DomainTabCreateMessages,DomainTabDeleteMessages, andDomainTabVerifyMessagesduplicate the domain-management message interfaces rather than importing/aliasing them. Reuse the existingDomainCreateMessages,DomainDeleteMessages, andDomainVerifyMessagestypes to avoid duplicated shape maintenance.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/core/src/i18n/custom-messages/my-organization/idp-management/sso-domain/sso-domain-tab-types.ts` around lines 7 - 54, Replace the duplicated DomainTabCreateMessages, DomainTabDeleteMessages, and DomainTabVerifyMessages definitions with imports or type aliases to DomainCreateMessages, DomainDeleteMessages, and DomainVerifyMessages in packages/core/src/i18n/custom-messages/my-organization/idp-management/sso-domain/sso-domain-tab-types.ts. The shared definitions in packages/core/src/i18n/custom-messages/my-organization/domain-management/domain-create-types.ts, domain-delete-types.ts, and domain-verify-types.ts require no direct changes.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Nitpick comments:
In
`@packages/core/src/i18n/custom-messages/my-organization/idp-management/sso-domain/sso-domain-tab-types.ts`:
- Around line 7-54: Replace the duplicated DomainTabCreateMessages,
DomainTabDeleteMessages, and DomainTabVerifyMessages definitions with imports or
type aliases to DomainCreateMessages, DomainDeleteMessages, and
DomainVerifyMessages in
packages/core/src/i18n/custom-messages/my-organization/idp-management/sso-domain/sso-domain-tab-types.ts.
The shared definitions in
packages/core/src/i18n/custom-messages/my-organization/domain-management/domain-create-types.ts,
domain-delete-types.ts, and domain-verify-types.ts require no direct changes.
In
`@packages/core/src/i18n/custom-messages/my-organization/idp-management/sso-provider/sso-provider-table-types.ts`:
- Around line 28-29: Rename the tooltip fields in the SSO provider table types
from enabled_tooltip and disabled_tooltip to enable_provider_tooltip and
disable_provider_tooltip, matching the naming convention used by
domain-configure-types and sso-domain-tab-types. Update all references to these
fields consistently.
In
`@packages/react/src/components/auth0/my-organization/shared/domain-management/domain-configure/__tests__/domain-configure-providers-modal.test.tsx`:
- Around line 97-103: Update the test for DomainConfigureProvidersModal to
assert the complete translated title using the mock domain value, ensuring
{domain} interpolation is verified. Also add an assertion for the complete
custom description rendered with that same domain value, replacing the partial
title match.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: 23e108c1-cfb0-47f6-93ce-cc12ef79a578
📒 Files selected for processing (12)
packages/core/src/i18n/custom-messages/my-organization/domain-management/domain-configure-types.tspackages/core/src/i18n/custom-messages/my-organization/domain-management/domain-create-types.tspackages/core/src/i18n/custom-messages/my-organization/domain-management/domain-delete-types.tspackages/core/src/i18n/custom-messages/my-organization/domain-management/domain-table-types.tspackages/core/src/i18n/custom-messages/my-organization/domain-management/domain-verify-types.tspackages/core/src/i18n/custom-messages/my-organization/idp-management/sso-domain/sso-domain-tab-types.tspackages/core/src/i18n/custom-messages/my-organization/idp-management/sso-provider/sso-provider-table-types.tspackages/react/src/components/auth0/my-organization/__tests__/domain-table.test.tsxpackages/react/src/components/auth0/my-organization/shared/domain-management/domain-configure/__tests__/domain-configure-providers-modal.test.tsxpackages/react/src/components/auth0/my-organization/shared/domain-management/domain-create/__tests__/domain-create-modal.test.tsxpackages/react/src/components/auth0/my-organization/shared/domain-management/domain-table/__tests__/domain-table-actions-column.test.tsxpackages/react/src/components/auth0/my-organization/shared/domain-management/domain-table/domain-table-actions-column.tsx
11 tasks
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
1 participant
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
Fix customMessages type definitions to align with the actual translator implementation, ensuring consumers can correctly override all domain management text via the
customMessagesprop.Why
The customMessages type definitions had structural mismatches with the translator key paths. For example, types had
header.titlebut the translator looked fordomain_table.header.title. This caused custom message overrides to silently fail.What
DomainTableMessagesto includedomain_tablewrapper matching the translator namespace/key structureshared.domain_statusesto allow status badge customizationmodalwrapper fromDomainCreateMessages,DomainVerifyMessages,DomainConfigureMessages, andDomainDeleteMessagessince child components use namespace with.modalsuffixdelete_button_text(was incorrectly namedcreate_button_text) in delete message typesenable_provider_tooltipanddisable_provider_tooltiptoDomainConfigureMessagesandSsoProviderTableMessagesDomainTableActionsColumnto use correct namespace and key prefixesPackages
packages/corepackages/reactexamplesTesting
Checklist
Contributing
Summary by CodeRabbit