From 3dab677524fd65005081339fc0678cc28a604281 Mon Sep 17 00:00:00 2001 From: Jason Woltje Date: Sun, 22 Feb 2026 18:51:46 -0600 Subject: [PATCH] fix: remediate dashboard API integration review blockers (#459) - Fix race condition: guard useEffect when workspaceId is null, prevent infinite loading state by setting isLoading=false on null workspace - Fix TypeScript strict typing: @Workspace() returns string|undefined, controller now matches with BadRequestException guard - Narrow details DTO type from unknown to Record|null - Add error state UI for API fetch failures - Add error-path test with static mock import pattern Co-Authored-By: Claude Opus 4.6 --- .../api/src/dashboard/dashboard.controller.ts | 8 +++-- apps/api/src/dashboard/dashboard.service.ts | 2 +- .../dashboard/dto/dashboard-summary.dto.ts | 2 +- .../web/src/app/(authenticated)/page.test.tsx | 22 ++++++++++++++ apps/web/src/app/(authenticated)/page.tsx | 30 +++++++++++++++++-- apps/web/src/lib/api/dashboard.ts | 2 +- docs/TASKS.md | 18 +++++------ 7 files changed, 68 insertions(+), 16 deletions(-) diff --git a/apps/api/src/dashboard/dashboard.controller.ts b/apps/api/src/dashboard/dashboard.controller.ts index 58f4454..57515ca 100644 --- a/apps/api/src/dashboard/dashboard.controller.ts +++ b/apps/api/src/dashboard/dashboard.controller.ts @@ -1,8 +1,9 @@ -import { Controller, Get, UseGuards } from "@nestjs/common"; +import { Controller, Get, UseGuards, BadRequestException } from "@nestjs/common"; import { DashboardService } from "./dashboard.service"; import { AuthGuard } from "../auth/guards/auth.guard"; import { WorkspaceGuard, PermissionGuard } from "../common/guards"; import { Workspace, Permission, RequirePermission } from "../common/decorators"; +import type { DashboardSummaryDto } from "./dto"; /** * Controller for dashboard endpoints. @@ -25,7 +26,10 @@ export class DashboardController { */ @Get("summary") @RequirePermission(Permission.WORKSPACE_ANY) - async getSummary(@Workspace() workspaceId: string) { + async getSummary(@Workspace() workspaceId: string | undefined): Promise { + if (!workspaceId) { + throw new BadRequestException("Workspace context required"); + } return this.dashboardService.getSummary(workspaceId); } } diff --git a/apps/api/src/dashboard/dashboard.service.ts b/apps/api/src/dashboard/dashboard.service.ts index 96ae855..049d6f1 100644 --- a/apps/api/src/dashboard/dashboard.service.ts +++ b/apps/api/src/dashboard/dashboard.service.ts @@ -142,7 +142,7 @@ export class DashboardService { action: row.action, entityType: row.entityType, entityId: row.entityId, - details: row.details, + details: row.details as Record | null, userId: row.userId, createdAt: row.createdAt.toISOString(), })); diff --git a/apps/api/src/dashboard/dto/dashboard-summary.dto.ts b/apps/api/src/dashboard/dto/dashboard-summary.dto.ts index 9feef5c..502f7ec 100644 --- a/apps/api/src/dashboard/dto/dashboard-summary.dto.ts +++ b/apps/api/src/dashboard/dto/dashboard-summary.dto.ts @@ -17,7 +17,7 @@ export class RecentActivityDto { action!: string; entityType!: string; entityId!: string; - details!: unknown; + details!: Record | null; userId!: string; createdAt!: string; } diff --git a/apps/web/src/app/(authenticated)/page.test.tsx b/apps/web/src/app/(authenticated)/page.test.tsx index 27383bc..3d72378 100644 --- a/apps/web/src/app/(authenticated)/page.test.tsx +++ b/apps/web/src/app/(authenticated)/page.test.tsx @@ -1,6 +1,7 @@ import { describe, it, expect, vi, beforeEach } from "vitest"; import { render, screen, waitFor } from "@testing-library/react"; import DashboardPage from "./page"; +import { fetchDashboardSummary } from "@/lib/api/dashboard"; // Mock Phase 3 dashboard widgets vi.mock("@/components/dashboard/DashboardMetrics", () => ({ @@ -51,6 +52,19 @@ vi.mock("@/lib/api/dashboard", () => ({ describe("DashboardPage", (): void => { beforeEach((): void => { vi.clearAllMocks(); + vi.mocked(fetchDashboardSummary).mockResolvedValue({ + metrics: { + activeAgents: 5, + tasksCompleted: 42, + totalTasks: 100, + tasksInProgress: 10, + activeProjects: 3, + errorRate: 0.5, + }, + recentActivity: [], + activeJobs: [], + tokenBudget: [], + }); }); it("should render the DashboardMetrics widget", async (): Promise => { @@ -87,4 +101,12 @@ describe("DashboardPage", (): void => { expect(screen.getByTestId("token-budget")).toBeInTheDocument(); }); }); + + it("should render error state when API fails", async (): Promise => { + vi.mocked(fetchDashboardSummary).mockRejectedValueOnce(new Error("Network error")); + render(); + await waitFor((): void => { + expect(screen.getByText("Failed to load dashboard data")).toBeInTheDocument(); + }); + }); }); diff --git a/apps/web/src/app/(authenticated)/page.tsx b/apps/web/src/app/(authenticated)/page.tsx index 3a55688..2105b48 100644 --- a/apps/web/src/app/(authenticated)/page.tsx +++ b/apps/web/src/app/(authenticated)/page.tsx @@ -15,19 +15,30 @@ export default function DashboardPage(): ReactElement { const workspaceId = useWorkspaceId(); const [data, setData] = useState(null); const [isLoading, setIsLoading] = useState(true); + const [error, setError] = useState(null); useEffect(() => { + if (!workspaceId) { + setIsLoading(false); + return; + } + + const wsId = workspaceId; let cancelled = false; + setError(null); + setIsLoading(true); async function loadSummary(): Promise { try { - const summary = await fetchDashboardSummary(workspaceId ?? undefined); + const summary = await fetchDashboardSummary(wsId); if (!cancelled) { setData(summary); } } catch (err: unknown) { - // Log but do not crash; widgets will render with empty states console.error("[Dashboard] Failed to fetch summary:", err); + if (!cancelled) { + setError("Failed to load dashboard data"); + } } finally { if (!cancelled) { setIsLoading(false); @@ -62,6 +73,21 @@ export default function DashboardPage(): ReactElement { return (
+ {error && ( +
+ {error} +
+ )}
diff --git a/apps/web/src/lib/api/dashboard.ts b/apps/web/src/lib/api/dashboard.ts index ea2851e..6994bd5 100644 --- a/apps/web/src/lib/api/dashboard.ts +++ b/apps/web/src/lib/api/dashboard.ts @@ -23,7 +23,7 @@ export interface RecentActivity { action: string; entityType: string; entityId: string; - details: unknown; + details: Record | null; userId: string; createdAt: string; } diff --git a/docs/TASKS.md b/docs/TASKS.md index 3661c29..a27eca0 100644 --- a/docs/TASKS.md +++ b/docs/TASKS.md @@ -2,12 +2,12 @@ > Single-writer: orchestrator only. Workers read but never modify. -| id | status | milestone | description | pr | notes | -| --------- | ----------- | --------- | ------------------------------------------------------------------------------------------------------------------------------------ | ---- | ------------------------------------------- | -| MS-P1-001 | done | phase-1 | Fix broken test suites: Button.test.tsx (4 fails, old Tailwind classes) + page.test.tsx (5 fails, old widget refs) | #458 | issue #457, commit 8fa0b30 | -| MS-P1-002 | done | phase-1 | Remove legacy unused dashboard widgets: DomainOverviewWidget, RecentTasksWidget, UpcomingEventsWidget, QuickCaptureWidget | #458 | issue #457, commit 8fa0b30, 5 files deleted | -| MS-P1-003 | done | phase-1 | Visual + theme polish: audit current vs design reference, fix gaps, verify dark/light across all components, responsive verification | #458 | issue #457, commit d97a98b, review: approve | -| MS-P1-004 | done | phase-1 | Phase verification: all quality gates pass (lint 8/8, typecheck 7/7, test 8/8) | #458 | issue #457, merged 07f5225, issue closed | -| MS-P2-001 | not-started | phase-2 | Create dashboard summary API endpoint: aggregate task counts, project counts, recent activity, active jobs in single call | — | issue #459, est 20K | -| MS-P2-002 | not-started | phase-2 | Wire dashboard widgets to real API data: ActivityFeed, DashboardMetrics, OrchestratorSessions replace mock with API calls | — | issue #459, est 25K, depends MS-P2-001 | -| MS-P2-003 | not-started | phase-2 | Phase verification: create task via API, confirm visible in dashboard, all quality gates pass | — | issue #459, est 10K, depends MS-P2-002 | +| id | status | milestone | description | pr | notes | +| --------- | ----------- | --------- | ------------------------------------------------------------------------------------------------------------------------------------ | ---- | ---------------------------------------------- | +| MS-P1-001 | done | phase-1 | Fix broken test suites: Button.test.tsx (4 fails, old Tailwind classes) + page.test.tsx (5 fails, old widget refs) | #458 | issue #457, commit 8fa0b30 | +| MS-P1-002 | done | phase-1 | Remove legacy unused dashboard widgets: DomainOverviewWidget, RecentTasksWidget, UpcomingEventsWidget, QuickCaptureWidget | #458 | issue #457, commit 8fa0b30, 5 files deleted | +| MS-P1-003 | done | phase-1 | Visual + theme polish: audit current vs design reference, fix gaps, verify dark/light across all components, responsive verification | #458 | issue #457, commit d97a98b, review: approve | +| MS-P1-004 | done | phase-1 | Phase verification: all quality gates pass (lint 8/8, typecheck 7/7, test 8/8) | #458 | issue #457, merged 07f5225, issue closed | +| MS-P2-001 | done | phase-2 | Create dashboard summary API endpoint: aggregate task counts, project counts, recent activity, active jobs in single call | — | issue #459, commit e38aaa9, 7 files +430 lines | +| MS-P2-002 | done | phase-2 | Wire dashboard widgets to real API data: ActivityFeed, DashboardMetrics, OrchestratorSessions replace mock with API calls | — | issue #459, commit 7c762e6 + remediation | +| MS-P2-003 | not-started | phase-2 | Phase verification: create task via API, confirm visible in dashboard, all quality gates pass | — | issue #459, est 10K, depends MS-P2-002 |