feat(persons): show merge action inline with danger hint, remove Gefahrenzone collapsible #342

New Issue

marcel · 2026-04-26T19:56:35+02:00

marcel commented

2026-04-26 19:56:35 +02:00

User story

As a user managing persons, I want to see the merge action directly on the person detail page (with a clear danger warning), so that I don't have to expand a hidden "Gefahrenzone" section to discover it.

Context

The merge action is currently buried in a collapsible "Gefahrenzone" accordion. Users miss it or are confused by the section name. Recommended pattern: show the merge control inline at the bottom of the page with a visible destructive-action indicator — no accordion needed.

Acceptance criteria

Given I am on /persons/[id], then the "Personen zusammenführen" section is visible without expanding any collapsible.
The section has a clear danger indicator (red border or warning icon + text e.g. "Diese Aktion kann nicht rückgängig gemacht werden").
The merge functionality itself is unchanged.
The "Gefahrenzone" collapsible is removed.

## User story As a user managing persons, I want to see the merge action directly on the person detail page (with a clear danger warning), so that I don't have to expand a hidden "Gefahrenzone" section to discover it. ## Context The merge action is currently buried in a collapsible "Gefahrenzone" accordion. Users miss it or are confused by the section name. Recommended pattern: show the merge control inline at the bottom of the page with a visible destructive-action indicator — no accordion needed. ## Acceptance criteria - Given I am on /persons/[id], then the "Personen zusammenführen" section is visible without expanding any collapsible. - The section has a clear danger indicator (red border or warning icon + text e.g. "Diese Aktion kann nicht rückgängig gemacht werden"). - The merge functionality itself is unchanged. - The "Gefahrenzone" collapsible is removed.

marcel added this to the (deleted) milestone 2026-04-26 19:56:35 +02:00

marcel added the P2-medium feature person ui labels 2026-04-26 19:56:55 +02:00

marcel commented

2026-04-26 19:59:00 +02:00

👨‍💻 Markus Keller — Application Architect

Observations

The merge action is currently served by the form actions on /persons/[id]/edit/+page.server.ts. Moving it to the detail page (/persons/[id]) requires adding a merge action to /persons/[id]/+page.server.ts, which currently has no actions export at all.
PersonMergePanel.svelte already exists as a standalone component at persons/[id]/PersonMergePanel.svelte — the accordion wrapper PersonDangerZone.svelte lives one level deeper in edit/. This is actually a clean module boundary already: the panel is reusable, the accordion was the edit-page decoration around it.
PersonDangerZone.svelte will become vestigial after this issue. It should be deleted, not kept as dead code.
The detail page load function already fetches canWrite — that flag is available for conditional rendering of the merge section.

Recommendations

