fix: harden launch readiness and lifecycle coverage#112
Merged
Conversation
Contributor
There was a problem hiding this comment.
Pull request overview
This PR hardens several “launch readiness” lifecycle and auth flows (notably OAuth intent handling and session rescheduling rules), improves admin UX for tutor assignment and session filtering, and adds/extends automated coverage (unit + Playwright E2E) to validate the full request→payment→matching→sessions lifecycle against local Supabase.
Changes:
- Enforce and test a 24-hour minimum-notice rule for session rescheduling.
- Add OAuth callback intent plumbing (flow/account_type) and parent-signup promotion logic; centralize callback URL building.
- Refactor/admin-ize status/filter helpers and add durable seeded Playwright lifecycle coverage.
Reviewed changes
Copilot reviewed 25 out of 28 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| package-lock.json | Dependency lock updates (incl. brace-expansion patch bump). |
| next-env.d.ts | Updates generated route-types import path for Next typed routes. |
| lib/services/sessions.ts | Adds late reschedule enforcement by checking current scheduled start before updating. |
| lib/services/session-status-transitions.ts | Introduces isSessionRescheduleAllowed helper. |
| lib/services/tests/session-status-transitions.test.ts | Adds unit coverage for the 24h reschedule cutoff helper. |
| lib/auth/utils.ts | Adds buildAuthCallbackUrl + shouldPromoteOAuthParentSignup helpers for OAuth intent and parent promotion. |
| lib/admin/users.ts | Adds shared admin helpers for payment status prioritization/badges and audit entry building. |
| lib/admin/sessions.ts | Adds shared admin helpers for session status filter parsing/options (incl. grouped no-show). |
| lib/admin/tests/users.test.ts | Unit tests for new admin user helpers. |
| lib/admin/tests/sessions.test.ts | Unit tests for new admin session filter helpers. |
| lib/tests/auth-utils.test.ts | Unit tests for new auth helpers and promotion logic. |
| e2e/lifecycle.spec.ts | Adds seeded end-to-end lifecycle test (request→payment→match→generate sessions→tutor completion). |
| e2e/fixtures/payment-proof.pdf | Adds a fixture file used by lifecycle E2E payment upload. |
| e2e/auth.spec.ts | Extends auth-page smoke coverage (parent/tutor sign-up tabs + verify resend controls). |
| app/dashboard/page.tsx | Updates dashboard header copy to reflect Parent vs Student role. |
| app/dashboard/layout.tsx | Updates dashboard layout badge to reflect Parent vs Student role. |
| app/auth/verify/page.tsx | Adds resend verification controls to verify page. |
| app/auth/sign-up/page.tsx | Uses centralized callback URL builder for Google OAuth signup (flow/account type). |
| app/auth/sign-in/SignInForm.tsx | Uses centralized callback URL builder for Google OAuth sign-in redirects. |
| app/auth/callback/route.ts | Reads OAuth intent params and conditionally promotes fresh parent signups; uses request.nextUrl. |
| app/admin/users/page.tsx | Refactors payment status aggregation/badge rendering via shared helpers. |
| app/admin/sessions/SessionFilters.tsx | Uses shared helper to populate session status filter options. |
| app/admin/sessions/page.tsx | Supports grouped/expanded status filtering via shared resolver. |
| app/admin/requests/actions.ts | Redirects successful tutor assignment directly to match detail (assigned=1), with added revalidation. |
| app/admin/requests/[id]/AssignTutorForm.tsx | Improves form accessibility by linking labels to inputs via htmlFor/id. |
| app/admin/matches/[id]/page.tsx | Displays a success banner when arriving from assignment redirect (assigned=1). |
| app/admin/matches/[id]/MatchActions.tsx | Improves edit form accessibility by linking labels to inputs via htmlFor/id. |
| app/admin/actions.ts | Fixes admin profile-update audit logging by correctly attributing the actor (admin) via helper. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Comment on lines
29
to
105
| @@ -92,10 +95,13 @@ export async function assignTutor({ | |||
| revalidatePath(`/admin/requests/${requestId}`) | |||
| revalidatePath('/admin/requests') | |||
| revalidatePath('/admin/matches') | |||
| return { matchId: match.id } | |||
| revalidatePath(`/admin/matches/${match.id}`) | |||
| assignedMatchId = match.id | |||
| } catch (err) { | |||
| return { error: err instanceof Error ? err.message : 'An unexpected error occurred.' } | |||
| } | |||
|
|
|||
| redirect(`/admin/matches/${assignedMatchId}?assigned=1`) | |||
| } | |||
There was a problem hiding this comment.
assignTutor now always redirects on success, but the function signature still suggests it returns { matchId }, and assignedMatchId is typed as string | null when used in the redirect URL. To avoid confusion and accidental /admin/matches/null?... redirects, consider changing the return type to reflect the redirect (e.g. no matchId), and/or move the redirect() call inside the try after asserting match.id is present (or throw if it isn't).
Comment on lines
+67
to
+170
| const userId = data.user.id | ||
| await admin | ||
| .from('user_profiles') | ||
| .update({ | ||
| display_name: params.displayName, | ||
| whatsapp_number: params.whatsapp, | ||
| timezone: 'Asia/Karachi', | ||
| primary_role: params.role, | ||
| }) | ||
| .eq('user_id', userId) | ||
|
|
||
| await admin.from('user_roles').upsert({ user_id: userId, role: params.role }) | ||
|
|
||
| return { id: userId, email: params.email, password: params.password, displayName: params.displayName } | ||
| } | ||
|
|
||
| async function createSeededAdmin(params: { | ||
| email: string | ||
| password: string | ||
| displayName: string | ||
| whatsapp: string | ||
| }) { | ||
| const admin = createServiceClient() | ||
| const { data, error } = await admin.auth.admin.createUser({ | ||
| email: params.email, | ||
| password: params.password, | ||
| email_confirm: true, | ||
| user_metadata: { | ||
| name: params.displayName, | ||
| role: 'student', | ||
| timezone: 'Asia/Karachi', | ||
| }, | ||
| }) | ||
|
|
||
| if (error || !data.user) { | ||
| throw new Error(`Failed to create admin test user: ${error?.message ?? 'unknown error'}`) | ||
| } | ||
|
|
||
| const userId = data.user.id | ||
| await admin | ||
| .from('user_profiles') | ||
| .update({ | ||
| display_name: params.displayName, | ||
| whatsapp_number: params.whatsapp, | ||
| timezone: 'Asia/Karachi', | ||
| primary_role: 'admin', | ||
| }) | ||
| .eq('user_id', userId) | ||
|
|
||
| await admin.from('user_roles').upsert({ user_id: userId, role: 'admin' }) | ||
|
|
||
| return { id: userId, email: params.email, password: params.password, displayName: params.displayName } | ||
| } | ||
|
|
||
| async function ensureSubject() { | ||
| const admin = createServiceClient() | ||
| const { data, error } = await admin | ||
| .from('subjects') | ||
| .upsert( | ||
| { | ||
| code: 'math', | ||
| name: 'Mathematics', | ||
| active: true, | ||
| sort_order: 1, | ||
| }, | ||
| { onConflict: 'code' }, | ||
| ) | ||
| .select('id, name') | ||
| .single() | ||
|
|
||
| if (error || !data) { | ||
| throw new Error(`Failed to ensure Mathematics subject: ${error?.message ?? 'unknown error'}`) | ||
| } | ||
|
|
||
| return data | ||
| } | ||
|
|
||
| async function seedApprovedTutor(tutorUserId: string, subjectId: number) { | ||
| const admin = createServiceClient() | ||
|
|
||
| await admin.from('tutor_profiles').upsert({ | ||
| tutor_user_id: tutorUserId, | ||
| approved: true, | ||
| bio: 'Lifecycle test tutor', | ||
| timezone: 'Asia/Karachi', | ||
| experience_years: 4, | ||
| education: 'BS Mathematics', | ||
| teaching_approach: 'Exam-focused', | ||
| }) | ||
|
|
||
| await admin.from('tutor_subjects').upsert({ | ||
| tutor_user_id: tutorUserId, | ||
| subject_id: subjectId, | ||
| level: 'o_levels', | ||
| }) | ||
|
|
||
| await admin.from('tutor_availability').upsert({ | ||
| tutor_user_id: tutorUserId, | ||
| windows: [ | ||
| { day: 1, start: '16:00', end: '20:00' }, | ||
| { day: 3, start: '16:00', end: '20:00' }, | ||
| { day: 5, start: '16:00', end: '20:00' }, | ||
| ], | ||
| }) |
There was a problem hiding this comment.
The lifecycle seeding helpers perform several Supabase update/upsert calls without checking .error/.data (e.g. user_profiles update, user_roles upsert, tutor_profiles/subjects/availability upserts). If any of these fail, the test may continue with partially-seeded state and fail later in harder-to-debug ways. Suggest capturing and throwing on each mutation error so the failure points to the actual seeding step.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Verification
npm run typechecknpm run lintnpm testnpm run build:localnpm run test:e2e:local