The bugs in the sidebar have been resolved and a feature has been added that starts capturing data when logged into the system. #69
Conversation
refactor: update Sidebar component to manage accordion state on navigation
|
Warning Rate limit exceeded@ArjinAlbay has exceeded the limit for the number of commits or files that can be reviewed per hour. Please wait 14 minutes and 7 seconds before requesting another review. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. 📒 Files selected for processing (2)
WalkthroughThis update introduces a new API route for fetching user analytics, adds a Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant App
participant DataInitializer
participant ActionItemsStore
participant QuickWinsStore
User->>App: Loads application
App->>DataInitializer: Mounts DataInitializer component
DataInitializer->>ActionItemsStore: Calls refreshData() if authenticated
DataInitializer->>QuickWinsStore: Calls fetchGoodIssues() and fetchEasyFixes() if authenticated
sequenceDiagram
participant Client
participant API (/api/user/[username])
participant GitHubAPIClient
Client->>API: GET /api/user/[username]
API->>GitHubAPIClient: getUserAnalytics(username)
GitHubAPIClient-->>API: analytics data or not found
API-->>Client: 200 with analytics data or error response
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~18 minutes Possibly related PRs
Poem
✨ Finishing Touches
🧪 Generate unit tests
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. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
🔭 Outside diff range comments (1)
src/app/search/page.tsx (1)
111-146: Prevent stale state updates in async analytics loadRace condition: if
userParamchanges while the request is in-flight, results for the previous user can overwrite the latest state. Add a simple guard to ignore stale responses.Apply this diff:
const loadUserAnalytics = useCallback(async () => { if (!userParam) return; setLoadingAnalytics(true); try { - const analytics = await githubAPIClient.getUserAnalytics(userParam); + const requestedUser = userParam; + const analytics = await githubAPIClient.getUserAnalytics(requestedUser); + // Ignore if userParam changed while the request was in-flight + if (requestedUser !== userParam) { + return; + } if (analytics) { const convertedAnalytics: UserAnalytics = { profile: analytics.profile ? { avatar_url: analytics.profile.avatar_url, login: analytics.profile.login, type: analytics.profile.type, - bio: analytics.profile.bio ?? null, + bio: analytics.profile.bio ?? null, public_repos: analytics.profile.public_repos, followers: analytics.profile.followers, following: analytics.profile.following, - location: analytics.profile.location ?? null, - company: analytics.profile.company ?? null, + location: analytics.profile.location ?? null, + company: analytics.profile.company ?? null, html_url: analytics.profile.html_url } : undefined, overview: analytics.overview, languages: analytics.languages, behavior: analytics.behavior }; setUserAnalytics(convertedAnalytics); } else { setUserAnalytics(null); } } catch (error) { console.error("Analytics error:", error); // Fallback to null if API fails setUserAnalytics(null); } finally { setLoadingAnalytics(false); } }, [userParam]);Optional: Since a dedicated API route was added, consider switching to
fetch(/api/user/[username])for a consistent server-side data path and easier error handling. I can provide a patch if you want to migrate.
🧹 Nitpick comments (4)
src/app/settings/page.tsx (1)
11-11: Remove commented-out GitHubTokenSetup or guard the entire sectionCommented import/usage leaves an empty “GitHub API Token” section. Either reinstate with a feature flag or remove the section to avoid confusing users.
Minimal cleanup:
-// import { GitHubTokenSetup } from '@/components/common/GitHubTokenSetup'And either remove the section header or hide it until needed:
- {/* <GitHubTokenSetup /> */} + {/* Token setup temporarily disabled; section intentionally hidden */}Also applies to: 51-51
src/components/layout/Sidebar.tsx (1)
25-26: Avoid hiding active content: auto-expand accordions based on routeCurrently, Action Required and Quick Wins stay collapsed even when their pages are active. Recommend deriving open state from pathname so active sections are expanded automatically.
Apply this diff to replace the dashboard-only close effect with route-synced open states:
- // Close accordions when navigating to dashboard - useEffect(() => { - if (pathname === '/dashboard') { - setActionRequiredOpen(false) - setQuickWinsOpen(false) - } - }, [pathname]) + // Sync accordions with current route + useEffect(() => { + setActionRequiredOpen(pathname === '/action-required') + setQuickWinsOpen(pathname === '/quick-wins') + }, [pathname])Also applies to: 28-34
src/app/api/user/[username]/route.ts (1)
35-40: Avoid brittle error checks based on message textString-matching on error messages (
includes('not found')) is fragile. Prefer checking a typed error (e.g., an error subclass with a status code) or catching specific errors fromgithubAPIClient.getUserAnalytics.
- If
getUserAnalyticsthrows with a known shape, check a status code (e.g.,error.status === 404).- Otherwise, make
getUserAnalyticsreturn a discriminated union or wrap errors with a standardized shape.If helpful, I can draft a small error utility to standardize API client errors.
src/components/quick-wins/hooks/useQuickWins.ts (1)
76-86: Avoid duplicate fetches when DataInitializer also triggers refreshesGiven DataInitializer may also call these fetch functions, guard repeated requests and silence floating Promises. This reduces redundant network traffic and flakiness.
Apply this diff:
useEffect(() => { // Load from cache first loadFromCache() // Then check if we need to fetch fresh data - if (isQuickWinsCacheExpired()) { - fetchGoodIssues() - fetchEasyFixes() - } - }, [loadFromCache, isQuickWinsCacheExpired, fetchGoodIssues, fetchEasyFixes]) + if (isQuickWinsCacheExpired()) { + if (!loading.goodIssues) void fetchGoodIssues(); + if (!loading.easyFixes) void fetchEasyFixes(); + } + }, [ + loadFromCache, + isQuickWinsCacheExpired, + fetchGoodIssues, + fetchEasyFixes, + loading.goodIssues, + loading.easyFixes + ])If the store already de-dupes in-flight requests, this becomes a no-op safety improvement and clarifies intent.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (10)
src/app/api/auth/[...nextauth]/route.ts(1 hunks)src/app/api/user/[username]/route.ts(1 hunks)src/app/layout.tsx(2 hunks)src/app/quick-wins/page.tsx(1 hunks)src/app/search/page.tsx(1 hunks)src/app/settings/page.tsx(2 hunks)src/components/layout/Sidebar.tsx(4 hunks)src/components/providers/DataInitializer.tsx(1 hunks)src/components/quick-wins/hooks/useQuickWins.ts(2 hunks)src/lib/api/github-api-client.ts(0 hunks)
💤 Files with no reviewable changes (1)
- src/lib/api/github-api-client.ts
🧰 Additional context used
🧬 Code Graph Analysis (3)
src/app/layout.tsx (1)
src/components/providers/DataInitializer.tsx (1)
DataInitializer(7-22)
src/components/providers/DataInitializer.tsx (1)
src/stores/quickWins.ts (1)
useQuickWinsStore(25-142)
src/app/api/user/[username]/route.ts (1)
src/lib/api/github-api-client.ts (1)
githubAPIClient(883-883)
🔇 Additional comments (3)
src/app/quick-wins/page.tsx (1)
17-17: Good addition—rate limit visibility near the header is appropriatePlacement just under PageHeader is sensible. Ensure RateLimitWarning renders nothing when limits are fine to avoid layout shift.
Also applies to: 73-73
src/app/layout.tsx (1)
10-10: Initialization order looks correctRunning DataInitializer after OAuthSessionSync and before children is sound. This should kick off data capture post-login.
Double-check that underlying store actions are idempotent (React Strict Mode can trigger effects twice in dev).
Also applies to: 43-44
src/components/layout/Sidebar.tsx (1)
218-223: All “good-first-issues” references updated to “good-issues”
No occurrences of the oldtab=good-first-issuesremain—links, active‐tab checks, stores, and types now consistently usegood-issues(query) andgoodIssues(state).
…rization parameters
…or better readability and efficiency
There was a problem hiding this comment.
Actionable comments posted: 3
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
src/app/api/auth/[...nextauth]/route.ts(1 hunks)src/app/api/user/[username]/route.ts(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- src/app/api/user/[username]/route.ts
| throw new Error('NEXTAUTH_SECRET is required in production.') | ||
| } | ||
| const authOptions = { | ||
| secret: NEXTAUTH_SECRET, |
There was a problem hiding this comment.
🛠️ Refactor suggestion
App Router: import NextAuth from "next-auth" and type authOptions
In app router route handlers, the recommended import is from "next-auth" (not "next-auth/next"). Also type the options to catch config mistakes early.
-import NextAuth from "next-auth/next"
+import NextAuth, { type NextAuthOptions } from "next-auth"
-const authOptions = {
+const authOptions: NextAuthOptions = {
secret: NEXTAUTH_SECRET,
providers: [📝 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.
| secret: NEXTAUTH_SECRET, | |
| import NextAuth, { type NextAuthOptions } from "next-auth" | |
| import GitHubProvider from "next-auth/providers/github" | |
| const NEXTAUTH_SECRET = process.env.NEXTAUTH_SECRET | |
| if (process.env.NODE_ENV === 'production' && !NEXTAUTH_SECRET) { | |
| throw new Error('NEXTAUTH_SECRET is required in production.') | |
| } | |
| const authOptions: NextAuthOptions = { | |
| secret: NEXTAUTH_SECRET, | |
| providers: [ | |
| GitHubProvider({ | |
| clientId: process.env.GITHUB_CLIENT_ID!, | |
| clientSecret: process.env.GITHUB_CLIENT_SECRET!, | |
| }), | |
| ], | |
| pages: { | |
| signIn: '/login', | |
| error: '/login' | |
| } | |
| } | |
| export { authOptions } | |
| export default NextAuth(authOptions) |
🤖 Prompt for AI Agents
In src/app/api/auth/[...nextauth]/route.ts at line 9, change the import of
NextAuth to come from "next-auth" instead of "next-auth/next". Additionally, add
a type annotation for the authOptions object to ensure type safety and catch
configuration errors early. This involves importing the appropriate type from
"next-auth" and applying it to the authOptions declaration.
1 participant
Summary by CodeRabbit
New Features
Improvements
Bug Fixes
Chores
Other