Twaha feat/admin#12
Conversation
There was a problem hiding this comment.
Pull request overview
This PR introduces a new “tactical” styled admin area in the Next.js app (dashboard + management pages), adds supporting UI components, and updates global styling/dependencies.
Changes:
- Add an
/adminroute group with pages for dashboard, members, projects, events, leaderboard config, and settings. - Introduce new reusable “Tactical*” UI components (table, cards, buttons, loading, feedback) used across admin pages.
- Add
lucide-reactdependency and update global CSS theme/styling; remove a DB import debug script.
Reviewed changes
Copilot reviewed 22 out of 23 changed files in this pull request and generated 14 comments.
Show a summary per file
| File | Description |
|---|---|
| test-db-import.ts | Removes DB import/debug logging script. |
| package.json | Adds lucide-react dependency. |
| bun.lock | Locks lucide-react dependency resolution. |
| components/ui/TacticalTable.tsx | New table UI component used by admin tables. |
| components/ui/TacticalLoading.tsx | New full-screen loading overlay for admin route. |
| components/ui/TacticalFeedback.tsx | New toast-like feedback component for admin actions. |
| components/ui/TacticalCard.tsx | New card component used across admin pages/layout. |
| components/ui/TacticalButton.tsx | New button component used across admin actions. |
| components/auth/SignOutButton.tsx | Client sign-out wrapper used by admin layout. |
| app/globals.css | Updates theme variables and adds global “tactical” visual effects/utilities. |
| app/admin/layout.tsx | Adds admin sidebar layout/navigation. |
| app/admin/loading.tsx | Adds admin route loading UI. |
| app/admin/page.tsx | Admin dashboard page (stats + links). |
| app/admin/members/page.tsx | Admin members listing page. |
| app/admin/members/MemberTable.tsx | Client members table with role toggling. |
| app/admin/projects/page.tsx | Admin projects listing page (includes counters). |
| app/admin/projects/ProjectTable.tsx | Client projects table with approve/delete actions. |
| app/admin/projects/actions.ts | Server actions for project delete/status updates. |
| app/admin/events/page.tsx | Admin events listing page. |
| app/admin/events/EventTable.tsx | Client events table with publish/delete actions. |
| app/admin/leaderboard/page.tsx | Admin leaderboard config + top-10 preview. |
| app/admin/leaderboard/WeightForm.tsx | Client form for editing score weights. |
| app/admin/settings/page.tsx | Admin settings/status display page. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| } | ||
| }, [timestamp]); | ||
|
|
||
| const borderStyles = variant === 'dashed' ? 'border-dashed border-zinc-600' : 'border-zinc-800'; |
There was a problem hiding this comment.
variant includes an "accent" option and is used as such (e.g. in the admin dashboard), but borderStyles only handles "dashed" vs default. As-is, the accent variant has no visual effect, which is likely unintended. Add styling branches for accent (and/or remove it from the type if not supported).
| const borderStyles = variant === 'dashed' ? 'border-dashed border-zinc-600' : 'border-zinc-800'; | |
| const borderStyles = | |
| variant === 'dashed' | |
| ? 'border-dashed border-zinc-600' | |
| : variant === 'accent' | |
| ? 'border-white/60 shadow-[0_0_0_1px_rgba(255,255,255,0.08)]' | |
| : 'border-zinc-800'; |
| startTransition(async () => { | ||
| addOptimisticAction({ type: 'approve', id }); | ||
| try { | ||
| const res = await fetch(`/api/admin/projects/${id}/approve`, { | ||
| method: "PATCH", | ||
| headers: { "Content-Type": "application/json" }, | ||
| body: JSON.stringify({ isApproved: true }) | ||
| }); | ||
| if (!res.ok) throw new Error(await res.text()); | ||
| router.refresh(); | ||
| setFeedback({ message: "PROJECT_CLEARANCE_GRANTHED", type: "success" }); | ||
| } catch (err) { | ||
| setFeedback({ | ||
| message: "APPROVE_FAILURE: " + (err instanceof Error ? err.message : "UNKNOWN"), | ||
| type: "error" | ||
| }); | ||
| } |
There was a problem hiding this comment.
handleApprove applies an optimistic update before the network request, but in the catch path there’s no rollback/re-sync. If the API call fails, the row will remain approved in optimisticProjects until a manual refresh, which is misleading. On error, revert the optimistic change (e.g., by keeping a snapshot / adding a compensating optimistic action) or at least router.refresh() to restore server truth.
| const handleDelete = async (id: string) => { | ||
| if (!confirm("CONFIRM_PROJECT_DELETION? THIS ACTION IS IRREVERSIBLE.")) return; | ||
| startTransition(async () => { | ||
| addOptimisticAction({ type: 'delete', id }); | ||
| try { | ||
| await deleteProject(id); | ||
| setFeedback({ message: "PROJECT_RECORD_ERASED", type: "success" }); | ||
| } catch (err) { | ||
| setFeedback({ | ||
| message: "DELETION_FAILURE: " + (err instanceof Error ? err.message : "UNKNOWN"), | ||
| type: "error" | ||
| }); | ||
| } | ||
| }); |
There was a problem hiding this comment.
handleDelete removes the project optimistically, but (1) on failure there’s no rollback/re-sync so the row will stay removed even if the delete didn’t happen, and (2) on success there’s no router.refresh(), so server-derived UI (e.g. the pending/approved counters rendered by the server page) won’t update. Consider refreshing after success and reverting (or refreshing) on error.
| <div className="grid grid-cols-1 lg:grid-cols-3 gap-12"> | ||
| <div className="lg:col-span-1"> | ||
| <TacticalCard title="WEIGHT_CONFIG" subtitle="Define algorithm parameters for score calculation."> | ||
| <WeightForm initialWeights={weights || { githubWeight: 30, lcWeight: 40, eventWeight: 30 }} /> |
There was a problem hiding this comment.
The fallback initialWeights here uses 30/40/30, but the /api/admin/config/weights endpoint (and WeightForm UI math) treat weights as fractions that sum to ~1.0 (e.g. 0.3/0.4/0.3). With the current fallback, the form will be permanently invalid and the range inputs will clamp incorrectly. Change the fallback to decimals (or update WeightForm + API contract to consistently use 0–100 everywhere).
| <WeightForm initialWeights={weights || { githubWeight: 30, lcWeight: 40, eventWeight: 30 }} /> | |
| <WeightForm initialWeights={weights || { githubWeight: 0.3, lcWeight: 0.4, eventWeight: 0.3 }} /> |
| </div> | ||
| </aside> | ||
|
|
||
| <main className="flex-1 relative bg-[url('https://grainy-gradients.vercel.app/noise.svg')] bg-repeat"> |
There was a problem hiding this comment.
This uses a third-party hosted asset (https://grainy-gradients.vercel.app/noise.svg) as a background image. That introduces an external dependency at runtime, can leak referrer information, and may violate CSP/offline requirements. Consider vendoring the SVG into /public (or inlining as a data URL) and referencing it locally instead.
| <main className="flex-1 relative bg-[url('https://grainy-gradients.vercel.app/noise.svg')] bg-repeat"> | |
| <main | |
| className="flex-1 relative bg-repeat" | |
| style={{ | |
| backgroundImage: | |
| 'url("data:image/svg+xml,%3Csvg xmlns=%27http://www.w3.org/2000/svg%27 width=%27160%27 height=%27160%27 viewBox=%270 0 160 160%27 fill=%27none%27%3E%3Cfilter id=%27n%27 x=%270%27 y=%270%27 width=%27100%25%27 height=%27100%25%27%3E%3CfeTurbulence type=%27fractalNoise%27 baseFrequency=%271.2%27 numOctaves=%274%27 stitchTiles=%27stitch%27/%3E%3C/filter%3E%3Crect width=%27160%27 height=%27160%27 filter=%27url(%2523n)%27 opacity=%270.12%27/%3E%3C/svg%3E")', | |
| }} | |
| > |
| <button | ||
| className={`${baseStyles} ${variants[variant]} ${sizes[size]} ${className}`} | ||
| {...props} | ||
| > | ||
| <span className="mr-2 opacity-50 group-hover:opacity-100 transition-opacity">{prefix}</span> |
There was a problem hiding this comment.
className is interpolated directly into the template string here, so when it’s undefined the literal "undefined" can end up in the DOM class attribute. Also, the prefix span uses group-hover:* but the button doesn’t include the group class, so that hover styling won’t activate. Consider defaulting className to an empty string (or conditionally appending it) and either add group to the button or remove group-hover usage.
| <ProjectTable initialProjects={projects.map(p => ({ | ||
| ...p, | ||
| leadName: p.lead?.name || "UNNAMED_LEAD", | ||
| leadGithub: p.lead?.githubUsername || null | ||
| }))} /> |
There was a problem hiding this comment.
initialProjects is built by spreading p, which includes the nested lead object from the Prisma query. Since the query selects lead.email, this can end up sending lead emails to the client even though the table only needs leadName/leadGithub. Build the object explicitly with only the fields needed by ProjectTable (and drop email from the select if it isn’t required).
|
|
||
| interface ProjectWithLead { | ||
| id: string; | ||
| title: string; | ||
| description: string | null; | ||
| status: "planning" | "active" | "completed" | "stalled"; | ||
| progressPct: number; | ||
| githubRepoUrl: string | null; | ||
| isApproved: boolean; | ||
| submittedAt: Date; | ||
| lead: { | ||
| name: string | null; | ||
| githubUsername: string | null; | ||
| } | null; | ||
| } | ||
|
|
||
| export default async function AdminProjectsPage() { | ||
| const session = await auth(); | ||
| if (session?.user?.role !== "admin") { | ||
| redirect("/"); | ||
| } | ||
|
|
||
| const projectsRaw = await db.project.findMany({ | ||
| include: { | ||
| lead: { | ||
| select: { | ||
| name: true, | ||
| email: true, | ||
| githubUsername: true | ||
| } | ||
| } | ||
| }, | ||
| orderBy: { | ||
| submittedAt: "desc" | ||
| } | ||
| }); | ||
|
|
||
| const projects = projectsRaw as unknown as ProjectWithLead[]; |
There was a problem hiding this comment.
The as unknown as ProjectWithLead[] assertion bypasses type-safety and can hide schema/query drift (e.g., missing/extra selected fields). Prefer deriving the type from Prisma (e.g. Prisma.ProjectGetPayload<{ include: ... }>), or constrain the query with select so TypeScript can infer the correct shape without unsafe casts.
| interface ProjectWithLead { | |
| id: string; | |
| title: string; | |
| description: string | null; | |
| status: "planning" | "active" | "completed" | "stalled"; | |
| progressPct: number; | |
| githubRepoUrl: string | null; | |
| isApproved: boolean; | |
| submittedAt: Date; | |
| lead: { | |
| name: string | null; | |
| githubUsername: string | null; | |
| } | null; | |
| } | |
| export default async function AdminProjectsPage() { | |
| const session = await auth(); | |
| if (session?.user?.role !== "admin") { | |
| redirect("/"); | |
| } | |
| const projectsRaw = await db.project.findMany({ | |
| include: { | |
| lead: { | |
| select: { | |
| name: true, | |
| email: true, | |
| githubUsername: true | |
| } | |
| } | |
| }, | |
| orderBy: { | |
| submittedAt: "desc" | |
| } | |
| }); | |
| const projects = projectsRaw as unknown as ProjectWithLead[]; | |
| import type { Prisma } from "@prisma/client"; | |
| const projectWithLeadArgs = Prisma.validator<Prisma.ProjectDefaultArgs>()({ | |
| include: { | |
| lead: { | |
| select: { | |
| name: true, | |
| email: true, | |
| githubUsername: true | |
| } | |
| } | |
| } | |
| }); | |
| type ProjectWithLead = Prisma.ProjectGetPayload<typeof projectWithLeadArgs>; | |
| export default async function AdminProjectsPage() { | |
| const session = await auth(); | |
| if (session?.user?.role !== "admin") { | |
| redirect("/"); | |
| } | |
| const projects: ProjectWithLead[] = await db.project.findMany({ | |
| ...projectWithLeadArgs, | |
| orderBy: { | |
| submittedAt: "desc" | |
| } | |
| }); |
| // Clear session and redirect, then force reload to purge any stale state | ||
| await signOut({ redirect: true, callbackUrl: "/login" }); | ||
| window.location.reload(); |
There was a problem hiding this comment.
signOut({ redirect: true, callbackUrl: "/login" }) will already navigate away; immediately calling window.location.reload() afterwards is redundant at best and can cause an extra full-page reload/flicker (and may not reliably run once navigation starts). Prefer relying on the redirect alone, or use redirect: false + router.push("/login")/router.refresh() if you need explicit state clearing.
| // Clear session and redirect, then force reload to purge any stale state | |
| await signOut({ redirect: true, callbackUrl: "/login" }); | |
| window.location.reload(); | |
| // Clear session and redirect to the login page | |
| await signOut({ redirect: true, callbackUrl: "/login" }); |
| const [isDismissed, setIsDismissed] = useState(false); | ||
|
|
||
| useEffect(() => { | ||
| if (message && !isDismissed && autoHideMs > 0) { | ||
| const timer = setTimeout(() => { | ||
| setIsDismissed(true); | ||
| if (onClear) onClear(); | ||
| }, autoHideMs); | ||
| return () => clearTimeout(timer); | ||
| } | ||
| }, [message, isDismissed, autoHideMs, onClear]); | ||
|
|
||
| if (!message || isDismissed) return null; |
There was a problem hiding this comment.
isDismissed persists across different message values. If the user dismisses one toast and then a new message is provided, this component will keep returning null and the new message won’t display unless the parent forces a remount. Consider resetting isDismissed to false whenever message changes so the component behaves correctly in isolation.
No description provided.