feat(data): normalize player dataset to 2022 FIFA World Cup Argentina squad (#435)#437
Merged
nanotaboada merged 7 commits intomasterfrom Mar 30, 2026
Merged
Conversation
|
Warning Rate limit exceeded
Your organization is not enrolled in usage-based pricing. Contact your admin to enable usage-based pricing to continue reviews beyond the rate limit, or try again in 5 minutes and 27 seconds. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (1)
WalkthroughNormalize in-repo player fixtures to the canonical 2022 Argentina squad: add Giovani Lo Celso (squad 27), correct three players' team/league values, replace hardcoded UUIDs with deterministic UUID v5 in PlayerData, align test fixtures/usages to use squad 27 for Create/Delete, update CHANGELOG and SonarCloud/.coderabbit config. Changes
Sequence Diagram(s)(omitted — changes are data/fixture updates and test/config adjustments without new multi-component control flow) Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Assessment against linked issues
Out-of-scope changes
Possibly related issues
Possibly related PRs
🚥 Pre-merge checks | ✅ 1 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (1 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 |
nanotaboada
force-pushed
the
feat/normalize-player-dataset
branch
from
2b35c8a to
27dbb85
Compare
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/Dotnet.Samples.AspNetCore.WebApi/Utilities/PlayerData.cs (1)
868-903:⚠️ Potential issue | 🟡 MinorData inconsistency:
MakeStarting11FromDeserializedJson()still contains outdated team/league values.The embedded JSON literal was not updated to match the November 2022 snapshot corrections applied to
MakeStarting11()andMakeStarting11WithId():
Player Current JSON value Expected value Enzo Fernández Chelsea FC / Premier League SL Benfica / Liga Portugal Mac Allister Liverpool FC Brighton & Hove Albion Messi Inter Miami CF / Major League Soccer Paris Saint-Germain / Ligue 1 This could cause confusion if this method is used for testing or demonstration purposes.
🛠️ Suggested fix
{ "firstName": "Enzo", "middleName": "Jeremías", "lastName": "Fernández", "dateOfBirth": "2001-01-16T00:00:00.000Z", "squadNumber": 24, "position": "Central Midfield", "abbrPosition": "CM", - "team": "Chelsea FC", - "league": "Premier League", + "team": "SL Benfica", + "league": "Liga Portugal", "starting11": true }, { "firstName": "Alexis", "middleName": null, "lastName": "Mac Allister", "dateOfBirth": "1998-12-23T00:00:00.000Z", "squadNumber": 20, "position": "Central Midfield", "abbrPosition": "CM", - "team": "Liverpool FC", + "team": "Brighton & Hove Albion", "league": "Premier League", "starting11": true }, { "firstName": "Lionel", "middleName": "Andrés", "lastName": "Messi", "dateOfBirth": "1987-06-23T00:00:00.000Z", "squadNumber": 10, "position": "Right Winger", "abbrPosition": "RW", - "team": "Inter Miami CF", - "league": "Major League Soccer", + "team": "Paris Saint-Germain", + "league": "Ligue 1", "starting11": true },🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/Dotnet.Samples.AspNetCore.WebApi/Utilities/PlayerData.cs` around lines 868 - 903, Update the embedded JSON literal inside MakeStarting11FromDeserializedJson() so the team/league entries match the corrected snapshot used by MakeStarting11() and MakeStarting11WithId(): change Enzo Fernández's team/league to "SL Benfica" / "Liga Portugal", change Alexis Mac Allister's team to "Brighton & Hove Albion", and change Lionel Messi's team/league to "Paris Saint-Germain" / "Ligue 1"; ensure the JSON keys (firstName, lastName, squadNumber, position, abbrPosition, team, league, starting11) remain unchanged and the literal parses correctly when deserialized.
🧹 Nitpick comments (1)
test/Dotnet.Samples.AspNetCore.WebApi.Tests/Unit/PlayerControllerTests.cs (1)
489-490: Test alignment with Lo Celso (squad 27) looks correct.The change properly aligns the delete test with the new canonical Create/Delete fixture. However, consider deriving
squadNumberfrom the response model to ensure consistency if the fixture ever changes:♻️ Optional: derive squadNumber from response
- var squadNumber = 27; var response = PlayerFakes.MakeResponseModelForCreate(); + var squadNumber = response.Dorsal;🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@test/Dotnet.Samples.AspNetCore.WebApi.Tests/Unit/PlayerControllerTests.cs` around lines 489 - 490, The test currently hardcodes var squadNumber = 27 while also calling PlayerFakes.MakeResponseModelForCreate(); update the test to derive squadNumber from the response model (e.g., read the squad number property off the object returned by PlayerFakes.MakeResponseModelForCreate()) so the Create/Delete fixture remains consistent if the fake changes; locate where squadNumber is declared and replace the literal with response.<squadNumberProperty> (use the actual property name on the response model).
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Outside diff comments:
In `@src/Dotnet.Samples.AspNetCore.WebApi/Utilities/PlayerData.cs`:
- Around line 868-903: Update the embedded JSON literal inside
MakeStarting11FromDeserializedJson() so the team/league entries match the
corrected snapshot used by MakeStarting11() and MakeStarting11WithId(): change
Enzo Fernández's team/league to "SL Benfica" / "Liga Portugal", change Alexis
Mac Allister's team to "Brighton & Hove Albion", and change Lionel Messi's
team/league to "Paris Saint-Germain" / "Ligue 1"; ensure the JSON keys
(firstName, lastName, squadNumber, position, abbrPosition, team, league,
starting11) remain unchanged and the literal parses correctly when deserialized.
---
Nitpick comments:
In `@test/Dotnet.Samples.AspNetCore.WebApi.Tests/Unit/PlayerControllerTests.cs`:
- Around line 489-490: The test currently hardcodes var squadNumber = 27 while
also calling PlayerFakes.MakeResponseModelForCreate(); update the test to derive
squadNumber from the response model (e.g., read the squad number property off
the object returned by PlayerFakes.MakeResponseModelForCreate()) so the
Create/Delete fixture remains consistent if the fake changes; locate where
squadNumber is declared and replace the literal with
response.<squadNumberProperty> (use the actual property name on the response
model).
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 7bf9ce87-d038-4bfd-a48e-fa26e1eedf25
⛔ Files ignored due to path filters (6)
src/Dotnet.Samples.AspNetCore.WebApi/Migrations/20250414191223_InitialCreate.csis excluded by!**/Migrations/**src/Dotnet.Samples.AspNetCore.WebApi/Migrations/20250414195445_SeedStarting11.csis excluded by!**/Migrations/**src/Dotnet.Samples.AspNetCore.WebApi/Migrations/20251221220614_SeedSubstitutes.csis excluded by!**/Migrations/**src/Dotnet.Samples.AspNetCore.WebApi/Migrations/20260329000000_NormalizePlayerDataset.Designer.csis excluded by!**/Migrations/**,!**/*.Designer.cssrc/Dotnet.Samples.AspNetCore.WebApi/Migrations/20260329000000_NormalizePlayerDataset.csis excluded by!**/Migrations/**src/Dotnet.Samples.AspNetCore.WebApi/storage/players-sqlite3.dbis excluded by!**/*.db,!**/storage/**,!**/*.db
📒 Files selected for processing (9)
CHANGELOG.mdsrc/Dotnet.Samples.AspNetCore.WebApi/Configurations/AuthorizeCheckOperationFilter.cssrc/Dotnet.Samples.AspNetCore.WebApi/Controllers/PlayerController.cssrc/Dotnet.Samples.AspNetCore.WebApi/Extensions/ServiceCollectionExtensions.cssrc/Dotnet.Samples.AspNetCore.WebApi/Middlewares/ExceptionMiddleware.cssrc/Dotnet.Samples.AspNetCore.WebApi/Utilities/PlayerData.cssrc/Dotnet.Samples.AspNetCore.WebApi/Utilities/SwaggerUtilities.cstest/Dotnet.Samples.AspNetCore.WebApi.Tests/Unit/PlayerControllerTests.cstest/Dotnet.Samples.AspNetCore.WebApi.Tests/Utilities/PlayerFakes.cs
Co-authored-by: Claude Sonnet 4.6 <noreply@anthropic.com>
nanotaboada
force-pushed
the
feat/normalize-player-dataset
branch
from
27dbb85 to
14efe16
Compare
Co-authored-by: Claude Sonnet 4.6 <noreply@anthropic.com>
nanotaboada
force-pushed
the
feat/normalize-player-dataset
branch
from
957fb83 to
690e8f6
Compare
Co-authored-by: Claude Sonnet 4.6 <noreply@anthropic.com>
There was a problem hiding this comment.
🧹 Nitpick comments (1)
sonar-project.properties (1)
43-44: RecheckProgram.cscoverage exclusion scope.At Line 44, excluding
**/Program.csis only safe if startup is boilerplate. If it contains custom middleware/auth/DI wiring, keep it covered (or cover via integration tests) so configuration regressions are visible.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@sonar-project.properties` around lines 43 - 44, Review Program.cs for any non-boilerplate startup code (custom middleware, auth setup, DI wiring, hosted service registration); if it contains such logic, remove the exclusion pattern "**/Program.cs" from sonar-project.properties so SonarQube measures its coverage, or narrow the exclusion to a safe subset instead of excluding the whole file; if you prefer to keep it excluded, add integration tests that exercise the middleware/DI/auth paths (the startup wiring in Program.cs) so configuration regressions are covered.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@sonar-project.properties`:
- Around line 43-44: Review Program.cs for any non-boilerplate startup code
(custom middleware, auth setup, DI wiring, hosted service registration); if it
contains such logic, remove the exclusion pattern "**/Program.cs" from
sonar-project.properties so SonarQube measures its coverage, or narrow the
exclusion to a safe subset instead of excluding the whole file; if you prefer to
keep it excluded, add integration tests that exercise the middleware/DI/auth
paths (the startup wiring in Program.cs) so configuration regressions are
covered.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: df579ad0-74a6-451d-b5c3-bde2fbf3cada
📒 Files selected for processing (4)
.coderabbit.yamlsonar-project.propertiessrc/Dotnet.Samples.AspNetCore.WebApi/Utilities/PlayerData.cstest/Dotnet.Samples.AspNetCore.WebApi.Tests/Unit/PlayerControllerTests.cs
✅ Files skipped from review due to trivial changes (1)
- .coderabbit.yaml
🚧 Files skipped from review as they are similar to previous changes (1)
- src/Dotnet.Samples.AspNetCore.WebApi/Utilities/PlayerData.cs
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
.sonarcloud.properties (1)
21-30: Wildcard patterns aren't supported in.sonarcloud.properties; use SonarCloud UI or migrate to CI-based config for maintainable exclusions.Manually enumerating every migration and test utility file is unsustainable.
.sonarcloud.propertiesdoesn't support wildcard patterns (*,**) forsonar.exclusions,sonar.test.exclusions, orsonar.cpd.exclusions—paths must be explicitly listed.Two recommended alternatives:
- SonarCloud UI (simpler): Project Administration → General Settings → Analysis Scope → exclude paths like
**/Migrations/*.cs(supports full wildcard patterns). No code changes needed.- CI-based analysis: Migrate to a CI workflow with
sonar-project.properties(e.g., GitHub Actions), which supports wildcards in SonarScanner properties.Alternatively, generate these lists via a build step, but the UI or CI approach is preferred for maintainability.
Also applies to: 38-57, 78-98
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In @.sonarcloud.properties around lines 21 - 30, The .sonarcloud.properties file is using sonar.exclusions to list many paths but that file does not support wildcard patterns—move these exclusions out of the checked-in .sonarcloud.properties and either (A) configure the wildcard exclusions in the SonarCloud UI under Project Administration → General Settings → Analysis Scope using patterns like **/Migrations/*.cs, or (B) migrate analysis to a CI-based sonar-project.properties (used by your SonarScanner in the pipeline) where sonar.exclusions, sonar.test.exclusions and sonar.cpd.exclusions can contain wildcards; update the repo by removing or minimizing the long explicit lists from .sonarcloud.properties and implement one of the two alternatives so future migration/test files can be excluded with wildcards.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In @.sonarcloud.properties:
- Around line 9-10: Update the stale comment in .sonarcloud.properties that
references "sonar-project.properties": remove or reword the line
"sonar-project.properties is kept for documentation and as a fallback if
CI-based analysis is ever introduced." so it no longer claims a file that this
PR deletes; ensure the comment accurately reflects current repo state (either
drop the fallback note or point to the correct alternate file/location).
---
Nitpick comments:
In @.sonarcloud.properties:
- Around line 21-30: The .sonarcloud.properties file is using sonar.exclusions
to list many paths but that file does not support wildcard patterns—move these
exclusions out of the checked-in .sonarcloud.properties and either (A) configure
the wildcard exclusions in the SonarCloud UI under Project Administration →
General Settings → Analysis Scope using patterns like **/Migrations/*.cs, or (B)
migrate analysis to a CI-based sonar-project.properties (used by your
SonarScanner in the pipeline) where sonar.exclusions, sonar.test.exclusions and
sonar.cpd.exclusions can contain wildcards; update the repo by removing or
minimizing the long explicit lists from .sonarcloud.properties and implement one
of the two alternatives so future migration/test files can be excluded with
wildcards.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: cc95be09-3e02-4c5d-ba24-966480cb96aa
⛔ Files ignored due to path filters (2)
src/Dotnet.Samples.AspNetCore.WebApi/Migrations/20260329000000_NormalizePlayerDataset.csis excluded by!**/Migrations/**src/Dotnet.Samples.AspNetCore.WebApi/storage/players-sqlite3.dbis excluded by!**/*.db,!**/storage/**,!**/*.db
📒 Files selected for processing (3)
.sonarcloud.propertiessonar-project.propertiessrc/Dotnet.Samples.AspNetCore.WebApi/Utilities/PlayerData.cs
💤 Files with no reviewable changes (1)
- sonar-project.properties
🚧 Files skipped from review as they are similar to previous changes (1)
- src/Dotnet.Samples.AspNetCore.WebApi/Utilities/PlayerData.cs
…#435) Co-authored-by: Claude Sonnet 4.6 <noreply@anthropic.com>
|
This was referenced Apr 7, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
FIFA_WORLD_CUP_QATAR_2022_ARGENTINA_SQUAD)NormalizePlayerDatasetto apply all data changes incrementallystorage/players-sqlite3.dbfrom migrations (27 players)PlayerFakes.MakeNew()to Lo Celso (squad 27) and update Delete controller test accordinglyTest plan
dotnet build --configuration Release— passes (0 warnings, 0 errors)dotnet test --settings .runsettings— 39/39 passedCloses #435
🤖 Generated with Claude Code
Summary by CodeRabbit
New Features
Bug Fixes
Changed
Tests
Chores