From da348fa45633269b188d97c3d38c962680d5127d Mon Sep 17 00:00:00 2001 From: Deborah Kaplan Date: Fri, 24 Jan 2025 07:27:55 -0800 Subject: [PATCH] feat!: Removing the long-deprecated legacy course_modes chooser (#36156) * feat!: Removing the long-deprecated legacy course_modes chooser `course_modes/choose.html` (and its corresponding `_upgrade_button.html`) were specifically only used for the edge case where an enterprise user found themselves in the non-enterprise learner dashboard, and attempted to enroll in a course outside of the enterprise flow. Once upon a time, in a 2U-only workflow, the commerce system would apply specific discounts for users within the said case. That's no longer true, and it has never been true outside of this one company. Removing this template cleans up a legacy version of a legacy page that was, realistically, exclusively seen by employees of 2U, and nobody else. Removes: * The corresponding testsfor behavior only seen in the legacy page. * A waffle flag since all cases route as if the flag is set: `VALUE_PROP_TRACK_SELECTION_FLAG`: `course_modes.use_new_track_selection` * Some variables set in `CourseModeView` which were only ever rendered in the legacy template (`title_content`, `has_credit_upsell`) have been removed from the class. * There is a high likelihood that the class is still a target for re-factoring now that the legacy view is gone, but I'm hesitant to touch something which is not covered by previously existing tests, because the logic around what template gets rendered when is complex. FIXES: APER-3779 FIXES: #36090 --- .../course_modes/tests/test_views.py | 140 +++-------- common/djangoapps/course_modes/views.py | 52 +--- .../course_modes/_upgrade_button.html | 36 --- lms/templates/course_modes/choose.html | 233 ------------------ 4 files changed, 41 insertions(+), 420 deletions(-) delete mode 100644 lms/templates/course_modes/_upgrade_button.html delete mode 100644 lms/templates/course_modes/choose.html diff --git a/common/djangoapps/course_modes/tests/test_views.py b/common/djangoapps/course_modes/tests/test_views.py index d7b069865317..b4a777d2d677 100644 --- a/common/djangoapps/course_modes/tests/test_views.py +++ b/common/djangoapps/course_modes/tests/test_views.py @@ -21,7 +21,6 @@ from common.djangoapps.student.tests.factories import CourseEnrollmentFactory, UserFactory from common.djangoapps.util.testing import UrlResetMixin from common.djangoapps.util.tests.mixins.discovery import CourseCatalogServiceMockMixin -from edx_toggles.toggles.testutils import override_waffle_flag # lint-amnesty, pylint: disable=wrong-import-order from lms.djangoapps.commerce.tests import test_utils as ecomm_test_utils from lms.djangoapps.commerce.tests.mocks import mock_payment_processors from lms.djangoapps.verify_student.services import IDVerificationService @@ -33,8 +32,6 @@ from xmodule.modulestore.tests.django_utils import ModuleStoreTestCase # lint-amnesty, pylint: disable=wrong-import-order from xmodule.modulestore.tests.factories import CourseFactory # lint-amnesty, pylint: disable=wrong-import-order -from ..views import VALUE_PROP_TRACK_SELECTION_FLAG - # Name of the method to mock for Content Type Gating. GATING_METHOD_NAME = 'openedx.features.content_type_gating.models.ContentTypeGatingConfig.enabled_for_enrollment' @@ -186,27 +183,6 @@ def test_suggested_prices(self, price_list): # TODO: Fix it so that response.templates works w/ mako templates, and then assert # that the right template rendered - @httpretty.activate - @ddt.data( - (['honor', 'verified', 'credit'], True), - (['honor', 'verified'], False), - ) - @ddt.unpack - def test_credit_upsell_message(self, available_modes, show_upsell): - # Create the course modes - for mode in available_modes: - CourseModeFactory.create(mode_slug=mode, course_id=self.course.id) - - # Check whether credit upsell is shown on the page - # This should *only* be shown when a credit mode is available - url = reverse('course_modes_choose', args=[str(self.course.id)]) - response = self.client.get(url) - - if show_upsell: - self.assertContains(response, "Credit") - else: - self.assertNotContains(response, "Credit") - @httpretty.activate @patch('common.djangoapps.course_modes.views.enterprise_customer_for_request') @patch('common.djangoapps.course_modes.views.get_course_final_price') @@ -240,29 +216,6 @@ def test_display_after_discounted_price( self.assertContains(response, discounted_price) self.assertContains(response, verified_mode.min_price) - @httpretty.activate - @ddt.data(True, False) - def test_congrats_on_enrollment_message(self, create_enrollment): - # Create the course mode - CourseModeFactory.create(mode_slug='verified', course_id=self.course.id) - - if create_enrollment: - CourseEnrollmentFactory( - is_active=True, - course_id=self.course.id, - user=self.user - ) - - # Check whether congratulations message is shown on the page - # This should *only* be shown when an enrollment exists - url = reverse('course_modes_choose', args=[str(self.course.id)]) - response = self.client.get(url) - - if create_enrollment: - self.assertContains(response, "Congratulations! You are now enrolled in") - else: - self.assertNotContains(response, "Congratulations! You are now enrolled in") - @ddt.data('professional', 'no-id-professional') def test_professional_enrollment(self, mode): # The only course mode is professional ed @@ -529,26 +482,24 @@ def test_errors(self, has_perm, post_params, error_msg, status_code, mock_has_pe 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)]) + 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) + # 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') + 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)) + # 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, **_): """ @@ -607,33 +558,19 @@ def _assert_unfbe_page(self, response, min_price=None, **_): # Check for the HTML element for courses with more than one mode self.assertContains(response, '
') - def _assert_legacy_page(self, response, **_): - """ - Assert choose.html was rendered. - """ - # Check for string unique to the legacy choose.html. - self.assertContains(response, "Choose Your Track") - # This string only occurs in lms/templates/course_modes/choose.html - # and related theme and translation files. - @override_settings(MKTG_URLS={'ROOT': 'https://www.example.edx.org'}) @ddt.data( - # gated_content_on, course_duration_limits_on, waffle_flag_on, expected_page_assertion_function - (True, True, True, _assert_fbe_page), - (True, False, True, _assert_unfbe_page), - (False, True, True, _assert_unfbe_page), - (False, False, True, _assert_unfbe_page), - (True, True, False, _assert_legacy_page), - (True, False, False, _assert_legacy_page), - (False, True, False, _assert_legacy_page), - (False, False, False, _assert_legacy_page), + # gated_content_on, course_duration_limits_on, expected_page_assertion_function + (True, True, _assert_fbe_page), + (True, False, _assert_unfbe_page), + (False, True, _assert_unfbe_page), + (False, False, _assert_unfbe_page), ) @ddt.unpack def test_track_selection_types( self, gated_content_on, course_duration_limits_on, - waffle_flag_on, expected_page_assertion_function ): """ @@ -644,7 +581,6 @@ def test_track_selection_types( verified course modes), the learner may view 3 different pages: 1. fbe.html - full FBE 2. unfbe.html - partial or no FBE - 3. choose.html - legacy track selection page This test checks that the right template is rendered. @@ -667,15 +603,11 @@ def test_track_selection_types( user=self.user ) - # Value Prop TODO (REV-2378): remove waffle flag from tests once the new Track Selection template is rolled out. - # Check whether new track selection template is rendered. - # This should *only* be shown when the waffle flag is on. - with override_waffle_flag(VALUE_PROP_TRACK_SELECTION_FLAG, active=waffle_flag_on): - with patch(GATING_METHOD_NAME, return_value=gated_content_on): - with patch(CDL_METHOD_NAME, return_value=course_duration_limits_on): - url = reverse('course_modes_choose', args=[str(self.course_that_started.id)]) - response = self.client.get(url) - expected_page_assertion_function(self, response, min_price=verified_mode.min_price) + with patch(GATING_METHOD_NAME, return_value=gated_content_on): + with patch(CDL_METHOD_NAME, return_value=course_duration_limits_on): + url = reverse('course_modes_choose', args=[str(self.course_that_started.id)]) + response = self.client.get(url) + expected_page_assertion_function(self, response, min_price=verified_mode.min_price) def test_verified_mode_only(self): # Create only the verified mode and enroll the user @@ -690,18 +622,16 @@ def test_verified_mode_only(self): user=self.user ) - # Value Prop TODO (REV-2378): remove waffle flag from tests once the new Track Selection template is rolled out. - with override_waffle_flag(VALUE_PROP_TRACK_SELECTION_FLAG, active=True): - with patch(GATING_METHOD_NAME, return_value=True): - with patch(CDL_METHOD_NAME, return_value=True): - url = reverse('course_modes_choose', args=[str(self.course_that_started.id)]) - response = self.client.get(url) - # Check that only the verified option is rendered - self.assertNotContains(response, "Choose a path for your course in") - self.assertContains(response, "Earn a certificate") - self.assertNotContains(response, "Access this course") - self.assertContains(response, '
') - self.assertNotContains(response, '
') + with patch(GATING_METHOD_NAME, return_value=True): + with patch(CDL_METHOD_NAME, return_value=True): + url = reverse('course_modes_choose', args=[str(self.course_that_started.id)]) + response = self.client.get(url) + # Check that only the verified option is rendered + self.assertNotContains(response, "Choose a path for your course in") + self.assertContains(response, "Earn a certificate") + self.assertNotContains(response, "Access this course") + self.assertContains(response, '
') + self.assertNotContains(response, '
') @skip_unless_lms diff --git a/common/djangoapps/course_modes/views.py b/common/djangoapps/course_modes/views.py index 759073a13583..09164dc40e7c 100644 --- a/common/djangoapps/course_modes/views.py +++ b/common/djangoapps/course_modes/views.py @@ -29,7 +29,6 @@ from common.djangoapps.course_modes.helpers import get_course_final_price, get_verified_track_links from common.djangoapps.edxmako.shortcuts import render_to_response from common.djangoapps.util.date_utils import strftime_localized_html -from edx_toggles.toggles import WaffleFlag # lint-amnesty, pylint: disable=wrong-import-order from lms.djangoapps.commerce.utils import EcommerceService from lms.djangoapps.experiments.utils import get_experiment_user_metadata_context from lms.djangoapps.verify_student.services import IDVerificationService @@ -47,17 +46,6 @@ LOG = logging.getLogger(__name__) -# .. toggle_name: course_modes.use_new_track_selection -# .. toggle_implementation: WaffleFlag -# .. toggle_default: False -# .. toggle_description: This flag enables the use of the new track selection template for testing purposes before full rollout -# .. toggle_use_cases: temporary -# .. toggle_creation_date: 2021-8-23 -# .. toggle_target_removal_date: None -# .. toggle_tickets: REV-2133 -# .. toggle_warning: This temporary feature toggle does not have a target removal date. -VALUE_PROP_TRACK_SELECTION_FLAG = WaffleFlag('course_modes.use_new_track_selection', __name__) - class ChooseModeView(View): """View used when the user is asked to pick a mode. @@ -158,18 +146,6 @@ def get(self, request, course_id, error=None): # lint-amnesty, pylint: disable= ) return redirect('{}?{}'.format(reverse('dashboard'), params)) - # When a credit mode is available, students will be given the option - # to upgrade from a verified mode to a credit mode at the end of the course. - # This allows students who have completed photo verification to be eligible - # for university credit. - # Since credit isn't one of the selectable options on the track selection page, - # we need to check *all* available course modes in order to determine whether - # a credit mode is available. If so, then we show slightly different messaging - # for the verified track. - has_credit_upsell = any( - CourseMode.is_credit_mode(mode) for mode - in CourseMode.modes_for_course(course_key, only_selectable=False) - ) course_id = str(course_key) gated_content = ContentTypeGatingConfig.enabled_for_enrollment( user=request.user, @@ -184,7 +160,6 @@ def get(self, request, course_id, error=None): # lint-amnesty, pylint: disable= ), "modes": modes, "is_single_mode": is_single_mode, - "has_credit_upsell": has_credit_upsell, "course_name": course.display_name_with_default, "course_org": course.display_org_with_default, "course_num": course.display_number_with_default, @@ -204,14 +179,6 @@ def get(self, request, course_id, error=None): # lint-amnesty, pylint: disable= ) ) - title_content = '' - if enrollment_mode: - title_content = _("Congratulations! You are now enrolled in {course_name}").format( - course_name=course.display_name_with_default - ) - - context["title_content"] = title_content - if "verified" in modes: verified_mode = modes["verified"] context["suggested_prices"] = [ @@ -266,19 +233,12 @@ def get(self, request, course_id, error=None): # lint-amnesty, pylint: disable= context['audit_access_deadline'] = formatted_audit_access_date fbe_is_on = deadline and gated_content - # Route to correct Track Selection page. - # 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 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 enterprise_customer, failover to old choose.html page - return render_to_response("course_modes/choose.html", context) + 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) @method_decorator(transaction.non_atomic_requests) @method_decorator(login_required) diff --git a/lms/templates/course_modes/_upgrade_button.html b/lms/templates/course_modes/_upgrade_button.html deleted file mode 100644 index 02ff82c101ba..000000000000 --- a/lms/templates/course_modes/_upgrade_button.html +++ /dev/null @@ -1,36 +0,0 @@ -<%page args="content_gating_enabled, course_duration_limit_enabled, min_price, price_before_discount" expression_filter="h"/> - -<%! -from django.utils.translation import gettext as _ -from openedx.core.djangolib.markup import HTML, Text -%> - -<%namespace name='static' file='../static_content.html'/> - -
  • - - % if content_gating_enabled or course_duration_limit_enabled: - -
    • - -<%static:require_module_async module_name="js/commerce/track_ecommerce_events" class_name="TrackECommerceEvents"> -var upgradeLink = $("#track_selection_upgrade"); - -TrackECommerceEvents.trackUpsellClick(upgradeLink, 'track_selection', { - pageName: "track_selection", - linkType: "button", - linkCategory: "(none)" -}); - - \ No newline at end of file diff --git a/lms/templates/course_modes/choose.html b/lms/templates/course_modes/choose.html deleted file mode 100644 index 78b6dcc26ebc..000000000000 --- a/lms/templates/course_modes/choose.html +++ /dev/null @@ -1,233 +0,0 @@ -<%page expression_filter="h"/> -<%inherit file="../main.html" /> -<%! -from django.utils.translation import gettext as _ -from django.urls import reverse -from openedx.core.djangolib.js_utils import js_escaped_string -from openedx.core.djangolib.markup import HTML, Text -%> - -<%block name="bodyclass">register verification-process step-select-track -<%block name="pagetitle"> - ${_("Enroll In {course_name} | Choose Your Track").format(course_name=course_name)} - - -<%block name="js_extra"> - - - -<%block name="content"> - % if error: -
    -
    - -
    -

    ${_("Sorry, there was an error when trying to enroll you")}

    -
    -

    ${error}

    -
    -
    -
    -
    - %endif - -
    -
    -
    -
    -
    -

    - ${title_content} -

    -
    - -
    - <% - b_tag_kwargs = {'b_start': HTML(''), 'b_end': HTML('')} - %> - % if "verified" in modes: -
    -
    - - % if has_credit_upsell: - % if content_gating_enabled or course_duration_limit_enabled: -

    ${_("Pursue Academic Credit with the Verified Track")}

    - % else: -

    ${_("Pursue Academic Credit with a Verified Certificate")}

    - % endif - -
    -

    ${_("Become eligible for academic credit and highlight your new skills and knowledge with a verified certificate. Use this valuable credential to qualify for academic credit, advance your career, or strengthen your school applications.")}

    -

    -

    -
    - % if content_gating_enabled or course_duration_limit_enabled: -

    ${_("Benefits of the Verified Track")}

    -
      -
    • ${Text(_("{b_start}Eligible for credit:{b_end} Receive academic credit after successfully completing the course")).format(**b_tag_kwargs)}
      • - % if course_duration_limit_enabled: -
    • ${Text(_("{b_start}Unlimited Course Access: {b_end}Learn at your own pace, and access materials anytime to brush up on what you've learned.")).format(**b_tag_kwargs)}
      • - % endif - % if content_gating_enabled: -
    • ${Text(_("{b_start}Graded Assignments: {b_end}Build your skills through graded assignments and projects.")).format(**b_tag_kwargs)}
      • - % endif -
    • ${Text(_("{b_start}Easily Sharable: {b_end}Add the certificate to your CV or resumé, or post it directly on LinkedIn.")).format(**b_tag_kwargs)}
      • -
    - % else: -

    ${_("Benefits of a Verified Certificate")}

    -
      -
    • ${Text(_("{b_start}Eligible for credit:{b_end} Receive academic credit after successfully completing the course")).format(**b_tag_kwargs)}
      • -
    • ${Text(_("{b_start}Official:{b_end} Receive an instructor-signed certificate with the institution's logo")).format(**b_tag_kwargs)}
      • -
    • ${Text(_("{b_start}Easily shareable:{b_end} Add the certificate to your CV or resumé, or post it directly on LinkedIn")).format(**b_tag_kwargs)}
      • -
    - % endif -
    -
    -
      - <%include file='_upgrade_button.html' args='content_gating_enabled=content_gating_enabled, course_duration_limit_enabled=course_duration_limit_enabled, currency=currency, currency_symbol=currency_symbol, min_price=min_price, price_before_discount=price_before_discount' /> -
    -
    -
    -

    -
    - % else: - % if content_gating_enabled or course_duration_limit_enabled: -

    ${_("Pursue the Verified Track")}

    - % else: -

    ${_("Pursue a Verified Certificate")}

    - % endif - - -
    -

    ${_("Highlight your new knowledge and skills with a verified certificate. Use this valuable credential to improve your job prospects and advance your career, or highlight your certificate in school applications.")}

    -

    -

    -
    - % if content_gating_enabled or course_duration_limit_enabled: -

    ${_("Benefits of the Verified Track")}

    -
      - % if course_duration_limit_enabled: -
    • ${Text(_("{b_start}Unlimited Course Access: {b_end}Learn at your own pace, and access materials anytime to brush up on what you've learned.")).format(**b_tag_kwargs)}
      • - % endif - % if content_gating_enabled: -
    • ${Text(_("{b_start}Graded Assignments: {b_end}Build your skills through graded assignments and projects.")).format(**b_tag_kwargs)}
      • - % endif -
    • ${Text(_("{b_start}Easily Sharable: {b_end}Add the certificate to your CV or resumé, or post it directly on LinkedIn.")).format(**b_tag_kwargs)}
      • -
    - % else: -

    ${_("Benefits of a Verified Certificate")}

    -
      -
    • ${Text(_("{b_start}Official: {b_end}Receive an instructor-signed certificate with the institution's logo")).format(**b_tag_kwargs)}
      • -
    • ${Text(_("{b_start}Easily shareable: {b_end}Add the certificate to your CV or resumé, or post it directly on LinkedIn")).format(**b_tag_kwargs)}
      • -
    • ${Text(_("{b_start}Motivating: {b_end}Give yourself an additional incentive to complete the course")).format(**b_tag_kwargs)}
      • -
    - % endif -
    -
    -
      - <%include file='_upgrade_button.html' args='content_gating_enabled=content_gating_enabled, course_duration_limit_enabled=course_duration_limit_enabled, currency=currency, currency_symbol=currency_symbol, min_price=min_price, price_before_discount=price_before_discount' /> -
    -
    -
    -

    -
    - % endif -
    -
    - % endif - - % if "honor" in modes: - - ${_("or")} - - -
    -
    - -

    ${_("Audit This Course")}

    -
    -

    ${_("Audit this course for free and have complete access to all the course material, activities, tests, and forums.")}

    -
    -
    - -
      -
    • - -
      • -
    -
    - % elif "audit" in modes: - - ${_("or")} - - -
    -
    - -

    ${_("Audit This Course (No Certificate)")}

    -
    - ## Translators: b_start notes the beginning of a section of text bolded for emphasis, and b_end marks the end of the bolded text. - % if content_gating_enabled and course_duration_limit_enabled: -

    ${Text(_("Audit this course for free and have access to course materials and discussions forums. {b_start}This track does not include graded assignments, or unlimited course access.{b_end}")).format(**b_tag_kwargs)}

    - % elif content_gating_enabled and not course_duration_limit_enabled: -

    ${Text(_("Audit this course for free and have access to course materials and discussions forums. {b_start}This track does not include graded assignments.{b_end}")).format(**b_tag_kwargs)}

    - % elif not content_gating_enabled and course_duration_limit_enabled: -

    ${Text(_("Audit this course for free and have access to course materials and discussions forums. {b_start}This track does not include unlimited course access.{b_end}")).format(**b_tag_kwargs)}

    - % else: -

    ${Text(_("Audit this course for free and have complete access to all the course material, activities, tests, and forums. {b_start}Please note that this track does not offer a certificate for learners who earn a passing grade.{b_end}")).format(**b_tag_kwargs)}

    - % endif -
    -
    - -
      -
    • - -
      • -
    -
    - % endif - - -
    -
    -
    -
    -
    -