Skip to content

Conversation

@paulirwin
Copy link
Member

Summary

This PR refactors the result extension methods to support RFC7807 Problem Details format and fixes the incorrect HTTP status code for authorization failures.

Fixes #8 and #10

Changes

  • Split monolithic ResultExtensions into separate concerns:
    • MinimalApiResultExtensions: Converts Result types to IResult for minimal APIs
    • MvcResultExtensions: Converts Result types to IActionResult for MVC controllers
  • Add RFC7807 Problem Details support via useProblemDetails parameter (default: true)
  • Add ValidationErrorExtensions utility class for standardized error conversion
  • Fix HTTP status code for authorization failures: 403 Forbidden instead of 401
  • Support legacy response formats when useProblemDetails=false
  • Comprehensive test coverage for all result types and response formats
  • Add example endpoints and controller demonstrating all result types

Test plan

  • All existing tests pass with improved coverage
  • Minimal API examples at /minimal-apis/results/ demonstrate all result types
  • MVC examples at /mvc/results/ demonstrate all result types
  • Problem details format properly handles validation errors and failures
  • Legacy response format still works for backward compatibility
  • HTTP 403 is correctly used for authorization failures

🤖 Generated with Claude Code

…TTP 403 fix

Split the monolithic ResultExtensions class into separate concerns:
- MinimalApiResultExtensions: Converts Result types to IResult for minimal APIs
- MvcResultExtensions: Converts Result types to IActionResult for MVC controllers

Key improvements:
- Add RFC7807 Problem Details support via useProblemDetails parameter (default: true)
- ValidationErrorExtensions provides error dictionary conversion for problem details
- Fix HTTP status code for authorization failures: use 403 Forbidden instead of 401
- Both extensions support legacy responses when useProblemDetails=false
- Comprehensive test coverage for all result types and response formats

Example usage:
- Add example minimal API endpoints at /minimal-apis/results/
- Add example MVC controller at /mvc/results/
- Register controllers and endpoints in Program.cs

