Compare commits

...

3 Commits

Author SHA1 Message Date
Marcel
f124529ee8 fix(stammbaum): seed gutter media-query state synchronously (#689)
Some checks failed
CI / Unit & Component Tests (pull_request) Failing after 2m32s
CI / OCR Service Tests (pull_request) Successful in 23s
CI / Backend Unit Tests (pull_request) Successful in 3m39s
CI / fail2ban Regex (pull_request) Failing after 45s
CI / Semgrep Security Scan (pull_request) Successful in 24s
CI / Compose Bucket Idempotency (pull_request) Successful in 1m4s
CI flagged two browser tests:

- "renders a G{n} label per occupied generation row …"
- "wraps the visible G3 text inside an aria-labelled group …"

Both queried g[role="text"] and got an empty array. Root cause:
isMdOrUp was initialised to false and only flipped to true inside a
$effect — but $effect runs after the first render, so the test's
post-render DOM scan saw the pre-effect (gutter-absent) state.

Seed the rune synchronously from window.matchMedia(...).matches when
window is available; SSR still picks the false branch and hydrates
without a layout flash. The effect now only attaches the change
listener for subsequent resizes.

Refs #689

Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
2026-05-28 16:22:09 +02:00
Marcel
61ca5a6e40 test(person): tighten generation null-clear coverage (#689)
Sara's QA concerns:

1. PersonControllerTest.updatePerson_returns200_whenGenerationNull was
   asymmetric — only checked status 200, no body assertion. Now also
   asserts `$.generation` is null in the JSON response, mirroring the
   in-range test's body check.

2. New full-stack PUT→DB→GET round-trip in PersonServiceIntegrationTest
   (updatePerson_clearGenerationToNull_readsBackNullFromDb) seeds a
   person with generation=3, calls updatePerson with generation=null,
   flushes the persistence context, and asserts the column reads back
   null from the DB. Without this we only had the mocked WebMvcTest
   boundary; nothing proved JPA actually wrote SQL NULL.

3. Sibling test (updatePerson_setGenerationToZero_readsBackZeroFromDb)
   pins the G 0 end-to-end so a primitive zero can't silently coerce
   to null anywhere along controller → service → JPA.

Refs #689

Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
2026-05-28 16:19:13 +02:00
Marcel
516a0a3814 refactor(person): single source of truth for generation bounds (#689)
Markus flagged the 0/10 range was duplicated across five sites (DB
CHECK, both importers, DTO @Min/@Max, dropdown range). New
PersonGeneration.MIN_GENERATION / MAX_GENERATION constants are now
the canonical Java source; the DTO annotations and both importer
guards reference them. The V70 SQL CHECK comment now points at the
Java constants so future widening updates one Java class plus one
SQL literal (Flyway forbids rewriting the migration in place).

Refs #689

Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
2026-05-28 16:16:26 +02:00
8 changed files with 104 additions and 25 deletions

View File

@@ -2,6 +2,7 @@ package org.raddatz.familienarchiv.importing;
import lombok.RequiredArgsConstructor; import lombok.RequiredArgsConstructor;
import lombok.extern.slf4j.Slf4j; import lombok.extern.slf4j.Slf4j;
import org.raddatz.familienarchiv.person.PersonGeneration;
import org.raddatz.familienarchiv.person.PersonService; import org.raddatz.familienarchiv.person.PersonService;
import org.raddatz.familienarchiv.person.PersonType; import org.raddatz.familienarchiv.person.PersonType;
import org.raddatz.familienarchiv.person.PersonUpsertCommand; import org.raddatz.familienarchiv.person.PersonUpsertCommand;
@@ -33,8 +34,6 @@ public class PersonRegisterImporter {
// carry an inline note. Out-of-range values are caught by the post-parse // carry an inline note. Out-of-range values are caught by the post-parse
// range guard, not by the regex. // range guard, not by the regex.
private static final Pattern GENERATION_PATTERN = Pattern.compile("^\\s*G?\\s*(-?\\d+)"); private static final Pattern GENERATION_PATTERN = Pattern.compile("^\\s*G?\\s*(-?\\d+)");
private static final int GENERATION_MIN = 0;
private static final int GENERATION_MAX = 10;
private final PersonService personService; private final PersonService personService;
@@ -68,16 +67,16 @@ public class PersonRegisterImporter {
/** /**
* Parses an optional {@code G n} generation cell. Returns null for blanks, * Parses an optional {@code G n} generation cell. Returns null for blanks,
* non-matching strings, and any value outside {@value #GENERATION_MIN}..{@value #GENERATION_MAX} * non-matching strings, and any value outside the {@link PersonGeneration}
* (mirroring the V70 CHECK). Out-of-range values log a WARN but never abort * bounds (mirroring the V70 CHECK). Out-of-range values log a WARN but
* the batch — REQ-IMP-001. * never abort the batch — REQ-IMP-001.
*/ */
static Integer parseGeneration(String raw, String personId) { static Integer parseGeneration(String raw, String personId) {
if (raw == null || raw.isBlank()) return null; if (raw == null || raw.isBlank()) return null;
Matcher m = GENERATION_PATTERN.matcher(raw); Matcher m = GENERATION_PATTERN.matcher(raw);
if (!m.find()) return null; if (!m.find()) return null;
int parsed = Integer.parseInt(m.group(1)); int parsed = Integer.parseInt(m.group(1));
if (parsed < GENERATION_MIN || parsed > GENERATION_MAX) { if (parsed < PersonGeneration.MIN_GENERATION || parsed > PersonGeneration.MAX_GENERATION) {
log.warn("Skipping out-of-range generation '{}' for row {}", raw, personId); log.warn("Skipping out-of-range generation '{}' for row {}", raw, personId);
return null; return null;
} }

View File

@@ -7,6 +7,7 @@ import lombok.extern.slf4j.Slf4j;
import org.raddatz.familienarchiv.exception.DomainException; import org.raddatz.familienarchiv.exception.DomainException;
import org.raddatz.familienarchiv.exception.ErrorCode; import org.raddatz.familienarchiv.exception.ErrorCode;
import org.raddatz.familienarchiv.person.Person; import org.raddatz.familienarchiv.person.Person;
import org.raddatz.familienarchiv.person.PersonGeneration;
import org.raddatz.familienarchiv.person.PersonService; import org.raddatz.familienarchiv.person.PersonService;
import org.raddatz.familienarchiv.person.PersonType; import org.raddatz.familienarchiv.person.PersonType;
import org.raddatz.familienarchiv.person.PersonUpsertCommand; import org.raddatz.familienarchiv.person.PersonUpsertCommand;
@@ -87,24 +88,21 @@ public class PersonTreeImporter {
} }
/** /**
* Returns the JSON {@code generation} value if present and in * Returns the JSON {@code generation} value if present and within the
* {@value #GENERATION_MIN}..{@value #GENERATION_MAX}; null otherwise. Out-of-range * {@link PersonGeneration} bounds; null otherwise. Out-of-range values
* values log a WARN but never abort the batch — mirrors the register-importer * log a WARN but never abort the batch — mirrors the register-importer
* skip-and-warn policy. * skip-and-warn policy.
*/ */
private static Integer generationOrNull(JsonNode node, String personId) { private static Integer generationOrNull(JsonNode node, String personId) {
Integer raw = intOrNull(node, "generation"); Integer raw = intOrNull(node, "generation");
if (raw == null) return null; if (raw == null) return null;
if (raw < GENERATION_MIN || raw > GENERATION_MAX) { if (raw < PersonGeneration.MIN_GENERATION || raw > PersonGeneration.MAX_GENERATION) {
log.warn("Skipping out-of-range generation '{}' for person {}", raw, personId); log.warn("Skipping out-of-range generation '{}' for person {}", raw, personId);
return null; return null;
} }
return raw; return raw;
} }
private static final int GENERATION_MIN = 0;
private static final int GENERATION_MAX = 10;
private int createRelationships(JsonNode relationships, Map<String, UUID> idByRowId) { private int createRelationships(JsonNode relationships, Map<String, UUID> idByRowId) {
int created = 0; int created = 0;
for (JsonNode node : relationships) { for (JsonNode node : relationships) {

View File

@@ -0,0 +1,16 @@
package org.raddatz.familienarchiv.person;
/**
* Single source of truth for the {@code persons.generation} value range.
* The DB CHECK in V70, the {@code PersonUpdateDTO} Bean Validation annotations,
* and the canonical importers all reference these constants so a future widening
* (e.g. accepting {@code G 1} ancestors) happens in one place. Mirror this file
* by hand in the V70 migration comment when adjusting bounds.
*/
public final class PersonGeneration {
public static final int MIN_GENERATION = 0;
public static final int MAX_GENERATION = 10;
private PersonGeneration() {}
}

View File

@@ -23,9 +23,9 @@ public class PersonUpdateDTO {
private String notes; private String notes;
private Integer birthYear; private Integer birthYear;
private Integer deathYear; private Integer deathYear;
// Mirror of the persons.generation CHECK constraint (V70). // Mirror of the persons.generation CHECK constraint (V70). Bounds live in
// null clears the field; 0..10 is the allowlist of valid indices. // PersonGeneration so DB, DTO, and importer all read from one place.
@Min(0) @Min(PersonGeneration.MIN_GENERATION)
@Max(10) @Max(PersonGeneration.MAX_GENERATION)
private Integer generation; private Integer generation;
} }

View File

@@ -10,11 +10,13 @@
ALTER TABLE persons ADD COLUMN generation SMALLINT; ALTER TABLE persons ADD COLUMN generation SMALLINT;
-- Allowlist of valid generation indices. The upper bound is intentionally -- Allowlist of valid generation indices. The 0..10 bounds mirror
-- loose — current data tops out at G 5, but a future G 6 → G 10 widening -- PersonGeneration.MIN_GENERATION / MAX_GENERATION in Java — keep the
-- needs no migration. A G 1 ancestor would require a separate one-shot -- two in sync (the DTO @Min/@Max and both importer range guards read from
-- shift migration (out of scope here; the layout's normalise step already -- those Java constants). Current data tops out at G 5, but a future G 6 →
-- handles negative seeds at render time). -- G 10 widening needs no migration. A G 1 ancestor would require a
-- separate one-shot shift migration (out of scope here; the layout's
-- normalise step already handles negative seeds at render time).
ALTER TABLE persons ADD CONSTRAINT chk_generation_range ALTER TABLE persons ADD CONSTRAINT chk_generation_range
CHECK (generation IS NULL OR generation BETWEEN 0 AND 10); CHECK (generation IS NULL OR generation BETWEEN 0 AND 10);

View File

@@ -746,6 +746,10 @@ class PersonControllerTest {
@Test @Test
@WithMockUser(authorities = "WRITE_ALL") @WithMockUser(authorities = "WRITE_ALL")
void updatePerson_returns200_whenGenerationNull() throws Exception { void updatePerson_returns200_whenGenerationNull() throws Exception {
// Symmetric body assertion: the response must echo generation as null (not
// absent), so the frontend re-hydrates the "(none)" option after a clear.
// Without this, the in-range test below would be the only end-to-end proof
// that the field flows through the controller.
Person saved = Person.builder().id(UUID.randomUUID()).firstName("Hans").lastName("Müller").build(); Person saved = Person.builder().id(UUID.randomUUID()).firstName("Hans").lastName("Müller").build();
when(personService.updatePerson(any(), any())).thenReturn(saved); when(personService.updatePerson(any(), any())).thenReturn(saved);
@@ -753,7 +757,8 @@ class PersonControllerTest {
.contentType(MediaType.APPLICATION_JSON) .contentType(MediaType.APPLICATION_JSON)
.content("{\"firstName\":\"Hans\",\"lastName\":\"Müller\"," .content("{\"firstName\":\"Hans\",\"lastName\":\"Müller\","
+ "\"personType\":\"PERSON\",\"generation\":null}")) + "\"personType\":\"PERSON\",\"generation\":null}"))
.andExpect(status().isOk()); .andExpect(status().isOk())
.andExpect(jsonPath("$.generation").value(org.hamcrest.Matchers.nullValue()));
} }
@Test @Test

View File

@@ -124,6 +124,59 @@ class PersonServiceIntegrationTest {
assertThat(personRepository.findById(target.getId())).isEmpty(); assertThat(personRepository.findById(target.getId())).isEmpty();
} }
// ─── generation full-stack round-trip (#689) ──────────────────────────────
@Test
void updatePerson_clearGenerationToNull_readsBackNullFromDb() {
// Sara's QA concern: pin the full PUT→DB→GET round-trip for the
// null-clear path. Without this we only have the WebMvcTest mocked
// boundary; nothing proved the JPA flush actually wrote SQL NULL.
Person seeded = personRepository.save(Person.builder()
.firstName("Hans").lastName("Raddatz")
.personType(PersonType.PERSON).generation(3).build());
entityManager.flush();
entityManager.clear();
PersonUpdateDTO dto = new PersonUpdateDTO();
dto.setPersonType(PersonType.PERSON);
dto.setFirstName("Hans");
dto.setLastName("Raddatz");
dto.setGeneration(null);
personService.updatePerson(seeded.getId(), dto);
entityManager.flush();
entityManager.clear();
Person reloaded = personRepository.findById(seeded.getId()).orElseThrow();
assertThat(reloaded.getGeneration()).isNull();
}
@Test
void updatePerson_setGenerationToZero_readsBackZeroFromDb() {
// Pin the G 0 case end-to-end. The form-action spec covers that 0
// doesn't get spread-dropped at the SvelteKit boundary; this test
// covers that the controller + service + JPA chain preserves the
// primitive zero (not coerced to null somewhere along the way).
Person seeded = personRepository.save(Person.builder()
.firstName("Walter").lastName("Raddatz")
.personType(PersonType.PERSON).build());
entityManager.flush();
entityManager.clear();
PersonUpdateDTO dto = new PersonUpdateDTO();
dto.setPersonType(PersonType.PERSON);
dto.setFirstName("Walter");
dto.setLastName("Raddatz");
dto.setGeneration(0);
personService.updatePerson(seeded.getId(), dto);
entityManager.flush();
entityManager.clear();
Person reloaded = personRepository.findById(seeded.getId()).orElseThrow();
assertThat(reloaded.getGeneration()).isEqualTo(0);
}
@Test @Test
void deletePerson_detachesSentAndReceivedReferences_beforeDelete_noOrphan() { void deletePerson_detachesSentAndReceivedReferences_beforeDelete_noOrphan() {
// A person referenced as BOTH a document sender and a document receiver must delete // A person referenced as BOTH a document sender and a document receiver must delete

View File

@@ -30,11 +30,17 @@ const layout = $derived.by<Layout>(() => buildLayout(nodes, edges));
// too costly on a 320 px screen). // too costly on a 320 px screen).
const GUTTER_WIDTH_DESKTOP = 100; const GUTTER_WIDTH_DESKTOP = 100;
const GUTTER_MEDIA_QUERY = '(min-width: 768px)'; const GUTTER_MEDIA_QUERY = '(min-width: 768px)';
let isMdOrUp = $state(false); // Seed synchronously so the first paint already has the right gutter state —
// otherwise the test (and a brief flash on real CSR mount) would see the
// pre-effect false. SSR has no window; the gutter stays hidden until hydrate.
let isMdOrUp = $state(
typeof window !== 'undefined' && typeof window.matchMedia === 'function'
? window.matchMedia(GUTTER_MEDIA_QUERY).matches
: false
);
$effect(() => { $effect(() => {
if (typeof window === 'undefined' || typeof window.matchMedia !== 'function') return; if (typeof window === 'undefined' || typeof window.matchMedia !== 'function') return;
const mq = window.matchMedia(GUTTER_MEDIA_QUERY); const mq = window.matchMedia(GUTTER_MEDIA_QUERY);
isMdOrUp = mq.matches;
const handler = (e: MediaQueryListEvent) => (isMdOrUp = e.matches); const handler = (e: MediaQueryListEvent) => (isMdOrUp = e.matches);
mq.addEventListener('change', handler); mq.addEventListener('change', handler);
return () => mq.removeEventListener('change', handler); return () => mq.removeEventListener('change', handler);