incorporate pr comments

Author: suejung-sentry

billing/views.py

@@ -92,7 +92,7 @@ def invoice_payment_failed(self, invoice: stripe.Invoice) -> None:
             if invoice.payment_intent:
                 payment_intent = stripe.PaymentIntent.retrieve(invoice.payment_intent)
                 if (
-                    payment_intent is not None
+                    payment_intent
                     and payment_intent.get("status") == "requires_action"
                     and payment_intent.get("next_action", {}).get("type")
                     == "verify_with_microdeposits"
@@ -129,12 +129,12 @@ def invoice_payment_failed(self, invoice: stripe.Invoice) -> None:
         payment_intent = stripe.PaymentIntent.retrieve(
             invoice["payment_intent"], expand=["payment_method"]
         )
-        card = (
-            payment_intent.get("payment_method", {}).get("card")
-            if payment_intent.get("payment_method")
-            and not isinstance(payment_intent.get("payment_method"), str)
-            else None
-        )
+
+        try:
+            card = payment_intent.payment_method.card
+        except AttributeError:
+            card = None
+
         template_vars = {
             "amount": invoice.total / 100,
             "card_type": card.brand if card else None,
@@ -374,10 +374,10 @@ def _has_unverified_initial_payment_method(
                 latest_invoice.payment_intent
             )
             return (
-                payment_intent is not None
-                and payment_intent.status == "requires_action"
-                and payment_intent.next_action is not None
-                and payment_intent.next_action.get("type")
+                payment_intent
+                and payment_intent.get("status") == "requires_action"
+                and payment_intent.get("next_action")
+                and payment_intent.get("next_action", {}).get("type")
                 == "verify_with_microdeposits"
             )
         return False

services/billing.py

@@ -556,19 +556,22 @@ def _is_unverified_payment_method(self, payment_method_id: str) -> bool:
             setup_intents = stripe.SetupIntent.list(
                 payment_method=payment_method_id, limit=1
             )
-            if (
-                setup_intents
-                and hasattr(setup_intents, "data")
-                and isinstance(setup_intents.data, list)
-                and len(setup_intents.data) > 0
-            ):
+
+            try:
                 latest_intent = setup_intents.data[0]
                 if (
                     latest_intent.status == "requires_action"
                     and latest_intent.next_action
                     and latest_intent.next_action.type == "verify_with_microdeposits"
                 ):
                     return True
+            except Exception as e:
+                log.error(
+                    "Error retrieving latest setup intent",
+                    payment_method_id=payment_method_id,
+                    extra=dict(error=e),
+                )
+                return False
 
         return False
 
@@ -987,21 +990,23 @@ def create_setup_intent(self, owner: Owner):
     def _cleanup_incomplete_subscription(
         self, subscription: stripe.Subscription, owner: Owner
     ):
-        latest_invoice = subscription.get("latest_invoice")
-        if not latest_invoice:
-            return None
-        payment_intent_id = latest_invoice.get("payment_intent")
-        if not payment_intent_id:
+        try:
+            payment_intent_id = subscription.latest_invoice.payment_intent
+        except Exception as e:
+            log.error(
+                "Latest invoice is missing payment intent id",
+                extra=dict(error=e),
+            )
             return None
 
         payment_intent = stripe.PaymentIntent.retrieve(payment_intent_id)
         if payment_intent.status == "requires_action":
             log.info(
                 "Subscription has pending payment verification",
                 extra=dict(
-                    subscription_id=subscription.id,
-                    payment_intent_id=payment_intent.id,
-                    payment_intent_status=payment_intent.status,
+                    subscription_id=subscription.get("id"),
+                    payment_intent_id=payment_intent.get("id"),
+                    payment_intent_status=payment_intent.get("status"),
                 ),
             )
             try:
@@ -1011,16 +1016,16 @@ def _cleanup_incomplete_subscription(
                 log.info(
                     "Deleted incomplete subscription",
                     extra=dict(
-                        subscription_id=subscription.id,
-                        payment_intent_id=payment_intent.id,
+                        subscription_id=subscription.get("id"),
+                        payment_intent_id=payment_intent.get("id"),
                     ),
                 )
             except Exception as e:
                 log.error(
                     "Failed to delete subscription",
                     extra=dict(
-                        subscription_id=subscription.id,
-                        payment_intent_id=payment_intent.id,
+                        subscription_id=subscription.get("id"),
+                        payment_intent_id=payment_intent.get("id"),
                         error=str(e),
                     ),
                 )

services/tests/test_billing.py

@@ -2239,11 +2239,9 @@ def test_update_plan_cleans_up_incomplete_subscription_and_creates_new_checkout(
         owner = OwnerFactory(stripe_subscription_id="sub_123")
         desired_plan = {"value": PlanName.CODECOV_PRO_YEARLY.value, "quantity": 10}
 
-        class MockSubscription:
-            def __init__(self):
-                self.status = "incomplete"
-
-        subscription = MockSubscription()
+        subscription = stripe.Subscription.construct_from(
+            {"status": "incomplete"}, "fake_api_key"
+        )
         get_subscription_mock.return_value = subscription
 
         self.billing_service.update_plan(owner, desired_plan)
@@ -2257,21 +2255,14 @@ def __init__(self):
     def test_cleanup_incomplete_subscription(self, delete_mock, retrieve_mock):
         owner = OwnerFactory(stripe_subscription_id="sub_123")
 
-        class MockSubscription:
-            id = "sub_123"
-
-            def get(self, key):
-                if key == "latest_invoice":
-                    return {"payment_intent": "pi_123"}
-                return None
-
-        subscription = MockSubscription()
-
-        class MockPaymentIntent:
-            id = "pi_123"
-            status = "requires_action"
-
-        retrieve_mock.return_value = MockPaymentIntent()
+        payment_intent = stripe.PaymentIntent.construct_from(
+            {"id": "pi_123", "status": "requires_action"}, "fake_api_key"
+        )
+        subscription = stripe.Subscription.construct_from(
+            {"id": "abcd", "latest_invoice": {"payment_intent": "pi_123"}},
+            "fake_api_key",
+        )
+        retrieve_mock.return_value = payment_intent
 
         self.billing_service._cleanup_incomplete_subscription(subscription, owner)
 
@@ -2285,13 +2276,9 @@ def test_cleanup_incomplete_subscription_no_latest_invoice(
     ):
         owner = OwnerFactory(stripe_subscription_id="sub_123")
 
-        class MockSubscription:
-            id = "sub_123"
-
-            def get(self, key):
-                return None
-
-        subscription = MockSubscription()
+        subscription = stripe.Subscription.construct_from(
+            {"id": "sub_123"}, "fake_api_key"
+        )
 
         result = self.billing_service._cleanup_incomplete_subscription(
             subscription, owner
@@ -2335,21 +2322,14 @@ def test_cleanup_incomplete_subscription_delete_fails(
     ):
         owner = OwnerFactory(stripe_subscription_id="sub_123")
 
-        class MockSubscription:
-            id = "sub_123"
-
-            def get(self, key):
-                if key == "latest_invoice":
-                    return {"payment_intent": "pi_123"}
-                return None
-
-        subscription = MockSubscription()
-
-        class MockPaymentIntent:
-            id = "pi_123"
-            status = "requires_action"
-
-        retrieve_mock.return_value = MockPaymentIntent()
+        payment_intent = stripe.PaymentIntent.construct_from(
+            {"id": "pi_123", "status": "requires_action"}, "fake_api_key"
+        )
+        subscription = stripe.Subscription.construct_from(
+            {"id": "abcd", "latest_invoice": {"payment_intent": "pi_123"}},
+            "fake_api_key",
+        )
+        retrieve_mock.return_value = payment_intent
         delete_mock.side_effect = Exception("Delete failed")
 
         result = self.billing_service._cleanup_incomplete_subscription(