feat(admin): assign groups when creating an invite link #566

New Issue

marcel · 2026-05-14T11:41:11+02:00

marcel commented

2026-05-14 11:41:11 +02:00

User story

As an admin, I want to select one or more groups when creating an invite link, so that the invited user is immediately placed in the right group (e.g. WRITE_ALL) the moment they claim their account — without needing a second manual step.

Current behaviour

The "Neue Einladung" form only accepts label, max uses, expiry, and prefill name/email. There is no group picker. After the invited user registers, they have no group and therefore no permissions. The admin must then open the user profile and assign groups manually.

Desired behaviour

The invite creation form shows a checkbox list of all groups (identical to the one used when creating a user directly). Any checked groups are stored on the InviteToken and assigned to the AppUser automatically on registration.

Acceptance criteria

Given I am on /admin/invites and open the "Neue Einladung" form
When the form loads
Then a "Gruppen (optional)" section appears with a checkbox per group

Given I create an invite with the "WRITE_ALL" group checked
When the invited person redeems the link and registers
Then their account immediately has the WRITE_ALL group
And no additional admin step is required

Given I create an invite with no groups checked
When the invited person redeems the link
Then their account has no groups (existing behaviour preserved)

Implementation notes

Backend — no changes needed.
CreateInviteRequest already has groupIds: List<UUID> and InviteService.redeemInvite() already passes token.getGroupIds() to userService.createUser().

Frontend — 3 files + i18n:

`frontend/src/routes/admin/invites/+page.server.ts`

load: fetch groups in parallel with the invites list:

import { createApiClient } from '$lib/shared/api.server';

// inside load():
const api = createApiClient(fetch);
const [invitesRes, groupsResult] = await Promise.all([
    fetch(`${apiUrl}/api/invites?status=...`),
    api.GET('/api/groups')
]);
// return groups: groupsResult.data ?? []

create action: collect and forward groupIds:

const groupIds = formData.getAll('groupIds') as string[];
// add to JSON body

`frontend/src/routes/admin/invites/+page.svelte`

Import UserGroupsSection from $lib/user/UserGroupsSection.svelte
Add groups to the data type
Insert inside the form grid (after the prefill-email field, before submit row):

<div class="sm:col-span-2">
    <p class="mb-2 font-sans text-xs font-bold tracking-widest text-ink-2 uppercase">
        {m.admin_new_invite_groups()}
    </p>
    <UserGroupsSection groups={data.groups} />
</div>

i18n (`frontend/messages/de.json`, `en.json`, `es.json`)

Add after "admin_new_invite_expires":

File	Key	Value
de.json	`admin_new_invite_groups`	`"Gruppen (optional)"`
en.json	`admin_new_invite_groups`	`"Groups (optional)"`
es.json	`admin_new_invite_groups`	`"Grupos (opcional)"`

Verification

docker-compose up -d
/admin/invites → open the new invite form → groups checkboxes visible
Create invite with one group checked → copy link → open in incognito → register
Admin: /admin/users → new user already has the selected group
cd frontend && npm run check — no type errors

## User story As an admin, I want to select one or more groups when creating an invite link, so that the invited user is immediately placed in the right group (e.g. WRITE_ALL) the moment they claim their account — without needing a second manual step. --- ## Current behaviour The "Neue Einladung" form only accepts label, max uses, expiry, and prefill name/email. There is no group picker. After the invited user registers, they have no group and therefore no permissions. The admin must then open the user profile and assign groups manually. --- ## Desired behaviour The invite creation form shows a checkbox list of all groups (identical to the one used when creating a user directly). Any checked groups are stored on the `InviteToken` and assigned to the `AppUser` automatically on registration. --- ## Acceptance criteria ```gherkin Given I am on /admin/invites and open the "Neue Einladung" form When the form loads Then a "Gruppen (optional)" section appears with a checkbox per group Given I create an invite with the "WRITE_ALL" group checked When the invited person redeems the link and registers Then their account immediately has the WRITE_ALL group And no additional admin step is required Given I create an invite with no groups checked When the invited person redeems the link Then their account has no groups (existing behaviour preserved) ``` --- ## Implementation notes **Backend — no changes needed.** `CreateInviteRequest` already has `groupIds: List<UUID>` and `InviteService.redeemInvite()` already passes `token.getGroupIds()` to `userService.createUser()`. **Frontend — 3 files + i18n:** ### `frontend/src/routes/admin/invites/+page.server.ts` `load`: fetch groups in parallel with the invites list: ```ts import { createApiClient } from '$lib/shared/api.server'; // inside load(): const api = createApiClient(fetch); const [invitesRes, groupsResult] = await Promise.all([ fetch(`${apiUrl}/api/invites?status=...`), api.GET('/api/groups') ]); // return groups: groupsResult.data ?? [] ``` `create` action: collect and forward groupIds: ```ts const groupIds = formData.getAll('groupIds') as string[]; // add to JSON body ``` ### `frontend/src/routes/admin/invites/+page.svelte` - Import `UserGroupsSection` from `$lib/user/UserGroupsSection.svelte` - Add `groups` to the `data` type - Insert inside the form grid (after the prefill-email field, before submit row): ```svelte <div class="sm:col-span-2"> <p class="mb-2 font-sans text-xs font-bold tracking-widest text-ink-2 uppercase"> {m.admin_new_invite_groups()} </p> <UserGroupsSection groups={data.groups} /> </div> ``` ### i18n (`frontend/messages/de.json`, `en.json`, `es.json`) Add after `"admin_new_invite_expires"`: | File | Key | Value | |------|-----|-------| | de.json | `admin_new_invite_groups` | `"Gruppen (optional)"` | | en.json | `admin_new_invite_groups` | `"Groups (optional)"` | | es.json | `admin_new_invite_groups` | `"Grupos (opcional)"` | --- ## Verification 1. `docker-compose up -d` 2. `/admin/invites` → open the new invite form → groups checkboxes visible 3. Create invite with one group checked → copy link → open in incognito → register 4. Admin: `/admin/users` → new user already has the selected group 5. `cd frontend && npm run check` — no type errors

