-
Notifications
You must be signed in to change notification settings - Fork 290
feat: add subsection visibility option to hide scores but include in final grade #1797
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
base: master
Are you sure you want to change the base?
feat: add subsection visibility option to hide scores but include in final grade #1797
Conversation
Thanks for the pull request, @Anas12091101! This repository is currently maintained by Once you've gone through the following steps feel free to tag them in a comment and let them know that your changes are ready for engineering review. 🔘 Get product approvalIf you haven't already, check this list to see if your contribution needs to go through the product review process.
🔘 Provide contextTo help your reviewers and other members of the community understand the purpose and larger context of your changes, feel free to add as much of the following information to the PR description as you can:
🔘 Get a green buildIf one or more checks are failing, continue working on your changes until this is no longer the case and your build turns green. Where can I find more information?If you'd like to get more details on all aspects of the review process for open source pull requests (OSPRs), check out the following resources: When can I expect my changes to be merged?Our goal is to get community contributions seen and reviewed as efficiently as possible. However, the amount of time that it takes to review and merge a PR can vary significantly based on factors such as:
💡 As a result it may take up to several weeks or months to complete a review and merge your PR. |
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## master #1797 +/- ##
==========================================
+ Coverage 90.83% 90.92% +0.08%
==========================================
Files 344 344
Lines 5794 5849 +55
Branches 1374 1392 +18
==========================================
+ Hits 5263 5318 +55
- Misses 512 514 +2
+ Partials 19 17 -2 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
819f1af
to
d99c306
Compare
@Anas12091101 Thank you for this contribution! Since it includes user-facing changes, you'll have to get approval on the product proposal before engineering review can start. I'll tag the team of product managers for you here. @openedx/openedx-product-managers Could you please have a look at this PR and the corresponding product proposal, and guide @Anas12091101 on next steps? This work might qualify as a fast-track change but I'm not completely sure. So if you could check what the right product review track would be as a first step, that would be great. -- CC @mphilbrick211 for openedx/edx-platform#37399 and openedx/frontend-app-authoring#2489 |
@itsjeyd the propsosal is linked in the PR description (https://openedx.atlassian.net/wiki/spaces/OEPM/pages/5150769174/Proposal+Add+option+to+show+overall+assessment+results+after+due+date+but+hide+individual+assessment+results) and its been approved. |
Found it. @itsjeyd openedx/platform-roadmap#460 |
@Anas12091101 can you add some screenshots for the lower part of the progress page? We'd like to see how a hidden grade is handled there versus a non-hidden one. Along with that, it would be helpful to have more explanation of how the timing is handled differently between the old "never show assessment results" and this new "... but show overall assessment results after due date" |
b37a834
to
c183e89
Compare
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.
Overall looks good—just a minor enhancement and a few message updates.
courseGradeFooterDueDateNotice: { | ||
id: 'progress.courseGrade.footer.dueDateNotice', | ||
defaultMessage: 'Some assignment scores are not yet included in your total grade. These grades will be released by {dueDate}.', | ||
description: 'This shown when there are pending assignments with due date in the future', |
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.
description: 'This shown when there are pending assignments with due date in the future', | |
description: 'This is shown when there are pending assignments with a due date in the future', |
hiddenScoreLockInfoText: { | ||
id: 'progress.hiddenScoreLockInfoText', | ||
defaultMessage: 'Shown when scores for an assignment type are hidden yet still counted toward the course grade.', | ||
description: 'Information text about hidden score label when learner have limited access to grades feature', |
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.
description: 'Information text about hidden score label when learner have limited access to grades feature', | |
description: 'Information text about hidden score label when learners have limited access to grades feature', |
}, | ||
hiddenScoreLockInfoText: { | ||
id: 'progress.hiddenScoreLockInfoText', | ||
defaultMessage: 'Shown when scores for an assignment type are hidden yet still counted toward the course grade.', |
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.
defaultMessage: 'Shown when scores for an assignment type are hidden yet still counted toward the course grade.', | |
defaultMessage: 'Scores for an assignment type are hidden but still counted toward the course grade.', |
}, []); | ||
|
||
// Returns True if this subsection is "hidden" | ||
const isSubsectionHidden = (sub) => sub.showGrades && sub.showCorrectness === 'never_but_include_grade'; |
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.
ShowGrades is false when due date is not passed. It results in 0% instead of a lock icon.
@pdpinch, In the In my test course, there are two sections, each with five subsections, configured with different visibility settings:
The learner will only see the visible subsection scores in the table, as shown in the screenshot below. ![]()
In the "Never show assessment results" option, the due date is not considered because those grades are excluded from the final results. In contrast, with the new option, the scores remain hidden from learners but are included in the final grades once the due date has passed. |
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 code itself seems generally fine, but as noted in the inline comment, I'm trying to understand whether the premise is being addressed properly.
// Scores for visible grades (exclude never_but_include_grade) | ||
const visibleScores = points.filter( | ||
(_, idx) => visibilities[idx] !== 'never_but_include_grade', | ||
); | ||
|
||
// Average all scores (for totalWeightedGrade) | ||
const overallAverage = parseFloat( | ||
(points.reduce((a, b) => a + b, 0) / points.length).toFixed(4), | ||
); | ||
totalWeightedGrade = overallAverage * assignmentWeight; | ||
if (visibleScores.length) { | ||
const visibleAverage = parseFloat( | ||
(visibleScores.reduce((a, b) => a + b, 0) / points.length).toFixed(4), | ||
); | ||
averageGrade = visibleAverage; | ||
weightedGrade = averageGrade * assignmentWeight; | ||
} |
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.
First: I'm always leery of manipulating critical data (such as a learner's grade) in the frontend. This here seems like a pretty heavy lift. And while I'm not an expert on how grading works in the backend, I expect this is duplicating backend logic. Yes, I realize that this kind of calculation is already being done in this very file - which is unfortunate - but this is not all.
If I understand correctly from the proposal, the point of hiding scores is to prevent answer sharing. However - correct me if I'm wrong - I deduce from this that the individual scores are actually being sent to the client, so that a crafty user could in fact find out the correspondence between all answers and scores.
If the whole premise is to prevent cheating, shouldn't we avoid passing all the individual grades to the frontend in the first place?
CC @pdpinch
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.
Oh, yes, you are correct @arbrandes. We should be hiding these grades in the API.
Supporting tickets
Proposal: https://openedx.atlassian.net/wiki/spaces/OEPM/pages/5150769174/Proposal+Add+option+to+show+overall+assessment+results+after+due+date+but+hide+individual+assessment+results
openedx/platform-roadmap#460
Internal ticket: https://github.com/mitodl/hq/issues/3859
Description
This PR introduces a new visibility option for assignment scores:
“Never show individual assessment results, but show overall assessment results after the due date.”
With this option, learners cannot see question-level correctness or scores at any time. However, once the due date has passed, they can view their overall score in the total grades section on the Progress page.
Additionally, for visibility settings that depend on the due date, this PR adds a banner on the Progress page indicating that grades are not yet released, along with the relevant due date information.
edx-platfom and authoring MFE changes are required to test this PR.
edx-platform PR: openedx/edx-platform#37399
authoring MFE PR: openedx/frontend-app-authoring#2489
Screenshots
Testing Instructions
Use this course tarball for testing: course._4j9qu3f.tar.gz
Verify that with the new visibility option:
Confirm that subsections with the new visibility option are not listed in the Detailed Grades table.
In the Grade Summary table, check two cases: