fix(work-orders): address PR14 phase-2 review feedback

SeaHaven.Services/Board/WorkOrderBoardConstants.cs

@@ -1,11 +0,0 @@
-namespace SeaHaven.Services.Board
-{
-    /// <summary>
-    /// Shared board constants for Phase 2+ features. Not referenced in Phase 1.
-    /// </summary>
-    public static class WorkOrderBoardConstants
-    {
-        public const int MaxWindowDays = 90;
-        public const int ClientSideThreshold = 300;
-    }
-}

SeaHaven.Services/Helpers/WorkOrderBoardMutationRules.cs

@@ -4,22 +4,25 @@ namespace SeaHaven.Services.Helpers
 {
     public static class WorkOrderBoardMutationRules
     {
-        private static readonly HashSet<LifecycleStatus?> ReadOnlyStatuses = new()
-        {
-            LifecycleStatus.Canceled,
-            LifecycleStatus.Completed
-        };
+        private static readonly HashSet<LifecycleStatus?> ReadOnlyStatuses =
+            LifecycleStatusSets.Terminal.ToHashSet();
 
         public static bool IsReadOnly(LifecycleStatus? status) => ReadOnlyStatuses.Contains(status);
 
         public static bool ShouldBlockStatusChangeWhenPastDue(string field, bool isPastDue)
             => field.Equals(WorkOrderBoardFieldNames.LifecycleStatus, StringComparison.OrdinalIgnoreCase) && isPastDue;
 
+        /// <summary>
+        /// SHOC rule: Incomplete + specific scheduled date + assignee → Scheduled.
+        /// Week-only targets do not auto-schedule (same as <see cref="WorkOrderDerivedFields.ApplyAutoScheduleIfEligible"/>).
+        /// </summary>
         public static bool ShouldAutoSchedule(
             LifecycleStatus? status,
             DateTime? scheduledDate,
-            string? assignTo)
+            string? assignTo,
+            bool? scheduleWeekOnly = null)
             => status == LifecycleStatus.Incomplete
+               && scheduleWeekOnly != true
                && scheduledDate.HasValue
                && !string.IsNullOrWhiteSpace(assignTo);
 

SeaHaven.Services/Implementation/WorkOrderBoardUpdateService.cs

@@ -30,6 +30,13 @@ namespace SeaHaven.Services.Implementation
             WorkOrderBoardPatchRequestDto request,
             string? actorId)
         {
+            // Intermediate SaveChanges (e.g. new dispatch identity) must stay atomic with the final patch save.
+            await using var transaction = _context.Database.IsRelational()
+                ? await _context.Database.BeginTransactionAsync()
+                : null;
+
+            try
+            {
             if (string.IsNullOrWhiteSpace(request.Field))
                 throw new WorkOrderBoardValidationException("InvalidField", "Field is required.");
 
@@ -94,12 +101,16 @@ namespace SeaHaven.Services.Implementation
             if (changes.Count == 0 && !_context.ChangeTracker.HasChanges())
             {
                 var unchanged = await LoadBoardRowAsync(workOrderId);
+                if (transaction is not null)
+                    await transaction.CommitAsync();
                 return unchanged ?? throw new WorkOrderBoardValidationException("NotFound", "Work order not found.");
             }
 
             if (changes.Count == 0)
             {
                 await _context.SaveChangesAsync();
+                if (transaction is not null)
+                    await transaction.CommitAsync();
                 var persisted = await LoadBoardRowAsync(workOrderId);
                 return persisted ?? throw new WorkOrderBoardValidationException("NotFound", "Work order not found.");
             }
@@ -127,13 +138,29 @@ namespace SeaHaven.Services.Implementation
             }
             catch (DbUpdateConcurrencyException)
             {
+                if (transaction is not null)
+                    await transaction.RollbackAsync();
                 _context.ChangeTracker.Clear();
                 var currentState = await LoadBoardRowAsync(workOrderId);
                 throw new WorkOrderBoardConcurrencyException(currentState);
             }
 
+            if (transaction is not null)
+                await transaction.CommitAsync();
+
             var row = await LoadBoardRowAsync(workOrderId);
             return row ?? throw new WorkOrderBoardValidationException("NotFound", "Work order not found.");
+            }
+            catch (WorkOrderBoardConcurrencyException)
+            {
+                throw;
+            }
+            catch
+            {
+                if (transaction is not null)
+                    await transaction.RollbackAsync();
+                throw;
+            }
         }
 
         private async Task<List<FieldChange>> ApplyFieldMutationAsync(
@@ -154,7 +181,7 @@ namespace SeaHaven.Services.Implementation
                 WorkOrderBoardFieldNames.ScheduledDate => ApplyScheduledDate(workOrder, value, auditField),
                 WorkOrderBoardFieldNames.TargetWeek => new List<FieldChange> { ApplyTargetWeek(workOrder, value, auditField) },
                 WorkOrderBoardFieldNames.ScheduleWeekOnly => new List<FieldChange> { ApplyBoolField(value, auditField, v => workOrder.ScheduleWeekOnly = v, () => workOrder.ScheduleWeekOnly) },
-                WorkOrderBoardFieldNames.VendorId => new List<FieldChange> { ApplyVendorId(dispatch!, value, auditField) },
+                WorkOrderBoardFieldNames.VendorId => new List<FieldChange> { await ApplyVendorIdAsync(dispatch!, value, auditField) },
                 WorkOrderBoardFieldNames.ApptDate => new List<FieldChange> { ApplyApptDate(dispatch!, value, auditField) },
                 WorkOrderBoardFieldNames.ApptTime => new List<FieldChange> { ApplyApptTime(workOrder, dispatch!, value, auditField) },
                 WorkOrderBoardFieldNames.DocStatus => new List<FieldChange> { ApplyDocStatus(workOrder, value, auditField) },
@@ -188,6 +215,8 @@ namespace SeaHaven.Services.Implementation
             if (!vendorIdForCreate.HasValue || vendorIdForCreate.Value <= 0)
                 throw new WorkOrderBoardValidationException("DispatchRequired", "A primary dispatch is required. Set vendorId first or provide primaryDispatchId.");
 
+            await EnsureVendorExistsAsync(vendorIdForCreate.Value);
+
             var created = new Dispatch
             {
                 WorkOrderId = workOrder.Id,
@@ -196,10 +225,18 @@ namespace SeaHaven.Services.Implementation
                 CreatedDate = DateTime.UtcNow
             };
             _context.Dispatches.Add(created);
-            workOrder.PrimaryDispatch = created;
+            await _context.SaveChangesAsync();
+            workOrder.PrimaryDispatchId = created.Id;
             return created;
         }
 