marcel added the P2-medium feature user labels 2026-05-14 11:41:14 +02:00

marcel commented

2026-05-14 11:53:03 +02:00

👨‍💻 Felix Brandt — Senior Fullstack Developer

Observations

Backend is fully implemented (CreateInviteRequest.groupIds, InviteService.redeemInvite() → userService.createUser() with groups). Zero backend work needed — this is a clean frontend-only change.
The /admin/users/new route is the exact reference implementation for this pattern: load() calls api.GET('/api/groups'), UserGroupsSection renders the checkboxes, formData.getAll('groupIds') collects the values. The issue correctly identifies this.
API error handling gap in the load function: the issue snippet uses groupsResult.data ?? [] but doesn't check !groupsResult.response.ok first. Per project conventions, the pattern is: check response.ok, then access data!. Using data ?? [] silently swallows a 403 or 500, which would make the groups section silently disappear in production without any indication of failure.
Inconsistent fetch style in the load function: the suggested code mixes raw fetch(...) for invites with api.GET(...) for groups inside Promise.all. Look at what the existing +page.server.ts already does for invites — if it already uses createApiClient, use that for both. If it uses raw fetch, either keep consistency or migrate both in the same PR.
No tests mentioned in the issue. Three test cases are needed before touching the implementation:
1. load() returns groups alongside invites (mock api.GET('/api/groups'), assert groups in return value)
2. create action extracts groupIds via formData.getAll('groupIds') and forwards them in the JSON body
3. create action sends groupIds: [] when no checkboxes are checked (preserves existing no-group behavior)

Recommendations

Fix the load function to guard properly:

const groupsResult = await api.GET('/api/groups');
if (!groupsResult.response.ok) {
  // groups are optional in this context — log but don't crash
  return { invites: ..., groups: [] };
}
return { invites: ..., groups: groupsResult.data! };

Write the three failing tests listed above before touching +page.server.ts or +page.svelte.
Keep UserGroupsSection import exactly as used in /admin/users/new — no wrapper, no abstraction. Reuse as-is.

## 👨‍💻 Felix Brandt — Senior Fullstack Developer ### Observations - Backend is fully implemented (`CreateInviteRequest.groupIds`, `InviteService.redeemInvite()` → `userService.createUser()` with groups). Zero backend work needed — this is a clean frontend-only change. - The `/admin/users/new` route is the exact reference implementation for this pattern: `load()` calls `api.GET('/api/groups')`, `UserGroupsSection` renders the checkboxes, `formData.getAll('groupIds')` collects the values. The issue correctly identifies this. - **API error handling gap in the load function**: the issue snippet uses `groupsResult.data ?? []` but doesn't check `!groupsResult.response.ok` first. Per project conventions, the pattern is: check `response.ok`, then access `data!`. Using `data ?? []` silently swallows a 403 or 500, which would make the groups section silently disappear in production without any indication of failure. - **Inconsistent fetch style in the load function**: the suggested code mixes raw `fetch(...)` for invites with `api.GET(...)` for groups inside `Promise.all`. Look at what the existing `+page.server.ts` already does for invites — if it already uses `createApiClient`, use that for both. If it uses raw `fetch`, either keep consistency or migrate both in the same PR. - **No tests mentioned in the issue.** Three test cases are needed before touching the implementation: 1. `load()` returns `groups` alongside invites (mock `api.GET('/api/groups')`, assert `groups` in return value) 2. `create` action extracts `groupIds` via `formData.getAll('groupIds')` and forwards them in the JSON body 3. `create` action sends `groupIds: []` when no checkboxes are checked (preserves existing no-group behavior) ### Recommendations - Fix the load function to guard properly: ```typescript const groupsResult = await api.GET('/api/groups'); if (!groupsResult.response.ok) { // groups are optional in this context — log but don't crash return { invites: ..., groups: [] }; } return { invites: ..., groups: groupsResult.data! }; ``` - Write the three failing tests listed above **before** touching `+page.server.ts` or `+page.svelte`. - Keep `UserGroupsSection` import exactly as used in `/admin/users/new` — no wrapper, no abstraction. Reuse as-is.

