feat(transcription): add "Alle als fertig markieren" bulk action to transcription panel #345

New Issue

marcel · 2026-04-26T19:56:41+02:00

marcel commented

2026-04-26 19:56:41 +02:00

User story

As a transcriber, I want a "Alle als fertig markieren" button in the transcription panel, so that I can approve all blocks after a review in one click instead of clicking each one individually.

Context

After reviewing a full document, users must click "Als fertig markieren" on every block one by one. For documents with many blocks this is tedious repetitive clicking — a bulk action eliminates the friction.

Acceptance criteria

Given the transcription panel is open and ≥ 2 blocks are not yet marked as ready, then an "Alle als fertig markieren" button is visible.
Given the user clicks the button, then all blocks in the current document are marked as ready in one action.
Given all blocks are already marked as ready, then the button is hidden or disabled.
Individual blocks can still be un-marked after the bulk action (no lock-in).

NFRs

For documents with many blocks (> 10), show a loading indicator during the operation.

## User story As a transcriber, I want a "Alle als fertig markieren" button in the transcription panel, so that I can approve all blocks after a review in one click instead of clicking each one individually. ## Context After reviewing a full document, users must click "Als fertig markieren" on every block one by one. For documents with many blocks this is tedious repetitive clicking — a bulk action eliminates the friction. ## Acceptance criteria - Given the transcription panel is open and ≥ 2 blocks are not yet marked as ready, then an "Alle als fertig markieren" button is visible. - Given the user clicks the button, then all blocks in the current document are marked as ready in one action. - Given all blocks are already marked as ready, then the button is hidden or disabled. - Individual blocks can still be un-marked after the bulk action (no lock-in). ## NFRs - For documents with many blocks (> 10), show a loading indicator during the operation.

marcel added this to the Demo Day — family get-together milestone 2026-04-26 19:56:41 +02:00

marcel added the P2-medium feature ui labels 2026-04-26 19:56:58 +02:00

marcel commented

2026-04-26 19:59:42 +02:00

🏗️ Markus Keller — Application Architect

Observations

The existing per-block review endpoint PUT /api/documents/{documentId}/transcription-blocks/{blockId}/review correctly toggles a single block. There is no bulk variant yet.
The reviewBlock service method is transactional and atomic per block — a bulk endpoint needs its own @Transactional boundary covering all N updates in a single transaction, or a deliberately separate approach with partial-success semantics. The issue is silent on which is required.
No new schema migration is needed — reviewed already exists on transcription_blocks. This is purely a service/controller addition.
The AC says "all blocks are marked as ready in one action." That implies a single atomic transaction, not N sequential HTTP calls from the frontend. A frontend loop calling the existing single-block endpoint N times is the wrong approach: it creates N round-trips, N separate transactions, and a visible state machine that can get stuck halfway through on a network hiccup.

Recommendations

Add a single new endpoint PUT /api/documents/{documentId}/transcription-blocks/review-all (no request body needed). The TranscriptionBlockController already has the /reorder precedent for document-level batch operations on the same base path.
Implement markAllBlocksReviewed(UUID documentId, UUID userId) in TranscriptionService with @Transactional — one transaction, one blockRepository.saveAll(blocks) after setting reviewed = true on each. This keeps the atomicity guarantee at the database layer where it belongs.
Do NOT add a markAllBlocksReviewed variant that calls reviewBlock() in a loop — that creates N separate transactions and breaks the audit log semantics (each existing BLOCK_REVIEWED audit event logs one block; a bulk action should emit a single ALL_BLOCKS_REVIEWED audit kind or emit nothing extra).
The audit concern is non-trivial: the existing reviewBlock emits AuditKind.BLOCK_REVIEWED per block. Calling it N times from a bulk action would flood the audit log. Either skip individual audit events in the bulk path and emit a new AuditKind.ALL_BLOCKS_REVIEWED, or accept no audit event for the bulk action (simpler, acceptable given this is a convenience shortcut, not a workflow gate).

Open Decisions

Audit semantics for the bulk action: emit one ALL_BLOCKS_REVIEWED audit event, emit N individual BLOCK_REVIEWED events, or emit nothing? The existing reviewBlock audit event is already per-block, so a bulk action creates 10× the noise if it reuses the same path.

