Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
31 changes: 26 additions & 5 deletions client/src/pages/StaffDashboard/StaffDashboard.tsx
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
import { useState } from 'react';
import { useEffect, useState } from 'react';

import { useAuthContext } from '@/contexts';
import { useApi } from '@/hooks';
Expand All @@ -15,18 +15,20 @@ import {

interface StaffMember {
id: string;
employeeId: string;
name: string;
position: string;
locationObjectId: string;
phone: string;
approvalStatus: string;
}
Comment on lines 16 to 23
Copy link

Copilot AI Jan 26, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Copilot uses AI. Check for mistakes.

export default function StaffDashboard() {
const navigate = useNavigate();
const { userService } = useApi();
const { userService, locationService } = useApi();
const { userObjectId } = useAuthContext();
const [searchTerm, setSearchTerm] = useState('');
const [filterRole, setFilterRole] = useState('');
const [filterLocation, setFilterLocation] = useState('');
const [currentPage, setCurrentPage] = useState(0); // MUI uses 0-based index
const [itemsPerPage, setItemsPerPage] = useState(10);
const [sortConfig, setSortConfig] = useState<{
Expand All @@ -38,6 +40,12 @@ export default function StaffDashboard() {
});

const { data: users, mutate } = userService.useUsers() || {};
const { data: locations } = locationService.useLocations() || {};
Comment on lines 40 to +43
Copy link

Copilot AI Jan 26, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Copilot uses AI. Check for mistakes.

// Reset to first page when filters change
useEffect(() => {
setCurrentPage(0);
}, [searchTerm, filterRole, filterLocation]);

// Handlers
const handleApproval = async (id: string, status: string) => {
Expand Down Expand Up @@ -75,9 +83,19 @@ export default function StaffDashboard() {
}
};

// Create location lookup map
const locationMap = new Map(
locations?.map(loc => [loc._id, loc.hubName]) ?? []
Comment on lines +86 to +88
Copy link

Copilot AI Jan 26, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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).

Suggested change
// 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])

Copilot uses AI. Check for mistakes.
);

// Get unique location names for filter dropdown
const uniqueLocations = Array.from(
new Set(locations?.map(loc => loc.hubName).filter(Boolean))
).sort();
Comment on lines +86 to +94
Copy link

Copilot AI Jan 26, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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]);

Copilot uses AI. Check for mistakes.

// Data processing pipeline
const staffMembers = transformUsersToStaff(users ?? []);
const filteredStaff = filterStaff(staffMembers, searchTerm, filterRole);
const staffMembers = transformUsersToStaff(users ?? [], locationMap);
const filteredStaff = filterStaff(staffMembers, searchTerm, filterRole, filterLocation);
const sortedStaff = sortStaff(filteredStaff, sortConfig);
const currentStaff = paginateStaff(sortedStaff, currentPage, itemsPerPage);

