From 1c3f403f38e59836a44f04868426243682a2cddd Mon Sep 17 00:00:00 2001 From: hajorg Date: Tue, 26 Mar 2024 11:11:13 +0100 Subject: [PATCH] fix: update and cleanup tests --- lms/djangoapps/grades/models.py | 2 +- lms/djangoapps/grades/models_api.py | 8 +-- lms/djangoapps/grades/tests/test_api.py | 59 ++++++++++++---------- lms/djangoapps/grades/tests/test_models.py | 49 ++++++++++-------- 4 files changed, 63 insertions(+), 55 deletions(-) diff --git a/lms/djangoapps/grades/models.py b/lms/djangoapps/grades/models.py index 1fface394d..a5608eb39a 100644 --- a/lms/djangoapps/grades/models.py +++ b/lms/djangoapps/grades/models.py @@ -537,7 +537,7 @@ class PersistentSubsectionGrade(TimeStampedModel): @classmethod def delete_subsection_grades_for_learner(cls, user_id, course_key): """ - Clears Subsection grade override for a learner in a course + Clears Subsection grades and overrides for a learner in a course Arguments: user_id: The user associated with the desired grade course_id: The id of the course associated with the desired grade diff --git a/lms/djangoapps/grades/models_api.py b/lms/djangoapps/grades/models_api.py index 5eef8d2369..0466fb2a56 100644 --- a/lms/djangoapps/grades/models_api.py +++ b/lms/djangoapps/grades/models_api.py @@ -107,9 +107,5 @@ def clear_user_course_grades(user_id, course_key): Given a user_id and course_key, clears persistent grades for a learner in a course """ with transaction.atomic(): - try: - _PersistentSubsectionGrade.delete_subsection_grades_for_learner(user_id, course_key) - _PersistentCourseGrade.delete_course_grade_for_learner(course_key, user_id) - return 'Grades deleted Successfully' - except Exception as e: # pylint: disable=broad-except - return f'Error deleting grades: {str(e)}' + _PersistentSubsectionGrade.delete_subsection_grades_for_learner(user_id, course_key) + _PersistentCourseGrade.delete_course_grade_for_learner(course_key, user_id) diff --git a/lms/djangoapps/grades/tests/test_api.py b/lms/djangoapps/grades/tests/test_api.py index 8848bb63c9..e42e9ea56f 100644 --- a/lms/djangoapps/grades/tests/test_api.py +++ b/lms/djangoapps/grades/tests/test_api.py @@ -1,7 +1,7 @@ """ Tests calling the grades api directly """ -from unittest.mock import patch, Mock +from unittest.mock import patch import ddt @@ -44,7 +44,7 @@ class OverrideSubsectionGradeTests(ModuleStoreTestCase): def setUp(self): super().setUp() - self.course = CourseFactory.create(org='edX', number='DemoX', display_name='Demo_Course', run='Spring2019') + self.course = CourseFactory.create() self.subsection = BlockFactory.create(parent=self.course, category="sequential", display_name="Subsection") self.grade = PersistentSubsectionGrade.update_or_create_grade( user_id=self.user.id, @@ -130,8 +130,8 @@ class ClearGradeTests(ModuleStoreTestCase): def setUp(self): super().setUp() - self.course = CourseFactory.create(org='edX', number='DemoX', display_name='Demo_Course', run='Spring2019') - self.subsection = BlockFactory.create(parent=self.course, category="sequential", display_name="Subsection") + self.course = CourseFactory.create() + self.subsection = BlockFactory.create(parent=self.course) self.grade = PersistentSubsectionGrade.update_or_create_grade( user_id=self.user.id, course_id=self.course.id, @@ -187,35 +187,52 @@ class ClearGradeTests(ModuleStoreTestCase): self.subsection.location ) - def test_clear_wrong_user_course_grades(self): - wrong_user = UserFactory() + def _create_and_get_user_grades(self, user_id): + """ Creates grades for a user and override object """ api.override_subsection_grade( - self.user.id, + user_id, self.course.id, self.subsection.location, overrider=self.overriding_user, earned_graded=0.0, comment='Test Override Comment', ) - override_obj = api.get_subsection_grade_override( - self.user.id, + return api.get_subsection_grade_override( + user_id, self.course.id, self.subsection.location ) - course_grade = PersistentCourseGrade.read(self.user.id, self.course.id) - self.assertIsNotNone(course_grade) - self.assertIsNotNone(override_obj) - api.clear_user_course_grades(wrong_user.id, self.course.id) + def test_clear_other_user_course_grades(self): + """ + Make sure it deletes grades for other_user and not self.user + """ + # Create grades for 2 users + other_user = UserFactory() + user_override_obj = self._create_and_get_user_grades(self.user.id) + other_user_override_obj = self._create_and_get_user_grades(other_user.id) + # fetch and assert grades are available for both users + user_course_grade = PersistentCourseGrade.read(self.user.id, self.course.id) + other_user_course_grade = PersistentCourseGrade.read(self.user.id, self.course.id) + self.assertIsNotNone(user_course_grade) + self.assertIsNotNone(user_override_obj) + self.assertIsNotNone(other_user_override_obj) + self.assertIsNotNone(other_user_course_grade) + + api.clear_user_course_grades(other_user.id, self.course.id) + + # assert grades after deletion for other_user after_clear_override_obj = api.get_subsection_grade_override( self.user.id, self.course.id, self.subsection.location ) - after_clear_course_grade = PersistentCourseGrade.read(self.user.id, self.course.id) + after_clear_user_course_grade = PersistentCourseGrade.read(self.user.id, self.course.id) + with self.assertRaises(PersistentCourseGrade.DoesNotExist): + PersistentCourseGrade.read(other_user.id, self.course.id) self.assertIsNotNone(after_clear_override_obj) - self.assertIsNotNone(after_clear_course_grade) + self.assertIsNotNone(after_clear_user_course_grade) @patch('lms.djangoapps.grades.models_api._PersistentSubsectionGrade') @patch('lms.djangoapps.grades.models_api._PersistentCourseGrade') @@ -223,15 +240,3 @@ class ClearGradeTests(ModuleStoreTestCase): api.clear_user_course_grades(self.user.id, self.course.id) mock_course_grade.delete_course_grade_for_learner.assert_called_with(self.course.id, self.user.id) mock_subsection_grade.delete_subsection_grades_for_learner.assert_called_with(self.user.id, self.course.id) - - @patch('lms.djangoapps.grades.models_api._PersistentSubsectionGrade') - @patch('lms.djangoapps.grades.models_api._PersistentCourseGrade') - def test_assert_clear_grade_exception(self, mock_course_grade, mock_subsection_grade): - with patch( - 'lms.djangoapps.grades.models_api._PersistentSubsectionGradeOverride', - Mock(side_effect=Exception) - ) as mock_override: - api.clear_user_course_grades(self.user.id, self.course.id) - self.assertRaises(Exception, mock_override) - self.assertFalse(mock_course_grade.called) - self.assertFalse(mock_subsection_grade.called) diff --git a/lms/djangoapps/grades/tests/test_models.py b/lms/djangoapps/grades/tests/test_models.py index 51278706b9..acdb20c5f1 100644 --- a/lms/djangoapps/grades/tests/test_models.py +++ b/lms/djangoapps/grades/tests/test_models.py @@ -352,6 +352,9 @@ class PersistentSubsectionGradeTest(GradesModelTestCase): self.user.id, self.course_key ) self.assertEqual(deleted, 1) + self.assertFalse(PersistentSubsectionGrade.objects.filter( + user_id=self.user.id, course_id=self.course_key).exists() + ) def test_clear_subsection_grade_override(self): grade = PersistentSubsectionGrade.update_or_create_grade(**self.params) @@ -510,36 +513,40 @@ class PersistentCourseGradesTest(GradesModelTestCase): } ) - def test_clear_grade(self): - another_params = { - "user_id": 123456, - "course_id": self.course_key, - "course_version": "JoeMcEwing", - "course_edited_timestamp": datetime( - year=2016, - month=8, - day=1, - hour=18, - minute=53, - second=24, - microsecond=354741, - tzinfo=pytz.UTC, - ), - "percent_grade": 77.8, - "letter_grade": "Great job", - "passed": True, + def test_clear_course_grade(self): + # create params for another user and another course + other_user = UserFactory.create() + other_user_params = { + **self.params, + 'user_id': other_user.id } - UserFactory(id=another_params['user_id']) + other_course_key = CourseLocator( + org='some_org', + course='some_other_course', + run='some_run' + ) + user_other_course_params = { + **self.params, + 'course_id': other_course_key + } + # create course grades based on different params PersistentCourseGrade.update_or_create(**self.params) - PersistentCourseGrade.update_or_create(**another_params) + PersistentCourseGrade.update_or_create(**other_user_params) + PersistentCourseGrade.update_or_create(**user_other_course_params) PersistentCourseGrade.delete_course_grade_for_learner( self.course_key, self.params['user_id'] ) + + # assert after deleteing grade for a single user and course with self.assertRaises(PersistentCourseGrade.DoesNotExist): PersistentCourseGrade.read(self.params['user_id'], self.course_key) - another_user_grade = PersistentCourseGrade.read(another_params['user_id'], self.course_key) + another_user_grade = PersistentCourseGrade.read(other_user_params['user_id'], self.course_key) self.assertIsNotNone(another_user_grade) + + self.assertTrue(PersistentCourseGrade.objects.filter( + user_id=self.params['user_id'], course_id=other_course_key).exists() + )