microplan quick fixes #1696
microplan quick fixes #1696
Conversation
|
Caution Review failedThe pull request is closed. 📝 WalkthroughWalkthroughThis pull request introduces various modifications across multiple components in the microplanning UI. Key changes include the addition of new styles in the SCSS file, updates 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 (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 19
🧹 Outside diff range comments (9)
health/micro-ui/web/micro-ui-internals/packages/modules/microplan/src/pages/employee/FacilityCatchmentMapping.js (2)
Line range hint
22-42: Add error handling for API failuresThe component fetches data using multiple hooks but doesn't handle error states (
planEmployeeError,errorPlan). Consider adding error boundaries or error messages for better user experience.Add error handling after the loading check:
if (isPlanEmpSearchLoading || isLoading || isLoadingPlanObject) return <Loader /> + if (planEmployeeError || errorPlan) { + return <Card> + <CardText>{t("ES_COMMON_ERROR_FETCHING_DATA")}</CardText> + </Card> + }
Line range hint
71-89: Document the workflow logicThe workflow state management and update logic is complex and would benefit from documentation explaining the business rules and state transitions.
Add JSDoc comments to explain the workflow:
+ /** + * Updates the workflow state for catchment mapping finalization + * @returns {Object} Updated plan configuration with workflow action + */ const updateWorkflowForFooterAction = () => {health/micro-ui/web/micro-ui-internals/packages/modules/microplan/src/components/UserAccess.js (1)
Line range hint
238-265: Consider adding ARIA attributes for better accessibilityWhile the UI structure is good, consider enhancing accessibility:
- Add aria-label to the search field
- Add role="status" to the loading state
- Add aria-live="polite" to the NoResultsFound component
health/micro-ui/web/micro-ui-internals/packages/modules/microplan/src/pages/employee/SetupMicroplan.js (1)
Line range hint
1-424: Consider breaking down the component for better maintainabilityThe
SetupMicroplancomponent has grown quite large and handles multiple responsibilities. Consider:
- Extracting form validation logic into a custom hook
- Moving stepper configuration to a separate component
- Creating a dedicated form submission handler component
This would improve:
- Code maintainability
- Testing capabilities
- Reusability of logic
health/micro-ui/web/micro-ui-internals/packages/modules/microplan/src/pages/employee/PopInbox.js (2)
Line range hint
1-545: Consider breaking down this component for better maintainability.The
PopInboxcomponent has grown quite large and handles multiple responsibilities including:
- Workflow state management
- Data fetching and transformation
- Complex UI rendering
- User role management
- Action handling
This could make it harder to maintain and test. Consider breaking it down into smaller, focused components:
- WorkflowActions component for handling workflow-related actions
- DataGrid component for the table and pagination
- FilterPanel component for search and filtering
Line range hint
498-502: Enhance error handling for navigation.The success callback in the WorkflowCommentPopUp doesn't handle potential navigation errors. Consider adding error boundaries or fallback navigation paths.
onSuccess={(data) => { history.push(`/${window.contextPath}/employee/microplan/population-finalise-success`, { fileName: data?.PlanConfiguration?.[0]?.name, message: t(`POPULATION_FINALISED_SUCCESSFUL`), back: t(`GO_BACK_TO_HOME`), backlink: `/${window.contextPath}/employee` + }).catch(error => { + setShowToast({ + key: "error", + label: t("NAVIGATION_ERROR"), + transitionTime: 5000 + }); }); }}health/micro-ui/web/micro-ui-internals/packages/modules/microplan/src/configs/UICustomizations.js (1)
Line range hint
99-104: Consider using route state instead of URL parameters.The current implementation uses URL parameters to pass both IDs and state. Consider using the router's state object for the
setup-completedflag while keeping only the essential identifiers in the URL. This would:
- Keep URLs cleaner and more semantic
- Prevent parameter manipulation
- Better align with React Router's best practices
- window.location.href = `/${window.contextPath}/employee/microplan/setup-microplan?key=${10}µplanId=${row.id}&campaignId=${ - row.CampaignDetails.id - }&setup-completed=${true}`; + const state = { setupCompleted: true }; + window.history.pushState( + { + microplanId: row.id, + campaignId: row.CampaignDetails.id, + setupCompleted: true + }, + "", + `/${window.contextPath}/employee/microplan/setup-microplan?key=${10}µplanId=${row.id}&campaignId=${row.CampaignDetails.id}` + ); + const navEvent = new PopStateEvent("popstate"); + window.dispatchEvent(navEvent);health/micro-ui/web/micro-ui-internals/packages/modules/microplan/src/pages/employee/PlanInbox.js (1)
Line range hint
553-555: Fix error handling in WorkflowCommentPopUp.The
onErrorcallback is trying to accesserror.response.data.Errorsbuterroris undefined in this context. It should use thedataparameter that's passed to the callback.Apply this fix:
- onError={(data) => { - setShowToast({ key: "error", label: t(error?.response?.data?.Errors?.[0]?.code) }); - }} + onError={(error) => { + setShowToast({ key: "error", label: t(error?.response?.data?.Errors?.[0]?.code || 'ERROR_UNKNOWN') }); + }}health/micro-ui/web/micro-ui-internals/packages/modules/microplan/src/components/UserUpload.js (1)
Line range hint
161-173: Improve error handling structure.The error handling logic is complex and could be simplified:
- Extract error parsing into a utility function:
const parseErrorMessage = (error) => { try { const parsedError = JSON.parse(error); return parsedError?.description || parsedError?.message || t("HCM_VALIDATION_FAILED"); } catch (e) { console.error("Error parsing JSON:", e); return t("HCM_VALIDATION_FAILED"); } };
- Simplify the error handling:
- if (temp?.status == "failed" && temp?.additionalDetails?.error) { - try { - const parsedError = JSON.parse(temp.additionalDetails.error); - const errorMessage = parsedError?.description || parsedError?.message || t("HCM_VALIDATION_FAILED"); - setShowToast({ key: "error", label: errorMessage, transitionTime: 5000000 }); - } catch (e) { - console.error("Error parsing JSON:", e); - setShowToast({ key: "error", label: t("HCM_VALIDATION_FAILED"), transitionTime: 5000000 }); - } - } else { - setShowToast({ key: "error", label: t("HCM_VALIDATION_FAILED"), transitionTime: 5000000 }); - } + const errorMessage = temp?.additionalDetails?.error + ? parseErrorMessage(temp.additionalDetails.error) + : t("HCM_VALIDATION_FAILED"); + setShowToast({ key: "error", label: errorMessage, transitionTime: 5000000 });
📜 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 (12)
- health/micro-ui/web/micro-ui-internals/example/public/index.html (1 hunks)
- health/micro-ui/web/micro-ui-internals/packages/css/src/components/microplanning.scss (1 hunks)
- health/micro-ui/web/micro-ui-internals/packages/modules/microplan/src/components/Response.js (1 hunks)
- health/micro-ui/web/micro-ui-internals/packages/modules/microplan/src/components/RoleTableComposer.js (5 hunks)
- health/micro-ui/web/micro-ui-internals/packages/modules/microplan/src/components/UserAccess.js (7 hunks)
- health/micro-ui/web/micro-ui-internals/packages/modules/microplan/src/components/UserUpload.js (12 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/pages/employee/FacilityCatchmentMapping.js (1 hunks)
- health/micro-ui/web/micro-ui-internals/packages/modules/microplan/src/pages/employee/PlanInbox.js (1 hunks)
- health/micro-ui/web/micro-ui-internals/packages/modules/microplan/src/pages/employee/PopInbox.js (1 hunks)
- health/micro-ui/web/micro-ui-internals/packages/modules/microplan/src/pages/employee/SetupMicroplan.js (2 hunks)
- health/micro-ui/web/public/index.html (1 hunks)
🧰 Additional context used
📓 Path-based instructions (9)
health/micro-ui/web/micro-ui-internals/packages/modules/microplan/src/components/Response.js (1)
Pattern
**/*.js: checkhealth/micro-ui/web/micro-ui-internals/packages/modules/microplan/src/components/RoleTableComposer.js (1)
Pattern
**/*.js: checkhealth/micro-ui/web/micro-ui-internals/packages/modules/microplan/src/components/UserAccess.js (1)
Pattern
**/*.js: checkhealth/micro-ui/web/micro-ui-internals/packages/modules/microplan/src/components/UserUpload.js (1)
Pattern
**/*.js: checkhealth/micro-ui/web/micro-ui-internals/packages/modules/microplan/src/configs/UICustomizations.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/PlanInbox.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/SetupMicroplan.js (1)
Pattern
**/*.js: check
📓 Learnings (1)
health/micro-ui/web/micro-ui-internals/packages/modules/microplan/src/components/UserUpload.js (4)
Learnt from: siddhant-nawale-egov PR: egovernments/DIGIT-Frontend#845 File: micro-ui/web/micro-ui-internals/packages/modules/hcm-microplanning/src/components/Upload.js:460-461 Timestamp: 2024-06-12T08:18:44.708Z Learning: Error handling for Shapefile parsing in `Upload.js` is managed commonly and is handled elsewhere in the codebase, as clarified by the user.Learnt from: siddhant-nawale-egov PR: egovernments/DIGIT-Frontend#845 File: micro-ui/web/micro-ui-internals/packages/modules/hcm-microplanning/src/components/Upload.js:460-461 Timestamp: 2024-10-08T20:11:07.772Z Learning: Error handling for Shapefile parsing in `Upload.js` is managed commonly and is handled elsewhere in the codebase, as clarified by the user.Learnt from: siddhant-nawale-egov PR: egovernments/DIGIT-Frontend#845 File: micro-ui/web/micro-ui-internals/packages/modules/hcm-microplanning/src/components/Upload.js:460-461 Timestamp: 2024-10-08T20:11:12.539Z Learning: Error handling for Shapefile parsing in `Upload.js` is managed commonly and is handled elsewhere in the codebase, as clarified by the user.Learnt from: siddhant-nawale-egov PR: egovernments/DIGIT-Frontend#204 File: micro-ui/web/micro-ui-internals/packages/modules/hcm-microplanning/src/pages/employee/Upload.js:468-481 Timestamp: 2024-03-13T07:33:45.211Z Learning: The error message in the `checkForErrorInUploadedFile` function within `Upload.js` is being localized and improved for better user experience, as clarified by the user.
🪛 Biome
health/micro-ui/web/micro-ui-internals/packages/modules/microplan/src/components/Response.js
[error] 13-13: 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] 20-44: 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)
health/micro-ui/web/micro-ui-internals/packages/modules/microplan/src/components/RoleTableComposer.js
[error] 413-413: 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)
health/micro-ui/web/micro-ui-internals/packages/modules/microplan/src/components/UserUpload.js
[error] 487-500: 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)
🔇 Additional comments (16)
health/micro-ui/web/micro-ui-internals/example/public/index.html (2)
19-19: LGTM! CSS version update looks good.The update from version 0.1.11 to 0.1.12 of
@egovernments/digit-ui-health-cssis a minor version bump, suggesting backward-compatible changes.
19-19: Verify the CSS version update across all environments.Since this is in the example directory, ensure that the same CSS version is used consistently across all environments and other HTML templates.
✅ Verification successful
CSS version is consistent across HTML files
The search results confirm that the CSS version
0.1.12is used consistently in both HTML files where thedigit-ui-health-csspackage is referenced:
health/micro-ui/web/public/index.htmlhealth/micro-ui/web/micro-ui-internals/example/public/index.html🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Check for inconsistent CSS versions across HTML files # Expected: All files should use the same version (0.1.12) echo "Searching for digit-ui-health-css version references:" rg --type html "digit-ui-health-css@[0-9]+\.[0-9]+\.[0-9]+" -A 1Length of output: 684
health/micro-ui/web/public/index.html (1)
14-14: LGTM! Verify version consistency across the codebase.The CSS version update looks good, but let's ensure it's consistent across all files.
✅ Verification successful
Version consistency verified across HTML files
All references to
digit-ui-health-cssin the codebase are using version0.1.12, confirming consistent versioning.🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Check for any inconsistent versions of digit-ui-health-css # Expect: All references should use version 0.1.12 rg --type html --type json "digit-ui-health-css@[0-9]+" .Length of output: 384
health/micro-ui/web/micro-ui-internals/packages/modules/microplan/src/components/Response.js (2)
8-57: Implementation looks good overallThe Response component is well-structured with proper:
- State management
- Internationalization
- Error handling
- Fallback navigation
The changes align well with the PR objectives for microplan quick fixes.
🧰 Tools
🪛 Biome
[error] 13-13: 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] 20-44: 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)
46-54: Verify the new CSS class implementationThe code adds a new
microplan-response-buttonclass. Let's verify its implementation in the stylesheet.✅ Verification successful
CSS class implementation is verified
The
microplan-response-buttonclass is properly implemented in the stylesheet. It contains specific styling for the button's icon-label-container to ensure proper label width:.microplan-response-button { .icon-label-container { .digit-button-label { min-width: max-content; } } }🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Search for the new CSS class definition rg "microplan-response-button" --type css --type scssLength of output: 82
Script:
#!/bin/bash # Search for the CSS class definition with broader file patterns rg "microplan-response-button" -g "*.css" -g "*.scss" -g "*.less" -g "*.sass" # Also search in JavaScript/TypeScript files as styles might be defined inline or in styled-components rg "microplan-response-button" -g "*.js" -g "*.jsx" -g "*.ts" -g "*.tsx"Length of output: 407
Script:
#!/bin/bash # Let's check the actual CSS implementation in the scss file cat health/micro-ui/web/micro-ui-internals/packages/css/src/components/microplanning.scssLength of output: 57612
health/micro-ui/web/micro-ui-internals/packages/modules/microplan/src/components/UserAccess.js (4)
4-4: LGTM: Import statement updated correctlyThe addition of CustomLoader import aligns with its usage in the DataTable component.
80-80: LGTM: Data mapping syntax improvedThe planData assignment has been simplified while maintaining the same functionality.
Line range hint
211-216: LGTM: Button styling is consistentThe button properties follow the component's design system with appropriate variation and icon placement.
Line range hint
223-247: Verify error handling for search and paginationLet's verify the error handling for various scenarios:
✅ Verification successful
Based on the search results, I can now provide a comprehensive verification of the error handling in the component. Let me generate the final response:
Error handling is properly implemented across search and pagination scenarios
The component and its related dependencies have comprehensive error handling:
- Search functionality has proper error handling through
isPlanEmpSearchLoadingstate- Empty state handling via
!planEmployee?.data || planEmployee?.data?.length === 0check- Error propagation from API calls with proper error message display via Toast component
- Pagination errors are handled through the
planEmployeeErrorstate from the hookThe error scenarios are well covered through:
- Loading states management
- Empty results handling with NoResultsFound component
- API error handling with proper user feedback
- Data validation before operations
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Search for error handling patterns in the codebase echo "Checking error handling patterns..." rg -A 5 "catch|error|fail" health/micro-ui/web/micro-ui-internals/packages/modules/microplan/ # Check for similar components' error handling echo "Checking similar components..." ast-grep --pattern 'catch (error) { $$$$ }'Length of output: 165576
health/micro-ui/web/micro-ui-internals/packages/modules/microplan/src/pages/employee/SetupMicroplan.js (1)
35-35: LGTM: Clean query param extractionThe addition of
setupCompletedwith optional chaining is a safe and clean implementation for handling query parameters.health/micro-ui/web/micro-ui-internals/packages/modules/microplan/src/pages/employee/PopInbox.js (1)
498-498: Verify window.contextPath availability.The change to use
window.contextPathfor dynamic URL construction is correct, but ensure that this global variable is always available when the component renders.✅ Verification successful
The use of window.contextPath is properly initialized and safe to use
The verification shows that
window.contextPathis consistently initialized across the application:
- It's set in multiple App.js entry points using
window?.globalConfigs?.getConfig("CONTEXT_PATH")- There's a fallback value of "digit-ui" in the example configurations
- The optional chaining ensures safe access to the global config
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Search for window.contextPath initialization rg -A 3 "window.contextPath\s*="Length of output: 4218
health/micro-ui/web/micro-ui-internals/packages/modules/microplan/src/configs/UICustomizations.js (1)
103-103: Verify the impact of the new query parameter.The addition of
setup-completed=trueto the URL suggests this is used to control the view/edit mode of the setup-microplan page. Please ensure that:
- The setup-microplan page correctly handles this parameter
- The parameter cannot be manipulated to bypass edit restrictions
Let's verify the usage of this parameter:
✅ Verification successful
Let me gather more information about how the
setup-completedparameter is handled in the SetupMicroplan component.
Let me check how the
setupCompletedparameter affects the component's behavior and UI rendering.
The
setup-completedparameter is properly implemented and secureThe verification shows that the
setup-completedparameter is used appropriately in the SetupMicroplan component to:
- Control the visibility of action buttons - showing only a "GO_BACK_TO_MY_MICROPLAN" button when in completed view mode
- The parameter is properly parsed from URL query params and doesn't affect any critical functionality or bypass any security measures
The implementation aligns with the intended view-only functionality for completed microplans.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Search for setup-completed parameter usage rg -l 'setup-completed' # Search for route handling in setup-microplan rg -A 5 'setup-microplan'Length of output: 8953
Script:
#!/bin/bash # Check how setup-completed is used in SetupMicroplan.js rg -A 10 'setup-completed' 'health/micro-ui/web/micro-ui-internals/packages/modules/microplan/src/pages/employee/SetupMicroplan.js' # Check for any state management related to setup-completed ast-grep --pattern 'const [$_] = useSearchParams()' 'health/micro-ui/web/micro-ui-internals/packages/modules/microplan/src/pages/employee/SetupMicroplan.js'Length of output: 823
Script:
#!/bin/bash # Search for usage of setupCompleted variable in conditional statements or UI rendering rg -A 15 'setupCompleted' 'health/micro-ui/web/micro-ui-internals/packages/modules/microplan/src/pages/employee/SetupMicroplan.js' # Look for any form or action handlers that might be affected by setupCompleted ast-grep --pattern 'if ($setupCompleted) { $$$ }'Length of output: 1424
health/micro-ui/web/micro-ui-internals/packages/modules/microplan/src/pages/employee/PlanInbox.js (1)
550-550: LGTM! Consistent with contextPath updates.The update to use
/${window.contextPath}/employeefor the backlink property aligns with similar changes across other components, ensuring consistent URL handling based on the application context.Let's verify similar contextPath usage in related components:
✅ Verification successful
Consistent usage of contextPath in backlink properties verified
The verification shows that all related microplan components consistently use
/${window.contextPath}/employeefor backlink properties:
- FacilityCatchmentMapping.js
- PlanInbox.js
- PopInbox.js
- Response.js (with fallback)
- createUpdatePlanProject.js
- UserUpload.js
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Search for backlink properties in related microplan components rg -A 2 'backlink.*employee' health/micro-ui/web/micro-ui-internals/packages/modules/microplan/Length of output: 2611
health/micro-ui/web/micro-ui-internals/packages/css/src/components/microplanning.scss (1)
2690-2691: 🧹 Nitpick (assertive)Fix the indentation.
The closing brace is not properly aligned with its opening brace.
Apply this diff to fix the indentation:
.search-field-wrapper.search.custom-both-clear-search{ align-items: center; }Likely invalid or redundant comment.
health/micro-ui/web/micro-ui-internals/packages/modules/microplan/src/components/RoleTableComposer.js (2)
35-38:CustomLoadercomponent is properly defined and exportedThe
CustomLoadercomponent is correctly defined and exported for use as a custom loading indicator in theDataTable.
398-399: Verify variable assignments for hierarchy comparisonThere might be an inconsistency in how
selectedHierarchyandincomingHierarchyare assigned. Currently,selectedHierarchyis obtained fromHrmsData, andincomingHierarchyis obtained fromrowData, which is the opposite of howselectedBoundariesandincomingBoundariesare assigned. Please verify that the variables are correctly assigned for an accurate comparison.
health/micro-ui/web/micro-ui-internals/example/public/index.html
Outdated
Show resolved
Hide resolved
health/micro-ui/web/micro-ui-internals/packages/modules/microplan/src/components/Response.js
Show resolved
Hide resolved
health/micro-ui/web/micro-ui-internals/packages/modules/microplan/src/components/Response.js
Show resolved
Hide resolved
health/micro-ui/web/micro-ui-internals/packages/modules/microplan/src/components/Response.js
Show resolved
Hide resolved
...cro-ui/web/micro-ui-internals/packages/modules/microplan/src/components/RoleTableComposer.js
Outdated
Show resolved
Hide resolved
...cro-ui/web/micro-ui-internals/packages/modules/microplan/src/components/RoleTableComposer.js
Outdated
Show resolved
Hide resolved
...cro-ui/web/micro-ui-internals/packages/modules/microplan/src/components/RoleTableComposer.js
Outdated
Show resolved
Hide resolved
...cro-ui/web/micro-ui-internals/packages/modules/microplan/src/components/RoleTableComposer.js
Outdated
Show resolved
Hide resolved
...cro-ui/web/micro-ui-internals/packages/modules/microplan/src/components/RoleTableComposer.js
Outdated
Show resolved
Hide resolved
...cro-ui/web/micro-ui-internals/packages/modules/microplan/src/components/RoleTableComposer.js
Outdated
Show resolved
Hide resolved
nabeelmd-eGov
force-pushed
the
microplan-quick-fixes
branch
from
4612f01
to
f423902
Compare
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Outside diff range comments (2)
health/micro-ui/web/micro-ui-internals/packages/modules/microplan/src/components/RoleTableComposer.js (1)
Line range hint 298-316: Remove unused parameter from handleUpdateAssignEmployee.
The updateAssignee parameter is passed to the function but never used in its implementation. This could lead to confusion about the function's behavior.
-const handleUpdateAssignEmployee = (row, updateAssignee) => {
+const handleUpdateAssignEmployee = (row) => {
setIsLoading(true);
const payload = {
PlanEmployeeAssignment: {health/micro-ui/web/micro-ui-internals/packages/modules/microplan/src/pages/employee/PlanInbox.js (1)
Line range hint 553-555: Fix error handling in WorkflowCommentPopUp
The error handler is trying to access properties of an undefined 'error' variable. This could cause runtime errors when API calls fail.
Apply this fix:
- setShowToast({ key: "error", label: t(error?.response?.data?.Errors?.[0]?.code) });
+ setShowToast({ key: "error", label: t(data?.response?.data?.Errors?.[0]?.code) });📜 Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
📒 Files selected for processing (2)
- health/micro-ui/web/micro-ui-internals/packages/modules/microplan/src/components/RoleTableComposer.js (4 hunks)
- health/micro-ui/web/micro-ui-internals/packages/modules/microplan/src/pages/employee/PlanInbox.js (3 hunks)
🧰 Additional context used
📓 Path-based instructions (2)
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/pages/employee/PlanInbox.js (1)
Pattern **/*.js: check
🪛 Biome
health/micro-ui/web/micro-ui-internals/packages/modules/microplan/src/components/RoleTableComposer.js
[error] 401-401: 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/RoleTableComposer.js (2)
Line range hint 35-41: LGTM: CustomLoader component implementation.
The CustomLoader component is well-implemented using styled-components with proper animation and styling.
388-393: 🧹 Nitpick (assertive)
Verify the impact of removing boundary and hierarchy comparison logic.
The commented-out code contains important business logic for comparing boundaries and hierarchies. Please verify that removing this logic doesn't affect the assignment/unassignment functionality.
Remove the commented code if it's no longer needed:
- // const selectedBoundaries = rowData?.find((i) => i.employeeId === row.employeeId)?.selectedBoundaries?.map((i) => i?.code);
- // const incomingBoundaries = HrmsData?.planSearchData.find((i) => i.employeeId === row.employeeId)?.jurisdiction;
- // const selectedHierarchy = HrmsData?.planSearchData.find((i) => i.employeeId === row.employeeId)?.selectedHierarchy?.boundaryType;
- // const incomingHierarchy = rowData?.find((i) => i.employeeId === row.employeeId)?.hierarchyLevel;
- // const isHierarchyEqual = !incomingHierarchy ? true : _.isEqual(incomingHierarchy, selectedHierarchy);
- // const isBoundaryEqual = !incomingBoundaries || incomingBoundaries?.length === 0 ? true : _.isEqual(incomingBoundaries, selectedBoundaries);health/micro-ui/web/micro-ui-internals/packages/modules/microplan/src/pages/employee/PlanInbox.js (2)
312-337: LGTM: Translation implementation looks good!
The changes properly implement internationalization for the DataTable columns using the t() function while maintaining the fallback to "NA" for undefined values.
550-550: LGTM: Dynamic URL path implementation is correct!
The backlink URL now correctly uses window.contextPath for dynamic path construction, which improves deployment flexibility.
...cro-ui/web/micro-ui-internals/packages/modules/microplan/src/components/RoleTableComposer.js
Outdated
Show resolved
Hide resolved
...cro-ui/web/micro-ui-internals/packages/modules/microplan/src/components/RoleTableComposer.js
Show resolved
Hide resolved
Choose the appropriate template for your PR:
Summary by CodeRabbit
Release Notes
New Features
UserAccesscomponent with a new loading indicator.ActionBarin theSetupMicroplancomponent based on query parameters.Improvements
MicroplanSearchConfigandUserManagementConfig.Bug Fixes
Style