Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
18 changes: 18 additions & 0 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -173,6 +173,24 @@ as a normal field update, and `ct adopt group <id>` captures it. Which group
fields are managed vs. deliberately left to the CT UI is recorded in
[`docs/group-field-decisions.md`](docs/group-field-decisions.md).

**Same-named groups (#75):** ChurchTools guards group creation by NAME, not by
this tool's logical key — `POST /groups` 400s (`forbidden.duplicate.group`) if a
group with that name already exists, even when the two are legitimately
distinct (e.g. an archived and an active "Kids Elternabend 2026" event signup).
Opt in per-declaration to create it anyway:

```ts
ct.group({ key: "kids_2026_b", name: "Kids Elternabend 2026", groupTypeId: 2, allowDuplicateName: true });
```

`allowDuplicateName` sends CT's `force: true` on the CREATE request only — it is
never a managed field (not diffed, not in state, not touched on update, and
never adopted). **Never set it as a default**; it exists for the rare
intentional-duplicate case. If a create 400s on this guard without the flag
set, `ct apply`'s stop message explains the likely cause (an unmanaged existing
group that should be adopted with `ct adopt group <id> --key <key>`) and the
opt-in as the alternative.

Machine-readable output goes to **stdout** (pipe/`jq` it); human status lines go
to **stderr**.

