From e07635559c4d1fe8c6cdb54fc3e3f0c09127bd67 Mon Sep 17 00:00:00 2001 From: atesker <andrewtr@gmail.com> Date: Fri, 27 Sep 2019 15:11:57 -0400 Subject: [PATCH] EDUCATOR-4365 - display max score for not started assignment pep8 moved includes refactor 1 correted properties update unit test pep update cr 2 made failure case a function --- .../grades/rest_api/v1/gradebook_views.py | 71 ++++++++++-------- .../rest_api/v1/tests/test_gradebook_views.py | 72 ++++++++++++++++--- .../grades/subsection_grade_factory.py | 6 +- 3 files changed, 108 insertions(+), 41 deletions(-) diff --git a/lms/djangoapps/grades/rest_api/v1/gradebook_views.py b/lms/djangoapps/grades/rest_api/v1/gradebook_views.py index 3a13719f304..71b61a7a7e8 100644 --- a/lms/djangoapps/grades/rest_api/v1/gradebook_views.py +++ b/lms/djangoapps/grades/rest_api/v1/gradebook_views.py @@ -9,6 +9,7 @@ from contextlib import contextmanager from functools import wraps import six +from django.contrib.auth import get_user_model from django.core.cache import cache from django.db.models import Case, Exists, F, OuterRef, When, Q from django.urls import reverse @@ -42,7 +43,9 @@ from lms.djangoapps.grades.rest_api.serializers import ( ) from lms.djangoapps.grades.rest_api.v1.utils import USER_MODEL, CourseEnrollmentPagination, GradeViewMixin from lms.djangoapps.grades.subsection_grade import CreateSubsectionGrade +from lms.djangoapps.grades.subsection_grade_factory import SubsectionGradeFactory from lms.djangoapps.grades.tasks import recalculate_subsection_grade_v3 +from lms.djangoapps.course_blocks.api import get_course_blocks from openedx.core.djangoapps.course_groups import cohorts from openedx.core.djangoapps.util.forms import to_bool from openedx.core.lib.api.view_utils import ( @@ -1044,44 +1047,52 @@ class SubsectionGradeView(GradeViewMixin, APIView): developer_message='Invalid UserID', error_code='invalid_user_id' ) - - try: - original_grade = PersistentSubsectionGrade.read_grade(user_id, usage_key) - except PersistentSubsectionGrade.DoesNotExist: - results = SubsectionGradeResponseSerializer({ - 'original_grade': None, - 'override': None, - 'history': [], - 'subsection_id': usage_key, - 'user_id': user_id, - 'course_id': None, - }) - - return Response(results.data) - - limit_history_request_value = request.GET.get('history_record_limit') - if limit_history_request_value is not None: + override = None + history = [] + history_record_limit = request.GET.get('history_record_limit') + if history_record_limit is not None: try: - history_record_limit = int(limit_history_request_value) + history_record_limit = int(history_record_limit) except ValueError: history_record_limit = 0 try: - override = original_grade.override - if limit_history_request_value is not None: - history = reversed(list(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 = [] + original_grade = PersistentSubsectionGrade.read_grade(user_id, usage_key) + if original_grade is not None and hasattr(original_grade, 'override'): + override = original_grade.override + history = list(original_grade.override.history.all().order_by('history_date')[:history_record_limit]) + grade_data = { + 'earned_all': original_grade.earned_all, + 'possible_all': original_grade.possible_all, + 'earned_graded': original_grade.earned_graded, + 'possible_graded': original_grade.possible_graded, + } + + except PersistentSubsectionGrade.DoesNotExist: + grade_data = self._get_grade_data_for_not_attempted_assignment(user_id, usage_key) results = SubsectionGradeResponseSerializer({ - 'original_grade': original_grade, + 'original_grade': grade_data, 'override': override, 'history': history, - 'subsection_id': original_grade.usage_key, - 'user_id': original_grade.user_id, - 'course_id': original_grade.course_id, + 'subsection_id': usage_key, + 'user_id': user_id, + 'course_id': usage_key.course_key, }) return Response(results.data) + + def _get_grade_data_for_not_attempted_assignment(self, user_id, usage_key): + """ + Return grade for an assignment that wasn't attempted + """ + student = get_user_model().objects.get(id=user_id) + course_structure = get_course_blocks(student, usage_key) + subsection_grade_factory = SubsectionGradeFactory(student, course_structure=course_structure) + grade = subsection_grade_factory.create(course_structure[usage_key], read_only=True, force_calculate=True) + grade_data = { + 'earned_all': grade.all_total.earned, + 'possible_all': grade.all_total.possible, + 'earned_graded': grade.graded_total.earned, + 'possible_graded': grade.graded_total.possible, + } + return grade_data 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 6a5046b52c6..0041547ecc2 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 @@ -1644,7 +1644,8 @@ class SubsectionGradeViewTest(GradebookViewTestBase): cls.record_b = BlockRecord(locator=cls.locator_b, weight=1, raw_possible=10, graded=True) cls.block_records = BlockRecordList([cls.record_a, cls.record_b], cls.course_key) cls.usage_key = cls.subsections[cls.chapter_1.location][0].location - cls.user_id = 12345 + cls.user = UserFactory.create() + cls.user_id = cls.user.id cls.params = { "user_id": cls.user_id, "usage_key": cls.usage_key, @@ -1671,6 +1672,54 @@ class SubsectionGradeViewTest(GradebookViewTestBase): ) return "{0}?user_id={1}".format(base_url, user_id or self.user_id) + @patch('lms.djangoapps.grades.subsection_grade_factory.SubsectionGradeFactory.create') + @ddt.data( + 'login_staff', + 'login_course_admin', + 'login_course_staff', + ) + def test_no_grade(self, login_method, mocked_factory): + getattr(self, login_method)() + user_no_grade = UserFactory.create() + all_total_mock = MagicMock( + earned=1, + possible=2, + ) + graded_total_mock = MagicMock( + earned=3, + possible=4, + ) + mock_return_value = MagicMock( + all_total=all_total_mock, + graded_total=graded_total_mock + ) + mocked_factory.return_value = mock_return_value + with self.assertRaises(PersistentSubsectionGrade.DoesNotExist): + PersistentSubsectionGrade.objects.get( + user_id=user_no_grade.id, + course_id=self.usage_key.course_key, + usage_key=self.usage_key + ) + + resp = self.client.get( + self.get_url(subsection_id=self.usage_key, user_id=user_no_grade.id) + ) + + expected_data = { + 'original_grade': OrderedDict([ + ('earned_all', 1.0), + ('possible_all', 2.0), + ('earned_graded', 3.0), + ('possible_graded', 4.0) + ]), + 'user_id': user_no_grade.id, + 'override': None, + 'course_id': text_type(self.usage_key.course_key), + 'subsection_id': text_type(self.usage_key), + 'history': [] + } + self.assertEqual(expected_data, resp.data) + @ddt.data( 'login_staff', 'login_course_admin', @@ -1690,7 +1739,7 @@ class SubsectionGradeViewTest(GradebookViewTestBase): ('earned_graded', 6.0), ('possible_graded', 8.0) ]), - 'user_id': 12345, + 'user_id': self.user_id, 'override': None, 'course_id': text_type(self.course_key), 'subsection_id': text_type(self.usage_key), @@ -1727,7 +1776,7 @@ class SubsectionGradeViewTest(GradebookViewTestBase): ('earned_graded', 6.0), ('possible_graded', 8.0) ]), - 'user_id': 12345, + 'user_id': self.user_id, 'override': OrderedDict([ ('earned_all_override', 0.0), ('possible_all_override', 12.0), @@ -1784,7 +1833,7 @@ class SubsectionGradeViewTest(GradebookViewTestBase): ('earned_graded', 6.0), ('possible_graded', 8.0) ]), - 'user_id': 12345, + 'user_id': self.user_id, 'override': OrderedDict([ ('earned_all_override', 0.0), ('possible_all_override', 12.0), @@ -1874,15 +1923,20 @@ class SubsectionGradeViewTest(GradebookViewTestBase): feature=GradeOverrideFeatureEnum.gradebook, ) + other_user = UserFactory.create() resp = self.client.get( - self.get_url(subsection_id=self.usage_key, user_id=6789) + self.get_url(subsection_id=self.usage_key, user_id=other_user.id) ) - expected_data = { - 'original_grade': None, - 'user_id': 6789, + 'original_grade': OrderedDict([ + ('earned_all', 0.0), + ('possible_all', 0.0), + ('earned_graded', 0.0), + ('possible_graded', 0.0) + ]), + 'user_id': other_user.id, 'override': None, - 'course_id': None, + 'course_id': text_type(self.usage_key.course_key), 'subsection_id': text_type(self.usage_key), 'history': [] } diff --git a/lms/djangoapps/grades/subsection_grade_factory.py b/lms/djangoapps/grades/subsection_grade_factory.py index c7f89b8cc52..8765b503f96 100644 --- a/lms/djangoapps/grades/subsection_grade_factory.py +++ b/lms/djangoapps/grades/subsection_grade_factory.py @@ -34,11 +34,13 @@ class SubsectionGradeFactory(object): self._cached_subsection_grades = None self._unsaved_subsection_grades = OrderedDict() - def create(self, subsection, read_only=False): + def create(self, subsection, read_only=False, force_calculate=False): """ Returns the SubsectionGrade object for the student and subsection. If read_only is True, doesn't save any updates to the grades. + force_calculate - If true, will cause this function to return a `CreateSubsectionGrade` object if no cached + grade currently exists, even if the assume_zero_if_absent flag is enabled for the course. """ self._log_event( log.debug, u"create, read_only: {0}, subsection: {1}".format(read_only, subsection.location), subsection, @@ -46,7 +48,7 @@ class SubsectionGradeFactory(object): subsection_grade = self._get_bulk_cached_grade(subsection) if not subsection_grade: - if assume_zero_if_absent(self.course_data.course_key): + if assume_zero_if_absent(self.course_data.course_key) and not force_calculate: subsection_grade = ZeroSubsectionGrade(subsection, self.course_data) else: subsection_grade = CreateSubsectionGrade( -- GitLab