fix(decisions): feed the detail pages' type picker from the registry - #1111
Merged
Conversation
#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>
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 11:31 UTC
Download the full PDF report from the workflow artifacts.
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.
Closes the known gap #1109 filed: its index-page fix could not reach the detail pages, because
CnDetailPagehad no seam to reach them through.The defect
#1099 moved the
decisionTypevocabulary out of the stored schema'senumand into thedecision_typesapp config, which is now the only authority. #1109 spliced it back into the Decisions and Motions index dialogs throughCnIndexPage'sform-dialogslot, and said so plainly: the detail pages' built-in Edit dialog still read the emptied enum, so the picker offered nothing while the field stayed required. A form that cannot be completed.CnDetailPagegrew the same slot, with deliberately the same name and the same scope, in nextcloud-vue#944, published as 2.34.0. This takes that version and wires the detail surfaces.Four surfaces, not one
The gap named the decision detail page. The sweep found four, and the fourth is the one worth naming:
DecisionDetail/decisions/:iddecisionMotionDetail/motions/:iddecisionType=motionAmendmentDetail/amendments/:iddecisionType=amendmentDecisionIntegrations/decisions/:id/integrationstype: "detail"with aconfig.schemaCnPageRenderersetsshowEditActionon a schema-bound detail page unconditionally, so the integrations surface offers the same Edit dialog over the same schema and had the same empty picker. Nothing about the page suggests it, which is exactly why it was missed.MotionIntegrationsis deliberately NOT wired: it mountsCnDetailPageby hand and passes no schema, so it renders no form dialog at all.No second copy of the vocabulary logic
All four reuse #1109's
DecisionFormDialogand itswithDecisionTypeVocabulary()helper, through each page'sslotsmap. Six wirings, one splice, so they cannot drift. The schema handed to the form is a copy; the store's schema is never mutated.The locked-modal trap
One behavioural change in
DecisionFormDialog, and it is the reason the slot's contract is shaped the way it is. A replacement holds the only ref to its ownCnFormDialog, which raisesloadingon submit; onlysetResult()lowers it, andno-closeis bound toloading.CnDetailPageleaves the form open on a failed edit, so a replacement that drops the resolved result strands the user in a modal that can neither retry nor close, with nothing in the console.settleFormDialogResult()(new,src/dialogs/formDialogResult.js) handsconfirm's resolved{ success, data }/{ error }back to the dialog, and treats "resolved to nothing" as a normal outcome rather than a fault. That is still whatCnIndexPage's confirm does, so the same component keeps serving the index pages unchanged.Tests
tests/vitest/decisionDetailFormDialog.spec.js, 15 tests in this repo's idiom (logic in importable.js, manifest asserted againstsrc/manifest.jsonitself, since vitest here runs without@vitejs/plugin-vue):MotionDetail's wiring fails the run.lib/Settings/decidesk_register.json's Decision carries nodecisionTypeenum and lists it as required, which is precisely zero options in the built-in dialog. Passing that same schema through the helper yields the registry vocabulary, and the seed while the fetch is still in flight.settleFormDialogResultcloses on success, unlocks on failure, stays out of the way for a confirm that resolves to nothing, and survives the dialog having unmounted mid-save.Verified locally
formatcheck:manifest(Ajv)check:schema-l10n,test:l10n,check:nav-ceiling--scope-to-diffvsorigin/developmentpackage.jsonandpackage-lock.jsonboth on 2.34.0, installed build carries the slotNo PHP changed, so the PHP suite is untouched by this diff.
🤖 Generated with Claude Code