Merge pull request #16109 from edx/ssemenova/ed-1315-part2

Remove "final course details" message, replace with another

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

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,
         )
 
@@ -98,6 +106,44 @@ class CertificateDisplayTestBase(SharedModuleStoreTestCase):
 
 @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):
     """
     Tests of certificate display.

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)

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,

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)

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_
       </div>
       % 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"/>