Skip to content

Commit

Permalink
incorporate PR comments
Browse files Browse the repository at this point in the history
  • Loading branch information
suejung-sentry committed Jan 14, 2025
1 parent 21a7792 commit 8ea30ac
Show file tree
Hide file tree
Showing 5 changed files with 21 additions and 14 deletions.
10 changes: 6 additions & 4 deletions api/internal/owner/views.py
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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)

Expand Down
2 changes: 1 addition & 1 deletion api/internal/tests/views/test_account_viewset.py
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
2 changes: 1 addition & 1 deletion codecov/settings_base.py
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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")

Expand Down
14 changes: 8 additions & 6 deletions services/billing.py
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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
Expand All @@ -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(

Check warning on line 621 in services/billing.py

View check run for this annotation

Codecov Notifications / codecov/patch

services/billing.py#L620-L621

Added lines #L620 - L621 were not covered by tests
"Unable to update billing email for payment method",
extra=dict(
payment_method=default_payment_method,
error=str(e),
),
)

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

0 comments on commit 8ea30ac

Please sign in to comment.