From 138bf446e487443b31df7931e925556b188da16e Mon Sep 17 00:00:00 2001 From: Marcel Date: Sun, 31 May 2026 12:12:33 +0200 Subject: [PATCH 1/8] feat(tag): add subtree document-count rollup to tag tree (#698) Add subtreeDocumentCount to TagTreeNodeDTO, populated by a new recursive-CTE aggregate query that builds a tag closure and counts distinct documents per ancestor subtree. The direct documentCount is unchanged; getTagTree now maps both counts onto each node from two aggregate queries (no N+1). Co-Authored-By: Claude Opus 4.8 --- .../familienarchiv/tag/TagRepository.java | 27 +++++++++++ .../familienarchiv/tag/TagService.java | 34 +++++++++----- .../familienarchiv/tag/TagTreeNodeDTO.java | 3 ++ .../familienarchiv/tag/TagControllerTest.java | 4 +- .../familienarchiv/tag/TagServiceTest.java | 47 +++++++++++++++++++ 5 files changed, 101 insertions(+), 14 deletions(-) diff --git a/backend/src/main/java/org/raddatz/familienarchiv/tag/TagRepository.java b/backend/src/main/java/org/raddatz/familienarchiv/tag/TagRepository.java index f1b3b7ab..108b4bc8 100644 --- a/backend/src/main/java/org/raddatz/familienarchiv/tag/TagRepository.java +++ b/backend/src/main/java/org/raddatz/familienarchiv/tag/TagRepository.java @@ -126,4 +126,31 @@ public interface TagRepository extends JpaRepository { */ @Query(value = "SELECT tag_id AS tagId, COUNT(*) AS count FROM document_tags GROUP BY tag_id", nativeQuery = true) List findDocumentCountsPerTag(); + + /** + * Returns (tagId, count) pairs where count is the number of distinct documents tagged + * with that tag or any of its descendants (full subtree rollup). + *

+ * Builds a tag closure of (ancestor_id, descendant_id) pairs via a recursive CTE — each tag is + * its own ancestor at depth 0, then descends into children (depth guard of 50 levels prevents a + * cycle or pathological depth from running away) — joins it to {@code document_tags} on the + * descendant, and counts distinct documents per ancestor. A document tagged with several tags in + * the same subtree is therefore counted once. Tags whose entire subtree holds no documents do + * not appear in the result (they default to 0 in the tree). One aggregate query for all tags. + */ + @Query(value = """ + WITH RECURSIVE closure AS ( + SELECT id AS ancestor_id, id AS descendant_id, 0 AS depth FROM tag + UNION ALL + SELECT c.ancestor_id, t.id AS descendant_id, c.depth + 1 + FROM tag t + JOIN closure c ON t.parent_id = c.descendant_id + WHERE c.depth < 50 + ) + SELECT c.ancestor_id AS tagId, COUNT(DISTINCT dt.document_id) AS count + FROM closure c + JOIN document_tags dt ON dt.tag_id = c.descendant_id + GROUP BY c.ancestor_id + """, nativeQuery = true) + List findSubtreeDocumentCountsPerTag(); } diff --git a/backend/src/main/java/org/raddatz/familienarchiv/tag/TagService.java b/backend/src/main/java/org/raddatz/familienarchiv/tag/TagService.java index 14e1e9fa..1dac8610 100644 --- a/backend/src/main/java/org/raddatz/familienarchiv/tag/TagService.java +++ b/backend/src/main/java/org/raddatz/familienarchiv/tag/TagService.java @@ -172,19 +172,27 @@ public class TagService { } /** - * Returns all tags assembled into a tree with document counts per node. - * Uses a single aggregate query to avoid N+1 behaviour. - * NOTE: document counts are global per tag, not scoped to any search filter. - * The tree endpoint is only used for the admin sidebar, so this is intentional. + * Returns all tags assembled into a tree, each node carrying two counts: + * {@code documentCount} — documents tagged with that exact tag (direct) — and + * {@code subtreeDocumentCount} — distinct documents tagged with that tag or any descendant + * (subtree rollup). Each count comes from one aggregate query (no N+1). + * NOTE: counts are global per tag, not scoped to any search filter. + * Consumed by the reader surfaces (/themen page, dashboard ThemenWidget — which read the + * subtree rollup) as well as the admin sidebar and tag operation previews (which read the + * direct count). */ public List getTagTree() { List all = tagRepository.findAll(); - Map counts = tagRepository.findDocumentCountsPerTag().stream() - .collect(Collectors.toMap( - TagRepository.TagCount::getTagId, - TagRepository.TagCount::getCount - )); - return buildTree(all, counts); + Map counts = toCountMap(tagRepository.findDocumentCountsPerTag()); + Map subtreeCounts = toCountMap(tagRepository.findSubtreeDocumentCountsPerTag()); + return buildTree(all, counts, subtreeCounts); + } + + private static Map toCountMap(List counts) { + return counts.stream().collect(Collectors.toMap( + TagRepository.TagCount::getTagId, + TagRepository.TagCount::getCount + )); } // ─── private helpers ───────────────────────────────────────────────────── @@ -259,12 +267,14 @@ public class TagService { } } - private List buildTree(List tags, Map counts) { + private List buildTree(List tags, Map counts, + Map subtreeCounts) { Map nodeById = new LinkedHashMap<>(); for (Tag tag : tags) { int documentCount = counts.getOrDefault(tag.getId(), 0L).intValue(); + int subtreeDocumentCount = subtreeCounts.getOrDefault(tag.getId(), 0L).intValue(); nodeById.put(tag.getId(), new TagTreeNodeDTO( - tag.getId(), tag.getName(), tag.getColor(), documentCount, + tag.getId(), tag.getName(), tag.getColor(), documentCount, subtreeDocumentCount, new ArrayList<>(), tag.getParentId() )); } diff --git a/backend/src/main/java/org/raddatz/familienarchiv/tag/TagTreeNodeDTO.java b/backend/src/main/java/org/raddatz/familienarchiv/tag/TagTreeNodeDTO.java index 2b8a60db..3399da04 100644 --- a/backend/src/main/java/org/raddatz/familienarchiv/tag/TagTreeNodeDTO.java +++ b/backend/src/main/java/org/raddatz/familienarchiv/tag/TagTreeNodeDTO.java @@ -10,5 +10,8 @@ public record TagTreeNodeDTO( @Schema(requiredMode = Schema.RequiredMode.REQUIRED) String name, String color, @Schema(requiredMode = Schema.RequiredMode.REQUIRED) int documentCount, + @Schema(requiredMode = Schema.RequiredMode.REQUIRED, + description = "Distinct documents tagged with this tag or any descendant tag (subtree rollup)") + int subtreeDocumentCount, List children, @Schema(description = "Parent tag ID, null for root tags") UUID parentId) {} diff --git a/backend/src/test/java/org/raddatz/familienarchiv/tag/TagControllerTest.java b/backend/src/test/java/org/raddatz/familienarchiv/tag/TagControllerTest.java index 1504f1fa..4d2ccae9 100644 --- a/backend/src/test/java/org/raddatz/familienarchiv/tag/TagControllerTest.java +++ b/backend/src/test/java/org/raddatz/familienarchiv/tag/TagControllerTest.java @@ -102,8 +102,8 @@ class TagControllerTest { void getTagTree_returns200_withTreeStructure() throws Exception { UUID parentId = UUID.randomUUID(); UUID childId = UUID.randomUUID(); - TagTreeNodeDTO child = new TagTreeNodeDTO(childId, "Haus", null, 0, List.of(), parentId); - TagTreeNodeDTO parent = new TagTreeNodeDTO(parentId, "Immobilie", "teal", 0, List.of(child), null); + TagTreeNodeDTO child = new TagTreeNodeDTO(childId, "Haus", null, 0, 0, List.of(), parentId); + TagTreeNodeDTO parent = new TagTreeNodeDTO(parentId, "Immobilie", "teal", 0, 0, List.of(child), null); when(tagService.getTagTree()).thenReturn(List.of(parent)); mockMvc.perform(get("/api/tags/tree")) diff --git a/backend/src/test/java/org/raddatz/familienarchiv/tag/TagServiceTest.java b/backend/src/test/java/org/raddatz/familienarchiv/tag/TagServiceTest.java index 486f4afe..e9e06a70 100644 --- a/backend/src/test/java/org/raddatz/familienarchiv/tag/TagServiceTest.java +++ b/backend/src/test/java/org/raddatz/familienarchiv/tag/TagServiceTest.java @@ -278,6 +278,53 @@ class TagServiceTest { verify(tagRepository, times(1)).findDocumentCountsPerTag(); } + @Test + void getTagTree_populatesSubtreeDocumentCount_fromRollupQuery() { + UUID tagId = UUID.randomUUID(); + Tag tag = Tag.builder().id(tagId).name("Reisen").build(); + TagRepository.TagCount subtreeEntry = mock(TagRepository.TagCount.class); + when(subtreeEntry.getTagId()).thenReturn(tagId); + when(subtreeEntry.getCount()).thenReturn(7L); + when(tagRepository.findAll()).thenReturn(List.of(tag)); + when(tagRepository.findDocumentCountsPerTag()).thenReturn(List.of()); + when(tagRepository.findSubtreeDocumentCountsPerTag()).thenReturn(List.of(subtreeEntry)); + + var tree = tagService.getTagTree(); + + assertThat(tree.get(0).subtreeDocumentCount()).isEqualTo(7); + } + + @Test + void getTagTree_keepsDirectAndSubtreeCountsIndependent() { + UUID tagId = UUID.randomUUID(); + Tag tag = Tag.builder().id(tagId).name("Reisen").build(); + TagRepository.TagCount directEntry = mock(TagRepository.TagCount.class); + when(directEntry.getTagId()).thenReturn(tagId); + when(directEntry.getCount()).thenReturn(2L); + TagRepository.TagCount subtreeEntry = mock(TagRepository.TagCount.class); + when(subtreeEntry.getTagId()).thenReturn(tagId); + when(subtreeEntry.getCount()).thenReturn(7L); + when(tagRepository.findAll()).thenReturn(List.of(tag)); + when(tagRepository.findDocumentCountsPerTag()).thenReturn(List.of(directEntry)); + when(tagRepository.findSubtreeDocumentCountsPerTag()).thenReturn(List.of(subtreeEntry)); + + var node = tagService.getTagTree().get(0); + + assertThat(node.documentCount()).isEqualTo(2); + assertThat(node.subtreeDocumentCount()).isEqualTo(7); + } + + @Test + void getTagTree_callsFindSubtreeDocumentCountsPerTag_exactlyOnce() { + when(tagRepository.findAll()).thenReturn(List.of()); + when(tagRepository.findDocumentCountsPerTag()).thenReturn(List.of()); + when(tagRepository.findSubtreeDocumentCountsPerTag()).thenReturn(List.of()); + + tagService.getTagTree(); + + verify(tagRepository, times(1)).findSubtreeDocumentCountsPerTag(); + } + // ─── resolveEffectiveColors ─────────────────────────────────────────────── @Test -- 2.49.1 From 2f1538754e9c3d25ddbd8a37c7b5b767334bbfd4 Mon Sep 17 00:00:00 2001 From: Marcel Date: Sun, 31 May 2026 12:15:15 +0200 Subject: [PATCH 2/8] test(tag): validate subtree rollup CTE against real Postgres (#698) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Cover AC#1-4 (leaf=direct, distinct overlap counted once, full descendant depth), REQ-THEMEN-05 (empty subtree absent), REQ-THEMEN-06 (cycle terminates via the 50-level guard) and AC#7 (rollup equals distinct documents found by the real tag-search expansion — count↔destination parity). Testcontainers postgres:16-alpine since the recursive CTE + COUNT(DISTINCT) needs real PG. Co-Authored-By: Claude Opus 4.8 --- .../TagRollupRepositoryIntegrationTest.java | 179 ++++++++++++++++++ 1 file changed, 179 insertions(+) create mode 100644 backend/src/test/java/org/raddatz/familienarchiv/tag/TagRollupRepositoryIntegrationTest.java diff --git a/backend/src/test/java/org/raddatz/familienarchiv/tag/TagRollupRepositoryIntegrationTest.java b/backend/src/test/java/org/raddatz/familienarchiv/tag/TagRollupRepositoryIntegrationTest.java new file mode 100644 index 00000000..5fd150aa --- /dev/null +++ b/backend/src/test/java/org/raddatz/familienarchiv/tag/TagRollupRepositoryIntegrationTest.java @@ -0,0 +1,179 @@ +package org.raddatz.familienarchiv.tag; + +import jakarta.persistence.EntityManager; +import org.junit.jupiter.api.Test; +import org.raddatz.familienarchiv.PostgresContainerConfig; +import org.raddatz.familienarchiv.config.FlywayConfig; +import org.raddatz.familienarchiv.document.Document; +import org.raddatz.familienarchiv.document.DocumentRepository; +import org.raddatz.familienarchiv.document.DocumentSpecifications; +import org.raddatz.familienarchiv.document.DocumentStatus; +import org.springframework.beans.factory.annotation.Autowired; +import org.springframework.boot.data.jpa.test.autoconfigure.DataJpaTest; +import org.springframework.boot.jdbc.test.autoconfigure.AutoConfigureTestDatabase; +import org.springframework.context.annotation.Import; + +import java.util.HashSet; +import java.util.List; +import java.util.Map; +import java.util.Set; +import java.util.UUID; +import java.util.stream.Collectors; + +import static org.assertj.core.api.Assertions.assertThat; +import static org.assertj.core.api.Assertions.assertThatCode; + +/** + * Real-Postgres validation of the subtree document-count rollup ({@link TagRepository + * #findSubtreeDocumentCountsPerTag}). The recursive CTE + COUNT(DISTINCT) cannot be exercised on + * H2, so these run against {@code postgres:16-alpine} via Testcontainers. Covers issue #698 + * AC#1–#4, #6 (REQ-THEMEN-06 cycle guard) and #7 (count↔destination parity). + */ +@DataJpaTest +@AutoConfigureTestDatabase(replace = AutoConfigureTestDatabase.Replace.NONE) +@Import({PostgresContainerConfig.class, FlywayConfig.class}) +class TagRollupRepositoryIntegrationTest { + + @Autowired private TagRepository tagRepository; + @Autowired private DocumentRepository documentRepository; + @Autowired private EntityManager entityManager; + + // ─── helpers ────────────────────────────────────────────────────────────── + + private Tag tag(String name, UUID parentId) { + return tagRepository.save(Tag.builder().name(name).parentId(parentId).build()); + } + + private Document docWithTags(String title, Tag... tags) { + return documentRepository.save(Document.builder() + .title(title) + .originalFilename(title + ".pdf") + .status(DocumentStatus.UPLOADED) + .tags(new HashSet<>(Set.of(tags))) + .build()); + } + + private Map rollup() { + entityManager.flush(); + entityManager.clear(); + return tagRepository.findSubtreeDocumentCountsPerTag().stream() + .collect(Collectors.toMap(TagRepository.TagCount::getTagId, TagRepository.TagCount::getCount)); + } + + // ─── AC#4 — rollup of a leaf equals its direct count ──────────────────────── + + @Test + void leafTag_subtreeCount_equalsItsDirectCount() { + Tag leaf = tag("Tagebuch", null); + docWithTags("a", leaf); + docWithTags("b", leaf); + docWithTags("c", leaf); + + assertThat(rollup().get(leaf.getId())).isEqualTo(3L); + } + + // ─── AC#1 + AC#2 — parent rolls up children, distinct (shared doc counted once) ── + + @Test + void parentTag_rollsUpChildDocuments_countingSharedDocumentOnce() { + Tag reisen = tag("Reisen", null); + Tag italien = tag("Italien", reisen.getId()); + + Document shared = docWithTags("shared", reisen, italien); // tagged with both + docWithTags("reisenOnly", reisen); + docWithTags("it1", italien); + docWithTags("it2", italien); + docWithTags("it3", italien); + docWithTags("it4", italien); + + Map rollup = rollup(); + + // Reisen direct {shared, reisenOnly} = 2; Italien {shared, it1..it4} = 5; union distinct = 6 + assertThat(rollup.get(reisen.getId())).isEqualTo(6L); + assertThat(rollup.get(italien.getId())).isEqualTo(5L); + assertThat(shared.getId()).isNotNull(); + } + + // ─── AC#3 — full descendant depth (grandchildren included) ────────────────── + + @Test + void rollup_includesGrandchildDocuments_atFullDepth() { + Tag reisen = tag("Reisen", null); + Tag italien = tag("Italien", reisen.getId()); + Tag rom = tag("Rom", italien.getId()); + + docWithTags("r1", reisen); + docWithTags("i1", italien); + docWithTags("rom1", rom); + docWithTags("rom2", rom); + docWithTags("rom3", rom); + + Map rollup = rollup(); + + assertThat(rollup.get(reisen.getId())).isEqualTo(5L); // 1 + 1 + 3, all distinct + assertThat(rollup.get(italien.getId())).isEqualTo(4L); // 1 + 3 + assertThat(rollup.get(rom.getId())).isEqualTo(3L); + } + + // ─── REQ-THEMEN-05 — a tag whose whole subtree is empty is absent (→ 0) ───── + + @Test + void tagWithEmptySubtree_isAbsentFromRollup() { + Tag empty = tag("Leer", null); + Tag emptyChild = tag("LeerKind", empty.getId()); + + Map rollup = rollup(); + + assertThat(rollup).doesNotContainKey(empty.getId()); + assertThat(rollup).doesNotContainKey(emptyChild.getId()); + } + + // ─── REQ-THEMEN-06 — a hierarchy cycle terminates safely via the depth guard ── + + @Test + void rollup_terminatesSafely_whenHierarchyContainsCycle() { + Tag a = tag("CycleA", null); + Tag b = tag("CycleB", a.getId()); + // Close the loop: A.parent = B (DB only forbids parent_id == id, so a 2-node cycle is insertable) + a.setParentId(b.getId()); + tagRepository.save(a); + + docWithTags("ca", a); + docWithTags("cb", b); + + assertThatCode(this::rollup).doesNotThrowAnyException(); // depth guard prevents a runaway recursion + Map rollup = rollup(); + + // COUNT(DISTINCT document_id) dedupes documents reached via repeated cycle paths + assertThat(rollup.get(a.getId())).isEqualTo(2L); + assertThat(rollup.get(b.getId())).isEqualTo(2L); + } + + // ─── AC#7 — count↔destination parity with the real search expansion ───────── + + @Test + void subtreeCount_equalsDistinctDocumentsFoundByTagSearch_parity() { + // Uniquely-named root so name-based search expansion lines up with the per-id rollup. + Tag root = tag("ZzzParitaetReise", null); + Tag child = tag("ZzzParitaetItalien", root.getId()); + Tag grandchild = tag("ZzzParitaetRom", child.getId()); + + docWithTags("p_shared", root, child); // overlap inside the subtree + docWithTags("p_root", root); + docWithTags("p_child", child); + docWithTags("p_gc1", grandchild); + docWithTags("p_gc2", grandchild); + + entityManager.flush(); + entityManager.clear(); + + long rollupCount = rollup().get(root.getId()); + + List searchExpansionIds = tagRepository.findDescendantIdsByName("ZzzParitaetReise"); + var spec = DocumentSpecifications.hasTags(List.of(new HashSet<>(searchExpansionIds)), true); + long distinctSearchResults = documentRepository.findAll(spec).stream() + .map(Document::getId).distinct().count(); + + assertThat(rollupCount).isEqualTo(distinctSearchResults); + } +} -- 2.49.1 From 5ea47d4ec7f9116cc4d90ea1cb4d372a871a56f6 Mon Sep 17 00:00:00 2001 From: Marcel Date: Sun, 31 May 2026 12:15:52 +0200 Subject: [PATCH 3/8] docs(tag): document the dual document counts on the tag tree (#698) Record that getTagTree returns both documentCount (direct, read by admin surfaces) and subtreeDocumentCount (rollup, read by the reader surfaces), matching the corrected getTagTree JavaDoc. Co-Authored-By: Claude Opus 4.8 --- .../src/main/java/org/raddatz/familienarchiv/tag/README.md | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/backend/src/main/java/org/raddatz/familienarchiv/tag/README.md b/backend/src/main/java/org/raddatz/familienarchiv/tag/README.md index 4f34833e..f1d6d16e 100644 --- a/backend/src/main/java/org/raddatz/familienarchiv/tag/README.md +++ b/backend/src/main/java/org/raddatz/familienarchiv/tag/README.md @@ -7,6 +7,13 @@ Hierarchical document categories. Tags form a tree via a self-referencing `paren Entity: `Tag` (self-referencing `parent_id` tree). Features: tag CRUD, hierarchical deletion (cascade to descendants), tag typeahead, admin tag management (rename, reparent, merge). +## Tag tree counts (`getTagTree`) + +`GET /api/tags/tree` returns each node with **two** document counts, from two aggregate queries (no N+1): + +- `documentCount` — documents tagged with that **exact** tag (direct). Read by the admin surfaces (sidebar tree, merge preview, delete-impact guard), which describe direct-document operations. +- `subtreeDocumentCount` — **distinct** documents tagged with that tag **or any descendant** (subtree rollup, recursive-CTE closure, depth guard ≤50). Read by the reader surfaces (`/themen` page, dashboard `ThemenWidget`) so the box number matches what `/documents?tag=X` actually finds. + ## What this domain does NOT own - Documents — the `document_tags` join table is on the document side. `Tag` does not hold document references. -- 2.49.1 From 16f1fe761651b0be4057ce2cd0d4615572ebfd0c Mon Sep 17 00:00:00 2001 From: Marcel Date: Sun, 31 May 2026 12:17:40 +0200 Subject: [PATCH 4/8] feat(themen): key reader tag visibility on the subtree rollup (#698) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Regenerate the TagTreeNodeDTO type with subtreeDocumentCount and switch hasAnyDocuments to read it directly — the backend rollup already includes all descendants, so the recursive children walk is no longer needed. Reader surfaces now hide a topic only when its whole subtree is empty. Co-Authored-By: Claude Opus 4.8 --- frontend/src/lib/generated/api.ts | 5 ++++ .../src/lib/shared/utils/tagUtils.test.ts | 27 ++++++++++--------- frontend/src/lib/shared/utils/tagUtils.ts | 8 +++++- 3 files changed, 27 insertions(+), 13 deletions(-) diff --git a/frontend/src/lib/generated/api.ts b/frontend/src/lib/generated/api.ts index 5b059dd5..8f9e303e 100644 --- a/frontend/src/lib/generated/api.ts +++ b/frontend/src/lib/generated/api.ts @@ -2230,6 +2230,11 @@ export interface components { color?: string; /** Format: int32 */ documentCount: number; + /** + * Format: int32 + * @description Distinct documents tagged with this tag or any descendant tag (subtree rollup) + */ + subtreeDocumentCount: number; children?: components["schemas"]["TagTreeNodeDTO"][]; /** * Format: uuid diff --git a/frontend/src/lib/shared/utils/tagUtils.test.ts b/frontend/src/lib/shared/utils/tagUtils.test.ts index 5fe4a874..87b33763 100644 --- a/frontend/src/lib/shared/utils/tagUtils.test.ts +++ b/frontend/src/lib/shared/utils/tagUtils.test.ts @@ -4,26 +4,29 @@ import type { components } from '$lib/generated/api'; type TagTreeNodeDTO = components['schemas']['TagTreeNodeDTO']; -function makeNode(documentCount: number, children: TagTreeNodeDTO[] = []): TagTreeNodeDTO { - return { id: 'id', name: 'name', documentCount, children }; +function makeNode( + documentCount: number, + subtreeDocumentCount: number, + children: TagTreeNodeDTO[] = [] +): TagTreeNodeDTO { + return { id: 'id', name: 'name', documentCount, subtreeDocumentCount, children }; } describe('hasAnyDocuments', () => { - it('returns false for a leaf node with documentCount=0', () => { - expect(hasAnyDocuments(makeNode(0))).toBe(false); + it('returns false for a node whose subtree holds no documents', () => { + expect(hasAnyDocuments(makeNode(0, 0))).toBe(false); }); - it('returns true for a leaf node with documentCount=3', () => { - expect(hasAnyDocuments(makeNode(3))).toBe(true); + it('returns true for a node whose subtree holds documents', () => { + expect(hasAnyDocuments(makeNode(3, 3))).toBe(true); }); - it('returns true for a root with documentCount=0 but a child with documentCount=5', () => { - const node = makeNode(0, [makeNode(5)]); - expect(hasAnyDocuments(node)).toBe(true); + it('keys on the subtree rollup, not direct documentCount: 0 direct but rollup 5 → true', () => { + // The rollup already includes descendants — a single field read, no recursion over children. + expect(hasAnyDocuments(makeNode(0, 5))).toBe(true); }); - it('returns false for a root with documentCount=0 and all children also 0', () => { - const node = makeNode(0, [makeNode(0), makeNode(0)]); - expect(hasAnyDocuments(node)).toBe(false); + it('keys on the subtree rollup, not direct documentCount: 5 direct but rollup 0 → false', () => { + expect(hasAnyDocuments(makeNode(5, 0))).toBe(false); }); }); diff --git a/frontend/src/lib/shared/utils/tagUtils.ts b/frontend/src/lib/shared/utils/tagUtils.ts index a7fe138d..89826e68 100644 --- a/frontend/src/lib/shared/utils/tagUtils.ts +++ b/frontend/src/lib/shared/utils/tagUtils.ts @@ -2,6 +2,12 @@ import type { components } from '$lib/generated/api'; type TagTreeNodeDTO = components['schemas']['TagTreeNodeDTO']; +/** + * Whether a tag's whole subtree holds any documents — keyed on the subtree rollup + * (`subtreeDocumentCount`), which the backend already computes across all descendants. + * Used by the reader surfaces (/themen page, dashboard ThemenWidget) to hide empty topics. + * A single field read: no recursion needed, the rollup is authoritative. + */ export function hasAnyDocuments(node: TagTreeNodeDTO): boolean { - return (node.documentCount ?? 0) > 0 || (node.children ?? []).some(hasAnyDocuments); + return (node.subtreeDocumentCount ?? 0) > 0; } -- 2.49.1 From 29ea27319a381a61ebfe103842df04b0ee492bb5 Mon Sep 17 00:00:00 2001 From: Marcel Date: Sun, 31 May 2026 12:25:52 +0200 Subject: [PATCH 5/8] feat(themen): show the subtree rollup count on reader surfaces (#698) The /themen page (box header, child rows, aria-labels) and the dashboard ThemenWidget now display subtreeDocumentCount instead of the direct documentCount, so a topic's number reflects its whole sub-topic tree and matches what /documents?tag=X actually returns. A parent with 0 direct documents but documents under its children now shows a non-zero total. Co-Authored-By: Claude Opus 4.8 --- .../src/lib/shared/dashboard/ThemenWidget.svelte | 8 ++++---- .../shared/dashboard/ThemenWidget.svelte.spec.ts | 13 +++++++++++-- frontend/src/routes/themen/+page.svelte | 8 ++++---- frontend/src/routes/themen/page.svelte.spec.ts | 13 +++++++++++-- 4 files changed, 30 insertions(+), 12 deletions(-) diff --git a/frontend/src/lib/shared/dashboard/ThemenWidget.svelte b/frontend/src/lib/shared/dashboard/ThemenWidget.svelte index cef35418..1e8cbd20 100644 --- a/frontend/src/lib/shared/dashboard/ThemenWidget.svelte +++ b/frontend/src/lib/shared/dashboard/ThemenWidget.svelte @@ -41,8 +41,8 @@ const shownTags = $derived(visibleTags.slice(0, MAX_VISIBLE_TAGS)); {#each shownTags as tag (tag.id)} 0 + ? ', ' + m.themen_dokumente({ count: tag.subtreeDocumentCount }) : ''}" class="flex cursor-pointer items-stretch overflow-hidden rounded-sm border border-line bg-canvas hover:bg-surface focus-visible:ring-2 focus-visible:ring-brand-navy focus-visible:outline-none" style="min-height: 56px" @@ -54,9 +54,9 @@ const shownTags = $derived(visibleTags.slice(0, MAX_VISIBLE_TAGS)); > {tag.name} - {#if tag.documentCount > 0} + {#if tag.subtreeDocumentCount > 0} - {m.themen_dokumente({ count: tag.documentCount })} + {m.themen_dokumente({ count: tag.subtreeDocumentCount })} {/if} diff --git a/frontend/src/lib/shared/dashboard/ThemenWidget.svelte.spec.ts b/frontend/src/lib/shared/dashboard/ThemenWidget.svelte.spec.ts index 521ddeba..a8e1e552 100644 --- a/frontend/src/lib/shared/dashboard/ThemenWidget.svelte.spec.ts +++ b/frontend/src/lib/shared/dashboard/ThemenWidget.svelte.spec.ts @@ -12,9 +12,10 @@ type TagTreeNodeDTO = components['schemas']['TagTreeNodeDTO']; function makeTag( name: string, documentCount: number, - children: TagTreeNodeDTO[] = [] + children: TagTreeNodeDTO[] = [], + subtreeDocumentCount: number = documentCount ): TagTreeNodeDTO { - return { id: 'id-' + name, name, documentCount, children }; + return { id: 'id-' + name, name, documentCount, subtreeDocumentCount, children }; } describe('ThemenWidget', () => { @@ -32,6 +33,14 @@ describe('ThemenWidget', () => { expect(document.body.textContent).not.toContain('Leer'); }); + it('displays the subtree rollup count for a tag with 0 direct documents', async () => { + // 0 direct documents, but the subtree rolls up to 8 — the widget shows the rollup. + const tags = [makeTag('Reisen', 0, [], 8)]; + render(ThemenWidget, { tags }); + expect(document.body.textContent).toContain('Reisen'); + expect(document.body.textContent).toContain('8'); + }); + it('shows the empty state text when all tags are filtered out', async () => { render(ThemenWidget, { tags: [makeTag('Leer', 0)] }); expect(document.body.textContent).toMatch(/Noch keine Themen/); diff --git a/frontend/src/routes/themen/+page.svelte b/frontend/src/routes/themen/+page.svelte index 40daf0cf..c0a50b6c 100644 --- a/frontend/src/routes/themen/+page.svelte +++ b/frontend/src/routes/themen/+page.svelte @@ -41,14 +41,14 @@ const visibleTree = $derived.by(() => data.tree.filter(hasAnyDocuments)); 0 + ? ', ' + m.themen_dokumente({ count: tag.subtreeDocumentCount }) : ''}" class="flex min-h-[56px] items-center justify-between px-4 pt-4 pb-3 hover:bg-canvas focus-visible:ring-2 focus-visible:ring-brand-navy focus-visible:outline-none focus-visible:ring-inset" > {tag.name} - {#if tag.documentCount > 0}{tag.documentCount}{/if} + {#if tag.subtreeDocumentCount > 0}{tag.subtreeDocumentCount}{/if} @@ -63,7 +63,7 @@ const visibleTree = $derived.by(() => data.tree.filter(hasAnyDocuments)); > {child.name} - {#if child.documentCount > 0}{child.documentCount}{/if} + {#if child.subtreeDocumentCount > 0}{child.subtreeDocumentCount}{/if} diff --git a/frontend/src/routes/themen/page.svelte.spec.ts b/frontend/src/routes/themen/page.svelte.spec.ts index d9d9634a..4e0b9b55 100644 --- a/frontend/src/routes/themen/page.svelte.spec.ts +++ b/frontend/src/routes/themen/page.svelte.spec.ts @@ -12,9 +12,10 @@ type TagTreeNodeDTO = components['schemas']['TagTreeNodeDTO']; function makeTag( name: string, documentCount: number, - children: TagTreeNodeDTO[] = [] + children: TagTreeNodeDTO[] = [], + subtreeDocumentCount: number = documentCount ): TagTreeNodeDTO { - return { id: 'id-' + name, name, documentCount, children }; + return { id: 'id-' + name, name, documentCount, subtreeDocumentCount, children }; } describe('/themen +page', () => { @@ -48,6 +49,14 @@ describe('/themen +page', () => { expect(document.body.textContent).toContain('Kriegsbriefe'); }); + it('shows the subtree rollup in the header for a parent with 0 direct documents', async () => { + // AC#5: 0 direct docs, but the subtree rolls up to 7 (child contributes 3). + const tree = [makeTag('Reisen', 0, [makeTag('Italien', 3)], 7)]; + render(ThemenPage, { data: { tree } }); + expect(document.body.textContent).toContain('Reisen'); + expect(document.body.textContent).toContain('7'); + }); + it('shows "+ N weitere" when a root tag has more than 5 children', async () => { const children = Array.from({ length: 7 }, (_, i) => makeTag(`Kind${i}`, i + 1)); const tree = [makeTag('Briefe', 10, children)]; -- 2.49.1 From 1fc74f8892f0578bbeeae88dcd9a88c80bae98ae Mon Sep 17 00:00:00 2001 From: Marcel Date: Sun, 31 May 2026 12:26:18 +0200 Subject: [PATCH 6/8] test(tag): add subtreeDocumentCount to admin tree fixtures (#698) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit TagTreeNodeDTO now requires subtreeDocumentCount, so the admin sidebar test fixtures (TagTreeNode, TagsListPanel) need the field to type-check. The admin sidebar still renders the direct documentCount — these fixtures only gain the new field, no behaviour change. Co-Authored-By: Claude Opus 4.8 --- .../routes/admin/tags/TagTreeNode.svelte.test.ts | 13 ++++++++++++- .../src/routes/admin/tags/layout.svelte.spec.ts | 13 ++++++++++++- 2 files changed, 24 insertions(+), 2 deletions(-) diff --git a/frontend/src/routes/admin/tags/TagTreeNode.svelte.test.ts b/frontend/src/routes/admin/tags/TagTreeNode.svelte.test.ts index 5b43cc23..4aa04f7b 100644 --- a/frontend/src/routes/admin/tags/TagTreeNode.svelte.test.ts +++ b/frontend/src/routes/admin/tags/TagTreeNode.svelte.test.ts @@ -22,6 +22,7 @@ const leafNode = (overrides: Record = {}) => ({ name: 'Personen', color: 'sage', documentCount: 5, + subtreeDocumentCount: 5, parentId: null, children: [], ...overrides @@ -32,8 +33,18 @@ const parentNode = (overrides: Record = {}) => ({ name: 'Orte', color: 'sienna', documentCount: 0, + subtreeDocumentCount: 2, parentId: null, - children: [{ id: 'tc1', name: 'Berlin', color: null, documentCount: 2, children: [] }], + children: [ + { + id: 'tc1', + name: 'Berlin', + color: null, + documentCount: 2, + subtreeDocumentCount: 2, + children: [] + } + ], ...overrides }); diff --git a/frontend/src/routes/admin/tags/layout.svelte.spec.ts b/frontend/src/routes/admin/tags/layout.svelte.spec.ts index 058e8574..eacc2034 100644 --- a/frontend/src/routes/admin/tags/layout.svelte.spec.ts +++ b/frontend/src/routes/admin/tags/layout.svelte.spec.ts @@ -15,9 +15,18 @@ const tree = [ name: 'Familie', color: undefined, documentCount: 3, + subtreeDocumentCount: 5, parentId: undefined, children: [ - { id: 't2', name: 'Eltern', color: undefined, documentCount: 2, parentId: 't1', children: [] } + { + id: 't2', + name: 'Eltern', + color: undefined, + documentCount: 2, + subtreeDocumentCount: 2, + parentId: 't1', + children: [] + } ] }, { @@ -25,6 +34,7 @@ const tree = [ name: 'Urlaub', color: 'teal', documentCount: 0, + subtreeDocumentCount: 0, parentId: undefined, children: [] } @@ -128,6 +138,7 @@ describe('TagsListPanel — color dot', () => { id: 't1', name: 'Familie', documentCount: 0, + subtreeDocumentCount: 0, parentId: undefined, children: [], color: undefined -- 2.49.1 From 6d4aa8bd5c279f912077d7109ab887cc2b2796ca Mon Sep 17 00:00:00 2001 From: Marcel Date: Sun, 31 May 2026 12:26:42 +0200 Subject: [PATCH 7/8] test(admin-tags): pin merge/delete previews to the direct count (#698) Characterization tests for AC#8: the merge preview and the delete-impact warning describe direct-document operations, so they must report the tag's direct documentCount, never a subtree rollup. Both tests pass a stray subtreeDocumentCount and assert it does not leak into the preview, so a future change can't silently desync a destructive-action preview. Co-Authored-By: Claude Opus 4.8 --- .../tags/[id]/TagDeleteGuard.svelte.spec.ts | 15 +++++++++++ .../tags/[id]/TagMergeZone.svelte.spec.ts | 26 +++++++++++++++++++ 2 files changed, 41 insertions(+) diff --git a/frontend/src/routes/admin/tags/[id]/TagDeleteGuard.svelte.spec.ts b/frontend/src/routes/admin/tags/[id]/TagDeleteGuard.svelte.spec.ts index c55fd080..dbcd3069 100644 --- a/frontend/src/routes/admin/tags/[id]/TagDeleteGuard.svelte.spec.ts +++ b/frontend/src/routes/admin/tags/[id]/TagDeleteGuard.svelte.spec.ts @@ -68,6 +68,21 @@ describe('TagDeleteGuard', () => { renderWithConfirm(); await expect.element(page.getByText(/3 Dokument/)).toBeInTheDocument(); }); + + // Characterization (#698): the delete-impact warning describes a destructive single-tag delete, + // which removes only the tag's DIRECT document_tags rows — so it must show documentCount, never + // a subtree rollup. Pinned so a future change can't silently desync this warning. + it('shows the direct documentCount in the impact summary, not a subtree rollup', async () => { + const tagWithStraySubtree = { + id: 't1', + name: 'Familie', + documentCount: 3, + subtreeDocumentCount: 99 + }; + renderWithConfirm({ tag: tagWithStraySubtree, allTags }); + await expect.element(page.getByText(/3 Dokument/)).toBeInTheDocument(); + expect(document.body.textContent).not.toContain('99'); + }); }); describe('TagDeleteGuard – confirmation dialog', () => { diff --git a/frontend/src/routes/admin/tags/[id]/TagMergeZone.svelte.spec.ts b/frontend/src/routes/admin/tags/[id]/TagMergeZone.svelte.spec.ts index bace59b7..6adf946b 100644 --- a/frontend/src/routes/admin/tags/[id]/TagMergeZone.svelte.spec.ts +++ b/frontend/src/routes/admin/tags/[id]/TagMergeZone.svelte.spec.ts @@ -66,6 +66,32 @@ describe('TagMergeZone – step flow', () => { }); }); +describe('TagMergeZone – preview uses the direct document count (characterization, #698)', () => { + it('shows the tag direct documentCount in the merge preview, not a subtree rollup', async () => { + vi.stubGlobal( + 'fetch', + vi.fn().mockResolvedValue({ + ok: true, + json: vi.fn().mockResolvedValue([{ id: 't2', name: 'Reise' }]) + }) + ); + // documentCount (direct) = 3; a stray subtree rollup of 99 must NOT leak into the preview — + // merge re-tags only direct documents, so the preview has to stay the direct count. + const mergeTag = { id: 't1', name: 'Familie', documentCount: 3, subtreeDocumentCount: 99 }; + render(TagMergeZone, { tag: mergeTag, allTags, form: null }); + + const input = page.getByRole('combobox'); + await input.fill('R'); + await vi.advanceTimersByTimeAsync(300); + await page.getByRole('option', { name: 'Reise' }).click(); + await vi.advanceTimersByTimeAsync(0); + + await expect.element(page.getByTestId('merge-step2')).toBeInTheDocument(); + expect(document.body.textContent).toContain('3 Dokumente'); + expect(document.body.textContent).not.toContain('99'); + }); +}); + describe('TagMergeZone – stale state reset', () => { it('resets target selection when tag prop changes', async () => { vi.stubGlobal( -- 2.49.1 From a76999c3d4a2050ba417a0723d44a7210414e7de Mon Sep 17 00:00:00 2001 From: Marcel Date: Sun, 31 May 2026 12:40:54 +0200 Subject: [PATCH 8/8] test(tag): explicitly stub the subtree rollup query in getTagTree tests (#698) Address review nit: the older getTagTree tests relied on Mockito's default empty-list return for findSubtreeDocumentCountsPerTag. Stub it explicitly so the two-query contract is self-documenting. Co-Authored-By: Claude Opus 4.8 --- .../java/org/raddatz/familienarchiv/tag/TagServiceTest.java | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/backend/src/test/java/org/raddatz/familienarchiv/tag/TagServiceTest.java b/backend/src/test/java/org/raddatz/familienarchiv/tag/TagServiceTest.java index e9e06a70..3cb2762f 100644 --- a/backend/src/test/java/org/raddatz/familienarchiv/tag/TagServiceTest.java +++ b/backend/src/test/java/org/raddatz/familienarchiv/tag/TagServiceTest.java @@ -199,6 +199,7 @@ class TagServiceTest { void getTagTree_returnsEmptyList_whenNoTags() { when(tagRepository.findAll()).thenReturn(List.of()); when(tagRepository.findDocumentCountsPerTag()).thenReturn(List.of()); + when(tagRepository.findSubtreeDocumentCountsPerTag()).thenReturn(List.of()); assertThat(tagService.getTagTree()).isEmpty(); } @@ -213,6 +214,7 @@ class TagServiceTest { ); when(tagRepository.findAll()).thenReturn(tags); when(tagRepository.findDocumentCountsPerTag()).thenReturn(List.of()); + when(tagRepository.findSubtreeDocumentCountsPerTag()).thenReturn(List.of()); var tree = tagService.getTagTree(); @@ -228,6 +230,7 @@ class TagServiceTest { Tag child = Tag.builder().id(childId).name("Child").parentId(parentId).build(); when(tagRepository.findAll()).thenReturn(List.of(parent, child)); when(tagRepository.findDocumentCountsPerTag()).thenReturn(List.of()); + when(tagRepository.findSubtreeDocumentCountsPerTag()).thenReturn(List.of()); var tree = tagService.getTagTree(); @@ -247,6 +250,7 @@ class TagServiceTest { Tag child = Tag.builder().id(childId).name("Child").parentId(parentId).build(); when(tagRepository.findAll()).thenReturn(List.of(parent, child)); when(tagRepository.findDocumentCountsPerTag()).thenReturn(List.of()); + when(tagRepository.findSubtreeDocumentCountsPerTag()).thenReturn(List.of()); var tree = tagService.getTagTree(); @@ -262,6 +266,7 @@ class TagServiceTest { when(countEntry.getCount()).thenReturn(5L); when(tagRepository.findAll()).thenReturn(List.of(tag)); when(tagRepository.findDocumentCountsPerTag()).thenReturn(List.of(countEntry)); + when(tagRepository.findSubtreeDocumentCountsPerTag()).thenReturn(List.of()); var tree = tagService.getTagTree(); @@ -272,6 +277,7 @@ class TagServiceTest { void getTagTree_callsFindDocumentCountsPerTag_exactlyOnce() { when(tagRepository.findAll()).thenReturn(List.of()); when(tagRepository.findDocumentCountsPerTag()).thenReturn(List.of()); + when(tagRepository.findSubtreeDocumentCountsPerTag()).thenReturn(List.of()); tagService.getTagTree(); -- 2.49.1