[ENG-559] feat: add pagination for user depts#16456
Conversation
✅ Deploy Preview for care-ohc ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
Greptile SummaryThis PR adds search and pagination to the user departments tab and pre-populates the role selector with the user's current role when the edit sheet opens.
Confidence Score: 5/5Safe to merge — changes are additive (pagination + search) and the existing role-update/remove flows are unaffected. Both changed files make straightforward, well-scoped additions. The pagination implementation correctly resets to page 1 on search (verified in useFilters source), query invalidation uses prefix keys that still match the expanded query key, and the new i18n keys are present in all locale files. No files require special attention. Important Files Changed
Reviews (2): Last reviewed commit: "chore: add aria label" | Re-trigger Greptile |
| const [selectedRole, setSelectedRole] = useState<RoleBase>({ | ||
| ...userRole.role, | ||
| }); |
There was a problem hiding this comment.
Stale selection persists across sheet open/close cycles
useState initializes selectedRole only once when the component mounts. Because this component lives permanently in the DOM inside each rendered DepartmentCard, the selectedRole state is never reset between opens. If a user selects a different role, cancels without saving, then reopens the sheet, they will see the previously-chosen (but unsaved) role pre-selected — which is misleading and would cause an unintended role change if they then click "Update Role". Add a useEffect that resets to userRole.role whenever open transitions to true.
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Repository UI Review profile: ASSERTIVE Plan: Pro Run ID: 📒 Files selected for processing (1)
Walkthrough
ChangesUser Management UX
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
There was a problem hiding this comment.
Pull request overview
Adds pagination and search support to the “User Departments” view, and ensures the “Edit Facility User Role” sheet opens with the current role pre-selected—improving usability when users belong to many departments.
Changes:
- Added
useFilters-driven pagination (limit/offset) for user departments listing. - Added department name search input with debounced fetching and updated empty-state messaging for “no results”.
- Initialized role selection state in the role-edit sheet using the existing
userRole.role.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
| src/pages/Facility/settings/organizations/components/EditFacilityUserRoleSheet.tsx | Pre-populates selected role state from the user’s current role when the component mounts. |
| src/components/Users/UserDepartmentsTab.tsx | Adds search + limit/offset pagination to the departments list and renders pagination controls + search-aware empty state. |
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src/components/Users/UserDepartmentsTab.tsx`:
- Around line 157-162: The Input component in UserDepartmentsTab.tsx (around
line 157) currently relies only on a placeholder for accessibility, which is not
reliable for screen-reader users. Add an `aria-label` attribute to the Input
component with an appropriate accessible label (such as "Search departments") to
provide a consistent accessible name that screen readers can reliably announce,
ensuring compliance with accessibility guidelines.
- Around line 134-139: The offset calculation for pagination in the queryParams
object uses an unsafe expression that can produce invalid values when
qParams.page contains zero, negative numbers, or non-numeric strings from the
URL query state. Instead of directly using ((qParams.page || 1) - 1), validate
and normalize the page parameter first by parsing it as an integer and ensuring
it is at least 1, then use the normalized value in the offset calculation.
Reference the validation patterns established in src/hooks/useFilters.tsx to
align with the URL-derived state handling contract.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: a4b08e8f-7439-4c97-9ab2-9c005d227c43
📒 Files selected for processing (2)
src/components/Users/UserDepartmentsTab.tsxsrc/pages/Facility/settings/organizations/components/EditFacilityUserRoleSheet.tsx
🎭 Playwright Test ResultsStatus: ❌ Failed
📊 Detailed results are available in the playwright-final-report artifact. Run: #9550 |
Deploying care-preview with
|
| Latest commit: |
dbf6e2e
|
| Status: | ✅ Deploy successful! |
| Preview URL: | https://68edea89.care-preview-a7w.pages.dev |
| Branch Preview URL: | https://eng-559-dept-pagination.care-preview-a7w.pages.dev |
Proposed Changes
Tagging: @ohcnetwork/care-fe-code-reviewers
Merge Checklist
Summary by CodeRabbit
Release Notes
New Features
Bug Fixes
UX Improvements