marcel/familienarchiv
1
0
Fork 0
Code Issues 119 Pull Requests 2 Actions Packages Projects Releases Wiki Activity

feat: support multiple senders per document #216

New Issue
Open
opened 2026-04-08 21:14:42 +02:00 by marcel · 6 comments
marcel commented 2026-04-08 21:14:42 +02:00
Owner
Copy Link

Problem

Document.sender is a @ManyToOne relationship (single sender_id FK), but real-world documents can have multiple senders — e.g. Von-column entries like "Gerd u Brigitte de Gruyter" or "Hans u Alma Cram". Currently the model can only store one sender per document.

This is a prerequisite for #214 (Von-column multi-person splitting). Without this change, routing Von entries through parseReceivers() would produce multiple persons with nowhere to store them.

Solution

Change the sender relationship from @ManyToOne to @ManyToMany, symmetric with how receivers already work.

Backend

File Change
Document.java Replace @ManyToOne Person sender with @ManyToMany Set<Person> senders (new join table document_senders)
V{n}__multiple_senders.sql Flyway migration: create document_senders join table, migrate existing sender_id data, drop sender_id column
DocumentUpdateDTO.java Change senderId (single UUID) to senderIds (List)
DocumentService.java Update create/update logic for multiple senders
DocumentSpecifications.java Update search queries that filter by sender
MassImportService.java Update doc.setSender() → doc.getSenders().add()

Frontend

File Change
api.ts Regenerate from OpenAPI spec
Document detail/edit pages Replace single sender display/input with multi-sender (reuse PersonMultiSelect or PersonChipRow)
Document list / search Update sender column display for multiple senders
DocumentMetadataDrawer.svelte Update sender display

Acceptance criteria

  • A document can have 0, 1, or many senders
  • Existing single-sender data is migrated without loss
  • Document edit form supports selecting multiple senders
  • Document detail view displays multiple senders
  • Search/filter by sender still works with the new model
  • API types regenerated

Found in

Architecture discussion on #214 — identified as a prerequisite for Von-column multi-person splitting.

## Problem `Document.sender` is a `@ManyToOne` relationship (single `sender_id` FK), but real-world documents can have multiple senders — e.g. Von-column entries like "Gerd u Brigitte de Gruyter" or "Hans u Alma Cram". Currently the model can only store one sender per document. This is a prerequisite for #214 (Von-column multi-person splitting). Without this change, routing Von entries through `parseReceivers()` would produce multiple persons with nowhere to store them. ## Solution Change the sender relationship from `@ManyToOne` to `@ManyToMany`, symmetric with how receivers already work. ### Backend | File | Change | |---|---| | `Document.java` | Replace `@ManyToOne Person sender` with `@ManyToMany Set<Person> senders` (new join table `document_senders`) | | `V{n}__multiple_senders.sql` | Flyway migration: create `document_senders` join table, migrate existing `sender_id` data, drop `sender_id` column | | `DocumentUpdateDTO.java` | Change `senderId` (single UUID) to `senderIds` (List<UUID>) | | `DocumentService.java` | Update create/update logic for multiple senders | | `DocumentSpecifications.java` | Update search queries that filter by sender | | `MassImportService.java` | Update `doc.setSender()` → `doc.getSenders().add()` | ### Frontend | File | Change | |---|---| | `api.ts` | Regenerate from OpenAPI spec | | Document detail/edit pages | Replace single sender display/input with multi-sender (reuse `PersonMultiSelect` or `PersonChipRow`) | | Document list / search | Update sender column display for multiple senders | | `DocumentMetadataDrawer.svelte` | Update sender display | ## Acceptance criteria - [ ] A document can have 0, 1, or many senders - [ ] Existing single-sender data is migrated without loss - [ ] Document edit form supports selecting multiple senders - [ ] Document detail view displays multiple senders - [ ] Search/filter by sender still works with the new model - [ ] API types regenerated ## Found in Architecture discussion on #214 — identified as a prerequisite for Von-column multi-person splitting.
marcel added the featureperson labels 2026-04-08 21:15:08 +02:00
marcel commented 2026-04-08 21:15:56 +02:00
Author
Owner
Copy Link

