From d5ce713fd8f5e29e92018f84f4a22c895d18d866 Mon Sep 17 00:00:00 2001 From: CABA1998 Date: Tue, 23 Apr 2024 15:20:48 -0500 Subject: [PATCH] Revert "refactor: Replace PDF course certificate view code (#30397)" This reverts commit 116431cb8106a3efdfbe5eb6551601ee30f2970d. --- .../student/tests/test_certificates.py | 82 ++++++++++++++++- lms/djangoapps/courseware/tests/test_views.py | 88 ++++++++++++++++--- .../static/support/js/models/certificate.js | 1 + .../js/spec/views/certificates_spec.js | 18 ++-- .../templates/certificates_results.underscore | 9 ++ lms/templates/courseware/progress.html | 2 + .../_dashboard_certificate_information.html | 21 +++++ .../learner-achievements-fragment.html | 46 +++++++--- 8 files changed, 236 insertions(+), 31 deletions(-) diff --git a/common/djangoapps/student/tests/test_certificates.py b/common/djangoapps/student/tests/test_certificates.py index f935653e346..a9f63356614 100644 --- a/common/djangoapps/student/tests/test_certificates.py +++ b/common/djangoapps/student/tests/test_certificates.py @@ -15,6 +15,7 @@ from xmodule.modulestore.tests.factories import CourseFactory from xmodule.data import CertificatesDisplayBehaviors +from common.djangoapps.course_modes.models import CourseMode from common.djangoapps.student.tests.factories import CourseEnrollmentFactory, UserFactory from lms.djangoapps.certificates.api import get_certificate_url from lms.djangoapps.certificates.data import CertificateStatuses @@ -35,6 +36,7 @@ class CertificateDisplayTestBase(SharedModuleStoreTestCase): MODULESTORE = TEST_DATA_MONGO_AMNESTY_MODULESTORE USERNAME = "test_user" PASSWORD = "password" + DOWNLOAD_URL = "http://www.example.com/certificate.pdf" @classmethod def setUpClass(cls): @@ -61,7 +63,7 @@ def _check_linkedin_visibility(self, is_visible): else: self.assertNotContains(response, '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, @@ -71,10 +73,38 @@ def _create_certificate(self, enrollment_mode): user=self.user, course_id=self.course.id, mode=enrollment_mode, + download_url=download_url, status=CertificateStatuses.downloadable, grade=0.98, ) + def _check_can_download_certificate(self): + """ + Inspect the dashboard to see if a certificate can be downloaded. + """ + response = self.client.get(reverse('dashboard')) + self.assertContains(response, 'Download my') + self.assertContains(response, self.DOWNLOAD_URL) + + def _check_can_download_certificate_no_id(self): + """ + Inspects the dashboard to see if a certificate for a non verified course enrollment + is present + """ + response = self.client.get(reverse('dashboard')) + self.assertContains(response, 'Download') + self.assertContains(response, self.DOWNLOAD_URL) + + def _check_can_not_download_certificate(self): + """ + Make sure response does not have any of the download certificate buttons + """ + response = self.client.get(reverse('dashboard')) + self.assertNotContains(response, 'View Test_Certificate') + self.assertNotContains(response, 'Download my Test_Certificate') + self.assertNotContains(response, 'Download my Test_Certificate') + self.assertNotContains(response, self.DOWNLOAD_URL) + @ddt.ddt @unittest.skipUnless(settings.ROOT_URLCONF == 'lms.urls', 'Test only valid in lms') @@ -101,6 +131,7 @@ def _check_message(self, visible_date): # lint-amnesty, pylint: disable=missing if is_past: self.assertNotContains(response, test_message) self.assertNotContains(response, "View Test_Certificate") + self._check_can_download_certificate() else: self.assertContains(response, test_message) @@ -144,6 +175,27 @@ class CertificateDisplayTest(CertificateDisplayTestBase): Tests of certificate display. """ + @ddt.data('verified', 'professional') + @patch.dict('django.conf.settings.FEATURES', {'CERTIFICATES_HTML_VIEW': False}) + def test_display_verified_certificate(self, enrollment_mode): + self._create_certificate(enrollment_mode) + self._check_can_download_certificate() + + @patch.dict('django.conf.settings.FEATURES', {'CERTIFICATES_HTML_VIEW': False}) + def test_no_certificate_status_no_problem(self): + with patch('common.djangoapps.student.views.dashboard.cert_info', return_value={}): + self._create_certificate('honor') + self._check_can_not_download_certificate() + + @patch.dict('django.conf.settings.FEATURES', {'CERTIFICATES_HTML_VIEW': False}) + def test_display_verified_certificate_no_id(self): + """ + Confirm that if we get a certificate with a no-id-professional mode + we still can download our certificate + """ + self._create_certificate(CourseMode.NO_ID_PROFESSIONAL_MODE) + self._check_can_download_certificate_no_id() + @ddt.data('verified', 'honor', 'professional') def test_unverified_certificate_message(self, enrollment_mode): cert = self._create_certificate(enrollment_mode) @@ -173,6 +225,34 @@ def test_post_to_linkedin_visibility(self): self._check_linkedin_visibility(True) +@ddt.ddt +@unittest.skipUnless(settings.ROOT_URLCONF == 'lms.urls', 'Test only valid in lms') +class CertificateDisplayTestHtmlView(CertificateDisplayTestBase): + """ + Tests of webview certificate display + """ + + @classmethod + def setUpClass(cls): + super().setUpClass() + cls.course.cert_html_view_enabled = True + cls.course.save() + cls.store.update_item(cls.course, cls.USERNAME) + + @ddt.data('verified', 'honor') + @override_settings(CERT_NAME_SHORT='Test_Certificate') + @patch.dict('django.conf.settings.FEATURES', {'CERTIFICATES_HTML_VIEW': True}) + def test_display_download_certificate_button(self, enrollment_mode): + """ + 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 web view certificate Download button should not be visible. + """ + self._create_certificate(enrollment_mode, download_url='') + self._check_can_not_download_certificate() + + @ddt.ddt @unittest.skipUnless(settings.ROOT_URLCONF == 'lms.urls', 'Test only valid in lms') class CertificateDisplayTestLinkedHtmlView(CertificateDisplayTestBase): diff --git a/lms/djangoapps/courseware/tests/test_views.py b/lms/djangoapps/courseware/tests/test_views.py index faed691b9e7..7234ec78b0b 100644 --- a/lms/djangoapps/courseware/tests/test_views.py +++ b/lms/djangoapps/courseware/tests/test_views.py @@ -1442,6 +1442,7 @@ def test_view_certificate_link(self): user=self.user, course_id=self.course.id, status=CertificateStatuses.downloadable, + download_url="http://www.example.com/certificate.pdf", mode='honor' ) @@ -1492,6 +1493,34 @@ def test_view_certificate_link(self): self.assertContains(resp, "Your certificate is available") self.assertContains(resp, "earned a certificate for this course.") + @patch.dict('django.conf.settings.FEATURES', {'CERTIFICATES_HTML_VIEW': False}) + def test_view_certificate_link_hidden(self): + """ + If certificate web view is disabled then certificate web view button should not appear for user who certificate + is available/generated + """ + GeneratedCertificateFactory.create( + user=self.user, + course_id=self.course.id, + status=CertificateStatuses.downloadable, + download_url="http://www.example.com/certificate.pdf", + mode='honor' + ) + + # Enable the feature, but do not enable it for this course + CertificateGenerationConfiguration(enabled=True).save() + + # Enable certificate generation for this course + certs_api.set_cert_generation_enabled(self.course.id, True) + + with patch('lms.djangoapps.grades.course_grade_factory.CourseGradeFactory.read') as mock_create: + course_grade = mock_create.return_value + course_grade.passed = True + course_grade.summary = {'grade': 'Pass', 'percent': 0.75, 'section_breakdown': [], 'grade_breakdown': {}} + + resp = self._get_progress_page() + self.assertContains(resp, "Download Your Certificate") + @ddt.data( (True, 52), (False, 52), @@ -1563,7 +1592,9 @@ def test_page_with_invalidated_certificate_with_html_view(self): Verify that for html certs if certificate is marked as invalidated than re-generate button should not appear on progress page. """ - generated_certificate = self.generate_certificate("honor") + generated_certificate = self.generate_certificate( + "http://www.example.com/certificate.pdf", "honor" + ) # Course certificate configurations certificates = [ @@ -1598,7 +1629,9 @@ def test_page_with_allowlisted_certificate_with_html_view(self): """ Verify that view certificate appears for an allowlisted user """ - generated_certificate = self.generate_certificate("honor") + generated_certificate = self.generate_certificate( + "http://www.example.com/certificate.pdf", "honor" + ) # Course certificate configurations certificates = [ @@ -1632,6 +1665,24 @@ def test_page_with_allowlisted_certificate_with_html_view(self): self.assertContains(resp, "View Certificate") self.assert_invalidate_certificate(generated_certificate) + def test_page_with_invalidated_certificate_with_pdf(self): + """ + Verify that for pdf certs if certificate is marked as invalidated than + re-generate button should not appear on progress page. + """ + generated_certificate = self.generate_certificate( + "http://www.example.com/certificate.pdf", "honor" + ) + + with patch('lms.djangoapps.grades.course_grade_factory.CourseGradeFactory.read') as mock_create: + course_grade = mock_create.return_value + course_grade.passed = True + course_grade.summary = {'grade': 'Pass', 'percent': 0.75, 'section_breakdown': [], 'grade_breakdown': {}} + + resp = self._get_progress_page() + self.assertContains(resp, 'Download Your Certificate') + self.assert_invalidate_certificate(generated_certificate) + @ddt.data( *itertools.product( ( @@ -1722,7 +1773,9 @@ def test_invalidated_cert_data(self): """ Verify that invalidated cert data is returned if cert is invalidated. """ - generated_certificate = self.generate_certificate("honor") + generated_certificate = self.generate_certificate( + "http://www.example.com/certificate.pdf", "honor" + ) CertificateInvalidationFactory.create( generated_certificate=generated_certificate, @@ -1740,7 +1793,9 @@ def test_downloadable_get_cert_data(self): Verify that downloadable cert data is returned if cert is downloadable even when DISABLE_HONOR_CERTIFICATES feature flag is turned ON. """ - self.generate_certificate("honor") + self.generate_certificate( + "http://www.example.com/certificate.pdf", "honor" + ) response = views.get_cert_data( self.user, self.course, CourseMode.HONOR, MagicMock(passed=True) ) @@ -1752,7 +1807,9 @@ def test_generating_get_cert_data(self): """ Verify that generating cert data is returned if cert is generating. """ - self.generate_certificate("honor") + 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_generating=True)): response = views.get_cert_data(self.user, self.course, CourseMode.HONOR, MagicMock(passed=True)) @@ -1764,7 +1821,9 @@ def test_unverified_get_cert_data(self): """ Verify that unverified cert data is returned if cert is unverified. """ - self.generate_certificate("honor") + 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_unverified=True)): response = views.get_cert_data(self.user, self.course, CourseMode.HONOR, MagicMock(passed=True)) @@ -1776,7 +1835,9 @@ def test_request_get_cert_data(self): """ Verify that requested cert data is returned if cert is to be requested. """ - self.generate_certificate("honor") + 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()): response = views.get_cert_data(self.user, self.course, CourseMode.HONOR, MagicMock(passed=True)) @@ -1788,7 +1849,9 @@ def test_earned_but_not_available_get_cert_data(self): """ Verify that earned but not available cert data is returned if cert has been earned, but isn't available. """ - self.generate_certificate("verified") + self.generate_certificate( + "http://www.example.com/certificate.pdf", "verified" + ) with patch('lms.djangoapps.certificates.api.certificate_downloadable_status', return_value=self.mock_certificate_downloadable_status(earned_but_not_available=True)): response = views.get_cert_data(self.user, self.course, CourseMode.VERIFIED, MagicMock(passed=True)) @@ -1834,14 +1897,16 @@ def assert_invalidate_certificate(self, certificate): self.assertContains(resp, 'Your certificate has been invalidated') self.assertContains(resp, 'Please contact your course team if you have any questions.') self.assertNotContains(resp, 'View my Certificate') + self.assertNotContains(resp, 'Download my Certificate') - def generate_certificate(self, mode): + def generate_certificate(self, url, mode): """ Dry method to generate certificate. """ generated_certificate = GeneratedCertificateFactory.create( user=self.user, course_id=self.course.id, status=CertificateStatuses.downloadable, + download_url=url, mode=mode ) CertificateGenerationConfiguration(enabled=True).save() @@ -1849,7 +1914,7 @@ def generate_certificate(self, mode): return generated_certificate def mock_certificate_downloadable_status( - self, is_downloadable=False, is_generating=False, is_unverified=False, uuid=None, + self, is_downloadable=False, is_generating=False, is_unverified=False, uuid=None, download_url=None, earned_but_not_available=None, ): """Dry method to mock certificate downloadable status response.""" @@ -1857,7 +1922,8 @@ def mock_certificate_downloadable_status( 'is_downloadable': is_downloadable, 'is_generating': is_generating, 'is_unverified': is_unverified, - 'uuid': uuid, + 'download_url': uuid, + 'uuid': download_url, 'earned_but_not_available': earned_but_not_available, } diff --git a/lms/djangoapps/support/static/support/js/models/certificate.js b/lms/djangoapps/support/static/support/js/models/certificate.js index b9dc6a68e10..da7322ff0aa 100644 --- a/lms/djangoapps/support/static/support/js/models/certificate.js +++ b/lms/djangoapps/support/static/support/js/models/certificate.js @@ -7,6 +7,7 @@ course_key: null, type: null, status: null, + download_url: null, grade: null, created: null, modified: null diff --git a/lms/djangoapps/support/static/support/js/spec/views/certificates_spec.js b/lms/djangoapps/support/static/support/js/spec/views/certificates_spec.js index a1630f039e6..e41a8212a89 100644 --- a/lms/djangoapps/support/static/support/js/spec/views/certificates_spec.js +++ b/lms/djangoapps/support/static/support/js/spec/views/certificates_spec.js @@ -16,6 +16,7 @@ define([ grade: '0.0', type: 'honor', course_key: 'course-v1:edX+DemoX+Demo_Course', + download_url: null, modified: '2015-08-06T19:47:07+00:00', regenerate: true }, @@ -26,6 +27,7 @@ define([ grade: '1.0', type: 'verified', course_key: 'edx/test/2015', + download_url: 'http://www.example.com/certificate.pdf', modified: '2015-08-06T19:47:05+00:00', regenerate: true } @@ -39,6 +41,7 @@ define([ grade: '', type: '', course_key: 'edx/test1/2016', + download_url: null, modified: '', regenerate: false } @@ -114,15 +117,17 @@ define([ expect(results[0][0]).toEqual(REGENERATE_SEARCH_RESULTS[0].course_key); expect(results[0][1]).toEqual(REGENERATE_SEARCH_RESULTS[0].type); expect(results[0][2]).toEqual(REGENERATE_SEARCH_RESULTS[0].status); - expect(results[0][3]).toEqual(REGENERATE_SEARCH_RESULTS[0].grade); - expect(results[0][4]).toEqual(REGENERATE_SEARCH_RESULTS[0].modified); + expect(results[0][3]).toContain('Not available'); + expect(results[0][4]).toEqual(REGENERATE_SEARCH_RESULTS[0].grade); + expect(results[0][5]).toEqual(REGENERATE_SEARCH_RESULTS[0].modified); // Check the second row of results expect(results[1][0]).toEqual(REGENERATE_SEARCH_RESULTS[1].course_key); expect(results[1][1]).toEqual(REGENERATE_SEARCH_RESULTS[1].type); expect(results[1][2]).toEqual(REGENERATE_SEARCH_RESULTS[1].status); - expect(results[1][3]).toEqual(REGENERATE_SEARCH_RESULTS[1].grade); - expect(results[1][4]).toEqual(REGENERATE_SEARCH_RESULTS[1].modified); + expect(results[1][3]).toContain(REGENERATE_SEARCH_RESULTS[1].download_url); + expect(results[1][4]).toEqual(REGENERATE_SEARCH_RESULTS[1].grade); + expect(results[1][5]).toEqual(REGENERATE_SEARCH_RESULTS[1].modified); searchFor('student@example.com', 'edx/test1/2016', requests, GENERATE_SEARCH_RESULTS); @@ -133,8 +138,9 @@ define([ expect(results[0][0]).toEqual(GENERATE_SEARCH_RESULTS[0].course_key); expect(results[0][1]).toEqual(GENERATE_SEARCH_RESULTS[0].type); expect(results[0][2]).toEqual(GENERATE_SEARCH_RESULTS[0].status); - expect(results[0][3]).toEqual(GENERATE_SEARCH_RESULTS[0].grade); - expect(results[0][4]).toEqual(GENERATE_SEARCH_RESULTS[0].modified); + expect(results[0][3]).toContain('Not available'); + expect(results[0][4]).toEqual(GENERATE_SEARCH_RESULTS[0].grade); + expect(results[0][5]).toEqual(GENERATE_SEARCH_RESULTS[0].modified); }); it('searches for certificates and displays a message when there are no results', function() { diff --git a/lms/djangoapps/support/static/support/templates/certificates_results.underscore b/lms/djangoapps/support/static/support/templates/certificates_results.underscore index 7f1b0b9c8b5..9ccd9372398 100644 --- a/lms/djangoapps/support/static/support/templates/certificates_results.underscore +++ b/lms/djangoapps/support/static/support/templates/certificates_results.underscore @@ -6,6 +6,7 @@ <%- gettext("Course Key") %> <%- gettext("Type") %> <%- gettext("Status") %> + <%- gettext("Download URL") %> <%- gettext("Grade") %> <%- gettext("Last Updated") %> @@ -17,6 +18,14 @@ <%- cert.get("course_key") %> <%- cert.get("type") %> <%- cert.get("status") %> + + <% if (cert.get("download_url")) { %> + ">Download + <%- gettext("Download the user's certificate") %> + <% } else { %> + <%- gettext("Not available") %> + <% } %> + <%- cert.get("grade") %> <%- cert.get("modified") %> diff --git a/lms/templates/courseware/progress.html b/lms/templates/courseware/progress.html index f0f1850e6ea..63b340c1e42 100644 --- a/lms/templates/courseware/progress.html +++ b/lms/templates/courseware/progress.html @@ -83,6 +83,8 @@

${_(certificate_data.title)}

%if certificate_data.cert_web_view_url: ${_("View Certificate")} ${_("Opens in a new browser window")} + %elif certificate_data.cert_status == CertificateStatuses.downloadable and certificate_data.download_url: + ${_("Download Your Certificate")} ${_("Opens in a new browser window")} %elif certificate_data.cert_status == CertificateStatuses.requesting: %endif diff --git a/lms/templates/dashboard/_dashboard_certificate_information.html b/lms/templates/dashboard/_dashboard_certificate_information.html index 5f3436a7f71..51e2c58dc6e 100644 --- a/lms/templates/dashboard/_dashboard_certificate_information.html +++ b/lms/templates/dashboard/_dashboard_certificate_information.html @@ -107,6 +107,27 @@ ${_("View my {cert_name_short}").format(cert_name_short=cert_name_short)} + % elif cert_status['status'] == CertificateStatuses.downloadable and enrollment.mode in CourseMode.NON_VERIFIED_MODES: +
  • + + ${_("Download my {cert_name_short}").format(cert_name_short=cert_name_short,)} + +
    • + % elif cert_status['status'] == CertificateStatuses.downloadable and enrollment.mode == 'verified' and cert_status['mode'] == 'honor': +
  • + + ${_("Download my {cert_name_short}").format(cert_name_short=cert_name_short)} + +
    • + % elif cert_status['status'] == CertificateStatuses.downloadable and enrollment.mode in CourseMode.VERIFIED_MODES: +
  • + + ${_("Download my {cert_name_short}").format(cert_name_short=cert_name_short)} + +
    • % endif % if cert_status['show_survey_button']: diff --git a/openedx/features/learner_profile/templates/learner_profile/learner-achievements-fragment.html b/openedx/features/learner_profile/templates/learner_profile/learner-achievements-fragment.html index 33fe77186f6..8818a284feb 100644 --- a/openedx/features/learner_profile/templates/learner_profile/learner-achievements-fragment.html +++ b/openedx/features/learner_profile/templates/learner_profile/learner-achievements-fragment.html @@ -15,6 +15,7 @@

    Course Certificates

    % if course_certificates: % for certificate in course_certificates: <% + certificate_url = certificate['download_url'] course = certificate['course'] completion_date_message_html = Text(_('Completed {completion_date_html}')).format( @@ -33,20 +34,39 @@

    Course Certificates

    ), ) %> -
    - + % endif % endfor % elif own_profile: