Skip to content

GH Issue 1632: Stream unread notifications for the notification panel #8190

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
cnathe merged 5 commits into from
Oct 21, 2026
Merged

GH Issue 1632: Stream unread notifications for the notification panel #8190

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
c0c6018
GH Issue 1632: Stream unread notifications for the notification panel
cnathe Oct 16, 2026
682ddd4
fix comment spacing
cnathe Oct 16, 2026
b439c59
Merge remote-tracking branch 'origin/develop' into fb_notification_la…
cnathe Oct 16, 2026
0fb3975
remove unused NotificationService methods
cnathe Oct 16, 2026
7d5a67e
Collapse notification list lookups into a single streaming forEachNot…
cnathe Oct 16, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
27 changes: 8 additions & 19 deletions api/src/org/labkey/api/admin/notification/NotificationService.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,7 @@

import java.io.IOException;
import java.util.List;
import java.util.function.Consumer;

/**
* Service for adding/getting/removing user notifications attached to specific objects.
Expand Down Expand Up @@ -62,34 +63,22 @@ Notification sendMessageForRecipient(Container c, User createdByUser, User recip
ActionURL linkUrl, String id, String type) throws MessagingException, ValidationException, IOException;

/*
* Returns a list of notifications for a specific user. Sorted descending by created date.
* Streams notifications for a specific user to the consumer without collecting them, optionally limited to the
* given type labels (e.g, 'Pipeline'). Sorted descending by created date.
*/
default List<Notification> getNotificationsByUser(Container container, int notifyUserId, boolean unreadOnly)
{
return getNotificationsByUser(container, notifyUserId, unreadOnly, null);
}
void forEachNotification(@Nullable Container container, int notifyUserId, @Nullable List<String> typeLabels, boolean unreadOnly, @Nullable ContainerFilter containerFilter, Consumer<Notification> consumer);

List<Notification> getNotificationsByUser(Container container, int notifyUserId, boolean unreadOnly, @Nullable ContainerFilter containerFilter);
/*
* Returns just the count of notifications for a specific user. At any given instant, will match the length of the
* list returned by getNotificationsByUser(), and is significantly more efficient to query.
* Returns just the count of unread notifications for a specific user. At any given instant, will match the number
* of unread notifications streamed by forEachNotification().
*/
long getUnreadNotificationCountByUser(@Nullable Container container, int notifyUserId);

/*
* Returns a list of notifications for a specific user based on the specified type.
* Returns true if the user has at least one read notification.
*/
List<Notification> getNotificationsByType(Container container, @NotNull String type, int notifyUserId, boolean unreadOnly);

/*
* Returns a list of notifications for a specific user based on the specified type labels (e.g, 'Pipeline').
*/
default List<Notification> getNotificationsByTypeLabels(Container container, @NotNull List<String> typeLabels, int notifyUserId, boolean unreadOnly)
{
return getNotificationsByTypeLabels(container, typeLabels, notifyUserId, unreadOnly, null);
}
boolean hasReadNotificationsByUser(@Nullable Container container, int notifyUserId);

List<Notification> getNotificationsByTypeLabels(Container container, @NotNull List<String> typeLabels, int notifyUserId, boolean unreadOnly, @Nullable ContainerFilter containerFilter);
/*
* Returns a notification based on the notification's RowId.
*/
Expand Down
53 changes: 18 additions & 35 deletions core/src/org/labkey/core/notification/NotificationController.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@
package org.labkey.core.notification;

import org.apache.commons.lang3.StringUtils;
import org.apache.commons.lang3.mutable.MutableInt;
import org.json.JSONObject;
import org.labkey.api.action.ApiResponse;
import org.labkey.api.action.ApiSimpleResponse;
Expand Down Expand Up @@ -159,11 +160,9 @@ public ApiResponse execute(NotificationsForm form, BindException errors)
if (form.isByContainer() && !StringUtils.isEmpty(form.getContainerFilter()))
containerFilter = ContainerFilter.getContainerFilterByName(form.getContainerFilter(), getContainer(), getUser());

List<Notification> notifications;
if (form.getTypeLabels() == null || form.getTypeLabels().isEmpty())
notifications = service.getNotificationsByUser(notificationContainer, getUser().getUserId(), true, containerFilter);
else
notifications = service.getNotificationsByTypeLabels(notificationContainer, form.getTypeLabels(), getUser().getUserId(), true, containerFilter);
// TODO: replace with a single bulk UPDATE of ReadOn; this runs a select, update, and websocket event per unread notification
List<Notification> notifications = new ArrayList<>();
service.forEachNotification(notificationContainer, getUser().getUserId(), form.getTypeLabels(), true, containerFilter, notifications::add);

