From e07a9e4dd430cb5db040d0e667a154a2571f9556 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Fran=C3=A7ois=20Voron?= Date: Wed, 8 Jan 2025 14:23:44 +0100 Subject: [PATCH] server/transaction: fix payout not working if refunded payments appear at the end If a refunded payment appear last, or if it's big enough, we can't compensate its outstanding amount with subsequent transfers, leaving us with an outstanding amount. To solve that, we simply sort the transactions by their transferrable amount, so the negative one appear first and can be compensated by the positive ones appearing after. --- server/polar/transaction/service/payout.py | 7 +++ .../tests/transaction/service/test_payout.py | 57 ++++++++++++------- 2 files changed, 43 insertions(+), 21 deletions(-) diff --git a/server/polar/transaction/service/payout.py b/server/polar/transaction/service/payout.py index 42bc8b4769..d00ac18e93 100644 --- a/server/polar/transaction/service/payout.py +++ b/server/polar/transaction/service/payout.py @@ -445,6 +445,13 @@ async def _prepare_stripe_payout( ) ) + # Sort payment_balance_transactions by increasing transferable amount + # This way, if we have negative transferrable amount, they'll increase the outstanding amount + # and be compensated by the positive transferrable amounts coming after + payment_balance_transactions.sort( + key=lambda balance_transaction: balance_transaction.transferable_amount + ) + # Compute transfers out of each payment balance, making sure to subtract the outstanding amount transfers: list[tuple[str, int, Transaction]] = [] for balance_transaction in payment_balance_transactions: diff --git a/server/tests/transaction/service/test_payout.py b/server/tests/transaction/service/test_payout.py index 7f3b73e6f3..1516e856e4 100644 --- a/server/tests/transaction/service/test_payout.py +++ b/server/tests/transaction/service/test_payout.py @@ -42,7 +42,7 @@ def stripe_service_mock(mocker: MockerFixture) -> MagicMock: async def create_payment_transaction( save_fixture: SaveFixture, *, - amount: int = 1000, + amount: int = 10000, charge_id: str = "STRIPE_CHARGE_ID", ) -> Transaction: transaction = Transaction( @@ -63,7 +63,7 @@ async def create_payment_transaction( async def create_refund_transaction( save_fixture: SaveFixture, *, - amount: int = -1000, + amount: int = -10000, charge_id: str = "STRIPE_CHARGE_ID", refund_id: str = "STRIPE_REFUND_ID", ) -> Transaction: @@ -88,7 +88,7 @@ async def create_balance_transaction( *, account: Account, currency: str = "usd", - amount: int = 1000, + amount: int = 10000, payment_transaction: Transaction | None = None, balance_reversal_transaction: Transaction | None = None, payout_transaction: Transaction | None = None, @@ -357,37 +357,46 @@ async def test_stripe_refund( payment_transaction_1 = await create_payment_transaction( save_fixture, charge_id="CHARGE_ID_1" ) - balance_transaction_1 = await create_balance_transaction( + balance_transaction_payment_1 = await create_balance_transaction( save_fixture, account=account, payment_transaction=payment_transaction_1 ) + balance_transaction_fee_1 = await create_balance_transaction( + save_fixture, + account=account, + amount=-100, + balance_reversal_transaction=balance_transaction_payment_1, + ) payment_transaction_2 = await create_payment_transaction( save_fixture, charge_id="CHARGE_ID_2" ) - balance_transaction_2 = await create_balance_transaction( + balance_transaction_payment_2 = await create_balance_transaction( save_fixture, account=account, payment_transaction=payment_transaction_2 ) + balance_transaction_fee_2 = await create_balance_transaction( + save_fixture, + account=account, + amount=-100, + balance_reversal_transaction=balance_transaction_payment_2, + ) - assert payment_transaction_1.charge_id is not None - refund_transaction_1 = await create_refund_transaction( + assert payment_transaction_2.charge_id is not None + refund_transaction_2 = await create_refund_transaction( save_fixture, - amount=-payment_transaction_1.amount, - charge_id=payment_transaction_1.charge_id, + amount=-payment_transaction_2.amount, + charge_id=payment_transaction_2.charge_id, ) - balance_transaction_3 = await create_balance_transaction( + balance_transaction_refund_2 = await create_balance_transaction( save_fixture, account=account, - amount=refund_transaction_1.amount, - balance_reversal_transaction=balance_transaction_1, + amount=refund_transaction_2.amount, + balance_reversal_transaction=balance_transaction_payment_2, ) stripe_service_mock.transfer.return_value = SimpleNamespace( id="STRIPE_TRANSFER_ID", balance_transaction="STRIPE_BALANCE_TRANSACTION_ID" ) - # then - session.expunge_all() - payout = await payout_transaction_service.create_payout( session, account=account ) @@ -400,12 +409,18 @@ async def test_stripe_refund( assert payout.account_currency == "usd" assert payout.account_amount < 0 - assert len(payout.paid_transactions) == 3 + len( + assert len(payout.paid_transactions) == 5 + len( payout.account_incurred_transactions ) - assert payout.paid_transactions[0].id == balance_transaction_1.id - assert payout.paid_transactions[1].id == balance_transaction_2.id - assert payout.paid_transactions[2].id == balance_transaction_3.id + assert set(t.id for t in payout.paid_transactions).issuperset( + { + balance_transaction_payment_1.id, + balance_transaction_fee_1.id, + balance_transaction_payment_2.id, + balance_transaction_fee_2.id, + balance_transaction_refund_2.id, + } + ) assert len(payout.incurred_transactions) > 0 assert ( @@ -452,9 +467,9 @@ async def test_stripe_refund_of_paid_payment( account=account, processor=PaymentProcessor.stripe, currency="usd", - amount=-1000, + amount=-10000, account_currency="usd", - account_amount=-1000, + account_amount=-10000, tax_amount=0, ) await save_fixture(previous_payout)