diff --git a/commerce_coordinator/apps/commercetools/stripe_payment_finalize.py b/commerce_coordinator/apps/commercetools/stripe_payment_finalize.py index 21c884143..759f4e9cb 100644 --- a/commerce_coordinator/apps/commercetools/stripe_payment_finalize.py +++ b/commerce_coordinator/apps/commercetools/stripe_payment_finalize.py @@ -160,6 +160,24 @@ def _ensure_pending_fulfilment(client, order): ) +def _heal_existing_order(client, order, payment_intent_id, payment, metadata): + """Heal fulfillment and PI metadata without re-emitting analytics.""" + _ensure_pending_fulfilment(client, order) + if not metadata.get("order_id") or metadata.get("ct_payment_id") != payment.id: + _backfill_pi_metadata( + payment_intent_id, + order.id, + payment.id, + existing_metadata=metadata, + ) + return FinalizeResult( + order_id=order.id, + order_number=order.order_number or "", + payment_id=payment.id, + already_existed=True, + ) + + def finalize_ct_order_from_stripe_pi( payment_intent_id: str, *, @@ -300,25 +318,28 @@ def _finalize_ct_order_from_stripe_pi_locked( "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, + return _heal_existing_order( + client, existing_order, payment_intent_id, payment, metadata, ) # --- Load cart and create order --- cart = client.get_cart_by_id(ct_cart_id) - order = client.create_order_from_cart(cart) + try: + order = client.create_order_from_cart(cart) + except CommercetoolsError as create_error: + try: + existing_order = client.get_order_by_payment_id(payment.id) + except ValueError: + existing_order = None + if existing_order is None: + raise create_error + logger.info( + "[finalize_ct_order] Order %s appeared after create failed for payment %s; healing", + existing_order.id, payment.id, + ) + return _heal_existing_order( + client, existing_order, payment_intent_id, payment, metadata, + ) # --- Transition line items → PENDING_FULFILMENT --- order = _ensure_pending_fulfilment(client, order) diff --git a/commerce_coordinator/apps/commercetools/tests/test_stripe_payment_finalize.py b/commerce_coordinator/apps/commercetools/tests/test_stripe_payment_finalize.py index d2ce45a11..8afdc0d53 100644 --- a/commerce_coordinator/apps/commercetools/tests/test_stripe_payment_finalize.py +++ b/commerce_coordinator/apps/commercetools/tests/test_stripe_payment_finalize.py @@ -307,6 +307,87 @@ def test_order_already_exists_skips( client.create_order_from_cart.assert_not_called() mock_track.assert_not_called() + def test_create_failure_heals_when_order_appears_on_requery( + self, MockClient, mock_track, mock_stripe, _mock_lock, _mock_unlock + ): + """A concurrent writer winning cart conversion is healed by authoritative re-query.""" + 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()) + cart = gen_cart(cart_id="cart-uuid", customer_id=existing_order.customer_id) + + client = MockClient.return_value + client.get_payment_by_key.return_value = payment + client.get_order_by_payment_id.side_effect = [ValueError("not found"), existing_order] + client.get_cart_by_id.return_value = cart + client.create_order_from_cart.side_effect = _ct_error("UnverifiedAlreadyOrderedShape") + 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) + self.assertEqual(client.get_order_by_payment_id.call_count, 2) + mock_track.assert_not_called() + + def test_create_failure_reraises_original_when_requery_is_empty( + self, MockClient, mock_track, mock_stripe, _mock_lock, _mock_unlock + ): + """Unrelated create failures retain retry/quarantine behavior when no order exists.""" + 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") + cart = gen_cart(cart_id="cart-uuid") + create_error = _ct_error("ConcurrentModification") + + 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.side_effect = create_error + + with self.assertRaises(CommercetoolsError) as ctx: + finalize_ct_order_from_stripe_pi("pi_test123", source="webhook") + + self.assertIs(ctx.exception, create_error) + self.assertEqual(client.get_order_by_payment_id.call_count, 2) + mock_track.assert_not_called() + + def test_create_failure_propagates_requery_commercetools_error( + self, MockClient, mock_track, mock_stripe, _mock_lock, _mock_unlock + ): + """A failed authoritative re-query must drive the Celery retry.""" + 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") + cart = gen_cart(cart_id="cart-uuid") + create_error = _ct_error("InvalidOperation") + lookup_error = _ct_error("ConcurrentModification") + + client = MockClient.return_value + client.get_payment_by_key.return_value = payment + client.get_order_by_payment_id.side_effect = [ValueError("not found"), lookup_error] + client.get_cart_by_id.return_value = cart + client.create_order_from_cart.side_effect = create_error + + with self.assertRaises(CommercetoolsError) as ctx: + finalize_ct_order_from_stripe_pi("pi_test123", source="webhook") + + self.assertIs(ctx.exception, lookup_error) + self.assertEqual(client.get_order_by_payment_id.call_count, 2) + mock_track.assert_not_called() + def test_charge_already_present_skips_creation( self, MockClient, mock_track, mock_stripe, _mock_lock, _mock_unlock ):