From b18e4a6d6588e80be4575de1891d51f10040ebef Mon Sep 17 00:00:00 2001 From: Zach Aysan Date: Mon, 13 Nov 2023 08:14:06 -0500 Subject: [PATCH] fix: Handle payment errors during user flow (#2951) Co-authored-by: Matthew Elwell --- api/organisations/chargebee/chargebee.py | 20 +++++- api/organisations/invites/tests/test_views.py | 64 ++++++++++++++++++- api/organisations/subscriptions/exceptions.py | 8 +++ .../test_unit_chargebee_chargebee.py | 20 +++--- 4 files changed, 99 insertions(+), 13 deletions(-) diff --git a/api/organisations/chargebee/chargebee.py b/api/organisations/chargebee/chargebee.py index bae610e51188..e67d622a0fb4 100644 --- a/api/organisations/chargebee/chargebee.py +++ b/api/organisations/chargebee/chargebee.py @@ -12,6 +12,7 @@ from ..subscriptions.exceptions import ( CannotCancelChargebeeSubscription, UpgradeSeatsError, + UpgradeSeatsPaymentFailure, ) from .cache import ChargebeeCache from .constants import ADDITIONAL_SEAT_ADDON_ID @@ -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) @@ -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 @@ -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 diff --git a/api/organisations/invites/tests/test_views.py b/api/organisations/invites/tests/test_views.py index ee70fa0de140..50766db77887 100644 --- a/api/organisations/invites/tests/test_views.py +++ b/api/organisations/invites/tests/test_views.py @@ -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 @@ -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) @@ -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 ): diff --git a/api/organisations/subscriptions/exceptions.py b/api/organisations/subscriptions/exceptions.py index 35b3c3e75f48..1b24e20bd007 100644 --- a/api/organisations/subscriptions/exceptions.py +++ b/api/organisations/subscriptions/exceptions.py @@ -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" diff --git a/api/tests/unit/organisations/chargebee/test_unit_chargebee_chargebee.py b/api/tests/unit/organisations/chargebee/test_unit_chargebee_chargebee.py index efa3770adf3b..45a4c8460acd 100644 --- a/api/tests/unit/organisations/chargebee/test_unit_chargebee_chargebee.py +++ b/api/tests/unit/organisations/chargebee/test_unit_chargebee_chargebee.py @@ -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"