From f6d429f786f37024e8795036a387f4690f38f109 Mon Sep 17 00:00:00 2001 From: Arunmozhi Date: Thu, 3 Oct 2024 04:47:42 +1000 Subject: [PATCH] fix: use "price" in ecommerce data loader (#4450) * 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: https://github.com/openedx/ecommerce/pull/4050 * refactor: apply review suggestion on fix * fix: update all references of `price_excl_tax` to price * fix: remove extra item added accidentally --- .../apps/course_metadata/data_loaders/api.py | 12 +++++----- .../data_loaders/tests/mock_data.py | 22 +++++++++---------- .../data_loaders/tests/test_api.py | 6 ++--- 3 files changed, 21 insertions(+), 19 deletions(-) diff --git a/course_discovery/apps/course_metadata/data_loaders/api.py b/course_discovery/apps/course_metadata/data_loaders/api.py index 1c16ad920f..ed49343ba3 100644 --- a/course_discovery/apps/course_metadata/data_loaders/api.py +++ b/course_discovery/apps/course_metadata/data_loaders/api.py @@ -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 @@ -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 = { @@ -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( @@ -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: diff --git a/course_discovery/apps/course_metadata/data_loaders/tests/mock_data.py b/course_discovery/apps/course_metadata/data_loaders/tests/mock_data.py index a84d870cbb..5013e898f6 100644 --- a/course_discovery/apps/course_metadata/data_loaders/tests/mock_data.py +++ b/course_discovery/apps/course_metadata/data_loaders/tests/mock_data.py @@ -195,7 +195,7 @@ "stockrecords": [ { "price_currency": "USD", - "price_excl_tax": "0.00", + "price": "0.00", "partner_sku": "sku001", } ] @@ -225,7 +225,7 @@ "stockrecords": [ { "price_currency": "EUR", - "price_excl_tax": "0.00", + "price": "0.00", "partner_sku": "sku002", } ] @@ -242,7 +242,7 @@ "stockrecords": [ { "price_currency": "EUR", - "price_excl_tax": "25.00", + "price": "25.00", "partner_sku": "sku003", } ] @@ -260,7 +260,7 @@ "stockrecords": [ { "price_currency": "EUR", - "price_excl_tax": "250.00", + "price": "250.00", "partner_sku": "mobile.android.sku003", } ] @@ -277,7 +277,7 @@ "stockrecords": [ { "price_currency": "EUR", - "price_excl_tax": "25.00", + "price": "25.00", "partner_sku": "sku004" } ] @@ -304,7 +304,7 @@ "stockrecords": [ { "price_currency": "USD", - "price_excl_tax": "0.00", + "price": "0.00", "partner_sku": "sku005", } ] @@ -321,7 +321,7 @@ "stockrecords": [ { "price_currency": "USD", - "price_excl_tax": "25.00", + "price": "25.00", "partner_sku": "sku006", } ] @@ -350,7 +350,7 @@ "stockrecords": [ { "price_currency": "USD", - "price_excl_tax": "250.00", + "price": "250.00", "partner_sku": "sku007", } ] @@ -379,7 +379,7 @@ "stockrecords": [ { "price_currency": "USD", - "price_excl_tax": "250.00", + "price": "250.00", "partner_sku": "sku008", } ] @@ -404,7 +404,7 @@ "stockrecords": [ { "price_currency": "123", - "price_excl_tax": "0.00", + "price": "0.00", "partner_sku": "sku009", } ] @@ -429,7 +429,7 @@ "stockrecords": [ { "price_currency": "USD", - "price_excl_tax": "0.00", + "price": "0.00", "partner_sku": "sku010", } ] diff --git a/course_discovery/apps/course_metadata/data_loaders/tests/test_api.py b/course_discovery/apps/course_metadata/data_loaders/tests/test_api.py index 57b1a0575e..6d3c04f06c 100644 --- a/course_discovery/apps/course_metadata/data_loaders/tests/test_api.py +++ b/course_discovery/apps/course_metadata/data_loaders/tests/test_api.py @@ -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"}) @@ -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 @@ -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']