diff --git a/docs/handbuch/blueprints.md b/docs/handbuch/blueprints.md index 867536c..b0aef90 100644 --- a/docs/handbuch/blueprints.md +++ b/docs/handbuch/blueprints.md @@ -4,8 +4,8 @@ sources: - src/config/context.ts - src/engine/graph.ts - src/engine/hierarchy.ts -sources_hash: bb6f89956ace833d -reviewed: 2026-08-26 +sources_hash: 6f4be8d3a93113ce +reviewed: 2026-08-28 --- # Blueprints (parametrized, reusable config) @@ -249,6 +249,12 @@ fields are the surface where this matters most in a blueprint, because a field is owned by exactly one group: a blueprint instantiated twice creates two independent `wahl` fields, and a reference must say _which group's_. +The reference's `` is the portable local ct-cli key. Its declaration may +map that key to a different, exact ChurchTools `referenceName` (#158), for +example `key: "stand_bewerbung"` plus `referenceName: "stand-bewerbung"`. +Ruleset resolution follows that mapping; it never treats `-` and `_` as the +same API identity. + This matters more in a blueprint than in a hand-written flat config, because the `${campus}_`-prefixed key is itself computed (`` `${campus}_kids_lead` ``, not a literal string) — a copy-paste slip in diff --git a/docs/handbuch/dynamic-groups.md b/docs/handbuch/dynamic-groups.md index 6ef723a..c30d134 100644 --- a/docs/handbuch/dynamic-groups.md +++ b/docs/handbuch/dynamic-groups.md @@ -6,8 +6,8 @@ sources: - src/engine/dynamic.ts - src/engine/synthetic.ts - src/application/operations/adopt-group.ts -sources_hash: c49d089ed6662ea7 -reviewed: 2026-08-17 +sources_hash: e4e2d552d54802f9 +reviewed: 2026-08-28 --- # Auto-groups (dynamic groups) @@ -87,16 +87,24 @@ inside a ruleset, so the wrong id applies cleanly and silently computes the wrong membership. Use the group-scoped reference instead: ```ts -ref.groupMemberField("ojbp_2026_27_praktikum_1", "wahl"); +ref.groupMemberField("ojbp_2026_27_praktikum_1", "stand_bewerbung"); ``` -The resolver maps it to this host's id from -`GET /groups/{groupId}/memberfields`. A field this config declares but that does -not exist on the host yet resolves to a pending marker and is completed during -apply, right after the create that minted it. A reference to a field the target -group does not declare fails at config-eval time, before any network call — the -declared key and the referenced one being compared in their normalised form, so -`"Wahl"` finds a field declared as `wahl`. See +The second argument is the portable local ct-cli key, not ChurchTools' API +identity. Its field declaration may map it explicitly to an exact +`referenceName`, for example `key: "stand_bewerbung"` and +`referenceName: "stand-bewerbung"`. The resolver uses that declaration to find +the host's numeric id from `GET /groups/{groupId}/memberfields`; the live +`referenceName` is compared byte-for-byte, so `stand-bewerbung` and +`stand_bewerbung` are not interchangeable (#158). + +A field this config declares but that does not exist on the host yet resolves +to a pending marker and is completed during apply, right after the create that +minted it. A reference to a field the target group does not declare fails at +config-eval time, before any network call. Local keys are compared in their +normalised form, so `"Stand Bewerbung"` finds a declaration keyed +`stand_bewerbung`; that normalization never changes the declaration's exact API +`referenceName`. See [Group member fields](group-member-fields.md). ## Supplying a ruleset — three ways diff --git a/docs/handbuch/group-member-fields.md b/docs/handbuch/group-member-fields.md index ea79a48..a12e272 100644 --- a/docs/handbuch/group-member-fields.md +++ b/docs/handbuch/group-member-fields.md @@ -1,5 +1,5 @@ --- -sources_hash: 984c67a4f952f9fc +sources_hash: 740acdc4e12e5d8e title: Group member fields sources: - src/engine/member-fields.ts @@ -7,10 +7,10 @@ sources: - src/config/context.ts - src/application/operations/adopt-group.ts - src/application/operations/destroy.ts -reviewed: 2026-08-26 +reviewed: 2026-08-28 --- -# Group member fields (#135) +# Group member fields (#135, #158) A ChurchTools group can ask its members for extra information — "Wahl", "Praktikumsplatz", a free-text note. Those are **group member fields**: field @@ -60,8 +60,9 @@ ct.group({ groupTypeId: 5, memberFields: [ { - key: "wahl", - name: "Wahl", + key: "stand_bewerbung", + referenceName: "stand-bewerbung", + name: "Stand", fieldTypeCode: "text", requiredInRegistrationForm: true, }, @@ -77,8 +78,11 @@ ct.group({ | `memberFields: []` | managed, none declared — still never deletes an existing field | | `memberFields: [ … ]` | these fields are created/updated; anything else is left in place | -`key` is the group-local key and is unique within the group. Everything else is -a ChurchTools member-field property. +`key` is the group-local ct-cli key and is unique within the group. +`referenceName` is the exact ChurchTools identity used by dynamic rulesets. The +two are deliberately separate: `key: "stand_bewerbung"` may carry +`referenceName: "stand-bewerbung"`. If `referenceName` is omitted it defaults +to `key` for backwards compatibility. ### Managed properties @@ -86,15 +90,17 @@ a ChurchTools member-field property. `noteInSignupForm`, `requiredInRegistrationForm`, `useInRegistrationForm`, `securityLevel`, `sortKey`. -Two readable/writable properties are deliberately **not** managed: +Two readable/writable properties need special handling: - **`id`** — host-specific; see above. -- **`referenceName`** — this _is_ the local identity, not a diffable property. - It is what a create sends and, when no state-bound id exists, what a later run - matches on. Managing it would let a rename silently re-key the resource and - re-create the field instead of updating it. (A field created in the - ChurchTools UI, where CT may mint its own `referenceName`, is matched by its - slugged `name` as a fallback.) +- **`referenceName`** — exact ChurchTools identity, kept separate from the local + key. It is sent unchanged on create and compared byte-for-byte on every later + plan; punctuation and case are significant, so `foo-bar` and `foo_bar` are + different. It is never silently PATCHed. A mismatch makes the plan + **INCOMPLETE** and tells the operator to perform an explicit replacement with + `ct destroy --member-field ::` followed by plan/apply. A + name fallback is used only for legacy/UI rows that genuinely carry no + `referenceName`; an existing different value is never ignored. A property outside the managed list still passes through to ChurchTools unchanged — it only earns a warning, and it is never diffed. @@ -108,7 +114,9 @@ ct adopt group --children-of ojbp_2025_26 --with-member-fields The emitted snippet carries a `memberFields:` block with every group-scoped field and **no ChurchTools ids** — paste it, re-key it for the next year, and -`ct plan` proposes fresh groups and fresh fields. +`ct plan` proposes fresh groups and fresh fields. Adoption keeps every non-empty +live `referenceName` byte-for-byte in the snippet while deriving the separate +local `key` as a portable slug. The same adoption stores each live field id in the owning group's instance-specific `memberFields` state map. This is deliberately separate from @@ -154,9 +162,10 @@ already adopted in that run. memberField:wahl: {"name":"Wahl"} -> {"name":"Wahl (neu)"} ``` -The actual side is narrowed to exactly the properties the declaration names, so -a server default ChurchTools returns can never make the two sides differ -forever: **a clean apply re-plans as a no-op.** +The actual side includes the exact `referenceName` and is otherwise narrowed to +the mutable properties the declaration names, so a server default ChurchTools +returns can never make the two sides differ forever: **a clean apply re-plans +as a no-op.** The same projection applies inside `options`: ChurchTools assigns host-specific ids to select options, while a portable config can declare `{ name }`. Those @@ -214,11 +223,12 @@ elsewhere, so a member field is referenced by its portable identity: ref.groupMemberField("ojbp_2026_27_praktikum_1", "wahl"); ``` -Local keys are compared in their **normalised** form throughout — `"Wahl"` and -`"wahl"` are the same field, whether they appear in a declaration, in a -reference, in `ct destroy --member-field`, or as ChurchTools' own -`referenceName` on the live row. Two declarations in one group that differ only -in case are therefore rejected as duplicates. +Local ct-cli keys are compared in their **normalised** form throughout — +`"Wahl"` and `"wahl"` are the same local key in declarations, typed references, +state and `ct destroy --member-field`. Two declarations in one group that +differ only in case are therefore rejected as duplicates. ChurchTools +`referenceName` is a separate value and is always compared exactly; local-key +normalisation never applies to it. Three things follow: diff --git a/docs/handbuch/permissions.md b/docs/handbuch/permissions.md index 2ce8587..9c63b46 100644 --- a/docs/handbuch/permissions.md +++ b/docs/handbuch/permissions.md @@ -7,8 +7,8 @@ sources: - src/resolve/resolver.ts - src/resolve/refs.ts - src/config/context.ts -sources_hash: b5486f5d014a5912 -reviewed: 2026-08-26 +sources_hash: 469fab5b33c1e5c4 +reviewed: 2026-08-28 --- # Permissions (`ct.groupRole` / `ct.groupTypeRole` / `ct.status`) @@ -479,7 +479,9 @@ group-scoped member-field definition, addressed by its portable `(group key, local field key)` pair — #135). They share this file's resolver and its "managed state first, then live lookup, else a hard error at plan time" rules, but they are referenced from **dynamic-group rulesets**, not from grant -scopes. See [Group member fields](group-member-fields.md). +scopes. For `group-member-field`, the logical pair selects a declaration; that +declaration's exact ChurchTools `referenceName` selects the live row (#158). +See [Group member fields](group-member-fields.md). **Why this matters:** campus ids are host-specific — Mainz is `0` on eqrm prod and `6` on eqrm dev. A campus-scoped grant written as a numeric literal is diff --git a/src/application/operations/adopt-group.ts b/src/application/operations/adopt-group.ts index a8e8493..f7ccdd7 100644 --- a/src/application/operations/adopt-group.ts +++ b/src/application/operations/adopt-group.ts @@ -224,8 +224,8 @@ interface MemberFieldsCapture { * Emits `{ key, ...managed properties }` per field and **never a ChurchTools id** — the field's * identity in config is the group key plus its local key (`ojbp_2026_27_praktikum_1::wahl`), so the * same blueprint applied to another group, or another host, mints its own fields rather than - * resolving against this host's numbering. The local key comes from CT's own `referenceName` - * (slugged), falling back to the slugged name for a field created in the ChurchTools UI. + * resolving against this host's numbering. The local key is a slug for ct-cli/state only; the live + * `referenceName` is emitted separately and byte-for-byte so punctuation remains API identity. * * Opt-in only (`--with-member-fields`) — and that opt-in is TRANSITIONAL, not a statement that * member fields are optional: they are a category-2 owned structural child, so the flip to @@ -296,6 +296,9 @@ async function captureMemberFields( } ids[canonical] = fieldId; const declaration: Record = { key: localKeyOf(row) }; + if (typeof row.referenceName === "string" && row.referenceName.length > 0) { + declaration.referenceName = row.referenceName; + } for (const prop of MEMBER_FIELD_PROPS) { if (row[prop] !== undefined) declaration[prop] = row[prop]; } diff --git a/src/config/context.ts b/src/config/context.ts index c5593c0..3b1c3e9 100644 --- a/src/config/context.ts +++ b/src/config/context.ts @@ -101,6 +101,8 @@ export type DynamicInput = boolean | string | { status: DynamicStatus; ruleset: */ export interface MemberFieldInput { key: string; + /** Exact ChurchTools/ruleset identity; defaults to the local ct-cli key. Never silently renamed. */ + referenceName?: string; [prop: string]: unknown; } @@ -132,6 +134,8 @@ export interface ResourceInput { * (`ojbp_2026_27_praktikum_1::wahl`), because a member field belongs to exactly one group and is * not globally reusable. Two groups declaring `wahl` stay independent fields with different * ChurchTools ids. A declaration may never carry a ChurchTools field id. + * `referenceName` is separate exact ChurchTools identity and defaults to `key`; hyphens and + * underscores are not equivalent there because dynamic rulesets use that string verbatim. * * A field dropped from this list is NEVER deleted — see `ct destroy --member-field`. */ @@ -430,11 +434,12 @@ function normalizeMemberFields( } const specs: MemberFieldSpec[] = []; const seen = new Set(); + const seenReferenceNames = new Set(); for (const raw of input) { if (raw === null || typeof raw !== "object" || Array.isArray(raw)) { throw new Error(`group "${key}": each entry of "memberFields" must be an object with a "key".`); } - const { key: localKey, ...props } = raw as MemberFieldInput; + const { key: localKey, referenceName: configuredReferenceName, ...props } = raw as MemberFieldInput; if (typeof localKey !== "string" || localKey.length === 0) { throw new Error(`group "${key}": each entry of "memberFields" needs a non-empty string "key".`); } @@ -450,6 +455,22 @@ function normalizeMemberFields( ); } seen.add(canonical); + if ( + configuredReferenceName !== undefined && + (typeof configuredReferenceName !== "string" || configuredReferenceName.length === 0) + ) { + throw new Error( + `group "${key}" member field "${localKey}": "referenceName" must be a non-empty string when provided.`, + ); + } + const referenceName = configuredReferenceName ?? localKey; + if (seenReferenceNames.has(referenceName)) { + throw new Error( + `group "${key}": duplicate member field referenceName ${JSON.stringify(referenceName)}. ` + + `ChurchTools reference names are exact identities and must be unique within a group.`, + ); + } + seenReferenceNames.add(referenceName); for (const forbidden of MEMBER_FIELD_FORBIDDEN_PROPS) { if (props[forbidden] !== undefined) { throw new Error( @@ -476,7 +497,7 @@ function normalizeMemberFields( ); } } - specs.push({ key: localKey, props }); + specs.push({ key: localKey, referenceName, props }); } return specs; } @@ -685,8 +706,8 @@ function validateMemberFieldRefs(resources: DesiredResource[], byKey: Map memberFieldStateKey(f.key) === memberFieldStateKey(ref.field))) { const declared = target.memberFields.map((f) => f.key).join(", "); throw new Error( diff --git a/src/engine/member-fields.ts b/src/engine/member-fields.ts index 88f53a4..2563630 100644 --- a/src/engine/member-fields.ts +++ b/src/engine/member-fields.ts @@ -67,9 +67,9 @@ export function parseMemberFieldIdentity(raw: string): { group: string; field: s * writable, minus the two that must never be managed: * * - `id` — host-specific, and the entire portability guarantee is that it never appears in config. - * - `referenceName` — this IS the local identity (see {@link matchesLocalKey}), not a diffable - * property. Managing it would let a plain rename silently re-key the resource, and the field - * would then be re-created rather than updated on the next apply. + * - `referenceName` — exact ChurchTools identity, kept separately on {@link MemberFieldSpec}. + * It is compared exactly and sent on create, but never PATCHed: changing it requires an explicit + * replacement because dynamic-group rulesets address this exact string. * * A declared property outside this list still passes through to ChurchTools untouched (the escape * hatch, mirroring the registry's unknown-field behaviour) — it only earns a warning. @@ -101,7 +101,9 @@ export function isManagedMemberFieldProp(name: string): boolean { export interface MemberFieldSpec { /** Local key, unique within the owning group. Half of the portable `::` identity. */ key: string; - /** The declared properties — never an id, never `referenceName`. Diffed as one unit. */ + /** Exact identity used by ChurchTools and by ruleset `groupMemberFields` assignments (#158). */ + referenceName: string; + /** The declared mutable properties — never an id, never `referenceName`. Diffed as one unit. */ props: Record; } @@ -191,10 +193,15 @@ export function knownMemberFieldId(state: State, groupKey: string, localKey: str return group?.memberFields?.[memberFieldStateKey(localKey)]; } -/** Prefer a state-bound id; use the portable live key only when no known id is present in the response. */ +/** + * Prefer a state-bound id; otherwise match ChurchTools' identity-bearing `referenceName` EXACTLY. + * A name fallback is permitted only for old/UI rows that genuinely carry no referenceName. Once CT + * supplies one, punctuation and case are data: `foo-bar` and `foo_bar` are different identities. + */ export function matchingMemberFieldRows( rows: MemberFieldRow[], localKey: string, + referenceName: string, knownId?: number, ): MemberFieldRow[] { if (knownId !== undefined) { @@ -203,23 +210,37 @@ export function matchingMemberFieldRows( // when a response variant was parsed incompletely. return rows.filter((row) => memberFieldRowId(row) === knownId); } - return rows.filter((row) => matchesLocalKey(row, localKey)); + return rows.filter((row) => { + const liveReference = memberFieldReferenceName(row); + if (liveReference !== undefined) return liveReference === referenceName; + const name = row.name; + return typeof name === "string" && slug(name) === slug(localKey); + }); } /** - * The local key a live row answers to. `referenceName` is CT's own stable, non-numeric handle - * within the group and is what a create sends, so it wins; `name` is the fallback for a row created - * in the ChurchTools UI, where CT may mint its own referenceName. Slugged on both sides so a key - * derived from a German name (`Wahl` → `wahl`) matches either spelling. + * The exact identity-bearing ChurchTools reference name, when the row carries one. */ -export function localKeyOf(row: MemberFieldRow): string { +export function memberFieldReferenceName(row: MemberFieldRow): string | undefined { const reference = row.referenceName; - if (typeof reference === "string" && reference.length > 0) return slug(reference); + return typeof reference === "string" && reference.length > 0 ? reference : undefined; +} + +/** + * A stable local key for adoption/delete-candidate display. This is deliberately NOT API identity: + * it remains a ct-cli slug, while {@link memberFieldReferenceName} preserves the exact CT string. + */ +export function localKeyOf(row: MemberFieldRow): string { + const reference = memberFieldReferenceName(row); + if (reference !== undefined) return slug(reference); const name = row.name; return typeof name === "string" ? slug(name) : ""; } -/** Does this live row carry the declared local key? (See {@link localKeyOf}.) */ +/** + * Legacy/local-key affinity used only for diagnostics and destructive target lookup. Never use it + * to establish API identity for plan/apply; use {@link matchingMemberFieldRows} there. + */ export function matchesLocalKey(row: MemberFieldRow, localKey: string): boolean { const wanted = slug(localKey); if (localKeyOf(row) === wanted) return true; diff --git a/src/engine/synthetic.ts b/src/engine/synthetic.ts index 6289f90..d5e09c2 100644 --- a/src/engine/synthetic.ts +++ b/src/engine/synthetic.ts @@ -16,7 +16,6 @@ import { mapConcurrent } from "../util/concurrency.js"; import { info, warn, formatError } from "../ui.js"; import { normalizeDynamic, normalizeRuleset, putRulesetBody, resolveRulesetRef } from "./dynamic.js"; import { formatPortablizeWarnings, scanUnportablized } from "../config/query-refs.js"; -import { slug } from "../resources/registry.js"; import { actualMemberFieldProps, groupScopedRows, @@ -28,11 +27,13 @@ import { memberFieldItemPath, memberFieldLocalKey, memberFieldPseudo, + memberFieldReferenceName, memberFieldRowId, matchingMemberFieldRows, memberFieldsCreatePath, memberFieldsReadPath, MEMBER_FIELD_PREFIX, + matchesLocalKey, type MemberFieldRow, } from "./member-fields.js"; import type { DynamicStatus } from "./types.js"; @@ -215,9 +216,9 @@ async function readMemberFieldsForWrite( * become a delete here. The live field is still surfaced as a DELETE CANDIDATE (a warning * naming its portable identity and the explicit `ct destroy --member-field` that removes it), * so "left alone" never means "unnoticed". - * 2. **Round-trips to a no-op.** The actual side is narrowed to exactly the properties the - * declaration names (`actualMemberFieldProps`), so a server-defaulted sibling CT returns can - * never make the two sides unequal forever. + * 2. **Round-trips to a no-op.** The actual side includes the exact identity-bearing + * `referenceName`, plus mutable properties narrowed to what the declaration names + * (`actualMemberFieldProps`), so server defaults never create cosmetic drift. * 3. **Ordered before `dynamic`.** This entry is registered ahead of the dynamic field, so the * folded `memberField:*` keys land in the desired bag before `dynamic`, `diffFields` emits them * in that order, and `applySyntheticFields` therefore creates the fields before installing a @@ -264,7 +265,43 @@ const memberFieldsField: SyntheticField = { `\`ct destroy --member-field ${identity}\` and re-run.` ); }); - return { key: d.key, rows, stale, errors }; + const mismatched = new Set(); + for (const spec of d.memberFields!) { + if (stale.has(spec.key)) continue; + const knownId = knownMemberFieldId(state, d.key, spec.key); + const matches = matchingMemberFieldRows(rows, spec.key, spec.referenceName, knownId); + const affinity = + matches.length > 0 ? matches : rows.filter((row) => matchesLocalKey(row, spec.key)); + if (affinity.length === 0) continue; + if (affinity.length > 1) { + mismatched.add(spec.key); + const identity = memberFieldIdentity(d.key, spec.key); + errors.push( + `member field ${identity}: no row has the exact ChurchTools referenceName ` + + `${JSON.stringify(spec.referenceName)}, but ${affinity.length} rows collapse onto local ` + + `ct-cli key ${JSON.stringify(spec.key)} (${affinity + .map((row) => JSON.stringify(memberFieldReferenceName(row) ?? "")) + .join(", ")}). "-" and "_" are not equivalent API identities. Refusing to plan a ` + + `duplicate; choose a distinct local key or replace the intended field explicitly.`, + ); + continue; + } + const row = affinity[0]!; + const liveReference = memberFieldReferenceName(row); + if (liveReference === spec.referenceName) continue; + mismatched.add(spec.key); + const identity = memberFieldIdentity(d.key, spec.key); + errors.push( + `member field ${identity} (#${String(memberFieldRowId(row) ?? "unknown")}): exact ` + + `ChurchTools referenceName is ${liveReference === undefined ? "missing" : JSON.stringify(liveReference)}, ` + + `but config requires ${JSON.stringify(spec.referenceName)}. Local ct-cli key ` + + `${JSON.stringify(spec.key)} does not change that API identity; "-" and "_" are not ` + + `equivalent. ct will not rename an identity-bearing field silently. Replace it ` + + `explicitly with \`ct destroy --member-field ${identity}\`, then re-run plan/apply.`, + ); + } + for (const message of errors) warn(message); + return { key: d.key, rows, stale, mismatched, errors }; } catch (err) { // Same honesty rule as the dynamic fold (#126): an unread actual is NOT a known-absent one, // so the desired side stays unfolded and the group is reported unreadable rather than having @@ -273,6 +310,7 @@ const memberFieldsField: SyntheticField = { key: d.key, rows: undefined, stale: new Set(), + mismatched: new Set(), errors: [`member fields ${d.key} (#${managed.id}): ${formatError(err)}`], }; } @@ -282,6 +320,7 @@ const memberFieldsField: SyntheticField = { const unreadableKeys = new Set(unreadable); const rowsByKey = new Map(outcomes.filter((o) => o.rows !== undefined).map((o) => [o.key, o.rows!])); const staleByKey = new Map(outcomes.map((o) => [o.key, o.stale])); + const mismatchedByKey = new Map(outcomes.map((o) => [o.key, o.mismatched])); const augmented = desired.map((d) => { if (d.type !== "group" || d.memberFields === undefined) return d; @@ -290,24 +329,34 @@ const memberFieldsField: SyntheticField = { const a = actual.get(d.key); const fields = { ...d.fields }; const stale = staleByKey.get(d.key); + const mismatched = mismatchedByKey.get(d.key); for (const spec of d.memberFields) { if (stale?.has(spec.key)) continue; // stale state binding — reported above, left unreconciled + if (mismatched?.has(spec.key)) continue; // exact identity mismatch — explicit replacement only const pseudo = memberFieldPseudo(spec.key); - fields[pseudo] = spec.props; + fields[pseudo] = { referenceName: spec.referenceName, ...spec.props }; if (!a || !rows) continue; - const matches = matchingMemberFieldRows(rows, spec.key, knownMemberFieldId(state, d.key, spec.key)); + const matches = matchingMemberFieldRows( + rows, + spec.key, + spec.referenceName, + knownMemberFieldId(state, d.key, spec.key), + ); // >1 live match means the local key is ambiguous on this host — a blind update would pick // one arbitrarily, so leave the actual side absent and let the ambiguity surface where it // can be acted on (the apply path refuses it by name). if (matches.length === 1) { - a[pseudo] = actualMemberFieldProps(matches[0]!, spec.props); + a[pseudo] = { + referenceName: memberFieldReferenceName(matches[0]!), + ...actualMemberFieldProps(matches[0]!, spec.props), + }; } } if (rows) { - const declared = new Set(d.memberFields.map((f) => slug(f.key))); + const declared = new Set(d.memberFields.map((f) => f.referenceName)); for (const row of rows) { const local = localKeyOf(row); - if (!local || declared.has(local)) continue; + if (!local || declared.has(memberFieldReferenceName(row) ?? "")) continue; warn( `group "${d.key}": member field "${memberFieldIdentity(d.key, local)}" exists in ` + `ChurchTools but is not declared — DELETE CANDIDATE, left untouched. ct never removes a ` + @@ -323,14 +372,20 @@ const memberFieldsField: SyntheticField = { async apply({ client, state, id, key, change, reads }) { const local = memberFieldLocalKey(change.field); if (local === undefined) return; - const props = change.to as Record | undefined; + const desired = change.to as Record | undefined; // No desired value = the field is not declared. `diffFields` cannot even produce such a change // (it walks the desired side), so this is belt-and-braces: apply never deletes a member field. - if (props === undefined || props === null) return; + if (desired === undefined || desired === null) return; + const { referenceName, ...props } = desired; + if (typeof referenceName !== "string" || referenceName.length === 0) { + throw new Error( + `group member field "${memberFieldIdentity(key, local)}": desired referenceName is missing.`, + ); + } const rows = await readMemberFieldsForWrite(client, id, reads); const knownId = knownMemberFieldId(state, key, local); - const matches = matchingMemberFieldRows(rows, local, knownId); + const matches = matchingMemberFieldRows(rows, local, referenceName, knownId); if (matches.length > 1) { throw new Error( `group member field "${memberFieldIdentity(key, local)}": ${matches.length} fields on group ` + @@ -348,6 +403,17 @@ const memberFieldsField: SyntheticField = { }; if (matches.length === 1) { + const liveReference = memberFieldReferenceName(matches[0]!); + if (liveReference !== referenceName) { + throw new Error( + `group member field "${memberFieldIdentity(key, local)}": ChurchTools field ` + + `#${String(memberFieldRowId(matches[0]!) ?? "unknown")} has exact referenceName ` + + `${liveReference === undefined ? "" : JSON.stringify(liveReference)}, but config ` + + `requires ${JSON.stringify(referenceName)}. Refusing to rename it silently; replace it ` + + `explicitly with \`ct destroy --member-field ${memberFieldIdentity(key, local)}\`, then ` + + `re-run plan/apply.`, + ); + } const fieldId = memberFieldRowId(matches[0]!); if (fieldId === undefined) { throw new Error( @@ -381,12 +447,10 @@ const memberFieldsField: SyntheticField = { const createPath = memberFieldsCreatePath(id); assertNotPeople(createPath); - // `referenceName` is the local key, not a managed property: it is CT's own stable, non-numeric - // handle within the group, and sending it at create is what lets every later run find this - // field by its portable identity instead of by a host-specific id. + // Exact API identity is configured separately from the local ct-cli/state key (#158). const created = await client.request("POST", createPath, { ...props, - referenceName: local, + referenceName, }); const newId = memberFieldId(created); if (newId === undefined) { diff --git a/src/engine/types.ts b/src/engine/types.ts index 9d2361e..6aadefa 100644 --- a/src/engine/types.ts +++ b/src/engine/types.ts @@ -31,6 +31,7 @@ export interface DesiredResource { * and `dynamic`: `undefined` = member fields are not managed for this group; `[]` = managed with * none declared (which still never deletes an existing one — see engine/synthetic.ts). Only valid * on a group, because a member field belongs to exactly one group and is not globally reusable. + * Each spec keeps its local ct-cli `key` separate from the exact ChurchTools `referenceName`. */ memberFields?: MemberFieldSpec[]; /** Logical keys this resource must be applied after (includes `parent`/`parents`). */ diff --git a/src/resolve/resolver.ts b/src/resolve/resolver.ts index b77c95d..79769bf 100644 --- a/src/resolve/resolver.ts +++ b/src/resolve/resolver.ts @@ -216,11 +216,11 @@ export class Resolver { */ private readonly declaredRoleDefTypes = new Map(); /** - * Group key → the slugged LOCAL member-field keys that group declares (#135). A member field is - * group-scoped, so "does this config declare it?" can only be answered per group — the same local - * key on two groups is two independent fields. Drives the pending decision below. + * Group key → slugged LOCAL member-field key → exact ChurchTools referenceName (#135/#158). A + * member field is group-scoped, so "does this config declare it?" can only be answered per group; + * the value keeps the normalised ct-cli lookup separate from exact API identity. */ - private readonly declaredMemberFields = new Map>(); + private readonly declaredMemberFields = new Map>(); /** Per-group member-field list cache, keyed by group id, fetched at most once per run. */ private readonly memberFieldLists = new Map[]>>(); @@ -246,7 +246,10 @@ export class Resolver { } } if (d.type === "group" && d.memberFields !== undefined) { - this.declaredMemberFields.set(d.key, new Set(d.memberFields.map((f) => slug(f.key)))); + this.declaredMemberFields.set( + d.key, + new Map(d.memberFields.map((f) => [slug(f.key), f.referenceName])), + ); } } } @@ -499,7 +502,8 @@ export class Resolver { * anything is written. */ private async resolveGroupMemberField(r: GroupMemberFieldRef, site: string): Promise { - const declaresField = this.declaredMemberFields.get(r.group)?.has(slug(r.field)) === true; + const declaredReferenceName = this.declaredMemberFields.get(r.group)?.get(slug(r.field)); + const declaresField = declaredReferenceName !== undefined; const managed = this.state.resources[r.group]; if (!managed || managed.type !== "group") { if (declaresField) return pendingRef(r); @@ -515,7 +519,22 @@ export class Resolver { } const rows = await this.memberFieldList(managed.id); const knownId = knownMemberFieldId(this.state, r.group, r.field); - const matches = matchingMemberFieldRows(rows, r.field, knownId); + const desiredReferenceName = declaredReferenceName ?? r.field; + const matches = matchingMemberFieldRows(rows, r.field, desiredReferenceName, knownId); + if (matches.length === 1) { + const liveReference = matches[0]!.referenceName; + if (liveReference !== desiredReferenceName) { + throw new Error( + `Cannot resolve ${refLabel(r)} referenced at ${site} on ${this.host}: ChurchTools field ` + + `#${String(memberFieldRowId(matches[0]!))} is state-bound to ` + + `"${memberFieldIdentity(r.group, r.field)}", but its exact referenceName is ` + + `${liveReference === undefined ? "missing" : JSON.stringify(liveReference)} instead of ` + + `${JSON.stringify(desiredReferenceName)}. ` + + `ct will not rename an identity-bearing field silently. Replace it explicitly with ` + + `\`ct destroy --member-field ${memberFieldIdentity(r.group, r.field)}\`, then re-run plan/apply.`, + ); + } + } if (matches.length > 1) { const list = matches .map((row) => `${JSON.stringify(row.name ?? row.referenceName)} (#${String(memberFieldRowId(row))})`) diff --git a/tests/adopt-group-command.test.ts b/tests/adopt-group-command.test.ts index 951c2e9..cc6735f 100644 --- a/tests/adopt-group-command.test.ts +++ b/tests/adopt-group-command.test.ts @@ -607,10 +607,12 @@ describe("ct adopt group --with-member-fields (#135)", () => { ]); expect(snippet).toContain("memberFields:"); expect(snippet).toContain('key: "wahl"'); + expect(snippet).toContain('referenceName: "wahl"'); expect(snippet).toContain('name: "Wahl"'); expect(snippet).toContain('fieldTypeCode: "text"'); expect(snippet).toContain("requiredInRegistrationForm: true"); expect(snippet).toContain('key: "notiz"'); + expect(snippet).toContain('referenceName: "notiz"'); // The whole portability guarantee: no host-specific field id anywhere in the emitted config. expect(snippet).not.toContain("701"); expect(snippet).not.toContain("702"); diff --git a/tests/member-fields.test.ts b/tests/member-fields.test.ts index 0c5f84a..da4e8c5 100644 --- a/tests/member-fields.test.ts +++ b/tests/member-fields.test.ts @@ -11,6 +11,7 @@ import { isGroupScopedMemberField, actualMemberFieldProps, groupScopedRows, + matchingMemberFieldRows, } from "../src/engine/member-fields.js"; const HOST = "https://mychurch.church.tools"; @@ -225,7 +226,9 @@ describe("group member fields — plan (#135)", () => { const { plan } = await buildPlan(ct.client, state, [praktikum("praktikum_1", "Praktikum 1")]); const rendered = plain(renderPlan(plan)); expect(rendered).toContain("+ group.praktikum_1"); - expect(rendered).toContain('memberField:wahl: {"name":"Wahl","fieldTypeCode":"text"}'); + expect(rendered).toContain( + 'memberField:wahl: {"referenceName":"wahl","name":"Wahl","fieldTypeCode":"text"}', + ); expect(rendered).toContain("Plan: 1 to create, 0 to update, 0 to delete."); }); @@ -253,7 +256,7 @@ describe("group member fields — plan (#135)", () => { const rendered = plain(renderPlan(plan)); expect(rendered).toContain("~ group.praktikum_1 (#100)"); expect(rendered).toContain( - 'memberField:wahl: {"name":"Wahl","fieldTypeCode":"text"} -> {"name":"Wahl (neu)","fieldTypeCode":"text"}', + 'memberField:wahl: {"referenceName":"wahl","name":"Wahl","fieldTypeCode":"text"} -> {"referenceName":"wahl","name":"Wahl (neu)","fieldTypeCode":"text"}', ); expect(rendered).toContain("Plan: 0 to create, 1 to update, 0 to delete."); }); @@ -435,7 +438,9 @@ describe("group member fields — apply (#135)", () => { it("normalises live response variants and PATCHes the current state-bound id", async () => { const ct = makeCt(); - ct.memberFields[100] = [{ id: 501, type: "group", name: "Birkman", fieldTypeCode: "textarea" }]; + ct.memberFields[100] = [ + { id: 501, type: "group", referenceName: "birkmann", name: "Birkman", fieldTypeCode: "textarea" }, + ]; const realGet = ct.get.getMockImplementation()!; ct.get.mockImplementation(async (path: string) => { if (path === "/groups/100/memberfields") { @@ -524,6 +529,174 @@ describe("group member fields — apply (#135)", () => { const stored = ct.rulesets[gid] as { query: { "==": unknown[] } }; expect(stored.query["=="][1]).toBe(state.resources.praktikum_1!.memberFields!.wahl); }); + + it("keeps local key, exact API referenceName, and ruleset assignment separate (#158)", async () => { + const ct = makeCt(); + const state = stateWith({}); + const { ct: dsl, resources } = createContext(); + dsl.group({ + key: "praktikum_1", + name: "Praktikum 1", + groupTypeId: 5, + groupStatusId: 1, + memberFields: [ + { + key: "stand_bewerbung", + referenceName: "stand-bewerbung", + name: "Stand", + fieldTypeCode: "select", + }, + ], + dynamic: { + status: "active", + ruleset: { + description: "Stand Bewerbung", + query: {}, + process: { + queryResultOnly: { + none: { + handleMembership: { + groupMemberFields: { "stand-bewerbung": "❓Offen" }, + }, + }, + }, + }, + }, + }, + }); + + const { plan } = await buildPlan(ct.client, state, resources); + expect( + plan.items[0]!.changes.find((change) => change.field === "memberField:stand_bewerbung")?.to, + ).toMatchObject({ referenceName: "stand-bewerbung" }); + await executePlan(plan, { client: ct.client, state, statePath: "s.json", save: async () => {} }); + + const groupId = state.resources.praktikum_1!.id; + expect(ct.memberFields[groupId]).toEqual([ + expect.objectContaining({ referenceName: "stand-bewerbung", name: "Stand" }), + ]); + expect(state.resources.praktikum_1!.memberFields).toEqual({ stand_bewerbung: expect.any(Number) }); + expect(ct.rulesets[groupId]).toMatchObject({ + process: { + queryResultOnly: { + none: { + handleMembership: { + groupMemberFields: { "stand-bewerbung": "❓Offen" }, + }, + }, + }, + }, + }); + }); +}); + +describe("group member fields — exact referenceName identity (#158)", () => { + it("marks an existing punctuation mismatch incomplete and never plans a silent rename", async () => { + const ct = makeCt(); + ct.memberFields[100] = [ + { + id: 501, + type: "group", + referenceName: "stand_bewerbung", + name: "Stand", + fieldTypeCode: "select", + }, + ]; + const state = stateWith({ + praktikum_1: { + type: "group", + id: 100, + fields: { name: "Praktikum 1", groupTypeId: 5, groupStatusId: 1 }, + }, + }); + state.resources.praktikum_1!.memberFields = { stand_bewerbung: 501 }; + const { ct: dsl, resources } = createContext(); + dsl.group({ + key: "praktikum_1", + name: "Praktikum 1", + groupTypeId: 5, + groupStatusId: 1, + memberFields: [ + { + key: "stand_bewerbung", + referenceName: "stand-bewerbung", + name: "Stand", + fieldTypeCode: "select", + }, + ], + }); + + const { plan, fetchErrors } = await buildPlan(ct.client, state, resources); + expect(fetchErrors.join("\n")).toMatch( + /stand_bewerbung.*"stand_bewerbung".*"stand-bewerbung".*not rename.*ct destroy --member-field praktikum_1::stand_bewerbung/s, + ); + expect(warnings.join("\n")).toContain('exact ChurchTools referenceName is "stand_bewerbung"'); + expect(plan.items[0]!.changes.some((change) => change.field.startsWith("memberField:"))).toBe(false); + }); + + it("refuses a duplicate create for an unbound row whose normalised spelling merely looks equal", async () => { + const ct = makeCt(); + ct.memberFields[100] = [{ id: 501, type: "group", referenceName: "stand_bewerbung", name: "Stand" }]; + const state = stateWith({ + praktikum_1: { + type: "group", + id: 100, + fields: { name: "Praktikum 1", groupTypeId: 5, groupStatusId: 1 }, + }, + }); + const { ct: dsl, resources } = createContext(); + dsl.group({ + key: "praktikum_1", + name: "Praktikum 1", + groupTypeId: 5, + groupStatusId: 1, + memberFields: [ + { + key: "stand_bewerbung", + referenceName: "stand-bewerbung", + name: "Stand", + }, + ], + }); + + const { plan, fetchErrors } = await buildPlan(ct.client, state, resources); + expect(fetchErrors.join("\n")).toMatch(/"stand_bewerbung".*"stand-bewerbung"/s); + expect(plan.items[0]!.changes.some((change) => change.field.startsWith("memberField:"))).toBe(false); + }); + + it("does not collapse hyphens and underscores when matching API reference names", () => { + const rows = [ + { id: 1, referenceName: "foo-bar", name: "Foo" }, + { id: 2, referenceName: "foo_bar", name: "Foo" }, + ]; + expect(matchingMemberFieldRows(rows, "foo_bar", "foo-bar")).toEqual([rows[0]]); + expect(matchingMemberFieldRows(rows, "foo-bar", "foo_bar")).toEqual([rows[1]]); + }); + + it("rejects duplicate exact API identities but permits hyphen/underscore as distinct", () => { + const { ct } = createContext(); + expect(() => + ct.group({ + key: "duplicate", + name: "Duplicate", + memberFields: [ + { key: "first", referenceName: "same", name: "First" }, + { key: "second", referenceName: "same", name: "Second" }, + ], + }), + ).toThrow(/duplicate member field referenceName "same"/); + + expect(() => + ct.group({ + key: "distinct", + name: "Distinct", + memberFields: [ + { key: "first", referenceName: "foo-bar", name: "First" }, + { key: "second", referenceName: "foo_bar", name: "Second" }, + ], + }), + ).not.toThrow(); + }); }); describe("group member fields — one spelling of identity (#135 review)", () => {