Skip to content

Vikas taking over for Vamshi - Update criteria for Bios ready to post to be based on Total Valid Weekly Summaries#3529

Open
guthaVamshi wants to merge 6 commits intodevelopmentfrom
Vamshi-WeeklySummariesFilter
Open

Vikas taking over for Vamshi - Update criteria for Bios ready to post to be based on Total Valid Weekly Summaries#3529
guthaVamshi wants to merge 6 commits intodevelopmentfrom
Vamshi-WeeklySummariesFilter

Conversation

@guthaVamshi
Copy link
Copy Markdown
Contributor

Description

image

🛠️ What This PR Does

This update modifies the filter logic for the Bio Status in the Weekly Summaries Report under:

Owner Login → Reports → Weekly Summaries Report

Previously, qualification was based on the number of weeks a person had been on the team. This has now been corrected to reflect actual participation and contribution.


✅ Changes Made

  • Updated the Bio Status filter criteria to:
    • Require 80 or more total volunteer hours (already implemented)
    • Require 8 or more valid Weekly Summaries submitted (instead of 8+ weeks on the team)

🤔 Why This Matters

This ensures Bio Status is granted only to individuals who have consistently met expectations, regardless of how long they’ve been part of the project. It fairly excludes time off, missed weeks, or inactive periods and makes the system performance-based.


🎯 Outcome

Only individuals who have:

  • Logged 80+ total volunteer hours, AND
  • Submitted 8+ valid Weekly Summaries

will meet the Bio Status criteria.


🔗 Related PRs

Not related to any other PR.


🧪 How to Test

  1. Check out the current branch
  2. Run npm install and start the server locally
  3. Clear site data/cache
  4. Log in as an Owner user
  5. Navigate to: Dashboard → Reports → Weekly Summaries Report
  6. Click on Filter by Bio Status
  7. Verify that results include only users who submitted 8+ valid Weekly Summaries

📸 Screenshots / Videos

image

Note:

Please refresh your cache before testing. Let us know if any edge cases are missed.

@netlify
Copy link
Copy Markdown

netlify Bot commented May 15, 2025

Deploy Preview for highestgoodnetwork-dev ready!

Name Link
🔨 Latest commit b4853f3
🔍 Latest deploy log https://app.netlify.com/projects/highestgoodnetwork-dev/deploys/6941ab088e83810008664b58
😎 Deploy Preview https://deploy-preview-3529--highestgoodnetwork-dev.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify project configuration.

@guthaVamshi guthaVamshi requested a review from mvreddy13 May 15, 2025 18:22
Copy link
Copy Markdown

@nikhilpittala16 nikhilpittala16 left a comment

Choose a reason for hiding this comment

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

I have tested the PR and everything works as expected.

PR.vid.34.mp4

@guthaVamshi guthaVamshi added the High Priority - Please Review First This is an important PR we'd like to get merged as soon as possible label May 16, 2025
@CasstielP
Copy link
Copy Markdown
Contributor

Tested PR3529, setup was successfully, was able to render the targeted components, UI seemed well proportioned. Bio status toggle buttoned work properly that only displayed with users who submitted 8+ valid weekly summaries.
image

@guthaVamshi
Copy link
Copy Markdown
Contributor Author

Tested PR3529, setup was successfully, was able to render the targeted components, UI seemed well proportioned. Bio status toggle buttoned work properly that only displayed with users who submitted 8+ valid weekly summaries. image

Hello CasstieIP,

Thank you for reviewing the pull request. Since you seem to be satisfied with the testing, could you please update the status to "Approved" at your convenience?

Appreciate your time and support.

Ram-blip
Ram-blip previously approved these changes May 27, 2025
Copy link
Copy Markdown
Contributor

@Ram-blip Ram-blip left a comment

Choose a reason for hiding this comment

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

Tested the PR and everything looks good! The Bio Status filter now correctly checks for 80+ hours and 8+ valid summaries in both light and dark mode. Great job and Approving the PR.

PR 3529

@1709abhishek
Copy link
Copy Markdown

Screenshot 2025-06-07 at 12 23 27 PM It works fine. tested on my system. one thing i want to mention is, the UI for dark mode, for example the toggle buttons and very non user intuitive.

