Moving data fetching to backend -Security #143
Conversation
❌ Deploy Preview for github-spy failed.
|
WalkthroughThis update introduces a backend middleware for authenticating GitHub users, new backend API endpoints for fetching user GitHub data and analytics, and corresponding frontend changes to utilize these endpoints. The frontend adds a user analytics route and page, updates the home page logic for data fetching, and comments out the previous GitHub data hook. Dependency versions are also updated. Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant Frontend (Home)
participant Backend (/api/github/get-data)
participant GitHub API
User->>Frontend (Home): Enter username & token, click Fetch
Frontend (Home)->>Backend (/api/github/get-data): POST username, token
Backend (/api/github/get-data)->>GitHub API: Authenticated requests for issues & PRs
GitHub API-->>Backend (/api/github/get-data): Issues & PRs data
Backend (/api/github/get-data)-->>Frontend (Home): JSON response with issues & PRs
Frontend (Home)-->>User: Display data or error
sequenceDiagram
participant User
participant Frontend (Analytics)
participant Backend (/api/github/user-profile)
participant GitHub API
User->>Frontend (Analytics): Navigate to /analytics with username & token
Frontend (Analytics)->>Backend (/api/github/user-profile): POST username, token
Backend (/api/github/user-profile)->>GitHub API: Fetch profile, repos, events
GitHub API-->>Backend (/api/github/user-profile): Profile, repos, events data
Backend (/api/github/user-profile)-->>Frontend (Analytics): JSON analytics data
Frontend (Analytics)-->>User: (Planned) Display analytics
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~18 minutes Assessment against linked issues
Assessment against linked issues: Out-of-scope changesNo out-of-scope changes detected. Suggested labels
Suggested reviewers
Poem
Note ⚡️ Unit Test Generation is now available in beta!Learn more here, or try it out under "Finishing Touches" below. ✨ 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: 5
🧹 Nitpick comments (8)
src/hooks/useGitHubData.ts (1)
1-62: Consider removing commented code once backend implementation is stable.While commenting out the code preserves the original implementation for reference during the migration, consider removing this dead code once the backend implementation is thoroughly tested and stable. Keeping commented code long-term can clutter the codebase.
The original implementation shows good practices with proper pagination and error handling, which should be maintained in the backend equivalent.
backend/middlewares/authenticateGitHub.js (1)
2-2: Remove or update the comment.The comment "//ashish-choudhari-git Code" should be removed or updated to follow proper code documentation standards.
-//ashish-choudhari-git Codesrc/pages/UserAnalytics/UserAnalytics.tsx (2)
4-4: Fix content inconsistency.The heading displays "Text Analytics" but the component is named
UserAnalytics. This creates confusion about the component's purpose.- <h1>Text Analytics coming soon</h1> + <h1>User Analytics coming soon</h1>
3-3: Consider using Material-UI theming for consistency.The component uses inline styles instead of the Material-UI theming system used throughout the rest of the application. This breaks visual consistency.
+import { Box, Typography } from "@mui/material"; +import { useTheme } from "@mui/material/styles"; + export default function UserAnalytics() { + const theme = useTheme(); return ( - <div style={{ fontFamily: "sans-serif", textAlign: "center", marginTop: "50px" }}> - <h1>User Analytics coming soon</h1> - </div> + <Box + sx={{ + textAlign: "center", + mt: 6, + color: theme.palette.text.primary, + }} + > + <Typography variant="h4" component="h1"> + User Analytics coming soon + </Typography> + </Box> ); }src/pages/Home/Home.tsx (2)
87-100: Fix typo in error message.There's a typo in the error message for the token validation.
- setError('Personal acess token is required.') + setError('Personal access token is required.')
234-261: Improve analytics button UX and accessibility.The analytics button implementation has good conditional logic but could be enhanced for better user experience.
<Button variant="outlined" onClick={handleViewAnalytics} + disabled={!username.trim() || !token.trim()} startIcon={<BarChart3 size={20} />} sx={{ minWidth: "140px", borderRadius: "8px", color: "secondary.main", borderColor: "secondary.main", "&:hover": { borderColor: "secondary.dark", backgroundColor: "secondary.light", color: "secondary.dark", }, + "&:disabled": { + opacity: 0.6, + cursor: "not-allowed", + }, }} > - {username && token ? 'View Analytics' : 'Enter Data First'} + View Analytics </Button>backend/routes/details.js (2)
208-245: Move helper functions outside the route handler.The helper functions are defined inside the route handler, which recreates them on each request. This impacts performance and code organization.
+// Helper functions moved outside the route handler +function isConsecutiveDay(date1, date2) { + const d1 = new Date(date1); + const d2 = new Date(date2); + const diffTime = Math.abs(d2 - d1); + const diffDays = Math.ceil(diffTime / (1000 * 60 * 60 * 24)); + return diffDays === 1; +} + +function isRecentDate(date, today) { + const diffTime = Math.abs(new Date(today) - new Date(date)); + const diffDays = Math.ceil(diffTime / (1000 * 60 * 60 * 24)); + return diffDays <= 1; +} + +function calculateCurrentStreak(dates) { + let streak = 0; + for (let i = dates.length - 1; i > 0; i--) { + if (isConsecutiveDay(dates[i-1], dates[i])) { + streak++; + } else { + break; + } + } + return streak + 1; +} + +function getDayFullName(shortDay) { + const dayMap = { + 'Sun': 'Sunday', + 'Mon': 'Monday', + 'Tue': 'Tuesday', + 'Wed': 'Wednesday', + 'Thu': 'Thursday', + 'Fri': 'Friday', + 'Sat': 'Saturday' + }; + return dayMap[shortDay] || 'Monday'; +} router.post('/user-profile', authenticateGitHub, async (req, res) => { // ... route implementation - - // Helper functions - function isConsecutiveDay(date1, date2) { - // ... implementation - } - // ... other helper functions });
260-260: Address TODO comment for commit history.The commit history is left as an empty array with a TODO comment. This might confuse users expecting this data.
The commit history feature is marked as requiring "more complex API calls." Would you like me to help implement this using GitHub's GraphQL API to fetch commit history data, or should this be tracked as a separate issue?
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (8)
backend/middlewares/authenticateGitHub.js(1 hunks)backend/routes/details.js(1 hunks)backend/server.js(1 hunks)package.json(1 hunks)src/Routes/Router.tsx(2 hunks)src/hooks/useGitHubData.ts(1 hunks)src/pages/Home/Home.tsx(6 hunks)src/pages/UserAnalytics/UserAnalytics.tsx(1 hunks)
🧰 Additional context used
🧬 Code Graph Analysis (1)
src/Routes/Router.tsx (1)
src/pages/UserAnalytics/UserAnalytics.tsx (1)
UserAnalytics(1-7)
🔇 Additional comments (8)
package.json (1)
18-18: axios 1.11.0 is confirmed as the latest stable release
- Released July 23, 2025, on npm.
- Includes bug fixes for
form-data, large-BufferRangeError, and ESM/CJS TypeScript declarations.- No known vulnerabilities as of now.
You can safely proceed with this upgrade.
src/Routes/Router.tsx (2)
9-9: LGTM: Clean import addition.The import statement follows the existing pattern and correctly imports the UserAnalytics component.
20-20: LGTM: Analytics route properly configured.The new route is correctly configured and follows the existing routing pattern. Note that based on the relevant code snippets, the UserAnalytics component currently shows a placeholder message, which aligns with the incremental development approach mentioned in the PR objectives.
backend/server.js (1)
26-27: GitHub routes verified and approved
- Confirmed backend/routes/details.js exists and properly exports an Express router.
- Endpoints
/get-dataand/user-profileare implemented with the authenticateGitHub middleware.- Integration in backend/server.js (remains at lines 26–27) is correct.
No further changes needed.
src/pages/Home/Home.tsx (2)
27-28: LGTM! Proper deprecation of old hooks.The commented-out import statements clearly indicate the migration from client-side hooks to backend API integration, which aligns with the PR's security objectives.
103-115: LGTM! Well-implemented navigation logic.The analytics navigation properly validates credentials before navigation and passes them securely via React Router state.
backend/routes/details.js (2)
6-71: LGTM! Well-structured basic data endpoint.The
/get-dataendpoint is well-implemented with proper error handling, data transformation, and response structure. The use ofPromise.allfor concurrent API calls is efficient.
74-90: LGTM! Good start to analytics endpoint.The user profile and repository fetching logic is correct and follows GitHub API best practices.
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
backend/routes/details.js (1)
14-43: Consider using an established concurrency control library.While this custom limiter implementation appears functional, using a well-tested library like
p-limitwould be more maintainable and reliable.Replace the custom implementation with
p-limit:-// Helper function to limit concurrent API calls -const createLimiter = (limit) => { - let running = 0; - const queue = []; - // ... rest of implementation -}; +const pLimit = require('p-limit');Then use it like:
const limit = pLimit(3); // Usage: await limit(() => someAsyncFunction())
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
backend/routes/details.js(1 hunks)package.json(1 hunks)src/pages/Home/Home.tsx(6 hunks)
✅ Files skipped from review due to trivial changes (1)
- package.json
🚧 Files skipped from review as they are similar to previous changes (1)
- src/pages/Home/Home.tsx
🔇 Additional comments (7)
backend/routes/details.js (7)
1-11: LGTM! Clean module setup with appropriate caching strategy.The module imports, cache configuration, and helper function setup follow Express.js best practices. The 30-minute cache duration for language data is a reasonable balance between freshness and API efficiency.
46-56: Excellent memory management with periodic cache cleanup.The cache cleanup mechanism properly prevents memory leaks by removing expired entries. The 15-minute cleanup interval is well-balanced with the 30-minute cache duration.
58-123: Well-implemented data fetching endpoint with proper error handling.The endpoint efficiently fetches issues and PRs using concurrent API calls, properly transforms the data, and includes comprehensive error handling. The search queries and response structure are appropriate for the Home page requirements.
126-222: Excellent improvements addressing previous performance concerns.This implementation successfully addresses the rate limiting and performance issues from previous reviews by:
- Limiting concurrent API calls to 3 (respecting GitHub's recommendations)
- Implementing 30-minute caching for language data
- Processing repositories in batches with delays
- Graceful error handling for individual repository failures
The concurrency control and caching strategy significantly improve performance while being respectful to GitHub's API limits.
230-357: Excellent implementation addressing previous contribution calculation concerns.This GraphQL-based approach successfully addresses the previous review feedback by:
- Using GitHub's official
contributionsCollectionAPI for accurate data- Querying comprehensive data from current and previous years
- Handling API failures gracefully with
Promise.allSettled- Calculating accurate streaks based on real contribution dates
- Providing a reasonable fallback when GraphQL is unavailable
The implementation now provides precise contribution statistics instead of the previous arbitrary calculation.
377-414: Well-implemented helper functions for date calculations.The helper functions correctly handle:
- Consecutive day detection for streak calculations
- Recent date validation for current streak determination
- Backward streak calculation from the most recent contribution
- Day name mapping for display purposes
The date arithmetic and logic are sound and handle the edge cases appropriately.
416-438: Comprehensive response structure with proper error handling.The response compilation provides a well-structured analytics payload including:
- User profile and repository data
- Language usage statistics
- Detailed contribution metrics
- Repository rankings and highlights
- Social statistics
The error handling properly logs issues and returns appropriate HTTP status codes. The placeholder for commit history is appropriately noted for future implementation.
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (2)
backend/routes/details.js (2)
1-1: Consider adding a more descriptive header comment.While the attribution is noted, consider adding a brief description of the module's purpose and main functionality.
-//Aashish Choudhari's Code | GSSoC Contributor +/** + * GitHub Data API Routes + * Provides endpoints for fetching GitHub user data and analytics + * Author: Aashish Choudhari | GSSoC Contributor + */
399-399: Implement commit history functionality.The commit history is currently a placeholder. Consider implementing this feature or removing the TODO comment.
Would you like me to help implement the commit history fetching logic or create an issue to track this feature?
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
backend/package.json(1 hunks)backend/routes/details.js(1 hunks)package.json(1 hunks)
✅ Files skipped from review due to trivial changes (2)
- backend/package.json
- package.json
🔇 Additional comments (4)
backend/routes/details.js (4)
7-26: LGTM! Well-implemented caching strategy.The in-memory cache with automatic cleanup prevents memory leaks and reduces API calls. The 30-minute cache duration and 15-minute cleanup interval are reasonable choices.
28-93: LGTM! Clean endpoint implementation.The
/get-dataendpoint is well-structured with proper error handling and data transformation. The parallel API calls usingPromise.alloptimize performance.
113-188: Excellent implementation of concurrency control and caching.This addresses the previous review comment about rate limiting issues. The implementation includes:
- Proper concurrency limiting with p-limit (3 concurrent requests)
- Intelligent caching with expiration
- Batch processing with delays between batches
- Comprehensive error handling
200-327: Improved contribution calculation using GraphQL API.This addresses the previous review comment about inaccurate contribution calculations. The implementation now uses GitHub's GraphQL API for precise contribution data from both current and previous years.
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (1)
src/pages/Home/Home.tsx (1)
120-144: Improved error handling addresses previous security concerns.The updated error handling properly addresses the previous review comment by not logging the actual token and providing more informative error messages. The implementation correctly follows the suggested improvements from the past review.
However, consider adding cleanup logic to handle component unmounting during async operations:
const fetchData= async()=>{ + const abortController = new AbortController(); setLoading(true); setError(''); try{ - const response = await axios.post(`${backendUrl}/api/github/get-data`,{ - username, token - }); //we get data from backend by providng username and token + const response = await axios.post(`${backendUrl}/api/github/get-data`,{ + username, token + }, { + signal: abortController.signal + }); //we get data from backend by providing username and token setIssues(response.data.issues); setPrs(response.data.prs); }catch(err:any){ + if (axios.isCancel(err)) { + return; // Request was cancelled, don't update state + } console.error('Error response:', err.response?.status, err.response?.statusText); setError(err.response?.data?.message || 'Failed to fetch GitHub data. Please check your credentials.'); setIssues([]); setPrs([]); } finally{ setLoading(false); } + + return () => { + abortController.abort(); + }; }
🧹 Nitpick comments (3)
src/pages/Home/Home.tsx (3)
27-28: Clean up commented code before production.The previous hooks are commented out but still present in the codebase. Consider removing these commented lines to keep the code clean, especially since the functionality has been completely replaced with local state management.
-// import { useGitHubAuth } from "../../hooks/useGitHubAuth"; -// import { useGitHubData } from "../../hooks/useGitHubData";- // const { - // username, - // setUsername, - // token, - // setToken, - // error: authError, - // getOctokit, - // } = useGitHubAuth(); - // const octokit = getOctokit(); - // const { - // issues, - // prs, - // loading, - // error: dataError, - // fetchData, - // } = useGitHubData(octokit);Also applies to: 49-64
87-100: Fix typo in error message and improve validation logic.There's a typo in the error message and the validation function could be simplified since it doesn't need to be async.
- const validateCredentials= async()=>{ + const validateCredentials = () => { if(!username.trim()){ - setError('Username is required.') + setError('Username is required.'); return false; } if(!token.trim()){ - setError('Personal acess token is required.') + setError('Personal access token is required.'); return false; } setError(''); return true; - }
147-154: Update function call to match validation function signature.The validation function is called with
awaitbut it's no longer async after the suggested fix above.const handleSubmit = async (e: React.FormEvent<HTMLFormElement>)=> { e.preventDefault(); - const isValid = await validateCredentials(); + const isValid = validateCredentials(); if(isValid){ fetchData(); } };
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
backend/package.json(1 hunks)package.json(1 hunks)src/hooks/useGitHubData.ts(1 hunks)src/pages/Home/Home.tsx(6 hunks)
✅ Files skipped from review due to trivial changes (1)
- package.json
🚧 Files skipped from review as they are similar to previous changes (2)
- src/hooks/useGitHubData.ts
- backend/package.json
🔇 Additional comments (2)
src/pages/Home/Home.tsx (2)
232-259: Well-designed UI with good user experience considerations.The button layout and conditional styling provide clear user feedback. The "View Analytics" button appropriately changes its label based on credential availability, and both buttons are properly disabled during loading states.
337-340: Simplified error handling improves code maintainability.The consolidated error display logic is cleaner and more maintainable compared to handling separate auth and data errors. This change aligns well with the new unified state management approach.
| const handleViewAnalytics = () => { | ||
| if (!username.trim() || !token.trim()) { | ||
| setError('Please enter username and token first'); | ||
| return; | ||
| } | ||
| // Pass username and token as state to analytics page | ||
| navigate('/analytics', { | ||
| state: { | ||
| username: username.trim(), | ||
| token: token.trim() | ||
| } | ||
| }); | ||
| }; |
There was a problem hiding this comment.
Security concern: Token passed via navigation state.
Passing the GitHub personal access token through React Router's navigation state could expose it in browser developer tools and potentially in browser history. Consider alternative approaches for passing sensitive data to the analytics page.
Alternative approaches:
- Store credentials in a secure context or state management solution
- Pass only a session identifier and retrieve credentials server-side
- Use encrypted storage or memory-only storage
const handleViewAnalytics = () => {
if (!username.trim() || !token.trim()) {
setError('Please enter username and token first');
return;
}
- // Pass username and token as state to analytics page
- navigate('/analytics', {
- state: {
- username: username.trim(),
- token: token.trim()
- }
- });
+ // Consider using a secure method to pass credentials
+ // such as encrypted session storage or server-side session
+ navigate('/analytics', {
+ state: {
+ username: username.trim()
+ // TODO: Implement secure token passing mechanism
+ }
+ });
};📝 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.
| const handleViewAnalytics = () => { | |
| if (!username.trim() || !token.trim()) { | |
| setError('Please enter username and token first'); | |
| return; | |
| } | |
| // Pass username and token as state to analytics page | |
| navigate('/analytics', { | |
| state: { | |
| username: username.trim(), | |
| token: token.trim() | |
| } | |
| }); | |
| }; | |
| const handleViewAnalytics = () => { | |
| if (!username.trim() || !token.trim()) { | |
| setError('Please enter username and token first'); | |
| return; | |
| } | |
| // Consider using a secure method to pass credentials | |
| // such as encrypted session storage or server-side session | |
| navigate('/analytics', { | |
| state: { | |
| username: username.trim() | |
| // TODO: Implement secure token passing mechanism | |
| } | |
| }); | |
| }; |
🤖 Prompt for AI Agents
In src/pages/Home/Home.tsx around lines 103 to 115, avoid passing the GitHub
token via React Router's navigation state as it exposes sensitive data. Instead,
refactor to store the token securely using a React context or a state management
library accessible by the analytics page, or implement a session-based approach
where only a session ID is passed and credentials are retrieved securely
server-side. Remove the token from the navigation state and ensure the analytics
page accesses the token from the secure storage method chosen.
|
@mehul-m-prajapati this bot is giving suggestions again And again, I am doing that too. I just want to know what's, what should I do , so that PR can be merged. |
|
no need to do anything. I will review merge after some time @ashish-choudhari-git |
There was a problem hiding this comment.
Actionable comments posted: 1
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
src/Routes/Router.tsx(1 hunks)src/hooks/useGitHubData.ts(1 hunks)src/pages/Home/Home.tsx(6 hunks)
🚧 Files skipped from review as they are similar to previous changes (2)
- src/Routes/Router.tsx
- src/pages/Home/Home.tsx
🔇 Additional comments (1)
src/hooks/useGitHubData.ts (1)
1-68: Acknowledge security improvement.Moving GitHub data fetching from frontend to backend is an excellent security improvement that addresses the key concerns mentioned in the PR objectives:
- Protects the GitHub personal access token from client-side exposure
- Reduces API rate limiting issues on the client side
- Centralizes authentication and API management
This architectural change significantly improves the application's security posture.
| // import { useState, useCallback } from 'react'; | ||
|
|
||
| return { | ||
| items: response.data.items, | ||
| total: response.data.total_count, | ||
| }; | ||
| }; | ||
| // export const useGitHubData = (octokit) => { | ||
| // const [issues, setIssues] = useState([]); | ||
| // const [prs, setPrs] = useState([]); | ||
| // const [loading, setLoading] = useState(false); | ||
| // const [error, setError] = useState(''); | ||
|
|
||
| const fetchData = useCallback( | ||
| async (username: string, page = 1, perPage = 10) => { | ||
| const octokit = getOctokit(); | ||
| // const fetchAll = async (url, params) => { | ||
| // let page = 1; | ||
| // let results = []; | ||
| // let hasMore = true; | ||
|
|
||
| if (!octokit || !username || rateLimited) return; | ||
| // while (hasMore) { | ||
| // const response = await octokit.request(url, { ...params, page }); | ||
| // results = results.concat(response.data.items); | ||
| // hasMore = response.data.items.length === 100; | ||
| // page++; | ||
| // } | ||
|
|
||
| setLoading(true); | ||
| setError(''); | ||
| // return results; | ||
| // }; | ||
|
|
||
| // const fetchData = useCallback(async (username) => { | ||
| // if (!octokit || !username) return; | ||
|
|
||
| // setLoading(true); | ||
| // setError(''); | ||
|
|
||
| // try { | ||
| // const [issuesResponse, prsResponse] = await Promise.all([ | ||
| // fetchAll('GET /search/issues', { | ||
| // q: `author:${username} is:issue`, | ||
| // sort: 'created', | ||
| // order: 'desc', | ||
| // per_page: 100, | ||
| // }), | ||
| // fetchAll('GET /search/issues', { | ||
| // q: `author:${username} is:pr`, | ||
| // sort: 'created', | ||
| // order: 'desc', | ||
| // per_page: 100, | ||
| // }), | ||
| // ]); | ||
|
|
||
| // setIssues(issuesResponse); | ||
| // setPrs(prsResponse); | ||
| // } catch (err) { | ||
| // setError(err.message); | ||
| // } finally { | ||
| // setLoading(false); | ||
| // } | ||
| // }, [octokit]); | ||
|
|
||
| // return { | ||
| // issues, | ||
| // prs, | ||
| // loading, | ||
| // error, | ||
| // fetchData, | ||
| // }; | ||
| // }; |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Consider removing commented code or updating the hook for backend integration.
The entire hook implementation has been commented out as part of the security improvement to move GitHub data fetching to the backend. While this change addresses the security concerns mentioned in the PR objectives, leaving large blocks of commented code is not a best practice for production code.
Consider one of these approaches:
- Remove the file entirely if the hook is no longer needed
- Update the hook to work with the new backend API endpoints instead of direct Octokit calls
- Add clear documentation if keeping temporarily during migration
If you choose to update the hook for backend integration, here's a suggested refactor:
-// import { useState, useCallback } from 'react';
+import { useState, useCallback } from 'react';
+import axios from 'axios';
-// export const useGitHubData = (octokit) => {
+export const useGitHubData = () => {
-// const [issues, setIssues] = useState([]);
-// const [prs, setPrs] = useState([]);
-// const [loading, setLoading] = useState(false);
-// const [error, setError] = useState('');
+ const [issues, setIssues] = useState([]);
+ const [prs, setPrs] = useState([]);
+ const [loading, setLoading] = useState(false);
+ const [error, setError] = useState('');
+ const fetchData = useCallback(async (username, token) => {
+ if (!username || !token) return;
+
+ setLoading(true);
+ setError('');
+
+ try {
+ const response = await axios.post('/api/github/get-data', {
+ username,
+ token
+ });
+
+ setIssues(response.data.issues || []);
+ setPrs(response.data.prs || []);
+ } catch (err) {
+ setError(err.response?.data?.message || err.message);
+ } finally {
+ setLoading(false);
+ }
+ }, []);
+ return {
+ issues,
+ prs,
+ loading,
+ error,
+ fetchData,
+ };
+};This approach maintains the hook pattern while leveraging the secure backend API.
📝 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.
| // import { useState, useCallback } from 'react'; | |
| return { | |
| items: response.data.items, | |
| total: response.data.total_count, | |
| }; | |
| }; | |
| // export const useGitHubData = (octokit) => { | |
| // const [issues, setIssues] = useState([]); | |
| // const [prs, setPrs] = useState([]); | |
| // const [loading, setLoading] = useState(false); | |
| // const [error, setError] = useState(''); | |
| const fetchData = useCallback( | |
| async (username: string, page = 1, perPage = 10) => { | |
| const octokit = getOctokit(); | |
| // const fetchAll = async (url, params) => { | |
| // let page = 1; | |
| // let results = []; | |
| // let hasMore = true; | |
| if (!octokit || !username || rateLimited) return; | |
| // while (hasMore) { | |
| // const response = await octokit.request(url, { ...params, page }); | |
| // results = results.concat(response.data.items); | |
| // hasMore = response.data.items.length === 100; | |
| // page++; | |
| // } | |
| setLoading(true); | |
| setError(''); | |
| // return results; | |
| // }; | |
| // const fetchData = useCallback(async (username) => { | |
| // if (!octokit || !username) return; | |
| // setLoading(true); | |
| // setError(''); | |
| // try { | |
| // const [issuesResponse, prsResponse] = await Promise.all([ | |
| // fetchAll('GET /search/issues', { | |
| // q: `author:${username} is:issue`, | |
| // sort: 'created', | |
| // order: 'desc', | |
| // per_page: 100, | |
| // }), | |
| // fetchAll('GET /search/issues', { | |
| // q: `author:${username} is:pr`, | |
| // sort: 'created', | |
| // order: 'desc', | |
| // per_page: 100, | |
| // }), | |
| // ]); | |
| // setIssues(issuesResponse); | |
| // setPrs(prsResponse); | |
| // } catch (err) { | |
| // setError(err.message); | |
| // } finally { | |
| // setLoading(false); | |
| // } | |
| // }, [octokit]); | |
| // return { | |
| // issues, | |
| // prs, | |
| // loading, | |
| // error, | |
| // fetchData, | |
| // }; | |
| // }; | |
| import { useState, useCallback } from 'react'; | |
| import axios from 'axios'; | |
| export const useGitHubData = () => { | |
| const [issues, setIssues] = useState([]); | |
| const [prs, setPrs] = useState([]); | |
| const [loading, setLoading] = useState(false); | |
| const [error, setError] = useState(''); | |
| const fetchData = useCallback(async (username, token) => { | |
| if (!username || !token) return; | |
| setLoading(true); | |
| setError(''); | |
| try { | |
| const response = await axios.post('/api/github/get-data', { | |
| username, | |
| token | |
| }); | |
| setIssues(response.data.issues || []); | |
| setPrs(response.data.prs || []); | |
| } catch (err) { | |
| setError(err.response?.data?.message || err.message); | |
| } finally { | |
| setLoading(false); | |
| } | |
| }, []); | |
| return { | |
| issues, | |
| prs, | |
| loading, | |
| error, | |
| fetchData, | |
| }; | |
| }; |
🤖 Prompt for AI Agents
In src/hooks/useGitHubData.ts from lines 3 to 64, the entire hook code is
commented out due to moving GitHub data fetching to the backend for security
reasons. To fix this, either remove the file entirely if it's no longer needed,
or refactor the hook to call the new backend API endpoints instead of using
Octokit directly. If keeping the hook temporarily during migration, add clear
documentation explaining its status. This will clean up the codebase and align
with best practices.
|
hello ! @mehul-m-prajapati i request you to please review my code and merge that, because other contributor fetching data from frontend and making changes. If it would not be done,then whole frontend need to change later looking forward for your response. |
|
no need of changing 17 files. |
2 participants
Related Issue
Description
Moved GitHub data fetching from the frontend (using hooks and Octokit) to the backend.
Updated details.js in the router to fetch data from the backend API instead of calling Octokit directly in the frontend.
This improves security and reduces API rate limit issues on the client side. Because Github personal access token is privacy aspect.
Also added "View Analytics" button to view all info of github. that I will push shortly step by step.
How Has This Been Tested?
Verified API response. proof is screen shot. if any problem occur ,I have here to resolve it quickly.
Screenshots (if applicable)
Type of Change
Summary by CodeRabbit
New Features
Bug Fixes
Chores
Refactor