feat(admin): admin portal — pagination, audit log, allowlist, analytics (closes #69)

[AndresL230]

Closes#69.

Summary

• Pagination + server-side search on /api/admin/users; new last_sign_in_at column written on every Google callback.
• New endpoints: unapprove user; achievement triggers list/update/delete; achievement_cosmetics + role_cosmetics link/unlink/list; admin_audit_log read; analytics overview (totals + 30-day series + role counts); allowlist approve/revoke.
• Self-protection guards: cannot revoke own admin, cannot revoke last admin, cannot delete admin role, cannot unapprove self.
• Audit log written for every admin mutation via a single sanctioned helper (services/admin_audit.py).
• Frontend Admin.tsx gains Allowlist + Audit tabs and rewires Users / Achievements / Cosmetics / Analytics to the new endpoints — all using existing card / chip / btn / label-micro / var(--accent) design tokens (no new design language).
• assign_role is now idempotent (upsert on (user_id, role_id)); granted_by auto-fills from session.

Schema

• New table admin_audit_log (actor_id, action, target_type, target_id, payload jsonb, created_at) with FK actor_id ON DELETE RESTRICT and read-path indexes on created_at / actor / target.
• New column users.last_sign_in_at and index on users.created_at.

Migration file: backend/db/migration_admin_portal.sql — run once in the Supabase SQL editor.

Test plan

• Run the SQL migration in the dev Supabase project.
• cd backend && python -m pytest tests/test_admin_routes.py tests/test_admin_audit.py tests/test_users_search.py tests/test_supabase.py tests/test_auth_state.py -q (81 tests, all green on this branch).
• cd frontend && npx tsc --noEmit (clean).
• Manual smoke: visit /admin as an admin and walk every tab.
  • Users: paginate, debounced search, approve/unapprove, role assign/revoke.
  • Allowlist: add an email, revoke, re-approve.
  • Roles: try deleting the admin role (must 409); try revoking own admin (409); reassigning an existing role must NOT 500.
  • Achievements: Manage on a row, add/edit/delete a trigger, link/unlink a cosmetic chip.
  • Cosmetics: Manage on a row, link/unlink a role chip.
  • Analytics: totals match DB, sparkline tooltips show daily counts.
  • Audit: every action above appears as a row, filters by action/target_type work.

Out of scope

• Impersonate user (mentioned in feat(admin): admin portal UI — users, allowlist, roles, achievements, cosmetics #69 — touches session minting; deferred).
• EmailStr validation on AllowlistEmailBody — currently email: str. Worth tightening in a follow-up.

Plan

Implementation followed docs/superpowers/plans/2026-05-04-admin-portal.md (committed in this branch).

Summary by CodeRabbit

• New Features
  • Admin dashboard expanded with allowlist management and audit log sections
  • User management improved with pagination, search, and last sign-in date tracking
  • Achievement trigger and cosmetic linking management tools added
  • Admin audit logging tracks all administrative actions system-wide
  • Analytics dashboard displays signup trends and role distribution metrics

[coderabbitai]

📝 Walkthrough

Walkthrough

This PR implements a comprehensive admin portal spanning backend and frontend. It adds database schema for audit logging, a new select_with_count() pagination helper, admin audit tracking across mutations, paginated/searchable user listing, achievement trigger and cosmetic-linking management, role protection rules, allowlist administration, analytics computation, and a fully featured admin UI with tabs for users (paginated/searchable), allowlist, roles, achievements, cosmetics, and audit logs.

Changes

Admin Portal – Complete Implementation

Layer / File(s)	Summary
Database Schema & Indexing backend/db/migration_admin_portal.sql	Creates admin_audit_log table with actor/action/target metadata and timestamps; adds users.last_sign_in_at column; indexes created for time-based and actor-based audit queries.
Database Query Helpers backend/db/connection.py	Adds SupabaseTable.select_with_count() synchronous method that returns (rows, total) by parsing Content-Range header; supports offset alongside limit.
Request Body Models backend/models/__init__.py	Introduces UpdateAchievementTriggerBody, LinkAchievementCosmeticBody, LinkRoleCosmeticBody, and AllowlistEmailBody for admin endpoints.
Audit Service backend/services/admin_audit.py	Adds log_admin_action() to insert audit rows into admin_audit_log with error suppression; ensures audit failures don't block originating operations.
User Pagination & Search backend/services/users_search.py	Adds paginate_users() implementing DB-level pagination when no query provided and decrypt-all-then-filter approach for name/email search; attaches per-user roles and returns {users, total, page, page_size}.
Admin Routes backend/routes/admin.py	Adds/extends 20+ endpoints: role CRUD with admin-role self-protection, achievement management with trigger CRUD and cosmetic linking, cosmetic management, user listing (paginated), approval/unapproval with self-guard, role/achievement-cosmetic linking, allowlist administration, audit log querying, and analytics overview computation. All mutations emit audit events via log_admin_action().
Auth Integration backend/routes/auth.py	Tracks last_sign_in_at on Google OAuth callback (both existing and new users); adds test helper _stamp_last_sign_in_for_test() to support test scenarios without full OAuth flow.
Backend Tests backend/tests/test_admin_audit.py, test_admin_routes.py, test_auth_state.py, test_supabase.py, test_users_search.py	Comprehensive unit and integration test coverage: audit logging (row insertion and error suppression), user pagination with/without search and role attachment, role protection rules, achievement/cosmetic/allowlist/audit operations, analytics computation, and select_with_count() parsing.
Frontend Types frontend/src/lib/types.ts	Adds AllowlistEmail, AchievementTrigger, AdminAuditEntry, analytics-related types (AnalyticsTotals, AnalyticsDayPoint, AnalyticsRoleCount, AnalyticsOverview), AdminUserListItem, and PaginatedUsers; extends Cosmetic with optional unlock_source field.
Frontend API Wrappers frontend/src/lib/api.ts	Adds ~17 new admin helpers for paginated user listing (with search), user unapproval, role-cosmetic linking, achievement trigger CRUD, achievement-cosmetic linking, allowlist management, audit log querying with filters, and analytics overview retrieval.
Frontend Admin UI frontend/src/components/screens/Admin.tsx	Expands Admin component to include Users tab (paginated with debounced search, approve/unapprove, last-seen display), Allowlist tab (email list with approve/revoke), Audit tab (paginated audit log with action/target filters and payload display), and extends Roles, Achievements, Cosmetics tabs with row-expansion UIs for managing linked cosmetics/roles and achievement triggers. Refactors Analytics to use adminAnalyticsOverview() with metric cards and sparklines. Upgrades CatalogRow component to support expandable management sections.

Sequence Diagram

sequenceDiagram
participant Admin as Admin User
participant FrontendUI as Frontend UI
participant API as Admin API
participant Auth as Auth Service
participant DB as Database
rect rgba(100, 200, 150, 0.5)
note over Admin,DB: User Search & Pagination Flow
Admin->>FrontendUI: Enter search query, navigate to page 2
FrontendUI->>FrontendUI: Debounce search input
FrontendUI->>API: adminFetchUsers({q: "alice", page: 2, page_size: 50})
API->>Auth: get_session_user_id() → verify admin
API->>DB: paginate_users(q="alice", page=2, page_size=50)
alt Query provided (search)
DB->>DB: Decrypt all users, filter by substring match
DB->>DB: Compute total from filtered set, slice page
else No query (list)
DB->>DB: select_with_count (offset=50, limit=50)
end
DB->>DB: Attach roles per user from user_roles
DB-->>API: {users: [...], total: 137, page: 2, page_size: 50}
API-->>FrontendUI: PaginatedUsers response
FrontendUI->>FrontendUI: Render users table with Last Seen, Joined dates
FrontendUI-->>Admin: Display page 2 of 3, approve/unapprove buttons
end
rect rgba(150, 180, 220, 0.5)
note over Admin,DB: Admin Audit Logging Flow
Admin->>FrontendUI: Click "Approve" button for user
FrontendUI->>API: adminApproveUser(userId)
API->>Auth: get_session_user_id() → actor_id
API->>DB: UPDATE users SET is_approved=true WHERE id=userId
API->>DB: log_admin_action(actor_id, "user.approve", "user", userId, {...})
DB->>DB: INSERT INTO admin_audit_log (actor_id, action, target_type, target_id, created_at)
DB-->>API: Audit row inserted (or error caught & logged)
API-->>FrontendUI: {approved: true}
FrontendUI-->>Admin: Show success, refresh user list
Admin->>FrontendUI: Visit Audit tab
FrontendUI->>API: adminAuditLog({page: 1, action: "user.approve"})
DB->>DB: SELECT * FROM admin_audit_log WHERE action='user.approve' ORDER BY created_at DESC
DB-->>API: Paginated audit rows
API-->>FrontendUI: Audit entries with timestamps, payload JSON
FrontendUI-->>Admin: Display filtered audit log
end
rect rgba(200, 150, 180, 0.5)
note over Admin,DB: Role-Cosmetic Linking Flow
Admin->>FrontendUI: Expand role row, toggle linked cosmetic
FrontendUI->>API: adminLinkRoleCosmetic(roleId, cosmeticId)
API->>Auth: get_session_user_id() → actor_id
API->>DB: UPSERT INTO role_cosmetics (role_id, cosmetic_id)
API->>DB: log_admin_action(actor_id, "role_cosmetic.link", "role_cosmetic", null, {...})
DB->>DB: INSERT INTO admin_audit_log
DB-->>API: Link upserted, audit logged
API-->>FrontendUI: {linked: true}
FrontendUI->>FrontendUI: Update expanded row UI to reflect new link
FrontendUI-->>Admin: Show cosmetic chip as linked in role details
end

Loading

Estimated Code Review Effort

🎯 4 (Complex) | ⏱️ ~45 minutes

Possibly Related PRs

• SaplingLearn/Sapling#56: Both PRs modify backend/routes/auth.py and the users table schema; PR #56 adds user field persistence during OAuth callback while this PR extends that to include last_sign_in_at tracking.

Poem

🐰 A warren of admins, now armed and aware,
With audit trails woven through each admin's care,
Cosmetics and triggers link up with grace,
While pages of users scroll at a measured pace,
The portal is gleaming—no more fumbling in the dark!

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 9.26% which is insufficient. The required threshold is 80.00%.	Write docstrings for the functions missing them to satisfy the coverage threshold.

✅ Passed checks (4 passed)

Check name	Status	Explanation
Title check	✅ Passed	The title accurately reflects the main changes: pagination, audit log, allowlist, and analytics for the admin portal. It is specific, concise, and directly related to the primary changeset.
Description check	✅ Passed	The description is comprehensive and follows the template structure with Summary, Changes Made (implicit via detailed sections), Testing checklist, and Out of Scope items. All key implementation details are documented.
Linked Issues check	✅ Passed	The PR fully addresses issue #69 objectives: pagination and search for users [#69], unapprove endpoint [#69], audit logging for mutations [#69], role protection guards [#69], achievement triggers management [#69], role/achievement cosmetic linking [#69], allowlist endpoints [#69], analytics overview [#69], and role assignment idempotency [#69].
Out of Scope Changes check	✅ Passed	All code changes are directly aligned with the stated objectives. Out-of-scope items (impersonation, stricter email validation) are explicitly deferred and not included in the changeset.

_{✏️ Tip: You can configure your own custom pre-merge checks in the settings.}

✨ Finishing Touches

📝 Generate docstrings

• Create stacked PR
• Commit on current branch

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Commit unit tests in branch feat/admin-portal

---

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

[cloudflare-workers-and-pages]

Deploying with Cloudflare Workers

The latest updates on your project. Learn more about integrating Git with Workers.

Status	Name	Latest Commit	Preview URL	Updated (UTC)
✅ Deployment successful! View logs	frontend	1c23a86	Commit Preview URL Branch Preview URL	May 04 2026, 11:23 PM

[coderabbitai]

Actionable comments posted: 4

🧹 Nitpick comments (2)

backend/services/users_search.py (1)

20-26: ⚡ Quick win

N+1 queries in _attach_roles — collapse to a single batch request

Each call to _attach_roles issues one Supabase REST request per user. With _MAX_PAGE_SIZE = 200 that's up to 200 sequential HTTP round-trips per admin page load. PostgREST's in.(...) filter fetches all roles in one request:

♻️ Proposed refactor

 def _attach_roles(users: list[dict]) -> None:
- for user in users:- rows = table("user_roles").select(- "roles(id,name,slug,color,icon,description,is_staff_assigned,is_earnable,display_priority)",- filters={"user_id": f"eq.{user['id']}"},- )- user["roles"] = [r["roles"] for r in (rows or []) if r.get("roles")]+ if not users:+ return+ ids = ",".join(u["id"] for u in users)+ all_rows = table("user_roles").select(+ "user_id,roles(id,name,slug,color,icon,description,is_staff_assigned,is_earnable,display_priority)",+ filters={"user_id": f"in.({ids})"},+ ) or []+ role_map: dict[str, list] = {}+ for r in all_rows:+ if r.get("roles"):+ role_map.setdefault(r["user_id"], []).append(r["roles"])+ for user in users:+ user["roles"] = role_map.get(user["id"], [])

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@backend/services/users_search.py` around lines 20 - 26, The _attach_roles
function issues an individual Supabase/PostgREST request per user causing N+1
queries; change it to fetch all roles in one batch by querying
table("user_roles") with a single select using the user_id
in.(<list_of_user_ids>) filter, retrieving "user_id" and the nested "roles(...)"
fields, then group the returned rows by user_id and assign user["roles"] =
[r["roles"] for r in grouped[user_id]] (or [] if none) for each user; keep
function name _attach_roles, use the same table("user_roles") select call but
replace per-user filters with the in filter and ensure you handle empty input
lists and absent role entries.

backend/routes/auth.py (1)

44-51: ⚡ Quick win

Move _stamp_last_sign_in_for_test out of the production module and hoist the datetime import

Two related issues:

1. _stamp_last_sign_in_for_test is a test-only helper sitting in the production auth.py module. Any package import of routes.auth exposes it, and it exists only because datetime.now is buried inside conditional branches where standard monkeypatch can't reach it.
2. from datetime import datetime as _dt, timezone as _tz is duplicated in three places (lines 47, 325, 343). Python caches module imports, so this isn't a correctness bug, but it's unnecessarily noisy.

The cleanest resolution is to promote the import to module level, which then makes routes.auth.datetime directly patchable in tests and removes the need for the test seam entirely:

♻️ Proposed refactor

+from datetime import datetime as _dt, timezone as _tz
import json
...
-def _stamp_last_sign_in_for_test(user_id: str) -> None:- """Test seam: write last_sign_in_at ..."""- from datetime import datetime, timezone- table("users").update(- {"last_sign_in_at": datetime.now(timezone.utc).isoformat()},- filters={"id": f"eq.{user_id}"},- )--
...
if existing:
- from datetime import datetime as _dt, timezone as _tz
table("users").update({
...
"last_sign_in_at": _dt.now(_tz.utc).isoformat(),
}, ...)
else:
- from datetime import datetime as _dt, timezone as _tz
table("users").insert({
...
"last_sign_in_at": _dt.now(_tz.utc).isoformat(),
})

With _dt at module level, test_auth_state.py can replace the test helper with a direct monkeypatch:

# test_auth_state.pyimportroutes.authasauth_moduledeftest_callback_stamps_last_sign_in(monkeypatch):
fixed=_dt(2024, 1, 1, tzinfo=_tz.utc)
monkeypatch.setattr(auth_module, "_dt", lambda*a, **kw: fixed)
...

Also applies to: 325-325, 343-343

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@backend/routes/auth.py` around lines 44 - 51, Move the test-only helper
_stamp_last_sign_in_for_test out of the production routes.auth module into the
test suite and hoist the datetime import to module scope: add a module-level
import like "from datetime import datetime as _dt, timezone as _tz" in
routes.auth, replace any local in-function imports and uses of
datetime.now(timezone.utc) with _dt(..., tzinfo=_tz.utc) (or calls to
_dt.now(_tz.utc)), and remove the test helper from the production file so tests
can monkeypatch the module-level _dt/_tz instead; update any references to
_stamp_last_sign_in_for_test in tests to call the moved helper in the test
package.

🤖 Prompt for all review comments with AI agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@backend/routes/admin.py`:
- Around line 558-570: The approvals series is currently counted by signup date
(using created_at) rather than the approval timestamp; update the logic in the
loop that builds by_day_approvals/approvals_by_day to use an approval timestamp
(e.g., check for an approved_at or similar field on each user instead of
created_at) and parse that timestamp into a date (like you do with created_at)
before incrementing by_day_approvals[d.isoformat()]. If there is no approval
timestamp available, either add/compute one at approval time (e.g., set
approved_at when is_approved transitions true) or rename the metric from
“Approvals” to something like “Approved users by signup date” to reflect the
current derivation (refer to variables/users, created_at, is_approved,
by_day_approvals, approvals_by_day, window).
- Around line 95-109: The code currently allows a client to override the grantor
via body.granted_by; instead, always derive the grantor from the session: set
granted_by = get_session_user_id(request) (i.e., the existing actor variable)
and remove usage of body.granted_by when building the upsert payload and the
log_admin_action payload so both table("user_roles").upsert and log_admin_action
use the trusted actor value only.
In `@backend/services/admin_audit.py`:
- Line 31: The log currently calls log.exception("admin_audit_log write failed:
%s", row) which will serialize sensitive identifiers (row.actor_id,
row.target_id); change the logging to avoid including identifying fields by
creating a sanitized version of row that omits or redacts actor_id and target_id
and log that instead (or log only non-identifying metadata such as event type,
timestamp, and error context). Update the call site using log.exception to pass
the sanitized data or a minimal message and error context rather than the full
row so actor_id/target_id are never written to logs.
In `@frontend/src/components/screens/Admin.tsx`:
- Around line 492-511: loadDetails writes shared state (setTriggers,
setLinkedCosmeticIds, setAllCosmetics) unguarded so a stale async response for a
previously requested id can overwrite the currently open achievement; fix by
capturing/validating the expected open id before applying results (e.g., read
current openId from a ref or compare openId at the time of resolution) and only
call setTriggers/setLinkedCosmeticIds/setAllCosmetics when the resolved id still
matches the current openId (or use an AbortController per loadDetails to cancel
stale requests), keeping function names loadDetails, toggleOpen, setTriggers,
setLinkedCosmeticIds, setAllCosmetics and openId as the reference points.
---
Nitpick comments:
In `@backend/routes/auth.py`:
- Around line 44-51: Move the test-only helper _stamp_last_sign_in_for_test out
of the production routes.auth module into the test suite and hoist the datetime
import to module scope: add a module-level import like "from datetime import
datetime as _dt, timezone as _tz" in routes.auth, replace any local in-function
imports and uses of datetime.now(timezone.utc) with _dt(..., tzinfo=_tz.utc) (or
calls to _dt.now(_tz.utc)), and remove the test helper from the production file
so tests can monkeypatch the module-level _dt/_tz instead; update any references
to _stamp_last_sign_in_for_test in tests to call the moved helper in the test
package.
In `@backend/services/users_search.py`:
- Around line 20-26: The _attach_roles function issues an individual
Supabase/PostgREST request per user causing N+1 queries; change it to fetch all
roles in one batch by querying table("user_roles") with a single select using
the user_id in.(<list_of_user_ids>) filter, retrieving "user_id" and the nested
"roles(...)" fields, then group the returned rows by user_id and assign
user["roles"] = [r["roles"] for r in grouped[user_id]] (or [] if none) for each
user; keep function name _attach_roles, use the same table("user_roles") select
call but replace per-user filters with the in filter and ensure you handle empty
input lists and absent role entries.

🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

• Push a commit to this branch (recommended)
• Create a new PR with the fixes

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro Plus

Run ID: e976a667-d4b6-4fac-9347-f27bc7d43539

📥 Commits

Reviewing files that changed from the base of the PR and between db5ba48 and 1c23a86.

📒 Files selected for processing (16)

• backend/db/connection.py
• backend/db/migration_admin_portal.sql
• backend/models/__init__.py
• backend/routes/admin.py
• backend/routes/auth.py
• backend/services/admin_audit.py
• backend/services/users_search.py
• backend/tests/test_admin_audit.py
• backend/tests/test_admin_routes.py
• backend/tests/test_auth_state.py
• backend/tests/test_supabase.py
• backend/tests/test_users_search.py
• docs/superpowers/plans/2026-05-04-admin-portal.md
• frontend/src/components/screens/Admin.tsx
• frontend/src/lib/api.ts
• frontend/src/lib/types.ts

[coderabbitai]

⚠️ Potential issue | 🟠 Major | ⚡ Quick win

Do not trust client-supplied granted_by.

granted_by = body.granted_by or actor lets the caller forge who granted the role. That breaks the server-side provenance this PR is adding and makes both the row data and the audit trail unreliable. Stamp granted_by from get_session_user_id(request) unconditionally.

Suggested fix

 def assign_role(body: AssignRoleBody, request: Request):
require_admin(request)
actor = get_session_user_id(request)
- granted_by = body.granted_by or actor+ granted_by = actor
table("user_roles").upsert(
{
"user_id": body.user_id,
"role_id": body.role_id,
"granted_by": granted_by,

📝 Committable suggestion

‼️IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change

	actor=get_session_user_id(request)
	granted_by=body.granted_byoractor
	table("user_roles").upsert(
	{
	"user_id": body.user_id,
	"role_id": body.role_id,
	"granted_by": granted_by,
	"granted_at": datetime.now(timezone.utc).isoformat(),
	},
	on_conflict="user_id,role_id",
	)
	log_admin_action(
	actor_id=actor, action="role.assign", target_type="role", target_id=body.role_id,
	payload={"user_id": body.user_id, "granted_by": granted_by},
	)
	actor=get_session_user_id(request)
	granted_by=actor
	table("user_roles").upsert(
	{
	"user_id": body.user_id,
	"role_id": body.role_id,
	"granted_by": granted_by,
	"granted_at": datetime.now(timezone.utc).isoformat(),
	},
	on_conflict="user_id,role_id",
	)
	log_admin_action(
	actor_id=actor, action="role.assign", target_type="role", target_id=body.role_id,
	payload={"user_id": body.user_id, "granted_by": granted_by},
	)

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@backend/routes/admin.py` around lines 95 - 109, The code currently allows a
client to override the grantor via body.granted_by; instead, always derive the
grantor from the session: set granted_by = get_session_user_id(request) (i.e.,
the existing actor variable) and remove usage of body.granted_by when building
the upsert payload and the log_admin_action payload so both
table("user_roles").upsert and log_admin_action use the trusted actor value
only.

[coderabbitai]

⚠️ Potential issue | 🟠 Major | 🏗️ Heavy lift

The “approvals” series is grouped by signup date, not approval date.

approvals_by_day is incremented from created_at for users whose current is_approved is true. A user approved today will be counted on their original signup day, so the Analytics tab’s “Approvals · last 30 days” chart is wrong. This needs either a real approval timestamp or a renamed metric that matches the current derivation.

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@backend/routes/admin.py` around lines 558 - 570, The approvals series is
currently counted by signup date (using created_at) rather than the approval
timestamp; update the logic in the loop that builds
by_day_approvals/approvals_by_day to use an approval timestamp (e.g., check for
an approved_at or similar field on each user instead of created_at) and parse
that timestamp into a date (like you do with created_at) before incrementing
by_day_approvals[d.isoformat()]. If there is no approval timestamp available,
either add/compute one at approval time (e.g., set approved_at when is_approved
transitions true) or rename the metric from “Approvals” to something like
“Approved users by signup date” to reflect the current derivation (refer to
variables/users, created_at, is_approved, by_day_approvals, approvals_by_day,
window).

[coderabbitai]

⚠️ Potential issue | 🟠 Major | ⚡ Quick win

log.exception serializes user identifiers (actor_id, target_id) into application logs

row includes actor_id and target_id, which encode Google sub-claim–based user IDs — persistent identifiers that can be linked back to real persons. If application logs are forwarded to external aggregators this violates GDPR/CCPA guidance against logging user identifiers.

🔒 Proposed fix — log only non-identifying metadata

- log.exception("admin_audit_log write failed: %s", row)+ log.exception(+ "admin_audit_log write failed action=%s target_type=%s",+ row.get("action"),+ row.get("target_type"),+ )

As per coding guidelines, logging sensitive data such as user identifiers is classified as a compliance/privacy risk (GDPR/CCPA).

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@backend/services/admin_audit.py` at line 31, The log currently calls
log.exception("admin_audit_log write failed: %s", row) which will serialize
sensitive identifiers (row.actor_id, row.target_id); change the logging to avoid
including identifying fields by creating a sanitized version of row that omits
or redacts actor_id and target_id and log that instead (or log only
non-identifying metadata such as event type, timestamp, and error context).
Update the call site using log.exception to pass the sanitized data or a minimal
message and error context rather than the full row so actor_id/target_id are
never written to logs.

[coderabbitai]

⚠️ Potential issue | 🟠 Major | ⚡ Quick win

Guard achievement detail state against stale async responses.

loadDetails() writes shared triggers / linkedCosmeticIds state without verifying that the response still belongs to the currently expanded achievement. If an admin opens A and quickly opens B, A’s slower response can overwrite B’s panel, and the inline trigger actions can then target the wrong record IDs.

Suggested fix

+ const detailsRequestRef = React.useRef(0);+ const openIdRef = React.useRef<string | null>(null);+
const loadDetails = React.useCallback(async (id: string) => {
+ const requestId = ++detailsRequestRef.current;
try {
const [t, l, c] = await Promise.all([
adminListTriggers(id),
adminListAchievementCosmetics(id),
allCosmetics.length ? Promise.resolve({ cosmetics: allCosmetics }) : adminListCosmetics(),
]);
+ if (detailsRequestRef.current !== requestId || openIdRef.current !== id) return;
setTriggers(t.triggers || []);
setLinkedCosmeticIds((l.links || []).map(x => x.cosmetic_id));
if (!allCosmetics.length) setAllCosmetics(c.cosmetics || []);
} catch (err) {
toast.error(`Detail load failed: ${String(err)}`);
}
}, [allCosmetics, toast]);
const toggleOpen = (id: string) => {
- if (openId === id) { setOpenId(null); return; }+ if (openId === id) {+ openIdRef.current = null;+ setOpenId(null);+ return;+ }+ openIdRef.current = id;+ setTriggers([]);+ setLinkedCosmeticIds([]);
setOpenId(id);
loadDetails(id);
};

📝 Committable suggestion

‼️IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change

	constloadDetails=React.useCallback(async(id: string)=>{
	try{
	const[t,l,c]=awaitPromise.all([
	adminListTriggers(id),
	adminListAchievementCosmetics(id),
	allCosmetics.length ? Promise.resolve({cosmetics: allCosmetics}) : adminListCosmetics(),
	]);
	setTriggers(t.triggers||[]);
	setLinkedCosmeticIds((l.links||[]).map(x=>x.cosmetic_id));
	if(!allCosmetics.length)setAllCosmetics(c.cosmetics||[]);
	}catch(err){
	toast.error(`Detail load failed: ${String(err)}`);
	}
	},[allCosmetics,toast]);
	consttoggleOpen=(id: string)=>{
	if(openId===id){setOpenId(null);return;}
	setOpenId(id);
	loadDetails(id);
	};
	constdetailsRequestRef=React.useRef(0);
	constopenIdRef=React.useRef<string|null>(null);
	constloadDetails=React.useCallback(async(id: string)=>{
	constrequestId=++detailsRequestRef.current;
	try{
	const[t,l,c]=awaitPromise.all([
	adminListTriggers(id),
	adminListAchievementCosmetics(id),
	allCosmetics.length ? Promise.resolve({cosmetics: allCosmetics}) : adminListCosmetics(),
	]);
	if(detailsRequestRef.current!==requestId||openIdRef.current!==id)return;
	setTriggers(t.triggers||[]);
	setLinkedCosmeticIds((l.links||[]).map(x=>x.cosmetic_id));
	if(!allCosmetics.length)setAllCosmetics(c.cosmetics||[]);
	}catch(err){
	toast.error(`Detail load failed: ${String(err)}`);
	}
	},[allCosmetics,toast]);
	consttoggleOpen=(id: string)=>{
	if(openId===id){
	openIdRef.current=null;
	setOpenId(null);
	return;
	}
	openIdRef.current=id;
	setTriggers([]);
	setLinkedCosmeticIds([]);
	setOpenId(id);
	loadDetails(id);
	};

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@frontend/src/components/screens/Admin.tsx` around lines 492 - 511,
loadDetails writes shared state (setTriggers, setLinkedCosmeticIds,
setAllCosmetics) unguarded so a stale async response for a previously requested
id can overwrite the currently open achievement; fix by capturing/validating the
expected open id before applying results (e.g., read current openId from a ref
or compare openId at the time of resolution) and only call
setTriggers/setLinkedCosmeticIds/setAllCosmetics when the resolved id still
matches the current openId (or use an AbortController per loadDetails to cancel
stale requests), keeping function names loadDetails, toggleOpen, setTriggers,
setLinkedCosmeticIds, setAllCosmetics and openId as the reference points.

[AndresL230]

Closes#69.

Summary

• Pagination + server-side search on /api/admin/users; new last_sign_in_at column written on every Google callback.
• New endpoints: unapprove user; achievement triggers list/update/delete; achievement_cosmetics + role_cosmetics link/unlink/list; admin_audit_log read; analytics overview (totals + 30-day series + role counts); allowlist approve/revoke.
• Self-protection guards: cannot revoke own admin, cannot revoke last admin, cannot delete admin role, cannot unapprove self.
• Audit log written for every admin mutation via a single sanctioned helper (services/admin_audit.py).
• Frontend Admin.tsx gains Allowlist + Audit tabs and rewires Users / Achievements / Cosmetics / Analytics to the new endpoints — all using existing card / chip / btn / label-micro / var(--accent) design tokens (no new design language).
• assign_role is now idempotent (upsert on (user_id, role_id)); granted_by auto-fills from session.

Schema

• New table admin_audit_log (actor_id, action, target_type, target_id, payload jsonb, created_at) with FK actor_id ON DELETE RESTRICT and read-path indexes on created_at / actor / target.
• New column users.last_sign_in_at and index on users.created_at.

Migration file: backend/db/migration_admin_portal.sql — run once in the Supabase SQL editor.

Test plan

• Run the SQL migration in the dev Supabase project.
• cd backend && python -m pytest tests/test_admin_routes.py tests/test_admin_audit.py tests/test_users_search.py tests/test_supabase.py tests/test_auth_state.py -q (81 tests, all green on this branch).
• cd frontend && npx tsc --noEmit (clean).
• Manual smoke: visit /admin as an admin and walk every tab.
  • Users: paginate, debounced search, approve/unapprove, role assign/revoke.
  • Allowlist: add an email, revoke, re-approve.
  • Roles: try deleting the admin role (must 409); try revoking own admin (409); reassigning an existing role must NOT 500.
  • Achievements: Manage on a row, add/edit/delete a trigger, link/unlink a cosmetic chip.
  • Cosmetics: Manage on a row, link/unlink a role chip.
  • Analytics: totals match DB, sparkline tooltips show daily counts.
  • Audit: every action above appears as a row, filters by action/target_type work.

Out of scope

• Impersonate user (mentioned in feat(admin): admin portal UI — users, allowlist, roles, achievements, cosmetics #69 — touches session minting; deferred).
• EmailStr validation on AllowlistEmailBody — currently email: str. Worth tightening in a follow-up.

Plan

Implementation followed docs/superpowers/plans/2026-05-04-admin-portal.md (committed in this branch).

Summary by CodeRabbit

• New Features
  • Admin dashboard expanded with allowlist management and audit log sections
  • User management improved with pagination, search, and last sign-in date tracking
  • Achievement trigger and cosmetic linking management tools added
  • Admin audit logging tracks all administrative actions system-wide
  • Analytics dashboard displays signup trends and role distribution metrics

[coderabbitai]

📝 Walkthrough

Walkthrough

This PR implements a comprehensive admin portal spanning backend and frontend. It adds database schema for audit logging, a new select_with_count() pagination helper, admin audit tracking across mutations, paginated/searchable user listing, achievement trigger and cosmetic-linking management, role protection rules, allowlist administration, analytics computation, and a fully featured admin UI with tabs for users (paginated/searchable), allowlist, roles, achievements, cosmetics, and audit logs.

Changes

Admin Portal – Complete Implementation

Layer / File(s)	Summary
Database Schema & Indexing backend/db/migration_admin_portal.sql	Creates admin_audit_log table with actor/action/target metadata and timestamps; adds users.last_sign_in_at column; indexes created for time-based and actor-based audit queries.
Database Query Helpers backend/db/connection.py	Adds SupabaseTable.select_with_count() synchronous method that returns (rows, total) by parsing Content-Range header; supports offset alongside limit.
Request Body Models backend/models/__init__.py	Introduces UpdateAchievementTriggerBody, LinkAchievementCosmeticBody, LinkRoleCosmeticBody, and AllowlistEmailBody for admin endpoints.
Audit Service backend/services/admin_audit.py	Adds log_admin_action() to insert audit rows into admin_audit_log with error suppression; ensures audit failures don't block originating operations.
User Pagination & Search backend/services/users_search.py	Adds paginate_users() implementing DB-level pagination when no query provided and decrypt-all-then-filter approach for name/email search; attaches per-user roles and returns {users, total, page, page_size}.
Admin Routes backend/routes/admin.py	Adds/extends 20+ endpoints: role CRUD with admin-role self-protection, achievement management with trigger CRUD and cosmetic linking, cosmetic management, user listing (paginated), approval/unapproval with self-guard, role/achievement-cosmetic linking, allowlist administration, audit log querying, and analytics overview computation. All mutations emit audit events via log_admin_action().
Auth Integration backend/routes/auth.py	Tracks last_sign_in_at on Google OAuth callback (both existing and new users); adds test helper _stamp_last_sign_in_for_test() to support test scenarios without full OAuth flow.
Backend Tests backend/tests/test_admin_audit.py, test_admin_routes.py, test_auth_state.py, test_supabase.py, test_users_search.py	Comprehensive unit and integration test coverage: audit logging (row insertion and error suppression), user pagination with/without search and role attachment, role protection rules, achievement/cosmetic/allowlist/audit operations, analytics computation, and select_with_count() parsing.
Frontend Types frontend/src/lib/types.ts	Adds AllowlistEmail, AchievementTrigger, AdminAuditEntry, analytics-related types (AnalyticsTotals, AnalyticsDayPoint, AnalyticsRoleCount, AnalyticsOverview), AdminUserListItem, and PaginatedUsers; extends Cosmetic with optional unlock_source field.
Frontend API Wrappers frontend/src/lib/api.ts	Adds ~17 new admin helpers for paginated user listing (with search), user unapproval, role-cosmetic linking, achievement trigger CRUD, achievement-cosmetic linking, allowlist management, audit log querying with filters, and analytics overview retrieval.
Frontend Admin UI frontend/src/components/screens/Admin.tsx	Expands Admin component to include Users tab (paginated with debounced search, approve/unapprove, last-seen display), Allowlist tab (email list with approve/revoke), Audit tab (paginated audit log with action/target filters and payload display), and extends Roles, Achievements, Cosmetics tabs with row-expansion UIs for managing linked cosmetics/roles and achievement triggers. Refactors Analytics to use adminAnalyticsOverview() with metric cards and sparklines. Upgrades CatalogRow component to support expandable management sections.

Sequence Diagram

sequenceDiagram
participant Admin as Admin User
participant FrontendUI as Frontend UI
participant API as Admin API
participant Auth as Auth Service
participant DB as Database
rect rgba(100, 200, 150, 0.5)
note over Admin,DB: User Search & Pagination Flow
Admin->>FrontendUI: Enter search query, navigate to page 2
FrontendUI->>FrontendUI: Debounce search input
FrontendUI->>API: adminFetchUsers({q: "alice", page: 2, page_size: 50})
API->>Auth: get_session_user_id() → verify admin
API->>DB: paginate_users(q="alice", page=2, page_size=50)
alt Query provided (search)
DB->>DB: Decrypt all users, filter by substring match
DB->>DB: Compute total from filtered set, slice page
else No query (list)
DB->>DB: select_with_count (offset=50, limit=50)
end
DB->>DB: Attach roles per user from user_roles
DB-->>API: {users: [...], total: 137, page: 2, page_size: 50}
API-->>FrontendUI: PaginatedUsers response
FrontendUI->>FrontendUI: Render users table with Last Seen, Joined dates
FrontendUI-->>Admin: Display page 2 of 3, approve/unapprove buttons
end
rect rgba(150, 180, 220, 0.5)
note over Admin,DB: Admin Audit Logging Flow
Admin->>FrontendUI: Click "Approve" button for user
FrontendUI->>API: adminApproveUser(userId)
API->>Auth: get_session_user_id() → actor_id
API->>DB: UPDATE users SET is_approved=true WHERE id=userId
API->>DB: log_admin_action(actor_id, "user.approve", "user", userId, {...})
DB->>DB: INSERT INTO admin_audit_log (actor_id, action, target_type, target_id, created_at)
DB-->>API: Audit row inserted (or error caught & logged)
API-->>FrontendUI: {approved: true}
FrontendUI-->>Admin: Show success, refresh user list
Admin->>FrontendUI: Visit Audit tab
FrontendUI->>API: adminAuditLog({page: 1, action: "user.approve"})
DB->>DB: SELECT * FROM admin_audit_log WHERE action='user.approve' ORDER BY created_at DESC
DB-->>API: Paginated audit rows
API-->>FrontendUI: Audit entries with timestamps, payload JSON
FrontendUI-->>Admin: Display filtered audit log
end
rect rgba(200, 150, 180, 0.5)
note over Admin,DB: Role-Cosmetic Linking Flow
Admin->>FrontendUI: Expand role row, toggle linked cosmetic
FrontendUI->>API: adminLinkRoleCosmetic(roleId, cosmeticId)
API->>Auth: get_session_user_id() → actor_id
API->>DB: UPSERT INTO role_cosmetics (role_id, cosmetic_id)
API->>DB: log_admin_action(actor_id, "role_cosmetic.link", "role_cosmetic", null, {...})
DB->>DB: INSERT INTO admin_audit_log
DB-->>API: Link upserted, audit logged
API-->>FrontendUI: {linked: true}
FrontendUI->>FrontendUI: Update expanded row UI to reflect new link
FrontendUI-->>Admin: Show cosmetic chip as linked in role details
end

Loading

Estimated Code Review Effort

🎯 4 (Complex) | ⏱️ ~45 minutes

Possibly Related PRs

• SaplingLearn/Sapling#56: Both PRs modify backend/routes/auth.py and the users table schema; PR #56 adds user field persistence during OAuth callback while this PR extends that to include last_sign_in_at tracking.

Poem

🐰 A warren of admins, now armed and aware,
With audit trails woven through each admin's care,
Cosmetics and triggers link up with grace,
While pages of users scroll at a measured pace,
The portal is gleaming—no more fumbling in the dark!

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 9.26% which is insufficient. The required threshold is 80.00%.	Write docstrings for the functions missing them to satisfy the coverage threshold.

✅ Passed checks (4 passed)

Check name	Status	Explanation
Title check	✅ Passed	The title accurately reflects the main changes: pagination, audit log, allowlist, and analytics for the admin portal. It is specific, concise, and directly related to the primary changeset.
Description check	✅ Passed	The description is comprehensive and follows the template structure with Summary, Changes Made (implicit via detailed sections), Testing checklist, and Out of Scope items. All key implementation details are documented.
Linked Issues check	✅ Passed	The PR fully addresses issue #69 objectives: pagination and search for users [#69], unapprove endpoint [#69], audit logging for mutations [#69], role protection guards [#69], achievement triggers management [#69], role/achievement cosmetic linking [#69], allowlist endpoints [#69], analytics overview [#69], and role assignment idempotency [#69].
Out of Scope Changes check	✅ Passed	All code changes are directly aligned with the stated objectives. Out-of-scope items (impersonation, stricter email validation) are explicitly deferred and not included in the changeset.

_{✏️ Tip: You can configure your own custom pre-merge checks in the settings.}

✨ Finishing Touches

📝 Generate docstrings

• Create stacked PR
• Commit on current branch

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Commit unit tests in branch feat/admin-portal

---

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

[cloudflare-workers-and-pages]

Deploying with Cloudflare Workers

The latest updates on your project. Learn more about integrating Git with Workers.

Status	Name	Latest Commit	Preview URL	Updated (UTC)
✅ Deployment successful! View logs	frontend	1c23a86	Commit Preview URL Branch Preview URL	May 04 2026, 11:23 PM

[coderabbitai]

Actionable comments posted: 4

🧹 Nitpick comments (2)

backend/services/users_search.py (1)

20-26: ⚡ Quick win

N+1 queries in _attach_roles — collapse to a single batch request

Each call to _attach_roles issues one Supabase REST request per user. With _MAX_PAGE_SIZE = 200 that's up to 200 sequential HTTP round-trips per admin page load. PostgREST's in.(...) filter fetches all roles in one request:

♻️ Proposed refactor

 def _attach_roles(users: list[dict]) -> None:
- for user in users:- rows = table("user_roles").select(- "roles(id,name,slug,color,icon,description,is_staff_assigned,is_earnable,display_priority)",- filters={"user_id": f"eq.{user['id']}"},- )- user["roles"] = [r["roles"] for r in (rows or []) if r.get("roles")]+ if not users:+ return+ ids = ",".join(u["id"] for u in users)+ all_rows = table("user_roles").select(+ "user_id,roles(id,name,slug,color,icon,description,is_staff_assigned,is_earnable,display_priority)",+ filters={"user_id": f"in.({ids})"},+ ) or []+ role_map: dict[str, list] = {}+ for r in all_rows:+ if r.get("roles"):+ role_map.setdefault(r["user_id"], []).append(r["roles"])+ for user in users:+ user["roles"] = role_map.get(user["id"], [])

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@backend/services/users_search.py` around lines 20 - 26, The _attach_roles
function issues an individual Supabase/PostgREST request per user causing N+1
queries; change it to fetch all roles in one batch by querying
table("user_roles") with a single select using the user_id
in.(<list_of_user_ids>) filter, retrieving "user_id" and the nested "roles(...)"
fields, then group the returned rows by user_id and assign user["roles"] =
[r["roles"] for r in grouped[user_id]] (or [] if none) for each user; keep
function name _attach_roles, use the same table("user_roles") select call but
replace per-user filters with the in filter and ensure you handle empty input
lists and absent role entries.

backend/routes/auth.py (1)

44-51: ⚡ Quick win

Move _stamp_last_sign_in_for_test out of the production module and hoist the datetime import

Two related issues:

1. _stamp_last_sign_in_for_test is a test-only helper sitting in the production auth.py module. Any package import of routes.auth exposes it, and it exists only because datetime.now is buried inside conditional branches where standard monkeypatch can't reach it.
2. from datetime import datetime as _dt, timezone as _tz is duplicated in three places (lines 47, 325, 343). Python caches module imports, so this isn't a correctness bug, but it's unnecessarily noisy.

The cleanest resolution is to promote the import to module level, which then makes routes.auth.datetime directly patchable in tests and removes the need for the test seam entirely:

♻️ Proposed refactor

+from datetime import datetime as _dt, timezone as _tz
import json
...
-def _stamp_last_sign_in_for_test(user_id: str) -> None:- """Test seam: write last_sign_in_at ..."""- from datetime import datetime, timezone- table("users").update(- {"last_sign_in_at": datetime.now(timezone.utc).isoformat()},- filters={"id": f"eq.{user_id}"},- )--
...
if existing:
- from datetime import datetime as _dt, timezone as _tz
table("users").update({
...
"last_sign_in_at": _dt.now(_tz.utc).isoformat(),
}, ...)
else:
- from datetime import datetime as _dt, timezone as _tz
table("users").insert({
...
"last_sign_in_at": _dt.now(_tz.utc).isoformat(),
})

With _dt at module level, test_auth_state.py can replace the test helper with a direct monkeypatch:

# test_auth_state.pyimportroutes.authasauth_moduledeftest_callback_stamps_last_sign_in(monkeypatch):
fixed=_dt(2024, 1, 1, tzinfo=_tz.utc)
monkeypatch.setattr(auth_module, "_dt", lambda*a, **kw: fixed)
...

Also applies to: 325-325, 343-343

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@backend/routes/auth.py` around lines 44 - 51, Move the test-only helper
_stamp_last_sign_in_for_test out of the production routes.auth module into the
test suite and hoist the datetime import to module scope: add a module-level
import like "from datetime import datetime as _dt, timezone as _tz" in
routes.auth, replace any local in-function imports and uses of
datetime.now(timezone.utc) with _dt(..., tzinfo=_tz.utc) (or calls to
_dt.now(_tz.utc)), and remove the test helper from the production file so tests
can monkeypatch the module-level _dt/_tz instead; update any references to
_stamp_last_sign_in_for_test in tests to call the moved helper in the test
package.

🤖 Prompt for all review comments with AI agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@backend/routes/admin.py`:
- Around line 558-570: The approvals series is currently counted by signup date
(using created_at) rather than the approval timestamp; update the logic in the
loop that builds by_day_approvals/approvals_by_day to use an approval timestamp
(e.g., check for an approved_at or similar field on each user instead of
created_at) and parse that timestamp into a date (like you do with created_at)
before incrementing by_day_approvals[d.isoformat()]. If there is no approval
timestamp available, either add/compute one at approval time (e.g., set
approved_at when is_approved transitions true) or rename the metric from
“Approvals” to something like “Approved users by signup date” to reflect the
current derivation (refer to variables/users, created_at, is_approved,
by_day_approvals, approvals_by_day, window).
- Around line 95-109: The code currently allows a client to override the grantor
via body.granted_by; instead, always derive the grantor from the session: set
granted_by = get_session_user_id(request) (i.e., the existing actor variable)
and remove usage of body.granted_by when building the upsert payload and the
log_admin_action payload so both table("user_roles").upsert and log_admin_action
use the trusted actor value only.
In `@backend/services/admin_audit.py`:
- Line 31: The log currently calls log.exception("admin_audit_log write failed:
%s", row) which will serialize sensitive identifiers (row.actor_id,
row.target_id); change the logging to avoid including identifying fields by
creating a sanitized version of row that omits or redacts actor_id and target_id
and log that instead (or log only non-identifying metadata such as event type,
timestamp, and error context). Update the call site using log.exception to pass
the sanitized data or a minimal message and error context rather than the full
row so actor_id/target_id are never written to logs.
In `@frontend/src/components/screens/Admin.tsx`:
- Around line 492-511: loadDetails writes shared state (setTriggers,
setLinkedCosmeticIds, setAllCosmetics) unguarded so a stale async response for a
previously requested id can overwrite the currently open achievement; fix by
capturing/validating the expected open id before applying results (e.g., read
current openId from a ref or compare openId at the time of resolution) and only
call setTriggers/setLinkedCosmeticIds/setAllCosmetics when the resolved id still
matches the current openId (or use an AbortController per loadDetails to cancel
stale requests), keeping function names loadDetails, toggleOpen, setTriggers,
setLinkedCosmeticIds, setAllCosmetics and openId as the reference points.
---
Nitpick comments:
In `@backend/routes/auth.py`:
- Around line 44-51: Move the test-only helper _stamp_last_sign_in_for_test out
of the production routes.auth module into the test suite and hoist the datetime
import to module scope: add a module-level import like "from datetime import
datetime as _dt, timezone as _tz" in routes.auth, replace any local in-function
imports and uses of datetime.now(timezone.utc) with _dt(..., tzinfo=_tz.utc) (or
calls to _dt.now(_tz.utc)), and remove the test helper from the production file
so tests can monkeypatch the module-level _dt/_tz instead; update any references
to _stamp_last_sign_in_for_test in tests to call the moved helper in the test
package.
In `@backend/services/users_search.py`:
- Around line 20-26: The _attach_roles function issues an individual
Supabase/PostgREST request per user causing N+1 queries; change it to fetch all
roles in one batch by querying table("user_roles") with a single select using
the user_id in.(<list_of_user_ids>) filter, retrieving "user_id" and the nested
"roles(...)" fields, then group the returned rows by user_id and assign
user["roles"] = [r["roles"] for r in grouped[user_id]] (or [] if none) for each
user; keep function name _attach_roles, use the same table("user_roles") select
call but replace per-user filters with the in filter and ensure you handle empty
input lists and absent role entries.

🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

• Push a commit to this branch (recommended)
• Create a new PR with the fixes

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro Plus

Run ID: e976a667-d4b6-4fac-9347-f27bc7d43539

📥 Commits

Reviewing files that changed from the base of the PR and between db5ba48 and 1c23a86.

📒 Files selected for processing (16)

• backend/db/connection.py
• backend/db/migration_admin_portal.sql
• backend/models/__init__.py
• backend/routes/admin.py
• backend/routes/auth.py
• backend/services/admin_audit.py
• backend/services/users_search.py
• backend/tests/test_admin_audit.py
• backend/tests/test_admin_routes.py
• backend/tests/test_auth_state.py
• backend/tests/test_supabase.py
• backend/tests/test_users_search.py
• docs/superpowers/plans/2026-05-04-admin-portal.md
• frontend/src/components/screens/Admin.tsx
• frontend/src/lib/api.ts
• frontend/src/lib/types.ts

[coderabbitai]

⚠️ Potential issue | 🟠 Major | ⚡ Quick win

Do not trust client-supplied granted_by.

granted_by = body.granted_by or actor lets the caller forge who granted the role. That breaks the server-side provenance this PR is adding and makes both the row data and the audit trail unreliable. Stamp granted_by from get_session_user_id(request) unconditionally.

Suggested fix

 def assign_role(body: AssignRoleBody, request: Request):
require_admin(request)
actor = get_session_user_id(request)
- granted_by = body.granted_by or actor+ granted_by = actor
table("user_roles").upsert(
{
"user_id": body.user_id,
"role_id": body.role_id,
"granted_by": granted_by,

📝 Committable suggestion

‼️IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change

	actor=get_session_user_id(request)
	granted_by=body.granted_byoractor
	table("user_roles").upsert(
	{
	"user_id": body.user_id,
	"role_id": body.role_id,
	"granted_by": granted_by,
	"granted_at": datetime.now(timezone.utc).isoformat(),
	},
	on_conflict="user_id,role_id",
	)
	log_admin_action(
	actor_id=actor, action="role.assign", target_type="role", target_id=body.role_id,
	payload={"user_id": body.user_id, "granted_by": granted_by},
	)
	actor=get_session_user_id(request)
	granted_by=actor
	table("user_roles").upsert(
	{
	"user_id": body.user_id,
	"role_id": body.role_id,
	"granted_by": granted_by,
	"granted_at": datetime.now(timezone.utc).isoformat(),
	},
	on_conflict="user_id,role_id",
	)
	log_admin_action(
	actor_id=actor, action="role.assign", target_type="role", target_id=body.role_id,
	payload={"user_id": body.user_id, "granted_by": granted_by},
	)

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@backend/routes/admin.py` around lines 95 - 109, The code currently allows a
client to override the grantor via body.granted_by; instead, always derive the
grantor from the session: set granted_by = get_session_user_id(request) (i.e.,
the existing actor variable) and remove usage of body.granted_by when building
the upsert payload and the log_admin_action payload so both
table("user_roles").upsert and log_admin_action use the trusted actor value
only.

[coderabbitai]

⚠️ Potential issue | 🟠 Major | 🏗️ Heavy lift

The “approvals” series is grouped by signup date, not approval date.

approvals_by_day is incremented from created_at for users whose current is_approved is true. A user approved today will be counted on their original signup day, so the Analytics tab’s “Approvals · last 30 days” chart is wrong. This needs either a real approval timestamp or a renamed metric that matches the current derivation.

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@backend/routes/admin.py` around lines 558 - 570, The approvals series is
currently counted by signup date (using created_at) rather than the approval
timestamp; update the logic in the loop that builds
by_day_approvals/approvals_by_day to use an approval timestamp (e.g., check for
an approved_at or similar field on each user instead of created_at) and parse
that timestamp into a date (like you do with created_at) before incrementing
by_day_approvals[d.isoformat()]. If there is no approval timestamp available,
either add/compute one at approval time (e.g., set approved_at when is_approved
transitions true) or rename the metric from “Approvals” to something like
“Approved users by signup date” to reflect the current derivation (refer to
variables/users, created_at, is_approved, by_day_approvals, approvals_by_day,
window).

[coderabbitai]

⚠️ Potential issue | 🟠 Major | ⚡ Quick win

log.exception serializes user identifiers (actor_id, target_id) into application logs

row includes actor_id and target_id, which encode Google sub-claim–based user IDs — persistent identifiers that can be linked back to real persons. If application logs are forwarded to external aggregators this violates GDPR/CCPA guidance against logging user identifiers.

🔒 Proposed fix — log only non-identifying metadata

- log.exception("admin_audit_log write failed: %s", row)+ log.exception(+ "admin_audit_log write failed action=%s target_type=%s",+ row.get("action"),+ row.get("target_type"),+ )

As per coding guidelines, logging sensitive data such as user identifiers is classified as a compliance/privacy risk (GDPR/CCPA).

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@backend/services/admin_audit.py` at line 31, The log currently calls
log.exception("admin_audit_log write failed: %s", row) which will serialize
sensitive identifiers (row.actor_id, row.target_id); change the logging to avoid
including identifying fields by creating a sanitized version of row that omits
or redacts actor_id and target_id and log that instead (or log only
non-identifying metadata such as event type, timestamp, and error context).
Update the call site using log.exception to pass the sanitized data or a minimal
message and error context rather than the full row so actor_id/target_id are
never written to logs.

[coderabbitai]

⚠️ Potential issue | 🟠 Major | ⚡ Quick win

Guard achievement detail state against stale async responses.

loadDetails() writes shared triggers / linkedCosmeticIds state without verifying that the response still belongs to the currently expanded achievement. If an admin opens A and quickly opens B, A’s slower response can overwrite B’s panel, and the inline trigger actions can then target the wrong record IDs.

Suggested fix

+ const detailsRequestRef = React.useRef(0);+ const openIdRef = React.useRef<string | null>(null);+
const loadDetails = React.useCallback(async (id: string) => {
+ const requestId = ++detailsRequestRef.current;
try {
const [t, l, c] = await Promise.all([
adminListTriggers(id),
adminListAchievementCosmetics(id),
allCosmetics.length ? Promise.resolve({ cosmetics: allCosmetics }) : adminListCosmetics(),
]);
+ if (detailsRequestRef.current !== requestId || openIdRef.current !== id) return;
setTriggers(t.triggers || []);
setLinkedCosmeticIds((l.links || []).map(x => x.cosmetic_id));
if (!allCosmetics.length) setAllCosmetics(c.cosmetics || []);
} catch (err) {
toast.error(`Detail load failed: ${String(err)}`);
}
}, [allCosmetics, toast]);
const toggleOpen = (id: string) => {
- if (openId === id) { setOpenId(null); return; }+ if (openId === id) {+ openIdRef.current = null;+ setOpenId(null);+ return;+ }+ openIdRef.current = id;+ setTriggers([]);+ setLinkedCosmeticIds([]);
setOpenId(id);
loadDetails(id);
};

📝 Committable suggestion

‼️IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change

	constloadDetails=React.useCallback(async(id: string)=>{
	try{
	const[t,l,c]=awaitPromise.all([
	adminListTriggers(id),
	adminListAchievementCosmetics(id),
	allCosmetics.length ? Promise.resolve({cosmetics: allCosmetics}) : adminListCosmetics(),
	]);
	setTriggers(t.triggers||[]);
	setLinkedCosmeticIds((l.links||[]).map(x=>x.cosmetic_id));
	if(!allCosmetics.length)setAllCosmetics(c.cosmetics||[]);
	}catch(err){
	toast.error(`Detail load failed: ${String(err)}`);
	}
	},[allCosmetics,toast]);
	consttoggleOpen=(id: string)=>{
	if(openId===id){setOpenId(null);return;}
	setOpenId(id);
	loadDetails(id);
	};
	constdetailsRequestRef=React.useRef(0);
	constopenIdRef=React.useRef<string|null>(null);
	constloadDetails=React.useCallback(async(id: string)=>{
	constrequestId=++detailsRequestRef.current;
	try{
	const[t,l,c]=awaitPromise.all([
	adminListTriggers(id),
	adminListAchievementCosmetics(id),
	allCosmetics.length ? Promise.resolve({cosmetics: allCosmetics}) : adminListCosmetics(),
	]);
	if(detailsRequestRef.current!==requestId||openIdRef.current!==id)return;
	setTriggers(t.triggers||[]);
	setLinkedCosmeticIds((l.links||[]).map(x=>x.cosmetic_id));
	if(!allCosmetics.length)setAllCosmetics(c.cosmetics||[]);
	}catch(err){
	toast.error(`Detail load failed: ${String(err)}`);
	}
	},[allCosmetics,toast]);
	consttoggleOpen=(id: string)=>{
	if(openId===id){
	openIdRef.current=null;
	setOpenId(null);
	return;
	}
	openIdRef.current=id;
	setTriggers([]);
	setLinkedCosmeticIds([]);
	setOpenId(id);
	loadDetails(id);
	};

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@frontend/src/components/screens/Admin.tsx` around lines 492 - 511,
loadDetails writes shared state (setTriggers, setLinkedCosmeticIds,
setAllCosmetics) unguarded so a stale async response for a previously requested
id can overwrite the currently open achievement; fix by capturing/validating the
expected open id before applying results (e.g., read current openId from a ref
or compare openId at the time of resolution) and only call
setTriggers/setLinkedCosmeticIds/setAllCosmetics when the resolved id still
matches the current openId (or use an AbortController per loadDetails to cancel
stale requests), keeping function names loadDetails, toggleOpen, setTriggers,
setLinkedCosmeticIds, setAllCosmetics and openId as the reference points.

[AndresL230]

Closes#69.

Summary

• Pagination + server-side search on /api/admin/users; new last_sign_in_at column written on every Google callback.
• New endpoints: unapprove user; achievement triggers list/update/delete; achievement_cosmetics + role_cosmetics link/unlink/list; admin_audit_log read; analytics overview (totals + 30-day series + role counts); allowlist approve/revoke.
• Self-protection guards: cannot revoke own admin, cannot revoke last admin, cannot delete admin role, cannot unapprove self.
• Audit log written for every admin mutation via a single sanctioned helper (services/admin_audit.py).
• Frontend Admin.tsx gains Allowlist + Audit tabs and rewires Users / Achievements / Cosmetics / Analytics to the new endpoints — all using existing card / chip / btn / label-micro / var(--accent) design tokens (no new design language).
• assign_role is now idempotent (upsert on (user_id, role_id)); granted_by auto-fills from session.

Schema

• New table admin_audit_log (actor_id, action, target_type, target_id, payload jsonb, created_at) with FK actor_id ON DELETE RESTRICT and read-path indexes on created_at / actor / target.
• New column users.last_sign_in_at and index on users.created_at.

Migration file: backend/db/migration_admin_portal.sql — run once in the Supabase SQL editor.

Test plan

• Run the SQL migration in the dev Supabase project.
• cd backend && python -m pytest tests/test_admin_routes.py tests/test_admin_audit.py tests/test_users_search.py tests/test_supabase.py tests/test_auth_state.py -q (81 tests, all green on this branch).
• cd frontend && npx tsc --noEmit (clean).
• Manual smoke: visit /admin as an admin and walk every tab.
  • Users: paginate, debounced search, approve/unapprove, role assign/revoke.
  • Allowlist: add an email, revoke, re-approve.
  • Roles: try deleting the admin role (must 409); try revoking own admin (409); reassigning an existing role must NOT 500.
  • Achievements: Manage on a row, add/edit/delete a trigger, link/unlink a cosmetic chip.
  • Cosmetics: Manage on a row, link/unlink a role chip.
  • Analytics: totals match DB, sparkline tooltips show daily counts.
  • Audit: every action above appears as a row, filters by action/target_type work.

Out of scope

• Impersonate user (mentioned in feat(admin): admin portal UI — users, allowlist, roles, achievements, cosmetics #69 — touches session minting; deferred).
• EmailStr validation on AllowlistEmailBody — currently email: str. Worth tightening in a follow-up.

Plan

Implementation followed docs/superpowers/plans/2026-05-04-admin-portal.md (committed in this branch).

Summary by CodeRabbit

• New Features
  • Admin dashboard expanded with allowlist management and audit log sections
  • User management improved with pagination, search, and last sign-in date tracking
  • Achievement trigger and cosmetic linking management tools added
  • Admin audit logging tracks all administrative actions system-wide
  • Analytics dashboard displays signup trends and role distribution metrics

[coderabbitai]

📝 Walkthrough

Walkthrough

This PR implements a comprehensive admin portal spanning backend and frontend. It adds database schema for audit logging, a new select_with_count() pagination helper, admin audit tracking across mutations, paginated/searchable user listing, achievement trigger and cosmetic-linking management, role protection rules, allowlist administration, analytics computation, and a fully featured admin UI with tabs for users (paginated/searchable), allowlist, roles, achievements, cosmetics, and audit logs.

Changes

Admin Portal – Complete Implementation

Layer / File(s)	Summary
Database Schema & Indexing backend/db/migration_admin_portal.sql	Creates admin_audit_log table with actor/action/target metadata and timestamps; adds users.last_sign_in_at column; indexes created for time-based and actor-based audit queries.
Database Query Helpers backend/db/connection.py	Adds SupabaseTable.select_with_count() synchronous method that returns (rows, total) by parsing Content-Range header; supports offset alongside limit.
Request Body Models backend/models/__init__.py	Introduces UpdateAchievementTriggerBody, LinkAchievementCosmeticBody, LinkRoleCosmeticBody, and AllowlistEmailBody for admin endpoints.
Audit Service backend/services/admin_audit.py	Adds log_admin_action() to insert audit rows into admin_audit_log with error suppression; ensures audit failures don't block originating operations.
User Pagination & Search backend/services/users_search.py	Adds paginate_users() implementing DB-level pagination when no query provided and decrypt-all-then-filter approach for name/email search; attaches per-user roles and returns {users, total, page, page_size}.
Admin Routes backend/routes/admin.py	Adds/extends 20+ endpoints: role CRUD with admin-role self-protection, achievement management with trigger CRUD and cosmetic linking, cosmetic management, user listing (paginated), approval/unapproval with self-guard, role/achievement-cosmetic linking, allowlist administration, audit log querying, and analytics overview computation. All mutations emit audit events via log_admin_action().
Auth Integration backend/routes/auth.py	Tracks last_sign_in_at on Google OAuth callback (both existing and new users); adds test helper _stamp_last_sign_in_for_test() to support test scenarios without full OAuth flow.
Backend Tests backend/tests/test_admin_audit.py, test_admin_routes.py, test_auth_state.py, test_supabase.py, test_users_search.py	Comprehensive unit and integration test coverage: audit logging (row insertion and error suppression), user pagination with/without search and role attachment, role protection rules, achievement/cosmetic/allowlist/audit operations, analytics computation, and select_with_count() parsing.
Frontend Types frontend/src/lib/types.ts	Adds AllowlistEmail, AchievementTrigger, AdminAuditEntry, analytics-related types (AnalyticsTotals, AnalyticsDayPoint, AnalyticsRoleCount, AnalyticsOverview), AdminUserListItem, and PaginatedUsers; extends Cosmetic with optional unlock_source field.
Frontend API Wrappers frontend/src/lib/api.ts	Adds ~17 new admin helpers for paginated user listing (with search), user unapproval, role-cosmetic linking, achievement trigger CRUD, achievement-cosmetic linking, allowlist management, audit log querying with filters, and analytics overview retrieval.
Frontend Admin UI frontend/src/components/screens/Admin.tsx	Expands Admin component to include Users tab (paginated with debounced search, approve/unapprove, last-seen display), Allowlist tab (email list with approve/revoke), Audit tab (paginated audit log with action/target filters and payload display), and extends Roles, Achievements, Cosmetics tabs with row-expansion UIs for managing linked cosmetics/roles and achievement triggers. Refactors Analytics to use adminAnalyticsOverview() with metric cards and sparklines. Upgrades CatalogRow component to support expandable management sections.

Sequence Diagram

sequenceDiagram
participant Admin as Admin User
participant FrontendUI as Frontend UI
participant API as Admin API
participant Auth as Auth Service
participant DB as Database
rect rgba(100, 200, 150, 0.5)
note over Admin,DB: User Search & Pagination Flow
Admin->>FrontendUI: Enter search query, navigate to page 2
FrontendUI->>FrontendUI: Debounce search input
FrontendUI->>API: adminFetchUsers({q: "alice", page: 2, page_size: 50})
API->>Auth: get_session_user_id() → verify admin
API->>DB: paginate_users(q="alice", page=2, page_size=50)
alt Query provided (search)
DB->>DB: Decrypt all users, filter by substring match
DB->>DB: Compute total from filtered set, slice page
else No query (list)
DB->>DB: select_with_count (offset=50, limit=50)
end
DB->>DB: Attach roles per user from user_roles
DB-->>API: {users: [...], total: 137, page: 2, page_size: 50}
API-->>FrontendUI: PaginatedUsers response
FrontendUI->>FrontendUI: Render users table with Last Seen, Joined dates
FrontendUI-->>Admin: Display page 2 of 3, approve/unapprove buttons
end
rect rgba(150, 180, 220, 0.5)
note over Admin,DB: Admin Audit Logging Flow
Admin->>FrontendUI: Click "Approve" button for user
FrontendUI->>API: adminApproveUser(userId)
API->>Auth: get_session_user_id() → actor_id
API->>DB: UPDATE users SET is_approved=true WHERE id=userId
API->>DB: log_admin_action(actor_id, "user.approve", "user", userId, {...})
DB->>DB: INSERT INTO admin_audit_log (actor_id, action, target_type, target_id, created_at)
DB-->>API: Audit row inserted (or error caught & logged)
API-->>FrontendUI: {approved: true}
FrontendUI-->>Admin: Show success, refresh user list
Admin->>FrontendUI: Visit Audit tab
FrontendUI->>API: adminAuditLog({page: 1, action: "user.approve"})
DB->>DB: SELECT * FROM admin_audit_log WHERE action='user.approve' ORDER BY created_at DESC
DB-->>API: Paginated audit rows
API-->>FrontendUI: Audit entries with timestamps, payload JSON
FrontendUI-->>Admin: Display filtered audit log
end
rect rgba(200, 150, 180, 0.5)
note over Admin,DB: Role-Cosmetic Linking Flow
Admin->>FrontendUI: Expand role row, toggle linked cosmetic
FrontendUI->>API: adminLinkRoleCosmetic(roleId, cosmeticId)
API->>Auth: get_session_user_id() → actor_id
API->>DB: UPSERT INTO role_cosmetics (role_id, cosmetic_id)
API->>DB: log_admin_action(actor_id, "role_cosmetic.link", "role_cosmetic", null, {...})
DB->>DB: INSERT INTO admin_audit_log
DB-->>API: Link upserted, audit logged
API-->>FrontendUI: {linked: true}
FrontendUI->>FrontendUI: Update expanded row UI to reflect new link
FrontendUI-->>Admin: Show cosmetic chip as linked in role details
end

Loading

Estimated Code Review Effort

🎯 4 (Complex) | ⏱️ ~45 minutes

Possibly Related PRs

• SaplingLearn/Sapling#56: Both PRs modify backend/routes/auth.py and the users table schema; PR #56 adds user field persistence during OAuth callback while this PR extends that to include last_sign_in_at tracking.

Poem

🐰 A warren of admins, now armed and aware,
With audit trails woven through each admin's care,
Cosmetics and triggers link up with grace,
While pages of users scroll at a measured pace,
The portal is gleaming—no more fumbling in the dark!

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 9.26% which is insufficient. The required threshold is 80.00%.	Write docstrings for the functions missing them to satisfy the coverage threshold.

✅ Passed checks (4 passed)

Check name	Status	Explanation
Title check	✅ Passed	The title accurately reflects the main changes: pagination, audit log, allowlist, and analytics for the admin portal. It is specific, concise, and directly related to the primary changeset.
Description check	✅ Passed	The description is comprehensive and follows the template structure with Summary, Changes Made (implicit via detailed sections), Testing checklist, and Out of Scope items. All key implementation details are documented.
Linked Issues check	✅ Passed	The PR fully addresses issue #69 objectives: pagination and search for users [#69], unapprove endpoint [#69], audit logging for mutations [#69], role protection guards [#69], achievement triggers management [#69], role/achievement cosmetic linking [#69], allowlist endpoints [#69], analytics overview [#69], and role assignment idempotency [#69].
Out of Scope Changes check	✅ Passed	All code changes are directly aligned with the stated objectives. Out-of-scope items (impersonation, stricter email validation) are explicitly deferred and not included in the changeset.

_{✏️ Tip: You can configure your own custom pre-merge checks in the settings.}

✨ Finishing Touches

📝 Generate docstrings

• Create stacked PR
• Commit on current branch

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Commit unit tests in branch feat/admin-portal

---

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

[cloudflare-workers-and-pages]

Deploying with Cloudflare Workers

The latest updates on your project. Learn more about integrating Git with Workers.

Status	Name	Latest Commit	Preview URL	Updated (UTC)
✅ Deployment successful! View logs	frontend	1c23a86	Commit Preview URL Branch Preview URL	May 04 2026, 11:23 PM

[coderabbitai]

Actionable comments posted: 4

🧹 Nitpick comments (2)

backend/services/users_search.py (1)

20-26: ⚡ Quick win

N+1 queries in _attach_roles — collapse to a single batch request

Each call to _attach_roles issues one Supabase REST request per user. With _MAX_PAGE_SIZE = 200 that's up to 200 sequential HTTP round-trips per admin page load. PostgREST's in.(...) filter fetches all roles in one request:

♻️ Proposed refactor

 def _attach_roles(users: list[dict]) -> None:
- for user in users:- rows = table("user_roles").select(- "roles(id,name,slug,color,icon,description,is_staff_assigned,is_earnable,display_priority)",- filters={"user_id": f"eq.{user['id']}"},- )- user["roles"] = [r["roles"] for r in (rows or []) if r.get("roles")]+ if not users:+ return+ ids = ",".join(u["id"] for u in users)+ all_rows = table("user_roles").select(+ "user_id,roles(id,name,slug,color,icon,description,is_staff_assigned,is_earnable,display_priority)",+ filters={"user_id": f"in.({ids})"},+ ) or []+ role_map: dict[str, list] = {}+ for r in all_rows:+ if r.get("roles"):+ role_map.setdefault(r["user_id"], []).append(r["roles"])+ for user in users:+ user["roles"] = role_map.get(user["id"], [])

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@backend/services/users_search.py` around lines 20 - 26, The _attach_roles
function issues an individual Supabase/PostgREST request per user causing N+1
queries; change it to fetch all roles in one batch by querying
table("user_roles") with a single select using the user_id
in.(<list_of_user_ids>) filter, retrieving "user_id" and the nested "roles(...)"
fields, then group the returned rows by user_id and assign user["roles"] =
[r["roles"] for r in grouped[user_id]] (or [] if none) for each user; keep
function name _attach_roles, use the same table("user_roles") select call but
replace per-user filters with the in filter and ensure you handle empty input
lists and absent role entries.

backend/routes/auth.py (1)

44-51: ⚡ Quick win

Move _stamp_last_sign_in_for_test out of the production module and hoist the datetime import

Two related issues:

1. _stamp_last_sign_in_for_test is a test-only helper sitting in the production auth.py module. Any package import of routes.auth exposes it, and it exists only because datetime.now is buried inside conditional branches where standard monkeypatch can't reach it.
2. from datetime import datetime as _dt, timezone as _tz is duplicated in three places (lines 47, 325, 343). Python caches module imports, so this isn't a correctness bug, but it's unnecessarily noisy.

The cleanest resolution is to promote the import to module level, which then makes routes.auth.datetime directly patchable in tests and removes the need for the test seam entirely:

♻️ Proposed refactor

+from datetime import datetime as _dt, timezone as _tz
import json
...
-def _stamp_last_sign_in_for_test(user_id: str) -> None:- """Test seam: write last_sign_in_at ..."""- from datetime import datetime, timezone- table("users").update(- {"last_sign_in_at": datetime.now(timezone.utc).isoformat()},- filters={"id": f"eq.{user_id}"},- )--
...
if existing:
- from datetime import datetime as _dt, timezone as _tz
table("users").update({
...
"last_sign_in_at": _dt.now(_tz.utc).isoformat(),
}, ...)
else:
- from datetime import datetime as _dt, timezone as _tz
table("users").insert({
...
"last_sign_in_at": _dt.now(_tz.utc).isoformat(),
})

With _dt at module level, test_auth_state.py can replace the test helper with a direct monkeypatch:

# test_auth_state.pyimportroutes.authasauth_moduledeftest_callback_stamps_last_sign_in(monkeypatch):
fixed=_dt(2024, 1, 1, tzinfo=_tz.utc)
monkeypatch.setattr(auth_module, "_dt", lambda*a, **kw: fixed)
...

Also applies to: 325-325, 343-343

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@backend/routes/auth.py` around lines 44 - 51, Move the test-only helper
_stamp_last_sign_in_for_test out of the production routes.auth module into the
test suite and hoist the datetime import to module scope: add a module-level
import like "from datetime import datetime as _dt, timezone as _tz" in
routes.auth, replace any local in-function imports and uses of
datetime.now(timezone.utc) with _dt(..., tzinfo=_tz.utc) (or calls to
_dt.now(_tz.utc)), and remove the test helper from the production file so tests
can monkeypatch the module-level _dt/_tz instead; update any references to
_stamp_last_sign_in_for_test in tests to call the moved helper in the test
package.

🤖 Prompt for all review comments with AI agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@backend/routes/admin.py`:
- Around line 558-570: The approvals series is currently counted by signup date
(using created_at) rather than the approval timestamp; update the logic in the
loop that builds by_day_approvals/approvals_by_day to use an approval timestamp
(e.g., check for an approved_at or similar field on each user instead of
created_at) and parse that timestamp into a date (like you do with created_at)
before incrementing by_day_approvals[d.isoformat()]. If there is no approval
timestamp available, either add/compute one at approval time (e.g., set
approved_at when is_approved transitions true) or rename the metric from
“Approvals” to something like “Approved users by signup date” to reflect the
current derivation (refer to variables/users, created_at, is_approved,
by_day_approvals, approvals_by_day, window).
- Around line 95-109: The code currently allows a client to override the grantor
via body.granted_by; instead, always derive the grantor from the session: set
granted_by = get_session_user_id(request) (i.e., the existing actor variable)
and remove usage of body.granted_by when building the upsert payload and the
log_admin_action payload so both table("user_roles").upsert and log_admin_action
use the trusted actor value only.
In `@backend/services/admin_audit.py`:
- Line 31: The log currently calls log.exception("admin_audit_log write failed:
%s", row) which will serialize sensitive identifiers (row.actor_id,
row.target_id); change the logging to avoid including identifying fields by
creating a sanitized version of row that omits or redacts actor_id and target_id
and log that instead (or log only non-identifying metadata such as event type,
timestamp, and error context). Update the call site using log.exception to pass
the sanitized data or a minimal message and error context rather than the full
row so actor_id/target_id are never written to logs.
In `@frontend/src/components/screens/Admin.tsx`:
- Around line 492-511: loadDetails writes shared state (setTriggers,
setLinkedCosmeticIds, setAllCosmetics) unguarded so a stale async response for a
previously requested id can overwrite the currently open achievement; fix by
capturing/validating the expected open id before applying results (e.g., read
current openId from a ref or compare openId at the time of resolution) and only
call setTriggers/setLinkedCosmeticIds/setAllCosmetics when the resolved id still
matches the current openId (or use an AbortController per loadDetails to cancel
stale requests), keeping function names loadDetails, toggleOpen, setTriggers,
setLinkedCosmeticIds, setAllCosmetics and openId as the reference points.
---
Nitpick comments:
In `@backend/routes/auth.py`:
- Around line 44-51: Move the test-only helper _stamp_last_sign_in_for_test out
of the production routes.auth module into the test suite and hoist the datetime
import to module scope: add a module-level import like "from datetime import
datetime as _dt, timezone as _tz" in routes.auth, replace any local in-function
imports and uses of datetime.now(timezone.utc) with _dt(..., tzinfo=_tz.utc) (or
calls to _dt.now(_tz.utc)), and remove the test helper from the production file
so tests can monkeypatch the module-level _dt/_tz instead; update any references
to _stamp_last_sign_in_for_test in tests to call the moved helper in the test
package.
In `@backend/services/users_search.py`:
- Around line 20-26: The _attach_roles function issues an individual
Supabase/PostgREST request per user causing N+1 queries; change it to fetch all
roles in one batch by querying table("user_roles") with a single select using
the user_id in.(<list_of_user_ids>) filter, retrieving "user_id" and the nested
"roles(...)" fields, then group the returned rows by user_id and assign
user["roles"] = [r["roles"] for r in grouped[user_id]] (or [] if none) for each
user; keep function name _attach_roles, use the same table("user_roles") select
call but replace per-user filters with the in filter and ensure you handle empty
input lists and absent role entries.

🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

• Push a commit to this branch (recommended)
• Create a new PR with the fixes

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro Plus

Run ID: e976a667-d4b6-4fac-9347-f27bc7d43539

📥 Commits

Reviewing files that changed from the base of the PR and between db5ba48 and 1c23a86.

📒 Files selected for processing (16)

• backend/db/connection.py
• backend/db/migration_admin_portal.sql
• backend/models/__init__.py
• backend/routes/admin.py
• backend/routes/auth.py
• backend/services/admin_audit.py
• backend/services/users_search.py
• backend/tests/test_admin_audit.py
• backend/tests/test_admin_routes.py
• backend/tests/test_auth_state.py
• backend/tests/test_supabase.py
• backend/tests/test_users_search.py
• docs/superpowers/plans/2026-05-04-admin-portal.md
• frontend/src/components/screens/Admin.tsx
• frontend/src/lib/api.ts
• frontend/src/lib/types.ts

[coderabbitai]

⚠️ Potential issue | 🟠 Major | ⚡ Quick win

Do not trust client-supplied granted_by.

granted_by = body.granted_by or actor lets the caller forge who granted the role. That breaks the server-side provenance this PR is adding and makes both the row data and the audit trail unreliable. Stamp granted_by from get_session_user_id(request) unconditionally.

Suggested fix

 def assign_role(body: AssignRoleBody, request: Request):
require_admin(request)
actor = get_session_user_id(request)
- granted_by = body.granted_by or actor+ granted_by = actor
table("user_roles").upsert(
{
"user_id": body.user_id,
"role_id": body.role_id,
"granted_by": granted_by,

📝 Committable suggestion

‼️IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change

	actor=get_session_user_id(request)
	granted_by=body.granted_byoractor
	table("user_roles").upsert(
	{
	"user_id": body.user_id,
	"role_id": body.role_id,
	"granted_by": granted_by,
	"granted_at": datetime.now(timezone.utc).isoformat(),
	},
	on_conflict="user_id,role_id",
	)
	log_admin_action(
	actor_id=actor, action="role.assign", target_type="role", target_id=body.role_id,
	payload={"user_id": body.user_id, "granted_by": granted_by},
	)
	actor=get_session_user_id(request)
	granted_by=actor
	table("user_roles").upsert(
	{
	"user_id": body.user_id,
	"role_id": body.role_id,
	"granted_by": granted_by,
	"granted_at": datetime.now(timezone.utc).isoformat(),
	},
	on_conflict="user_id,role_id",
	)
	log_admin_action(
	actor_id=actor, action="role.assign", target_type="role", target_id=body.role_id,
	payload={"user_id": body.user_id, "granted_by": granted_by},
	)

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@backend/routes/admin.py` around lines 95 - 109, The code currently allows a
client to override the grantor via body.granted_by; instead, always derive the
grantor from the session: set granted_by = get_session_user_id(request) (i.e.,
the existing actor variable) and remove usage of body.granted_by when building
the upsert payload and the log_admin_action payload so both
table("user_roles").upsert and log_admin_action use the trusted actor value
only.

[coderabbitai]

⚠️ Potential issue | 🟠 Major | 🏗️ Heavy lift

The “approvals” series is grouped by signup date, not approval date.

approvals_by_day is incremented from created_at for users whose current is_approved is true. A user approved today will be counted on their original signup day, so the Analytics tab’s “Approvals · last 30 days” chart is wrong. This needs either a real approval timestamp or a renamed metric that matches the current derivation.

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@backend/routes/admin.py` around lines 558 - 570, The approvals series is
currently counted by signup date (using created_at) rather than the approval
timestamp; update the logic in the loop that builds
by_day_approvals/approvals_by_day to use an approval timestamp (e.g., check for
an approved_at or similar field on each user instead of created_at) and parse
that timestamp into a date (like you do with created_at) before incrementing
by_day_approvals[d.isoformat()]. If there is no approval timestamp available,
either add/compute one at approval time (e.g., set approved_at when is_approved
transitions true) or rename the metric from “Approvals” to something like
“Approved users by signup date” to reflect the current derivation (refer to
variables/users, created_at, is_approved, by_day_approvals, approvals_by_day,
window).

[coderabbitai]

⚠️ Potential issue | 🟠 Major | ⚡ Quick win

log.exception serializes user identifiers (actor_id, target_id) into application logs

row includes actor_id and target_id, which encode Google sub-claim–based user IDs — persistent identifiers that can be linked back to real persons. If application logs are forwarded to external aggregators this violates GDPR/CCPA guidance against logging user identifiers.

🔒 Proposed fix — log only non-identifying metadata

- log.exception("admin_audit_log write failed: %s", row)+ log.exception(+ "admin_audit_log write failed action=%s target_type=%s",+ row.get("action"),+ row.get("target_type"),+ )

As per coding guidelines, logging sensitive data such as user identifiers is classified as a compliance/privacy risk (GDPR/CCPA).

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@backend/services/admin_audit.py` at line 31, The log currently calls
log.exception("admin_audit_log write failed: %s", row) which will serialize
sensitive identifiers (row.actor_id, row.target_id); change the logging to avoid
including identifying fields by creating a sanitized version of row that omits
or redacts actor_id and target_id and log that instead (or log only
non-identifying metadata such as event type, timestamp, and error context).
Update the call site using log.exception to pass the sanitized data or a minimal
message and error context rather than the full row so actor_id/target_id are
never written to logs.

[coderabbitai]

⚠️ Potential issue | 🟠 Major | ⚡ Quick win

Guard achievement detail state against stale async responses.

loadDetails() writes shared triggers / linkedCosmeticIds state without verifying that the response still belongs to the currently expanded achievement. If an admin opens A and quickly opens B, A’s slower response can overwrite B’s panel, and the inline trigger actions can then target the wrong record IDs.

Suggested fix

+ const detailsRequestRef = React.useRef(0);+ const openIdRef = React.useRef<string | null>(null);+
const loadDetails = React.useCallback(async (id: string) => {
+ const requestId = ++detailsRequestRef.current;
try {
const [t, l, c] = await Promise.all([
adminListTriggers(id),
adminListAchievementCosmetics(id),
allCosmetics.length ? Promise.resolve({ cosmetics: allCosmetics }) : adminListCosmetics(),
]);
+ if (detailsRequestRef.current !== requestId || openIdRef.current !== id) return;
setTriggers(t.triggers || []);
setLinkedCosmeticIds((l.links || []).map(x => x.cosmetic_id));
if (!allCosmetics.length) setAllCosmetics(c.cosmetics || []);
} catch (err) {
toast.error(`Detail load failed: ${String(err)}`);
}
}, [allCosmetics, toast]);
const toggleOpen = (id: string) => {
- if (openId === id) { setOpenId(null); return; }+ if (openId === id) {+ openIdRef.current = null;+ setOpenId(null);+ return;+ }+ openIdRef.current = id;+ setTriggers([]);+ setLinkedCosmeticIds([]);
setOpenId(id);
loadDetails(id);
};

📝 Committable suggestion

‼️IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change

	constloadDetails=React.useCallback(async(id: string)=>{
	try{
	const[t,l,c]=awaitPromise.all([
	adminListTriggers(id),
	adminListAchievementCosmetics(id),
	allCosmetics.length ? Promise.resolve({cosmetics: allCosmetics}) : adminListCosmetics(),
	]);
	setTriggers(t.triggers||[]);
	setLinkedCosmeticIds((l.links||[]).map(x=>x.cosmetic_id));
	if(!allCosmetics.length)setAllCosmetics(c.cosmetics||[]);
	}catch(err){
	toast.error(`Detail load failed: ${String(err)}`);
	}
	},[allCosmetics,toast]);
	consttoggleOpen=(id: string)=>{
	if(openId===id){setOpenId(null);return;}
	setOpenId(id);
	loadDetails(id);
	};
	constdetailsRequestRef=React.useRef(0);
	constopenIdRef=React.useRef<string|null>(null);
	constloadDetails=React.useCallback(async(id: string)=>{
	constrequestId=++detailsRequestRef.current;
	try{
	const[t,l,c]=awaitPromise.all([
	adminListTriggers(id),
	adminListAchievementCosmetics(id),
	allCosmetics.length ? Promise.resolve({cosmetics: allCosmetics}) : adminListCosmetics(),
	]);
	if(detailsRequestRef.current!==requestId||openIdRef.current!==id)return;
	setTriggers(t.triggers||[]);
	setLinkedCosmeticIds((l.links||[]).map(x=>x.cosmetic_id));
	if(!allCosmetics.length)setAllCosmetics(c.cosmetics||[]);
	}catch(err){
	toast.error(`Detail load failed: ${String(err)}`);
	}
	},[allCosmetics,toast]);
	consttoggleOpen=(id: string)=>{
	if(openId===id){
	openIdRef.current=null;
	setOpenId(null);
	return;
	}
	openIdRef.current=id;
	setTriggers([]);
	setLinkedCosmeticIds([]);
	setOpenId(id);
	loadDetails(id);
	};

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@frontend/src/components/screens/Admin.tsx` around lines 492 - 511,
loadDetails writes shared state (setTriggers, setLinkedCosmeticIds,
setAllCosmetics) unguarded so a stale async response for a previously requested
id can overwrite the currently open achievement; fix by capturing/validating the
expected open id before applying results (e.g., read current openId from a ref
or compare openId at the time of resolution) and only call
setTriggers/setLinkedCosmeticIds/setAllCosmetics when the resolved id still
matches the current openId (or use an AbortController per loadDetails to cancel
stale requests), keeping function names loadDetails, toggleOpen, setTriggers,
setLinkedCosmeticIds, setAllCosmetics and openId as the reference points.

[AndresL230]

Closes#69.

Summary

• Pagination + server-side search on /api/admin/users; new last_sign_in_at column written on every Google callback.
• New endpoints: unapprove user; achievement triggers list/update/delete; achievement_cosmetics + role_cosmetics link/unlink/list; admin_audit_log read; analytics overview (totals + 30-day series + role counts); allowlist approve/revoke.
• Self-protection guards: cannot revoke own admin, cannot revoke last admin, cannot delete admin role, cannot unapprove self.
• Audit log written for every admin mutation via a single sanctioned helper (services/admin_audit.py).
• Frontend Admin.tsx gains Allowlist + Audit tabs and rewires Users / Achievements / Cosmetics / Analytics to the new endpoints — all using existing card / chip / btn / label-micro / var(--accent) design tokens (no new design language).
• assign_role is now idempotent (upsert on (user_id, role_id)); granted_by auto-fills from session.

Schema

• New table admin_audit_log (actor_id, action, target_type, target_id, payload jsonb, created_at) with FK actor_id ON DELETE RESTRICT and read-path indexes on created_at / actor / target.
• New column users.last_sign_in_at and index on users.created_at.

Migration file: backend/db/migration_admin_portal.sql — run once in the Supabase SQL editor.

Test plan

• Run the SQL migration in the dev Supabase project.
• cd backend && python -m pytest tests/test_admin_routes.py tests/test_admin_audit.py tests/test_users_search.py tests/test_supabase.py tests/test_auth_state.py -q (81 tests, all green on this branch).
• cd frontend && npx tsc --noEmit (clean).
• Manual smoke: visit /admin as an admin and walk every tab.
  • Users: paginate, debounced search, approve/unapprove, role assign/revoke.
  • Allowlist: add an email, revoke, re-approve.
  • Roles: try deleting the admin role (must 409); try revoking own admin (409); reassigning an existing role must NOT 500.
  • Achievements: Manage on a row, add/edit/delete a trigger, link/unlink a cosmetic chip.
  • Cosmetics: Manage on a row, link/unlink a role chip.
  • Analytics: totals match DB, sparkline tooltips show daily counts.
  • Audit: every action above appears as a row, filters by action/target_type work.

Out of scope

• Impersonate user (mentioned in feat(admin): admin portal UI — users, allowlist, roles, achievements, cosmetics #69 — touches session minting; deferred).
• EmailStr validation on AllowlistEmailBody — currently email: str. Worth tightening in a follow-up.

Plan

Implementation followed docs/superpowers/plans/2026-05-04-admin-portal.md (committed in this branch).

Summary by CodeRabbit

• New Features
  • Admin dashboard expanded with allowlist management and audit log sections
  • User management improved with pagination, search, and last sign-in date tracking
  • Achievement trigger and cosmetic linking management tools added
  • Admin audit logging tracks all administrative actions system-wide
  • Analytics dashboard displays signup trends and role distribution metrics

[coderabbitai]

📝 Walkthrough

Walkthrough

This PR implements a comprehensive admin portal spanning backend and frontend. It adds database schema for audit logging, a new select_with_count() pagination helper, admin audit tracking across mutations, paginated/searchable user listing, achievement trigger and cosmetic-linking management, role protection rules, allowlist administration, analytics computation, and a fully featured admin UI with tabs for users (paginated/searchable), allowlist, roles, achievements, cosmetics, and audit logs.

Changes

Admin Portal – Complete Implementation

Layer / File(s)	Summary
Database Schema & Indexing backend/db/migration_admin_portal.sql	Creates admin_audit_log table with actor/action/target metadata and timestamps; adds users.last_sign_in_at column; indexes created for time-based and actor-based audit queries.
Database Query Helpers backend/db/connection.py	Adds SupabaseTable.select_with_count() synchronous method that returns (rows, total) by parsing Content-Range header; supports offset alongside limit.
Request Body Models backend/models/__init__.py	Introduces UpdateAchievementTriggerBody, LinkAchievementCosmeticBody, LinkRoleCosmeticBody, and AllowlistEmailBody for admin endpoints.
Audit Service backend/services/admin_audit.py	Adds log_admin_action() to insert audit rows into admin_audit_log with error suppression; ensures audit failures don't block originating operations.
User Pagination & Search backend/services/users_search.py	Adds paginate_users() implementing DB-level pagination when no query provided and decrypt-all-then-filter approach for name/email search; attaches per-user roles and returns {users, total, page, page_size}.
Admin Routes backend/routes/admin.py	Adds/extends 20+ endpoints: role CRUD with admin-role self-protection, achievement management with trigger CRUD and cosmetic linking, cosmetic management, user listing (paginated), approval/unapproval with self-guard, role/achievement-cosmetic linking, allowlist administration, audit log querying, and analytics overview computation. All mutations emit audit events via log_admin_action().
Auth Integration backend/routes/auth.py	Tracks last_sign_in_at on Google OAuth callback (both existing and new users); adds test helper _stamp_last_sign_in_for_test() to support test scenarios without full OAuth flow.
Backend Tests backend/tests/test_admin_audit.py, test_admin_routes.py, test_auth_state.py, test_supabase.py, test_users_search.py	Comprehensive unit and integration test coverage: audit logging (row insertion and error suppression), user pagination with/without search and role attachment, role protection rules, achievement/cosmetic/allowlist/audit operations, analytics computation, and select_with_count() parsing.
Frontend Types frontend/src/lib/types.ts	Adds AllowlistEmail, AchievementTrigger, AdminAuditEntry, analytics-related types (AnalyticsTotals, AnalyticsDayPoint, AnalyticsRoleCount, AnalyticsOverview), AdminUserListItem, and PaginatedUsers; extends Cosmetic with optional unlock_source field.
Frontend API Wrappers frontend/src/lib/api.ts	Adds ~17 new admin helpers for paginated user listing (with search), user unapproval, role-cosmetic linking, achievement trigger CRUD, achievement-cosmetic linking, allowlist management, audit log querying with filters, and analytics overview retrieval.
Frontend Admin UI frontend/src/components/screens/Admin.tsx	Expands Admin component to include Users tab (paginated with debounced search, approve/unapprove, last-seen display), Allowlist tab (email list with approve/revoke), Audit tab (paginated audit log with action/target filters and payload display), and extends Roles, Achievements, Cosmetics tabs with row-expansion UIs for managing linked cosmetics/roles and achievement triggers. Refactors Analytics to use adminAnalyticsOverview() with metric cards and sparklines. Upgrades CatalogRow component to support expandable management sections.

Sequence Diagram

sequenceDiagram
participant Admin as Admin User
participant FrontendUI as Frontend UI
participant API as Admin API
participant Auth as Auth Service
participant DB as Database
rect rgba(100, 200, 150, 0.5)
note over Admin,DB: User Search & Pagination Flow
Admin->>FrontendUI: Enter search query, navigate to page 2
FrontendUI->>FrontendUI: Debounce search input
FrontendUI->>API: adminFetchUsers({q: "alice", page: 2, page_size: 50})
API->>Auth: get_session_user_id() → verify admin
API->>DB: paginate_users(q="alice", page=2, page_size=50)
alt Query provided (search)
DB->>DB: Decrypt all users, filter by substring match
DB->>DB: Compute total from filtered set, slice page
else No query (list)
DB->>DB: select_with_count (offset=50, limit=50)
end
DB->>DB: Attach roles per user from user_roles
DB-->>API: {users: [...], total: 137, page: 2, page_size: 50}
API-->>FrontendUI: PaginatedUsers response
FrontendUI->>FrontendUI: Render users table with Last Seen, Joined dates
FrontendUI-->>Admin: Display page 2 of 3, approve/unapprove buttons
end
rect rgba(150, 180, 220, 0.5)
note over Admin,DB: Admin Audit Logging Flow
Admin->>FrontendUI: Click "Approve" button for user
FrontendUI->>API: adminApproveUser(userId)
API->>Auth: get_session_user_id() → actor_id
API->>DB: UPDATE users SET is_approved=true WHERE id=userId
API->>DB: log_admin_action(actor_id, "user.approve", "user", userId, {...})
DB->>DB: INSERT INTO admin_audit_log (actor_id, action, target_type, target_id, created_at)
DB-->>API: Audit row inserted (or error caught & logged)
API-->>FrontendUI: {approved: true}
FrontendUI-->>Admin: Show success, refresh user list
Admin->>FrontendUI: Visit Audit tab
FrontendUI->>API: adminAuditLog({page: 1, action: "user.approve"})
DB->>DB: SELECT * FROM admin_audit_log WHERE action='user.approve' ORDER BY created_at DESC
DB-->>API: Paginated audit rows
API-->>FrontendUI: Audit entries with timestamps, payload JSON
FrontendUI-->>Admin: Display filtered audit log
end
rect rgba(200, 150, 180, 0.5)
note over Admin,DB: Role-Cosmetic Linking Flow
Admin->>FrontendUI: Expand role row, toggle linked cosmetic
FrontendUI->>API: adminLinkRoleCosmetic(roleId, cosmeticId)
API->>Auth: get_session_user_id() → actor_id
API->>DB: UPSERT INTO role_cosmetics (role_id, cosmetic_id)
API->>DB: log_admin_action(actor_id, "role_cosmetic.link", "role_cosmetic", null, {...})
DB->>DB: INSERT INTO admin_audit_log
DB-->>API: Link upserted, audit logged
API-->>FrontendUI: {linked: true}
FrontendUI->>FrontendUI: Update expanded row UI to reflect new link
FrontendUI-->>Admin: Show cosmetic chip as linked in role details
end

Loading

Estimated Code Review Effort

🎯 4 (Complex) | ⏱️ ~45 minutes

Possibly Related PRs

• SaplingLearn/Sapling#56: Both PRs modify backend/routes/auth.py and the users table schema; PR #56 adds user field persistence during OAuth callback while this PR extends that to include last_sign_in_at tracking.

Poem

🐰 A warren of admins, now armed and aware,
With audit trails woven through each admin's care,
Cosmetics and triggers link up with grace,
While pages of users scroll at a measured pace,
The portal is gleaming—no more fumbling in the dark!

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 9.26% which is insufficient. The required threshold is 80.00%.	Write docstrings for the functions missing them to satisfy the coverage threshold.

✅ Passed checks (4 passed)

Check name	Status	Explanation
Title check	✅ Passed	The title accurately reflects the main changes: pagination, audit log, allowlist, and analytics for the admin portal. It is specific, concise, and directly related to the primary changeset.
Description check	✅ Passed	The description is comprehensive and follows the template structure with Summary, Changes Made (implicit via detailed sections), Testing checklist, and Out of Scope items. All key implementation details are documented.
Linked Issues check	✅ Passed	The PR fully addresses issue #69 objectives: pagination and search for users [#69], unapprove endpoint [#69], audit logging for mutations [#69], role protection guards [#69], achievement triggers management [#69], role/achievement cosmetic linking [#69], allowlist endpoints [#69], analytics overview [#69], and role assignment idempotency [#69].
Out of Scope Changes check	✅ Passed	All code changes are directly aligned with the stated objectives. Out-of-scope items (impersonation, stricter email validation) are explicitly deferred and not included in the changeset.

_{✏️ Tip: You can configure your own custom pre-merge checks in the settings.}

✨ Finishing Touches

📝 Generate docstrings

• Create stacked PR
• Commit on current branch

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Commit unit tests in branch feat/admin-portal

---

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

[cloudflare-workers-and-pages]

Deploying with Cloudflare Workers

The latest updates on your project. Learn more about integrating Git with Workers.

Status	Name	Latest Commit	Preview URL	Updated (UTC)
✅ Deployment successful! View logs	frontend	1c23a86	Commit Preview URL Branch Preview URL	May 04 2026, 11:23 PM

[coderabbitai]

Actionable comments posted: 4

🧹 Nitpick comments (2)

backend/services/users_search.py (1)

20-26: ⚡ Quick win

N+1 queries in _attach_roles — collapse to a single batch request

Each call to _attach_roles issues one Supabase REST request per user. With _MAX_PAGE_SIZE = 200 that's up to 200 sequential HTTP round-trips per admin page load. PostgREST's in.(...) filter fetches all roles in one request:

♻️ Proposed refactor

 def _attach_roles(users: list[dict]) -> None:
- for user in users:- rows = table("user_roles").select(- "roles(id,name,slug,color,icon,description,is_staff_assigned,is_earnable,display_priority)",- filters={"user_id": f"eq.{user['id']}"},- )- user["roles"] = [r["roles"] for r in (rows or []) if r.get("roles")]+ if not users:+ return+ ids = ",".join(u["id"] for u in users)+ all_rows = table("user_roles").select(+ "user_id,roles(id,name,slug,color,icon,description,is_staff_assigned,is_earnable,display_priority)",+ filters={"user_id": f"in.({ids})"},+ ) or []+ role_map: dict[str, list] = {}+ for r in all_rows:+ if r.get("roles"):+ role_map.setdefault(r["user_id"], []).append(r["roles"])+ for user in users:+ user["roles"] = role_map.get(user["id"], [])

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@backend/services/users_search.py` around lines 20 - 26, The _attach_roles
function issues an individual Supabase/PostgREST request per user causing N+1
queries; change it to fetch all roles in one batch by querying
table("user_roles") with a single select using the user_id
in.(<list_of_user_ids>) filter, retrieving "user_id" and the nested "roles(...)"
fields, then group the returned rows by user_id and assign user["roles"] =
[r["roles"] for r in grouped[user_id]] (or [] if none) for each user; keep
function name _attach_roles, use the same table("user_roles") select call but
replace per-user filters with the in filter and ensure you handle empty input
lists and absent role entries.

backend/routes/auth.py (1)

44-51: ⚡ Quick win

Move _stamp_last_sign_in_for_test out of the production module and hoist the datetime import

Two related issues:

1. _stamp_last_sign_in_for_test is a test-only helper sitting in the production auth.py module. Any package import of routes.auth exposes it, and it exists only because datetime.now is buried inside conditional branches where standard monkeypatch can't reach it.
2. from datetime import datetime as _dt, timezone as _tz is duplicated in three places (lines 47, 325, 343). Python caches module imports, so this isn't a correctness bug, but it's unnecessarily noisy.

The cleanest resolution is to promote the import to module level, which then makes routes.auth.datetime directly patchable in tests and removes the need for the test seam entirely:

♻️ Proposed refactor

+from datetime import datetime as _dt, timezone as _tz
import json
...
-def _stamp_last_sign_in_for_test(user_id: str) -> None:- """Test seam: write last_sign_in_at ..."""- from datetime import datetime, timezone- table("users").update(- {"last_sign_in_at": datetime.now(timezone.utc).isoformat()},- filters={"id": f"eq.{user_id}"},- )--
...
if existing:
- from datetime import datetime as _dt, timezone as _tz
table("users").update({
...
"last_sign_in_at": _dt.now(_tz.utc).isoformat(),
}, ...)
else:
- from datetime import datetime as _dt, timezone as _tz
table("users").insert({
...
"last_sign_in_at": _dt.now(_tz.utc).isoformat(),
})

With _dt at module level, test_auth_state.py can replace the test helper with a direct monkeypatch:

# test_auth_state.pyimportroutes.authasauth_moduledeftest_callback_stamps_last_sign_in(monkeypatch):
fixed=_dt(2024, 1, 1, tzinfo=_tz.utc)
monkeypatch.setattr(auth_module, "_dt", lambda*a, **kw: fixed)
...

Also applies to: 325-325, 343-343

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@backend/routes/auth.py` around lines 44 - 51, Move the test-only helper
_stamp_last_sign_in_for_test out of the production routes.auth module into the
test suite and hoist the datetime import to module scope: add a module-level
import like "from datetime import datetime as _dt, timezone as _tz" in
routes.auth, replace any local in-function imports and uses of
datetime.now(timezone.utc) with _dt(..., tzinfo=_tz.utc) (or calls to
_dt.now(_tz.utc)), and remove the test helper from the production file so tests
can monkeypatch the module-level _dt/_tz instead; update any references to
_stamp_last_sign_in_for_test in tests to call the moved helper in the test
package.

🤖 Prompt for all review comments with AI agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@backend/routes/admin.py`:
- Around line 558-570: The approvals series is currently counted by signup date
(using created_at) rather than the approval timestamp; update the logic in the
loop that builds by_day_approvals/approvals_by_day to use an approval timestamp
(e.g., check for an approved_at or similar field on each user instead of
created_at) and parse that timestamp into a date (like you do with created_at)
before incrementing by_day_approvals[d.isoformat()]. If there is no approval
timestamp available, either add/compute one at approval time (e.g., set
approved_at when is_approved transitions true) or rename the metric from
“Approvals” to something like “Approved users by signup date” to reflect the
current derivation (refer to variables/users, created_at, is_approved,
by_day_approvals, approvals_by_day, window).
- Around line 95-109: The code currently allows a client to override the grantor
via body.granted_by; instead, always derive the grantor from the session: set
granted_by = get_session_user_id(request) (i.e., the existing actor variable)
and remove usage of body.granted_by when building the upsert payload and the
log_admin_action payload so both table("user_roles").upsert and log_admin_action
use the trusted actor value only.
In `@backend/services/admin_audit.py`:
- Line 31: The log currently calls log.exception("admin_audit_log write failed:
%s", row) which will serialize sensitive identifiers (row.actor_id,
row.target_id); change the logging to avoid including identifying fields by
creating a sanitized version of row that omits or redacts actor_id and target_id
and log that instead (or log only non-identifying metadata such as event type,
timestamp, and error context). Update the call site using log.exception to pass
the sanitized data or a minimal message and error context rather than the full
row so actor_id/target_id are never written to logs.
In `@frontend/src/components/screens/Admin.tsx`:
- Around line 492-511: loadDetails writes shared state (setTriggers,
setLinkedCosmeticIds, setAllCosmetics) unguarded so a stale async response for a
previously requested id can overwrite the currently open achievement; fix by
capturing/validating the expected open id before applying results (e.g., read
current openId from a ref or compare openId at the time of resolution) and only
call setTriggers/setLinkedCosmeticIds/setAllCosmetics when the resolved id still
matches the current openId (or use an AbortController per loadDetails to cancel
stale requests), keeping function names loadDetails, toggleOpen, setTriggers,
setLinkedCosmeticIds, setAllCosmetics and openId as the reference points.
---
Nitpick comments:
In `@backend/routes/auth.py`:
- Around line 44-51: Move the test-only helper _stamp_last_sign_in_for_test out
of the production routes.auth module into the test suite and hoist the datetime
import to module scope: add a module-level import like "from datetime import
datetime as _dt, timezone as _tz" in routes.auth, replace any local in-function
imports and uses of datetime.now(timezone.utc) with _dt(..., tzinfo=_tz.utc) (or
calls to _dt.now(_tz.utc)), and remove the test helper from the production file
so tests can monkeypatch the module-level _dt/_tz instead; update any references
to _stamp_last_sign_in_for_test in tests to call the moved helper in the test
package.
In `@backend/services/users_search.py`:
- Around line 20-26: The _attach_roles function issues an individual
Supabase/PostgREST request per user causing N+1 queries; change it to fetch all
roles in one batch by querying table("user_roles") with a single select using
the user_id in.(<list_of_user_ids>) filter, retrieving "user_id" and the nested
"roles(...)" fields, then group the returned rows by user_id and assign
user["roles"] = [r["roles"] for r in grouped[user_id]] (or [] if none) for each
user; keep function name _attach_roles, use the same table("user_roles") select
call but replace per-user filters with the in filter and ensure you handle empty
input lists and absent role entries.

🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

• Push a commit to this branch (recommended)
• Create a new PR with the fixes

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro Plus

Run ID: e976a667-d4b6-4fac-9347-f27bc7d43539

📥 Commits

Reviewing files that changed from the base of the PR and between db5ba48 and 1c23a86.

📒 Files selected for processing (16)

• backend/db/connection.py
• backend/db/migration_admin_portal.sql
• backend/models/__init__.py
• backend/routes/admin.py
• backend/routes/auth.py
• backend/services/admin_audit.py
• backend/services/users_search.py
• backend/tests/test_admin_audit.py
• backend/tests/test_admin_routes.py
• backend/tests/test_auth_state.py
• backend/tests/test_supabase.py
• backend/tests/test_users_search.py
• docs/superpowers/plans/2026-05-04-admin-portal.md
• frontend/src/components/screens/Admin.tsx
• frontend/src/lib/api.ts
• frontend/src/lib/types.ts

[coderabbitai]

⚠️ Potential issue | 🟠 Major | ⚡ Quick win

Do not trust client-supplied granted_by.

granted_by = body.granted_by or actor lets the caller forge who granted the role. That breaks the server-side provenance this PR is adding and makes both the row data and the audit trail unreliable. Stamp granted_by from get_session_user_id(request) unconditionally.

Suggested fix

 def assign_role(body: AssignRoleBody, request: Request):
require_admin(request)
actor = get_session_user_id(request)
- granted_by = body.granted_by or actor+ granted_by = actor
table("user_roles").upsert(
{
"user_id": body.user_id,
"role_id": body.role_id,
"granted_by": granted_by,

📝 Committable suggestion

‼️IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change

	actor=get_session_user_id(request)
	granted_by=body.granted_byoractor
	table("user_roles").upsert(
	{
	"user_id": body.user_id,
	"role_id": body.role_id,
	"granted_by": granted_by,
	"granted_at": datetime.now(timezone.utc).isoformat(),
	},
	on_conflict="user_id,role_id",
	)
	log_admin_action(
	actor_id=actor, action="role.assign", target_type="role", target_id=body.role_id,
	payload={"user_id": body.user_id, "granted_by": granted_by},
	)
	actor=get_session_user_id(request)
	granted_by=actor
	table("user_roles").upsert(
	{
	"user_id": body.user_id,
	"role_id": body.role_id,
	"granted_by": granted_by,
	"granted_at": datetime.now(timezone.utc).isoformat(),
	},
	on_conflict="user_id,role_id",
	)
	log_admin_action(
	actor_id=actor, action="role.assign", target_type="role", target_id=body.role_id,
	payload={"user_id": body.user_id, "granted_by": granted_by},
	)

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@backend/routes/admin.py` around lines 95 - 109, The code currently allows a
client to override the grantor via body.granted_by; instead, always derive the
grantor from the session: set granted_by = get_session_user_id(request) (i.e.,
the existing actor variable) and remove usage of body.granted_by when building
the upsert payload and the log_admin_action payload so both
table("user_roles").upsert and log_admin_action use the trusted actor value
only.

[coderabbitai]

⚠️ Potential issue | 🟠 Major | 🏗️ Heavy lift

The “approvals” series is grouped by signup date, not approval date.

approvals_by_day is incremented from created_at for users whose current is_approved is true. A user approved today will be counted on their original signup day, so the Analytics tab’s “Approvals · last 30 days” chart is wrong. This needs either a real approval timestamp or a renamed metric that matches the current derivation.

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@backend/routes/admin.py` around lines 558 - 570, The approvals series is
currently counted by signup date (using created_at) rather than the approval
timestamp; update the logic in the loop that builds
by_day_approvals/approvals_by_day to use an approval timestamp (e.g., check for
an approved_at or similar field on each user instead of created_at) and parse
that timestamp into a date (like you do with created_at) before incrementing
by_day_approvals[d.isoformat()]. If there is no approval timestamp available,
either add/compute one at approval time (e.g., set approved_at when is_approved
transitions true) or rename the metric from “Approvals” to something like
“Approved users by signup date” to reflect the current derivation (refer to
variables/users, created_at, is_approved, by_day_approvals, approvals_by_day,
window).

[coderabbitai]

⚠️ Potential issue | 🟠 Major | ⚡ Quick win

log.exception serializes user identifiers (actor_id, target_id) into application logs

row includes actor_id and target_id, which encode Google sub-claim–based user IDs — persistent identifiers that can be linked back to real persons. If application logs are forwarded to external aggregators this violates GDPR/CCPA guidance against logging user identifiers.

🔒 Proposed fix — log only non-identifying metadata

- log.exception("admin_audit_log write failed: %s", row)+ log.exception(+ "admin_audit_log write failed action=%s target_type=%s",+ row.get("action"),+ row.get("target_type"),+ )

As per coding guidelines, logging sensitive data such as user identifiers is classified as a compliance/privacy risk (GDPR/CCPA).

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@backend/services/admin_audit.py` at line 31, The log currently calls
log.exception("admin_audit_log write failed: %s", row) which will serialize
sensitive identifiers (row.actor_id, row.target_id); change the logging to avoid
including identifying fields by creating a sanitized version of row that omits
or redacts actor_id and target_id and log that instead (or log only
non-identifying metadata such as event type, timestamp, and error context).
Update the call site using log.exception to pass the sanitized data or a minimal
message and error context rather than the full row so actor_id/target_id are
never written to logs.

[coderabbitai]

⚠️ Potential issue | 🟠 Major | ⚡ Quick win

Guard achievement detail state against stale async responses.

loadDetails() writes shared triggers / linkedCosmeticIds state without verifying that the response still belongs to the currently expanded achievement. If an admin opens A and quickly opens B, A’s slower response can overwrite B’s panel, and the inline trigger actions can then target the wrong record IDs.

Suggested fix

+ const detailsRequestRef = React.useRef(0);+ const openIdRef = React.useRef<string | null>(null);+
const loadDetails = React.useCallback(async (id: string) => {
+ const requestId = ++detailsRequestRef.current;
try {
const [t, l, c] = await Promise.all([
adminListTriggers(id),
adminListAchievementCosmetics(id),
allCosmetics.length ? Promise.resolve({ cosmetics: allCosmetics }) : adminListCosmetics(),
]);
+ if (detailsRequestRef.current !== requestId || openIdRef.current !== id) return;
setTriggers(t.triggers || []);
setLinkedCosmeticIds((l.links || []).map(x => x.cosmetic_id));
if (!allCosmetics.length) setAllCosmetics(c.cosmetics || []);
} catch (err) {
toast.error(`Detail load failed: ${String(err)}`);
}
}, [allCosmetics, toast]);
const toggleOpen = (id: string) => {
- if (openId === id) { setOpenId(null); return; }+ if (openId === id) {+ openIdRef.current = null;+ setOpenId(null);+ return;+ }+ openIdRef.current = id;+ setTriggers([]);+ setLinkedCosmeticIds([]);
setOpenId(id);
loadDetails(id);
};

📝 Committable suggestion

‼️IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change

	constloadDetails=React.useCallback(async(id: string)=>{
	try{
	const[t,l,c]=awaitPromise.all([
	adminListTriggers(id),
	adminListAchievementCosmetics(id),
	allCosmetics.length ? Promise.resolve({cosmetics: allCosmetics}) : adminListCosmetics(),
	]);
	setTriggers(t.triggers||[]);
	setLinkedCosmeticIds((l.links||[]).map(x=>x.cosmetic_id));
	if(!allCosmetics.length)setAllCosmetics(c.cosmetics||[]);
	}catch(err){
	toast.error(`Detail load failed: ${String(err)}`);
	}
	},[allCosmetics,toast]);
	consttoggleOpen=(id: string)=>{
	if(openId===id){setOpenId(null);return;}
	setOpenId(id);
	loadDetails(id);
	};
	constdetailsRequestRef=React.useRef(0);
	constopenIdRef=React.useRef<string|null>(null);
	constloadDetails=React.useCallback(async(id: string)=>{
	constrequestId=++detailsRequestRef.current;
	try{
	const[t,l,c]=awaitPromise.all([
	adminListTriggers(id),
	adminListAchievementCosmetics(id),
	allCosmetics.length ? Promise.resolve({cosmetics: allCosmetics}) : adminListCosmetics(),
	]);
	if(detailsRequestRef.current!==requestId||openIdRef.current!==id)return;
	setTriggers(t.triggers||[]);
	setLinkedCosmeticIds((l.links||[]).map(x=>x.cosmetic_id));
	if(!allCosmetics.length)setAllCosmetics(c.cosmetics||[]);
	}catch(err){
	toast.error(`Detail load failed: ${String(err)}`);
	}
	},[allCosmetics,toast]);
	consttoggleOpen=(id: string)=>{
	if(openId===id){
	openIdRef.current=null;
	setOpenId(null);
	return;
	}
	openIdRef.current=id;
	setTriggers([]);
	setLinkedCosmeticIds([]);
	setOpenId(id);
	loadDetails(id);
	};

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@frontend/src/components/screens/Admin.tsx` around lines 492 - 511,
loadDetails writes shared state (setTriggers, setLinkedCosmeticIds,
setAllCosmetics) unguarded so a stale async response for a previously requested
id can overwrite the currently open achievement; fix by capturing/validating the
expected open id before applying results (e.g., read current openId from a ref
or compare openId at the time of resolution) and only call
setTriggers/setLinkedCosmeticIds/setAllCosmetics when the resolved id still
matches the current openId (or use an AbortController per loadDetails to cancel
stale requests), keeping function names loadDetails, toggleOpen, setTriggers,
setLinkedCosmeticIds, setAllCosmetics and openId as the reference points.

[AndresL230]

Closes#69.

Summary

• Pagination + server-side search on /api/admin/users; new last_sign_in_at column written on every Google callback.
• New endpoints: unapprove user; achievement triggers list/update/delete; achievement_cosmetics + role_cosmetics link/unlink/list; admin_audit_log read; analytics overview (totals + 30-day series + role counts); allowlist approve/revoke.
• Self-protection guards: cannot revoke own admin, cannot revoke last admin, cannot delete admin role, cannot unapprove self.
• Audit log written for every admin mutation via a single sanctioned helper (services/admin_audit.py).
• Frontend Admin.tsx gains Allowlist + Audit tabs and rewires Users / Achievements / Cosmetics / Analytics to the new endpoints — all using existing card / chip / btn / label-micro / var(--accent) design tokens (no new design language).
• assign_role is now idempotent (upsert on (user_id, role_id)); granted_by auto-fills from session.

Schema

• New table admin_audit_log (actor_id, action, target_type, target_id, payload jsonb, created_at) with FK actor_id ON DELETE RESTRICT and read-path indexes on created_at / actor / target.
• New column users.last_sign_in_at and index on users.created_at.

Migration file: backend/db/migration_admin_portal.sql — run once in the Supabase SQL editor.

Test plan

• Run the SQL migration in the dev Supabase project.
• cd backend && python -m pytest tests/test_admin_routes.py tests/test_admin_audit.py tests/test_users_search.py tests/test_supabase.py tests/test_auth_state.py -q (81 tests, all green on this branch).
• cd frontend && npx tsc --noEmit (clean).
• Manual smoke: visit /admin as an admin and walk every tab.
  • Users: paginate, debounced search, approve/unapprove, role assign/revoke.
  • Allowlist: add an email, revoke, re-approve.
  • Roles: try deleting the admin role (must 409); try revoking own admin (409); reassigning an existing role must NOT 500.
  • Achievements: Manage on a row, add/edit/delete a trigger, link/unlink a cosmetic chip.
  • Cosmetics: Manage on a row, link/unlink a role chip.
  • Analytics: totals match DB, sparkline tooltips show daily counts.
  • Audit: every action above appears as a row, filters by action/target_type work.

Out of scope

• Impersonate user (mentioned in feat(admin): admin portal UI — users, allowlist, roles, achievements, cosmetics #69 — touches session minting; deferred).
• EmailStr validation on AllowlistEmailBody — currently email: str. Worth tightening in a follow-up.

Plan

Implementation followed docs/superpowers/plans/2026-05-04-admin-portal.md (committed in this branch).

Summary by CodeRabbit

• New Features
  • Admin dashboard expanded with allowlist management and audit log sections
  • User management improved with pagination, search, and last sign-in date tracking
  • Achievement trigger and cosmetic linking management tools added
  • Admin audit logging tracks all administrative actions system-wide
  • Analytics dashboard displays signup trends and role distribution metrics

[coderabbitai]

📝 Walkthrough

Walkthrough

This PR implements a comprehensive admin portal spanning backend and frontend. It adds database schema for audit logging, a new select_with_count() pagination helper, admin audit tracking across mutations, paginated/searchable user listing, achievement trigger and cosmetic-linking management, role protection rules, allowlist administration, analytics computation, and a fully featured admin UI with tabs for users (paginated/searchable), allowlist, roles, achievements, cosmetics, and audit logs.

Changes

Admin Portal – Complete Implementation

Layer / File(s)	Summary
Database Schema & Indexing backend/db/migration_admin_portal.sql	Creates admin_audit_log table with actor/action/target metadata and timestamps; adds users.last_sign_in_at column; indexes created for time-based and actor-based audit queries.
Database Query Helpers backend/db/connection.py	Adds SupabaseTable.select_with_count() synchronous method that returns (rows, total) by parsing Content-Range header; supports offset alongside limit.
Request Body Models backend/models/__init__.py	Introduces UpdateAchievementTriggerBody, LinkAchievementCosmeticBody, LinkRoleCosmeticBody, and AllowlistEmailBody for admin endpoints.
Audit Service backend/services/admin_audit.py	Adds log_admin_action() to insert audit rows into admin_audit_log with error suppression; ensures audit failures don't block originating operations.
User Pagination & Search backend/services/users_search.py	Adds paginate_users() implementing DB-level pagination when no query provided and decrypt-all-then-filter approach for name/email search; attaches per-user roles and returns {users, total, page, page_size}.
Admin Routes backend/routes/admin.py	Adds/extends 20+ endpoints: role CRUD with admin-role self-protection, achievement management with trigger CRUD and cosmetic linking, cosmetic management, user listing (paginated), approval/unapproval with self-guard, role/achievement-cosmetic linking, allowlist administration, audit log querying, and analytics overview computation. All mutations emit audit events via log_admin_action().
Auth Integration backend/routes/auth.py	Tracks last_sign_in_at on Google OAuth callback (both existing and new users); adds test helper _stamp_last_sign_in_for_test() to support test scenarios without full OAuth flow.
Backend Tests backend/tests/test_admin_audit.py, test_admin_routes.py, test_auth_state.py, test_supabase.py, test_users_search.py	Comprehensive unit and integration test coverage: audit logging (row insertion and error suppression), user pagination with/without search and role attachment, role protection rules, achievement/cosmetic/allowlist/audit operations, analytics computation, and select_with_count() parsing.
Frontend Types frontend/src/lib/types.ts	Adds AllowlistEmail, AchievementTrigger, AdminAuditEntry, analytics-related types (AnalyticsTotals, AnalyticsDayPoint, AnalyticsRoleCount, AnalyticsOverview), AdminUserListItem, and PaginatedUsers; extends Cosmetic with optional unlock_source field.
Frontend API Wrappers frontend/src/lib/api.ts	Adds ~17 new admin helpers for paginated user listing (with search), user unapproval, role-cosmetic linking, achievement trigger CRUD, achievement-cosmetic linking, allowlist management, audit log querying with filters, and analytics overview retrieval.
Frontend Admin UI frontend/src/components/screens/Admin.tsx	Expands Admin component to include Users tab (paginated with debounced search, approve/unapprove, last-seen display), Allowlist tab (email list with approve/revoke), Audit tab (paginated audit log with action/target filters and payload display), and extends Roles, Achievements, Cosmetics tabs with row-expansion UIs for managing linked cosmetics/roles and achievement triggers. Refactors Analytics to use adminAnalyticsOverview() with metric cards and sparklines. Upgrades CatalogRow component to support expandable management sections.

Sequence Diagram

sequenceDiagram
participant Admin as Admin User
participant FrontendUI as Frontend UI
participant API as Admin API
participant Auth as Auth Service
participant DB as Database
rect rgba(100, 200, 150, 0.5)
note over Admin,DB: User Search & Pagination Flow
Admin->>FrontendUI: Enter search query, navigate to page 2
FrontendUI->>FrontendUI: Debounce search input
FrontendUI->>API: adminFetchUsers({q: "alice", page: 2, page_size: 50})
API->>Auth: get_session_user_id() → verify admin
API->>DB: paginate_users(q="alice", page=2, page_size=50)
alt Query provided (search)
DB->>DB: Decrypt all users, filter by substring match
DB->>DB: Compute total from filtered set, slice page
else No query (list)
DB->>DB: select_with_count (offset=50, limit=50)
end
DB->>DB: Attach roles per user from user_roles
DB-->>API: {users: [...], total: 137, page: 2, page_size: 50}
API-->>FrontendUI: PaginatedUsers response
FrontendUI->>FrontendUI: Render users table with Last Seen, Joined dates
FrontendUI-->>Admin: Display page 2 of 3, approve/unapprove buttons
end
rect rgba(150, 180, 220, 0.5)
note over Admin,DB: Admin Audit Logging Flow
Admin->>FrontendUI: Click "Approve" button for user
FrontendUI->>API: adminApproveUser(userId)
API->>Auth: get_session_user_id() → actor_id
API->>DB: UPDATE users SET is_approved=true WHERE id=userId
API->>DB: log_admin_action(actor_id, "user.approve", "user", userId, {...})
DB->>DB: INSERT INTO admin_audit_log (actor_id, action, target_type, target_id, created_at)
DB-->>API: Audit row inserted (or error caught & logged)
API-->>FrontendUI: {approved: true}
FrontendUI-->>Admin: Show success, refresh user list
Admin->>FrontendUI: Visit Audit tab
FrontendUI->>API: adminAuditLog({page: 1, action: "user.approve"})
DB->>DB: SELECT * FROM admin_audit_log WHERE action='user.approve' ORDER BY created_at DESC
DB-->>API: Paginated audit rows
API-->>FrontendUI: Audit entries with timestamps, payload JSON
FrontendUI-->>Admin: Display filtered audit log
end
rect rgba(200, 150, 180, 0.5)
note over Admin,DB: Role-Cosmetic Linking Flow
Admin->>FrontendUI: Expand role row, toggle linked cosmetic
FrontendUI->>API: adminLinkRoleCosmetic(roleId, cosmeticId)
API->>Auth: get_session_user_id() → actor_id
API->>DB: UPSERT INTO role_cosmetics (role_id, cosmetic_id)
API->>DB: log_admin_action(actor_id, "role_cosmetic.link", "role_cosmetic", null, {...})
DB->>DB: INSERT INTO admin_audit_log
DB-->>API: Link upserted, audit logged
API-->>FrontendUI: {linked: true}
FrontendUI->>FrontendUI: Update expanded row UI to reflect new link
FrontendUI-->>Admin: Show cosmetic chip as linked in role details
end

Loading

Estimated Code Review Effort

🎯 4 (Complex) | ⏱️ ~45 minutes

Possibly Related PRs

• SaplingLearn/Sapling#56: Both PRs modify backend/routes/auth.py and the users table schema; PR #56 adds user field persistence during OAuth callback while this PR extends that to include last_sign_in_at tracking.

Poem

🐰 A warren of admins, now armed and aware,
With audit trails woven through each admin's care,
Cosmetics and triggers link up with grace,
While pages of users scroll at a measured pace,
The portal is gleaming—no more fumbling in the dark!

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 9.26% which is insufficient. The required threshold is 80.00%.	Write docstrings for the functions missing them to satisfy the coverage threshold.

✅ Passed checks (4 passed)

Check name	Status	Explanation
Title check	✅ Passed	The title accurately reflects the main changes: pagination, audit log, allowlist, and analytics for the admin portal. It is specific, concise, and directly related to the primary changeset.
Description check	✅ Passed	The description is comprehensive and follows the template structure with Summary, Changes Made (implicit via detailed sections), Testing checklist, and Out of Scope items. All key implementation details are documented.
Linked Issues check	✅ Passed	The PR fully addresses issue #69 objectives: pagination and search for users [#69], unapprove endpoint [#69], audit logging for mutations [#69], role protection guards [#69], achievement triggers management [#69], role/achievement cosmetic linking [#69], allowlist endpoints [#69], analytics overview [#69], and role assignment idempotency [#69].
Out of Scope Changes check	✅ Passed	All code changes are directly aligned with the stated objectives. Out-of-scope items (impersonation, stricter email validation) are explicitly deferred and not included in the changeset.

_{✏️ Tip: You can configure your own custom pre-merge checks in the settings.}

✨ Finishing Touches

📝 Generate docstrings

• Create stacked PR
• Commit on current branch

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Commit unit tests in branch feat/admin-portal

---

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

[cloudflare-workers-and-pages]

Deploying with Cloudflare Workers

The latest updates on your project. Learn more about integrating Git with Workers.

Status	Name	Latest Commit	Preview URL	Updated (UTC)
✅ Deployment successful! View logs	frontend	1c23a86	Commit Preview URL Branch Preview URL	May 04 2026, 11:23 PM

[coderabbitai]

Actionable comments posted: 4

🧹 Nitpick comments (2)

backend/services/users_search.py (1)

20-26: ⚡ Quick win

N+1 queries in _attach_roles — collapse to a single batch request

Each call to _attach_roles issues one Supabase REST request per user. With _MAX_PAGE_SIZE = 200 that's up to 200 sequential HTTP round-trips per admin page load. PostgREST's in.(...) filter fetches all roles in one request:

♻️ Proposed refactor

 def _attach_roles(users: list[dict]) -> None:
- for user in users:- rows = table("user_roles").select(- "roles(id,name,slug,color,icon,description,is_staff_assigned,is_earnable,display_priority)",- filters={"user_id": f"eq.{user['id']}"},- )- user["roles"] = [r["roles"] for r in (rows or []) if r.get("roles")]+ if not users:+ return+ ids = ",".join(u["id"] for u in users)+ all_rows = table("user_roles").select(+ "user_id,roles(id,name,slug,color,icon,description,is_staff_assigned,is_earnable,display_priority)",+ filters={"user_id": f"in.({ids})"},+ ) or []+ role_map: dict[str, list] = {}+ for r in all_rows:+ if r.get("roles"):+ role_map.setdefault(r["user_id"], []).append(r["roles"])+ for user in users:+ user["roles"] = role_map.get(user["id"], [])

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@backend/services/users_search.py` around lines 20 - 26, The _attach_roles
function issues an individual Supabase/PostgREST request per user causing N+1
queries; change it to fetch all roles in one batch by querying
table("user_roles") with a single select using the user_id
in.(<list_of_user_ids>) filter, retrieving "user_id" and the nested "roles(...)"
fields, then group the returned rows by user_id and assign user["roles"] =
[r["roles"] for r in grouped[user_id]] (or [] if none) for each user; keep
function name _attach_roles, use the same table("user_roles") select call but
replace per-user filters with the in filter and ensure you handle empty input
lists and absent role entries.

backend/routes/auth.py (1)

44-51: ⚡ Quick win

Move _stamp_last_sign_in_for_test out of the production module and hoist the datetime import

Two related issues:

1. _stamp_last_sign_in_for_test is a test-only helper sitting in the production auth.py module. Any package import of routes.auth exposes it, and it exists only because datetime.now is buried inside conditional branches where standard monkeypatch can't reach it.
2. from datetime import datetime as _dt, timezone as _tz is duplicated in three places (lines 47, 325, 343). Python caches module imports, so this isn't a correctness bug, but it's unnecessarily noisy.

The cleanest resolution is to promote the import to module level, which then makes routes.auth.datetime directly patchable in tests and removes the need for the test seam entirely:

♻️ Proposed refactor

+from datetime import datetime as _dt, timezone as _tz
import json
...
-def _stamp_last_sign_in_for_test(user_id: str) -> None:- """Test seam: write last_sign_in_at ..."""- from datetime import datetime, timezone- table("users").update(- {"last_sign_in_at": datetime.now(timezone.utc).isoformat()},- filters={"id": f"eq.{user_id}"},- )--
...
if existing:
- from datetime import datetime as _dt, timezone as _tz
table("users").update({
...
"last_sign_in_at": _dt.now(_tz.utc).isoformat(),
}, ...)
else:
- from datetime import datetime as _dt, timezone as _tz
table("users").insert({
...
"last_sign_in_at": _dt.now(_tz.utc).isoformat(),
})

With _dt at module level, test_auth_state.py can replace the test helper with a direct monkeypatch:

# test_auth_state.pyimportroutes.authasauth_moduledeftest_callback_stamps_last_sign_in(monkeypatch):
fixed=_dt(2024, 1, 1, tzinfo=_tz.utc)
monkeypatch.setattr(auth_module, "_dt", lambda*a, **kw: fixed)
...

Also applies to: 325-325, 343-343

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@backend/routes/auth.py` around lines 44 - 51, Move the test-only helper
_stamp_last_sign_in_for_test out of the production routes.auth module into the
test suite and hoist the datetime import to module scope: add a module-level
import like "from datetime import datetime as _dt, timezone as _tz" in
routes.auth, replace any local in-function imports and uses of
datetime.now(timezone.utc) with _dt(..., tzinfo=_tz.utc) (or calls to
_dt.now(_tz.utc)), and remove the test helper from the production file so tests
can monkeypatch the module-level _dt/_tz instead; update any references to
_stamp_last_sign_in_for_test in tests to call the moved helper in the test
package.

🤖 Prompt for all review comments with AI agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@backend/routes/admin.py`:
- Around line 558-570: The approvals series is currently counted by signup date
(using created_at) rather than the approval timestamp; update the logic in the
loop that builds by_day_approvals/approvals_by_day to use an approval timestamp
(e.g., check for an approved_at or similar field on each user instead of
created_at) and parse that timestamp into a date (like you do with created_at)
before incrementing by_day_approvals[d.isoformat()]. If there is no approval
timestamp available, either add/compute one at approval time (e.g., set
approved_at when is_approved transitions true) or rename the metric from
“Approvals” to something like “Approved users by signup date” to reflect the
current derivation (refer to variables/users, created_at, is_approved,
by_day_approvals, approvals_by_day, window).
- Around line 95-109: The code currently allows a client to override the grantor
via body.granted_by; instead, always derive the grantor from the session: set
granted_by = get_session_user_id(request) (i.e., the existing actor variable)
and remove usage of body.granted_by when building the upsert payload and the
log_admin_action payload so both table("user_roles").upsert and log_admin_action
use the trusted actor value only.
In `@backend/services/admin_audit.py`:
- Line 31: The log currently calls log.exception("admin_audit_log write failed:
%s", row) which will serialize sensitive identifiers (row.actor_id,
row.target_id); change the logging to avoid including identifying fields by
creating a sanitized version of row that omits or redacts actor_id and target_id
and log that instead (or log only non-identifying metadata such as event type,
timestamp, and error context). Update the call site using log.exception to pass
the sanitized data or a minimal message and error context rather than the full
row so actor_id/target_id are never written to logs.
In `@frontend/src/components/screens/Admin.tsx`:
- Around line 492-511: loadDetails writes shared state (setTriggers,
setLinkedCosmeticIds, setAllCosmetics) unguarded so a stale async response for a
previously requested id can overwrite the currently open achievement; fix by
capturing/validating the expected open id before applying results (e.g., read
current openId from a ref or compare openId at the time of resolution) and only
call setTriggers/setLinkedCosmeticIds/setAllCosmetics when the resolved id still
matches the current openId (or use an AbortController per loadDetails to cancel
stale requests), keeping function names loadDetails, toggleOpen, setTriggers,
setLinkedCosmeticIds, setAllCosmetics and openId as the reference points.
---
Nitpick comments:
In `@backend/routes/auth.py`:
- Around line 44-51: Move the test-only helper _stamp_last_sign_in_for_test out
of the production routes.auth module into the test suite and hoist the datetime
import to module scope: add a module-level import like "from datetime import
datetime as _dt, timezone as _tz" in routes.auth, replace any local in-function
imports and uses of datetime.now(timezone.utc) with _dt(..., tzinfo=_tz.utc) (or
calls to _dt.now(_tz.utc)), and remove the test helper from the production file
so tests can monkeypatch the module-level _dt/_tz instead; update any references
to _stamp_last_sign_in_for_test in tests to call the moved helper in the test
package.
In `@backend/services/users_search.py`:
- Around line 20-26: The _attach_roles function issues an individual
Supabase/PostgREST request per user causing N+1 queries; change it to fetch all
roles in one batch by querying table("user_roles") with a single select using
the user_id in.(<list_of_user_ids>) filter, retrieving "user_id" and the nested
"roles(...)" fields, then group the returned rows by user_id and assign
user["roles"] = [r["roles"] for r in grouped[user_id]] (or [] if none) for each
user; keep function name _attach_roles, use the same table("user_roles") select
call but replace per-user filters with the in filter and ensure you handle empty
input lists and absent role entries.

🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

• Push a commit to this branch (recommended)
• Create a new PR with the fixes

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro Plus

Run ID: e976a667-d4b6-4fac-9347-f27bc7d43539

📥 Commits

Reviewing files that changed from the base of the PR and between db5ba48 and 1c23a86.

📒 Files selected for processing (16)

• backend/db/connection.py
• backend/db/migration_admin_portal.sql
• backend/models/__init__.py
• backend/routes/admin.py
• backend/routes/auth.py
• backend/services/admin_audit.py
• backend/services/users_search.py
• backend/tests/test_admin_audit.py
• backend/tests/test_admin_routes.py
• backend/tests/test_auth_state.py
• backend/tests/test_supabase.py
• backend/tests/test_users_search.py
• docs/superpowers/plans/2026-05-04-admin-portal.md
• frontend/src/components/screens/Admin.tsx
• frontend/src/lib/api.ts
• frontend/src/lib/types.ts

[coderabbitai]

⚠️ Potential issue | 🟠 Major | ⚡ Quick win

Do not trust client-supplied granted_by.

granted_by = body.granted_by or actor lets the caller forge who granted the role. That breaks the server-side provenance this PR is adding and makes both the row data and the audit trail unreliable. Stamp granted_by from get_session_user_id(request) unconditionally.

Suggested fix

 def assign_role(body: AssignRoleBody, request: Request):
require_admin(request)
actor = get_session_user_id(request)
- granted_by = body.granted_by or actor+ granted_by = actor
table("user_roles").upsert(
{
"user_id": body.user_id,
"role_id": body.role_id,
"granted_by": granted_by,

📝 Committable suggestion

‼️IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change

	actor=get_session_user_id(request)
	granted_by=body.granted_byoractor
	table("user_roles").upsert(
	{
	"user_id": body.user_id,
	"role_id": body.role_id,
	"granted_by": granted_by,
	"granted_at": datetime.now(timezone.utc).isoformat(),
	},
	on_conflict="user_id,role_id",
	)
	log_admin_action(
	actor_id=actor, action="role.assign", target_type="role", target_id=body.role_id,
	payload={"user_id": body.user_id, "granted_by": granted_by},
	)
	actor=get_session_user_id(request)
	granted_by=actor
	table("user_roles").upsert(
	{
	"user_id": body.user_id,
	"role_id": body.role_id,
	"granted_by": granted_by,
	"granted_at": datetime.now(timezone.utc).isoformat(),
	},
	on_conflict="user_id,role_id",
	)
	log_admin_action(
	actor_id=actor, action="role.assign", target_type="role", target_id=body.role_id,
	payload={"user_id": body.user_id, "granted_by": granted_by},
	)

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@backend/routes/admin.py` around lines 95 - 109, The code currently allows a
client to override the grantor via body.granted_by; instead, always derive the
grantor from the session: set granted_by = get_session_user_id(request) (i.e.,
the existing actor variable) and remove usage of body.granted_by when building
the upsert payload and the log_admin_action payload so both
table("user_roles").upsert and log_admin_action use the trusted actor value
only.

[coderabbitai]

⚠️ Potential issue | 🟠 Major | 🏗️ Heavy lift

The “approvals” series is grouped by signup date, not approval date.

approvals_by_day is incremented from created_at for users whose current is_approved is true. A user approved today will be counted on their original signup day, so the Analytics tab’s “Approvals · last 30 days” chart is wrong. This needs either a real approval timestamp or a renamed metric that matches the current derivation.

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@backend/routes/admin.py` around lines 558 - 570, The approvals series is
currently counted by signup date (using created_at) rather than the approval
timestamp; update the logic in the loop that builds
by_day_approvals/approvals_by_day to use an approval timestamp (e.g., check for
an approved_at or similar field on each user instead of created_at) and parse
that timestamp into a date (like you do with created_at) before incrementing
by_day_approvals[d.isoformat()]. If there is no approval timestamp available,
either add/compute one at approval time (e.g., set approved_at when is_approved
transitions true) or rename the metric from “Approvals” to something like
“Approved users by signup date” to reflect the current derivation (refer to
variables/users, created_at, is_approved, by_day_approvals, approvals_by_day,
window).

[coderabbitai]

⚠️ Potential issue | 🟠 Major | ⚡ Quick win

log.exception serializes user identifiers (actor_id, target_id) into application logs

row includes actor_id and target_id, which encode Google sub-claim–based user IDs — persistent identifiers that can be linked back to real persons. If application logs are forwarded to external aggregators this violates GDPR/CCPA guidance against logging user identifiers.

🔒 Proposed fix — log only non-identifying metadata

- log.exception("admin_audit_log write failed: %s", row)+ log.exception(+ "admin_audit_log write failed action=%s target_type=%s",+ row.get("action"),+ row.get("target_type"),+ )

As per coding guidelines, logging sensitive data such as user identifiers is classified as a compliance/privacy risk (GDPR/CCPA).

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@backend/services/admin_audit.py` at line 31, The log currently calls
log.exception("admin_audit_log write failed: %s", row) which will serialize
sensitive identifiers (row.actor_id, row.target_id); change the logging to avoid
including identifying fields by creating a sanitized version of row that omits
or redacts actor_id and target_id and log that instead (or log only
non-identifying metadata such as event type, timestamp, and error context).
Update the call site using log.exception to pass the sanitized data or a minimal
message and error context rather than the full row so actor_id/target_id are
never written to logs.

[coderabbitai]

⚠️ Potential issue | 🟠 Major | ⚡ Quick win

Guard achievement detail state against stale async responses.

loadDetails() writes shared triggers / linkedCosmeticIds state without verifying that the response still belongs to the currently expanded achievement. If an admin opens A and quickly opens B, A’s slower response can overwrite B’s panel, and the inline trigger actions can then target the wrong record IDs.

Suggested fix

+ const detailsRequestRef = React.useRef(0);+ const openIdRef = React.useRef<string | null>(null);+
const loadDetails = React.useCallback(async (id: string) => {
+ const requestId = ++detailsRequestRef.current;
try {
const [t, l, c] = await Promise.all([
adminListTriggers(id),
adminListAchievementCosmetics(id),
allCosmetics.length ? Promise.resolve({ cosmetics: allCosmetics }) : adminListCosmetics(),
]);
+ if (detailsRequestRef.current !== requestId || openIdRef.current !== id) return;
setTriggers(t.triggers || []);
setLinkedCosmeticIds((l.links || []).map(x => x.cosmetic_id));
if (!allCosmetics.length) setAllCosmetics(c.cosmetics || []);
} catch (err) {
toast.error(`Detail load failed: ${String(err)}`);
}
}, [allCosmetics, toast]);
const toggleOpen = (id: string) => {
- if (openId === id) { setOpenId(null); return; }+ if (openId === id) {+ openIdRef.current = null;+ setOpenId(null);+ return;+ }+ openIdRef.current = id;+ setTriggers([]);+ setLinkedCosmeticIds([]);
setOpenId(id);
loadDetails(id);
};

📝 Committable suggestion

‼️IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change

	constloadDetails=React.useCallback(async(id: string)=>{
	try{
	const[t,l,c]=awaitPromise.all([
	adminListTriggers(id),
	adminListAchievementCosmetics(id),
	allCosmetics.length ? Promise.resolve({cosmetics: allCosmetics}) : adminListCosmetics(),
	]);
	setTriggers(t.triggers||[]);
	setLinkedCosmeticIds((l.links||[]).map(x=>x.cosmetic_id));
	if(!allCosmetics.length)setAllCosmetics(c.cosmetics||[]);
	}catch(err){
	toast.error(`Detail load failed: ${String(err)}`);
	}
	},[allCosmetics,toast]);
	consttoggleOpen=(id: string)=>{
	if(openId===id){setOpenId(null);return;}
	setOpenId(id);
	loadDetails(id);
	};
	constdetailsRequestRef=React.useRef(0);
	constopenIdRef=React.useRef<string|null>(null);
	constloadDetails=React.useCallback(async(id: string)=>{
	constrequestId=++detailsRequestRef.current;
	try{
	const[t,l,c]=awaitPromise.all([
	adminListTriggers(id),
	adminListAchievementCosmetics(id),
	allCosmetics.length ? Promise.resolve({cosmetics: allCosmetics}) : adminListCosmetics(),
	]);
	if(detailsRequestRef.current!==requestId||openIdRef.current!==id)return;
	setTriggers(t.triggers||[]);
	setLinkedCosmeticIds((l.links||[]).map(x=>x.cosmetic_id));
	if(!allCosmetics.length)setAllCosmetics(c.cosmetics||[]);
	}catch(err){
	toast.error(`Detail load failed: ${String(err)}`);
	}
	},[allCosmetics,toast]);
	consttoggleOpen=(id: string)=>{
	if(openId===id){
	openIdRef.current=null;
	setOpenId(null);
	return;
	}
	openIdRef.current=id;
	setTriggers([]);
	setLinkedCosmeticIds([]);
	setOpenId(id);
	loadDetails(id);
	};

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@frontend/src/components/screens/Admin.tsx` around lines 492 - 511,
loadDetails writes shared state (setTriggers, setLinkedCosmeticIds,
setAllCosmetics) unguarded so a stale async response for a previously requested
id can overwrite the currently open achievement; fix by capturing/validating the
expected open id before applying results (e.g., read current openId from a ref
or compare openId at the time of resolution) and only call
setTriggers/setLinkedCosmeticIds/setAllCosmetics when the resolved id still
matches the current openId (or use an AbortController per loadDetails to cancel
stale requests), keeping function names loadDetails, toggleOpen, setTriggers,
setLinkedCosmeticIds, setAllCosmetics and openId as the reference points.

[AndresL230]

Closes#69.

Summary

• Pagination + server-side search on /api/admin/users; new last_sign_in_at column written on every Google callback.
• New endpoints: unapprove user; achievement triggers list/update/delete; achievement_cosmetics + role_cosmetics link/unlink/list; admin_audit_log read; analytics overview (totals + 30-day series + role counts); allowlist approve/revoke.
• Self-protection guards: cannot revoke own admin, cannot revoke last admin, cannot delete admin role, cannot unapprove self.
• Audit log written for every admin mutation via a single sanctioned helper (services/admin_audit.py).
• Frontend Admin.tsx gains Allowlist + Audit tabs and rewires Users / Achievements / Cosmetics / Analytics to the new endpoints — all using existing card / chip / btn / label-micro / var(--accent) design tokens (no new design language).
• assign_role is now idempotent (upsert on (user_id, role_id)); granted_by auto-fills from session.

Schema

• New table admin_audit_log (actor_id, action, target_type, target_id, payload jsonb, created_at) with FK actor_id ON DELETE RESTRICT and read-path indexes on created_at / actor / target.
• New column users.last_sign_in_at and index on users.created_at.

Migration file: backend/db/migration_admin_portal.sql — run once in the Supabase SQL editor.

Test plan

• Run the SQL migration in the dev Supabase project.
• cd backend && python -m pytest tests/test_admin_routes.py tests/test_admin_audit.py tests/test_users_search.py tests/test_supabase.py tests/test_auth_state.py -q (81 tests, all green on this branch).
• cd frontend && npx tsc --noEmit (clean).
• Manual smoke: visit /admin as an admin and walk every tab.
  • Users: paginate, debounced search, approve/unapprove, role assign/revoke.
  • Allowlist: add an email, revoke, re-approve.
  • Roles: try deleting the admin role (must 409); try revoking own admin (409); reassigning an existing role must NOT 500.
  • Achievements: Manage on a row, add/edit/delete a trigger, link/unlink a cosmetic chip.
  • Cosmetics: Manage on a row, link/unlink a role chip.
  • Analytics: totals match DB, sparkline tooltips show daily counts.
  • Audit: every action above appears as a row, filters by action/target_type work.

Out of scope

• Impersonate user (mentioned in feat(admin): admin portal UI — users, allowlist, roles, achievements, cosmetics #69 — touches session minting; deferred).
• EmailStr validation on AllowlistEmailBody — currently email: str. Worth tightening in a follow-up.

Plan

Implementation followed docs/superpowers/plans/2026-05-04-admin-portal.md (committed in this branch).

Summary by CodeRabbit

• New Features
  • Admin dashboard expanded with allowlist management and audit log sections
  • User management improved with pagination, search, and last sign-in date tracking
  • Achievement trigger and cosmetic linking management tools added
  • Admin audit logging tracks all administrative actions system-wide
  • Analytics dashboard displays signup trends and role distribution metrics

[coderabbitai]

📝 Walkthrough

Walkthrough

This PR implements a comprehensive admin portal spanning backend and frontend. It adds database schema for audit logging, a new select_with_count() pagination helper, admin audit tracking across mutations, paginated/searchable user listing, achievement trigger and cosmetic-linking management, role protection rules, allowlist administration, analytics computation, and a fully featured admin UI with tabs for users (paginated/searchable), allowlist, roles, achievements, cosmetics, and audit logs.

Changes

Admin Portal – Complete Implementation

Layer / File(s)	Summary
Database Schema & Indexing backend/db/migration_admin_portal.sql	Creates admin_audit_log table with actor/action/target metadata and timestamps; adds users.last_sign_in_at column; indexes created for time-based and actor-based audit queries.
Database Query Helpers backend/db/connection.py	Adds SupabaseTable.select_with_count() synchronous method that returns (rows, total) by parsing Content-Range header; supports offset alongside limit.
Request Body Models backend/models/__init__.py	Introduces UpdateAchievementTriggerBody, LinkAchievementCosmeticBody, LinkRoleCosmeticBody, and AllowlistEmailBody for admin endpoints.
Audit Service backend/services/admin_audit.py	Adds log_admin_action() to insert audit rows into admin_audit_log with error suppression; ensures audit failures don't block originating operations.
User Pagination & Search backend/services/users_search.py	Adds paginate_users() implementing DB-level pagination when no query provided and decrypt-all-then-filter approach for name/email search; attaches per-user roles and returns {users, total, page, page_size}.
Admin Routes backend/routes/admin.py	Adds/extends 20+ endpoints: role CRUD with admin-role self-protection, achievement management with trigger CRUD and cosmetic linking, cosmetic management, user listing (paginated), approval/unapproval with self-guard, role/achievement-cosmetic linking, allowlist administration, audit log querying, and analytics overview computation. All mutations emit audit events via log_admin_action().
Auth Integration backend/routes/auth.py	Tracks last_sign_in_at on Google OAuth callback (both existing and new users); adds test helper _stamp_last_sign_in_for_test() to support test scenarios without full OAuth flow.
Backend Tests backend/tests/test_admin_audit.py, test_admin_routes.py, test_auth_state.py, test_supabase.py, test_users_search.py	Comprehensive unit and integration test coverage: audit logging (row insertion and error suppression), user pagination with/without search and role attachment, role protection rules, achievement/cosmetic/allowlist/audit operations, analytics computation, and select_with_count() parsing.
Frontend Types frontend/src/lib/types.ts	Adds AllowlistEmail, AchievementTrigger, AdminAuditEntry, analytics-related types (AnalyticsTotals, AnalyticsDayPoint, AnalyticsRoleCount, AnalyticsOverview), AdminUserListItem, and PaginatedUsers; extends Cosmetic with optional unlock_source field.
Frontend API Wrappers frontend/src/lib/api.ts	Adds ~17 new admin helpers for paginated user listing (with search), user unapproval, role-cosmetic linking, achievement trigger CRUD, achievement-cosmetic linking, allowlist management, audit log querying with filters, and analytics overview retrieval.
Frontend Admin UI frontend/src/components/screens/Admin.tsx	Expands Admin component to include Users tab (paginated with debounced search, approve/unapprove, last-seen display), Allowlist tab (email list with approve/revoke), Audit tab (paginated audit log with action/target filters and payload display), and extends Roles, Achievements, Cosmetics tabs with row-expansion UIs for managing linked cosmetics/roles and achievement triggers. Refactors Analytics to use adminAnalyticsOverview() with metric cards and sparklines. Upgrades CatalogRow component to support expandable management sections.

Sequence Diagram

sequenceDiagram
participant Admin as Admin User
participant FrontendUI as Frontend UI
participant API as Admin API
participant Auth as Auth Service
participant DB as Database
rect rgba(100, 200, 150, 0.5)
note over Admin,DB: User Search & Pagination Flow
Admin->>FrontendUI: Enter search query, navigate to page 2
FrontendUI->>FrontendUI: Debounce search input
FrontendUI->>API: adminFetchUsers({q: "alice", page: 2, page_size: 50})
API->>Auth: get_session_user_id() → verify admin
API->>DB: paginate_users(q="alice", page=2, page_size=50)
alt Query provided (search)
DB->>DB: Decrypt all users, filter by substring match
DB->>DB: Compute total from filtered set, slice page
else No query (list)
DB->>DB: select_with_count (offset=50, limit=50)
end
DB->>DB: Attach roles per user from user_roles
DB-->>API: {users: [...], total: 137, page: 2, page_size: 50}
API-->>FrontendUI: PaginatedUsers response
FrontendUI->>FrontendUI: Render users table with Last Seen, Joined dates
FrontendUI-->>Admin: Display page 2 of 3, approve/unapprove buttons
end
rect rgba(150, 180, 220, 0.5)
note over Admin,DB: Admin Audit Logging Flow
Admin->>FrontendUI: Click "Approve" button for user
FrontendUI->>API: adminApproveUser(userId)
API->>Auth: get_session_user_id() → actor_id
API->>DB: UPDATE users SET is_approved=true WHERE id=userId
API->>DB: log_admin_action(actor_id, "user.approve", "user", userId, {...})
DB->>DB: INSERT INTO admin_audit_log (actor_id, action, target_type, target_id, created_at)
DB-->>API: Audit row inserted (or error caught & logged)
API-->>FrontendUI: {approved: true}
FrontendUI-->>Admin: Show success, refresh user list
Admin->>FrontendUI: Visit Audit tab
FrontendUI->>API: adminAuditLog({page: 1, action: "user.approve"})
DB->>DB: SELECT * FROM admin_audit_log WHERE action='user.approve' ORDER BY created_at DESC
DB-->>API: Paginated audit rows
API-->>FrontendUI: Audit entries with timestamps, payload JSON
FrontendUI-->>Admin: Display filtered audit log
end
rect rgba(200, 150, 180, 0.5)
note over Admin,DB: Role-Cosmetic Linking Flow
Admin->>FrontendUI: Expand role row, toggle linked cosmetic
FrontendUI->>API: adminLinkRoleCosmetic(roleId, cosmeticId)
API->>Auth: get_session_user_id() → actor_id
API->>DB: UPSERT INTO role_cosmetics (role_id, cosmetic_id)
API->>DB: log_admin_action(actor_id, "role_cosmetic.link", "role_cosmetic", null, {...})
DB->>DB: INSERT INTO admin_audit_log
DB-->>API: Link upserted, audit logged
API-->>FrontendUI: {linked: true}
FrontendUI->>FrontendUI: Update expanded row UI to reflect new link
FrontendUI-->>Admin: Show cosmetic chip as linked in role details
end

Loading

Estimated Code Review Effort

🎯 4 (Complex) | ⏱️ ~45 minutes

Possibly Related PRs

• SaplingLearn/Sapling#56: Both PRs modify backend/routes/auth.py and the users table schema; PR #56 adds user field persistence during OAuth callback while this PR extends that to include last_sign_in_at tracking.

Poem

🐰 A warren of admins, now armed and aware,
With audit trails woven through each admin's care,
Cosmetics and triggers link up with grace,
While pages of users scroll at a measured pace,
The portal is gleaming—no more fumbling in the dark!

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 9.26% which is insufficient. The required threshold is 80.00%.	Write docstrings for the functions missing them to satisfy the coverage threshold.

✅ Passed checks (4 passed)

Check name	Status	Explanation
Title check	✅ Passed	The title accurately reflects the main changes: pagination, audit log, allowlist, and analytics for the admin portal. It is specific, concise, and directly related to the primary changeset.
Description check	✅ Passed	The description is comprehensive and follows the template structure with Summary, Changes Made (implicit via detailed sections), Testing checklist, and Out of Scope items. All key implementation details are documented.
Linked Issues check	✅ Passed	The PR fully addresses issue #69 objectives: pagination and search for users [#69], unapprove endpoint [#69], audit logging for mutations [#69], role protection guards [#69], achievement triggers management [#69], role/achievement cosmetic linking [#69], allowlist endpoints [#69], analytics overview [#69], and role assignment idempotency [#69].
Out of Scope Changes check	✅ Passed	All code changes are directly aligned with the stated objectives. Out-of-scope items (impersonation, stricter email validation) are explicitly deferred and not included in the changeset.

_{✏️ Tip: You can configure your own custom pre-merge checks in the settings.}

✨ Finishing Touches

📝 Generate docstrings

• Create stacked PR
• Commit on current branch

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Commit unit tests in branch feat/admin-portal

---

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

[cloudflare-workers-and-pages]

Deploying with Cloudflare Workers

The latest updates on your project. Learn more about integrating Git with Workers.

Status	Name	Latest Commit	Preview URL	Updated (UTC)
✅ Deployment successful! View logs	frontend	1c23a86	Commit Preview URL Branch Preview URL	May 04 2026, 11:23 PM

[coderabbitai]

Actionable comments posted: 4

🧹 Nitpick comments (2)

backend/services/users_search.py (1)

20-26: ⚡ Quick win

N+1 queries in _attach_roles — collapse to a single batch request

Each call to _attach_roles issues one Supabase REST request per user. With _MAX_PAGE_SIZE = 200 that's up to 200 sequential HTTP round-trips per admin page load. PostgREST's in.(...) filter fetches all roles in one request:

♻️ Proposed refactor

 def _attach_roles(users: list[dict]) -> None:
- for user in users:- rows = table("user_roles").select(- "roles(id,name,slug,color,icon,description,is_staff_assigned,is_earnable,display_priority)",- filters={"user_id": f"eq.{user['id']}"},- )- user["roles"] = [r["roles"] for r in (rows or []) if r.get("roles")]+ if not users:+ return+ ids = ",".join(u["id"] for u in users)+ all_rows = table("user_roles").select(+ "user_id,roles(id,name,slug,color,icon,description,is_staff_assigned,is_earnable,display_priority)",+ filters={"user_id": f"in.({ids})"},+ ) or []+ role_map: dict[str, list] = {}+ for r in all_rows:+ if r.get("roles"):+ role_map.setdefault(r["user_id"], []).append(r["roles"])+ for user in users:+ user["roles"] = role_map.get(user["id"], [])

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@backend/services/users_search.py` around lines 20 - 26, The _attach_roles
function issues an individual Supabase/PostgREST request per user causing N+1
queries; change it to fetch all roles in one batch by querying
table("user_roles") with a single select using the user_id
in.(<list_of_user_ids>) filter, retrieving "user_id" and the nested "roles(...)"
fields, then group the returned rows by user_id and assign user["roles"] =
[r["roles"] for r in grouped[user_id]] (or [] if none) for each user; keep
function name _attach_roles, use the same table("user_roles") select call but
replace per-user filters with the in filter and ensure you handle empty input
lists and absent role entries.

backend/routes/auth.py (1)

44-51: ⚡ Quick win

Move _stamp_last_sign_in_for_test out of the production module and hoist the datetime import

Two related issues:

1. _stamp_last_sign_in_for_test is a test-only helper sitting in the production auth.py module. Any package import of routes.auth exposes it, and it exists only because datetime.now is buried inside conditional branches where standard monkeypatch can't reach it.
2. from datetime import datetime as _dt, timezone as _tz is duplicated in three places (lines 47, 325, 343). Python caches module imports, so this isn't a correctness bug, but it's unnecessarily noisy.

The cleanest resolution is to promote the import to module level, which then makes routes.auth.datetime directly patchable in tests and removes the need for the test seam entirely:

♻️ Proposed refactor

+from datetime import datetime as _dt, timezone as _tz
import json
...
-def _stamp_last_sign_in_for_test(user_id: str) -> None:- """Test seam: write last_sign_in_at ..."""- from datetime import datetime, timezone- table("users").update(- {"last_sign_in_at": datetime.now(timezone.utc).isoformat()},- filters={"id": f"eq.{user_id}"},- )--
...
if existing:
- from datetime import datetime as _dt, timezone as _tz
table("users").update({
...
"last_sign_in_at": _dt.now(_tz.utc).isoformat(),
}, ...)
else:
- from datetime import datetime as _dt, timezone as _tz
table("users").insert({
...
"last_sign_in_at": _dt.now(_tz.utc).isoformat(),
})

With _dt at module level, test_auth_state.py can replace the test helper with a direct monkeypatch:

# test_auth_state.pyimportroutes.authasauth_moduledeftest_callback_stamps_last_sign_in(monkeypatch):
fixed=_dt(2024, 1, 1, tzinfo=_tz.utc)
monkeypatch.setattr(auth_module, "_dt", lambda*a, **kw: fixed)
...

Also applies to: 325-325, 343-343

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@backend/routes/auth.py` around lines 44 - 51, Move the test-only helper
_stamp_last_sign_in_for_test out of the production routes.auth module into the
test suite and hoist the datetime import to module scope: add a module-level
import like "from datetime import datetime as _dt, timezone as _tz" in
routes.auth, replace any local in-function imports and uses of
datetime.now(timezone.utc) with _dt(..., tzinfo=_tz.utc) (or calls to
_dt.now(_tz.utc)), and remove the test helper from the production file so tests
can monkeypatch the module-level _dt/_tz instead; update any references to
_stamp_last_sign_in_for_test in tests to call the moved helper in the test
package.

🤖 Prompt for all review comments with AI agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@backend/routes/admin.py`:
- Around line 558-570: The approvals series is currently counted by signup date
(using created_at) rather than the approval timestamp; update the logic in the
loop that builds by_day_approvals/approvals_by_day to use an approval timestamp
(e.g., check for an approved_at or similar field on each user instead of
created_at) and parse that timestamp into a date (like you do with created_at)
before incrementing by_day_approvals[d.isoformat()]. If there is no approval
timestamp available, either add/compute one at approval time (e.g., set
approved_at when is_approved transitions true) or rename the metric from
“Approvals” to something like “Approved users by signup date” to reflect the
current derivation (refer to variables/users, created_at, is_approved,
by_day_approvals, approvals_by_day, window).
- Around line 95-109: The code currently allows a client to override the grantor
via body.granted_by; instead, always derive the grantor from the session: set
granted_by = get_session_user_id(request) (i.e., the existing actor variable)
and remove usage of body.granted_by when building the upsert payload and the
log_admin_action payload so both table("user_roles").upsert and log_admin_action
use the trusted actor value only.
In `@backend/services/admin_audit.py`:
- Line 31: The log currently calls log.exception("admin_audit_log write failed:
%s", row) which will serialize sensitive identifiers (row.actor_id,
row.target_id); change the logging to avoid including identifying fields by
creating a sanitized version of row that omits or redacts actor_id and target_id
and log that instead (or log only non-identifying metadata such as event type,
timestamp, and error context). Update the call site using log.exception to pass
the sanitized data or a minimal message and error context rather than the full
row so actor_id/target_id are never written to logs.
In `@frontend/src/components/screens/Admin.tsx`:
- Around line 492-511: loadDetails writes shared state (setTriggers,
setLinkedCosmeticIds, setAllCosmetics) unguarded so a stale async response for a
previously requested id can overwrite the currently open achievement; fix by
capturing/validating the expected open id before applying results (e.g., read
current openId from a ref or compare openId at the time of resolution) and only
call setTriggers/setLinkedCosmeticIds/setAllCosmetics when the resolved id still
matches the current openId (or use an AbortController per loadDetails to cancel
stale requests), keeping function names loadDetails, toggleOpen, setTriggers,
setLinkedCosmeticIds, setAllCosmetics and openId as the reference points.
---
Nitpick comments:
In `@backend/routes/auth.py`:
- Around line 44-51: Move the test-only helper _stamp_last_sign_in_for_test out
of the production routes.auth module into the test suite and hoist the datetime
import to module scope: add a module-level import like "from datetime import
datetime as _dt, timezone as _tz" in routes.auth, replace any local in-function
imports and uses of datetime.now(timezone.utc) with _dt(..., tzinfo=_tz.utc) (or
calls to _dt.now(_tz.utc)), and remove the test helper from the production file
so tests can monkeypatch the module-level _dt/_tz instead; update any references
to _stamp_last_sign_in_for_test in tests to call the moved helper in the test
package.
In `@backend/services/users_search.py`:
- Around line 20-26: The _attach_roles function issues an individual
Supabase/PostgREST request per user causing N+1 queries; change it to fetch all
roles in one batch by querying table("user_roles") with a single select using
the user_id in.(<list_of_user_ids>) filter, retrieving "user_id" and the nested
"roles(...)" fields, then group the returned rows by user_id and assign
user["roles"] = [r["roles"] for r in grouped[user_id]] (or [] if none) for each
user; keep function name _attach_roles, use the same table("user_roles") select
call but replace per-user filters with the in filter and ensure you handle empty
input lists and absent role entries.

🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

• Push a commit to this branch (recommended)
• Create a new PR with the fixes

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro Plus

Run ID: e976a667-d4b6-4fac-9347-f27bc7d43539

📥 Commits

Reviewing files that changed from the base of the PR and between db5ba48 and 1c23a86.

📒 Files selected for processing (16)

• backend/db/connection.py
• backend/db/migration_admin_portal.sql
• backend/models/__init__.py
• backend/routes/admin.py
• backend/routes/auth.py
• backend/services/admin_audit.py
• backend/services/users_search.py
• backend/tests/test_admin_audit.py
• backend/tests/test_admin_routes.py
• backend/tests/test_auth_state.py
• backend/tests/test_supabase.py
• backend/tests/test_users_search.py
• docs/superpowers/plans/2026-05-04-admin-portal.md
• frontend/src/components/screens/Admin.tsx
• frontend/src/lib/api.ts
• frontend/src/lib/types.ts

[coderabbitai]

⚠️ Potential issue | 🟠 Major | ⚡ Quick win

Do not trust client-supplied granted_by.

granted_by = body.granted_by or actor lets the caller forge who granted the role. That breaks the server-side provenance this PR is adding and makes both the row data and the audit trail unreliable. Stamp granted_by from get_session_user_id(request) unconditionally.

Suggested fix

 def assign_role(body: AssignRoleBody, request: Request):
require_admin(request)
actor = get_session_user_id(request)
- granted_by = body.granted_by or actor+ granted_by = actor
table("user_roles").upsert(
{
"user_id": body.user_id,
"role_id": body.role_id,
"granted_by": granted_by,

📝 Committable suggestion

‼️IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change

	actor=get_session_user_id(request)
	granted_by=body.granted_byoractor
	table("user_roles").upsert(
	{
	"user_id": body.user_id,
	"role_id": body.role_id,
	"granted_by": granted_by,
	"granted_at": datetime.now(timezone.utc).isoformat(),
	},
	on_conflict="user_id,role_id",
	)
	log_admin_action(
	actor_id=actor, action="role.assign", target_type="role", target_id=body.role_id,
	payload={"user_id": body.user_id, "granted_by": granted_by},
	)
	actor=get_session_user_id(request)
	granted_by=actor
	table("user_roles").upsert(
	{
	"user_id": body.user_id,
	"role_id": body.role_id,
	"granted_by": granted_by,
	"granted_at": datetime.now(timezone.utc).isoformat(),
	},
	on_conflict="user_id,role_id",
	)
	log_admin_action(
	actor_id=actor, action="role.assign", target_type="role", target_id=body.role_id,
	payload={"user_id": body.user_id, "granted_by": granted_by},
	)

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@backend/routes/admin.py` around lines 95 - 109, The code currently allows a
client to override the grantor via body.granted_by; instead, always derive the
grantor from the session: set granted_by = get_session_user_id(request) (i.e.,
the existing actor variable) and remove usage of body.granted_by when building
the upsert payload and the log_admin_action payload so both
table("user_roles").upsert and log_admin_action use the trusted actor value
only.

[coderabbitai]

⚠️ Potential issue | 🟠 Major | 🏗️ Heavy lift

The “approvals” series is grouped by signup date, not approval date.

approvals_by_day is incremented from created_at for users whose current is_approved is true. A user approved today will be counted on their original signup day, so the Analytics tab’s “Approvals · last 30 days” chart is wrong. This needs either a real approval timestamp or a renamed metric that matches the current derivation.

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@backend/routes/admin.py` around lines 558 - 570, The approvals series is
currently counted by signup date (using created_at) rather than the approval
timestamp; update the logic in the loop that builds
by_day_approvals/approvals_by_day to use an approval timestamp (e.g., check for
an approved_at or similar field on each user instead of created_at) and parse
that timestamp into a date (like you do with created_at) before incrementing
by_day_approvals[d.isoformat()]. If there is no approval timestamp available,
either add/compute one at approval time (e.g., set approved_at when is_approved
transitions true) or rename the metric from “Approvals” to something like
“Approved users by signup date” to reflect the current derivation (refer to
variables/users, created_at, is_approved, by_day_approvals, approvals_by_day,
window).

[coderabbitai]

⚠️ Potential issue | 🟠 Major | ⚡ Quick win

log.exception serializes user identifiers (actor_id, target_id) into application logs

row includes actor_id and target_id, which encode Google sub-claim–based user IDs — persistent identifiers that can be linked back to real persons. If application logs are forwarded to external aggregators this violates GDPR/CCPA guidance against logging user identifiers.

🔒 Proposed fix — log only non-identifying metadata

- log.exception("admin_audit_log write failed: %s", row)+ log.exception(+ "admin_audit_log write failed action=%s target_type=%s",+ row.get("action"),+ row.get("target_type"),+ )

As per coding guidelines, logging sensitive data such as user identifiers is classified as a compliance/privacy risk (GDPR/CCPA).

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@backend/services/admin_audit.py` at line 31, The log currently calls
log.exception("admin_audit_log write failed: %s", row) which will serialize
sensitive identifiers (row.actor_id, row.target_id); change the logging to avoid
including identifying fields by creating a sanitized version of row that omits
or redacts actor_id and target_id and log that instead (or log only
non-identifying metadata such as event type, timestamp, and error context).
Update the call site using log.exception to pass the sanitized data or a minimal
message and error context rather than the full row so actor_id/target_id are
never written to logs.

[coderabbitai]

⚠️ Potential issue | 🟠 Major | ⚡ Quick win

Guard achievement detail state against stale async responses.

loadDetails() writes shared triggers / linkedCosmeticIds state without verifying that the response still belongs to the currently expanded achievement. If an admin opens A and quickly opens B, A’s slower response can overwrite B’s panel, and the inline trigger actions can then target the wrong record IDs.

Suggested fix

+ const detailsRequestRef = React.useRef(0);+ const openIdRef = React.useRef<string | null>(null);+
const loadDetails = React.useCallback(async (id: string) => {
+ const requestId = ++detailsRequestRef.current;
try {
const [t, l, c] = await Promise.all([
adminListTriggers(id),
adminListAchievementCosmetics(id),
allCosmetics.length ? Promise.resolve({ cosmetics: allCosmetics }) : adminListCosmetics(),
]);
+ if (detailsRequestRef.current !== requestId || openIdRef.current !== id) return;
setTriggers(t.triggers || []);
setLinkedCosmeticIds((l.links || []).map(x => x.cosmetic_id));
if (!allCosmetics.length) setAllCosmetics(c.cosmetics || []);
} catch (err) {
toast.error(`Detail load failed: ${String(err)}`);
}
}, [allCosmetics, toast]);
const toggleOpen = (id: string) => {
- if (openId === id) { setOpenId(null); return; }+ if (openId === id) {+ openIdRef.current = null;+ setOpenId(null);+ return;+ }+ openIdRef.current = id;+ setTriggers([]);+ setLinkedCosmeticIds([]);
setOpenId(id);
loadDetails(id);
};

📝 Committable suggestion

‼️IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change

	constloadDetails=React.useCallback(async(id: string)=>{
	try{
	const[t,l,c]=awaitPromise.all([
	adminListTriggers(id),
	adminListAchievementCosmetics(id),
	allCosmetics.length ? Promise.resolve({cosmetics: allCosmetics}) : adminListCosmetics(),
	]);
	setTriggers(t.triggers||[]);
	setLinkedCosmeticIds((l.links||[]).map(x=>x.cosmetic_id));
	if(!allCosmetics.length)setAllCosmetics(c.cosmetics||[]);
	}catch(err){
	toast.error(`Detail load failed: ${String(err)}`);
	}
	},[allCosmetics,toast]);
	consttoggleOpen=(id: string)=>{
	if(openId===id){setOpenId(null);return;}
	setOpenId(id);
	loadDetails(id);
	};
	constdetailsRequestRef=React.useRef(0);
	constopenIdRef=React.useRef<string|null>(null);
	constloadDetails=React.useCallback(async(id: string)=>{
	constrequestId=++detailsRequestRef.current;
	try{
	const[t,l,c]=awaitPromise.all([
	adminListTriggers(id),
	adminListAchievementCosmetics(id),
	allCosmetics.length ? Promise.resolve({cosmetics: allCosmetics}) : adminListCosmetics(),
	]);
	if(detailsRequestRef.current!==requestId||openIdRef.current!==id)return;
	setTriggers(t.triggers||[]);
	setLinkedCosmeticIds((l.links||[]).map(x=>x.cosmetic_id));
	if(!allCosmetics.length)setAllCosmetics(c.cosmetics||[]);
	}catch(err){
	toast.error(`Detail load failed: ${String(err)}`);
	}
	},[allCosmetics,toast]);
	consttoggleOpen=(id: string)=>{
	if(openId===id){
	openIdRef.current=null;
	setOpenId(null);
	return;
	}
	openIdRef.current=id;
	setTriggers([]);
	setLinkedCosmeticIds([]);
	setOpenId(id);
	loadDetails(id);
	};

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@frontend/src/components/screens/Admin.tsx` around lines 492 - 511,
loadDetails writes shared state (setTriggers, setLinkedCosmeticIds,
setAllCosmetics) unguarded so a stale async response for a previously requested
id can overwrite the currently open achievement; fix by capturing/validating the
expected open id before applying results (e.g., read current openId from a ref
or compare openId at the time of resolution) and only call
setTriggers/setLinkedCosmeticIds/setAllCosmetics when the resolved id still
matches the current openId (or use an AbortController per loadDetails to cancel
stale requests), keeping function names loadDetails, toggleOpen, setTriggers,
setLinkedCosmeticIds, setAllCosmetics and openId as the reference points.

[AndresL230]

Closes#69.

Summary

• Pagination + server-side search on /api/admin/users; new last_sign_in_at column written on every Google callback.
• New endpoints: unapprove user; achievement triggers list/update/delete; achievement_cosmetics + role_cosmetics link/unlink/list; admin_audit_log read; analytics overview (totals + 30-day series + role counts); allowlist approve/revoke.
• Self-protection guards: cannot revoke own admin, cannot revoke last admin, cannot delete admin role, cannot unapprove self.
• Audit log written for every admin mutation via a single sanctioned helper (services/admin_audit.py).
• Frontend Admin.tsx gains Allowlist + Audit tabs and rewires Users / Achievements / Cosmetics / Analytics to the new endpoints — all using existing card / chip / btn / label-micro / var(--accent) design tokens (no new design language).
• assign_role is now idempotent (upsert on (user_id, role_id)); granted_by auto-fills from session.

Schema

• New table admin_audit_log (actor_id, action, target_type, target_id, payload jsonb, created_at) with FK actor_id ON DELETE RESTRICT and read-path indexes on created_at / actor / target.
• New column users.last_sign_in_at and index on users.created_at.

Migration file: backend/db/migration_admin_portal.sql — run once in the Supabase SQL editor.

Test plan

• Run the SQL migration in the dev Supabase project.
• cd backend && python -m pytest tests/test_admin_routes.py tests/test_admin_audit.py tests/test_users_search.py tests/test_supabase.py tests/test_auth_state.py -q (81 tests, all green on this branch).
• cd frontend && npx tsc --noEmit (clean).
• Manual smoke: visit /admin as an admin and walk every tab.
  • Users: paginate, debounced search, approve/unapprove, role assign/revoke.
  • Allowlist: add an email, revoke, re-approve.
  • Roles: try deleting the admin role (must 409); try revoking own admin (409); reassigning an existing role must NOT 500.
  • Achievements: Manage on a row, add/edit/delete a trigger, link/unlink a cosmetic chip.
  • Cosmetics: Manage on a row, link/unlink a role chip.
  • Analytics: totals match DB, sparkline tooltips show daily counts.
  • Audit: every action above appears as a row, filters by action/target_type work.

Out of scope

• Impersonate user (mentioned in feat(admin): admin portal UI — users, allowlist, roles, achievements, cosmetics #69 — touches session minting; deferred).
• EmailStr validation on AllowlistEmailBody — currently email: str. Worth tightening in a follow-up.

Plan

Implementation followed docs/superpowers/plans/2026-05-04-admin-portal.md (committed in this branch).

Summary by CodeRabbit

• New Features
  • Admin dashboard expanded with allowlist management and audit log sections
  • User management improved with pagination, search, and last sign-in date tracking
  • Achievement trigger and cosmetic linking management tools added
  • Admin audit logging tracks all administrative actions system-wide
  • Analytics dashboard displays signup trends and role distribution metrics

[coderabbitai]

📝 Walkthrough

Walkthrough

This PR implements a comprehensive admin portal spanning backend and frontend. It adds database schema for audit logging, a new select_with_count() pagination helper, admin audit tracking across mutations, paginated/searchable user listing, achievement trigger and cosmetic-linking management, role protection rules, allowlist administration, analytics computation, and a fully featured admin UI with tabs for users (paginated/searchable), allowlist, roles, achievements, cosmetics, and audit logs.

Changes

Admin Portal – Complete Implementation

Layer / File(s)	Summary
Database Schema & Indexing backend/db/migration_admin_portal.sql	Creates admin_audit_log table with actor/action/target metadata and timestamps; adds users.last_sign_in_at column; indexes created for time-based and actor-based audit queries.
Database Query Helpers backend/db/connection.py	Adds SupabaseTable.select_with_count() synchronous method that returns (rows, total) by parsing Content-Range header; supports offset alongside limit.
Request Body Models backend/models/__init__.py	Introduces UpdateAchievementTriggerBody, LinkAchievementCosmeticBody, LinkRoleCosmeticBody, and AllowlistEmailBody for admin endpoints.
Audit Service backend/services/admin_audit.py	Adds log_admin_action() to insert audit rows into admin_audit_log with error suppression; ensures audit failures don't block originating operations.
User Pagination & Search backend/services/users_search.py	Adds paginate_users() implementing DB-level pagination when no query provided and decrypt-all-then-filter approach for name/email search; attaches per-user roles and returns {users, total, page, page_size}.
Admin Routes backend/routes/admin.py	Adds/extends 20+ endpoints: role CRUD with admin-role self-protection, achievement management with trigger CRUD and cosmetic linking, cosmetic management, user listing (paginated), approval/unapproval with self-guard, role/achievement-cosmetic linking, allowlist administration, audit log querying, and analytics overview computation. All mutations emit audit events via log_admin_action().
Auth Integration backend/routes/auth.py	Tracks last_sign_in_at on Google OAuth callback (both existing and new users); adds test helper _stamp_last_sign_in_for_test() to support test scenarios without full OAuth flow.
Backend Tests backend/tests/test_admin_audit.py, test_admin_routes.py, test_auth_state.py, test_supabase.py, test_users_search.py	Comprehensive unit and integration test coverage: audit logging (row insertion and error suppression), user pagination with/without search and role attachment, role protection rules, achievement/cosmetic/allowlist/audit operations, analytics computation, and select_with_count() parsing.
Frontend Types frontend/src/lib/types.ts	Adds AllowlistEmail, AchievementTrigger, AdminAuditEntry, analytics-related types (AnalyticsTotals, AnalyticsDayPoint, AnalyticsRoleCount, AnalyticsOverview), AdminUserListItem, and PaginatedUsers; extends Cosmetic with optional unlock_source field.
Frontend API Wrappers frontend/src/lib/api.ts	Adds ~17 new admin helpers for paginated user listing (with search), user unapproval, role-cosmetic linking, achievement trigger CRUD, achievement-cosmetic linking, allowlist management, audit log querying with filters, and analytics overview retrieval.
Frontend Admin UI frontend/src/components/screens/Admin.tsx	Expands Admin component to include Users tab (paginated with debounced search, approve/unapprove, last-seen display), Allowlist tab (email list with approve/revoke), Audit tab (paginated audit log with action/target filters and payload display), and extends Roles, Achievements, Cosmetics tabs with row-expansion UIs for managing linked cosmetics/roles and achievement triggers. Refactors Analytics to use adminAnalyticsOverview() with metric cards and sparklines. Upgrades CatalogRow component to support expandable management sections.

Sequence Diagram

sequenceDiagram
participant Admin as Admin User
participant FrontendUI as Frontend UI
participant API as Admin API
participant Auth as Auth Service
participant DB as Database
rect rgba(100, 200, 150, 0.5)
note over Admin,DB: User Search & Pagination Flow
Admin->>FrontendUI: Enter search query, navigate to page 2
FrontendUI->>FrontendUI: Debounce search input
FrontendUI->>API: adminFetchUsers({q: "alice", page: 2, page_size: 50})
API->>Auth: get_session_user_id() → verify admin
API->>DB: paginate_users(q="alice", page=2, page_size=50)
alt Query provided (search)
DB->>DB: Decrypt all users, filter by substring match
DB->>DB: Compute total from filtered set, slice page
else No query (list)
DB->>DB: select_with_count (offset=50, limit=50)
end
DB->>DB: Attach roles per user from user_roles
DB-->>API: {users: [...], total: 137, page: 2, page_size: 50}
API-->>FrontendUI: PaginatedUsers response
FrontendUI->>FrontendUI: Render users table with Last Seen, Joined dates
FrontendUI-->>Admin: Display page 2 of 3, approve/unapprove buttons
end
rect rgba(150, 180, 220, 0.5)
note over Admin,DB: Admin Audit Logging Flow
Admin->>FrontendUI: Click "Approve" button for user
FrontendUI->>API: adminApproveUser(userId)
API->>Auth: get_session_user_id() → actor_id
API->>DB: UPDATE users SET is_approved=true WHERE id=userId
API->>DB: log_admin_action(actor_id, "user.approve", "user", userId, {...})
DB->>DB: INSERT INTO admin_audit_log (actor_id, action, target_type, target_id, created_at)
DB-->>API: Audit row inserted (or error caught & logged)
API-->>FrontendUI: {approved: true}
FrontendUI-->>Admin: Show success, refresh user list
Admin->>FrontendUI: Visit Audit tab
FrontendUI->>API: adminAuditLog({page: 1, action: "user.approve"})
DB->>DB: SELECT * FROM admin_audit_log WHERE action='user.approve' ORDER BY created_at DESC
DB-->>API: Paginated audit rows
API-->>FrontendUI: Audit entries with timestamps, payload JSON
FrontendUI-->>Admin: Display filtered audit log
end
rect rgba(200, 150, 180, 0.5)
note over Admin,DB: Role-Cosmetic Linking Flow
Admin->>FrontendUI: Expand role row, toggle linked cosmetic
FrontendUI->>API: adminLinkRoleCosmetic(roleId, cosmeticId)
API->>Auth: get_session_user_id() → actor_id
API->>DB: UPSERT INTO role_cosmetics (role_id, cosmetic_id)
API->>DB: log_admin_action(actor_id, "role_cosmetic.link", "role_cosmetic", null, {...})
DB->>DB: INSERT INTO admin_audit_log
DB-->>API: Link upserted, audit logged
API-->>FrontendUI: {linked: true}
FrontendUI->>FrontendUI: Update expanded row UI to reflect new link
FrontendUI-->>Admin: Show cosmetic chip as linked in role details
end

Loading

Estimated Code Review Effort

🎯 4 (Complex) | ⏱️ ~45 minutes

Possibly Related PRs

• SaplingLearn/Sapling#56: Both PRs modify backend/routes/auth.py and the users table schema; PR #56 adds user field persistence during OAuth callback while this PR extends that to include last_sign_in_at tracking.

Poem

🐰 A warren of admins, now armed and aware,
With audit trails woven through each admin's care,
Cosmetics and triggers link up with grace,
While pages of users scroll at a measured pace,
The portal is gleaming—no more fumbling in the dark!

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 9.26% which is insufficient. The required threshold is 80.00%.	Write docstrings for the functions missing them to satisfy the coverage threshold.

✅ Passed checks (4 passed)

Check name	Status	Explanation
Title check	✅ Passed	The title accurately reflects the main changes: pagination, audit log, allowlist, and analytics for the admin portal. It is specific, concise, and directly related to the primary changeset.
Description check	✅ Passed	The description is comprehensive and follows the template structure with Summary, Changes Made (implicit via detailed sections), Testing checklist, and Out of Scope items. All key implementation details are documented.
Linked Issues check	✅ Passed	The PR fully addresses issue #69 objectives: pagination and search for users [#69], unapprove endpoint [#69], audit logging for mutations [#69], role protection guards [#69], achievement triggers management [#69], role/achievement cosmetic linking [#69], allowlist endpoints [#69], analytics overview [#69], and role assignment idempotency [#69].
Out of Scope Changes check	✅ Passed	All code changes are directly aligned with the stated objectives. Out-of-scope items (impersonation, stricter email validation) are explicitly deferred and not included in the changeset.

_{✏️ Tip: You can configure your own custom pre-merge checks in the settings.}

✨ Finishing Touches

📝 Generate docstrings

• Create stacked PR
• Commit on current branch

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Commit unit tests in branch feat/admin-portal

---

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

[cloudflare-workers-and-pages]

Deploying with Cloudflare Workers

The latest updates on your project. Learn more about integrating Git with Workers.

Status	Name	Latest Commit	Preview URL	Updated (UTC)
✅ Deployment successful! View logs	frontend	1c23a86	Commit Preview URL Branch Preview URL	May 04 2026, 11:23 PM

[coderabbitai]

Actionable comments posted: 4

🧹 Nitpick comments (2)

backend/services/users_search.py (1)

20-26: ⚡ Quick win

N+1 queries in _attach_roles — collapse to a single batch request

Each call to _attach_roles issues one Supabase REST request per user. With _MAX_PAGE_SIZE = 200 that's up to 200 sequential HTTP round-trips per admin page load. PostgREST's in.(...) filter fetches all roles in one request:

♻️ Proposed refactor

 def _attach_roles(users: list[dict]) -> None:
- for user in users:- rows = table("user_roles").select(- "roles(id,name,slug,color,icon,description,is_staff_assigned,is_earnable,display_priority)",- filters={"user_id": f"eq.{user['id']}"},- )- user["roles"] = [r["roles"] for r in (rows or []) if r.get("roles")]+ if not users:+ return+ ids = ",".join(u["id"] for u in users)+ all_rows = table("user_roles").select(+ "user_id,roles(id,name,slug,color,icon,description,is_staff_assigned,is_earnable,display_priority)",+ filters={"user_id": f"in.({ids})"},+ ) or []+ role_map: dict[str, list] = {}+ for r in all_rows:+ if r.get("roles"):+ role_map.setdefault(r["user_id"], []).append(r["roles"])+ for user in users:+ user["roles"] = role_map.get(user["id"], [])

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@backend/services/users_search.py` around lines 20 - 26, The _attach_roles
function issues an individual Supabase/PostgREST request per user causing N+1
queries; change it to fetch all roles in one batch by querying
table("user_roles") with a single select using the user_id
in.(<list_of_user_ids>) filter, retrieving "user_id" and the nested "roles(...)"
fields, then group the returned rows by user_id and assign user["roles"] =
[r["roles"] for r in grouped[user_id]] (or [] if none) for each user; keep
function name _attach_roles, use the same table("user_roles") select call but
replace per-user filters with the in filter and ensure you handle empty input
lists and absent role entries.

backend/routes/auth.py (1)

44-51: ⚡ Quick win

Move _stamp_last_sign_in_for_test out of the production module and hoist the datetime import

Two related issues:

1. _stamp_last_sign_in_for_test is a test-only helper sitting in the production auth.py module. Any package import of routes.auth exposes it, and it exists only because datetime.now is buried inside conditional branches where standard monkeypatch can't reach it.
2. from datetime import datetime as _dt, timezone as _tz is duplicated in three places (lines 47, 325, 343). Python caches module imports, so this isn't a correctness bug, but it's unnecessarily noisy.

The cleanest resolution is to promote the import to module level, which then makes routes.auth.datetime directly patchable in tests and removes the need for the test seam entirely:

♻️ Proposed refactor

+from datetime import datetime as _dt, timezone as _tz
import json
...
-def _stamp_last_sign_in_for_test(user_id: str) -> None:- """Test seam: write last_sign_in_at ..."""- from datetime import datetime, timezone- table("users").update(- {"last_sign_in_at": datetime.now(timezone.utc).isoformat()},- filters={"id": f"eq.{user_id}"},- )--
...
if existing:
- from datetime import datetime as _dt, timezone as _tz
table("users").update({
...
"last_sign_in_at": _dt.now(_tz.utc).isoformat(),
}, ...)
else:
- from datetime import datetime as _dt, timezone as _tz
table("users").insert({
...
"last_sign_in_at": _dt.now(_tz.utc).isoformat(),
})

With _dt at module level, test_auth_state.py can replace the test helper with a direct monkeypatch:

# test_auth_state.pyimportroutes.authasauth_moduledeftest_callback_stamps_last_sign_in(monkeypatch):
fixed=_dt(2024, 1, 1, tzinfo=_tz.utc)
monkeypatch.setattr(auth_module, "_dt", lambda*a, **kw: fixed)
...

Also applies to: 325-325, 343-343

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@backend/routes/auth.py` around lines 44 - 51, Move the test-only helper
_stamp_last_sign_in_for_test out of the production routes.auth module into the
test suite and hoist the datetime import to module scope: add a module-level
import like "from datetime import datetime as _dt, timezone as _tz" in
routes.auth, replace any local in-function imports and uses of
datetime.now(timezone.utc) with _dt(..., tzinfo=_tz.utc) (or calls to
_dt.now(_tz.utc)), and remove the test helper from the production file so tests
can monkeypatch the module-level _dt/_tz instead; update any references to
_stamp_last_sign_in_for_test in tests to call the moved helper in the test
package.

🤖 Prompt for all review comments with AI agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@backend/routes/admin.py`:
- Around line 558-570: The approvals series is currently counted by signup date
(using created_at) rather than the approval timestamp; update the logic in the
loop that builds by_day_approvals/approvals_by_day to use an approval timestamp
(e.g., check for an approved_at or similar field on each user instead of
created_at) and parse that timestamp into a date (like you do with created_at)
before incrementing by_day_approvals[d.isoformat()]. If there is no approval
timestamp available, either add/compute one at approval time (e.g., set
approved_at when is_approved transitions true) or rename the metric from
“Approvals” to something like “Approved users by signup date” to reflect the
current derivation (refer to variables/users, created_at, is_approved,
by_day_approvals, approvals_by_day, window).
- Around line 95-109: The code currently allows a client to override the grantor
via body.granted_by; instead, always derive the grantor from the session: set
granted_by = get_session_user_id(request) (i.e., the existing actor variable)
and remove usage of body.granted_by when building the upsert payload and the
log_admin_action payload so both table("user_roles").upsert and log_admin_action
use the trusted actor value only.
In `@backend/services/admin_audit.py`:
- Line 31: The log currently calls log.exception("admin_audit_log write failed:
%s", row) which will serialize sensitive identifiers (row.actor_id,
row.target_id); change the logging to avoid including identifying fields by
creating a sanitized version of row that omits or redacts actor_id and target_id
and log that instead (or log only non-identifying metadata such as event type,
timestamp, and error context). Update the call site using log.exception to pass
the sanitized data or a minimal message and error context rather than the full
row so actor_id/target_id are never written to logs.
In `@frontend/src/components/screens/Admin.tsx`:
- Around line 492-511: loadDetails writes shared state (setTriggers,
setLinkedCosmeticIds, setAllCosmetics) unguarded so a stale async response for a
previously requested id can overwrite the currently open achievement; fix by
capturing/validating the expected open id before applying results (e.g., read
current openId from a ref or compare openId at the time of resolution) and only
call setTriggers/setLinkedCosmeticIds/setAllCosmetics when the resolved id still
matches the current openId (or use an AbortController per loadDetails to cancel
stale requests), keeping function names loadDetails, toggleOpen, setTriggers,
setLinkedCosmeticIds, setAllCosmetics and openId as the reference points.
---
Nitpick comments:
In `@backend/routes/auth.py`:
- Around line 44-51: Move the test-only helper _stamp_last_sign_in_for_test out
of the production routes.auth module into the test suite and hoist the datetime
import to module scope: add a module-level import like "from datetime import
datetime as _dt, timezone as _tz" in routes.auth, replace any local in-function
imports and uses of datetime.now(timezone.utc) with _dt(..., tzinfo=_tz.utc) (or
calls to _dt.now(_tz.utc)), and remove the test helper from the production file
so tests can monkeypatch the module-level _dt/_tz instead; update any references
to _stamp_last_sign_in_for_test in tests to call the moved helper in the test
package.
In `@backend/services/users_search.py`:
- Around line 20-26: The _attach_roles function issues an individual
Supabase/PostgREST request per user causing N+1 queries; change it to fetch all
roles in one batch by querying table("user_roles") with a single select using
the user_id in.(<list_of_user_ids>) filter, retrieving "user_id" and the nested
"roles(...)" fields, then group the returned rows by user_id and assign
user["roles"] = [r["roles"] for r in grouped[user_id]] (or [] if none) for each
user; keep function name _attach_roles, use the same table("user_roles") select
call but replace per-user filters with the in filter and ensure you handle empty
input lists and absent role entries.

🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

• Push a commit to this branch (recommended)
• Create a new PR with the fixes

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro Plus

Run ID: e976a667-d4b6-4fac-9347-f27bc7d43539

📥 Commits

Reviewing files that changed from the base of the PR and between db5ba48 and 1c23a86.

📒 Files selected for processing (16)

• backend/db/connection.py
• backend/db/migration_admin_portal.sql
• backend/models/__init__.py
• backend/routes/admin.py
• backend/routes/auth.py
• backend/services/admin_audit.py
• backend/services/users_search.py
• backend/tests/test_admin_audit.py
• backend/tests/test_admin_routes.py
• backend/tests/test_auth_state.py
• backend/tests/test_supabase.py
• backend/tests/test_users_search.py
• docs/superpowers/plans/2026-05-04-admin-portal.md
• frontend/src/components/screens/Admin.tsx
• frontend/src/lib/api.ts
• frontend/src/lib/types.ts

[coderabbitai]

⚠️ Potential issue | 🟠 Major | ⚡ Quick win

Do not trust client-supplied granted_by.

granted_by = body.granted_by or actor lets the caller forge who granted the role. That breaks the server-side provenance this PR is adding and makes both the row data and the audit trail unreliable. Stamp granted_by from get_session_user_id(request) unconditionally.

Suggested fix

 def assign_role(body: AssignRoleBody, request: Request):
require_admin(request)
actor = get_session_user_id(request)
- granted_by = body.granted_by or actor+ granted_by = actor
table("user_roles").upsert(
{
"user_id": body.user_id,
"role_id": body.role_id,
"granted_by": granted_by,

📝 Committable suggestion

‼️IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change

	actor=get_session_user_id(request)
	granted_by=body.granted_byoractor
	table("user_roles").upsert(
	{
	"user_id": body.user_id,
	"role_id": body.role_id,
	"granted_by": granted_by,
	"granted_at": datetime.now(timezone.utc).isoformat(),
	},
	on_conflict="user_id,role_id",
	)
	log_admin_action(
	actor_id=actor, action="role.assign", target_type="role", target_id=body.role_id,
	payload={"user_id": body.user_id, "granted_by": granted_by},
	)
	actor=get_session_user_id(request)
	granted_by=actor
	table("user_roles").upsert(
	{
	"user_id": body.user_id,
	"role_id": body.role_id,
	"granted_by": granted_by,
	"granted_at": datetime.now(timezone.utc).isoformat(),
	},
	on_conflict="user_id,role_id",
	)
	log_admin_action(
	actor_id=actor, action="role.assign", target_type="role", target_id=body.role_id,
	payload={"user_id": body.user_id, "granted_by": granted_by},
	)

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@backend/routes/admin.py` around lines 95 - 109, The code currently allows a
client to override the grantor via body.granted_by; instead, always derive the
grantor from the session: set granted_by = get_session_user_id(request) (i.e.,
the existing actor variable) and remove usage of body.granted_by when building
the upsert payload and the log_admin_action payload so both
table("user_roles").upsert and log_admin_action use the trusted actor value
only.

[coderabbitai]

⚠️ Potential issue | 🟠 Major | 🏗️ Heavy lift

The “approvals” series is grouped by signup date, not approval date.

approvals_by_day is incremented from created_at for users whose current is_approved is true. A user approved today will be counted on their original signup day, so the Analytics tab’s “Approvals · last 30 days” chart is wrong. This needs either a real approval timestamp or a renamed metric that matches the current derivation.

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@backend/routes/admin.py` around lines 558 - 570, The approvals series is
currently counted by signup date (using created_at) rather than the approval
timestamp; update the logic in the loop that builds
by_day_approvals/approvals_by_day to use an approval timestamp (e.g., check for
an approved_at or similar field on each user instead of created_at) and parse
that timestamp into a date (like you do with created_at) before incrementing
by_day_approvals[d.isoformat()]. If there is no approval timestamp available,
either add/compute one at approval time (e.g., set approved_at when is_approved
transitions true) or rename the metric from “Approvals” to something like
“Approved users by signup date” to reflect the current derivation (refer to
variables/users, created_at, is_approved, by_day_approvals, approvals_by_day,
window).

[coderabbitai]

⚠️ Potential issue | 🟠 Major | ⚡ Quick win

log.exception serializes user identifiers (actor_id, target_id) into application logs

row includes actor_id and target_id, which encode Google sub-claim–based user IDs — persistent identifiers that can be linked back to real persons. If application logs are forwarded to external aggregators this violates GDPR/CCPA guidance against logging user identifiers.

🔒 Proposed fix — log only non-identifying metadata

- log.exception("admin_audit_log write failed: %s", row)+ log.exception(+ "admin_audit_log write failed action=%s target_type=%s",+ row.get("action"),+ row.get("target_type"),+ )

As per coding guidelines, logging sensitive data such as user identifiers is classified as a compliance/privacy risk (GDPR/CCPA).

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@backend/services/admin_audit.py` at line 31, The log currently calls
log.exception("admin_audit_log write failed: %s", row) which will serialize
sensitive identifiers (row.actor_id, row.target_id); change the logging to avoid
including identifying fields by creating a sanitized version of row that omits
or redacts actor_id and target_id and log that instead (or log only
non-identifying metadata such as event type, timestamp, and error context).
Update the call site using log.exception to pass the sanitized data or a minimal
message and error context rather than the full row so actor_id/target_id are
never written to logs.

[coderabbitai]

⚠️ Potential issue | 🟠 Major | ⚡ Quick win

Guard achievement detail state against stale async responses.

loadDetails() writes shared triggers / linkedCosmeticIds state without verifying that the response still belongs to the currently expanded achievement. If an admin opens A and quickly opens B, A’s slower response can overwrite B’s panel, and the inline trigger actions can then target the wrong record IDs.

Suggested fix

+ const detailsRequestRef = React.useRef(0);+ const openIdRef = React.useRef<string | null>(null);+
const loadDetails = React.useCallback(async (id: string) => {
+ const requestId = ++detailsRequestRef.current;
try {
const [t, l, c] = await Promise.all([
adminListTriggers(id),
adminListAchievementCosmetics(id),
allCosmetics.length ? Promise.resolve({ cosmetics: allCosmetics }) : adminListCosmetics(),
]);
+ if (detailsRequestRef.current !== requestId || openIdRef.current !== id) return;
setTriggers(t.triggers || []);
setLinkedCosmeticIds((l.links || []).map(x => x.cosmetic_id));
if (!allCosmetics.length) setAllCosmetics(c.cosmetics || []);
} catch (err) {
toast.error(`Detail load failed: ${String(err)}`);
}
}, [allCosmetics, toast]);
const toggleOpen = (id: string) => {
- if (openId === id) { setOpenId(null); return; }+ if (openId === id) {+ openIdRef.current = null;+ setOpenId(null);+ return;+ }+ openIdRef.current = id;+ setTriggers([]);+ setLinkedCosmeticIds([]);
setOpenId(id);
loadDetails(id);
};

📝 Committable suggestion

‼️IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change

	constloadDetails=React.useCallback(async(id: string)=>{
	try{
	const[t,l,c]=awaitPromise.all([
	adminListTriggers(id),
	adminListAchievementCosmetics(id),
	allCosmetics.length ? Promise.resolve({cosmetics: allCosmetics}) : adminListCosmetics(),
	]);
	setTriggers(t.triggers||[]);
	setLinkedCosmeticIds((l.links||[]).map(x=>x.cosmetic_id));
	if(!allCosmetics.length)setAllCosmetics(c.cosmetics||[]);
	}catch(err){
	toast.error(`Detail load failed: ${String(err)}`);
	}
	},[allCosmetics,toast]);
	consttoggleOpen=(id: string)=>{
	if(openId===id){setOpenId(null);return;}
	setOpenId(id);
	loadDetails(id);
	};
	constdetailsRequestRef=React.useRef(0);
	constopenIdRef=React.useRef<string|null>(null);
	constloadDetails=React.useCallback(async(id: string)=>{
	constrequestId=++detailsRequestRef.current;
	try{
	const[t,l,c]=awaitPromise.all([
	adminListTriggers(id),
	adminListAchievementCosmetics(id),
	allCosmetics.length ? Promise.resolve({cosmetics: allCosmetics}) : adminListCosmetics(),
	]);
	if(detailsRequestRef.current!==requestId||openIdRef.current!==id)return;
	setTriggers(t.triggers||[]);
	setLinkedCosmeticIds((l.links||[]).map(x=>x.cosmetic_id));
	if(!allCosmetics.length)setAllCosmetics(c.cosmetics||[]);
	}catch(err){
	toast.error(`Detail load failed: ${String(err)}`);
	}
	},[allCosmetics,toast]);
	consttoggleOpen=(id: string)=>{
	if(openId===id){
	openIdRef.current=null;
	setOpenId(null);
	return;
	}
	openIdRef.current=id;
	setTriggers([]);
	setLinkedCosmeticIds([]);
	setOpenId(id);
	loadDetails(id);
	};

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@frontend/src/components/screens/Admin.tsx` around lines 492 - 511,
loadDetails writes shared state (setTriggers, setLinkedCosmeticIds,
setAllCosmetics) unguarded so a stale async response for a previously requested
id can overwrite the currently open achievement; fix by capturing/validating the
expected open id before applying results (e.g., read current openId from a ref
or compare openId at the time of resolution) and only call
setTriggers/setLinkedCosmeticIds/setAllCosmetics when the resolved id still
matches the current openId (or use an AbortController per loadDetails to cancel
stale requests), keeping function names loadDetails, toggleOpen, setTriggers,
setLinkedCosmeticIds, setAllCosmetics and openId as the reference points.

[AndresL230]

Closes#69.

Summary

• Pagination + server-side search on /api/admin/users; new last_sign_in_at column written on every Google callback.
• New endpoints: unapprove user; achievement triggers list/update/delete; achievement_cosmetics + role_cosmetics link/unlink/list; admin_audit_log read; analytics overview (totals + 30-day series + role counts); allowlist approve/revoke.
• Self-protection guards: cannot revoke own admin, cannot revoke last admin, cannot delete admin role, cannot unapprove self.
• Audit log written for every admin mutation via a single sanctioned helper (services/admin_audit.py).
• Frontend Admin.tsx gains Allowlist + Audit tabs and rewires Users / Achievements / Cosmetics / Analytics to the new endpoints — all using existing card / chip / btn / label-micro / var(--accent) design tokens (no new design language).
• assign_role is now idempotent (upsert on (user_id, role_id)); granted_by auto-fills from session.

Schema

• New table admin_audit_log (actor_id, action, target_type, target_id, payload jsonb, created_at) with FK actor_id ON DELETE RESTRICT and read-path indexes on created_at / actor / target.
• New column users.last_sign_in_at and index on users.created_at.

Migration file: backend/db/migration_admin_portal.sql — run once in the Supabase SQL editor.

Test plan

• Run the SQL migration in the dev Supabase project.
• cd backend && python -m pytest tests/test_admin_routes.py tests/test_admin_audit.py tests/test_users_search.py tests/test_supabase.py tests/test_auth_state.py -q (81 tests, all green on this branch).
• cd frontend && npx tsc --noEmit (clean).
• Manual smoke: visit /admin as an admin and walk every tab.
  • Users: paginate, debounced search, approve/unapprove, role assign/revoke.
  • Allowlist: add an email, revoke, re-approve.
  • Roles: try deleting the admin role (must 409); try revoking own admin (409); reassigning an existing role must NOT 500.
  • Achievements: Manage on a row, add/edit/delete a trigger, link/unlink a cosmetic chip.
  • Cosmetics: Manage on a row, link/unlink a role chip.
  • Analytics: totals match DB, sparkline tooltips show daily counts.
  • Audit: every action above appears as a row, filters by action/target_type work.

Out of scope

• Impersonate user (mentioned in feat(admin): admin portal UI — users, allowlist, roles, achievements, cosmetics #69 — touches session minting; deferred).
• EmailStr validation on AllowlistEmailBody — currently email: str. Worth tightening in a follow-up.

Plan

Implementation followed docs/superpowers/plans/2026-05-04-admin-portal.md (committed in this branch).

Summary by CodeRabbit

• New Features
  • Admin dashboard expanded with allowlist management and audit log sections
  • User management improved with pagination, search, and last sign-in date tracking
  • Achievement trigger and cosmetic linking management tools added
  • Admin audit logging tracks all administrative actions system-wide
  • Analytics dashboard displays signup trends and role distribution metrics

[coderabbitai]

📝 Walkthrough

Walkthrough

This PR implements a comprehensive admin portal spanning backend and frontend. It adds database schema for audit logging, a new select_with_count() pagination helper, admin audit tracking across mutations, paginated/searchable user listing, achievement trigger and cosmetic-linking management, role protection rules, allowlist administration, analytics computation, and a fully featured admin UI with tabs for users (paginated/searchable), allowlist, roles, achievements, cosmetics, and audit logs.

Changes

Admin Portal – Complete Implementation

Layer / File(s)	Summary
Database Schema & Indexing backend/db/migration_admin_portal.sql	Creates admin_audit_log table with actor/action/target metadata and timestamps; adds users.last_sign_in_at column; indexes created for time-based and actor-based audit queries.
Database Query Helpers backend/db/connection.py	Adds SupabaseTable.select_with_count() synchronous method that returns (rows, total) by parsing Content-Range header; supports offset alongside limit.
Request Body Models backend/models/__init__.py	Introduces UpdateAchievementTriggerBody, LinkAchievementCosmeticBody, LinkRoleCosmeticBody, and AllowlistEmailBody for admin endpoints.
Audit Service backend/services/admin_audit.py	Adds log_admin_action() to insert audit rows into admin_audit_log with error suppression; ensures audit failures don't block originating operations.
User Pagination & Search backend/services/users_search.py	Adds paginate_users() implementing DB-level pagination when no query provided and decrypt-all-then-filter approach for name/email search; attaches per-user roles and returns {users, total, page, page_size}.
Admin Routes backend/routes/admin.py	Adds/extends 20+ endpoints: role CRUD with admin-role self-protection, achievement management with trigger CRUD and cosmetic linking, cosmetic management, user listing (paginated), approval/unapproval with self-guard, role/achievement-cosmetic linking, allowlist administration, audit log querying, and analytics overview computation. All mutations emit audit events via log_admin_action().
Auth Integration backend/routes/auth.py	Tracks last_sign_in_at on Google OAuth callback (both existing and new users); adds test helper _stamp_last_sign_in_for_test() to support test scenarios without full OAuth flow.
Backend Tests backend/tests/test_admin_audit.py, test_admin_routes.py, test_auth_state.py, test_supabase.py, test_users_search.py	Comprehensive unit and integration test coverage: audit logging (row insertion and error suppression), user pagination with/without search and role attachment, role protection rules, achievement/cosmetic/allowlist/audit operations, analytics computation, and select_with_count() parsing.
Frontend Types frontend/src/lib/types.ts	Adds AllowlistEmail, AchievementTrigger, AdminAuditEntry, analytics-related types (AnalyticsTotals, AnalyticsDayPoint, AnalyticsRoleCount, AnalyticsOverview), AdminUserListItem, and PaginatedUsers; extends Cosmetic with optional unlock_source field.
Frontend API Wrappers frontend/src/lib/api.ts	Adds ~17 new admin helpers for paginated user listing (with search), user unapproval, role-cosmetic linking, achievement trigger CRUD, achievement-cosmetic linking, allowlist management, audit log querying with filters, and analytics overview retrieval.
Frontend Admin UI frontend/src/components/screens/Admin.tsx	Expands Admin component to include Users tab (paginated with debounced search, approve/unapprove, last-seen display), Allowlist tab (email list with approve/revoke), Audit tab (paginated audit log with action/target filters and payload display), and extends Roles, Achievements, Cosmetics tabs with row-expansion UIs for managing linked cosmetics/roles and achievement triggers. Refactors Analytics to use adminAnalyticsOverview() with metric cards and sparklines. Upgrades CatalogRow component to support expandable management sections.

Sequence Diagram

sequenceDiagram
participant Admin as Admin User
participant FrontendUI as Frontend UI
participant API as Admin API
participant Auth as Auth Service
participant DB as Database
rect rgba(100, 200, 150, 0.5)
note over Admin,DB: User Search & Pagination Flow
Admin->>FrontendUI: Enter search query, navigate to page 2
FrontendUI->>FrontendUI: Debounce search input
FrontendUI->>API: adminFetchUsers({q: "alice", page: 2, page_size: 50})
API->>Auth: get_session_user_id() → verify admin
API->>DB: paginate_users(q="alice", page=2, page_size=50)
alt Query provided (search)
DB->>DB: Decrypt all users, filter by substring match
DB->>DB: Compute total from filtered set, slice page
else No query (list)
DB->>DB: select_with_count (offset=50, limit=50)
end
DB->>DB: Attach roles per user from user_roles
DB-->>API: {users: [...], total: 137, page: 2, page_size: 50}
API-->>FrontendUI: PaginatedUsers response
FrontendUI->>FrontendUI: Render users table with Last Seen, Joined dates
FrontendUI-->>Admin: Display page 2 of 3, approve/unapprove buttons
end
rect rgba(150, 180, 220, 0.5)
note over Admin,DB: Admin Audit Logging Flow
Admin->>FrontendUI: Click "Approve" button for user
FrontendUI->>API: adminApproveUser(userId)
API->>Auth: get_session_user_id() → actor_id
API->>DB: UPDATE users SET is_approved=true WHERE id=userId
API->>DB: log_admin_action(actor_id, "user.approve", "user", userId, {...})
DB->>DB: INSERT INTO admin_audit_log (actor_id, action, target_type, target_id, created_at)
DB-->>API: Audit row inserted (or error caught & logged)
API-->>FrontendUI: {approved: true}
FrontendUI-->>Admin: Show success, refresh user list
Admin->>FrontendUI: Visit Audit tab
FrontendUI->>API: adminAuditLog({page: 1, action: "user.approve"})
DB->>DB: SELECT * FROM admin_audit_log WHERE action='user.approve' ORDER BY created_at DESC
DB-->>API: Paginated audit rows
API-->>FrontendUI: Audit entries with timestamps, payload JSON
FrontendUI-->>Admin: Display filtered audit log
end
rect rgba(200, 150, 180, 0.5)
note over Admin,DB: Role-Cosmetic Linking Flow
Admin->>FrontendUI: Expand role row, toggle linked cosmetic
FrontendUI->>API: adminLinkRoleCosmetic(roleId, cosmeticId)
API->>Auth: get_session_user_id() → actor_id
API->>DB: UPSERT INTO role_cosmetics (role_id, cosmetic_id)
API->>DB: log_admin_action(actor_id, "role_cosmetic.link", "role_cosmetic", null, {...})
DB->>DB: INSERT INTO admin_audit_log
DB-->>API: Link upserted, audit logged
API-->>FrontendUI: {linked: true}
FrontendUI->>FrontendUI: Update expanded row UI to reflect new link
FrontendUI-->>Admin: Show cosmetic chip as linked in role details
end

Loading

Estimated Code Review Effort

🎯 4 (Complex) | ⏱️ ~45 minutes

Possibly Related PRs

• SaplingLearn/Sapling#56: Both PRs modify backend/routes/auth.py and the users table schema; PR #56 adds user field persistence during OAuth callback while this PR extends that to include last_sign_in_at tracking.

Poem

🐰 A warren of admins, now armed and aware,
With audit trails woven through each admin's care,
Cosmetics and triggers link up with grace,
While pages of users scroll at a measured pace,
The portal is gleaming—no more fumbling in the dark!

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 9.26% which is insufficient. The required threshold is 80.00%.	Write docstrings for the functions missing them to satisfy the coverage threshold.

✅ Passed checks (4 passed)

Check name	Status	Explanation
Title check	✅ Passed	The title accurately reflects the main changes: pagination, audit log, allowlist, and analytics for the admin portal. It is specific, concise, and directly related to the primary changeset.
Description check	✅ Passed	The description is comprehensive and follows the template structure with Summary, Changes Made (implicit via detailed sections), Testing checklist, and Out of Scope items. All key implementation details are documented.
Linked Issues check	✅ Passed	The PR fully addresses issue #69 objectives: pagination and search for users [#69], unapprove endpoint [#69], audit logging for mutations [#69], role protection guards [#69], achievement triggers management [#69], role/achievement cosmetic linking [#69], allowlist endpoints [#69], analytics overview [#69], and role assignment idempotency [#69].
Out of Scope Changes check	✅ Passed	All code changes are directly aligned with the stated objectives. Out-of-scope items (impersonation, stricter email validation) are explicitly deferred and not included in the changeset.

_{✏️ Tip: You can configure your own custom pre-merge checks in the settings.}

✨ Finishing Touches

📝 Generate docstrings

• Create stacked PR
• Commit on current branch

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Commit unit tests in branch feat/admin-portal

---

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

[cloudflare-workers-and-pages]

Deploying with Cloudflare Workers

The latest updates on your project. Learn more about integrating Git with Workers.

Status	Name	Latest Commit	Preview URL	Updated (UTC)
✅ Deployment successful! View logs	frontend	1c23a86	Commit Preview URL Branch Preview URL	May 04 2026, 11:23 PM

[coderabbitai]

Actionable comments posted: 4

🧹 Nitpick comments (2)

backend/services/users_search.py (1)

20-26: ⚡ Quick win

N+1 queries in _attach_roles — collapse to a single batch request

Each call to _attach_roles issues one Supabase REST request per user. With _MAX_PAGE_SIZE = 200 that's up to 200 sequential HTTP round-trips per admin page load. PostgREST's in.(...) filter fetches all roles in one request:

♻️ Proposed refactor

 def _attach_roles(users: list[dict]) -> None:
- for user in users:- rows = table("user_roles").select(- "roles(id,name,slug,color,icon,description,is_staff_assigned,is_earnable,display_priority)",- filters={"user_id": f"eq.{user['id']}"},- )- user["roles"] = [r["roles"] for r in (rows or []) if r.get("roles")]+ if not users:+ return+ ids = ",".join(u["id"] for u in users)+ all_rows = table("user_roles").select(+ "user_id,roles(id,name,slug,color,icon,description,is_staff_assigned,is_earnable,display_priority)",+ filters={"user_id": f"in.({ids})"},+ ) or []+ role_map: dict[str, list] = {}+ for r in all_rows:+ if r.get("roles"):+ role_map.setdefault(r["user_id"], []).append(r["roles"])+ for user in users:+ user["roles"] = role_map.get(user["id"], [])

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@backend/services/users_search.py` around lines 20 - 26, The _attach_roles
function issues an individual Supabase/PostgREST request per user causing N+1
queries; change it to fetch all roles in one batch by querying
table("user_roles") with a single select using the user_id
in.(<list_of_user_ids>) filter, retrieving "user_id" and the nested "roles(...)"
fields, then group the returned rows by user_id and assign user["roles"] =
[r["roles"] for r in grouped[user_id]] (or [] if none) for each user; keep
function name _attach_roles, use the same table("user_roles") select call but
replace per-user filters with the in filter and ensure you handle empty input
lists and absent role entries.

backend/routes/auth.py (1)

44-51: ⚡ Quick win

Move _stamp_last_sign_in_for_test out of the production module and hoist the datetime import

Two related issues:

1. _stamp_last_sign_in_for_test is a test-only helper sitting in the production auth.py module. Any package import of routes.auth exposes it, and it exists only because datetime.now is buried inside conditional branches where standard monkeypatch can't reach it.
2. from datetime import datetime as _dt, timezone as _tz is duplicated in three places (lines 47, 325, 343). Python caches module imports, so this isn't a correctness bug, but it's unnecessarily noisy.

The cleanest resolution is to promote the import to module level, which then makes routes.auth.datetime directly patchable in tests and removes the need for the test seam entirely:

♻️ Proposed refactor

+from datetime import datetime as _dt, timezone as _tz
import json
...
-def _stamp_last_sign_in_for_test(user_id: str) -> None:- """Test seam: write last_sign_in_at ..."""- from datetime import datetime, timezone- table("users").update(- {"last_sign_in_at": datetime.now(timezone.utc).isoformat()},- filters={"id": f"eq.{user_id}"},- )--
...
if existing:
- from datetime import datetime as _dt, timezone as _tz
table("users").update({
...
"last_sign_in_at": _dt.now(_tz.utc).isoformat(),
}, ...)
else:
- from datetime import datetime as _dt, timezone as _tz
table("users").insert({
...
"last_sign_in_at": _dt.now(_tz.utc).isoformat(),
})

With _dt at module level, test_auth_state.py can replace the test helper with a direct monkeypatch:

# test_auth_state.pyimportroutes.authasauth_moduledeftest_callback_stamps_last_sign_in(monkeypatch):
fixed=_dt(2024, 1, 1, tzinfo=_tz.utc)
monkeypatch.setattr(auth_module, "_dt", lambda*a, **kw: fixed)
...

Also applies to: 325-325, 343-343

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@backend/routes/auth.py` around lines 44 - 51, Move the test-only helper
_stamp_last_sign_in_for_test out of the production routes.auth module into the
test suite and hoist the datetime import to module scope: add a module-level
import like "from datetime import datetime as _dt, timezone as _tz" in
routes.auth, replace any local in-function imports and uses of
datetime.now(timezone.utc) with _dt(..., tzinfo=_tz.utc) (or calls to
_dt.now(_tz.utc)), and remove the test helper from the production file so tests
can monkeypatch the module-level _dt/_tz instead; update any references to
_stamp_last_sign_in_for_test in tests to call the moved helper in the test
package.

🤖 Prompt for all review comments with AI agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@backend/routes/admin.py`:
- Around line 558-570: The approvals series is currently counted by signup date
(using created_at) rather than the approval timestamp; update the logic in the
loop that builds by_day_approvals/approvals_by_day to use an approval timestamp
(e.g., check for an approved_at or similar field on each user instead of
created_at) and parse that timestamp into a date (like you do with created_at)
before incrementing by_day_approvals[d.isoformat()]. If there is no approval
timestamp available, either add/compute one at approval time (e.g., set
approved_at when is_approved transitions true) or rename the metric from
“Approvals” to something like “Approved users by signup date” to reflect the
current derivation (refer to variables/users, created_at, is_approved,
by_day_approvals, approvals_by_day, window).
- Around line 95-109: The code currently allows a client to override the grantor
via body.granted_by; instead, always derive the grantor from the session: set
granted_by = get_session_user_id(request) (i.e., the existing actor variable)
and remove usage of body.granted_by when building the upsert payload and the
log_admin_action payload so both table("user_roles").upsert and log_admin_action
use the trusted actor value only.
In `@backend/services/admin_audit.py`:
- Line 31: The log currently calls log.exception("admin_audit_log write failed:
%s", row) which will serialize sensitive identifiers (row.actor_id,
row.target_id); change the logging to avoid including identifying fields by
creating a sanitized version of row that omits or redacts actor_id and target_id
and log that instead (or log only non-identifying metadata such as event type,
timestamp, and error context). Update the call site using log.exception to pass
the sanitized data or a minimal message and error context rather than the full
row so actor_id/target_id are never written to logs.
In `@frontend/src/components/screens/Admin.tsx`:
- Around line 492-511: loadDetails writes shared state (setTriggers,
setLinkedCosmeticIds, setAllCosmetics) unguarded so a stale async response for a
previously requested id can overwrite the currently open achievement; fix by
capturing/validating the expected open id before applying results (e.g., read
current openId from a ref or compare openId at the time of resolution) and only
call setTriggers/setLinkedCosmeticIds/setAllCosmetics when the resolved id still
matches the current openId (or use an AbortController per loadDetails to cancel
stale requests), keeping function names loadDetails, toggleOpen, setTriggers,
setLinkedCosmeticIds, setAllCosmetics and openId as the reference points.
---
Nitpick comments:
In `@backend/routes/auth.py`:
- Around line 44-51: Move the test-only helper _stamp_last_sign_in_for_test out
of the production routes.auth module into the test suite and hoist the datetime
import to module scope: add a module-level import like "from datetime import
datetime as _dt, timezone as _tz" in routes.auth, replace any local in-function
imports and uses of datetime.now(timezone.utc) with _dt(..., tzinfo=_tz.utc) (or
calls to _dt.now(_tz.utc)), and remove the test helper from the production file
so tests can monkeypatch the module-level _dt/_tz instead; update any references
to _stamp_last_sign_in_for_test in tests to call the moved helper in the test
package.
In `@backend/services/users_search.py`:
- Around line 20-26: The _attach_roles function issues an individual
Supabase/PostgREST request per user causing N+1 queries; change it to fetch all
roles in one batch by querying table("user_roles") with a single select using
the user_id in.(<list_of_user_ids>) filter, retrieving "user_id" and the nested
"roles(...)" fields, then group the returned rows by user_id and assign
user["roles"] = [r["roles"] for r in grouped[user_id]] (or [] if none) for each
user; keep function name _attach_roles, use the same table("user_roles") select
call but replace per-user filters with the in filter and ensure you handle empty
input lists and absent role entries.

🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

• Push a commit to this branch (recommended)
• Create a new PR with the fixes

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro Plus

Run ID: e976a667-d4b6-4fac-9347-f27bc7d43539

📥 Commits

Reviewing files that changed from the base of the PR and between db5ba48 and 1c23a86.

📒 Files selected for processing (16)

• backend/db/connection.py
• backend/db/migration_admin_portal.sql
• backend/models/__init__.py
• backend/routes/admin.py
• backend/routes/auth.py
• backend/services/admin_audit.py
• backend/services/users_search.py
• backend/tests/test_admin_audit.py
• backend/tests/test_admin_routes.py
• backend/tests/test_auth_state.py
• backend/tests/test_supabase.py
• backend/tests/test_users_search.py
• docs/superpowers/plans/2026-05-04-admin-portal.md
• frontend/src/components/screens/Admin.tsx
• frontend/src/lib/api.ts
• frontend/src/lib/types.ts

[coderabbitai]

⚠️ Potential issue | 🟠 Major | ⚡ Quick win

Do not trust client-supplied granted_by.

granted_by = body.granted_by or actor lets the caller forge who granted the role. That breaks the server-side provenance this PR is adding and makes both the row data and the audit trail unreliable. Stamp granted_by from get_session_user_id(request) unconditionally.

Suggested fix

 def assign_role(body: AssignRoleBody, request: Request):
require_admin(request)
actor = get_session_user_id(request)
- granted_by = body.granted_by or actor+ granted_by = actor
table("user_roles").upsert(
{
"user_id": body.user_id,
"role_id": body.role_id,
"granted_by": granted_by,

📝 Committable suggestion

‼️IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change

	actor=get_session_user_id(request)
	granted_by=body.granted_byoractor
	table("user_roles").upsert(
	{
	"user_id": body.user_id,
	"role_id": body.role_id,
	"granted_by": granted_by,
	"granted_at": datetime.now(timezone.utc).isoformat(),
	},
	on_conflict="user_id,role_id",
	)
	log_admin_action(
	actor_id=actor, action="role.assign", target_type="role", target_id=body.role_id,
	payload={"user_id": body.user_id, "granted_by": granted_by},
	)
	actor=get_session_user_id(request)
	granted_by=actor
	table("user_roles").upsert(
	{
	"user_id": body.user_id,
	"role_id": body.role_id,
	"granted_by": granted_by,
	"granted_at": datetime.now(timezone.utc).isoformat(),
	},
	on_conflict="user_id,role_id",
	)
	log_admin_action(
	actor_id=actor, action="role.assign", target_type="role", target_id=body.role_id,
	payload={"user_id": body.user_id, "granted_by": granted_by},
	)

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@backend/routes/admin.py` around lines 95 - 109, The code currently allows a
client to override the grantor via body.granted_by; instead, always derive the
grantor from the session: set granted_by = get_session_user_id(request) (i.e.,
the existing actor variable) and remove usage of body.granted_by when building
the upsert payload and the log_admin_action payload so both
table("user_roles").upsert and log_admin_action use the trusted actor value
only.

[coderabbitai]

⚠️ Potential issue | 🟠 Major | 🏗️ Heavy lift

The “approvals” series is grouped by signup date, not approval date.

approvals_by_day is incremented from created_at for users whose current is_approved is true. A user approved today will be counted on their original signup day, so the Analytics tab’s “Approvals · last 30 days” chart is wrong. This needs either a real approval timestamp or a renamed metric that matches the current derivation.

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@backend/routes/admin.py` around lines 558 - 570, The approvals series is
currently counted by signup date (using created_at) rather than the approval
timestamp; update the logic in the loop that builds
by_day_approvals/approvals_by_day to use an approval timestamp (e.g., check for
an approved_at or similar field on each user instead of created_at) and parse
that timestamp into a date (like you do with created_at) before incrementing
by_day_approvals[d.isoformat()]. If there is no approval timestamp available,
either add/compute one at approval time (e.g., set approved_at when is_approved
transitions true) or rename the metric from “Approvals” to something like
“Approved users by signup date” to reflect the current derivation (refer to
variables/users, created_at, is_approved, by_day_approvals, approvals_by_day,
window).

[coderabbitai]

⚠️ Potential issue | 🟠 Major | ⚡ Quick win

log.exception serializes user identifiers (actor_id, target_id) into application logs

row includes actor_id and target_id, which encode Google sub-claim–based user IDs — persistent identifiers that can be linked back to real persons. If application logs are forwarded to external aggregators this violates GDPR/CCPA guidance against logging user identifiers.

🔒 Proposed fix — log only non-identifying metadata

- log.exception("admin_audit_log write failed: %s", row)+ log.exception(+ "admin_audit_log write failed action=%s target_type=%s",+ row.get("action"),+ row.get("target_type"),+ )

As per coding guidelines, logging sensitive data such as user identifiers is classified as a compliance/privacy risk (GDPR/CCPA).

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@backend/services/admin_audit.py` at line 31, The log currently calls
log.exception("admin_audit_log write failed: %s", row) which will serialize
sensitive identifiers (row.actor_id, row.target_id); change the logging to avoid
including identifying fields by creating a sanitized version of row that omits
or redacts actor_id and target_id and log that instead (or log only
non-identifying metadata such as event type, timestamp, and error context).
Update the call site using log.exception to pass the sanitized data or a minimal
message and error context rather than the full row so actor_id/target_id are
never written to logs.

[coderabbitai]

⚠️ Potential issue | 🟠 Major | ⚡ Quick win

Guard achievement detail state against stale async responses.

loadDetails() writes shared triggers / linkedCosmeticIds state without verifying that the response still belongs to the currently expanded achievement. If an admin opens A and quickly opens B, A’s slower response can overwrite B’s panel, and the inline trigger actions can then target the wrong record IDs.

Suggested fix

+ const detailsRequestRef = React.useRef(0);+ const openIdRef = React.useRef<string | null>(null);+
const loadDetails = React.useCallback(async (id: string) => {
+ const requestId = ++detailsRequestRef.current;
try {
const [t, l, c] = await Promise.all([
adminListTriggers(id),
adminListAchievementCosmetics(id),
allCosmetics.length ? Promise.resolve({ cosmetics: allCosmetics }) : adminListCosmetics(),
]);
+ if (detailsRequestRef.current !== requestId || openIdRef.current !== id) return;
setTriggers(t.triggers || []);
setLinkedCosmeticIds((l.links || []).map(x => x.cosmetic_id));
if (!allCosmetics.length) setAllCosmetics(c.cosmetics || []);
} catch (err) {
toast.error(`Detail load failed: ${String(err)}`);
}
}, [allCosmetics, toast]);
const toggleOpen = (id: string) => {
- if (openId === id) { setOpenId(null); return; }+ if (openId === id) {+ openIdRef.current = null;+ setOpenId(null);+ return;+ }+ openIdRef.current = id;+ setTriggers([]);+ setLinkedCosmeticIds([]);
setOpenId(id);
loadDetails(id);
};

📝 Committable suggestion

‼️IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change

	constloadDetails=React.useCallback(async(id: string)=>{
	try{
	const[t,l,c]=awaitPromise.all([
	adminListTriggers(id),
	adminListAchievementCosmetics(id),
	allCosmetics.length ? Promise.resolve({cosmetics: allCosmetics}) : adminListCosmetics(),
	]);
	setTriggers(t.triggers||[]);
	setLinkedCosmeticIds((l.links||[]).map(x=>x.cosmetic_id));
	if(!allCosmetics.length)setAllCosmetics(c.cosmetics||[]);
	}catch(err){
	toast.error(`Detail load failed: ${String(err)}`);
	}
	},[allCosmetics,toast]);
	consttoggleOpen=(id: string)=>{
	if(openId===id){setOpenId(null);return;}
	setOpenId(id);
	loadDetails(id);
	};
	constdetailsRequestRef=React.useRef(0);
	constopenIdRef=React.useRef<string|null>(null);
	constloadDetails=React.useCallback(async(id: string)=>{
	constrequestId=++detailsRequestRef.current;
	try{
	const[t,l,c]=awaitPromise.all([
	adminListTriggers(id),
	adminListAchievementCosmetics(id),
	allCosmetics.length ? Promise.resolve({cosmetics: allCosmetics}) : adminListCosmetics(),
	]);
	if(detailsRequestRef.current!==requestId||openIdRef.current!==id)return;
	setTriggers(t.triggers||[]);
	setLinkedCosmeticIds((l.links||[]).map(x=>x.cosmetic_id));
	if(!allCosmetics.length)setAllCosmetics(c.cosmetics||[]);
	}catch(err){
	toast.error(`Detail load failed: ${String(err)}`);
	}
	},[allCosmetics,toast]);
	consttoggleOpen=(id: string)=>{
	if(openId===id){
	openIdRef.current=null;
	setOpenId(null);
	return;
	}
	openIdRef.current=id;
	setTriggers([]);
	setLinkedCosmeticIds([]);
	setOpenId(id);
	loadDetails(id);
	};

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@frontend/src/components/screens/Admin.tsx` around lines 492 - 511,
loadDetails writes shared state (setTriggers, setLinkedCosmeticIds,
setAllCosmetics) unguarded so a stale async response for a previously requested
id can overwrite the currently open achievement; fix by capturing/validating the
expected open id before applying results (e.g., read current openId from a ref
or compare openId at the time of resolution) and only call
setTriggers/setLinkedCosmeticIds/setAllCosmetics when the resolved id still
matches the current openId (or use an AbortController per loadDetails to cancel
stale requests), keeping function names loadDetails, toggleOpen, setTriggers,
setLinkedCosmeticIds, setAllCosmetics and openId as the reference points.