diff --git a/backend/src/test/java/org/raddatz/familienarchiv/controller/AnnotationControllerTest.java b/backend/src/test/java/org/raddatz/familienarchiv/controller/AnnotationControllerTest.java index 7a4546b6..368a21a0 100644 --- a/backend/src/test/java/org/raddatz/familienarchiv/controller/AnnotationControllerTest.java +++ b/backend/src/test/java/org/raddatz/familienarchiv/controller/AnnotationControllerTest.java @@ -154,6 +154,13 @@ class AnnotationControllerTest { .andExpect(status().isForbidden()); } + @Test + @WithMockUser(authorities = "READ_ALL") + void deleteAnnotation_returns403_whenUserHasOnlyReadAllPermission() throws Exception { + mockMvc.perform(delete("/api/documents/" + UUID.randomUUID() + "/annotations/" + UUID.randomUUID())) + .andExpect(status().isForbidden()); + } + @Test @WithMockUser(authorities = "ANNOTATE_ALL") void deleteAnnotation_returns204_whenHasAnnotatePermission() throws Exception { diff --git a/backend/src/test/java/org/raddatz/familienarchiv/controller/TranscriptionBlockControllerTest.java b/backend/src/test/java/org/raddatz/familienarchiv/controller/TranscriptionBlockControllerTest.java index 2231e9fb..31073057 100644 --- a/backend/src/test/java/org/raddatz/familienarchiv/controller/TranscriptionBlockControllerTest.java +++ b/backend/src/test/java/org/raddatz/familienarchiv/controller/TranscriptionBlockControllerTest.java @@ -260,6 +260,13 @@ class TranscriptionBlockControllerTest { .andExpect(status().isForbidden()); } + @Test + @WithMockUser(authorities = "READ_ALL") + void deleteBlock_returns403_whenUserHasOnlyReadAllPermission() throws Exception { + mockMvc.perform(delete(URL_BLOCK)) + .andExpect(status().isForbidden()); + } + @Test @WithMockUser(authorities = "WRITE_ALL") void deleteBlock_returns204_whenAuthorised() throws Exception { diff --git a/frontend/src/lib/components/AnnotationLayer.svelte b/frontend/src/lib/components/AnnotationLayer.svelte index ac7dbc7c..129fef36 100644 --- a/frontend/src/lib/components/AnnotationLayer.svelte +++ b/frontend/src/lib/components/AnnotationLayer.svelte @@ -18,7 +18,8 @@ let { dimmed = false, flashAnnotationId = null, onDraw, - onAnnotationClick + onAnnotationClick, + onDeleteRequest }: { annotations: Annotation[]; canDraw: boolean; @@ -29,6 +30,7 @@ let { flashAnnotationId?: string | null; onDraw: (rect: DrawRect) => void; onAnnotationClick?: (id: string) => void; + onDeleteRequest?: (annotationId: string) => void; } = $props(); let drawStart = $state<{ x: number; y: number } | null>(null); @@ -112,6 +114,8 @@ const containerStyle = $derived( dimmed={dimmed} blockNumber={blockNumbers[annotation.id]} isFlashing={flashAnnotationId === annotation.id} + showDelete={canDraw} + onDeleteRequest={() => onDeleteRequest?.(annotation.id)} onclick={() => onAnnotationClick?.(annotation.id)} onpointerenter={() => (hoveredId = annotation.id)} onpointerleave={() => (hoveredId = null)} diff --git a/frontend/src/lib/components/AnnotationLayer.svelte.spec.ts b/frontend/src/lib/components/AnnotationLayer.svelte.spec.ts index 7a4b4e07..b134c50f 100644 --- a/frontend/src/lib/components/AnnotationLayer.svelte.spec.ts +++ b/frontend/src/lib/components/AnnotationLayer.svelte.spec.ts @@ -98,7 +98,7 @@ describe('AnnotationLayer', () => { expect(el2.style.opacity).toBe('1'); }); - it('does not show delete buttons (annotations owned by blocks)', async () => { + it('does not show delete button when annotation is not hovered or active', async () => { render(AnnotationLayer, { annotations: [makeAnnotation('ann-1')], canDraw: true, @@ -107,6 +107,19 @@ describe('AnnotationLayer', () => { }); await expect.element(page.getByTestId('annotation-ann-1')).toBeInTheDocument(); - expect(page.getByRole('button', { name: /löschen/i }).query()).toBeNull(); + expect(page.getByTestId('annotation-delete-ann-1').query()).toBeNull(); + }); + + it('does not show delete button when canDraw is false even if annotation is active', async () => { + render(AnnotationLayer, { + annotations: [makeAnnotation('ann-1')], + canDraw: false, + color: '#00C7B1', + activeAnnotationId: 'ann-1', + onDraw: () => {} + }); + + await expect.element(page.getByTestId('annotation-ann-1')).toBeInTheDocument(); + expect(page.getByTestId('annotation-delete-ann-1').query()).toBeNull(); }); }); diff --git a/frontend/src/lib/components/AnnotationShape.svelte b/frontend/src/lib/components/AnnotationShape.svelte index a7c667f2..31be34f4 100644 --- a/frontend/src/lib/components/AnnotationShape.svelte +++ b/frontend/src/lib/components/AnnotationShape.svelte @@ -11,6 +11,8 @@ let { blockNumber = undefined, isFlashing = false, isResizable = false, + showDelete = false, + onDeleteRequest, onclick, onpointerenter, onpointerleave @@ -23,11 +25,15 @@ let { blockNumber?: number | undefined; isFlashing?: boolean; isResizable?: boolean; + showDelete?: boolean; + onDeleteRequest?: () => void; onclick: () => void; onpointerenter: () => void; onpointerleave: () => void; } = $props(); +const deleteVisible = $derived(showDelete && (isHovered || isActive)); + function hexToRgba(hex: string, alpha: number): string { const r = parseInt(hex.slice(1, 3), 16); const g = parseInt(hex.slice(3, 5), 16); @@ -83,6 +89,7 @@ let shapeStyle = $derived( onclick={onclick} onkeydown={(e) => { if (e.key === 'Enter' || e.key === ' ') onclick(); + if (e.key === 'Delete' && showDelete) onDeleteRequest?.(); }} onpointerenter={onpointerenter} onpointerleave={onpointerleave} @@ -112,6 +119,51 @@ let shapeStyle = $derived( {blockNumber} {/if} + {#if deleteVisible} + + {/if} {#if isResizable} {/if} diff --git a/frontend/src/lib/components/AnnotationShape.svelte.spec.ts b/frontend/src/lib/components/AnnotationShape.svelte.spec.ts new file mode 100644 index 00000000..87f1cae1 --- /dev/null +++ b/frontend/src/lib/components/AnnotationShape.svelte.spec.ts @@ -0,0 +1,177 @@ +import { describe, it, expect, vi, afterEach } from 'vitest'; +import { cleanup, render } from 'vitest-browser-svelte'; +import { page } from 'vitest/browser'; +import AnnotationShape from './AnnotationShape.svelte'; + +afterEach(cleanup); + +function makeAnnotation(id = 'ann-1') { + return { + id, + documentId: 'doc-1', + pageNumber: 1, + x: 0.1, + y: 0.1, + width: 0.3, + height: 0.2, + color: '#00C7B1', + createdAt: new Date().toISOString() + }; +} + +describe('AnnotationShape', () => { + it('renders the annotation element', async () => { + render(AnnotationShape, { + annotation: makeAnnotation(), + isHovered: false, + isActive: false, + onclick: () => {}, + onpointerenter: () => {}, + onpointerleave: () => {} + }); + + await expect.element(page.getByTestId('annotation-ann-1')).toBeInTheDocument(); + }); + + it('does not show delete button when showDelete is false', async () => { + render(AnnotationShape, { + annotation: makeAnnotation(), + isHovered: true, + isActive: false, + showDelete: false, + onDeleteRequest: vi.fn(), + onclick: () => {}, + onpointerenter: () => {}, + onpointerleave: () => {} + }); + + expect(page.getByTestId('annotation-delete-ann-1').query()).toBeNull(); + }); + + it('does not show delete button when showDelete is true but neither hovered nor active', async () => { + render(AnnotationShape, { + annotation: makeAnnotation(), + isHovered: false, + isActive: false, + showDelete: true, + onDeleteRequest: vi.fn(), + onclick: () => {}, + onpointerenter: () => {}, + onpointerleave: () => {} + }); + + expect(page.getByTestId('annotation-delete-ann-1').query()).toBeNull(); + }); + + it('shows delete button when showDelete is true and isHovered is true', async () => { + render(AnnotationShape, { + annotation: makeAnnotation(), + isHovered: true, + isActive: false, + showDelete: true, + onDeleteRequest: vi.fn(), + onclick: () => {}, + onpointerenter: () => {}, + onpointerleave: () => {} + }); + + await expect.element(page.getByTestId('annotation-delete-ann-1')).toBeInTheDocument(); + }); + + it('shows delete button when showDelete is true and isActive is true', async () => { + render(AnnotationShape, { + annotation: makeAnnotation(), + isHovered: false, + isActive: true, + showDelete: true, + onDeleteRequest: vi.fn(), + onclick: () => {}, + onpointerenter: () => {}, + onpointerleave: () => {} + }); + + await expect.element(page.getByTestId('annotation-delete-ann-1')).toBeInTheDocument(); + }); + + it('calls onDeleteRequest when delete button is clicked', async () => { + const onDeleteRequest = vi.fn(); + + render(AnnotationShape, { + annotation: makeAnnotation(), + isHovered: true, + isActive: false, + showDelete: true, + onDeleteRequest, + onclick: () => {}, + onpointerenter: () => {}, + onpointerleave: () => {} + }); + + const deleteBtn = page.getByTestId('annotation-delete-ann-1'); + await deleteBtn.click(); + + expect(onDeleteRequest).toHaveBeenCalledOnce(); + }); + + it('does not call onclick when delete button is clicked', async () => { + const onclick = vi.fn(); + const onDeleteRequest = vi.fn(); + + render(AnnotationShape, { + annotation: makeAnnotation(), + isHovered: true, + isActive: false, + showDelete: true, + onDeleteRequest, + onclick, + onpointerenter: () => {}, + onpointerleave: () => {} + }); + + const deleteBtn = page.getByTestId('annotation-delete-ann-1'); + await deleteBtn.click(); + + expect(onclick).not.toHaveBeenCalled(); + expect(onDeleteRequest).toHaveBeenCalledOnce(); + }); + + it('calls onDeleteRequest when Delete key is pressed on the annotation', async () => { + const onDeleteRequest = vi.fn(); + + render(AnnotationShape, { + annotation: makeAnnotation(), + isHovered: false, + isActive: true, + showDelete: true, + onDeleteRequest, + onclick: () => {}, + onpointerenter: () => {}, + onpointerleave: () => {} + }); + + const annotationEl = page.getByTestId('annotation-ann-1').element() as HTMLElement; + annotationEl.dispatchEvent(new KeyboardEvent('keydown', { key: 'Delete', bubbles: true })); + + expect(onDeleteRequest).toHaveBeenCalledOnce(); + }); + + it('does not call onDeleteRequest on Delete key when showDelete is false', async () => { + const onDeleteRequest = vi.fn(); + + render(AnnotationShape, { + annotation: makeAnnotation(), + isHovered: false, + isActive: true, + showDelete: false, + onDeleteRequest, + onclick: () => {}, + onpointerenter: () => {}, + onpointerleave: () => {} + }); + + const annotationEl = page.getByTestId('annotation-ann-1').element() as HTMLElement; + annotationEl.dispatchEvent(new KeyboardEvent('keydown', { key: 'Delete', bubbles: true })); + + expect(onDeleteRequest).not.toHaveBeenCalled(); + }); +}); diff --git a/frontend/src/lib/components/DocumentViewer.svelte b/frontend/src/lib/components/DocumentViewer.svelte index c48d1f2a..b22f3d6b 100644 --- a/frontend/src/lib/components/DocumentViewer.svelte +++ b/frontend/src/lib/components/DocumentViewer.svelte @@ -24,6 +24,7 @@ type Props = { flashAnnotationId?: string | null; onAnnotationClick: (id: string) => void; onTranscriptionDraw?: (rect: DrawRect) => void; + onDeleteAnnotationRequest?: (annotationId: string) => void; }; let { @@ -38,7 +39,8 @@ let { annotationsDimmed = false, flashAnnotationId = null, onAnnotationClick, - onTranscriptionDraw + onTranscriptionDraw, + onDeleteAnnotationRequest }: Props = $props(); @@ -98,6 +100,7 @@ let { flashAnnotationId={flashAnnotationId} onAnnotationClick={onAnnotationClick} onTranscriptionDraw={onTranscriptionDraw} + onDeleteAnnotationRequest={onDeleteAnnotationRequest} documentFileHash={doc.fileHash ?? null} /> {:else if fileUrl} diff --git a/frontend/src/lib/components/PdfViewer.svelte b/frontend/src/lib/components/PdfViewer.svelte index 844aa338..7642b197 100644 --- a/frontend/src/lib/components/PdfViewer.svelte +++ b/frontend/src/lib/components/PdfViewer.svelte @@ -18,6 +18,7 @@ let { activeAnnotationId = $bindable(null), onAnnotationClick, onTranscriptionDraw, + onDeleteAnnotationRequest, documentFileHash, annotationsDimmed = false, flashAnnotationId = null @@ -30,6 +31,7 @@ let { activeAnnotationId?: string | null; onAnnotationClick?: (id: string) => void; onTranscriptionDraw?: (rect: DrawRect) => void; + onDeleteAnnotationRequest?: (annotationId: string) => void; documentFileHash?: string | null; annotationsDimmed?: boolean; flashAnnotationId?: string | null; @@ -264,6 +266,7 @@ function handleAnnotationClick(id: string) { flashAnnotationId={flashAnnotationId} onDraw={handleDraw} onAnnotationClick={handleAnnotationClick} + onDeleteRequest={onDeleteAnnotationRequest} /> {/if} diff --git a/frontend/src/routes/documents/[id]/+page.svelte b/frontend/src/routes/documents/[id]/+page.svelte index 06a38484..2cac8cf8 100644 --- a/frontend/src/routes/documents/[id]/+page.svelte +++ b/frontend/src/routes/documents/[id]/+page.svelte @@ -13,9 +13,12 @@ import { getErrorMessage } from '$lib/errors'; import { translateOcrProgress } from '$lib/ocr/translateOcrProgress'; import { createFileLoader } from '$lib/hooks/useFileLoader.svelte'; import { scrollToCommentFromQuery } from '$lib/utils/deepLinkScroll'; +import { getConfirmService } from '$lib/services/confirm.svelte.js'; let { data } = $props(); +const { confirm } = getConfirmService(); + const doc = $derived(data.document); const canWrite = $derived(data.canWrite ?? false); const currentUserId = $derived((data.user?.id as string | undefined) ?? null); @@ -105,6 +108,26 @@ async function deleteBlock(blockId: string) { annotationReloadKey++; } +async function handleAnnotationDeleteRequest(annotationId: string) { + const confirmed = await confirm({ + title: m.transcription_block_delete_confirm(), + destructive: true + }); + if (!confirmed) return; + + const block = transcriptionBlocks.find((b) => b.annotationId === annotationId); + if (block) { + await deleteBlock(block.id); + } else { + // Annotation has no linked block — delete the annotation directly + const res = await fetch(`/api/documents/${doc.id}/annotations/${annotationId}`, { + method: 'DELETE' + }); + if (!res.ok) throw new Error('Delete annotation failed'); + annotationReloadKey++; + } +} + async function reviewToggle(blockId: string) { const res = await fetch(`/api/documents/${doc.id}/transcription-blocks/${blockId}/review`, { method: 'PUT' @@ -381,6 +404,7 @@ onMount(() => { bind:activeAnnotationId={activeAnnotationId} onAnnotationClick={handleAnnotationClick} onTranscriptionDraw={createBlockFromDraw} + onDeleteAnnotationRequest={handleAnnotationDeleteRequest} />