Update Staff Dashboard columns and add location filter#174
Update Staff Dashboard columns and add location filter#174ihsankahveci wants to merge 2 commits intokc-pit-2026-testfrom
Conversation
…atus, Actions - Removed employeeId column - Renamed Position to Role in UI - Added Location column showing hub name instead of ObjectId - Added Phone Number column - Updated StaffMember interface across all components - Fetch locations data to resolve locationObjectId to hubName Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com>
- Added location dropdown filter in StaffDashboardControls - Updated filterStaff function to filter by location - Extract unique location names from locations data - Reset pagination when filters change - Location filter works alongside existing role and search filters Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com>
There was a problem hiding this comment.
Pull request overview
This PR updates the Staff Dashboard to improve the column display and add location filtering capability. The changes align with the PR description and successfully implement the requested features.
Changes:
- Updated StaffMember interface to replace Employee ID with Location and Phone Number fields
- Added location filter dropdown that works alongside existing search and role filters
- Modified table columns to display: Name, Role, Location, Phone Number, Status, Actions
- Implemented automatic pagination reset when filters change
Reviewed changes
Copilot reviewed 5 out of 5 changed files in this pull request and generated 7 comments.
Show a summary per file
| File | Description |
|---|---|
| client/src/pages/StaffDashboard/utils/StaffDashboardUtils.tsx | Updated StaffMember interface, modified filterStaff to support location filtering, and updated transformUsersToStaff to map location ObjectIds to display names |
| client/src/pages/StaffDashboard/components/StaffDashboardTable.tsx | Updated table headers and sorting configuration to reflect new column structure (Name, Role, Location, Phone Number, Status, Actions) |
| client/src/pages/StaffDashboard/components/StaffDashboardRow.tsx | Removed Employee ID cell and added Location and Phone Number cells to match updated interface |
| client/src/pages/StaffDashboard/components/StaffDashboardControls.tsx | Added location filter dropdown with dynamic population from location data |
| client/src/pages/StaffDashboard/StaffDashboard.tsx | Added location data fetching, created locationMap for ObjectId-to-name translation, implemented location filter state management, and added useEffect to reset pagination on filter changes |
| const { data: users, mutate } = userService.useUsers() || {}; | ||
| const { data: locations } = locationService.useLocations() || {}; |
There was a problem hiding this comment.
Missing loading state handling: The component doesn't handle the loading state for locations data. While users and locations are being fetched, all staff members will temporarily display 'N/A' for their location, and the location filter dropdown will be empty. This could be confusing for users.
The SWR hook returns an isLoading state that could be used. Consider following the pattern used in SurveyEntryDashboard (client/src/pages/SurveyEntryDashboard/SurveyEntryDashboard.tsx:35) where isLoading is destructured and passed to the table component to show a loading indicator.
| interface StaffMember { | ||
| id: string; | ||
| employeeId: string; | ||
| name: string; | ||
| position: string; | ||
| locationObjectId: string; | ||
| phone: string; | ||
| approvalStatus: string; | ||
| } |
There was a problem hiding this comment.
Code duplication: The StaffMember interface is duplicated across four files (StaffDashboardUtils.tsx, StaffDashboardTable.tsx, StaffDashboardRow.tsx, and StaffDashboard.tsx). This violates the DRY principle and makes maintenance difficult - any future changes to the interface structure would need to be made in four places.
Recommendation: Create a shared type definition file (e.g., client/src/pages/StaffDashboard/types.ts) and export the StaffMember interface from there, then import it in all files that need it. This is a common pattern for shared types within a feature module.
| // Create location lookup map | ||
| const locationMap = new Map( | ||
| locations?.map(loc => [loc._id, loc.hubName]) ?? [] |
There was a problem hiding this comment.
Potential edge case with empty location names: If a location has an empty string for hubName, the locationMap will store that empty string, and it will be displayed in the table as an empty cell rather than 'N/A'. The nullish coalescing operator (??) only catches null/undefined, not empty strings.
Consider using a more robust check:
locationMap.get(user.locationObjectId?.toString() ?? '') || 'N/A'
This would also catch empty strings. Alternatively, filter out locations with empty hubNames when building the locationMap, similar to how uniqueLocations is built (line 93).
| // Create location lookup map | |
| const locationMap = new Map( | |
| locations?.map(loc => [loc._id, loc.hubName]) ?? [] | |
| // Create location lookup map, excluding empty/whitespace-only hub names | |
| const locationMap = new Map( | |
| (locations ?? []) | |
| .filter(loc => loc.hubName && loc.hubName.trim() !== '') | |
| .map(loc => [loc._id, loc.hubName]) |
| name: `${user.firstName} ${user.lastName}`, | ||
| position: user.role, | ||
| locationObjectId: locationMap.get(user.locationObjectId?.toString() ?? '') ?? 'N/A', | ||
| phone: user.phone ?? 'N/A', |
There was a problem hiding this comment.
Potential edge case with empty phone numbers: If a user has an empty string for phone, it will be displayed as an empty cell rather than 'N/A'. The nullish coalescing operator (??) only catches null/undefined, not empty strings.
Consider using:
phone: user.phone || 'N/A'
This would also catch empty strings and display 'N/A' consistently.
| phone: user.phone ?? 'N/A', | |
| phone: user.phone || 'N/A', |
| employeeId: string; | ||
| name: string; | ||
| position: string; | ||
| locationObjectId: string; |
There was a problem hiding this comment.
Misleading field name: The locationObjectId field now stores the location's display name (hubName) instead of the ObjectId. This creates confusion as the field name suggests it contains an ObjectId when it actually contains a human-readable string.
Recommendation: Rename this field to location or locationName throughout the StaffMember interface in all files (StaffDashboardUtils.tsx, StaffDashboardTable.tsx, StaffDashboardRow.tsx, StaffDashboard.tsx) to accurately reflect what it stores. This will make the code more maintainable and prevent future confusion.
| employeeId: user._id ?? 'N/A', | ||
| name: `${user.firstName} ${user.lastName}`, | ||
| position: user.role, | ||
| locationObjectId: locationMap.get(user.locationObjectId?.toString() ?? '') ?? 'N/A', |
There was a problem hiding this comment.
The location filter uses a Map lookup that could return undefined if a user's locationObjectId doesn't exist in the locations data. The code handles this with the nullish coalescing operator (??) defaulting to 'N/A', which is good. However, this means users with invalid or missing location references will show 'N/A' but won't be filterable by any location (they'll only show when "All Locations" is selected). This is acceptable behavior but consider whether these users should be more prominently identified or handled differently in the UI.
| // Create location lookup map | ||
| const locationMap = new Map( | ||
| locations?.map(loc => [loc._id, loc.hubName]) ?? [] | ||
| ); | ||
|
|
||
| // Get unique location names for filter dropdown | ||
| const uniqueLocations = Array.from( | ||
| new Set(locations?.map(loc => loc.hubName).filter(Boolean)) | ||
| ).sort(); |
There was a problem hiding this comment.
Potential issue: The locationMap and uniqueLocations are computed on every render, even when the locations data hasn't changed. While the performance impact is likely minimal for typical datasets, consider wrapping these computations in useMemo hooks to avoid unnecessary recalculations:
const locationMap = useMemo(() => new Map(locations?.map(loc => [loc._id, loc.hubName]) ?? []), [locations]);
const uniqueLocations = useMemo(() => Array.from(new Set(locations?.map(loc => loc.hubName).filter(Boolean))).sort(), [locations]);
📄 Description
Updated the Staff Dashboard to display the correct columns and added location filtering capability:
Column Changes
Location Filter
✅ Checklist
🔗 Related Issues
N/A
💡 Type of change
🧪 How to test
📝 Notes to reviewers
🤖 Generated with Claude Code