Payment: no retry, and better exception handling

app/core/payment/payment_tool.py

@@ -1,6 +1,6 @@
 import logging
 import uuid
-from datetime import UTC, datetime
+from datetime import UTC, date, datetime
 from typing import TYPE_CHECKING
 
 from authlib.integrations.requests_client import OAuth2Session
@@ -19,7 +19,6 @@
 
 from app.core.payment import cruds_payment, models_payment, schemas_payment
 from app.core.payment.types_payment import HelloAssoConfig
-from app.core.users import schemas_users
 from app.core.utils import security
 from app.types.exceptions import (
     MissingHelloAssoCheckoutIdError,
@@ -130,7 +129,10 @@
         checkout_amount: int,
         checkout_name: str,
         db: AsyncSession,
-        payer_user: schemas_users.CoreUser | None = None,
+        payer_firstname: str,
+        payer_name: str,
+        payer_email: str,
+        payer_birthday: date | None = None,
         redirection_uri: str | None = None,
     ) -> schemas_payment.Checkout:
         """
@@ -152,23 +154,22 @@
         This method use HelloAsso API. It may raise exceptions if HA checkout initialization fails.
         Exceptions can be imported from `helloasso_python` package.
         """
-        configuration = self.get_hello_asso_configuration()
 
         redirection_uri = redirection_uri or self._redirection_uri
         if not redirection_uri:
             raise UnsetRedirectionUriError
 
         # We want to ensure that any error is logged, even if modules tries to try/except this method
         # Thus we catch any exception and log it, then reraise it
+        exception_start = f"Payment: failed to init a checkout with HA for module {module} and name {checkout_name} for payer {payer_firstname} {payer_name}"
+
         try:
-            payer: HelloAssoApiV5ModelsCartsCheckoutPayer | None = None
-            if payer_user:
-                payer = HelloAssoApiV5ModelsCartsCheckoutPayer(
-                    firstName=payer_user.firstname,
-                    lastName=payer_user.name,
-                    email=payer_user.email,
-                    dateOfBirth=payer_user.birthday,
-                )
+            payer = HelloAssoApiV5ModelsCartsCheckoutPayer(
+                firstName=payer_firstname,
+                lastName=payer_name,
+                email=payer_email,
+                dateOfBirth=payer_birthday,
+            )
 
             checkout_model_id = uuid.uuid4()
             secret = security.generate_token(nbytes=12)
@@ -188,35 +189,36 @@
                 ).model_dump(),
             )
 
-            response: HelloAssoApiV5ModelsCartsInitCheckoutResponse
+            response: HelloAssoApiV5ModelsCartsInitCheckoutResponse | None = None
+            configuration = self.get_hello_asso_configuration()
             with ApiClient(configuration) as api_client:
                 checkout_api = CheckoutApi(api_client)
