ADR: Contributor reviews & ratings
ADR: Contributor reviews & ratings
Section titled “ADR: Contributor reviews & ratings”Status
Section titled “Status”PROPOSED
Context
Section titled “Context”The spec at specs/contributor-reviews-ratings.md introduces the
long-deferred review layer originally sketched as OQ-3 of ADR
global-user-identity. Every behavioural decision the spec needs has
already been pinned in its Decisions locked section after three
critic rounds — this ADR therefore does not restate those
decisions. Its job is to frame the feature inside the existing
architecture: where in the schema graph the new table sits, which
existing ADRs it leans on, what we deliberately built around rather
than into, and what the alternatives were at the architectural
(not behavioural) level.
The feature reaches across four schemas (activities, bookings,
companies, users), bridges two API surfaces (client, business),
ships on three consumers (gym_app, tktspace-web,
tktspace-business), and rides through a fourth consumer
(tickets_app) as a regen no-op. Designing it as an isolated module
would be a mistake; the value of an ADR here is anchoring it to the
prior decisions it inherits.
AC ↔ Decision mapping
Section titled “AC ↔ Decision mapping”| AC | Primary decisions | Notes |
|---|---|---|
| AC-1 | D1 (table placement), D2 (cross-schema ref mode) | New activities.contributor_reviews; bare-uuid refs to users.users.id, bookings.bookings.id. |
| AC-2 | D3 (validation pipeline), D4 (race resolution) | 6-step server-side check pipeline; INSERT ... ON CONFLICT DO NOTHING closes the AC-10 retry race. |
| AC-3 | D5 (edit window enforcement) | Server-side 7d window; 403 not_author vs 409 edit_window_expired split. |
| AC-4 | D5 | Same author + window rule, idempotent 204 on success. |
| AC-5 | D6 (aggregation freshness), D8 (cache posture) | On-the-fly aggregate; per-Authorization cache split. |
| AC-6 | D6, D7 (cursor), D8 | Reuse the (created_at, id) opaque base64 cursor pattern from ADR favorite-activities D6. |
| AC-7 | D9 (eligibility placement) | Booking-scoped; reuses AC-2 step-2 ownership chain. |
| AC-8 | D10 (business surface shape), D11 (permission) | Path-scoped /companies/{companyId}/...; new Permission.READ_REVIEWS. |
| AC-9 | D12 (activity-detail embedding) | ratingSummary optional/absent for offline contributors; preview-narrower than the full summary DTO. |
| AC-10 | D13 (gym_app prompt + suppression) | Server-side existing-review filter + local-prefs dismissal; deep-link bypass. |
| AC-11 | D14 (web optimistic UI) | Local prepend on POST success; 60s public cache for peers. |
| AC-12 | D14 | Read-only business tab; explicit “moderation coming” empty state. |
| AC-13 | D15 (i18n surface) | New reviews#* keys, EN/UK/RU mandatory; DE/FR fall back to EN. |
| AC-14 | D16 (deep-link plumbing) | tktspace://bookings/{id}/review + universal link; auth returnUrl for unauthed users. |
| AC-15 | D17 (tickets_app regen-only) | Regenerate, no UI; intentionally accept the unused-bytes cost. |
Decision
Section titled “Decision”D1 — Table placement: activities.contributor_reviews
Section titled “D1 — Table placement: activities.contributor_reviews”The table lives in the activities Postgres schema, alongside
contributors, sessions, and the other activity-bound entities.
Rationale:
- The review is about a contributor on an activity — the row’s
eligibility hinges on
session.activity_id,sessionCoaches,contributors, all owned byactivities. Placing the table next to its dominant join sources keeps the hot read query (activity-detail → ratingSummaryper contributor) within theactivitiesschema search-path. - Two of the three soft references (
booking_id,target_user_id) point out ofactivities(tobookings,users). Putting the table inbookingsoruserswould not reduce cross-schema joins — it would just rotate which join is cross-schema. Theactivitiesplacement also matches whereuserActivityFavoriteslives (favorites is also user-side-of-a- join with cross-schema target). - Future “moderation by sphere admin” follow-ups will most likely
re-use the existing
activities-schema audit log infrastructure, not invent ausers-schema audit log.
D2 — Cross-schema references: soft refs only
Section titled “D2 — Cross-schema references: soft refs only”All three references (target_user_id → users.users.id,
reviewer_user_id → users.users.id, booking_id →
bookings.bookings.id) are bare uuid columns with no
.references(...) clause in Drizzle. This is the rule from ADR
cross-schema-references-without-fk (ACCEPTED).
This means:
- Integrity is enforced by the application layer (AC-2 step 6 EXISTS-check on insert; read-time JOIN-LEFT tolerance for ghosts).
- No DB cascade fires when a
usersorbookingsrow vanishes — the user-deletion / booking-deletion service owns the cleanup (writeshidden_ator nullsreviewer_user_id). - The pre-existing
userActivityFavoritesrow that DOES declare a cross-schema FK tousers.users(id)(verified attktspace-backend/libs/shared/data-access-db/src/lib/schema/activities.schema.ts:462—userId: uuid('user_id').notNull().references(() => users.id, { onDelete: 'cascade' })) is a known ADR violation, not the pattern to copy. Cleaning it up is a separate ticket (out of scope here, but documented so the next person doesn’t propagate it).
The trade-off: a service bug that forgets the EXISTS-check can produce an orphan row. The mitigation is the AC-2 step ordering (check before insert, never select-then-insert) and the dedicated race test the spec demands in §Testing strategy.
D3 — AC-2 validation as a fixed-order pipeline
Section titled “D3 — AC-2 validation as a fixed-order pipeline”The spec pins a 7-step server-side check order with first-failure
short-circuit. The architectural point worth recording is why
fixed-order: each step’s failure carries a distinct HTTP code
(403, 404, or 409 with a discriminator), and the order is
calibrated to leak the minimum information. The booking-existence
check (step 2, 404 booking_not_found) necessarily precedes the
ownership check (403 booking_not_owned) because both come from
the same SELECT — a non-owner CAN still distinguish “exists but
not mine” from “does not exist” by status code. This existence
leak is accepted: a non-owner who guessed a real booking id has
already won the lottery, and the rest of the pipeline reveals
nothing else. After ownership, ordering minimises information by
checking state invariants (booking.status, session.endsAt)
before contributor membership before target-user existence. The
target_user_id EXISTS check fires LATE (step 6) because it’s
the most expensive (cross-schema) and an attacker already knew
about the target’s existence (it came from the activity-detail
payload they just opened).
The single SELECT in step 2 surfaces session_id, status,
ends_at, activity_id, and the customer→user chain in one
round-trip, so steps 3–5 are pure in-memory checks against the
result of step 2. Step 5’s fallback (sessionCoaches vs activity-level
contributors) is intentionally duplicated from
contributor-search-and-session-filter AC-4 — splitting it out into
a shared helper is a refactor, not a v1 concern, but the parity
must hold (any future change to that fallback rule must touch both
sites or they will diverge silently).
D4 — Race resolution via UNIQUE + ON CONFLICT DO NOTHING
Section titled “D4 — Race resolution via UNIQUE + ON CONFLICT DO NOTHING”AC-10’s gym_app prompt fires on session-end push delivery, which can
race with a foreground app’s app-resume background sync triggering
the same prompt. Both can post the same (bookingId, targetUserId)
nearly simultaneously. The DB UNIQUE constraint
(booking_id, target_user_id) + the INSERT ... ON CONFLICT (booking_id, target_user_id) DO NOTHING RETURNING * idiom guarantees:
- Exactly one row materialises (DB serialises the conflict).
- The losing INSERT returns zero rows → service maps to
409 already_reviewed(NOT a 500). - No SELECT-then-INSERT window where another caller can win.
Same idiom that ADR favorite-activities D3 uses for the favorites
upsert. We rely on the same Postgres semantic (skipped rows do not
return) and reuse the same controller-level result-length branch.
D5 — 7-day edit/delete window enforced server-side
Section titled “D5 — 7-day edit/delete window enforced server-side”The window is computed against created_at + interval '7 days' on
the server (a now()-side comparison), never trusted from the client.
Clock skew is not the client’s problem.
The AC-3 error split is the part worth recording here:
reviewer_user_id !== bearer→403 not_author. Authorisation failure; no leak of resource existence beyond whatGETalready reveals (the review’sidis opaque per the cursor list).created_at + 7d ≤ now()→409 edit_window_expired. Resource- state conflict; the resource exists, the caller authored it, but it’s frozen.
Both run AFTER the row-existence check (404 review_not_found for
missing id). Order chosen to align with the AC-2 ordering principle:
existence first, then authorisation, then state.
D6 — Aggregation freshness: read-time, not materialised
Section titled “D6 — Aggregation freshness: read-time, not materialised”The spec locks “computed on read” with a revisit threshold of ~1M rows. Architecturally this means:
RatingSummaryDto.histogramand.averageare derived in a single SQL aggregate againstcontributor_reviewsfiltered bytarget_user_id. The partial indexcontributor_reviews_target_visible_idx(declared in AC-1 withWHERE hidden_at IS NULL) covers this read.- AC-9’s activity-detail embedding does the same aggregate per contributor in a single LATERAL JOIN, NOT a per-row subquery — the spec includes the SQL sketch precisely so backend-dev doesn’t N+1.
- We deliberately do NOT introduce a
user_public_profile_statstable (mentioned as a future option in ADRglobal-user-identity§ “Reviews aggregate”). That table will earn its place when the row count crosses ~1M; today the on-read cost is dominated by the partial index lookup (sub-millisecond on expected scale).
D7 — Cursor pagination: reuse the favorites convention
Section titled “D7 — Cursor pagination: reuse the favorites convention”AC-6 (GET /api/client/contributors/{userId}/reviews) and AC-8 are
cursor-paginated with (created_at DESC, id DESC). The cursor is an
opaque base64-url string base64url(JSON.stringify({ ts, id })) —
identical encoding to ADR favorite-activities D6. We reuse the
same encode/decode helpers (lifted into a shared util if it isn’t
already; backend-dev’s call). Malformed cursors → 400 with
errors.reviews.invalid_cursor.
limit ≤ 50 (vs favorites’ 100) because each row carries an author
preview (globalName, avatarUrl) and a free-text body — payload
size per row is larger. The default is 20.
D8 — HTTP cache posture: per-Authorization split
Section titled “D8 — HTTP cache posture: per-Authorization split”Public-anonymous reads of AC-5/AC-6 get Cache-Control: public, max-age=60, must-revalidate + Vary: Authorization. Authenticated
reads get Cache-Control: private, max-age=15. AC-8 (business) gets
Cache-Control: no-store.
The architectural reason for the Vary: Authorization is that an
unauthenticated CDN edge response MUST NOT be served to an
authenticated caller (whose just-posted optimistic review would
otherwise be invisible for up to 60s). The 15s private window
balances “see my own edits” against “absorb obvious refresh spam”.
No ETag/If-None-Match in v1 — the win on a 15s/60s window is
marginal and the implementation cost is not.
D9 — AC-7 eligibility lives at /bookings/{id}/..., not /me/...
Section titled “D9 — AC-7 eligibility lives at /bookings/{id}/..., not /me/...”Eligibility is a property OF a booking (does this caller, on this
booking, get to review). /me/... is reserved for resources owned
by the caller across the whole platform. The deep-link
tktspace://bookings/{id}/review already reads booking-scoped, so
the endpoint URL falls out naturally.
The contributor list in the response uses the same
sessionCoaches → contributors fallback as AC-2 step 5 — same parity
warning as D3: any change to the fallback rule must touch both
sites.
D10 — Business surface: path-scoped /companies/{companyId}/...
Section titled “D10 — Business surface: path-scoped /companies/{companyId}/...”AC-8 lives at GET /api/business/companies/{companyId}/contributors/ {userId}/reviews. The spec calls this “the existing business-panel
path convention”, which is forward-looking — today every business
endpoint scopes via the x-company-id header. Per the spec
approval this feature introduces the path-scoped shape, intentionally.
Concretely: the path carries companyId; the x-company-id header
remains the security scheme (its value MUST match the path param —
CompanyRolesGuard validates). The response is filtered to reviews
whose booking → session → activity → company_id === :companyId.
Architectural callout: this is the first business endpoint in
the contract carrying companyId as a path parameter. Future
business-side admin endpoints SHOULD follow this pattern when the
operation logically scopes to “a company that may not be my active
company” (e.g. a future super-admin-like cross-company report tool);
the existing header-only scope still works for operations on the
caller’s active company.
D11 — New Permission.READ_REVIEWS on the role-permissions matrix
Section titled “D11 — New Permission.READ_REVIEWS on the role-permissions matrix”A new enum value READ_REVIEWS is added to
libs/features/companies/src/lib/permissions/role-permissions.ts.
Role mapping (locked by the spec):
| Role | READ_REVIEWS |
|---|---|
| OWNER | ✓ (auto via Object.values(Permission)) |
| ADMIN | ✓ |
| MANAGER | ✓ |
| COACH | ✗ — coaches read their own reviews via the public client surface (AC-6). |
COACH is explicitly excluded because granting it would mean a
coach could read every other contributor’s reviews at their company,
which is cross-contributor oversight that the spec reserves for
admin/manager roles. A coach inspecting their own reviews has no
need for the business surface — the public client surface is
identical from a row-content perspective and is the canonical path
for any user (authed or anonymous).
D12 — Activity-detail embedding: ratingSummary is optional, not nullable-required
Section titled “D12 — Activity-detail embedding: ratingSummary is optional, not nullable-required”AC-9 embeds ratingSummary: RatingSummaryPreviewDto per contributor
on GET /api/client/activities/{id}. The DTO encodes three states:
| State | DTO presence |
|---|---|
| Real contributor, no reviews yet | ratingSummary: { average: null, count: 0 } |
| Real contributor with N reviews | ratingSummary: { average: 4.7, count: N } |
Offline contributor (no user_id) | ratingSummary field absent (not null) |
The third row is the architecturally novel one: an offline
contributor (a companyMember whose userId IS NULL) is
structurally unreviewable because the review system targets
users.id. Marking ratingSummary as optional rather than
nullable-required lets consumers branch on presence (if contributor.ratingSummary != null in Dart; ?.ratingSummary in TS)
rather than on count === 0, which preserves the distinction at the
wire level.
RatingSummaryPreviewDto = { average: number | null, count: number }
is deliberately narrower than RatingSummaryDto (which adds the
5-bucket histogram). The histogram is fetched on-demand via AC-5
when the user taps “see all reviews”. Payload-size budget per
contributor is ~16 bytes vs ~60 bytes if the histogram were inlined
— small absolute, but the activity-detail response is the hot
client-side request and we keep it lean.
We deliberately do NOT embed top-N review previews in the activity response (rejected in the spec § Decisions locked). The dialog opens via AC-6 and the activity-detail response stays small and cacheable as a unit (D8).
D13 — Gym_app prompt: server-state-driven + local-prefs suppression
Section titled “D13 — Gym_app prompt: server-state-driven + local-prefs suppression”The “rate your contributor” bottom sheet is gated by two state sources:
- Server-side (AC-7): each contributor in the response carries
existingReviewId. Already-reviewed contributors are skipped before the sheet renders. - Client-side (local prefs):
reviewPromptDismissed:{bookingId}inSharedPreferences. When set, the prompt is suppressed for that booking entirely, regardless of AC-7.
The deep-link from AC-14 bypasses the local-prefs gate (user
explicitly requested the action). Server-side existingReviewId != null still suppresses the per-contributor rating widget within the
sheet, so a user can deep-link in to review the second contributor
on a booking even after reviewing the first.
The two-source design is intentional: server-side covers the
“already done” case across re-installs and device changes;
client-side covers the “I dismissed this prompt” case without
roundtripping to a dismissedAt column. The local-prefs key is
booking-scoped (not user-scoped) so reviewing a different booking
later still triggers the prompt.
D14 — Web: optimistic UI on submit; business: read-only tab
Section titled “D14 — Web: optimistic UI on submit; business: read-only tab”Web AC-11:
- On submit success, prepend the new review into the dialog’s local
list and bump the parent summary’s
count+ recomputedaverageimmediately. Don’t wait for AC-5/AC-6 revalidation. - The 60s public cache means anonymous peers see the change at most 60s later; authenticated peers see it after their 15s private cache cycle. This is an acceptable consistency window for the “social proof” character of reviews.
Business AC-12:
- The “Reviews” tab on the contributor-member detail page is read-only v1. No moderation buttons.
- The empty state explicitly says “Moderation tools coming in a follow-up ticket” so the absence is intentional and visible, not a forgotten TODO.
D15 — i18n surface: reviews#* namespace
Section titled “D15 — i18n surface: reviews#* namespace”All new keys live under the reviews#* prefix (in line with the
existing favorites#*, passes#* conventions). EN/UK/RU are
mandatory; DE/FR fall back to EN. The full key list is in spec AC-13.
The mobile + web key catalogue is unified — the same reviews#*
keys are reused by tktspace-web for the dialog (web pulls from
its own translations file but uses the same key shape to ease
copy/paste reviews from product). tktspace-business uses its own
namespace business.reviews.* since that surface has zero overlap
with end-user copy.
D16 — Deep-linking: scheme + universal link, auth-redirect aware
Section titled “D16 — Deep-linking: scheme + universal link, auth-redirect aware”- Mobile:
tktspace://bookings/{bookingId}/review. - Web/universal:
https://tktspace.app/bookings/{bookingId}/review.
The deep-link target is wired in this ticket (router entries on
gym_app and tktspace-web that hand off to AC-7 + a “leave a review”
sheet). The push that emits this link is a separate notifications
ticket (bullmq-queues-notifications-sessions follow-up).
Unauthenticated callers hitting the universal link are routed
through the existing auth-redirect plumbing — returnUrl preserves
the booking-scoped path, the user signs in, and lands back on the
review sheet. No bespoke “return to review after sign-in” code path
is added; we lean on existing plumbing.
D17 — tickets_app: regen-only no-op
Section titled “D17 — tickets_app: regen-only no-op”The Chopper-generated packages/api client picks up the new
endpoints when client.openapi.yaml changes. tickets_app consumes
the regenerated package but does NOT wire any reviews UI. Adding a
UI is explicitly deferred — tickets_app has events, not coached
sessions.
The bundle-size cost of the unused DTOs/endpoint methods is ≤ a few hundred bytes of compiled Dart and is intentionally accepted. Per-app codegen scoping (i.e. splitting the generated client into per-app barrels) is out of scope; if/when the unused- DTO cost becomes meaningful, that’s a separate codegen-tooling ticket.
Considered alternatives
Section titled “Considered alternatives”Alt 1 — Target companyMember.id instead of users.id
Section titled “Alt 1 — Target companyMember.id instead of users.id”Trade-off: simpler tenancy story (no cross-schema soft ref to
users; the FK chain to companies is intra-schema). But:
- A contributor who coaches at three gyms ends up with three
rating profiles, one per
companyMemberrow. The whole point of the parent ADRglobal-user-identityis that a person has ONE global identity across companies; their rating must travel with them. - Reviews would need cross-schema aggregation to answer “what’s Ivan’s rating?” — exactly the question the activity-detail card wants to answer.
- Offline contributors (the
userId IS NULLrows) would become reviewable, then suddenly become unreviewable the moment they sign up and get linked — a brittle, surprising transition.
Rejected. target_user_id → users.id is the right shape, even at
the cost of three soft cross-schema refs.
Alt 2 — Materialise per-user rating aggregates eagerly
Section titled “Alt 2 — Materialise per-user rating aggregates eagerly”Trade-off: a user_public_profile_stats table (reviewCount,
ratingAverage, ratingHistogramJson) updated by trigger or by an
application after-write hook. Read becomes a single PK lookup,
activity-detail embedding is one extra row per contributor.
But:
- At current scale (~hundreds of reviews) the on-read aggregate using the partial index is sub-millisecond. Materialisation buys nothing measurable today and adds write-path complexity.
- Triggers tie the
activitiesschema to writes elsewhere (booking-deletion writinghidden_atfrom outsideactivities) — exactly the cross-schema entanglement the architecture deliberately avoids (ADRcross-schema-references-without-fk). - An after-write hook would have to be idempotent across the AC-2 ON-CONFLICT-DO-NOTHING idiom AND the soft-hide path AND the reviewer-deletion null-out path — three branches, each with its own consistency window.
Rejected for v1. The spec re-opens this if the table crosses ~1M rows.
Alt 3 — Hard-delete row on moderation soft-hide
Section titled “Alt 3 — Hard-delete row on moderation soft-hide”Trade-off: simpler queries (no WHERE hidden_at IS NULL everywhere).
But:
- Loses the audit trail. A moderation decision is itself a reviewable action (operator support, dispute resolution).
- The
hidden_atcolumn costs one nullable timestamp per row; filtering it out is a partial index, not a full scan. - A future “unhide” flow becomes impossible without a separate archive table. Soft-hide is the strictly more flexible choice.
Rejected. hidden_at NULL is the soft-hide column; partial index
makes the filtering free; the spec calls out the monotonicity
assumption (hidden_at set once, never cleared) and what to do if
that ever changes.
Alt 4 — Allow anonymous reviews
Section titled “Alt 4 — Allow anonymous reviews”Trade-off: more reviews submitted (lower friction). But:
- Loses the eligibility chain entirely — anyone could review anyone with no proof of having attended a session.
- Spec rules it out explicitly. v1 will not ship this; if it ever
does, the column shape (
reviewer_user_id NULL = "Former user") already supports a future `reviewer_user_id NULL- isAnonymous TRUE` extension without a migration.
Rejected. The spec’s “no anonymous reviews v1” stands.
Alt 5 — Auto-cascade on booking/user deletion via DB triggers or in-schema FK
Section titled “Alt 5 — Auto-cascade on booking/user deletion via DB triggers or in-schema FK”Trade-off: zero application code in the deletion path; DB takes care of it. But:
- Direct contradiction of ADR
cross-schema-references-without-fk— we cannot declare an FK across schemas. A trigger could be written but would couple deletion timing across schemas and is the exact failure mode the parent ADR exists to prevent. - The current rule (bookings/user-deletion service writes
hidden_at/reviewer_user_id = NULL) is explicit, traceable in application logs, and easy to test.
Rejected. Application-layer deletion cleanup is the right choice.
Surface impact and field-visibility justification
Section titled “Surface impact and field-visibility justification”Client surface (contracts/client.openapi.yaml)
Section titled “Client surface (contracts/client.openapi.yaml)”New paths:
| Path | Method | AC | Auth |
|---|---|---|---|
/contributor-reviews | POST | AC-2 | Bearer |
/contributor-reviews/{id} | PATCH | AC-3 | Bearer |
/contributor-reviews/{id} | DELETE | AC-4 | Bearer |
/contributors/{userId}/rating-summary | GET | AC-5 | optional |
/contributors/{userId}/reviews | GET | AC-6 | optional |
/bookings/{bookingId}/reviews-eligibility | GET | AC-7 | Bearer |
New schemas:
ContributorReviewDto— read shape (server returns from POST/PATCH and inside AC-6’s array). Fields:id,bookingId,targetUserId,rating,body? nullable,createdAt,updatedAt,author? nullable({ userId, globalName, avatarUrl? }ornullwhen the reviewer row was redacted).ContributorReviewCreateBodyDto—{ bookingId, targetUserId, rating, body? }.ContributorReviewPatchBodyDto—{ rating?, body? }.RatingSummaryDto—{ average: number|null, count: integer, histogram: { "5": int, "4": int, "3": int, "2": int, "1": int } }.RatingSummaryPreviewDto—{ average: number|null, count: integer }(subset; histogram intentionally absent for the activity-detail embed).ReviewsEligibilityDto—{ bookingId, eligibleAt, isEligibleNow, contributors: [{ userId, globalName, avatarUrl?, existingReviewId? }] }.
Field additions to existing schemas:
ContributorPreviewDto.ratingSummary?: RatingSummaryPreviewDto— optional, not nullable. Absent for offline contributors (member.userId IS NULL) per AC-9. The schema description must spell this out so consumers branch on presence.
Field-visibility justification — why these and not others:
ContributorReviewDto.authoris the public identity card (globalName+avatarUrl), NOT the underlyinguserIdplus email/phone. The reviewer’s PII never leaves the server.userIdis included to let the UI link to the reviewer’s public profile (a future ticket), not to enable correlation outside the platform.ContributorReviewDto.authoris nullable so the “Former user” rendering (reviewer_user_id IS NULL) is unambiguous.ReviewsEligibilityDto.contributors[].userIdIS exposed because it’s the input to the AC-2 POST — the caller needs it to leave the review.globalNameandavatarUrlare exposed because the sheet renders them next to each rating widget.
Business surface (contracts/business.openapi.yaml)
Section titled “Business surface (contracts/business.openapi.yaml)”New path:
| Path | Method | AC | Auth |
|---|---|---|---|
/companies/{companyId}/contributors/{userId}/reviews | GET | AC-8 | Bearer + x-company-id + READ_REVIEWS |
New schemas (independent from the client surface — never share types):
ContributorReviewDto(business) — same field NAMES as the client surface, declared independently. Read-only shape; noCreate/Patchvariants because the business surface does not write reviews.RatingSummaryDto(business) —{ average, count, histogram }. We DO include the histogram here even though the AC-8 endpoint doesn’t return it today: a future “moderation summary” follow-up is likely to need it, and the shape is cheap to publish.ReviewListResponseDto(business) —{ items: ContributorReviewDto[], nextCursor: string|null }.
Field-visibility justification — what stays off the business surface that you might expect:
- No
internalNotes, no moderator-only flags. v1 is read-only; there is nothing operator-internal to expose. A future moderation ticket addshiddenAt,hiddenBy,hiddenReasonas a separate patch. - No
reviewerEmail/reviewerPhone. Per the parent ADRglobal-user-identity, identity exposure on the business surface goes throughEmbeddedUserAdminDtoshape only. The author preview onContributorReviewDtocarriesglobalName+avatarUrl, same as the client surface. We do NOT addemailto the business reviews response — that would be the first business endpoint to leakusers.emailfor a contributor’s reviewer, and the principle is “minimum identity surface”. - No
bookingId/sessionId. The business surface filters reviews by company already; an admin reading reviews doesn’t need the deep link into a specific booking to do moderation. If moderation ever does want it, that’s an additive field in a follow-up.
Super-admin surface (contracts/super-admin.openapi.yaml)
Section titled “Super-admin surface (contracts/super-admin.openapi.yaml)”Not touched. v1 has no moderation surface and no cross-company analytics on reviews.
Data model changes
Section titled “Data model changes”| Table | Change | Rationale |
|---|---|---|
activities.contributor_reviews (NEW) | id PK, target_user_id, reviewer_user_id NULL, booking_id, rating smallint CHECK 1..5, body NULL, hidden_at NULL, created_at, updated_at; UNIQUE(booking_id, target_user_id); partial idx (target_user_id, created_at DESC) WHERE hidden_at IS NULL; idx(booking_id). | D1, D2, D4 |
Cross-schema references (all bare uuid, no .references(...) per
ADR cross-schema-references-without-fk):
target_user_id→ conceptuallyusers.users.idreviewer_user_id→ conceptuallyusers.users.idbooking_id→ conceptuallybookings.bookings.id
Invariants (recorded here per the parent ADR’s “Documentation rule”):
target_user_idandreviewer_user_idintegrity is enforced by AC-2 step 6 on insert and by JOIN-LEFT reads that tolerate the row vanishing (renders “Former user” when nullable, filters out when target is gone).booking_idintegrity is enforced by AC-2 step 2 on insert. The customer-cascade caveat (booking → company_customer ON DELETE CASCADE insidebookings) means an upstream company_customer hard-delete drops the booking, which leaves a reviewer row pointing at a vanished booking. Reads tolerate this; the customer- deletion service SHOULD also writehidden_at = now()to the affected reviews. Documented so the customer-deletion service owner sees the dependency.hidden_atis monotonic v1 (set once, never cleared). The partial index relies on this. A future “unhide” flow MUST redesign the index (drop the partial filter or split into two indexes).
Migration generation
Section titled “Migration generation”Per feedback_drizzle_migrations — backend migrations are produced
by drizzle-kit generate from schema edits; hand edits are
minimised. The implementer (Phase C, backend-dev) follows:
-
Edit
libs/shared/data-access-db/src/lib/schema/activities.schema.tsto add thecontributorReviewstable definition. Bare-uuid columns, no.references(...). Concrete shape:export const contributorReviews = activitiesSchema.table('contributor_reviews',{id: uuid('id').primaryKey().default(sql`gen_random_uuid()`),targetUserId: uuid('target_user_id').notNull(),reviewerUserId: uuid('reviewer_user_id'),bookingId: uuid('booking_id').notNull(),rating: smallint('rating').notNull(),body: text('body'),hiddenAt: timestamp('hidden_at', { withTimezone: true }),createdAt: timestamp('created_at', { withTimezone: true }).defaultNow().notNull(),updatedAt: timestamp('updated_at', { withTimezone: true }).defaultNow().notNull(),},(t) => ({bookingTargetUnique: unique('contributor_reviews_booking_target_unique').on(t.bookingId, t.targetUserId),targetVisibleIdx: index('contributor_reviews_target_visible_idx').on(t.targetUserId, t.createdAt.desc()).where(sql`hidden_at IS NULL`),bookingIdx: index('contributor_reviews_booking_idx').on(t.bookingId),ratingCheck: check('contributor_reviews_rating_check', sql`rating BETWEEN 1 AND 5`),}),); -
Run
npx drizzle-kit generate --config=drizzle.config.ts --name=contributor_reviewsfrom the backend repo root. -
Partial-index caveat (per AC-1 in the spec): grep across
libs/shared/data-access-db/src/lib/schema/shows no existing partial-index usage. Verify the pinned Drizzle version emitsCREATE INDEX ... WHERE hidden_at IS NULLcorrectly in the generated migration. If it doesn’t, hand-edit the migration file to add theWHERE hidden_at IS NULLclause and append a comment-- drizzle-kit drift: partial index added by handso the next gen doesn’t try to revert. Perfeedback_drizzle_migrationswe don’t write migrations from scratch — only minimal edits to the generated file are allowed. -
Run the
migration-safety-checkskill on the generated SQL. -
No backfill needed — the table is empty on creation.
The drafts/migration-contributor-reviews-ratings.sql file in this
repo is a preview only — illustrative, not executed.
Backend module placement
Section titled “Backend module placement”libs/features/reviews/ (NEW)
Section titled “libs/features/reviews/ (NEW)”The reviews domain doesn’t fit cleanly inside any existing feature
lib (activities is for activity CRUD/booking; companies is for
company members; bookings writes the booking that grants
eligibility but doesn’t own the review row). A new dedicated lib
mirrors the structure of passes / favorites:
libs/features/reviews/├── project.json├── tsconfig.json├── tsconfig.lib.json├── eslint.config.mjs└── src/ ├── index.ts └── lib/ ├── reviews.module.ts ├── reviews-client.module.ts (controller + service for /api/client/contributor-reviews & friends) ├── reviews-admin.module.ts (controller + service for /api/business/companies/{companyId}/contributors/{userId}/reviews) ├── controllers/ │ ├── reviews-client.controller.ts │ ├── reviews-eligibility-client.controller.ts (AC-7; consider keeping on bookings controller — see note) │ └── reviews-admin.controller.ts ├── services/ │ ├── reviews-client.service.ts │ ├── reviews-eligibility.service.ts │ └── reviews-admin.service.ts └── dto/ ├── contributor-review-create-body.dto.ts ├── contributor-review-patch-body.dto.ts ├── contributor-review.response.dto.ts ├── rating-summary.response.dto.ts ├── rating-summary-preview.response.dto.ts └── reviews-eligibility.response.dto.tsWired into:
apps/api/src/app/modules/client-api/client-api.module.ts— addReviewsClientModulealongsidePassesClientModule,FavoritesClientModule, etc.apps/api/src/app/modules/admin-api/admin-api.module.ts— addReviewsAdminModule.
Note on AC-7 placement: the eligibility endpoint lives at
/api/client/bookings/{bookingId}/reviews-eligibility. Backend-dev
may put the controller in libs/features/bookings/ instead of
libs/features/reviews/ (the booking-ownership chain is a bookings
concern). Either placement is acceptable; the dependency direction
must be: bookings → reviews for type imports (eligibility DTO),
NOT the reverse. ADR doesn’t pin this — it’s a Phase C judgment
call.
Permission.READ_REVIEWS — new enum value
Section titled “Permission.READ_REVIEWS — new enum value”Add to libs/features/companies/src/lib/permissions/role-permissions.ts:
Permission.READ_REVIEWS(new enum value).ADMINandMANAGERarrays gain the new permission.COACHarray does NOT gain it (D11).OWNERauto-binds viaObject.values(Permission).
Rollback safety: removing the enum value cleanly removes it from
ADMIN/MANAGER arrays. OWNER re-derives via
Object.values(Permission) so its grant set shrinks accordingly.
ContributorPreviewDto.ratingSummary projection
Section titled “ContributorPreviewDto.ratingSummary projection”libs/features/activities/src/lib/services/activities-client.service.ts
findById projection gains the LATERAL JOIN sketched in spec AC-9.
The mapper sets ratingSummary: undefined (i.e. field absent in the
serialised JSON) when member.user_id IS NULL, and to
{ average, count } otherwise. The class-transformer config on
the DTO must use @Expose({ groups: ['client'] }) semantics such
that undefined is genuinely omitted from the JSON (not serialised
as null) — backend-dev verifies this in the integration test for
AC-9.
Frontend implications
Section titled “Frontend implications”tktspace-mobile-app/apps/gym_app
Section titled “tktspace-mobile-app/apps/gym_app”- Mobile packages touched (per spec frontmatter):
packages/api— auto-regenerated from the patchedclient.openapi.yaml(melos run sync:spec && melos run generate:api).packages/profile— review UI widgets + state for the contributor profile page (aggregate + reviews list) AND the AC-10 bottom sheet.packages/i18n— add thereviews#*keys to EN/UK/RU; DE/FR fall back to EN.packages/auth— untouched.
- New router target:
tktspace://bookings/{bookingId}/review→ opens AC-7 + leaves-review sheet. - New local prefs key:
reviewPromptDismissed:{bookingId}inSharedPreferences.
apps/tickets_app
Section titled “apps/tickets_app”Regen-only (D17). No UI wiring.
tktspace-web
Section titled “tktspace-web”- Files touched:
src/app/pages/event/event.page.html(and.ts) — under each contributor card add the star bar + count (from AC-9 embedded summary), the “All reviews” button opening aTuiDialogthat paginates AC-6, and the “Leave a review” CTA driven by AC-7.src/app/core/api/— regenerated bynpm run generate.- New i18n keys in the web translations under the
reviews#*namespace (kept aligned with the mobile catalogue).
- SSR considerations: the embedded
ratingSummaryarrives in the SSR-rendered HTML; the dialog (AC-6 fetch) is client-only. The optimistic-UI flow on submit must not break hydration — the initial SSR list of reviews is empty (the dialog isn’t open at SSR time), so this is naturally hydration-safe.
tktspace-business
Section titled “tktspace-business”- Files touched:
- New “Reviews” tab on the contributor-member detail page (the
member detail page is in
client/src/app/features/dashboard/members/...; backend-dev confirms the exact route at Phase C). client/src/app/core/api/— regenerated bynpm run generate:api.- Empty state copy “Moderation tools coming in a follow-up ticket”.
- New “Reviews” tab on the contributor-member detail page (the
member detail page is in
- No new permissions in the UI shell — the existing
CompanyRolesGuard+ role-permissions matrix carriesPermission.READ_REVIEWS.
tktspace-landing
Section titled “tktspace-landing”Not touched.
- Cross-schema soft-ref orphan creation. A service bug that
forgets the AC-2 step 6 EXISTS-check on
users.userswould create a review pointing at a vanished user. Mitigation: integration test for the case (spec § Testing strategy). - Race on AC-10 retries. The push-driven prompt can fire from
multiple sources. Mitigated by D4 (
INSERT ... ON CONFLICT DO NOTHING). Risk that the rate-limit (1 POST per (user, booking) per minute) is bypassed by clock-skew — acceptable; the DB conflict resolution still wins. - Partial-index portability. AC-1 flags this. If Drizzle
doesn’t emit the
WHERE hidden_at IS NULLcleanly, hand-edit the generated migration with a single-line drift comment so next gen doesn’t undo it. - Cache poisoning across
Vary: Authorization. Misconfigured CDN edge could serve an unauthenticated response to an authenticated caller. Mitigation: the cache directive includesVary: Authorization(spec § HTTP caching); ops needs to verify the edge honours it for the launch window. - Activity-detail payload growth. Per-contributor
ratingSummaryadds ~16 bytes/contributor. For an activity with ~5 contributors that’s < 100 bytes; the spec accepts this. Worst-case 20-contributor activity is < 400 bytes — still acceptable. tickets_appunused-DTO bytes. D17 / AC-15. Intentionally accepted.- Cross-schema deletion timing. A
users.usersredaction has to be followed by anUPDATE contributor_reviews SET hidden_at = now() WHERE target_user_id = :id(orreviewer_user_id = NULLif redacting a reviewer). If the user-deletion service is not atomic across these UPDATEs, a vanished-target window exists. AC-9 does NOT add a per-row EXISTS subquery to absorb this — the spec accepts the bounded inconsistency window; if the deletion path cannot guarantee atomicity, AC-9 must be revised to add the EXISTS subquery.
Rollout plan
Section titled “Rollout plan”No feature flag. Single coordinated rollout — reviews is purely
additive (new table + new endpoints + one field on
ContributorPreviewDto). The backend can ship first independently:
_workflowMR (this ADR + both contract patches) lands onmain.tktspace-backendMR — schema edit,drizzle-kit generate,migration-safety-check, implement service code + AC-9 LATERAL JOIN, integration tests including the AC-2 step 7 race test. Deploys independently; zero observable customer impact (no client/business code calls the endpoints yet, and the newratingSummaryfield is optional onContributorPreviewDto).- In parallel:
tktspace-businessMR —npm run generate:api, wire the Reviews tab on contributor-member detail. Ship after backend deploys. - In parallel:
tktspace-webMR —npm run generate, wire the star bar + dialog + Leave-a-review CTA onsrc/app/pages/event/.... Ship after backend deploys. - In parallel:
tktspace-mobile-appMR —melos run sync:spec && melos run generate:api, build the AC-10 sheet, the contributor profile reviews list, the deep-link router targets, addreviews#*keys to EN/UK/RU.tickets_appregen-only. Ship a new gym_app build.
Backwards-compat:
- POST/PATCH/DELETE/GET
/api/client/contributor-reviewsand friends are net-new. ContributorPreviewDto.ratingSummaryis optional — old client builds that haven’t regenerated simply ignore the unknown field. Mobile/web/business clients are forward-compatible.GET /api/business/companies/{companyId}/contributors/{userId}/ reviewsis net-new — no existing business behaviour changes.- Migration is additive — no destructive ops, no FK churn.
Rollback:
- Revert backend MR → endpoint controllers and service code
disappear; the
ContributorPreviewDto.ratingSummaryfield simply stops being populated (consumers still tolerate it as optional). DROP TABLE activities.contributor_reviews;— no FK from other tables (cross-schema soft refs, by D2).- Remove
Permission.READ_REVIEWSfrom the enum AND fromADMIN/MANAGERrole-permission arrays inrole-permissions.ts.OWNERauto-unbinds. - Regenerate consumer clients — DTOs disappear, no breakage in callers because every new field/endpoint was optional/additive.
Stop scope (non-goals)
Section titled “Stop scope (non-goals)”- No contributor reply to a review. Planned follow-up.
- No moderation UI (column
hidden_atexists; UI deferred). - No
tickets_appUI — tickets are for events, not coached sessions. - No push notification template for “rate your contributor” — separate notifications-side ticket.
- No materialised aggregate table — only if reads slow past ~1M rows.
- No anonymous reviews v1.
- No
/contributor/:slugpage in web — separate ticket (depends onuserPublicProfile.slugUI). - No cross-contributor rating import.
- No
userActivityFavoritescross-schema FK cleanup. Pre- existing ADR violation; separate cleanup ticket.
Contract patch outlines
Section titled “Contract patch outlines”Both contract files are PATCHED in this MR. The summary below mirrors the YAML edits applied to each file so reviewers can audit field-by-field.
contracts/client.openapi.yaml
Section titled “contracts/client.openapi.yaml”New paths:
/contributor-reviews—post(AC-2). BodyContributorReviewCreateBodyDto. 201/403/404/409/401 (codes per spec)./contributor-reviews/{id}—patch(AC-3) +delete(AC-4). PATCH bodyContributorReviewPatchBodyDto. 200/204/403/404/409/401./contributors/{userId}/rating-summary—get(AC-5). 200RatingSummaryDto. Public; cache headers per D8./contributors/{userId}/reviews—get(AC-6). Cursor-paginated list. Public; cache headers per D8./bookings/{bookingId}/reviews-eligibility—get(AC-7). 200ReviewsEligibilityDto. Bearer required.
New schemas:
ContributorReviewDtoContributorReviewCreateBodyDtoContributorReviewPatchBodyDtoRatingSummaryDtoRatingSummaryPreviewDtoReviewsEligibilityDtoReviewAuthorPreviewDto(helper, used byContributorReviewDto.author)ReviewListResponseDto(cursor-paginated wrapper for AC-6)EligibilityContributorDto(helper, used byReviewsEligibilityDto.contributors[])
Field additions to existing schemas:
ContributorPreviewDto.ratingSummary?: RatingSummaryPreviewDto— optional, NOT nullable (absent for offline contributors per D12 / AC-9).
contracts/business.openapi.yaml
Section titled “contracts/business.openapi.yaml”New path:
/companies/{companyId}/contributors/{userId}/reviews—get(AC-8). Cursor-paginated;Cache-Control: no-store. PermissionREAD_REVIEWS.
New schemas (declared independently from the client surface):
ContributorReviewDto(business)RatingSummaryDto(business; includeshistogrameven though AC-8 doesn’t return it inline — forward-compat for a moderation follow-up)ReviewAuthorPreviewDto(business)ReviewListResponseDto(business)
Field additions to existing schemas: none.
STATUS: READY_FOR_REVIEW