for (Notification notification : notifications)
{
Expand Down Expand Up @@ -297,33 +296,26 @@ public ApiResponse execute(NotificationsForm form, BindException errors)
if (form.isByContainer() && !StringUtils.isEmpty(form.getContainerFilter()))
containerFilter = ContainerFilter.getContainerFilterByName(form.getContainerFilter(), getContainer(), getUser());

List<Notification> notifications;
if (form.getTypeLabels() == null || form.getTypeLabels().isEmpty())
notifications = service.getNotificationsByUser(notificationContainer, getUser().getUserId(), false, containerFilter);
else
notifications = service.getNotificationsByTypeLabels(notificationContainer, form.getTypeLabels(), getUser().getUserId(), false, containerFilter);

int i = 0;
int unreadCount = 0;
for (Notification notification : notifications)
{
if (form.getMaxRows() == null || i < form.getMaxRows())
MutableInt totalRows = new MutableInt();
MutableInt unreadCount = new MutableInt();
service.forEachNotification(notificationContainer, getUser().getUserId(), form.getTypeLabels(), false, containerFilter, notification -> {
if (form.getMaxRows() == null || totalRows.intValue() < form.getMaxRows())
{
Map<String, Object> notifPropMap = notification.asPropMap();
notifPropMap.put("CreatedBy", UserManager.getDisplayName(asInteger(notifPropMap.get("CreatedBy")), getUser()));
notifPropMap.put("TypeLabel", service.getNotificationTypeLabel(notification.getType()));
notifPropMap.put("IconCls", service.getNotificationTypeIconCls(notification.getType()));
notificationList.add(notifPropMap);
}
i++;
totalRows.increment();
if (notification.getReadOn() == null)
unreadCount++;
}
unreadCount.increment();
});

ApiSimpleResponse response = new ApiSimpleResponse();
response.put("notifications", notificationList);
response.put("totalRows", notifications.size());
response.put("unreadCount", unreadCount);
response.put("totalRows", totalRows.intValue());
response.put("unreadCount", unreadCount.intValue());
response.put("success", true);
return response;
}
Expand Down Expand Up @@ -393,21 +385,14 @@ private JSONObject getNotificationJson(User user)
{
Map<Integer, Map<String, Object>> notificationsPropMap = new IntHashMap<>();
Map<String, List<Integer>> notificationGroupingsMap = new TreeMap<>();
int unreadCount = 0;
boolean hasRead = false;

NotificationService service = NotificationService.get();
if (service != null && user != null && !user.isGuest())
{
List<Notification> userNotifications = service.getNotificationsByUser(null, user.getUserId(), false, null);
for (Notification notification : userNotifications)
{
if (notification.getReadOn() != null)
{
hasRead = true;
continue;
}

// GH Issue 1632: load only unread rows; read history can include a large number of rows for a given user
hasRead = service.hasReadNotificationsByUser(null, user.getUserId());
service.forEachNotification(null, user.getUserId(), null, true, null, notification -> {
Map<String, Object> notifPropMap = notification.asPropMap();
notifPropMap.put("CreatedBy", UserManager.getDisplayName(asInteger(notifPropMap.get("CreatedBy")), user));
notifPropMap.put("IconCls", service.getNotificationTypeIconCls(notification.getType()));
Expand All @@ -419,14 +404,12 @@ private JSONObject getNotificationJson(User user)
notificationGroupingsMap.put(groupLabel, new ArrayList<>());
}
notificationGroupingsMap.get(groupLabel).add(notification.getRowId());

unreadCount++;
}
});
}

JSONObject notifications = new JSONObject(notificationsPropMap);
notifications.put("grouping", notificationGroupingsMap);
notifications.put("unreadCount", unreadCount);
notifications.put("unreadCount", notificationsPropMap.size());
notifications.put("hasRead", hasRead);
return notifications;
}
Expand Down
55 changes: 31 additions & 24 deletions core/src/org/labkey/core/notification/NotificationServiceImpl.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -64,6 +64,8 @@
import java.util.Map;
import java.util.concurrent.ConcurrentHashMap;
import java.util.concurrent.TimeUnit;
import java.util.function.Consumer;
import java.util.stream.Stream;

public class NotificationServiceImpl implements ContainerManager.ContainerListener, NotificationService
{
Expand Down Expand Up @@ -204,9 +206,20 @@ private void clearUnreadCache()
}

@Override
public List<Notification> getNotificationsByUser(Container container, int notifyUserId, boolean unreadOnly, @Nullable ContainerFilter containerFilter)
public void forEachNotification(@Nullable Container container, int notifyUserId, @Nullable List<String> typeLabels, boolean unreadOnly, @Nullable ContainerFilter containerFilter, Consumer<Notification> consumer)
{
return getNotificationsByUserOrType(container, null, notifyUserId, unreadOnly, containerFilter);
List<String> types = null;
if (typeLabels != null && !typeLabels.isEmpty())
{
types = new ArrayList<>();
for (String label : typeLabels)
types.addAll(_labelTypesMap.get(label));
}

try (Stream<Notification> stream = createSelectorByUserOrType(container, types, notifyUserId, unreadOnly, containerFilter).uncachedStream(Notification.class))
{
stream.forEach(consumer);
}
}

@Override
Expand All @@ -216,17 +229,12 @@ public long getUnreadNotificationCountByUser(@Nullable Container container, int
}