marcel commented

2026-05-14 11:53:16 +02:00

🏗️ Markus Keller — Application Architect

Observations

No schema changes, no migrations, no new packages. InviteToken already carries a Set<UUID> groupIds via @ElementCollection with eager fetch. The backend service layer is complete. This is a frontend-only PR with near-zero architectural surface.
Layer boundaries are clean: the groups list flows from GET /api/groups → server load function → component props. No client-side API calls, no cross-domain repository access.
Fetch inconsistency in the suggested load code: the snippet shows fetch(apiUrl + '/api/invites?...') (raw fetch) alongside api.GET('/api/groups') (typed client) inside the same Promise.all. This mixes two fetch styles in one function. The typed client should be used for both, or neither — the existing invite fetch in the file dictates which direction to standardize.
The GET /api/groups endpoint requires ADMIN_PERMISSION: the invite creation page is admin-only, so the server load runs with the admin session cookie. This works correctly. No special wiring needed.
No documentation triggers: this change adds no new routes, no new backend domain modules, no new Docker services, no new ErrorCodes, no new Permissions. The architecture docs (CLAUDE.md, C4 diagrams) do not need updating.
The pattern of reusing UserGroupsSection.svelte across two admin forms is appropriate — it's not accidental duplication, it's a shared domain component.

Recommendations

Standardize the fetch approach: look at what the current +page.server.ts already uses for the invites list and match it for the groups fetch. Don't introduce a second style.
No ADR needed — this is a UI addition to an existing backend capability, not an architectural decision.
Consider whether the groups return value from load() should be typed explicitly with the generated UserGroup[] type. That keeps TypeScript's compile-time coverage strong and avoids relying on inference through the data ?? [] fallback.

## 🏗️ Markus Keller — Application Architect ### Observations - **No schema changes, no migrations, no new packages.** `InviteToken` already carries a `Set<UUID> groupIds` via `@ElementCollection` with eager fetch. The backend service layer is complete. This is a frontend-only PR with near-zero architectural surface. - **Layer boundaries are clean**: the groups list flows from `GET /api/groups` → server load function → component props. No client-side API calls, no cross-domain repository access. - **Fetch inconsistency in the suggested load code**: the snippet shows `fetch(apiUrl + '/api/invites?...')` (raw fetch) alongside `api.GET('/api/groups')` (typed client) inside the same `Promise.all`. This mixes two fetch styles in one function. The typed client should be used for both, or neither — the existing invite fetch in the file dictates which direction to standardize. - **The `GET /api/groups` endpoint requires `ADMIN_PERMISSION`**: the invite creation page is admin-only, so the server load runs with the admin session cookie. This works correctly. No special wiring needed. - **No documentation triggers**: this change adds no new routes, no new backend domain modules, no new Docker services, no new ErrorCodes, no new Permissions. The architecture docs (`CLAUDE.md`, C4 diagrams) do not need updating. - The pattern of reusing `UserGroupsSection.svelte` across two admin forms is appropriate — it's not accidental duplication, it's a shared domain component. ### Recommendations - Standardize the fetch approach: look at what the current `+page.server.ts` already uses for the invites list and match it for the groups fetch. Don't introduce a second style. - No ADR needed — this is a UI addition to an existing backend capability, not an architectural decision. - Consider whether the `groups` return value from `load()` should be typed explicitly with the generated `UserGroup[]` type. That keeps TypeScript's compile-time coverage strong and avoids relying on inference through the `data ?? []` fallback.

marcel commented

2026-05-14 11:53:31 +02:00

🔐 Nora "NullX" Steiner — Application Security Engineer

