add redirect behavior when accessing expired course and add tests

This commit is contained in:
Matthew Piatetsky
2018-10-17 13:00:40 -04:00
parent c51ee5ecba
commit 3c47d19a52
7 changed files with 119 additions and 23 deletions

View File

@@ -27,7 +27,10 @@ from openedx.core.djangoapps.content.course_overviews.models import CourseOvervi
from openedx.core.djangoapps.content.course_overviews.tests.factories import CourseOverviewFactory
from openedx.core.djangoapps.site_configuration.tests.test_util import with_site_configuration_context
from pyquery import PyQuery as pq
from openedx.core.djangoapps.schedules.config import COURSE_UPDATE_WAFFLE_FLAG
from openedx.core.djangoapps.schedules.tests.factories import ScheduleFactory
from openedx.core.djangoapps.user_authn.cookies import _get_user_info_cookie_data
from openedx.core.djangoapps.waffle_utils.testutils import override_waffle_flag
from student.helpers import DISABLE_UNENROLL_CERT_STATES
from student.models import CourseEnrollment, UserProfile
from student.signals import REFUND_ORDER
@@ -306,7 +309,7 @@ class StudentDashboardTests(SharedModuleStoreTestCase, MilestonesTestCaseMixin,
'type': 'verified'
}
response = self.client.get(self.path)
self.assertIn('class="enter-course hidden"', response.content)
self.assertIn('class="course-target-link enter-course hidden"', response.content)
self.assertIn('You must select a session to access the course.', response.content)
self.assertIn('<div class="course-entitlement-selection-container ">', response.content)
self.assertIn('Related Programs:', response.content)
@@ -580,7 +583,7 @@ class StudentDashboardTests(SharedModuleStoreTestCase, MilestonesTestCaseMixin,
def _get_html_for_view_course_button(course_key_string, course_run_string):
return '''
<a href="/courses/{course_key}/course/"
class="enter-course "
class="course-target-link enter-course"
data-course-key="{course_key}">
View Course
<span class="sr">
@@ -593,7 +596,7 @@ class StudentDashboardTests(SharedModuleStoreTestCase, MilestonesTestCaseMixin,
def _get_html_for_resume_course_button(course_key_string, resume_block_key_string, course_run_string):
return '''
<a href="/courses/{course_key}/jump_to/{url_to_block}"
class="enter-course "
class="course-target-link enter-course"
data-course-key="{course_key}">
Resume Course
<span class="sr">
@@ -718,6 +721,37 @@ class StudentDashboardTests(SharedModuleStoreTestCase, MilestonesTestCaseMixin,
dashboard_html
)
@override_waffle_flag(COURSE_UPDATE_WAFFLE_FLAG, True)
def test_content_gating_course_card_changes(self):
"""
When a course is expired, the links on the course card should be removed.
Links will be removed from the course title, course image and button (View Course/Resume Course).
The course card should have an access expired message.
"""
self.override_waffle_switch(True)
course = CourseFactory.create(start=self.THREE_YEARS_AGO)
enrollment = CourseEnrollmentFactory.create(
user=self.user,
course_id=course.id
)
schedule = ScheduleFactory(start=self.THREE_YEARS_AGO, enrollment=enrollment)
response = self.client.get(reverse('dashboard'))
dashboard_html = self._remove_whitespace_from_html_string(response.content)
access_expired_substring = 'Accessexpired'
course_link_class = 'course-target-link'
self.assertNotIn(
course_link_class,
dashboard_html
)
self.assertIn(
access_expired_substring,
dashboard_html
)
def test_dashboard_with_resume_buttons_and_view_buttons(self):
'''
The Test creates a four-course-card dashboard. The user completes course

View File

@@ -767,6 +767,8 @@ def student_dashboard(request):
redirect_message = _("The course you are looking for is closed for enrollment as of {date}.").format(
date=request.GET['course_closed']
)
elif 'expired_message' in request.GET:
redirect_message = request.GET['expired_message']
else:
redirect_message = ''

View File

@@ -9,7 +9,8 @@ from xmodule.course_metadata_utils import DEFAULT_START_DATE
class AccessResponse(object):
"""Class that represents a response from a has_access permission check."""
def __init__(self, has_access, error_code=None, developer_message=None, user_message=None, user_fragment=None):
def __init__(self, has_access, error_code=None, developer_message=None, user_message=None,
detailed_user_message=None, user_fragment=None):
"""
Creates an AccessResponse object.
@@ -21,13 +22,16 @@ class AccessResponse(object):
to show the developer
user_message (String): optional - default is None. Message to
show the user
detailed_user_message (String): optional - default is None. Message to
show the user when additional context like the course name is necessary
user_fragment (:py:class:`~web_fragments.fragment.Fragment`): optional -
An html fragment to display to the user if their access is denied.
An html fragment to display to the user if their access is denied
"""
self.has_access = has_access
self.error_code = error_code
self.developer_message = developer_message
self.user_message = user_message
self.detailed_user_message = detailed_user_message
self.user_fragment = user_fragment
if has_access:
assert error_code is None
@@ -58,15 +62,17 @@ class AccessResponse(object):
"error_code": self.error_code,
"developer_message": self.developer_message,
"user_message": self.user_message,
"detailed_user_message": self.detailed_user_message,
"user_fragment": self.user_fragment,
}
def __repr__(self):
return "AccessResponse({!r}, {!r}, {!r}, {!r}, {!r})".format(
return "AccessResponse({!r}, {!r}, {!r}, {!r}, {!r}, {!r})".format(
self.has_access,
self.error_code,
self.developer_message,
self.user_message,
self.detailed_user_message,
self.user_fragment,
)
@@ -79,6 +85,7 @@ class AccessResponse(object):
self.error_code == other.error_code and
self.developer_message == other.developer_message and
self.user_message == other.user_message and
self.detailed_user_message == other.detailed_user_message and
self.user_fragment == other.user_fragment
)
@@ -89,7 +96,7 @@ class AccessError(AccessResponse):
denial in has_access. Contains the error code, user and developer
messages. Subclasses represent specific errors.
"""
def __init__(self, error_code, developer_message, user_message, user_fragment=None):
def __init__(self, error_code, developer_message, user_message, detailed_user_message=None, user_fragment=None):
"""
Creates an AccessError object.
@@ -100,10 +107,12 @@ class AccessError(AccessResponse):
error_code (String): unique identifier for the specific type of
error developer_message (String): message to show the developer
user_message (String): message to show the user
detailed_user_message (String): message to show the user with additional context like the course name
user_fragment (:py:class:`~web_fragments.fragment.Fragment`): HTML to show the user
"""
super(AccessError, self).__init__(False, error_code, developer_message, user_message, user_fragment)
super(AccessError, self).__init__(False, error_code, developer_message, user_message,
detailed_user_message, user_fragment)
class StartDateError(AccessError):

View File

@@ -8,6 +8,7 @@ from datetime import datetime
import branding
import pytz
from openedx.features.course_duration_limits.access import AuditExpiredError
from courseware.access import has_access
from courseware.access_response import StartDateError, MilestoneAccessError
from courseware.date_summary import (
@@ -143,6 +144,15 @@ def check_course_access(course, user, action, check_if_enrolled=False, check_sur
params=params.urlencode()
), access_response)
# Redirect if AuditExpiredError
if isinstance(access_response, AuditExpiredError):
params = QueryDict(mutable=True)
params['expired_message'] = access_response.detailed_user_message
raise CourseAccessRedirect('{dashboard_url}?{params}'.format(
dashboard_url=reverse('dashboard'),
params=params.urlencode()
), access_response)
# Redirect if the user must answer a survey before entering the course.
if isinstance(access_response, MilestoneAccessError):
raise CourseAccessRedirect('{dashboard_url}'.format(

View File

@@ -34,7 +34,7 @@ from util.course import get_link_for_about_page, get_encoded_course_sharing_utm_
cert_name_long = settings.CERT_NAME_LONG
billing_email = settings.PAYMENT_SUPPORT_EMAIL
is_course_expired = getattr(show_courseware_link, 'error_code') == 'audit_expired'
is_course_expired = hasattr(show_courseware_link, 'error_code') and show_courseware_link.error_code == 'audit_expired'
%>
<%namespace name='static' file='../static_content.html'/>
@@ -68,7 +68,7 @@ from util.course import get_link_for_about_page, get_encoded_course_sharing_utm_
<div class="wrapper-course-image" aria-hidden="true">
% if show_courseware_link and not is_unfulfilled_entitlement:
% if not is_course_blocked and not is_course_expired:
<a href="${course_target}" data-course-key="${enrollment.course_id}" class="cover" tabindex="-1">
<a href="${course_target}" data-course-key="${enrollment.course_id}" class="cover course-target-link" tabindex="-1">
<img src="${course_overview.image_urls['small']}" class="course-image" alt="${_('{course_number} {course_name} Home Page').format(course_number=course_overview.number, course_name=course_overview.display_name_with_default)}" />
</a>
% else:
@@ -95,7 +95,7 @@ from util.course import get_link_for_about_page, get_encoded_course_sharing_utm_
<h3 class="course-title" id="course-title-${enrollment.course_id}">
% if show_courseware_link and not is_unfulfilled_entitlement:
% if not is_course_blocked and not is_course_expired:
<a data-course-key="${enrollment.course_id}" href="${course_target}">${course_overview.display_name_with_default}</a>
<a data-course-key="${enrollment.course_id}" href="${course_target}" class="course-target-link">${course_overview.display_name_with_default}</a>
% else:
<a class="disable-look" data-course-key="${enrollment.course_id}">${course_overview.display_name_with_default}</a>
% endif
@@ -130,7 +130,7 @@ from util.course import get_link_for_about_page, get_encoded_course_sharing_utm_
%>
<span class="info-date-block-container">
% if is_course_expired:
% if not is_unfulfilled_entitlement and is_course_expired:
<span class="info-date-block" data-course-key="${enrollment.course_id}">
${show_courseware_link.user_message}
<span class="sr">
@@ -174,27 +174,27 @@ from util.course import get_link_for_about_page, get_encoded_course_sharing_utm_
</div>
<div class="wrapper-course-actions">
<div class="course-actions">
% if show_courseware_link or is_unfulfilled_entitlement:
% if (show_courseware_link or is_unfulfilled_entitlement) and not is_course_expired:
% if course_overview.has_ended():
% if not is_course_blocked and not is_course_expired:
<a href="${course_target}" class="enter-course archived" data-course-key="${enrollment.course_id}">${_('View Archived Course')}<span class="sr">&nbsp;${course_overview.display_name_with_default}</span></a>
% if not is_course_blocked:
<a href="${course_target}" class="enter-course archived course-target-link" data-course-key="${enrollment.course_id}">${_('View Archived Course')}<span class="sr">&nbsp;${course_overview.display_name_with_default}</span></a>
% else:
<a class="enter-course-blocked archived" data-course-key="${enrollment.course_id}">${_('View Archived Course')}<span class="sr">&nbsp;${course_overview.display_name_with_default}</span></a>
<a class="enter-course-blocked archived course-target-link" data-course-key="${enrollment.course_id}">${_('View Archived Course')}<span class="sr">&nbsp;${course_overview.display_name_with_default}</span></a>
% endif
% else:
% if resume_button_url != '':
<a href="${resume_button_url}"
class="enter-course ${'hidden' if is_unfulfilled_entitlement else ''}"
class="course-target-link enter-course ${'hidden' if is_unfulfilled_entitlement else ''}"
data-course-key="${enrollment.course_id}">
${_('Resume Course')}
<span class="sr">
&nbsp;${course_overview.display_name_with_default}
</span>
</a>
% elif not is_course_blocked and not is_course_expired:
% elif not is_course_blocked:
<a href="${course_target}"
class="enter-course ${'hidden' if is_unfulfilled_entitlement else ''}"
class="course-target-link enter-course ${'hidden' if is_unfulfilled_entitlement else ''}"
data-course-key="${enrollment.course_id}">
${_('View Course')}
<span class="sr">

View File

@@ -9,9 +9,10 @@ from django.apps import apps
from django.utils import timezone
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.content.course_overviews.models import CourseOverview
class AuditExpiredError(AccessError):
@@ -21,9 +22,19 @@ class AuditExpiredError(AccessError):
def __init__(self, user, course, expiration_date):
error_code = "audit_expired"
developer_message = "User {} had access to {} until {}".format(user, course, expiration_date)
# TODO: Translate the expiration_date
user_message = _("Course access expired on ") + expiration_date.strftime("%B %d, %Y")
super(AuditExpiredError, self).__init__(error_code, developer_message, user_message)
expiration_date = strftime_localized(expiration_date, DEFAULT_SHORT_DATE_FORMAT)
user_message = _("Access expired on {expiration_date}").format(expiration_date=expiration_date)
try:
course_name = CourseOverview.get_from_id(course.id).display_name_with_default
detailed_user_message = _("Access to {course_name} expired on {expiration_date}").format(
course_name=course_name,
expiration_date=expiration_date
)
except CourseOverview.DoesNotExist:
detailed_user_message = _("Access to the course you were looking for expired on {expiration_date}").format(
expiration_date=expiration_date
)
super(AuditExpiredError, self).__init__(error_code, developer_message, user_message, detailed_user_message)
def get_user_course_expiration_date(user, course):

View File

@@ -20,6 +20,8 @@ from courseware.tests.factories import StaffFactory
from lms.djangoapps.commerce.models import CommerceConfiguration
from lms.djangoapps.commerce.utils import EcommerceService
from lms.djangoapps.course_goals.api import add_course_goal, remove_course_goal
from openedx.core.djangoapps.content.course_overviews.models import CourseOverview
from openedx.core.djangoapps.schedules.tests.factories import ScheduleFactory
from openedx.core.djangoapps.waffle_utils.testutils import WAFFLE_TABLES, override_waffle_flag
from openedx.features.course_experience import (
SHOW_REVIEWS_TOOL_FLAG,
@@ -317,6 +319,34 @@ class TestCourseHomePageAccess(CourseHomePageTestCase):
)
self.assertRedirects(response, expected_url)
@mock.patch.dict(settings.FEATURES, {'DISABLE_START_DATES': False})
def test_expired_course(self):
"""
Ensure that a user accessing an expired course sees a redirect to
the student dashboard, not a 404.
"""
three_years_ago = now() - timedelta(days=(365 * 3))
course = CourseFactory.create(start=three_years_ago)
user = self.create_user_for_course(course, CourseUserType.ENROLLED)
enrollment = CourseEnrollment.get_enrollment(user, course.id)
ScheduleFactory(start=three_years_ago, enrollment=enrollment)
url = course_home_url(course)
response = self.client.get(url)
expiration_date = strftime_localized(course.start + timedelta(weeks=8), 'SHORT_DATE')
expected_params = QueryDict(mutable=True)
course_name = CourseOverview.get_from_id(course.id).display_name_with_default
expected_params['expired_message'] = 'Access to {run} expired on {expiration_date}'.format(
run=course_name,
expiration_date=expiration_date
)
expected_url = '{url}?{params}'.format(
url=reverse('dashboard'),
params=expected_params.urlencode()
)
self.assertRedirects(response, expected_url)
@mock.patch.dict(settings.FEATURES, {'DISABLE_START_DATES': False})
@mock.patch("util.date_utils.strftime_localized")
def test_non_live_course_other_language(self, mock_strftime_localized):