More wip

api/src/shop/pay.py

@@ -36,7 +36,6 @@
 
 def make_purchase(member_id: int, purchase: Purchase) -> Transaction:
     """Pay using the data in purchase, the purchase structure should be validated according to schema."""
-
     payment_method_id: str = purchase.stripe_payment_method_id
 
     transaction = create_transaction(member_id=member_id, purchase=purchase)
@@ -160,10 +159,12 @@ def setup_payment_method(data_dict: Any, member_id: int) -> SetupPaymentMethodRe
         raise BadRequest(message=f"Invalid data: {e}")
 
     member = db_session.query(Member).get(member_id)
-    assert member is not None
+    if member is None:
+        raise BadRequest(f"Unable to find member with id {member_id}")
 
     stripe_customer = get_and_sync_stripe_customer(member)
-    assert stripe_customer is not None
+    if stripe_customer is None:
+        raise BadRequest(f"Unable to find corresponding stripe member {member}")
 
     if data.setup_intent_id is None:
         try:
@@ -205,14 +206,16 @@ def cancel_subscriptions(data_dict: Any, user_id: int) -> None:
         raise BadRequest(message=f"Invalid data: {e}")
 
     member = db_session.query(Member).get(user_id)
-    assert member is not None
+    if member is None:
+        raise BadRequest(f"Unable to find member with id {user_id}")
 
     if SubscriptionType.MEMBERSHIP in data.subscriptions and SubscriptionType.LAB not in data.subscriptions:
         # This should be handled automatically by the frontend with a nice popup, but we will enforce it here
         data.subscriptions.append(SubscriptionType.LAB)
 
     stripe_customer = get_and_sync_stripe_customer(member)
-    assert stripe_customer is not None
+    if stripe_customer is None:
+        raise BadRequest(f"Unable to find corresponding stripe member {member}")
 
     for sub in data.subscriptions:
         cancel_subscription(member, stripe_customer, sub)
@@ -225,10 +228,12 @@ def start_subscriptions(data_dict: Any, user_id: int) -> None:
         raise BadRequest(message=f"Invalid data: {e}")
 
     member = db_session.query(Member).get(user_id)
-    assert member is not None
+    if member is None:
+        raise BadRequest(f"Unable to find member with id {user_id}")
 
     stripe_customer = get_and_sync_stripe_customer(member)
-    assert stripe_customer is not None
+    if stripe_customer is None:
+        raise BadRequest(f"Unable to find corresponding stripe member {member}")
 
     if not stripe_customer.invoice_settings["default_payment_method"]:
         raise BadRequest(message="You must add a default payment method before starting a subscription.")

api/src/shop/stripe_charge.py

@@ -20,7 +20,7 @@ def raise_from_stripe_invalid_request_error(e):
     raise PaymentFailed(log=f"stripe charge failed: {str(e)}", level=EXCEPTION)
 
 
