From 01a895bbf75006d442a590e5386a9f0de18a42f2 Mon Sep 17 00:00:00 2001 From: Peter Keating Date: Thu, 28 Feb 2019 16:43:22 +0000 Subject: [PATCH] Fix notifications not sending when user has multiple of same type When a user has multiple notifications of the same type, the user may not receive the notification depending on the order which the notification was setup. To fix this the notification logic has changed from looping over users to looping over notifications. Fixes #4713. --- .../Services/NotificationService.cs | 78 +++++++++++-------- 1 file changed, 45 insertions(+), 33 deletions(-) diff --git a/src/Umbraco.Core/Services/NotificationService.cs b/src/Umbraco.Core/Services/NotificationService.cs index 07d6e4e9e5..ccf9b9da00 100644 --- a/src/Umbraco.Core/Services/NotificationService.cs +++ b/src/Umbraco.Core/Services/NotificationService.cs @@ -124,40 +124,52 @@ namespace Umbraco.Core.Services var notifications = GetUsersNotifications(users.Select(x => x.Id), action, Enumerable.Empty(), Constants.ObjectTypes.DocumentGuid).ToList(); if (notifications.Count == 0) break; - var i = 0; - foreach (var user in users) - { - // continue if there's no notification for this user - if (notifications[i].UserId != user.Id) continue; // next user - - for (var j = 0; j < entitiesL.Count; j++) - { - var content = entitiesL[j]; - var path = paths[j]; - - // test if the notification applies to the path ie to this entity - if (path.Contains(notifications[i].EntityId) == false) continue; // next entity - - if (prevVersionDictionary.ContainsKey(content.Id) == false) - { - prevVersionDictionary[content.Id] = GetPreviousVersion(content.Id); - } - - // queue notification - var req = CreateNotificationRequest(operatingUser, user, content, prevVersionDictionary[content.Id], actionName, http, createSubject, createBody); - Enqueue(req); - } - - // skip other notifications for this user, essentially this means moving i to the next index of notifications - // for the next user. - do - { - i++; - } while (i < notifications.Count && notifications[i].UserId == user.Id); - - if (i >= notifications.Count) break; // break if no more notifications - } + while (notifications.Count > 0) + { + var notification = notifications[0]; + var isMatched = false; + // grab user whose associated to the notification + var user = users.Where(x => x.Id == notification.UserId).FirstOrDefault(); + + if (user == null) + { + notifications.RemoveAll(x => x.UserId == notification.UserId); + } + + for (var j = 0; j < entitiesL.Count; j++) + { + var content = entitiesL[j]; + var path = paths[j]; + + // test if the notification applies to the path ie to this entity + if (path.Contains(notification.EntityId) == false) continue; // next entity + + isMatched = true; + + if (prevVersionDictionary.ContainsKey(content.Id) == false) + { + prevVersionDictionary[content.Id] = GetPreviousVersion(content.Id); + } + + // queue notification + var req = CreateNotificationRequest(operatingUser, user, content, prevVersionDictionary[content.Id], actionName, http, createSubject, createBody); + Enqueue(req); + + // don't process any further entities as a notification has been sent + break; + } + + // when a match has been found, skip other notifications for user. + if (isMatched) + { + notifications.RemoveAll(x => x.UserId == notification.UserId); + continue; + } + + notifications.Remove(notification); + } + // load more users if any id = users.Count == pagesz ? users.Last().Id + 1 : -1;