🏆 Transform Projects to Zenith Hall with Enhanced Branding #228
Conversation
…vicon fixes ✨ Features: - Renamed /projects to /zenith-hall with prestigious branding - Added comprehensive favicon configuration (light/dark theme support) - Enhanced project showcase with 'Hall of Excellence' theme - Professional tribute description for community recognition 🎨 UI/UX Improvements: - Updated navigation to 'Zenith Hall' - Enhanced search placeholder and filter labels - Improved professional terminology throughout - Added theme-aware favicon support 🔧 Technical Improvements: - Fixed React unescaped entities warning - Resolved TypeScript linting issues - Added proper favicon.ico fallback - Enhanced metadata configuration 📁 File Changes: - Moved app/projects/ → app/zenith-hall/ - Updated header navigation links - Enhanced layout.tsx with favicon configuration - Improved SEO metadata with icon support 🏆 Result: Prestigious hall of excellence showcasing community achievements
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
WalkthroughRenamed Projects -> Zenith Hall with redirects; added favicon metadata; replaced server-side DOMPurify with isomorphic-dompurify and stronger input type checks; added protected Jobs feature with paid/free apply flows and Razorpay; introduced multiple test-related pages/APIs and profile API changes; improved sidebar accessibility; removed a debug profile route. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
participant U as User
participant JP as JobsPage
participant AD as ApplyDialog
participant API1 as /api/internships/create-order
participant RZ as Razorpay Checkout
participant API2 as /api/internships/apply
participant API3 as /api/internships/my-applications
U->>JP: Visit /protected/jobs
JP->>API3: Fetch applied applications (?t=timestamp)
API3-->>JP: Applied IDs
U->>AD: Open Apply (select options, cover note)
alt Free internship
AD->>API2: Submit application (amount 0)
API2-->>AD: Success/Failure
else Paid internship
AD->>JP: Request payment
JP->>API1: Create order (amount, discount if premium)
API1-->>JP: orderId, amount
JP->>RZ: Open checkout (orderId)
RZ-->>JP: Callback (paymentId, signature)
JP->>API2: Submit application (+payment details)
API2-->>JP: Success/Failure
end
JP-->>U: Toast result and update UI
sequenceDiagram
autonumber
participant M as withInputValidation
participant R as Request Body
participant S as Sanitizer (isomorphic-dompurify)
M->>R: Read fields
loop For each field
alt Non-string value
M-->>M: Add "<key>: must be a string" error
else String value
M->>S: sanitize(value)
S-->>M: cleaned value
end
end
M-->>M: Return cleaned data or errors
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60–90 minutes Pre-merge checks (2 passed, 1 warning)❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
Poem
Tip 👮 Agentic pre-merge checks are now available in preview!Pro plan users can now enable pre-merge checks in their settings to enforce checklists before merging PRs.
Please see the documentation for more information. Example: reviews:
pre_merge_checks:
custom_checks:
- name: "Undocumented Breaking Changes"
mode: "warning"
instructions: |
Pass/fail criteria: All breaking changes to public APIs, CLI flags, environment variables, configuration keys, database schemas, or HTTP/GraphQL endpoints must be documented in the "Breaking Change" section of the PR description and in CHANGELOG.md. Exclude purely internal or private changes (e.g., code not exported from package entry points or explicitly marked as internal).Please share your feedback with us on this Discord post. ✨ Finishing Touches
🧪 Generate unit tests
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
lib/security/input-validation.ts (2)
1-7: Make DOMPurify isomorphic and drop theanycast (prevents client bundle breakage).Using
jsdomat module scope risks bundling Node-only deps into client code and theanycast hides type issues. Preferisomorphic-dompurify, which works in both SSR and the browser, and removes the need foreslint-disable.Apply:
-import DOMPurify from 'dompurify'; -import { JSDOM } from 'jsdom'; - -// Create a DOMPurify instance for server-side use -const window = new JSDOM('').window; -// eslint-disable-next-line @typescript-eslint/no-explicit-any -const purify = DOMPurify(window as any); +import DOMPurify from 'isomorphic-dompurify'; +// Works in both server and client without jsdom +const purify = DOMPurify;Dev note: add the dependency
isomorphic-dompurify.
469-477: Guard against non-string inputs inwithInputValidationto avoid runtime errors.
validateTextcalls.trim()and assumes a string; non-strings will throw. Enforce type and give a clear error.Apply:
- const value = body[key]; - const result = InputValidator.validateText(value, options); + const value = body[key]; + if (typeof value !== 'string') { + errors.push(`${key}: must be a string`); + continue; + } + const result = InputValidator.validateText(value, options);
🧹 Nitpick comments (5)
lib/security/input-validation.ts (2)
434-447: Reduce false positives in XSS validation or rename to reflect behavior.Treating any sanitization diff as “invalid” can reject harmless user input (e.g., stray angle brackets). Consider returning the sanitized value as valid, or rename to
detectDangerousContentand use it only for detection flows.
116-145: Email regex is simplistic; consider a vetted validator.For production-grade validation (IDNs, uncommon but valid formats), prefer
validator.js’sisEmailwith options.I can replace this with
validator.jsand unit tests if you want.components/header.tsx (1)
61-62: Navigation rename looks good; addaria-currentfor a11y.Expose the active state to screen readers.
Apply:
- <Link + <Link key={item.href} href={item.href} + aria-current={isActive(item.href) ? "page" : undefined}Apply the same in the mobile nav link below.
- <Link + <Link key={item.href} href={item.href} + aria-current={isActive(item.href) ? "page" : undefined}app/zenith-hall/page.tsx (2)
99-101: Decorative pill should not be a button.It’s non-interactive. Use a non-focusable element to avoid confusion for assistive tech.
Apply:
- <button className="bg-slate-800 no-underline group relative shadow-2xl shadow-zinc-900 rounded-full p-px text-sm font-semibold leading-6 text-white inline-block cursor-default"> + <div aria-hidden="true" role="presentation" className="bg-slate-800 no-underline group relative shadow-2xl shadow-zinc-900 rounded-full p-px text-sm font-semibold leading-6 text-white inline-block cursor-default"> ... - </button> + </div>
124-128: Add basic a11y to search input.Use
type="search"and an explicit label/aria-label.Apply:
- <Input - type="text" - placeholder="Search our hall of excellence by project, technology, or member..." + <Input + type="search" + aria-label="Search the Zenith Hall" + placeholder="Search by project, technology, or member..." className="w-full pl-12 pr-4 py-3 text-lg bg-background/70 backdrop-blur-sm rounded-full focus:ring-2 focus:ring-primary/50"
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
public/favicon.icois excluded by!**/*.ico
📒 Files selected for processing (5)
app/layout.tsx(1 hunks)app/zenith-hall/page.tsx(3 hunks)components/header.tsx(1 hunks)lib/security/input-validation.ts(1 hunks)lib/seo/metadata.ts(1 hunks)
🧰 Additional context used
🧬 Code graph analysis (1)
app/zenith-hall/page.tsx (3)
components/ui/badge.tsx (1)
Badge(36-36)components/ui/button.tsx (1)
Button(59-59)components/ui/dropdown-menu.tsx (3)
DropdownMenuTrigger(187-187)DropdownMenuContent(188-188)DropdownMenuLabel(192-192)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: Test Suite
🔇 Additional comments (3)
lib/security/input-validation.ts (1)
1-7: Ensure no client components importlib/security/input-validation.ts. This module depends onjsdomand will break client bundles; manually verify it isn’t pulled into any file with a"use client"directive.app/zenith-hall/page.tsx (2)
107-108: Hero heading/copy update LGTM.Branding reads cleaner and aligns with the new nav.
141-142: Filter label change LGTM.Wording is clearer for users.
| { href: "/zenith-hall", label: "Zenith Hall" }, | ||
| { href: "/blog", label: "Blog" }, |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Add a redirect from /projects → /zenith-hall to preserve old links.
Prevents 404s from bookmarks, SEO, and external refs.
Add to next.config.js:
module.exports = {
async redirects() {
return [
+ { source: '/projects', destination: '/zenith-hall', permanent: true },
+ { source: '/projects/:path*', destination: '/zenith-hall', permanent: true },
];
},
};Committable suggestion skipped: line range outside the PR's diff.
🤖 Prompt for AI Agents
In components/header.tsx around lines 61 to 62 you added a new "Zenith Hall" nav
item but there is no redirect for legacy /projects links; to preserve old links
and SEO add a redirect in next.config.js that maps path "/projects" to
"/zenith-hall" (use a permanent redirect/301), ensure the redirects array is
exported from next.config.js and return the new rule so both dev and production
builds route /projects to /zenith-hall.
| // Favicon configuration | ||
| icons: { | ||
| icon: [ | ||
| { url: '/favicon.ico', type: 'image/x-icon' }, | ||
| { url: '/codeunia-favicon-light.svg', media: '(prefers-color-scheme: light)' }, | ||
| { url: '/codeunia-favicon-dark.svg', media: '(prefers-color-scheme: dark)' }, | ||
| { url: '/codeunia-favicon-light.svg' } | ||
| ], | ||
| apple: '/codeunia-favicon-light.svg', | ||
| shortcut: '/favicon.ico' | ||
| }, | ||
|
|
There was a problem hiding this comment.
💡 Verification agent
🧩 Analysis chain
Avoid duplicate favicon declarations and fix Apple touch icon format.
You’re defining icons here and again in app/layout.tsx; keep one source of truth (prefer Metadata). Also, iOS Safari requires a PNG for apple-touch-icon; SVG won’t render.
Apply:
- // Favicon configuration
- icons: {
- icon: [
- { url: '/favicon.ico', type: 'image/x-icon' },
- { url: '/codeunia-favicon-light.svg', media: '(prefers-color-scheme: light)' },
- { url: '/codeunia-favicon-dark.svg', media: '(prefers-color-scheme: dark)' },
- { url: '/codeunia-favicon-light.svg' }
- ],
- apple: '/codeunia-favicon-light.svg',
- shortcut: '/favicon.ico'
- },
+ // Favicon configuration
+ icons: {
+ icon: [
+ { url: '/favicon.ico', type: 'image/x-icon' },
+ { url: '/codeunia-favicon-light.svg', media: '(prefers-color-scheme: light)' },
+ { url: '/codeunia-favicon-dark.svg', media: '(prefers-color-scheme: dark)' },
+ { url: '/codeunia-favicon-light.svg' }
+ ],
+ apple: [{ url: '/apple-touch-icon.png', sizes: '180x180' }],
+ shortcut: ['/favicon.ico']
+ },Verification script (checks files exist and flags duplicates in layout):
🏁 Script executed:
#!/bin/bash
# Icons present?
fd -a 'favicon|apple-touch' --base-directory . public || true
# Check if layout also injects <link rel="icon"> (should be removed if using Metadata)
rg -n "rel=[\"']icon|apple-touch-icon|shortcut icon" app/layout.tsx -n || trueLength of output: 686
Consolidate favicon config and fix Apple touch icon
- Remove duplicate
<link rel="icon">/<link rel="apple-touch-icon">tags inapp/layout.tsx(lines 73–78)—rely solely on theiconsfield inlib/seo/metadata.ts. - Update
icons.appleto an array entry with a PNG (e.g./apple-touch-icon.png) andsizes: '180x180', since iOS Safari doesn’t support SVG for apple-touch-icon.
🤖 Prompt for AI Agents
In lib/seo/metadata.ts around lines 58–69, the favicon configuration currently
duplicates entries and sets icons.apple to an SVG; update it to consolidate
favicon entries (remove the duplicate '/codeunia-favicon-light.svg' entry) and
replace the icons.apple string with an array entry for an apple-touch-icon using
a PNG (e.g. { url: '/apple-touch-icon.png', sizes: '180x180', type: 'image/png'
}). Also remove the duplicate <link rel="icon">/<link rel="apple-touch-icon">
tags from app/layout.tsx (lines 73–78) so the app relies solely on the icons
field in lib/seo/metadata.ts.
✨ New Features: - Add /protected/jobs page with real internship data - Integrate full internship application system for authenticated users 🔧 Code Review Fixes: - Replace DOMPurify with isomorphic-dompurify (prevents client bundle issues) - Add type guard for non-string inputs in validation - Remove duplicate favicon tags from layout.tsx - Add /projects → /zenith-hall redirect in next.config.js - Update Apple touch icon to PNG format for iOS compatibility ♿ Accessibility Improvements: - Add SheetTitle components to StudentSidebar and AdminSidebar - Fix console accessibility errors for screen readers 📦 Dependencies: - Add isomorphic-dompurify package - Create apple-touch-icon.png for better iOS support 🧪 Testing: - Build passes with no warnings or errors - All accessibility issues resolved - Real internship data properly integrated
There was a problem hiding this comment.
Actionable comments posted: 6
🧹 Nitpick comments (7)
components/users/StudentSidebar.tsx (1)
36-36: Accessible Sheet title looks good. Also add aria-labels to icon-only buttons.SheetTitle with sr-only is a solid a11y improvement. Add aria-label to the Menu icon button for screen readers.
Apply:
-<Button variant="outline" size="icon" className="border-zinc-700 bg-zinc-900 hover:bg-purple-700/10"> +<Button aria-label="Open navigation menu" variant="outline" size="icon" className="border-zinc-700 bg-zinc-900 hover:bg-purple-700/10">Also applies to: 89-89
components/admin/Sidebar.tsx (1)
34-34: Admin SheetTitle a11y: nice. Add aria-label to the mobile Menu trigger.Same a11y nit: label the icon-only button.
-<Button variant="outline" size="icon" className="md:hidden fixed top-4 left-4 z-50 border-zinc-700 bg-zinc-900 hover:bg-purple-700/10"> +<Button aria-label="Open admin navigation menu" variant="outline" size="icon" className="md:hidden fixed top-4 left-4 z-50 border-zinc-700 bg-zinc-900 hover:bg-purple-700/10">Also applies to: 74-74
lib/security/input-validation.ts (1)
467-470: Optional: support numbers/booleans by coercion when schema allows.If some endpoints expect numeric fields, consider an option like schema[key].coerceString to String(value) before validation to avoid rigid failures.
app/protected/jobs/page.tsx (4)
182-190: Avoid injecting Razorpay script repeatedly; reuse if already loaded.Repeated opens will append multiple scripts. Check window.Razorpay or reuse a single script tag.
-// Load Razorpay and open checkout -const script = document.createElement('script') -script.src = 'https://checkout.razorpay.com/v1/checkout.js' -script.async = true -document.body.appendChild(script) -await new Promise((resolve, reject) => { - script.onload = resolve - script.onerror = reject -}) +// Load Razorpay once +if (!(window as any).Razorpay) { + let script = document.getElementById('razorpay-sdk') as HTMLScriptElement | null + if (!script) { + script = document.createElement('script') + script.id = 'razorpay-sdk' + script.src = 'https://checkout.razorpay.com/v1/checkout.js' + script.async = true + document.body.appendChild(script) + } + await new Promise((resolve, reject) => { + if ((window as any).Razorpay) return resolve(null) + script!.onload = resolve + script!.onerror = reject + }) +}Also applies to: 238-239
389-392: Dialog pricing ignores Premium state — align message with card/checkout.Show the discounted text when premium is active to avoid confusing users.
-<div className="text-sm"><span className="font-medium">Price:</span> ₹699 (4 weeks) / ₹999 (6 weeks)</div> +{(() => { + const isPremium = profile?.is_premium && profile?.premium_expires_at && new Date(profile.premium_expires_at) > new Date() + return ( + <div className="text-sm"> + <span className="font-medium">Price:</span>{' '} + {isPremium ? ( + <>₹350 (4 weeks) / ₹500 (6 weeks) - Premium 50% off! 🎉 <span className="ml-1 text-xs text-muted-foreground line-through">Regular: ₹699 / ₹999</span></> + ) : ( + <>₹699 (4 weeks) / ₹999 (6 weeks)</> + )} + </div> + ) +})()}
503-505: Confirm button label should reflect duration and premium discount.The CTA currently shows regular prices only. Compute dynamic label.
-<Button onClick={handleApply}>{selected?.type === 'Paid' ? (selectedDuration === 6 ? 'Pay ₹999' : selectedDuration === 4 ? 'Pay ₹699' : 'Pay') : 'Submit Application'}</Button> +<Button onClick={handleApply}> + {selected?.type === 'Paid' ? (() => { + if (!selectedDuration) return 'Pay' + const isPremium = profile?.is_premium && profile?.premium_expires_at && new Date(profile.premium_expires_at) > new Date() + const price = selectedDuration === 6 ? (isPremium ? 500 : 999) : (isPremium ? 350 : 699) + return `Pay ₹${price}` + })() : 'Submit Application'} +</Button>
123-137: Type safety: pass the correct update type to updateProfile.You’re asserting to Profile, but updateProfile expects ProfileUpdateData. Type profileDraft accordingly and avoid the cast.
-import type { Profile } from '@/types/profile' +import type { ProfileUpdateData } from '@/types/profile' ... -const [profileDraft, setProfileDraft] = useState({ +const [profileDraft, setProfileDraft] = useState<ProfileUpdateData>({ first_name: '', last_name: '', github_url: '', linkedin_url: '' }) ... -const ok = await updateProfile(profileDraft as Profile) +const ok = await updateProfile(profileDraft)If ProfileUpdateData lives elsewhere, import from the correct module.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (2)
package-lock.jsonis excluded by!**/package-lock.jsonpublic/apple-touch-icon.pngis excluded by!**/*.png
📒 Files selected for processing (8)
app/protected/jobs/page.tsx(1 hunks)app/protected/layout.tsx(1 hunks)components/admin/Sidebar.tsx(2 hunks)components/users/StudentSidebar.tsx(2 hunks)lib/security/input-validation.ts(2 hunks)lib/seo/metadata.ts(1 hunks)next.config.ts(1 hunks)package.json(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- lib/seo/metadata.ts
🧰 Additional context used
🧬 Code graph analysis (3)
components/users/StudentSidebar.tsx (1)
components/ui/sheet.tsx (1)
SheetTitle(137-137)
components/admin/Sidebar.tsx (1)
components/ui/sheet.tsx (1)
SheetTitle(137-137)
app/protected/jobs/page.tsx (4)
lib/hooks/useAuth.ts (1)
useAuth(7-106)hooks/useProfile.ts (1)
useProfile(6-80)lib/api-fetch.ts (1)
apiFetch(14-50)lib/services/profile.ts (1)
updateProfile(59-83)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: Test Suite
🔇 Additional comments (1)
lib/security/input-validation.ts (1)
1-3: Switch to isomorphic-dompurify — good server/client alignment.This simplifies SSR safety without jsdom wiring. No issues spotted.
| // initial and on user change | ||
| useMemo(() => { if (user?.id) loadApplied() }, [user?.id, loadApplied]) | ||
|
|
There was a problem hiding this comment.
🛠️ Refactor suggestion
Don’t use useMemo for side effects — switch to useEffect.
useMemo is not for effects and may be skipped. Use useEffect to load applications on user change.
-// initial and on user change
-useMemo(() => { if (user?.id) loadApplied() }, [user?.id, loadApplied])
+// initial and on user change
+useEffect(() => { if (user?.id) loadApplied() }, [user?.id, loadApplied])📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| // initial and on user change | |
| useMemo(() => { if (user?.id) loadApplied() }, [user?.id, loadApplied]) | |
| // initial and on user change | |
| useEffect(() => { | |
| if (user?.id) loadApplied() | |
| }, [user?.id, loadApplied]) |
🤖 Prompt for AI Agents
In app/protected/jobs/page.tsx around lines 120 to 122, the code uses useMemo
for a side effect (loading applications) which is incorrect; replace useMemo
with useEffect, run the effect when user?.id or loadApplied change, and inside
the effect check if user?.id is truthy before calling loadApplied; if
loadApplied is async, call it from an inner async function and guard with an
isMounted flag or cancellation to avoid setting state on unmounted components.
| const handleApply = useCallback(async () => { | ||
| if (!user?.id) { | ||
| toast.error('Please sign in to apply') | ||
| return | ||
| } | ||
| if (!selected) return | ||
| if (!selectedDomain || !selectedLevel) { | ||
| toast.error('Please select domain and level') | ||
| return | ||
| } | ||
| if (!selectedDuration) { | ||
| toast.error('Please select duration') | ||
| return | ||
| } | ||
|
|
||
| try { | ||
| if (!user?.id) { | ||
| toast.error('Not authenticated. Please sign in again.') | ||
| return | ||
| } | ||
| if (selected.type === 'Paid') { | ||
| const basePrice = selectedDuration === 6 ? 999 : 699 | ||
|
|
||
| // Check if user is a premium member for 50% discount | ||
| const isPremium = profile?.is_premium && profile?.premium_expires_at && | ||
| new Date(profile.premium_expires_at) > new Date() | ||
|
|
||
| const price = isPremium ? Math.floor(basePrice / 2) : basePrice | ||
|
|
There was a problem hiding this comment.
🛠️ Refactor suggestion
Prevent double submissions during apply/payment.
Add an “applying” guard to avoid duplicate orders/charges and disable CTAs while in-flight.
-const [savingProfile, setSavingProfile] = useState(false)
+const [savingProfile, setSavingProfile] = useState(false)
+const [applying, setApplying] = useState(false)
...
-try {
+try {
+ if (applying) return
+ setApplying(true)
if (!user?.id) {
toast.error('Not authenticated. Please sign in again.')
return
}
...
} catch (e) {
toast.error(e instanceof Error ? e.message : 'Failed to apply')
} finally {
+ setApplying(false)
}-<Button onClick={handleApply}>
+<Button onClick={handleApply} disabled={applying}>Also applies to: 502-505
🤖 Prompt for AI Agents
In app/protected/jobs/page.tsx around lines 139 to 167 (and also apply same
change at lines 502-505), the apply flow lacks a guard to prevent double
submissions; introduce an "applying" boolean state and check it at the top of
handleApply to return early if already true, set applying = true immediately
before starting async work, and set applying = false in a finally block so it
always resets after success or error; additionally wire this state to disable
the apply/payment CTAs (buttons/controls) while applying is true so users cannot
trigger duplicate orders/charges.
| { | ||
| title: "Jobs", | ||
| url: "/protected/jobs", | ||
| icon: Briefcase, | ||
| }, |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Duplicate “Jobs” link (also under Career) — de-duplicate to avoid confusion.
You now have two items pointing to /protected/jobs (“Jobs” here and “Job Opportunities” under Career). Keep one to maintain IA clarity.
Apply one of:
- {
- title: "Jobs",
- url: "/protected/jobs",
- icon: Briefcase,
- },or rename/retarget as needed.
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| { | |
| title: "Jobs", | |
| url: "/protected/jobs", | |
| icon: Briefcase, | |
| }, |
🤖 Prompt for AI Agents
In app/protected/layout.tsx around lines 91-95 there is a duplicate navigation
item pointing to /protected/jobs ("Jobs") which also exists as "Job
Opportunities" under Career; remove this duplicate entry (delete the object at
those lines) to keep a single link, or if you intended a different destination,
rename the title and update the url/icon to the correct route so the two entries
are distinct.
| if (typeof value !== 'string') { | ||
| errors.push(`${key}: must be a string`); | ||
| continue; | ||
| } |
There was a problem hiding this comment.
Required-field regression: missing values now report “must be a string”.
The new typeof check fires for undefined/null, masking “required” errors and changing API behavior. Let validateText handle required-ness for absent values; reserve the type error for non-string present values.
Apply:
- const value = body[key];
- if (typeof value !== 'string') {
- errors.push(`${key}: must be a string`);
- continue;
- }
- const result = InputValidator.validateText(value, options);
+ const raw = body[key];
+ if (raw === undefined || raw === null) {
+ // Let validateText emit required/empty errors as appropriate
+ const result = InputValidator.validateText('', options);
+ if (!result.isValid) errors.push(`${key}: ${result.error}`);
+ else validatedData[key as keyof T] = result.sanitizedValue as T[keyof T];
+ continue;
+ }
+ if (typeof raw !== 'string') {
+ errors.push(`${key}: must be a string`);
+ continue;
+ }
+ const result = InputValidator.validateText(raw, options);📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| if (typeof value !== 'string') { | |
| errors.push(`${key}: must be a string`); | |
| continue; | |
| } | |
| const raw = body[key]; | |
| if (raw === undefined || raw === null) { | |
| // Let validateText emit required/empty errors as appropriate | |
| const result = InputValidator.validateText('', options); | |
| if (!result.isValid) errors.push(`${key}: ${result.error}`); | |
| else validatedData[key as keyof T] = result.sanitizedValue as T[keyof T]; | |
| continue; | |
| } | |
| if (typeof raw !== 'string') { | |
| errors.push(`${key}: must be a string`); | |
| continue; | |
| } | |
| const result = InputValidator.validateText(raw, options); |
🤖 Prompt for AI Agents
In lib/security/input-validation.ts around lines 467 to 470, the added typeof
check treats undefined/null as non-strings and overwrites required-field errors;
change the guard so it only flags a type error for values that are present but
not strings (e.g., if value !== undefined && value !== null && typeof value !==
'string'), allowing validateText to handle required-ness for absent values and
only continue when you intentionally handled the non-string present case.
| async redirects() { | ||
| return [ | ||
| { source: '/projects', destination: '/zenith-hall', permanent: true }, | ||
| { source: '/projects/:path*', destination: '/zenith-hall', permanent: true }, | ||
| ]; | ||
| }, | ||
|
|
There was a problem hiding this comment.
Deep-link redirect drops path segments.
/projects/:path* → /zenith-hall loses the subpath. Preserve it to avoid 404s and SEO loss.
async redirects() {
return [
{ source: '/projects', destination: '/zenith-hall', permanent: true },
- { source: '/projects/:path*', destination: '/zenith-hall', permanent: true },
+ { source: '/projects/:path*', destination: '/zenith-hall/:path*', permanent: true },
];
},📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| async redirects() { | |
| return [ | |
| { source: '/projects', destination: '/zenith-hall', permanent: true }, | |
| { source: '/projects/:path*', destination: '/zenith-hall', permanent: true }, | |
| ]; | |
| }, | |
| async redirects() { | |
| return [ | |
| { source: '/projects', destination: '/zenith-hall', permanent: true }, | |
| { source: '/projects/:path*', destination: '/zenith-hall/:path*', permanent: true }, | |
| ]; | |
| }, |
🤖 Prompt for AI Agents
In next.config.ts around lines 63 to 69, the redirect for deep links drops path
segments because the destination is a fixed path; update the second redirect so
it preserves the captured segments by using a matching destination with the same
wildcard (e.g., change destination to /zenith-hall/:path*), leaving the first
redirect (/projects → /zenith-hall) as-is and keeping permanent: true.
| "html-to-image": "^1.11.13", | ||
| "html2canvas": "^1.4.1", | ||
| "ioredis": "^5.7.0", | ||
| "isomorphic-dompurify": "^2.26.0", |
There was a problem hiding this comment.
💡 Verification agent
🧩 Analysis chain
Add isomorphic-dompurify — verify duplicate DOMPurify deps and typings.
Good addition. Since you also have "dompurify" and "@types/dompurify", please confirm we’re not importing DOMPurify directly elsewhere. If unused, consider pruning to reduce install size and avoid version skew.
Run to check for direct DOMPurify imports:
🏁 Script executed:
#!/bin/bash
rg -nP "from ['\"]dompurify['\"]|require\\(['\"]dompurify['\"]\\)"Length of output: 67
Replace direct DOMPurify import with isomorphic-dompurify
- In lib/security/html-sanitizer.ts, change
import DOMPurify from 'dompurify'
to
import DOMPurify from 'isomorphic-dompurify' - Once updated, remove the now-unused dompurify and @types/dompurify dependencies to prevent duplication and version drift.
🤖 Prompt for AI Agents
In package.json around line 71 and in lib/security/html-sanitizer.ts, replace
the direct DOMPurify dependency/import: update the import in
lib/security/html-sanitizer.ts from "dompurify" to "isomorphic-dompurify" and
then remove the now-unused "dompurify" and "@types/dompurify" entries from
package.json (and from lockfile if present); ensure TypeScript types still
resolve (install/update types if needed for isomorphic-dompurify) and run a
build/test to confirm no remaining references to the removed packages.
- Remove duplicate name display in profile pages - Remove console logs from profile fetching - Fix SSL certificate errors by creating proper API endpoints - Remove debug API endpoint that was causing security issues - Fix TypeScript errors in test components - Clean up ESLint warnings (unused imports/variables) - Improve profile data fetching architecture - Ensure clean build with no warnings or errors All profile-related issues resolved and build is now production-ready.
There was a problem hiding this comment.
Actionable comments posted: 11
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
app/tests/[id]/results/page.tsx (1)
60-69: Enforce server-side authZ: add RLS or server route to protect test_attempts and profiles
- No RLS policies found in supabase/migrations or any *.sql; reads of test_attempts and profiles are currently unprotected.
- Define RLS policies on test_attempts and profiles to allow SELECT only when auth.uid() = user_id or user is admin.
- Move email out of client-side fetch (drop from
.select()) and return via a secured server route or Supabase RPC.- (Optional) Keep the client-side user-ID check in app/tests/[id]/results/page.tsx as UX-level guard—but not in lieu of RLS/server checks.
♻️ Duplicate comments (1)
app/protected/layout.tsx (1)
111-115: Duplicate “Jobs” link to /protected/jobs — keep a single entry.There’s already “Job Opportunities” → /protected/jobs in Career. Remove this duplicate under Activities to reduce IA noise.
{ - title: "Jobs", - url: "/protected/jobs", - icon: Briefcase, - }, + // (removed duplicate "Jobs" entry; see Career > Job Opportunities) + },
🧹 Nitpick comments (24)
components/ai/AIChat.tsx (2)
185-187: Avoid widget flicker and handle route changes viausePathname.Reading
window.location.pathnamein an effect causes a one-frame flash on /ai and test-taking pages and won’t react to client-side route changes. UseusePathname()and simplify the condition.-import { useRouter } from 'next/navigation'; +import { useRouter, usePathname } from 'next/navigation'; @@ -// Don't show the floating widget on the dedicated AI page -const [currentPath, setCurrentPath] = useState(''); - -useEffect(() => { - setCurrentPath(window.location.pathname); -}, []); +// Current path for conditional rendering +const pathname = usePathname(); @@ -// Hide the widget if we're on the dedicated AI page or during test taking -if (currentPath === '/ai' || currentPath.includes('/tests/') && currentPath.includes('/take')) { +// Hide the widget on the AI page or during test-taking flows +if (pathname === '/ai' || (pathname.startsWith('/tests/') && pathname.includes('/take'))) { return null; }
185-187: Add parentheses for clarity around||/&&.Operator precedence is correct but subtle; parentheses improve readability.
-if (currentPath === '/ai' || currentPath.includes('/tests/') && currentPath.includes('/take')) { +if (currentPath === '/ai' || (currentPath.includes('/tests/') && currentPath.includes('/take'))) {app/protected/layout.tsx (1)
73-92: Verify intent: “Browse Tests” points to public/testsfrom a protected sidebar.If that’s deliberate, all good. If you meant the protected dashboard, consider labeling (“Browse Public Tests”) or routing to
/protected/testsinstead.app/protected/tests/page.tsx (2)
218-221: Handle zero/unknown max scores informatScore.Avoid “NaN%” and misleading denominators.
-const formatScore = (score: number, maxScore: number) => { - const percentage = Math.round((score / maxScore) * 100) - return `${score}/${maxScore} (${percentage}%)` -} +const formatScore = (score: number, maxScore: number) => { + if (!maxScore) return `${score} — N/A` + const percentage = Math.round((score / maxScore) * 100) + return `${score}/${maxScore} (${percentage}%)` +}
96-117: Fetch registrations and attempts in parallel to reduce TTFB.The two Supabase queries are independent; run them concurrently.
-// Fetch test registrations -const { data: registrationsData, error: registrationsError } = await supabase - .from('test_registrations') - .select(` - id, - test_id, - status, - registration_date, - tests!inner( - id, - name, - description, - duration_minutes, - passing_score, - max_attempts, - enable_leaderboard - ) - `) - .eq('user_id', user.id) - .order('registration_date', { ascending: false }) - -// Fetch test attempts -const { data: attemptsData, error: attemptsError } = await supabase - .from('test_attempts') - .select(` - id, - test_id, - score, - max_score, - time_taken_minutes, - passed, - status, - submitted_at, - tests!inner( - id, - name, - description, - passing_score, - enable_leaderboard - ) - `) - .eq('user_id', user.id) - .not('submitted_at', 'is', null) - .order('submitted_at', { ascending: false }) +const [ + { data: registrationsData, error: registrationsError }, + { data: attemptsData, error: attemptsError }, +] = await Promise.all([ + supabase + .from('test_registrations') + .select(` + id, + test_id, + status, + registration_date, + tests!inner( + id, + name, + description, + duration_minutes, + passing_score, + max_attempts, + enable_leaderboard + ) + `) + .eq('user_id', user.id) + .order('registration_date', { ascending: false }), + supabase + .from('test_attempts') + .select(` + id, + test_id, + score, + max_score, + time_taken_minutes, + passed, + status, + submitted_at, + tests!inner( + id, + name, + description, + passing_score, + enable_leaderboard + ) + `) + .eq('user_id', user.id) + .not('submitted_at', 'is', null) + .order('submitted_at', { ascending: false }), +])Also applies to: 119-142
components/dashboard/TestStatsWidget.tsx (6)
155-168: Use the fetched registrations or drop it.You fetch
totalRegistrationsbut don’t surface it. Either show it as a third quick stat or remove the query/state to cut cost.Option A (show it):
- <div className="grid grid-cols-2 gap-4"> + <div className="grid grid-cols-3 gap-4"> @@ </div> + <div className="text-center p-3 bg-amber-50 dark:bg-amber-900/20 rounded-lg"> + <div className="text-2xl font-bold text-amber-600 dark:text-amber-400"> + {stats.totalRegistrations} + </div> + <div className="text-sm text-amber-600 dark:text-amber-400">Registrations</div> + </div>Option B (remove it): I can provide a diff to strip the interface, state, query, and tile—say the word.
89-96: Avoid unsafe casts; type the attempts row.
(attempt.tests as { name?: string })hides shape issues. Define a local type and use it in mapping.// Above component type AttemptRow = { id: string score: number max_score: number time_taken_minutes: number | null passed: boolean submitted_at: string tests: { name: string } | null } // When using attemptsData: const rows = (attemptsData ?? []) as AttemptRow[] const recentAttempts = rows.map(a => ({ id: a.id, test_name: a.tests?.name ?? 'Unknown Test', score: a.score, max_score: a.max_score, passed: a.passed, submitted_at: a.submitted_at }))
201-205: Clarify failure status icon.Using a clock for failed attempts can be confusing; suggest an
XCirclefor failures.-import { - Target, - Trophy, - CheckCircle, - Clock, - BookOpen -} from 'lucide-react' +import { Target, Trophy, CheckCircle, XCircle, BookOpen } from 'lucide-react' @@ - ) : ( - <Clock className="h-4 w-4 text-red-500" /> - )} + ) : ( + <XCircle className="h-4 w-4 text-red-500" /> + )}Also applies to: 9-14
125-143: Loading UX/accessibility nits.Consider
aria-busyon the Card andaria-live="polite"on the skeleton wrapper to aid SRs.
56-61: Minor query optimization.If you only need the count, keep
head: true(good) and consider.select('id', { count: 'exact', head: true })to reflect intent; same cost, clearer.
62-79: Confirm RLS prevents cross-user reads.Client-side filtering by
user_idrelies on Row Level Security. Ensuretest_attemptsandtest_registrationshave policies likeusing (user_id = auth.uid()).Example (Postgres):
alter table test_attempts enable row level security; create policy "attempts are only visible to owner" on public.test_attempts for select using (user_id = auth.uid());app/tests/[id]/results/page.tsx (3)
401-401: Don’t default to a fake email.Passing
user@example.comcan mislead downstream flows. Either:
- derive from the authenticated user; or
- pass an empty value and let the generator handle it.
- email: attempt.profiles?.email || 'user@example.com', + email: attempt.profiles?.email || '',If you need the real email, fetch it once via
supabase.auth.getUser()and store it in state.
70-82: Reduce state churn: set attempt once after profile fetch.Two
setAttemptcalls cause extra renders. Populate once after the optional profile lookup.- setAttempt(attemptData) - - // Fetch user profile separately - if (attemptData?.user_id) { - const { data: profileData, error: profileError } = await supabase - .from('profiles') - .select('first_name, last_name, username') - .eq('id', attemptData.user_id) - .single() - - if (!profileError && profileData) { - // Add profile data to attempt object - setAttempt(prev => prev ? { ...prev, profiles: profileData } : prev) - } - } + let profiles + if (attemptData?.user_id) { + const { data: profileData } = await supabase + .from('profiles') + .select('first_name, last_name, username') + .eq('id', attemptData.user_id) + .single() + profiles = profileData ?? undefined + } + setAttempt(profiles ? { ...attemptData, profiles } : attemptData)
3-3: Memoize the Supabase client to avoid re-instantiation on re-renders.Small optimization to keep the client stable.
-import { useState, useEffect, useCallback } from "react" +import { useState, useEffect, useCallback, useMemo } from "react" @@ - const supabase = createClient() + const supabase = useMemo(() => createClient(), [])Also applies to: 34-34
app/api/tests/register/route.ts (2)
36-66: Profile bootstrap path: avoid trusting client email; derive from auth when possible.When creating a minimal profile, prefer
sessionUser.emailoveruserEmailfrom the request body.- email: userEmail, + email: userEmail ?? sessionUser?.email ?? null,
29-34: Return 409 for duplicates instead of 400.Semantically more accurate when a resource already exists.
- { status: 400 } + { status: 409 }hooks/useProfile.ts (2)
127-140: Abort stale requests and disable caching for fresher reads.Prevent setState on unmounted and avoid cached responses for profile pages.
- const fetchPublicProfile = async () => { + const fetchPublicProfile = async () => { if (!username) return try { setLoading(true) setError(null) - const response = await fetch(`/api/profile/${username}`) + const controller = new AbortController() + const response = await fetch(`/api/profile/${username}`, { + signal: controller.signal, + cache: 'no-store', + credentials: 'same-origin', + }) const data = await response.json() @@ - useEffect(() => { - if (username) { - fetchPublicProfile() - } else { - setProfile(null) - setLoading(false) - } - }, [username]) + useEffect(() => { + let aborted = false + ;(async () => { + if (username) { + await fetchPublicProfile() + } else { + if (!aborted) { + setProfile(null) + setLoading(false) + } + } + })() + return () => { aborted = true } + }, [username])Also applies to: 149-156
133-140: Runtime type guard for API response.Ensure
data.profilematchesProfilebefore setting state to avoid downstream undefined access.- setProfile(data.profile) + if (data && typeof data === 'object' && data.profile) { + setProfile(data.profile as Profile) + } else { + throw new Error('Malformed profile payload') + }app/api/profile/[username]/route.ts (1)
41-41: Optional: add cache headers for public data.Short TTL helps reduce load without staleness issues.
- return NextResponse.json({ profile }); + return NextResponse.json({ profile }, { headers: { 'Cache-Control': 'public, s-maxage=60, stale-while-revalidate=300' } });app/tests/[id]/leaderboard/page.tsx (3)
89-99: Deduplicate user IDs and guard empty IN() queries.Avoid redundant IDs and skip the profiles query when there are no participants.
-// Fetch user profiles for all participants -const userIds = leaderboardData?.map(entry => entry.user_id) || [] -const { data: profilesData, error: profilesError } = await supabase - .from('profiles') - .select('id, first_name, last_name, username, email') - .in('id', userIds) +// Fetch public profile basics for all unique participants +const userIds = Array.from(new Set((leaderboardData ?? []).map(e => e.user_id))) +let profilesData = null +let profilesError = null +if (userIds.length > 0) { + const res = await supabase + .from('profiles') + .select('id, first_name, last_name, username, is_public') + .in('id', userIds) + .eq('is_public', true) + profilesData = res.data + profilesError = res.error +}
96-102: Prefer a typed Map and initialize in one pass.Minor readability improvement.
-// Create a map of user_id to profile data -const profilesMap = new Map() +// Create a map of user_id -> public profile basics +const profilesMap = new Map<string, { id: string; first_name?: string | null; last_name?: string | null; username?: string | null }>() if (!profilesError && profilesData) { - profilesData.forEach(profile => { - profilesMap.set(profile.id, profile) - }) + profilesData.forEach((profile) => profilesMap.set(profile.id, profile)) }
260-316: UI copy: reflect anonymized email.Since emails are hidden, consider renaming the subtext label to “Profile” or removing the second line to avoid implying an email will be shown.
components/users/PublicProfileView.tsx (1)
3-3: Remove unused React import.Not needed with the new JSX runtime.
-import React from 'react'app/tests/page.tsx (1)
343-377: Consider removing debug console logs before production.The extensive console logging in
getRegistrationStatusprovides good debugging information but should be cleaned up for production deployment.Apply this diff to remove the debug logs:
- console.log('🔍 Registration status check for test:', test.name); - console.log(' - Now:', now.toISOString()); - console.log(' - Reg start:', regStart?.toISOString()); - console.log(' - Reg end:', regEnd?.toISOString()); - console.log(' - User registered:', userRegistrations.has(test.id)); - // Check if user is registered for this test if (userRegistrations.has(test.id)) { - console.log(' - Status: registered'); return { status: 'registered', badge: <Badge variant="default" className="pointer-events-none bg-blue-500/10 text-blue-500 border-blue-500/20">Registered</Badge> }; } // Check registration dates if (regStart && now < regStart) { - console.log(' - Status: pending'); return { status: 'pending', badge: <Badge variant="outline" className="pointer-events-none bg-yellow-500/10 text-yellow-500 border-yellow-500/20">Registration Pending</Badge>, message: `Registration starts ${regStart.toLocaleDateString()} at ${regStart.toLocaleTimeString()}` }; } else if (regEnd && now > regEnd) { - console.log(' - Status: closed'); return { status: 'closed', badge: <Badge variant="destructive" className="pointer-events-none bg-red-500/10 text-red-500 border-red-500/20">Registration Closed</Badge>, message: `Registration ended ${regEnd.toLocaleDateString()} at ${regEnd.toLocaleTimeString()}` }; } else { - console.log(' - Status: open'); return { status: 'open', badge: <Badge variant="default" className="pointer-events-none bg-green-500/10 text-green-500 border-green-500/20">Registration Open</Badge>, message: regEnd ? `Registration ends ${regEnd.toLocaleDateString()} at ${regEnd.toLocaleTimeString()}` : 'Registration open' }; }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (15)
app/api/debug/profile/[username]/route.ts(0 hunks)app/api/profile/[username]/route.ts(1 hunks)app/api/tests/register/route.ts(1 hunks)app/protected/layout.tsx(2 hunks)app/protected/tests/page.tsx(1 hunks)app/tests/[id]/leaderboard/page.tsx(1 hunks)app/tests/[id]/page.tsx(2 hunks)app/tests/[id]/results/page.tsx(3 hunks)app/tests/page.tsx(2 hunks)components/ai/AIChat.tsx(1 hunks)components/dashboard/DashboardContent.tsx(1 hunks)components/dashboard/TestStatsWidget.tsx(1 hunks)components/users/PublicProfileView.tsx(2 hunks)hooks/useProfile.ts(1 hunks)lib/services/profile.ts(0 hunks)
💤 Files with no reviewable changes (2)
- app/api/debug/profile/[username]/route.ts
- lib/services/profile.ts
✅ Files skipped from review due to trivial changes (1)
- components/dashboard/DashboardContent.tsx
🧰 Additional context used
🧬 Code graph analysis (4)
app/api/tests/register/route.ts (1)
lib/supabase/server.ts (1)
createServiceClient(43-45)
app/tests/page.tsx (2)
lib/services/profile.ts (1)
getSupabaseClient(5-7)components/ui/badge.tsx (1)
Badge(36-36)
app/protected/tests/page.tsx (5)
components/ui/badge.tsx (1)
Badge(36-36)components/ui/card.tsx (5)
Card(77-77)CardContent(82-82)CardHeader(78-78)CardTitle(80-80)CardDescription(81-81)components/ui/tabs.tsx (4)
Tabs(55-55)TabsList(55-55)TabsTrigger(55-55)TabsContent(55-55)components/ui/button.tsx (1)
Button(59-59)lib/utils.ts (1)
cn(4-6)
components/users/PublicProfileView.tsx (1)
hooks/useProfile.ts (1)
usePublicProfileByUsername(122-164)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: Security & Code Quality
🔇 Additional comments (11)
app/api/tests/register/route.ts (1)
21-35: Authenticate and authorize registration endpoint
– In app/api/tests/register/route.ts, usenext/headersand Supabase’screateServerClientto fetch the session user and return 401 ifsessionUser.id !== userId.
– Ensure a unique DB constraint on(test_id, user_id)intest_registrationsand handle the23505conflict by returning a 409.components/users/PublicProfileView.tsx (2)
36-36: LGTM: hook adoption simplifies data flow.Switching to
usePublicProfileByUsernamecentralizes fetching/err handling and aligns with the new API route.
45-45: Double-check: activity graph gating remains owner-only.
isOwnProfilestill protects the contribution graph. Good; ensure the API behind it also enforces auth/ownership.Would you like a quick grep script to confirm no other surfaces display private profile fields to unauthenticated users?
app/tests/[id]/page.tsx (4)
158-170: Good duplicate prevention logic added.The pre-insert duplicate check is well-implemented and prevents race conditions by checking for existing registrations before attempting to register.
179-189: Enhanced registration data persistence with comprehensive fields.The registration now stores all relevant user information including attempt_count, registration_date, and optional fields like phone, institution, etc. This provides good data tracking capabilities.
191-194: Improved error handling with descriptive logging.The enhanced error handling provides better debugging information while maintaining the existing error flow.
212-215: Consistent error handling pattern applied.The catch block now uses typed error parameters and provides user-friendly error messages, which improves the overall error handling consistency.
app/tests/page.tsx (4)
163-175: Well-implemented two-step registration flow with database pre-check.The addition of a database pre-check before API registration prevents duplicate registrations and provides immediate UI feedback. The local state synchronization when duplicates are found is a good UX improvement.
177-200: Robust API integration with proper error handling.The API call to
/api/tests/registeris well-structured with proper error handling and response validation. The state updates and UI refresh after successful registration maintain data consistency.
201-204: Enhanced error logging and user feedback.The improved error handling provides comprehensive logging while maintaining user-friendly error messages.
177-189: Resolved:/api/tests/registerendpoint exists and correctly handlestestId,userId,userEmail, anduserMetadata.
| export async function GET( | ||
| request: NextRequest, | ||
| { params }: { params: Promise<{ username: string }> } | ||
| ) { | ||
| try { | ||
| const { username } = await params; | ||
|
|
There was a problem hiding this comment.
🛠️ Refactor suggestion
Fix Next.js route handler params typing (remove Promise/await).
params is not a Promise in App Router route handlers. Typing it as a Promise and awaiting it is incorrect and unnecessary.
Apply:
-export async function GET(
- request: NextRequest,
- { params }: { params: Promise<{ username: string }> }
-) {
+export async function GET(
+ request: NextRequest,
+ { params }: { params: { username: string } }
+) {
try {
- const { username } = await params;
+ const { username } = params;📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| export async function GET( | |
| request: NextRequest, | |
| { params }: { params: Promise<{ username: string }> } | |
| ) { | |
| try { | |
| const { username } = await params; | |
| export async function GET( | |
| request: NextRequest, | |
| { params }: { params: { username: string } } | |
| ) { | |
| try { | |
| const { username } = params; | |
| // …rest of your handler… |
🤖 Prompt for AI Agents
In app/api/profile/[username]/route.ts around lines 4 to 10, the handler types
`params` as a Promise and awaits it which is incorrect for Next.js App Router;
change the function signature to accept params as a plain object (e.g. { params:
{ username: string } }) and remove the await when destructuring so you directly
read const { username } = params; keep the rest of the handler logic unchanged.
| if (!username) { | ||
| return NextResponse.json( | ||
| { error: 'Username is required' }, | ||
| { status: 400 } | ||
| ); | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Add basic username validation (length/charset) to harden the endpoint.
Guard early to avoid noisy queries and enforce your username contract.
Example patch:
if (!username) {
return NextResponse.json(
{ error: 'Username is required' },
{ status: 400 }
);
}
+ // Basic validation: 3–30 chars, alnum/underscore/dot
+ if (!/^[a-zA-Z0-9._]{3,30}$/.test(username)) {
+ return NextResponse.json(
+ { error: 'Invalid username format' },
+ { status: 400 }
+ );
+ }📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| if (!username) { | |
| return NextResponse.json( | |
| { error: 'Username is required' }, | |
| { status: 400 } | |
| ); | |
| } | |
| if (!username) { | |
| return NextResponse.json( | |
| { error: 'Username is required' }, | |
| { status: 400 } | |
| ); | |
| } | |
| // Basic validation: 3–30 chars, alnum/underscore/dot | |
| if (!/^[a-zA-Z0-9._]{3,30}$/.test(username)) { | |
| return NextResponse.json( | |
| { error: 'Invalid username format' }, | |
| { status: 400 } | |
| ); | |
| } |
🤖 Prompt for AI Agents
In app/api/profile/[username]/route.ts around lines 11 to 16, the handler
currently only checks presence of username; add basic validation to reject
usernames that don't meet length and charset rules (e.g. trim input, require
length between 3 and 30 characters and only allow a defined charset such as
lowercase/uppercase letters, digits, underscores and hyphens or your project's
chosen pattern). Implement a simple regex check against that charset and length,
returning NextResponse.json({ error: 'Invalid username' }, { status: 400 }) for
invalid values so the endpoint bails early before running queries.
| const { data: profile, error } = await supabase | ||
| .from('profiles') | ||
| .select('*') | ||
| .eq('username', username) | ||
| .eq('is_public', true) | ||
| .single(); | ||
|
|
||
| if (error) { | ||
| if (error.code === 'PGRST116') { | ||
| return NextResponse.json( | ||
| { error: 'Profile not found or not public' }, | ||
| { status: 404 } | ||
| ); | ||
| } | ||
| console.error('Error fetching public profile:', error); | ||
| return NextResponse.json( | ||
| { error: 'Failed to fetch profile' }, | ||
| { status: 500 } | ||
| ); | ||
| } | ||
|
|
||
| return NextResponse.json({ profile }); |
There was a problem hiding this comment.
Avoid PII leakage: don’t select '*' on a public endpoint; whitelist safe public fields.
This route is publicly accessible yet returns the whole row (including email/phone if present). That leaks PII regardless of the UI’s selective rendering. Only return a curated set of public fields.
Apply:
- const { data: profile, error } = await supabase
- .from('profiles')
- .select('*')
- .eq('username', username)
- .eq('is_public', true)
- .single();
+ const { data: profile, error } = await supabase
+ .from('profiles')
+ .select(`
+ id,
+ username,
+ first_name,
+ last_name,
+ bio,
+ location,
+ current_position,
+ company,
+ github_url,
+ linkedin_url,
+ twitter_url,
+ skills,
+ is_public
+ `)
+ .eq('username', username)
+ .eq('is_public', true)
+ .maybeSingle();And simplify not-found handling:
- if (error) {
- if (error.code === 'PGRST116') {
- return NextResponse.json(
- { error: 'Profile not found or not public' },
- { status: 404 }
- );
- }
- console.error('Error fetching public profile:', error);
- return NextResponse.json(
- { error: 'Failed to fetch profile' },
- { status: 500 }
- );
- }
+ if (error) {
+ console.error('Error fetching public profile:', error);
+ return NextResponse.json({ error: 'Failed to fetch profile' }, { status: 500 });
+ }
+ if (!profile) {
+ return NextResponse.json({ error: 'Profile not found or not public' }, { status: 404 });
+ }📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| const { data: profile, error } = await supabase | |
| .from('profiles') | |
| .select('*') | |
| .eq('username', username) | |
| .eq('is_public', true) | |
| .single(); | |
| if (error) { | |
| if (error.code === 'PGRST116') { | |
| return NextResponse.json( | |
| { error: 'Profile not found or not public' }, | |
| { status: 404 } | |
| ); | |
| } | |
| console.error('Error fetching public profile:', error); | |
| return NextResponse.json( | |
| { error: 'Failed to fetch profile' }, | |
| { status: 500 } | |
| ); | |
| } | |
| return NextResponse.json({ profile }); | |
| // Fetch only safe public fields for the given username | |
| const { data: profile, error } = await supabase | |
| .from('profiles') | |
| .select(` | |
| id, | |
| username, | |
| first_name, | |
| last_name, | |
| bio, | |
| location, | |
| current_position, | |
| company, | |
| github_url, | |
| linkedin_url, | |
| twitter_url, | |
| skills, | |
| is_public | |
| `) | |
| .eq('username', username) | |
| .eq('is_public', true) | |
| .maybeSingle(); | |
| // Handle real errors | |
| if (error) { | |
| console.error('Error fetching public profile:', error); | |
| return NextResponse.json( | |
| { error: 'Failed to fetch profile' }, | |
| { status: 500 } | |
| ); | |
| } | |
| // Handle "not found or not public" | |
| if (!profile) { | |
| return NextResponse.json( | |
| { error: 'Profile not found or not public' }, | |
| { status: 404 } | |
| ); | |
| } | |
| return NextResponse.json({ profile }); |
| export async function POST(request: NextRequest) { | ||
| try { | ||
| const { testId, userId, userEmail, userMetadata } = await request.json() | ||
|
|
||
| if (!testId || !userId) { | ||
| return NextResponse.json( | ||
| { error: 'Test ID and User ID are required' }, | ||
| { status: 400 } | ||
| ) | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Validate input with a schema and return precise status codes.
Add a Zod (or similar) schema to validate testId, userId, userEmail, userMetadata. Use 422 for validation errors.
+import { z } from 'zod'
+
export async function POST(request: NextRequest) {
try {
- const { testId, userId, userEmail, userMetadata } = await request.json()
+ const body = await request.json()
+ const { testId, userId, userEmail, userMetadata } = z.object({
+ testId: z.string().min(1),
+ userId: z.string().min(1),
+ userEmail: z.string().email().optional(),
+ userMetadata: z.record(z.any()).optional(),
+ }).parse(body)
- if (!testId || !userId) {
- return NextResponse.json(
- { error: 'Test ID and User ID are required' },
- { status: 400 }
- )
- }
+ // continue...📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| export async function POST(request: NextRequest) { | |
| try { | |
| const { testId, userId, userEmail, userMetadata } = await request.json() | |
| if (!testId || !userId) { | |
| return NextResponse.json( | |
| { error: 'Test ID and User ID are required' }, | |
| { status: 400 } | |
| ) | |
| } | |
| import { z } from 'zod' | |
| export async function POST(request: NextRequest) { | |
| try { | |
| const body = await request.json() | |
| const { testId, userId, userEmail, userMetadata } = z.object({ | |
| testId: z.string().min(1), | |
| userId: z.string().min(1), | |
| userEmail: z.string().email().optional(), | |
| userMetadata: z.record(z.any()).optional(), | |
| }).parse(body) | |
| // continue... |
🤖 Prompt for AI Agents
In app/api/tests/register/route.ts around lines 4 to 13, the endpoint currently
performs ad-hoc checks for testId and userId; replace that with a Zod schema
validating testId, userId, userEmail, and userMetadata (types and any
required/optional rules), parse the incoming JSON using schema.safeParse (or
parseAsync) and, on validation failure, return NextResponse.json with the
validation errors and status 422; on success, use the validated values from the
schema result for the rest of the handler.
| // Use service role client to bypass RLS | ||
| const serviceSupabase = createServiceClient( | ||
| process.env.NEXT_PUBLIC_SUPABASE_URL!, | ||
| process.env.SUPABASE_SERVICE_ROLE_KEY! | ||
| ) | ||
|
|
There was a problem hiding this comment.
🛠️ Refactor suggestion
Guard env vars early; fail fast if misconfigured.
Non-null assertions won’t protect at runtime.
- const serviceSupabase = createServiceClient(
- process.env.NEXT_PUBLIC_SUPABASE_URL!,
- process.env.SUPABASE_SERVICE_ROLE_KEY!
- )
+ const SUPABASE_URL = process.env.NEXT_PUBLIC_SUPABASE_URL
+ const SERVICE_KEY = process.env.SUPABASE_SERVICE_ROLE_KEY
+ if (!SUPABASE_URL || !SERVICE_KEY) {
+ console.error('Supabase env vars missing')
+ return NextResponse.json({ error: 'Server misconfiguration' }, { status: 500 })
+ }
+ const serviceSupabase = createServiceClient(SUPABASE_URL, SERVICE_KEY)📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| // Use service role client to bypass RLS | |
| const serviceSupabase = createServiceClient( | |
| process.env.NEXT_PUBLIC_SUPABASE_URL!, | |
| process.env.SUPABASE_SERVICE_ROLE_KEY! | |
| ) | |
| // Use service role client to bypass RLS | |
| const SUPABASE_URL = process.env.NEXT_PUBLIC_SUPABASE_URL | |
| const SERVICE_KEY = process.env.SUPABASE_SERVICE_ROLE_KEY | |
| if (!SUPABASE_URL || !SERVICE_KEY) { | |
| console.error('Supabase env vars missing') | |
| return NextResponse.json({ error: 'Server misconfiguration' }, { status: 500 }) | |
| } | |
| const serviceSupabase = createServiceClient(SUPABASE_URL, SERVICE_KEY) |
🤖 Prompt for AI Agents
In app/api/tests/register/route.ts around lines 15 to 20, the code uses non-null
assertions on NEXT_PUBLIC_SUPABASE_URL and SUPABASE_SERVICE_ROLE_KEY which do
not protect at runtime; add an explicit guard at the top of this module or just
before createServiceClient that checks both environment variables and throws a
clear Error if either is missing (e.g. “Missing SUPABASE env var:
NEXT_PUBLIC_SUPABASE_URL or SUPABASE_SERVICE_ROLE_KEY”), and then call
createServiceClient without non-null assertions so the function receives
validated strings.
| // Process leaderboard data with proper user names | ||
| const processedLeaderboard = (leaderboardData || []).map((entry, index) => { | ||
| const profile = profilesMap.get(entry.user_id) | ||
| const displayName = profile?.username || | ||
| (profile?.first_name && profile?.last_name ? | ||
| `${profile.first_name} ${profile.last_name}` : | ||
| `User ${entry.user_id.slice(0, 8)}...`) | ||
|
|
||
| return { | ||
| id: entry.id, | ||
| user_id: entry.user_id, | ||
| user_email: profile?.email || `User ${entry.user_id.slice(0, 8)}...`, | ||
| user_name: displayName, | ||
| score: entry.score, | ||
| max_score: entry.max_score, | ||
| time_taken_minutes: entry.time_taken_minutes, | ||
| passed: entry.passed, | ||
| submitted_at: entry.submitted_at, | ||
| rank: index + 1 | ||
| } | ||
| }) |
There was a problem hiding this comment.
Do not expose user emails on the public leaderboard; show names only for public profiles.
Currently you render profile.email for every entry. That discloses PII and also leaks info for non-public profiles. Only use public-facing fields and anonymize otherwise.
-const processedLeaderboard = (leaderboardData || []).map((entry, index) => {
+const processedLeaderboard = (leaderboardData || []).map((entry, index) => {
const profile = profilesMap.get(entry.user_id)
const displayName = profile?.username ||
(profile?.first_name && profile?.last_name ?
`${profile.first_name} ${profile.last_name}` :
`User ${entry.user_id.slice(0, 8)}...`)
return {
id: entry.id,
user_id: entry.user_id,
- user_email: profile?.email || `User ${entry.user_id.slice(0, 8)}...`,
+ // Hide email on public leaderboard
+ user_email: '—',
user_name: displayName,
score: entry.score,
max_score: entry.max_score,
time_taken_minutes: entry.time_taken_minutes,
passed: entry.passed,
submitted_at: entry.submitted_at,
rank: index + 1
}
})📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| // Process leaderboard data with proper user names | |
| const processedLeaderboard = (leaderboardData || []).map((entry, index) => { | |
| const profile = profilesMap.get(entry.user_id) | |
| const displayName = profile?.username || | |
| (profile?.first_name && profile?.last_name ? | |
| `${profile.first_name} ${profile.last_name}` : | |
| `User ${entry.user_id.slice(0, 8)}...`) | |
| return { | |
| id: entry.id, | |
| user_id: entry.user_id, | |
| user_email: profile?.email || `User ${entry.user_id.slice(0, 8)}...`, | |
| user_name: displayName, | |
| score: entry.score, | |
| max_score: entry.max_score, | |
| time_taken_minutes: entry.time_taken_minutes, | |
| passed: entry.passed, | |
| submitted_at: entry.submitted_at, | |
| rank: index + 1 | |
| } | |
| }) | |
| // Process leaderboard data with proper user names | |
| const processedLeaderboard = (leaderboardData || []).map((entry, index) => { | |
| const profile = profilesMap.get(entry.user_id) | |
| const displayName = profile?.username || | |
| (profile?.first_name && profile?.last_name ? | |
| `${profile.first_name} ${profile.last_name}` : | |
| `User ${entry.user_id.slice(0, 8)}...`) | |
| return { | |
| id: entry.id, | |
| user_id: entry.user_id, | |
| // Hide email on public leaderboard | |
| user_email: '—', | |
| user_name: displayName, | |
| score: entry.score, | |
| max_score: entry.max_score, | |
| time_taken_minutes: entry.time_taken_minutes, | |
| passed: entry.passed, | |
| submitted_at: entry.submitted_at, | |
| rank: index + 1 | |
| } | |
| }) |
| name: attempt.profiles?.username || | ||
| (attempt.profiles?.first_name && attempt.profiles?.last_name ? | ||
| `${attempt.profiles.first_name} ${attempt.profiles.last_name}` : | ||
| `User ${attempt.user_id?.slice(0, 8)}`), | ||
| score: attempt.score, | ||
| testName: test.name, | ||
| cert_id: `CU-TEST-${test.id}-${attempt.user_id}`, | ||
| email: 'user@example.com', | ||
| email: attempt.profiles?.email || 'user@example.com', |
There was a problem hiding this comment.
Fix “User undefined” fallback and avoid unstable string interpolation.
If attempt.user_id is falsy, the current template literal renders “User undefined”. Use explicit nullish checks.
- name: attempt.profiles?.username ||
- (attempt.profiles?.first_name && attempt.profiles?.last_name ?
- `${attempt.profiles.first_name} ${attempt.profiles.last_name}` :
- `User ${attempt.user_id?.slice(0, 8)}`),
+ name:
+ attempt.profiles?.username ??
+ (attempt.profiles?.first_name && attempt.profiles?.last_name
+ ? `${attempt.profiles.first_name} ${attempt.profiles.last_name}`
+ : (attempt.user_id ? `User ${attempt.user_id.slice(0, 8)}` : 'User')),📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| name: attempt.profiles?.username || | |
| (attempt.profiles?.first_name && attempt.profiles?.last_name ? | |
| `${attempt.profiles.first_name} ${attempt.profiles.last_name}` : | |
| `User ${attempt.user_id?.slice(0, 8)}`), | |
| score: attempt.score, | |
| testName: test.name, | |
| cert_id: `CU-TEST-${test.id}-${attempt.user_id}`, | |
| email: 'user@example.com', | |
| email: attempt.profiles?.email || 'user@example.com', | |
| name: | |
| attempt.profiles?.username ?? | |
| (attempt.profiles?.first_name && attempt.profiles?.last_name | |
| ? `${attempt.profiles.first_name} ${attempt.profiles.last_name}` | |
| : (attempt.user_id ? `User ${attempt.user_id.slice(0, 8)}` : 'User')), | |
| score: attempt.score, | |
| testName: test.name, | |
| cert_id: `CU-TEST-${test.id}-${attempt.user_id}`, | |
| email: attempt.profiles?.email || 'user@example.com', |
🤖 Prompt for AI Agents
In app/tests/[id]/results/page.tsx around lines 394 to 401, the fallback uses
template interpolation with a possibly falsy attempt.user_id which can produce
"User undefined"; change the logic to explicitly check for a
non-null/non-undefined user_id before slicing or interpolating (e.g. if
attempt.user_id != null then use attempt.user_id.slice(0,8) in the "User ..."
fallback, otherwise use a stable literal like "User unknown"); apply the same
nullish check when building cert_id so you don't interpolate undefined into
`CU-TEST-...` (use a safe placeholder or omit the user part when user_id is
null/undefined).
| cert_id: `CU-TEST-${test.id}-${attempt.user_id}`, | ||
| email: 'user@example.com', | ||
| email: attempt.profiles?.email || 'user@example.com', |
There was a problem hiding this comment.
💡 Verification agent
🧩 Analysis chain
Avoid exposing raw user_id in certificate IDs.
Embedding attempt.user_id in cert_id leaks internal IDs to publicly shareable artifacts. Prefer attempt.id, a random UUID, or a hash.
- cert_id: `CU-TEST-${test.id}-${attempt.user_id}`,
+ cert_id: `CU-TEST-${test.id}-${attempt.id}`,If you need stability across regenerations, use a server-generated opaque token.
🏁 Script executed:
#!/bin/bash
# Locate and inspect the public results page under app/tests/[id]/results/page.tsx
file=$(fd -e tsx "page.tsx" app/tests)
echo "Page file: $file"
# Show context around cert_id usage
sed -n '350,420p' "$file"
# Check for attempt.id vs attempt.user_id in this file
echo "Occurrences of attempt.id:"
grep -n "attempt.id" "$file" || echo "none"
echo "Occurrences of attempt.user_id:"
grep -n "attempt.user_id" "$file" || echo "none"Length of output: 748
🏁 Script executed:
#!/bin/bash
# Inspect lines 360–450 in app/tests/[id]/results/page.tsx
file='app/tests/[id]/results/page.tsx'
if [ -f "$file" ]; then
echo "----- Context around cert_id usage -----"
sed -n '360,450p' "$file"
echo
echo "----- cert_id occurrences -----"
grep -n "cert_id" "$file"
echo
echo "----- attempt.id occurrences -----"
grep -n "attempt\.id" "$file" || echo "none"
echo
echo "----- attempt.user_id occurrences -----"
grep -n "attempt\.user_id" "$file" || echo "none"
else
echo "File not found: $file"
fiLength of output: 3691
Replace raw user ID in certificate identifier
Use the attempt record’s primary ID instead of user_id to avoid exposing internal user IDs:
- cert_id: `CU-TEST-${test.id}-${attempt.user_id}`,
+ cert_id: `CU-TEST-${test.id}-${attempt.id}`,If you need consistent but opaque certificate tokens across regenerations, generate them server-side.
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| cert_id: `CU-TEST-${test.id}-${attempt.user_id}`, | |
| email: 'user@example.com', | |
| email: attempt.profiles?.email || 'user@example.com', | |
| cert_id: `CU-TEST-${test.id}-${attempt.id}`, | |
| email: attempt.profiles?.email || 'user@example.com', |
🤖 Prompt for AI Agents
In app/tests/[id]/results/page.tsx around lines 400 to 401, the certificate
identifier currently uses the raw attempt.user_id which exposes internal user
IDs; replace that with the attempt record’s primary id (e.g., attempt.id) when
building cert_id to avoid leaking user identifiers, and if you need a stable but
opaque token across regenerations move generation to the server (create and
persist a certificate_token) rather than deriving it from user_id.
| @@ -0,0 +1,245 @@ | |||
| 'use client' | |||
|
|
|||
| import React, { useState, useEffect } from 'react' | |||
There was a problem hiding this comment.
🛠️ Refactor suggestion
Stop infinite re-fetch; memoize Supabase client and add effect cleanup. Also surface registrations query errors.
createClient() is called on each render and included in the effect deps, causing the effect to re-run on every render (re-fetch loop). Memoize the client and guard setState on unmount. Also check the registrations query error.
-import React, { useState, useEffect } from 'react'
+import React, { useState, useEffect, useMemo } from 'react'
@@
- const supabase = createClient()
+ const supabase = useMemo(() => createClient(), [])
@@
- useEffect(() => {
- const fetchTestStats = async () => {
- try {
- setLoading(true)
+ useEffect(() => {
+ let cancelled = false
+ const fetchTestStats = async () => {
+ try {
+ setLoading(true)
@@
- const { count: registrationsCount } = await supabase
+ const { count: registrationsCount, error: registrationsError } = await supabase
.from('test_registrations')
.select('*', { count: 'exact', head: true })
.eq('user_id', userId)
+ if (registrationsError) throw registrationsError
@@
- setStats({
+ if (cancelled) return
+ setStats({
totalRegistrations: registrationsCount || 0,
totalAttempts,
passedTests,
averageScore,
totalTimeSpent,
recentAttempts
})
@@
- } finally {
- setLoading(false)
- }
+ } finally {
+ if (!cancelled) setLoading(false)
+ }
}
if (userId) {
fetchTestStats()
}
- }, [userId, supabase])
+ return () => {
+ cancelled = true
+ }
+ }, [userId, supabase])Also applies to: 48-49, 50-117
🤖 Prompt for AI Agents
In components/dashboard/TestStatsWidget.tsx around lines 3 and also covering
48-49 and 50-117, the Supabase client is being recreated on every render and
included in the useEffect deps which triggers an infinite re-fetch loop; memoize
the client (use useMemo or a module-level singleton or useRef) so the same
instance is reused across renders, remove the unstable client from the effect
dependency list (use the memoized/stable reference instead), add an
isMounted/aborted flag in the effect cleanup to guard state updates after
unmount, and explicitly check the registrations query result for errors—log or
surface the error and avoid calling setState when the query returned an error.
| const totalAttempts = attemptsData?.length || 0 | ||
| const passedTests = attemptsData?.filter(attempt => attempt.passed).length || 0 | ||
| const averageScore = totalAttempts > 0 | ||
| ? Math.round(attemptsData.reduce((sum, attempt) => sum + (attempt.score / attempt.max_score * 100), 0) / totalAttempts) | ||
| : 0 | ||
| const totalTimeSpent = attemptsData?.reduce((sum, attempt) => sum + (attempt.time_taken_minutes || 0), 0) || 0 | ||
|
|
There was a problem hiding this comment.
🛠️ Refactor suggestion
Guard divide-by-zero/NaN when computing percentages.
If any attempt has max_score 0/null, the math yields NaN/Infinity.
- const averageScore = totalAttempts > 0
- ? Math.round(attemptsData.reduce((sum, attempt) => sum + (attempt.score / attempt.max_score * 100), 0) / totalAttempts)
- : 0
+ const averageScore = totalAttempts > 0
+ ? Math.round(
+ attemptsData.reduce((sum, a) => {
+ const max = Number(a.max_score) || 0
+ const sc = Number(a.score) || 0
+ return sum + (max > 0 ? (sc / max) * 100 : 0)
+ }, 0) / totalAttempts
+ )
+ : 0
@@
- const formatScore = (score: number, maxScore: number) => {
- const percentage = Math.round((score / maxScore) * 100)
- return `${percentage}%`
- }
+ const formatScore = (score: number, maxScore: number) => {
+ if (!maxScore || maxScore <= 0) return '0%'
+ const percentage = Math.round((score / maxScore) * 100)
+ return `${percentage}%`
+ }Also applies to: 120-123
🤖 Prompt for AI Agents
In components/dashboard/TestStatsWidget.tsx around lines 82-88 (and similarly at
120-123), the average/percentage calculations can produce NaN/Infinity if an
attempt has max_score 0/null; update the reduce logic to skip or treat such
attempts safely and compute the average using a validAttempts count: when
reducing, only add (attempt.score / attempt.max_score * 100) when
Number.isFinite(attempt.max_score) && attempt.max_score > 0 (or treat
max_score<=0 as zero contribution), track the number of valid attempts in the
same pass, then divide the total percentage sum by validAttempts (falling back
to 0 if validAttempts is 0); ensure any per-attempt ratio uses a conditional
(max_score ? score/max_score : 0) so no division by zero occurs.
🎯 Overview
This PR transforms the Projects section into a prestigious 'Zenith Hall' showcasing community excellence, while also fixing favicon issues, improving overall branding, and addressing all code review feedback.
✨ Key Features
🏛️ Zenith Hall Transformation
💼 Protected Jobs Dashboard
🎨 Enhanced User Experience
🔧 Technical Improvements
🛠️ Code Review Fixes
♿ Accessibility Improvements
📁 File Changes
app/projects/→app/zenith-hall/(renamed directory)app/protected/jobs/(new protected jobs page)app/layout.tsxwith favicon configurationcomponents/header.tsxnavigationlib/seo/metadata.tswith icon supportlib/security/input-validation.tsnext.config.tswith redirects🧪 Testing
🎯 Impact
📸 Preview
/zenith-hallto see the new prestigious hall of excellence/protected/jobsto see the student jobs dashboard/projectslinks automatically redirect to/zenith-hallReady for review and merge! 🚀
Summary by CodeRabbit
New Features
Style
Accessibility
Chores