fixed product chip and summary issue #1941
Conversation
📝 WalkthroughWalkthroughThe pull request involves updates to several files primarily related to the health micro UI component. Key changes include the increment of the CSS version from Changes
Possibly related PRs
Suggested reviewers
Poem
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 (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 6
🧹 Outside diff range comments (4)
health/micro-ui/web/micro-ui-internals/example/public/index.html (1)
Line range hint
17-20: Clean up commented codeThere are several commented-out CSS links that appear to be outdated or unused. Consider removing these if they're no longer needed to improve code maintainability.
- <!-- added below css for hcm-workbench module inclusion--> - <!-- <link rel="stylesheet" href="https://unpkg.com/@egovernments/[email protected]/dist/index.css" /> --> - - <!-- <link rel="stylesheet" href="https://unpkg.com/@egovernments/digit-ui-css/dist/index.css"/> -->health/micro-ui/web/micro-ui-internals/packages/css/src/pages/employee/index.scss (1)
Line range hint
1-924: Consider refactoring repeated table styles using SCSS mixinsThe file has several table styles with similar properties. Consider creating a mixin for common table styles to improve maintainability and reduce code duplication. For example:
@mixin campaign-table-styles { margin-bottom: 1rem; border: 1px solid #d6d5d4; overflow: hidden; border-radius: 4px; table { background-color: #fafafa; border: 1px solid #d6d5d4; border-collapse: collapse; border-radius: 1rem; border-width: 0px !important; tbody { tr:nth-child(odd) { background-color: white; } } th { border-right: 1px solid #d6d5d4; &:last-child { border-right: none; } } td { padding: 1rem; border-right: 1px solid #d6d5d4; &:last-child { border-right: none; } } } } .campaign-attribute-table, .campaign-product-table { @include campaign-table-styles; }health/micro-ui/web/micro-ui-internals/packages/modules/campaign-manager/src/pages/employee/deliveryRule/AddDeliverycontext.js (2)
Line range hint
831-835: Consider optimizing the structure code filteringThe nested flatMap operations for filtering selected structure codes could be simplified for better performance.
Consider using a Set for faster lookups:
-const selectedStructureCodes = campaignData?.flatMap((cycle) => - cycle?.deliveries?.flatMap((delivery) => delivery?.deliveryRules?.flatMap((rule) => rule?.attributes?.map((attribute) => attribute?.value))) -); +const selectedStructureCodes = new Set( + campaignData?.flatMap((cycle) => + cycle?.deliveries?.flatMap((delivery) => + delivery?.deliveryRules?.flatMap((rule) => + rule?.attributes?.map((attribute) => attribute?.value) + ) + ) + )?.filter(Boolean) +);
Line range hint
1-1000: Consider improving component architectureThe current implementation has several areas that could be improved for better maintainability and performance:
- Components are tightly coupled with significant prop drilling
- Complex state management spread across context and local state
- Mixed responsibilities in components
Consider these architectural improvements:
- Split into smaller, more focused components
- Create custom hooks for shared logic
- Use composition instead of prop drilling
- Implement proper memoization for expensive computations
Would you like me to provide specific examples of how to implement these improvements?
📜 Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
⛔ Files ignored due to path filters (1)
health/micro-ui/web/micro-ui-internals/packages/css/package.jsonis excluded by!**/*.json
📒 Files selected for processing (6)
health/micro-ui/web/micro-ui-internals/example/public/index.html(1 hunks)health/micro-ui/web/micro-ui-internals/packages/css/src/pages/employee/campaignCycle.scss(1 hunks)health/micro-ui/web/micro-ui-internals/packages/css/src/pages/employee/index.scss(1 hunks)health/micro-ui/web/micro-ui-internals/packages/modules/campaign-manager/src/components/CampaignSummary.js(1 hunks)health/micro-ui/web/micro-ui-internals/packages/modules/campaign-manager/src/pages/employee/deliveryRule/AddDeliverycontext.js(1 hunks)health/micro-ui/web/public/index.html(1 hunks)
🧰 Additional context used
📓 Path-based instructions (2)
health/micro-ui/web/micro-ui-internals/packages/modules/campaign-manager/src/components/CampaignSummary.js (1)
Pattern **/*.js: check
health/micro-ui/web/micro-ui-internals/packages/modules/campaign-manager/src/pages/employee/deliveryRule/AddDeliverycontext.js (1)
Pattern **/*.js: check
🪛 Biome (1.9.4)
health/micro-ui/web/micro-ui-internals/packages/modules/campaign-manager/src/components/CampaignSummary.js
[error] 202-202: This let declares a variable that is only assigned once.
'currentCycleIndex' is never reassigned.
Safe fix: Use const instead.
(lint/style/useConst)
[error] 203-203: This let declares a variable that is only assigned once.
'currentCycle' is never reassigned.
Safe fix: Use const instead.
(lint/style/useConst)
[error] 228-228: This let declares a variable that is only assigned once.
'attributes' is never reassigned.
Safe fix: Use const instead.
(lint/style/useConst)
🔇 Additional comments (4)
health/micro-ui/web/micro-ui-internals/example/public/index.html (1)
15-15: Verify version synchronization across the application
The CSS version has been updated to 0.1.53. Let's ensure this version is consistent across all relevant files.
✅ Verification successful
Version synchronization verified across main and example files
The version 0.1.53 is correctly synchronized between:
health/micro-ui/web/public/index.html(main file)health/micro-ui/web/micro-ui-internals/example/public/index.html(example file)
Note: The version 0.1.0 found in README.md is expected as it's typically used for documentation purposes with a basic example.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Check for any mismatched versions of digit-ui-health-css across the codebase
# Expected: All files should reference version 0.1.53
rg -l "digit-ui-health-css@" | xargs rg "digit-ui-health-css@[0-9]+\.[0-9]+\.[0-9]+"
Length of output: 575
health/micro-ui/web/micro-ui-internals/packages/css/src/pages/employee/campaignCycle.scss (1)
289-289: LGTM! Verify the visual spacing.
The addition of margin-bottom: 1rem to .add-rule-btn is a reasonable styling fix that provides consistent spacing below rule buttons.
Please verify that this spacing adjustment:
- Properly aligns with the product chips in the UI
- Maintains consistent spacing with other similar button elements
health/micro-ui/web/micro-ui-internals/packages/css/src/pages/employee/index.scss (1)
313-313: LGTM! Enhanced user interaction feedback
The addition of cursor: pointer to .campaign-preview-edit-container improves user experience by providing visual feedback for the interactive edit container.
health/micro-ui/web/micro-ui-internals/packages/modules/campaign-manager/src/pages/employee/deliveryRule/AddDeliverycontext.js (1)
Line range hint 831-835: LGTM: Improved product chip validation
The additional check for both value and quantity properties ensures that product chips are only rendered when all required data is present.
| <link rel="stylesheet" href="https://unpkg.com/@egovernments/[email protected]/dist/index.css" /> | ||
| <link rel="stylesheet" href="https://unpkg.com/@egovernments/[email protected]/dist/index.css" /> | ||
| <link rel="stylesheet" href="https://unpkg.com/@egovernments/[email protected].52/dist/index.css" /> | ||
| <link rel="stylesheet" href="https://unpkg.com/@egovernments/[email protected].53/dist/index.css" /> |
There was a problem hiding this comment.
🧹 Nitpick (assertive)
Consider adding integrity hashes for enhanced security
The CSS resources are loaded from unpkg.com without integrity hashes. Consider adding integrity checks to prevent supply chain attacks.
Example format:
- <link rel="stylesheet" href="https://unpkg.com/@egovernments/[email protected]/dist/index.css" />
+ <link rel="stylesheet" href="https://unpkg.com/@egovernments/[email protected]/dist/index.css"
+ integrity_no="sha384-[hash]"
+ crossorigin="anonymous" />Committable suggestion skipped: line range outside the PR's diff.
| @@ -10,7 +10,7 @@ | |||
| <link rel="stylesheet" href="https://unpkg.com/@egovernments/[email protected]/dist/index.css" /> | |||
| <link rel="stylesheet" href="https://unpkg.com/@egovernments/[email protected]/dist/index.css" /> | |||
| <!-- added below css for hcm-workbench module inclusion--> | |||
| <link rel="stylesheet" href="https://unpkg.com/@egovernments/[email protected].52/dist/index.css" /> | |||
| <link rel="stylesheet" href="https://unpkg.com/@egovernments/[email protected].53/dist/index.css" /> | |||
There was a problem hiding this comment.
🧹 Nitpick (assertive)
Consider documenting the CSS changes.
To help with future maintenance, consider documenting the specific product chip and summary issues that were fixed in this CSS version update.
💡 Codebase verification
Package version 0.1.53 is not accessible on unpkg CDN
The specified CSS package version @egovernments/[email protected] returns a 404 error, indicating that this version has not been published or is not accessible through the unpkg CDN. This needs to be addressed before merging the PR.
- Ensure the package
@egovernments/digit-ui-health-cssversion 0.1.53 is published to npm - Consider reverting to the previous working version (0.1.52) until the new version is published
🔗 Analysis chain
LGTM! Verify the package accessibility.
The CSS version update appears to be related to fixing product chip and summary issues.
Let's verify that the updated package exists and is accessible:
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Verify the health CSS package version exists and is accessible
# Expected: HTTP 200 status code if the package exists and is accessible
curl -I "https://unpkg.com/@egovernments/[email protected]/dist/index.css"
Length of output: 889
| const parseConditionAndCreateRules = (condition, ruleKey, products) => { | ||
| // const conditionParts = condition.split("and").map((part) => part.trim()); | ||
| // let attributes = []; | ||
| let attributes = []; | ||
|
|
||
| if (isPreview) { | ||
| // Handle preview condition `3<=ageandage<11` or `3 <= Age < 11` in "IN_BETWEEN" style | ||
| const inBetweenMatch = condition.match(/(\d+)(<=|<|>=|>)(\w+)and(\w+)(<=|<|>=|>)(\d+)/); | ||
| if (inBetweenMatch) { | ||
| const toValue = inBetweenMatch[1].trim(); | ||
| const fromValue = inBetweenMatch[6].trim(); | ||
| const attributeCode = inBetweenMatch[3].trim(); | ||
|
|
||
| attributes.push({ | ||
| key: attributes.length + 1, | ||
| operator: { code: "IN_BETWEEN" }, | ||
| attribute: { code: attributeCode }, | ||
| fromValue, | ||
| toValue, | ||
| }); | ||
| } else { | ||
| // Handle regular conditions in preview mode | ||
| const conditionParts = condition.split("and").map((part) => part.trim()); | ||
| conditionParts.forEach((part) => { | ||
| const match = part.match(/(.*?)\s*(<=|>=|<|>|==|!=)\s*(.*)/); | ||
| if (match) { | ||
| const attributeCode = match[1].trim(); | ||
| const operatorSymbol = match[2].trim(); | ||
| const value = match[3].trim(); | ||
| attributes.push({ | ||
| key: attributes.length + 1, | ||
| value, | ||
| operator: { code: operatorMapping[operatorSymbol] }, | ||
| attribute: { code: attributeCode }, | ||
| }); | ||
| } | ||
| }); | ||
| } | ||
| } else { | ||
| const conditionParts = condition.split("and").map((part) => part.trim()); | ||
| conditionParts.forEach((part) => { | ||
| const parts = part.split(" ").filter(Boolean); | ||
|
|
||
| // Handle "IN_BETWEEN" operator | ||
| if (parts.length === 5 && (parts[1] === "<=" || parts[1] === "<") && (parts[3] === "<" || parts[3] === "<=")) { | ||
| const toValue = parts[0]; | ||
| const fromValue = parts[4]; | ||
| attributes.push({ | ||
| key: attributes.length + 1, | ||
| operator: { code: operatorMapping["IN_BETWEEN"] }, | ||
| attribute: { code: parts[2] }, | ||
| fromValue, | ||
| toValue, | ||
| }); | ||
| } else { | ||
| const match = part.match(/(.*?)\s*(<=|>=|<|>|==|!=)\s*(.*)/); | ||
| if (match) { | ||
| const attributeCode = match[1].trim(); | ||
| const operatorSymbol = match[2].trim(); | ||
| const value = match[3].trim(); | ||
| attributes.push({ | ||
| key: attributes.length + 1, | ||
| value, | ||
| operator: { code: operatorMapping[operatorSymbol] }, | ||
| attribute: { code: attributeCode }, | ||
| }); | ||
| } | ||
| } | ||
| }); | ||
| } | ||
| return [{ | ||
| ruleKey: ruleKey + 1, | ||
| delivery: {}, | ||
| products, | ||
| attributes, | ||
| }]; | ||
| }; | ||
| const mapDoseCriteriaToDeliveryRules = (doseCriteria) => { | ||
| return doseCriteria?.flatMap((criteria, ruleKey) => { | ||
| const products = mapProductVariants(criteria.ProductVariants); | ||
| return parseConditionAndCreateRules(criteria.condition, ruleKey, products); | ||
| }); | ||
| }; | ||
|
|
||
| const mapDeliveries = (deliveries) => { | ||
| return deliveries?.map((delivery, deliveryIndex) => ({ | ||
| active: deliveryIndex === 0, | ||
| deliveryIndex: String(deliveryIndex + 1), | ||
| deliveryRules: mapDoseCriteriaToDeliveryRules(delivery.doseCriteria), | ||
| })); | ||
| }; | ||
|
|
||
| const transformedCycles = cycles.map((cycle) => ({ | ||
| active: true, | ||
| cycleIndex: String(cycle.id), | ||
| deliveries: mapDeliveries(cycle.deliveries), | ||
| })); | ||
|
|
||
| return transformedCycles; | ||
| } |
There was a problem hiding this comment.
🧹 Nitpick (assertive)
Consider refactoring reverseDeliveryRemap into smaller functions
The reverseDeliveryRemap function is quite lengthy and contains complex nested logic. Refactoring it into smaller, modular functions can improve readability, maintainability, and facilitate easier testing.
🧰 Tools
🪛 Biome (1.9.4)
[error] 202-202: This let declares a variable that is only assigned once.
'currentCycleIndex' is never reassigned.
Safe fix: Use const instead.
(lint/style/useConst)
[error] 203-203: This let declares a variable that is only assigned once.
'currentCycle' is never reassigned.
Safe fix: Use const instead.
(lint/style/useConst)
[error] 228-228: This let declares a variable that is only assigned once.
'attributes' is never reassigned.
Safe fix: Use const instead.
(lint/style/useConst)
| let currentCycleIndex = null; | ||
| let currentCycle = null; |
There was a problem hiding this comment.
🧹 Nitpick (assertive)
Use const instead of let for variables that are not reassigned
The variables currentCycleIndex and currentCycle are declared with let but never reassigned after their initial assignment. Declaring them with const improves code readability and clearly indicates that these variables are constants.
Suggested fix:
const reversedData = [];
- let currentCycleIndex = null;
- let currentCycle = null;
+ const currentCycleIndex = null;
+ const currentCycle = null;📝 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.
| let currentCycleIndex = null; | |
| let currentCycle = null; | |
| const currentCycleIndex = null; | |
| const currentCycle = null; |
🧰 Tools
🪛 Biome (1.9.4)
[error] 202-202: This let declares a variable that is only assigned once.
'currentCycleIndex' is never reassigned.
Safe fix: Use const instead.
(lint/style/useConst)
[error] 203-203: This let declares a variable that is only assigned once.
'currentCycle' is never reassigned.
Safe fix: Use const instead.
(lint/style/useConst)
| const parseConditionAndCreateRules = (condition, ruleKey, products) => { | ||
| // const conditionParts = condition.split("and").map((part) => part.trim()); | ||
| // let attributes = []; | ||
| let attributes = []; |
There was a problem hiding this comment.
🧹 Nitpick (assertive)
Use const instead of let for attributes since it's not reassigned
The variable attributes is initialized but not reassigned. Using const is preferred for variables that remain constant to enhance code clarity.
Suggested fix:
- let attributes = [];
+ const attributes = [];📝 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.
| let attributes = []; | |
| const attributes = []; |
🧰 Tools
🪛 Biome (1.9.4)
[error] 228-228: This let declares a variable that is only assigned once.
'attributes' is never reassigned.
Safe fix: Use const instead.
(lint/style/useConst)
Summary by CodeRabbit
New Features
Bug Fixes
Documentation