From 6a01093423be51ddca765c95eee853d23aed65b3 Mon Sep 17 00:00:00 2001
From: Matt Tuchfarber <mtuchfarber@edx.org>
Date: Mon, 22 Oct 2018 21:35:08 -0400
Subject: [PATCH] Flesh out expiration date for user-course combo logic
---
lms/djangoapps/courseware/tests/test_views.py | 8 +-
.../grades/api/v1/tests/test_views.py | 2 +-
.../features/course_duration_limits/access.py | 35 +++++++--
.../course_duration_limits/tests/__init__.py | 0
.../tests/test_course_expiration.py | 76 +++++++++++++++++++
.../tests/views/test_course_home.py | 2 +-
6 files changed, 109 insertions(+), 14 deletions(-)
create mode 100644 openedx/features/course_duration_limits/tests/__init__.py
create mode 100644 openedx/features/course_duration_limits/tests/test_course_expiration.py
diff --git a/lms/djangoapps/courseware/tests/test_views.py b/lms/djangoapps/courseware/tests/test_views.py
index 0ed6538ba01..100fa6d5808 100644
--- a/lms/djangoapps/courseware/tests/test_views.py
+++ b/lms/djangoapps/courseware/tests/test_views.py
@@ -1433,8 +1433,8 @@ class ProgressPageTests(ProgressPageBaseTests):
@override_waffle_flag(CONTENT_TYPE_GATING_FLAG, True)
@ddt.data(
- (True, 40),
- (False, 39)
+ (True, 39),
+ (False, 38)
)
@ddt.unpack
def test_progress_queries_paced_courses(self, self_paced, query_count):
@@ -1446,8 +1446,8 @@ class ProgressPageTests(ProgressPageBaseTests):
@override_waffle_flag(CONTENT_TYPE_GATING_FLAG, True)
@patch.dict(settings.FEATURES, {'ASSUME_ZERO_GRADE_IF_ABSENT_FOR_ALL_TESTS': False})
@ddt.data(
- (False, 47, 30),
- (True, 39, 26)
+ (False, 46, 29),
+ (True, 38, 25)
)
@ddt.unpack
def test_progress_queries(self, enable_waffle, initial, subsequent):
diff --git a/lms/djangoapps/grades/api/v1/tests/test_views.py b/lms/djangoapps/grades/api/v1/tests/test_views.py
index 6d76ab4827c..54389d5f283 100644
--- a/lms/djangoapps/grades/api/v1/tests/test_views.py
+++ b/lms/djangoapps/grades/api/v1/tests/test_views.py
@@ -14,7 +14,7 @@ from rest_framework import status
from rest_framework.test import APITestCase
from six import text_type
-from lms.djangoapps.courseware.tests.factories import GlobalStaffFactory, StaffFactory
+from lms.djangoapps.courseware.tests.factories import GlobalStaffFactory
from lms.djangoapps.grades.api.v1.views import CourseGradesView
from lms.djangoapps.grades.config.waffle import waffle_flags, WRITABLE_GRADEBOOK
from lms.djangoapps.grades.course_data import CourseData
diff --git a/openedx/features/course_duration_limits/access.py b/openedx/features/course_duration_limits/access.py
index 6a523d57f25..7f6c46bdb71 100644
--- a/openedx/features/course_duration_limits/access.py
+++ b/openedx/features/course_duration_limits/access.py
@@ -12,9 +12,14 @@ from django.utils.translation import ugettext as _
from util.date_utils import DEFAULT_SHORT_DATE_FORMAT, strftime_localized
from lms.djangoapps.courseware.access_response import AccessError
from lms.djangoapps.courseware.access_utils import ACCESS_GRANTED
+from openedx.core.djangoapps.catalog.utils import get_course_run_details
from openedx.core.djangoapps.content.course_overviews.models import CourseOverview
+MIN_DURATION = timedelta(weeks=4)
+MAX_DURATION = timedelta(weeks=12)
+
+
class AuditExpiredError(AccessError):
"""
Access denied because the user's audit timespan has expired
@@ -43,11 +48,19 @@ def get_user_course_expiration_date(user, course):
"""
Return course expiration date for given user course pair.
Return None if the course does not expire.
+ Defaults to MIN_DURATION.
+
+ Business Logic:
+ -
+ - should be bounded with min / max
+ - if fields are missing, default to minimum time
"""
- # TODO: Update business logic based on REV-531
+
+ access_duration = MIN_DURATION
+
CourseEnrollment = apps.get_model('student.CourseEnrollment')
enrollment = CourseEnrollment.get_enrollment(user, course.id)
- if enrollment is None or enrollment.mode == 'verified':
+ if enrollment is None or enrollment.mode != 'audit':
return None
try:
@@ -55,13 +68,19 @@ def get_user_course_expiration_date(user, course):
except CourseEnrollment.schedule.RelatedObjectDoesNotExist:
start_date = max(enrollment.created, course.start)
- access_duration = timedelta(weeks=8)
- if hasattr(course, 'pacing') and course.pacing == 'instructor':
- if course.end and course.start:
- access_duration = course.end - course.start
+ if course.self_paced:
+ # self-paced expirations should be start date plus the marketing course length 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)
+ access_duration = timedelta(weeks=expected_weeks)
+ elif not course.self_paced and course.end and course.start:
+ # instructor-paced expirations should be the start date plus the length of the course
+ access_duration = course.end - course.start
+
+ # available course time should bound my the min and max duration
+ access_duration = max(MIN_DURATION, min(MAX_DURATION, access_duration))
- expiration_date = start_date + access_duration
- return expiration_date
+ return start_date + access_duration
def check_course_expired(user, course):
diff --git a/openedx/features/course_duration_limits/tests/__init__.py b/openedx/features/course_duration_limits/tests/__init__.py
new file mode 100644
index 00000000000..e69de29bb2d
diff --git a/openedx/features/course_duration_limits/tests/test_course_expiration.py b/openedx/features/course_duration_limits/tests/test_course_expiration.py
new file mode 100644
index 00000000000..07e1d859ab1
--- /dev/null
+++ b/openedx/features/course_duration_limits/tests/test_course_expiration.py
@@ -0,0 +1,76 @@
+"""
+Contains tests to verify correctness of course expiration functionality
+"""
+from datetime import timedelta
+from django.utils.timezone import now
+import ddt
+import mock
+
+
+from course_modes.models import CourseMode
+from openedx.features.course_duration_limits.access import get_user_course_expiration_date, MIN_DURATION, MAX_DURATION
+from student.models import CourseEnrollment
+from student.tests.factories import UserFactory
+from xmodule.modulestore.tests.django_utils import ModuleStoreTestCase
+from xmodule.modulestore.tests.factories import CourseFactory
+
+
+@ddt.ddt
+class CourseExpirationTestCase(ModuleStoreTestCase):
+ """Tests to verify the get_user_course_expiration_date function is working correctly"""
+ def setUp(self):
+ super(CourseExpirationTestCase, self).setUp()
+ self.course = CourseFactory(
+ start=now() - timedelta(weeks=10),
+ )
+ self.user = UserFactory()
+
+ def tearDown(self):
+ CourseEnrollment.unenroll(self.user, self.course.id)
+ super(CourseExpirationTestCase, self).tearDown()
+
+ def test_enrollment_mode(self):
+ """Tests that verified enrollments do not have an expiration"""
+ CourseEnrollment.enroll(self.user, self.course.id, CourseMode.VERIFIED)
+ 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],
+ )
+ @ddt.unpack
+ def test_self_paced_with_weeks_to_complete(
+ self,
+ weeks_to_complete,
+ expected_difference,
+ 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)
+ """
+ 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)
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 cfbcac43f31..d414c945a37 100644
--- a/openedx/features/course_experience/tests/views/test_course_home.py
+++ b/openedx/features/course_experience/tests/views/test_course_home.py
@@ -337,7 +337,7 @@ class TestCourseHomePageAccess(CourseHomePageTestCase):
url = course_home_url(course)
response = self.client.get(url)
- expiration_date = strftime_localized(course.start + timedelta(weeks=8), 'SHORT_DATE')
+ expiration_date = strftime_localized(course.start + timedelta(weeks=4), 'SHORT_DATE')
expected_params = QueryDict(mutable=True)
course_name = CourseOverview.get_from_id(course.id).display_name_with_default
expected_params['access_response_error'] = 'Access to {run} expired on {expiration_date}'.format(
--
GitLab