fix(clinical): detect duplicate schedules by SQL error number

test/ClinicalScheduler/ScheduleEditServiceTest.cs

@@ -1,3 +1,4 @@
+using Microsoft.Data.SqlClient;
 using Microsoft.EntityFrameworkCore;
 using Microsoft.Extensions.Logging;
 using Microsoft.Extensions.Options;
@@ -23,7 +24,7 @@ public class ScheduleEditServiceTest : IDisposable
         private readonly IPermissionValidator _mockPermissionValidator;
         private readonly IUserHelper _mockUserHelper;
         private readonly IEmailTemplateRenderer _mockEmailTemplateRenderer;
-        private readonly ClinicalSchedulerContext _context;
+        private readonly ToggleThrowContext _context;
         private readonly TestableScheduleEditService _service;
         private bool _disposed;
 
@@ -33,7 +34,7 @@ public ScheduleEditServiceTest()
             var options = new DbContextOptionsBuilder<ClinicalSchedulerContext>()
                 .UseInMemoryDatabase(databaseName: Guid.NewGuid().ToString())
                 .Options;
-            _context = new ClinicalSchedulerContext(options);
+            _context = new ToggleThrowContext(options);
 
             _mockPermissionService = Substitute.For<ISchedulePermissionService>();
             _mockAuditService = Substitute.For<IScheduleAuditService>();
@@ -427,6 +428,52 @@ public async Task AddInstructorAsync_WithScheduleConflicts_ThrowsInvalidOperatio
                 $"Expected duplicate schedule error but got: {ex.Message}");
         }
 