Observations

Group UUID validation is server-side and correct: InviteService.createInvite() calls userService.findGroupsByIds(groupIds) which validates that the submitted UUIDs correspond to real groups before storing them on the token. An attacker cannot inject arbitrary UUIDs via the form and get them silently persisted.
Authorization on the groups endpoint: GET /api/groups is protected by ADMIN_PERMISSION. The SvelteKit server load function runs with the admin session cookie, so this is correctly authorized. The browser never directly calls /api/groups.
No mass assignment risk: the form collects groupIds as an explicit string array via formData.getAll('groupIds') — same pattern as /admin/users/new. Only UUID strings can appear here (checkbox values are set by the server-rendered group list, not free-form input). No risk of unexpected field injection.
Privilege escalation by design: an admin with invite creation rights can assign any group — including ADMIN — to an invited user. This is intentional: only admins reach this form. Verify that invite creation requires a specific permission (@RequirePermission on the create action in the server file, or equivalent). If creating an invite requires only being logged in (not ADMIN specifically), a non-admin user who somehow reaches the form could escalate privileges.
No audit trail mentioned: invite creation with groups is a privilege-granting event. Check whether InviteService.createInvite() emits an audit log entry and whether it includes the assigned groupIds. If not, this is a gap — an admin assigning ADMIN to an invite token should be auditable.
No XSS surface: checkbox values are UUIDs from the server-rendered groups list. No user-typed text reaches the DOM through this flow.

Recommendations

Before implementing, confirm: what @RequirePermission annotation guards the invite creation endpoint (POST /api/invites)? It should be ADMIN or ADMIN_USER — WRITE_ALL would be too permissive for a privilege-granting action.
Add a @WebMvcTest test: POST /api/invites with a user carrying only WRITE_ALL should return 403. This test should exist regardless of this issue, but adding group assignment makes the permission boundary more consequential.
Check the audit log output of InviteService.createInvite() and verify that groupIds appears in the log entry.

## 🔐 Nora "NullX" Steiner — Application Security Engineer ### Observations - **Group UUID validation is server-side and correct**: `InviteService.createInvite()` calls `userService.findGroupsByIds(groupIds)` which validates that the submitted UUIDs correspond to real groups before storing them on the token. An attacker cannot inject arbitrary UUIDs via the form and get them silently persisted. - **Authorization on the groups endpoint**: `GET /api/groups` is protected by `ADMIN_PERMISSION`. The SvelteKit server load function runs with the admin session cookie, so this is correctly authorized. The browser never directly calls `/api/groups`. - **No mass assignment risk**: the form collects `groupIds` as an explicit string array via `formData.getAll('groupIds')` — same pattern as `/admin/users/new`. Only UUID strings can appear here (checkbox values are set by the server-rendered group list, not free-form input). No risk of unexpected field injection. - **Privilege escalation by design**: an admin with invite creation rights can assign any group — including `ADMIN` — to an invited user. This is intentional: only admins reach this form. **Verify that invite creation requires a specific permission** (`@RequirePermission` on the `create` action in the server file, or equivalent). If creating an invite requires only being logged in (not `ADMIN` specifically), a non-admin user who somehow reaches the form could escalate privileges. - **No audit trail mentioned**: invite creation with groups is a privilege-granting event. Check whether `InviteService.createInvite()` emits an audit log entry and whether it includes the assigned `groupIds`. If not, this is a gap — an admin assigning `ADMIN` to an invite token should be auditable. - **No XSS surface**: checkbox values are UUIDs from the server-rendered groups list. No user-typed text reaches the DOM through this flow. ### Recommendations - Before implementing, confirm: what `@RequirePermission` annotation guards the invite creation endpoint (`POST /api/invites`)? It should be `ADMIN` or `ADMIN_USER` — `WRITE_ALL` would be too permissive for a privilege-granting action. - Add a `@WebMvcTest` test: `POST /api/invites` with a user carrying only `WRITE_ALL` should return 403. This test should exist regardless of this issue, but adding group assignment makes the permission boundary more consequential. - Check the audit log output of `InviteService.createInvite()` and verify that `groupIds` appears in the log entry.

marcel commented

2026-05-14 11:53:44 +02:00

🧪 Sara Holt — QA Engineer

Observations

