-
Notifications
You must be signed in to change notification settings - Fork 19
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
localizations for user management and user tag #1950
Conversation
📝 WalkthroughWalkthroughThe changes in this pull request involve significant modifications to the Changes
Possibly related PRs
Suggested reviewers
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 3
🧹 Outside diff range comments (6)
health/micro-ui/web/micro-ui-internals/packages/modules/microplan/src/configs/UICustomizations.js (4)
Line range hint
72-146
: Consider refactoring repeated localization patternsThe switch cases for handling localizations follow similar patterns that could be refactored into a reusable helper function to reduce code duplication.
Consider creating a helper function like this:
const renderLocalizedField = (value, prefix = "", options = {}) => { if (value && value !== "NA") { return ( <div style={options.style}> <p>{t(prefix ? Digit.Utils.locale.getTransformedLocale(prefix + value) : value)}</p> </div> ); } return <div><p>{t("NA")}</p></div>; };Then use it in the switch cases:
case "CAMPAIGN_DISEASE": return renderLocalizedField(value, "MICROPLAN_DISEASE_"); case "CAMPAIGN_TYPE": return renderLocalizedField(value, "MICROPLAN_TYPE_");
Line range hint
297-374
: Standardize "NA" text keys across the fileThere's inconsistent usage of NA text keys ("NA" vs "ES_COMMON_NA"). This should be standardized for better maintainability.
Standardize to use "ES_COMMON_NA" consistently:
-<p>{t("NA")}</p> +<p>{t("ES_COMMON_NA")}</p>Extract common styles to a constant
The text wrapping styles are duplicated across multiple cases.
Consider extracting the common styles:
const TEXT_WRAP_STYLES = { maxWidth: "15rem", wordWrap: "break-word", whiteSpace: "normal", overflowWrap: "break-word" }; // Then use it in the render methods: <div style={TEXT_WRAP_STYLES}>
Line range hint
392-408
: Consider extracting validation regex to constantsThe mobile number validation regex should be extracted to a named constant for better maintainability and reusability.
+const MOBILE_NUMBER_REGEX = /^[0-9]{10}$/; customValidationCheck: (data) => { const { phone } = data; - const mobileRegex = /^[0-9]{10}$/; if (!phone || phone.trim() === "") { return false; } - if (!mobileRegex.test(phone)) { + if (!MOBILE_NUMBER_REGEX.test(phone)) { return { error: true, label: "INVALID_MOBILE_NUMBER" }; } return false; }
Line range hint
589-604
: Remove commented debug codeThere's a commented line for debugging popup functionality that should be removed.
-// removed this because due to popup crashing on dev -// onClick={() => console.log("temp action")}health/micro-ui/web/micro-ui-internals/packages/modules/microplan/src/components/RoleTableComposer.js (1)
Line range hint
1-644
: Consider performance optimizations.The component could benefit from several performance optimizations:
- Memoize callback functions using
useCallback
- Memoize derived state using
useMemo
- Debounce search input changes
Example optimizations:
// Memoize callbacks const handleSearchSubmit = useCallback((e) => { if (number?.length > 0 && number?.length !== 10) { setShowToast({ key: "error", label: t("INVALID_MOBILE_NUMBER_LENGTH") }); return; } setCurrentPage(1); setFilters({ name: name, number: number, }); }, [number, name]); // Memoize derived state const columns = useMemo(() => [ // ... column definitions ], [t]); // Only re-create when translations change // Debounce search input const debouncedSetName = useCallback( debounce((value) => setName(value), 300), [] );health/micro-ui/web/micro-ui-internals/packages/modules/microplan/src/configs/UserManagementConfig.js (1)
Line range hint
1-179
: Consider localizing remaining static strings.Several strings in the configuration are still using static text instead of localization keys:
- Search fields: "NAME", "CONTACT_NUMBER"
- Filter section: "ROLES"
- Button labels: "Search"
Consider updating these to follow the same localization pattern for consistency across the application.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
📒 Files selected for processing (4)
health/micro-ui/web/micro-ui-internals/example/src/UICustomizations.js
(1 hunks)health/micro-ui/web/micro-ui-internals/packages/modules/microplan/src/components/RoleTableComposer.js
(1 hunks)health/micro-ui/web/micro-ui-internals/packages/modules/microplan/src/configs/UICustomizations.js
(1 hunks)health/micro-ui/web/micro-ui-internals/packages/modules/microplan/src/configs/UserManagementConfig.js
(3 hunks)
🧰 Additional context used
📓 Path-based instructions (4)
health/micro-ui/web/micro-ui-internals/packages/modules/microplan/src/components/RoleTableComposer.js (1)
Pattern **/*.js
: check
health/micro-ui/web/micro-ui-internals/packages/modules/microplan/src/configs/UserManagementConfig.js (1)
Pattern **/*.js
: check
health/micro-ui/web/micro-ui-internals/example/src/UICustomizations.js (1)
Pattern **/*.js
: check
health/micro-ui/web/micro-ui-internals/packages/modules/microplan/src/configs/UICustomizations.js (1)
Pattern **/*.js
: check
🔇 Additional comments (3)
health/micro-ui/web/micro-ui-internals/packages/modules/microplan/src/configs/UICustomizations.js (1)
Line range hint 531-544
: Role localization implementation looks good
The implementation correctly uses the "MP_ROLE_" prefix for localizing role codes, which aligns with the PR objectives.
health/micro-ui/web/micro-ui-internals/example/src/UICustomizations.js (1)
Line range hint 1248-1256
: Ensure consistent use of the updated key 'MP_USER_MANAGEMENT_ROLE'
Please verify that all references to the key "Role"
have been updated to "MP_USER_MANAGEMENT_ROLE"
throughout the codebase to maintain consistency and prevent potential issues.
Run the following script to identify any remaining occurrences of the key "Role"
:
health/micro-ui/web/micro-ui-internals/packages/modules/microplan/src/configs/UserManagementConfig.js (1)
85-100
: LGTM! Verify localization keys exist in translation files.
The column labels have been properly updated with consistent 'MP_USER_MANAGEMENT_' prefixes, which aligns with localization best practices.
Let's verify these localization keys exist in the translation files:
localizations for user management and user tag
Summary by CodeRabbit
New Features
Bug Fixes
Documentation
Refactor