Skip to content

ADR: Contributor reviews & ratings

PROPOSED

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.

ACPrimary decisionsNotes
AC-1D1 (table placement), D2 (cross-schema ref mode)New activities.contributor_reviews; bare-uuid refs to users.users.id, bookings.bookings.id.
AC-2D3 (validation pipeline), D4 (race resolution)6-step server-side check pipeline; INSERT ... ON CONFLICT DO NOTHING closes the AC-10 retry race.
AC-3D5 (edit window enforcement)Server-side 7d window; 403 not_author vs 409 edit_window_expired split.
AC-4D5Same author + window rule, idempotent 204 on success.
AC-5D6 (aggregation freshness), D8 (cache posture)On-the-fly aggregate; per-Authorization cache split.
AC-6D6, D7 (cursor), D8Reuse the (created_at, id) opaque base64 cursor pattern from ADR favorite-activities D6.
AC-7D9 (eligibility placement)Booking-scoped; reuses AC-2 step-2 ownership chain.
AC-8D10 (business surface shape), D11 (permission)Path-scoped /companies/{companyId}/...; new Permission.READ_REVIEWS.
AC-9D12 (activity-detail embedding)ratingSummary optional/absent for offline contributors; preview-narrower than the full summary DTO.
AC-10D13 (gym_app prompt + suppression)Server-side existing-review filter + local-prefs dismissal; deep-link bypass.
AC-11D14 (web optimistic UI)Local prepend on POST success; 60s public cache for peers.
AC-12D14Read-only business tab; explicit “moderation coming” empty state.
AC-13D15 (i18n surface)New reviews#* keys, EN/UK/RU mandatory; DE/FR fall back to EN.
AC-14D16 (deep-link plumbing)tktspace://bookings/{id}/review + universal link; auth returnUrl for unauthed users.
AC-15D17 (tickets_app regen-only)Regenerate, no UI; intentionally accept the unused-bytes cost.

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:

  1. The review is about a contributor on an activity — the row’s eligibility hinges on session.activity_id, sessionCoaches, contributors, all owned by activities. Placing the table next to its dominant join sources keeps the hot read query (activity-detail → ratingSummary per contributor) within the activities schema search-path.
  2. Two of the three soft references (booking_id, target_user_id) point out of activities (to bookings, users). Putting the table in bookings or users would not reduce cross-schema joins — it would just rotate which join is cross-schema. The activities placement also matches where userActivityFavorites lives (favorites is also user-side-of-a- join with cross-schema target).
  3. Future “moderation by sphere admin” follow-ups will most likely re-use the existing activities-schema audit log infrastructure, not invent a users-schema audit log.

D2 — Cross-schema references: soft refs only

Section titled “D2 — Cross-schema references: soft refs only”

