diff --git a/common/djangoapps/student/helpers.py b/common/djangoapps/student/helpers.py index 792a07647f..bec5d01179 100644 --- a/common/djangoapps/student/helpers.py +++ b/common/djangoapps/student/helpers.py @@ -31,7 +31,7 @@ VERIFY_STATUS_NEED_TO_REVERIFY = "verify_need_to_reverify" DISABLE_UNENROLL_CERT_STATES = [ 'generating', - 'ready', + 'downloadable', ] diff --git a/common/djangoapps/student/tests/test_certificates.py b/common/djangoapps/student/tests/test_certificates.py index 9f6c8989cf..8cd89492e3 100644 --- a/common/djangoapps/student/tests/test_certificates.py +++ b/common/djangoapps/student/tests/test_certificates.py @@ -2,12 +2,14 @@ import unittest +import datetime import ddt import mock from django.conf import settings from django.core.urlresolvers import reverse from django.test.utils import override_settings from mock import patch +from pytz import UTC from certificates.api import get_certificate_url # pylint: disable=import-error from certificates.models import CertificateStatuses # pylint: disable=import-error @@ -22,6 +24,9 @@ from xmodule.modulestore.tests.factories import CourseFactory # pylint: disable=no-member +PAST_DATE = datetime.datetime.now(UTC) - datetime.timedelta(days=2) +FUTURE_DATE = datetime.datetime.now(UTC) + datetime.timedelta(days=2) + class CertificateDisplayTestBase(SharedModuleStoreTestCase): """Tests display of certificates on the student dashboard. """ @@ -57,13 +62,16 @@ class CertificateDisplayTestBase(SharedModuleStoreTestCase): 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) + CourseEnrollmentFactory.create( + user=self.user, + course_id=self.course.id, + mode=enrollment_mode) return GeneratedCertificateFactory( user=self.user, course_id=self.course.id, mode=enrollment_mode, download_url=self.DOWNLOAD_URL, - status="downloadable", + status=CertificateStatuses.downloadable, grade=0.98, ) @@ -96,6 +104,44 @@ class CertificateDisplayTestBase(SharedModuleStoreTestCase): self.assertNotContains(response, self.DOWNLOAD_URL) +@ddt.ddt +@unittest.skipUnless(settings.ROOT_URLCONF == 'lms.urls', 'Test only valid in lms') +class CertificateDashboardMessageDisplayTest(CertificateDisplayTestBase): + """ + Tests the certificates messages for a course in the dashboard. + """ + + @classmethod + def setUpClass(cls): + super(CertificateDashboardMessageDisplayTest, cls).setUpClass() + cls.course.certificates_display_behavior = "end" + cls.course.save() + cls.store.update_item(cls.course, cls.USERNAME) + + def _check_message(self, certificate_available_date): + response = self.client.get(reverse('dashboard')) + + if datetime.datetime.now(UTC) < certificate_available_date: + self.assertContains(response, u"Your certificate will be available on") + self.assertNotContains(response, u"View Test_Certificate") + else: + self._check_can_download_certificate() + + @ddt.data(False) + def test_certificate_available_date(self, past_certificate_available_date): + cert = self._create_certificate('verified') + cert.status = CertificateStatuses.downloadable + cert.save() + + certificate_available_date = PAST_DATE if past_certificate_available_date else FUTURE_DATE + + self.course.certificate_available_date = certificate_available_date + self.course.save() + self.store.update_item(self.course, self.USERNAME) + + self._check_message(certificate_available_date) + + @ddt.ddt @unittest.skipUnless(settings.ROOT_URLCONF == 'lms.urls', 'Test only valid in lms') class CertificateDisplayTest(CertificateDisplayTestBase): diff --git a/common/djangoapps/student/tests/test_views.py b/common/djangoapps/student/tests/test_views.py index 4dc046093a..8984a57a57 100644 --- a/common/djangoapps/student/tests/test_views.py +++ b/common/djangoapps/student/tests/test_views.py @@ -48,26 +48,27 @@ class TestStudentDashboardUnenrollments(SharedModuleStoreTestCase): super(TestStudentDashboardUnenrollments, self).setUp() self.user = UserFactory() self.enrollment = CourseEnrollmentFactory(course_id=self.course.id, user=self.user) - self.cert_status = None + self.cert_status = 'processing' self.client.login(username=self.user.username, password=PASSWORD) def mock_cert(self, _user, _course_overview, _course_mode): """ Return a preset certificate status. """ - if self.cert_status is not None: - return { - 'status': self.cert_status, - 'can_unenroll': self.cert_status not in DISABLE_UNENROLL_CERT_STATES - } - else: - return {} + return { + 'status': self.cert_status, + 'can_unenroll': self.cert_status not in DISABLE_UNENROLL_CERT_STATES, + 'certificate_message_viewable': True, + 'grade': 100, + 'show_disabled_download_button': False, + 'show_download_url': False, + 'show_survey_button': False + } @ddt.data( ('notpassing', 1), ('restricted', 1), ('processing', 1), - (None, 1), ('generating', 0), - ('ready', 0), + ('downloadable', 0), ) @ddt.unpack def test_unenroll_available(self, cert_status, unenroll_action_count): @@ -83,9 +84,8 @@ class TestStudentDashboardUnenrollments(SharedModuleStoreTestCase): ('notpassing', 200), ('restricted', 200), ('processing', 200), - (None, 200), ('generating', 400), - ('ready', 400), + ('downloadable', 400), ) @ddt.unpack @patch.object(CourseEnrollment, 'unenroll') @@ -108,16 +108,9 @@ class TestStudentDashboardUnenrollments(SharedModuleStoreTestCase): else: course_enrollment.assert_not_called() - def test_no_cert_status(self): - """ Assert that the dashboard loads when cert_status is None.""" - with patch('student.views.cert_info', return_value=None): - response = self.client.get(reverse('dashboard')) - - self.assertEqual(response.status_code, 200) - def test_cant_unenroll_status(self): """ Assert that the dashboard loads when cert_status does not allow for unenrollment""" - with patch('certificates.models.certificate_status_for_student', return_value={'status': 'ready'}): + with patch('certificates.models.certificate_status_for_student', return_value={'status': 'downloadable'}): response = self.client.get(reverse('dashboard')) self.assertEqual(response.status_code, 200) diff --git a/common/djangoapps/student/tests/tests.py b/common/djangoapps/student/tests/tests.py index f48b16979f..f22180c8ea 100644 --- a/common/djangoapps/student/tests/tests.py +++ b/common/djangoapps/student/tests/tests.py @@ -84,6 +84,7 @@ class CourseEndingTest(TestCase): { 'status': 'processing', 'show_disabled_download_button': False, + 'certificate_message_viewable': False, 'show_download_url': False, 'show_survey_button': False, 'can_unenroll': True, @@ -96,6 +97,7 @@ class CourseEndingTest(TestCase): { 'status': 'processing', 'show_disabled_download_button': False, + 'certificate_message_viewable': True, 'show_download_url': False, 'show_survey_button': False, 'mode': None, @@ -112,6 +114,7 @@ class CourseEndingTest(TestCase): { 'status': 'generating', 'show_disabled_download_button': True, + 'certificate_message_viewable': True, 'show_download_url': False, 'show_survey_button': True, 'survey_url': survey_url, @@ -128,6 +131,7 @@ class CourseEndingTest(TestCase): { 'status': 'generating', 'show_disabled_download_button': True, + 'certificate_message_viewable': True, 'show_download_url': False, 'show_survey_button': True, 'survey_url': survey_url, @@ -140,7 +144,8 @@ class CourseEndingTest(TestCase): download_url = 'http://s3.edx/cert' cert_status = { - 'status': 'downloadable', 'grade': '0.67', + 'status': 'downloadable', + 'grade': '0.67', 'download_url': download_url, 'mode': 'honor' } @@ -148,8 +153,9 @@ class CourseEndingTest(TestCase): self.assertEqual( _cert_info(user, course, cert_status, course_mode), { - 'status': 'ready', + 'status': 'downloadable', 'show_disabled_download_button': False, + 'certificate_message_viewable': True, 'show_download_url': True, 'download_url': download_url, 'show_survey_button': True, @@ -171,6 +177,7 @@ class CourseEndingTest(TestCase): { 'status': 'notpassing', 'show_disabled_download_button': False, + 'certificate_message_viewable': True, 'show_download_url': False, 'show_survey_button': True, 'survey_url': survey_url, @@ -192,6 +199,7 @@ class CourseEndingTest(TestCase): { 'status': 'notpassing', 'show_disabled_download_button': False, + 'certificate_message_viewable': True, 'show_download_url': False, 'show_survey_button': False, 'grade': '0.67', @@ -251,6 +259,7 @@ class CourseEndingTest(TestCase): { 'status': 'generating', 'show_disabled_download_button': True, + 'certificate_message_viewable': True, 'show_download_url': False, 'show_survey_button': True, 'survey_url': survey_url, @@ -283,6 +292,7 @@ class CourseEndingTest(TestCase): _cert_info(user, course, cert_status, course_mode), { 'status': 'processing', + 'certificate_message_viewable': False, 'show_disabled_download_button': False, 'show_download_url': False, 'show_survey_button': False, diff --git a/common/djangoapps/student/views.py b/common/djangoapps/student/views.py index 47223681f2..d992641c98 100644 --- a/common/djangoapps/student/views.py +++ b/common/djangoapps/student/views.py @@ -12,7 +12,6 @@ from urlparse import parse_qs, urlsplit, urlunsplit import analytics import edx_oauth2_provider -import waffle from django.conf import settings from django.contrib import messages from django.contrib.auth import authenticate, login, logout @@ -73,6 +72,7 @@ from lms.djangoapps.verify_student.models import SoftwareSecurePhotoVerification from notification_prefs.views import enable_notifications from openedx.core.djangoapps import monitoring_utils from openedx.core.djangoapps.catalog.utils import get_programs_with_type +from openedx.core.djangoapps.certificates.api import certificates_viewable_for_course from openedx.core.djangoapps.credit.email_utils import get_credit_provider_display_names, make_providers_strings from openedx.core.djangoapps.embargo import api as embargo_api from openedx.core.djangoapps.external_auth.login_and_register import login as external_auth_login @@ -233,9 +233,10 @@ def cert_info(user, course_overview, course_mode): course_mode (str): The enrollment mode (honor, verified, audit, etc.) Returns: - dict: Empty dict if certificates are disabled or hidden, or a dictionary with keys: - 'status': one of 'generating', 'ready', 'notpassing', 'processing', 'restricted' + dict: A dictionary with keys: + 'status': one of 'generating', 'downloadable', 'notpassing', 'processing', 'restricted' 'show_download_url': bool + 'certificate_message_viewable': bool -- if certificates are viewable 'download_url': url, only present if show_download_url is True 'show_disabled_download_button': bool -- true if state is 'generating' 'show_survey_button': bool @@ -243,9 +244,6 @@ def cert_info(user, course_overview, course_mode): 'grade': if status is not 'processing' 'can_unenroll': if status allows for unenrollment """ - if not course_overview.may_certify(): - return {} - # Note: this should be rewritten to use the certificates API return _cert_info( user, course_overview, @@ -328,7 +326,7 @@ def _cert_info(user, course_overview, cert_status, course_mode): # pylint: disa # simplify the status for the template using this lookup table template_state = { CertificateStatuses.generating: 'generating', - CertificateStatuses.downloadable: 'ready', + CertificateStatuses.downloadable: 'downloadable', CertificateStatuses.notpassing: 'notpassing', CertificateStatuses.restricted: 'restricted', CertificateStatuses.auditing: 'auditing', @@ -341,6 +339,7 @@ def _cert_info(user, course_overview, cert_status, course_mode): # pylint: disa default_info = { 'status': default_status, + 'certificate_message_viewable': False, 'show_disabled_download_button': False, 'show_download_url': False, 'show_survey_button': False, @@ -359,14 +358,15 @@ def _cert_info(user, course_overview, cert_status, course_mode): # pylint: disa status_dict = { 'status': status, - 'show_download_url': status == 'ready', + 'certificate_message_viewable': certificates_viewable_for_course(course_overview), + 'show_download_url': status == 'downloadable', 'show_disabled_download_button': status == 'generating', 'mode': cert_status.get('mode', None), 'linked_in_url': None, 'can_unenroll': status not in DISABLE_UNENROLL_CERT_STATES, } - if (status in ('generating', 'ready', 'notpassing', 'restricted', 'auditing', 'unverified') and + if (status in ('generating', 'downloadable', 'notpassing', 'restricted', 'auditing', 'unverified') and course_overview.end_of_course_survey_url is not None): status_dict.update({ 'show_survey_button': True, @@ -374,8 +374,8 @@ def _cert_info(user, course_overview, cert_status, course_mode): # pylint: disa else: status_dict['show_survey_button'] = False - if status == 'ready': - # showing the certificate web view button if certificate is ready state and feature flags are enabled. + if status == 'downloadable': + # showing the certificate web view button if certificate is downloadable state and feature flags are enabled. if has_html_certificates_enabled(course_overview): if course_overview.has_any_active_web_certificate: status_dict.update({ @@ -410,7 +410,7 @@ def _cert_info(user, course_overview, cert_status, course_mode): # pylint: disa cert_status['download_url'] ) - if status in {'generating', 'ready', 'notpassing', 'restricted', 'auditing', 'unverified'}: + if status in {'generating', 'downloadable', 'notpassing', 'restricted', 'auditing', 'unverified'}: cert_grade_percent = -1 persisted_grade_percent = -1 persisted_grade = CourseGradeFactory().read(user, course=course_overview, create_if_needed=False) diff --git a/lms/templates/dashboard/_dashboard_certificate_information.html b/lms/templates/dashboard/_dashboard_certificate_information.html index 60d41066c0..c7a3b8be17 100644 --- a/lms/templates/dashboard/_dashboard_certificate_information.html +++ b/lms/templates/dashboard/_dashboard_certificate_information.html @@ -18,103 +18,132 @@ from course_modes.models import CourseMode %> <% -if cert_status['status'] == 'generating': +if (cert_status['status'] == 'generating' or cert_status['status'] == 'downloadable') and not cert_status['certificate_message_viewable']: + status_css_class = 'course-status-processing' +elif cert_status['status'] == 'generating': status_css_class = 'course-status-certrendering' -elif cert_status['status'] == 'ready': +elif cert_status['status'] == 'downloadable': status_css_class = 'course-status-certavailable' elif cert_status['status'] == 'notpassing': status_css_class = 'course-status-certnotavailable' else: status_css_class = 'course-status-processing' %> -% if not (cert_status['status'] == 'processing' and course_overview.self_paced): + +% if cert_status['status'] != 'processing':
- % if cert_status['status'] == 'processing' and not course_overview.self_paced: -

