Skip to content

Commit 2832f56

Browse files
committed
docs(reviews): feature doc for catalog ratings (ADR-010)
Documents the authorization hole that existed, the submit/moderation/ aggregate flows, the authorization summary table, and known gaps (no DienstDetail page yet; residual direct-API risk for already- authorized groups). Screenshots not captured — no live instance available without touching the shared dev environment, per this repo's existing organisation-merge.md precedent.
1 parent 4e80622 commit 2832f56

1 file changed

Lines changed: 114 additions & 0 deletions

File tree

docs/features/catalog-ratings.md

Lines changed: 114 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,114 @@
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

Comments
 (0)