fixed facilitysearch issue #1697
Conversation
📝 WalkthroughWalkthroughThe pull request includes updates to several files primarily focused on modifying CSS styles and updating the stylesheet links for the health UI component. The HTML files have had their links changed to reference a new version of the CSS package, while the SCSS file introduces new classes and modifies existing ones. Additionally, a JavaScript component has been adjusted for improved styling capabilities without altering functionality. 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: 3
🧹 Outside diff range comments (6)
health/micro-ui/web/micro-ui-internals/packages/css/src/pages/employee/facility.scss (2)
Line range hint
1-3: Consider improving the popup's responsive design.While setting a max-width helps control the popup size, consider these improvements:
- Replace
!importantwith more specific selectors- Add media queries for better mobile responsiveness
- Use relative units (e.g.,
vw) for dynamic sizing.facility-popup { - max-width: 85% !important; + max-width: min(85%, 1200px); + width: 100%; + @media screen and (max-width: 768px) { + max-width: 95%; + } }
Line range hint
11-13: Consider using design system variables for consistency.The border color should ideally come from your design system's color palette for better maintainability.
.facility-popup-table-card { - border: 1px solid #d6d5d4; + border: 1px solid var(--border-color, #d6d5d4); + border-radius: var(--border-radius-regular, 4px); }health/micro-ui/web/public/index.html (1)
Line range hint
8-14: Consider adding integrity hashes for CDN resources.The application is loading multiple CSS files from unpkg.com without SHA integrity hashes. This poses a security risk as the content served by the CDN could potentially be compromised.
Consider adding integrity hashes to ensure the authenticity of the loaded resources:
- <link rel="stylesheet" href="https://unpkg.com/@egovernments/digit-ui-css@1.8.2-beta.34/dist/index.css" /> - <link rel="stylesheet" href="https://unpkg.com/@egovernments/digit-ui-components-css@0.0.2-beta.39/dist/index.css" /> - <link rel="stylesheet" href="https://unpkg.com/@egovernments/digit-ui-health-css@0.1.12/dist/index.css" /> + <link rel="stylesheet" href="https://unpkg.com/@egovernments/digit-ui-css@1.8.2-beta.34/dist/index.css" + integrity="sha384-[generated-hash]" crossorigin="anonymous" /> + <link rel="stylesheet" href="https://unpkg.com/@egovernments/digit-ui-components-css@0.0.2-beta.39/dist/index.css" + integrity="sha384-[generated-hash]" crossorigin="anonymous" /> + <link rel="stylesheet" href="https://unpkg.com/@egovernments/digit-ui-health-css@0.1.12/dist/index.css" + integrity="sha384-[generated-hash]" crossorigin="anonymous" />Additionally, consider:
- Using a more stable CDN or self-hosting these assets
- Implementing a fallback mechanism in case the CDN is unavailable
- Bundling these CSS files with your application for better control and performance
health/micro-ui/web/micro-ui-internals/packages/modules/microplan/src/hooks/useFacilityCatchmentMapping.js (2)
Line range hint
1-1: Fix typo in function nameThe function name has a typo:
useFcilityCatchmentMappingis missing an 'a'. It should beuseFacilityCatchmentMapping.-const useFcilityCatchmentMapping = (props) => { +const useFacilityCatchmentMapping = (props) => {
Line range hint
51-56: Add error handling for API failuresThe hook returns a revalidate function that does nothing (
() => {}). Consider handling API errors and providing proper error state.return { isLoading, isFetching, data, refetch, - revalidate: () => {}, + error: data?.error, + revalidate: refetch, };health/micro-ui/web/micro-ui-internals/packages/modules/microplan/src/components/SearchJurisdiction.js (1)
Line range hint
69-87: Consider performance and accessibility improvements.The boundary selection implementation could benefit from the following enhancements:
- Performance:
- Memoize the filtered boundaries to prevent unnecessary recalculations
- Batch state updates where possible
- Accessibility:
- Add ARIA labels for screen readers
- Ensure proper keyboard navigation support
Example implementation:
+const memoizedBoundaryOptions = useMemo(() => { + if (!selectedHierarchy) return []; + const userBoundaries = Digit.Utils.microplanv1.filterBoundariesByJurisdiction(boundaries, jurisdiction.boundaryCodes); + const filteredBoundaries = userBoundaries.filter((row) => row.type === selectedHierarchy.boundaryType); + return Digit.Utils.microplanv1.groupByParent(filteredBoundaries); +}, [selectedHierarchy, boundaries, jurisdiction]); <MultiSelectDropdown variant={"nestedmultiselect"} selected={selectedBoundaries} onSelect={handleBoundarySelect} className="searchjurisdiction-multiselectdropdown" + aria-label={t("SELECT_BOUNDARIES")} + role="listbox" isSearchable={true} t={t} addCategorySelectAllCheck={true} addSelectAllCheck={true} - options={boundaryOptions} + options={memoizedBoundaryOptions} optionsKey={"name"} name={"nestedmultiselectoptions"} />
📜 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 (5)
- 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/facility.scss (1 hunks)
- health/micro-ui/web/micro-ui-internals/packages/modules/microplan/src/components/SearchJurisdiction.js (2 hunks)
- health/micro-ui/web/micro-ui-internals/packages/modules/microplan/src/hooks/useFacilityCatchmentMapping.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/microplan/src/components/SearchJurisdiction.js (1)
Pattern
**/*.js: checkhealth/micro-ui/web/micro-ui-internals/packages/modules/microplan/src/hooks/useFacilityCatchmentMapping.js (1)
Pattern
**/*.js: check
🔇 Additional comments (6)
health/micro-ui/web/micro-ui-internals/packages/css/src/pages/employee/facility.scss (2)
Line range hint
5-9: LGTM! Well-structured flex layouts.The flex layouts for both wrappers are well implemented with appropriate spacing and alignment properties.
Also applies to: 15-19
21-23: LGTM! Clear class naming.The class name follows a consistent naming convention and the styling is appropriately scoped.
health/micro-ui/web/micro-ui-internals/example/public/index.html (1)
19-19: Version update looks good, but needs verification.The CSS package version update from 0.1.11 to 0.1.12 appears to be related to fixing the facility search issue mentioned in the PR title.
Let's verify if this version contains the necessary changes:
✅ Verification successful
Version update is justified and contains necessary facility search fixes
The git history shows that the CSS package version update from 0.1.11 to 0.1.12 includes recent changes to fix facility search issues:
- Added new styles for facility popup layout and search results
- Fixed margin issue in facility search jurisdiction dropdown
- Improved facility popup table card styling
These changes directly correspond to the PR's objective of fixing facility search issues.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify the changes in the new CSS version and its relation to facility search # Test: Check for any facility-related CSS changes in the repository rg -i "facility|search" --type cssLength of output: 16646
Script:
#!/bin/bash # Let's check the facility.scss file content to understand the CSS changes cat health/micro-ui/web/micro-ui-internals/packages/css/src/pages/employee/facility.scss # Also check git history for recent changes to facility.scss git log -p health/micro-ui/web/micro-ui-internals/packages/css/src/pages/employee/facility.scssLength of output: 3190
health/micro-ui/web/public/index.html (1)
14-14: Verify if this CSS update fixes the facility search issue.The PR description mentions fixing a facility search issue, but this change only updates the health CSS package version. Please clarify how this CSS update addresses the facility search functionality problem.
✅ Verification successful
Based on the git history and code changes, I can now generate a final response:
The CSS version update is related to facility search functionality fixes
The version bump from 0.1.11 to 0.1.12 is directly tied to fixing the facility search issue as evidenced by commit ffc956b which shows:
- A fix in the facility search query parameter construction in
useFacilityCatchmentMapping.js- The bug was related to incorrect property access in the search query (
facilityTypeandstatuswere missing.nameproperty)The CSS package update was part of the same commit that fixed this functionality issue.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Check for related changes in the facility search related files # and the changelog of the CSS package # Look for facility search related files echo "Searching for facility search related files..." rg -l -i "facilitysearch|facility.?search" # Look for package.json files that might reference this CSS package echo "Checking for package dependencies..." fd package.json --exec grep -l "digit-ui-health-css"Length of output: 662
Script:
#!/bin/bash # Let's check the content of the found files to understand the facility search implementation # and any potential CSS-related issues # Check the facility catchment mapping hook echo "=== Facility Catchment Mapping Hook ===" cat health/micro-ui/web/micro-ui-internals/packages/modules/microplan/src/hooks/useFacilityCatchmentMapping.js # Check the facility mapping config echo -e "\n=== Facility Mapping Config ===" cat health/micro-ui/web/micro-ui-internals/packages/modules/microplan/src/configs/FacilityMappingConfig.js # Check the CSS package.json for version history echo -e "\n=== CSS Package.json ===" cat health/micro-ui/web/micro-ui-internals/packages/css/package.json # Check for any recent changes in these files echo -e "\n=== Recent Git History ===" git log -p --since="1 month ago" -- \ health/micro-ui/web/micro-ui-internals/packages/modules/microplan/src/hooks/useFacilityCatchmentMapping.js \ health/micro-ui/web/micro-ui-internals/packages/modules/microplan/src/configs/FacilityMappingConfig.js \ health/micro-ui/web/micro-ui-internals/packages/css/package.jsonLength of output: 49473
health/micro-ui/web/micro-ui-internals/packages/modules/microplan/src/hooks/useFacilityCatchmentMapping.js (1)
Line range hint
13-26: Verify the impact of active filterThe search is filtered by
active: true. Ensure this doesn't exclude valid inactive plans that should be visible.✅ Verification successful
Based on the search results, I can now provide a conclusive response about the active filter's impact:
The
active: truefilter is correctly implemented and should remainThe codebase shows this is an intentional design pattern used consistently across multiple components for plan employee assignments:
- In inbox views (PlanInbox.js, PopInbox.js) - Shows only active assignments for approvers
- In user access management (UserAccess.js) - Filters active assignments for role management
- In facility catchment mapping (FacilityCatchmentMapping.js) - Shows active assignments for mapping
The system has proper handling for inactive plans through:
- Explicit toggle functionality in UserAccess.js and RoleTableComposer.js that can switch
activestatus- Separate flows for managing active/inactive assignments through the PlanEmployeeAssignment update API
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Check if there are any inactive plans that should be visible ast-grep --pattern 'PlanEmployeeAssignmentSearchCriteria { $$$ active: $_, $$$ }'Length of output: 89
Script:
#!/bin/bash # Let's search for any references to PlanEmployeeAssignmentSearchCriteria in the codebase rg -A 5 "PlanEmployeeAssignmentSearchCriteria" # Also search for any API or service definitions related to plan assignments rg -A 5 "PlanEmployeeAssignment" # Look for any documentation or configuration files mentioning plan visibility rules fd -e md -e yaml -e json -X rg -l "plan.*visibility|plan.*active"Length of output: 26821
health/micro-ui/web/micro-ui-internals/packages/modules/microplan/src/components/SearchJurisdiction.js (1)
Line range hint
69-87: Verify if the facility search issue is fully resolved.The changes appear to be purely cosmetic. Given that this PR aims to fix a facility search issue, please verify:
- The search functionality works correctly with the selected boundaries
- The boundary selection properly filters facilities
- The clear/search actions work as expected
Let's check for any related facility search implementations:
✅ Verification successful
Let me check the implementation of
handleBoundarySelectand related validation logic:
Based on the code analysis, I can now provide a final response:
The cosmetic changes are appropriate and the search functionality appears to be properly implemented.
The code review reveals:
- The boundary selection handler (
handleBoundarySelect) correctly processes selected boundaries- The facility search implementation in
useFacilityCatchmentMapping.jsproperly uses the selected boundaries for filtering- The validation utilities include robust boundary validation logic that ensures data integrity
The changes in the PR maintain the existing search functionality while improving the UI layout, which is appropriate for this bugfix.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Search for facility search related code echo "Searching for facility search implementations..." rg -l "facility.*search|search.*facility" --type js # Check for any error handling or validation related to boundary selection echo "Checking boundary selection validation..." rg "handleBoundarySelect|validateBoundary" --type jsLength of output: 2391
Script:
#!/bin/bash # Check handleBoundarySelect implementation and related code echo "Checking boundary selection handler implementation..." rg -A 10 "handleBoundarySelect = \(" health/micro-ui/web/micro-ui-internals/packages/modules/microplan/src/components/SearchJurisdiction.js # Check facility catchment mapping implementation echo "Checking facility catchment mapping implementation..." cat health/micro-ui/web/micro-ui-internals/packages/modules/microplan/src/hooks/useFacilityCatchmentMapping.js # Check validation utilities echo "Checking validation utilities..." cat health/micro-ui/web/micro-ui-internals/packages/modules/microplan/src/utils/utilities.jsLength of output: 18184
...i/web/micro-ui-internals/packages/modules/microplan/src/hooks/useFacilityCatchmentMapping.js
Show resolved
Hide resolved
...ro-ui/web/micro-ui-internals/packages/modules/microplan/src/components/SearchJurisdiction.js
Show resolved
Hide resolved
Choose the appropriate template for your PR:
Summary by CodeRabbit
SearchJurisdictioncomponent.