From 05087bfac2f7b814175d23219cbbf13d49232180 Mon Sep 17 00:00:00 2001 From: Nimisha Asthagiri Date: Fri, 6 Jan 2017 00:42:28 -0500 Subject: [PATCH] Management command to Reset Grades TNL-6251 --- .../management/commands/reset_grades.py | 150 +++++++++ .../commands/tests/test_reset_grades.py | 296 ++++++++++++++++++ lms/djangoapps/grades/models.py | 36 ++- 3 files changed, 480 insertions(+), 2 deletions(-) create mode 100644 lms/djangoapps/grades/management/commands/reset_grades.py create mode 100644 lms/djangoapps/grades/management/commands/tests/test_reset_grades.py diff --git a/lms/djangoapps/grades/management/commands/reset_grades.py b/lms/djangoapps/grades/management/commands/reset_grades.py new file mode 100644 index 0000000000..43e9776b6c --- /dev/null +++ b/lms/djangoapps/grades/management/commands/reset_grades.py @@ -0,0 +1,150 @@ +""" +Reset persistent grades for learners. +""" +from datetime import datetime +import logging +from textwrap import dedent + +from django.core.management.base import BaseCommand, CommandError +from django.db.models import Count + +from opaque_keys import InvalidKeyError +from opaque_keys.edx.keys import CourseKey + +from lms.djangoapps.grades.models import PersistentSubsectionGrade, PersistentCourseGrade + + +log = logging.getLogger(__name__) + + +DATE_FORMAT = "%Y-%m-%d %H:%M" + + +class Command(BaseCommand): + """ + Reset persistent grades for learners. + """ + help = dedent(__doc__).strip() + + def add_arguments(self, parser): + """ + Add arguments to the command parser. + """ + parser.add_argument( + '--dry_run', + action='store_true', + default=False, + dest='dry_run', + help="Output what we're going to do, but don't actually do it. To actually delete, use --delete instead." + ) + parser.add_argument( + '--delete', + action='store_true', + default=False, + dest='delete', + help="Actually perform the deletions of the course. For a Dry Run, use --dry_run instead." + ) + parser.add_argument( + '--courses', + dest='courses', + nargs='+', + help='Reset persistent grades for the list of courses provided.', + ) + parser.add_argument( + '--all_courses', + action='store_true', + dest='all_courses', + default=False, + help='Reset persistent grades for all courses.', + ) + parser.add_argument( + '--modified_start', + dest='modified_start', + help='Starting range for modified date (inclusive): e.g. "2016-08-23 16:43"', + ) + parser.add_argument( + '--modified_end', + dest='modified_end', + help='Ending range for modified date (inclusive): e.g. "2016-12-23 16:43"', + ) + + def handle(self, *args, **options): + course_keys = None + modified_start = None + modified_end = None + + run_mode = self._get_mutually_exclusive_option(options, 'delete', 'dry_run') + courses_mode = self._get_mutually_exclusive_option(options, 'courses', 'all_courses') + + if options.get('modified_start'): + modified_start = datetime.strptime(options['modified_start'], DATE_FORMAT) + + if options.get('modified_end'): + if not modified_start: + raise CommandError('Optional value for modified_end provided without a value for modified_start.') + modified_end = datetime.strptime(options['modified_end'], DATE_FORMAT) + + if courses_mode == 'courses': + try: + course_keys = [CourseKey.from_string(course_key_string) for course_key_string in options['courses']] + except InvalidKeyError as error: + raise CommandError('Invalid key specified: {}'.format(error.message)) + + log.info("reset_grade: Started in %s mode!", run_mode) + + operation = self._query_grades if run_mode == 'dry_run' else self._delete_grades + + operation(PersistentSubsectionGrade, course_keys, modified_start, modified_end) + operation(PersistentCourseGrade, course_keys, modified_start, modified_end) + + log.info("reset_grade: Finished in %s mode!", run_mode) + + def _delete_grades(self, grade_model_class, *args, **kwargs): + """ + Deletes the requested grades in the given model, filtered by the provided args and kwargs. + """ + grades_query_set = grade_model_class.query_grades(*args, **kwargs) + num_rows_to_delete = grades_query_set.count() + + log.info("reset_grade: Deleting %s: %d row(s).", grade_model_class.__name__, num_rows_to_delete) + + grade_model_class.delete_grades(*args, **kwargs) + + log.info("reset_grade: Deleted %s: %d row(s).", grade_model_class.__name__, num_rows_to_delete) + + def _query_grades(self, grade_model_class, *args, **kwargs): + """ + Queries the requested grades in the given model, filtered by the provided args and kwargs. + """ + total_for_all_courses = 0 + + grades_query_set = grade_model_class.query_grades(*args, **kwargs) + grades_stats = grades_query_set.values('course_id').order_by().annotate(total=Count('course_id')) + + for stat in grades_stats: + total_for_all_courses += stat['total'] + log.info( + "reset_grade: Would delete %s for COURSE %s: %d row(s).", + grade_model_class.__name__, + stat['course_id'], + stat['total'], + ) + + log.info( + "reset_grade: Would delete %s in TOTAL: %d row(s).", + grade_model_class.__name__, + total_for_all_courses, + ) + + def _get_mutually_exclusive_option(self, options, option_1, option_2): + """ + Validates that exactly one of the 2 given options is specified. + Returns the name of the found option. + """ + if not options.get(option_1) and not options.get(option_2): + raise CommandError('Either --{} or --{} must be specified.'.format(option_1, option_2)) + + if options.get(option_1) and options.get(option_2): + raise CommandError('Both --{} and --{} cannot be specified.'.format(option_1, option_2)) + + return option_1 if options.get(option_1) else option_2 diff --git a/lms/djangoapps/grades/management/commands/tests/test_reset_grades.py b/lms/djangoapps/grades/management/commands/tests/test_reset_grades.py new file mode 100644 index 0000000000..647b9c8e31 --- /dev/null +++ b/lms/djangoapps/grades/management/commands/tests/test_reset_grades.py @@ -0,0 +1,296 @@ +""" +Tests for reset_grades management command. +""" +from datetime import datetime, timedelta +import ddt +from django.core.management.base import CommandError +from django.test import TestCase +from freezegun import freeze_time +from mock import patch, MagicMock + +from lms.djangoapps.grades.management.commands import reset_grades +from lms.djangoapps.grades.models import PersistentSubsectionGrade, PersistentCourseGrade +from opaque_keys.edx.locator import CourseLocator, BlockUsageLocator + + +@ddt.ddt +class TestResetGrades(TestCase): + """ + Tests generate course blocks management command. + """ + num_users = 3 + num_courses = 5 + num_subsections = 7 + + def setUp(self): + super(TestResetGrades, self).setUp() + self.command = reset_grades.Command() + + self.user_ids = [user_id for user_id in range(self.num_users)] + + self.course_keys = [] + for course_index in range(self.num_courses): + self.course_keys.append( + CourseLocator( + org='some_org', + course='some_course', + run=unicode(course_index), + ) + ) + + self.subsection_keys_by_course = {} + for course_key in self.course_keys: + subsection_keys_in_course = [] + for subsection_index in range(self.num_subsections): + subsection_keys_in_course.append( + BlockUsageLocator( + course_key=course_key, + block_type='sequential', + block_id=unicode(subsection_index), + ) + ) + self.subsection_keys_by_course[course_key] = subsection_keys_in_course + + def _update_or_create_grades(self, courses_keys=None): + """ + Creates grades for all courses and subsections. + """ + if courses_keys is None: + courses_keys = self.course_keys + + course_grade_params = { + "course_version": "JoeMcEwing", + "course_edited_timestamp": datetime( + year=2016, + month=8, + day=1, + hour=18, + minute=53, + second=24, + microsecond=354741, + ), + "percent_grade": 77.7, + "letter_grade": "Great job", + "passed": True, + } + subsection_grade_params = { + "course_version": "deadbeef", + "subtree_edited_timestamp": "2016-08-01 18:53:24.354741", + "earned_all": 6.0, + "possible_all": 12.0, + "earned_graded": 6.0, + "possible_graded": 8.0, + "visible_blocks": MagicMock(), + "attempted": True, + } + + for course_key in courses_keys: + for user_id in self.user_ids: + course_grade_params['user_id'] = user_id + course_grade_params['course_id'] = course_key + PersistentCourseGrade.update_or_create_course_grade(**course_grade_params) + for subsection_key in self.subsection_keys_by_course[course_key]: + subsection_grade_params['user_id'] = user_id + subsection_grade_params['usage_key'] = subsection_key + PersistentSubsectionGrade.update_or_create_grade(**subsection_grade_params) + + def _assert_grades_exist_for_courses(self, course_keys): + """ + Assert grades for given courses exist. + """ + for course_key in course_keys: + self.assertIsNotNone(PersistentCourseGrade.read_course_grade(self.user_ids[0], course_key)) + for subsection_key in self.subsection_keys_by_course[course_key]: + self.assertIsNotNone(PersistentSubsectionGrade.read_grade(self.user_ids[0], subsection_key)) + + def _assert_grades_absent_for_courses(self, course_keys): + """ + Assert grades for given courses do not exist. + """ + for course_key in course_keys: + with self.assertRaises(PersistentCourseGrade.DoesNotExist): + PersistentCourseGrade.read_course_grade(self.user_ids[0], course_key) + + for subsection_key in self.subsection_keys_by_course[course_key]: + with self.assertRaises(PersistentSubsectionGrade.DoesNotExist): + PersistentSubsectionGrade.read_grade(self.user_ids[0], subsection_key) + + def _assert_stat_logged(self, mock_log, num_rows, grade_model_class, message_substring, log_offset): + self.assertIn('reset_grade: ' + message_substring, mock_log.info.call_args_list[log_offset][0][0]) + self.assertEqual(grade_model_class.__name__, mock_log.info.call_args_list[log_offset][0][1]) + self.assertEqual(num_rows, mock_log.info.call_args_list[log_offset][0][2]) + + def _assert_course_delete_stat_logged(self, mock_log, num_rows): + self._assert_stat_logged(mock_log, num_rows, PersistentCourseGrade, 'Deleted', log_offset=4) + + def _assert_subsection_delete_stat_logged(self, mock_log, num_rows): + self._assert_stat_logged(mock_log, num_rows, PersistentSubsectionGrade, 'Deleted', log_offset=2) + + def _assert_course_query_stat_logged(self, mock_log, num_rows, num_courses=None): + if num_courses is None: + num_courses = self.num_courses + log_offset = num_courses + 1 + num_courses + 1 + self._assert_stat_logged(mock_log, num_rows, PersistentCourseGrade, 'Would delete', log_offset) + + def _assert_subsection_query_stat_logged(self, mock_log, num_rows, num_courses=None): + if num_courses is None: + num_courses = self.num_courses + log_offset = num_courses + 1 + self._assert_stat_logged(mock_log, num_rows, PersistentSubsectionGrade, 'Would delete', log_offset) + + def _date_from_now(self, days=None): + return datetime.now() + timedelta(days=days) + + def _date_str_from_now(self, days=None): + future_date = self._date_from_now(days=days) + return future_date.strftime(reset_grades.DATE_FORMAT) + + @patch('lms.djangoapps.grades.management.commands.reset_grades.log') + def test_reset_all_courses(self, mock_log): + self._update_or_create_grades() + self._assert_grades_exist_for_courses(self.course_keys) + + with self.assertNumQueries(4): + self.command.handle(delete=True, all_courses=True) + + self._assert_grades_absent_for_courses(self.course_keys) + self._assert_subsection_delete_stat_logged( + mock_log, + num_rows=self.num_users * self.num_courses * self.num_subsections, + ) + self._assert_course_delete_stat_logged( + mock_log, + num_rows=self.num_users * self.num_courses, + ) + + @patch('lms.djangoapps.grades.management.commands.reset_grades.log') + @ddt.data(1, 2, 3) + def test_reset_some_courses(self, num_courses_to_reset, mock_log): + self._update_or_create_grades() + self._assert_grades_exist_for_courses(self.course_keys) + + with self.assertNumQueries(4): + self.command.handle( + delete=True, + courses=[unicode(course_key) for course_key in self.course_keys[:num_courses_to_reset]] + ) + + self._assert_grades_absent_for_courses(self.course_keys[:num_courses_to_reset]) + self._assert_grades_exist_for_courses(self.course_keys[num_courses_to_reset:]) + self._assert_subsection_delete_stat_logged( + mock_log, + num_rows=self.num_users * num_courses_to_reset * self.num_subsections, + ) + self._assert_course_delete_stat_logged( + mock_log, + num_rows=self.num_users * num_courses_to_reset, + ) + + def test_reset_by_modified_start_date(self): + self._update_or_create_grades() + self._assert_grades_exist_for_courses(self.course_keys) + + num_courses_with_updated_grades = 2 + with freeze_time(self._date_from_now(days=4)): + self._update_or_create_grades(self.course_keys[:num_courses_with_updated_grades]) + + with self.assertNumQueries(4): + self.command.handle(delete=True, modified_start=self._date_str_from_now(days=2), all_courses=True) + + self._assert_grades_absent_for_courses(self.course_keys[:num_courses_with_updated_grades]) + self._assert_grades_exist_for_courses(self.course_keys[num_courses_with_updated_grades:]) + + def test_reset_by_modified_start_end_date(self): + self._update_or_create_grades() + self._assert_grades_exist_for_courses(self.course_keys) + + with freeze_time(self._date_from_now(days=3)): + self._update_or_create_grades(self.course_keys[:2]) + with freeze_time(self._date_from_now(days=5)): + self._update_or_create_grades(self.course_keys[2:4]) + + with self.assertNumQueries(4): + self.command.handle( + delete=True, + modified_start=self._date_str_from_now(days=2), + modified_end=self._date_str_from_now(days=4), + all_courses=True, + ) + + # Only grades for courses modified within the 2->4 days + # should be deleted. + self._assert_grades_absent_for_courses(self.course_keys[:2]) + self._assert_grades_exist_for_courses(self.course_keys[2:]) + + @patch('lms.djangoapps.grades.management.commands.reset_grades.log') + def test_dry_run_all_courses(self, mock_log): + self._update_or_create_grades() + self._assert_grades_exist_for_courses(self.course_keys) + + with self.assertNumQueries(2): + self.command.handle(dry_run=True, all_courses=True) + + self._assert_grades_exist_for_courses(self.course_keys) + self._assert_subsection_query_stat_logged( + mock_log, + num_rows=self.num_users * self.num_courses * self.num_subsections, + ) + self._assert_course_query_stat_logged( + mock_log, + num_rows=self.num_users * self.num_courses, + ) + + @patch('lms.djangoapps.grades.management.commands.reset_grades.log') + @ddt.data(1, 2, 3) + def test_dry_run_some_courses(self, num_courses_to_query, mock_log): + self._update_or_create_grades() + self._assert_grades_exist_for_courses(self.course_keys) + + with self.assertNumQueries(2): + self.command.handle( + dry_run=True, + courses=[unicode(course_key) for course_key in self.course_keys[:num_courses_to_query]] + ) + + self._assert_grades_exist_for_courses(self.course_keys) + self._assert_subsection_query_stat_logged( + mock_log, + num_rows=self.num_users * num_courses_to_query * self.num_subsections, + num_courses=num_courses_to_query, + ) + self._assert_course_query_stat_logged( + mock_log, + num_rows=self.num_users * num_courses_to_query, + num_courses=num_courses_to_query, + ) + + @patch('lms.djangoapps.grades.management.commands.reset_grades.log') + def test_reset_no_existing_grades(self, mock_log): + self._assert_grades_absent_for_courses(self.course_keys) + + with self.assertNumQueries(4): + self.command.handle(delete=True, all_courses=True) + + self._assert_grades_absent_for_courses(self.course_keys) + self._assert_subsection_delete_stat_logged(mock_log, num_rows=0) + self._assert_course_delete_stat_logged(mock_log, num_rows=0) + + def test_invalid_key(self): + with self.assertRaisesRegexp(CommandError, 'Invalid key specified.*invalid/key'): + self.command.handle(dry_run=True, courses=['invalid/key']) + + def test_no_run_mode(self): + with self.assertRaisesMessage(CommandError, 'Either --delete or --dry_run must be specified.'): + self.command.handle(all_courses=True) + + def test_both_run_modes(self): + with self.assertRaisesMessage(CommandError, 'Both --delete and --dry_run cannot be specified.'): + self.command.handle(all_courses=True, dry_run=True, delete=True) + + def test_no_course_mode(self): + with self.assertRaisesMessage(CommandError, 'Either --courses or --all_courses must be specified.'): + self.command.handle(dry_run=True) + + def test_both_course_modes(self): + with self.assertRaisesMessage(CommandError, 'Both --courses and --all_courses cannot be specified.'): + self.command.handle(dry_run=True, all_courses=True, courses=['some/course/key']) diff --git a/lms/djangoapps/grades/models.py b/lms/djangoapps/grades/models.py index fcf8c6da23..e95cda16e0 100644 --- a/lms/djangoapps/grades/models.py +++ b/lms/djangoapps/grades/models.py @@ -38,6 +38,38 @@ BLOCK_RECORD_LIST_VERSION = 1 BlockRecord = namedtuple('BlockRecord', ['locator', 'weight', 'raw_possible', 'graded']) +class DeleteGradesMixin(object): + """ + A Mixin class that provides functionality to delete grades. + """ + + @classmethod + def query_grades(cls, course_ids=None, modified_start=None, modified_end=None): + """ + Queries all the grades in the table, filtered by the provided arguments. + """ + kwargs = {} + + if course_ids: + kwargs['course_id__in'] = [course_id for course_id in course_ids] + + if modified_start: + if modified_end: + kwargs['modified__range'] = (modified_start, modified_end) + else: + kwargs['modified__gt'] = modified_start + + return cls.objects.filter(**kwargs) + + @classmethod + def delete_grades(cls, *args, **kwargs): + """ + Deletes all the grades in the table, filtered by the provided arguments. + """ + query = cls.query_grades(*args, **kwargs) + query.delete() + + class BlockRecordList(tuple): """ An immutable ordered list of BlockRecord objects. @@ -208,7 +240,7 @@ class VisibleBlocks(models.Model): cls.bulk_create(non_existent_brls) -class PersistentSubsectionGrade(TimeStampedModel): +class PersistentSubsectionGrade(DeleteGradesMixin, TimeStampedModel): """ A django model tracking persistent grades at the subsection level. """ @@ -458,7 +490,7 @@ class PersistentSubsectionGrade(TimeStampedModel): ) -class PersistentCourseGrade(TimeStampedModel): +class PersistentCourseGrade(DeleteGradesMixin, TimeStampedModel): """ A django model tracking persistent course grades. """