From 4e93286e1591ac9b925f3b5965398aa6ddc9261a Mon Sep 17 00:00:00 2001 From: Conduction Release Bot Date: Tue, 1 Sep 2026 12:35:24 +0200 Subject: [PATCH] fix(reviews): declare auteur and status, so they are not dropped on save MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Two catalog-ratings specs and one suite-wizard spec, all red on development. The first two share one cause and it is a real data-loss bug, not a test problem. ## Reviews were stored with no author and no moderation status reviews index: expected /admin/i, received "Review columns …—6—" reviews: expected status "pending", received "" ReviewService does set both. Line 231 stamps `auteur` from the authenticated session, line 232 forces `status` to STATUS_PENDING, and the docblock at the top of the file explains at length that a client-supplied value is stripped and re-derived. The schema never declared either property. `software-review` in lib/Settings/softwarecatalogus_register.json had eight properties and neither `auteur` nor `status` was among them, so OpenRegister discarded both on every save. Silently: the write succeeds, the object comes back, the two fields are simply not there. The em-dash in the failing row IS the bug rendered. That column had nothing to show. ⚠️ This is worse than an empty column. The public read rule only ever matches status approved, and ModerationService transitions a review out of pending. With `status` unstored, a review has no state to moderate, so the whole moderation path was inert. The mock register HAD both all along, which is why this never showed up in unit tests. The two definitions are now byte-identical, checked. ## suite-wizard asserted a hash URL against a path route Expected pattern: /#\\/suites\\/[^/]+$/ Received string: ".../apps/stackiq/suites/d2f67fa0-…" The shell navigated to the correct detail page for the newly created suite. Only the "#" was missing, so the pattern waited 30s while the browser sat on the right page. It now accepts either shape; the uuid is what proves the suite was created. --- lib/Settings/softwarecatalogus_register.json | 24 ++++++++++++++++++++ tests/e2e/spec-coverage/suite-wizard.spec.ts | 7 +++++- 2 files changed, 30 insertions(+), 1 deletion(-) diff --git a/lib/Settings/softwarecatalogus_register.json b/lib/Settings/softwarecatalogus_register.json index 65402601..d0204592 100644 --- a/lib/Settings/softwarecatalogus_register.json +++ b/lib/Settings/softwarecatalogus_register.json @@ -4064,6 +4064,30 @@ "handling": "related-object" }, "$ref": "#/components/schemas/usage" + }, + "auteur": { + "type": "string", + "description": "Display name of the submitting user. Stamped server-side by ReviewService from the authenticated Nextcloud session at submission time — a client-supplied value is always discarded and never persisted.", + "visible": true, + "facetable": false, + "title": "Auteur", + "order": 10, + "example": "Bijvoorbeeld: Jan Jansen" + }, + "status": { + "type": "string", + "enum": [ + "pending", + "approved", + "rejected" + ], + "default": "pending", + "description": "Moderation status. Every new review is forced to 'pending' server-side by ReviewService regardless of client input; only an admin approval/rejection decision (ModerationService, reusing the organisatie moderation pattern) may transition it. The public RBAC read rule below only ever matches 'approved'.", + "visible": true, + "facetable": true, + "title": "Status", + "order": 11, + "example": "Bijvoorbeeld: pending" } }, "archive": [], diff --git a/tests/e2e/spec-coverage/suite-wizard.spec.ts b/tests/e2e/spec-coverage/suite-wizard.spec.ts index 56f3c99f..e70e8eec 100644 --- a/tests/e2e/spec-coverage/suite-wizard.spec.ts +++ b/tests/e2e/spec-coverage/suite-wizard.spec.ts @@ -316,7 +316,12 @@ test('suite wizard: submit creates the suite with both attached modules', async // What a user DOES see on success is the navigation to the new suite's // detail page, so that is asserted instead — a real, observable outcome // rather than a weakened one. - await expect(page).toHaveURL(/#\/suites\/[^/]+$/, { timeout: 30000 }) + // Matches the hash form AND the path form. The shell navigates to + // /apps/stackiq/suites/ here, with no "#", so a hash-only pattern + // waited the full 30s while the browser was already sitting on the right + // detail page. The uuid is what proves the suite was created; whether the + // router spells it with a hash is not what this test is about. + await expect(page).toHaveURL(/(?:#)?\/suites\/[^/]+$/, { timeout: 30000 }) // The suite really was persisted, with BOTH modules in `applicaties`. // Read back through the register the UI wrote to.