diff --git a/package-lock.json b/package-lock.json index 205e141c..53a19b10 100644 --- a/package-lock.json +++ b/package-lock.json @@ -9,7 +9,7 @@ "version": "0.1.0", "license": "EUPL-1.2", "dependencies": { - "@conduction/nextcloud-vue": "^2.31.1", + "@conduction/nextcloud-vue": "^2.34.0", "@nextcloud/auth": "^2.6.0", "@nextcloud/axios": "~2.5.2", "@nextcloud/capabilities": "^1.2.1", @@ -562,9 +562,9 @@ } }, "node_modules/@conduction/nextcloud-vue": { - "version": "2.31.1", - "resolved": "https://registry.npmjs.org/@conduction/nextcloud-vue/-/nextcloud-vue-2.31.1.tgz", - "integrity": "sha512-tF1/7yNaBgxj5iHhpNuxVdUrPugfy2ePQZILl7y+fFJTq68tR7WPs8EqPin8aaCBc6ZxgWp1ciyY+NW9rXBMYA==", + "version": "2.34.0", + "resolved": "https://registry.npmjs.org/@conduction/nextcloud-vue/-/nextcloud-vue-2.34.0.tgz", + "integrity": "sha512-btUz646+ftoBZKnuARxTOWaOsUgYz3ih258BjV/IpJC/Zf3VCoUDepNOWFZNC30Uj+8xykMOgUcq/uqk4cUnCw==", "license": "EUPL-1.2", "dependencies": { "@ckpack/vue-color": "^1.6.0", diff --git a/package.json b/package.json index b11d234f..05d4476a 100644 --- a/package.json +++ b/package.json @@ -37,7 +37,7 @@ "extends @nextcloud/browserslist-config" ], "dependencies": { - "@conduction/nextcloud-vue": "^2.31.1", + "@conduction/nextcloud-vue": "^2.34.0", "@nextcloud/auth": "^2.6.0", "@nextcloud/axios": "~2.5.2", "@nextcloud/capabilities": "^1.2.1", diff --git a/src/dialogs/DecisionFormDialog.vue b/src/dialogs/DecisionFormDialog.vue index c1377f1b..3869f131 100644 --- a/src/dialogs/DecisionFormDialog.vue +++ b/src/dialogs/DecisionFormDialog.vue @@ -5,9 +5,13 @@ Dialog: schema-driven create/edit form for Decision objects, with the decisionType picker fed from the registry. - This is a manifest `form-dialog` slot replacement for the Decisions and - Motions index pages (wired via each page's `slots` map in - src/manifest.json). The built-in dialog those pages otherwise render + This is a manifest `form-dialog` slot replacement for every decidiq + surface that renders the Decision schema in a form: the Decisions and + Motions INDEX pages, and the Decision, Motion, Amendment and Decision + integrations DETAIL pages (wired via each page's `slots` map in + src/manifest.json). CnIndexPage and CnDetailPage deliberately name the + slot the same and scope it the same, so one component serves both. + The built-in dialog those pages otherwise render builds its type picker from `properties.decisionType.enum` in the stored schema — and decision-types-as-configuration (#1099) deliberately emptied that enum, making the `decision_types` app config the only @@ -30,10 +34,11 @@ @@ -43,6 +48,7 @@ import { listDecisionTypes, withDecisionTypeVocabulary, } from '../integrations/decisionLink.js' +import { settleFormDialogResult } from './formDialogResult.js' export default { name: 'DecisionFormDialog', @@ -104,5 +110,34 @@ export default { async mounted() { this.decisionTypes = await listDecisionTypes() }, + + methods: { + /** + * Save through the page's own persistence path, then hand the + * outcome back to the dialog that submitted it. + * + * The result matters because this dialog is ours, not the page's: + * CnFormDialog raises `loading` on submit and only `setResult()` + * lowers it, with `no-close` bound to `loading`. On CnDetailPage a + * failed edit leaves the form open, so dropping the result would + * strand the user in a modal that can neither retry nor close. + * CnIndexPage's `confirm` resolves to nothing and closes the dialog + * by flipping `show` instead, which settleFormDialogResult() treats + * as a normal outcome rather than a fault, so this one component + * still serves both pages. + * + * @param {object} formData The submitted form data. + * @param {?object} extra CnFormDialog's second confirm argument + * (extension answers), passed through untouched. + * + * @return {Promise} + * + * @spec openspec/changes/decision-types-as-configuration/specs/decidesk-contract-decision-hub/spec.md + */ + async onConfirm(formData, extra) { + const result = await this.confirm(formData, extra) + settleFormDialogResult(this.$refs.dialog, result) + }, + }, } diff --git a/src/dialogs/formDialogResult.js b/src/dialogs/formDialogResult.js new file mode 100644 index 00000000..dd1ed770 --- /dev/null +++ b/src/dialogs/formDialogResult.js @@ -0,0 +1,51 @@ +/** + * SPDX-FileCopyrightText: 2026 Conduction B.V. + * SPDX-License-Identifier: EUPL-1.2 + * + * Result plumbing for a `form-dialog` slot replacement. + * + * A page that replaces its built-in form dialog through the `form-dialog` + * slot renders its own `CnFormDialog`, and the host holds no ref to it. That + * makes the save result the only channel back: `CnFormDialog` raises its + * `loading` flag on submit and ONLY `setResult()` lowers it again, while + * `no-close` is bound to `loading`. A replacement that drops the result + * therefore locks its own modal open, on a successful save as well as on a + * failed one, with no error and nothing in the console. + * + * `CnDetailPage`'s slot `confirm` resolves to `{ success: true, data }` or + * `{ error }` for exactly this reason. `CnIndexPage`'s does not resolve to + * anything yet (nextcloud-vue#944 left that half alone deliberately), and it + * closes its dialog by flipping the slot's `show` instead — so this helper + * has to accept "no result" as a normal outcome rather than a fault, and one + * replacement component can serve both pages. + * + * Kept in a plain .js module because this repo's vitest runs on plain Vite + * with no @vitejs/plugin-vue, so a `.vue` file cannot be imported by a spec. + * The logic that must be tested lives here and the SFC calls it. + * + * @spec openspec/changes/decision-types-as-configuration/specs/decidesk-contract-decision-hub/spec.md + */ + +/** + * Hand a slot `confirm`'s resolved result back to the dialog that submitted it. + * + * @param {?object} dialog The CnFormDialog instance (a `$refs` entry), or null + * when the dialog has already unmounted because `show` went false. + * @param {?object} result What the slot's `confirm` resolved to: + * `{ success: true, data }` / `{ error }` on CnDetailPage, `undefined` on a + * page whose confirm returns nothing. + * + * @return {boolean} True when the dialog was settled, false when there was + * nothing to settle (no dialog, or no result to settle it with). + * + * @spec openspec/changes/decision-types-as-configuration/specs/decidesk-contract-decision-hub/spec.md + */ +export function settleFormDialogResult(dialog, result) { + // `setResult` reads `resultData.success`, so a null / undefined result + // would throw rather than close anything. A page whose confirm resolves + // to nothing closes its dialog itself; leave that path alone. + if (!result || typeof result !== 'object') return false + if (!dialog || typeof dialog.setResult !== 'function') return false + dialog.setResult(result) + return true +} diff --git a/src/manifest.json b/src/manifest.json index d899923e..709e1a00 100644 --- a/src/manifest.json +++ b/src/manifest.json @@ -640,6 +640,9 @@ "route": "/decisions/:id/integrations", "type": "detail", "title": "Decision integrations", + "slots": { + "form-dialog": "DecisionFormDialog" + }, "config": { "register": "decidiq", "schema": "decision", @@ -876,7 +879,9 @@ "id": "MotionDetail", "route": "/motions/:id", "type": "detail", + "_note": "decision-types-as-configuration: same wiring and same reason as DecisionDetail. This page renders the Decision schema too, so its built-in Edit dialog showed an empty decisionType picker until the form-dialog slot (nextcloud-vue#944) let DecisionFormDialog splice the registry vocabulary in.", "slots": { + "form-dialog": "DecisionFormDialog", "widget-motion-amendments": "MotionAmendmentsTab", "widget-motion-votes": "MotionVotesTab", "widget-motion-amendment-order": "MotionAmendmentOrderTab", @@ -960,7 +965,9 @@ }, "documentationUrl": "https://decidiq.conduction.nl" }, + "_note": "decision-types-as-configuration: same wiring and same reason as DecisionDetail. This page renders the Decision schema too, so its built-in Edit dialog showed an empty decisionType picker until the form-dialog slot (nextcloud-vue#944) let DecisionFormDialog splice the registry vocabulary in.", "slots": { + "form-dialog": "DecisionFormDialog", "widget-amend-diff": "AmendmentDiffTab", "widget-amend-parent": "AmendmentParentMotionTab" } @@ -1131,7 +1138,9 @@ "id": "DecisionDetail", "route": "/decisions/:id", "type": "detail", + "_note": "decision-types-as-configuration: this detail page's built-in Edit dialog renders the same Decision schema the index pages do, and #1099 deliberately emptied properties.decisionType.enum (the decision_types app config is the only authority), so its type picker offered nothing and the required field blocked the save. #1109 fixed the two index pages through CnIndexPage's form-dialog slot; CnDetailPage grew the same slot with the same scope in nextcloud-vue#944, so the same DecisionFormDialog is wired here rather than a second copy of the vocabulary logic. The note stays a SIBLING of slots: CnPageRenderer resolves every slots entry as a registry name and warns on one it cannot find.", "slots": { + "form-dialog": "DecisionFormDialog", "widget-decision-lifecycle": "DecisionLifecycleTab", "widget-decision-actions": "ActionItemsSurface", "widget-decision-related": "RelatedDecisionsTab", diff --git a/tests/vitest/decisionDetailFormDialog.spec.js b/tests/vitest/decisionDetailFormDialog.spec.js new file mode 100644 index 00000000..2167a1cc --- /dev/null +++ b/tests/vitest/decisionDetailFormDialog.spec.js @@ -0,0 +1,217 @@ +/** + * SPDX-FileCopyrightText: 2026 Conduction B.V. + * SPDX-License-Identifier: EUPL-1.2 + * + * Unit tests for the decision-type picker on decidiq's DETAIL pages. + * + * The defect these pin: #1099 moved the `decisionType` vocabulary out of the + * stored schema's `enum` into the `decision_types` app config, and #1109 + * spliced it back into the two INDEX pages through CnIndexPage's + * `form-dialog` slot. CnDetailPage had no such slot, so every detail page + * that renders the Decision schema kept showing an EMPTY required picker in + * its built-in Edit dialog. nextcloud-vue#944 added the slot with the same + * name and the same scope; these tests hold the four decidiq surfaces that + * needed it to the wiring, and hold the wiring itself to the two things that + * make it work: the schema copy carries the vocabulary, and the save result + * gets back to the dialog that submitted it. + * + * Component mounting is deliberately absent: this repo's vitest runs on plain + * Vite with no @vitejs/plugin-vue, so a `.vue` file cannot be imported by a + * spec (see registerDetailWidgets.spec.js). The logic therefore lives in + * importable .js modules, and the manifest wiring is asserted against + * src/manifest.json itself. + * + * @spec openspec/changes/decision-types-as-configuration/specs/decidesk-contract-decision-hub/spec.md + */ + +import { readFileSync } from 'node:fs' +import { fileURLToPath } from 'node:url' +import { describe, expect, it, vi } from 'vitest' +import { settleFormDialogResult } from '../../src/dialogs/formDialogResult.js' +import { + FALLBACK_DECISION_TYPES, + withDecisionTypeVocabulary, +} from '../../src/integrations/decisionLink.js' + +// decisionLink.js reaches @nextcloud/axios (and through it @nextcloud/auth, +// which touches `window`) at import time; these tests exercise its pure +// helpers, so the transport is stubbed rather than exercised. +vi.mock('@nextcloud/axios', () => ({ + default: { get: vi.fn(), post: vi.fn() }, +})) + +vi.mock('@nextcloud/l10n', () => ({ + translate: (app, text) => text, +})) + +/** + * Read a repo file relative to this spec. + * + * @param {string} relative Path relative to tests/vitest/. + * + * @return {string} The file contents. + */ +function read(relative) { + return readFileSync(fileURLToPath(new URL(relative, import.meta.url)), 'utf8') +} + +const manifest = JSON.parse(read('../../src/manifest.json')) +const registrySource = read('../../src/registry.js') + +/** Every page that renders the Decision schema through CnDetailPage. */ +const decisionDetailPages = manifest.pages.filter( + (page) => page.type === 'detail' && page.config?.schema === 'decision', +) + +/** + * The Decision schema exactly as the register ships it: what a detail page's + * built-in dialog renders, decisionType enum and all (there is none). + * + * @return {object} A fresh copy of the stored Decision schema. + */ +function storedDecisionSchema() { + return JSON.parse(read('../../lib/Settings/decidesk_register.json')).components + .schemas.Decision +} + +describe('the decision detail pages wire the form-dialog slot', () => { + it('covers every detail page bound to the decision schema', () => { + // Equality, not a subset: a new decision-backed detail page added + // without the wiring has exactly this defect, and the test that only + // checked the known four would stay green while it shipped. + expect(decisionDetailPages.map((page) => page.id).sort()).toEqual([ + 'AmendmentDetail', + 'DecisionDetail', + 'DecisionIntegrations', + 'MotionDetail', + ]) + }) + + it.each(decisionDetailPages.map((page) => page.id))( + '%s replaces its built-in form dialog with DecisionFormDialog', + (id) => { + const page = decisionDetailPages.find((entry) => entry.id === id) + + expect(page.slots?.['form-dialog']).toBe('DecisionFormDialog') + }, + ) + + it('the index pages keep the same wiring, so one component serves both', () => { + const indexPages = manifest.pages.filter( + (page) => page.type === 'index' && page.config?.schema === 'decision', + ) + + expect(indexPages.map((page) => page.id).sort()).toEqual([ + 'Decisions', + 'Motions', + ]) + for (const page of indexPages) { + expect(page.slots?.['form-dialog']).toBe('DecisionFormDialog') + } + }) + + it('DecisionFormDialog is registered, so the renderer can resolve the name', () => { + // CnPageRenderer resolves every `slots` value against the registry and + // only warns when it cannot: an unregistered name renders the built-in + // dialog again, which is the defect this change removes. + expect(registrySource).toContain( + "import DecisionFormDialog from './dialogs/DecisionFormDialog.vue'", + ) + expect(registrySource).toContain( + 'DecisionFormDialog: page(DecisionFormDialog)', + ) + }) +}) + +describe('the detail-page picker offers the registry vocabulary', () => { + it('renders no options at all without the wiring (negative control)', () => { + // This is what the built-in dialog renders from: the stored schema, + // whose decisionType is a free-text string by design since #1099. + const stored = storedDecisionSchema() + + expect(stored.properties.decisionType).toBeTruthy() + expect(stored.properties.decisionType.enum).toBeUndefined() + expect(stored.required).toContain('decisionType') + }) + + it('offers the registry types once the schema passes through the slot', () => { + const enriched = withDecisionTypeVocabulary(storedDecisionSchema(), [ + 'motion', + 'advice', + 'subsidie-besluit', + ]) + + expect(enriched.properties.decisionType.enum).toEqual([ + 'motion', + 'advice', + 'subsidie-besluit', + ]) + expect(enriched.properties.decisionType.enumLabels.motion).toBeTruthy() + }) + + it('offers the shipped seed while the registry has not answered', () => { + // The slot component fetches on mount, so the first open can land + // before the answer does. An empty picker there is the same defect. + expect( + withDecisionTypeVocabulary(storedDecisionSchema(), null).properties + .decisionType.enum, + ).toEqual(FALLBACK_DECISION_TYPES) + }) + + it('never mutates the store schema every other surface reads', () => { + const stored = storedDecisionSchema() + const enriched = withDecisionTypeVocabulary(stored, ['motion']) + + expect(stored.properties.decisionType.enum).toBeUndefined() + expect(enriched).not.toBe(stored) + expect(enriched.properties.decisionType.title).toBe( + stored.properties.decisionType.title, + ) + }) +}) + +describe('settleFormDialogResult (the locked-modal trap)', () => { + const fakeDialog = () => ({ setResult: vi.fn() }) + + it('closes the dialog on a successful save', () => { + const dialog = fakeDialog() + + expect( + settleFormDialogResult(dialog, { success: true, data: { id: '1' } }), + ).toBe(true) + expect(dialog.setResult).toHaveBeenCalledWith({ + success: true, + data: { id: '1' }, + }) + }) + + it('unlocks the dialog on a failed save so the user can retry or close', () => { + // CnFormDialog raises `loading` on submit and only setResult lowers + // it, with `no-close` bound to `loading`. On the error path the page + // leaves the form open, so a dropped result strands the user in a + // modal that can do neither. + const dialog = fakeDialog() + + expect(settleFormDialogResult(dialog, { error: 'Save failed' })).toBe(true) + expect(dialog.setResult).toHaveBeenCalledWith({ error: 'Save failed' }) + }) + + it('leaves a confirm that resolves to nothing alone', () => { + // CnIndexPage's confirm resolves to undefined and closes its dialog by + // flipping `show`. setResult reads `resultData.success`, so passing it + // nothing would throw where the index pages work today. + const dialog = fakeDialog() + + expect(settleFormDialogResult(dialog, undefined)).toBe(false) + expect(settleFormDialogResult(dialog, null)).toBe(false) + expect(settleFormDialogResult(dialog, 'saved')).toBe(false) + expect(dialog.setResult).not.toHaveBeenCalled() + }) + + it('survives the dialog having unmounted while the save was in flight', () => { + // A successful edit sets `show` false, which unmounts the replacement + // and empties the ref before the awaited confirm returns. + expect(() => settleFormDialogResult(null, { success: true })).not.toThrow() + expect(settleFormDialogResult({}, { success: true })).toBe(false) + }) +})