-
Notifications
You must be signed in to change notification settings - Fork 0
feat: Propagate Result<T> from handlers to endpoints (#81, #83, #85, #87) #92
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
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,121 @@ | ||
| # Result<T> Handler Propagation Complete | ||
|
|
||
| **Date**: 2025-01-02 | ||
| **Author**: Sam (Backend Developer) | ||
| **Issues**: #81, #83, #85, #87 | ||
| **Branch**: `squad/81-result-t-handlers` | ||
| **Commit**: `9885078` | ||
|
|
||
| ## Decision | ||
|
|
||
| All API handlers have been updated to propagate `Result<T>` from repositories to endpoints, completing the Result pattern implementation across all four domains (Issues, Categories, Statuses, Comments). | ||
|
|
||
| ## Implementation Details | ||
|
|
||
| ### Handler Changes | ||
|
|
||
| **Return Type Changes**: | ||
| - Get handlers: `Task<TDto?>` → `Task<Result<TDto>>` | ||
| - Update handlers: `Task<TDto>` → `Task<Result<TDto>>` | ||
| - Delete handlers: `Task<bool>` → `Task<Result<bool>>` | ||
|
|
||
| **Error Handling Pattern**: | ||
| ```csharp | ||
| // Validation errors | ||
| if (!validationResult.IsValid) | ||
| return Result.Fail<TDto>("Validation failed", ResultErrorCode.Validation); | ||
|
|
||
| // Not found errors | ||
| if (getResult.Failure || getResult.Value is null) | ||
| return Result.Fail<TDto>($"Entity with ID '{id}' was not found.", ResultErrorCode.NotFound); | ||
|
|
||
| // Conflict errors (archived entities) | ||
| if (entity.Archived) | ||
| return Result.Fail<TDto>($"Entity is archived.", ResultErrorCode.Conflict); | ||
|
|
||
| // Propagate repository results | ||
| return await _repository.UpdateAsync(entity, cancellationToken); | ||
| ``` | ||
|
|
||
| **Delete Handler Pattern** (special case): | ||
| ```csharp | ||
| // Repository.ArchiveAsync returns Result, not Result<bool> | ||
| var archiveResult = await _repository.ArchiveAsync(id, cancellationToken); | ||
| return archiveResult.Success | ||
| ? Result.Ok(true) | ||
| : Result.Fail<bool>(archiveResult.Error!, archiveResult.ErrorCode); | ||
| ``` | ||
|
|
||
| ### Endpoint Changes | ||
|
|
||
| **HTTP Response Mapping**: | ||
| ```csharp | ||
| // Get endpoints | ||
| var result = await handler.Handle(query); | ||
| return result.Success ? Results.Ok(result.Value) : Results.NotFound(); | ||
|
|
||
| // Update endpoints | ||
| var result = await handler.Handle(command); | ||
| if (!result.Success) | ||
| return result.ErrorCode == ResultErrorCode.NotFound ? Results.NotFound() | ||
| : result.ErrorCode == ResultErrorCode.Conflict ? Results.Conflict(result.Error) | ||
| : Results.BadRequest(result.Error); | ||
| return Results.Ok(result.Value); | ||
|
|
||
| // Delete endpoints | ||
| var result = await handler.Handle(command); | ||
| return result.Success ? Results.NoContent() : Results.NotFound(); | ||
| ``` | ||
|
|
||
| ## Files Changed | ||
|
|
||
| ### Issues (#81) | ||
| - `src/Api/Handlers/Issues/GetIssueHandler.cs` | ||
| - `src/Api/Handlers/Issues/DeleteIssueHandler.cs` | ||
| - `src/Api/Handlers/Issues/UpdateIssueHandler.cs` | ||
| - `src/Api/Handlers/Issues/UpdateIssueStatusHandler.cs` | ||
| - `src/Api/Handlers/Issues/IssueEndpoints.cs` | ||
|
|
||
| ### Categories (#83) | ||
| - `src/Api/Handlers/Categories/GetCategoryHandler.cs` | ||
| - `src/Api/Handlers/Categories/DeleteCategoryHandler.cs` | ||
| - `src/Api/Handlers/Categories/UpdateCategoryHandler.cs` | ||
| - `src/Api/Handlers/Categories/CategoryEndpoints.cs` | ||
|
|
||
| ### Statuses (#85) | ||
| - `src/Api/Handlers/Statuses/GetStatusHandler.cs` | ||
| - `src/Api/Handlers/Statuses/DeleteStatusHandler.cs` | ||
| - `src/Api/Handlers/Statuses/UpdateStatusHandler.cs` | ||
| - `src/Api/Handlers/Statuses/StatusEndpoints.cs` | ||
|
|
||
| ### Comments (#87) | ||
| - `src/Api/Handlers/Comments/GetCommentHandler.cs` | ||
| - `src/Api/Handlers/Comments/DeleteCommentHandler.cs` | ||
| - `src/Api/Handlers/Comments/UpdateCommentHandler.cs` | ||
| - `src/Api/Handlers/Comments/CommentEndpoints.cs` | ||
|
|
||
| ## Testing Status | ||
|
|
||
| - ✅ **src/ compiles successfully** — No compilation errors in production code | ||
| - ❌ **tests/ have compilation errors** — Expected, waiting for Gimli to update test assertions | ||
|
|
||
| ## Blocked Work | ||
|
|
||
| **Gimli** must update all handler tests to: | ||
| 1. Expect `Result<T>` return types instead of direct DTOs | ||
| 2. Assert on `result.Success` and `result.Failure` | ||
| 3. Access values via `result.Value` | ||
| 4. Check error codes via `result.ErrorCode` | ||
| 5. Update FluentAssertions patterns (e.g., `result.Success.Should().BeTrue()`) | ||
|
|
||
| ## PR Status | ||
|
|
||
| ⚠️ **DO NOT MERGE** until test suite is updated by Gimli. | ||
|
|
||
| Branch: `squad/81-result-t-handlers` | ||
| PR: https://github.com/mpaulosky/IssueManager/pull/new/squad/81-result-t-handlers | ||
|
|
||
| ## Related Decisions | ||
|
|
||
| - `.squad/decisions/inbox/aragorn-objectid-result-pattern.md` — Original Result<T> pattern decision | ||
| - `.squad/decisions/inbox/sam-objectid-endpoint-parsing.md` — ObjectId parsing at endpoints (sprint #80) |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,2 @@ | ||
| <wpf:ResourceDictionary xml:space="preserve" xmlns:x="http://schemas.microsoft.com/winfx/2006/xaml" xmlns:s="clr-namespace:System;assembly=mscorlib" xmlns:ss="urn:shemas-jetbrains-com:settings-storage-xaml" xmlns:wpf="http://schemas.microsoft.com/winfx/2006/xaml/presentation"> | ||
| <s:Boolean x:Key="/Default/UserDictionary/Words/=contentinfo/@EntryIndexedValue">True</s:Boolean></wpf:ResourceDictionary> | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -25,8 +25,8 @@ public static IEndpointRouteBuilder MapCategoryEndpoints(this IEndpointRouteBuil | |
| if (!ObjectId.TryParse(id, out var objectId)) | ||
| return Results.BadRequest("Invalid ID format"); | ||
| var query = new GetCategoryQuery(objectId); | ||
| var category = await handler.Handle(query); | ||
| return category is not null ? Results.Ok(category) : Results.NotFound(); | ||
| var result = await handler.Handle(query); | ||
| return result.Success ? Results.Ok(result.Value) : Results.NotFound(); | ||
| }) | ||
| .WithName("GetCategory") | ||
| .WithSummary("Get a category by ID") | ||
|
|
@@ -50,7 +50,9 @@ public static IEndpointRouteBuilder MapCategoryEndpoints(this IEndpointRouteBuil | |
| return Results.BadRequest("Invalid ID format"); | ||
| var commandWithId = command with { Id = objectId }; | ||
| var result = await handler.Handle(commandWithId); | ||
| return result is not null ? Results.Ok(result) : Results.NotFound(); | ||
| if (!result.Success) | ||
| return result.ErrorCode == ResultErrorCode.NotFound ? Results.NotFound() : Results.BadRequest(result.Error); | ||
| return Results.Ok(result.Value); | ||
| }) | ||
| .WithName("UpdateCategory") | ||
| .WithSummary("Update an existing category") | ||
|
|
@@ -65,7 +67,7 @@ public static IEndpointRouteBuilder MapCategoryEndpoints(this IEndpointRouteBuil | |
| return Results.BadRequest("Invalid ID format"); | ||
| var command = new DeleteCategoryCommand { Id = objectId }; | ||
| var result = await handler.Handle(command); | ||
| return result ? Results.NoContent() : Results.NotFound(); | ||
| return result.Success ? Results.NoContent() : Results.NotFound(); | ||
|
Comment on lines
68
to
+70
|
||
| }) | ||
| .WithName("DeleteCategory") | ||
| .WithSummary("Delete (archive) a category") | ||
|
|
||
| Original file line number | Diff line number | Diff line change | ||||
|---|---|---|---|---|---|---|
|
|
@@ -40,23 +40,23 @@ public DeleteCategoryHandler(ICategoryRepository repository, DeleteCategoryValid | |||||
| /// </summary> | ||||||
| /// <param name="command">The command containing the category ID to delete.</param> | ||||||
| /// <param name="cancellationToken">A token to cancel the asynchronous operation.</param> | ||||||
| /// <returns><see langword="true"/> if the category was successfully archived; otherwise, <see langword="false"/>.</returns> | ||||||
| /// <returns>A task that represents the asynchronous operation. The task result contains a result indicating success or failure.</returns> | ||||||
| /// <exception cref="ValidationException">Thrown when the command fails validation.</exception> | ||||||
| /// <exception cref="NotFoundException">Thrown when the category is not found.</exception> | ||||||
|
Comment on lines
44
to
45
|
||||||
| /// <exception cref="ValidationException">Thrown when the command fails validation.</exception> | |
| /// <exception cref="NotFoundException">Thrown when the category is not found.</exception> |
| Original file line number | Diff line number | Diff line change | ||
|---|---|---|---|---|
|
|
@@ -39,11 +39,10 @@ public GetCategoryHandler(ICategoryRepository repository) | |||
| /// </summary> | ||||
| /// <param name="query">The query containing the category ID to retrieve.</param> | ||||
| /// <param name="cancellationToken">A token to cancel the asynchronous operation.</param> | ||||
| /// <returns>A task that represents the asynchronous operation. The task result contains the category as a <see cref="CategoryDto"/>, or <see langword="null"/> if not found.</returns> | ||||
| /// <returns>A task that represents the asynchronous operation. The task result contains the result with category as a <see cref="CategoryDto"/>, or an error if not found.</returns> | ||||
| /// <exception cref="ArgumentException">Thrown when the category ID is null or empty.</exception> | ||||
|
||||
| /// <exception cref="ArgumentException">Thrown when the category ID is null or empty.</exception> |
| Original file line number | Diff line number | Diff line change | ||||
|---|---|---|---|---|---|---|
|
|
@@ -40,29 +40,25 @@ public UpdateCategoryHandler(ICategoryRepository repository, UpdateCategoryValid | |||||
| /// </summary> | ||||||
| /// <param name="command">The command containing the updated category information.</param> | ||||||
| /// <param name="cancellationToken">A token to cancel the asynchronous operation.</param> | ||||||
| /// <returns>A task that represents the asynchronous operation. The task result contains the updated category as a <see cref="CategoryDto"/>.</returns> | ||||||
| /// <returns>A task that represents the asynchronous operation. The task result contains the result with updated category as a <see cref="CategoryDto"/>.</returns> | ||||||
| /// <exception cref="ValidationException">Thrown when the command fails validation.</exception> | ||||||
| /// <exception cref="NotFoundException">Thrown when the category is not found or cannot be updated.</exception> | ||||||
|
Comment on lines
44
to
45
|
||||||
| /// <exception cref="ValidationException">Thrown when the command fails validation.</exception> | |
| /// <exception cref="NotFoundException">Thrown when the category is not found or cannot be updated.</exception> |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -25,8 +25,8 @@ public static IEndpointRouteBuilder MapCommentEndpoints(this IEndpointRouteBuild | |
| if (!ObjectId.TryParse(id, out var objectId)) | ||
| return Results.BadRequest("Invalid ID format"); | ||
| var query = new GetCommentQuery(objectId); | ||
| var comment = await handler.Handle(query); | ||
| return comment is not null ? Results.Ok(comment) : Results.NotFound(); | ||
| var result = await handler.Handle(query); | ||
| return result.Success ? Results.Ok(result.Value) : Results.NotFound(); | ||
| }) | ||
| .WithName("GetComment") | ||
| .WithSummary("Get a comment by ID") | ||
|
|
@@ -50,7 +50,9 @@ public static IEndpointRouteBuilder MapCommentEndpoints(this IEndpointRouteBuild | |
| return Results.BadRequest("Invalid ID format"); | ||
| var commandWithId = command with { Id = objectId }; | ||
| var result = await handler.Handle(commandWithId); | ||
| return result is not null ? Results.Ok(result) : Results.NotFound(); | ||
| if (!result.Success) | ||
| return result.ErrorCode == ResultErrorCode.NotFound ? Results.NotFound() : Results.BadRequest(result.Error); | ||
| return Results.Ok(result.Value); | ||
| }) | ||
| .WithName("UpdateComment") | ||
| .WithSummary("Update an existing comment") | ||
|
|
@@ -65,7 +67,7 @@ public static IEndpointRouteBuilder MapCommentEndpoints(this IEndpointRouteBuild | |
| return Results.BadRequest("Invalid ID format"); | ||
| var command = new DeleteCommentCommand { Id = objectId }; | ||
| var result = await handler.Handle(command); | ||
| return result ? Results.NoContent() : Results.NotFound(); | ||
| return result.Success ? Results.NoContent() : Results.NotFound(); | ||
|
Comment on lines
68
to
+70
|
||
| }) | ||
| .WithName("DeleteComment") | ||
| .WithSummary("Delete (archive) a comment") | ||
|
|
||
| Original file line number | Diff line number | Diff line change | ||||
|---|---|---|---|---|---|---|
|
|
@@ -40,23 +40,23 @@ public DeleteCommentHandler(ICommentRepository repository, DeleteCommentValidato | |||||
| /// </summary> | ||||||
| /// <param name="command">The command containing the comment ID to delete.</param> | ||||||
| /// <param name="cancellationToken">A token to cancel the asynchronous operation.</param> | ||||||
| /// <returns><see langword="true"/> if the comment was successfully archived; otherwise, <see langword="false"/>.</returns> | ||||||
| /// <returns>A task that represents the asynchronous operation. The task result contains a result indicating success or failure.</returns> | ||||||
| /// <exception cref="ValidationException">Thrown when the command fails validation.</exception> | ||||||
| /// <exception cref="NotFoundException">Thrown when the comment is not found.</exception> | ||||||
|
Comment on lines
44
to
45
|
||||||
| /// <exception cref="ValidationException">Thrown when the command fails validation.</exception> | |
| /// <exception cref="NotFoundException">Thrown when the comment is not found.</exception> |
| Original file line number | Diff line number | Diff line change | ||
|---|---|---|---|---|
|
|
@@ -39,11 +39,10 @@ public GetCommentHandler(ICommentRepository repository) | |||
| /// </summary> | ||||
| /// <param name="query">The query containing the comment ID to retrieve.</param> | ||||
| /// <param name="cancellationToken">A token to cancel the asynchronous operation.</param> | ||||
| /// <returns>A task that represents the asynchronous operation. The task result contains the comment as a <see cref="CommentDto"/>, or <see langword="null"/> if not found.</returns> | ||||
| /// <returns>A task that represents the asynchronous operation. The task result contains the result with comment as a <see cref="CommentDto"/>, or an error if not found.</returns> | ||||
| /// <exception cref="ArgumentException">Thrown when the comment ID is null or empty.</exception> | ||||
|
||||
| /// <exception cref="ArgumentException">Thrown when the comment ID is null or empty.</exception> |
| Original file line number | Diff line number | Diff line change | ||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
|
|
@@ -40,28 +40,24 @@ public UpdateCommentHandler(ICommentRepository repository, UpdateCommentValidato | |||||||||||||
| /// </summary> | ||||||||||||||
| /// <param name="command">The command containing the updated comment information.</param> | ||||||||||||||
| /// <param name="cancellationToken">A token to cancel the asynchronous operation.</param> | ||||||||||||||
| /// <returns>A task that represents the asynchronous operation. The task result contains the updated comment as a <see cref="CommentDto"/>.</returns> | ||||||||||||||
| /// <returns>A task that represents the asynchronous operation. The task result contains the result with updated comment as a <see cref="CommentDto"/>.</returns> | ||||||||||||||
| /// <exception cref="ValidationException">Thrown when the command fails validation.</exception> | ||||||||||||||
| /// <exception cref="NotFoundException">Thrown when the comment is not found or cannot be updated.</exception> | ||||||||||||||
|
Comment on lines
44
to
45
|
||||||||||||||
| /// <exception cref="ValidationException">Thrown when the command fails validation.</exception> | |
| /// <exception cref="NotFoundException">Thrown when the comment is not found or cannot be updated.</exception> |
Copilot
AI
Mar 4, 2026
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.
Returning _repository.UpdateAsync(...) directly can yield ErrorCode=None on not-found/update-failed (CommentRepository.UpdateAsync doesn’t set ResultErrorCode.NotFound). That will make CommentEndpoints return 400 instead of 404 for missing comments. Consider translating failed updates into a NotFound-coded Result here (or update the repository to set codes).
| return await _repository.UpdateAsync(updatedComment, cancellationToken); | |
| var updateResult = await _repository.UpdateAsync(updatedComment, cancellationToken); | |
| if (updateResult.Failure || updateResult.Value is null) | |
| return Result.Fail<CommentDto>($"Comment with ID '{command.Id}' was not found or could not be updated.", ResultErrorCode.NotFound); | |
| return updateResult; |
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.
The JetBrains settings XML namespace appears misspelled ("shemas" vs "schemas"), which may prevent Rider/ReSharper from loading this .DotSettings file. Fix the namespace value to the correct "urn:schemas-jetbrains-com:settings-storage-xaml".