From 0b1f795dee51aa46dfba4f74ed2a3d0eaa4f4524 Mon Sep 17 00:00:00 2001 From: Matt OD Date: Wed, 15 Jul 2026 01:26:27 -0700 Subject: [PATCH 1/2] =?UTF-8?q?chore:=20resolve=20issue=20#150=20=E2=80=94?= =?UTF-8?q?=20resolve=20manager=20name=20by=20ID=20when=20absent=20from=20?= =?UTF-8?q?loaded=20list?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The employee list is loaded via listEmployeesWithRatings with the active filter/search and a 200-row cap, so a manager can be missing from it (e.g. filtered to active while the manager is on leave, or beyond the cap). The detail panel then fell back to rendering the raw manager ID. Add useResolvedManagerName: use the manager from the loaded list when present, otherwise fetch by ID via the existing get_employee command (no backend change). Returns undefined until resolved, so the raw ID shows only for a genuinely dangling reference. Hook is called before the early return to satisfy the Rules of Hooks. Verification: 72 frontend tests pass (4 new covering in-list, fetch-when-absent, no-manager, and fetch-failure fallback), tsc --noEmit clean. Resolves #150. Co-Authored-By: Claude Opus 4.8 (1M context) --- src/components/employees/EmployeeDetail.tsx | 14 ++-- .../detail/useResolvedManagerName.test.ts | 64 +++++++++++++++++++ .../detail/useResolvedManagerName.ts | 56 ++++++++++++++++ 3 files changed, 130 insertions(+), 4 deletions(-) create mode 100644 src/components/employees/detail/useResolvedManagerName.test.ts create mode 100644 src/components/employees/detail/useResolvedManagerName.ts 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..c6dc7ad --- /dev/null +++ b/src/components/employees/detail/useResolvedManagerName.ts @@ -0,0 +1,56 @@ +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; + } + + 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; +} From 9db62d15ce0f3e2cf7a58dc9b1b2453ac8a6cc11 Mon Sep 17 00:00:00 2001 From: Matt OD Date: Wed, 15 Jul 2026 08:26:28 -0700 Subject: [PATCH 2/2] fix: reset fetched manager before a new fetch (review finding) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Switching between two employees whose managers both need fetch-resolution briefly showed the previous employee's manager name while the new request was in flight — the fetch branch never cleared the prior result. Co-Authored-By: Claude Fable 5 --- src/components/employees/detail/useResolvedManagerName.ts | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/src/components/employees/detail/useResolvedManagerName.ts b/src/components/employees/detail/useResolvedManagerName.ts index c6dc7ad..5fbddba 100644 --- a/src/components/employees/detail/useResolvedManagerName.ts +++ b/src/components/employees/detail/useResolvedManagerName.ts @@ -36,6 +36,11 @@ export function useResolvedManagerName( 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) => {