diff --git a/lms/djangoapps/course_home_api/dates/v1/tests/test_views.py b/lms/djangoapps/course_home_api/dates/v1/tests/test_views.py index 34d4905afb..8d18a36aa6 100644 --- a/lms/djangoapps/course_home_api/dates/v1/tests/test_views.py +++ b/lms/djangoapps/course_home_api/dates/v1/tests/test_views.py @@ -31,15 +31,10 @@ class DatesTabTestViews(BaseCourseHomeTests): response = self.client.get(self.url) self.assertEqual(response.status_code, 200) - # Pulling out the date blocks to check learner has access. The Verification Deadline Date - # should not be accessible to the audit learner, but accessible to the verified learner. + # Pulling out the date blocks to check learner has access. date_blocks = response.data.get('course_date_blocks') - if enrollment_mode == CourseMode.AUDIT: - self.assertFalse(response.data.get('learner_is_verified')) - self.assertTrue(any(block.get('learner_has_access') is False for block in date_blocks)) - else: - self.assertTrue(response.data.get('learner_is_verified')) - self.assertTrue(all(block.get('learner_has_access') for block in date_blocks)) + self.assertEqual(response.data.get('learner_is_verified'), enrollment_mode == CourseMode.VERIFIED) + self.assertTrue(all(block.get('learner_has_access') for block in date_blocks)) @COURSE_HOME_MICROFRONTEND.override(active=True) @COURSE_HOME_MICROFRONTEND_DATES_TAB.override(active=True) diff --git a/lms/djangoapps/courseware/courses.py b/lms/djangoapps/courseware/courses.py index 075b7e5b1a..f87c0004e3 100644 --- a/lms/djangoapps/courseware/courses.py +++ b/lms/djangoapps/courseware/courses.py @@ -47,7 +47,6 @@ from lms.djangoapps.courseware.masquerade import check_content_start_date_for_ma from lms.djangoapps.courseware.model_data import FieldDataCache from lms.djangoapps.courseware.module_render import get_module from edxmako.shortcuts import render_to_string -from lms.djangoapps.certificates import api as certs_api from lms.djangoapps.courseware.access_utils import ( check_authentication, check_enrollment, @@ -463,7 +462,6 @@ def get_course_date_blocks(course, user, request=None, include_access=False, """ blocks = [] if RELATIVE_DATES_FLAG.is_enabled(course.id): - blocks.append(CourseExpiredDate(course, user)) blocks.extend(get_course_assignment_date_blocks( course, user, request, num_return=num_assignments, include_access=include_access, include_past_dates=include_past_dates, @@ -472,18 +470,18 @@ def get_course_date_blocks(course, user, request=None, include_access=False, # Adding these in after the assignment blocks so in the case multiple blocks have the same date, # these blocks will be sorted to come after the assignments. See https://openedx.atlassian.net/browse/AA-158 default_block_classes = [ + CertificateAvailableDate, CourseEndDate, + CourseExpiredDate, CourseStartDate, TodaysDate, VerificationDeadlineDate, VerifiedUpgradeDeadlineDate, ] - if not course.self_paced and certs_api.get_active_web_certificate(course): - default_block_classes.insert(0, CertificateAvailableDate) - blocks.extend([cls(course, user) for cls in default_block_classes]) - return sorted((b for b in blocks if b.date and (b.is_enabled or include_past_dates)), key=date_block_key_fn) + blocks = filter(lambda b: b.is_allowed and b.date and (include_past_dates or b.is_enabled), blocks) + return sorted(blocks, key=date_block_key_fn) def date_block_key_fn(block): diff --git a/lms/djangoapps/courseware/date_summary.py b/lms/djangoapps/courseware/date_summary.py index f401f96f25..a03f248ef5 100644 --- a/lms/djangoapps/courseware/date_summary.py +++ b/lms/djangoapps/courseware/date_summary.py @@ -20,6 +20,7 @@ from lazy import lazy from pytz import utc from course_modes.models import CourseMode, get_cosmetic_verified_display_price +from lms.djangoapps.certificates.api import get_active_web_certificate from lms.djangoapps.courseware.utils import verified_upgrade_deadline_link, can_show_verified_upgrade from lms.djangoapps.verify_student.models import VerificationDeadline from lms.djangoapps.verify_student.services import IDVerificationService @@ -140,6 +141,19 @@ class DateSummary(object): absolute='{date}', ) + @lazy + def is_allowed(self): + """ + Whether or not this summary block is applicable or active for its course. + + For example, a DateSummary might only make sense for a self-paced course, and + you could restrict it here. + + You should not make time-sensitive checks here. That sort of thing belongs in + is_enabled. + """ + return True + @property def is_enabled(self): """ @@ -148,9 +162,10 @@ class DateSummary(object): By default, the summary is only shown if its date is in the future. """ - if self.date is not None: - return self.current_time.date() <= self.date.date() - return False + return ( + self.date is not None and + self.current_time.date() <= self.date.date() + ) def deadline_has_passed(self): """ @@ -294,10 +309,7 @@ class CourseEndDate(DateSummary): """ css_class = 'end-date' title = ugettext_lazy('Course End') - - @property - def is_enabled(self): - return self.date is not None + is_enabled = True @property def description(self): @@ -432,6 +444,10 @@ class CourseExpiredDate(DateSummary): def title(self): return _('Audit Access Expires') + @lazy + def is_allowed(self): + return RELATIVE_DATES_FLAG.is_enabled(self.course.id) + class CertificateAvailableDate(DateSummary): """ @@ -440,21 +456,12 @@ class CertificateAvailableDate(DateSummary): css_class = 'certificate-available-date' title = ugettext_lazy('Certificate Available') - @property - def active_certificates(self): - return [ - certificate for certificate in self.course.certificates.get('certificates', []) - if certificate.get('is_active', False) - ] - - @property - def is_enabled(self): + @lazy + def is_allowed(self): return ( can_show_certificate_available_date_field(self.course) and self.has_certificate_modes and - self.date is not None and - self.current_time <= self.date and - len(self.active_certificates) > 0 + get_active_web_certificate(self.course) ) @property @@ -519,18 +526,8 @@ class VerifiedUpgradeDeadlineDate(DateSummary): def enrollment(self): return CourseEnrollment.get_enrollment(self.user, self.course_id) - @property - def is_enabled(self): - """ - Whether or not this summary block should be shown. - - By default, the summary is only shown if it has date and the date is in the - future and the user's enrollment is in upsell modes - """ - is_enabled = super(VerifiedUpgradeDeadlineDate, self).is_enabled - if not is_enabled: - return False - + @lazy + def is_allowed(self): return can_show_verified_upgrade(self.user, self.enrollment, self.course) @lazy @@ -627,6 +624,7 @@ class VerificationDeadlineDate(DateSummary): Displays the date by which the user must complete the verification process. """ + is_enabled = True @property def css_class(self): @@ -685,13 +683,13 @@ class VerificationDeadlineDate(DateSummary): return 'verification-deadline-date' @lazy - def is_enabled(self): - if self.date is None: - return False - (mode, is_active) = CourseEnrollment.enrollment_mode_for_user(self.user, self.course_id) - if is_active and mode == 'verified': - return self.verification_status in ('expired', 'none', 'must_reverify') - return False + def is_allowed(self): + mode, is_active = CourseEnrollment.enrollment_mode_for_user(self.user, self.course_id) + return ( + is_active and + mode == 'verified' and + self.verification_status in ('expired', 'none', 'must_reverify') + ) @lazy def verification_status(self): diff --git a/lms/djangoapps/courseware/tests/test_date_summary.py b/lms/djangoapps/courseware/tests/test_date_summary.py index 1ca4e79435..d52a2aaca5 100644 --- a/lms/djangoapps/courseware/tests/test_date_summary.py +++ b/lms/djangoapps/courseware/tests/test_date_summary.py @@ -329,6 +329,7 @@ class CourseDateSummaryTest(SharedModuleStoreTestCase): user = create_user() block = TodaysDate(course, user) self.assertTrue(block.is_enabled) + self.assertTrue(block.is_allowed) self.assertEqual(block.date, datetime.now(utc)) self.assertEqual(block.title, 'current_datetime') @@ -505,7 +506,7 @@ class CourseDateSummaryTest(SharedModuleStoreTestCase): CourseEnrollmentFactory(course_id=course.id, user=user, mode=CourseMode.AUDIT) block = CertificateAvailableDate(course, user) self.assertEqual(block.date, None) - self.assertFalse(block.is_enabled) + self.assertFalse(block.is_allowed) ## CertificateAvailableDate @waffle.testutils.override_switch('certificates.auto_certificate_generation', True) @@ -517,7 +518,7 @@ class CourseDateSummaryTest(SharedModuleStoreTestCase): course.save() block = CertificateAvailableDate(course, verified_user) self.assertNotEqual(block.date, None) - self.assertFalse(block.is_enabled) + self.assertFalse(block.is_allowed) def test_no_certificate_available_date_for_audit_course(self): """ @@ -539,7 +540,7 @@ class CourseDateSummaryTest(SharedModuleStoreTestCase): # Verify Certificate Available Date is not enabled for learner. block = CertificateAvailableDate(course, audit_user) - self.assertFalse(block.is_enabled) + self.assertFalse(block.is_allowed) self.assertNotEqual(block.date, None) @waffle.testutils.override_switch('certificates.auto_certificate_generation', True) @@ -558,7 +559,7 @@ class CourseDateSummaryTest(SharedModuleStoreTestCase): for block in (CertificateAvailableDate(course, audit_user), CertificateAvailableDate(course, verified_user)): self.assertIsNotNone(course.certificate_available_date) self.assertEqual(block.date, course.certificate_available_date) - self.assertTrue(block.is_enabled) + self.assertTrue(block.is_allowed) ## VerificationDeadlineDate def test_no_verification_deadline(self): @@ -566,14 +567,15 @@ class CourseDateSummaryTest(SharedModuleStoreTestCase): user = create_user() CourseEnrollmentFactory(course_id=course.id, user=user, mode=CourseMode.VERIFIED) block = VerificationDeadlineDate(course, user) - self.assertFalse(block.is_enabled) + self.assertIsNone(block.date) + self.assertTrue(block.is_allowed) def test_no_verified_enrollment(self): course = create_course_run(days_till_start=-1) user = create_user() CourseEnrollmentFactory(course_id=course.id, user=user, mode=CourseMode.AUDIT) block = VerificationDeadlineDate(course, user) - self.assertFalse(block.is_enabled) + self.assertFalse(block.is_allowed) def test_verification_deadline_date_upcoming(self): with freeze_time('2015-01-02'): diff --git a/lms/djangoapps/courseware/tests/test_views.py b/lms/djangoapps/courseware/tests/test_views.py index 83674c543e..68858852d6 100644 --- a/lms/djangoapps/courseware/tests/test_views.py +++ b/lms/djangoapps/courseware/tests/test_views.py @@ -3083,6 +3083,7 @@ class DatesTabTestCase(ModuleStoreTestCase): self.course = CourseFactory.create(start=now + timedelta(days=-1), self_paced=True) self.course.end = now + timedelta(days=3) + ContentTypeGatingConfig.objects.create(enabled=True, enabled_as_of=datetime(2018, 1, 1)) CourseModeFactory(course_id=self.course.id, mode_slug=CourseMode.AUDIT) CourseModeFactory( course_id=self.course.id, @@ -3116,6 +3117,8 @@ class DatesTabTestCase(ModuleStoreTestCase): due=now + timedelta(days=1), # Setting this to tomorrow so it'll show the 'Due Next' pill graded=True, ) + vertical = ItemFactory.create(category='vertical', parent_location=subsection.location) + ItemFactory.create(category='problem', parent_location=vertical.location) with patch('lms.djangoapps.courseware.views.views.get_enrollment') as mock_get_enrollment: mock_get_enrollment.return_value = { @@ -3123,7 +3126,7 @@ class DatesTabTestCase(ModuleStoreTestCase): } response = self._get_response(self.course) self.assertContains(response, subsection.display_name) - # Show the Verification Deadline for everyone + # Show the Verification Deadline for verified only self.assertContains(response, 'Verification Deadline') # Make sure pill exists for today's date self.assertContains(response, '
') @@ -3148,8 +3151,8 @@ class DatesTabTestCase(ModuleStoreTestCase): mock_set_custom_metric.assert_has_calls(expected_calls, any_order=True) self.assertContains(response, subsection.display_name) - # Show the Verification Deadline for everyone - self.assertContains(response, 'Verification Deadline') + # Don't show the Verification Deadline for audit + self.assertNotContains(response, 'Verification Deadline') # Pill doesn't exist for assignment due tomorrow self.assertNotContains(response, '
') # Should have verified pills for audit enrollments diff --git a/openedx/features/course_experience/tests/views/test_course_home.py b/openedx/features/course_experience/tests/views/test_course_home.py index 577a5b1cb0..85f1600a73 100644 --- a/openedx/features/course_experience/tests/views/test_course_home.py +++ b/openedx/features/course_experience/tests/views/test_course_home.py @@ -219,7 +219,7 @@ class TestCourseHomePage(CourseHomePageTestCase): # Fetch the view and verify the query counts # TODO: decrease query count as part of REVO-28 - with self.assertNumQueries(77, table_blacklist=QUERY_COUNT_TABLE_BLACKLIST): + with self.assertNumQueries(75, table_blacklist=QUERY_COUNT_TABLE_BLACKLIST): with check_mongo_calls(4): url = course_home_url(self.course) self.client.get(url)