diff --git a/lms/djangoapps/grades/course_data.py b/lms/djangoapps/grades/course_data.py index db121f7a280a40d21ee7c6a252075c88671344c9..96e137355ad5ad9eefe8e4801d8f860657c1291b 100644 --- a/lms/djangoapps/grades/course_data.py +++ b/lms/djangoapps/grades/course_data.py @@ -120,4 +120,16 @@ class CourseData: @property def effective_structure(self): + """ + Get whichever course block structure is already loaded, if any. + + This may give either the user-specific course structure or the generic + structure, depending on which is cached at the moment. Because of that, + this should only be used for queries related to the root block of the + course, which will always exist in either structure. + + For anything else, such as queries involving course sections or blocks, + use either .structure or .collected_structure to explicitly state + whether you want the user-specific version of the course or not. + """ return self._structure or self._collected_block_structure diff --git a/lms/djangoapps/grades/course_grade.py b/lms/djangoapps/grades/course_grade.py index 1f14ceecea75d0210f5c9bc2c921bf93c0968c91..21c8b3ec080dcf789822fa6b5556b81fba2a9fdb 100644 --- a/lms/djangoapps/grades/course_grade.py +++ b/lms/djangoapps/grades/course_grade.py @@ -55,12 +55,23 @@ class CourseGradeBase: """ Returns the subsection grade for the given subsection usage key. - Note: does NOT check whether the user has access to the subsection. - Assumes that if a grade exists, the user has access to it. If the - grade doesn't exist then either the user does not have access to - it or hasn't attempted any problems in the subsection. - """ - return self._get_subsection_grade(self.course_data.effective_structure[subsection_key]) + Raises `KeyError` if the course structure does not contain the key. + + If the course structure contains the key, this will always succeed + (and return a grade) regardless of whether the user can access that section; + it is up to the caller to ensure that the grade isn't + shown to users that shouldn't be able to access it + (e.g. a student shouldn't see a grade for an unreleased subsection); + """ + # look in the user structure first and fallback to the collected; + # however, we assume the state of course_data is intentional, + # so we use effective_structure to avoid additional fetching + subsection = ( + self.course_data.effective_structure[subsection_key] + if subsection_key in self.course_data.effective_structure + else self.course_data.collected_structure[subsection_key] + ) + return self._get_subsection_grade(subsection) @lazy def graded_subsections_by_format(self): diff --git a/lms/djangoapps/instructor_task/tests/test_tasks_helper.py b/lms/djangoapps/instructor_task/tests/test_tasks_helper.py index 8d3ffb08ba36ebd455665c73361e0ea63152da4d..d6dc29f21f41546219cb44636775dc07135a8099 100644 --- a/lms/djangoapps/instructor_task/tests/test_tasks_helper.py +++ b/lms/djangoapps/instructor_task/tests/test_tasks_helper.py @@ -1699,6 +1699,8 @@ class TestGradeReport(TestReportMixin, InstructorTaskModuleTestCase): """ Creates a course with various subsections for testing """ + in_the_past = datetime.now(UTC) - timedelta(days=5) + in_the_future = datetime.now(UTC) + timedelta(days=5) self.course = CourseFactory.create( grading_policy={ "GRADER": [ @@ -1710,6 +1712,7 @@ class TestGradeReport(TestReportMixin, InstructorTaskModuleTestCase): }, ], }, + metadata={"start": in_the_past} ) self.chapter = ItemFactory.create(parent=self.course, category='chapter') @@ -1741,11 +1744,21 @@ class TestGradeReport(TestReportMixin, InstructorTaskModuleTestCase): metadata={'graded': True, 'format': 'Homework'}, display_name='Empty', ) + self.unreleased_section = ItemFactory.create( + parent=self.chapter, + category='sequential', + metadata={'graded': True, 'format': 'Homework', 'start': in_the_future}, + display_name='Unreleased' + ) + self.define_option_problem(u'Unreleased', parent=self.unreleased_section) - def test_grade_report(self): + @patch.dict(settings.FEATURES, {'DISABLE_START_DATES': False}) + @ddt.data(True, False) + def test_grade_report(self, persistent_grades_enabled): self.submit_student_answer(self.student.username, 'Problem1', ['Option 1']) - with patch('lms.djangoapps.instructor_task.tasks_helper.runner._get_current_task'): + with patch('lms.djangoapps.instructor_task.tasks_helper.runner._get_current_task'), \ + patch.dict(settings.FEATURES, {'PERSISTENT_GRADES_ENABLED_FOR_ALL_TESTS': persistent_grades_enabled}): result = CourseGradeReport.generate(None, None, self.course.id, None, 'graded') self.assertDictContainsSubset( {'action_name': 'graded', 'attempted': 1, 'succeeded': 1, 'failed': 0}, @@ -1761,7 +1774,8 @@ class TestGradeReport(TestReportMixin, InstructorTaskModuleTestCase): 'Homework 1: Subsection': '0.5', 'Homework 2: Unattempted': 'Not Attempted', 'Homework 3: Empty': 'Not Attempted', - 'Homework (Avg)': str(1.0 / 6.0), + 'Homework 4: Unreleased': 'Not Attempted', + 'Homework (Avg)': str(0.5 / 4), }, ], ignore_other_columns=True, @@ -1798,7 +1812,8 @@ class TestGradeReport(TestReportMixin, InstructorTaskModuleTestCase): 'Homework 1: Subsection': '0.5', 'Homework 2: Unattempted': '1.0', 'Homework 3: Empty': 'Not Attempted', - 'Homework (Avg)': str(3.0 / 6.0), + 'Homework 4: Unreleased': 'Not Attempted', + 'Homework (Avg)': str(1.5 / 4), }, ], ignore_other_columns=True, @@ -1835,12 +1850,12 @@ class TestGradeReport(TestReportMixin, InstructorTaskModuleTestCase): def test_fast_generation(self, create_non_zero_grade): if create_non_zero_grade: self.submit_student_answer(self.student.username, 'Problem1', ['Option 1']) - with patch('lms.djangoapps.instructor_task.tasks_helper.runner._get_current_task'): - with patch('lms.djangoapps.grades.course_data.get_course_blocks') as mock_course_blocks: - with patch('lms.djangoapps.grades.subsection_grade.get_score') as mock_get_score: - CourseGradeReport.generate(None, None, self.course.id, None, 'graded') - assert not mock_get_score.called - assert not mock_course_blocks.called + with patch('lms.djangoapps.instructor_task.tasks_helper.runner._get_current_task'), \ + patch('lms.djangoapps.grades.course_data.get_course_blocks') as mock_course_blocks, \ + patch('lms.djangoapps.grades.subsection_grade.get_score') as mock_get_score: + CourseGradeReport.generate(None, None, self.course.id, None, 'graded') + assert not mock_course_blocks.called + assert not mock_get_score.called @ddt.ddt