Compare commits
9 Commits
07300aeff7
...
f80dda74f0
| Author | SHA1 | Date | |
|---|---|---|---|
|
|
f80dda74f0 | ||
|
|
22603a4b04 | ||
|
|
461a8b125d | ||
|
|
a670ba014c | ||
|
|
a9cac08f3c | ||
|
|
4cc725d546 | ||
|
|
535594378a | ||
|
|
e93b09f1e2 | ||
|
|
46d1f5c6d8 |
3
.gitignore
vendored
3
.gitignore
vendored
@@ -30,3 +30,6 @@ frontend/yarn.lock
|
||||
**/.venv/
|
||||
**/__pycache__/
|
||||
*.pyc
|
||||
|
||||
# Canonical import artifacts live only on the ops host (PII).
|
||||
# See tools/import-normalizer/.gitignore — load-bearing for that policy.
|
||||
|
||||
@@ -24,7 +24,6 @@ import software.amazon.awssdk.services.s3.model.PutObjectRequest;
|
||||
import org.raddatz.familienarchiv.tag.TagService;
|
||||
|
||||
import java.io.File;
|
||||
import java.io.FileInputStream;
|
||||
import java.io.IOException;
|
||||
import java.io.InputStream;
|
||||
import java.nio.file.Files;
|
||||
@@ -79,6 +78,7 @@ public class DocumentImporter {
|
||||
private final TagService tagService;
|
||||
private final S3Client s3Client;
|
||||
private final ThumbnailAsyncRunner thumbnailAsyncRunner;
|
||||
private final FileStreamOpener fileStreamOpener;
|
||||
|
||||
@Value("${app.s3.bucket:familienarchiv}")
|
||||
private String bucketName;
|
||||
@@ -178,39 +178,61 @@ public class DocumentImporter {
|
||||
String s3Key, String contentType, DocumentStatus status) {
|
||||
Document doc = existing != null ? existing
|
||||
: Document.builder().originalFilename(index).build();
|
||||
applyAttribution(doc, row);
|
||||
applyDates(doc, row);
|
||||
applyAuthoritativeAssociations(doc, row);
|
||||
applyFileMetadata(doc, s3Key, contentType, status, index);
|
||||
applyComputedFlags(doc);
|
||||
return doc;
|
||||
}
|
||||
|
||||
// Sender + raw sender/receiver text. The raw cells are always retained verbatim, even
|
||||
// when a person is linked — the load-bearing invariant behind the merge story (ADR-025).
|
||||
private void applyAttribution(Document doc, CanonicalSheetReader.Row row) {
|
||||
String senderName = row.get("sender_name");
|
||||
String receiverNames = row.get("receiver_names");
|
||||
Person sender = resolveSender(row.get("sender_person_id"), senderName);
|
||||
Set<Person> receivers = resolveReceivers(row.get("receiver_person_ids"));
|
||||
doc.setSender(sender);
|
||||
doc.setSenderText(blankToNull(senderName));
|
||||
doc.setReceiverText(blankToNull(receiverNames));
|
||||
}
|
||||
|
||||
LocalDate date = parseIsoDate(row.get("date_iso"));
|
||||
DatePrecision precision = parsePrecision(row.get("date_precision"));
|
||||
LocalDate dateEnd = parseIsoDate(row.get("date_end"));
|
||||
String dateRaw = blankToNull(row.get("date_raw"));
|
||||
String location = blankToNull(row.get("location"));
|
||||
// Date triplet + raw + location. Pure value parsing, no semantic logic.
|
||||
private void applyDates(Document doc, CanonicalSheetReader.Row row) {
|
||||
doc.setDocumentDate(parseIsoDate(row.get("date_iso")));
|
||||
doc.setMetaDatePrecision(parsePrecision(row.get("date_precision")));
|
||||
doc.setMetaDateEnd(parseIsoDate(row.get("date_end")));
|
||||
doc.setMetaDateRaw(blankToNull(row.get("date_raw")));
|
||||
doc.setLocation(blankToNull(row.get("location")));
|
||||
doc.setSummary(blankToNull(row.get("summary")));
|
||||
}
|
||||
|
||||
doc.setTitle(buildTitle(index, date, precision, dateEnd, dateRaw, location));
|
||||
// Receivers and tags are owned by the canonical row (ADR-025): clear then re-populate so a
|
||||
// shrunk set on re-import prunes stale links rather than accumulating them. The
|
||||
// "preserve human edits" rule does NOT extend to these collections.
|
||||
private void applyAuthoritativeAssociations(Document doc, CanonicalSheetReader.Row row) {
|
||||
Set<Person> receivers = resolveReceivers(row.get("receiver_person_ids"), row.get("receiver_names"));
|
||||
doc.getReceivers().clear();
|
||||
doc.getReceivers().addAll(receivers);
|
||||
attachTag(doc, row.get("tags"));
|
||||
}
|
||||
|
||||
// S3 key, content type, status, and the index-derived title.
|
||||
private void applyFileMetadata(Document doc, String s3Key, String contentType,
|
||||
DocumentStatus status, String index) {
|
||||
doc.setStatus(status);
|
||||
doc.setFilePath(s3Key);
|
||||
doc.setContentType(contentType);
|
||||
doc.setSender(sender);
|
||||
doc.setSenderText(blankToNull(senderName));
|
||||
// The canonical row is authoritative for receivers/tags (ADR-025): clear then
|
||||
// re-populate so a shrunk set on re-import prunes stale links rather than
|
||||
// accumulating them. The raw sender_text/receiver_text retention is separate.
|
||||
doc.getReceivers().clear();
|
||||
doc.getReceivers().addAll(receivers);
|
||||
doc.setReceiverText(blankToNull(receiverNames));
|
||||
doc.setDocumentDate(date);
|
||||
doc.setMetaDatePrecision(precision);
|
||||
doc.setMetaDateEnd(dateEnd);
|
||||
doc.setMetaDateRaw(dateRaw);
|
||||
doc.setLocation(location);
|
||||
doc.setSummary(blankToNull(row.get("summary")));
|
||||
attachTag(doc, row.get("tags"));
|
||||
doc.setMetadataComplete(doc.getDocumentDate() != null || sender != null || !receivers.isEmpty());
|
||||
return doc;
|
||||
doc.setTitle(buildTitle(index, doc.getDocumentDate(), doc.getMetaDatePrecision(),
|
||||
doc.getMetaDateEnd(), doc.getMetaDateRaw(), doc.getLocation()));
|
||||
}
|
||||
|
||||
// metadataComplete: a document counts as fully described if any of the three "who/when"
|
||||
// pieces is filled. Called last so the upstream setters have already populated the doc.
|
||||
private void applyComputedFlags(Document doc) {
|
||||
doc.setMetadataComplete(doc.getDocumentDate() != null
|
||||
|| doc.getSender() != null
|
||||
|| !doc.getReceivers().isEmpty());
|
||||
}
|
||||
|
||||
// The title carries the date at the HONEST precision (never a fabricated day) via the
|
||||
@@ -234,10 +256,18 @@ public class DocumentImporter {
|
||||
return resolvePerson(slug, rawName);
|
||||
}
|
||||
|
||||
private Set<Person> resolveReceivers(String slugs) {
|
||||
// Zips the parallel `receiver_person_ids` and `receiver_names` columns by position so an
|
||||
// unresolved receiver becomes a provisional Person whose lastName is the human name from
|
||||
// `receiver_names`, not the slug. If the names list is shorter than the slugs list (rare —
|
||||
// canonical data zips them 1:1), missing entries fall back to slug-as-name.
|
||||
private Set<Person> resolveReceivers(String slugs, String names) {
|
||||
List<String> slugList = CanonicalSheetReader.splitList(slugs);
|
||||
List<String> nameList = CanonicalSheetReader.splitList(names);
|
||||
Set<Person> receivers = new LinkedHashSet<>();
|
||||
for (String slug : CanonicalSheetReader.splitList(slugs)) {
|
||||
receivers.add(resolvePerson(slug, slug));
|
||||
for (int i = 0; i < slugList.size(); i++) {
|
||||
String slug = slugList.get(i);
|
||||
String name = i < nameList.size() ? nameList.get(i) : slug;
|
||||
receivers.add(resolvePerson(slug, name));
|
||||
}
|
||||
return receivers;
|
||||
}
|
||||
@@ -319,13 +349,10 @@ public class DocumentImporter {
|
||||
return INDEX_PATTERN.matcher(index).matches();
|
||||
}
|
||||
|
||||
// package-private: a Mockito spy in tests can override to inject IOException
|
||||
InputStream openFileStream(File file) throws IOException {
|
||||
return new FileInputStream(file);
|
||||
}
|
||||
|
||||
private boolean isPdfMagicBytes(File file) throws IOException {
|
||||
try (InputStream is = openFileStream(file)) {
|
||||
// FileStreamOpener is injected so tests can stub a throwing implementation for the
|
||||
// IO-error branch without spying on the importer itself.
|
||||
try (InputStream is = fileStreamOpener.open(file)) {
|
||||
byte[] header = is.readNBytes(4);
|
||||
return header.length == 4
|
||||
&& header[0] == 0x25 // %
|
||||
|
||||
@@ -0,0 +1,33 @@
|
||||
package org.raddatz.familienarchiv.importing;
|
||||
|
||||
import org.springframework.stereotype.Component;
|
||||
|
||||
import java.io.File;
|
||||
import java.io.FileInputStream;
|
||||
import java.io.IOException;
|
||||
import java.io.InputStream;
|
||||
|
||||
/**
|
||||
* Test seam for opening a {@link File} as an {@link InputStream}. Extracted so the magic-byte
|
||||
* check in {@link DocumentImporter} can be unit-tested for the IO-error branch by injecting a
|
||||
* mock that throws, without needing a Mockito spy on the importer itself.
|
||||
*
|
||||
* <p>Production uses {@link DefaultFileStreamOpener}, a one-line delegate to
|
||||
* {@code new FileInputStream(file)}.
|
||||
*/
|
||||
@FunctionalInterface
|
||||
public interface FileStreamOpener {
|
||||
|
||||
/** Opens {@code file} for sequential reads. Caller closes the returned stream. */
|
||||
InputStream open(File file) throws IOException;
|
||||
|
||||
/** Default production implementation: plain {@code FileInputStream}. */
|
||||
@Component
|
||||
final class DefaultFileStreamOpener implements FileStreamOpener {
|
||||
|
||||
@Override
|
||||
public InputStream open(File file) throws IOException {
|
||||
return new FileInputStream(file);
|
||||
}
|
||||
}
|
||||
}
|
||||
@@ -49,12 +49,18 @@ class DocumentImporterTest {
|
||||
@Mock TagService tagService;
|
||||
@Mock S3Client s3Client;
|
||||
@Mock ThumbnailAsyncRunner thumbnailAsyncRunner;
|
||||
@Mock FileStreamOpener fileStreamOpener;
|
||||
|
||||
DocumentImporter importer;
|
||||
|
||||
@BeforeEach
|
||||
void setUp() {
|
||||
importer = new DocumentImporter(documentService, personService, tagService, s3Client, thumbnailAsyncRunner);
|
||||
void setUp() throws java.io.IOException {
|
||||
// Default opener delegates to FileInputStream — tests that need to force an IOException
|
||||
// override this stub locally (load_skipsFile_whenMagicByteCheckThrowsIoException).
|
||||
lenient().when(fileStreamOpener.open(any(File.class)))
|
||||
.thenAnswer(inv -> new java.io.FileInputStream(inv.getArgument(0, File.class)));
|
||||
importer = new DocumentImporter(documentService, personService, tagService, s3Client,
|
||||
thumbnailAsyncRunner, fileStreamOpener);
|
||||
ReflectionTestUtils.setField(importer, "bucketName", "test-bucket");
|
||||
}
|
||||
|
||||
@@ -305,11 +311,11 @@ class DocumentImporterTest {
|
||||
lenient().when(documentService.findByOriginalFilename(any())).thenReturn(Optional.empty());
|
||||
Path xlsx = writeDocs(tempDir, docRow("W-0001", "", "", "", "", "", "", "", ""));
|
||||
|
||||
DocumentImporter spyImporter = org.mockito.Mockito.spy(importer);
|
||||
org.mockito.Mockito.doThrow(new java.io.IOException("read error"))
|
||||
.when(spyImporter).openFileStream(any(File.class));
|
||||
// FileStreamOpener is injected — stub it to throw, no spy on the importer needed.
|
||||
org.mockito.Mockito.when(fileStreamOpener.open(any(File.class)))
|
||||
.thenThrow(new java.io.IOException("read error"));
|
||||
|
||||
DocumentImporter.LoadResult result = spyImporter.load(xlsx.toFile());
|
||||
DocumentImporter.LoadResult result = importer.load(xlsx.toFile());
|
||||
|
||||
assertThat(result.skippedFiles())
|
||||
.extracting(ImportStatus.SkippedFile::reason)
|
||||
@@ -424,6 +430,60 @@ class DocumentImporterTest {
|
||||
&& "Herbert Cram|Clara".equals(d.getReceiverText())));
|
||||
}
|
||||
|
||||
@Test
|
||||
void load_provisionalReceiverUsesHumanNameFromReceiverNames_notSlug(@TempDir Path tempDir) throws Exception {
|
||||
// Regression: resolveReceivers used to pass the slug as both `sourceRef` AND `lastName`,
|
||||
// so an unresolved receiver "smith-john" became a provisional Person with
|
||||
// lastName="smith-john". The fix consumes the parallel `receiver_names` column.
|
||||
ReflectionTestUtils.setField(importer, "importDir", tempDir.toString());
|
||||
Person provisional = Person.builder().id(UUID.randomUUID()).sourceRef("smith-john")
|
||||
.lastName("John Smith").provisional(true).build();
|
||||
when(documentService.findByOriginalFilename("W-0050")).thenReturn(Optional.empty());
|
||||
when(documentService.save(any())).thenAnswer(inv -> inv.getArgument(0));
|
||||
when(personService.findBySourceRef("smith-john")).thenReturn(Optional.empty());
|
||||
when(personService.upsertBySourceRef(any())).thenReturn(provisional);
|
||||
Path xlsx = writeDocs(tempDir, docRow("W-0050", "", "",
|
||||
"smith-john", "John Smith", "", "", "", ""));
|
||||
|
||||
importer.load(xlsx.toFile());
|
||||
|
||||
org.mockito.ArgumentCaptor<PersonUpsertCommand> captor =
|
||||
org.mockito.ArgumentCaptor.forClass(PersonUpsertCommand.class);
|
||||
verify(personService).upsertBySourceRef(captor.capture());
|
||||
assertThat(captor.getValue().sourceRef()).isEqualTo("smith-john");
|
||||
assertThat(captor.getValue().lastName()).isEqualTo("John Smith");
|
||||
assertThat(captor.getValue().provisional()).isTrue();
|
||||
}
|
||||
|
||||
@Test
|
||||
void load_provisionalReceiverFallsBackToSlug_whenNamesListShorterThanSlugs(@TempDir Path tempDir) throws Exception {
|
||||
// Parallel-list zip: if the names list is shorter than the slugs list, slugs without a
|
||||
// matching name fall back to slug as the display name. This is the "missing name" case
|
||||
// (rare in canonical data but the contract must define it).
|
||||
ReflectionTestUtils.setField(importer, "importDir", tempDir.toString());
|
||||
Person alice = Person.builder().id(UUID.randomUUID()).sourceRef("alice-jones")
|
||||
.lastName("Alice Jones").provisional(true).build();
|
||||
Person bob = Person.builder().id(UUID.randomUUID()).sourceRef("bob-roe")
|
||||
.lastName("bob-roe").provisional(true).build();
|
||||
when(documentService.findByOriginalFilename("W-0051")).thenReturn(Optional.empty());
|
||||
when(documentService.save(any())).thenAnswer(inv -> inv.getArgument(0));
|
||||
when(personService.findBySourceRef("alice-jones")).thenReturn(Optional.empty());
|
||||
when(personService.findBySourceRef("bob-roe")).thenReturn(Optional.empty());
|
||||
when(personService.upsertBySourceRef(any())).thenReturn(alice).thenReturn(bob);
|
||||
Path xlsx = writeDocs(tempDir, docRow("W-0051", "", "",
|
||||
"alice-jones|bob-roe", "Alice Jones", "", "", "", ""));
|
||||
|
||||
importer.load(xlsx.toFile());
|
||||
|
||||
org.mockito.ArgumentCaptor<PersonUpsertCommand> captor =
|
||||
org.mockito.ArgumentCaptor.forClass(PersonUpsertCommand.class);
|
||||
verify(personService, org.mockito.Mockito.times(2)).upsertBySourceRef(captor.capture());
|
||||
assertThat(captor.getAllValues()).extracting(PersonUpsertCommand::sourceRef)
|
||||
.containsExactly("alice-jones", "bob-roe");
|
||||
assertThat(captor.getAllValues()).extracting(PersonUpsertCommand::lastName)
|
||||
.containsExactly("Alice Jones", "bob-roe");
|
||||
}
|
||||
|
||||
// ─── clean date values parse without semantic logic ──────────────────────────────
|
||||
|
||||
@Test
|
||||
|
||||
@@ -36,7 +36,9 @@
|
||||
# accidentally share an import source. Must be
|
||||
# readable by the backend container's UID
|
||||
# (currently root via the OpenJDK image — any
|
||||
# world-readable directory works).
|
||||
# world-readable directory works). Canonical
|
||||
# artifacts are NOT in git (PII — ADR-025); ops
|
||||
# syncs them in beside the PDFs out-of-band.
|
||||
|
||||
networks:
|
||||
archiv-net:
|
||||
@@ -224,6 +226,10 @@ services:
|
||||
# Read-only; the canonical importer only reads them from /import.
|
||||
# Required — no default — so staging and prod cannot accidentally share an
|
||||
# import source. CI workflows pin this per-env (see .gitea/workflows/).
|
||||
# NOTE: the canonical artifacts are NOT version-controlled (they contain real
|
||||
# family PII — see ADR-025). Ops must produce them locally from the Python
|
||||
# normalizer (tools/import-normalizer/) and sync them into this host path
|
||||
# alongside the <index>.pdf corpus before triggering an import.
|
||||
volumes:
|
||||
- ${IMPORT_HOST_DIR:?Set IMPORT_HOST_DIR to a host path holding the import payload (canonical artifacts + <index>.pdf files). See docs/DEPLOYMENT.md.}:/import:ro
|
||||
environment:
|
||||
|
||||
@@ -64,8 +64,10 @@ name classification via `findOrCreateByAlias`, an ODS/XXE XML path). It is **del
|
||||
|
||||
The rebuild is a `CanonicalImportOrchestrator` driving four single-responsibility loaders in
|
||||
an explicit dependency DAG — `TagTreeImporter` → `PersonRegisterImporter` →
|
||||
`PersonTreeImporter` → `DocumentImporter` — that **consume the committed canonical artifacts**
|
||||
(`tools/import-normalizer/out/`). A shared `CanonicalSheetReader` maps columns **by header
|
||||
`PersonTreeImporter` → `DocumentImporter` — that **consume the canonical artifacts produced
|
||||
by the offline Python normalizer** (`tools/import-normalizer/out/`, synced onto the ops host
|
||||
alongside the PDFs — see "Canonical artifacts are produced locally, NOT version-controlled"
|
||||
below). A shared `CanonicalSheetReader` maps columns **by header
|
||||
name** (not by index) and fails closed (`IMPORT_ARTIFACT_INVALID`) on a missing header. Each
|
||||
loader calls the **owning domain's service**, never a repository (layering rule); the tree
|
||||
loader uses `RelationshipService`, never the relationship repository.
|
||||
@@ -173,3 +175,27 @@ Settled sub-decisions:
|
||||
added to all three native `SELECT`s (`findAllWithDocumentCount`, `searchWithDocumentCount`,
|
||||
`findTopByDocumentCount`) or it would silently return `false`. Guarded by integration tests
|
||||
against real Postgres.
|
||||
|
||||
---
|
||||
|
||||
## Canonical artifacts are produced locally, NOT version-controlled
|
||||
|
||||
The four files in `tools/import-normalizer/out/` —
|
||||
`canonical-documents.xlsx`, `canonical-persons.xlsx`, `canonical-tag-tree.xlsx`,
|
||||
`canonical-persons-tree.json` — contain real family PII (names, addresses, attribution
|
||||
prose) and are **deliberately excluded from the git index** via
|
||||
`tools/import-normalizer/.gitignore`. They are regenerated locally from the source
|
||||
spreadsheet by running the Python normalizer, and synced into the ops host's
|
||||
`IMPORT_HOST_DIR` out-of-band (alongside the `<index>.pdf` corpus) — the same mechanism
|
||||
that delivers the PDFs.
|
||||
|
||||
The contract between normalizer and importer is the **header schema** (column names,
|
||||
their types, the `Precision` enum strings, the slug shape) — not the file contents.
|
||||
`CanonicalSheetReader` maps columns by header name and fails closed
|
||||
(`IMPORT_ARTIFACT_INVALID`) on a missing header, which is what locks the contract; the
|
||||
file-level golden fixtures stay outside the repo.
|
||||
|
||||
A future maintainer must not "fix" CI by checking these artifacts back in — they are
|
||||
PII, the regression that prompted this rule. Tests use small synthetic fixtures
|
||||
constructed in-process (`DocumentImporterTest`, `CanonicalImportIntegrationTest`) rather
|
||||
than real-corpus snapshots.
|
||||
|
||||
@@ -231,7 +231,9 @@ complete.*
|
||||
### 4.6 Canonical output & provenance (`FR-OUT`, `FR-PROV`) — resolves IMP-01, IMP-09, IMP-12
|
||||
|
||||
- **REQ-OUT-01** — The normalizer shall write `out/canonical-documents.xlsx` and
|
||||
`out/canonical-persons.xlsx` with the headered schemas in §6.
|
||||
`out/canonical-persons.xlsx` with the headered schemas in §6. The `out/` directory is
|
||||
**gitignored** (real family PII — see ADR-025); ops syncs the regenerated files onto the
|
||||
import host alongside the PDFs out-of-band.
|
||||
- **REQ-PROV-01** — Every canonical document row shall carry `source_row` (1-based row number
|
||||
in the source sheet) so any value can be traced back to the original.
|
||||
- **REQ-PROV-02** — Every canonical row shall carry a `needs_review` field listing zero or more
|
||||
|
||||
@@ -42,6 +42,12 @@ built) transforms the raw xlsx + person register into a clean canonical dataset
|
||||
re-run. The Java importer is adjusted to consume the canonical contract in a later **Phase 2**.
|
||||
See the spec for the full contract.
|
||||
|
||||
The canonical artifacts themselves (the `out/` files) are **produced locally and not
|
||||
version-controlled** — they contain real family PII. They are synced onto the ops host's
|
||||
`IMPORT_HOST_DIR` alongside the PDFs, out-of-band. The contract is the header schema in
|
||||
`02-normalization-spec.md` §6, not any particular file in `out/`. See ADR-025 for the full
|
||||
rationale.
|
||||
|
||||
## Status board
|
||||
|
||||
| ID | Issue | Severity | Status |
|
||||
|
||||
@@ -71,7 +71,13 @@ export default defineConfig(
|
||||
message:
|
||||
'text-accent is decorative-only (#a1dcd8 in light mode = 1.52:1 contrast — WCAG fail). Use text-primary or text-ink-2 for text labels.'
|
||||
}
|
||||
]
|
||||
],
|
||||
// Primary XSS guard: any {@html ...} block in a Svelte template is a potential
|
||||
// injection sink. This rule replaces the regex CI guard's role as the primary
|
||||
// defense (the CI regex stays as a backstop). For any legitimate use (e.g.
|
||||
// trusted server-rendered Markdown), suppress with an inline
|
||||
// `<!-- eslint-disable-next-line svelte/no-at-html-tags -->` and a justification.
|
||||
'svelte/no-at-html-tags': 'error'
|
||||
}
|
||||
},
|
||||
{
|
||||
|
||||
@@ -155,6 +155,9 @@
|
||||
"persons_review_delete_confirm_title": "Person löschen",
|
||||
"persons_review_delete_confirm_text": "Diese Person wird endgültig gelöscht. Dokumentverweise bleiben erhalten, verlieren aber diese Person.",
|
||||
"persons_review_delete_confirm_button": "Person löschen",
|
||||
"persons_review_confirm_confirm_title": "Person bestätigen",
|
||||
"persons_review_confirm_confirm_text": "Diese Person wird als bestätigt markiert und erscheint nicht mehr in der Prüfliste.",
|
||||
"persons_review_confirm_confirm_button": "Bestätigen",
|
||||
"persons_review_merge_label": "Mit welcher Person zusammenführen?",
|
||||
"persons_field_first_name": "Vorname",
|
||||
"persons_field_last_name": "Nachname",
|
||||
|
||||
@@ -155,6 +155,9 @@
|
||||
"persons_review_delete_confirm_title": "Delete person",
|
||||
"persons_review_delete_confirm_text": "This person will be permanently deleted. Document references are kept but lose this person.",
|
||||
"persons_review_delete_confirm_button": "Delete person",
|
||||
"persons_review_confirm_confirm_title": "Confirm person",
|
||||
"persons_review_confirm_confirm_text": "This person will be marked as confirmed and will no longer appear in the review list.",
|
||||
"persons_review_confirm_confirm_button": "Confirm",
|
||||
"persons_review_merge_label": "Merge into which person?",
|
||||
"persons_field_first_name": "First name",
|
||||
"persons_field_last_name": "Last name",
|
||||
|
||||
@@ -155,6 +155,9 @@
|
||||
"persons_review_delete_confirm_title": "Eliminar persona",
|
||||
"persons_review_delete_confirm_text": "Esta persona se eliminará de forma permanente. Las referencias de documentos se conservan pero pierden a esta persona.",
|
||||
"persons_review_delete_confirm_button": "Eliminar persona",
|
||||
"persons_review_confirm_confirm_title": "Confirmar persona",
|
||||
"persons_review_confirm_confirm_text": "Esta persona se marcará como confirmada y dejará de aparecer en la lista de revisión.",
|
||||
"persons_review_confirm_confirm_button": "Confirmar",
|
||||
"persons_review_merge_label": "¿Fusionar con qué persona?",
|
||||
"persons_field_first_name": "Nombre",
|
||||
"persons_field_last_name": "Apellido",
|
||||
|
||||
@@ -62,7 +62,19 @@ const deleteBtn =
|
||||
>
|
||||
{m.persons_review_action_rename()}
|
||||
</button>
|
||||
<form method="POST" action="?/confirm" use:enhance>
|
||||
<form
|
||||
method="POST"
|
||||
action="?/confirm"
|
||||
use:enhance={async ({ cancel }) => {
|
||||
const ok = await confirm({
|
||||
title: m.persons_review_confirm_confirm_title(),
|
||||
body: m.persons_review_confirm_confirm_text(),
|
||||
confirmLabel: m.persons_review_confirm_confirm_button(),
|
||||
destructive: false
|
||||
});
|
||||
if (!ok) cancel();
|
||||
}}
|
||||
>
|
||||
<input type="hidden" name="id" value={person.id} />
|
||||
<button type="submit" class={actionBtn}>{m.persons_review_action_confirm()}</button>
|
||||
</form>
|
||||
|
||||
@@ -31,7 +31,10 @@ export async function load({ url, fetch, locals }) {
|
||||
}
|
||||
|
||||
export const actions = {
|
||||
confirm: async ({ request, fetch }) => {
|
||||
confirm: async ({ request, fetch, locals }) => {
|
||||
if (!hasWriteAll(locals)) {
|
||||
return fail(403, { error: getErrorMessage('FORBIDDEN') });
|
||||
}
|
||||
const id = (await request.formData()).get('id') as string;
|
||||
const api = createApiClient(fetch);
|
||||
const result = await api.PATCH('/api/persons/{id}/confirm', {
|
||||
@@ -45,7 +48,10 @@ export const actions = {
|
||||
return { success: true };
|
||||
},
|
||||
|
||||
delete: async ({ request, fetch }) => {
|
||||
delete: async ({ request, fetch, locals }) => {
|
||||
if (!hasWriteAll(locals)) {
|
||||
return fail(403, { error: getErrorMessage('FORBIDDEN') });
|
||||
}
|
||||
const id = (await request.formData()).get('id') as string;
|
||||
const api = createApiClient(fetch);
|
||||
const result = await api.DELETE('/api/persons/{id}', {
|
||||
@@ -59,7 +65,10 @@ export const actions = {
|
||||
return { success: true };
|
||||
},
|
||||
|
||||
merge: async ({ request, fetch }) => {
|
||||
merge: async ({ request, fetch, locals }) => {
|
||||
if (!hasWriteAll(locals)) {
|
||||
return fail(403, { error: getErrorMessage('FORBIDDEN') });
|
||||
}
|
||||
const formData = await request.formData();
|
||||
const id = formData.get('id') as string;
|
||||
const targetPersonId = formData.get('targetPersonId') as string;
|
||||
@@ -79,7 +88,10 @@ export const actions = {
|
||||
return { success: true };
|
||||
},
|
||||
|
||||
rename: async ({ request, fetch }) => {
|
||||
rename: async ({ request, fetch, locals }) => {
|
||||
if (!hasWriteAll(locals)) {
|
||||
return fail(403, { error: getErrorMessage('FORBIDDEN') });
|
||||
}
|
||||
const formData = await request.formData();
|
||||
const id = formData.get('id') as string;
|
||||
const firstName = (formData.get('firstName') as string)?.trim() || undefined;
|
||||
|
||||
@@ -31,7 +31,7 @@ const hasResults = $derived(data.persons.length > 0);
|
||||
|
||||
{#if form?.error}
|
||||
<p
|
||||
class="mb-4 rounded border border-red-200 bg-red-50 px-3 py-2 text-sm text-red-600"
|
||||
class="mb-4 rounded border border-danger bg-danger/10 px-3 py-2 text-sm text-danger"
|
||||
role="alert"
|
||||
>
|
||||
{form.error}
|
||||
|
||||
252
frontend/src/routes/persons/review/page.server.spec.ts
Normal file
252
frontend/src/routes/persons/review/page.server.spec.ts
Normal file
@@ -0,0 +1,252 @@
|
||||
import { describe, it, expect, vi, beforeEach } from 'vitest';
|
||||
|
||||
vi.mock('$lib/shared/api.server', () => ({
|
||||
createApiClient: vi.fn(),
|
||||
extractErrorCode: (e: unknown) => (e as { code?: string } | undefined)?.code
|
||||
}));
|
||||
|
||||
import { actions } from './+page.server';
|
||||
import { createApiClient } from '$lib/shared/api.server';
|
||||
|
||||
beforeEach(() => vi.clearAllMocks());
|
||||
|
||||
const writer = { groups: [{ permissions: ['READ_ALL', 'WRITE_ALL'] }] };
|
||||
const reader = { groups: [{ permissions: ['READ_ALL'] }] };
|
||||
|
||||
/** Mock the typed client with a single response stubbed for every verb. */
|
||||
function mockApi(response: { ok: boolean; status: number; error?: unknown }) {
|
||||
const result = {
|
||||
response: { ok: response.ok, status: response.status },
|
||||
error: response.error,
|
||||
data: response.ok ? {} : undefined
|
||||
};
|
||||
const apiCall = vi.fn(() => Promise.resolve(result));
|
||||
vi.mocked(createApiClient).mockReturnValue({
|
||||
GET: apiCall,
|
||||
PATCH: apiCall,
|
||||
POST: apiCall,
|
||||
PUT: apiCall,
|
||||
DELETE: apiCall
|
||||
} as unknown as ReturnType<typeof createApiClient>);
|
||||
return apiCall;
|
||||
}
|
||||
|
||||
/** Build a SvelteKit RequestEvent with a FormData body and a user shape. */
|
||||
function runAction(
|
||||
action: (typeof actions)[keyof typeof actions],
|
||||
formData: FormData,
|
||||
user: unknown
|
||||
) {
|
||||
return action({
|
||||
request: new Request('http://localhost', { method: 'POST', body: formData }),
|
||||
fetch: vi.fn() as unknown as typeof fetch,
|
||||
locals: { user } as App.Locals
|
||||
// eslint-disable-next-line @typescript-eslint/no-explicit-any
|
||||
} as any);
|
||||
}
|
||||
|
||||
describe('persons/review confirm action', () => {
|
||||
it('returns { success: true } on backend 200', async () => {
|
||||
const apiCall = mockApi({ ok: true, status: 200 });
|
||||
const fd = new FormData();
|
||||
fd.append('id', 'p-1');
|
||||
|
||||
const result = await runAction(actions.confirm, fd, writer);
|
||||
|
||||
expect(apiCall).toHaveBeenCalledOnce();
|
||||
expect(result).toEqual({ success: true });
|
||||
});
|
||||
|
||||
it('returns fail(403) on backend 403', async () => {
|
||||
mockApi({ ok: false, status: 403, error: { code: 'FORBIDDEN' } });
|
||||
const fd = new FormData();
|
||||
fd.append('id', 'p-1');
|
||||
|
||||
const result = await runAction(actions.confirm, fd, writer);
|
||||
|
||||
expect(result).toMatchObject({ status: 403 });
|
||||
});
|
||||
|
||||
it('returns fail(404) on backend 404', async () => {
|
||||
mockApi({ ok: false, status: 404, error: { code: 'NOT_FOUND' } });
|
||||
const fd = new FormData();
|
||||
fd.append('id', 'p-missing');
|
||||
|
||||
const result = await runAction(actions.confirm, fd, writer);
|
||||
|
||||
expect(result).toMatchObject({ status: 404 });
|
||||
});
|
||||
|
||||
it('returns fail(403) when the user lacks WRITE_ALL (server-side guard)', async () => {
|
||||
const apiCall = mockApi({ ok: true, status: 200 });
|
||||
const fd = new FormData();
|
||||
fd.append('id', 'p-1');
|
||||
|
||||
const result = await runAction(actions.confirm, fd, reader);
|
||||
|
||||
expect(apiCall).not.toHaveBeenCalled();
|
||||
expect(result).toMatchObject({ status: 403 });
|
||||
});
|
||||
});
|
||||
|
||||
describe('persons/review delete action', () => {
|
||||
it('returns { success: true } on backend 200', async () => {
|
||||
const apiCall = mockApi({ ok: true, status: 200 });
|
||||
const fd = new FormData();
|
||||
fd.append('id', 'p-1');
|
||||
|
||||
const result = await runAction(actions.delete, fd, writer);
|
||||
|
||||
expect(apiCall).toHaveBeenCalledOnce();
|
||||
expect(result).toEqual({ success: true });
|
||||
});
|
||||
|
||||
it('returns fail(403) on backend 403', async () => {
|
||||
mockApi({ ok: false, status: 403, error: { code: 'FORBIDDEN' } });
|
||||
const fd = new FormData();
|
||||
fd.append('id', 'p-1');
|
||||
|
||||
const result = await runAction(actions.delete, fd, writer);
|
||||
|
||||
expect(result).toMatchObject({ status: 403 });
|
||||
});
|
||||
|
||||
it('returns fail(404) on backend 404', async () => {
|
||||
mockApi({ ok: false, status: 404, error: { code: 'NOT_FOUND' } });
|
||||
const fd = new FormData();
|
||||
fd.append('id', 'p-missing');
|
||||
|
||||
const result = await runAction(actions.delete, fd, writer);
|
||||
|
||||
expect(result).toMatchObject({ status: 404 });
|
||||
});
|
||||
|
||||
it('returns fail(403) when the user lacks WRITE_ALL (server-side guard)', async () => {
|
||||
const apiCall = mockApi({ ok: true, status: 200 });
|
||||
const fd = new FormData();
|
||||
fd.append('id', 'p-1');
|
||||
|
||||
const result = await runAction(actions.delete, fd, reader);
|
||||
|
||||
expect(apiCall).not.toHaveBeenCalled();
|
||||
expect(result).toMatchObject({ status: 403 });
|
||||
});
|
||||
});
|
||||
|
||||
describe('persons/review merge action', () => {
|
||||
it('returns { success: true } on backend 200', async () => {
|
||||
const apiCall = mockApi({ ok: true, status: 200 });
|
||||
const fd = new FormData();
|
||||
fd.append('id', 'p-1');
|
||||
fd.append('targetPersonId', 'p-2');
|
||||
|
||||
const result = await runAction(actions.merge, fd, writer);
|
||||
|
||||
expect(apiCall).toHaveBeenCalledOnce();
|
||||
expect(result).toEqual({ success: true });
|
||||
});
|
||||
|
||||
it('returns fail(400) when targetPersonId is missing', async () => {
|
||||
const apiCall = mockApi({ ok: true, status: 200 });
|
||||
const fd = new FormData();
|
||||
fd.append('id', 'p-1');
|
||||
|
||||
const result = await runAction(actions.merge, fd, writer);
|
||||
|
||||
expect(apiCall).not.toHaveBeenCalled();
|
||||
expect(result).toMatchObject({ status: 400 });
|
||||
});
|
||||
|
||||
it('returns fail(403) on backend 403', async () => {
|
||||
mockApi({ ok: false, status: 403, error: { code: 'FORBIDDEN' } });
|
||||
const fd = new FormData();
|
||||
fd.append('id', 'p-1');
|
||||
fd.append('targetPersonId', 'p-2');
|
||||
|
||||
const result = await runAction(actions.merge, fd, writer);
|
||||
|
||||
expect(result).toMatchObject({ status: 403 });
|
||||
});
|
||||
|
||||
it('returns fail(404) on backend 404', async () => {
|
||||
mockApi({ ok: false, status: 404, error: { code: 'NOT_FOUND' } });
|
||||
const fd = new FormData();
|
||||
fd.append('id', 'p-1');
|
||||
fd.append('targetPersonId', 'p-missing');
|
||||
|
||||
const result = await runAction(actions.merge, fd, writer);
|
||||
|
||||
expect(result).toMatchObject({ status: 404 });
|
||||
});
|
||||
|
||||
it('returns fail(403) when the user lacks WRITE_ALL (server-side guard)', async () => {
|
||||
const apiCall = mockApi({ ok: true, status: 200 });
|
||||
const fd = new FormData();
|
||||
fd.append('id', 'p-1');
|
||||
fd.append('targetPersonId', 'p-2');
|
||||
|
||||
const result = await runAction(actions.merge, fd, reader);
|
||||
|
||||
expect(apiCall).not.toHaveBeenCalled();
|
||||
expect(result).toMatchObject({ status: 403 });
|
||||
});
|
||||
});
|
||||
|
||||
describe('persons/review rename action', () => {
|
||||
it('returns { success: true } on backend 200', async () => {
|
||||
const apiCall = mockApi({ ok: true, status: 200 });
|
||||
const fd = new FormData();
|
||||
fd.append('id', 'p-1');
|
||||
fd.append('lastName', 'Smith');
|
||||
|
||||
const result = await runAction(actions.rename, fd, writer);
|
||||
|
||||
expect(apiCall).toHaveBeenCalledOnce();
|
||||
expect(result).toEqual({ success: true });
|
||||
});
|
||||
|
||||
it('returns fail(400) when lastName is missing', async () => {
|
||||
const apiCall = mockApi({ ok: true, status: 200 });
|
||||
const fd = new FormData();
|
||||
fd.append('id', 'p-1');
|
||||
|
||||
const result = await runAction(actions.rename, fd, writer);
|
||||
|
||||
expect(apiCall).not.toHaveBeenCalled();
|
||||
expect(result).toMatchObject({ status: 400 });
|
||||
});
|
||||
|
||||
it('returns fail(403) on backend 403', async () => {
|
||||
mockApi({ ok: false, status: 403, error: { code: 'FORBIDDEN' } });
|
||||
const fd = new FormData();
|
||||
fd.append('id', 'p-1');
|
||||
fd.append('lastName', 'Smith');
|
||||
|
||||
const result = await runAction(actions.rename, fd, writer);
|
||||
|
||||
expect(result).toMatchObject({ status: 403 });
|
||||
});
|
||||
|
||||
it('returns fail(404) on backend 404', async () => {
|
||||
mockApi({ ok: false, status: 404, error: { code: 'NOT_FOUND' } });
|
||||
const fd = new FormData();
|
||||
fd.append('id', 'p-missing');
|
||||
fd.append('lastName', 'Smith');
|
||||
|
||||
const result = await runAction(actions.rename, fd, writer);
|
||||
|
||||
expect(result).toMatchObject({ status: 404 });
|
||||
});
|
||||
|
||||
it('returns fail(403) when the user lacks WRITE_ALL (server-side guard)', async () => {
|
||||
const apiCall = mockApi({ ok: true, status: 200 });
|
||||
const fd = new FormData();
|
||||
fd.append('id', 'p-1');
|
||||
fd.append('lastName', 'Smith');
|
||||
|
||||
const result = await runAction(actions.rename, fd, reader);
|
||||
|
||||
expect(apiCall).not.toHaveBeenCalled();
|
||||
expect(result).toMatchObject({ status: 403 });
|
||||
});
|
||||
});
|
||||
4
tools/import-normalizer/.gitignore
vendored
4
tools/import-normalizer/.gitignore
vendored
@@ -1,7 +1,5 @@
|
||||
.venv/
|
||||
out/*
|
||||
!out/canonical-persons-tree.json
|
||||
!out/*.xlsx
|
||||
out/
|
||||
review/
|
||||
__pycache__/
|
||||
*.pyc
|
||||
|
||||
Binary file not shown.
File diff suppressed because it is too large
Load Diff
Binary file not shown.
Binary file not shown.
Reference in New Issue
Block a user