From 2967c575a65a09896045b32e4197a3107744c01a Mon Sep 17 00:00:00 2001 From: Ahtesham Quraish Date: Wed, 6 Nov 2024 11:24:52 +0500 Subject: [PATCH 1/4] refactor: make course mode logic dynamic Description: Make Fulfillment logic dynamic to support all course modes SONIC-732 --- .../apps/commercetools/catalog_info/edx_utils.py | 10 ++++++++++ .../apps/commercetools/sub_messages/tasks.py | 3 ++- .../commercetools/tests/catalog_info/test_edx_utils.py | 4 ++++ .../apps/commercetools/tests/raw_ct_order.json | 4 ++++ 4 files changed, 20 insertions(+), 1 deletion(-) diff --git a/commerce_coordinator/apps/commercetools/catalog_info/edx_utils.py b/commerce_coordinator/apps/commercetools/catalog_info/edx_utils.py index 78a32931..50187d81 100644 --- a/commerce_coordinator/apps/commercetools/catalog_info/edx_utils.py +++ b/commerce_coordinator/apps/commercetools/catalog_info/edx_utils.py @@ -40,6 +40,16 @@ def is_edx_lms_order(order: CTOrder) -> bool: return len(get_edx_items(order)) >= 1 +def get_course_mode_from_ct_order(order: CTOrder) -> str: + mode = 'verified' + for line_item in order.line_items: + for attribute in line_item.variant.attributes: + if attribute.name == 'mode': + mode = attribute.value + + return mode + + def get_edx_lms_user_id(customer: CTCustomer): return customer.custom.fields[EdXFieldNames.LMS_USER_ID] diff --git a/commerce_coordinator/apps/commercetools/sub_messages/tasks.py b/commerce_coordinator/apps/commercetools/sub_messages/tasks.py index 3ec13378..77996b65 100644 --- a/commerce_coordinator/apps/commercetools/sub_messages/tasks.py +++ b/commerce_coordinator/apps/commercetools/sub_messages/tasks.py @@ -9,6 +9,7 @@ from commerce_coordinator.apps.commercetools.catalog_info.constants import TwoUKeys from commerce_coordinator.apps.commercetools.catalog_info.edx_utils import ( + get_course_mode_from_ct_order, get_edx_is_sanctioned, get_edx_items, get_edx_lms_user_id, @@ -83,7 +84,7 @@ def fulfill_order_placed_message_signal_task( 'order_id': order.id, 'provider_id': None, 'edx_lms_user_id': lms_user_id, - 'course_mode': 'verified', + 'course_mode': get_course_mode_from_ct_order(order=order), 'date_placed': order.last_modified_at.strftime(ISO_8601_FORMAT), 'source_system': source_system, } diff --git a/commerce_coordinator/apps/commercetools/tests/catalog_info/test_edx_utils.py b/commerce_coordinator/apps/commercetools/tests/catalog_info/test_edx_utils.py index d74f1229..ef1f7a36 100644 --- a/commerce_coordinator/apps/commercetools/tests/catalog_info/test_edx_utils.py +++ b/commerce_coordinator/apps/commercetools/tests/catalog_info/test_edx_utils.py @@ -6,6 +6,7 @@ from commercetools.platform.models import Order as CTOrder from commerce_coordinator.apps.commercetools.catalog_info.edx_utils import ( + get_course_mode_from_ct_order, get_edx_items, get_edx_lms_user_id, get_edx_lms_user_name, @@ -58,6 +59,9 @@ def test_get_edx_items(self): def test_is_edx_lms_order(self): self.assertTrue(is_edx_lms_order(self.order)) + def test_get_course_mode_from_ct_order(self): + self.assertEqual(get_course_mode_from_ct_order(self.order), 'certified') + def test_get_edx_lms_user_id(self): self.assertEqual(get_edx_lms_user_id(self.user), DEFAULT_EDX_LMS_USER_ID) diff --git a/commerce_coordinator/apps/commercetools/tests/raw_ct_order.json b/commerce_coordinator/apps/commercetools/tests/raw_ct_order.json index 37ec698f..2b9daa66 100644 --- a/commerce_coordinator/apps/commercetools/tests/raw_ct_order.json +++ b/commerce_coordinator/apps/commercetools/tests/raw_ct_order.json @@ -39,6 +39,10 @@ "name": "brand-text", "value": "MichiganX" }, + { + "name": "mode", + "value": "certified" + }, { "name": "date-created", "value": "2019-08-21T00:02:00.000Z" From 7082b2089eef4fdedadee043515c71a48c698fb2 Mon Sep 17 00:00:00 2001 From: Ahtesham Quraish Date: Mon, 11 Nov 2024 12:12:40 +0500 Subject: [PATCH 2/4] fix: address comments Description: Address comments SONIC-732 --- .../apps/commercetools/catalog_info/edx_utils.py | 9 ++++----- .../apps/commercetools/sub_messages/tasks.py | 2 +- .../commercetools/tests/catalog_info/test_edx_utils.py | 2 +- 3 files changed, 6 insertions(+), 7 deletions(-) diff --git a/commerce_coordinator/apps/commercetools/catalog_info/edx_utils.py b/commerce_coordinator/apps/commercetools/catalog_info/edx_utils.py index 50187d81..ef4f74ef 100644 --- a/commerce_coordinator/apps/commercetools/catalog_info/edx_utils.py +++ b/commerce_coordinator/apps/commercetools/catalog_info/edx_utils.py @@ -40,12 +40,11 @@ def is_edx_lms_order(order: CTOrder) -> bool: return len(get_edx_items(order)) >= 1 -def get_course_mode_from_ct_order(order: CTOrder) -> str: +def get_course_mode_from_ct_order(line_item: CTLineItem) -> str: mode = 'verified' - for line_item in order.line_items: - for attribute in line_item.variant.attributes: - if attribute.name == 'mode': - mode = attribute.value + for attribute in line_item.variant.attributes: + if attribute.name == 'mode': + mode = attribute.value return mode diff --git a/commerce_coordinator/apps/commercetools/sub_messages/tasks.py b/commerce_coordinator/apps/commercetools/sub_messages/tasks.py index 77996b65..2ba60a7d 100644 --- a/commerce_coordinator/apps/commercetools/sub_messages/tasks.py +++ b/commerce_coordinator/apps/commercetools/sub_messages/tasks.py @@ -84,7 +84,6 @@ def fulfill_order_placed_message_signal_task( 'order_id': order.id, 'provider_id': None, 'edx_lms_user_id': lms_user_id, - 'course_mode': get_course_mode_from_ct_order(order=order), 'date_placed': order.last_modified_at.strftime(ISO_8601_FORMAT), 'source_system': source_system, } @@ -113,6 +112,7 @@ def fulfill_order_placed_message_signal_task( **default_params, 'course_id': get_edx_product_course_run_key(item), # likely not correct 'line_item_id': item.id, + 'course_mode': get_course_mode_from_ct_order(item), 'item_quantity': item.quantity, 'line_item_state_id': line_item_state_id, 'message_id': message_id diff --git a/commerce_coordinator/apps/commercetools/tests/catalog_info/test_edx_utils.py b/commerce_coordinator/apps/commercetools/tests/catalog_info/test_edx_utils.py index ef1f7a36..531fb84d 100644 --- a/commerce_coordinator/apps/commercetools/tests/catalog_info/test_edx_utils.py +++ b/commerce_coordinator/apps/commercetools/tests/catalog_info/test_edx_utils.py @@ -60,7 +60,7 @@ def test_is_edx_lms_order(self): self.assertTrue(is_edx_lms_order(self.order)) def test_get_course_mode_from_ct_order(self): - self.assertEqual(get_course_mode_from_ct_order(self.order), 'certified') + self.assertEqual(get_course_mode_from_ct_order(get_edx_items(self.order)[0]), 'certified') def test_get_edx_lms_user_id(self): self.assertEqual(get_edx_lms_user_id(self.user), DEFAULT_EDX_LMS_USER_ID) From 48cd27a19db6aed8e38d6efad76ed05f369a4049 Mon Sep 17 00:00:00 2001 From: Ahtesham Quraish Date: Wed, 13 Nov 2024 13:54:49 +0500 Subject: [PATCH 3/4] fix: create util of get_line_item_attribute Description Create util for get_line_item_attribute SONIC --- .../commercetools/catalog_info/edx_utils.py | 20 ++++++++++++++++--- .../apps/commercetools/sub_messages/tasks.py | 20 ++++--------------- 2 files changed, 21 insertions(+), 19 deletions(-) diff --git a/commerce_coordinator/apps/commercetools/catalog_info/edx_utils.py b/commerce_coordinator/apps/commercetools/catalog_info/edx_utils.py index ef4f74ef..ab442c97 100644 --- a/commerce_coordinator/apps/commercetools/catalog_info/edx_utils.py +++ b/commerce_coordinator/apps/commercetools/catalog_info/edx_utils.py @@ -40,11 +40,25 @@ def is_edx_lms_order(order: CTOrder) -> bool: return len(get_edx_items(order)) >= 1 +def get_line_item_attribute(in_line_item, in_attribute_name): # pragma no cover + """Utility to get line item's attribute's value.""" + attribute_value = None + for attribute in in_line_item.variant.attributes: + if attribute.name == in_attribute_name and hasattr(attribute, 'value'): + if isinstance(attribute.value, dict): + attribute_value = attribute.value.get('label', None) + elif isinstance(attribute.value, str): + attribute_value = attribute.value + break + + return attribute_value + + def get_course_mode_from_ct_order(line_item: CTLineItem) -> str: + course_mode = get_line_item_attribute(line_item, 'mode') mode = 'verified' - for attribute in line_item.variant.attributes: - if attribute.name == 'mode': - mode = attribute.value + if course_mode is not None: + mode = course_mode return mode diff --git a/commerce_coordinator/apps/commercetools/sub_messages/tasks.py b/commerce_coordinator/apps/commercetools/sub_messages/tasks.py index 2ba60a7d..d08f8b16 100644 --- a/commerce_coordinator/apps/commercetools/sub_messages/tasks.py +++ b/commerce_coordinator/apps/commercetools/sub_messages/tasks.py @@ -17,6 +17,7 @@ get_edx_order_workflow_state_key, get_edx_payment_intent_id, get_edx_product_course_run_key, + get_line_item_attribute, is_edx_lms_order ) from commerce_coordinator.apps.commercetools.clients import CommercetoolsAPIClient @@ -203,19 +204,6 @@ def fulfill_order_returned_signal_task( ): """Celery task for an order return (and refunded) message.""" - def _get_line_item_attribute(in_line_item, in_attribute_name): # pragma no cover - """Utility to get line item's attribute's value.""" - attribute_value = None - for attribute in in_line_item.variant.attributes: - if attribute.name == in_attribute_name and hasattr(attribute, 'value'): - if isinstance(attribute.value, dict): - attribute_value = attribute.value.get('label', None) - elif isinstance(attribute.value, str): - attribute_value = attribute.value - break - - return attribute_value - def _cents_to_dollars(in_amount): return in_amount.cent_amount / pow( 10, in_amount.fraction_digits @@ -300,10 +288,10 @@ def _prepare_segment_event_properties(in_order, return_line_item_return_id): 'name': line_item.name['en-US'], 'price': _cents_to_dollars(line_item.price.value), 'quantity': line_item.quantity, - 'category': _get_line_item_attribute(line_item, 'primary-subject-area'), + 'category': get_line_item_attribute(line_item, 'primary-subject-area'), 'image_url': line_item.variant.images[0].url if line_item.variant.images else None, - 'brand': _get_line_item_attribute(line_item, 'brand-text'), - 'url': _get_line_item_attribute(line_item, 'url-course'), + 'brand': get_line_item_attribute(line_item, 'brand-text'), + 'url': get_line_item_attribute(line_item, 'url-course'), 'lob': 'edX', # TODO: Decision was made to hardcode this value for phase 1. 'product_type': line_item.product_type.obj.name if hasattr(line_item.product_type.obj, 'name') else None From fa24efc1829c937ef63292888c059514ad15dd8c Mon Sep 17 00:00:00 2001 From: Ahtesham Quraish Date: Thu, 14 Nov 2024 11:41:23 +0500 Subject: [PATCH 4/4] fix: move the get order util to util Description: Move the get order util to util folder SONIC --- .../commercetools/catalog_info/edx_utils.py | 23 ------------------- .../apps/commercetools/catalog_info/utils.py | 20 ++++++++++++++++ .../apps/commercetools/sub_messages/tasks.py | 6 +++-- .../tests/catalog_info/test_edx_utils.py | 4 ---- .../tests/catalog_info/test_utils.py | 23 +++++++++++++++++-- .../commercetools/tests/raw_ct_order.json | 2 +- 6 files changed, 46 insertions(+), 32 deletions(-) diff --git a/commerce_coordinator/apps/commercetools/catalog_info/edx_utils.py b/commerce_coordinator/apps/commercetools/catalog_info/edx_utils.py index ab442c97..78a32931 100644 --- a/commerce_coordinator/apps/commercetools/catalog_info/edx_utils.py +++ b/commerce_coordinator/apps/commercetools/catalog_info/edx_utils.py @@ -40,29 +40,6 @@ def is_edx_lms_order(order: CTOrder) -> bool: return len(get_edx_items(order)) >= 1 -def get_line_item_attribute(in_line_item, in_attribute_name): # pragma no cover - """Utility to get line item's attribute's value.""" - attribute_value = None - for attribute in in_line_item.variant.attributes: - if attribute.name == in_attribute_name and hasattr(attribute, 'value'): - if isinstance(attribute.value, dict): - attribute_value = attribute.value.get('label', None) - elif isinstance(attribute.value, str): - attribute_value = attribute.value - break - - return attribute_value - - -def get_course_mode_from_ct_order(line_item: CTLineItem) -> str: - course_mode = get_line_item_attribute(line_item, 'mode') - mode = 'verified' - if course_mode is not None: - mode = course_mode - - return mode - - def get_edx_lms_user_id(customer: CTCustomer): return customer.custom.fields[EdXFieldNames.LMS_USER_ID] diff --git a/commerce_coordinator/apps/commercetools/catalog_info/utils.py b/commerce_coordinator/apps/commercetools/catalog_info/utils.py index 79018407..8cc4a631 100644 --- a/commerce_coordinator/apps/commercetools/catalog_info/utils.py +++ b/commerce_coordinator/apps/commercetools/catalog_info/utils.py @@ -3,6 +3,7 @@ from commercetools.platform.models import Attribute as CTAttribute from commercetools.platform.models import CentPrecisionMoney as CTCentPrecisionMoney from commercetools.platform.models import HighPrecisionMoney as CTHighPrecisionMoney +from commercetools.platform.models import LineItem as CTLineItem from commercetools.platform.models import LocalizedString as CTLocalizedString from commercetools.platform.models import MoneyType as CTMoneyType from commercetools.platform.models import Price as CTPrice @@ -32,6 +33,25 @@ def ls(string_dict: LSLike) -> CTLocalizedString: return string_dict +def get_line_item_attribute(in_line_item, in_attribute_name): # pragma no cover + """Utility to get line item's attribute's value.""" + attribute_value = None + for attribute in in_line_item.variant.attributes: + if attribute.name == in_attribute_name and hasattr(attribute, 'value'): + if isinstance(attribute.value, dict): + attribute_value = attribute.value.get('label', None) + elif isinstance(attribute.value, str): + attribute_value = attribute.value + break + + return attribute_value + + +def get_course_mode_from_ct_order(line_item: CTLineItem) -> str: + course_mode = get_line_item_attribute(line_item, 'mode') + return course_mode or 'verified' + + def ls_eq(a: LSLike, b: LSLike) -> bool: if a is None or b is None: return False diff --git a/commerce_coordinator/apps/commercetools/sub_messages/tasks.py b/commerce_coordinator/apps/commercetools/sub_messages/tasks.py index d08f8b16..4ccdcddb 100644 --- a/commerce_coordinator/apps/commercetools/sub_messages/tasks.py +++ b/commerce_coordinator/apps/commercetools/sub_messages/tasks.py @@ -9,7 +9,6 @@ from commerce_coordinator.apps.commercetools.catalog_info.constants import TwoUKeys from commerce_coordinator.apps.commercetools.catalog_info.edx_utils import ( - get_course_mode_from_ct_order, get_edx_is_sanctioned, get_edx_items, get_edx_lms_user_id, @@ -17,9 +16,12 @@ get_edx_order_workflow_state_key, get_edx_payment_intent_id, get_edx_product_course_run_key, - get_line_item_attribute, is_edx_lms_order ) +from commerce_coordinator.apps.commercetools.catalog_info.utils import ( + get_course_mode_from_ct_order, + get_line_item_attribute +) from commerce_coordinator.apps.commercetools.clients import CommercetoolsAPIClient from commerce_coordinator.apps.commercetools.constants import EMAIL_NOTIFICATION_CACHE_TTL_SECS from commerce_coordinator.apps.commercetools.filters import OrderRefundRequested diff --git a/commerce_coordinator/apps/commercetools/tests/catalog_info/test_edx_utils.py b/commerce_coordinator/apps/commercetools/tests/catalog_info/test_edx_utils.py index 531fb84d..d74f1229 100644 --- a/commerce_coordinator/apps/commercetools/tests/catalog_info/test_edx_utils.py +++ b/commerce_coordinator/apps/commercetools/tests/catalog_info/test_edx_utils.py @@ -6,7 +6,6 @@ from commercetools.platform.models import Order as CTOrder from commerce_coordinator.apps.commercetools.catalog_info.edx_utils import ( - get_course_mode_from_ct_order, get_edx_items, get_edx_lms_user_id, get_edx_lms_user_name, @@ -59,9 +58,6 @@ def test_get_edx_items(self): def test_is_edx_lms_order(self): self.assertTrue(is_edx_lms_order(self.order)) - def test_get_course_mode_from_ct_order(self): - self.assertEqual(get_course_mode_from_ct_order(get_edx_items(self.order)[0]), 'certified') - def test_get_edx_lms_user_id(self): self.assertEqual(get_edx_lms_user_id(self.user), DEFAULT_EDX_LMS_USER_ID) diff --git a/commerce_coordinator/apps/commercetools/tests/catalog_info/test_utils.py b/commerce_coordinator/apps/commercetools/tests/catalog_info/test_utils.py index e363dafa..62adddb0 100644 --- a/commerce_coordinator/apps/commercetools/tests/catalog_info/test_utils.py +++ b/commerce_coordinator/apps/commercetools/tests/catalog_info/test_utils.py @@ -1,14 +1,19 @@ """ Commcercetools API Utilities """ +from typing import Union from unittest import TestCase import ddt import pytest -from commercetools.platform.models import CentPrecisionMoney, HighPrecisionMoney, Price +from commercetools.platform.models import CentPrecisionMoney, HighPrecisionMoney +from commercetools.platform.models import Order as CTOrder +from commercetools.platform.models import Price from currencies import Currency from commerce_coordinator.apps.commercetools.catalog_info.constants import Languages +from commerce_coordinator.apps.commercetools.catalog_info.edx_utils import get_edx_items from commerce_coordinator.apps.commercetools.catalog_info.utils import ( attribute_dict, + get_course_mode_from_ct_order, ls, ls_eq, price_to_string, @@ -16,7 +21,8 @@ typed_money_to_string, un_ls ) -from commerce_coordinator.apps.core.tests.utils import name_test +from commerce_coordinator.apps.commercetools.tests.conftest import gen_order +from commerce_coordinator.apps.core.tests.utils import name_test, uuid4_str # _ JAPAN_YEN = "JPY" # 0 fractional digits @@ -29,12 +35,25 @@ class LocalizedStringsTests(TestCase): """ Localized String Utility Tests""" + order: Union[CTOrder, None] + + def setUp(self): + self.order = gen_order(uuid4_str()) + super().setUp() + + def tearDown(self): + self.order = None + super().tearDown() + # ls() def test_single_unknown_key_ls_creation(self): string = "test" result = ls({'ZZ': string}) self.assertEqual(result, {'ZZ': string, Languages.ENGLISH: string, Languages.US_ENGLISH: string}) + def test_get_course_mode_from_ct_order(self): + self.assertEqual(get_course_mode_from_ct_order(get_edx_items(self.order)[0]), 'professional') + def test_single_key_ls_creation(self): string = "test-2" result = ls({Languages.ENGLISH: string}) diff --git a/commerce_coordinator/apps/commercetools/tests/raw_ct_order.json b/commerce_coordinator/apps/commercetools/tests/raw_ct_order.json index 2b9daa66..a6ed0b46 100644 --- a/commerce_coordinator/apps/commercetools/tests/raw_ct_order.json +++ b/commerce_coordinator/apps/commercetools/tests/raw_ct_order.json @@ -41,7 +41,7 @@ }, { "name": "mode", - "value": "certified" + "value": "professional" }, { "name": "date-created",