🤖 Generated with [Claude Code](https://claude.com/claude-code)

Co-Authored-By: Claude <noreply@anthropic.com>
@paulirwin paulirwin requested a review from Copilot November 6, 2025 18:01
@github-actions
Copy link

github-actions bot commented Nov 6, 2025

Test Results

133 tests  +31   133 ✅ +33   0s ⏱️ ±0s
  1 suites ± 0     0 💤  -  2 
  1 files   ± 0     0 ❌ ± 0 

Results for commit fb88366. ± Comparison against base commit b204dbd.

This pull request removes 34 and adds 65 tests. Note that renamed tests count towards both.
F23.Kernel.Tests.AspNetCore.ResultExtensionsMinimalApiTests ‑ AggregateResult_Failure_Returns_First_Failed_Result
F23.Kernel.Tests.AspNetCore.ResultExtensionsMinimalApiTests ‑ AggregateResult_Success_Returns_NoContentResult
F23.Kernel.Tests.AspNetCore.ResultExtensionsMinimalApiTests ‑ PreconditionFailedResultT_ConcurrencyMismatch_Returns_StatusCodeResult
F23.Kernel.Tests.AspNetCore.ResultExtensionsMinimalApiTests ‑ PreconditionFailedResultT_Conflict_Returns_ConflictResult
F23.Kernel.Tests.AspNetCore.ResultExtensionsMinimalApiTests ‑ PreconditionFailedResultT_NotFound_Returns_NotFoundResult
F23.Kernel.Tests.AspNetCore.ResultExtensionsMinimalApiTests ‑ PreconditionFailedResult_ConcurrencyMismatch_Returns_StatusCodeResult
F23.Kernel.Tests.AspNetCore.ResultExtensionsMinimalApiTests ‑ PreconditionFailedResult_Conflict_Returns_ConflictResult
F23.Kernel.Tests.AspNetCore.ResultExtensionsMinimalApiTests ‑ PreconditionFailedResult_NotFound_Returns_NotFoundResult
F23.Kernel.Tests.AspNetCore.ResultExtensionsMinimalApiTests ‑ SuccessResultT_Returns_OkObjectResult
F23.Kernel.Tests.AspNetCore.ResultExtensionsMinimalApiTests ‑ SuccessResultT_With_SuccessMap_Returns_SuccessMapResult
…
F23.Kernel.Tests.AspNetCore.MinimalApiResultExtensionsTests ‑ AggregateResult_Failure_Returns_First_Failed_Result
F23.Kernel.Tests.AspNetCore.MinimalApiResultExtensionsTests ‑ AggregateResult_Success_Returns_NoContentResult
F23.Kernel.Tests.AspNetCore.MinimalApiResultExtensionsTests ‑ PreconditionFailedResultT_ConcurrencyMismatch_Returns_StatusCodeResult
F23.Kernel.Tests.AspNetCore.MinimalApiResultExtensionsTests ‑ PreconditionFailedResultT_ConcurrencyMismatch_With_UseProblemDetails_Returns_ProblemDetails
F23.Kernel.Tests.AspNetCore.MinimalApiResultExtensionsTests ‑ PreconditionFailedResultT_Conflict_Returns_ConflictResult
F23.Kernel.Tests.AspNetCore.MinimalApiResultExtensionsTests ‑ PreconditionFailedResultT_Conflict_With_UseProblemDetails_Returns_ProblemDetails
F23.Kernel.Tests.AspNetCore.MinimalApiResultExtensionsTests ‑ PreconditionFailedResultT_NotFound_Returns_NotFoundResult
F23.Kernel.Tests.AspNetCore.MinimalApiResultExtensionsTests ‑ PreconditionFailedResultT_NotFound_With_UseProblemDetails_Returns_ProblemDetails
F23.Kernel.Tests.AspNetCore.MinimalApiResultExtensionsTests ‑ PreconditionFailedResult_ConcurrencyMismatch_Returns_StatusCodeResult
F23.Kernel.Tests.AspNetCore.MinimalApiResultExtensionsTests ‑ PreconditionFailedResult_ConcurrencyMismatch_With_UseProblemDetails_Returns_ProblemDetails
…

♻️ This comment has been updated with latest results.

@paulirwin paulirwin added the notes:breaking-change List under Breaking Changes for release notes label Nov 6, 2025
Copy link

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull Request Overview

This pull request refactors the result extension methods by splitting them into separate files for MVC and Minimal API approaches, and adds support for RFC 7807 Problem Details responses.

  • Splits ResultExtensions.cs into MvcResultExtensions.cs and MinimalApiResultExtensions.cs for better separation of concerns
  • Adds optional useProblemDetails parameter (defaulting to true) to enable RFC 7807 Problem Details format for error responses
  • Introduces ValidationErrorExtensions.CreateErrorDictionary method for converting validation errors to the dictionary format required by Minimal API's ValidationProblem
  • Adds example endpoints demonstrating both MVC and Minimal API approaches

Reviewed Changes

Copilot reviewed 11 out of 12 changed files in this pull request and generated 6 comments.

Show a summary per file
File Description
src/F23.Kernel/ValidationErrorExtensions.cs New extension method to convert validation errors to dictionary format for Minimal API ValidationProblem support
src/F23.Kernel.AspNetCore/ResultExtensions.cs Removed - split into separate MVC and Minimal API files
src/F23.Kernel.AspNetCore/MvcResultExtensions.cs New file containing MVC-specific result extensions with Problem Details support
src/F23.Kernel.AspNetCore/MinimalApiResultExtensions.cs New file containing Minimal API-specific result extensions with Problem Details support
src/F23.Kernel.Tests/AspNetCore/ResultExtensionsTests.cs Removed - split into separate test files
src/F23.Kernel.Tests/AspNetCore/MvcResultExtensionsTests.cs New comprehensive test file for MVC result extensions including Problem Details tests
src/F23.Kernel.Tests/AspNetCore/MinimalApiResultExtensionsTests.cs New test file for Minimal API result extensions with basic Problem Details coverage
src/F23.Kernel.Examples.AspNetCore/Program.cs Updated to register controllers and map result endpoints
src/F23.Kernel.Examples.AspNetCore/Infrastructure/ResultsController.cs New MVC controller demonstrating various result types
src/F23.Kernel.Examples.AspNetCore/Core/ResultsEndpoints.cs New Minimal API endpoints demonstrating various result types
.gitignore Added IDE-specific ignore patterns

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Improve CreateErrorDictionary to properly handle and accumulate multiple
errors with the same key into arrays. This is critical for RFC7807 problem
details format where validation fields may have multiple error messages.

Changes:
- Handle duplicate keys by appending messages to existing array
- Use case-sensitive comparison (StringComparer.Ordinal)
- Efficient array allocation strategy
- Add comprehensive unit tests for all scenarios

Tests cover:
- Single error per key
- Multiple errors with different keys
- Multiple errors with same key (accumulation)
- Mixed single and multiple errors
- Empty collections and null validation
- Case sensitivity verification
- Error order preservation
- Special character keys

All 133 tests pass.

🤖 Generated with [Claude Code](https://claude.com/claude-code)

Co-Authored-By: Claude <noreply@anthropic.com>
@paulirwin paulirwin requested a review from Copilot November 6, 2025 18:14
@github-actions
Copy link

github-actions bot commented Nov 6, 2025

Code Coverage

Package Line Rate Branch Rate Complexity Health
F23.Kernel 91% 83% 129
F23.Kernel.AspNetCore 100% 100% 155
Summary 95% (347 / 366) 96% (195 / 204) 284

Copy link

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull Request Overview

Copilot reviewed 12 out of 13 changed files in this pull request and generated no new comments.


💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

@paulirwin paulirwin merged commit 4265228 into main Nov 7, 2025
13 checks passed
@paulirwin paulirwin deleted the issue/8 branch November 7, 2025 15:52
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

notes:breaking-change List under Breaking Changes for release notes

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Pass through messages for empty failure results

3 participants