Fix: CreateIssueCommand drops CategoryId and StatusId selected in form#130
Fix: CreateIssueCommand drops CategoryId and StatusId selected in form#130
Conversation
- Add CategoryId and StatusId properties to CreateIssueCommand - Inject ICategoryRepository and IStatusRepository into CreateIssueHandler - Look up category/status by ID in handler; fall back to Empty if null/not found - Extract reusable LookupByIdAsync<T> helper to eliminate duplication - Update CreateIssuePage.razor to pass CategoryId and StatusId from form - Update unit and integration test constructors for new dependencies - Add two new unit tests for the category/status lookup behaviour Closes #118 Agent-Logs-Url: https://github.com/mpaulosky/IssueManager/sessions/c4f50e37-fcd4-4e02-a375-d0b15f289a72 Co-authored-by: mpaulosky <60372079+mpaulosky@users.noreply.github.com>
There was a problem hiding this comment.
Pull request overview
Fixes issue creation so the Category and Status selected in the Blazor form are not dropped, and ensures the API persists the resolved DTOs rather than hardcoded Empty values.
Changes:
- Extended
CreateIssueCommandwith optionalCategoryIdandStatusId. - Updated the Blazor create page to map selected IDs into the command.
- Updated
CreateIssueHandlerto resolveCategoryDto/StatusDtofrom repositories via a shared lookup helper, with safe fallbacks.
Reviewed changes
Copilot reviewed 5 out of 6 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| tests/Api.Tests.Unit/Handlers/Issues/CreateIssueHandlerTests.cs | Updates handler construction for new dependencies; adds unit tests for lookup-found and lookup-null paths. |
| tests/Api.Tests.Integration/Handlers/CreateIssueHandlerTests.cs | Updates handler construction to satisfy new dependencies in integration test setup. |
| src/Web/package-lock.json | Lockfile reformat (whitespace/indentation) with no functional dependency changes shown in the excerpt. |
| src/Web/Components/Features/Issues/CreateIssuePage.razor | Passes CategoryId/StatusId from the form request into CreateIssueCommand. |
| src/Shared/Contracts/CreateIssueValidator.cs | Adds CategoryId/StatusId fields to the shared command contract. |
| src/Api/Handlers/Issues/CreateIssueHandler.cs | Injects category/status repositories and resolves DTOs by ID during issue creation. |
| result.Value!.Category.CategoryName.Should().Be("Bug"); | ||
| result.Value!.Status.StatusName.Should().Be("Open"); | ||
| await _categoryRepository.Received(1).GetByIdAsync(categoryId, Arg.Any<CancellationToken>()); | ||
| await _statusRepository.Received(1).GetByIdAsync(statusId, Arg.Any<CancellationToken>()); |
There was a problem hiding this comment.
The assertions in this test can pass even if the handler doesn’t apply the looked-up category/status to the IssueDto sent to CreateAsync, because CreateAsync is stubbed to return an IssueDto that already contains expectedCategory/expectedStatus. To make the test actually validate behavior, assert that _repository.CreateAsync(...) was called with an IssueDto whose Category and Status match the looked-up DTOs (e.g., using Arg.Is<IssueDto>(...) or capturing the argument).
| await _statusRepository.Received(1).GetByIdAsync(statusId, Arg.Any<CancellationToken>()); | |
| await _statusRepository.Received(1).GetByIdAsync(statusId, Arg.Any<CancellationToken>()); | |
| await _repository.Received(1).CreateAsync( | |
| Arg.Is<IssueDto>(issue => | |
| issue.Category == expectedCategory && | |
| issue.Status == expectedStatus), | |
| Arg.Any<CancellationToken>()); |
| if (!string.IsNullOrEmpty(id) && ObjectId.TryParse(id, out var objectId)) | ||
| { | ||
| var result = await getById(objectId, cancellationToken); | ||
| if (result.Success && result.Value is not null) | ||
| return result.Value; | ||
| } | ||
| return defaultValue; |
There was a problem hiding this comment.
LookupByIdAsync has important branches for (a) non-empty but unparseable IDs and (b) parseable IDs where the repository returns NotFound/Fail. There’s currently coverage for the “present and found” and “null IDs” paths, but not for these new fallback branches. Consider adding unit tests to ensure invalid/not-found IDs don’t call CreateAsync with unintended values and correctly fall back to CategoryDto.Empty/StatusDto.Empty.
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #130 +/- ##
==========================================
+ Coverage 55.52% 55.74% +0.22%
==========================================
Files 124 124
Lines 2826 2845 +19
Branches 313 315 +2
==========================================
+ Hits 1569 1586 +17
- Misses 1027 1029 +2
Partials 230 230
🚀 New features to boost your workflow:
|
|
✅ Ralph (Work Monitor): All test suites pass (unit, integration, bunit, architecture). The E2E failure is a pre-existing CI infrastructure issue (DB init failure, Redis SSL errors) unrelated to this PR's changes. Merging. |
HandleSubmitinCreateIssuePage.razorbuiltCreateIssueCommandwith onlyTitleandDescription, silently discarding the category and status the user selected. The handler then hardcodedCategoryDto.EmptyandStatusDto.Emptyon the created issue.Changes
CreateIssueCommand— AddedCategoryIdandStatusId(string?) properties.CreateIssuePage.razor—HandleSubmitnow mapsrequest.CategoryIdandrequest.StatusIdinto the command.CreateIssueHandler— InjectsICategoryRepositoryandIStatusRepository; resolves the full DTO by ID at creation time, falling back toEmptyif the ID is absent, unparseable, or not found. Lookup pattern extracted into a genericLookupByIdAsync<T>helper to avoid duplication.