feat(dashboard): admin-only "view as" control on my-reviews page #77

Merged
rcsheets merged 1 commit from feat/reviews-view-as into main 2026-07-22 08:27:29 +00:00
Owner

Admins often sign in to the dashboard with a dedicated admin account that never authors PRs, so their "my reviews" page comes up empty. This adds an admin-only "view as" control to /reviews:

  • One-off: GET /reviews?as=<email> scopes the list to that identity, with a banner and a "view your own" escape link.
  • Saved default: a new users.default_view_email column (migration 000010) stores a preferred identity; "Save as my default" / "Clear default" buttons manage it via admin-gated POST /reviews/view-as. Saving your own email normalizes to "no preference".

Resolution order for admins: ?as= param → saved default → own email.

Non-admins are unaffected: effectiveReviewEmail ignores both the param and any stored preference unless the session is in the admin group, so a regular user cannot browse someone else's review list. Admins can already see every review via /admin/reviews, so this adds no new exposure.

Includes table-driven tests for the resolution logic, including the non-admin lockout cases. gofmt, go vet, go build, and go test -race ./... all pass.

🤖 Generated with Claude Code

Admins often sign in to the dashboard with a dedicated admin account that never authors PRs, so their "my reviews" page comes up empty. This adds an admin-only "view as" control to `/reviews`: - **One-off**: `GET /reviews?as=<email>` scopes the list to that identity, with a banner and a "view your own" escape link. - **Saved default**: a new `users.default_view_email` column (migration 000010) stores a preferred identity; "Save as my default" / "Clear default" buttons manage it via admin-gated `POST /reviews/view-as`. Saving your own email normalizes to "no preference". Resolution order for admins: `?as=` param → saved default → own email. **Non-admins are unaffected**: `effectiveReviewEmail` ignores both the param and any stored preference unless the session is in the admin group, so a regular user cannot browse someone else's review list. Admins can already see every review via `/admin/reviews`, so this adds no new exposure. Includes table-driven tests for the resolution logic, including the non-admin lockout cases. `gofmt`, `go vet`, `go build`, and `go test -race ./...` all pass. 🤖 Generated with [Claude Code](https://claude.com/claude-code)
feat(dashboard): admin-only "view as" control on my-reviews page
All checks were successful
ci / check (pull_request) Successful in 45s
79e64e272d
Admins often sign in with a dedicated admin account that never authors
PRs, so their /reviews page comes up empty. Let an admin scope the page
to another email identity: one-off via ?as=<email>, or persistently via
a saved default_view_email preference on their users row.

Non-admins are unaffected — the param and any stored preference are
ignored unless the session is in the admin group, so a regular user
cannot browse someone else's review list (admins already see everything
via /admin/reviews, so this adds no new exposure).

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Collaborator

Automated review by pr-reviewer v0.36.0 | Safety Check | Claude Sonnet 5 | tracking id r-607e7b-cd9fbf
This is an AI-generated review and may contain mistakes.

Status: Completed


⚠️ The primary review (Mistral Small) failed. The review below is from Claude Sonnet 5, which reviewed the same diff.

Verdict: LGTM — Admin gating is correctly enforced both in the route grouping and in the resolution logic, with no secrets, injection, or breaking changes evident.

Reviewed the diff for the "view as" admin feature.

  • Access control: effectiveReviewEmail correctly gates the ?as= param and saved DefaultViewEmail behind isAdmin; non-admins always get their own email regardless of param/stored value. Tests cover the lockout cases explicitly.
  • The POST /reviews/view-as route is registered inside the r.Use(s.RequireAdmin) group, so it's admin-gated at the router level as claimed.
  • Migration adds a NOT NULL DEFAULT '' column with a safe down migration (DROP COLUMN IF EXISTS) — no backfill concerns since default is a constant.
  • Minor validation on SetDefaultViewEmail handler is light (strings.Contains(email, "@")) but this is just a display preference string with no injection risk (parameterized query) and no sensitive exposure — an admin can already view any user's reviews via /admin/reviews, so this doesn't introduce new exposure as the PR description states.
  • No secrets, no accidentally committed files, no obvious breaking changes to existing routes/behavior for non-admins.

Nothing here looks alarming for a quick safety pass.

*Automated review by [pr-reviewer](https://git.brooktrails.org/brooktrails/pr-reviewer) v0.36.0 | Safety Check | Claude Sonnet 5 | tracking id `r-607e7b-cd9fbf`* *This is an AI-generated review and may contain mistakes.* **Status:** ✅ Completed --- > ⚠️ The primary review (**Mistral Small**) failed. The review below is from **Claude Sonnet 5**, which reviewed the same diff. **✅ Verdict: LGTM** — Admin gating is correctly enforced both in the route grouping and in the resolution logic, with no secrets, injection, or breaking changes evident. Reviewed the diff for the "view as" admin feature. - Access control: `effectiveReviewEmail` correctly gates the `?as=` param and saved `DefaultViewEmail` behind `isAdmin`; non-admins always get their own email regardless of param/stored value. Tests cover the lockout cases explicitly. - The `POST /reviews/view-as` route is registered inside the `r.Use(s.RequireAdmin)` group, so it's admin-gated at the router level as claimed. - Migration adds a `NOT NULL DEFAULT ''` column with a safe down migration (`DROP COLUMN IF EXISTS`) — no backfill concerns since default is a constant. - Minor validation on `SetDefaultViewEmail` handler is light (`strings.Contains(email, "@")`) but this is just a display preference string with no injection risk (parameterized query) and no sensitive exposure — an admin can already view any user's reviews via `/admin/reviews`, so this doesn't introduce new exposure as the PR description states. - No secrets, no accidentally committed files, no obvious breaking changes to existing routes/behavior for non-admins. Nothing here looks alarming for a quick safety pass.
rcsheets deleted branch feat/reviews-view-as 2026-07-22 08:27:30 +00:00
Sign in to join this conversation.
No reviewers
No labels
No milestone
No project
No assignees
2 participants
Notifications
Due date
The due date is invalid or out of range. Please use the format "yyyy-mm-dd".

No due date set.

Dependencies

No dependencies set

Reference
brooktrails/pr-reviewer!77
No description provided.