Skip to content
This repository has been archived by the owner on Nov 4, 2024. It is now read-only.

fix: Embargo API fixed with new product type. #4034

Closed
wants to merge 3 commits into from
Closed

fix: Embargo API fixed with new product type. #4034

Changes from 2 commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
8ae474e
fix: Updated api callback with new product type and fixed API params
mobeenali12 Sep 25, 2023
88b8eec
fix: Comment updated
mobeenali12 Sep 25, 2023
5a31902
fix: Added headers to embargo api request.
mobeenali12 Oct 6, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
15 changes: 9 additions & 6 deletions ecommerce/extensions/payment/utils.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@
from django.utils.translation import ugettext_lazy as _
from oscar.core.loading import get_model

from ecommerce.core.constants import SEAT_PRODUCT_CLASS_NAME
from ecommerce.core.constants import SEAT_PRODUCT_CLASS_NAME, COURSE_ENTITLEMENT_PRODUCT_CLASS_NAME
from ecommerce.extensions.analytics.utils import parse_tracking_context

logger = logging.getLogger(__name__)
Expand Down Expand Up @@ -100,7 +100,6 @@ def clean_field_value(value):
"""
return re.sub(r'[\^:"\']', '', value)


def embargo_check(user, site, products, ip=None):
""" Checks if the user has access to purchase products by calling the LMS embargo API.

Expand All @@ -118,14 +117,18 @@ def embargo_check(user, site, products, ip=None):
_, _, ip = parse_tracking_context(user, usage='embargo')

for product in products:
# We only are checking Seats
if product.get_product_class().name == SEAT_PRODUCT_CLASS_NAME:
product_class_name = product.get_product_class().name

# We are checking Seats & Course Entitlement
if product_class_name == SEAT_PRODUCT_CLASS_NAME or product_class_name == COURSE_ENTITLEMENT_PRODUCT_CLASS_NAME:
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The last time when I reported this issue I tried using the same fix but it didn't work since for course entitlement products we didn't have course attribute set on them and hence the code would raise attribute errors as far as I remember. I would recommend double checking the code by debugging it locally and testing all the scenarios.

courses.append(product.course.id)

if courses:
params = {
'user': user,
'ip_address': ip,
'user': user.username,
# ip variable is None when testing it locally, giving it dummy string "test_local_ip"
# just to bypass the API, otherwise API returns "Missing Parameters" response.
'ip_address': ip or "test_local_ip",
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

we should not have a test string hard coded here, especially if it's only for local testing. if we need behavior that varies whether you are in a local dev environment vs stage/prod, i recommend we instead use a config variable that, if true, will override what the ip variable with something that works. that way, we prevent a user from potentially spoofing the values in their context to trigger this test behavior

'course_ids': courses
}

Expand Down
Loading