Repository navigation
fix: handle ChurchTools group children responses - #137
Merged
Merged
Conversation
bwl21
marked this pull request as ready for review
August 22, 2026 17:53
bwl21
changed the title
Review of eqrm#137 found the new children handling reads a paginated list endpoint with a plain `get`, so `--children-of` saw only CT's default first page (10 rows) and never descended into the rest of a wide Bereich — silently producing an incomplete config that `ct plan` then reports as unmanaged groups. Same bug class as eqrm#101. - read the endpoint with `getAll`, never a plain `get` (eqrm#101). That also subsumes the hand-rolled envelope normalization (`extractChildren`, now removed): `getAll` already absorbs both the bare-array and the `{ data: [...] }` page shapes, and an empty 204 body — which for a leaf group means "no children" rather than the hard failure it had become. Detecting a wholly unknown object shape goes with it; that shape has never been observed from CT, and shape normalization for list reads belongs in `CtClient`, which has its own coverage. - prefer `domainIdentifier` over `id` when resolving a child row. For a domain resource (`domainType: "group"`) `domainIdentifier` is the authoritative group id; a sibling `id` would be the hierarchy edge's own id, so preferring it walks into the wrong groups with no error. - gate the `apiUrl` fallback on `== null`, not `=== undefined`: a row of all-null ids with a usable `apiUrl` used to throw. - name the failing group in both error messages instead of printing the literal `/groups/{id}/children` placeholder — `--children-of` walks whole subtrees, so "some group somewhere" is not actionable. - restore the `collectSubtreeIds` JSDoc, which had drifted onto the function inserted beneath it. Claude-Session: https://claude.ai/code/session_013f7Wx1b9m8w2AVp4HCsnot
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
2 participants
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.
Summary
{ data: [...] }responses from the group children endpointdomainIdentifier, withapiUrlas a fallbackRoot cause
The live
GET /groups/{id}/childrenendpoint returns domain resources. Their group ID is exposed as a numericdomainIdentifier, not asid. AlthoughCtClientalready unwraps the normaldataenvelope, the traversal discarded every returned child because it only inspectedid.The previous implementation also treated every 404 from the children endpoint as a leaf. The ChurchTools OpenAPI defines that response as resource not found; a real leaf is represented by a successful empty collection.
Validation
npm test— 810 passed, 5 skippednpm run typechecknpm run lintnode .github/scripts/docs-staleness.mjsgit diff --checkFixes #136