diff --git a/lms/djangoapps/certificates/api.py b/lms/djangoapps/certificates/api.py index 4d226cdbc8..aa22db4e8d 100644 --- a/lms/djangoapps/certificates/api.py +++ b/lms/djangoapps/certificates/api.py @@ -22,6 +22,10 @@ def generate_user_certificates(student, course_key, course=None): """ It will add the add-cert request into the xqueue. + A new record will be created to track the certificate + generation task. If an error occurs while adding the certificate + to the queue, the task will have status 'error'. + Args: student (User) course_key (CourseKey) @@ -29,24 +33,9 @@ def generate_user_certificates(student, course_key, course=None): 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_key, course=course) - log.info( - ( - u"Added a certificate generation task to the XQueue " - u"for student %s in course '%s'. " - u"The new certificate status is '%s'." - ), - student.id, - unicode(course_key), - ret - ) - return ret + xqueue.add_cert(student, course_key, course=course) def certificate_downloadable_status(student, course_key): @@ -163,8 +152,6 @@ def generate_example_certificates(course_key): for cert in ExampleCertificateSet.create_example_set(course_key): xqueue.add_example_cert(cert) - log.info(u"Started generated example certificates for course '%s'.", course_key) - def example_certificates_status(course_key): """Check the status of example certificates for a course. diff --git a/lms/djangoapps/certificates/queue.py b/lms/djangoapps/certificates/queue.py index e0bef5a49b..4f27b14914 100644 --- a/lms/djangoapps/certificates/queue.py +++ b/lms/djangoapps/certificates/queue.py @@ -333,17 +333,32 @@ class XQueueCertInterface(object): new_status = status.generating cert.status = new_status cert.save() - self._send_to_xqueue(contents, key) - LOGGER.info( - ( - u"The certificate status has been set to '%s'. " - u"Sent a certificate grading task to the XQueue " - u"with the key '%s'. " - ), - key, - new_status - ) + try: + self._send_to_xqueue(contents, key) + except XQueueAddToQueueError as exc: + new_status = ExampleCertificate.STATUS_ERROR + cert.status = new_status + cert.error_reason = unicode(exc) + cert.save() + LOGGER.critical( + ( + u"Could not add certificate task to XQueue. " + u"The course was '%s' and the student was '%s'." + u"The certificate task status has been marked as 'error' " + u"and can be re-submitted with a management command." + ), student.id, course_id + ) + else: + LOGGER.info( + ( + u"The certificate status has been set to '%s'. " + u"Sent a certificate grading task to the XQueue " + u"with the key '%s'. " + ), + key, + new_status + ) else: new_status = status.notpassing cert.status = new_status @@ -410,11 +425,19 @@ class XQueueCertInterface(object): task_identifier=example_cert.uuid, callback_url_path=callback_url_path ) + LOGGER.info(u"Started generating example certificates for course '%s'.", example_cert.course_key) except XQueueAddToQueueError as exc: example_cert.update_status( ExampleCertificate.STATUS_ERROR, error_reason=unicode(exc) ) + LOGGER.critical( + ( + u"Could not add example certificate with uuid '%s' to XQueue. " + u"The exception was %s. " + u"The example certificate has been marked with status 'error'." + ), example_cert.uuid, unicode(exc) + ) def _send_to_xqueue(self, contents, key, task_identifier=None, callback_url_path='update_certificate'): """Create a new task on the XQueue. diff --git a/lms/djangoapps/certificates/tests/test_api.py b/lms/djangoapps/certificates/tests/test_api.py index 726d538a80..d88fb5fbdf 100644 --- a/lms/djangoapps/certificates/tests/test_api.py +++ b/lms/djangoapps/certificates/tests/test_api.py @@ -18,9 +18,10 @@ from certificates import api as certs_api from certificates.models import ( CertificateStatuses, CertificateGenerationConfiguration, - ExampleCertificate + ExampleCertificate, + GeneratedCertificate ) -from certificates.queue import XQueueCertInterface +from certificates.queue import XQueueCertInterface, XQueueAddToQueueError from certificates.tests.factories import GeneratedCertificateFactory @@ -103,8 +104,11 @@ class CertificateDownloadableStatusTests(ModuleStoreTestCase): ) +@override_settings(CERT_QUEUE='certificates') class GenerateUserCertificatesTest(ModuleStoreTestCase): - """Tests for the `generate_user_certificates` helper function. """ + """Tests for generating certificates for students. """ + + ERROR_REASON = "Kaboom!" def setUp(self): super(GenerateUserCertificatesTest, self).setUp() @@ -120,15 +124,44 @@ class GenerateUserCertificatesTest(ModuleStoreTestCase): self.enrollment = CourseEnrollment.enroll(self.student, self.course.id, mode='honor') self.request_factory = RequestFactory() - @override_settings(CERT_QUEUE='certificates') - @patch('courseware.grades.grade', Mock(return_value={'grade': 'Pass', 'percent': 0.75})) def test_new_cert_requests_into_xqueue_returns_generating(self): - # Mock `grade.grade` and return a summary with passing score. - # 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.id) - self.assertEqual(result, 'generating') + with self._mock_passing_grade(): + with self._mock_queue(): + certs_api.generate_user_certificates(self.student, self.course.id) + + # Verify that the certificate has status 'generating' + cert = GeneratedCertificate.objects.get(user=self.student, course_id=self.course.id) + self.assertEqual(cert.status, 'generating') + + def test_xqueue_submit_task_error(self): + with self._mock_passing_grade(): + with self._mock_queue(is_successful=False): + certs_api.generate_user_certificates(self.student, self.course.id) + + # Verify that the certificate has been marked with status error + cert = GeneratedCertificate.objects.get(user=self.student, course_id=self.course.id) + self.assertEqual(cert.status, 'error') + self.assertIn(self.ERROR_REASON, cert.error_reason) + + @contextmanager + def _mock_passing_grade(self): + """Mock the grading function to always return a passing grade. """ + symbol = 'courseware.grades.grade' + with patch(symbol) as mock_grade: + mock_grade.return_value = {'grade': 'Pass', 'percent': 0.75} + yield + + @contextmanager + def _mock_queue(self, is_successful=True): + """Mock the "send to XQueue" method to return either success or an error. """ + symbol = 'capa.xqueue_interface.XQueueInterface.send_to_queue' + with patch(symbol) as mock_send_to_queue: + if is_successful: + mock_send_to_queue.return_value = (0, "Successfully queued") + else: + mock_send_to_queue.side_effect = XQueueAddToQueueError(1, self.ERROR_REASON) + + yield mock_send_to_queue @ddt.ddt