diff --git a/src/components/employees/EmployeeDetail.tsx b/src/components/employees/EmployeeDetail.tsx index 6e29dfd..b164884 100644 --- a/src/components/employees/EmployeeDetail.tsx +++ b/src/components/employees/EmployeeDetail.tsx @@ -17,6 +17,7 @@ import { getReviewsForEmployee, getEnpsForEmployee, } from '../../lib/tauri-commands'; +import { useResolvedManagerName } from './detail/useResolvedManagerName'; // Subcomponents import { PrepBriefModal } from '../people_map'; @@ -69,6 +70,14 @@ export function EmployeeDetail() { const { selectedEmployee, selectedEmployeeId, openEditModal, employees } = useEmployees(); + // Resolve the manager's name, fetching by ID when the manager isn't in the + // filtered/capped loaded list (issue #150). Called unconditionally, before + // the early return below, to satisfy the Rules of Hooks. + const managerName = useResolvedManagerName( + selectedEmployee?.manager_id, + employees + ); + // Performance data state const [ratings, setRatings] = useState([]); const [reviews, setReviews] = useState([]); @@ -122,9 +131,6 @@ export function EmployeeDetail() { // Derived data const latestRating = ratings[0]; const latestEnps = enpsResponses[0]; - const manager = selectedEmployee.manager_id - ? employees.find((e) => e.id === selectedEmployee.manager_id) - : null; const hasPerformanceData = ratings.length > 0 || enpsResponses.length > 0 || reviews.length > 0; @@ -145,7 +151,7 @@ export function EmployeeDetail() { {/* Info sections */} diff --git a/src/components/employees/detail/useResolvedManagerName.test.ts b/src/components/employees/detail/useResolvedManagerName.test.ts new file mode 100644 index 0000000..9c902d0 --- /dev/null +++ b/src/components/employees/detail/useResolvedManagerName.test.ts @@ -0,0 +1,64 @@ +import { renderHook, waitFor } from '@testing-library/react'; +import { describe, it, expect } from 'vitest'; +import { mockCommands } from '../../../test/tauri'; +import type { EmployeeWithLatestRating } from '../../../lib/tauri-commands'; +import { useResolvedManagerName } from './useResolvedManagerName'; + +function emp( + overrides: Partial = {} +): EmployeeWithLatestRating { + return { + id: 'emp-1', + email: 'a@example.com', + full_name: 'Ada Example', + status: 'active', + is_sample: false, + created_at: '2026-01-01', + updated_at: '2026-01-01', + ...overrides, + }; +} + +describe('useResolvedManagerName', () => { + it('resolves the name from the loaded list when the manager is present', () => { + const employees = [emp({ id: 'mgr-1', full_name: 'Priya Raman' })]; + const { result } = renderHook(() => + useResolvedManagerName('mgr-1', employees) + ); + expect(result.current).toBe('Priya Raman'); + }); + + it('fetches the manager by ID when it is absent from the filtered/capped list', async () => { + // #150: manager exists but is not in the loaded (filtered, limit-200) list. + mockCommands({ + get_employee: (args) => { + expect(args.id).toBe('mgr-hidden'); + return emp({ id: 'mgr-hidden', full_name: 'Priya Raman' }); + }, + }); + const employees = [emp({ id: 'emp-2', full_name: 'Someone Else' })]; + const { result } = renderHook(() => + useResolvedManagerName('mgr-hidden', employees) + ); + // Not in the list -> undefined initially, resolved after the fetch settles. + expect(result.current).toBeUndefined(); + await waitFor(() => expect(result.current).toBe('Priya Raman')); + }); + + it('returns undefined when there is no manager', () => { + const { result } = renderHook(() => useResolvedManagerName(undefined, [])); + expect(result.current).toBeUndefined(); + }); + + it('falls back to undefined (raw-ID caller path) when the fetch fails', async () => { + mockCommands({ + get_employee: () => { + throw new Error('not found'); + }, + }); + const { result } = renderHook(() => + useResolvedManagerName('mgr-dangling', [emp({ id: 'emp-2' })]) + ); + await waitFor(() => expect(result.current).toBeUndefined()); + }); +}); diff --git a/src/components/employees/detail/useResolvedManagerName.ts b/src/components/employees/detail/useResolvedManagerName.ts new file mode 100644 index 0000000..5fbddba --- /dev/null +++ b/src/components/employees/detail/useResolvedManagerName.ts @@ -0,0 +1,61 @@ +import { useEffect, useState } from 'react'; +import type { Employee } from '../../../lib/types'; +import { + getEmployee, + type EmployeeWithLatestRating, +} from '../../../lib/tauri-commands'; + +/** + * Resolve a manager's display name for the employee detail panel. + * + * The loaded employee list is filtered and capped (`listEmployeesWithRatings` + * runs with the active filter/search and a limit of 200), so a manager can + * legitimately be absent from it — e.g. the list is filtered to active staff + * while the manager is on leave, or the org exceeds the page cap. Before, the + * panel fell back to rendering the raw manager ID in that case (issue #150). + * + * When the manager is present in the loaded list we use it directly; when it is + * not, we fetch it by ID via the existing `get_employee` command (no backend + * change). Returns `undefined` until a name is available, so the caller only + * falls back to the raw ID for a genuinely dangling reference. + */ +export function useResolvedManagerName( + managerId: string | null | undefined, + employees: EmployeeWithLatestRating[] +): string | undefined { + const managerInList = managerId + ? employees.find((e) => e.id === managerId) + : undefined; + + const [fetchedManager, setFetchedManager] = useState(null); + + useEffect(() => { + // Resolvable from the loaded list, or no manager at all — nothing to fetch. + if (!managerId || managerInList) { + setFetchedManager(null); + return; + } + + // Reset before fetching: without this, switching to another employee whose + // manager also needs a fetch would briefly show the PREVIOUS employee's + // fetched manager while the new request is in flight. + setFetchedManager(null); + + let cancelled = false; + getEmployee(managerId) + .then((emp) => { + if (!cancelled) setFetchedManager(emp); + }) + .catch(() => { + // Genuinely dangling reference — leave null so the caller falls back + // to the raw ID (mirrors the backend's skip-not-fatal behavior). + if (!cancelled) setFetchedManager(null); + }); + + return () => { + cancelled = true; + }; + }, [managerId, managerInList]); + + return (managerInList ?? fetchedManager)?.full_name; +}