diff --git a/common/djangoapps/student/tests/test_views.py b/common/djangoapps/student/tests/test_views.py index 4766cca92d..7e6c321db2 100644 --- a/common/djangoapps/student/tests/test_views.py +++ b/common/djangoapps/student/tests/test_views.py @@ -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('
', 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 ''' View Course @@ -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 ''' Resume Course @@ -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 diff --git a/common/djangoapps/student/views/dashboard.py b/common/djangoapps/student/views/dashboard.py index 722b717fbd..8a18d52e49 100644 --- a/common/djangoapps/student/views/dashboard.py +++ b/common/djangoapps/student/views/dashboard.py @@ -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 = '' diff --git a/lms/djangoapps/courseware/access_response.py b/lms/djangoapps/courseware/access_response.py index 4f254887e8..a12dfa05a7 100644 --- a/lms/djangoapps/courseware/access_response.py +++ b/lms/djangoapps/courseware/access_response.py @@ -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): diff --git a/lms/djangoapps/courseware/courses.py b/lms/djangoapps/courseware/courses.py index b18820d14d..a9d131b392 100644 --- a/lms/djangoapps/courseware/courses.py +++ b/lms/djangoapps/courseware/courses.py @@ -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( diff --git a/lms/templates/dashboard/_dashboard_course_listing.html b/lms/templates/dashboard/_dashboard_course_listing.html index e1472f3a8f..def15e99e4 100644 --- a/lms/templates/dashboard/_dashboard_course_listing.html +++ b/lms/templates/dashboard/_dashboard_course_listing.html @@ -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_
- % 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: - ${_('View Archived Course')} ${course_overview.display_name_with_default} + % if not is_course_blocked: + ${_('View Archived Course')} ${course_overview.display_name_with_default} % else: - ${_('View Archived Course')} ${course_overview.display_name_with_default} + ${_('View Archived Course')} ${course_overview.display_name_with_default} % endif % else: % if resume_button_url != '': ${_('Resume Course')}  ${course_overview.display_name_with_default} - % elif not is_course_blocked and not is_course_expired: + % elif not is_course_blocked: ${_('View Course')} diff --git a/openedx/features/course_duration_limits/access.py b/openedx/features/course_duration_limits/access.py index 27fad2fe2f..0ce4b23686 100644 --- a/openedx/features/course_duration_limits/access.py +++ b/openedx/features/course_duration_limits/access.py @@ -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): 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 de380d2968..3cf2918007 100644 --- a/openedx/features/course_experience/tests/views/test_course_home.py +++ b/openedx/features/course_experience/tests/views/test_course_home.py @@ -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):