@1709abhishek 1709abhishek self-requested a review June 7, 2025 16:24
1709abhishek
1709abhishek previously approved these changes Jun 7, 2025
@guthaVamshi guthaVamshi dismissed stale reviews from 1709abhishek and Ram-blip via 9054146 July 9, 2025 18:12
@guthaVamshi guthaVamshi closed this Jul 9, 2025
@guthaVamshi guthaVamshi force-pushed the Vamshi-WeeklySummariesFilter branch from 9054146 to ebade00 Compare July 9, 2025 18:30
@guthaVamshi guthaVamshi reopened this Jul 9, 2025
@one-community one-community added Needs New Developer This is a PR that is partially developed but needs someone new to take it over and finish it. do not review Do not review or look at code without full context and removed High Priority - Please Review First This is an important PR we'd like to get merged as soon as possible labels Aug 17, 2025
@sonarqubecloud
Copy link
Copy Markdown

Quality Gate Failed Quality Gate failed

Failed conditions
E Maintainability Rating on New Code (required ≥ A)

See analysis details on SonarQube Cloud

Catch issues before they fail your Quality Gate with our IDE extension SonarQube for IDE

@one-community one-community changed the title Vamshi - Update criteria for Bios ready to post to be based on Total Valid Weekly Summaries Kanishk taking over for Vamshi - Update criteria for Bios ready to post to be based on Total Valid Weekly Summaries Dec 4, 2025
@one-community one-community added High Priority - Please Review First This is an important PR we'd like to get merged as soon as possible and removed Needs New Developer This is a PR that is partially developed but needs someone new to take it over and finish it. do not review Do not review or look at code without full context labels Dec 4, 2025
Copy link
Copy Markdown

@Anusha-Gali Anusha-Gali left a comment

Choose a reason for hiding this comment

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

Hi Kanishk,

I have tried to review your PR locally but was unable to do so due to node version difference. The application should officially support Node.js v20.x but your package.json has the node version "node": ">=14.0.0 <15".
Screenshot 2025-12-03 at 8 07 57 PM

@SwathiAngadi
Copy link
Copy Markdown
Contributor

SwathiAngadi commented Dec 9, 2025

Hi Kanishk,

I checked into the above mentioned branch to review this PR, even I am facing the Node version issue, this branch requires version {"node":">=14.0.0 <15"} but app is running on V20

Screenshot 2025-12-09 001041

- Remove commented code from App.module.css
- Fix overflow shorthand issue (use overflow-y instead of overflow)
- Add generic font family fallback for FontAwesome
- Remove unused styles import from App.jsx
- Merge duplicate .issuesTableResponsive selector in IssueDashboard.module.css
- Remove unused daysInTeam variable from BioFunction.jsx
- Add PropTypes validation for summary.weeklySummariesCount in BioFunction and FormattedReport
@kanishkagarwal6101
Copy link
Copy Markdown
Contributor

Hi all, Thank you for the feedback, I have made the changes requested and fixed the conflicts and code quality issues, I request you to please test the branch again and let me know if it is satisfying the requirements, your feedback is greatly appreciated.

Copy link
Copy Markdown

@debadyuti23 debadyuti23 left a comment

Choose a reason for hiding this comment

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

Hi @kanishkagarwal6101 I have checked the changes in localhost. I can see some entries with weekly valid summaries with 0 as well. I have shared the video of my findings in the comment. Please let me know if this is expected or I might have missed anything.

PR_3529.mp4

@sonarqubecloud
Copy link
Copy Markdown

@kanishkagarwal6101
Copy link
Copy Markdown
Contributor

@debadyuti23 I request you to please review the pr again, there was a minor caveat that i changed and committed. Hopefully all good now. Your feedback is greatly appreciated.

Copy link
Copy Markdown

@Anusha-Gali Anusha-Gali left a comment

Choose a reason for hiding this comment

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

Hi Kanishk,

I have re-reviewed your PR locally and even though there are summaries whose count is >8 (have shown in the screenshots below), they do not get filtered when i select the "Filter by Bio Status" toggle and it only shows one data.
Screenshot 2025-12-16 at 10 20 17 PM
Screenshot 2025-12-16 at 10 20 23 PM
Screenshot 2025-12-16 at 10 20 38 PM
Screenshot 2025-12-16 at 10 20 57 PM
Screenshot 2025-12-16 at 10 21 15 PM

@abhinav-TB abhinav-TB self-assigned this Jan 3, 2026
Copy link
Copy Markdown
Contributor

@ShreyaMP1999 ShreyaMP1999 left a comment

Choose a reason for hiding this comment

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

