From 3a2cf12052c05e2d3547fc3402a118b0cdf295b8 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?dni=20=E2=9A=A1?= <office@dnilabs.com>
Date: Thu, 19 Dec 2024 10:22:37 +0100
Subject: [PATCH] fix: get_payments where clause does query wrong payments
 including tests (#2844)

* fix: get_payments where clause does query wrong payments (#2841)
---
 lnbits/core/crud/payments.py     | 21 +++++++---
 lnbits/core/views/payment_api.py |  2 -
 tests/unit/test_crud_payments.py | 66 ++++++++++++++++++++++++++++++++
 3 files changed, 82 insertions(+), 7 deletions(-)
 create mode 100644 tests/unit/test_crud_payments.py

diff --git a/lnbits/core/crud/payments.py b/lnbits/core/crud/payments.py
index 979c7b02..20528297 100644
--- a/lnbits/core/crud/payments.py
+++ b/lnbits/core/crud/payments.py
@@ -123,15 +123,20 @@ async def get_payments_paginated(
         clause.append("wallet_id = :wallet_id")
 
     if complete and pending:
-        pass
+        clause.append(
+            f"(status = '{PaymentState.SUCCESS}' OR status = '{PaymentState.PENDING}')"
+        )
     elif complete:
         clause.append(
-            f"((amount > 0 AND status = '{PaymentState.SUCCESS}') OR amount < 0)"
+            f"""
+            (
+                status = '{PaymentState.SUCCESS}'
+                OR (amount < 0 AND status = '{PaymentState.PENDING}')
+            )
+            """
         )
     elif pending:
         clause.append(f"status = '{PaymentState.PENDING}'")
-    else:
-        pass
 
     if outgoing and incoming:
         pass
@@ -289,8 +294,14 @@ async def get_payments_history(
     values = {
         "wallet_id": wallet_id,
     }
+    # count outgoing payments if they are still pending
     where = [
-        f"wallet_id = :wallet_id AND (status = '{PaymentState.SUCCESS}' OR amount < 0)"
+        f"""
+        wallet_id = :wallet_id AND (
+            status = '{PaymentState.SUCCESS}'
+            OR (amount < 0 AND status = '{PaymentState.PENDING}')
+        )
+        """
     ]
     transactions: list[dict] = await db.fetchall(
         f"""
diff --git a/lnbits/core/views/payment_api.py b/lnbits/core/views/payment_api.py
index e27773a9..98e1a8bb 100644
--- a/lnbits/core/views/payment_api.py
+++ b/lnbits/core/views/payment_api.py
@@ -108,8 +108,6 @@ async def api_payments_paginated(
     await update_pending_payments(key_info.wallet.id)
     page = await get_payments_paginated(
         wallet_id=key_info.wallet.id,
-        pending=True,
-        complete=True,
         filters=filters,
     )
     return page
diff --git a/tests/unit/test_crud_payments.py b/tests/unit/test_crud_payments.py
new file mode 100644
index 00000000..b177ed73
--- /dev/null
+++ b/tests/unit/test_crud_payments.py
@@ -0,0 +1,66 @@
+import pytest
+
+from lnbits.core.crud import create_wallet, get_payments, update_payment
+from lnbits.core.models import PaymentState
+from lnbits.core.services import create_user_account, update_wallet_balance
+
+
+async def update_payments(payments):
+    payments[0].status = PaymentState.FAILED
+    await update_payment(payments[0])
+    payments[1].status = PaymentState.PENDING
+    await update_payment(payments[1])
+    payments[2].status = PaymentState.PENDING
+    await update_payment(payments[2])
+
+
+@pytest.mark.anyio
+async def test_crud_get_payments(app):
+
+    user = await create_user_account()
+    wallet = await create_wallet(user_id=user.id)
+
+    for _ in range(11):
+        await update_wallet_balance(wallet, 10)
+        wallet.balance_msat += 10 * 1000
+        await update_wallet_balance(wallet, -10)
+        wallet.balance_msat += -10 * 1000
+
+    payments = await get_payments(wallet_id=wallet.id)
+    assert len(payments) == 22, "should return 22 successful payments"
+
+    payments = await get_payments(wallet_id=wallet.id, incoming=True)
+    assert len(payments) == 11, "should return 11 successful incoming payments"
+    await update_payments(payments)
+
+    payments = await get_payments(wallet_id=wallet.id, outgoing=True)
+    assert len(payments) == 11, "should return 11 successful outgoing payments"
+    await update_payments(payments)
+
+    payments = await get_payments(wallet_id=wallet.id, pending=True)
+    assert len(payments) == 4, "should return 4 pending payments"
+
+    # function signature should have Optional[bool] for complete and pending to make
+    # this distinction possible
+    payments = await get_payments(wallet_id=wallet.id, pending=False)
+    assert len(payments) == 22, "should return all payments"
+
+    payments = await get_payments(wallet_id=wallet.id, complete=True, pending=True)
+    assert len(payments) == 20, "should return 4 pending and 16 complete payments"
+
+    payments = await get_payments(wallet_id=wallet.id, complete=True, outgoing=True)
+    assert (
+        len(payments) == 10
+    ), "should return 8 complete outgoing payments and 2 pending outgoing payments"
+
+    payments = await get_payments(wallet_id=wallet.id)
+    assert len(payments) == 22, "should return all payments"
+
+    payments = await get_payments(wallet_id=wallet.id, complete=True)
+    assert (
+        len(payments) == 18
+    ), "should return 14 successful payment and 4 pending payments"
+
+    # both false should return failed payments
+    # payments = await get_payments(wallet_id=wallet.id, complete=False, pending=False)
+    # assert len(payments) == 2, "should return 2 failed payment"