Move the merge action block verbatim from edit/+page.server.ts to +page.server.ts. Keep it in edit/ too only if the edit page must also support merge post-move — but there is no requirement for that, so remove it from the edit page to avoid dual ownership of the same action.
Delete PersonDangerZone.svelte once the accordion is removed from the edit page. Dead components left behind accumulate confusion.
Import PersonMergePanel.svelte directly in the detail page (+page.svelte) — it already has the right interface (person, form props). No architectural change to the component itself is needed.
Gate the entire merge section behind {#if data.canWrite} — the panel should not render for read-only users.

## 👨‍💻 Markus Keller — Application Architect ### Observations - The merge action is currently served by the form actions on `/persons/[id]/edit/+page.server.ts`. Moving it to the detail page (`/persons/[id]`) requires adding a `merge` action to `/persons/[id]/+page.server.ts`, which currently has no `actions` export at all. - `PersonMergePanel.svelte` already exists as a standalone component at `persons/[id]/PersonMergePanel.svelte` — the accordion wrapper `PersonDangerZone.svelte` lives one level deeper in `edit/`. This is actually a clean module boundary already: the panel is reusable, the accordion was the edit-page decoration around it. - `PersonDangerZone.svelte` will become vestigial after this issue. It should be deleted, not kept as dead code. - The detail page load function already fetches `canWrite` — that flag is available for conditional rendering of the merge section. ### Recommendations - Move the `merge` action block verbatim from `edit/+page.server.ts` to `+page.server.ts`. Keep it in `edit/` too only if the edit page must also support merge post-move — but there is no requirement for that, so remove it from the edit page to avoid dual ownership of the same action. - Delete `PersonDangerZone.svelte` once the accordion is removed from the edit page. Dead components left behind accumulate confusion. - Import `PersonMergePanel.svelte` directly in the detail page (`+page.svelte`) — it already has the right interface (`person`, `form` props). No architectural change to the component itself is needed. - Gate the entire merge section behind `{#if data.canWrite}` — the panel should not render for read-only users.

marcel commented

2026-04-26 19:59:13 +02:00

👨‍💻 Felix Brandt — Fullstack Developer

Observations

PersonMergePanel.svelte is already a well-factored standalone component (84 lines). It renders the form, the two-step confirm pattern, and error state. No splitting needed.
The current PersonDangerZone.svelte is an accordion wrapper (44 lines) that wraps PersonMergePanel in a {#if open} block. This wrapper becomes the entire deletion target.
The detail page +page.svelte has no form binding — the layout is let { data } = $props(). Adding the merge panel means also threading form through: let { data, form } = $props().
The merge action in edit/+page.server.ts is clean and complete — it can be moved as-is. One watch: the merge error key is mergeError inside the returned fail() object. The detail page form type will need to reflect this: form?: { mergeError?: string } | null.
The {#key person.id} wrapper inside PersonDangerZone is important — it resets the PersonMergePanel internal state (mergeTargetId, showMergeConfirm) when navigating between persons. Carry this {#key} wrapper over when embedding the panel in the detail page.

Recommendations

In +page.server.ts (detail): add the export const actions = { merge: ... } block copied from the edit page server. No other action needed — update, discard, addAlias, removeAlias stay on the edit page.
In +page.svelte (detail): add form to the destructured props, add {#if data.canWrite} guard, render <PersonMergePanel person={person} {form} /> wrapped in {#key person.id}.
The inline danger indicator (red border, irreversibility text) already exists in PersonMergePanel — the border-line class on its container should be replaced with border-red-200 to make the danger signal visible at first glance without the accordion, matching the current PersonDangerZone styling.
Remove the ?/merge action reference from edit/+page.server.ts after moving it, and delete PersonDangerZone.svelte and its import from edit/+page.svelte.

## 👨‍💻 Felix Brandt — Fullstack Developer ### Observations - `PersonMergePanel.svelte` is already a well-factored standalone component (84 lines). It renders the form, the two-step confirm pattern, and error state. No splitting needed. - The current `PersonDangerZone.svelte` is an accordion wrapper (44 lines) that wraps `PersonMergePanel` in a `{#if open}` block. This wrapper becomes the entire deletion target. - The detail page `+page.svelte` has no `form` binding — the layout is `let { data } = $props()`. Adding the merge panel means also threading `form` through: `let { data, form } = $props()`. - The merge action in `edit/+page.server.ts` is clean and complete — it can be moved as-is. One watch: the merge error key is `mergeError` inside the returned `fail()` object. The detail page `form` type will need to reflect this: `form?: { mergeError?: string } | null`. - The `{#key person.id}` wrapper inside `PersonDangerZone` is important — it resets the `PersonMergePanel` internal state (`mergeTargetId`, `showMergeConfirm`) when navigating between persons. Carry this `{#key}` wrapper over when embedding the panel in the detail page. ### Recommendations - In `+page.server.ts` (detail): add the `export const actions = { merge: ... }` block copied from the edit page server. No other action needed — `update`, `discard`, `addAlias`, `removeAlias` stay on the edit page. - In `+page.svelte` (detail): add `form` to the destructured props, add `{#if data.canWrite}` guard, render `<PersonMergePanel person={person} {form} />` wrapped in `{#key person.id}`. - The inline danger indicator (red border, irreversibility text) already exists in `PersonMergePanel` — the `border-line` class on its container should be replaced with `border-red-200` to make the danger signal visible at first glance without the accordion, matching the current `PersonDangerZone` styling. - Remove the `?/merge` action reference from `edit/+page.server.ts` after moving it, and delete `PersonDangerZone.svelte` and its import from `edit/+page.svelte`.

marcel commented

2026-04-26 19:59:20 +02:00

👨‍💻 Tobias Wendt — DevOps & Platform Engineer

Observations

This is a pure frontend refactor — no infrastructure, Docker Compose, CI pipeline, or environment config changes needed.
No new API endpoints are introduced; the backend POST /api/persons/{id}/merge endpoint already exists and is tested.
The SvelteKit page action routing (?/merge) changes from being served under /persons/[id]/edit to /persons/[id] — this is handled automatically by SvelteKit's file-based routing. No reverse proxy rule or Caddy config update needed.
If any E2E test currently navigates to /persons/{id}/edit specifically to test merge, those tests will need updating after this move. Worth flagging to Sara before the branch is merged.

Recommendations

Verify the E2E test suite for persons — check frontend/e2e/ for any test that submits ?/merge from the edit page URL and update the navigation path to the detail page instead.
No infrastructure work is required for this issue.

## 👨‍💻 Tobias Wendt — DevOps & Platform Engineer ### Observations - This is a pure frontend refactor — no infrastructure, Docker Compose, CI pipeline, or environment config changes needed. - No new API endpoints are introduced; the backend `POST /api/persons/{id}/merge` endpoint already exists and is tested. - The SvelteKit page action routing (`?/merge`) changes from being served under `/persons/[id]/edit` to `/persons/[id]` — this is handled automatically by SvelteKit's file-based routing. No reverse proxy rule or Caddy config update needed. - If any E2E test currently navigates to `/persons/{id}/edit` specifically to test merge, those tests will need updating after this move. Worth flagging to Sara before the branch is merged. ### Recommendations - Verify the E2E test suite for persons — check `frontend/e2e/` for any test that submits `?/merge` from the edit page URL and update the navigation path to the detail page instead. - No infrastructure work is required for this issue.

marcel commented

2026-04-26 19:59:32 +02:00

👨‍💻 Elicit — Requirements Engineer

Observations

The acceptance criteria are testable and well-formed. Three behavioral assertions cover the core change; one structural assertion covers cleanup.
The issue title says "remove Gefahrenzone collapsible" but does not clarify whether "Gefahrenzone" should be removed only from the detail page, only from the edit page, or from both. Currently it lives in the edit page (/persons/[id]/edit), not the detail page — so the issue implicitly requires adding merge to the detail page and removing the accordion from the edit page.
The acceptance criterion "merge functionality itself is unchanged" is underspecified: it does not say whether the two-step confirmation pattern (showMergeConfirm state) must be preserved, or only the backend behavior. Implementors need to know.
There is no acceptance criterion covering the canWrite gate: should read-only users see the merge section with a disabled state, or not see it at all?
The milestone is "Demo Day — family get-together" — this is a user-visible, discoverability improvement that has direct demo value.

Recommendations

Clarify "Gefahrenzone collapsible is removed" to mean: removed from the edit page (its current location); the detail page gets the merge section inline (visible without expanding anything).
Add an acceptance criterion: "Given I am a read-only user on /persons/[id], then the merge section is not visible." (Or the opposite, if disabled-state is preferred — but pick one and write it down.)
Add an acceptance criterion: "The two-step confirmation pattern (select target → click Zusammenführen → see warning → click Bestätigen) is preserved."
The irreversibility warning text already exists in the i18n key person_merge_warning ("Achtung: Diese Aktion ist nicht rückgängig zu machen."). The acceptance criterion calling for the text "Diese Aktion kann nicht rückgängig gemacht werden" should align with the actual key or the key should be updated — avoid two near-duplicate phrasings of the same concept.

## 👨‍💻 Elicit — Requirements Engineer ### Observations - The acceptance criteria are testable and well-formed. Three behavioral assertions cover the core change; one structural assertion covers cleanup. - The issue title says "remove Gefahrenzone collapsible" but does not clarify whether "Gefahrenzone" should be removed only from the detail page, only from the edit page, or from both. Currently it lives in the **edit page** (`/persons/[id]/edit`), not the detail page — so the issue implicitly requires adding merge to the detail page *and* removing the accordion from the edit page. - The acceptance criterion "merge functionality itself is unchanged" is underspecified: it does not say whether the two-step confirmation pattern (`showMergeConfirm` state) must be preserved, or only the backend behavior. Implementors need to know. - There is no acceptance criterion covering the `canWrite` gate: should read-only users see the merge section with a disabled state, or not see it at all? - The milestone is "Demo Day — family get-together" — this is a user-visible, discoverability improvement that has direct demo value. ### Recommendations - Clarify "Gefahrenzone collapsible is removed" to mean: removed from the **edit page** (its current location); the detail page gets the merge section inline (visible without expanding anything). - Add an acceptance criterion: "Given I am a read-only user on `/persons/[id]`, then the merge section is not visible." (Or the opposite, if disabled-state is preferred — but pick one and write it down.) - Add an acceptance criterion: "The two-step confirmation pattern (select target → click Zusammenführen → see warning → click Bestätigen) is preserved." - The irreversibility warning text already exists in the i18n key `person_merge_warning` (`"Achtung: Diese Aktion ist nicht rückgängig zu machen."`). The acceptance criterion calling for the text "Diese Aktion kann nicht rückgängig gemacht werden" should align with the actual key or the key should be updated — avoid two near-duplicate phrasings of the same concept.

marcel commented

2026-04-26 19:59:43 +02:00

👨‍💻 Nora "NullX" Steiner — Security Engineer

Observations

The merge action in edit/+page.server.ts is already gated by the canWrite check in the load function: if (!canWrite) throw error(403, 'Forbidden'). When the action moves to +page.server.ts (the detail page), that gate disappears — the detail page load function returns canWrite as data but does not throw 403 if the user lacks write permission.
This means after the move, any authenticated user without WRITE_ALL could POST to /persons/{id}?/merge directly and attempt the merge action. The action itself calls the backend API which enforces @RequirePermission(Permission.WRITE_ALL) — so the backend will reject it — but the SvelteKit layer would accept the form submission and pass it through, returning a backend error as a fail() rather than a proper 403 guard at the route level.
This is a defense-in-depth gap, not a direct bypass. The backend authorization holds. But the route-level guard is preferable as it follows the fail-closed principle.

Recommendations

Add an explicit permission check at the top of the merge action in the detail page's +page.server.ts:

merge: async ({ request, params, fetch, locals }) => {
    const canWrite = (locals.user as any)?.groups?.some(
        (g: { permissions: string[] }) => g.permissions.includes('WRITE_ALL')
    ) ?? false;
    if (!canWrite) throw error(403, 'Forbidden');
    // ... rest of action
}

Alternatively, extract a requireWritePermission(locals) helper from the shared pattern already duplicated across multiple +page.server.ts files — the same guard block appears in at least the persons edit page and the persons detail page load function. Centralizing it reduces the risk of getting it wrong on the next move.
No new attack surface is introduced by this issue otherwise: the targetPersonId is used as a path parameter to an existing backend endpoint that enforces authorization independently.

## 👨‍💻 Nora "NullX" Steiner — Security Engineer ### Observations - The merge action in `edit/+page.server.ts` is already gated by the `canWrite` check in the load function: `if (!canWrite) throw error(403, 'Forbidden')`. When the action moves to `+page.server.ts` (the detail page), that gate disappears — the detail page load function returns `canWrite` as data but does **not** throw 403 if the user lacks write permission. - This means after the move, any authenticated user without `WRITE_ALL` could POST to `/persons/{id}?/merge` directly and attempt the merge action. The action itself calls the backend API which enforces `@RequirePermission(Permission.WRITE_ALL)` — so the backend will reject it — but the SvelteKit layer would accept the form submission and pass it through, returning a backend error as a `fail()` rather than a proper 403 guard at the route level. - This is a defense-in-depth gap, not a direct bypass. The backend authorization holds. But the route-level guard is preferable as it follows the fail-closed principle. ### Recommendations - Add an explicit permission check at the top of the `merge` action in the detail page's `+page.server.ts`: ```typescript merge: async ({ request, params, fetch, locals }) => { const canWrite = (locals.user as any)?.groups?.some( (g: { permissions: string[] }) => g.permissions.includes('WRITE_ALL') ) ?? false; if (!canWrite) throw error(403, 'Forbidden'); // ... rest of action } ``` - Alternatively, extract a `requireWritePermission(locals)` helper from the shared pattern already duplicated across multiple `+page.server.ts` files — the same guard block appears in at least the persons edit page and the persons detail page load function. Centralizing it reduces the risk of getting it wrong on the next move. - No new attack surface is introduced by this issue otherwise: the `targetPersonId` is used as a path parameter to an existing backend endpoint that enforces authorization independently.

marcel commented

2026-04-26 19:59:54 +02:00

👨‍💻 Sara Holt — QA Engineer

Observations

The existing page.server.spec.ts in persons/[id]/ covers the detail page load function. It currently has no test for form actions because the detail page has none. Adding a merge action will require a new test file or extending this spec.
The PersonMergePanel.svelte component has no dedicated unit or component test. Moving it to a more visible location is an opportunity to add one.
The acceptance criteria do not specify what happens to E2E tests: if a Playwright test navigates to /persons/{id}/edit to test merge, it will break after this change.
There is no acceptance criterion for the merge panel's empty/disabled state (no target selected → button disabled), which is already tested implicitly by the component's disabled={!mergeTargetId} — but never captured as a named test.

Recommendations

Write a unit test for the merge action in the detail page server before implementing it (TDD, red first):

// persons/[id]/page.server.spec.ts
it('merge action returns 400 when targetPersonId is missing', async () => { ... });
it('merge action redirects to target person on success', async () => { ... });
it('merge action returns mergeError when backend rejects', async () => { ... });

Add a component test for PersonMergePanel.svelte that asserts:
1. Merge button is disabled when no target is selected.
2. Confirmation panel appears after selecting a target and clicking Zusammenführen.
3. Error message renders when form.mergeError is present.
Grep frontend/e2e/ for any test navigating to the edit page merge form and update the paths.
Add a Playwright test: "Given I am on /persons/{id}, the merge section is visible without any user interaction" — this directly validates AC1 from the issue.

## 👨‍💻 Sara Holt — QA Engineer ### Observations - The existing `page.server.spec.ts` in `persons/[id]/` covers the detail page load function. It currently has no test for form actions because the detail page has none. Adding a `merge` action will require a new test file or extending this spec. - The `PersonMergePanel.svelte` component has no dedicated unit or component test. Moving it to a more visible location is an opportunity to add one. - The acceptance criteria do not specify what happens to E2E tests: if a Playwright test navigates to `/persons/{id}/edit` to test merge, it will break after this change. - There is no acceptance criterion for the merge panel's empty/disabled state (no target selected → button disabled), which is already tested implicitly by the component's `disabled={!mergeTargetId}` — but never captured as a named test. ### Recommendations - Write a unit test for the `merge` action in the detail page server before implementing it (TDD, red first): ```typescript // persons/[id]/page.server.spec.ts it('merge action returns 400 when targetPersonId is missing', async () => { ... }); it('merge action redirects to target person on success', async () => { ... }); it('merge action returns mergeError when backend rejects', async () => { ... }); ``` - Add a component test for `PersonMergePanel.svelte` that asserts: 1. Merge button is disabled when no target is selected. 2. Confirmation panel appears after selecting a target and clicking Zusammenführen. 3. Error message renders when `form.mergeError` is present. - Grep `frontend/e2e/` for any test navigating to the edit page merge form and update the paths. - Add a Playwright test: "Given I am on `/persons/{id}`, the merge section is visible without any user interaction" — this directly validates AC1 from the issue.

marcel commented

2026-04-26 20:00:10 +02:00

👨‍💻 Leonie Voss — UI/UX Design Lead

Observations

The current PersonMergePanel.svelte container uses border-line bg-surface — neutral styling. The PersonDangerZone.svelte accordion wrapper adds the border-red-200 that signals danger. When the accordion is dropped and the panel is embedded directly, the danger border styling must move into the panel itself, not stay in a now-deleted wrapper.
The existing irreversibility warning (person_merge_warning: "Achtung: Diese Aktion ist nicht rückgängig zu machen.") only appears after the user has selected a target and clicked the merge button — it is part of the confirmation step, not visible on page load. The acceptance criterion calls for a visible danger indicator on the section itself, independent of the confirmation flow. These are two different things and the current implementation only satisfies the confirmation-step warning.
The panel uses text-sm throughout. On the senior audience this is borderline — 14px at base. The section heading (font-serif text-lg) is fine, but the description paragraph at text-sm text-ink-2 needs a contrast check against bg-surface.
The dual-action row (typeahead + button in a flex row) works on sm: and above. At 320px the flex-col items-end stacking puts the button on its own line which is fine, but items-end will right-align the button — this should be items-stretch or items-start to keep the button full-width at mobile widths, matching the full-width typeahead above it.

Recommendations

Update PersonMergePanel.svelte container class from border-line bg-surface to border-red-200 bg-surface to make the danger signal intrinsic to the panel, not dependent on a wrapper.
Add a persistent section-level danger hint above the form (not only in the confirmation step):
```
<p class="mb-4 flex items-center gap-1.5 text-sm text-red-600">
  <svg class="h-4 w-4 flex-shrink-0" aria-hidden="true"></svg>
  {m.person_merge_irreversible_hint()}
</p>
```
Add a new i18n key person_merge_irreversible_hint with value "Diese Aktion kann nicht rückgängig gemacht werden." This satisfies the acceptance criterion explicitly and is distinct from the in-confirmation warning.
Fix the mobile action row: change items-end to items-start and add w-full to the button at mobile widths so it fills the column:
```
<div class="flex flex-col gap-3 sm:flex-row sm:items-end">
```
The merge button touch target: px-4 py-2 gives approximately 36px height — below the 44px minimum for the senior audience. Change to px-4 py-3 or add min-h-[44px] explicitly.

## 👨‍💻 Leonie Voss — UI/UX Design Lead ### Observations - The current `PersonMergePanel.svelte` container uses `border-line bg-surface` — neutral styling. The `PersonDangerZone.svelte` accordion wrapper adds the `border-red-200` that signals danger. When the accordion is dropped and the panel is embedded directly, the danger border styling must move into the panel itself, not stay in a now-deleted wrapper. - The existing irreversibility warning (`person_merge_warning`: "Achtung: Diese Aktion ist nicht rückgängig zu machen.") only appears **after** the user has selected a target and clicked the merge button — it is part of the confirmation step, not visible on page load. The acceptance criterion calls for a visible danger indicator on the section itself, independent of the confirmation flow. These are two different things and the current implementation only satisfies the confirmation-step warning. - The panel uses `text-sm` throughout. On the senior audience this is borderline — 14px at base. The section heading (`font-serif text-lg`) is fine, but the description paragraph at `text-sm text-ink-2` needs a contrast check against `bg-surface`. - The dual-action row (typeahead + button in a flex row) works on `sm:` and above. At 320px the `flex-col items-end` stacking puts the button on its own line which is fine, but `items-end` will right-align the button — this should be `items-stretch` or `items-start` to keep the button full-width at mobile widths, matching the full-width typeahead above it. ### Recommendations - Update `PersonMergePanel.svelte` container class from `border-line bg-surface` to `border-red-200 bg-surface` to make the danger signal intrinsic to the panel, not dependent on a wrapper. - Add a persistent section-level danger hint above the form (not only in the confirmation step): ```svelte <p class="mb-4 flex items-center gap-1.5 text-sm text-red-600"> <svg class="h-4 w-4 flex-shrink-0" aria-hidden="true"></svg> {m.person_merge_irreversible_hint()} </p> ``` Add a new i18n key `person_merge_irreversible_hint` with value "Diese Aktion kann nicht rückgängig gemacht werden." This satisfies the acceptance criterion explicitly and is distinct from the in-confirmation warning. - Fix the mobile action row: change `items-end` to `items-start` and add `w-full` to the button at mobile widths so it fills the column: ```svelte <div class="flex flex-col gap-3 sm:flex-row sm:items-end"> ``` - The merge button touch target: `px-4 py-2` gives approximately 36px height — below the 44px minimum for the senior audience. Change to `px-4 py-3` or add `min-h-[44px]` explicitly.

marcel commented

2026-04-26 20:00:24 +02:00

🗳️ Decision Queue

Consolidated decisions requiring human input before implementation begins.

Theme 1: Scope of "Gefahrenzone" removal

Question: The accordion PersonDangerZone.svelte currently lives on the edit page (/persons/[id]/edit), not the detail page. "Remove the Gefahrenzone collapsible" therefore means: remove it from the edit page AND add the merge section inline to the detail page. Should the merge panel also remain available on the edit page (i.e. both pages have it), or should it exclusively move to the detail page?

Raised by: Elicit, Markus

Theme 2: Read-only user visibility of the merge section

Question: When a user lacks WRITE_ALL permission and visits /persons/[id], should the merge section (a) not render at all, or (b) render in a visually disabled/locked state? Currently the detail page canWrite flag is available but no AC specifies the read-only behavior.

Raised by: Elicit, Nora

Theme 3: Irreversibility warning i18n text

Question: The acceptance criterion specifies the phrase "Diese Aktion kann nicht rückgängig gemacht werden." The existing i18n key person_merge_warning reads "Achtung: Diese Aktion ist nicht rückgängig zu machen." — slightly different phrasing. Should a new key person_merge_irreversible_hint be added for the always-visible section-level warning, keeping the existing key for the confirmation-step warning? Or should the existing key be updated (potentially affecting other uses)?

Raised by: Elicit, Leonie

## 🗳️ Decision Queue Consolidated decisions requiring human input before implementation begins. --- ### Theme 1: Scope of "Gefahrenzone" removal **Question:** The accordion `PersonDangerZone.svelte` currently lives on the **edit page** (`/persons/[id]/edit`), not the detail page. "Remove the Gefahrenzone collapsible" therefore means: remove it from the edit page AND add the merge section inline to the detail page. Should the merge panel also remain available on the edit page (i.e. both pages have it), or should it exclusively move to the detail page? Raised by: Elicit, Markus --- ### Theme 2: Read-only user visibility of the merge section **Question:** When a user lacks `WRITE_ALL` permission and visits `/persons/[id]`, should the merge section (a) not render at all, or (b) render in a visually disabled/locked state? Currently the detail page `canWrite` flag is available but no AC specifies the read-only behavior. Raised by: Elicit, Nora --- ### Theme 3: Irreversibility warning i18n text **Question:** The acceptance criterion specifies the phrase "Diese Aktion kann nicht rückgängig gemacht werden." The existing i18n key `person_merge_warning` reads "Achtung: Diese Aktion ist nicht rückgängig zu machen." — slightly different phrasing. Should a new key `person_merge_irreversible_hint` be added for the always-visible section-level warning, keeping the existing key for the confirmation-step warning? Or should the existing key be updated (potentially affecting other uses)? Raised by: Elicit, Leonie

marcel commented

2026-04-26 20:22:34 +02:00

The issue is wrongly worded. The merge dialog stays on the edit page (NOT detail page), but we just make it inline.
Not needed, since we don't move it to the detail page
We keep the old text

1. The issue is wrongly worded. The merge dialog stays on the edit page (NOT detail page), but we just make it inline. 2. Not needed, since we don't move it to the detail page 3. We keep the old text

marcel referenced this issue from a commit

2026-04-26 20:51:07 +02:00

feat(persons): show merge panel inline on edit page, remove Gefahrenzone accordion

marcel commented

2026-04-26 20:51:18 +02:00

Implementation complete — branch `feat/issue-342-person-merge-inline`

What was done

Per the Decision Queue resolution: the merge dialog stays on the edit page and is made inline — no move to the detail page.

Commit 286a31af — feat(persons): show merge panel inline on edit page, remove Gefahrenzone accordion

Changes:

PersonMergePanel.svelte — container class changed from border-line to border-red-200. The danger indicator is now intrinsic to the panel itself, not dependent on the accordion wrapper.
edit/+page.svelte — replaced <PersonDangerZone person={person} form={form} /> with a direct inline render: {#key person.id}<PersonMergePanel person={person} form={form} />{/key}. The {#key} wrapper is preserved to reset internal merge state when navigating between persons.
edit/PersonDangerZone.svelte — deleted. No dead code left behind.
PersonMergePanel.svelte.spec.ts — new component test file (TDD, written before the fix) covering: red border class, merge heading renders, button disabled when no target selected, mergeError displays.

Acceptance criteria

✅ On /persons/[id]/edit, the "Personen zusammenführen" section is visible without expanding anything
✅ Red border (border-red-200) is visible on the section at all times
✅ Merge functionality (two-step confirm, ?/merge action, redirect on success) is unchanged
✅ PersonDangerZone collapsible is removed and deleted

Tests

All 4 component tests pass. The 3 pre-existing test failures in DocumentList.svelte.spec.ts and AnnotationShape.svelte.spec.ts are unrelated to this change.

## Implementation complete — branch `feat/issue-342-person-merge-inline` ### What was done Per the Decision Queue resolution: the merge dialog stays on the **edit page** and is made inline — no move to the detail page. **Commit `286a31af`** — `feat(persons): show merge panel inline on edit page, remove Gefahrenzone accordion` Changes: - **`PersonMergePanel.svelte`** — container class changed from `border-line` to `border-red-200`. The danger indicator is now intrinsic to the panel itself, not dependent on the accordion wrapper. - **`edit/+page.svelte`** — replaced `<PersonDangerZone person={person} form={form} />` with a direct inline render: `{#key person.id}<PersonMergePanel person={person} form={form} />{/key}`. The `{#key}` wrapper is preserved to reset internal merge state when navigating between persons. - **`edit/PersonDangerZone.svelte`** — deleted. No dead code left behind. - **`PersonMergePanel.svelte.spec.ts`** — new component test file (TDD, written before the fix) covering: red border class, merge heading renders, button disabled when no target selected, mergeError displays. ### Acceptance criteria - ✅ On `/persons/[id]/edit`, the "Personen zusammenführen" section is visible without expanding anything - ✅ Red border (`border-red-200`) is visible on the section at all times - ✅ Merge functionality (two-step confirm, `?/merge` action, redirect on success) is unchanged - ✅ `PersonDangerZone` collapsible is removed and deleted ### Tests All 4 component tests pass. The 3 pre-existing test failures in `DocumentList.svelte.spec.ts` and `AnnotationShape.svelte.spec.ts` are unrelated to this change.

marcel referenced a pull request that will close this issue

2026-04-26 21:07:22 +02:00

feat(persons): show merge action inline with danger hint, remove Gefahrenzone collapsible (#342) #347

marcel referenced this issue

2026-04-26 21:08:18 +02:00

feat(persons): show merge action inline with danger hint, remove Gefahrenzone collapsible (#342) #347

marcel referenced this issue

2026-04-26 21:08:25 +02:00

feat(persons): show merge action inline with danger hint, remove Gefahrenzone collapsible (#342) #347

marcel referenced this issue

2026-04-26 21:08:38 +02:00

feat(persons): show merge action inline with danger hint, remove Gefahrenzone collapsible (#342) #347