Skip to content

Commit

Permalink
Merge branch 'navin/component-collection-api' into rpenido/navin/comp…
Browse files Browse the repository at this point in the history
…onent-collection-api-fix-meilisearch
  • Loading branch information
navinkarkera committed Oct 15, 2024
2 parents 758334f + c4ce75f commit 4248639
Show file tree
Hide file tree
Showing 26 changed files with 134 additions and 349 deletions.
2 changes: 1 addition & 1 deletion .github/workflows/ci-static-analysis.yml
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@ jobs:
matrix:
python-version:
- "3.11"
os: ["ubuntu-latest"]
os: ["ubuntu-22.04"]

steps:
- uses: actions/checkout@v4
Expand Down
4 changes: 2 additions & 2 deletions .github/workflows/migrations-check.yml
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@ jobs:
runs-on: ${{ matrix.os }}
strategy:
matrix:
os: [ubuntu-latest]
os: [ubuntu-22.04]
python-version:
- "3.11"
# 'pinned' is used to install the latest patch version of Django
Expand Down Expand Up @@ -126,7 +126,7 @@ jobs:
if: always()
needs:
- check_migrations
runs-on: ubuntu-latest
runs-on: ubuntu-22.04
steps:
- name: Decide whether the needed jobs succeeded or failed
# uses: re-actors/[email protected]
Expand Down
4 changes: 2 additions & 2 deletions .github/workflows/pylint-checks.yml
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@ on:

jobs:
run-pylint:
runs-on: ubuntu-latest
runs-on: ubuntu-22.04
strategy:
fail-fast: false
matrix:
Expand Down Expand Up @@ -75,7 +75,7 @@ jobs:
if: always()
needs:
- run-pylint
runs-on: ubuntu-latest
runs-on: ubuntu-22.04
steps:
- name: Decide whether the needed jobs succeeded or failed
# uses: re-actors/[email protected]
Expand Down
2 changes: 1 addition & 1 deletion .github/workflows/quality-checks.yml
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@ jobs:
runs-on: ${{ matrix.os }}
strategy:
matrix:
os: [ubuntu-latest]
os: [ubuntu-22.04]
python-version:
- "3.11"
node-version: [20]
Expand Down
2 changes: 1 addition & 1 deletion .github/workflows/static-assets-check.yml
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@ jobs:
runs-on: ${{ matrix.os }}
strategy:
matrix:
os: [ubuntu-latest]
os: [ubuntu-22.04]
python-version:
- "3.11"
node-version: [18, 20]
Expand Down
12 changes: 6 additions & 6 deletions .github/workflows/unit-tests.yml
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@ concurrency:
jobs:
run-tests:
name: ${{ matrix.shard_name }}(py=${{ matrix.python-version }},dj=${{ matrix.django-version }},mongo=${{ matrix.mongo-version }})
runs-on: ubuntu-latest
runs-on: ubuntu-22.04
strategy:
matrix:
python-version:
Expand Down Expand Up @@ -164,7 +164,7 @@ jobs:
overwrite: true

collect-and-verify:
runs-on: ubuntu-latest
runs-on: ubuntu-22.04
steps:
- uses: actions/checkout@v4
- name: Setup Python
Expand Down Expand Up @@ -229,7 +229,7 @@ jobs:
# https://github.com/orgs/community/discussions/33579
success:
name: Unit tests successful
runs-on: ubuntu-latest
runs-on: ubuntu-22.04
if: always()
needs: [run-tests]
steps:
Expand All @@ -240,7 +240,7 @@ jobs:
jobs: ${{ toJSON(needs) }}

compile-warnings-report:
runs-on: ubuntu-latest
runs-on: ubuntu-22.04
needs: [run-tests]
steps:
- uses: actions/checkout@v4
Expand Down Expand Up @@ -268,7 +268,7 @@ jobs:
overwrite: true

merge-artifacts:
runs-on: ubuntu-latest
runs-on: ubuntu-22.04
needs: [compile-warnings-report]
steps:
- name: Merge Pytest Warnings JSON Artifacts
Expand All @@ -288,7 +288,7 @@ jobs:
# Combine and upload coverage reports.
coverage:
if: (github.repository == 'edx/edx-platform-private') || (github.repository == 'openedx/edx-platform' && (startsWith(github.base_ref, 'open-release') == false))
runs-on: ubuntu-latest
runs-on: ubuntu-22.04
needs: [run-tests]
strategy:
matrix:
Expand Down
44 changes: 0 additions & 44 deletions common/djangoapps/course_modes/tests/test_views.py
Original file line number Diff line number Diff line change
Expand Up @@ -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(
'',
Expand Down
2 changes: 1 addition & 1 deletion common/djangoapps/course_modes/views.py
Original file line number Diff line number Diff line change
Expand Up @@ -241,7 +241,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

Expand Down
23 changes: 0 additions & 23 deletions lms/djangoapps/commerce/tests/test_utils.py
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand All @@ -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 \
Expand Down Expand Up @@ -186,27 +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
Expand Down
87 changes: 0 additions & 87 deletions lms/djangoapps/commerce/utils.py
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,6 @@
from django.utils.translation import gettext as _

from common.djangoapps.course_modes.models import CourseMode
from common.djangoapps.student.models import CourseEnrollmentAttribute
from openedx.core.djangoapps.commerce.utils import (
get_ecommerce_api_base_url,
get_ecommerce_api_client,
Expand All @@ -23,10 +22,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__)
Expand Down Expand Up @@ -110,15 +105,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.
Expand Down Expand Up @@ -261,10 +247,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)

Expand Down Expand Up @@ -295,75 +277,6 @@ def refund_seat(course_enrollment, change_mode=False):
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.
Expand Down
49 changes: 0 additions & 49 deletions lms/djangoapps/commerce/waffle.py

This file was deleted.

Loading

0 comments on commit 4248639

Please sign in to comment.