fix(sms): reject notification-list Type change on update (S6 review follow-up)
This commit is contained in:
@@ -1701,8 +1701,9 @@ public class ManagementActor : ReceiveActor
|
|||||||
var repo = sp.GetRequiredService<INotificationRepository>();
|
var repo = sp.GetRequiredService<INotificationRepository>();
|
||||||
var list = await repo.GetNotificationListByIdAsync(cmd.NotificationListId)
|
var list = await repo.GetNotificationListByIdAsync(cmd.NotificationListId)
|
||||||
?? throw new ManagementCommandException($"NotificationList with ID {cmd.NotificationListId} not found.");
|
?? 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.Name = cmd.Name;
|
||||||
list.Type = cmd.Type;
|
|
||||||
|
|
||||||
var existingRecipients = await repo.GetRecipientsByListIdAsync(cmd.NotificationListId);
|
var existingRecipients = await repo.GetRecipientsByListIdAsync(cmd.NotificationListId);
|
||||||
foreach (var r in existingRecipients)
|
foreach (var r in existingRecipients)
|
||||||
|
|||||||
@@ -1429,11 +1429,13 @@ public class ManagementActorTests : TestKit, IDisposable
|
|||||||
[Fact]
|
[Fact]
|
||||||
public void UpdateNotificationList_WithSmsType_PersistsSms()
|
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 notifRepo = Substitute.For<INotificationRepository>();
|
||||||
var existing = new Commons.Entities.Notifications.NotificationList("Ops")
|
var existing = new Commons.Entities.Notifications.NotificationList("Ops")
|
||||||
{
|
{
|
||||||
Id = 7,
|
Id = 7,
|
||||||
Type = Commons.Types.Enums.NotificationType.Email,
|
Type = Commons.Types.Enums.NotificationType.Sms,
|
||||||
};
|
};
|
||||||
notifRepo.GetNotificationListByIdAsync(7, Arg.Any<CancellationToken>()).Returns(existing);
|
notifRepo.GetNotificationListByIdAsync(7, Arg.Any<CancellationToken>()).Returns(existing);
|
||||||
notifRepo.GetRecipientsByListIdAsync(7, Arg.Any<CancellationToken>())
|
notifRepo.GetRecipientsByListIdAsync(7, Arg.Any<CancellationToken>())
|
||||||
@@ -1453,6 +1455,66 @@ public class ManagementActorTests : TestKit, IDisposable
|
|||||||
Assert.Equal(Commons.Types.Enums.NotificationType.Sms, existing.Type);
|
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]
|
[Fact]
|
||||||
public void UpdateSmsConfig_WithApiBaseUrlAndAuthToken_PersistsThem()
|
public void UpdateSmsConfig_WithApiBaseUrlAndAuthToken_PersistsThem()
|
||||||
{
|
{
|
||||||
|
|||||||
Reference in New Issue
Block a user