-                try:
-                    response = checkout_api.organizations_organization_slug_checkout_intents_post(
+                response = (
+                    checkout_api.organizations_organization_slug_checkout_intents_post(
                         self._helloasso_slug,
                         init_checkout_body,
                     )
                 except (UnauthorizedException, BadRequestException):
                     # We know that HelloAsso may refuse some payer infos, like using the firstname "test"
                     # Even when prefilling the payer infos,the user will be able to edit them on the payment page,
                     # so we can safely retry without the payer infos
                     if not payer_user:
                         hyperion_error_logger.exception(
                             f"Payment: failed to init a checkout with HA for module {module} and name {checkout_name} (no payer info provided).",
                         )
                     else:
                         payer_user_name = f"{payer_user.firstname} {payer_user.name}"
                         hyperion_error_logger.warning(
                             f"Payment: failed to init a checkout with HA for module {module} and name {checkout_name}. Retrying without payer infos for {payer_user_name}",
                         )
 
                         init_checkout_body.payer = None
                         try:
                             response = checkout_api.organizations_organization_slug_checkout_intents_post(
                                 self._helloasso_slug,
                                 init_checkout_body,
                             )
                         except UnauthorizedException:
                             # HelloAsso returned a 401 unauthorized again
                             hyperion_error_logger.exception(
                                 f"Payment: failed to init a checkout with HA for module {module} and name {checkout_name}, with and without payer {payer_user_name} infos",
@@ -231,25 +233,30 @@
                     hello_asso_checkout_id=response.id,
                     secret=secret,
                 )
-
                 await cruds_payment.create_checkout(db=db, checkout=checkout_model)
 
                 return schemas_payment.Checkout(
                     id=checkout_model_id,
                     payment_url=response.redirect_url,
                 )
-            hyperion_error_logger.error(
-                f"Payment: failed to init a checkout with HA for module {module} and name {checkout_name}. No checkout id returned",
-            )
-            raise MissingHelloAssoCheckoutIdError()  # noqa: TRY301
 
+        except BadRequestException as e:
+            # We know that HelloAsso may refuse some payer infos,
+            # e.g. >18 years old, valid email, and firstname and name without some characters and patterns.
+            # See https://dev.helloasso.com/docs/int%C3%A9grer-le-paiement-sur-votre-site#contr%C3%B4le-des-champs
+            # git remote set-url origin [email protected]:aeecleclair/Hyperion.git
+            hyperion_error_logger.exception(
+                f"{exception_start}: bad request for reason {e.data or e.body}.",
+            )
+            raise
+        except MissingHelloAssoCheckoutIdError:
+            hyperion_error_logger.exception(
+                f"{exception_start}: no checkout id returned.",
+            )
+            raise
         except Exception:
-            # Different from a 401 unauthorized
-            payer_user_name = ""
-            if payer_user:
-                payer_user_name = f"{payer_user.firstname} {payer_user.name}"
             hyperion_error_logger.exception(
-                f"Payment: failed to init a checkout with HA for module {module} and name {checkout_name} with payer {payer_user_name} infos",
+                exception_start,
             )
             raise
 

tests/test_payment.py

@@ -4,15 +4,10 @@
 import pytest
 import pytest_asyncio
 from fastapi.testclient import TestClient
-from helloasso_python.exceptions import UnauthorizedException
-from helloasso_python.models.hello_asso_api_v5_models_carts_init_checkout_body import (
-    HelloAssoApiV5ModelsCartsInitCheckoutBody,
-)
 from helloasso_python.models.hello_asso_api_v5_models_carts_init_checkout_response import (
     HelloAssoApiV5ModelsCartsInitCheckoutResponse,
 )
 from pytest_mock import MockerFixture
-from requests import Response
 from sqlalchemy.ext.asyncio import AsyncSession
 
 from app.core.payment import cruds_payment, models_payment, schemas_payment
@@ -464,79 +459,79 @@ async def test_payment_tool_init_checkout(
         assert created_checkout is not None
 
 
-async def test_payment_tool_init_checkout_with_one_failure(
-    mocker: MockerFixture,
-):
-    """
-    When HelloAsso init_checkout fail a first time, we want to retry a second time without payers infos.
-    """
-    redirect_url = "https://example.com"
-    # We create a mocked settings object with the required HelloAsso API credentials
-    settings: Settings = mocker.MagicMock()
-    settings.HELLOASSO_API_BASE = "https://example.com"
-    settings.HELLOASSO_CONFIGURATIONS = {
-        HelloAssoConfigName.CDR: HelloAssoConfig(
-            helloasso_client_id="clientid",
-            helloasso_client_secret="secret",
-            helloasso_slug="test",
-            redirect_url=redirect_url,
-        ),
-    }
-
-    payment_tool = PaymentTool(
-        config=settings.HELLOASSO_CONFIGURATIONS[HelloAssoConfigName.CDR],
-        helloasso_api_base=settings.HELLOASSO_API_BASE,
-    )
-
-    # We create a side effect for the `init_a_checkout` method that will raise an error the first time
-    # init_checkout is called with a payer, and return a mocked response the second time
-    def init_a_checkout_side_effect(
-        helloasso_slug: str,
-        init_checkout_body: HelloAssoApiV5ModelsCartsInitCheckoutBody,
-    ):
-        if init_checkout_body.payer is not None:
-            r = Response()
-            r.status_code = 400
-            raise UnauthorizedException
-        return HelloAssoApiV5ModelsCartsInitCheckoutResponse(
-            id=7,
-            redirect_url=redirect_url,
-        )
-
-    mocker.patch.object(
-        payment_tool,
-        "get_access_token",
-        return_value="access_token",
-    )
-
-    # We mock the whole HelloAssoAPIWrapper to avoid making real API calls
-    # and prevent the class initialization from failing to authenticate
-    # We mock the init checkout method to return a mocked response
-    mock_checkout_api = mocker.MagicMock()
-    mock_checkout_api.organizations_organization_slug_checkout_intents_post.side_effect = init_a_checkout_side_effect
-    mocker.patch(
-        "app.core.payment.payment_tool.CheckoutApi",
-        return_value=mock_checkout_api,
-    )
-
-    async with get_TestingSessionLocal()() as db:
-        returned_checkout = await payment_tool.init_checkout(
-            module="testtool",
-            checkout_amount=100,
-            checkout_name="test",
-            redirection_uri="redirect",
-            db=db,
-            payer_user=user_schema,
-        )
-
-        assert returned_checkout.payment_url == redirect_url
-
-        # We want to check that the checkout was created in the database
-        created_checkout = await payment_tool.get_checkout(
-            checkout_id=returned_checkout.id,
-            db=db,
-        )
-        assert created_checkout is not None
+# async def test_payment_tool_init_checkout_with_one_failure(
+#     mocker: MockerFixture,
+# ):
+#     """
+#     When HelloAsso init_checkout fail a first time, we want to retry a second time without payers infos.
+#     """
+#     redirect_url = "https://example.com"
+#     # We create a mocked settings object with the required HelloAsso API credentials
+#     settings: Settings = mocker.MagicMock()
+#     settings.HELLOASSO_API_BASE = "https://example.com"
+#     settings.HELLOASSO_CONFIGURATIONS = {
+#         HelloAssoConfigName.CDR: HelloAssoConfig(
+#             helloasso_client_id="clientid",
+#             helloasso_client_secret="secret",
+#             helloasso_slug="test",
+#             redirect_url=redirect_url,
+#         ),
+#     }
+
+#     payment_tool = PaymentTool(
+#         config=settings.HELLOASSO_CONFIGURATIONS[HelloAssoConfigName.CDR],
+#         helloasso_api_base=settings.HELLOASSO_API_BASE,
+#     )
+
+#     # We create a side effect for the `init_a_checkout` method that will raise an error the first time
+#     # init_checkout is called with a payer, and return a mocked response the second time
+#     def init_a_checkout_side_effect(
+#         helloasso_slug: str,
+#         init_checkout_body: HelloAssoApiV5ModelsCartsInitCheckoutBody,
+#     ):
+#         if init_checkout_body.payer is not None:
+#             r = Response()
+#             r.status_code = 400
+#             raise BadRequestException
+#         return HelloAssoApiV5ModelsCartsInitCheckoutResponse(
+#             id=7,
+#             redirect_url=redirect_url,
+#         )
+
+#     mocker.patch.object(
+#         payment_tool,
+#         "get_access_token",
+#         return_value="access_token",
+#     )
+
+#     # We mock the whole HelloAssoAPIWrapper to avoid making real API calls
+#     # and prevent the class initialization from failing to authenticate
+#     # We mock the init checkout method to return a mocked response
+#     mock_checkout_api = mocker.MagicMock()
+#     mock_checkout_api.organizations_organization_slug_checkout_intents_post.side_effect = init_a_checkout_side_effect
+#     mocker.patch(
+#         "app.core.payment.payment_tool.CheckoutApi",
+#         return_value=mock_checkout_api,
+#     )
+
+#     async with get_TestingSessionLocal()() as db:
+#         returned_checkout = await payment_tool.init_checkout(
+#             module="testtool",
+#             checkout_amount=100,
+#             checkout_name="test",
+#             redirection_uri="redirect",
+#             db=db,
+#             payer_user=user_schema,
+#         )
+
+#         assert returned_checkout.payment_url == redirect_url
+
+#         # We want to check that the checkout was created in the database
+#         created_checkout = await payment_tool.get_checkout(
+#             checkout_id=returned_checkout.id,
+#             db=db,
+#         )
+#         assert created_checkout is not None
 
 
 async def test_payment_tool_init_checkout_fail(