Conversation
There was a problem hiding this comment.
Pull request overview
This PR addresses multiple launch-blocker items by adding an in-app admin lead queue, fixing grouped “no-show” session filtering for analytics → sessions navigation, requiring tutor code-of-conduct acknowledgement at signup, and sanitizing PII/free-text from audit log details across server actions and RPCs.
Changes:
- Added
/admin/leadswith status filtering, WhatsApp handoff, admin notes, and dashboard/analytics counts. - Introduced a grouped
status=no_showfilter (mapping to bothno_show_student+no_show_tutor) and aligned sessions UI/filtering with analytics links. - Centralized audit-log
detailssanitization (plus an updated tutor RPC) and extracted tutor signup Zod validation + tests.
Reviewed changes
Copilot reviewed 27 out of 27 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
supabase/migrations/20260426000001_sanitize_session_audit_details.sql |
Replaces tutor_update_session audit logging to avoid storing tutor free-text notes in audit_logs.details. |
lib/validators/tutor-sign-up.ts |
Extracted tutor signup Zod schema including required conduct acknowledgement. |
lib/validators/lead-admin.ts |
Adds Zod schema for lead status/admin-notes updates. |
lib/validators/__tests__/tutor-sign-up.test.ts |
Adds regression tests for conduct acknowledgement requirement. |
lib/validators/__tests__/lead-admin.test.ts |
Adds validation tests for lead admin updates. |
lib/utils/session-filter.ts |
Implements grouped no_show filter mapping and filter option list. |
lib/utils/__tests__/session-filter.test.ts |
Tests grouped no-show filter behavior and options exposure. |
lib/services/sessions.ts |
Sanitizes audit details for session-related audit log inserts. |
lib/services/payments.ts |
Sanitizes audit details for package expiry audit logging. |
lib/audit/sanitize.ts |
Introduces sanitizeAuditDetails() redaction helper for audit detail payloads. |
lib/audit/__tests__/sanitize.test.ts |
Adds tests covering sensitive-key and sensitive-value redaction behavior. |
docs/plan-CorvEd.md |
Updates implementation plan checklist to reflect lead queue, grouped no-shows, and audit privacy hygiene. |
docs/GAP_ANALYSIS.md |
Updates route protection reference from middleware.ts to proxy.ts. |
app/dashboard/requests/actions.ts |
Sanitizes audit details for student request cancellation audit log entries. |
app/auth/sign-up/tutor/page.tsx |
Uses extracted schema and adds required code-of-conduct checkbox to tutor signup flow. |
app/admin/tutors/actions.ts |
Sanitizes audit details for tutor approval/revocation actions. |
app/admin/sessions/page.tsx |
Replaces legacy status-filter resolver with grouped-filter utility and updates label rendering/link propagation. |
app/admin/sessions/SessionFilters.tsx |
Uses shared SESSION_STATUS_FILTER_OPTIONS for status filter control. |
app/admin/requests/actions.ts |
Sanitizes audit details for multiple admin request/match actions and simplifies assignTutor return behavior. |
app/admin/payments/actions.ts |
Sanitizes audit details for payment audit events. |
app/admin/page.tsx |
Adds “Leads” card and new lead count to admin dashboard. |
app/admin/leads/page.tsx |
New admin lead queue page with filters, WhatsApp link, and inline status/notes updates. |
app/admin/leads/actions.ts |
New server action to update lead status/notes with validation and audit logging. |
app/admin/layout.tsx |
Adds Leads link to admin navigation. |
app/admin/analytics/page.tsx |
Adds “New Leads” metric card and query. |
app/admin/actions.ts |
Switches user-profile update audit logging to use sanitization (but introduces an audit actor attribution bug). |
README.md |
Updates docs to reflect proxy-based route protection, lead queue, conduct acknowledgement timing, and audit sanitization. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| await admin.from("audit_logs").insert([ | ||
| buildAdminUpdateUserProfileAuditEntry({ | ||
| actorUserId: adminUserId, | ||
| targetUserId: userId, | ||
| displayName: parsed.data.display_name, | ||
| }), | ||
| { | ||
| actor_user_id: userId, | ||
| action: "admin_update_user_profile", | ||
| entity_type: "user_profiles", | ||
| entity_id: userId, | ||
| details: sanitizeAuditDetails({ display_name: parsed.data.display_name }), | ||
| }, |
There was a problem hiding this comment.
In updateUserProfile, the audit log entry sets actor_user_id to the target userId rather than the admin performing the update. This breaks audit attribution (and conflicts with the existing buildAdminUpdateUserProfileAuditEntry behavior/test in lib/admin/users.ts). Capture the admin id returned by requireAdmin() and use it as actor_user_id (keeping entity_id as the edited profile’s user id).
| } | ||
|
|
||
| export function isSessionStatusFilter(value: string): value is SessionStatusFilter { | ||
| return value in SESSION_STATUS_FILTERS |
There was a problem hiding this comment.
isSessionStatusFilter uses the in operator against a plain object. Because in checks the prototype chain, values like __proto__ can return true and lead to unexpected runtime behavior when indexing SESSION_STATUS_FILTERS. Use an own-property check (e.g., Object.hasOwn(...) / hasOwnProperty.call) or make SESSION_STATUS_FILTERS an Object.create(null) map.
| return value in SESSION_STATUS_FILTERS | |
| return Object.prototype.hasOwnProperty.call(SESSION_STATUS_FILTERS, value) |
| <a | ||
| href={`/admin/sessions?student=${student.userId}&child=${encodeURIComponent(student.forStudentName ?? '')}`} | ||
| href={`/admin/sessions?student=${student.userId}&child=${encodeURIComponent(student.forStudentName ?? '')}${filterStatus ? `&view=past&status=${encodeURIComponent(filterStatus)}` : ''}`} | ||
| className="border-2 border-[#121212] px-3 py-1.5 text-xs font-bold uppercase tracking-widest text-[#121212] hover:bg-[#F0F0F0]" |
There was a problem hiding this comment.
The student-picker “View →” link forces view=past whenever a status filter is present. This makes filters like status=scheduled land on an empty past-sessions view, even when the student has upcoming scheduled sessions. Consider preserving the current view (or omitting view so the default applies) and only defaulting to past for filters that are inherently past-only (e.g. no-shows).
| await admin.from('audit_logs').insert([ | ||
| { | ||
| actor_user_id: adminUserId, | ||
| action: 'lead_status_updated', | ||
| entity_type: 'lead', | ||
| entity_id: leadId, | ||
| details: sanitizeAuditDetails({ | ||
| previous_status: previousLead.status, | ||
| status, | ||
| admin_notes, | ||
| }), | ||
| }, | ||
| ]) |
There was a problem hiding this comment.
updateLeadStatus inserts an audit log entry but does not check the returned error. Most other admin actions in this PR treat audit logging failures as actionable (they throw when auditError is present). Consider checking the insert result and surfacing failures consistently so audit gaps don’t go unnoticed.
|
@copilot apply changes based on the comments in this thread |
- Use adminUserId (from requireAdmin) as actor_user_id in updateUserProfile audit log - Use Object.prototype.hasOwnProperty.call in isSessionStatusFilter - Only force view=past for past-only status filters in sessions View link - Check and throw on audit log error in updateLeadStatus Agent-Logs-Url: https://github.com/Taleef7/CorvEd/sessions/ae5bca7b-7f33-498a-93a9-7185d1f632ca Co-authored-by: Taleef7 <89072337+Taleef7@users.noreply.github.com>
…n-filter.ts Agent-Logs-Url: https://github.com/Taleef7/CorvEd/sessions/ae5bca7b-7f33-498a-93a9-7185d1f632ca Co-authored-by: Taleef7 <89072337+Taleef7@users.noreply.github.com>
Applied all four review fixes in commits
|
Summary
status=no_showan intentional grouped filter for both student and tutor no-shows.audit_logs.details.proxy.tsroute protection.Closes #113.
Closes #114.
Closes #115.
Closes #144.
Notes
.github/copilot-instructions.md, but that file is intentionally ignored by.gitignoreand is not tracked in this repository. The tracked docs already referenceincrement_sessions_used(p_request_id), so I left Fix stale increment_sessions_used RPC example in contributor instructions #116 out of the closing list rather than force-adding an ignored local-instructions file.Verification
npm test-> 50 passed, 1 skippednpm run typecheck-> passednpm run lint-> exit 0 with 4 existing warnings in unrelated filesnpx supabase db reset-> applied all migrations including20260426000001_sanitize_session_audit_details.sqlnpm run build:local-> production build passed and included/admin/leads