The issue has well-written Gherkin acceptance criteria covering: form renders groups, invite with group → user has group on registration, invite without group → user has no group. That's a solid AC baseline.
Zero tests mentioned in the issue or verification steps. Verification step 2 is a manual click-through, not an automated test. This feature touches the registration flow — any regression here would give a newly invited user wrong permissions without any automated safety net.
Missing AC: what happens when the groups API is unavailable at form load time? The form should still render and allow invite creation without groups (optional field). Without an explicit AC for this, the implementation might throw an error that breaks the entire invite form.
Missing edge case AC: what if the same group is somehow submitted twice (client-side manipulation)? The backend should deduplicate or the DB constraint should handle it — but neither the issue nor the ACs address this.

Test plan (what should be written before implementation)

Unit (SvelteKit +page.server.ts direct import):

load() returns groups array from /api/groups alongside invites
load() returns groups: [] when groups API returns non-OK (form resilience)
create action extracts groupIds via formData.getAll('groupIds') and includes them in the POST body
create action sends groupIds: [] when no checkboxes are ticked

Integration (MockMvc / service layer, already backend-covered but verify):
5. redeemInvite() with stored groupIds → AppUser has those groups assigned

E2E (Playwright, critical user journey):
6. Admin creates invite with WRITE_ALL checked → invited user registers → user appears in /admin/users with WRITE_ALL group
7. Admin creates invite with no groups → invited user registers → user has no groups

Recommendations

CI time impact: steps 1–4 are fast load-function tests (milliseconds). Steps 5–6 require the full stack and belong in the existing E2E suite. No new infrastructure needed.
The E2E test in step 6 directly covers the acceptance criteria from the issue — wire it to the existing Playwright suite as a regression test for the invite flow.

## 🧪 Sara Holt — QA Engineer ### Observations - The issue has well-written Gherkin acceptance criteria covering: form renders groups, invite with group → user has group on registration, invite without group → user has no group. That's a solid AC baseline. - **Zero tests mentioned in the issue or verification steps.** Verification step 2 is a manual click-through, not an automated test. This feature touches the registration flow — any regression here would give a newly invited user wrong permissions without any automated safety net. - **Missing AC**: what happens when the groups API is unavailable at form load time? The form should still render and allow invite creation without groups (optional field). Without an explicit AC for this, the implementation might throw an error that breaks the entire invite form. - **Missing edge case AC**: what if the same group is somehow submitted twice (client-side manipulation)? The backend should deduplicate or the DB constraint should handle it — but neither the issue nor the ACs address this. ### Test plan (what should be written before implementation) **Unit (SvelteKit `+page.server.ts` direct import):** 1. `load()` returns `groups` array from `/api/groups` alongside invites 2. `load()` returns `groups: []` when groups API returns non-OK (form resilience) 3. `create` action extracts `groupIds` via `formData.getAll('groupIds')` and includes them in the POST body 4. `create` action sends `groupIds: []` when no checkboxes are ticked **Integration (MockMvc / service layer, already backend-covered but verify):** 5. `redeemInvite()` with stored `groupIds` → `AppUser` has those groups assigned **E2E (Playwright, critical user journey):** 6. Admin creates invite with `WRITE_ALL` checked → invited user registers → user appears in `/admin/users` with `WRITE_ALL` group 7. Admin creates invite with no groups → invited user registers → user has no groups ### Recommendations - CI time impact: steps 1–4 are fast load-function tests (milliseconds). Steps 5–6 require the full stack and belong in the existing E2E suite. No new infrastructure needed. - The E2E test in step 6 directly covers the acceptance criteria from the issue — wire it to the existing Playwright suite as a regression test for the invite flow.

marcel commented

2026-05-14 11:53:59 +02:00

🎨 Leonie Voss — UX Designer & Accessibility Strategist

Observations

Reusing UserGroupsSection.svelte is the right call. Consistency across /admin/users/new and /admin/invites means admins learn the pattern once. This is especially important for the senior admin audience who rely on muscle memory and calm, predictable layouts.
The label "Gruppen (optional)" is correct UX: it explicitly signals that no selection is valid, reducing cognitive load. Don't change it to just "Gruppen".
Section title markup in the issue: the issue uses inline font-sans text-xs font-bold tracking-widest text-ink-2 uppercase — note text-ink-2, not text-ink-3. All other section titles in the codebase use text-ink-3. Verify which token is used on the surrounding fields in +page.svelte and match it exactly. text-ink-2 is darker — using the wrong token breaks visual rhythm.
sm:col-span-2 wrapper: correct for a full-width section in a 2-column grid. Verify the invite form uses a 2-column grid (sm:grid-cols-2) — if it's single-column, the span wrapper is a no-op and can be omitted.
Accessibility in UserGroupsSection: verify the component uses a <fieldset> with <legend> to group the checkboxes semantically. A bare <div> wrapping checkboxes is not accessible — screen readers cannot associate the "Gruppen (optional)" label with the individual checkboxes unless a <fieldset>/<legend> or explicit aria-labelledby is used. This is a WCAG 1.3.1 failure if missing.
Touch targets: checkbox + label click area in UserGroupsSection must meet 44×44px minimum for the senior admin audience. Verify the component adds py-2 or equivalent padding to each checkbox row.
Mobile: on the invite form at 320px, the checkbox list should stack vertically with sufficient line height. No fixed widths should appear.