Expand All @@ -104,8 +122,11 @@ export default function StaffDashboard() {
<StaffDashboardControls
searchTerm={searchTerm}
filterRole={filterRole}
filterLocation={filterLocation}
locations={uniqueLocations}
onSearchChange={setSearchTerm}
onRoleFilterChange={setFilterRole}
onLocationFilterChange={setFilterLocation}
onNewUserClick={() => navigate('/add-new-user')}
/>

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -13,16 +13,22 @@ import {
interface StaffDashboardControlsProps {
searchTerm: string;
filterRole: string;
filterLocation: string;
locations: string[];
onSearchChange: (value: string) => void;
onRoleFilterChange: (value: string) => void;
onLocationFilterChange: (value: string) => void;
onNewUserClick: () => void;
}

export default function StaffDashboardControls({
searchTerm,
filterRole,
filterLocation,
locations,
onSearchChange,
onRoleFilterChange,
onLocationFilterChange,
onNewUserClick
}: StaffDashboardControlsProps) {
return (
Expand Down Expand Up @@ -64,6 +70,22 @@ export default function StaffDashboardControls({
</Select>
</FormControl>

<FormControl size="small" sx={{ minWidth: 150 }}>
<InputLabel>Filter Location</InputLabel>
<Select
value={filterLocation}
label="Filter Location"
onChange={e => onLocationFilterChange(e.target.value)}
>
<MenuItem value="">All Locations</MenuItem>
{locations.map(location => (
<MenuItem key={location} value={location}>
{location}
</MenuItem>
))}
</Select>
</FormControl>

<Button
variant="contained"
onClick={onNewUserClick}
Expand Down
18 changes: 5 additions & 13 deletions client/src/pages/StaffDashboard/components/StaffDashboardRow.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -9,17 +9,17 @@ import {
Stack,
TableCell,
TableRow,
Tooltip,
Typography
Tooltip
} from '@mui/material';

import { UserDocument } from '@/types/User';

interface StaffMember {
id: string;
employeeId: string;
name: string;
position: string;
locationObjectId: string;
phone: string;
approvalStatus: string;
}

Expand Down Expand Up @@ -73,18 +73,10 @@ export default function StaffDashboardRow({
'&:hover': { backgroundColor: '#f8f8f8' }
}}
>
<TableCell>
<Typography
variant="body2"
sx={{
fontSize: '0.85rem'
}}
>
{member.employeeId}
</Typography>
</TableCell>
<TableCell>{member.name}</TableCell>
<TableCell>{member.position}</TableCell>
<TableCell>{member.locationObjectId}</TableCell>
<TableCell>{member.phone}</TableCell>
<TableCell>
<Tooltip
title={
Expand Down
47 changes: 33 additions & 14 deletions client/src/pages/StaffDashboard/components/StaffDashboardTable.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -12,9 +12,10 @@ import StaffDashboardRow from './StaffDashboardRow';

interface StaffMember {
id: string;
employeeId: string;
name: string;
position: string;
locationObjectId: string;
phone: string;
approvalStatus: string;
}

Expand Down Expand Up @@ -52,15 +53,15 @@ export default function StaffDashboardTable({
}}
>
<TableSortLabel
active={sortConfig.key === 'employeeId'}
active={sortConfig.key === 'name'}
direction={
sortConfig.key === 'employeeId'
sortConfig.key === 'name'
? sortConfig.direction
: 'asc'
}
onClick={() => onSort('employeeId')}
onClick={() => onSort('name')}
>
Employee ID
Name
</TableSortLabel>
</TableCell>
<TableCell
Expand All @@ -70,15 +71,15 @@ export default function StaffDashboardTable({
}}
>
<TableSortLabel
active={sortConfig.key === 'name'}
active={sortConfig.key === 'position'}
direction={
sortConfig.key === 'name'
sortConfig.key === 'position'
? sortConfig.direction
: 'asc'
}
onClick={() => onSort('name')}
onClick={() => onSort('position')}
>
Name
Role
</TableSortLabel>
</TableCell>
<TableCell
Expand All @@ -88,15 +89,33 @@ export default function StaffDashboardTable({
}}
>
<TableSortLabel
active={sortConfig.key === 'position'}
active={sortConfig.key === 'locationObjectId'}
direction={
sortConfig.key === 'position'
sortConfig.key === 'locationObjectId'
? sortConfig.direction
: 'asc'
}
onClick={() => onSort('position')}
onClick={() => onSort('locationObjectId')}
>
Location
</TableSortLabel>
</TableCell>
<TableCell
sx={{
fontWeight: 600,
backgroundColor: '#f9f9f9'
}}
>
<TableSortLabel
active={sortConfig.key === 'phone'}
direction={
sortConfig.key === 'phone'
? sortConfig.direction
: 'asc'
}
onClick={() => onSort('phone')}
>
Position
Phone Number
</TableSortLabel>
</TableCell>
<TableCell
Expand Down Expand Up @@ -131,7 +150,7 @@ export default function StaffDashboardTable({
<TableBody>
{staff.length === 0 ? (
<TableRow>
<TableCell colSpan={5} align="center">
<TableCell colSpan={6} align="center">
No staff members found.
</TableCell>
</TableRow>
Expand Down
17 changes: 11 additions & 6 deletions client/src/pages/StaffDashboard/utils/StaffDashboardUtils.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -2,9 +2,10 @@ import { UserDocument } from '@/types/User';

interface StaffMember {
id: string;
employeeId: string;
name: string;
position: string;
locationObjectId: string;
Copy link

Copilot AI Jan 26, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Copilot uses AI. Check for mistakes.
phone: string;
approvalStatus: string;
}

Expand All @@ -27,17 +28,19 @@ export const getStatusColor = (
};

/**
* Filter staff members by search term and role
* Filter staff members by search term, role, and location
*/
export const filterStaff = (
staffMembers: StaffMember[],
searchTerm: string,
filterRole: string
filterRole: string,
filterLocation: string
): StaffMember[] => {
return staffMembers.filter(
(staff: StaffMember) =>
staff.name.toLowerCase().includes(searchTerm.toLowerCase()) &&
(filterRole ? staff.position === filterRole : true)
(filterRole ? staff.position === filterRole : true) &&
(filterLocation ? staff.locationObjectId === filterLocation : true)
);
};

Expand Down Expand Up @@ -84,15 +87,17 @@ export const paginateStaff = (
* Transform users data to staff members format
*/
export const transformUsersToStaff = (
users: UserDocument[] | undefined
users: UserDocument[] | undefined,
locationMap: Map<string, string>
): StaffMember[] => {
if (!users) return [];

return users.map((user: UserDocument) => ({
id: user._id,
employeeId: user._id ?? 'N/A',
name: `${user.firstName} ${user.lastName}`,
position: user.role,
locationObjectId: locationMap.get(user.locationObjectId?.toString() ?? '') ?? 'N/A',
Copy link

Copilot AI Jan 26, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Copilot uses AI. Check for mistakes.
phone: user.phone ?? 'N/A',
Copy link

Copilot AI Jan 26, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Suggested change
phone: user.phone ?? 'N/A',
phone: user.phone || 'N/A',

Copilot uses AI. Check for mistakes.
approvalStatus: user.approvalStatus ?? 'PENDING'
}));
};
Loading