Fix dashboard statistics legend to include zero-usage users#656
Fix dashboard statistics legend to include zero-usage users#656zhiqing0205 wants to merge 6 commits intoding113:devfrom
Conversation
Summary of ChangesHello @zhiqing0205, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request addresses a user experience inconsistency in the dashboard's statistics chart. Previously, users who had no recorded usage would still appear in the chart visualization but were omitted from the interactive legend, making it impossible to toggle their visibility. This change ensures that all users, regardless of their usage metrics, are consistently represented in the legend, allowing for full control over chart data display. Additionally, the PR includes general code hygiene improvements by resolving several Biome linting issues. Highlights
Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here. You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension. Footnotes
|
📝 WalkthroughWalkthrough将统计图表卡片的用户选择与渲染改为基于“非零使用”过滤:图表系列、图例与颜色映射仅针对有非零成本或调用的用户构建;同时清理若干无用导入/参数、调整少数 effect 依赖,并新增单元测试覆盖零使用场景。 Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 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)
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 |
There was a problem hiding this comment.
Code Review
This pull request fixes a mismatch in the Dashboard statistics chart by ensuring that users with zero usage are included in the legend and remain toggleable. It also includes minor Biome lint cleanups. The changes primarily involve modifying the StatisticsChartCard component to remove a filter that excluded zero-usage users from the legend, adding a unit test to verify the fix, and removing unused imports/params in other files.
There was a problem hiding this comment.
Code Review Summary
This PR removes the filter that excluded zero-usage users from the statistics chart legend, allowing all users to be toggled regardless of their usage. The changes are clean and focused.
PR Size: XS
- Lines changed: 115 (80 additions, 35 deletions)
- Files changed: 13
Issues Found
| Category | Critical | High | Medium | Low |
|---|---|---|---|---|
| Logic/Bugs | 0 | 0 | 0 | 0 |
| Security | 0 | 0 | 0 | 0 |
| Error Handling | 0 | 0 | 0 | 0 |
| Types | 0 | 0 | 0 | 0 |
| Comments/Docs | 0 | 0 | 0 | 0 |
| Tests | 0 | 0 | 0 | 0 |
| Simplification | 0 | 0 | 0 | 0 |
Review Coverage
- Logic and correctness - Clean
- Security (OWASP Top 10) - Clean
- Error handling - Clean
- Type safety - Clean
- Documentation accuracy - Clean
- Test coverage - Adequate (new test added)
- Code clarity - Good
Analysis
Main Change: The core fix in statistics-chart-card.tsx correctly removes the filter that was hiding zero-usage users from the legend while they still appeared in the chart. This resolves the mismatch described in the PR.
Lint Cleanups: The remaining changes are legitimate lint fixes:
- Removed unused imports (RefreshCw, Trash2, Power, ChartTooltipContent, ResponsiveContainer, cn, Agent, ProxyConfigWithCacheKey)
- Removed unused parameters (statusCode, allowedProviderTypes, Icon variable)
- Fixed React hook dependencies appropriately
Test Coverage: A new unit test was added to verify zero-usage users appear in the legend, which is appropriate for this behavioral change.
Hook Dependency Changes:
limit-rule-picker.tsx: RemovingavailableTypesfrom deps is correct since it's now a constant reference toLIMIT_TYPE_OPTIONSprobe-terminal.tsx: Changed fromlogstologs.lengthwith early return guard - this is a valid optimization to prevent unnecessary effect runs when log content changes but length doesn'tuse-lazy-filter-options.ts: Removed biome-ignore comment - theloadcallback correctly has no dependencies asfetcheris a stable closure parameter
All changes align with the stated purpose and follow project conventions.
Automated review by Claude AI
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In
`@src/app/`[locale]/dashboard/availability/_components/endpoint/probe-terminal.tsx:
- Around line 75-81: The useEffect that auto-scrolls (the hook referencing
logs.length, autoScroll, userScrolled and containerRef) fails when the server
returns a same-length array; change the dependency from logs.length to a stable
identifier for the newest entry (e.g., compute lastLogId = logs[logs.length -
1]?.id or timestamp) and use that lastLogId in the effect dependencies along
with autoScroll and userScrolled, keep the same empty-array guard and scroll
logic inside the effect so new arrivals with identical length still trigger
auto-scroll.
src/app/[locale]/dashboard/availability/_components/endpoint/probe-terminal.tsx
Outdated
Show resolved
Hide resolved
|
According to the functional design, users with zero usage should not appear in the statistics chart; otherwise, it becomes difficult to make selections when the number of users is too large. The expected modification here is for both the chart and the user list to be consistent in excluding zero-usage users. |
| @@ -399,10 +420,10 @@ export function StatisticsChartCard({ | |||
| <div className="flex items-center justify-center gap-2 mb-2"> | |||
| <button | |||
| onClick={() => setSelectedUserIds(new Set(data.users.map((u) => u.id)))} | |||
There was a problem hiding this comment.
"Select All" button selects all users including zero-usage users, but only non-zero users appear in legend. This creates inconsistent state.
| onClick={() => setSelectedUserIds(new Set(data.users.map((u) => u.id)))} | |
| onClick={() => setSelectedUserIds(new Set(nonZeroUsers.map((u) => u.id)))} |
Prompt To Fix With AI
This is a comment left during a code review.
Path: src/app/[locale]/dashboard/_components/bento/statistics-chart-card.tsx
Line: 422:422
Comment:
"Select All" button selects all users including zero-usage users, but only non-zero users appear in legend. This creates inconsistent state.
```suggestion
onClick={() => setSelectedUserIds(new Set(nonZeroUsers.map((u) => u.id)))}
```
How can I resolve this? If you propose a fix, please make it concise.
What
Fix a mismatch in the Dashboard statistics chart where users with zero usage were still rendered in the chart, but were missing from the user toggle buttons (legend) below.
Why
In Admin "Users" mode, the legend buttons control which user series are visible. Filtering out zero-usage users made them impossible to toggle, while their 0-value series still appeared in the chart.
Changes
StatisticsChartCardlegend (including zero-usage users).bun run lintpassing.Greptile Overview
Greptile Summary
Fixed a UX inconsistency in the Dashboard statistics chart where users with zero usage were rendered in the chart but missing from the legend toggle buttons.
Key Changes:
nonZeroUsersmemo to filter users with activity (cost > 0 OR calls > 0)nonZeroUserswhile maintaining color consistency viaoriginalIndexlookupnonZeroUsersinstead of all usersvisibleUsersfilters fromnonZeroUsers(not all users) to prevent rendering zero-usage seriesAdditional Changes:
lastLogIdinstead of entire logs arrayConfidence Score: 5/5
Important Files Changed
Sequence Diagram
sequenceDiagram participant User participant Component as StatisticsChartCard participant State as React State participant Memos as useMemo hooks participant Chart as AreaChart User->>Component: Load dashboard with user statistics Component->>State: Initialize selectedUserIds with all users Component->>Memos: Calculate userTotals (sum cost/calls per user) Memos-->>Component: Return totals for all users Component->>Memos: Filter nonZeroUsers (cost > 0 OR calls > 0) Memos-->>Component: Return only users with activity Component->>Memos: Filter visibleUsers (selected AND non-zero) Memos-->>Component: Return users to display in chart Component->>Chart: Render area series for visibleUsers only Chart-->>User: Display chart with visible user series Component->>Component: Render legend buttons for nonZeroUsers Component-->>User: Display toggleable buttons (zero-usage excluded) User->>Component: Click user toggle button Component->>State: Update selectedUserIds State-->>Component: Trigger re-render Component->>Memos: Recalculate visibleUsers Component->>Chart: Update chart with new visible users Chart-->>User: Display updated chart