Epic E5: Packages and Payments - #96
Conversation
- DB migration: packages, payments, audit_logs tables with RLS policies - lib/config/pricing.ts: PKR pricing constants for 8/12/20 session tiers - app/dashboard/packages/new: package selection page with tier cards - app/dashboard/packages/[id]: payment page with bank instructions + proof upload - components/dashboards/PackageSummary.tsx: session progress card - app/dashboard/page.tsx: embed PackageSummary per request - app/admin/payments/page.tsx: admin payments list with filter + actions - app/admin/payments/actions.ts: markPaymentPaid, markPaymentRejected with error handling and audit logging - Update admin layout with Payments nav link - Update README to reflect E5 functionality Closes #30 #31 #32 #33 #34 #35 #36" Co-authored-by: Taleef7 <89072337+Taleef7@users.noreply.github.com>
There was a problem hiding this comment.
Pull request overview
This PR implements a comprehensive packages and payments system for the CorvEd tutoring platform. Students can select from three package tiers (8, 12, or 20 sessions per month) with PKR pricing, submit payment via bank transfer with optional proof upload, and admins can verify payments which automatically activates packages and advances request status. The implementation includes full database schema with RLS policies, audit logging for admin actions, and integration throughout the student and admin dashboards.
Changes:
- Database schema for packages, payments, and audit_logs tables with appropriate constraints, indexes, RLS policies, and triggers
- Package selection flow allowing students to choose tiers and create payment records
- Payment submission page with bank transfer instructions, personalized reference format, and optional proof upload
- Admin payments dashboard with filtering, inline approval/rejection actions, and audit trail
- Package summary cards integrated into student dashboard showing tier, usage, and payment status
Reviewed changes
Copilot reviewed 11 out of 11 changed files in this pull request and generated 26 comments.
Show a summary per file
| File | Description |
|---|---|
| supabase/migrations/20260224000001_create_packages_payments.sql | Creates packages table with tier constraints (8/12/20), payments table with bank transfer method constraint, and audit_logs table; includes RLS policies and updated_at triggers |
| lib/config/pricing.ts | Defines package tiers with PKR pricing (8k/11k/16k) and payment instructions template with placeholder bank details |
| components/dashboards/PackageSummary.tsx | Displays package tier, date range, session usage with progress bar; handles pending/active/expired states |
| app/dashboard/requests/[id]/page.tsx | Updates "Select Package" link to include requestId query parameter |
| app/dashboard/page.tsx | Fetches packages for displayed requests and renders PackageSummary cards or "Select Package" CTA |
| app/dashboard/packages/new/page.tsx | Package selection UI with three tier cards; creates package, advances request to payment_pending, and creates initial payment record |
| app/dashboard/packages/[id]/page.tsx | Shows package details, bank transfer instructions with personalized reference, payment status, and optional proof upload form |
| app/admin/payments/page.tsx | Admin list view with status filtering (pending/paid/rejected/all), inline Mark Paid and Reject actions with optional note |
| app/admin/payments/actions.ts | Server actions for markPaymentPaid (atomically updates payment/package/request status), markPaymentRejected, and getPaymentProofUrl |
| app/admin/layout.tsx | Adds "Payments" navigation link to admin header |
| README.md | Updates feature checklist to reflect E5 completion and documents requirement to create payment-proofs storage bucket |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| <label className="mb-1 block text-sm font-medium text-zinc-700 dark:text-zinc-300"> | ||
| Payment proof screenshot (optional, max 5 MB) | ||
| </label> | ||
| <input | ||
| ref={fileRef} | ||
| type="file" | ||
| accept="image/jpeg,image/png,image/jpg,application/pdf" | ||
| onChange={(e) => setFile(e.target.files?.[0] ?? null)} | ||
| className="block w-full text-sm text-zinc-500 file:mr-3 file:rounded-lg file:border-0 file:bg-indigo-50 file:px-3 file:py-1.5 file:text-xs file:font-semibold file:text-indigo-700 hover:file:bg-indigo-100" | ||
| /> |
There was a problem hiding this comment.
The file input on line 329-335 doesn't have an accessible label association. While there's a label element above (line 326), it doesn't have a 'htmlFor' attribute linking to the input's id, which means screen readers won't properly announce the label when the input is focused. Add an id to the input and a matching htmlFor to the label.
There was a problem hiding this comment.
Fixed in 7160012 — added id="payment-proof" to the file input and htmlFor="payment-proof" to its label.
| await admin.from('audit_logs').insert([ | ||
| { | ||
| actor_user_id: adminUserId, | ||
| action: 'payment_marked_rejected', | ||
| entity_type: 'payment', | ||
| entity_id: paymentId, | ||
| details: { rejection_note: rejectionNote }, | ||
| }, | ||
| ]) |
There was a problem hiding this comment.
The audit log insert operation lacks error handling. If this operation fails, the function will continue silently without recording the admin action. This could lead to accountability gaps in the audit trail. Consider checking the error result and either logging it or throwing an exception to ensure audit integrity.
There was a problem hiding this comment.
Fixed in 7160012 — the audit log insert result is now checked and console.error is called if it fails, so accountability gaps are surfaced in server logs.
| const { data: newPkg, error: pkgError } = await supabase | ||
| .from('packages') | ||
| .insert([ | ||
| { | ||
| request_id: requestId, | ||
| tier_sessions: pkg.tier, | ||
| start_date: startDate, | ||
| end_date: endDate, | ||
| sessions_total: pkg.tier, | ||
| sessions_used: 0, | ||
| status: 'pending', | ||
| }, | ||
| ]) | ||
| .select() | ||
| .single() | ||
|
|
||
| if (pkgError || !newPkg) { | ||
| setError('Failed to create package. Please try again.') | ||
| setLoading(false) | ||
| return | ||
| } | ||
|
|
||
| // Advance request status to payment_pending | ||
| const { error: reqError } = await supabase | ||
| .from('requests') | ||
| .update({ status: 'payment_pending' }) | ||
| .eq('id', requestId) | ||
| .eq('status', 'new') | ||
|
|
||
| if (reqError) { | ||
| setError('Package created but failed to update request status. Please contact support.') | ||
| setLoading(false) | ||
| return | ||
| } | ||
|
|
||
| // Create initial payment row | ||
| const { data: payment, error: payError } = await supabase | ||
| .from('payments') | ||
| .insert([ | ||
| { | ||
| package_id: newPkg.id, | ||
| payer_user_id: user.id, | ||
| amount_pkr: pkg.pricePerMonthPkr, | ||
| method: 'bank_transfer', | ||
| status: 'pending', | ||
| }, | ||
| ]) | ||
| .select() | ||
| .single() | ||
|
|
||
| if (payError || !payment) { | ||
| setError('Package created but failed to create payment record. Please contact support.') | ||
| setLoading(false) | ||
| return | ||
| } |
There was a problem hiding this comment.
The package creation flow performs three separate database operations (insert package, update request, insert payment) without transaction guarantees. If the request update or payment insert fails after the package is created, the system will be in an inconsistent state. Consider implementing proper error recovery or using a database transaction to ensure atomicity.
| create policy "payments_update_payer_limited" | ||
| on public.payments for update to authenticated | ||
| using (payer_user_id = auth.uid() and status = 'pending') | ||
| with check (payer_user_id = auth.uid() and status = 'pending'); |
There was a problem hiding this comment.
The RLS policy payments_update_payer_limited allows users to update their own pending payments, but the check on line 111 verifies that the status remains 'pending' in both the USING and WITH CHECK clauses. However, this means users can only update payments that are currently pending AND must keep them pending. This prevents users from accidentally changing the status, but the current implementation in the payment detail page (app/dashboard/packages/[id]/page.tsx lines 161-167) updates reference and proof_path without explicitly including status in the update payload. While Supabase will use the existing status value, it's clearer to explicitly include status: 'pending' in the update to make the intent explicit and ensure the policy check passes predictably.
| const allowedTypes = ['image/jpeg', 'image/png', 'image/jpg', 'application/pdf'] | ||
| if (!allowedTypes.includes(file.type)) { | ||
| setError('Only JPEG, PNG, or PDF files are allowed.') | ||
| setSubmitting(false) | ||
| return | ||
| } |
There was a problem hiding this comment.
File type validation relies only on the MIME type from the client (file.type), which can be spoofed by malicious users. Consider adding server-side file type validation by checking file signatures (magic bytes) or using a library that validates file content, not just the extension or MIME type. Additionally, consider implementing virus scanning for uploaded files in production.
| if (pkgError) throw new Error(`Failed to activate package: ${pkgError.message}`) | ||
|
|
||
| const { error: reqError } = await admin | ||
| .from('requests') | ||
| .update({ status: 'ready_to_match' }) | ||
| .eq('id', requestId) | ||
|
|
||
| if (reqError) throw new Error(`Failed to advance request: ${reqError.message}`) | ||
|
|
There was a problem hiding this comment.
The markPaymentPaid function performs three separate database updates without transaction guarantees. If any update fails after the first succeeds, the system will be in an inconsistent state (e.g., payment marked as paid but package still pending). Consider using a database transaction or stored procedure to ensure atomicity, or implement compensating rollback logic.
| if (pkgError) throw new Error(`Failed to activate package: ${pkgError.message}`) | |
| const { error: reqError } = await admin | |
| .from('requests') | |
| .update({ status: 'ready_to_match' }) | |
| .eq('id', requestId) | |
| if (reqError) throw new Error(`Failed to advance request: ${reqError.message}`) | |
| if (pkgError) { | |
| // Best-effort rollback of payment update to avoid inconsistent state | |
| await admin | |
| .from('payments') | |
| .update({ | |
| status: 'pending', | |
| verified_by_user_id: null, | |
| verified_at: null, | |
| }) | |
| .eq('id', paymentId) | |
| throw new Error(`Failed to activate package: ${pkgError.message}`) | |
| } | |
| const { error: reqError } = await admin | |
| .from('requests') | |
| .update({ status: 'ready_to_match' }) | |
| .eq('id', requestId) | |
| if (reqError) { | |
| // Best-effort rollback of package and payment updates to avoid inconsistent state | |
| await admin | |
| .from('packages') | |
| .update({ status: 'pending' }) | |
| .eq('id', packageId) | |
| await admin | |
| .from('payments') | |
| .update({ | |
| status: 'pending', | |
| verified_by_user_id: null, | |
| verified_at: null, | |
| }) | |
| .eq('id', paymentId) | |
| throw new Error(`Failed to advance request: ${reqError.message}`) | |
| } |
There was a problem hiding this comment.
Fixed in 7160012 — markPaymentPaid now reverts earlier successful updates on subsequent failures, and all rollback operations log errors via console.error if the rollback itself fails.
| const { data: newPkg, error: pkgError } = await supabase | ||
| .from('packages') | ||
| .insert([ | ||
| { | ||
| request_id: requestId, | ||
| tier_sessions: pkg.tier, | ||
| start_date: startDate, | ||
| end_date: endDate, | ||
| sessions_total: pkg.tier, | ||
| sessions_used: 0, | ||
| status: 'pending', | ||
| }, | ||
| ]) | ||
| .select() | ||
| .single() |
There was a problem hiding this comment.
Missing validation to prevent duplicate active packages for the same request. A user could potentially create multiple packages for a single request by navigating to the package selection page multiple times. Consider adding a unique constraint on (request_id) where status = 'pending' or 'active', or implement client-side and server-side checks to prevent duplicates.
There was a problem hiding this comment.
Fixed in 7160012 — before creating a new package, the page now queries for any existing pending/active package for the same request via .maybeSingle() and redirects to it if found.
| const startDate = today.toISOString().split('T')[0] | ||
| const endDate = new Date(today.getFullYear(), today.getMonth() + 1, today.getDate()) | ||
| .toISOString() | ||
| .split('T')[0] |
There was a problem hiding this comment.
The date calculation uses new Date() which creates a date in the user's local timezone, but then converts it to ISO string and extracts the date part. This could lead to timezone-related bugs where users in different timezones get different start_date values for packages created at the same moment. Consider using UTC dates consistently, or use the user's stored timezone preference from their profile to ensure consistent behavior.
| const startDate = today.toISOString().split('T')[0] | |
| const endDate = new Date(today.getFullYear(), today.getMonth() + 1, today.getDate()) | |
| .toISOString() | |
| .split('T')[0] | |
| const startDate = today.toISOString().slice(0, 10) | |
| const endDate = new Date( | |
| Date.UTC(today.getUTCFullYear(), today.getUTCMonth() + 1, today.getUTCDate()), | |
| ) | |
| .toISOString() | |
| .slice(0, 10) |
There was a problem hiding this comment.
Fixed in 7160012 — dates now use Date.UTC(...) throughout, and today.toISOString().slice(0, 10) for the start date.
| <div className="h-2 w-full overflow-hidden rounded-full bg-zinc-100 dark:bg-zinc-700"> | ||
| <div | ||
| className="h-full rounded-full bg-indigo-500 transition-all" | ||
| style={{ width: `${pct}%` }} |
There was a problem hiding this comment.
The percentage calculation could theoretically exceed 100% if sessions_used > tier_sessions (due to manual adjustments or data errors). While capping at 100% in CSS might handle this, the progress bar width style at line 108 doesn't clamp the value. Consider using Math.min(100, pct) to ensure the percentage never exceeds 100%.
| style={{ width: `${pct}%` }} | |
| style={{ width: `${Math.min(100, pct)}%` }} |
There was a problem hiding this comment.
Fixed in 7160012 — pct is now clamped: Math.min(100, Math.round(...)), and sessionsRemaining uses Math.max(0, ...). The progress bar style.width will never exceed 100%.
| <form | ||
| action={async () => { | ||
| 'use server' | ||
| await markPaymentPaid(payment.id, pkg.id, req.id) | ||
| }} | ||
| > | ||
| <button | ||
| type="submit" | ||
| className="rounded-lg bg-green-600 px-3 py-1.5 text-xs font-semibold text-white transition hover:bg-green-700" | ||
| > | ||
| ✅ Mark Paid | ||
| </button> | ||
| </form> | ||
|
|
||
| <RejectForm paymentId={payment.id} /> | ||
| </div> | ||
| )} | ||
| </div> | ||
| ) | ||
| })} | ||
| </div> | ||
| )} | ||
| </div> | ||
| ) | ||
| } | ||
|
|
||
| // Inline client component for proof link (needs signed URL) | ||
| function ProofLinkButton({ proofPath }: { proofPath: string }) { | ||
| void proofPath | ||
| return ( | ||
| <span className="text-xs text-zinc-400 italic"> | ||
| Proof on file (view via Supabase Storage) | ||
| </span> | ||
| ) | ||
| } | ||
|
|
||
| // Inline reject form with note | ||
| function RejectForm({ paymentId }: { paymentId: string }) { | ||
| return ( | ||
| <form | ||
| action={async (formData: FormData) => { | ||
| 'use server' | ||
| const note = formData.get('note') as string | ||
| await markPaymentRejected(paymentId, note) | ||
| }} | ||
| className="flex items-center gap-2" | ||
| > | ||
| <input | ||
| type="text" | ||
| name="note" | ||
| placeholder="Rejection note (optional)" | ||
| className="rounded-lg border border-zinc-300 px-2 py-1 text-xs shadow-sm focus:border-red-400 focus:outline-none dark:border-zinc-600 dark:bg-zinc-800 dark:text-zinc-100" | ||
| /> | ||
| <button | ||
| type="submit" | ||
| className="rounded-lg bg-red-600 px-3 py-1.5 text-xs font-semibold text-white transition hover:bg-red-700" | ||
| > | ||
| ❌ Reject | ||
| </button> | ||
| </form> | ||
| ) |
There was a problem hiding this comment.
The server action forms (lines 168-180, 207-228) don't handle or display errors from the action functions. If markPaymentPaid or markPaymentRejected throws an error (e.g., due to database issues or authorization failure), the user won't receive any feedback. Consider wrapping the actions with error handling and using Next.js error boundaries or displaying error messages to the admin user.
There was a problem hiding this comment.
Fixed in 7160012 — introduced app/admin/payments/PaymentActions.tsx, a client component using useActionState that wraps markPaymentPaid and markPaymentRejected. Errors thrown by the actions are caught and displayed inline above each button. The old inline server action closures are removed.
|
@copilot apply changes based on the comments in this thread |
- Accessibility: add htmlFor/id to reference and file upload inputs
- Extract LEVEL_LABELS to lib/utils/request.ts (shared across 4 files)
- Pricing: factory fn removes sessionsPerMonth/tier duplication; bank details use CONFIGURE_BEFORE_LAUNCH placeholder
- DB migration: add index on payments.verified_by_user_id
- PackageSummary: Math.max(0) for sessionsRemaining; Math.min(100) for progress bar
- Dashboard: include expired packages in query
- Package selection: UTC dates with month-overflow clamping; duplicate guard with maybeSingle(); verify request status update affected rows
- Payment detail: maybeSingle() for payment query; sanitize filename (base + extension separately); sanitize pipe chars in reference string
- Admin actions: audit log console.error on failure; .eq('status','pending') guard before update; rollback error logging
- Admin payments: PaymentActions.tsx client component with useActionState for error display; validate pkg?.id && req?.id before action buttons"
Co-authored-by: Taleef7 <89072337+Taleef7@users.noreply.github.com>
All 26 review comments have been addressed in commit 7160012. Here's the summary of changes made:
|
Implements the full packages and payments system — package selection (8/12/20 sessions/month), PKR pricing display, bank transfer payment submission with optional proof upload, and admin payment verification that activates packages and advances request status.
Summary
Changes
supabase/migrations/20260224000001_create_packages_payments.sql):packagestable (tier_sessions 8/12/20 check constraint, start/end date, sessions_total/used, status enum, updated_at trigger, RLS),paymentstable (amount_pkr, method bank_transfer check constraint, reference, proof_path, rejection_note, verified_by_user_id/at, RLS),audit_logstable; indexes includingverified_by_user_idlib/config/pricing.ts):createPackageConfigfactory function derivessessionsPerMonthfromtier(eliminates duplication);PACKAGES[](8→PKR 8k, 12→PKR 11k, 20→PKR 16k);PAYMENT_INSTRUCTIONSwithCONFIGURE_BEFORE_LAUNCHplaceholders for bank details and a pipe-delimited reference formatLEVEL_LABELSextracted tolib/utils/request.tsand imported from there in all consumer files (eliminates duplication across 4 files)/dashboard/packages/new?requestId=): 3 tier cards with PKR pricing and policy notes; UTC dates with month-end overflow clamping; duplicate active/pending package guard (redirects to existing); verifies request status update affected rows before creating payment record/dashboard/packages/[id]): bank transfer instructions with personalised reference string (pipe chars stripped from interpolated values); optional proof upload (image/PDF ≤5 MB) with sanitized filename (base and extension processed separately); optional transaction reference;.maybeSingle()for payment query;htmlFor/idaccessibility attributes on both form inputs/admin/payments): filterable list (pending/paid/rejected/all), newest-first;PaymentActions.tsxclient component usinguseActionStatefor inline error display on Mark Paid/Reject forms; action buttons only rendered whenpkg?.id && req?.idare both presentactions.ts):.eq('status','pending')guard prevents double-processing; row-count check throws if payment already processed; compensating best-effort rollback withconsole.errorlogging on rollback failure; audit log failures surfaced viaconsole.errorcomponents/dashboards/PackageSummary.tsx):Math.max(0,...)for sessions remaining;Math.min(100,...)for progress bar percentage; handles pending/active/expired statesexpiredpackages in query so the expired state and renewal prompt are reachable;LEVEL_LABELSimported from shared utilTesting
Notes
payment-proofsSupabase Storage bucket must be created manually (private) — documented in READMElib/config/pricing.tsuseCONFIGURE_BEFORE_LAUNCHplaceholders — fill in before going livegetPaymentProofUrlaction inactions.tsfor future enhancement💡 You can make Copilot smarter by setting up custom instructions, customizing its development environment and configuring Model Context Protocol (MCP) servers. Learn more Copilot coding agent tips in the docs.