Recommendations

Before implementing, open UserGroupsSection.svelte and verify: (1) it uses <fieldset>/<legend>, (2) each checkbox row meets 44px touch target, (3) focus ring is visible (focus-visible:ring-2).
If the component is missing any of these, fix UserGroupsSection first — the fix benefits both the users/new and invites forms.
Use text-ink-3 for the section label to match the visual tone of other form sections unless the surrounding fields already use text-ink-2.

## 🎨 Leonie Voss — UX Designer & Accessibility Strategist ### Observations - **Reusing `UserGroupsSection.svelte` is the right call.** Consistency across `/admin/users/new` and `/admin/invites` means admins learn the pattern once. This is especially important for the senior admin audience who rely on muscle memory and calm, predictable layouts. - **The label `"Gruppen (optional)"` is correct UX**: it explicitly signals that no selection is valid, reducing cognitive load. Don't change it to just "Gruppen". - **Section title markup in the issue**: the issue uses inline `font-sans text-xs font-bold tracking-widest text-ink-2 uppercase` — note `text-ink-2`, not `text-ink-3`. All other section titles in the codebase use `text-ink-3`. Verify which token is used on the surrounding fields in `+page.svelte` and match it exactly. `text-ink-2` is darker — using the wrong token breaks visual rhythm. - **`sm:col-span-2` wrapper**: correct for a full-width section in a 2-column grid. Verify the invite form uses a 2-column grid (`sm:grid-cols-2`) — if it's single-column, the span wrapper is a no-op and can be omitted. - **Accessibility in `UserGroupsSection`**: verify the component uses a `<fieldset>` with `<legend>` to group the checkboxes semantically. A bare `<div>` wrapping checkboxes is not accessible — screen readers cannot associate the "Gruppen (optional)" label with the individual checkboxes unless a `<fieldset>/<legend>` or explicit `aria-labelledby` is used. This is a WCAG 1.3.1 failure if missing. - **Touch targets**: checkbox + label click area in `UserGroupsSection` must meet 44×44px minimum for the senior admin audience. Verify the component adds `py-2` or equivalent padding to each checkbox row. - **Mobile**: on the invite form at 320px, the checkbox list should stack vertically with sufficient line height. No fixed widths should appear. ### Recommendations - Before implementing, open `UserGroupsSection.svelte` and verify: (1) it uses `<fieldset>/<legend>`, (2) each checkbox row meets 44px touch target, (3) focus ring is visible (`focus-visible:ring-2`). - If the component is missing any of these, fix `UserGroupsSection` first — the fix benefits both the users/new and invites forms. - Use `text-ink-3` for the section label to match the visual tone of other form sections unless the surrounding fields already use `text-ink-2`.

marcel commented

2026-05-14 11:54:09 +02:00

🛠️ Tobias Wendt — DevOps & Platform Engineer

Observations

Zero infrastructure impact. No new Docker services, no environment variables, no compose changes, no CI modifications. The GET /api/groups endpoint is already live in the running backend. This is as clean as a change gets from an ops perspective.
Verification steps in the issue are correct: docker-compose up -d → manual test flow → npm run check. Nothing tricky here.
CI time: no new integration test containers, no new service dependencies. The only CI cost is the new frontend unit tests (fast) and an E2E test addition (already has the Docker stack available).
The self-hosted Gitea runner will handle this PR without any runner configuration changes.

Observations (positive — worth noting)

The implementation correctly keeps groups fetching on the server side (via +page.server.ts) rather than in the browser. This means the admin's session cookie is used for auth, no API key is exposed to the browser, and the page works without client-side JavaScript.
No new MinIO bucket, no new Hetzner S3 prefix, no new secret rotation needed.

Recommendations

Nothing to change from an infrastructure perspective. The only thing worth double-checking: run npm run check and cd backend && ./mvnw test locally before pushing, since the typed client must match the current OpenAPI spec. No npm run generate:api should be needed here (no backend model changes), but confirm the /api/groups route is already present in the generated types.

