fix(api): return 200 OK on empty collection and ignore Id in AutoMapper (#425)

[nanotaboada]

Summary

• GET /players now returns 200 OK [] when no players exist instead of 404 Not Found
• AutoMapper Player → PlayerResponseModel profile explicitly ignores the Id source member via ForSourceMember, making the exclusion intentional and future-proof
• Updated controller test: Get_Players_NonExisting_Returns404NotFound → Get_Players_Empty_Returns200OkWithEmptyList

Test plan

• GET /players returns 200 OK with [] when collection is empty
• GET /players returns 200 OK with player list when players exist
• AutoMapper profile compiles and all 39 tests pass
• dotnet build --configuration Release succeeds
• CSharpier check passes

Closes #425

🤖 Generated with Claude Code

Summary by CodeRabbit

• Bug Fixes
  • The /players endpoint now returns HTTP 200 OK with an empty list when no players exist, instead of HTTP 404 Not Found.

[coderabbitai]

Caution

Review failed

Pull request was closed or merged during review

Walkthrough

The PR resolves two REST compliance gaps: GET /players now returns 200 OK with an empty list when no players exist instead of 404 Not Found, and the AutoMapper profile explicitly ignores the Id field during Player to PlayerResponseModel mapping to enforce intentional exclusion.

Changes

Cohort / File(s)	Summary
Controller Logic src/Dotnet.Samples.AspNetCore.WebApi/Controllers/PlayerController.cs	Removed conditional that returned 404 when player list is empty; now always returns 200 OK regardless of list size. Removed corresponding ProducesResponseType(404) annotations and added structured logging for all GET /players requests.
Mapping Configuration src/Dotnet.Samples.AspNetCore.WebApi/Mappings/PlayerMappingProfile.cs	Added ForSourceMember(source => source.Id, options => options.DoNotValidate()) to the Player → PlayerResponseModel AutoMapper map to explicitly ignore the Id field during mapping.
Unit Tests test/Dotnet.Samples.AspNetCore.WebApi.Tests/Unit/PlayerControllerTests.cs	Renamed test method from Get_Players_NonExisting_Returns404NotFound to Get_Players_Empty_Returns200OkWithEmptyList and updated assertions to expect 200 OK with a non-null empty list instead of 404 ProblemDetails.
Changelog CHANGELOG.md	Documented the two REST compliance fixes in the Unreleased/Fixed section.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

Assessment against linked issues

Objective	Addressed	Explanation
GET /players returns 200 OK [] when no players exist (#425)	✅
AutoMapper profile explicitly ignores Id on Player → PlayerResponseModel mapping (#425)	✅
Tests updated to cover empty-collection case (#425)	✅
All pre-commit checks pass (#425)	❓	Cannot verify pre-commit execution status from code diff alone.

Possibly related PRs

• refactor(tests): rename test methods to Microsoft naming standard #399: Directly updates PlayerController tests and GET /players response expectations (same test method renamed and same endpoint behavior modified).
• refactor(api)!: add /squadNumber/ path segment to routes #420: Modifies PlayerController.GetAsync response behavior and ProducesResponseType annotations (overlaps same controller method changes).
• feat(service)!: migrate to AutoMapper for Model mapping #194: Introduces the Player → PlayerResponseModel AutoMapper mapping that is now being enhanced with explicit Id ignore configuration.

🚥 Pre-merge checks | ✅ 1 | ❌ 1

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 40.00% which is insufficient. The required threshold is 80.00%.	Write docstrings for the functions missing them to satisfy the coverage threshold.

✅ Passed checks (1 passed)

Check name	Status	Explanation
Title check	✅ Passed	The title clearly and specifically describes the two main changes: returning 200 OK for empty collections and making AutoMapper's Id ignore explicit. It follows Conventional Commits format (fix:) and is under 80 characters.

_{✏️ Tip: You can configure your own custom pre-merge checks in the settings.}

✨ Finishing Touches

📝 Generate docstrings

• Create stacked PR
• Commit on current branch

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Commit unit tests in branch feat/rest-polish-empty-collection-automapper-id

• 🛠️ sync documentation: Commit on current branch
• 🛠️ sync documentation: Create PR
• 🛠️ enforce http error handling: Commit on current branch
• 🛠️ enforce http error handling: Create PR
• 🛠️ idiomatic review: Commit on current branch
• 🛠️ idiomatic review: Create PR
• 🛠️ verify api contract: Commit on current branch
• 🛠️ verify api contract: Create PR

---

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.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

[sonarqubecloud]

Quality Gate passed

Issues
0 New issues
0 Accepted issues

Measures
0 Security Hotspots
0.0% Coverage on New Code
0.0% Duplication on New Code

See analysis details on SonarQube Cloud

[nanotaboada]

@coderabbitai review

[coderabbitai]

✅ Actions performed

Review triggered.

Note: CodeRabbit is an incremental review system and does not re-review already reviewed commits. This command is applicable only when automatic reviews are paused.