From 73df322a66775c7255cd1036082280889717f5c9 Mon Sep 17 00:00:00 2001 From: Joseph Doherty Date: Fri, 19 Jun 2026 10:40:09 -0400 Subject: [PATCH] feat(sms): CLI list --type/--phones + notification sms group + channel-aware recipients (S6) --- .../Commands/NotificationCommands.cs | 256 ++++++++++++++-- src/ZB.MOM.WW.ScadaBridge.CLI/README.md | 50 +++- .../Management/NotificationCommands.cs | 4 +- .../ManagementActor.cs | 39 ++- .../Commands/NotificationSmsCommandTests.cs | 273 ++++++++++++++++++ .../UpdateCommandContractTests.cs | 16 +- .../ManagementActorTests.cs | 37 +++ 7 files changed, 638 insertions(+), 37 deletions(-) create mode 100644 tests/ZB.MOM.WW.ScadaBridge.CLI.Tests/Commands/NotificationSmsCommandTests.cs diff --git a/src/ZB.MOM.WW.ScadaBridge.CLI/Commands/NotificationCommands.cs b/src/ZB.MOM.WW.ScadaBridge.CLI/Commands/NotificationCommands.cs index ac97adf9..530a638e 100644 --- a/src/ZB.MOM.WW.ScadaBridge.CLI/Commands/NotificationCommands.cs +++ b/src/ZB.MOM.WW.ScadaBridge.CLI/Commands/NotificationCommands.cs @@ -1,6 +1,7 @@ using System.CommandLine; using System.CommandLine.Parsing; using ZB.MOM.WW.ScadaBridge.Commons.Messages.Management; +using ZB.MOM.WW.ScadaBridge.Commons.Types.Enums; namespace ZB.MOM.WW.ScadaBridge.CLI.Commands; @@ -24,10 +25,43 @@ public static class NotificationCommands command.Add(BuildUpdate(urlOption, formatOption, usernameOption, passwordOption)); command.Add(BuildDelete(urlOption, formatOption, usernameOption, passwordOption)); command.Add(BuildSmtp(urlOption, formatOption, usernameOption, passwordOption)); + command.Add(BuildSms(urlOption, formatOption, usernameOption, passwordOption)); return command; } + // ------------------------------------------------------------------------ + // Notification list create/update options (static so the parsed values can + // be read back both from the SetAction and from the testable Build* helpers). + // ------------------------------------------------------------------------ + private static readonly Option ListCreateNameOption = + new("--name") { Description = "Notification list name", Required = true }; + private static readonly Option ListCreateEmailsOption = + new("--emails") { Description = "Comma-separated recipient emails (required for --type email; rejected for --type sms)" }; + private static readonly Option ListCreatePhonesOption = + new("--phones") { Description = "Comma-separated recipient phone numbers in E.164 (required for --type sms; rejected for --type email)" }; + private static readonly Option ListCreateTypeOption = CreateListTypeOption(); + + private static readonly Option ListUpdateIdOption = + new("--id") { Description = "Notification list ID", Required = true }; + private static readonly Option ListUpdateNameOption = + new("--name") { Description = "List name", Required = true }; + private static readonly Option ListUpdateEmailsOption = + new("--emails") { Description = "Comma-separated recipient emails (required for --type email; rejected for --type sms)" }; + private static readonly Option ListUpdatePhonesOption = + new("--phones") { Description = "Comma-separated recipient phone numbers in E.164 (required for --type sms; rejected for --type email)" }; + private static readonly Option ListUpdateTypeOption = CreateListTypeOption(); + + private static Option CreateListTypeOption() + { + var option = new Option("--type") + { + Description = "Delivery channel: email or sms (case-insensitive; default email)", + DefaultValueFactory = _ => "email", + }; + return option; + } + private static Command BuildGet(Option urlOption, Option formatOption, Option usernameOption, Option passwordOption) { var idOption = new Option("--id") { Description = "Notification list ID", Required = true }; @@ -44,27 +78,50 @@ public static class NotificationCommands private static Command BuildUpdate(Option urlOption, Option formatOption, Option usernameOption, Option passwordOption) { - var idOption = new Option("--id") { Description = "Notification list ID", Required = true }; - var nameOption = new Option("--name") { Description = "List name", Required = true }; - var emailsOption = new Option("--emails") { Description = "Comma-separated recipient emails", Required = true }; - var cmd = new Command("update") { Description = "Update a notification list" }; - cmd.Add(idOption); - cmd.Add(nameOption); - cmd.Add(emailsOption); + cmd.Add(ListUpdateIdOption); + cmd.Add(ListUpdateNameOption); + cmd.Add(ListUpdateTypeOption); + cmd.Add(ListUpdateEmailsOption); + cmd.Add(ListUpdatePhonesOption); cmd.SetAction(async (ParseResult result) => { - var id = result.GetValue(idOption); - var name = result.GetValue(nameOption)!; - var emailsRaw = result.GetValue(emailsOption)!; - var emails = emailsRaw.Split(',', StringSplitOptions.RemoveEmptyEntries | StringSplitOptions.TrimEntries).ToList(); + UpdateNotificationListCommand command; + try + { + command = BuildUpdateNotificationListCommand(result); + } + catch (ArgumentException ex) + { + OutputFormatter.WriteError(ex.Message, "INVALID_ARGUMENT"); + return 1; + } + return await CommandHelpers.ExecuteCommandAsync( - result, urlOption, formatOption, usernameOption, passwordOption, - new UpdateNotificationListCommand(id, name, emails)); + result, urlOption, formatOption, usernameOption, passwordOption, command); }); return cmd; } + /// + /// Builds the from a parsed + /// notification list update invocation, applying the channel-aware + /// recipient validation. Throws when the + /// channel and the supplied recipient flags are inconsistent. + /// + /// The parsed command-line result. + /// A validated . + internal static UpdateNotificationListCommand BuildUpdateNotificationListCommand(ParseResult result) + { + var id = result.GetValue(ListUpdateIdOption); + var name = result.GetValue(ListUpdateNameOption)!; + var (type, emails, phones) = ResolveListChannel( + result.GetValue(ListUpdateTypeOption)!, + result.GetValue(ListUpdateEmailsOption), + result.GetValue(ListUpdatePhonesOption)); + return new UpdateNotificationListCommand(id, name, emails, type, phones); + } + private static Command BuildSmtp(Option urlOption, Option formatOption, Option usernameOption, Option passwordOption) { var group = new Command("smtp") { Description = "Manage SMTP configuration" }; @@ -145,6 +202,80 @@ public static class NotificationCommands return new UpdateSmtpConfigCommand(id, server, port, authMode, from, tlsMode, credentials); } + private static Command BuildSms(Option urlOption, Option formatOption, Option usernameOption, Option passwordOption) + { + var group = new Command("sms") { Description = "Manage SMS (Twilio) configuration" }; + + var listCmd = new Command("list") { Description = "List SMS configurations (Auth Token shown as a presence flag only)" }; + listCmd.SetAction(async (ParseResult result) => + { + return await CommandHelpers.ExecuteCommandAsync( + result, urlOption, formatOption, usernameOption, passwordOption, new ListSmsConfigsCommand()); + }); + group.Add(listCmd); + + var updateCmd = new Command("update") { Description = "Update SMS configuration" }; + updateCmd.Add(SmsIdOption); + updateCmd.Add(SmsAccountSidOption); + updateCmd.Add(SmsFromNumberOption); + updateCmd.Add(SmsMessagingServiceSidOption); + updateCmd.Add(SmsApiBaseUrlOption); + updateCmd.Add(SmsAuthTokenOption); + updateCmd.SetAction(async (ParseResult result) => + { + return await CommandHelpers.ExecuteCommandAsync( + result, urlOption, formatOption, usernameOption, passwordOption, + BuildUpdateSmsConfigCommand(result)); + }); + group.Add(updateCmd); + + return group; + } + + // SMS update options are static so the parsed values can be read back both + // from the SetAction and from BuildUpdateSmsConfigCommand (used by tests). + private static readonly Option SmsIdOption = + new("--id") { Description = "SMS config ID", Required = true }; + private static readonly Option SmsAccountSidOption = + new("--account-sid") { Description = "Twilio Account SID", Required = true }; + private static readonly Option SmsFromNumberOption = + new("--from-number") { Description = "Sender phone number (E.164)", Required = true }; + private static readonly Option SmsMessagingServiceSidOption = + new("--messaging-service-sid") + { + Description = "Twilio Messaging Service SID (optional; OMITTING IT CLEARS the stored value)", + }; + private static readonly Option SmsApiBaseUrlOption = + new("--api-base-url") + { + Description = "API base URL override (optional; preserves existing if omitted)", + }; + private static readonly Option SmsAuthTokenOption = + new("--auth-token") + { + Description = "Twilio Auth Token (optional; PRESERVES the stored token if omitted; never printed back)", + }; + + /// + /// Builds the from a parsed sms update + /// invocation. Note the asymmetric preserve-vs-clear semantics enforced server-side: + /// omitting --auth-token / --api-base-url maps to null so the handler + /// PRESERVES the existing values, whereas omitting --messaging-service-sid also + /// maps to null but the handler OVERWRITES (clears) the stored value. + /// + /// The parsed command-line result from the sms update invocation. + /// An populated from the parsed result. + internal static UpdateSmsConfigCommand BuildUpdateSmsConfigCommand(ParseResult result) + { + var id = result.GetValue(SmsIdOption); + var accountSid = result.GetValue(SmsAccountSidOption)!; + var fromNumber = result.GetValue(SmsFromNumberOption)!; + var messagingServiceSid = result.GetValue(SmsMessagingServiceSidOption); + var apiBaseUrl = result.GetValue(SmsApiBaseUrlOption); + var authToken = result.GetValue(SmsAuthTokenOption); + return new UpdateSmsConfigCommand(id, accountSid, fromNumber, messagingServiceSid, apiBaseUrl, authToken); + } + private static Command BuildList(Option urlOption, Option formatOption, Option usernameOption, Option passwordOption) { var cmd = new Command("list") { Description = "List all notification lists" }; @@ -158,24 +289,103 @@ public static class NotificationCommands private static Command BuildCreate(Option urlOption, Option formatOption, Option usernameOption, Option passwordOption) { - var nameOption = new Option("--name") { Description = "Notification list name", Required = true }; - var emailsOption = new Option("--emails") { Description = "Comma-separated recipient emails", Required = true }; - var cmd = new Command("create") { Description = "Create a notification list" }; - cmd.Add(nameOption); - cmd.Add(emailsOption); + cmd.Add(ListCreateNameOption); + cmd.Add(ListCreateTypeOption); + cmd.Add(ListCreateEmailsOption); + cmd.Add(ListCreatePhonesOption); cmd.SetAction(async (ParseResult result) => { - var name = result.GetValue(nameOption)!; - var emailsRaw = result.GetValue(emailsOption)!; - var emails = emailsRaw.Split(',', StringSplitOptions.RemoveEmptyEntries | StringSplitOptions.TrimEntries).ToList(); + CreateNotificationListCommand command; + try + { + command = BuildCreateNotificationListCommand(result); + } + catch (ArgumentException ex) + { + OutputFormatter.WriteError(ex.Message, "INVALID_ARGUMENT"); + return 1; + } + return await CommandHelpers.ExecuteCommandAsync( - result, urlOption, formatOption, usernameOption, passwordOption, - new CreateNotificationListCommand(name, emails)); + result, urlOption, formatOption, usernameOption, passwordOption, command); }); return cmd; } + /// + /// Builds the from a parsed + /// notification list create invocation, applying the channel-aware + /// recipient validation. Throws when the + /// channel and the supplied recipient flags are inconsistent. + /// + /// The parsed command-line result. + /// A validated . + internal static CreateNotificationListCommand BuildCreateNotificationListCommand(ParseResult result) + { + var name = result.GetValue(ListCreateNameOption)!; + var (type, emails, phones) = ResolveListChannel( + result.GetValue(ListCreateTypeOption)!, + result.GetValue(ListCreateEmailsOption), + result.GetValue(ListCreatePhonesOption)); + return new CreateNotificationListCommand(name, emails, type, phones); + } + + /// + /// Parses the --type value (case-insensitive) and validates that the supplied + /// recipient flags match the channel: email requires --emails and rejects + /// --phones; sms requires --phones and rejects --emails. + /// + /// The raw --type value. + /// The raw --emails value, or null when omitted. + /// The raw --phones value, or null when omitted. + /// The resolved channel and recipient lists (the off-channel list is empty). + /// Thrown when the type is unknown or the recipient flags are inconsistent with the channel. + private static (NotificationType Type, IReadOnlyList Emails, IReadOnlyList? Phones) ResolveListChannel( + string typeRaw, string? emailsRaw, string? phonesRaw) + { + if (!Enum.TryParse(typeRaw, ignoreCase: true, out var type)) + { + throw new ArgumentException($"Invalid --type '{typeRaw}'. Expected 'email' or 'sms'."); + } + + var emails = SplitRecipients(emailsRaw); + var phones = SplitRecipients(phonesRaw); + + if (type == NotificationType.Email) + { + if (phones.Count > 0) + { + throw new ArgumentException("--phones is not valid for --type email; use --emails."); + } + + if (emails.Count == 0) + { + throw new ArgumentException("--emails is required for --type email."); + } + + return (type, emails, null); + } + + // Sms + if (emails.Count > 0) + { + throw new ArgumentException("--emails is not valid for --type sms; use --phones."); + } + + if (phones.Count == 0) + { + throw new ArgumentException("--phones is required for --type sms."); + } + + return (type, Array.Empty(), phones); + } + + private static IReadOnlyList SplitRecipients(string? raw) => + string.IsNullOrWhiteSpace(raw) + ? Array.Empty() + : raw.Split(',', StringSplitOptions.RemoveEmptyEntries | StringSplitOptions.TrimEntries).ToList(); + private static Command BuildDelete(Option urlOption, Option formatOption, Option usernameOption, Option passwordOption) { var idOption = new Option("--id") { Description = "Notification list ID", Required = true }; diff --git a/src/ZB.MOM.WW.ScadaBridge.CLI/README.md b/src/ZB.MOM.WW.ScadaBridge.CLI/README.md index a3f48fb1..3f1428af 100644 --- a/src/ZB.MOM.WW.ScadaBridge.CLI/README.md +++ b/src/ZB.MOM.WW.ScadaBridge.CLI/README.md @@ -985,31 +985,47 @@ scadabridge --url notification list #### `notification create` -Create a notification list with one or more recipients. +Create a notification list with one or more recipients. The `--type` flag selects the +delivery channel and decides which recipient flag is required: `email` (the default) +uses `--emails`; `sms` uses `--phones`. Supplying the wrong recipient flag for the +channel is rejected before the command is sent. ```sh +# Email list (default channel) scadabridge --url notification create --name --emails + +# SMS list +scadabridge --url notification create --name --type sms --phones ``` | Option | Required | Description | |--------|----------|-------------| | `--name` | yes | Notification list name | -| `--emails` | yes | Comma-separated list of recipient email addresses | +| `--type` | no | Delivery channel: `email` or `sms` (case-insensitive; default `email`) | +| `--emails` | conditional | Comma-separated recipient email addresses. **Required for `--type email`; rejected for `--type sms`.** | +| `--phones` | conditional | Comma-separated recipient phone numbers in E.164. **Required for `--type sms`; rejected for `--type email`.** | #### `notification update` Update a notification list. An update **replaces** the whole entity — every required -field below must be supplied, even if unchanged. +field below must be supplied, even if unchanged. As with `create`, `--type` selects the +channel and decides whether `--emails` or `--phones` is required. ```sh +# Email list scadabridge --url notification update --id --name --emails + +# SMS list +scadabridge --url notification update --id --name --type sms --phones ``` | Option | Required | Description | |--------|----------|-------------| | `--id` | yes | Notification list ID | | `--name` | yes | List name | -| `--emails` | yes | Comma-separated list of recipient email addresses | +| `--type` | no | Delivery channel: `email` or `sms` (case-insensitive; default `email`) | +| `--emails` | conditional | Comma-separated recipient email addresses. **Required for `--type email`; rejected for `--type sms`.** | +| `--phones` | conditional | Comma-separated recipient phone numbers in E.164. **Required for `--type sms`; rejected for `--type email`.** | #### `notification delete` @@ -1049,6 +1065,32 @@ scadabridge --url notification smtp update --id --server -- | `--tls-mode` | no | TLS mode: `None`, `StartTLS`, or `SSL` (preserves existing if omitted) | | `--credentials` | no | SMTP credentials — `username:password` for Basic, or client secret for OAuth2 (preserves existing if omitted) | +#### `notification sms list` + +Show the current SMS (Twilio) configuration. The Twilio **Auth Token is never returned** +— the listing reports it only as a `hasAuthToken` presence flag. + +```sh +scadabridge --url notification sms list +``` + +#### `notification sms update` + +Update the SMS (Twilio) configuration. + +```sh +scadabridge --url notification sms update --id --account-sid --from-number [--messaging-service-sid ] [--api-base-url ] [--auth-token ] +``` + +| Option | Required | Description | +|--------|----------|-------------| +| `--id` | yes | SMS config ID | +| `--account-sid` | yes | Twilio Account SID | +| `--from-number` | yes | Sender phone number (E.164) | +| `--messaging-service-sid` | no | Twilio Messaging Service SID. **Omitting it CLEARS the stored value** (the update overwrites it). | +| `--api-base-url` | no | API base URL override (preserves existing if omitted) | +| `--auth-token` | no | Twilio Auth Token. **Omitting it PRESERVES the stored token.** Never printed back. | + --- ### `security` — Security settings diff --git a/src/ZB.MOM.WW.ScadaBridge.Commons/Messages/Management/NotificationCommands.cs b/src/ZB.MOM.WW.ScadaBridge.Commons/Messages/Management/NotificationCommands.cs index 653e3f9d..f7fb2452 100644 --- a/src/ZB.MOM.WW.ScadaBridge.Commons/Messages/Management/NotificationCommands.cs +++ b/src/ZB.MOM.WW.ScadaBridge.Commons/Messages/Management/NotificationCommands.cs @@ -4,8 +4,8 @@ namespace ZB.MOM.WW.ScadaBridge.Commons.Messages.Management; public record ListNotificationListsCommand; public record GetNotificationListCommand(int NotificationListId); -public record CreateNotificationListCommand(string Name, IReadOnlyList RecipientEmails, NotificationType Type = NotificationType.Email); -public record UpdateNotificationListCommand(int NotificationListId, string Name, IReadOnlyList RecipientEmails, NotificationType Type = NotificationType.Email); +public record CreateNotificationListCommand(string Name, IReadOnlyList RecipientEmails, NotificationType Type = NotificationType.Email, IReadOnlyList? RecipientPhones = null); +public record UpdateNotificationListCommand(int NotificationListId, string Name, IReadOnlyList RecipientEmails, NotificationType Type = NotificationType.Email, IReadOnlyList? RecipientPhones = null); public record DeleteNotificationListCommand(int NotificationListId); public record ListSmtpConfigsCommand; public record UpdateSmtpConfigCommand(int SmtpConfigId, string Server, int Port, string AuthMode, string FromAddress, string? TlsMode = null, string? Credentials = null); diff --git a/src/ZB.MOM.WW.ScadaBridge.ManagementService/ManagementActor.cs b/src/ZB.MOM.WW.ScadaBridge.ManagementService/ManagementActor.cs index 8b7b5b12..9cdec411 100644 --- a/src/ZB.MOM.WW.ScadaBridge.ManagementService/ManagementActor.cs +++ b/src/ZB.MOM.WW.ScadaBridge.ManagementService/ManagementActor.cs @@ -1686,9 +1686,9 @@ public class ManagementActor : ReceiveActor { var repo = sp.GetRequiredService(); var list = new NotificationList(cmd.Name) { Type = cmd.Type }; - foreach (var email in cmd.RecipientEmails) + foreach (var recipient in BuildRecipients(cmd.Type, cmd.RecipientEmails, cmd.RecipientPhones)) { - list.Recipients.Add(new NotificationRecipient(email, email)); + list.Recipients.Add(recipient); } await repo.AddNotificationListAsync(list); await repo.SaveChangesAsync(); @@ -1710,12 +1710,10 @@ public class ManagementActor : ReceiveActor await repo.DeleteRecipientAsync(r.Id); } - foreach (var email in cmd.RecipientEmails) + foreach (var recipient in BuildRecipients(cmd.Type, cmd.RecipientEmails, cmd.RecipientPhones)) { - await repo.AddRecipientAsync(new NotificationRecipient(email, email) - { - NotificationListId = cmd.NotificationListId - }); + recipient.NotificationListId = cmd.NotificationListId; + await repo.AddRecipientAsync(recipient); } await repo.UpdateNotificationListAsync(list); @@ -1733,6 +1731,33 @@ public class ManagementActor : ReceiveActor return true; } + /// + /// SMS Notifications (S6): build the recipient set for a notification list according + /// to its delivery channel. Email lists map each + /// entry to an recipient; SMS lists map + /// each entry to an + /// recipient. The off-channel source list is ignored so an Email list never stores a + /// phone in EmailAddress (and vice versa). + /// + private static IEnumerable BuildRecipients( + NotificationType type, IReadOnlyList recipientEmails, IReadOnlyList? recipientPhones) + { + if (type == NotificationType.Sms) + { + foreach (var phone in recipientPhones ?? Array.Empty()) + { + yield return NotificationRecipient.ForSms(phone, phone); + } + } + else + { + foreach (var email in recipientEmails) + { + yield return NotificationRecipient.ForEmail(email, email); + } + } + } + /// /// MgmtSvc-020: project an SmtpConfiguration to a credential-free shape so the /// stored Credentials (SMTP password / OAuth2 client secret) never leaves this diff --git a/tests/ZB.MOM.WW.ScadaBridge.CLI.Tests/Commands/NotificationSmsCommandTests.cs b/tests/ZB.MOM.WW.ScadaBridge.CLI.Tests/Commands/NotificationSmsCommandTests.cs new file mode 100644 index 00000000..8dd7a187 --- /dev/null +++ b/tests/ZB.MOM.WW.ScadaBridge.CLI.Tests/Commands/NotificationSmsCommandTests.cs @@ -0,0 +1,273 @@ +using System.CommandLine; +using ZB.MOM.WW.ScadaBridge.CLI; +using ZB.MOM.WW.ScadaBridge.CLI.Commands; +using ZB.MOM.WW.ScadaBridge.Commons.Messages.Management; +using ZB.MOM.WW.ScadaBridge.Commons.Types.Enums; + +namespace ZB.MOM.WW.ScadaBridge.CLI.Tests.Commands; + +/// +/// Tests for the SMS surface of the notification command group (SMS Notifications S6): +/// the channel-aware --type / --phones flags on notification list create|update, +/// and the new notification sms list|update group. Pins the validation rules, the +/// per-channel command construction, and that the Twilio Auth Token is never echoed back. +/// +public class NotificationSmsCommandTests +{ + private static readonly Option Url = new("--url") { Recursive = true }; + private static readonly Option Username = new("--username") { Recursive = true }; + private static readonly Option Password = new("--password") { Recursive = true }; + private static readonly Option Format = CliOptions.CreateFormatOption(); + + private static Command Notification() => NotificationCommands.Build(Url, Format, Username, Password); + + private static Command ListCreate() => + Notification().Subcommands.Single(c => c.Name == "create"); + + private static Command ListUpdate() => + Notification().Subcommands.Single(c => c.Name == "update"); + + private static Command SmsGroup() => + Notification().Subcommands.Single(c => c.Name == "sms"); + + private static Command SmsUpdate() => + SmsGroup().Subcommands.Single(c => c.Name == "update"); + + // System.CommandLine's Parse takes a string[] (no params overload here), so route + // every invocation through a helper that wraps the variadic args into an array. + private static System.CommandLine.ParseResult Parse(Command command, params string[] args) + => command.Parse(args); + + // ---- list create/update: --type + --phones ---------------------------- + + [Fact] + public void ListCreate_SmsTypeWithPhones_BuildsSmsCommandWithRecipientPhones() + { + var parse = Parse(ListCreate(), + "--name", "Ops SMS", "--type", "sms", "--phones", "+15551230000,+15551230001"); + + Assert.Empty(parse.Errors); + var cmd = NotificationCommands.BuildCreateNotificationListCommand(parse); + + Assert.Equal("Ops SMS", cmd.Name); + Assert.Equal(NotificationType.Sms, cmd.Type); + Assert.Equal(new[] { "+15551230000", "+15551230001" }, cmd.RecipientPhones); + Assert.Empty(cmd.RecipientEmails); + } + + [Fact] + public void ListCreate_TypeIsCaseInsensitive() + { + var parse = Parse(ListCreate(), + "--name", "Ops SMS", "--type", "SMS", "--phones", "+15551230000"); + + Assert.Empty(parse.Errors); + var cmd = NotificationCommands.BuildCreateNotificationListCommand(parse); + Assert.Equal(NotificationType.Sms, cmd.Type); + } + + [Fact] + public void ListCreate_DefaultType_IsEmail() + { + var parse = Parse(ListCreate(), "--name", "Ops", "--emails", "ops@example.com"); + + Assert.Empty(parse.Errors); + var cmd = NotificationCommands.BuildCreateNotificationListCommand(parse); + + Assert.Equal(NotificationType.Email, cmd.Type); + Assert.Equal(new[] { "ops@example.com" }, cmd.RecipientEmails); + Assert.Null(cmd.RecipientPhones); + } + + [Fact] + public void ListCreate_EmailTypeWithPhones_Throws() + { + var parse = Parse(ListCreate(), + "--name", "Ops", "--type", "email", "--emails", "ops@example.com", "--phones", "+15551230000"); + + var ex = Assert.Throws( + () => NotificationCommands.BuildCreateNotificationListCommand(parse)); + Assert.Contains("--phones", ex.Message); + } + + [Fact] + public void ListCreate_SmsTypeWithoutPhones_Throws() + { + var parse = Parse(ListCreate(), "--name", "Ops SMS", "--type", "sms"); + + var ex = Assert.Throws( + () => NotificationCommands.BuildCreateNotificationListCommand(parse)); + Assert.Contains("--phones", ex.Message); + } + + [Fact] + public void ListCreate_EmailTypeWithoutEmails_Throws() + { + var parse = Parse(ListCreate(), "--name", "Ops", "--type", "email"); + + var ex = Assert.Throws( + () => NotificationCommands.BuildCreateNotificationListCommand(parse)); + Assert.Contains("--emails", ex.Message); + } + + [Fact] + public void ListCreate_SmsTypeWithEmails_Throws() + { + var parse = Parse(ListCreate(), + "--name", "Ops SMS", "--type", "sms", "--emails", "ops@example.com"); + + var ex = Assert.Throws( + () => NotificationCommands.BuildCreateNotificationListCommand(parse)); + Assert.Contains("--emails", ex.Message); + } + + [Fact] + public void ListCreate_UnknownType_Throws() + { + var parse = Parse(ListCreate(), + "--name", "Ops", "--type", "carrier-pigeon", "--emails", "ops@example.com"); + + Assert.Throws( + () => NotificationCommands.BuildCreateNotificationListCommand(parse)); + } + + [Fact] + public void ListUpdate_SmsTypeWithPhones_BuildsSmsCommandWithRecipientPhones() + { + var parse = Parse(ListUpdate(), + "--id", "7", "--name", "Ops SMS", "--type", "sms", "--phones", "+15551230000"); + + Assert.Empty(parse.Errors); + var cmd = NotificationCommands.BuildUpdateNotificationListCommand(parse); + + Assert.Equal(7, cmd.NotificationListId); + Assert.Equal(NotificationType.Sms, cmd.Type); + Assert.Equal(new[] { "+15551230000" }, cmd.RecipientPhones); + Assert.Empty(cmd.RecipientEmails); + } + + [Fact] + public void ListUpdate_EmailTypeWithPhones_Throws() + { + var parse = Parse(ListUpdate(), + "--id", "7", "--name", "Ops", "--type", "email", + "--emails", "ops@example.com", "--phones", "+15551230000"); + + var ex = Assert.Throws( + () => NotificationCommands.BuildUpdateNotificationListCommand(parse)); + Assert.Contains("--phones", ex.Message); + } + + // ---- notification sms group ------------------------------------------- + + [Fact] + public void Notification_HasSmsGroupWithListAndUpdate() + { + var subNames = SmsGroup().Subcommands.Select(c => c.Name).ToHashSet(); + Assert.Contains("list", subNames); + Assert.Contains("update", subNames); + } + + [Fact] + public void SmsUpdate_WithAllFlags_ProducesCommandCarryingThem() + { + var parse = Parse(SmsUpdate(), + "--id", "1", "--account-sid", "ACnew", "--from-number", "+15551110000", + "--messaging-service-sid", "MGnew", "--api-base-url", "https://new.example.com", + "--auth-token", "new-secret"); + + Assert.Empty(parse.Errors); + var cmd = NotificationCommands.BuildUpdateSmsConfigCommand(parse); + + Assert.Equal(1, cmd.SmsConfigId); + Assert.Equal("ACnew", cmd.AccountSid); + Assert.Equal("+15551110000", cmd.FromNumber); + Assert.Equal("MGnew", cmd.MessagingServiceSid); + Assert.Equal("https://new.example.com", cmd.ApiBaseUrl); + Assert.Equal("new-secret", cmd.AuthToken); + } + + [Fact] + public void SmsUpdate_WithoutOptionalFlags_ProducesCommandWithNulls() + { + var parse = Parse(SmsUpdate(), + "--id", "2", "--account-sid", "AConly", "--from-number", "+15552220000"); + + Assert.Empty(parse.Errors); + var cmd = NotificationCommands.BuildUpdateSmsConfigCommand(parse); + + Assert.Equal(2, cmd.SmsConfigId); + // messaging-service-sid maps to null (handler CLEARS it); auth-token / api-base-url + // map to null (handler PRESERVES them) — all three optional here. + Assert.Null(cmd.MessagingServiceSid); + Assert.Null(cmd.ApiBaseUrl); + Assert.Null(cmd.AuthToken); + } + + [Fact] + public void SmsUpdate_OptionalFlags_AreNotRequired() + { + var update = SmsUpdate(); + Assert.False(update.Options.Single(o => o.Name == "--messaging-service-sid").Required); + Assert.False(update.Options.Single(o => o.Name == "--api-base-url").Required); + Assert.False(update.Options.Single(o => o.Name == "--auth-token").Required); + } + + [Fact] + public void SmsCommands_ResolveViaRegistry() + { + // The CLI calls GetCommandName for every command it sends; both SMS commands + // must round-trip through the management command registry. + Assert.Equal(typeof(ListSmsConfigsCommand), + ManagementCommandRegistry.Resolve(ManagementCommandRegistry.GetCommandName(typeof(ListSmsConfigsCommand)))); + Assert.Equal(typeof(UpdateSmsConfigCommand), + ManagementCommandRegistry.Resolve(ManagementCommandRegistry.GetCommandName(typeof(UpdateSmsConfigCommand)))); + } + + // ---- Auth Token is never rendered ------------------------------------- +} + +/// +/// Console-capturing companion to — pins that the +/// Twilio Auth Token is never echoed to the rendered output. Lives in the shared "Console" +/// collection so the redirection does not race the other +/// console-capturing test classes. +/// +[Collection("Console")] +public class NotificationSmsAuthTokenRenderingTests +{ + [Fact] + public void SmsConfigListResponse_RenderedAsJson_DoesNotContainAuthToken() + { + // The server projects the AuthToken away to a HasAuthToken presence flag + // (SmsConfigPublicShape). The CLI renders that projected response verbatim, + // so a rendered SMS-config list never surfaces the secret token value. + var projected = new[] + { + new + { + Id = 1, + AccountSid = "ACxxxx", + FromNumber = "+15550000000", + HasAuthToken = true, + }, + }; + + var original = Console.Out; + using var sw = new StringWriter(); + try + { + Console.SetOut(sw); + OutputFormatter.WriteJson(projected); + } + finally + { + Console.SetOut(original); + } + + var rendered = sw.ToString(); + Assert.Contains("hasAuthToken", rendered); + Assert.DoesNotContain("authToken\"", rendered); + Assert.DoesNotContain("secret", rendered, StringComparison.OrdinalIgnoreCase); + } +} diff --git a/tests/ZB.MOM.WW.ScadaBridge.CLI.Tests/UpdateCommandContractTests.cs b/tests/ZB.MOM.WW.ScadaBridge.CLI.Tests/UpdateCommandContractTests.cs index 9ec144b5..289d32a9 100644 --- a/tests/ZB.MOM.WW.ScadaBridge.CLI.Tests/UpdateCommandContractTests.cs +++ b/tests/ZB.MOM.WW.ScadaBridge.CLI.Tests/UpdateCommandContractTests.cs @@ -67,7 +67,21 @@ public class UpdateCommandContractTests [Fact] public void NotificationUpdate_CoreFieldsRequired() - => AssertRequired(UpdateCommand(NotificationCommands.Build(Url, Format, Username, Password), "update"), "--name", "--emails"); + { + // Only --name is unconditionally required. The recipient flags are channel- + // conditional (SMS Notifications S6): --emails is required for --type email and + // --phones for --type sms, so neither can be flagged Required at the option level + // — the create/update builders validate the channel/recipient pairing instead. + var update = UpdateCommand(NotificationCommands.Build(Url, Format, Username, Password), "update"); + AssertRequired(update, "--name"); + + foreach (var name in new[] { "--emails", "--phones" }) + { + var option = update.Options.SingleOrDefault(o => o.Name == name); + Assert.True(option != null, $"notification update is missing option '{name}'."); + Assert.False(option!.Required, $"notification update option '{name}' must be conditionally validated, not Required."); + } + } [Fact] public void ApiMethodUpdate_CoreFieldsRequired() diff --git a/tests/ZB.MOM.WW.ScadaBridge.ManagementService.Tests/ManagementActorTests.cs b/tests/ZB.MOM.WW.ScadaBridge.ManagementService.Tests/ManagementActorTests.cs index ab0d1c76..3cfc4cea 100644 --- a/tests/ZB.MOM.WW.ScadaBridge.ManagementService.Tests/ManagementActorTests.cs +++ b/tests/ZB.MOM.WW.ScadaBridge.ManagementService.Tests/ManagementActorTests.cs @@ -1365,6 +1365,43 @@ public class ManagementActorTests : TestKit, IDisposable Assert.Equal(Commons.Types.Enums.NotificationType.Sms, added!.Type); } + [Fact] + public void CreateNotificationList_WithSmsTypeAndPhones_PersistsForSmsRecipients() + { + // S6: an SMS list builds recipients from RecipientPhones via NotificationRecipient.ForSms + // — PhoneNumber set, EmailAddress null (the off-channel RecipientEmails list is ignored). + var notifRepo = Substitute.For(); + Commons.Entities.Notifications.NotificationList? added = null; + notifRepo.When(r => r.AddNotificationListAsync( + Arg.Any(), Arg.Any())) + .Do(ci => added = ci.Arg()); + _services.AddScoped(_ => notifRepo); + + var actor = CreateActor(); + var envelope = Envelope( + new CreateNotificationListCommand( + "Ops SMS", + Array.Empty(), + Commons.Types.Enums.NotificationType.Sms, + new[] { "+15551230000", "+15551230001" }), + "Designer"); + + actor.Tell(envelope); + + var response = ExpectMsg(TimeSpan.FromSeconds(5)); + Assert.Equal(envelope.CorrelationId, response.CorrelationId); + Assert.NotNull(added); + Assert.Equal(Commons.Types.Enums.NotificationType.Sms, added!.Type); + Assert.Equal(2, added.Recipients.Count); + Assert.All(added.Recipients, r => + { + Assert.Null(r.EmailAddress); + Assert.NotNull(r.PhoneNumber); + }); + Assert.Equal(new[] { "+15551230000", "+15551230001" }, + added.Recipients.Select(r => r.PhoneNumber).ToArray()); + } + [Fact] public void CreateNotificationList_DefaultType_IsEmail() {