## 🏗️ Markus Keller — Application Architect ### Observations - The existing per-block review endpoint `PUT /api/documents/{documentId}/transcription-blocks/{blockId}/review` correctly toggles a single block. There is no bulk variant yet. - The `reviewBlock` service method is transactional and atomic per block — a bulk endpoint needs its own `@Transactional` boundary covering all N updates in a single transaction, or a deliberately separate approach with partial-success semantics. The issue is silent on which is required. - No new schema migration is needed — `reviewed` already exists on `transcription_blocks`. This is purely a service/controller addition. - The AC says "all blocks are marked as ready in one action." That implies a single atomic transaction, not N sequential HTTP calls from the frontend. A frontend loop calling the existing single-block endpoint N times is the wrong approach: it creates N round-trips, N separate transactions, and a visible state machine that can get stuck halfway through on a network hiccup. ### Recommendations - Add a single new endpoint `PUT /api/documents/{documentId}/transcription-blocks/review-all` (no request body needed). The `TranscriptionBlockController` already has the `/reorder` precedent for document-level batch operations on the same base path. - Implement `markAllBlocksReviewed(UUID documentId, UUID userId)` in `TranscriptionService` with `@Transactional` — one transaction, one `blockRepository.saveAll(blocks)` after setting `reviewed = true` on each. This keeps the atomicity guarantee at the database layer where it belongs. - Do NOT add a `markAllBlocksReviewed` variant that calls `reviewBlock()` in a loop — that creates N separate transactions and breaks the audit log semantics (each existing `BLOCK_REVIEWED` audit event logs one block; a bulk action should emit a single `ALL_BLOCKS_REVIEWED` audit kind or emit nothing extra). - The audit concern is non-trivial: the existing `reviewBlock` emits `AuditKind.BLOCK_REVIEWED` per block. Calling it N times from a bulk action would flood the audit log. Either skip individual audit events in the bulk path and emit a new `AuditKind.ALL_BLOCKS_REVIEWED`, or accept no audit event for the bulk action (simpler, acceptable given this is a convenience shortcut, not a workflow gate). ### Open Decisions - **Audit semantics for the bulk action**: emit one `ALL_BLOCKS_REVIEWED` audit event, emit N individual `BLOCK_REVIEWED` events, or emit nothing? The existing `reviewBlock` audit event is already per-block, so a bulk action creates 10× the noise if it reuses the same path.

marcel commented

2026-04-26 19:59:59 +02:00

👨‍💻 Felix Brandt — Fullstack Developer

Observations

The reviewToggle function in +page.svelte (line 108–115) makes one PUT per block. The bulk action must NOT be implemented by looping over transcriptionBlocks and calling this function N times — that produces N round-trips, N optimistic UI updates in sequence, and a half-marked state visible to the user if any call fails.
TranscriptionEditView.svelte already has reviewedCount, totalCount, and reviewProgress as $derived values — the sticky progress header updates automatically when the underlying blocks array changes. The bulk action just needs to flip reviewed = true on all items in the shared transcriptionBlocks state after the single HTTP call returns.
The "≥ 2 blocks not yet marked" visibility condition from the AC must live in a $derived, not inline in the template:

const showMarkAllButton = $derived(
  blocks.filter(b => !b.reviewed).length >= 2
);

The loading indicator for > 10 blocks (NFR) is a $state flag, not a derived. Pattern:

let markingAllReviewed = $state(false);

async function handleMarkAllReviewed() {
  markingAllReviewed = true;
  try {
    const res = await fetch(`/api/documents/${documentId}/transcription-blocks/review-all`, {
      method: 'PUT'
    });
    if (!res.ok) return;
    const updated: TranscriptionBlockData[] = await res.json();
    for (const b of updated) {
      const existing = blocks.find(x => x.id === b.id);
      if (existing) existing.reviewed = b.reviewed;
    }
  } finally {
    markingAllReviewed = false;
  }
}

The button belongs in TranscriptionEditView.svelte, inside the existing sticky progress header <div>, adjacent to the reviewedCount / totalCount text. That's where the user's eye is already looking.
Where should onMarkAllReviewed live? It's a document-level side-effect (updates transcriptionBlocks in +page.svelte). Options: (a) pass it down as a prop from page → EditView, consistent with onReviewToggle; (b) call it directly in EditView using a fetch if blocks are already client-side state there. The existing pattern passes callbacks down — stay consistent.

Recommendations

Add onMarkAllReviewed: () => Promise<void> to TranscriptionEditView's Props type, implement the handler in +page.svelte, and wire it through — mirrors the existing onReviewToggle pattern exactly.
The backend endpoint should return the full updated block list (same shape as listBlocks), so the frontend can do a single optimistic update pass instead of making assumptions about which blocks changed.
Write the failing Vitest test first: mock fetch, assert it is called once with PUT /api/documents/.../review-all, then assert all blocks in the reactive state have reviewed = true after resolution.

## 👨‍💻 Felix Brandt — Fullstack Developer ### Observations - The `reviewToggle` function in `+page.svelte` (line 108–115) makes one `PUT` per block. The bulk action must NOT be implemented by looping over `transcriptionBlocks` and calling this function N times — that produces N round-trips, N optimistic UI updates in sequence, and a half-marked state visible to the user if any call fails. - `TranscriptionEditView.svelte` already has `reviewedCount`, `totalCount`, and `reviewProgress` as `$derived` values — the sticky progress header updates automatically when the underlying `blocks` array changes. The bulk action just needs to flip `reviewed = true` on all items in the shared `transcriptionBlocks` state after the single HTTP call returns. - The "≥ 2 blocks not yet marked" visibility condition from the AC must live in a `$derived`, not inline in the template: ```svelte const showMarkAllButton = $derived( blocks.filter(b => !b.reviewed).length >= 2 ); ``` - The loading indicator for `> 10` blocks (NFR) is a `$state` flag, not a derived. Pattern: ```svelte let markingAllReviewed = $state(false); async function handleMarkAllReviewed() { markingAllReviewed = true; try { const res = await fetch(`/api/documents/${documentId}/transcription-blocks/review-all`, { method: 'PUT' }); if (!res.ok) return; const updated: TranscriptionBlockData[] = await res.json(); for (const b of updated) { const existing = blocks.find(x => x.id === b.id); if (existing) existing.reviewed = b.reviewed; } } finally { markingAllReviewed = false; } } ``` - The button belongs in `TranscriptionEditView.svelte`, inside the existing sticky progress header `<div>`, adjacent to the `reviewedCount / totalCount` text. That's where the user's eye is already looking. - Where should `onMarkAllReviewed` live? It's a document-level side-effect (updates `transcriptionBlocks` in `+page.svelte`). Options: (a) pass it down as a prop from page → EditView, consistent with `onReviewToggle`; (b) call it directly in `EditView` using a fetch if blocks are already client-side state there. The existing pattern passes callbacks down — stay consistent. ### Recommendations - Add `onMarkAllReviewed: () => Promise<void>` to `TranscriptionEditView`'s `Props` type, implement the handler in `+page.svelte`, and wire it through — mirrors the existing `onReviewToggle` pattern exactly. - The backend endpoint should return the full updated block list (same shape as `listBlocks`), so the frontend can do a single optimistic update pass instead of making assumptions about which blocks changed. - Write the failing Vitest test first: mock `fetch`, assert it is called once with `PUT /api/documents/.../review-all`, then assert all blocks in the reactive state have `reviewed = true` after resolution.

