Improve admin matching and pricing polish#154
Conversation
There was a problem hiding this comment.
Pull request overview
Improves admin request matching workflows and UI polish by adding request text search, availability overlap hints, breadcrumbs/profile links, and consistent PKR currency formatting.
Changes:
- Added shared
formatPkr()currency formatter and replaced ad-hoc PKR rendering across landing/dashboard/admin pages. - Added admin request text search (student/requester/subject) and extracted request-search helpers with tests.
- Added availability overlap helpers + UI hint badges, plus admin breadcrumbs and match-detail profile links; updated planning docs accordingly.
Reviewed changes
Copilot reviewed 20 out of 20 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
| lib/utils/currency.ts | Introduces shared PKR formatter used across UI. |
| lib/utils/availability.ts | Adds overlap computation utilities for structured availability windows. |
| lib/utils/tests/currency.test.ts | Unit tests for PKR formatter output. |
| lib/utils/tests/availability.test.ts | Unit tests for overlap and summary helpers. |
| lib/admin/request-search.ts | Adds normalization + tokenized in-memory filtering for admin requests. |
| lib/admin/tests/request-search.test.ts | Unit tests for request search normalization/filtering behavior. |
| components/admin/AdminBreadcrumbs.tsx | Adds reusable breadcrumbs component for admin detail pages. |
| components/admin/tests/AdminBreadcrumbs.test.ts | Verifies breadcrumbs render deterministic return links. |
| app/page.tsx | Switches landing package price display to formatPkr(). |
| app/dashboard/packages/new/page.tsx | Uses formatPkr() for package selection pricing display. |
| app/dashboard/packages/[id]/page.tsx | Uses formatPkr() for package payment amount display. |
| app/admin/payments/page.tsx | Uses formatPkr() for admin payment amount display. |
| app/admin/requests/page.tsx | Adds q search param support + composes search with existing filters/pagination. |
| app/admin/requests/RequestFilters.tsx | Adds search form UI and preserves filter params when submitting/clearing. |
| app/admin/requests/[id]/page.tsx | Adds breadcrumbs and passes request availability windows into matching UI. |
| app/admin/requests/[id]/AssignTutorForm.tsx | Adds overlap hint badges per tutor and wires overlap helper into tutor cards. |
| app/admin/tutors/[id]/page.tsx | Adds admin breadcrumbs on tutor detail page. |
| app/admin/matches/[id]/page.tsx | Adds breadcrumbs and links to admin user search / tutor detail from match detail. |
| docs/plan-CorvEd.md | Marks related MVP/backlog items as completed. |
| docs/GAP_ANALYSIS.md | Updates resolved doc gap to reflect p_request_id guidance cleanup. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| if (activeSearch) { | ||
| const [{ data: requestsData }, { data: subjectsData }] = await Promise.all([ | ||
| dataQuery, | ||
| subjectsQuery, | ||
| ]) |
There was a problem hiding this comment.
In the search branch (if (activeSearch)), dataQuery is executed without .range(...) / any limit, which means this path can fetch the full filtered requests dataset into memory before doing JS filtering + pagination. This is likely to become slow/expensive as the requests table grows (and can also be impacted by any PostgREST max-rows cap, making totalCount incomplete). Consider moving the text search into the DB (e.g., PostgREST or/ilike across for_student_name, user_profiles.display_name, and subjects.name, or a view/RPC for full-text search) so pagination + counts stay server-side.
| const { status, subject, level, page } = await searchParams | ||
| const { status, subject, level, q, page } = await searchParams | ||
| const activeStatus: FilterStatus = ALL_STATUSES.includes(status ?? '') ? status! : 'all' | ||
| const activeSearch = normalizeAdminRequestSearch(q) |
There was a problem hiding this comment.
activeSearch is normalized (trim/collapse whitespace + lowercased) and then reused for building URLs and as the search input's defaultValue. This means a user searching for e.g. "Amina Khan" will see the UI and subsequent links rewrite it to "amina khan", which is surprising and doesn't strictly preserve the query param as-entered. Consider keeping the raw q for display/URL generation and only normalizing internally for token matching.
| const activeSearch = normalizeAdminRequestSearch(q) | |
| const activeSearch = typeof q === 'string' ? q : '' | |
| const normalizedActiveSearch = normalizeAdminRequestSearch(activeSearch) |
| href={`/admin/users?search=${encodeURIComponent(studentProfile.display_name)}`} | ||
| className="text-[#1040C0] underline-offset-4 hover:underline" | ||
| > | ||
| {studentName} |
There was a problem hiding this comment.
The student profile link is gated on studentProfile.display_name and the search param uses studentProfile.display_name, but the link text uses studentName (which is the child name for parent-created requests). For parent requests this can look like the child's name links to the child's profile, but it actually navigates to the parent user search. Consider adjusting the link label to match the requester profile name (or rendering separate "Student" vs "Requester" rows) so the navigation target is unambiguous.
| {studentName} | |
| {studentProfile.display_name} |
| export function formatPkr(amount: number | null | undefined): string { | ||
| const value = Number.isFinite(amount) ? Math.max(0, Math.round(amount ?? 0)) : 0 | ||
| const formatted = new Intl.NumberFormat('en-PK', { | ||
| maximumFractionDigits: 0, | ||
| minimumFractionDigits: 0, | ||
| }).format(value) |
There was a problem hiding this comment.
formatPkr constructs a new Intl.NumberFormat instance on every call. This can add avoidable overhead when rendering lists (landing packages, admin payments, etc.). Consider hoisting a single formatter instance to module scope (or caching lazily) and reusing it inside formatPkr.
| export function formatPkr(amount: number | null | undefined): string { | |
| const value = Number.isFinite(amount) ? Math.max(0, Math.round(amount ?? 0)) : 0 | |
| const formatted = new Intl.NumberFormat('en-PK', { | |
| maximumFractionDigits: 0, | |
| minimumFractionDigits: 0, | |
| }).format(value) | |
| const pkrNumberFormatter = new Intl.NumberFormat('en-PK', { | |
| maximumFractionDigits: 0, | |
| minimumFractionDigits: 0, | |
| }) | |
| export function formatPkr(amount: number | null | undefined): string { | |
| const value = Number.isFinite(amount) ? Math.max(0, Math.round(amount ?? 0)) : 0 | |
| const formatted = pkrNumberFormatter.format(value) |
|
@copilot apply changes based on the comments in this thread |
…DB-level text search, link label fix Agent-Logs-Url: https://github.com/Taleef7/CorvEd/sessions/1814cfea-3d7d-4359-83cc-92d639d10484 Co-authored-by: Taleef7 <89072337+Taleef7@users.noreply.github.com>
All four review comments addressed in commit
|
Summary
Verification
Notes
Closes #116
Closes #123
Closes #124
Closes #135
Closes #147
Closes #148