fix(bulk-edit): cycle-2 blockers — restore initial-* props, missing import, scope Esc, edit-mode topbar
Some checks failed
CI / Unit & Component Tests (pull_request) Failing after 2m50s
CI / OCR Service Tests (pull_request) Successful in 27s
CI / Backend Unit Tests (pull_request) Failing after 2m54s
CI / Unit & Component Tests (push) Failing after 2m49s
CI / OCR Service Tests (push) Successful in 30s
CI / Backend Unit Tests (push) Failing after 2m55s

Felix B1 (data-loss regression on /documents/[id]/edit) — DocumentEditLayout
still passes initialDateIso, initialLocation, initialDocumentLocation, but
my cycle-1 cleanup removed those props. Result: existing values rendered
empty and a save would have overwritten them with "". Restored the props
on WhoWhenSection and DescriptionSection; initialisation now lives in
onMount so it runs exactly once and never stomps a parent-driven update on
a later prop change.

Felix B2 — `DescriptionSection.svelte:36` still had the top-level
`currentTitle = untrack(() => initialTitle)` mutation that I cleaned up in
WhoWhenSection but missed here. Same onMount-once treatment.

Leonie B5 — `enrich/+page.svelte:105` referenced `<BulkSelectionBar>` but
the import was lost in a prettier pass; svelte-check errored out and the
bar never rendered, leaving an 8 rem dead zone from the pb-32 reservation.
One-line fix: add the import.

Leonie B6 — Esc handler in `BulkSelectionBar` was unscoped and stole
Escape from NotificationBell, ConfirmDialog, HelpPopover, etc. (e.g.
selecting docs → opening notification bell → Esc would close the bell
AND silently wipe the selection). Now bails when an open dialog,
expanded menu, or popover is detected.

Elicit C1 — `BulkDocumentEditLayout` topbar now branches on `mode`:
shows "Massenbearbeitung" + "{count} werden bearbeitet" in edit mode
instead of the upload-flavoured "Mehrere Dokumente hochladen" + "werden
erstellt" copy. New i18n keys `bulk_edit_topbar_title` and
`bulk_edit_count_pill` in DE/EN/ES.

Tests added:
 - DocumentControllerTest.patchBulk_stripsCarriageReturnsAndNewlinesFromErrorMessages
   (Sara C2 follow-up — pin sanitizeForLog as a regression test)
 - BulkSelectionBar.spec — count=1 → "1 Dokument", count=2 → "2 Dokumente"
   (Sara C6 follow-up — pin the new bulk_edit_n_selected_one/_other branch)

Refs #225, PR #331

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
This commit is contained in:
Marcel
2026-04-25 17:17:33 +02:00
parent 1803db86b5
commit 8ce96294b0
10 changed files with 104 additions and 17 deletions

View File

@@ -1175,6 +1175,32 @@ class DocumentControllerTest {
.andExpect(jsonPath("$[0].pdfUrl").value("/api/documents/" + id + "/file"));
}
@Test
@WithMockUser(authorities = "WRITE_ALL")
void patchBulk_stripsCarriageReturnsAndNewlinesFromErrorMessages() throws Exception {
// Nora C4 — DocumentController.sanitizeForLog defends against
// CWE-117 (log injection) by replacing CR/LF in any free-form string
// it interpolates. Same helper now sanitises BulkEditError.message
// before it round-trips to the frontend.
when(userService.findByEmail(any())).thenReturn(AppUser.builder().id(UUID.randomUUID()).build());
UUID badId = UUID.randomUUID();
when(documentService.applyBulkEditToDocument(eq(badId), any(), any()))
.thenThrow(org.raddatz.familienarchiv.exception.DomainException.notFound(
org.raddatz.familienarchiv.exception.ErrorCode.DOCUMENT_NOT_FOUND,
"evil\r\nFAKE LOG ENTRY: admin logged in"));
mockMvc.perform(patch("/api/documents/bulk")
.contentType(MediaType.APPLICATION_JSON)
.content(bulkBody(badId.toString())))
.andExpect(status().isOk())
.andExpect(jsonPath("$.errors[0].message",
org.hamcrest.Matchers.not(org.hamcrest.Matchers.containsString("\n"))))
.andExpect(jsonPath("$.errors[0].message",
org.hamcrest.Matchers.not(org.hamcrest.Matchers.containsString("\r"))))
.andExpect(jsonPath("$.errors[0].message",
org.hamcrest.Matchers.containsString("evil_")));
}
@Test
@WithMockUser(authorities = "WRITE_ALL")
void patchBulk_returnsPartialFailureShape_whenServiceThrowsForOneDocument() throws Exception {

View File

@@ -896,5 +896,7 @@
"bulk_edit_clear_selection": "Auswahl aufheben",
"bulk_edit_clear_hint_keyboard": "Esc: Auswahl aufheben",
"bulk_edit_loading": "Dokumente werden geladen…",
"bulk_edit_all_x_failed": "Filter konnte nicht abgerufen werden — bitte erneut versuchen."
"bulk_edit_all_x_failed": "Filter konnte nicht abgerufen werden — bitte erneut versuchen.",
"bulk_edit_topbar_title": "Massenbearbeitung",
"bulk_edit_count_pill": "{count} werden bearbeitet"
}

