fix(decisions): feed every type picker from the registry, and carry the administrators group across the rename - #1109
Merged
rubenvdlinde merged 1 commit intoSep 3, 2026
Conversation
…he administrators group across the rename Two defects from the final one-engine acceptance proof. The cross-app create-proposal pickers were wired to the decision_types registry in #1104, but decidiq's OWN surfaces still read the stored schema's decisionType enum, which decision-types-as-configuration (#1099) deliberately emptied. The Add Decision dialog on the Decisions and Motions index pages therefore showed a required type picker with 'No results'. Those pages now wire CnIndexPage's form-dialog slot to a DecisionFormDialog that renders the same CnFormDialog over the same schema, with the vocabulary fetched through the shared decisionLink.js helpers from #1104 (registry endpoint, seeded-13 fallback, translated labels) and spliced in client-side by a new withDecisionTypeVocabulary helper. The sweep found the same class on two more surfaces: the agenda-item Motions tab and the motion Amendments tab render the decision schema too, and their empty required picker BLOCKED the save. There the type is the tab's discriminator rather than a choice, so the field is excluded from the form and written by onConfirm - which also fixes the motions tab creating rows with no decisionType at all, invisible to its own decisionType=motion filter. The detail pages' built-in edit dialog has no slot to replace and keeps the gap; that needs an nc-vue extension point. The register's authorization baseline named decidesk-administrators, the pre-rename app id, and OpenRegister's GroupProvisioner creates every group an authorization block names - so existing installs hold the admin group, and its hand-granted memberships, under the old id. A Nextcloud gid cannot be renamed, so the fix is create-and-migrate: the baseline now names decidiq-administrators first while keeping the old id honored (dropping it would strip access the moment the row re-imports, before any migration ran), and a MigrateAdminGroup repair step copies the old group's members into the new one. Idempotent, never deletes the old group, never removes a member, and a no-op on installs that never had the old group - those keep the documented admin-provisioned fail-closed posture. The register row version moves to 0.13.0 because the register import path skips on version_compare with no content-diff escape, unlike the schema path. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
rubenvdlinde
requested review from
SudoThijn,
WilcoLouwerse,
bbrands02,
remko48 and
rjzondervan
as code owners
September 3, 2026 08:36
Contributor
Quality Report — ConductionNL/decidiq @
|
| Check | PHP | Vue | Security | License | Tests |
|---|---|---|---|---|---|
| lint | ✅ | ||||
| phpcs | ✅ | ||||
| phpmd | ✅ | ||||
| psalm | ✅ | ||||
| phpstan | ✅ | ||||
| phpmetrics | ✅ | ||||
| eslint | ✅ | ||||
| stylelint | ✅ | ||||
| build | ✅ | ||||
| check-manifest | ✅ | ||||
| check-nav-ceiling | ✅ | ||||
| test-l10n | ✅ | ||||
| format | ✅ | ||||
| check-l10n-js | ✅ | ||||
| check-schema-l10n | ✅ | ||||
| composer | ✅ | ✅ 104/104 | |||
| npm | ✅ | ✅ 537/537 | |||
| app:check-code | ⏭️ | ||||
| info.xml | ✅ | ||||
| REUSE | ❌ | ||||
| PHPUnit | ✅ | ||||
| Newman | ✅ | ||||
| Playwright | ⏭️ deferred — runs on the promotion into beta/main, not on a pull request into development | ||||
| Hydra gates | ✅ |
Quality workflow — 2026-09-03 08:41 UTC
Download the full PDF report from the workflow artifacts.
rubenvdlinde
added a commit
that referenced
this pull request
Sep 3, 2026
…1111) #1109 fixed the Decisions and Motions INDEX pickers through CnIndexPage's form-dialog slot and filed the rest as a known gap: CnDetailPage had no such slot, so every detail page that renders the Decision schema kept showing an empty required decisionType picker in its built-in Edit dialog. #1099 emptied properties.decisionType.enum on purpose, and the detail pages had nowhere to splice the vocabulary back in. nextcloud-vue#944 added the slot, deliberately with the same name and the same scope as CnIndexPage's, so one replacement serves both page types. This takes 2.34.0 and wires FOUR surfaces, not the one the gap named: DecisionDetail, MotionDetail, AmendmentDetail and DecisionIntegrations. The last is easy to miss and renders the same form: a `type: "detail"` page with a `config.schema` gets its Edit affordance from CnPageRenderer unconditionally, so the picker was empty there too. All four reuse #1109's DecisionFormDialog and its withDecisionTypeVocabulary() helper rather than a second copy of the vocabulary logic, so the six wirings cannot drift. One behavioural change in that dialog. CnDetailPage's slot `confirm` resolves to `{ success: true, data }` / `{ error }` because a replacement holds the only ref to its own CnFormDialog: the dialog raises `loading` on submit, only setResult() lowers it, and `no-close` is bound to `loading`. A failed edit leaves the form open, so dropping the result strands the user in a modal that can neither retry nor close. settleFormDialogResult() hands the result back and treats "resolved to nothing" as normal, which is what CnIndexPage's confirm still does, so the same component keeps working on the index pages. Tests (tests/vitest/decisionDetailFormDialog.spec.js, 15): - every detail page bound to the decision schema is wired, asserted as an EQUALITY on the page set so a fifth one cannot ship unwired - the picker's negative control is the shipped schema itself: stored decisionType carries no enum and is required, so the built-in dialog renders zero options; passing it through the helper yields the registry vocabulary, and the store's schema is not mutated - settleFormDialogResult closes on success, unlocks on failure, and stays out of the way for a confirm that resolves to nothing or a dialog that unmounted mid-save Verified locally: vitest 28 files / 407 tests green, eslint 0 errors, prettier clean, stylelint 0 errors, manifest Ajv PASS, schema-l10n and l10n parity at baseline, hydra gates v1.11.0 diff-scoped green on the diff (gate-16 and gate-69 findings are pre-existing and untouched here). Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
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
1 participant
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.
Two defects from the final one-engine acceptance proof.
The Add Decision type picker showed 'No results'
The cross-app create-proposal pickers were wired to the decision_types registry in #1104. Decidiq's own surfaces were not. They still read the stored schema's decisionType enum, and #1099 deliberately emptied that enum. The registry became the only authority, and the built-in dialog never asked it.
Four surfaces carried the defect, and they get two different fixes:
form-dialogslot to a newDecisionFormDialog. It renders the same CnFormDialog over the same schema. The one difference: the schema copy driving the form carries the registry vocabulary, fetched through the shared decisionLink.js helpers from fix(approval-routes): advance stages through OR's patch path, and feed the type picker from the registry #1104. Same endpoint, same seeded-13 fallback, same translated labels. A newwithDecisionTypeVocabulary()helper in decisionLink.js does the splice, so the wirings cannot drift.onConfirm. This also fixes the motions tab creating rows with no decisionType at all, which fell out of its owndecisionType=motionfilter on the next refresh.Known gap: the detail pages' built-in edit dialog (CnDetailPage) has no slot to replace. Fixing that picker needs an nc-vue extension point. Filed here as a note rather than papered over.
The administrators group lived under the old app id
The register's authorization baseline named
decidesk-administrators. OpenRegister's GroupProvisioner creates every group an authorization block names, so existing installs hold the admin group, and its hand-granted memberships, under the pre-rename id.A Nextcloud gid cannot be renamed. IGroupManager offers create and delete only, and delete destroys memberships and shares. The chosen semantics are create-and-migrate, old id kept honored:
decidiq-administratorsfirst and keepsdecidesk-administratorsin the arrays. Dropping the old id would strip access the moment the row re-imports, before any migration ran. On a read-only group backend it would strip access forever.MigrateAdminGrouprepair step copies the old group's members into the new one. Idempotent. It never deletes the old group and never removes a member. On installs that never had the old group it is a no-op: the register import provisions the new group empty, keeping the documented admin-provisioned, fail-closed posture.version_compareand has no content-diff escape, unlike the schema path, so without the bump the authorization change never reaches existing installs.Verification
withDecisionTypeVocabulary🤖 Generated with Claude Code