✨ Feat: Add GitHub metrics preview on user profile page (Fixes #50) #168
Conversation
✅ Deploy Preview for github-spy ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
WalkthroughA new Changes
Sequence Diagram(s)sequenceDiagram
participant User as User
participant UserProfile as UserProfile Component
participant ContributorProfile as ContributorProfile Component
participant MetricCard as MetricCard Component
participant MetricsSite as metrics.lecoq.io
User->>UserProfile: Open user profile page
UserProfile->>MetricCard: Render MetricCard(username)
MetricCard->>MetricsSite: Load metrics iframe for username
MetricsSite-->>MetricCard: Iframe content loaded
MetricCard->>UserProfile: Display metrics preview
User->>ContributorProfile: Open contributor profile page
ContributorProfile->>MetricCard: Render MetricCard(username)
MetricCard->>MetricsSite: Load metrics iframe for username
MetricsSite-->>MetricCard: Iframe content loaded
MetricCard->>ContributorProfile: Display metrics preview
Estimated code review effort🎯 2 (Simple) | ⏱️ ~8 minutes Assessment against linked issues
Assessment against linked issues: Out-of-scope changesNo out-of-scope changes found. Suggested labels
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 skipped from review due to trivial 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 (
|
![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
🧹 Nitpick comments (2)
src/components/MetricCard.tsx (1)
20-20: Consider making the height responsive.The fixed height of 400px might not work well on all devices and screen sizes.
Consider using a responsive height approach:
- height="400" + style={{ height: 'min(400px, 50vh)' }}src/pages/UserProfile/UserProfile.tsx (1)
46-48: Consider conditional rendering for better UX.While the MetricCard handles falsy usernames internally, it would be better to conditionally render the entire metrics section only when a username exists.
Apply this diff to improve the user experience:
- {/* GitHub Metrics Preview */} - <h3 className="text-xl font-semibold mt-6 mb-2">GitHub Metrics</h3> - <MetricCard username={username || ""} /> + {/* GitHub Metrics Preview */} + {username && ( + <> + <h3 className="text-xl font-semibold mt-6 mb-2">GitHub Metrics</h3> + <MetricCard username={username} /> + </> + )}
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
src/components/MetricCard.tsx(1 hunks)src/pages/UserProfile/UserProfile.tsx(2 hunks)
🔇 Additional comments (4)
src/components/MetricCard.tsx (2)
1-5: Component structure looks good.The TypeScript interface and component setup follow React best practices.
10-10: Good defensive programming with early return.The null check prevents unnecessary rendering when username is not provided.
src/pages/UserProfile/UserProfile.tsx (2)
3-3: Clean import statement.The MetricCard import follows the established import pattern in the file.
46-50: Good integration and section placement.The GitHub Metrics section is well-positioned above the Pull Requests section, providing a logical flow for the user profile information.
|
|
||
| if (!username) return null; | ||
|
|
||
| const metricsURL = `https://metrics.lecoq.io/${username}`; |
There was a problem hiding this comment.
💡 Verification agent
🧩 Analysis chain
Verify the external service URL and add input validation.
The metrics.lecoq.io service URL construction should be validated for security. Consider sanitizing the username input to prevent potential issues.
🏁 Script executed:
#!/bin/bash
# Description: Verify the metrics.lecoq.io service is legitimate and check for any security considerations
# Expected: Service should be accessible and legitimate
# Check if the service responds properly
curl -I "https://metrics.lecoq.io/" 2>/dev/null | head -1
# Check if a sample metrics page loads
curl -I "https://metrics.lecoq.io/octocat" 2>/dev/null | head -1Length of output: 123
Sanitize username in metrics URL and handle service errors
To prevent URL injection and ensure robustness when metrics.lecoq.io returns errors:
- In src/components/MetricCard.tsx (line 12), wrap the
usernameinencodeURIComponent: - const metricsURL =
https://metrics.lecoq.io/${username};
- const metricsURL =
https://metrics.lecoq.io/${encodeURIComponent(username)};
- Add client-side validation (e.g., propTypes, a regex or a validation library) to allow only expected characters, such as alphanumerics, hyphens, and underscores:
const isValidUsername = /^[A-Za-z0-9_-]+$/.test(username); if (!isValidUsername) { // show an error message or fallback UI }
- Gracefully handle non-200 responses when fetching metrics (e.g., show a “Metrics unavailable” state on 4xx/5xx).
This ensures the URL can’t be tampered with and the UI degrades cleanly if the service errors.
🤖 Prompt for AI Agents
In src/components/MetricCard.tsx at line 12, sanitize the username by wrapping
it with encodeURIComponent when constructing the metricsURL to prevent URL
injection. Add client-side validation for the username prop using a regex that
allows only alphanumeric characters, hyphens, and underscores; if validation
fails, display an error message or fallback UI. Additionally, update the metrics
fetching logic to handle non-200 HTTP responses gracefully by showing a "Metrics
unavailable" state when the service returns errors.
| <iframe | ||
| src={metricsURL} | ||
| width="100%" | ||
| height="400" | ||
| frameBorder="0" | ||
| title="GitHub Metrics" | ||
| className="rounded-lg shadow-md" | ||
| onLoad={() => setLoading(false)} | ||
| ></iframe> | ||
| </div> |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Add accessibility improvements.
The iframe should have better accessibility support for screen readers and keyboard navigation.
Apply this diff to improve accessibility:
<iframe
src={metricsURL}
width="100%"
height="400"
frameBorder="0"
- title="GitHub Metrics"
+ title={`GitHub Metrics for ${username}`}
className="rounded-lg shadow-md"
onLoad={() => setLoading(false)}
+ role="img"
+ aria-label={`GitHub statistics and metrics for user ${username}`}
></iframe>📝 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.
| <iframe | |
| src={metricsURL} | |
| width="100%" | |
| height="400" | |
| frameBorder="0" | |
| title="GitHub Metrics" | |
| className="rounded-lg shadow-md" | |
| onLoad={() => setLoading(false)} | |
| ></iframe> | |
| </div> | |
| <iframe | |
| src={metricsURL} | |
| width="100%" | |
| height="400" | |
| frameBorder="0" | |
| title={`GitHub Metrics for ${username}`} | |
| className="rounded-lg shadow-md" | |
| onLoad={() => setLoading(false)} | |
| role="img" | |
| aria-label={`GitHub statistics and metrics for user ${username}`} | |
| ></iframe> | |
| </div> |
🤖 Prompt for AI Agents
In src/components/MetricCard.tsx around lines 17 to 26, the iframe lacks
accessibility features for screen readers and keyboard users. Add a meaningful
aria-label attribute to the iframe describing its content, and ensure the title
attribute is descriptive and unique. This will improve screen reader support and
keyboard navigation for the iframe element.
🛠️ Refactor suggestion
Add security attributes and improve error handling for the iframe.
The iframe lacks important security attributes and error handling mechanisms.
Apply this diff to enhance security and error handling:
+ const [error, setError] = useState(false);
+
return (
<div className="w-full flex flex-col items-center p-4">
{loading && <p className="text-gray-500 mb-2">Loading metrics...</p>}
+ {error && <p className="text-red-500 mb-2">Failed to load metrics. Please try again later.</p>}
+ {!error && (
<iframe
src={metricsURL}
width="100%"
height="400"
frameBorder="0"
title="GitHub Metrics"
className="rounded-lg shadow-md"
onLoad={() => setLoading(false)}
+ onError={() => {
+ setLoading(false);
+ setError(true);
+ }}
+ sandbox="allow-scripts allow-same-origin"
+ loading="lazy"
></iframe>
+ )}
</div>
);📝 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.
| <iframe | |
| src={metricsURL} | |
| width="100%" | |
| height="400" | |
| frameBorder="0" | |
| title="GitHub Metrics" | |
| className="rounded-lg shadow-md" | |
| onLoad={() => setLoading(false)} | |
| ></iframe> | |
| </div> | |
| const [error, setError] = useState(false); | |
| return ( | |
| <div className="w-full flex flex-col items-center p-4"> | |
| {loading && <p className="text-gray-500 mb-2">Loading metrics...</p>} | |
| {error && <p className="text-red-500 mb-2">Failed to load metrics. Please try again later.</p>} | |
| {!error && ( | |
| <iframe | |
| src={metricsURL} | |
| width="100%" | |
| height="400" | |
| frameBorder="0" | |
| title="GitHub Metrics" | |
| className="rounded-lg shadow-md" | |
| onLoad={() => setLoading(false)} | |
| onError={() => { | |
| setLoading(false); | |
| setError(true); | |
| }} | |
| sandbox="allow-scripts allow-same-origin" | |
| loading="lazy" | |
| ></iframe> | |
| )} | |
| </div> | |
| ); |
🤖 Prompt for AI Agents
In src/components/MetricCard.tsx around lines 17 to 26, the iframe element is
missing key security attributes like sandbox and allow attributes to restrict
its capabilities, and it lacks error handling for load failures. Add appropriate
sandbox attributes to limit iframe permissions, include the allow attribute if
needed, and implement an onError handler to manage loading errors gracefully,
such as setting an error state or displaying a fallback UI.
mehul-m-prajapati
force-pushed
the
feat/metrics-preview
branch
from
2a37603 to
dc372ea
Compare
There was a problem hiding this comment.
Actionable comments posted: 4
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
src/components/MetricCard.tsx(1 hunks)src/pages/UserProfile/UserProfile.tsx(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- src/components/MetricCard.tsx
🔇 Additional comments (3)
src/pages/UserProfile/UserProfile.tsx (3)
1-9: LGTM!The imports are appropriate and the PR type definition covers the essential properties used in the component.
34-34: LGTM!The loading state handling is appropriate and user-friendly.
46-49: Remove extra blank line before the next sectionThe GitHub Metrics section is solid—
MetricCardreturnsnullwhenusernameis empty, so theusername || ""fallback is handled gracefully.• File:
src/pages/UserProfile/UserProfile.tsx(around lines 46–49)
Drop the stray blank line between the Metrics and Pull Requests headings for cleaner formatting:<h3 className="text-xl font-semibold mt-6 mb-2">GitHub Metrics</h3> <MetricCard username={username || ""} /> - <h3 className="text-xl font-semibold mt-6 mb-2">Pull Requests</h3>
|
|
||
| export default function UserProfile() { | ||
| const { username } = useParams(); | ||
| const [profile, setProfile] = useState<any>(null); |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Replace any type with a proper interface for better type safety.
Using any for the profile state reduces type safety and makes the code prone to runtime errors.
Define a proper interface for the GitHub user profile:
+type GitHubUser = {
+ avatar_url: string;
+ login: string;
+ bio?: string;
+ // Add other properties as needed
+};
+
- const [profile, setProfile] = useState<any>(null);
+ const [profile, setProfile] = useState<GitHubUser | null>(null);🤖 Prompt for AI Agents
In src/pages/UserProfile/UserProfile.tsx at line 13, replace the useState type
from 'any' to a properly defined interface representing the GitHub user profile.
Define an interface that includes the expected properties of the profile object,
then use this interface as the generic type parameter in useState to improve
type safety and prevent runtime errors.
| useEffect(() => { | ||
| async function fetchData() { | ||
| if (!username) return; | ||
|
|
||
| const userRes = await fetch(`https://api.github.com/users/${username}`); | ||
| const userData = await userRes.json(); | ||
| setProfile(userData); | ||
|
|
||
| const prsRes = await fetch(`https://api.github.com/search/issues?q=author:${username}+type:pr`); | ||
| const prsData = await prsRes.json(); | ||
| setPRs(prsData.items); | ||
| setLoading(false); | ||
| } | ||
|
|
||
| fetchData(); | ||
| }, [username]); |
There was a problem hiding this comment.
Add comprehensive error handling for API requests.
The data fetching logic lacks error handling, which could cause the component to break when API requests fail or when rate limits are exceeded.
Apply this diff to add proper error handling:
useEffect(() => {
async function fetchData() {
if (!username) return;
+ try {
const userRes = await fetch(`https://api.github.com/users/${username}`);
+ if (!userRes.ok) {
+ throw new Error(`Failed to fetch user: ${userRes.status}`);
+ }
const userData = await userRes.json();
setProfile(userData);
const prsRes = await fetch(`https://api.github.com/search/issues?q=author:${username}+type:pr`);
+ if (!prsRes.ok) {
+ throw new Error(`Failed to fetch PRs: ${prsRes.status}`);
+ }
const prsData = await prsRes.json();
setPRs(prsData.items);
+ } catch (error) {
+ console.error('Error fetching data:', error);
+ // Consider adding error state to display user-friendly error messages
+ } finally {
setLoading(false);
+ }
}
fetchData();
}, [username]);📝 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.
| useEffect(() => { | |
| async function fetchData() { | |
| if (!username) return; | |
| const userRes = await fetch(`https://api.github.com/users/${username}`); | |
| const userData = await userRes.json(); | |
| setProfile(userData); | |
| const prsRes = await fetch(`https://api.github.com/search/issues?q=author:${username}+type:pr`); | |
| const prsData = await prsRes.json(); | |
| setPRs(prsData.items); | |
| setLoading(false); | |
| } | |
| fetchData(); | |
| }, [username]); | |
| useEffect(() => { | |
| async function fetchData() { | |
| if (!username) return; | |
| try { | |
| const userRes = await fetch(`https://api.github.com/users/${username}`); | |
| if (!userRes.ok) { | |
| throw new Error(`Failed to fetch user: ${userRes.status}`); | |
| } | |
| const userData = await userRes.json(); | |
| setProfile(userData); | |
| const prsRes = await fetch( | |
| `https://api.github.com/search/issues?q=author:${username}+type:pr` | |
| ); | |
| if (!prsRes.ok) { | |
| throw new Error(`Failed to fetch PRs: ${prsRes.status}`); | |
| } | |
| const prsData = await prsRes.json(); | |
| setPRs(prsData.items); | |
| } catch (error) { | |
| console.error('Error fetching data:', error); | |
| // Consider adding error state to display a user-friendly message | |
| } finally { | |
| setLoading(false); | |
| } | |
| } | |
| fetchData(); | |
| }, [username]); |
🤖 Prompt for AI Agents
In src/pages/UserProfile/UserProfile.tsx around lines 17 to 32, the fetchData
function lacks error handling for the API requests, which can cause the
component to break if requests fail or rate limits are exceeded. Wrap the entire
fetchData logic in a try-catch block, handle any errors by setting an error
state or logging the error, and ensure setLoading(false) is called in a finally
block to update the loading state regardless of success or failure.
| <div className="max-w-3xl mx-auto mt-10 p-4 bg-white shadow-xl rounded-xl"> | ||
| {profile && ( | ||
| <div className="text-center"> | ||
| <img src={profile.avatar_url} className="w-24 h-24 mx-auto rounded-full" /> |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Add alt attribute for accessibility.
The image element is missing an alt attribute, which is important for accessibility and SEO.
Apply this diff to add the alt attribute:
- <img src={profile.avatar_url} className="w-24 h-24 mx-auto rounded-full" />
+ <img src={profile.avatar_url} alt={`${profile.login}'s avatar`} className="w-24 h-24 mx-auto rounded-full" />📝 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.
| <img src={profile.avatar_url} className="w-24 h-24 mx-auto rounded-full" /> | |
| <img | |
| src={profile.avatar_url} | |
| alt={`${profile.login}'s avatar`} | |
| className="w-24 h-24 mx-auto rounded-full" | |
| /> |
🤖 Prompt for AI Agents
In src/pages/UserProfile/UserProfile.tsx at line 40, the img element lacks an
alt attribute, which is essential for accessibility and SEO. Add an alt
attribute to the img tag, ideally using a descriptive text such as the user's
name or a generic description like "User avatar" to improve screen reader
support and SEO.
| {prs.map((pr, i) => ( | ||
| <li key={i}> | ||
| <a href={pr.html_url} target="_blank" className="text-blue-600 hover:underline"> | ||
| {pr.title} | ||
| </a> | ||
| </li> |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Improve list key and add security attributes for external links.
Two improvements needed for better React practices and security:
- Using array index as key is not ideal for React lists
- External links should include security attributes
Apply this diff to address these issues:
{prs.map((pr, i) => (
- <li key={i}>
- <a href={pr.html_url} target="_blank" className="text-blue-600 hover:underline">
+ <li key={pr.html_url}>
+ <a href={pr.html_url} target="_blank" rel="noopener noreferrer" className="text-blue-600 hover:underline">
{pr.title}
</a>
</li>📝 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.
| {prs.map((pr, i) => ( | |
| <li key={i}> | |
| <a href={pr.html_url} target="_blank" className="text-blue-600 hover:underline"> | |
| {pr.title} | |
| </a> | |
| </li> | |
| {prs.map((pr, i) => ( | |
| <li key={pr.html_url}> | |
| <a | |
| href={pr.html_url} | |
| target="_blank" | |
| rel="noopener noreferrer" | |
| className="text-blue-600 hover:underline" | |
| > | |
| {pr.title} | |
| </a> | |
| </li> |
🤖 Prompt for AI Agents
In src/pages/UserProfile/UserProfile.tsx around lines 53 to 58, replace the
array index used as the key in the list items with a unique and stable
identifier from the pr object, such as pr.id. Additionally, add rel="noopener
noreferrer" to the anchor tags that open external links with target="_blank" to
improve security and prevent potential vulnerabilities.
|
@itschetna : Looks like API |
Hi @mehul-m-prajapati, |
2 participants
Related Issue
Description
This PR adds a GitHub Metrics Preview to the user profile page.
A new MetricCard component was created which embeds metrics.lecoq.io for the given username.
Added MetricCard.tsx component
Integrated MetricCard into UserProfile.tsx under a new GitHub Metrics section
Shows a loading state until the metrics iframe is ready
How Has This Been Tested?
Confirmed that the metrics iframe loads dynamically based on the username from route params.
Loading indicator disappears once metrics are displayed.
Screenshots (if applicable)
no
Type of Change
Summary by CodeRabbit