diff --git a/.specify/rtm.md b/.specify/rtm.md index 3b5a59d1..d1cab49d 100644 --- a/.specify/rtm.md +++ b/.specify/rtm.md @@ -158,3 +158,18 @@ | REQ-017 | derived Heirat sources SPOUSE_OF.fromDate + fromDatePrecision | #837 | relationship-edit-dates | `timeline/TimelineEventService#buildMarriageEvents` | `DerivedEventsAssemblyTest#should_emit_day_precision_heirat_from_spouse_fromDate` | Done | | REQ-018 | unauthenticated PUT → 401, no row modified | #837 | relationship-edit-dates | `person/relationship/RelationshipController` (SecurityConfig) | `RelationshipControllerTest#updateRelationship_returns401_whenUnauthenticated_and_does_not_touch_service` | Done | | REQ-019 | while a create/update request is in flight, submit is disabled + shows a progress indicator | #837 | relationship-edit-dates | `frontend/src/lib/person/relationship/AddRelationshipForm.svelte` | `AddRelationshipForm.svelte.spec.ts#disables the submit and shows a progress spinner while a submit is in flight` | Done | +| REQ-001 | TimelineEntryDTO carries rootTagId/rootTagName/rootTagColor for LETTER entries, assembled in-transaction (id+name+token only) | #835 | zeitstrahl-tag-chips | `timeline/TimelineEntryDTO`, `timeline/TimelineService#mapDocument` | `TimelineServiceTest#letter_with_tags_carries_its_primary_root_tag` | Done | +| REQ-002 | the three root-tag fields are nullable and not `@Schema(requiredMode = REQUIRED)` | #835 | zeitstrahl-tag-chips | `timeline/TimelineEntryDTO`, `frontend/src/lib/generated/api.ts` (optional) | `TimelineServiceTest#untagged_letter_has_no_root_tag_fields` (+ regenerated `api.ts` shows `rootTag*?`) | Done | +| REQ-003 | primary tag = root ancestor of the alphabetically-first assigned tag, resolved via TagService | #835 | zeitstrahl-tag-chips | `tag/TagService#resolveRootTags`, `timeline/TimelineService#primaryTag` | `TimelineServiceTest#letter_with_tags_carries_its_primary_root_tag`, `TagServiceTest#resolveRootTags_walksChildToRoot_withRootColor`, `TagServiceIntegrationTest#resolveRootTags_walksPersistedChainToRoot_withRootColor` | Done | +| REQ-004 | roots resolved in a single batched/memoized pass (≤ M findAncestorIds, no per-letter N+1); color from the root token | #835 | zeitstrahl-tag-chips | `tag/TagService#resolveRootTags`, `timeline/TimelineService#resolveLetterRootTags` | `TagServiceTest#resolveRootTags_memoizesPerDistinctTag_noNPlusOne`, `TimelineServiceTest#root_tags_resolved_in_a_single_batched_pass` | Done | +| REQ-005 | a letter with no tags → all three fields null; LetterCard renders no chip | #835 | zeitstrahl-tag-chips | `timeline/TimelineService#mapDocument`, `frontend/src/lib/timeline/LetterCard.svelte` | `TimelineServiceTest#untagged_letter_has_no_root_tag_fields`, `LetterCard.svelte.spec.ts#renders no chip when the letter has no root tag` | Done | +| REQ-006 | a letter with multiple tags → exactly one primary root (deterministic) | #835 | zeitstrahl-tag-chips | `timeline/TimelineService#primaryTag` | `TimelineServiceTest#letter_with_tags_carries_its_primary_root_tag`, `LetterCard.svelte.spec.ts#renders one root-tag chip beneath the meta line` | Done | +| REQ-007 | a colorless root → rootTagColor null; frontend renders a neutral chip, no `var(--c-tag-)` | #835 | zeitstrahl-tag-chips | `tag/TagService#resolveRootTags`, `frontend/src/lib/timeline/TagChip.svelte` | `TagServiceTest#resolveRootTags_returnsNullColor_whenRootHasNoColor`, `TimelineServiceTest#letter_primary_root_without_color_yields_null_color`, `TagChip.svelte.spec.ts#renders a neutral chip with no --c-tag- binding when color is null` | Done | +| REQ-008 | LetterCard with a rootTagName renders one §3 chip beneath the meta line | #835 | zeitstrahl-tag-chips | `frontend/src/lib/timeline/TagChip.svelte`, `LetterCard.svelte` | `TagChip.svelte.spec.ts#renders the tag name`, `LetterCard.svelte.spec.ts#renders one root-tag chip beneath the meta line` | Done | +| REQ-008a | a long name truncates with ellipsis, no horizontal overflow at 320px; full name in the chip title | #835 | zeitstrahl-tag-chips | `frontend/src/lib/timeline/TagChip.svelte`, `LetterCard.svelte` | `LetterCard.svelte.spec.ts#keeps a long tag name from overflowing the card at 320px`, `TagChip.svelte.spec.ts#exposes the full name as the chip title` | Done | +| REQ-009 | chip color applied via `var(--c-tag-{token})`, no raw hex | #835 | zeitstrahl-tag-chips | `frontend/src/lib/timeline/TagChip.svelte` | `TagChip.svelte.spec.ts#applies the color via var(--c-tag-{token}), never raw hex` | Done | +| REQ-010 | rootTagName rendered via `{...}` escaping; no `{@html}` in lib/timeline | #835 | zeitstrahl-tag-chips | `frontend/src/lib/timeline/TagChip.svelte` | `TagChip.svelte.spec.ts#renders an HTML-bearing name as inert text`, `grep -rn '@html' frontend/src/lib/timeline/` → zero | Done | +| REQ-011 | colored square aria-hidden; sr-only theme label prefix so color is never the only cue | #835 | zeitstrahl-tag-chips | `frontend/src/lib/timeline/TagChip.svelte` | `TagChip.svelte.spec.ts#prefixes the name with an sr-only theme label and a decorative square` | Done | +| REQ-012 | chip renders wherever a LetterCard renders (global timeline + expanded YearLetterStrip) | #835 | zeitstrahl-tag-chips | `frontend/src/lib/timeline/LetterCard.svelte` | `LetterCard.svelte.spec.ts#renders the chip inside an expanded YearLetterStrip too` | Done | +| REQ-013 | sr-only theme label is a Paraglide key present in de/en/es | #835 | zeitstrahl-tag-chips | `frontend/messages/{de,en,es}.json` | `messages.spec.ts#zeitstrahl tag-chip label key is present in all locales` | Done | +| REQ-014 | GET /api/timeline stays read-only (READ_ALL); no new endpoint/ErrorCode/IDOR; assembly logs UUIDs only | #835 | zeitstrahl-tag-chips | `timeline/TimelineController` (unchanged), `timeline/TimelineService` | `TimelineControllerTest#returns_200_with_read_all_permission`, `#returns_403_when_authenticated_without_read_all` (unchanged path); no tag names logged (review) | Done | diff --git a/backend/src/main/java/org/raddatz/familienarchiv/tag/RootTag.java b/backend/src/main/java/org/raddatz/familienarchiv/tag/RootTag.java new file mode 100644 index 00000000..987f3cd1 --- /dev/null +++ b/backend/src/main/java/org/raddatz/familienarchiv/tag/RootTag.java @@ -0,0 +1,13 @@ +package org.raddatz.familienarchiv.tag; + +import java.util.UUID; + +/** + * The root-ancestor view of a tag: its id, display name, and color token. + * Colors are stored only on root tags, so {@code color} is the authoritative token + * (one of {@link TagService#ALLOWED_TAG_COLORS}) or {@code null} when the root has none. + * Returned by {@link TagService#resolveRootTags} for read surfaces (the timeline chip, + * later the Thema buckets) that need a tag's theme without the entity graph. + */ +public record RootTag(UUID id, String name, String color) { +} 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 361d7d22..e6a5dfa8 100644 --- a/backend/src/main/java/org/raddatz/familienarchiv/tag/TagService.java +++ b/backend/src/main/java/org/raddatz/familienarchiv/tag/TagService.java @@ -175,6 +175,59 @@ public class TagService { }); } + /** + * Resolves each given tag to its root ancestor, returning a {@link RootTag} (id, name, color + * token) keyed by the input tag's id. A root tag maps to itself; a child is walked to the + * ancestor with no parent via {@link TagRepository#findAncestorIds} (one CTE per distinct + * non-root tag, memoized) plus a single batched {@code findAllById}, so a timeline of many + * letters sharing few tags costs O(distinct tags) queries, never O(letters). The color comes + * from the resolved root's stored token (null when the root has none). Safe on detached tags. + */ + public Map resolveRootTags(Collection tags) { + if (tags == null || tags.isEmpty()) return Map.of(); + + Map distinct = new LinkedHashMap<>(); + for (Tag tag : tags) { + if (tag != null && tag.getId() != null) distinct.putIfAbsent(tag.getId(), tag); + } + + Map> ancestorIdsByTagId = new HashMap<>(); + Set idsToLoad = new HashSet<>(); + for (Tag tag : distinct.values()) { + if (tag.getParentId() == null) continue; + List ancestorIds = tagRepository.findAncestorIds(tag.getId()); + ancestorIdsByTagId.put(tag.getId(), ancestorIds); + idsToLoad.addAll(ancestorIds); + } + + Map ancestorsById = idsToLoad.isEmpty() ? Map.of() + : tagRepository.findAllById(idsToLoad).stream() + .collect(Collectors.toMap(Tag::getId, t -> t)); + + Map result = new HashMap<>(); + for (Tag tag : distinct.values()) { + Tag root = resolveRoot(tag, ancestorIdsByTagId.get(tag.getId()), ancestorsById); + result.put(tag.getId(), new RootTag(root.getId(), root.getName(), root.getColor())); + } + return result; + } + + private Tag resolveRoot(Tag tag, List ancestorIds, Map ancestorsById) { + if (tag.getParentId() == null) return tag; + if (ancestorIds != null) { + for (UUID ancestorId : ancestorIds) { + Tag ancestor = ancestorsById.get(ancestorId); + if (ancestor != null && ancestor.getParentId() == null) return ancestor; + } + } + // No null-parent ancestor surfaced — the parent is orphaned or the chain is deeper than the + // findAncestorIds CTE's depth guard. Fall back to the tag as its own root, but surface it: + // a silently mislabeled root would otherwise be invisible. UUIDs only (no tag names logged). + log.warn("Tag {} has parent {} but no root surfaced from its ancestry; " + + "treating it as its own root.", tag.getId(), tag.getParentId()); + return tag; + } + /** * For each tag name, returns the set of that tag's ID plus all descendant IDs. * Used by DocumentService to expand selected filter tags before applying AND/OR logic. diff --git a/backend/src/main/java/org/raddatz/familienarchiv/timeline/TimelineEntryDTO.java b/backend/src/main/java/org/raddatz/familienarchiv/timeline/TimelineEntryDTO.java index 44cf88ec..0739cbfb 100644 --- a/backend/src/main/java/org/raddatz/familienarchiv/timeline/TimelineEntryDTO.java +++ b/backend/src/main/java/org/raddatz/familienarchiv/timeline/TimelineEntryDTO.java @@ -21,6 +21,13 @@ import java.util.UUID; *

