From 198f23ecdf46f660d4841f252f1cb0f1c000bef5 Mon Sep 17 00:00:00 2001 From: Suejung Shin Date: Mon, 13 Jan 2025 18:38:41 -0800 Subject: [PATCH] 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):