👨‍💻 Felix Brandt — Senior Fullstack Developer

Questions & Observations

  • DocumentUpdateDTO field rename — The issue says change senderId (UUID) to senderIds (List). This is a breaking API change — every frontend form action and server load function that reads or writes senderId will need updating. Has the full list of call sites been mapped? I'd search for senderId across the frontend before starting. Also, the new field should probably be Set<UUID> not List<UUID> for consistency with receivers which uses Set<Person>.

  • Sender input component choice — The issue suggests reusing PersonMultiSelect or PersonChipRow. Currently the sender field uses PersonTypeahead (single-person selector). Switching to PersonMultiSelect changes the interaction pattern significantly — chip-based multi-select vs. typeahead with single result. Is that the right UX, or should we keep the typeahead but allow multiple selections? This affects which component tests need updating.

  • Test strategy — The migration is the riskiest part. I'd want:

    • A @DataJpaTest with Testcontainers that loads the migration on a DB with existing single-sender data and verifies the join table is correctly populated
    • Unit tests for DocumentService update logic handling senderIds as a set
    • Frontend component tests for the sender multi-select in the edit form
    • A +page.server.ts load function test verifying the new senders array is passed to the page

  • DocumentSpecifications.java sender filter — Currently the sender search probably does a simple doc.sender.id = :senderId join. With @ManyToMany, this becomes a subquery or exists-join on the document_senders table. The Specification needs to mirror the pattern already used for receiver filtering — worth checking that the receiver spec is correct before duplicating it for senders.

Suggestions

  • Start with the Flyway migration and Document.java entity change. Write the migration test first (red), then the migration (green). The rest of the backend and frontend changes flow from there.
  • Rename senderId → senderIds in the DTO, but keep the API backward-compatible for one release by accepting both (single UUID auto-wrapped into a singleton set). This avoids a hard cutover.
## 👨‍💻 Felix Brandt — Senior Fullstack Developer ### Questions & Observations - **`DocumentUpdateDTO` field rename** — The issue says change `senderId` (UUID) to `senderIds` (List<UUID>). This is a breaking API change — every frontend form action and server load function that reads or writes `senderId` will need updating. Has the full list of call sites been mapped? I'd search for `senderId` across the frontend before starting. Also, the new field should probably be `Set<UUID>` not `List<UUID>` for consistency with `receivers` which uses `Set<Person>`. - **Sender input component choice** — The issue suggests reusing `PersonMultiSelect` or `PersonChipRow`. Currently the sender field uses `PersonTypeahead` (single-person selector). Switching to `PersonMultiSelect` changes the interaction pattern significantly — chip-based multi-select vs. typeahead with single result. Is that the right UX, or should we keep the typeahead but allow multiple selections? This affects which component tests need updating. - **Test strategy** — The migration is the riskiest part. I'd want: - A `@DataJpaTest` with Testcontainers that loads the migration on a DB with existing single-sender data and verifies the join table is correctly populated - Unit tests for `DocumentService` update logic handling `senderIds` as a set - Frontend component tests for the sender multi-select in the edit form - A `+page.server.ts` load function test verifying the new `senders` array is passed to the page - **`DocumentSpecifications.java` sender filter** — Currently the sender search probably does a simple `doc.sender.id = :senderId` join. With `@ManyToMany`, this becomes a subquery or exists-join on the `document_senders` table. The Specification needs to mirror the pattern already used for receiver filtering — worth checking that the receiver spec is correct before duplicating it for senders. ### Suggestions - Start with the Flyway migration and `Document.java` entity change. Write the migration test first (red), then the migration (green). The rest of the backend and frontend changes flow from there. - Rename `senderId` → `senderIds` in the DTO, but keep the API backward-compatible for one release by accepting both (single UUID auto-wrapped into a singleton set). This avoids a hard cutover.
marcel commented 2026-04-08 21:16:10 +02:00
Author
Owner
Copy Link

🏗️ Markus Keller — Application Architect

