diff --git a/README.md b/README.md index 5a8e511..62833af 100644 --- a/README.md +++ b/README.md @@ -256,10 +256,11 @@ confirmation — and a declaration marked `preventDestroy: true` blocks even tha | Auto-groups (dynamic groups) | the `dynamic` block on a group | [dynamic groups](docs/handbuch/dynamic-groups.md) | | Group member field definitions (group-scoped, never deleted implicitly) | the `memberFields` block on a group | [group member fields](docs/handbuch/group-member-fields.md) | | Repeated structure, parametrized | a plain function over the DSL | [blueprints](docs/handbuch/blueprints.md) | +| Permission scope master data (Bereiche, security levels, comment viewers) | `ct.department`, `ct.securityLevel`, `ct.commentViewer` | [permissions](docs/handbuch/permissions.md) | -Read-only by design: the person master-data model, security levels and -custom-field _definitions_ (`ct get person-masterdata`, `ct get data-fields`) — -schema in scope, per-record values never. See +Read-only by design: the person master-data model and custom-field +_definitions_ (`ct get person-masterdata`, `ct get data-fields`) — schema in +scope, per-record values never. See [field definitions](docs/handbuch/field-definitions.md). References are logical throughout: `groupType: "ministry_team"` resolves to that diff --git a/docs/api-coverage.md b/docs/api-coverage.md index bdbde1d..d85a98c 100644 --- a/docs/api-coverage.md +++ b/docs/api-coverage.md @@ -88,11 +88,12 @@ API-client libraries (5pm-HDH `churchtools-api` @ CT 3.104, bensteUEM `src/api/schema.d.ts` is git-ignored and was not available offline. Re-verify per the runbook's re-audit procedure once the schema is regenerated. -| Resource | Matched path(s) | GET | POST/PUT/PATCH/DELETE | Verdict | -| ------------------------------------ | ----------------------------------------- | ------------------ | ---------------------- | --------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------- | -| Person master-data model | `/person/masterdata` | ✅ (single object) | ❌ | **Read-only.** Versionable master-data model incl. the `securityLevels` enumeration. No write endpoint (edited in the CT master-data admin UI). | -| Data-field definitions (Datenfelder) | `/dbfields`, `/dbfields/{id}` | list ✅ / by-id ✅ | ❌ | **Read-only.** Unified person + group field definitions, discriminated by `fieldCategory`. Mutation only via legacy churchdb AJAX (`db_insert/update/deletefields`), not REST. | -| Security levels | `/securitylevels`, `/securitylevels/{id}` | list ✅ / by-id ✅ | ✅ POST, PATCH, DELETE | **Full CRUD — but not on the usual paths.** Live-probed 2026-08-14 (eqrm-dev, CT 3.135.2): `/securitylevels` is GET-only, and **create is `POST /securitylevels/{id}`** (body `{name}`), update is `PATCH /securitylevels/{id}` (body `{name, newid, forcereorder}` — reordering is first-class), delete is `DELETE /securitylevels/{id}`. **Managed since #110** — the registry gained a `createPath` hook and a `callerAssignedId` flag for it; `ct.securityLevel({ key, id, name })` declares one, `ct adopt security-level ` adopts one. Changing a declared id is refused at plan time (that is a renumber, which rewrites what every numeric `cc_securitylevel` scope means). Resolvable by name as a scope ref since #110. The ids are NOT protocol constants — `cc_securitylevel` is an editable master-data table with an auto-increment id. | +| Resource | Matched path(s) | GET | POST/PUT/PATCH/DELETE | Verdict | +| ------------------------------------ | ------------------------------------------------------- | ------------------ | ---------------------- | ------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------ | +| Person master-data model | `/person/masterdata` | ✅ (single object) | ❌ | **Read-only.** Versionable master-data model incl. the `securityLevels` enumeration. No write endpoint (edited in the CT master-data admin UI). | +| Data-field definitions (Datenfelder) | `/dbfields`, `/dbfields/{id}` | list ✅ / by-id ✅ | ❌ | **Read-only.** Unified person + group field definitions, discriminated by `fieldCategory`. Mutation only via legacy churchdb AJAX (`db_insert/update/deletefields`), not REST. | +| Security levels | `/securitylevels`, `/securitylevels/{id}` | list ✅ / by-id ✅ | ✅ POST, PATCH, DELETE | **Full CRUD — but not on the usual paths.** Live-probed 2026-08-14 (eqrm-dev, CT 3.135.2): `/securitylevels` is GET-only, and **create is `POST /securitylevels/{id}`** (body `{name}`), update is `PATCH /securitylevels/{id}` (body `{name, newid, forcereorder}` — reordering is first-class), delete is `DELETE /securitylevels/{id}`. **Managed since #110** — the registry gained a `createPath` hook and a `callerAssignedId` flag for it; `ct.securityLevel({ key, id, name })` declares one, `ct adopt security-level ` adopts one. Changing a declared id is refused at plan time (that is a renumber, which rewrites what every numeric `cc_securitylevel` scope means). Resolvable by name as a scope ref since #110. The ids are NOT protocol constants — `cc_securitylevel` is an editable master-data table with an auto-increment id. | +| Comment viewers (Kommentare-Viewer) | `/person/commentviewers`, `/person/commentviewers/{id}` | list ✅ / by-id ✅ | ✅ POST, PUT, DELETE | **Full CRUD on conventional paths.** Fully live-probed 2026-08-26 (eqrm-dev, CT 3.135.2 — one throwaway row created, read, updated and deleted): `GET /person/commentviewers` returns a flat `[{id, name, nameTranslated, sortKey}]` (`id: 0` — "Alle" — is a real, built-in row), **create is a plain `POST /person/commentviewers`** with CT minting the id, and the item path takes GET/PUT/DELETE (an absent id gives a clean 404 `error.notfound`). `nameTranslated` is derived from `name`, not independently writable. **Managed since #151** — `ct.commentViewer({ key, name, sortKey })` declares one, `ct adopt comment-viewer ` adopts one; no registry machinery was needed (CT mints the id, and the writes are REST). `cdb_comment_viewer` is the scope dimension of `churchdb:view comments`, and it was the last one a config could not express portably: the ids differ across hosts of the same deployment, and before #151 a config had only the raw numeric `dataId`. The minted id is an auto-increment that does not reuse deleted rows, so it can never be caller-assigned. | See [`docs/handbuch/field-definitions.md`](handbuch/field-definitions.md) for the full writability decision, evidence, and the schema/values boundary. diff --git a/docs/handbuch/blueprints.md b/docs/handbuch/blueprints.md index d0447a8..867536c 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: ed5171960e9206fa -reviewed: 2026-08-17 +sources_hash: bb6f89956ace833d +reviewed: 2026-08-26 --- # Blueprints (parametrized, reusable config) diff --git a/docs/handbuch/group-member-fields.md b/docs/handbuch/group-member-fields.md index 3159af3..0bd29f7 100644 --- a/docs/handbuch/group-member-fields.md +++ b/docs/handbuch/group-member-fields.md @@ -1,5 +1,5 @@ --- -sources_hash: 0dd7d04768dda2d2 +sources_hash: 777badbcc7fbbd7f title: Group member fields sources: - src/engine/member-fields.ts @@ -7,7 +7,7 @@ sources: - src/config/context.ts - src/commands/adopt-group.ts - src/commands/destroy.ts -reviewed: 2026-08-24 +reviewed: 2026-08-26 --- # Group member fields (#135) diff --git a/docs/handbuch/permissions.md b/docs/handbuch/permissions.md index 9bb49d2..2bda944 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: eaea3f87de51a46f -reviewed: 2026-08-25 +sources_hash: becf78e8e94dbfed +reviewed: 2026-08-26 --- # Permissions (`ct.groupRole` / `ct.groupTypeRole` / `ct.status`) @@ -463,14 +463,14 @@ ct.groupRole({ `{ campus: "koblenz" }` is sugar for `ref.campus("koblenz")` — the same `Ref` the rest of the DSL uses — so both spellings are interchangeable. -| `scopeField` | Reference form | Resolved against | -| -------------------- | ----------------------------------------- | ---------------------------------------------------------- | -| `cdb_gruppe` | `{ group: "" }` (or the bare string) | managed groups | -| `cdb_station` | `{ campus: "" }` | managed campuses, then `GET /campuses` | -| `cdb_gruppentyp` | `{ groupType: "" }` | managed group types, then `GET /group/grouptypes` | -| `cdb_bereich` | `{ department: "" }` | managed Bereiche, then `GET /departments` (#108) | -| `cc_securitylevel` | `{ securityLevel: "" }` | managed security levels, then `GET /securitylevels` (#110) | -| `cdb_comment_viewer` | `{ commentViewer: "" }` | `GET /person/commentviewers` **only** (#102) | +| `scopeField` | Reference form | Resolved against | +| -------------------- | ----------------------------------------- | ----------------------------------------------------------------- | +| `cdb_gruppe` | `{ group: "" }` (or the bare string) | managed groups | +| `cdb_station` | `{ campus: "" }` | managed campuses, then `GET /campuses` | +| `cdb_gruppentyp` | `{ groupType: "" }` | managed group types, then `GET /group/grouptypes` | +| `cdb_bereich` | `{ department: "" }` | managed Bereiche, then `GET /departments` (#108) | +| `cc_securitylevel` | `{ securityLevel: "" }` | managed security levels, then `GET /securitylevels` (#110) | +| `cdb_comment_viewer` | `{ commentViewer: "" }` | managed comment viewers, then `GET /person/commentviewers` (#151) | Two `Ref` kinds are deliberately **not** in that table because no permission dimension scopes by them: `group-type-role` (a `groupTypeRoleId`, addressed by @@ -508,23 +508,63 @@ Three things are hard errors at **plan** time, never a guessed `dataId`: group", so on e.g. a `cdb_station` right it would either fail confusingly or — worse — match an unrelated group that happens to carry that key. -### Catalog dimensions: referenceable, not declarable +### Comment viewers: from catalog-only to declarable (#151) -`cdb_comment_viewer` (Kommentare-Viewer) is the one remaining dimension `ct` -**reads but does not manage**, and not because ChurchTools cannot do it: +`cdb_comment_viewer` (Kommentare-Viewer) had a reference form since #102 but no +declaration behind it, and that combination turned out to be the worst of both +worlds. Every other option was worse still: -- **Comment viewers have plain REST CRUD** — `/person/commentviewers` plus - `POST`/`PUT`/`DELETE` on the item path, which would fit the resource registry - unchanged. They are catalog-only purely because nothing has needed to declare - one yet ([#102](https://github.com/eqrm/ct-cli/issues/102)). +- a **raw `dataId`** is host-specific. Read on two hosts of the same deployment + on 2026-08-24, three of the six viewer ids on one host did not exist on the + other at all, and the two ids present on both named _different_ categories on + each. An apply duly wrote a grant pointing at nothing, and nothing caught it — + `ct plan` compares the declared id against the live id, and they match; the id + is simply meaningless on the target host. +- a **name reference** did not rescue it, because the second host was missing the + NAMES as well. The reference hard-errored instead of resolving wrongly — safer, + but still not a config that applies to both hosts. +- **creating the viewers by hand** on each host is unmanaged master data, which + is the thing this tool exists not to need. -So the reference resolves by name on every host — which is what makes -`churchdb:view comments` declarable at all — but there is no `ct.commentViewer` -resource, no `ct adopt` for it, and a name that matches nothing is a **hard -error** rather than a create: +So comment viewers are a managed resource now: -(Security levels left this list in #110 and Bereiche in #108. Both are managed -resources now — see below.) +```ts +ct.commentViewer({ key: "dienstbereich", name: "Dienstbereich", sortKey: 40 }); +... +{ right: "churchdb:view comments", scope: [{ commentViewer: "dienstbereich" }] } +``` + +Declaring the viewer is what makes the NAME exist on both hosts, which is what +makes the reference portable — a name reference is only worth anything once +something guarantees the row is there. + +- **`ct adopt comment-viewer `** puts an existing viewer under management and + emits the declaration; `ct get comment-viewers` lists the names. +- **`/person/commentviewers` is conventional REST**, so this type needs none of + the special machinery the other two awkward master-data types did: ChurchTools + mints the id (unlike a security level) and the writes are REST (unlike a + Bereich). Fully live-probed on eqrm-dev, CT 3.135.2 (2026-08-26) — one + throwaway row created, read, updated and deleted, instance left as found: + + | Call | Result | + | ------------------------------------ | ------------------------------------------------- | + | `GET /person/commentviewers` | flat `[{id, name, nameTranslated, sortKey}]` | + | `POST /person/commentviewers` | 200 — **CT mints the id**; body `{name, sortKey}` | + | `GET /person/commentviewers/{id}` | 200; an absent id → clean 404 `error.notfound` | + | `PUT /person/commentviewers/{id}` | 200 | + | `DELETE /person/commentviewers/{id}` | 200 | + + The minted id is an auto-increment that does **not** reuse deleted rows and + moves faster than the visible row count (three consecutive probe creates on a + three-row instance minted 5, 8 and 11), which is why a viewer can never carry a + declared id the way a security level does — nothing can predict it. + `nameTranslated` is derived from `name`, not independently writable, so the + managed set (`name`, `sortKey`) is complete and a `PUT` cannot blank a sibling. + +- **Resolution is managed-state first, live catalog second.** An existing + reference to a viewer your config does not own keeps resolving exactly as it + did in #102, and a name that matches nothing anywhere is still a hard error + rather than a create: ``` Cannot resolve comment-viewer:nope referenced at … : no managed resource and @@ -532,11 +572,31 @@ no live comment-viewer at /person/commentviewers matches key "nope". Declare/adopt it, fix the key/name, or use a numeric id. ``` -Discover the names with `ct get comment-viewers`. Because -the id always comes from the live catalog, such a scope is never "pending" and is -never re-resolved at apply time — it is host-correct the moment it resolves. A -managed resource that happens to share the key does **not** shadow it (that would -be exactly the misgrant this feature exists to prevent). +- **`ct apply` never deletes**, as everywhere else. `ct destroy` can, and warns: + deleting a viewer reaches every person comment restricted to it and every grant + scoped to it. +- **`dataId: 0` — the built-in "Alle" viewer — is the one id you should _not_ + adopt.** ChurchTools ships it on every instance, so the number already means the + same thing everywhere, and it is the likely `dataId` on a real + `churchdb:view comments` grant. Adopting it would be actively harmful rather + than merely pointless: the emitted + `ct.commentViewer({ key: "alle", name: "Alle" })` finds no state entry on a + second host, so `ct apply` creates a **second** "Alle" with a fresh id and + scopes the grant to the duplicate. Worse, that host never complains: + resolution is managed-state-first, so `{ commentViewer: "alle" }` quietly + resolves to the config's own duplicate. The two identically named rows only + hard-error for a config reading the catalog _without_ a state entry — a third + host, or the same one after a state reset — so the damage surfaces somewhere + other than where it was caused. The adopter therefore + treats it like the `-1` sentinel — emitted as the bare number with a comment + saying what it is, never with an adoption hint. It stays a **number** rather + than a name reference on purpose: an admin can rename the row, and the id is + what does not move. + +With this, every scope dimension `ct` can name is also one it can declare — +Bereiche left the catalog-only list in #108, security levels in #110, and comment +viewers here. What remains numeric-only (`ccm_data_category`, `oauth_client`, +`cc_calcategory`, …) belongs to modules outside this tool's mandate. ### Bereiche: managed, but written through a non-REST endpoint (#108) @@ -631,11 +691,11 @@ For these, a `dataId` like `1`, `2`, `3` names a security level or a comment-viewer bucket — **not** a group — so `GET /groups/{1,2,3}` 404s. A `scope` array entry may therefore be a plain number instead of a string: -(Security levels gained a name-based form in #110 — see -[Catalog dimensions](#catalog-dimensions-referenceable-not-declarable). -Comment viewers gained one too, in #102. Numerics keep working for both; -calendar categories, OAuth clients and the other module dimensions still have -nothing but numerics.) +(Both of those dimensions gained a name-based form and are declarable resources +now — comment viewers in [#151](#comment-viewers-from-catalog-only-to-declarable-151), +security levels in [#110](#security-levels-a-managed-resource-with-a-declared-id-110). +Numerics keep working for both; calendar categories, OAuth clients and the other +module dimensions still have nothing but numerics.) ```ts { right: "churchdb:security level view own data", scope: [1, 2, 3, 5] }, @@ -739,11 +799,13 @@ guaranteed to be accepted by `ct plan` (the round trip is locked by tests): clearly-marked placeholder comment telling you to `ct adopt group ` first — scope keys must be state keys (see [Scope resolution](#scope-resolution)). - - **A catalog dimension `ct` reads but does not manage** (`cdb_bereich`, - `cc_securitylevel`): the emitter is pure — no client, no fetch — so it cannot - turn the id into a name. It emits the number plus a `NOTE` giving the portable - form (`{ department: "" }` / `{ securityLevel: "" }`) to write by - hand. + - **Another dimension with a logical form** (`cdb_station`, `cdb_gruppentyp`, + `cdb_bereich`, `cc_securitylevel`, `cdb_comment_viewer`): a `dataId` that + matches a resource **managed in your state file** is emitted as the portable + reference (`scope: [{ commentViewer: "dienstbereich" }]`). An unmanaged one + keeps its number and earns a `NOTE` naming the one command that fixes it — + `ct adopt comment-viewer `, then re-adopt the grants. The emitter is pure + (no client, no fetch), so it cannot turn an unmanaged id into a name itself. - **Any other scope dimension** (`cc_calcategory`, `oauth_client`, …): there is no group to adopt, so the `dataId`(s) are emitted directly as a numeric `scope: [1, 2, 3]` — always an active line, with a comment @@ -757,6 +819,14 @@ guaranteed to be accepted by `ct plan` (the round trip is locked by tests): host reads it. It is therefore already portable, and it is emitted with a one-line comment saying what it is — never with an adoption hint, because `ct adopt department -1` names a resource that cannot exist. +- **Built-in rows are already portable too** (#151). Same treatment as `-1`, for a + different reason: `cdb_comment_viewer` `0` ("Alle") _is_ a real row, but one every + instance has, so the number means the same thing on every host. It is emitted + verbatim with a comment naming it, never counted as an unmanaged host-specific + id, and never offered for adoption — see + [comment viewers](#comment-viewers-from-catalog-only-to-declarable-151) for what + adopting it would do to the second host. This is keyed **by dimension**: `0` on + any other scope field is an ordinary host-specific id and is still flagged. - **Admin-authored member rights are emitted as active grants** — **including** the writable `authId >= 10000` `churchdb:+…` member rights CT lets you set on `group_type_role`. There is no authId cutoff (#65). diff --git a/src/config/context.ts b/src/config/context.ts index 76db652..c5593c0 100644 --- a/src/config/context.ts +++ b/src/config/context.ts @@ -322,6 +322,23 @@ export interface ConfigContext { * `destroyWarning`: a delete reaches every person field and grant referencing the level. */ securityLevel(input: ResourceInput): void; + /** + * A COMMENT VIEWER (`/person/commentviewers` — "Kommentare-Viewer"), #151 — the `cdb_comment_viewer` + * scope dimension of `churchdb:view comments`. + * + * ```ts + * ct.commentViewer({ key: "dienstbereich", name: "Dienstbereich", sortKey: 40 }); + * ... + * { right: "churchdb:view comments", scope: [{ commentViewer: "dienstbereich" }] } + * ``` + * + * The last scope dimension a config could not express portably: before this, a `view comments` + * grant had to name a raw numeric `dataId`, which means a different viewer — or nothing — on + * another host, and neither `ct plan` nor a row count can see that. Declaring the viewer makes the + * NAME exist on every host, which is what makes `{ commentViewer: "…" }` resolve to the same thing + * everywhere. Master data (the enumeration), never a person or a comment. + */ + commentViewer(input: ResourceInput): void; /** * A BEREICH / department (`/departments`), #108 — the `cdb_bereich` scope dimension of * `churchdb:view alldata` ("Personen eines Bereiches sehen"). @@ -772,6 +789,7 @@ export function createContext(): { relationshipType: define("relationship-type"), personStatus: define("person-status"), securityLevel: define("security-level"), + commentViewer: define("comment-viewer"), department: define("department"), roleDefinition: define("group-role"), groupRole: definePermission("group_role"), diff --git a/src/permissions/adopt.ts b/src/permissions/adopt.ts index 8fa8079..1b78bd6 100644 --- a/src/permissions/adopt.ts +++ b/src/permissions/adopt.ts @@ -35,7 +35,13 @@ import { type GrantTuple, type RawPermission, } from "./grants.js"; -import { ALL_SCOPE_SENTINEL, GROUP_SCOPE_FIELD, SCOPE_REF_KIND } from "./scope.js"; +import { + ALL_SCOPE_SENTINEL, + builtinScopeIdName, + GROUP_SCOPE_FIELD, + isBuiltinScopeId, + SCOPE_REF_KIND, +} from "./scope.js"; /** DSL function name for each domain type — the call the emitted block should be pasted as. */ const DSL_FN: Record = { @@ -288,7 +294,7 @@ function grantLines(g: CollapsedGrant, rev: Map, state: St // — the whole point of #98, since these ids are host-specific (dev Mainz = 6, prod Mainz = 0), // so a numeric literal adopted from prod is a misgrant when replayed on dev. An unmanaged // dataId keeps its number and earns a NOTE pointing at the one command that fixes it. - // b) The dimension has none (`cc_securitylevel`, `cdb_bereich`, `oauth_client`, …) — its dataIds + // b) The dimension has none (`ccm_data_category`, `oauth_client`, …) — its dataIds // name something this tool has no managed representation for, so the numeric escape hatch // (#49) is the only honest output and the grant is always emitted as an ACTIVE line. const out: string[] = []; @@ -305,21 +311,40 @@ function grantLines(g: CollapsedGrant, rev: Map, state: St } if (g.dataIds.length > 0) { const dimension = SCOPE_REF_KIND[entry.scopeField]; - // A catalog-only dimension (departments) has no managed resource to look an id up in, and this - // emitter is deliberately pure — no client, no fetch — so it cannot turn the id into a name. - // Emit the number and point at the portable form the author can write by hand. + // A catalog-only dimension (`managed: false`) has no managed resource to look an id up in, and + // this emitter is deliberately pure — no client, no fetch — so it cannot turn the id into a + // name. Emit the number and point at the portable form the author can write by hand. No + // dimension is catalog-only today (comment viewers, the last one, became managed in #151); + // the branch stays because the flag is what a future read-only dimension would set. const sugar = dimension?.managed ? SCOPE_SUGAR_FIELD[dimension.type] : undefined; // #115: `-1` is not an id at all — it is ChurchTools' "alle" sentinel, meaning every value of // the dimension on whatever host reads it. So it is ALREADY portable, and none of the // host-specific-id advice below applies to it. Emitting `ct adopt department -1` sent people // looking for a resource that cannot exist, and in a real adoption run this fires on most of // the broadly-scoped grants — the interesting ones. - const realIds = g.dataIds.filter((id) => id !== ALL_SCOPE_SENTINEL); - const hasSentinel = g.dataIds.length > realIds.length; + // + // #151 generalised that: a dataId is ALREADY PORTABLE when it means the same thing on every + // host, which covers the `-1` sentinel AND a BUILT-IN row of the dimension (`cdb_comment_viewer` + // 0 = "Alle"). Both are emitted verbatim and neither counts as an unmanaged host-specific id. + // They differ in one respect worth keeping straight: `-1` is not an id at all, while a built-in + // IS a real row — just one every instance has. See `isBuiltinScopeId` for why adopting one is + // actively harmful rather than merely useless. + const portable = (id: number): boolean => + id === ALL_SCOPE_SENTINEL || isBuiltinScopeId(entry.scopeField!, id); + const realIds = g.dataIds.filter((id) => !portable(id)); + const hasSentinel = g.dataIds.includes(ALL_SCOPE_SENTINEL); if (hasSentinel) { const what = dimension ? `every ${dimension.type}` : `every value of "${entry.scopeField}"`; out.push(` // scope -1 is ChurchTools' "alle" sentinel — ${what}; host-independent.`); } + for (const id of g.dataIds) { + const builtin = builtinScopeIdName(entry.scopeField!, id); + if (builtin === null) continue; + out.push( + ` // scope ${id} is the built-in "${builtin}" ${dimension?.type ?? entry.scopeField} — ` + + `present on every instance, so the number is already portable (do not adopt it).`, + ); + } if (dimension && !dimension.managed && realIds.length > 0) { const field = SCOPE_SUGAR_FIELD[dimension.type] ?? dimension.type; out.push( @@ -332,7 +357,7 @@ function grantLines(g: CollapsedGrant, rev: Map, state: St const entries: string[] = []; const unmanaged: number[] = []; for (const id of [...g.dataIds].sort((a, b) => a - b)) { - if (id === ALL_SCOPE_SENTINEL) { + if (portable(id)) { // Portable as-is (see above) — emitted, but never counted as an unmanaged host-specific id. entries.push(String(id)); continue; @@ -361,9 +386,10 @@ function grantLines(g: CollapsedGrant, rev: Map, state: St `and re-adopt to make ${one ? "it" : "them all"} portable.`, ); } else if (!dimension) { - // Reached only for a dimension with NO logical form at all (`cc_securitylevel`, `oauth_client`, - // …). A catalog-only dimension (`cdb_bereich`) already got its own NOTE above — emitting this - // line there too would contradict it ("not a group, use numbers" vs "portable form exists"). + // Reached only for a dimension with NO logical form at all (`ccm_data_category`, + // `oauth_client`, …). A catalog-only dimension would already have got its own NOTE above — + // emitting this line there too would contradict it ("not a group, use numbers" vs "portable + // form exists"). out.push( ` // "${entry.name}" scopes by "${entry.scopeField}", not a group — using its numeric dataId(s) directly.`, ); diff --git a/src/permissions/scope.ts b/src/permissions/scope.ts index e5142ef..d81d9ba 100644 --- a/src/permissions/scope.ts +++ b/src/permissions/scope.ts @@ -37,6 +37,48 @@ export interface ScopeResolution { */ export const ALL_SCOPE_SENTINEL = -1; +/** + * Per-dimension dataIds that are BUILT IN to every ChurchTools instance, and therefore already + * portable — a config may write the number and mean the same thing on every host. + * + * Unlike {@link ALL_SCOPE_SENTINEL} these are REAL rows: `cdb_comment_viewer` `0` is the "Alle" + * comment viewer, which CT ships on every instance (present on both hosts of the same deployment in + * the 2026-08-24 two-host read; re-read on eqrm-dev 2026-08-26). That difference matters — a built-in + * row can be renamed or deleted by an admin, so it is emitted as its NUMBER (exact, and immune to a + * rename) rather than resolved to a name, and it is never counted as an unmanaged host-specific id. + * + * Adopting one would be actively harmful, which is why this table exists: `ct adopt comment-viewer 0` + * yields `ct.commentViewer({ key: "alle", name: "Alle" })`, and replaying THAT config on a second + * host with a fresh state file finds no state entry, POSTs a SECOND "Alle" (CT mints a fresh id) and + * scopes the grant to the duplicate instead of the built-in — the exact host-specific misgrant #151 + * exists to prevent. Note the duplicate does NOT announce itself on the host that created it: + * resolution is managed-state-first (`Resolver.resolve`), so `{ commentViewer: "alle" }` there + * silently resolves to the config's own duplicate and never reaches `resolveFromCatalog`'s ambiguity + * check. The two "Alle" rows only hard-error for a config reading that catalog WITHOUT a state entry + * — so the failure surfaces on a third host, or after a state reset, not where it was caused. + * + * Deliberately keyed BY DIMENSION, not a blanket "`0` is portable" rule: on any other scope field `0` + * is an ordinary host-specific id, and a global rule would silently stop flagging it. + * + * The NAME lives in this same table rather than in a parallel lookup: `isBuiltinScopeId` suppresses + * the adoption hint and `builtinScopeIdName` supplies the comment that explains the suppression, so + * two tables that disagreed would emit a bare `scope: [0]` with nothing saying why it was not flagged + * — precisely the confusion this exists to prevent. + */ +const BUILTIN_SCOPE_IDS: Readonly>>> = { + cdb_comment_viewer: { 0: "Alle" }, +}; + +/** Whether `id` on `scopeField` is a built-in row present on every host — see {@link BUILTIN_SCOPE_IDS}. */ +export function isBuiltinScopeId(scopeField: string, id: number): boolean { + return builtinScopeIdName(scopeField, id) !== null; +} + +/** The human name of a built-in scope id, for the note the adopter emits next to the number. */ +export function builtinScopeIdName(scopeField: string, id: number): string | null { + return BUILTIN_SCOPE_IDS[scopeField]?.[id] ?? null; +} + /** * The catalog `scopeField` naming the GROUP dimension. The only dimension a bare string scope entry * may address — every other one needs a typed ref (see {@link SCOPE_REF_KIND}) or a numeric dataId. @@ -59,16 +101,16 @@ export const GROUP_SCOPE_FIELD = "cdb_gruppe"; * - `cc_securitylevel` → security levels (`/securitylevels`, #110) — e.g. `churchdb:+see persons` * - `cdb_comment_viewer` → comment viewers (`/person/commentviewers`, #102) — `churchdb:view comments` * - * `managed` separates the MANAGED resource kinds — groups, campuses, group types and (since #110) - * security levels — from the read-only catalogs. For a managed kind a scope target can be declared in - * the same config (resolving pending, then re-resolved at apply time) and a state-backed id is worth - * re-resolving. For a catalog kind the reference always resolves by NAME against the live catalog and - * hard-errors when the name is absent — strictly better than a silent numeric misgrant, but never a - * create. Neither remaining catalog kind is unmanaged because "ChurchTools cannot": - * - `cdb_bereich` has no REST write at all; the admin UI writes it through the legacy master-data - * endpoint (#108/#109). - * - `cdb_comment_viewer` has conventional REST CRUD that WOULD fit the registry unchanged (#102) — - * it is catalog-only purely because nothing has needed to declare one yet. + * `managed` separates the MANAGED resource kinds from the read-only catalogs. For a managed kind a + * scope target can be declared in the same config (resolving pending, then re-resolved at apply time) + * and a state-backed id is worth re-resolving. For a catalog kind the reference always resolves by + * NAME against the live catalog and hard-errors when the name is absent — strictly better than a + * silent numeric misgrant, but never a create. + * + * Every dimension listed here is managed today: departments joined in #108 (writes go through the + * legacy master-data endpoint), security levels in #110, and comment viewers in #151 — the last one + * a config could not express portably at all, because a `churchdb:view comments` grant had no choice + * but a raw host-specific `dataId`. * * The remaining scoped dimensions (`ccm_data_category`, `oauth_client`, `cc_calcategory`, …) stay * numeric-only because this tool has no way to address their values by a host-independent name today — @@ -82,7 +124,7 @@ export const SCOPE_REF_KIND: Readonly ({ __ctRef: true, diff --git a/src/resolve/resolver.ts b/src/resolve/resolver.ts index f2e6c20..66e1bb2 100644 --- a/src/resolve/resolver.ts +++ b/src/resolve/resolver.ts @@ -74,6 +74,11 @@ const REF_KIND_TYPE: Partial> = { // ordering is what lets a config declare a level and scope a grant to it in the same run (the ref // resolves to a PendingRef, carried as a pending scope). "security-level": "security-level", + // Comment viewers became a managed resource in #151 — same ordering as security levels above, and + // for the sharper reason: their ids genuinely differ across hosts of the same deployment, so a + // config that declares its viewers must resolve `{ commentViewer: "…" }` against what IT owns + // before falling back to `/person/commentviewers` for a viewer it does not. + "comment-viewer": "comment-viewer", "role-def": "group-role", group: "group", }; diff --git a/src/resources/registry.ts b/src/resources/registry.ts index 9edaba7..7cefb32 100644 --- a/src/resources/registry.ts +++ b/src/resources/registry.ts @@ -514,6 +514,64 @@ export const RESOURCES: Record = { deriveKey: (r) => slug(str(r, "name")), managedFields: (r) => ({ id: r.id, name: r.name }), }), + /** + * COMMENT VIEWERS (#151) — `cdb_comment_viewer`, the scope dimension of `churchdb:view comments` + * ("Kommentare-Viewer"). Managed so a `view comments` grant means the same thing on every host. + * + * It was catalog-only until now, and that is exactly what made the dimension unportable: a config + * granting `churchdb:view comments` had to write a raw, host-specific `dataId`, and the same + * number names a different viewer (or nothing) on another instance. Read on two hosts of the same + * deployment 2026-08-24: three of prod's six ids do not exist on dev at all, and the two that do + * exist on both name different categories on each. Nothing detected the + * mismatch — a plan compares the declared id against the live id, and they match; the id is simply + * meaningless on the target host. Switching to a name ref did not fix it either: dev lacks the + * NAMES too, so the ref failed to resolve rather than resolving wrongly. Only a declarable + * resource makes the names exist on both hosts, which is what makes a name ref portable. + * + * `/person/commentviewers` is conventional REST, so this needs none of the machinery the other two + * awkward master-data types did: CT mints the id (unlike `security-level`) and writes are REST + * (unlike `department`). FULLY live-probed on eqrm-dev, CT 3.135.2 (2026-08-26) — one throwaway + * row created, read, updated and deleted, instance left as found: + * + * GET /person/commentviewers → flat `[{id, name, nameTranslated, sortKey}]` + * POST /person/commentviewers → 200, CT MINTS the id (body `{name, sortKey}`) + * GET /person/commentviewers/{id} → 200; an absent id → clean 404 `error.notfound` + * PUT /person/commentviewers/{id} → 200 + * DELETE /person/commentviewers/{id} → 200 + * + * The minted id is a bare auto-increment that does NOT reuse deleted rows and moves faster than the + * visible row count (three consecutive probe creates on a 3-row instance minted 5, 8, 11), which is + * the concrete reason a comment viewer can never be `callerAssignedId`: nothing can predict it. + * + * `name` and `sortKey` are the whole editable surface. NB the row carries a fourth column the #109 + * list does not mention, `nameTranslated` — but CT DERIVES it from `name` (the probe's PUT sent only + * `{name, sortKey}` and `nameTranslated` followed `name`), so it is not an unmanaged sibling a PUT + * could blank, and the managed set is complete. + */ + "comment-viewer": define({ + collectionPath: "/person/commentviewers", + updateMethod: "PUT", + // Tier 0: grants scope by comment viewer, and permissions apply after every resource tier. + tier: 0, + destroyWarning: + "deleting a comment viewer reaches every person comment restricted to it and every grant " + + "scoped to it (`churchdb:view comments`) — the viewer id is referenced instance-wide. Verify " + + "it is unused first (`ct get comment-viewers`, `ct report permissions`).", + deriveKey: (r) => slug(str(r, "name")), + managedFields: (r) => ({ name: r.name, sortKey: r.sortKey }), + // `sortKey` is managed but not mandatory in a hand-authored declaration; CT's create validator + // for the 3-column master-data tables rejects a missing integer column, so supply a neutral one + // (a declared value still wins — createDefaults merges UNDER the body). + createDefaults: () => ({ sortKey: 0 }), + // No `fetchOne`: `GET /person/commentviewers/{id}` exists and behaves (probed 2026-08-26, see + // above), so the DEFAULT item read is correct here. This type briefly carried a collection- + // filtering hook while that path was unverified — guessing it would have risked #108's failure + // mode, where a 404 reads as "vanished in ChurchTools" and every apply duplicates the viewer. + // The probe retired the guess: a present id returns the row, an absent one returns a clean 404, + // which is exactly the distinction the default read needs. It also drops one full collection + // read per managed viewer per plan/apply/destroy — `fetchActual` fans out concurrently, so the + // hook cost N round-trips against a rate-limited API where the default costs N cheap item reads. + }), "group-role": define({ collectionPath: "/group/roles", updateMethod: "PUT", diff --git a/tests/comment-viewer-resource.test.ts b/tests/comment-viewer-resource.test.ts new file mode 100644 index 0000000..12f2fe5 --- /dev/null +++ b/tests/comment-viewer-resource.test.ts @@ -0,0 +1,274 @@ +/** + * Comment viewers as a managed resource (#151) — the last scope dimension a config could not express + * portably. + * + * `cdb_comment_viewer` had a reference form (`{ commentViewer: "…" }`, #102) but no declaration + * behind it, so a config granting `churchdb:view comments` had to write a raw numeric `dataId`. That + * id is host-specific: read on two hosts of the same deployment on 2026-08-24, three of prod's six + * viewer ids did not exist on dev at all, and the two present on both named different categories on + * each. Nothing detected it — `ct plan` compares the declared id against the live id and they match; + * the id is simply meaningless on the target host. The name ref was no escape either, because the + * NAMES were missing on the second host too, so the ref hard-errored rather than resolving wrongly. + * + * Declaring the viewer is what makes the name exist on both hosts, which is what makes the ref + * portable. `/person/commentviewers` is conventional REST, so this type needs none of the machinery + * the other two awkward master-data types did: CT mints the id (unlike `security-level`) and the + * writes are REST (unlike `department`). Fully live-probed on eqrm-dev, CT 3.135.2 (2026-08-26): + * collection GET/POST, item GET/PUT/DELETE, absent item id → clean 404. + * + * The one dataId that is NOT host-specific is `0`, the built-in "Alle" viewer CT ships everywhere — + * so it is emitted as a number and never offered for adoption (see the "Alle" test below). + */ +import { describe, it, expect, vi } from "vitest"; +import { evaluateConfig } from "../src/config/context.js"; +import { executePlan } from "../src/engine/execute.js"; +import { computePlan } from "../src/engine/plan.js"; +import { tierOf, isKnownType } from "../src/engine/graph.js"; +import { applyPermissionPlan } from "../src/permissions/apply.js"; +import { buildPermissionPlan } from "../src/permissions/plan.js"; +import { RESOURCES, configSnippet, knownFields } from "../src/resources/registry.js"; +import { emptyState, type State } from "../src/state/state.js"; +import type { DesiredPermission } from "../src/permissions/types.js"; +import type { DesiredResource, Plan } from "../src/engine/types.js"; +import type { CtClient } from "../src/api/ctClient.js"; + +const HOST = "https://mychurch.church.tools"; +const PATH = "/person/commentviewers"; +const SPEC = RESOURCES["comment-viewer"]!; + +/** `churchdb:view comments` — authId 113, the one right scoped by `cdb_comment_viewer`. */ +const VIEW_COMMENTS = "churchdb:view comments"; + +const noSave: (path: string, state: State) => Promise = async () => {}; + +function recorder(responses: Record = {}) { + const calls: { method: string; path: string; body?: unknown }[] = []; + const client = { + request: async (method: string, path: string, body?: unknown): Promise => { + calls.push({ method, path, body }); + return (responses[`${method} ${path}`] ?? {}) as T; + }, + }; + return { client, calls }; +} + +/** State holding one viewer under key `dienstbereich`, as a prior apply would have left it. */ +function stateWithViewer(id: number): State { + return { + version: 1, + host: HOST, + resources: { + dienstbereich: { + type: "comment-viewer", + id, + key: "dienstbereich", + fields: { name: "Dienstbereich", sortKey: 40 }, + adoptedAt: "t", + updatedAt: "t", + }, + }, + }; +} + +/** A grant scoped by a comment viewer, declared with the portable ref form. */ +const viewerScoped: DesiredPermission = { + key: "pastoral_care_grants", + domainType: "group_role", + domainId: 900, + grants: [{ right: VIEW_COMMENTS, scope: [{ commentViewer: "dienstbereich" }] }], +}; + +describe("declaring, creating and updating a comment viewer (#151)", () => { + it("is a plain collection POST — CT mints the id, unlike a security level", async () => { + const state = emptyState(HOST); + const { client, calls } = recorder({ [`POST ${PATH}`]: { id: 7 } }); + const plan: Plan = { + items: [ + { + type: "comment-viewer", + key: "dienstbereich", + id: null, + action: "create", + changes: [ + { field: "name", from: undefined, to: "Dienstbereich" }, + { field: "sortKey", from: undefined, to: 40 }, + ], + }, + ], + }; + + const res = await executePlan(plan, { client, state, statePath: "unused", save: noSave }); + expect(res.failed).toBeUndefined(); + expect(calls).toEqual([{ method: "POST", path: PATH, body: { name: "Dienstbereich", sortKey: 40 } }]); + expect(state.resources.dienstbereich).toMatchObject({ type: "comment-viewer", id: 7 }); + // Pinned against a future refactor that makes the caller-assigned-id path unconditional: this + // type must NOT post to `/person/commentviewers/{id}` the way a security level does. + expect(SPEC.createPath).toBeUndefined(); + expect(SPEC.callerAssignedId).toBeUndefined(); + }); + + it("supplies a neutral sortKey at create when the declaration omits it", () => { + // `sortkey` is a non-nullable integer column on these 3-column master-data tables, so a + // hand-authored declaration carrying only `name` still needs a valid create body. A DECLARED + // value wins (createDefaults merges UNDER the body) — that ordering is the executor's, so all + // this pins is that the default exists and is the neutral one. + expect(SPEC.createDefaults?.({ name: "Dienstbereich" })).toEqual({ sortKey: 0 }); + }); + + it("updates with a PUT carrying the whole managed set", async () => { + const state = stateWithViewer(4); + const { client, calls } = recorder(); + const plan: Plan = { + items: [ + { + type: "comment-viewer", + key: "dienstbereich", + id: 4, + action: "update", + actual: { name: "Dienstbereich", sortKey: 40 }, + changes: [{ field: "name", from: "Dienstbereich", to: "Dienstbereiche" }], + }, + ], + }; + + const res = await executePlan(plan, { client, state, statePath: "unused", save: noSave }); + expect(res.failed).toBeUndefined(); + // PUT is a full replace, so the unchanged sibling has to travel with it — `name` and `sortKey` + // are the whole editable surface, so nothing outside the managed set can be blanked. + expect(calls).toEqual([ + { method: "PUT", path: `${PATH}/4`, body: { name: "Dienstbereiche", sortKey: 40 } }, + ]); + }); + + it("applies in the master-data tier, before permissions", () => { + expect(isKnownType("comment-viewer")).toBe(true); + expect(tierOf("comment-viewer")).toBe(0); + }); + + it("names what a delete reaches, the way person-status and security-level do", () => { + expect(SPEC.destroyWarning).toMatch(/churchdb:view comments/); + expect(SPEC.destroyWarning).toMatch(/ct get comment-viewers/); + }); +}); + +describe("adopt → config → plan round-trips to a no-op (#151)", () => { + it("uses the DEFAULT item read — `GET {itemPath}` exists here (probed 2026-08-26)", () => { + // This type briefly carried a collection-filtering `fetchOne` because a GET on the item path was + // unprobed, and guessing it has a genuinely bad failure mode (#108): a 404 reads as "vanished in + // ChurchTools", so every plan proposes creating the viewer again. The live probe retired the + // guess — `GET /person/commentviewers/{id}` returns the row, and an ABSENT id returns a clean + // 404 `error.notfound`, which is exactly the distinction the default read needs. Keeping the hook + // would cost one full collection read per managed viewer per plan/apply/destroy (`fetchActual` + // fans out concurrently) against a rate-limited API. `department` is the only type that still + // needs the hook, because `/departments/{id}` genuinely does not exist. + expect(SPEC.fetchOne).toBeUndefined(); + }); + + it("emits a declaration that diffs clean against the row it was adopted from", async () => { + const live = { id: 4, name: "Dienstbereich", sortKey: 40 }; + const fields = SPEC.managedFields(live); + expect(fields).toEqual({ name: "Dienstbereich", sortKey: 40 }); + + const snippet = configSnippet("comment-viewer", "dienstbereich", fields); + expect(snippet).toContain("commentViewer("); + expect(snippet).toContain('name: "Dienstbereich"'); + + const { resources } = await evaluateConfig((ct) => { + ct.commentViewer({ key: "dienstbereich", name: "Dienstbereich", sortKey: 40 }); + }); + expect(resources[0]).toMatchObject({ type: "comment-viewer", key: "dienstbereich" }); + const plan = computePlan(resources, stateWithViewer(4), new Map([["dienstbereich", live]])); + expect(plan.items.map((i) => i.action)).toEqual(["no-op"]); + }); + + it("accepts `name` and `sortKey` as known fields — no unknown-field warning", () => { + expect(knownFields("comment-viewer")).toEqual(new Set(["name", "sortKey"])); + }); +}); + +describe("the same config means the same thing on two hosts (#151)", () => { + /** A client whose permission reads are empty and whose viewer catalog is host-specific. */ + function mockClient(viewers: { id: number; name: string }[] = [], newId = 555) { + const calls: { method: string; path: string; body?: unknown }[] = []; + const get = vi.fn(async (path: string) => (path === PATH ? viewers : [])); + const request = vi.fn(async (method: string, path: string, body?: unknown) => { + calls.push({ method, path, body }); + if (method === "POST" && path === PATH) return { id: newId }; + return {}; + }); + return { client: { get, request } as unknown as CtClient, calls }; + } + + it("resolves the ref from MANAGED state, so the dataId differs per host and the config does not", async () => { + // The exact failure #151 exists to close: the declared viewer is id 4 on one host and id 2 on the + // other. Written as a raw `scope: [4]` the second host gets a grant pointing at a different + // viewer — or at nothing — and no plan, row count or diff can see it. + for (const id of [4, 2]) { + const { client } = mockClient(); + const { items, fetchErrors } = await buildPermissionPlan(client, stateWithViewer(id), [viewerScoped]); + expect(fetchErrors).toEqual([]); + expect(items[0]?.diff.toPut).toEqual([ + { authId: 113, dataId: [id], type: "grant", scopeKey: "dienstbereich", scopeType: "comment-viewer" }, + ]); + } + }); + + it("a viewer declared in the SAME run is pending at plan time and gets its real id at apply time", async () => { + // The fresh-host case: nothing exists yet, so the viewer and the grant that scopes to it have to + // land in one run. Before #151 this could not be expressed at all — the viewer had to be created + // by hand on every target instance first. + const desired: DesiredResource[] = [ + { + type: "comment-viewer", + key: "dienstbereich", + fields: { name: "Dienstbereich", sortKey: 40 }, + dependsOn: [], + }, + ]; + const { client, calls } = mockClient([], 555); + const state = emptyState(HOST); + const { items } = await buildPermissionPlan(client, state, [viewerScoped], desired); + expect(items[0]?.diff.toPut).toEqual([ + { + authId: 113, + dataId: [], + type: "grant", + scopeKey: "dienstbereich", + scopeType: "comment-viewer", + pending: true, + }, + ]); + + const createViewer: Plan = { + items: [ + { + type: "comment-viewer", + key: "dienstbereich", + id: null, + action: "create", + changes: [ + { field: "name", from: undefined, to: "Dienstbereich" }, + { field: "sortKey", from: undefined, to: 40 }, + ], + }, + ], + }; + await executePlan(createViewer, { client, state, statePath: "unused", save: noSave }); + expect(state.resources.dienstbereich?.id).toBe(555); + + const res = await applyPermissionPlan(items, client, state); + expect(res.granted).toBe(1); + const put = calls.find((c) => c.method === "PUT" && c.path === "/permissions/group_role/900"); + expect(put?.body).toEqual({ authId: 113, type: "grant", dataId: [555] }); + }); + + it("still falls back to the live catalog for a viewer this config does not own", async () => { + // The compatibility half of the promotion: a name ref against an unmanaged viewer keeps working + // exactly as it did in #102, so no existing config has to change. + const { client } = mockClient([{ id: 2, name: "Dienstbereich" }]); + const { items, fetchErrors } = await buildPermissionPlan(client, emptyState(HOST), [viewerScoped]); + expect(fetchErrors).toEqual([]); + // Catalog-resolved: already host-correct, so no managed identity is carried for re-resolution. + expect(items[0]?.diff.toPut).toEqual([{ authId: 113, dataId: [2], type: "grant" }]); + }); +}); diff --git a/tests/permission-adopt.test.ts b/tests/permission-adopt.test.ts index b613ed2..08db9bb 100644 --- a/tests/permission-adopt.test.ts +++ b/tests/permission-adopt.test.ts @@ -491,21 +491,43 @@ describe("emitAdoptedGrants", () => { expect(block).toContain('scope: [{ campus: "koblenz" }, 99]'); }); - it("a catalog-only dimension gets ONE note — the portable form, not the 'not a group' line", () => { + it("points an UNMANAGED comment-viewer scope at `ct adopt comment-viewer` (#151)", () => { + // This dimension used to be the last catalog-only one, and got the weaker "portable form: + // { commentViewer: \"\" }" note — honest then, useless in practice, because the NAME did + // not exist on the other host either. Comment viewers are a managed resource now, so the advice + // is the same as for every other dimension: adopt the viewer, then re-adopt the grants. const rows: RawPermission[] = [ - // cdb_comment_viewer — a catalog ct reads but does not manage (#102). Bereiche used to be the - // example here; they became a managed resource in #108, so they now take the adopt-hint path. { authId: 113, dataId: 4, type: "grant", domainId: 42, meta: { modifiedPid: 5 } }, ]; const block = emitAdoptedGrants({ domainType: "group_role", domainId: 42, rows, state: emptyState() }); - expect(block).toContain('Portable form: { commentViewer: "" }'); - // The numeric-escape-hatch line is for dimensions with NO logical form; emitting it here too - // would contradict the portable-form note directly above it. + expect(block).toContain("ct adopt comment-viewer 4"); + // The numeric-escape-hatch line is for dimensions with NO logical form at all. expect(block).not.toContain("not a group"); expect(block).toContain('{ right: "churchdb:view comments", scope: [4] }'); }); + it("emits a MANAGED comment-viewer scope as the portable ref form (#151)", () => { + const state = emptyState(); + state.resources.dienstbereich = { + type: "comment-viewer", + id: 4, + key: "dienstbereich", + fields: { name: "Dienstbereich", sortKey: 40 }, + adoptedAt: "t", + updatedAt: "t", + }; + const rows: RawPermission[] = [ + { authId: 113, dataId: 4, type: "grant", domainId: 42, meta: { modifiedPid: 5 } }, + ]; + const block = emitAdoptedGrants({ domainType: "group_role", domainId: 42, rows, state }); + + // The whole point of #151: what used to be a raw host-specific 4 is now a name that means the + // same thing on the host this config is replayed against. + expect(block).toContain('scope: [{ commentViewer: "dienstbereich" }]'); + expect(block).not.toContain("ct adopt comment-viewer"); + }); + it("points an unmanaged Bereich scope at `ct adopt department` now that Bereiche are managed (#108)", () => { const rows: RawPermission[] = [ { authId: 102, dataId: 4, type: "grant", domainId: 42, meta: { modifiedPid: 5 } }, // cdb_bereich @@ -619,6 +641,62 @@ describe("emitAdoptedGrants", () => { }); }); +describe('the built-in "Alle" comment viewer (dataId 0) is not a host-specific id (#151)', () => { + // `churchdb:view comments` (authId 113) scopes by `cdb_comment_viewer`. Unlike `-1`, `0` is a REAL + // row — but it is one CT ships on every instance, so the number already means the same thing on + // every host. Adopting it is the trap: `ct adopt comment-viewer 0` yields + // `ct.commentViewer({ key: "alle", name: "Alle" })`, and replaying that on a second host with a + // fresh state finds no state entry, POSTs a SECOND "Alle", and scopes the grant to the duplicate — + // the exact misgrant #151 exists to prevent, plus a catalog with two "Alle" rows that makes + // `{ commentViewer: "alle" }` permanently ambiguous. + const alleRows: RawPermission[] = [ + { authId: 113, dataId: 0, type: "grant", domainId: 42, meta: { modifiedPid: 5 } }, + ]; + + const block = (rows: RawPermission[] = alleRows) => + emitAdoptedGrants({ domainType: "group_role", domainId: 42, rows, state: emptyState() }); + + it("never tells you to adopt it", () => { + expect(block()).not.toMatch(/ct adopt comment-viewer 0/); + }); + + it("does not call it host-specific, because it is not", () => { + expect(block()).not.toMatch(/0 is not managed/); + expect(block()).not.toMatch(/host-specific number/); + }); + + it("says what it actually is", () => { + expect(block()).toMatch(/scope 0 is the built-in "Alle" comment-viewer/); + expect(block()).toContain("present on every instance"); + }); + + it("emits the NUMBER, not a name — a built-in row can still be renamed by an admin", () => { + expect(block()).toContain("scope: [0]"); + expect(block()).not.toContain("{ commentViewer:"); + }); + + it("still flags a REAL unmanaged viewer id in the same grant", () => { + const mixed: RawPermission[] = [ + { authId: 113, dataId: 0, type: "grant", domainId: 42, meta: { modifiedPid: 5 } }, + { authId: 113, dataId: 4, type: "grant", domainId: 42, meta: { modifiedPid: 5 } }, + ]; + const out = block(mixed); + expect(out).toContain("scope: [0, 4]"); + expect(out).toMatch(/ct adopt comment-viewer 4/); + expect(out).not.toMatch(/ct adopt comment-viewer 0/); + // The "4 is not managed" note must name 4 alone, never "0, 4". + expect(out).not.toMatch(/0, 4 (is|are) not managed/); + }); + + it("is scoped to the comment-viewer dimension — `0` elsewhere is an ordinary id", () => { + // `churchdb:view alldata` (102) scopes by `cdb_bereich`, where 0 has no special meaning. + const other: RawPermission[] = [ + { authId: 102, dataId: 0, type: "grant", domainId: 42, meta: { modifiedPid: 5 } }, + ]; + expect(block(other)).toMatch(/ct adopt department 0/); + }); +}); + describe('the `-1` "alle" sentinel is not a host-specific id (#115)', () => { // `churchdb:view alldata` (authId 102) scopes by `cdb_bereich`; `churchdb:view station` (124) by // `cdb_station`. A grant scoped to "alle" comes back as `dataId: -1`.