Conversation
❌ Deploy Preview for develop-devlovers failed.
|
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
|
Important Review skippedReview was skipped due to path filters ⛔ Files ignored due to path filters (1)
CodeRabbit blocks several paths by default. You can override this behavior by explicitly including those paths in the path filters. For example, including You can disable this status message by setting the Use the checkbox below for a quick retry:
📝 WalkthroughWalkthroughThis PR introduces an achievements system recognizing sponsors and stargazers on the leaderboard and dashboard, adds a new feedback API endpoint with email submission and file attachment support, implements in-page scroll navigation for dashboard stats, and refactors card styling. It also fetches GitHub stargazer data to identify achievement eligibility. Changes
Sequence Diagram(s)sequenceDiagram
participant Client as Frontend Client
participant LBPage as Leaderboard Page
participant API as Backend/DB
participant GitHub as GitHub API
Client->>LBPage: Load leaderboard
LBPage->>API: getLeaderboardData()
Note over API: Fetch users from DB
API->>API: computeAchievements()<br/>per user stats
LBPage->>GitHub: getAllStargazers()
Note over GitHub: Fetch stargazer list
GitHub-->>LBPage: [stargazers]
LBPage->>LBPage: Match sponsors &<br/>stargazers to users
LBPage->>LBPage: Inject achievements<br/>into user objects
API-->>LBPage: [users + achievements]
LBPage->>Client: Render LeaderboardTable<br/>+ AchievementPips
Client->>Client: Display sponsor badges<br/>& achievement icons
sequenceDiagram
participant User as User
participant Form as Feedback Form
participant API as /api/feedback<br/>Endpoint
participant Gmail as Gmail SMTP
User->>Form: Fill form + attach files
User->>Form: Submit
Form->>Form: Validate fields &<br/>file sizes
Form->>API: POST (multipart data)
API->>API: Parse form data<br/>& validate files
API->>Gmail: Send HTML email<br/>with attachments
Gmail-->>API: Email sent
API-->>Form: { success: true }
Form->>User: Clear form &<br/>show success
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
Suggested labels
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 1 | ❌ 2❌ Failed checks (1 warning, 1 inconclusive)
✅ Passed checks (1 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. 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: 7
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (3)
frontend/components/dashboard/ActivityHeatmapCard.tsx (1)
471-474:⚠️ Potential issue | 🟡 MinorTooltip strings are hardcoded English — not i18n'd.
'No activity','attempt', and'attempts'bypass thet()hook. All other visible strings in this component use translations.♻️ Suggested fix (add keys to translation file and use t())
- {tooltip.count === 0 - ? 'No activity' - : `${tooltip.count} ${tooltip.count === 1 ? 'attempt' : 'attempts'}`} + {tooltip.count === 0 + ? t('noActivity') + : t('attemptsCount', { count: tooltip.count })}🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@frontend/components/dashboard/ActivityHeatmapCard.tsx` around lines 471 - 474, The tooltip text in ActivityHeatmapCard is hardcoded — replace 'No activity', 'attempt', and 'attempts' with translation keys via the t() hook used elsewhere in the component (e.g., use t('activityHeatmap.noActivity'), t('activityHeatmap.attempt_singular'), t('activityHeatmap.attempt_plural') or a pluralized t call) and add corresponding entries to the translations file; update the rendering logic that currently uses tooltip.count to call t(...) with the correct key or pluralization so all displayed strings in ActivityHeatmapCard are localized.frontend/components/dashboard/FeedbackForm.tsx (1)
162-172:⚠️ Potential issue | 🟡 MinorRemove unnecessary validation handlers from the
readOnlyemail field.The email field is
readOnlyand cannot be edited by the user, so therequiredattribute,onInvalid, andonInputhandlers are redundant. Since the database enforces the email field as non-nullable, the field will never be empty. Simplify by removing the unnecessary validation logic:Suggested change
<input name="email" type="email" placeholder={t('email')} defaultValue={userEmail ?? ''} - required readOnly - onInvalid={e => (e.target as HTMLInputElement).setCustomValidity(t('requiredField'))} - onInput={e => (e.target as HTMLInputElement).setCustomValidity('')} className={`${inputStyles} cursor-default select-none`} />🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@frontend/components/dashboard/FeedbackForm.tsx` around lines 162 - 172, The email input in FeedbackForm (the <input name="email" ... /> element) is readOnly and backed by a non-nullable database value, so remove the redundant validation: delete the required attribute and both handlers onInvalid and onInput, and keep the readOnly/defaultValue/className logic intact (preserve userEmail defaulting). This simplifies the JSX in the FeedbackForm component and avoids unnecessary client-side validation for an uneditable field.frontend/components/dashboard/ProfileCard.tsx (1)
133-133:⚠️ Potential issue | 🟡 MinorRemove the unsupported
fallbackparameter from the translation call.
useTranslationsfromnext-intl4.8.3 does not accept afallbackoption as the second argument. The second argument is reserved for interpolation values like{ variable: value }. The{ fallback: 'Global Rank' }object will be silently ignored sinceglobalRankdoes not contain a{fallback}placeholder.The key
globalRankexists in all locale files, so the code currently works, but the API usage is incorrect. Either remove the fallback parameter entirely, or if you need fallback behavior for missing keys, configuregetMessageFallbackin theNextIntlClientProvidersetup.This pattern appears in multiple components (QuizResult.tsx, StatsCard.tsx, QuizResultsSection.tsx). All instances should be updated consistently.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@frontend/components/dashboard/ProfileCard.tsx` at line 133, Remove the unsupported fallback object passed as the second argument to the next-intl translation calls (e.g., the t('globalRank', { fallback: 'Global Rank' }) usage in ProfileCard.tsx) — replace calls like t('globalRank', { fallback: '...' }) with a simple t('globalRank') and apply the same change across the other components mentioned (QuizResult.tsx, StatsCard.tsx, QuizResultsSection.tsx); if you actually need runtime fallback behavior for missing keys, implement it centrally via getMessageFallback in the NextIntlClientProvider instead of passing a fallback option to t.
🧹 Nitpick comments (9)
frontend/components/header/AppMobileMenu.tsx (1)
121-133: LGTM — scroll-lock approach is correct and the cleanup is safe.Setting
overflow-y: hiddenon<html>(instead ofposition: fixedon<body>) avoids the well-known scroll-position jump and is the reliable cross-browser mechanism for full-page scroll lock. The fixed-position nav withoverflow-y-auto overscroll-containremains independently scrollable since it is out of normal flow. The effect lifecycle handles all four state transitions correctly (open/close/mount/unmount).One optional improvement: save and restore the pre-existing inline value rather than resetting to
'', which would preserve anyoverflow-yset on<html>by third-party code before the menu opened:♻️ Optional: save/restore pattern
- useEffect(() => { - if (open) { - document.documentElement.style.overflowY = 'hidden'; - return () => { - document.documentElement.style.overflowY = ''; - }; - } - }, [open]); + useEffect(() => { + if (open) { + const prev = document.documentElement.style.overflowY; + document.documentElement.style.overflowY = 'hidden'; + return () => { + document.documentElement.style.overflowY = prev; + }; + } + }, [open]);🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@frontend/components/header/AppMobileMenu.tsx` around lines 121 - 133, Change the effect in useEffect that currently sets document.documentElement.style.overflowY = 'hidden' and later resets it to '' so it preserves any pre-existing inline value: inside the effect (hook for open) capture const prev = document.documentElement.style.overflowY before setting 'hidden', then in the cleanup restore document.documentElement.style.overflowY = prev; update the effect referencing open and keep the same return behavior in AppMobileMenu's useEffect to ensure prior inline overflow-y is restored on close/unmount.frontend/lib/github-stars.ts (1)
68-68: Optional: renamepage_datatopageDatato follow TypeScript camelCase conventions.♻️ Suggested rename
- const page_data: { login: string; avatar_url: string }[] = await res.json(); - if (page_data.length === 0) break; + const pageData: { login: string; avatar_url: string }[] = await res.json(); + if (pageData.length === 0) break; for (const s of page_data) { + for (const s of pageData) { all.push({ login: s.login.toLowerCase(), avatarBase: s.avatar_url.split('?')[0], }); } - if (page_data.length < 100) break; + if (pageData.length < 100) break;🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@frontend/lib/github-stars.ts` at line 68, Rename the snake_case variable page_data to camelCase pageData throughout the function: change the declaration const page_data: { login: string; avatar_url: string }[] = await res.json(); to use pageData and update its type annotation (and any usages of page_data) to reflect the new name, including any destructuring or property accesses (e.g., avatar_url can remain as-is if coming from GitHub). Ensure all references to page_data in the file are updated to pageData to maintain consistency with TypeScript camelCase conventions.frontend/app/[locale]/leaderboard/page.tsx (1)
40-46: DRY: sponsor matching is duplicated fromdashboard/page.tsx'sfindSponsor.Consider extracting the sponsor lookup into a shared utility (e.g.,
frontend/lib/about/github-sponsors.tsor a newfrontend/lib/sponsor-utils.ts) to keep the logic in one place, especially since the leaderboard version introduced a regression (missing null guards) compared to the dashboard version.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@frontend/app/`[locale]/leaderboard/page.tsx around lines 40 - 46, Extract the sponsor-matching logic into a shared function (e.g., findSponsor or matchSponsor) and replace the inline logic that sets matchedSponsor in leaderboard/page.tsx with a call to that function; implement the same utility in frontend/lib (github-sponsors.ts or sponsor-utils.ts), ensure it accepts the sponsors array and a user/email/name input, preserve the dashboard/page.tsx's null guards (check for s.email, s.login, s.name, s.avatarUrl and user.avatar) and the same comparison rules (lowercasing and avatarUrl split('?')[0] match), and update both leaderboard/page.tsx and dashboard/page.tsx to import and use the new function so the logic is centralized and regression-free.frontend/components/leaderboard/LeaderboardTable.tsx (1)
106-138: Each context row creates a separate<table>— pragmatic but worth noting.Wrapping each context row in its own
<table>to enable thebox-shadowglow on the parent<div>is a reasonable workaround since<tr>doesn't supportbox-shadowwell. The duplicated<colgroup>with identical percentage widths ensures column alignment with the main table.Just be aware that if the main table's
<colgroup>widths change, these must be updated in sync — consider extracting the colgroup widths into a shared constant.♻️ Extract shared column config
const COL_WIDTHS = { rank: 'w-[15%] sm:w-[12%]', user: '', // auto score: 'w-[25%] sm:w-[20%]', } as const;Then use it in both the main table and context table colgroups.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@frontend/components/leaderboard/LeaderboardTable.tsx` around lines 106 - 138, Context rows each render their own <table> with duplicated <colgroup> widths; extract and reuse a shared constant to keep column widths in sync. Create a COL_WIDTHS constant (e.g., with keys rank/user/score) and replace the hardcoded class strings in the colgroup of both the main table and the contextRows mapping (where TableRow is rendered) to reference COL_WIDTHS.rank / COL_WIDTHS.user / COL_WIDTHS.score so any future width change is made in one place.frontend/components/leaderboard/AchievementPips.tsx (2)
84-93: Only four hardcoded achievement IDs are recognized; others are silently dropped.The component only renders
golden_patron,silver_patron,supporter, andstar_gazer. If new achievement types are added to theAchievementsystem, they won't appear here without updating this component. This seems intentional, but a brief comment at the top documenting which IDs are rendered (and why others aren't) would help future maintainers.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@frontend/components/leaderboard/AchievementPips.tsx` around lines 84 - 93, The component currently only recognizes four hardcoded achievement IDs via the sponsorBadge and hexPips logic (looking up 'golden_patron', 'silver_patron', 'supporter', and 'star_gazer'), which will silently drop any other Achievement entries; update AchievementPips.tsx to add a brief top-of-file or top-of-component comment explaining explicitly which IDs are rendered and why other achievement types are intentionally ignored (e.g., design/UX reasons or layout constraints), referencing the sponsorBadge and hexPips variables so future maintainers know to update those lookups if more IDs should be supported.
100-110: Tooltip position can drift if the page is scrolled while hovering.
getBoundingClientRect()gives viewport-relative coordinates at the moment ofmouseEnter, but the tooltip's fixed position isn't updated on scroll. For these small hex pips the user is unlikely to scroll while hovering, so this is a minor UX edge case.If you ever want to harden this, consider dismissing the tooltip on scroll or recalculating position.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@frontend/components/leaderboard/AchievementPips.tsx` around lines 100 - 110, Tooltip position can drift after getBoundingClientRect() is computed in handleMouseEnter because viewport changes on scroll; fix by tracking the hovered element and updating or dismissing the tooltip on scroll. Modify handleMouseEnter to store the event.currentTarget (or a stable ref to that element) alongside the tooltip state, add a scroll listener (attached to window) that on scroll either recalculates the tooltip position using the stored element's getBoundingClientRect() and calls setTooltip(...) with new x/y or calls the tooltip hide function to dismiss it, and ensure you remove the scroll listener on mouseLeave and component unmount; update code paths using handleMouseEnter, setTooltip, and any mouseLeave cleanup to manage the stored target and listener lifecycle.frontend/components/dashboard/ProfileCard.tsx (1)
142-156: "Joined" stat block doesn't usestatItemBase, breaking visual consistency.The other three stat items (Attempts, Points, Global Rank) all use the shared
statItemBaseclass, including hover transitions. The "Joined" block uses hardcoded classes that lack the hover effects (hover:border-...,hover:bg-...,transition-all), creating a visual inconsistency.♻️ Proposed fix — use statItemBase for consistency
- <div className="flex flex-row items-center gap-2 sm:gap-3 rounded-2xl border border-gray-100 bg-white/50 p-2 sm:p-3 text-left dark:border-white/5 dark:bg-black/20 xl:flex-row-reverse xl:items-center xl:text-right xl:p-3 xl:px-4"> + <div className={statItemBase}>🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@frontend/components/dashboard/ProfileCard.tsx` around lines 142 - 156, The "Joined" stat block in ProfileCard.tsx is using hardcoded classes instead of the shared statItemBase, so it lacks hover and transition styles; update the outer wrapper for the Joined block to use the existing statItemBase class (combine statItemBase with any unique classes needed for layout like xl:flex-row-reverse or padding overrides) where the Calendar/createdAt rendering occurs (inside the ProfileCard component) so it inherits hover:border-..., hover:bg-..., and transition-all like the other stat items.frontend/components/dashboard/FeedbackForm.tsx (1)
249-261: No client-side file size validation — large files will only be rejected after upload.The 5 MB limit is enforced server-side (413 response), but the user won't see an error until the entire file has been uploaded. For better UX, validate file size when files are selected and surface an immediate error.
♻️ Proposed addition inside the onChange handler
const newFiles = Array.from(e.target.files ?? []); + const MAX_FILE_SIZE = 5 * 1024 * 1024; + const oversized = newFiles.filter(f => f.size > MAX_FILE_SIZE); + if (oversized.length > 0) { + // TODO: surface a user-visible error (e.g. via state) + return; + } const merged = [...accumulatedFilesRef.current];frontend/app/api/feedback/route.ts (1)
45-48: Remove duplicate SMTP transporter — reuse the shared one fromlib/email/transporter.ts.The feedback route creates a duplicate Gmail transporter instance with identical configuration to the existing shared transporter. Consolidating to the shared
mailerexport reduces configuration duplication and simplifies future provider changes.♻️ Proposed refactor
-import nodemailer from 'nodemailer'; import { NextRequest, NextResponse } from 'next/server'; + +import { mailer } from '@/lib/email/transporter';Then remove the local
createTransportcall at lines 45-48 and use the importedmailerdirectly.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@frontend/app/api/feedback/route.ts` around lines 45 - 48, The route is creating a duplicate SMTP transporter (`const mailer = nodemailer.createTransport(...)`) — remove that local `mailer` creation and use the shared `mailer` exported from lib/email/transporter.ts instead; delete the local `createTransport` call in frontend/app/api/feedback/route.ts and replace any uses of the local `mailer` in the feedback handler (e.g., the function that calls mailer.sendMail/send) to reference the imported shared `mailer` export so configuration is centralized.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@frontend/app/`[locale]/leaderboard/page.tsx:
- Around line 65-69: The current hasStarred logic can miss users whose displayed
username differs from their GitHub login; replace or augment the username-based
check with the same resolution used elsewhere (call resolveGitHubLogin(user) or
use user.githubLogin if present) and compare its lowercased value against
stargazerLogins, keeping the existing avatarBase/avatar fallback (avatarBase and
stargazerAvatars) intact; this ensures you match by resolved GitHub login rather
than display name to avoid false negatives in hasStarred.
- Around line 40-46: The matchedSponsor lookup can throw when s.login or s.name
are null; update the sponsors.find predicate used for matchedSponsor so each
comparison guards the field before calling toLowerCase (e.g., check s.login and
s.name exist like the dashboard findSponsor does), i.e., only call
s.login.toLowerCase() or s.name.toLowerCase() when those properties are truthy
(and still use nameLower/user.avatar checks and the s.avatarUrl split check as
currently written).
In `@frontend/app/api/feedback/route.ts`:
- Line 53: The replyTo header is built from user-controlled variables ("name"
and "email") in frontend/app/api/feedback/route.ts and is vulnerable to header
injection; before constructing replyTo: `"${name}" <${email}>`, sanitize the
name by stripping or escaping CR/LF and RFC‑5322 special characters (quotes,
angle brackets, backslashes, control chars) or replace with a safe fallback
(e.g., omit display name) and validate email with a strict pattern; update the
code that builds the replyTo header (the replyTo assignment) to use the
sanitized/validated values so no user input can inject headers.
- Around line 32-43: The code currently checks MAX_FILE_SIZE per file but allows
unlimited attachments; add a cap on the total number of attachments (e.g.,
MAX_FILES = 5 or 10) and enforce it before processing: after obtaining
rawFiles/files (symbols: rawFiles, files) validate files.length <= MAX_FILES and
return an error response (similar to the existing 413 or a 400) if exceeded;
this prevents building the attachments array (symbol: attachments) when too many
files are sent and avoids excessive memory/email size.
- Around line 56-62: The HTML email body directly interpolates unescaped user
input (name, email, category, message) causing HTML injection; add a small
HTML-escape helper (e.g., escapeHtml) that replaces &, <, >, and " (and
optionally ') with HTML entities, use escapeHtml(name), escapeHtml(email),
escapeHtml(category) when building the template, and for message first run
escapeHtml(message) then replace newlines with <br> (i.e.,
escapeHtml(message).replace(/\n/g,'<br>')) before interpolation so all
user-supplied values are sanitized in the template generation in route.ts.
In `@frontend/app/globals.css`:
- Around line 183-186: The CSS rule inside the &:hover block is triggering
stylelint's "empty line before declaration" rule; open the &:hover selector in
globals.css and insert a blank line before the border-color declaration so there
is an empty line between the `@apply` line and the border-color line (i.e., update
the &:hover block in globals.css to have a blank line before border-color).
In `@frontend/components/leaderboard/LeaderboardPodium.tsx`:
- Line 133: The fallback SPONSOR_TIER_STYLE.supporter is undefined so the
nullish coalescing is a no-op; update the expression that picks the style (where
SPONSOR_TIER_STYLE[sponsorAch.id] is used) to either remove the misleading
fallback or replace it with an actual key that exists in SPONSOR_TIER_STYLE
(e.g., SPONSOR_TIER_STYLE['silver_patron'] or
SPONSOR_TIER_STYLE['golden_patron']), or add a real 'supporter' entry to the
SPONSOR_TIER_STYLE map; refer to SPONSOR_TIER_STYLE, sponsorAch.id and
getSponsorAchievement to choose the correct existing default.
---
Outside diff comments:
In `@frontend/components/dashboard/ActivityHeatmapCard.tsx`:
- Around line 471-474: The tooltip text in ActivityHeatmapCard is hardcoded —
replace 'No activity', 'attempt', and 'attempts' with translation keys via the
t() hook used elsewhere in the component (e.g., use
t('activityHeatmap.noActivity'), t('activityHeatmap.attempt_singular'),
t('activityHeatmap.attempt_plural') or a pluralized t call) and add
corresponding entries to the translations file; update the rendering logic that
currently uses tooltip.count to call t(...) with the correct key or
pluralization so all displayed strings in ActivityHeatmapCard are localized.
In `@frontend/components/dashboard/FeedbackForm.tsx`:
- Around line 162-172: The email input in FeedbackForm (the <input name="email"
... /> element) is readOnly and backed by a non-nullable database value, so
remove the redundant validation: delete the required attribute and both handlers
onInvalid and onInput, and keep the readOnly/defaultValue/className logic intact
(preserve userEmail defaulting). This simplifies the JSX in the FeedbackForm
component and avoids unnecessary client-side validation for an uneditable field.
In `@frontend/components/dashboard/ProfileCard.tsx`:
- Line 133: Remove the unsupported fallback object passed as the second argument
to the next-intl translation calls (e.g., the t('globalRank', { fallback:
'Global Rank' }) usage in ProfileCard.tsx) — replace calls like t('globalRank',
{ fallback: '...' }) with a simple t('globalRank') and apply the same change
across the other components mentioned (QuizResult.tsx, StatsCard.tsx,
QuizResultsSection.tsx); if you actually need runtime fallback behavior for
missing keys, implement it centrally via getMessageFallback in the
NextIntlClientProvider instead of passing a fallback option to t.
---
Nitpick comments:
In `@frontend/app/`[locale]/leaderboard/page.tsx:
- Around line 40-46: Extract the sponsor-matching logic into a shared function
(e.g., findSponsor or matchSponsor) and replace the inline logic that sets
matchedSponsor in leaderboard/page.tsx with a call to that function; implement
the same utility in frontend/lib (github-sponsors.ts or sponsor-utils.ts),
ensure it accepts the sponsors array and a user/email/name input, preserve the
dashboard/page.tsx's null guards (check for s.email, s.login, s.name,
s.avatarUrl and user.avatar) and the same comparison rules (lowercasing and
avatarUrl split('?')[0] match), and update both leaderboard/page.tsx and
dashboard/page.tsx to import and use the new function so the logic is
centralized and regression-free.
In `@frontend/app/api/feedback/route.ts`:
- Around line 45-48: The route is creating a duplicate SMTP transporter (`const
mailer = nodemailer.createTransport(...)`) — remove that local `mailer` creation
and use the shared `mailer` exported from lib/email/transporter.ts instead;
delete the local `createTransport` call in frontend/app/api/feedback/route.ts
and replace any uses of the local `mailer` in the feedback handler (e.g., the
function that calls mailer.sendMail/send) to reference the imported shared
`mailer` export so configuration is centralized.
In `@frontend/components/dashboard/ProfileCard.tsx`:
- Around line 142-156: The "Joined" stat block in ProfileCard.tsx is using
hardcoded classes instead of the shared statItemBase, so it lacks hover and
transition styles; update the outer wrapper for the Joined block to use the
existing statItemBase class (combine statItemBase with any unique classes needed
for layout like xl:flex-row-reverse or padding overrides) where the
Calendar/createdAt rendering occurs (inside the ProfileCard component) so it
inherits hover:border-..., hover:bg-..., and transition-all like the other stat
items.
In `@frontend/components/header/AppMobileMenu.tsx`:
- Around line 121-133: Change the effect in useEffect that currently sets
document.documentElement.style.overflowY = 'hidden' and later resets it to '' so
it preserves any pre-existing inline value: inside the effect (hook for open)
capture const prev = document.documentElement.style.overflowY before setting
'hidden', then in the cleanup restore document.documentElement.style.overflowY =
prev; update the effect referencing open and keep the same return behavior in
AppMobileMenu's useEffect to ensure prior inline overflow-y is restored on
close/unmount.
In `@frontend/components/leaderboard/AchievementPips.tsx`:
- Around line 84-93: The component currently only recognizes four hardcoded
achievement IDs via the sponsorBadge and hexPips logic (looking up
'golden_patron', 'silver_patron', 'supporter', and 'star_gazer'), which will
silently drop any other Achievement entries; update AchievementPips.tsx to add a
brief top-of-file or top-of-component comment explaining explicitly which IDs
are rendered and why other achievement types are intentionally ignored (e.g.,
design/UX reasons or layout constraints), referencing the sponsorBadge and
hexPips variables so future maintainers know to update those lookups if more IDs
should be supported.
- Around line 100-110: Tooltip position can drift after getBoundingClientRect()
is computed in handleMouseEnter because viewport changes on scroll; fix by
tracking the hovered element and updating or dismissing the tooltip on scroll.
Modify handleMouseEnter to store the event.currentTarget (or a stable ref to
that element) alongside the tooltip state, add a scroll listener (attached to
window) that on scroll either recalculates the tooltip position using the stored
element's getBoundingClientRect() and calls setTooltip(...) with new x/y or
calls the tooltip hide function to dismiss it, and ensure you remove the scroll
listener on mouseLeave and component unmount; update code paths using
handleMouseEnter, setTooltip, and any mouseLeave cleanup to manage the stored
target and listener lifecycle.
In `@frontend/components/leaderboard/LeaderboardTable.tsx`:
- Around line 106-138: Context rows each render their own <table> with
duplicated <colgroup> widths; extract and reuse a shared constant to keep column
widths in sync. Create a COL_WIDTHS constant (e.g., with keys rank/user/score)
and replace the hardcoded class strings in the colgroup of both the main table
and the contextRows mapping (where TableRow is rendered) to reference
COL_WIDTHS.rank / COL_WIDTHS.user / COL_WIDTHS.score so any future width change
is made in one place.
In `@frontend/lib/github-stars.ts`:
- Line 68: Rename the snake_case variable page_data to camelCase pageData
throughout the function: change the declaration const page_data: { login:
string; avatar_url: string }[] = await res.json(); to use pageData and update
its type annotation (and any usages of page_data) to reflect the new name,
including any destructuring or property accesses (e.g., avatar_url can remain
as-is if coming from GitHub). Ensure all references to page_data in the file are
updated to pageData to maintain consistency with TypeScript camelCase
conventions.
| const matchedSponsor = sponsors.find( | ||
| s => | ||
| (s.email && s.email.toLowerCase() === emailLower) || | ||
| (nameLower && s.login.toLowerCase() === nameLower) || | ||
| (nameLower && s.name.toLowerCase() === nameLower) || | ||
| (user.avatar && s.avatarUrl && user.avatar.includes(s.avatarUrl.split('?')[0])) | ||
| ); |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
# Find the Sponsor type definition
fd -t f -e ts -e tsx | xargs rg -l "interface Sponsor|type Sponsor" | head -5Repository: DevLoversTeam/devlovers.net
Length of output: 151
🏁 Script executed:
# Search for Sponsor type with field details
rg "interface Sponsor|type Sponsor" -A 15 --type ts --type tsxRepository: DevLoversTeam/devlovers.net
Length of output: 98
🏁 Script executed:
# Check the dashboard/page.tsx findSponsor implementation
fd -path "*/dashboard/page.tsx" -type fRepository: DevLoversTeam/devlovers.net
Length of output: 239
🏁 Script executed:
# Check the leaderboard/page.tsx implementation at lines 40-46
fd -path "*/leaderboard/page.tsx" -type fRepository: DevLoversTeam/devlovers.net
Length of output: 239
🏁 Script executed:
# Find the files mentioned
fd "dashboard/page" -o fd "leaderboard/page"Repository: DevLoversTeam/devlovers.net
Length of output: 194
🏁 Script executed:
# Search for Sponsor type definition across the repo
rg "interface Sponsor|type Sponsor" -A 15Repository: DevLoversTeam/devlovers.net
Length of output: 2252
🏁 Script executed:
# Check the github-sponsors.ts file
cat -n frontend/lib/about/github-sponsors.tsRepository: DevLoversTeam/devlovers.net
Length of output: 3985
🏁 Script executed:
find . -path "*/dashboard/page.tsx" -o -path "*/leaderboard/page.tsx"Repository: DevLoversTeam/devlovers.net
Length of output: 158
🏁 Script executed:
# Search for findSponsor in dashboard
rg "findSponsor" -B 5 -A 10Repository: DevLoversTeam/devlovers.net
Length of output: 2738
🏁 Script executed:
# Look for the leaderboard page.tsx file
rg "const matchedSponsor = sponsors.find" -B 5 -A 10Repository: DevLoversTeam/devlovers.net
Length of output: 1328
Missing null guards on s.login and s.name will throw TypeError at runtime.
The dashboard's findSponsor function properly guards both fields:
if (userName && s.login && s.login.toLowerCase() === userName) return true;
if (userName && s.name && s.name.toLowerCase() === userName) return true;The leaderboard's matchedSponsor lookup omits these guards. Although the Sponsor interface declares these as non-nullable strings, the actual GitHub API can return null for name (as evidenced by the fallback in github-sponsors.ts line 82: name: node.sponsorEntity.name || node.sponsorEntity.login). Calling .toLowerCase() on null throws a TypeError.
Fix
const matchedSponsor = sponsors.find(
s =>
(s.email && s.email.toLowerCase() === emailLower) ||
- (nameLower && s.login.toLowerCase() === nameLower) ||
- (nameLower && s.name.toLowerCase() === nameLower) ||
+ (nameLower && s.login && s.login.toLowerCase() === nameLower) ||
+ (nameLower && s.name && s.name.toLowerCase() === nameLower) ||
(user.avatar && s.avatarUrl && user.avatar.includes(s.avatarUrl.split('?')[0]))
);🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@frontend/app/`[locale]/leaderboard/page.tsx around lines 40 - 46, The
matchedSponsor lookup can throw when s.login or s.name are null; update the
sponsors.find predicate used for matchedSponsor so each comparison guards the
field before calling toLowerCase (e.g., check s.login and s.name exist like the
dashboard findSponsor does), i.e., only call s.login.toLowerCase() or
s.name.toLowerCase() when those properties are truthy (and still use
nameLower/user.avatar checks and the s.avatarUrl split check as currently
written).
| const avatarBase = user.avatar?.split('?')[0] ?? ''; | ||
| const hasStarred = | ||
| stargazerLogins.has(nameLower) || | ||
| (avatarBase.includes('avatars.githubusercontent.com') && | ||
| stargazerAvatars.has(avatarBase)); |
There was a problem hiding this comment.
Username-based stargazer matching may produce false negatives.
stargazerLogins.has(nameLower) compares the app's username field against GitHub logins. A user whose GitHub login differs from their display name (common with OAuth display names) won't be matched this way, even if they have starred the repo. The avatar-URL fallback partially compensates, but users without GitHub avatars won't benefit.
This is a limitation given no GitHub provider ID is available on the leaderboard rows — just worth documenting or using the same resolveGitHubLogin path where feasible.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@frontend/app/`[locale]/leaderboard/page.tsx around lines 65 - 69, The current
hasStarred logic can miss users whose displayed username differs from their
GitHub login; replace or augment the username-based check with the same
resolution used elsewhere (call resolveGitHubLogin(user) or use user.githubLogin
if present) and compare its lowercased value against stargazerLogins, keeping
the existing avatarBase/avatar fallback (avatarBase and stargazerAvatars)
intact; this ensures you match by resolved GitHub login rather than display name
to avoid false negatives in hasStarred.
| const rawFiles = formData.getAll('attachment'); | ||
| const files = rawFiles.filter((f): f is File => f instanceof File && f.size > 0); | ||
|
|
||
| const attachments: { filename: string; content: Buffer; contentType: string }[] = []; | ||
|
|
||
| for (const f of files) { | ||
| if (f.size > MAX_FILE_SIZE) { | ||
| return NextResponse.json({ success: false, tooLarge: true }, { status: 413 }); | ||
| } | ||
| const buffer = Buffer.from(await f.arrayBuffer()); | ||
| attachments.push({ filename: f.name, content: buffer, contentType: f.type }); | ||
| } |
There was a problem hiding this comment.
No limit on total number of attachments.
A malicious client can send hundreds of small files, each under 5 MB, causing high memory usage and potentially large emails. Consider capping the number of attachments (e.g., 5–10 files).
🛡️ Proposed fix
+const MAX_ATTACHMENTS = 10;
+
const rawFiles = formData.getAll('attachment');
const files = rawFiles.filter((f): f is File => f instanceof File && f.size > 0);
+if (files.length > MAX_ATTACHMENTS) {
+ return NextResponse.json({ success: false }, { status: 400 });
+}
+
const attachments: { filename: string; content: Buffer; contentType: string }[] = [];🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@frontend/app/api/feedback/route.ts` around lines 32 - 43, The code currently
checks MAX_FILE_SIZE per file but allows unlimited attachments; add a cap on the
total number of attachments (e.g., MAX_FILES = 5 or 10) and enforce it before
processing: after obtaining rawFiles/files (symbols: rawFiles, files) validate
files.length <= MAX_FILES and return an error response (similar to the existing
413 or a 400) if exceeded; this prevents building the attachments array (symbol:
attachments) when too many files are sent and avoids excessive memory/email
size.
| try { | ||
| await mailer.sendMail({ | ||
| from: emailFrom, | ||
| replyTo: `"${name}" <${email}>`, |
There was a problem hiding this comment.
Email header injection risk via user-controlled name in replyTo.
The name value is embedded directly into the replyTo header. A crafted name containing ", newlines, or RFC 5322 special characters could manipulate the header. Sanitize or strip these characters before embedding.
🛡️ Proposed fix
- replyTo: `"${name}" <${email}>`,
+ replyTo: `"${name.replace(/["\\\r\n]/g, '')}" <${email}>`,📝 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.
| replyTo: `"${name}" <${email}>`, | |
| replyTo: `"${name.replace(/["\\\r\n]/g, '')}" <${email}>`, |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@frontend/app/api/feedback/route.ts` at line 53, The replyTo header is built
from user-controlled variables ("name" and "email") in
frontend/app/api/feedback/route.ts and is vulnerable to header injection; before
constructing replyTo: `"${name}" <${email}>`, sanitize the name by stripping or
escaping CR/LF and RFC‑5322 special characters (quotes, angle brackets,
backslashes, control chars) or replace with a safe fallback (e.g., omit display
name) and validate email with a strict pattern; update the code that builds the
replyTo header (the replyTo assignment) to use the sanitized/validated values so
no user input can inject headers.
| html: ` | ||
| <p><strong>Name:</strong> ${name}</p> | ||
| <p><strong>Email:</strong> ${email}</p> | ||
| <p><strong>Category:</strong> ${category}</p> | ||
| <p><strong>Message:</strong></p> | ||
| <p>${message.replace(/\n/g, '<br>')}</p> | ||
| `, |
There was a problem hiding this comment.
HTML injection in email body — user input is not sanitized.
name, email, category, and message are interpolated directly into the HTML string. A malicious user can inject arbitrary HTML/scripts into the email body. While most modern email clients sandbox HTML, this is still a security risk (e.g., phishing links, styled spoofed content).
At minimum, escape <, >, &, and " before embedding in HTML.
🛡️ Proposed fix — add a simple HTML-escape helper
+function escapeHtml(str: string): string {
+ return str
+ .replace(/&/g, '&')
+ .replace(/</g, '<')
+ .replace(/>/g, '>')
+ .replace(/"/g, '"');
+}
+
export async function POST(req: NextRequest) {Then use it in the template:
html: `
- <p><strong>Name:</strong> ${name}</p>
- <p><strong>Email:</strong> ${email}</p>
- <p><strong>Category:</strong> ${category}</p>
+ <p><strong>Name:</strong> ${escapeHtml(name)}</p>
+ <p><strong>Email:</strong> ${escapeHtml(email)}</p>
+ <p><strong>Category:</strong> ${escapeHtml(category)}</p>
<p><strong>Message:</strong></p>
- <p>${message.replace(/\n/g, '<br>')}</p>
+ <p>${escapeHtml(message).replace(/\n/g, '<br>')}</p>
`,📝 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.
| html: ` | |
| <p><strong>Name:</strong> ${name}</p> | |
| <p><strong>Email:</strong> ${email}</p> | |
| <p><strong>Category:</strong> ${category}</p> | |
| <p><strong>Message:</strong></p> | |
| <p>${message.replace(/\n/g, '<br>')}</p> | |
| `, | |
| function escapeHtml(str: string): string { | |
| return str | |
| .replace(/&/g, '&') | |
| .replace(/</g, '<') | |
| .replace(/>/g, '>') | |
| .replace(/"/g, '"'); | |
| } | |
| export async function POST(req: NextRequest) { | |
| // ... existing code ... | |
| html: ` | |
| <p><strong>Name:</strong> ${escapeHtml(name)}</p> | |
| <p><strong>Email:</strong> ${escapeHtml(email)}</p> | |
| <p><strong>Category:</strong> ${escapeHtml(category)}</p> | |
| <p><strong>Message:</strong></p> | |
| <p>${escapeHtml(message).replace(/\n/g, '<br>')}</p> | |
| `, |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@frontend/app/api/feedback/route.ts` around lines 56 - 62, The HTML email body
directly interpolates unescaped user input (name, email, category, message)
causing HTML injection; add a small HTML-escape helper (e.g., escapeHtml) that
replaces &, <, >, and " (and optionally ') with HTML entities, use
escapeHtml(name), escapeHtml(email), escapeHtml(category) when building the
template, and for message first run escapeHtml(message) then replace newlines
with <br> (i.e., escapeHtml(message).replace(/\n/g,'<br>')) before interpolation
so all user-supplied values are sanitized in the template generation in
route.ts.
| &:hover { | ||
| @apply -translate-y-1 shadow-md; | ||
| border-color: color-mix(in srgb, var(--accent-primary) 30%, transparent); | ||
| } |
There was a problem hiding this comment.
Fix stylelint “empty line before declaration” warning.
Add a blank line before border-color inside &:hover to satisfy the configured rule.
🧹 Suggested lint fix
&:hover {
`@apply` -translate-y-1 shadow-md;
+
border-color: color-mix(in srgb, var(--accent-primary) 30%, transparent);
}🧰 Tools
🪛 Stylelint (17.3.0)
[error] 185-185: Expected empty line before declaration (declaration-empty-line-before)
(declaration-empty-line-before)
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@frontend/app/globals.css` around lines 183 - 186, The CSS rule inside the
&:hover block is triggering stylelint's "empty line before declaration" rule;
open the &:hover selector in globals.css and insert a blank line before the
border-color declaration so there is an empty line between the `@apply` line and
the border-color line (i.e., update the &:hover block in globals.css to have a
blank line before border-color).
| const sponsorAch = getSponsorAchievement(user); | ||
| if (!sponsorAch) return null; | ||
| const tierStyle = | ||
| SPONSOR_TIER_STYLE[sponsorAch.id] ?? SPONSOR_TIER_STYLE.supporter; |
There was a problem hiding this comment.
SPONSOR_TIER_STYLE.supporter is undefined — the fallback is a no-op.
'supporter' is not a key in SPONSOR_TIER_STYLE, so SPONSOR_TIER_STYLE.supporter resolves to undefined at runtime (TypeScript's Record<string, string> index signature hides this). The ?? fallback therefore never provides a real style. Since getSponsorAchievement only returns golden_patron or silver_patron, the fallback is never actually needed — but it reads as if it provides a meaningful default.
🛠️ Proposed fix
- const tierStyle =
- SPONSOR_TIER_STYLE[sponsorAch.id] ?? SPONSOR_TIER_STYLE.supporter;
+ const tierStyle = SPONSOR_TIER_STYLE[sponsorAch.id] ?? '';🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@frontend/components/leaderboard/LeaderboardPodium.tsx` at line 133, The
fallback SPONSOR_TIER_STYLE.supporter is undefined so the nullish coalescing is
a no-op; update the expression that picks the style (where
SPONSOR_TIER_STYLE[sponsorAch.id] is used) to either remove the misleading
fallback or replace it with an actual key that exists in SPONSOR_TIER_STYLE
(e.g., SPONSOR_TIER_STYLE['silver_patron'] or
SPONSOR_TIER_STYLE['golden_patron']), or add a real 'supporter' entry to the
SPONSOR_TIER_STYLE map; refer to SPONSOR_TIER_STYLE, sponsorAch.id and
getSponsorAchievement to choose the correct existing default.
2 participants
Summary by CodeRabbit
Release Notes
New Features
Improvements