From 058e85f86fa944c5a58714fbe5bc85ee2a72f84f Mon Sep 17 00:00:00 2001 From: Suejung Shin Date: Mon, 6 Jan 2025 16:31:39 -0800 Subject: [PATCH 1/7] feat: Add stripe setupIntent api --- api/internal/owner/views.py | 17 +++++++++++++++++ services/billing.py | 24 ++++++++++++++++++++++++ services/tests/test_billing.py | 10 ++++++++++ 3 files changed, 51 insertions(+) diff --git a/api/internal/owner/views.py b/api/internal/owner/views.py index 96fe636844..9b7ba0730e 100644 --- a/api/internal/owner/views.py +++ b/api/internal/owner/views.py @@ -113,6 +113,23 @@ def update_billing_address(self, request, *args, **kwargs): return Response(self.get_serializer(owner).data) + @action(detail=False, methods=["get"]) + @stripe_safe + def setup_intent(self, request, *args, **kwargs): + """ + GET a Stripe setupIntent clientSecret for updating payment method + """ + try: + billing = BillingService(requesting_user=request.current_owner) + client_secret = billing.get_setup_intent(self.owner) + return Response({"client_secret": client_secret}) + except Exception as e: + log.error( + f"Error getting setup intent for owner {self.owner.ownerid}", + extra={"error": str(e)}, + ) + raise ValidationError(detail="Unable to create setup intent") + class UsersOrderingFilter(filters.OrderingFilter): def get_valid_fields(self, queryset, view, context=None): fields = super().get_valid_fields(queryset, view, context=context or {}) diff --git a/services/billing.py b/services/billing.py index 4568f26d40..db419c19c2 100644 --- a/services/billing.py +++ b/services/billing.py @@ -59,6 +59,10 @@ def modify_subscription(self, owner, plan): def create_checkout_session(self, owner, plan): pass + @abstractmethod + def create_setup_intent(self, owner): + pass + @abstractmethod def get_subscription(self, owner): pass @@ -539,6 +543,23 @@ def create_checkout_session(self, owner: Owner, desired_plan): ) return session["id"] + @_log_stripe_error + def create_setup_intent(self, owner: Owner): + log.info( + "Stripe create setup intent for owner", + extra=dict( + owner_id=owner.ownerid, + user_id=self.requesting_user.ownerid, + subscription_id=owner.stripe_subscription_id, + customer_id=owner.stripe_customer_id, + ), + ) + setup_intent = stripe.SetupIntent.create( + payment_method_types=['card', 'us_bank_account'], + customer=owner.stripe_customer_id, + ) + return setup_intent.client_secret + @_log_stripe_error def update_payment_method(self, owner: Owner, payment_method): log.info( @@ -722,6 +743,9 @@ def __init__(self, payment_service=None, requesting_user=None): def get_subscription(self, owner): return self.payment_service.get_subscription(owner) + def get_setup_intent(self, owner): + return self.payment_service.create_setup_intent(owner) + def get_schedule(self, owner): return self.payment_service.get_schedule(owner) diff --git a/services/tests/test_billing.py b/services/tests/test_billing.py index 66b3b1b383..eaed0adb6b 100644 --- a/services/tests/test_billing.py +++ b/services/tests/test_billing.py @@ -1825,6 +1825,15 @@ def test_apply_cancellation_discount_existing_coupon( assert not customer_modify_mock.called assert not coupon_create_mock.called + @patch("services.billing.stripe.SetupIntent.create") + def test_get_setup_intent(self, setup_intent_create_mock): + owner = OwnerFactory(stripe_customer_id="test-customer-id") + setup_intent_create_mock.return_value = {"client_secret": "test-client-secret"} + resp = self.stripe.get_setup_intent(owner) + self.stripe.payment_service.get_setup_intent.assert_called_once_with(owner) + + assert resp.client_secret == "test-client-secret" + class MockPaymentService(AbstractPaymentService): def list_filtered_invoices(self, owner, limit=10): @@ -2022,3 +2031,4 @@ def test_get_invoice(self, get_invoice_mock): owner = OwnerFactory() self.billing_service.get_invoice(owner, "abc") get_invoice_mock.assert_called_once_with(owner, "abc") + From 0d8036433c30b6a4499695139306efea46fc0962 Mon Sep 17 00:00:00 2001 From: Suejung Shin Date: Sun, 12 Jan 2025 22:57:40 -0800 Subject: [PATCH 2/7] add end to end ach items --- api/internal/owner/serializers.py | 9 +++++++ api/internal/owner/views.py | 3 ++- billing/views.py | 3 +++ codecov/settings_base.py | 6 ++--- graphql_api/types/invoice/invoice.graphql | 8 +++++++ services/billing.py | 29 +++++++++++++++++++---- 6 files changed, 50 insertions(+), 8 deletions(-) diff --git a/api/internal/owner/serializers.py b/api/internal/owner/serializers.py index 954f2972fc..42d42bb09b 100644 --- a/api/internal/owner/serializers.py +++ b/api/internal/owner/serializers.py @@ -108,8 +108,17 @@ class StripeCardSerializer(serializers.Serializer): last4 = serializers.CharField() +class StripeUSBankAccountSerializer(serializers.Serializer): + account_holder_type = serializers.CharField() + account_type = serializers.CharField() + bank_name = serializers.CharField() + last4 = serializers.CharField() + routing_number = serializers.CharField() + + class StripePaymentMethodSerializer(serializers.Serializer): card = StripeCardSerializer(read_only=True) + us_bank_account = StripeUSBankAccountSerializer(read_only=True) billing_details = serializers.JSONField(read_only=True) diff --git a/api/internal/owner/views.py b/api/internal/owner/views.py index 9b7ba0730e..5b90a8c371 100644 --- a/api/internal/owner/views.py +++ b/api/internal/owner/views.py @@ -85,7 +85,8 @@ def update_email(self, request, *args, **kwargs): raise ValidationError(detail="No new_email sent") owner = self.get_object() billing = BillingService(requesting_user=request.current_owner) - billing.update_email_address(owner, new_email) + should_propagate = request.data.get("should_propagate_to_payment_methods", False) + billing.update_email_address(owner, new_email, should_propagate_to_payment_methods=should_propagate) return Response(self.get_serializer(owner).data) @action(detail=False, methods=["patch"]) diff --git a/billing/views.py b/billing/views.py index 8060ee4b4c..ab143b0cc4 100644 --- a/billing/views.py +++ b/billing/views.py @@ -312,6 +312,7 @@ def customer_subscription_created(self, subscription: stripe.Subscription) -> No self._log_updated([owner]) def customer_subscription_updated(self, subscription: stripe.Subscription) -> None: + print("CUSTOMER SUBSCRIPTION UPDATED", subscription) owners: QuerySet[Owner] = Owner.objects.filter( stripe_subscription_id=subscription.id, stripe_customer_id=subscription.customer, @@ -408,6 +409,8 @@ def customer_subscription_updated(self, subscription: stripe.Subscription) -> No ) def customer_updated(self, customer: stripe.Customer) -> None: + print("CUSTOMER UPDATED", customer) + new_default_payment_method = customer["invoice_settings"][ "default_payment_method" ] diff --git a/codecov/settings_base.py b/codecov/settings_base.py index 721bbfa0ca..2c4fd1755f 100644 --- a/codecov/settings_base.py +++ b/codecov/settings_base.py @@ -98,9 +98,7 @@ "setup", "graphql", "query_cost_threshold", default=10000 ) -GRAPHQL_RATE_LIMIT_ENABLED = get_config( - "setup", "graphql", "rate_limit_enabled", default=True -) +GRAPHQL_RATE_LIMIT_ENABLED = get_config("setup", "graphql", "rate_limit_enabled", default=True) GRAPHQL_RATE_LIMIT_RPM = get_config("setup", "graphql", "rate_limit_rpm", default=300) @@ -428,6 +426,8 @@ SHELTER_PUBSUB_PROJECT_ID = get_config("setup", "shelter", "pubsub_project_id") SHELTER_PUBSUB_SYNC_REPO_TOPIC_ID = get_config("setup", "shelter", "sync_repo_topic_id") +STRIPE_PAYMENT_METHOD_CONFIGURATION_ID = get_config("setup", "stripe", "payment_method_configuration", default=None) + # Allows to do migrations from another module MIGRATION_MODULES = { "codecov_auth": "shared.django_apps.codecov_auth.migrations", diff --git a/graphql_api/types/invoice/invoice.graphql b/graphql_api/types/invoice/invoice.graphql index 1c6b2ae1ec..c7ad1e8265 100644 --- a/graphql_api/types/invoice/invoice.graphql +++ b/graphql_api/types/invoice/invoice.graphql @@ -43,6 +43,14 @@ type Card { last4: String } +type BankAccount { + accountHolderType: String + accountType: String + bankName: String + last4: String + routingNumber: String +} + type BillingDetails { address: Address email: String diff --git a/services/billing.py b/services/billing.py index db419c19c2..28414549d5 100644 --- a/services/billing.py +++ b/services/billing.py @@ -516,8 +516,8 @@ def create_checkout_session(self, owner: Owner, desired_plan): ) session = stripe.checkout.Session.create( + payment_method_configuration=settings.STRIPE_PAYMENT_METHOD_CONFIGURATION_ID, billing_address_collection="required", - payment_method_types=["card"], payment_method_collection="if_required", client_reference_id=str(owner.ownerid), success_url=success_url, @@ -601,7 +601,7 @@ def update_payment_method(self, owner: Owner, payment_method): ) @_log_stripe_error - def update_email_address(self, owner: Owner, email_address: str): + def update_email_address(self, owner: Owner, email_address: str, should_propagate_to_payment_methods: bool = False): if not re.fullmatch(r"[^@]+@[^@]+\.[^@]+", email_address): return None @@ -616,6 +616,27 @@ def update_email_address(self, owner: Owner, email_address: str): f"Stripe successfully updated email address for owner {owner.ownerid} by user #{self.requesting_user.ownerid}" ) + if should_propagate_to_payment_methods: + try: + default_payment_method = stripe.Customer.retrieve( + owner.stripe_customer_id + ).invoice_settings.default_payment_method + + stripe.PaymentMethod.modify( + default_payment_method, + billing_details={"email": email_address}, + ) + log.info( + f"Stripe successfully updated billing email for payment method {default_payment_method}" + ) + except Exception: + log.error( + "Unable to update billing email for payment method", + extra=dict( + payment_method=default_payment_method, + ), + ) + @_log_stripe_error def update_billing_address(self, owner: Owner, name, billing_address): log.info(f"Stripe update billing address for owner {owner.ownerid}") @@ -786,14 +807,14 @@ def update_payment_method(self, owner, payment_method): """ return self.payment_service.update_payment_method(owner, payment_method) - def update_email_address(self, owner: Owner, email_address: str): + def update_email_address(self, owner: Owner, email_address: str, should_propagate_to_payment_methods: bool = False): """ Takes an owner and a new email. Email is a string coming directly from the front-end. If the owner has a payment id and if it's a valid email, the payment service will update the email address in the upstream service. Otherwise returns None. """ - return self.payment_service.update_email_address(owner, email_address) + return self.payment_service.update_email_address(owner, email_address, should_propagate_to_payment_methods) def update_billing_address(self, owner: Owner, name: str, billing_address): """ From 198f23ecdf46f660d4841f252f1cb0f1c000bef5 Mon Sep 17 00:00:00 2001 From: Suejung Shin Date: Mon, 13 Jan 2025 18:38:41 -0800 Subject: [PATCH 3/7] move create-setup-intent to graphql api --- api/internal/owner/serializers.py | 3 -- api/internal/owner/views.py | 36 ++++++++++--- billing/views.py | 3 -- codecov/settings_base.py | 8 ++- .../interactors/create_stripe_setup_intent.py | 38 ++++++++++++++ codecov_auth/commands/owner/owner.py | 4 ++ .../test_create_stripe_setup_intent.py | 34 ++++++++++++ .../inputs/create_stripe_setup_intent.graphql | 3 ++ graphql_api/types/invoice/invoice.graphql | 6 +-- graphql_api/types/mutation/__init__.py | 2 + .../create_stripe_setup_intent/__init__.py | 12 +++++ .../create_stripe_setup_intent.graphql | 6 +++ .../create_stripe_setup_intent.py | 24 +++++++++ graphql_api/types/mutation/mutation.graphql | 1 + graphql_api/types/mutation/mutation.py | 6 +++ services/billing.py | 52 ++++++++++--------- services/tests/test_billing.py | 22 ++++---- 17 files changed, 207 insertions(+), 53 deletions(-) create mode 100644 codecov_auth/commands/owner/interactors/create_stripe_setup_intent.py create mode 100644 graphql_api/tests/mutation/test_create_stripe_setup_intent.py create mode 100644 graphql_api/types/inputs/create_stripe_setup_intent.graphql create mode 100644 graphql_api/types/mutation/create_stripe_setup_intent/__init__.py create mode 100644 graphql_api/types/mutation/create_stripe_setup_intent/create_stripe_setup_intent.graphql create mode 100644 graphql_api/types/mutation/create_stripe_setup_intent/create_stripe_setup_intent.py diff --git a/api/internal/owner/serializers.py b/api/internal/owner/serializers.py index 42d42bb09b..b4c54c2fe7 100644 --- a/api/internal/owner/serializers.py +++ b/api/internal/owner/serializers.py @@ -109,11 +109,8 @@ class StripeCardSerializer(serializers.Serializer): class StripeUSBankAccountSerializer(serializers.Serializer): - account_holder_type = serializers.CharField() - account_type = serializers.CharField() bank_name = serializers.CharField() last4 = serializers.CharField() - routing_number = serializers.CharField() class StripePaymentMethodSerializer(serializers.Serializer): diff --git a/api/internal/owner/views.py b/api/internal/owner/views.py index 5b90a8c371..90116d4e32 100644 --- a/api/internal/owner/views.py +++ b/api/internal/owner/views.py @@ -80,13 +80,31 @@ def update_payment(self, request, *args, **kwargs): @action(detail=False, methods=["patch"]) @stripe_safe def update_email(self, request, *args, **kwargs): + """ + Update the email address associated with the owner's billing account. + + Args: + request: The HTTP request object containing: + - new_email: The new email address to update to + - should_propagate_to_payment_methods: Optional boolean flag to update email on payment methods (default False) + + Returns: + Response with serialized owner data + + Raises: + ValidationError: If no new_email is provided in the request + """ new_email = request.data.get("new_email") if not new_email: raise ValidationError(detail="No new_email sent") owner = self.get_object() billing = BillingService(requesting_user=request.current_owner) - should_propagate = request.data.get("should_propagate_to_payment_methods", False) - billing.update_email_address(owner, new_email, should_propagate_to_payment_methods=should_propagate) + should_propagate = request.data.get( + "should_propagate_to_payment_methods", False + ) + billing.update_email_address( + owner, new_email, should_propagate_to_payment_methods=should_propagate + ) return Response(self.get_serializer(owner).data) @action(detail=False, methods=["patch"]) @@ -113,16 +131,21 @@ def update_billing_address(self, request, *args, **kwargs): billing.update_billing_address(owner, name, billing_address=formatted_address) return Response(self.get_serializer(owner).data) - - @action(detail=False, methods=["get"]) + @action(detail=False, methods=["post"]) @stripe_safe def setup_intent(self, request, *args, **kwargs): """ - GET a Stripe setupIntent clientSecret for updating payment method + Create a Stripe SetupIntent to securely collect payment details. + + Returns: + Response with SetupIntent client_secret for frontend payment method setup. + + Raises: + ValidationError: If SetupIntent creation fails """ try: billing = BillingService(requesting_user=request.current_owner) - client_secret = billing.get_setup_intent(self.owner) + client_secret = billing.create_setup_intent(self.owner) return Response({"client_secret": client_secret}) except Exception as e: log.error( @@ -131,6 +154,7 @@ def setup_intent(self, request, *args, **kwargs): ) raise ValidationError(detail="Unable to create setup intent") + class UsersOrderingFilter(filters.OrderingFilter): def get_valid_fields(self, queryset, view, context=None): fields = super().get_valid_fields(queryset, view, context=context or {}) diff --git a/billing/views.py b/billing/views.py index ab143b0cc4..8060ee4b4c 100644 --- a/billing/views.py +++ b/billing/views.py @@ -312,7 +312,6 @@ def customer_subscription_created(self, subscription: stripe.Subscription) -> No self._log_updated([owner]) def customer_subscription_updated(self, subscription: stripe.Subscription) -> None: - print("CUSTOMER SUBSCRIPTION UPDATED", subscription) owners: QuerySet[Owner] = Owner.objects.filter( stripe_subscription_id=subscription.id, stripe_customer_id=subscription.customer, @@ -409,8 +408,6 @@ def customer_subscription_updated(self, subscription: stripe.Subscription) -> No ) def customer_updated(self, customer: stripe.Customer) -> None: - print("CUSTOMER UPDATED", customer) - new_default_payment_method = customer["invoice_settings"][ "default_payment_method" ] diff --git a/codecov/settings_base.py b/codecov/settings_base.py index 2c4fd1755f..bab173aa7f 100644 --- a/codecov/settings_base.py +++ b/codecov/settings_base.py @@ -98,7 +98,9 @@ "setup", "graphql", "query_cost_threshold", default=10000 ) -GRAPHQL_RATE_LIMIT_ENABLED = get_config("setup", "graphql", "rate_limit_enabled", default=True) +GRAPHQL_RATE_LIMIT_ENABLED = get_config( + "setup", "graphql", "rate_limit_enabled", default=True +) GRAPHQL_RATE_LIMIT_RPM = get_config("setup", "graphql", "rate_limit_rpm", default=300) @@ -426,7 +428,9 @@ SHELTER_PUBSUB_PROJECT_ID = get_config("setup", "shelter", "pubsub_project_id") SHELTER_PUBSUB_SYNC_REPO_TOPIC_ID = get_config("setup", "shelter", "sync_repo_topic_id") -STRIPE_PAYMENT_METHOD_CONFIGURATION_ID = get_config("setup", "stripe", "payment_method_configuration", default=None) +STRIPE_PAYMENT_METHOD_CONFIGURATION_ID = get_config( + "setup", "stripe", "payment_method_configuration", default=None +) # Allows to do migrations from another module MIGRATION_MODULES = { diff --git a/codecov_auth/commands/owner/interactors/create_stripe_setup_intent.py b/codecov_auth/commands/owner/interactors/create_stripe_setup_intent.py new file mode 100644 index 0000000000..3e9ba28b6f --- /dev/null +++ b/codecov_auth/commands/owner/interactors/create_stripe_setup_intent.py @@ -0,0 +1,38 @@ +import logging + +import stripe + +from codecov.commands.base import BaseInteractor +from codecov.commands.exceptions import Unauthenticated, Unauthorized, ValidationError +from codecov.db import sync_to_async +from codecov_auth.helpers import current_user_part_of_org +from codecov_auth.models import Owner +from services.billing import BillingService + +log = logging.getLogger(__name__) + +class CreateStripeSetupIntentInteractor(BaseInteractor): + def validate(self, owner_obj: Owner) -> None: + if not self.current_user.is_authenticated: + raise Unauthenticated() + if not owner_obj: + raise ValidationError("Owner not found") + if not current_user_part_of_org(self.current_owner, owner_obj): + raise Unauthorized() + + def create_setup_intent(self, owner_obj: Owner) -> stripe.SetupIntent: + try: + billing = BillingService(requesting_user=self.current_owner) + return billing.create_setup_intent(owner_obj) + except Exception as e: + log.error( + f"Error getting setup intent for owner {owner_obj.ownerid}", + extra={"error": str(e)}, + ) + raise ValidationError("Unable to create setup intent") + + @sync_to_async + def execute(self, owner: str) -> stripe.SetupIntent: + owner_obj = Owner.objects.filter(username=owner, service=self.service).first() + self.validate(owner_obj) + return self.create_setup_intent(owner_obj) diff --git a/codecov_auth/commands/owner/owner.py b/codecov_auth/commands/owner/owner.py index 2b97f5017a..53b21a9577 100644 --- a/codecov_auth/commands/owner/owner.py +++ b/codecov_auth/commands/owner/owner.py @@ -2,6 +2,7 @@ from .interactors.cancel_trial import CancelTrialInteractor from .interactors.create_api_token import CreateApiTokenInteractor +from .interactors.create_stripe_setup_intent import CreateStripeSetupIntentInteractor from .interactors.create_user_token import CreateUserTokenInteractor from .interactors.delete_session import DeleteSessionInteractor from .interactors.fetch_owner import FetchOwnerInteractor @@ -28,6 +29,9 @@ class OwnerCommands(BaseCommand): def create_api_token(self, name): return self.get_interactor(CreateApiTokenInteractor).execute(name) + def create_stripe_setup_intent(self, owner): + return self.get_interactor(CreateStripeSetupIntentInteractor).execute(owner) + def delete_session(self, sessionid: int): return self.get_interactor(DeleteSessionInteractor).execute(sessionid) diff --git a/graphql_api/tests/mutation/test_create_stripe_setup_intent.py b/graphql_api/tests/mutation/test_create_stripe_setup_intent.py new file mode 100644 index 0000000000..9bbef0dfe6 --- /dev/null +++ b/graphql_api/tests/mutation/test_create_stripe_setup_intent.py @@ -0,0 +1,34 @@ +from unittest.mock import patch +from django.test import TransactionTestCase +from shared.django_apps.core.tests.factories import OwnerFactory + +from codecov_auth.models import Session +from graphql_api.tests.helper import GraphQLTestHelper + +query = """ +mutation($input: CreateStripeSetupIntentInput!) { + createStripeSetupIntent(input: $input) { + error { + __typename + } + clientSecret + } +} +""" + + +class CreateStripeSetupIntentTestCase(GraphQLTestHelper, TransactionTestCase): + def setUp(self): + self.owner = OwnerFactory(username="codecov-user") + + def test_when_unauthenticated(self): + data = self.gql_request(query, variables={"input": {"owner": "somename"}}) + assert data["createStripeSetupIntent"]["error"]["__typename"] == "UnauthenticatedError" + + @patch("services.billing.stripe.SetupIntent.create") + def test_when_authenticated(self, setup_intent_create_mock): + setup_intent_create_mock.return_value = {"client_secret": "test-client-secret"} + data = self.gql_request( + query, owner=self.owner, variables={"input": {"owner": self.owner.username}} + ) + assert data["createStripeSetupIntent"]["clientSecret"] == "test-client-secret" diff --git a/graphql_api/types/inputs/create_stripe_setup_intent.graphql b/graphql_api/types/inputs/create_stripe_setup_intent.graphql new file mode 100644 index 0000000000..5fe147ba5b --- /dev/null +++ b/graphql_api/types/inputs/create_stripe_setup_intent.graphql @@ -0,0 +1,3 @@ +input CreateStripeSetupIntentInput { + owner: String! +} diff --git a/graphql_api/types/invoice/invoice.graphql b/graphql_api/types/invoice/invoice.graphql index c7ad1e8265..ef11d76607 100644 --- a/graphql_api/types/invoice/invoice.graphql +++ b/graphql_api/types/invoice/invoice.graphql @@ -34,6 +34,7 @@ type Period { type PaymentMethod { billingDetails: BillingDetails card: Card + usBankAccount: USBankAccount } type Card { @@ -43,12 +44,9 @@ type Card { last4: String } -type BankAccount { - accountHolderType: String - accountType: String +type USBankAccount { bankName: String last4: String - routingNumber: String } type BillingDetails { diff --git a/graphql_api/types/mutation/__init__.py b/graphql_api/types/mutation/__init__.py index 244e529cc9..d5da48d7f4 100644 --- a/graphql_api/types/mutation/__init__.py +++ b/graphql_api/types/mutation/__init__.py @@ -3,6 +3,7 @@ from .activate_measurements import gql_activate_measurements from .cancel_trial import gql_cancel_trial from .create_api_token import gql_create_api_token +from .create_stripe_setup_intent import gql_create_stripe_setup_intent from .create_user_token import gql_create_user_token from .delete_component_measurements import gql_delete_component_measurements from .delete_flag import gql_delete_flag @@ -30,6 +31,7 @@ mutation = ariadne_load_local_graphql(__file__, "mutation.graphql") mutation = mutation + gql_create_api_token +mutation = mutation + gql_create_stripe_setup_intent mutation = mutation + gql_sync_with_git_provider mutation = mutation + gql_delete_session mutation = mutation + gql_set_yaml_on_owner diff --git a/graphql_api/types/mutation/create_stripe_setup_intent/__init__.py b/graphql_api/types/mutation/create_stripe_setup_intent/__init__.py new file mode 100644 index 0000000000..967ae52579 --- /dev/null +++ b/graphql_api/types/mutation/create_stripe_setup_intent/__init__.py @@ -0,0 +1,12 @@ +from graphql_api.helpers.ariadne import ariadne_load_local_graphql + +from .create_stripe_setup_intent import ( + error_create_stripe_setup_intent, + resolve_create_stripe_setup_intent, +) + +gql_create_stripe_setup_intent = ariadne_load_local_graphql( + __file__, "create_stripe_setup_intent.graphql" +) + +__all__ = ["error_create_stripe_setup_intent", "resolve_create_stripe_setup_intent"] diff --git a/graphql_api/types/mutation/create_stripe_setup_intent/create_stripe_setup_intent.graphql b/graphql_api/types/mutation/create_stripe_setup_intent/create_stripe_setup_intent.graphql new file mode 100644 index 0000000000..f103cce860 --- /dev/null +++ b/graphql_api/types/mutation/create_stripe_setup_intent/create_stripe_setup_intent.graphql @@ -0,0 +1,6 @@ +union CreateStripeSetupIntentError = UnauthenticatedError | ValidationError + +type CreateStripeSetupIntentPayload { + error: CreateStripeSetupIntentError + clientSecret: String +} diff --git a/graphql_api/types/mutation/create_stripe_setup_intent/create_stripe_setup_intent.py b/graphql_api/types/mutation/create_stripe_setup_intent/create_stripe_setup_intent.py new file mode 100644 index 0000000000..57e6df5c0d --- /dev/null +++ b/graphql_api/types/mutation/create_stripe_setup_intent/create_stripe_setup_intent.py @@ -0,0 +1,24 @@ +from typing import Dict + +from ariadne import UnionType +from ariadne.types import GraphQLResolveInfo + +from graphql_api.helpers.mutation import ( + resolve_union_error_type, + wrap_error_handling_mutation, +) + + +@wrap_error_handling_mutation +async def resolve_create_stripe_setup_intent( + _, info: GraphQLResolveInfo, input: Dict[str, any] +) -> Dict[str, any]: + command = info.context["executor"].get_command("owner") + resp = await command.create_stripe_setup_intent(input.get("owner")) + return { + "client_secret": resp["client_secret"], + } + + +error_create_stripe_setup_intent = UnionType("CreateStripeSetupIntentError") +error_create_stripe_setup_intent.type_resolver(resolve_union_error_type) diff --git a/graphql_api/types/mutation/mutation.graphql b/graphql_api/types/mutation/mutation.graphql index e8ab068efd..9bab9d6b2d 100644 --- a/graphql_api/types/mutation/mutation.graphql +++ b/graphql_api/types/mutation/mutation.graphql @@ -1,5 +1,6 @@ type Mutation { createApiToken(input: CreateApiTokenInput!): CreateApiTokenPayload + createStripeSetupIntent(input: CreateStripeSetupIntentInput!): CreateStripeSetupIntentPayload createUserToken(input: CreateUserTokenInput!): CreateUserTokenPayload revokeUserToken(input: RevokeUserTokenInput!): RevokeUserTokenPayload setYamlOnOwner(input: SetYamlOnOwnerInput!): SetYamlOnOwnerPayload diff --git a/graphql_api/types/mutation/mutation.py b/graphql_api/types/mutation/mutation.py index 1b487432f1..2a47c64138 100644 --- a/graphql_api/types/mutation/mutation.py +++ b/graphql_api/types/mutation/mutation.py @@ -6,6 +6,10 @@ ) from .cancel_trial import error_cancel_trial, resolve_cancel_trial from .create_api_token import error_create_api_token, resolve_create_api_token +from .create_stripe_setup_intent import ( + error_create_stripe_setup_intent, + resolve_create_stripe_setup_intent, +) from .create_user_token import error_create_user_token, resolve_create_user_token from .delete_component_measurements import ( error_delete_component_measurements, @@ -64,6 +68,7 @@ # Here, bind the resolvers from each subfolder to the Mutation type mutation_bindable.field("createApiToken")(resolve_create_api_token) +mutation_bindable.field("createStripeSetupIntent")(resolve_create_stripe_setup_intent) mutation_bindable.field("createUserToken")(resolve_create_user_token) mutation_bindable.field("revokeUserToken")(resolve_revoke_user_token) mutation_bindable.field("setYamlOnOwner")(resolve_set_yaml_on_owner) @@ -103,6 +108,7 @@ mutation_resolvers = [ mutation_bindable, error_create_api_token, + error_create_stripe_setup_intent, error_create_user_token, error_revoke_user_token, error_set_yaml_error, diff --git a/services/billing.py b/services/billing.py index 28414549d5..ea0d66fb6c 100644 --- a/services/billing.py +++ b/services/billing.py @@ -59,10 +59,6 @@ def modify_subscription(self, owner, plan): def create_checkout_session(self, owner, plan): pass - @abstractmethod - def create_setup_intent(self, owner): - pass - @abstractmethod def get_subscription(self, owner): pass @@ -87,6 +83,10 @@ def get_schedule(self, owner): def apply_cancellation_discount(self, owner: Owner): pass + @abstractmethod + def create_setup_intent(self, owner): + pass + class StripeService(AbstractPaymentService): def __init__(self, requesting_user): @@ -543,23 +543,6 @@ def create_checkout_session(self, owner: Owner, desired_plan): ) return session["id"] - @_log_stripe_error - def create_setup_intent(self, owner: Owner): - log.info( - "Stripe create setup intent for owner", - extra=dict( - owner_id=owner.ownerid, - user_id=self.requesting_user.ownerid, - subscription_id=owner.stripe_subscription_id, - customer_id=owner.stripe_customer_id, - ), - ) - setup_intent = stripe.SetupIntent.create( - payment_method_types=['card', 'us_bank_account'], - customer=owner.stripe_customer_id, - ) - return setup_intent.client_secret - @_log_stripe_error def update_payment_method(self, owner: Owner, payment_method): log.info( @@ -706,6 +689,21 @@ def apply_cancellation_discount(self, owner: Owner): coupon=owner.stripe_coupon_id, ) + @_log_stripe_error + def create_setup_intent(self, owner: Owner) -> stripe.SetupIntent: + log.info( + "Stripe create setup intent for owner", + extra=dict( + owner_id=owner.ownerid, + user_id=self.requesting_user.ownerid, + subscription_id=owner.stripe_subscription_id, + customer_id=owner.stripe_customer_id, + ), + ) + return stripe.SetupIntent.create( + payment_method_types=['card', 'us_bank_account'], + customer=owner.stripe_customer_id, + ) class EnterprisePaymentService(AbstractPaymentService): # enterprise has no payments setup so these are all noops @@ -743,6 +741,9 @@ def get_schedule(self, owner): def apply_cancellation_discount(self, owner: Owner): pass + def create_setup_intent(self, owner): + pass + class BillingService: payment_service = None @@ -764,9 +765,6 @@ def __init__(self, payment_service=None, requesting_user=None): def get_subscription(self, owner): return self.payment_service.get_subscription(owner) - def get_setup_intent(self, owner): - return self.payment_service.create_setup_intent(owner) - def get_schedule(self, owner): return self.payment_service.get_schedule(owner) @@ -826,3 +824,9 @@ def update_billing_address(self, owner: Owner, name: str, billing_address): def apply_cancellation_discount(self, owner: Owner): return self.payment_service.apply_cancellation_discount(owner) + + def create_setup_intent(self, owner: Owner): + """ + Creates a SetupIntent for the given owner to securely collect payment details + """ + return self.payment_service.create_setup_intent(owner) diff --git a/services/tests/test_billing.py b/services/tests/test_billing.py index eaed0adb6b..832744ea83 100644 --- a/services/tests/test_billing.py +++ b/services/tests/test_billing.py @@ -1512,7 +1512,7 @@ def test_create_checkout_session_with_no_stripe_customer_id( create_checkout_session_mock.assert_called_once_with( billing_address_collection="required", - payment_method_types=["card"], + payment_method_configuration=settings.STRIPE_PAYMENT_METHOD_CONFIGURATION_ID, payment_method_collection="if_required", client_reference_id=str(owner.ownerid), customer=None, @@ -1538,7 +1538,6 @@ def test_create_checkout_session_with_no_stripe_customer_id( tax_id_collection={"enabled": True}, customer_update=None, ) - @patch("services.billing.stripe.checkout.Session.create") def test_create_checkout_session_with_stripe_customer_id( self, create_checkout_session_mock @@ -1560,7 +1559,7 @@ def test_create_checkout_session_with_stripe_customer_id( create_checkout_session_mock.assert_called_once_with( billing_address_collection="required", - payment_method_types=["card"], + payment_method_configuration=settings.STRIPE_PAYMENT_METHOD_CONFIGURATION_ID, payment_method_collection="if_required", client_reference_id=str(owner.ownerid), customer=owner.stripe_customer_id, @@ -1825,14 +1824,12 @@ def test_apply_cancellation_discount_existing_coupon( assert not customer_modify_mock.called assert not coupon_create_mock.called - @patch("services.billing.stripe.SetupIntent.create") - def test_get_setup_intent(self, setup_intent_create_mock): + @patch("services.billing.stripe.SetupIntent.create") + def test_create_setup_intent(self, setup_intent_create_mock): owner = OwnerFactory(stripe_customer_id="test-customer-id") setup_intent_create_mock.return_value = {"client_secret": "test-client-secret"} - resp = self.stripe.get_setup_intent(owner) - self.stripe.payment_service.get_setup_intent.assert_called_once_with(owner) - - assert resp.client_secret == "test-client-secret" + resp = self.stripe.create_setup_intent(owner) + assert resp["client_secret"] == "test-client-secret" class MockPaymentService(AbstractPaymentService): @@ -1869,6 +1866,9 @@ def get_schedule(self, owner): def apply_cancellation_discount(self, owner): pass + def create_setup_intent(self, owner): + pass + class BillingServiceTests(TestCase): def setUp(self): @@ -2023,8 +2023,8 @@ def test_update_payment_method(self, get_subscription_mock): @patch("services.tests.test_billing.MockPaymentService.update_email_address") def test_email_address(self, get_subscription_mock): owner = OwnerFactory() - self.billing_service.update_email_address(owner, "test@gmail.com") - get_subscription_mock.assert_called_once_with(owner, "test@gmail.com") + self.billing_service.update_email_address(owner, "test@gmail.com", False) + get_subscription_mock.assert_called_once_with(owner, "test@gmail.com", False) @patch("services.tests.test_billing.MockPaymentService.get_invoice") def test_get_invoice(self, get_invoice_mock): From 5301ad62bc27dffdff6a44452d9b955d42b23488 Mon Sep 17 00:00:00 2001 From: Suejung Shin Date: Mon, 13 Jan 2025 19:14:22 -0800 Subject: [PATCH 4/7] cleanup and add tests --- api/internal/owner/views.py | 23 ----------- .../tests/views/test_account_viewset.py | 40 +++++++++++++++++++ .../interactors/create_stripe_setup_intent.py | 1 + .../test_create_stripe_setup_intent.py | 39 ++++++++++++++++-- .../create_stripe_setup_intent.graphql | 2 +- .../create_stripe_setup_intent.py | 6 +-- services/billing.py | 23 ++++++++--- services/tests/test_billing.py | 2 +- 8 files changed, 100 insertions(+), 36 deletions(-) diff --git a/api/internal/owner/views.py b/api/internal/owner/views.py index 90116d4e32..71f8933794 100644 --- a/api/internal/owner/views.py +++ b/api/internal/owner/views.py @@ -131,29 +131,6 @@ def update_billing_address(self, request, *args, **kwargs): billing.update_billing_address(owner, name, billing_address=formatted_address) return Response(self.get_serializer(owner).data) - @action(detail=False, methods=["post"]) - @stripe_safe - def setup_intent(self, request, *args, **kwargs): - """ - Create a Stripe SetupIntent to securely collect payment details. - - Returns: - Response with SetupIntent client_secret for frontend payment method setup. - - Raises: - ValidationError: If SetupIntent creation fails - """ - try: - billing = BillingService(requesting_user=request.current_owner) - client_secret = billing.create_setup_intent(self.owner) - return Response({"client_secret": client_secret}) - except Exception as e: - log.error( - f"Error getting setup intent for owner {self.owner.ownerid}", - extra={"error": str(e)}, - ) - raise ValidationError(detail="Unable to create setup intent") - class UsersOrderingFilter(filters.OrderingFilter): def get_valid_fields(self, queryset, view, context=None): diff --git a/api/internal/tests/views/test_account_viewset.py b/api/internal/tests/views/test_account_viewset.py index 1aabeab654..98d283bc9e 100644 --- a/api/internal/tests/views/test_account_viewset.py +++ b/api/internal/tests/views/test_account_viewset.py @@ -1228,6 +1228,46 @@ def test_update_email_address(self, modify_customer_mock, retrieve_mock): self.current_owner.stripe_customer_id, email=new_email ) + @patch("services.billing.stripe.Subscription.retrieve") + @patch("services.billing.stripe.Customer.modify") + @patch("services.billing.stripe.PaymentMethod.modify") + @patch("services.billing.stripe.Customer.retrieve") + def test_update_email_address_with_propagate( + self, + customer_retrieve_mock, + payment_method_mock, + modify_customer_mock, + retrieve_mock, + ): + self.current_owner.stripe_customer_id = "flsoe" + self.current_owner.stripe_subscription_id = "djfos" + self.current_owner.save() + + payment_method_id = "pm_123" + customer_retrieve_mock.return_value = { + "invoice_settings": {"default_payment_method": payment_method_id} + } + + new_email = "test@gmail.com" + kwargs = { + "service": self.current_owner.service, + "owner_username": self.current_owner.username, + } + data = {"new_email": new_email, "should_propagate_to_payment_methods": True} + url = reverse("account_details-update-email", kwargs=kwargs) + response = self.client.patch(url, data=data, format="json") + assert response.status_code == status.HTTP_200_OK + + modify_customer_mock.assert_called_once_with( + self.current_owner.stripe_customer_id, email=new_email + ) + customer_retrieve_mock.assert_called_once_with( + self.current_owner.stripe_customer_id + ) + payment_method_mock.assert_called_once_with( + payment_method_id, billing_details={"email": new_email} + ) + def test_update_billing_address_without_body(self): kwargs = { "service": self.current_owner.service, diff --git a/codecov_auth/commands/owner/interactors/create_stripe_setup_intent.py b/codecov_auth/commands/owner/interactors/create_stripe_setup_intent.py index 3e9ba28b6f..5270488015 100644 --- a/codecov_auth/commands/owner/interactors/create_stripe_setup_intent.py +++ b/codecov_auth/commands/owner/interactors/create_stripe_setup_intent.py @@ -11,6 +11,7 @@ log = logging.getLogger(__name__) + class CreateStripeSetupIntentInteractor(BaseInteractor): def validate(self, owner_obj: Owner) -> None: if not self.current_user.is_authenticated: diff --git a/graphql_api/tests/mutation/test_create_stripe_setup_intent.py b/graphql_api/tests/mutation/test_create_stripe_setup_intent.py index 9bbef0dfe6..609d84f579 100644 --- a/graphql_api/tests/mutation/test_create_stripe_setup_intent.py +++ b/graphql_api/tests/mutation/test_create_stripe_setup_intent.py @@ -1,8 +1,8 @@ from unittest.mock import patch + from django.test import TransactionTestCase from shared.django_apps.core.tests.factories import OwnerFactory -from codecov_auth.models import Session from graphql_api.tests.helper import GraphQLTestHelper query = """ @@ -23,10 +23,43 @@ def setUp(self): def test_when_unauthenticated(self): data = self.gql_request(query, variables={"input": {"owner": "somename"}}) - assert data["createStripeSetupIntent"]["error"]["__typename"] == "UnauthenticatedError" + assert ( + data["createStripeSetupIntent"]["error"]["__typename"] + == "UnauthenticatedError" + ) + + def test_when_unauthorized(self): + other_owner = OwnerFactory(username="other-user") + data = self.gql_request( + query, + owner=self.owner, + variables={"input": {"owner": other_owner.username}}, + ) + assert ( + data["createStripeSetupIntent"]["error"]["__typename"] + == "UnauthorizedError" + ) + + @patch("services.billing.stripe.SetupIntent.create") + def test_when_validation_error(self, setup_intent_create_mock): + setup_intent_create_mock.side_effect = Exception("Some error") + data = self.gql_request( + query, owner=self.owner, variables={"input": {"owner": self.owner.username}} + ) + assert ( + data["createStripeSetupIntent"]["error"]["__typename"] == "ValidationError" + ) + + def test_when_owner_not_found(self): + data = self.gql_request( + query, owner=self.owner, variables={"input": {"owner": "nonexistent-user"}} + ) + assert ( + data["createStripeSetupIntent"]["error"]["__typename"] == "ValidationError" + ) @patch("services.billing.stripe.SetupIntent.create") - def test_when_authenticated(self, setup_intent_create_mock): + def test_success(self, setup_intent_create_mock): setup_intent_create_mock.return_value = {"client_secret": "test-client-secret"} data = self.gql_request( query, owner=self.owner, variables={"input": {"owner": self.owner.username}} diff --git a/graphql_api/types/mutation/create_stripe_setup_intent/create_stripe_setup_intent.graphql b/graphql_api/types/mutation/create_stripe_setup_intent/create_stripe_setup_intent.graphql index f103cce860..9f33c57626 100644 --- a/graphql_api/types/mutation/create_stripe_setup_intent/create_stripe_setup_intent.graphql +++ b/graphql_api/types/mutation/create_stripe_setup_intent/create_stripe_setup_intent.graphql @@ -1,4 +1,4 @@ -union CreateStripeSetupIntentError = UnauthenticatedError | ValidationError +union CreateStripeSetupIntentError = UnauthenticatedError | UnauthorizedError | ValidationError type CreateStripeSetupIntentPayload { error: CreateStripeSetupIntentError diff --git a/graphql_api/types/mutation/create_stripe_setup_intent/create_stripe_setup_intent.py b/graphql_api/types/mutation/create_stripe_setup_intent/create_stripe_setup_intent.py index 57e6df5c0d..0b1f0590e2 100644 --- a/graphql_api/types/mutation/create_stripe_setup_intent/create_stripe_setup_intent.py +++ b/graphql_api/types/mutation/create_stripe_setup_intent/create_stripe_setup_intent.py @@ -1,4 +1,4 @@ -from typing import Dict +from typing import Any, Dict from ariadne import UnionType from ariadne.types import GraphQLResolveInfo @@ -11,8 +11,8 @@ @wrap_error_handling_mutation async def resolve_create_stripe_setup_intent( - _, info: GraphQLResolveInfo, input: Dict[str, any] -) -> Dict[str, any]: + _: Any, info: GraphQLResolveInfo, input: Dict[str, str] +) -> Dict[str, str]: command = info.context["executor"].get_command("owner") resp = await command.create_stripe_setup_intent(input.get("owner")) return { diff --git a/services/billing.py b/services/billing.py index ea0d66fb6c..874e08fbb7 100644 --- a/services/billing.py +++ b/services/billing.py @@ -584,7 +584,12 @@ def update_payment_method(self, owner: Owner, payment_method): ) @_log_stripe_error - def update_email_address(self, owner: Owner, email_address: str, should_propagate_to_payment_methods: bool = False): + def update_email_address( + self, + owner: Owner, + email_address: str, + should_propagate_to_payment_methods: bool = False, + ): if not re.fullmatch(r"[^@]+@[^@]+\.[^@]+", email_address): return None @@ -603,7 +608,7 @@ def update_email_address(self, owner: Owner, email_address: str, should_propagat try: default_payment_method = stripe.Customer.retrieve( owner.stripe_customer_id - ).invoice_settings.default_payment_method + )["invoice_settings"]["default_payment_method"] stripe.PaymentMethod.modify( default_payment_method, @@ -701,10 +706,11 @@ def create_setup_intent(self, owner: Owner) -> stripe.SetupIntent: ), ) return stripe.SetupIntent.create( - payment_method_types=['card', 'us_bank_account'], + payment_method_configuration=settings.STRIPE_PAYMENT_METHOD_CONFIGURATION_ID, customer=owner.stripe_customer_id, ) + class EnterprisePaymentService(AbstractPaymentService): # enterprise has no payments setup so these are all noops @@ -805,14 +811,21 @@ def update_payment_method(self, owner, payment_method): """ return self.payment_service.update_payment_method(owner, payment_method) - def update_email_address(self, owner: Owner, email_address: str, should_propagate_to_payment_methods: bool = False): + def update_email_address( + self, + owner: Owner, + email_address: str, + should_propagate_to_payment_methods: bool = False, + ): """ Takes an owner and a new email. Email is a string coming directly from the front-end. If the owner has a payment id and if it's a valid email, the payment service will update the email address in the upstream service. Otherwise returns None. """ - return self.payment_service.update_email_address(owner, email_address, should_propagate_to_payment_methods) + return self.payment_service.update_email_address( + owner, email_address, should_propagate_to_payment_methods + ) def update_billing_address(self, owner: Owner, name: str, billing_address): """ diff --git a/services/tests/test_billing.py b/services/tests/test_billing.py index 832744ea83..a2edd6640a 100644 --- a/services/tests/test_billing.py +++ b/services/tests/test_billing.py @@ -1538,6 +1538,7 @@ def test_create_checkout_session_with_no_stripe_customer_id( tax_id_collection={"enabled": True}, customer_update=None, ) + @patch("services.billing.stripe.checkout.Session.create") def test_create_checkout_session_with_stripe_customer_id( self, create_checkout_session_mock @@ -2031,4 +2032,3 @@ def test_get_invoice(self, get_invoice_mock): owner = OwnerFactory() self.billing_service.get_invoice(owner, "abc") get_invoice_mock.assert_called_once_with(owner, "abc") - From 8ea30aced7748bc8a2ce547147fdef49759700ea Mon Sep 17 00:00:00 2001 From: Suejung Shin Date: Tue, 14 Jan 2025 13:43:49 -0800 Subject: [PATCH 5/7] incorporate PR comments --- api/internal/owner/views.py | 10 ++++++---- api/internal/tests/views/test_account_viewset.py | 2 +- codecov/settings_base.py | 2 +- .../interactors/create_stripe_setup_intent.py | 7 +++++-- services/billing.py | 14 ++++++++------ 5 files changed, 21 insertions(+), 14 deletions(-) diff --git a/api/internal/owner/views.py b/api/internal/owner/views.py index 71f8933794..74900ab2a0 100644 --- a/api/internal/owner/views.py +++ b/api/internal/owner/views.py @@ -86,7 +86,7 @@ def update_email(self, request, *args, **kwargs): Args: request: The HTTP request object containing: - new_email: The new email address to update to - - should_propagate_to_payment_methods: Optional boolean flag to update email on payment methods (default False) + - apply_to_default_payment_method: Boolean flag to update email on the default payment method (default False) Returns: Response with serialized owner data @@ -99,11 +99,13 @@ def update_email(self, request, *args, **kwargs): raise ValidationError(detail="No new_email sent") owner = self.get_object() billing = BillingService(requesting_user=request.current_owner) - should_propagate = request.data.get( - "should_propagate_to_payment_methods", False + apply_to_default_payment_method = request.data.get( + "apply_to_default_payment_method", False ) billing.update_email_address( - owner, new_email, should_propagate_to_payment_methods=should_propagate + owner, + new_email, + apply_to_default_payment_method=apply_to_default_payment_method, ) return Response(self.get_serializer(owner).data) diff --git a/api/internal/tests/views/test_account_viewset.py b/api/internal/tests/views/test_account_viewset.py index 98d283bc9e..c183cfef24 100644 --- a/api/internal/tests/views/test_account_viewset.py +++ b/api/internal/tests/views/test_account_viewset.py @@ -1253,7 +1253,7 @@ def test_update_email_address_with_propagate( "service": self.current_owner.service, "owner_username": self.current_owner.username, } - data = {"new_email": new_email, "should_propagate_to_payment_methods": True} + data = {"new_email": new_email, "apply_to_default_payment_method": True} url = reverse("account_details-update-email", kwargs=kwargs) response = self.client.patch(url, data=data, format="json") assert response.status_code == status.HTTP_200_OK diff --git a/codecov/settings_base.py b/codecov/settings_base.py index 73a4af3636..736d7c678d 100644 --- a/codecov/settings_base.py +++ b/codecov/settings_base.py @@ -430,7 +430,7 @@ SHELTER_PUBSUB_SYNC_REPO_TOPIC_ID = get_config("setup", "shelter", "sync_repo_topic_id") STRIPE_PAYMENT_METHOD_CONFIGURATION_ID = get_config( - "setup", "stripe", "payment_method_configuration", default=None + "setup", "stripe", "payment_method_configuration_id", default=None ) # Allows to do migrations from another module diff --git a/codecov_auth/commands/owner/interactors/create_stripe_setup_intent.py b/codecov_auth/commands/owner/interactors/create_stripe_setup_intent.py index 5270488015..0fc5598a67 100644 --- a/codecov_auth/commands/owner/interactors/create_stripe_setup_intent.py +++ b/codecov_auth/commands/owner/interactors/create_stripe_setup_intent.py @@ -27,8 +27,11 @@ def create_setup_intent(self, owner_obj: Owner) -> stripe.SetupIntent: return billing.create_setup_intent(owner_obj) except Exception as e: log.error( - f"Error getting setup intent for owner {owner_obj.ownerid}", - extra={"error": str(e)}, + "Error getting setup intent", + extra={ + "ownerid": owner_obj.ownerid, + "error": str(e), + }, ) raise ValidationError("Unable to create setup intent") diff --git a/services/billing.py b/services/billing.py index 874e08fbb7..412415cd49 100644 --- a/services/billing.py +++ b/services/billing.py @@ -588,7 +588,7 @@ def update_email_address( self, owner: Owner, email_address: str, - should_propagate_to_payment_methods: bool = False, + apply_to_default_payment_method: bool = False, ): if not re.fullmatch(r"[^@]+@[^@]+\.[^@]+", email_address): return None @@ -604,7 +604,7 @@ def update_email_address( f"Stripe successfully updated email address for owner {owner.ownerid} by user #{self.requesting_user.ownerid}" ) - if should_propagate_to_payment_methods: + if apply_to_default_payment_method: try: default_payment_method = stripe.Customer.retrieve( owner.stripe_customer_id @@ -617,11 +617,12 @@ def update_email_address( log.info( f"Stripe successfully updated billing email for payment method {default_payment_method}" ) - except Exception: + except Exception as e: log.error( "Unable to update billing email for payment method", extra=dict( payment_method=default_payment_method, + error=str(e), ), ) @@ -700,7 +701,7 @@ def create_setup_intent(self, owner: Owner) -> stripe.SetupIntent: "Stripe create setup intent for owner", extra=dict( owner_id=owner.ownerid, - user_id=self.requesting_user.ownerid, + requesting_user_id=self.requesting_user.ownerid, subscription_id=owner.stripe_subscription_id, customer_id=owner.stripe_customer_id, ), @@ -815,7 +816,7 @@ def update_email_address( self, owner: Owner, email_address: str, - should_propagate_to_payment_methods: bool = False, + apply_to_default_payment_method: bool = False, ): """ Takes an owner and a new email. Email is a string coming directly from @@ -824,7 +825,7 @@ def update_email_address( Otherwise returns None. """ return self.payment_service.update_email_address( - owner, email_address, should_propagate_to_payment_methods + owner, email_address, apply_to_default_payment_method ) def update_billing_address(self, owner: Owner, name: str, billing_address): @@ -841,5 +842,6 @@ def apply_cancellation_discount(self, owner: Owner): def create_setup_intent(self, owner: Owner): """ Creates a SetupIntent for the given owner to securely collect payment details + See https://docs.stripe.com/api/setup_intents/create """ return self.payment_service.create_setup_intent(owner) From 530136ca3e3dfa3a6aff6ca77d586741682bd032 Mon Sep 17 00:00:00 2001 From: Suejung Shin Date: Tue, 14 Jan 2025 16:53:55 -0800 Subject: [PATCH 6/7] add logs --- services/billing.py | 9 ++++++++- 1 file changed, 8 insertions(+), 1 deletion(-) diff --git a/services/billing.py b/services/billing.py index 412415cd49..e720f22c7e 100644 --- a/services/billing.py +++ b/services/billing.py @@ -615,14 +615,21 @@ def update_email_address( billing_details={"email": email_address}, ) log.info( - f"Stripe successfully updated billing email for payment method {default_payment_method}" + f"Stripe successfully updated billing email for payment method", + extra=dict( + payment_method=default_payment_method, + stripe_customer_id=owner.stripe_customer_id, + ownerid=owner.ownerid, + ), ) except Exception as e: log.error( "Unable to update billing email for payment method", extra=dict( payment_method=default_payment_method, + stripe_customer_id=owner.stripe_customer_id, error=str(e), + ownerid=owner.ownerid, ), ) From 3567fbbe35d6e2e8f3680fb2501cb3b4dc098802 Mon Sep 17 00:00:00 2001 From: Suejung Shin Date: Wed, 15 Jan 2025 16:37:35 -0800 Subject: [PATCH 7/7] fix lint --- services/billing.py | 2 +- utils/test_utils.py | 8 ++++---- 2 files changed, 5 insertions(+), 5 deletions(-) diff --git a/services/billing.py b/services/billing.py index e720f22c7e..825cd7ccab 100644 --- a/services/billing.py +++ b/services/billing.py @@ -615,7 +615,7 @@ def update_email_address( billing_details={"email": email_address}, ) log.info( - f"Stripe successfully updated billing email for payment method", + "Stripe successfully updated billing email for payment method", extra=dict( payment_method=default_payment_method, stripe_customer_id=owner.stripe_customer_id, diff --git a/utils/test_utils.py b/utils/test_utils.py index 6cac27e04e..6deeb8251a 100644 --- a/utils/test_utils.py +++ b/utils/test_utils.py @@ -45,10 +45,10 @@ def app(self) -> str: migrate_to = None def setUp(self) -> None: - assert self.migrate_from and self.migrate_to, ( - "TestCase '{}' must define migrate_from and migrate_to properties".format( - type(self).__name__ - ) + assert ( + self.migrate_from and self.migrate_to + ), "TestCase '{}' must define migrate_from and migrate_to properties".format( + type(self).__name__ ) self.migrate_from = [(self.app, self.migrate_from)] self.migrate_to = [(self.app, self.migrate_to)]