feat: implement analytics dashboard and fix server-side filtering#255
Conversation
✅ Deploy Preview for github-spy ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
|
Warning Rate limit exceeded
You’ve run out of usage credits. Purchase more in the billing tab. ⌛ 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. ℹ️ Review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (1)
📝 WalkthroughWalkthroughAdds a Recharts Dashboard component, extends useGitHubData to accept typed filters and fetch issues/PRs concurrently with request-id guarding, integrates the Dashboard into Tracker with debounced effect-driven fetching, and applies typing and error-handling tweaks across hooks and pages. ChangesAnalytics Dashboard Implementation
Sequence DiagramsequenceDiagram
participant User
participant Tracker
participant useDebounce
participant useGitHubData
participant GitHubAPI
participant Dashboard
User->>Tracker: set filters (search/repo/date/state)
Tracker->>useDebounce: debounce inputs
Tracker->>useGitHubData: fetchData(username, page, perPage, activeTab, filters)
useGitHubData->>GitHubAPI: GET /search/issues?q=... (issues & PRs)
GitHubAPI-->>useGitHubData: responses
useGitHubData-->>Tracker: issues/prs + totalIssues/totalPrs (guarded by requestId)
Tracker->>Dashboard: pass totals + currentTabData + theme
Dashboard-->>User: render pie & bar charts
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
Suggested labels
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 3❌ Failed checks (2 warnings, 1 inconclusive)
✅ Passed checks (2 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 |
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
There was a problem hiding this comment.
🎉 Thank you @ishwari418 for your contribution. Please make sure your PR follows https://github.com/GitMetricsLab/github_tracker/blob/main/CONTRIBUTING.md#-pull-request-guidelines
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
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 (1)
src/hooks/useGitHubData.ts (1)
47-89:⚠️ Potential issue | 🔴 Critical | ⚡ Quick win
rateLimitedis set but never cleared — the hook gets permanently blocked.Once a 403 is hit,
setRateLimited(true)runs and the early return on line 50 (if (!octokit || !username || rateLimited) return;) prevents every subsequent call for the lifetime of the component. There's no timeout, no reset on token/username change, and no manual recovery path, so users have to do a full page reload after a single rate-limit error — even after the GitHub window resets or they paste a different PAT.Consider resetting
rateLimitedwhengetOctokit/usernamechanges, or expose aresetfunction from the hook and call it on form submit / auth change. Optionally honor theX-RateLimit-Resetheader and auto-clear after that timestamp.🛠️ Suggested reset on auth change
const fetchData = useCallback( async (username: string, page = 1, perPage = 10, type?: 'issue' | 'pr', filters: any = {}) => { const octokit = getOctokit(); if (!octokit || !username || rateLimited) return; + // reset transient errors on a fresh, non-rate-limited call + setError(''); ... }, [getOctokit, rateLimited] ); + + const reset = useCallback(() => { + setRateLimited(false); + setError(''); + }, []); + return { issues, prs, totalIssues, totalPrs, loading, error, fetchData, + reset, };🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/hooks/useGitHubData.ts` around lines 47 - 89, The hook permanently blocks fetches because fetchData (inside useCallback) returns early when the rateLimited flag is true; update the hook to clear rateLimited when auth or username changes and provide a manual reset: add logic to reset setRateLimited(false) when getOctokit() or the username argument changes (e.g., in an effect watching getOctokit/username) and expose a reset function from the hook (e.g., resetRateLimit) that calls setRateLimited(false) so callers (form submit/auth change) can clear the block; optionally, when catching the 403 in fetchData, parse the X-RateLimit-Reset header from the response returned by fetchPaginated and schedule an automatic setRateLimited(false) after that timestamp.
🧹 Nitpick comments (5)
src/hooks/useGitHubData.ts (1)
12-19: ⚡ Quick winReplace
filters: anywith an explicit type.
anyremoves the main benefit of TS here — typos in keys (startDatainstead ofstartDate) silently produce broken queries. A small interface near the top of the file documents the contract and protects callers like Tracker.tsx.♻️ Suggested type
+export interface GitHubDataFilters { + search?: string; + repo?: string; + startDate?: string; + endDate?: string; + state?: 'all' | 'open' | 'closed' | 'merged'; +} + const fetchPaginated = async ( octokit: any, username: string, type: string, page = 1, per_page = 10, - filters: any = {} + filters: GitHubDataFilters = {} ) => {- async (username: string, page = 1, perPage = 10, type?: 'issue' | 'pr', filters: any = {}) => { + async (username: string, page = 1, perPage = 10, type?: 'issue' | 'pr', filters: GitHubDataFilters = {}) => {Also applies to: 47-48
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/hooks/useGitHubData.ts` around lines 12 - 19, Define a strict interface (e.g., GitHubFetchFilters) near the top of the file and replace the loose filters: any in the fetchPaginated function signature with filters: GitHubFetchFilters; the interface should include the expected keys (for example optional startDate?: string, endDate?: string, repoNames?: string[], includeForks?: boolean, etc.) so typos are caught at compile time, and update the other occurrences that currently use filters: any (the ones referenced around the other function signatures/calls) to use GitHubFetchFilters as well.src/pages/Tracker/Tracker.tsx (1)
131-171: 💤 Low valueReuse
currentFilteredDatainstead of recomputing the tab selection.
currentFilteredDataon Line 132 already representstab === 0 ? issues : prs. Passing it through keeps the two sites in sync if the selection rule ever changes.♻️ Tiny dedup
- <Dashboard - totalIssues={totalIssues} - totalPrs={totalPrs} - data={tab === 0 ? issues : prs} - theme={theme} - /> + <Dashboard + totalIssues={totalIssues} + totalPrs={totalPrs} + data={currentFilteredData} + theme={theme} + />🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/pages/Tracker/Tracker.tsx` around lines 131 - 171, The Dashboard is being passed a recomputed tab selection (data={tab === 0 ? issues : prs}) even though currentFilteredData already holds that value; update the Dashboard invocation to pass data={currentFilteredData} instead (referencing currentFilteredData and the Dashboard component) so the selection logic is centralized and stays consistent if the rule changes.src/components/Dashboard.tsx (3)
27-30: 💤 Low valueConsider adding empty state handling.
When both
totalIssuesandtotalPrsare 0, the pie chart renders but displays no meaningful data. Consider showing a "No data available" message instead.📊 Proposed empty state check
+const hasData = totalIssues > 0 || totalPrs > 0; + const pieData = [ { name: 'Issues', value: totalIssues }, { name: 'Pull Requests', value: totalPrs }, ];Then conditionally render the chart or a fallback message based on
hasData.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/components/Dashboard.tsx` around lines 27 - 30, The pie chart should handle the empty state when both totalIssues and totalPrs are 0: compute a hasData flag (e.g., const hasData = totalIssues + totalPrs > 0) and conditionally render either the pie chart using pieData or a simple fallback UI/message like "No data available" inside the Dashboard component (where pieData, totalIssues and totalPrs are defined) so the chart area shows a clear empty state instead of an empty pie.
32-32: 💤 Low valueConsider using theme-aware colors for better consistency.
The hard-coded
COLORSarray doesn't adapt to the theme. Using theme palette colors would ensure consistency with the rest of the application's color scheme.🎨 Proposed theme-aware colors
-const COLORS = ['#0088FE', '#00C49F']; +const COLORS = [theme.palette.primary.main, theme.palette.secondary.main];🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/components/Dashboard.tsx` at line 32, The hard-coded COLORS array in Dashboard (const COLORS) should be replaced with theme-aware colors so the chart adapts to the app palette; inside the Dashboard component use the app theme (e.g., via useTheme() or the project’s theme hook) and derive COLORS from theme.palette (for example primary/main and secondary/main or suitable contrast/variant keys) and update any consumers of COLORS (chart props or components that reference const COLORS) to use the new theme-derived array.
17-22: ⚡ Quick winReplace
anytypes with proper TypeScript interfaces.Using
anyfordataandthemedefeats TypeScript's type safety and can lead to runtime errors if the wrong shape is passed.♻️ Proposed type definitions
+interface DataItem { + repository_url: string; + // Add other properties as needed based on the actual data structure +} + +interface Theme { + palette: { + background: { + paper: string; + }; + }; +} + interface DashboardProps { totalIssues: number; totalPrs: number; - data: any[]; - theme: any; + data: DataItem[]; + theme: Theme; }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/components/Dashboard.tsx` around lines 17 - 22, The DashboardProps uses unsafe any types for data and theme; define proper interfaces (e.g., DataItem or IssuePrItem and DashboardTheme) that reflect the actual shapes used by the component (for DataItem include fields like id, type ('issue'|'pr'), title, createdAt, author, state, url and any metadata the component accesses; for DashboardTheme include colors, spacing, and any theme properties read by the component) and replace data: any[] with data: DataItem[] and theme: any with theme: DashboardTheme in the DashboardProps (and update the Dashboard component signature and any usages to import/consume these interfaces).
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@package.json`:
- Line 22: package.json includes server-side dependencies "express" and
"mongoose" that likely aren't used in this frontend React/Vite project; search
the repository for imports/usages of express and mongoose (look for "express"
and "mongoose" strings and any server-related folders or files) and if there are
no imports or backend files (no API/server code), remove these dependencies from
package.json and package-lock or yarn.lock and update install scripts (run
npm/yarn install after removal); if they are required by a new backend module,
instead move that module into a dedicated backend package or workspace and add
express/mongoose only to that server package to avoid bloating the frontend.
In `@src/components/Dashboard.tsx`:
- Around line 36-39: The loop using data.forEach that computes repoName from
item.repository_url is unsafe and fragile: add a null/undefined guard for item
and item.repository_url before splitting and fallback when parsing fails, then
derive repoName more robustly (e.g., trim, remove trailing slashes, attempt to
parse with the URL constructor or split on '/' from the end while ignoring empty
segments) and only increment repoCounts[repoName] when repoName is non-empty;
update the function/loop that references repository_url and repoCounts to use
this validated parsing and fallback behavior to avoid runtime errors.
In `@src/hooks/useGitHubData.ts`:
- Around line 56-76: When fetching GitHub data in the type-branching block,
avoid leaving the inactive tab’s total stale and stop older requests from
stomping newer state: update the flow in the function that calls fetchPaginated
so that when filters/search/repo/date change you still fetch both sides (or at
minimum fetch the other side’s total) instead of only updating
setIssues/setTotalIssues or setPrs/setTotalPrs for the active type; and add
request cancellation/ordering protection by using an AbortController passed into
fetchPaginated/octokit.request or by tracking an incremental requestId in the
enclosing fetchData and ignoring any responses whose id is older than the latest
before calling setIssues/setPrs/setTotalIssues/setTotalPrs.
In `@src/pages/Tracker/Tracker.tsx`:
- Line 95: The effect in the Tracker component currently includes searchTitle
and selectedRepo in its dependency array and those states update on every
keystroke (handlers around lines 175–202), causing a GitHub Search API call per
character; fix by introducing debounced versions of those inputs (e.g.,
useDebouncedValue or a small debounce hook) and use the debounced values in the
effect dependency array and API call instead of raw searchTitle/selectedRepo,
with the input handlers still updating immediate state but the API driven only
by the debounced state (debounce delay ~300–500ms) so rapid typing won’t trigger
multiple search requests.
- Around line 82-101: The effect that calls fetchData omits username from its
dependency array and handleSubmit is a no-op when page is already 0, so add
username to the useEffect deps and make handleSubmit explicitly trigger a fetch
(or set a submitted boolean that is included in the deps) to guarantee an
initial fetch; specifically, update the dependency list of the useEffect that
references fetchData/username/tab/page/... to include username, and modify
handleSubmit (which currently calls setPage(0)) to either call
fetchData(username, 1, ROWS_PER_PAGE, tab === 0 ? "issue" : "pr", { search:
searchTitle, repo: selectedRepo, startDate, endDate, state: tab === 0 ?
issueFilter : prFilter }) directly or set a new submitted state that the
useEffect watches so the fetch runs even when page === 0.
---
Outside diff comments:
In `@src/hooks/useGitHubData.ts`:
- Around line 47-89: The hook permanently blocks fetches because fetchData
(inside useCallback) returns early when the rateLimited flag is true; update the
hook to clear rateLimited when auth or username changes and provide a manual
reset: add logic to reset setRateLimited(false) when getOctokit() or the
username argument changes (e.g., in an effect watching getOctokit/username) and
expose a reset function from the hook (e.g., resetRateLimit) that calls
setRateLimited(false) so callers (form submit/auth change) can clear the block;
optionally, when catching the 403 in fetchData, parse the X-RateLimit-Reset
header from the response returned by fetchPaginated and schedule an automatic
setRateLimited(false) after that timestamp.
---
Nitpick comments:
In `@src/components/Dashboard.tsx`:
- Around line 27-30: The pie chart should handle the empty state when both
totalIssues and totalPrs are 0: compute a hasData flag (e.g., const hasData =
totalIssues + totalPrs > 0) and conditionally render either the pie chart using
pieData or a simple fallback UI/message like "No data available" inside the
Dashboard component (where pieData, totalIssues and totalPrs are defined) so the
chart area shows a clear empty state instead of an empty pie.
- Line 32: The hard-coded COLORS array in Dashboard (const COLORS) should be
replaced with theme-aware colors so the chart adapts to the app palette; inside
the Dashboard component use the app theme (e.g., via useTheme() or the project’s
theme hook) and derive COLORS from theme.palette (for example primary/main and
secondary/main or suitable contrast/variant keys) and update any consumers of
COLORS (chart props or components that reference const COLORS) to use the new
theme-derived array.
- Around line 17-22: The DashboardProps uses unsafe any types for data and
theme; define proper interfaces (e.g., DataItem or IssuePrItem and
DashboardTheme) that reflect the actual shapes used by the component (for
DataItem include fields like id, type ('issue'|'pr'), title, createdAt, author,
state, url and any metadata the component accesses; for DashboardTheme include
colors, spacing, and any theme properties read by the component) and replace
data: any[] with data: DataItem[] and theme: any with theme: DashboardTheme in
the DashboardProps (and update the Dashboard component signature and any usages
to import/consume these interfaces).
In `@src/hooks/useGitHubData.ts`:
- Around line 12-19: Define a strict interface (e.g., GitHubFetchFilters) near
the top of the file and replace the loose filters: any in the fetchPaginated
function signature with filters: GitHubFetchFilters; the interface should
include the expected keys (for example optional startDate?: string, endDate?:
string, repoNames?: string[], includeForks?: boolean, etc.) so typos are caught
at compile time, and update the other occurrences that currently use filters:
any (the ones referenced around the other function signatures/calls) to use
GitHubFetchFilters as well.
In `@src/pages/Tracker/Tracker.tsx`:
- Around line 131-171: The Dashboard is being passed a recomputed tab selection
(data={tab === 0 ? issues : prs}) even though currentFilteredData already holds
that value; update the Dashboard invocation to pass data={currentFilteredData}
instead (referencing currentFilteredData and the Dashboard component) so the
selection logic is centralized and stays consistent if the rule changes.
🪄 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: defaults
Review profile: CHILL
Plan: Pro
Run ID: b38d8a7c-dcbf-42d0-9f20-ef0cd431796c
📒 Files selected for processing (4)
package.jsonsrc/components/Dashboard.tsxsrc/hooks/useGitHubData.tssrc/pages/Tracker/Tracker.tsx
| "@primer/octicons-react": "^19.15.5", | ||
| "@vitejs/plugin-react": "^4.3.3", | ||
| "axios": "^1.7.7", | ||
| "express": "^5.2.1", |
There was a problem hiding this comment.
Verify that express and mongoose are actually required.
These are backend/server-side packages being added to a frontend React application. Based on the PR context, this appears to be a Vite-based frontend-only app with no backend server implementation shown. Adding server-side dependencies to a frontend project can:
- Bloat
node_modulesunnecessarily - Create confusion about the application architecture
- Potentially cause build issues if accidentally imported in client code
If server-side filtering is implemented purely through the GitHub Search API (as the PR summary suggests), these dependencies may not be needed.
Run the following script to check if these packages are actually imported anywhere in the codebase:
#!/bin/bash
# Description: Search for imports of express and mongoose in the source code
echo "=== Searching for 'express' imports ==="
rg -n --type=ts --type=tsx --type=js --type=jsx "from ['\"]express['\"]|require\(['\"]express['\"]\)" src/
echo -e "\n=== Searching for 'mongoose' imports ==="
rg -n --type=ts --type=tsx --type=js --type=jsx "from ['\"]mongoose['\"]|require\(['\"]mongoose['\"]\)" src/
echo -e "\n=== Checking for any server/backend files ==="
fd -e js -e ts "server|backend|api" --type fAlso applies to: 25-25
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@package.json` at line 22, package.json includes server-side dependencies
"express" and "mongoose" that likely aren't used in this frontend React/Vite
project; search the repository for imports/usages of express and mongoose (look
for "express" and "mongoose" strings and any server-related folders or files)
and if there are no imports or backend files (no API/server code), remove these
dependencies from package.json and package-lock or yarn.lock and update install
scripts (run npm/yarn install after removal); if they are required by a new
backend module, instead move that module into a dedicated backend package or
workspace and add express/mongoose only to that server package to avoid bloating
the frontend.
mehul-m-prajapati
added
quality:clean
gssoc:approved
level:advanced
type:feature
and removed
gssoc26
labels
|
@ishwari418 : good work, pls resolve conflicts |
Thanks for the review, I will soon fix the conflicts.... |
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 (1)
src/hooks/useGitHubData.ts (1)
94-128:⚠️ Potential issue | 🔴 Critical | ⚡ Quick winFix malformed try/catch block with unreachable state updates.
Lines 94–127 contain two consecutive catch blocks (invalid syntax) and attempts to reference
issueRes/prResinside the catch handler where they are out of scope. Lines 103–107 incorrectly call state setters with variables that don't exist in the catch scope and would cause a ReferenceError at runtime. Consolidate into a single catch block with proper error handling as suggested.Suggested fix
- } catch (err: unknown) { - if (requestId === lastRequestId.current) { - const error = err as { status?: number; message?: string }; - if (error.status === 403) { - setError('GitHub API rate limit exceeded. Please wait or use a token.'); - setRateLimited(true); - } else { - setError(error.message || 'Failed to fetch data'); - } - setIssues(issueRes.items); - setPrs(prRes.items); - setTotalIssues(issueRes.total); - setTotalPrs(prRes.total); - setRateLimited(false); - } catch (err: any) { - const errorMessage = err.message?.toLowerCase() || ""; - if (err.status === 403) { - setError('GitHub API rate limit exceeded. Please provide a PAT to continue.'); - setRateLimited(true); - } else if (errorMessage.includes("do not exist")){ - setError('User not found. Please check the spelling of the GitHub username.'); - } else if (err.status === 401 || errorMessage.includes("permission")){ - setError('Private repository detected. Please input PAT.'); - }else if(err.status===404){ - setError('Resource not found.'); - } - else if (errorMessage.includes("validation failed")) { - setError('Invalid GitHub username or insufficient permissions.'); - } - else { - setError( - 'Unable to fetch GitHub data. Please verify the username, token, or network connection.' - ); - } + } catch (err: unknown) { + if (requestId !== lastRequestId.current) return; + const e = err as { status?: number; message?: string }; + const errorMessage = e.message?.toLowerCase() || ''; + if (e.status === 403) { + setError('GitHub API rate limit exceeded. Please provide a PAT to continue.'); + setRateLimited(true); + } else if (errorMessage.includes('do not exist')) { + setError('User not found. Please check the spelling of the GitHub username.'); + } else if (e.status === 401 || errorMessage.includes('permission')) { + setError('Private repository detected. Please input PAT.'); + } else if (e.status === 404) { + setError('Resource not found.'); + } else if (errorMessage.includes('validation failed')) { + setError('Invalid GitHub username or insufficient permissions.'); + } else { + setError('Unable to fetch GitHub data. Please verify the username, token, or network connection.'); + }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/hooks/useGitHubData.ts` around lines 94 - 128, The try/catch is malformed: there are two consecutive catch blocks and the catch scope incorrectly references issueRes/prRes and updates state (setIssues, setPrs, setTotalIssues, setTotalPrs) that are only defined in the try. Fix by merging into a single catch that first checks requestId === lastRequestId.current, then maps errors (inspect err.status and err.message/errorMessage) to the appropriate setError and setRateLimited calls, and remove any references to issueRes or prRes from the catch; leave state updates that use issueRes/prRes inside the try where they belong and keep the finally block intact in useGitHubData.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@package.json`:
- Around line 57-64: Fix the invalid JSON and duplicate dependency entries in
devDependencies: add the missing comma after the "vite": "^5.4.10" entry to make
the object parseable, remove the earlier duplicate entries for "jasmine",
"passport", "passport-local", "supertest", and "vite" so each package appears
only once, and ensure the retained versions are the intended ones (e.g., keep
"jasmine": "^5.13.0" if that's the desired version). Confirm the final
package.json parses as valid JSON after these edits.
In `@src/hooks/useGitHubData.ts`:
- Around line 72-75: The fetchData callback in useGitHubData references the
rateLimited state but the useCallback dependency array only includes getOctokit,
causing a stale closure; update the useCallback dependencies for fetchData (the
callback defined around line 72) to include rateLimited in addition to
getOctokit so the callback sees changes to rateLimited and stops making API
calls when setRateLimited(true).
- Line 72: The hook's rateLimited flag is never cleared on success, so update
the success path in useGitHubData (the block that handles successful octokit
responses — currently around the code handling the response at the success path
after checking octokit/username) to call setRateLimited(false) so future
requests are allowed; locate the success-handling logic that currently updates
state with the fetched data (the same place where user/repo data is set) and add
setRateLimited(false) there.
In `@src/pages/ContributorProfile/ContributorProfile.tsx`:
- Around line 11-15: The Profile type in ContributorProfile.tsx is too strict:
change the bio property from string to string | null to match GitHub's /users
nullable bio; locate the type alias named Profile (avatar_url, login, bio) and
update the bio declaration to accept null so downstream code that consumes
Profile matches the API shape.
In `@src/pages/Signup/Signup.tsx`:
- Around line 41-52: The catch block in Signup.tsx is malformed (there are two
catch-like blocks/statements and a stray axios.post inside the catch), causing a
compile error; fix by consolidating into a single try/catch: remove the
duplicate/stray axios.post call inside the catch, add an error parameter to the
catch (e.g., catch (err)), and handle/log the error and setMessage appropriately
(use setMessage with an error message and optionally log err) while keeping the
original axios.post call inside the try that uses backendUrl and formData and
the navigate logic after success.
---
Outside diff comments:
In `@src/hooks/useGitHubData.ts`:
- Around line 94-128: The try/catch is malformed: there are two consecutive
catch blocks and the catch scope incorrectly references issueRes/prRes and
updates state (setIssues, setPrs, setTotalIssues, setTotalPrs) that are only
defined in the try. Fix by merging into a single catch that first checks
requestId === lastRequestId.current, then maps errors (inspect err.status and
err.message/errorMessage) to the appropriate setError and setRateLimited calls,
and remove any references to issueRes or prRes from the catch; leave state
updates that use issueRes/prRes inside the try where they belong and keep the
finally block intact in useGitHubData.
🪄 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: defaults
Review profile: CHILL
Plan: Pro
Run ID: 3234814f-ed65-4e52-a8e1-b305f2b74792
📒 Files selected for processing (10)
package.jsonsrc/components/Dashboard.tsxsrc/hooks/useDebounce.tssrc/hooks/useGitHubAuth.tssrc/hooks/useGitHubData.tssrc/pages/ContributorProfile/ContributorProfile.tsxsrc/pages/Contributors/Contributors.tsxsrc/pages/Login/Login.tsxsrc/pages/Signup/Signup.tsxsrc/pages/Tracker/Tracker.tsx
💤 Files with no reviewable changes (1)
- src/hooks/useGitHubAuth.ts
✅ Files skipped from review due to trivial changes (1)
- src/pages/Contributors/Contributors.tsx
🚧 Files skipped from review as they are similar to previous changes (1)
- src/pages/Tracker/Tracker.tsx
| "jasmine": "^5.9.0", | ||
| "mongoose": "^9.6.2", | ||
| "passport": "^0.7.0", | ||
| "passport-local": "^1.0.0", | ||
| "supertest": "^7.1.4", | ||
| "typescript-eslint": "^8.59.3", | ||
| "vite": "^5.4.10" | ||
| "jasmine": "^5.13.0", |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
head -75 package.json | tail -30Repository: GitMetricsLab/github_tracker
Length of output: 943
Fix invalid JSON and duplicate dependency keys in devDependencies.
Line 62 is missing a trailing comma after "vite": "^5.4.10", making package.json unparseable. Additionally, jasmine, passport, passport-local, supertest, and vite appear twice with conflicting or redundant versions—remove the first occurrences and keep only the second set.
Suggested cleanup
- "jasmine": "^5.9.0",
"mongoose": "^9.6.2",
- "passport": "^0.7.0",
- "passport-local": "^1.0.0",
- "supertest": "^7.1.4",
"typescript-eslint": "^8.59.3",
- "vite": "^5.4.10"
+ "vite": "^5.4.10",
"jasmine": "^5.13.0",
"jasmine-spec-reporter": "^7.0.0",
"jsdom": "^29.1.1",
- "passport": "^0.7.0",
- "passport-local": "^1.0.0",
"supertest": "^7.2.2",
- "vite": "^5.4.10",
"vitest": "^4.1.6"🧰 Tools
🪛 Biome (2.4.15)
[error] 64-64: expected , but instead found "jasmine"
(parse)
[error] 57-57: The key jasmine was already declared.
(lint/suspicious/noDuplicateObjectKeys)
[error] 59-59: The key passport was already declared.
(lint/suspicious/noDuplicateObjectKeys)
[error] 60-60: The key passport-local was already declared.
(lint/suspicious/noDuplicateObjectKeys)
[error] 61-61: The key supertest was already declared.
(lint/suspicious/noDuplicateObjectKeys)
[error] 63-63: The key vite was already declared.
(lint/suspicious/noDuplicateObjectKeys)
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@package.json` around lines 57 - 64, Fix the invalid JSON and duplicate
dependency entries in devDependencies: add the missing comma after the "vite":
"^5.4.10" entry to make the object parseable, remove the earlier duplicate
entries for "jasmine", "passport", "passport-local", "supertest", and "vite" so
each package appears only once, and ensure the retained versions are the
intended ones (e.g., keep "jasmine": "^5.13.0" if that's the desired version).
Confirm the final package.json parses as valid JSON after these edits.
| if (!octokit || !username || rateLimited) return; | ||
|
|
||
| if (!octokit || !username) return; | ||
|
|
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
cat -n src/hooks/useGitHubData.ts | head -200Repository: GitMetricsLab/github_tracker
Length of output: 5737
Add rateLimited to the useCallback dependency array.
fetchData references rateLimited in the guard condition at line 72, but the useCallback dependency array at line 134 only includes [getOctokit]. This creates a stale closure where the callback retains the initial false value of rateLimited. When setRateLimited(true) is called during rate limiting (lines 99, 112), the callback doesn't recognize the state change and continues attempting API calls despite rate limiting.
Add rateLimited to the dependency array:
Suggested fix
const fetchData = useCallback(
async (...) => {
...
},
- [getOctokit]
+ [getOctokit, rateLimited]
);📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| if (!octokit || !username || rateLimited) return; | |
| if (!octokit || !username) return; | |
| const fetchData = useCallback( | |
| async (...) => { | |
| ... | |
| }, | |
| [getOctokit, rateLimited] | |
| ); |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/hooks/useGitHubData.ts` around lines 72 - 75, The fetchData callback in
useGitHubData references the rateLimited state but the useCallback dependency
array only includes getOctokit, causing a stale closure; update the useCallback
dependencies for fetchData (the callback defined around line 72) to include
rateLimited in addition to getOctokit so the callback sees changes to
rateLimited and stops making API calls when setRateLimited(true).
| type Profile = { | ||
| avatar_url: string; | ||
| login: string; | ||
| bio: string; | ||
| }; |
There was a problem hiding this comment.
🧩 Analysis chain
🌐 Web query:
In GitHub REST API users response schema, is bio nullable (string | null)?
💡 Result:
Yes, in the GitHub REST API users response schema, the bio field is defined as nullable (string | null) [1][2][3][4]. Official documentation explicitly lists bio as required and of type string or null [1][4].
Citations:
- 1: https://docs.github.com/rest/users/users
- 2: https://docs.github.com/en/rest/users/users?apiVersion=2022-11-28
- 3: https://docs.github.com/en/rest/users/users?apiVersion=2026-03-10
- 4: https://docs.github.com/en/rest/users/users
Change Profile.bio to string | null to match GitHub API contract.
The GitHub API's /users endpoint returns bio as nullable. The current type bio: string is too strict for the API response shape.
Proposed fix
type Profile = {
avatar_url: string;
login: string;
- bio: string;
+ bio: string | null;
};📝 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.
| type Profile = { | |
| avatar_url: string; | |
| login: string; | |
| bio: string; | |
| }; | |
| type Profile = { | |
| avatar_url: string; | |
| login: string; | |
| bio: string | null; | |
| }; |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/pages/ContributorProfile/ContributorProfile.tsx` around lines 11 - 15,
The Profile type in ContributorProfile.tsx is too strict: change the bio
property from string to string | null to match GitHub's /users nullable bio;
locate the type alias named Profile (avatar_url, login, bio) and update the bio
declaration to accept null so downstream code that consumes Profile matches the
API shape.
Removed redundant signup error handling and response logic.
Removed setting state for issues and PRs in the error handling block.
|
🎉🎉 Thank you for your contribution! Your PR #255 has been merged! 🎉🎉 |
2 participants
Related Issue
Description
This PR introduces two major improvements to the GitHub Tracker:
Analytics Dashboard: Added a visual summary section at the top of the Tracker page using Recharts. It includes a Pie Chart for contribution mix (Issues vs. PRs) and a Bar Chart for activity by repository.
Server-Side Filtering: Refactored the filtering and search logic to use the GitHub Search API. This fixes a bug where filters only applied to the first 10 items. Users can now search and filter across their entire contribution history.
Code Optimization: Renamed core components for better maintainability and optimized API requests to reduce rate-limit usage.
How Has This Been Tested?
Manual Testing: Verified on localhost:5173 using a Personal Access Token.
Search Verification: Confirmed that searching for titles now works across multiple pages of data.
Visual Check: Verified that the Dashboard is fully responsive and compatible with the existing Dark/Light mode theme.
Type of Change
Bug fix
New feature
Summary by CodeRabbit
New Features
Improvements
Chores