Merge pull request #7218 from edx/will/student-cert-xqueue-error

Certificate status when XQueue is unavailable.

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.

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.

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