From b1e8dbe2d4345e3002a9531ca5845ad9766511a2 Mon Sep 17 00:00:00 2001 From: atesker Date: Wed, 3 Jul 2019 13:28:27 -0400 Subject: [PATCH] EDUCATOR-4353 updated history override backend unit test fixes code review comments --- .../migrations/0016_auto_20190703_1446.py | 35 +++++++++++++++++++ lms/djangoapps/grades/models.py | 11 ++++-- lms/djangoapps/grades/rest_api/serializers.py | 3 +- .../grades/rest_api/v1/gradebook_views.py | 21 +++++++++-- .../rest_api/v1/tests/test_gradebook_views.py | 6 ++-- 5 files changed, 67 insertions(+), 9 deletions(-) create mode 100644 lms/djangoapps/grades/migrations/0016_auto_20190703_1446.py diff --git a/lms/djangoapps/grades/migrations/0016_auto_20190703_1446.py b/lms/djangoapps/grades/migrations/0016_auto_20190703_1446.py new file mode 100644 index 0000000000..b7c21d6e61 --- /dev/null +++ b/lms/djangoapps/grades/migrations/0016_auto_20190703_1446.py @@ -0,0 +1,35 @@ +# -*- coding: utf-8 -*- +# Generated by Django 1.11.21 on 2019-07-03 14:46 +from __future__ import unicode_literals + +from django.db import migrations, models + + +class Migration(migrations.Migration): + + dependencies = [ + ('grades', '0015_historicalpersistentsubsectiongradeoverride'), + ] + + operations = [ + migrations.AddField( + model_name='historicalpersistentsubsectiongradeoverride', + name='override_reason', + field=models.CharField(blank=True, max_length=300, null=True), + ), + migrations.AddField( + model_name='historicalpersistentsubsectiongradeoverride', + name='system', + field=models.CharField(blank=True, max_length=100, null=True), + ), + migrations.AddField( + model_name='persistentsubsectiongradeoverride', + name='override_reason', + field=models.CharField(blank=True, max_length=300, null=True), + ), + migrations.AddField( + model_name='persistentsubsectiongradeoverride', + name='system', + field=models.CharField(blank=True, max_length=100, null=True), + ), + ] diff --git a/lms/djangoapps/grades/models.py b/lms/djangoapps/grades/models.py index b43826e5b0..278a5ef31d 100644 --- a/lms/djangoapps/grades/models.py +++ b/lms/djangoapps/grades/models.py @@ -27,7 +27,6 @@ from coursewarehistoryextended.fields import UnsignedBigIntAutoField, UnsignedBi from lms.djangoapps.grades import events, constants from openedx.core.lib.cache_utils import get_cache from simple_history.models import HistoricalRecords -from simple_history.utils import update_change_reason log = logging.getLogger(__name__) @@ -652,6 +651,10 @@ class PersistentSubsectionGradeOverride(models.Model): possible_all_override = models.FloatField(null=True, blank=True) earned_graded_override = models.FloatField(null=True, blank=True) possible_graded_override = models.FloatField(null=True, blank=True) + # store the source of the system that caused the override + system = models.CharField(max_length=100, blank=True, null=True) + # store the reason for the override + override_reason = models.CharField(max_length=300, blank=True, null=True) _CACHE_NAMESPACE = u"grades.models.PersistentSubsectionGradeOverride" @@ -707,11 +710,13 @@ class PersistentSubsectionGradeOverride(models.Model): subsection_grade_model: The PersistentSubsectionGrade object associated with this override. override_data: The parameters of score values used to create the override record. """ + grade_defaults = cls._prepare_override_params(subsection_grade_model, override_data) + grade_defaults['override_reason'] = override_data['comment'] if 'comment' in override_data else None + grade_defaults['system'] = override_data['system'] if 'system' in override_data else None override, _ = PersistentSubsectionGradeOverride.objects.update_or_create( grade=subsection_grade_model, - defaults=cls._prepare_override_params(subsection_grade_model, override_data), + defaults=grade_defaults, ) - update_change_reason(override, feature) action = action or PersistentSubsectionGradeOverrideHistory.CREATE_OR_UPDATE diff --git a/lms/djangoapps/grades/rest_api/serializers.py b/lms/djangoapps/grades/rest_api/serializers.py index 99322a6485..5c40629977 100644 --- a/lms/djangoapps/grades/rest_api/serializers.py +++ b/lms/djangoapps/grades/rest_api/serializers.py @@ -85,7 +85,8 @@ class SubsectionGradeOverrideSimpleHistorySerializer(serializers.Serializer): history_id = serializers.IntegerField() earned_all_override = serializers.FloatField() earned_graded_override = serializers.FloatField() - history_change_reason = serializers.CharField() + override_reason = serializers.CharField() + system = serializers.CharField() history_date = serializers.DateTimeField() history_type = serializers.CharField() history_user = serializers.CharField() diff --git a/lms/djangoapps/grades/rest_api/v1/gradebook_views.py b/lms/djangoapps/grades/rest_api/v1/gradebook_views.py index e6843b8c2b..9a60987bc8 100644 --- a/lms/djangoapps/grades/rest_api/v1/gradebook_views.py +++ b/lms/djangoapps/grades/rest_api/v1/gradebook_views.py @@ -333,6 +333,8 @@ class GradebookView(GradeViewMixin, PaginatedAPIView): **Example Request** GET /api/grades/v1/gradebook/{course_id}/ - Get gradebook entries for all users in course GET /api/grades/v1/gradebook/{course_id}/?username={username} - Get grades for specific user in course + GET /api/grades/v1/gradebook/{course_id}/?username={username}?history_record_limit={number} + - Get grades for specific user in course, only show {number} latest records GET /api/grades/v1/gradebook/{course_id}/?user_contains={user_contains} GET /api/grades/v1/gradebook/{course_id}/?username_contains={username_contains} GET /api/grades/v1/gradebook/{course_id}/?cohort_id={cohort_id} @@ -589,7 +591,8 @@ class GradebookBulkUpdateView(GradeViewMixin, PaginatedAPIView): "earned_all_override": 11, "possible_all_override": 11, "earned_graded_override": 11, - "possible_graded_override": 11 + "possible_graded_override": 11, + "comment": "reason for override" } }, { @@ -599,7 +602,8 @@ class GradebookBulkUpdateView(GradeViewMixin, PaginatedAPIView): "earned_all_override": 10, "possible_all_override": 15, "earned_graded_override": 9, - "possible_graded_override": 12 + "possible_graded_override": 12, + "comment": "reason for override" } } ] @@ -736,6 +740,7 @@ class GradebookBulkUpdateView(GradeViewMixin, PaginatedAPIView): Helper method to create a `PersistentSubsectionGradeOverride` object and send a `SUBSECTION_OVERRIDE_CHANGED` signal. """ + override_data['system'] = grades_constants.GradeOverrideFeatureEnum.gradebook override = PersistentSubsectionGradeOverride.update_or_create_override( requesting_user=request_user, subsection_grade_model=subsection_grade_model, @@ -923,9 +928,19 @@ class SubsectionGradeView(GradeViewMixin, APIView): return Response(results.data) + limit_history_request_value = request.GET.get('history_record_limit') + if limit_history_request_value is not None: + try: + history_record_limit = int(limit_history_request_value) + except ValueError: + history_record_limit = 0 + try: override = original_grade.override - history = override.history.all() + if limit_history_request_value is not None: + history = override.history.all().order_by('history_date')[:history_record_limit] + else: + history = override.history.all().order_by('history_date') except PersistentSubsectionGradeOverride.DoesNotExist: override = None history = [] 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 519e0649a1..5361155226 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 @@ -1485,7 +1485,8 @@ class SubsectionGradeViewTest(GradebookViewTestBase): ('history_id', 1), ('earned_all_override', 0.0), ('earned_graded_override', 0.0), - ('history_change_reason', None), + ('override_reason', None), + ('system', None), ('history_date', '2019-01-01T00:00:00Z'), ('history_type', u'+'), ('history_user', None), @@ -1541,7 +1542,8 @@ class SubsectionGradeViewTest(GradebookViewTestBase): ('history_id', 1), ('earned_all_override', 0.0), ('earned_graded_override', 0.0), - ('history_change_reason', 'GRADEBOOK'), + ('override_reason', None), + ('system', None), ('history_date', '2019-01-01T00:00:00Z'), ('history_type', u'+'), ('history_user', None),