Questions & Observations

  • Join table design — The document_senders join table should mirror document_receivers exactly: (document_id UUID, person_id UUID, PRIMARY KEY (document_id, person_id)) with FK constraints and ON DELETE CASCADE on the document side. Is there an ordering requirement (first sender = primary sender)? If so, a join table with an ordinal column would be needed instead of a plain @ManyToMany. I'd say no — senders are unordered, same as receivers.

  • Fetch strategy — receivers uses FetchType.EAGER on the @ManyToMany. The same should apply to senders for consistency, but be aware this means every documentRepository.findById() now produces two additional joins. For 1500 documents with typically 1 sender each, this is fine. Worth noting for future scale.

  • Migration safety — The migration needs to be split into two steps within the same Flyway file: (1) create document_senders and populate from sender_id, (2) drop sender_id. If step 2 runs before step 1 completes, data is lost. Standard approach: INSERT INTO document_senders SELECT id, sender_id FROM documents WHERE sender_id IS NOT NULL, then ALTER TABLE documents DROP COLUMN sender_id.

  • Search/filter symmetry — With senders and receivers both being @ManyToMany to Person, the DocumentSpecifications code for filtering by sender and by receiver should be structurally identical. Consider extracting a shared spec method hasPersonInRole(role, personId) that both use.

Suggestions

  • Keep the migration in a single Flyway file with the two-step approach above. No need for a multi-file migration for this.
  • Do not add an ordinal column to the join table unless there's a concrete requirement for sender ordering. YAGNI.
## 🏗️ Markus Keller — Application Architect ### Questions & Observations - **Join table design** — The `document_senders` join table should mirror `document_receivers` exactly: `(document_id UUID, person_id UUID, PRIMARY KEY (document_id, person_id))` with FK constraints and `ON DELETE CASCADE` on the document side. Is there an ordering requirement (first sender = primary sender)? If so, a join table with an `ordinal` column would be needed instead of a plain `@ManyToMany`. I'd say no — senders are unordered, same as receivers. - **Fetch strategy** — `receivers` uses `FetchType.EAGER` on the `@ManyToMany`. The same should apply to `senders` for consistency, but be aware this means every `documentRepository.findById()` now produces two additional joins. For 1500 documents with typically 1 sender each, this is fine. Worth noting for future scale. - **Migration safety** — The migration needs to be split into two steps within the same Flyway file: (1) create `document_senders` and populate from `sender_id`, (2) drop `sender_id`. If step 2 runs before step 1 completes, data is lost. Standard approach: `INSERT INTO document_senders SELECT id, sender_id FROM documents WHERE sender_id IS NOT NULL`, then `ALTER TABLE documents DROP COLUMN sender_id`. - **Search/filter symmetry** — With senders and receivers both being `@ManyToMany` to `Person`, the `DocumentSpecifications` code for filtering by sender and by receiver should be structurally identical. Consider extracting a shared spec method `hasPersonInRole(role, personId)` that both use. ### Suggestions - Keep the migration in a single Flyway file with the two-step approach above. No need for a multi-file migration for this. - Do not add an `ordinal` column to the join table unless there's a concrete requirement for sender ordering. YAGNI.
marcel commented 2026-04-08 21:16:24 +02:00
Author
Owner
Copy Link

🧪 Sara Holt — QA Engineer & Test Strategist

Questions & Observations

  • Migration test coverage — This is the highest-risk change. I want an integration test (@SpringBootTest + Testcontainers) that:

    1. Inserts documents with sender_id values using raw SQL (pre-migration state)
    2. Runs the Flyway migration
    3. Verifies document_senders rows match the original sender_id values
    4. Verifies documents with sender_id = NULL have no rows in document_senders
    5. Verifies the sender_id column no longer exists

  • Acceptance criteria gaps — The criteria say "Search/filter by sender still works" but don't specify: does filtering by one sender return documents where that person is one of multiple senders, or only documents where they're the sole sender? The answer should be "any document where that person is a sender" (same as receiver filtering), but this needs to be explicit in the test.

  • Regression scope — Sender is referenced in:

    • Document search/list page
    • Document detail page
    • Document edit form
    • Document metadata drawer
    • Mass import service
    • Briefwechsel/conversation pages (they filter by sender/receiver pairs)

    Each of these is a regression test target. The conversation pages are the one most likely to break silently — they probably assume a single sender when building the correspondence timeline.

  • Edge case: document with 0 senders — The acceptance criteria include "0 senders". Is this a new valid state, or was it already possible (nullable sender_id)? If it's new, existing code that does doc.getSender().getLastName() without a null check will NPE. With @ManyToMany and an empty set, the risk shifts to doc.getSenders().iterator().next() or similar first-element access patterns.

