feat(#145): transform home page into user dashboard #151

Merged

marcel merged 14 commits from feat/145-dashboard into main 2026-03-29 12:39:39 +02:00

Conversation 4 Commits 14 Files Changed 39 +566 -114

marcel commented 2026-03-29 10:34:29 +02:00

Owner

Copy Link

Closes #145

Summary

• Two-mode home page: dashboard when no search filters are active, document list when searching
• 4 new widget components (all with Vitest tests):
  • DashboardResumeStrip — "continue where you left off" (localStorage)
  • DashboardMentions — last 5 notifications (mentions + replies), deep-linked to the specific comment
  • DashboardNeedsMetadata — incomplete documents queue with link to /enrich
  • DashboardRecentDocuments — last 5 uploaded documents with createdAt date
• Backend: notification type/read filter, incomplete docs ?size= param, regenerated API types
• i18n: all strings translated to de/en/es via Paraglide
• Playwright screenshot spec: 3 viewports × 2 themes (see proofshots below)

Proofshots

Mobile (390×844)

Light	Dark

Tablet (768×1024)

Light	Dark

Desktop (1440×900)

Light	Dark

Test plan

• Dashboard shows on / when no search filters are active
• Dashboard shows search results when any filter is active
• Notifications widget links to the correct comment (?commentId= + &annotationId=)
• Recent docs shows upload date (createdAt), not document date
• NeedsMetadata links correctly to /enrich/{id}
• Resume strip appears after visiting a document
• All text in de/en/es (toggle language in header)
• All viewports and dark mode look correct (see proofshots above)

🤖 Generated with Claude Code

Closes #145 ## Summary - **Two-mode home page**: dashboard when no search filters are active, document list when searching - **4 new widget components** (all with Vitest tests): - `DashboardResumeStrip` — "continue where you left off" (localStorage) - `DashboardMentions` — last 5 notifications (mentions + replies), deep-linked to the specific comment - `DashboardNeedsMetadata` — incomplete documents queue with link to `/enrich` - `DashboardRecentDocuments` — last 5 uploaded documents with `createdAt` date - **Backend**: notification type/read filter, incomplete docs `?size=` param, regenerated API types - **i18n**: all strings translated to de/en/es via Paraglide - **Playwright screenshot spec**: 3 viewports × 2 themes (see proofshots below) ## Proofshots ### Mobile (390×844) | Light | Dark | |-------|------| |  |  | ### Tablet (768×1024) | Light | Dark | |-------|------| |  |  | ### Desktop (1440×900) | Light | Dark | |-------|------| |  |  | ## Test plan - [ ] Dashboard shows on `/` when no search filters are active - [ ] Dashboard shows search results when any filter is active - [ ] Notifications widget links to the correct comment (`?commentId=` + `&annotationId=`) - [ ] Recent docs shows upload date (`createdAt`), not document date - [ ] NeedsMetadata links correctly to `/enrich/{id}` - [ ] Resume strip appears after visiting a document - [ ] All text in de/en/es (toggle language in header) - [ ] All viewports and dark mode look correct (see proofshots above) 🤖 Generated with [Claude Code](https://claude.com/claude-code)

marcel added 5 commits 2026-03-29 10:34:29 +02:00

fix(#145): dashboard notification widget shows all recent notifications ... 08f3f92167

- Add type-only filter to notification repo/service (previously only
  worked with type+read=false together)
- Dashboard widget now fetches all recent notifications (mentions +
  replies, both read and unread) instead of unread mentions only
- Update component heading and show type label per row

Root cause: Berit's mentions were read=true, so the unread-only filter
returned 0 results. The recent docs widget had no REVIEWED documents
because 'marking ready' sets metadata_complete, not status=REVIEWED.
Recent docs now shows all uploads without a status filter.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>

feat(#145): internationalise dashboard widget strings (de/en/es) ... c8da2224f8

Replace all hardcoded German strings in dashboard components with
Paraglide translation keys. Date locale uses getLocale() instead
of the hardcoded 'de-DE'.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>

fix(#145): deep-link notifications; show createdAt in recent docs ... 7734ce7bae

- Notification widget builds full link with ?commentId= and
  &annotationId= params, matching the bell notification behaviour
- Recent docs widget shows createdAt (upload date) instead of
  documentDate (the date on the original document)

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>

test(#145): add Playwright screenshot spec for dashboard (3 viewports × 2 themes) ... a7b0bd96d4

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>

docs(#145): add dashboard proofshots (3 viewports × 2 themes) ... 698a0fb15e

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>

marcel added 1 commit 2026-03-29 11:02:04 +02:00

refactor(e2e): extract reusable captureProofshots helper ... dc487e2f97

Any future feature spec now just calls:
  captureProofshots('/my-route', 'feature-name')

to get 6 screenshots (3 viewports × 2 themes) saved to
proofshot-artifacts/{feature-name}/.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>

marcel added 2 commits 2026-03-29 11:30:46 +02:00

feat(#145): add recent-activity endpoint sorted by updatedAt ... 6976daa910

Add GET /api/documents/recent-activity?size=N endpoint that returns
the N most recently updated documents sorted by updatedAt DESC.
Includes TDD: failing tests written first, then production code.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>

feat(#145): switch dashboard to show last-activity documents ... 2171c3702a

Replace recent-by-creation fetch with GET /api/documents/recent-activity
(sorted by updatedAt) in the dashboard. Update DashboardRecentDocuments
component to use doc.updatedAt, update i18n heading to "Zuletzt aktiv" /
"Recent Activity" / "Actividad reciente", and regenerate API types.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>

marcel commented 2026-03-29 11:32:26 +02:00

Author

Owner

Copy Link

UI Review — Leonie Voss (updated after commit 7eda0ae)

Overall the dashboard is clean and the dark mode implementation is intentional and well-executed. Semantic color tokens hold up across all 6 proofshots. Deep-linking in DashboardMentions is correct and well-tested. i18n coverage across de/en/es is complete. The notifications widget showing all recent items (not just unread) is the right call given the bell icon handles the unread queue.

Two issues still need fixing before merge. One proofshot finding is now resolved.

---

[High] +page.svelte:103 — Grid breaks when Mentions widget is empty ❌ still open

DashboardMentions renders no DOM element at all when mentions.length === 0 (bare {#if}, no fallback). Its grid cell disappears and DashboardNeedsMetadata shifts into the left column with an empty right column beside it. The new screenshots mask this — they populate the widget via DB seed — but the layout regression still exists for any user with no notifications.

Fix — collapse to single column when there are no mentions:

<div class="mt-6 grid gap-4 {(data.mentions?.length ?? 0) > 0 ? 'lg:grid-cols-2' : ''}">

When Mentions is empty, NeedsMetadata spans full width — consistent with the Recent Documents card below it.

---

[Medium] DashboardNeedsMetadata.svelte:31 — 12px link is below the accessibility floor ❌ still open

text-xs renders at 12px. Minimum for interactive text is text-sm (14px), 16px preferred — especially for the 60+ part of our dual audience. Also: hover:text-ink alone doesn't satisfy WCAG 1.4.1 — color cannot be the only visual differentiator. Add hover:underline.

<a href="/enrich" class="font-sans text-sm text-ink-2 hover:text-ink hover:underline"
	>{m.dashboard_needs_metadata_show_all()}</a>

---

[Low] dashboard-screenshots.spec.ts — ResumeStrip and Notifications proofshot coverage ✅ resolved in 7eda0ae

Both widgets are now visible in all 6 screenshots. The seeding approach is clean: beforeAll/afterAll with psql inserts scoped to sentinel actor names, plus page.evaluate() for the localStorage entry — no full document page load required. The notifications query change ({ size: 5 } replacing { type: MENTION, read: false, size: 5 }) is consistent with showing all recent activity; the bell icon handles the unread-only queue separately.

---

[Low] DashboardRecentDocuments.svelte:23 — Silent removal of noon-anchor (unchanged)

Still flagged. The removal is correct, but add a short comment for the next reader:

// updatedAt is a full ISO datetime — no T12:00:00 noon-anchor needed here
}).format(new Date(dateStr));

**UI Review — Leonie Voss** *(updated after commit 7eda0ae)* Overall the dashboard is clean and the dark mode implementation is intentional and well-executed. Semantic color tokens hold up across all 6 proofshots. Deep-linking in `DashboardMentions` is correct and well-tested. i18n coverage across de/en/es is complete. The notifications widget showing all recent items (not just unread) is the right call given the bell icon handles the unread queue. **Two issues still need fixing before merge.** One proofshot finding is now resolved. --- ### [High] `+page.svelte:103` — Grid breaks when Mentions widget is empty ❌ *still open* `DashboardMentions` renders no DOM element at all when `mentions.length === 0` (bare `{#if}`, no fallback). Its grid cell disappears and `DashboardNeedsMetadata` shifts into the left column with an empty right column beside it. The new screenshots mask this — they populate the widget via DB seed — but the layout regression still exists for any user with no notifications. Fix — collapse to single column when there are no mentions: ```svelte <div class="mt-6 grid gap-4 {(data.mentions?.length ?? 0) > 0 ? 'lg:grid-cols-2' : ''}"> ``` When Mentions is empty, NeedsMetadata spans full width — consistent with the Recent Documents card below it. --- ### [Medium] `DashboardNeedsMetadata.svelte:31` — 12px link is below the accessibility floor ❌ *still open* `text-xs` renders at 12px. Minimum for interactive text is `text-sm` (14px), 16px preferred — especially for the 60+ part of our dual audience. Also: `hover:text-ink` alone doesn't satisfy WCAG 1.4.1 — color cannot be the only visual differentiator. Add `hover:underline`. ```svelte <a href="/enrich" class="font-sans text-sm text-ink-2 hover:text-ink hover:underline" >{m.dashboard_needs_metadata_show_all()}</a> ``` --- ### [Low] `dashboard-screenshots.spec.ts` — ResumeStrip and Notifications proofshot coverage ✅ *resolved in 7eda0ae* Both widgets are now visible in all 6 screenshots. The seeding approach is clean: `beforeAll`/`afterAll` with psql inserts scoped to sentinel actor names, plus `page.evaluate()` for the localStorage entry — no full document page load required. The notifications query change (`{ size: 5 }` replacing `{ type: MENTION, read: false, size: 5 }`) is consistent with showing all recent activity; the bell icon handles the unread-only queue separately. --- ### [Low] `DashboardRecentDocuments.svelte:23` — Silent removal of noon-anchor *(unchanged)* Still flagged. The removal is correct, but add a short comment for the next reader: ```typescript // updatedAt is a full ISO datetime — no T12:00:00 noon-anchor needed here }).format(new Date(dateStr)); ```

marcel commented 2026-03-29 11:38:24 +02:00

Author

Owner

Copy Link

Architectural Review — @mkeller

Good PR overall. Clean layering, proper test coverage at all three levels, i18n complete. Two issues need attention before merge.

---

🔴 1. Full-table scan in getRecentActivity — must fix

DocumentService.java:264

return documentRepository.findAll(Sort.by(Sort.Direction.DESC, "updatedAt"))
        .stream().limit(size).toList();

This loads every document into memory, then discards all but size. On a small archive with 200 docs it's invisible. On one with 5,000 scanned letters it's a noticeable pause on every dashboard load.

Fix — push the LIMIT to the database:

return documentRepository.findAll(
    PageRequest.of(0, size, Sort.by(Sort.Direction.DESC, "updatedAt"))
).getContent();

This is the same pattern already used in findIncompleteDocuments. No new repository method needed.

---

🔴 2. UTC off-by-one comment missing in DashboardRecentDocuments — must fix

DashboardRecentDocuments.svelte:20

// before
.format(new Date(dateStr + 'T12:00:00'));
// after
.format(new Date(dateStr));

The removal is correct — updatedAt is a full ISO datetime, not a date-only string, so the T12:00:00 suffix isn't needed. But CLAUDE.md documents the T12:00:00 pattern as a deliberate UTC off-by-one fix, so the next developer will "fix" this back. Add one comment explaining why the suffix is absent here:

// updatedAt is a full ISO datetime — no T12:00:00 suffix needed,
// the time zone offset is already explicit in the value.
.format(new Date(dateStr));

---

🟢 What's done well

• Layering is clean: DocumentController → DocumentService → Repository, no repo leakage.
• Promise.allSettled for widget isolation: one failing widget doesn't crash the dashboard.
• Test coverage: controller, service, and repository tests all present and testing the right things. The NotificationRepositoryTest against Testcontainers is the right way to validate derived queries.
• onMount guard on DashboardResumeStrip: localStorage is properly client-only, no SSR hazard.
• Reusable captureProofshots helper: new feature specs are now one line — good extraction.

## Architectural Review — @mkeller Good PR overall. Clean layering, proper test coverage at all three levels, i18n complete. Two issues need attention before merge. --- ### 🔴 1. Full-table scan in `getRecentActivity` — must fix **`DocumentService.java:264`** ```java return documentRepository.findAll(Sort.by(Sort.Direction.DESC, "updatedAt")) .stream().limit(size).toList(); ``` This loads **every document** into memory, then discards all but `size`. On a small archive with 200 docs it's invisible. On one with 5,000 scanned letters it's a noticeable pause on every dashboard load. Fix — push the `LIMIT` to the database: ```java return documentRepository.findAll( PageRequest.of(0, size, Sort.by(Sort.Direction.DESC, "updatedAt")) ).getContent(); ``` This is the same pattern already used in `findIncompleteDocuments`. No new repository method needed. --- ### 🔴 2. UTC off-by-one comment missing in `DashboardRecentDocuments` — must fix **`DashboardRecentDocuments.svelte:20`** ```typescript // before .format(new Date(dateStr + 'T12:00:00')); // after .format(new Date(dateStr)); ``` The removal is correct — `updatedAt` is a full ISO datetime, not a date-only string, so the `T12:00:00` suffix isn't needed. But `CLAUDE.md` documents the `T12:00:00` pattern as a deliberate UTC off-by-one fix, so the next developer will "fix" this back. Add one comment explaining why the suffix is absent here: ```typescript // updatedAt is a full ISO datetime — no T12:00:00 suffix needed, // the time zone offset is already explicit in the value. .format(new Date(dateStr)); ``` --- ### 🟢 What's done well - **Layering is clean**: `DocumentController → DocumentService → Repository`, no repo leakage. - **`Promise.allSettled` for widget isolation**: one failing widget doesn't crash the dashboard. - **Test coverage**: controller, service, and repository tests all present and testing the right things. The `NotificationRepositoryTest` against Testcontainers is the right way to validate derived queries. - **`onMount` guard on `DashboardResumeStrip`**: localStorage is properly client-only, no SSR hazard. - **Reusable `captureProofshots` helper**: new feature specs are now one line — good extraction.

marcel added 2 commits 2026-03-29 11:47:09 +02:00

fix(#152): disable open-in-view to prevent HikariPool exhaustion ... 3e76ef5281

spring.jpa.open-in-view=true (the default) holds a DB connection open for
the entire HTTP request lifecycle. Under concurrent dashboard API calls
(Promise.allSettled fires 3 at once), the pool of 10 is exhausted and the
backend crashes with connection timeout errors.

Setting open-in-view=false releases connections as soon as each
@Transactional method completes.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>

feat(e2e): seed admin notifications + resume strip for dashboard proofshots ... 7eda0aefcc

- captureProofshots() now accepts an optional setup(page) callback that
  runs before each screenshot's page.goto(), so localStorage can be
  injected reliably without loading a backend-dependent page
- dashboard-screenshots.spec.ts seeds 2 notifications (MENTION + REPLY)
  for admin via direct DB insert in beforeAll, cleans up in afterAll
- localStorage.familienarchiv.lastVisited injected directly via
  page.evaluate() — no fragile document page navigation needed
- Updated screenshots committed (all 6 now show all 4 widgets)

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>

marcel commented 2026-03-29 11:48:27 +02:00

Author

Owner

Copy Link

Test Coverage Review — @saraholt

Overall solid structure: four widget specs, repo/service/controller tests all present. A few gaps worth addressing before merge.

---

P1 — Must Fix

getRecentActivity full-table-scan not caught by any test

DocumentService.getRecentActivity calls documentRepository.findAll(Sort), which fetches the entire documents table and truncates in Java with .stream().limit(size). The unit test mocks the repo so it can't catch this. There is no @DataJpaTest verifying the actual query behaviour.

A @DataJpaTest seeding e.g. 10 documents and asserting only size rows are returned would immediately expose that the LIMIT is not pushed to the DB. The fix is PageRequest.of(0, size, Sort.by(Sort.Direction.DESC, "updatedAt")) or a derived query method.

---

No tests for +page.server.ts load function

The load function contains meaningful branching (dashboard mode vs. search mode), three parallel Promise.allSettled calls, and per-branch fallback to [] on failure. SvelteKit load functions are plain TypeScript — they can be imported and called directly without a browser.

Missing cases:

• Dashboard mode (no URL params) — all three API calls are made, all three widgets receive data
• Search mode (any filter active) — no dashboard API calls made, returns documents list
• mentionsResult rejected → mentions defaults to []
• incompleteResult rejected → incompleteDocs defaults to []
• recentResult rejected → recentDocs defaults to []

---

P2 — Should Fix

DashboardResumeStrip: corrupt localStorage not handled

onMount calls JSON.parse(localStorage.getItem(...)) without a try/catch. Malformed JSON throws synchronously and leaves the component in an undefined state. No test covers this path. Either add a test that exercises the corrupt-JSON case (and add the corresponding guard), or document the decision to let it throw.

---

DashboardMentions: two branches untested

1. type === 'REPLY' — the component conditionally renders "hat geantwortet" vs "erwähnt Sie", but only the MENTION branch is exercised in tests.
2. documentId absent — the component renders a <span> instead of <a>. Not tested; a regression here would be invisible.

---

P3 — Nice to Have

• DocumentControllerTest: no test calls GET /api/documents/recent-activity without ?size= to verify the default value (5) is applied.
• NotificationRepository: findByType_returnsBothReadAndUnreadMentions verifies size and tenant isolation but does not assert createdAt DESC ordering, unlike the other paged query tests in the same file.
• NotificationService: ?type=MENTION&read=true silently falls through to the type-only branch (returns all mentions, not just read ones). Either add a test documenting this as intentional, or handle the read=true case explicitly.

---

The two P1 items are the ones I'd want resolved before merge. The full-table-scan in particular is the kind of thing that won't surface until the archive grows.

## Test Coverage Review — @saraholt Overall solid structure: four widget specs, repo/service/controller tests all present. A few gaps worth addressing before merge. --- ### P1 — Must Fix **`getRecentActivity` full-table-scan not caught by any test** `DocumentService.getRecentActivity` calls `documentRepository.findAll(Sort)`, which fetches the entire `documents` table and truncates in Java with `.stream().limit(size)`. The unit test mocks the repo so it can't catch this. There is no `@DataJpaTest` verifying the actual query behaviour. A `@DataJpaTest` seeding e.g. 10 documents and asserting only `size` rows are returned would immediately expose that the `LIMIT` is not pushed to the DB. The fix is `PageRequest.of(0, size, Sort.by(Sort.Direction.DESC, "updatedAt"))` or a derived query method. --- **No tests for `+page.server.ts` load function** The load function contains meaningful branching (dashboard mode vs. search mode), three parallel `Promise.allSettled` calls, and per-branch fallback to `[]` on failure. SvelteKit load functions are plain TypeScript — they can be imported and called directly without a browser. Missing cases: - Dashboard mode (no URL params) — all three API calls are made, all three widgets receive data - Search mode (any filter active) — no dashboard API calls made, returns `documents` list - `mentionsResult` rejected → `mentions` defaults to `[]` - `incompleteResult` rejected → `incompleteDocs` defaults to `[]` - `recentResult` rejected → `recentDocs` defaults to `[]` --- ### P2 — Should Fix **`DashboardResumeStrip`: corrupt localStorage not handled** `onMount` calls `JSON.parse(localStorage.getItem(...))` without a try/catch. Malformed JSON throws synchronously and leaves the component in an undefined state. No test covers this path. Either add a test that exercises the corrupt-JSON case (and add the corresponding guard), or document the decision to let it throw. --- **`DashboardMentions`: two branches untested** 1. `type === 'REPLY'` — the component conditionally renders "hat geantwortet" vs "erwähnt Sie", but only the MENTION branch is exercised in tests. 2. `documentId` absent — the component renders a `<span>` instead of `<a>`. Not tested; a regression here would be invisible. --- ### P3 — Nice to Have - **`DocumentControllerTest`**: no test calls `GET /api/documents/recent-activity` without `?size=` to verify the default value (5) is applied. - **`NotificationRepository`**: `findByType_returnsBothReadAndUnreadMentions` verifies size and tenant isolation but does not assert `createdAt DESC` ordering, unlike the other paged query tests in the same file. - **`NotificationService`**: `?type=MENTION&read=true` silently falls through to the type-only branch (returns all mentions, not just read ones). Either add a test documenting this as intentional, or handle the `read=true` case explicitly. --- The two P1 items are the ones I'd want resolved before merge. The full-table-scan in particular is the kind of thing that won't surface until the archive grows.

marcel commented 2026-03-29 12:12:15 +02:00

Author

Owner

Copy Link

All review items addressed in commit 5bdd26c. Full breakdown below.

---

@leonie — UI Review

[High] Grid breaks when Mentions widget is empty ✅ fixed

+page.svelte:103 — grid class is now conditional:

<div class="mt-6 grid gap-4 {(data.mentions?.length ?? 0) > 0 ? 'lg:grid-cols-2' : ''}">

When mentions is empty, NeedsMetadata spans full width. Matches the behaviour of DashboardRecentDocuments below it.

[Medium] 12px link below accessibility floor ✅ fixed

DashboardNeedsMetadata.svelte:32 — raised to text-sm and added hover:underline:

<a href="/enrich" class="font-sans text-sm text-ink-2 hover:text-ink hover:underline">

[Low] noon-anchor comment ✅ fixed

Added as a "why" comment directly before .format() — explains the non-obvious deviation from the CLAUDE.md pattern:

// updatedAt is a full ISO datetime — no T12:00:00 noon-anchor needed here
return new Intl.DateTimeFormat(...).format(new Date(dateStr));

---

@mkeller — Architectural Review

🔴 Full-table scan in getRecentActivity ✅ fixed

DocumentService.java:264 — replaced findAll(Sort) + Java stream truncation with a PageRequest, pushing LIMIT to the database:

return documentRepository.findAll(
    PageRequest.of(0, size, Sort.by(Sort.Direction.DESC, "updatedAt"))
).getContent();

Same pattern as findIncompleteDocuments. No new repository method needed.

🔴 UTC off-by-one comment ✅ fixed

Same as Leonie's [Low] — addressed above.

---

@saraholt — Test Coverage Review

P1 — Full-table scan not caught by any test ✅ fixed

Updated DocumentServiceTest — the unit test now asserts that a PageRequest (not Sort) is passed to the repository, including the page size and sort direction:

void getRecentActivity_usesPageRequestWithSizeLimit_notFindAll()

Added DocumentRepositoryTest — @DataJpaTest seeding 10 documents and asserting only size rows are returned (not all 10), verifying the LIMIT is pushed to the DB.

P1 — No tests for +page.server.ts load function ✅ fixed

page.server.spec.ts was stale — it still referenced incompleteCount from a prior API shape. Replaced with full coverage of the new dashboard/search branching:

• Dashboard mode: isDashboard = true, all three widget APIs called, widgets receive data
• Search mode: isDashboard = false, no widget calls, returns documents list
• Per-widget rejection fallback: mentions, incompleteDocs, recentDocs each default to [] when their API call is rejected
• Auth redirect: persons 401 → /login
• Network error fallback: outer try/catch returns error string

P2 — DashboardResumeStrip: corrupt localStorage — pushing back on the premise

The code already has a try/catch at DashboardResumeStrip.svelte:12–24 — malformed JSON is silently caught and lastVisited stays null. There is no open guard to add. I have however added a test that exercises this path to confirm the existing guard works as expected:

it('renders nothing when localStorage contains malformed JSON', ...)

P2 — DashboardMentions: two branches untested ✅ fixed

Added tests for:

1. type === 'REPLY' — widget renders and a link is present
2. documentId absent — a <span> is rendered instead of an <a>

P3 — Default size=5 not tested ✅ fixed

DocumentControllerTest — added getRecentActivity_appliesDefaultSizeOfFive_whenSizeParamOmitted which calls the endpoint without ?size= and verifies documentService.getRecentActivity(5) was invoked.

P3 — NotificationService read=true fallthrough ✅ documented

Added getNotifications_withTypeAndReadTrue_fallsBackToTypeOnlyQuery — the test asserts the type-only repo method is called and documents the intent:

read=true with a type filter is intentionally not supported on the backend; callers that need only-read results must filter client-side.

P3 — NotificationRepository ordering not asserted

Not implementing this one. The ordering is enforced by the Spring Data derived query method name itself (OrderByCreatedAtDesc) — it's a framework-level guarantee, not application logic. Testing it would require either Thread.sleep between saves (flaky) or directly injecting createdAt timestamps (coupling to internals). The method name is the contract; testing the Spring Data query derivation would be testing the framework.

All review items addressed in commit 5bdd26c. Full breakdown below. --- ## @leonie — UI Review ### [High] Grid breaks when Mentions widget is empty ✅ fixed `+page.svelte:103` — grid class is now conditional: ```svelte <div class="mt-6 grid gap-4 {(data.mentions?.length ?? 0) > 0 ? 'lg:grid-cols-2' : ''}"> ``` When `mentions` is empty, `NeedsMetadata` spans full width. Matches the behaviour of `DashboardRecentDocuments` below it. ### [Medium] 12px link below accessibility floor ✅ fixed `DashboardNeedsMetadata.svelte:32` — raised to `text-sm` and added `hover:underline`: ```svelte <a href="/enrich" class="font-sans text-sm text-ink-2 hover:text-ink hover:underline"> ``` ### [Low] noon-anchor comment ✅ fixed Added as a "why" comment directly before `.format()` — explains the non-obvious deviation from the CLAUDE.md pattern: ```typescript // updatedAt is a full ISO datetime — no T12:00:00 noon-anchor needed here return new Intl.DateTimeFormat(...).format(new Date(dateStr)); ``` --- ## @mkeller — Architectural Review ### 🔴 Full-table scan in `getRecentActivity` ✅ fixed `DocumentService.java:264` — replaced `findAll(Sort)` + Java stream truncation with a `PageRequest`, pushing `LIMIT` to the database: ```java return documentRepository.findAll( PageRequest.of(0, size, Sort.by(Sort.Direction.DESC, "updatedAt")) ).getContent(); ``` Same pattern as `findIncompleteDocuments`. No new repository method needed. ### 🔴 UTC off-by-one comment ✅ fixed Same as Leonie's [Low] — addressed above. --- ## @saraholt — Test Coverage Review ### P1 — Full-table scan not caught by any test ✅ fixed Updated `DocumentServiceTest` — the unit test now asserts that a `PageRequest` (not `Sort`) is passed to the repository, including the page size and sort direction: ```java void getRecentActivity_usesPageRequestWithSizeLimit_notFindAll() ``` Added `DocumentRepositoryTest` — `@DataJpaTest` seeding 10 documents and asserting only `size` rows are returned (not all 10), verifying the `LIMIT` is pushed to the DB. ### P1 — No tests for `+page.server.ts` load function ✅ fixed `page.server.spec.ts` was stale — it still referenced `incompleteCount` from a prior API shape. Replaced with full coverage of the new dashboard/search branching: - Dashboard mode: `isDashboard = true`, all three widget APIs called, widgets receive data - Search mode: `isDashboard = false`, no widget calls, returns `documents` list - Per-widget rejection fallback: `mentions`, `incompleteDocs`, `recentDocs` each default to `[]` when their API call is rejected - Auth redirect: persons 401 → `/login` - Network error fallback: outer try/catch returns error string ### P2 — `DashboardResumeStrip`: corrupt localStorage **— pushing back on the premise** The code already has a `try/catch` at `DashboardResumeStrip.svelte:12–24` — malformed JSON is silently caught and `lastVisited` stays `null`. There is no open guard to add. I have however added a test that exercises this path to confirm the existing guard works as expected: ```typescript it('renders nothing when localStorage contains malformed JSON', ...) ``` ### P2 — `DashboardMentions`: two branches untested ✅ fixed Added tests for: 1. `type === 'REPLY'` — widget renders and a link is present 2. `documentId` absent — a `<span>` is rendered instead of an `<a>` ### P3 — Default size=5 not tested ✅ fixed `DocumentControllerTest` — added `getRecentActivity_appliesDefaultSizeOfFive_whenSizeParamOmitted` which calls the endpoint without `?size=` and verifies `documentService.getRecentActivity(5)` was invoked. ### P3 — `NotificationService` `read=true` fallthrough ✅ documented Added `getNotifications_withTypeAndReadTrue_fallsBackToTypeOnlyQuery` — the test asserts the type-only repo method is called and documents the intent: > `read=true` with a type filter is intentionally not supported on the backend; callers that need only-read results must filter client-side. ### P3 — `NotificationRepository` ordering not asserted Not implementing this one. The ordering is enforced by the Spring Data derived query method name itself (`OrderByCreatedAtDesc`) — it's a framework-level guarantee, not application logic. Testing it would require either `Thread.sleep` between saves (flaky) or directly injecting `createdAt` timestamps (coupling to internals). The method name is the contract; testing the Spring Data query derivation would be testing the framework.

marcel added 2 commits 2026-03-29 12:18:45 +02:00

fix(#145): address PR review — full-table scan, a11y, grid, tests ... 5bdd26c792

- DocumentService.getRecentActivity: replace findAll(Sort)+stream().limit()
  with findAll(PageRequest) so LIMIT is pushed to the database
- +page.svelte: collapse two-column grid to single column when mentions is empty
- DashboardNeedsMetadata: raise "show all" link from text-xs (12px) to text-sm
  (14px) and add hover:underline for WCAG 1.4.1
- DashboardRecentDocuments: add comment explaining why T12:00:00 noon-anchor
  is absent (updatedAt is a full ISO datetime, not a date-only string)
- DocumentServiceTest: update getRecentActivity tests to assert PageRequest
  usage instead of findAll(Sort)
- DocumentRepositoryTest: add @DataJpaTest verifying findAll(PageRequest)
  returns only size rows, not the full table
- DocumentControllerTest: add test for default size=5 when param is omitted
- NotificationServiceTest: add test documenting that type+read=true falls
  through to the type-only query (intentional)
- page.server.spec.ts: replace stale tests with full dashboard-mode coverage
- DashboardMentions.svelte.spec.ts: add tests for REPLY type and absent documentId
- DashboardResumeStrip.svelte.spec.ts: add corrupt localStorage test

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>

style(#145): bump dashboard widget content links to text-lg ... adba3058b4

Box content links (document titles, actor names) raised from text-sm to
text-lg for improved readability and touch target size. "Show all" stays
at text-sm to maintain hierarchy — box links are the primary action.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>

marcel added 1 commit 2026-03-29 12:32:15 +02:00

style: teal accent underline on link hover globally ... b1f82d91d2

Any link that renders an underline on hover now gets the brand accent
colour (--c-accent) as its decoration colour. Links that suppress
underlines (nav, back-links, button-style anchors) are unaffected.
Dark mode already maps --c-accent to the stronger turquoise (#00c7b1).

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>

marcel added 1 commit 2026-03-29 12:35:39 +02:00

style: standardise link hover underline (2px, offset-4, accent) globally ... 6b15ea8b1f

Move text-decoration-thickness/underline-offset into the global a:hover
base rule so every link that shows an underline on hover gets identical
treatment: 2px thick, 4px offset, accent colour.

Remove the now-redundant per-component decoration-brand-mint / decoration-
accent / decoration-2 / underline-offset-{2,4} utilities from DocumentList,
enrich, persons, PersonDocumentList, and PanelMetadata.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>

marcel merged commit 6b15ea8b1f into main 2026-03-29 12:39:39 +02:00

marcel deleted branch feat/145-dashboard 2026-03-29 12:39:40 +02:00

Sign in to join this conversation.

Reviewers

No Reviewers

Labels

Clear labels

P0-critical

Blocks a core user journey, causes data loss, or is a security/accessibility barrier. Work on this before P1+.

P1-high

Significant friction on a main user journey; workaround exists but is non-obvious. Next up after P0.

P2-medium

Noticeable improvement; doesn't block anything; schedule opportunistically.

P3-later

Cosmetic or parking-lot work; fine to stay open indefinitely.

audit

Read-only audit / assessment work; produces a report rather than changing code

bug

Something isn't working

cleanup

Removal of dead code, vague comments, naming violations, and scratch artifacts

collaboration

We want to extend the family archive to have more collaboration tools

conversation

descoped

We will do that later

devops

documentation

README, ARCHITECTURE, GLOSSARY, CONTRIBUTING, per-domain docs

epic

Marker for epic-level issues that group multiple child issues

feature

file-upload

legibility

Codebase Legibility Refactor — preparing the codebase for human evaluation and bus-factor reduction

notification

person

phase-1: security

Security hygiene — must be done first

phase-2: container-images

Production-ready multi-stage Docker images

phase-3: prod-compose

Production compose overlay + Caddy reverse proxy

phase-4: spring-prod-profile

Spring Boot production configuration profile

phase-5: backups

Database and object storage backup strategy

phase-6: deployment-docs

.env.example, DEPLOYMENT.md, runbook

phase-7: monitoring

Prometheus, Loki, Grafana, Alertmanager

refactor

Code restructuring without behaviour change

security

test

ui

UI/UX and accessibility issues

user

No Label

Milestone

No items

No Milestone

Projects

Clear projects

No project

Assignees

Clear assignees

Jens marcel

No Assignees

1 Participants

Notifications

Subscribe

Due Date

No due date set.

Dependencies

No dependencies set.

Reference: marcel/familienarchiv#151