From 7bdcc61651c8977ac3e4c26ee93aa2ed0615367f Mon Sep 17 00:00:00 2001 From: Alexandre Brandizzi Date: Wed, 16 Sep 2026 20:15:30 -0300 Subject: [PATCH] fix(work-orders): bind GetWorkorderById id from the query string (SH-374) --- .../WorkOrderGetByIdBindingTests.cs | 106 ++++++++++++++++++ .../WorkOrderRouteContractTests.cs | 6 +- .../Controllers/WorkOrderController.cs | 10 +- 3 files changed, 117 insertions(+), 5 deletions(-) create mode 100644 Api.SeaHavenIndustries.Tests/WorkOrderGetByIdBindingTests.cs diff --git a/Api.SeaHavenIndustries.Tests/WorkOrderGetByIdBindingTests.cs b/Api.SeaHavenIndustries.Tests/WorkOrderGetByIdBindingTests.cs new file mode 100644 index 0000000..2d88138 --- /dev/null +++ b/Api.SeaHavenIndustries.Tests/WorkOrderGetByIdBindingTests.cs @@ -0,0 +1,106 @@ +using Api.SeaHavenIndustries.Controllers; +using Data.SeaHavenIndustries; +using FluentAssertions; +using Microsoft.AspNetCore.Http; +using Microsoft.AspNetCore.Mvc; +using Microsoft.AspNetCore.Mvc.Controllers; +using Microsoft.AspNetCore.Mvc.Infrastructure; +using Microsoft.AspNetCore.Mvc.ModelBinding; +using Microsoft.Extensions.DependencyInjection; +using Microsoft.Extensions.Logging; +using Moq; +using SeaHaven.DataServices.Models; +using SeaHaven.Services.Interfaces; +using Xunit; + +namespace Api.SeaHavenIndustries.Tests; + +/// +/// SH-374: GET api/WorkOrder/GetWorkorderById?id= must read the id from the query string, while +/// GET api/WorkOrder/{id} keeps reading it from the route. Both must return the same work order, +/// and an unknown id keeps the existing "ID not found!" response. +/// +public class WorkOrderGetByIdBindingTests +{ + private const int ExistingId = 374; + private const int UnknownId = 999_999; + + private static ControllerActionDescriptor ActionForTemplate(string relativeTemplate) + { + var services = new ServiceCollection(); + services.AddLogging(); + services.AddMvcCore().AddApplicationPart(typeof(WorkOrderController).Assembly); + + using var provider = services.BuildServiceProvider(); + return provider.GetRequiredService().ActionDescriptors.Items + .OfType() + .Where(cad => cad.ControllerTypeInfo == typeof(WorkOrderController)) + .Where(cad => cad.ActionConstraints! + .OfType() + .Any(c => c.HttpMethods.Contains("GET"))) + .Single(cad => string.Equals( + cad.AttributeRouteInfo?.Template?.Trim('/'), + $"api/WorkOrder/{relativeTemplate}", + StringComparison.Ordinal)); + } + + [Theory] + [InlineData("GetWorkorderById", "Query")] + [InlineData("{id:int}", "Path")] + public void Id_binds_from_the_source_its_route_provides(string relativeTemplate, string expectedSource) + { + var action = ActionForTemplate(relativeTemplate); + + var id = action.Parameters.Single(p => p.Name == "id"); + id.BindingInfo.Should().NotBeNull(); + id.BindingInfo!.BindingSource!.Id.Should().Be(expectedSource); + } + + private static (WorkOrderController Controller, WorkOrderDetailReadModel Existing) NewController() + { + var existing = new WorkOrderDetailReadModel { Id = ExistingId, Title = "Leaking roof" }; + var service = new Mock(); + service.Setup(s => s.GetWorkOrderDetailAsync(ExistingId, It.IsAny())).ReturnsAsync(existing); + service.Setup(s => s.GetWorkOrderDetailAsync(UnknownId, It.IsAny())) + .ReturnsAsync((WorkOrderDetailReadModel?)null); + + var controller = new WorkOrderController( + service.Object, + Mock.Of(), + Mock.Of>()) + { + ControllerContext = new ControllerContext { HttpContext = new DefaultHttpContext() } + }; + return (controller, existing); + } + + [Fact] + public async Task Existing_id_returns_the_same_work_order_through_both_routes() + { + var (controller, existing) = NewController(); + + var byQuery = await controller.GetWorkorderById(ExistingId); + var byRoute = await controller.GetWorkorderByRouteId(ExistingId); + + byQuery.Should().BeOfType().Which.Value.Should().BeSameAs(existing); + byRoute.Should().BeOfType().Which.Value.Should().BeSameAs(existing); + } + + [Fact] + public async Task Unknown_id_keeps_the_not_found_response_through_both_routes() + { + var (controller, _) = NewController(); + + foreach (var result in new[] + { + await controller.GetWorkorderById(UnknownId), + await controller.GetWorkorderByRouteId(UnknownId), + }) + { + var body = result.Should().BeOfType().Which.Value + .Should().BeOfType().Subject; + body.Status.Should().Be("Error"); + body.Message.Should().Be("ID not found!"); + } + } +} diff --git a/Api.SeaHavenIndustries.Tests/WorkOrderRouteContractTests.cs b/Api.SeaHavenIndustries.Tests/WorkOrderRouteContractTests.cs index 6411337..1bb17e5 100644 --- a/Api.SeaHavenIndustries.Tests/WorkOrderRouteContractTests.cs +++ b/Api.SeaHavenIndustries.Tests/WorkOrderRouteContractTests.cs @@ -17,8 +17,8 @@ namespace Api.SeaHavenIndustries.Tests; /// combination is registered more than once by different actions. /// /// Routes are read from the ASP.NET Core action descriptor provider so the EXACT templates the -/// framework will dispatch are compared, including multi-attribute actions (e.g. Editworkorder, -/// GetWorkorderById) and the two shared controller-level base routes. +/// framework will dispatch are compared, including multi-attribute actions (e.g. Editworkorder) +/// and the two shared controller-level base routes. /// public class WorkOrderRouteContractTests { @@ -39,7 +39,7 @@ public class WorkOrderRouteContractTests /// WorkOrderController exposed, plus the author-only board-comment edit endpoint (SH-122). /// Every action is reachable under both api/WorkOrder and api/workorders; that base-route /// duplication is collapsed here, so this is the distinct action-relative contract. 52 routes - /// come from 50 actions (Editworkorder and GetWorkorderById each bind two routes). + /// come from 51 actions (Editworkorder binds two routes). /// private static readonly HashSet ExpectedWorkOrderEndpoints = new(StringComparer.Ordinal) { diff --git a/Api.SeaHavenIndustries/Controllers/WorkOrderController.cs b/Api.SeaHavenIndustries/Controllers/WorkOrderController.cs index c7928de..9c73034 100644 --- a/Api.SeaHavenIndustries/Controllers/WorkOrderController.cs +++ b/Api.SeaHavenIndustries/Controllers/WorkOrderController.cs @@ -208,9 +208,15 @@ namespace Api.SeaHavenIndustries.Controllers } } - [HttpGet("{id:int}")] + // SH-374: two actions, one per route, so each binds id from the source its route provides. + // A single action carrying both routes inferred id as [FromRoute] and the query route got 0. [HttpGet("GetWorkorderById")] - public async Task GetWorkorderById(int id) + public Task GetWorkorderById([FromQuery] int id) => GetWorkorderDetail(id); + + [HttpGet("{id:int}")] + public Task GetWorkorderByRouteId([FromRoute] int id) => GetWorkorderDetail(id); + + private async Task GetWorkorderDetail(int id) { try {