Skip to content

Conversation

@polesye
Copy link
Contributor

@polesye polesye commented Dec 3, 2013

This PR adds fix for LTI's max_score method.

When LTI module is not graded (attribute graded=False) method max_score returns value of weight attribute, that is 1.0 by 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.

@ghost ghost assigned nedbat Dec 3, 2013
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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 ?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@nedbat This is result of discussion with @cpennington

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Note, that class attribute is removed.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

OK.

@auraz
Copy link
Contributor

auraz commented Dec 4, 2013

@nedbat Is this good to merge?

auraz added a commit that referenced this pull request Dec 5, 2013
@auraz auraz merged commit bceadd4 into master Dec 5, 2013
@auraz auraz deleted the anton/fix-lti-scores branch December 5, 2013 10:40
jenkins-ks pushed a commit to nttks/edx-platform that referenced this pull request Jun 23, 2017
jenkins-ks pushed a commit to nttks/edx-platform that referenced this pull request Jun 23, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants