From 8ea30aced7748bc8a2ce547147fdef49759700ea Mon Sep 17 00:00:00 2001 From: Suejung Shin Date: Tue, 14 Jan 2025 13:43:49 -0800 Subject: [PATCH] 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)