From 337e63e0da95568da516074c589d9e3dd663ae05 Mon Sep 17 00:00:00 2001 From: Jade Olivier Date: Thu, 18 Jul 2024 15:10:09 +0200 Subject: [PATCH 1/2] feat: added missing tests --- .../apps/commercetools/clients.py | 12 +-- .../apps/commercetools/tests/conftest.py | 4 +- .../apps/commercetools/tests/test_clients.py | 84 +++++++++++++++++++ commerce_coordinator/apps/lms/tasks.py | 2 +- .../apps/lms/tests/test_tasks.py | 34 ++++++++ 5 files changed, 128 insertions(+), 8 deletions(-) diff --git a/commerce_coordinator/apps/commercetools/clients.py b/commerce_coordinator/apps/commercetools/clients.py index 1e768155..877871b9 100644 --- a/commerce_coordinator/apps/commercetools/clients.py +++ b/commerce_coordinator/apps/commercetools/clients.py @@ -200,7 +200,7 @@ def get_customer_by_lms_user_id(self, lms_user_id: int) -> Optional[CTCustomer]: return results.results[0] def get_order_by_id(self, order_id: str, expand: ExpandList = DEFAULT_ORDER_EXPANSION) \ - -> CTOrder: # pragma no cover + -> CTOrder: """ Fetch an order by the Order ID (UUID) @@ -214,7 +214,7 @@ def get_order_by_id(self, order_id: str, expand: ExpandList = DEFAULT_ORDER_EXPA return self.base_client.orders.get_by_id(order_id, expand=list(expand)) def get_order_by_number(self, order_number: str, expand: ExpandList = DEFAULT_ORDER_EXPANSION) \ - -> CTOrder: # pragma no cover + -> CTOrder: """ Fetch an order by the Order Number (Human readable order number) @@ -279,19 +279,19 @@ def get_orders_for_customer(self, edx_lms_user_id: int, offset=0, return orders, customer - def get_customer_by_id(self, customer_id: str) -> CTCustomer: # pragma no cover + def get_customer_by_id(self, customer_id: str) -> CTCustomer: logger.info(f"[CommercetoolsAPIClient] - Attempting to find customer with ID {customer_id}") return self.base_client.customers.get_by_id(customer_id) - def get_state_by_id(self, state_id: str) -> CTLineItemState: # pragma no cover + def get_state_by_id(self, state_id: str) -> CTLineItemState: logger.info(f"[CommercetoolsAPIClient] - Attempting to find state with id {state_id}") return self.base_client.states.get_by_id(state_id) - def get_state_by_key(self, state_key: str) -> CTLineItemState: # pragma no cover + def get_state_by_key(self, state_key: str) -> CTLineItemState: logger.info(f"[CommercetoolsAPIClient] - Attempting to find state with key {state_key}") return self.base_client.states.get_by_key(state_key) - def get_payment_by_key(self, payment_key: str) -> CTPayment: # pragma no cover + def get_payment_by_key(self, payment_key: str) -> CTPayment: logger.info(f"[CommercetoolsAPIClient] - Attempting to find payment with key {payment_key}") return self.base_client.payments.get_by_key(payment_key) diff --git a/commerce_coordinator/apps/commercetools/tests/conftest.py b/commerce_coordinator/apps/commercetools/tests/conftest.py index 07a8f444..45146722 100644 --- a/commerce_coordinator/apps/commercetools/tests/conftest.py +++ b/commerce_coordinator/apps/commercetools/tests/conftest.py @@ -24,6 +24,7 @@ from commercetools.platform.models import TransactionState, TransactionType from commercetools.platform.models import TypeReference as CTTypeReference from commercetools.platform.models.state import State as CTLineItemState +from commercetools.platform.models.state import StateTypeEnum as CTStateType from commercetools.testing import BackendRepository from commerce_coordinator.apps.commercetools.catalog_info.constants import EdXFieldNames @@ -165,6 +166,7 @@ def gen_payment(): version=1, created_at=datetime.now(), last_modified_at=datetime.now(), + key="pi_4MtwBwLkdIwGlenn28a3tqPa", amount_planned=4900, payment_method_info={}, payment_status=PaymentState.PAID, @@ -321,7 +323,7 @@ def gen_line_item_state() -> CTLineItemState: created_at=datetime.now(), last_modified_at=datetime.now(), key='2u-fulfillment-pending-state', - type='LineItemState', + type=CTStateType.LINE_ITEM_STATE, initial=False, built_in=False, ) diff --git a/commerce_coordinator/apps/commercetools/tests/test_clients.py b/commerce_coordinator/apps/commercetools/tests/test_clients.py index e30059d3..c7343b4c 100644 --- a/commerce_coordinator/apps/commercetools/tests/test_clients.py +++ b/commerce_coordinator/apps/commercetools/tests/test_clients.py @@ -183,6 +183,90 @@ def test_get_customer_by_lms_user_id_should_fail_on_more_than_1(self): with pytest.raises(ValueError) as _: _ = self.client_set.client.get_customer_by_lms_user_id(id_num) + def test_get_order_by_id(self): + base_url = self.client_set.get_base_url_from_client() + order_id = "mock_order_id" + expected_order = gen_order(order_id) + + with requests_mock.Mocker(real_http=True, case_sensitive=False) as mocker: + mocker.get( + f"{base_url}orders/{order_id}", + json=expected_order.serialize() + ) + + result = self.client_set.client.get_order_by_id(order_id) + self.assertEqual(result, expected_order) + + def test_get_order_by_number(self): + base_url = self.client_set.get_base_url_from_client() + expected_order = gen_order("mock_order_id") + order_number = expected_order.order_number + + with requests_mock.Mocker(real_http=True, case_sensitive=False) as mocker: + mocker.get( + f"{base_url}orders/order-number={order_number}", + json=expected_order.serialize() + ) + + result = self.client_set.client.get_order_by_number(order_number) + self.assertEqual(result, expected_order) + + def test_get_customer_by_id(self): + base_url = self.client_set.get_base_url_from_client() + expected_customer = gen_example_customer() + customer_id = expected_customer.id + + with requests_mock.Mocker(real_http=True, case_sensitive=False) as mocker: + mocker.get( + f"{base_url}customers/{customer_id}", + json=expected_customer.serialize() + ) + + result = self.client_set.client.get_customer_by_id(customer_id) + self.assertEqual(result, expected_customer) + + def test_get_state_by_key(self): + base_url = self.client_set.get_base_url_from_client() + state_key = '2u-fulfillment-pending-state' + expected_state = gen_line_item_state() + + with requests_mock.Mocker(real_http=True, case_sensitive=False) as mocker: + mocker.get( + f"{base_url}states/key={state_key}", + json=expected_state.serialize() + ) + + result = self.client_set.client.get_state_by_key(state_key) + self.assertEqual(result, expected_state) + + def test_get_state_by_id(self): + base_url = self.client_set.get_base_url_from_client() + expected_state = gen_line_item_state() + state_id = expected_state.id + + with requests_mock.Mocker(real_http=True, case_sensitive=False) as mocker: + mocker.get( + f"{base_url}states/{state_id}", + json=expected_state.serialize() + ) + + result = self.client_set.client.get_state_by_id(state_id) + self.assertEqual(result, expected_state) + + def test_get_payment_by_key(self): + base_url = self.client_set.get_base_url_from_client() + payment_key = "pi_4MtwBwLkdIwGlenn28a3tqPa" + expected_payment = gen_payment() + + with requests_mock.Mocker(real_http=True, case_sensitive=False) as mocker: + mocker.get( + f"{base_url}payments/key={payment_key}", + json=expected_payment.serialize() + ) + + result = self.client_set.client.get_payment_by_key(payment_key) + self.assertEqual(result.id, expected_payment.id) + def test_order_history_throws_if_user_not_found(self): with pytest.raises(ValueError) as _: _ = self.client_set.client.get_orders_for_customer(995) diff --git a/commerce_coordinator/apps/lms/tasks.py b/commerce_coordinator/apps/lms/tasks.py index 8a5ee2e7..93b403e5 100644 --- a/commerce_coordinator/apps/lms/tasks.py +++ b/commerce_coordinator/apps/lms/tasks.py @@ -87,7 +87,7 @@ def fulfill_order_placed_send_enroll_in_course_task( }) # Updating the order version and stateID after the transition to 'Fulfillment Failure' - if self.request.retries > 0: # pragma no cover + if self.request.retries > 0: client = CommercetoolsAPIClient() # A retry means the current line item state on the order would be a failure state line_item_state_id = client.get_state_by_key(TwoUKeys.FAILURE_FULFILMENT_STATE).id diff --git a/commerce_coordinator/apps/lms/tests/test_tasks.py b/commerce_coordinator/apps/lms/tests/test_tasks.py index a5b2fa84..500f9e42 100644 --- a/commerce_coordinator/apps/lms/tests/test_tasks.py +++ b/commerce_coordinator/apps/lms/tests/test_tasks.py @@ -6,7 +6,10 @@ from unittest.mock import patch, sentinel from django.test import TestCase +from requests import RequestException +from commerce_coordinator.apps.commercetools.catalog_info.constants import TwoUKeys +from commerce_coordinator.apps.commercetools.tests.conftest import gen_line_item_state, gen_order from commerce_coordinator.apps.core.models import User from commerce_coordinator.apps.lms.tasks import fulfill_order_placed_send_enroll_in_course_task from commerce_coordinator.apps.lms.tests.constants import ( @@ -101,3 +104,34 @@ def test_passes_through_client_return(self, mock_client): res = uut(*self.unpack_for_uut(EXAMPLE_FULFILLMENT_SIGNAL_PAYLOAD)) # pylint: disable=no-value-for-parameter logger.info('result: %s', res) self.assertEqual(res, sentinel.mock_client_return_value) + + @patch('commerce_coordinator.apps.lms.tasks.CommercetoolsAPIClient.get_state_by_key') + @patch('commerce_coordinator.apps.lms.tasks.CommercetoolsAPIClient.get_order_by_id') + def test_retry_logic(self, mock_ct_get_order, mock_ct_get_state, mock_client): + """ + Check if the retry logic updates the line item state ID and order version correctly. + """ + mock_ct_get_state.return_value = gen_line_item_state() + mock_ct_get_order.return_value = gen_order('mock_order_id') + + retry_payload = EXAMPLE_FULFILLMENT_SIGNAL_PAYLOAD.copy() + retry_payload['line_item_state_id'] = 'initial-state-id' + retry_payload['order_version'] = 1 + + with mock_client: + mock_client().enroll_user_in_course.side_effect = RequestException() + + try: + uut.apply( + args=self.unpack_for_uut(retry_payload), + throw=False + ) + except RequestException: + pass + + expected_state_payload = EXAMPLE_FULFILLMENT_SIGNAL_PAYLOAD.copy() + expected_state_payload['line_item_state_id'] = '2u-fulfillment-failure-state' + expected_state_payload['order_version'] = 2 + + mock_ct_get_state.assert_called_with(TwoUKeys.FAILURE_FULFILMENT_STATE) + mock_ct_get_order.assert_called_with(EXAMPLE_FULFILLMENT_SIGNAL_PAYLOAD.get('order_id')) From 4e81a8424cbd0db2085fd646a95e564282ebdfdb Mon Sep 17 00:00:00 2001 From: Jade Olivier Date: Thu, 18 Jul 2024 15:21:12 +0200 Subject: [PATCH 2/2] fix: minor coverage tweak --- commerce_coordinator/apps/lms/tests/test_tasks.py | 15 +++++---------- 1 file changed, 5 insertions(+), 10 deletions(-) diff --git a/commerce_coordinator/apps/lms/tests/test_tasks.py b/commerce_coordinator/apps/lms/tests/test_tasks.py index 500f9e42..595d68a2 100644 --- a/commerce_coordinator/apps/lms/tests/test_tasks.py +++ b/commerce_coordinator/apps/lms/tests/test_tasks.py @@ -118,16 +118,11 @@ def test_retry_logic(self, mock_ct_get_order, mock_ct_get_state, mock_client): retry_payload['line_item_state_id'] = 'initial-state-id' retry_payload['order_version'] = 1 - with mock_client: - mock_client().enroll_user_in_course.side_effect = RequestException() - - try: - uut.apply( - args=self.unpack_for_uut(retry_payload), - throw=False - ) - except RequestException: - pass + mock_client().enroll_user_in_course.side_effect = RequestException() + uut.apply( + args=self.unpack_for_uut(retry_payload), + throw=False + ) expected_state_payload = EXAMPLE_FULFILLMENT_SIGNAL_PAYLOAD.copy() expected_state_payload['line_item_state_id'] = '2u-fulfillment-failure-state'