Fix certificate api when retrieving certificates from deleted courses

This commit fixes a problem with the Certificates API that make it fail when trying to retrieve user certificates from courses that don't exist anymore.
The problem lies in the Certificate API not checking if the courses being retrieved by some user actually exist, to fix this, this commit improves the fault tolerance of the Certificates API.

This issue was found when investigating why a user profile page (/u/username) was returning 404's.
Turns out that LearnerAchievementsFragmentView used the certificates api to retrieve certificate information, which did not check if the course exists before trying to pull information from it, resulting in a cascade of errors that lead to a 404 on the user's profile page.

Signed-off-by: Giovanni Cimolin da Silva <giovannicimolin@gmail.com>
This commit is contained in:
Giovanni Cimolin da Silva
2018-11-27 15:59:39 -02:00
parent 22b12b037a
commit 8563cf437a
2 changed files with 47 additions and 22 deletions

View File

@@ -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):

View File

@@ -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')