[Bugfix:InstructorUI] Fixing histogram filters#12425
Conversation
…, not changing anything there
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## main #12425 +/- ##
============================================
- Coverage 21.73% 21.72% -0.02%
- Complexity 9623 9628 +5
============================================
Files 268 268
Lines 36167 36194 +27
Branches 486 486
============================================
Hits 7862 7862
- Misses 27823 27850 +27
Partials 482 482
Flags with carried forward coverage won't be shown. Click here to find out more. 🚀 New features to boost your workflow:
|
John-Roy123
left a comment
There was a problem hiding this comment.
I was able to replicate the issue on main - the histogram would not update if I changed any of the filters. With this change, it now works correctly and updates according to the filters the user selects.
Rkoester47
left a comment
There was a problem hiding this comment.
I tested the changes from this branch on the gradeable "Stats of Autograde and TA Homework (C System Calls)" and found what I believe to be an issue. When NULL is not included, I the autograding histrogram counts 34 students with a score of 2/10, and 15 students with a score of 10/10.
In the NULL section on the gradeable page, I have 15 null students with 2/10 and 11 null students with 10/10. This should bring the autograding histrogram count up to 49 and 26 respectively. However, I only have 48 and 25 on the autograding histrogram when I click to include the null section.
Can you confirm on your end to see if you are experiencing the same off by one count?
While my counts were a bit different you are correct. This was due to the logic for detecting bad submissions being incorrect. I have pushed a fix for this. When testing, be aware that some bad submissions also have version conflict and as such they will not be included in the histogram (3 in the C System Calls gradeable). |
Rkoester47
left a comment
There was a problem hiding this comment.
After your most recent changes, the count for null students being included/excluded from the histogram looks correct to me.
### Why is this Change Important & Necessary? Closes Submitty#12211 This fixes the issue where changing the null section and withdrawn filters will not update the gradeable histograms. ### What is the New Behavior? Before, the null section and withdrawn filters would not change anything when enabled/disabled. Now, they should affect the histograms and will include the correct information when enabled. ### What steps should a reviewer take to reproduce or test the bug or new feature? 1. Log in as instructor. 2. Go to any gradeable with grading completed/partially completed. 3. Click Statistics & Charts 5. Verify that the histograms (manual, autograding, and overall) update accordingly with the filters. ### Automated Testing & Documentation Manual testing: Tested on Linux with Opera to verify the accuracy of the new histogram data generation. No new automated testing has been added, issue will be created to add more testing for the histograms. ### Other information This is not a breaking change. There are no known security concerns with this change. Modified files include: site/app/controllers/grading/ElectronicGraderController.php Note: Many of the whitespace changes in the file were to fix issues the php linter was complaining about. --------- Co-authored-by: Rkoester47 <koestr@rpi.edu>
Why is this Change Important & Necessary?
Closes #12211
This fixes the issue where changing the null section and withdrawn filters will not update the gradeable histograms.
What is the New Behavior?
Before, the null section and withdrawn filters would not change anything when enabled/disabled. Now, they should affect the histograms and will include the correct information when enabled.
What steps should a reviewer take to reproduce or test the bug or new feature?
Automated Testing & Documentation
Manual testing: Tested on Linux with Opera to verify the accuracy of the new histogram data generation.
No new automated testing has been added, issue will be created to add more testing for the histograms.
Other information
This is not a breaking change.
There are no known security concerns with this change.
Modified files include:
site/app/controllers/grading/ElectronicGraderController.php
Note: Many of the whitespace changes in the file were to fix issues the php linter was complaining about.