From 0d97a2aacaf6ad1361a4f5ba91414a4dc0df3b43 Mon Sep 17 00:00:00 2001 From: Felix Kotschenreuther Date: Mon, 28 Sep 2026 14:17:08 +0200 Subject: [PATCH 1/3] feat(apply): migrate a group's type through POST /groups/{id}/grouptype MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Closes #171. A `groupTypeId` change was planned as an ordinary field update and sent on the regular group update, which ChurchTools always rejects: PATCH /groups/1182 -> 400 groupTypeId: validation.always.invalid So the plan looked applicable and the apply died partway through. CT migrates a group's type through a dedicated endpoint instead, taking a mapping from every role of the old type onto a role of the new one: POST /groups/{id}/grouptype { groupTypeId, roleMapping: { : } } Verified live (CT 3.137.0-RC21, 2026-09-28) by migrating a group and migrating it back: both sides of `roleMapping` are groupTypeRoleIds from /group/roles, keyed by the roles of the group's CURRENT type; the endpoint answers 204 with no body. That mapping decides where existing MEMBERSHIPS land, so it is never guessed. It is derived only where the answer cannot cost anyone their role: name exactly one role of the target type carries the same name empty-role the old role holds no members in this group, so nothing can move declared a `roleMapping` on the group — always wins A role that holds members and has no same-named target is refused at PLAN time, naming the role, its member count and the target type's roles. Nothing is written before that refusal, which keeps `plan` honest: it no longer renders an applicable-looking diff that is guaranteed to fail. The plan spells the migration out under the field rather than showing a bare `groupTypeId: 5 -> 4`, because applying it moves people: ~ group.team_academy_first_year (#1182) groupTypeId: 5 -> 4 via POST /groups/1182/grouptype — role mapping (members follow their role): Mitglied -> Mitglied (7 members, matched by name) Supporter -> Mitglied (0 members, matched by empty-role) Apply POSTs the migration first, then PATCHes the remaining fields with `groupTypeId` withheld — so a refused migration leaves every other field untouched, and state still records the new type, so a re-plan is a no-op. Costs nothing when no type changes: the resolver issues no request at all. When one does it reads /group/roles once plus one members page per migrating group; the members read is what makes an empty role safe to map, so it is not optional. Verified against a live instance: the two groups that could never converge now render a full derived mapping and plan cleanly. Claude-Session: https://claude.ai/code/session_016XVmiQY44pjx1u9Fu4iSDP --- docs/configuration.md | 47 ++++ docs/group-field-decisions.md | 2 +- docs/handbuch/blueprints.md | 2 +- docs/handbuch/group-member-fields.md | 2 +- docs/handbuch/permissions.md | 2 +- src/config/context.ts | 28 +++ src/engine/build.ts | 8 + src/engine/execute.ts | 20 +- src/engine/grouptype.ts | 285 ++++++++++++++++++++++++ src/engine/render.ts | 16 ++ src/engine/types.ts | 14 ++ tests/context.test.ts | 50 +++++ tests/grouptype-migration-apply.test.ts | 189 ++++++++++++++++ tests/grouptype-migration.test.ts | 233 +++++++++++++++++++ 14 files changed, 892 insertions(+), 6 deletions(-) create mode 100644 src/engine/grouptype.ts create mode 100644 tests/grouptype-migration-apply.test.ts create mode 100644 tests/grouptype-migration.test.ts diff --git a/docs/configuration.md b/docs/configuration.md index 21a430c..f0634c5 100644 --- a/docs/configuration.md +++ b/docs/configuration.md @@ -102,6 +102,53 @@ intentional-duplicate case. If a create 400s on this guard without the flag set, that should be adopted with `ct adopt group --key `) and the opt-in as the alternative. +### Changing a group's type + +`groupTypeId` is managed, but changing it on an existing group is a **migration**, +not a field update. ChurchTools refuses the field on the ordinary group update +(`HTTP 400 groupTypeId: validation.always.invalid`) and exposes +`POST /groups/{id}/grouptype` instead, which takes a mapping from every role of the +old type onto a role of the new one. That mapping decides **where existing +memberships land**: whoever holds a role mapping to `X` holds `X` afterwards. + +`ct plan` resolves the mapping up front and renders it, so the diff shows what the +apply will actually do: + +``` + ~ group.team_academy_first_year (#1182) + groupTypeId: 5 -> 4 + via POST /groups/1182/grouptype — role mapping (members follow their role): + Mitglied -> Mitglied (7 members, matched by name) + Leiter -> Leiter (2 members, matched by name) + Supporter -> Mitglied (0 members, matched by empty-role) +``` + +ct derives a mapping only where the answer cannot cost anyone their role: + +| matched by | when | +| ------------ | ------------------------------------------------------------- | +| `name` | exactly one role of the target type carries the same name | +| `empty-role` | the old role holds no members here, so no membership can move | +| `declared` | you said so (below) — always wins | + +A role that **holds members** and has no same-named target is a decision ct will +not make for you. The plan refuses, naming the role, its member count and the +target type's roles, and you answer it on the group: + +```ts +ct.group({ + key: "team_academy_first_year", + name: "Academy First Year 26/27", + groupType: "merkmal", + roleMapping: { Supporter: "Mitglied" }, +}); +``` + +`roleMapping` is old role **name** → new role name (so one config stays portable +across hosts), compared as slugs. Like `allowDuplicateName` it is never a managed +field: not diffed, not in state, never adopted, and read only when the type +actually changes. + ## Output conventions Machine-readable output goes to **stdout** (pipe/`jq` it); human status lines go diff --git a/docs/group-field-decisions.md b/docs/group-field-decisions.md index 1129e32..b2e467d 100644 --- a/docs/group-field-decisions.md +++ b/docs/group-field-decisions.md @@ -26,7 +26,7 @@ field — a plain top-level key. `campusId` is wired the same deliberate way as | Field | Decision | Rationale | | ---------------------------------------------------- | -------------------------- | --------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------- | | `name` | **managed** | Core identity; already managed. | -| `groupTypeId` | **managed** | Determines the group's kind and role template; already managed. **Changing it on an existing group is not a normal update:** ChurchTools rejects `groupTypeId` on the regular group update (`HTTP 400 groupTypeId: validation.always.invalid`) and requires `POST /groups/{id}/grouptype` with a role mapping. ct still plans it as a normal update, so the apply fails (#171). Until that is fixed, change a group's type in the ChurchTools UI and adopt the drift. | +| `groupTypeId` | **managed** | Determines the group's kind and role template; already managed. **Changing it on an existing group is a MIGRATION, not a normal update:** ChurchTools rejects `groupTypeId` on the regular group update (`HTTP 400 groupTypeId: validation.always.invalid`) and requires `POST /groups/{id}/grouptype` with a role mapping, because that mapping decides where existing memberships land. ct now plans and applies it as such (#171): the plan renders the endpoint and the full role mapping, apply POSTs the migration and withholds `groupTypeId` from the PATCH. The mapping is derived only where it cannot cost anyone their role — a same-named role in the target type, or a role holding no members — and a role that holds members with no same-named target is refused at plan time until a `roleMapping` is declared on the group. | | `groupStatusId` | **managed** | Lifecycle status; already managed. | | **`campusId`** | **managed (new, #21)** | The campus link is the tool's core "instantiate this area per campus" requirement. Numeric escape hatch only — an existing CT campus id (or `null` to clear). A _logical_ `campus: "key"` reference (resolving a same-run campus by key) is **deferred to [#20](https://github.com/eqrm/ct-cli/issues/20)**; the DSL rejects a `campus` field with a pointer to #20 so it can't slip through as an un-diffable phantom. | | `parents` (hierarchy) | **opt-in synthetic** | Group→group hierarchy is reconciled through its own endpoint, not the group body — see `src/engine/synthetic.ts`. Opt-in via `parents: [...]`. | diff --git a/docs/handbuch/blueprints.md b/docs/handbuch/blueprints.md index c00d7b1..d493c90 100644 --- a/docs/handbuch/blueprints.md +++ b/docs/handbuch/blueprints.md @@ -4,7 +4,7 @@ sources: - src/config/context.ts - src/engine/graph.ts - src/engine/hierarchy.ts -sources_hash: 636599577c600553 +sources_hash: 57e2a48f85e93d55 reviewed: 2026-08-28 --- diff --git a/docs/handbuch/group-member-fields.md b/docs/handbuch/group-member-fields.md index 6c0afb5..b59bf8a 100644 --- a/docs/handbuch/group-member-fields.md +++ b/docs/handbuch/group-member-fields.md @@ -1,5 +1,5 @@ --- -sources_hash: ffdca84fefe83fde +sources_hash: 802017b4678885e0 title: Group member fields sources: - src/engine/member-fields.ts diff --git a/docs/handbuch/permissions.md b/docs/handbuch/permissions.md index fb7c166..0ec6b63 100644 --- a/docs/handbuch/permissions.md +++ b/docs/handbuch/permissions.md @@ -7,7 +7,7 @@ sources: - src/resolve/resolver.ts - src/resolve/refs.ts - src/config/context.ts -sources_hash: ac44a97574ed9ba9 +sources_hash: 398256b5381fd827 reviewed: 2026-08-28 --- diff --git a/src/config/context.ts b/src/config/context.ts index db69dd3..6e1305e 100644 --- a/src/config/context.ts +++ b/src/config/context.ts @@ -156,6 +156,15 @@ export interface ResourceInput { * NEVER force by default — omit (or `false`) to keep CT's guard on. */ allowDuplicateName?: boolean; + /** + * Group-only (#171): where existing memberships land when this group's `groupTypeId` changes, as + * old role NAME → new role NAME. Consulted ONLY on a type change; never diffed, never in state. + * + * ct derives the mapping itself wherever the answer cannot cost anyone their role — a same-named + * role in the target type, or a role holding no members — and refuses at plan time otherwise. This + * is how that refusal is answered. Names, not ids, so one config stays portable across hosts. + */ + roleMapping?: Record; [field: string]: unknown; } @@ -537,6 +546,7 @@ function toDesired(type: string, input: ResourceInput, location?: string): Desir dynamic, memberFields, allowDuplicateName, + roleMapping, ...fields } = input; if (!key || typeof key !== "string") { @@ -553,6 +563,23 @@ function toDesired(type: string, input: ResourceInput, location?: string): Desir throw new Error(`${type} "${key}": "allowDuplicateName" must be a boolean.`); } } + // Group-only role mapping for a type migration (#171). Destructured out above like + // `allowDuplicateName`, so it never reaches `fields` and never trips the unknown-field warning. + if (roleMapping !== undefined) { + if (type !== "group") { + throw new Error(`${type} "${key}": "roleMapping" is only valid on a group.`); + } + if (typeof roleMapping !== "object" || roleMapping === null || Array.isArray(roleMapping)) { + throw new Error(`${type} "${key}": "roleMapping" must be an object of old role name -> new role name.`); + } + for (const [from, to] of Object.entries(roleMapping)) { + if (typeof to !== "string" || to.trim() === "") { + throw new Error( + `${type} "${key}": "roleMapping.${from}" must be a non-empty target role name (got ${JSON.stringify(to)}).`, + ); + } + } + } // A nullish/empty `parent` is "no parent", not an opt-in to managed-empty hierarchy. if (parent != null && typeof parent !== "string") { throw new Error(`${type} "${key}": "parent" must be a string key.`); @@ -668,6 +695,7 @@ function toDesired(type: string, input: ResourceInput, location?: string): Desir dependsOn: edges, preventDestroy, allowDuplicateName, + roleMapping, }; } diff --git a/src/engine/build.ts b/src/engine/build.ts index 91fd191..2a05a6c 100644 --- a/src/engine/build.ts +++ b/src/engine/build.ts @@ -11,6 +11,7 @@ import type { ManagedResource, State } from "../state/state.js"; import type { DesiredResource, Plan } from "./types.js"; import { RESOURCES, type CtWriteClient } from "../resources/registry.js"; import { computePlan } from "./plan.js"; +import { resolveGroupTypeMigrations } from "./grouptype.js"; import { foldSynthetic } from "./synthetic.js"; import { Resolver } from "../resolve/resolver.js"; import { collectPendingRefKeys } from "../resolve/refs.js"; @@ -162,6 +163,13 @@ export async function buildPlan( }); const plan = computePlan(ordered, state, actual, { unresolved, fetchFailed }); + + // A `groupTypeId` change is a MIGRATION, not a field update (#171): CT refuses the field on the + // ordinary update and wants `POST /groups/{id}/grouptype` with a role mapping. Resolved here, while + // a client is in hand, so the plan renders the mapping the apply will send — and so an underivable + // mapping fails now instead of halfway through a write. + await resolveGroupTypeMigrations(client, plan, new Map(ordered.map((d) => [d.key, d.roleMapping]))); + const warnings = [...fetchWarnings, ...(folded.warnings ?? [])]; return { plan, actual, fetchErrors, ...(warnings.length > 0 ? { warnings } : {}) }; } diff --git a/src/engine/execute.ts b/src/engine/execute.ts index 0b217dd..91b6895 100644 --- a/src/engine/execute.ts +++ b/src/engine/execute.ts @@ -15,6 +15,7 @@ import { upsert, saveState } from "../state/state.js"; import type { FieldChange, Plan, PlanItem } from "./types.js"; import { RESOURCES, type CtWriteClient } from "../resources/registry.js"; import { assertNotPeople } from "./guard.js"; +import { roleMappingPayload } from "./grouptype.js"; import { isSyntheticField, syntheticField } from "./synthetic.js"; import { reresolvePendingValue } from "../resolve/resolver.js"; import { hasPendingRef } from "../resolve/refs.js"; @@ -234,7 +235,22 @@ export async function executePlan(plan: Plan, deps: ExecuteDeps): Promise !isSyntheticField(c.field)); + // A group-type change cannot ride the ordinary update — CT answers 400 + // `groupTypeId: validation.always.invalid` — so it goes through the migration endpoint FIRST + // and is then withheld from the field write (#171). Ordering matters: the migration rewrites + // the group's role instances, and a PATCH of the remaining fields is safe either side of it, + // but doing the migration first means a failure here leaves every other field untouched. + const migration = item.groupTypeMigration; + if (migration) { + const migrationPath = `/groups/${id}/grouptype`; + assertNotPeople(migrationPath); + await client.request("POST", migrationPath, { + groupTypeId: migration.toGroupTypeId, + roleMapping: roleMappingPayload(migration), + }); + } + const fieldChanges = migration ? changes.filter((c) => c.field !== "groupTypeId") : changes; + const hasFieldChange = fieldChanges.some((c) => !isSyntheticField(c.field)); if (hasFieldChange) { const path = spec.itemPath(id); assertNotPeople(path); @@ -246,7 +262,7 @@ export async function executePlan(plan: Plan, deps: ExecuteDeps): Promise, "roleMapping": { : } } + * + * Verified live against CT 3.137.0-RC21 on 2026-09-28 by migrating a group and migrating it back: + * both keys and values of `roleMapping` are **`groupTypeRoleId`s** (the type-level role ids from + * `/group/roles`), keyed by the roles of the group's CURRENT type. The endpoint answers `204` with an + * empty body. The UI asks a human to fill that mapping in, because it decides where every existing + * MEMBERSHIP lands: a member holding a role that maps to X holds X afterwards. + * + * That is why this module refuses to guess. The mapping is derived only where the answer cannot + * cost anyone their role: + * + * 1. `declared` — an explicit `roleMapping` in config always wins. + * 2. `name` — exactly one role of the target type carries the same name (slug-compared). + * 3. `empty-role` — the old role holds NO members in this group, so no membership can move. + * It still needs a target (the payload is keyed by every current-type role), and + * the target type's default role is used. + * + * Anything else — a role that HOLDS MEMBERS and has no same-named target — is returned as + * {@link UnmappableRole} so the caller can refuse at PLAN time, before a single write. That is the + * honest-plan rule: a plan that cannot be applied must not render as applicable. + */ + +import { slug } from "../resources/registry.js"; +import type { Plan } from "./types.js"; + +/** A `/group/roles` row, reduced to what a migration needs. */ +export interface GroupTypeRole { + id: number; + name: string; + groupTypeId: number; + /** CT's `leader` | `participant` discriminator. Used only to pick a sane default target. */ + type?: string; + isDefault?: boolean; +} + +/** Why a role's target was chosen — rendered in the plan so the operator can see the reasoning. */ +export type RoleMappingReason = "declared" | "name" | "empty-role"; + +export interface RoleMappingEntry { + fromId: number; + fromName: string; + toId: number; + toName: string; + reason: RoleMappingReason; + /** Members of this group holding the old role. `empty-role` is only ever chosen when this is 0. */ + members: number; +} + +/** A role holding members that cannot be mapped without guessing. Blocks the plan. */ +export interface UnmappableRole { + id: number; + name: string; + members: number; + /** Role names of the target type, offered in the error so the operator can declare one. */ + candidates: string[]; +} + +export interface GroupTypeMigration { + fromGroupTypeId: number; + toGroupTypeId: number; + entries: RoleMappingEntry[]; +} + +export interface DeriveRoleMappingInput { + /** Roles of the group's CURRENT type — the payload is keyed by these. */ + sourceRoles: GroupTypeRole[]; + /** Roles of the type the group is moving TO. */ + targetRoles: GroupTypeRole[]; + /** Members per `groupTypeRoleId` in this group. A role absent from the map holds none. */ + memberCounts: ReadonlyMap; + /** Config's explicit mapping, old role NAME → new role NAME (slug-compared). */ + declared?: Readonly>; +} + +export type DeriveRoleMappingResult = + { ok: true; entries: RoleMappingEntry[] } | { ok: false; unmappable: UnmappableRole[] }; + +/** The role a membership lands in when its old role holds nobody: the target type's default. */ +function defaultTargetRole(targetRoles: GroupTypeRole[], like?: string): GroupTypeRole | undefined { + return ( + targetRoles.find((r) => r.isDefault === true) ?? + // No flagged default (CT allows that): keep the leader/participant character rather than + // silently promoting a participant into a leader role. + targetRoles.find((r) => like !== undefined && r.type === like) ?? + targetRoles[0] + ); +} + +/** + * Build the `roleMapping` for a group-type migration, or report every role that would need a guess. + * + * Deterministic and side-effect free: the same inputs always produce the same mapping, which is what + * lets `plan` render it and `apply` send exactly what was rendered. + */ +export function deriveRoleMapping(input: DeriveRoleMappingInput): DeriveRoleMappingResult { + const { sourceRoles, targetRoles, memberCounts } = input; + const declared = input.declared ?? {}; + + // Target lookups. A name is only a usable key when exactly ONE target role carries it — CT does not + // forbid duplicates, and "one of the two Leiter roles" is precisely the guess this must not make. + const targetsBySlug = new Map(); + for (const role of targetRoles) { + const key = slug(role.name); + const bucket = targetsBySlug.get(key); + if (bucket) bucket.push(role); + else targetsBySlug.set(key, [role]); + } + const uniqueTarget = (name: string): GroupTypeRole | undefined => { + const bucket = targetsBySlug.get(slug(name)); + return bucket !== undefined && bucket.length === 1 ? bucket[0] : undefined; + }; + + const declaredBySlug = new Map(); + for (const [from, to] of Object.entries(declared)) { + declaredBySlug.set(slug(from), to); + } + + const entries: RoleMappingEntry[] = []; + const unmappable: UnmappableRole[] = []; + const candidates = targetRoles.map((r) => r.name); + + for (const source of sourceRoles) { + const members = memberCounts.get(source.id) ?? 0; + + const declaredTargetName = declaredBySlug.get(slug(source.name)); + if (declaredTargetName !== undefined) { + const target = uniqueTarget(declaredTargetName); + if (!target) { + // A declared mapping that does not land is a config error, not a guess to be papered over. + unmappable.push({ id: source.id, name: source.name, members, candidates }); + continue; + } + entries.push({ + fromId: source.id, + fromName: source.name, + toId: target.id, + toName: target.name, + reason: "declared", + members, + }); + continue; + } + + const byName = uniqueTarget(source.name); + if (byName) { + entries.push({ + fromId: source.id, + fromName: source.name, + toId: byName.id, + toName: byName.name, + reason: "name", + members, + }); + continue; + } + + if (members === 0) { + const fallback = defaultTargetRole(targetRoles, source.type); + if (fallback) { + entries.push({ + fromId: source.id, + fromName: source.name, + toId: fallback.id, + toName: fallback.name, + reason: "empty-role", + members, + }); + continue; + } + } + + unmappable.push({ id: source.id, name: source.name, members, candidates }); + } + + return unmappable.length > 0 ? { ok: false, unmappable } : { ok: true, entries }; +} + +/** The wire payload for `POST /groups/{id}/grouptype`. */ +export function roleMappingPayload(migration: GroupTypeMigration): Record { + const mapping: Record = {}; + for (const entry of migration.entries) { + mapping[String(entry.fromId)] = entry.toId; + } + return mapping; +} + +/** Roles of one group type, in the order `/group/roles` returned them. */ +export function rolesOfType(roles: GroupTypeRole[], groupTypeId: number): GroupTypeRole[] { + return roles.filter((r) => r.groupTypeId === groupTypeId); +} + +/** Members per `groupTypeRoleId` from a `GET /groups/{id}/members` page. */ +export function memberCountsByRole(members: { groupTypeRoleId?: number }[]): Map { + const counts = new Map(); + for (const member of members) { + const roleId = member.groupTypeRoleId; + if (typeof roleId === "number") { + counts.set(roleId, (counts.get(roleId) ?? 0) + 1); + } + } + return counts; +} + +/** + * Resolve the migration for every group update whose `groupTypeId` changes, so the plan renders the + * same mapping the apply will send. + * + * Costs nothing on the common path: with no type change it issues no request at all. When there is + * one it reads `/group/roles` once plus one members page per migrating group — and that members read + * is what makes an empty role safe to map automatically, so it is not optional. + * + * Throws {@link unmappableRolesError} when a mapping cannot be derived without guessing. Deliberate: + * the alternative is rendering an applicable-looking plan that CT rejects mid-apply (#171). + */ +export async function resolveGroupTypeMigrations( + client: { get(path: string): Promise }, + plan: Plan, + declaredByKey: ReadonlyMap | undefined>, +): Promise { + const migrating = plan.items.filter( + (item) => + item.type === "group" && + item.action === "update" && + item.id !== null && + item.changes.some((c) => c.field === "groupTypeId"), + ); + if (migrating.length === 0) return; + + const roles = await client.get("/group/roles"); + const catalog = Array.isArray(roles) ? roles : []; + + for (const item of migrating) { + const change = item.changes.find((c) => c.field === "groupTypeId")!; + const fromGroupTypeId = Number(change.from); + const toGroupTypeId = Number(change.to); + if (!Number.isFinite(fromGroupTypeId) || !Number.isFinite(toGroupTypeId)) continue; + + const members = await client.get<{ groupTypeRoleId?: number }[]>(`/groups/${item.id}/members?limit=200`); + const declared = declaredByKey.get(item.key); + const result = deriveRoleMapping({ + sourceRoles: rolesOfType(catalog, fromGroupTypeId), + targetRoles: rolesOfType(catalog, toGroupTypeId), + memberCounts: memberCountsByRole(Array.isArray(members) ? members : []), + ...(declared ? { declared } : {}), + }); + if (!result.ok) { + throw unmappableRolesError(item.key, fromGroupTypeId, toGroupTypeId, result.unmappable); + } + item.groupTypeMigration = { fromGroupTypeId, toGroupTypeId, entries: result.entries }; + } +} + +/** + * The plan-time refusal (#171). Names every blocking role with its member count and the target + * type's role names, so the fix — a declared `roleMapping` — can be written without another round + * trip to the API. + */ +export function unmappableRolesError( + groupKey: string, + fromGroupTypeId: number, + toGroupTypeId: number, + unmappable: UnmappableRole[], +): Error { + const lines = unmappable.map( + (role) => + ` - "${role.name}" (role ${role.id}, ${role.members} member(s)) has no same-named role in group type ${toGroupTypeId}`, + ); + return new Error( + `group "${groupKey}": cannot change groupTypeId ${fromGroupTypeId} -> ${toGroupTypeId} without a role mapping.\n` + + ` ChurchTools migrates a group's type through POST /groups/{id}/grouptype, which maps every role of the\n` + + ` current type onto a role of the new one — that mapping decides where existing MEMBERSHIPS land, so ct\n` + + ` will not guess it:\n` + + lines.join("\n") + + `\n Declare the mapping on the group (old role name -> new role name), e.g.\n` + + ` roleMapping: { ${unmappable.map((r) => `"${r.name}": ""`).join(", ")} }\n` + + ` Target type ${toGroupTypeId} offers: ${unmappable[0]?.candidates.join(", ") ?? "(no roles)"}.`, + ); +} diff --git a/src/engine/render.ts b/src/engine/render.ts index 6ebcd4a..fe34e9b 100644 --- a/src/engine/render.ts +++ b/src/engine/render.ts @@ -55,6 +55,22 @@ export function renderPlan(plan: Plan): string { ? ` ${c.field}: ${fmt(c.to)}` : ` ${c.field}: ${fmt(c.from)} -> ${fmt(c.to)}`, ); + // A group-type change is a migration through a different endpoint, and it MOVES MEMBERSHIPS. + // Rendering it as a plain field line would under-report what applying it does (#171), so the + // mapping is spelled out under the field: every role, where it lands, and why. + if (c.field === "groupTypeId" && item.groupTypeMigration) { + lines.push( + pc.dim(` via POST /groups/${item.id}/grouptype — role mapping (members follow their role):`), + ); + for (const entry of item.groupTypeMigration.entries) { + const members = entry.members === 1 ? "1 member" : `${entry.members} members`; + lines.push( + pc.dim( + ` ${entry.fromName} -> ${entry.toName} (${members}, matched by ${entry.reason})`, + ), + ); + } + } } } diff --git a/src/engine/types.ts b/src/engine/types.ts index 6aadefa..19847f0 100644 --- a/src/engine/types.ts +++ b/src/engine/types.ts @@ -3,6 +3,7 @@ * plan produced by diffing desired vs state vs actual. */ +import type { GroupTypeMigration } from "./grouptype.js"; import type { MemberFieldSpec } from "./member-fields.js"; export type { MemberFieldSpec }; @@ -45,6 +46,12 @@ export interface DesiredResource { * touches the update path. `undefined` = not opted in (CT's default guard stays on). */ allowDuplicateName?: boolean; + /** + * Group-only: explicit role mapping for a `groupTypeId` migration (#171), old role NAME → new role + * NAME. Only consulted when the group's type actually changes; never diffed and never written as a + * field. Names, not ids, so one config stays portable across hosts. + */ + roleMapping?: Record; } export type PlanAction = "create" | "update" | "delete" | "no-op"; @@ -116,6 +123,13 @@ export interface PlanItem { * when set, and never touches update/delete. Undefined elsewhere. */ allowDuplicateName?: boolean; + /** + * Group-only: set when this update changes `groupTypeId` (#171). ChurchTools refuses that field on + * the ordinary update, so the executor sends `POST /groups/{id}/grouptype` with this mapping and + * leaves `groupTypeId` out of the PATCH body. Resolved during `buildPlan` (it needs the live role + * catalog), so the plan renders the same mapping the apply will send. + */ + groupTypeMigration?: GroupTypeMigration; } /** Pick a stable human-facing label without coupling renderers to resource-specific branches. */ diff --git a/tests/context.test.ts b/tests/context.test.ts index 5820391..ec4744d 100644 --- a/tests/context.test.ts +++ b/tests/context.test.ts @@ -409,6 +409,56 @@ describe("allowDuplicateName create-time opt-in (#75)", () => { }); }); +describe("roleMapping for a group-type migration (#171)", () => { + it("is carried on the resource but kept out of managed fields (not diffed/adopted)", () => { + const { ct, resources } = createContext(); + ct.group({ key: "academy", name: "Academy", groupTypeId: 4, roleMapping: { Supporter: "Leiter" } }); + expect(resources[0]?.roleMapping).toEqual({ Supporter: "Leiter" }); + expect(resources[0]?.fields).toEqual({ name: "Academy", groupTypeId: 4 }); + }); + + it("defaults to undefined when not declared", () => { + const { ct, resources } = createContext(); + ct.group({ key: "g", name: "G" }); + expect(resources[0]?.roleMapping).toBeUndefined(); + }); + + it("does not trip the unknown-field warning", () => { + const spy = vi.spyOn(process.stderr, "write").mockImplementation(() => true); + try { + const { ct } = createContext(); + ct.group({ key: "g", name: "G", roleMapping: { A: "B" } }); + expect(spy).not.toHaveBeenCalled(); + } finally { + spy.mockRestore(); + } + }); + + it("rejects the option on a non-group type", () => { + const { ct } = createContext(); + expect(() => ct.campus({ key: "c", name: "C", roleMapping: { A: "B" } } as never)).toThrow( + /roleMapping.*only valid on a group/i, + ); + }); + + it("rejects a non-object value", () => { + const { ct } = createContext(); + expect(() => ct.group({ key: "g", name: "G", roleMapping: ["A"] as never })).toThrow( + /roleMapping.*must be an object/i, + ); + }); + + it("rejects an empty or non-string target role name", () => { + const { ct } = createContext(); + expect(() => ct.group({ key: "g", name: "G", roleMapping: { A: "" } })).toThrow( + /roleMapping\.A.*non-empty target role name/i, + ); + expect(() => ct.group({ key: "g", name: "G", roleMapping: { A: 7 as never } })).toThrow( + /roleMapping\.A.*non-empty target role name/i, + ); + }); +}); + describe("permission declarations", () => { it("collects groupRole / groupTypeRole with validated grants", async () => { const mod = (ct: ConfigContext) => { diff --git a/tests/grouptype-migration-apply.test.ts b/tests/grouptype-migration-apply.test.ts new file mode 100644 index 0000000..7bea143 --- /dev/null +++ b/tests/grouptype-migration-apply.test.ts @@ -0,0 +1,189 @@ +import { describe, expect, it, vi } from "vitest"; + +import { executePlan } from "../src/engine/execute.js"; +import { resolveGroupTypeMigrations } from "../src/engine/grouptype.js"; +import { renderPlan } from "../src/engine/render.js"; +import { emptyState, type State } from "../src/state/state.js"; +import type { CtClient } from "../src/api/ctClient.js"; +import type { Plan, PlanItem } from "../src/engine/types.js"; + +const HOST = "https://mychurch.church.tools"; + +/** Two types whose roles overlap partly by name — the shape #171 is about. */ +const ROLE_CATALOG = [ + { id: 105, name: "Mitglied", groupTypeId: 5, type: "participant", isDefault: true }, + { id: 108, name: "Leiter", groupTypeId: 5, type: "leader", isDefault: false }, + { id: 201, name: "Supporter", groupTypeId: 5, type: "participant", isDefault: false }, + { id: 114, name: "Mitglied", groupTypeId: 4, type: "participant", isDefault: true }, + { id: 30, name: "Leiter", groupTypeId: 4, type: "leader", isDefault: false }, +]; + +function mockClient(members: { groupTypeRoleId?: number }[]) { + const calls: { method: string; path: string; body?: unknown }[] = []; + const get = vi.fn(async (path: string) => { + if (path === "/group/roles") return ROLE_CATALOG; + if (path.startsWith("/groups/") && path.includes("/members")) return members; + return []; + }); + const request = vi.fn(async (method: string, path: string, body?: unknown) => { + calls.push({ method, path, body }); + return {}; + }); + return { client: { get, request } as unknown as CtClient, calls, get }; +} + +const typeChange = (): PlanItem => ({ + type: "group", + key: "team_academy_first_year", + displayName: "Academy First Year", + id: 1182, + action: "update", + changes: [ + { field: "groupTypeId", from: 5, to: 4, source: "config" }, + { field: "name", from: "Academy", to: "Academy First Year", source: "config" }, + ], + actual: { name: "Academy", groupTypeId: 5, groupStatusId: 1, campusId: null }, +}); + +const planWith = (item: PlanItem): Plan => ({ items: [item] }); + +describe("resolveGroupTypeMigrations", () => { + it("attaches the mapping a plan can render and an apply can send", async () => { + const { client } = mockClient([]); + const plan = planWith(typeChange()); + await resolveGroupTypeMigrations(client, plan, new Map()); + + const migration = plan.items[0]!.groupTypeMigration; + expect(migration).toBeDefined(); + expect(migration).toMatchObject({ fromGroupTypeId: 5, toGroupTypeId: 4 }); + expect(migration!.entries.map((e) => `${e.fromName}->${e.toName}`)).toEqual([ + "Mitglied->Mitglied", + "Leiter->Leiter", + "Supporter->Mitglied", + ]); + }); + + it("issues no request at all when nothing changes type", async () => { + const { client, get } = mockClient([]); + const plan = planWith({ + type: "group", + key: "g", + id: 7, + action: "update", + changes: [{ field: "name", from: "a", to: "b", source: "config" }], + }); + await resolveGroupTypeMigrations(client, plan, new Map()); + expect(get).not.toHaveBeenCalled(); + expect(plan.items[0]!.groupTypeMigration).toBeUndefined(); + }); + + it("refuses at PLAN time when an occupied role has nowhere unambiguous to go", async () => { + // Two people hold "Supporter", which type 4 has no equivalent for. + const { client } = mockClient([{ groupTypeRoleId: 201 }, { groupTypeRoleId: 201 }]); + const plan = planWith(typeChange()); + await expect(resolveGroupTypeMigrations(client, plan, new Map())).rejects.toThrow( + /cannot change groupTypeId 5 -> 4 without a role mapping/, + ); + }); + + it("accepts the declared mapping that answers that refusal", async () => { + const { client } = mockClient([{ groupTypeRoleId: 201 }, { groupTypeRoleId: 201 }]); + const plan = planWith(typeChange()); + await resolveGroupTypeMigrations( + client, + plan, + new Map([["team_academy_first_year", { Supporter: "Leiter" }]]), + ); + expect(plan.items[0]!.groupTypeMigration!.entries.find((e) => e.fromId === 201)).toMatchObject({ + toId: 30, + reason: "declared", + members: 2, + }); + }); +}); + +describe("renderPlan — a type change must not read like an ordinary field update", () => { + it("spells out the endpoint and every role's destination", async () => { + const { client } = mockClient([{ groupTypeRoleId: 105 }]); + const plan = planWith(typeChange()); + await resolveGroupTypeMigrations(client, plan, new Map()); + + const out = renderPlan(plan); + expect(out).toContain("groupTypeId: 5 -> 4"); + expect(out).toContain("via POST /groups/1182/grouptype"); + expect(out).toContain("Mitglied -> Mitglied (1 member, matched by name)"); + expect(out).toContain("Supporter -> Mitglied (0 members, matched by empty-role)"); + }); +}); + +describe("executePlan — the migration endpoint, not the group PATCH", () => { + async function applyTypeChange(members: { groupTypeRoleId?: number }[] = []) { + const { client, calls } = mockClient(members); + const plan = planWith(typeChange()); + await resolveGroupTypeMigrations(client, plan, new Map()); + const state: State = emptyState(HOST); + state.resources.team_academy_first_year = { + type: "group", + id: 1182, + key: "team_academy_first_year", + fields: { name: "Academy", groupTypeId: 5, groupStatusId: 1, campusId: null }, + adoptedAt: "2026-01-01T00:00:00.000Z", + updatedAt: "2026-01-01T00:00:00.000Z", + }; + await executePlan(plan, { client, state, statePath: "unused", save: async () => {} }); + return { calls, state }; + } + + it("POSTs /groups/{id}/grouptype with the rendered mapping", async () => { + const { calls } = await applyTypeChange(); + const migration = calls.find((c) => c.path === "/groups/1182/grouptype"); + expect(migration).toBeDefined(); + expect(migration!.method).toBe("POST"); + expect(migration!.body).toEqual({ + groupTypeId: 4, + roleMapping: { "105": 114, "108": 30, "201": 114 }, + }); + }); + + it("withholds groupTypeId from the group PATCH — CT rejects the whole request otherwise", async () => { + const { calls } = await applyTypeChange(); + const patch = calls.find((c) => c.method === "PATCH" && c.path === "/groups/1182"); + expect(patch).toBeDefined(); + expect(patch!.body).toEqual({ name: "Academy First Year" }); + expect(patch!.body).not.toHaveProperty("groupTypeId"); + }); + + it("migrates BEFORE touching other fields, so a refusal leaves the rest untouched", async () => { + const { calls } = await applyTypeChange(); + const order = calls.map((c) => `${c.method} ${c.path}`); + expect(order.indexOf("POST /groups/1182/grouptype")).toBeLessThan(order.indexOf("PATCH /groups/1182")); + }); + + it("records the new type in state, so a re-plan is a no-op", async () => { + const { state } = await applyTypeChange(); + expect(state.resources.team_academy_first_year?.fields).toMatchObject({ + groupTypeId: 4, + name: "Academy First Year", + }); + }); + + it("sends no PATCH at all when the type is the only change", async () => { + const { client, calls } = mockClient([]); + const item = typeChange(); + item.changes = [{ field: "groupTypeId", from: 5, to: 4, source: "config" }]; + const plan = planWith(item); + await resolveGroupTypeMigrations(client, plan, new Map()); + const state: State = emptyState(HOST); + state.resources.team_academy_first_year = { + type: "group", + id: 1182, + key: "team_academy_first_year", + fields: { name: "Academy", groupTypeId: 5, groupStatusId: 1, campusId: null }, + adoptedAt: "2026-01-01T00:00:00.000Z", + updatedAt: "2026-01-01T00:00:00.000Z", + }; + await executePlan(plan, { client, state, statePath: "unused", save: async () => {} }); + expect(calls.filter((c) => c.method === "PATCH")).toEqual([]); + expect(calls.map((c) => c.path)).toEqual(["/groups/1182/grouptype"]); + }); +}); diff --git a/tests/grouptype-migration.test.ts b/tests/grouptype-migration.test.ts new file mode 100644 index 0000000..a636dd7 --- /dev/null +++ b/tests/grouptype-migration.test.ts @@ -0,0 +1,233 @@ +import { describe, expect, it } from "vitest"; + +import { + deriveRoleMapping, + memberCountsByRole, + roleMappingPayload, + rolesOfType, + unmappableRolesError, + type GroupTypeRole, +} from "../src/engine/grouptype.js"; + +/** + * Real role catalogs read from a live instance (eqrm-dev, CT 3.137.0-RC21, 2026-09-28). Kept verbatim + * rather than simplified: the interesting cases in #171 are exactly the shapes CT actually ships — + * a type whose roles are a superset of the target's, and stock lowercase role keys sitting next to + * church-named ones. + */ +const ROLES: GroupTypeRole[] = [ + // 18 "AppModule" + { id: 60, name: "participant", groupTypeId: 18, type: "participant", isDefault: false }, + { id: 63, name: "leader", groupTypeId: 18, type: "leader", isDefault: false }, + { id: 162, name: "Teilnehmer", groupTypeId: 18, type: "participant", isDefault: true }, + { id: 165, name: "Systemdesign", groupTypeId: 18, type: "leader", isDefault: false }, + { id: 168, name: "Eigentümer", groupTypeId: 18, type: "leader", isDefault: false }, + { id: 279, name: "Read", groupTypeId: 18, type: "participant", isDefault: false }, + { id: 282, name: "Admin", groupTypeId: 18, type: "leader", isDefault: false }, + { id: 285, name: "Write", groupTypeId: 18, type: "participant", isDefault: false }, + // 21 "Local Lead" + { id: 66, name: "participant", groupTypeId: 21, type: "participant", isDefault: false }, + { id: 69, name: "leader", groupTypeId: 21, type: "leader", isDefault: false }, + { id: 171, name: "Teilnehmer", groupTypeId: 21, type: "participant", isDefault: true }, + { id: 174, name: "Leiter", groupTypeId: 21, type: "leader", isDefault: false }, + { id: 177, name: "Organisator", groupTypeId: 21, type: "leader", isDefault: false }, + // 5 "Team" + { id: 32, name: "participant", groupTypeId: 5, type: "participant", isDefault: false }, + { id: 35, name: "leader", groupTypeId: 5, type: "leader", isDefault: false }, + { id: 105, name: "Mitglied", groupTypeId: 5, type: "participant", isDefault: true }, + { id: 108, name: "Leiter", groupTypeId: 5, type: "leader", isDefault: false }, + { id: 111, name: "Organisator", groupTypeId: 5, type: "leader", isDefault: false }, + { id: 201, name: "Supporter", groupTypeId: 5, type: "participant", isDefault: false }, + // 4 "Merkmal" + { id: 29, name: "Teilnehmer", groupTypeId: 4, type: "participant", isDefault: false }, + { id: 30, name: "Leiter", groupTypeId: 4, type: "leader", isDefault: false }, + { id: 114, name: "Mitglied", groupTypeId: 4, type: "participant", isDefault: true }, + { id: 117, name: "Eigentümer", groupTypeId: 4, type: "leader", isDefault: false }, + { id: 150, name: "Organisator", groupTypeId: 4, type: "leader", isDefault: false }, +]; + +const derive = (from: number, to: number, counts: Map, declared?: Record) => + deriveRoleMapping({ + sourceRoles: rolesOfType(ROLES, from), + targetRoles: rolesOfType(ROLES, to), + memberCounts: counts, + ...(declared ? { declared } : {}), + }); + +describe("rolesOfType", () => { + it("selects only the requested type's roles", () => { + expect(rolesOfType(ROLES, 21).map((r) => r.id)).toEqual([66, 69, 171, 174, 177]); + }); +}); + +describe("memberCountsByRole", () => { + it("counts members per role and ignores rows without a role", () => { + const counts = memberCountsByRole([ + { groupTypeRoleId: 171 }, + { groupTypeRoleId: 171 }, + { groupTypeRoleId: 174 }, + {}, + ]); + expect(counts.get(171)).toBe(2); + expect(counts.get(174)).toBe(1); + expect(counts.size).toBe(2); + }); +}); + +describe("deriveRoleMapping — the Academy case (#139): Team -> Merkmal, no members", () => { + it("maps same-named roles by name and routes the rest to the default, since nothing can move", () => { + const result = derive(5, 4, new Map()); + expect(result.ok).toBe(true); + if (!result.ok) return; + + const byName = Object.fromEntries(result.entries.map((e) => [e.fromName, e])); + expect(byName.Mitglied).toMatchObject({ toId: 114, toName: "Mitglied", reason: "name" }); + expect(byName.Leiter).toMatchObject({ toId: 30, toName: "Leiter", reason: "name" }); + expect(byName.Organisator).toMatchObject({ toId: 150, toName: "Organisator", reason: "name" }); + + // No "Supporter"/"participant"/"leader" in Merkmal — but every one of them is empty, so the + // migration is still risk-free and must not be refused. + expect(byName.Supporter).toMatchObject({ toId: 114, reason: "empty-role", members: 0 }); + expect(byName.participant).toMatchObject({ toId: 114, reason: "empty-role" }); + expect(byName.leader).toMatchObject({ toId: 114, reason: "empty-role" }); + }); + + it("produces a payload keyed by the CURRENT type's role ids", () => { + const result = derive(5, 4, new Map()); + expect(result.ok).toBe(true); + if (!result.ok) return; + expect(roleMappingPayload({ fromGroupTypeId: 5, toGroupTypeId: 4, entries: result.entries })).toEqual({ + "32": 114, + "35": 114, + "105": 114, + "108": 30, + "111": 150, + "201": 114, + }); + }); +}); + +describe("deriveRoleMapping — refuses to guess where a membership would move", () => { + it("blocks an occupied role that has no same-named target", () => { + // "Supporter" holds two people and Merkmal has no Supporter: where they land is a decision. + const result = derive(5, 4, new Map([[201, 2]])); + expect(result.ok).toBe(false); + if (result.ok) return; + expect(result.unmappable).toHaveLength(1); + expect(result.unmappable[0]).toMatchObject({ id: 201, name: "Supporter", members: 2 }); + expect(result.unmappable[0]!.candidates).toContain("Mitglied"); + }); + + it("still maps the empty roles when a different role blocks", () => { + const result = derive(5, 4, new Map([[201, 1]])); + expect(result.ok).toBe(false); + if (result.ok) return; + // Only the occupied, unmatched role blocks — the report stays narrow and actionable. + expect(result.unmappable.map((r) => r.name)).toEqual(["Supporter"]); + }); + + it("an occupied role WITH a same-named target is fine", () => { + const result = derive(5, 4, new Map([[105, 9]])); + expect(result.ok).toBe(true); + if (!result.ok) return; + expect(result.entries.find((e) => e.fromId === 105)).toMatchObject({ + toId: 114, + reason: "name", + members: 9, + }); + }); +}); + +describe("deriveRoleMapping — declared mappings", () => { + it("unblocks an occupied role and wins over a name match", () => { + const result = derive( + 5, + 4, + new Map([ + [201, 2], + [105, 1], + ]), + { + Supporter: "Teilnehmer", + Mitglied: "Eigentümer", + }, + ); + expect(result.ok).toBe(true); + if (!result.ok) return; + expect(result.entries.find((e) => e.fromId === 201)).toMatchObject({ + toId: 29, + toName: "Teilnehmer", + reason: "declared", + }); + // Declared beats the same-named target: Mitglied -> Eigentümer, not Mitglied -> Mitglied. + expect(result.entries.find((e) => e.fromId === 105)).toMatchObject({ + toId: 117, + toName: "Eigentümer", + reason: "declared", + }); + }); + + it("is slug-compared, so case and spacing do not have to match CT exactly", () => { + const result = derive(5, 4, new Map([[201, 2]]), { supporter: "teilnehmer" }); + expect(result.ok).toBe(true); + if (!result.ok) return; + expect(result.entries.find((e) => e.fromId === 201)).toMatchObject({ toId: 29, reason: "declared" }); + }); + + it("treats a declared target that does not exist as a blocker, not a fallback", () => { + const result = derive(5, 4, new Map([[201, 2]]), { Supporter: "Nonexistent" }); + expect(result.ok).toBe(false); + if (result.ok) return; + expect(result.unmappable.map((r) => r.name)).toEqual(["Supporter"]); + }); +}); + +describe("deriveRoleMapping — ambiguity", () => { + const duplicated: GroupTypeRole[] = [ + { id: 1, name: "Leiter", groupTypeId: 99, type: "leader", isDefault: false }, + { id: 2, name: "Leiter", groupTypeId: 99, type: "leader", isDefault: false }, + { id: 3, name: "Mitglied", groupTypeId: 99, type: "participant", isDefault: true }, + ]; + + it("refuses a name match when the target type carries that name twice", () => { + const result = deriveRoleMapping({ + sourceRoles: [{ id: 108, name: "Leiter", groupTypeId: 5, type: "leader" }], + targetRoles: duplicated, + memberCounts: new Map([[108, 3]]), + }); + expect(result.ok).toBe(false); + if (result.ok) return; + expect(result.unmappable[0]).toMatchObject({ name: "Leiter", members: 3 }); + }); +}); + +describe("deriveRoleMapping — the migration this was verified against", () => { + it("reverses the live Local Lead -> AppModule migration onto the ruleset's own role", () => { + // amCheckin (#1817) after the UI moved it 18 -> 21: three members sat in 171, and + // rulesets/amcheckin.json assigns appmodule/"Read" (279). Name matching handles the rest. + const result = derive(21, 18, new Map([[171, 3]]), { Teilnehmer: "Read" }); + expect(result.ok).toBe(true); + if (!result.ok) return; + expect(roleMappingPayload({ fromGroupTypeId: 21, toGroupTypeId: 18, entries: result.entries })).toEqual({ + "66": 60, + "69": 63, + "171": 279, + "174": 162, + "177": 162, + }); + }); +}); + +describe("unmappableRolesError", () => { + it("names the role, its member count and the target type's roles", () => { + const message = unmappableRolesError("team_academy_first_year", 5, 4, [ + { id: 201, name: "Supporter", members: 2, candidates: ["Mitglied", "Leiter"] }, + ]).message; + expect(message).toContain("team_academy_first_year"); + expect(message).toContain("groupTypeId 5 -> 4"); + expect(message).toContain('"Supporter" (role 201, 2 member(s))'); + expect(message).toContain("POST /groups/{id}/grouptype"); + expect(message).toContain("roleMapping"); + expect(message).toContain("Mitglied, Leiter"); + }); +}); From 318da5870dd4d1d76e928684cbafee49b23197bf Mon Sep 17 00:00:00 2001 From: Felix Kotschenreuther Date: Mon, 28 Sep 2026 14:20:36 +0200 Subject: [PATCH 2/3] style: prettier-format the group-field-decisions table Claude-Session: https://claude.ai/code/session_016XVmiQY44pjx1u9Fu4iSDP --- docs/group-field-decisions.md | 28 ++++++++++++++-------------- 1 file changed, 14 insertions(+), 14 deletions(-) diff --git a/docs/group-field-decisions.md b/docs/group-field-decisions.md index b2e467d..449158e 100644 --- a/docs/group-field-decisions.md +++ b/docs/group-field-decisions.md @@ -23,20 +23,20 @@ field — a plain top-level key. `campusId` is wired the same deliberate way as ## Decision table -| Field | Decision | Rationale | -| ---------------------------------------------------- | -------------------------- | --------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------- | -| `name` | **managed** | Core identity; already managed. | -| `groupTypeId` | **managed** | Determines the group's kind and role template; already managed. **Changing it on an existing group is a MIGRATION, not a normal update:** ChurchTools rejects `groupTypeId` on the regular group update (`HTTP 400 groupTypeId: validation.always.invalid`) and requires `POST /groups/{id}/grouptype` with a role mapping, because that mapping decides where existing memberships land. ct now plans and applies it as such (#171): the plan renders the endpoint and the full role mapping, apply POSTs the migration and withholds `groupTypeId` from the PATCH. The mapping is derived only where it cannot cost anyone their role — a same-named role in the target type, or a role holding no members — and a role that holds members with no same-named target is refused at plan time until a `roleMapping` is declared on the group. | -| `groupStatusId` | **managed** | Lifecycle status; already managed. | -| **`campusId`** | **managed (new, #21)** | The campus link is the tool's core "instantiate this area per campus" requirement. Numeric escape hatch only — an existing CT campus id (or `null` to clear). A _logical_ `campus: "key"` reference (resolving a same-run campus by key) is **deferred to [#20](https://github.com/eqrm/ct-cli/issues/20)**; the DSL rejects a `campus` field with a pointer to #20 so it can't slip through as an un-diffable phantom. | -| `parents` (hierarchy) | **opt-in synthetic** | Group→group hierarchy is reconciled through its own endpoint, not the group body — see `src/engine/synthetic.ts`. Opt-in via `parents: [...]`. | -| `dynamic` (auto-group ruleset) | **opt-in synthetic** | Ruleset + status live behind a dedicated endpoint (#14); opt-in via the `dynamic` block. See `docs/handbuch/dynamic-groups.md`. | -| `allowDuplicateName` (CT's `force` create flag, #75) | **create-only, unmanaged** | Not a field on the live group at all — it's a request-only escape hatch for `POST /groups`' same-name guard (`forbidden.duplicate.group`). Opt-in per declaration; sent as `force: true` on CREATE only. Deliberately NOT a registry `managedFields` entry: it has no live value to diff against, so it is destructured out of the DSL input before the field bag (never triggers the unknown-field warning, never lands in state, never adopted, never touched on update). | -| `visibility` | **out of scope** | Not rights-bearing structure; instance-/policy-specific and easily changed in the UI. No demand in #21 to manage it. Promote later only with its own registry entry + tests. | -| `note` | **out of scope** | Free-text annotation, not structure. Managing it would fight human edits in the UI for no structural benefit. | -| `autoAccept` / open-for-members settings | **out of scope** | Membership-request policy — adjacent to _who is in a group_, which the tool never manages (`assertNotPeople`, `src/engine/guard.ts`). Left to the UI. | -| chat status | **out of scope** | Chat/messaging toggle, outside the structural mandate (README: "campuses, structural groups, hierarchies, group types/roles, permission & auto-groups"). | -| sort key | **out of scope** | Presentation ordering, not structure. (Note: `sortKey` _is_ managed on the master-data types `age-group`/`target-group`, where ordering is the resource's point; on a group it is cosmetic.) | +| Field | Decision | Rationale | +| ---------------------------------------------------- | -------------------------- | ---------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------- | +| `name` | **managed** | Core identity; already managed. | +| `groupTypeId` | **managed** | Determines the group's kind and role template; already managed. **Changing it on an existing group is a MIGRATION, not a normal update:** ChurchTools rejects `groupTypeId` on the regular group update (`HTTP 400 groupTypeId: validation.always.invalid`) and requires `POST /groups/{id}/grouptype` with a role mapping, because that mapping decides where existing memberships land. ct now plans and applies it as such (#171): the plan renders the endpoint and the full role mapping, apply POSTs the migration and withholds `groupTypeId` from the PATCH. The mapping is derived only where it cannot cost anyone their role — a same-named role in the target type, or a role holding no members — and a role that holds members with no same-named target is refused at plan time until a `roleMapping` is declared on the group. | +| `groupStatusId` | **managed** | Lifecycle status; already managed. | +| **`campusId`** | **managed (new, #21)** | The campus link is the tool's core "instantiate this area per campus" requirement. Numeric escape hatch only — an existing CT campus id (or `null` to clear). A _logical_ `campus: "key"` reference (resolving a same-run campus by key) is **deferred to [#20](https://github.com/eqrm/ct-cli/issues/20)**; the DSL rejects a `campus` field with a pointer to #20 so it can't slip through as an un-diffable phantom. | +| `parents` (hierarchy) | **opt-in synthetic** | Group→group hierarchy is reconciled through its own endpoint, not the group body — see `src/engine/synthetic.ts`. Opt-in via `parents: [...]`. | +| `dynamic` (auto-group ruleset) | **opt-in synthetic** | Ruleset + status live behind a dedicated endpoint (#14); opt-in via the `dynamic` block. See `docs/handbuch/dynamic-groups.md`. | +| `allowDuplicateName` (CT's `force` create flag, #75) | **create-only, unmanaged** | Not a field on the live group at all — it's a request-only escape hatch for `POST /groups`' same-name guard (`forbidden.duplicate.group`). Opt-in per declaration; sent as `force: true` on CREATE only. Deliberately NOT a registry `managedFields` entry: it has no live value to diff against, so it is destructured out of the DSL input before the field bag (never triggers the unknown-field warning, never lands in state, never adopted, never touched on update). | +| `visibility` | **out of scope** | Not rights-bearing structure; instance-/policy-specific and easily changed in the UI. No demand in #21 to manage it. Promote later only with its own registry entry + tests. | +| `note` | **out of scope** | Free-text annotation, not structure. Managing it would fight human edits in the UI for no structural benefit. | +| `autoAccept` / open-for-members settings | **out of scope** | Membership-request policy — adjacent to _who is in a group_, which the tool never manages (`assertNotPeople`, `src/engine/guard.ts`). Left to the UI. | +| chat status | **out of scope** | Chat/messaging toggle, outside the structural mandate (README: "campuses, structural groups, hierarchies, group types/roles, permission & auto-groups"). | +| sort key | **out of scope** | Presentation ordering, not structure. (Note: `sortKey` _is_ managed on the master-data types `age-group`/`target-group`, where ordering is the resource's point; on a group it is cosmetic.) | ## State-snapshot migration From 82a50cd4da327abf96680a41f563566feb99574c Mon Sep 17 00:00:00 2001 From: Felix Kotschenreuther Date: Mon, 28 Sep 2026 14:51:40 +0200 Subject: [PATCH 3/3] fix(apply): page member/role reads and refuse unsafe group-type migrations - read /group/roles and group members with getAll: a role whose holders sit past page 1 counted as empty and was mapped away by empty-role - refuse a groupTypeId change whose target is a pending ref (was silently planned as the PATCH CT always rejects) - refuse roleMapping keys that name no current-type role, and occupied roles the catalog does not list under the current type - name a failed DECLARED target in the refusal instead of asking for it again - render the role mapping in the markdown plan - replace real group identifiers in docs/tests with generic ones --- docs/configuration.md | 8 +- src/engine/grouptype.ts | 87 +++++++++++++--- src/engine/markdown.ts | 20 ++++ tests/grouptype-migration-apply.test.ts | 128 +++++++++++++++++++----- tests/grouptype-migration.test.ts | 8 +- 5 files changed, 204 insertions(+), 47 deletions(-) diff --git a/docs/configuration.md b/docs/configuration.md index f0634c5..1cbf38b 100644 --- a/docs/configuration.md +++ b/docs/configuration.md @@ -115,9 +115,9 @@ memberships land**: whoever holds a role mapping to `X` holds `X` afterwards. apply will actually do: ``` - ~ group.team_academy_first_year (#1182) + ~ group.youth_team (#42) groupTypeId: 5 -> 4 - via POST /groups/1182/grouptype — role mapping (members follow their role): + via POST /groups/42/grouptype — role mapping (members follow their role): Mitglied -> Mitglied (7 members, matched by name) Leiter -> Leiter (2 members, matched by name) Supporter -> Mitglied (0 members, matched by empty-role) @@ -137,8 +137,8 @@ target type's roles, and you answer it on the group: ```ts ct.group({ - key: "team_academy_first_year", - name: "Academy First Year 26/27", + key: "youth_team", + name: "Youth Team", groupType: "merkmal", roleMapping: { Supporter: "Mitglied" }, }); diff --git a/src/engine/grouptype.ts b/src/engine/grouptype.ts index aaf00e1..6de5fd9 100644 --- a/src/engine/grouptype.ts +++ b/src/engine/grouptype.ts @@ -60,6 +60,8 @@ export interface UnmappableRole { members: number; /** Role names of the target type, offered in the error so the operator can declare one. */ candidates: string[]; + /** Set when a DECLARED target is what failed — missing from, or ambiguous in, the target type. */ + declaredTarget?: string; } export interface GroupTypeMigration { @@ -134,7 +136,13 @@ export function deriveRoleMapping(input: DeriveRoleMappingInput): DeriveRoleMapp const target = uniqueTarget(declaredTargetName); if (!target) { // A declared mapping that does not land is a config error, not a guess to be papered over. - unmappable.push({ id: source.id, name: source.name, members, candidates }); + unmappable.push({ + id: source.id, + name: source.name, + members, + candidates, + declaredTarget: declaredTargetName, + }); continue; } entries.push({ @@ -208,6 +216,22 @@ export function memberCountsByRole(members: { groupTypeRoleId?: number }[]): Map return counts; } +type ListReader = { + get(path: string): Promise; + getAll?(path: string, options?: { limit?: number }): Promise<{ data: T[] }>; +}; + +/** + * Read a whole CT list, every page (#101). Load-bearing for the member read: a role whose holders all + * sit past the first page would count as EMPTY and be mapped away by `empty-role` — moving people + * without asking, the one thing this module exists to prevent. `getAll` is optional only so `{ get }` + * test doubles stay usable; the real client always provides it. + */ +async function readList(client: ListReader, path: string): Promise { + const rows = client.getAll ? (await client.getAll(path)).data : await client.get(path); + return Array.isArray(rows) ? rows : []; +} + /** * Resolve the migration for every group update whose `groupTypeId` changes, so the plan renders the * same mapping the apply will send. @@ -220,7 +244,7 @@ export function memberCountsByRole(members: { groupTypeRoleId?: number }[]): Map * the alternative is rendering an applicable-looking plan that CT rejects mid-apply (#171). */ export async function resolveGroupTypeMigrations( - client: { get(path: string): Promise }, + client: ListReader, plan: Plan, declaredByKey: ReadonlyMap | undefined>, ): Promise { @@ -233,21 +257,56 @@ export async function resolveGroupTypeMigrations( ); if (migrating.length === 0) return; - const roles = await client.get("/group/roles"); - const catalog = Array.isArray(roles) ? roles : []; + const catalog = await readList(client, "/group/roles"); for (const item of migrating) { const change = item.changes.find((c) => c.field === "groupTypeId")!; - const fromGroupTypeId = Number(change.from); - const toGroupTypeId = Number(change.to); - if (!Number.isFinite(fromGroupTypeId) || !Number.isFinite(toGroupTypeId)) continue; + // Skipping here would leave `groupTypeId` on the ordinary update, which CT always rejects — the + // very #171 failure. A target that is still a pending ref (a group type created in this same run) + // has no roles to map onto yet, so refuse and say how to converge instead. + const fromGroupTypeId = typeof change.from === "number" ? change.from : NaN; + const toGroupTypeId = typeof change.to === "number" ? change.to : NaN; + if (!Number.isInteger(fromGroupTypeId) || !Number.isInteger(toGroupTypeId)) { + throw new Error( + `group "${item.key}": cannot plan the groupTypeId change ${JSON.stringify(change.from)} -> ` + + `${JSON.stringify(change.to)} as a migration: both types must already exist in ChurchTools, because ` + + `the role mapping is read from them. If the target type is created in this run, apply it first ` + + `(e.g. without the group's type change), then re-plan.`, + ); + } - const members = await client.get<{ groupTypeRoleId?: number }[]>(`/groups/${item.id}/members?limit=200`); + const members = await readList<{ groupTypeRoleId?: number }>(client, `/groups/${item.id}/members`); const declared = declaredByKey.get(item.key); + const sourceRoles = rolesOfType(catalog, fromGroupTypeId); + const memberCounts = memberCountsByRole(members); + + // A declared key that names no role of the current type is a typo, not a no-op: ignoring it would + // let a name match (or empty-role) decide where members land, contrary to what the config says. + const sourceSlugs = new Set(sourceRoles.map((r) => slug(r.name))); + const strayKeys = Object.keys(declared ?? {}).filter((k) => !sourceSlugs.has(slug(k))); + if (strayKeys.length > 0) { + throw new Error( + `group "${item.key}": roleMapping names ${strayKeys.map((k) => `"${k}"`).join(", ")}, which ` + + `${strayKeys.length === 1 ? "is not a role" : "are not roles"} of its current group type ` + + `${fromGroupTypeId} (${sourceRoles.map((r) => r.name).join(", ") || "no roles"}).`, + ); + } + + // Every occupied role must be covered by the mapping. A member holding a role the catalog did not + // list for the current type would otherwise fall outside the payload with nobody noticing. + const covered = new Set(sourceRoles.map((r) => r.id)); + const orphaned = [...memberCounts.keys()].filter((id) => !covered.has(id)); + if (orphaned.length > 0) { + throw new Error( + `group "${item.key}": members hold role id(s) ${orphaned.join(", ")}, which /group/roles does not ` + + `list under the group's current type ${fromGroupTypeId}; refusing to migrate without mapping them.`, + ); + } + const result = deriveRoleMapping({ - sourceRoles: rolesOfType(catalog, fromGroupTypeId), + sourceRoles, targetRoles: rolesOfType(catalog, toGroupTypeId), - memberCounts: memberCountsByRole(Array.isArray(members) ? members : []), + memberCounts, ...(declared ? { declared } : {}), }); if (!result.ok) { @@ -268,9 +327,11 @@ export function unmappableRolesError( toGroupTypeId: number, unmappable: UnmappableRole[], ): Error { - const lines = unmappable.map( - (role) => - ` - "${role.name}" (role ${role.id}, ${role.members} member(s)) has no same-named role in group type ${toGroupTypeId}`, + const lines = unmappable.map((role) => + role.declaredTarget !== undefined + ? ` - "${role.name}" (role ${role.id}, ${role.members} member(s)) is declared -> "${role.declaredTarget}", ` + + `but group type ${toGroupTypeId} has no single role of that name (missing or ambiguous)` + : ` - "${role.name}" (role ${role.id}, ${role.members} member(s)) has no same-named role in group type ${toGroupTypeId}`, ); return new Error( `group "${groupKey}": cannot change groupTypeId ${fromGroupTypeId} -> ${toGroupTypeId} without a role mapping.\n` + diff --git a/src/engine/markdown.ts b/src/engine/markdown.ts index d241b88..b6c4e7d 100644 --- a/src/engine/markdown.ts +++ b/src/engine/markdown.ts @@ -691,6 +691,26 @@ function renderResourceSection( "", ); } + // A group-type change moves memberships through its own endpoint (#171). The text renderer spells + // the mapping out; this view is what a GitOps reviewer approves, so it must not show less. + if (item.groupTypeMigration) { + lines.push( + locale === "de-DE" + ? `Gruppentyp-Wechsel über \`POST /groups/${item.id}/grouptype\` — Mitglieder folgen ihrer Rolle:` + : `Group-type migration via \`POST /groups/${item.id}/grouptype\` — members follow their role:`, + "", + ); + for (const entry of item.groupTypeMigration.entries) { + const members = + locale === "de-DE" + ? `${entry.members} ${entry.members === 1 ? "Mitglied" : "Mitglieder"}` + : `${entry.members} ${entry.members === 1 ? "member" : "members"}`; + lines.push( + `- ${escapeMarkdown(entry.fromName)} → ${escapeMarkdown(entry.toName)} (${members}, ${entry.reason})`, + ); + } + lines.push(""); + } } return lines; } diff --git a/tests/grouptype-migration-apply.test.ts b/tests/grouptype-migration-apply.test.ts index 7bea143..7b0b947 100644 --- a/tests/grouptype-migration-apply.test.ts +++ b/tests/grouptype-migration-apply.test.ts @@ -2,6 +2,7 @@ import { describe, expect, it, vi } from "vitest"; import { executePlan } from "../src/engine/execute.js"; import { resolveGroupTypeMigrations } from "../src/engine/grouptype.js"; +import { renderPlanMarkdown } from "../src/engine/markdown.js"; import { renderPlan } from "../src/engine/render.js"; import { emptyState, type State } from "../src/state/state.js"; import type { CtClient } from "../src/api/ctClient.js"; @@ -34,15 +35,15 @@ function mockClient(members: { groupTypeRoleId?: number }[]) { const typeChange = (): PlanItem => ({ type: "group", - key: "team_academy_first_year", - displayName: "Academy First Year", - id: 1182, + key: "youth_team", + displayName: "Youth Team", + id: 42, action: "update", changes: [ { field: "groupTypeId", from: 5, to: 4, source: "config" }, - { field: "name", from: "Academy", to: "Academy First Year", source: "config" }, + { field: "name", from: "Youth", to: "Youth Team", source: "config" }, ], - actual: { name: "Academy", groupTypeId: 5, groupStatusId: 1, campusId: null }, + actual: { name: "Youth", groupTypeId: 5, groupStatusId: 1, campusId: null }, }); const planWith = (item: PlanItem): Plan => ({ items: [item] }); @@ -86,14 +87,24 @@ describe("resolveGroupTypeMigrations", () => { ); }); + it("counts members past the first page — a role is not 'empty' because page 1 missed it", async () => { + // A plain GET sees only page 1 (no Supporter); the full list has two. Reading page 1 alone would + // map Supporter away as `empty-role` and move both people without asking. + const { client: base } = mockClient([]); + const getAll = vi.fn(async (path: string) => ({ + data: path === "/group/roles" ? ROLE_CATALOG : [{ groupTypeRoleId: 201 }, { groupTypeRoleId: 201 }], + })); + const client = { ...base, getAll } as unknown as CtClient; + await expect(resolveGroupTypeMigrations(client, planWith(typeChange()), new Map())).rejects.toThrow( + /"Supporter" \(role 201, 2 member\(s\)\)/, + ); + expect(getAll).toHaveBeenCalledWith("/groups/42/members"); + }); + it("accepts the declared mapping that answers that refusal", async () => { const { client } = mockClient([{ groupTypeRoleId: 201 }, { groupTypeRoleId: 201 }]); const plan = planWith(typeChange()); - await resolveGroupTypeMigrations( - client, - plan, - new Map([["team_academy_first_year", { Supporter: "Leiter" }]]), - ); + await resolveGroupTypeMigrations(client, plan, new Map([["youth_team", { Supporter: "Leiter" }]])); expect(plan.items[0]!.groupTypeMigration!.entries.find((e) => e.fromId === 201)).toMatchObject({ toId: 30, reason: "declared", @@ -110,7 +121,7 @@ describe("renderPlan — a type change must not read like an ordinary field upda const out = renderPlan(plan); expect(out).toContain("groupTypeId: 5 -> 4"); - expect(out).toContain("via POST /groups/1182/grouptype"); + expect(out).toContain("via POST /groups/42/grouptype"); expect(out).toContain("Mitglied -> Mitglied (1 member, matched by name)"); expect(out).toContain("Supporter -> Mitglied (0 members, matched by empty-role)"); }); @@ -122,11 +133,11 @@ describe("executePlan — the migration endpoint, not the group PATCH", () => { const plan = planWith(typeChange()); await resolveGroupTypeMigrations(client, plan, new Map()); const state: State = emptyState(HOST); - state.resources.team_academy_first_year = { + state.resources.youth_team = { type: "group", - id: 1182, - key: "team_academy_first_year", - fields: { name: "Academy", groupTypeId: 5, groupStatusId: 1, campusId: null }, + id: 42, + key: "youth_team", + fields: { name: "Youth", groupTypeId: 5, groupStatusId: 1, campusId: null }, adoptedAt: "2026-01-01T00:00:00.000Z", updatedAt: "2026-01-01T00:00:00.000Z", }; @@ -136,7 +147,7 @@ describe("executePlan — the migration endpoint, not the group PATCH", () => { it("POSTs /groups/{id}/grouptype with the rendered mapping", async () => { const { calls } = await applyTypeChange(); - const migration = calls.find((c) => c.path === "/groups/1182/grouptype"); + const migration = calls.find((c) => c.path === "/groups/42/grouptype"); expect(migration).toBeDefined(); expect(migration!.method).toBe("POST"); expect(migration!.body).toEqual({ @@ -147,23 +158,23 @@ describe("executePlan — the migration endpoint, not the group PATCH", () => { it("withholds groupTypeId from the group PATCH — CT rejects the whole request otherwise", async () => { const { calls } = await applyTypeChange(); - const patch = calls.find((c) => c.method === "PATCH" && c.path === "/groups/1182"); + const patch = calls.find((c) => c.method === "PATCH" && c.path === "/groups/42"); expect(patch).toBeDefined(); - expect(patch!.body).toEqual({ name: "Academy First Year" }); + expect(patch!.body).toEqual({ name: "Youth Team" }); expect(patch!.body).not.toHaveProperty("groupTypeId"); }); it("migrates BEFORE touching other fields, so a refusal leaves the rest untouched", async () => { const { calls } = await applyTypeChange(); const order = calls.map((c) => `${c.method} ${c.path}`); - expect(order.indexOf("POST /groups/1182/grouptype")).toBeLessThan(order.indexOf("PATCH /groups/1182")); + expect(order.indexOf("POST /groups/42/grouptype")).toBeLessThan(order.indexOf("PATCH /groups/42")); }); it("records the new type in state, so a re-plan is a no-op", async () => { const { state } = await applyTypeChange(); - expect(state.resources.team_academy_first_year?.fields).toMatchObject({ + expect(state.resources.youth_team?.fields).toMatchObject({ groupTypeId: 4, - name: "Academy First Year", + name: "Youth Team", }); }); @@ -174,16 +185,81 @@ describe("executePlan — the migration endpoint, not the group PATCH", () => { const plan = planWith(item); await resolveGroupTypeMigrations(client, plan, new Map()); const state: State = emptyState(HOST); - state.resources.team_academy_first_year = { + state.resources.youth_team = { type: "group", - id: 1182, - key: "team_academy_first_year", - fields: { name: "Academy", groupTypeId: 5, groupStatusId: 1, campusId: null }, + id: 42, + key: "youth_team", + fields: { name: "Youth", groupTypeId: 5, groupStatusId: 1, campusId: null }, adoptedAt: "2026-01-01T00:00:00.000Z", updatedAt: "2026-01-01T00:00:00.000Z", }; await executePlan(plan, { client, state, statePath: "unused", save: async () => {} }); expect(calls.filter((c) => c.method === "PATCH")).toEqual([]); - expect(calls.map((c) => c.path)).toEqual(["/groups/1182/grouptype"]); + expect(calls.map((c) => c.path)).toEqual(["/groups/42/grouptype"]); + }); +}); + +describe("resolveGroupTypeMigrations — refusals instead of a silent fallback to the PATCH", () => { + it("refuses a target type that is still a pending ref, rather than planning the PATCH CT rejects", async () => { + const { client } = mockClient([]); + const item = typeChange(); + item.changes[0] = { + field: "groupTypeId", + from: 5, + to: { __pendingRef: "grouptype:new_type" }, + source: "config", + }; + await expect(resolveGroupTypeMigrations(client, planWith(item), new Map())).rejects.toThrow( + /both types must already exist in ChurchTools/, + ); + }); + + it("refuses a declared key that names no role of the current type (a typo is not a no-op)", async () => { + const { client } = mockClient([]); + await expect( + resolveGroupTypeMigrations( + client, + planWith(typeChange()), + new Map([["youth_team", { Leitr: "Mitglied" }]]), + ), + ).rejects.toThrow(/roleMapping names "Leitr", which is not a role of its current group type 5/); + }); + + it("refuses when members hold a role the catalog does not list under the current type", async () => { + const { client } = mockClient([{ groupTypeRoleId: 999 }]); + await expect(resolveGroupTypeMigrations(client, planWith(typeChange()), new Map())).rejects.toThrow( + /role id\(s\) 999/, + ); + }); + + it("says a DECLARED target is what failed, instead of asking for the declaration again", async () => { + const { client } = mockClient([{ groupTypeRoleId: 201 }]); + await expect( + resolveGroupTypeMigrations( + client, + planWith(typeChange()), + new Map([["youth_team", { Supporter: "Coach" }]]), + ), + ).rejects.toThrow(/is declared -> "Coach", but group type 4 has no single role of that name/); + }); +}); + +describe("renderPlanMarkdown — the GitOps view shows the mapping too", () => { + it("lists every role's destination under the migration endpoint", async () => { + const { client } = mockClient([{ groupTypeRoleId: 105 }]); + const plan = planWith(typeChange()); + await resolveGroupTypeMigrations(client, plan, new Map()); + const out = renderPlanMarkdown(plan, [], { + environment: "test", + host: HOST, + churchToolsVersion: "3.137.0", + configPath: "ct.config.ts", + stateHost: HOST, + generatedAt: new Date("2026-09-28T12:00:00.000Z"), + locale: "en", + }); + expect(out).toContain("Group-type migration via `POST /groups/42/grouptype`"); + expect(out).toContain("- Mitglied → Mitglied (1 member, name)"); + expect(out).toContain("- Supporter → Mitglied (0 members, empty-role)"); }); }); diff --git a/tests/grouptype-migration.test.ts b/tests/grouptype-migration.test.ts index a636dd7..4998352 100644 --- a/tests/grouptype-migration.test.ts +++ b/tests/grouptype-migration.test.ts @@ -10,7 +10,7 @@ import { } from "../src/engine/grouptype.js"; /** - * Real role catalogs read from a live instance (eqrm-dev, CT 3.137.0-RC21, 2026-09-28). Kept verbatim + * Real role catalogs read from a live instance (a dev instance, CT 3.137.0-RC21, 2026-09-28). Kept verbatim * rather than simplified: the interesting cases in #171 are exactly the shapes CT actually ships — * a type whose roles are a superset of the target's, and stock lowercase role keys sitting next to * church-named ones. @@ -74,7 +74,7 @@ describe("memberCountsByRole", () => { }); }); -describe("deriveRoleMapping — the Academy case (#139): Team -> Merkmal, no members", () => { +describe("deriveRoleMapping — Team -> Merkmal, no members", () => { it("maps same-named roles by name and routes the rest to the default, since nothing can move", () => { const result = derive(5, 4, new Map()); expect(result.ok).toBe(true); @@ -220,10 +220,10 @@ describe("deriveRoleMapping — the migration this was verified against", () => describe("unmappableRolesError", () => { it("names the role, its member count and the target type's roles", () => { - const message = unmappableRolesError("team_academy_first_year", 5, 4, [ + const message = unmappableRolesError("youth_team", 5, 4, [ { id: 201, name: "Supporter", members: 2, candidates: ["Mitglied", "Leiter"] }, ]).message; - expect(message).toContain("team_academy_first_year"); + expect(message).toContain("youth_team"); expect(message).toContain("groupTypeId 5 -> 4"); expect(message).toContain('"Supporter" (role 201, 2 member(s))'); expect(message).toContain("POST /groups/{id}/grouptype");