Tested this PR locally as an Owner user. After enabling Filter by Bio Status, results do not match the expected behavior (users with 8+ valid Weekly Summaries). The filtered list still includes users with “Bio announcement: Not requested/posted” and “Weekly Summary: Not provided!”. Also seeing an API error toast (404 on filter list request), which may be affecting the filter logic. Screenshots attached.

Screenshot 2026-01-03 at 3 55 59 PM Screenshot 2026-01-03 at 3 56 29 PM

@abhinav-TB abhinav-TB removed their assignment Jan 3, 2026
Copy link
Copy Markdown
Contributor

@Shravan-neelamsetty Shravan-neelamsetty left a comment

Choose a reason for hiding this comment

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

Hi Kanishk, I tested this PR locally by logging in as Owner and navigating to Dashboard → Reports → Weekly Summaries Report. When I enabled "Filter by Bio Status," the results show only 1 user (Total Team Members: 1), despite there being multiple users in the system who should meet the criteria of 8+ valid weekly summaries. The single user shown has 48 valid summaries which exceeds the requirement, but the filter appears to be excluding other users who should also qualify. This suggests the filter logic is either too restrictive or not working correctly.

Screenshot 2026-01-09 at 6 38 39 PM

Copy link
Copy Markdown

@Vikas-8055 Vikas-8055 left a comment

Choose a reason for hiding this comment

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

Hi Kanishk, I tested this PR locally by logging in as Owner and navigating to Dashboard → Reports → Weekly Summaries Report. When I enabled "Filter by Bio Status," the results show only 1 user (Total Team Members: 1), despite there being multiple users in the system who should meet the criteria of 8+ valid weekly summaries AND 80+ volunteer hours. The single user shown has 87 valid summaries which exceeds the requirement, but the filter appears to be excluding other users who should also qualify. This suggests the filter logic is either too restrictive or not working correctly.

Screenshot 2026-01-10 at 1 26 25 PM

@abhinav-TB
Copy link
Copy Markdown
Member

Hi Kanishk, I have tested this PR locally. The filter does not display all having valid summaries > 8. It just displays one person
image
image

Copy link
Copy Markdown

@Ganesh112001 Ganesh112001 left a comment

Choose a reason for hiding this comment

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

Hi Kanishk, I tested PR #3529 locally and verified that the Bio Status filter criteria update works correctly. After checking out the Vamshi-WeeklySummariesFilter branch and navigating to the Weekly Summaries Report at Dashboard → Reports → Weekly Summaries Report, I applied the "Filter by Bio Status" and confirmed that it now correctly filters users based on the updated criteria. The filter displays only users who have both 80 or more total volunteer hours AND 8 or more valid Weekly Summaries submitted, replacing the previous incorrect criteria that was based on weeks on the team. I verified that users meeting both conditions appear in the filtered results, while users with only one condition met (such as 80+ hours but fewer than 8 summaries, or 8+ summaries but fewer than 80 hours) are correctly excluded. The filter applies quickly in real-time, the clear filter functionality works properly, and no console errors were observed. This change ensures Bio Status is granted fairly based on actual participation and contribution rather than just time on the team, making the system performance-based as intended. Everything functions correctly and is ready for merge!

Screenshot 2026-01-16 at 2 45 57 AM

Copy link
Copy Markdown

@rohanrastogi311 rohanrastogi311 left a comment

Choose a reason for hiding this comment

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

Hi Kanishk,

Well done with this implementation.

PR 3529 Screenshot

Copy link
Copy Markdown

@Vikas-8055 Vikas-8055 left a comment

Choose a reason for hiding this comment

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

I tested this PR locally by checking out the Vamshi-WeeklySummariesFilter branch, running npm install, and clearing site data before testing, with screenshots attached for reference. Logged in as an Owner and navigated to Dashboard → Reports → Weekly Summaries Report, enabled the “Filter by Bio Status,” and verified that the filter correctly returns only users who meet both criteria: 80+ total volunteer hours and 8+ valid Weekly Summaries submitted. Users who logged hours but missed weekly summaries are correctly excluded, confirming that the logic now evaluates actual participation rather than time on the team. The feature works as expected in both light and dark modes, with no console errors observed. Approved and ready to merge.

Image

@maithili20
Copy link
Copy Markdown
Contributor

