From 0d62895be42fdee393a0b8bb0ecf4ce4269f6246 Mon Sep 17 00:00:00 2001 From: Diana Huang Date: Thu, 7 Aug 2014 14:58:04 -0400 Subject: [PATCH 1/2] Modify certificates_show_before_end_date behavior Hide the certificate notification box when there's no certificate information, even when the flag certificates_show_before_end_date is turned on. ECOM-11 --- common/djangoapps/student/tests/tests.py | 18 +++++++++++++++++- common/djangoapps/student/views.py | 3 +++ 2 files changed, 20 insertions(+), 1 deletion(-) diff --git a/common/djangoapps/student/tests/tests.py b/common/djangoapps/student/tests/tests.py index 9afc044f57..1f1baa7002 100644 --- a/common/djangoapps/student/tests/tests.py +++ b/common/djangoapps/student/tests/tests.py @@ -54,7 +54,7 @@ class CourseEndingTest(TestCase): def test_cert_info(self): user = Mock(username="fred") survey_url = "http://a_survey.com" - course = Mock(end_of_course_survey_url=survey_url) + course = Mock(end_of_course_survey_url=survey_url, certificates_show_before_end=False) self.assertEqual(_cert_info(user, course, None), {'status': 'processing', @@ -133,6 +133,22 @@ class CourseEndingTest(TestCase): 'mode': 'honor' }) + # test when the status is true, we get the correct results out + course2.certificates_show_before_end = True + cert_status = {'status': 'unavailable'} + self.assertIsNone(_cert_info(user, course2, cert_status)) + + cert_status = {'status': 'notpassing', 'grade': '67', + 'download_url': download_url, 'mode': 'honor'} + self.assertEqual(_cert_info(user, course2, cert_status), + {'status': 'notpassing', + 'show_disabled_download_button': False, + 'show_download_url': False, + 'show_survey_button': False, + 'grade': '67', + 'mode': 'honor' + }) + @override_settings(MODULESTORE=TEST_DATA_MIXED_MODULESTORE) class DashboardTest(TestCase): diff --git a/common/djangoapps/student/views.py b/common/djangoapps/student/views.py index d5a2fc2314..ec049b0b65 100644 --- a/common/djangoapps/student/views.py +++ b/common/djangoapps/student/views.py @@ -285,6 +285,9 @@ def _cert_info(user, course, cert_status): if cert_status is None: return default_info + if cert_status['status'] == 'unavailable' and course.certificates_show_before_end: + return None + status = template_state.get(cert_status['status'], default_status) d = {'status': status, From 7865e2fb3c453401d5e3321b45dc3559a6b57cf8 Mon Sep 17 00:00:00 2001 From: Diana Huang Date: Fri, 8 Aug 2014 10:32:27 -0400 Subject: [PATCH 2/2] Move certificates_show_before_end into a new variable Mark it as deprecated, but maintain backwards compatibility. ECOM-11 --- common/djangoapps/student/tests/tests.py | 15 ++++----------- common/djangoapps/student/views.py | 4 +++- common/lib/xmodule/xmodule/course_module.py | 17 +++++++++++++---- .../xmodule/tests/test_course_module.py | 18 ++++++++++++------ 4 files changed, 32 insertions(+), 22 deletions(-) diff --git a/common/djangoapps/student/tests/tests.py b/common/djangoapps/student/tests/tests.py index 1f1baa7002..abb1903da7 100644 --- a/common/djangoapps/student/tests/tests.py +++ b/common/djangoapps/student/tests/tests.py @@ -54,7 +54,7 @@ class CourseEndingTest(TestCase): def test_cert_info(self): user = Mock(username="fred") survey_url = "http://a_survey.com" - course = Mock(end_of_course_survey_url=survey_url, certificates_show_before_end=False) + course = Mock(end_of_course_survey_url=survey_url, certificates_display_behavior='end') self.assertEqual(_cert_info(user, course, None), {'status': 'processing', @@ -133,21 +133,14 @@ class CourseEndingTest(TestCase): 'mode': 'honor' }) - # test when the status is true, we get the correct results out - course2.certificates_show_before_end = True + # test when the display is unavailable or notpassing, we get the correct results out + course2.certificates_display_behavior = 'early_no_info' cert_status = {'status': 'unavailable'} self.assertIsNone(_cert_info(user, course2, cert_status)) cert_status = {'status': 'notpassing', 'grade': '67', 'download_url': download_url, 'mode': 'honor'} - self.assertEqual(_cert_info(user, course2, cert_status), - {'status': 'notpassing', - 'show_disabled_download_button': False, - 'show_download_url': False, - 'show_survey_button': False, - 'grade': '67', - 'mode': 'honor' - }) + self.assertIsNone(_cert_info(user, course2, cert_status)) @override_settings(MODULESTORE=TEST_DATA_MIXED_MODULESTORE) diff --git a/common/djangoapps/student/views.py b/common/djangoapps/student/views.py index ec049b0b65..4abdc0861c 100644 --- a/common/djangoapps/student/views.py +++ b/common/djangoapps/student/views.py @@ -285,7 +285,9 @@ def _cert_info(user, course, cert_status): if cert_status is None: return default_info - if cert_status['status'] == 'unavailable' and course.certificates_show_before_end: + is_hidden_status = cert_status['status'] in ('unavailable', 'processing', 'generating', 'notpassing') + + if course.certificates_display_behavior == 'early_no_info' and is_hidden_status: return None status = template_state.get(cert_status['status'], default_status) diff --git a/common/lib/xmodule/xmodule/course_module.py b/common/lib/xmodule/xmodule/course_module.py index 1d71819f2d..2c7d84acde 100644 --- a/common/lib/xmodule/xmodule/course_module.py +++ b/common/lib/xmodule/xmodule/course_module.py @@ -219,8 +219,8 @@ class CourseFields(object): scope=Scope.settings ) display_name = String( - help=_("Enter the name of the course as it should appear in the edX.org course list."), - default="Empty", + help=_("Enter the name of the course as it should appear in the edX.org course list."), + default="Empty", display_name=_("Course Display Name"), scope=Scope.settings ) @@ -452,7 +452,15 @@ class CourseFields(object): display_name=_("Certificates Downloadable Before End"), help=_("Enter true or false. If true, students can download certificates before the course ends, if they've met certificate requirements."), scope=Scope.settings, - default=False + default=False, + deprecated=True + ) + + certificates_display_behavior = String( + display_name=_("Certificates Display Behavior"), + help=_("Has three possible states: 'end', 'early_with_info', 'early_no_info'. 'end' is the default behavior, where certificates will only appear after a course has ended. 'early_with_info' will display all certificate information before a course has ended. 'early_no_info' will hide all certificate information unless a student has earned a certificate."), + scope=Scope.settings, + default="end" ) course_image = String( display_name=_("Course About Page Image"), @@ -711,7 +719,8 @@ class CourseDescriptor(CourseFields, SequenceDescriptor): """ Return True if it is acceptable to show the student a certificate download link """ - return self.certificates_show_before_end or self.has_ended() + show_early = self.certificates_display_behavior in ('early_with_info', 'early_no_info') or self.certificates_show_before_end + return show_early or self.has_ended() def has_started(self): return datetime.now(UTC()) > self.start diff --git a/common/lib/xmodule/xmodule/tests/test_course_module.py b/common/lib/xmodule/xmodule/tests/test_course_module.py index 4aed11ece8..c430d672c8 100644 --- a/common/lib/xmodule/xmodule/tests/test_course_module.py +++ b/common/lib/xmodule/xmodule/tests/test_course_module.py @@ -49,7 +49,7 @@ class DummySystem(ImportSystem): ) -def get_dummy_course(start, announcement=None, is_new=None, advertised_start=None, end=None, certs=False): +def get_dummy_course(start, announcement=None, is_new=None, advertised_start=None, end=None, certs='end'): """Get a dummy course""" system = DummySystem(load_error_modules=True) @@ -70,7 +70,7 @@ def get_dummy_course(start, announcement=None, is_new=None, advertised_start=Non {is_new} {advertised_start} {end} - certificates_show_before_end="{certs}"> + certificates_display_behavior="{certs}"> Two houses, ... @@ -100,10 +100,12 @@ class HasEndedMayCertifyTestCase(unittest.TestCase): #""".format(org=ORG, course=COURSE) past_end = (datetime.now() - timedelta(days=12)).strftime("%Y-%m-%dT%H:%M:00") future_end = (datetime.now() + timedelta(days=12)).strftime("%Y-%m-%dT%H:%M:00") - self.past_show_certs = get_dummy_course("2012-01-01T12:00", end=past_end, certs=True) - self.past_noshow_certs = get_dummy_course("2012-01-01T12:00", end=past_end, certs=False) - self.future_show_certs = get_dummy_course("2012-01-01T12:00", end=future_end, certs=True) - self.future_noshow_certs = get_dummy_course("2012-01-01T12:00", end=future_end, certs=False) + self.past_show_certs = get_dummy_course("2012-01-01T12:00", end=past_end, certs='early_with_info') + self.past_show_certs_no_info = get_dummy_course("2012-01-01T12:00", end=past_end, certs='early_no_info') + self.past_noshow_certs = get_dummy_course("2012-01-01T12:00", end=past_end, certs='end') + self.future_show_certs = get_dummy_course("2012-01-01T12:00", end=future_end, certs='early_with_info') + self.future_show_certs_no_info = get_dummy_course("2012-01-01T12:00", end=future_end, certs='early_no_info') + self.future_noshow_certs = get_dummy_course("2012-01-01T12:00", end=future_end, certs='end') #self.past_show_certs = system.process_xml(sample_xml.format(end=past_end, cert=True)) #self.past_noshow_certs = system.process_xml(sample_xml.format(end=past_end, cert=False)) #self.future_show_certs = system.process_xml(sample_xml.format(end=future_end, cert=True)) @@ -112,15 +114,19 @@ class HasEndedMayCertifyTestCase(unittest.TestCase): def test_has_ended(self): """Check that has_ended correctly tells us when a course is over.""" self.assertTrue(self.past_show_certs.has_ended()) + self.assertTrue(self.past_show_certs_no_info.has_ended()) self.assertTrue(self.past_noshow_certs.has_ended()) self.assertFalse(self.future_show_certs.has_ended()) + self.assertFalse(self.future_show_certs_no_info.has_ended()) self.assertFalse(self.future_noshow_certs.has_ended()) def test_may_certify(self): """Check that may_certify correctly tells us when a course may wrap.""" self.assertTrue(self.past_show_certs.may_certify()) self.assertTrue(self.past_noshow_certs.may_certify()) + self.assertTrue(self.past_show_certs_no_info.may_certify()) self.assertTrue(self.future_show_certs.may_certify()) + self.assertTrue(self.future_show_certs_no_info.may_certify()) self.assertFalse(self.future_noshow_certs.may_certify())