From 2e0f85c3606b3d0e2f6d732d6564aa0136adc386 Mon Sep 17 00:00:00 2001 From: Marcel Date: Mon, 25 May 2026 15:08:04 +0200 Subject: [PATCH] fix(review): address reviewer concerns from PR #661 - Replace brittle createdAt===updatedAt isNew() check with a 7-day recency window (created within last 7 days = new) - Add createdAt/updatedAt to searchItem fixture in page.server.spec.ts and assert they are propagated to recentDocs - Replace null timestamps in DocumentListItem test fixtures with a fixed LocalDateTime to satisfy the @Schema(required) contract Co-Authored-By: Claude Sonnet 4.6 --- .../document/DocumentControllerTest.java | 6 +++-- .../document/DocumentSearchResultTest.java | 7 ++++-- .../shared/dashboard/ReaderRecentDocs.svelte | 2 +- .../dashboard/ReaderRecentDocs.svelte.spec.ts | 24 ++++++++++++------- .../dashboard/ReaderRecentDocs.svelte.test.ts | 12 +++++----- frontend/src/routes/page.server.spec.ts | 6 ++++- 6 files changed, 36 insertions(+), 21 deletions(-) diff --git a/backend/src/test/java/org/raddatz/familienarchiv/document/DocumentControllerTest.java b/backend/src/test/java/org/raddatz/familienarchiv/document/DocumentControllerTest.java index 359211ba..fe15ba3b 100644 --- a/backend/src/test/java/org/raddatz/familienarchiv/document/DocumentControllerTest.java +++ b/backend/src/test/java/org/raddatz/familienarchiv/document/DocumentControllerTest.java @@ -135,7 +135,8 @@ class DocumentControllerTest { .thenReturn(DocumentSearchResult.of(List.of(new DocumentListItem( docId, "Brief an Anna", "brief.pdf", null, null, null, List.of(), List.of(), null, null, null, null, - 0, List.of(), matchData, null, null)))); + 0, List.of(), matchData, + LocalDateTime.of(2026, 1, 15, 10, 0), LocalDateTime.of(2026, 1, 15, 10, 0))))); mockMvc.perform(get("/api/documents/search").param("q", "Brief")) .andExpect(status().isOk()) @@ -153,7 +154,8 @@ class DocumentControllerTest { .thenReturn(DocumentSearchResult.of(List.of(new DocumentListItem( docId, "Brief an Anna", "brief.pdf", null, null, null, List.of(), List.of(), null, null, null, null, - 0, List.of(), matchData, null, null)))); + 0, List.of(), matchData, + LocalDateTime.of(2026, 1, 15, 10, 0), LocalDateTime.of(2026, 1, 15, 10, 0))))); mockMvc.perform(get("/api/documents/search")) .andExpect(status().isOk()) diff --git a/backend/src/test/java/org/raddatz/familienarchiv/document/DocumentSearchResultTest.java b/backend/src/test/java/org/raddatz/familienarchiv/document/DocumentSearchResultTest.java index 9effa3bc..a487e272 100644 --- a/backend/src/test/java/org/raddatz/familienarchiv/document/DocumentSearchResultTest.java +++ b/backend/src/test/java/org/raddatz/familienarchiv/document/DocumentSearchResultTest.java @@ -5,6 +5,7 @@ import org.junit.jupiter.api.Test; import org.raddatz.familienarchiv.audit.ActivityActorDTO; import org.springframework.data.domain.PageRequest; +import java.time.LocalDateTime; import java.util.List; import java.util.UUID; @@ -16,7 +17,8 @@ class DocumentSearchResultTest { return new DocumentListItem( docId, "Test", "test.pdf", null, null, null, List.of(), List.of(), null, null, null, null, - 0, List.of(), SearchMatchData.empty(), null, null); + 0, List.of(), SearchMatchData.empty(), + LocalDateTime.of(2026, 1, 15, 10, 0), LocalDateTime.of(2026, 1, 15, 10, 0)); } @Test @@ -66,7 +68,8 @@ class DocumentSearchResultTest { DocumentListItem item = new DocumentListItem( id, "T", "t.pdf", null, null, null, List.of(), List.of(), null, null, null, null, - 75, List.of(actor), SearchMatchData.empty(), null, null); + 75, List.of(actor), SearchMatchData.empty(), + LocalDateTime.of(2026, 1, 15, 10, 0), LocalDateTime.of(2026, 1, 15, 10, 0)); DocumentSearchResult result = DocumentSearchResult.of(List.of(item)); diff --git a/frontend/src/lib/shared/dashboard/ReaderRecentDocs.svelte b/frontend/src/lib/shared/dashboard/ReaderRecentDocs.svelte index 5721accc..98985f95 100644 --- a/frontend/src/lib/shared/dashboard/ReaderRecentDocs.svelte +++ b/frontend/src/lib/shared/dashboard/ReaderRecentDocs.svelte @@ -12,7 +12,7 @@ interface Props { const { documents }: Props = $props(); function isNew(doc: DocumentListItem): boolean { - return new Date(doc.createdAt).getTime() === new Date(doc.updatedAt).getTime(); + return new Date(doc.createdAt).getTime() > Date.now() - 7 * 24 * 60 * 60 * 1000; } diff --git a/frontend/src/lib/shared/dashboard/ReaderRecentDocs.svelte.spec.ts b/frontend/src/lib/shared/dashboard/ReaderRecentDocs.svelte.spec.ts index c13c92b1..51dd5d8e 100644 --- a/frontend/src/lib/shared/dashboard/ReaderRecentDocs.svelte.spec.ts +++ b/frontend/src/lib/shared/dashboard/ReaderRecentDocs.svelte.spec.ts @@ -88,8 +88,14 @@ describe('ReaderRecentDocs', () => { expect(thumb!.className).toMatch(/rounded-/); }); - it('shows "Neu" accent-pill badge when createdAt equals updatedAt', async () => { - render(ReaderRecentDocs, { documents: [baseDoc] }); + it('shows "Neu" accent-pill badge when document was created within the last 7 days', async () => { + const recentDoc: Document = { + ...baseDoc, + id: 'doc-recent', + createdAt: new Date(Date.now() - 2 * 24 * 60 * 60 * 1000).toISOString(), + updatedAt: new Date(Date.now() - 1 * 24 * 60 * 60 * 1000).toISOString() + }; + render(ReaderRecentDocs, { documents: [recentDoc] }); const badge = page.getByText(/^Neu$/i); await expect.element(badge).toBeInTheDocument(); const cls = ((await badge.element()) as HTMLElement).className; @@ -98,7 +104,7 @@ describe('ReaderRecentDocs', () => { expect(cls).toMatch(/\btext-ink\b/); }); - it('shows no badge when updatedAt differs from createdAt', async () => { + it('shows no badge when document was created more than 7 days ago', async () => { render(ReaderRecentDocs, { documents: [updatedDoc] }); const badge = page.getByText(/^Neu$/i); await expect.element(badge).not.toBeInTheDocument(); @@ -106,14 +112,14 @@ describe('ReaderRecentDocs', () => { await expect.element(updatedBadge).not.toBeInTheDocument(); }); - it('shows "Neu" badge when createdAt and updatedAt represent the same instant in different ISO formats', async () => { - const sameInstantDoc: Document = { + it('shows "Neu" badge when document was created 6 days ago', async () => { + const almostOldDoc: Document = { ...baseDoc, - id: 'doc-same-instant', - createdAt: '2025-01-01T12:00:00Z', - updatedAt: '2025-01-01T12:00:00.000Z' + id: 'doc-almost-old', + createdAt: new Date(Date.now() - 6 * 24 * 60 * 60 * 1000).toISOString(), + updatedAt: new Date(Date.now() - 5 * 24 * 60 * 60 * 1000).toISOString() }; - render(ReaderRecentDocs, { documents: [sameInstantDoc] }); + render(ReaderRecentDocs, { documents: [almostOldDoc] }); const badge = page.getByText(/^Neu$/i); await expect.element(badge).toBeInTheDocument(); }); diff --git a/frontend/src/lib/shared/dashboard/ReaderRecentDocs.svelte.test.ts b/frontend/src/lib/shared/dashboard/ReaderRecentDocs.svelte.test.ts index d15e6f7f..6c93c55e 100644 --- a/frontend/src/lib/shared/dashboard/ReaderRecentDocs.svelte.test.ts +++ b/frontend/src/lib/shared/dashboard/ReaderRecentDocs.svelte.test.ts @@ -31,25 +31,25 @@ describe('ReaderRecentDocs', () => { .toHaveAttribute('href', '/documents'); }); - it('renders the New badge when createdAt equals updatedAt', async () => { + it('renders the New badge when document was created within the last 7 days', async () => { + const recentDate = new Date(Date.now() - 2 * 24 * 60 * 60 * 1000).toISOString(); + const laterUpdate = new Date(Date.now() - 1 * 24 * 60 * 60 * 1000).toISOString(); render(ReaderRecentDocs, { props: { - documents: [ - makeDoc({ createdAt: '2026-04-15T10:00:00Z', updatedAt: '2026-04-15T10:00:00Z' }) - ] + documents: [makeDoc({ createdAt: recentDate, updatedAt: laterUpdate })] } }); await expect.element(page.getByText('Neu')).toBeVisible(); }); - it('hides the New badge when document was updated after creation', async () => { + it('hides the New badge when document was created more than 7 days ago', async () => { render(ReaderRecentDocs, { props: { documents: [ makeDoc({ createdAt: '2026-04-15T10:00:00Z', - updatedAt: '2026-04-15T11:00:00Z' + updatedAt: '2026-04-15T10:00:00Z' }) ] } diff --git a/frontend/src/routes/page.server.spec.ts b/frontend/src/routes/page.server.spec.ts index 27ce1e30..a04cf1fe 100644 --- a/frontend/src/routes/page.server.spec.ts +++ b/frontend/src/routes/page.server.spec.ts @@ -403,7 +403,9 @@ describe('home page load — reader branch (isReader = !canWrite && !canAnnotate receivers: [], tags: [], contributors: [], - matchData: { titleOffsets: [], senderMatched: false } + matchData: { titleOffsets: [], senderMatched: false }, + createdAt: '2026-05-01T10:00:00Z', + updatedAt: '2026-05-10T08:00:00Z' }; const mockGet = vi .fn() @@ -436,6 +438,8 @@ describe('home page load — reader branch (isReader = !canWrite && !canAnnotate expect(result.recentDocs).toHaveLength(1); expect(result.recentDocs[0]).toBeDefined(); expect(result.recentDocs[0].id).toBe('d1'); + expect(result.recentDocs[0].createdAt).toBe('2026-05-01T10:00:00Z'); + expect(result.recentDocs[0].updatedAt).toBe('2026-05-10T08:00:00Z'); } });