Hi @kanishkagarwal6101
I reviewed your PR and it works well in dark and light mode. But the filter does not seem to work correctly. I have multiple entries which have total valid weekly summaries>8 but they are not shown when filter is turned. I can only see 1 record. Please find the screenshots below
Screenshot 2026-02-10 at 3 39 08 PM
Screenshot 2026-02-10 at 3 39 25 PM

Copy link
Copy Markdown

@naznin07 naznin07 left a comment

Choose a reason for hiding this comment

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

Unit tests for BioFunction criteria and dropdown menu logic.
PR #3529

Copy link
Copy Markdown
Contributor

@saisandeepkoritala saisandeepkoritala left a comment

Choose a reason for hiding this comment

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

Hi Kaniskh,

I reviewed your PR and it works well in dark and light mode. But the filter seems to have issue. I have more than one entries which have total valid weekly summaries greater than 8 but they are not shown when filter is turned. I can only see 1 record. Please find the screenshots for refrence.

Image Image

@SharadhaKasiviswanathan
Copy link
Copy Markdown
Contributor

Tested locally on /weeklysummariesreport against the dev backend.
The new weeklySummariesCount >= 8 rule is taking effect, and the report page loaded correctly in my local test. I possibly think of two follow-ups that can be made before merge:

  1. The permission description for highlightEligibleBios is still stale. It still says the bio highlight is based on “80 tangible hours, 60 days on the team...”, but the implemented rule is now based on weeklySummariesCount >= 8. Trying to update the permission text so it matches the actual behavior might be helpful.
  2. The tests were not fully updated for this rule change. FormattedReport.test.jsx still uses weeklySummariesCount as a string and only raises a prop-type warning, and BioFunction.test.jsx still passes userId as a number and does not cover the new weeklySummariesCount >= 8 eligibility path. Updating/adding regression tests for the new bio-highlight/filter can also be done.

@SharadhaKasiviswanathan
Copy link
Copy Markdown
Contributor

I found another issue while testing the Weekly Summaries Report locally:
When switching to the “Last Week” tab and older tabs, the “Filter by Bio Status” toggle behaves opposite to what the UI suggests: with the toggle off, the list is already filtered, and when the toggle is turned on, it stops behaving like a bio-only filter. This looks like a tab specific state or filter sync bug. I have attached the screenshots below.
Please try checking the bio filter behavior across all week tabs and adding a regression test for:

  1. toggle off => no bio-status filtering
  2. toggle on => only bio-eligible users are shown
  3. same behavior remains correct after switching from “This Week” to “Last Week” and older tabs
Screenshot 2026-03-29 at 1 22 24 AM Screenshot 2026-03-29 at 1 22 56 AM

@one-community one-community changed the title Kanishk taking over for Vamshi - Update criteria for Bios ready to post to be based on Total Valid Weekly Summaries Vikas taking over for Vamshi - Update criteria for Bios ready to post to be based on Total Valid Weekly Summaries Apr 5, 2026
Copy link
Copy Markdown

@rajanidi1999 rajanidi1999 left a comment

Choose a reason for hiding this comment

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

This is a good step towards migrating to CSS modules, but there are a few concerns to address before merging. Converting global styles to modules (e.g., App.css) may unintentionally break existing styling across the app, especially with mixed usage of :global. Also noticed some duplicate className props and mixed usage of old and module-based class names, which could lead to bugs or styling inconsistencies. Recommend cleaning these up and verifying UI impact before final approval.

Copy link
Copy Markdown

@rajanidi1999 rajanidi1999 left a comment

Choose a reason for hiding this comment

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

Hi,
I have tested your PR and its implemented successfully.

Image

Copy link
Copy Markdown

@rithika-paii rithika-paii left a comment

Choose a reason for hiding this comment

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

Hi Vikas,

I reviewed PR #3529 and tested the Weekly Summaries Report filter locally in both light and dark modes. The overall functionality is clear, but I found a couple of issues:

Bio Status filter issue: When applying the filter, only one user is displayed, even though multiple users appear to meet the expected criteria (8+ valid weekly summaries). This suggests the filtering logic may not be correctly including all qualifying users.

Image

Dark mode issue: In dark mode, the text on the page is not visible, indicating that the styling is not properly adapted for dark mode. This affects usability and readability.

Image Image

Based on these observations, the filter logic and dark mode styling need further fixes before this PR can be approved.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

High Priority - Please Review First This is an important PR we'd like to get merged as soon as possible

Projects

None yet

Development

Successfully merging this pull request may close these issues.