Content 'Send to publish' notifications doesn't work for more root nodes

[ricbrady]

Which exact Umbraco version are you using? For example: 9.0.1 - don't just write v9

9.4.1

Bug summary

I would like to set the notification for the 'send to publish' action.

In my Umbraco I have 2 Root nodes and I've configured the notifications for both.

The problem is that I receive the notifications only for the nodes that are under the first root node where I've configured the subscription. No way to receive something for the second.

Specifics

No response

Steps to reproduce

Subscrive to receive notifications for 'SendToPublish' action for 2 root nodes contents and try to send some content to be published.
You will receive the notification only for the nodes where you subribe first

Expected result / actual result

Receive notification for all configured nodes

---

This item has been added to our backlog AB#26479

[nathanwoulfe]

Hey @ricbrady, I've had a quick look, this issue applies to all notifications - I've subscribed to publish notifications and confirm that they only send for the first root node. Will see if I can find why, but this certainly looks like something needing attention.

[nathanwoulfe]

Looks like the problem is in this block, particularly the check at L113:

Umbraco-CMS/src/Umbraco.Infrastructure/Services/Implement/NotificationService.cs

Lines 97 to 130 in 5bfab13

	var users = _userService.GetNextUsers(id, pagesz).Where(x => x.IsApproved).ToList();
	var notifications = GetUsersNotifications(users.Select(x => x.Id), action, Enumerable.Empty<int>(), Cms.Core.Constants.ObjectTypes.Document).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, siteUri, 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);

Because notifications are inherited, it's assumed the first notification in the set can be used to determine if the notifications apply to the current node (by checking the notification id against the current entity's path). If the notification id is in the entity path, send the email.

The issue with sibling root nodes is because the first notification id will be the first root node id, so will never match on the sibling nodes. This also means notifications will only work for the first root node with notifications enabled (that might be the second, third node), since the notifications are treated as representing a path to a node, not siblings.

Passing the entity ids to GetUsersNotifications (L98) fixes the problem for the root nodes, but breaks notifications for nested nodes... Maybe needs some additional logic to check the node is at the root, if so send the ids, otherwise send the empty int enumerable. Definitely needs proper investigation, but I think this is where the problem originates.

[ricbrady]

Any news?

[kjac]

Hi @ricbrady,

Sorry for the belated response here.

I can reproduce this in V10+, so this definitively is an ongoing issue.

I have created a PR that fixes it for V10+ in #13805

[nikolajlauridsen]

Fixed in #13805, thanks for reporting 😄