Vikas taking over for Vamshi - Update criteria for Bios ready to post to be based on Total Valid Weekly Summaries#3529
Conversation
✅ Deploy Preview for highestgoodnetwork-dev ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
9054146 to
ebade00
Compare
|
- 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
|
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. |
debadyuti23
left a comment
There was a problem hiding this comment.
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
…d of daysInTeam > 60
|
|
@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. |
ShreyaMP1999
left a comment
There was a problem hiding this comment.
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.
Shravan-neelamsetty
left a comment
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
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.
Ganesh112001
left a comment
There was a problem hiding this comment.
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!
There was a problem hiding this comment.
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.
|
Hi @kanishkagarwal6101 |
saisandeepkoritala
left a comment
There was a problem hiding this comment.
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.
|
Tested locally on /weeklysummariesreport against the dev backend.
|
rajanidi1999
left a comment
There was a problem hiding this comment.
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.
rithika-paii
left a comment
There was a problem hiding this comment.
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.
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.
Based on these observations, the filter logic and dark mode styling need further fixes before this PR can be approved.



























Description
🛠️ What This PR Does
This update modifies the filter logic for the Bio Status in the Weekly Summaries Report under:
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
🤔 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:
will meet the Bio Status criteria.
🔗 Related PRs
Not related to any other PR.
🧪 How to Test
npm installand start the server locallyDashboard → Reports → Weekly Summaries Report📸 Screenshots / Videos
Note:
Please refresh your cache before testing. Let us know if any edge cases are missed.