+        [Fact]
+        public async Task AddInstructorAsync_DbSaveThrowsUniqueConstraint2627_ThrowsAlreadyScheduled()
+        {
+            // Arrange - pre-checks pass, but the database rejects the insert with a unique-constraint
+            // violation (a race condition slipping past the in-app duplicate check). SQL error 2627
+            // must be classified as a duplicate and surfaced to callers as the "already scheduled" path.
+            _context.SaveException = SqlExceptionFactory.Create(2627);
+            var testYear = DateTime.Now.Year + 1;
+
+            // Act & Assert
+            var ex = await Assert.ThrowsAsync<InvalidOperationException>(() =>
+                _service.AddInstructorAsync("test123", 1, new[] { 1 }, testYear, false, TestContext.Current.CancellationToken));
+            Assert.Contains("already be scheduled", ex.Message);
+            Assert.IsType<SqlException>(ex.InnerException);
+        }
+
+        [Fact]
+        public async Task AddInstructorAsync_DbSaveThrowsDuplicateKey2601WrappedInDbUpdate_ThrowsAlreadyScheduled()
+        {
+            // Arrange - EF wraps provider errors in DbUpdateException. SQL error 2601 (duplicate key in a
+            // unique index) must be unwrapped from the inner SqlException and classified as a duplicate.
+            _context.SaveException = new DbUpdateException("save failed", SqlExceptionFactory.Create(2601));
+            var testYear = DateTime.Now.Year + 1;
+
+            // Act & Assert
+            var ex = await Assert.ThrowsAsync<InvalidOperationException>(() =>
+                _service.AddInstructorAsync("test123", 1, new[] { 1 }, testYear, false, TestContext.Current.CancellationToken));
+            Assert.Contains("already be scheduled", ex.Message);
+            Assert.IsType<DbUpdateException>(ex.InnerException);
+        }
+
+        [Fact]
+        public async Task AddInstructorAsync_DbSaveThrowsNonDuplicateSqlError_ThrowsGenericDatabaseError()
+        {
+            // Arrange - a non-duplicate database error (547 = constraint/foreign-key violation) must NOT be
+            // treated as a duplicate; it falls through to the generic "database operation failed" message.
+            _context.SaveException = SqlExceptionFactory.Create(547);
+            var testYear = DateTime.Now.Year + 1;
+
+            // Act & Assert
+            var ex = await Assert.ThrowsAsync<InvalidOperationException>(() =>
+                _service.AddInstructorAsync("test123", 1, new[] { 1 }, testYear, false, TestContext.Current.CancellationToken));
+            Assert.Contains("Database operation failed", ex.Message);
+            Assert.IsType<SqlException>(ex.InnerException);
+        }
+
         [Fact]
         public async Task RemoveInstructorScheduleAsync_ValidSchedule_RemovesSuccessfully()
         {

test/ClinicalScheduler/SqlExceptionFactory.cs

@@ -0,0 +1,79 @@
+using System.Reflection;
+using Microsoft.Data.SqlClient;
+
+namespace Viper.test.ClinicalScheduler
+{
+    /// <summary>
+    /// Builds <see cref="SqlException"/> instances carrying a specific SQL Server error number.
+    /// SqlException/SqlError/SqlErrorCollection have no public constructors, so this reaches the
+    /// library's internal members via reflection. Targets Microsoft.Data.SqlClient and is resilient
+    /// to constructor-signature changes across versions.
+    /// </summary>
+    internal static class SqlExceptionFactory
+    {
+        public static SqlException Create(int errorNumber)
+        {
+            var errors = NewErrorCollection();
+            AddError(errors, NewError(errorNumber));
+            return NewException(errors);
+        }
+
+        private static SqlErrorCollection NewErrorCollection()
+            => (SqlErrorCollection)Activator.CreateInstance(typeof(SqlErrorCollection), nonPublic: true)!;
+
+        private static SqlError NewError(int number)
+        {
+            var ctor = typeof(SqlError)
+                .GetConstructors(BindingFlags.Instance | BindingFlags.NonPublic | BindingFlags.Public)
+                .OrderByDescending(c => c.GetParameters().Length)
+                .FirstOrDefault()
+                ?? throw new InvalidOperationException("No SqlError constructor found via reflection.");
+
+            var parameters = ctor.GetParameters();
+            var numberIndex = Array.FindIndex(parameters, p => string.Equals(p.Name, "infoNumber", StringComparison.OrdinalIgnoreCase));
+            if (numberIndex < 0)
+            {
+                numberIndex = Array.FindIndex(parameters, p => p.ParameterType == typeof(int));
+            }
+
+            var args = new object?[parameters.Length];
+            for (var i = 0; i < parameters.Length; i++)
+            {
+                args[i] = i == numberIndex ? number : DefaultArg(parameters[i]);
+            }
+
+            return (SqlError)ctor.Invoke(args);
+        }
+
+        private static object? DefaultArg(ParameterInfo parameter)
+        {
+            var type = parameter.ParameterType;
+            if (type == typeof(string)) return "test";
+            if (type == typeof(byte)) return (byte)0;
+            if (type == typeof(int)) return 0;
+            if (type == typeof(uint)) return 0u;
+            if (type == typeof(bool)) return false;
+            if (parameter.HasDefaultValue) return parameter.DefaultValue;
+            return type.IsValueType ? Activator.CreateInstance(type) : null;
+        }
+
+        private static void AddError(SqlErrorCollection errors, SqlError error)
+        {
+            var add = typeof(SqlErrorCollection).GetMethod("Add", BindingFlags.Instance | BindingFlags.NonPublic)
+                ?? throw new InvalidOperationException("SqlErrorCollection.Add not found via reflection.");
+            add.Invoke(errors, new object[] { error });
+        }
+
+        private static SqlException NewException(SqlErrorCollection errors)
+        {
+            var create = typeof(SqlException).GetMethod(
+                "CreateException",
+                BindingFlags.Static | BindingFlags.NonPublic,
+                binder: null,
+                types: new[] { typeof(SqlErrorCollection), typeof(string) },
+                modifiers: null)
+                ?? throw new InvalidOperationException("SqlException.CreateException(SqlErrorCollection, string) not found via reflection.");
+            return (SqlException)create.Invoke(null, new object[] { errors, "11.0.0" })!;
+        }
+    }
+}

test/ClinicalScheduler/ToggleThrowContext.cs

@@ -0,0 +1,27 @@
+using Microsoft.EntityFrameworkCore;
+using Viper.Classes.SQLContext;
+
+namespace Viper.test.ClinicalScheduler
+{
+    /// <summary>
+    /// In-memory <see cref="ClinicalSchedulerContext"/> that can be told to throw a specific exception
+    /// from SaveChanges, so tests can exercise the database-failure handling in
+    /// <see cref="Viper.Areas.ClinicalScheduler.Services.ScheduleEditService"/>. Leave
+    /// <see cref="SaveException"/> null for normal behavior (e.g. seeding test data).
+    /// </summary>
+    internal sealed class ToggleThrowContext : ClinicalSchedulerContext
+    {
+        public ToggleThrowContext(DbContextOptions<ClinicalSchedulerContext> options) : base(options)
+        {
+        }
+
+        /// <summary>When set, the next SaveChanges call throws this exception instead of persisting.</summary>
+        public Exception? SaveException { get; set; }
+
+        public override int SaveChanges()
+            => SaveException is not null ? throw SaveException : base.SaveChanges();
+
+        public override Task<int> SaveChangesAsync(CancellationToken cancellationToken = default)
+            => SaveException is not null ? Task.FromException<int>(SaveException) : base.SaveChangesAsync(cancellationToken);
+    }
+}

web/Areas/ClinicalScheduler/Services/ScheduleEditService.cs

@@ -247,12 +247,9 @@ await _auditService.LogPrimaryEvaluatorSetAsync(
                 _logger.LogError(saveEx, "Database save failed for MothraId='{MothraId}', RotationId={RotationId}, WeekIds=[{WeekIds}]",
                     LogSanitizer.SanitizeId(mothraId), rotationId, string.Join(",", weekIds));
 
-                // Check if this is a database constraint violation (typically a duplicate key error)
-                var errorMessage = saveEx.Message?.ToLower();
-                var innerMessage = saveEx.InnerException?.Message?.ToLower();
-
-                if ((errorMessage != null && (errorMessage.Contains("duplicate") || errorMessage.Contains("unique") || errorMessage.Contains("constraint") || errorMessage.Contains("violation of primary key"))) ||
-                    (innerMessage != null && (innerMessage.Contains("duplicate") || innerMessage.Contains("unique") || innerMessage.Contains("constraint") || innerMessage.Contains("violation of primary key"))))
+                // Only a unique/duplicate-key violation means the instructor is already scheduled.
+                // Other DB errors (foreign-key, check constraints, etc.) fall through to the generic message.
+                if (IsDuplicateKeyViolation(saveEx))
                 {
                     throw new InvalidOperationException($"Instructor {mothraId} appears to already be scheduled for one or more of the specified weeks. Please refresh the page and try again.", saveEx);
                 }
@@ -262,6 +259,15 @@ await _auditService.LogPrimaryEvaluatorSetAsync(
             }
         }
 
+        // SQL Server: 2627 = unique constraint violation, 2601 = duplicate key in a unique index.
+        // EF wraps the provider error in DbUpdateException, so unwrap to the underlying SqlException.
+        private static bool IsDuplicateKeyViolation(Exception ex)
+        {
+            var sqlEx = ex as SqlException ?? ex.InnerException as SqlException;
+            return sqlEx is not null
+                && sqlEx.Errors.Cast<SqlError>().Any(e => e.Number is 2627 or 2601);
+        }
+
         public async Task<(bool success, bool wasPrimaryEvaluator, string? instructorName)> RemoveInstructorScheduleAsync(
             int instructorScheduleId,
             CancellationToken cancellationToken = default)