feat(dashboard): add time period filter to all charts#1129
Conversation
Add optional timeWindow parameter to all 9 analytics queries: - requestStatsByChannel, requestStatsByModel, requestStatsByAPIKey - tokenStatsByChannel, tokenStatsByModel, tokenStatsByAPIKey - costStatsByChannel, costStatsByModel, costStatsByAPIKey Ultraworked with [Sisyphus](https://github.com/code-yeongyu/oh-my-openagent) Co-authored-by: Sisyphus <[email protected]>
Add time window filtering to all 9 analytics resolvers: - RequestStatsByChannel, RequestStatsByModel, RequestStatsByAPIKey - TokenStatsByChannel, TokenStatsByModel, TokenStatsByAPIKey - CostStatsByChannel, CostStatsByModel, CostStatsByAPIKey Supports day/week/month/allTime values. Skip filter when timeWindow is nil, empty, or allTime. Ultraworked with [Sisyphus](https://github.com/code-yeongyu/oh-my-openagent) Co-authored-by: Sisyphus <[email protected]>
Regenerate generated.go with timeWindow parameter support. Ultraworked with [Sisyphus](https://github.com/code-yeongyu/oh-my-openagent) Co-authored-by: Sisyphus <[email protected]>
- Add TimePeriod type (day/week/month/allTime) - Update all 9 analytics hooks to accept timeWindow parameter - Include timeWindow in query keys for proper caching - Update GraphQL queries to pass timeWindow variable Ultraworked with [Sisyphus](https://github.com/code-yeongyu/oh-my-openagent) Co-authored-by: Sisyphus <[email protected]>
Add Tabs UI to all 6 analytics charts for time period selection: - requests-by-channel-chart, requests-by-model-chart, requests-by-api-key-chart - tokens-by-channel-chart, tokens-by-model-chart, tokens-by-api-key-chart Features: - 4 options: allTime (default), month, week, day - Time period passed to data hooks - Fix TypeScript error (remove invalid isAnimationActive from BarChart) Ultraworked with [Sisyphus](https://github.com/code-yeongyu/oh-my-openagent) Co-authored-by: Sisyphus <[email protected]>
Summary of ChangesHello, 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 significantly refactors the dashboard's time period selection mechanism by introducing a reusable component and integrating dynamic time window filtering into the backend GraphQL queries. These changes streamline the frontend UI, improve data fetching efficiency, and provide a more robust and user-friendly experience for viewing dashboard statistics. 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. Footnotes
|
There was a problem hiding this comment.
Code Review
This pull request refactors the dashboard components to support time period selection for various charts. A new TimePeriodSelector component is introduced, and backend resolvers are updated to filter data based on the selected time window. My review focuses on a bug in the new selector component and an opportunity to reduce code duplication in the backend resolvers.
| } | ||
|
|
||
| export function TimePeriodSelector<T extends string>({ value, onChange, periods }: TimePeriodSelectorProps<T>) { | ||
| const effectivePeriods = periods ?? getDefaultPeriods(value); |
There was a problem hiding this comment.
The use of getDefaultPeriods(value) here introduces a bug. The set of available time periods shouldn't change based on the currently selected value. For example, if a user selects 'day' from a list that includes 'allTime', the 'allTime' option will disappear on the next render, preventing them from switching back.
The TimePeriodSelector should use a fixed default set of periods if none are provided via props. Please remove the getDefaultPeriods function (lines 16-22) and simplify this line.
| const effectivePeriods = periods ?? getDefaultPeriods(value); | |
| const effectivePeriods = periods ?? DEFAULT_PERIODS; |
Summary
Adds a shared
TimePeriodSelectorcomponent that enables consistent time period filtering across all dashboard charts. Users can now filter dashboard data byAll Time,Month,Week, orDay.Purpose
Before this change, dashboard charts displayed static data without any time-based filtering capability. This feature allows users to:
Changes
New Component
TimePeriodSelectorinfrontend/src/components/time-period-selector.tsxDashboard Integration
Refactoring
getDefaultPeriodsfunctionFastestTimeWindowto shared locationTesting