diff --git a/app/apps/bank_sync/providers/base.py b/app/apps/bank_sync/providers/base.py index 810a338e..fc1b9168 100644 --- a/app/apps/bank_sync/providers/base.py +++ b/app/apps/bank_sync/providers/base.py @@ -23,6 +23,9 @@ class RemoteTransaction: description: str = "" notes: str = "" currency: str = "" + # Not settled yet. Imported unpaid and updated when it posts, so only return + # these when the provider keeps the id once they post + pending: bool = False class BankSyncProvider(ABC): diff --git a/app/apps/bank_sync/sync.py b/app/apps/bank_sync/sync.py index f3e95ed3..3961c7b1 100644 --- a/app/apps/bank_sync/sync.py +++ b/app/apps/bank_sync/sync.py @@ -1,6 +1,7 @@ import logging from datetime import timedelta +from django.db.models import Min from django.utils import timezone from apps.bank_sync.models import BankSyncAccount, BankSyncService @@ -15,6 +16,9 @@ logger = logging.getLogger(__name__) OVERLAP_DAYS = 7 # How far apart a synced transaction and the one it matches can be dated MATCH_DAYS = 3 +# Pending ones older than this are taken as cancelled and no longer followed. A +# card's bill cycle is at most ~40 days +PENDING_MAX_DAYS = 60 def refresh_remote_accounts(service: BankSyncService, provider=None) -> None: @@ -58,10 +62,39 @@ def _match_existing(link: BankSyncAccount, internal_id, type_, amount, date) -> return True +def _settle(transaction: Transaction, amount, date) -> None: + """The bank posted a transaction that was still unpaid: take its final values. + + Happens once, so later edits stick. One marked paid by hand is left alone. + """ + if transaction.is_paid or transaction.deleted: + return + # Only when it still follows the date, so one picked by the user is kept + if transaction.reference_date == transaction.date.replace(day=1): + transaction.reference_date = date.replace(day=1) + transaction.is_paid = True + transaction.amount = amount + transaction.date = date + # save() keeps a transfer's other side in step. It sends no signal, and as a + # bank update rather than a user edit, rules don't run again + transaction.save() + + def _sync_account(service: BankSyncService, link: BankSyncAccount, provider) -> int: + prefix = f"{service.service_type}:{link.external_id}:" start = link.start_date if link.last_synced_date: start = max(start, link.last_synced_date - timedelta(days=OVERLAP_DAYS)) + # Pending ones can post weeks later (a card's bill closing): look back far + # enough to see it happen + oldest_unpaid = Transaction.userless_all_objects.filter( + internal_id__startswith=prefix, + is_paid=False, + deleted=False, + date__gte=timezone.localdate() - timedelta(days=PENDING_MAX_DAYS), + ).aggregate(Min("date"))["date__min"] + if oldest_unpaid: + start = min(start, oldest_unpaid) created = 0 seen_currencies = set() @@ -73,16 +106,22 @@ def _sync_account(service: BankSyncService, link: BankSyncAccount, provider) -> if link.currencies and remote.currency not in link.currencies: continue - internal_id = f"{service.service_type}:{link.external_id}:{remote.id}" - # userless_all_objects: also sees soft deleted ones, so deleting a synced - # transaction doesn't bring it back on the next run - if Transaction.userless_all_objects.filter(internal_id=internal_id).exists(): - continue - + internal_id = prefix + remote.id type_ = ( Transaction.Type.INCOME if remote.amount > 0 else Transaction.Type.EXPENSE ) amount = abs(remote.amount) + + # userless_all_objects: also sees soft deleted ones, so deleting a synced + # transaction doesn't bring it back on the next run + existing = Transaction.userless_all_objects.filter( + internal_id=internal_id + ).first() + if existing: + if not remote.pending: + _settle(existing, amount, remote.date) + continue + if service.match_existing and _match_existing( link, internal_id, type_, amount, remote.date ): @@ -96,7 +135,7 @@ def _sync_account(service: BankSyncService, link: BankSyncAccount, provider) -> date=remote.date, description=remote.description[:500], notes=remote.notes, - is_paid=True, + is_paid=not remote.pending, internal_id=internal_id, ) created += 1 diff --git a/app/apps/bank_sync/tests/test_bank_sync.py b/app/apps/bank_sync/tests/test_bank_sync.py index 104f01da..a74a7f25 100644 --- a/app/apps/bank_sync/tests/test_bank_sync.py +++ b/app/apps/bank_sync/tests/test_bank_sync.py @@ -295,6 +295,76 @@ class SyncTests(TestCase): self._sync() self.assertEqual(self.provider.calls, [("7", date(2026, 10, 25))]) + def _pending(self, days_ago, amount="-20", pending=True): + day = timezone.localdate() - timedelta(days=days_ago) + return RemoteTransaction("p", day, Decimal(amount), "Hotel", pending=pending) + + def _hotel(self): + return Transaction.userless_all_objects.get(internal_id="lunchflow:7:p") + + def test_pending_is_imported_unpaid_and_settled_once_when_it_posts(self): + self.link.start_date = timezone.localdate() - timedelta(days=90) + self.link.save() + self.provider.transactions = [self._pending(30)] + self.assertEqual(self._sync(), 1) + self.assertFalse(self._hotel().is_paid) + + # Still pending with another amount: left alone, edits stick + self.provider.transactions = [self._pending(30, "-22")] + self._sync() + self.assertEqual(self._hotel().amount, Decimal("20")) + # And it's still looked for, way past the overlap + self.assertEqual(self.provider.calls[-1][1], timezone.localdate() - timedelta(days=30)) + + # Posts with its final amount and date + self.provider.transactions = [self._pending(29, "-25.509", pending=False)] + self.assertEqual(self._sync(), 0) + hotel = self._hotel() + self.assertTrue(hotel.is_paid) + self.assertEqual(hotel.amount, Decimal("25.50")) + self.assertEqual(hotel.date, timezone.localdate() - timedelta(days=29)) + self.assertEqual(hotel.reference_date, hotel.date.replace(day=1)) + + # Settled: later changes on the bank don't touch it, look back is normal + self.provider.transactions = [self._pending(29, "-99", pending=False)] + self._sync() + self.assertEqual(self._hotel().amount, Decimal("25.50")) + self.assertEqual(self.provider.calls[-1][1], timezone.localdate() - timedelta(days=7)) + + def test_settling_keeps_a_reference_date_picked_by_the_user(self): + self.link.start_date = timezone.localdate() - timedelta(days=90) + self.link.save() + self.provider.transactions = [self._pending(10)] + self._sync() + Transaction.userless_all_objects.filter(internal_id="lunchflow:7:p").update( + emit_signal=False, reference_date=date(2030, 1, 1) + ) + self.provider.transactions = [self._pending(10, pending=False)] + self._sync() + self.assertEqual(self._hotel().reference_date, date(2030, 1, 1)) + self.assertTrue(self._hotel().is_paid) + + def test_unpaid_marked_paid_by_hand_is_left_alone(self): + self.link.start_date = timezone.localdate() - timedelta(days=90) + self.link.save() + self.provider.transactions = [self._pending(10)] + self._sync() + Transaction.userless_all_objects.filter(internal_id="lunchflow:7:p").update( + emit_signal=False, is_paid=True + ) + self.provider.transactions = [self._pending(10, "-99", pending=False)] + self._sync() + self.assertEqual(self._hotel().amount, Decimal("20")) + + def test_old_pending_is_no_longer_followed(self): + self.link.start_date = timezone.localdate() - timedelta(days=200) + self.link.save() + self.provider.transactions = [self._pending(61)] + self._sync() + self._sync() + # Taken as cancelled: back to the normal overlap + self.assertEqual(self.provider.calls[-1][1], timezone.localdate() - timedelta(days=7)) + def test_unlinked_accounts_are_skipped(self): self.link.account = None self.link.save()