feat: implement analytics dashboard and fix server-side filtering#255
Open
ishwari418 wants to merge 2 commits into
Open
feat: implement analytics dashboard and fix server-side filtering#255ishwari418 wants to merge 2 commits into
ishwari418 wants to merge 2 commits into
Conversation
✅ Deploy Preview for github-spy ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
Contributor
|
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 (10)
📝 WalkthroughWalkthroughThis PR adds an analytics dashboard with interactive charts to the GitHub tracker application. It extends the data fetching hook with server-side filtering capabilities, introduces a new Dashboard component using recharts to visualize issue/PR distribution and repository activity, and refactors the Tracker page to integrate the dashboard and support advanced filtering. ChangesAnalytics Dashboard Implementation
Sequence DiagramsequenceDiagram
participant User
participant Tracker
participant useGitHubData
participant GitHubAPI
participant Dashboard
User->>Tracker: Set filters (search, repo, date, state)
Tracker->>Tracker: Build filters object
Tracker->>useGitHubData: fetchData(username, page, type, filters)
useGitHubData->>GitHubAPI: GET /search/issues?q=...+filters
GitHubAPI-->>useGitHubData: issues and PRs
useGitHubData-->>Tracker: data with totalIssues/totalPrs
Tracker->>Dashboard: Pass totals + data + theme
Dashboard-->>User: Render pie chart + bar chart
Tracker-->>User: Render data table
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
Suggested labels
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 inconclusive)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Tip 💬 Introducing Slack Agent: The best way for teams to turn conversations into code.Slack Agent is built on CodeRabbit's deep understanding of your code, so your team can collaborate across the entire SDLC without losing context.
Built for teams:
One agent for your entire SDLC. Right inside Slack. 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}
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 (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", |
Contributor
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.
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
1 participant
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.
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
Release Notes