From 6085852c99de529b4bb3bee2e60eb3ce38e08e6f Mon Sep 17 00:00:00 2001 From: Matt Hughes Date: Fri, 27 Sep 2019 10:31:35 -0400 Subject: [PATCH] Attribute grade overrides from proctoring to the final reviewer More generally, makes it so we don't ever pull in the user to whom we're attributing overrides from the request, but always use the one passed in via method parameters Also pass through comments, if they're provided via this method JIRA:EDUCATOR-4641 --- lms/djangoapps/grades/models.py | 22 +++++++++++-------- .../rest_api/v1/tests/test_gradebook_views.py | 21 ++++++++++++++++++ lms/djangoapps/grades/services.py | 6 +++-- requirements/edx/base.txt | 2 +- requirements/edx/development.txt | 2 +- requirements/edx/testing.txt | 2 +- 6 files changed, 41 insertions(+), 14 deletions(-) diff --git a/lms/djangoapps/grades/models.py b/lms/djangoapps/grades/models.py index 6fed1d8abf..c6cb91b1d8 100644 --- a/lms/djangoapps/grades/models.py +++ b/lms/djangoapps/grades/models.py @@ -679,6 +679,7 @@ class PersistentSubsectionGradeOverride(models.Model): # model in the grades app, which will fail. if 'grades' in apps.app_configs: history = HistoricalRecords() + _history_user = None def __str__(self): return u', '.join([ @@ -734,15 +735,18 @@ class PersistentSubsectionGradeOverride(models.Model): log.info(u'Creating override for user ***{}*** for PersistentSubsectionGrade' u'***{}*** with override data ***{}*** and derived grade_defaults ***{}***.' .format(requesting_user, subsection_grade_model, override_data, grade_defaults)) - override, _ = PersistentSubsectionGradeOverride.objects.update_or_create( - grade=subsection_grade_model, - defaults=grade_defaults, - ) - - override_history_entry = override.history.first() - if not override_history_entry.history_user and requesting_user: - override_history_entry.history_user = requesting_user - override_history_entry.save() + try: + override = PersistentSubsectionGradeOverride.objects.get(grade=subsection_grade_model) + for key, value in six.iteritems(grade_defaults): + setattr(override, key, value) + except PersistentSubsectionGradeOverride.DoesNotExist: + override = PersistentSubsectionGradeOverride(grade=subsection_grade_model, **grade_defaults) + if requesting_user: + # setting this on a non-field attribute which simple + # history reads from to determine which user to attach to + # the history row + override._history_user = requesting_user # pylint: disable=protected-access + override.save() return override diff --git a/lms/djangoapps/grades/rest_api/v1/tests/test_gradebook_views.py b/lms/djangoapps/grades/rest_api/v1/tests/test_gradebook_views.py index ebbde7dab6..6a5046b52c 100644 --- a/lms/djangoapps/grades/rest_api/v1/tests/test_gradebook_views.py +++ b/lms/djangoapps/grades/rest_api/v1/tests/test_gradebook_views.py @@ -1813,6 +1813,27 @@ class SubsectionGradeViewTest(GradebookViewTestBase): assert expected_data == resp.data + def test_comment_appears(self): + """ + Test that comments passed (e.g. from proctoring) appear in the history rows + """ + proctoring_failure_fake_comment = "Failed Test Proctoring" + self.login_course_staff() + override = PersistentSubsectionGradeOverride.update_or_create_override( + requesting_user=self.global_staff, + subsection_grade_model=self.grade, + earned_all_override=0.0, + earned_graded_override=0.0, + feature=GradeOverrideFeatureEnum.proctoring, + comment=proctoring_failure_fake_comment + ) + + resp = self.client.get( + self.get_url(subsection_id=self.usage_key) + ) + + assert resp.data['history'][0]['override_reason'] == proctoring_failure_fake_comment + @ddt.data( 'login_staff', ) diff --git a/lms/djangoapps/grades/services.py b/lms/djangoapps/grades/services.py index 3d0798e272..713dc26da8 100644 --- a/lms/djangoapps/grades/services.py +++ b/lms/djangoapps/grades/services.py @@ -29,7 +29,7 @@ class GradesService(object): def override_subsection_grade( self, user_id, course_key_or_id, usage_key_or_id, earned_all=None, earned_graded=None, - feature=api.constants.GradeOverrideFeatureEnum.proctoring + feature=api.constants.GradeOverrideFeatureEnum.proctoring, overrider=None, comment=None ): """ Creates a PersistentSubsectionGradeOverride corresponding to the given @@ -46,7 +46,9 @@ class GradesService(object): usage_key_or_id, earned_all=earned_all, earned_graded=earned_graded, - feature=feature) + feature=feature, + overrider=overrider, + comment=comment) def undo_override_subsection_grade(self, user_id, course_key_or_id, usage_key_or_id, feature=api.constants.GradeOverrideFeatureEnum.proctoring): diff --git a/requirements/edx/base.txt b/requirements/edx/base.txt index 6d48f10453..7400dc4716 100644 --- a/requirements/edx/base.txt +++ b/requirements/edx/base.txt @@ -113,7 +113,7 @@ edx-oauth2-provider==1.3.1 edx-opaque-keys[django]==2.0.0 edx-organizations==2.1.0 edx-proctoring-proctortrack==1.0.5 -edx-proctoring==2.0.9 +edx-proctoring==2.1.0 edx-rbac==1.0.3 # via edx-enterprise edx-rest-api-client==1.9.2 edx-search==1.2.2 diff --git a/requirements/edx/development.txt b/requirements/edx/development.txt index 91dbb61564..78a595b1e1 100644 --- a/requirements/edx/development.txt +++ b/requirements/edx/development.txt @@ -137,7 +137,7 @@ edx-oauth2-provider==1.3.1 edx-opaque-keys[django]==2.0.0 edx-organizations==2.1.0 edx-proctoring-proctortrack==1.0.5 -edx-proctoring==2.0.9 +edx-proctoring==2.1.0 edx-rbac==1.0.3 edx-rest-api-client==1.9.2 edx-search==1.2.2 diff --git a/requirements/edx/testing.txt b/requirements/edx/testing.txt index e4686de764..554bef52ce 100644 --- a/requirements/edx/testing.txt +++ b/requirements/edx/testing.txt @@ -133,7 +133,7 @@ edx-oauth2-provider==1.3.1 edx-opaque-keys[django]==2.0.0 edx-organizations==2.1.0 edx-proctoring-proctortrack==1.0.5 -edx-proctoring==2.0.9 +edx-proctoring==2.1.0 edx-rbac==1.0.3 edx-rest-api-client==1.9.2 edx-search==1.2.2