Suggestions

  • Add an explicit acceptance criterion: "Briefwechsel/conversation timeline works correctly with multi-sender documents"
  • Write a parameterized test for the search filter: single sender, multi-sender, no sender — all three should be tested.
## 🧪 Sara Holt — QA Engineer & Test Strategist ### Questions & Observations - **Migration test coverage** — This is the highest-risk change. I want an integration test (`@SpringBootTest` + Testcontainers) that: 1. Inserts documents with `sender_id` values using raw SQL (pre-migration state) 2. Runs the Flyway migration 3. Verifies `document_senders` rows match the original `sender_id` values 4. Verifies documents with `sender_id = NULL` have no rows in `document_senders` 5. Verifies the `sender_id` column no longer exists - **Acceptance criteria gaps** — The criteria say "Search/filter by sender still works" but don't specify: does filtering by one sender return documents where that person is one of multiple senders, or only documents where they're the sole sender? The answer should be "any document where that person is a sender" (same as receiver filtering), but this needs to be explicit in the test. - **Regression scope** — Sender is referenced in: - Document search/list page - Document detail page - Document edit form - Document metadata drawer - Mass import service - Briefwechsel/conversation pages (they filter by sender/receiver pairs) Each of these is a regression test target. The conversation pages are the one most likely to break silently — they probably assume a single sender when building the correspondence timeline. - **Edge case: document with 0 senders** — The acceptance criteria include "0 senders". Is this a new valid state, or was it already possible (nullable `sender_id`)? If it's new, existing code that does `doc.getSender().getLastName()` without a null check will NPE. With `@ManyToMany` and an empty set, the risk shifts to `doc.getSenders().iterator().next()` or similar first-element access patterns. ### Suggestions - Add an explicit acceptance criterion: "Briefwechsel/conversation timeline works correctly with multi-sender documents" - Write a parameterized test for the search filter: single sender, multi-sender, no sender — all three should be tested.
marcel commented 2026-04-08 21:16:35 +02:00
Author
Owner
Copy Link

🔒 Nora "NullX" Steiner — Application Security Engineer

Questions & Observations

  • Mass assignment risk on the new senderIds field — The DocumentUpdateDTO currently has senderId (single UUID). Changing to senderIds (list/set of UUIDs) means the API now accepts an array of person IDs. The existing @RequirePermission(WRITE_ALL) on the update endpoint covers authorization, but verify that:

    • The service validates each UUID in senderIds exists in the persons table before creating the association
    • A user can't inject arbitrary UUIDs to associate documents with persons they shouldn't have access to (this is mostly a data integrity concern in this app, not a privilege escalation — but worth confirming)

  • No new attack surface — This is a structural change to an existing relationship. The trust boundary doesn't change: all sender data comes through the same authenticated API or the import pipeline. The @ManyToMany join table uses parameterized JPA queries, so no injection risk from the new table.

  • Migration data integrity — The migration should include a constraint: FOREIGN KEY (person_id) REFERENCES persons(id) on the new join table. Without it, orphaned person references could accumulate if persons are deleted without cascading to document_senders.

Suggestions

  • No new security concerns introduced. Verify FK constraints on the join table and confirm each senderId is validated against the persons table before association. Standard data integrity, not a security issue per se.
