force subsection grades to update when course grade updates - #15627
Conversation
|
jenkins run python |
2cada4f to
f4e8f21
Compare
|
jenkins run python |
iloveagent57
left a comment
iloveagent57
left a comment
There was a problem hiding this comment.
Looks good, I could go either way on the comment I left.
There was a problem hiding this comment.
nitpick: if you wanted to cut down on the lines of code, you could have a kwargs dict that holds all of the common kwargs, add the conditional method = ... line back, then do:
if force_update:
kwargs['force_update_subsections'] = True
course_grade = method(**kwargs)There was a problem hiding this comment.
I like that much better.
There was a problem hiding this comment.
Q: why does this need to be passed in? Won't we always want this to be true in this update method, per caller's expectations?
There was a problem hiding this comment.
We need to pass it in because we also call CourseGradeFactory().update when a subsection grade is updated. If we assume true for all cases, then all subsection grades will update whenever a learner submits a problem.
There was a problem hiding this comment.
I believe you should be able to undo these changes, if we agree that this boolean doesn't need to default to false.
There was a problem hiding this comment.
You can revert these changes per my comment above regarding not needing to pass force_update_subsections to the CourseGradeFactory's update method - since it's assumed from the method's contract.
There was a problem hiding this comment.
Are you planning to add any non-mock tests - to verify that any previously stored Subsection-grades do get updated? And, to capture what happens to old zero grades?
There was a problem hiding this comment.
I'd like to write a more robust test but refrained for now since this change is somewhat time-sensitive. I can add that in this PR if we're not comfortable merging without it.
7f6531f to
66fdd2f
Compare
|
EdX Release Notice: This PR has been deployed to the staging environment in preparation for a release to production on Monday, July 24, 2017. |
|
EdX Release Notice: This PR has been deployed to the production environment. |
|
EdX Release Notice: This PR has been rolled back from the production environment. |
|
I've cherry-picked this onto Ginkgo (along with #15701) |
5 participants
EDUCATOR-915
Forces subsection grades to update when iterating over course grades and forcing them to update.
Reviewers:
@nasthagiri
@iloveagent57
FYI: @edx/educator-neem