feat(validation): add DateOfBirth rules and fix Birth null mapping#397
Merged
nanotaboada merged 5 commits intomasterfrom Mar 7, 2026
Merged
Conversation
) - Add optional DateOfBirth range validation (past, >= 1900-01-01) in PlayerRequestModelValidator using FluentValidation When(HasValue) - Fix PlayerMappingProfile to return null instead of "" for Birth when DateOfBirth is not provided - Add PlayerValidatorTests covering all validator rule branches - Update PlayerFakes Birth computation to match null-safe mapping - Add TestResults/ to .gitignore (exposed by running coverage suite) Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
WalkthroughAdds conditional DateOfBirth validation (must be past and ≥1900 when provided), makes Player→PlayerResponseModel Birth mapping null-safe (returns null when no date), adds unit tests for validator and updates test fakes, plus a VS Code MCP config and .gitignore entry for TestResults/. Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related issues
Possibly related PRs
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 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
changed the title
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (2)
test/Dotnet.Samples.AspNetCore.WebApi.Tests/Utilities/PlayerFakes.cs (1)
104-104: LGTM!The Birth formatting logic correctly mirrors the production mapping in
PlayerMappingProfile.cs, ensuring test data consistency.Consider extracting a helper method to reduce duplication of the formatting logic across the three call sites:
private static string? FormatBirth(DateTime? dateOfBirth) => dateOfBirth.HasValue ? $"{dateOfBirth.Value:MMMM d, yyyy}" : null;Also applies to: 160-160, 184-186
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@test/Dotnet.Samples.AspNetCore.WebApi.Tests/Utilities/PlayerFakes.cs` at line 104, Extract the duplicated birth-formatting logic into a private helper (e.g., FormatBirth) inside PlayerFakes so all three call sites use it instead of repeating the ternary; replace instances like Birth = player.DateOfBirth.HasValue ? $"{player.DateOfBirth.Value:MMMM d, yyyy}" : null with Birth = FormatBirth(player.DateOfBirth) and implement FormatBirth(DateTime? dateOfBirth) to return the same formatted string or null.test/Dotnet.Samples.AspNetCore.WebApi.Tests/Unit/PlayerValidatorTests.cs (1)
172-224: Comprehensive DateOfBirth test coverage.The three test cases properly exercise all validation branches:
null→ passes (field is optional)- Future date → fails
- Before 1900 → fails
For consistency with the validator fix suggested earlier, consider using
DateTimeKind.Utcon line 215:request.DateOfBirth = new DateTime(1899, 12, 31, 0, 0, 0, DateTimeKind.Utc);🤖 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/PlayerValidatorTests.cs` around lines 172 - 224, Update the test that sets request.DateOfBirth to 1899-12-31 to construct the DateTime with DateTimeKind.Utc so the validator sees a UTC date; locate the test method GivenValidateAsync_WhenDateOfBirthIsBeforeYear1900_ThenValidationShouldFail and modify the assignment to request.DateOfBirth to use new DateTime(1899, 12, 31, 0, 0, 0, DateTimeKind.Utc) (ensuring consistency with other tests that use UTC and the validator expectations).
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In @.vscode/mcp.json:
- Around line 23-26: Update the "github_token" prompt entry's description to
instruct contributors to use fine-grained personal access tokens (not classic
PATs), enumerate the minimum repository permissions required (Contents: read or
read & write, Issues: read or read & write, Pull requests: read or read & write
— adjust to actual usage), and state that if a classic PAT is unavoidable the
minimum scope required is repo; locate the entry by the id "github_token" and
modify its "description" field accordingly to include these permission and
token-type guidance.
In
`@src/Dotnet.Samples.AspNetCore.WebApi/Validators/PlayerRequestModelValidator.cs`:
- Line 50: The validator's minimum date uses an unspecified DateTime which
mismatches the UTC comparison on line 48; update the `.Must(date => date >= new
DateTime(1900, 1, 1))` check to use a UTC-kind date (e.g., construct the
boundary with DateTimeKind.Utc or call DateTime.SpecifyKind) so the comparison
against DateTime.UtcNow is consistent; keep the predicate on the same lambda in
PlayerRequestModelValidator.
---
Nitpick comments:
In `@test/Dotnet.Samples.AspNetCore.WebApi.Tests/Unit/PlayerValidatorTests.cs`:
- Around line 172-224: Update the test that sets request.DateOfBirth to
1899-12-31 to construct the DateTime with DateTimeKind.Utc so the validator sees
a UTC date; locate the test method
GivenValidateAsync_WhenDateOfBirthIsBeforeYear1900_ThenValidationShouldFail and
modify the assignment to request.DateOfBirth to use new DateTime(1899, 12, 31,
0, 0, 0, DateTimeKind.Utc) (ensuring consistency with other tests that use UTC
and the validator expectations).
In `@test/Dotnet.Samples.AspNetCore.WebApi.Tests/Utilities/PlayerFakes.cs`:
- Line 104: Extract the duplicated birth-formatting logic into a private helper
(e.g., FormatBirth) inside PlayerFakes so all three call sites use it instead of
repeating the ternary; replace instances like Birth =
player.DateOfBirth.HasValue ? $"{player.DateOfBirth.Value:MMMM d, yyyy}" : null
with Birth = FormatBirth(player.DateOfBirth) and implement FormatBirth(DateTime?
dateOfBirth) to return the same formatted string or null.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 46bb7163-2305-42e5-9b55-393675543841
📒 Files selected for processing (6)
.gitignore.vscode/mcp.jsonsrc/Dotnet.Samples.AspNetCore.WebApi/Mappings/PlayerMappingProfile.cssrc/Dotnet.Samples.AspNetCore.WebApi/Validators/PlayerRequestModelValidator.cstest/Dotnet.Samples.AspNetCore.WebApi.Tests/Unit/PlayerValidatorTests.cstest/Dotnet.Samples.AspNetCore.WebApi.Tests/Utilities/PlayerFakes.cs
) - Add DateOfBirth must-be-past and >= 1900-01-01 UTC validation rules to PlayerRequestModelValidator - Extract FormatBirth helper in PlayerFakes to eliminate duplicated birth date formatting logic across test utilities - Add PlayerValidatorTests covering valid and invalid DateOfBirth cases Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
- Add .vscode/mcp.json with docker-based GitHub MCP server entry - Prompt for fine-grained PAT with repo and issues scopes Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
test/Dotnet.Samples.AspNetCore.WebApi.Tests/Unit/PlayerValidatorTests.cs (1)
172-224: Add boundary coverage fortodayand exactly1900-01-01.The new DOB tests cover null, future, and pre-1900 values, but they don’t lock down the two boundaries this PR introduces: rejecting today and accepting 1900-01-01. Adding those cases would protect the new validator rule from off-by-one regressions.
🤖 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/PlayerValidatorTests.cs` around lines 172 - 224, Add two unit tests in PlayerValidatorTests: one that sets request.DateOfBirth = DateTime.UtcNow.Date (or DateTime.UtcNow without time part) and asserts validator.ValidateAsync(request) fails and contains an error with PropertyName "DateOfBirth" to cover the "today" boundary, and another that sets request.DateOfBirth = new DateTime(1900,1,1,0,0,0,DateTimeKind.Utc) and asserts validation succeeds (IsValid true) to ensure 1900-01-01 is accepted; use the existing CreateValidator(), PlayerFakes.MakeRequestModelForCreate(), and validator.ValidateAsync(...) pattern so the new tests match the style of the surrounding tests.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In
`@src/Dotnet.Samples.AspNetCore.WebApi/Validators/PlayerRequestModelValidator.cs`:
- Around line 47-49: The DateOfBirth rule currently compares full DateTime
values (RuleFor(...).Must(date => date < DateTime.UtcNow)), which allows
same-day dates earlier than now; change the predicate to compare only the date
component (e.g., use date.Value.Date < DateTime.UtcNow.Date) so same-day birth
dates are rejected and ensure you handle the nullable DateOfBirth
(DateOfBirth.Value.Date) consistently in the Must lambda for the
PlayerRequestModelValidator.
---
Nitpick comments:
In `@test/Dotnet.Samples.AspNetCore.WebApi.Tests/Unit/PlayerValidatorTests.cs`:
- Around line 172-224: Add two unit tests in PlayerValidatorTests: one that sets
request.DateOfBirth = DateTime.UtcNow.Date (or DateTime.UtcNow without time
part) and asserts validator.ValidateAsync(request) fails and contains an error
with PropertyName "DateOfBirth" to cover the "today" boundary, and another that
sets request.DateOfBirth = new DateTime(1900,1,1,0,0,0,DateTimeKind.Utc) and
asserts validation succeeds (IsValid true) to ensure 1900-01-01 is accepted; use
the existing CreateValidator(), PlayerFakes.MakeRequestModelForCreate(), and
validator.ValidateAsync(...) pattern so the new tests match the style of the
surrounding tests.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 02244ca5-19a6-4fac-afc5-1508e2c20edc
📒 Files selected for processing (4)
.vscode/mcp.jsonsrc/Dotnet.Samples.AspNetCore.WebApi/Validators/PlayerRequestModelValidator.cstest/Dotnet.Samples.AspNetCore.WebApi.Tests/Unit/PlayerValidatorTests.cstest/Dotnet.Samples.AspNetCore.WebApi.Tests/Utilities/PlayerFakes.cs
🚧 Files skipped from review as they are similar to previous changes (2)
- .vscode/mcp.json
- test/Dotnet.Samples.AspNetCore.WebApi.Tests/Utilities/PlayerFakes.cs
#344) - Change DateOfBirth predicate from `date < DateTime.UtcNow` to `date!.Value.Date < DateTime.UtcNow.Date` to reject same-day dates - Access nullable value via `.Value.Date` consistently in both Must lambdas, safe within the When(HasValue) guard Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
|
This was referenced Mar 21, 2026
11 tasks
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.
Closes #344
Summary by CodeRabbit
New Features
Bug Fixes
Tests
Chores