diff --git a/backend/src/main/java/org/raddatz/familienarchiv/controller/NotificationController.java b/backend/src/main/java/org/raddatz/familienarchiv/controller/NotificationController.java index 7e6b40b9..75eee19e 100644 --- a/backend/src/main/java/org/raddatz/familienarchiv/controller/NotificationController.java +++ b/backend/src/main/java/org/raddatz/familienarchiv/controller/NotificationController.java @@ -1,9 +1,11 @@ package org.raddatz.familienarchiv.controller; import lombok.RequiredArgsConstructor; +import org.raddatz.familienarchiv.dto.NotificationDTO; import org.raddatz.familienarchiv.dto.NotificationPreferenceDTO; import org.raddatz.familienarchiv.model.AppUser; -import org.raddatz.familienarchiv.model.Notification; +import org.raddatz.familienarchiv.security.Permission; +import org.raddatz.familienarchiv.security.RequirePermission; import org.raddatz.familienarchiv.service.NotificationService; import org.raddatz.familienarchiv.service.UserService; import org.springframework.data.domain.Page; @@ -17,13 +19,14 @@ import java.util.UUID; @RestController @RequiredArgsConstructor +@RequirePermission(Permission.READ_ALL) public class NotificationController { private final NotificationService notificationService; private final UserService userService; @GetMapping("/api/notifications") - public Page getNotifications( + public Page getNotifications( @RequestParam(defaultValue = "0") int page, @RequestParam(defaultValue = "10") int size, Authentication authentication) { @@ -40,7 +43,7 @@ public class NotificationController { } @PatchMapping("/api/notifications/{id}/read") - public Notification markOneRead( + public NotificationDTO markOneRead( @PathVariable UUID id, Authentication authentication) { AppUser user = resolveUser(authentication); diff --git a/backend/src/main/java/org/raddatz/familienarchiv/controller/UserSearchController.java b/backend/src/main/java/org/raddatz/familienarchiv/controller/UserSearchController.java index ae43f518..f2333d5e 100644 --- a/backend/src/main/java/org/raddatz/familienarchiv/controller/UserSearchController.java +++ b/backend/src/main/java/org/raddatz/familienarchiv/controller/UserSearchController.java @@ -3,6 +3,8 @@ package org.raddatz.familienarchiv.controller; import lombok.RequiredArgsConstructor; import org.raddatz.familienarchiv.dto.MentionDTO; import org.raddatz.familienarchiv.model.AppUser; +import org.raddatz.familienarchiv.security.Permission; +import org.raddatz.familienarchiv.security.RequirePermission; import org.raddatz.familienarchiv.service.UserSearchService; import org.springframework.web.bind.annotation.GetMapping; import org.springframework.web.bind.annotation.RequestParam; @@ -12,6 +14,7 @@ import java.util.List; @RestController @RequiredArgsConstructor +@RequirePermission(Permission.READ_ALL) public class UserSearchController { private final UserSearchService userSearchService; diff --git a/backend/src/main/java/org/raddatz/familienarchiv/dto/NotificationDTO.java b/backend/src/main/java/org/raddatz/familienarchiv/dto/NotificationDTO.java new file mode 100644 index 00000000..26c91db8 --- /dev/null +++ b/backend/src/main/java/org/raddatz/familienarchiv/dto/NotificationDTO.java @@ -0,0 +1,16 @@ +package org.raddatz.familienarchiv.dto; + +import org.raddatz.familienarchiv.model.NotificationType; + +import java.time.LocalDateTime; +import java.util.UUID; + +public record NotificationDTO( + UUID id, + NotificationType type, + UUID documentId, + UUID referenceId, + boolean read, + LocalDateTime createdAt, + String actorName +) {} diff --git a/backend/src/main/java/org/raddatz/familienarchiv/model/DocumentComment.java b/backend/src/main/java/org/raddatz/familienarchiv/model/DocumentComment.java index e2376979..26294bb8 100644 --- a/backend/src/main/java/org/raddatz/familienarchiv/model/DocumentComment.java +++ b/backend/src/main/java/org/raddatz/familienarchiv/model/DocumentComment.java @@ -64,7 +64,7 @@ public class DocumentComment { private List replies = new ArrayList<>(); // JPA join table for structured mention references — not serialized directly - @ManyToMany(fetch = FetchType.LAZY) + @ManyToMany(fetch = FetchType.EAGER) @JoinTable( name = "comment_mentions", joinColumns = @JoinColumn(name = "comment_id"), diff --git a/backend/src/main/java/org/raddatz/familienarchiv/model/Notification.java b/backend/src/main/java/org/raddatz/familienarchiv/model/Notification.java index b888ad75..e415e574 100644 --- a/backend/src/main/java/org/raddatz/familienarchiv/model/Notification.java +++ b/backend/src/main/java/org/raddatz/familienarchiv/model/Notification.java @@ -46,8 +46,7 @@ public class Notification { @Schema(requiredMode = Schema.RequiredMode.REQUIRED) private LocalDateTime createdAt; - // Populated by NotificationService before serialization — not persisted. - @Transient + @Column(name = "actor_name") @Schema(requiredMode = Schema.RequiredMode.REQUIRED) private String actorName; } diff --git a/backend/src/main/java/org/raddatz/familienarchiv/repository/NotificationRepository.java b/backend/src/main/java/org/raddatz/familienarchiv/repository/NotificationRepository.java index bf3b8a19..da161912 100644 --- a/backend/src/main/java/org/raddatz/familienarchiv/repository/NotificationRepository.java +++ b/backend/src/main/java/org/raddatz/familienarchiv/repository/NotificationRepository.java @@ -8,7 +8,6 @@ import org.springframework.data.jpa.repository.Modifying; import org.springframework.data.jpa.repository.Query; import org.springframework.data.repository.query.Param; -import java.util.List; import java.util.UUID; public interface NotificationRepository extends JpaRepository { @@ -20,6 +19,4 @@ public interface NotificationRepository extends JpaRepository findByRecipientIdOrderByCreatedAtDesc(UUID recipientId); } diff --git a/backend/src/main/java/org/raddatz/familienarchiv/service/CommentService.java b/backend/src/main/java/org/raddatz/familienarchiv/service/CommentService.java index 29e16fca..4d932c84 100644 --- a/backend/src/main/java/org/raddatz/familienarchiv/service/CommentService.java +++ b/backend/src/main/java/org/raddatz/familienarchiv/service/CommentService.java @@ -6,12 +6,13 @@ import org.raddatz.familienarchiv.exception.DomainException; import org.raddatz.familienarchiv.exception.ErrorCode; import org.raddatz.familienarchiv.model.AppUser; import org.raddatz.familienarchiv.model.DocumentComment; -import org.raddatz.familienarchiv.repository.AppUserRepository; import org.raddatz.familienarchiv.repository.CommentRepository; import org.springframework.stereotype.Service; import org.springframework.transaction.annotation.Transactional; +import java.util.LinkedHashSet; import java.util.List; +import java.util.Set; import java.util.UUID; @Service @@ -19,7 +20,7 @@ import java.util.UUID; public class CommentService { private final CommentRepository commentRepository; - private final AppUserRepository userRepository; + private final UserService userService; private final NotificationService notificationService; public List getCommentsForDocument(UUID documentId) { @@ -73,7 +74,10 @@ public class CommentService { saveMentions(reply, mentionedUserIds); DocumentComment saved = commentRepository.save(reply); withMentionDTOs(saved); - notificationService.notifyReply(saved, root); + + Set participantIds = collectParticipantIds(root); + participantIds.remove(author.getId()); + notificationService.notifyReply(saved, participantIds); notificationService.notifyMentions(mentionedUserIds, saved); return saved; } @@ -99,6 +103,10 @@ public class CommentService { commentRepository.delete(comment); } + public List findReplies(UUID parentId) { + return commentRepository.findByParentId(parentId); + } + // ─── private helpers ────────────────────────────────────────────────────── private List withRepliesAndMentions(List roots) { @@ -113,7 +121,7 @@ public class CommentService { private void saveMentions(DocumentComment comment, List mentionedUserIds) { if (mentionedUserIds == null || mentionedUserIds.isEmpty()) return; - List users = userRepository.findAllById(mentionedUserIds); + List users = userService.findAllById(mentionedUserIds); comment.setMentions(users); } @@ -124,6 +132,16 @@ public class CommentService { comment.setMentionDTOs(dtos); } + private Set collectParticipantIds(DocumentComment root) { + Set ids = new LinkedHashSet<>(); + if (root.getAuthorId() != null) ids.add(root.getAuthorId()); + commentRepository.findByParentId(root.getId()) + .forEach(reply -> { + if (reply.getAuthorId() != null) ids.add(reply.getAuthorId()); + }); + return ids; + } + private DocumentComment findComment(UUID documentId, UUID commentId) { return commentRepository.findById(commentId) .filter(c -> documentId.equals(c.getDocumentId())) diff --git a/backend/src/main/java/org/raddatz/familienarchiv/service/NotificationService.java b/backend/src/main/java/org/raddatz/familienarchiv/service/NotificationService.java index cec6629e..d6d371a0 100644 --- a/backend/src/main/java/org/raddatz/familienarchiv/service/NotificationService.java +++ b/backend/src/main/java/org/raddatz/familienarchiv/service/NotificationService.java @@ -2,16 +2,14 @@ package org.raddatz.familienarchiv.service; import lombok.RequiredArgsConstructor; import lombok.extern.slf4j.Slf4j; +import org.raddatz.familienarchiv.dto.NotificationDTO; import org.raddatz.familienarchiv.exception.DomainException; import org.raddatz.familienarchiv.exception.ErrorCode; import org.raddatz.familienarchiv.model.AppUser; import org.raddatz.familienarchiv.model.DocumentComment; import org.raddatz.familienarchiv.model.Notification; import org.raddatz.familienarchiv.model.NotificationType; -import org.raddatz.familienarchiv.repository.AppUserRepository; -import org.raddatz.familienarchiv.repository.CommentRepository; import org.raddatz.familienarchiv.repository.NotificationRepository; -import org.springframework.beans.factory.annotation.Autowired; import org.springframework.beans.factory.annotation.Value; import org.springframework.data.domain.Page; import org.springframework.data.domain.Pageable; @@ -19,9 +17,9 @@ import org.springframework.mail.MailException; import org.springframework.mail.SimpleMailMessage; import org.springframework.mail.javamail.JavaMailSender; import org.springframework.stereotype.Service; +import org.springframework.transaction.annotation.Propagation; import org.springframework.transaction.annotation.Transactional; -import java.util.LinkedHashSet; import java.util.List; import java.util.Optional; import java.util.Set; @@ -33,11 +31,8 @@ import java.util.UUID; public class NotificationService { private final NotificationRepository notificationRepository; - private final CommentRepository commentRepository; - private final AppUserRepository userRepository; - - @Autowired(required = false) - private JavaMailSender mailSender; + private final UserService userService; + private final Optional mailSender; @Value("${app.mail.from:noreply@familienarchiv.local}") private String mailFrom; @@ -46,24 +41,21 @@ public class NotificationService { private String baseUrl; /** - * Creates REPLY notifications for all participants in the thread that the given reply belongs to, - * excluding the replier themselves. + * Creates REPLY notifications for all participants in the thread, excluding the replier. + * Runs in a separate transaction so a notification failure cannot roll back the parent comment. */ - @Transactional - public void notifyReply(DocumentComment reply, DocumentComment root) { - Set participantIds = collectParticipantIds(root); - participantIds.remove(reply.getAuthorId()); + @Transactional(propagation = Propagation.REQUIRES_NEW) + public void notifyReply(DocumentComment reply, Set participantIds) { + if (participantIds.isEmpty()) return; - for (UUID participantId : participantIds) { - Optional recipientOpt = userRepository.findById(participantId); - if (recipientOpt.isEmpty()) continue; - - AppUser recipient = recipientOpt.get(); + List recipients = userService.findAllById(participantIds); + for (AppUser recipient : recipients) { Notification notification = Notification.builder() .recipient(recipient) .type(NotificationType.REPLY) .documentId(reply.getDocumentId()) .referenceId(reply.getId()) + .actorName(reply.getAuthorName()) .build(); notificationRepository.save(notification); @@ -75,19 +67,20 @@ public class NotificationService { /** * Creates MENTION notifications for each mentioned user. + * Runs in a separate transaction so a notification failure cannot roll back the parent comment. */ - @Transactional + @Transactional(propagation = Propagation.REQUIRES_NEW) public void notifyMentions(List mentionedUserIds, DocumentComment comment) { - for (UUID mentionedUserId : mentionedUserIds) { - Optional recipientOpt = userRepository.findById(mentionedUserId); - if (recipientOpt.isEmpty()) continue; + if (mentionedUserIds == null || mentionedUserIds.isEmpty()) return; - AppUser recipient = recipientOpt.get(); + List recipients = userService.findAllById(mentionedUserIds); + for (AppUser recipient : recipients) { Notification notification = Notification.builder() .recipient(recipient) .type(NotificationType.MENTION) .documentId(comment.getDocumentId()) .referenceId(comment.getId()) + .actorName(comment.getAuthorName()) .build(); notificationRepository.save(notification); @@ -97,8 +90,9 @@ public class NotificationService { } } - public Page getNotifications(UUID userId, Pageable pageable) { - return notificationRepository.findByRecipientIdOrderByCreatedAtDesc(userId, pageable); + public Page getNotifications(UUID userId, Pageable pageable) { + return notificationRepository.findByRecipientIdOrderByCreatedAtDesc(userId, pageable) + .map(this::toDTO); } public long countUnread(UUID userId) { @@ -111,7 +105,7 @@ public class NotificationService { } @Transactional - public Notification markRead(UUID notificationId, UUID userId) { + public NotificationDTO markRead(UUID notificationId, UUID userId) { Notification notification = notificationRepository.findById(notificationId) .orElseThrow(() -> DomainException.notFound( ErrorCode.NOTIFICATION_NOT_FOUND, "Notification not found: " + notificationId)); @@ -119,29 +113,26 @@ public class NotificationService { throw DomainException.forbidden("Notification belongs to a different user"); } notification.setRead(true); - return notificationRepository.save(notification); + return toDTO(notificationRepository.save(notification)); } @Transactional public AppUser updatePreferences(UUID userId, boolean notifyOnReply, boolean notifyOnMention) { - AppUser user = userRepository.findById(userId) - .orElseThrow(() -> DomainException.notFound(ErrorCode.USER_NOT_FOUND, "User not found: " + userId)); - user.setNotifyOnReply(notifyOnReply); - user.setNotifyOnMention(notifyOnMention); - return userRepository.save(user); + return userService.updateNotificationPreferences(userId, notifyOnReply, notifyOnMention); } // ─── private helpers ────────────────────────────────────────────────────── - private Set collectParticipantIds(DocumentComment root) { - Set ids = new LinkedHashSet<>(); - if (root.getAuthorId() != null) ids.add(root.getAuthorId()); - - commentRepository.findByParentId(root.getId()) - .forEach(reply -> { - if (reply.getAuthorId() != null) ids.add(reply.getAuthorId()); - }); - return ids; + private NotificationDTO toDTO(Notification n) { + return new NotificationDTO( + n.getId(), + n.getType(), + n.getDocumentId(), + n.getReferenceId(), + n.isRead(), + n.getCreatedAt(), + n.getActorName() + ); } private void buildCommentPath(DocumentComment comment, StringBuilder sb) { @@ -152,7 +143,7 @@ public class NotificationService { } private void sendNotificationEmail(AppUser recipient, DocumentComment comment, NotificationType type) { - if (mailSender == null) { + if (mailSender.isEmpty()) { log.warn("Mail sender not configured — skipping notification email to {}", recipient.getEmail()); return; } @@ -179,7 +170,7 @@ public class NotificationService { message.setText(body); try { - mailSender.send(message); + mailSender.get().send(message); } catch (MailException e) { log.error("Failed to send notification email to {}: {}", recipient.getEmail(), e.getMessage()); } diff --git a/backend/src/main/java/org/raddatz/familienarchiv/service/UserService.java b/backend/src/main/java/org/raddatz/familienarchiv/service/UserService.java index c1988b95..fd639751 100644 --- a/backend/src/main/java/org/raddatz/familienarchiv/service/UserService.java +++ b/backend/src/main/java/org/raddatz/familienarchiv/service/UserService.java @@ -18,6 +18,7 @@ import org.springframework.security.crypto.password.PasswordEncoder; import org.springframework.stereotype.Service; import org.springframework.transaction.annotation.Transactional; +import java.util.Collection; import java.util.HashSet; import java.util.List; import java.util.Optional; @@ -78,6 +79,18 @@ public class UserService { .orElseThrow(() -> DomainException.notFound(ErrorCode.USER_NOT_FOUND, "No user found for id: " + id)); } + public List findAllById(Collection ids) { + return userRepository.findAllById(ids); + } + + @Transactional + public AppUser updateNotificationPreferences(UUID userId, boolean notifyOnReply, boolean notifyOnMention) { + AppUser user = getById(userId); + user.setNotifyOnReply(notifyOnReply); + user.setNotifyOnMention(notifyOnMention); + return userRepository.save(user); + } + @Transactional public AppUser updateProfile(UUID userId, UpdateProfileDTO dto) { AppUser user = getById(userId); diff --git a/backend/src/main/resources/db/migration/V18__add_actor_name_to_notifications.sql b/backend/src/main/resources/db/migration/V18__add_actor_name_to_notifications.sql new file mode 100644 index 00000000..15f89449 --- /dev/null +++ b/backend/src/main/resources/db/migration/V18__add_actor_name_to_notifications.sql @@ -0,0 +1 @@ +ALTER TABLE notifications ADD COLUMN actor_name VARCHAR(255); diff --git a/backend/src/test/java/org/raddatz/familienarchiv/controller/NotificationControllerTest.java b/backend/src/test/java/org/raddatz/familienarchiv/controller/NotificationControllerTest.java index 10541df9..1014aac5 100644 --- a/backend/src/test/java/org/raddatz/familienarchiv/controller/NotificationControllerTest.java +++ b/backend/src/test/java/org/raddatz/familienarchiv/controller/NotificationControllerTest.java @@ -2,8 +2,8 @@ package org.raddatz.familienarchiv.controller; import org.junit.jupiter.api.Test; import org.raddatz.familienarchiv.config.SecurityConfig; +import org.raddatz.familienarchiv.dto.NotificationDTO; import org.raddatz.familienarchiv.model.AppUser; -import org.raddatz.familienarchiv.model.Notification; import org.raddatz.familienarchiv.model.NotificationType; import org.raddatz.familienarchiv.security.PermissionAspect; import org.raddatz.familienarchiv.service.CustomUserDetailsService; @@ -20,6 +20,7 @@ import org.springframework.security.test.context.support.WithMockUser; import org.springframework.test.context.bean.override.mockito.MockitoBean; import org.springframework.test.web.servlet.MockMvc; +import java.time.LocalDateTime; import java.util.List; import java.util.UUID; @@ -52,15 +53,22 @@ class NotificationControllerTest { @Test @WithMockUser(username = "testuser") + void getNotifications_returns403_whenUserLacksPermission() throws Exception { + mockMvc.perform(get("/api/notifications")) + .andExpect(status().isForbidden()); + } + + @Test + @WithMockUser(username = "testuser", authorities = {"READ_ALL"}) void getNotifications_returns200WithList_whenAuthenticated() throws Exception { AppUser user = AppUser.builder().id(USER_ID).username("testuser").build(); - Notification n = Notification.builder() - .id(UUID.randomUUID()).recipient(user) - .type(NotificationType.REPLY).read(false).build(); + NotificationDTO dto = new NotificationDTO( + UUID.randomUUID(), NotificationType.REPLY, UUID.randomUUID(), + UUID.randomUUID(), false, LocalDateTime.now(), "Anna Smith"); when(userService.findByUsername("testuser")).thenReturn(user); when(notificationService.getNotifications(eq(USER_ID), any())) - .thenReturn(new PageImpl<>(List.of(n), PageRequest.of(0, 10), 1)); + .thenReturn(new PageImpl<>(List.of(dto), PageRequest.of(0, 10), 1)); mockMvc.perform(get("/api/notifications")) .andExpect(status().isOk()) @@ -68,7 +76,7 @@ class NotificationControllerTest { } @Test - @WithMockUser(username = "testuser") + @WithMockUser(username = "testuser", authorities = {"READ_ALL"}) void getNotifications_returnsOnlyCurrentUsersNotifications() throws Exception { AppUser user = AppUser.builder().id(USER_ID).username("testuser").build(); when(userService.findByUsername("testuser")).thenReturn(user); @@ -90,7 +98,7 @@ class NotificationControllerTest { } @Test - @WithMockUser(username = "testuser") + @WithMockUser(username = "testuser", authorities = {"READ_ALL"}) void markAllRead_returns204_whenAuthenticated() throws Exception { AppUser user = AppUser.builder().id(USER_ID).username("testuser").build(); when(userService.findByUsername("testuser")).thenReturn(user); @@ -104,7 +112,13 @@ class NotificationControllerTest { // ─── PATCH /api/notifications/{id}/read ────────────────────────────────── @Test - @WithMockUser(username = "testuser") + void markOneRead_returns401_whenUnauthenticated() throws Exception { + mockMvc.perform(patch("/api/notifications/" + UUID.randomUUID() + "/read")) + .andExpect(status().isUnauthorized()); + } + + @Test + @WithMockUser(username = "testuser", authorities = {"READ_ALL"}) void markOneRead_returns403_whenNotificationBelongsToDifferentUser() throws Exception { AppUser user = AppUser.builder().id(USER_ID).username("testuser").build(); UUID notifId = UUID.randomUUID(); @@ -127,7 +141,7 @@ class NotificationControllerTest { } @Test - @WithMockUser(username = "testuser") + @WithMockUser(username = "testuser", authorities = {"READ_ALL"}) void getPreferences_returnsCurrentPreferences() throws Exception { AppUser user = AppUser.builder().id(USER_ID).username("testuser") .notifyOnReply(true).notifyOnMention(false).build(); @@ -142,7 +156,7 @@ class NotificationControllerTest { // ─── PUT /api/users/me/notification-preferences ────────────────────────── @Test - @WithMockUser(username = "testuser") + @WithMockUser(username = "testuser", authorities = {"READ_ALL"}) void updatePreferences_persistsBothBooleans() throws Exception { AppUser user = AppUser.builder().id(USER_ID).username("testuser") .notifyOnReply(false).notifyOnMention(false).build(); diff --git a/backend/src/test/java/org/raddatz/familienarchiv/controller/UserSearchControllerTest.java b/backend/src/test/java/org/raddatz/familienarchiv/controller/UserSearchControllerTest.java index 571f561e..896ae452 100644 --- a/backend/src/test/java/org/raddatz/familienarchiv/controller/UserSearchControllerTest.java +++ b/backend/src/test/java/org/raddatz/familienarchiv/controller/UserSearchControllerTest.java @@ -16,7 +16,9 @@ import org.springframework.test.web.servlet.MockMvc; import java.util.List; import java.util.UUID; +import java.util.stream.IntStream; +import static org.hamcrest.Matchers.lessThanOrEqualTo; import static org.mockito.ArgumentMatchers.anyString; import static org.mockito.Mockito.when; import static org.springframework.test.web.servlet.request.MockMvcRequestBuilders.get; @@ -40,6 +42,13 @@ class UserSearchControllerTest { @Test @WithMockUser + void search_returns403_whenUserLacksPermission() throws Exception { + mockMvc.perform(get("/api/users/search").param("q", "Hans")) + .andExpect(status().isForbidden()); + } + + @Test + @WithMockUser(authorities = {"READ_ALL"}) void search_returns200_whenAuthenticated() throws Exception { AppUser user = AppUser.builder().id(UUID.randomUUID()) .firstName("Hans").lastName("Mueller").username("hans").build(); @@ -51,7 +60,7 @@ class UserSearchControllerTest { } @Test - @WithMockUser + @WithMockUser(authorities = {"READ_ALL"}) void search_returnsEmptyList_whenQueryIsEmpty() throws Exception { when(userSearchService.search("")).thenReturn(List.of()); @@ -61,11 +70,16 @@ class UserSearchControllerTest { } @Test - @WithMockUser + @WithMockUser(authorities = {"READ_ALL"}) void search_returnsAtMostTenResults() throws Exception { - when(userSearchService.search(anyString())).thenReturn(List.of()); + List elevenUsers = IntStream.range(0, 11) + .mapToObj(i -> AppUser.builder().id(UUID.randomUUID()) + .firstName("User").lastName(String.valueOf(i)).username("u" + i).build()) + .toList(); + when(userSearchService.search(anyString())).thenReturn(elevenUsers.subList(0, 10)); mockMvc.perform(get("/api/users/search").param("q", "a")) - .andExpect(status().isOk()); + .andExpect(status().isOk()) + .andExpect(jsonPath("$.length()").value(lessThanOrEqualTo(10))); } } diff --git a/backend/src/test/java/org/raddatz/familienarchiv/service/CommentServiceTest.java b/backend/src/test/java/org/raddatz/familienarchiv/service/CommentServiceTest.java index 50d74467..6d3e8abe 100644 --- a/backend/src/test/java/org/raddatz/familienarchiv/service/CommentServiceTest.java +++ b/backend/src/test/java/org/raddatz/familienarchiv/service/CommentServiceTest.java @@ -9,7 +9,6 @@ import org.raddatz.familienarchiv.exception.DomainException; import org.raddatz.familienarchiv.model.AppUser; import org.raddatz.familienarchiv.model.DocumentComment; import org.raddatz.familienarchiv.model.UserGroup; -import org.raddatz.familienarchiv.repository.AppUserRepository; import org.raddatz.familienarchiv.repository.CommentRepository; import java.time.LocalDateTime; @@ -21,6 +20,8 @@ import java.util.UUID; import static org.assertj.core.api.Assertions.assertThat; import static org.assertj.core.api.Assertions.assertThatThrownBy; import static org.mockito.ArgumentMatchers.any; +import static org.mockito.ArgumentMatchers.anyList; +import static org.mockito.ArgumentMatchers.anySet; import static org.mockito.ArgumentMatchers.eq; import static org.mockito.Mockito.never; import static org.mockito.Mockito.verify; @@ -32,7 +33,7 @@ import static org.springframework.http.HttpStatus.NOT_FOUND; class CommentServiceTest { @Mock CommentRepository commentRepository; - @Mock AppUserRepository userRepository; + @Mock UserService userService; @Mock NotificationService notificationService; @InjectMocks CommentService commentService; @@ -65,6 +66,23 @@ class CommentServiceTest { assertThat(result.getAuthorName()).isEqualTo("hans42"); } + @Test + void postComment_triggersNotifyMentions_whenMentionedUserIdsProvided() { + UUID docId = UUID.randomUUID(); + UUID mentionedId = UUID.randomUUID(); + AppUser author = AppUser.builder().id(UUID.randomUUID()).username("hans").firstName("Hans").lastName("M").build(); + AppUser mentioned = AppUser.builder().id(mentionedId).username("anna").firstName("Anna").lastName("S").build(); + DocumentComment saved = DocumentComment.builder() + .id(UUID.randomUUID()).documentId(docId).authorName("Hans M").content("Hey @Anna S").build(); + + when(userService.findAllById(List.of(mentionedId))).thenReturn(List.of(mentioned)); + when(commentRepository.save(any())).thenReturn(saved); + + commentService.postComment(docId, null, "Hey @Anna S", List.of(mentionedId), author); + + verify(notificationService).notifyMentions(eq(List.of(mentionedId)), eq(saved)); + } + // ─── replyToComment ─────────────────────────────────────────────────────── @Test @@ -95,6 +113,7 @@ class CommentServiceTest { when(commentRepository.findById(replyId)).thenReturn(Optional.of(existingReply)); when(commentRepository.findById(rootId)).thenReturn(Optional.of(root)); + when(commentRepository.findByParentId(rootId)).thenReturn(List.of(existingReply)); DocumentComment saved = DocumentComment.builder() .id(UUID.randomUUID()).documentId(docId).parentId(rootId).content("Reply2").authorName("anna").build(); when(commentRepository.save(any())).thenReturn(saved); @@ -114,6 +133,7 @@ class CommentServiceTest { .id(rootId).documentId(docId).parentId(null).content("Root").authorName("Hans").build(); when(commentRepository.findById(rootId)).thenReturn(Optional.of(root)); + when(commentRepository.findByParentId(rootId)).thenReturn(List.of()); DocumentComment saved = DocumentComment.builder() .id(UUID.randomUUID()).documentId(docId).parentId(rootId).content("Reply").authorName("anna").build(); when(commentRepository.save(any())).thenReturn(saved); @@ -124,7 +144,7 @@ class CommentServiceTest { } @Test - void replyToComment_triggersNotification_afterSave() { + void replyToComment_triggersNotifyReply_afterSave() { UUID docId = UUID.randomUUID(); UUID rootId = UUID.randomUUID(); AppUser author = AppUser.builder().id(UUID.randomUUID()).username("anna").build(); @@ -135,11 +155,35 @@ class CommentServiceTest { .id(UUID.randomUUID()).documentId(docId).parentId(rootId).content("Reply").authorName("anna").build(); when(commentRepository.findById(rootId)).thenReturn(Optional.of(root)); + when(commentRepository.findByParentId(rootId)).thenReturn(List.of()); when(commentRepository.save(any())).thenReturn(saved); commentService.replyToComment(docId, rootId, "Reply", List.of(), author); - verify(notificationService).notifyReply(eq(saved), eq(root)); + verify(notificationService).notifyReply(eq(saved), anySet()); + } + + @Test + void replyToComment_triggersNotifyMentions_whenMentionedUserIdsProvided() { + UUID docId = UUID.randomUUID(); + UUID rootId = UUID.randomUUID(); + UUID mentionedId = UUID.randomUUID(); + AppUser author = AppUser.builder().id(UUID.randomUUID()).username("anna").build(); + AppUser mentioned = AppUser.builder().id(mentionedId).username("bob").firstName("Bob").lastName("J").build(); + + DocumentComment root = DocumentComment.builder() + .id(rootId).documentId(docId).parentId(null).content("Root").authorName("Hans").build(); + DocumentComment saved = DocumentComment.builder() + .id(UUID.randomUUID()).documentId(docId).parentId(rootId).content("Hey @Bob J").authorName("anna").build(); + + when(userService.findAllById(List.of(mentionedId))).thenReturn(List.of(mentioned)); + when(commentRepository.findById(rootId)).thenReturn(Optional.of(root)); + when(commentRepository.findByParentId(rootId)).thenReturn(List.of()); + when(commentRepository.save(any())).thenReturn(saved); + + commentService.replyToComment(docId, rootId, "Hey @Bob J", List.of(mentionedId), author); + + verify(notificationService).notifyMentions(eq(List.of(mentionedId)), eq(saved)); } // ─── editComment ────────────────────────────────────────────────────────── diff --git a/backend/src/test/java/org/raddatz/familienarchiv/service/NotificationServiceTest.java b/backend/src/test/java/org/raddatz/familienarchiv/service/NotificationServiceTest.java index c515cc7c..bee93f92 100644 --- a/backend/src/test/java/org/raddatz/familienarchiv/service/NotificationServiceTest.java +++ b/backend/src/test/java/org/raddatz/familienarchiv/service/NotificationServiceTest.java @@ -4,20 +4,18 @@ import org.junit.jupiter.api.BeforeEach; import org.junit.jupiter.api.Test; import org.junit.jupiter.api.extension.ExtendWith; import org.mockito.ArgumentCaptor; -import org.mockito.InjectMocks; import org.mockito.Mock; import org.mockito.junit.jupiter.MockitoExtension; -import org.springframework.test.util.ReflectionTestUtils; +import org.raddatz.familienarchiv.dto.NotificationDTO; import org.raddatz.familienarchiv.exception.DomainException; import org.raddatz.familienarchiv.model.*; -import org.raddatz.familienarchiv.repository.AppUserRepository; -import org.raddatz.familienarchiv.repository.CommentRepository; import org.raddatz.familienarchiv.repository.NotificationRepository; import org.springframework.mail.SimpleMailMessage; import org.springframework.mail.javamail.JavaMailSender; import java.util.List; import java.util.Optional; +import java.util.Set; import java.util.UUID; import static org.assertj.core.api.Assertions.assertThat; @@ -29,11 +27,10 @@ import static org.mockito.Mockito.*; class NotificationServiceTest { @Mock NotificationRepository notificationRepository; - @Mock CommentRepository commentRepository; - @Mock AppUserRepository userRepository; + @Mock UserService userService; @Mock JavaMailSender mailSender; - @InjectMocks NotificationService notificationService; + NotificationService notificationService; private AppUser userA; private AppUser userB; @@ -41,9 +38,7 @@ class NotificationServiceTest { @BeforeEach void setUp() { - // mailSender is @Autowired(required=false) — not in the @RequiredArgsConstructor - // constructor, so Mockito won't inject it automatically. Inject explicitly. - ReflectionTestUtils.setField(notificationService, "mailSender", mailSender); + notificationService = new NotificationService(notificationRepository, userService, Optional.of(mailSender)); userA = AppUser.builder().id(UUID.randomUUID()).username("userA") .firstName("Anna").lastName("Smith").email("a@test.com") @@ -59,17 +54,13 @@ class NotificationServiceTest { // ─── notifyReply ────────────────────────────────────────────────────────── @Test - void notifyReply_createsNotificationForThreadParticipant() { - DocumentComment root = commentWithAuthor(UUID.randomUUID(), null, userA.getId()); - DocumentComment existing = commentWithAuthor(UUID.randomUUID(), root.getId(), userB.getId()); - DocumentComment reply = commentWithAuthor(UUID.randomUUID(), root.getId(), userC.getId()); + void notifyReply_createsNotificationForThreadParticipants() { + DocumentComment reply = commentWithAuthor(UUID.randomUUID(), null, userC.getId(), "Clara Doe"); - when(commentRepository.findByParentId(root.getId())).thenReturn(List.of(existing, reply)); - when(userRepository.findById(userA.getId())).thenReturn(Optional.of(userA)); - when(userRepository.findById(userB.getId())).thenReturn(Optional.of(userB)); + when(userService.findAllById(Set.of(userA.getId(), userB.getId()))).thenReturn(List.of(userA, userB)); when(notificationRepository.save(any())).thenAnswer(inv -> inv.getArgument(0)); - notificationService.notifyReply(reply, root); + notificationService.notifyReply(reply, Set.of(userA.getId(), userB.getId())); ArgumentCaptor captor = ArgumentCaptor.forClass(Notification.class); verify(notificationRepository, times(2)).save(captor.capture()); @@ -79,57 +70,30 @@ class NotificationServiceTest { .containsExactlyInAnyOrder(userA.getId(), userB.getId()); assertThat(saved).allMatch(n -> n.getType() == NotificationType.REPLY); assertThat(saved).allMatch(n -> !n.isRead()); + assertThat(saved).allMatch(n -> "Clara Doe".equals(n.getActorName())); } @Test - void notifyReply_doesNotNotifyTheReplierThemselves() { - // userA is both a thread participant and the replier - DocumentComment root = commentWithAuthor(UUID.randomUUID(), null, userA.getId()); - DocumentComment reply = commentWithAuthor(UUID.randomUUID(), root.getId(), userA.getId()); + void notifyReply_doesNothing_whenParticipantSetIsEmpty() { + DocumentComment reply = commentWithAuthor(UUID.randomUUID(), null, userA.getId(), "Anna Smith"); - when(commentRepository.findByParentId(root.getId())).thenReturn(List.of(reply)); - - notificationService.notifyReply(reply, root); + notificationService.notifyReply(reply, Set.of()); verify(notificationRepository, never()).save(any()); } - @Test - void notifyReply_deduplicatesParticipants() { - // userB has posted twice in the thread — should get exactly one notification - DocumentComment root = commentWithAuthor(UUID.randomUUID(), null, userA.getId()); - DocumentComment first = commentWithAuthor(UUID.randomUUID(), root.getId(), userB.getId()); - DocumentComment second = commentWithAuthor(UUID.randomUUID(), root.getId(), userB.getId()); - DocumentComment reply = commentWithAuthor(UUID.randomUUID(), root.getId(), userC.getId()); - - when(commentRepository.findByParentId(root.getId())).thenReturn(List.of(first, second, reply)); - when(userRepository.findById(userA.getId())).thenReturn(Optional.of(userA)); - when(userRepository.findById(userB.getId())).thenReturn(Optional.of(userB)); - when(notificationRepository.save(any())).thenAnswer(inv -> inv.getArgument(0)); - - notificationService.notifyReply(reply, root); - - // userA (root) + userB (deduplicated) = 2 notifications, not 3 - verify(notificationRepository, times(2)).save(any()); - } - @Test void notifyReply_sendsEmailOnlyToUsersWithReplyNotificationsEnabled() { userA.setNotifyOnReply(true); userB.setNotifyOnReply(false); - DocumentComment root = commentWithAuthor(UUID.randomUUID(), null, userA.getId()); - DocumentComment existing = commentWithAuthor(UUID.randomUUID(), root.getId(), userB.getId()); - DocumentComment reply = commentWithAuthor(UUID.randomUUID(), root.getId(), userC.getId()); + DocumentComment reply = commentWithAuthor(UUID.randomUUID(), null, userC.getId(), "Clara Doe"); - when(commentRepository.findByParentId(root.getId())).thenReturn(List.of(existing, reply)); - when(userRepository.findById(userA.getId())).thenReturn(Optional.of(userA)); - when(userRepository.findById(userB.getId())).thenReturn(Optional.of(userB)); + when(userService.findAllById(Set.of(userA.getId(), userB.getId()))).thenReturn(List.of(userA, userB)); when(notificationRepository.save(any())).thenAnswer(inv -> inv.getArgument(0)); - notificationService.notifyReply(reply, root); + notificationService.notifyReply(reply, Set.of(userA.getId(), userB.getId())); - // Only userA has email enabled — one email sent verify(mailSender, times(1)).send(any(SimpleMailMessage.class)); } @@ -137,9 +101,8 @@ class NotificationServiceTest { @Test void notifyMentions_createsNotificationPerMentionedUser() { - DocumentComment comment = commentWithAuthor(UUID.randomUUID(), null, userC.getId()); - when(userRepository.findById(userA.getId())).thenReturn(Optional.of(userA)); - when(userRepository.findById(userB.getId())).thenReturn(Optional.of(userB)); + DocumentComment comment = commentWithAuthor(UUID.randomUUID(), null, userC.getId(), "Clara Doe"); + when(userService.findAllById(List.of(userA.getId(), userB.getId()))).thenReturn(List.of(userA, userB)); when(notificationRepository.save(any())).thenAnswer(inv -> inv.getArgument(0)); notificationService.notifyMentions(List.of(userA.getId(), userB.getId()), comment); @@ -151,6 +114,16 @@ class NotificationServiceTest { assertThat(saved).extracting(n -> n.getRecipient().getId()) .containsExactlyInAnyOrder(userA.getId(), userB.getId()); assertThat(saved).allMatch(n -> n.getType() == NotificationType.MENTION); + assertThat(saved).allMatch(n -> "Clara Doe".equals(n.getActorName())); + } + + @Test + void notifyMentions_doesNothing_whenListIsEmpty() { + DocumentComment comment = commentWithAuthor(UUID.randomUUID(), null, userA.getId(), "Anna Smith"); + + notificationService.notifyMentions(List.of(), comment); + + verify(notificationRepository, never()).save(any()); } @Test @@ -158,9 +131,8 @@ class NotificationServiceTest { userA.setNotifyOnMention(true); userB.setNotifyOnMention(false); - DocumentComment comment = commentWithAuthor(UUID.randomUUID(), null, userC.getId()); - when(userRepository.findById(userA.getId())).thenReturn(Optional.of(userA)); - when(userRepository.findById(userB.getId())).thenReturn(Optional.of(userB)); + DocumentComment comment = commentWithAuthor(UUID.randomUUID(), null, userC.getId(), "Clara Doe"); + when(userService.findAllById(List.of(userA.getId(), userB.getId()))).thenReturn(List.of(userA, userB)); when(notificationRepository.save(any())).thenAnswer(inv -> inv.getArgument(0)); notificationService.notifyMentions(List.of(userA.getId(), userB.getId()), comment); @@ -170,6 +142,16 @@ class NotificationServiceTest { // ─── markRead ───────────────────────────────────────────────────────────── + @Test + void markRead_throwsNotFound_whenNotificationDoesNotExist() { + UUID notifId = UUID.randomUUID(); + when(notificationRepository.findById(notifId)).thenReturn(Optional.empty()); + + assertThatThrownBy(() -> notificationService.markRead(notifId, userA.getId())) + .isInstanceOf(DomainException.class) + .hasMessageContaining("Notification not found"); + } + @Test void markRead_throwsForbidden_whenNotificationBelongsToDifferentUser() { Notification notification = Notification.builder() @@ -186,15 +168,33 @@ class NotificationServiceTest { .hasMessageContaining("different user"); } + // ─── markAllRead ────────────────────────────────────────────────────────── + + @Test + void markAllRead_delegatesToRepository() { + notificationService.markAllRead(userA.getId()); + + verify(notificationRepository).markAllReadByRecipientId(userA.getId()); + } + + // ─── countUnread ────────────────────────────────────────────────────────── + + @Test + void countUnread_delegatesToRepository() { + when(notificationRepository.countByRecipientIdAndReadFalse(userA.getId())).thenReturn(3L); + + assertThat(notificationService.countUnread(userA.getId())).isEqualTo(3L); + } + // ─── private helpers ────────────────────────────────────────────────────── - private DocumentComment commentWithAuthor(UUID id, UUID parentId, UUID authorId) { + private DocumentComment commentWithAuthor(UUID id, UUID parentId, UUID authorId, String authorName) { return DocumentComment.builder() .id(id) .documentId(UUID.randomUUID()) .parentId(parentId) .authorId(authorId) - .authorName("Author") + .authorName(authorName) .content("content") .build(); } diff --git a/frontend/messages/de.json b/frontend/messages/de.json index a7c1abdd..19480fed 100644 --- a/frontend/messages/de.json +++ b/frontend/messages/de.json @@ -304,6 +304,7 @@ "notification_prefs_heading": "Benachrichtigungen", "notification_pref_reply": "E-Mail, wenn jemand auf meinen Kommentar antwortet", "notification_pref_mention": "E-Mail, wenn jemand mich in einem Kommentar erwähnt", + "notification_unread": "ungelesen", "mention_btn_label": "Person erwähnen", "mention_popup_empty": "Keine Nutzer gefunden" } diff --git a/frontend/messages/en.json b/frontend/messages/en.json index 7f5d0893..a32a89db 100644 --- a/frontend/messages/en.json +++ b/frontend/messages/en.json @@ -304,6 +304,7 @@ "notification_prefs_heading": "Notifications", "notification_pref_reply": "Email when someone replies to my comment", "notification_pref_mention": "Email when someone mentions me in a comment", + "notification_unread": "unread", "mention_btn_label": "Mention person", "mention_popup_empty": "No users found" } diff --git a/frontend/messages/es.json b/frontend/messages/es.json index d145b9e9..0126e943 100644 --- a/frontend/messages/es.json +++ b/frontend/messages/es.json @@ -304,6 +304,7 @@ "notification_prefs_heading": "Notificaciones", "notification_pref_reply": "Correo cuando alguien responde a mi comentario", "notification_pref_mention": "Correo cuando alguien me menciona en un comentario", + "notification_unread": "no leído", "mention_btn_label": "Mencionar persona", "mention_popup_empty": "No se encontraron usuarios" } diff --git a/frontend/src/lib/components/CommentThread.svelte b/frontend/src/lib/components/CommentThread.svelte index 0a67aefb..de350d72 100644 --- a/frontend/src/lib/components/CommentThread.svelte +++ b/frontend/src/lib/components/CommentThread.svelte @@ -1,5 +1,5 @@ diff --git a/frontend/src/lib/components/NotificationBell.svelte b/frontend/src/lib/components/NotificationBell.svelte index 533ce5b0..639d0ba6 100644 --- a/frontend/src/lib/components/NotificationBell.svelte +++ b/frontend/src/lib/components/NotificationBell.svelte @@ -117,16 +117,15 @@ function attachClickOutside(node: HTMLElement) { } function relativeTime(isoString: string): string { - const now = Date.now(); - const then = new Date(isoString).getTime(); - const diffMs = now - then; + const diffMs = Date.now() - new Date(isoString).getTime(); const diffMin = Math.floor(diffMs / 60000); - if (diffMin < 1) return 'gerade eben'; - if (diffMin < 60) return `vor ${diffMin} Min.`; + const rtf = new Intl.RelativeTimeFormat(undefined, { numeric: 'auto' }); + if (diffMin < 1) return rtf.format(0, 'minute'); + if (diffMin < 60) return rtf.format(-diffMin, 'minute'); const diffH = Math.floor(diffMin / 60); - if (diffH < 24) return `vor ${diffH} Std.`; + if (diffH < 24) return rtf.format(-diffH, 'hour'); const diffD = Math.floor(diffH / 24); - return `vor ${diffD} Tag${diffD !== 1 ? 'en' : ''}`; + return rtf.format(-diffD, 'day'); } onMount(() => { @@ -232,12 +231,10 @@ onDestroy(() => {
    {#each notifications as notification (notification.id)}
  • -
    markRead(notification)} - onkeydown={(e) => e.key === 'Enter' && markRead(notification)} - class="flex cursor-pointer items-start gap-3 border-b border-line px-4 py-3 last:border-b-0 hover:bg-canvas + class="flex w-full cursor-pointer items-start gap-3 border-b border-line px-4 py-3 text-left last:border-b-0 hover:bg-canvas {!notification.read ? 'bg-accent-bg/20' : ''}" > @@ -291,10 +288,10 @@ onDestroy(() => { {#if !notification.read} {/if} -
    +
    • {/each}
diff --git a/frontend/src/lib/utils/mention.spec.ts b/frontend/src/lib/utils/mention.spec.ts index 2da73497..301cbabe 100644 --- a/frontend/src/lib/utils/mention.spec.ts +++ b/frontend/src/lib/utils/mention.spec.ts @@ -92,10 +92,10 @@ describe('renderBody', () => { expect(result).toContain('AT&T'); }); - it('wraps @mention in an anchor tag', () => { + it('wraps @mention in a mention span', () => { const mentions: MentionDTO[] = [{ id: 'uuid-1', firstName: 'Hans', lastName: 'Müller' }]; const result = renderBody('Hey @Hans Müller!', mentions); - expect(result).toContain(' { it('replaces all occurrences of the same mention', () => { const mentions: MentionDTO[] = [{ id: 'uuid-1', firstName: 'Hans', lastName: 'Müller' }]; const result = renderBody('@Hans Müller and @Hans Müller', mentions); - const linkCount = (result.match(/ { + const mentions: MentionDTO[] = [{ id: 'u1', firstName: '