From cc7c9bb50f5ea855fdff3fdee622fc2ecb9b997f Mon Sep 17 00:00:00 2001 From: Ruben van der Linde Date: Thu, 3 Sep 2026 10:32:41 +0200 Subject: [PATCH] fix(decisions): feed every type picker from the registry, and carry the 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 --- appinfo/info.xml | 26 ++ lib/Repair/MigrateAdminGroup.php | 208 ++++++++++++++ lib/Settings/decidesk_register.json | 6 +- src/components/tabs/AgendaMotionsTab.vue | 22 +- src/components/tabs/MotionAmendmentsTab.vue | 24 +- src/dialogs/DecisionFormDialog.vue | 108 ++++++++ src/integrations/decisionLink.js | 48 ++++ src/manifest.json | 4 + src/registry.js | 12 + tests/Unit/Repair/MigrateAdminGroupTest.php | 292 ++++++++++++++++++++ tests/vitest/decisionTypes.spec.js | 62 +++++ 11 files changed, 802 insertions(+), 10 deletions(-) create mode 100644 lib/Repair/MigrateAdminGroup.php create mode 100644 src/dialogs/DecisionFormDialog.vue create mode 100644 tests/Unit/Repair/MigrateAdminGroupTest.php diff --git a/appinfo/info.xml b/appinfo/info.xml index a8105d855..edc417f34 100644 --- a/appinfo/info.xml +++ b/appinfo/info.xml @@ -288,6 +288,21 @@ Vrij en open source onder de EUPL-1.2-licentie. a fresh install has no stages to repair. --> OCA\Decidiq\Migration\RepairDecisionStageLabels + + OCA\Decidiq\Repair\MigrateAdminGroup OCA\Decidiq\Repair\SeedDecisionTypes + + OCA\Decidiq\Repair\MigrateAdminGroup diff --git a/lib/Repair/MigrateAdminGroup.php b/lib/Repair/MigrateAdminGroup.php new file mode 100644 index 000000000..598d485fd --- /dev/null +++ b/lib/Repair/MigrateAdminGroup.php @@ -0,0 +1,208 @@ + `decidiq` app-id rename. + * + * The register configuration's authorization baseline names an + * administrators group for the griffie/secretariat that must edit records it + * did not create, and OpenRegister's GroupProvisioner CREATES every group an + * authorization block names during the register import. Until this change the + * block named `decidesk-administrators` — the pre-rename app id — so every + * existing install holds a group under the OLD id, with real memberships an + * administrator granted by hand. + * + * WHY MIGRATE RATHER THAN RENAME. A Nextcloud group id (gid) is the primary + * key of the group across every backend; `IGroupManager` exposes create and + * delete but no rename, and `IGroup::setDisplayName()` changes the label, not + * the id the authorization rows match on. The only rename available would be + * delete-and-recreate, which destroys memberships and shares. So this step + * CREATES `decidiq-administrators` (when the import's GroupProvisioner has not + * already) and COPIES every member of the old group into it. + * + * WHY THE OLD GROUP STAYS, HONORED. The authorization arrays in + * `lib/Settings/decidesk_register.json` now name BOTH groups, new id first. + * The old group is never deleted and never has a member removed, for the same + * reason the other rename legs keep their old rows: a rollback still finds + * them, and an install where this step could not copy a member (an LDAP-backed + * read-only group backend refuses local writes) keeps working through the old + * name. The old name can only be retired once no install's group backend + * depends on it — a decision about data, not a migration. + * + * SAFETY. Idempotent and non-destructive: + * - when the old group does not exist there is nothing to migrate and the + * step is a no-op — a fresh install gets the NEW group provisioned empty + * by the register import, preserving the documented admin-provisioned, + * fail-closed posture (no group means owner-plus-admin only); + * - a member already in the new group is skipped, so a second run is a + * no-op; + * - every failure is logged and the loop continues. This step is registered + * under ``, where a throwing repair step means the app never + * enables at all — one uncopyable membership is not worth that. + * + * Registered under BOTH `` and `` in + * `appinfo/info.xml`, after InitializeSettings — the import that step + * triggers provisions the new group, so this step normally only copies + * members (and creates the group itself only when the import could not). + * + * @category Repair + * @package OCA\Decidiq\Repair + * + * @author Conduction Development Team + * @copyright 2026 Conduction B.V. + * @license EUPL-1.2 https://joinup.ec.europa.eu/collection/eupl/eupl-text-eupl-12 + * + * SPDX-FileCopyrightText: 2026 Conduction B.V. . + * SPDX-License-Identifier: EUPL-1.2 + * + * @version GIT: + * + * @link https://conduction.nl + */ + +declare(strict_types=1); + +namespace OCA\Decidiq\Repair; + +use OCP\IGroupManager; +use OCP\Migration\IOutput; +use OCP\Migration\IRepairStep; +use Psr\Log\LoggerInterface; +use Throwable; + +/** + * Copy the decidesk-administrators group's members to decidiq-administrators. + * + * @spec exclude No canonical spec covers the `decidesk` -> `decidiq` + * administrators-group migration. Pointing this at an existing spec would + * report conformance to a requirement that says nothing about it. + */ +class MigrateAdminGroup implements IRepairStep { + + /** + * The administrators group id under the OLD app id. + * + * Deliberately still `decidesk`: this constant and the honored entry in + * `lib/Settings/decidesk_register.json` are the places that are supposed + * to keep saying it. + * + * @var string + */ + private const OLD_GROUP_ID = 'decidesk-administrators'; + + /** + * The canonical administrators group id under the current app id. + * + * @var string + */ + private const NEW_GROUP_ID = 'decidiq-administrators'; + + /** + * Constructor for MigrateAdminGroup. + * + * @param IGroupManager $groupManager Group existence, creation and membership. + * @param LoggerInterface $logger Logger for members that fail to copy. + * + * @return void + */ + public function __construct( + private readonly IGroupManager $groupManager, + private readonly LoggerInterface $logger, + ) { + + }//end __construct() + + /** + * The repair step name. + * + * @return string + * + * @spec exclude One-off decidesk->decidiq app-id rename plumbing: it + * mirrors group memberships between the old-id and new-id + * administrators groups and adds no behaviour of its own. + */ + public function getName(): string { + return 'Copy the Decidiq administrators group from the decidesk group id'; + }//end getName() + + /** + * Ensure decidiq-administrators exists and carries every member of the + * old-id group. + * + * @param IOutput $output Repair output channel. + * + * @return void + * + * @spec exclude One-off decidesk->decidiq app-id rename plumbing: it + * mirrors group memberships between the old-id and new-id + * administrators groups and adds no behaviour of its own. The + * authorization the groups feed is specified in + * lib/Settings/decidesk_register.json's register-level baseline. + */ + public function run(IOutput $output): void { + try { + if ($this->groupManager->groupExists(self::OLD_GROUP_ID) === false) { + // Nothing to migrate. The register import provisions the new + // group (empty) from the authorization declaration; creating + // it here too would only duplicate that work. + return; + } + + $oldGroup = $this->groupManager->get(self::OLD_GROUP_ID); + if ($oldGroup === null) { + return; + } + + if ($this->groupManager->groupExists(self::NEW_GROUP_ID) === false) { + $this->groupManager->createGroup(self::NEW_GROUP_ID); + } + + $newGroup = $this->groupManager->get(self::NEW_GROUP_ID); + if ($newGroup === null) { + $this->logger->warning( + 'Decidiq: could not resolve the new administrators group; ' + . 'the old-id group stays honored by the authorization baseline', + ['group' => self::NEW_GROUP_ID] + ); + return; + } + + $copied = 0; + foreach ($oldGroup->getUsers() as $user) { + try { + if ($newGroup->inGroup($user) === true) { + continue; + } + + $newGroup->addUser($user); + $copied++; + } catch (Throwable $e) { + $this->logger->warning( + 'Decidiq: could not copy one administrators-group member; ' + . 'the member keeps access through the old-id group', + ['user' => $user->getUID(), 'exception' => $e->getMessage()] + ); + } + }//end foreach + + $output->info( + sprintf( + 'Decidiq administrators group: copied %d member(s) from %s to %s', + $copied, + self::OLD_GROUP_ID, + self::NEW_GROUP_ID + ) + ); + } catch (Throwable $e) { + // A repair step registered under that throws stops the + // app enabling entirely; a group backend hiccup is not worth that. + $this->logger->error( + 'Decidiq: administrators-group migration failed; ' + . 'members keep access through the old-id group', + ['exception' => $e] + ); + }//end try + }//end run() +}//end class diff --git a/lib/Settings/decidesk_register.json b/lib/Settings/decidesk_register.json index 59482538b..d39233988 100644 --- a/lib/Settings/decidesk_register.json +++ b/lib/Settings/decidesk_register.json @@ -57,8 +57,8 @@ "slug": "decidiq", "title": "Decidiq Register", "description": "Universal decision-making platform for governance bodies, associations, corporate boards, and operational meetings. Holds every governance body, meeting, agenda item, decision, vote, minutes, consultation and integrity object the app reads and writes.", - "version": "0.12.0", - "_authorizationNote": "REGISTER-LEVEL BASELINE, and it is a BEHAVIOUR CHANGE. Until this block existed, every decidesk schema without its own `authorization` block was read AND WRITE open to every authenticated user through OpenRegister's own /apps/openregister/api/objects/decidiq/ API — the API the frontend uses directly under ADR-022, which no decidesk controller guard sits in front of. OpenRegister's PermissionHandler::hasGroupPermission() tests `empty($authorization)`, and PHP's empty() is true for null and [] alike, so an ABSENT block takes the same default-OPEN branch as an empty one; `enforce_default_closed` defaults to false and, even when enabled, closes only create/update/delete. Measured on this tree before the change: 93 schemas, 24 carrying a block (all read-only), this register row carrying none — so 69 schemas, including Decision, VotingRound, Vote, Participant and EngagementRecord, granted create/update/delete to any logged-in account. Same shape as docudesk#631, where a plain user overwrote another user's template. THE CASCADE IS WHY THIS SITS ON THE REGISTER ROW: PermissionHandler::resolveAuthorization() uses a schema's own block when it has one and falls back to the register's only when it does not, so this baseline reaches exactly the 69 unprotected schemas and changes nothing for the 24 that already declare their own (their public-read rules are untouched). WHAT EACH ACTION IS FOR: read and list name BOTH `authenticated` AND `public`, and create names `authenticated`, so no read goes dark and any member can still raise a decision, cast a vote or file a reaction — those flows are unchanged. `public` on the READ actions is not a widening: before this block existed the default-OPEN branch granted every action to every principal INCLUDING the anonymous one, so omitting `public` here would have CLOSED anonymous reads — a separate and much larger policy change than the write hole this block exists to fix, and one that would 403 every #[PublicPage] citizen-participation surface. CI proved it rather than theory: the first version of this block omitted `public` and all six PHPUnit legs failed with `User 'Anonymous' does not have permission to 'read' objects in schema 'Meeting'`. `public` appears on NO write action, so the anonymous fail-closed rule from openregister#1955 keeps denying anonymous create/update/delete exactly as it did before. update/delete are NOT granted to `authenticated`, which is the whole fix: OpenRegister bypasses the object OWNER unconditionally and SQL-side before any rule is consulted, and bypasses `admin` too, so the author of an object keeps full control of it and only OTHER users lose the ability to rewrite or destroy it. `decidesk-administrators` is the named Nextcloud group for a griffie/secretariat that must edit records it did not create; it is admin-provisioned, and on an instance where it does not exist this simply resolves to owner-plus-admin, which is fail-closed. EVERY ACTION IS WRITTEN OUT ON PURPOSE: once a block is non-empty, OpenRegister DENIES any action the block omits, so an omitted `list` or `create` here would silently break the app rather than secure it. Per-body actor authorization (chair / signatory scopes, x-decidesk-rbac-scopes above) is a separate layer and is unaffected — OpenRegister cannot template a group name per object, which is why that determination is made at the app boundary.", + "version": "0.13.0", + "_authorizationNote": "REGISTER-LEVEL BASELINE, and it is a BEHAVIOUR CHANGE. Until this block existed, every decidesk schema without its own `authorization` block was read AND WRITE open to every authenticated user through OpenRegister's own /apps/openregister/api/objects/decidiq/ API — the API the frontend uses directly under ADR-022, which no decidesk controller guard sits in front of. OpenRegister's PermissionHandler::hasGroupPermission() tests `empty($authorization)`, and PHP's empty() is true for null and [] alike, so an ABSENT block takes the same default-OPEN branch as an empty one; `enforce_default_closed` defaults to false and, even when enabled, closes only create/update/delete. Measured on this tree before the change: 93 schemas, 24 carrying a block (all read-only), this register row carrying none — so 69 schemas, including Decision, VotingRound, Vote, Participant and EngagementRecord, granted create/update/delete to any logged-in account. Same shape as docudesk#631, where a plain user overwrote another user's template. THE CASCADE IS WHY THIS SITS ON THE REGISTER ROW: PermissionHandler::resolveAuthorization() uses a schema's own block when it has one and falls back to the register's only when it does not, so this baseline reaches exactly the 69 unprotected schemas and changes nothing for the 24 that already declare their own (their public-read rules are untouched). WHAT EACH ACTION IS FOR: read and list name BOTH `authenticated` AND `public`, and create names `authenticated`, so no read goes dark and any member can still raise a decision, cast a vote or file a reaction — those flows are unchanged. `public` on the READ actions is not a widening: before this block existed the default-OPEN branch granted every action to every principal INCLUDING the anonymous one, so omitting `public` here would have CLOSED anonymous reads — a separate and much larger policy change than the write hole this block exists to fix, and one that would 403 every #[PublicPage] citizen-participation surface. CI proved it rather than theory: the first version of this block omitted `public` and all six PHPUnit legs failed with `User 'Anonymous' does not have permission to 'read' objects in schema 'Meeting'`. `public` appears on NO write action, so the anonymous fail-closed rule from openregister#1955 keeps denying anonymous create/update/delete exactly as it did before. update/delete are NOT granted to `authenticated`, which is the whole fix: OpenRegister bypasses the object OWNER unconditionally and SQL-side before any rule is consulted, and bypasses `admin` too, so the author of an object keeps full control of it and only OTHER users lose the ability to rewrite or destroy it. `decidiq-administrators` is the named Nextcloud group for a griffie/secretariat that must edit records it did not create. It was first shipped as `decidesk-administrators` — the pre-rename app id — and OpenRegister's GroupProvisioner creates every group an authorization block names, so existing installs hold that old-id group with real memberships. BOTH names therefore stay in the arrays: dropping the old one would strip access from every member the moment this row re-imports, before the MigrateAdminGroup repair step has copied anyone (and on an install where the repair cannot add a member — an LDAP-backed read-only group backend — forever). MigrateAdminGroup copies members old-to-new; the old group is never deleted and never has members removed, matching the other rename legs. On an instance where neither group exists this resolves to owner-plus-admin, which is fail-closed. EVERY ACTION IS WRITTEN OUT ON PURPOSE: once a block is non-empty, OpenRegister DENIES any action the block omits, so an omitted `list` or `create` here would silently break the app rather than secure it. Per-body actor authorization (chair / signatory scopes, x-decidesk-rbac-scopes above) is a separate layer and is unaffected — OpenRegister cannot template a group name per object, which is why that determination is made at the app boundary.", "authorization": { "read": [ "authenticated", @@ -72,9 +72,11 @@ "authenticated" ], "update": [ + "decidiq-administrators", "decidesk-administrators" ], "delete": [ + "decidiq-administrators", "decidesk-administrators" ] }, diff --git a/src/components/tabs/AgendaMotionsTab.vue b/src/components/tabs/AgendaMotionsTab.vue index 7c9c12728..245c09d24 100644 --- a/src/components/tabs/AgendaMotionsTab.vue +++ b/src/components/tabs/AgendaMotionsTab.vue @@ -159,9 +159,18 @@ export default { ] }, - /** @spec openspec/specs/relation-tab-ui/spec.md */ + /** + * `decisionType` is excluded because it is this tab's DISCRIMINATOR, + * not a choice: every row here is a Decision of decisionType=motion + * (the fetch filters on it) and onConfirm writes the value itself. + * Leaving it on the form showed an EMPTY required picker — + * decision-types-as-configuration (#1099) dropped the enum from the + * stored schema, so the select had no options and blocked the save. + * + * @spec openspec/specs/relation-tab-ui/spec.md + */ excludedFields() { - return ['id', 'uuid', 'agendaItem', 'created', 'updated'] + return ['id', 'uuid', 'agendaItem', 'decisionType', 'created', 'updated'] }, }, @@ -209,7 +218,7 @@ export default { }, /** - * @param row + * @param row The decision row to edit. * @spec openspec/specs/relation-tab-ui/spec.md */ async openEdit(row) { @@ -221,14 +230,19 @@ export default { }, /** - * @param formData + * @param formData The submitted form values. * @spec openspec/specs/relation-tab-ui/spec.md */ async onConfirm(formData) { const store = ensureRelationType('motion') try { + // The discriminator is written HERE, not picked on the form — + // see excludedFields. Without this, a create carried no + // decisionType at all and the row fell out of this tab's own + // decisionType=motion filter on the next refresh. await store.saveObject('motion', { ...formData, + decisionType: 'motion', agendaItem: this.objectId, }) this.$refs.formDialog?.setResult({ success: true }) diff --git a/src/components/tabs/MotionAmendmentsTab.vue b/src/components/tabs/MotionAmendmentsTab.vue index adca45615..e7746b7fe 100644 --- a/src/components/tabs/MotionAmendmentsTab.vue +++ b/src/components/tabs/MotionAmendmentsTab.vue @@ -163,9 +163,25 @@ export default { ] }, - /** @spec openspec/specs/relation-tab-ui/spec.md */ + /** + * `decisionType` is excluded because it is this tab's DISCRIMINATOR, + * not a choice: every row here is a Decision of decisionType=amendment + * and onConfirm already writes the value itself. Leaving it on the + * form showed an EMPTY required picker — decision-types-as-configuration + * (#1099) dropped the enum from the stored schema, so the select had + * no options and blocked the save. + * + * @spec openspec/specs/relation-tab-ui/spec.md + */ excludedFields() { - return ['id', 'uuid', 'parentMotion', 'created', 'updated'] + return [ + 'id', + 'uuid', + 'parentMotion', + 'decisionType', + 'created', + 'updated', + ] }, }, @@ -213,7 +229,7 @@ export default { }, /** - * @param row + * @param row The decision row to edit. * @spec openspec/specs/relation-tab-ui/spec.md */ async openEdit(row) { @@ -225,7 +241,7 @@ export default { }, /** - * @param formData + * @param formData The submitted form values. * @spec openspec/specs/relation-tab-ui/spec.md */ async onConfirm(formData) { diff --git a/src/dialogs/DecisionFormDialog.vue b/src/dialogs/DecisionFormDialog.vue new file mode 100644 index 000000000..c1377f1b4 --- /dev/null +++ b/src/dialogs/DecisionFormDialog.vue @@ -0,0 +1,108 @@ + + + + + + + diff --git a/src/integrations/decisionLink.js b/src/integrations/decisionLink.js index 77b2dabf7..3a6fd5c37 100644 --- a/src/integrations/decisionLink.js +++ b/src/integrations/decisionLink.js @@ -232,6 +232,54 @@ export function proposalFormSchema(types) { } } +/** + * Inject the registry's decisionType vocabulary into an OpenRegister + * decision schema. + * + * decision-types-as-configuration (#1099) deliberately dropped the `enum` + * from the stored schema declaration — the `decision_types` app config is + * the only authority. That left every schema-driven form (the built-in + * create/edit dialog on the Decisions and Motions index pages) rendering an + * empty type picker: the select widget reads `properties.decisionType.enum` + * and found nothing. This helper closes the gap the same way the cross-app + * pickers were closed in #1104: the vocabulary comes from + * {@link listDecisionTypes} (registry endpoint, seed fallback) and gets + * spliced into the schema right before the form renders. The schema on the + * SERVER stays enum-free; only the client-side copy driving the picker is + * enriched. + * + * @param {object} schema The OpenRegister decision schema (as handed to the + * form dialog). + * @param {?string[]} types The registry vocabulary, or null while it loads — + * the shipped seed fills in. + * + * @return {object} A shallow clone with `properties.decisionType` carrying + * the vocabulary as `enum` + translated `enumLabels`, or + * the input untouched when it has no decisionType property. + * + * @spec openspec/changes/decision-types-as-configuration/specs/decidesk-contract-decision-hub/spec.md + */ +export function withDecisionTypeVocabulary(schema, types) { + if (!schema || typeof schema !== 'object') return schema + const properties = schema.properties + if (!properties || typeof properties !== 'object' || !properties.decisionType) { + return schema + } + const offered = + Array.isArray(types) && types.length > 0 ? types : FALLBACK_DECISION_TYPES + return { + ...schema, + properties: { + ...properties, + decisionType: { + ...properties.decisionType, + enum: offered, + enumLabels: decisionTypeLabels(), + }, + }, + } +} + /** * Classify a decision into one of the three presentation buckets. * diff --git a/src/manifest.json b/src/manifest.json index 9d1747ad3..d899923ed 100644 --- a/src/manifest.json +++ b/src/manifest.json @@ -831,6 +831,8 @@ "route": "/motions", "type": "index", "title": "Motions", + "slots": { "form-dialog": "DecisionFormDialog" }, + "_note": "decision-types-as-configuration: the built-in create/edit dialog reads properties.decisionType.enum from the stored schema, which is deliberately empty since #1099 (the decision_types app config is the only authority), so its type picker rendered 'No results'. DecisionFormDialog is the same CnFormDialog over the same schema, with the vocabulary fetched from GET /api/v1/decision-types through the shared decisionLink.js helpers (#1104) and spliced in client-side. The note stays a SIBLING of slots: CnPageRenderer resolves every slots entry as a registry name and warns on one it cannot find.", "config": { "register": "decidiq", "schema": "decision", @@ -1030,6 +1032,8 @@ "route": "/decisions", "type": "index", "title": "Decisions", + "slots": { "form-dialog": "DecisionFormDialog" }, + "_note": "Same wiring as the Motions page, and for the same reason: the stored decision schema carries no decisionType enum since #1099, so the built-in dialog's type picker rendered 'No results'. DecisionFormDialog feeds the picker from the decision_types registry via the shared decisionLink.js helpers (#1104).", "config": { "register": "decidiq", "schema": "decision", diff --git a/src/registry.js b/src/registry.js index 8d8ba6986..a449be5d2 100644 --- a/src/registry.js +++ b/src/registry.js @@ -64,6 +64,7 @@ import MotionAmendmentsTab from './components/tabs/MotionAmendmentsTab.vue' import MotionVotesTab from './components/tabs/MotionVotesTab.vue' import MotionVotingRoundTab from './components/tabs/MotionVotingRoundTab.vue' import RelatedDecisionsTab from './components/tabs/RelatedDecisionsTab.vue' +import DecisionFormDialog from './dialogs/DecisionFormDialog.vue' import ActiveDecisionsKpiWidget from './views/dashboard/widgets/ActiveDecisionsKpiWidget.vue' // Dashboard v2 widgets (decidesk-dashboard-v2-widgets). Bespoke CnDashboardPage // slot components registered under kind: "widget". @@ -244,6 +245,17 @@ export default { // table tab. The manifest decision action-items tab points here. ActionItemsSurface: page(ActionItemsSurface), + // --- Form-dialog slot replacement (decision-types-as-configuration). --- + // Mounted into CnIndexPage's `form-dialog` slot on the Decisions and + // Motions pages (each page's `slots` map). Same CnFormDialog over the + // same schema as the built-in dialog, except the decisionType picker is + // fed from the `decision_types` registry via the shared decisionLink.js + // helpers (#1104) — the stored schema deliberately carries no enum since + // #1099, so the built-in picker rendered "No results". Registered with + // page() because slot resolution (resolveRegistryName) ignores `kind`; + // what matters is that the kind is one CnAppRoot's validator knows. + DecisionFormDialog: page(DecisionFormDialog), + // --- Dashboard v2 widgets (decidesk-dashboard-v2-widgets). --- // Eleven CnDashboardPage slot components. CnPageRenderer / CnWidgetGrid // resolve these by name once decidesk-dashboard-v2-layout references them diff --git a/tests/Unit/Repair/MigrateAdminGroupTest.php b/tests/Unit/Repair/MigrateAdminGroupTest.php new file mode 100644 index 000000000..89862d4bd --- /dev/null +++ b/tests/Unit/Repair/MigrateAdminGroupTest.php @@ -0,0 +1,292 @@ +, where an + * escaping exception stops the app enabling at all. + * + * @category Test + * @package OCA\Decidiq\Tests\Unit\Repair + * + * @author Conduction Development Team + * @copyright 2026 Conduction B.V. + * @license EUPL-1.2 https://joinup.ec.europa.eu/collection/eupl/eupl-text-eupl-12 + * + * @link https://conduction.nl + * + * SPDX-FileCopyrightText: 2026 Conduction B.V. + * SPDX-License-Identifier: EUPL-1.2 + */ + +declare(strict_types=1); + +namespace OCA\Decidiq\Tests\Unit\Repair; + +use OCA\Decidiq\Repair\MigrateAdminGroup; +use OCP\IGroup; +use OCP\IGroupManager; +use OCP\IUser; +use OCP\Migration\IOutput; +use PHPUnit\Framework\TestCase; +use Psr\Log\LoggerInterface; +use RuntimeException; + +/** + * Tests for MigrateAdminGroup. + * + * @covers \OCA\Decidiq\Repair\MigrateAdminGroup + */ +class MigrateAdminGroupTest extends TestCase { + + /** + * The administrators group id under the old app id. + * + * @var string + */ + private const OLD_GROUP = 'decidesk-administrators'; + + /** + * The administrators group id under the current app id. + * + * @var string + */ + private const NEW_GROUP = 'decidiq-administrators'; + + /** + * Build a user double. + * + * @param string $uid The user id. + * + * @return IUser + */ + private function user(string $uid): IUser { + $user = $this->createMock(originalClassName: IUser::class); + $user->method('getUID')->willReturn($uid); + return $user; + }//end user() + + /** + * The step must do nothing at all when the old-id group does not exist. + * + * A fresh install must keep the documented admin-provisioned posture: the + * register import provisions the new group, and this step creating one + * would duplicate that work. + * + * @return void + */ + public function testNoOldGroupMeansNoWork(): void { + $groupManager = $this->createMock(originalClassName: IGroupManager::class); + $groupManager->method('groupExists')->willReturn(false); + $groupManager->expects($this->never())->method('createGroup'); + $groupManager->expects($this->never())->method('get'); + + $output = $this->createMock(originalClassName: IOutput::class); + $output->expects($this->never())->method('info'); + + $step = new MigrateAdminGroup( + groupManager: $groupManager, + logger: $this->createMock(originalClassName: LoggerInterface::class), + ); + $step->run(output: $output); + }//end testNoOldGroupMeansNoWork() + + /** + * Members of the old group are copied into a newly created new group. + * + * @return void + */ + public function testCreatesNewGroupAndCopiesMembers(): void { + $alice = $this->user(uid: 'alice'); + $bob = $this->user(uid: 'bob'); + + $oldGroup = $this->createMock(originalClassName: IGroup::class); + $oldGroup->method('getUsers')->willReturn([$alice, $bob]); + $oldGroup->expects($this->never())->method('removeUser'); + + $newGroup = $this->createMock(originalClassName: IGroup::class); + $newGroup->method('inGroup')->willReturn(false); + $added = []; + $newGroup->method('addUser')->willReturnCallback( + function (IUser $user) use (&$added): void { + $added[] = $user->getUID(); + } + ); + + $groupManager = $this->createMock(originalClassName: IGroupManager::class); + $groupManager->method('groupExists')->willReturnMap( + [ + [self::OLD_GROUP, true], + [self::NEW_GROUP, false], + ] + ); + $groupManager->expects($this->once()) + ->method('createGroup') + ->with(self::NEW_GROUP); + $groupManager->method('get')->willReturnMap( + [ + [self::OLD_GROUP, $oldGroup], + [self::NEW_GROUP, $newGroup], + ] + ); + + $output = $this->createMock(originalClassName: IOutput::class); + $output->expects($this->once())->method('info') + ->with($this->stringContains(string: 'copied 2 member(s)')); + + $step = new MigrateAdminGroup( + groupManager: $groupManager, + logger: $this->createMock(originalClassName: LoggerInterface::class), + ); + $step->run(output: $output); + + $this->assertSame(expected: ['alice', 'bob'], actual: $added); + }//end testCreatesNewGroupAndCopiesMembers() + + /** + * A member already present in the new group is skipped, so a re-run + * copies nothing. + * + * @return void + */ + public function testRerunCopiesNothing(): void { + $alice = $this->user(uid: 'alice'); + + $oldGroup = $this->createMock(originalClassName: IGroup::class); + $oldGroup->method('getUsers')->willReturn([$alice]); + + $newGroup = $this->createMock(originalClassName: IGroup::class); + $newGroup->method('inGroup')->willReturn(true); + $newGroup->expects($this->never())->method('addUser'); + + $groupManager = $this->createMock(originalClassName: IGroupManager::class); + $groupManager->method('groupExists')->willReturn(true); + $groupManager->expects($this->never())->method('createGroup'); + $groupManager->method('get')->willReturnMap( + [ + [self::OLD_GROUP, $oldGroup], + [self::NEW_GROUP, $newGroup], + ] + ); + + $output = $this->createMock(originalClassName: IOutput::class); + $output->expects($this->once())->method('info') + ->with($this->stringContains(string: 'copied 0 member(s)')); + + $step = new MigrateAdminGroup( + groupManager: $groupManager, + logger: $this->createMock(originalClassName: LoggerInterface::class), + ); + $step->run(output: $output); + }//end testRerunCopiesNothing() + + /** + * One member whose write fails is logged and the rest still copy — and + * nothing escapes the step. + * + * @return void + */ + public function testOneFailingMemberDoesNotAbortTheRest(): void { + $alice = $this->user(uid: 'alice'); + $bob = $this->user(uid: 'bob'); + + $oldGroup = $this->createMock(originalClassName: IGroup::class); + $oldGroup->method('getUsers')->willReturn([$alice, $bob]); + + $newGroup = $this->createMock(originalClassName: IGroup::class); + $newGroup->method('inGroup')->willReturn(false); + $added = []; + $newGroup->method('addUser')->willReturnCallback( + function (IUser $user) use (&$added): void { + if ($user->getUID() === 'alice') { + throw new RuntimeException(message: 'read-only backend'); + } + + $added[] = $user->getUID(); + } + ); + + $groupManager = $this->createMock(originalClassName: IGroupManager::class); + $groupManager->method('groupExists')->willReturn(true); + $groupManager->method('get')->willReturnMap( + [ + [self::OLD_GROUP, $oldGroup], + [self::NEW_GROUP, $newGroup], + ] + ); + + $logger = $this->createMock(originalClassName: LoggerInterface::class); + $logger->expects($this->once())->method('warning'); + + $output = $this->createMock(originalClassName: IOutput::class); + $output->expects($this->once())->method('info') + ->with($this->stringContains(string: 'copied 1 member(s)')); + + $step = new MigrateAdminGroup(groupManager: $groupManager, logger: $logger); + $step->run(output: $output); + + $this->assertSame(expected: ['bob'], actual: $added); + }//end testOneFailingMemberDoesNotAbortTheRest() + + /** + * An unresolvable new group is logged, not thrown — the old-id group + * stays honored by the authorization baseline. + * + * @return void + */ + public function testUnresolvableNewGroupLogsAndReturns(): void { + $oldGroup = $this->createMock(originalClassName: IGroup::class); + $oldGroup->expects($this->never())->method('getUsers'); + + $groupManager = $this->createMock(originalClassName: IGroupManager::class); + $groupManager->method('groupExists')->willReturnMap( + [ + [self::OLD_GROUP, true], + [self::NEW_GROUP, false], + ] + ); + $groupManager->method('get')->willReturnMap( + [ + [self::OLD_GROUP, $oldGroup], + [self::NEW_GROUP, null], + ] + ); + + $logger = $this->createMock(originalClassName: LoggerInterface::class); + $logger->expects($this->once())->method('warning'); + + $output = $this->createMock(originalClassName: IOutput::class); + $output->expects($this->never())->method('info'); + + $step = new MigrateAdminGroup(groupManager: $groupManager, logger: $logger); + $step->run(output: $output); + }//end testUnresolvableNewGroupLogsAndReturns() + + /** + * The step names itself. + * + * @return void + */ + public function testGetName(): void { + $step = new MigrateAdminGroup( + groupManager: $this->createMock(originalClassName: IGroupManager::class), + logger: $this->createMock(originalClassName: LoggerInterface::class), + ); + + $this->assertNotSame(expected: '', actual: $step->getName()); + }//end testGetName() +}//end class diff --git a/tests/vitest/decisionTypes.spec.js b/tests/vitest/decisionTypes.spec.js index 7f82f08de..890816e16 100644 --- a/tests/vitest/decisionTypes.spec.js +++ b/tests/vitest/decisionTypes.spec.js @@ -19,6 +19,7 @@ import { FALLBACK_DECISION_TYPES, listDecisionTypes, proposalFormSchema, + withDecisionTypeVocabulary, } from '../../src/integrations/decisionLink.js' const get = vi.fn() @@ -131,3 +132,64 @@ describe('proposalFormSchema', () => { } }) }) + +// The defect this pins: decidiq's OWN "Add Decision" dialog (the built-in +// index-page form on the Decisions and Motions pages) reads the STORED +// schema's decisionType enum, which #1099 deliberately emptied — so the +// picker showed "No results" while the cross-app pickers listed 14 types. +// The manifest wires DecisionFormDialog into those pages' form-dialog slot, +// and that wrapper enriches the schema through this helper. +describe('withDecisionTypeVocabulary', () => { + const enumlessSchema = () => ({ + title: 'Decision', + properties: { + title: { type: 'string', title: 'Title' }, + decisionType: { type: 'string', title: 'Decision type' }, + }, + required: ['title', 'text', 'decisionType'], + }) + + it('splices the registry vocabulary into an enum-less schema', () => { + const enriched = withDecisionTypeVocabulary(enumlessSchema(), [ + 'motion', + 'advice', + 'subsidie-besluit', + ]) + + expect(enriched.properties.decisionType.enum).toEqual([ + 'motion', + 'advice', + 'subsidie-besluit', + ]) + expect(enriched.properties.decisionType.enumLabels.motion).toBeTruthy() + // Everything else is preserved untouched. + expect(enriched.properties.decisionType.title).toBe('Decision type') + expect(enriched.properties.title).toEqual(enumlessSchema().properties.title) + expect(enriched.required).toEqual(enumlessSchema().required) + }) + + it('never mutates the input schema', () => { + const schema = enumlessSchema() + withDecisionTypeVocabulary(schema, ['motion']) + + expect(schema.properties.decisionType.enum).toBeUndefined() + }) + + it('falls back to the shipped seed while the registry has not answered', () => { + expect( + withDecisionTypeVocabulary(enumlessSchema(), null).properties + .decisionType.enum, + ).toEqual(FALLBACK_DECISION_TYPES) + expect( + withDecisionTypeVocabulary(enumlessSchema(), []).properties.decisionType + .enum, + ).toEqual(FALLBACK_DECISION_TYPES) + }) + + it('hands back a schema without a decisionType property untouched', () => { + const other = { properties: { name: { type: 'string' } } } + + expect(withDecisionTypeVocabulary(other, ['motion'])).toBe(other) + expect(withDecisionTypeVocabulary(null, ['motion'])).toBe(null) + }) +})