All three references (target_user_idusers.users.id, reviewer_user_idusers.users.id, booking_idbookings.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 users or bookings row vanishes — the user-deletion / booking-deletion service owns the cleanup (writes hidden_at or nulls reviewer_user_id).
  • The pre-existing userActivityFavorites row that DOES declare a cross-schema FK to users.users(id) (verified at tktspace-backend/libs/shared/data-access-db/src/lib/schema/activities.schema.ts:462userId: 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 !== bearer403 not_author. Authorisation failure; no leak of resource existence beyond what GET already reveals (the review’s id is 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.histogram and .average are derived in a single SQL aggregate against contributor_reviews filtered by target_user_id. The partial index contributor_reviews_target_visible_idx (declared in AC-1 with WHERE 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_stats table (mentioned as a future option in ADR global-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):

RoleREAD_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:

StateDTO presence
Real contributor, no reviews yetratingSummary: { average: null, count: 0 }
Real contributor with N reviewsratingSummary: { 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:

  1. Server-side (AC-7): each contributor in the response carries existingReviewId. Already-reviewed contributors are skipped before the sheet renders.
  2. Client-side (local prefs): reviewPromptDismissed:{bookingId} in SharedPreferences. 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 + recomputed average immediately. 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.

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.

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.

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.

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 companyMember row. The whole point of the parent ADR global-user-identity is 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 NULL rows) 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 activities schema to writes elsewhere (booking-deletion writing hidden_at from outside activities) — exactly the cross-schema entanglement the architecture deliberately avoids (ADR cross-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_at column 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.

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:

PathMethodACAuth
/contributor-reviewsPOSTAC-2Bearer
/contributor-reviews/{id}PATCHAC-3Bearer
/contributor-reviews/{id}DELETEAC-4Bearer
/contributors/{userId}/rating-summaryGETAC-5optional
/contributors/{userId}/reviewsGETAC-6optional
/bookings/{bookingId}/reviews-eligibilityGETAC-7Bearer

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? } or null when 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?: RatingSummaryPreviewDtooptional, 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.author is the public identity card (globalName + avatarUrl), NOT the underlying userId plus email/phone. The reviewer’s PII never leaves the server. userId is included to let the UI link to the reviewer’s public profile (a future ticket), not to enable correlation outside the platform.
  • ContributorReviewDto.author is nullable so the “Former user” rendering (reviewer_user_id IS NULL) is unambiguous.
  • ReviewsEligibilityDto.contributors[].userId IS exposed because it’s the input to the AC-2 POST — the caller needs it to leave the review. globalName and avatarUrl are 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:

PathMethodACAuth
/companies/{companyId}/contributors/{userId}/reviewsGETAC-8Bearer + 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; no Create/Patch variants 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 adds hiddenAt, hiddenBy, hiddenReason as a separate patch.
  • No reviewerEmail / reviewerPhone. Per the parent ADR global-user-identity, identity exposure on the business surface goes through EmbeddedUserAdminDto shape only. The author preview on ContributorReviewDto carries globalName + avatarUrl, same as the client surface. We do NOT add email to the business reviews response — that would be the first business endpoint to leak users.email for 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.

TableChangeRationale
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 → conceptually users.users.id
  • reviewer_user_id → conceptually users.users.id
  • booking_id → conceptually bookings.bookings.id

Invariants (recorded here per the parent ADR’s “Documentation rule”):

  • target_user_id and reviewer_user_id integrity 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_id integrity is enforced by AC-2 step 2 on insert. The customer-cascade caveat (booking → company_customer ON DELETE CASCADE inside bookings) 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 write hidden_at = now() to the affected reviews. Documented so the customer-deletion service owner sees the dependency.
  • hidden_at is 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).

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:

  1. Edit libs/shared/data-access-db/src/lib/schema/activities.schema.ts to add the contributorReviews table 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`),
    }),
    );
  2. Run npx drizzle-kit generate --config=drizzle.config.ts --name=contributor_reviews from the backend repo root.

  3. 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 emits CREATE INDEX ... WHERE hidden_at IS NULL correctly in the generated migration. If it doesn’t, hand-edit the migration file to add the WHERE hidden_at IS NULL clause and append a comment -- drizzle-kit drift: partial index added by hand so the next gen doesn’t try to revert. Per feedback_drizzle_migrations we don’t write migrations from scratch — only minimal edits to the generated file are allowed.

  4. Run the migration-safety-check skill on the generated SQL.

  5. 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.

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.ts

Wired into:

  • apps/api/src/app/modules/client-api/client-api.module.ts — add ReviewsClientModule alongside PassesClientModule, FavoritesClientModule, etc.
  • apps/api/src/app/modules/admin-api/admin-api.module.ts — add ReviewsAdminModule.

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: bookingsreviews 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).
  • ADMIN and MANAGER arrays gain the new permission.
  • COACH array does NOT gain it (D11).
  • OWNER auto-binds via Object.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.

  • Mobile packages touched (per spec frontmatter):
    • packages/api — auto-regenerated from the patched client.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 the reviews#* 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} in SharedPreferences.

Regen-only (D17). No UI wiring.

  • 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 a TuiDialog that paginates AC-6, and the “Leave a review” CTA driven by AC-7.
    • src/app/core/api/ — regenerated by npm run generate.
    • New i18n keys in the web translations under the reviews#* namespace (kept aligned with the mobile catalogue).
  • SSR considerations: the embedded ratingSummary arrives 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.
  • 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 by npm run generate:api.
    • Empty state copy “Moderation tools coming in a follow-up ticket”.
  • No new permissions in the UI shell — the existing CompanyRolesGuard + role-permissions matrix carries Permission.READ_REVIEWS.

Not touched.

  • Cross-schema soft-ref orphan creation. A service bug that forgets the AC-2 step 6 EXISTS-check on users.users would 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 NULL cleanly, 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 includes Vary: Authorization (spec § HTTP caching); ops needs to verify the edge honours it for the launch window.
  • Activity-detail payload growth. Per-contributor ratingSummary adds ~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_app unused-DTO bytes. D17 / AC-15. Intentionally accepted.
  • Cross-schema deletion timing. A users.users redaction has to be followed by an UPDATE contributor_reviews SET hidden_at = now() WHERE target_user_id = :id (or reviewer_user_id = NULL if 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.

No feature flag. Single coordinated rollout — reviews is purely additive (new table + new endpoints + one field on ContributorPreviewDto). The backend can ship first independently:

  1. _workflow MR (this ADR + both contract patches) lands on main.
  2. tktspace-backend MR — 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 new ratingSummary field is optional on ContributorPreviewDto).
  3. In parallel: tktspace-business MR — npm run generate:api, wire the Reviews tab on contributor-member detail. Ship after backend deploys.
  4. In parallel: tktspace-web MR — npm run generate, wire the star bar + dialog + Leave-a-review CTA on src/app/pages/event/.... Ship after backend deploys.
  5. In parallel: tktspace-mobile-app MR — melos run sync:spec && melos run generate:api, build the AC-10 sheet, the contributor profile reviews list, the deep-link router targets, add reviews#* keys to EN/UK/RU. tickets_app regen-only. Ship a new gym_app build.

Backwards-compat:

  • POST/PATCH/DELETE/GET /api/client/contributor-reviews and friends are net-new.
  • ContributorPreviewDto.ratingSummary is 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}/ reviews is net-new — no existing business behaviour changes.
  • Migration is additive — no destructive ops, no FK churn.

Rollback:

  1. Revert backend MR → endpoint controllers and service code disappear; the ContributorPreviewDto.ratingSummary field simply stops being populated (consumers still tolerate it as optional).
  2. DROP TABLE activities.contributor_reviews; — no FK from other tables (cross-schema soft refs, by D2).
  3. Remove Permission.READ_REVIEWS from the enum AND from ADMIN/MANAGER role-permission arrays in role-permissions.ts. OWNER auto-unbinds.
  4. Regenerate consumer clients — DTOs disappear, no breakage in callers because every new field/endpoint was optional/additive.
  • No contributor reply to a review. Planned follow-up.
  • No moderation UI (column hidden_at exists; UI deferred).
  • No tickets_app UI — 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/:slug page in web — separate ticket (depends on userPublicProfile.slug UI).
  • No cross-contributor rating import.
  • No userActivityFavorites cross-schema FK cleanup. Pre- existing ADR violation; separate cleanup ticket.

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.

New paths:

  • /contributor-reviewspost (AC-2). Body ContributorReviewCreateBodyDto. 201/403/404/409/401 (codes per spec).
  • /contributor-reviews/{id}patch (AC-3) + delete (AC-4). PATCH body ContributorReviewPatchBodyDto. 200/204/403/404/409/401.
  • /contributors/{userId}/rating-summaryget (AC-5). 200 RatingSummaryDto. Public; cache headers per D8.
  • /contributors/{userId}/reviewsget (AC-6). Cursor-paginated list. Public; cache headers per D8.
  • /bookings/{bookingId}/reviews-eligibilityget (AC-7). 200 ReviewsEligibilityDto. Bearer required.

New schemas:

  • ContributorReviewDto
  • ContributorReviewCreateBodyDto
  • ContributorReviewPatchBodyDto
  • RatingSummaryDto
  • RatingSummaryPreviewDto
  • ReviewsEligibilityDto
  • ReviewAuthorPreviewDto (helper, used by ContributorReviewDto.author)
  • ReviewListResponseDto (cursor-paginated wrapper for AC-6)
  • EligibilityContributorDto (helper, used by ReviewsEligibilityDto.contributors[])

Field additions to existing schemas:

  • ContributorPreviewDto.ratingSummary?: RatingSummaryPreviewDto — optional, NOT nullable (absent for offline contributors per D12 / AC-9).

New path:

  • /companies/{companyId}/contributors/{userId}/reviewsget (AC-8). Cursor-paginated; Cache-Control: no-store. Permission READ_REVIEWS.

New schemas (declared independently from the client surface):

  • ContributorReviewDto (business)
  • RatingSummaryDto (business; includes histogram even 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