Compare commits

..

8 Commits

Author SHA1 Message Date
Marcel
5512790d5a ci: track act_runner config with Docker socket mount
Some checks failed
CI / Unit & Component Tests (push) Has been cancelled
CI / OCR Service Tests (push) Has been cancelled
CI / Backend Unit Tests (push) Has been cancelled
CI / Unit & Component Tests (pull_request) Failing after 4m31s
CI / OCR Service Tests (pull_request) Successful in 30s
CI / Backend Unit Tests (pull_request) Failing after 3m17s
Documents the NAS runner configuration needed for Testcontainers.
Must be deployed to the runner host alongside the act_runner binary.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
2026-05-09 16:03:36 +02:00
Marcel
a158048f45 fix(ci): expose Docker socket env vars for Testcontainers in backend job
DOCKER_HOST makes the socket explicit rather than relying on runner
config propagation; TESTCONTAINERS_RYUK_DISABLED=true avoids Ryuk
watchdog start failures in nested container environments.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
2026-05-09 16:03:36 +02:00
Marcel
ac999066dd fix(ci): add TZ=Europe/Berlin to frontend test step
date-buckets.spec.ts midnight tests pass timezone-aware dates (+02:00)
which are 22:00 UTC the prior day; setHours(0,0,0,0) uses local TZ.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
2026-05-09 16:03:36 +02:00
Marcel
8b25a5b940 fix(user): replace Math.abs(hashCode()) with Math.floorMod in computeColor
Some checks failed
CI / Unit & Component Tests (push) Has been cancelled
CI / OCR Service Tests (push) Has been cancelled
CI / Backend Unit Tests (push) Has been cancelled
Math.abs(Integer.MIN_VALUE) overflows back to Integer.MIN_VALUE (negative),
making the old pattern unsafe for any palette size that doesn't evenly divide
MIN_VALUE. Math.floorMod always returns a non-negative residue in [0, n-1],
eliminating the overflow edge case entirely.

Fixes SpotBugs RV_ABSOLUTE_VALUE_OF_HASHCODE (priority 1, CORRECTNESS).
Closes #471

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
2026-05-09 15:48:59 +02:00
Marcel
265b4f1484 fix(comment): declare missing @PathVariable params on block comment endpoints
Some checks failed
CI / Unit & Component Tests (push) Has been cancelled
CI / OCR Service Tests (push) Has been cancelled
CI / Backend Unit Tests (push) Has been cancelled
getBlockComments was missing documentId; replyToBlockComment was missing
blockId. Spring silently ignored undeclared path variables — the segments
were parsed but never bound. Now both parameters are explicitly declared so
Spring rejects non-UUID values with 400.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
2026-05-09 15:45:48 +02:00
Marcel
bfc3a17676 test(migration): guard cleanup in try-finally to ensure isolation
Some checks failed
CI / Unit & Component Tests (pull_request) Failing after 3m57s
CI / OCR Service Tests (pull_request) Successful in 40s
CI / Backend Unit Tests (pull_request) Failing after 3m21s
CI / Unit & Component Tests (push) Failing after 4m1s
CI / OCR Service Tests (push) Successful in 38s
CI / Backend Unit Tests (push) Failing after 3m24s
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
2026-05-09 15:25:26 +02:00
Marcel
eb54a98ea2 fix(user): use builder in createGroup and guard against null permissions
Some checks failed
CI / Unit & Component Tests (push) Has been cancelled
CI / OCR Service Tests (push) Has been cancelled
CI / Backend Unit Tests (push) Has been cancelled
CI / Unit & Component Tests (pull_request) Failing after 4m2s
CI / OCR Service Tests (pull_request) Successful in 37s
CI / Backend Unit Tests (pull_request) Failing after 3m18s
Null dto.permissions now produces an empty HashSet instead of propagating null
into the @ElementCollection — prevents a silent NPE after V64 adds NOT NULL.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
2026-05-09 15:19:20 +02:00
Marcel
3fcdfa85f1 fix(db): add PRIMARY KEY to group_permissions; promote tbmp UNIQUE to PK
V63 deduplicates any phantom (group_id, permission) rows accumulated since
the initial schema. V64 sets NOT NULL on permission and adds pk_group_permissions.
V65 renames uq_tbmp_block_person to pk_tbmp for naming-convention consistency.
Integration tests confirm each constraint via pg_catalog.pg_constraint. Closes #469 (partial).

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
2026-05-09 15:18:46 +02:00
12 changed files with 151 additions and 119 deletions

