Finalise Action for Facility Catchment + Validations #1642
Conversation
📝 Walkthrough📝 WalkthroughWalkthroughThis pull request introduces several modifications across multiple components within the microplan module. Key changes include enhancements to error handling and user feedback mechanisms in the 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: 20
🧹 Outside diff range comments (6)
health/micro-ui/web/micro-ui-internals/packages/modules/microplan/src/components/accessbilityPopUP.js (1)
Line range hint
17-23: Improve null safety with optional chaining.The access to nested properties could be safer with optional chaining.
- if (census?.additionalDetails?.accessibilityDetails) { - setDropdown1Value(census.additionalDetails.accessibilityDetails.roadCondition); - setDropdown2Value(census.additionalDetails.accessibilityDetails.terrain); - setDropdown3Value(census.additionalDetails.accessibilityDetails.transportationMode); + if (census?.additionalDetails?.accessibilityDetails) { + setDropdown1Value(census?.additionalDetails?.accessibilityDetails?.roadCondition); + setDropdown2Value(census?.additionalDetails?.accessibilityDetails?.terrain); + setDropdown3Value(census?.additionalDetails?.accessibilityDetails?.transportationMode); }health/micro-ui/web/micro-ui-internals/packages/modules/microplan/src/components/editVillagePopulationPopUP.js (1)
Line range hint
9-28: Add validation for population input values.The population values are stored as strings without validation. Consider:
- Adding numeric validation
- Preventing negative values
- Ensuring the confirmed population doesn't exceed reasonable limits
const [confirmedTotalPopulation, setConfirmedTotalPopulation] = useState(""); const [confirmedTargetPopulation, setConfirmedTargetPopulation] = useState(""); +const [errors, setErrors] = useState({}); + +const validatePopulation = (value, field) => { + const num = Number(value); + if (isNaN(num)) return `${field} must be a number`; + if (num < 0) return `${field} cannot be negative`; + if (num > 1000000000) return `${field} exceeds maximum limit`; + return null; +}; + +const handlePopulationChange = (value, field) => { + const error = validatePopulation(value, field); + setErrors(prev => ({ ...prev, [field]: error })); + if (field === 'totalPopulation') setConfirmedTotalPopulation(value); + if (field === 'targetPopulation') setConfirmedTargetPopulation(value); +};health/micro-ui/web/micro-ui-internals/packages/modules/microplan/src/components/WorkflowCommentPopUp.js (2)
Line range hint
13-83: Consider adding unit tests for the payload transformation logic.The
setCommentInPayloadForListfunction handles complex nested object transformations. To ensure reliability and catch potential edge cases, consider adding unit tests for this function.Would you like me to help generate unit test cases for the payload transformation scenarios?
🧰 Tools
🪛 Biome
[error] 113-113: Change to an optional chain.
Unsafe fix: Change to an optional chain.
(lint/complexity/useOptionalChain)
Line range hint
1-7: Add PropTypes validation for component props.The component accepts several important props (
onClose,heading,submitLabel, etc.) but lacks PropTypes validation. Consider adding proper prop validation to improve component reliability and documentation.Example implementation:
+import PropTypes from 'prop-types'; const WorkflowCommentPopUp = ({ onClose, heading, submitLabel, url, requestPayload, commentPath, onSuccess, onError }) => { // ... component implementation ... }; + +WorkflowCommentPopUp.propTypes = { + onClose: PropTypes.func.isRequired, + heading: PropTypes.string.isRequired, + submitLabel: PropTypes.string.isRequired, + url: PropTypes.string.isRequired, + requestPayload: PropTypes.object.isRequired, + commentPath: PropTypes.string.isRequired, + onSuccess: PropTypes.func, + onError: PropTypes.func +};🧰 Tools
🪛 Biome
[error] 113-113: Change to an optional chain.
Unsafe fix: Change to an optional chain.
(lint/complexity/useOptionalChain)
health/micro-ui/web/micro-ui-internals/packages/modules/microplan/src/pages/employee/index.js (1)
Line range hint
198-203: Remove extra empty lines.There are multiple consecutive empty lines after the route definition. Consider keeping just one empty line for better code organization.
Apply this diff:
<PrivateRoute path={`${path}/village-finalise-success`} component={() => <Response />} /> - - -health/micro-ui/web/micro-ui-internals/packages/modules/microplan/src/pages/employee/PopInbox.js (1)
Line range hint
169-170: Document the purpose of filtering EDIT actions.Add a comment explaining why EDIT actions are being filtered out from the workflow actions. This will help future maintainers understand the business logic behind this decision.
+ // Filter out EDIT actions as they are not applicable in the current workflow state const actionsToHide = actionsMain?.filter(action => action.action.includes("EDIT"))?.map(action => action.action);🧰 Tools
🪛 Biome
[error] 175-175: 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)
📜 Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
📒 Files selected for processing (7)
- health/micro-ui/web/micro-ui-internals/packages/modules/microplan/src/components/WorkflowCommentPopUp.js (1 hunks)
- health/micro-ui/web/micro-ui-internals/packages/modules/microplan/src/components/accessbilityPopUP.js (3 hunks)
- health/micro-ui/web/micro-ui-internals/packages/modules/microplan/src/components/editVillagePopulationPopUP.js (3 hunks)
- health/micro-ui/web/micro-ui-internals/packages/modules/microplan/src/components/securityPopUp.js (4 hunks)
- health/micro-ui/web/micro-ui-internals/packages/modules/microplan/src/pages/employee/FacilityCatchmentMapping.js (1 hunks)
- health/micro-ui/web/micro-ui-internals/packages/modules/microplan/src/pages/employee/PopInbox.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 (7)
health/micro-ui/web/micro-ui-internals/packages/modules/microplan/src/components/WorkflowCommentPopUp.js (1)
Pattern
**/*.js: checkhealth/micro-ui/web/micro-ui-internals/packages/modules/microplan/src/components/accessbilityPopUP.js (1)
Pattern
**/*.js: checkhealth/micro-ui/web/micro-ui-internals/packages/modules/microplan/src/components/editVillagePopulationPopUP.js (1)
Pattern
**/*.js: checkhealth/micro-ui/web/micro-ui-internals/packages/modules/microplan/src/components/securityPopUp.js (1)
Pattern
**/*.js: checkhealth/micro-ui/web/micro-ui-internals/packages/modules/microplan/src/pages/employee/FacilityCatchmentMapping.js (1)
Pattern
**/*.js: checkhealth/micro-ui/web/micro-ui-internals/packages/modules/microplan/src/pages/employee/PopInbox.js (1)
Pattern
**/*.js: checkhealth/micro-ui/web/micro-ui-internals/packages/modules/microplan/src/pages/employee/index.js (1)
Pattern
**/*.js: check
🪛 Biome
health/micro-ui/web/micro-ui-internals/packages/modules/microplan/src/components/accessbilityPopUP.js
[error] 76-76: Change to an optional chain.
Unsafe fix: Change to an optional chain.
(lint/complexity/useOptionalChain)
[error] 91-91: Avoid passing children using a prop
The canonical way to pass children in React is to use JSX elements
(lint/correctness/noChildrenProp)
[error] 129-136: 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)
[error] 137-145: 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)
health/micro-ui/web/micro-ui-internals/packages/modules/microplan/src/components/editVillagePopulationPopUP.js
[error] 66-66: Change to an optional chain.
Unsafe fix: Change to an optional chain.
(lint/complexity/useOptionalChain)
[error] 81-81: Avoid passing children using a prop
The canonical way to pass children in React is to use JSX elements
(lint/correctness/noChildrenProp)
[error] 125-132: 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)
[error] 133-141: 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)
health/micro-ui/web/micro-ui-internals/packages/modules/microplan/src/components/securityPopUp.js
[error] 62-62: Change to an optional chain.
Unsafe fix: Change to an optional chain.
(lint/complexity/useOptionalChain)
[error] 79-79: Avoid passing children using a prop
The canonical way to pass children in React is to use JSX elements
(lint/correctness/noChildrenProp)
health/micro-ui/web/micro-ui-internals/packages/modules/microplan/src/pages/employee/FacilityCatchmentMapping.js
[error] 56-56: 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)
[error] 104-106: 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] 112-112: 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)
health/micro-ui/web/micro-ui-internals/packages/modules/microplan/src/pages/employee/PopInbox.js
[error] 175-175: 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)
🔇 Additional comments (4)
health/micro-ui/web/micro-ui-internals/packages/modules/microplan/src/components/securityPopUp.js (2)
1-6: LGTM: Clean imports and proper prop additionsThe component signature and imports have been updated appropriately to support the new error handling functionality.
74-146: LGTM: Well-structured UI with proper error handlingThe PopUp and Toast components are well integrated with appropriate positioning and state management. The button states are handled correctly during loading.
🧰 Tools
🪛 Biome
[error] 79-79: Avoid passing children using a prop
The canonical way to pass children in React is to use JSX elements
(lint/correctness/noChildrenProp)
health/micro-ui/web/micro-ui-internals/packages/modules/microplan/src/pages/employee/index.js (1)
198-198: Verify the navigation to this success route.The new route for handling village finalization success looks good. However, ensure that components triggering this route (likely the village finalization form/action) correctly navigate to this path.
Let's verify the usage of this route:
✅ Verification successful
The search results show that the
village-finalise-successroute is properly integrated with the system. It's being navigated to from theFacilityCatchmentMappingcomponent usinghistory.push()with appropriate success message and navigation parameters.Let me verify the Response component usage to ensure it can handle these parameters:
Let me check the implementation of the Response component specifically in the microplan module:
Based on the verification results, I can now provide the final response:
The success route is properly integrated with the Response component
The Response component is correctly implemented to handle the success state passed from the FacilityCatchmentMapping component. It:
- Receives state via useLocation() hook
- Renders success message and description from the state
- Provides proper back navigation with configurable back link and label
- Uses PanelCard to show success status with proper styling
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Find components that might navigate to this success route # Test: Search for navigation or redirect logic to village-finalise-success rg -A 3 "village-finalise-success" --type jsLength of output: 1254
Script:
#!/bin/bash # Check the Response component implementation ast-grep --pattern 'const Response = $_' -l jsLength of output: 85715
Script:
#!/bin/bash # Find the Response component implementation in the microplan module cat health/micro-ui/web/micro-ui-internals/packages/modules/microplan/src/components/Response.jsLength of output: 2490
health/micro-ui/web/micro-ui-internals/packages/modules/microplan/src/pages/employee/PopInbox.js (1)
Line range hint
392-399: Address TODO comment and hardcoded values in success navigation.The success navigation contains a TODO comment and hardcoded values that need attention:
- The filename is marked as needing an update when the API is successful
- The success message and navigation paths are hardcoded
Let's verify if these values should be dynamic:
Would you like help implementing a more dynamic success navigation that derives values from the API response?
🧰 Tools
🪛 Biome
[error] 175-175: 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)
...h/micro-ui/web/micro-ui-internals/packages/modules/microplan/src/components/securityPopUp.js
Show resolved
Hide resolved
...h/micro-ui/web/micro-ui-internals/packages/modules/microplan/src/components/securityPopUp.js
Show resolved
Hide resolved
...cro-ui/web/micro-ui-internals/packages/modules/microplan/src/components/accessbilityPopUP.js
Show resolved
Hide resolved
...cro-ui/web/micro-ui-internals/packages/modules/microplan/src/components/accessbilityPopUP.js
Show resolved
Hide resolved
...cro-ui/web/micro-ui-internals/packages/modules/microplan/src/components/accessbilityPopUP.js
Show resolved
Hide resolved
...micro-ui-internals/packages/modules/microplan/src/pages/employee/FacilityCatchmentMapping.js
Show resolved
Hide resolved
...micro-ui-internals/packages/modules/microplan/src/pages/employee/FacilityCatchmentMapping.js
Show resolved
Hide resolved
...micro-ui-internals/packages/modules/microplan/src/pages/employee/FacilityCatchmentMapping.js
Show resolved
Hide resolved
...micro-ui-internals/packages/modules/microplan/src/pages/employee/FacilityCatchmentMapping.js
Show resolved
Hide resolved
...micro-ui-internals/packages/modules/microplan/src/pages/employee/FacilityCatchmentMapping.js
Show resolved
Hide resolved
...micro-ui-internals/packages/modules/microplan/src/pages/employee/FacilityCatchmentMapping.js
Outdated
Show resolved
Hide resolved
...micro-ui-internals/packages/modules/microplan/src/pages/employee/FacilityCatchmentMapping.js
Outdated
Show resolved
Hide resolved
...micro-ui-internals/packages/modules/microplan/src/pages/employee/FacilityCatchmentMapping.js
Outdated
Show resolved
Hide resolved
Summary by CodeRabbit
Release Notes
New Features
WorkflowCommentPopUp,AccessibilityPopUp,EditVillagePopulationPopUp, andSecurityPopUp, with dynamic toast notifications for better error reporting.Bug Fixes
PopInboxcomponent by refining conditions for API calls to prevent potential runtime errors.Improvements
FacilityCatchmentMappingcomponent, enhancing user interaction based on permissions.