diff --git a/lms/djangoapps/certificates/api.py b/lms/djangoapps/certificates/api.py index 5f5b915fb7..b08d016c4b 100644 --- a/lms/djangoapps/certificates/api.py +++ b/lms/djangoapps/certificates/api.py @@ -67,6 +67,7 @@ def format_certificate_for_user(username, cert): # NOTE: the download URL is not currently being set for webview certificates. # In the future, we can update this to construct a URL to the webview certificate # for courses that have this feature enabled. + "is_pdf_certificate": bool(cert.download_url), "download_url": ( cert.download_url or get_certificate_url(cert.user.id, cert.course_id) if cert.status == CertificateStatuses.downloadable diff --git a/lms/djangoapps/certificates/queue.py b/lms/djangoapps/certificates/queue.py index 613e19ab06..0a85e75d70 100644 --- a/lms/djangoapps/certificates/queue.py +++ b/lms/djangoapps/certificates/queue.py @@ -137,6 +137,12 @@ class XQueueCertInterface(object): certificate.status ) + if certificate.download_url: + self._log_pdf_cert_generation_discontinued_warning( + student.id, course_id, certificate.status, certificate.download_url + ) + return None + certificate.status = status.unavailable certificate.save() @@ -237,8 +243,15 @@ class XQueueCertInterface(object): status.unverified, ] - cert_status = certificate_status_for_student(student, course_id)['status'] + cert_status_dict = certificate_status_for_student(student, course_id) + cert_status = cert_status_dict.get('status') + download_url = cert_status_dict.get('download_url') cert = None + if download_url: + self._log_pdf_cert_generation_discontinued_warning( + student.id, course_id, cert_status, download_url + ) + return None if cert_status not in valid_statuses: LOGGER.warning( @@ -585,3 +598,19 @@ class XQueueCertInterface(object): exc = XQueueAddToQueueError(error, msg) LOGGER.critical(unicode(exc)) raise exc + + def _log_pdf_cert_generation_discontinued_warning(self, student_id, course_id, cert_status, download_url): + """Logs PDF certificate generation discontinued warning.""" + LOGGER.warning( + ( + u"PDF certificate generation discontinued, canceling " + u"PDF certificate generation for student %s " + u"in course '%s' " + u"with status '%s' " + u"and download_url '%s'." + ), + student_id, + unicode(course_id), + cert_status, + download_url + ) diff --git a/lms/djangoapps/certificates/tests/test_queue.py b/lms/djangoapps/certificates/tests/test_queue.py index 7f1307fa2f..3d820778d9 100644 --- a/lms/djangoapps/certificates/tests/test_queue.py +++ b/lms/djangoapps/certificates/tests/test_queue.py @@ -11,6 +11,7 @@ from django.test import TestCase from django.test.utils import override_settings from mock import Mock, patch from opaque_keys.edx.locator import CourseLocator +from testfixtures import LogCapture # It is really unfortunate that we are using the XQueue client # code from the capa library. In the future, we should move this @@ -19,7 +20,7 @@ from opaque_keys.edx.locator import CourseLocator # in our `XQueueCertInterface` implementation. from capa.xqueue_interface import XQueueInterface from lms.djangoapps.certificates.models import CertificateStatuses, ExampleCertificate, ExampleCertificateSet, GeneratedCertificate -from lms.djangoapps.certificates.queue import XQueueCertInterface +from lms.djangoapps.certificates.queue import LOGGER, XQueueCertInterface from lms.djangoapps.certificates.tests.factories import CertificateWhitelistFactory, GeneratedCertificateFactory from course_modes.models import CourseMode from lms.djangoapps.grades.tests.utils import mock_passing_grade @@ -276,6 +277,80 @@ class XQueueCertInterfaceAddCertificateTest(ModuleStoreTestCase): expected_status ) + def test_regen_cert_with_pdf_certificate(self): + """ + Test that regenerating PDF certifcate log warning message and certificate + status remains unchanged. + """ + download_url = 'http://www.example.com/certificate.pdf' + # Create an existing verifed enrollment and certificate + CourseEnrollmentFactory( + user=self.user_2, + course_id=self.course.id, + is_active=True, + mode=CourseMode.VERIFIED, + ) + GeneratedCertificateFactory( + user=self.user_2, + course_id=self.course.id, + grade='1.0', + status=CertificateStatuses.downloadable, + mode=GeneratedCertificate.MODES.verified, + download_url=download_url + ) + + self._assert_pdf_cert_generation_dicontinued_logs(download_url) + + def test_add_cert_with_existing_pdf_certificate(self): + """ + Test that add certifcate for existing PDF certificate log warning + message and certificate status remains unchanged. + """ + download_url = 'http://www.example.com/certificate.pdf' + # Create an existing verifed enrollment and certificate + CourseEnrollmentFactory( + user=self.user_2, + course_id=self.course.id, + is_active=True, + mode=CourseMode.VERIFIED, + ) + GeneratedCertificateFactory( + user=self.user_2, + course_id=self.course.id, + grade='1.0', + status=CertificateStatuses.downloadable, + mode=GeneratedCertificate.MODES.verified, + download_url=download_url + ) + + self._assert_pdf_cert_generation_dicontinued_logs(download_url, add_cert=True) + + def _assert_pdf_cert_generation_dicontinued_logs(self, download_url, add_cert=False): + """Assert PDF certificate generation discontinued logs.""" + with LogCapture(LOGGER.name) as log: + if add_cert: + self.xqueue.add_cert(self.user_2, self.course.id) + else: + self.xqueue.regen_cert(self.user_2, self.course.id) + log.check_present( + ( + LOGGER.name, + 'WARNING', + ( + u"PDF certificate generation discontinued, canceling " + u"PDF certificate generation for student {student_id} " + u"in course '{course_id}' " + u"with status '{status}' " + u"and download_url '{download_url}'." + ).format( + student_id=self.user_2.id, + course_id=unicode(self.course.id), + status=CertificateStatuses.downloadable, + download_url=download_url + ) + ) + ) + @override_settings(CERT_QUEUE='certificates') class XQueueCertInterfaceExampleCertificateTest(TestCase): diff --git a/lms/djangoapps/certificates/tests/test_support_views.py b/lms/djangoapps/certificates/tests/test_support_views.py index 882bffc039..6e6b3491aa 100644 --- a/lms/djangoapps/certificates/tests/test_support_views.py +++ b/lms/djangoapps/certificates/tests/test_support_views.py @@ -192,6 +192,7 @@ class CertificateSearchTests(CertificateSupportTestCase): self.assertEqual(retrieved_cert["status"], self.CERT_STATUS) self.assertEqual(retrieved_cert["type"], self.CERT_MODE) self.assertEqual(retrieved_cert["download_url"], self.CERT_DOWNLOAD_URL) + self.assertFalse(retrieved_cert["regenerate"]) @override_settings(FEATURES=FEATURES_WITH_CERTS_ENABLED) def test_download_link(self): @@ -213,6 +214,7 @@ class CertificateSearchTests(CertificateSupportTestCase): kwargs={"user_id": self.student.id, "course_id": self.course.id} ) ) + self.assertTrue(retrieved_cert["regenerate"]) def _search(self, user_filter, course_filter=None): """Execute a search and return the response. """ @@ -267,6 +269,10 @@ class CertificateRegenerateTests(CertificateSupportTestCase): self.assertEqual(response.status_code, 403) def test_regenerate_certificate(self): + """Test web certificate regenration.""" + self.cert.download_url = '' + self.cert.save() + response = self._regenerate( course_key=self.course.id, username=self.STUDENT_USERNAME, diff --git a/lms/djangoapps/certificates/views/support.py b/lms/djangoapps/certificates/views/support.py index 159da3b953..4153133f88 100644 --- a/lms/djangoapps/certificates/views/support.py +++ b/lms/djangoapps/certificates/views/support.py @@ -96,7 +96,7 @@ def search_certificates(request): cert["course_key"] = unicode(cert["course_key"]) cert["created"] = cert["created"].isoformat() cert["modified"] = cert["modified"].isoformat() - cert["regenerate"] = True + cert["regenerate"] = not cert['is_pdf_certificate'] course_id = urllib.quote_plus(request.GET.get("course_id", ""), safe=':/') if course_id: