Stats page #384
Stats page #384
Conversation
Summary of ChangesHello @Doezer, 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 enhances the application by adding a robust statistics feature. It provides users with a dedicated page to view detailed insights into their game library, including various metrics like average rating, completion rate, and metadata health, along with breakdowns by status, genre, and platform. The implementation centralizes the statistics calculation logic into a new utility, improving maintainability and consistency across the application, while also updating existing components like the Dashboard to leverage this new functionality. Highlights
Changelog
Ignored Files
Activity
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
|
Codecov Report✅ All modified and coverable lines are covered by tests. 📢 Thoughts on this report? Let us know! |
![gemini-code-assist[bot]](https://waps.l3s.uni-hannover.de/live/im_/https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request introduces a new statistics page and refactors the existing stats logic into a reusable utility. The changes are well-structured, with the new logic in client/src/lib/stats.ts and comprehensive tests. The new stats page provides a great overview of the user's library. My review includes a few suggestions to improve correctness, performance, and maintainability. I've identified a bug in the average release year calculation, suggested performance improvements for status breakdown, and pointed out ways to make the code more deterministic and cleaner. Overall, this is a great addition.
| const datedGames = games.filter((g) => g.releaseDate); | ||
| const avgReleaseYear = | ||
| datedGames.length > 0 | ||
| ? Math.round( | ||
| datedGames.reduce((acc, g) => { | ||
| const year = new Date(g.releaseDate!).getFullYear(); | ||
| return acc + (isNaN(year) ? 0 : year); | ||
| }, 0) / datedGames.length | ||
| ) | ||
| : "N/A"; |
There was a problem hiding this comment.
The current implementation for calculating avgReleaseYear can produce an incorrect average if any releaseDate string is invalid. When new Date(g.releaseDate!).getFullYear() results in NaN, your code adds 0 to the sum but still includes the game in the datedGames.length divisor, which skews the result.
A more robust approach is to first map all dates to their year, then filter out any invalid NaN values before calculating the average. This ensures the average is only calculated over valid years.
const years = games
.map((g) => (g.releaseDate ? new Date(g.releaseDate).getFullYear() : NaN))
.filter((year) => !isNaN(year));
const avgReleaseYear =
years.length > 0
? Math.round(years.reduce((acc, year) => acc + year, 0) / years.length)
: "N/A";| expect(stats.topGenre?.name).toBe("RPG"); // Action (2), RPG (2) - RPG comes after Action in sort if counts are equal? No, sort order of entries is based on appearance or stable sort. Wait, Action:2, RPG:2. My implementation: sorted[0][0]. RPG:2, Action:2. | ||
| // Let's re-verify the sort: .sort((a,b) => b[1]-a[1]). If equal, order is preserved from original entries. | ||
| // Action (2), RPG (2). |
There was a problem hiding this comment.
This comment regarding the topGenre calculation appears to be based on an incorrect count of genres in the mock data (Action (2), RPG (2)) and adds confusion about the sorting logic. Based on the provided mockGames, the counts are actually RPG: 3 and Action: 1. Removing this misleading comment would improve the clarity and correctness of the test.
| expect(stats.topGenre?.name).toBe("RPG"); // Action (2), RPG (2) - RPG comes after Action in sort if counts are equal? No, sort order of entries is based on appearance or stable sort. Wait, Action:2, RPG:2. My implementation: sorted[0][0]. RPG:2, Action:2. | |
| // Let's re-verify the sort: .sort((a,b) => b[1]-a[1]). If equal, order is preserved from original entries. | |
| // Action (2), RPG (2). | |
| expect(stats.topGenre?.name).toBe("RPG"); |
| if (items.length === 0) return null; | ||
| const counts: Record<string, number> = {}; | ||
| items.forEach((item) => (counts[item] = (counts[item] || 0) + 1)); | ||
| const sorted = Object.entries(counts).sort((a, b) => b[1] - a[1]); |
There was a problem hiding this comment.
The sort function (a, b) => b[1] - a[1] is not stable. If two items have the same count, their returned order is not guaranteed across JavaScript engines, which can lead to non-deterministic results for "Top" items (e.g., Top Genre). To ensure consistent and predictable results, you can add a secondary sort criterion, such as sorting by name alphabetically.
| const sorted = Object.entries(counts).sort((a, b) => b[1] - a[1]); | |
| const sorted = Object.entries(counts).sort((a, b) => b[1] - a[1] || a[0].localeCompare(b[0])); |
| const statusBreakdown = { | ||
| wanted: games.filter((g) => g.status === "wanted").length, | ||
| owned: games.filter((g) => g.status === "owned").length, | ||
| completed: games.filter((g) => g.status === "completed").length, | ||
| downloading: games.filter((g) => g.status === "downloading").length, | ||
| }; |
There was a problem hiding this comment.
The statusBreakdown is calculated by filtering the games array four separate times. For large game libraries, this can be inefficient as it requires multiple passes over the data. A more performant approach would be to use a single loop (e.g., with reduce or forEach) to iterate over the games array only once and build the statusBreakdown object.
| } from "recharts"; | ||
| import { Card, CardContent, CardHeader, CardTitle } from "@/components/ui/card"; | ||
|
|
||
| const COLORS = ["#ef4444", "#3b82f6", "#10b981", "#8b5cf6"]; // wanted, owned, completed, downloading |
There was a problem hiding this comment.
The COLORS constant is declared but never used in this component. The pie chart uses the chartColors object for its color scheme. This unused constant should be removed to avoid dead code.
| queryFn: async () => { | ||
| const token = localStorage.getItem("token"); | ||
| const headers: Record<string, string> = {}; | ||
| if (token) { | ||
| headers["Authorization"] = `Bearer ${token}`; | ||
| } | ||
| const response = await fetch("/api/games?includeHidden=true", { headers }); | ||
| if (!response.ok) throw new Error("Failed to fetch games"); | ||
| return response.json(); | ||
| }, |
There was a problem hiding this comment.
The queryFn for fetching games duplicates logic for handling authentication tokens, which is also present in other components like Dashboard.tsx. To improve maintainability and adhere to the DRY (Don't Repeat Yourself) principle, this data fetching logic should be centralized. Consider creating a shared API client function that handles auth headers and can be reused across all useQuery calls that require authentication.
1 participant
This pull request introduces a new statistics feature for the game library, including a dedicated statistics page, reusable stats calculation logic, and UI enhancements. The main changes are the addition of a robust stats utility, integration of the stats page into the app navigation, updates to the dashboard to use the new stats logic, and comprehensive tests to ensure correctness.
Statistics Feature Implementation:
StatsPage(client/src/pages/stats.tsx) that displays detailed library statistics, including total games, average rating, completion rate, metadata health, status breakdown (with pie chart), and quick info for top genre, platform, publisher, unique developers, and average release year.calculateLibraryStats, inclient/src/lib/stats.ts, which computes all relevant metrics for the library and exposes them via a structured interface.calculateLibraryStatsinclient/src/lib/__tests__/stats.test.tsto verify correctness for various scenarios, including empty and incomplete libraries.App Integration and UI Updates:
client/src/components/Dashboard.tsx) to use the newcalculateLibraryStatsutility for statistics, simplifying and consolidating stats logic. Also adjusted the dashboard stats card layout for improved clarity. [1] [2] [3]Minor Workflow Change:
.github/workflows/deploy.yml.