|
| 1 | +<!-- |
| 2 | + - SPDX-FileCopyrightText: 2026 Conduction B.V. <info@conduction.nl> |
| 3 | + - SPDX-License-Identifier: EUPL-1.2 |
| 4 | + --> |
| 5 | + |
| 6 | +# Catalog ratings |
| 7 | + |
| 8 | +Turns the previously dormant `beoordeeling` (review) schema into a working, |
| 9 | +**moderated** ratings-and-testimonials feature for modules and services — |
| 10 | +and closes the authorization hole it shipped with (world-readable, no |
| 11 | +create/update/delete rules, no attributable author). See |
| 12 | +[VNG Softwarecatalogus issue #49](https://github.com/VNG-Realisatie/Softwarecatalogus/issues/49) |
| 13 | +and softwarecatalog#375. |
| 14 | + |
| 15 | +Specification: [`openspec/specs/catalog-ratings/spec.md`](../../openspec/specs/catalog-ratings/spec.md). |
| 16 | + |
| 17 | +## Why it existed but didn't work |
| 18 | + |
| 19 | +The `beoordeeling` schema was part of the published VNG data model but had |
| 20 | +never been wired up: `authorization` was `{"read": ["public"]}` with **no** |
| 21 | +create/update/delete rules at all, and no author or owning-organisation |
| 22 | +binding. Shipping a ratings UI on top of that as-is would have been |
| 23 | +world-readable with undefined write rules and no accountability. This |
| 24 | +change fixes the schema first, then builds the feature on top of the fixed |
| 25 | +schema. |
| 26 | + |
| 27 | +## Submitting a review |
| 28 | + |
| 29 | +From a module's detail page, a signed-in catalog user clicks **"Write a |
| 30 | +review"** (`ReviewsPanel.vue`, a body widget on `ModuleDetail`), which opens |
| 31 | +`SubmitReviewModal.vue`: a title, a 1-10 rating, and a testimonial. There is |
| 32 | +**no "your name" field** — the author is always the authenticated Nextcloud |
| 33 | +session, bound server-side by `ReviewService`; anything the client sends for |
| 34 | +`auteur` is discarded. |
| 35 | + |
| 36 | +``` |
| 37 | +POST /apps/softwarecatalog/api/reviews |
| 38 | +{ "review": {"naam": "Solid intake flow", "waardering": 9, "beschrijvingLang": "..."}, |
| 39 | + "subjectType": "module", "subjectId": "<module uuid>" } |
| 40 | +``` |
| 41 | + |
| 42 | +Every submission lands `status: "pending"` — it is not yet visible to |
| 43 | +anyone outside the catalog's internal groups. |
| 44 | + |
| 45 | +## Moderation |
| 46 | + |
| 47 | +Reviews are approved/rejected through the **same** `ModerationQueue.vue` |
| 48 | +component already used for anonymous organisation registration, now |
| 49 | +parameterised by a `type` prop (`organisatie`, default, or `beoordeeling`). |
| 50 | +A second instance renders in **Settings → Review moderation**, backed by the |
| 51 | +same admin-gated `ModerationController`/`ModerationService` |
| 52 | +(`#[AuthorizedAdminSetting]`), selected via `?type=beoordeeling`. Approving |
| 53 | +sets `status: "approved"`; rejecting sets `status: "rejected"` and the |
| 54 | +review stays hidden. |
| 55 | + |
| 56 | +## Fail-closed public read |
| 57 | + |
| 58 | +`beoordeeling.authorization.read` is no longer an unconditional `["public"]` |
| 59 | +grant. It is `[{"group":"public","match":{"status":"approved"}}, <internal |
| 60 | +catalog groups>]` — unauthenticated readers only ever see `approved` |
| 61 | +reviews; `pending`/`rejected` reviews are invisible to them. This is |
| 62 | +declared in a new **fragment**, `lib/Settings/register.d/catalog-ratings.json` |
| 63 | +(ADR-037) — the shipped monolith `softwarecatalogus_register.json` is never |
| 64 | +edited directly (an edit there is a silent no-op on installed instances). |
| 65 | + |
| 66 | +A subtlety in the fragment merge itself was fixed as part of closing this |
| 67 | +hole: the generic register-fragment merge concatenates list values (correct |
| 68 | +for most schema properties), which would have left the dangerous bare |
| 69 | +`"public"` entry in place even after the fragment "added" a narrower rule. |
| 70 | +`SettingsService::deepMergeConfig()` now replaces (rather than |
| 71 | +concatenates) list values within any `authorization` block specifically, so |
| 72 | +the fragment genuinely removes the wide-open base rule. |
| 73 | + |
| 74 | +## Aggregate rating |
| 75 | + |
| 76 | +Module (and, once a `DienstDetail` page exists — see Known gaps below, |
| 77 | +dienst) detail pages show an average rating + review count, computed by |
| 78 | +`ReviewAggregateService` from **approved reviews only**. A module with zero |
| 79 | +approved reviews shows a null average / zero count rather than an error. |
| 80 | + |
| 81 | +## Authorization summary |
| 82 | + |
| 83 | +| Action | Who | |
| 84 | +|---|---| |
| 85 | +| Read approved reviews | Anyone (public) | |
| 86 | +| Read pending/rejected reviews | Internal catalog groups + the review's own author (owner privilege) | |
| 87 | +| Create | Authenticated catalog-user groups (never anonymous) | |
| 88 | +| Update | The review's author (owner privilege) or an org-scoped admin group | |
| 89 | +| Delete | `software-catalog-admins` only, or the review's author (owner privilege) | |
| 90 | + |
| 91 | +## Known gaps / follow-ups |
| 92 | + |
| 93 | +- **No `DienstDetail` page yet.** The submit/aggregate backend is |
| 94 | + subject-type-agnostic (`module` or `dienst`), and `beoordeeling` already |
| 95 | + supports a `diensten` relation, but the softwarecatalog manifest has no |
| 96 | + `/diensten/:id` detail route today (`Diensten` is a `type: custom` faceted |
| 97 | + index with no per-row detail page) — that is a pre-existing gap unrelated |
| 98 | + to the authorization fix this change makes. Filed as a follow-up to wire |
| 99 | + `ReviewsPanel` onto that page once it exists. |
| 100 | +- **Residual direct-API risk.** A user already in an authorized `create` |
| 101 | + group could bypass `ReviewController` and call OpenRegister's generic |
| 102 | + object API directly, setting `auteur`/`status` themselves on that path. |
| 103 | + This is an existing, accepted trust boundary shared by every other schema |
| 104 | + in this app; the public read gate is enforced independently of which path |
| 105 | + wrote the object. |
| 106 | + |
| 107 | +## Screenshots |
| 108 | + |
| 109 | +Not captured in this change — per this repo's convention (see |
| 110 | +`organisation-merge.md`), Playwright screenshot capture against a live |
| 111 | +instance was out of bounds for this session (no live Nextcloud instance |
| 112 | +without touching the shared dev environment). Follow-up: capture the |
| 113 | +"Write a review" flow, the aggregate rating panel, and the review moderation |
| 114 | +queue per ADR-010 once verified against a running instance. |
0 commit comments