View File

@@ -896,5 +896,7 @@
"bulk_edit_clear_selection": "Clear selection",
"bulk_edit_clear_hint_keyboard": "Esc: clear selection",
"bulk_edit_loading": "Loading documents…",
"bulk_edit_all_x_failed": "Could not load filter results — please retry."
"bulk_edit_all_x_failed": "Could not load filter results — please retry.",
"bulk_edit_topbar_title": "Bulk edit",
"bulk_edit_count_pill": "{count} will be edited"
}

View File

@@ -896,5 +896,7 @@
"bulk_edit_clear_selection": "Limpiar selección",
"bulk_edit_clear_hint_keyboard": "Esc: limpiar selección",
"bulk_edit_loading": "Cargando documentos…",
"bulk_edit_all_x_failed": "No se pudieron cargar los resultados del filtro; vuelve a intentarlo."
"bulk_edit_all_x_failed": "No se pudieron cargar los resultados del filtro; vuelve a intentarlo.",
"bulk_edit_topbar_title": "Edición masiva",
"bulk_edit_count_pill": "Se editarán {count}"
}

View File

@@ -324,12 +324,20 @@ async function retrySave() {
</a>
<span class="text-ink-3" aria-hidden="true">·</span>
<span class="font-serif text-sm font-bold text-ink">
{isMulti ? m.bulk_title_multi() : m.bulk_title_single()}
{#if mode === 'edit'}
{m.bulk_edit_topbar_title()}
{:else}
{isMulti ? m.bulk_title_multi() : m.bulk_title_single()}
{/if}
</span>
{#if isMulti}
<span class="ml-auto flex items-center gap-3">
<span class="rounded-[2px] bg-accent px-2 py-0.5 text-xs font-bold text-primary">
{m.bulk_count_pill({ count: files.size })}
{#if mode === 'edit'}
{m.bulk_edit_count_pill({ count: files.size })}
{:else}
{m.bulk_count_pill({ count: files.size })}
{/if}
</span>
<button
type="button"

View File

@@ -18,11 +18,17 @@ function clearAll() {
// Escape clears the selection — keyboard escape hatch when the user has
// drilled into a 50-row selection and wants to bail without Tab-ing through
// the whole footer (WCAG 2.1.1).
// the whole footer (WCAG 2.1.1). Bails when an open dialog, expanded menu,
// or popover is in front so we don't steal Esc from NotificationBell,
// ConfirmDialog, HelpPopover, etc.
function onEscape(e: KeyboardEvent) {
if (e.key === 'Escape' && visible) {
clearAll();
}
if (e.key !== 'Escape' || !visible) return;
if (e.defaultPrevented) return;
const overlay = document.querySelector(
'dialog[open], [aria-expanded="true"], [role="menu"]:not([hidden]), [role="dialog"]:not([hidden])'
);
if (overlay) return;
clearAll();
}
</script>

View File

@@ -32,6 +32,23 @@ describe('BulkSelectionBar', () => {
await expect.element(page.getByTestId('bulk-selection-count')).toHaveTextContent('2');
});
it('uses the singular plural form for count=1 (not "1 Dokumente")', async () => {
bulkSelectionStore.add('only');
render(BulkSelectionBar, { canWrite: true });
await expect
.element(page.getByTestId('bulk-selection-count'))
.toHaveTextContent('1 Dokument ausgewählt');
});
it('uses the plural form for count=2', async () => {
bulkSelectionStore.add('a');
bulkSelectionStore.add('b');
render(BulkSelectionBar, { canWrite: true });
await expect
.element(page.getByTestId('bulk-selection-count'))
.toHaveTextContent('2 Dokumente ausgewählt');
});
it('clear button empties the store', async () => {
bulkSelectionStore.add('a');
bulkSelectionStore.add('b');

View File

@@ -1,5 +1,5 @@
<script lang="ts">
import { untrack } from 'svelte';
import { onMount } from 'svelte';
import TagInput, { type Tag } from '$lib/components/TagInput.svelte';
import FieldLabelBadge from './FieldLabelBadge.svelte';
import { m } from '$lib/paraglide/messages.js';
@@ -11,6 +11,7 @@ let {
archiveBox = $bindable(''),
archiveFolder = $bindable(''),
initialTitle = '',
initialDocumentLocation = '',
initialSummary = '',
titleRequired = false,
suggestedTitle = '',
@@ -23,6 +24,7 @@ let {
archiveBox?: string;
archiveFolder?: string;
initialTitle?: string;
initialDocumentLocation?: string;
initialSummary?: string;
titleRequired?: boolean;
suggestedTitle?: string;
@@ -30,10 +32,17 @@ let {
editMode?: boolean;
} = $props();
// currentTitle seeds from initialTitle once at mount; subsequent edits flow
// through the oninput handler that flips titleDirty.
// Seed bindables from initial-* props once at mount and only when the parent
// hasn't already supplied a non-empty value through the binding. onMount runs
// exactly once per instance, so this never stomps a parent-driven update on a
// later prop change. Required by the single-doc edit flow which seeds from
// the document; bulk-edit consumers leave the initial-* unset and bind their
// own state.
let titleDirty = $state(false);
currentTitle = untrack(() => initialTitle);
onMount(() => {
if (!currentTitle && initialTitle) currentTitle = initialTitle;
if (!documentLocation && initialDocumentLocation) documentLocation = initialDocumentLocation;
});
const titleValue = $derived(titleDirty ? currentTitle : suggestedTitle || currentTitle);
</script>

View File

@@ -1,5 +1,5 @@
<script lang="ts">
import { untrack } from 'svelte';
import { onMount, untrack } from 'svelte';
import PersonTypeahead from '$lib/components/PersonTypeahead.svelte';
import PersonMultiSelect from '$lib/components/PersonMultiSelect.svelte';
import FieldLabelBadge from './FieldLabelBadge.svelte';
@@ -13,6 +13,8 @@ let {
senderId = $bindable(''),
selectedReceivers = $bindable<Person[]>([]),
dateIso = $bindable(''),
initialDateIso = '',
initialLocation = '',
initialSenderName = '',
suggestedDateIso = '',
suggestedSenderName = '',
@@ -22,6 +24,8 @@ let {
senderId?: string;
selectedReceivers?: Person[];
dateIso?: string;
initialDateIso?: string;
initialLocation?: string;
initialSenderName?: string;
suggestedDateIso?: string;
suggestedSenderName?: string;
@@ -29,11 +33,20 @@ let {
editMode?: boolean;
} = $props();
// Seed dateDisplay from the bindable's current value once at mount; subsequent
// edits flow through handleDateInput which writes back to dateIso.
let dateDisplay = $state(untrack(() => isoToGerman(dateIso)));
// dateDisplay seeds from the bindable's value or initialDateIso once at mount
// and is then user-driven. onMount runs exactly once, so this never stomps
// the parent's dateIso on a later prop change.
let dateDisplay = $state('');
let dateDirty = $state(false);
onMount(() => {
const seed = dateIso || initialDateIso;
if (seed) {
dateDisplay = isoToGerman(seed);
if (!dateIso) dateIso = seed;
}
});
const dateInvalid = $derived(dateDirty && dateDisplay.length > 0 && dateIso === '');
function handleDateInput(e: Event) {
@@ -119,6 +132,7 @@ $effect(() => {
id="location"
type="text"
name="location"
value={initialLocation}
placeholder={m.form_placeholder_location()}
class="block w-full rounded border border-line px-2 py-3 text-sm shadow-sm focus:outline-none focus-visible:ring-2 focus-visible:ring-focus-ring"
/>

View File

@@ -1,6 +1,7 @@
<script lang="ts">
import { m } from '$lib/paraglide/messages.js';
import BackButton from '$lib/components/BackButton.svelte';
import BulkSelectionBar from '$lib/components/document/BulkSelectionBar.svelte';
import { bulkSelectionStore } from '$lib/stores/bulkSelection.svelte';
let { data } = $props();