From 538117d1144e0dae04c8e94e58b4d7bcd033a308 Mon Sep 17 00:00:00 2001
From: Joseph Doherty <dohejw01@gmail.com>
Date: Fri, 19 Jun 2026 10:51:58 -0400
Subject: [PATCH] fix(sms): reject notification-list Type change on update (S6
 review follow-up)

---
 .../ManagementActor.cs                        |  3 +-
 .../ManagementActorTests.cs                   | 64 ++++++++++++++++++-
 2 files changed, 65 insertions(+), 2 deletions(-)

diff --git a/src/ZB.MOM.WW.ScadaBridge.ManagementService/ManagementActor.cs b/src/ZB.MOM.WW.ScadaBridge.ManagementService/ManagementActor.cs
index 9cdec411..734f6e5f 100644
--- a/src/ZB.MOM.WW.ScadaBridge.ManagementService/ManagementActor.cs
+++ b/src/ZB.MOM.WW.ScadaBridge.ManagementService/ManagementActor.cs
@@ -1701,8 +1701,9 @@ public class ManagementActor : ReceiveActor
         var repo = sp.GetRequiredService<INotificationRepository>();
         var list = await repo.GetNotificationListByIdAsync(cmd.NotificationListId)
             ?? throw new ManagementCommandException($"NotificationList with ID {cmd.NotificationListId} not found.");
+        if (list.Type != cmd.Type)
+            throw new ManagementCommandException("A notification list's Type cannot be changed after creation.");
         list.Name = cmd.Name;
-        list.Type = cmd.Type;
 
         var existingRecipients = await repo.GetRecipientsByListIdAsync(cmd.NotificationListId);
         foreach (var r in existingRecipients)
diff --git a/tests/ZB.MOM.WW.ScadaBridge.ManagementService.Tests/ManagementActorTests.cs b/tests/ZB.MOM.WW.ScadaBridge.ManagementService.Tests/ManagementActorTests.cs
index 3cfc4cea..70037387 100644
--- a/tests/ZB.MOM.WW.ScadaBridge.ManagementService.Tests/ManagementActorTests.cs
+++ b/tests/ZB.MOM.WW.ScadaBridge.ManagementService.Tests/ManagementActorTests.cs
@@ -1429,11 +1429,13 @@ public class ManagementActorTests : TestKit, IDisposable
     [Fact]
     public void UpdateNotificationList_WithSmsType_PersistsSms()
     {
+        // Updating an existing SMS list (same type) must succeed and preserve the
+        // SMS channel — the immutability guard only fires when Type changes.
         var notifRepo = Substitute.For<INotificationRepository>();
         var existing = new Commons.Entities.Notifications.NotificationList("Ops")
         {
             Id = 7,
-            Type = Commons.Types.Enums.NotificationType.Email,
+            Type = Commons.Types.Enums.NotificationType.Sms,
         };
         notifRepo.GetNotificationListByIdAsync(7, Arg.Any<CancellationToken>()).Returns(existing);
         notifRepo.GetRecipientsByListIdAsync(7, Arg.Any<CancellationToken>())
@@ -1453,6 +1455,66 @@ public class ManagementActorTests : TestKit, IDisposable
         Assert.Equal(Commons.Types.Enums.NotificationType.Sms, existing.Type);
     }
 
+    [Fact]
+    public void UpdateNotificationList_ChangingType_IsRejected()
+    {
+        // Attempting to change an Email list's Type to Sms must return a
+        // COMMAND_FAILED error — the Type field is immutable after creation.
+        var notifRepo = Substitute.For<INotificationRepository>();
+        var existing = new Commons.Entities.Notifications.NotificationList("Ops")
+        {
+            Id = 8,
+            Type = Commons.Types.Enums.NotificationType.Email,
+        };
+        notifRepo.GetNotificationListByIdAsync(8, Arg.Any<CancellationToken>()).Returns(existing);
+        notifRepo.GetRecipientsByListIdAsync(8, Arg.Any<CancellationToken>())
+            .Returns(new List<Commons.Entities.Notifications.NotificationRecipient>());
+        _services.AddScoped(_ => notifRepo);
+
+        var actor = CreateActor();
+        var envelope = Envelope(
+            new UpdateNotificationListCommand(8, "Ops", new[] { "+15551230000" },
+                Commons.Types.Enums.NotificationType.Sms),
+            "Designer");
+
+        actor.Tell(envelope);
+
+        var response = ExpectMsg<ManagementError>(TimeSpan.FromSeconds(5));
+        Assert.Equal(envelope.CorrelationId, response.CorrelationId);
+        Assert.Equal("COMMAND_FAILED", response.ErrorCode);
+        Assert.Contains("Type cannot be changed", response.Error);
+    }
+
+    [Fact]
+    public void UpdateNotificationList_SameEmailType_Succeeds()
+    {
+        // Updating an Email list while keeping Type = Email must succeed — the
+        // immutability guard must not fire on same-type updates.
+        var notifRepo = Substitute.For<INotificationRepository>();
+        var existing = new Commons.Entities.Notifications.NotificationList("Alerts")
+        {
+            Id = 9,
+            Type = Commons.Types.Enums.NotificationType.Email,
+        };
+        notifRepo.GetNotificationListByIdAsync(9, Arg.Any<CancellationToken>()).Returns(existing);
+        notifRepo.GetRecipientsByListIdAsync(9, Arg.Any<CancellationToken>())
+            .Returns(new List<Commons.Entities.Notifications.NotificationRecipient>());
+        _services.AddScoped(_ => notifRepo);
+
+        var actor = CreateActor();
+        var envelope = Envelope(
+            new UpdateNotificationListCommand(9, "Alerts-Renamed", new[] { "ops@example.com" },
+                Commons.Types.Enums.NotificationType.Email),
+            "Designer");
+
+        actor.Tell(envelope);
+
+        var response = ExpectMsg<ManagementSuccess>(TimeSpan.FromSeconds(5));
+        Assert.Equal(envelope.CorrelationId, response.CorrelationId);
+        Assert.Equal("Alerts-Renamed", existing.Name);
+        Assert.Equal(Commons.Types.Enums.NotificationType.Email, existing.Type);
+    }
+
     [Fact]
     public void UpdateSmsConfig_WithApiBaseUrlAndAuthToken_PersistsThem()
     {