Fix for SOL-475 (double exam grader bug) #7820
Conversation
mattdrayer
force-pushed
the
mattdrayer/SOL-475
branch
from
90f138b
to
78cd432
Compare
| @@ -270,6 +271,14 @@ def _delete_entrance_exam(request, course_key): | |||
| } | |||
| CourseMetadata.update_from_dict(metadata, course, request.user) | |||
|
|
|||
| # Remove any/all Entrance Exam graders from the course | |||
| grading_model = CourseGradingModel.fetch(course_key) | |||
There was a problem hiding this comment.
Seems like this same operation is repeated within the create_xblock helper. Might be better to make it a helper in its own right, and call it from here, and within the other helper.
There was a problem hiding this comment.
Yep, good call -- I've generalized this logic into views/helpers.py
mattdrayer
force-pushed
the
mattdrayer/SOL-475
branch
from
78cd432
to
9c4930c
Compare
| graders = CourseGradingModel.fetch(self.course_key).graders | ||
| count = 0 | ||
| for grader in graders: | ||
| if grader['type'] == 'Entrance Exam': |
There was a problem hiding this comment.
GRADER_TYPES['ENTRANCE_EXAM'] ? - Yes, it's only a test, so maybe that's just fine
There was a problem hiding this comment.
ARG -- yes, you are right, it should be changed to use the dict as well (and there was another spot in this file, too) -- good eye, Mr. James, good eye :)
mattdrayer
force-pushed
the
mattdrayer/SOL-475
branch
from
9c4930c
to
5e78779
Compare
|
@chrisndodge -- once @martynjames has given the thumbs-up on this PR it would be great if you could take a quick spin thru for the 2nd review -- it's small enough that we do not need to distract another team. |
| @@ -25,6 +25,14 @@ | |||
|
|
|||
| __all__ = ['edge', 'event', 'landing'] | |||
|
|
|||
| GRADER_TYPES = { | |||
There was a problem hiding this comment.
Maybe a comment - this is not a complete list of grader types because they are user-changeable, but rather a list of common types... If I understand it correctly...
There was a problem hiding this comment.
I added a comment acknowledging the good-intentioned futility of the attempt ;)
|
Code looks good to me 👍. I haven't run a full test pass, because the bug record doesn't quite give reproduction steps - but it does seem like this code will avoid a likely problem because it seems to ensure that only one entrance exam record remains. |
- Remove grader(s) upon deletion of existing entrance exam - Remove grader(s) upon addition of new entrance exam
mattdrayer
force-pushed
the
mattdrayer/SOL-475
branch
from
5e78779
to
c6ecf25
Compare
|
@chrisndodge -- this PR is ready for your review |
|
Jenkins, test this please. |
|
In terms of code, this looks good. Just reverse engineering the JIRA (I can look it up, but I want to just replay what I'm seeing here): the problem was we were adding a new grading section to the grade policy document, but we weren't removing the policy when the exam was removed, right? |
|
That is correct, yes -- basically we were building up some grader type cruft with each remove/add cycle, and the duplicate graders were resulting in duplicate entries on the instructor report |
|
yep, then this looks good to me |
|
Great, assuming that's a +1 from you, I'll merge after Jenkins approves |
Fix for SOL-475 (double exam grader bug)
@martynjames -- here is a fix for SOL-475 -- if you could take a look when you have a moment that would be great. @marcotuts, FYI