Skip to content

Commit

Permalink
fix: Handle payment errors during user flow (#2951)
Browse files Browse the repository at this point in the history
Co-authored-by: Matthew Elwell <matthew.elwell@flagsmith.com>
  • Loading branch information
zachaysan and matthewelwell authored Nov 13, 2023
1 parent a056713 commit b18e4a6
Show file tree
Hide file tree
Showing 4 changed files with 99 additions and 13 deletions.
20 changes: 19 additions & 1 deletion api/organisations/chargebee/chargebee.py
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@
from ..subscriptions.exceptions import (
CannotCancelChargebeeSubscription,
UpgradeSeatsError,
UpgradeSeatsPaymentFailure,
)
from .cache import ChargebeeCache
from .constants import ADDITIONAL_SEAT_ADDON_ID
Expand All @@ -21,6 +22,15 @@

logger = logging.getLogger(__name__)

CHARGEBEE_PAYMENT_ERROR_CODES = [
"payment_processing_failed",
"payment_method_verification_failed",
"payment_method_not_present",
"payment_gateway_currency_incompatible",
"payment_intent_invalid",
"payment_intent_invalid_amount",
]


def get_subscription_data_from_hosted_page(hosted_page_id):
hosted_page = get_hosted_page(hosted_page_id)
Expand Down Expand Up @@ -155,7 +165,6 @@ def add_single_seat(subscription_id: str):
try:
subscription = chargebee.Subscription.retrieve(subscription_id).subscription
addons = subscription.addons or []

current_seats = next(
(
addon.quantity
Expand All @@ -177,6 +186,15 @@ def add_single_seat(subscription_id: str):
)

except ChargebeeAPIError as e:
api_error_code = e.json_obj["api_error_code"]
if api_error_code in CHARGEBEE_PAYMENT_ERROR_CODES:
logger.warning(
f"Payment declined ({api_error_code}) during additional "
f"seat upgrade to a CB subscription for subscription_id "
f"{subscription_id}"
)
raise UpgradeSeatsPaymentFailure() from e

msg = (
"Failed to add additional seat to CB subscription for subscription id: %s"
% subscription_id
Expand Down
64 changes: 61 additions & 3 deletions api/organisations/invites/tests/test_views.py
Original file line number Diff line number Diff line change
@@ -1,16 +1,20 @@
import json
import typing
from datetime import timedelta

import pytest
from chargebee import APIError as ChargebeeAPIError
from django.conf import settings
from django.urls import reverse
from django.utils import timezone
from pytest_django.fixtures import SettingsWrapper
from pytest_lazyfixture import lazy_fixture
from pytest_mock.plugin import MockerFixture
from rest_framework import status
from rest_framework.test import APITestCase
from rest_framework.test import APIClient, APITestCase

from organisations.invites.models import Invite, InviteLink
from organisations.models import Organisation, OrganisationRole
from organisations.models import Organisation, OrganisationRole, Subscription
from users.models import FFAdminUser


Expand Down Expand Up @@ -282,7 +286,6 @@ def test_join_organisation_returns_400_if_exceeds_plan_limit(
settings.ENABLE_CHARGEBEE = True
settings.AUTO_SEAT_UPGRADE_PLANS = ["scale-up"]
url = reverse(url, args=[invite_object.hash])

# When
response = test_user_client.post(url)

Expand All @@ -294,6 +297,61 @@ def test_join_organisation_returns_400_if_exceeds_plan_limit(
)


@pytest.mark.parametrize(
"invite_object, url",
[
(lazy_fixture("invite"), "api-v1:users:user-join-organisation"),
(lazy_fixture("invite_link"), "api-v1:users:user-join-organisation-link"),
],
)
def test_join_organisation_returns_400_if_payment_fails(
test_user_client: APIClient,
organisation: Organisation,
admin_user: FFAdminUser,
invite_object: typing.Union[Invite, InviteLink],
url: str,
subscription: Subscription,
settings: SettingsWrapper,
mocker: MockerFixture,
):
# Given
settings.ENABLE_CHARGEBEE = True
settings.AUTO_SEAT_UPGRADE_PLANS = ["scale-up"]

url = reverse(url, args=[invite_object.hash])

subscription.plan = "scale-up"
subscription.subscription_id = "chargemepls"
subscription.save()

mocked_cb_subscription = mocker.MagicMock(addons=[])

mocked_chargebee = mocker.patch("organisations.chargebee.chargebee.chargebee")
mocked_chargebee.Subscription.retrieve.return_value = mocked_cb_subscription

chargebee_response_data = {
"message": "Subscription cannot be created as the payment collection failed. Gateway Error: Card declined.",
"type": "payment",
"api_error_code": "payment_processing_failed",
"param": "item_id",
"error_code": "DeprecatedField",
}

mocked_chargebee.Subscription.update.side_effect = ChargebeeAPIError(
http_code=400, json_obj=chargebee_response_data
)

# When
response = test_user_client.post(url)

# Then
assert response.status_code == status.HTTP_400_BAD_REQUEST
assert (
response.json()["detail"]
== "Joining the organisation has failed due to a payment issue. Please contact your organisation's admin."
)


def test_join_organisation_from_link_returns_403_if_invite_links_disabled(
test_user_client, organisation, invite_link, settings
):
Expand Down
8 changes: 8 additions & 0 deletions api/organisations/subscriptions/exceptions.py
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,14 @@ class UpgradeSeatsError(APIException):
default_detail = "Failed to upgrade seats in Chargebee"


class UpgradeSeatsPaymentFailure(APIException):
status_code = 400
default_detail = (
"Joining the organisation has failed due to a payment issue. "
"Please contact your organisation's admin."
)


class SubscriptionDoesNotSupportSeatUpgrade(APIException):
status_code = 400
default_detail = "Please Upgrade your plan to add additional seats/users"
Original file line number Diff line number Diff line change
Expand Up @@ -518,17 +518,19 @@ def test_add_single_seat_without_existing_addon(mocker):

def test_add_single_seat_throws_upgrade_seats_error_error_if_api_error(mocker, caplog):
# Given

# Chargebee's APIError requires additional arguments to instantiate it so instead
# we mock it with our own exception here to test that it is caught correctly
class MockException(Exception):
pass

mocked_chargebee = mocker.patch("organisations.chargebee.chargebee.chargebee")

mocker.patch("organisations.chargebee.chargebee.ChargebeeAPIError", MockException)

mocked_chargebee.Subscription.update.side_effect = MockException
# Typical non-payment related error from Chargebee.
chargebee_response_data = {
"message": "82sa2Sqa5 not found",
"type": "invalid_request",
"api_error_code": "resource_not_found",
"param": "item_id",
"error_code": "DeprecatedField",
}
mocked_chargebee.Subscription.update.side_effect = APIError(
http_code=404, json_obj=chargebee_response_data
)

# Let's create a (mocked) subscription object
subscription_id = "sub-id"
Expand Down

3 comments on commit b18e4a6

@vercel
Copy link

@vercel vercel bot commented on b18e4a6 Nov 13, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@vercel
Copy link

@vercel vercel bot commented on b18e4a6 Nov 13, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@vercel
Copy link

@vercel vercel bot commented on b18e4a6 Nov 13, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Successfully deployed to the following URLs:

docs – ./docs

docs-flagsmith.vercel.app
docs-git-main-flagsmith.vercel.app
docs.bullet-train.io
docs.flagsmith.com

Please sign in to comment.