Fix LTI max_score method. #1819
Conversation
| @@ -375,7 +380,7 @@ def oauth_params(self, custom_parameters, client_key, client_secret): | |||
| return params | |||
|
|
|||
| def max_score(self): | |||
| return self.weight | |||
| return self.weight if self.graded else 0 | |||
There was a problem hiding this comment.
Can be the case, when this is practice, not graded problem. So student will submit answer, and LTI Provider will return 0.3. But self.graded is False, so 0 * 0.3 will be saved. And this is wrong. I will think how to fix that.
| scope=Scope.settings, | ||
| values={"min": 0}, | ||
| ) | ||
| has_score = Boolean(help="Does this LTI module have score?", default=False, scope=Scope.settings) |
There was a problem hiding this comment.
This doesn't look like the right way to do this. has_score is a class attribute that is never modified. Is it correct to make it a field in this module that shadows the class attribute? @cpennington ?
There was a problem hiding this comment.
@nedbat This is result of discussion with @cpennington
There was a problem hiding this comment.
Note, that class attribute is removed.
There was a problem hiding this comment.
Yeah, we talked about this this morning. has_score is never accessed as a class attribute, and it covers exactly the use-case that @auraz was concerned about (distinguishing between non-scored and scored LTI modules), in a way that 'graded' doesn't.
|
@nedbat Is this good to merge? |
This PR adds fix for LTI's
max_scoremethod.When LTI module is not graded (attribute
graded=False) methodmax_scorereturns value ofweightattribute, that is1.0by default.It should not be so, because it affects on total score (each additional module adds +1 to the total max score).
@nedbat review, please.