Merge pull request #10968 from edx/mattdrayer/SOL-1519

mattdrayer/SOL-1519: Include course-level configuration in feature check

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

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

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

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

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',

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, "<li class=\"wrapper-organization\">", 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)
@@ -485,26 +489,17 @@ 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,

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

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

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({