Fixes from PR #178 review:
Migration fixes:
- V18/V19: fix FK references from app_users to users (correct table name)
- V18: change annotation_id FK from ON DELETE CASCADE to ON DELETE RESTRICT
(block is aggregate root, cascade flows from block, not annotation)
Backend fixes:
- TranscriptionService.deleteBlock(): remove userId param, delete block first
then annotation directly via repository (no ownership check — block owns annotation)
- TranscriptionService.sanitizeText(): remove flawed regex HTML stripping,
textarea content is plain text by design — just enforce max length
- TranscriptionBlockController.requireUserId(): throw DomainException.unauthorized()
instead of silently returning null on auth failure
- CreateTranscriptionBlockDTO: add @Min/@Positive validation on coordinates
- Add @Slf4j logging to TranscriptionService for create/delete operations
Frontend fixes:
- Delete DocumentBottomPanel.svelte entirely (issue #175 requirement)
- Remove redundant mode exclusivity $effect (handled at toggle call sites)
- Remove dead handleCommentClick + onCommentClick prop (comments are future work)
- Remove quote hint UI (depends on comment feature)
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
TranscriptionBlock entity with @Version optimistic locking
TranscriptionBlockVersion for edit history
TranscriptionService facade: CRUD, reorder, version history
TranscriptionBlockController: REST endpoints under /api/documents/{docId}/transcription-blocks
DTOs: Create, Update, Reorder
ErrorCode: TRANSCRIPTION_BLOCK_NOT_FOUND, TRANSCRIPTION_BLOCK_CONFLICT
DocumentComment: add block_id field for block-level comment threads
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
V18: transcription_blocks table with optimistic locking version column
V19: transcription_block_versions for edit history capture
V20: add block_id FK to document_comments for block-level threads
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Blockers (14):
- B1: fix senderName/receiverName to use $derived instead of $state + sync $effect
- B2: migrate all korrespondenz components from messages-extra shim to paraglide m.*
- B3: i18n CorrespondenzEmptyState (heading, subtext, search placeholder)
- B4: add response.ok checks to admin layout server load
- B5: add response.ok checks to korrespondenz page server load
- B6: add page.server.spec.ts with 5 test suites for korrespondenz load function
- B7: add axe-core accessibility checks to all e2e korrespondenz tests
- B8: add Testcontainers JPQL tests for findSinglePersonCorrespondence (DISTINCT + sender)
- B9: hide auth reset-token endpoint from OpenAPI spec; remove from generated api.ts
- B11: replace amber hardcoded hex colors in SinglePersonHintBar with brand tokens
- B12: replace clipboard emoji with Heroicons SVG in SinglePersonHintBar
- B13: create DateInput component (German dd.mm.yyyy); use it in CorrespondenzFilterControls
- B14: add Paraglide compile step to CI workflow before lint/test
Suggestions (11):
- S1: make CorrespondentSuggestionsDropdown a pure display component; lift fetch to PersonBar
- S2: fix leftover messages-extra import in ConversationTimeline; use brand tokens for status dots
- S3: add intent comment to EntityNav openFlyout behavior
- S4: rename canManageGroups → canManagePermissions throughout admin
- S6: remove domFlush helper from DateInput spec; use expect.poll instead
- S7: replace test.skip with throw new Error in bilateral e2e tests
- S8: add inverse aria-disabled test for filter strip
- S9: remove sm:min-h-0 from sort button to preserve 44px touch target
- S10: add title attributes to tablet trigger buttons in EntityNav
- S11: delete messages-extra.ts shim entirely
Also: fix admin pages revealing blank strip at bottom (-mb-6 on admin layout)
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
A query of only spaces previously fell through to findAllWithDocumentCount,
exposing the full person list. Whitespace-only queries now return empty.
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
When receiverId is omitted, returns all documents where the person is
sender or receiver (single-person mode). Bilateral mode is unchanged.
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Fixes IDOR: the endpoint was publicly accessible to any authenticated user.
Now requires ADMIN_USER permission, matching all other user management endpoints.
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Resolved conflicts in messages/de.json, en.json, es.json by keeping
both the persons-redesign keys (feature branch) and the notification
keys (main) in all three locale files.
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Native queries compute sender + receiver document count in one SQL call,
eliminating N+1. GET /api/persons now returns PersonSummaryDTO list.
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
createPerson now takes PersonUpdateDTO, persisting birthYear, deathYear,
notes in addition to firstName, lastName, alias.
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Added PERSON_NOT_FOUND to ErrorCode; getById, updatePerson, mergePersons
now throw DomainException.notFound for missing persons.
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
birthYear and deathYear must be positive integers; extracted shared
validateYears() method for reuse in createPerson.
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
firstName/lastName max 100, alias max 200, notes max 5000 chars.
PUT /api/persons/{id} returns 400 for oversized fields.
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
POST /api/persons, PUT /api/persons/{id}, POST /api/persons/{id}/merge
now return 403 for READ_ALL-only users.
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
NullX Finding 2: unbounded size param allowed full table scan. Added
spring-boot-starter-validation, @Validated on the controller, @Min(1) @Max(100)
on the size param, and ConstraintViolationException → 400 in GlobalExceptionHandler.
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Notification rows in the history page need the document title. Added
findTitlesByIds(Collection<UUID>) to DocumentService (one query via a new
JPQL projection on DocumentRepository). NotificationService.getNotifications()
now fetches all titles for the page in a single extra query and maps them into
the DTO. documentTitle is null when the document has been deleted.
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
NullX Finding 1: GET /api/notifications?read=false with no type param fell through
to the all-notifications branch, silently ignoring the read filter. Added
findByRecipientIdAndReadFalseOrderByCreatedAtDesc to NotificationRepository and
the missing Boolean.FALSE.equals(read) branch in NotificationService.
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
- DocumentService.getRecentActivity: replace findAll(Sort)+stream().limit()
with findAll(PageRequest) so LIMIT is pushed to the database
- +page.svelte: collapse two-column grid to single column when mentions is empty
- DashboardNeedsMetadata: raise "show all" link from text-xs (12px) to text-sm
(14px) and add hover:underline for WCAG 1.4.1
- DashboardRecentDocuments: add comment explaining why T12:00:00 noon-anchor
is absent (updatedAt is a full ISO datetime, not a date-only string)
- DocumentServiceTest: update getRecentActivity tests to assert PageRequest
usage instead of findAll(Sort)
- DocumentRepositoryTest: add @DataJpaTest verifying findAll(PageRequest)
returns only size rows, not the full table
- DocumentControllerTest: add test for default size=5 when param is omitted
- NotificationServiceTest: add test documenting that type+read=true falls
through to the type-only query (intentional)
- page.server.spec.ts: replace stale tests with full dashboard-mode coverage
- DashboardMentions.svelte.spec.ts: add tests for REPLY type and absent documentId
- DashboardResumeStrip.svelte.spec.ts: add corrupt localStorage test
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
spring.jpa.open-in-view=true (the default) holds a DB connection open for
the entire HTTP request lifecycle. Under concurrent dashboard API calls
(Promise.allSettled fires 3 at once), the pool of 10 is exhausted and the
backend crashes with connection timeout errors.
Setting open-in-view=false releases connections as soon as each
@Transactional method completes.
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Add GET /api/documents/recent-activity?size=N endpoint that returns
the N most recently updated documents sorted by updatedAt DESC.
Includes TDD: failing tests written first, then production code.
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
- Add type-only filter to notification repo/service (previously only
worked with type+read=false together)
- Dashboard widget now fetches all recent notifications (mentions +
replies, both read and unread) instead of unread mentions only
- Update component heading and show type label per row
Root cause: Berit's mentions were read=true, so the unread-only filter
returned 0 results. The recent docs widget had no REVIEWED documents
because 'marking ready' sets metadata_complete, not status=REVIEWED.
Recent docs now shows all uploads without a status filter.
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Dashboard "Recently Added" widget calls ?status=REVIEWED&size=5.
Null status is a no-op — existing callers without the param are unaffected.
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Dashboard widget calls ?size=3 to cap the list. Response now returns
{id, title} DTO instead of full Document entity.
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Dashboard widget uses ?type=MENTION&read=false to fetch unread mentions.
Also adds MethodArgumentTypeMismatchException → 400 handler so invalid
enum values in any @RequestParam return 400 instead of 500.
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Add unit tests for all service classes. Cover happy paths, error paths, and edge cases including structurally unreachable null guards via reflection to reach 90.2% branch coverage (431/478) in the service package.
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Native queries bypass the JPA first-level cache; flush+clear is required before
reloading entities to see the updated state in the same transaction.
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
- PersonControllerTest: expand from 2 to 26 tests — covers all endpoints
(GET persons/id/correspondents/documents, POST create/merge, PUT update)
and all validation branches (missing/blank firstName, lastName,
targetPersonId → 400). Reveals and fixes a real bug: ResponseStatusException
thrown by controllers was caught by the catch-all ExceptionHandler(Exception)
in GlobalExceptionHandler, returning 500 instead of the intended status.
Fix: add explicit ExceptionHandler(ResponseStatusException) handler.
- DocumentSpecificationsTest: 18 @DataJpaTest tests covering every branch in
DocumentSpecifications (hasText null/blank/match/case, hasSender null/match,
hasReceiver null/match, isBetween both-null/both-set/start-only/end-only,
hasTags null/empty/match/AND-logic/case/whitespace-skip). This is the
primary driver of the 0% repository branch coverage reported in #148.
- PersonRepositoryTest: 10 new tests for previously untested native queries —
findCorrespondents (order by doc count), findCorrespondentsWithFilter
(case-insensitive), reassignSender, insertMissingReceiverReference
(no-duplicate guard), deleteReceiverReferences.
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Adds JaCoCo 0.8.12 with prepare-agent, report, and check executions.
Baseline measured at 46.8% branch coverage. Gate set at 42% (baseline
minus 5%) to prevent regression while giving room to close the gap.
Excluded from measurement: DTOs, config classes, model entities,
ErrorCode enum — these contain no testable branch logic.
Target is 80%; gap documented in issue #120.
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Adds spring-boot-testcontainers and testcontainers-postgresql deps.
PostgresContainerConfig declares a shared @ServiceConnection container
used by DocumentRepositoryTest, PersonRepositoryTest, and an
ApplicationContextTest smoke test.
Flyway migrations are imported via FlywayConfig and run on every test
execution, verifying the migration chain against a real PostgreSQL 16
container. No H2 is used.
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
All five comment write endpoints (post doc comment, reply to doc comment,
post annotation comment, reply to annotation comment, edit comment) only
listed ANNOTATE_ALL in @RequirePermission. Users with WRITE_ALL received
403 on every comment action. Same pattern as the annotation fix.
Tests: CommentControllerTest (+5 RED→GREEN for WRITE_ALL on each method).
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
@RequirePermission on POST and DELETE annotation endpoints previously
only listed ANNOTATE_ALL. Users with WRITE_ALL (but not ANNOTATE_ALL)
received 403. A user who can write documents should also be able to
annotate them — both permissions now accepted on both methods.
Also updates canAnnotate in +layout.server.ts to match, so the UI
correctly reflects annotation capability for WRITE_ALL users.
Tests: AnnotationControllerTest (+2 RED→GREEN).
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Replaced one-way checked={...} with bind:group={selected} driven by a
writable $derived. In Svelte 5, the $derived pattern guarantees the DOM
checked state is always in sync at FormData capture time, so groupIds
is never accidentally sent as [] when the admin edits their own profile.
Sending groupIds:[] causes adminUpdateUser to clear all groups, which
revokes the admin's own permissions on the next request.
Tests: UserServiceTest (+4 for adminUpdateUser group behaviour),
page.svelte.spec.ts (+1 FormData assertion at submit time).
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
- Remove @RequirePermission(READ_ALL) from NotificationController class level so
authenticated users with any permission (or none) can access their own notifications
- Add V19 migration, annotationId field to Notification entity and NotificationDTO
- NotificationService now stores annotationId from comment on both REPLY and MENTION
- Update controller tests: permission tests now expect 200, DTO constructor includes annotationId
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
@RequirePermission now accepts Permission[] so a single annotation can
express "any of these" rather than a single required permission.
PermissionAspect updated accordingly — all existing single-value usages
compile unchanged (Java auto-wraps scalars in arrays for annotation attrs).
NotificationController: preference endpoints (GET/PUT /api/users/me/
notification-preferences) override the class-level READ_ALL gate with
{READ_ALL, WRITE_ALL, ANNOTATE_ALL} so users without READ_ALL can still
manage their own settings. Notification list endpoints retain READ_ALL.
UserSearchController: same broadened set so ANNOTATE_ALL users can search
for users to @mention when writing comments.
Tests: added WRITE_ALL and ANNOTATE_ALL passing cases for preferences and
user search; added 403 case for preferences with no permission; confirmed
WRITE_ALL cannot reach notification list endpoints.
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
BLOCKERs:
- Remove direct AppUserRepository/CommentRepository access from CommentService and
NotificationService — replaced with UserService.findAllById() and UserService
(fixes layering contract from CLAUDE.md)
- Switch Optional<JavaMailSender> constructor injection — removes @Autowired(required=false)
field and ReflectionTestUtils hack in tests
- Add @RequirePermission(READ_ALL) to UserSearchController — prevents user enumeration
without read access
Data bug:
- Promote actorName from @Transient to persisted VARCHAR column (V18 migration)
- Set actorName in notifyReply and notifyMentions from comment.getAuthorName()
Architecture:
- Add @RequirePermission(READ_ALL) to NotificationController
- Introduce NotificationDTO — controller returns DTO instead of Notification entity,
eliminating lazy-load N+1 and AppUser field leakage
- Change mentions FetchType to EAGER — fixes LazyInitializationException outside transaction
- Add @Transactional(propagation=REQUIRES_NEW) to notifyReply/notifyMentions so a
notification failure cannot roll back the parent comment
- N+1 fix: replace per-ID findById loops with single findAllById bulk fetch
- Move collectParticipantIds to CommentService; notifyReply accepts Set<UUID> directly
Security:
- Escape displayName before injecting into renderBody HTML span
- Replace <a href="#"> with <span class="mention"> — no profile page to link to, and
the anchor's scroll-to-top behaviour is harmful
Tests added/fixed:
- markRead_throwsNotFound, markAllRead_delegatesToRepository, countUnread_delegatesToRepository
- markOneRead_returns401, @RequirePermission 403 coverage for both controllers
- postComment/replyToComment_triggersNotifyMentions_whenMentionedUserIdsProvided
- search_returnsAtMostTenResults now asserts $.length() <= 10
- XSS regression test for escaped displayName in mention.spec.ts
Frontend minors:
- relativeTime() uses Intl.RelativeTimeFormat (locale-aware, not German-hardcoded)
- aria-label uses m.notification_unread() Paraglide key (de/en/es added)
- <div role="button"> replaced with <button> (native Enter+Space handling)
- onDestroy clears debounceTimer in MentionEditor
- setTimeout(100) replaced with await tick() + requestAnimationFrame in CommentThread
- Notification prefs form uses checkbox name attributes + formData.has() pattern
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
When a document is created without an explicit title (null or blank),
the service now derives the title from the uploaded filename using the
same titleFromFilename() logic already used by storeDocument — stripping
the extension for plain names and formatting structured names as
"Firstname Lastname (DD.MM.YYYY)".
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
storeDocument() now uses the ParsedFilename record to also set
documentDate and sender on new quick-uploads. Sender lookup is
an exact case-insensitive first+last name match — no new persons
are created. Unmatched filenames behave as before.
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Replace the four fixed regexes with a split-based algorithm:
- first segment = date → last segment = firstName, rest = lastName parts
- last segment = date → second-to-last = firstName, rest = lastName parts
18881025_de_Gruyter_Walter.pdf now correctly yields "Walter de Gruyter".
Simple two-segment names behave identically to before.
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
titleFromFilename() mirrors the same four patterns as the frontend
parseFilename() utility. Dropzone uploads to Mueller_Hans_19650312.pdf
now land with title "Hans Mueller (12.03.1965)" instead of the raw
stripped filename.
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Adds a metadata_complete column (default true for existing rows) to drive
the enrichment queue. New drop-zone uploads always start as false; createDocument
uses an explicit DTO flag or a heuristic (any of date/sender/receivers present →
true); the mass importer applies the same heuristic per row.
New endpoints: GET /api/documents/incomplete-count, /incomplete, /incomplete/next.
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
- storeDocument now returns StoreResult(document, isNew) to distinguish
new uploads from updates to existing documents
- QuickUploadResult gains an `updated` list alongside `created`
- Frontend shows an amber warning with a "View document" link for duplicates
instead of silently re-uploading and leaving the user confused
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
- Add V14 migration: ON DELETE CASCADE for document_tags and document_receivers
so deleting a document removes its join-table rows automatically
- Rename default form action to 'update' in the edit page — SvelteKit forbids
mixing a default action with named actions (was causing 500 on delete)
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>