From 13cb2c28343f573b713287624fa87ada5ac6969e Mon Sep 17 00:00:00 2001 From: Will Daly Date: Mon, 2 Mar 2015 09:32:10 -0500 Subject: [PATCH] ECOM-1142: Management command for error certs. Add a management comand to regenerate certificates with status 'error', optionally restricted to a subset of courses. --- lms/djangoapps/certificates/api.py | 15 +- .../commands/resubmit_error_certificates.py | 121 ++++++++++++++++ lms/djangoapps/certificates/queue.py | 6 +- lms/djangoapps/certificates/tests/test_api.py | 2 +- .../tests/test_resubmit_error_certificates.py | 129 ++++++++++++++++++ lms/djangoapps/courseware/views.py | 2 +- 6 files changed, 265 insertions(+), 10 deletions(-) create mode 100644 lms/djangoapps/certificates/management/commands/resubmit_error_certificates.py create mode 100644 lms/djangoapps/certificates/tests/test_resubmit_error_certificates.py diff --git a/lms/djangoapps/certificates/api.py b/lms/djangoapps/certificates/api.py index ac99a8f3c4..4d226cdbc8 100644 --- a/lms/djangoapps/certificates/api.py +++ b/lms/djangoapps/certificates/api.py @@ -18,19 +18,24 @@ from certificates.queue import XQueueCertInterface log = logging.getLogger("edx.certificate") -def generate_user_certificates(student, course): +def generate_user_certificates(student, course_key, course=None): """ It will add the add-cert request into the xqueue. Args: - student (object): user - course (object): course + student (User) + course_key (CourseKey) + + Keyword Arguments: + course (Course): Optionally provide the course object; if not provided + it will be loaded. Returns: returns status of generated certificate + """ xqueue = XQueueCertInterface() - ret = xqueue.add_cert(student, course.id, course=course) + ret = xqueue.add_cert(student, course_key, course=course) log.info( ( u"Added a certificate generation task to the XQueue " @@ -38,7 +43,7 @@ def generate_user_certificates(student, course): u"The new certificate status is '%s'." ), student.id, - unicode(course.id), + unicode(course_key), ret ) return ret diff --git a/lms/djangoapps/certificates/management/commands/resubmit_error_certificates.py b/lms/djangoapps/certificates/management/commands/resubmit_error_certificates.py new file mode 100644 index 0000000000..b17bba7242 --- /dev/null +++ b/lms/djangoapps/certificates/management/commands/resubmit_error_certificates.py @@ -0,0 +1,121 @@ +"""Management command for re-submitting certificates with an error status. + +Certificates may have "error" status for a variety of reasons, +but the most likely is that the course was misconfigured +in the certificates worker. + +This management command identifies certificate tasks +that have an error status and re-resubmits them. + +Example usage: + + # Re-submit certificates for *all* courses + $ ./manage.py lms resubmit_error_certificates + + # Re-submit certificates for particular courses + $ ./manage.py lms resubmit_error_certificates -c edX/DemoX/Fall_2015 -c edX/DemoX/Spring_2016 + +""" +import logging +from optparse import make_option +from django.core.management.base import BaseCommand, CommandError + +from xmodule.modulestore.django import modulestore +from opaque_keys import InvalidKeyError +from opaque_keys.edx.keys import CourseKey +from certificates import api as certs_api +from certificates.models import GeneratedCertificate, CertificateStatuses + + +LOGGER = logging.getLogger(__name__) + + +class Command(BaseCommand): + """Resubmit certificates with error status. """ + + option_list = BaseCommand.option_list + ( + make_option( + '-c', '--course', + metavar='COURSE_KEY', + dest='course_key_list', + action='append', + default=[], + help='Only re-submit certificates for these courses.' + ), + ) + + def handle(self, *args, **options): + """Resubmit certificates with status 'error'. + + Arguments: + username (unicode): Identifier for the certificate's user. + + Keyword Arguments: + course_key_list (list): List of course key strings. + + Raises: + CommandError + + """ + only_course_keys = [] + for course_key_str in options.get('course_key_list', []): + try: + only_course_keys.append(CourseKey.from_string(course_key_str)) + except InvalidKeyError: + raise CommandError( + '"{course_key_str}" is not a valid course key.'.format( + course_key_str=course_key_str + ) + ) + + if only_course_keys: + LOGGER.info( + ( + u'Starting to re-submit certificates with status "error" ' + u'in these courses: %s' + ), ", ".join([unicode(key) for key in only_course_keys]) + ) + else: + LOGGER.info(u'Starting to re-submit certificates with status "error".') + + # Retrieve the IDs of generated certificates with + # error status in the set of courses we're considering. + queryset = ( + GeneratedCertificate.objects.select_related('user') + ).filter(status=CertificateStatuses.error) + if only_course_keys: + queryset = queryset.filter(course_id__in=only_course_keys) + + resubmit_list = [(cert.user, cert.course_id) for cert in queryset] + course_cache = {} + resubmit_count = 0 + for user, course_key in resubmit_list: + course = self._load_course_with_cache(course_key, course_cache) + + if course is not None: + certs_api.generate_user_certificates(user, course_key, course=course) + resubmit_count += 1 + LOGGER.info( + ( + u"Re-submitted certificate for user %s " + u"in course '%s'" + ), user.id, course_key + ) + else: + LOGGER.error( + ( + u"Could not find course for course key '%s'. " + u"Certificate for user %s will not be resubmitted." + ), course_key, user.id + ) + + LOGGER.info("Finished resubmitting %s certificate tasks", resubmit_count) + + def _load_course_with_cache(self, course_key, course_cache): + """Retrieve the course, then cache it to avoid Mongo queries. """ + course = ( + course_cache[course_key] if course_key in course_cache + else modulestore().get_course(course_key, depth=0) + ) + course_cache[course_key] = course + return course diff --git a/lms/djangoapps/certificates/queue.py b/lms/djangoapps/certificates/queue.py index 661c6b42a1..e0bef5a49b 100644 --- a/lms/djangoapps/certificates/queue.py +++ b/lms/djangoapps/certificates/queue.py @@ -345,8 +345,8 @@ class XQueueCertInterface(object): new_status ) else: - cert_status = status.notpassing - cert.status = cert_status + new_status = status.notpassing + cert.status = new_status cert.save() LOGGER.info( @@ -357,7 +357,7 @@ class XQueueCertInterface(object): ), student.id, unicode(course_id), - cert_status + new_status ) return new_status diff --git a/lms/djangoapps/certificates/tests/test_api.py b/lms/djangoapps/certificates/tests/test_api.py index 75cf9b1f09..726d538a80 100644 --- a/lms/djangoapps/certificates/tests/test_api.py +++ b/lms/djangoapps/certificates/tests/test_api.py @@ -127,7 +127,7 @@ class GenerateUserCertificatesTest(ModuleStoreTestCase): # New requests save into xqueue and return the status with patch('capa.xqueue_interface.XQueueInterface.send_to_queue') as mock_send_to_queue: mock_send_to_queue.return_value = (0, "Successfully queued") - result = certs_api.generate_user_certificates(self.student, self.course) + result = certs_api.generate_user_certificates(self.student, self.course.id) self.assertEqual(result, 'generating') diff --git a/lms/djangoapps/certificates/tests/test_resubmit_error_certificates.py b/lms/djangoapps/certificates/tests/test_resubmit_error_certificates.py new file mode 100644 index 0000000000..564b02a968 --- /dev/null +++ b/lms/djangoapps/certificates/tests/test_resubmit_error_certificates.py @@ -0,0 +1,129 @@ +"""Tests for the resubmit_error_certificates management command. """ +import ddt +from django.core.management.base import CommandError + +from opaque_keys.edx.locator import CourseLocator +from xmodule.modulestore.tests.django_utils import ModuleStoreTestCase +from xmodule.modulestore.tests.factories import CourseFactory, check_mongo_calls +from student.tests.factories import UserFactory, CourseEnrollmentFactory +from certificates.management.commands import resubmit_error_certificates +from certificates.models import GeneratedCertificate, CertificateStatuses + + +@ddt.ddt +class ResubmitErrorCertificatesTest(ModuleStoreTestCase): + """Tests for the resubmit_error_certificates management command. """ + + def setUp(self): + super(ResubmitErrorCertificatesTest, self).setUp() + self.user = UserFactory.create() + self.courses = [ + CourseFactory.create() + for __ in range(3) + ] + + def test_resubmit_error_certificate(self): + # Create a certificate with status 'error' + self._create_cert(self.courses[0].id, self.user, CertificateStatuses.error) + + # Re-submit all certificates with status 'error' + with check_mongo_calls(1): + self._run_command() + + # Expect that the certificate was re-submitted + self._assert_cert_status(self.courses[0].id, self.user, CertificateStatuses.notpassing) + + def test_resubmit_error_certificate_in_a_course(self): + # Create a certificate with status 'error' + # in three courses. + for idx in range(3): + self._create_cert(self.courses[idx].id, self.user, CertificateStatuses.error) + + # Re-submit certificates for two of the courses + self._run_command(course_key_list=[ + unicode(self.courses[0].id), + unicode(self.courses[1].id) + ]) + + # Expect that the first two courses have been re-submitted, + # but not the third course. + self._assert_cert_status(self.courses[0].id, self.user, CertificateStatuses.notpassing) + self._assert_cert_status(self.courses[1].id, self.user, CertificateStatuses.notpassing) + self._assert_cert_status(self.courses[2].id, self.user, CertificateStatuses.error) + + @ddt.data( + CertificateStatuses.deleted, + CertificateStatuses.deleting, + CertificateStatuses.downloadable, + CertificateStatuses.generating, + CertificateStatuses.notpassing, + CertificateStatuses.regenerating, + CertificateStatuses.restricted, + CertificateStatuses.unavailable, + ) + def test_resubmit_error_certificate_skips_non_error_certificates(self, other_status): + # Create certificates with an error status and some other status + self._create_cert(self.courses[0].id, self.user, CertificateStatuses.error) + self._create_cert(self.courses[1].id, self.user, other_status) + + # Re-submit certificates for all courses + self._run_command() + + # Only the certificate with status "error" should have been re-submitted + self._assert_cert_status(self.courses[0].id, self.user, CertificateStatuses.notpassing) + self._assert_cert_status(self.courses[1].id, self.user, other_status) + + def test_resubmit_error_certificate_none_found(self): + self._create_cert(self.courses[0].id, self.user, CertificateStatuses.downloadable) + self._run_command() + self._assert_cert_status(self.courses[0].id, self.user, CertificateStatuses.downloadable) + + def test_course_caching(self): + # Create multiple certificates for the same course + self._create_cert(self.courses[0].id, UserFactory.create(), CertificateStatuses.error) + self._create_cert(self.courses[0].id, UserFactory.create(), CertificateStatuses.error) + self._create_cert(self.courses[0].id, UserFactory.create(), CertificateStatuses.error) + + # Verify that we make only one Mongo query + # because the course is cached. + with check_mongo_calls(1): + self._run_command() + + def test_invalid_course_key(self): + invalid_key = u"invalid/" + with self.assertRaisesRegexp(CommandError, invalid_key): + self._run_command(course_key_list=[invalid_key]) + + def test_course_does_not_exist(self): + phantom_course = CourseLocator(org='phantom', course='phantom', run='phantom') + self._create_cert(phantom_course, self.user, 'error') + self._run_command() + + # Expect that the certificate was NOT resubmitted + # since the course doesn't actually exist. + self._assert_cert_status(phantom_course, self.user, CertificateStatuses.error) + + def _create_cert(self, course_key, user, status): + """Create a certificate entry. """ + # Enroll the user in the course + CourseEnrollmentFactory.create( + user=user, + course_id=course_key + ) + + # Create the certificate + GeneratedCertificate.objects.create( + user=user, + course_id=course_key, + status=status + ) + + def _run_command(self, *args, **kwargs): + """Run the management command to generate a fake cert. """ + command = resubmit_error_certificates.Command() + return command.handle(*args, **kwargs) + + def _assert_cert_status(self, course_key, user, expected_status): + """Check the status of a certificate. """ + cert = GeneratedCertificate.objects.get(user=user, course_id=course_key) + self.assertEqual(cert.status, expected_status) diff --git a/lms/djangoapps/courseware/views.py b/lms/djangoapps/courseware/views.py index 305415f277..b20372cc11 100644 --- a/lms/djangoapps/courseware/views.py +++ b/lms/djangoapps/courseware/views.py @@ -1304,7 +1304,7 @@ def generate_user_cert(request, course_id): certificate_status = certificate_downloadable_status(student, course.id) if not certificate_status["is_downloadable"] and not certificate_status["is_generating"]: - generate_user_certificates(student, course) + generate_user_certificates(student, course.id, course=course) _track_successful_certificate_generation(student.id, course.id) return HttpResponse(_("Creating certificate"))