From 518218ba7dbed70e0ea89c53a3e7279e0af56638 Mon Sep 17 00:00:00 2001 From: Ahsan Ulhaq Date: Fri, 1 Jul 2016 01:48:19 +0500 Subject: [PATCH] Tech Debt on Certificate generate panel ECOM-4681 --- lms/djangoapps/certificates/models.py | 2 + lms/djangoapps/courseware/tests/test_views.py | 105 ++++++++++- lms/djangoapps/courseware/views/views.py | 164 +++++++++++++----- lms/templates/courseware/progress.html | 56 ++---- 4 files changed, 226 insertions(+), 101 deletions(-) diff --git a/lms/djangoapps/certificates/models.py b/lms/djangoapps/certificates/models.py index 6aa10be8d7..c61e59eddd 100644 --- a/lms/djangoapps/certificates/models.py +++ b/lms/djangoapps/certificates/models.py @@ -89,6 +89,8 @@ class CertificateStatuses(object): audit_passing = 'audit_passing' audit_notpassing = 'audit_notpassing' unverified = 'unverified' + invalidated = 'invalidated' + requesting = 'requesting' readable_statuses = { downloadable: "already received", diff --git a/lms/djangoapps/courseware/tests/test_views.py b/lms/djangoapps/courseware/tests/test_views.py index 1644f53250..3ebac39977 100644 --- a/lms/djangoapps/courseware/tests/test_views.py +++ b/lms/djangoapps/courseware/tests/test_views.py @@ -1104,6 +1104,7 @@ class StartDateTests(ModuleStoreTestCase): self.assertContains(response, "2015-JULY-17") +# pylint: disable=protected-access, no-member @attr('shard_1') @ddt.ddt class ProgressPageTests(ModuleStoreTestCase): @@ -1310,7 +1311,8 @@ class ProgressPageTests(ModuleStoreTestCase): ) self.assertNotContains(resp, u"View Your Certificate") self.assertNotContains(resp, u"You can now view your certificate") - self.assertContains(resp, u"We're creating your certificate.") + self.assertContains(resp, "working on it...") + self.assertContains(resp, "creating your certificate") @patch.dict('django.conf.settings.FEATURES', {'CERTIFICATES_HTML_VIEW': False}) @patch('courseware.grades.grade', Mock(return_value={'grade': 'Pass', 'percent': 0.75, 'section_breakdown': [], @@ -1401,8 +1403,6 @@ class ProgressPageTests(ModuleStoreTestCase): generated_certificate = self.generate_certificate( "http://www.example.com/certificate.pdf", "honor" ) - CertificateGenerationConfiguration(enabled=True).save() - certs_api.set_cert_generation_enabled(self.course.id, True) # Course certificate configurations certificates = [ @@ -1438,14 +1438,14 @@ class ProgressPageTests(ModuleStoreTestCase): "http://www.example.com/certificate.pdf", "honor" ) - CertificateGenerationConfiguration(enabled=True).save() - certs_api.set_cert_generation_enabled(self.course.id, True) resp = self.client.get( reverse('progress', args=[unicode(self.course.id)]) ) self.assertContains(resp, u'Download Your Certificate') self.assert_invalidate_certificate(generated_certificate) + @patch('courseware.grades.grade', Mock(return_value={'grade': 'Pass', 'percent': 0.75, 'section_breakdown': [], + 'grade_breakdown': []})) def test_message_for_audit_mode(self): """ Verify that message appears on progress page, if learner is enrolled in audit mode. @@ -1462,6 +1462,80 @@ class ProgressPageTests(ModuleStoreTestCase): u'You are enrolled in the audit track for this course. The audit track does not include a certificate.' ) + def test_invalidated_cert_data(self): + """ + Verify that invalidated cert data is returned if cert is invalidated. + """ + generated_certificate = self.generate_certificate( + "http://www.example.com/certificate.pdf", "honor" + ) + + CertificateInvalidationFactory.create( + generated_certificate=generated_certificate, + invalidated_by=self.user + ) + # Invalidate user certificate + generated_certificate.invalidate() + response = views._get_cert_data(self.user, self.course, self.course.id, True, CourseMode.HONOR) + self.assertEqual(response.cert_status, 'invalidated') + self.assertEqual(response.title, 'Your certificate has been invalidated') + + def test_downloadable_get_cert_data(self): + """ + Verify that downloadable cert data is returned if cert is downloadable. + """ + self.generate_certificate( + "http://www.example.com/certificate.pdf", "honor" + ) + with patch('certificates.api.certificate_downloadable_status', + return_value=self.mock_certificate_downloadable_status(is_downloadable=True)): + response = views._get_cert_data(self.user, self.course, self.course.id, True, CourseMode.HONOR) + + self.assertEqual(response.cert_status, 'downloadable') + self.assertEqual(response.title, 'Your certificate is available') + + def test_generating_get_cert_data(self): + """ + Verify that generating cert data is returned if cert is generating. + """ + self.generate_certificate( + "http://www.example.com/certificate.pdf", "honor" + ) + with patch('certificates.api.certificate_downloadable_status', + return_value=self.mock_certificate_downloadable_status(is_generating=True)): + response = views._get_cert_data(self.user, self.course, self.course.id, True, CourseMode.HONOR) + + self.assertEqual(response.cert_status, 'generating') + self.assertEqual(response.title, "We're working on it...") + + def test_unverified_get_cert_data(self): + """ + Verify that unverified cert data is returned if cert is unverified. + """ + self.generate_certificate( + "http://www.example.com/certificate.pdf", "honor" + ) + with patch('certificates.api.certificate_downloadable_status', + return_value=self.mock_certificate_downloadable_status(is_unverified=True)): + response = views._get_cert_data(self.user, self.course, self.course.id, True, CourseMode.HONOR) + + self.assertEqual(response.cert_status, 'unverified') + self.assertEqual(response.title, "Certificate unavailable") + + def test_request_get_cert_data(self): + """ + Verify that requested cert data is returned if cert is to be requested. + """ + self.generate_certificate( + "http://www.example.com/certificate.pdf", "honor" + ) + with patch('certificates.api.certificate_downloadable_status', + return_value=self.mock_certificate_downloadable_status()): + response = views._get_cert_data(self.user, self.course, self.course.id, True, CourseMode.HONOR) + + self.assertEqual(response.cert_status, 'requesting') + self.assertEqual(response.title, "Congratulations, you qualified for a certificate!") + def assert_invalidate_certificate(self, certificate): """ Dry method to mark certificate as invalid. And assert the response. """ CertificateInvalidationFactory.create( @@ -1473,21 +1547,38 @@ class ProgressPageTests(ModuleStoreTestCase): resp = self.client.get( reverse('progress', args=[unicode(self.course.id)]) ) + self.assertNotContains(resp, u'Request Certificate') - self.assertContains(resp, u'Your certificate has been invalidated.') + self.assertContains(resp, u'Your certificate has been invalidated') self.assertContains(resp, u'Please contact your course team if you have any questions.') self.assertNotContains(resp, u'View Your Certificate') self.assertNotContains(resp, u'Download Your Certificate') def generate_certificate(self, url, mode): """ Dry method to generate certificate. """ - return GeneratedCertificateFactory.create( + + generated_certificate = GeneratedCertificateFactory.create( user=self.user, course_id=self.course.id, status=CertificateStatuses.downloadable, download_url=url, mode=mode ) + CertificateGenerationConfiguration(enabled=True).save() + certs_api.set_cert_generation_enabled(self.course.id, True) + return generated_certificate + + def mock_certificate_downloadable_status( + self, is_downloadable=False, is_generating=False, is_unverified=False, uuid=None, download_url=None + ): + """Dry method to mock certificate downloadable status response.""" + return { + 'is_downloadable': is_downloadable, + 'is_generating': is_generating, + 'is_unverified': is_unverified, + 'download_url': uuid, + 'uuid': download_url, + } @attr('shard_1') diff --git a/lms/djangoapps/courseware/views/views.py b/lms/djangoapps/courseware/views/views.py index fc6778f11b..14af5a71f3 100644 --- a/lms/djangoapps/courseware/views/views.py +++ b/lms/djangoapps/courseware/views/views.py @@ -5,7 +5,7 @@ Courseware views functions import json import logging import urllib -from collections import OrderedDict +from collections import OrderedDict, namedtuple from datetime import datetime import analytics @@ -40,6 +40,7 @@ import survey.utils import survey.views from lms.djangoapps.ccx.utils import prep_course_for_grading from certificates import api as certs_api +from certificates.models import CertificateStatuses from course_blocks.api import get_course_blocks from openedx.core.djangoapps.models.course_details import CourseDetails from commerce.utils import EcommerceService @@ -103,6 +104,8 @@ log = logging.getLogger("edx.courseware") # credit and verified modes. REQUIREMENTS_DISPLAY_MODES = CourseMode.CREDIT_MODES + [CourseMode.VERIFIED] +CertData = namedtuple("CertData", ["cert_status", "title", "msg", "download_url", "cert_web_view_url"]) + def user_groups(user): """ @@ -691,17 +694,17 @@ def _progress(request, course_key, student_id): return redirect(reverse('course_survey', args=[unicode(course.id)])) staff_access = bool(has_access(request.user, 'staff', course)) - try: - coach_access = has_ccx_coach_role(request.user, course_key) - except CCXLocatorValidationException: - coach_access = False - - has_access_on_students_profiles = staff_access or coach_access if student_id is None or student_id == request.user.id: # always allowed to see your own profile student = request.user else: + try: + coach_access = has_ccx_coach_role(request.user, course_key) + except CCXLocatorValidationException: + coach_access = False + + has_access_on_students_profiles = staff_access or coach_access # Requesting access to a different student's profile if not has_access_on_students_profiles: raise Http404 @@ -721,26 +724,16 @@ def _progress(request, course_key, student_id): course_structure = get_course_blocks(student, course.location) courseware_summary = grades.progress_summary(student, course, course_structure) + if courseware_summary is None: + # This means the student didn't have access to the course (which the instructor requested) + raise Http404 + grade_summary = grades.grade(student, course, course_structure=course_structure) studio_url = get_studio_url(course, 'settings/grading') - if courseware_summary is None: - #This means the student didn't have access to the course (which the instructor requested) - raise Http404 - # checking certificate generation configuration enrollment_mode, is_active = CourseEnrollment.enrollment_mode_for_user(student, course_key) - # If the learner is in verified modes and the student did not have - # their ID verified, we need to show message to ask learner to verify their ID first - missing_required_verification = enrollment_mode in CourseMode.VERIFIED_MODES and \ - not SoftwareSecurePhotoVerification.user_is_verified(student) - - show_generate_cert_btn = ( - is_active and CourseMode.is_eligible_for_certificate(enrollment_mode) - and certs_api.cert_generation_enabled(course_key) - ) - context = { 'course': course, 'courseware_summary': courseware_summary, @@ -749,42 +742,117 @@ def _progress(request, course_key, student_id): 'staff_access': staff_access, 'student': student, 'passed': is_course_passed(course, grade_summary), - 'show_generate_cert_btn': show_generate_cert_btn, 'credit_course_requirements': _credit_course_requirements(course_key, student), - 'missing_required_verification': missing_required_verification, - 'certificate_invalidated': False, - 'enrollment_mode': enrollment_mode, + 'certificate_data': _get_cert_data(student, course, course_key, is_active, enrollment_mode) } - if show_generate_cert_btn: - # If current certificate is invalidated by instructor - # then show the certificate invalidated message. - context.update({ - 'certificate_invalidated': certs_api.is_certificate_invalid(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': certs_api.get_certificate_url(course_id=course_key, uuid=cert_status['uuid']), - }) - else: - context.update({ - 'is_downloadable': False, - 'is_generating': True, - 'download_url': None - }) - with outer_atomic(): response = render_to_response('courseware/progress.html', context) return response +def _get_cert_data(student, course, course_key, is_active, enrollment_mode): + """Returns students course certificate related data. + + Arguments: + student (User): Student for whom certificate to retrieve. + course (Course): Course object for which certificate data to retrieve. + course_key (CourseKey): Course identifier for course. + is_active (Bool): Boolean value to check if course is active. + enrollment_mode (String): Course mode in which student is enrolled. + + Returns: + returns dict if course certificate is available else None. + """ + + if enrollment_mode == CourseMode.AUDIT: + return CertData( + CertificateStatuses.audit_passing, + 'Your enrollment: Audit track', + 'You are enrolled in the audit track for this course. The audit track does not include a certificate.', + download_url=None, + cert_web_view_url=None + ) + + show_generate_cert_btn = ( + is_active and CourseMode.is_eligible_for_certificate(enrollment_mode) + and certs_api.cert_generation_enabled(course_key) + ) + + if not show_generate_cert_btn: + return None + + if certs_api.is_certificate_invalid(student, course_key): + return CertData( + CertificateStatuses.invalidated, + 'Your certificate has been invalidated', + 'Please contact your course team if you have any questions.', + download_url=None, + cert_web_view_url=None + ) + + cert_downloadable_status = certs_api.certificate_downloadable_status(student, course_key) + + if cert_downloadable_status['is_downloadable']: + cert_status = CertificateStatuses.downloadable + title = 'Your certificate is available' + msg = 'You can keep working for a higher grade, or request your certificate now.' + if certs_api.has_html_certificates_enabled(course_key, course): + if certs_api.get_active_web_certificate(course) is not None: + cert_web_view_url = certs_api.get_certificate_url( + course_id=course_key, uuid=cert_downloadable_status['uuid'] + ) + return CertData(cert_status, title, msg, download_url=None, cert_web_view_url=cert_web_view_url) + else: + return CertData( + CertificateStatuses.generating, + "We're working on it...", + "We're creating your certificate. You can keep working in your courses and a link " + "to it will appear here and on your Dashboard when it is ready.", + download_url=None, + cert_web_view_url=None + ) + + return CertData( + cert_status, title, msg, download_url=cert_downloadable_status['download_url'], cert_web_view_url=None + ) + + if cert_downloadable_status['is_generating']: + return CertData( + CertificateStatuses.generating, + "We're working on it...", + "We're creating your certificate. You can keep working in your courses and a link to " + "it will appear here and on your Dashboard when it is ready.", + download_url=None, + cert_web_view_url=None + ) + + # If the learner is in verified modes and the student did not have + # their ID verified, we need to show message to ask learner to verify their ID first + missing_required_verification = enrollment_mode in CourseMode.VERIFIED_MODES and \ + not SoftwareSecurePhotoVerification.user_is_verified(student) + + if missing_required_verification or cert_downloadable_status['is_unverified']: + platform_name = theming_helpers.get_value('PLATFORM_NAME', settings.PLATFORM_NAME) + return CertData( + CertificateStatuses.unverified, + 'Certificate unavailable', + 'You have not received a certificate because you do not have a current {platform_name} verified ' + 'identity.'.format(platform_name=platform_name), + download_url=None, + cert_web_view_url=None + ) + + return CertData( + CertificateStatuses.requesting, + 'Congratulations, you qualified for a certificate!', + 'You can keep working for a higher grade, or request your certificate now.', + download_url=None, + cert_web_view_url=None + ) + + def _credit_course_requirements(course_key, student): """Return information about which credit requirements a user has satisfied. diff --git a/lms/templates/courseware/progress.html b/lms/templates/courseware/progress.html index 3d4250497a..5e5b5d52d7 100644 --- a/lms/templates/courseware/progress.html +++ b/lms/templates/courseware/progress.html @@ -3,6 +3,7 @@ <%def name="online_help_token()"><% return "progress" %> <%! from course_modes.models import CourseMode +from certificates.models import CertificateStatuses from django.utils.translation import ugettext as _ from django.core.urlresolvers import reverse from util.date_utils import get_time_display, DEFAULT_SHORT_DATE_FORMAT @@ -49,66 +50,29 @@ from django.utils.http import urlquote_plus

${_("Course Progress for Student '{username}' ({email})").format(username=student.username, email=student.email) | h}

- %if enrollment_mode == CourseMode.AUDIT: -
-
-

${_("Your enrollment: Audit track")}

-

${_("You are enrolled in the audit track for this course. The audit track does not include a certificate.")}

-
-
- %elif show_generate_cert_btn: + %if certificate_data:
%if passed:
<% post_url = reverse('generate_user_cert', args=[unicode(course.id)]) %> - ## If current certificate is invalidated by instructor then don't show the generate button. - % if certificate_invalidated: -

${_("Your certificate has been invalidated. Please contact your course team if you have any questions.")}

- %elif is_downloadable:
-

${_("Your certificate is available")}

-

- ${_("You can keep working for a higher grade, or request your certificate now.")} -

+

${certificate_data.title | h}

+

${certificate_data.msg | h}

- %if show_cert_web_view and cert_web_view_url: - + %if certificate_data.cert_web_view_url: + ${_("View Certificate")} - %elif download_url: - + %elif certificate_data.cert_status == CertificateStatuses.downloadable and certificate_data.download_url: + ${_("Download Your Certificate")} + %elif certificate_data.cert_status == CertificateStatuses.requesting: + %endif
- %elif is_generating: -
- ## Translators: This message appears to users when the system is processessing course certificates, which can take a few hours. -

${_("We're working on it...")}

-

${_("We're creating your certificate. You can keep working in your courses and a link to it will appear here and on your Dashboard when it is ready.")}

-
-
- %elif missing_required_verification or is_unverified: - ## The is_unverified variable comes from certificate status. - ## At the moment, the assumption is we should not show generate certificate button - ## if the certificate previously generated is in unverified status -
- ## Translators: This message appears to users when the users have not completed identity verification. -

${_("Certificate unavailable")}

-

${_("You have not received a certificate because you do not have a current {platform_name} verified identity. ").format(platform_name=settings.PLATFORM_NAME)} ${_("Verify your identity now.")}

-
-
- %else: -
-

${_("Congratulations, you qualified for a certificate!")}

-

${_("You can keep working for a higher grade, or request your certificate now.")}

-
-
- -
- %endif
%endif