Merge branch 'dev' into feat/ab/sh-328-permission-overrides

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;
+
+/// <summary>
+/// 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.
+/// </summary>
+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<IActionDescriptorCollectionProvider>().ActionDescriptors.Items
+            .OfType<ControllerActionDescriptor>()
+            .Where(cad => cad.ControllerTypeInfo == typeof(WorkOrderController))
+            .Where(cad => cad.ActionConstraints!
+                .OfType<Microsoft.AspNetCore.Mvc.ActionConstraints.HttpMethodActionConstraint>()
+                .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<IWorkOrderService>();
+        service.Setup(s => s.GetWorkOrderDetailAsync(ExistingId, It.IsAny<int?>())).ReturnsAsync(existing);
+        service.Setup(s => s.GetWorkOrderDetailAsync(UnknownId, It.IsAny<int?>()))
+            .ReturnsAsync((WorkOrderDetailReadModel?)null);
+
+        var controller = new WorkOrderController(
+            service.Object,
+            Mock.Of<IWorkOrderAccountResolver>(),
+            Mock.Of<ILogger<WorkOrderController>>())
+        {
+            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<OkObjectResult>().Which.Value.Should().BeSameAs(existing);
+        byRoute.Should().BeOfType<OkObjectResult>().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<BadRequestObjectResult>().Which.Value
+                .Should().BeOfType<Response>().Subject;
+            body.Status.Should().Be("Error");
+            body.Message.Should().Be("ID not found!");
+        }
+    }
+}

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.
 /// </summary>
 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).
     /// </summary>
     private static readonly HashSet<string> ExpectedWorkOrderEndpoints = new(StringComparer.Ordinal)
     {

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<IActionResult> GetWorkorderById(int id)
+        public Task<IActionResult> GetWorkorderById([FromQuery] int id) => GetWorkorderDetail(id);
+
+        [HttpGet("{id:int}")]
+        public Task<IActionResult> GetWorkorderByRouteId([FromRoute] int id) => GetWorkorderDetail(id);
+
+        private async Task<IActionResult> GetWorkorderDetail(int id)
         {
             try
             {