feat: manage group member fields as group-scoped resources (#135) - #150
Merged
Merged
Conversation
A group member field has a ChurchTools id but belongs to exactly one group and
is not globally reusable, so it is modelled as an owned sub-resource of the
group rather than a registry type: declared inside `ct.group({ memberFields:
[...] })`, folded per field into `memberField:<localKey>` pseudo-fields, and
applied inline with its group.
Identity is the managed group key plus the local field key
(`ojbp_2026_27_praktikum_1::wahl`). No ChurchTools field id ever reaches
authored config or an adopted blueprint — declaring one is a config error.
- adopt: `ct adopt group <id|--children-of> --with-member-fields` (opt-in only)
- plan: per-field creates and updates; the actual side is narrowed to the
declared properties so a clean apply re-plans as a no-op
- apply: creates fields after the owning group exists, updates in place by
identity (PATCH, PUT fallback on 405/501)
- delete: never implicit. A dropped field produces no desired diff key at all;
it is reported as a DELETE CANDIDATE and removed only by the explicit
`ct destroy --member-field <group>::<field>`, which inherits the owning
group's backup, typed confirmation and preventDestroy guardrails
- rulesets: `ref.groupMemberField(group, field)` resolves per host; a reference
to a field the target group does not declare fails at config-eval time, and
one into an adopted group is hard-errored at plan time — both before apply
- ordering: synthetic fields are applied in registration order (parents →
memberField:* → dynamic), and synthetic pending refs are resolved immediately
before each write, so a ruleset can name a field created in the same run
Also fixes a related fold gate: a config whose only dynamic group was created by
that same run never folded its desired `dynamic` block, so its ruleset silently
did not apply on the first run.
Closes #135.
Claude-Session: https://claude.ai/code/session_018JShVZYNLaRb4hF5KbHCXG
… and use one spelling of identity Review findings on #135: - `ct destroy --member-field g::f --target g` deleted the group even when the field's DELETE had just failed — taking the field with it, right after the run printed "Nothing further was deleted". The loop now reports success and the caller holds back the group deletes. - The `memberFields` state map was written under the raw declaration key and read under the raw ref key, while every live-row comparison slugs. A `wahl` declaration plus a `ref.groupMemberField(g, "Wahl")` therefore hard-failed mid-apply, after the group and field had been created. `memberFieldStateKey` is now the one canonical spelling — writer, reader, `ct destroy`'s state cleanup, the declaration-uniqueness check and the ruleset-ref check all use it. - `isGroupScopedMemberField` returned on the FIRST string among type/fieldCategory/source/fieldSource, so a row carrying a field TYPE in `type` ("text") would exclude every row: adopt emits nothing and apply POSTs a duplicate field on every run. Rows are now classified by value — "group" includes, a known non-group source excludes, anything unrecognised includes. - A group's N member-field changes issued N identical `GET /groups/{id}/memberfields`. They share one per-item read cache now. - `--with-member-fields` swallowed only a 404, so a 403 or a 429 on one group aborted a `--children-of` adoption partway, with earlier groups already in state. Every read failure is now warned about and the group adopted without its fields, as the comment always claimed. Also states in the flag's help text (and the handbook) that the opt-in is transitional, per docs/adoption-contract.md (#141). Claude-Session: https://claude.ai/code/session_018JShVZYNLaRb4hF5KbHCXG
The pinned-plan assertions matched only locally: picocolors enables colour when the ambient CI env var is set, so every runner saw escape codes in `renderPlan` output. Strip them at the assertion — colour is render.test.ts's subject. Handbook pages re-read against the sources this branch changed; the member-field and dynamic-group pages gained the destroy gate, the normalised-key rule and the tolerant `--with-member-fields` read. Claude-Session: https://claude.ai/code/session_018JShVZYNLaRb4hF5KbHCXG
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Group member fields — the field definitions a group asks its members for — are now a managed, portable, group-scoped resource.
DSL shape, and why
Nested on the owning group, as an opt-in synthetic field alongside
parentsanddynamic:The issue left
memberFields: [...]vs.ct.groupMemberField({ group, key, ... })open. Nested won for three concrete reasons, all of them about matching the existing resource model rather than aesthetics:AdoptableResourceis built around a fixedcollectionPathand anitemPath(id). A member field's every path needs the group id (/groups/{groupId}/memberfields/group/{fieldId}), which no registry hook is given —fetchOne(client, id)andwriter.remove({client, id})included. Making it a registry type would have meant widening those signatures for a single caller.group-member-fieldtype would have to be tier ≥ 2 (it needs the group), which puts field creation strictly after ruleset install — the opposite of what the issue asks for. Nested, the fields and the ruleset are changes on the same item and the registration order inSYNTHETIC_FIELDS(parents→memberField:*→dynamic) fixes it structurally.Each declared field folds into its own pseudo-field,
memberField:<localKey>, rather than onememberFieldsarray blob, soplandiffs and renders per field.Identity scheme
<managed group key>::<local field key>— e.g.ojbp_2026_27_praktikum_1::wahl.Two groups declaring
wahlstay two independent resources with two different ChurchTools ids, and there is no state-key collision: the field has no top-level state entry at all, only amemberFields: { wahl: 501 }id map on the owning group's entry (never diffed, never written to CT).No ChurchTools field id ever reaches authored config or an adopted blueprint. Declaring
id/groupMemberFieldId/groupIdon a member field is a hard config error, not a warning. The local key is carried on the CT side asreferenceName(sent at create, matched slug-insensitively on every later run, with the sluggednameas a fallback for a field made in the ChurchTools UI).Delete guardrail
applycannot delete a member field, and not by a check — by construction.diffFieldswalks the desired side, so a field dropped frommemberFieldsproduces no key, no change and no write. There is no code path that could propose it.It is still surfaced rather than silently ignored: every plan warns per undeclared live field, naming its portable identity and the exact command that removes it. That mirrors the existing delete-candidate concept (
applynever deletes;destroydoes), adapted to a resource that has no state key of its own:That path inherits the group's guardrails — pre-delete backup of the field definition into the same backup file, typed confirmation, protected-env confirmation, and the owning group's
preventDestroy(protecting a group protects what it owns). Member fields are deleted before any--targetresources, so a group delete cannot swallow them first.Apply ordering, and ruleset references
ref.groupMemberField(group, field)is a new compoundRefkind, so a ruleset names a field portably instead of freezing this host's numeric id into a file that is applied elsewhere.evaluateConfigthrows, naming what the group does declare.GET /groups/{id}/memberfieldsand hard-errors there — still before apply writes.PendingRef. Ordering makes that safe: synthetic fields are applied in registration order, andapplySyntheticFieldsnow re-resolves each change immediately before applying it (rather than all of them up front), sodynamicsees the id thememberField:*create just recorded. Cross-group references add a dependency edge onto the owning group viacollectPendingRefKeys.Adoption default is deferred to #141
--with-member-fieldsis explicit opt-in only, on bothct adopt group <id>andct adopt group --children-of <id>, exactly as #135 specifies. Whether owned child resources are adopted by default is a project-wide contract and is deliberately left to #141 — this PR does not decide it.Properties excluded from management
id— host-specific; the portability guarantee.referenceName— this is the local identity, not a diffable property. Managing it would let a rename silently re-key the resource, so the next apply would re-create the field instead of updating it.Everything else #135 lists is managed:
name,fieldTypeCode,defaultValue,options,nameInSignupForm,note,noteInSignupForm,requiredInRegistrationForm,useInRegistrationForm,securityLevel,sortKey. A property outside that set still passes through to CT untouched (the usual escape hatch) and only warns.The actual side of each diff is narrowed to exactly the properties the declaration names — one level deeper than
diffFieldsdoes at the resource level, same meaning. Without that, any server default CT echoes back would make the two sides unequal forever and re-propose the same update on every run.Incidental fix
A config whose only dynamic group was created by that same run never folded its desired
dynamicblock (the fold returned early when nothing was readable), so its ruleset silently did not apply on the first run — while the same config next to an already-adopted dynamic group applied it fine. The gate is now on the desired opt-in, matchingparents.Tests
tests/member-fields.test.ts(16), plus additions totests/adopt-group-command.test.tsandtests/destroy-command.test.ts. All fixtures/mocks — no live API calls. Covering:No changes.even after CT adds unmanaged siblingsfetch-failed/INCOMPLETE, never fabricated creates<group>::<field>ref resolves to the just-minted idct destroy --member-fielddeletes through the group-scoped path, is blocked by the group'spreventDestroy, and rejects malformed/unmanaged targets before any network callnpm test && npm run typecheck && npm run lint && npm run buildall pass;docs-staleness.mjsis clean (blueprints/dynamic-groups/permissions re-read and re-signed, newdocs/handbuch/group-member-fields.mdpage added to the nav,docs/api-coverage.mdrow added).Closes #135.
https://claude.ai/code/session_018JShVZYNLaRb4hF5KbHCXG