From fd0f9254fc99f69a9eb8dd7060fa6159ffa0c2d1 Mon Sep 17 00:00:00 2001 From: Suejung Shin Date: Mon, 13 Jan 2025 19:14:22 -0800 Subject: [PATCH] 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 +- services/billing.py | 23 ++++++++--- services/tests/test_billing.py | 2 +- 7 files changed, 97 insertions(+), 33 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/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") -