Statspage - #4873
Conversation
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Plus Run ID: ⛔ Files ignored due to path filters (1)
📒 Files selected for processing (3)
🚧 Files skipped from review as they are similar to previous changes (2)
WalkthroughPlayer statistics now support recent aggregate data, all filter selections, Humans Vs Nations, summary metrics, sectioned tables, and updated English translations. Tests cover schema parsing, aggregation, rendering, filtering, and missing values. ChangesPlayer statistics
Estimated code review effort: 4 (Complex) | ~45 minutes Possibly related PRs
Suggested labels: Suggested reviewers: Sequence Diagram(s)sequenceDiagram
participant PlayerStatsTreeView
participant PlayerStatsTree
participant PlayerStatsSummary
participant PlayerStatsTable
PlayerStatsTreeView->>PlayerStatsTree: select filters
PlayerStatsTree->>PlayerStatsTreeView: return aggregated statistics
PlayerStatsTreeView->>PlayerStatsSummary: provide selected leaf
PlayerStatsSummary->>PlayerStatsTreeView: render summary metrics
PlayerStatsTreeView->>PlayerStatsTable: provide selected statistics
PlayerStatsTable->>PlayerStatsTreeView: render statistic sections
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
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.
🧹 Nitpick comments (1)
tests/client/PlayerStatsSummary.test.ts (1)
373-392: 🎯 Functional Correctness | 🔵 Trivial | ⚡ Quick winSet the filter state before checking the leaf.
PlayerStatsTreeViewinitializes all filters toALL_SELECTION. This test never selectsPublicorFree For All, so it exercises the all-types aggregation branch. With one fixture leaf, the assertion can pass without proving the exact branch described in the comment.Set
tree.selectedType = GameType.Publicandtree.selectedMode = GameMode.FFAbefore appending the element. If this test must coverMedium, use a non-public fixture because the Public path ignores difficulty.Suggested test setup
const tree = new PlayerStatsTreeView(); + tree.selectedType = GameType.Public; + tree.selectedMode = GameMode.FFA; tree.statsTree = {🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@tests/client/PlayerStatsSummary.test.ts` around lines 373 - 392, Update the test setup for “keeps a leaf's own recent stats when no tree-level aggregate exists” by assigning tree.selectedType = GameType.Public and tree.selectedMode = GameMode.FFA before appending the tree, so it exercises the exact-selection branch rather than all-types aggregation. Keep the existing leaf and assertion unchanged unless the implementation’s Public path requires a non-public fixture to cover Medium.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Nitpick comments:
In `@tests/client/PlayerStatsSummary.test.ts`:
- Around line 373-392: Update the test setup for “keeps a leaf's own recent
stats when no tree-level aggregate exists” by assigning tree.selectedType =
GameType.Public and tree.selectedMode = GameMode.FFA before appending the tree,
so it exercises the exact-selection branch rather than all-types aggregation.
Keep the existing leaf and assertion unchanged unless the implementation’s
Public path requires a non-public fixture to cover Medium.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: 7ed436b1-e102-4d98-9ac5-58efbb5d06af
📒 Files selected for processing (8)
resources/lang/en.jsonsrc/client/components/baseComponents/stats/PlayerStatsSummary.tssrc/client/components/baseComponents/stats/PlayerStatsTable.tssrc/client/components/baseComponents/stats/PlayerStatsTree.tssrc/core/ApiSchemas.tstests/StatsSchema.test.tstests/client/PlayerStatsSummary.test.tstests/client/PlayerStatsTable.test.ts
🚧 Files skipped from review as they are similar to previous changes (7)
- tests/StatsSchema.test.ts
- tests/client/PlayerStatsTable.test.ts
- src/core/ApiSchemas.ts
- resources/lang/en.json
- src/client/components/baseComponents/stats/PlayerStatsTable.ts
- src/client/components/baseComponents/stats/PlayerStatsSummary.ts
- src/client/components/baseComponents/stats/PlayerStatsTree.ts
Description:
Please complete the following:
Please put your Discord username so you can be contacted if a bug or regression is found:
w.o.n