Type field: {@code null} for {@link Kind#LETTER} entries; frontend must not render * an event-type badge for letters. * + *

Root-tag fields ({@code rootTagId}/{@code rootTagName}/{@code rootTagColor}): the + * letter's primary root tag — the root ancestor of its alphabetically-first assigned tag (#835). + * All three are {@code null} for non-{@link Kind#LETTER} entries and for letters with no tags; + * {@code rootTagColor} is additionally {@code null} when the resolved root carries no color token. + * They are deliberately NOT {@code @Schema(requiredMode = REQUIRED)} so the generated TypeScript + * types stay optional. + * *

Callers of {@code TimelineEventService.assembleDerivedEvents()} must independently enforce * {@code READ_ALL} authorization before invoking that method (see ADR-043). */ @@ -37,6 +44,9 @@ public record TimelineEntryDTO( UUID eventId, UUID documentId, List linkedPersonIds, - DerivedEventType derivedType + DerivedEventType derivedType, + UUID rootTagId, + String rootTagName, + String rootTagColor ) { } diff --git a/backend/src/main/java/org/raddatz/familienarchiv/timeline/TimelineEventService.java b/backend/src/main/java/org/raddatz/familienarchiv/timeline/TimelineEventService.java index a66ef3c5..f2ee6d7e 100644 --- a/backend/src/main/java/org/raddatz/familienarchiv/timeline/TimelineEventService.java +++ b/backend/src/main/java/org/raddatz/familienarchiv/timeline/TimelineEventService.java @@ -266,7 +266,8 @@ public class TimelineEventService { Kind.EVENT, p.getBirthDatePrecision(), true, "", "", p.getBirthDate(), null, p.getDisplayName(), EventType.PERSONAL, - null, null, List.of(p.getId()), DerivedEventType.BIRTH)) + null, null, List.of(p.getId()), DerivedEventType.BIRTH, + null, null, null)) .toList(); } @@ -277,7 +278,8 @@ public class TimelineEventService { Kind.EVENT, p.getDeathDatePrecision(), true, "", "", p.getDeathDate(), null, p.getDisplayName(), EventType.PERSONAL, - null, null, List.of(p.getId()), DerivedEventType.DEATH)) + null, null, List.of(p.getId()), DerivedEventType.DEATH, + null, null, null)) .toList(); } @@ -301,7 +303,8 @@ public class TimelineEventService { title, EventType.PERSONAL, null, null, List.of(r.getPerson().getId(), r.getRelatedPerson().getId()), - DerivedEventType.MARRIAGE)); + DerivedEventType.MARRIAGE, + null, null, null)); } } return result; diff --git a/backend/src/main/java/org/raddatz/familienarchiv/timeline/TimelineService.java b/backend/src/main/java/org/raddatz/familienarchiv/timeline/TimelineService.java index f55ccd91..7a084205 100644 --- a/backend/src/main/java/org/raddatz/familienarchiv/timeline/TimelineService.java +++ b/backend/src/main/java/org/raddatz/familienarchiv/timeline/TimelineService.java @@ -10,12 +10,16 @@ import org.raddatz.familienarchiv.exception.DomainException; import org.raddatz.familienarchiv.exception.ErrorCode; import org.raddatz.familienarchiv.person.Person; import org.raddatz.familienarchiv.person.PersonService; +import org.raddatz.familienarchiv.tag.RootTag; +import org.raddatz.familienarchiv.tag.Tag; +import org.raddatz.familienarchiv.tag.TagService; import org.springframework.stereotype.Service; import org.springframework.transaction.annotation.Transactional; import java.util.ArrayList; import java.util.Comparator; +import java.util.HashMap; import java.util.LinkedHashMap; import java.util.List; import java.util.Map; @@ -52,6 +56,7 @@ public class TimelineService { private final TimelineEventService timelineEventService; private final DocumentService documentService; private final PersonService personService; + private final TagService tagService; /** * Assembles the timeline for the given filter. All filters are ANDed. @@ -95,11 +100,15 @@ public class TimelineService { } // ── letters ───────────────────────────────────────────────────────── - List docs = fetchDocuments(filter.personId()); - for (Document doc : docs) { + List letters = new ArrayList<>(); + for (Document doc : fetchDocuments(filter.personId())) { if (!passesLetterGenerationFilter(doc, genPersonIds)) continue; if (!passesYearFilter(doc.getDocumentDate(), doc.getMetaDatePrecision(), filter)) continue; - entries.add(mapDocument(doc)); + letters.add(doc); + } + Map rootByDocId = resolveLetterRootTags(letters); + for (Document doc : letters) { + entries.add(mapDocument(doc, rootByDocId)); } return bucket(entries); @@ -217,11 +226,15 @@ public class TimelineService { ev.getId(), null, personIds, + null, + null, + null, null ); } - private TimelineEntryDTO mapDocument(Document doc) { + private TimelineEntryDTO mapDocument(Document doc, Map rootByDocId) { + RootTag root = rootByDocId.get(doc.getId()); return new TimelineEntryDTO( Kind.LETTER, doc.getMetaDatePrecision(), @@ -235,10 +248,47 @@ public class TimelineService { null, doc.getId(), List.of(), - null + null, + root == null ? null : root.id(), + root == null ? null : root.name(), + root == null ? null : root.color() ); } + /** + * Resolves each letter's primary root tag in one batched pass, keyed by document id — no + * per-letter N+1: each letter contributes only its alphabetically-first assigned tag (#835), + * so the {@code min()} scan over a letter's tag set runs exactly once here (not again at map + * time), and {@link TagService#resolveRootTags} memoizes the ancestry walk per distinct tag. + */ + private Map resolveLetterRootTags(List letters) { + Map primaryByDocId = new LinkedHashMap<>(); + for (Document doc : letters) { + Tag primary = primaryTag(doc); + if (primary != null) primaryByDocId.put(doc.getId(), primary); + } + if (primaryByDocId.isEmpty()) return Map.of(); + + Map rootByTagId = + tagService.resolveRootTags(new ArrayList<>(primaryByDocId.values())); + Map rootByDocId = new HashMap<>(); + primaryByDocId.forEach((docId, primary) -> { + RootTag root = rootByTagId.get(primary.getId()); + if (root != null) rootByDocId.put(docId, root); + }); + return rootByDocId; + } + + /** A letter's primary tag: the alphabetically-first of its assigned tags by name (#835). */ + private static Tag primaryTag(Document doc) { + Set tags = doc.getTags(); + if (tags == null || tags.isEmpty()) return null; + return tags.stream() + .filter(t -> t.getName() != null) + .min(Comparator.comparing(Tag::getName)) + .orElse(null); + } + private String resolveSenderName(Document doc) { if (doc.getSender() != null) return doc.getSender().getDisplayName(); String text = doc.getSenderText(); diff --git a/backend/src/test/java/org/raddatz/familienarchiv/tag/TagServiceIntegrationTest.java b/backend/src/test/java/org/raddatz/familienarchiv/tag/TagServiceIntegrationTest.java new file mode 100644 index 00000000..b0742566 --- /dev/null +++ b/backend/src/test/java/org/raddatz/familienarchiv/tag/TagServiceIntegrationTest.java @@ -0,0 +1,61 @@ +package org.raddatz.familienarchiv.tag; + +import org.junit.jupiter.api.BeforeEach; +import org.junit.jupiter.api.Test; +import org.raddatz.familienarchiv.PostgresContainerConfig; +import org.raddatz.familienarchiv.config.FlywayConfig; +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.List; +import java.util.Map; +import java.util.UUID; + +import static org.assertj.core.api.Assertions.assertThat; + +/** + * Real-Postgres proof that {@link TagService#resolveRootTags} walks a persisted tag chain to its + * true root through the recursive-CTE {@link TagRepository#findAncestorIds}. The CTE cannot run on + * H2, so this uses {@code postgres:16-alpine} via Testcontainers. Exhaustive case coverage lives in + * {@link TagServiceTest} (mocked); this pins the DB-dependent ancestry walk (issue #835, REQ-003/004). + */ +@DataJpaTest +@AutoConfigureTestDatabase(replace = AutoConfigureTestDatabase.Replace.NONE) +@Import({PostgresContainerConfig.class, FlywayConfig.class}) +class TagServiceIntegrationTest { + + @Autowired private TagRepository tagRepository; + private TagService tagService; + + @BeforeEach + void setUp() { + tagService = new TagService(tagRepository); + } + + private Tag tag(String name, String color, UUID parentId) { + return tagRepository.save(Tag.builder().name(name).color(color).parentId(parentId).build()); + } + + @Test + void resolveRootTags_walksPersistedChainToRoot_withRootColor() { + // leaf → mid → root resolves to the root's (id, name, color) via the real recursive CTE. + Tag root = tag("Krieg", "sienna", null); + Tag mid = tag("Feldpost", null, root.getId()); + Tag leaf = tag("Briefe von der Front", null, mid.getId()); + + Map result = tagService.resolveRootTags(List.of(leaf)); + + assertThat(result.get(leaf.getId())).isEqualTo(new RootTag(root.getId(), "Krieg", "sienna")); + } + + @Test + void resolveRootTags_returnsRootItself_forPersistedRoot() { + Tag root = tag("Weihnachten", "amber", null); + + Map result = tagService.resolveRootTags(List.of(root)); + + assertThat(result.get(root.getId())).isEqualTo(new RootTag(root.getId(), "Weihnachten", "amber")); + } +} 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 3c097073..1851292d 100644 --- a/backend/src/test/java/org/raddatz/familienarchiv/tag/TagServiceTest.java +++ b/backend/src/test/java/org/raddatz/familienarchiv/tag/TagServiceTest.java @@ -12,6 +12,7 @@ import org.raddatz.familienarchiv.tag.Tag; import org.raddatz.familienarchiv.tag.TagRepository; import java.util.List; +import java.util.Map; import java.util.Optional; import java.util.Set; import java.util.UUID; @@ -450,6 +451,74 @@ class TagServiceTest { assertThat(child2.getColor()).isEqualTo("sienna"); } + // ─── resolveRootTags ─────────────────────────────────────────────────────── + + @Test + void resolveRootTags_returnsTagItself_whenTagIsRoot() { + // REQ-003/004: a root tag (no parent) is its own primary root — no ancestry walk, no load. + Tag root = Tag.builder().id(UUID.randomUUID()).name("Krieg").color("sienna").build(); + + Map result = tagService.resolveRootTags(List.of(root)); + + assertThat(result.get(root.getId())).isEqualTo(new RootTag(root.getId(), "Krieg", "sienna")); + verify(tagRepository, never()).findAncestorIds(any()); + verify(tagRepository, never()).findAllById(any()); + } + + @Test + void resolveRootTags_walksChildToRoot_withRootColor() { + // REQ-003/004: a nested child resolves to its root's id/name/color via one CTE + one batch. + UUID rootId = UUID.randomUUID(); + UUID midId = UUID.randomUUID(); + Tag rootTag = Tag.builder().id(rootId).name("Krieg").color("sienna").build(); + Tag mid = Tag.builder().id(midId).name("Feldpost").parentId(rootId).build(); + Tag child = Tag.builder().id(UUID.randomUUID()).name("Briefe von der Front").parentId(midId).build(); + when(tagRepository.findAncestorIds(child.getId())).thenReturn(List.of(midId, rootId)); + when(tagRepository.findAllById(Set.of(midId, rootId))).thenReturn(List.of(mid, rootTag)); + + Map result = tagService.resolveRootTags(List.of(child)); + + assertThat(result.get(child.getId())).isEqualTo(new RootTag(rootId, "Krieg", "sienna")); + } + + @Test + void resolveRootTags_memoizesPerDistinctTag_noNPlusOne() { + // REQ-004: two letters sharing one tag id ⇒ a single findAncestorIds + a single batch load. + UUID rootId = UUID.randomUUID(); + UUID childId = UUID.randomUUID(); + Tag rootTag = Tag.builder().id(rootId).name("Krieg").color("sienna").build(); + Tag childA = Tag.builder().id(childId).name("Front").parentId(rootId).build(); + Tag childB = Tag.builder().id(childId).name("Front").parentId(rootId).build(); + when(tagRepository.findAncestorIds(childId)).thenReturn(List.of(rootId)); + when(tagRepository.findAllById(Set.of(rootId))).thenReturn(List.of(rootTag)); + + Map result = tagService.resolveRootTags(List.of(childA, childB)); + + assertThat(result.get(childId)).isEqualTo(new RootTag(rootId, "Krieg", "sienna")); + verify(tagRepository, times(1)).findAncestorIds(childId); + verify(tagRepository, times(1)).findAllById(any()); + } + + @Test + void resolveRootTags_returnsNullColor_whenRootHasNoColor() { + // REQ-007: a colorless root yields RootTag.color() == null (frontend renders a neutral chip). + UUID rootId = UUID.randomUUID(); + Tag rootTag = Tag.builder().id(rootId).name("Allgemein").build(); + Tag child = Tag.builder().id(UUID.randomUUID()).name("Notiz").parentId(rootId).build(); + when(tagRepository.findAncestorIds(child.getId())).thenReturn(List.of(rootId)); + when(tagRepository.findAllById(Set.of(rootId))).thenReturn(List.of(rootTag)); + + Map result = tagService.resolveRootTags(List.of(child)); + + assertThat(result.get(child.getId())).isEqualTo(new RootTag(rootId, "Allgemein", null)); + } + + @Test + void resolveRootTags_returnsEmptyMap_forEmptyInput() { + assertThat(tagService.resolveRootTags(List.of())).isEmpty(); + verify(tagRepository, never()).findAncestorIds(any()); + } + // ─── mergeTags ──────────────────────────────────────────────────────────── @Test diff --git a/backend/src/test/java/org/raddatz/familienarchiv/timeline/TimelineServiceTest.java b/backend/src/test/java/org/raddatz/familienarchiv/timeline/TimelineServiceTest.java index 549c4f10..06255ecb 100644 --- a/backend/src/test/java/org/raddatz/familienarchiv/timeline/TimelineServiceTest.java +++ b/backend/src/test/java/org/raddatz/familienarchiv/timeline/TimelineServiceTest.java @@ -11,13 +11,19 @@ import org.raddatz.familienarchiv.document.DocumentService; import org.raddatz.familienarchiv.exception.DomainException; import org.raddatz.familienarchiv.person.Person; import org.raddatz.familienarchiv.person.PersonService; +import org.raddatz.familienarchiv.tag.RootTag; +import org.raddatz.familienarchiv.tag.Tag; +import org.raddatz.familienarchiv.tag.TagService; import java.time.LocalDate; +import java.util.HashSet; import java.util.List; +import java.util.Map; import java.util.Set; import java.util.UUID; import static org.assertj.core.api.Assertions.*; +import static org.mockito.ArgumentMatchers.anyList; import static org.mockito.Mockito.*; @ExtendWith(MockitoExtension.class) @@ -27,6 +33,7 @@ class TimelineServiceTest { @Mock TimelineEventService timelineEventService; @Mock DocumentService documentService; @Mock PersonService personService; + @Mock TagService tagService; @InjectMocks TimelineService timelineService; @@ -61,9 +68,11 @@ class TimelineServiceTest { UUID id1 = UUID.fromString("00000000-0000-0000-0000-000000000001"); UUID id2 = UUID.fromString("00000000-0000-0000-0000-000000000002"); var e1 = new TimelineEntryDTO(Kind.LETTER, DatePrecision.DAY, false, "", "", - LocalDate.of(1914, 7, 28), null, "Same Title", null, null, id1, List.of(), null); + LocalDate.of(1914, 7, 28), null, "Same Title", null, null, id1, List.of(), null, + null, null, null); var e2 = new TimelineEntryDTO(Kind.LETTER, DatePrecision.DAY, false, "", "", - LocalDate.of(1914, 7, 28), null, "Same Title", null, null, id2, List.of(), null); + LocalDate.of(1914, 7, 28), null, "Same Title", null, null, id2, List.of(), null, + null, null, null); var sorted = List.of(e2, e1).stream() .sorted(TimelineService.WITHIN_BAND_ORDER) @@ -423,13 +432,98 @@ class TimelineServiceTest { // ─── Helpers ───────────────────────────────────────────────────────────── + // ─── root-tag chip enrichment (#835) ───────────────────────────────────── + + @Test + void letter_with_tags_carries_its_primary_root_tag() { + // REQ-003/006: the primary tag is the root ancestor of the alphabetically-first + // assigned tag ("Briefe von der Front" < "Zeitung"), resolved to root "Krieg". + UUID kriegId = UUID.randomUUID(); + Tag front = Tag.builder().id(UUID.randomUUID()).name("Briefe von der Front").parentId(kriegId).build(); + Tag zeitung = Tag.builder().id(UUID.randomUUID()).name("Zeitung").build(); + Document doc = docWithTags(LocalDate.of(1916, 5, 1), DatePrecision.MONTH, front, zeitung); + when(eventRepository.findAll()).thenReturn(List.of()); + when(timelineEventService.assembleDerivedEvents()).thenReturn(List.of()); + when(documentService.getAllForTimeline()).thenReturn(List.of(doc)); + when(tagService.resolveRootTags(anyList())) + .thenReturn(Map.of(front.getId(), new RootTag(kriegId, "Krieg", "sienna"))); + + TimelineEntryDTO entry = onlyLetterEntry(timelineService.assemble(noFilters())); + + assertThat(entry.rootTagId()).isEqualTo(kriegId); + assertThat(entry.rootTagName()).isEqualTo("Krieg"); + assertThat(entry.rootTagColor()).isEqualTo("sienna"); + } + + @Test + void untagged_letter_has_no_root_tag_fields() { + // REQ-005: a letter with no tags carries null id/name/color — and never hits TagService. + Document doc = docWithDate(LocalDate.of(1909, 3, 1), DatePrecision.MONTH); + when(eventRepository.findAll()).thenReturn(List.of()); + when(timelineEventService.assembleDerivedEvents()).thenReturn(List.of()); + when(documentService.getAllForTimeline()).thenReturn(List.of(doc)); + + TimelineEntryDTO entry = onlyLetterEntry(timelineService.assemble(noFilters())); + + assertThat(entry.rootTagId()).isNull(); + assertThat(entry.rootTagName()).isNull(); + assertThat(entry.rootTagColor()).isNull(); + verify(tagService, never()).resolveRootTags(anyList()); + } + + @Test + void letter_primary_root_without_color_yields_null_color() { + // REQ-007: a colorless root → rootTagColor null, id+name still present (neutral chip). + UUID rootId = UUID.randomUUID(); + Tag allgemein = Tag.builder().id(rootId).name("Allgemein").build(); + Document doc = docWithTags(LocalDate.of(1910, 2, 1), DatePrecision.MONTH, allgemein); + when(eventRepository.findAll()).thenReturn(List.of()); + when(timelineEventService.assembleDerivedEvents()).thenReturn(List.of()); + when(documentService.getAllForTimeline()).thenReturn(List.of(doc)); + when(tagService.resolveRootTags(anyList())) + .thenReturn(Map.of(rootId, new RootTag(rootId, "Allgemein", null))); + + TimelineEntryDTO entry = onlyLetterEntry(timelineService.assemble(noFilters())); + + assertThat(entry.rootTagId()).isEqualTo(rootId); + assertThat(entry.rootTagName()).isEqualTo("Allgemein"); + assertThat(entry.rootTagColor()).isNull(); + } + + @Test + void root_tags_resolved_in_a_single_batched_pass() { + // REQ-004: many letters → exactly one resolveRootTags call (no per-letter N+1). + UUID kriegId = UUID.randomUUID(); + Tag krieg = Tag.builder().id(kriegId).name("Krieg").color("sienna").build(); + Tag weihnachten = Tag.builder().id(UUID.randomUUID()).name("Weihnachten").color("amber").build(); + Document a = docWithTags(LocalDate.of(1915, 1, 1), DatePrecision.YEAR, krieg); + Document b = docWithTags(LocalDate.of(1916, 12, 1), DatePrecision.MONTH, weihnachten); + Document c = docWithTags(LocalDate.of(1917, 1, 1), DatePrecision.YEAR, krieg); + when(eventRepository.findAll()).thenReturn(List.of()); + when(timelineEventService.assembleDerivedEvents()).thenReturn(List.of()); + when(documentService.getAllForTimeline()).thenReturn(List.of(a, b, c)); + when(tagService.resolveRootTags(anyList())).thenReturn(Map.of( + kriegId, new RootTag(kriegId, "Krieg", "sienna"), + weihnachten.getId(), new RootTag(weihnachten.getId(), "Weihnachten", "amber"))); + + timelineService.assemble(noFilters()); + + verify(tagService, times(1)).resolveRootTags(anyList()); + } + + private static TimelineEntryDTO onlyLetterEntry(TimelineDTO result) { + assertThat(result.years()).hasSize(1); + return result.years().get(0).entries().get(0); + } + private static TimelineFilter noFilters() { return new TimelineFilter(null, null, null, null, null); } private static TimelineEntryDTO letter(LocalDate date, DatePrecision precision, String title) { return new TimelineEntryDTO(Kind.LETTER, precision, false, "", "", - date, null, title, null, null, UUID.randomUUID(), List.of(), null); + date, null, title, null, null, UUID.randomUUID(), List.of(), null, + null, null, null); } private static Document docWithDate(LocalDate date, DatePrecision precision) { @@ -437,6 +531,12 @@ class TimelineServiceTest { .metaDatePrecision(precision).documentDate(date).build(); } + private static Document docWithTags(LocalDate date, DatePrecision precision, Tag... tags) { + return Document.builder().id(UUID.randomUUID()).title("Brief") + .metaDatePrecision(precision).documentDate(date) + .tags(new HashSet<>(Set.of(tags))).build(); + } + private static Document docWithDate(LocalDate date, DatePrecision precision, String title) { return Document.builder().id(UUID.randomUUID()).title(title) .metaDatePrecision(precision).documentDate(date).build(); diff --git a/frontend/messages/de.json b/frontend/messages/de.json index 492c3ac0..125f0a11 100644 --- a/frontend/messages/de.json +++ b/frontend/messages/de.json @@ -1051,6 +1051,7 @@ "timeline_provenance_derived": "abgeleitet", "timeline_provenance_curated": "kuratiert", "timeline_letter_glyph_label": "Brief", + "timeline_tag_chip_label": "Thema", "timeline_layer_historical_suffix": "historisch", "timeline_strip_density_caption": "Monats-Dichte", "timeline_events_count": "{count} Ereignisse", diff --git a/frontend/messages/en.json b/frontend/messages/en.json index f1b04bf2..9ff1000e 100644 --- a/frontend/messages/en.json +++ b/frontend/messages/en.json @@ -1051,6 +1051,7 @@ "timeline_provenance_derived": "derived", "timeline_provenance_curated": "curated", "timeline_letter_glyph_label": "Letter", + "timeline_tag_chip_label": "Topic", "timeline_layer_historical_suffix": "historical", "timeline_strip_density_caption": "Monthly density", "timeline_events_count": "{count} events", diff --git a/frontend/messages/es.json b/frontend/messages/es.json index d495201e..2cd4548c 100644 --- a/frontend/messages/es.json +++ b/frontend/messages/es.json @@ -1051,6 +1051,7 @@ "timeline_provenance_derived": "derivado", "timeline_provenance_curated": "curado", "timeline_letter_glyph_label": "Carta", + "timeline_tag_chip_label": "Tema", "timeline_layer_historical_suffix": "histórico", "timeline_strip_density_caption": "Densidad mensual", "timeline_events_count": "{count} eventos", diff --git a/frontend/src/lib/generated/api.ts b/frontend/src/lib/generated/api.ts index 7241b00c..33f9b3ab 100644 --- a/frontend/src/lib/generated/api.ts +++ b/frontend/src/lib/generated/api.ts @@ -2463,6 +2463,10 @@ export interface components { linkedPersonIds?: string[]; /** @enum {string} */ derivedType?: "BIRTH" | "DEATH" | "MARRIAGE"; + /** Format: uuid */ + rootTagId?: string; + rootTagName?: string; + rootTagColor?: string; }; TimelineYearDTO: { /** Format: int32 */ diff --git a/frontend/src/lib/messages.spec.ts b/frontend/src/lib/messages.spec.ts index eadd4030..542a0d50 100644 --- a/frontend/src/lib/messages.spec.ts +++ b/frontend/src/lib/messages.spec.ts @@ -90,4 +90,12 @@ describe('message key parity', () => { expect(es, `missing key in es: ${key}`).toHaveProperty(key); } }); + + // #835 REQ-013: the letter chip's sr-only theme label is a Paraglide key in every + // locale so color is never the only cue; the tag NAME is rendered as data, not translated. + it('zeitstrahl tag-chip label key is present in all locales (#835 REQ-013)', () => { + expect(de).toMatchObject({ timeline_tag_chip_label: 'Thema' }); + expect(en).toMatchObject({ timeline_tag_chip_label: 'Topic' }); + expect(es).toMatchObject({ timeline_tag_chip_label: 'Tema' }); + }); }); diff --git a/frontend/src/lib/timeline/LetterCard.svelte b/frontend/src/lib/timeline/LetterCard.svelte index 4f44a6b4..593156fc 100644 --- a/frontend/src/lib/timeline/LetterCard.svelte +++ b/frontend/src/lib/timeline/LetterCard.svelte @@ -2,6 +2,7 @@ import * as m from '$lib/paraglide/messages.js'; import { timelineDateLabel } from './dateLabel'; import GlyphLabel from './GlyphLabel.svelte'; +import TagChip from './TagChip.svelte'; import type { components } from '$lib/generated/api'; type TimelineEntryDTO = components['schemas']['TimelineEntryDTO']; @@ -46,4 +47,9 @@ const receiver = $derived( · {dateLabel} {/if} + {#if entry.rootTagName} + + + {/if} diff --git a/frontend/src/lib/timeline/LetterCard.svelte.spec.ts b/frontend/src/lib/timeline/LetterCard.svelte.spec.ts index f9c928bc..b60c0f5f 100644 --- a/frontend/src/lib/timeline/LetterCard.svelte.spec.ts +++ b/frontend/src/lib/timeline/LetterCard.svelte.spec.ts @@ -1,7 +1,9 @@ import { describe, it, expect, afterEach } from 'vitest'; import { cleanup, render } from 'vitest-browser-svelte'; +import { tick } from 'svelte'; import * as m from '$lib/paraglide/messages.js'; import LetterCard from './LetterCard.svelte'; +import YearLetterStrip from './YearLetterStrip.svelte'; import { timelineDateLabel } from './dateLabel'; import { makeEntry } from './test-factories'; @@ -86,4 +88,42 @@ describe('LetterCard', () => { expect(document.body.textContent).toContain(evil); expect(document.querySelector('a script')).toBeNull(); }); + + it('renders one root-tag chip beneath the meta line when rootTagName is present (REQ-008)', () => { + render(LetterCard, { entry: makeEntry({ rootTagName: 'Familie', rootTagColor: 'sage' }) }); + const chips = document.querySelectorAll('[data-testid="tag-chip"]'); + expect(chips).toHaveLength(1); + expect(chips[0].textContent).toContain('Familie'); + }); + + it('renders no chip when the letter has no root tag (REQ-005/006)', () => { + render(LetterCard, { entry: makeEntry({ rootTagName: undefined, rootTagColor: undefined }) }); + expect(document.querySelector('[data-testid="tag-chip"]')).toBeNull(); + }); + + it('keeps a long tag name from overflowing the card at 320px, full name in the title (REQ-008a)', () => { + document.body.style.width = '320px'; + render(LetterCard, { + entry: makeEntry({ + rootTagName: 'Briefe von der Front und aus der Heimat', + rootTagColor: 'sienna' + }) + }); + const link = document.querySelector('a') as HTMLAnchorElement; + expect(link.scrollWidth).toBeLessThanOrEqual(link.clientWidth); + const chip = document.querySelector('[data-testid="tag-chip"]') as HTMLElement; + expect(chip.getAttribute('title')).toBe('Briefe von der Front und aus der Heimat'); + document.body.style.width = ''; + }); + + it('renders the chip inside an expanded YearLetterStrip too (REQ-012)', async () => { + render(YearLetterStrip, { + letters: [makeEntry({ rootTagName: 'Familie', rootTagColor: 'sage', documentId: 'doc-1' })], + year: 1909 + }); + (document.querySelector('[data-testid="strip-expand"]') as HTMLButtonElement).click(); + await tick(); + const chip = document.querySelector('[data-testid="tag-chip"]'); + expect(chip?.textContent).toContain('Familie'); + }); }); diff --git a/frontend/src/lib/timeline/TagChip.svelte b/frontend/src/lib/timeline/TagChip.svelte new file mode 100644 index 00000000..1cceecc9 --- /dev/null +++ b/frontend/src/lib/timeline/TagChip.svelte @@ -0,0 +1,41 @@ + + + + {m.timeline_tag_chip_label()}: + + + + {name} + diff --git a/frontend/src/lib/timeline/TagChip.svelte.spec.ts b/frontend/src/lib/timeline/TagChip.svelte.spec.ts new file mode 100644 index 00000000..4d31199b --- /dev/null +++ b/frontend/src/lib/timeline/TagChip.svelte.spec.ts @@ -0,0 +1,46 @@ +import { describe, it, expect, afterEach } from 'vitest'; +import { cleanup, render } from 'vitest-browser-svelte'; +import * as m from '$lib/paraglide/messages.js'; +import TagChip from './TagChip.svelte'; + +afterEach(() => cleanup()); + +describe('TagChip', () => { + it('renders the tag name (REQ-008)', () => { + render(TagChip, { name: 'Familie', color: 'sage' }); + expect(document.body.textContent).toContain('Familie'); + }); + + it('prefixes the name with an sr-only theme label and a decorative square (REQ-011)', () => { + render(TagChip, { name: 'Krieg', color: 'sienna' }); + const srOnly = document.querySelector('.sr-only'); + expect(srOnly?.textContent).toContain(m.timeline_tag_chip_label()); + const square = document.querySelector('[data-testid="tag-chip-square"]'); + expect(square?.getAttribute('aria-hidden')).toBe('true'); + }); + + it('applies the color via var(--c-tag-{token}), never raw hex (REQ-009)', () => { + render(TagChip, { name: 'Krieg', color: 'sienna' }); + const square = document.querySelector('[data-testid="tag-chip-square"]') as HTMLElement; + expect(square.getAttribute('style')).toContain('var(--c-tag-sienna)'); + }); + + it('renders a neutral chip with no --c-tag- binding when color is null (REQ-007)', () => { + render(TagChip, { name: 'Allgemein', color: null }); + expect(document.body.textContent).toContain('Allgemein'); + expect(document.body.innerHTML).not.toContain('var(--c-tag-'); + }); + + it('exposes the full name as the chip title so a truncated name stays reachable (REQ-008a)', () => { + render(TagChip, { name: 'Briefe von der Front', color: 'sienna' }); + const chip = document.querySelector('[data-testid="tag-chip"]') as HTMLElement; + expect(chip.getAttribute('title')).toBe('Briefe von der Front'); + }); + + it('renders an HTML-bearing name as inert text, never markup (REQ-010)', () => { + const evil = ''; + render(TagChip, { name: evil, color: null }); + expect(document.body.textContent).toContain(evil); + expect(document.querySelector('img')).toBeNull(); + }); +});