Merge pull request #647 from nagisa77/codex/fix-pagination-issue-in-notification-queries

Fix notification pagination after filtering disabled types

backend/src/main/java/com/openisle/repository/NotificationRepository.java

@@ -17,7 +17,10 @@ public interface NotificationRepository extends JpaRepository<Notification, Long
     List<Notification> findByUserAndReadOrderByCreatedAtDesc(User user, boolean read);
     Page<Notification> findByUserOrderByCreatedAtDesc(User user, Pageable pageable);
     Page<Notification> findByUserAndReadOrderByCreatedAtDesc(User user, boolean read, Pageable pageable);
+    Page<Notification> findByUserAndTypeNotInOrderByCreatedAtDesc(User user, java.util.Collection<NotificationType> types, Pageable pageable);
+    Page<Notification> findByUserAndReadAndTypeNotInOrderByCreatedAtDesc(User user, boolean read, java.util.Collection<NotificationType> types, Pageable pageable);
     long countByUserAndRead(User user, boolean read);
+    long countByUserAndReadAndTypeNotIn(User user, boolean read, java.util.Collection<NotificationType> types);
     List<Notification> findByPost(Post post);
     List<Notification> findByComment(Comment comment);
 

backend/src/main/java/com/openisle/service/NotificationService.java

@@ -23,7 +23,6 @@ import java.util.HashSet;
 import java.util.List;
 import java.util.ArrayList;
 import java.util.concurrent.Executor;
-import java.util.stream.Collectors;
 
 /** Service for creating and retrieving notifications. */
 @Service
@@ -187,11 +186,19 @@ public class NotificationService {
         org.springframework.data.domain.Pageable pageable = org.springframework.data.domain.PageRequest.of(page, size);
         org.springframework.data.domain.Page<Notification> result;
         if (read == null) {
+            if (disabled.isEmpty()) {
                 result = notificationRepository.findByUserOrderByCreatedAtDesc(user, pageable);
             } else {
-            result = notificationRepository.findByUserAndReadOrderByCreatedAtDesc(user, read, pageable);
+                result = notificationRepository.findByUserAndTypeNotInOrderByCreatedAtDesc(user, disabled, pageable);
             }
-        return result.stream().filter(n -> !disabled.contains(n.getType())).collect(Collectors.toList());
+        } else {
+            if (disabled.isEmpty()) {
+                result = notificationRepository.findByUserAndReadOrderByCreatedAtDesc(user, read, pageable);
+            } else {
+                result = notificationRepository.findByUserAndReadAndTypeNotInOrderByCreatedAtDesc(user, read, disabled, pageable);
+            }
+        }
+        return result.getContent();
     }
 
     public void markRead(String username, List<Long> ids) {
@@ -210,8 +217,10 @@ public class NotificationService {
         User user = userRepository.findByUsername(username)
                 .orElseThrow(() -> new com.openisle.exception.NotFoundException("User not found"));
         Set<NotificationType> disabled = user.getDisabledNotificationTypes();
-        return notificationRepository.findByUserAndReadOrderByCreatedAtDesc(user, false).stream()
+        if (disabled.isEmpty()) {
-                .filter(n -> !disabled.contains(n.getType())).count();
+            return notificationRepository.countByUserAndRead(user, false);
+        }
+        return notificationRepository.countByUserAndReadAndTypeNotIn(user, false, disabled);
     }
 
     public void notifyMentions(String content, User fromUser, Post post, Comment comment) {

backend/src/test/java/com/openisle/service/NotificationServiceTest.java

@@ -11,6 +11,7 @@ import org.mockito.Mockito;
 
 import java.util.List;
 import java.util.Optional;
+import java.util.HashSet;
 import org.springframework.data.domain.PageImpl;
 import org.springframework.data.domain.Pageable;
 
@@ -64,6 +65,7 @@ class NotificationServiceTest {
         User user = new User();
         user.setId(2L);
         user.setUsername("bob");
+        user.setDisabledNotificationTypes(new HashSet<>());
         when(uRepo.findByUsername("bob")).thenReturn(Optional.of(user));
 
         Notification n = new Notification();
@@ -90,6 +92,7 @@ class NotificationServiceTest {
         User user = new User();
         user.setId(3L);
         user.setUsername("carl");
+        user.setDisabledNotificationTypes(new HashSet<>());
         when(uRepo.findByUsername("carl")).thenReturn(Optional.of(user));
         when(nRepo.countByUserAndRead(user, false)).thenReturn(5L);
 
@@ -99,6 +102,56 @@ class NotificationServiceTest {
         verify(nRepo).countByUserAndRead(user, false);
     }
 
+    @Test
+    void listNotificationsFiltersDisabledTypes() {
+        NotificationRepository nRepo = mock(NotificationRepository.class);
+        UserRepository uRepo = mock(UserRepository.class);
+        ReactionRepository rRepo = mock(ReactionRepository.class);
+        EmailSender email = mock(EmailSender.class);
+        PushNotificationService push = mock(PushNotificationService.class);
+        Executor executor = Runnable::run;
+        NotificationService service = new NotificationService(nRepo, uRepo, email, push, rRepo, executor);
+        org.springframework.test.util.ReflectionTestUtils.setField(service, "websiteUrl", "https://ex.com");
+
+        User user = new User();
+        user.setId(4L);
+        user.setUsername("dana");
+        when(uRepo.findByUsername("dana")).thenReturn(Optional.of(user));
+
+        Notification n = new Notification();
+        when(nRepo.findByUserAndTypeNotInOrderByCreatedAtDesc(eq(user), eq(user.getDisabledNotificationTypes()), any(Pageable.class)))
+                .thenReturn(new PageImpl<>(List.of(n)));
+
+        List<Notification> list = service.listNotifications("dana", null, 0, 10);
+
+        assertEquals(1, list.size());
+        verify(nRepo).findByUserAndTypeNotInOrderByCreatedAtDesc(eq(user), eq(user.getDisabledNotificationTypes()), any(Pageable.class));
+    }
+
+    @Test
+    void countUnreadFiltersDisabledTypes() {
+        NotificationRepository nRepo = mock(NotificationRepository.class);
+        UserRepository uRepo = mock(UserRepository.class);
+        ReactionRepository rRepo = mock(ReactionRepository.class);
+        EmailSender email = mock(EmailSender.class);
+        PushNotificationService push = mock(PushNotificationService.class);
+        Executor executor = Runnable::run;
+        NotificationService service = new NotificationService(nRepo, uRepo, email, push, rRepo, executor);
+        org.springframework.test.util.ReflectionTestUtils.setField(service, "websiteUrl", "https://ex.com");
+
+        User user = new User();
+        user.setId(5L);
+        user.setUsername("erin");
+        when(uRepo.findByUsername("erin")).thenReturn(Optional.of(user));
+        when(nRepo.countByUserAndReadAndTypeNotIn(eq(user), eq(false), eq(user.getDisabledNotificationTypes())))
+                .thenReturn(2L);
+
+        long count = service.countUnread("erin");
+
+        assertEquals(2L, count);
+        verify(nRepo).countByUserAndReadAndTypeNotIn(eq(user), eq(false), eq(user.getDisabledNotificationTypes()));
+    }
+
     @Test
     void createRegisterRequestNotificationsDeletesOldOnes() {
         NotificationRepository nRepo = mock(NotificationRepository.class);