diff --git a/commerce_coordinator/apps/commercetools/clients.py b/commerce_coordinator/apps/commercetools/clients.py index 02fc8029d..ff756f663 100644 --- a/commerce_coordinator/apps/commercetools/clients.py +++ b/commerce_coordinator/apps/commercetools/clients.py @@ -957,6 +957,67 @@ def create_return_payment_transaction( ) raise err + def create_charge_payment_transaction( + self, + payment_id: str, + payment_version: int, + charge_id: str, + amount_in_cents: int, + currency_code: str, + charge_created: datetime.datetime, + ) -> Payment: + """ + Add a Charge transaction to an existing CT Payment, mirroring + create_return_payment_transaction but for TransactionType.CHARGE. + + Args: + payment_id: CT Payment ID (UUID) + payment_version: Current version of the CT payment + charge_id: Stripe Charge ID (used as interaction_id for idempotency) + amount_in_cents: Charge amount in minor currency units + currency_code: ISO 4217 currency code (e.g. 'USD') + charge_created: Timestamp of the Stripe charge + + Returns: + Updated Payment object with the new Charge transaction + """ + try: + logger.info( + f"[CommercetoolsAPIClient] - Creating charge transaction for " + f"payment {payment_id} with charge {charge_id}" + ) + + amount_as_money = Money( + cent_amount=amount_in_cents, + currency_code=currency_code.upper(), + ) + + transaction_draft = TransactionDraft( + type=TransactionType.CHARGE, + amount=amount_as_money, + timestamp=charge_created, + state=TransactionState.SUCCESS, + interaction_id=charge_id, + ) + + add_transaction_action = PaymentAddTransactionAction( + transaction=transaction_draft + ) + + return self.base_client.payments.update_by_id( + id=payment_id, + version=payment_version, + actions=[add_transaction_action], + ) + except CommercetoolsError as err: + handle_commercetools_error( + "[CommercetoolsAPIClient.create_charge_payment_transaction]", + err, + f"Unable to create charge transaction for payment {payment_id}, " + f"charge {charge_id}", + ) + raise err + def update_line_item_on_fulfillment( self, entitlement_uuid: str, @@ -1342,6 +1403,28 @@ def update_customer( ) raise err + @conditional_retry + def get_cart_by_id(self, cart_id: str) -> Cart: + """ + Fetch a cart by its ID. + + Args: + cart_id (str): Cart ID (UUID) + + Returns: + Cart object + """ + try: + logger.info(f"[CommercetoolsAPIClient] - Attempting to find cart with ID {cart_id}") + return self.base_client.carts.get_by_id(cart_id) + except CommercetoolsError as err: + handle_commercetools_error( + "[CommercetoolsAPIClient.get_cart_by_id]", + err, + f"Failed to find cart with ID {cart_id}", + ) + raise err + @conditional_retry def get_customer_cart(self, customer_id: str) -> Optional[Cart]: """ @@ -1800,7 +1883,7 @@ def get_order_by_payment_id(self, payment_id: str) -> Order: ) if not response or not response.results: - raise Exception(f"No order found for payment ID {payment_id}") + raise ValueError(f"No order found for payment ID {payment_id}") return response.results[0] diff --git a/commerce_coordinator/apps/commercetools/signals.py b/commerce_coordinator/apps/commercetools/signals.py index bca8b5850..3724dd269 100644 --- a/commerce_coordinator/apps/commercetools/signals.py +++ b/commerce_coordinator/apps/commercetools/signals.py @@ -6,6 +6,7 @@ from commerce_coordinator.apps.commercetools.catalog_info.constants import TwoUKeys from commerce_coordinator.apps.commercetools.tasks import ( + finalize_commercetools_stripe_payment_task, fulfillment_completed_update_ct_line_item_task, refund_from_mobile_task, refund_from_paypal_task, @@ -94,6 +95,19 @@ def revoke_line_items(**kwargs): return async_result.id +@log_receiver(logger) +def finalize_commercetools_stripe_payment(**kwargs): + """ + Receive the payment_succeeded_commercetools_signal and dispatch + the shared finalize task for a CT Stripe PaymentIntent. + """ + async_result = finalize_commercetools_stripe_payment_task.delay( + payment_intent_id=kwargs["payment_intent_id"], + source="webhook", + ) + return async_result.id + + @log_receiver(logger) def revoke_line_mobile_order(**kwargs): """ diff --git a/commerce_coordinator/apps/commercetools/stripe_payment_finalize.py b/commerce_coordinator/apps/commercetools/stripe_payment_finalize.py new file mode 100644 index 000000000..21c884143 --- /dev/null +++ b/commerce_coordinator/apps/commercetools/stripe_payment_finalize.py @@ -0,0 +1,421 @@ +""" +Shared finalization logic for CommerceTools orders originating from Stripe +PaymentIntents (UPI webhook). + +Parity source: customer-twou finalizeStripePayment + runPostPaymentActions. +""" + +import datetime +import logging +from dataclasses import dataclass + +import stripe +from commercetools import CommercetoolsError +from commercetools.platform.models import TransactionType + +from commerce_coordinator.apps.commercetools.catalog_info.constants import TwoUKeys +from commerce_coordinator.apps.commercetools.catalog_info.edx_utils import ( + cents_to_dollars, + get_edx_lms_user_id, + get_product_from_line_item +) +from commerce_coordinator.apps.commercetools.clients import CommercetoolsAPIClient +from commerce_coordinator.apps.core.memcache import safe_key +from commerce_coordinator.apps.core.segment import track +from commerce_coordinator.apps.core.tasks import acquire_task_lock, release_task_lock + +logger = logging.getLogger(__name__) + +FINALIZE_LOCK_PREFIX = "finalize_ct_order_from_stripe_pi" +# Default lock TTL is 60s; this path can exceed that under CT latency. +# 5 minutes covers a slow run without pinning a crashed worker for 30 minutes. +FINALIZE_LOCK_EXPIRE = 300 + + +class FinalizeError(Exception): + """Non-retryable finalization error (quarantine candidate).""" + + def __init__( + self, + message: str, + *, + ct_payment_id: str = "unknown", + ct_cart_id: str = "unknown", + ): + super().__init__(message) + self.ct_payment_id = ct_payment_id or "unknown" + self.ct_cart_id = ct_cart_id or "unknown" + + +class FinalizeInProgressError(Exception): + """Another worker holds the finalize lock for this PI.""" + + +@dataclass +class FinalizeResult: + order_id: str + order_number: str + payment_id: str + already_existed: bool = False + + +def _payment_has_charge_for(payment, charge_id: str) -> bool: + """Check whether the CT payment already has a Charge txn for this charge.""" + if not payment.transactions: + return False + return any( + t.type == TransactionType.CHARGE and t.interaction_id == charge_id + for t in payment.transactions + ) + + +def _backfill_pi_metadata( + payment_intent_id: str, + order_id: str, + payment_id: str, + *, + existing_metadata: dict | None = None, +) -> None: + """ + Write order_id / ct_payment_id onto the Stripe PaymentIntent (idempotent). + + Merges with existing metadata so keys like source_system / ct_cart_id are preserved + even if Stripe treats metadata as a full replacement. + """ + try: + merged = dict(existing_metadata or {}) + merged["order_id"] = order_id + merged["ct_payment_id"] = payment_id + stripe.PaymentIntent.modify( + payment_intent_id, + metadata=merged, + ) + except Exception: # pylint: disable=broad-exception-caught + logger.warning( + "[finalize_ct_order] Failed to backfill PI metadata for %s", + payment_intent_id, + exc_info=True, + ) + + +def _discount_amount_dollars(cart) -> float: + """Extract cart-level discount as dollars from CT cart shapes.""" + discount_on_total = getattr(cart, "discount_on_total_price", None) + if not discount_on_total: + return 0 + + discounted_amount = getattr(discount_on_total, "discounted_amount", None) + if discounted_amount is not None: + return cents_to_dollars(discounted_amount) + + # Fallback if a money-like object was passed directly (tests / older shapes) + if hasattr(discount_on_total, "cent_amount"): + return cents_to_dollars(discount_on_total) + + return 0 + + +def _line_item_has_state_id(line_item, state_id: str) -> bool: + """True if any ItemState on the line item references the given state ID.""" + for item_state in (getattr(line_item, "state", None) or []): + ref = getattr(item_state, "state", None) + if ref is not None and getattr(ref, "id", None) == state_id: + return True + return False + + +def _ensure_pending_fulfilment(client, order): + """ + Transition line items still in Initial → PENDING_FULFILMENT. + + Uses TwoUKeys.INITIAL_FULFILMENT_STATE (looked up by key) as from_state so we do + not depend on line_items[0].state[0], which is fragile on partial-success retries. + """ + if not order.line_items: + logger.warning( + "[finalize_ct_order] Order %s has no line items; cannot transition fulfillment", + order.id, + ) + return order + + initial_state = client.get_state_by_key(TwoUKeys.INITIAL_FULFILMENT_STATE) + items_needing_transition = [ + item for item in order.line_items + if _line_item_has_state_id(item, initial_state.id) + ] + if not items_needing_transition: + logger.info( + "[finalize_ct_order] Order %s line items already past Initial; skipping transition", + order.id, + ) + return order + + return client.update_line_items_transition_state( + order_id=order.id, + order_version=order.version, + line_items=items_needing_transition, + from_state_id=initial_state.id, + new_state_key=TwoUKeys.PENDING_FULFILMENT_STATE, + use_state_id=True, + ) + + +def finalize_ct_order_from_stripe_pi( + payment_intent_id: str, + *, + source: str, + client: CommercetoolsAPIClient | None = None, +) -> FinalizeResult: + """ + Shared finalize path used by the webhook Celery task. + + Steps (parity with customer-twou finalizeStripePayment): + 1. Retrieve / validate Stripe PaymentIntent + 2. Resolve CT Payment (by key = pi.id or metadata.ct_payment_id) + 3. Resolve CT Cart (by metadata.ct_cart_id) + 4. Add Charge transaction if absent (idempotent by interaction_id) + 5. If order already exists: heal PENDING_FULFILMENT + PI metadata, return + 6. Create order from cart → COMPLETE / PAID / SHIPPED + 7. Transition line items → PENDING_FULFILMENT (from Initial by key) + 8. Emit Segment Order Completed (plan 18, is_mobile=False) + 9. Backfill PI metadata with order_id + ct_payment_id + + Args: + payment_intent_id: Stripe PaymentIntent ID + source: quarantine log context (typically 'webhook') + client: Optional pre-built CT client (avoids re-init in loops) + + Returns: + FinalizeResult with order details + + Raises: + FinalizeError: on non-retryable problems (missing metadata, etc.) + FinalizeInProgressError: when another writer holds the PI finalize lock + CommercetoolsError: on transient CT failures (retryable by caller) + """ + lock_key = safe_key( + key=payment_intent_id, + key_prefix=FINALIZE_LOCK_PREFIX, + version="1", + ) + if not acquire_task_lock(lock_key, FINALIZE_LOCK_EXPIRE): + raise FinalizeInProgressError( + f"Finalize already in progress for PaymentIntent {payment_intent_id}" + ) + + try: + return _finalize_ct_order_from_stripe_pi_locked( + payment_intent_id, source=source, client=client, + ) + finally: + release_task_lock(lock_key) + + +def _finalize_ct_order_from_stripe_pi_locked( + payment_intent_id: str, + *, + source: str, + client: CommercetoolsAPIClient | None = None, +) -> FinalizeResult: + """Finalize body; caller holds the PI lock.""" + if client is None: + client = CommercetoolsAPIClient() + + pi = stripe.PaymentIntent.retrieve(payment_intent_id) + metadata = pi.metadata or {} + + if pi.status != "succeeded": + raise FinalizeError( + f"PaymentIntent {payment_intent_id} status is '{pi.status}', expected 'succeeded'", + ct_cart_id=metadata.get("ct_cart_id", "unknown"), + ct_payment_id=metadata.get("ct_payment_id", "unknown"), + ) + + if metadata.get("source_system") != "commercetools": + raise FinalizeError( + f"PaymentIntent {payment_intent_id} source_system is " + f"'{metadata.get('source_system')}', expected 'commercetools'", + ct_cart_id=metadata.get("ct_cart_id", "unknown"), + ct_payment_id=metadata.get("ct_payment_id", "unknown"), + ) + + ct_cart_id = metadata.get("ct_cart_id") + if not ct_cart_id: + raise FinalizeError( + f"PaymentIntent {payment_intent_id} missing metadata.ct_cart_id", + ct_payment_id=metadata.get("ct_payment_id", "unknown"), + ) + + ct_payment_id_from_meta = metadata.get("ct_payment_id") + + # --- Resolve CT Payment --- + if ct_payment_id_from_meta: + try: + payment = client.base_client.payments.get_by_id(ct_payment_id_from_meta) + except CommercetoolsError as err: + # Only fall back on true not-found; re-raise transient CT failures for retry. + if err.code != "ResourceNotFound": + raise + logger.warning( + "[finalize_ct_order] ct_payment_id %s from metadata not found, " + "falling back to key lookup for pi %s", + ct_payment_id_from_meta, payment_intent_id, + ) + payment = client.get_payment_by_key(payment_intent_id) + else: + payment = client.get_payment_by_key(payment_intent_id) + + # --- Add Charge transaction if absent --- + latest_charge = pi.latest_charge + if latest_charge and isinstance(latest_charge, str): + latest_charge = stripe.Charge.retrieve(latest_charge) + + if latest_charge and not _payment_has_charge_for(payment, latest_charge.id): + payment = client.create_charge_payment_transaction( + payment_id=payment.id, + payment_version=payment.version, + charge_id=latest_charge.id, + amount_in_cents=latest_charge.amount, + currency_code=latest_charge.currency, + charge_created=datetime.datetime.fromtimestamp( + latest_charge.created, tz=datetime.timezone.utc + ), + ) + logger.info( + "[finalize_ct_order] Added Charge txn for pi=%s charge=%s", + payment_intent_id, latest_charge.id, + ) + + # --- Check if order already exists --- + # ValueError = not found (aligned with client docstring). CommercetoolsError must + # propagate so Celery can retry instead of creating a duplicate order. + try: + existing_order = client.get_order_by_payment_id(payment.id) + except ValueError: + existing_order = None + + if existing_order is not None: + logger.info( + "[finalize_ct_order] Order %s already exists for payment %s (pi=%s), " + "skipping creation; healing PENDING_FULFILMENT and PI metadata", + existing_order.id, payment.id, payment_intent_id, + ) + # Partial-success heal: order may exist while line items are still Initial. + _ensure_pending_fulfilment(client, existing_order) + if not metadata.get("order_id") or metadata.get("ct_payment_id") != payment.id: + _backfill_pi_metadata( + payment_intent_id, + existing_order.id, + payment.id, + existing_metadata=metadata, + ) + return FinalizeResult( + order_id=existing_order.id, + order_number=existing_order.order_number or "", + payment_id=payment.id, + already_existed=True, + ) + + # --- Load cart and create order --- + cart = client.get_cart_by_id(ct_cart_id) + order = client.create_order_from_cart(cart) + + # --- Transition line items → PENDING_FULFILMENT --- + order = _ensure_pending_fulfilment(client, order) + + # --- Emit Segment Order Completed (plan 18, web) --- + _emit_web_order_completed(client, order, cart, payment) + + # --- Backfill PI metadata --- + _backfill_pi_metadata( + payment_intent_id, + order.id, + payment.id, + existing_metadata=metadata, + ) + + logger.info( + "[finalize_ct_order] Successfully finalized order %s for pi=%s source=%s", + order.id, payment_intent_id, source, + ) + + return FinalizeResult( + order_id=order.id, + order_number=order.order_number or "", + payment_id=payment.id, + ) + + +def _emit_web_order_completed(client, order, cart, payment): + """Emit Segment 'Order Completed' event for the web/UPI path (plan 18).""" + try: + customer = client.get_customer_by_id(order.customer_id) + lms_user_id = get_edx_lms_user_id(customer) + + standalone_price = cart.total_price + products = [ + get_product_from_line_item(item, standalone_price) + for item in cart.line_items + ] + + payment_method = "unknown" + if payment.payment_method_info and payment.payment_method_info.method: + payment_method = payment.payment_method_info.method + + discount_codes = getattr(cart, "discount_codes", []) or [] + discount_code = None + coupon_name = [] + if discount_codes: + codes_as_dicts = [] + for dc in discount_codes: + code_obj = getattr(dc, "discount_code", None) + if code_obj is not None and hasattr(code_obj, "obj") and code_obj.obj: + codes_as_dicts.append({"code": getattr(code_obj.obj, "code", None)}) + elif hasattr(dc, "code"): + codes_as_dicts.append({"code": dc.code}) + elif isinstance(dc, dict) and "code" in dc: + codes_as_dicts.append(dc) + codes_as_dicts = [d for d in codes_as_dicts if d.get("code")] + if codes_as_dicts: + discount_code = codes_as_dicts[-1].get("code") + coupon_name = [ + d["code"] for d in codes_as_dicts[:-1] + if d.get("code") + ] + + taxed_amount = 0 + if order.taxed_price and order.taxed_price.total_tax: + taxed_amount = cents_to_dollars(order.taxed_price.total_tax) + + event_props = { + "track_plan_id": 18, + "trigger_source": "server-side", + "order_id": order.id, + "checkout_id": cart.id, + "currency": standalone_price.currency_code, + "total": cents_to_dollars(standalone_price), + "tax": taxed_amount, + "coupon": discount_code, + "coupon_name": coupon_name, + "discount": _discount_amount_dollars(cart), + "payment_method": payment_method, + "processor_name": "stripe", + "products": products, + "is_mobile": False, + "multi_item_cart_enabled": len(cart.line_items) > 1, + } + + track( + lms_user_id=lms_user_id, + event="Order Completed", + properties=event_props, + ) + logger.info( + "[finalize_ct_order] Emitted Segment Order Completed for order %s, user %s", + order.id, lms_user_id, + ) + except Exception: # pylint: disable=broad-exception-caught + logger.warning( + "[finalize_ct_order] Failed to emit Segment Order Completed for order %s", + order.id, exc_info=True, + ) diff --git a/commerce_coordinator/apps/commercetools/tasks.py b/commerce_coordinator/apps/commercetools/tasks.py index ce46f335b..48e9a3efe 100644 --- a/commerce_coordinator/apps/commercetools/tasks.py +++ b/commerce_coordinator/apps/commercetools/tasks.py @@ -36,6 +36,7 @@ from commerce_coordinator.apps.order_fulfillment.serializers import OrderRevokeLineRequestSerializer from .clients import CommercetoolsAPIClient, Refund +from .stripe_payment_finalize import FinalizeError, FinalizeInProgressError, finalize_ct_order_from_stripe_pi from .utils import ( convert_ct_cent_amount_to_localized_price, get_lob_from_variant_attr, @@ -552,3 +553,120 @@ def revoke_line_mobile_order_task(payment_id: str): f"on course {course_run_key} and {logging_data}") return True + + +def _log_quarantine(*, pi_id, ct_payment_id, ct_cart_id, reason, source): + """ + Structured quarantine log for finalize failures that exhaust retries + or hit non-retryable errors. Fixed field contract for ops queries. + """ + logger.error( + "[quarantine] Finalize failure | pi_id=%s ct_payment_id=%s " + "ct_cart_id=%s reason=%s source=%s", + pi_id, ct_payment_id, ct_cart_id, reason, source, + extra={ + "quarantine": True, + "pi_id": pi_id, + "ct_payment_id": ct_payment_id, + "ct_cart_id": ct_cart_id, + "reason": reason, + "source": source, + }, + ) + + +@shared_task( + bind=True, + autoretry_for=(CommercetoolsError, stripe.error.StripeError), + max_retries=5, + retry_kwargs={"countdown": 300}, # 5 minutes between retries, to cover time during outage +) +def finalize_commercetools_stripe_payment_task( + self, + payment_intent_id: str, + source: str = "webhook", +): + """ + Celery task wrapping the shared finalize path for a Stripe + PaymentIntent that originated from a CommerceTools cart. + + Bounded retries on transient CT/Stripe errors; non-retryable failures + are quarantined via structured log. + """ + tag = "finalize_commercetools_stripe_payment_task" + + try: + result = finalize_ct_order_from_stripe_pi( + payment_intent_id, source=source, + ) + if result.already_existed: + logger.info( + "[%s] Order %s already existed for pi=%s; fulfillment/metadata heal applied", + tag, result.order_id, payment_intent_id, + ) + else: + logger.info( + "[%s] Finalized order %s for pi=%s", + tag, result.order_id, payment_intent_id, + ) + return result.order_id + + except FinalizeInProgressError: + logger.info( + "[%s] Finalize lock held for pi=%s; retrying in %s seconds", + tag, payment_intent_id, TASK_LOCK_RETRY, + ) + finalize_commercetools_stripe_payment_task.apply_async( + kwargs={ + "payment_intent_id": payment_intent_id, + "source": source, + }, + countdown=TASK_LOCK_RETRY, + ) + return None + + except FinalizeError as exc: + _log_quarantine( + pi_id=payment_intent_id, + ct_payment_id=getattr(exc, "ct_payment_id", "unknown"), + ct_cart_id=getattr(exc, "ct_cart_id", "unknown"), + reason=str(exc), + source=source, + ) + return None + + except (CommercetoolsError, stripe.error.StripeError): + if self.request.retries >= self.max_retries: + ct_payment_id, ct_cart_id = _quarantine_ids_from_pi(payment_intent_id) + _log_quarantine( + pi_id=payment_intent_id, + ct_payment_id=ct_payment_id, + ct_cart_id=ct_cart_id, + reason="max retries exhausted on transient error", + source=source, + ) + raise + + except Exception as exc: + ct_payment_id, ct_cart_id = _quarantine_ids_from_pi(payment_intent_id) + _log_quarantine( + pi_id=payment_intent_id, + ct_payment_id=ct_payment_id, + ct_cart_id=ct_cart_id, + reason=f"unexpected error: {exc}", + source=source, + ) + raise + + +def _quarantine_ids_from_pi(payment_intent_id: str) -> tuple[str, str]: + """Best-effort ct_payment_id / ct_cart_id from Stripe PI metadata for quarantine logs.""" + try: + pi = stripe.PaymentIntent.retrieve(payment_intent_id) + metadata = pi.metadata or {} + return ( + metadata.get("ct_payment_id") or "unknown", + metadata.get("ct_cart_id") or "unknown", + ) + except Exception: # pylint: disable=broad-exception-caught + return "unknown", "unknown" diff --git a/commerce_coordinator/apps/commercetools/tests/test_clients.py b/commerce_coordinator/apps/commercetools/tests/test_clients.py index 0c2889059..eb9c8b150 100644 --- a/commerce_coordinator/apps/commercetools/tests/test_clients.py +++ b/commerce_coordinator/apps/commercetools/tests/test_clients.py @@ -1,6 +1,6 @@ """ Commercetools API Client(s) Testing """ -from datetime import datetime +from datetime import datetime, timezone from unittest.mock import MagicMock, Mock import pytest @@ -1845,12 +1845,36 @@ def test_get_order_by_payment_id_no_order_found(self): status_code=200 ) - with self.assertRaises(Exception) as exc: + with self.assertRaises(ValueError) as exc: self.client_set.client.get_order_by_payment_id(payment_id) # Verify the exception message self.assertEqual(str(exc.exception), f"No order found for payment ID {payment_id}") + def test_create_charge_payment_transaction(self): + """Add a Charge transaction to an existing CT payment.""" + base_url = self.client_set.get_base_url_from_client() + mock_response_payment = gen_payment() + charge_created = datetime.fromtimestamp(1692942318, tz=timezone.utc) + + with requests_mock.Mocker(real_http=True, case_sensitive=False) as mocker: + mocker.post( + f"{base_url}payments/{mock_response_payment.id}", + json=mock_response_payment.serialize(), + status_code=200 + ) + + result = self.client_set.client.create_charge_payment_transaction( + payment_id=mock_response_payment.id, + payment_version=mock_response_payment.version, + charge_id="ch_3P9RWsH4caH7G0X11toRGUJf", + amount_in_cents=4900, + currency_code="usd", + charge_created=charge_created, + ) + + self.assertEqual(result.id, mock_response_payment.id) + def test_get_credit_variant_by_course_run(self): base_url = self.client_set.get_base_url_from_client() course_run_key = "course-v1:edX+DemoX+2025_T1" diff --git a/commerce_coordinator/apps/commercetools/tests/test_finalize_task.py b/commerce_coordinator/apps/commercetools/tests/test_finalize_task.py new file mode 100644 index 000000000..4fde55007 --- /dev/null +++ b/commerce_coordinator/apps/commercetools/tests/test_finalize_task.py @@ -0,0 +1,85 @@ +""" +Tests for the finalize_commercetools_stripe_payment_task Celery task. +""" +# Celery's bind=True self argument is supplied by the task decorator. +# pylint: disable=no-value-for-parameter + +from unittest.mock import patch + +from django.test import TestCase + +from commerce_coordinator.apps.commercetools.stripe_payment_finalize import ( + FinalizeError, + FinalizeInProgressError, + FinalizeResult +) +from commerce_coordinator.apps.commercetools.tasks import finalize_commercetools_stripe_payment_task + +FINALIZE_PATH = "commerce_coordinator.apps.commercetools.tasks.finalize_ct_order_from_stripe_pi" + + +class TestFinalizeTask(TestCase): + """Tests for the Celery task wrapping the shared Stripe/CT finalize path.""" + + @patch(FINALIZE_PATH) + def test_happy_path_returns_order_id(self, mock_finalize): + mock_finalize.return_value = FinalizeResult( + order_id="order-123", + order_number="2U-2026000001", + payment_id="pay-456", + ) + + result = finalize_commercetools_stripe_payment_task("pi_test") + self.assertEqual(result, "order-123") + mock_finalize.assert_called_once_with("pi_test", source="webhook") + + @patch(FINALIZE_PATH) + def test_already_existed_returns_order_id(self, mock_finalize): + mock_finalize.return_value = FinalizeResult( + order_id="order-existing", + order_number="2U-2026000002", + payment_id="pay-789", + already_existed=True, + ) + + result = finalize_commercetools_stripe_payment_task("pi_test") + self.assertEqual(result, "order-existing") + + @patch(FINALIZE_PATH) + @patch("commerce_coordinator.apps.commercetools.tasks._log_quarantine") + def test_finalize_error_quarantines_and_returns_none( + self, mock_quarantine, mock_finalize + ): + mock_finalize.side_effect = FinalizeError("missing ct_cart_id") + + result = finalize_commercetools_stripe_payment_task("pi_bad") + self.assertIsNone(result) + mock_quarantine.assert_called_once() + call_kwargs = mock_quarantine.call_args[1] + self.assertEqual(call_kwargs["pi_id"], "pi_bad") + self.assertEqual(call_kwargs["source"], "webhook") + self.assertIn("missing ct_cart_id", call_kwargs["reason"]) + + @patch(FINALIZE_PATH) + @patch("commerce_coordinator.apps.commercetools.tasks._log_quarantine") + def test_unexpected_error_quarantines_and_reraises( + self, mock_quarantine, mock_finalize + ): + mock_finalize.side_effect = RuntimeError("boom") + + with self.assertRaises(RuntimeError): + finalize_commercetools_stripe_payment_task("pi_boom") + + mock_quarantine.assert_called_once() + + @patch.object(finalize_commercetools_stripe_payment_task, "apply_async") + @patch(FINALIZE_PATH) + def test_lock_contention_reschedules(self, mock_finalize, mock_apply_async): + mock_finalize.side_effect = FinalizeInProgressError("locked") + + result = finalize_commercetools_stripe_payment_task("pi_busy") + + self.assertIsNone(result) + mock_apply_async.assert_called_once() + call_kwargs = mock_apply_async.call_args.kwargs + self.assertEqual(call_kwargs["kwargs"]["payment_intent_id"], "pi_busy") diff --git a/commerce_coordinator/apps/commercetools/tests/test_stripe_payment_finalize.py b/commerce_coordinator/apps/commercetools/tests/test_stripe_payment_finalize.py new file mode 100644 index 000000000..d2ce45a11 --- /dev/null +++ b/commerce_coordinator/apps/commercetools/tests/test_stripe_payment_finalize.py @@ -0,0 +1,464 @@ +""" +Tests for the shared CT order finalization from Stripe PaymentIntents. +""" +# Class-level patch decorators inject every mock into each test method. +# pylint: disable=unused-argument + +import datetime +from unittest.mock import MagicMock, patch + +from commercetools import CommercetoolsError +from commercetools.platform.models import ( + CentPrecisionMoney, + Payment, + PaymentMethodInfo, + PaymentState, + Transaction, + TransactionState, + TransactionType +) +from django.test import TestCase + +from commerce_coordinator.apps.commercetools.catalog_info.constants import TwoUKeys +from commerce_coordinator.apps.commercetools.stripe_payment_finalize import ( + FinalizeError, + FinalizeInProgressError, + _payment_has_charge_for, + finalize_ct_order_from_stripe_pi +) +from commerce_coordinator.apps.commercetools.tests.conftest import gen_cart, gen_customer, gen_order +from commerce_coordinator.apps.core.tests.utils import uuid4_str + + +def _ct_error(code: str, message: str = "boom") -> CommercetoolsError: + """Build a CommercetoolsError whose .code property matches production CT errors.""" + response = MagicMock() + err_obj = MagicMock() + err_obj.code = code + response.errors = [err_obj] + return CommercetoolsError( + message=message, + errors=[{"code": code, "message": message}], + response=response, + correlation_id="corr", + ) + + +def _stub_initial_matching_order(client, order): + """Make get_state_by_key(Initial) match the order's first line-item state id.""" + initial = MagicMock() + initial.id = order.line_items[0].state[0].state.id + initial.key = TwoUKeys.INITIAL_FULFILMENT_STATE + client.get_state_by_key.return_value = initial + return initial + + +def _stub_initial_unrelated(client): + """Initial state id that will not match order line items (already past Initial).""" + initial = MagicMock() + initial.id = "initial-state-id-unrelated" + initial.key = TwoUKeys.INITIAL_FULFILMENT_STATE + client.get_state_by_key.return_value = initial + return initial + + +def _mock_pi( + pi_id="pi_test123", + pi_status="succeeded", + source_system="commercetools", + ct_cart_id="cart-uuid", + ct_payment_id=None, + order_id=None, + latest_charge="ch_test456", +): + """Build a Stripe PaymentIntent stub with CT-linking metadata.""" + pi = MagicMock() + pi.id = pi_id + pi.status = pi_status + pi.metadata = { + "source_system": source_system, + "ct_cart_id": ct_cart_id, + } + if ct_payment_id: + pi.metadata["ct_payment_id"] = ct_payment_id + if order_id: + pi.metadata["order_id"] = order_id + pi.latest_charge = latest_charge + return pi + + +def _mock_charge(charge_id="ch_test456", amount=4900, currency="usd"): + """Build a Stripe Charge stub for the PaymentIntent's latest charge.""" + charge = MagicMock() + charge.id = charge_id + charge.amount = amount + charge.currency = currency + charge.created = 1700000000 + return charge + + +def _mock_payment(payment_id=None, version=1, has_charge=False, charge_id="ch_test456"): + """Build a CT Payment, optionally already carrying a successful Charge transaction.""" + txns = [] + if has_charge: + txns.append(Transaction( + id=uuid4_str(), + type=TransactionType.CHARGE, + amount=CentPrecisionMoney(cent_amount=4900, currency_code="USD", fraction_digits=2), + state=TransactionState.SUCCESS, + interaction_id=charge_id, + timestamp=datetime.datetime.now(), + )) + return Payment( + id=payment_id or uuid4_str(), + version=version, + created_at=datetime.datetime.now(), + last_modified_at=datetime.datetime.now(), + amount_planned=CentPrecisionMoney(cent_amount=4900, currency_code="USD", fraction_digits=2), + payment_method_info=PaymentMethodInfo(method="upi"), + payment_status=PaymentState.PAID, + transactions=txns, + interface_interactions=[], + ) + + +class TestPaymentHasChargeFor(TestCase): + """Tests for Charge transaction idempotency detection on a CT payment.""" + + def test_no_transactions(self): + payment = _mock_payment() + self.assertFalse(_payment_has_charge_for(payment, "ch_test")) + + def test_has_matching_charge(self): + payment = _mock_payment(has_charge=True, charge_id="ch_match") + self.assertTrue(_payment_has_charge_for(payment, "ch_match")) + + def test_has_different_charge(self): + payment = _mock_payment(has_charge=True, charge_id="ch_other") + self.assertFalse(_payment_has_charge_for(payment, "ch_match")) + + +@patch("commerce_coordinator.apps.commercetools.stripe_payment_finalize.release_task_lock") +@patch( + "commerce_coordinator.apps.commercetools.stripe_payment_finalize.acquire_task_lock", + return_value=True, +) +@patch("commerce_coordinator.apps.commercetools.stripe_payment_finalize.stripe") +@patch("commerce_coordinator.apps.commercetools.stripe_payment_finalize.track") +@patch("commerce_coordinator.apps.commercetools.stripe_payment_finalize.CommercetoolsAPIClient") +class TestFinalizeCTOrderFromStripePI(TestCase): + """Tests for finalizing a CT order from a Stripe PaymentIntent.""" + + def test_happy_path(self, MockClient, mock_track, mock_stripe, _mock_lock, _mock_unlock): + """Full finalize: charge + order + line state + segment + PI metadata.""" + pi = _mock_pi() + charge = _mock_charge() + mock_stripe.PaymentIntent.retrieve.return_value = pi + mock_stripe.Charge.retrieve.return_value = charge + + payment = _mock_payment(payment_id="pay-123") + order = gen_order(uuid4_str()) + cart = gen_cart(cart_id="cart-uuid", customer_id=order.customer_id) + customer = gen_customer("test@example.com", "testuser") + + client = MockClient.return_value + client.get_payment_by_key.return_value = payment + client.create_charge_payment_transaction.return_value = payment + client.get_order_by_payment_id.side_effect = ValueError("not found") + client.get_cart_by_id.return_value = cart + client.create_order_from_cart.return_value = order + client.update_line_items_transition_state.return_value = order + client.get_customer_by_id.return_value = customer + initial = _stub_initial_matching_order(client, order) + + result = finalize_ct_order_from_stripe_pi("pi_test123", source="webhook") + + self.assertEqual(result.order_id, order.id) + self.assertFalse(result.already_existed) + client.create_charge_payment_transaction.assert_called_once() + client.create_order_from_cart.assert_called_once_with(cart) + client.get_state_by_key.assert_called_with(TwoUKeys.INITIAL_FULFILMENT_STATE) + client.update_line_items_transition_state.assert_called_once() + transition_kwargs = client.update_line_items_transition_state.call_args.kwargs + self.assertEqual(transition_kwargs["from_state_id"], initial.id) + self.assertEqual(transition_kwargs["new_state_key"], TwoUKeys.PENDING_FULFILMENT_STATE) + mock_track.assert_called_once() + mock_stripe.PaymentIntent.modify.assert_called_once_with( + "pi_test123", + metadata={ + "source_system": "commercetools", + "ct_cart_id": "cart-uuid", + "order_id": order.id, + "ct_payment_id": "pay-123", + }, + ) + + def test_order_already_exists_heals_fulfillment_and_metadata( + self, MockClient, mock_track, mock_stripe, _mock_lock, _mock_unlock + ): + """Existing order with Initial line items still transitions + heals PI metadata.""" + pi = _mock_pi() + charge = _mock_charge() + mock_stripe.PaymentIntent.retrieve.return_value = pi + mock_stripe.Charge.retrieve.return_value = charge + + payment = _mock_payment(payment_id="pay-123", has_charge=True, charge_id="ch_test456") + existing_order = gen_order(uuid4_str()) + + client = MockClient.return_value + client.get_payment_by_key.return_value = payment + client.get_order_by_payment_id.return_value = existing_order + client.update_line_items_transition_state.return_value = existing_order + initial = _stub_initial_matching_order(client, existing_order) + + result = finalize_ct_order_from_stripe_pi("pi_test123", source="webhook") + + self.assertTrue(result.already_existed) + self.assertEqual(result.order_id, existing_order.id) + client.create_order_from_cart.assert_not_called() + client.update_line_items_transition_state.assert_called_once() + transition_kwargs = client.update_line_items_transition_state.call_args.kwargs + self.assertEqual(transition_kwargs["from_state_id"], initial.id) + self.assertEqual(transition_kwargs["new_state_key"], TwoUKeys.PENDING_FULFILMENT_STATE) + mock_track.assert_not_called() + mock_stripe.PaymentIntent.modify.assert_called_once_with( + "pi_test123", + metadata={ + "source_system": "commercetools", + "ct_cart_id": "cart-uuid", + "order_id": existing_order.id, + "ct_payment_id": "pay-123", + }, + ) + + def test_order_already_exists_skips_transition_when_past_initial( + self, MockClient, mock_track, mock_stripe, _mock_lock, _mock_unlock + ): + """When order exists and line items are past Initial, do not re-transition.""" + existing_order = gen_order(uuid4_str()) + pi = _mock_pi(order_id=existing_order.id, ct_payment_id="pay-123") + charge = _mock_charge() + mock_stripe.PaymentIntent.retrieve.return_value = pi + mock_stripe.Charge.retrieve.return_value = charge + + payment = _mock_payment(payment_id="pay-123", has_charge=True, charge_id="ch_test456") + + client = MockClient.return_value + client.base_client.payments.get_by_id.return_value = payment + client.get_order_by_payment_id.return_value = existing_order + _stub_initial_unrelated(client) + + result = finalize_ct_order_from_stripe_pi("pi_test123", source="webhook") + + self.assertTrue(result.already_existed) + client.update_line_items_transition_state.assert_not_called() + mock_stripe.PaymentIntent.modify.assert_not_called() + + def test_lock_contention_raises_in_progress( + self, MockClient, mock_track, mock_stripe, mock_lock, _mock_unlock + ): + mock_lock.return_value = False + with self.assertRaises(FinalizeInProgressError): + finalize_ct_order_from_stripe_pi("pi_test123", source="webhook") + MockClient.assert_not_called() + + def test_ct_outage_on_order_lookup_propagates( + self, MockClient, mock_track, mock_stripe, _mock_lock, _mock_unlock + ): + """CommercetoolsError during order lookup must not be treated as not-found.""" + pi = _mock_pi() + charge = _mock_charge() + mock_stripe.PaymentIntent.retrieve.return_value = pi + mock_stripe.Charge.retrieve.return_value = charge + + payment = _mock_payment(payment_id="pay-123", has_charge=True, charge_id="ch_test456") + + client = MockClient.return_value + client.get_payment_by_key.return_value = payment + client.get_order_by_payment_id.side_effect = _ct_error("ConcurrentModification") + + with self.assertRaises(CommercetoolsError): + finalize_ct_order_from_stripe_pi("pi_test123", source="webhook") + + client.create_order_from_cart.assert_not_called() + + def test_order_already_exists_skips( + self, MockClient, mock_track, mock_stripe, _mock_lock, _mock_unlock + ): + """When order already exists for the payment, skip creation.""" + pi = _mock_pi() + charge = _mock_charge() + mock_stripe.PaymentIntent.retrieve.return_value = pi + mock_stripe.Charge.retrieve.return_value = charge + + payment = _mock_payment(payment_id="pay-123", has_charge=True, charge_id="ch_test456") + existing_order = gen_order(uuid4_str()) + + client = MockClient.return_value + client.get_payment_by_key.return_value = payment + client.get_order_by_payment_id.return_value = existing_order + client.update_line_items_transition_state.return_value = existing_order + _stub_initial_matching_order(client, existing_order) + + result = finalize_ct_order_from_stripe_pi("pi_test123", source="webhook") + + self.assertTrue(result.already_existed) + self.assertEqual(result.order_id, existing_order.id) + client.create_order_from_cart.assert_not_called() + mock_track.assert_not_called() + + def test_charge_already_present_skips_creation( + self, MockClient, mock_track, mock_stripe, _mock_lock, _mock_unlock + ): + """When charge transaction already exists, don't add another.""" + pi = _mock_pi() + charge = _mock_charge() + mock_stripe.PaymentIntent.retrieve.return_value = pi + mock_stripe.Charge.retrieve.return_value = charge + + payment = _mock_payment(payment_id="pay-123", has_charge=True, charge_id="ch_test456") + order = gen_order(uuid4_str()) + cart = gen_cart(cart_id="cart-uuid", customer_id=order.customer_id) + customer = gen_customer("test@example.com", "testuser") + + client = MockClient.return_value + client.get_payment_by_key.return_value = payment + client.get_order_by_payment_id.side_effect = ValueError("not found") + client.get_cart_by_id.return_value = cart + client.create_order_from_cart.return_value = order + client.update_line_items_transition_state.return_value = order + client.get_customer_by_id.return_value = customer + _stub_initial_matching_order(client, order) + + result = finalize_ct_order_from_stripe_pi("pi_test123", source="webhook") + + client.create_charge_payment_transaction.assert_not_called() + self.assertFalse(result.already_existed) + + def test_pi_not_succeeded_raises_finalize_error( + self, MockClient, mock_track, mock_stripe, _mock_lock, _mock_unlock + ): + pi = _mock_pi(pi_status="requires_payment_method") + mock_stripe.PaymentIntent.retrieve.return_value = pi + + with self.assertRaises(FinalizeError) as ctx: + finalize_ct_order_from_stripe_pi("pi_test123", source="webhook") + + self.assertIn("requires_payment_method", str(ctx.exception)) + + def test_wrong_source_system_raises_finalize_error( + self, MockClient, mock_track, mock_stripe, _mock_lock, _mock_unlock + ): + pi = _mock_pi(source_system="edx/commerce_coordinator?v=1") + mock_stripe.PaymentIntent.retrieve.return_value = pi + + with self.assertRaises(FinalizeError): + finalize_ct_order_from_stripe_pi("pi_test123", source="webhook") + + def test_missing_ct_cart_id_raises_finalize_error( + self, MockClient, mock_track, mock_stripe, _mock_lock, _mock_unlock + ): + pi = _mock_pi(ct_cart_id=None) + pi.metadata.pop("ct_cart_id", None) + mock_stripe.PaymentIntent.retrieve.return_value = pi + + with self.assertRaises(FinalizeError) as ctx: + finalize_ct_order_from_stripe_pi("pi_test123", source="webhook") + + self.assertEqual(ctx.exception.ct_cart_id, "unknown") + + def test_resolves_payment_by_ct_payment_id( + self, MockClient, mock_track, mock_stripe, _mock_lock, _mock_unlock + ): + """When ct_payment_id is in metadata, use it for lookup.""" + pi = _mock_pi(ct_payment_id="pay-from-meta") + charge = _mock_charge() + mock_stripe.PaymentIntent.retrieve.return_value = pi + mock_stripe.Charge.retrieve.return_value = charge + + payment = _mock_payment(payment_id="pay-from-meta", has_charge=True, charge_id="ch_test456") + existing_order = gen_order(uuid4_str()) + + client = MockClient.return_value + client.base_client.payments.get_by_id.return_value = payment + client.get_order_by_payment_id.return_value = existing_order + client.update_line_items_transition_state.return_value = existing_order + _stub_initial_matching_order(client, existing_order) + + result = finalize_ct_order_from_stripe_pi("pi_test123", source="webhook") + + client.base_client.payments.get_by_id.assert_called_once_with("pay-from-meta") + self.assertTrue(result.already_existed) + + def test_ct_payment_id_not_found_falls_back_to_key( + self, MockClient, mock_track, mock_stripe, _mock_lock, _mock_unlock + ): + """ResourceNotFound on metadata.ct_payment_id falls back to PI key lookup.""" + pi = _mock_pi(ct_payment_id="pay-stale") + charge = _mock_charge() + mock_stripe.PaymentIntent.retrieve.return_value = pi + mock_stripe.Charge.retrieve.return_value = charge + + payment = _mock_payment(payment_id="pay-by-key", has_charge=True, charge_id="ch_test456") + existing_order = gen_order(uuid4_str()) + + client = MockClient.return_value + client.base_client.payments.get_by_id.side_effect = _ct_error("ResourceNotFound") + client.get_payment_by_key.return_value = payment + client.get_order_by_payment_id.return_value = existing_order + client.update_line_items_transition_state.return_value = existing_order + _stub_initial_matching_order(client, existing_order) + + result = finalize_ct_order_from_stripe_pi("pi_test123", source="webhook") + + client.get_payment_by_key.assert_called_once_with("pi_test123") + self.assertTrue(result.already_existed) + + def test_ct_payment_id_transient_error_propagates( + self, MockClient, mock_track, mock_stripe, _mock_lock, _mock_unlock + ): + """Non-not-found CommercetoolsError on ct_payment_id lookup must not fall back.""" + pi = _mock_pi(ct_payment_id="pay-from-meta") + mock_stripe.PaymentIntent.retrieve.return_value = pi + + client = MockClient.return_value + client.base_client.payments.get_by_id.side_effect = _ct_error("ConcurrentModification") + + with self.assertRaises(CommercetoolsError): + finalize_ct_order_from_stripe_pi("pi_test123", source="webhook") + + client.get_payment_by_key.assert_not_called() + + def test_segment_event_has_web_properties( + self, MockClient, mock_track, mock_stripe, _mock_lock, _mock_unlock + ): + """Segment Order Completed should have is_mobile=False, plan 18, payment_method=upi.""" + pi = _mock_pi() + charge = _mock_charge() + mock_stripe.PaymentIntent.retrieve.return_value = pi + mock_stripe.Charge.retrieve.return_value = charge + + payment = _mock_payment(payment_id="pay-123") + order = gen_order(uuid4_str()) + cart = gen_cart(cart_id="cart-uuid", customer_id=order.customer_id) + customer = gen_customer("test@example.com", "testuser") + + client = MockClient.return_value + client.get_payment_by_key.return_value = payment + client.create_charge_payment_transaction.return_value = payment + client.get_order_by_payment_id.side_effect = ValueError("not found") + client.get_cart_by_id.return_value = cart + client.create_order_from_cart.return_value = order + client.update_line_items_transition_state.return_value = order + client.get_customer_by_id.return_value = customer + _stub_initial_matching_order(client, order) + + finalize_ct_order_from_stripe_pi("pi_test123", source="webhook") + + call_kwargs = mock_track.call_args + props = call_kwargs[1]["properties"] if "properties" in call_kwargs[1] else call_kwargs[0][2] + self.assertFalse(props["is_mobile"]) + self.assertEqual(props["track_plan_id"], 18) + self.assertEqual(props["trigger_source"], "server-side") + self.assertEqual(props["processor_name"], "stripe") + self.assertEqual(props["payment_method"], "upi") diff --git a/commerce_coordinator/apps/stripe/exceptions.py b/commerce_coordinator/apps/stripe/exceptions.py index 54fdd27f0..8c46aff4a 100644 --- a/commerce_coordinator/apps/stripe/exceptions.py +++ b/commerce_coordinator/apps/stripe/exceptions.py @@ -21,6 +21,12 @@ class UnhandledStripeEventAPIError(APIException): default_code = 'unhandled_stripe_event' +class StripeWebhookDispatchAPIError(APIException): + status_code = 503 + default_detail = 'Failed to enqueue Stripe webhook handler.' + default_code = 'stripe_webhook_dispatch_error' + + class StripeIntentCreateAPIError(APIException): status_code = 502 default_detail = 'Error while creating payment intent on payment gateway.' diff --git a/commerce_coordinator/apps/stripe/signals.py b/commerce_coordinator/apps/stripe/signals.py index 453a95124..9d230ea9c 100644 --- a/commerce_coordinator/apps/stripe/signals.py +++ b/commerce_coordinator/apps/stripe/signals.py @@ -5,3 +5,4 @@ payment_processed_signal = CoordinatorSignal() payment_refunded_signal = CoordinatorSignal() +payment_succeeded_commercetools_signal = CoordinatorSignal() diff --git a/commerce_coordinator/apps/stripe/tests/test_views.py b/commerce_coordinator/apps/stripe/tests/test_views.py index d5476d843..2bd9e434c 100644 --- a/commerce_coordinator/apps/stripe/tests/test_views.py +++ b/commerce_coordinator/apps/stripe/tests/test_views.py @@ -79,6 +79,171 @@ def test_stripe_signature_verification_error(self): ) ) + @mock.patch('stripe.Webhook.construct_event') + @mock.patch('commerce_coordinator.apps.stripe.views.payment_succeeded_commercetools_signal.send_robust') + def test_ct_payment_succeeded_fires_signal(self, mock_ct_signal, mock_construct_event): + """ + Verify payment_succeeded_commercetools_signal is emitted for + payment_intent.succeeded with source_system=commercetools. + """ + pi_id = 'pi_ct_test_123' + self.mock_stripe_event.type = StripeEventType.PAYMENT_SUCCESS.value + metadata = {'source_system': 'commercetools', 'ct_cart_id': 'cart-uuid'} + self.mock_stripe_event.data.object.id = pi_id + self.mock_stripe_event.data.object.metadata = StripeObject() + self.mock_stripe_event.data.object.metadata.update(metadata) + self.mock_stripe_event.data.object.amount = 4900 + mock_construct_event.return_value = self.mock_stripe_event + mock_ct_signal.return_value = [(lambda **kwargs: None, 'celery-task-id')] + + response = self.client.post( + self.url, data={}, format='json', **self.mock_header + ) + + self.assertEqual(response.status_code, status.HTTP_200_OK) + mock_ct_signal.assert_called_once_with( + sender=WebhookView, + payment_intent_id=pi_id, + ) + + @mock.patch('stripe.Webhook.construct_event') + @mock.patch('commerce_coordinator.apps.stripe.views.payment_succeeded_commercetools_signal.send_robust') + def test_ct_payment_succeeded_dispatch_failure_returns_503_and_clears_running( + self, mock_ct_signal, mock_construct_event + ): + """ + A failed send_robust must not ACK Stripe or leave the SingleInvocation + flag set; otherwise Stripe will not retry and duplicates are suppressed. + """ + pi_id = 'pi_ct_broker_down' + + def _receiver(**kwargs): + pass + + self.mock_stripe_event.type = StripeEventType.PAYMENT_SUCCESS.value + metadata = {'source_system': 'commercetools', 'ct_cart_id': 'cart-uuid'} + self.mock_stripe_event.data.object.id = pi_id + self.mock_stripe_event.data.object.metadata = StripeObject() + self.mock_stripe_event.data.object.metadata.update(metadata) + self.mock_stripe_event.data.object.amount = 4900 + mock_construct_event.return_value = self.mock_stripe_event + mock_ct_signal.return_value = [(_receiver, RuntimeError('Celery broker down'))] + + response = self.client.post( + self.url, data={}, format='json', **self.mock_header + ) + + self.assertEqual(response.status_code, status.HTTP_503_SERVICE_UNAVAILABLE) + self.assertFalse( + WebhookView._is_running(WebhookView.__name__, pi_id) # pylint: disable=protected-access + ) + + @mock.patch('stripe.Webhook.construct_event') + @mock.patch('commerce_coordinator.apps.stripe.views.payment_succeeded_commercetools_signal.send_robust') + @mock.patch.object(WebhookView, '_is_running', return_value=True) + def test_ct_payment_succeeded_single_invocation_short_circuits( + self, mock_is_running, mock_ct_signal, mock_construct_event + ): + """Duplicate CT success delivery short-circuits via SingleInvocation.""" + pi_id = 'pi_ct_dup' + self.mock_stripe_event.type = StripeEventType.PAYMENT_SUCCESS.value + metadata = {'source_system': 'commercetools', 'ct_cart_id': 'cart-uuid'} + self.mock_stripe_event.data.object.id = pi_id + self.mock_stripe_event.data.object.metadata = StripeObject() + self.mock_stripe_event.data.object.metadata.update(metadata) + self.mock_stripe_event.data.object.amount = 4900 + mock_construct_event.return_value = self.mock_stripe_event + + response = self.client.post( + self.url, data={}, format='json', **self.mock_header + ) + + self.assertEqual(response.status_code, status.HTTP_200_OK) + mock_is_running.assert_called() + mock_ct_signal.assert_not_called() + + @mock.patch('stripe.Webhook.construct_event') + @mock.patch('commerce_coordinator.apps.stripe.views.payment_succeeded_commercetools_signal.send_robust') + def test_ct_payment_failed_returns_200_no_signal(self, mock_ct_signal, mock_construct_event): + """ + Verify payment_intent.payment_failed with source_system=commercetools + returns 200 but does NOT fire the CT succeeded signal. + """ + self.mock_stripe_event.type = StripeEventType.PAYMENT_FAILED.value + metadata = {'source_system': 'commercetools', 'ct_cart_id': 'cart-uuid'} + self.mock_stripe_event.data.object.id = 'pi_ct_fail' + self.mock_stripe_event.data.object.metadata = StripeObject() + self.mock_stripe_event.data.object.metadata.update(metadata) + self.mock_stripe_event.data.object.amount = 4900 + mock_construct_event.return_value = self.mock_stripe_event + + response = self.client.post( + self.url, data={}, format='json', **self.mock_header + ) + + self.assertEqual(response.status_code, status.HTTP_200_OK) + mock_ct_signal.assert_not_called() + + @mock.patch('stripe.Webhook.construct_event') + @mock.patch('commerce_coordinator.apps.stripe.views.payment_processed_signal.send_robust') + @mock.patch('commerce_coordinator.apps.stripe.views.payment_succeeded_commercetools_signal.send_robust') + def test_legacy_payment_succeeded_fires_processed_signal( + self, mock_ct_signal, mock_processed_signal, mock_construct_event + ): + """ + Verify legacy source_system still fires payment_processed_signal, + not the CT signal. + """ + pi_id = 'pi_legacy' + source_system = settings.PAYMENT_PROCESSOR_CONFIG['edx']['stripe']['source_system_identifier'] + self.mock_stripe_event.type = StripeEventType.PAYMENT_SUCCESS.value + metadata = { + 'source_system': source_system, + 'edx_lms_user_id': '123', + 'order_number': 'EDX-000001', + 'payment_number': 'PAY-001', + } + self.mock_stripe_event.data.object.id = pi_id + self.mock_stripe_event.data.object.metadata = StripeObject() + self.mock_stripe_event.data.object.metadata.update(metadata) + self.mock_stripe_event.data.object.amount = 4900 + self.mock_stripe_event.data.object.currency = 'usd' + mock_construct_event.return_value = self.mock_stripe_event + + response = self.client.post( + self.url, data={}, format='json', **self.mock_header + ) + + self.assertEqual(response.status_code, status.HTTP_200_OK) + mock_ct_signal.assert_not_called() + mock_processed_signal.assert_called_once() + + @mock.patch('stripe.Webhook.construct_event') + @mock.patch('commerce_coordinator.apps.stripe.views.payment_processed_signal.send_robust') + @mock.patch('commerce_coordinator.apps.stripe.views.payment_succeeded_commercetools_signal.send_robust') + def test_unknown_source_system_skips_both_signals( + self, mock_ct_signal, mock_processed_signal, mock_construct_event + ): + """ + Verify that an unrecognized source_system returns 200 + but does not fire any signal. + """ + self.mock_stripe_event.type = StripeEventType.PAYMENT_SUCCESS.value + metadata = {'source_system': 'unknown_system'} + self.mock_stripe_event.data.object.id = 'pi_unknown' + self.mock_stripe_event.data.object.metadata = StripeObject() + self.mock_stripe_event.data.object.metadata.update(metadata) + self.mock_stripe_event.data.object.amount = 4900 + mock_construct_event.return_value = self.mock_stripe_event + + response = self.client.post( + self.url, data={}, format='json', **self.mock_header + ) + + self.assertEqual(response.status_code, status.HTTP_200_OK) + mock_ct_signal.assert_not_called() + mock_processed_signal.assert_not_called() + @ddt.data( name_test( "Test 2U order refund and correct source_system", @@ -151,3 +316,56 @@ def test_payment_refunded_event( ) else: mock_refund_task.assert_not_called() + + @mock.patch('stripe.Webhook.construct_event') + @mock.patch('commerce_coordinator.apps.stripe.views.payment_refunded_signal.send_robust') + @mock.patch('commerce_coordinator.apps.stripe.views.is_legacy_order', return_value=False) + @mock.patch('commerce_coordinator.apps.stripe.views.is_commercetools_stripe_refund', return_value=True) + @mock.patch.object(WebhookView, 'mark_running') + @mock.patch.object(WebhookView, '_is_running', return_value=False) + def test_refund_falls_back_to_event_id_when_idempotency_key_missing( + self, + mock_is_running, + mock_mark_running, + mock_is_ct_refund, + mock_is_legacy, + mock_refund_task, + mock_construct_event, + ): + """Null Stripe request.idempotency_key must not become the SingleInvocation key.""" + payment_intent_id = 'pi_refund_no_idem' + refund_data = { + 'id': "re_missing_idem", + 'amount': 1000, + 'charge': "ch_missing_idem", + 'created': 1692942318, + 'currency': "usd", + 'payment_intent': payment_intent_id, + 'status': "succeeded", + } + event_id = 'evt_refund_123' + source_system = settings.PAYMENT_PROCESSOR_CONFIG['edx']['stripe']['source_system_identifier'] + self.mock_stripe_event.type = StripeEventType.PAYMENT_REFUNDED.value + self.mock_stripe_event.id = event_id + self.mock_stripe_event.get.side_effect = lambda key, default=None: { + 'request': {'idempotency_key': None}, + 'id': event_id, + }.get(key, default) + self.mock_stripe_event.data.object.payment_intent = payment_intent_id + self.mock_stripe_event.data.object.refunds.data = [refund_data] + metadata = { + 'order_number': '2U-123456', + 'source_system': source_system, + } + self.mock_stripe_event.data.object.metadata = StripeObject() + self.mock_stripe_event.data.object.metadata.update(metadata) + mock_construct_event.return_value = self.mock_stripe_event + + response = self.client.post(self.url, data={}, format='json', **self.mock_header) + + self.assertEqual(response.status_code, status.HTTP_200_OK) + mock_is_running.assert_called_with(WebhookView.__name__, event_id) + mock_mark_running.assert_called_with(WebhookView.__name__, event_id) + mock_refund_task.assert_called_once() + mock_is_ct_refund.assert_called() + mock_is_legacy.assert_called() diff --git a/commerce_coordinator/apps/stripe/views.py b/commerce_coordinator/apps/stripe/views.py index 33a778d2f..034607461 100644 --- a/commerce_coordinator/apps/stripe/views.py +++ b/commerce_coordinator/apps/stripe/views.py @@ -11,15 +11,21 @@ from rest_framework.response import Response from commerce_coordinator.apps.core.constants import PaymentState +from commerce_coordinator.apps.core.signal_helpers import format_signal_results from commerce_coordinator.apps.core.views import SingleInvocationAPIView from commerce_coordinator.apps.rollout.utils import is_commercetools_stripe_refund, is_legacy_order from commerce_coordinator.apps.stripe.constants import StripeEventType from commerce_coordinator.apps.stripe.exceptions import ( InvalidPayloadAPIError, SignatureVerificationAPIError, + StripeWebhookDispatchAPIError, UnhandledStripeEventAPIError ) -from commerce_coordinator.apps.stripe.signals import payment_processed_signal, payment_refunded_signal +from commerce_coordinator.apps.stripe.signals import ( + payment_processed_signal, + payment_refunded_signal, + payment_succeeded_commercetools_signal +) logger = logging.getLogger(__name__) @@ -61,73 +67,79 @@ def post(self, request): raise SignatureVerificationAPIError from e # Handle the event - if event.type == StripeEventType.PAYMENT_SUCCESS: - payment_state = PaymentState.COMPLETED.value - elif event.type == StripeEventType.PAYMENT_FAILED: - payment_state = PaymentState.FAILED.value - elif event.type == StripeEventType.PAYMENT_REFUNDED: - idempotency_key = event.get('request').get('idempotency_key') - if self._is_running(tag, idempotency_key): # pragma no cover - self.meta_should_mark_not_running = False - return Response(status=status.HTTP_200_OK) - else: - self.mark_running(tag, idempotency_key) - - event_object = event.data.object - order_number = event_object.metadata.order_number - is_legacy_order_check = is_legacy_order(order_number) - is_ct_order_check = is_commercetools_stripe_refund(event_object.metadata.get('source_system')) - payment_intent_id = event_object.payment_intent - - if not is_legacy_order_check and is_ct_order_check: - event_source_system_identifier = event_object.metadata.get('source_system') - refunds = event_object.refunds.data - latest_refund = max(refunds, key=lambda refund: refund['created']) - - logger.info( - '[Stripe webhooks] refund event %s with payment intent ID [%s] ' - 'and order number [%s], source: [%s].', - event.type, - payment_intent_id, - order_number, - event_source_system_identifier, - ) - - payment_refunded_signal.send_robust( - sender=self.__class__, - payment_intent_id=payment_intent_id, - stripe_refund=latest_refund, - order_number=order_number, - ) - else: - logger.info( - '[Stripe webhooks] skipping refund event %s with payment intent ID [%s] ' - 'and order number [%s], as it is not a Commercetools order.', - event.type, - payment_intent_id, - order_number, - ) + if event.type in (StripeEventType.PAYMENT_SUCCESS.value, StripeEventType.PAYMENT_FAILED.value): + payment_intent = event.data.object + event_source_system = payment_intent.metadata.get('source_system') + + if event_source_system == 'commercetools': + return self._handle_commercetools_payment_event(tag, event, payment_intent) + + return self._handle_legacy_payment_event(event, payment_intent, event_source_system, payload) + + if event.type == StripeEventType.PAYMENT_REFUNDED.value: + return self._handle_refund_event(tag, event) + + raise UnhandledStripeEventAPIError + + def _handle_commercetools_payment_event(self, tag, event, payment_intent): + """Route CommerceTools-originated PaymentIntents (UPI) to the async finalize path.""" + if event.type != StripeEventType.PAYMENT_SUCCESS.value: + logger.info( + '[Stripe webhooks] CT payment_intent.payment_failed for PI [%s], ignoring', + payment_intent.id, + ) return Response(status=status.HTTP_200_OK) - else: - raise UnhandledStripeEventAPIError - payment_intent = event.data.object + if self._is_running(tag, payment_intent.id): # pragma no cover + self.meta_should_mark_not_running = False + return Response(status=status.HTTP_200_OK) + + self.mark_running(tag, payment_intent.id) + + logger.info( + '[Stripe webhooks] CT payment_intent.succeeded for PI [%s]', + payment_intent.id, + ) + + results = payment_succeeded_commercetools_signal.send_robust( + sender=self.__class__, + payment_intent_id=payment_intent.id, + ) + self._assert_signal_dispatched(results, payment_intent_id=payment_intent.id) + return Response(status=status.HTTP_200_OK) + + def _assert_signal_dispatched(self, results, *, payment_intent_id): + """Raise so Stripe retries if enqueue failed; handle_exception clears the running flag.""" + formatted = format_signal_results(results) + if not results or any(entry["error"] for entry in formatted.values()): + logger.error( + '[Stripe webhooks] Failed to enqueue CT finalize for PI [%s]: %s', + payment_intent_id, + formatted, + ) + raise StripeWebhookDispatchAPIError + + def _handle_legacy_payment_event(self, event, payment_intent, event_source_system, payload): + """Route legacy edX ecommerce PaymentIntents to the existing processed signal.""" + if event.type == StripeEventType.PAYMENT_SUCCESS.value: + payment_state = PaymentState.COMPLETED.value + else: + payment_state = PaymentState.FAILED.value - event_source_system_identifier = payment_intent.metadata.get('source_system') logger.info( '[Stripe webhooks] event %s with amount %d and payment intent ID [%s], source: [%s].', event.type, payment_intent.amount, payment_intent.id, - event_source_system_identifier, + event_source_system, ) - if event_source_system_identifier != source_system_identifier: + if event_source_system != source_system_identifier: logger.info( '[Stripe webhooks] Skipping event %s with payment intent ID [%s], source: [%s].', event.type, payment_intent.id, - event_source_system_identifier, + event_source_system, ) return Response(status=status.HTTP_200_OK) @@ -143,3 +155,52 @@ def post(self, request): provider_response_body=payload, ) return Response(status=status.HTTP_200_OK) + + def _handle_refund_event(self, tag, event): + """Route Commercetools refunds to the refund signal, skipping legacy orders.""" + request = event.get('request') or {} + idempotency_key = request.get('idempotency_key') if hasattr(request, 'get') else None + # Stripe request.idempotency_key can be null; fall back to event.id so + # unrelated refunds do not collide on a shared None cache key. + invocation_key = idempotency_key or event.get('id') or getattr(event, 'id', None) + if self._is_running(tag, invocation_key): # pragma no cover + self.meta_should_mark_not_running = False + return Response(status=status.HTTP_200_OK) + + self.mark_running(tag, invocation_key) + + event_object = event.data.object + order_number = event_object.metadata.order_number + is_legacy_order_check = is_legacy_order(order_number) + is_ct_order_check = is_commercetools_stripe_refund(event_object.metadata.get('source_system')) + payment_intent_id = event_object.payment_intent + + if not is_legacy_order_check and is_ct_order_check: + event_source_system_identifier = event_object.metadata.get('source_system') + refunds = event_object.refunds.data + latest_refund = max(refunds, key=lambda refund: refund['created']) + + logger.info( + '[Stripe webhooks] refund event %s with payment intent ID [%s] ' + 'and order number [%s], source: [%s].', + event.type, + payment_intent_id, + order_number, + event_source_system_identifier, + ) + + payment_refunded_signal.send_robust( + sender=self.__class__, + payment_intent_id=payment_intent_id, + stripe_refund=latest_refund, + order_number=order_number, + ) + else: + logger.info( + '[Stripe webhooks] skipping refund event %s with payment intent ID [%s] ' + 'and order number [%s], as it is not a Commercetools order.', + event.type, + payment_intent_id, + order_number, + ) + return Response(status=status.HTTP_200_OK) diff --git a/commerce_coordinator/settings/base.py b/commerce_coordinator/settings/base.py index 4efb3d31f..835b1cb4d 100644 --- a/commerce_coordinator/settings/base.py +++ b/commerce_coordinator/settings/base.py @@ -338,6 +338,9 @@ def root(*path_fragments): "commerce_coordinator.apps.iap.signals.revoke_line_mobile_order_signal": [ "commerce_coordinator.apps.commercetools.signals.revoke_line_mobile_order", ], + "commerce_coordinator.apps.stripe.signals.payment_succeeded_commercetools_signal": [ + "commerce_coordinator.apps.commercetools.signals.finalize_commercetools_stripe_payment", + ], } # Default timeouts for requests