Skip to content

Conversation

Adez017
Copy link
Member

@Adez017 Adez017 commented Sep 26, 2025

Description

Fixes #601

Type of Change

  • New feature (e.g., new page, component, or functionality)
  • Bug fix (non-breaking change that fixes an issue)
  • UI/UX improvement (design, layout, or styling updates)
  • Performance optimization (e.g., code splitting, caching)
  • Documentation update (README, contribution guidelines, etc.)
  • Other (please specify):

Changes Made

Dependencies

  • List any new dependencies or tools required for this change.
  • Mention any version updates or configurations that need to be considered.

Checklist

  • My code follows the style guidelines of this project.
  • I have tested my changes across major browsers and devices
  • My changes do not generate new console warnings or errors .
  • I ran npm run build and attached screenshot(s) in this PR.
  • This is already assigned Issue to me, not an unassigned issue.
Screen.Recording.2025-09-26.110034.mp4

@Copilot Copilot AI review requested due to automatic review settings September 26, 2025 05:34
Copy link

vercel bot commented Sep 26, 2025

@Adez017 is attempting to deploy a commit to the recode Team on Vercel.

A member of the Team first needs to authorize it.

Copy link

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 👇🏻

  1. Get free Consultation use code recode50 to get free: Mentorship for free.

  2. Get the Ebook for free use code recode at checkout: Data Science cheatsheet for Beginners.

  3. Check out this weekly Newsletter: Sanjay's Newsletter.

If there are any specific instructions or feedback regarding your PR, we'll provide them here. Thanks again for your contribution! 😊

@Adez017
Copy link
Member Author

Adez017 commented Sep 26, 2025

Check it out @sanjay-kv @iitzIrFan

Copy link
Contributor

@Copilot Copilot AI left a 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 fixes a filtering issue in the PR Modal by implementing proper time-based filtering for pull requests. The change refactors the stats provider to store all PR data and compute filtered results based on the selected time period, ensuring the modal displays the correct PRs for the chosen filter.

  • Enhanced the stats provider to store full PR history and compute filtered results dynamically
  • Updated the leaderboard to use context-based time filtering instead of simulated filtering
  • Modified the PR modal to display filtered PRs based on the current time selection

Reviewed Changes

Copilot reviewed 3 out of 3 changed files in this pull request and generated 3 comments.

File Description
src/lib/statsProvider.tsx Added time filtering logic, enhanced data storage to keep full PR history, and exposed filtering functions
src/components/dashboard/LeaderBoard/leaderboard.tsx Removed local time filtering simulation and integrated with context-based filtering
src/components/dashboard/LeaderBoard/PRListModal.tsx Updated to use filtered PRs from context instead of static contributor data

Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.

Comment on lines 99 to 108
// Time filter utility functions
const getTimeFilterDate = (filter: TimeFilter): Date | null => {
const now = new Date();
switch (filter) {
case 'week':
return new Date(now.getTime() - 7 * 24 * 60 * 60 * 1000);
case 'month':
return new Date(now.getTime() - 30 * 24 * 60 * 60 * 1000);
case 'year':
return new Date(now.getTime() - 365 * 24 * 60 * 60 * 1000);
Copy link

Copilot AI Sep 26, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The time calculation uses magic numbers for milliseconds. Consider extracting these into named constants for better maintainability.

Suggested change
// Time filter utility functions
const getTimeFilterDate = (filter: TimeFilter): Date | null => {
const now = new Date();
switch (filter) {
case 'week':
return new Date(now.getTime() - 7 * 24 * 60 * 60 * 1000);
case 'month':
return new Date(now.getTime() - 30 * 24 * 60 * 60 * 1000);
case 'year':
return new Date(now.getTime() - 365 * 24 * 60 * 60 * 1000);
// Time-related constants for better maintainability
const MILLISECONDS_PER_SECOND = 1000;
const SECONDS_PER_MINUTE = 60;
const MINUTES_PER_HOUR = 60;
const HOURS_PER_DAY = 24;
const DAYS_PER_WEEK = 7;
const DAYS_PER_MONTH = 30; // Approximate
const DAYS_PER_YEAR = 365; // Non-leap year
const MILLISECONDS_PER_DAY = HOURS_PER_DAY * MINUTES_PER_HOUR * SECONDS_PER_MINUTE * MILLISECONDS_PER_SECOND;
const MILLISECONDS_PER_WEEK = DAYS_PER_WEEK * MILLISECONDS_PER_DAY;
const MILLISECONDS_PER_MONTH = DAYS_PER_MONTH * MILLISECONDS_PER_DAY;
const MILLISECONDS_PER_YEAR = DAYS_PER_YEAR * MILLISECONDS_PER_DAY;
// Time filter utility functions
const getTimeFilterDate = (filter: TimeFilter): Date | null => {
const now = new Date();
switch (filter) {
case 'week':
return new Date(now.getTime() - MILLISECONDS_PER_WEEK);
case 'month':
return new Date(now.getTime() - MILLISECONDS_PER_MONTH);
case 'year':
return new Date(now.getTime() - MILLISECONDS_PER_YEAR);

Copilot uses AI. Check for mistakes.

<p className={`pr-modal-subtitle ${isDark ? "dark" : "light"}`}>
{contributor.prs} merged PR{contributor.prs !== 1 ? 's' : ''}{contributor.points} points
{/*Show filtered count and add filter info */}
{filteredPRs.length} merged PR{filteredPRs.length !== 1 ? 's' : ''}{filteredPRs.length * 10} points
Copy link

Copilot AI Sep 26, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The points calculation uses a magic number (10). Consider importing POINTS_PER_PR constant from statsProvider or defining it as a shared constant to maintain consistency.

Copilot uses AI. Check for mistakes.

Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
@Copilot Copilot AI review requested due to automatic review settings September 26, 2025 07:30
Copy link
Contributor

@Copilot Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull Request Overview

Copilot reviewed 3 out of 3 changed files in this pull request and generated 1 comment.


Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.

onChange={(e) => {
setTimePeriod(e.target.value as TimePeriod);
// Use setTimeFilter from context
setTimeFilter(e.target.value as any);
Copy link

Copilot AI Sep 26, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Using as any bypasses TypeScript's type safety. Since TimeFilter is already defined, cast to the specific type: setTimeFilter(e.target.value as TimeFilter) to maintain type safety.

Suggested change
setTimeFilter(e.target.value as any);
setTimeFilter(e.target.value as TimeFilter);

Copilot uses AI. Check for mistakes.

Copy link

vercel bot commented Sep 27, 2025

The latest updates on your projects. Learn more about Vercel for GitHub.

Project Deployment Preview Comments Updated (UTC)
recode-website Ready Ready Preview Comment Sep 27, 2025 2:01am

@sanjay-kv sanjay-kv moved this to In Progress in @recode-web Sep 27, 2025
@sanjay-kv sanjay-kv added this to the recode:launch 3.0 milestone Sep 27, 2025
@sanjay-kv sanjay-kv merged commit cfb379c into recodehive:main Sep 27, 2025
3 checks passed
@github-project-automation github-project-automation bot moved this from In Progress to Done in @recode-web Sep 27, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

Status: Done

Development

Successfully merging this pull request may close these issues.

Fix the leaderboard PR for filters

2 participants