marcel commented

2026-04-26 20:00:10 +02:00

🔒 Nora "NullX" Steiner — Security Engineer

Observations

The existing PUT /{blockId}/review is correctly guarded with @RequirePermission(Permission.WRITE_ALL). The new bulk endpoint must carry the same annotation — do not forget it because there is no request body to validate.
The reviewBlock service method accepts documentId + blockId and calls getBlock(documentId, blockId) which uses findByIdAndDocumentId — this is the correct IDOR guard. The bulk variant markAllBlocksReviewed(documentId, userId) must operate only on blocks belonging to documentId. Using blockRepository.findByDocumentIdOrderBySortOrderAsc(documentId) (the existing query) scopes the fetch to the authenticated document — this is correct. Do not use blockRepository.findAll() + filter in Java.
No new input validation surface is introduced (no request body), which is good. The documentId path variable is already a UUID, which Spring auto-validates at binding time.
The authentication requirement (Authentication authentication → requireUserId(authentication)) must still be present so the updatedBy field and the audit log receive the correct user ID — same as createBlock and reviewBlock.

Recommendations

Declare the new endpoint exactly as:

@PutMapping("/review-all")
@RequirePermission(Permission.WRITE_ALL)
public List<TranscriptionBlock> markAllBlocksReviewed(
    @PathVariable UUID documentId,
    Authentication authentication) {
    UUID userId = requireUserId(authentication);
    return transcriptionService.markAllBlocksReviewed(documentId, userId);
}

No request body, no additional parameters. The permission annotation is not optional.

Add a @WebMvcTest security test verifying that PUT /review-all returns 401 when unauthenticated and 403 when authenticated with READ_ALL only — following the same pattern as the existing TranscriptionBlockControllerTest for /{blockId}/review. This feature has no meaningful attack surface beyond the existing permission model, but the tests prove the guard is not accidentally missing.

## 🔒 Nora "NullX" Steiner — Security Engineer ### Observations - The existing `PUT /{blockId}/review` is correctly guarded with `@RequirePermission(Permission.WRITE_ALL)`. The new bulk endpoint must carry the same annotation — do not forget it because there is no request body to validate. - The `reviewBlock` service method accepts `documentId` + `blockId` and calls `getBlock(documentId, blockId)` which uses `findByIdAndDocumentId` — this is the correct IDOR guard. The bulk variant `markAllBlocksReviewed(documentId, userId)` must operate only on blocks belonging to `documentId`. Using `blockRepository.findByDocumentIdOrderBySortOrderAsc(documentId)` (the existing query) scopes the fetch to the authenticated document — this is correct. Do not use `blockRepository.findAll()` + filter in Java. - No new input validation surface is introduced (no request body), which is good. The `documentId` path variable is already a UUID, which Spring auto-validates at binding time. - The authentication requirement (`Authentication authentication` → `requireUserId(authentication)`) must still be present so the `updatedBy` field and the audit log receive the correct user ID — same as `createBlock` and `reviewBlock`. ### Recommendations - Declare the new endpoint exactly as: ```java @PutMapping("/review-all") @RequirePermission(Permission.WRITE_ALL) public List<TranscriptionBlock> markAllBlocksReviewed( @PathVariable UUID documentId, Authentication authentication) { UUID userId = requireUserId(authentication); return transcriptionService.markAllBlocksReviewed(documentId, userId); } ``` No request body, no additional parameters. The permission annotation is not optional. - Add a `@WebMvcTest` security test verifying that `PUT /review-all` returns 401 when unauthenticated and 403 when authenticated with `READ_ALL` only — following the same pattern as the existing `TranscriptionBlockControllerTest` for `/{blockId}/review`. This feature has no meaningful attack surface beyond the existing permission model, but the tests prove the guard is not accidentally missing.

marcel commented

2026-04-26 20:00:24 +02:00

🧪 Sara Holt — QA Engineer

Observations

The AC has four testable behaviors: button visible when ≥ 2 unreviewed blocks; all blocks marked on click; button hidden/disabled when all already reviewed; individual un-marking still works after bulk action. Each is an independent test case. Do not bundle them into one test.
The NFR ("loading indicator for > 10 blocks") is untestable in the current form — there is no measurable completion threshold. A deterministic test can only verify: (a) the indicator appears immediately when the call starts, and (b) disappears when the call settles. The > 10 blocks trigger is client-side branching in the UI; it needs a Vitest component test with a controlled block list size.
The existing reviewToggle has zero unit test coverage in the Vitest suite (only E2E). The bulk action should not repeat this gap.