${_("Final course details are being wrapped up at this time. Your final standing will be available shortly.")}

- % elif cert_status['status'] in ('generating', 'ready', 'notpassing', 'restricted', 'auditing', 'unverified'): -

${_("Your final grade:")} - ${"{0:.0f}%".format(float(cert_status['grade'])*100)}. - % if cert_status['status'] == 'notpassing': - % if enrollment.mode != 'audit': - ${_("Grade required for a {cert_name_short}:").format(cert_name_short=cert_name_short)} - % else: - ${_("Grade required to pass this course:")} - % endif - ${"{0:.0f}%".format(float(course_overview.lowest_passing_grade)*100)}. - % elif cert_status['status'] == 'restricted' and enrollment.mode == 'verified': -

- ${Text(_("Your verified {cert_name_long} is being held pending confirmation that the issuance of your {cert_name_short} is in compliance with strict U.S. embargoes on Iran, Cuba, Syria and Sudan. If you think our system has mistakenly identified you as being connected with one of those countries, please let us know by contacting {email}. If you would like a refund on your {cert_name_long}, please contact our billing address {billing_email}")).format(email=HTML('{email}.').format(email=settings.CONTACT_EMAIL), billing_email=HTML('{email}').format(email=settings.PAYMENT_SUPPORT_EMAIL), cert_name_short=cert_name_short, cert_name_long=cert_name_long)} -

