From b0f40a111cab59b11e7688dc33d38916f1c9892d Mon Sep 17 00:00:00 2001
From: Jansen Kantor <jkantor@edx.org>
Date: Thu, 19 Sep 2019 13:43:35 -0400
Subject: [PATCH] add requesting_user to simple-history model (#21683)

---
 lms/djangoapps/grades/api.py                  |   3 +-
 lms/djangoapps/grades/models.py               |   5 +
 .../rest_api/v1/tests/test_gradebook_views.py |   4 +-
 lms/djangoapps/grades/tests/test_api.py       | 113 ++++++++++++++++++
 requirements/edx/base.txt                     |   2 +-
 requirements/edx/development.txt              |   2 +-
 requirements/edx/testing.txt                  |   2 +-
 7 files changed, 125 insertions(+), 6 deletions(-)
 create mode 100644 lms/djangoapps/grades/tests/test_api.py

diff --git a/lms/djangoapps/grades/api.py b/lms/djangoapps/grades/api.py
index 7802cf70819..d9b76afe4b5 100644
--- a/lms/djangoapps/grades/api.py
+++ b/lms/djangoapps/grades/api.py
@@ -40,7 +40,7 @@ def graded_subsections_for_course_id(course_id):
 
 def override_subsection_grade(
         user_id, course_key_or_id, usage_key_or_id, overrider=None, earned_all=None, earned_graded=None,
-        feature=constants.GradeOverrideFeatureEnum.proctoring
+        feature=constants.GradeOverrideFeatureEnum.proctoring, comment=None,
 ):
     """
     Creates a PersistentSubsectionGradeOverride corresponding to the given
@@ -68,6 +68,7 @@ def override_subsection_grade(
         system=feature,
         earned_all_override=earned_all,
         earned_graded_override=earned_graded,
+        comment=comment,
     )
 
     # Cache a new event id and event type which the signal handler will use to emit a tracking log event.
diff --git a/lms/djangoapps/grades/models.py b/lms/djangoapps/grades/models.py
index a887833ad33..d433eb2a687 100644
--- a/lms/djangoapps/grades/models.py
+++ b/lms/djangoapps/grades/models.py
@@ -737,6 +737,11 @@ class PersistentSubsectionGradeOverride(models.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()
+
         action = action or PersistentSubsectionGradeOverrideHistory.CREATE_OR_UPDATE
 
         PersistentSubsectionGradeOverrideHistory.objects.create(
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 1ca5d784e2f..36067291feb 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
@@ -1812,8 +1812,8 @@ class SubsectionGradeViewTest(GradebookViewTestBase):
                 ('system', None),
                 ('history_date', '2019-01-01T00:00:00Z'),
                 ('history_type', u'+'),
-                ('history_user', None),
-                ('history_user_id', None),
+                ('history_user', self.global_staff.username),
+                ('history_user_id', self.global_staff.id),
                 ('id', 1),
                 ('possible_all_override', 12.0),
                 ('possible_graded_override', 8.0),
diff --git a/lms/djangoapps/grades/tests/test_api.py b/lms/djangoapps/grades/tests/test_api.py
new file mode 100644
index 00000000000..eef4875e001
--- /dev/null
+++ b/lms/djangoapps/grades/tests/test_api.py
@@ -0,0 +1,113 @@
+""" Tests calling the grades api directly """
+
+from __future__ import absolute_import
+
+import ddt
+from mock import patch
+
+from lms.djangoapps.grades import api
+from lms.djangoapps.grades.models import (
+    PersistentSubsectionGrade,
+    PersistentSubsectionGradeOverride,
+)
+from student.tests.factories import UserFactory
+from xmodule.modulestore.tests.django_utils import ModuleStoreTestCase
+from xmodule.modulestore.tests.factories import CourseFactory, ItemFactory
+
+
+@ddt.ddt
+class OverrideSubsectionGradeTests(ModuleStoreTestCase):
+    """
+    Tests for the override subsection grades api call
+    """
+
+    @classmethod
+    def setUpTestData(cls):
+        super(OverrideSubsectionGradeTests, cls).setUpTestData()
+        cls.user = UserFactory()
+        cls.overriding_user = UserFactory()
+        cls.signal_patcher = patch('lms.djangoapps.grades.signals.signals.SUBSECTION_OVERRIDE_CHANGED.send')
+        cls.signal_patcher.start()
+        cls.id_patcher = patch('lms.djangoapps.grades.api.create_new_event_transaction_id')
+        cls.mock_create_id = cls.id_patcher.start()
+        cls.mock_create_id.return_value = 1
+        cls.type_patcher = patch('lms.djangoapps.grades.api.set_event_transaction_type')
+        cls.type_patcher.start()
+
+    @classmethod
+    def tearDownClass(cls):
+        super(OverrideSubsectionGradeTests, cls).tearDownClass()
+        cls.signal_patcher.stop()
+        cls.id_patcher.stop()
+        cls.type_patcher.stop()
+
+    def setUp(self):
+        super(OverrideSubsectionGradeTests, self).setUp()
+        self.course = CourseFactory.create(org='edX', number='DemoX', display_name='Demo_Course', run='Spring2019')
+        self.subsection = ItemFactory.create(parent=self.course, category="subsection", display_name="Subsection")
+        self.grade = PersistentSubsectionGrade.update_or_create_grade(
+            user_id=self.user.id,
+            course_id=self.course.id,
+            usage_key=self.subsection.location,
+            first_attempted=None,
+            visible_blocks=[],
+            earned_all=6.0,
+            possible_all=6.0,
+            earned_graded=5.0,
+            possible_graded=5.0
+        )
+
+    def tearDown(self):
+        super(OverrideSubsectionGradeTests, self).tearDown()
+        PersistentSubsectionGradeOverride.objects.all().delete()  # clear out all previous overrides
+
+    @ddt.data(0.0, None, 3.0)
+    def test_override_subsection_grade(self, earned_graded):
+        api.override_subsection_grade(
+            self.user.id,
+            self.course.id,
+            self.subsection.location,
+            overrider=self.overriding_user,
+            earned_graded=earned_graded,
+            comment='Test Override Comment',
+        )
+        override_obj = api.get_subsection_grade_override(
+            self.user.id,
+            self.course.id,
+            self.subsection.location
+        )
+        self.assertIsNotNone(override_obj)
+        self.assertEqual(override_obj.earned_graded_override, earned_graded)
+        self.assertEqual(override_obj.override_reason, 'Test Override Comment')
+
+        for i in range(3):
+            override_obj.override_reason = 'this field purposefully left blank'
+            override_obj.earned_graded_override = i
+            override_obj.save()
+
+        api.override_subsection_grade(
+            self.user.id,
+            self.course.id,
+            self.subsection.location,
+            overrider=self.overriding_user,
+            earned_graded=earned_graded,
+            comment='Test Override Comment 2',
+        )
+        override_obj = api.get_subsection_grade_override(
+            self.user.id,
+            self.course.id,
+            self.subsection.location
+        )
+
+        self.assertIsNotNone(override_obj)
+        self.assertEqual(override_obj.earned_graded_override, earned_graded)
+        self.assertEqual(override_obj.override_reason, 'Test Override Comment 2')
+
+        self.assertEqual(5, len(override_obj.history.all()))
+        for history_entry in override_obj.history.all():
+            if history_entry.override_reason.startswith('Test Override Comment'):
+                self.assertEquals(self.overriding_user, history_entry.history_user)
+                self.assertEquals(self.overriding_user.id, history_entry.history_user_id)
+            else:
+                self.assertIsNone(history_entry.history_user)
+                self.assertIsNone(history_entry.history_user_id)
diff --git a/requirements/edx/base.txt b/requirements/edx/base.txt
index efe7ee039c8..f812f6b7135 100644
--- a/requirements/edx/base.txt
+++ b/requirements/edx/base.txt
@@ -97,7 +97,7 @@ docutils==0.15.2          # via botocore
 drf-yasg==1.16
 edx-ace==0.1.10
 edx-analytics-data-api-client==0.15.3
-edx-bulk-grades==0.6.0
+edx-bulk-grades==0.6.1
 edx-ccx-keys==1.0.0
 edx-celeryutils==0.3.0
 edx-completion==2.0.0
diff --git a/requirements/edx/development.txt b/requirements/edx/development.txt
index 58e9b77fd96..a930954e9f9 100644
--- a/requirements/edx/development.txt
+++ b/requirements/edx/development.txt
@@ -121,7 +121,7 @@ docutils==0.15.2
 drf-yasg==1.16
 edx-ace==0.1.10
 edx-analytics-data-api-client==0.15.3
-edx-bulk-grades==0.6.0
+edx-bulk-grades==0.6.1
 edx-ccx-keys==1.0.0
 edx-celeryutils==0.3.0
 edx-completion==2.0.0
diff --git a/requirements/edx/testing.txt b/requirements/edx/testing.txt
index c047db6bc38..fb6307bc5b3 100644
--- a/requirements/edx/testing.txt
+++ b/requirements/edx/testing.txt
@@ -117,7 +117,7 @@ docutils==0.15.2
 drf-yasg==1.16
 edx-ace==0.1.10
 edx-analytics-data-api-client==0.15.3
-edx-bulk-grades==0.6.0
+edx-bulk-grades==0.6.1
 edx-ccx-keys==1.0.0
 edx-celeryutils==0.3.0
 edx-completion==2.0.0
-- 
GitLab