Recommendations

Unit (Vitest, server project): Test the markAllBlocksReviewed service method in isolation with MockitoExtension. Two cases: (1) blocks that are already reviewed stay reviewed (idempotency); (2) unreviewed blocks become reviewed. Verify blockRepository.saveAll() is called once with the correct collection.
Controller slice (@WebMvcTest): Four tests — 200 happy path with WRITE_ALL, 401 unauthenticated, 403 with READ_ALL only, 200 with empty block list (zero blocks → should return empty list without error).
Component (Vitest Browser): Render TranscriptionEditView with a controlled block list. Assert button is visible when 2+ unreviewed blocks exist, hidden when all are reviewed. Mock the onMarkAllReviewed callback and assert it is called exactly once on button click.
E2E (Playwright): One scenario covering the full journey — open document with multiple transcribed blocks → click "Alle als fertig markieren" → verify all blocks show the reviewed state visually → click individual block to un-mark → verify only that block reverts. Do not create one E2E test per AC item; that belongs at the unit layer.
The AC says "button is hidden or disabled" — pick one and test it. Hidden and disabled have different accessibility semantics. Decide in implementation; the test must reflect the actual choice.

## 🧪 Sara Holt — QA Engineer ### Observations - The AC has four testable behaviors: button visible when ≥ 2 unreviewed blocks; all blocks marked on click; button hidden/disabled when all already reviewed; individual un-marking still works after bulk action. Each is an independent test case. Do not bundle them into one test. - The NFR ("loading indicator for > 10 blocks") is untestable in the current form — there is no measurable completion threshold. A deterministic test can only verify: (a) the indicator appears immediately when the call starts, and (b) disappears when the call settles. The `> 10 blocks` trigger is client-side branching in the UI; it needs a Vitest component test with a controlled block list size. - The existing `reviewToggle` has zero unit test coverage in the Vitest suite (only E2E). The bulk action should not repeat this gap. ### Recommendations - **Unit (Vitest, server project):** Test the `markAllBlocksReviewed` service method in isolation with `MockitoExtension`. Two cases: (1) blocks that are already reviewed stay reviewed (idempotency); (2) unreviewed blocks become reviewed. Verify `blockRepository.saveAll()` is called once with the correct collection. - **Controller slice (`@WebMvcTest`):** Four tests — 200 happy path with `WRITE_ALL`, 401 unauthenticated, 403 with `READ_ALL` only, 200 with empty block list (zero blocks → should return empty list without error). - **Component (Vitest Browser):** Render `TranscriptionEditView` with a controlled block list. Assert button is visible when 2+ unreviewed blocks exist, hidden when all are reviewed. Mock the `onMarkAllReviewed` callback and assert it is called exactly once on button click. - **E2E (Playwright):** One scenario covering the full journey — open document with multiple transcribed blocks → click "Alle als fertig markieren" → verify all blocks show the reviewed state visually → click individual block to un-mark → verify only that block reverts. Do not create one E2E test per AC item; that belongs at the unit layer. - The AC says "button is hidden or disabled" — pick one and test it. Hidden and disabled have different accessibility semantics. Decide in implementation; the test must reflect the actual choice.

marcel commented

2026-04-26 20:00:40 +02:00

🎨 Leonie Voss — UI/UX Designer

Observations

The TranscriptionEditView.svelte sticky progress header already shows reviewedCount / totalCount geprüft and a mint progress bar. This is the natural home for the bulk action button — it is visually adjacent to the context that justifies the action (the review count), and sticky positioning means it remains accessible while scrolling through long block lists.
The primary audience for this action is 60+ transcribers on laptops/tablets. The touch target must be ≥ 44px height. A compact text link styled as a secondary action (below the progress bar) works better than a large primary button — this is a power-user shortcut, not the primary call-to-action.
"Hidden or disabled" (AC item 3) has a meaningful UX difference. Disabled is preferable: it communicates to the user that the button exists and why they cannot use it, rather than making it disappear silently. Screen readers can also announce a disabled button with a descriptive label. Use disabled + title="Alle Blöcke sind bereits als fertig markiert".
The loading state NFR mentions > 10 blocks as the trigger. Visually, showing a spinner inside or beside the button is the right affordance — replacing button text with a spinner is cleaner than an external indicator.

Recommendations

Place the button in the sticky progress header <div>, right-aligned, on the same line as the progress text:

