Skip to content

Commit

Permalink
cleanup and add tests
Browse files Browse the repository at this point in the history
  • Loading branch information
suejung-sentry committed Jan 14, 2025
1 parent 198f23e commit fd0f925
Show file tree
Hide file tree
Showing 7 changed files with 97 additions and 33 deletions.
23 changes: 0 additions & 23 deletions api/internal/owner/views.py
Original file line number Diff line number Diff line change
Expand Up @@ -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):
Expand Down
40 changes: 40 additions & 0 deletions api/internal/tests/views/test_account_viewset.py
Original file line number Diff line number Diff line change
Expand Up @@ -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 = "[email protected]"
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,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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:
Expand Down
39 changes: 36 additions & 3 deletions graphql_api/tests/mutation/test_create_stripe_setup_intent.py
Original file line number Diff line number Diff line change
@@ -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 = """
Expand All @@ -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}}
Expand Down
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
union CreateStripeSetupIntentError = UnauthenticatedError | ValidationError
union CreateStripeSetupIntentError = UnauthenticatedError | UnauthorizedError | ValidationError

type CreateStripeSetupIntentPayload {
error: CreateStripeSetupIntentError
Expand Down
23 changes: 18 additions & 5 deletions services/billing.py
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand All @@ -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,
Expand Down Expand Up @@ -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

Expand Down Expand Up @@ -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):
"""
Expand Down
2 changes: 1 addition & 1 deletion services/tests/test_billing.py
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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")

0 comments on commit fd0f925

Please sign in to comment.