-
Notifications
You must be signed in to change notification settings - Fork 313
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
Allow assessment bonus points through manual grading over 100% #9714
base: master
Are you sure you want to change the base?
Conversation
All images
|
How does this interact with #8890 and/or the various question point/percentage changes that are coming? |
Note that an assessment's score is still based on the points, which is based on the sum of question points. Since question points are the "weight" within the assessment, there is a direct correlation between the points and the bonus points. I don't think this interacts directly with #8890 or with rubric percentages, which refers to the score within the question. |
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.
I'm fine with this change, although I think that it might be even nicer to fix #9714 by only applying max_bonus_points
to auto points, and always allowing manual points to exceed 100% of the allocated manual points. With the change in this PR, an instructor will have to anticipate that they will be awarding bonus manual points and explicitly allow that (with a precise upper limit) in the assessment, which feels error prone to me.
All that being said, the current PR is simple and clearly an improvement over the current state, so we should proceed with it.
@@ -33,7 +33,7 @@ BEGIN | |||
SELECT | |||
c.id, g.id, u.user_id, a.id, a.type, | |||
a.max_points, ai.max_points, | |||
COALESCE(a.max_bonus_points, 0), ai.max_bonus_points, | |||
GREATEST(a.max_bonus_points, 0), ai.max_bonus_points, |
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.
Is this change simply to guard against the fact that a user might set assessment.max_bonus_points
to a negative value?
Resolves #9713.