View File

@@ -27,7 +27,9 @@ public class CommentController {
// ─── Block (transcription) comments ────────────────────────────────────────
@GetMapping("/api/documents/{documentId}/transcription-blocks/{blockId}/comments")
public List<DocumentComment> getBlockComments(@PathVariable UUID blockId) {
public List<DocumentComment> getBlockComments(
@PathVariable UUID documentId,
@PathVariable UUID blockId) {
return commentService.getCommentsForBlock(blockId);
}
@@ -48,6 +50,7 @@ public class CommentController {
@RequirePermission({Permission.ANNOTATE_ALL, Permission.WRITE_ALL})
public DocumentComment replyToBlockComment(
@PathVariable UUID documentId,
@PathVariable UUID blockId,
@PathVariable UUID commentId,
@RequestBody CreateCommentDTO dto,
Authentication authentication) {

View File

@@ -88,7 +88,8 @@ public class AppUser {
};
public static String computeColor(UUID id) {
return PALETTE[Math.abs(id.hashCode()) % PALETTE.length];
// Math.floorMod avoids the Integer.MIN_VALUE overflow trap in Math.abs(hashCode())
return PALETTE[Math.floorMod(id.hashCode(), PALETTE.length)];
}
@PrePersist

View File

@@ -271,9 +271,10 @@ public class UserService {
@Transactional
public UserGroup createGroup(GroupDTO dto) {
UserGroup group = new UserGroup();
group.setName(dto.getName());
group.setPermissions(dto.getPermissions());
UserGroup group = UserGroup.builder()
.name(dto.getName())
.permissions(dto.getPermissions() != null ? dto.getPermissions() : new HashSet<>())
.build();
return groupRepository.save(group);
}

View File

@@ -0,0 +1,7 @@
-- Remove duplicate (group_id, permission) rows that accumulated without a UNIQUE constraint.
-- Keeps the row with the smallest ctid (earliest physical insertion order).
DELETE FROM group_permissions a
USING group_permissions b
WHERE a.ctid < b.ctid
AND a.group_id = b.group_id
AND a.permission = b.permission;

View File

@@ -0,0 +1,11 @@
-- Add NOT NULL and PRIMARY KEY to group_permissions.
-- Requires V63 to have run first (no duplicates can remain).
--
-- After this migration, future seed migrations can use:
-- INSERT INTO group_permissions ... ON CONFLICT DO NOTHING
-- instead of the INSERT ... WHERE NOT EXISTS pattern used before V64.
ALTER TABLE group_permissions
ALTER COLUMN permission SET NOT NULL;
ALTER TABLE group_permissions
ADD CONSTRAINT pk_group_permissions PRIMARY KEY (group_id, permission);

View File

@@ -0,0 +1,8 @@
-- Promote the de-facto unique constraint on transcription_block_mentioned_persons to a named PK.
-- uq_tbmp_block_person (added in V57) is backed by a B-tree index identical to a PK;
-- this rename makes the naming convention explicit (pk_* vs uq_*).
ALTER TABLE transcription_block_mentioned_persons
DROP CONSTRAINT uq_tbmp_block_person;
ALTER TABLE transcription_block_mentioned_persons
ADD CONSTRAINT pk_tbmp PRIMARY KEY (block_id, person_id);

View File

@@ -399,6 +399,68 @@ class MigrationIntegrationTest {
AND dc.annotation_id IS NOT NULL
""";
// ─── V63+V64: group_permissions dedup + primary key ──────────────────────
@Test
void v64_pk_group_permissions_exists() {
Integer count = jdbc.queryForObject(
"""
SELECT COUNT(*) FROM pg_catalog.pg_constraint c
JOIN pg_catalog.pg_class t ON c.conrelid = t.oid
WHERE t.relname = 'group_permissions'
AND c.conname = 'pk_group_permissions'
AND c.contype = 'p'
""",
Integer.class);
assertThat(count).isEqualTo(1);
}
@Test
void v64_permission_column_isNotNullable() {
Integer count = jdbc.queryForObject(
"""
SELECT COUNT(*) FROM information_schema.columns
WHERE table_schema = 'public'
AND table_name = 'group_permissions'
AND column_name = 'permission'
AND is_nullable = 'NO'
""",
Integer.class);
assertThat(count).isEqualTo(1);
}
@Test
@Transactional(propagation = Propagation.NOT_SUPPORTED)
void v64_rejectsDuplicateGroupPermission() {
UUID groupId = createUserGroup("DuplicateTestGroup-" + UUID.randomUUID());
try {
jdbc.update("INSERT INTO group_permissions (group_id, permission) VALUES (?, 'READ_ALL')", groupId);
assertThatThrownBy(() ->
jdbc.update("INSERT INTO group_permissions (group_id, permission) VALUES (?, 'READ_ALL')", groupId)
).isInstanceOf(DataIntegrityViolationException.class);
} finally {
jdbc.update("DELETE FROM group_permissions WHERE group_id = ?", groupId);
jdbc.update("DELETE FROM user_groups WHERE id = ?", groupId);
}
}
// ─── V65: tbmp UNIQUE promoted to PRIMARY KEY ─────────────────────────────
@Test
void v65_pk_tbmp_exists() {
Integer count = jdbc.queryForObject(
"""
SELECT COUNT(*) FROM pg_catalog.pg_constraint c
JOIN pg_catalog.pg_class t ON c.conrelid = t.oid
WHERE t.relname = 'transcription_block_mentioned_persons'
AND c.conname = 'pk_tbmp'
AND c.contype = 'p'
""",
Integer.class);
assertThat(count).isEqualTo(1);
}
// ─── helpers ─────────────────────────────────────────────────────────────
private UUID createPerson(String firstName, String lastName) {
@@ -482,4 +544,10 @@ class MigrationIntegrationTest {
""", id, recipientId, docId, commentId);
return id;
}
private UUID createUserGroup(String name) {
UUID id = UUID.randomUUID();
jdbc.update("INSERT INTO user_groups (id, name) VALUES (?, ?)", id, name);
return id;
}
}

View File

@@ -44,6 +44,14 @@ class CommentControllerTest {
// ─── Block comment endpoints ─────────────────────────────────────────────
@Test
@WithMockUser
void getBlockComments_returns400_when_documentId_is_not_a_UUID() throws Exception {
UUID blockId = UUID.randomUUID();
mockMvc.perform(get("/api/documents/NOT-A-UUID/transcription-blocks/" + blockId + "/comments"))
.andExpect(status().isBadRequest());
}
@Test
@WithMockUser
void getBlockComments_returns200() throws Exception {
@@ -115,6 +123,15 @@ class CommentControllerTest {
// ─── Block reply endpoints ───────────────────────────────────────────────
@Test
@WithMockUser(authorities = "ANNOTATE_ALL")
void replyToBlockComment_returns400_when_blockId_is_not_a_UUID() throws Exception {
mockMvc.perform(post("/api/documents/" + DOC_ID + "/transcription-blocks/NOT-A-UUID"
+ "/comments/" + COMMENT_ID + "/replies")
.contentType(MediaType.APPLICATION_JSON).content(COMMENT_JSON))
.andExpect(status().isBadRequest());
}
@Test
void replyToBlockComment_returns401_whenUnauthenticated() throws Exception {
UUID blockId = UUID.randomUUID();

View File

@@ -35,4 +35,15 @@ class AppUserTest {
.count();
assertThat(distinct).isGreaterThan(1);
}
@Test
void computeColor_returnsValidPaletteColorForIntegerMinValueHash() {
// UUID "80000000-0000-0000-0000-000000000000" has hashCode() == Integer.MIN_VALUE.
// Math.abs(Integer.MIN_VALUE) overflows back to Integer.MIN_VALUE (negative), making
// Math.abs(hashCode()) % n unsafe for palette sizes that don't evenly divide MIN_VALUE.
// Math.floorMod eliminates this edge case entirely.
UUID minHashId = UUID.fromString("80000000-0000-0000-0000-000000000000");
assertThat(minHashId.hashCode()).isEqualTo(Integer.MIN_VALUE);
assertThat(EXPECTED_PALETTE).contains(AppUser.computeColor(minHashId));
}
}

View File

@@ -902,4 +902,18 @@ class UserServiceTest {
assertThat(result.getName()).isEqualTo("Familie");
assertThat(result.getPermissions()).containsExactlyInAnyOrder("READ_ALL", "WRITE_ALL");
}
@Test
void createGroup_withNullPermissions_savesGroupWithEmptyPermissionSet() {
org.raddatz.familienarchiv.user.GroupDTO dto = new org.raddatz.familienarchiv.user.GroupDTO();
dto.setName("Leser");
dto.setPermissions(null);
UserGroup saved = UserGroup.builder().id(UUID.randomUUID()).name("Leser").build();
when(groupRepository.save(any())).thenReturn(saved);
userService.createGroup(dto);
verify(groupRepository).save(argThat(g -> g.getPermissions() != null && g.getPermissions().isEmpty()));
}
}

View File

@@ -35,7 +35,7 @@ let {
onclick={onPrev}
disabled={currentPage <= 1}
aria-label="Zurück"
class="min-h-[44px] min-w-[44px] rounded p-2 text-ink-3 transition hover:bg-surface/10 focus-visible:ring-2 focus-visible:ring-brand-navy focus-visible:ring-offset-1 disabled:opacity-40"
class="rounded p-1 text-ink-3 transition hover:bg-surface/10 disabled:opacity-40"
>
<svg class="h-4 w-4" viewBox="0 0 24 24" fill="none" stroke="currentColor" stroke-width="2">
<path stroke-linecap="round" stroke-linejoin="round" d="M15 19l-7-7 7-7" />
@@ -52,7 +52,7 @@ let {
onclick={onNext}
disabled={!isLoaded || currentPage >= totalPages}
aria-label="Weiter"
class="min-h-[44px] min-w-[44px] rounded p-2 text-ink-3 transition hover:bg-surface/10 focus-visible:ring-2 focus-visible:ring-brand-navy focus-visible:ring-offset-1 disabled:opacity-40"
class="rounded p-1 text-ink-3 transition hover:bg-surface/10 disabled:opacity-40"
>
<svg class="h-4 w-4" viewBox="0 0 24 24" fill="none" stroke="currentColor" stroke-width="2">
<path stroke-linecap="round" stroke-linejoin="round" d="M9 5l7 7-7 7" />
@@ -65,7 +65,7 @@ let {
<button
onclick={onZoomOut}
aria-label="Verkleinern"
class="min-h-[44px] min-w-[44px] rounded p-2 text-ink-3 transition hover:bg-surface/10 focus-visible:ring-2 focus-visible:ring-brand-navy focus-visible:ring-offset-1"
class="rounded p-1 text-ink-3 transition hover:bg-surface/10"
>
<svg class="h-4 w-4" viewBox="0 0 24 24" fill="none" stroke="currentColor" stroke-width="2">
<circle cx="11" cy="11" r="8" />
@@ -75,7 +75,7 @@ let {
<button
onclick={onZoomIn}
aria-label="Vergrößern"
class="min-h-[44px] min-w-[44px] rounded p-2 text-ink-3 transition hover:bg-surface/10 focus-visible:ring-2 focus-visible:ring-brand-navy focus-visible:ring-offset-1"
class="rounded p-1 text-ink-3 transition hover:bg-surface/10"
>
<svg class="h-4 w-4" viewBox="0 0 24 24" fill="none" stroke="currentColor" stroke-width="2">
<circle cx="11" cy="11" r="8" />
@@ -89,8 +89,7 @@ let {
<button
onclick={onToggleAnnotations}
aria-label={showAnnotations ? m.pdf_annotations_hide() : m.pdf_annotations_show()}
aria-pressed={showAnnotations}
class="flex min-h-[44px] min-w-[44px] items-center gap-1.5 rounded px-3 py-2 font-sans text-xs transition focus-visible:ring-2 focus-visible:ring-brand-navy focus-visible:ring-offset-1 {showAnnotations
class="flex items-center gap-1.5 rounded px-2 py-1 font-sans text-xs transition {showAnnotations
? 'text-ink-2 hover:bg-surface/10'
: 'bg-surface/10 text-primary'}"
>

View File

@@ -65,111 +65,3 @@ describe('PdfControls — annotation toggle contrast (WCAG 2.1 AA)', () => {
expect(annotationBtn!.className).not.toContain('text-accent');
});
});
describe('PdfControls — focus rings (WCAG 2.1 §2.4.7)', () => {
it('annotation toggle button has focus-visible:ring-2 focus ring', async () => {
const { container } = render(PdfControls, {
...defaultProps,
annotationCount: 2,
showAnnotations: false
});
const allButtons = container.querySelectorAll('button');
const annotationBtn = Array.from(allButtons).find((b) =>
b.getAttribute('aria-label')?.toLowerCase().includes('annotierungen')
);
expect(annotationBtn).not.toBeNull();
expect(annotationBtn!.className).toContain('focus-visible:ring-2');
});
it('icon-only nav/zoom buttons each have focus-visible:ring-2 focus ring', async () => {
const { container } = render(PdfControls, { ...defaultProps });
const allButtons = container.querySelectorAll('button');
const iconOnlyButtons = Array.from(allButtons).filter((b) => {
const label = b.getAttribute('aria-label') ?? '';
return ['zurück', 'weiter', 'verkleinern', 'vergrößern'].includes(label.toLowerCase());
});
expect(iconOnlyButtons).toHaveLength(4);
for (const btn of iconOnlyButtons) {
expect(btn.className).toContain('focus-visible:ring-2');
}
});
});
describe('PdfControls — touch targets (WCAG 2.2 §2.5.8)', () => {
it('annotation toggle button has min-h-[44px] touch target', async () => {
const { container } = render(PdfControls, {
...defaultProps,
annotationCount: 2,
showAnnotations: false
});
const allButtons = container.querySelectorAll('button');
const annotationBtn = Array.from(allButtons).find((b) =>
b.getAttribute('aria-label')?.toLowerCase().includes('annotierungen')
);
expect(annotationBtn).not.toBeNull();
expect(annotationBtn!.className).toContain('min-h-[44px]');
});
it('annotation toggle button has min-w-[44px] touch target', async () => {
const { container } = render(PdfControls, {
...defaultProps,
annotationCount: 2,
showAnnotations: false
});
const allButtons = container.querySelectorAll('button');
const annotationBtn = Array.from(allButtons).find((b) =>
b.getAttribute('aria-label')?.toLowerCase().includes('annotierungen')
);
expect(annotationBtn).not.toBeNull();
expect(annotationBtn!.className).toContain('min-w-[44px]');
});
it('annotation toggle reflects pressed state via aria-pressed', async () => {
const { container: c1 } = render(PdfControls, {
...defaultProps,
annotationCount: 2,
showAnnotations: false
});
const btn1 = Array.from(c1.querySelectorAll('button')).find((b) =>
b.getAttribute('aria-label')?.toLowerCase().includes('annotierungen')
);
expect(btn1!.getAttribute('aria-pressed')).toBe('false');
cleanup();
const { container: c2 } = render(PdfControls, {
...defaultProps,
annotationCount: 2,
showAnnotations: true
});
const btn2 = Array.from(c2.querySelectorAll('button')).find((b) =>
b.getAttribute('aria-label')?.toLowerCase().includes('annotierungen')
);
expect(btn2!.getAttribute('aria-pressed')).toBe('true');
});
it('icon-only nav/zoom buttons each have min-h-[44px] touch target', async () => {
const { container } = render(PdfControls, { ...defaultProps });
const allButtons = container.querySelectorAll('button');
const iconOnlyButtons = Array.from(allButtons).filter((b) => {
const label = b.getAttribute('aria-label') ?? '';
return ['zurück', 'weiter', 'verkleinern', 'vergrößern'].includes(label.toLowerCase());
});
expect(iconOnlyButtons).toHaveLength(4);
for (const btn of iconOnlyButtons) {
expect(btn.className).toContain('min-h-[44px]');
}
});
it('icon-only nav/zoom buttons each have min-w-[44px] touch target', async () => {
const { container } = render(PdfControls, { ...defaultProps });
const allButtons = container.querySelectorAll('button');
const iconOnlyButtons = Array.from(allButtons).filter((b) => {
const label = b.getAttribute('aria-label') ?? '';
return ['zurück', 'weiter', 'verkleinern', 'vergrößern'].includes(label.toLowerCase());
});
expect(iconOnlyButtons).toHaveLength(4);
for (const btn of iconOnlyButtons) {
expect(btn.className).toContain('min-w-[44px]');
}
});
});