-
Notifications
You must be signed in to change notification settings - Fork 15
feat: add 15 substitute players to database via migration #334
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
Added GetSubstitutes() and GetSubstitutesWithId() methods to PlayerData utility, following the same pattern as Starting 11. Created SeedSubstitutes migration to seed 15 substitute players with squad numbers 1, 2, 4, 5, 6, 8, 12, 14, 15, 16, 17, 18, 21, 22, 25. Database now contains 26 total players: 11 starting (Starting11=true) and 15 substitutes (Starting11=false). All squad numbers verified conflict-free. Additional improvements: - Fixed run-migrations-and-copy-database.sh to use storage/ path - Updated copilot-instructions.md with correct database paths - Removed obsolete Data/players-sqlite3.db file Closes #332
…Core.WebApi into feature/seed-substitute-players
WalkthroughAdds 15 substitute player seeds via two new PlayerData methods, updates the migration/copy script to use Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes
Possibly related PRs
Pre-merge checks and finishing touches✅ Passed checks (5 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
📜 Recent review detailsConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro 📒 Files selected for processing (4)
🧰 Additional context used📓 Path-based instructions (1)**/*.cs📄 CodeRabbit inference engine (.github/copilot-instructions.md)
Files:
⚙️ CodeRabbit configuration file
Files:
🧠 Learnings (19)📓 Common learnings📚 Learning: 2025-12-11T00:21:00.156ZApplied to files:
📚 Learning: 2025-12-11T00:21:37.722ZApplied to files:
📚 Learning: 2025-12-11T00:21:37.722ZApplied to files:
📚 Learning: 2025-12-11T00:21:37.722ZApplied to files:
📚 Learning: 2025-12-11T00:21:37.722ZApplied to files:
📚 Learning: 2025-12-11T00:21:37.722ZApplied to files:
📚 Learning: 2025-12-11T00:21:37.722ZApplied to files:
📚 Learning: 2025-12-11T00:21:00.156ZApplied to files:
📚 Learning: 2025-12-11T00:21:05.739ZApplied to files:
📚 Learning: 2025-12-11T00:21:00.156ZApplied to files:
📚 Learning: 2025-12-11T00:21:00.156ZApplied to files:
📚 Learning: 2025-12-11T00:21:05.739ZApplied to files:
📚 Learning: 2025-12-11T00:21:00.156ZApplied to files:
📚 Learning: 2025-12-11T00:21:00.156ZApplied to files:
📚 Learning: 2025-12-11T00:21:37.722ZApplied to files:
📚 Learning: 2025-12-11T00:21:05.739ZApplied to files:
📚 Learning: 2025-12-11T00:20:35.681ZApplied to files:
📚 Learning: 2025-12-11T00:20:54.702ZApplied to files:
⏰ Context from checks skipped due to timeout of 120000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
🔇 Additional comments (7)
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 |
Coverage summary from CodacySee diff coverage on Codacy
Coverage variation details
Coverage variation is the difference between the coverage for the head and common ancestor commits of the pull request branch: Diff coverage details
Diff coverage is the percentage of lines that are covered by tests out of the coverable lines that the pull request added or modified: See your quality gate settings Change summary preferences |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 1
🧹 Nitpick comments (1)
src/Dotnet.Samples.AspNetCore.WebApi/Utilities/PlayerData.cs (1)
314-725: Consider refactoring to reduce duplication.GetSubstitutes() and GetSubstitutesWithId() contain significant code duplication (only the Id field differs). While this pattern is consistent with the existing MakeStarting11() / MakeStarting11WithId() methods, consider extracting a shared builder method to improve maintainability.
💡 Potential refactoring approach
private static Player CreateSubstitutePlayer( string firstName, string? middleName, string lastName, DateTime dateOfBirth, int squadNumber, string position, string abbrPosition, string team, string league, Guid? id = null) { var player = new Player { FirstName = firstName, MiddleName = middleName, LastName = lastName, DateOfBirth = dateOfBirth, SquadNumber = squadNumber, Position = position, AbbrPosition = abbrPosition, Team = team, League = league, Starting11 = false }; if (id.HasValue) player.Id = id.Value; return player; }Then call this helper from both methods. Note: This same refactoring could apply to the existing Starting11 methods for consistency.
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (4)
src/Dotnet.Samples.AspNetCore.WebApi/Migrations/20251221220614_SeedSubstitutes.Designer.csis excluded by!**/Migrations/**,!**/*.Designer.cssrc/Dotnet.Samples.AspNetCore.WebApi/Migrations/20251221220614_SeedSubstitutes.csis excluded by!**/Migrations/**src/Dotnet.Samples.AspNetCore.WebApi/Migrations/PlayerDbContextModelSnapshot.csis excluded by!**/Migrations/**src/Dotnet.Samples.AspNetCore.WebApi/storage/players-sqlite3.dbis excluded by!**/*.db,!**/storage/**,!**/*.db
📒 Files selected for processing (4)
.github/copilot-instructions.md(2 hunks)README.md(1 hunks)scripts/run-migrations-and-copy-database.sh(1 hunks)src/Dotnet.Samples.AspNetCore.WebApi/Utilities/PlayerData.cs(1 hunks)
🧰 Additional context used
📓 Path-based instructions (1)
**/*.cs
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
**/*.cs: UsePascalCasefor class names, methods, and public properties in C#
UsecamelCasefor local variables and private fields in C#
Useasync/awaitconsistently for asynchronous code in C#
Prefervarfor local variable declarations where the type is obvious in C#
Nullable reference types are enabled in C# projects
UseCSharpierformatting standards for code style in C#
Do not useConfigureAwait(false)in ASP.NET Core contexts in C#
Implement proper logging with structured logging patterns usingILogger<T>in ASP.NET Core
Format C# code using CSharpier for consistent code style across the project
Files:
src/Dotnet.Samples.AspNetCore.WebApi/Utilities/PlayerData.cs
⚙️ CodeRabbit configuration file
**/*.cs: - Follow C# naming conventions: PascalCase for classes/methods/properties, camelCase for local variables
- Ensure nullable reference types are handled properly
- Verify async/await patterns are used consistently
- Check for proper dependency injection usage
- Validate that var is used appropriately for obvious types
- Ensure ILogger is used for logging, not Console.WriteLine
- Code should follow CSharpier formatting standards
Files:
src/Dotnet.Samples.AspNetCore.WebApi/Utilities/PlayerData.cs
🧠 Learnings (9)
📓 Common learnings
Learnt from: CR
Repo: nanotaboada/Dotnet.Samples.AspNetCore.WebApi PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-12-11T00:21:37.722Z
Learning: Applies to **/Models/*.cs : Use the Player entity with properties: Squad Numbers (unique, 1-99), Positions (2-character abbreviations: GK, RB, LB, CB, DM, CM, RW, AM, CF, SS, LW), Starting11 (boolean), Team, and League
📚 Learning: 2025-12-11T00:21:37.722Z
Learnt from: CR
Repo: nanotaboada/Dotnet.Samples.AspNetCore.WebApi PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-12-11T00:21:37.722Z
Learning: Applies to **/Models/*.cs : Use the Player entity with properties: Squad Numbers (unique, 1-99), Positions (2-character abbreviations: GK, RB, LB, CB, DM, CM, RW, AM, CF, SS, LW), Starting11 (boolean), Team, and League
Applied to files:
README.mdsrc/Dotnet.Samples.AspNetCore.WebApi/Utilities/PlayerData.cs
📚 Learning: 2025-12-11T00:21:37.722Z
Learnt from: CR
Repo: nanotaboada/Dotnet.Samples.AspNetCore.WebApi PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-12-11T00:21:37.722Z
Learning: Applies to **/Repositories/*.cs : Write EF Core queries using LINQ in C# data access code
Applied to files:
README.md
📚 Learning: 2025-12-11T00:21:37.722Z
Learnt from: CR
Repo: nanotaboada/Dotnet.Samples.AspNetCore.WebApi PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-12-11T00:21:37.722Z
Learning: Applies to **/Services/*.cs : Use AutoMapper for object transformations between entities and DTOs in ASP.NET Core services
Applied to files:
README.md
📚 Learning: 2025-12-11T00:21:37.722Z
Learnt from: CR
Repo: nanotaboada/Dotnet.Samples.AspNetCore.WebApi PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-12-11T00:21:37.722Z
Learning: Applies to {Program.cs,Startup.cs} : Use `DbContextPool` for better EF Core performance in ASP.NET Core applications
Applied to files:
README.md
📚 Learning: 2025-12-11T00:21:37.722Z
Learnt from: CR
Repo: nanotaboada/Dotnet.Samples.AspNetCore.WebApi PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-12-11T00:21:37.722Z
Learning: Applies to **/Mappings/*.cs : Use AutoMapper for clean object transformations between domain entities and DTOs in C#
Applied to files:
README.md
📚 Learning: 2025-12-11T00:21:37.722Z
Learnt from: CR
Repo: nanotaboada/Dotnet.Samples.AspNetCore.WebApi PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-12-11T00:21:37.722Z
Learning: Applies to **/test/**/*.cs : Place unit tests under `test/` folder following the structure of the main project (e.g., `Unit/PlayerServiceTests.cs`)
Applied to files:
README.md
📚 Learning: 2025-12-11T00:20:35.681Z
Learnt from: CR
Repo: nanotaboada/Dotnet.Samples.AspNetCore.WebApi PR: 0
File: Dockerfile:0-0
Timestamp: 2025-12-11T00:20:35.681Z
Learning: Applies to Dockerfile : Copy pre-seeded database files and persistent storage directories into the container during build, and configure volume mount points for Compose orchestration
Applied to files:
.github/copilot-instructions.md
📚 Learning: 2025-12-11T00:20:54.702Z
Learnt from: CR
Repo: nanotaboada/Dotnet.Samples.AspNetCore.WebApi PR: 0
File: compose.yaml:0-0
Timestamp: 2025-12-11T00:20:54.702Z
Learning: Applies to **/docker-compose.{yml,yaml} : Use environment variables to configure application paths and settings (e.g., STORAGE_PATH=/storage/players-sqlite3.db) rather than hardcoding values
Applied to files:
.github/copilot-instructions.md
⏰ Context from checks skipped due to timeout of 120000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: Codacy Static Code Analysis
🔇 Additional comments (5)
README.md (1)
78-78: LGTM! Documentation clarification is accurate.The updated comment correctly reflects that the Data/ directory contains both the EF Core DbContext and migrations.
scripts/run-migrations-and-copy-database.sh (1)
8-9: LGTM! Path updates align with the project's storage reorganization.The script now correctly references the storage/ directory for both source and target paths, consistent with the broader PR changes and documentation updates.
src/Dotnet.Samples.AspNetCore.WebApi/Utilities/PlayerData.cs (2)
314-511: LGTM! GetSubstitutes() implementation meets requirements.The method correctly returns 15 substitute players with:
- Squad numbers matching issue #332 requirements (1,2,4,5,6,8,12,14,15,16,17,18,21,22,25)
- All players marked as Starting11 = false
- No conflicts with starting 11 squad numbers
- Correct position abbreviations per coding guidelines
513-725: LGTM! GetSubstitutesWithId() follows established pattern.This method mirrors GetSubstitutes() with hardcoded GUIDs, consistent with the existing MakeStarting11() / MakeStarting11WithId() pattern in the codebase. The deterministic GUIDs enable reproducible migrations as specified in the PR objectives.
.github/copilot-instructions.md (1)
297-316: LGTM! Comprehensive database workflow documentation.The new "Database Workflow Explained" section clearly documents:
- The three use cases (Clone & Run, Recreate from Scratch, Docker)
- The migration workflow with specific paths
- The 26-player seed data (11 starting + 15 substitutes)
- The dotnet-ef CLI requirement
The updated path references (lines 343-345) correctly distinguish between Development, Container, and Runtime environments, which will help developers understand the storage structure.
Also applies to: 343-345
|
@coderabbitai generate docstrings |
✅ Actions performedInitiated docstring generation; will generate only if new commits exist. |
|
Note Docstrings generation - SUCCESS |
Docstrings generation was requested by @nanotaboada. * #334 (comment) The following files were modified: * `scripts/run-migrations-and-copy-database.sh` * `src/Dotnet.Samples.AspNetCore.WebApi/Utilities/PlayerData.cs`
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 0
🧹 Nitpick comments (2)
src/Dotnet.Samples.AspNetCore.WebApi/Utilities/PlayerData.cs (2)
314-511: LGTM! Team name typos have been corrected.The method implementation is correct and follows the established pattern. All 15 substitute players have
Starting11 = falseand use the expected squad numbers (1, 2, 4, 5, 6, 8, 12, 14, 15, 16, 17, 18, 21, 22, 25). The team name typos flagged in the previous review ("Villareal" → "Villarreal" at line 352, and "Nottingham Forrest" → "Nottingham Forest" at line 365) have been corrected.📝 Optional: Consider adding XML documentation
For consistency with
MakeStarting11FromDeserializedJson(), consider adding an XML documentation comment:+ /// <summary> + /// Returns a list of 15 substitute players with Starting11 set to false. + /// This method does not include Id fields. + /// </summary> + /// <returns>A list of substitute Player entities.</returns> public static List<Player> GetSubstitutes()
513-725: LGTM! Deterministic GUIDs and data consistency maintained.The method correctly provides hardcoded GUIDs for each substitute player, enabling deterministic database migrations. The data is consistent with
GetSubstitutes()(same players, squad numbers, teams, and leagues), and team name typos have been corrected at lines 554 and 568.📝 Optional: Consider adding XML documentation
For consistency and clarity, consider adding an XML documentation comment:
+ /// <summary> + /// Returns a list of 15 substitute players with Starting11 set to false. + /// This method includes deterministic Id (GUID) fields for use in database migrations. + /// </summary> + /// <returns>A list of substitute Player entities with hardcoded Ids.</returns> public static List<Player> GetSubstitutesWithId()
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
src/Dotnet.Samples.AspNetCore.WebApi/Utilities/PlayerData.cs
🧰 Additional context used
📓 Path-based instructions (1)
**/*.cs
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
**/*.cs: UsePascalCasefor class names, methods, and public properties in C#
UsecamelCasefor local variables and private fields in C#
Useasync/awaitconsistently for asynchronous code in C#
Prefervarfor local variable declarations where the type is obvious in C#
Nullable reference types are enabled in C# projects
UseCSharpierformatting standards for code style in C#
Do not useConfigureAwait(false)in ASP.NET Core contexts in C#
Implement proper logging with structured logging patterns usingILogger<T>in ASP.NET Core
Format C# code using CSharpier for consistent code style across the project
Files:
src/Dotnet.Samples.AspNetCore.WebApi/Utilities/PlayerData.cs
⚙️ CodeRabbit configuration file
**/*.cs: - Follow C# naming conventions: PascalCase for classes/methods/properties, camelCase for local variables
- Ensure nullable reference types are handled properly
- Verify async/await patterns are used consistently
- Check for proper dependency injection usage
- Validate that var is used appropriately for obvious types
- Ensure ILogger is used for logging, not Console.WriteLine
- Code should follow CSharpier formatting standards
Files:
src/Dotnet.Samples.AspNetCore.WebApi/Utilities/PlayerData.cs
🧠 Learnings (2)
📓 Common learnings
Learnt from: CR
Repo: nanotaboada/Dotnet.Samples.AspNetCore.WebApi PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-12-11T00:21:37.722Z
Learning: Applies to **/Models/*.cs : Use the Player entity with properties: Squad Numbers (unique, 1-99), Positions (2-character abbreviations: GK, RB, LB, CB, DM, CM, RW, AM, CF, SS, LW), Starting11 (boolean), Team, and League
📚 Learning: 2025-12-11T00:21:37.722Z
Learnt from: CR
Repo: nanotaboada/Dotnet.Samples.AspNetCore.WebApi PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-12-11T00:21:37.722Z
Learning: Applies to **/Models/*.cs : Use the Player entity with properties: Squad Numbers (unique, 1-99), Positions (2-character abbreviations: GK, RB, LB, CB, DM, CM, RW, AM, CF, SS, LW), Starting11 (boolean), Team, and League
Applied to files:
src/Dotnet.Samples.AspNetCore.WebApi/Utilities/PlayerData.cs
⏰ Context from checks skipped due to timeout of 120000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: Codacy Static Code Analysis
- Clarify database workflow in copilot-instructions.md with detailed explanation of migration process and three use cases - Update Data/ folder description in README.md structure diagram - Add XML documentation comments for PlayerData methods - Fix team name typos - Add log function documentation in run-migrations script
08f3ade to
b9eadfc
Compare
|



Summary
Adds 15 substitute players to the SQLite database using EF Core migrations, bringing the total roster to 26 players (11 starting + 15 substitutes).
Changes
PlayerDatautility withGetSubstitutes()andGetSubstitutesWithId()methodsSeedSubstitutesmigration (20251221220614) with hardcoded GUIDs for repeatabilitycopilot-instructions.mdREADME.mdData/database path referencesVerification
Related Issue
Closes #332
Summary by CodeRabbit
Documentation
Chores
✏️ Tip: You can customize this high-level summary in your review settings.