From 2a81fed0e13c759cc62cc74a30c0bfe719b302ee Mon Sep 17 00:00:00 2001 From: Attiya Ishaque Date: Tue, 24 Sep 2024 12:43:43 +0500 Subject: [PATCH] feat: remove all the commerce coordinator --- .../course_modes/tests/test_views.py | 44 ------- common/djangoapps/course_modes/views.py | 3 +- lms/djangoapps/commerce/tests/test_utils.py | 24 ---- lms/djangoapps/commerce/utils.py | 120 +----------------- lms/djangoapps/commerce/waffle.py | 49 ------- lms/envs/common.py | 7 - lms/templates/course_modes/choose.html | 3 +- .../course_modes/track_selection.html | 13 +- 8 files changed, 13 insertions(+), 250 deletions(-) delete mode 100644 lms/djangoapps/commerce/waffle.py diff --git a/common/djangoapps/course_modes/tests/test_views.py b/common/djangoapps/course_modes/tests/test_views.py index f49d1bc23b7b..d7b069865317 100644 --- a/common/djangoapps/course_modes/tests/test_views.py +++ b/common/djangoapps/course_modes/tests/test_views.py @@ -149,50 +149,6 @@ def test_no_id_redirect_otto(self): self.assertRedirects(response, '/test_basket/add/?sku=TEST', fetch_redirect_response=False) ecomm_test_utils.update_commerce_config(enabled=False) - def test_verified_mode_response_contains_course_run_key(self): - # Create only the verified mode and enroll the user - CourseModeFactory.create( - mode_slug='verified', - course_id=self.course_that_started.id, - min_price=149, - sku="dummy" - ) - CourseEnrollmentFactory( - is_active=True, - course_id=self.course_that_started.id, - 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): - with patch("common.djangoapps.course_modes.views.EcommerceService.is_enabled", return_value=True): - url = reverse('course_modes_choose', args=[str(self.course_that_started.id)]) - response = self.client.get(url) - self.assertContains(response, "&course_run_key=") - self.assertContains(response, self.course_that_started.id) - - def test_response_without_verified_sku_does_not_contain_course_run_key(self): - CourseModeFactory.create( - mode_slug='verified', - course_id=self.course_that_started.id, - ) - CourseEnrollmentFactory( - is_active=True, - course_id=self.course_that_started.id, - 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): - with patch("common.djangoapps.course_modes.views.EcommerceService.is_enabled", return_value=True): - url = reverse('course_modes_choose', args=[str(self.course_that_started.id)]) - response = self.client.get(url) - self.assertNotContains(response, "&course_run_key=") - @httpretty.activate @ddt.data( '', diff --git a/common/djangoapps/course_modes/views.py b/common/djangoapps/course_modes/views.py index 821c59dc0524..046767a26b89 100644 --- a/common/djangoapps/course_modes/views.py +++ b/common/djangoapps/course_modes/views.py @@ -195,7 +195,6 @@ def get(self, request, course_id, error=None): # lint-amnesty, pylint: disable= "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'), - "course_run_key": course_id, } context.update( get_experiment_user_metadata_context( @@ -241,7 +240,7 @@ def get(self, request, course_id, error=None): # lint-amnesty, pylint: disable= if verified_mode.sku: context["use_ecommerce_payment_flow"] = ecommerce_service.is_enabled(request.user) - context["ecommerce_payment_page"] = ecommerce_service.get_add_to_basket_url() + context["ecommerce_payment_page"] = ecommerce_service.payment_page_url() context["sku"] = verified_mode.sku context["bulk_sku"] = verified_mode.bulk_sku diff --git a/lms/djangoapps/commerce/tests/test_utils.py b/lms/djangoapps/commerce/tests/test_utils.py index 6c793433dff6..5e85bd4fde71 100644 --- a/lms/djangoapps/commerce/tests/test_utils.py +++ b/lms/djangoapps/commerce/tests/test_utils.py @@ -11,7 +11,6 @@ from django.test import TestCase from django.test.client import RequestFactory from django.test.utils import override_settings -from edx_toggles.toggles.testutils import override_waffle_flag from opaque_keys.edx.locator import CourseLocator from waffle.testutils import override_switch @@ -21,7 +20,6 @@ from common.djangoapps.student.tests.factories import TEST_PASSWORD, UserFactory from lms.djangoapps.commerce.models import CommerceConfiguration from lms.djangoapps.commerce.utils import EcommerceService, refund_entitlement, refund_seat -from lms.djangoapps.commerce.waffle import ENABLE_TRANSITION_TO_COORDINATOR_CHECKOUT from openedx.core.djangolib.testing.utils import skip_unless_lms from openedx.core.lib.log_utils import audit_log from xmodule.modulestore.tests.django_utils import \ @@ -186,28 +184,6 @@ def test_get_checkout_page_url_with_enterprise_catalog_uuid(self, skus, enterpri assert url == expected_url - @override_settings(COMMERCE_COORDINATOR_URL_ROOT='http://coordinator_url') - @override_settings(ECOMMERCE_PUBLIC_URL_ROOT='http://ecommerce_url') - @ddt.data( - {'coordinator_flag_active': True}, - {'coordinator_flag_active': False} - ) - @ddt.unpack - def test_get_add_to_basket_url(self, coordinator_flag_active): - with override_waffle_flag(ENABLE_TRANSITION_TO_COORDINATOR_CHECKOUT, active=coordinator_flag_active): - - ecommerce_service = EcommerceService() - result = ecommerce_service.get_add_to_basket_url() - - if coordinator_flag_active: - expected_url = 'http://coordinator_url/lms/payment_page_redirect/' - else: - expected_url = 'http://ecommerce_url/test_basket/add/' - - self.assertIsNotNone(result) - self.assertEqual(expected_url, result) - - @ddt.ddt @skip_unless_lms class RefundUtilMethodTests(ModuleStoreTestCase): diff --git a/lms/djangoapps/commerce/utils.py b/lms/djangoapps/commerce/utils.py index 82ed8c483024..be66ff86db88 100644 --- a/lms/djangoapps/commerce/utils.py +++ b/lms/djangoapps/commerce/utils.py @@ -11,6 +11,7 @@ from django.contrib.auth import get_user_model from django.urls import reverse from django.utils.translation import gettext as _ +from opaque_keys.edx.keys import CourseKey from common.djangoapps.course_modes.models import CourseMode from common.djangoapps.student.models import CourseEnrollmentAttribute @@ -23,10 +24,6 @@ from openedx.core.djangoapps.theming import helpers as theming_helpers from .models import CommerceConfiguration -from .waffle import ( # lint-amnesty, pylint: disable=invalid-django-waffle-import - should_redirect_to_commerce_coordinator_checkout, - should_redirect_to_commerce_coordinator_refunds, -) from edx_django_utils.plugins import pluggable_override log = logging.getLogger(__name__) @@ -110,15 +107,6 @@ def payment_page_url(self): """ return self.get_absolute_ecommerce_url(self.config.basket_checkout_page) - def get_add_to_basket_url(self): - """ Return the URL for the payment page based on the waffle switch. - Example: - http://localhost/enabled_service_api_path - """ - if should_redirect_to_commerce_coordinator_checkout(): - return urljoin(settings.COMMERCE_COORDINATOR_URL_ROOT, settings.COORDINATOR_CHECKOUT_REDIRECT_PATH) - return self.payment_page_url() - @pluggable_override('OVERRIDE_GET_CHECKOUT_PAGE_URL') def get_checkout_page_url(self, *skus, **kwargs): """ Construct the URL to the ecommerce checkout page and include products. @@ -260,10 +248,6 @@ def refund_seat(course_enrollment, change_mode=False): course_key_str = str(course_enrollment.course_id) enrollee = course_enrollment.user - if should_redirect_to_commerce_coordinator_refunds(): - if _refund_in_commerce_coordinator(course_enrollment, change_mode): - return - service_user = User.objects.get(username=settings.ECOMMERCE_SERVICE_WORKER_USERNAME) api_client = get_ecommerce_api_client(service_user) @@ -286,110 +270,16 @@ def refund_seat(course_enrollment, change_mode=False): mode=course_enrollment.mode, user=enrollee, ) - if change_mode: - _auto_enroll(course_enrollment) + if change_mode and CourseMode.can_auto_enroll(course_id=CourseKey.from_string(course_key_str)): + course_enrollment.update_enrollment(mode=CourseMode.auto_enroll_mode(course_id=course_key_str), + is_active=False, skip_refund=True) + course_enrollment.save() else: log.info('No refund opened for user [%s], course [%s]', enrollee.id, course_key_str) return refund_ids -def _refund_in_commerce_coordinator(course_enrollment, change_mode): - """ - Helper function to perform refund in Commerce Coordinator. - - Parameters: - course_enrollment (CourseEnrollment): the enrollment to refund. - change_mode (bool): whether the enrollment should be auto-enrolled into - the default course mode after refund. - - Returns: - bool: True if refund was performed. False if refund is not applicable - to Commerce Coordinator. - """ - enrollment_source_system = course_enrollment.get_order_attribute_value("source_system") - course_key_str = str(course_enrollment.course_id) - - # Commerce Coordinator enrollments will have an orders.source_system enrollment attribute. - # Redirect to Coordinator only if the source_system is safelisted as Coordinator's in settings. - - if enrollment_source_system and enrollment_source_system in settings.COMMERCE_COORDINATOR_REFUND_SOURCE_SYSTEMS: - log.info('Redirecting refund to Commerce Coordinator for user [%s], course [%s]...', - course_enrollment.user_id, course_key_str) - - # Re-use Ecommerce API client factory to build an API client for Commerce Coordinator... - service_user = get_user_model().objects.get( - username=settings.COMMERCE_COORDINATOR_SERVICE_WORKER_USERNAME - ) - api_client = get_ecommerce_api_client(service_user) - refunds_url = urljoin( - settings.COMMERCE_COORDINATOR_URL_ROOT, - settings.COMMERCE_COORDINATOR_REFUND_PATH - ) - - # Build request, raising exception if Coordinator returns non-200. - enrollment_attributes = CourseEnrollmentAttribute.get_enrollment_attributes(course_enrollment) - - try: - api_client.post( - refunds_url, - json={ - 'course_id': course_key_str, - 'username': course_enrollment.username, - 'enrollment_attributes': enrollment_attributes - } - ).raise_for_status() - - except Exception as exc: # pylint: disable=broad-except - # Catch any possible exceptions from the Commerce Coordinator service to ensure we fail gracefully - log.exception( - "Unexpected exception while attempting to refund user in Coordinator [%s], " - "course key [%s] message: [%s]", - course_enrollment.username, - course_key_str, - str(exc) - ) - - # Refund was successfully sent to Commerce Coordinator - log.info('Refund successfully sent to Commerce Coordinator for user [%s], course [%s].', - course_enrollment.user_id, course_key_str) - if change_mode: - _auto_enroll(course_enrollment) - return True - else: - # Refund was not meant to be sent to Commerce Coordinator - log.info('Continuing refund without Commerce Coordinator redirect for user [%s], course [%s]...', - course_enrollment.user_id, course_key_str) - return False - - -def _auto_enroll(course_enrollment): - """ - Helper method to update an enrollment to a default course mode. - - Arguments: - course_enrollment (CourseEnrollment): The course_enrollment to update. - - Returns: - bool: True if auto-enroll is successful. False if auto-enroll is not applicable. - """ - enrollment_course_id = course_enrollment.course_id - - if CourseMode.can_auto_enroll(course_id=enrollment_course_id): - auto_enroll_mode = CourseMode.auto_enroll_mode(course_id=enrollment_course_id) - course_enrollment.update_enrollment(mode=auto_enroll_mode, is_active=False, skip_refund=True) - course_enrollment.save() - log.info( - 'Auto-enrolled user [%s], course [%s] in mode [%s].', - course_enrollment.user_id, - enrollment_course_id, - auto_enroll_mode - ) - return True - else: - return False - - def _process_refund(refund_ids, api_client, mode, user, always_notify=False): """ Helper method to process a refund for a given course_product. This method assumes that the User has already diff --git a/lms/djangoapps/commerce/waffle.py b/lms/djangoapps/commerce/waffle.py deleted file mode 100644 index a36586a52d9b..000000000000 --- a/lms/djangoapps/commerce/waffle.py +++ /dev/null @@ -1,49 +0,0 @@ -""" -Configuration for features of Commerce App -""" -from edx_toggles.toggles import WaffleFlag - -# Namespace for Commerce waffle flags. -WAFFLE_FLAG_NAMESPACE = "commerce" - -# .. toggle_name: commerce.transition_to_coordinator.checkout -# .. toggle_implementation: WaffleFlag -# .. toggle_default: False -# .. toggle_description: Allows to redirect checkout to Commerce Coordinator API -# .. toggle_use_cases: temporary -# .. toggle_creation_date: 2023-11-22 -# .. toggle_target_removal_date: TBA -# .. toggle_tickets: SONIC-99 -# .. toggle_status: supported -ENABLE_TRANSITION_TO_COORDINATOR_CHECKOUT = WaffleFlag( - f"{WAFFLE_FLAG_NAMESPACE}.transition_to_coordinator.checkout", - __name__, -) - -# .. toggle_name: commerce.transition_to_coordinator.refund -# .. toggle_implementation: WaffleFlag -# .. toggle_default: False -# .. toggle_description: Allows to redirect refunds to Commerce Coordinator API -# .. toggle_use_cases: temporary -# .. toggle_creation_date: 2024-03-26 -# .. toggle_target_removal_date: TBA -# .. toggle_tickets: SONIC-382 -# .. toggle_status: supported -ENABLE_TRANSITION_TO_COORDINATOR_REFUNDS = WaffleFlag( - f"{WAFFLE_FLAG_NAMESPACE}.transition_to_coordinator.refunds", - __name__, -) - - -def should_redirect_to_commerce_coordinator_checkout(): - """ - Redirect learners to Commerce Coordinator checkout. - """ - return ENABLE_TRANSITION_TO_COORDINATOR_CHECKOUT.is_enabled() - - -def should_redirect_to_commerce_coordinator_refunds(): - """ - Redirect learners to Commerce Coordinator refunds. - """ - return ENABLE_TRANSITION_TO_COORDINATOR_REFUNDS.is_enabled() diff --git a/lms/envs/common.py b/lms/envs/common.py index 334669215397..a1ac42d81e65 100644 --- a/lms/envs/common.py +++ b/lms/envs/common.py @@ -4315,13 +4315,6 @@ def _make_locale_paths(settings): # pylint: disable=missing-function-docstring ECOMMERCE_SERVICE_WORKER_USERNAME = 'ecommerce_worker' ECOMMERCE_API_SIGNING_KEY = 'SET-ME-PLEASE' -# E-Commerce Commerce Coordinator Configuration -COMMERCE_COORDINATOR_URL_ROOT = 'http://localhost:8140' -COMMERCE_COORDINATOR_REFUND_PATH = '/lms/refund/' -COMMERCE_COORDINATOR_REFUND_SOURCE_SYSTEMS = ('SET-ME-PLEASE',) -COMMERCE_COORDINATOR_SERVICE_WORKER_USERNAME = 'commerce_coordinator_worker' -COORDINATOR_CHECKOUT_REDIRECT_PATH = '/lms/payment_page_redirect/' - # Exam Service EXAMS_SERVICE_URL = 'http://localhost:18740/api/v1' diff --git a/lms/templates/course_modes/choose.html b/lms/templates/course_modes/choose.html index 78b6dcc26ebc..6a15d48ec589 100644 --- a/lms/templates/course_modes/choose.html +++ b/lms/templates/course_modes/choose.html @@ -45,8 +45,7 @@ $('button[name=verified_mode]').click(function(e){ e.preventDefault(); window.location.href = '${ecommerce_payment_page | n, js_escaped_string}?sku=' + - encodeURIComponent('${sku | n, js_escaped_string}') + - '&course_run_key=' + encodeURIComponent('${course_run_key | n, js_escaped_string}'); + encodeURIComponent('${sku | n, js_escaped_string}') ; }); % endif }); diff --git a/lms/templates/course_modes/track_selection.html b/lms/templates/course_modes/track_selection.html index 557bd18844ca..3e26580ab2f1 100644 --- a/lms/templates/course_modes/track_selection.html +++ b/lms/templates/course_modes/track_selection.html @@ -21,8 +21,7 @@ $('button[name=verified_mode]').click(function(e){ e.preventDefault(); window.location.href = '${ecommerce_payment_page | n, js_escaped_string}?sku=' + - encodeURIComponent('${sku | n, js_escaped_string}') + - '&course_run_key=' + encodeURIComponent('${course_run_key | n, js_escaped_string}'); + encodeURIComponent('${sku | n, js_escaped_string}'); }); }); % endif @@ -35,9 +34,9 @@ $('.popover-icon').click(function(e){ e.stopPropagation(); if ($('.popover').css("visibility") == "hidden" || $('.popover').css("visibility") == "" ) { - $('.popover').css({"visibility":"visible", "opacity": 1}); + $('.popover').css({"visibility":"visible", "opacity": 1}); } else { - $('.popover').css({"visibility":"hidden", "opacity": 0}); + $('.popover').css({"visibility":"hidden", "opacity": 0}); } }); }); @@ -52,7 +51,7 @@ } window.addEventListener("resize", onresize); - // responsive: show more + // responsive: show more $(function(){ if($(window).width() <= 768) { $('.collapsible-item').css({"display":"none"}); @@ -64,7 +63,7 @@ e.preventDefault(); $('.collapsible').css({"display":"none"}); $('.collapsible-item').css({"display":"list-item"}); - }); + }); }); @@ -112,7 +111,7 @@

${currency_symbol}${min_price} ${currency}

${_("Earn a certificate")}

- <%block name="track_selection_certificate_bullets"/> + <%block name="track_selection_certificate_bullets"/>