Improve stripe tests

api/src/shop/pay.py

@@ -18,6 +18,7 @@
     start_subscription,
 )
 from shop.stripe_customer import get_and_sync_stripe_customer
+from shop.stripe_util import retry
 from service.db import db_session
 from core import auth
 from membership.views import member_entity
@@ -168,18 +169,20 @@ def setup_payment_method(data_dict: Any, member_id: int) -> SetupPaymentMethodRe
 
     if data.setup_intent_id is None:
         try:
-            payment_method = stripe.PaymentMethod.retrieve(data.stripe_payment_method_id)
+            payment_method = retry(lambda: stripe.PaymentMethod.retrieve(data.stripe_payment_method_id))
         except:
             raise BadRequest(message="The payment method is not valid.")
 
-        setup_intent = stripe.SetupIntent.create(
-            payment_method_types=["card"],
-            metadata={},
-            payment_method=payment_method.stripe_id,
-            customer=stripe_customer.stripe_id,
+        setup_intent = retry(
+            lambda: stripe.SetupIntent.create(
+                payment_method_types=["card"],
+                metadata={},
+                payment_method=payment_method.stripe_id,
+                customer=stripe_customer.stripe_id,
+            )
         )
     else:
-        setup_intent = stripe.SetupIntent.retrieve(data.setup_intent_id)
+        setup_intent = retry(lambda: stripe.SetupIntent.retrieve(data.setup_intent_id))
 
     try:
         handle_setup_intent(setup_intent)
@@ -258,7 +261,7 @@ def cleanup_pending_members(relevant_email: str) -> None:
         # We delete the customer just to keep things tidy. It's not strictly necessary.
         if member.stripe_customer_id is not None:
             try:
-                stripe.Customer.delete(member.stripe_customer_id)
+                retry(lambda: stripe.Customer.delete(member.stripe_customer_id))
             except:
                 # If it cannot be deleted, we don't care
                 pass

api/src/shop/stripe_charge.py

@@ -6,7 +6,7 @@
 from service.error import InternalServerError, EXCEPTION
 from shop.models import Transaction
 from shop.stripe_constants import CURRENCY, ChargeStatus
-from shop.stripe_util import convert_to_stripe_amount
+from shop.stripe_util import convert_to_stripe_amount, retry
 from shop.transactions import PaymentFailed, payment_success
 
 
@@ -30,11 +30,13 @@ def create_stripe_charge(transaction: Transaction, card_source_id) -> stripe.Cha
     stripe_amount = convert_to_stripe_amount(transaction.amount)
 
     try:
-        return stripe.Charge.create(
-            amount=stripe_amount,
-            currency=CURRENCY,
-            description=f"charge for transaction id {transaction.id}",
-            source=card_source_id,
+        return retry(
+            lambda: stripe.Charge.create(
+                amount=stripe_amount,
+                currency=CURRENCY,
+                description=f"charge for transaction id {transaction.id}",
+                source=card_source_id,
+            )
         )
     except InvalidRequestError as e:
         raise_from_stripe_invalid_request_error(e)

api/src/shop/stripe_customer.py

@@ -108,7 +108,7 @@ def delete_stripe_customer(member_id: int) -> None:
         raise NotFound(f"Unable to find member with id {member_id}")
     if member.stripe_customer_id is not None:
         # Note: This will also delete all subscriptions
-        stripe.Customer.delete(member.stripe_customer_id)
+        retry(lambda: stripe.Customer.delete(member.stripe_customer_id))
 
     member.stripe_customer_id = None
     db_session.flush()

api/src/shop/stripe_event.py

@@ -10,7 +10,7 @@
 from datetime import timezone
 from shop import stripe_subscriptions
 import shop.transactions
-from shop.stripe_util import event_semantic_time
+from shop.stripe_util import event_semantic_time, retry
 
 from service.error import BadRequest, InternalServerError
 from shop.models import (
@@ -223,17 +223,21 @@ def stripe_invoice_event(subtype: EventSubtype, event: stripe.Event, current_tim
             # Attach a makerspace transaction id to the stripe invoice item.
             # This is nice to have in the future if we need to match them somehow.
             # In the vast majority of all cases, this will just contain a single transaction id.
-            stripe.Invoice.modify(
-                invoice["id"],
-                metadata={
-                    MakerspaceMetadataKeys.TRANSACTION_IDS.value: ",".join([str(x) for x in transaction_ids]),
-                },
+            retry(
+                lambda: stripe.Invoice.modify(
+                    invoice["id"],
+                    metadata={
+                        MakerspaceMetadataKeys.TRANSACTION_IDS.value: ",".join([str(x) for x in transaction_ids]),
+                    },
+                )
             )
-            stripe.PaymentIntent.modify(
-                invoice["payment_intent"],
-                metadata={
-                    MakerspaceMetadataKeys.TRANSACTION_IDS.value: ",".join([str(x) for x in transaction_ids]),
-                },
+            retry(
+                lambda: stripe.PaymentIntent.modify(
+                    invoice["payment_intent"],
+                    metadata={
+                        MakerspaceMetadataKeys.TRANSACTION_IDS.value: ",".join([str(x) for x in transaction_ids]),
+                    },
+                )
             )
 
         return

api/src/shop/stripe_payment_intent.py

@@ -20,7 +20,7 @@
     CURRENCY,
     SetupFutureUsage,
 )
-from shop.stripe_util import convert_to_stripe_amount, replace_default_payment_method
+from shop.stripe_util import retry, convert_to_stripe_amount, replace_default_payment_method
 from shop.transactions import (
     PaymentFailed,
     payment_success,
@@ -91,7 +91,7 @@ def create_client_response(transaction: Transaction, payment_intent: PaymentInte
         return create_action_required_response(transaction, payment_intent)
 
     elif status == PaymentIntentStatus.REQUIRES_CONFIRMATION:
-        confirmed_intent = stripe.PaymentIntent.confirm(payment_intent.id)
+        confirmed_intent = retry(lambda: stripe.PaymentIntent.confirm(payment_intent.id))
         assert PaymentIntentStatus(confirmed_intent.status) != PaymentIntentStatus.REQUIRES_CONFIRMATION
         return create_client_response(transaction, confirmed_intent)
 
@@ -118,7 +118,7 @@ def confirm_stripe_payment_intent(transaction_id: int) -> PartialPayment:
     if transaction.status != Transaction.PENDING:
         raise BadRequest(f"transaction ({transaction_id}) is not pending")
 
-    payment_intent = stripe.PaymentIntent.retrieve(pending.stripe_token)
+    payment_intent = retry(lambda: stripe.PaymentIntent.retrieve(pending.stripe_token))
     status = PaymentIntentStatus(payment_intent.status)
     if status == PaymentIntentStatus.CANCELED:
         raise BadRequest(f"unexpected stripe payment intent status {status}")
@@ -160,23 +160,25 @@ def pay_with_stripe(transaction: Transaction, payment_method_id: str, setup_futu
         stripe_customer = get_and_sync_stripe_customer(member)
         assert stripe_customer is not None
 
-        payment_intent = stripe.PaymentIntent.create(
-            payment_method=payment_method_id,
-            amount=convert_to_stripe_amount(transaction.amount),
-            currency=CURRENCY,
-            customer=stripe_customer.stripe_id,
-            description=f"charge for transaction id {transaction.id}",
-            confirmation_method="manual",
-            confirm=True,
-            # One might think that off_session could be set to true to make payments possible without
-            # user interaction. Sadly, it seems that most cards require 3d secure verification, which
-            # is not possible with off_session payments.
-            # Subscriptions may instead email the user to ask them to verify the payment.
-            off_session=False,
-            setup_future_usage=SetupFutureUsage.OFF_SESSION.value if setup_future_usage else None,
-            metadata={
-                MakerspaceMetadataKeys.TRANSACTION_IDS.value: transaction.id,
-            },
+        payment_intent = retry(
+            lambda: stripe.PaymentIntent.create(
+                payment_method=payment_method_id,
+                amount=convert_to_stripe_amount(transaction.amount),
+                currency=CURRENCY,
+                customer=stripe_customer.stripe_id,
+                description=f"charge for transaction id {transaction.id}",
+                confirmation_method="manual",
+                confirm=True,
+                # One might think that off_session could be set to true to make payments possible without
+                # user interaction. Sadly, it seems that most cards require 3d secure verification, which
+                # is not possible with off_session payments.
+                # Subscriptions may instead email the user to ask them to verify the payment.
+                off_session=False,
+                setup_future_usage=SetupFutureUsage.OFF_SESSION.value if setup_future_usage else None,
+                metadata={
+                    MakerspaceMetadataKeys.TRANSACTION_IDS.value: transaction.id,
+                },
+            )
         )
 
         db_session.add(StripePending(transaction_id=transaction.id, stripe_token=payment_intent.stripe_id))

api/src/shop/stripe_product_price.py

@@ -19,14 +19,19 @@
 
 
 def makeradmin_to_stripe_recurring(makeradmin_product: Product, price_type: PriceType) -> StripeRecurring | None:
+    subscription_category_id = get_subscription_category().id
     if price_type == PriceType.RECURRING or price_type == PriceType.BINDING_PERIOD:
+        if makeradmin_product.category.id != subscription_category_id:
+            raise ValueError(f"Unexpected price type {price_type} for non-subscription product {makeradmin_product.id}")
         if makeradmin_product.unit in makeradmin_unit_to_stripe_unit:
             interval = makeradmin_unit_to_stripe_unit[makeradmin_product.unit]
         else:
             raise ValueError(f"Unexpected unit {makeradmin_product.unit} in makeradmin product {makeradmin_product.id}")
         interval_count = makeradmin_product.smallest_multiple if price_type == PriceType.BINDING_PERIOD else 1
         return StripeRecurring(interval=interval, interval_count=interval_count)
     else:
+        if makeradmin_product.category.id == subscription_category_id:
+            raise ValueError(f"Unexpected price type {price_type} for subscription product {makeradmin_product.id}")
         return None
 
 
@@ -61,16 +66,21 @@ def get_stripe_prices(
 
 def eq_makeradmin_stripe_product(makeradmin_product: Product, stripe_product: stripe.Product) -> bool:
     """Check that the essential parts of the product are the same in both makeradmin and stripe"""
-    # TODO check metadata?
-    # TODO how to handle this?
-    # if stripe_product.id != makeradmin_product.id:
-    #    raise BadRequest(f"Stripe product id and makeradmin product id does not match")
+    if stripe_product.id != str(makeradmin_product.id):
+        raise BadRequest(
+            f"Stripe product id {stripe_product.id} and makeradmin product id {makeradmin_product.id} does not match"
+        )
     return stripe_product.name == makeradmin_product.name
 
 
-# TODO use are dicts equal function
 def eq_makeradmin_stripe_price(makeradmin_product: Product, stripe_price: stripe.Price, price_type: PriceType) -> bool:
     """Check that the essential parts of the price are the same in both makeradmin and stripe"""
+    lookup_key_for_product = get_stripe_price_lookup_key(makeradmin_product, price_type)
+    if stripe_price.lookup_key != lookup_key_for_product:
+        raise BadRequest(
+            f"Stripe price lookup key {stripe_price.lookup_key} and corresponding key from makeradmin product {makeradmin_product.id} and PriceType {price_type} does not match"
+        )
+
     recurring = makeradmin_to_stripe_recurring(makeradmin_product, price_type)
     different = []
 
@@ -79,6 +89,9 @@ def eq_makeradmin_stripe_price(makeradmin_product: Product, stripe_price: stripe
             return False
         different.append(stripe_price.recurring.get("interval") != recurring.interval)
         different.append(stripe_price.recurring.get("interval_count") != recurring.interval_count)
+    else:
+        if stripe_price.recurring is not None:
+            return False
     different.append(stripe_price.unit_amount != stripe_amount_from_makeradmin_product(makeradmin_product, recurring))
     different.append(stripe_price.currency != CURRENCY)
     different.append(stripe_price.metadata.get("price_type") != price_type.value)

api/src/shop/stripe_subscriptions.py

@@ -61,15 +61,6 @@ class SubscriptionType(str, Enum):
 
 logger = getLogger("makeradmin")
 
-# Binding period in months.
-# Set to zero to disable binding periods.
-# Setting it to 1 is not particularly useful, since it will be the same as a normal subscription.
-# TODO need to fix this
-BINDING_PERIOD = {
-    SubscriptionType.MEMBERSHIP: 0,
-    SubscriptionType.LAB: 2,
-}
-
 
 SUBSCRIPTION_PRODUCTS: Optional[Dict[SubscriptionType, int]] = None
 
@@ -315,7 +306,7 @@ def resume_paused_subscription(
         # We can just wait for it to start as normal
         return False
 
-    subscription = stripe.Subscription.retrieve(subscription_id)
+    subscription = retry(lambda: stripe.Subscription.retrieve(subscription_id))
     # If the subscription is not paused, we can just do nothing.
     if subscription["pause_collection"] is None:
         return False
@@ -354,12 +345,14 @@ def pause_subscription(
             # that already had membership when they signed up for the subscription.
             return False
         elif subscription_id.startswith("sub_"):
-            stripe.Subscription.modify(
-                subscription_id,
-                pause_collection={
-                    "behavior": "void",
-                    "resumes_at": None,
-                },
+            retry(
+                lambda: stripe.Subscription.modify(
+                    subscription_id,
+                    pause_collection={
+                        "behavior": "void",
+                        "resumes_at": None,
+                    },
+                )
             )
             return True
         else:
@@ -402,14 +395,14 @@ def cancel_subscription(
                 SubscriptionScheduleStatus.NOT_STARTED,
                 SubscriptionScheduleStatus.ACTIVE,
             ]:
-                stripe.SubscriptionSchedule.release(subscription_id)
+                retry(lambda: stripe.SubscriptionSchedule.release(subscription_id))
 
                 if schedule["subscription"]:
                     # Also delete the subscription which the schedule drives, if one exists
-                    stripe.Subscription.delete(schedule["subscription"])
+                    retry(lambda: stripe.Subscription.delete(schedule["subscription"]))
 
         elif subscription_id.startswith("sub_"):
-            stripe.Subscription.delete(subscription_id)
+            retry(lambda: stripe.Subscription.delete(subscription_id))
         else:
             assert False
     except stripe.InvalidRequestError as e:
@@ -502,7 +495,7 @@ def list_subscriptions(member_id: int) -> List[SubscriptionInfo]:
     ]:
         if sub_id is not None:
             if sub_id.startswith("sub_sched_"):
-                sched = stripe.SubscriptionSchedule.retrieve(sub_id)
+                sched = retry(lambda: stripe.SubscriptionSchedule.retrieve(sub_id))
                 status = SubscriptionScheduleStatus(sched.status)
                 if status == SubscriptionScheduleStatus.NOT_STARTED:
                     # The subscription is scheduled to start at some point in the future.

api/src/shop/stripe_util.py

@@ -96,15 +96,17 @@ def event_semantic_time(event: stripe.Event) -> datetime:
 
 
 def replace_default_payment_method(customer_id: str, payment_method_id: str) -> None:
-    stripe.Customer.modify(
-        customer_id,
-        invoice_settings={"default_payment_method": payment_method_id},
+    retry(
+        lambda: stripe.Customer.modify(
+            customer_id,
+            invoice_settings={"default_payment_method": payment_method_id},
+        )
     )
 
     # Delete all previous payment methods to keep things clean
     for pm in stripe.PaymentMethod.list(customer=customer_id).auto_paging_iter():
         if pm.id != payment_method_id:
-            stripe.PaymentMethod.detach(pm.id)
+            retry(lambda: stripe.PaymentMethod.detach(pm.id))
 
 
 T = TypeVar("T")