marcel/familienarchiv
1
0
Fork 0
Code Issues 117 Pull Requests 1 Actions 1 Packages Projects Releases Wiki Activity

feat: notifications, @mentions, and comment deep-links (#71 #72 #73) #127

Merged
marcel merged 19 commits from feat/71-72-73-notifications-mentions-deeplinks into main 2026-03-28 16:06:59 +01:00
Conversation 12 Commits 19 Files Changed 54 +2695 -69
54 changed files with 2695 additions and 69 deletions
Download Patch File Download Diff File Expand all files Collapse all files

4
backend/src/main/java/org/raddatz/familienarchiv/controller/AnnotationController.java
View File

@@ -35,7 +35,7 @@ public class AnnotationController {
@PostMapping
@ResponseStatus(HttpStatus.CREATED)
@RequirePermission(Permission.ANNOTATE_ALL)
@RequirePermission({Permission.ANNOTATE_ALL, Permission.WRITE_ALL})
public DocumentAnnotation createAnnotation(
@PathVariable UUID documentId,
@RequestBody CreateAnnotationDTO dto,
@@ -47,7 +47,7 @@ public class AnnotationController {
@DeleteMapping("/{annotationId}")
@ResponseStatus(HttpStatus.NO_CONTENT)
@RequirePermission(Permission.ANNOTATE_ALL)
@RequirePermission({Permission.ANNOTATE_ALL, Permission.WRITE_ALL})
public void deleteAnnotation(
@PathVariable UUID documentId,
@PathVariable UUID annotationId,

18
backend/src/main/java/org/raddatz/familienarchiv/controller/CommentController.java
View File

@@ -33,25 +33,25 @@ public class CommentController {
@PostMapping("/api/documents/{documentId}/comments")
@ResponseStatus(HttpStatus.CREATED)
@RequirePermission(Permission.ANNOTATE_ALL)
@RequirePermission({Permission.ANNOTATE_ALL, Permission.WRITE_ALL})
public DocumentComment postDocumentComment(
@PathVariable UUID documentId,
@RequestBody CreateCommentDTO dto,
Authentication authentication) {
AppUser author = resolveUser(authentication);
return commentService.postComment(documentId, null, dto.getContent(), author);
return commentService.postComment(documentId, null, dto.getContent(), dto.getMentionedUserIds(), author);
}
@PostMapping("/api/documents/{documentId}/comments/{commentId}/replies")
@ResponseStatus(HttpStatus.CREATED)
@RequirePermission(Permission.ANNOTATE_ALL)
@RequirePermission({Permission.ANNOTATE_ALL, Permission.WRITE_ALL})
public DocumentComment replyToDocumentComment(
@PathVariable UUID documentId,
@PathVariable UUID commentId,
@RequestBody CreateCommentDTO dto,
Authentication authentication) {
AppUser author = resolveUser(authentication);
return commentService.replyToComment(documentId, commentId, dto.getContent(), author);
return commentService.replyToComment(documentId, commentId, dto.getContent(), dto.getMentionedUserIds(), author);
}
// ─── Annotation comments ──────────────────────────────────────────────────
@@ -63,32 +63,32 @@ public class CommentController {
@PostMapping("/api/documents/{documentId}/annotations/{annotationId}/comments")
@ResponseStatus(HttpStatus.CREATED)
@RequirePermission(Permission.ANNOTATE_ALL)
@RequirePermission({Permission.ANNOTATE_ALL, Permission.WRITE_ALL})
public DocumentComment postAnnotationComment(
@PathVariable UUID documentId,
@PathVariable UUID annotationId,
@RequestBody CreateCommentDTO dto,
Authentication authentication) {
AppUser author = resolveUser(authentication);
return commentService.postComment(documentId, annotationId, dto.getContent(), author);
return commentService.postComment(documentId, annotationId, dto.getContent(), dto.getMentionedUserIds(), author);
}
@PostMapping("/api/documents/{documentId}/annotations/{annotationId}/comments/{commentId}/replies")
@ResponseStatus(HttpStatus.CREATED)
@RequirePermission(Permission.ANNOTATE_ALL)
@RequirePermission({Permission.ANNOTATE_ALL, Permission.WRITE_ALL})
public DocumentComment replyToAnnotationComment(
@PathVariable UUID documentId,
@PathVariable UUID commentId,
@RequestBody CreateCommentDTO dto,
Authentication authentication) {
AppUser author = resolveUser(authentication);
return commentService.replyToComment(documentId, commentId, dto.getContent(), author);
return commentService.replyToComment(documentId, commentId, dto.getContent(), dto.getMentionedUserIds(), author);
}
// ─── Edit and delete (shared) ─────────────────────────────────────────────
@PatchMapping("/api/documents/{documentId}/comments/{commentId}")
@RequirePermission(Permission.ANNOTATE_ALL)
@RequirePermission({Permission.ANNOTATE_ALL, Permission.WRITE_ALL})
public DocumentComment editComment(
@PathVariable UUID documentId,
@PathVariable UUID commentId,

97
backend/src/main/java/org/raddatz/familienarchiv/controller/NotificationController.java Normal file
View File

@@ -0,0 +1,97 @@
package org.raddatz.familienarchiv.controller;
marcel commented 2026-03-27 23:34:56 +01:00
Review
Copy Link

⚠️ MAJOR — No @RequirePermission on notification controller

All five notification endpoints have no @RequirePermission annotation. Resolving the current user via authentication.getName() is not the same as the AOP PermissionAspect gate — a disabled or low-privilege account that passes the Spring Security filter layer can still call these endpoints.

The NotificationControllerTest tests 401 for unauthenticated, but never tests that a user with no permissions is blocked (403). This gap means a regression here would not be caught by CI.

Fix: Add @RequirePermission(Permission.READ_ALL) at the controller class level. Update tests to cover the 403 case.

⚠️ **MAJOR — No `@RequirePermission` on notification controller** All five notification endpoints have no `@RequirePermission` annotation. Resolving the current user via `authentication.getName()` is not the same as the AOP `PermissionAspect` gate — a disabled or low-privilege account that passes the Spring Security filter layer can still call these endpoints. The `NotificationControllerTest` tests 401 for unauthenticated, but never tests that a user with *no permissions* is blocked (403). This gap means a regression here would not be caught by CI. **Fix:** Add `@RequirePermission(Permission.READ_ALL)` at the controller class level. Update tests to cover the 403 case.
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.security.Permission;
import org.raddatz.familienarchiv.security.RequirePermission;
import org.raddatz.familienarchiv.service.NotificationService;
import org.raddatz.familienarchiv.service.SseEmitterRegistry;
import org.raddatz.familienarchiv.service.UserService;
import org.springframework.data.domain.Page;
import org.springframework.data.domain.PageRequest;
import org.springframework.data.domain.Sort;
import org.springframework.http.HttpStatus;
import org.springframework.http.MediaType;
import org.springframework.security.core.Authentication;
import org.springframework.web.bind.annotation.*;
import org.springframework.web.servlet.mvc.method.annotation.SseEmitter;
import java.util.Map;
import java.util.UUID;
@RestController
@RequiredArgsConstructor
marcel commented 2026-03-27 22:11:33 +01:00
Review
Copy Link

Avoid returning the Notification entity directly from the controller.

Notification has @ManyToOne(fetch = FetchType.LAZY) on recipient. Jackson will trigger lazy loading for every notification in the page to serialize the AppUser — that's another N+1 in the serialization path. Worse, AppUser contains fields (password hash, groups) that should never leave the backend.

Create a NotificationDTO record:

public record NotificationDTO(
    UUID id,
    NotificationType type,
    UUID documentId,
    UUID referenceId,
    boolean read,
    LocalDateTime createdAt,
    String actorName
) {}

Return Page<NotificationDTO> from getNotifications and NotificationDTO from markOneRead. The service maps Notification → NotificationDTO before returning. This sidesteps both the N+1 and the data-leakage risk.

**Avoid returning the `Notification` entity directly from the controller.** `Notification` has `@ManyToOne(fetch = FetchType.LAZY)` on `recipient`. Jackson will trigger lazy loading for every notification in the page to serialize the `AppUser` — that's another N+1 in the serialization path. Worse, `AppUser` contains fields (password hash, groups) that should never leave the backend. Create a `NotificationDTO` record: ```java public record NotificationDTO( UUID id, NotificationType type, UUID documentId, UUID referenceId, boolean read, LocalDateTime createdAt, String actorName ) {} ``` Return `Page<NotificationDTO>` from `getNotifications` and `NotificationDTO` from `markOneRead`. The service maps `Notification → NotificationDTO` before returning. This sidesteps both the N+1 and the data-leakage risk.
public class NotificationController {
private final NotificationService notificationService;
private final UserService userService;
private final SseEmitterRegistry sseEmitterRegistry;
// These endpoints are intentionally open to any authenticated user —
// they return and mutate only the current user's own notifications, scoped
// by the resolved user identity. No additional permission check is required.
@GetMapping(value = "/api/notifications/stream", produces = MediaType.TEXT_EVENT_STREAM_VALUE)
public SseEmitter stream(Authentication authentication) {
AppUser user = resolveUser(authentication);
return sseEmitterRegistry.register(user.getId());
}
@GetMapping("/api/notifications")
public Page<NotificationDTO> getNotifications(
@RequestParam(defaultValue = "0") int page,
@RequestParam(defaultValue = "10") int size,
Authentication authentication) {
AppUser user = resolveUser(authentication);
PageRequest pageable = PageRequest.of(page, size, Sort.by("createdAt").descending());
return notificationService.getNotifications(user.getId(), pageable);
}
@GetMapping("/api/notifications/unread-count")
public Map<String, Long> countUnread(Authentication authentication) {
AppUser user = resolveUser(authentication);
return Map.of("count", notificationService.countUnread(user.getId()));
}
@PostMapping("/api/notifications/read-all")
@ResponseStatus(HttpStatus.NO_CONTENT)
public void markAllRead(Authentication authentication) {
AppUser user = resolveUser(authentication);
notificationService.markAllRead(user.getId());
}
@PatchMapping("/api/notifications/{id}/read")
public NotificationDTO markOneRead(
@PathVariable UUID id,
Authentication authentication) {
AppUser user = resolveUser(authentication);
return notificationService.markRead(id, user.getId());
}
@GetMapping("/api/users/me/notification-preferences")
@RequirePermission({Permission.READ_ALL, Permission.WRITE_ALL, Permission.ANNOTATE_ALL})
public NotificationPreferenceDTO getPreferences(Authentication authentication) {
AppUser user = resolveUser(authentication);
return new NotificationPreferenceDTO(user.isNotifyOnReply(), user.isNotifyOnMention());
}
@PutMapping("/api/users/me/notification-preferences")
@RequirePermission({Permission.READ_ALL, Permission.WRITE_ALL, Permission.ANNOTATE_ALL})
public NotificationPreferenceDTO updatePreferences(
@RequestBody NotificationPreferenceDTO dto,
Authentication authentication) {
AppUser user = resolveUser(authentication);
AppUser updated = notificationService.updatePreferences(
user.getId(), dto.notifyOnReply(), dto.notifyOnMention());
return new NotificationPreferenceDTO(updated.isNotifyOnReply(), updated.isNotifyOnMention());
}
// ─── private helpers ──────────────────────────────────────────────────────
private AppUser resolveUser(Authentication authentication) {
return userService.findByUsername(authentication.getName());
}
}

32
backend/src/main/java/org/raddatz/familienarchiv/controller/UserSearchController.java Normal file
View File

@@ -0,0 +1,32 @@
package org.raddatz.familienarchiv.controller;
marcel commented 2026-03-27 23:34:55 +01:00
Review
Copy Link

🚨 BLOCKER — User enumeration endpoint has no permission check

GET /api/users/search?q= exposes full user list (name + UUID) to any authenticated session with zero permission check. An attacker with any valid cookie can enumerate all users by iterating single-character queries (?q=a, ?q=b, …). For a family archive this is a real privacy violation.

Fix: Add @RequirePermission(Permission.READ_ALL) at the controller class level, consistent with every other controller in this codebase. The UserSearchControllerTest must then also test the 403 path for a user without permissions.

🚨 **BLOCKER — User enumeration endpoint has no permission check** `GET /api/users/search?q=` exposes full user list (name + UUID) to any authenticated session with zero permission check. An attacker with any valid cookie can enumerate all users by iterating single-character queries (`?q=a`, `?q=b`, …). For a family archive this is a real privacy violation. **Fix:** Add `@RequirePermission(Permission.READ_ALL)` at the controller class level, consistent with every other controller in this codebase. The `UserSearchControllerTest` must then also test the 403 path for a user without permissions.
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;
import org.springframework.web.bind.annotation.RestController;
import java.util.List;
@RestController
@RequiredArgsConstructor
@RequirePermission({Permission.READ_ALL, Permission.WRITE_ALL, Permission.ANNOTATE_ALL})
public class UserSearchController {
private final UserSearchService userSearchService;
@GetMapping("/api/users/search")
public List<MentionDTO> search(@RequestParam(defaultValue = "") String q) {
return userSearchService.search(q).stream()
.map(this::toMentionDTO)
.toList();
}
private MentionDTO toMentionDTO(AppUser user) {
return new MentionDTO(user.getId(), user.getFirstName(), user.getLastName());
}
}

5
backend/src/main/java/org/raddatz/familienarchiv/dto/CreateCommentDTO.java
View File

@@ -2,7 +2,12 @@ package org.raddatz.familienarchiv.dto;
import lombok.Data;
import java.util.ArrayList;
import java.util.List;
import java.util.UUID;
@Data
public class CreateCommentDTO {
private String content;
private List<UUID> mentionedUserIds = new ArrayList<>();
}

11
backend/src/main/java/org/raddatz/familienarchiv/dto/MentionDTO.java Normal file
View File

@@ -0,0 +1,11 @@
package org.raddatz.familienarchiv.dto;
import io.swagger.v3.oas.annotations.media.Schema;
import java.util.UUID;
public record MentionDTO(
@Schema(requiredMode = Schema.RequiredMode.REQUIRED) UUID id,
@Schema(requiredMode = Schema.RequiredMode.REQUIRED) String firstName,
@Schema(requiredMode = Schema.RequiredMode.REQUIRED) String lastName
) {}

18
backend/src/main/java/org/raddatz/familienarchiv/dto/NotificationDTO.java Normal file
View File

@@ -0,0 +1,18 @@
package org.raddatz.familienarchiv.dto;
import io.swagger.v3.oas.annotations.media.Schema;
import org.raddatz.familienarchiv.model.NotificationType;
import java.time.LocalDateTime;
import java.util.UUID;
public record NotificationDTO(
@Schema(requiredMode = Schema.RequiredMode.REQUIRED) UUID id,
@Schema(requiredMode = Schema.RequiredMode.REQUIRED) NotificationType type,
UUID documentId,
UUID referenceId,
UUID annotationId,
@Schema(requiredMode = Schema.RequiredMode.REQUIRED) boolean read,
@Schema(requiredMode = Schema.RequiredMode.REQUIRED) LocalDateTime createdAt,
String actorName
) {}

3
backend/src/main/java/org/raddatz/familienarchiv/dto/NotificationPreferenceDTO.java Normal file
View File

@@ -0,0 +1,3 @@
package org.raddatz.familienarchiv.dto;
public record NotificationPreferenceDTO(boolean notifyOnReply, boolean notifyOnMention) {}

4
backend/src/main/java/org/raddatz/familienarchiv/exception/ErrorCode.java
View File

@@ -50,6 +50,10 @@ public enum ErrorCode {
/** The comment with the given ID does not exist. 404 */
COMMENT_NOT_FOUND,
// --- Notifications ---
/** The notification with the given ID does not exist. 404 */
NOTIFICATION_NOT_FOUND,
// --- Generic ---
/** Request validation failed (missing or malformed fields). 400 */
VALIDATION_ERROR,

10
backend/src/main/java/org/raddatz/familienarchiv/model/AppUser.java
View File

@@ -51,6 +51,16 @@ public class AppUser {
@Schema(requiredMode = Schema.RequiredMode.REQUIRED)
private boolean enabled = true; // Um User zu sperren ohne sie zu löschen
@Column(nullable = false)
@Builder.Default
@Schema(requiredMode = Schema.RequiredMode.REQUIRED)
private boolean notifyOnReply = false;
@Column(nullable = false)
@Builder.Default
@Schema(requiredMode = Schema.RequiredMode.REQUIRED)
private boolean notifyOnMention = false;
// Ein User kann in mehreren Gruppen sein
@ManyToMany(fetch = FetchType.EAGER)
@JoinTable(name = "users_groups", joinColumns = @JoinColumn(name = "user_id"), inverseJoinColumns = @JoinColumn(name = "group_id"))

19
backend/src/main/java/org/raddatz/familienarchiv/model/DocumentComment.java
View File

@@ -1,10 +1,12 @@
package org.raddatz.familienarchiv.model;
marcel commented 2026-03-27 23:34:57 +01:00
Review
Copy Link

⚠️ MAJOR — @Transient field + FetchType.LAZY = potential LazyInitializationException at runtime

mentionDTOs is a @Transient field that is populated by the service before returning the entity. mentions is mapped as FetchType.LAZY. Read methods in CommentService are intentionally non-@Transactional (per architecture rules) — which means calling comment.getMentions() from withMentionDTOs() outside a transaction will trigger a LazyInitializationException.

This will not show up in unit tests (Mockito) but will explode in the integration test or production when the entity is fetched fresh from the repo.

Fix: Either change mentions to FetchType.EAGER (acceptable for a small bounded collection like mentions), or — better — introduce a proper CommentResponseDTO so you're not mutating a JPA entity with transient display state.

⚠️ **MAJOR — `@Transient` field + `FetchType.LAZY` = potential `LazyInitializationException` at runtime** `mentionDTOs` is a `@Transient` field that is populated by the service before returning the entity. `mentions` is mapped as `FetchType.LAZY`. Read methods in `CommentService` are intentionally non-`@Transactional` (per architecture rules) — which means calling `comment.getMentions()` from `withMentionDTOs()` outside a transaction will trigger a `LazyInitializationException`. This will not show up in unit tests (Mockito) but will explode in the integration test or production when the entity is fetched fresh from the repo. **Fix:** Either change `mentions` to `FetchType.EAGER` (acceptable for a small bounded collection like mentions), or — better — introduce a proper `CommentResponseDTO` so you're not mutating a JPA entity with transient display state.
import com.fasterxml.jackson.annotation.JsonIgnore;
import io.swagger.v3.oas.annotations.media.Schema;
import jakarta.persistence.*;
import lombok.*;
import org.hibernate.annotations.CreationTimestamp;
import org.hibernate.annotations.UpdateTimestamp;
import org.raddatz.familienarchiv.dto.MentionDTO;
import java.time.LocalDateTime;
import java.util.ArrayList;
@@ -60,4 +62,21 @@ public class DocumentComment {
@Builder.Default
@Schema(requiredMode = Schema.RequiredMode.REQUIRED)
private List<DocumentComment> replies = new ArrayList<>();
// JPA join table for structured mention references — not serialized directly
@ManyToMany(fetch = FetchType.EAGER)
@JoinTable(
name = "comment_mentions",
joinColumns = @JoinColumn(name = "comment_id"),
inverseJoinColumns = @JoinColumn(name = "user_id")
)
@JsonIgnore
@Builder.Default
private List<AppUser> mentions = new ArrayList<>();
// Populated by CommentService before serialization — not persisted.
@Transient
@Builder.Default
@Schema(requiredMode = Schema.RequiredMode.REQUIRED)
private List<MentionDTO> mentionDTOs = new ArrayList<>();
}

55
backend/src/main/java/org/raddatz/familienarchiv/model/Notification.java Normal file
View File

@@ -0,0 +1,55 @@
package org.raddatz.familienarchiv.model;
import io.swagger.v3.oas.annotations.media.Schema;
import jakarta.persistence.*;
import lombok.*;
import org.hibernate.annotations.CreationTimestamp;
import java.time.LocalDateTime;
import java.util.UUID;
@Entity
@Table(name = "notifications")
@Data
@NoArgsConstructor
@AllArgsConstructor
@Builder
public class Notification {
@Id
@GeneratedValue(strategy = GenerationType.UUID)
@Schema(requiredMode = Schema.RequiredMode.REQUIRED)
private UUID id;
@ManyToOne(fetch = FetchType.LAZY)
@JoinColumn(name = "recipient_id", nullable = false)
@Schema(requiredMode = Schema.RequiredMode.REQUIRED)
private AppUser recipient;
@Enumerated(EnumType.STRING)
@Column(nullable = false)
@Schema(requiredMode = Schema.RequiredMode.REQUIRED)
private NotificationType type;
@Column(name = "document_id")
private UUID documentId;
@Column(name = "reference_id")
private UUID referenceId;
@Column(name = "annotation_id")
private UUID annotationId;
@Column(nullable = false)
@Builder.Default
@Schema(requiredMode = Schema.RequiredMode.REQUIRED)
private boolean read = false;
@CreationTimestamp
@Schema(requiredMode = Schema.RequiredMode.REQUIRED)
private LocalDateTime createdAt;
@Column(name = "actor_name")
@Schema(requiredMode = Schema.RequiredMode.REQUIRED)
marcel commented 2026-03-27 22:11:31 +01:00
Review
Copy Link

Bug: actorName is never set — will always be null in every API response.

actorName is @Transient (not persisted), so it must be populated before serialization. But neither notifyReply nor notifyMentions in NotificationService ever calls .actorName(...) on the builder. The frontend renders notification.actorName directly in the bell dropdown — users will see an empty string where the sender's name should be.

Fix in notifyReply:

.actorName(reply.getAuthorName())

Fix in notifyMentions:

.actorName(comment.getAuthorName())

For getNotifications() (the paginated fetch), stored notifications already lack actorName, so you'll also need to re-hydrate it from the comment or store it as a plain column — storing it as a VARCHAR column in the migration is the simplest option, since authorName on DocumentComment is already a denormalized string.

**Bug: `actorName` is never set — will always be `null` in every API response.** `actorName` is `@Transient` (not persisted), so it must be populated before serialization. But neither `notifyReply` nor `notifyMentions` in `NotificationService` ever calls `.actorName(...)` on the builder. The frontend renders `notification.actorName` directly in the bell dropdown — users will see an empty string where the sender's name should be. Fix in `notifyReply`: ```java .actorName(reply.getAuthorName()) ``` Fix in `notifyMentions`: ```java .actorName(comment.getAuthorName()) ``` For `getNotifications()` (the paginated fetch), stored notifications already lack `actorName`, so you'll also need to re-hydrate it from the comment or store it as a plain column — storing it as a `VARCHAR` column in the migration is the simplest option, since `authorName` on `DocumentComment` is already a denormalized string.
private String actorName;
}

6
backend/src/main/java/org/raddatz/familienarchiv/model/NotificationType.java Normal file
View File

@@ -0,0 +1,6 @@
package org.raddatz.familienarchiv.model;
public enum NotificationType {
REPLY,
MENTION
}

10
backend/src/main/java/org/raddatz/familienarchiv/repository/AppUserRepository.java
View File

@@ -1,10 +1,13 @@
package org.raddatz.familienarchiv.repository;
import org.raddatz.familienarchiv.model.AppUser;
import org.springframework.data.domain.Pageable;
import org.springframework.data.jpa.repository.JpaRepository;
import org.springframework.data.jpa.repository.Query;
import org.springframework.data.repository.query.Param;
import org.springframework.stereotype.Repository;
import java.util.List;
import java.util.Optional;
import java.util.UUID;
@@ -12,4 +15,9 @@ import java.util.UUID;
public interface AppUserRepository extends JpaRepository<AppUser, UUID> {
Optional<AppUser> findByUsername(String username);
Optional<AppUser> findByEmail(String email);
@Query("SELECT u FROM AppUser u WHERE " +
"LOWER(COALESCE(u.firstName, '') || ' ' || COALESCE(u.lastName, '')) LIKE LOWER(CONCAT('%', :q, '%')) " +
"OR LOWER(u.username) LIKE LOWER(CONCAT('%', :q, '%'))")
List<AppUser> searchByNameOrUsername(@Param("q") String q, Pageable pageable);
}

22
backend/src/main/java/org/raddatz/familienarchiv/repository/NotificationRepository.java Normal file
View File

@@ -0,0 +1,22 @@
package org.raddatz.familienarchiv.repository;
marcel commented 2026-03-27 23:35:11 +01:00
Review
Copy Link

ℹ️ INFO — Unused method: findByRecipientIdOrderByCreatedAtDesc

The non-paginated List<Notification> findByRecipientIdOrderByCreatedAtDesc(UUID) is declared but never called — the paginated findByRecipientId(UUID, Pageable) is used exclusively. Dead code in repositories adds noise and risks someone accidentally calling the unbounded query on a user with thousands of notifications.

Fix: Remove the method.

ℹ️ **INFO — Unused method: `findByRecipientIdOrderByCreatedAtDesc`** The non-paginated `List<Notification> findByRecipientIdOrderByCreatedAtDesc(UUID)` is declared but never called — the paginated `findByRecipientId(UUID, Pageable)` is used exclusively. Dead code in repositories adds noise and risks someone accidentally calling the unbounded query on a user with thousands of notifications. **Fix:** Remove the method.
import org.raddatz.familienarchiv.model.Notification;
import org.springframework.data.domain.Page;
import org.springframework.data.domain.Pageable;
import org.springframework.data.jpa.repository.JpaRepository;
import org.springframework.data.jpa.repository.Modifying;
import org.springframework.data.jpa.repository.Query;
import org.springframework.data.repository.query.Param;
import java.util.UUID;
public interface NotificationRepository extends JpaRepository<Notification, UUID> {
Page<Notification> findByRecipientIdOrderByCreatedAtDesc(UUID recipientId, Pageable pageable);
long countByRecipientIdAndReadFalse(UUID recipientId);
@Modifying
@Query("UPDATE Notification n SET n.read = true WHERE n.recipient.id = :userId")
void markAllReadByRecipientId(@Param("userId") UUID userId);
}

17
backend/src/main/java/org/raddatz/familienarchiv/security/PermissionAspect.java
View File

@@ -23,7 +23,7 @@ public class PermissionAspect {
RequirePermission permission = getAnnotation(joinPoint);
if (permission != null) {
validateUserAccess(permission.value());
validateUserAccess(permission.value()); // value() is now Permission[]
}
return joinPoint.proceed();
@@ -43,18 +43,23 @@ public class PermissionAspect {
return joinPoint.getTarget().getClass().getAnnotation(RequirePermission.class);
}
private void validateUserAccess(Permission requiredPerm) {
private void validateUserAccess(Permission[] requiredPerms) {
Authentication auth = SecurityContextHolder.getContext().getAuthentication();
if (auth == null || !auth.isAuthenticated()) {
throw DomainException.unauthorized("Not authenticated");
}
boolean hasPermission = auth.getAuthorities().stream()
.anyMatch(a -> a.getAuthority().equals(requiredPerm.name()));
boolean hasAny = auth.getAuthorities().stream()
.anyMatch(a -> {
for (Permission p : requiredPerms) {
if (a.getAuthority().equals(p.name())) return true;
}
return false;
});
if (!hasPermission) {
throw DomainException.forbidden("Missing required permission: " + requiredPerm.name());
if (!hasAny) {
throw DomainException.forbidden("Missing required permission");
}
}
}

2
backend/src/main/java/org/raddatz/familienarchiv/security/RequirePermission.java
View File

@@ -8,5 +8,5 @@ import java.lang.annotation.Target;
@Target({ElementType.METHOD, ElementType.TYPE})
@Retention(RetentionPolicy.RUNTIME)
public @interface RequirePermission {
Permission value(); // e.g. "ADMIN" or "WRITE_ALL"
Permission[] value(); // one or more — user needs any of the listed permissions
}

67
backend/src/main/java/org/raddatz/familienarchiv/service/CommentService.java
View File

@@ -1,6 +1,7 @@
package org.raddatz.familienarchiv.service;
marcel commented 2026-03-27 23:34:54 +01:00
Review
Copy Link

🚨 BLOCKER — Architecture violation: direct repository access across domain boundary

CommentService directly injects AppUserRepository in saveMentions(). CLAUDE.md is explicit: services must never reach into another domain's repository. Cross-domain data access must go through the owning service.

This breaks the layering contract and untestable in isolation through the intended interface — any CommentServiceTest now has to mock a repository it shouldn't even know about.

Fix: Add UserService.findAllById(Collection<UUID>), inject UserService into CommentService, and call that instead.

🚨 **BLOCKER — Architecture violation: direct repository access across domain boundary** `CommentService` directly injects `AppUserRepository` in `saveMentions()`. CLAUDE.md is explicit: services must never reach into another domain's repository. Cross-domain data access must go through the owning service. This breaks the layering contract and untestable in isolation through the intended interface — any `CommentServiceTest` now has to mock a repository it shouldn't even know about. **Fix:** Add `UserService.findAllById(Collection<UUID>)`, inject `UserService` into `CommentService`, and call that instead.
import lombok.RequiredArgsConstructor;
import org.raddatz.familienarchiv.dto.MentionDTO;
import org.raddatz.familienarchiv.exception.DomainException;
import org.raddatz.familienarchiv.exception.ErrorCode;
import org.raddatz.familienarchiv.model.AppUser;
@@ -9,7 +10,9 @@ 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
@@ -17,20 +20,23 @@ import java.util.UUID;
public class CommentService {
marcel commented 2026-03-27 22:11:37 +01:00
Review
Copy Link

Layering violation: CommentService directly injects AppUserRepository.

From CLAUDE.md (strictly enforced):

Services never reach into another domain's repository. Call the other domain's service instead.

AppUserRepository belongs to the user domain. CommentService should inject UserService and call a method like userService.findAllById(mentionedUserIds). If UserService doesn't expose that method yet, add it there.

Same violation exists in NotificationService, which injects both AppUserRepository and CommentRepository. NotificationService is a new service so there's more latitude, but it should still go through UserService for user lookups and CommentService for comment lookups rather than reaching into their repositories directly.

**Layering violation: `CommentService` directly injects `AppUserRepository`.** From CLAUDE.md (strictly enforced): > Services never reach into another domain's repository. Call the other domain's service instead. `AppUserRepository` belongs to the user domain. `CommentService` should inject `UserService` and call a method like `userService.findAllById(mentionedUserIds)`. If `UserService` doesn't expose that method yet, add it there. Same violation exists in `NotificationService`, which injects both `AppUserRepository` and `CommentRepository`. `NotificationService` is a new service so there's more latitude, but it should still go through `UserService` for user lookups and `CommentService` for comment lookups rather than reaching into their repositories directly.
private final CommentRepository commentRepository;
private final UserService userService;
private final NotificationService notificationService;
public List<DocumentComment> getCommentsForDocument(UUID documentId) {
List<DocumentComment> roots =
commentRepository.findByDocumentIdAndAnnotationIdIsNullAndParentIdIsNull(documentId);
return withReplies(roots);
return withRepliesAndMentions(roots);
}
public List<DocumentComment> getCommentsForAnnotation(UUID annotationId) {
List<DocumentComment> roots = commentRepository.findByAnnotationIdAndParentIdIsNull(annotationId);
return withReplies(roots);
return withRepliesAndMentions(roots);
}
@Transactional
public DocumentComment postComment(UUID documentId, UUID annotationId, String content, AppUser author) {
public DocumentComment postComment(UUID documentId, UUID annotationId, String content,
List<UUID> mentionedUserIds, AppUser author) {
DocumentComment comment = DocumentComment.builder()
.documentId(documentId)
.annotationId(annotationId)
@@ -38,11 +44,16 @@ public class CommentService {
.authorId(author.getId())
.authorName(resolveAuthorName(author))
.build();
return commentRepository.save(comment);
saveMentions(comment, mentionedUserIds);
DocumentComment saved = commentRepository.save(comment);
withMentionDTOs(saved);
notificationService.notifyMentions(mentionedUserIds, saved);
return saved;
}
@Transactional
public DocumentComment replyToComment(UUID documentId, UUID commentId, String content, AppUser author) {
public DocumentComment replyToComment(UUID documentId, UUID commentId, String content,
List<UUID> mentionedUserIds, AppUser author) {
DocumentComment target = commentRepository.findById(commentId)
.orElseThrow(() -> DomainException.notFound(
ErrorCode.COMMENT_NOT_FOUND, "Comment not found: " + commentId));
@@ -60,7 +71,15 @@ public class CommentService {
.authorId(author.getId())
.authorName(resolveAuthorName(author))
.build();
return commentRepository.save(reply);
saveMentions(reply, mentionedUserIds);
DocumentComment saved = commentRepository.save(reply);
withMentionDTOs(saved);
Set<UUID> participantIds = collectParticipantIds(root);
participantIds.remove(author.getId());
notificationService.notifyReply(saved, participantIds);
notificationService.notifyMentions(mentionedUserIds, saved);
return saved;
}
@Transactional
@@ -84,13 +103,45 @@ public class CommentService {
commentRepository.delete(comment);
}
public List<DocumentComment> findReplies(UUID parentId) {
return commentRepository.findByParentId(parentId);
}
// ─── private helpers ──────────────────────────────────────────────────────
private List<DocumentComment> withReplies(List<DocumentComment> roots) {
roots.forEach(root -> root.setReplies(commentRepository.findByParentId(root.getId())));
private List<DocumentComment> withRepliesAndMentions(List<DocumentComment> roots) {
roots.forEach(root -> {
List<DocumentComment> replies = commentRepository.findByParentId(root.getId());
replies.forEach(this::withMentionDTOs);
root.setReplies(replies);
withMentionDTOs(root);
});
return roots;
}
private void saveMentions(DocumentComment comment, List<UUID> mentionedUserIds) {
if (mentionedUserIds == null || mentionedUserIds.isEmpty()) return;
List<AppUser> users = userService.findAllById(mentionedUserIds);
comment.setMentions(users);
}
private void withMentionDTOs(DocumentComment comment) {
List<MentionDTO> dtos = comment.getMentions().stream()
.map(u -> new MentionDTO(u.getId(), u.getFirstName(), u.getLastName()))
.toList();
comment.setMentionDTOs(dtos);
}
private Set<UUID> collectParticipantIds(DocumentComment root) {
Set<UUID> 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()))

187
backend/src/main/java/org/raddatz/familienarchiv/service/NotificationService.java Normal file
View File

@@ -0,0 +1,187 @@
package org.raddatz.familienarchiv.service;
marcel commented 2026-03-27 23:34:55 +01:00
Review
Copy Link

🚨 BLOCKER — Fragile mixed injection + ReflectionTestUtils hack

The class uses @RequiredArgsConstructor but mailSender is injected via @Autowired(required = false) — a mixed injection approach. The test is forced to use ReflectionTestUtils.setField() to inject the mock. This is a maintenance trap: if mailSender is ever renamed, the reflection call silently breaks at runtime with no compile-time error — the email path stops being tested without any CI failure.

Fix: Inject Optional<JavaMailSender> as a constructor parameter (works with Lombok @RequiredArgsConstructor) or use @ConditionalOnBean(JavaMailSender.class) on a separate email-sending component. Either approach is injection-safe and doesn't require reflection in tests.

🚨 **BLOCKER — Fragile mixed injection + ReflectionTestUtils hack** The class uses `@RequiredArgsConstructor` but `mailSender` is injected via `@Autowired(required = false)` — a mixed injection approach. The test is forced to use `ReflectionTestUtils.setField()` to inject the mock. This is a maintenance trap: if `mailSender` is ever renamed, the reflection call silently breaks at runtime with no compile-time error — the email path stops being tested without any CI failure. **Fix:** Inject `Optional<JavaMailSender>` as a constructor parameter (works with Lombok `@RequiredArgsConstructor`) or use `@ConditionalOnBean(JavaMailSender.class)` on a separate email-sending component. Either approach is injection-safe and doesn't require reflection in tests.
marcel commented 2026-03-27 23:34:57 +01:00
Review
Copy Link

⚠️ MAJOR — Notification failure can silently roll back the parent comment

notifyReply() and notifyMentions() are @Transactional and are called from within the already-@Transactional replyToComment() / postComment() in CommentService. They join the outer transaction. If a notification save fails (DB constraint, unexpected null), it rolls back the entire outer transaction — the comment itself is lost.

A user's comment should never disappear because a notification couldn't be persisted.

Fix: Annotate both methods with @Transactional(propagation = Propagation.REQUIRES_NEW) to isolate notification failures, or use @Async to fire them outside the comment transaction entirely.

⚠️ **MAJOR — Notification failure can silently roll back the parent comment** `notifyReply()` and `notifyMentions()` are `@Transactional` and are called from within the already-`@Transactional` `replyToComment()` / `postComment()` in `CommentService`. They join the outer transaction. If a notification save fails (DB constraint, unexpected null), it rolls back the entire outer transaction — the comment itself is lost. A user's comment should never disappear because a notification couldn't be persisted. **Fix:** Annotate both methods with `@Transactional(propagation = Propagation.REQUIRES_NEW)` to isolate notification failures, or use `@Async` to fire them outside the comment transaction entirely.
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.NotificationRepository;
import org.springframework.beans.factory.annotation.Value;
import org.springframework.data.domain.Page;
import org.springframework.data.domain.Pageable;
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.List;
import java.util.Optional;
import java.util.Set;
import java.util.UUID;
@Service
@RequiredArgsConstructor
@Slf4j
public class NotificationService {
private final NotificationRepository notificationRepository;
private final UserService userService;
private final Optional<JavaMailSender> mailSender;
private final SseEmitterRegistry sseEmitterRegistry;
@Value("${app.mail.from:noreply@familienarchiv.local}")
private String mailFrom;
@Value("${app.base-url:http://localhost:3000}")
private String baseUrl;
/**
* 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(propagation = Propagation.REQUIRES_NEW)
public void notifyReply(DocumentComment reply, Set<UUID> participantIds) {
if (participantIds.isEmpty()) return;
List<AppUser> recipients = userService.findAllById(participantIds);
for (AppUser recipient : recipients) {
Notification notification = Notification.builder()
.recipient(recipient)
.type(NotificationType.REPLY)
.documentId(reply.getDocumentId())
.referenceId(reply.getId())
marcel commented 2026-03-27 22:11:31 +01:00
Outdated
Review
Copy Link

N+1: findById inside a loop.

For every participant ID you're issuing a separate SELECT. If a thread has 10 participants that's 10 round-trips to the database before a single notification is written.

Replace the loop body with a single bulk fetch:

public void notifyReply(DocumentComment reply, DocumentComment root) {
    Set<UUID> participantIds = collectParticipantIds(root);
    participantIds.remove(reply.getAuthorId());
    if (participantIds.isEmpty()) return;

    List<AppUser> recipients = userRepository.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);
        if (recipient.isNotifyOnReply()) {
            sendNotificationEmail(recipient, reply, NotificationType.REPLY);
        }
    }
}

Same pattern applies to notifyMentions below.

**N+1: `findById` inside a loop.** For every participant ID you're issuing a separate SELECT. If a thread has 10 participants that's 10 round-trips to the database before a single notification is written. Replace the loop body with a single bulk fetch: ```java public void notifyReply(DocumentComment reply, DocumentComment root) { Set<UUID> participantIds = collectParticipantIds(root); participantIds.remove(reply.getAuthorId()); if (participantIds.isEmpty()) return; List<AppUser> recipients = userRepository.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); if (recipient.isNotifyOnReply()) { sendNotificationEmail(recipient, reply, NotificationType.REPLY); } } } ``` Same pattern applies to `notifyMentions` below.
.annotationId(reply.getAnnotationId())
.actorName(reply.getAuthorName())
.build();
saveAndPush(notification);
if (recipient.isNotifyOnReply()) {
sendNotificationEmail(recipient, reply, NotificationType.REPLY);
}
}
}
/**
* Creates MENTION notifications for each mentioned user.
* Runs in a separate transaction so a notification failure cannot roll back the parent comment.
*/
@Transactional(propagation = Propagation.REQUIRES_NEW)
public void notifyMentions(List<UUID> mentionedUserIds, DocumentComment comment) {
if (mentionedUserIds == null || mentionedUserIds.isEmpty()) return;
List<AppUser> recipients = userService.findAllById(mentionedUserIds);
for (AppUser recipient : recipients) {
Notification notification = Notification.builder()
.recipient(recipient)
.type(NotificationType.MENTION)
marcel commented 2026-03-27 22:11:32 +01:00
Review
Copy Link

N+1: same issue as notifyReply — findById in a loop.

For notifyMentions:

List<AppUser> recipients = userRepository.findAllById(mentionedUserIds);
for (AppUser recipient : recipients) { ... }

findAllById emits a single SELECT … WHERE id IN (…) instead of one query per user.

**N+1: same issue as `notifyReply` — `findById` in a loop.** For `notifyMentions`: ```java List<AppUser> recipients = userRepository.findAllById(mentionedUserIds); for (AppUser recipient : recipients) { ... } ``` `findAllById` emits a single `SELECT … WHERE id IN (…)` instead of one query per user.
.documentId(comment.getDocumentId())
.referenceId(comment.getId())
.annotationId(comment.getAnnotationId())
.actorName(comment.getAuthorName())
.build();
saveAndPush(notification);
if (recipient.isNotifyOnMention()) {
sendNotificationEmail(recipient, comment, NotificationType.MENTION);
}
}
}
public Page<NotificationDTO> getNotifications(UUID userId, Pageable pageable) {
return notificationRepository.findByRecipientIdOrderByCreatedAtDesc(userId, pageable)
.map(this::toDTO);
}
public long countUnread(UUID userId) {
return notificationRepository.countByRecipientIdAndReadFalse(userId);
}
@Transactional
public void markAllRead(UUID userId) {
notificationRepository.markAllReadByRecipientId(userId);
}
@Transactional
public NotificationDTO markRead(UUID notificationId, UUID userId) {
Notification notification = notificationRepository.findById(notificationId)
.orElseThrow(() -> DomainException.notFound(
ErrorCode.NOTIFICATION_NOT_FOUND, "Notification not found: " + notificationId));
if (!notification.getRecipient().getId().equals(userId)) {
throw DomainException.forbidden("Notification belongs to a different user");
}
notification.setRead(true);
return toDTO(notificationRepository.save(notification));
}
@Transactional
public AppUser updatePreferences(UUID userId, boolean notifyOnReply, boolean notifyOnMention) {
return userService.updateNotificationPreferences(userId, notifyOnReply, notifyOnMention);
}
// ─── private helpers ──────────────────────────────────────────────────────
private void saveAndPush(Notification notification) {
Notification saved = notificationRepository.save(notification);
sseEmitterRegistry.send(saved.getRecipient().getId(), toDTO(saved));
}
private NotificationDTO toDTO(Notification n) {
return new NotificationDTO(
n.getId(),
n.getType(),
n.getDocumentId(),
n.getReferenceId(),
n.getAnnotationId(),
n.isRead(),
n.getCreatedAt(),
n.getActorName()
);
}
private void buildCommentPath(DocumentComment comment, StringBuilder sb) {
sb.append("?commentId=").append(comment.getId());
if (comment.getAnnotationId() != null) {
sb.append("&annotationId=").append(comment.getAnnotationId());
}
}
private void sendNotificationEmail(AppUser recipient, DocumentComment comment, NotificationType type) {
if (mailSender.isEmpty()) {
log.warn("Mail sender not configured — skipping notification email to {}", recipient.getEmail());
return;
}
if (recipient.getEmail() == null || recipient.getEmail().isBlank()) return;
StringBuilder path = new StringBuilder("/documents/").append(comment.getDocumentId());
buildCommentPath(comment, path);
String link = baseUrl + path;
String subject = type == NotificationType.REPLY
? "Neue Antwort auf deinen Kommentar — Familienarchiv"
: "Du wurdest in einem Kommentar erwähnt — Familienarchiv";
String body = type == NotificationType.REPLY
? "Hallo,\n\njemand hat auf einen Kommentar geantwortet, an dem du beteiligt warst.\n\n"
+ "Zum Kommentar:\n" + link + "\n\nDein Familienarchiv-Team"
: "Hallo,\n\njemand hat dich in einem Kommentar erwähnt.\n\n"
+ "Zum Kommentar:\n" + link + "\n\nDein Familienarchiv-Team";
SimpleMailMessage message = new SimpleMailMessage();
message.setFrom(mailFrom);
message.setTo(recipient.getEmail());
message.setSubject(subject);
message.setText(body);
try {
mailSender.get().send(message);
} catch (MailException e) {
log.error("Failed to send notification email to {}: {}", recipient.getEmail(), e.getMessage());
}
}
}

36
backend/src/main/java/org/raddatz/familienarchiv/service/SseEmitterRegistry.java Normal file
View File

@@ -0,0 +1,36 @@
package org.raddatz.familienarchiv.service;
import lombok.extern.slf4j.Slf4j;
import org.springframework.stereotype.Component;
import org.springframework.web.servlet.mvc.method.annotation.SseEmitter;
import java.io.IOException;
import java.util.UUID;
import java.util.concurrent.ConcurrentHashMap;
@Component
@Slf4j
public class SseEmitterRegistry {
private final ConcurrentHashMap<UUID, SseEmitter> emitters = new ConcurrentHashMap<>();
public SseEmitter register(UUID userId) {
SseEmitter emitter = new SseEmitter(0L); // 0 = no timeout; EventSource reconnects automatically
emitters.put(userId, emitter);
emitter.onCompletion(() -> emitters.remove(userId, emitter));
emitter.onTimeout(() -> emitters.remove(userId, emitter));
emitter.onError(e -> emitters.remove(userId, emitter));
return emitter;
}
public void send(UUID userId, Object data) {
SseEmitter emitter = emitters.get(userId);
if (emitter == null) return;
try {
emitter.send(SseEmitter.event().name("notification").data(data));
} catch (IOException e) {
log.debug("SSE send failed for user {} — removing emitter", userId);
emitters.remove(userId, emitter);
}
}
}

23
backend/src/main/java/org/raddatz/familienarchiv/service/UserSearchService.java Normal file
View File

@@ -0,0 +1,23 @@
package org.raddatz.familienarchiv.service;
import lombok.RequiredArgsConstructor;
import org.raddatz.familienarchiv.model.AppUser;
import org.raddatz.familienarchiv.repository.AppUserRepository;
import org.springframework.data.domain.PageRequest;
import org.springframework.stereotype.Service;
import java.util.List;
@Service
@RequiredArgsConstructor
public class UserSearchService {
private static final int MAX_RESULTS = 10;
private final AppUserRepository userRepository;
public List<AppUser> search(String query) {
if (query == null || query.isBlank()) return List.of();
return userRepository.searchByNameOrUsername(query.trim(), PageRequest.of(0, MAX_RESULTS));
}
}

13
backend/src/main/java/org/raddatz/familienarchiv/service/UserService.java
View File

@@ -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<AppUser> findAllById(Collection<UUID> 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);

18
backend/src/main/resources/db/migration/V16__notifications_and_preferences.sql Normal file
View File

@@ -0,0 +1,18 @@
-- Notification preferences on the user record — no separate entity needed
ALTER TABLE users ADD COLUMN notify_on_reply BOOLEAN NOT NULL DEFAULT false;
ALTER TABLE users ADD COLUMN notify_on_mention BOOLEAN NOT NULL DEFAULT false;
-- In-app notifications
CREATE TABLE notifications (
id UUID PRIMARY KEY DEFAULT gen_random_uuid(),
recipient_id UUID NOT NULL REFERENCES users(id) ON DELETE CASCADE,
type VARCHAR(32) NOT NULL, -- 'REPLY' | 'MENTION'
document_id UUID,
reference_id UUID, -- commentId that triggered this notification
annotation_id UUID,
read BOOLEAN NOT NULL DEFAULT false,
created_at TIMESTAMP NOT NULL DEFAULT now(),
actor_name VARCHAR(255)
);
CREATE INDEX idx_notifications_recipient ON notifications(recipient_id, read, created_at DESC);

5
backend/src/main/resources/db/migration/V17__comment_mentions.sql Normal file
View File

@@ -0,0 +1,5 @@
CREATE TABLE comment_mentions (
comment_id UUID NOT NULL REFERENCES document_comments(id) ON DELETE CASCADE,
user_id UUID NOT NULL REFERENCES users(id) ON DELETE CASCADE,
PRIMARY KEY (comment_id, user_id)
);

23
backend/src/test/java/org/raddatz/familienarchiv/controller/AnnotationControllerTest.java
View File

@@ -81,6 +81,29 @@ class AnnotationControllerTest {
.andExpect(status().isForbidden());
}
@Test
@WithMockUser(authorities = "WRITE_ALL")
void createAnnotation_returns201_whenHasWriteAllPermission() throws Exception {
UUID docId = UUID.randomUUID();
DocumentAnnotation saved = DocumentAnnotation.builder()
.id(UUID.randomUUID()).documentId(docId).pageNumber(1)
.x(0.1).y(0.1).width(0.2).height(0.2).color("#ff0000").build();
when(documentService.getDocumentById(any())).thenReturn(Document.builder().build());
when(annotationService.createAnnotation(any(), any(), any(), any())).thenReturn(saved);
mockMvc.perform(post("/api/documents/" + docId + "/annotations")
.contentType(MediaType.APPLICATION_JSON)
.content(ANNOTATION_JSON))
.andExpect(status().isCreated());
}
@Test
@WithMockUser(authorities = "WRITE_ALL")
void deleteAnnotation_returns204_whenHasWriteAllPermission() throws Exception {
mockMvc.perform(delete("/api/documents/" + UUID.randomUUID() + "/annotations/" + UUID.randomUUID()))
.andExpect(status().isNoContent());
}
@Test
@WithMockUser(authorities = "ANNOTATE_ALL")
void createAnnotation_returns201_whenHasAnnotatePermission() throws Exception {

71
backend/src/test/java/org/raddatz/familienarchiv/controller/CommentControllerTest.java
View File

@@ -81,7 +81,7 @@ class CommentControllerTest {
void postDocumentComment_returns201_whenHasPermission() throws Exception {
DocumentComment saved = DocumentComment.builder()
.id(COMMENT_ID).documentId(DOC_ID).authorName("Hans").content("Test comment").build();
when(commentService.postComment(any(), any(), any(), any())).thenReturn(saved);
when(commentService.postComment(any(), any(), any(), any(), any())).thenReturn(saved);
mockMvc.perform(post("/api/documents/" + DOC_ID + "/comments")
.contentType(MediaType.APPLICATION_JSON).content(COMMENT_JSON))
@@ -89,6 +89,18 @@ class CommentControllerTest {
.andExpect(jsonPath("$.content").value("Test comment"));
}
@Test
@WithMockUser(authorities = "WRITE_ALL")
void postDocumentComment_returns201_whenHasWriteAllPermission() throws Exception {
DocumentComment saved = DocumentComment.builder()
.id(COMMENT_ID).documentId(DOC_ID).authorName("Hans").content("Test comment").build();
when(commentService.postComment(any(), any(), any(), any(), any())).thenReturn(saved);
mockMvc.perform(post("/api/documents/" + DOC_ID + "/comments")
.contentType(MediaType.APPLICATION_JSON).content(COMMENT_JSON))
.andExpect(status().isCreated());
}
// ─── POST /api/documents/{documentId}/comments/{commentId}/replies ────────
@Test
@@ -104,7 +116,20 @@ class CommentControllerTest {
DocumentComment saved = DocumentComment.builder()
.id(UUID.randomUUID()).documentId(DOC_ID).parentId(COMMENT_ID)
.authorName("Anna").content("Test comment").build();
when(commentService.replyToComment(any(), any(), any(), any())).thenReturn(saved);
when(commentService.replyToComment(any(), any(), any(), any(), any())).thenReturn(saved);
mockMvc.perform(post("/api/documents/" + DOC_ID + "/comments/" + COMMENT_ID + "/replies")
.contentType(MediaType.APPLICATION_JSON).content(COMMENT_JSON))
.andExpect(status().isCreated());
}
@Test
@WithMockUser(authorities = "WRITE_ALL")
void replyToComment_returns201_whenHasWriteAllPermission() throws Exception {
DocumentComment saved = DocumentComment.builder()
.id(UUID.randomUUID()).documentId(DOC_ID).parentId(COMMENT_ID)
.authorName("Anna").content("Test comment").build();
when(commentService.replyToComment(any(), any(), any(), any(), any())).thenReturn(saved);
mockMvc.perform(post("/api/documents/" + DOC_ID + "/comments/" + COMMENT_ID + "/replies")
.contentType(MediaType.APPLICATION_JSON).content(COMMENT_JSON))
@@ -163,6 +188,18 @@ class CommentControllerTest {
.andExpect(status().isOk());
}
@Test
@WithMockUser(authorities = "WRITE_ALL")
void editComment_returns200_whenHasWriteAllPermission() throws Exception {
DocumentComment updated = DocumentComment.builder()
.id(COMMENT_ID).documentId(DOC_ID).authorName("Hans").content("Test comment").build();
when(commentService.editComment(any(), any(), any(), any())).thenReturn(updated);
mockMvc.perform(patch("/api/documents/" + DOC_ID + "/comments/" + COMMENT_ID)
.contentType(MediaType.APPLICATION_JSON).content(COMMENT_JSON))
.andExpect(status().isOk());
}
// ─── POST /api/documents/{documentId}/annotations/{annId}/comments ────────
@Test
@@ -179,7 +216,20 @@ class CommentControllerTest {
DocumentComment saved = DocumentComment.builder()
.id(UUID.randomUUID()).documentId(DOC_ID).annotationId(ANN_ID)
.authorName("Hans").content("Test comment").build();
when(commentService.postComment(any(), any(), any(), any())).thenReturn(saved);
when(commentService.postComment(any(), any(), any(), any(), any())).thenReturn(saved);
mockMvc.perform(post("/api/documents/" + DOC_ID + "/annotations/" + ANN_ID + "/comments")
.contentType(MediaType.APPLICATION_JSON).content(COMMENT_JSON))
.andExpect(status().isCreated());
}
@Test
@WithMockUser(authorities = "WRITE_ALL")
void postAnnotationComment_returns201_whenHasWriteAllPermission() throws Exception {
DocumentComment saved = DocumentComment.builder()
.id(UUID.randomUUID()).documentId(DOC_ID).annotationId(ANN_ID)
.authorName("Hans").content("Test comment").build();
when(commentService.postComment(any(), any(), any(), any(), any())).thenReturn(saved);
mockMvc.perform(post("/api/documents/" + DOC_ID + "/annotations/" + ANN_ID + "/comments")
.contentType(MediaType.APPLICATION_JSON).content(COMMENT_JSON))
@@ -194,7 +244,20 @@ class CommentControllerTest {
DocumentComment saved = DocumentComment.builder()
.id(UUID.randomUUID()).documentId(DOC_ID).annotationId(ANN_ID)
.parentId(COMMENT_ID).authorName("Anna").content("Test comment").build();
when(commentService.replyToComment(any(), any(), any(), any())).thenReturn(saved);
when(commentService.replyToComment(any(), any(), any(), any(), any())).thenReturn(saved);
mockMvc.perform(post("/api/documents/" + DOC_ID + "/annotations/" + ANN_ID + "/comments/" + COMMENT_ID + "/replies")
.contentType(MediaType.APPLICATION_JSON).content(COMMENT_JSON))
.andExpect(status().isCreated());
}
@Test
@WithMockUser(authorities = "WRITE_ALL")
void replyToAnnotationComment_returns201_whenHasWriteAllPermission() throws Exception {
DocumentComment saved = DocumentComment.builder()
.id(UUID.randomUUID()).documentId(DOC_ID).annotationId(ANN_ID)
.parentId(COMMENT_ID).authorName("Anna").content("Test comment").build();
when(commentService.replyToComment(any(), any(), any(), any(), any())).thenReturn(saved);
mockMvc.perform(post("/api/documents/" + DOC_ID + "/annotations/" + ANN_ID + "/comments/" + COMMENT_ID + "/replies")
.contentType(MediaType.APPLICATION_JSON).content(COMMENT_JSON))

306
backend/src/test/java/org/raddatz/familienarchiv/controller/NotificationControllerTest.java Normal file
View File

@@ -0,0 +1,306 @@
package org.raddatz.familienarchiv.controller;
marcel commented 2026-03-27 23:34:58 +01:00
Review
Copy Link

⚠️ MAJOR — PATCH /api/notifications/{id}/read missing 401 test

Every other endpoint in this controller has an unauthenticated (401) test. The markOneRead endpoint is the only one missing it. This inconsistency means a security regression specifically on this endpoint would not be caught by CI.

Fix: Add markOneRead_returns401_whenUnauthenticated() mirroring the pattern already used for getNotifications_returns401_whenUnauthenticated.

⚠️ **MAJOR — `PATCH /api/notifications/{id}/read` missing 401 test** Every other endpoint in this controller has an unauthenticated (401) test. The `markOneRead` endpoint is the only one missing it. This inconsistency means a security regression specifically on this endpoint would not be caught by CI. **Fix:** Add `markOneRead_returns401_whenUnauthenticated()` mirroring the pattern already used for `getNotifications_returns401_whenUnauthenticated`.
import org.junit.jupiter.api.Test;
import org.raddatz.familienarchiv.config.SecurityConfig;
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.NotificationType;
import org.raddatz.familienarchiv.security.PermissionAspect;
import org.raddatz.familienarchiv.service.CustomUserDetailsService;
import org.raddatz.familienarchiv.service.NotificationService;
import org.raddatz.familienarchiv.service.SseEmitterRegistry;
import org.raddatz.familienarchiv.service.UserService;
import org.springframework.beans.factory.annotation.Autowired;
import org.springframework.boot.autoconfigure.aop.AopAutoConfiguration;
import org.springframework.boot.webmvc.test.autoconfigure.WebMvcTest;
import org.springframework.context.annotation.Import;
import org.springframework.data.domain.PageImpl;
import org.springframework.data.domain.PageRequest;
import org.springframework.http.MediaType;
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;
import static org.mockito.ArgumentMatchers.any;
import static org.mockito.ArgumentMatchers.eq;
import static org.mockito.Mockito.doThrow;
import static org.mockito.Mockito.verify;
import static org.mockito.Mockito.when;
import static org.springframework.http.MediaType.TEXT_EVENT_STREAM_VALUE;
import static org.springframework.test.web.servlet.request.MockMvcRequestBuilders.*;
import static org.springframework.test.web.servlet.result.MockMvcResultMatchers.*;
@WebMvcTest(NotificationController.class)
@Import({SecurityConfig.class, PermissionAspect.class, AopAutoConfiguration.class})
class NotificationControllerTest {
@Autowired MockMvc mockMvc;
@MockitoBean NotificationService notificationService;
@MockitoBean UserService userService;
@MockitoBean SseEmitterRegistry sseEmitterRegistry;
@MockitoBean CustomUserDetailsService customUserDetailsService;
private static final UUID USER_ID = UUID.randomUUID();
// ─── GET /api/notifications ───────────────────────────────────────────────
@Test
void getNotifications_returns401_whenUnauthenticated() throws Exception {
mockMvc.perform(get("/api/notifications"))
.andExpect(status().isUnauthorized());
}
@Test
@WithMockUser(username = "testuser")
void getNotifications_returns200_whenAuthenticatedWithNoPermissions() throws Exception {
AppUser user = AppUser.builder().id(USER_ID).username("testuser").build();
when(userService.findByUsername("testuser")).thenReturn(user);
when(notificationService.getNotifications(eq(USER_ID), any()))
.thenReturn(new PageImpl<>(List.of()));
mockMvc.perform(get("/api/notifications"))
.andExpect(status().isOk());
}
@Test
@WithMockUser(username = "testuser", authorities = {"READ_ALL"})
void getNotifications_returns200WithList_whenAuthenticated() throws Exception {
AppUser user = AppUser.builder().id(USER_ID).username("testuser").build();
NotificationDTO dto = new NotificationDTO(
UUID.randomUUID(), NotificationType.REPLY, UUID.randomUUID(),
UUID.randomUUID(), null, false, LocalDateTime.now(), "Anna Smith");
when(userService.findByUsername("testuser")).thenReturn(user);
when(notificationService.getNotifications(eq(USER_ID), any()))
.thenReturn(new PageImpl<>(List.of(dto), PageRequest.of(0, 10), 1));
mockMvc.perform(get("/api/notifications"))
.andExpect(status().isOk())
.andExpect(jsonPath("$.content").isArray());
}
@Test
@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);
when(notificationService.getNotifications(eq(USER_ID), any()))
.thenReturn(new PageImpl<>(List.of()));
mockMvc.perform(get("/api/notifications"))
.andExpect(status().isOk());
verify(notificationService).getNotifications(eq(USER_ID), any());
}
// ─── POST /api/notifications/read-all ────────────────────────────────────
@Test
void markAllRead_returns401_whenUnauthenticated() throws Exception {
mockMvc.perform(post("/api/notifications/read-all"))
.andExpect(status().isUnauthorized());
}
@Test
@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);
mockMvc.perform(post("/api/notifications/read-all"))
.andExpect(status().isNoContent());
verify(notificationService).markAllRead(USER_ID);
}
// ─── PATCH /api/notifications/{id}/read ──────────────────────────────────
@Test
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();
when(userService.findByUsername("testuser")).thenReturn(user);
org.mockito.Mockito.doThrow(
org.raddatz.familienarchiv.exception.DomainException.forbidden("not yours"))
.when(notificationService).markRead(notifId, USER_ID);
mockMvc.perform(patch("/api/notifications/" + notifId + "/read"))
.andExpect(status().isForbidden());
}
// ─── GET /api/users/me/notification-preferences ──────────────────────────
@Test
void getPreferences_returns401_whenUnauthenticated() throws Exception {
mockMvc.perform(get("/api/users/me/notification-preferences"))
.andExpect(status().isUnauthorized());
}
@Test
@WithMockUser(username = "testuser")
void getPreferences_returns403_whenUserHasNoPermission() throws Exception {
mockMvc.perform(get("/api/users/me/notification-preferences"))
.andExpect(status().isForbidden());
}
@Test
@WithMockUser(username = "testuser", authorities = {"READ_ALL"})
void getPreferences_returns200_whenUserHasReadAll() throws Exception {
AppUser user = AppUser.builder().id(USER_ID).username("testuser")
.notifyOnReply(true).notifyOnMention(false).build();
when(userService.findByUsername("testuser")).thenReturn(user);
mockMvc.perform(get("/api/users/me/notification-preferences"))
.andExpect(status().isOk())
.andExpect(jsonPath("$.notifyOnReply").value(true))
.andExpect(jsonPath("$.notifyOnMention").value(false));
}
@Test
@WithMockUser(username = "testuser", authorities = {"WRITE_ALL"})
void getPreferences_returns200_whenUserHasWriteAll() throws Exception {
AppUser user = AppUser.builder().id(USER_ID).username("testuser")
.notifyOnReply(false).notifyOnMention(true).build();
when(userService.findByUsername("testuser")).thenReturn(user);
mockMvc.perform(get("/api/users/me/notification-preferences"))
.andExpect(status().isOk())
.andExpect(jsonPath("$.notifyOnMention").value(true));
}
@Test
@WithMockUser(username = "testuser", authorities = {"ANNOTATE_ALL"})
void getPreferences_returns200_whenUserHasAnnotateAll() throws Exception {
AppUser user = AppUser.builder().id(USER_ID).username("testuser")
.notifyOnReply(false).notifyOnMention(false).build();
when(userService.findByUsername("testuser")).thenReturn(user);
mockMvc.perform(get("/api/users/me/notification-preferences"))
.andExpect(status().isOk());
}
@Test
@WithMockUser(username = "testuser", authorities = {"WRITE_ALL"})
void getNotifications_returns200_whenUserHasOnlyWriteAll() throws Exception {
AppUser user = AppUser.builder().id(USER_ID).username("testuser").build();
when(userService.findByUsername("testuser")).thenReturn(user);
when(notificationService.getNotifications(eq(USER_ID), any()))
.thenReturn(new PageImpl<>(List.of()));
mockMvc.perform(get("/api/notifications"))
.andExpect(status().isOk());
}
// ─── PUT /api/users/me/notification-preferences ──────────────────────────
@Test
@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();
when(userService.findByUsername("testuser")).thenReturn(user);
AppUser updated = AppUser.builder().id(USER_ID).username("testuser")
.notifyOnReply(true).notifyOnMention(true).build();
when(notificationService.updatePreferences(USER_ID, true, true)).thenReturn(updated);
mockMvc.perform(put("/api/users/me/notification-preferences")
.contentType(MediaType.APPLICATION_JSON)
.content("{\"notifyOnReply\":true,\"notifyOnMention\":true}"))
.andExpect(status().isOk())
.andExpect(jsonPath("$.notifyOnReply").value(true))
.andExpect(jsonPath("$.notifyOnMention").value(true));
}
@Test
@WithMockUser(username = "testuser", authorities = {"WRITE_ALL"})
void updatePreferences_returns200_whenUserHasWriteAll() throws Exception {
AppUser user = AppUser.builder().id(USER_ID).username("testuser")
.notifyOnReply(false).notifyOnMention(false).build();
when(userService.findByUsername("testuser")).thenReturn(user);
AppUser updated = AppUser.builder().id(USER_ID).username("testuser")
.notifyOnReply(true).notifyOnMention(false).build();
when(notificationService.updatePreferences(USER_ID, true, false)).thenReturn(updated);
mockMvc.perform(put("/api/users/me/notification-preferences")
.contentType(MediaType.APPLICATION_JSON)
.content("{\"notifyOnReply\":true,\"notifyOnMention\":false}"))
.andExpect(status().isOk())
.andExpect(jsonPath("$.notifyOnReply").value(true));
}
// ─── GET /api/notifications/unread-count ─────────────────────────────────
@Test
void countUnread_returns401_whenUnauthenticated() throws Exception {
mockMvc.perform(get("/api/notifications/unread-count"))
.andExpect(status().isUnauthorized());
}
@Test
@WithMockUser(username = "testuser", authorities = {"READ_ALL"})
void countUnread_returns200WithCount_whenAuthenticated() throws Exception {
AppUser user = AppUser.builder().id(USER_ID).username("testuser").build();
when(userService.findByUsername("testuser")).thenReturn(user);
when(notificationService.countUnread(USER_ID)).thenReturn(3L);
mockMvc.perform(get("/api/notifications/unread-count"))
.andExpect(status().isOk())
.andExpect(jsonPath("$.count").value(3));
}
// ─── PATCH /api/notifications/{id}/read — additional cases ───────────────
// ─── GET /api/notifications/stream ───────────────────────────────────────
@Test
void stream_returns401_whenUnauthenticated() throws Exception {
mockMvc.perform(get("/api/notifications/stream")
.accept(TEXT_EVENT_STREAM_VALUE))
.andExpect(status().isUnauthorized());
}
@Test
@WithMockUser(username = "testuser", authorities = {"READ_ALL"})
void stream_returns200_whenAuthenticated() throws Exception {
AppUser user = AppUser.builder().id(USER_ID).username("testuser").build();
when(userService.findByUsername("testuser")).thenReturn(user);
when(sseEmitterRegistry.register(USER_ID)).thenReturn(new org.springframework.web.servlet.mvc.method.annotation.SseEmitter());
mockMvc.perform(get("/api/notifications/stream")
.accept(TEXT_EVENT_STREAM_VALUE))
.andExpect(status().isOk());
}
// ─── PATCH /api/notifications/{id}/read — additional cases ───────────────
@Test
@WithMockUser(username = "testuser", authorities = {"READ_ALL"})
void markOneRead_returns404_whenNotificationDoesNotExist() throws Exception {
AppUser user = AppUser.builder().id(USER_ID).username("testuser").build();
UUID notifId = UUID.randomUUID();
when(userService.findByUsername("testuser")).thenReturn(user);
doThrow(DomainException.notFound(ErrorCode.NOTIFICATION_NOT_FOUND, "Notification not found: " + notifId))
.when(notificationService).markRead(notifId, USER_ID);
mockMvc.perform(patch("/api/notifications/" + notifId + "/read"))
.andExpect(status().isNotFound());
}
}

94
backend/src/test/java/org/raddatz/familienarchiv/controller/UserSearchControllerTest.java Normal file
View File

@@ -0,0 +1,94 @@
package org.raddatz.familienarchiv.controller;
marcel commented 2026-03-27 23:35:11 +01:00
Review
Copy Link

🔵 MINOR — search_returnsAtMostTenResults does not assert the count

The test name promises a cap of 10 results but the assertion only checks isOk(). If the LIMIT 10 in the repository query is accidentally removed, this test still passes.

Fix:

.andExpect(jsonPath("$.length()").value(lessThanOrEqualTo(10)));
🔵 **MINOR — `search_returnsAtMostTenResults` does not assert the count** The test name promises a cap of 10 results but the assertion only checks `isOk()`. If the `LIMIT 10` in the repository query is accidentally removed, this test still passes. **Fix:** ```java .andExpect(jsonPath("$.length()").value(lessThanOrEqualTo(10))); ```
import org.junit.jupiter.api.Test;
import org.raddatz.familienarchiv.config.SecurityConfig;
import org.raddatz.familienarchiv.model.AppUser;
import org.raddatz.familienarchiv.security.PermissionAspect;
import org.raddatz.familienarchiv.service.CustomUserDetailsService;
import org.raddatz.familienarchiv.service.UserSearchService;
import org.springframework.beans.factory.annotation.Autowired;
import org.springframework.boot.autoconfigure.aop.AopAutoConfiguration;
import org.springframework.boot.webmvc.test.autoconfigure.WebMvcTest;
import org.springframework.context.annotation.Import;
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.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;
import static org.springframework.test.web.servlet.result.MockMvcResultMatchers.jsonPath;
import static org.springframework.test.web.servlet.result.MockMvcResultMatchers.status;
@WebMvcTest(UserSearchController.class)
@Import({SecurityConfig.class, PermissionAspect.class, AopAutoConfiguration.class})
class UserSearchControllerTest {
@Autowired MockMvc mockMvc;
@MockitoBean UserSearchService userSearchService;
@MockitoBean CustomUserDetailsService customUserDetailsService;
@Test
void search_returns401_whenUnauthenticated() throws Exception {
mockMvc.perform(get("/api/users/search").param("q", "Hans"))
.andExpect(status().isUnauthorized());
}
@Test
@WithMockUser
void search_returns403_whenUserLacksPermission() throws Exception {
mockMvc.perform(get("/api/users/search").param("q", "Hans"))
.andExpect(status().isForbidden());
}
@Test
@WithMockUser(authorities = {"ANNOTATE_ALL"})
void search_returns200_whenUserHasAnnotateAll() throws Exception {
when(userSearchService.search("Hans")).thenReturn(List.of());
mockMvc.perform(get("/api/users/search").param("q", "Hans"))
.andExpect(status().isOk());
}
@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();
when(userSearchService.search("Hans")).thenReturn(List.of(user));
mockMvc.perform(get("/api/users/search").param("q", "Hans"))
.andExpect(status().isOk())
.andExpect(jsonPath("$[0].firstName").value("Hans"));
}
@Test
@WithMockUser(authorities = {"READ_ALL"})
void search_returnsEmptyList_whenQueryIsEmpty() throws Exception {
when(userSearchService.search("")).thenReturn(List.of());
mockMvc.perform(get("/api/users/search").param("q", ""))
.andExpect(status().isOk())
.andExpect(jsonPath("$").isEmpty());
}
@Test
@WithMockUser(authorities = {"READ_ALL"})
void search_returnsAtMostTenResults() throws Exception {
List<AppUser> 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(jsonPath("$.length()").value(lessThanOrEqualTo(10)));
}
}

77
backend/src/test/java/org/raddatz/familienarchiv/service/CommentServiceTest.java
View File

@@ -20,6 +20,9 @@ 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;
import static org.mockito.Mockito.when;
@@ -30,6 +33,8 @@ import static org.springframework.http.HttpStatus.NOT_FOUND;
class CommentServiceTest {
@Mock CommentRepository commentRepository;
@Mock UserService userService;
@Mock NotificationService notificationService;
@InjectMocks CommentService commentService;
// ─── postComment ──────────────────────────────────────────────────────────
@@ -43,7 +48,7 @@ class CommentServiceTest {
.id(UUID.randomUUID()).documentId(docId).authorName("Hans Müller").content("Test").build();
when(commentRepository.save(any())).thenReturn(saved);
DocumentComment result = commentService.postComment(docId, null, "Test", author);
DocumentComment result = commentService.postComment(docId, null, "Test", List.of(), author);
assertThat(result.getAuthorName()).isEqualTo("Hans Müller");
}
@@ -56,11 +61,28 @@ class CommentServiceTest {
.id(UUID.randomUUID()).documentId(docId).authorName("hans42").content("Test").build();
when(commentRepository.save(any())).thenReturn(saved);
DocumentComment result = commentService.postComment(docId, null, "Test", author);
DocumentComment result = commentService.postComment(docId, null, "Test", List.of(), author);
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
@@ -70,7 +92,7 @@ class CommentServiceTest {
AppUser author = AppUser.builder().id(UUID.randomUUID()).username("anna").build();
when(commentRepository.findById(commentId)).thenReturn(Optional.empty());
assertThatThrownBy(() -> commentService.replyToComment(docId, commentId, "Reply", author))
assertThatThrownBy(() -> commentService.replyToComment(docId, commentId, "Reply", List.of(), author))
.isInstanceOf(DomainException.class)
.satisfies(e -> assertThat(((DomainException) e).getStatus()).isEqualTo(NOT_FOUND));
@@ -91,11 +113,12 @@ 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);
DocumentComment result = commentService.replyToComment(docId, replyId, "Reply2", author);
DocumentComment result = commentService.replyToComment(docId, replyId, "Reply2", List.of(), author);
assertThat(result.getParentId()).isEqualTo(rootId);
}
@@ -110,15 +133,59 @@ 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);
DocumentComment result = commentService.replyToComment(docId, rootId, "Reply", author);
DocumentComment result = commentService.replyToComment(docId, rootId, "Reply", List.of(), author);
assertThat(result.getParentId()).isEqualTo(rootId);
}
@Test
void replyToComment_triggersNotifyReply_afterSave() {
UUID docId = UUID.randomUUID();
UUID rootId = UUID.randomUUID();
AppUser author = AppUser.builder().id(UUID.randomUUID()).username("anna").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("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), 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 ──────────────────────────────────────────────────────────
@Test

231
backend/src/test/java/org/raddatz/familienarchiv/service/NotificationServiceTest.java Normal file
View File

@@ -0,0 +1,231 @@
package org.raddatz.familienarchiv.service;
marcel commented 2026-03-27 23:34:57 +01:00
Review
Copy Link

⚠️ MAJOR — Several service method paths are untested

Missing test cases:

  • markRead_throwsNotFound_whenNotificationDoesNotExist — the NOTIFICATION_NOT_FOUND code path is real and user-facing (stale ID from frontend). No test covers it.
  • markAllRead_delegatesToRepository — markAllRead() is tested end-to-end via the controller but never in isolation at the service layer.
  • countUnread_delegatesToRepository — same gap.

These are not exotic edge cases — they are production paths that the frontend already exercises.

⚠️ **MAJOR — Several service method paths are untested** Missing test cases: - `markRead_throwsNotFound_whenNotificationDoesNotExist` — the `NOTIFICATION_NOT_FOUND` code path is real and user-facing (stale ID from frontend). No test covers it. - `markAllRead_delegatesToRepository` — `markAllRead()` is tested end-to-end via the controller but never in isolation at the service layer. - `countUnread_delegatesToRepository` — same gap. These are not exotic edge cases — they are production paths that the frontend already exercises.
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.Mock;
import org.mockito.junit.jupiter.MockitoExtension;
import org.raddatz.familienarchiv.dto.NotificationDTO;
import org.raddatz.familienarchiv.exception.DomainException;
import org.raddatz.familienarchiv.model.*;
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;
import static org.assertj.core.api.Assertions.assertThatThrownBy;
import static org.mockito.ArgumentMatchers.any;
import static org.mockito.ArgumentMatchers.eq;
import static org.mockito.Mockito.*;
@ExtendWith(MockitoExtension.class)
class NotificationServiceTest {
@Mock NotificationRepository notificationRepository;
@Mock UserService userService;
@Mock JavaMailSender mailSender;
@Mock SseEmitterRegistry sseEmitterRegistry;
NotificationService notificationService;
private AppUser userA;
private AppUser userB;
private AppUser userC;
@BeforeEach
void setUp() {
notificationService = new NotificationService(notificationRepository, userService, Optional.of(mailSender), sseEmitterRegistry);
userA = AppUser.builder().id(UUID.randomUUID()).username("userA")
.firstName("Anna").lastName("Smith").email("a@test.com")
.notifyOnReply(false).notifyOnMention(false).build();
userB = AppUser.builder().id(UUID.randomUUID()).username("userB")
.firstName("Bob").lastName("Jones").email("b@test.com")
.notifyOnReply(false).notifyOnMention(false).build();
userC = AppUser.builder().id(UUID.randomUUID()).username("userC")
.firstName("Clara").lastName("Doe").email("c@test.com")
.notifyOnReply(false).notifyOnMention(false).build();
}
// ─── notifyReply ──────────────────────────────────────────────────────────
@Test
void notifyReply_createsNotificationForThreadParticipants() {
DocumentComment reply = commentWithAuthor(UUID.randomUUID(), null, userC.getId(), "Clara Doe");
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, Set.of(userA.getId(), userB.getId()));
ArgumentCaptor<Notification> captor = ArgumentCaptor.forClass(Notification.class);
verify(notificationRepository, times(2)).save(captor.capture());
List<Notification> saved = captor.getAllValues();
assertThat(saved).extracting(n -> n.getRecipient().getId())
.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_doesNothing_whenParticipantSetIsEmpty() {
DocumentComment reply = commentWithAuthor(UUID.randomUUID(), null, userA.getId(), "Anna Smith");
notificationService.notifyReply(reply, Set.of());
verify(notificationRepository, never()).save(any());
}
@Test
void notifyReply_sendsEmailOnlyToUsersWithReplyNotificationsEnabled() {
userA.setNotifyOnReply(true);
userB.setNotifyOnReply(false);
DocumentComment reply = commentWithAuthor(UUID.randomUUID(), null, userC.getId(), "Clara Doe");
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, Set.of(userA.getId(), userB.getId()));
verify(mailSender, times(1)).send(any(SimpleMailMessage.class));
}
// ─── notifyMentions ───────────────────────────────────────────────────────
@Test
void notifyMentions_createsNotificationPerMentionedUser() {
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);
ArgumentCaptor<Notification> captor = ArgumentCaptor.forClass(Notification.class);
verify(notificationRepository, times(2)).save(captor.capture());
List<Notification> saved = captor.getAllValues();
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
void notifyMentions_sendsEmailOnlyToUsersWithMentionNotificationsEnabled() {
userA.setNotifyOnMention(true);
userB.setNotifyOnMention(false);
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);
verify(mailSender, times(1)).send(any(SimpleMailMessage.class));
}
// ─── SSE push ─────────────────────────────────────────────────────────────
@Test
void notifyReply_pushesEventToRegistry_forEachRecipient() {
DocumentComment reply = commentWithAuthor(UUID.randomUUID(), null, userC.getId(), "Clara Doe");
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, Set.of(userA.getId(), userB.getId()));
verify(sseEmitterRegistry).send(eq(userA.getId()), any(NotificationDTO.class));
verify(sseEmitterRegistry).send(eq(userB.getId()), any(NotificationDTO.class));
}
@Test
void notifyMentions_pushesEventToRegistry_forEachMentionedUser() {
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);
verify(sseEmitterRegistry).send(eq(userA.getId()), any(NotificationDTO.class));
verify(sseEmitterRegistry).send(eq(userB.getId()), any(NotificationDTO.class));
}
// ─── 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()
.id(UUID.randomUUID())
.recipient(userA)
.type(NotificationType.REPLY)
.read(false)
.build();
when(notificationRepository.findById(notification.getId())).thenReturn(Optional.of(notification));
assertThatThrownBy(() -> notificationService.markRead(notification.getId(), userB.getId()))
.isInstanceOf(DomainException.class)
.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, String authorName) {
return DocumentComment.builder()
.id(id)
.documentId(UUID.randomUUID())
.parentId(parentId)
.authorId(authorId)
.authorName(authorName)
.content("content")
.build();
}
}

37
backend/src/test/java/org/raddatz/familienarchiv/service/SseEmitterRegistryTest.java Normal file
View File

@@ -0,0 +1,37 @@
package org.raddatz.familienarchiv.service;
import org.junit.jupiter.api.Test;
import org.springframework.web.servlet.mvc.method.annotation.SseEmitter;
import java.util.UUID;
import static org.assertj.core.api.Assertions.assertThat;
import static org.assertj.core.api.Assertions.assertThatCode;
class SseEmitterRegistryTest {
private final SseEmitterRegistry registry = new SseEmitterRegistry();
@Test
void register_returnsEmitter() {
SseEmitter emitter = registry.register(UUID.randomUUID());
assertThat(emitter).isNotNull();
}
@Test
void send_doesNothing_whenNoEmitterRegistered() {
assertThatCode(() -> registry.send(UUID.randomUUID(), "data"))
.doesNotThrowAnyException();
}
@Test
void register_replacesExistingEmitter_forSameUser() {
UUID userId = UUID.randomUUID();
SseEmitter first = registry.register(userId);
SseEmitter second = registry.register(userId);
assertThat(first).isNotSameAs(second);
}
}

75
backend/src/test/java/org/raddatz/familienarchiv/service/UserServiceTest.java
View File

@@ -5,17 +5,20 @@ import org.junit.jupiter.api.extension.ExtendWith;
import org.mockito.InjectMocks;
import org.mockito.Mock;
import org.mockito.junit.jupiter.MockitoExtension;
import org.raddatz.familienarchiv.dto.AdminUpdateUserRequest;
import org.raddatz.familienarchiv.dto.ChangePasswordDTO;
import org.raddatz.familienarchiv.dto.CreateUserRequest;
import org.raddatz.familienarchiv.dto.UpdateProfileDTO;
import org.raddatz.familienarchiv.exception.DomainException;
import org.raddatz.familienarchiv.model.AppUser;
import org.raddatz.familienarchiv.model.UserGroup;
import org.raddatz.familienarchiv.repository.AppUserRepository;
import org.raddatz.familienarchiv.repository.UserGroupRepository;
import org.springframework.security.crypto.password.PasswordEncoder;
import java.util.List;
import java.util.Optional;
import java.util.Set;
import java.util.UUID;
import static org.assertj.core.api.Assertions.assertThat;
@@ -216,6 +219,78 @@ class UserServiceTest {
verify(userRepository).save(argThat(u -> "newHash".equals(u.getPassword())));
}
// ─── adminUpdateUser ──────────────────────────────────────────────────────
@Test
void adminUpdateUser_updatesNameFields() {
UUID id = UUID.randomUUID();
AppUser user = AppUser.builder().id(id).username("admin").build();
when(userRepository.findById(id)).thenReturn(Optional.of(user));
when(userRepository.save(any())).thenAnswer(inv -> inv.getArgument(0));
AdminUpdateUserRequest dto = new AdminUpdateUserRequest();
dto.setFirstName("Ada"); dto.setLastName("Lovelace");
AppUser result = userService.adminUpdateUser(id, dto);
assertThat(result.getFirstName()).isEqualTo("Ada");
assertThat(result.getLastName()).isEqualTo("Lovelace");
}
@Test
void adminUpdateUser_preservesGroups_whenGroupIdsIsNull() {
UUID id = UUID.randomUUID();
UserGroup adminGroup = UserGroup.builder().id(UUID.randomUUID()).name("Administrators").build();
AppUser user = AppUser.builder().id(id).username("admin").groups(Set.of(adminGroup)).build();
when(userRepository.findById(id)).thenReturn(Optional.of(user));
when(userRepository.save(any())).thenAnswer(inv -> inv.getArgument(0));
AdminUpdateUserRequest dto = new AdminUpdateUserRequest();
dto.setFirstName("Ada"); // groupIds left null → don't change groups
AppUser result = userService.adminUpdateUser(id, dto);
assertThat(result.getGroups()).containsExactly(adminGroup);
}
@Test
void adminUpdateUser_updatesGroups_whenGroupIdsProvided() {
UUID id = UUID.randomUUID();
UserGroup oldGroup = UserGroup.builder().id(UUID.randomUUID()).name("Viewers").build();
UserGroup newGroup = UserGroup.builder().id(UUID.randomUUID()).name("Editors").build();
AppUser user = AppUser.builder().id(id).username("max").groups(Set.of(oldGroup)).build();
when(userRepository.findById(id)).thenReturn(Optional.of(user));
when(groupRepository.findAllById(List.of(newGroup.getId()))).thenReturn(List.of(newGroup));
when(userRepository.save(any())).thenAnswer(inv -> inv.getArgument(0));
AdminUpdateUserRequest dto = new AdminUpdateUserRequest();
dto.setGroupIds(List.of(newGroup.getId()));
AppUser result = userService.adminUpdateUser(id, dto);
assertThat(result.getGroups()).containsExactly(newGroup);
}
@Test
void adminUpdateUser_clearsGroups_whenGroupIdsIsEmptyList() {
// Sending groupIds:[] is the explicit "remove from all groups" signal.
// The frontend must NEVER send [] accidentally — it must always include
// the currently-selected group checkboxes.
UUID id = UUID.randomUUID();
UserGroup adminGroup = UserGroup.builder().id(UUID.randomUUID()).name("Administrators").build();
AppUser user = AppUser.builder().id(id).username("admin").groups(Set.of(adminGroup)).build();
when(userRepository.findById(id)).thenReturn(Optional.of(user));
when(groupRepository.findAllById(List.of())).thenReturn(List.of());
when(userRepository.save(any())).thenAnswer(inv -> inv.getArgument(0));
AdminUpdateUserRequest dto = new AdminUpdateUserRequest();
dto.setGroupIds(List.of()); // empty list → intentional "remove all groups"
AppUser result = userService.adminUpdateUser(id, dto);
assertThat(result.getGroups()).isEmpty();
}
// ─── getGroupById ─────────────────────────────────────────────────────────
@Test

15
frontend/messages/de.json
View File

@@ -294,5 +294,18 @@
"enrich_done_body": "Alle Dokumente wurden bearbeitet.",
"enrich_back_to_list": "Zurück zur Liste",
"comment_empty_hint": "Noch keine Kommentare – starte die Diskussion!",
"comment_start_discussion": "Diskussion starten →"
"comment_start_discussion": "Diskussion starten →",
"notification_bell_label": "Benachrichtigungen",
"notification_bell_unread_label": "{count} ungelesene Benachrichtigungen",
"notification_mark_all_read": "Alle gelesen",
"notification_empty": "Keine neuen Benachrichtigungen",
"notification_type_reply": "{actor} hat auf deinen Kommentar geantwortet",
"notification_type_mention": "{actor} hat dich in einem Kommentar erwähnt",
"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_prefs_no_email": "Bitte trage zuerst eine E-Mail-Adresse ein, um Benachrichtigungen zu erhalten.",
"notification_unread": "ungelesen",
"mention_btn_label": "Person erwähnen",
"mention_popup_empty": "Keine Nutzer gefunden"
}

15
frontend/messages/en.json
View File

@@ -294,5 +294,18 @@
"enrich_done_body": "All documents have been processed.",
"enrich_back_to_list": "Back to list",
"comment_empty_hint": "No comments yet – start the discussion!",
"comment_start_discussion": "Start discussion →"
"comment_start_discussion": "Start discussion →",
"notification_bell_label": "Notifications",
"notification_bell_unread_label": "{count} unread notifications",
"notification_mark_all_read": "Mark all read",
"notification_empty": "No new notifications",
"notification_type_reply": "{actor} replied to your comment",
"notification_type_mention": "{actor} mentioned you in a comment",
"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_prefs_no_email": "Please add an email address above to receive notifications.",
"notification_unread": "unread",
"mention_btn_label": "Mention person",
"mention_popup_empty": "No users found"
}

15
frontend/messages/es.json
View File

@@ -294,5 +294,18 @@
"enrich_done_body": "Todos los documentos han sido procesados.",
"enrich_back_to_list": "Volver a la lista",
"comment_empty_hint": "Aún no hay comentarios – ¡inicia la discusión!",
"comment_start_discussion": "Iniciar discusión →"
"comment_start_discussion": "Iniciar discusión →",
"notification_bell_label": "Notificaciones",
"notification_bell_unread_label": "{count} notificaciones sin leer",
"notification_mark_all_read": "Marcar todo como leído",
"notification_empty": "No hay notificaciones nuevas",
"notification_type_reply": "{actor} respondió a tu comentario",
"notification_type_mention": "{actor} te mencionó en un comentario",
"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_prefs_no_email": "Por favor, añade una dirección de correo electrónico para recibir notificaciones.",
"notification_unread": "no leído",
"mention_btn_label": "Mencionar persona",
"mention_popup_empty": "No se encontraron usuarios"
}

3
frontend/src/lib/components/AnnotationSidePanel.svelte
View File

@@ -9,6 +9,7 @@ type Props = {
canComment: boolean;
currentUserId: string | null;
canAdmin: boolean;
targetCommentId?: string | null;
onClose: () => void;
};
@@ -19,6 +20,7 @@ let {
canComment,
currentUserId,
canAdmin,
targetCommentId = null,
onClose
}: Props = $props();
@@ -57,6 +59,7 @@ const visible = $derived(activeAnnotationId !== null);
canComment={canComment}
currentUserId={currentUserId}
canAdmin={canAdmin}
targetCommentId={targetCommentId}
loadOnMount={true}
/>
{/key}

76
frontend/src/lib/components/AnnotationSidePanel.svelte.spec.ts Normal file
View File

@@ -0,0 +1,76 @@
import { describe, it, expect, vi, afterEach } from 'vitest';
import { cleanup, render } from 'vitest-browser-svelte';
import { page } from 'vitest/browser';
import AnnotationSidePanel from './AnnotationSidePanel.svelte';
afterEach(() => {
cleanup();
vi.restoreAllMocks();
});
vi.stubGlobal(
'fetch',
vi.fn().mockResolvedValue({
ok: true,
json: async () => []
})
);
const baseProps = {
documentId: 'doc-1',
activeAnnotationPage: 1,
canComment: true,
currentUserId: 'user-1',
canAdmin: false,
onClose: vi.fn()
};
describe('AnnotationSidePanel – visibility', () => {
it('is hidden (translated off-screen) when activeAnnotationId is null', async () => {
render(AnnotationSidePanel, { ...baseProps, activeAnnotationId: null });
const panel = document.querySelector('[data-testid="annotation-side-panel"]');
expect(panel?.classList.contains('translate-x-full')).toBe(true);
expect(panel?.classList.contains('translate-x-0')).toBe(false);
});
it('is visible when activeAnnotationId is set', async () => {
render(AnnotationSidePanel, { ...baseProps, activeAnnotationId: 'ann-1' });
const panel = document.querySelector('[data-testid="annotation-side-panel"]');
expect(panel?.classList.contains('translate-x-0')).toBe(true);
expect(panel?.classList.contains('translate-x-full')).toBe(false);
});
});
describe('AnnotationSidePanel – close button', () => {
it('calls onClose when the close button is clicked', async () => {
const onClose = vi.fn();
render(AnnotationSidePanel, { ...baseProps, activeAnnotationId: 'ann-1', onClose });
await page.getByRole('button', { name: /schließen/i }).click();
expect(onClose).toHaveBeenCalledOnce();
});
});
describe('AnnotationSidePanel – targetCommentId forwarding', () => {
it('renders CommentThread when annotation is active', async () => {
render(AnnotationSidePanel, {
...baseProps,
activeAnnotationId: 'ann-1',
targetCommentId: 'comment-42'
});
// CommentThread renders inside the panel when activeAnnotationId is set
const panel = document.querySelector('[data-testid="annotation-side-panel"]');
expect(panel).not.toBeNull();
expect(panel?.classList.contains('translate-x-0')).toBe(true);
});
it('does not render CommentThread when annotation is null', async () => {
render(AnnotationSidePanel, {
...baseProps,
activeAnnotationId: null,
targetCommentId: 'comment-42'
});
// Panel is hidden and no fetch should have been triggered for comments
const panel = document.querySelector('[data-testid="annotation-side-panel"]');
expect(panel?.classList.contains('translate-x-full')).toBe(true);
});
});

94
frontend/src/lib/components/CommentThread.svelte
View File

@@ -1,7 +1,10 @@
<script lang="ts">
marcel commented 2026-03-27 23:35:11 +01:00
Review
Copy Link

🔵 MINOR — setTimeout(100) magic delay for deep-link scroll is flaky

setTimeout(() => {
  const el = document.querySelector(`[data-comment-id="${targetCommentId}"]`);
  el?.scrollIntoView({ behavior: 'smooth', block: 'center' });
}, 100);

On slow devices or slow network responses, 100ms may not be enough for the panel to render and the data-comment-id element to be in the DOM. The scrollIntoView call then silently no-ops — the deep link appears broken with no error.

Fix: Use await tick() (Svelte's DOM-flush promise) followed by a requestAnimationFrame, or a MutationObserver that waits for the element to appear rather than assuming a fixed delay.

🔵 **MINOR — `setTimeout(100)` magic delay for deep-link scroll is flaky** ```typescript setTimeout(() => { const el = document.querySelector(`[data-comment-id="${targetCommentId}"]`); el?.scrollIntoView({ behavior: 'smooth', block: 'center' }); }, 100); ``` On slow devices or slow network responses, 100ms may not be enough for the panel to render and the `data-comment-id` element to be in the DOM. The `scrollIntoView` call then silently no-ops — the deep link appears broken with no error. **Fix:** Use `await tick()` (Svelte's DOM-flush promise) followed by a `requestAnimationFrame`, or a `MutationObserver` that waits for the element to appear rather than assuming a fixed delay.
import { onMount, untrack } from 'svelte';
import { onMount, tick, untrack } from 'svelte';
import { m } from '$lib/paraglide/messages.js';
import type { Comment, CommentReply } from '$lib/types';
import MentionEditor from '$lib/components/MentionEditor.svelte';
import { renderBody, extractContent } from '$lib/utils/mention';
import type { MentionDTO } from '$lib/types';
type Props = {
documentId: string;
@@ -11,6 +14,7 @@ type Props = {
canComment: boolean;
currentUserId: string | null;
canAdmin: boolean;
targetCommentId?: string | null;
onCountChange?: (count: number) => void;
};
@@ -22,16 +26,21 @@ let {
canComment,
currentUserId,
canAdmin,
targetCommentId = null,
onCountChange
}: Props = $props();
let comments: Comment[] = $state(untrack(() => [...initialComments]));
let highlightedCommentId: string | null = $state(untrack(() => targetCommentId ?? null));
let newText: string = $state('');
let replyingTo: string | null = $state(null);
let replyText: string = $state('');
let editingId: string | null = $state(null);
let editText: string = $state('');
let posting: boolean = $state(false);
let newMentionCandidates: MentionDTO[] = $state([]);
let replyMentionCandidates: MentionDTO[] = $state([]);
let editMentionCandidates: MentionDTO[] = $state([]);
const commentsBase = $derived(
annotationId
@@ -76,13 +85,15 @@ async function postComment() {
if (!text || posting) return;
posting = true;
try {
const { content, mentionedUserIds } = extractContent(text, newMentionCandidates);
const res = await fetch(commentsBase, {
method: 'POST',
headers: { 'Content-Type': 'application/json' },
body: JSON.stringify({ content: text })
body: JSON.stringify({ content, mentionedUserIds })
});
if (res.ok) {
newText = '';
newMentionCandidates = [];
await reload();
}
} finally {
@@ -95,13 +106,15 @@ async function postReply(threadId: string) {
if (!text || posting) return;
posting = true;
try {
const { content, mentionedUserIds } = extractContent(text, replyMentionCandidates);
const res = await fetch(`${commentsBase}/${threadId}/replies`, {
method: 'POST',
headers: { 'Content-Type': 'application/json' },
body: JSON.stringify({ content: text })
body: JSON.stringify({ content, mentionedUserIds })
});
if (res.ok) {
replyText = '';
replyMentionCandidates = [];
replyingTo = null;
await reload();
}
@@ -115,13 +128,15 @@ async function saveEdit(commentId: string) {
if (!text || posting) return;
posting = true;
try {
const { content, mentionedUserIds } = extractContent(text, editMentionCandidates);
const res = await fetch(`/api/documents/${documentId}/comments/${commentId}`, {
method: 'PATCH',
headers: { 'Content-Type': 'application/json' },
body: JSON.stringify({ content: text })
body: JSON.stringify({ content, mentionedUserIds })
});
if (res.ok) {
editingId = null;
editMentionCandidates = [];
await reload();
}
} finally {
@@ -147,6 +162,7 @@ async function deleteComment(commentId: string) {
function startEdit(comment: Comment | CommentReply) {
editingId = comment.id;
editText = comment.content;
editMentionCandidates = [];
}
function cancelEdit() {
@@ -164,13 +180,32 @@ function cancelReply() {
replyText = '';
}
onMount(() => {
onMount(async () => {
if (loadOnMount) {
reload();
} else {
const total = initialComments.reduce((s, c) => s + 1 + c.replies.length, 0);
onCountChange?.(total);
}
if (targetCommentId) {
await tick();
requestAnimationFrame(() => {
const el = document.querySelector(`[data-comment-id="${targetCommentId}"]`);
el?.scrollIntoView({ behavior: 'smooth', block: 'center' });
});
// Remove highlight on first user interaction
const clearHighlight = () => {
highlightedCommentId = null;
document.removeEventListener('click', clearHighlight, true);
document.removeEventListener('keydown', clearHighlight, true);
document.removeEventListener('scroll', clearHighlight, true);
};
document.addEventListener('click', clearHighlight, true);
document.addEventListener('keydown', clearHighlight, true);
document.addEventListener('scroll', clearHighlight, true);
}
});
</script>
@@ -181,11 +216,13 @@ onMount(() => {
{#snippet commentEntry(comment: Comment | CommentReply, threadId: string, showReplyButton: boolean)}
{#if editingId === comment.id}
<div class="flex flex-col gap-2">
<textarea
class="w-full resize-none rounded border border-line px-3 py-2 font-serif text-sm text-ink focus:ring-1 focus:ring-accent focus:outline-none"
rows={3}
<MentionEditor
bind:value={editText}
></textarea>
bind:mentionCandidates={editMentionCandidates}
rows={3}
disabled={posting}
onsubmit={() => saveEdit(comment.id)}
/>
<div class="flex items-center gap-3">
<button
class="rounded bg-primary px-3 py-1.5 font-sans text-xs font-medium text-primary-fg hover:bg-primary/80 disabled:opacity-40"
@@ -215,7 +252,10 @@ onMount(() => {
</span>
{/if}
</div>
<p class="mt-1 font-serif text-sm leading-relaxed text-ink-2">{comment.content}</p>
<p class="mt-1 font-serif text-sm leading-relaxed text-ink-2">
<!-- eslint-disable-next-line svelte/no-at-html-tags -- renderBody escapes all HTML before injecting mention links -->
{@html renderBody(comment.content, comment.mentionDTOs ?? [])}
</p>
</div>
{#if canModify(comment)}
<div class="flex shrink-0 items-center gap-2">
@@ -269,13 +309,23 @@ onMount(() => {
{#each comments as thread, ti (thread.id)}
<div class={ti > 0 ? 'border-t border-line pt-4' : ''}>
<!-- Root comment -->
<div>
<div
data-comment-id={thread.id}
class={highlightedCommentId === thread.id
? 'rounded ring-2 ring-accent ring-offset-1 transition-shadow'
: ''}
>
{@render commentEntry(thread, thread.id, thread.replies.length === 0)}
</div>
<!-- Replies -->
{#each thread.replies as reply, ri (reply.id)}
<div class="mt-3 ml-6 border-l-2 border-line pl-4">
<div
data-comment-id={reply.id}
class="mt-3 ml-6 border-l-2 border-line pl-4 {highlightedCommentId === reply.id
? 'rounded ring-2 ring-accent ring-offset-1 transition-shadow'
: ''}"
>
{@render commentEntry(reply, thread.id, ri === thread.replies.length - 1)}
</div>
{/each}
@@ -283,12 +333,14 @@ onMount(() => {
<!-- Reply compose box -->
{#if replyingTo === thread.id}
<div class="mt-3 ml-6 flex flex-col gap-2">
<textarea
class="w-full resize-none rounded border border-line px-3 py-2 font-serif text-sm text-ink focus:ring-1 focus:ring-accent focus:outline-none"
<MentionEditor
bind:value={replyText}
bind:mentionCandidates={replyMentionCandidates}
rows={3}
placeholder={m.comment_placeholder()}
bind:value={replyText}
></textarea>
disabled={posting}
onsubmit={() => postReply(thread.id)}
/>
<div class="flex items-center gap-3">
<button
class="rounded bg-primary px-3 py-1.5 font-sans text-xs font-medium text-primary-fg hover:bg-primary/80 disabled:opacity-40"
@@ -313,12 +365,14 @@ onMount(() => {
{#if canComment}
<div class={comments.length > 0 ? 'border-t border-line pt-4' : ''}>
<div class="flex flex-col gap-2">
<textarea
class="w-full resize-none rounded border border-line px-3 py-2 font-serif text-sm text-ink focus:ring-1 focus:ring-accent focus:outline-none"
<MentionEditor
bind:value={newText}
bind:mentionCandidates={newMentionCandidates}
rows={3}
placeholder={m.comment_placeholder()}
bind:value={newText}
></textarea>
disabled={posting}
onsubmit={postComment}
/>
<div>
<button
class="rounded bg-primary px-3 py-1.5 font-sans text-xs font-medium text-primary-fg hover:bg-primary/80 disabled:opacity-40"

5
frontend/src/lib/components/DocumentBottomPanel.svelte
View File

@@ -28,6 +28,7 @@ type Props = {
open: boolean;
height: number;
activeTab: DocumentPanelTab;
targetCommentId?: string | null;
};
let {
@@ -38,7 +39,8 @@ let {
canAdmin,
open = $bindable(),
height = $bindable(),
activeTab = $bindable()
activeTab = $bindable(),
targetCommentId = null
}: Props = $props();
const MIN_HEIGHT = 52; // drag handle (8px) + tab bar (~44px)
@@ -180,6 +182,7 @@ function handleCountChange(count: number) {
canComment={canComment}
currentUserId={currentUserId}
canAdmin={canAdmin}
targetCommentId={targetCommentId}
onCountChange={handleCountChange}
/>
{:else if activeTab === 'history'}

237
frontend/src/lib/components/MentionEditor.svelte Normal file
View File

@@ -0,0 +1,237 @@
<script lang="ts">
marcel commented 2026-03-27 23:35:09 +01:00
Review
Copy Link

🔵 MINOR — debounceTimer not cleared on component destroy

The debounceTimer is cleared inside closePopup(), but there is no onDestroy hook. If the component unmounts while a debounced search is pending (user navigates away mid-typing), the timeout fires on an unmounted component and causes a stale state update.

Fix:

import { onDestroy } from 'svelte';
onDestroy(() => clearTimeout(debounceTimer));
🔵 **MINOR — `debounceTimer` not cleared on component destroy** The `debounceTimer` is cleared inside `closePopup()`, but there is no `onDestroy` hook. If the component unmounts while a debounced search is pending (user navigates away mid-typing), the timeout fires on an unmounted component and causes a stale state update. **Fix:** ```typescript import { onDestroy } from 'svelte'; onDestroy(() => clearTimeout(debounceTimer)); ```
import { onDestroy, tick } from 'svelte';
import { detectMention } from '$lib/utils/mention';
import type { MentionDTO } from '$lib/types';
import { m } from '$lib/paraglide/messages.js';
type Props = {
value: string;
mentionCandidates: MentionDTO[];
placeholder?: string;
rows?: number;
disabled?: boolean;
onsubmit?: () => void;
};
let {
value = $bindable(''),
mentionCandidates = $bindable([]),
placeholder = '',
rows = 3,
disabled = false,
onsubmit
}: Props = $props();
let query: string | null = $state(null);
let results: MentionDTO[] = $state([]);
let highlightedIndex = $state(0);
let mentionStart = $state(0);
let textarea: HTMLTextAreaElement | null = null;
let debounceTimer: ReturnType<typeof setTimeout> | undefined;
function attachTextarea(node: HTMLTextAreaElement) {
textarea = node;
return () => {
textarea = null;
};
}
function handleInput() {
if (!textarea) return;
const cursorPos = textarea.selectionStart;
const detected = detectMention(value, cursorPos);
if (detected === null) {
closePopup();
return;
}
// Calculate where the @ starts
const before = value.slice(0, cursorPos);
const atIndex = before.lastIndexOf('@');
mentionStart = atIndex;
if (query !== detected) {
query = detected;
highlightedIndex = 0;
scheduleSearch(detected);
}
}
function scheduleSearch(q: string) {
clearTimeout(debounceTimer);
if (!q) {
results = [];
return;
}
debounceTimer = setTimeout(async () => {
try {
const res = await fetch(`/api/users/search?q=${encodeURIComponent(q)}`);
if (res.ok) {
const data: MentionDTO[] = await res.json();
results = data.slice(0, 5);
} else {
results = [];
}
} catch {
results = [];
}
}, 200);
}
async function selectUser(user: MentionDTO) {
if (!textarea) return;
const displayName = `${user.firstName} ${user.lastName}`;
// Replace @partialQuery with @FirstName LastName (plus trailing space)
const replacement = `@${displayName} `;
const cursorPos = textarea.selectionStart;
const before = value.slice(0, mentionStart);
const after = value.slice(cursorPos);
value = before + replacement + after;
// Deduplicate and add to candidates
if (!mentionCandidates.some((c) => c.id === user.id)) {
mentionCandidates = [...mentionCandidates, user];
}
closePopup();
// Reposition cursor after the inserted mention
await tick();
if (!textarea) return;
const pos = mentionStart + replacement.length;
textarea.selectionStart = pos;
textarea.selectionEnd = pos;
textarea.focus();
}
function closePopup() {
query = null;
results = [];
highlightedIndex = 0;
clearTimeout(debounceTimer);
}
function handleKeydown(e: KeyboardEvent) {
if (e.ctrlKey && e.key === 'Enter') {
e.preventDefault();
onsubmit?.();
return;
}
if (query === null) return;
if (e.key === 'Escape') {
e.preventDefault();
closePopup();
return;
}
if (e.key === 'ArrowDown') {
e.preventDefault();
if (results.length > 0) {
highlightedIndex = (highlightedIndex + 1) % results.length;
}
return;
}
if (e.key === 'ArrowUp') {
e.preventDefault();
if (results.length > 0) {
highlightedIndex = (highlightedIndex - 1 + results.length) % results.length;
}
return;
}
if (e.key === 'Enter' && results.length > 0) {
e.preventDefault();
selectUser(results[highlightedIndex]);
return;
}
}
async function handleAtButtonClick() {
if (!textarea) return;
const pos = textarea.selectionStart;
const before = value.slice(0, pos);
const after = value.slice(pos);
// Ensure @ is preceded by whitespace or is at the start
const needsSpace = before.length > 0 && !/\s$/.test(before);
const insertion = needsSpace ? ' @' : '@';
value = before + insertion + after;
await tick();
if (!textarea) return;
const newPos = pos + insertion.length;
textarea.selectionStart = newPos;
textarea.selectionEnd = newPos;
textarea.focus();
// Trigger mention detection after inserting @
const detected = detectMention(value, newPos);
if (detected !== null) {
mentionStart = newPos - 1;
query = detected;
highlightedIndex = 0;
scheduleSearch(detected);
}
}
onDestroy(() => clearTimeout(debounceTimer));
const popupOpen = $derived(query !== null);
</script>
<div class="relative">
<textarea
{@attach attachTextarea}
class="w-full resize-none rounded border border-line px-3 py-2 font-serif text-sm text-ink focus:ring-1 focus:ring-accent focus:outline-none"
rows={rows}
placeholder={placeholder}
disabled={disabled}
bind:value={value}
oninput={handleInput}
onkeydown={handleKeydown}
></textarea>
{#if popupOpen}
<div
class="absolute z-20 mt-1 w-64 overflow-hidden rounded-sm border border-line bg-surface shadow-lg"
role="listbox"
aria-label={m.mention_btn_label()}
>
{#if results.length === 0}
<p class="px-3 py-2 font-sans text-sm text-ink-3">{m.mention_popup_empty()}</p>
{:else}
{#each results as user, i (user.id)}
<div
class="w-full px-3 py-2 text-left font-sans text-sm text-ink hover:bg-canvas {i === highlightedIndex ? 'bg-canvas' : ''}"
role="option"
aria-selected={i === highlightedIndex}
tabindex="-1"
onmousedown={(e) => {
// Use mousedown to fire before textarea blur
e.preventDefault();
selectUser(user);
}}
>
{user.firstName}
{user.lastName}
</div>
{/each}
{/if}
</div>
{/if}
<button
type="button"
aria-label={m.mention_btn_label()}
disabled={disabled}
class="mt-1 rounded border border-line px-2 py-0.5 font-sans text-xs font-medium text-ink-3 transition-colors hover:border-ink hover:text-ink disabled:opacity-40"
onclick={handleAtButtonClick}
>
@
</button>
</div>

322
frontend/src/lib/components/NotificationBell.svelte Normal file
View File

@@ -0,0 +1,322 @@
<script lang="ts">
marcel commented 2026-03-27 23:35:08 +01:00
Review
Copy Link

🔵 MINOR — relativeTime() returns hard-coded German strings

"gerade eben", "vor {n} Min.", "vor {n} Std.", "vor {n} Tag(en)" are hard-coded German regardless of the active Paraglide locale. The project supports de/en/es and notification messages use m.* keys — but timestamps fall back to German for all locales.

Fix: Use Intl.RelativeTimeFormat with the current locale, or add Paraglide keys (e.g. m.notification_time_just_now(), m.notification_time_minutes_ago({ n })).

🔵 **MINOR — `relativeTime()` returns hard-coded German strings** `"gerade eben"`, `"vor {n} Min."`, `"vor {n} Std."`, `"vor {n} Tag(en)"` are hard-coded German regardless of the active Paraglide locale. The project supports de/en/es and notification messages use `m.*` keys — but timestamps fall back to German for all locales. **Fix:** Use `Intl.RelativeTimeFormat` with the current locale, or add Paraglide keys (e.g. `m.notification_time_just_now()`, `m.notification_time_minutes_ago({ n })`).
import { onMount, onDestroy } from 'svelte';
marcel commented 2026-03-27 23:35:08 +01:00
Review
Copy Link

🔵 MINOR — aria-label="ungelesen" is hard-coded German

The unread indicator dot has aria-label="ungelesen". Screen readers in English or Spanish will read "ungelesen" — a German word. This is an a11y regression.

Fix: Use a Paraglide key: aria-label={m.notification_unread()}.

🔵 **MINOR — `aria-label="ungelesen"` is hard-coded German** The unread indicator dot has `aria-label="ungelesen"`. Screen readers in English or Spanish will read "ungelesen" — a German word. This is an a11y regression. **Fix:** Use a Paraglide key: `aria-label={m.notification_unread()}`.
import { goto } from '$app/navigation';
marcel commented 2026-03-27 23:35:09 +01:00
Review
Copy Link

🔵 MINOR — <div role="button"> does not handle the Space key

Each notification item is a <div role="button" tabindex="0"> with a manual onkeydown handler. The handler only handles Enter — but ARIA authoring practices specify that both Enter and Space activate buttons. Keyboard-only users pressing Space will have no response.

Fix: Replace <div role="button"> with a real <button> element. Native <button> handles both Enter and Space for free, is focusable by default, and requires no tabindex or manual keyboard wiring.

🔵 **MINOR — `<div role="button">` does not handle the Space key** Each notification item is a `<div role="button" tabindex="0">` with a manual `onkeydown` handler. The handler only handles `Enter` — but ARIA authoring practices specify that both `Enter` *and* `Space` activate buttons. Keyboard-only users pressing Space will have no response. **Fix:** Replace `<div role="button">` with a real `<button>` element. Native `<button>` handles both Enter and Space for free, is focusable by default, and requires no `tabindex` or manual keyboard wiring.
import { m } from '$lib/paraglide/messages.js';
type NotificationItem = {
id: string;
type: 'REPLY' | 'MENTION';
documentId: string;
referenceId: string;
annotationId: string | null;
read: boolean;
createdAt: string;
actorName: string;
};
let notifications: NotificationItem[] = $state([]);
let unreadCount: number = $state(0);
let open = $state(false);
// DOM refs managed via attachments
let bellButtonEl: HTMLButtonElement | null = null;
let firstFocusableEl: HTMLButtonElement | null = null;
let eventSource: EventSource | null = null;
async function fetchNotifications() {
try {
const res = await fetch('/api/notifications?size=10');
if (res.ok) {
const data = await res.json();
notifications = data.content ?? [];
}
} catch (e) {
console.error('Failed to fetch notifications', e);
}
}
async function fetchUnreadCount() {
try {
const res = await fetch('/api/notifications/unread-count');
if (res.ok) {
const data = await res.json();
unreadCount = data.count;
}
} catch (e) {
console.error('Failed to fetch unread count', e);
}
}
async function toggleDropdown() {
open = !open;
if (open) {
await fetchNotifications();
// defer focus until DOM updates
setTimeout(() => {
firstFocusableEl?.focus();
}, 0);
}
}
function closeDropdown() {
open = false;
bellButtonEl?.focus();
}
async function markRead(notification: NotificationItem) {
if (!notification.read) {
try {
await fetch(`/api/notifications/${notification.id}/read`, { method: 'PATCH' });
notification.read = true;
unreadCount = Math.max(0, unreadCount - 1);
} catch (e) {
console.error('Failed to mark notification as read', e);
}
}
const url = notification.annotationId
? `/documents/${notification.documentId}?commentId=${notification.referenceId}&annotationId=${notification.annotationId}`
: `/documents/${notification.documentId}?commentId=${notification.referenceId}`;
closeDropdown();
goto(url);
}
async function markAllRead() {
try {
await fetch('/api/notifications/read-all', { method: 'POST' });
for (const n of notifications) {
n.read = true;
}
unreadCount = 0;
} catch (e) {
console.error('Failed to mark all notifications as read', e);
}
}
function handleKeydown(event: KeyboardEvent) {
if (event.key === 'Escape' && open) {
event.stopPropagation();
closeDropdown();
}
}
// Attachment: stores the element reference for the bell button
function attachBellButton(node: HTMLButtonElement) {
bellButtonEl = node;
return () => {
bellButtonEl = null;
};
}
// Attachment: stores the element reference for the first focusable element in the dropdown
function attachFirstFocusable(node: HTMLButtonElement) {
firstFocusableEl = node;
return () => {
firstFocusableEl = null;
};
}
// Attachment: closes dropdown when clicking outside the wrapper element
function attachClickOutside(node: HTMLElement) {
const handleClick = (event: MouseEvent) => {
if (!node.contains(event.target as Node) && !event.defaultPrevented) {
if (open) {
open = false;
}
marcel commented 2026-03-27 22:11:38 +01:00
Outdated
Review
Copy Link

Hardcoded German strings bypass Paraglide.

relativeTime() returns 'gerade eben', 'vor X Min.', 'vor X Std.', 'vor X Tagen' as raw string literals. The aria-label="ungelesen" on the unread dot a few lines below has the same problem.

Both need translation keys in de.json / en.json / es.json and should go through m.xxx() like every other user-facing string in this codebase. The app already has Spanish and English users — relative timestamps showing German to them is a bug, not a cosmetic issue.

**Hardcoded German strings bypass Paraglide.** `relativeTime()` returns `'gerade eben'`, `'vor X Min.'`, `'vor X Std.'`, `'vor X Tagen'` as raw string literals. The `aria-label="ungelesen"` on the unread dot a few lines below has the same problem. Both need translation keys in `de.json` / `en.json` / `es.json` and should go through `m.xxx()` like every other user-facing string in this codebase. The app already has Spanish and English users — relative timestamps showing German to them is a bug, not a cosmetic issue.
}
};
document.addEventListener('click', handleClick, true);
return () => {
document.removeEventListener('click', handleClick, true);
};
}
function relativeTime(isoString: string): string {
const diffMs = Date.now() - new Date(isoString).getTime();
const diffMin = Math.floor(diffMs / 60000);
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 rtf.format(-diffH, 'hour');
const diffD = Math.floor(diffH / 24);
return rtf.format(-diffD, 'day');
}
onMount(() => {
fetchUnreadCount();
eventSource = new EventSource('/api/notifications/stream');
eventSource.addEventListener('notification', (e) => {
const notification = JSON.parse(e.data) as NotificationItem;
notifications = [notification, ...notifications];
if (!notification.read) unreadCount += 1;
});
});
onDestroy(() => {
eventSource?.close();
});
</script>
<svelte:window onkeydown={handleKeydown} />
<div class="relative" {@attach attachClickOutside}>
<!-- Bell button -->
<button
{@attach attachBellButton}
type="button"
onclick={toggleDropdown}
aria-label={unreadCount > 0
? m.notification_bell_unread_label({ count: unreadCount })
: m.notification_bell_label()}
aria-expanded={open}
aria-haspopup="true"
class="relative rounded-sm p-2 text-ink-2 transition-colors hover:text-ink focus:outline-none focus-visible:ring-2 focus-visible:ring-accent"
>
<!-- Bell SVG -->
<svg
xmlns="http://www.w3.org/2000/svg"
class="h-5 w-5"
fill="none"
viewBox="0 0 24 24"
stroke="currentColor"
stroke-width="2"
aria-hidden="true"
>
<path
stroke-linecap="round"
stroke-linejoin="round"
d="M15 17h5l-1.405-1.405A2.032 2.032 0 0118 14.158V11a6.002 6.002 0 00-4-5.659V5a2 2 0 10-4 0v.341C7.67 6.165 6 8.388 6 11v3.159c0 .538-.214 1.055-.595 1.436L4 17h5m6 0v1a3 3 0 11-6 0v-1m6 0H9"
/>
</svg>
<!-- Unread badge -->
{#if unreadCount > 0}
<span
aria-live="polite"
aria-atomic="true"
class="absolute -top-1 -right-1 flex h-5 min-w-5 items-center justify-center rounded-full bg-primary px-1 text-[10px] font-bold text-primary-fg"
>
{unreadCount}
</span>
{/if}
</button>
<!-- Dropdown -->
{#if open}
<div
role="dialog"
aria-modal="true"
aria-label={m.notification_bell_label()}
class="absolute right-0 z-50 mt-2 w-80 overflow-hidden rounded-sm border border-line bg-surface shadow-lg"
>
<!-- Header -->
<div class="flex items-center justify-between border-b border-line px-4 py-3">
<span class="text-xs font-bold tracking-widest text-ink-2 uppercase">
{m.notification_bell_label()}
</span>
{#if notifications.length > 0}
<button
{@attach attachFirstFocusable}
type="button"
onclick={markAllRead}
class="text-xs font-medium text-ink-3 transition-colors hover:text-ink"
>
{m.notification_mark_all_read()}
</button>
{/if}
</div>
<!-- Notification list -->
{#if notifications.length === 0}
<!-- Empty state -->
<div class="flex flex-col items-center gap-2 px-4 py-8 text-center text-sm text-ink-3">
<svg
xmlns="http://www.w3.org/2000/svg"
class="h-8 w-8 text-ink-3 opacity-40"
fill="none"
viewBox="0 0 24 24"
stroke="currentColor"
stroke-width="1.5"
aria-hidden="true"
>
<path
stroke-linecap="round"
stroke-linejoin="round"
d="M15 17h5l-1.405-1.405A2.032 2.032 0 0118 14.158V11a6.002 6.002 0 00-4-5.659V5a2 2 0 10-4 0v.341C7.67 6.165 6 8.388 6 11v3.159c0 .538-.214 1.055-.595 1.436L4 17h5m6 0v1a3 3 0 11-6 0v-1m6 0H9"
/>
</svg>
<span>{m.notification_empty()}</span>
</div>
{:else}
<ul role="list">
{#each notifications as notification (notification.id)}
<li>
<button
type="button"
onclick={() => markRead(notification)}
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' : ''}"
>
<!-- Type icon -->
<span class="mt-0.5 shrink-0 text-ink-3" aria-hidden="true">
{#if notification.type === 'REPLY'}
<!-- Reply icon -->
<svg
xmlns="http://www.w3.org/2000/svg"
class="h-4 w-4"
fill="none"
viewBox="0 0 24 24"
stroke="currentColor"
stroke-width="2"
>
<path
stroke-linecap="round"
stroke-linejoin="round"
d="M3 10h10a8 8 0 018 8v2M3 10l6 6m-6-6l6-6"
/>
</svg>
{:else}
<!-- Mention icon -->
<svg
xmlns="http://www.w3.org/2000/svg"
class="h-4 w-4"
fill="none"
viewBox="0 0 24 24"
stroke="currentColor"
stroke-width="2"
>
<path
stroke-linecap="round"
stroke-linejoin="round"
d="M16 12a4 4 0 10-8 0 4 4 0 008 0zm0 0v1.5a2.5 2.5 0 005 0V12a9 9 0 10-9 9m4.5-1.206a8.959 8.959 0 01-4.5 1.207"
/>
</svg>
{/if}
</span>
<!-- Text + time -->
<div class="min-w-0 flex-1">
<p class="text-sm leading-snug text-ink">
{notification.type === 'REPLY'
? m.notification_type_reply({ actor: notification.actorName })
: m.notification_type_mention({ actor: notification.actorName })}
</p>
<p class="mt-1 text-xs text-ink-3">{relativeTime(notification.createdAt)}</p>
</div>
<!-- Unread dot -->
{#if !notification.read}
<span
class="mt-1.5 h-2 w-2 shrink-0 rounded-full bg-primary"
aria-label={m.notification_unread()}
></span>
{/if}
</button>
</li>
{/each}
</ul>
{/if}
</div>
{/if}
</div>

13
frontend/src/lib/components/PanelDiscussion.svelte
View File

@@ -8,11 +8,19 @@ type Props = {
canComment: boolean;
currentUserId: string | null;
canAdmin: boolean;
targetCommentId?: string | null;
onCountChange?: (count: number) => void;
};
let { documentId, initialComments, canComment, currentUserId, canAdmin, onCountChange }: Props =
$props();
let {
documentId,
initialComments,
canComment,
currentUserId,
canAdmin,
targetCommentId = null,
onCountChange
}: Props = $props();
</script>
<div class="flex-1 overflow-y-auto p-6">
@@ -22,6 +30,7 @@ let { documentId, initialComments, canComment, currentUserId, canAdmin, onCountC
canComment={canComment}
currentUserId={currentUserId}
canAdmin={canAdmin}
targetCommentId={targetCommentId}
onCountChange={onCountChange}
/>
</div>

4
frontend/src/lib/components/user/UserGroupsSection.svelte
View File

@@ -6,6 +6,8 @@ let {
groups: { id: string; name: string }[];
selectedGroupIds?: string[];
} = $props();
let selected = $derived([...selectedGroupIds]);
</script>
<div class="flex flex-wrap gap-3">
@@ -15,7 +17,7 @@ let {
type="checkbox"
name="groupIds"
value={group.id}
checked={selectedGroupIds.includes(group.id)}
bind:group={selected}
class="rounded border-line text-ink focus:ring-accent"
/>
{group.name}

8
frontend/src/lib/types.ts
View File

@@ -1,3 +1,9 @@
export type MentionDTO = {
id: string;
firstName: string;
lastName: string;
};
export type CommentReply = {
id: string;
authorId: string | null;
@@ -5,6 +11,7 @@ export type CommentReply = {
content: string;
createdAt: string;
updatedAt: string;
mentionDTOs?: MentionDTO[];
};
export type Comment = {
@@ -15,6 +22,7 @@ export type Comment = {
createdAt: string;
updatedAt: string;
replies: CommentReply[];
mentionDTOs?: MentionDTO[];
};
export type DocumentPanelTab = 'metadata' | 'transcription' | 'discussion' | 'history';

128
frontend/src/lib/utils/mention.spec.ts Normal file
View File

@@ -0,0 +1,128 @@
import { describe, it, expect } from 'vitest';
marcel commented 2026-03-27 23:35:11 +01:00
Review
Copy Link

🔵 MINOR — XSS coverage missing in renderBody test suite

The spec covers HTML escaping of the comment body content, but has no test where a mention.firstName or mention.lastName contains HTML-special characters. This means the XSS vector identified in renderBody (Finding #9 above) would not be caught by CI even after the fix is applied — leaving the regression unguarded.

Fix: Add a test case:

it('should escape HTML in mention display names', () => {
  const body = '@<script>';
  const mentions = [{ id: 'u1', firstName: '<script>', lastName: 'alert(1)' }];
  const html = renderBody(body, mentions);
  expect(html).not.toContain('<script>');
  expect(html).toContain('&lt;script&gt;');
});
🔵 **MINOR — XSS coverage missing in `renderBody` test suite** The spec covers HTML escaping of the comment body content, but has no test where a `mention.firstName` or `mention.lastName` contains HTML-special characters. This means the XSS vector identified in `renderBody` (Finding #9 above) would not be caught by CI even after the fix is applied — leaving the regression unguarded. **Fix:** Add a test case: ```typescript it('should escape HTML in mention display names', () => { const body = '@<script>'; const mentions = [{ id: 'u1', firstName: '<script>', lastName: 'alert(1)' }]; const html = renderBody(body, mentions); expect(html).not.toContain('<script>'); expect(html).toContain('&lt;script&gt;'); }); ```
import { detectMention, extractContent, renderBody } from './mention';
import type { MentionDTO } from '$lib/types';
// ─── detectMention ────────────────────────────────────────────────────────────
describe('detectMention', () => {
it('returns null when text has no @', () => {
expect(detectMention('hello world', 11)).toBeNull();
});
it('returns null when @ is not the most recent trigger word', () => {
// cursor is past a completed mention (next word started)
expect(detectMention('hello @Hans Müller more', 22)).toBeNull();
});
it('returns empty string immediately after @', () => {
expect(detectMention('hello @', 7)).toBe('');
});
it('returns query text after @', () => {
expect(detectMention('hello @Han', 10)).toBe('Han');
});
it('returns null when @ is preceded by a letter (email address pattern)', () => {
expect(detectMention('user@example', 12)).toBeNull();
});
it('returns query for @ at the very start of string', () => {
expect(detectMention('@Hans', 5)).toBe('Hans');
});
it('returns null when cursor is before the @', () => {
expect(detectMention('@Hans', 0)).toBeNull();
});
});
// ─── extractContent ───────────────────────────────────────────────────────────
describe('extractContent', () => {
it('returns empty arrays for empty string', () => {
const result = extractContent('', []);
expect(result.content).toBe('');
expect(result.mentionedUserIds).toEqual([]);
});
it('returns plain content unchanged when no candidates', () => {
const result = extractContent('Hello world', []);
expect(result.content).toBe('Hello world');
expect(result.mentionedUserIds).toEqual([]);
});
it('extracts user id when @FirstName LastName is in content', () => {
const candidates: MentionDTO[] = [{ id: 'uuid-1', firstName: 'Hans', lastName: 'Müller' }];
const result = extractContent('Hey @Hans Müller how are you?', candidates);
expect(result.mentionedUserIds).toContain('uuid-1');
});
it('deduplicates user ids when same user mentioned twice', () => {
const candidates: MentionDTO[] = [{ id: 'uuid-1', firstName: 'Hans', lastName: 'Müller' }];
const result = extractContent('@Hans Müller and @Hans Müller again', candidates);
expect(result.mentionedUserIds).toHaveLength(1);
expect(result.mentionedUserIds).toContain('uuid-1');
});
it('collects multiple distinct users', () => {
const candidates: MentionDTO[] = [
{ id: 'uuid-1', firstName: 'Hans', lastName: 'Müller' },
{ id: 'uuid-2', firstName: 'Anna', lastName: 'Schmidt' }
];
const result = extractContent('@Hans Müller and @Anna Schmidt', candidates);
expect(result.mentionedUserIds).toContain('uuid-1');
expect(result.mentionedUserIds).toContain('uuid-2');
});
});
// ─── renderBody ───────────────────────────────────────────────────────────────
describe('renderBody', () => {
it('returns escaped plain text when no mentions', () => {
expect(renderBody('Hello world', [])).toBe('Hello world');
});
it('escapes < and > in content', () => {
const result = renderBody('<script>alert(1)</script>', []);
expect(result).toContain('&lt;script&gt;');
expect(result).not.toContain('<script>');
});
it('escapes & in content', () => {
const result = renderBody('AT&T', []);
expect(result).toContain('AT&amp;T');
});
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('<span');
expect(result).toContain('class="mention"');
expect(result).toContain('Hans Müller');
});
it('does not double-encode already escaped text', () => {
const mentions: MentionDTO[] = [{ id: 'uuid-1', firstName: 'Hans', lastName: 'Müller' }];
const result = renderBody('Check @Hans Müller', mentions);
expect(result).not.toContain('&amp;');
});
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 spanCount = (result.match(/<span /g) ?? []).length;
expect(spanCount).toBe(2);
});
it('escapes HTML special chars in mention display names', () => {
const mentions: MentionDTO[] = [{ id: 'u1', firstName: '<script>', lastName: 'alert(1)' }];
const result = renderBody('@<script> alert(1)', mentions);
expect(result).not.toContain('<script>');
expect(result).toContain('&lt;script&gt;');
});
it('converts newlines to <br>', () => {
const result = renderBody('line1\nline2', []);
expect(result).toContain('<br>');
expect(result).not.toContain('\n');
});
});

72
frontend/src/lib/utils/mention.ts Normal file
View File

@@ -0,0 +1,72 @@
import type { MentionDTO } from '$lib/types';
marcel commented 2026-03-27 23:34:59 +01:00
Review
Copy Link

⚠️ MAJOR — Stored XSS vector in renderBody: mention display names are not escaped

const link = `<a class="mention" data-user-id="${mention.id}" href="#">@${displayName}</a>`;

The comment body text is correctly escape-first-then-process, but mention.firstName and mention.lastName are injected into the link string without any HTML escaping. An admin who registers a user with lastName: '"\><img src=x onerror=alert(1)>' produces a working XSS payload rendered in every comment that mentions that user.

Fix: Apply the same HTML escape treatment to displayName before injecting it. Also add a test in mention.spec.ts:

it('escapes HTML special chars in mention display name', () => {
  const mentions = [{ id: 'u1', firstName: '<script>', lastName: 'alert(1)' }];
  const result = renderBody('@<script> alert(1)', mentions);
  expect(result).not.toContain('<script>');
});
⚠️ **MAJOR — Stored XSS vector in `renderBody`: mention display names are not escaped** ```typescript const link = `<a class="mention" data-user-id="${mention.id}" href="#">@${displayName}</a>`; ``` The comment body text is correctly escape-first-then-process, but `mention.firstName` and `mention.lastName` are injected into the link string without any HTML escaping. An admin who registers a user with `lastName: '"\><img src=x onerror=alert(1)>'` produces a working XSS payload rendered in every comment that mentions that user. **Fix:** Apply the same HTML escape treatment to `displayName` before injecting it. Also add a test in `mention.spec.ts`: ```typescript it('escapes HTML special chars in mention display name', () => { const mentions = [{ id: 'u1', firstName: '<script>', lastName: 'alert(1)' }]; const result = renderBody('@<script> alert(1)', mentions); expect(result).not.toContain('<script>'); }); ```
/**
* Given the current textarea value and cursor position, returns the
* @-mention query being typed (the text after the last triggering @),
* or null if no mention is active.
*
* Rules:
* - @ must be preceded by whitespace or be at the start of the string
* - The text between @ and the cursor must not contain a space (a
* completed mention word already has a space)
*/
export function detectMention(text: string, cursorPos: number): string | null {
const before = text.slice(0, cursorPos);
const atIndex = before.lastIndexOf('@');
if (atIndex === -1) return null;
// @ must be at start or preceded by whitespace
if (atIndex > 0 && !/\s/.test(before[atIndex - 1])) return null;
const query = before.slice(atIndex + 1);
// If the query contains a space the user has moved past the trigger word
if (query.includes(' ')) return null;
return query;
}
/**
* Given the raw textarea value and a list of candidate users (from the
* mention popup selections), returns the plain content string and the
* de-duplicated list of mentioned user IDs.
*/
export function extractContent(
text: string,
candidates: MentionDTO[]
): { content: string; mentionedUserIds: string[] } {
const seen = new Set<string>();
for (const user of candidates) {
const displayName = `${user.firstName} ${user.lastName}`.trim();
if (text.includes(`@${displayName}`)) {
seen.add(user.id);
}
}
return { content: text, mentionedUserIds: [...seen] };
}
/**
* Renders a comment body as safe HTML:
* 1. Escapes all HTML-special characters in the raw content
* 2. Replaces every @FirstName LastName occurrence with an anchor link
* 3. Converts newlines to <br>
*/
export function renderBody(content: string, mentions: MentionDTO[]): string {
let escaped = content
.replaceAll('&', '&amp;')
.replaceAll('<', '&lt;')
.replaceAll('>', '&gt;')
.replaceAll('"', '&quot;');
for (const mention of mentions) {
const displayName = `${mention.firstName} ${mention.lastName}`.trim();
const escapedDisplayName = displayName
.replaceAll('&', '&amp;')
marcel commented 2026-03-27 22:11:39 +01:00
Outdated
Review
Copy Link

href="#" scrolls the page to the top on click.

Mention links currently point to href="#". In most browsers clicking them fires the default anchor behaviour — the page jumps to the top, which is disorienting inside a document viewer.

Either use href="javascript:void(0)" (old-school but harmless) or, better, drop the <a> entirely and use a <span> styled as a mention chip:

const link = `<span class="mention" data-user-id="${mention.id}">@${displayName}</span>`;

There's no profile page to navigate to for family archive members, so a link that goes nowhere is actively worse than a styled span.

**`href="#"` scrolls the page to the top on click.** Mention links currently point to `href="#"`. In most browsers clicking them fires the default anchor behaviour — the page jumps to the top, which is disorienting inside a document viewer. Either use `href="javascript:void(0)"` (old-school but harmless) or, better, drop the `<a>` entirely and use a `<span>` styled as a mention chip: ```ts const link = `<span class="mention" data-user-id="${mention.id}">@${displayName}</span>`; ``` There's no profile page to navigate to for family archive members, so a link that goes nowhere is actively worse than a styled span.
.replaceAll('<', '&lt;')
.replaceAll('>', '&gt;')
.replaceAll('"', '&quot;');
const span = `<span class="mention" data-user-id="${mention.id}">@${escapedDisplayName}</span>`;
escaped = escaped.replaceAll(`@${escapedDisplayName}`, span);
}
return escaped.replaceAll('\n', '<br>');
}

4
frontend/src/routes/+layout.server.ts
View File

@@ -5,6 +5,8 @@ export const load: LayoutServerLoad = async ({ locals }) => {
return {
user: locals.user,
canWrite: groups.some((g) => g.permissions.includes('WRITE_ALL')),
canAnnotate: groups.some((g) => g.permissions.includes('ANNOTATE_ALL'))
canAnnotate: groups.some(
(g) => g.permissions.includes('WRITE_ALL') || g.permissions.includes('ANNOTATE_ALL')
)
};
};

6
frontend/src/routes/+layout.svelte
View File

@@ -4,6 +4,7 @@ import { page } from '$app/state';
import { onMount } from 'svelte';
import LanguageSwitcher from '$lib/components/LanguageSwitcher.svelte';
import ThemeToggle from '$lib/components/ThemeToggle.svelte';
import NotificationBell from '$lib/components/NotificationBell.svelte';
import AppNav from './AppNav.svelte';
import UserMenu from './UserMenu.svelte';
@@ -52,6 +53,11 @@ const userInitials = $derived.by(() => {
<!-- Theme toggle -->
<ThemeToggle />
<!-- Notification bell (authenticated users only) -->
{#if data?.user}
<NotificationBell />
{/if}
<!-- User menu -->
<UserMenu userInitials={userInitials} />
</div>

8
frontend/src/routes/admin/users/[id]/page.svelte.spec.ts
View File

@@ -94,6 +94,14 @@ describe('Admin edit user page – rendering', () => {
expect(checkbox?.checked).toBe(false);
});
it('includes pre-selected group ids in FormData at submit time (guards against groupIds being empty)', async () => {
render(Page, { data: baseData, form: null });
const form = document.querySelector('form')!;
const formData = new FormData(form);
expect(formData.getAll('groupIds')).toContain('g1');
expect(formData.getAll('groupIds')).not.toContain('g2');
});
it('password fields are empty by default', async () => {
render(Page, { data: baseData, form: null });
const passwordInputs = document.querySelectorAll<HTMLInputElement>('input[type="password"]');

15
frontend/src/routes/documents/[id]/+page.svelte
View File

@@ -1,5 +1,6 @@
<script lang="ts">
import { onMount } from 'svelte';
import { page } from '$app/state';
import DocumentTopBar from '$lib/components/DocumentTopBar.svelte';
import DocumentViewer from '$lib/components/DocumentViewer.svelte';
import DocumentBottomPanel from '$lib/components/DocumentBottomPanel.svelte';
@@ -8,6 +9,9 @@ import type { DocumentPanelTab } from '$lib/types';
let { data } = $props();
const targetCommentId = $derived(page.url.searchParams.get('commentId'));
const targetAnnotationId = $derived(page.url.searchParams.get('annotationId'));
const doc = $derived(data.document);
const canComment = $derived((data.canAnnotate || data.canWrite) ?? false);
const canAdmin = $derived(
@@ -92,7 +96,14 @@ onMount(() => {
if (!isNaN(h) && h >= 80) panelHeight = h;
}
if (savedOpen === 'true') {
if (targetAnnotationId) {
// Deep-link into an annotation comment: open the side panel
activeAnnotationId = targetAnnotationId;
} else if (targetCommentId) {
// Deep-link into a document-level comment: open discussion tab
panelOpen = true;
activeTab = 'discussion';
} else if (savedOpen === 'true') {
panelOpen = true;
} else if (savedOpen === null && !doc?.filePath) {
// No prior state and no file — open to metadata so the panel is immediately useful.
@@ -162,6 +173,7 @@ $effect(() => {
canComment={canComment}
currentUserId={currentUserId}
canAdmin={canAdmin}
targetCommentId={targetAnnotationId ? targetCommentId : null}
onClose={() => {
activeAnnotationId = null;
activeAnnotationPage = null;
@@ -175,6 +187,7 @@ $effect(() => {
canComment={canComment}
currentUserId={currentUserId}
canAdmin={canAdmin}
targetCommentId={targetCommentId}
bind:open={panelOpen}
bind:height={panelHeight}
bind:activeTab={activeTab}

23
frontend/src/routes/layout.css
View File

@@ -160,7 +160,28 @@
filter: invert(1);
}
/* ─── 7. Base styles ───────────────────────────────────────────────────────── */
/* ─── 7. @mention chip ─────────────────────────────────────────────────────── */
/*
Rendered by renderBody() via {@html ...} in CommentThread.svelte.
Must live in global CSS — Svelte scoped styles don't reach injected HTML.
*/
.mention {
display: inline;
color: var(--c-ink);
background-color: var(--c-accent-bg);
border-radius: 3px;
padding: 0 3px;
font-weight: 600;
font-style: normal;
cursor: default;
transition: background-color 0.15s ease;
}
.mention:hover {
background-color: color-mix(in srgb, var(--c-accent) 25%, transparent);
}
/* ─── 8. Base styles ───────────────────────────────────────────────────────── */
@layer base {
html {
overscroll-behavior: none;

4
frontend/src/routes/layout.svelte.spec.ts
View File

@@ -1,8 +1,10 @@
import { afterEach, describe, expect, it } from 'vitest';
import { afterEach, describe, expect, it, vi } from 'vitest';
import { cleanup, render } from 'vitest-browser-svelte';
import { page, userEvent } from 'vitest/browser';
import { createRawSnippet } from 'svelte';
vi.mock('$env/static/public', () => ({ PUBLIC_NOTIFICATION_POLL_MS: '60000' }));
afterEach(cleanup);
const emptySnippet = createRawSnippet(() => ({ render: () => '<span></span>' }));

30
frontend/src/routes/profile/+page.server.ts
View File

@@ -1,10 +1,15 @@
import { fail } from '@sveltejs/kit';
marcel commented 2026-03-27 23:35:08 +01:00
Review
Copy Link

⚠️ MAJOR — Checkbox preference values are unreliable without JS

The updateNotificationPrefs action reads:

notifyOnReply: formData.get('notifyOnReply') === 'true',

…from a <input type="hidden"> that is synced to a Svelte $state boolean via JS. When JS is disabled or use:enhance fails, unchecked checkboxes are not submitted in standard HTML forms — the hidden input will always carry its last JS-bound value, not the actual checkbox state. Preferences can be silently saved incorrectly.

Fix: Either read the checkbox directly using formData.has('notifyOnReply') (standard HTML checkbox presence pattern), or explicitly document and guard that this form requires JavaScript to function correctly.

⚠️ **MAJOR — Checkbox preference values are unreliable without JS** The `updateNotificationPrefs` action reads: ```typescript notifyOnReply: formData.get('notifyOnReply') === 'true', ``` …from a `<input type="hidden">` that is synced to a Svelte `$state` boolean via JS. When JS is disabled or `use:enhance` fails, unchecked checkboxes are not submitted in standard HTML forms — the hidden input will always carry its last JS-bound value, not the actual checkbox state. Preferences can be silently saved incorrectly. **Fix:** Either read the checkbox directly using `formData.has('notifyOnReply')` (standard HTML checkbox presence pattern), or explicitly document and guard that this form requires JavaScript to function correctly.
import { env } from '$env/dynamic/private';
import type { PageServerLoad, Actions } from './$types';
import { createApiClient } from '$lib/api.server';
import { getErrorMessage } from '$lib/errors';
export const load: PageServerLoad = async ({ locals }) => {
return { user: locals.user };
const apiBase = () => env.API_INTERNAL_URL || 'http://localhost:8080';
export const load: PageServerLoad = async ({ locals, fetch }) => {
const res = await fetch(`${apiBase()}/api/users/me/notification-preferences`);
const notificationPrefs = res.ok ? await res.json() : null;
return { user: locals.user, notificationPrefs };
};
export const actions: Actions = {
@@ -50,5 +55,26 @@ export const actions: Actions = {
}
return { passwordSuccess: true };
},
updateNotificationPrefs: async ({ request, fetch }) => {
const formData = await request.formData();
const body = {
notifyOnReply: formData.has('notifyOnReply'),
notifyOnMention: formData.has('notifyOnMention')
};
const res = await fetch(`${apiBase()}/api/users/me/notification-preferences`, {
method: 'PUT',
headers: { 'Content-Type': 'application/json' },
body: JSON.stringify(body)
});
if (!res.ok) {
const data = await res.json().catch(() => ({}));
return fail(res.status, { prefsError: getErrorMessage(data?.code) });
}
return { prefsSuccess: true };
}
};

71
frontend/src/routes/profile/+page.svelte
View File

@@ -1,9 +1,14 @@
<script lang="ts">
import { enhance } from '$app/forms';
import { m } from '$lib/paraglide/messages.js';
import PersonalInfoForm from './PersonalInfoForm.svelte';
import PasswordChangeForm from './PasswordChangeForm.svelte';
let { data, form } = $props();
let notifyOnReply = $derived(data.notificationPrefs?.notifyOnReply ?? false);
let notifyOnMention = $derived(data.notificationPrefs?.notifyOnMention ?? false);
const hasEmail = $derived(!!data.user?.email);
</script>
<div class="mx-auto max-w-7xl px-4 py-8 sm:px-6 lg:px-8">
@@ -30,4 +35,70 @@ let { data, form } = $props();
<PersonalInfoForm user={data.user} form={form} />
<PasswordChangeForm form={form} />
</div>
<!-- Notification preferences -->
<div class="mt-6 rounded-sm border border-line bg-surface p-6 shadow-sm">
<h2 class="mb-5 text-xs font-bold tracking-widest text-ink-3 uppercase">
{m.notification_prefs_heading()}
</h2>
{#if form?.prefsSuccess}
<div class="mb-5 rounded border border-green-200 bg-green-50 p-3 text-sm text-green-700">
{m.profile_saved()}
</div>
{/if}
{#if form?.prefsError}
<div class="mb-5 rounded border border-red-200 bg-red-50 p-3 text-sm text-red-700">
{form.prefsError}
</div>
{/if}
<form
method="POST"
action="?/updateNotificationPrefs"
use:enhance={() => async ({ update }) => update({ reset: false })}
>
<div class="space-y-4">
<label
class="flex items-start gap-3 {hasEmail ? 'cursor-pointer' : 'cursor-not-allowed opacity-40'}"
>
<input
type="checkbox"
name="notifyOnReply"
bind:checked={notifyOnReply}
disabled={!hasEmail}
class="mt-0.5 h-4 w-4 rounded border-line accent-primary"
/>
<span class="text-sm text-ink">{m.notification_pref_reply()}</span>
</label>
<label
class="flex items-start gap-3 {hasEmail ? 'cursor-pointer' : 'cursor-not-allowed opacity-40'}"
>
<input
type="checkbox"
name="notifyOnMention"
bind:checked={notifyOnMention}
disabled={!hasEmail}
class="mt-0.5 h-4 w-4 rounded border-line accent-primary"
/>
<span class="text-sm text-ink">{m.notification_pref_mention()}</span>
</label>
</div>
{#if !hasEmail}
<p class="mt-3 text-xs text-ink-3">
{m.notification_prefs_no_email()}
</p>
{/if}
<button
type="submit"
disabled={!hasEmail}
class="mt-5 rounded-sm bg-primary px-5 py-2 font-sans text-xs font-bold tracking-widest text-primary-fg uppercase transition-opacity {hasEmail ? 'hover:opacity-80' : 'cursor-not-allowed opacity-40'}"
>
{m.btn_save()}
</button>
</form>
</div>
</div>