## 🔒 Nora "NullX" Steiner — Application Security Engineer ### Questions & Observations - **Mass assignment risk on the new `senderIds` field** — The `DocumentUpdateDTO` currently has `senderId` (single UUID). Changing to `senderIds` (list/set of UUIDs) means the API now accepts an array of person IDs. The existing `@RequirePermission(WRITE_ALL)` on the update endpoint covers authorization, but verify that: - The service validates each UUID in `senderIds` exists in the `persons` table before creating the association - A user can't inject arbitrary UUIDs to associate documents with persons they shouldn't have access to (this is mostly a data integrity concern in this app, not a privilege escalation — but worth confirming) - **No new attack surface** — This is a structural change to an existing relationship. The trust boundary doesn't change: all sender data comes through the same authenticated API or the import pipeline. The `@ManyToMany` join table uses parameterized JPA queries, so no injection risk from the new table. - **Migration data integrity** — The migration should include a constraint: `FOREIGN KEY (person_id) REFERENCES persons(id)` on the new join table. Without it, orphaned person references could accumulate if persons are deleted without cascading to `document_senders`. ### Suggestions - No new security concerns introduced. Verify FK constraints on the join table and confirm each `senderId` is validated against the `persons` table before association. Standard data integrity, not a security issue per se.
marcel commented 2026-04-08 21:16:48 +02:00
Author
Owner
Copy Link

🎨 Leonie Voss — UI/UX Design Lead

Questions & Observations

  • Sender display in document list — Currently the document list shows a single sender name per row. With multiple senders, how should this display? Options:

    • Comma-separated names: "Gerd de Gruyter, Brigitte de Gruyter" — simple but can get long
    • First sender + "+N" overflow pill: "Gerd de Gruyter +1" — compact, consistent with how we handle receiver overflow in PersonChipRow

    I'd recommend the overflow pill approach — it's already established in the receiver display and keeps the table column width manageable. The OverflowPillButton component already exists for this.

  • Document edit form — sender input — Switching from PersonTypeahead (single select) to PersonMultiSelect (chip-based multi-select) is the right call. The PersonMultiSelect component already handles the interaction pattern: typeahead search → select → chip appears → can remove chips. No new component needed.

  • Document detail view / metadata drawer — The sender currently displays as a single PersonChip. With multiple senders, use PersonChipRow (same as receivers). This makes the sender and receiver sections visually symmetric, which is cleaner.

  • Mobile consideration — Multiple sender chips in the metadata drawer need to wrap gracefully on small viewports. PersonChipRow already handles this with flex-wrap, so no additional work needed — just verify it at 320px.

Suggestions

  • Use the overflow pill pattern for document list display (consistent with receivers)
  • Use PersonMultiSelect in the edit form and PersonChipRow in detail/drawer views
  • No new components needed — this is entirely a reuse of existing patterns, making the sender/receiver UI symmetric
## 🎨 Leonie Voss — UI/UX Design Lead ### Questions & Observations - **Sender display in document list** — Currently the document list shows a single sender name per row. With multiple senders, how should this display? Options: - Comma-separated names: "Gerd de Gruyter, Brigitte de Gruyter" — simple but can get long - First sender + "+N" overflow pill: "Gerd de Gruyter +1" — compact, consistent with how we handle receiver overflow in `PersonChipRow` I'd recommend the overflow pill approach — it's already established in the receiver display and keeps the table column width manageable. The `OverflowPillButton` component already exists for this. - **Document edit form — sender input** — Switching from `PersonTypeahead` (single select) to `PersonMultiSelect` (chip-based multi-select) is the right call. The `PersonMultiSelect` component already handles the interaction pattern: typeahead search → select → chip appears → can remove chips. No new component needed. - **Document detail view / metadata drawer** — The sender currently displays as a single `PersonChip`. With multiple senders, use `PersonChipRow` (same as receivers). This makes the sender and receiver sections visually symmetric, which is cleaner. - **Mobile consideration** — Multiple sender chips in the metadata drawer need to wrap gracefully on small viewports. `PersonChipRow` already handles this with `flex-wrap`, so no additional work needed — just verify it at 320px. ### Suggestions - Use the overflow pill pattern for document list display (consistent with receivers) - Use `PersonMultiSelect` in the edit form and `PersonChipRow` in detail/drawer views - No new components needed — this is entirely a reuse of existing patterns, making the sender/receiver UI symmetric
marcel commented 2026-04-08 21:16:55 +02:00
Author
Owner
Copy Link

