-
Notifications
You must be signed in to change notification settings - Fork 121
Added backend for leaderbaord #490
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
Conversation
@Adez017 is attempting to deploy a commit to the recode Team on Vercel. A member of the Team first needs to authorize it. |
Thank you for submitting your pull request! 🙌 We'll review it as soon as possible. The estimated time for response is 5–8 hrs. In the meantime, please provide all necessary screenshots and make sure you run - npm build run , command and provide a screenshot, a video recording, or an image of the update you made below, which helps speed up the review and assignment. If you have questions, reach out to LinkedIn. Your contributions are highly appreciated!😊 Note: I maintain the repo issue every day twice at 8:00 AM IST and 9:00 PM IST. If your PR goes stale for more than one day, you can tag and comment on this same issue by tagging @sanjay-kv. We are here to help you on this journey of open source. Consistent 20 contributions are eligible for sponsorship 💰 🎁 check our list of amazing people we sponsored so far: GitHub Sponsorship. ✨ 📚Your perks for contribution to this community 👇🏻
If there are any specific instructions or feedback regarding your PR, we'll provide them here. Thanks again for your contribution! 😊 |
Hi @sanjay-kv @iitzIrFan , i had ushed the backend code inside the dashboard folder , by now it need some small fixes regarding the file path for root consideration. didn't changed anything in the dashboard file currently . waiting for the next instructions |
@iitzIrFan can u review this im not sure it can be in the same repo @iitzIrFan |
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.
Pull Request Overview
This PR adds a complete backend implementation for a leaderboard system that tracks GitHub organization contributors based on merged pull requests. The backend fetches PR data from GitHub's API, calculates user scores, and provides REST endpoints to serve leaderboard data.
- Implements Express.js server with endpoints for leaderboard data retrieval
- Adds GitHub API integration to fetch merged PRs and calculate contributor scores
- Includes scheduled job functionality to automatically update leaderboard data
Reviewed Changes
Copilot reviewed 5 out of 6 changed files in this pull request and generated 6 comments.
Show a summary per file
File | Description |
---|---|
src/pages/dashboard/Leaderboard_backend/server.js | Main Express server with API endpoints and file initialization |
src/pages/dashboard/Leaderboard_backend/package.json | Dependencies configuration for the backend service |
src/pages/dashboard/Leaderboard_backend/jobs/update_leaderboard.js | Scheduled job to automatically update leaderboard data |
src/pages/dashboard/Leaderboard_backend/functions/org_leaderboard.js | Core logic for fetching GitHub data and generating leaderboard |
src/pages/dashboard/Leaderboard_backend/.gitignore | Git ignore configuration for node_modules |
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
|
||
app.use(express.json()); | ||
app.use(cors()); | ||
generateOrgLeaderboard(); |
Copilot
AI
Sep 10, 2025
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.
Calling generateOrgLeaderboard() synchronously at startup without error handling could cause the server to crash if the GitHub API is unavailable or returns an error during initialization.
generateOrgLeaderboard(); | |
(async () => { | |
try { | |
await generateOrgLeaderboard(); | |
} catch (err) { | |
console.error("Failed to generate org leaderboard at startup:", err); | |
} | |
})(); |
Copilot uses AI. Check for mistakes.
app.get("/Org_Leaderboard", (req, res) => { | ||
console.log("got the request"); | ||
fs.readFile("org_leaderboard.json", "utf8", function (err, data) { | ||
if (err) throw err; |
Copilot
AI
Sep 10, 2025
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.
Using throw err
in an async callback will crash the server instead of sending an error response to the client. Replace with proper error handling that sends an HTTP error response.
if (err) throw err; | |
if (err) { | |
console.error("Error reading org_leaderboard.json:", err); | |
return res.status(500).send({ success: false, message: "Failed to read leaderboard data." }); | |
} |
Copilot uses AI. Check for mistakes.
JSON.stringify(default_json), | ||
"utf8", | ||
function (err) { | ||
if (err) throw err; |
Copilot
AI
Sep 10, 2025
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.
Using throw err
in an async callback will crash the server during startup. Consider using console.error() and graceful error handling instead.
if (err) throw err; | |
if (err) { | |
console.error("Error resetting org_leaderboard.json:", err); | |
return; | |
} |
Copilot uses AI. Check for mistakes.
let leaderboard = {}; | ||
|
||
const timer = ms => new Promise(res => setTimeout(res, ms)); | ||
|
||
async function generateOrgLeaderboard() { |
Copilot
AI
Sep 10, 2025
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.
The leaderboard object is declared at module level, which means it will accumulate data across multiple function calls without being reset. This will cause duplicate entries and incorrect scores when the function runs multiple times.
let leaderboard = {}; | |
const timer = ms => new Promise(res => setTimeout(res, ms)); | |
async function generateOrgLeaderboard() { | |
const timer = ms => new Promise(res => setTimeout(res, ms)); | |
async function generateOrgLeaderboard() { | |
let leaderboard = {}; |
Copilot uses AI. Check for mistakes.
updatedTimestring: new Date().toLocaleString() | ||
}; | ||
|
||
fs.writeFileSync("org_leaderboard.json", JSON.stringify(json, null, 2)); |
Copilot
AI
Sep 10, 2025
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.
Using synchronous file operations (writeFileSync) in an async function can block the event loop. Consider using fs.promises.writeFile or fs.writeFile with a callback for better performance.
fs.writeFileSync("org_leaderboard.json", JSON.stringify(json, null, 2)); | |
await fs.promises.writeFile("org_leaderboard.json", JSON.stringify(json, null, 2)); |
Copilot uses AI. Check for mistakes.
|
||
if (!leaderboard[user.id].pr_urls.includes(pr.html_url)) { | ||
leaderboard[user.id].pr_urls.push(pr.html_url); | ||
leaderboard[user.id].score += 10; // scoring rule (static here) |
Copilot
AI
Sep 10, 2025
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.
[nitpick] The score value (10) is a magic number. Consider defining this as a named constant at the top of the file for better maintainability.
Copilot uses AI. Check for mistakes.
@Adez017 is this even working locally if, yes I would love to see the video preview of this cuz on my side it gives errors, I tried multiple times but, failed to build your code ! ![]() |
Thanks for the review @iitzIrFan |
Hi @iitzIrFan , actually i tried creating a separate repo for this and then pushed the changes in the PR , on my local machine its workign fine . below is the video you can refer : |
I was thinking as it is implemented here, https://www.recodehive.com/dashboard#contributors I wanted you to just clone it or you can convert your repo into an API [express] for Recode from GSSoC. Then just make the frontend and fetch the required data from your new api ! |
Nice idea , how about hosting the backend separate and just use the API to fectch the data . @iitzIrFan |
Yeah that's what I was thinking about, let's wait for @sanjay-kv to put his thoughts on this ! |
By the i will be needing help with frontend , i am not that good 😁 |
@Adez017 No issue, you can propose your design @sanjay-kv will conform it 😄 |
how about this kind of @iitzIrFan @sanjay-kv |
It looks good and I am guessing that, this is for the dark theme :) |
its good <3 @Adez017 |
Okay , so let me create a complete working one . @sanjay-kv @iitzIrFan |
Happy for this pr @Adez017 |
can i merge this? @iitzIrFan |
Yes @iitzIrFan , also i am thinking of directly integrate the data fetching logic inside the same file . let me give it a try and will look is it working or not |
let me know when to host. |
how about this one @sanjay-kv @iitzIrFan |
Also need your help here @iitzIrFan , the sidebar is not visible and also i had imported the icons but they are not alinged . please if possible help out . I need assistance , i tried but its failing too many times |
@Adez017 lets keep the same design as gssoc, i would suggest |
Can i get a frontend code for the reference @sanjay-kv |
How about this one @sanjay-kv ![]() |
Hi @sanjay-kv @iitzIrFan , the dashboard is working now , but here is the things need to be fixed :
And to be honest i need help to fix this things , i had tried everything possible at my end . |
Here is the Update , i had restored the functionality :here is the video update . |
This looks good, you can remove me, allcontributor from the PR list i think can hide it @Adez017 |
Nw , your development is good, you will get the help from @iitzIrFan on the following. |
@Adez017 appreciate the time you spent on this. its great. <3 real opensource developer |
Done @sanjay-kv |
Will review and update ! @Adez017 @sanjay-kv |
@Adez017 can you run |
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.
@Adez017 For now just run npm run build
and if, errors then, try solving them or tag, happy to help :)
Sure ! @iitzIrFan , just to confirm you are trying for this : https://github.com/Adez017/recode-website/tree/dahsboard-leaderboard |
Hi @iitzIrFan , i had run the |
I appreciate your effort on this, maybe this will be the last bug |
Same error was seen by when, I reviewed last night @sanjay-kv , waiting for @Adez017 to fix this problem creatively. |
Hi @sanjay-kv , did you take n this from my fork link that i mentioned ? |
Hi @sanjay-kv , @iitzIrFan , i think there is any confusion. after seeing the logs i think you had tried this PR but actually i had made the changes in a new branch in a my fork with branch as leaderboard-dashboard. |
let me open the fresh PR with the changes . @sanjay-kv @iitzIrFan |
so this PR we are not using if im right? |
Yeah you can close this one as he had opened fresh one for it. |
Description
Provide a brief summary of the changes made to the website and the motivation behind them. Include any relevant issues or tickets.
This helps fast tracking your PR and merge it, Check the respective box below.
Fixes #158
Type of Change
Changes Made
Dependencies
Checklist
npm run build
and attached scrrenshot in this PR.