From a9c1e1cf43b5152fe0f846d2b0ee0da3445d0c3d Mon Sep 17 00:00:00 2001 From: Tyler Hallada Date: Fri, 11 Aug 2017 13:53:34 -0400 Subject: [PATCH] Squashed commit of the following: commit 12880fa21eac0c2e69b2d832d42829f00484561d Author: Tyler Hallada Date: Fri Aug 11 11:17:22 2017 -0400 Tweak exam override messaging in progress.html commit 6f90352dc27b3a199eea7efaf84b72df50f570a9 Merge: e0e9f83789 7c535f5fb9 Author: Tyler Hallada Date: Thu Aug 10 17:38:13 2017 -0400 Merge remote-tracking branch 'origin/master' into EDUCATOR-926 commit e0e9f83789efa837577cedbcd5807c6fdba249e4 Author: Tyler Hallada Date: Thu Aug 10 17:35:26 2017 -0400 Revert "Comment out actual override and log instead" This reverts commit 4953cf30d9161ac0e9b9eab3f73b7bcf5960926e. commit 27d6b537c8915ecd1af8af51c8f9509db6ea0a26 Author: Tyler Hallada Date: Thu Aug 10 17:34:56 2017 -0400 Revert "Fix tests" This reverts commit 1b2fec21296f9ae022a8fa6a4c196a55a05a5098. commit cabddd504ddcb4f92aeed8b5fec4bff0d64a8de3 Author: Tyler Hallada Date: Thu Aug 10 17:34:36 2017 -0400 Revert "Remove override behavior and log instead" This reverts commit 1ab188033741e5a147746fa15ab965570439abb7. commit dc9407f4606804e00d5aee4e80d309c7d7f9c437 Author: Tyler Hallada Date: Thu Aug 10 17:34:15 2017 -0400 Revert "Fix python tests and use proctoring 1.0.0" This reverts commit 842ce8365f142ce6bb69d3a9848e638ee3221c88. --- lms/djangoapps/grades/models.py | 17 ++++- lms/djangoapps/grades/services.py | 78 ++++++++++++++------ lms/djangoapps/grades/tests/test_models.py | 5 +- lms/djangoapps/grades/tests/test_services.py | 34 ++++++++- lms/djangoapps/grades/tests/test_tasks.py | 16 ++-- lms/templates/courseware/progress.html | 17 ++--- 6 files changed, 121 insertions(+), 46 deletions(-) diff --git a/lms/djangoapps/grades/models.py b/lms/djangoapps/grades/models.py index 006880c580..d11a6bdb35 100644 --- a/lms/djangoapps/grades/models.py +++ b/lms/djangoapps/grades/models.py @@ -412,7 +412,22 @@ class PersistentSubsectionGrade(DeleteGradesMixin, TimeStampedModel): usage_key = params.pop('usage_key') # apply grade override if one exists before saving model - # EDUCTATOR-1127: remove override until this behavior is verified in production + try: + override = PersistentSubsectionGradeOverride.objects.get( + grade__user_id=user_id, + grade__course_id=usage_key.course_key, + grade__usage_key=usage_key, + ) + if override.earned_all_override is not None: + params['earned_all'] = override.earned_all_override + if override.possible_all_override is not None: + params['possible_all'] = override.possible_all_override + if override.earned_graded_override is not None: + params['earned_graded'] = override.earned_graded_override + if override.possible_graded_override is not None: + params['possible_graded'] = override.possible_graded_override + except PersistentSubsectionGradeOverride.DoesNotExist: + pass grade, _ = cls.objects.update_or_create( user_id=user_id, diff --git a/lms/djangoapps/grades/services.py b/lms/djangoapps/grades/services.py index 53ca93fab3..475e29c860 100644 --- a/lms/djangoapps/grades/services.py +++ b/lms/djangoapps/grades/services.py @@ -1,6 +1,5 @@ from datetime import datetime -import logging import pytz from opaque_keys.edx.keys import CourseKey, UsageKey @@ -12,8 +11,6 @@ from .constants import ScoreDatabaseTableEnum from .models import PersistentSubsectionGrade, PersistentSubsectionGradeOverride from .signals.signals import SUBSECTION_OVERRIDE_CHANGED -log = logging.getLogger(__name__) - def _get_key(key_or_id, key_cls): """ @@ -73,21 +70,40 @@ class GradesService(object): Fires off a recalculate_subsection_grade async task to update the PersistentSubsectionGrade table. Will not override earned_all or earned_graded value if they are None. Both default to None. """ + # prevent circular imports: + from .signals.handlers import SUBSECTION_OVERRIDE_EVENT_TYPE + course_key = _get_key(course_key_or_id, CourseKey) usage_key = _get_key(usage_key_or_id, UsageKey) - log.info( - u"EDUCATOR-1127: Subsection grade override for user {user_id} on subsection {usage_key} in course " - u"{course_key} would be created with params: {params}" - .format( - user_id=unicode(user_id), - usage_key=unicode(usage_key), - course_key=unicode(course_key), - params=unicode({ - 'earned_all': earned_all, - 'earned_graded': earned_graded, - }) - ) + grade = PersistentSubsectionGrade.objects.get( + user_id=user_id, + course_id=course_key, + usage_key=usage_key + ) + + # Create override that will prevent any future updates to grade + override, _ = PersistentSubsectionGradeOverride.objects.update_or_create( + grade=grade, + earned_all_override=earned_all, + earned_graded_override=earned_graded + ) + + # Cache a new event id and event type which the signal handler will use to emit a tracking log event. + create_new_event_transaction_id() + set_event_transaction_type(SUBSECTION_OVERRIDE_EVENT_TYPE) + + # Signal will trigger subsection recalculation which will call PersistentSubsectionGrade.update_or_create_grade + # which will use the above override to update the grade before writing to the table. + SUBSECTION_OVERRIDE_CHANGED.send( + sender=None, + user_id=user_id, + course_id=unicode(course_key), + usage_id=unicode(usage_key), + only_if_higher=False, + modified=override.modified, + score_deleted=False, + score_db_table=ScoreDatabaseTableEnum.overrides ) def undo_override_subsection_grade(self, user_id, course_key_or_id, usage_key_or_id): @@ -97,17 +113,33 @@ class GradesService(object): Fires off a recalculate_subsection_grade async task to update the PersistentSubsectionGrade table. If the override does not exist, no error is raised, it just triggers the recalculation. """ + # prevent circular imports: + from .signals.handlers import SUBSECTION_OVERRIDE_EVENT_TYPE + course_key = _get_key(course_key_or_id, CourseKey) usage_key = _get_key(usage_key_or_id, UsageKey) - log.info( - u"EDUCATOR-1127: Subsection grade override for user {user_id} on subsection {usage_key} in course " - u"{course_key} would be deleted" - .format( - user_id=unicode(user_id), - usage_key=unicode(usage_key), - course_key=unicode(course_key) - ) + override = self.get_subsection_grade_override(user_id, course_key, usage_key) + # Older rejected exam attempts that transition to verified might not have an override created + if override is not None: + override.delete() + + # Cache a new event id and event type which the signal handler will use to emit a tracking log event. + create_new_event_transaction_id() + set_event_transaction_type(SUBSECTION_OVERRIDE_EVENT_TYPE) + + # Signal will trigger subsection recalculation which will call PersistentSubsectionGrade.update_or_create_grade + # which will no longer use the above deleted override, and instead return the grade to the original score from + # the actual problem responses before writing to the table. + SUBSECTION_OVERRIDE_CHANGED.send( + sender=None, + user_id=user_id, + course_id=unicode(course_key), + usage_id=unicode(usage_key), + only_if_higher=False, + modified=datetime.now().replace(tzinfo=pytz.UTC), # Not used when score_deleted=True + score_deleted=True, + score_db_table=ScoreDatabaseTableEnum.overrides ) def should_override_grade_on_rejected_exam(self, course_key_or_id): diff --git a/lms/djangoapps/grades/tests/test_models.py b/lms/djangoapps/grades/tests/test_models.py index 9fabce19d0..39e31e273b 100644 --- a/lms/djangoapps/grades/tests/test_models.py +++ b/lms/djangoapps/grades/tests/test_models.py @@ -312,9 +312,8 @@ class PersistentSubsectionGradeTest(GradesModelTestCase): override = PersistentSubsectionGradeOverride(grade=grade, earned_all_override=0.0, earned_graded_override=0.0) override.save() grade = PersistentSubsectionGrade.update_or_create_grade(**self.params) - # EDUCATOR-1127 Override is not enabled yet, change to 0.0 when enabled - self.assertEqual(grade.earned_all, 6.0) - self.assertEqual(grade.earned_graded, 6.0) + self.assertEqual(grade.earned_all, 0.0) + self.assertEqual(grade.earned_graded, 0.0) def _assert_tracker_emitted_event(self, tracker_mock, grade): """ diff --git a/lms/djangoapps/grades/tests/test_services.py b/lms/djangoapps/grades/tests/test_services.py index 8a172d0bef..98262a88df 100644 --- a/lms/djangoapps/grades/tests/test_services.py +++ b/lms/djangoapps/grades/tests/test_services.py @@ -171,10 +171,28 @@ class GradesServiceTests(ModuleStoreTestCase): self.course.id, self.subsection.location ) - self.assertIsNone(override_obj) + self.assertIsNotNone(override_obj) + self.assertEqual(override_obj.earned_all_override, override['earned_all']) + self.assertEqual(override_obj.earned_graded_override, override['earned_graded']) + + self.assertEqual( + self.mock_signal.call_args, + call( + sender=None, + user_id=self.user.id, + course_id=unicode(self.course.id), + usage_id=unicode(self.subsection.location), + only_if_higher=False, + modified=override_obj.modified, + score_deleted=False, + score_db_table=ScoreDatabaseTableEnum.overrides + ) + ) @freeze_time('2017-01-01') def test_undo_override_subsection_grade(self): + override, _ = PersistentSubsectionGradeOverride.objects.update_or_create(grade=self.grade) + self.service.undo_override_subsection_grade( user_id=self.user.id, course_key_or_id=self.course.id, @@ -184,6 +202,20 @@ class GradesServiceTests(ModuleStoreTestCase): override = self.service.get_subsection_grade_override(self.user.id, self.course.id, self.subsection.location) self.assertIsNone(override) + self.assertEqual( + self.mock_signal.call_args, + call( + sender=None, + user_id=self.user.id, + course_id=unicode(self.course.id), + usage_id=unicode(self.subsection.location), + only_if_higher=False, + modified=datetime.now().replace(tzinfo=pytz.UTC), + score_deleted=True, + score_db_table=ScoreDatabaseTableEnum.overrides + ) + ) + @ddt.data( ['edX/DemoX/Demo_Course', CourseKey.from_string('edX/DemoX/Demo_Course'), CourseKey], ['course-v1:edX+DemoX+Demo_Course', CourseKey.from_string('course-v1:edX+DemoX+Demo_Course'), CourseKey], diff --git a/lms/djangoapps/grades/tests/test_tasks.py b/lms/djangoapps/grades/tests/test_tasks.py index d27bc8206d..005fba01cc 100644 --- a/lms/djangoapps/grades/tests/test_tasks.py +++ b/lms/djangoapps/grades/tests/test_tasks.py @@ -163,10 +163,10 @@ class RecalculateSubsectionGradeTest(HasCourseWithProblemsMixin, ModuleStoreTest self.assertEquals(mock_block_structure_create.call_count, 1) @ddt.data( - (ModuleStoreEnum.Type.mongo, 1, 29, True), - (ModuleStoreEnum.Type.mongo, 1, 25, False), - (ModuleStoreEnum.Type.split, 3, 29, True), - (ModuleStoreEnum.Type.split, 3, 25, False), + (ModuleStoreEnum.Type.mongo, 1, 30, True), + (ModuleStoreEnum.Type.mongo, 1, 26, False), + (ModuleStoreEnum.Type.split, 3, 30, True), + (ModuleStoreEnum.Type.split, 3, 26, False), ) @ddt.unpack def test_query_counts(self, default_store, num_mongo_calls, num_sql_calls, create_multiple_subsections): @@ -178,8 +178,8 @@ class RecalculateSubsectionGradeTest(HasCourseWithProblemsMixin, ModuleStoreTest self._apply_recalculate_subsection_grade() @ddt.data( - (ModuleStoreEnum.Type.mongo, 1, 29), - (ModuleStoreEnum.Type.split, 3, 29), + (ModuleStoreEnum.Type.mongo, 1, 30), + (ModuleStoreEnum.Type.split, 3, 30), ) @ddt.unpack def test_query_counts_dont_change_with_more_content(self, default_store, num_mongo_calls, num_sql_calls): @@ -239,8 +239,8 @@ class RecalculateSubsectionGradeTest(HasCourseWithProblemsMixin, ModuleStoreTest self.assertEqual(len(PersistentSubsectionGrade.bulk_read_grades(self.user.id, self.course.id)), 0) @ddt.data( - (ModuleStoreEnum.Type.mongo, 1, 26), - (ModuleStoreEnum.Type.split, 3, 26), + (ModuleStoreEnum.Type.mongo, 1, 27), + (ModuleStoreEnum.Type.split, 3, 27), ) @ddt.unpack def test_persistent_grades_enabled_on_course(self, default_store, num_mongo_queries, num_sql_queries): diff --git a/lms/templates/courseware/progress.html b/lms/templates/courseware/progress.html index ce71201f62..c60c2226ef 100644 --- a/lms/templates/courseware/progress.html +++ b/lms/templates/courseware/progress.html @@ -183,18 +183,15 @@ from django.utils.http import urlquote_plus %endif

- <%doc> - EDUCATOR-1127: Do not display override notice until override is enabled +

%if section.override is not None: -

- %if section.format is not None and section.format == "Exam": - ${_("Exam grade has been overridden due to a failed proctoring review.")} - %else: - ${_("Section grade has been overridden.")} - %endif -

+ %if section.format is not None and section.format == "Exam": + ${_("Suspicious activity detected during proctored exam review. Exam score 0.")} + %else: + ${_("Section grade has been overridden.")} + %endif %endif - +

%if len(section.problem_scores.values()) > 0: %if section.show_grades(staff_access):