From 4f939f79f918fd3f0f2505e28be2c9034b582942 Mon Sep 17 00:00:00 2001 From: Zia Fazal Date: Thu, 3 Dec 2015 16:18:02 +0500 Subject: [PATCH] added new endpoint which renders certificate by uuid fixed broken tests and added some optimisations fixed migration history --- .../student/tests/test_certificates.py | 9 +-- common/djangoapps/student/views.py | 6 +- lms/djangoapps/certificates/api.py | 32 ++++++--- .../migrations/0005_auto_20151208_0801.py | 19 ++++++ lms/djangoapps/certificates/models.py | 7 +- .../certificates/tests/factories.py | 2 + lms/djangoapps/certificates/tests/test_api.py | 22 +++--- .../certificates/tests/test_views.py | 1 - .../certificates/tests/test_webview_views.py | 17 ++--- lms/djangoapps/certificates/urls.py | 9 ++- lms/djangoapps/certificates/views/webview.py | 67 +++++++++---------- lms/djangoapps/courseware/tests/test_views.py | 5 +- lms/djangoapps/courseware/views.py | 10 +-- 13 files changed, 114 insertions(+), 92 deletions(-) create mode 100644 lms/djangoapps/certificates/migrations/0005_auto_20151208_0801.py diff --git a/common/djangoapps/student/tests/test_certificates.py b/common/djangoapps/student/tests/test_certificates.py index 313f9b54f7..ff7c57ecb9 100644 --- a/common/djangoapps/student/tests/test_certificates.py +++ b/common/djangoapps/student/tests/test_certificates.py @@ -82,12 +82,9 @@ 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): - test_url = get_certificate_url( - user_id=self.user.id, - course_id=unicode(self.course.id) - ) + cert = self._create_certificate(enrollment_mode) + test_url = get_certificate_url(uuid=cert.verify_uuid) - self._create_certificate(enrollment_mode) certificates = [ { 'id': 0, @@ -165,7 +162,7 @@ class CertificateDisplayTest(ModuleStoreTestCase): def _create_certificate(self, enrollment_mode): """Simulate that the user has a generated certificate. """ CourseEnrollmentFactory.create(user=self.user, course_id=self.course.id, mode=enrollment_mode) - GeneratedCertificateFactory( + return GeneratedCertificateFactory( user=self.user, course_id=self.course.id, mode=enrollment_mode, diff --git a/common/djangoapps/student/views.py b/common/djangoapps/student/views.py index 097b1ad93f..fcb5bd8904 100644 --- a/common/djangoapps/student/views.py +++ b/common/djangoapps/student/views.py @@ -337,13 +337,9 @@ def _cert_info(user, course_overview, cert_status, course_mode): # pylint: disa # showing the certificate web view button if certificate is ready state and feature flags are enabled. if has_html_certificates_enabled(course_overview.id, course_overview): if course_overview.has_any_active_web_certificate: - certificate_url = get_certificate_url( - user_id=user.id, - course_id=unicode(course_overview.id), - ) status_dict.update({ 'show_cert_web_view': True, - 'cert_web_view_url': u'{url}'.format(url=certificate_url) + 'cert_web_view_url': get_certificate_url(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 71827d7883..6067a61475 100644 --- a/lms/djangoapps/certificates/api.py +++ b/lms/djangoapps/certificates/api.py @@ -167,7 +167,7 @@ def certificate_downloadable_status(student, course_key): course_key (CourseKey): ID associated with the course Returns: - Dict containing student passed status also download url for cert if available + Dict containing student passed status also download url, uuid for cert if available """ current_status = certificate_status_for_student(student, course_key) @@ -178,12 +178,14 @@ def certificate_downloadable_status(student, course_key): 'is_downloadable': False, 'is_generating': True if current_status['status'] in [CertificateStatuses.generating, CertificateStatuses.error] else False, - 'download_url': None + 'download_url': None, + 'uuid': None, } if current_status['status'] == CertificateStatuses.downloadable: response_data['is_downloadable'] = True response_data['download_url'] = current_status['download_url'] or get_certificate_url(student.id, course_key) + response_data['uuid'] = current_status['uuid'] return response_data @@ -332,19 +334,27 @@ def example_certificates_status(course_key): return ExampleCertificateSet.latest_status(course_key) -def get_certificate_url(user_id, course_id): +def get_certificate_url(user_id=None, course_id=None, uuid=None): """ - :return certificate url + :return certificate url for web or pdf certs. In case of web certs returns either old + or new cert url based on given parameters. For web certs if `uuid` is it would return + new uuid based cert url url otherwise old url. """ url = "" if settings.FEATURES.get('CERTIFICATES_HTML_VIEW', False): - url = reverse( - 'certificates:html_view', - kwargs={ - "user_id": str(user_id), - "course_id": unicode(course_id), - } - ) + if uuid: + url = reverse( + 'certificates:render_cert_by_uuid', + kwargs=dict(certificate_uuid=uuid) + ) + elif user_id and course_id: + url = reverse( + 'certificates:html_view', + kwargs={ + "user_id": str(user_id), + "course_id": unicode(course_id), + } + ) else: try: if isinstance(course_id, basestring): diff --git a/lms/djangoapps/certificates/migrations/0005_auto_20151208_0801.py b/lms/djangoapps/certificates/migrations/0005_auto_20151208_0801.py new file mode 100644 index 0000000000..134824cac0 --- /dev/null +++ b/lms/djangoapps/certificates/migrations/0005_auto_20151208_0801.py @@ -0,0 +1,19 @@ +# -*- coding: utf-8 -*- +from __future__ import unicode_literals + +from django.db import migrations, models + + +class Migration(migrations.Migration): + + dependencies = [ + ('certificates', '0004_certificategenerationhistory'), + ] + + operations = [ + migrations.AlterField( + model_name='generatedcertificate', + name='verify_uuid', + field=models.CharField(default=b'', max_length=32, db_index=True, blank=True), + ), + ] diff --git a/lms/djangoapps/certificates/models.py b/lms/djangoapps/certificates/models.py index 20d70c42db..db82043440 100644 --- a/lms/djangoapps/certificates/models.py +++ b/lms/djangoapps/certificates/models.py @@ -162,7 +162,7 @@ class GeneratedCertificate(models.Model): user = models.ForeignKey(User) course_id = CourseKeyField(max_length=255, blank=True, default=None) - verify_uuid = models.CharField(max_length=32, blank=True, default='') + verify_uuid = models.CharField(max_length=32, blank=True, default='', db_index=True) download_uuid = models.CharField(max_length=32, blank=True, default='') download_url = models.CharField(max_length=128, blank=True, default='') grade = models.CharField(max_length=5, blank=True, default='') @@ -301,7 +301,8 @@ def certificate_status_for_student(student, course_id): user=student, course_id=course_id) cert_status = { 'status': generated_certificate.status, - 'mode': generated_certificate.mode + 'mode': generated_certificate.mode, + 'uuid': generated_certificate.verify_uuid, } if generated_certificate.grade: cert_status['grade'] = generated_certificate.grade @@ -311,7 +312,7 @@ def certificate_status_for_student(student, course_id): return cert_status except GeneratedCertificate.DoesNotExist: pass - return {'status': CertificateStatuses.unavailable, 'mode': GeneratedCertificate.MODES.honor} + return {'status': CertificateStatuses.unavailable, 'mode': GeneratedCertificate.MODES.honor, 'uuid': None} def certificate_info_for_user(user, course_id, grade, user_is_whitelisted=None): diff --git a/lms/djangoapps/certificates/tests/factories.py b/lms/djangoapps/certificates/tests/factories.py index 0c68994c69..469dfeaabf 100644 --- a/lms/djangoapps/certificates/tests/factories.py +++ b/lms/djangoapps/certificates/tests/factories.py @@ -1,6 +1,7 @@ # Factories are self documenting # pylint: disable=missing-docstring import factory +from uuid import uuid4 from django.core.files.base import ContentFile from factory.django import DjangoModelFactory, ImageField @@ -21,6 +22,7 @@ class GeneratedCertificateFactory(DjangoModelFactory): status = CertificateStatuses.unavailable mode = GeneratedCertificate.MODES.honor name = '' + verify_uuid = uuid4().hex class CertificateWhitelistFactory(DjangoModelFactory): diff --git a/lms/djangoapps/certificates/tests/test_api.py b/lms/djangoapps/certificates/tests/test_api.py index 8ef0c159ef..02973fa7a1 100644 --- a/lms/djangoapps/certificates/tests/test_api.py +++ b/lms/djangoapps/certificates/tests/test_api.py @@ -23,7 +23,8 @@ from certificates.models import ( CertificateStatuses, CertificateGenerationConfiguration, ExampleCertificate, - GeneratedCertificate + GeneratedCertificate, + certificate_status_for_student, ) from certificates.queue import XQueueCertInterface, XQueueAddToQueueError from certificates.tests.factories import GeneratedCertificateFactory @@ -105,13 +106,13 @@ class CertificateDownloadableStatusTests(WebCertificateTestMixin, ModuleStoreTes status=CertificateStatuses.generating, mode='verified' ) - self.assertEqual( certs_api.certificate_downloadable_status(self.student, self.course.id), { 'is_downloadable': False, 'is_generating': True, - 'download_url': None + 'download_url': None, + 'uuid': None, } ) @@ -128,7 +129,8 @@ class CertificateDownloadableStatusTests(WebCertificateTestMixin, ModuleStoreTes { 'is_downloadable': False, 'is_generating': True, - 'download_url': None + 'download_url': None, + 'uuid': None } ) @@ -138,7 +140,8 @@ class CertificateDownloadableStatusTests(WebCertificateTestMixin, ModuleStoreTes { 'is_downloadable': False, 'is_generating': False, - 'download_url': None + 'download_url': None, + 'uuid': None, } ) @@ -147,12 +150,12 @@ class CertificateDownloadableStatusTests(WebCertificateTestMixin, ModuleStoreTes Verifies certificate_downloadable_status returns the correct response for PDF certificates. """ - GeneratedCertificateFactory.create( + cert = GeneratedCertificateFactory.create( user=self.student, course_id=self.course.id, status=CertificateStatuses.downloadable, mode='verified', - download_url='www.google.com' + download_url='www.google.com', ) self.assertEqual( @@ -160,7 +163,8 @@ class CertificateDownloadableStatusTests(WebCertificateTestMixin, ModuleStoreTes { 'is_downloadable': True, 'is_generating': False, - 'download_url': 'www.google.com' + 'download_url': 'www.google.com', + 'uuid': cert.verify_uuid } ) @@ -178,6 +182,7 @@ class CertificateDownloadableStatusTests(WebCertificateTestMixin, ModuleStoreTes with self._mock_passing_grade(): certs_api.generate_user_certificates(self.student, self.course.id) + cert_status = certificate_status_for_student(self.student, self.course.id) self.assertEqual( certs_api.certificate_downloadable_status(self.student, self.course.id), { @@ -187,6 +192,7 @@ class CertificateDownloadableStatusTests(WebCertificateTestMixin, ModuleStoreTes user_id=self.student.id, # pylint: disable=no-member course_id=self.course.id, ), + 'uuid': cert_status['uuid'] } ) diff --git a/lms/djangoapps/certificates/tests/test_views.py b/lms/djangoapps/certificates/tests/test_views.py index 755025de1c..95e017101b 100644 --- a/lms/djangoapps/certificates/tests/test_views.py +++ b/lms/djangoapps/certificates/tests/test_views.py @@ -210,7 +210,6 @@ class MicrositeCertificatesViewsTests(ModuleStoreTestCase): self.cert = GeneratedCertificate.objects.create( user=self.user, course_id=self.course_id, - verify_uuid=uuid4(), download_uuid=uuid4(), grade="0.95", key='the_key', diff --git a/lms/djangoapps/certificates/tests/test_webview_views.py b/lms/djangoapps/certificates/tests/test_webview_views.py index 943fccecbc..5f16a72bae 100644 --- a/lms/djangoapps/certificates/tests/test_webview_views.py +++ b/lms/djangoapps/certificates/tests/test_webview_views.py @@ -33,6 +33,7 @@ from certificates.tests.factories import ( CertificateHtmlViewConfigurationFactory, LinkedInAddToProfileConfigurationFactory, BadgeAssertionFactory, + GeneratedCertificateFactory, ) from util import organizations_helpers as organizations_api from django.test.client import RequestFactory @@ -80,10 +81,9 @@ class CertificatesViewsTests(ModuleStoreTestCase, EventTrackingTestCase): self.client.login(username=self.user.username, password='foo') self.request = RequestFactory().request() - self.cert = GeneratedCertificate.objects.create( + self.cert = GeneratedCertificateFactory.create( user=self.user, course_id=self.course_id, - verify_uuid=uuid4(), download_uuid=uuid4(), download_url="http://www.example.com/certificates/download", grade="0.95", @@ -164,11 +164,9 @@ 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( - user_id=self.user.id, - course_id=unicode(self.course.id) - ) + test_url = get_certificate_url(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) @override_settings(FEATURES=FEATURES_WITH_CERTS_ENABLED) @@ -178,12 +176,9 @@ 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( - user_id=self.user.id, - course_id=unicode(self.course.id) - ) + test_url = get_certificate_url(uuid=self.cert.verify_uuid) response = self.client.get(test_url) - + self.assertEqual(response.status_code, 200) # the URL should not be present self.assertNotIn(urllib.quote_plus(self.request.build_absolute_uri(test_url)), response.content) diff --git a/lms/djangoapps/certificates/urls.py b/lms/djangoapps/certificates/urls.py index 73865ed091..beb2ca3b9b 100644 --- a/lms/djangoapps/certificates/urls.py +++ b/lms/djangoapps/certificates/urls.py @@ -10,13 +10,20 @@ from certificates import views urlpatterns = patterns( '', - # Certificates HTML view + # Certificates HTML view end point to render web certs by user and course url( r'^user/(?P[^/]*)/course/{course_id}'.format(course_id=settings.COURSE_ID_PATTERN), views.render_html_view, name='html_view' ), + # Certificates HTML view end point to render web certs by certificate_uuid + url( + r'^(?P[0-9a-f]{32})$', + views.render_cert_by_uuid, + name='render_cert_by_uuid' + ), + # End-points used by student support # The views in the lms/djangoapps/support use these end-points # to retrieve certificate information and regenerate certificates. diff --git a/lms/djangoapps/certificates/views/webview.py b/lms/djangoapps/certificates/views/webview.py index 5d47772d81..592a8eb6c5 100644 --- a/lms/djangoapps/certificates/views/webview.py +++ b/lms/djangoapps/certificates/views/webview.py @@ -9,7 +9,7 @@ import urllib from django.conf import settings from django.contrib.auth.models import User -from django.http import HttpResponse +from django.http import HttpResponse, Http404 from django.template import RequestContext from django.utils.translation import ugettext as _ from django.utils.encoding import smart_str @@ -271,12 +271,7 @@ def _update_social_context(request, context, course, user, user_certificate, pla ) ) - share_url = request.build_absolute_uri( - reverse( - 'certificates:html_view', - kwargs=dict(user_id=str(user.id), course_id=unicode(course.id)) - ) - ) + share_url = request.build_absolute_uri(get_certificate_url(uuid=user_certificate.verify_uuid)) context['share_url'] = share_url twitter_url = '' if context.get('twitter_share_enabled', False): @@ -298,10 +293,7 @@ def _update_social_context(request, context, course, user, user_certificate, pla course.id, course.display_name, user_certificate.mode, - smart_str(request.build_absolute_uri(get_certificate_url( - user_id=user.id, - course_id=unicode(course.id) - ))) + smart_str(share_url) ) @@ -341,34 +333,25 @@ def _get_user_certificate(request, user, course_key, course, preview_mode=None): Returns None if there is no certificate generated for given user otherwise returns `GeneratedCertificate` instance. """ - try: - # Attempt to load the user's generated certificate data - if preview_mode: - user_certificate = GeneratedCertificate.objects.get( - user=user, - course_id=course_key, - mode=preview_mode - ) - else: - user_certificate = GeneratedCertificate.objects.get( - user=user, - course_id=course_key, - status=CertificateStatuses.downloadable - ) - - # If there's no generated certificate data for this user, we need to see if we're in 'preview' mode... - # If we are, we'll need to create a mock version of the user_certificate container for previewing - except GeneratedCertificate.DoesNotExist: - if preview_mode and ( - has_access(request.user, 'instructor', course) or - has_access(request.user, 'staff', course)): + user_certificate = None + if preview_mode: + # certificate is being previewed from studio + if has_access(request.user, 'instructor', course) or has_access(request.user, 'staff', course): user_certificate = GeneratedCertificate( mode=preview_mode, verify_uuid=unicode(uuid4().hex), modified_date=datetime.now().date() ) - else: - return None + else: + # certificate is being viewed by learner or public + try: + user_certificate = GeneratedCertificate.objects.get( + user=user, + course_id=course_key, + status=CertificateStatuses.downloadable + ) + except GeneratedCertificate.DoesNotExist: + pass return user_certificate @@ -475,13 +458,27 @@ def _update_organization_context(context, course): context['organization_logo'] = organization_logo +def render_cert_by_uuid(request, certificate_uuid): + """ + This public view generates an HTML representation of the specified certificate + """ + try: + certificate = GeneratedCertificate.objects.get( + verify_uuid=certificate_uuid, + status=CertificateStatuses.downloadable + ) + return render_html_view(request, certificate.user.id, unicode(certificate.course_id)) + except GeneratedCertificate.DoesNotExist: + raise Http404 + + @handle_500( template_path="certificates/server-error.html", test_func=lambda request: request.GET.get('preview', None) ) def render_html_view(request, user_id, course_id): """ - This public view generates an HTML representation of the specified student's certificate + This public view generates an HTML representation of the specified user and course If a certificate is not available, we display a "Sorry!" screen instead """ preview_mode = request.GET.get('preview', None) diff --git a/lms/djangoapps/courseware/tests/test_views.py b/lms/djangoapps/courseware/tests/test_views.py index 6f671f8f51..379a513c97 100644 --- a/lms/djangoapps/courseware/tests/test_views.py +++ b/lms/djangoapps/courseware/tests/test_views.py @@ -908,10 +908,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( - user_id=self.user.id, - course_id=self.course.id - ) + cert_url = certs_api.get_certificate_url(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 6c2ecf1ce3..4da0d403fc 100644 --- a/lms/djangoapps/courseware/views.py +++ b/lms/djangoapps/courseware/views.py @@ -966,18 +966,14 @@ def _progress(request, course_key, student_id): } if show_generate_cert_btn: - context.update(certs_api.certificate_downloadable_status(student, course_key)) + cert_status = certs_api.certificate_downloadable_status(student, course_key) + context.update(cert_status) # showing the certificate web view button if feature flags are enabled. if certs_api.has_html_certificates_enabled(course_key, course): if certs_api.get_active_web_certificate(course) is not None: context.update({ 'show_cert_web_view': True, - 'cert_web_view_url': u'{url}'.format( - url=certs_api.get_certificate_url( - user_id=student.id, - course_id=unicode(course.id) - ) - ) + 'cert_web_view_url': certs_api.get_certificate_url(uuid=cert_status['uuid']), }) else: context.update({