feat: dashboard revamp with KPIs, charts, and widgets#18
Merged
Systemsaholic merged 16 commits intomainfrom Mar 4, 2026
Merged
feat: dashboard revamp with KPIs, charts, and widgets#18Systemsaholic merged 16 commits intomainfrom
Systemsaholic merged 16 commits intomainfrom
Conversation
Widget-based dashboard with KPI cards, charts, trip widgets, task/payment tracking, and collapsible sidebar. Supports agent (personal) and admin (agency-wide) views. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
…arts, and widgets Add full getOverview() method to DashboardService with parallel query execution for: - KPI metrics (bookings, net sales, commission) with period-over-period trends - Recent trips and leaving-soon widgets with traveler counts - Tasks due with overdue detection and linked entity names - Payments due with join chain to trip names - Monthly sales/commission charts with optional YoY comparison - Current month projection based on daily run rate - Agent leaderboard (admin only) with sales volume and booking counts Injects TripAccessService and TaskAccessService for RBAC-scoped queries. Agents see only their accessible trips; admins see agency-wide data. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
- Add GET /dashboard/overview controller endpoint with Swagger docs - Create useDashboardOverview hook with query key factory and types Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
- KPI cards with trend indicators (personal + agency variants) - Trip card + scrollable trip card row (Jump Back In, Leaving Soon) - Tasks Due widget with inline completion - Payments Due widget with overdue highlighting - Sales + Commission bar charts with YoY overlay and projection toggles - Collapsible sidebar: quick-create, mini calendar, today's tasks, agent leaderboard Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
…rors Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Use keepPreviousData so React Query shows stale data while refetching with a new query key. Only show skeleton on true initial load (isPending). Show a subtle spinner in the header during background refetches. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
- Fix quick-create links to use existing routes (/trips?create=true) - Separate admin personal KPIs from agency KPIs (scope to owner_id) - Filter tasks-due to only include tasks with a due date set - Invalidate dashboard cache when completing a task Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
The trips table displayed all agents as "Unassigned" because findAll() only returned ownerId (UUID) without joining user_profiles. Now batch- loads owner profiles and populates the owner field with name and email. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Read localStorage in useEffect instead of useState initializer to avoid server/client mismatch. Server always renders kanban (default), then client hydrates to the stored preference. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
PostgreSQL ANY() needs an explicit ::text[] cast when Drizzle binds a string[] parameter, otherwise it's treated as a scalar causing a query failure. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Replace ANY() with IN() for tag filter query - Drizzle binds JS arrays as scalar params which breaks ANY(). Add keepPreviousData to useTrips so filter changes don't flash a loading skeleton. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
📝 WalkthroughWalkthroughThis pull request implements a comprehensive dashboard revamp, introducing a new backend endpoint Changes
Sequence DiagramsequenceDiagram
participant Admin as Admin Client
participant Hook as useDashboardOverview
participant API as GET /dashboard/overview
participant Service as DashboardService
participant DB as Database
participant Cache as React Query Cache
Admin->>Hook: useDashboardOverview({ period, year, includeYoy })
Hook->>Cache: Check query cache
alt Cache miss or stale
Hook->>API: GET /dashboard/overview?period=...&year=...&includeYoy=...
API->>Service: getOverview(auth, query)
par Parallel Data Fetching
Service->>DB: Fetch KPI metrics (bookings, sales, commission)
Service->>DB: Fetch recent trips & leaving soon
Service->>DB: Fetch tasks due & payments due
Service->>DB: Fetch monthly sales/commission data
Service->>DB: Fetch current month projections
Service->>DB: Fetch agent leaderboard (if admin)
and
DB-->>Service: KPI data
DB-->>Service: Trip summaries
DB-->>Service: Task & payment summaries
DB-->>Service: Monthly chart data
DB-->>Service: Projection data
DB-->>Service: Leaderboard entries
end
Service->>Service: Compute trends, format response
Service-->>API: DashboardOverview payload
API-->>Hook: Structured response
Hook->>Cache: Store DashboardOverview
end
Hook-->>Admin: { data, isPending, isError }
Admin->>Admin: Render KPI cards, charts, widgets, sidebar from data
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches
🧪 Generate unit tests (beta)
Tip Try Coding Plans. Let us write the prompt for your AI agent so you can ship faster (with fewer bugs). Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 19
🧹 Nitpick comments (5)
apps/admin/src/app/dashboard/_components/sidebar-agent-leaderboard.tsx (1)
15-17: Make initials generation resilient to missing name fields.
getInitialscurrently assumes both names are present. A defensive fallback avoids blank badges (and runtime surprises if nulls slip through).♻️ Proposed refactor
-function getInitials(firstName: string, lastName: string): string { - return `${firstName.charAt(0)}${lastName.charAt(0)}`.toUpperCase() +function getInitials(firstName?: string | null, lastName?: string | null): string { + const first = firstName?.trim().charAt(0) ?? '' + const last = lastName?.trim().charAt(0) ?? '' + return (first + last).toUpperCase() || 'A' }Also applies to: 43-45
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/admin/src/app/dashboard/_components/sidebar-agent-leaderboard.tsx` around lines 15 - 17, getInitials currently assumes firstName and lastName are always present; make it defensive by normalizing inputs (e.g., const f = (firstName || '').trim(); const l = (lastName || '').trim()), then build initials from whichever name parts exist (first char of f and/or first char of l), fallback to a single letter or empty string if both are blank, and always return .toUpperCase(); update any other duplicate implementations/usages of getInitials (the other initials generation sites) to use this resilient logic.docs/plans/2026-03-03-dashboard-revamp-implementation.md (4)
830-856: Expand verification to include functional and RBAC testing.The verification section only includes typechecks and a backward compatibility test. Given the complexity of this dashboard feature with role-based views (admin vs agent), parallel queries, and multiple data sources, consider adding:
Additional verification steps to add:
RBAC verification: Test that agents only see their own trips and admin sees agency-wide data
# Test as agent user curl -H "Authorization: Bearer $AGENT_TOKEN" \ http://localhost:3101/api/v1/dashboard/overview | \ jq '.agency' # Should be null for agents # Test as admin user curl -H "Authorization: Bearer $ADMIN_TOKEN" \ http://localhost:3101/api/v1/dashboard/overview | \ jq '.agency' # Should have data for adminsPerformance baseline: Measure response time for the expensive endpoint
time curl -s http://localhost:3101/api/v1/dashboard/overview > /dev/null # Should complete in < 500ms for reasonable data volumesEdge case testing: Verify behavior with no data, first day of month (projection), first year in system (no YoY data)
Frontend integration test: Verify all widgets render with test data
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@docs/plans/2026-03-03-dashboard-revamp-implementation.md` around lines 830 - 856, Add functional and RBAC checks, performance baseline, edge-case, and frontend integration steps to the verification section: include RBAC curl tests for the /api/v1/dashboard/overview endpoint using AGENT_TOKEN and ADMIN_TOKEN to assert agents see null agency and admins see agency data (reference endpoint /api/v1/dashboard/overview), add a performance check measuring response time (e.g., time curl) with an acceptance threshold (e.g., <500ms) for the expensive overview endpoint, add edge-case checks for empty DB, first-day-of-month and first-year-of-system scenarios verifying projections and absence of YoY data, and add a frontend integration step that loads the dashboard UI with test fixtures to assert all widgets render; update the verification checklist in the Task 13 section to include these items and commit the changes.
268-284: Consider rate limiting for this expensive endpoint.The
/dashboard/overviewendpoint executes 9+ parallel database queries and could be resource-intensive. While the dashboard likely has low request frequency, consider implementing rate limiting to prevent abuse or accidental DoS from aggressive client-side polling.Example approaches:
- Use NestJS
@Throttle()decorator from@nestjs/throttler- Configure API gateway rate limits
- Add CDN caching for admin users with 30-60 second TTL
The frontend hook already sets
staleTime: 30_000(line 450), which provides some client-side throttling.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@docs/plans/2026-03-03-dashboard-revamp-implementation.md` around lines 268 - 284, The GET /dashboard/overview controller method (getOverview) is expensive and needs server-side rate limiting; add a throttling mechanism (e.g., NestJS `@Throttle` on getOverview or controller, or integrate a global ThrottlerGuard) and/or short-lived cache around dashboardService.getOverview keyed by user+query to avoid repeated DB hits; update the controller method (getOverview) to use `@Throttle`(...) or a caching decorator/middleware and ensure DashboardOverviewQueryDto-based query params are included in the cache key or throttling scope so you protect this expensive endpoint from client polling/abuse.
331-415: Consider type sharing to avoid duplication.The type definitions (lines 331-415) duplicate the interfaces from the backend DTO (
apps/api/src/dashboard/dto/dashboard-overview.dto.ts). This creates a maintenance risk where changes to the backend types might not be reflected in the frontend, leading to type drift.Consider one of these approaches:
- Shared types package: Create a
@tailfire/typespackage for shared API contracts- Type generation: Use a tool like
ts-restor OpenAPI code generation to auto-generate frontend types from backend DTOs- Type import: If using a monorepo setup that allows cross-package type imports, import directly from the backend (though this can create build dependencies)
For now, the duplication is acceptable but should be noted as technical debt.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@docs/plans/2026-03-03-dashboard-revamp-implementation.md` around lines 331 - 415, The frontend duplicates backend DTOs (KpiMetrics, TripSummary, TaskDueSummary, PaymentDueSummary, MonthlySalesData, MonthlyCommissionData, ProjectionData, AgentLeaderboardEntry, DashboardOverview); remove this duplication by moving these interfaces into a shared contract and importing them: either (1) create a `@tailfire/types` package and export the interfaces used by the dashboard, update frontend imports of KpiMetrics/TripSummary/.../DashboardOverview to import from that package, (2) or add a generation step (ts-rest/OpenAPI) to produce these types from apps/api/src/dashboard/dto/dashboard-overview.dto.ts and point frontend imports to the generated types, or (3) if monorepo permits, import the DTOs directly from the backend package; then delete the local interface copies in docs/plans/2026-03-03-dashboard-revamp-implementation.md.
440-451: Consider addingplaceholderDataoption mentioned in PR objectives.The PR description mentions using "keepPreviousData to avoid full-page skeleton flashes" when toggling filters, but this implementation plan doesn't include that option in the
useQueryconfiguration.In React Query v5 (which uses TanStack Query),
keepPreviousDatahas been replaced withplaceholderData:return useQuery({ queryKey: dashboardKeys.overview(period, chartYear, includeYoy), queryFn: () => { const params = new URLSearchParams({ period, chartYear: String(chartYear), includeYoy: String(includeYoy), }) return api.get<DashboardOverview>(`/dashboard/overview?${params}`) }, staleTime: 30_000, + placeholderData: (previousData) => previousData, })This prevents the UI from showing full skeleton screens when toggling between MTD/YTD or enabling YoY comparisons.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@docs/plans/2026-03-03-dashboard-revamp-implementation.md` around lines 440 - 451, The useQuery call for dashboard overview is missing a placeholderData option to retain the previous result while a new fetch is in-flight; add a placeholderData that returns the previous cached data for dashboardKeys.overview so the UI can reuse the prior DashboardOverview until the new response arrives (use the same queryKey: dashboardKeys.overview(period, chartYear, includeYoy) and queryClient.getQueryData(...) inside placeholderData to fetch and return the previous data).
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@apps/admin/src/app/dashboard/_components/dashboard-sidebar.tsx`:
- Around line 18-21: Update the DashboardSidebar API to match the Sheet
contract: in the DashboardSidebarProps interface replace onToggle: () => void
with onOpenChange: (open: boolean) => void, update the component destructuring
to use onOpenChange instead of onToggle, keep passing that handler into the
Sheet component's onOpenChange prop, and change the desktop close button handler
(previously onClick={onToggle}) to explicitly call onOpenChange(false). Also
update the parent (dashboard/page.tsx) to pass a state-setter style callback
(e.g., setIsOpen) or a function that accepts the boolean open argument rather
than a toggle-only function.
In `@apps/admin/src/app/dashboard/_components/payments-due-widget.tsx`:
- Line 4: The code is using new Date(payment.dueDate) which will shift date-only
strings by timezone; replace those usages by parsing the API date string with
date-fns parse using the explicit format 'yyyy-MM-dd' (e.g., call
parse(payment.dueDate, 'yyyy-MM-dd', new Date())) then pass that parsed Date
into format(...) where currently new Date(...) is used; update all occurrences
(e.g., in the PaymentsDueWidget component where payment.dueDate is converted)
and add the parse import from date-fns.
In `@apps/admin/src/app/dashboard/_components/sales-chart.tsx`:
- Around line 67-71: The year <select> in the SalesChart component is missing an
accessible label; add one by either adding an associated <label> (create a
unique id for the select and render <label htmlFor="{id}">Year</label>) or by
adding an aria-label/aria-labelledby attribute on the <select> that references a
visible label or the text "Year"; update the element using the identifiers
around value={year} and onChange={onYearChange} so screen readers can announce
its purpose.
In `@apps/admin/src/app/dashboard/_components/sidebar-today-tasks.tsx`:
- Around line 15-16: todayStr is computed in UTC and used to filter tasks
(todayTasks = tasks.filter(...)), which misclassifies tasks for users in non-UTC
timezones; replace that with a local-day string (e.g. build YYYY-MM-DD from new
Date().getFullYear(), getMonth()+1, getDate() or use
toLocaleDateString('en-CA')) and use that value when creating todayTasks via
tasks.filter. Also change the checkbox onChange handler so the completion
mutation is only invoked when the checkbox becomes checked (guard the mutation
call behind event.target.checked or the checked boolean) instead of firing on
both check and uncheck transitions.
In `@apps/admin/src/app/dashboard/_components/tasks-due-widget.tsx`:
- Around line 45-50: The Checkbox onCheckedChange handler in the TasksDueWidget
currently calls completeTask.mutate on every toggle (in the Checkbox component),
which triggers on uncheck and corrupts audit fields; change the handler in the
Checkbox used inside TasksDueWidget so it only calls completeTask.mutate when
the new checked value is truthy (checked === true), early-returns otherwise, and
guard against concurrent calls by checking completeTask.isLoading before
mutating; also bind the Checkbox disabled prop to completeTask.isLoading (or a
local pending state) so the box is disabled while the mutation is in flight.
- Around line 68-71: Replace the usage of new Date(task.dueDate) with date-fns'
parseISO to avoid timezone shifts: import parseISO from 'date-fns/parseISO' (or
add to existing date-fns imports) and change the formatter call in the
TasksDueWidget rendering where format(new Date(task.dueDate), 'MMM d') is used
to format(parseISO(task.dueDate), 'MMM d'); ensure you update any relevant
imports so parseISO is available and remove new Date usage.
In `@apps/admin/src/app/dashboard/_components/trip-card.tsx`:
- Around line 29-31: The date formatting in the TripCard component currently
uses format(new Date(trip.startDate)) and format(new Date(trip.endDate)), which
treats "YYYY-MM-DD" as UTC midnight and can shift the displayed calendar day;
replace those new Date(...) calls with a local-date constructor (e.g., implement
a small helper like parseLocalDate(dateStr) that splits "YYYY-MM-DD" into year,
month, day and returns new Date(y, m-1, d)) and call
format(parseLocalDate(trip.startDate)) / format(parseLocalDate(trip.endDate));
alternatively, if you need explicit timezone handling, use date-fns-tz parsing
with the user's timezone and pass the timezone-aware Date into format.
In `@apps/admin/src/app/dashboard/page.tsx`:
- Around line 124-144: The MTD/YTD buttons and the icon-only sidebar toggle lack
accessibility semantics: update the two period buttons rendered where setPeriod
and period are used to include aria-pressed attributes (e.g.,
aria-pressed={period === 'mtd'} and aria-pressed={period === 'ytd'}) so screen
readers receive the pressed state, and add an accessible name and expanded state
to the icon-only toggle (the Button that calls sidebar.toggle and renders
<PanelRight />) by adding aria-label="Toggle sidebar" and, if available,
aria-expanded={sidebar.open} (or aria-pressed if no open state exists); keep the
existing onClick handlers (setPeriod and sidebar.toggle) unchanged.
In `@apps/api/src/dashboard/dashboard.service.ts`:
- Around line 148-149: The admin personal widgets currently pass the unscoped
tripIds ('all') to getRecentTrips and getLeavingSoon; update the calls to pass
personalTripIds instead so the widgets are limited to the admin's personal
trips. Locate where getRecentTrips(...) and getLeavingSoon(...) are invoked
(they currently receive tripIds) and replace those arguments with
personalTripIds (the same ID set used for admin personal KPIs) so both recent
and leaving-soon widgets use the personal scope.
- Around line 193-197: The prior-period range currently uses the full previous
month (priorStartDate/priorEndDate), which misaligns MTD comparisons; change
priorEndDate to match the same day-of-month as "now" in the prior month (i.e.,
set priorEndDate to new Date(now.getFullYear(), now.getMonth()-1,
min(now.getDate(), daysInPriorMonth))) and compute daysInPriorMonth via new
Date(priorYear, priorMonth+1, 0).getDate(); keep priorStartDate as the 1st of
the prior month and ensure any inclusive/exclusive semantics match the current
MTD range used for calculations.
- Around line 220-225: The code builds SQL IN lists by concatenating tripIds
into sql.raw which risks SQL injection and malformed SQL; replace those usages
(e.g., the tripFilter declaration and the repeated patterns at the other listed
locations) with parameterized construction using sql.join over tripIds mapped to
sql`${id}` so the DB driver binds each id as a parameter (for example, replace
sql.raw(`('${tripIds.join("','")}')`) with sql.join(tripIds.map(id =>
sql`${id}`), sql`,`) inside the sql`... IN (...)` expression). Ensure you handle
the three branches (all, empty array -> false, non-empty -> parameterized IN)
consistently across tripFilter and the other occurrences.
In `@apps/api/src/trips/trips.service.ts`:
- Around line 409-433: The code unconditionally fetches and attaches owner
emails (see ownerIds, ownerMap and the this.db.client.select call) and then sets
owner: {id, name, email} in the returned trips map (inside trips.map using
this.mapToResponseDto), which can leak emails when the request lacks auth scope;
change the logic to guard both the DB select and the email field: only query for
email (omit email column or skip the select entirely) when the current request
has the proper auth scope/permission, and when building the owner object only
include the email property if permission is present (otherwise set owner to {id,
name} or undefined as before). Ensure the permission check is used before
executing the select and before adding email to the owner returned in the
trips.map result.
In `@docs/plans/2026-03-03-dashboard-revamp-design.md`:
- Around line 29-69: The markdown contains two fenced code blocks rendered as
ASCII art that lack language identifiers (triggering markdownlint MD040); update
each opening fence (the triple backticks surrounding the dashboard ASCII
diagrams) to include a language token such as text or asciidoc (e.g., replace
``` with ```text) so both fenced blocks comply with MD040 and tooling; this
change should be applied to both occurrences referenced in the diff.
- Around line 187-192: Add a blank line before and after the query-parameters
markdown table (the block starting with the header "Param | Type | Default |
Description" and the rows for `period`, `chartYear`, and `includeYoy`) so the
table is surrounded by empty lines to satisfy MD058; edit the surrounding
content in docs/plans/2026-03-03-dashboard-revamp-design.md to insert one blank
line above the table and one blank line below it.
In `@docs/plans/2026-03-03-dashboard-revamp-implementation.md`:
- Around line 548-565: The TasksDueWidget guidance omits invalidating
dashboard-related caches after calling useCompleteTask; update the
TasksDueWidget spec to call queryClient.invalidateQueries for both taskKeys.all
and dashboardKeys.all in the useCompleteTask onSuccess handler so the task list
and KPIs refresh immediately after completion (reference the TasksDueWidget prop
shape, the useCompleteTask mutation, and use of queryClient.invalidateQueries
with taskKeys.all and dashboardKeys.all).
- Around line 214-245: The current guidance to use Promise.all() in getOverview
will cause a single failing query to fail the whole dashboard; change the
parallel execution to Promise.allSettled([...]) for the calls to getKpiMetrics,
getRecentTrips, getLeavingSoon, getTasksDue, getPaymentsDue, getMonthlySales,
getMonthlyCommission, getProjection and getAgentLeaderboard, then inspect each
settled result (status === 'fulfilled' ? value : fallback) and replace failures
with safe defaults (e.g., getEmptyKpiMetrics(), empty arrays or nulls) while
logging errors via this.logger.error(result.reason) so individual widget
failures degrade gracefully instead of bubbling up a full failure.
- Around line 691-697: Update the guidance for persisting the sidebar state to
show a hydration-safe pattern: explain that the Sidebar wrapper (props isOpen,
onToggle) must avoid direct localStorage reads during SSR, and recommend
initializing state for the sidebar (e.g., isOpen) to a SSR-safe default
(closed), then reading the saved value inside a client-only effect (useEffect)
after hydration and writing changes back to localStorage (key like
"dashboard-sidebar-open") inside the toggle handler (handleToggle or the zustand
store's setter); alternatively reference using an SSR-aware library such as
use-local-storage-state to encapsulate this behavior.
- Around line 229-234: The projection formula in the projection helper (uses
variables daysElapsed, totalDaysInMonth, actualSales, actualCommission and
computes salesProjectedCents / commissionProjectedDollars) can divide by zero or
produce bad early-month projections; add a guard in that helper to treat
daysElapsed < 2 (or another chosen small threshold) as insufficient data and
return projected values equal to actuals (i.e., salesProjected = actualSales and
commissionProjected = actualCommission) along with daysElapsed and
totalDaysInMonth, otherwise perform the existing safe projection calculation
downstream.
- Around line 66-72: The chartYear property currently sets a module-load default
via "chartYear?: number = new Date().getFullYear()", which freezes the year
until restart; remove the inline initializer so the DTO only declares
"chartYear?: number" (keep decorators `@ApiPropertyOptional`, `@IsOptional`, `@Type`,
`@IsInt`, `@Min`, `@Max`), and implement per-request defaulting in the
controller/service that consumes the DTO (e.g., use "const year =
query.chartYear ?? new Date().getFullYear()" inside the handler or service
method) so the current year is evaluated for each request.
---
Nitpick comments:
In `@apps/admin/src/app/dashboard/_components/sidebar-agent-leaderboard.tsx`:
- Around line 15-17: getInitials currently assumes firstName and lastName are
always present; make it defensive by normalizing inputs (e.g., const f =
(firstName || '').trim(); const l = (lastName || '').trim()), then build
initials from whichever name parts exist (first char of f and/or first char of
l), fallback to a single letter or empty string if both are blank, and always
return .toUpperCase(); update any other duplicate implementations/usages of
getInitials (the other initials generation sites) to use this resilient logic.
In `@docs/plans/2026-03-03-dashboard-revamp-implementation.md`:
- Around line 830-856: Add functional and RBAC checks, performance baseline,
edge-case, and frontend integration steps to the verification section: include
RBAC curl tests for the /api/v1/dashboard/overview endpoint using AGENT_TOKEN
and ADMIN_TOKEN to assert agents see null agency and admins see agency data
(reference endpoint /api/v1/dashboard/overview), add a performance check
measuring response time (e.g., time curl) with an acceptance threshold (e.g.,
<500ms) for the expensive overview endpoint, add edge-case checks for empty DB,
first-day-of-month and first-year-of-system scenarios verifying projections and
absence of YoY data, and add a frontend integration step that loads the
dashboard UI with test fixtures to assert all widgets render; update the
verification checklist in the Task 13 section to include these items and commit
the changes.
- Around line 268-284: The GET /dashboard/overview controller method
(getOverview) is expensive and needs server-side rate limiting; add a throttling
mechanism (e.g., NestJS `@Throttle` on getOverview or controller, or integrate a
global ThrottlerGuard) and/or short-lived cache around
dashboardService.getOverview keyed by user+query to avoid repeated DB hits;
update the controller method (getOverview) to use `@Throttle`(...) or a caching
decorator/middleware and ensure DashboardOverviewQueryDto-based query params are
included in the cache key or throttling scope so you protect this expensive
endpoint from client polling/abuse.
- Around line 331-415: The frontend duplicates backend DTOs (KpiMetrics,
TripSummary, TaskDueSummary, PaymentDueSummary, MonthlySalesData,
MonthlyCommissionData, ProjectionData, AgentLeaderboardEntry,
DashboardOverview); remove this duplication by moving these interfaces into a
shared contract and importing them: either (1) create a `@tailfire/types` package
and export the interfaces used by the dashboard, update frontend imports of
KpiMetrics/TripSummary/.../DashboardOverview to import from that package, (2) or
add a generation step (ts-rest/OpenAPI) to produce these types from
apps/api/src/dashboard/dto/dashboard-overview.dto.ts and point frontend imports
to the generated types, or (3) if monorepo permits, import the DTOs directly
from the backend package; then delete the local interface copies in
docs/plans/2026-03-03-dashboard-revamp-implementation.md.
- Around line 440-451: The useQuery call for dashboard overview is missing a
placeholderData option to retain the previous result while a new fetch is
in-flight; add a placeholderData that returns the previous cached data for
dashboardKeys.overview so the UI can reuse the prior DashboardOverview until the
new response arrives (use the same queryKey: dashboardKeys.overview(period,
chartYear, includeYoy) and queryClient.getQueryData(...) inside placeholderData
to fetch and return the previous data).
ℹ️ Review info
Configuration used: defaults
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (25)
apps/admin/package.jsonapps/admin/src/app/dashboard/_components/commission-chart.tsxapps/admin/src/app/dashboard/_components/dashboard-sidebar.tsxapps/admin/src/app/dashboard/_components/kpi-cards.tsxapps/admin/src/app/dashboard/_components/payments-due-widget.tsxapps/admin/src/app/dashboard/_components/sales-chart.tsxapps/admin/src/app/dashboard/_components/sidebar-agent-leaderboard.tsxapps/admin/src/app/dashboard/_components/sidebar-mini-calendar.tsxapps/admin/src/app/dashboard/_components/sidebar-quick-create.tsxapps/admin/src/app/dashboard/_components/sidebar-today-tasks.tsxapps/admin/src/app/dashboard/_components/tasks-due-widget.tsxapps/admin/src/app/dashboard/_components/trip-card-row.tsxapps/admin/src/app/dashboard/_components/trip-card.tsxapps/admin/src/app/dashboard/page.tsxapps/admin/src/app/trips/page.tsxapps/admin/src/hooks/use-dashboard.tsapps/admin/src/hooks/use-tasks.tsapps/admin/src/hooks/use-trips.tsapps/api/src/dashboard/dashboard.controller.tsapps/api/src/dashboard/dashboard.module.tsapps/api/src/dashboard/dashboard.service.tsapps/api/src/dashboard/dto/dashboard-overview.dto.tsapps/api/src/trips/trips.service.tsdocs/plans/2026-03-03-dashboard-revamp-design.mddocs/plans/2026-03-03-dashboard-revamp-implementation.md
Comment on lines
+18
to
+21
| interface DashboardSidebarProps { | ||
| isOpen: boolean | ||
| onToggle: () => void | ||
| isAdmin: boolean |
There was a problem hiding this comment.
🛠️ Refactor suggestion | 🟠 Major
🧩 Analysis chain
🏁 Script executed:
cat -n apps/admin/src/app/dashboard/_components/dashboard-sidebar.tsx | head -80Repository: Systemsaholic/tailfire
Length of output: 3377
🏁 Script executed:
rg -n -C5 'DashboardSidebar' apps/admin/src/app/dashboard/page.tsxRepository: Systemsaholic/tailfire
Length of output: 920
🏁 Script executed:
sed -n '21,50p' apps/admin/src/app/dashboard/page.tsxRepository: Systemsaholic/tailfire
Length of output: 878
🏁 Script executed:
fd -name 'sheet.tsx' -o -name 'sheet.ts' | head -5Repository: Systemsaholic/tailfire
Length of output: 296
🏁 Script executed:
fd 'sheet' apps components packages --type f -e tsx -e ts | grep -i sheet | head -10Repository: Systemsaholic/tailfire
Length of output: 409
🏁 Script executed:
cat apps/admin/src/components/ui/sheet.tsxRepository: Systemsaholic/tailfire
Length of output: 4346
🏁 Script executed:
# Verify parent pass-through needs to change too
sed -n '199,205p' apps/admin/src/app/dashboard/page.tsxRepository: Systemsaholic/tailfire
Length of output: 278
Replace toggle-only callback with explicit state-driven handler to match Sheet API contract.
The Sheet component's onOpenChange callback receives a boolean parameter indicating the next open state, but the current onToggle function ignores this parameter and simply toggles regardless. This violates the API contract and can cause state desynchronization in edge cases. Additionally, the desktop close button at line 75 should explicitly close the sidebar rather than toggle.
Changes required:
- Line 20: Rename
onToggle: () => voidtoonOpenChange: (open: boolean) => void - Line 39: Update destructuring to
onOpenChange - Line 55: Sheet already passes
onOpenChangecorrectly; just ensure prop name matches - Line 75: Change
onClick={onToggle}toonClick={() => onOpenChange(false)}to explicitly close
Parent code in dashboard/page.tsx will also need updating to pass a state-setter callback instead of a toggle function.
Proposed refactor
interface DashboardSidebarProps {
isOpen: boolean
- onToggle: () => void
+ onOpenChange: (open: boolean) => void
isAdmin: boolean
tasks: TaskDueSummary[]
leaderboard: AgentLeaderboardEntry[] | null
}
...
-export function DashboardSidebar({ isOpen, onToggle, isAdmin, tasks, leaderboard }: DashboardSidebarProps) {
+export function DashboardSidebar({ isOpen, onOpenChange, isAdmin, tasks, leaderboard }: DashboardSidebarProps) {
...
- <Sheet open={isOpen} onOpenChange={onToggle}>
+ <Sheet open={isOpen} onOpenChange={onOpenChange}>
...
- <Button variant="ghost" size="sm" className="h-6 w-6 p-0" onClick={onToggle}>
+ <Button variant="ghost" size="sm" className="h-6 w-6 p-0" onClick={() => onOpenChange(false)}>📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| interface DashboardSidebarProps { | |
| isOpen: boolean | |
| onToggle: () => void | |
| isAdmin: boolean | |
| interface DashboardSidebarProps { | |
| isOpen: boolean | |
| onOpenChange: (open: boolean) => void | |
| isAdmin: boolean |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@apps/admin/src/app/dashboard/_components/dashboard-sidebar.tsx` around lines
18 - 21, Update the DashboardSidebar API to match the Sheet contract: in the
DashboardSidebarProps interface replace onToggle: () => void with onOpenChange:
(open: boolean) => void, update the component destructuring to use onOpenChange
instead of onToggle, keep passing that handler into the Sheet component's
onOpenChange prop, and change the desktop close button handler (previously
onClick={onToggle}) to explicitly call onOpenChange(false). Also update the
parent (dashboard/page.tsx) to pass a state-setter style callback (e.g.,
setIsOpen) or a function that accepts the boolean open argument rather than a
toggle-only function.
| 'use client' | ||
|
|
||
| import Link from 'next/link' | ||
| import { format } from 'date-fns' |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
# Verify producer/consumer of dueDate formatting.
rg -n "String\\(row\\.due_date\\)|new Date\\(payment\\.dueDate\\)|parseISO\\(payment\\.dueDate\\)" \
apps/api/src/dashboard/dashboard.service.ts \
apps/admin/src/app/dashboard/_components/payments-due-widget.tsxRepository: Systemsaholic/tailfire
Length of output: 305
🏁 Script executed:
# Also check the full context around the problematic lines
cat -n apps/admin/src/app/dashboard/_components/payments-due-widget.tsx | sed -n '1,65p'Repository: Systemsaholic/tailfire
Length of output: 3034
🏁 Script executed:
# Check the API response and how the due_date is constructed
rg -B5 -A5 "due_date" apps/api/src/dashboard/dashboard.service.ts | head -50Repository: Systemsaholic/tailfire
Length of output: 1557
Avoid new Date() for date-only strings; use parse() to prevent timezone-dependent date shifts.
new Date(payment.dueDate) interprets date-only strings (e.g., "2024-12-25") as UTC midnight. In Western timezones (UTC-5 to UTC-8), this renders the previous day. The API returns date-only strings from PostgreSQL's date type, so use explicit format parsing.
💡 Suggested fix
-import { format } from 'date-fns'
+import { format, parse } from 'date-fns'
...
- {format(new Date(payment.dueDate), 'MMM d')}
+ {format(parse(payment.dueDate, 'yyyy-MM-dd', new Date()), 'MMM d')}Also applies to: 56-59
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@apps/admin/src/app/dashboard/_components/payments-due-widget.tsx` at line 4,
The code is using new Date(payment.dueDate) which will shift date-only strings
by timezone; replace those usages by parsing the API date string with date-fns
parse using the explicit format 'yyyy-MM-dd' (e.g., call parse(payment.dueDate,
'yyyy-MM-dd', new Date())) then pass that parsed Date into format(...) where
currently new Date(...) is used; update all occurrences (e.g., in the
PaymentsDueWidget component where payment.dueDate is converted) and add the
parse import from date-fns.
Comment on lines
+67
to
+71
| <select | ||
| value={year} | ||
| onChange={(e) => onYearChange(Number(e.target.value))} | ||
| className="text-xs border rounded px-2 py-1" | ||
| > |
There was a problem hiding this comment.
Label the year selector for screen readers.
The year <select> has no accessible label, so its purpose is ambiguous to assistive technologies.
💡 Suggested fix
<select
+ aria-label="Select sales year"
value={year}
onChange={(e) => onYearChange(Number(e.target.value))}
className="text-xs border rounded px-2 py-1"
>📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| <select | |
| value={year} | |
| onChange={(e) => onYearChange(Number(e.target.value))} | |
| className="text-xs border rounded px-2 py-1" | |
| > | |
| <select | |
| aria-label="Select sales year" | |
| value={year} | |
| onChange={(e) => onYearChange(Number(e.target.value))} | |
| className="text-xs border rounded px-2 py-1" | |
| > |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@apps/admin/src/app/dashboard/_components/sales-chart.tsx` around lines 67 -
71, The year <select> in the SalesChart component is missing an accessible
label; add one by either adding an associated <label> (create a unique id for
the select and render <label htmlFor="{id}">Year</label>) or by adding an
aria-label/aria-labelledby attribute on the <select> that references a visible
label or the text "Year"; update the element using the identifiers around
value={year} and onChange={onYearChange} so screen readers can announce its
purpose.
Comment on lines
+15
to
+16
| const todayStr = new Date().toISOString().split('T')[0] | ||
| const todayTasks = tasks.filter((t) => t.dueDate === todayStr) |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
cat -n apps/admin/src/app/dashboard/_components/sidebar-today-tasks.tsxRepository: Systemsaholic/tailfire
Length of output: 1690
🏁 Script executed:
# Find and examine the use-dashboard hook to understand dueDate format
find apps -name "use-dashboard*" -type fRepository: Systemsaholic/tailfire
Length of output: 103
🏁 Script executed:
cat -n apps/admin/src/hooks/use-dashboard.tsRepository: Systemsaholic/tailfire
Length of output: 4050
🏁 Script executed:
# Check if date-fns is in the project dependencies
grep -r "date-fns" package.json packages/*/package.json 2>/dev/null | head -20Repository: Systemsaholic/tailfire
Length of output: 232
🏁 Script executed:
# Search for backend code that returns TaskDueSummary to understand dueDate format
find apps -name "*.ts" -type f | xargs grep -l "TaskDueSummary\|tasksDue\|dueDate" | grep -E "(controller|service|repository)" | head -10Repository: Systemsaholic/tailfire
Length of output: 571
🏁 Script executed:
cat -n apps/api/src/dashboard/dashboard.service.ts | head -200Repository: Systemsaholic/tailfire
Length of output: 8704
🏁 Script executed:
grep -A 50 "async getTasksDue" apps/api/src/dashboard/dashboard.service.tsRepository: Systemsaholic/tailfire
Length of output: 2644
🏁 Script executed:
# Find the tasks schema definition to see dueDate column type
find apps -name "*.ts" -path "*/schema*" -type f | xargs grep -l "tasks\|dueDate" 2>/dev/null | head -5Repository: Systemsaholic/tailfire
Length of output: 48
🏁 Script executed:
# Find database schema files
find apps/api -type f -name "*.ts" | xargs grep -l "export const tasks\|createTable.*tasks" | head -5Repository: Systemsaholic/tailfire
Length of output: 48
🏁 Script executed:
# Find database/schema files more broadly
find apps -type d -name "schema" -o -name "migrations" -o -name "db"Repository: Systemsaholic/tailfire
Length of output: 110
🏁 Script executed:
ls -la apps/api/src/db/Repository: Systemsaholic/tailfire
Length of output: 438
🏁 Script executed:
# Find schema definition files - likely in migrations or drizzle
find apps -type f -name "*.ts" | xargs grep -l "integer\|timestamp\|date.*dueDate\|tasks.*dueDate" 2>/dev/null | grep -E "(migration|schema|db)" | head -10Repository: Systemsaholic/tailfire
Length of output: 120
🏁 Script executed:
# Search for drizzle schema definition
rg "dueDate.*timestamp|dueDate.*date" --type ts -A 2 -B 2 | head -50Repository: Systemsaholic/tailfire
Length of output: 3517
🏁 Script executed:
# Find and examine useCompleteTask hook
find apps -name "*use-tasks*" -o -name "*complete-task*" | grep -E "\.ts$"Repository: Systemsaholic/tailfire
Length of output: 99
🏁 Script executed:
cat -n apps/admin/src/hooks/use-tasks.tsRepository: Systemsaholic/tailfire
Length of output: 16997
🏁 Script executed:
cat -n apps/admin/package.json | head -50Repository: Systemsaholic/tailfire
Length of output: 2256
Use local-day comparison for "today" and guard completion on checkbox checked state.
Line 15–16 compares a UTC date string against stored date-only strings, which can misclassify tasks when the user's local timezone differs from the server's. Line 34 also triggers the completion mutation on both check and uncheck transitions instead of only on checked.
🛠️ Proposed fix
+import { isToday, parseISO } from 'date-fns'
...
- const todayStr = new Date().toISOString().split('T')[0]
- const todayTasks = tasks.filter((t) => t.dueDate === todayStr)
+ const todayTasks = tasks.filter((task) => {
+ if (!task.dueDate) return false
+ return isToday(parseISO(task.dueDate))
+ })
...
<Checkbox
className="mt-0.5"
- onCheckedChange={() => completeTask.mutate({ id: task.id })}
+ onCheckedChange={(checked) => {
+ if (checked === true && !completeTask.isPending) {
+ completeTask.mutate({ id: task.id })
+ }
+ }}
+ disabled={completeTask.isPending}
/>Also applies to: 32–35
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@apps/admin/src/app/dashboard/_components/sidebar-today-tasks.tsx` around
lines 15 - 16, todayStr is computed in UTC and used to filter tasks (todayTasks
= tasks.filter(...)), which misclassifies tasks for users in non-UTC timezones;
replace that with a local-day string (e.g. build YYYY-MM-DD from new
Date().getFullYear(), getMonth()+1, getDate() or use
toLocaleDateString('en-CA')) and use that value when creating todayTasks via
tasks.filter. Also change the checkbox onChange handler so the completion
mutation is only invoked when the checkbox becomes checked (guard the mutation
call behind event.target.checked or the checked boolean) instead of firing on
both check and uncheck transitions.
Comment on lines
+45
to
+50
| <Checkbox | ||
| className="mt-0.5" | ||
| onCheckedChange={() => { | ||
| completeTask.mutate({ id: task.id }) | ||
| }} | ||
| /> |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
# First, find the widget file to confirm the current code
fd 'tasks-due-widget.tsx$' apps/admin/srcRepository: Systemsaholic/tailfire
Length of output: 127
🏁 Script executed:
# Then search for the task controller and service files in the API
fd -t f 'tasks\.(controller|service)\.ts$' apps/api/srcRepository: Systemsaholic/tailfire
Length of output: 140
🏁 Script executed:
# Get an overview of the repository structure
ls -la apps/Repository: Systemsaholic/tailfire
Length of output: 387
🏁 Script executed:
# Read the widget file to see the current implementation
cat -n apps/admin/src/app/dashboard/_components/tasks-due-widget.tsx | head -80Repository: Systemsaholic/tailfire
Length of output: 3704
🏁 Script executed:
# Read the tasks controller
cat -n apps/api/src/tasks/tasks.controller.tsRepository: Systemsaholic/tailfire
Length of output: 10290
🏁 Script executed:
# Read the tasks service
cat -n apps/api/src/tasks/tasks.service.tsRepository: Systemsaholic/tailfire
Length of output: 33713
Guard checkbox mutation to fire only when explicitly checked.
Line 47 currently mutates on every state change, including unchecks. Unchecking the box will attempt to complete the task again; without server-side idempotency checks, this overwrites completedAt and completedBy (corrupting the audit trail) and can insert duplicate comment entries.
Only fire the mutation when checked === true, add a pending guard, and disable the checkbox during request:
Proposed fix
<Checkbox
className="mt-0.5"
- onCheckedChange={() => {
- completeTask.mutate({ id: task.id })
- }}
+ onCheckedChange={(checked) => {
+ if (checked === true && !completeTask.isPending) {
+ completeTask.mutate({ id: task.id })
+ }
+ }}
+ disabled={completeTask.isPending}
/>📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| <Checkbox | |
| className="mt-0.5" | |
| onCheckedChange={() => { | |
| completeTask.mutate({ id: task.id }) | |
| }} | |
| /> | |
| <Checkbox | |
| className="mt-0.5" | |
| onCheckedChange={(checked) => { | |
| if (checked === true && !completeTask.isPending) { | |
| completeTask.mutate({ id: task.id }) | |
| } | |
| }} | |
| disabled={completeTask.isPending} | |
| /> |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@apps/admin/src/app/dashboard/_components/tasks-due-widget.tsx` around lines
45 - 50, The Checkbox onCheckedChange handler in the TasksDueWidget currently
calls completeTask.mutate on every toggle (in the Checkbox component), which
triggers on uncheck and corrupts audit fields; change the handler in the
Checkbox used inside TasksDueWidget so it only calls completeTask.mutate when
the new checked value is truthy (checked === true), early-returns otherwise, and
guard against concurrent calls by checking completeTask.isLoading before
mutating; also bind the Checkbox disabled prop to completeTask.isLoading (or a
local pending state) so the box is disabled while the mutation is in flight.
Comment on lines
+66
to
+72
| @ApiPropertyOptional({ default: new Date().getFullYear() }) | ||
| @IsOptional() | ||
| @Type(() => Number) | ||
| @IsInt() | ||
| @Min(2020) | ||
| @Max(2100) | ||
| chartYear?: number = new Date().getFullYear() |
There was a problem hiding this comment.
Default year value evaluated at module load, not per request.
The default value new Date().getFullYear() is evaluated once when the module loads, not on each request. If the API service runs continuously across New Year's Eve, requests in the new year will still default to the previous year until the service restarts.
🔧 Recommended fix: Remove the default initializer
Move the default logic to the service layer or controller where it will be evaluated per-request:
`@ApiPropertyOptional`({ default: new Date().getFullYear() })
`@IsOptional`()
`@Type`(() => Number)
`@IsInt`()
`@Min`(2020)
`@Max`(2100)
- chartYear?: number = new Date().getFullYear()
+ chartYear?: numberThen in the service or controller:
const year = query.chartYear ?? new Date().getFullYear()📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| @ApiPropertyOptional({ default: new Date().getFullYear() }) | |
| @IsOptional() | |
| @Type(() => Number) | |
| @IsInt() | |
| @Min(2020) | |
| @Max(2100) | |
| chartYear?: number = new Date().getFullYear() | |
| `@ApiPropertyOptional`({ default: new Date().getFullYear() }) | |
| `@IsOptional`() | |
| `@Type`(() => Number) | |
| `@IsInt`() | |
| `@Min`(2020) | |
| `@Max`(2100) | |
| chartYear?: number |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@docs/plans/2026-03-03-dashboard-revamp-implementation.md` around lines 66 -
72, The chartYear property currently sets a module-load default via "chartYear?:
number = new Date().getFullYear()", which freezes the year until restart; remove
the inline initializer so the DTO only declares "chartYear?: number" (keep
decorators `@ApiPropertyOptional`, `@IsOptional`, `@Type`, `@IsInt`, `@Min`, `@Max`), and
implement per-request defaulting in the controller/service that consumes the DTO
(e.g., use "const year = query.chartYear ?? new Date().getFullYear()" inside the
handler or service method) so the current year is evaluated for each request.
Comment on lines
+214
to
+245
|
|
||
| The service must: | ||
| 1. Accept `auth: AuthContext`, `query: DashboardOverviewQueryDto` | ||
| 2. Build scoped conditions (admin = agencyId only; agent = owned/shared/inbound trips) | ||
| 3. Run all queries in parallel via `Promise.all()` | ||
| 4. Compute trends by running same KPI queries for prior period | ||
| 5. Return `DashboardOverview` | ||
|
|
||
| **Key implementation details:** | ||
|
|
||
| - **Personal trip scoping**: Use `TripAccessService.getAccessibleTripIds(auth)` — returns `'all'` for admin or `string[]` for agents. | ||
| - **KPI period**: For MTD: `>= startOfMonth(now())`. For YTD: `>= startOfYear(now())`. | ||
| - **Net sales**: `SUM(CASE WHEN transactionType = 'payment' THEN amountCents ELSE 0 END) - SUM(CASE WHEN transactionType = 'refund' THEN amountCents ELSE 0 END)` | ||
| - **Commission**: `SUM(commissionAmount::numeric)` from `commission_tracking` where `commissionStatus = 'received'`. Note: `commissionAmount` is `decimal(10,2)` in dollars. | ||
| - **Trend**: Run the same query for the prior equivalent period (prior month for MTD, prior year for YTD). Compute `((current - prior) / prior) * 100`. Return null if prior is zero. | ||
| - **Monthly charts**: Single query per chart using `date_trunc('month', created_at)` grouped by month, with `generate_series` for all 12 months. For YoY, add a WHERE for previous year in same query using UNION ALL or conditional aggregation. | ||
| - **Projection**: For current month: `projected = (actualSoFar / daysElapsed) * totalDaysInMonth`. Use `date_part('day', now())` for daysElapsed. | ||
| - **Tasks**: Use `TaskAccessService.buildAccessConditions(auth)` for WHERE clause. Filter `status = 'pending'`, sort overdue first then by dueDate. | ||
| - **Payments due**: Join `expected_payment_items` → `payment_schedule_config` → `activity_pricing` → trips. Filter `paidAmountCents < expectedAmountCents` and `status IN ('pending', 'partial', 'overdue')`. | ||
| - **Leaderboard** (admin only): Group `payment_transactions` by trip `ownerId` (joined through trips), filter by agencyId + period, order by net sales DESC, LIMIT 5. Join `user_profiles` for names. | ||
|
|
||
| This is a large service method. Organize as private helper methods: | ||
| - `private getKpiMetrics(agencyId, tripIds, startDate, endDate): Promise<KpiMetrics>` | ||
| - `private getRecentTrips(auth, tripIds): Promise<TripSummary[]>` | ||
| - `private getLeavingSoon(auth, tripIds): Promise<TripSummary[]>` | ||
| - `private getTasksDue(auth): Promise<TaskDueSummary[]>` | ||
| - `private getPaymentsDue(agencyId, tripIds): Promise<PaymentDueSummary[]>` | ||
| - `private getMonthlySales(agencyId, tripIds, year, includeYoy): Promise<MonthlySalesData[]>` | ||
| - `private getMonthlyCommission(agencyId, tripIds, year, includeYoy): Promise<MonthlyCommissionData[]>` | ||
| - `private getProjection(agencyId, tripIds): Promise<ProjectionData>` | ||
| - `private getAgentLeaderboard(agencyId, startDate, endDate): Promise<AgentLeaderboardEntry[]>` | ||
|
|
There was a problem hiding this comment.
🛠️ Refactor suggestion | 🟠 Major
Add error handling guidance for parallel queries.
Line 219 mentions using Promise.all() for parallel queries, but provides no guidance on error handling. With 9+ independent data sources (KPIs, trips, tasks, payments, charts, leaderboard), a failure in any single query will cause the entire dashboard to fail.
Consider using Promise.allSettled() instead to allow partial dashboard rendering when some widgets fail. This improves UX by showing available data rather than a complete error state.
💡 Recommended pattern for graceful degradation
Add guidance to the service implementation section:
// In getOverview method:
const [
personalKpisResult,
agencyKpisResult,
recentTripsResult,
// ... other results
] = await Promise.allSettled([
this.getKpiMetrics(agencyId, tripIds, startDate, endDate),
isAdmin ? this.getKpiMetrics(agencyId, 'all', startDate, endDate) : null,
this.getRecentTrips(auth, tripIds),
// ... other calls
])
// Handle each result safely
const personal = personalKpisResult.status === 'fulfilled'
? personalKpisResult.value
: this.getEmptyKpiMetrics()
// Log failures for monitoring
if (personalKpisResult.status === 'rejected') {
this.logger.error('Failed to fetch personal KPIs', personalKpisResult.reason)
}This ensures one failing widget doesn't crash the entire dashboard.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@docs/plans/2026-03-03-dashboard-revamp-implementation.md` around lines 214 -
245, The current guidance to use Promise.all() in getOverview will cause a
single failing query to fail the whole dashboard; change the parallel execution
to Promise.allSettled([...]) for the calls to getKpiMetrics, getRecentTrips,
getLeavingSoon, getTasksDue, getPaymentsDue, getMonthlySales,
getMonthlyCommission, getProjection and getAgentLeaderboard, then inspect each
settled result (status === 'fulfilled' ? value : fallback) and replace failures
with safe defaults (e.g., getEmptyKpiMetrics(), empty arrays or nulls) while
logging errors via this.logger.error(result.reason) so individual widget
failures degrade gracefully instead of bubbling up a full failure.
Comment on lines
+229
to
+234
| - **Monthly charts**: Single query per chart using `date_trunc('month', created_at)` grouped by month, with `generate_series` for all 12 months. For YoY, add a WHERE for previous year in same query using UNION ALL or conditional aggregation. | ||
| - **Projection**: For current month: `projected = (actualSoFar / daysElapsed) * totalDaysInMonth`. Use `date_part('day', now())` for daysElapsed. | ||
| - **Tasks**: Use `TaskAccessService.buildAccessConditions(auth)` for WHERE clause. Filter `status = 'pending'`, sort overdue first then by dueDate. | ||
| - **Payments due**: Join `expected_payment_items` → `payment_schedule_config` → `activity_pricing` → trips. Filter `paidAmountCents < expectedAmountCents` and `status IN ('pending', 'partial', 'overdue')`. | ||
| - **Leaderboard** (admin only): Group `payment_transactions` by trip `ownerId` (joined through trips), filter by agencyId + period, order by net sales DESC, LIMIT 5. Join `user_profiles` for names. | ||
|
|
There was a problem hiding this comment.
Projection calculation vulnerable to division by zero on first day of month.
Line 231 describes the projection formula as projected = (actualSoFar / daysElapsed) * totalDaysInMonth. On the first day of the month (when daysElapsed = 0 or potentially 1 depending on implementation), this will either throw a division error or produce incorrect projections.
🛡️ Recommended fix: Add guard clause for early-month edge case
Add guidance to handle the first few days of the month:
// In the projection helper method:
const daysElapsed = Number(result.daysElapsed)
const totalDays = Number(result.totalDaysInMonth)
// Avoid projection on first 2 days when data is insufficient
if (daysElapsed < 2) {
return {
salesActualCents: actualSales,
salesProjectedCents: actualSales,
commissionActualDollars: actualCommission,
commissionProjectedDollars: actualCommission,
daysElapsed,
totalDaysInMonth: totalDays,
}
}
// Safe to project after a few days of data
const salesProjectedCents = Math.round((actualSales / daysElapsed) * totalDays)
const commissionProjectedDollars = (actualCommission / daysElapsed) * totalDays🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@docs/plans/2026-03-03-dashboard-revamp-implementation.md` around lines 229 -
234, The projection formula in the projection helper (uses variables
daysElapsed, totalDaysInMonth, actualSales, actualCommission and computes
salesProjectedCents / commissionProjectedDollars) can divide by zero or produce
bad early-month projections; add a guard in that helper to treat daysElapsed < 2
(or another chosen small threshold) as insufficient data and return projected
values equal to actuals (i.e., salesProjected = actualSales and
commissionProjected = actualCommission) along with daysElapsed and
totalDaysInMonth, otherwise perform the existing safe projection calculation
downstream.
Comment on lines
+548
to
+565
| **Step 1: Create tasks-due-widget.tsx** | ||
|
|
||
| Card with "Tasks Due" header and "View all" link to `/tasks?sort=dueDate`. | ||
|
|
||
| List of up to 5 tasks. Each row shows: | ||
| - Checkbox (uses existing `useCompleteTask()` mutation from `@/hooks/use-tasks`) | ||
| - Task title | ||
| - Linked trip/contact name (small muted text) | ||
| - Due date (formatted, right-aligned) | ||
| - Priority badge (colored: urgent=red, high=orange, medium=yellow, low=gray) | ||
| - Overdue badge: red "OVERDUE" + days count | ||
|
|
||
| When checkbox clicked, optimistic update: strikethrough + fade animation, then remove from list. | ||
|
|
||
| Props: `{ tasks: TaskDueSummary[] }` | ||
|
|
||
| Empty state: "All caught up!" with checkmark icon. | ||
|
|
There was a problem hiding this comment.
🛠️ Refactor suggestion | 🟠 Major
Add guidance for dashboard cache invalidation on task completion.
The PR description mentions "invalidate dashboard cache on task completion," but the implementation guidance for the TasksDueWidget (line 553-560) doesn't mention invalidating the dashboard query cache after the useCompleteTask() mutation succeeds.
📝 Recommended addition to implementation guidance
Add a note in the TasksDueWidget specification:
// After task completion, invalidate dashboard cache to refresh KPIs and task list
const { mutate: completeTask } = useCompleteTask({
onSuccess: () => {
// Invalidate both tasks and dashboard queries
queryClient.invalidateQueries({ queryKey: taskKeys.all })
queryClient.invalidateQueries({ queryKey: dashboardKeys.all })
},
})This ensures the dashboard reflects the updated task count and KPIs immediately after a task is marked complete.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@docs/plans/2026-03-03-dashboard-revamp-implementation.md` around lines 548 -
565, The TasksDueWidget guidance omits invalidating dashboard-related caches
after calling useCompleteTask; update the TasksDueWidget spec to call
queryClient.invalidateQueries for both taskKeys.all and dashboardKeys.all in the
useCompleteTask onSuccess handler so the task list and KPIs refresh immediately
after completion (reference the TasksDueWidget prop shape, the useCompleteTask
mutation, and use of queryClient.invalidateQueries with taskKeys.all and
dashboardKeys.all).
Comment on lines
+691
to
+697
|
|
||
| Container that wraps all sidebar widgets. Accepts `isOpen` and `onToggle` props. When open: fixed-width right panel (280px). When closed: hidden. Transition with `transition-all duration-300`. | ||
|
|
||
| Renders: QuickCreate → MiniCalendar → TodayTasks → (admin only) AgentLeaderboard. | ||
|
|
||
| Store sidebar state in localStorage via a simple zustand store or direct `localStorage.getItem/setItem`. | ||
|
|
There was a problem hiding this comment.
🛠️ Refactor suggestion | 🟠 Major
Add guidance to prevent hydration mismatch with localStorage.
Line 696 mentions storing sidebar state in localStorage, but direct localStorage access during SSR can cause hydration mismatches in Next.js 15. The PR objectives mention "persist trips view mode safely (avoid hydration mismatch)," indicating awareness of this issue, but no guidance is provided here.
🔧 Recommended pattern to avoid hydration issues
Add implementation guidance to safely use localStorage:
// Use useEffect to read from localStorage only on client
const [isOpen, setIsOpen] = useState(false) // Default to closed for SSR
useEffect(() => {
// Only run on client after hydration
const stored = localStorage.getItem('dashboard-sidebar-open')
if (stored !== null) {
setIsOpen(stored === 'true')
}
}, [])
// Save to localStorage on change
const handleToggle = () => {
setIsOpen(prev => {
const newValue = !prev
localStorage.setItem('dashboard-sidebar-open', String(newValue))
return newValue
})
}Alternatively, use a library like use-local-storage-state that handles SSR hydration automatically.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@docs/plans/2026-03-03-dashboard-revamp-implementation.md` around lines 691 -
697, Update the guidance for persisting the sidebar state to show a
hydration-safe pattern: explain that the Sidebar wrapper (props isOpen,
onToggle) must avoid direct localStorage reads during SSR, and recommend
initializing state for the sidebar (e.g., isOpen) to a SSR-safe default
(closed), then reading the saved value inside a client-only effect (useEffect)
after hydration and writing changes back to localStorage (key like
"dashboard-sidebar-open") inside the toggle handler (handleToggle or the zustand
store's setter); alternatively reference using an SSR-aware library such as
use-local-storage-state to encapsulate this behavior.
This was referenced Mar 15, 2026
This was referenced Mar 26, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
keepPreviousDataTest plan
/dashboard— verify KPIs load with correct values, charts render, widgets show data/trips— verify tag filter works (select a tag, trips filter correctly)pnpm --filter @tailfire/admin typecheckpasses with 0 errors🤖 Generated with Claude Code
Summary by CodeRabbit