<div class="sticky top-0 z-10 border-b border-line bg-surface px-4 pt-3 pb-2">
  <div class="flex items-center justify-between">
    <p class="font-sans text-xs text-ink-2">
      <span class="font-semibold text-ink">{reviewedCount} / {totalCount}</span> geprüft
    </p>
    {#if showMarkAllButton}
      <button
        onclick={handleMarkAllReviewed}
        disabled={markingAllReviewed}
        class="flex items-center gap-1.5 min-h-[44px] px-3 font-sans text-xs font-medium
               text-brand-navy/60 hover:text-brand-navy disabled:opacity-40
               transition-colors focus-visible:ring-2 focus-visible:ring-brand-navy rounded-sm"
      >
        {#if markingAllReviewed}
          <!-- spinner icon -->
        {:else}
          <!-- checkmark icon -->
        {/if}
        Alle als fertig markieren
      </button>
    {/if}
  </div>
  <!-- progress bar -->
</div>

Use focus-visible:ring-2 focus-visible:ring-brand-navy for the focus indicator — consistent with the project's keyboard navigation pattern.
Do not use brand-mint text on a bg-surface background for the button label — that combination fails WCAG AA at normal text size. Use text-brand-navy/60 (at 60% opacity it still clears 4.5:1 against bg-surface).

Open Decisions

Hidden vs. disabled when all blocks are reviewed: The issue leaves this open ("hidden or disabled"). Disabled with a tooltip is the better accessible choice, but it requires a title/aria-description. If you go with hidden, the progress header loses its action entirely — users may not realize the feature exists until they un-mark a block.

## 🎨 Leonie Voss — UI/UX Designer ### Observations - The `TranscriptionEditView.svelte` sticky progress header already shows `reviewedCount / totalCount geprüft` and a mint progress bar. This is the natural home for the bulk action button — it is visually adjacent to the context that justifies the action (the review count), and sticky positioning means it remains accessible while scrolling through long block lists. - The primary audience for this action is 60+ transcribers on laptops/tablets. The touch target must be ≥ 44px height. A compact text link styled as a secondary action (below the progress bar) works better than a large primary button — this is a power-user shortcut, not the primary call-to-action. - "Hidden or disabled" (AC item 3) has a meaningful UX difference. **Disabled** is preferable: it communicates to the user that the button exists and why they cannot use it, rather than making it disappear silently. Screen readers can also announce a disabled button with a descriptive label. Use `disabled` + `title="Alle Blöcke sind bereits als fertig markiert"`. - The loading state NFR mentions `> 10 blocks` as the trigger. Visually, showing a spinner inside or beside the button is the right affordance — replacing button text with a spinner is cleaner than an external indicator. ### Recommendations - Place the button in the sticky progress header `<div>`, right-aligned, on the same line as the progress text: ```svelte <div class="sticky top-0 z-10 border-b border-line bg-surface px-4 pt-3 pb-2"> <div class="flex items-center justify-between"> <p class="font-sans text-xs text-ink-2"> <span class="font-semibold text-ink">{reviewedCount} / {totalCount}</span> geprüft </p> {#if showMarkAllButton} <button onclick={handleMarkAllReviewed} disabled={markingAllReviewed} class="flex items-center gap-1.5 min-h-[44px] px-3 font-sans text-xs font-medium text-brand-navy/60 hover:text-brand-navy disabled:opacity-40 transition-colors focus-visible:ring-2 focus-visible:ring-brand-navy rounded-sm" > {#if markingAllReviewed}  {:else}  {/if} Alle als fertig markieren </button> {/if} </div>  </div> ``` - Use `focus-visible:ring-2 focus-visible:ring-brand-navy` for the focus indicator — consistent with the project's keyboard navigation pattern. - Do not use `brand-mint` text on a `bg-surface` background for the button label — that combination fails WCAG AA at normal text size. Use `text-brand-navy/60` (at 60% opacity it still clears 4.5:1 against `bg-surface`). ### Open Decisions - **Hidden vs. disabled when all blocks are reviewed**: The issue leaves this open ("hidden or disabled"). Disabled with a tooltip is the better accessible choice, but it requires a title/aria-description. If you go with hidden, the progress header loses its action entirely — users may not realize the feature exists until they un-mark a block.

marcel commented

2026-04-26 20:00:57 +02:00

📋 Elicit — Requirements Engineer

Observations

The user story is well-formed and the AC is mostly testable, but three precision gaps will cause implementation ambiguity:

Gap 1 — "Alle Blöcke" scope is undefined. The AC says "all blocks in the current document are marked as ready in one action." Does "current document" mean: (a) all blocks regardless of page, or (b) only blocks visible in the current viewport/panel state? For a multi-page document with 30 blocks across 4 pages, is the expectation that all 30 are marked at once? This needs to be explicit because the backend listBlocks(documentId) returns all blocks for the document, but the user may expect only visible blocks to be affected if the UI shows pagination.

Gap 2 — "Hidden or disabled" is a decision, not a requirement. The AC presents two options ("hidden or disabled") as equivalent. They are not: hidden means the feature disappears from the UI, disabled means it is visible but inactive. These produce different user experiences and different accessibility semantics. The requirement should specify one.

Gap 3 — Loading indicator trigger ("for documents with > 10 blocks") creates a conditional UX. Users with 8 blocks get no loading feedback; users with 12 get a spinner. The threshold is arbitrary and invisible to users. A simpler requirement: always show a loading indicator during the operation. The NFR should be "show a loading indicator during the operation; the interaction must complete within 3 seconds for documents with ≤ 50 blocks."

The issue has no failure path AC: what happens if the backend call fails midway through? Does the UI show an error? Do partially-updated blocks stay updated? The reviewBlock single-block path silently returns without updating the UI on failure (if (!res.ok) return). A bulk action failing silently is a worse user experience.

Recommendations

Add this AC item: Given the user clicks the button and the operation fails, then an error message is shown and the block states are unchanged. (This forces the implementation to handle failure atomically — if backend is a single transaction, all-or-nothing is guaranteed.)
Replace "hidden or disabled" with a single definitive choice. Recommend: disabled with aria-label="Alle Blöcke sind bereits als fertig markiert" when all blocks are already reviewed.
Replace the conditional NFR with: The loading indicator is always shown during the operation. The operation must complete within 3 seconds for documents with ≤ 50 blocks on a local network.
Clarify scope: "all blocks" means all transcription blocks for the document, regardless of page number or viewport position.

Open Decisions

Failure atomicity: Should a partial failure (network drops mid-bulk) revert all changes or leave successfully-updated blocks as reviewed? This requires a decision on whether the backend operation is a single transaction (all-or-nothing) or sequential updates (best-effort). The AC currently implies all-or-nothing, but does not state it.

## 📋 Elicit — Requirements Engineer ### Observations - The user story is well-formed and the AC is mostly testable, but three precision gaps will cause implementation ambiguity: **Gap 1 — "Alle Blöcke" scope is undefined.** The AC says "all blocks in the current document are marked as ready in one action." Does "current document" mean: (a) all blocks regardless of page, or (b) only blocks visible in the current viewport/panel state? For a multi-page document with 30 blocks across 4 pages, is the expectation that all 30 are marked at once? This needs to be explicit because the backend `listBlocks(documentId)` returns all blocks for the document, but the user may expect only visible blocks to be affected if the UI shows pagination. **Gap 2 — "Hidden or disabled" is a decision, not a requirement.** The AC presents two options ("hidden or disabled") as equivalent. They are not: hidden means the feature disappears from the UI, disabled means it is visible but inactive. These produce different user experiences and different accessibility semantics. The requirement should specify one. **Gap 3 — Loading indicator trigger ("for documents with > 10 blocks") creates a conditional UX.** Users with 8 blocks get no loading feedback; users with 12 get a spinner. The threshold is arbitrary and invisible to users. A simpler requirement: always show a loading indicator during the operation. The NFR should be "show a loading indicator during the operation; the interaction must complete within 3 seconds for documents with ≤ 50 blocks." - The issue has no failure path AC: what happens if the backend call fails midway through? Does the UI show an error? Do partially-updated blocks stay updated? The `reviewBlock` single-block path silently returns without updating the UI on failure (`if (!res.ok) return`). A bulk action failing silently is a worse user experience. ### Recommendations - Add this AC item: **Given the user clicks the button and the operation fails, then an error message is shown and the block states are unchanged.** (This forces the implementation to handle failure atomically — if backend is a single transaction, all-or-nothing is guaranteed.) - Replace "hidden or disabled" with a single definitive choice. Recommend: **disabled with `aria-label="Alle Blöcke sind bereits als fertig markiert"` when all blocks are already reviewed.** - Replace the conditional NFR with: **The loading indicator is always shown during the operation. The operation must complete within 3 seconds for documents with ≤ 50 blocks on a local network.** - Clarify scope: **"all blocks" means all transcription blocks for the document, regardless of page number or viewport position.** ### Open Decisions - **Failure atomicity**: Should a partial failure (network drops mid-bulk) revert all changes or leave successfully-updated blocks as reviewed? This requires a decision on whether the backend operation is a single transaction (all-or-nothing) or sequential updates (best-effort). The AC currently implies all-or-nothing, but does not state it.

marcel commented

2026-04-26 20:01:05 +02:00

🚀 Tobias Wendt — DevOps & Platform

Observations

This feature adds one new endpoint and one new service method. No new infrastructure, no schema migration, no Docker Compose changes needed. The operational footprint is zero.
The only platform consideration is response time. findByDocumentIdOrderBySortOrderAsc(documentId) runs against an indexed column (document_id). For the expected document scale (≤ 50 blocks), the query + saveAll will complete well under 100ms. No caching, no async, no extra infrastructure warranted.
The NFR mentions a "loading indicator during the operation" for > 10 blocks. At the current data scale, the operation is fast enough that a loading indicator is a UX affordance, not a performance mitigation. This does not require any backend changes.

Recommendations

No infrastructure changes needed. Ship it.
If the markAllBlocksReviewed endpoint becomes a hotspot in future (unlikely at family archive scale), the existing Prometheus/Actuator metrics will surface it before any manual intervention is needed. No pre-emptive optimization.
The new PUT /review-all endpoint should appear in the existing backend/api_tests/ HTTP test file for transcription-blocks so it can be exercised manually during development without spinning up the frontend.

## 🚀 Tobias Wendt — DevOps & Platform ### Observations - This feature adds one new endpoint and one new service method. No new infrastructure, no schema migration, no Docker Compose changes needed. The operational footprint is zero. - The only platform consideration is response time. `findByDocumentIdOrderBySortOrderAsc(documentId)` runs against an indexed column (`document_id`). For the expected document scale (≤ 50 blocks), the query + `saveAll` will complete well under 100ms. No caching, no async, no extra infrastructure warranted. - The NFR mentions a "loading indicator during the operation" for `> 10` blocks. At the current data scale, the operation is fast enough that a loading indicator is a UX affordance, not a performance mitigation. This does not require any backend changes. ### Recommendations - No infrastructure changes needed. Ship it. - If the `markAllBlocksReviewed` endpoint becomes a hotspot in future (unlikely at family archive scale), the existing Prometheus/Actuator metrics will surface it before any manual intervention is needed. No pre-emptive optimization. - The new `PUT /review-all` endpoint should appear in the existing `backend/api_tests/` HTTP test file for `transcription-blocks` so it can be exercised manually during development without spinning up the frontend.

marcel commented

2026-04-26 20:01:24 +02:00

🗳️ Decision Queue

Consolidated open decisions from the persona reviews. Each needs a human call before implementation starts.

Theme 1: Button state when all blocks are reviewed

Raised by: Leonie (UX), Sara (QA), Elicit (Requirements)

The AC says "hidden or disabled" — but these are meaningfully different:

Hidden: the button disappears entirely. Users who un-mark one block and want to re-bulk-mark may not find the button again. Simpler to implement and test.
Disabled (recommended): the button stays visible, greyed out, with a descriptive title/aria-label explaining why it is inactive. Screen readers can still announce it. The feature remains discoverable.

Decision needed: Choose one. Suggested: disabled with title="Alle Blöcke sind bereits als fertig markiert".

Theme 2: Audit log semantics for the bulk action

Raised by: Markus (Architect)

The existing reviewBlock service method emits AuditKind.BLOCK_REVIEWED once per block. If the bulk action calls reviewBlock N times internally, it emits N audit events — noisy. If it uses a separate code path (recommended), the options are:

Option A: Emit a new AuditKind.ALL_BLOCKS_REVIEWED (one event, carries document ID)
Option B: Emit nothing — bulk marking is a convenience shortcut, not an audited workflow gate
Option C: Emit N individual BLOCK_REVIEWED events (noisy, not recommended)

Decision needed: Option A or B. Option B is simpler; Option A gives a cleaner audit trail if the audit log is ever surfaced to users.

Theme 3: Failure atomicity

Raised by: Elicit (Requirements)

If the backend uses a single @Transactional method for the bulk operation, failure is automatically all-or-nothing. If the frontend implements bulk marking as N sequential calls (explicitly forbidden by Markus and Felix, but worth confirming), partial failure is possible.

Decision needed: Confirm the implementation is a single backend transaction (all-or-nothing). This should be the default given the existing @Transactional service pattern — just needs to be explicit so the AC can be updated to state it.

Theme 4: Loading indicator trigger threshold

Raised by: Elicit (Requirements)

The NFR currently reads "for documents with > 10 blocks, show a loading indicator." This creates a conditional UX branch that is invisible to users (why does the spinner appear for some documents but not others?).

Decision needed: Always show the loading indicator during the operation (remove the > 10 threshold), or keep the conditional. Always-show is simpler to implement and test.

## 🗳️ Decision Queue Consolidated open decisions from the persona reviews. Each needs a human call before implementation starts. --- ### Theme 1: Button state when all blocks are reviewed **Raised by**: Leonie (UX), Sara (QA), Elicit (Requirements) The AC says "hidden or disabled" — but these are meaningfully different: - **Hidden**: the button disappears entirely. Users who un-mark one block and want to re-bulk-mark may not find the button again. Simpler to implement and test. - **Disabled** (recommended): the button stays visible, greyed out, with a descriptive `title`/`aria-label` explaining why it is inactive. Screen readers can still announce it. The feature remains discoverable. **Decision needed**: Choose one. Suggested: **disabled** with `title="Alle Blöcke sind bereits als fertig markiert"`. --- ### Theme 2: Audit log semantics for the bulk action **Raised by**: Markus (Architect) The existing `reviewBlock` service method emits `AuditKind.BLOCK_REVIEWED` once per block. If the bulk action calls `reviewBlock` N times internally, it emits N audit events — noisy. If it uses a separate code path (recommended), the options are: - **Option A**: Emit a new `AuditKind.ALL_BLOCKS_REVIEWED` (one event, carries document ID) - **Option B**: Emit nothing — bulk marking is a convenience shortcut, not an audited workflow gate - **Option C**: Emit N individual `BLOCK_REVIEWED` events (noisy, not recommended) **Decision needed**: Option A or B. Option B is simpler; Option A gives a cleaner audit trail if the audit log is ever surfaced to users. --- ### Theme 3: Failure atomicity **Raised by**: Elicit (Requirements) If the backend uses a single `@Transactional` method for the bulk operation, failure is automatically all-or-nothing. If the frontend implements bulk marking as N sequential calls (explicitly forbidden by Markus and Felix, but worth confirming), partial failure is possible. **Decision needed**: Confirm the implementation is a single backend transaction (all-or-nothing). This should be the default given the existing `@Transactional` service pattern — just needs to be explicit so the AC can be updated to state it. --- ### Theme 4: Loading indicator trigger threshold **Raised by**: Elicit (Requirements) The NFR currently reads "for documents with > 10 blocks, show a loading indicator." This creates a conditional UX branch that is invisible to users (why does the spinner appear for some documents but not others?). **Decision needed**: Always show the loading indicator during the operation (remove the > 10 threshold), or keep the conditional. Always-show is simpler to implement and test.

marcel commented

2026-04-26 20:27:32 +02:00

Disabled
C, log it as n saves. We roll them up later anyway
All or nothing
Always show indicator for block updates

1. Disabled 2. C, log it as n saves. We roll them up later anyway 3. All or nothing 4. Always show indicator for block updates

marcel referenced this issue from a commit

2026-04-26 20:54:43 +02:00

feat(transcription): add bulk "Alle als fertig markieren" action to transcription panel

marcel commented

2026-04-26 20:55:01 +02:00

Implementation complete — branch `feat/issue-345-bulk-mark-reviewed`

What was done

Backend (PUT /api/documents/{documentId}/transcription-blocks/review-all)

Added markAllBlocksReviewed(UUID documentId, UUID userId) to TranscriptionService — @Transactional, single database round-trip via blockRepository.saveAll(), emits one BLOCK_REVIEWED audit event per previously-unreviewed block (Decision 2: Option C)
Added markAllBlocksReviewed endpoint to TranscriptionBlockController with @RequirePermission(Permission.WRITE_ALL) and Authentication authentication → requireUserId() pattern matching existing endpoints
Returns the full updated block list (same shape as listBlocks) for a clean frontend update pass

Backend tests

TranscriptionServiceTest: 5 new unit tests — sets all unreviewed blocks to reviewed, idempotent when already reviewed, emits N audit events for N unreviewed blocks, does not emit audit for already-reviewed blocks, returns empty list for empty document
TranscriptionBlockControllerTest: 5 new @WebMvcTest tests — 401 unauthenticated, 403 with READ_ALL only, 200 happy path with block list, 200 with empty list, 401 when user not found in database

All 68 backend tests pass (32 service + 36 controller).

Frontend

Added onMarkAllReviewed?: () => Promise<void> prop to TranscriptionEditView (optional, consistent with onTriggerOcr pattern)
Button placed in the sticky progress header, right-aligned next to the reviewedCount / totalCount geprüft text
Button is disabled (not hidden) when all blocks already reviewed — title="Alle Blöcke sind bereits als fertig markiert" (Decision 1)
Loading spinner replaces checkmark icon during operation (Decision 4: always show indicator, no threshold)
markAllReviewed() handler added to documents/[id]/+page.svelte, wired through as onMarkAllReviewed

Frontend tests (TranscriptionEditView.svelte.spec.ts)

5 new Vitest Browser component tests — button visible when prop provided + unreviewed blocks exist, button absent when prop not provided, button disabled when all reviewed, calls callback exactly once on click, button disabled while in-flight.

All 25 TranscriptionEditView tests pass.

Decisions applied

#	Decision	Choice
1	Button when all reviewed	Disabled with title tooltip
2	Audit log	N individual BLOCK_REVIEWED events (one per unreviewed block)
3	Atomicity	All-or-nothing via `@Transactional`
4	Loading indicator	Always show during operation

## Implementation complete — branch `feat/issue-345-bulk-mark-reviewed` ### What was done **Backend (`PUT /api/documents/{documentId}/transcription-blocks/review-all`)** - Added `markAllBlocksReviewed(UUID documentId, UUID userId)` to `TranscriptionService` — `@Transactional`, single database round-trip via `blockRepository.saveAll()`, emits one `BLOCK_REVIEWED` audit event per previously-unreviewed block (Decision 2: Option C) - Added `markAllBlocksReviewed` endpoint to `TranscriptionBlockController` with `@RequirePermission(Permission.WRITE_ALL)` and `Authentication authentication` → `requireUserId()` pattern matching existing endpoints - Returns the full updated block list (same shape as `listBlocks`) for a clean frontend update pass **Backend tests** - `TranscriptionServiceTest`: 5 new unit tests — sets all unreviewed blocks to reviewed, idempotent when already reviewed, emits N audit events for N unreviewed blocks, does not emit audit for already-reviewed blocks, returns empty list for empty document - `TranscriptionBlockControllerTest`: 5 new `@WebMvcTest` tests — 401 unauthenticated, 403 with `READ_ALL` only, 200 happy path with block list, 200 with empty list, 401 when user not found in database All 68 backend tests pass (32 service + 36 controller). **Frontend** - Added `onMarkAllReviewed?: () => Promise<void>` prop to `TranscriptionEditView` (optional, consistent with `onTriggerOcr` pattern) - Button placed in the sticky progress header, right-aligned next to the `reviewedCount / totalCount geprüft` text - Button is **disabled** (not hidden) when all blocks already reviewed — `title="Alle Blöcke sind bereits als fertig markiert"` (Decision 1) - Loading spinner replaces checkmark icon during operation (Decision 4: always show indicator, no threshold) - `markAllReviewed()` handler added to `documents/[id]/+page.svelte`, wired through as `onMarkAllReviewed` **Frontend tests** (`TranscriptionEditView.svelte.spec.ts`) 5 new Vitest Browser component tests — button visible when prop provided + unreviewed blocks exist, button absent when prop not provided, button disabled when all reviewed, calls callback exactly once on click, button disabled while in-flight. All 25 `TranscriptionEditView` tests pass. ### Decisions applied | # | Decision | Choice | |---|---|---| | 1 | Button when all reviewed | **Disabled** with title tooltip | | 2 | Audit log | **N individual BLOCK_REVIEWED events** (one per unreviewed block) | | 3 | Atomicity | **All-or-nothing** via `@Transactional` | | 4 | Loading indicator | **Always show** during operation |

marcel referenced a pull request that will close this issue

2026-04-26 21:07:49 +02:00

feat(transcription): add "Alle als fertig markieren" bulk action (#345) #352

marcel referenced this issue

2026-04-26 21:08:18 +02:00

feat(persons): show merge action inline with danger hint, remove Gefahrenzone collapsible (#342) #347

marcel referenced this issue

2026-04-26 21:08:27 +02:00

feat(transcription): add "Alle als fertig markieren" bulk action (#345) #352

marcel referenced this issue

2026-04-26 21:09:02 +02:00

feat(persons): show merge action inline with danger hint, remove Gefahrenzone collapsible (#342) #347