From c98cdbd1f7092b5b4bd3d9174f4799c3cec0cb4d Mon Sep 17 00:00:00 2001 From: julianajlk Date: Fri, 29 Oct 2021 10:02:41 -0400 Subject: [PATCH] feat: Add Track Selection error handling (#28941) REV-2355 --- .../course_modes/tests/test_views.py | 54 +++++++++++++++---- common/djangoapps/course_modes/views.py | 43 +++++++++------ lms/static/sass/views/_track_selection.scss | 26 +++++++++ lms/templates/course_modes/error.html | 36 +++++++++++++ .../course_modes/track_selection.html | 1 - 5 files changed, 132 insertions(+), 28 deletions(-) create mode 100644 lms/templates/course_modes/error.html diff --git a/common/djangoapps/course_modes/tests/test_views.py b/common/djangoapps/course_modes/tests/test_views.py index f1a130466e..9d2d57a013 100644 --- a/common/djangoapps/course_modes/tests/test_views.py +++ b/common/djangoapps/course_modes/tests/test_views.py @@ -401,17 +401,6 @@ class CourseModeViewTest(CatalogIntegrationMixin, UrlResetMixin, ModuleStoreTest assert mode == CourseMode.DEFAULT_MODE_SLUG assert is_active is True - def test_unsupported_enrollment_mode_failure(self): - # Create the supported course modes - for mode in ('honor', 'verified'): - CourseModeFactory.create(mode_slug=mode, course_id=self.course.id) - - # Choose an unsupported mode (POST request) - choose_track_url = reverse('course_modes_choose', args=[str(self.course.id)]) - response = self.client.post(choose_track_url, self.POST_PARAMS_FOR_COURSE_MODE['unsupported']) - - assert 400 == response.status_code - @unittest.skipUnless(settings.ROOT_URLCONF == 'lms.urls', 'Test only valid in lms') def test_default_mode_creation(self): # Hit the mode creation endpoint with no querystring params, to create an honor mode @@ -518,6 +507,49 @@ class CourseModeViewTest(CatalogIntegrationMixin, UrlResetMixin, ModuleStoreTest redirect_url = reverse('dashboard') + '?course_closed=1%2F1%2F15%2C+12%3A00+AM' self.assertRedirects(response, redirect_url) + @ddt.data( + (False, {'audit_mode': True}, 'Enrollment is closed', 302), + (False, {'verified_mode': True, 'contribution': '1.23'}, 'Enrollment is closed', 302), + (True, {'verified_mode': True, 'contribution': 'abc'}, 'Invalid amount selected', 200), + (True, {'verified_mode': True, 'contribution': '0.1'}, 'No selected price or selected price is too low.', 200), + (True, {'unsupported_mode': True}, 'Enrollment mode not supported', 200), + ) + @ddt.unpack + @patch('django.contrib.auth.models.PermissionsMixin.has_perm') + def test_errors(self, has_perm, post_params, error_msg, status_code, mock_has_perm): + """ + Test the error template is rendered on different types of errors. + When the chosen CourseMode is 'honor' or 'audit' via POST, + it redirects to dashboard, but if there's an error in the process, + it shows the error template. + If the user does not have permission to enroll, GET is called with error message, + but it also redirects to dashboard. + """ + # Create course modes + for mode in ('audit', 'honor', 'verified'): + CourseModeFactory.create(mode_slug=mode, course_id=self.course.id) + + # Value Prop TODO (REV-2378): remove waffle flag from tests once flag is removed. + with override_waffle_flag(VALUE_PROP_TRACK_SELECTION_FLAG, active=True): + mock_has_perm.return_value = has_perm + url = reverse('course_modes_choose', args=[str(self.course.id)]) + + # Choose mode (POST request) + response = self.client.post(url, post_params) + self.assertEqual(response.status_code, status_code) + + if has_perm: + self.assertContains(response, error_msg) + self.assertContains(response, 'Sorry, we were unable to enroll you') + + # Check for CTA button on error page + marketing_root = settings.MKTG_URLS.get('ROOT') + search_courses_url = urljoin(marketing_root, '/search?tab=course') + self.assertContains(response, search_courses_url) + self.assertContains(response, 'Explore all courses') + else: + self.assertTrue(CourseEnrollment.is_enrollment_closed(self.user, self.course)) + def _assert_fbe_page(self, response, min_price=None, **_): """ Assert fbe.html was rendered. diff --git a/common/djangoapps/course_modes/views.py b/common/djangoapps/course_modes/views.py index a279224d29..dfc876a95c 100644 --- a/common/djangoapps/course_modes/views.py +++ b/common/djangoapps/course_modes/views.py @@ -11,9 +11,10 @@ import six import waffle # lint-amnesty, pylint: disable=invalid-django-waffle-import from babel.dates import format_datetime from babel.numbers import get_currency_symbol +from django.conf import settings from django.contrib.auth.decorators import login_required from django.db import transaction -from django.http import HttpResponse, HttpResponseBadRequest +from django.http import HttpResponse from django.shortcuts import redirect from django.urls import reverse from django.utils.decorators import method_decorator @@ -23,6 +24,7 @@ from django.views.generic.base import View from edx_django_utils.monitoring.utils import increment from ipware.ip import get_client_ip from opaque_keys.edx.keys import CourseKey +from urllib.parse import urljoin from common.djangoapps.course_modes.models import CourseMode from common.djangoapps.course_modes.helpers import get_course_final_price, get_verified_track_links @@ -133,7 +135,6 @@ class ChooseModeView(View): if purchase_workflow == "bulk" and professional_mode.bulk_sku: redirect_url = ecommerce_service.get_checkout_page_url(professional_mode.bulk_sku) return redirect(redirect_url) - course = modulestore().get_course(course_key) # If there isn't a verified mode available, then there's nothing @@ -152,6 +153,10 @@ class ChooseModeView(View): locale = to_locale(get_language()) enrollment_end_date = format_datetime(course.enrollment_end, 'short', locale=locale) params = six.moves.urllib.parse.urlencode({'course_closed': enrollment_end_date}) + LOG.info( + '[Track Selection Check] Enrollment is closed redirect for course [%s], user [%s]', + course_id, request.user.username + ) return redirect('{}?{}'.format(reverse('dashboard'), params)) # When a credit mode is available, students will be given the option @@ -187,6 +192,7 @@ class ChooseModeView(View): "nav_hidden": True, "content_gating_enabled": gated_content, "course_duration_limit_enabled": CourseDurationLimitConfig.enabled_for_enrollment(request.user, course), + "search_courses_url": urljoin(settings.MKTG_URLS.get('ROOT'), '/search?tab=course'), } context.update( get_experiment_user_metadata_context( @@ -258,17 +264,17 @@ class ChooseModeView(View): fbe_is_on = deadline and gated_content # Route to correct Track Selection page. - # REV-2133 TODO Value Prop: remove waffle flag after testing is completed - # and happy path version is ready to be rolled out to all users. + # REV-2378 TODO Value Prop: remove waffle flag after all edge cases for track selection are completed. if VALUE_PROP_TRACK_SELECTION_FLAG.is_enabled(): - if not error: # TODO: Remove by executing REV-2355 - if not enterprise_customer_for_request(request): # TODO: Remove by executing REV-2342 - if fbe_is_on: - return render_to_response("course_modes/fbe.html", context) - else: - return render_to_response("course_modes/unfbe.html", context) + if not enterprise_customer_for_request(request): # TODO: Remove by executing REV-2342 + if error: + return render_to_response("course_modes/error.html", context) + if fbe_is_on: + return render_to_response("course_modes/fbe.html", context) + else: + return render_to_response("course_modes/unfbe.html", context) - # If error or enterprise_customer, failover to old choose.html page + # If enterprise_customer, failover to old choose.html page return render_to_response("course_modes/choose.html", context) @method_decorator(transaction.non_atomic_requests) @@ -282,9 +288,10 @@ class ChooseModeView(View): course_id (unicode): The slash-separated course key. Returns: - Status code 400 when the requested mode is unsupported. When the honor mode - is selected, redirects to the dashboard. When the verified mode is selected, - returns error messages if the indicated contribution amount is invalid or + When the requested mode is unsupported, returns error message. + When the honor mode is selected, redirects to the dashboard. + When the verified mode is selected, returns error messages + if the indicated contribution amount is invalid or below the minimum, otherwise redirects to the verification flow. """ @@ -296,7 +303,10 @@ class ChooseModeView(View): course = modulestore().get_course(course_key) if not user.has_perm(ENROLL_IN_COURSE, course): error_msg = _("Enrollment is closed") - LOG.info('[Track Selection Check] Error: [%s], for course [%s]', error_msg, course_id) + LOG.info( + '[Track Selection Check] Error: [%s], for course [%s], user [%s]', + error_msg, course_id, request.user.username + ) return self.get(request, course_id, error=error_msg) requested_mode = self._get_requested_mode(request.POST) @@ -307,7 +317,8 @@ class ChooseModeView(View): '[Track Selection Check] Error: requested enrollment mode [%s] is not supported for course [%s]', requested_mode, course_id ) - return HttpResponseBadRequest(_("Enrollment mode not supported")) + error_msg = _("Enrollment mode not supported") + return self.get(request, course_id, error=error_msg) if requested_mode == 'audit': # If the learner has arrived at this screen via the traditional enrollment workflow, diff --git a/lms/static/sass/views/_track_selection.scss b/lms/static/sass/views/_track_selection.scss index a50e77d457..88e636e59c 100644 --- a/lms/static/sass/views/_track_selection.scss +++ b/lms/static/sass/views/_track_selection.scss @@ -219,6 +219,32 @@ content: ''; } + section.error-container { + height: 42rem; + } + + .error-button { + margin-top: 2.8rem; + margin-bottom: 1.5rem; + padding: 10px 16px; + box-shadow: none; + border-radius: 0px; + position: relative; + } + + .error-button span { + font-weight: 500; + font-size: 18px; + color: #fff; + } + + .error-button:hover { + background: #2D494E; + border-color: transparent; + color: #fff; + text-decoration: none; + } + @media (max-width: map-get($grid-breakpoints, 'sm')) { .collapsible { margin-bottom: 1.3rem; diff --git a/lms/templates/course_modes/error.html b/lms/templates/course_modes/error.html new file mode 100644 index 0000000000..93e384e52f --- /dev/null +++ b/lms/templates/course_modes/error.html @@ -0,0 +1,36 @@ +<%page expression_filter="h"/> +<%inherit file="../main.html" /> +<%! +from django.utils.translation import ugettext as _ +from openedx.core.djangolib.markup import HTML, Text +from django.urls import reverse +from openedx.core.djangolib.js_utils import js_escaped_string +%> + +<%namespace name='static' file='/static_content.html'/> +<%block name="bodyclass">step-select-track verification-process +<%block name="pagetitle"> + ${_("Unable to enroll in {course_name}").format(course_name=course_name)} + + +<%block name="header_extras"> + + + +<%block name="content"> +
+
+
+

${_("Sorry, we were unable to enroll you in")} ${_(course_name)}.

+
+

${_("Error:")} ${_(error)}

+ +
+
+ \ No newline at end of file diff --git a/lms/templates/course_modes/track_selection.html b/lms/templates/course_modes/track_selection.html index ca8feadd46..d151867179 100644 --- a/lms/templates/course_modes/track_selection.html +++ b/lms/templates/course_modes/track_selection.html @@ -1,6 +1,5 @@ <%page expression_filter="h"/> <%inherit file="../main.html" /> -<%namespace name='static' file='/static_content.html'/> <%! from django.utils.translation import ugettext as _ from openedx.core.djangolib.markup import HTML, Text