diff --git a/common/djangoapps/student/tests/test_certificates.py b/common/djangoapps/student/tests/test_certificates.py index ff7c57ecb9..321cc74d90 100644 --- a/common/djangoapps/student/tests/test_certificates.py +++ b/common/djangoapps/student/tests/test_certificates.py @@ -82,9 +82,6 @@ class CertificateDisplayTest(ModuleStoreTestCase): @override_settings(CERT_NAME_SHORT='Test_Certificate') @patch.dict('django.conf.settings.FEATURES', {'CERTIFICATES_HTML_VIEW': True}) def test_linked_student_to_web_view_credential(self, enrollment_mode): - cert = self._create_certificate(enrollment_mode) - test_url = get_certificate_url(uuid=cert.verify_uuid) - certificates = [ { 'id': 0, @@ -100,6 +97,9 @@ class CertificateDisplayTest(ModuleStoreTestCase): self.course.save() # pylint: disable=no-member self.store.update_item(self.course, self.user.id) + cert = self._create_certificate(enrollment_mode) + test_url = get_certificate_url(course_id=self.course.id, uuid=cert.verify_uuid) + response = self.client.get(reverse('dashboard')) self.assertContains(response, u'View Test_Certificate') diff --git a/common/djangoapps/student/views.py b/common/djangoapps/student/views.py index 7d3bd84e03..7f15b12f78 100644 --- a/common/djangoapps/student/views.py +++ b/common/djangoapps/student/views.py @@ -340,7 +340,7 @@ def _cert_info(user, course_overview, cert_status, course_mode): # pylint: disa if course_overview.has_any_active_web_certificate: status_dict.update({ 'show_cert_web_view': True, - 'cert_web_view_url': get_certificate_url(uuid=cert_status['uuid']) + 'cert_web_view_url': get_certificate_url(course_id=course_overview.id, uuid=cert_status['uuid']) }) else: # don't show download certificate button if we don't have an active certificate for course diff --git a/lms/djangoapps/certificates/api.py b/lms/djangoapps/certificates/api.py index 6067a61475..8904ab707e 100644 --- a/lms/djangoapps/certificates/api.py +++ b/lms/djangoapps/certificates/api.py @@ -10,6 +10,7 @@ from django.conf import settings from django.core.urlresolvers import reverse from eventtracking import tracker +from opaque_keys import InvalidKeyError from opaque_keys.edx.keys import CourseKey from openedx.core.djangoapps.content.course_overviews.models import CourseOverview @@ -288,16 +289,33 @@ def has_html_certificates_enabled(course_key, course=None): of one. course (CourseDescriptor|CourseOverview): A course. """ - html_certificates_enabled = False - try: + # If the feature is disabled, then immediately return a False + if not settings.FEATURES.get('CERTIFICATES_HTML_VIEW', False): + return False + + # If we don't have a course object, we'll need to assemble one + if not course: + # Initialize a course key if necessary if not isinstance(course_key, CourseKey): - course_key = CourseKey.from_string(course_key) - course = course if course else CourseOverview.get_from_id(course_key) - if settings.FEATURES.get('CERTIFICATES_HTML_VIEW', False) and course.cert_html_view_enabled: - html_certificates_enabled = True - except: # pylint: disable=bare-except - pass - return html_certificates_enabled + try: + course_key = CourseKey.from_string(course_key) + except InvalidKeyError: + log.warning( + ('Unable to parse course_key "%s"', course_key), + exc_info=True + ) + return False + # Pull the course data from the cache + try: + course = CourseOverview.get_from_id(course_key) + except: # pylint: disable=bare-except + log.warning( + ('Unable to load CourseOverview object for course_key "%s"', unicode(course_key)), + exc_info=True + ) + + # Return the flag on the course object + return course.cert_html_view_enabled if course else False def example_certificates_status(course_key): @@ -341,7 +359,7 @@ def get_certificate_url(user_id=None, course_id=None, uuid=None): new uuid based cert url url otherwise old url. """ url = "" - if settings.FEATURES.get('CERTIFICATES_HTML_VIEW', False): + if has_html_certificates_enabled(course_id): if uuid: url = reverse( 'certificates:render_cert_by_uuid', @@ -356,9 +374,16 @@ def get_certificate_url(user_id=None, course_id=None, uuid=None): } ) else: - try: - if isinstance(course_id, basestring): + if isinstance(course_id, basestring): + try: course_id = CourseKey.from_string(course_id) + except InvalidKeyError: + log.warning( + ('Unable to parse course_id "%s"', course_id), + exc_info=True + ) + return url + try: user_certificate = GeneratedCertificate.objects.get( user=user_id, course_id=course_id diff --git a/lms/djangoapps/certificates/tests/test_support_views.py b/lms/djangoapps/certificates/tests/test_support_views.py index ccb7ddf19d..b20c45db51 100644 --- a/lms/djangoapps/certificates/tests/test_support_views.py +++ b/lms/djangoapps/certificates/tests/test_support_views.py @@ -79,10 +79,34 @@ class CertificateSupportTestCase(TestCase): @ddt.ddt -class CertificateSearchTests(CertificateSupportTestCase): +class CertificateSearchTests(ModuleStoreTestCase, CertificateSupportTestCase): """ Tests for the certificate search end-point used by the support team. """ + def setUp(self): + """ + Create a course + """ + super(CertificateSearchTests, self).setUp() + self.course = CourseFactory() + self.course.cert_html_view_enabled = True + + #course certificate configurations + certificates = [ + { + 'id': 1, + 'name': 'Name 1', + 'description': 'Description 1', + 'course_title': 'course_title_1', + 'signatories': [], + 'version': 1, + 'is_active': True + } + ] + + self.course.certificates = {'certificates': certificates} + self.course.save() # pylint: disable=no-member + self.store.update_item(self.course, self.user.id) @ddt.data( (GlobalStaff, True), @@ -141,6 +165,7 @@ class CertificateSearchTests(CertificateSupportTestCase): @override_settings(FEATURES=FEATURES_WITH_CERTS_ENABLED) def test_download_link(self): + self.cert.course_id = self.course.id # pylint: disable=no-member self.cert.download_url = '' self.cert.save() @@ -155,7 +180,7 @@ class CertificateSearchTests(CertificateSupportTestCase): retrieved_cert["download_url"], reverse( 'certificates:html_view', - kwargs={"user_id": self.student.id, "course_id": self.CERT_COURSE_KEY} # pylint: disable=no-member + kwargs={"user_id": self.student.id, "course_id": self.course.id} # pylint: disable=no-member ) ) diff --git a/lms/djangoapps/certificates/tests/test_views.py b/lms/djangoapps/certificates/tests/test_views.py index 95e017101b..5fb3c5cf5b 100644 --- a/lms/djangoapps/certificates/tests/test_views.py +++ b/lms/djangoapps/certificates/tests/test_views.py @@ -198,6 +198,9 @@ class MicrositeCertificatesViewsTests(ModuleStoreTestCase): self.course = CourseFactory.create( org='testorg', number='run1', display_name='refundable course' ) + self.course.cert_html_view_enabled = True + self.course.save() + self.store.update_item(self.course, self.user.id) self.course_id = self.course.location.course_key self.user = UserFactory.create( email='joe_user@edx.org', diff --git a/lms/djangoapps/certificates/tests/test_webview_views.py b/lms/djangoapps/certificates/tests/test_webview_views.py index 488590f293..d25ad0e643 100644 --- a/lms/djangoapps/certificates/tests/test_webview_views.py +++ b/lms/djangoapps/certificates/tests/test_webview_views.py @@ -164,7 +164,7 @@ class CertificatesViewsTests(ModuleStoreTestCase, EventTrackingTestCase): Test: LinkedIn share URL. """ self._add_course_certificates(count=1, signatory_count=1, is_active=True) - test_url = get_certificate_url(uuid=self.cert.verify_uuid) + test_url = get_certificate_url(course_id=self.course.id, uuid=self.cert.verify_uuid) response = self.client.get(test_url) self.assertEqual(response.status_code, 200) self.assertIn(urllib.quote_plus(self.request.build_absolute_uri(test_url)), response.content) @@ -176,7 +176,7 @@ class CertificatesViewsTests(ModuleStoreTestCase, EventTrackingTestCase): Test: LinkedIn share URL should not be visible when called from within a microsite (for now) """ self._add_course_certificates(count=1, signatory_count=1, is_active=True) - test_url = get_certificate_url(uuid=self.cert.verify_uuid) + test_url = get_certificate_url(course_id=self.cert.course_id, uuid=self.cert.verify_uuid) response = self.client.get(test_url) self.assertEqual(response.status_code, 200) # the URL should not be present @@ -316,11 +316,11 @@ class CertificatesViewsTests(ModuleStoreTestCase, EventTrackingTestCase): @override_settings(FEATURES=FEATURES_WITH_CERTS_ENABLED) def test_render_html_view_valid_certificate(self): + self._add_course_certificates(count=1, signatory_count=2) test_url = get_certificate_url( user_id=self.user.id, course_id=unicode(self.course.id) ) - self._add_course_certificates(count=1, signatory_count=2) response = self.client.get(test_url) self.assertIn(str(self.cert.verify_uuid), response.content) @@ -342,11 +342,11 @@ class CertificatesViewsTests(ModuleStoreTestCase, EventTrackingTestCase): Tests taht Certificate HTML Web View returns Certificate only if certificate status is 'downloadable', for other statuses it should return "Invalid Certificate". """ + self._add_course_certificates(count=1, signatory_count=2) test_url = get_certificate_url( user_id=self.user.id, course_id=unicode(self.course.id) ) - self._add_course_certificates(count=1, signatory_count=2) # Validate certificate response = self.client.get(test_url) @@ -365,11 +365,11 @@ class CertificatesViewsTests(ModuleStoreTestCase, EventTrackingTestCase): """ Tests that Certificate HTML Web View returns "Cannot Find Certificate" if certificate has been invalidated. """ + self._add_course_certificates(count=1, signatory_count=2) test_url = get_certificate_url( user_id=self.user.id, course_id=unicode(self.course.id) ) - self._add_course_certificates(count=1, signatory_count=2) # Validate certificate response = self.client.get(test_url) @@ -384,11 +384,12 @@ class CertificatesViewsTests(ModuleStoreTestCase, EventTrackingTestCase): @override_settings(FEATURES=FEATURES_WITH_CERTS_ENABLED) def test_render_html_view_with_valid_signatories(self): + self._add_course_certificates(count=1, signatory_count=2) test_url = get_certificate_url( user_id=self.user.id, course_id=unicode(self.course.id) ) - self._add_course_certificates(count=1, signatory_count=2) + response = self.client.get(test_url) self.assertIn('course_title_0', response.content) self.assertIn('Signatory_Name 0', response.content) @@ -399,10 +400,6 @@ class CertificatesViewsTests(ModuleStoreTestCase, EventTrackingTestCase): @override_settings(FEATURES=FEATURES_WITH_CERTS_ENABLED) def test_course_display_name_not_override_with_course_title(self): # if certificate in descriptor has not course_title then course name should not be overridden with this title. - test_url = get_certificate_url( - user_id=self.user.id, - course_id=unicode(self.course.id) - ) test_certificates = [ { 'id': 0, @@ -417,6 +414,11 @@ class CertificatesViewsTests(ModuleStoreTestCase, EventTrackingTestCase): self.course.cert_html_view_enabled = True self.course.save() self.store.update_item(self.course, self.user.id) + test_url = get_certificate_url( + user_id=self.user.id, + course_id=unicode(self.course.id) + ) + response = self.client.get(test_url) self.assertNotIn('test_course_title_0', response.content) self.assertIn('refundable course', response.content) @@ -429,11 +431,12 @@ class CertificatesViewsTests(ModuleStoreTestCase, EventTrackingTestCase): Then web certificate should display that course number and course org set in advance settings instead of original course number and course org. """ + self._add_course_certificates(count=1, signatory_count=2) test_url = get_certificate_url( user_id=self.user.id, course_id=unicode(self.course.id) ) - self._add_course_certificates(count=1, signatory_count=2) + self.course.display_coursenumber = "overridden_number" self.course.display_organization = "overridden_org" self.store.update_item(self.course, self.user.id) @@ -444,10 +447,6 @@ class CertificatesViewsTests(ModuleStoreTestCase, EventTrackingTestCase): @override_settings(FEATURES=FEATURES_WITH_CERTS_ENABLED) def test_certificate_view_without_org_logo(self): - test_url = get_certificate_url( - user_id=self.user.id, - course_id=unicode(self.course.id) - ) test_certificates = [ { 'id': 0, @@ -461,17 +460,22 @@ class CertificatesViewsTests(ModuleStoreTestCase, EventTrackingTestCase): self.course.cert_html_view_enabled = True self.course.save() self.store.update_item(self.course, self.user.id) + + test_url = get_certificate_url( + user_id=self.user.id, + course_id=unicode(self.course.id) + ) response = self.client.get(test_url) # make sure response html has only one organization logo container for edX self.assertContains(response, "
  • ", 1) @override_settings(FEATURES=FEATURES_WITH_CERTS_ENABLED) def test_render_html_view_without_signatories(self): + self._add_course_certificates(count=1, signatory_count=0) test_url = get_certificate_url( user_id=self.user.id, course_id=unicode(self.course) ) - self._add_course_certificates(count=1, signatory_count=0) response = self.client.get(test_url) self.assertNotIn('Signatory_Name 0', response.content) self.assertNotIn('Signatory_Title 0', response.content) @@ -484,27 +488,18 @@ class CertificatesViewsTests(ModuleStoreTestCase, EventTrackingTestCase): ) self.assertIn(str(self.cert.download_url), test_url) - @override_settings(FEATURES=FEATURES_WITH_CERTS_ENABLED) - def test_render_html_view_invalid_course_id(self): - test_url = get_certificate_url( - user_id=self.user.id, - course_id='az/23423/4vs' - ) - - response = self.client.get(test_url) - self.assertIn('invalid', response.content) - @override_settings(FEATURES=FEATURES_WITH_CERTS_ENABLED) def test_render_html_view_invalid_course(self): - test_url = get_certificate_url( + test_url = "/certificates/user/{user_id}/course/{course_id}".format( user_id=self.user.id, - course_id='missing/course/key' + course_id="missing/course/key" ) response = self.client.get(test_url) self.assertIn('invalid', response.content) @override_settings(FEATURES=FEATURES_WITH_CERTS_ENABLED) def test_render_html_view_invalid_user(self): + self._add_course_certificates(count=1, signatory_count=0) test_url = get_certificate_url( user_id=111, course_id=unicode(self.course.id) @@ -514,22 +509,25 @@ class CertificatesViewsTests(ModuleStoreTestCase, EventTrackingTestCase): @override_settings(FEATURES=FEATURES_WITH_CERTS_ENABLED) def test_render_html_view_invalid_user_certificate(self): - self.cert.delete() - self.assertEqual(len(GeneratedCertificate.objects.all()), 0) + self._add_course_certificates(count=1, signatory_count=0) test_url = get_certificate_url( user_id=self.user.id, course_id=unicode(self.course.id) ) + self.cert.delete() + self.assertEqual(len(GeneratedCertificate.objects.all()), 0) + response = self.client.get(test_url) self.assertIn('invalid', response.content) @override_settings(FEATURES=FEATURES_WITH_CERTS_ENABLED, PLATFORM_NAME=u'Űńíćődé Űńívéŕśítӳ') def test_render_html_view_with_unicode_platform_name(self): + self._add_course_certificates(count=1, signatory_count=0) + test_url = get_certificate_url( user_id=self.user.id, - course_id=unicode(self.course) + course_id=unicode(self.course.id) ) - self._add_course_certificates(count=1, signatory_count=0) response = self.client.get(test_url) self.assertEqual(response.status_code, 200) @@ -588,6 +586,10 @@ class CertificatesViewsTests(ModuleStoreTestCase, EventTrackingTestCase): @override_settings(FEATURES=FEATURES_WITH_CERTS_ENABLED) def test_render_html_view_invalid_certificate_configuration(self): + self.course.cert_html_view_enabled = True + self.course.save() + self.store.update_item(self.course, self.user.id) + test_url = get_certificate_url( user_id=self.user.id, course_id=unicode(self.course.id) @@ -597,6 +599,7 @@ class CertificatesViewsTests(ModuleStoreTestCase, EventTrackingTestCase): @override_settings(FEATURES=FEATURES_WITH_CERTS_ENABLED) def test_render_500_view_invalid_certificate_configuration(self): + self._add_course_certificates(count=1, signatory_count=2) CertificateHtmlViewConfiguration.objects.all().update(enabled=False) test_url = get_certificate_url( @@ -637,12 +640,13 @@ class CertificatesViewsTests(ModuleStoreTestCase, EventTrackingTestCase): @override_settings(FEATURES=FEATURES_WITH_CERTS_ENABLED) def test_evidence_event_sent(self): + self._add_course_certificates(count=1, signatory_count=2) + cert_url = get_certificate_url( user_id=self.user.id, course_id=self.course_id ) test_url = '{}?evidence_visit=1'.format(cert_url) - self._add_course_certificates(count=1, signatory_count=2) self.recreate_tracker() assertion = BadgeAssertionFactory.create( user=self.user, course_id=self.course_id, diff --git a/lms/djangoapps/certificates/views/webview.py b/lms/djangoapps/certificates/views/webview.py index 68c77d05c3..95a70ead7f 100644 --- a/lms/djangoapps/certificates/views/webview.py +++ b/lms/djangoapps/certificates/views/webview.py @@ -271,7 +271,7 @@ def _update_social_context(request, context, course, user, user_certificate, pla ) ) - share_url = request.build_absolute_uri(get_certificate_url(uuid=user_certificate.verify_uuid)) + share_url = request.build_absolute_uri(get_certificate_url(course_id=course.id, uuid=user_certificate.verify_uuid)) context['share_url'] = share_url twitter_url = '' if context.get('twitter_share_enabled', False): diff --git a/lms/djangoapps/courseware/tests/test_views.py b/lms/djangoapps/courseware/tests/test_views.py index fca93fb254..64b9d35bd6 100644 --- a/lms/djangoapps/courseware/tests/test_views.py +++ b/lms/djangoapps/courseware/tests/test_views.py @@ -910,7 +910,7 @@ class ProgressPageTests(ModuleStoreTestCase): self.assertContains(resp, u"View Certificate") self.assertContains(resp, u"You can keep working for a higher grade") - cert_url = certs_api.get_certificate_url(uuid=certificate.verify_uuid) + cert_url = certs_api.get_certificate_url(course_id=self.course.id, uuid=certificate.verify_uuid) self.assertContains(resp, cert_url) # when course certificate is not active diff --git a/lms/djangoapps/courseware/views.py b/lms/djangoapps/courseware/views.py index e71864bd77..453de67781 100644 --- a/lms/djangoapps/courseware/views.py +++ b/lms/djangoapps/courseware/views.py @@ -980,7 +980,7 @@ def _progress(request, course_key, student_id): if certs_api.get_active_web_certificate(course) is not None: context.update({ 'show_cert_web_view': True, - 'cert_web_view_url': certs_api.get_certificate_url(uuid=cert_status['uuid']), + 'cert_web_view_url': certs_api.get_certificate_url(course_id=course_key, uuid=cert_status['uuid']), }) else: context.update({