- % elif cert_status['status'] == 'restricted': -

- ${Text(_("Your {cert_name_long} is being held pending confirmation that the issuance of your {cert_name_short} is in compliance with strict U.S. embargoes on Iran, Cuba, Syria and Sudan. If you think our system has mistakenly identified you as being connected with one of those countries, please let us know by contacting {email}.")).format(email=HTML('{email}.').format(email=settings.CONTACT_EMAIL), cert_name_short=cert_name_short, cert_name_long=cert_name_long)} -

- % elif cert_status['status'] == 'unverified': -

- ${Text(_("Your certificate was not issued because you do not have a current verified identity with {platform_name}. ")).format(platform_name=settings.PLATFORM_NAME)} - ${Text(_("Verify your identity now."))} -

+ % if not cert_status['certificate_message_viewable']: + % if (cert_status['status'] == 'generating' or cert_status['status'] == 'downloadable'): +

+ <% + certificate_available_date_string = course_overview.certificate_available_date.strftime('%Y-%m-%dT%H:%M:%S%z') + container_string = _("Your certificate will be available on or before {date}") + format = 'shortDate' + %> + +

% endif -

- % endif + % else: +

${_("Your final grade:")} + ${"{0:.0f}%".format(float(cert_status['grade'])*100)}. - % if cert_status['show_disabled_download_button'] or cert_status['show_download_url'] or cert_status['show_survey_button']: -

