🐛 Avoids credit transaction on unsuccesful payment (⚠️ devops) (#5106)

pcrespov · web-flow · commit 34b9c0fae5da · 2023-11-29T15:18:00.000Z
diff --git a/services/payments/src/simcore_service_payments/api/rest/_acknowledgements.py b/services/payments/src/simcore_service_payments/api/rest/_acknowledgements.py
@@ -3,10 +3,6 @@
 
 from fastapi import APIRouter, BackgroundTasks, Depends, HTTPException, status
 from servicelib.logging_utils import log_context
-from simcore_postgres_database.models.payments_methods import InitPromptAckFlowState
-from simcore_postgres_database.models.payments_transactions import (
-    PaymentTransactionState,
-)
 
 from ..._constants import ACKED, PGDB
 from ...core.errors import PaymentMethodNotFoundError, PaymentNotFoundError
@@ -54,27 +50,21 @@ async def acknowledge_payment(
         f"{payment_id=}",
     ):
         try:
-            transaction = await repo_pay.update_ack_payment_transaction(
-                payment_id=payment_id,
-                completion_state=(
-                    PaymentTransactionState.SUCCESS
-                    if ack.success
-                    else PaymentTransactionState.FAILED
-                ),
-                state_message=ack.message,
-                invoice_url=ack.invoice_url,
+            transaction = await payments.acknowledge_one_time_payment(
+                repo_pay, payment_id=payment_id, ack=ack
             )
         except PaymentNotFoundError as err:
             raise HTTPException(
                 status_code=status.HTTP_404_NOT_FOUND, detail=f"{err}"
             ) from err
 
-    if transaction.state == PaymentTransactionState.SUCCESS:
-        assert f"{payment_id}" == f"{transaction.payment_id}"  # nosec
-        background_tasks.add_task(payments.on_payment_completed, transaction, rut_api)
+    assert f"{payment_id}" == f"{transaction.payment_id}"  # nosec
+    background_tasks.add_task(
+        payments.on_payment_completed, transaction, rut_api, notify_enabled=True
+    )
 
     if ack.saved:
-        created = await payments_methods.create_payment_method(
+        created = await payments_methods.insert_payment_method(
             repo=repo_methods,
             payment_method_id=ack.saved.payment_method_id,
             user_id=transaction.user_id,
@@ -102,7 +92,6 @@ async def acknowledge_payment_method(
         f"{payment_method_id=}",
     ):
         try:
-
             acked = await payments_methods.acknowledge_creation_of_payment_method(
                 repo=repo, payment_method_id=payment_method_id, ack=ack
             )
@@ -111,6 +100,5 @@ async def acknowledge_payment_method(
                 status_code=status.HTTP_404_NOT_FOUND, detail=f"{err}"
             ) from err
 
-    if acked.state == InitPromptAckFlowState.SUCCESS:
         assert f"{payment_method_id}" == f"{acked.payment_method_id}"  # nosec
         background_tasks.add_task(payments_methods.on_payment_method_completed, acked)
diff --git a/services/payments/src/simcore_service_payments/services/payments.py b/services/payments/src/simcore_service_payments/services/payments.py
@@ -144,40 +144,47 @@ async def acknowledge_one_time_payment(
 
 
 async def on_payment_completed(
-    transaction: PaymentsTransactionsDB, rut_api: ResourceUsageTrackerApi
+    transaction: PaymentsTransactionsDB,
+    rut_api: ResourceUsageTrackerApi,
+    *,
+    notify_enabled: bool,
 ):
     assert transaction.completed_at is not None  # nosec
     assert transaction.initiated_at < transaction.completed_at  # nosec
 
-    _logger.debug(
-        "Notify front-end of payment -> sio SOCKET_IO_PAYMENT_COMPLETED_EVENT "
-    )
-
-    with log_context(
-        _logger,
-        logging.INFO,
-        "%s: Top-up %s credits for %s",
-        RUT,
-        f"{transaction.osparc_credits}",
-        f"{transaction.payment_id=}",
-    ):
-        credit_transaction_id = await rut_api.create_credit_transaction(
-            product_name=transaction.product_name,
-            wallet_id=transaction.wallet_id,
-            wallet_name=f"id={transaction.wallet_id}",
-            user_id=transaction.user_id,
-            user_email=transaction.user_email,
-            osparc_credits=transaction.osparc_credits,
-            payment_transaction_id=transaction.payment_id,
-            created_at=transaction.completed_at,
+    if notify_enabled:
+        _logger.debug(
+            "Notify front-end of payment -> sio SOCKET_IO_PAYMENT_COMPLETED_EVENT "
+            "socketio.notify_payment_completed(sio, user_primary_group_id=gid, payment=transaction)"
         )
 
-    _logger.debug(
-        "%s: Response to %s was %s",
-        RUT,
-        f"{transaction.payment_id=}",
-        f"{credit_transaction_id=}",
-    )
+    if transaction.state == PaymentTransactionState.SUCCESS:
+        with log_context(
+            _logger,
+            logging.INFO,
+            "%s: Top-up %s credits for %s",
+            RUT,
+            f"{transaction.osparc_credits}",
+            f"{transaction.payment_id=}",
+        ):
+            assert transaction.state == PaymentTransactionState.SUCCESS  # nosec
+            credit_transaction_id = await rut_api.create_credit_transaction(
+                product_name=transaction.product_name,
+                wallet_id=transaction.wallet_id,
+                wallet_name=f"id={transaction.wallet_id}",
+                user_id=transaction.user_id,
+                user_email=transaction.user_email,
+                osparc_credits=transaction.osparc_credits,
+                payment_transaction_id=transaction.payment_id,
+                created_at=transaction.completed_at,
+            )
+
+        _logger.debug(
+            "%s: Response to %s was %s",
+            RUT,
+            f"{transaction.payment_id=}",
+            f"{credit_transaction_id=}",
+        )
 
 
 async def pay_with_payment_method(  # noqa: PLR0913
@@ -247,7 +254,8 @@ async def pay_with_payment_method(  # noqa: PLR0913
         invoice_url=ack.invoice_url,
     )
 
-    await on_payment_completed(transaction, rut)
+    # NOTE: notifications here are done as background-task after responding `POST /wallets/{wallet_id}/payments-methods/{payment_method_id}:pay`
+    await on_payment_completed(transaction, rut, notify_enabled=False)
 
     return transaction.to_api_model()
 
diff --git a/services/payments/src/simcore_service_payments/services/payments_methods.py b/services/payments/src/simcore_service_payments/services/payments_methods.py
@@ -119,24 +119,25 @@ async def acknowledge_creation_of_payment_method(
 
 
 async def on_payment_method_completed(payment_method: PaymentsMethodsDB):
-    assert payment_method.state == InitPromptAckFlowState.SUCCESS  # nosec
     assert payment_method.completed_at is not None  # nosec
     assert payment_method.initiated_at < payment_method.completed_at  # nosec
 
-    _logger.debug(
-        "Notify front-end of payment -> sio (SOCKET_IO_PAYMENT_METHOD_ACKED_EVENT) "
-    )
+    if payment_method.state == InitPromptAckFlowState.SUCCESS:
+        _logger.debug("Notify front-end of payment-method created! ")
 
 
-async def create_payment_method(
+async def insert_payment_method(
     repo: PaymentsMethodsRepo,
     *,
     payment_method_id: PaymentMethodID,
     user_id: UserID,
     wallet_id: WalletID,
     ack: AckPaymentMethod,
 ) -> PaymentsMethodsDB:
-    """Direct creation of payment-method"""
+    """Direct creation of payment-method.
+    NOTE: that this does NOT communicates with the gateway.
+    Used e.g. when gateway saved payment method after one-time payment
+    """
     return await repo.insert_payment_method(
         payment_method_id=payment_method_id,
         user_id=user_id,
diff --git a/services/payments/tests/unit/conftest.py b/services/payments/tests/unit/conftest.py
@@ -5,7 +5,7 @@
 # pylint: disable=unused-variable
 
 
-from collections.abc import AsyncIterator, Awaitable, Callable, Iterator
+from collections.abc import AsyncIterable, AsyncIterator, Awaitable, Callable, Iterator
 from pathlib import Path
 from typing import Any, NamedTuple
 from unittest.mock import Mock
@@ -20,6 +20,9 @@
 from fastapi import FastAPI, status
 from fastapi.encoders import jsonable_encoder
 from models_library.api_schemas_webserver.wallets import PaymentMethodID
+from models_library.users import UserID
+from models_library.wallets import WalletID
+from pydantic import parse_obj_as
 from pytest_mock import MockerFixture
 from pytest_simcore.helpers.rawdata_fakers import random_payment_method_data
 from pytest_simcore.helpers.typing_env import EnvVarsDict
@@ -29,6 +32,8 @@
 from simcore_postgres_database.models.payments_transactions import payments_transactions
 from simcore_service_payments.core.application import create_app
 from simcore_service_payments.core.settings import ApplicationSettings
+from simcore_service_payments.db.payments_methods_repo import PaymentsMethodsRepo
+from simcore_service_payments.models.db import PaymentsMethodsDB
 from simcore_service_payments.models.payments_gateway import (
     BatchGetPaymentMethods,
     GetPaymentMethod,
@@ -39,8 +44,10 @@
     PaymentMethodsBatch,
 )
 from simcore_service_payments.models.schemas.acknowledgements import (
+    AckPaymentMethod,
     AckPaymentWithPaymentMethod,
 )
+from simcore_service_payments.services import payments_methods
 from toolz.dicttoolz import get_in
 
 #
@@ -123,7 +130,41 @@ def payments_clean_db(postgres_db: sa.engine.Engine) -> Iterator[None]:
         con.execute(payments_transactions.delete())
 
 
-#
+@pytest.fixture
+async def create_fake_payment_method_in_db(
+    app: FastAPI,
+) -> AsyncIterable[
+    Callable[[PaymentMethodID, WalletID, UserID], Awaitable[PaymentsMethodsDB]]
+]:
+    _repo = PaymentsMethodsRepo(app.state.engine)
+    _created = []
+
+    async def _(
+        payment_method_id: PaymentMethodID,
+        wallet_id: WalletID,
+        user_id: UserID,
+    ) -> PaymentsMethodsDB:
+        acked = await payments_methods.insert_payment_method(
+            repo=_repo,
+            payment_method_id=payment_method_id,
+            user_id=user_id,
+            wallet_id=wallet_id,
+            ack=AckPaymentMethod(
+                success=True,
+                message=f"Created with {create_fake_payment_method_in_db.__name__}",
+            ),
+        )
+        _created.append(acked)
+        return acked
+
+    yield _
+
+    for acked in _created:
+        await _repo.delete_payment_method(
+            acked.payment_method_id, user_id=acked.user_id, wallet_id=acked.wallet_id
+        )
+
+
 MAX_TIME_FOR_APP_TO_STARTUP = 10
 MAX_TIME_FOR_APP_TO_SHUTDOWN = 10
 
@@ -195,7 +236,19 @@ def _cancel_200(request: httpx.Request):
 
 
 @pytest.fixture
-def mock_payments_methods_routes(faker: Faker) -> Iterator[Callable]:
+def no_funds_payment_method_id(faker: Faker) -> PaymentMethodID:
+    """Fake Paymets-Gateway will decline payments with this payment-method id due to insufficient -funds
+
+    USE create_fake_payment_method_in_db to inject this payment-method in DB
+    Emulates https://stripe.com/docs/testing#declined-payments
+    """
+    return parse_obj_as(PaymentMethodID, "no_funds_payment_method_id")
+
+
+@pytest.fixture
+def mock_payments_methods_routes(
+    faker: Faker, no_funds_payment_method_id: PaymentMethodID
+) -> Iterator[Callable]:
     class PaymentMethodInfoTuple(NamedTuple):
         init: InitPaymentMethod
         get: GetPaymentMethod
@@ -259,6 +312,21 @@ def _pay(request: httpx.Request, pm_id: PaymentMethodID):
             _get(request, pm_id)
 
             payment_id = faker.uuid4()
+
+            if pm_id == no_funds_payment_method_id:
+                # SEE https://stripe.com/docs/testing#declined-payments
+                return httpx.Response(
+                    status.HTTP_200_OK,
+                    json=jsonable_encoder(
+                        AckPaymentWithPaymentMethod(
+                            success=False,
+                            message=f"Insufficient Fonds '{pm_id}'",
+                            invoice_url=None,
+                            payment_id=payment_id,
+                        )
+                    ),
+                )
+
             return httpx.Response(
                 status.HTTP_200_OK,
                 json=jsonable_encoder(
diff --git a/services/payments/tests/unit/test_rpc_payments_methods.py b/services/payments/tests/unit/test_rpc_payments_methods.py
@@ -32,7 +32,7 @@
     PaymentTransactionState,
 )
 from simcore_service_payments.models.schemas.acknowledgements import AckPaymentMethod
-from simcore_service_payments.services.payments_methods import create_payment_method
+from simcore_service_payments.services.payments_methods import insert_payment_method
 
 pytest_simcore_core_services_selection = [
     "postgres",
@@ -215,7 +215,7 @@ async def test_webserver_pay_with_payment_method_workflow(
     assert app
 
     # faking Payment method
-    created = await create_payment_method(
+    created = await insert_payment_method(
         repo=PaymentsMethodsRepo(app.state.engine),
         payment_method_id=faker.uuid4(),
         user_id=user_id,
diff --git a/services/payments/tests/unit/test_services_payments.py b/services/payments/tests/unit/test_services_payments.py
