From 2d028f8246577914c39bba58dfd45f2bc542c7df Mon Sep 17 00:00:00 2001 From: Peter Fogg Date: Wed, 10 Feb 2016 10:21:42 -0500 Subject: [PATCH] Add an explicit cutoff date for audit cert granting. The previous logic was a convoluted way of doing the same thing, and has already led to one bug. Instead of hoping that the bugs are ironed out now and that future devs maintain this logic properly, let's just set a real cutoff date. --- lms/djangoapps/certificates/queue.py | 4 +- .../certificates/tests/test_queue.py | 61 +++++++++++++++---- lms/envs/aws.py | 6 ++ lms/envs/common.py | 4 ++ 4 files changed, 61 insertions(+), 14 deletions(-) diff --git a/lms/djangoapps/certificates/queue.py b/lms/djangoapps/certificates/queue.py index db744e89d8..422e24f2f2 100644 --- a/lms/djangoapps/certificates/queue.py +++ b/lms/djangoapps/certificates/queue.py @@ -326,8 +326,8 @@ class XQueueCertInterface(object): # analytics. Only do this if the certificate is new, or # already marked as ineligible -- we don't want to mark # existing audit certs as ineligible. - if (created or cert.status in (CertificateStatuses.audit_passing, CertificateStatuses.audit_notpassing)) \ - and not is_eligible_for_certificate: + cutoff = settings.AUDIT_CERT_CUTOFF_DATE + if (cutoff and cert.created_date >= cutoff) and not is_eligible_for_certificate: cert.status = CertificateStatuses.audit_passing if passing else CertificateStatuses.audit_notpassing cert.save() LOGGER.info( diff --git a/lms/djangoapps/certificates/tests/test_queue.py b/lms/djangoapps/certificates/tests/test_queue.py index fa76262078..15e8dfaa90 100644 --- a/lms/djangoapps/certificates/tests/test_queue.py +++ b/lms/djangoapps/certificates/tests/test_queue.py @@ -1,6 +1,7 @@ # -*- coding: utf-8 -*- """Tests for the XQueue certificates interface. """ from contextlib import contextmanager +from datetime import datetime, timedelta import ddt import json from mock import patch, Mock @@ -8,6 +9,8 @@ from nose.plugins.attrib import attr from django.test import TestCase from django.test.utils import override_settings +import freezegun +import pytz from course_modes.models import CourseMode from opaque_keys.edx.locator import CourseLocator @@ -81,6 +84,7 @@ class XQueueCertInterfaceAddCertificateTest(ModuleStoreTestCase): self.assertIsNotNone(certificate.verify_uuid) @ddt.data('honor', 'audit') + @override_settings(AUDIT_CERT_CUTOFF_DATE=datetime.now(pytz.UTC) - timedelta(days=1)) def test_add_cert_with_honor_certificates(self, mode): """Test certificates generations for honor and audit modes.""" template_name = 'certificate-template-{id.org}-{id.course}.pdf'.format( @@ -206,13 +210,45 @@ class XQueueCertInterfaceAddCertificateTest(ModuleStoreTestCase): self.assertFalse(mock_send.called) @ddt.data( - (CertificateStatuses.downloadable, 'Pass', CertificateStatuses.generating), - (CertificateStatuses.audit_passing, 'Pass', CertificateStatuses.audit_passing), - (CertificateStatuses.audit_notpassing, 'Pass', CertificateStatuses.audit_passing), - (CertificateStatuses.audit_notpassing, None, CertificateStatuses.audit_notpassing), + # Eligible and should stay that way + ( + CertificateStatuses.downloadable, + datetime.now(pytz.UTC) - timedelta(days=2), + 'Pass', + CertificateStatuses.generating + ), + # Ensure that certs in the wrong state can be fixed by regeneration + ( + CertificateStatuses.downloadable, + datetime.now(pytz.UTC) - timedelta(hours=1), + 'Pass', + CertificateStatuses.audit_passing + ), + # Ineligible and should stay that way + ( + CertificateStatuses.audit_passing, + datetime.now(pytz.UTC) - timedelta(hours=1), + 'Pass', + CertificateStatuses.audit_passing + ), + # As above + ( + CertificateStatuses.audit_notpassing, + datetime.now(pytz.UTC) - timedelta(hours=1), + 'Pass', + CertificateStatuses.audit_passing + ), + # As above + ( + CertificateStatuses.audit_notpassing, + datetime.now(pytz.UTC) - timedelta(hours=1), + None, + CertificateStatuses.audit_notpassing + ), ) @ddt.unpack - def test_regen_audit_certs_eligibility(self, status, grade, expected_status): + @override_settings(AUDIT_CERT_CUTOFF_DATE=datetime.now(pytz.UTC) - timedelta(days=1)) + def test_regen_audit_certs_eligibility(self, status, created_date, grade, expected_status): """ Test that existing audit certificates remain eligible even if cert generation is re-run. @@ -224,13 +260,14 @@ class XQueueCertInterfaceAddCertificateTest(ModuleStoreTestCase): is_active=True, mode=CourseMode.AUDIT, ) - GeneratedCertificateFactory( - user=self.user_2, - course_id=self.course.id, - grade='1.0', - status=status, - mode=GeneratedCertificate.MODES.audit, - ) + with freezegun.freeze_time(created_date): + GeneratedCertificateFactory( + user=self.user_2, + course_id=self.course.id, + grade='1.0', + status=status, + mode=GeneratedCertificate.MODES.audit, + ) # Run grading/cert generation again with patch('courseware.grades.grade', Mock(return_value={'grade': grade, 'percent': 0.75})): diff --git a/lms/envs/aws.py b/lms/envs/aws.py index f8dac1dd51..d16dcd6b4a 100644 --- a/lms/envs/aws.py +++ b/lms/envs/aws.py @@ -19,6 +19,8 @@ Common traits: import datetime import json +import dateutil + from .common import * from openedx.core.lib.logsettings import get_logger_config import os @@ -756,3 +758,7 @@ MICROSITE_DATABASE_TEMPLATE_CACHE_TTL = ENV_TOKENS.get( # Course Content Bookmarks Settings MAX_BOOKMARKS_PER_COURSE = ENV_TOKENS.get('MAX_BOOKMARKS_PER_COURSE', MAX_BOOKMARKS_PER_COURSE) + +# Cutoff date for granting audit certificates +if ENV_TOKENS.get('AUDIT_CERT_CUTOFF_DATE', None): + AUDIT_CERT_CUTOFF_DATE = dateutil.parser.parse(ENV_TOKENS.get('AUDIT_CERT_CUTOFF_DATE')) diff --git a/lms/envs/common.py b/lms/envs/common.py index 23f6ca65b3..5fe1ac28bf 100644 --- a/lms/envs/common.py +++ b/lms/envs/common.py @@ -2755,6 +2755,10 @@ MOBILE_APP_USER_AGENT_REGEXES = [ DEPRECATED_ADVANCED_COMPONENT_TYPES = [] +# Cutoff date for granting audit certificates + +AUDIT_CERT_CUTOFF_DATE = None + ################################ Settings for Credentials Service ################################ CREDENTIALS_SERVICE_USERNAME = 'credentials_service_user'