fix(bulk-edit): cycle-3 polish — Felix C2/C3/C4/C5 + Sara coverage gaps
Some checks failed
CI / Unit & Component Tests (pull_request) Failing after 2m54s
CI / OCR Service Tests (pull_request) Successful in 39s
CI / Backend Unit Tests (pull_request) Failing after 2m56s
CI / Unit & Component Tests (push) Failing after 3m6s
CI / Backend Unit Tests (push) Failing after 2m56s
CI / OCR Service Tests (push) Successful in 34s
Some checks failed
CI / Unit & Component Tests (pull_request) Failing after 2m54s
CI / OCR Service Tests (pull_request) Successful in 39s
CI / Backend Unit Tests (pull_request) Failing after 2m56s
CI / Unit & Component Tests (push) Failing after 3m6s
CI / Backend Unit Tests (push) Failing after 2m56s
CI / OCR Service Tests (push) Successful in 34s
Felix C2 — `BatchMetadataRequest` controller now uses `@Valid` so future @Size/etc. annotations on the record actually fire. Felix C3 — Auto-clear `$effect` in `+layout.svelte` reads `bulkSelectionStore.size` inside `untrack()` so the effect only re-fires on route change, not on every checkbox toggle. Felix C4 — `BulkDocumentEditLayout` edit-mode hydration loop now lives inside `onMount` (not at top-level script) so the SvelteMap mutation is unambiguously tied to instance lifecycle, matching the pattern used by `WhoWhenSection`/`DescriptionSection` after the cycle-2 fix. Felix C5 — Replaced fully-qualified `java.util.LinkedHashSet` in `DocumentController` with a top-of-file import. Sara coverage — six new spec files / blocks pin the cycle-1 and cycle-2 behaviours that were previously untested: - `WhoWhenSection.svelte.spec.ts` — onMount seeding from initialDateIso / initialLocation; doesn't stomp parent-bound dateIso; hideDate / editMode branch - `DescriptionSection.svelte.spec.ts` — onMount seeding from initialTitle / initialDocumentLocation; doesn't stomp parent-bound values; archive-box / archive-folder fields visible only in editMode - `BulkSelectionBar.svelte.spec.ts` — Esc-scope guard tests for `<dialog>` open and `aria-expanded` popover present - `BulkDocumentEditLayout.svelte.spec.ts` — topbar reads "Massenbearbeitung" + "werden bearbeitet" in edit mode (not the upload-flavoured "hochladen"/"werden erstellt" copy) - `DocumentControllerTest.patchBulk_returns400_whenArchiveBoxExceeds255Chars` — pins the @Size validator on archiveBox via the @Valid wiring Refs #225, PR #331 Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
This commit was merged in pull request #331.
This commit is contained in:
@@ -3,6 +3,7 @@ package org.raddatz.familienarchiv.controller;
|
||||
import java.io.IOException;
|
||||
import java.time.LocalDate;
|
||||
import java.util.ArrayList;
|
||||
import java.util.LinkedHashSet;
|
||||
import java.util.List;
|
||||
import java.util.Map;
|
||||
import java.util.Optional;
|
||||
@@ -272,7 +273,7 @@ public class DocumentController {
|
||||
// Dedupe duplicate document IDs while preserving submission order. A
|
||||
// double-click on "Alle X editieren" would otherwise hit each document
|
||||
// twice and inflate the `updated` count returned to the user.
|
||||
java.util.LinkedHashSet<UUID> uniqueIds = new java.util.LinkedHashSet<>(dto.getDocumentIds());
|
||||
LinkedHashSet<UUID> uniqueIds = new LinkedHashSet<>(dto.getDocumentIds());
|
||||
|
||||
for (UUID id : uniqueIds) {
|
||||
try {
|
||||
@@ -324,7 +325,7 @@ public class DocumentController {
|
||||
|
||||
@PostMapping(value = "/batch-metadata", consumes = MediaType.APPLICATION_JSON_VALUE)
|
||||
@RequirePermission(Permission.READ_ALL)
|
||||
public List<DocumentBatchSummary> batchMetadata(@RequestBody BatchMetadataRequest request, Authentication authentication) {
|
||||
public List<DocumentBatchSummary> batchMetadata(@RequestBody @Valid BatchMetadataRequest request, Authentication authentication) {
|
||||
if (request == null || request.ids() == null || request.ids().isEmpty()) {
|
||||
throw new ResponseStatusException(HttpStatus.BAD_REQUEST, "ids is required");
|
||||
}
|
||||
|
||||
@@ -996,6 +996,24 @@ class DocumentControllerTest {
|
||||
.andExpect(jsonPath("$.code").value("BULK_EDIT_TOO_MANY_IDS"));
|
||||
}
|
||||
|
||||
@Test
|
||||
@WithMockUser(authorities = "WRITE_ALL")
|
||||
void patchBulk_returns400_whenArchiveBoxExceeds255Chars() throws Exception {
|
||||
// Tobias C2 — DocumentBulkEditDTO.archiveBox carries @Size(max=255).
|
||||
// Without @Valid on @RequestBody this would silently land an
|
||||
// arbitrarily long string; the test pins both the annotation and
|
||||
// the controller-level @Valid wiring.
|
||||
when(userService.findByEmail(any())).thenReturn(AppUser.builder().id(UUID.randomUUID()).build());
|
||||
UUID id = UUID.randomUUID();
|
||||
String tooLong = "x".repeat(256);
|
||||
|
||||
String body = "{\"documentIds\":[\"" + id + "\"],\"archiveBox\":\"" + tooLong + "\"}";
|
||||
mockMvc.perform(patch("/api/documents/bulk")
|
||||
.contentType(MediaType.APPLICATION_JSON)
|
||||
.content(body))
|
||||
.andExpect(status().isBadRequest());
|
||||
}
|
||||
|
||||
@Test
|
||||
@WithMockUser(authorities = "WRITE_ALL")
|
||||
void patchBulk_acceptsExactly500Ids_atTheCap() throws Exception {
|
||||
|
||||
@@ -1,7 +1,7 @@
|
||||
<script lang="ts">
|
||||
import { SvelteMap } from 'svelte/reactivity';
|
||||
import { goto } from '$app/navigation';
|
||||
import { onDestroy, untrack } from 'svelte';
|
||||
import { onDestroy, onMount, untrack } from 'svelte';
|
||||
import { m } from '$lib/paraglide/messages.js';
|
||||
import { getConfirmService } from '$lib/services/confirm.svelte.js';
|
||||
import type { ConfirmService } from '$lib/services/confirm.svelte.js';
|
||||
@@ -73,8 +73,11 @@ let archiveFolder = $state('');
|
||||
|
||||
// Hydrate edit-mode entries on mount. The IDs in bulkSelectionStore drive the
|
||||
// fetch upstream in the route — by the time this layout mounts, the metadata
|
||||
// has already been resolved into `initialEditEntries`.
|
||||
if (mode === 'edit') {
|
||||
// has already been resolved into `initialEditEntries`. Wrapped in onMount so
|
||||
// the SvelteMap mutation is unambiguously tied to instance lifecycle, not to
|
||||
// the script body's first execution (Felix C4 cycle 3).
|
||||
onMount(() => {
|
||||
if (mode !== 'edit') return;
|
||||
for (const entry of untrack(() => initialEditEntries)) {
|
||||
files.set(entry.id, {
|
||||
id: entry.id,
|
||||
@@ -85,7 +88,7 @@ if (mode === 'edit') {
|
||||
});
|
||||
if (!activeId) activeId = entry.id;
|
||||
}
|
||||
}
|
||||
});
|
||||
|
||||
// --- Derived ---
|
||||
const isMulti = $derived(files.size >= 2);
|
||||
|
||||
@@ -401,6 +401,24 @@ describe('BulkDocumentEditLayout — mode="edit"', () => {
|
||||
expect(replaceBadges.length).toBeGreaterThanOrEqual(4);
|
||||
});
|
||||
|
||||
it('topbar reads "Massenbearbeitung" + "{count} werden bearbeitet" in edit mode', async () => {
|
||||
// Elicit C1 fix — upload-flavoured "Mehrere Dokumente hochladen" /
|
||||
// "werden erstellt" copy must not appear when mode === 'edit'.
|
||||
const { container } = render(BulkDocumentEditLayout, {
|
||||
mode: 'edit',
|
||||
initialEditEntries: [editEntry(1), editEntry(2)]
|
||||
});
|
||||
// Topbar title slot
|
||||
const topbar = container.querySelector('span.font-bold.text-ink');
|
||||
expect(topbar?.textContent).toContain('Massenbearbeitung');
|
||||
// Count pill
|
||||
const pill = container.querySelector('span.bg-accent');
|
||||
expect(pill?.textContent).toContain('werden bearbeitet');
|
||||
// Negative: must NOT show upload-flavoured copy
|
||||
expect(topbar?.textContent ?? '').not.toContain('hochladen');
|
||||
expect(pill?.textContent ?? '').not.toContain('werden erstellt');
|
||||
});
|
||||
|
||||
it('shows the archiveBox and archiveFolder bulk-only inputs', async () => {
|
||||
const { container } = render(BulkDocumentEditLayout, {
|
||||
mode: 'edit',
|
||||
|
||||
@@ -86,4 +86,37 @@ describe('BulkSelectionBar', () => {
|
||||
// Nothing to clear, no error.
|
||||
expect(bulkSelectionStore.size).toBe(0);
|
||||
});
|
||||
|
||||
it('Escape does not clear when an open <dialog> is present (Leonie B6 scope guard)', async () => {
|
||||
bulkSelectionStore.add('a');
|
||||
bulkSelectionStore.add('b');
|
||||
render(BulkSelectionBar, { canWrite: true });
|
||||
|
||||
// Simulate a ConfirmDialog being open in front of the bar.
|
||||
const overlay = document.createElement('dialog');
|
||||
overlay.setAttribute('open', '');
|
||||
document.body.appendChild(overlay);
|
||||
try {
|
||||
window.dispatchEvent(new KeyboardEvent('keydown', { key: 'Escape' }));
|
||||
// Escape is captured by the dialog, not the bar — selection survives.
|
||||
expect(bulkSelectionStore.size).toBe(2);
|
||||
} finally {
|
||||
overlay.remove();
|
||||
}
|
||||
});
|
||||
|
||||
it('Escape does not clear when an aria-expanded popover is present', async () => {
|
||||
bulkSelectionStore.add('a');
|
||||
render(BulkSelectionBar, { canWrite: true });
|
||||
|
||||
const trigger = document.createElement('button');
|
||||
trigger.setAttribute('aria-expanded', 'true');
|
||||
document.body.appendChild(trigger);
|
||||
try {
|
||||
window.dispatchEvent(new KeyboardEvent('keydown', { key: 'Escape' }));
|
||||
expect(bulkSelectionStore.size).toBe(1);
|
||||
} finally {
|
||||
trigger.remove();
|
||||
}
|
||||
});
|
||||
});
|
||||
|
||||
@@ -0,0 +1,50 @@
|
||||
import { afterEach, describe, expect, it } from 'vitest';
|
||||
import { cleanup, render } from 'vitest-browser-svelte';
|
||||
import DescriptionSection from './DescriptionSection.svelte';
|
||||
|
||||
afterEach(() => cleanup());
|
||||
|
||||
describe('DescriptionSection — onMount seeding (Felix B1/B2 fix regression fence)', () => {
|
||||
it('pre-fills the title input from initialTitle when currentTitle is empty', async () => {
|
||||
render(DescriptionSection, { initialTitle: 'Brief an Anna' });
|
||||
const titleInput = document.querySelector('input#title') as HTMLInputElement;
|
||||
expect(titleInput).not.toBeNull();
|
||||
expect(titleInput.value).toBe('Brief an Anna');
|
||||
});
|
||||
|
||||
it('does not stomp a parent-bound currentTitle that is already non-empty', async () => {
|
||||
render(DescriptionSection, {
|
||||
currentTitle: 'Parent Title',
|
||||
initialTitle: 'Should Not Win'
|
||||
});
|
||||
const titleInput = document.querySelector('input#title') as HTMLInputElement;
|
||||
expect(titleInput.value).toBe('Parent Title');
|
||||
});
|
||||
|
||||
it('pre-fills the documentLocation input from initialDocumentLocation', async () => {
|
||||
render(DescriptionSection, { initialDocumentLocation: 'Schrank 3, Mappe B' });
|
||||
const locationInput = document.querySelector('input#documentLocation') as HTMLInputElement;
|
||||
expect(locationInput.value).toBe('Schrank 3, Mappe B');
|
||||
});
|
||||
|
||||
it('does not stomp a parent-bound documentLocation that is already non-empty', async () => {
|
||||
render(DescriptionSection, {
|
||||
documentLocation: 'Bound Value',
|
||||
initialDocumentLocation: 'Should Not Win'
|
||||
});
|
||||
const locationInput = document.querySelector('input#documentLocation') as HTMLInputElement;
|
||||
expect(locationInput.value).toBe('Bound Value');
|
||||
});
|
||||
|
||||
it('renders the editMode-only archiveBox + archiveFolder fields when editMode=true', async () => {
|
||||
render(DescriptionSection, { editMode: true, hideTitle: true });
|
||||
expect(document.querySelector('[data-testid="description-archive-box"]')).not.toBeNull();
|
||||
expect(document.querySelector('[data-testid="description-archive-folder"]')).not.toBeNull();
|
||||
});
|
||||
|
||||
it('hides the editMode-only archive fields when editMode=false', async () => {
|
||||
render(DescriptionSection, { editMode: false });
|
||||
expect(document.querySelector('[data-testid="description-archive-box"]')).toBeNull();
|
||||
expect(document.querySelector('[data-testid="description-archive-folder"]')).toBeNull();
|
||||
});
|
||||
});
|
||||
@@ -0,0 +1,42 @@
|
||||
import { afterEach, describe, expect, it } from 'vitest';
|
||||
import { cleanup, render } from 'vitest-browser-svelte';
|
||||
import { page } from 'vitest/browser';
|
||||
import WhoWhenSection from './WhoWhenSection.svelte';
|
||||
|
||||
afterEach(() => cleanup());
|
||||
|
||||
describe('WhoWhenSection — onMount seeding (Felix B1 fix regression fence)', () => {
|
||||
it('pre-fills the date input from initialDateIso when the bindable is empty', async () => {
|
||||
render(WhoWhenSection, { initialDateIso: '2024-03-15' });
|
||||
// isoToGerman('2024-03-15') → '15.03.2024'
|
||||
const dateInput = document.querySelector('input#documentDate') as HTMLInputElement;
|
||||
expect(dateInput).not.toBeNull();
|
||||
expect(dateInput.value).toBe('15.03.2024');
|
||||
});
|
||||
|
||||
it('does not stomp a parent-bound dateIso that is already non-empty', async () => {
|
||||
// dateIso bindable is '2026-01-01' from the parent; initialDateIso is the
|
||||
// "fallback seed". onMount must not overwrite the already-bound value.
|
||||
render(WhoWhenSection, { dateIso: '2026-01-01', initialDateIso: '1900-01-01' });
|
||||
const dateInput = document.querySelector('input#documentDate') as HTMLInputElement;
|
||||
expect(dateInput.value).toBe('01.01.2026');
|
||||
});
|
||||
|
||||
it('hides the date field when hideDate=true (bulk-edit mode)', async () => {
|
||||
render(WhoWhenSection, { hideDate: true });
|
||||
await expect.element(page.getByTestId('who-when-date')).not.toBeInTheDocument();
|
||||
});
|
||||
|
||||
it('renders the meta_location input only outside editMode', async () => {
|
||||
const { rerender } = render(WhoWhenSection, { editMode: true });
|
||||
expect(document.querySelector('input#location')).toBeNull();
|
||||
await rerender({ editMode: false });
|
||||
expect(document.querySelector('input#location')).not.toBeNull();
|
||||
});
|
||||
|
||||
it('pre-fills the location input from initialLocation', async () => {
|
||||
render(WhoWhenSection, { editMode: false, initialLocation: 'Berlin' });
|
||||
const locationInput = document.querySelector('input#location') as HTMLInputElement;
|
||||
expect(locationInput.value).toBe('Berlin');
|
||||
});
|
||||
});
|
||||
@@ -1,7 +1,7 @@
|
||||
<script lang="ts">
|
||||
import './layout.css';
|
||||
import { page } from '$app/state';
|
||||
import { onMount } from 'svelte';
|
||||
import { onMount, untrack } from 'svelte';
|
||||
import * as m from '$lib/paraglide/messages.js';
|
||||
import LanguageSwitcher from '$lib/components/LanguageSwitcher.svelte';
|
||||
import ThemeToggle from '$lib/components/ThemeToggle.svelte';
|
||||
@@ -22,6 +22,9 @@ provideConfirmService();
|
||||
// surface the BulkSelectionBar. Without this the selection silently follows
|
||||
// the user to /persons / /admin etc. and reappears as a stale 3-doc count
|
||||
// when they wander back to /documents — Felix C4 on PR #331.
|
||||
//
|
||||
// `bulkSelectionStore.size` is read inside `untrack` so the effect only
|
||||
// re-fires on route change, not on every checkbox toggle (Felix C3 cycle 3).
|
||||
$effect(() => {
|
||||
const path = page.url.pathname;
|
||||
const inBulkContext =
|
||||
@@ -29,8 +32,10 @@ $effect(() => {
|
||||
path.startsWith('/documents/') ||
|
||||
path === '/enrich' ||
|
||||
path.startsWith('/enrich/');
|
||||
if (!inBulkContext && bulkSelectionStore.size > 0) {
|
||||
bulkSelectionStore.clear();
|
||||
if (!inBulkContext) {
|
||||
untrack(() => {
|
||||
if (bulkSelectionStore.size > 0) bulkSelectionStore.clear();
|
||||
});
|
||||
}
|
||||
});
|
||||
|
||||
|
||||
Reference in New Issue
Block a user