+        private async Task EnsureVendorExistsAsync(int vendorId)
+        {
+            var vendorExists = await _context.Vendors.AnyAsync(v => v.Id == vendorId && v.IsDeleted != true);
+            if (!vendorExists)
+                throw new WorkOrderBoardValidationException("VendorNotFound", "vendorId does not exist.");
+        }
+
         private async Task<FieldChange> ApplyWoNumber(WorkOrder workOrder, string? value, string auditField)
         {
             if (!WorkOrderNumberNormalizer.TryNormalize(value, out var normalized, out var error))
@@ -384,12 +421,14 @@ namespace SeaHaven.Services.Implementation
             return FieldChange.ForField(auditField, old, parsed.ToString());
         }
 
-        private static FieldChange ApplyVendorId(Dispatch dispatch, string? value, string auditField)
+        private async Task<FieldChange> ApplyVendorIdAsync(Dispatch dispatch, string? value, string auditField)
         {
             var vendorId = ParseVendorIdHint(value);
             if (!vendorId.HasValue)
                 throw new WorkOrderBoardValidationException("InvalidValue", "vendorId must be a positive integer.");
 
+            await EnsureVendorExistsAsync(vendorId.Value);
+
             var old = dispatch.VendorId.ToString();
             if (dispatch.VendorId == vendorId.Value)
                 return FieldChange.Unchanged(auditField, ResolveDispatchIdForAudit(dispatch));
@@ -461,7 +500,11 @@ namespace SeaHaven.Services.Implementation
         private static List<FieldChange> ApplyAutoScheduleSideEffects(WorkOrder workOrder)
         {
             var changes = new List<FieldChange>();
-            if (!WorkOrderBoardMutationRules.ShouldAutoSchedule(workOrder.LifecycleStatus, workOrder.ScheduledDate, workOrder.AssignTo))
+            if (!WorkOrderBoardMutationRules.ShouldAutoSchedule(
+                    workOrder.LifecycleStatus,
+                    workOrder.ScheduledDate,
+                    workOrder.AssignTo,
+                    workOrder.ScheduleWeekOnly))
                 return changes;
 
             var old = workOrder.LifecycleStatus?.ToString();

SeaHavenIndustries.Tests/WorkOrderBoardMutationRulesTests.cs

@@ -42,6 +42,16 @@ public class WorkOrderBoardMutationRulesTests
             "dispatcher-1"));
     }
 