-def create_stripe_charge(transaction, card_source_id) -> stripe.Charge:
+def create_stripe_charge(transaction: Transaction, card_source_id) -> stripe.Charge:
     if transaction.status != Transaction.PENDING:
         raise InternalServerError(
             f"unexpected status of transaction",

api/src/shop/stripe_customer.py

@@ -29,6 +29,7 @@ def are_metadata_dicts_equivalent(a: Dict[str, Any], b: Dict[str, Any]) -> bool:
 
 
 # TODO cleanup
+# TODO create some tests
 def get_and_sync_stripe_customer(
     makeradmin_member: Member, test_clock: Optional[stripe.test_helpers.TestClock] = None
 ) -> stripe.Customer:

api/src/shop/stripe_event.py

@@ -217,6 +217,7 @@ def stripe_invoice_event(subtype: EventSubtype, event: stripe.Event, current_tim
             # Presumably, it should be fixed by immediately pausing the subscription *before* the first
             # invoice is paid, and making sure the invoice does not contain the binding period.
             if subscription_type == SubscriptionType.LAB and member.labaccess_agreement_at is None:
+                # TODO input stripe customer instead of member id
                 stripe_subscriptions.pause_subscription(member_id, SubscriptionType.LAB, test_clock=None)
 
         if len(transaction_ids) > 0:

api/src/shop/stripe_payment_intent.py

@@ -157,7 +157,7 @@ def pay_with_stripe(transaction: Transaction, payment_method_id: str, setup_futu
     try:
         member = db_session.query(Member).get(transaction.member_id)
         assert member is not None
-        stripe_customer = get_and_sync_stripe_customer(member, test_clock=None)
+        stripe_customer = get_and_sync_stripe_customer(member)
         assert stripe_customer is not None
 
         payment_intent = stripe.PaymentIntent.create(

api/src/shop/stripe_subscriptions.py

@@ -53,7 +53,6 @@
     SubscriptionStatus,
     CURRENCY,
 )
-from shop.stripe_util import get_subscription_category
 import stripe.error
 
 
@@ -101,12 +100,6 @@ def get_stripe_subscriptions(stripe_customer_id: str, active_only: bool = True)
     ]
 
 
-@dataclass
-class ProductPricing:
-    recurring_price: stripe.Price
-    binding_period_price: Optional[stripe.Price]
-
-
 def calc_start_ts(current_end_date: date, now: datetime) -> datetime:
     end_dt = datetime.combine(current_end_date, time(0, 0, 0, tzinfo=timezone.utc))
 
@@ -150,10 +143,7 @@ def start_subscription(
     expected_to_pay_now: Optional[Decimal] = None,
     expected_to_pay_recurring: Optional[Decimal] = None,
 ) -> str:
-    if member is None:
-        raise BadRequest(f"Unable to find member with id {member.member_id}")
-    if stripe_customer is None:
-        raise BadRequest(f"Unable to find corresponding stripe member {member}")
+    # TODO maybe fix this?
     assert member.stripe_customer_id == stripe_customer.stripe_id
 
     logger.info(f"Attempting to start new subscription {subscription_type}")
@@ -169,6 +159,7 @@ def start_subscription(
             makeradmin_subscription_product,
             get_price_level_for_member(member),
         )
+        # Fetch fresh price objects from stripe to make sure we have the latest price
         stripe_product, stripe_prices = get_and_sync_stripe_product_and_prices(makeradmin_subscription_product)
 
         # Check that the price is as expected
@@ -179,9 +170,10 @@ def start_subscription(
                 # If the member already has the relevant membership, the subscription will start at the end of the current period, and nothing is paid right now
                 to_pay_now = Decimal(0)
             else:
-                # Fetch a fresh price object from stripe to make sure we have the latest price
-                to_pay_now_price = stripe.Price.retrieve(
-                    (price.binding_period_price or price.recurring_price).stripe_id
+                to_pay_now_price = (
+                    stripe_prices[PriceType.BINDING_PERIOD]
+                    if PriceType.BINDING_PERIOD in stripe_prices
+                    else stripe_prices[PriceType.RECURRING]
                 )
                 to_pay_now = convert_from_stripe_amount(to_pay_now_price["unit_amount"]) * (1 - discount.fraction_off)
             if to_pay_now != expected_to_pay_now:
@@ -190,9 +182,7 @@ def start_subscription(
                 )
 
         if expected_to_pay_recurring:
-            # Fetch a fresh price object from stripe to make sure we have the latest price
-            # Why? Doesn't this make the cache pointless?
-            to_pay_recurring_price = stripe.Price.retrieve(price.recurring_price.stripe_id)
+            to_pay_recurring_price = stripe_prices[PriceType.RECURRING]
             to_pay_recurring = convert_from_stripe_amount(to_pay_recurring_price["unit_amount"]) * (
                 1 - discount.fraction_off
             )
@@ -247,12 +237,12 @@ def start_subscription(
         # And we might not even want to change it even if we increase the binding period.
         # The binding period is primarily to prevent people from subscribing and then
         # immediately cancelling.
-        if price.binding_period_price and not was_already_member:
+        if PriceType.BINDING_PERIOD in stripe_prices and not was_already_member:
             phases.append(
                 {
                     "items": [
                         {
-                            "price": price.binding_period_price.stripe_id,
+                            "price": stripe_prices[PriceType.BINDING_PERIOD].stripe_id,
                             "metadata": metadata,
                         },
                     ],
@@ -268,7 +258,7 @@ def start_subscription(
             {
                 "items": [
                     {
-                        "price": price.recurring_price.stripe_id,
+                        "price": stripe_prices[PriceType.RECURRING].stripe_id,
                         "metadata": metadata,
                     },
                 ],
@@ -305,15 +295,12 @@ def start_subscription(
 
 
 def resume_paused_subscription(
-    member_id: int,
+    member: Member,
+    stripe_customer: stripe.Customer,
     subscription_type: SubscriptionType,
     earliest_start_at: Optional[datetime],
     test_clock: Optional[stripe.test_helpers.TestClock],
 ) -> bool:
-    member: Optional[Member] = db_session.query(Member).get(member_id)
-    if member is None:
-        raise BadRequest(f"Unable to find member with id {member_id}")
-
     if subscription_type == SubscriptionType.MEMBERSHIP:
         subscription_id = member.stripe_membership_subscription_id
     elif subscription_type == SubscriptionType.LAB:
@@ -338,23 +325,16 @@ def resume_paused_subscription(
     # but it was very tricky to do it while handling binding periods correctly.
     # So we just cancel the subscription and start a new one.
     # Much less code, much lower risk of bugs. Everyone is happy :)
-    cancel_subscription(member_id, subscription_type, test_clock)
-    start_subscription(member_id, subscription_type, test_clock, earliest_start_at)
+    cancel_subscription(member, stripe_customer, subscription_type)
+    start_subscription(member, stripe_customer, subscription_type, earliest_start_at)
     return True
 
 
-def pause_subscription(
-    member_id: int,
-    subscription_type: SubscriptionType,
-    test_clock: Optional[stripe.test_helpers.TestClock],
-) -> bool:
-    member: Optional[Member] = db_session.query(Member).get(member_id)
-    if member is None:
-        raise BadRequest(f"Unable to find member with id {member_id}")
-    stripe_customer = get_and_sync_stripe_customer(member, test_clock)
+def pause_subscription(stripe_customer: stripe.Customer, subscription_type: SubscriptionType) -> bool:
     assert stripe_customer is not None
-    assert member.stripe_customer_id == stripe_customer.stripe_id
 
+    # TODO some better way to handle this?
+    # TODO getters for subscriptions for stripe customer?
     if subscription_type == SubscriptionType.MEMBERSHIP:
         subscription_id = member.stripe_membership_subscription_id
     elif subscription_type == SubscriptionType.LAB:
@@ -398,12 +378,6 @@ def pause_subscription(
 
 
 def cancel_subscription(member: Member, stripe_customer: stripe.Customer, subscription_type: SubscriptionType) -> bool:
-    if member is None:
-        raise BadRequest(f"Unable to find member with id {member.member_id}")
-    if stripe_customer is None:
-        raise BadRequest(f"Unable to find corresponding stripe member {member}")
-    assert member.stripe_customer_id == stripe_customer.stripe_id
-
     if subscription_type == SubscriptionType.MEMBERSHIP:
         subscription_id = member.stripe_membership_subscription_id
     elif subscription_type == SubscriptionType.LAB:

api/src/shop/stripe_util.py

@@ -159,6 +159,7 @@ def _create_stripe_price(
     return stripe_price
 
 
+# TODO make some tests for the get and sync methods
 def get_and_sync_stripe_product(makeradmin_product: Product) -> stripe.Product:
     try:
         stripe_product = get_or_create_stripe_product(makeradmin_product)

api/src/shop/transactions.py

@@ -12,6 +12,7 @@
 from messages.message import send_message
 from messages.models import MessageTemplate
 from shop.stripe_constants import MakerspaceMetadataKeys
+from shop.stripe_customer import get_and_sync_stripe_customer
 from shop.stripe_discounts import get_discount_for_product, get_price_level_for_member
 from shop.stripe_subscriptions import SubscriptionType, resume_paused_subscription
 
@@ -184,8 +185,13 @@ def complete_pending_action(action: TransactionAction) -> None:
 
 def activate_paused_labaccess_subscription(member_id: int, earliest_start_at: datetime) -> None:
     member = db_session.query(Member).get(member_id)
+    if member is None:
+        raise BadRequest(f"Unable to find member with id {member_id}")
+    stripe_customer = get_and_sync_stripe_customer(member)
+    if stripe_customer is None:
+        raise BadRequest(f"Unable to find corresponding stripe member {member}")
     if member is not None and member.stripe_labaccess_subscription_id is not None:
-        resume_paused_subscription(member.member_id, SubscriptionType.LAB, earliest_start_at, test_clock=None)
+        resume_paused_subscription(member, stripe_customer, SubscriptionType.LAB, earliest_start_at, test_clock=None)
 
 
 def ship_add_labaccess_action(

api/src/shop/views.py

@@ -52,7 +52,6 @@
 from logging import getLogger
 
 from shop.stripe_subscriptions import (
-    cancel_subscription,
     get_subscription_products,
     list_subscriptions,
     open_stripe_customer_portal,

api/src/systest/api/purchase_test.py

@@ -25,6 +25,7 @@ def test_purchase_from_existing_member_using_non_3ds_card_works(self):
             CartItem(self.p1_id, p1_count),
         ]
 
+        # TODO check that this tests works properly and runs. It shouldnt work to send a card directly
         payment_method = stripe.PaymentMethod.create(type="card", card=self.card(VALID_NON_3DS_CARD_NO))
 
         purchase = Purchase(