diff --git a/api/src/shop/pay.py b/api/src/shop/pay.py index d2a2f96a3..b9f9441c8 100644 --- a/api/src/shop/pay.py +++ b/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 diff --git a/api/src/shop/stripe_charge.py b/api/src/shop/stripe_charge.py index d681ffe42..c5a06540e 100644 --- a/api/src/shop/stripe_charge.py +++ b/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) diff --git a/api/src/shop/stripe_customer.py b/api/src/shop/stripe_customer.py index fcdf71fbc..48fcc6e70 100644 --- a/api/src/shop/stripe_customer.py +++ b/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() diff --git a/api/src/shop/stripe_event.py b/api/src/shop/stripe_event.py index b787c7de5..473d842e9 100644 --- a/api/src/shop/stripe_event.py +++ b/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 diff --git a/api/src/shop/stripe_payment_intent.py b/api/src/shop/stripe_payment_intent.py index 440c19814..b8daa0f80 100644 --- a/api/src/shop/stripe_payment_intent.py +++ b/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)) diff --git a/api/src/shop/stripe_product_price.py b/api/src/shop/stripe_product_price.py index cab793004..ada295072 100644 --- a/api/src/shop/stripe_product_price.py +++ b/api/src/shop/stripe_product_price.py @@ -19,7 +19,10 @@ 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: @@ -27,6 +30,8 @@ def makeradmin_to_stripe_recurring(makeradmin_product: Product, price_type: Pric 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) diff --git a/api/src/shop/stripe_subscriptions.py b/api/src/shop/stripe_subscriptions.py index de47571ec..2610f49a2 100644 --- a/api/src/shop/stripe_subscriptions.py +++ b/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. diff --git a/api/src/shop/stripe_util.py b/api/src/shop/stripe_util.py index 1a2fe5c6e..30df4c5ca 100644 --- a/api/src/shop/stripe_util.py +++ b/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") diff --git a/api/src/shop/test/stripe_product_price_test.py b/api/src/shop/test/stripe_product_price_test.py index 3fd6c948e..e33db4f41 100644 --- a/api/src/shop/test/stripe_product_price_test.py +++ b/api/src/shop/test/stripe_product_price_test.py @@ -25,7 +25,7 @@ replace_stripe_price, ) from test_aid.systest_config import STRIPE_PRIVATE_KEY -from shop.stripe_util import convert_to_stripe_amount +from shop.stripe_util import convert_to_stripe_amount, get_subscription_category from shop.models import Product from shop import stripe_constants import stripe @@ -37,10 +37,25 @@ class StripeRecurringWithoutStripeTest(ShopTestMixin, FlaskTestBase): models = [membership.models, messages.models, shop.models, core.models] + @classmethod + def setUpClass(self) -> None: + super().setUpClass() + self.subscription_category_id = get_subscription_category().id + self.not_subscription_category_id = self.db.create_category(name="Not Subscriptions").id + + def test_makeradmin_to_stripe_recurring_fixed(self) -> None: + makeradmin_test_product = self.db.create_product( + unit="st", + category_id=self.not_subscription_category_id, + ) + recurring = makeradmin_to_stripe_recurring(makeradmin_test_product, stripe_constants.PriceType.FIXED_PRICE) + assert recurring is None + def test_makeradmin_to_stripe_recurring_recurring(self) -> None: makeradmin_test_product = self.db.create_product( unit="mån", smallest_multiple=3, + category_id=self.subscription_category_id, ) recurring = makeradmin_to_stripe_recurring(makeradmin_test_product, stripe_constants.PriceType.RECURRING) assert recurring.interval == "month" @@ -50,6 +65,7 @@ def test_makeradmin_to_stripe_recurring_binding(self) -> None: makeradmin_test_product = self.db.create_product( unit="mån", smallest_multiple=3, + category_id=self.subscription_category_id, ) recurring = makeradmin_to_stripe_recurring(makeradmin_test_product, stripe_constants.PriceType.BINDING_PERIOD) assert recurring.interval == "month" @@ -58,11 +74,28 @@ def test_makeradmin_to_stripe_recurring_binding(self) -> None: def test_makeradmin_to_stripe_recurring_wrong_unit(self) -> None: makeradmin_test_product = self.db.create_product( unit="st", + category_id=self.subscription_category_id, ) with self.assertRaises(ValueError) as context: makeradmin_to_stripe_recurring(makeradmin_test_product, stripe_constants.PriceType.RECURRING), self.assertTrue("Unexpected unit" in str(context.exception)) + def test_makeradmin_to_stripe_recurring_price_type_missmatch_sub(self) -> None: + makeradmin_test_product = self.db.create_product( + category_id=self.subscription_category_id, + ) + with self.assertRaises(ValueError) as context: + makeradmin_to_stripe_recurring(makeradmin_test_product, stripe_constants.PriceType.FIXED_PRICE), + self.assertTrue("Unexpected price type" in str(context.exception)) + + def test_makeradmin_to_stripe_recurring_price_type_missmatch_not_sub(self) -> None: + makeradmin_test_product = self.db.create_product( + category_id=self.not_subscription_category_id, + ) + with self.assertRaises(ValueError) as context: + makeradmin_to_stripe_recurring(makeradmin_test_product, stripe_constants.PriceType.RECURRING), + self.assertTrue("Unexpected price type" in str(context.exception)) + class StripeProductPriceTest(ShopTestMixin, FlaskTestBase): # The products id in makeradmin have to be unique in each test to prevent race conditions @@ -350,7 +383,7 @@ def test_get_sync_price(self) -> None: ) def test_equal_product(self) -> None: - makeradmin_test_eq_product = self.db.create_product( + makeradmin_test_product = self.db.create_product( name="test eq", price=200.0, id=self.base_stripe_id + 10, @@ -358,28 +391,22 @@ def test_equal_product(self) -> None: smallest_multiple=1, category_id=self.subscription_category.id, ) - makeradmin_test_not_eq_product = self.db.create_product( - name="test not eq", - price=200.0, - id=self.base_stripe_id - 1, - unit="mån", - smallest_multiple=1, - category_id=self.subscription_category.id, - ) - self.seen_products.append(makeradmin_test_eq_product) - stripe_test_product = get_and_sync_stripe_product(makeradmin_test_eq_product) + self.seen_products.append(makeradmin_test_product) + stripe_test_product = get_and_sync_stripe_product(makeradmin_test_product) assert stripe_test_product - stripe_test_prices = get_and_sync_stripe_prices_for_product(makeradmin_test_eq_product, stripe_test_product) + stripe_test_prices = get_and_sync_stripe_prices_for_product(makeradmin_test_product, stripe_test_product) assert stripe_test_prices assert len(stripe_test_prices) == 1 - assert eq_makeradmin_stripe_product(makeradmin_test_eq_product, stripe_test_product) - assert not eq_makeradmin_stripe_product(makeradmin_test_not_eq_product, stripe_test_product) + assert eq_makeradmin_stripe_product(makeradmin_test_product, stripe_test_product) - def test_equal_price(self) -> None: - makeradmin_test_eq_product = self.db.create_product( + makeradmin_test_product.name = "test not eq name" + assert not eq_makeradmin_stripe_product(makeradmin_test_product, stripe_test_product) + + def test_equal_price_non_binding(self) -> None: + makeradmin_test_product = self.db.create_product( name="test eq price", price=200.0, id=self.base_stripe_id + 11, @@ -387,66 +414,72 @@ def test_equal_price(self) -> None: smallest_multiple=1, category_id=self.subscription_category.id, ) - product_not_eq_price = self.db.create_product( - name="test not eq price", - price=210.0, - id=self.base_stripe_id - 1, - unit="mån", - smallest_multiple=1, - category_id=self.subscription_category.id, - ) - product_not_eq_unit = self.db.create_product( - name="test not eq unit", - price=200.0, - id=self.base_stripe_id - 2, - unit="year", - smallest_multiple=1, - category_id=self.subscription_category.id, - ) - product_not_eq_id = self.db.create_product( - name="test not eq mult", - price=200.0, - id=self.base_stripe_id - 3, - unit="mån", - smallest_multiple=2, - category_id=self.subscription_category.id, - ) - product_not_eq_cat = self.db.create_product( - name="test not eq category", - price=200.0, - id=self.base_stripe_id - 4, - unit="mån", - smallest_multiple=2, - category_id=self.not_subscription_category.id, - ) - makeradmin_test_products_not_eq = [ - product_not_eq_price, - product_not_eq_unit, - product_not_eq_id, - product_not_eq_cat, - ] - price_types = [ - stripe_constants.PriceType.RECURRING, - stripe_constants.PriceType.RECURRING, - stripe_constants.PriceType.BINDING_PERIOD, - stripe_constants.PriceType.FIXED_PRICE, - ] - self.seen_products.append(makeradmin_test_eq_product) - stripe_test_product = get_and_sync_stripe_product(makeradmin_test_eq_product) + not_equal_variables = { + "price": 210, + "unit": "year", + } + self.seen_products.append(makeradmin_test_product) + stripe_test_product = get_and_sync_stripe_product(makeradmin_test_product) assert stripe_test_product - stripe_test_prices = get_and_sync_stripe_prices_for_product(makeradmin_test_eq_product, stripe_test_product) + stripe_test_prices = get_and_sync_stripe_prices_for_product(makeradmin_test_product, stripe_test_product) assert stripe_test_prices assert len(stripe_test_prices) == 1 assert eq_makeradmin_stripe_price( - makeradmin_test_eq_product, + makeradmin_test_product, stripe_test_prices[stripe_constants.PriceType.RECURRING], stripe_constants.PriceType.RECURRING, ) - for product, price_type in zip(makeradmin_test_products_not_eq, price_types): + + for key, value in not_equal_variables.items(): + old_value = getattr(makeradmin_test_product, key) + setattr(makeradmin_test_product, key, value) assert not eq_makeradmin_stripe_price( - product, + makeradmin_test_product, stripe_test_prices[stripe_constants.PriceType.RECURRING], + stripe_constants.PriceType.RECURRING, + ) + setattr(makeradmin_test_product, key, old_value) + + def test_equal_price_with_binding(self) -> None: + makeradmin_test_product = self.db.create_product( + name="test eq price binding", + price=250.0, + id=self.base_stripe_id + 12, + unit="mån", + smallest_multiple=2, + category_id=self.subscription_category.id, + ) + not_equal_variables = { + "price": 110, + "unit": "year", + "smallest_multiple": 3, + } + self.seen_products.append(makeradmin_test_product) + stripe_test_product = get_and_sync_stripe_product(makeradmin_test_product) + assert stripe_test_product + + stripe_test_prices = get_and_sync_stripe_prices_for_product(makeradmin_test_product, stripe_test_product) + assert stripe_test_prices + assert len(stripe_test_prices) == 2 + + for price_type in [stripe_constants.PriceType.RECURRING, stripe_constants.PriceType.BINDING_PERIOD]: + assert eq_makeradmin_stripe_price( + makeradmin_test_product, + stripe_test_prices[price_type], price_type, ) + + logger.info(stripe_test_prices[stripe_constants.PriceType.BINDING_PERIOD]) + for key, value in not_equal_variables.items(): + old_value = getattr(makeradmin_test_product, key) + setattr(makeradmin_test_product, key, value) + logger.info(f"key {key} value {value}") + logger.info(makeradmin_test_product) + assert not eq_makeradmin_stripe_price( + makeradmin_test_product, + stripe_test_prices[stripe_constants.PriceType.BINDING_PERIOD], + stripe_constants.PriceType.BINDING_PERIOD, + ) + setattr(makeradmin_test_product, key, old_value) diff --git a/api/src/shop/test/subscriptions_test.py b/api/src/shop/test/subscriptions_test.py index 7edcf6516..30c62f96e 100644 --- a/api/src/shop/test/subscriptions_test.py +++ b/api/src/shop/test/subscriptions_test.py @@ -11,11 +11,8 @@ from shop.stripe_constants import MakerspaceMetadataKeys from shop.stripe_customer import get_and_sync_stripe_customer -from shop.stripe_util import event_semantic_time, get_subscription_category -from shop.stripe_subscriptions import ( - BINDING_PERIOD, - SubscriptionType, -) +from shop.stripe_util import event_semantic_time, get_subscription_category, retry +from shop.stripe_subscriptions import SubscriptionType from shop.stripe_setup import setup_stripe_products from shop.stripe_product_price import ( get_stripe_product, @@ -78,12 +75,14 @@ def attach_and_set_payment_method( stripe_member = get_and_sync_stripe_customer(member, test_clock=test_clock) assert stripe_member is not None - payment_method = stripe.PaymentMethod.attach(card_token.value, customer=stripe_member.stripe_id) - stripe.Customer.modify( - stripe_member.stripe_id, - invoice_settings={ - "default_payment_method": payment_method.stripe_id, - }, + payment_method = retry(lambda: stripe.PaymentMethod.attach(card_token.value, customer=stripe_member.stripe_id)) + retry( + lambda: stripe.Customer.modify( + stripe_member.stripe_id, + invoice_settings={ + "default_payment_method": payment_method.stripe_id, + }, + ) ) @@ -109,6 +108,7 @@ def setUpClass(self) -> None: subscription_category = get_subscription_category() self.membership_subscription_product = self.db.create_product( + id=100, name="test subscriptions membership", price=200.0, unit="år", @@ -121,10 +121,11 @@ def setUpClass(self) -> None: ) self.access_subscription_product = self.db.create_product( + id=101, name="test subscriptions access", price=350.0, unit="mån", - smallest_multiple=2, # TODO need to test with multiple values + smallest_multiple=2, category_id=subscription_category.id, product_metadata={ MakerspaceMetadataKeys.ALLOWED_PRICE_LEVELS.value: ["low_income_discount"], @@ -134,24 +135,6 @@ def setUpClass(self) -> None: setup_stripe_products() - @classmethod - def tearDownClass(self) -> None: - # It is not possible to delete prices through the api so we set them as inactive instead - for makeradmin_product in [self.membership_subscription_product, self.access_subscription_product]: - stripe_product = get_stripe_product(makeradmin_product) - if stripe_product is None: - continue - if stripe_product.active: - deactivate_stripe_product(stripe_product) - stripe_prices = get_stripe_prices(stripe_product) - if stripe_prices is None: - continue - for price in stripe_prices: - if price.active: - deactivate_stripe_price(price) - - super().tearDownClass() - @skipIf(not STRIPE_PRIVATE_KEY, "subscriptions tests require stripe api key in .env file") def setUp(self) -> None: db_session.query(Member).delete() @@ -160,7 +143,6 @@ def setUp(self) -> None: self.seen_event_ids = set() self.earliest_possible_event_time = datetime.now(timezone.utc) self.clocks_to_destroy: List[FakeClock] = [] - # stripe_setup.set_stripe_key(True) disable_loggers = ["stripe"] @@ -495,17 +477,13 @@ def test_subscriptions_member_deleted(self) -> None: self.advance_clock(clock, now + time_delta(days=10)) assert member.deleted_at is not None - assert stripe.Customer.retrieve(stripe_customer_id).deleted + assert retry(lambda: stripe.Customer.retrieve(stripe_customer_id)).deleted - # TODO fix this test to work with new stripe setup and no binding period def test_subscriptions_binding_period(self) -> None: """ Checks that a lab subscription is started with a binding period """ - binding_period = BINDING_PERIOD[SubscriptionType.LAB] - if binding_period <= 0: - pytest.skip("No binding period for lab access") - + binding_period = self.access_subscription_product.smallest_multiple (now, clock, member) = self.setup_single_member() stripe_subscriptions.start_subscription( @@ -539,7 +517,7 @@ def test_subscriptions_resubscribe(self) -> None: """ Checks that a subscription can be cancelled, and the member can resubscribe immediately """ - binding_period = 2 # TODO fix this, split into two tests + binding_period = self.access_subscription_product.smallest_multiple if binding_period <= 0: pytest.skip("No binding period for lab access") @@ -589,9 +567,7 @@ def test_subscriptions_failing_card(self) -> None: """ Checks that if a subscription fails to charge, the subscription is deleted after a while """ - binding_period = BINDING_PERIOD[SubscriptionType.LAB] - if binding_period <= 0: - pytest.skip("No binding period for lab access") + binding_period = self.access_subscription_product.smallest_multiple (now, clock, member) = self.setup_single_member() @@ -628,9 +604,7 @@ def test_subscriptions_retry_card(self) -> None: """ Checks that if a subscription fails to charge, the subscription is retried a few times and then nenewed when we switch to a new card """ - binding_period = BINDING_PERIOD[SubscriptionType.LAB] - if binding_period <= 0: - pytest.skip("No binding period for lab access") + binding_period = self.access_subscription_product.smallest_multiple (now, clock, member) = self.setup_single_member() diff --git a/api/src/systest/api/register_test.py b/api/src/systest/api/register_test.py index 6dae35d49..c4075ee98 100644 --- a/api/src/systest/api/register_test.py +++ b/api/src/systest/api/register_test.py @@ -4,6 +4,7 @@ import stripe from shop.stripe_payment_intent import PaymentIntentResult from shop.stripe_constants import MakerspaceMetadataKeys +from shop.stripe_util import retry from shop.stripe_subscriptions import SubscriptionType from shop.transactions import CartItem, Purchase from shop.pay import MemberInfo, RegisterRequest @@ -29,7 +30,7 @@ class Test(ApiShopTestMixin, ApiTest): ] def test_registering_new_member_works_and_returns_token(self) -> None: - payment_method = stripe.PaymentMethod.create(type="card", card=self.card(VALID_NON_3DS_CARD_NO)) + payment_method = retry(lambda: stripe.PaymentMethod.create(type="card", card=self.card(VALID_NON_3DS_CARD_NO))) member = self.obj.create_member() register: RegisterRequest = RegisterRequest( @@ -121,7 +122,7 @@ def test_registering_with_existing_member_email_does_not_work_and_does_not_retur ) def test_registering_with_failed_payment_does_not_work_and_does_not_return_token(self) -> None: - payment_method = stripe.PaymentMethod.create(type="card", card=self.card(EXPIRED_3DS_CARD_NO)) + payment_method = retry(lambda: stripe.PaymentMethod.create(type="card", card=self.card(EXPIRED_3DS_CARD_NO))) member = self.obj.create_member()