From 6e8c266c4b1eb809543617e9e7c00601ae76cb4e Mon Sep 17 00:00:00 2001 From: Jade Olivier Date: Thu, 20 Jun 2024 14:57:13 +0200 Subject: [PATCH] fix: added check for duplicate refund transactions (#243) * fix: added check for duplicate refund transactions * fix: added stripe refund signal to test file --- .../apps/commercetools/tasks.py | 5 ++++ .../apps/commercetools/tests/conftest.py | 30 ++++++++++++++++--- .../apps/commercetools/tests/test_tasks.py | 24 ++++++++++++++- .../apps/commercetools/tests/test_utils.py | 26 +++++++++++++++- .../apps/commercetools/utils.py | 15 ++++++++++ commerce_coordinator/settings/base.py | 3 ++ commerce_coordinator/settings/test.py | 3 ++ 7 files changed, 100 insertions(+), 6 deletions(-) diff --git a/commerce_coordinator/apps/commercetools/tasks.py b/commerce_coordinator/apps/commercetools/tasks.py index c88c786e..dab53759 100644 --- a/commerce_coordinator/apps/commercetools/tasks.py +++ b/commerce_coordinator/apps/commercetools/tasks.py @@ -10,6 +10,7 @@ from django.conf import settings from .clients import CommercetoolsAPIClient +from .utils import has_full_refund_transaction logger = logging.getLogger(__name__) @@ -58,6 +59,10 @@ def refund_from_stripe_task( client = CommercetoolsAPIClient() try: payment = client.get_payment_by_key(payment_intent_id) + if has_full_refund_transaction(payment): + logger.info(f"Stripe charge.refunded event received, but Payment with ID {payment.id} " + f"already has a full refund. Skipping task to add refund transaction") + return None updated_payment = client.create_return_payment_transaction( payment_id=payment.id, payment_version=payment.version, diff --git a/commerce_coordinator/apps/commercetools/tests/conftest.py b/commerce_coordinator/apps/commercetools/tests/conftest.py index 75c51ad0..07a8f444 100644 --- a/commerce_coordinator/apps/commercetools/tests/conftest.py +++ b/commerce_coordinator/apps/commercetools/tests/conftest.py @@ -168,16 +168,38 @@ def gen_payment(): amount_planned=4900, payment_method_info={}, payment_status=PaymentState.PAID, - transactions=[gen_transaction()], + transactions=[gen_transaction(TransactionType.REFUND, 4900)], interface_interactions=[] ) -def gen_transaction() -> CTTransaction: +def gen_payment_with_multiple_transactions(*args): + """ + Generate a CTPayment object with multiple transaction records + """ + transactions = [] + for i in range(0, len(args), 2): + transaction = gen_transaction(args[i], args[i+1]) + transactions.append(transaction) + + return CTPayment( + id=uuid4_str(), + version=1, + created_at=datetime.now(), + last_modified_at=datetime.now(), + amount_planned=4900, + payment_method_info={}, + payment_status=PaymentState.PAID, + transactions=transactions, + interface_interactions=[] + ) + + +def gen_transaction(transaction_type=None, amount=None) -> CTTransaction: return CTTransaction( id=uuid4_str(), - type=TransactionType.REFUND, - amount=4900, + type=transaction_type, + amount=amount, timestamp=datetime.now(), state=TransactionState.SUCCESS, interaction_id='ch_3P9RWsH4caH7G0X11toRGUJf' diff --git a/commerce_coordinator/apps/commercetools/tests/test_tasks.py b/commerce_coordinator/apps/commercetools/tests/test_tasks.py index 507426a4..3b2e6863 100644 --- a/commerce_coordinator/apps/commercetools/tests/test_tasks.py +++ b/commerce_coordinator/apps/commercetools/tests/test_tasks.py @@ -7,13 +7,14 @@ import stripe from commercetools import CommercetoolsError +from commercetools.platform.models import TransactionType from django.test import TestCase from commerce_coordinator.apps.commercetools.tasks import ( refund_from_stripe_task, update_line_item_state_on_fulfillment_completion ) -from commerce_coordinator.apps.commercetools.tests.conftest import gen_payment +from commerce_coordinator.apps.commercetools.tests.conftest import gen_payment, gen_payment_with_multiple_transactions from commerce_coordinator.apps.commercetools.tests.constants import ( EXAMPLE_RETURNED_ORDER_STRIPE_CLIENT_PAYLOAD, EXAMPLE_RETURNED_ORDER_STRIPE_SIGNAL_PAYLOAD, @@ -121,6 +122,27 @@ def test_correct_arguments_passed(self, mock_client): stripe_refund=mock_stripe_refund ) + def test_full_refund_already_exists(self, mock_client): + ''' + Check if the payment already has a full refund, the task logs the + appropriate message and skips creating a refund transaction. + ''' + mock_payment = gen_payment_with_multiple_transactions( + TransactionType.CHARGE, 4900, + TransactionType.REFUND, 4900 + ) + mock_payment.id = 'f988e0c5-ea44-4111-a7f2-39ecf6af9840' + + mock_client.return_value.get_payment_by_key.return_value = mock_payment + + with patch('commerce_coordinator.apps.commercetools.tasks.logger') as mock_logger: + result = refund_from_stripe_task(*self.unpack_for_uut(EXAMPLE_RETURNED_ORDER_STRIPE_SIGNAL_PAYLOAD)) + self.assertIsNone(result) + mock_logger.info.assert_called_once_with( + f"Stripe charge.refunded event received, but Payment with ID {mock_payment.id} " + f"already has a full refund. Skipping task to add refund transaction" + ) + @patch('commerce_coordinator.apps.commercetools.tasks.logger') def test_exception_handling(self, mock_logger, mock_client): ''' diff --git a/commerce_coordinator/apps/commercetools/tests/test_utils.py b/commerce_coordinator/apps/commercetools/tests/test_utils.py index f07552e9..fbf9b635 100644 --- a/commerce_coordinator/apps/commercetools/tests/test_utils.py +++ b/commerce_coordinator/apps/commercetools/tests/test_utils.py @@ -15,13 +15,19 @@ from mock import Mock, patch from commerce_coordinator.apps.commercetools.catalog_info.edx_utils import get_edx_lms_user_name -from commerce_coordinator.apps.commercetools.tests.conftest import gen_example_customer, gen_order, gen_payment +from commerce_coordinator.apps.commercetools.tests.conftest import ( + gen_example_customer, + gen_order, + gen_payment, + gen_payment_with_multiple_transactions +) from commerce_coordinator.apps.commercetools.tests.constants import EXAMPLE_FULFILLMENT_SIGNAL_PAYLOAD from commerce_coordinator.apps.commercetools.utils import ( create_zendesk_ticket, extract_ct_order_information_for_braze_canvas, extract_ct_product_information_for_braze_canvas, get_braze_client, + has_full_refund_transaction, has_refund_transaction, send_order_confirmation_email, send_refund_notification, @@ -205,6 +211,24 @@ def test_has_refund_transaction_without_refund(self): self.assertFalse(has_refund_transaction(payment)) +class TestHasFullRefundTransaction(unittest.TestCase): + """ + Tests for Has Full Refund Transaction Utils function + """ + + def test_has_full_refund_transaction_with_full_refund(self): + payment = gen_payment_with_multiple_transactions(TransactionType.CHARGE, 4900, TransactionType.REFUND, 4900) + self.assertTrue(has_full_refund_transaction(payment)) + + def test_has_partial_refund_transaction(self): + payment = gen_payment_with_multiple_transactions(TransactionType.CHARGE, 4900, TransactionType.REFUND, 2500) + self.assertFalse(has_full_refund_transaction(payment)) + + def test_has_no_refund_transaction(self): + payment = gen_payment_with_multiple_transactions(TransactionType.CHARGE, 4900) + self.assertFalse(has_full_refund_transaction(payment)) + + class TestTranslateStripeRefundStatus(unittest.TestCase): """ Tests for Translating Stripes Refund Status Utils class diff --git a/commerce_coordinator/apps/commercetools/utils.py b/commerce_coordinator/apps/commercetools/utils.py index cd92dc2b..5d387be6 100644 --- a/commerce_coordinator/apps/commercetools/utils.py +++ b/commerce_coordinator/apps/commercetools/utils.py @@ -139,6 +139,21 @@ def has_refund_transaction(payment: Payment): return False +def has_full_refund_transaction(payment: Payment): + """ + Utility to determine is CT payment has an existing 'refund' transaction for the full + charge amount + """ + # get charge transaction and get amount then check against refund. + charge_amount = 0 + for transaction in payment.transactions: + if transaction.type == TransactionType.CHARGE: + charge_amount = transaction.amount + if transaction.type == TransactionType.REFUND and transaction.amount == charge_amount: # pragma no cover + return True + return False + + def translate_stripe_refund_status_to_transaction_status(stripe_refund_status: str): """ Utility to translate stripe's refund object's status attribute to a valid CT transaction state diff --git a/commerce_coordinator/settings/base.py b/commerce_coordinator/settings/base.py index e9ac5770..a8e842f6 100644 --- a/commerce_coordinator/settings/base.py +++ b/commerce_coordinator/settings/base.py @@ -327,6 +327,9 @@ def root(*path_fragments): 'commerce_coordinator.apps.commercetools.sub_messages.signals_dispatch.fulfill_order_returned_signal': [ 'commerce_coordinator.apps.commercetools.sub_messages.signals_delayed.fulfill_order_returned_signal', ], + 'commerce_coordinator.apps.stripe.signals.payment_refunded_signal': [ + 'commerce_coordinator.apps.commercetools.signals.refund_from_stripe', + ], } # Default timeouts for requests diff --git a/commerce_coordinator/settings/test.py b/commerce_coordinator/settings/test.py index 99f93110..36178042 100644 --- a/commerce_coordinator/settings/test.py +++ b/commerce_coordinator/settings/test.py @@ -84,6 +84,9 @@ 'commerce_coordinator.apps.commercetools.sub_messages.signals_dispatch.fulfill_order_returned_signal': [ 'commerce_coordinator.apps.commercetools.sub_messages.signals_delayed.fulfill_order_returned_signal', ], + 'commerce_coordinator.apps.stripe.signals.payment_refunded_signal': [ + 'commerce_coordinator.apps.commercetools.signals.refund_from_stripe', + ], } COMMERCETOOLS_CONFIG = {