Fix Notification Service (#2187)

bcgov · Aug 16, 2024 · d25f98c · d25f98c
1 parent 3ccfec5
commit d25f98c
Show file tree

Hide file tree

Showing 5 changed files with 22 additions and 17 deletions.
diff --git a/api/net/Areas/Subscriber/Controllers/WorkOrderController.cs b/api/net/Areas/Subscriber/Controllers/WorkOrderController.cs
@@ -183,7 +183,7 @@ public async Task<IActionResult> RequestAnonymousTranscriptionAsync(long content
         if (content.Source?.DisableTranscribe == true) return BadRequest("Cannot request transcription");
         if (content.IsApproved || content.ContentType != Entities.ContentType.AudioVideo || !content.FileReferences.Any())
         {
-            // The transcript has already been approved, do not allow new requests.
+            // The transcript has already been approved, or this request is invalid, do not allow new requests.
             var result = new
             {
                 StatusCode = HttpStatusCode.OK,

diff --git a/app/subscriber/src/features/content/view-content/ViewContent.tsx b/app/subscriber/src/features/content/view-content/ViewContent.tsx
@@ -385,7 +385,8 @@ export const ViewContent: React.FC<IViewContentProps> = ({ setActiveContent, act
               isAV &&
               !content?.source?.disableTranscribe &&
               !content.isApproved &&
-              !isTranscriptRequestor
+              !isTranscriptRequestor &&
+              !!content?.fileReferences.length
             }
           >
             <Button

diff --git a/libs/net/dal/Services/NotificationService.cs b/libs/net/dal/Services/NotificationService.cs
@@ -69,6 +69,8 @@ public IEnumerable<Notification> Find(NotificationFilter filter)
             query = query.Where(n => n.OwnerId == filter.OwnerId.Value);
         if (filter.SubscriberUserId.HasValue)
             query = query.Where(n => n.SubscribersManyToMany.Any(ns => ns.IsSubscribed && ns.UserId == filter.SubscriberUserId.Value));
+        if (filter.IsEnabled.HasValue)
+            query = query.Where(n => n.IsEnabled == filter.IsEnabled);
 
         if (filter.Sort?.Any() == true)
         {

diff --git a/libs/net/models/Filters/NotificationFilter.cs b/libs/net/models/Filters/NotificationFilter.cs
@@ -29,6 +29,10 @@ public class NotificationFilter : PageFilter
     /// get/set - Only include content owned by this user.
     /// </summary>
     public int? SubscriberUserId { get; set; }
+
+    /// <summary>
+    /// get/set - Only include notifications that are enabled.
+    public bool? IsEnabled { get; set; }
     #endregion
 
     #region Constructors
@@ -42,6 +46,7 @@ public NotificationFilter(Dictionary<string, StringValues> queryParams) : base(q
         this.NotificationType = filter.GetEnumNullValue<NotificationType>(nameof(this.NotificationType));
         this.OwnerId = filter.GetIntNullValue(nameof(this.OwnerId));
         this.SubscriberUserId = filter.GetIntNullValue(nameof(this.SubscriberUserId));
+        this.IsEnabled = filter.GetBoolNullValue(nameof(this.IsEnabled));
     }
     #endregion
 }
diff --git a/services/net/notification/NotificationManager.cs b/services/net/notification/NotificationManager.cs
@@ -290,28 +290,24 @@ private async Task ProcessNotificationAsync(ConsumeResult<string, NotificationRe
                     if (request.NotificationId.HasValue)
                     {
                         var notification = await this.Api.GetNotificationAsync(request.NotificationId.Value);
-                        if (notification != null)
+                        if (notification != null && notification.IsEnabled)
                         {
-                            if (notification.IsEnabled)
-                            {
-                                this.NotificationValidator.InitializeNotification(notification);
-                                if (request.IgnoreValidation || await this.NotificationValidator.ConfirmSendAsync())
-                                    await SendNotificationAsync(request, notification, content);
-                                else
-                                    this.Logger.LogInformation("Notification not sent.  Notification: {notification}, Content ID: {contentId}", notification.Id, content.Id);
-                            }
+                            this.NotificationValidator.InitializeNotification(notification);
+                            if (request.IgnoreValidation || await this.NotificationValidator.ConfirmSendAsync())
+                                await SendNotificationAsync(request, notification, content);
                             else
-                                this.Logger.LogDebug("Notification is disabled.  Notification: {notification}", notification.Id);
+                                this.Logger.LogInformation("Notification not sent.  Notification: {notification}, Content ID: {contentId}", notification.Id, content.Id);
                         }
                         else
-                            this.Logger.LogDebug("Notification does not exist.  Notification: {notification}", request.NotificationId);
+                            this.Logger.LogDebug("Notification does not exist or is disabled.  Notification: {notification}", request.NotificationId);
                     }
                     else
                     {
                         // Only fetch notifications that are configured to alert on index.
                         var notifications = await this.Api.FindNotificationsAsync(new TNO.Models.Filters.NotificationFilter()
                         {
-                            AlertOnIndex = true
+                            AlertOnIndex = true,
+                            IsEnabled = true,
                         });
                         foreach (var notification in notifications)
                         {
@@ -411,7 +407,8 @@ private async Task ResetAlertAsync(API.Areas.Services.Models.Content.ContentMode
         subscribers.AddRange(content.UserNotifications.Where(cun => cun.User != null && cun.IsSubscribed && cun.User.AccountType != UserAccountType.Distribution)
             .Select(cun => new API.Areas.Services.Models.Notification.UserModel(cun.User!)));
 
-        return subscribers;
+        // Remove duplicate subscribers.  This can occur if a user is both subscribed to the content and the notification.
+        return subscribers.GroupBy(s => s.Id).Select(s => s.First());
     }
 
     /// <summary>
@@ -431,7 +428,7 @@ private async Task SendNotificationAsync(NotificationRequestModel request, API.A
         var contexts = new List<EmailContextModel>();
         if (!String.IsNullOrWhiteSpace(request.To))
         {
-            // Add a context for the requested list of users in addition to the subscribers.
+            // When a notification request has specified 'To' it means only send it to the emails in that property.
             var requestTo = request.To.Split(",").Select(v => v.Trim());
             contexts.Add(new EmailContextModel(requestTo, new Dictionary<string, object>(), DateTime.Now));
         }
@@ -443,7 +440,7 @@ private async Task SendNotificationAsync(NotificationRequestModel request, API.A
         // There are no subscribers, or a notification has been sent for this content to all the subscribers.
         if (!contexts.Any())
         {
-            if (!notification.Subscribers.Any(s => s.IsSubscribed))
+            if (!subscribers.Any())
                 this.Logger.LogInformation("Notification '{name}' does not have subscribers.", notification.Name);
             else
                 this.Logger.LogInformation("Notification '{name}' is not sent because all users have already received this content.", notification.Name);