Improved Guest Page, Side Bar Layout and readme.md#196
Merged
sumagnadas merged 9 commits intoOpenLake:mainfrom Mar 24, 2026
Merged
Improved Guest Page, Side Bar Layout and readme.md#196sumagnadas merged 9 commits intoOpenLake:mainfrom
sumagnadas merged 9 commits intoOpenLake:mainfrom
Conversation
…ed user data handling, improved guest mode layout
This comment has been minimized.
This comment has been minimized.
Contributor
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
WalkthroughThis PR introduces a comprehensive feature expansion with AtCoder platform support, an achievement/badge system, discussion post voting, backend-backed friends management, and a significant sidebar/navigation UI redesign. It includes new Django models, migrations, API endpoints, Celery tasks, and refactored frontend components with responsive styling updates. Changes
Sequence DiagramssequenceDiagram
participant Client as Client Browser
participant Server as Django Server
participant Database as Database
Client->>Server: POST /achievements/unlock/<br/>(slug, tier)
activate Server
Server->>Database: get_or_create Achievement<br/>(user, slug, tier)
activate Database
Database-->>Server: Achievement object
deactivate Database
Server-->>Client: 201 Created or<br/>200 Already unlocked
deactivate Server
sequenceDiagram
participant Client as Client Browser
participant Server as Django Server
participant Database as Database
Client->>Server: PUT /blogs/<br/>(title, action: like/dislike)
activate Server
Server->>Database: Get PostVote(user, post)
activate Database
Database-->>Server: PostVote or None
deactivate Database
alt Vote exists & same action
Server->>Database: Delete PostVote
Server->>Database: Decrement post.likes/dislikes
else Vote exists & different action
Server->>Database: Update vote_type (toggle)
Server->>Database: Increment one counter,<br/>decrement other
else No vote exists
Server->>Database: Create PostVote
Server->>Database: Increment post.likes/dislikes
end
Server-->>Client: 200 Updated blog payload
deactivate Server
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 1 | ❌ 2❌ Failed checks (2 warnings)
✅ 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}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 5
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (4)
app/src/components/Heatmap.jsx (1)
142-188:⚠️ Potential issue | 🟠 MajorMissing
calculateStreakcall inprocessGitHubData.
processCodeforcesData(line 139) callscalculateStreak(heatmapArray)after setting the heatmap data, butprocessGitHubDatadoes not. This means GitHub contributions won't update the streak display.🐛 Proposed fix
console.log("Processed heatmap data:", heatmapArray); // Debug log setHeatmapData(heatmapArray); + calculateStreak(heatmapArray); };🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@app/src/components/Heatmap.jsx` around lines 142 - 188, processGitHubData builds heatmapArray but never updates the streak like processCodeforcesData does; after setHeatmapData(heatmapArray) call calculateStreak(heatmapArray) to compute and update the streak display (ensure you call the same calculateStreak function used by processCodeforcesData so GitHub contributions update the streak UI).app/src/components/CodechefTable.jsx (1)
70-78:⚠️ Potential issue | 🟠 MajorBug: Profile link points to wrong domain.
The CodeChef table links usernames to
codeforces.cominstead ofcodechef.com. Users will be redirected to non-existent or incorrect profiles.🐛 Proposed fix
<Button variant="link" asChild> <a - href={`https://codeforces.com/profile/${username}`} + href={`https://www.codechef.com/users/${username}`} target="_blank" rel="noreferrer" >🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@app/src/components/CodechefTable.jsx` around lines 70 - 78, The profile anchor in the CodechefTable.jsx component currently builds URLs using the wrong domain (https://codeforces.com/profile/${username}); update the href to use the CodeChef domain (https://www.codechef.com/users/${username}) so usernames link to the correct profiles—locate the anchor inside the Button (variant="link" asChild) in the CodechefTable component and change the URL template accordingly, keeping target="_blank" and rel="noreferrer" behavior intact.api/leaderboard/views.py (1)
125-146:⚠️ Potential issue | 🟠 MajorRace condition: TOCTOU vulnerability in user creation.
The check-then-create pattern (
exists()followed bysave()) has a race condition. Two concurrent POST requests for the same username can both pass the existence check, then one will fail withIntegrityErrorwhen the secondsave()violates the unique constraint.Use
get_or_createorupdate_or_createfor atomic operation:🐛 Proposed fix using get_or_create
- # check if user already exists - if githubUser.objects.filter(username=username).exists(): - return Response( - {"message": "User already exists"}, status=status.HTTP_200_OK - ) - # Create user and fetch GitHub data github_data = self.fetch_github_data(username) if not github_data: return Response( {"error": "GitHub user not found"}, status=status.HTTP_404_NOT_FOUND ) - gh_user = githubUser( - username=username, + gh_user, created = githubUser.objects.update_or_create( + username=username, + defaults={ + "avatar": github_data["avatar"], + "repositories": github_data["repositories"], + "stars": github_data["stars"], + "contributions": github_data["contributions"], + } + ) - avatar=github_data["avatar"], - repositories=github_data["repositories"], - stars=github_data["stars"], - contributions=github_data["contributions"], - last_updated=github_data["last_updated"], - ) - gh_user.save() - return Response(GH_Serializer(gh_user).data, status=status.HTTP_201_CREATED) + status_code = status.HTTP_201_CREATED if created else status.HTTP_200_OK + return Response(GH_Serializer(gh_user).data, status=status_code)🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@api/leaderboard/views.py` around lines 125 - 146, The current check-then-create in the view (githubUser.exists() followed by creating githubUser and gh_user.save()) is vulnerable to a TOCTOU race; replace this pattern with an atomic operation such as githubUser.objects.get_or_create(...) or githubUser.objects.update_or_create(...) when creating the record after calling fetch_github_data(username), and handle the returned (instance, created) tuple appropriately (or catch IntegrityError around save() as a fallback). Ensure you populate fields (avatar, repositories, stars, contributions, last_updated) in the get_or_create/update_or_create call or update the instance when created==False to avoid duplicate creation errors.app/src/components/CodeforcesTable.jsx (1)
96-99:⚠️ Potential issue | 🟡 MinorReplace
{ timeout: 10000 }withAbortControllerto implement request timeouts.The native
fetchAPI does not support thetimeoutoption and will ignore it. Requests will wait indefinitely (or until the browser's default timeout). UseAbortControllerwithAbortSignalto implement timeouts.Example fix using AbortController
const fetchWithTimeout = async (url, timeoutMs = 10000) => { const controller = new AbortController(); const timeoutId = setTimeout(() => controller.abort(), timeoutMs); try { const response = await fetch(url, { signal: controller.signal }); clearTimeout(timeoutId); return response; } catch (error) { clearTimeout(timeoutId); throw error; } };🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@app/src/components/CodeforcesTable.jsx` around lines 96 - 99, Replace the unsupported { timeout: 10000 } option in the fetch call that creates cfResponse with an AbortController-based timeout; create a helper like fetchWithTimeout (or inline controller logic) that constructs an AbortController, sets a setTimeout to controller.abort() after the desired ms, calls fetch(url, { signal: controller.signal }), clears the timeout on success or failure, and rethrows errors so callers of the function in CodeforcesTable.jsx can handle AbortError/timeouts appropriately.
🧹 Nitpick comments (26)
app/src/components/GoToTop.jsx (1)
9-28: Unnecessary React Fragment.When returning a single element, the Fragment wrapper is redundant. You can return the button directly.
♻️ Suggested simplification
return ( - <> <button className="bg-foreground text-background" style={{ position: "fixed", bottom: "3rem", right: "3rem", zIndex: "999", display: "flex", justifyContent: "center", width: "3rem", height: "3rem", alignItems: "center", borderRadius: "50%", }} onClick={gotop} > <ArrowUp /> </button> - </> );🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@app/src/components/GoToTop.jsx` around lines 9 - 28, The JSX in GoToTop.jsx wraps a single button element in an unnecessary React Fragment; remove the surrounding <>...</> and return the <button> directly from the component (keep the existing props, inline styles, onClick handler gotop, and child ArrowUp unchanged) so the component renders the button without the redundant fragment.app/src/components/GithubTable.jsx (3)
197-199: Misleading variable namefruitin filter callback.This appears to be a copy-paste artifact. Rename to something meaningful like
friendorusername.setGithubfriends((current) => - current.filter((fruit) => fruit !== e), + current.filter((friend) => friend !== e), );🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@app/src/components/GithubTable.jsx` around lines 197 - 199, The filter callback in the setGithubfriends update uses a misleading name "fruit"; update the callback parameter to a meaningful identifier (e.g., "friend" or "username") so the code reads setGithubfriends((current) => current.filter((friend) => friend !== e)); locate this inside the component where setGithubfriends is called and replace "fruit" with the chosen identifier in that callback only.
205-212: Missinggetghfriendsin useEffect dependency array.The
getghfriendsfunction is called inside this effect but not listed in the dependency array. This can lead to stale closures. Either add it to the deps array (after wrapping withuseCallback) or move the function logic inline.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@app/src/components/GithubTable.jsx` around lines 205 - 212, The effect calls getghfriends but doesn’t include it in the dependency array, which can cause stale closures; either wrap getghfriends in useCallback (e.g., memoize the getghfriends function so it only changes when its real dependencies change) and then add getghfriends to the useEffect deps, or move the getghfriends logic inline inside the useEffect and keep the dependency array as [isAuthenticated]; ensure setGithubfriends and setGHshowfriends remain available from React state and update only within the effect or the memoized function.
46-46: Unused destructured variables fromuseSidebar().
openandisMobileare destructured but no longer used after the layout simplification. Consider removing them to avoid confusion.- const { open, isMobile } = useSidebar(); + const { } = useSidebar();Or remove the import entirely if no sidebar context is needed.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@app/src/components/GithubTable.jsx` at line 46, Remove the unused destructured variables from useSidebar() in GithubTable.jsx: delete "open" and "isMobile" from the const { open, isMobile } = useSidebar() call (or remove the useSidebar() call/import entirely if no sidebar context is needed anywhere in this component) so the component no longer declares unused variables and the import is cleaned up accordingly.app/src/components/AtcoderTable.jsx (3)
16-16: Unused destructured variables fromuseSidebar().
openandisMobileare not used. Consider removing.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@app/src/components/AtcoderTable.jsx` at line 16, The destructured variables open and isMobile from useSidebar() in AtcoderTable.jsx are unused; remove them by changing the destructuring to only grab the needed value(s) (e.g., const { /* neededProp */ } = useSidebar() or simply const sidebar = useSidebar() if you need the whole object) or call useSidebar() without destructuring if nothing is needed; update any references to open or isMobile (none expected) and run lint to ensure no remaining unused variable warnings.
6-11: Unused imports:Avatar,AvatarFallback,AvatarImage,User.These are imported but not used in this component's column definitions.
-import { - Avatar, - AvatarFallback, - AvatarImage, -} from "@/components/ui/avatar"; -import { User } from "lucide-react";🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@app/src/components/AtcoderTable.jsx` around lines 6 - 11, The file imports Avatar, AvatarFallback, AvatarImage and User but they are not referenced in the AtcoderTable component's column definitions; remove these unused imports (Avatar, AvatarFallback, AvatarImage, User) from the top of AtcoderTable.jsx or replace the import with only symbols actually used by the component so the linter warning goes away and dead imports are eliminated.
102-102: Avoid full page reload after adding user.
window.location.reload()provides poor UX. Consider lifting state up or using a callback prop to trigger a data refetch instead.- window.location.reload(); + // TODO: Trigger parent refresh via callback prop + // onUserAdded?.();🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@app/src/components/AtcoderTable.jsx` at line 102, AtcoderTable currently forces a full page reload via window.location.reload() after adding a user; remove that call and instead invoke a passed-in callback or trigger a local state update to refresh the list. Update the add-user handler (e.g., the function containing window.location.reload()) to call a prop like onUserAdded or call a local fetchUsers/refreshData method after the add completes, or lift the users state up and update it from the parent so the UI re-renders without a reload. Ensure the component accepts the callback prop (onUserAdded/refreshUsers) and uses it in place of window.location.reload() so new data is fetched/reconciled gracefully.app/src/components/Register.jsx (1)
43-43: Unused destructured variables fromuseSidebar().
openandisMobileare destructured but no longer used after removing dynamic width styling.- const { open, isMobile } = useSidebar();Consider removing this line and the import if sidebar context is not needed elsewhere.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@app/src/components/Register.jsx` at line 43, The destructured variables open and isMobile from useSidebar() in Register.jsx are unused; remove the unnecessary destructuring (and the useSidebar import if it becomes unused) to eliminate dead code. Locate the line "const { open, isMobile } = useSidebar();" and replace it with either a no-op call to useSidebar() if side effects are needed or remove the call entirely, and then remove the corresponding import of useSidebar from the top of the file if nothing else uses it.app/src/components/LeetcodeTable.jsx (3)
216-223: Missinggetltfriendsin useEffect dependency array.Similar to GithubTable, the async function
getltfriendsis called but not in deps, which may cause stale closure issues.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@app/src/components/LeetcodeTable.jsx` around lines 216 - 223, The useEffect in LeetcodeTable.jsx references the async function getltfriends but doesn't include it in the dependency array, risking stale closures; either add getltfriends to the dependency array of the useEffect([isAuthenticated, getltfriends]) or memoize getltfriends with useCallback (so its identity is stable) and then include that memoized function in the deps; keep existing calls to setLeetcodefriends and setLTshowfriends and the isAuthenticated check unchanged.
46-46: Unused destructured variables fromuseSidebar().
openandisMobileare no longer used after the layout change. Consider removing.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@app/src/components/LeetcodeTable.jsx` at line 46, The destructured variables open and isMobile from useSidebar() are unused; update the call in LeetcodeTable.jsx by removing those identifiers so you only extract what’s needed (e.g., const { /*keep only used props*/ } = useSidebar() or simply call useSidebar() without destructuring), and ensure there are no remaining references to open or isMobile in functions or JSX (check any usages in render, handlers, or effects that might need refactoring or safe removal).
208-210: Misleading variable namefruitin filter callback.Same issue as in GithubTable. Rename to
friendorusernamefor clarity.setLeetcodefriends((current) => - current.filter((fruit) => fruit !== e), + current.filter((friend) => friend !== e), );🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@app/src/components/LeetcodeTable.jsx` around lines 208 - 210, The filter callback inside setLeetcodefriends uses a misleading parameter name `fruit`; change it to a clearer name like `friend` or `username` and update any references in the callback (e.g., the equality check against `e`) so the code reads setLeetcodefriends((current) => current.filter((friend) => friend !== e)); this makes the intent in LeetcodeTable.jsx and the setLeetcodefriends state updater clearer and consistent with GithubTable.app/src/components/Login.jsx (1)
31-31: Unused destructured variables fromuseSidebar().
openandisMobileare no longer used. Consider removing the destructuring and potentially the import.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@app/src/components/Login.jsx` at line 31, The destructured variables open and isMobile from useSidebar() in the Login component are unused; remove them from the destructuring (replace "const { open, isMobile } = useSidebar();" with a call that only uses needed values or simply "useSidebar();" if no values are needed) and remove the useSidebar import if nothing from it is used elsewhere in Login.jsx; ensure you only keep the specific hook usage required by the component (e.g., keep useSidebar() or specific properties like isOpen if actually used).app/src/components/Heatmap.jsx (2)
21-33: Missing dependencies in useEffect.The effect references
fetchGitHubContributions,fetchCodeforcesData, andgenerateMockDatabut doesn't include them in the dependency array. This can cause stale closures. Consider usinguseCallbackfor these functions or restructuring to avoid the warning.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@app/src/components/Heatmap.jsx` around lines 21 - 33, The useEffect in Heatmap.jsx references fetchGitHubContributions, fetchCodeforcesData, and generateMockData but doesn't list them in the dependency array, causing stale closures; fix by either wrapping those functions in useCallback (e.g., const fetchGitHubContributions = useCallback(..., [itsDeps]), const fetchCodeforcesData = useCallback(..., [itsDeps]), const generateMockData = useCallback(..., [itsDeps])) and then adding them to the useEffect dependency array, or move their implementations directly inside the useEffect so the effect only depends on platform and username; ensure any internal state or props those functions use are included in their useCallback dependency arrays.
227-239: Remove commented-out debug logs.These commented console.log statements are debug artifacts that should be removed rather than left in the codebase.
if (level > 0) { streak++; newStreakCells.add(i); - // console.log(`Streak incremented at index ${i}, level: ${level}, streak: ${streak}`); } else { if (i === flatData.length - 1) { - // Skip today if no activity yet - // console.log(`Today (index ${i}) has no activity, continuing streak check...`); continue; } else { - // console.log(`Streak broken at index ${i} (yesterday or older), level: ${level}`); break; } } } - // console.log(`Final calculated streak for ${platform}: ${streak}`); setCurrentStreak(streak);🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@app/src/components/Heatmap.jsx` around lines 227 - 239, In the Heatmap component's streak-calculation loop (the code referencing flatData, index i, level and streak), remove all commented-out console.log debug lines and any adjacent commented debug messages (e.g., "// console.log(`Streak incremented...`)", "// console.log(`Today (index ${i})...`)", "// console.log(`Streak broken...`)", "// console.log(`Final calculated streak...`)") so the loop contains only active logic and necessary comments.app/src/App.css (1)
6-16: Replaceheight: 100vhwithmin-height: 100dvhfor better mobile viewport handling.Setting
height: 100vhon.Appcauses issues on mobile browsers where100vhdoesn't account for the dynamic address bar. Usemin-height: 100dvhinstead—the dynamic viewport height unit is now broadly supported (94%+ of browsers as of 2026) and designed specifically for this problem.Additionally,
overflow-x: hiddenonhtml, bodycan hide overflowing content rather than addressing underlying layout issues. Ensure this is intentional.Suggested fix
.App { overflow-x: hidden; max-width: 100%; - height: 100vh; + min-height: 100dvh; overflow-y: auto; }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@app/src/App.css` around lines 6 - 16, Replace the hard height on the root app container and reconsider global overflow: for the .App selector change the fixed "height: 100vh" to "min-height: 100dvh" to use the dynamic viewport height on mobile (keep existing overflow-y: auto); review the global "html, body { overflow-x: hidden; max-width: 100vw; }" usage and remove or narrow it if it was masking layout/overflow bugs (or add a comment explaining it's intentional) so the root/container selectors reflect the intended behavior.app/src/components/Profile.jsx (1)
35-35: Remove unuseduseSidebarhook call.The destructured values
openandisMobileare no longer used after removing the dynamic width calculation. Remove the unused hook call to avoid unnecessary re-renders and improve code clarity.♻️ Proposed fix
-import { useSidebar } from "@/components/ui/sidebar"; ... - const { open, isMobile } = useSidebar();🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@app/src/components/Profile.jsx` at line 35, Remove the unused useSidebar hook call in Profile.jsx: the destructured values open and isMobile are no longer referenced, so delete the line "const { open, isMobile } = useSidebar();" (and the import of useSidebar if it becomes unused) to prevent unnecessary re-renders and clean up the component. Ensure no other references to open or isMobile remain before removing the import.api/leaderboard/views.py (1)
112-113: Prefer catching specific exceptions.Catching broad
Exceptioncan mask unexpected errors. Consider catching specific exceptions likerequests.RequestExceptionfor network errors.♻️ Proposed fix
- except Exception as e: - logger.error(f"Failed to fetch GitHub data for {user.username}: {e}") + except requests.RequestException as e: + logger.error(f"Failed to fetch GitHub data for {user.username}: {e}") + except (KeyError, TypeError) as e: + logger.error(f"Invalid GitHub data format for {user.username}: {e}")🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@api/leaderboard/views.py` around lines 112 - 113, The current except block catches a broad Exception for the GitHub fetch and should be narrowed: replace "except Exception as e" with specific network/HTTP exceptions such as "except requests.RequestException as e" (optionally also handle requests.HTTPError, requests.Timeout, requests.ConnectionError separately if you want different messages), ensure the requests module is imported, keep the existing logger.error(f"Failed to fetch GitHub data for {user.username}: {e}") inside that specific except, and allow other unexpected exceptions to propagate (or re-raise) so they aren't silently swallowed.app/src/components/CodeforcesTable.jsx (2)
62-62: Remove debug console.log statements.Multiple
console.logstatements (lines 62, 80, 89, 95, 128, 140, 149) should be removed before merging to avoid polluting browser console in production.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@app/src/components/CodeforcesTable.jsx` at line 62, Remove all development console.log calls in the CodeforcesTable component (e.g., the logs at lines 62, 80, 89, 95, 128, 140, 149) so they don't leak to production; locate the prints inside the CodeforcesTable component (within its useEffect hooks and any helper functions that fetch or process userData) and either delete the console.log statements or wrap them behind a NODE_ENV check (e.g., only call console.debug when process.env.NODE_ENV !== 'production') or replace with a proper logger if available.
26-26: Remove unuseduseSidebarhook call.The destructured values
openandisMobileare no longer used after removing the dynamic width calculation.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@app/src/components/CodeforcesTable.jsx` at line 26, Remove the unused useSidebar hook invocation in CodeforcesTable.jsx: delete the line "const { open, isMobile } = useSidebar();" and any related destructuring, and also remove the now-unused useSidebar import (and its import specifier) from the top of the file to avoid unused-import warnings.app/src/components/CodechefTable.jsx (1)
36-36: Remove unuseduseSidebarhook call.The destructured values
openandisMobileare no longer used after removing the dynamic width calculation.♻️ Proposed fix
-import { useSidebar } from "@/components/ui/sidebar"; ... - const { open, isMobile } = useSidebar();🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@app/src/components/CodechefTable.jsx` at line 36, In the CodechefTable component remove the unused useSidebar hook call and its destructuring (the line "const { open, isMobile } = useSidebar();") and also remove the corresponding import of useSidebar; this cleans up unused variables in CodechefTable.jsx and prevents importing a hook that is no longer referenced.app/src/components/Navbar.jsx (1)
212-212: Simplify redundant nullish coalescing.
(user ?? false)is equivalent to justuserin a boolean/ternary context since falsy values already evaluate correctly.✨ Proposed simplification
- {(user ?? false) ? ( + {user ? (🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@app/src/components/Navbar.jsx` at line 212, The JSX conditional uses a redundant nullish coalescing expression "(user ?? false)" — replace it with the simpler truthy check "user" wherever it appears in the Navbar component's JSX conditional rendering so the ternary or logical rendering uses "user" directly (look for the JSX expression using (user ?? false) in Navbar.jsx and update it to user).app/src/components/NavMenu.jsx (2)
75-75: Redundant condition:globalStreak >= 0is always true.Since
globalStreakis computed withMath.max(..., 0), it will always be >= 0. Theuser &&check is sufficient.✨ Proposed simplification
- {user && globalStreak >= 0 && ( + {user && (🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@app/src/components/NavMenu.jsx` at line 75, Remove the redundant `globalStreak >= 0` check in the NavMenu JSX conditional — since `globalStreak` is already computed with `Math.max(..., 0)` it will always be >= 0; update the conditional expression `{user && globalStreak >= 0 && (...)}` to simply `{user && (...)}` referencing the `globalStreak` variable and the JSX block in NavMenu.jsx so the UI logic relies only on `user` for rendering.
62-69: Search input appears non-functional.The search input is rendered but has no
onChangehandler or associated search logic. Consider either implementing search functionality or removing the placeholder input to avoid user confusion.Would you like me to help implement a basic search functionality, or should this be tracked as a TODO for a future PR?
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@app/src/components/NavMenu.jsx` around lines 62 - 69, The Search input in NavMenu.jsx is rendered but not wired up; add a controlled state (e.g., const [searchQuery, setSearchQuery] = useState('')) and an onChange handler (e.g., function handleSearchChange(e) { setSearchQuery(e.target.value); }) and pass it to the Input as onChange={handleSearchChange} and value={searchQuery}; then either call a search function from that handler (or a debounced wrapper) such as fetchSearchResults(searchQuery) or invoke a prop callback (e.g., props.onSearch(searchQuery)) to perform filtering/fetching, and remove the input entirely if you prefer to postpone implementing search. Ensure you reference the Input component and the NavMenu.jsx component's user conditional when making the change.app/package.json (1)
44-44: Consider removing redundantradix-uimeta-package.You're already using individual
@radix-ui/*packages (lines 13-23). Theradix-uimeta-package bundles all Radix components, which increases bundle size unnecessarily. If you only need specific components, the individual packages you already have are sufficient.🗑️ Proposed fix
- "radix-ui": "^1.4.3",🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@app/package.json` at line 44, The package.json currently includes the redundant meta-package "radix-ui" which duplicates the already listed individual `@radix-ui/`* packages and increases bundle size; remove the "radix-ui" dependency entry from package.json (the line with "radix-ui": "^1.4.3") so only the specific `@radix-ui/`* packages (those listed earlier) remain, then run your package manager install to update lockfile and verify builds/tests pass.app/src/components/PlatformStreakFetcher.jsx (1)
40-64: Consider extracting shared streak calculation logic.The streak calculation algorithm is nearly identical across
fetchLeetcodeStreak,fetchAtcoderStreak, andfetchCodechefStreak. Extracting a shared helper would reduce duplication and ensure consistent behavior.♻️ Example helper extraction
// Helper to calculate streak from sorted descending day timestamps function calculateStreakFromDays(sortedDays, now) { let streak = 0; for (let i = 0; i < sortedDays.length; i++) { const dayTime = sortedDays[i]; const dayDiff = Math.floor((now.getTime() - dayTime) / (1000 * 60 * 60 * 24)); if (streak === 0) { if (dayDiff <= 1) { streak = 1; } else { break; } } else { const prevTime = sortedDays[i - 1]; const roundedDiff = Math.round((prevTime - dayTime) / (1000 * 60 * 60 * 24)); if (roundedDiff === 1) { streak++; } else if (roundedDiff === 0) { continue; } else { break; } } } return streak; }Also applies to: 100-123, 154-181
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@app/src/components/PlatformStreakFetcher.jsx` around lines 40 - 64, Extract the duplicated streak loop into a shared helper (e.g., calculateStreakFromDays) and replace the logic inside fetchLeetcodeStreak, fetchAtcoderStreak, and fetchCodechefStreak with calls to that helper; the helper should accept an array of sorted descending day timestamps (or numeric day keys converted to ms) and a Date/now value, implement the same logic currently in the loop (initial day check using dayDiff <= 1, skipping same-day entries, incrementing on roundedDiff === 1, breaking otherwise) and return the computed streak number so each fetch* function simply prepares the sorted timestamps and returns calculateStreakFromDays(sortedDays, now).app/src/components/HomePage.jsx (1)
37-37: Useconstinstead ofvarfor module-level constant.
rankis never reassigned, so useconst. If this value should be dynamic, consider moving it into component state or computing it from props.✨ Proposed fix
-var rank = 0; +const rank = 0;🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@app/src/components/HomePage.jsx` at line 37, The module-level variable "rank" is declared with var but never reassigned; update its declaration to use const (replace the "var rank = 0;" declaration with "const rank = 0;") in HomePage.jsx, or if rank is meant to change, move it into component state (e.g., useState inside the relevant component) or compute it from props instead.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@app/src/components/Navbar.jsx`:
- Line 189: The Link in Navbar.jsx uses a relative path via to={linkLower};
update it to use an absolute route by prepending a slash (i.e., set the Link's
to prop to `/${linkLower}`) so it matches the isActive check pattern used
earlier (see the other use of `/${linkLower}`) and prevents relative routing
issues; locate the Link element that references linkLower and change its to
value accordingly.
- Around line 118-127: The off-by-one bug prevents the Blogs spacing because
isBlogs is set with index === 7 but Blogs is at index 6; update the condition in
the map (where isActive and isBlogs are computed for items.map) to correctly
identify Blogs — either change index === 7 to index === 6 or, more robustly,
check the item identity (e.g., item.title === "Blogs" or item.url === "/blogs")
before passing the className to SidebarMenuItem so the "mt-8" spacing is applied
to the Blogs entry.
In `@app/src/components/PlatformStreakFetcher.jsx`:
- Around line 134-190: The fetchCodechefStreak function assumes data.heatMap
exists (so streak logic never runs); fix by either (A) switching the fetch URL
to an endpoint that returns submission/activity dates and update
fetchCodechefStreak to parse that response (e.g., request an
activity/submissions API that returns dates and build the sortedEntries from
that), or (B) change the backend API to include a heatMap/activity array for the
existing endpoint so fetchCodechefStreak can use it; in either case remove the
data.heatMap guard and instead parse the actual activity field returned (or
return early with updateStreak(platform, 0) only when that activity array is
absent/empty), keeping function name fetchCodechefStreak and final
updateStreak(platform, streak) usage. Ensure error handling stays and add a
small comment in fetchCodechefStreak indicating which field it expects from the
API.
In `@app/src/index.css`:
- Around line 5-14: The css sets html, body { overflow: hidden } which prevents
window.scrollTo in GoToTop.jsx from working because the scrollable container is
.App (overflow-y: auto, height: 100vh); fix by either removing overflow: hidden
from the html, body rule in index.css so window scrolling works, or modify
GoToTop.jsx to target the .App element (query/select the element with class
"App" and call its scrollTo or set scrollTop with smooth behavior) instead of
calling window.scrollTo; update references to window.scrollTo({ left:0, top:0,
behavior:"smooth" }) to use the .App element's scrolling API when choosing that
approach.
In `@README.md`:
- Line 18: Fix the typo and improve readability in the README sentence by
changing "there achievements" to "their achievements" and replacing "on the
basis of" with "based on" so the line reads e.g. "Students can also login and
view their achievements based on previous accomplishments"; update the exact
sentence in README.md accordingly.
---
Outside diff comments:
In `@api/leaderboard/views.py`:
- Around line 125-146: The current check-then-create in the view
(githubUser.exists() followed by creating githubUser and gh_user.save()) is
vulnerable to a TOCTOU race; replace this pattern with an atomic operation such
as githubUser.objects.get_or_create(...) or
githubUser.objects.update_or_create(...) when creating the record after calling
fetch_github_data(username), and handle the returned (instance, created) tuple
appropriately (or catch IntegrityError around save() as a fallback). Ensure you
populate fields (avatar, repositories, stars, contributions, last_updated) in
the get_or_create/update_or_create call or update the instance when
created==False to avoid duplicate creation errors.
In `@app/src/components/CodechefTable.jsx`:
- Around line 70-78: The profile anchor in the CodechefTable.jsx component
currently builds URLs using the wrong domain
(https://codeforces.com/profile/${username}); update the href to use the
CodeChef domain (https://www.codechef.com/users/${username}) so usernames link
to the correct profiles—locate the anchor inside the Button (variant="link"
asChild) in the CodechefTable component and change the URL template accordingly,
keeping target="_blank" and rel="noreferrer" behavior intact.
In `@app/src/components/CodeforcesTable.jsx`:
- Around line 96-99: Replace the unsupported { timeout: 10000 } option in the
fetch call that creates cfResponse with an AbortController-based timeout; create
a helper like fetchWithTimeout (or inline controller logic) that constructs an
AbortController, sets a setTimeout to controller.abort() after the desired ms,
calls fetch(url, { signal: controller.signal }), clears the timeout on success
or failure, and rethrows errors so callers of the function in
CodeforcesTable.jsx can handle AbortError/timeouts appropriately.
In `@app/src/components/Heatmap.jsx`:
- Around line 142-188: processGitHubData builds heatmapArray but never updates
the streak like processCodeforcesData does; after setHeatmapData(heatmapArray)
call calculateStreak(heatmapArray) to compute and update the streak display
(ensure you call the same calculateStreak function used by processCodeforcesData
so GitHub contributions update the streak UI).
---
Nitpick comments:
In `@api/leaderboard/views.py`:
- Around line 112-113: The current except block catches a broad Exception for
the GitHub fetch and should be narrowed: replace "except Exception as e" with
specific network/HTTP exceptions such as "except requests.RequestException as e"
(optionally also handle requests.HTTPError, requests.Timeout,
requests.ConnectionError separately if you want different messages), ensure the
requests module is imported, keep the existing logger.error(f"Failed to fetch
GitHub data for {user.username}: {e}") inside that specific except, and allow
other unexpected exceptions to propagate (or re-raise) so they aren't silently
swallowed.
In `@app/package.json`:
- Line 44: The package.json currently includes the redundant meta-package
"radix-ui" which duplicates the already listed individual `@radix-ui/`* packages
and increases bundle size; remove the "radix-ui" dependency entry from
package.json (the line with "radix-ui": "^1.4.3") so only the specific
`@radix-ui/`* packages (those listed earlier) remain, then run your package
manager install to update lockfile and verify builds/tests pass.
In `@app/src/App.css`:
- Around line 6-16: Replace the hard height on the root app container and
reconsider global overflow: for the .App selector change the fixed "height:
100vh" to "min-height: 100dvh" to use the dynamic viewport height on mobile
(keep existing overflow-y: auto); review the global "html, body { overflow-x:
hidden; max-width: 100vw; }" usage and remove or narrow it if it was masking
layout/overflow bugs (or add a comment explaining it's intentional) so the
root/container selectors reflect the intended behavior.
In `@app/src/components/AtcoderTable.jsx`:
- Line 16: The destructured variables open and isMobile from useSidebar() in
AtcoderTable.jsx are unused; remove them by changing the destructuring to only
grab the needed value(s) (e.g., const { /* neededProp */ } = useSidebar() or
simply const sidebar = useSidebar() if you need the whole object) or call
useSidebar() without destructuring if nothing is needed; update any references
to open or isMobile (none expected) and run lint to ensure no remaining unused
variable warnings.
- Around line 6-11: The file imports Avatar, AvatarFallback, AvatarImage and
User but they are not referenced in the AtcoderTable component's column
definitions; remove these unused imports (Avatar, AvatarFallback, AvatarImage,
User) from the top of AtcoderTable.jsx or replace the import with only symbols
actually used by the component so the linter warning goes away and dead imports
are eliminated.
- Line 102: AtcoderTable currently forces a full page reload via
window.location.reload() after adding a user; remove that call and instead
invoke a passed-in callback or trigger a local state update to refresh the list.
Update the add-user handler (e.g., the function containing
window.location.reload()) to call a prop like onUserAdded or call a local
fetchUsers/refreshData method after the add completes, or lift the users state
up and update it from the parent so the UI re-renders without a reload. Ensure
the component accepts the callback prop (onUserAdded/refreshUsers) and uses it
in place of window.location.reload() so new data is fetched/reconciled
gracefully.
In `@app/src/components/CodechefTable.jsx`:
- Line 36: In the CodechefTable component remove the unused useSidebar hook call
and its destructuring (the line "const { open, isMobile } = useSidebar();") and
also remove the corresponding import of useSidebar; this cleans up unused
variables in CodechefTable.jsx and prevents importing a hook that is no longer
referenced.
In `@app/src/components/CodeforcesTable.jsx`:
- Line 62: Remove all development console.log calls in the CodeforcesTable
component (e.g., the logs at lines 62, 80, 89, 95, 128, 140, 149) so they don't
leak to production; locate the prints inside the CodeforcesTable component
(within its useEffect hooks and any helper functions that fetch or process
userData) and either delete the console.log statements or wrap them behind a
NODE_ENV check (e.g., only call console.debug when process.env.NODE_ENV !==
'production') or replace with a proper logger if available.
- Line 26: Remove the unused useSidebar hook invocation in CodeforcesTable.jsx:
delete the line "const { open, isMobile } = useSidebar();" and any related
destructuring, and also remove the now-unused useSidebar import (and its import
specifier) from the top of the file to avoid unused-import warnings.
In `@app/src/components/GithubTable.jsx`:
- Around line 197-199: The filter callback in the setGithubfriends update uses a
misleading name "fruit"; update the callback parameter to a meaningful
identifier (e.g., "friend" or "username") so the code reads
setGithubfriends((current) => current.filter((friend) => friend !== e)); locate
this inside the component where setGithubfriends is called and replace "fruit"
with the chosen identifier in that callback only.
- Around line 205-212: The effect calls getghfriends but doesn’t include it in
the dependency array, which can cause stale closures; either wrap getghfriends
in useCallback (e.g., memoize the getghfriends function so it only changes when
its real dependencies change) and then add getghfriends to the useEffect deps,
or move the getghfriends logic inline inside the useEffect and keep the
dependency array as [isAuthenticated]; ensure setGithubfriends and
setGHshowfriends remain available from React state and update only within the
effect or the memoized function.
- Line 46: Remove the unused destructured variables from useSidebar() in
GithubTable.jsx: delete "open" and "isMobile" from the const { open, isMobile }
= useSidebar() call (or remove the useSidebar() call/import entirely if no
sidebar context is needed anywhere in this component) so the component no longer
declares unused variables and the import is cleaned up accordingly.
In `@app/src/components/GoToTop.jsx`:
- Around line 9-28: The JSX in GoToTop.jsx wraps a single button element in an
unnecessary React Fragment; remove the surrounding <>...</> and return the
<button> directly from the component (keep the existing props, inline styles,
onClick handler gotop, and child ArrowUp unchanged) so the component renders the
button without the redundant fragment.
In `@app/src/components/Heatmap.jsx`:
- Around line 21-33: The useEffect in Heatmap.jsx references
fetchGitHubContributions, fetchCodeforcesData, and generateMockData but doesn't
list them in the dependency array, causing stale closures; fix by either
wrapping those functions in useCallback (e.g., const fetchGitHubContributions =
useCallback(..., [itsDeps]), const fetchCodeforcesData = useCallback(...,
[itsDeps]), const generateMockData = useCallback(..., [itsDeps])) and then
adding them to the useEffect dependency array, or move their implementations
directly inside the useEffect so the effect only depends on platform and
username; ensure any internal state or props those functions use are included in
their useCallback dependency arrays.
- Around line 227-239: In the Heatmap component's streak-calculation loop (the
code referencing flatData, index i, level and streak), remove all commented-out
console.log debug lines and any adjacent commented debug messages (e.g., "//
console.log(`Streak incremented...`)", "// console.log(`Today (index
${i})...`)", "// console.log(`Streak broken...`)", "// console.log(`Final
calculated streak...`)") so the loop contains only active logic and necessary
comments.
In `@app/src/components/HomePage.jsx`:
- Line 37: The module-level variable "rank" is declared with var but never
reassigned; update its declaration to use const (replace the "var rank = 0;"
declaration with "const rank = 0;") in HomePage.jsx, or if rank is meant to
change, move it into component state (e.g., useState inside the relevant
component) or compute it from props instead.
In `@app/src/components/LeetcodeTable.jsx`:
- Around line 216-223: The useEffect in LeetcodeTable.jsx references the async
function getltfriends but doesn't include it in the dependency array, risking
stale closures; either add getltfriends to the dependency array of the
useEffect([isAuthenticated, getltfriends]) or memoize getltfriends with
useCallback (so its identity is stable) and then include that memoized function
in the deps; keep existing calls to setLeetcodefriends and setLTshowfriends and
the isAuthenticated check unchanged.
- Line 46: The destructured variables open and isMobile from useSidebar() are
unused; update the call in LeetcodeTable.jsx by removing those identifiers so
you only extract what’s needed (e.g., const { /*keep only used props*/ } =
useSidebar() or simply call useSidebar() without destructuring), and ensure
there are no remaining references to open or isMobile in functions or JSX (check
any usages in render, handlers, or effects that might need refactoring or safe
removal).
- Around line 208-210: The filter callback inside setLeetcodefriends uses a
misleading parameter name `fruit`; change it to a clearer name like `friend` or
`username` and update any references in the callback (e.g., the equality check
against `e`) so the code reads setLeetcodefriends((current) =>
current.filter((friend) => friend !== e)); this makes the intent in
LeetcodeTable.jsx and the setLeetcodefriends state updater clearer and
consistent with GithubTable.
In `@app/src/components/Login.jsx`:
- Line 31: The destructured variables open and isMobile from useSidebar() in the
Login component are unused; remove them from the destructuring (replace "const {
open, isMobile } = useSidebar();" with a call that only uses needed values or
simply "useSidebar();" if no values are needed) and remove the useSidebar import
if nothing from it is used elsewhere in Login.jsx; ensure you only keep the
specific hook usage required by the component (e.g., keep useSidebar() or
specific properties like isOpen if actually used).
In `@app/src/components/Navbar.jsx`:
- Line 212: The JSX conditional uses a redundant nullish coalescing expression
"(user ?? false)" — replace it with the simpler truthy check "user" wherever it
appears in the Navbar component's JSX conditional rendering so the ternary or
logical rendering uses "user" directly (look for the JSX expression using (user
?? false) in Navbar.jsx and update it to user).
In `@app/src/components/NavMenu.jsx`:
- Line 75: Remove the redundant `globalStreak >= 0` check in the NavMenu JSX
conditional — since `globalStreak` is already computed with `Math.max(..., 0)`
it will always be >= 0; update the conditional expression `{user && globalStreak
>= 0 && (...)}` to simply `{user && (...)}` referencing the `globalStreak`
variable and the JSX block in NavMenu.jsx so the UI logic relies only on `user`
for rendering.
- Around line 62-69: The Search input in NavMenu.jsx is rendered but not wired
up; add a controlled state (e.g., const [searchQuery, setSearchQuery] =
useState('')) and an onChange handler (e.g., function handleSearchChange(e) {
setSearchQuery(e.target.value); }) and pass it to the Input as
onChange={handleSearchChange} and value={searchQuery}; then either call a search
function from that handler (or a debounced wrapper) such as
fetchSearchResults(searchQuery) or invoke a prop callback (e.g.,
props.onSearch(searchQuery)) to perform filtering/fetching, and remove the input
entirely if you prefer to postpone implementing search. Ensure you reference the
Input component and the NavMenu.jsx component's user conditional when making the
change.
In `@app/src/components/PlatformStreakFetcher.jsx`:
- Around line 40-64: Extract the duplicated streak loop into a shared helper
(e.g., calculateStreakFromDays) and replace the logic inside
fetchLeetcodeStreak, fetchAtcoderStreak, and fetchCodechefStreak with calls to
that helper; the helper should accept an array of sorted descending day
timestamps (or numeric day keys converted to ms) and a Date/now value, implement
the same logic currently in the loop (initial day check using dayDiff <= 1,
skipping same-day entries, incrementing on roundedDiff === 1, breaking
otherwise) and return the computed streak number so each fetch* function simply
prepares the sorted timestamps and returns calculateStreakFromDays(sortedDays,
now).
In `@app/src/components/Profile.jsx`:
- Line 35: Remove the unused useSidebar hook call in Profile.jsx: the
destructured values open and isMobile are no longer referenced, so delete the
line "const { open, isMobile } = useSidebar();" (and the import of useSidebar if
it becomes unused) to prevent unnecessary re-renders and clean up the component.
Ensure no other references to open or isMobile remain before removing the
import.
In `@app/src/components/Register.jsx`:
- Line 43: The destructured variables open and isMobile from useSidebar() in
Register.jsx are unused; remove the unnecessary destructuring (and the
useSidebar import if it becomes unused) to eliminate dead code. Locate the line
"const { open, isMobile } = useSidebar();" and replace it with either a no-op
call to useSidebar() if side effects are needed or remove the call entirely, and
then remove the corresponding import of useSidebar from the top of the file if
nothing else uses it.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: d7d77988-da8e-4a9d-b2d3-c04504fe2658
⛔ Files ignored due to path filters (1)
app/pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (24)
README.mdapi/leaderboard/models.pyapi/leaderboard/users.pyapi/leaderboard/views.pyapp/.env.frontendapp/package.jsonapp/src/App.cssapp/src/App.jsxapp/src/components/AtcoderTable.jsxapp/src/components/CodechefTable.jsxapp/src/components/CodeforcesTable.jsxapp/src/components/GithubTable.jsxapp/src/components/GoToTop.jsxapp/src/components/Heatmap.jsxapp/src/components/HomePage.jsxapp/src/components/LeetcodeTable.jsxapp/src/components/Login.jsxapp/src/components/NavMenu.jsxapp/src/components/Navbar.jsxapp/src/components/PlatformStreakFetcher.jsxapp/src/components/Profile.jsxapp/src/components/Register.jsxapp/src/components/ui/sidebar.jsxapp/src/index.css
💤 Files with no reviewable changes (1)
- app/.env.frontend
Comment on lines
+118
to
+127
| {items.map((item, index) => { | ||
| const isActive = pathname === item.url; | ||
| const isBlogs = index === 7; | ||
| return ( | ||
| <SidebarMenuItem | ||
| key={item.title} | ||
| className={cn( | ||
| "relative", | ||
| isBlogs && "mt-8" // Add distance before Blogs | ||
| )} |
Contributor
There was a problem hiding this comment.
Off-by-one error: Blogs is at index 6, not 7.
The items array has 7 elements (indices 0-6). Blogs is at index 6, so index === 7 will never be true and the mt-8 spacing will never be applied.
🐛 Proposed fix
{items.map((item, index) => {
const isActive = pathname === item.url;
- const isBlogs = index === 7;
+ const isBlogs = index === 6;
return (📝 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.
Suggested change
| {items.map((item, index) => { | |
| const isActive = pathname === item.url; | |
| const isBlogs = index === 7; | |
| return ( | |
| <SidebarMenuItem | |
| key={item.title} | |
| className={cn( | |
| "relative", | |
| isBlogs && "mt-8" // Add distance before Blogs | |
| )} | |
| {items.map((item, index) => { | |
| const isActive = pathname === item.url; | |
| const isBlogs = index === 6; | |
| return ( | |
| <SidebarMenuItem | |
| key={item.title} | |
| className={cn( | |
| "relative", | |
| isBlogs && "mt-8" // Add distance before Blogs | |
| )} |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@app/src/components/Navbar.jsx` around lines 118 - 127, The off-by-one bug
prevents the Blogs spacing because isBlogs is set with index === 7 but Blogs is
at index 6; update the condition in the map (where isActive and isBlogs are
computed for items.map) to correctly identify Blogs — either change index === 7
to index === 6 or, more robustly, check the item identity (e.g., item.title ===
"Blogs" or item.url === "/blogs") before passing the className to
SidebarMenuItem so the "mt-8" spacing is applied to the Blogs entry.
Comment on lines
+134
to
+190
| const fetchCodechefStreak = async () => { | ||
| try { | ||
| const response = await fetch(`https://codechef-api.vercel.app/handle/${username}`); | ||
| const data = await response.json(); | ||
|
|
||
| if (data && data.heatMap) { | ||
| // heatMap is an array of { date: "YYYY-MM-DD", value: number } | ||
| const sortedEntries = data.heatMap | ||
| .filter(entry => entry.value > 0) | ||
| .sort((a, b) => new Date(b.date) - new Date(a.date)); | ||
|
|
||
| if (sortedEntries.length === 0) { | ||
| updateStreak(platform, 0); | ||
| return; | ||
| } | ||
|
|
||
| let streak = 0; | ||
| const now = new Date(); | ||
| now.setHours(0, 0, 0, 0); | ||
|
|
||
| for (let i = 0; i < sortedEntries.length; i++) { | ||
| const entryDate = new Date(sortedEntries[i].date); | ||
| entryDate.setHours(0, 0, 0, 0); | ||
|
|
||
| if (streak === 0) { | ||
| const diffTime = now.getTime() - entryDate.getTime(); | ||
| const diffDays = Math.floor(diffTime / (1000 * 60 * 60 * 24)); | ||
|
|
||
| if (diffDays <= 1) { // Today or Yesterday | ||
| streak = 1; | ||
| } else { | ||
| break; | ||
| } | ||
| } else { | ||
| const prevDate = new Date(sortedEntries[i-1].date); | ||
| prevDate.setHours(0, 0, 0, 0); | ||
| const diffTime = prevDate.getTime() - entryDate.getTime(); | ||
| const diffDays = Math.round(diffTime / (1000 * 60 * 60 * 24)); | ||
|
|
||
| if (diffDays === 1) { | ||
| streak++; | ||
| } else if (diffDays === 0) { | ||
| continue; // Same day, skip | ||
| } else { | ||
| break; | ||
| } | ||
| } | ||
| } | ||
| updateStreak(platform, streak); | ||
| } else { | ||
| updateStreak(platform, 0); | ||
| } | ||
| } catch (error) { | ||
| console.error("Error fetching CodeChef streak:", error); | ||
| updateStreak(platform, 0); | ||
| } | ||
| }; |
Contributor
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Verify CodeChef API response structure
curl -s "https://codechef-api.vercel.app/handle/tourist" | jq 'keys'Repository: OpenLake/Leaderboard-Pro
Length of output: 124
🏁 Script executed:
# Get raw CodeChef API response
curl -s "https://codechef-api.vercel.app/handle/tourist"Repository: OpenLake/Leaderboard-Pro
Length of output: 145
🏁 Script executed:
# Search codebase for CodeChef API references and heatMap usage
rg -i "heatmap\|codechef.*api" --type js --type jsx -A 3 -B 3Repository: OpenLake/Leaderboard-Pro
Length of output: 95
🏁 Script executed:
# Check the views.py file mentioned in the review for context
cat -n api/leaderboard/views.py | sed -n '259,280p'Repository: OpenLake/Leaderboard-Pro
Length of output: 1091
🏁 Script executed:
# Search for heatMap references anywhere in the codebase
rg "heatMap|heatmap" --type-listRepository: OpenLake/Leaderboard-Pro
Length of output: 457
🏁 Script executed:
# Search for heatMap in all file types
find . -type f \( -name "*.js" -o -name "*.jsx" -o -name "*.py" -o -name "*.json" \) | xargs grep -l "heatMap\|heatmap" 2>/dev/nullRepository: OpenLake/Leaderboard-Pro
Length of output: 183
🏁 Script executed:
# Check PlatformStreakFetcher.jsx to see full context of all streak fetch functions
cat app/src/components/PlatformStreakFetcher.jsx | head -200Repository: OpenLake/Leaderboard-Pro
Length of output: 7382
CodeChef API does not return heatMap field — CodeChef streak will always be 0.
The CodeChef API endpoint (https://codechef-api.vercel.app/handle/{username}) returns user rating data (currentRating, highestRating, globalRank, countryRank, profile) as confirmed in api/leaderboard/views.py:263-277. It does not return a heatMap array. Since the frontend code checks for data.heatMap at line 139, this condition will always be false, causing the streak to always be set to 0 (line 161).
The CodeChef streak feature is fundamentally broken due to an API response mismatch. Either the API endpoint must be changed to return submission activity data, or the streak calculation logic must be rewritten to work with the available rating/ranking data.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@app/src/components/PlatformStreakFetcher.jsx` around lines 134 - 190, The
fetchCodechefStreak function assumes data.heatMap exists (so streak logic never
runs); fix by either (A) switching the fetch URL to an endpoint that returns
submission/activity dates and update fetchCodechefStreak to parse that response
(e.g., request an activity/submissions API that returns dates and build the
sortedEntries from that), or (B) change the backend API to include a
heatMap/activity array for the existing endpoint so fetchCodechefStreak can use
it; in either case remove the data.heatMap guard and instead parse the actual
activity field returned (or return early with updateStreak(platform, 0) only
when that activity array is absent/empty), keeping function name
fetchCodechefStreak and final updateStreak(platform, streak) usage. Ensure error
handling stays and add a small comment in fetchCodechefStreak indicating which
field it expects from the API.
Comment on lines
+5
to
+14
| html, body { | ||
| margin: 0; | ||
| font-family: | ||
| -apple-system, BlinkMacSystemFont, "Segoe UI", "Roboto", "Oxygen", | ||
| "Ubuntu", "Cantarell", "Fira Sans", "Droid Sans", "Helvetica Neue", | ||
| sans-serif; | ||
| -webkit-font-smoothing: antialiased; | ||
| -moz-osx-font-smoothing: grayscale; | ||
| overflow: hidden; | ||
| height: 100%; |
Contributor
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Verify GoToTop component uses window.scrollTo
rg -n "window.scrollTo" app/src/components/GoToTop.jsx
# Check if there are other components using window scroll APIs
rg -n "window\.(scrollTo|scroll|scrollY|scrollX|pageYOffset)" app/src/Repository: OpenLake/Leaderboard-Pro
Length of output: 226
🏁 Script executed:
cat app/src/App.css | head -50Repository: OpenLake/Leaderboard-Pro
Length of output: 745
🏁 Script executed:
# Check for scroll event listeners or other scroll-related code
rg -n "scroll" app/src/ --type css --type jsx --type js -i | head -30Repository: OpenLake/Leaderboard-Pro
Length of output: 95
🏁 Script executed:
# Check git diff for index.css
git diff HEAD app/src/index.cssRepository: OpenLake/Leaderboard-Pro
Length of output: 50
🏁 Script executed:
# Read full index.css to see complete context
cat app/src/index.cssRepository: OpenLake/Leaderboard-Pro
Length of output: 4569
🏁 Script executed:
cat public/index.htmlRepository: OpenLake/Leaderboard-Pro
Length of output: 117
🏁 Script executed:
# Check git log to see recent changes to index.css
git log --oneline -10 app/src/index.cssRepository: OpenLake/Leaderboard-Pro
Length of output: 77
🏁 Script executed:
# Find HTML files in the repository
find . -name "*.html" -type f | head -20Repository: OpenLake/Leaderboard-Pro
Length of output: 216
🏁 Script executed:
# Check the app structure - look for index.jsx or main entry point
find app/src -name "index.*" -o -name "App.*" | grep -E "\.(jsx|js|tsx|ts)$"Repository: OpenLake/Leaderboard-Pro
Length of output: 122
🏁 Script executed:
cat app/index.htmlRepository: OpenLake/Leaderboard-Pro
Length of output: 1423
🏁 Script executed:
head -20 app/src/index.jsxRepository: OpenLake/Leaderboard-Pro
Length of output: 472
🏁 Script executed:
# Check the GoToTop component to see the context
cat app/src/components/GoToTop.jsxRepository: OpenLake/Leaderboard-Pro
Length of output: 816
🏁 Script executed:
# Check if GoToTop is imported/used in App.jsx
rg -n "GoToTop" app/src/Repository: OpenLake/Leaderboard-Pro
Length of output: 291
window.scrollTo() will not work with current CSS configuration.
The html, body element has overflow: hidden, making it non-scrollable. The actual scroll container is .App which has overflow-y: auto and height: 100vh. When GoToTop.jsx calls window.scrollTo({ left: 0, top: 0, behavior: "smooth" }), it attempts to scroll the window/html element, which has overflow: hidden and therefore cannot scroll. The scroll-to-top button will silently fail.
To fix this, either:
- Remove
overflow: hiddenfromhtml, bodyinindex.css - Update
GoToTop.jsxto scroll.Appinstead ofwindow
🧰 Tools
🪛 Stylelint (17.4.0)
[error] 8-8: Unexpected quotes around "Roboto" (font-family-name-quotes)
(font-family-name-quotes)
[error] 8-8: Unexpected quotes around "Oxygen" (font-family-name-quotes)
(font-family-name-quotes)
[error] 9-9: Unexpected quotes around "Ubuntu" (font-family-name-quotes)
(font-family-name-quotes)
[error] 9-9: Unexpected quotes around "Cantarell" (font-family-name-quotes)
(font-family-name-quotes)
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@app/src/index.css` around lines 5 - 14, The css sets html, body { overflow:
hidden } which prevents window.scrollTo in GoToTop.jsx from working because the
scrollable container is .App (overflow-y: auto, height: 100vh); fix by either
removing overflow: hidden from the html, body rule in index.css so window
scrolling works, or modify GoToTop.jsx to target the .App element (query/select
the element with class "App" and call its scrollTo or set scrollTop with smooth
behavior) instead of calling window.scrollTo; update references to
window.scrollTo({ left:0, top:0, behavior:"smooth" }) to use the .App element's
scrolling API when choosing that approach.
This comment has been minimized.
This comment has been minimized.
Contributor
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (1)
api/leaderboard/views.py (1)
596-630: Extract the AtCoder scraper into one shared helper.This parsing logic is now duplicated almost verbatim here and in
api/leaderboard/celery.py:283-304. The next markup fix will have to land twice, and it is easy for the API path and worker path to drift again. Centralizing the scrape/parsing code will make future fixes much safer.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@api/leaderboard/views.py` around lines 596 - 630, The AtCoder scraping/parsing logic in get_atcoder_data is duplicated in the Celery worker; extract the shared HTML parsing into a single helper (e.g., parse_atcoder_profile or parse_atcoder_html) that accepts the page HTML or BeautifulSoup and returns the same instance dict keys ("rating","highest_rating","rank"), then update get_atcoder_data to call that helper after fetching page.text and update the duplicate code in the Celery task to reuse the helper too; ensure the new helper is placed in a common module imported by both (and update imports) so future markup fixes are applied once.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@api/leaderboard/celery.py`:
- Around line 283-304: The scrape loop builds an instance dict but may append
incomplete records; ensure the code validates that instance contains all
required keys ("rating", "highest_rating", "rank") before appending and treat a
missing field as a failure that aborts the Celery update (e.g., raise an
exception or return an error) so the fallback preserves the previous record;
update the logic around the instance append in the table-parsing block (and the
similar logic around lines handling the other table at 306-307) to perform this
validation and trigger failure when any of those three fields are absent.
In `@api/leaderboard/views.py`:
- Around line 637-646: The request handler is performing live AtCoder refreshes
by iterating active_users and calling get_atcoder_data when
AtcoderUser.is_outdated is true, which conflicts with the background beat job
cadence; change the view to be read-only and avoid in-request refreshes: stop
calling get_atcoder_data and user.save() inside the loop (keep returning cached
user.rating/highest_rating/rank), or alternatively align the staleness window by
making the is_outdated threshold match the beat job schedule so the view will
rarely trigger updates; reference AtcoderUser.is_outdated, the loop over
active_users, and the get_atcoder_data call to locate the change.
- Around line 99-113: The GET handler currently iterates gh_users and calls
fetch_github_data for each user, causing O(n) outbound HTTP I/O; remove that
per-request refresh loop from GithubUserAPI.get() so the view becomes read-only
and only reads from the DB, and instead schedule updates to run asynchronously
(e.g., enqueue tasks that call a background worker function like
refresh_github_user_data(username) which wraps fetch_github_data + save). If you
need on-demand refreshing, expose a separate endpoint or a targeted POST that
enqueues a refresh for a single user (calling the same background task) rather
than performing network I/O inside the GET.
---
Nitpick comments:
In `@api/leaderboard/views.py`:
- Around line 596-630: The AtCoder scraping/parsing logic in get_atcoder_data is
duplicated in the Celery worker; extract the shared HTML parsing into a single
helper (e.g., parse_atcoder_profile or parse_atcoder_html) that accepts the page
HTML or BeautifulSoup and returns the same instance dict keys
("rating","highest_rating","rank"), then update get_atcoder_data to call that
helper after fetching page.text and update the duplicate code in the Celery task
to reuse the helper too; ensure the new helper is placed in a common module
imported by both (and update imports) so future markup fixes are applied once.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 875c30fe-0122-4727-aff1-ba5d0b5fb151
📒 Files selected for processing (4)
api/leaderboard/celery.pyapi/leaderboard/migrations/0012_alter_codeforcesuser_last_activity.pyapi/leaderboard/settings.pyapi/leaderboard/views.py
…d fronted for freinds section. Backend to be updated
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
Contributor
There was a problem hiding this comment.
Actionable comments posted: 14
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (4)
api/leaderboard/views.py (1)
742-748:⚠️ Potential issue | 🟠 MajorField name mismatch:
"description"should be"discription".The
ReplyPostmodel defines the field asdiscription(with typo), but line 743 uses"description". This meanssetattrwill create a new attribute on the instance rather than updating the actual model field, and the change won't be persisted to the database.🔧 Proposed fix
# Update allowed fields - for field in ["description", "likes", "dislikes"]: + for field in ["discription", "likes", "dislikes"]: if field in request.data: setattr(reply, field, request.data[field])Note: The model field name itself contains a typo (
discriptioninstead ofdescription). Consider creating a migration to rename the field for long-term maintainability.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@api/leaderboard/views.py` around lines 742 - 748, The loop is updating "description" which mismatches the ReplyPost model's actual field name "discription", causing setattr to create a non-persistent attribute; update the allowed fields list in the view to use "discription" (replace "description" with "discription") so setattr(reply, "discription", ...) updates the real model field before reply.save(), and consider creating a follow-up migration to rename the model field for long-term correctness; relevant symbols: reply, ReplyPost_Serializer, and the model field discription.app/src/components/CodeforcesTable.jsx (3)
839-868:⚠️ Potential issue | 🟠 MajorRace condition:
setContestIdis async, sofetchContestStandings()uses stale value.When clicking the quick contest buttons,
setContestIdis called followed immediately byfetchContestStandings(). Since React state updates are asynchronous,fetchContestStandingswill use the oldcontestIdvalue, not the newly set one.🔧 Proposed fix - pass contest ID directly
- <Button - variant="outline" - size="sm" - onClick={() => { - setContestId("1902"); - fetchContestStandings(); - }} - > + <Button + variant="outline" + size="sm" + onClick={() => { + setContestId("1902"); + }} + > 1902 (Div. 3) </Button>Then use a
useEffectto trigger fetch whencontestIdchanges, or modifyfetchContestStandingsto accept an optional contest ID parameter:const fetchContestStandings = async (overrideId) => { const idToFetch = overrideId ?? contestId; if (!idToFetch) { ... } // use idToFetch instead of contestId };🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@app/src/components/CodeforcesTable.jsx` around lines 839 - 868, The quick-button handlers race because setContestId is async so fetchContestStandings reads the old contestId; fix by changing fetchContestStandings to accept an optional overrideId (e.g., function fetchContestStandings(overrideId) { const idToFetch = overrideId ?? contestId; ... }) and update the button onClick handlers to call fetchContestStandings("1902") / fetchContestStandings("1907") / fetchContestStandings("1073") after calling setContestId, or alternatively remove the immediate fetch and rely on a useEffect watching contestId (useEffect(() => { if (contestId) fetchContestStandings(); }, [contestId])) so the fetch always uses the latest contestId.
148-152:⚠️ Potential issue | 🟠 MajorFallback to first user is misleading and incorrect.
If user data cannot be fetched, falling back to
codeforcesUsers[0]will display a random user's data as "You". This could confuse users and show incorrect information. The fallback should benullor show an error state instead.🔧 Proposed fix
// If we get here, something went wrong console.warn("Could not fetch logged in user data"); - if (codeforcesUsers.length > 0) { - setLoggedInUser(codeforcesUsers[0]); - } + setLoggedInUser(null);🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@app/src/components/CodeforcesTable.jsx` around lines 148 - 152, The current fallback sets setLoggedInUser(codeforcesUsers[0]) when fetching user data fails, which incorrectly shows a random user as "You"; change this to clear the logged-in user and surface an error state instead. In the CodeforcesTable.jsx component replace the fallback call to setLoggedInUser(codeforcesUsers[0]) with setLoggedInUser(null) (or an explicit error flag/state like setUserFetchError(true)), and ensure the render logic that reads loggedInUser handles null/error (e.g., shows an error message or login prompt) so no other component will display another user's data as the current user.
96-99:⚠️ Potential issue | 🟠 MajorThe
timeoutoption is not supported by the nativefetchAPI and will be silently ignored.The
{ timeout: 10000 }option passed tofetch()has no effect. Nativefetchdoes not support a timeout parameter in its options object. This pattern appears three times (lines 96-99, 109-112, 176-179).Use
AbortSignalinstead to implement request timeouts:Fix using AbortController
- const cfResponse = await fetch( - `https://codeforces.com/api/user.info?handles=${currentUsername}`, - { timeout: 10000 } - ); + const controller = new AbortController(); + const timeoutId = setTimeout(() => controller.abort(), 10000); + try { + const cfResponse = await fetch( + `https://codeforces.com/api/user.info?handles=${currentUsername}`, + { signal: controller.signal } + ); + clearTimeout(timeoutId);Consider extracting a helper function to avoid repetition across all three instances.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@app/src/components/CodeforcesTable.jsx` around lines 96 - 99, The fetch calls in CodeforcesTable.jsx (the cfResponse fetch and the two other similar fetch calls) currently pass a non‑existent { timeout: 10000 } option which is ignored; replace this pattern by creating a small helper (e.g., fetchWithTimeout) that uses AbortController to set a 10s timeout, pass controller.signal to fetch, and clear the timeout on completion; then update the three places (the cfResponse fetch and the two other fetch invocations in the component) to call fetchWithTimeout(url, { method, ... }) so all requests properly abort after 10s.
🧹 Nitpick comments (6)
app/src/components/Blogs.jsx (1)
220-229: Client-side guard is good but insufficient without backend enforcement.The conditional
currentUsername === blog.usernamecorrectly restricts the delete button visibility to the blog author. However, this is a cosmetic check only — security must be enforced server-side (as noted above).Minor UX note: Consider using a
Dialogcomponent for delete confirmation instead ofwindow.confirm()to maintain consistency with the "Create Blog" dialog pattern used elsewhere in this component.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@app/src/components/Blogs.jsx` around lines 220 - 229, The client-side check currentUsername === blog.username only hides the delete button but does not enforce security; update the backend delete handler (the API route/controller that handles blog deletion) to verify the authenticated user matches blog.username and return 403/unauthorized if not, and add tests for unauthorized delete attempts; on the frontend, replace the window.confirm() flow inside handleDeleteBlog with the app's Dialog-based confirmation (the same Dialog pattern used for the "Create Blog" dialog in Blogs.jsx) so deletion uses a consistent modal confirmation UI and then call the delete API only after positive confirmation.app/src/components/Friends.jsx (1)
48-60: Sequential fetching could be parallelized for better performance.The
loadAllFriendsfunction awaits eachfetchFriendscall sequentially, which is slower than necessary. Since the requests are independent, they can run in parallel.⚡ Proposed optimization using Promise.all
useEffect(() => { const loadAllFriends = async () => { setLoading(true); - for (const platform of PLATFORMS) { - await fetchFriends(platform.id); - } + await Promise.all(PLATFORMS.map(platform => fetchFriends(platform.id))); setLoading(false); }; if (user) { loadAllFriends(); } }, [user]);🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@app/src/components/Friends.jsx` around lines 48 - 60, The current loadAllFriends function runs fetchFriends sequentially by awaiting each call in a for loop; replace that with parallel execution using Promise.all to invoke fetchFriends for every PLATFORMS item (e.g., map PLATFORMS to platform.id and call fetchFriends) so all requests run concurrently, and keep the setLoading(true) before launching Promise.all and setLoading(false) after awaiting Promise.all to preserve loading state; update the useEffect block that calls loadAllFriends to use the new parallel implementation.api/leaderboard/views.py (2)
813-817: Consider adding pagination for scalability.The
UserNamesListendpoint returns allUserNamesobjects without pagination. As the user base grows, this could lead to performance issues and large response payloads.💡 Example with pagination
from rest_framework.pagination import PageNumberPagination class UserNamesList(APIView): def get(self, request): usernames = UserNames.objects.all() paginator = PageNumberPagination() paginator.page_size = 50 page = paginator.paginate_queryset(usernames, request) serializer = UserNamesSerializer(page, many=True) return paginator.get_paginated_response(serializer.data)🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@api/leaderboard/views.py` around lines 813 - 817, The UserNamesList.get currently returns all UserNames objects causing unbounded payloads; update UserNamesList.get to use DRF pagination (e.g., PageNumberPagination) by creating a paginator, setting a suitable page_size (e.g., 50), calling paginator.paginate_queryset(UserNames.objects.all(), request) and passing the resulting page to UserNamesSerializer(many=True), then return paginator.get_paginated_response(serializer.data) instead of Response(serializer.data); reference class UserNamesList, method get, model UserNames and serializer UserNamesSerializer when making the change.
658-658: Remove duplicate import.
IsAuthenticatedOrReadOnlyis already imported at line 13. This duplicate import is unnecessary.🧹 Proposed fix
-from rest_framework.permissions import IsAuthenticatedOrReadOnly - class AtcoderViewSet(mixins.ListModelMixin, mixins.CreateModelMixin, generics.GenericAPIView):🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@api/leaderboard/views.py` at line 658, Remove the duplicate import of IsAuthenticatedOrReadOnly from rest_framework.permissions — keep the single existing import (the one at the top of the file) and delete the later redundant line `from rest_framework.permissions import IsAuthenticatedOrReadOnly`; verify no other imports were accidentally removed and run linters/tests to confirm no import errors in views referencing IsAuthenticatedOrReadOnly.app/src/components/CodeforcesTable.jsx (1)
289-291: Missing dependency in useEffect may cause stale data.The
useEffectcallinggetcffriends()has an empty dependency array, butgetcffriendschecksisAuthenticated. If the user logs in after component mount, friends won't be fetched. Consider addingisAuthenticatedto the dependency array.♻️ Proposed fix
useEffect(() => { getcffriends(); - }, []); + }, [isAuthenticated]);🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@app/src/components/CodeforcesTable.jsx` around lines 289 - 291, The effect that calls getcffriends() uses an empty dependency array so it won't re-run when authentication changes; update the useEffect that currently calls getcffriends() to include isAuthenticated in its dependency array and ensure getcffriends is stable (either memoize it with useCallback or move the fetch logic inline inside the useEffect) so friends are fetched when isAuthenticated becomes true; reference the existing getcffriends function and the useEffect that calls it to locate where to change the dependency array.api/leaderboard/models.py (1)
194-203: Consider adding validation forslugandtierfields.The
Achievementmodel accepts arbitrary strings forslugandtierwithout validation. This could lead to inconsistent data (e.g., typos like "bronz" vs "bronze", or invalid achievement slugs). Consider addingchoicesor a validator to ensure data integrity.💡 Example with choices
class Achievement(models.Model): TIER_CHOICES = [ ("bronze", "Bronze"), ("silver", "Silver"), ("gold", "Gold"), ("platinum", "Platinum"), ] user = models.ForeignKey(User, on_delete=models.CASCADE) slug = models.CharField(max_length=64) tier = models.CharField(max_length=64, choices=TIER_CHOICES) unlocked_at = models.DateTimeField(auto_now_add=True)Alternatively, create a separate
AchievementDefinitionmodel and use a ForeignKey forslug.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@api/leaderboard/models.py` around lines 194 - 203, The Achievement model currently allows arbitrary strings for slug and tier; add validation by defining a canonical set of tiers (e.g., TIER_CHOICES) and applying it to the tier field (tier = models.CharField(..., choices=TIER_CHOICES)) and either add a validator (RegexValidator or custom validator) to the slug field or replace slug with a ForeignKey to a new AchievementDefinition model to enforce valid slugs; update the Achievement class (fields slug, tier and any import of validators or new AchievementDefinition) accordingly and ensure migrations are created.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@api/leaderboard/celery.py`:
- Around line 187-202: The task computes gh_user.stars incorrectly by using
fetch_starred_repos (count of repos the user has starred) instead of summing
stargazers_count like github_user_update; replace that logic by calling a
function that paginates the user's repos and sums repo["stargazers_count"]
(e.g., implement fetch_user_repo_stars or reuse the repo-summing logic from
github_user_update) and assign its result to gh_user.stars while keeping
gh_user.repositories and fetch_github_contributions as-is.
- Around line 179-195: fetch_github_contributions, fetch_starred_repos, and the
direct requests.get(url) call do not set timeouts and fetch_starred_repos
targets the "starred by user" endpoint instead of summing stars on the user's
repos; add a sensible timeout (e.g. timeout=5) to every requests.get invocation
in these functions and replace fetch_starred_repos implementation to call the
user's repos endpoint (https://api.github.com/users/{username}/repos), iterate
the returned repo objects and sum the stargazers_count (handle pagination if
needed), returning that sum — keep function names fetch_github_contributions and
fetch_starred_repos (or rename to fetch_repo_stars if preferred) but ensure the
three requests.get calls include the timeout param and the stars metric uses
stargazers_count from the repos response.
In `@api/leaderboard/friends.py`:
- Around line 489-502: The current atcoderFriendAddition handler mutates a list
from find_one and then $set which can race; replace that logic by performing an
atomic update on the atcoderFriends collection: call update_one(filter={"_id":
user}, update={"$addToSet": {"Friends": friendName}}, upsert=True) instead of
reading and mutating the Friends list, and similarly change
atcoderFriendDeletion to call update_one(filter={"_id": user}, update={"$pull":
{"Friends": friendName}}) so removals are atomic; remove the
find_one/mutate/$set branches and any insert_one path and rely on the collection
update operations.
- Around line 514-515: The code is incorrectly wrapping serializer.errors in
braces which attempts to create a set from a dict and raises TypeError; for
every occurrence replace Response({serializer.errors},
status=status.HTTP_400_BAD_REQUEST) with Response(serializer.errors,
status=status.HTTP_400_BAD_REQUEST) so the field-level validation errors are
returned correctly; search for usages of "Response({serializer.errors}" (and
similarly where serializer.errors is wrapped in {}), and update them to pass
serializer.errors directly while keeping the same status.HTTP_400_BAD_REQUEST
and surrounding logic intact.
In `@api/leaderboard/management/commands/update_db.py`:
- Around line 9-15: The import list in update_db.py dropped codechefUser but the
function codechef_user_update() still references codechefUser, causing a
NameError; restore codechefUser to the import tuple (alongside LeetcodeUser,
codeforcesUser, githubUser, openlakeContributor, AtcoderUser) or else refactor
codechef_user_update() to stop referencing codechefUser (e.g., remove or replace
usages), ensuring the symbol codechefUser is defined before being dereferenced.
- Around line 305-344: The scrape builds an instance dict but appends it even
when rating/highest_rating/rank are missing, allowing partial payloads to
overwrite good data; modify the try block after the table loop to validate that
instance contains all three keys ("rating", "highest_rating", "rank") before
appending to updates (or else raise/handle as a scrape failure), e.g., check the
presence of those keys on instance built in the try for ac_user and only call
updates.append(instance) when all are present; otherwise fall back to creating
the fallback instance using ac_user.rating/ac_user.highest_rating/ac_user.rank
(the same object currently built in the except) or re-raise to hit the except
handler so AC_Update_Serializer never receives partial data.
In `@api/leaderboard/root.py`:
- Around line 62-70: Add the main AtCoder leaderboard entry to the root links
mapping: insert an "atcoder-leaderboard" key alongside the existing
"atcoder-friend-addition"/"atcoder-friend-deletion"/"atcoder-friend-list"
entries and set its value to a reverse(...) call for the AtCoder leaderboard
view using the same request and format arguments (i.e.,
reverse("<ATCODER_LEADERBOARD_URL_NAME>", request=request, format=format));
replace <ATCODER_LEADERBOARD_URL_NAME> with the actual URL name used by the
leaderboard view so clients discovering via the API root will receive the
AtCoder leaderboard URL.
In `@api/leaderboard/serializers.py`:
- Around line 142-151: DiscussionPost and ReplyPost are being created without
setting the author FK, causing username to be NULL and the ReadOnlyField source
"username.username" to fail; update the creation calls in
DiscussionPostManage.post and DiscussionReplyManage.post so the calls to
DiscussionPost.objects.create(...) and ReplyPost.objects.create(...) include
username=request.user (i.e., pass the current request.user as the
username/author FK when creating the objects).
In `@api/leaderboard/views.py`:
- Around line 348-350: The code uses timedelta in the loop that updates outdated
users (the for user in cc_users.filter(... last_updated__lt=datetime.now() -
timedelta(minutes=15))[:5] block) but timedelta is not imported; fix by adding
timedelta to the datetime imports (e.g., include "timedelta" in the existing
datetime import at the top of api/leaderboard/views.py) so the symbol timedelta
is defined when the cc_users filter runs.
In `@app/src/components/Blogs.jsx`:
- Around line 108-131: Add authentication and server-side authorization:
decorate the DiscussionPostManage view (class) with
`@permission_classes`([IsAuthenticated]) and in its delete method
(DiscussionPostManage.delete) load the DiscussionPost by title/ID and verify
request.user == post.author before performing deletion; return 403 if not
authorized. Update the frontend handleDeleteBlog in Blogs.jsx to include the
auth token in request headers (e.g., Authorization: Bearer <token> and existing
Content-Type) so the backend can authenticate the caller. Also ensure the
backend uses the authenticated user rather than trusting username in
request.data when determining which post to delete.
In `@app/src/components/Friends.jsx`:
- Around line 204-206: AvatarFallback currently uses handle[0].toUpperCase()
which will throw if handle is an empty string or falsy; change the rendering to
safely derive the initial by checking handle (e.g., const initial = handle &&
handle.length ? handle[0].toUpperCase() : '?' ) and use that initial inside
AvatarFallback (update the code around Avatar / AvatarFallback to reference the
new safe initial), ensuring you also handle null/undefined by falling back to a
default character.
- Around line 12-18: PLATFORMS is missing an Atcoder entry so the Friends
component can't call the backend atcoder endpoints; add an object to the
PLATFORMS array in Friends.jsx with id "atcoder" and appropriate name/icon/color
and set suffix to match the backend pattern ("atcoder") so the component builds
URLs like atcoderFA/ (or alternatively align the backend URL pattern to use the
short suffix if you prefer); update any UI code that expects the suffix string
to ensure it uses the new "atcoder" suffix when constructing friend API routes.
- Around line 32-38: The code currently calls
JSON.parse(localStorage.getItem("authTokens")) directly (in the fetch block and
at other sites) which can throw on null or corrupted data; add a small safe
helper function (e.g., getAuthTokens) that wraps localStorage.getItem in a
try/catch and returns null on errors, then replace direct JSON.parse calls with
getAuthTokens() and guard uses of authTokens (e.g., Authorization header
creation in the fetch call and any access to authTokens.access) so you only set
the header or proceed when getAuthTokens() returns a non-null object; update all
occurrences referenced in this file (the fetch block and the other spots around
lines with JSON.parse) to use this helper.
In `@app/src/components/Navbar.jsx`:
- Around line 118-120: The active-state logic marks multiple items active on "/"
because isActive is set by a simple pathname === item.url; update the logic
inside items.map (where isActive and isBlogs are computed) so only one item
becomes active for the root: either give placeholder entries distinct routes
(change their item.url values from "/" to unique routes) or special-case the
home path by changing the isActive condition to something like pathname ===
item.url && !(pathname === "/" && index !== <desiredHomeIndex>) so only the
intended index (e.g., Dashboard) is active when pathname is "/". Ensure you
update references within the same map block (isActive, item.url, index) and
remove the ambiguous placeholders if you choose to assign unique routes.
---
Outside diff comments:
In `@api/leaderboard/views.py`:
- Around line 742-748: The loop is updating "description" which mismatches the
ReplyPost model's actual field name "discription", causing setattr to create a
non-persistent attribute; update the allowed fields list in the view to use
"discription" (replace "description" with "discription") so setattr(reply,
"discription", ...) updates the real model field before reply.save(), and
consider creating a follow-up migration to rename the model field for long-term
correctness; relevant symbols: reply, ReplyPost_Serializer, and the model field
discription.
In `@app/src/components/CodeforcesTable.jsx`:
- Around line 839-868: The quick-button handlers race because setContestId is
async so fetchContestStandings reads the old contestId; fix by changing
fetchContestStandings to accept an optional overrideId (e.g., function
fetchContestStandings(overrideId) { const idToFetch = overrideId ?? contestId;
... }) and update the button onClick handlers to call
fetchContestStandings("1902") / fetchContestStandings("1907") /
fetchContestStandings("1073") after calling setContestId, or alternatively
remove the immediate fetch and rely on a useEffect watching contestId
(useEffect(() => { if (contestId) fetchContestStandings(); }, [contestId])) so
the fetch always uses the latest contestId.
- Around line 148-152: The current fallback sets
setLoggedInUser(codeforcesUsers[0]) when fetching user data fails, which
incorrectly shows a random user as "You"; change this to clear the logged-in
user and surface an error state instead. In the CodeforcesTable.jsx component
replace the fallback call to setLoggedInUser(codeforcesUsers[0]) with
setLoggedInUser(null) (or an explicit error flag/state like
setUserFetchError(true)), and ensure the render logic that reads loggedInUser
handles null/error (e.g., shows an error message or login prompt) so no other
component will display another user's data as the current user.
- Around line 96-99: The fetch calls in CodeforcesTable.jsx (the cfResponse
fetch and the two other similar fetch calls) currently pass a non‑existent {
timeout: 10000 } option which is ignored; replace this pattern by creating a
small helper (e.g., fetchWithTimeout) that uses AbortController to set a 10s
timeout, pass controller.signal to fetch, and clear the timeout on completion;
then update the three places (the cfResponse fetch and the two other fetch
invocations in the component) to call fetchWithTimeout(url, { method, ... }) so
all requests properly abort after 10s.
---
Nitpick comments:
In `@api/leaderboard/models.py`:
- Around line 194-203: The Achievement model currently allows arbitrary strings
for slug and tier; add validation by defining a canonical set of tiers (e.g.,
TIER_CHOICES) and applying it to the tier field (tier = models.CharField(...,
choices=TIER_CHOICES)) and either add a validator (RegexValidator or custom
validator) to the slug field or replace slug with a ForeignKey to a new
AchievementDefinition model to enforce valid slugs; update the Achievement class
(fields slug, tier and any import of validators or new AchievementDefinition)
accordingly and ensure migrations are created.
In `@api/leaderboard/views.py`:
- Around line 813-817: The UserNamesList.get currently returns all UserNames
objects causing unbounded payloads; update UserNamesList.get to use DRF
pagination (e.g., PageNumberPagination) by creating a paginator, setting a
suitable page_size (e.g., 50), calling
paginator.paginate_queryset(UserNames.objects.all(), request) and passing the
resulting page to UserNamesSerializer(many=True), then return
paginator.get_paginated_response(serializer.data) instead of
Response(serializer.data); reference class UserNamesList, method get, model
UserNames and serializer UserNamesSerializer when making the change.
- Line 658: Remove the duplicate import of IsAuthenticatedOrReadOnly from
rest_framework.permissions — keep the single existing import (the one at the top
of the file) and delete the later redundant line `from
rest_framework.permissions import IsAuthenticatedOrReadOnly`; verify no other
imports were accidentally removed and run linters/tests to confirm no import
errors in views referencing IsAuthenticatedOrReadOnly.
In `@app/src/components/Blogs.jsx`:
- Around line 220-229: The client-side check currentUsername === blog.username
only hides the delete button but does not enforce security; update the backend
delete handler (the API route/controller that handles blog deletion) to verify
the authenticated user matches blog.username and return 403/unauthorized if not,
and add tests for unauthorized delete attempts; on the frontend, replace the
window.confirm() flow inside handleDeleteBlog with the app's Dialog-based
confirmation (the same Dialog pattern used for the "Create Blog" dialog in
Blogs.jsx) so deletion uses a consistent modal confirmation UI and then call the
delete API only after positive confirmation.
In `@app/src/components/CodeforcesTable.jsx`:
- Around line 289-291: The effect that calls getcffriends() uses an empty
dependency array so it won't re-run when authentication changes; update the
useEffect that currently calls getcffriends() to include isAuthenticated in its
dependency array and ensure getcffriends is stable (either memoize it with
useCallback or move the fetch logic inline inside the useEffect) so friends are
fetched when isAuthenticated becomes true; reference the existing getcffriends
function and the useEffect that calls it to locate where to change the
dependency array.
In `@app/src/components/Friends.jsx`:
- Around line 48-60: The current loadAllFriends function runs fetchFriends
sequentially by awaiting each call in a for loop; replace that with parallel
execution using Promise.all to invoke fetchFriends for every PLATFORMS item
(e.g., map PLATFORMS to platform.id and call fetchFriends) so all requests run
concurrently, and keep the setLoading(true) before launching Promise.all and
setLoading(false) after awaiting Promise.all to preserve loading state; update
the useEffect block that calls loadAllFriends to use the new parallel
implementation.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 9f423dd2-a9fb-4281-bbfa-6979734e7e81
📒 Files selected for processing (13)
api/leaderboard/celery.pyapi/leaderboard/friends.pyapi/leaderboard/management/commands/update_db.pyapi/leaderboard/models.pyapi/leaderboard/root.pyapi/leaderboard/serializers.pyapi/leaderboard/urls.pyapi/leaderboard/views.pyapp/src/App.jsxapp/src/components/Blogs.jsxapp/src/components/CodeforcesTable.jsxapp/src/components/Friends.jsxapp/src/components/Navbar.jsx
Comment on lines
+32
to
+38
| try { | ||
| const authTokens = JSON.parse(localStorage.getItem("authTokens")); | ||
| const response = await fetch(`${BACKEND}/${platformId}${platform.suffix}L/`, { | ||
| headers: { | ||
| Authorization: `Bearer ${authTokens.access}`, | ||
| }, | ||
| }); |
Contributor
There was a problem hiding this comment.
Unguarded JSON.parse on localStorage can crash if data is corrupted.
If localStorage.getItem("authTokens") returns null or invalid JSON, JSON.parse will throw. This pattern appears in multiple places (lines 33, 70, 98).
🛡️ Proposed defensive fix
try {
- const authTokens = JSON.parse(localStorage.getItem("authTokens"));
+ const stored = localStorage.getItem("authTokens");
+ if (!stored) return;
+ const authTokens = JSON.parse(stored);
const response = await fetch(`${BACKEND}/${platformId}${platform.suffix}L/`, {Consider extracting a helper function to safely retrieve auth tokens:
const getAuthTokens = () => {
try {
const stored = localStorage.getItem("authTokens");
return stored ? JSON.parse(stored) : null;
} catch {
return null;
}
};🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@app/src/components/Friends.jsx` around lines 32 - 38, The code currently
calls JSON.parse(localStorage.getItem("authTokens")) directly (in the fetch
block and at other sites) which can throw on null or corrupted data; add a small
safe helper function (e.g., getAuthTokens) that wraps localStorage.getItem in a
try/catch and returns null on errors, then replace direct JSON.parse calls with
getAuthTokens() and guard uses of authTokens (e.g., Authorization header
creation in the fetch call and any access to authTokens.access) so you only set
the header or proceed when getAuthTokens() returns a non-null object; update all
occurrences referenced in this file (the fetch block and the other spots around
lines with JSON.parse) to use this helper.
Comment on lines
+204
to
+206
| <Avatar className="h-10 w-10"> | ||
| <AvatarFallback>{handle[0].toUpperCase()}</AvatarFallback> | ||
| </Avatar> |
Contributor
There was a problem hiding this comment.
Potential crash if handle is an empty string.
handle[0].toUpperCase() will throw if handle is an empty string (accessing index 0 of empty string returns undefined, and calling toUpperCase() on undefined throws).
🛡️ Proposed fix
- <AvatarFallback>{handle[0].toUpperCase()}</AvatarFallback>
+ <AvatarFallback>{handle?.[0]?.toUpperCase() || "?"}</AvatarFallback>📝 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.
Suggested change
| <Avatar className="h-10 w-10"> | |
| <AvatarFallback>{handle[0].toUpperCase()}</AvatarFallback> | |
| </Avatar> | |
| <Avatar className="h-10 w-10"> | |
| <AvatarFallback>{handle?.[0]?.toUpperCase() || "?"}</AvatarFallback> | |
| </Avatar> |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@app/src/components/Friends.jsx` around lines 204 - 206, AvatarFallback
currently uses handle[0].toUpperCase() which will throw if handle is an empty
string or falsy; change the rendering to safely derive the initial by checking
handle (e.g., const initial = handle && handle.length ? handle[0].toUpperCase()
: '?' ) and use that initial inside AvatarFallback (update the code around
Avatar / AvatarFallback to reference the new safe initial), ensuring you also
handle null/undefined by falling back to a default character.
Comment on lines
+118
to
+120
| {items.map((item, index) => { | ||
| const isActive = pathname === item.url; | ||
| const isBlogs = index === 7; |
Contributor
There was a problem hiding this comment.
Only one nav item should become active for /.
Line 119 compares only the URL, so Dashboard, Leaderboards, and Analytics all become active on / because each item still points there. Give the placeholder entries distinct routes, or special-case the home row so only one item gets the active indicator.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@app/src/components/Navbar.jsx` around lines 118 - 120, The active-state logic
marks multiple items active on "/" because isActive is set by a simple pathname
=== item.url; update the logic inside items.map (where isActive and isBlogs are
computed) so only one item becomes active for the root: either give placeholder
entries distinct routes (change their item.url values from "/" to unique routes)
or special-case the home path by changing the isActive condition to something
like pathname === item.url && !(pathname === "/" && index !==
<desiredHomeIndex>) so only the intended index (e.g., Dashboard) is active when
pathname is "/". Ensure you update references within the same map block
(isActive, item.url, index) and remove the ambiguous placeholders if you choose
to assign unique routes.
… and UI, and add a MAINTAINERS.md file.
This comment has been minimized.
This comment has been minimized.
Contributor
There was a problem hiding this comment.
Actionable comments posted: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (5)
app/src/components/Blogs.jsx (2)
48-58:⚠️ Potential issue | 🟡 MinorGuard against missing auth tokens in authenticated actions.
Unlike
fetchBlogswhich safely checksauthTokens?.accessbefore adding the header,handleCreateBlogdirectly accessesauthTokens.access. If an unauthenticated user somehow triggers this (the "Write Blog" button is always visible), this will either throw a TypeError or send a malformed"Bearer undefined"header.Proposed fix: add early return or disable the submit action for unauthenticated users
const handleCreateBlog = async () => { if (!title.trim() || !description.trim()) return; + if (!authTokens?.access) { + console.error("Authentication required to create a blog"); + return; + } setLoading(true); try {Alternatively, disable the dialog trigger for unauthenticated users:
-<DialogTrigger asChild> - <Button className="gap-2"> +<DialogTrigger asChild disabled={!authTokens?.access}> + <Button className="gap-2" disabled={!authTokens?.access}>🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@app/src/components/Blogs.jsx` around lines 48 - 58, handleCreateBlog currently accesses authTokens.access directly and can send "Bearer undefined" or throw; update handleCreateBlog to guard using authTokens?.access (or check if authTokens && authTokens.access) and early-return with a user-facing error or prevent submission if missing. Alternatively, disable the "Write Blog" dialog/submit when authTokens?.access is falsy; ensure the Authorization header is only added when authTokens?.access exists and mirror the safe pattern used in fetchBlogs to avoid TypeError or malformed headers.
90-101:⚠️ Potential issue | 🟡 MinorGuard against missing auth tokens in like/dislike interactions.
Same issue as
handleCreateBlog: the like/dislike buttons are visible to all users, buthandleInteractassumesauthTokens.accessexists. Unauthenticated users clicking these will cause errors or send malformed headers.Proposed fix: add auth guard
const handleInteract = async (post, action) => { + if (!authTokens?.access) { + console.error("Authentication required to interact"); + return; + } let updatedLikes = post.likes; let updatedDislikes = post.dislikes;🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@app/src/components/Blogs.jsx` around lines 90 - 101, The like/dislike handler currently assumes authTokens.access exists (seen in the fetch call inside handleInteract), which will break for unauthenticated users; update handleInteract to first check authTokens and authTokens.access and early-return (or show a login prompt/toast) if missing, preventing the fetch to `${BACKEND}/discussionpost/` and avoiding malformed Authorization headers; ensure the same guard is applied before making the PUT request and before any UI state that assumes auth (use the symbols authTokens.access, handleInteract, and BACKEND to locate the change).api/leaderboard/management/commands/update_db.py (1)
166-183:⚠️ Potential issue | 🟡 MinorMissing timeout on nested API request.
Line 171 makes a
requests.get()call to the GitHub API without a timeout. While the initial page fetch (line 159) now has a timeout, this nested call could hang indefinitely and block the entire update process.🔧 Suggested fix
url = f"https://api.github.com/users/{gh_user.username}/repos" - response = requests.get(url).json() + response = requests.get(url, headers=headers, timeout=10).json()🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@api/leaderboard/management/commands/update_db.py` around lines 166 - 183, The nested GitHub API call using requests.get(url) (where response = requests.get(url).json() and later used to compute repositories/stars for gh_user) lacks a timeout and can hang; update that call to include a sensible timeout (e.g. timeout=5) and wrap it in a try/except for requests.exceptions.RequestException so you can fallback (e.g. treat response as [] or log the error) before computing repositories/stars and appending instance to updates to avoid blocking the update_db workflow.api/leaderboard/views.py (2)
610-628:⚠️ Potential issue | 🟠 MajorPUT allows any authenticated user to edit any post's content.
The PUT method only looks up the post by title without verifying ownership. While the comments note this allows anyone to like/dislike, it also permits modifying
titleanddiscriptionof posts authored by others. Consider separating like/dislike updates from content editing, or adding ownership checks for content fields.🔧 Proposed fix - restrict content editing to author
def put(self, request): try: post = DiscussionPost.objects.get(title=request.data["title"]) except DiscussionPost.DoesNotExist: return Response( {"error": "Post not found"}, status=status.HTTP_404_NOT_FOUND ) - for field in ["title", "discription", "likes", "dislikes", "comments"]: + # Anyone can update likes/dislikes/comments + for field in ["likes", "dislikes", "comments"]: + if field in request.data: + setattr(post, field, request.data[field]) + + # Only author can edit content + if post.username == request.user: + for field in ["title", "discription"]: + if field in request.data: + setattr(post, field, request.data[field])🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@api/leaderboard/views.py` around lines 610 - 628, The PUT handler in put(self, request) fetches DiscussionPost by title but doesn't restrict edits to content fields; update it so ownership is enforced: before applying updates, check if any content fields ("title", "discription") are present in request.data and if so verify post.author == request.user (or return a 403 Response if not); allow non-authors to modify only non-content fields ("likes", "dislikes", "comments"); keep the current save/serialize flow after the checks.
722-728:⚠️ Potential issue | 🔴 CriticalField name mismatch:
descriptionvsdiscription.The loop iterates over
"description"but the ReplyPost model (permodels.pyline 165) uses the misspelled field"discription". Thesetattrwill create a new attribute instead of updating the model field, causing content updates to silently fail.🐛 Proposed fix
- for field in ["description", "likes", "dislikes"]: + for field in ["discription", "likes", "dislikes"]:🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@api/leaderboard/views.py` around lines 722 - 728, The loop in the view that updates reply fields uses the correct intent "description" but the Reply model actually defines the misspelled field "discription", so setattr creates a new attribute instead of updating the stored field; update the view to set the actual model field name (use "discription" instead of "description") when iterating over allowed fields (or accept both keys by mapping "description" -> "discription"), ensuring the Reply instance is modified before reply.save() and serialized via ReplyPost_Serializer(reply).
♻️ Duplicate comments (3)
app/src/components/Blogs.jsx (1)
112-135:⚠️ Potential issue | 🟡 MinorAdd auth guard and consider using blog ID for deletion.
Auth guard: For consistency with
fetchBlogsand safety, add a null check forauthTokens?.access.Title-based identification: Deleting by title (line 123) could be fragile if duplicate titles exist. Using
post.idwould be more reliable if the backend supports it.Proposed fix
const handleDeleteBlog = async (post) => { if (!window.confirm("Are you sure you want to delete this blog?")) return; + if (!authTokens?.access) { + console.error("Authentication required to delete a blog"); + return; + } try { const response = await fetch(`${BACKEND}/discussionpost/`, { method: "DELETE", headers: { "Content-Type": "application/json", "Authorization": `Bearer ${authTokens.access}`, }, body: JSON.stringify({ - title: post.title, + id: post.id, // More reliable than title if backend supports it }), });
User feedback on delete failure (from past review).
The past review suggested adding user-facing feedback (e.g.,
alert()) when deletion fails. Currently, onlyconsole.erroris used on lines 130 and 133, which users won't see.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@app/src/components/Blogs.jsx` around lines 112 - 135, handleDeleteBlog should guard against missing auth and use a stable identifier for deletion: check authTokens?.access before making the request (same pattern as fetchBlogs) and prefer sending post.id (or include the id in the DELETE URL like `${BACKEND}/discussionpost/${post.id}`) instead of title to avoid collisions; also replace or complement console.error calls in handleDeleteBlog with user-facing feedback (e.g., alert or toast) on both non-ok responses and caught errors so the user is informed when deletion fails.app/src/components/Friends.jsx (2)
205-207:⚠️ Potential issue | 🟡 MinorPotential crash if
handleis an empty string.
handle[0].toUpperCase()will throwTypeErrorwhenhandleis an empty string since""[0]isundefined.🛡️ Proposed fix
- <AvatarFallback>{handle[0].toUpperCase()}</AvatarFallback> + <AvatarFallback>{handle?.[0]?.toUpperCase() || "?"}</AvatarFallback>🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@app/src/components/Friends.jsx` around lines 205 - 207, The AvatarFallback usage can crash when handle is an empty string because handle[0].toUpperCase() will access undefined; update the Friends component to guard and compute a safe fallback before rendering (e.g., derive a variable like avatarInitial = handle && handle.length > 0 ? handle[0].toUpperCase() : '?' ) and pass that to <AvatarFallback> (refer to AvatarFallback and the handle variable in Friends.jsx); ensure the logic handles null/undefined too so AvatarFallback always receives a string.
33-39:⚠️ Potential issue | 🟡 MinorUnguarded
JSON.parsecan crash on null or corrupted data.
localStorage.getItem("authTokens")may returnnullor corrupted JSON, causingJSON.parseto throw. This pattern appears at lines 34, 71, and 99.🛡️ Proposed fix - add a helper function
+const getAuthTokens = () => { + try { + const stored = localStorage.getItem("authTokens"); + return stored ? JSON.parse(stored) : null; + } catch { + return null; + } +}; + const fetchFriends = async (platformId) => { const platform = PLATFORMS.find(p => p.id === platformId); if (!platform) return; try { - const authTokens = JSON.parse(localStorage.getItem("authTokens")); + const authTokens = getAuthTokens(); + if (!authTokens?.access) return; const response = await fetch(`${BACKEND}/${platformId}${platform.suffix}L/`, {🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@app/src/components/Friends.jsx` around lines 33 - 39, Replace the unguarded JSON.parse(localStorage.getItem("authTokens")) calls with a small helper (e.g., getStoredAuthTokens or parseAuthTokens) that reads localStorage, wraps JSON.parse in try/catch, returns null on failure, and logs or handles errors; then update the fetch callers in Friends.jsx (the try block fetching `${BACKEND}/${platformId}${platform.suffix}L/` and the other usages around lines 71 and 99) to call this helper and guard access to authTokens.access (early return or refresh/unauth flow) so you never dereference access on a null/invalid token object.
🧹 Nitpick comments (5)
api/leaderboard/management/commands/update_db.py (2)
270-275: Consider adding timeouts for consistency.The PR adds timeouts to CodeChef, GitHub (partial), LeetCode, and AtCoder requests, but
openlake_contributor_updatestill has requests without timeouts at lines 270 and 275. For consistent behavior and to prevent hangs, consider adding timeouts here as well.♻️ Suggested fix
- response = requests.get(url, headers=headers).json() + response = requests.get(url, headers=headers, timeout=10).json() for i in range(len(response)): repo_url = str(response[i]["contributors_url"]) try: - repo_response = requests.get(repo_url).json() + repo_response = requests.get(repo_url, headers=headers, timeout=10).json()🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@api/leaderboard/management/commands/update_db.py` around lines 270 - 275, The requests in openlake_contributor_update currently call requests.get(...).json() without timeouts (specifically the call that assigns response = requests.get(url, headers=headers).json() and the repo_response = requests.get(repo_url).json()), which can hang; update both calls to pass a timeout (e.g., timeout=5 or a project-constant like HTTP_TIMEOUT) to requests.get and handle potential requests.exceptions.Timeout/RequestException where appropriate in openlake_contributor_update to maintain consistency with other providers.
300-301: Moveimport reto module level.Importing inside the function adds overhead on each call and deviates from Python conventions. Move this import to the top of the file with other imports.
♻️ Suggested fix
At the top of the file (after line 6):
import reThen remove line 301.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@api/leaderboard/management/commands/update_db.py` around lines 300 - 301, The import re is inside the atcoder_user_update function which causes repeated import overhead and violates convention; move the import to the module level with the other imports (add a single top-of-file import for re) and remove the in-function statement from atcoder_user_update so the function uses the module-level re.app/src/components/Friends.jsx (1)
49-61: Consider fetching friends in parallel for faster loading.The sequential
await fetchFriends(platform.id)in a loop makes 6 requests serially. UsingPromise.allwould load all platforms concurrently.♻️ Optional refactor for parallel fetching
useEffect(() => { const loadAllFriends = async () => { setLoading(true); - for (const platform of PLATFORMS) { - await fetchFriends(platform.id); - } + await Promise.all(PLATFORMS.map(platform => fetchFriends(platform.id))); setLoading(false); }; if (user) { loadAllFriends(); } }, [user]);🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@app/src/components/Friends.jsx` around lines 49 - 61, The current loadAllFriends function performs sequential requests by awaiting fetchFriends(platform.id) inside a for loop, slowing loading; change it to run all fetches concurrently using Promise.all over PLATFORMS.map(p => fetchFriends(p.id)) and await that single Promise, while preserving setLoading(true) before and setLoading(false) after; update loadAllFriends and its use in the useEffect (references: loadAllFriends, fetchFriends, PLATFORMS, setLoading, useEffect) so all platform fetches run in parallel and errors are still handled appropriately.api/leaderboard/views.py (1)
761-784: Add validation forslugandtiervalues.The endpoint accepts any arbitrary strings for
slugandtierwithout validating against a defined set of valid achievements. The model fields have no constraints beyondmax_length=64and no validators are applied. This allows clients to create invalid or unintended achievement records. If achievements are predefined or have specific valid values, add validation to restrict accepted values.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@api/leaderboard/views.py` around lines 761 - 784, The endpoint currently allows any slug/tier strings; update AchievementUnlock.post to validate the incoming slug and tier against the canonical set of allowed achievements before creating records: either query the authoritative source (e.g., an AchievementDefinition model or a module-level VALID_ACHIEVEMENTS mapping) to confirm the slug exists and that the tier is among the allowed tiers for that slug, and return a 400 Response with an explanatory error if not valid; alternatively, move validation into an AchievementUnlockSerializer that performs the same checks and use serializer.is_valid() before calling Achievement.objects.get_or_create, referencing the Achievement model, AchievementUnlock view, and AchievementSerializer to locate where to apply this validation.api/leaderboard/celery.py (1)
220-221: Remove the redundant manual assignment and let Django managelast_updated.The
githubUser.last_updatedfield is defined withauto_now=Trueinapi/leaderboard/models.py(line 17), which automatically updates it to the current datetime on everysave(). The manual assignment at line 220 is overwritten immediately and represents a type mismatch (float vs DateTimeField). Removing it clarifies field ownership.Suggested cleanup
- gh_user.last_updated = datetime.now().timestamp() gh_user.save()🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@api/leaderboard/celery.py` around lines 220 - 221, Remove the redundant manual assignment to gh_user.last_updated before saving: the githubUser model's last_updated uses auto_now=True so Django will set it on gh_user.save(); delete the line that sets gh_user.last_updated = datetime.now().timestamp() and leave gh_user.save() unchanged to avoid the float/DateTimeField mismatch and let Django manage the timestamp.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@api/leaderboard/celery.py`:
- Around line 180-209: fetch_github_contributions currently returns 0 on any
non-200 response and fetch_repo_stars may return a partial total if a later
paged request fails; change both functions (fetch_github_contributions and
fetch_repo_stars) to return None (or raise a clear exception) when an upstream
request fails so callers can detect failed fetches and avoid persisting
zero/partial values, and update the caller logic that saves metrics to skip
persistence when these functions return None; for fetch_repo_stars specifically,
if any page request returns non-200 or parsing of the Link header fails, abort
and return None instead of the accumulated total.
In `@api/leaderboard/friends.py`:
- Line 59: Wrap DRF validation errors into the JSON shape the frontend expects
by transforming serializer.errors into a single "message" string before
returning; update the return that currently does Response(serializer.errors,
status=status.HTTP_400_BAD_REQUEST) to instead extract the first field error
(e.g., next(iter(serializer.errors.values()))[0] or join messages) and return
Response({"message": "<extracted message>"}, status=status.HTTP_400_BAD_REQUEST)
so the frontend (Friends.jsx) can read errorData.message consistently.
---
Outside diff comments:
In `@api/leaderboard/management/commands/update_db.py`:
- Around line 166-183: The nested GitHub API call using requests.get(url) (where
response = requests.get(url).json() and later used to compute repositories/stars
for gh_user) lacks a timeout and can hang; update that call to include a
sensible timeout (e.g. timeout=5) and wrap it in a try/except for
requests.exceptions.RequestException so you can fallback (e.g. treat response as
[] or log the error) before computing repositories/stars and appending instance
to updates to avoid blocking the update_db workflow.
In `@api/leaderboard/views.py`:
- Around line 610-628: The PUT handler in put(self, request) fetches
DiscussionPost by title but doesn't restrict edits to content fields; update it
so ownership is enforced: before applying updates, check if any content fields
("title", "discription") are present in request.data and if so verify
post.author == request.user (or return a 403 Response if not); allow non-authors
to modify only non-content fields ("likes", "dislikes", "comments"); keep the
current save/serialize flow after the checks.
- Around line 722-728: The loop in the view that updates reply fields uses the
correct intent "description" but the Reply model actually defines the misspelled
field "discription", so setattr creates a new attribute instead of updating the
stored field; update the view to set the actual model field name (use
"discription" instead of "description") when iterating over allowed fields (or
accept both keys by mapping "description" -> "discription"), ensuring the Reply
instance is modified before reply.save() and serialized via
ReplyPost_Serializer(reply).
In `@app/src/components/Blogs.jsx`:
- Around line 48-58: handleCreateBlog currently accesses authTokens.access
directly and can send "Bearer undefined" or throw; update handleCreateBlog to
guard using authTokens?.access (or check if authTokens && authTokens.access) and
early-return with a user-facing error or prevent submission if missing.
Alternatively, disable the "Write Blog" dialog/submit when authTokens?.access is
falsy; ensure the Authorization header is only added when authTokens?.access
exists and mirror the safe pattern used in fetchBlogs to avoid TypeError or
malformed headers.
- Around line 90-101: The like/dislike handler currently assumes
authTokens.access exists (seen in the fetch call inside handleInteract), which
will break for unauthenticated users; update handleInteract to first check
authTokens and authTokens.access and early-return (or show a login prompt/toast)
if missing, preventing the fetch to `${BACKEND}/discussionpost/` and avoiding
malformed Authorization headers; ensure the same guard is applied before making
the PUT request and before any UI state that assumes auth (use the symbols
authTokens.access, handleInteract, and BACKEND to locate the change).
---
Duplicate comments:
In `@app/src/components/Blogs.jsx`:
- Around line 112-135: handleDeleteBlog should guard against missing auth and
use a stable identifier for deletion: check authTokens?.access before making the
request (same pattern as fetchBlogs) and prefer sending post.id (or include the
id in the DELETE URL like `${BACKEND}/discussionpost/${post.id}`) instead of
title to avoid collisions; also replace or complement console.error calls in
handleDeleteBlog with user-facing feedback (e.g., alert or toast) on both non-ok
responses and caught errors so the user is informed when deletion fails.
In `@app/src/components/Friends.jsx`:
- Around line 205-207: The AvatarFallback usage can crash when handle is an
empty string because handle[0].toUpperCase() will access undefined; update the
Friends component to guard and compute a safe fallback before rendering (e.g.,
derive a variable like avatarInitial = handle && handle.length > 0 ?
handle[0].toUpperCase() : '?' ) and pass that to <AvatarFallback> (refer to
AvatarFallback and the handle variable in Friends.jsx); ensure the logic handles
null/undefined too so AvatarFallback always receives a string.
- Around line 33-39: Replace the unguarded
JSON.parse(localStorage.getItem("authTokens")) calls with a small helper (e.g.,
getStoredAuthTokens or parseAuthTokens) that reads localStorage, wraps
JSON.parse in try/catch, returns null on failure, and logs or handles errors;
then update the fetch callers in Friends.jsx (the try block fetching
`${BACKEND}/${platformId}${platform.suffix}L/` and the other usages around lines
71 and 99) to call this helper and guard access to authTokens.access (early
return or refresh/unauth flow) so you never dereference access on a null/invalid
token object.
---
Nitpick comments:
In `@api/leaderboard/celery.py`:
- Around line 220-221: Remove the redundant manual assignment to
gh_user.last_updated before saving: the githubUser model's last_updated uses
auto_now=True so Django will set it on gh_user.save(); delete the line that sets
gh_user.last_updated = datetime.now().timestamp() and leave gh_user.save()
unchanged to avoid the float/DateTimeField mismatch and let Django manage the
timestamp.
In `@api/leaderboard/management/commands/update_db.py`:
- Around line 270-275: The requests in openlake_contributor_update currently
call requests.get(...).json() without timeouts (specifically the call that
assigns response = requests.get(url, headers=headers).json() and the
repo_response = requests.get(repo_url).json()), which can hang; update both
calls to pass a timeout (e.g., timeout=5 or a project-constant like
HTTP_TIMEOUT) to requests.get and handle potential
requests.exceptions.Timeout/RequestException where appropriate in
openlake_contributor_update to maintain consistency with other providers.
- Around line 300-301: The import re is inside the atcoder_user_update function
which causes repeated import overhead and violates convention; move the import
to the module level with the other imports (add a single top-of-file import for
re) and remove the in-function statement from atcoder_user_update so the
function uses the module-level re.
In `@api/leaderboard/views.py`:
- Around line 761-784: The endpoint currently allows any slug/tier strings;
update AchievementUnlock.post to validate the incoming slug and tier against the
canonical set of allowed achievements before creating records: either query the
authoritative source (e.g., an AchievementDefinition model or a module-level
VALID_ACHIEVEMENTS mapping) to confirm the slug exists and that the tier is
among the allowed tiers for that slug, and return a 400 Response with an
explanatory error if not valid; alternatively, move validation into an
AchievementUnlockSerializer that performs the same checks and use
serializer.is_valid() before calling Achievement.objects.get_or_create,
referencing the Achievement model, AchievementUnlock view, and
AchievementSerializer to locate where to apply this validation.
In `@app/src/components/Friends.jsx`:
- Around line 49-61: The current loadAllFriends function performs sequential
requests by awaiting fetchFriends(platform.id) inside a for loop, slowing
loading; change it to run all fetches concurrently using Promise.all over
PLATFORMS.map(p => fetchFriends(p.id)) and await that single Promise, while
preserving setLoading(true) before and setLoading(false) after; update
loadAllFriends and its use in the useEffect (references: loadAllFriends,
fetchFriends, PLATFORMS, setLoading, useEffect) so all platform fetches run in
parallel and errors are still handled appropriately.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: c3b2b340-4720-4419-9553-617e4af10c81
📒 Files selected for processing (7)
api/leaderboard/celery.pyapi/leaderboard/friends.pyapi/leaderboard/management/commands/update_db.pyapi/leaderboard/migrations/0013_achievement.pyapi/leaderboard/views.pyapp/src/components/Blogs.jsxapp/src/components/Friends.jsx
This comment has been minimized.
This comment has been minimized.
Contributor
There was a problem hiding this comment.
Actionable comments posted: 5
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (5)
api/leaderboard/views.py (1)
753-774:⚠️ Potential issue | 🟡 Minor
DiscussionReplyManage.putassumes one reply per user per post.The query
ReplyPost.objects.get(username=request.user, parent=post)will raiseMultipleObjectsReturnedif a user has posted multiple replies to the same discussion. Consider usingfilter().first()or requiring a reply ID.🔧 Proposed fix - require reply ID
def put(self, request): + reply_id = request.data.get("reply_id") + if not reply_id: + return Response( + {"error": "reply_id is required"}, status=status.HTTP_400_BAD_REQUEST + ) + try: - post = DiscussionPost.objects.get(title=request.data["title"]) - except DiscussionPost.DoesNotExist: + reply = ReplyPost.objects.get(id=reply_id, username=request.user) + except ReplyPost.DoesNotExist: return Response( - {"error": "Post not found"}, status=status.HTTP_404_NOT_FOUND + {"error": "Reply not found or not owned by you"}, status=status.HTTP_404_NOT_FOUND ) - - try: - reply = ReplyPost.objects.get(username=request.user, parent=post) - except ReplyPost.DoesNotExist: - return Response( - {"error": "Reply not found"}, status=status.HTTP_404_NOT_FOUND - )🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@api/leaderboard/views.py` around lines 753 - 774, The put handler DiscussionReplyManage.put currently assumes a single reply per user per post by calling ReplyPost.objects.get(username=request.user, parent=post), which can raise MultipleObjectsReturned; require a reply identifier instead and fetch by it: read reply_id from request.data (e.g., "reply_id"), then call ReplyPost.objects.get(id=reply_id, username=request.user, parent=post) and handle ReplyPost.DoesNotExist returning 404; alternatively, if you prefer not to require an ID, replace the get with ReplyPost.objects.filter(username=request.user, parent=post).first() and return 404 when the result is None—ensure you update validation and error messages accordingly.api/leaderboard/api/views.py (1)
196-204:⚠️ Potential issue | 🟡 MinorMissing AtCoder username support in
registerUser.The
registerUserfunction handlescc_uname,cf_uname,gh_uname, andlt_uname, but does not extract or persistac_uname. This creates an inconsistency withpost_UserNameswhich does support AtCoder, and means users registering with an AtCoder username won't have it saved.🔧 Proposed fix
lt_uname = request.data.get("lt_uname", "") + ac_uname = request.data.get("ac_uname", "") if not all([first_name, email, username]):And update the UserNames creation:
userName = UserNames( user=user, cc_uname=cc_uname, cf_uname=cf_uname, gh_uname=gh_uname, lt_uname=lt_uname, + ac_uname=ac_uname, )And add AtcoderUser creation:
if lt_uname: lt_user = LeetcodeUser(username=lt_uname) lt_user.save() logger.info(f"LeetCode username {lt_uname} saved") + if ac_uname: + ac_user = AtcoderUser(username=ac_uname) + ac_user.save() + logger.info(f"AtCoder username {ac_uname} saved")🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@api/leaderboard/api/views.py` around lines 196 - 204, registerUser currently collects and saves cc_uname, cf_uname, gh_uname, lt_uname into the UserNames model but omits the AtCoder username; update the request parsing in registerUser to also extract ac_uname, include ac_uname when constructing/saving the UserNames instance (UserNames(..., ac_uname=ac_uname)), and, where other per-platform user records are created (e.g., CodeChefUser/CodeForcesUser/LeetCodeUser creation logic), add creation of the AtcoderUser record linked to the user using ac_uname so AtCoder usernames are persisted consistently with post_UserNames.app/src/components/Blogs.jsx (1)
39-41:⚠️ Potential issue | 🟡 MinorMissing
authTokensin useEffect dependency array.The
fetchBlogsfunction usesauthTokens?.accessto conditionally send the Authorization header, butauthTokensis not listed in the dependency array. This meansfetchBlogswon't re-run when the user logs in or out, potentially showing staleuser_votedata.Proposed fix
useEffect(() => { fetchBlogs(); - }, []); + }, [authTokens?.access]);🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@app/src/components/Blogs.jsx` around lines 39 - 41, The useEffect currently calls fetchBlogs() with an empty dependency array, but fetchBlogs reads authTokens?.access so add authTokens to the dependency array of the useEffect that invokes fetchBlogs (i.e., update the effect that contains useEffect(() => { fetchBlogs(); }, [])) so it re-runs when authTokens changes; ensure fetchBlogs is stable (memoize with useCallback if necessary) or include its reference in the dependency list to avoid stale closures.app/src/components/Achievements.jsx (2)
16-61:⚠️ Potential issue | 🟡 MinorMissing
authTokensin useEffect dependency array causes stale closure.The effect uses
authTokens.accessin bothfetchAllStatsandfetchUnlocked, but only[user, BACKEND]are in the dependency array. This causes:
- ESLint exhaustive-deps warning
- If tokens refresh while the component is mounted, requests will use stale tokens
- On initial mount,
authTokensmay be undefined briefly, causing the request to failProposed fix
fetchAllStats(); fetchUnlocked(); - }, [user, BACKEND]); + }, [user, BACKEND, authTokens?.access]);🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@app/src/components/Achievements.jsx` around lines 16 - 61, The useEffect closure reads authTokens.access in fetchAllStats and fetchUnlocked but the dependency array only lists [user, BACKEND], causing stale tokens and an ESLint exhaustive-deps warning; update the effect dependencies to include authTokens (or authTokens.access) and any setters used, and ensure fetchAllStats and fetchUnlocked use the latest authTokens by referencing the same variable inside the effect (functions: fetchAllStats, fetchUnlocked) or memoizing tokens before the calls; also guard early when authTokens or authTokens.access is falsy to avoid firing requests with undefined tokens and maintain setIsLoading/setUnlockedAchievements behavior.
64-102:⚠️ Potential issue | 🟠 MajorAchievement unlock effect inefficiently re-evaluates all achievements when one unlocks.
This effect includes
unlockedAchievementsin its dependency array, causing it to re-run whenever an achievement is unlocked. While thehasItcheck prevents duplicate API calls for the same achievement, the effect still re-evaluates all achievements on every unlock, which is inefficient. Additionally, multipleunlockAchievementcalls can be in flight simultaneously, creating potential race conditions where concurrent requests update state unpredictably.Remove
unlockedAchievementsfrom the dependency array:- }, [stats, user, BACKEND, globalStreak, unlockedAchievements]); + // eslint-disable-next-line react-hooks/exhaustive-deps + }, [stats, user, BACKEND, globalStreak]);The effect should only re-evaluate when stats, user, BACKEND, or streak changes—not when unlocked achievements change. The initial check ensures new unlocks are properly detected without re-running when the unlock list updates.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@app/src/components/Achievements.jsx` around lines 64 - 102, The effect re-runs on every unlockedAchievements change causing unnecessary full re-evaluations and possible race conditions; remove unlockedAchievements from the useEffect dependency array so the effect only runs on stats, user, BACKEND, or globalStreak changes, and prevent concurrent duplicate unlock requests by adding an in-flight tracker (e.g., a useRef Set called inflightUnlocks) that unlockAchievement checks/updates before sending a POST; keep setUnlockedAchievements using the functional updater (setUnlockedAchievements(prev => [...prev, newUnlock])) as used to avoid lost updates from concurrent responses.
🧹 Nitpick comments (7)
api/leaderboard/views.py (3)
349-360: Use timezone-aware datetime for consistency.
datetime.now()returns a naive datetime without timezone info. The model'slast_updatedfield usesauto_now=Truewhich stores timezone-aware datetimes. Comparing naive and aware datetimes can cause subtle bugs. Use Django's timezone-awarenow()instead.♻️ Proposed fix
Add to imports:
+from django.utils.timezone import nowThen update the filter:
- for user in cc_users.filter(last_updated__lt=datetime.now() - timedelta(minutes=15))[:5]: + for user in cc_users.filter(last_updated__lt=now() - timedelta(minutes=15))[:5]:And update line 359:
- user.last_updated = datetime.now() + user.last_updated = now()🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@api/leaderboard/views.py` around lines 349 - 360, The code is comparing naive datetimes; replace uses of datetime.now() with Django's timezone-aware now(): import django.utils.timezone.now and update the cc_users.filter(...) condition and the assignment user.last_updated = ... to use timezone.now() instead of datetime.now(); keep timedelta usage as-is and leave get_codechef_data, user.rating, user.save() logic unchanged.
696-696: Remove duplicate import.
IsAuthenticatedOrReadOnlyis already imported at line 13.♻️ Proposed fix
-from rest_framework.permissions import IsAuthenticatedOrReadOnly - class AtcoderViewSet(mixins.ListModelMixin, mixins.CreateModelMixin, generics.GenericAPIView):🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@api/leaderboard/views.py` at line 696, Remove the duplicate import of IsAuthenticatedOrReadOnly from rest_framework.permissions in the file; locate the redundant import statement (the one shown at the top of the snippet) and delete it so only the original import (already present near the top of the module) remains, ensuring no other imports on that line are affected.
593-608: N+1 query problem when annotating user votes.This loop queries
PostVotefor each post individually, causing N+1 database queries. For a page with many posts, this will significantly impact performance.♻️ Proposed fix using a single query
def get(self, request): posts = DiscussionPost.objects.all() - data = [] - for post in posts: - post_data = DiscussionPost_Serializer(post).data - # Annotate with the authenticated user's current vote - if request.user.is_authenticated: - try: - vote = PostVote.objects.get(user=request.user, post=post) - post_data["user_vote"] = vote.vote_type - except PostVote.DoesNotExist: - post_data["user_vote"] = None - else: - post_data["user_vote"] = None - data.append(post_data) - return Response(data, status=status.HTTP_200_OK) + serialized = DiscussionPost_Serializer(posts, many=True).data + + if request.user.is_authenticated: + # Fetch all votes for this user in one query + user_votes = { + v.post_id: v.vote_type + for v in PostVote.objects.filter(user=request.user, post__in=posts) + } + for post_data in serialized: + post_data["user_vote"] = user_votes.get(post_data["id"]) + else: + for post_data in serialized: + post_data["user_vote"] = None + + return Response(serialized, status=status.HTTP_200_OK)🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@api/leaderboard/views.py` around lines 593 - 608, The get method on the view is causing an N+1 query by calling PostVote.objects.get for every DiscussionPost; fix it by loading all relevant votes in one query when request.user.is_authenticated (use PostVote.objects.filter(user=request.user, post__in=posts)), build a mapping from post id to vote_type, then iterate posts and set post_data["user_vote"] = votes_map.get(post.id, None) before appending; keep using DiscussionPost_Serializer(post).data and only fall back to per-post logic when the user is anonymous.api/leaderboard/users.py (1)
72-77: Avoid bareexceptand move import to top of file.The bare
exceptclause catches all exceptions includingKeyboardInterruptandSystemExit, which can mask real issues. Also, theAtcoderUserSerializerimport should be at the module level with other serializer imports for consistency and slight performance benefit.♻️ Proposed fix
Add to imports at line 14-21:
from leaderboard.serializers import ( CC_Serializer, CF_Serializer, GH_Serializer, LT_Serializer, OL_Serializer, UserNamesSerializer, + AtcoderUserSerializer, )Then update lines 72-77:
try: atcoderDetails = AtcoderUser.objects.get(username=userDetails["ac_uname"]) - from leaderboard.serializers import AtcoderUserSerializer atcoderDetails = AtcoderUserSerializer(atcoderDetails).data - except: + except (AtcoderUser.DoesNotExist, KeyError): atcoderDetails = {}🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@api/leaderboard/users.py` around lines 72 - 77, Replace the bare except and inline import: move the AtcoderUserSerializer import to the module-level with other serializers, then wrap the lookup in a try/except that only handles AtcoderUser.DoesNotExist (i.e. catch that specific exception from AtcoderUser.objects.get) and set atcoderDetails = {} in that case; do not silence other exceptions so errors propagate. Ensure you reference AtcoderUser.objects.get and AtcoderUserSerializer when updating the code.api/leaderboard/api/views.py (1)
152-156: Remove duplicate import and redundant logging configuration.The
loggingmodule is already imported at line 1, andloggeris already initialized at line 29. This duplicate setup is unnecessary andlogging.basicConfig()should generally be called once at application startup, not in a module.♻️ Proposed fix
-import logging - -# Configure logging -logging.basicConfig(level=logging.DEBUG) -logger = logging.getLogger(__name__)🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@api/leaderboard/api/views.py` around lines 152 - 156, This module contains a duplicate import and redundant logging setup: remove the second "import logging", the "logging.basicConfig(...)" call and the duplicate "logger = logging.getLogger(__name__)" so the module uses the single existing logger instance already initialized elsewhere (the earlier logger variable). Ensure no calls to logging.basicConfig remain in this module and that all logging uses the existing logger symbol.app/src/components/Blogs.jsx (2)
75-96: Optimistic update logic has edge case issues when swapping votes.When swapping from like to dislike (or vice versa), the logic for decrementing the opposite counter is correct, but the increment logic doesn't account for the swap scenario properly. When
swappingis true:
- Line 88-90: For
action === "like", it correctly adds 1 to likes, but theswappingbranch decrements likes by 1 when it should leave it alone (the swap adds +1 to the new action).- The ternary nesting makes this hard to follow and may produce incorrect counts in edge cases.
Additionally,
Math.max(0, ...)is only applied to the decrement on swap, but not to thealreadyVoteddecrement case (lines 89, 92), which could theoretically go negative if state becomes inconsistent.Clearer optimistic update logic
setBlogs(prevBlogs => prevBlogs.map(b => { if (b.id !== post.id) return b; const alreadyVoted = b.user_vote === action; const swapping = b.user_vote && b.user_vote !== action; + + let newLikes = b.likes; + let newDislikes = b.dislikes; + + if (action === "like") { + newLikes = alreadyVoted ? Math.max(0, b.likes - 1) : b.likes + 1; + if (swapping) newDislikes = Math.max(0, b.dislikes - 1); + } else { + newDislikes = alreadyVoted ? Math.max(0, b.dislikes - 1) : b.dislikes + 1; + if (swapping) newLikes = Math.max(0, b.likes - 1); + } + return { ...b, - likes: action === "like" - ? alreadyVoted ? b.likes - 1 : b.likes + 1 - : swapping ? Math.max(0, b.likes - 1) : b.likes, - dislikes: action === "dislike" - ? alreadyVoted ? b.dislikes - 1 : b.dislikes + 1 - : swapping ? Math.max(0, b.dislikes - 1) : b.dislikes, + likes: newLikes, + dislikes: newDislikes, user_vote: alreadyVoted ? null : action, }; }));🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@app/src/components/Blogs.jsx` around lines 75 - 96, The optimistic update in handleInteract incorrectly handles swap cases and can produce negative counts; refactor the setBlogs update for clarity: compute alreadyVoted = b.user_vote === action and swapping = b.user_vote && b.user_vote !== action, then update likes and dislikes with explicit branches that (1) if alreadyVoted decrement the respective counter using Math.max(0, ...) to avoid negatives and set user_vote null, (2) if swapping decrement the old counter using Math.max(0, ...) and increment the new counter, and (3) if neither just increment the targeted counter; update user_vote to alreadyVoted ? null : action so counts always reflect the swap correctly in handleInteract and the mapped blog object.
112-114: Silent failure on network errors loses user feedback.When the catch block runs, it silently refetches without notifying the user that their action may have failed. Combined with the optimistic update, users might see their vote appear, then disappear without explanation.
Consider adding user feedback on error
- } catch { - fetchBlogs(); + } catch (error) { + console.error("Error updating vote:", error); + fetchBlogs(); // Revert optimistic update }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@app/src/components/Blogs.jsx` around lines 112 - 114, The catch block currently swallows errors and only calls fetchBlogs(), which hides network failures and provides no user feedback after an optimistic update; change the catch to accept the error (catch (err)) and in that handler: log the error, show a user-facing notification (e.g., call your toast or setError handler) explaining the vote failed, and revert the optimistic UI by calling fetchBlogs() (or undo the local state change) so the UI and server stay consistent; update the catch around the network-vote call (the function invoking fetchBlogs() and the optimistic update, e.g., voteBlog/handleVote) to implement these steps.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@api/leaderboard/users.py`:
- Around line 34-44: The early return inside the UserNames.DoesNotExist
exception handler returns a Response missing the "atcoder" key, causing
inconsistent API responses; update the Response returned in that exception
branch (the dict constructed in the except for UserNames.DoesNotExist) to
include "atcoder": {} so it matches the normal response shape returned later
(see the usual response dict with "codechef", "codeforces", "github",
"leetcode", "openlake", and "atcoder").
In `@api/leaderboard/views.py`:
- Around line 833-837: UserNamesList currently returns all UserNames to anyone;
update it to enforce authentication and/or scope results: add permission_classes
= [IsAuthenticated] on the UserNamesList class and change get to only return the
authenticated user’s records (e.g. UserNames.objects.filter(user=request.user))
or, if the model supports public visibility, filter to only public entries (e.g.
UserNames.objects.filter(is_public=True)) before serializing with
UserNamesSerializer; ensure you import and use
rest_framework.permissions.IsAuthenticated.
In `@app/src/components/Achievements.jsx`:
- Around line 73-74: The Authorization header currently uses authTokens.access
directly which can throw if authTokens is undefined; update the header
construction in Achievements.jsx to use optional chaining and a safe fallback
(e.g., `Bearer ${authTokens?.access || ''}`) or omit the header when no token is
present (build headers conditionally where the template literal
`'Authorization': `Bearer ${authTokens.access}`` is used) so that
authTokens.access is never accessed without a guard.
- Line 48: In fetchUnlocked, the Authorization header uses authTokens.access
directly which can throw if authTokens is undefined; change it to use optional
chaining (authTokens?.access) so the header construction is safe and consistent
with other calls—update the headers object in the fetchUnlocked function to
reference authTokens?.access and ensure any related bearer string formatting
still works when access is undefined.
- Around line 22-24: The fetch call in Achievements.jsx uses authTokens.access
directly which can throw if authTokens is null/undefined even when user is
truthy; update the logic around the fetch (the block that calls
fetch(`${BACKEND}/userDetails/`, { headers: { Authorization: `Bearer
${authTokens.access}` } })) to guard for missing tokens (e.g., check authTokens
&& authTokens.access or use optional chaining authTokens?.access) and bail out
or handle the no-token case (skip the request, set fallback state, or trigger
auth flow) so the component no longer throws a TypeError during auth
initialization race conditions.
---
Outside diff comments:
In `@api/leaderboard/api/views.py`:
- Around line 196-204: registerUser currently collects and saves cc_uname,
cf_uname, gh_uname, lt_uname into the UserNames model but omits the AtCoder
username; update the request parsing in registerUser to also extract ac_uname,
include ac_uname when constructing/saving the UserNames instance (UserNames(...,
ac_uname=ac_uname)), and, where other per-platform user records are created
(e.g., CodeChefUser/CodeForcesUser/LeetCodeUser creation logic), add creation of
the AtcoderUser record linked to the user using ac_uname so AtCoder usernames
are persisted consistently with post_UserNames.
In `@api/leaderboard/views.py`:
- Around line 753-774: The put handler DiscussionReplyManage.put currently
assumes a single reply per user per post by calling
ReplyPost.objects.get(username=request.user, parent=post), which can raise
MultipleObjectsReturned; require a reply identifier instead and fetch by it:
read reply_id from request.data (e.g., "reply_id"), then call
ReplyPost.objects.get(id=reply_id, username=request.user, parent=post) and
handle ReplyPost.DoesNotExist returning 404; alternatively, if you prefer not to
require an ID, replace the get with
ReplyPost.objects.filter(username=request.user, parent=post).first() and return
404 when the result is None—ensure you update validation and error messages
accordingly.
In `@app/src/components/Achievements.jsx`:
- Around line 16-61: The useEffect closure reads authTokens.access in
fetchAllStats and fetchUnlocked but the dependency array only lists [user,
BACKEND], causing stale tokens and an ESLint exhaustive-deps warning; update the
effect dependencies to include authTokens (or authTokens.access) and any setters
used, and ensure fetchAllStats and fetchUnlocked use the latest authTokens by
referencing the same variable inside the effect (functions: fetchAllStats,
fetchUnlocked) or memoizing tokens before the calls; also guard early when
authTokens or authTokens.access is falsy to avoid firing requests with undefined
tokens and maintain setIsLoading/setUnlockedAchievements behavior.
- Around line 64-102: The effect re-runs on every unlockedAchievements change
causing unnecessary full re-evaluations and possible race conditions; remove
unlockedAchievements from the useEffect dependency array so the effect only runs
on stats, user, BACKEND, or globalStreak changes, and prevent concurrent
duplicate unlock requests by adding an in-flight tracker (e.g., a useRef Set
called inflightUnlocks) that unlockAchievement checks/updates before sending a
POST; keep setUnlockedAchievements using the functional updater
(setUnlockedAchievements(prev => [...prev, newUnlock])) as used to avoid lost
updates from concurrent responses.
In `@app/src/components/Blogs.jsx`:
- Around line 39-41: The useEffect currently calls fetchBlogs() with an empty
dependency array, but fetchBlogs reads authTokens?.access so add authTokens to
the dependency array of the useEffect that invokes fetchBlogs (i.e., update the
effect that contains useEffect(() => { fetchBlogs(); }, [])) so it re-runs when
authTokens changes; ensure fetchBlogs is stable (memoize with useCallback if
necessary) or include its reference in the dependency list to avoid stale
closures.
---
Nitpick comments:
In `@api/leaderboard/api/views.py`:
- Around line 152-156: This module contains a duplicate import and redundant
logging setup: remove the second "import logging", the
"logging.basicConfig(...)" call and the duplicate "logger =
logging.getLogger(__name__)" so the module uses the single existing logger
instance already initialized elsewhere (the earlier logger variable). Ensure no
calls to logging.basicConfig remain in this module and that all logging uses the
existing logger symbol.
In `@api/leaderboard/users.py`:
- Around line 72-77: Replace the bare except and inline import: move the
AtcoderUserSerializer import to the module-level with other serializers, then
wrap the lookup in a try/except that only handles AtcoderUser.DoesNotExist (i.e.
catch that specific exception from AtcoderUser.objects.get) and set
atcoderDetails = {} in that case; do not silence other exceptions so errors
propagate. Ensure you reference AtcoderUser.objects.get and
AtcoderUserSerializer when updating the code.
In `@api/leaderboard/views.py`:
- Around line 349-360: The code is comparing naive datetimes; replace uses of
datetime.now() with Django's timezone-aware now(): import
django.utils.timezone.now and update the cc_users.filter(...) condition and the
assignment user.last_updated = ... to use timezone.now() instead of
datetime.now(); keep timedelta usage as-is and leave get_codechef_data,
user.rating, user.save() logic unchanged.
- Line 696: Remove the duplicate import of IsAuthenticatedOrReadOnly from
rest_framework.permissions in the file; locate the redundant import statement
(the one shown at the top of the snippet) and delete it so only the original
import (already present near the top of the module) remains, ensuring no other
imports on that line are affected.
- Around line 593-608: The get method on the view is causing an N+1 query by
calling PostVote.objects.get for every DiscussionPost; fix it by loading all
relevant votes in one query when request.user.is_authenticated (use
PostVote.objects.filter(user=request.user, post__in=posts)), build a mapping
from post id to vote_type, then iterate posts and set post_data["user_vote"] =
votes_map.get(post.id, None) before appending; keep using
DiscussionPost_Serializer(post).data and only fall back to per-post logic when
the user is anonymous.
In `@app/src/components/Blogs.jsx`:
- Around line 75-96: The optimistic update in handleInteract incorrectly handles
swap cases and can produce negative counts; refactor the setBlogs update for
clarity: compute alreadyVoted = b.user_vote === action and swapping =
b.user_vote && b.user_vote !== action, then update likes and dislikes with
explicit branches that (1) if alreadyVoted decrement the respective counter
using Math.max(0, ...) to avoid negatives and set user_vote null, (2) if
swapping decrement the old counter using Math.max(0, ...) and increment the new
counter, and (3) if neither just increment the targeted counter; update
user_vote to alreadyVoted ? null : action so counts always reflect the swap
correctly in handleInteract and the mapped blog object.
- Around line 112-114: The catch block currently swallows errors and only calls
fetchBlogs(), which hides network failures and provides no user feedback after
an optimistic update; change the catch to accept the error (catch (err)) and in
that handler: log the error, show a user-facing notification (e.g., call your
toast or setError handler) explaining the vote failed, and revert the optimistic
UI by calling fetchBlogs() (or undo the local state change) so the UI and server
stay consistent; update the catch around the network-vote call (the function
invoking fetchBlogs() and the optimistic update, e.g., voteBlog/handleVote) to
implement these steps.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 67ae560e-5ad9-41e9-9142-fc76a91af64d
📒 Files selected for processing (11)
api/leaderboard/api/views.pyapi/leaderboard/migrations/0014_postvote.pyapi/leaderboard/migrations/0015_usernames_ac_uname.pyapi/leaderboard/models.pyapi/leaderboard/users.pyapi/leaderboard/views.pyapp/src/Context/AuthContext.jsxapp/src/components/Achievements.jsxapp/src/components/Blogs.jsxapp/src/components/HomePage.jsxapp/src/components/Profile.jsx
🚧 Files skipped from review as they are similar to previous changes (1)
- app/src/components/HomePage.jsx
Comment on lines
+22
to
+24
| const res = await fetch(`${BACKEND}/userDetails/`, { | ||
| headers: { Authorization: `Bearer ${authTokens.access}` }, | ||
| }); |
Contributor
There was a problem hiding this comment.
Potential crash if authTokens is null/undefined when user exists.
Line 23 accesses authTokens.access directly without optional chaining. If there's a brief window where user is truthy but authTokens hasn't been set yet (race condition during auth initialization), this will throw a TypeError.
Proposed fix: Add optional chaining and guard
const fetchAllStats = async () => {
try {
setIsLoading(true);
+ if (!authTokens?.access) {
+ setIsLoading(false);
+ return;
+ }
const res = await fetch(`${BACKEND}/userDetails/`, {
- headers: { Authorization: `Bearer ${authTokens.access}` },
+ headers: { Authorization: `Bearer ${authTokens?.access}` },
});🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@app/src/components/Achievements.jsx` around lines 22 - 24, The fetch call in
Achievements.jsx uses authTokens.access directly which can throw if authTokens
is null/undefined even when user is truthy; update the logic around the fetch
(the block that calls fetch(`${BACKEND}/userDetails/`, { headers: {
Authorization: `Bearer ${authTokens.access}` } })) to guard for missing tokens
(e.g., check authTokens && authTokens.access or use optional chaining
authTokens?.access) and bail out or handle the no-token case (skip the request,
set fallback state, or trigger auth flow) so the component no longer throws a
TypeError during auth initialization race conditions.
Comment on lines
+73
to
74
| 'Authorization': `Bearer ${authTokens.access}` | ||
| }, |
Contributor
There was a problem hiding this comment.
Same optional chaining issue with authTokens.access.
Consistent with the earlier finding, this line also accesses authTokens.access without optional chaining.
Proposed fix
headers: {
'Content-Type': 'application/json',
- 'Authorization': `Bearer ${authTokens.access}`
+ 'Authorization': `Bearer ${authTokens?.access}`
},📝 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.
Suggested change
| 'Authorization': `Bearer ${authTokens.access}` | |
| }, | |
| headers: { | |
| 'Content-Type': 'application/json', | |
| 'Authorization': `Bearer ${authTokens?.access}` | |
| }, |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@app/src/components/Achievements.jsx` around lines 73 - 74, The Authorization
header currently uses authTokens.access directly which can throw if authTokens
is undefined; update the header construction in Achievements.jsx to use optional
chaining and a safe fallback (e.g., `Bearer ${authTokens?.access || ''}`) or
omit the header when no token is present (build headers conditionally where the
template literal `'Authorization': `Bearer ${authTokens.access}`` is used) so
that authTokens.access is never accessed without a guard.
…d to show submission calendar;
This comment has been minimized.
This comment has been minimized.
@check-spelling-bot Report🔴 Please reviewSee the 📂 files view, the 📜action log, or 📝 job summary for details.Unrecognized words (34)These words are not needed and should be removedCRA elems leetcoderankingccpsSome files were automatically ignored 🙈These sample patterns would exclude them: You should consider adding them to: File matching is via Perl regular expressions. To check these files, more of their words need to be in the dictionary than not. You can use To accept these unrecognized words as correct, update file exclusions, and remove the previously acknowledged and now absent words, you could run the following commands... in a clone of the git@github.com:Arpit-Mahajan09/Leaderboard-Pro.git repository curl -s -S -L 'https://raw.githubusercontent.com/check-spelling/check-spelling/main/apply.pl' |
perl - 'https://github.com/OpenLake/Leaderboard-Pro/actions/runs/23354018728/attempts/1' &&
git commit -m 'Update check-spelling metadata'Available 📚 dictionaries could cover words (expected and unrecognized) not in the 📘 dictionaryThis includes both expected items (592) from .github/actions/spelling/expect.txt and unrecognized words (34)
Consider adding them (in with:
extra_dictionaries: |
cspell:django/dict/django.txt
cspell:software-terms/dict/softwareTerms.txt
cspell:python/src/common/extra.txt
cspell:npm/dict/npm.txt
cspell:fullstack/dict/fullstack.txtTo stop checking additional dictionaries, add (in check_extra_dictionaries: ""Warnings
|
| Count | |
|---|---|
| 1 |
See
If you see a bunch of garbage
If it relates to a ...
well-formed pattern
See if there's a pattern that would match it.
If not, try writing one and adding it to the patterns.txt file.
Patterns are Perl 5 Regular Expressions - you can test yours before committing to verify it will match your lines.
Note that patterns can't match multiline strings.
binary-ish string
Please add a file path to the excludes.txt file instead of just accepting the garbage.
File paths are Perl 5 Regular Expressions - you can test yours before committing to verify it will match your files.
^ refers to the file's path from the root of the repository, so ^README\.md$ would exclude README.md (on whichever branch you're using).
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Pull Request Description
Title: feat: Enhance streak system, sidebar transitions, user data handling & guest mode
Summary
This PR includes a set of improvements and fixes across the frontend and backend of the Leaderboard Pro application.
Changes
-Streak System
Enhanced streak calculation logic across all platforms (Codeforces, LeetCode, GitHub, AtCoder, CodeChef)
Added PlatformStreakFetcher component for modular platform-specific streak fetching
Fixed timezone and date-handling issues that caused streaks to incorrectly display as 0
Navbar now displays the global highest streak across all integrated platforms
-UI / Frontend
Improved sidebar transitions and layout (consolidated navigation groups, better spacing)
Navbar updated with streak display and improved toggle behavior
Fixed scaling/overflow issues on main content area
LandingPage layout and styling improvements
MobileMenu refined for better responsiveness
User & Auth
Improved user data handling and state management in App.jsx
Hardened guest mode layout — fixed crash on guest homepage
Improved login/register flows
Backend
Updated views.py with improved API handling
Minor fixes to models.py and users.py
Dependencies
Updated package.json and pnpm-lock.yaml with latest dependency versions
Testing
Streaks verified for all platforms
Guest mode and authenticated flows tested
Sidebar and navbar interactions tested on desktop and mobile viewports
You can paste this directly into the GitHub PR description at: 👉 https://github.com/Arpit-Mahajan09/Leaderboard-Pro/pull/new/feature/my-changes
Summary by CodeRabbit
New Features
Improvements