-
Notifications
You must be signed in to change notification settings - Fork 63
Improved the feature of Contributor's page #147
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Conversation
✅ Deploy Preview for github-spy ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
WalkthroughThe Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant ContributorsPage
participant GitHubAPI
User->>ContributorsPage: Visit Contributors Page
ContributorsPage->>GitHubAPI: Fetch repo stats (stars, forks, issues)
ContributorsPage->>GitHubAPI: Fetch contributors
ContributorsPage->>GitHubAPI: Fetch pull requests
ContributorsPage->>GitHubAPI: Fetch issues
GitHubAPI-->>ContributorsPage: Return data
ContributorsPage->>ContributorsPage: Calculate metrics & summary stats
ContributorsPage->>User: Render dashboard (summary cards, table, graphs)
User->>ContributorsPage: Interact (pagination, filters)
ContributorsPage->>ContributorsPage: Update UI accordingly
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Assessment against linked issues
Poem
Note ⚡️ Unit Test Generation is now available in beta!Learn more here, or try it out under "Finishing Touches" below. 📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (1)
💤 Files with no reviewable changes (1)
✨ Finishing Touches🧪 Generate unit tests
🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🎉 Thank you @denshaw-09 for your contribution. Please make sure your PR follows https://github.com/GitMetricsLab/github_tracker/blob/main/CONTRIBUTING.md#-pull-request-guidelines
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 5
🧹 Nitpick comments (2)
src/pages/Contributors/Contributors.tsx (2)
179-182: Extract inline SVG icons to reusable components.The inline SVG icons for PR, Issues, and Engagement should be extracted to separate components for better maintainability and reusability.
// Create an icons file or folder const PullRequestIcon = () => ( <svg width="22" height="22" viewBox="0 0 24 24" fill="none" xmlns="http://www.w3.org/2000/svg"> <path d="M7 7V17" stroke="#64748b" strokeWidth="2" strokeLinecap="round"/> <path d="M17 7V17" stroke="#64748b" strokeWidth="2" strokeLinecap="round"/> <circle cx="7" cy="5" r="2" fill="#64748b"/> <circle cx="7" cy="19" r="2" fill="#64748b"/> <circle cx="17" cy="5" r="2" fill="#64748b"/> <circle cx="17" cy="19" r="2" fill="#64748b"/> </svg> );Also applies to: 208-210, 236-238
36-372: Consider architectural improvements for production readiness.Additional suggestions for a production-ready implementation:
- Implement data caching to reduce API calls
- Add loading states for individual metrics
- Handle partial data loading failures gracefully
- Consider using a state management solution for complex data
- Add unit tests for data calculations and UI components
Would you like me to help create a more robust data fetching layer with proper caching and error handling?
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
backend/.env.sample(1 hunks)src/pages/Contributors/Contributors.tsx(1 hunks)
🔇 Additional comments (1)
backend/.env.sample (1)
1-1: Confirm database name update across the codebaseA search for “githubTracker” across
.js,.ts,.json,.yaml,.yml,.env, and.mdfiles returned no matches. To avoid any runtime failures, please manually verify that no legacy references to the old database name remain in:
- Configuration files (e.g., CI/CD YAML, deployment scripts)
- Documentation or READMEs
- Any custom scripts or tooling
| const REPO_OWNER = "mehul-m-prajapati"; | ||
| const REPO_NAME = "github_tracker"; | ||
| const REPO_FULL = `${REPO_OWNER}/${REPO_NAME}`; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Replace hardcoded repository values with dynamic props.
The PR objective is to create a feature that showcases ANY user's top contributions, but these hardcoded values limit the component to only one specific repository. This doesn't meet the requirement of sharing a URL for different users.
-const REPO_OWNER = "mehul-m-prajapati";
-const REPO_NAME = "github_tracker";
-const REPO_FULL = `${REPO_OWNER}/${REPO_NAME}`;
+interface ContributorsPageProps {
+ repoOwner: string;
+ repoName: string;
+}
+
+const ContributorsPage: React.FC<ContributorsPageProps> = ({ repoOwner, repoName }) => {
+ const REPO_FULL = `${repoOwner}/${repoName}`;Committable suggestion skipped: line range outside the PR's diff.
🤖 Prompt for AI Agents
In src/pages/Contributors/Contributors.tsx around lines 30 to 32, the repository
owner and name are hardcoded as constants, which restricts the component to a
single repository. To fix this, refactor the component to accept the repository
owner and name as props, then construct the full repository string dynamically
from these props instead of using fixed values. This will allow the component to
display contributions for any user's repository as intended.
| useEffect(() => { | ||
| const fetchContributors = async () => { | ||
| const fetchData = async () => { | ||
| setLoading(true); | ||
| try { | ||
| const response = await axios.get(GITHUB_REPO_CONTRIBUTORS_URL, { | ||
| withCredentials: false, | ||
| }); | ||
| setContributors(response.data); | ||
| // Repo stats (stars, forks, open issues) | ||
| const repoRes = await axios.get(`https://api.github.com/repos/${REPO_FULL}`); | ||
| setRepoStats(repoRes.data); | ||
|
|
||
| // Contributors | ||
| const contribRes = await axios.get(`https://api.github.com/repos/${REPO_FULL}/contributors?per_page=100`); | ||
| setContributors(contribRes.data); | ||
|
|
||
| // PRs (all states) | ||
| const prRes = await axios.get(`https://api.github.com/repos/${REPO_FULL}/pulls?state=all&per_page=100`); | ||
| const prs = prRes.data; | ||
| const opened = prs.length; | ||
| const merged = prs.filter(pr => pr.merged_at).length; | ||
| // Calculate PR velocity (mock: 1d if any, else '-') | ||
| const velocity = opened > 0 ? "1d" : "-"; | ||
| setPrStats({ opened, merged, velocity }); | ||
|
|
||
| // Issues (all states, filter out PRs) | ||
| const issueRes = await axios.get(`https://api.github.com/repos/${REPO_FULL}/issues?state=all&per_page=100`); | ||
| const issues = issueRes.data.filter(issue => !issue.pull_request); | ||
| const openedIssues = issues.length; | ||
| const closedIssues = issues.filter(issue => issue.state === "closed").length; | ||
| // Calculate Issue velocity (mock: 5d if any, else '-') | ||
| const issueVelocity = openedIssues > 0 ? "5d" : "-"; | ||
| setIssueStats({ opened: openedIssues, closed: closedIssues, velocity: issueVelocity }); | ||
| } catch (err) { | ||
| setError("Failed to fetch contributors. Please try again later."); | ||
| setRepoStats(null); | ||
| setContributors([]); | ||
| setPrStats({ opened: 0, merged: 0, velocity: "-" }); | ||
| setIssueStats({ opened: 0, closed: 0, velocity: "-" }); | ||
| } finally { | ||
| setLoading(false); | ||
| } | ||
| }; | ||
| fetchData(); | ||
| }, [period]); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
💡 Verification agent
❓ Verification inconclusive
Address API rate limits and incomplete data fetching.
Several issues with the data fetching implementation:
- No GitHub API rate limit handling - this will fail quickly for unauthenticated requests
- Using
per_page=100without pagination means you'll miss data if there are more than 100 items - The
periodstate changes trigger refetch but isn't used in the API calls - Mock velocity calculations should be replaced with real metrics
Consider implementing:
- GitHub API authentication
- Proper pagination handling
- Real velocity calculations based on the selected period
- Rate limit monitoring
🏁 Script executed:
#!/bin/bash
# Description: Check if there's any authentication setup for GitHub API
# Search for GitHub API authentication
rg -A 5 "github.*token|GITHUB_TOKEN|Authorization.*github" --type ts --type tsx --type jsLength of output: 119
🏁 Script executed:
#!/bin/bash
# Search for environment variables related to GitHub authentication
rg -n "process\\.env" -g "*.ts" -g "*.tsx"
# Look for custom axios instances or default header overrides
rg -n "axios\\.create" -g "*.ts" -g "*.tsx"
rg -n "axios\\.defaults" -g "*.ts" -g "*.tsx"
# Check for explicit Authorization header usage
rg -n "Authorization" -g "*.ts" -g "*.tsx"Length of output: 178
Implement robust GitHub API integration and real metrics
The current Contributors.tsx data fetching has several gaps that can lead to incomplete or unauthenticated data. Please address the following in src/pages/Contributors/Contributors.tsx:
-
Authentication (lines 53, 57, 61, 70):
• NoAuthorizationheader or token usage – unauthenticated calls are limited to 60/hour.
• Add support forprocess.env.GITHUB_TOKEN(or similar) in your axios requests. -
Pagination (all calls with
per_page=100):
• You’ll miss any data beyond page 1.
• Implement a loop or follow theLinkheader to fetch all pages of contributors, PRs, and issues. -
Unused
perioddependency (useEffect deps):
• The dropdown’speriodvalue is not applied to any API query.
• Passsince/untilquery parameters (or switch to the GraphQL API) to filter data by the selected period. -
Mock velocity calculations (lines 66, 75):
• Replace hard-coded"1d"/"5d"with actual durations computed from PR/issue timestamps (e.g. average or median time to merge/close).
| <TableHead> | ||
| <TableRow> | ||
| <TableCell padding="checkbox"></TableCell> | ||
| <TableCell>REPOSITORY</TableCell> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Clarify data model: repositories vs contributors.
The table header shows "REPOSITORY" but the data displayed is contributor information. This creates confusion about what data is being presented.
Either:
- Change the header to "CONTRIBUTOR" to match the data
- Or restructure to actually show repository data as the PR objectives suggest
The PR objective mentions showing "top contributions" which could be interpreted as either top repositories or top contributors.
Also applies to: 297-304
🤖 Prompt for AI Agents
In src/pages/Contributors/Contributors.tsx around lines 270 and 297-304, the
table header "REPOSITORY" does not match the contributor data displayed, causing
confusion. To fix this, either rename the header to "CONTRIBUTOR" to align with
the data shown or modify the data rows to display repository information if the
intent is to show top repositories. Ensure consistency between the header and
the data content throughout these lines.
Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 0
♻️ Duplicate comments (5)
src/pages/Contributors/Contributors.tsx (5)
30-32: Replace hardcoded repository values with dynamic props.This issue was already identified in previous reviews. The hardcoded values prevent the component from showcasing different users' contributions as required by the PR objectives.
48-87: Address API integration issues identified in previous reviews.The data fetching implementation still has the same unresolved issues from previous reviews:
- No GitHub API authentication - will hit rate limits quickly
per_page=100without pagination loses data beyond page 1perioddependency triggers refetch but isn't used in API queries- Mock velocity calculations (lines 66, 75) should use real metrics
94-143: Replace mock implementations with real calculations.The helper functions still return mock/hardcoded data as identified in previous reviews. This doesn't provide real insights into contributor activity.
270-270: Clarify data model: repositories vs contributors.This table header confusion was already identified in previous reviews. The header shows "REPOSITORY" but displays contributor data.
299-299: Remove hardcoded avatar path.This avatar source issue was already flagged in previous reviews. The implementation now uses dynamic avatar URLs which addresses the concern.
🧹 Nitpick comments (1)
src/pages/Contributors/Contributors.tsx (1)
356-356: Fix inconsistent terminology in pagination text.The pagination text says "repos" but should say "contributors" since that's what the table actually displays.
-Showing {paginatedContributors.length > 0 ? (page - 1) * perPage + 1 : 0} - {Math.min(page * perPage, contributors.length)} of {contributors.length} repos +Showing {paginatedContributors.length > 0 ? (page - 1) * perPage + 1 : 0} - {Math.min(page * perPage, contributors.length)} of {contributors.length} contributors
Related Issue
Description
This PR completely redesigns the Contributors page to match the modern dashboard UI shown in the provided reference image.
Summary Cards:
Added Pull Requests, Issues, and Engagement cards at the top, each with a relevant icon inside a rounded box.
Labels ("Opened", "Merged", "Velocity", etc.) are now above the numbers, with bold numbers and lighter, smaller labels.
Engagement card now includes the "Activity Ratio" label above a pill-shaped "Low" button with a down-arrow icon.
How Has This Been Tested?
Manually verified the UI matches the reference image for:
Summary cards (icons, layout, labels, numbers, pill/button for activity ratio)
Table layout and styling (activity pills, PR overview bar, avatars, etc.)
Checked that all data loads correctly from the GitHub API and updates on period change.
Screenshots (if applicable)
Type of Change
Summary by CodeRabbit
New Features
Refactor
Chores