Skip to content

Commit

Permalink
feat!: Removing the long-deprecated legacy course_modes chooser (#36156)
Browse files Browse the repository at this point in the history 
* 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
  • Loading branch information
@deborahgu
deborahgu authored Jan 24, 2025
1 parent 5f13b5e commit da348fa
Show file tree
Hide file tree
Showing 4 changed files with 41 additions and 420 deletions.
140 changes: 35 additions & 105 deletions common/djangoapps/course_modes/tests/test_views.py
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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'

Expand Down Expand Up @@ -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')
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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, '<span>Explore all courses</span>')
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, '<span>Explore all courses</span>')
else:
self.assertTrue(CourseEnrollment.is_enrollment_closed(self.user, self.course))

def _assert_fbe_page(self, response, min_price=None, **_):
"""
Expand Down Expand Up @@ -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, '<div class="grid-options">')

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
):
"""
Expand All @@ -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.
Expand All @@ -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
Expand All @@ -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, '<div class="grid-single">')
self.assertNotContains(response, '<div class="grid-options">')
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, '<div class="grid-single">')
self.assertNotContains(response, '<div class="grid-options">')


@skip_unless_lms
Expand Down
52 changes: 6 additions & 46 deletions common/djangoapps/course_modes/views.py
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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.
Expand Down Expand Up @@ -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,
Expand All @@ -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,
Expand All @@ -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"] = [
Expand Down Expand Up @@ -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)
Expand Down
36 changes: 0 additions & 36 deletions lms/templates/course_modes/_upgrade_button.html
View file

This file was deleted.

Loading

0 comments on commit da348fa

Please sign in to comment.