-
Notifications
You must be signed in to change notification settings - Fork 42
FEATURE/2327 app configuration screen development #2385
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
FEATURE/2327 app configuration screen development #2385
Conversation
📝 WalkthroughWalkthroughThis set of changes updates several components within the campaign-manager module of the micro-UI. The updates include modifying how locale objects are handled (from strings to objects with value/label), dynamically configuring the MDMS service endpoint, refining the fetching and usage of MDMS data (including adding schema code and removing some modules), and enhancing localization logic to support dynamic and multiple locales. Navigation URLs are also updated to include new query parameters. No changes are made to the signatures or exports of public entities. Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant AppLocalisationWrapper
participant SessionStorage
participant MDMS
participant AppLocalisationTable
User->>AppLocalisationWrapper: Loads component
AppLocalisationWrapper->>SessionStorage: Reads enabledModules, currentLocale
AppLocalisationWrapper->>MDMS: Fetches AppScreenLocalisationConfig (with caching)
AppLocalisationWrapper->>MDMS: Fetches localisation data (dynamic locale array)
MDMS-->>AppLocalisationWrapper: Returns localisation data
AppLocalisationWrapper->>AppLocalisationTable: Provides context (enabledModules, locales)
User->>AppLocalisationTable: Interacts with locale tabs (now objects)
AppLocalisationTable->>AppLocalisationWrapper: Updates/reads localization state
Possibly related PRs
Suggested reviewers
Poem
✨ Finishing Touches
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. 🪧 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
📜 Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (5)
health/micro-ui/web/micro-ui-internals/packages/modules/campaign-manager/src/pages/employee/NewCampaignCreate/AppFeatures.js
(2 hunks)health/micro-ui/web/micro-ui-internals/packages/modules/campaign-manager/src/pages/employee/appConfigurationRedesign/AppConfigurationParentLayer.js
(1 hunks)health/micro-ui/web/micro-ui-internals/packages/modules/campaign-manager/src/pages/employee/appConfigurationRedesign/AppConfigurationWrapper.js
(1 hunks)health/micro-ui/web/micro-ui-internals/packages/modules/campaign-manager/src/pages/employee/appConfigurationRedesign/AppLocalisationTable.js
(4 hunks)health/micro-ui/web/micro-ui-internals/packages/modules/campaign-manager/src/pages/employee/appConfigurationRedesign/AppLocalisationWrapper.js
(5 hunks)
🧰 Additional context used
📓 Path-based instructions (1)
`**/*.js`: check
**/*.js
: check
health/micro-ui/web/micro-ui-internals/packages/modules/campaign-manager/src/pages/employee/appConfigurationRedesign/AppConfigurationParentLayer.js
health/micro-ui/web/micro-ui-internals/packages/modules/campaign-manager/src/pages/employee/appConfigurationRedesign/AppLocalisationTable.js
health/micro-ui/web/micro-ui-internals/packages/modules/campaign-manager/src/pages/employee/appConfigurationRedesign/AppLocalisationWrapper.js
health/micro-ui/web/micro-ui-internals/packages/modules/campaign-manager/src/pages/employee/appConfigurationRedesign/AppConfigurationWrapper.js
health/micro-ui/web/micro-ui-internals/packages/modules/campaign-manager/src/pages/employee/NewCampaignCreate/AppFeatures.js
🧬 Code Graph Analysis (2)
health/micro-ui/web/micro-ui-internals/packages/modules/campaign-manager/src/pages/employee/appConfigurationRedesign/AppConfigurationParentLayer.js (1)
health/micro-ui/web/micro-ui-internals/packages/modules/campaign-manager/src/pages/employee/appConfigurationScreenParent/AppConfigurationParentLayer.js (1)
reqCriteriaForm
(76-95)
health/micro-ui/web/micro-ui-internals/packages/modules/campaign-manager/src/pages/employee/appConfigurationRedesign/AppLocalisationTable.js (2)
health/micro-ui/web/micro-ui-internals/packages/modules/campaign-manager/src/pages/employee/appConfigurationScreenParent/AppLocalisationTable.js (4)
activeLocale
(28-28)availableLocales
(27-27)currentLocale
(26-26)columns
(30-50)health/micro-ui/web/micro-ui-internals/packages/modules/campaign-manager/src/pages/employee/appConfigurationRedesign/AppLocalisationWrapper.js (3)
currentLocale
(40-40)updateLocalization
(45-50)locState
(35-35)
🪛 Biome (1.9.4)
health/micro-ui/web/micro-ui-internals/packages/modules/campaign-manager/src/pages/employee/appConfigurationRedesign/AppLocalisationWrapper.js
[error] 22-22: Avoid the use of spread (...
) syntax on accumulators.
Spread syntax should be avoided on accumulators (like those in .reduce
) because it causes a time complexity of O(n^2)
.
Consider methods such as .splice or .push instead.
(lint/performance/noAccumulatingSpread)
[error] 57-57: Use Number.POSITIVE_INFINITY instead of the equivalent global.
ES2015 moved some globals into the Number namespace for consistency.
Safe fix: Use Number.POSITIVE_INFINITY instead.
(lint/style/useNumberNamespace)
[error] 58-58: Use Number.POSITIVE_INFINITY instead of the equivalent global.
ES2015 moved some globals into the Number namespace for consistency.
Safe fix: Use Number.POSITIVE_INFINITY instead.
(lint/style/useNumberNamespace)
⏰ Context from checks skipped due to timeout of 10000ms (2)
- GitHub Check: Analyze (javascript-typescript)
- GitHub Check: Analyze (actions)
🔇 Additional comments (10)
health/micro-ui/web/micro-ui-internals/packages/modules/campaign-manager/src/pages/employee/appConfigurationRedesign/AppConfigurationParentLayer.js (1)
185-187
: Good use of configurable MDMS context pathThe implementation of a configurable MDMS context path is a good practice. By retrieving the path from global configs with a fallback value, you've made the code more maintainable and adaptable to different environments.
health/micro-ui/web/micro-ui-internals/packages/modules/campaign-manager/src/pages/employee/appConfigurationRedesign/AppConfigurationWrapper.js (1)
273-273
:✅ Verification successful
Improved separation of concerns in MDMS data fetching
Removing the "AppScreenLocalisationConfig" from this hook call is good for separation of concerns, as the localization data is now handled by the AppLocalisationWrapper component.
Please verify that the AppLocalisationWrapper component properly fetches this localization data to ensure no functionality is lost.
🏁 Script executed:
#!/bin/bash # Verify that AppLocalisationWrapper is fetching the localization config rg --type js "AppScreenLocalisationConfig" -A 5 -B 5 health/micro-ui/web/micro-ui-internals/packages/modules/campaign-manager/Length of output: 7032
Verified localization fetching in AppLocalisationWrapper
The
AppLocalisationWrapper
component correctly retrievesAppScreenLocalisationConfig
via theuseCustomMDMS
hook:
- File:
health/micro-ui/web/micro-ui-internals/packages/modules/campaign-manager/src/pages/employee/appConfigurationRedesign/AppLocalisationWrapper.js
Hook call includes[{ name: "AppScreenLocalisationConfig" }]
.- The
select
option returnsdata?.[MODULE_CONSTANTS]?.["AppScreenLocalisationConfig"]?.[0]
.No functionality is lost.
health/micro-ui/web/micro-ui-internals/packages/modules/campaign-manager/src/pages/employee/NewCampaignCreate/AppFeatures.js (2)
32-33
: Added schema code for enhanced MDMS integrationAdding the schema code parameter to the useCustomMDMS hook aligns with the broader pattern of MDMS refinements in this PR.
77-79
: Updated navigation with formId parameterThe URL now includes a formId parameter which supports the app configuration redesign feature.
health/micro-ui/web/micro-ui-internals/packages/modules/campaign-manager/src/pages/employee/appConfigurationRedesign/AppLocalisationWrapper.js (6)
1-1
: Updated imports for enhanced functionality.The additions of
useState
and theLoader
component improve the component's capabilities for state management and loading state visualization.Also applies to: 3-3
39-40
: Dynamic locale configuration from session storage.Good improvement to retrieve language settings from session storage instead of hardcoding values, making the component more flexible and adaptable to user preferences.
42-44
: Updated addMissingKey to utilize dynamic modules.The function now correctly passes the dynamic
enabledModules
to the reducer, aligning with the previous changes for flexible localization.
68-70
: Dynamic locale handling based on available languages.The changes properly determine whether to use a single locale or multiple locales based on the contents of
enabledModules
, improving the flexibility of the localization system.🧰 Tools
🪛 Biome (1.9.4)
[error] 70-70: Unnecessary use of boolean literals in conditional expression.
Simplify your code by directly assigning the result without using a ternary operator.
If your goal is negation, you may use the logical NOT (!) or double NOT (!!) operator for clearer and concise code.
Check for more details about NOT operator.
Unsafe fix: Remove the conditional expression with(lint/complexity/noUselessTernary)
80-91
: Enhanced useEffect for localization data handling.The effect now includes additional context data (current locale and enabled modules) when dispatching localization data to the reducer, ensuring consumers have access to complete information.
94-96
: Context provider now includes enabledModules.Exposing
enabledModules
through the context is a good improvement, allowing child components to access available languages without additional prop drilling.
Choose the appropriate template for your PR:
Feature PR
Feature Request
JIRA ID
Module
Description
Related Issues
Bugfix PR
Bugfix Request
JIRA ID
Module
Description
Root Cause
Related Issues
Release PR
Summary by CodeRabbit