- -
+ % endif - % if cert_status['show_download_url'] and cert_status['linked_in_url']: - - % endif + % if cert_status['show_download_url'] and cert_status['certificate_message_viewable'] and enrollment.mode == 'verified' and cert_status['mode'] == 'honor': +
+ ${_('Since we did not have a valid set of verification photos from you when your {cert_name_long} was generated, we could not grant you a verified {cert_name_short}. An honor code {cert_name_short} has been granted instead.').format(cert_name_short=cert_name_short, cert_name_long=cert_name_long)} +
+ % endif - % if cert_status['show_download_url'] and enrollment.mode == 'verified' and cert_status['mode'] == 'honor': -
- ${_('Since we did not have a valid set of verification photos from you when your {cert_name_long} was generated, we could not grant you a verified {cert_name_short}. An honor code {cert_name_short} has been granted instead.').format(cert_name_short=cert_name_short, cert_name_long=cert_name_long)} -
% endif - % endif
% endif diff --git a/lms/templates/dashboard/_dashboard_course_listing.html b/lms/templates/dashboard/_dashboard_course_listing.html index 31c3ca4c0b..6430c8e5e9 100644 --- a/lms/templates/dashboard/_dashboard_course_listing.html +++ b/lms/templates/dashboard/_dashboard_course_listing.html @@ -281,9 +281,7 @@ from util.course import get_link_for_about_page, get_encoded_course_sharing_utm_ % endif - % if course_overview.may_certify() and cert_status: <%include file='_dashboard_certificate_information.html' args='cert_status=cert_status,course_overview=course_overview, enrollment=enrollment, reverify_link=reverify_link'/> - % endif % if credit_status is not None: <%include file="_dashboard_credit_info.html" args="credit_status=credit_status"/>