feat: enhance dashboard charts with time-range control and metrics #51
Conversation
…shed adaptive metrics** - add NetworkThroughputChart, UserActivityChart, and DashboardDeviceStats to the dashboard - introduce a global timeframe selector (5 min, 1 hr, 24 hr, 7 days) that drives all charts - fetch aggregated port stats for all switches and reuse DeviceStatsChart in a simplified wrapper - implement user-consumption sorting, top-user highlighting, and summary cards - add empty states, logging, and refresh intervals for clearer monitoring feedback - restyle charts for consistency with device overview (gradient fills, unified tooltips, improved axes) - auto-convert throughput (K/M/Gbps) and consumption (K/M/G/TB) for readability - surface headline metrics: total data transferred, peak throughput, and trend vs previous period - fetch previous-period data to compute trend and render only once both datasets resolve (no flicker) **Overview:** Dashboard now offers live throughput, consumption, and device stats with unified time-range control, adaptive metric formatting, and consistent UI. Critical metrics (total, peak, trends, top consumers) are surfaced up front, and charts remain readable across all traffic scales.
WalkthroughAdds client-side time-range state and a Network Overview to the dashboard; introduces three new chart components (NetworkThroughputChart, UserActivityChart, DashboardDeviceStats) that fetch and render aggregated metrics; makes AggregateTimeSeriesPoint fields optional and relaxes null-checks in port utilization chart. Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant DashboardPage
participant TimeRangeSelect
participant NetworkThroughputChart
participant UserActivityChart
participant DashboardDeviceStats
participant API
User->>DashboardPage: Open dashboard
DashboardPage->>DashboardPage: init timeRange ("1hour")
rect rgba(200,220,255,0.25)
User->>TimeRangeSelect: change range
TimeRangeSelect->>DashboardPage: update timeRange
DashboardPage->>NetworkThroughputChart: pass timeRange
DashboardPage->>UserActivityChart: pass timeRange
DashboardPage->>DashboardDeviceStats: mount
end
rect rgba(220,200,255,0.12)
NetworkThroughputChart->>API: fetchPortStatsAggregate (current & previous)
API-->>NetworkThroughputChart: aggregated throughput
NetworkThroughputChart->>NetworkThroughputChart: compute peak/total & render
end
rect rgba(255,220,200,0.12)
UserActivityChart->>API: fetchAggregateFlowsByUser(timeRange)
API-->>UserActivityChart: per-user aggregates
UserActivityChart->>UserActivityChart: convert/sort & render
end
rect rgba(220,255,200,0.12)
DashboardDeviceStats->>API: fetch devices
API-->>DashboardDeviceStats: device list
loop per device with lan_ip_address
DashboardDeviceStats->>API: fetchDeviceStatsAggregate(device, timeRange)
API-->>DashboardDeviceStats: device stats or empty
end
DashboardDeviceStats->>DashboardDeviceStats: pick first device with data & render
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes
Possibly related PRs
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
Tip 📝 Customizable high-level summaries are now available in beta!You can now customize how CodeRabbit generates the high-level summary in your pull requests — including its content, structure, tone, and formatting.
Example instruction:
Note: This feature is currently in beta for Pro-tier users, and pricing will be announced later. 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: 3
🧹 Nitpick comments (10)
ui/ui/src/lib/types.ts (1)
1157-1165: AggregateTimeSeriesPoint optional fields and total_throughput look consistentMaking utilization/throughput fields optional and adding
total_throughputmatches the new dashboard usage and relaxed null checks. Just ensure any consumers that build keys fromip_address/port_nameeither only receive per-port records or explicitly skip records where those are absent.ui/ui/src/components/graphs/port-utilization/PortUtilizationChart.tsx (1)
70-94: Using!= nullfor utilization guards correctly against undefinedSwitching to
point.avg_utilization != null/point.max_utilization != nullis the right adjustment now that these fields can be undefined; it prevents NaNs from leaking into the chart when the backend omits values.If you ever want to distinguish “unknown” from real 0% in the tooltip, consider rendering an
"N/A"label when the stored value isnullrather than treating it as0.ui/ui/src/components/dashboard/NetworkThroughputChart.tsx (3)
45-97: Metrics and trend calculation are sound; consider reusing aggregated seriesThe aggregation to
timeMapfor peak and total transferred, plus the trend percentage againstpreviousPeriodTotal, is logically consistent given Mbps input and fixedintervalSeconds. This same “timestamp → summed throughput” reduction is repeated in several places, though; extracting auseMemothat returns the aggregated series once (and reusing it for metrics, trend, and chart data) would reduce duplication and make it easier to evolve the logic (e.g., to switch tototal_throughputif the backend starts populating it).Also applies to: 115-126
184-272: Data fetching and previous-period comparison are reasonable; consider cancellationFetching current and 2×-window data in parallel, then splitting the wider window at the midpoint to approximate the “previous period” total, is a pragmatic approach and looks correct given a regular bucket cadence. The polling and cleanup with
setInterval/clearIntervalare also handled properly. For extra robustness, you might later introduce anAbortControllerand pass its signal intofetchPortStatsAggregateso that in-flight requests from an oldtimeRangedon’t race with newer ones.
150-180: Y-axis auto-scaling is good; align chart label with chosen unitThe
getYAxisUnitlogic correctly picks Mbps/Gbps/Kbps based on the max aggregated throughput. Right now,ChartContainer’s config label is hardcoded to"Mbps", so the legend/unit hint can diverge from the actual Y-axis unit when the data is very small or very large. Consider wiring the label toyAxisConfig.unitso everything stays in sync.Also applies to: 328-419
ui/ui/src/app/dashboard/page.tsx (1)
18-24: Time range selector and wiring look correct; avoid duplicating TimeRange typeState management and the Select wiring for
timeRangeare straightforward and correctly thread the value into bothNetworkThroughputChartandUserActivityChart. Since the same"5min" | "1hour" | "24hour" | "7days"union is now defined in three files, consider extracting a sharedTimeRangetype (e.g., in a smalldashboard/typesmodule) to keep them from drifting if you add new ranges later.Also applies to: 30-35, 66-89
ui/ui/src/components/dashboard/UserActivityChart.tsx (1)
15-19: Data loading and TimeRange handling work; consider shared type and basic error handlingMapping
timeRange→ period string and polling cadence is clear, and the effect correctly refreshes ontokenortimeRangechanges with proper interval cleanup. It might be worth (a) reusing a sharedTimeRangetype with the dashboard/throughput chart to avoid drift, and (b) wrappingfetchAggregateFlowsByUserin a smalltry/catchso a transient network/API error doesn’t surface as an unhandled exception but instead shows the empty-state message.Also applies to: 103-130, 132-156
ui/ui/src/components/dashboard/DashboardDeviceStats.tsx (3)
37-39: Improve type safety for response normalization.The type assertion bypasses TypeScript's type checking. Consider using a type guard function instead to ensure runtime safety.
Add a type guard before the normalization logic:
// At the top of the file or in a utils file function isResultsResponse(value: unknown): value is { results: NetworkDeviceDetails[] } { return ( typeof value === 'object' && value !== null && 'results' in value && Array.isArray((value as { results: unknown }).results) ); }Then use it:
// handle paginated or array response let devices: NetworkDeviceDetails[] = []; if (Array.isArray(switches)) { devices = switches; - } else if (switches && typeof switches === 'object' && 'results' in switches) { - const results = (switches as { results: NetworkDeviceDetails[] }).results; - devices = Array.isArray(results) ? results : []; + } else if (isResultsResponse(switches)) { + devices = switches.results; }
107-108: Consider preventing overlapping refresh requests.If
load()takes longer than 60 seconds (e.g., due to slow network or multiple device retries), the interval will trigger another request before the previous one completes, potentially causing overlapping requests and unnecessary load.Consider using
setTimeoutrecursively instead ofsetInterval:}; load(); - // Refresh every minute like the other charts - const interval = setInterval(load, 60000); + // Refresh every minute like the other charts, but wait for previous load to complete + let timeoutId: NodeJS.Timeout; + const scheduleNext = () => { + timeoutId = setTimeout(async () => { + await load(); + if (isMounted) scheduleNext(); + }, 60000); + }; + scheduleNext(); return () => { isMounted = false; abortController.abort(); - clearInterval(interval); + clearTimeout(timeoutId); };
111-128: LGTM! Consider adding ARIA labels for accessibility.The loading and empty states are well-implemented with clear user feedback. As an optional enhancement, consider adding
roleandaria-labelattributes to improve screen reader support.Optional: Add accessibility attributes:
<Card> <CardHeader> <CardTitle>Device Statistics</CardTitle> </CardHeader> <CardContent> - <div className="flex items-center justify-center min-h-[300px] text-muted-foreground"> + <div + className="flex items-center justify-center min-h-[300px] text-muted-foreground" + role="status" + aria-label="No devices available" + > No monitored devices found. </div> </CardContent> </Card>
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (6)
ui/ui/src/app/dashboard/page.tsx(3 hunks)ui/ui/src/components/dashboard/DashboardDeviceStats.tsx(1 hunks)ui/ui/src/components/dashboard/NetworkThroughputChart.tsx(1 hunks)ui/ui/src/components/dashboard/UserActivityChart.tsx(1 hunks)ui/ui/src/components/graphs/port-utilization/PortUtilizationChart.tsx(1 hunks)ui/ui/src/lib/types.ts(1 hunks)
🧰 Additional context used
🧬 Code graph analysis (4)
ui/ui/src/app/dashboard/page.tsx (2)
ui/ui/src/components/dashboard/NetworkThroughputChart.tsx (1)
NetworkThroughputChart(19-425)ui/ui/src/components/dashboard/UserActivityChart.tsx (1)
UserActivityChart(21-205)
ui/ui/src/components/dashboard/DashboardDeviceStats.tsx (6)
ui/ui/src/lib/types.ts (2)
DeviceStatsTimeSeriesPoint(1177-1185)NetworkDeviceDetails(72-83)backend/control_center/general/views.py (1)
switches(570-574)ui/ui/src/lib/devices.ts (1)
fetchSwitches(97-104)ui/ui/src/lib/deviceStats.ts (1)
fetchDeviceStatsAggregate(34-54)ui/ui/src/components/graphs/device-stats/DeviceStatsSkeleton.tsx (1)
DeviceStatsSkeleton(26-44)ui/ui/src/components/graphs/device-stats/DeviceStatsChart.tsx (1)
DeviceStatsChart(40-193)
ui/ui/src/components/dashboard/UserActivityChart.tsx (1)
ui/ui/src/lib/networkData.ts (1)
fetchAggregateFlowsByUser(43-53)
ui/ui/src/components/dashboard/NetworkThroughputChart.tsx (2)
ui/ui/src/lib/types.ts (1)
AggregateTimeSeriesPoint(1157-1166)ui/ui/src/lib/portUtilization.ts (1)
fetchPortStatsAggregate(64-95)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: Docker Build Validation
🔇 Additional comments (4)
ui/ui/src/components/dashboard/UserActivityChart.tsx (1)
44-101: Unit conversion and chart rendering are coherent
formatBytesandgetYAxisUnitcorrectly treat stored values as MB and convert to B/KB/MB/GB/TB as needed, and the Bar/LabelList usage produces intuitive per-user labels even with the numeric X-axis hidden. The overall chart behavior (sorted descending, top user highlighted, labels usingformatBytes) looks consistent and easy to interpret.Also applies to: 157-199
ui/ui/src/components/dashboard/DashboardDeviceStats.tsx (3)
1-17: LGTM!Imports are well-organized and all necessary dependencies are included.
18-23: LGTM!Component state initialization is properly typed and uses appropriate default values.
130-159: LGTM!The main render logic properly handles both empty data and data-available states with clear, user-friendly messaging. The display of data point count and device IP provides helpful context to users.
| 1, // 1 hour | ||
| "1 minute" |
There was a problem hiding this comment.
Major: Hardcoded time range contradicts PR objectives.
The PR summary states: "A shared time-range selector and headline metrics... are introduced." However, this component hardcodes a 1-hour window with 1-minute granularity instead of using a shared time-range control. This creates an inconsistent user experience where other dashboard charts may respond to time-range changes but device stats remain fixed at 1 hour.
Consider accepting time range parameters as props or from a shared context to align with the dashboard's time-range selector mentioned in the PR objectives.
🤖 Prompt for AI Agents
In ui/ui/src/components/dashboard/DashboardDeviceStats.tsx around lines 65-66,
the component currently hardcodes a 1-hour window and 1-minute granularity which
contradicts the PR's shared time-range objective; update the component to accept
the dashboard time-range (either via props or from the shared time-range
context/store) and use those values for the window and resolution instead of the
hardcoded "1 hour / 1 minute" so the device stats react to global time-range
changes and remain consistent with other dashboard charts.
- cancel in-flight stats requests when the component unmounts or the auth token changes by wiring fetchDeviceStatsAggregate to an AbortController signal - guard all setState calls behind an isMounted flag to avoid React warnings and memory leaks - remove noisy console.log/warn debug traces while keeping real error logging in place Overview: - Prevents pressure from uncancelled polling requests. - Keeps the console clean and avoids state updates after unmount.
There was a problem hiding this comment.
Actionable comments posted: 0
♻️ Duplicate comments (1)
ui/ui/src/components/dashboard/DashboardDeviceStats.tsx (1)
61-62: Hardcoded time range contradicts PR objectives.The 1-hour window and 1-minute granularity are still hardcoded, which is inconsistent with the PR's stated goal of introducing "A shared time-range selector." This creates a dashboard where other charts respond to time-range changes but device stats remain fixed.
As noted in the previous review, consider accepting time range parameters via props or from a shared context to align with the dashboard's time-range selector.
🧹 Nitpick comments (2)
ui/ui/src/components/dashboard/DashboardDeviceStats.tsx (2)
31-31: Update misleading comment.The comment states "Get first switch" but the code actually fetches all switches and iterates through them sequentially (lines 52-78) to find one with available stats data.
Apply this diff:
- // 1. Get first switch + // Get all switches and try each until we find one with stats data const switches = await fetchSwitches(token);
86-93: Consider displaying error state to users.When an error occurs, the component logs it but only shows a generic empty state to the user. Users cannot distinguish between "no data available" and "error fetching data," which could lead to confusion during troubleshooting.
Consider adding an error state:
const [data, setData] = useState<DeviceStatsTimeSeriesPoint[]>([]); const [device, setDevice] = useState<NetworkDeviceDetails | null>(null); const [loading, setLoading] = useState(true); + const [error, setError] = useState<string | null>(null);Then in the catch block:
} catch (e) { if (abortController.signal.aborted) return; console.error("Failed to load device stats for dashboard", e); if (e instanceof Error) { console.error("Error details:", e.message, e.stack); } if (!isMounted) return; + setError("Failed to load device statistics. Please try again later."); setData([]);And render it in the UI:
+ if (error) { + return ( + <Card> + <CardHeader> + <CardTitle>Device Statistics</CardTitle> + </CardHeader> + <CardContent> + <div className="flex items-center justify-center min-h-[300px] text-destructive"> + {error} + </div> + </CardContent> + </Card> + ); + } + if (!device) {
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
ui/ui/src/components/dashboard/DashboardDeviceStats.tsx(1 hunks)
🧰 Additional context used
🧬 Code graph analysis (1)
ui/ui/src/components/dashboard/DashboardDeviceStats.tsx (5)
ui/ui/src/lib/types.ts (2)
DeviceStatsTimeSeriesPoint(1177-1185)NetworkDeviceDetails(72-83)ui/ui/src/lib/devices.ts (1)
fetchSwitches(97-104)ui/ui/src/lib/deviceStats.ts (1)
fetchDeviceStatsAggregate(34-54)ui/ui/src/components/graphs/device-stats/DeviceStatsSkeleton.tsx (1)
DeviceStatsSkeleton(26-44)ui/ui/src/components/graphs/device-stats/DeviceStatsChart.tsx (1)
DeviceStatsChart(40-193)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: Docker Build Validation
🔇 Additional comments (4)
ui/ui/src/components/dashboard/DashboardDeviceStats.tsx (4)
1-23: LGTM! Clean component setup.The imports are well-organized and the state management is properly typed. The component structure follows React best practices.
24-108: Well done! Memory leak prevention properly implemented.The component now correctly handles cleanup with
AbortController, theisMountedflag, and proper state update guards. This addresses the critical memory leak issue from the previous review.
110-127: LGTM! Well-structured loading and empty states.The skeleton loader provides good UX during data fetching, and the empty state clearly communicates when no devices are available for monitoring.
129-158: LGTM! Clear and informative rendering logic.The component provides good user feedback with device identification in the card description, data point counts, and helpful empty states when no statistics are available.
1 participant
Summary by CodeRabbit
New Features
Improvements
✏️ Tip: You can customize this high-level summary in your review settings.