Expand Down
1 change: 1 addition & 0 deletions docs/group-field-decisions.md
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,7 @@ field — a plain top-level key. `campusId` is wired the same deliberate way as
| **`campusId`** | **managed (new, #21)** | The campus link is the tool's core "instantiate this area per campus" requirement. Numeric escape hatch only — an existing CT campus id (or `null` to clear). A *logical* `campus: "key"` reference (resolving a same-run campus by key) is **deferred to [#20](https://github.com/eqrm/ct-cli/issues/20)**; the DSL rejects a `campus` field with a pointer to #20 so it can't slip through as an un-diffable phantom. |
| `parents` (hierarchy) | **opt-in synthetic** | Group→group hierarchy is reconciled through its own endpoint, not the group body — see `src/engine/synthetic.ts`. Opt-in via `parents: [...]`. |
| `dynamic` (auto-group ruleset) | **opt-in synthetic** | Ruleset + status live behind a dedicated endpoint (#14); opt-in via the `dynamic` block. See `docs/dynamic-groups.md`. |
| `allowDuplicateName` (CT's `force` create flag, #75) | **create-only, unmanaged** | Not a field on the live group at all — it's a request-only escape hatch for `POST /groups`' same-name guard (`forbidden.duplicate.group`). Opt-in per declaration; sent as `force: true` on CREATE only. Deliberately NOT a registry `managedFields` entry: it has no live value to diff against, so it is destructured out of the DSL input before the field bag (never triggers the unknown-field warning, never lands in state, never adopted, never touched on update). |
| `visibility` | **out of scope** | Not rights-bearing structure; instance-/policy-specific and easily changed in the UI. No demand in #21 to manage it. Promote later only with its own registry entry + tests. |
| `note` | **out of scope** | Free-text annotation, not structure. Managing it would fight human edits in the UI for no structural benefit. |
| `autoAccept` / open-for-members settings | **out of scope** | Membership-request policy — adjacent to *who is in a group*, which the tool never manages (`assertNotPeople`, `src/engine/guard.ts`). Left to the UI. |
Expand Down
23 changes: 22 additions & 1 deletion src/config/context.ts
Original file line number Diff line number Diff line change
Expand Up @@ -104,6 +104,15 @@ export interface ResourceInput {
* setting `false` (or removing it) and re-applying before you destroy.
*/
preventDestroy?: boolean;
/**
* Group-only (#75): opt in to CT's `force` create flag so a group can be created with the same
* name as an existing one. `POST /groups` otherwise 400s (`forbidden.duplicate.group`) whenever
* a same-named group already exists — CT's guard is name-based, not key-based, so two
* legitimately same-named groups (e.g. an archived and an active event signup) need this to
* both be creatable. Create-time only: never diffed, never in state, never sent on update.
* NEVER force by default — omit (or `false`) to keep CT's guard on.
*/
allowDuplicateName?: boolean;
[field: string]: unknown;
}

Expand Down Expand Up @@ -229,10 +238,21 @@ function desugarDynamic(type: string, key: string, dynamic: unknown): DynamicSpe
}

function toDesired(type: string, input: ResourceInput, location?: string): DesiredResource {
const { key, parent, parents, dependsOn = [], preventDestroy, dynamic, ...fields } = input;
const { key, parent, parents, dependsOn = [], preventDestroy, dynamic, allowDuplicateName, ...fields } = input;
if (!key || typeof key !== "string") {
throw new Error(`${type} declaration is missing a string "key".`);
}
// Group-only opt-in (#75) into CT's `force` create flag — see ResourceInput.allowDuplicateName.
// Destructured out above (never reaches `fields`), so it is accepted but never diffed/managed/
// adopted, and never trips the unknown-field warning below.
if (allowDuplicateName !== undefined) {
if (type !== "group") {
throw new Error(`${type} "${key}": "allowDuplicateName" is only valid on a group.`);
}
if (typeof allowDuplicateName !== "boolean") {
throw new Error(`${type} "${key}": "allowDuplicateName" must be a boolean.`);
}
}
// A nullish/empty `parent` is "no parent", not an opt-in to managed-empty hierarchy.
if (parent != null && typeof parent !== "string") {
throw new Error(`${type} "${key}": "parent" must be a string key.`);
Expand Down Expand Up @@ -316,6 +336,7 @@ function toDesired(type: string, input: ResourceInput, location?: string): Desir
dynamic: dynamicSpec,
dependsOn: edges,
preventDestroy,
allowDuplicateName,
};
}

Expand Down
61 changes: 58 additions & 3 deletions src/engine/execute.ts
Original file line number Diff line number Diff line change
Expand Up @@ -9,16 +9,61 @@
* their own dedicated endpoints, not the owning resource's body — see synthetic.ts.
*/
import type { CtClient } from "../api/ctClient.js";
import { CtApiError } from "../api/ctClient.js";
import type { ManagedResource, State } from "../state/state.js";
import { upsert, saveState } from "../state/state.js";
import type { FieldChange, Plan } from "./types.js";
import type { FieldChange, Plan, PlanItem } from "./types.js";
import { RESOURCES } from "../resources/registry.js";
import { assertNotPeople } from "./guard.js";
import { isSyntheticField, syntheticField } from "./synthetic.js";
import { reresolvePendingValue } from "../resolve/resolver.js";
import { hasPendingRef } from "../resolve/refs.js";
import { formatError } from "../ui.js";

/**
* The messageKey CT's `POST /groups` 400s with when a same-named group already exists and the
* body did not carry `force: true` (#75). Confirmed against the ChurchTools OpenAPI spec's
* analogous `POST /persons` duplicate-guard error envelope (`forbidden.duplicate.person`, the
* same `{ message, messageKey, translatedMessage, args, errors }` shape) and the live 400 text
* observed against a dev rehearsal instance (issue #75): "Duplicate found. Use force flag to
* create group with same name." `POST /groups` itself is undocumented beyond "Bad Request" in the
* spec, so both the messageKey AND a text fallback are checked below.
*/
const DUPLICATE_GROUP_MESSAGE_KEY = "forbidden.duplicate.group";

/** Detect CT's same-name group-creation guard (#75) from a caught create error. */
function isDuplicateGroupNameError(err: unknown): boolean {
if (!(err instanceof CtApiError) || err.status !== 400) return false;
const body = err.body;
if (!body || typeof body !== "object") return false;
const b = body as Record<string, unknown>;
if (b.messageKey === DUPLICATE_GROUP_MESSAGE_KEY) return true;
const message = b.message;
return (
typeof message === "string" &&
/duplicate/i.test(message) &&
/force flag/i.test(message) &&
/group/i.test(message)
);
}

/**
* Append actionable guidance (#75) to an otherwise-formatted stop message when a group create
* failed on CT's same-name guard without the `allowDuplicateName` opt-in: point at adopting the
* existing group (the usual accident) or opting in (the rare intentional-duplicate case). Reuses
* `formatError`'s output verbatim — never forks the HTTP status/body formatting.
*/
function withDuplicateGroupGuidance(message: string, item: PlanItem): string {
return (
`${message}\n` +
`Guidance: a group named like "${item.key}" likely already exists in ChurchTools. If it should ` +
`be managed by this tool, adopt it instead of creating a new one: ` +
`\`ct adopt group <id> --env <env> --key ${item.key}\` (find its id with \`ct get groups\`). ` +
`If two groups sharing this name is intentional, set \`allowDuplicateName: true\` on this ` +
`group's declaration and re-apply`
);
}

export interface ExecuteDeps {
client: Pick<CtClient, "request">;
state: State;
Expand Down Expand Up @@ -109,7 +154,10 @@ export async function executePlan(plan: Plan, deps: ExecuteDeps): Promise<Execut
// win) for the POST ONLY. State still records `body` (the managed fields), so the defaults
// stay unmanaged — a later plan neither diffs nor reverts them. A field still missing after
// this surfaces as CT's HTTP 400 (#71), not a silent omission.
const createBody = spec.createDefaults ? { ...spec.createDefaults(body), ...body } : body;
const defaultedBody = spec.createDefaults ? { ...spec.createDefaults(body), ...body } : body;
// CT's same-name group-creation guard (#75): opt-in only, via `force: true` on the POST body.
// Never a managed field — not in `body`/state, so it never diffs and never touches update.
const createBody = item.allowDuplicateName ? { ...defaultedBody, force: true } : defaultedBody;
assertNotPeople(spec.collectionPath);
const res = await client.request<{ id: number }>("POST", spec.collectionPath, createBody);
if (typeof res.id !== "number") {
Expand Down Expand Up @@ -155,11 +203,18 @@ export async function executePlan(plan: Plan, deps: ExecuteDeps): Promise<Execut
// Route through the same formatter the top-level handler uses (#50) so a mid-apply
// CtApiError's HTTP status + response body survive into the "Stopped at" line (#71) —
// without it, the stop message was undiagnosable ("... failed", no status/body).
let message = formatError(err);
// #75: a group create that hit CT's same-name guard without opting in gets actionable
// guidance appended — most often this is an unmanaged existing group that should be
// adopted, not an intentional duplicate.
if (item.action === "create" && item.type === "group" && !item.allowDuplicateName && isDuplicateGroupNameError(err)) {
message = withDuplicateGroupGuidance(message, item);
}
return {
created,
updated,
skippedDeletes,
failed: { key: item.key, message: formatError(err) },
failed: { key: item.key, message },
};
}
}
Expand Down
2 changes: 2 additions & 0 deletions src/engine/plan.ts
Original file line number Diff line number Diff line change
Expand Up @@ -153,6 +153,7 @@ export function computePlan(
action: "create",
changes: attributeCreate(diffFields(d.fields, {})),
preventDestroy: d.preventDestroy,
allowDuplicateName: d.allowDuplicateName,
});
continue;
}
Expand Down Expand Up @@ -199,6 +200,7 @@ export function computePlan(
changes: attributeCreate(diffFields(d.fields, {})),
note: "recreate",
preventDestroy: d.preventDestroy,
allowDuplicateName: d.allowDuplicateName,
});
continue;
}
Expand Down
13 changes: 13 additions & 0 deletions src/engine/types.ts
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,13 @@ export interface DesiredResource {
dependsOn: string[];
/** Lifecycle flag: block `ct destroy` for this resource. Never diffed or sent to the API. */
preventDestroy?: boolean;
/**
* Group-only create-time opt-in (#75) into CT's same-name guard: `POST /groups` 400s with
* `forbidden.duplicate.group` when a group with that name already exists, unless the body
* carries `force: true`. Sent on CREATE only — never diffed, never in `fields`/state, never
* touches the update path. `undefined` = not opted in (CT's default guard stays on).
*/
allowDuplicateName?: boolean;
}