## 🛠️ Tobias Wendt — DevOps & Platform Engineer ### Observations - **Zero infrastructure impact.** No new Docker services, no environment variables, no compose changes, no CI modifications. The `GET /api/groups` endpoint is already live in the running backend. This is as clean as a change gets from an ops perspective. - **Verification steps in the issue are correct**: `docker-compose up -d` → manual test flow → `npm run check`. Nothing tricky here. - **CI time**: no new integration test containers, no new service dependencies. The only CI cost is the new frontend unit tests (fast) and an E2E test addition (already has the Docker stack available). - The self-hosted Gitea runner will handle this PR without any runner configuration changes. ### Observations (positive — worth noting) - The implementation correctly keeps groups fetching on the server side (via `+page.server.ts`) rather than in the browser. This means the admin's session cookie is used for auth, no API key is exposed to the browser, and the page works without client-side JavaScript. - No new MinIO bucket, no new Hetzner S3 prefix, no new secret rotation needed. ### Recommendations - Nothing to change from an infrastructure perspective. The only thing worth double-checking: run `npm run check` and `cd backend && ./mvnw test` locally before pushing, since the typed client must match the current OpenAPI spec. No `npm run generate:api` should be needed here (no backend model changes), but confirm the `/api/groups` route is already present in the generated types.

marcel commented

2026-05-14 11:54:24 +02:00

📋 Elicit — Requirements Engineer

Observations

