Skip to content

Commit

Permalink
feat!: Removing the long-deprecated legacy course_modes chooser
Browse files Browse the repository at this point in the history 
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 committed Jan 23, 2025
1 parent fd33a2f commit 15c2655
Show file tree
Hide file tree
Showing 2 changed files with 41 additions and 131 deletions.
125 changes: 35 additions & 90 deletions common/djangoapps/course_modes/tests/test_views.py
View file
Original file line number Diff line number Diff line change
Expand Up @@ -33,8 +33,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 +184,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 +217,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 +483,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 @@ -609,18 +561,17 @@ def _assert_unfbe_page(self, response, min_price=None, **_):

@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),
# 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 Down Expand Up @@ -653,15 +604,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 @@ -676,18 +623,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
47 changes: 6 additions & 41 deletions common/djangoapps/course_modes/views.py
View file
Original file line number Diff line number Diff line change
Expand Up @@ -47,17 +47,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 +147,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 +161,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 +180,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,15 +234,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 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:
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

0 comments on commit 15c2655

Please sign in to comment.