BUGFIX/HCMPRE-2243 : Showing only supported campaign types and minor fixes #2187
Conversation
…lan ui (#2185) * BUGFIX/HCMPRE-2243 : Showing only supported campaign types for microplan * Added review changes * Updated to hcm-microplanning module * Fixed typo
📝 WalkthroughWalkthroughThis pull request updates the campaign filtering logic in the CampaignDetails component and enhances data retrieval in the employee page. In CampaignDetails.js, the new variable Changes
Possibly related PRs
Poem
Tip 🌐 Web search-backed reviews and chat
✨ Finishing Touches
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: 1
📜 Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (2)
health/micro-ui/web/micro-ui-internals/packages/modules/microplan/src/components/CampaignDetails.js(2 hunks)health/micro-ui/web/micro-ui-internals/packages/modules/microplan/src/pages/employee/index.js(1 hunks)
🧰 Additional context used
📓 Path-based instructions (2)
health/micro-ui/web/micro-ui-internals/packages/modules/microplan/src/components/CampaignDetails.js (1)
Pattern **/*.js: check
health/micro-ui/web/micro-ui-internals/packages/modules/microplan/src/pages/employee/index.js (1)
Pattern **/*.js: check
⏰ Context from checks skipped due to timeout of 10000ms (2)
- GitHub Check: Analyze (javascript-typescript)
- GitHub Check: Analyze (actions)
🔇 Additional comments (2)
health/micro-ui/web/micro-ui-internals/packages/modules/microplan/src/components/CampaignDetails.js (1)
60-60: LGTM! Safely extracts campaign type codes.The implementation correctly uses optional chaining and provides a fallback empty array, ensuring type safety.
health/micro-ui/web/micro-ui-internals/packages/modules/microplan/src/pages/employee/index.js (1)
142-142: LGTM! Added MDMS data retrieval for campaign types.The implementation correctly integrates with the existing MDMS data fetching pattern and state management.
...micro-ui/web/micro-ui-internals/packages/modules/microplan/src/components/CampaignDetails.js
Show resolved
Hide resolved
BUGFIX/HCMPRE-2250 : Added loader while data is loading for the Boundary KPI
Swathi-eGov
dismissed stale reviews from nabeelmd-eGov, coderabbitai[bot], and themself
via
168c43f
nipunarora-eGov
changed the title
There was a problem hiding this comment.
Actionable comments posted: 7
🔭 Outside diff range comments (4)
health/micro-ui/web/micro-ui-internals/packages/modules/microplan/src/utils/utilities.js (4)
250-256: Remove commented-out code.Remove the commented-out version of
updateUrlParamsas it's no longer needed and the new implementation is already in place.
293-300: Complete the implementation ofmergeAssumptions.The function contains only comments describing what needs to be done but lacks implementation.
Would you like me to help implement this function based on the provided comments?
392-394: Complete the implementation offetchBoundaryOptions.The function is declared but not implemented.
Would you like me to help implement this function based on its parameters and usage context?
513-527: Add JSDoc documentation for exported functions.The exported utility functions lack documentation describing their purpose, parameters, and return values. This documentation would improve maintainability and help other developers understand how to use these functions correctly.
Example for one function:
/** * Creates a status map counting occurrences of each type in the data array. * @param {Array} data - Array of objects containing type information * @param {Array} boundaryHierarchy - Array of boundary objects with boundaryType * @returns {Object} Map of types to their counts */ function createStatusMap(data, boundaryHierarchy) { // ... implementation }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Plan: Pro
⛔ 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 (8)
health/micro-ui/web/micro-ui-internals/example/public/index.html(1 hunks)health/micro-ui/web/micro-ui-internals/packages/css/src/components/microplan.scss(1 hunks)health/micro-ui/web/micro-ui-internals/packages/modules/microplan/src/components/BoundaryKpi.js(1 hunks)health/micro-ui/web/micro-ui-internals/packages/modules/microplan/src/components/BoundarySelection.js(4 hunks)health/micro-ui/web/micro-ui-internals/packages/modules/microplan/src/components/InboxFilterWrapper.js(2 hunks)health/micro-ui/web/micro-ui-internals/packages/modules/microplan/src/pages/employee/PlanInbox.js(3 hunks)health/micro-ui/web/micro-ui-internals/packages/modules/microplan/src/utils/utilities.js(1 hunks)health/micro-ui/web/public/index.html(1 hunks)
🧰 Additional context used
📓 Path-based instructions (1)
`**/*.js`: check
**/*.js: check
health/micro-ui/web/micro-ui-internals/packages/modules/microplan/src/components/BoundaryKpi.jshealth/micro-ui/web/micro-ui-internals/packages/modules/microplan/src/utils/utilities.jshealth/micro-ui/web/micro-ui-internals/packages/modules/microplan/src/components/BoundarySelection.jshealth/micro-ui/web/micro-ui-internals/packages/modules/microplan/src/components/InboxFilterWrapper.jshealth/micro-ui/web/micro-ui-internals/packages/modules/microplan/src/pages/employee/PlanInbox.js
📓 Learnings (2)
health/micro-ui/web/micro-ui-internals/packages/modules/microplan/src/components/InboxFilterWrapper.js (1)
Learnt from: Ramkrishna-egov
PR: egovernments/DIGIT-Frontend#1562
File: health/micro-ui/web/micro-ui-internals/packages/modules/microplan/src/components/InboxFilterWrapper.js:57-57
Timestamp: 2024-11-12T10:40:17.779Z
Learning: In the `InboxFilterWrapper` component, `optionsKey` should always be set to `"name"` because options are transformed and created within the component.
health/micro-ui/web/micro-ui-internals/packages/modules/microplan/src/pages/employee/PlanInbox.js (1)
Learnt from: rachana-egov
PR: egovernments/DIGIT-Frontend#1847
File: health/micro-ui/web/micro-ui-internals/packages/modules/microplan/src/pages/employee/PlanInbox.js:308-314
Timestamp: 2024-11-18T04:35:51.535Z
Learning: In `PlanInbox.js`, the variable `planWithCensus?.StatusCount[selectedFilter]` is always defined in the given context, so null checks are unnecessary.
🪛 Biome (1.9.4)
health/micro-ui/web/micro-ui-internals/packages/modules/microplan/src/components/BoundarySelection.js
[error] 117-149: JSX elements without children should be marked as self-closing. In JSX, it is valid for any element to be self-closing.
Unsafe fix: Use a SelfClosingElement instead
(lint/style/useSelfClosingElements)
[error] 135-146: Missing key property for this element in iterable.
The order of the items may change, and having a key can help React identify which item was moved.
Check the React documentation.
(lint/correctness/useJsxKeyInIterable)
⏰ Context from checks skipped due to timeout of 10000ms (2)
- GitHub Check: Analyze (javascript-typescript)
- GitHub Check: Analyze (actions)
🔇 Additional comments (11)
health/micro-ui/web/micro-ui-internals/example/public/index.html (1)
15-15: CSS Stylesheet Version Update ConfirmedThe link for the Digit UI Health CSS has been successfully updated from an older version (likely 0.2.44) to version 0.2.45. This change ensures that any fixes or improvements in the new CSS release are applied across the UI. Please verify in a staging environment that the new stylesheet does not inadvertently override any custom styling.
health/micro-ui/web/public/index.html (1)
13-13: Consistent CSS Version UpdateThe updated stylesheet link now points to Digit UI Health CSS version 0.2.45, mirroring the change made in the internals example. This consistency across files is crucial for uniform styling and behavior. Ensure the new version is compatible with all dependent components and that any custom modifications (if any) are still applied correctly.
health/micro-ui/web/micro-ui-internals/packages/modules/microplan/src/components/BoundaryKpi.js (1)
3-3: LGTM!The Loader import is correctly added from the shared component library.
health/micro-ui/web/micro-ui-internals/packages/modules/microplan/src/pages/employee/PlanInbox.js (3)
67-70: LGTM! Good practice using a descriptive variable name and clear role check.The
isRootApprovercheck is well-implemented using the optional chaining operator andincludes()method.
882-884: LGTM! CSS class name matches the new styles.The class name
planInbox-filtercard-table-wrapperaligns with the new styles inmicroplan.scss.
98-103: Verify the workflow behavior for non-root approvers.The conditional spread operator usage for the
assigneefield could affect workflow routing. Please ensure this change doesn't bypass any required approvals.✅ Verification successful
🏁 Script executed:
#!/bin/bash # Description: Check workflow configuration and role assignments # Search for workflow configuration rg -A 10 "PLAN_ESTIMATION" # Search for role assignments in the workflow ast-grep --pattern 'roles: [$_]'Length of output: 16514
Workflow Routing for Non-root Approvers is Working as Intended
The investigation confirms that non-root approvers correctly receive the
assigneefield (set touser.info.uuid). This aligns with the configured workflow logic that uses distinct roles ("PLAN_ESTIMATION_APPROVER" vs. "ROOT_PLAN_ESTIMATION_APPROVER") to control task routing. In contrast, users with the root role bypass this criteria, as intended.
- Non-root users: The conditional spread operator adds
{ assignee: user.info.uuid }, effectively filtering tasks.- Root approvers: The check (
isRootApprover) prevents the filter, ensuring they can access broader tasks.Reviewing the workflow configuration and role checks in the code confirms that this change does not bypass any required approvals.
health/micro-ui/web/micro-ui-internals/packages/css/src/components/microplan.scss (1)
552-562: LGTM! Well-structured CSS using BEM methodology.The new classes follow good CSS practices:
- Uses BEM naming convention for better maintainability
- Properly handles flex layout with
align-items: stretch- Ensures content container grows with
flex: 1health/micro-ui/web/micro-ui-internals/packages/modules/microplan/src/components/InboxFilterWrapper.js (3)
172-172: LGTM! Correct usage of optionsKey="name".The code correctly follows the established pattern of using
optionsKey="name"for both RadioButtons and MultiSelectDropdown components.Also applies to: 217-217
164-164: Verify the CSS class definition for "plan-inbox-filtercard".The addition of class-based styling is good for maintainability. However, ensure that the "plan-inbox-filtercard" class is properly defined in your CSS files.
Run this script to locate the CSS definition:
✅ Verification successful
🏁 Script executed:
#!/bin/bash # Description: Search for the CSS class definition rg -g '*.css' -g '*.scss' 'plan-inbox-filtercard'Length of output: 158
CSS class definition is correctly implemented.
The class "plan-inbox-filtercard" is defined in the filehealth/micro-ui/web/micro-ui-internals/packages/css/src/components/microplan.scss, confirming that the styling change inInboxFilterWrapper.jsis supported as intended.
166-166: Good improvement moving from inline styles to class-based styling.The change from inline styles to class-based styling improves maintainability.
Run this script to locate the CSS definition:
✅ Verification successful
🏁 Script executed:
#!/bin/bash # Description: Search for the CSS class definition rg -g '*.css' -g '*.scss' 'gap-between-dropdowns'Length of output: 155
Change Verified: CSS class is defined as expected
The inline style was replaced correctly with the class "gap-between-dropdowns." I verified that the CSS definition exists in the file
health/micro-ui/web/micro-ui-internals/packages/css/src/components/microplan.scss. The update improves maintainability, and no issues were found.health/micro-ui/web/micro-ui-internals/packages/modules/microplan/src/components/BoundarySelection.js (1)
21-21: LGTM!The addition of
showPopupstate is appropriate for managing the popup's visibility.
health/micro-ui/web/micro-ui-internals/packages/modules/microplan/src/components/BoundaryKpi.js
Show resolved
Hide resolved
health/micro-ui/web/micro-ui-internals/packages/modules/microplan/src/components/BoundaryKpi.js
Show resolved
Hide resolved
health/micro-ui/web/micro-ui-internals/packages/modules/microplan/src/utils/utilities.js
Show resolved
Hide resolved
...cro-ui/web/micro-ui-internals/packages/modules/microplan/src/components/BoundarySelection.js
Show resolved
Hide resolved
...cro-ui/web/micro-ui-internals/packages/modules/microplan/src/components/BoundarySelection.js
Show resolved
Hide resolved
...cro-ui/web/micro-ui-internals/packages/modules/microplan/src/components/BoundarySelection.js
Show resolved
Hide resolved
...cro-ui/web/micro-ui-internals/packages/modules/microplan/src/components/BoundarySelection.js
Show resolved
Hide resolved
Summary by CodeRabbit
New Features
Bug Fixes
Chores