export type PlanAction = "create" | "update" | "delete" | "no-op";
Expand Down Expand Up @@ -89,6 +96,12 @@ export interface PlanItem {
* on desired-side items; undefined on delete-side items.
*/
preventDestroy?: boolean;
/**
* CT's `force` create flag opt-in, carried from `DesiredResource.allowDuplicateName` (#75).
* Only meaningful on `action === "create"`; the executor sends `force: true` in the POST body
* when set, and never touches update/delete. Undefined elsewhere.
*/
allowDuplicateName?: boolean;
}

export interface Plan {
Expand Down
47 changes: 47 additions & 0 deletions tests/context.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -362,6 +362,53 @@ describe("preventDestroy lifecycle flag", () => {
});
});

describe("allowDuplicateName create-time opt-in (#75)", () => {
it("is carried on the resource but kept out of managed fields (not diffed/adopted)", () => {
const { ct, resources } = createContext();
ct.group({ key: "kids_2026_b", name: "Kids Elternabend 2026", groupTypeId: 2, allowDuplicateName: true });
expect(resources[0]?.allowDuplicateName).toBe(true);
expect(resources[0]?.fields).toEqual({ name: "Kids Elternabend 2026", groupTypeId: 2 });
});

it("defaults to undefined when not declared", () => {
const { ct, resources } = createContext();
ct.group({ key: "kids", name: "Kids" });
expect(resources[0]?.allowDuplicateName).toBeUndefined();
});

it("does not trip the unknown-field warning", () => {
const spy = vi.spyOn(process.stderr, "write").mockImplementation(() => true);
try {
const { ct } = createContext();
ct.group({ key: "kids_2026_b", name: "Kids Elternabend 2026", allowDuplicateName: true });
expect(spy).not.toHaveBeenCalled();
} finally {
spy.mockRestore();
}
});

it("rejects the flag on a non-group type", () => {
const { ct } = createContext();
expect(() => ct.campus({ key: "c", name: "C", allowDuplicateName: true } as never)).toThrow(
/allowDuplicateName.*only valid on a group/i,
);
});

it("rejects a non-boolean value", () => {
const { ct } = createContext();
expect(() => ct.group({ key: "g", name: "G", allowDuplicateName: "yes" as never })).toThrow(
/allowDuplicateName.*must be a boolean/i,
);
});

it("`allowDuplicateName: false` is accepted and still kept out of fields", () => {
const { ct, resources } = createContext();
ct.group({ key: "g", name: "G", allowDuplicateName: false });
expect(resources[0]?.allowDuplicateName).toBe(false);
expect(resources[0]?.fields).toEqual({ name: "G" });
});
});

describe("permission declarations", () => {
it("collects groupRole / groupTypeRole with validated grants", async () => {
const mod = (ct: ConfigContext) => {
Expand Down
Loading
Loading