⚙️ Tobias Wendt — DevOps & Platform Engineer

Questions & Observations

  • Flyway migration on production data — The migration needs to run on the production database with 1500+ documents. The INSERT INTO document_senders SELECT id, sender_id FROM documents WHERE sender_id IS NOT NULL is a single-pass bulk insert — should complete in under a second for this data volume. No downtime concern, but the migration should be tested against a copy of production data before deploying.

  • No infrastructure changes — No new services, no config changes, no new environment variables. The Docker Compose setup is unaffected. The only thing that changes is the database schema (handled by Flyway) and the application code.

  • API type regeneration — The issue mentions npm run generate:api to regenerate TypeScript types. This requires the backend running with --spring.profiles.active=dev. Make sure the CI pipeline or the developer workflow includes this step after the backend changes are merged — stale types will cause frontend build failures.

Suggestions

  • No action needed from my side. Clean schema migration + application code change. Just ensure the Flyway migration is tested with realistic data volumes before production deployment.
## ⚙️ Tobias Wendt — DevOps & Platform Engineer ### Questions & Observations - **Flyway migration on production data** — The migration needs to run on the production database with 1500+ documents. The `INSERT INTO document_senders SELECT id, sender_id FROM documents WHERE sender_id IS NOT NULL` is a single-pass bulk insert — should complete in under a second for this data volume. No downtime concern, but the migration should be tested against a copy of production data before deploying. - **No infrastructure changes** — No new services, no config changes, no new environment variables. The Docker Compose setup is unaffected. The only thing that changes is the database schema (handled by Flyway) and the application code. - **API type regeneration** — The issue mentions `npm run generate:api` to regenerate TypeScript types. This requires the backend running with `--spring.profiles.active=dev`. Make sure the CI pipeline or the developer workflow includes this step after the backend changes are merged — stale types will cause frontend build failures. ### Suggestions - No action needed from my side. Clean schema migration + application code change. Just ensure the Flyway migration is tested with realistic data volumes before production deployment.
Sign in to join this conversation.
No Branch/Tag Specified
Branches Tags
Labels
Clear labels
P0-critical
Blocks a core user journey, causes data loss, or is a security/accessibility barrier. Work on this before P1+.
 P1-high
Significant friction on a main user journey; workaround exists but is non-obvious. Next up after P0.
 P2-medium
Noticeable improvement; doesn't block anything; schedule opportunistically.
 P3-later
Cosmetic or parking-lot work; fine to stay open indefinitely.
 audit
Read-only audit / assessment work; produces a report rather than changing code
 bug
Something isn't working
 cleanup
Removal of dead code, vague comments, naming violations, and scratch artifacts
 collaboration
We want to extend the family archive to have more collaboration tools
 conversation
descoped
We will do that later
 devops
documentation
README, ARCHITECTURE, GLOSSARY, CONTRIBUTING, per-domain docs
 epic
Marker for epic-level issues that group multiple child issues
 feature
file-upload
legibility
Codebase Legibility Refactor — preparing the codebase for human evaluation and bus-factor reduction
 notification
person
phase-1: security
Security hygiene — must be done first
 phase-2: container-images
Production-ready multi-stage Docker images
 phase-3: prod-compose
Production compose overlay + Caddy reverse proxy
 phase-4: spring-prod-profile
Spring Boot production configuration profile
 phase-5: backups
Database and object storage backup strategy
 phase-6: deployment-docs
.env.example, DEPLOYMENT.md, runbook
 phase-7: monitoring
Prometheus, Loki, Grafana, Alertmanager
 refactor
Code restructuring without behaviour change
 security
test
ui
UI/UX and accessibility issues
 user
No Label feature person
Milestone
No items
No Milestone
Projects
Clear projects
No project
Assignees
Clear assignees
Jens marcel
No Assignees
1 Participants
Notifications
Due Date
No due date set.
Dependencies

No dependencies set.

Reference: marcel/familienarchiv#216
Delete Branch "%!s()"

Deleting a branch is permanent. Although the deleted branch may continue to exist for a short time before it actually gets removed, it CANNOT be undone in most cases. Continue?