Skip to content

Commit

Permalink
fix: use "price" in ecommerce data loader (openedx#4450)
Browse files Browse the repository at this point in the history 
* chore: bump upload-artifact GH task to v4

* fix: fallback to 'price' in ecommerce api loader

The Django Oscar upgrade of ecommerce changed the
item's price field from `price_excl_tax` to just `price`
causing the EcommerceApi data loader to fail.

This commit checks for the `price_excl_tax` key and
falls back to the 'price' value.

Ref: openedx-unsupported/ecommerce#4050

* refactor: apply review suggestion on fix

* fix: update all references of `price_excl_tax` to price

* fix: remove extra item added accidentally
  • Loading branch information
@tecoholic
tecoholic committed Oct 4, 2024
1 parent 50a9cc3 commit f6d429f
Show file tree
Hide file tree
Showing 3 changed files with 21 additions and 19 deletions.
12 changes: 7 additions & 5 deletions course_discovery/apps/course_metadata/data_loaders/api.py
View file
Original file line number Diff line number Diff line change
Expand Up @@ -573,7 +573,7 @@ def update_seats(self, body):
def update_seat(self, course_run, product_body):
stock_record = product_body['stockrecords'][0]
currency_code = stock_record['price_currency']
price = Decimal(stock_record['price_excl_tax'])
price = Decimal(stock_record.get('price_excl_tax', stock_record['price']))
sku = stock_record['partner_sku']

# For more context see ADR docs/decisions/0025-dont-sync-mobile-skus-on-discovery.rst
Expand Down Expand Up @@ -644,9 +644,11 @@ def update_seat(self, course_run, product_body):
def validate_stockrecord(self, stockrecords, title, product_class):
"""
Argument:
body (dict): product data from ecommerce, either entitlement or enrollment code
sockrecords (list): a list of stock records to validate from ecommerce
title (str): product title
product_class (str): either entitlement or enrollment code
Returns:
product sku if no exceptions, else None
True when all validation checks pass, else None
"""
# Map product_class keys with how they should be displayed in the exception messages.
product_classes = {
Expand Down Expand Up @@ -681,7 +683,7 @@ def validate_stockrecord(self, stockrecords, title, product_class):

try:
currency_code = stock_record['price_currency']
Decimal(stock_record['price_excl_tax'])
Decimal(stock_record.get('price_excl_tax', stock_record['price']))
sku = stock_record['partner_sku']
except (KeyError, ValueError):
msg = 'A necessary stockrecord field is missing or incorrectly set for {product} {title}'.format(
Expand Down Expand Up @@ -720,7 +722,7 @@ def update_entitlement(self, body):

stock_record = stockrecords[0]
currency_code = stock_record['price_currency']
price = Decimal(stock_record['price_excl_tax'])
price = Decimal(stock_record.get('price_excl_tax', stock_record['price']))
sku = stock_record['partner_sku']

try:
Expand Down
22 changes: 11 additions & 11 deletions course_discovery/apps/course_metadata/data_loaders/tests/mock_data.py
View file
Original file line number Diff line number Diff line change
Expand Up @@ -195,7 +195,7 @@
"stockrecords": [
{
"price_currency": "USD",
"price_excl_tax": "0.00",
"price": "0.00",
"partner_sku": "sku001",
}
]
Expand Down Expand Up @@ -225,7 +225,7 @@
"stockrecords": [
{
"price_currency": "EUR",
"price_excl_tax": "0.00",
"price": "0.00",
"partner_sku": "sku002",
}
]
Expand All @@ -242,7 +242,7 @@
"stockrecords": [
{
"price_currency": "EUR",
"price_excl_tax": "25.00",
"price": "25.00",
"partner_sku": "sku003",
}
]
Expand All @@ -260,7 +260,7 @@
"stockrecords": [
{
"price_currency": "EUR",
"price_excl_tax": "250.00",
"price": "250.00",
"partner_sku": "mobile.android.sku003",
}
]
Expand All @@ -277,7 +277,7 @@
"stockrecords": [
{
"price_currency": "EUR",
"price_excl_tax": "25.00",
"price": "25.00",
"partner_sku": "sku004"
}
]
Expand All @@ -304,7 +304,7 @@
"stockrecords": [
{
"price_currency": "USD",
"price_excl_tax": "0.00",
"price": "0.00",
"partner_sku": "sku005",
}
]
Expand All @@ -321,7 +321,7 @@
"stockrecords": [
{
"price_currency": "USD",
"price_excl_tax": "25.00",
"price": "25.00",
"partner_sku": "sku006",
}
]
Expand Down Expand Up @@ -350,7 +350,7 @@
"stockrecords": [
{
"price_currency": "USD",
"price_excl_tax": "250.00",
"price": "250.00",
"partner_sku": "sku007",
}
]
Expand Down Expand Up @@ -379,7 +379,7 @@
"stockrecords": [
{
"price_currency": "USD",
"price_excl_tax": "250.00",
"price": "250.00",
"partner_sku": "sku008",
}
]
Expand All @@ -404,7 +404,7 @@
"stockrecords": [
{
"price_currency": "123",
"price_excl_tax": "0.00",
"price": "0.00",
"partner_sku": "sku009",
}
]
Expand All @@ -429,7 +429,7 @@
"stockrecords": [
{
"price_currency": "USD",
"price_excl_tax": "0.00",
"price": "0.00",
"partner_sku": "sku010",
}
]
Expand Down
6 changes: 3 additions & 3 deletions course_discovery/apps/course_metadata/data_loaders/tests/test_api.py
View file
Original file line number Diff line number Diff line change
Expand Up @@ -738,7 +738,7 @@ def mock_products_api(self, alt_course=None, alt_currency=None, alt_mode=None, h

stockrecord = {
"price_currency": alt_currency if alt_currency else "USD",
"price_excl_tax": "10.00",
"price": "10.00",
}
if valid_stockrecord:
stockrecord.update({"partner_sku": "sku132"})
Expand Down Expand Up @@ -825,7 +825,7 @@ def assert_seats_loaded(self, body, mock_products):
for product in products:
stock_record = product['stockrecords'][0]
price_currency = stock_record['price_currency']
price = Decimal(stock_record['price_excl_tax'])
price = Decimal(stock_record['price'])
sku = stock_record['partner_sku']
certificate_type = Seat.AUDIT
credit_provider = None
Expand Down Expand Up @@ -877,7 +877,7 @@ def assert_entitlements_loaded(self, body):
course = Course.objects.get(uuid=attributes['UUID'])
stock_record = datum['stockrecords'][0]
price_currency = stock_record['price_currency']
price = Decimal(stock_record['price_excl_tax'])
price = Decimal(stock_record['price'])
sku = stock_record['partner_sku']

mode_name = attributes['certificate_type']
Expand Down

0 comments on commit f6d429f

Please sign in to comment.