From 7a0856ddf7bfe1bd5c7754e9638527e5238cab4b Mon Sep 17 00:00:00 2001 From: Alexandre Brandizzi Date: Wed, 19 Aug 2026 13:31:16 -0300 Subject: [PATCH] feat(vendors): confirm-to-deactivate with open work orders (SH-254) SH-44 and SH-82 both left "blocks, or requires explicit confirmation" to be decided with the team, and the implementation took the blocking branch. SH-254 settles it the other way: the approved design offers "Deactivate anyway" beside the list of open work orders. Deactivation with open work orders is now permitted, but only when the caller says it has shown them: ConfirmOpenWorkOrders on the update DTO and a confirmOpenWorkOrders query parameter on the delete route. Absent the flag the existing guard still throws, so nothing deactivates by accident and no caller loses the check by omission. confirmOpenWorkOrders is a required parameter on DeleteVendorAsync rather than an optional one, so every call site states its intent. --- .../VendorControllerTests.cs | 6 +- .../VendorServiceTests.cs | 80 ++++++++++++++++++- .../Controllers/VendorController.cs | 10 ++- Api.SeaHavenIndustries/DTOs/Vendor_DTO.cs | 4 + SeaHaven.Services/DTOs/VendorDTOs.cs | 5 ++ .../Implementation/VendorService.cs | 10 ++- .../Interfaces/IVendorService.cs | 2 +- 7 files changed, 106 insertions(+), 11 deletions(-) diff --git a/Api.SeaHavenIndustries.Tests/VendorControllerTests.cs b/Api.SeaHavenIndustries.Tests/VendorControllerTests.cs index 24477c4..e0df2a9 100644 --- a/Api.SeaHavenIndustries.Tests/VendorControllerTests.cs +++ b/Api.SeaHavenIndustries.Tests/VendorControllerTests.cs @@ -256,7 +256,7 @@ public class VendorControllerTests { var openWorkOrders = new List { new() { WorkOrderId = 7 } }; var service = new Mock(); - service.Setup(x => x.DeleteVendorAsync(It.IsAny(), It.IsAny())) + service.Setup(x => x.DeleteVendorAsync(It.IsAny(), It.IsAny(), It.IsAny())) .ThrowsAsync(new VendorDeactivationBlockedException("SECRET-internal-reason", openWorkOrders)); var logger = new Mock>(); @@ -293,7 +293,7 @@ public class VendorControllerTests public async Task Delete_MapsDeactivationBlocked_ToConflict() { var service = new Mock(); - service.Setup(x => x.DeleteVendorAsync(It.IsAny(), It.IsAny())) + service.Setup(x => x.DeleteVendorAsync(It.IsAny(), It.IsAny(), It.IsAny())) .ThrowsAsync(new VendorDeactivationBlockedException("blocked", new List())); var controller = NewController(service); @@ -307,7 +307,7 @@ public class VendorControllerTests public async Task Delete_MapsNotFoundInvalidOperation_ToNotFound() { var service = new Mock(); - service.Setup(x => x.DeleteVendorAsync(It.IsAny(), It.IsAny())) + service.Setup(x => x.DeleteVendorAsync(It.IsAny(), It.IsAny(), It.IsAny())) .ThrowsAsync(new InvalidOperationException()); var controller = NewController(service); diff --git a/Api.SeaHavenIndustries.Tests/VendorServiceTests.cs b/Api.SeaHavenIndustries.Tests/VendorServiceTests.cs index 1816257..24c4ea2 100644 --- a/Api.SeaHavenIndustries.Tests/VendorServiceTests.cs +++ b/Api.SeaHavenIndustries.Tests/VendorServiceTests.cs @@ -99,7 +99,7 @@ public class VendorServiceTests data.Setup(x => x.GetLinkedWorkOrdersAsync(9)).ReturnsAsync(new List()); data.Setup(x => x.UpdateAsync(existing)).Returns(Task.CompletedTask); - await NewService(data).DeleteVendorAsync(9, "42"); + await NewService(data).DeleteVendorAsync(9, "42", confirmOpenWorkOrders: false); existing.IsActive.Should().BeFalse(); existing.IsDeleted.Should().NotBeTrue(); @@ -289,6 +289,84 @@ public class VendorServiceTests badUrl.Errors.Should().Contain(error => error.PropertyName == nameof(CreateVendorDTO.GoogleMapsUrl)); } + [Fact] + public async Task UpdateVendor_DeactivatingWithOpenWorkOrders_WhenConfirmed_Deactivates() + { + var existing = new Vendor { Id = 21, CompanyName = "Confirmed", IsActive = true }; + var data = new Mock(); + data.Setup(x => x.GetByIdAsync(21)).ReturnsAsync(existing); + data.Setup(x => x.GetLinkedWorkOrdersAsync(21)) + .ReturnsAsync(new List + { + new() + { + WorkOrderId = 501, + WorkOrderNumber = "WO-501", + Status = "Scheduled", + LifecycleStatus = LifecycleStatus.Scheduled + } + }); + data.Setup(x => x.UpdateAsync(existing)).Returns(Task.CompletedTask); + + await NewService(data).UpdateVendorAsync( + 21, + new UpdateVendorDTO { IsActive = false, ConfirmOpenWorkOrders = true }, + "42"); + + existing.IsActive.Should().BeFalse(); + data.Verify(x => x.UpdateAsync(existing), Times.Once); + } + + [Fact] + public async Task DeleteVendor_WithOpenWorkOrders_WhenNotConfirmed_ThrowsAndLeavesActive() + { + var existing = new Vendor { Id = 22, CompanyName = "Guarded Delete", IsActive = true }; + var data = new Mock(); + data.Setup(x => x.GetByIdAsync(22)).ReturnsAsync(existing); + data.Setup(x => x.GetLinkedWorkOrdersAsync(22)) + .ReturnsAsync(new List + { + new() + { + WorkOrderId = 502, + WorkOrderNumber = "WO-502", + Status = "Scheduled", + LifecycleStatus = LifecycleStatus.Scheduled + } + }); + + var act = () => NewService(data).DeleteVendorAsync(22, "42", confirmOpenWorkOrders: false); + + await act.Should().ThrowAsync(); + existing.IsActive.Should().BeTrue(); + data.Verify(x => x.UpdateAsync(It.IsAny()), Times.Never); + } + + [Fact] + public async Task DeleteVendor_WithOpenWorkOrders_WhenConfirmed_Deactivates() + { + var existing = new Vendor { Id = 23, CompanyName = "Confirmed Delete", IsActive = true }; + var data = new Mock(); + data.Setup(x => x.GetByIdAsync(23)).ReturnsAsync(existing); + data.Setup(x => x.GetLinkedWorkOrdersAsync(23)) + .ReturnsAsync(new List + { + new() + { + WorkOrderId = 503, + WorkOrderNumber = "WO-503", + Status = "Scheduled", + LifecycleStatus = LifecycleStatus.Scheduled + } + }); + data.Setup(x => x.UpdateAsync(existing)).Returns(Task.CompletedTask); + + await NewService(data).DeleteVendorAsync(23, "42", confirmOpenWorkOrders: true); + + existing.IsActive.Should().BeFalse(); + data.Verify(x => x.UpdateAsync(existing), Times.Once); + } + [Fact] public async Task UpdateVendor_DeactivatingWithOpenWorkOrders_ThrowsAndLeavesActive() { diff --git a/Api.SeaHavenIndustries/Controllers/VendorController.cs b/Api.SeaHavenIndustries/Controllers/VendorController.cs index ea76250..1051fcf 100644 --- a/Api.SeaHavenIndustries/Controllers/VendorController.cs +++ b/Api.SeaHavenIndustries/Controllers/VendorController.cs @@ -185,7 +185,8 @@ namespace Api.SeaHavenIndustries.Controllers TradeSpecialties = model.TradeSpecialties, GoogleMapsUrl = model.GoogleMapsUrl, Notes = model.Notes, - IsActive = model.IsActive + IsActive = model.IsActive, + ConfirmOpenWorkOrders = model.ConfirmOpenWorkOrders }; var userId = User.FindFirstValue(ClaimTypes.NameIdentifier); @@ -221,7 +222,10 @@ namespace Api.SeaHavenIndustries.Controllers [HttpDelete("{id}")] [HttpPost("Delete")] - public async Task Delete([FromRoute] int? id, [FromQuery(Name = "id")] int? queryId = null) + public async Task Delete( + [FromRoute] int? id, + [FromQuery(Name = "id")] int? queryId = null, + [FromQuery] bool confirmOpenWorkOrders = false) { try { @@ -234,7 +238,7 @@ namespace Api.SeaHavenIndustries.Controllers if (userId == null) return Unauthorized(new Response { Status = "Error", Message = "User not authenticated" }); - await _vendorService.DeleteVendorAsync(vendorId, userId); + await _vendorService.DeleteVendorAsync(vendorId, userId, confirmOpenWorkOrders); return Ok(new DataResponse { Message = "Vendor Deactivated", Status = "200" }); } catch (VendorDeactivationBlockedException dbex) diff --git a/Api.SeaHavenIndustries/DTOs/Vendor_DTO.cs b/Api.SeaHavenIndustries/DTOs/Vendor_DTO.cs index 6249bb8..6d7b7b8 100644 --- a/Api.SeaHavenIndustries/DTOs/Vendor_DTO.cs +++ b/Api.SeaHavenIndustries/DTOs/Vendor_DTO.cs @@ -22,6 +22,10 @@ namespace Api.SeaHavenIndustries.DTOs public class EditVendor_DTO : Vendor_DTO { public int Id { get; set; } + + // SH-254: set when the caller has been shown the vendor's open work orders + // and chose to deactivate anyway. Without it the open-work-order guard holds. + public bool ConfirmOpenWorkOrders { get; set; } } public class WorkOrderVendorUpdate_DTO diff --git a/SeaHaven.Services/DTOs/VendorDTOs.cs b/SeaHaven.Services/DTOs/VendorDTOs.cs index 908c5c5..bf30e27 100644 --- a/SeaHaven.Services/DTOs/VendorDTOs.cs +++ b/SeaHaven.Services/DTOs/VendorDTOs.cs @@ -80,6 +80,11 @@ namespace SeaHaven.Services.DTOs public string? GoogleMapsUrl { get; set; } public string? Notes { get; set; } public bool? IsActive { get; set; } + + // SH-254: deactivating a vendor with open work orders is allowed, but only + // when the caller has seen those work orders and said so. Absent this flag + // the open-work-order guard still blocks. + public bool ConfirmOpenWorkOrders { get; set; } } public class VendorDeactivationImpactDTO diff --git a/SeaHaven.Services/Implementation/VendorService.cs b/SeaHaven.Services/Implementation/VendorService.cs index 4944a58..cadddfe 100644 --- a/SeaHaven.Services/Implementation/VendorService.cs +++ b/SeaHaven.Services/Implementation/VendorService.cs @@ -193,7 +193,7 @@ namespace SeaHaven.Services.Implementation if (vendor == null) throw new InvalidOperationException($"Vendor with ID {id} not found"); - if (dto.IsActive.HasValue && !dto.IsActive.Value) + if (dto.IsActive.HasValue && !dto.IsActive.Value && !dto.ConfirmOpenWorkOrders) await AssertNoOpenLinkedWorkOrdersAsync(id); if (dto.Name != null) vendor.CompanyName = dto.Name; @@ -224,13 +224,14 @@ namespace SeaHaven.Services.Implementation return MapToDTO(vendor); } - public async Task DeleteVendorAsync(int id, string userId) + public async Task DeleteVendorAsync(int id, string userId, bool confirmOpenWorkOrders) { var vendor = await _vendorDataService.GetByIdAsync(id); if (vendor == null) throw new InvalidOperationException($"Vendor with ID {id} not found"); - await AssertNoOpenLinkedWorkOrdersAsync(id); + if (!confirmOpenWorkOrders) + await AssertNoOpenLinkedWorkOrdersAsync(id); vendor.IsActive = false; vendor.LastModificationTime = DateTime.UtcNow; @@ -502,6 +503,9 @@ namespace SeaHaven.Services.Implementation dto.CompanyPhone = VendorPhoneNormalizer.NormalizeToCanonical(dto.CompanyPhone); } + // Guard for the unconfirmed path only. SH-44 and SH-82 left "blocks or requires + // explicit confirmation" to be settled with the team; SH-254 settles it as + // explicit confirmation, so a caller that has not confirmed is still blocked. private async Task AssertNoOpenLinkedWorkOrdersAsync(int vendorId) { var linked = await _vendorDataService.GetLinkedWorkOrdersAsync(vendorId); diff --git a/SeaHaven.Services/Interfaces/IVendorService.cs b/SeaHaven.Services/Interfaces/IVendorService.cs index b7238da..68e8ae9 100644 --- a/SeaHaven.Services/Interfaces/IVendorService.cs +++ b/SeaHaven.Services/Interfaces/IVendorService.cs @@ -30,7 +30,7 @@ namespace SeaHaven.Services.Interfaces CancellationToken cancellationToken = default); Task CreateVendorAsync(CreateVendorDTO dto, string userId); Task UpdateVendorAsync(int id, UpdateVendorDTO dto, string userId); - Task DeleteVendorAsync(int id, string userId); + Task DeleteVendorAsync(int id, string userId, bool confirmOpenWorkOrders); Task VendorExistsAsync(int id); Task GetTotalVendorCountAsync(); Task GetDeactivationImpactAsync(int vendorId);