From ddafbb0d2937d7c481ed1530bc5495ea2131fe46 Mon Sep 17 00:00:00 2001 From: Nimisha Asthagiri Date: Mon, 1 Oct 2018 09:55:51 -0400 Subject: [PATCH] POC: Stop-gap implementation to freeze grades. --- lms/djangoapps/grades/config/waffle.py | 8 +- lms/djangoapps/grades/tasks.py | 42 ++++- lms/djangoapps/grades/tests/test_tasks.py | 188 ++++++++++++++++++++-- 3 files changed, 220 insertions(+), 18 deletions(-) diff --git a/lms/djangoapps/grades/config/waffle.py b/lms/djangoapps/grades/config/waffle.py index c53c63eb73..ea7ae34923 100644 --- a/lms/djangoapps/grades/config/waffle.py +++ b/lms/djangoapps/grades/config/waffle.py @@ -13,6 +13,7 @@ DISABLE_REGRADE_ON_POLICY_CHANGE = u'disable_regrade_on_policy_change' # Course Flags REJECTED_EXAM_OVERRIDES_GRADE = u'rejected_exam_overrides_grade' +ENFORCE_FREEZE_GRADE_AFTER_COURSE_END = u'enforce_freeze_grade_after_course_end' def waffle(): @@ -32,6 +33,11 @@ def waffle_flags(): REJECTED_EXAM_OVERRIDES_GRADE: CourseWaffleFlag( namespace, REJECTED_EXAM_OVERRIDES_GRADE, - flag_undefined_default=True + flag_undefined_default=True, + ), + ENFORCE_FREEZE_GRADE_AFTER_COURSE_END: CourseWaffleFlag( + namespace, + ENFORCE_FREEZE_GRADE_AFTER_COURSE_END, + flag_undefined_default=False, ) } diff --git a/lms/djangoapps/grades/tasks.py b/lms/djangoapps/grades/tasks.py index 3542acb4fc..7e09ad7d85 100644 --- a/lms/djangoapps/grades/tasks.py +++ b/lms/djangoapps/grades/tasks.py @@ -2,29 +2,32 @@ This module contains tasks for asynchronous execution of grade updates. """ +from datetime import timedelta from logging import getLogger import six from celery import task from celery_utils.persist_on_failure import LoggedPersistOnFailureTask -from courseware.model_data import get_score from django.conf import settings from django.contrib.auth.models import User from django.core.exceptions import ValidationError from django.db.utils import DatabaseError +from django.utils import timezone from edx_django_utils.monitoring import set_custom_metric, set_custom_metrics_for_course_key - -from lms.djangoapps.course_blocks.api import get_course_blocks -from lms.djangoapps.grades.config.models import ComputeGradesSetting from opaque_keys.edx.keys import CourseKey, UsageKey from opaque_keys.edx.locator import CourseLocator -from student.models import CourseEnrollment from submissions import api as sub_api + +from courseware.model_data import get_score +from lms.djangoapps.course_blocks.api import get_course_blocks +from lms.djangoapps.grades.config.models import ComputeGradesSetting +from openedx.core.djangoapps.content.course_overviews.models import CourseOverview +from student.models import CourseEnrollment from track.event_transaction_utils import set_event_transaction_id, set_event_transaction_type from util.date_utils import from_timestamp from xmodule.modulestore.django import modulestore -from .config.waffle import DISABLE_REGRADE_ON_POLICY_CHANGE, waffle +from .config.waffle import DISABLE_REGRADE_ON_POLICY_CHANGE, ENFORCE_FREEZE_GRADE_AFTER_COURSE_END, waffle, waffle_flags from .constants import ScoreDatabaseTableEnum from .course_grade_factory import CourseGradeFactory from .exceptions import DatabaseNotReadyError @@ -57,6 +60,9 @@ def compute_all_grades_for_course(**kwargs): log.debug('Grades: ignoring policy change regrade due to waffle switch') else: course_key = CourseKey.from_string(kwargs.pop('course_key')) + if _are_grades_frozen(course_key): + log.info("Attempted compute_all_grades_for_course for course '%s', but grades are frozen.", course_key) + return for course_key_string, offset, batch_size in _course_task_args(course_key=course_key, **kwargs): kwargs.update({ 'course_key': course_key_string, @@ -109,6 +115,10 @@ def compute_grades_for_course(course_key, offset, batch_size, **kwargs): # pyli offset. """ course_key = CourseKey.from_string(course_key) + if _are_grades_frozen(course_key): + log.info("Attempted compute_grades_for_course for course '%s', but grades are frozen.", course_key) + return + enrollments = CourseEnrollment.objects.filter(course_id=course_key).order_by('created') student_iter = (enrollment.user for enrollment in enrollments[offset:offset + batch_size]) for result in CourseGradeFactory().iter(users=student_iter, course_key=course_key, force_update=True): @@ -138,6 +148,12 @@ def recalculate_course_and_subsection_grades_for_user(self, **kwargs): # pylint user = User.objects.get(id=user_id) course_key = CourseKey.from_string(course_key_str) + if _are_grades_frozen(course_key): + log.info( + "Attempted recalculate_course_and_subsection_grades_for_user for course '%s', but grades are frozen.", + course_key, + ) + return previous_course_grade = CourseGradeFactory().read(user, course_key=course_key) if previous_course_grade and previous_course_grade.attempted: @@ -189,6 +205,10 @@ def _recalculate_subsection_grade(self, **kwargs): """ try: course_key = CourseLocator.from_string(kwargs['course_id']) + if _are_grades_frozen(course_key): + log.info("Attempted _recalculate_subsection_grade for course '%s', but grades are frozen.", course_key) + return + scored_block_usage_key = UsageKey.from_string(kwargs['usage_id']).replace(course_key=course_key) set_custom_metrics_for_course_key(course_key) @@ -328,3 +348,13 @@ def _course_task_args(course_key, **kwargs): for offset in six.moves.range(0, enrollment_count, batch_size): yield (six.text_type(course_key), offset, batch_size) + + +def _are_grades_frozen(course_key): + """ Returns whether grades are frozen for the given course. """ + if waffle_flags()[ENFORCE_FREEZE_GRADE_AFTER_COURSE_END].is_enabled(course_key): + course = CourseOverview.get_from_id(course_key) + if course.end: + freeze_grade_date = course.end + timedelta(30) + now = timezone.now() + return now > freeze_grade_date diff --git a/lms/djangoapps/grades/tests/test_tasks.py b/lms/djangoapps/grades/tests/test_tasks.py index a938e5e2e5..75a023c0fb 100644 --- a/lms/djangoapps/grades/tests/test_tasks.py +++ b/lms/djangoapps/grades/tests/test_tasks.py @@ -12,10 +12,12 @@ import pytz import six from django.conf import settings from django.db.utils import IntegrityError +from django.utils import timezone from mock import MagicMock, patch from lms.djangoapps.grades import tasks from lms.djangoapps.grades.config.models import PersistentGradesEnabledFlag +from lms.djangoapps.grades.config.waffle import waffle_flags, ENFORCE_FREEZE_GRADE_AFTER_COURSE_END from lms.djangoapps.grades.constants import ScoreDatabaseTableEnum from lms.djangoapps.grades.models import PersistentCourseGrade, PersistentSubsectionGrade from lms.djangoapps.grades.services import GradesService @@ -23,10 +25,13 @@ from lms.djangoapps.grades.signals.signals import PROBLEM_WEIGHTED_SCORE_CHANGED from lms.djangoapps.grades.tasks import ( RECALCULATE_GRADE_DELAY_SECONDS, _course_task_args, + compute_all_grades_for_course, + compute_grades_for_course, compute_grades_for_course_v2, recalculate_subsection_grade_v3 ) from openedx.core.djangoapps.content.block_structure.exceptions import BlockStructureNotFound +from openedx.core.djangoapps.waffle_utils.testutils import override_waffle_flag from student.models import CourseEnrollment, anonymous_id_for_user from student.tests.factories import UserFactory from track.event_transaction_utils import create_new_event_transaction_id, get_event_transaction_id @@ -36,6 +41,7 @@ from xmodule.modulestore.django import modulestore from xmodule.modulestore.tests.django_utils import ModuleStoreTestCase from xmodule.modulestore.tests.factories import CourseFactory, ItemFactory, check_mongo_calls + from .utils import mock_get_score @@ -55,7 +61,7 @@ class HasCourseWithProblemsMixin(object): """ Mixin to provide tests with a sample course with graded subsections """ - def set_up_course(self, enable_persistent_grades=True, create_multiple_subsections=False): + def set_up_course(self, enable_persistent_grades=True, create_multiple_subsections=False, course_end=None): """ Configures the course for this test. """ @@ -63,6 +69,7 @@ class HasCourseWithProblemsMixin(object): org='edx', name='course', run='run', + end=course_end ) if not enable_persistent_grades: PersistentGradesEnabledFlag.objects.create(enabled=False) @@ -168,10 +175,10 @@ class RecalculateSubsectionGradeTest(HasCourseWithProblemsMixin, ModuleStoreTest self.assertEquals(mock_block_structure_create.call_count, 1) @ddt.data( - (ModuleStoreEnum.Type.mongo, 1, 28, True), - (ModuleStoreEnum.Type.mongo, 1, 28, False), - (ModuleStoreEnum.Type.split, 3, 28, True), - (ModuleStoreEnum.Type.split, 3, 28, False), + (ModuleStoreEnum.Type.mongo, 1, 30, True), + (ModuleStoreEnum.Type.mongo, 1, 30, False), + (ModuleStoreEnum.Type.split, 3, 30, True), + (ModuleStoreEnum.Type.split, 3, 30, False), ) @ddt.unpack def test_query_counts(self, default_store, num_mongo_calls, num_sql_calls, create_multiple_subsections): @@ -183,8 +190,8 @@ class RecalculateSubsectionGradeTest(HasCourseWithProblemsMixin, ModuleStoreTest self._apply_recalculate_subsection_grade() @ddt.data( - (ModuleStoreEnum.Type.mongo, 1, 28), - (ModuleStoreEnum.Type.split, 3, 28), + (ModuleStoreEnum.Type.mongo, 1, 30), + (ModuleStoreEnum.Type.split, 3, 30), ) @ddt.unpack def test_query_counts_dont_change_with_more_content(self, default_store, num_mongo_calls, num_sql_calls): @@ -229,8 +236,8 @@ class RecalculateSubsectionGradeTest(HasCourseWithProblemsMixin, ModuleStoreTest ) @ddt.data( - (ModuleStoreEnum.Type.mongo, 1, 12), - (ModuleStoreEnum.Type.split, 3, 12), + (ModuleStoreEnum.Type.mongo, 1, 14), + (ModuleStoreEnum.Type.split, 3, 14), ) @ddt.unpack def test_persistent_grades_not_enabled_on_course(self, default_store, num_mongo_queries, num_sql_queries): @@ -244,8 +251,8 @@ class RecalculateSubsectionGradeTest(HasCourseWithProblemsMixin, ModuleStoreTest self.assertEqual(len(PersistentSubsectionGrade.bulk_read_grades(self.user.id, self.course.id)), 0) @ddt.data( - (ModuleStoreEnum.Type.mongo, 1, 29), - (ModuleStoreEnum.Type.split, 3, 29), + (ModuleStoreEnum.Type.mongo, 1, 31), + (ModuleStoreEnum.Type.split, 3, 31), ) @ddt.unpack def test_persistent_grades_enabled_on_course(self, default_store, num_mongo_queries, num_sql_queries): @@ -498,3 +505,162 @@ class RecalculateGradesForUserTest(HasCourseWithProblemsMixin, ModuleStoreTestCa factory.read.assert_called_once_with(self.user, course_key=self.course.id) self.assertFalse(factory.update.called) + + +@ddt.ddt +class FreezeGradingAfterCourseEndTest(HasCourseWithProblemsMixin, ModuleStoreTestCase): + """ + Test enforce_freeze_grade_after_course_end waffle flag controlling grading tasks. + """ + def setUp(self): + super(FreezeGradingAfterCourseEndTest, self).setUp() + self.users = [UserFactory.create() for _ in xrange(12)] + self.user = self.users[0] + self.freeze_grade_flag = waffle_flags()[ENFORCE_FREEZE_GRADE_AFTER_COURSE_END] + + def _assert_log(self, mock_log, method_name): + self.assertTrue(mock_log.info.called) + log_message = u"Attempted {} for course '%s', but grades are frozen.".format(method_name) + self.assertIn( + log_message, + mock_log.info.call_args_list[0][0][0] + ) + + def _assert_for_freeze_grade_flag( + self, + result, + freeze_flag_value, + end_date_adjustment, + mock_log, + mock_call, + task_name + ): + self.assertTrue(result.successful) + if freeze_flag_value and end_date_adjustment > 30: + mock_call.assert_not_called() + self._assert_log(mock_log, task_name) + else: + mock_call.assert_called_once() + + @ddt.data( + *itertools.product( + (True, False), + (29, 31) + ) + ) + @ddt.unpack + @patch('lms.djangoapps.grades.tasks.log') + def test_compute_all_grades_for_course(self, freeze_flag_value, end_date_adjustment, mock_log): + self.set_up_course(course_end=timezone.now() - timedelta(end_date_adjustment)) + for user in self.users: + CourseEnrollment.enroll(user, self.course.id) + + with override_waffle_flag(self.freeze_grade_flag, active=freeze_flag_value): + with patch( + 'lms.djangoapps.grades.tasks.compute_grades_for_course_v2.apply_async', + return_value=None + ) as mock_compute_grades: + result = compute_all_grades_for_course.apply_async( + kwargs={ + 'course_key': six.text_type(self.course.id) + } + ) + self._assert_for_freeze_grade_flag( + result, + freeze_flag_value, + end_date_adjustment, + mock_log, + mock_compute_grades, + 'compute_all_grades_for_course' + ) + + @ddt.data( + *itertools.product( + (True, False), + (29, 31) + ) + ) + @ddt.unpack + @patch('lms.djangoapps.grades.tasks.log') + def test_compute_grades_for_course(self, freeze_flag_value, end_date_adjustment, mock_log): + self.set_up_course(course_end=timezone.now() - timedelta(end_date_adjustment)) + for user in self.users: + CourseEnrollment.enroll(user, self.course.id) + + with override_waffle_flag(self.freeze_grade_flag, active=freeze_flag_value): + with patch('lms.djangoapps.grades.tasks.CourseGradeFactory') as mock_factory: + factory = mock_factory.return_value + with mock_get_score(1, 2): + result = compute_grades_for_course.apply_async( + kwargs={ + 'course_key': six.text_type(self.course.id), + 'batch_size': 2, + 'offset': 4, + } + ) + self._assert_for_freeze_grade_flag( + result, + freeze_flag_value, + end_date_adjustment, + mock_log, + factory.iter, + 'compute_grades_for_course' + ) + + @ddt.data( + *itertools.product( + (True, False), + (29, 31) + ) + ) + @ddt.unpack + @patch('lms.djangoapps.grades.tasks.log') + def test_recalculate_course_and_subsection_grades(self, freeze_flag_value, end_date_adjustment, mock_log): + self.set_up_course(course_end=timezone.now() - timedelta(end_date_adjustment)) + CourseEnrollment.enroll(self.user, self.course.id) + with override_waffle_flag(self.freeze_grade_flag, active=freeze_flag_value): + with patch('lms.djangoapps.grades.tasks.CourseGradeFactory') as mock_factory: + factory = mock_factory.return_value + kwargs = { + 'user_id': self.user.id, + 'course_key': six.text_type(self.course.id), + } + + result = tasks.recalculate_course_and_subsection_grades_for_user.apply_async(kwargs=kwargs) + self._assert_for_freeze_grade_flag( + result, + freeze_flag_value, + end_date_adjustment, + mock_log, + factory.read, + 'recalculate_course_and_subsection_grades_for_user' + ) + + @ddt.data( + *itertools.product( + (True, False), + (29, 31) + ) + ) + @ddt.unpack + @patch('lms.djangoapps.grades.tasks.log') + def test_recalculate_subsection_grade_v3(self, freeze_flag_value, end_date_adjustment, mock_log): + self.set_up_course(course_end=timezone.now() - timedelta(end_date_adjustment)) + for user in self.users: + CourseEnrollment.enroll(user, self.course.id) + + with override_waffle_flag(self.freeze_grade_flag, active=freeze_flag_value): + modified_datetime = datetime.utcnow().replace(tzinfo=pytz.UTC) - timedelta(days=1) + with patch( + 'lms.djangoapps.grades.tasks.GradesService', + return_value=MockGradesService(mocked_return_value=MagicMock(modified=modified_datetime)) + ) as mock_grade_service: + result = recalculate_subsection_grade_v3.apply_async(kwargs=self.recalculate_subsection_grade_kwargs) + self._assert_for_freeze_grade_flag( + result, + freeze_flag_value, + end_date_adjustment, + mock_log, + mock_grade_service, + '_recalculate_subsection_grade' + )