From 2d04b7980a0a6a3468a9e359623581ee1970fb82 Mon Sep 17 00:00:00 2001
From: Waheed Ahmed <waheed.ahmed@arbisoft.com>
Date: Wed, 22 Jan 2020 14:00:55 +0500
Subject: [PATCH] Fix already earned honor PDF certificates.
Learner who have already earned PDF honor certificates in old courses
are unable to see the certificate links on dashboard and course progress
pages since `course.cert_html_view_enabled` is deprecated and default to
True for all courses.
PROD-60
---
common/djangoapps/student/helpers.py | 2 ++
.../student/tests/test_certificates.py | 8 +++----
lms/djangoapps/courseware/tests/test_views.py | 21 +++++++------------
lms/djangoapps/courseware/views/views.py | 12 +++++++----
openedx/core/djangoapps/certificates/api.py | 6 +++++-
5 files changed, 27 insertions(+), 22 deletions(-)
diff --git a/common/djangoapps/student/helpers.py b/common/djangoapps/student/helpers.py
index c4b55f67180..e31b533cff0 100644
--- a/common/djangoapps/student/helpers.py
+++ b/common/djangoapps/student/helpers.py
@@ -493,6 +493,8 @@ def _cert_info(user, course_overview, cert_status):
'show_cert_web_view': True,
'cert_web_view_url': get_certificate_url(course_id=course_overview.id, uuid=cert_status['uuid'])
})
+ elif cert_status['download_url']:
+ status_dict['download_url'] = cert_status['download_url']
else:
# don't show download certificate button if we don't have an active certificate for course
status_dict['status'] = 'unavailable'
diff --git a/common/djangoapps/student/tests/test_certificates.py b/common/djangoapps/student/tests/test_certificates.py
index e7efe1c67cf..29827969918 100644
--- a/common/djangoapps/student/tests/test_certificates.py
+++ b/common/djangoapps/student/tests/test_certificates.py
@@ -60,7 +60,7 @@ class CertificateDisplayTestBase(SharedModuleStoreTestCase):
else:
self.assertNotContains(response, u'Add Certificate to LinkedIn Profile')
- def _create_certificate(self, enrollment_mode):
+ def _create_certificate(self, enrollment_mode, download_url=DOWNLOAD_URL):
"""Simulate that the user has a generated certificate. """
CourseEnrollmentFactory.create(
user=self.user,
@@ -70,7 +70,7 @@ class CertificateDisplayTestBase(SharedModuleStoreTestCase):
user=self.user,
course_id=self.course.id,
mode=enrollment_mode,
- download_url=self.DOWNLOAD_URL,
+ download_url=download_url,
status=CertificateStatuses.downloadable,
grade=0.98,
)
@@ -258,9 +258,9 @@ class CertificateDisplayTestHtmlView(CertificateDisplayTestBase):
Tests if CERTIFICATES_HTML_VIEW is True
and course has enabled web certificates via cert_html_view_enabled setting
and no active certificate configuration available
- then any of the Download certificate button should not be visible.
+ then any of the web view certificate Download button should not be visible.
"""
- self._create_certificate(enrollment_mode)
+ self._create_certificate(enrollment_mode, download_url='')
self._check_can_not_download_certificate()
diff --git a/lms/djangoapps/courseware/tests/test_views.py b/lms/djangoapps/courseware/tests/test_views.py
index c62882bde9b..bc27d5efe21 100644
--- a/lms/djangoapps/courseware/tests/test_views.py
+++ b/lms/djangoapps/courseware/tests/test_views.py
@@ -1272,6 +1272,7 @@ class ProgressPageBaseTests(ModuleStoreTestCase):
# pylint: disable=protected-access
+@patch('lms.djangoapps.certificates.api.get_active_web_certificate', PropertyMock(return_value=True))
@ddt.ddt
class ProgressPageTests(ProgressPageBaseTests):
"""
@@ -1370,7 +1371,6 @@ class ProgressPageTests(ProgressPageBaseTests):
resp = self._get_progress_page()
self.assertNotContains(resp, 'Request Certificate')
- @patch('lms.djangoapps.certificates.api.get_active_web_certificate', PropertyMock(return_value=True))
@patch.dict('django.conf.settings.FEATURES', {'CERTIFICATES_HTML_VIEW': True})
def test_view_certificate_for_unverified_student(self):
"""
@@ -1402,7 +1402,6 @@ class ProgressPageTests(ProgressPageBaseTests):
self.assertNotContains(resp, u"Certificate unavailable")
self.assertContains(resp, u"Your certificate is available")
- @patch('lms.djangoapps.certificates.api.get_active_web_certificate', PropertyMock(return_value=True))
@patch.dict('django.conf.settings.FEATURES', {'CERTIFICATES_HTML_VIEW': True})
def test_view_certificate_link(self):
"""
@@ -1464,7 +1463,6 @@ class ProgressPageTests(ProgressPageBaseTests):
self.assertContains(resp, "Your certificate is available")
self.assertContains(resp, "earned a certificate for this course.")
- @patch('lms.djangoapps.certificates.api.get_active_web_certificate', PropertyMock(return_value=True))
@patch.dict('django.conf.settings.FEATURES', {'CERTIFICATES_HTML_VIEW': False})
def test_view_certificate_link_hidden(self):
"""
@@ -1494,8 +1492,8 @@ class ProgressPageTests(ProgressPageBaseTests):
self.assertContains(resp, u"Download Your Certificate")
@ddt.data(
- (True, 49),
- (False, 48)
+ (True, 51),
+ (False, 50)
)
@ddt.unpack
def test_progress_queries_paced_courses(self, self_paced, query_count):
@@ -1508,8 +1506,8 @@ class ProgressPageTests(ProgressPageBaseTests):
@patch.dict(settings.FEATURES, {'ASSUME_ZERO_GRADE_IF_ABSENT_FOR_ALL_TESTS': False})
@ddt.data(
- (False, 57, 37),
- (True, 48, 32)
+ (False, 59, 38),
+ (True, 50, 33)
)
@ddt.unpack
def test_progress_queries(self, enable_waffle, initial, subsequent):
@@ -1528,7 +1526,6 @@ class ProgressPageTests(ProgressPageBaseTests):
), check_mongo_calls(1):
self._get_progress_page()
- @patch('lms.djangoapps.certificates.api.get_active_web_certificate', PropertyMock(return_value=True))
@ddt.data(
*itertools.product(
(
@@ -1650,7 +1647,6 @@ class ProgressPageTests(ProgressPageBaseTests):
self.assertContains(resp, u"View Certificate")
self.assert_invalidate_certificate(generated_certificate)
- @patch('lms.djangoapps.certificates.api.get_active_web_certificate', PropertyMock(return_value=True))
def test_page_with_invalidated_certificate_with_pdf(self):
"""
Verify that for pdf certs if certificate is marked as invalidated than
@@ -1733,7 +1729,6 @@ class ProgressPageTests(ProgressPageBaseTests):
self.assertNotContains(response, bannerText, html=True)
@patch('lms.djangoapps.courseware.views.views.is_course_passed', PropertyMock(return_value=True))
- @patch('lms.djangoapps.certificates.api.get_active_web_certificate', PropertyMock(return_value=True))
@override_settings(FEATURES=FEATURES_WITH_DISABLE_HONOR_CERTIFICATE)
@ddt.data(CourseMode.AUDIT, CourseMode.HONOR)
def test_message_for_ineligible_mode(self, course_mode):
@@ -1780,9 +1775,9 @@ class ProgressPageTests(ProgressPageBaseTests):
self.generate_certificate(
"http://www.example.com/certificate.pdf", "honor"
)
- with patch('lms.djangoapps.certificates.api.certificate_downloadable_status',
- return_value=self.mock_certificate_downloadable_status(is_downloadable=True)):
- response = views._get_cert_data(self.user, self.course, CourseMode.HONOR, MagicMock(passed=True))
+ response = views._get_cert_data(
+ self.user, self.course, CourseMode.HONOR, MagicMock(passed=True)
+ )
self.assertEqual(response.cert_status, 'downloadable')
self.assertEqual(response.title, 'Your certificate is available')
diff --git a/lms/djangoapps/courseware/views/views.py b/lms/djangoapps/courseware/views/views.py
index 3653202787d..c7b7d8d79a5 100644
--- a/lms/djangoapps/courseware/views/views.py
+++ b/lms/djangoapps/courseware/views/views.py
@@ -1111,9 +1111,9 @@ def _progress(request, course_key, student_id):
'student': student,
'credit_course_requirements': _credit_course_requirements(course_key, student),
'course_expiration_fragment': course_expiration_fragment,
+ 'certificate_data': _get_cert_data(student, course, enrollment_mode, course_grade)
}
- if certs_api.get_active_web_certificate(course):
- context['certificate_data'] = _get_cert_data(student, course, enrollment_mode, course_grade)
+
context.update(
get_experiment_user_metadata_context(
course,
@@ -1136,7 +1136,7 @@ def _downloadable_certificate_message(course, cert_downloadable_status):
course_id=course.id, uuid=cert_downloadable_status['uuid']
)
)
- else:
+ elif not cert_downloadable_status['download_url']:
return GENERATING_CERT_DATA
return _downloadable_cert_data(download_url=cert_downloadable_status['download_url'])
@@ -1189,7 +1189,11 @@ def _get_cert_data(student, course, enrollment_mode, course_grade=None):
if not auto_certs_api.can_show_certificate_message(course, student, course_grade, certificates_enabled_for_course):
return
- return _certificate_message(student, course, enrollment_mode)
+ cert_data = _certificate_message(student, course, enrollment_mode)
+ if not certs_api.get_active_web_certificate(course) and not auto_certs_api.is_valid_pdf_certificate(cert_data):
+ return
+
+ return cert_data
def _credit_course_requirements(course_key, student):
diff --git a/openedx/core/djangoapps/certificates/api.py b/openedx/core/djangoapps/certificates/api.py
index 4d9cd20680a..3da7de73125 100644
--- a/openedx/core/djangoapps/certificates/api.py
+++ b/openedx/core/djangoapps/certificates/api.py
@@ -9,7 +9,7 @@ from datetime import datetime
import six
from pytz import UTC
-from lms.djangoapps.certificates.models import CertificateWhitelist
+from lms.djangoapps.certificates.models import CertificateStatuses, CertificateWhitelist
from openedx.core.djangoapps.certificates.config import waffle
from student.models import CourseEnrollment
@@ -112,3 +112,7 @@ def display_date_for_certificate(course, certificate):
if _course_uses_available_date(course) and course.certificate_available_date < datetime.now(UTC):
return course.certificate_available_date
return certificate.modified_date
+
+
+def is_valid_pdf_certificate(cert_data):
+ return cert_data.cert_status == CertificateStatuses.downloadable and cert_data.download_url
--
GitLab