From c6ecf2551e3b9ae3faab09eb98fb3f94235bb10f Mon Sep 17 00:00:00 2001 From: Matt Drayer Date: Mon, 27 Apr 2015 13:44:34 -0400 Subject: [PATCH] Fix for SOL-475 (double exam grader bug) - Remove grader(s) upon deletion of existing entrance exam - Remove grader(s) upon addition of new entrance exam --- .../contentstore/views/entrance_exam.py | 5 +++- cms/djangoapps/contentstore/views/helpers.py | 30 +++++++++++++++++-- .../views/tests/test_entrance_exam.py | 11 ++++++- 3 files changed, 42 insertions(+), 4 deletions(-) diff --git a/cms/djangoapps/contentstore/views/entrance_exam.py b/cms/djangoapps/contentstore/views/entrance_exam.py index 2643ccf15e..a6b1f85fc4 100644 --- a/cms/djangoapps/contentstore/views/entrance_exam.py +++ b/cms/djangoapps/contentstore/views/entrance_exam.py @@ -10,7 +10,7 @@ from django.contrib.auth.decorators import login_required from django_future.csrf import ensure_csrf_cookie from django.http import HttpResponse, HttpResponseBadRequest -from contentstore.views.helpers import create_xblock +from contentstore.views.helpers import create_xblock, remove_entrance_exam_graders from contentstore.views.item import delete_item from models.settings.course_metadata import CourseMetadata from opaque_keys.edx.keys import CourseKey, UsageKey @@ -270,6 +270,9 @@ def _delete_entrance_exam(request, course_key): } CourseMetadata.update_from_dict(metadata, course, request.user) + # Clean up any pre-existing entrance exam graders + remove_entrance_exam_graders(course_key, request.user) + return HttpResponse(status=204) diff --git a/cms/djangoapps/contentstore/views/helpers.py b/cms/djangoapps/contentstore/views/helpers.py index 9b2b0886df..72df1bc44d 100644 --- a/cms/djangoapps/contentstore/views/helpers.py +++ b/cms/djangoapps/contentstore/views/helpers.py @@ -25,6 +25,17 @@ from models.settings.course_grading import CourseGradingModel __all__ = ['edge', 'event', 'landing'] +# Note: Grader types are used throughout the platform but most usages are simply in-line +# strings. In addition, new grader types can be defined on the fly anytime one is needed +# (because they're just strings). This dict is an attempt to constrain the sprawl in Studio. +GRADER_TYPES = { + "HOMEWORK": "Homework", + "LAB": "Lab", + "ENTRANCE_EXAM": "Entrance Exam", + "MIDTERM_EXAM": "Midterm Exam", + "FINAL_EXAM": "Final Exam" +} + # points to the temporary course landing page with log in and sign up def landing(request, org, course, coursename): @@ -173,6 +184,18 @@ def usage_key_with_run(usage_key_string): return usage_key +def remove_entrance_exam_graders(course_key, user): + """ + Removes existing entrance exam graders attached to the specified course + Typically used when adding/removing an entrance exam. + """ + grading_model = CourseGradingModel.fetch(course_key) + graders = grading_model.graders + for i, grader in enumerate(graders): + if grader['type'] == GRADER_TYPES['ENTRANCE_EXAM']: + CourseGradingModel.delete_grader(course_key, i, user) + + def create_xblock(parent_locator, user, category, display_name, boilerplate=None, is_entrance_exam=False): """ Performs the actual grunt work of creating items/xblocks -- knows nothing about requests, views, etc. @@ -228,11 +251,14 @@ def create_xblock(parent_locator, user, category, display_name, boilerplate=None # Entrance Exams: Grader assignment if settings.FEATURES.get('ENTRANCE_EXAMS', False): - course = store.get_course(usage_key.course_key) + course_key = usage_key.course_key + course = store.get_course(course_key) if hasattr(course, 'entrance_exam_enabled') and course.entrance_exam_enabled: if category == 'sequential' and parent_locator == course.entrance_exam_id: + # Clean up any pre-existing entrance exam graders + remove_entrance_exam_graders(course_key, user) grader = { - "type": "Entrance Exam", + "type": GRADER_TYPES['ENTRANCE_EXAM'], "min_count": 0, "drop_count": 0, "short_label": "Entrance", diff --git a/cms/djangoapps/contentstore/views/tests/test_entrance_exam.py b/cms/djangoapps/contentstore/views/tests/test_entrance_exam.py index 0a720f98e8..3f07a1bcba 100644 --- a/cms/djangoapps/contentstore/views/tests/test_entrance_exam.py +++ b/cms/djangoapps/contentstore/views/tests/test_entrance_exam.py @@ -11,6 +11,7 @@ from django.test.client import RequestFactory from contentstore.tests.utils import AjaxEnabledTestClient, CourseTestCase from contentstore.utils import reverse_url from contentstore.views.entrance_exam import create_entrance_exam, update_entrance_exam, delete_entrance_exam +from contentstore.views.helpers import GRADER_TYPES from models.settings.course_grading import CourseGradingModel from models.settings.course_metadata import CourseMetadata from opaque_keys.edx.keys import UsageKey @@ -84,7 +85,7 @@ class EntranceExamHandlerTests(CourseTestCase): seq_locator_string = json.loads(resp.content).get('locator') seq_locator = UsageKey.from_string(seq_locator_string) section_grader_type = CourseGradingModel.get_section_grader_type(seq_locator) - self.assertEqual('Entrance Exam', section_grader_type['graderType']) + self.assertEqual(GRADER_TYPES['ENTRANCE_EXAM'], section_grader_type['graderType']) def test_contentstore_views_entrance_exam_get(self): """ @@ -140,6 +141,14 @@ class EntranceExamHandlerTests(CourseTestCase): resp = self.client.get(self.exam_url) self.assertEqual(resp.status_code, 200) + # Confirm that we have only one Entrance Exam grader after re-adding the exam (validates SOL-475) + graders = CourseGradingModel.fetch(self.course_key).graders + count = 0 + for grader in graders: + if grader['type'] == GRADER_TYPES['ENTRANCE_EXAM']: + count += 1 + self.assertEqual(count, 1) + def test_contentstore_views_entrance_exam_delete_bogus_course(self): """ Unit Test: test_contentstore_views_entrance_exam_delete_bogus_course