From dfe189e9bbf069fd9da3f1a06249dd6e1e7af838 Mon Sep 17 00:00:00 2001 From: Matthew Piatetsky Date: Tue, 6 Nov 2018 16:34:46 -0500 Subject: [PATCH] change access_duration to be weeks_to_complete or MIN_DURATION for both SP and IP courses --- .../tests/test_field_override_performance.py | 54 +++++++-------- .../django_comment_client/base/tests.py | 8 +-- .../features/course_duration_limits/access.py | 14 ++-- .../tests/test_course_expiration.py | 67 ++++++++++++------- .../tests/views/test_course_home.py | 2 +- .../tests/views/test_course_updates.py | 2 +- 6 files changed, 79 insertions(+), 68 deletions(-) diff --git a/lms/djangoapps/ccx/tests/test_field_override_performance.py b/lms/djangoapps/ccx/tests/test_field_override_performance.py index bc2383a30c..3b58229b65 100644 --- a/lms/djangoapps/ccx/tests/test_field_override_performance.py +++ b/lms/djangoapps/ccx/tests/test_field_override_performance.py @@ -237,18 +237,18 @@ class TestFieldOverrideMongoPerformance(FieldOverridePerformanceTestCase): # # of sql queries to default, # # of mongo queries, # ) - ('no_overrides', 1, True, False): (20, 1), - ('no_overrides', 2, True, False): (20, 1), - ('no_overrides', 3, True, False): (20, 1), - ('ccx', 1, True, False): (20, 1), - ('ccx', 2, True, False): (20, 1), - ('ccx', 3, True, False): (20, 1), - ('no_overrides', 1, False, False): (20, 1), - ('no_overrides', 2, False, False): (20, 1), - ('no_overrides', 3, False, False): (20, 1), - ('ccx', 1, False, False): (20, 1), - ('ccx', 2, False, False): (20, 1), - ('ccx', 3, False, False): (20, 1), + ('no_overrides', 1, True, False): (21, 1), + ('no_overrides', 2, True, False): (21, 1), + ('no_overrides', 3, True, False): (21, 1), + ('ccx', 1, True, False): (21, 1), + ('ccx', 2, True, False): (21, 1), + ('ccx', 3, True, False): (21, 1), + ('no_overrides', 1, False, False): (21, 1), + ('no_overrides', 2, False, False): (21, 1), + ('no_overrides', 3, False, False): (21, 1), + ('ccx', 1, False, False): (21, 1), + ('ccx', 2, False, False): (21, 1), + ('ccx', 3, False, False): (21, 1), } @@ -260,19 +260,19 @@ class TestFieldOverrideSplitPerformance(FieldOverridePerformanceTestCase): __test__ = True TEST_DATA = { - ('no_overrides', 1, True, False): (20, 3), - ('no_overrides', 2, True, False): (20, 3), - ('no_overrides', 3, True, False): (20, 3), - ('ccx', 1, True, False): (20, 3), - ('ccx', 2, True, False): (20, 3), - ('ccx', 3, True, False): (20, 3), - ('ccx', 1, True, True): (21, 3), - ('ccx', 2, True, True): (21, 3), - ('ccx', 3, True, True): (21, 3), - ('no_overrides', 1, False, False): (20, 3), - ('no_overrides', 2, False, False): (20, 3), - ('no_overrides', 3, False, False): (20, 3), - ('ccx', 1, False, False): (20, 3), - ('ccx', 2, False, False): (20, 3), - ('ccx', 3, False, False): (20, 3), + ('no_overrides', 1, True, False): (21, 3), + ('no_overrides', 2, True, False): (21, 3), + ('no_overrides', 3, True, False): (21, 3), + ('ccx', 1, True, False): (21, 3), + ('ccx', 2, True, False): (21, 3), + ('ccx', 3, True, False): (21, 3), + ('ccx', 1, True, True): (22, 3), + ('ccx', 2, True, True): (22, 3), + ('ccx', 3, True, True): (22, 3), + ('no_overrides', 1, False, False): (21, 3), + ('no_overrides', 2, False, False): (21, 3), + ('no_overrides', 3, False, False): (21, 3), + ('ccx', 1, False, False): (21, 3), + ('ccx', 2, False, False): (21, 3), + ('ccx', 3, False, False): (21, 3), } diff --git a/lms/djangoapps/django_comment_client/base/tests.py b/lms/djangoapps/django_comment_client/base/tests.py index 730607787c..b6a8f235b9 100644 --- a/lms/djangoapps/django_comment_client/base/tests.py +++ b/lms/djangoapps/django_comment_client/base/tests.py @@ -405,8 +405,8 @@ class ViewsQueryCountTestCase( @override_waffle_flag(CONTENT_TYPE_GATING_FLAG, True) @ddt.data( - (ModuleStoreEnum.Type.mongo, 3, 4, 37), - (ModuleStoreEnum.Type.split, 3, 13, 37), + (ModuleStoreEnum.Type.mongo, 3, 4, 38), + (ModuleStoreEnum.Type.split, 3, 13, 38), ) @ddt.unpack @count_queries @@ -415,8 +415,8 @@ class ViewsQueryCountTestCase( @override_waffle_flag(CONTENT_TYPE_GATING_FLAG, True) @ddt.data( - (ModuleStoreEnum.Type.mongo, 3, 3, 33), - (ModuleStoreEnum.Type.split, 3, 10, 33), + (ModuleStoreEnum.Type.mongo, 3, 3, 34), + (ModuleStoreEnum.Type.split, 3, 10, 34), ) @ddt.unpack @count_queries diff --git a/openedx/features/course_duration_limits/access.py b/openedx/features/course_duration_limits/access.py index b16207f84a..62be42580a 100644 --- a/openedx/features/course_duration_limits/access.py +++ b/openedx/features/course_duration_limits/access.py @@ -69,16 +69,12 @@ def get_user_course_expiration_date(user, course): except CourseEnrollment.schedule.RelatedObjectDoesNotExist: content_availability_date = max(enrollment.created, course.start) - if course.self_paced: - # The user course expiration date for self paced courses is the - # content availability date plus the weeks_to_complete field from course-discovery. - discovery_course_details = get_course_run_details(course.id, ['weeks_to_complete']) - expected_weeks = discovery_course_details['weeks_to_complete'] or int(MIN_DURATION.days / 7) + # The user course expiration date is the content availability date + # plus the weeks_to_complete field from course-discovery. + discovery_course_details = get_course_run_details(course.id, ['weeks_to_complete']) + expected_weeks = discovery_course_details.get('weeks_to_complete') + if expected_weeks: access_duration = timedelta(weeks=expected_weeks) - elif not course.self_paced and course.end and course.start: - # The user course expiration date for instructor paced courses is the - # content availability date plus the duration of the course (course end date minus course start date). - access_duration = course.end - course.start # Course access duration is bounded by the min and max duration. access_duration = max(MIN_DURATION, min(MAX_DURATION, access_duration)) diff --git a/openedx/features/course_duration_limits/tests/test_course_expiration.py b/openedx/features/course_duration_limits/tests/test_course_expiration.py index 07e1d859ab..aec144afd0 100644 --- a/openedx/features/course_duration_limits/tests/test_course_expiration.py +++ b/openedx/features/course_duration_limits/tests/test_course_expiration.py @@ -35,42 +35,57 @@ class CourseExpirationTestCase(ModuleStoreTestCase): result = get_user_course_expiration_date(self.user, self.course) self.assertEqual(result, None) - def test_instructor_paced(self): - """Tests that instructor paced courses give the learner start_date - end_date time in the course""" - expected_difference = timedelta(weeks=6) - self.course.self_paced = False - self.course.end = self.course.start + expected_difference - enrollment = CourseEnrollment.enroll(self.user, self.course.id, CourseMode.AUDIT) - result = get_user_course_expiration_date(self.user, self.course) - self.assertEqual(result, enrollment.created + expected_difference) - - def test_instructor_paced_no_end_date(self): - """Tests that instructor paced with no end dates returns default (minimum)""" - self.course.self_paced = False - enrollment = CourseEnrollment.enroll(self.user, self.course.id, CourseMode.AUDIT) - result = get_user_course_expiration_date(self.user, self.course) - self.assertEqual(result, enrollment.created + MIN_DURATION) - @mock.patch("openedx.features.course_duration_limits.access.get_course_run_details") @ddt.data( - [int(MIN_DURATION.days / 7) - 1, MIN_DURATION], - [7, timedelta(weeks=7)], - [int(MAX_DURATION.days / 7) + 1, MAX_DURATION], - [None, MIN_DURATION], + [int(MIN_DURATION.days / 7) - 1, MIN_DURATION, False], + [7, timedelta(weeks=7), False], + [int(MAX_DURATION.days / 7) + 1, MAX_DURATION, False], + [None, MIN_DURATION, False], + [int(MIN_DURATION.days / 7) - 1, MIN_DURATION, True], + [7, timedelta(weeks=7), True], + [int(MAX_DURATION.days / 7) + 1, MAX_DURATION, True], + [None, MIN_DURATION, True], ) @ddt.unpack - def test_self_paced_with_weeks_to_complete( + def test_all_courses_with_weeks_to_complete( self, weeks_to_complete, - expected_difference, + access_duration, + self_paced, mock_get_course_run_details, ): """ - Tests that self paced courses allow for a (bounded) # of weeks in courses determined via - weeks_to_complete field in discovery. If the field doesn't exist, it should return default (minimum) + Test that access_duration for a course is equal to the value of the weeks_to_complete field in discovery. + If weeks_to_complete is None, access_duration will be the MIN_DURATION constant. + """ - self.course.self_paced = True + if self_paced: + self.course.self_paced = True mock_get_course_run_details.return_value = {'weeks_to_complete': weeks_to_complete} enrollment = CourseEnrollment.enroll(self.user, self.course.id, CourseMode.AUDIT) result = get_user_course_expiration_date(self.user, self.course) - self.assertEqual(result, enrollment.created + expected_difference) + self.assertEqual(result, enrollment.created + access_duration) + + @mock.patch("openedx.features.course_duration_limits.access.get_course_run_details") + def test_content_availability_date(self, mock_get_course_run_details): + """ + Content availability date is course start date or enrollment date, whichever is later. + """ + access_duration = timedelta(weeks=7) + mock_get_course_run_details.return_value = {'weeks_to_complete': 7} + + # Content availability date is enrollment date + start_date = now() - timedelta(weeks=10) + past_course = CourseFactory(start=start_date) + enrollment = CourseEnrollment.enroll(self.user, past_course.id, CourseMode.AUDIT) + result = get_user_course_expiration_date(self.user, past_course) + content_availability_date = enrollment.created + self.assertEqual(result, content_availability_date + access_duration) + + # Content availability date is course start date + start_date = now() + timedelta(weeks=10) + future_course = CourseFactory(start=start_date) + enrollment = CourseEnrollment.enroll(self.user, future_course.id, CourseMode.AUDIT) + result = get_user_course_expiration_date(self.user, future_course) + content_availability_date = start_date + self.assertEqual(result, content_availability_date + access_duration) 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 f3c468dbe0..d1fa9e4b84 100644 --- a/openedx/features/course_experience/tests/views/test_course_home.py +++ b/openedx/features/course_experience/tests/views/test_course_home.py @@ -180,7 +180,7 @@ class TestCourseHomePage(CourseHomePageTestCase): course_home_url(self.course) # Fetch the view and verify the query counts - with self.assertNumQueries(66, table_blacklist=QUERY_COUNT_TABLE_BLACKLIST): + with self.assertNumQueries(67, table_blacklist=QUERY_COUNT_TABLE_BLACKLIST): with check_mongo_calls(4): url = course_home_url(self.course) self.client.get(url) diff --git a/openedx/features/course_experience/tests/views/test_course_updates.py b/openedx/features/course_experience/tests/views/test_course_updates.py index 08bb6fe40e..10ea37078b 100644 --- a/openedx/features/course_experience/tests/views/test_course_updates.py +++ b/openedx/features/course_experience/tests/views/test_course_updates.py @@ -127,7 +127,7 @@ class TestCourseUpdatesPage(SharedModuleStoreTestCase): course_updates_url(self.course) # Fetch the view and verify that the query counts haven't changed - with self.assertNumQueries(38, table_blacklist=QUERY_COUNT_TABLE_BLACKLIST): + with self.assertNumQueries(39, table_blacklist=QUERY_COUNT_TABLE_BLACKLIST): with check_mongo_calls(4): url = course_updates_url(self.course) self.client.get(url)