@Override
public List<Notification> getNotificationsByType(Container container, @NotNull String type, int notifyUserId, boolean unreadOnly)
{
return getNotificationsByUserOrType(container, Collections.singletonList(type), notifyUserId, unreadOnly, null);
}

@Override
public List<Notification> getNotificationsByTypeLabels(Container container, @NotNull List<String> typeLabels, int notifyUserId, boolean unreadOnly, @Nullable ContainerFilter containerFilter)
public boolean hasReadNotificationsByUser(@Nullable Container container, int notifyUserId)
{
List<String> types = new ArrayList<>();
typeLabels.forEach(label -> types.addAll(_labelTypesMap.get(label)));
return getNotificationsByUserOrType(container, types, notifyUserId, unreadOnly, containerFilter);
SimpleFilter filter = null == container ? new SimpleFilter() : SimpleFilter.createContainerFilter(container);
filter.addCondition(FieldKey.fromParts("UserID"), notifyUserId);
filter.addCondition(FieldKey.fromParts("ReadOn"), null, CompareType.NONBLANK);
return new TableSelector(getTable(), filter, null).exists();
}

private TableSelector createSelectorByUserOrType(Container container, @Nullable List<String> types, int notifyUserId, boolean unreadOnly, @Nullable ContainerFilter containerFilter)
Expand All @@ -248,11 +256,6 @@ else if (null != container)
return new TableSelector(getTable(), filter, sort);
}

private List<Notification> getNotificationsByUserOrType(Container container, @Nullable List<String> types, int notifyUserId, boolean unreadOnly, @Nullable ContainerFilter containerFilter)
{
return createSelectorByUserOrType(container, types, notifyUserId, unreadOnly, containerFilter).getArrayList(Notification.class);
}

@Override
public Notification getNotification(int rowId)
{
Expand Down Expand Up @@ -492,10 +495,12 @@ public void verifyInsert() throws ValidationException
verifyNotificationDetails(_notif1, _service.getNotification(_container, _notif1));
verifyNotificationDetails(_notif2, _service.getNotification(_container, "objectId2", "type2", _user.getUserId()));

// verify notification query by type
assertEquals("Unexpected number of notifications for type 'type1'", 1, _service.getNotificationsByType(_container, "type1", _user.getUserId(), false).size());
assertEquals("Unexpected number of notifications for type 'type2'", 1, _service.getNotificationsByType(_container, "type2", _user.getUserId(), false).size());
assertEquals("Unexpected number of notifications for type 'type3'", 0, _service.getNotificationsByType(_container, "type3", _user.getUserId(), false).size());
assertNull("Unexpected notification for type 'type3'", _service.getNotification(_container, "objectId1", "type3", _user.getUserId()));

// verify streaming notification query by user
List<Notification> streamed = new ArrayList<>();
_service.forEachNotification(_container, _user.getUserId(), null, false, null, streamed::add);
assertEquals("Unexpected number of streamed notifications", 2, streamed.size());
}

@Test
Expand All @@ -504,11 +509,13 @@ public void verifyMarkAsRead()
// no records updated when we give it a mis-matching objectId
int count = _service.markAsRead(_container, _user, "objectId0", Collections.singletonList("type1"), _user.getUserId());
assertEquals("Unexpected number of notifications updated", 0, count);
assertFalse("Unexpected read notification", _service.hasReadNotificationsByUser(_container, _user.getUserId()));

// mark one record as read and verify that it isn't deleted or changed in any other way
count = _service.markAsRead(_container, _user, "objectId1", Collections.singletonList("type1"), _user.getUserId());
assertEquals("Unexpected number of notifications updated", 1, count);
assertEquals("Unexpected number of unread notifications", 0, _service.getNotificationsByType(_container, "type1", _user.getUserId(), true).size());
assertNotNull("Expected notification to be marked as read", _service.getNotification(_container, _notif1).getReadOn());
assertTrue("Expected a read notification", _service.hasReadNotificationsByUser(_container, _user.getUserId()));
verifyNotificationDetails(_notif1, _service.getNotification(_container, _notif1));
}

Expand All @@ -519,10 +526,10 @@ public void verifyRemove()
int count = _service.removeNotifications(_container, "objectId0", Collections.singletonList("type2"), _user.getUserId());
assertEquals("Unexpected number of notifications removed", 0, count);

// remove a record and verify it has been deleted using the type query
// remove a record and verify it has been deleted
count = _service.removeNotifications(_container, "objectId2", Collections.singletonList("type2"), _user.getUserId());
assertEquals("Unexpected number of notifications removed", 1, count);
assertEquals("Unexpected number of notifications for type 'type2'", 0, _service.getNotificationsByType(_container, "type2", _user.getUserId(), false).size());
assertNull("Unexpected notification for type 'type2'", _service.getNotification(_container, "objectId2", "type2", _user.getUserId()));
}

private void addNotification(Container container, User user, Notification notification, String expectedErrorPrefix) throws ValidationException
Expand Down
Loading