Merge pull request #19329 from open-craft/giovanni/bb-547-ospr

BB-547 - Fix certificate api when retrieving certificates from deleted courses

lms/djangoapps/certificates/api.py

@@ -53,25 +53,28 @@ def format_certificate_for_user(username, cert):
 
     Returns: dict
     """
-    return {
-        "username": username,
-        "course_key": cert.course_id,
-        "type": cert.mode,
-        "status": cert.status,
-        "grade": cert.grade,
-        "created": cert.created_date,
-        "modified": cert.modified_date,
-        "is_passing": is_passing_status(cert.status),
+    try:
+        return {
+            "username": username,
+            "course_key": cert.course_id,
+            "type": cert.mode,
+            "status": cert.status,
+            "grade": cert.grade,
+            "created": cert.created_date,
+            "modified": cert.modified_date,
+            "is_passing": is_passing_status(cert.status),
 
-        # 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.
-        "download_url": (
-            cert.download_url or get_certificate_url(cert.user.id, cert.course_id)
-            if cert.status == CertificateStatuses.downloadable
-            else None
-        ),
-    }
+            # 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.
+            "download_url": (
+                cert.download_url or get_certificate_url(cert.user.id, cert.course_id)
+                if cert.status == CertificateStatuses.downloadable
+                else None
+            ),
+        }
+    except CourseOverview.DoesNotExist:
+        return None
 
 
 def get_certificates_for_user(username):
@@ -99,10 +102,13 @@ def get_certificates_for_user(username):
     ]
 
     """
-    return [
-        format_certificate_for_user(username, cert)
-        for cert in GeneratedCertificate.eligible_certificates.filter(user__username=username).order_by("course_id")
-    ]
+    certs = []
+    # Checks if certificates are not None before adding them to list
+    for cert in GeneratedCertificate.eligible_certificates.filter(user__username=username).order_by("course_id"):
+        formatted_cert = format_certificate_for_user(username, cert)
+        if formatted_cert:
+            certs.append(formatted_cert)
+    return certs
 
 
 def get_certificate_for_user(username, course_key):

lms/djangoapps/certificates/tests/test_api.py

@@ -14,6 +14,7 @@ from django.test.utils import override_settings
 from django.utils import timezone
 from freezegun import freeze_time
 from mock import patch
+from opaque_keys.edx.keys import CourseKey
 from opaque_keys.edx.locator import CourseLocator
 import pytz
 
@@ -359,6 +360,7 @@ class CertificateGetTests(SharedModuleStoreTestCase):
         cls.student = UserFactory()
         cls.student_no_cert = UserFactory()
         cls.uuid = uuid.uuid4().hex
+        cls.nonexistent_course_id = CourseKey.from_string('course-v1:some+fake+course')
         cls.web_cert_course = CourseFactory.create(
             org='edx',
             number='verified_1',
@@ -391,6 +393,12 @@ class CertificateGetTests(SharedModuleStoreTestCase):
             grade="0.99",
             verify_uuid=cls.uuid,
         )
+        # certificate for a course that will be deleted
+        GeneratedCertificateFactory.create(
+            user=cls.student,
+            course_id=cls.nonexistent_course_id,
+            status=CertificateStatuses.downloadable
+        )
 
     @classmethod
     def tearDownClass(cls):
@@ -494,6 +502,17 @@ class CertificateGetTests(SharedModuleStoreTestCase):
         )
         self.assertEqual('www.gmail.com', cert_url)
 
+    def test_get_certificate_with_deleted_course(self):
+        """
+        Test the case when there is a certificate but the course was deleted.
+        """
+        self.assertIsNone(
+            certs_api.get_certificate_for_user(
+                self.student.username,
+                self.nonexistent_course_id
+            )
+        )
+
 
 @attr(shard=1)
 @override_settings(CERT_QUEUE='certificates')