Issue quality is high. Clear user story, explicit current/desired behavior, Gherkin acceptance criteria, concrete implementation notes with code snippets. This meets Definition of Ready on almost every axis.
Missing AC: groups unavailable at form load. The groups section is "optional" per the label, but the ACs don't specify behavior when GET /api/groups fails (503, network timeout, permission error). Should the form still render and allow invite creation without the group picker? Or should the form display an error? A requirement like: "Given the groups API is unavailable, when the admin opens the new invite form, then the form renders without the groups section and invite creation without groups remains possible" would close this gap.
Missing requirement: group list ordering. The issue says "a checkbox per group" but doesn't specify ordering. Alphabetical by name is the expected convention — without stating it, the implementation might return groups in insertion order (random from the DB perspective). Add: "Groups are displayed in alphabetical order by name."
Edge case not in ACs: deleted group between invite creation and redemption. If a group is deleted after an invite is created (but before it's redeemed), what happens? The codebase research shows InviteService.createInvite() validates group UUIDs at creation time, but redeemInvite() may skip validation (it just passes the stored UUIDs to createUser()). This is a requirements gap that could surface as a silent failure in production. Recommend adding: "If a group associated with an invite is deleted before redemption, the invite can still be redeemed; the deleted group is silently skipped." (Or alternatively: the redemption fails with a clear error — whichever behavior is intended.)
Non-functional requirements: no accessibility requirement is stated, even though the group picker must be accessible (keyboard-navigable checkboxes, screen-reader-labeled fieldset). Recommend adding a note: "The group picker must be keyboard-accessible and screen-reader-compatible (WCAG 2.1 AA)."

Recommendations

Add three ACs before implementation starts:
1. Groups API failure → form still usable (groups section hidden or empty, not a crash)
2. Groups display alphabetically by name
3. Redemption behavior when a group is deleted post-invite-creation
These are not blockers for a P2 issue, but closing them now costs 5 minutes and prevents a future bug report.

## 📋 Elicit — Requirements Engineer ### Observations - **Issue quality is high.** Clear user story, explicit current/desired behavior, Gherkin acceptance criteria, concrete implementation notes with code snippets. This meets Definition of Ready on almost every axis. - **Missing AC: groups unavailable at form load.** The groups section is "optional" per the label, but the ACs don't specify behavior when `GET /api/groups` fails (503, network timeout, permission error). Should the form still render and allow invite creation without the group picker? Or should the form display an error? A requirement like: *"Given the groups API is unavailable, when the admin opens the new invite form, then the form renders without the groups section and invite creation without groups remains possible"* would close this gap. - **Missing requirement: group list ordering.** The issue says "a checkbox per group" but doesn't specify ordering. Alphabetical by name is the expected convention — without stating it, the implementation might return groups in insertion order (random from the DB perspective). Add: *"Groups are displayed in alphabetical order by name."* - **Edge case not in ACs: deleted group between invite creation and redemption.** If a group is deleted after an invite is created (but before it's redeemed), what happens? The codebase research shows `InviteService.createInvite()` validates group UUIDs at creation time, but `redeemInvite()` may skip validation (it just passes the stored UUIDs to `createUser()`). This is a requirements gap that could surface as a silent failure in production. Recommend adding: *"If a group associated with an invite is deleted before redemption, the invite can still be redeemed; the deleted group is silently skipped."* (Or alternatively: the redemption fails with a clear error — whichever behavior is intended.) - **Non-functional requirements**: no accessibility requirement is stated, even though the group picker must be accessible (keyboard-navigable checkboxes, screen-reader-labeled fieldset). Recommend adding a note: *"The group picker must be keyboard-accessible and screen-reader-compatible (WCAG 2.1 AA)."* ### Recommendations - Add three ACs before implementation starts: 1. Groups API failure → form still usable (groups section hidden or empty, not a crash) 2. Groups display alphabetically by name 3. Redemption behavior when a group is deleted post-invite-creation - These are not blockers for a P2 issue, but closing them now costs 5 minutes and prevents a future bug report.

marcel commented

2026-05-14 11:54:52 +02:00

🗳️ Decision Queue — Action Required

2 decisions need your input before implementation starts.

Requirements / Business Logic

Behavior when groups API fails at form load time — The groups field is labeled "optional," so the natural default is: form still renders, group picker silently absent, admin can still create the invite without groups. But this should be an explicit AC, not an assumption. Options: (A) groups section hidden, form fully usable — recommended, lowest friction; (B) error banner shown, invite creation blocked. (Raised by: Elicit, Felix, Sara)
Behavior when a group is deleted between invite creation and redemption — At creation time, InviteService validates that the group UUIDs exist. At redemption time, the stored UUIDs are passed straight to createUser() without re-validation. If a group is deleted in the meantime: (A) redemption succeeds and the deleted group is silently skipped (user gets fewer groups than invited, no error); (B) redemption fails with a clear error (admin must re-issue the invite). This is a business decision — (A) is more resilient and matches how similar systems behave, but (B) surfaces the inconsistency explicitly. (Raised by: Elicit, Nora)

## 🗳️ Decision Queue — Action Required _2 decisions need your input before implementation starts._ ### Requirements / Business Logic - **Behavior when groups API fails at form load time** — The groups field is labeled "optional," so the natural default is: form still renders, group picker silently absent, admin can still create the invite without groups. But this should be an explicit AC, not an assumption. Options: (A) groups section hidden, form fully usable — **recommended, lowest friction**; (B) error banner shown, invite creation blocked. _(Raised by: Elicit, Felix, Sara)_ - **Behavior when a group is deleted between invite creation and redemption** — At creation time, `InviteService` validates that the group UUIDs exist. At redemption time, the stored UUIDs are passed straight to `createUser()` without re-validation. If a group is deleted in the meantime: (A) redemption succeeds and the deleted group is silently skipped (user gets fewer groups than invited, no error); (B) redemption fails with a clear error (admin must re-issue the invite). This is a business decision — (A) is more resilient and matches how similar systems behave, but (B) surfaces the inconsistency explicitly. _(Raised by: Elicit, Nora)_

marcel commented

2026-05-14 12:00:23 +02:00

Form should still work, but we should show a warning why the groups are missing
A. the group service should probably check if there are pending invites with the group it wants to delete and throw an error if there are invites, so an admin cannot delete a group that is in a pending invite

Form should still work, but we should show a warning why the groups are missing A. the group service should probably check if there are pending invites with the group it wants to delete and throw an error if there are invites, so an admin cannot delete a group that is in a pending invite

Sign in to join this conversation.

1 Participants

Notifications

Due Date

No due date set.

Dependencies

No dependencies set.

Reference: marcel/familienarchiv#566

feat(admin): assign groups when creating an invite link #566

User story

Current behaviour

Desired behaviour

Acceptance criteria

Implementation notes

frontend/src/routes/admin/invites/+page.server.ts

frontend/src/routes/admin/invites/+page.svelte

i18n (frontend/messages/de.json, en.json, es.json)

Verification

👨‍💻 Felix Brandt — Senior Fullstack Developer

Observations

Recommendations

🏗️ Markus Keller — Application Architect

Observations

Recommendations

🔐 Nora "NullX" Steiner — Application Security Engineer

Observations

Recommendations

🧪 Sara Holt — QA Engineer

Observations

Test plan (what should be written before implementation)

Recommendations

🎨 Leonie Voss — UX Designer & Accessibility Strategist

Observations

Recommendations

🛠️ Tobias Wendt — DevOps & Platform Engineer

Observations

Observations (positive — worth noting)

Recommendations

📋 Elicit — Requirements Engineer

Observations

Recommendations

🗳️ Decision Queue — Action Required

Requirements / Business Logic

`frontend/src/routes/admin/invites/+page.server.ts`

`frontend/src/routes/admin/invites/+page.svelte`

i18n (`frontend/messages/de.json`, `en.json`, `es.json`)