+    [Fact]
+    public void ShouldAutoSchedule_FalseWhenScheduleWeekOnly()
+    {
+        Assert.False(WorkOrderBoardMutationRules.ShouldAutoSchedule(
+            LifecycleStatus.Incomplete,
+            new DateTime(2026, 6, 25),
+            "dispatcher-1",
+            scheduleWeekOnly: true));
+    }
+
     [Fact]
     public void IsReschedule_WhenPreviousDateExistsAndChanges()
     {

SeaHavenIndustries.Tests/WorkOrderBoardUpdateServiceTests.cs

@@ -274,4 +274,182 @@ public class WorkOrderBoardUpdateServiceTests
 
         Assert.Equal("DuplicateWoNumber", ex.Code);
     }
+
+    [Fact]
+    public async Task PatchField_WeekOnly_DoesNotAutoScheduleOnAssignTo()
+    {
+        var (context, service) = CreateSut();
+        var wo = new WorkOrder
+        {
+            Id = 1,
+            LifecycleStatus = LifecycleStatus.Incomplete,
+            ScheduledDate = new DateTime(2026, 6, 25),
+            ScheduleWeekOnly = true,
+            TargetWeek = new DateOnly(2026, 6, 22),
+            RowVersion = new byte[] { 1, 0, 0, 0, 0, 0, 0, 1 }
+        };
+        context.workOrders.Add(wo);
+        await context.SaveChangesAsync();
+
+        var result = await service.PatchFieldAsync(1, new WorkOrderBoardPatchRequestDto
+        {
+            Field = WorkOrderBoardFieldNames.AssignTo,
+            Value = "user-a",
+            WorkOrderVersion = ToVersion(wo)
+        }, "actor-1");
+
+        Assert.Equal(LifecycleStatus.Incomplete, result.LifecycleStatus);
+        Assert.Equal("user-a", result.DispatcherId);
+    }
+
+    [Fact]
+    public async Task PatchField_ApptTime_UpdatesScheduledStart()
+    {
+        var (context, service) = CreateSut();
+        var dispatch = new Dispatch
+        {
+            Id = 10,
+            WorkOrderId = 1,
+            VendorId = 1,
+            ScheduledDate = new DateTime(2026, 6, 25),
+            RowVersion = new byte[] { 1, 0, 0, 0, 0, 0, 0, 2 }
+        };
+        var wo = new WorkOrder
+        {
+            Id = 1,
+            LifecycleStatus = LifecycleStatus.Scheduled,
+            PrimaryDispatchId = 10,
+            ScheduledDate = new DateTime(2026, 6, 25),
+            RowVersion = new byte[] { 1, 0, 0, 0, 0, 0, 0, 1 }
+        };
+        context.Vendors.Add(new Vendor { Id = 1, CompanyName = "Vendor A" });
+        context.Dispatches.Add(dispatch);
+        context.workOrders.Add(wo);
+        await context.SaveChangesAsync();
+
+        var result = await service.PatchFieldAsync(1, new WorkOrderBoardPatchRequestDto
+        {
+            Field = WorkOrderBoardFieldNames.ApptTime,
+            Value = "09:00-10:00",
+            WorkOrderVersion = ToVersion(wo),
+            DispatchVersion = ToVersion(dispatch),
+            PrimaryDispatchId = 10
+        }, "actor-1");
+
+        Assert.Contains("09:00", result.ApptTime);
+        var reloaded = await context.workOrders.FindAsync(1);
+        Assert.Equal(new DateTime(2026, 6, 25, 9, 0, 0), reloaded!.ScheduledStart);
+        Assert.Equal(new DateTime(2026, 6, 25, 10, 0, 0), reloaded.ScheduledEnd);
+    }
+
+    [Fact]
+    public async Task PatchField_ApptTime_DashPrefixedToken_TreatedAsSingleStart()
+    {
+        var (context, service) = CreateSut();
+        var dispatch = new Dispatch
+        {
+            Id = 10,
+            WorkOrderId = 1,
+            VendorId = 1,
+            ScheduledDate = new DateTime(2026, 6, 25),
+            RowVersion = new byte[] { 1, 0, 0, 0, 0, 0, 0, 2 }
+        };
+        var wo = new WorkOrder
+        {
+            Id = 1,
+            LifecycleStatus = LifecycleStatus.Scheduled,
+            PrimaryDispatchId = 10,
+            ScheduledDate = new DateTime(2026, 6, 25),
+            RowVersion = new byte[] { 1, 0, 0, 0, 0, 0, 0, 1 }
+        };
+        context.Vendors.Add(new Vendor { Id = 1, CompanyName = "Vendor A" });
+        context.Dispatches.Add(dispatch);
+        context.workOrders.Add(wo);
+        await context.SaveChangesAsync();
+
+        await service.PatchFieldAsync(1, new WorkOrderBoardPatchRequestDto
+        {
+            Field = WorkOrderBoardFieldNames.ApptTime,
+            Value = "-30",
+            WorkOrderVersion = ToVersion(wo),
+            DispatchVersion = ToVersion(dispatch),
+            PrimaryDispatchId = 10
+        }, "actor-1");
+
+        var reloaded = await context.workOrders.FindAsync(1);
+        Assert.NotNull(reloaded!.ScheduledStart);
+        Assert.Equal(new DateTime(2026, 6, 25), reloaded.ScheduledEnd);
+        Assert.Equal(new DateTime(2026, 6, 25).Add(TimeSpan.FromDays(30)), reloaded.ScheduledStart);
+    }
+
+    [Fact]
+    public async Task PatchField_DispatchFieldWithoutVersion_ThrowsDispatchVersionRequired()
+    {
+        var (context, service) = CreateSut();
+        var dispatch = new Dispatch
+        {
+            Id = 10,
+            WorkOrderId = 1,
+            VendorId = 1,
+            RowVersion = new byte[] { 1, 0, 0, 0, 0, 0, 0, 2 }
+        };
+        var wo = new WorkOrder
+        {
+            Id = 1,
+            LifecycleStatus = LifecycleStatus.Scheduled,
+            PrimaryDispatchId = 10,
+            RowVersion = new byte[] { 1, 0, 0, 0, 0, 0, 0, 1 }
+        };
+        context.Vendors.Add(new Vendor { Id = 1, CompanyName = "Vendor A" });
+        context.Dispatches.Add(dispatch);
+        context.workOrders.Add(wo);
+        await context.SaveChangesAsync();
+
+        var ex = await Assert.ThrowsAsync<WorkOrderBoardValidationException>(() =>
+            service.PatchFieldAsync(1, new WorkOrderBoardPatchRequestDto
+            {
+                Field = WorkOrderBoardFieldNames.ApptDate,
+                Value = "2026-06-26",
+                WorkOrderVersion = ToVersion(wo),
+                PrimaryDispatchId = 10
+            }, "actor-1"));
+
+        Assert.Equal("DispatchVersionRequired", ex.Code);
+    }
+
+    [Fact]
+    public async Task PatchField_VendorIdNotFound_Throws()
+    {
+        var (context, service) = CreateSut();
+        var dispatch = new Dispatch
+        {
+            Id = 10,
+            WorkOrderId = 1,
+            VendorId = 1,
+            RowVersion = new byte[] { 1, 0, 0, 0, 0, 0, 0, 2 }
+        };
+        var wo = new WorkOrder
+        {
+            Id = 1,
+            LifecycleStatus = LifecycleStatus.Scheduled,
+            PrimaryDispatchId = 10,
+            RowVersion = new byte[] { 1, 0, 0, 0, 0, 0, 0, 1 }
+        };
+        context.Vendors.Add(new Vendor { Id = 1, CompanyName = "Vendor A" });
+        context.Dispatches.Add(dispatch);
+        context.workOrders.Add(wo);
+        await context.SaveChangesAsync();
+
+        var ex = await Assert.ThrowsAsync<WorkOrderBoardValidationException>(() =>
+            service.PatchFieldAsync(1, new WorkOrderBoardPatchRequestDto
+            {
+                Field = WorkOrderBoardFieldNames.VendorId,
+                Value = "999",
+                WorkOrderVersion = ToVersion(wo),
+                DispatchVersion = ToVersion(dispatch),
+                PrimaryDispatchId = 10
+            }, "actor-1"));
+
+        Assert.Equal("VendorNotFound", ex.Code);
+    }
 }