From bc2cfb9982eb865a86ddc2d1f6c292d125d611bb Mon Sep 17 00:00:00 2001 From: Syed Sajjad Hussain Shah Date: Fri, 20 Dec 2024 13:16:21 +0500 Subject: [PATCH 1/2] feat: update line items state in single api call --- .../apps/commercetools/clients.py | 59 ++++++++++++++ .../apps/commercetools/sub_messages/tasks.py | 23 +++--- .../apps/commercetools/tests/constants.py | 13 +++ .../tests/sub_messages/test_tasks.py | 1 + .../apps/commercetools/tests/test_clients.py | 80 +++++++++++++++++++ 5 files changed, 166 insertions(+), 10 deletions(-) diff --git a/commerce_coordinator/apps/commercetools/clients.py b/commerce_coordinator/apps/commercetools/clients.py index e6dc617da..e63df58d5 100644 --- a/commerce_coordinator/apps/commercetools/clients.py +++ b/commerce_coordinator/apps/commercetools/clients.py @@ -16,6 +16,7 @@ from commercetools.platform.models import CustomerSetCustomTypeAction as CTCustomerSetCustomTypeAction from commercetools.platform.models import CustomerSetFirstNameAction, CustomerSetLastNameAction from commercetools.platform.models import FieldContainer as CTFieldContainer +from commercetools.platform.models import LineItem as CTLineItem from commercetools.platform.models import Money as CTMoney from commercetools.platform.models import Order as CTOrder from commercetools.platform.models import ( @@ -619,6 +620,64 @@ def update_line_item_transition_state_on_fulfillment( handle_commercetools_error(err, f"Unable to update LineItemState of order {order_id}", True) return None + def update_all_line_items_transition_state_on_fulfillment( + self, + order_id: str, + order_version: int, + line_items: List[CTLineItem], + from_state_id: str, + new_state_key: str, + ) -> CTOrder: + """ + Update Commercetools order line item state for all items in one call. + Args: + order_id (str): Order ID (UUID) + order_version (int): Current version of order + line_items (List[object]): List of line item objects + from_state_id (str): ID of LineItemState to transition from + new_state_key (str): Key of LineItemState to transition to + Returns (CTOrder): Updated order object or + Returns (CTOrder): Current un-updated order + Raises Exception: Error if update was unsuccessful. + """ + + from_state_key = self.get_state_by_id(from_state_id).key + + logger.info( + f"[CommercetoolsAPIClient] - Transitioning all line item states for order with ID {order_id} " + f"from {from_state_key} to {new_state_key}" + ) + + try: + if new_state_key != from_state_key: + actions = [ + OrderTransitionLineItemStateAction( + line_item_id=item.id, + quantity=item.quantity, + from_state=StateResourceIdentifier(key=from_state_key), + to_state=StateResourceIdentifier(key=new_state_key), + ) + for item in line_items + ] + + updated_fulfillment_line_item_order = self.base_client.orders.update_by_id( + id=order_id, + version=order_version, + actions=actions, + ) + + return updated_fulfillment_line_item_order + else: + logger.info( + f"All line items already have the correct state {new_state_key}. " + "Not attempting to transition LineItemState" + ) + return self.get_order_by_id(order_id) + except CommercetoolsError as err: + # Logs & ignores version conflict errors due to duplicate Commercetools messages + handle_commercetools_error(err, f"Unable to update all LineItemStates of order {order_id}", True) + return None + def retire_customer_anonymize_fields( self, customer_id: str, diff --git a/commerce_coordinator/apps/commercetools/sub_messages/tasks.py b/commerce_coordinator/apps/commercetools/sub_messages/tasks.py index a1d5590a9..bf19aed0a 100644 --- a/commerce_coordinator/apps/commercetools/sub_messages/tasks.py +++ b/commerce_coordinator/apps/commercetools/sub_messages/tasks.py @@ -98,19 +98,22 @@ def fulfill_order_placed_message_signal_task( canvas_entry_properties = {"products": []} canvas_entry_properties.update(extract_ct_order_information_for_braze_canvas(customer, order)) + logger.info( + f"[CT-{tag}] Transitioning all line items for order {order.id} to {TwoUKeys.PROCESSING_FULFILMENT_STATE}" + ) + updated_order = client.update_all_line_items_transition_state_on_fulfillment( + order_id=order.id, + order_version=order.version, + line_items=get_edx_items(order), + from_state_id=line_item_state_id, + new_state_key=TwoUKeys.PROCESSING_FULFILMENT_STATE + ) + if not updated_order: + return True + for item in get_edx_items(order): logger.debug(f'[CT-{tag}] processing edX order {order_id}, line item {item.variant.sku}, ' f'message id: {message_id}') - updated_order = client.update_line_item_transition_state_on_fulfillment( - order.id, - order.version, - item.id, - item.quantity, - line_item_state_id, - TwoUKeys.PROCESSING_FULFILMENT_STATE - ) - if not updated_order: - return True # from here we will always be transitioning from a 'Fulfillment Processing' state line_item_state_id = client.get_state_by_key(TwoUKeys.PROCESSING_FULFILMENT_STATE).id diff --git a/commerce_coordinator/apps/commercetools/tests/constants.py b/commerce_coordinator/apps/commercetools/tests/constants.py index dfe63c061..6b57ba19f 100644 --- a/commerce_coordinator/apps/commercetools/tests/constants.py +++ b/commerce_coordinator/apps/commercetools/tests/constants.py @@ -81,6 +81,19 @@ 'to_state_key': TwoUKeys.SUCCESS_FULFILMENT_STATE } +EXAMPLE_UPDATE_ALL_LINE_ITEMS_SIGNAL_PAYLOAD = { + 'order_id': '61ec1afa-1b0e-4234-ae28-f997728054fa', + 'order_version': 2, + 'line_items': [ + { + 'line_item_id': '822d77c4-00a6-4fb9-909b-094ef0b8c4b9', + 'item_quantity': 1, + } + ], + 'from_state_id': '8f2e888e-9777-4557-9a7f-c649153770c2', + 'to_state_key': TwoUKeys.SUCCESS_FULFILMENT_STATE +} + EXAMPLE_RETURNED_ORDER_STRIPE_SIGNAL_PAYLOAD = { 'payment_intent_id': 'pi_3PNWMsH4caH7G0X109NekCG5', 'stripe_refund': { diff --git a/commerce_coordinator/apps/commercetools/tests/sub_messages/test_tasks.py b/commerce_coordinator/apps/commercetools/tests/sub_messages/test_tasks.py index d8e3163cc..a0594b4a9 100644 --- a/commerce_coordinator/apps/commercetools/tests/sub_messages/test_tasks.py +++ b/commerce_coordinator/apps/commercetools/tests/sub_messages/test_tasks.py @@ -87,6 +87,7 @@ def __init__(self, *args, **kwargs): self.get_state_by_key = self.state_by_key_mock self.get_payment_by_key = self.payment_mock self.update_line_item_transition_state_on_fulfillment = self.updated_line_item_mock + self.update_all_line_items_transition_state_on_fulfillment = self.updated_line_item_mock self.create_return_for_order = self.create_return_item_mock self.create_return_payment_transaction = self.payment_mock self.update_return_payment_state_after_successful_refund = self.order_mock diff --git a/commerce_coordinator/apps/commercetools/tests/test_clients.py b/commerce_coordinator/apps/commercetools/tests/test_clients.py index a3b6cfdea..c3983e529 100644 --- a/commerce_coordinator/apps/commercetools/tests/test_clients.py +++ b/commerce_coordinator/apps/commercetools/tests/test_clients.py @@ -691,6 +691,86 @@ def test_update_line_item_state_exception(self, mock_state_by_id): log_mock.assert_called_with(expected_message) + @patch('commerce_coordinator.apps.commercetools.clients.CommercetoolsAPIClient.get_state_by_id') + def test_successful_order_all_line_items_state_update(self, mock_state_by_id): + base_url = self.client_set.get_base_url_from_client() + + mock_order = gen_order("mock_order_id") + mock_order.version = "2" + mock_line_item_state = gen_line_item_state() + mock_line_item_state.key = TwoUKeys.PROCESSING_FULFILMENT_STATE + mock_order.line_items[0].state[0].state = mock_line_item_state + + mock_state_by_id().return_value = mock_line_item_state + + mock_response_order = gen_order("mock_order_id") + mock_response_order.version = 3 + mock_response_line_item_state = gen_line_item_state() + mock_response_line_item_state.id = "mock_success_id" + mock_response_line_item_state.key = TwoUKeys.SUCCESS_FULFILMENT_STATE + mock_response_order.line_items[0].state[0].state = mock_response_line_item_state + + with requests_mock.Mocker(real_http=True, case_sensitive=False) as mocker: + mocker.post( + f"{base_url}orders/{mock_response_order.id}", + json=mock_response_order.serialize(), + status_code=200 + ) + + result = self.client_set.client.update_all_line_items_transition_state_on_fulfillment( + mock_order.id, + mock_order.version, + mock_order.line_items, + TwoUKeys.PENDING_FULFILMENT_STATE, + TwoUKeys.SUCCESS_FULFILMENT_STATE + ) + + self.assertEqual(result.line_items[0].state[0].state.id, mock_response_line_item_state.id) + + @patch('commerce_coordinator.apps.commercetools.clients.CommercetoolsAPIClient.get_state_by_id') + def test_update_all_line_items_state_exception(self, mock_state_by_id): + mock_order = gen_order("mock_order_id") + mock_order.version = "1" + base_url = self.client_set.get_base_url_from_client() + mock_state_by_id().return_value = gen_line_item_state() + mock_error_response: CommercetoolsError = { + "message": "Could not create return for order mock_order_id", + "errors": [ + { + "code": "ConcurrentModification", + "message": "Object [mock_order_id] has a " + "different version than expected. Expected: 2 - Actual: 1." + }, + ], + "response": {}, + "correlation_id": "None" + } + + with requests_mock.Mocker(real_http=True, case_sensitive=False) as mocker: + mocker.post( + f"{base_url}orders/mock_order_id", + json=mock_error_response, + status_code=409 + ) + + with patch('commerce_coordinator.apps.commercetools.clients.logging.Logger.info') as log_mock: + self.client_set.client.update_all_line_items_transition_state_on_fulfillment( + mock_order.id, + mock_order.version, + mock_order.line_items, + TwoUKeys.PENDING_FULFILMENT_STATE, + TwoUKeys.SUCCESS_FULFILMENT_STATE + ) + + expected_message = ( + f"[CommercetoolsError] Unable to update all LineItemStates " + f"of order mock_order_id " + f"- Correlation ID: {mock_error_response['correlation_id']}, " + f"Details: {mock_error_response['errors']}" + ) + + log_mock.assert_called_with(expected_message) + @patch('commerce_coordinator.apps.commercetools.clients.CommercetoolsAPIClient.get_state_by_id') @patch('commerce_coordinator.apps.commercetools.clients.CommercetoolsAPIClient.get_order_by_id') def test_order_line_item_in_correct_state(self, mock_order_by_id, mock_state_by_id): From 7801a4b37407c9236f19078ca79e0a01fa9d25d1 Mon Sep 17 00:00:00 2001 From: Syed Sajjad Hussain Shah Date: Wed, 1 Jan 2025 12:38:48 +0500 Subject: [PATCH 2/2] fix: improve logging --- .../apps/commercetools/clients.py | 19 ++++++++++++------- .../apps/commercetools/sub_messages/tasks.py | 2 +- .../tests/sub_messages/test_tasks.py | 2 +- .../apps/commercetools/tests/test_clients.py | 8 ++++---- 4 files changed, 18 insertions(+), 13 deletions(-) diff --git a/commerce_coordinator/apps/commercetools/clients.py b/commerce_coordinator/apps/commercetools/clients.py index e63df58d5..a76a9a29f 100644 --- a/commerce_coordinator/apps/commercetools/clients.py +++ b/commerce_coordinator/apps/commercetools/clients.py @@ -620,7 +620,7 @@ def update_line_item_transition_state_on_fulfillment( handle_commercetools_error(err, f"Unable to update LineItemState of order {order_id}", True) return None - def update_all_line_items_transition_state_on_fulfillment( + def update_line_items_transition_state( self, order_id: str, order_version: int, @@ -644,8 +644,10 @@ def update_all_line_items_transition_state_on_fulfillment( from_state_key = self.get_state_by_id(from_state_id).key logger.info( - f"[CommercetoolsAPIClient] - Transitioning all line item states for order with ID {order_id} " - f"from {from_state_key} to {new_state_key}" + f"[CommercetoolsAPIClient] - Transitioning line item states for order ID '{order_id}'. " + f"From State: '{from_state_key}' " + f"To State: '{new_state_key}' " + f"Line Item IDs: {', '.join(item.id for item in line_items)}" ) try: @@ -660,13 +662,11 @@ def update_all_line_items_transition_state_on_fulfillment( for item in line_items ] - updated_fulfillment_line_item_order = self.base_client.orders.update_by_id( + return self.base_client.orders.update_by_id( id=order_id, version=order_version, actions=actions, ) - - return updated_fulfillment_line_item_order else: logger.info( f"All line items already have the correct state {new_state_key}. " @@ -675,7 +675,12 @@ def update_all_line_items_transition_state_on_fulfillment( return self.get_order_by_id(order_id) except CommercetoolsError as err: # Logs & ignores version conflict errors due to duplicate Commercetools messages - handle_commercetools_error(err, f"Unable to update all LineItemStates of order {order_id}", True) + handle_commercetools_error( + err, + f"Failed to update LineItemStates for order ID '{order_id}'. " + f"Line Item IDs: {', '.join(item.id for item in line_items)}", + True + ) return None def retire_customer_anonymize_fields( diff --git a/commerce_coordinator/apps/commercetools/sub_messages/tasks.py b/commerce_coordinator/apps/commercetools/sub_messages/tasks.py index bf19aed0a..41a821f6e 100644 --- a/commerce_coordinator/apps/commercetools/sub_messages/tasks.py +++ b/commerce_coordinator/apps/commercetools/sub_messages/tasks.py @@ -101,7 +101,7 @@ def fulfill_order_placed_message_signal_task( logger.info( f"[CT-{tag}] Transitioning all line items for order {order.id} to {TwoUKeys.PROCESSING_FULFILMENT_STATE}" ) - updated_order = client.update_all_line_items_transition_state_on_fulfillment( + updated_order = client.update_line_items_transition_state( order_id=order.id, order_version=order.version, line_items=get_edx_items(order), diff --git a/commerce_coordinator/apps/commercetools/tests/sub_messages/test_tasks.py b/commerce_coordinator/apps/commercetools/tests/sub_messages/test_tasks.py index a0594b4a9..4ca0ebafe 100644 --- a/commerce_coordinator/apps/commercetools/tests/sub_messages/test_tasks.py +++ b/commerce_coordinator/apps/commercetools/tests/sub_messages/test_tasks.py @@ -87,7 +87,7 @@ def __init__(self, *args, **kwargs): self.get_state_by_key = self.state_by_key_mock self.get_payment_by_key = self.payment_mock self.update_line_item_transition_state_on_fulfillment = self.updated_line_item_mock - self.update_all_line_items_transition_state_on_fulfillment = self.updated_line_item_mock + self.update_line_items_transition_state = self.updated_line_item_mock self.create_return_for_order = self.create_return_item_mock self.create_return_payment_transaction = self.payment_mock self.update_return_payment_state_after_successful_refund = self.order_mock diff --git a/commerce_coordinator/apps/commercetools/tests/test_clients.py b/commerce_coordinator/apps/commercetools/tests/test_clients.py index c3983e529..300f7eedf 100644 --- a/commerce_coordinator/apps/commercetools/tests/test_clients.py +++ b/commerce_coordinator/apps/commercetools/tests/test_clients.py @@ -717,7 +717,7 @@ def test_successful_order_all_line_items_state_update(self, mock_state_by_id): status_code=200 ) - result = self.client_set.client.update_all_line_items_transition_state_on_fulfillment( + result = self.client_set.client.update_line_items_transition_state( mock_order.id, mock_order.version, mock_order.line_items, @@ -754,7 +754,7 @@ def test_update_all_line_items_state_exception(self, mock_state_by_id): ) with patch('commerce_coordinator.apps.commercetools.clients.logging.Logger.info') as log_mock: - self.client_set.client.update_all_line_items_transition_state_on_fulfillment( + self.client_set.client.update_line_items_transition_state( mock_order.id, mock_order.version, mock_order.line_items, @@ -763,8 +763,8 @@ def test_update_all_line_items_state_exception(self, mock_state_by_id): ) expected_message = ( - f"[CommercetoolsError] Unable to update all LineItemStates " - f"of order mock_order_id " + f"[CommercetoolsError] Failed to update LineItemStates " + f"for order ID 'mock_order_id'. Line Item IDs: {mock_order.line_items[0].id} " f"- Correlation ID: {mock_error_response['correlation_id']}, " f"Details: {mock_error_response['errors']}" )