Repository: bvanelli/actualpy
Pull request #118 (2025-03-01 21:30:23)

Bank sync/correct sorting
Fixes the incorrect sorting of the Actual transactions since the resolution of https://github.com/actualbudget/actual-server/issues/556

Since the server and frontend code have been moved together, all the bank sync validations are now being done on backend, to allow support for more integrations. This means `actualpy` has to handle less logic when evaluating the bank sync.

This PR solves a couple of issues:

- Correctly calculates the starting balance for goCardless integration
- Correctly sorts the transactions by starting with the "Starting Balance", then consuming all sync transactions in reverse
- Correctly set the sort_order field by multiplying it by 1000 to convert to milliseconds
- Added `Accounts.last_sync` and `Transactions.raw_synced_data`. Nothing has been implemented for them

codecov[bot]: ## [Codecov](https://app.codecov.io/gh/bvanelli/actualpy/pull/118?dropdown=coverage&src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=Brunno+Vanelli) Report
All modified and coverable lines are covered by tests :white_check_mark:
> Project coverage is 97.94%. Comparing base [(`ea640f2`)](https://app.codecov.io/gh/bvanelli/actualpy/commit/ea640f2750b13cde6cf71200415a0503e33687b9?dropdown=coverage&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=Brunno+Vanelli) to head [(`e4d18e8`)](https://app.codecov.io/gh/bvanelli/actualpy/commit/e4d18e8e60421226bca9fd28b4607209cf8f2510?dropdown=coverage&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=Brunno+Vanelli).

<details><summary>Additional details and impacted files</summary>


```diff
@@           Coverage Diff           @@
##             main     #118   +/-   ##
=======================================
  Coverage   97.93%   97.94%           
=======================================
  Files          16       16           
  Lines        2329     2336    +7     
=======================================
+ Hits         2281     2288    +7     
  Misses         48       48           
```

</details>

[:umbrella: View full report in Codecov by Sentry](https://app.codecov.io/gh/bvanelli/actualpy/pull/118?dropdown=coverage&src=pr&el=continue&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=Brunno+Vanelli).   
:loudspeaker: Have feedback on the report? [Share it here](https://about.codecov.io/codecov-pr-comment-feedback/?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=Brunno+Vanelli).


Patch:
diff --git a/actual/__init__.py b/actual/__init__.py
index 27b4068..b288576 100644
--- a/actual/__init__.py
+++ b/actual/__init__.py
@@ -488,10 +488,29 @@ class Actual(ActualServer):
             )
         new_transactions = new_transactions_data.data.transactions.all
         imported_transactions = []
-        for transaction in new_transactions:
+        if is_first_sync:
+            # actual uses 'startingBalance', that already comes in cents and should be enough for our purposes
+            # https://github.com/actualbudget/actual/blob/f09f4af667ddd57e031dcdb0d428ae935aa2afad/packages/loot-core/src/server/accounts/sync.ts#L740-L752
+            balance_to_use = new_transactions_data.data.balance
+            # For simpleFin, the startingBalance is actually the current balance, so we have to use it to deduce the
+            # actual startingBalance
+            if acct.account_sync_source and acct.account_sync_source.lower() == "simplefin":
+                current_balance = new_transactions_data.data.balance
+                balance_to_use = current_balance - sum(t.transaction_amount.amount for t in new_transactions)
+            if balance_to_use:
+                payee = None if acct.offbudget else get_or_create_payee(self.session, "Starting Balance")
+                # get date from the oldest transaction. There seems to be a bug here, and it gets the youngest transaction.
+                oldest_date = new_transactions[-1].date if new_transactions else datetime.date.today()
+                reconciled_transaction = create_transaction(
+                    self.session, oldest_date, acct, payee, notes=None, amount=balance_to_use, cleared=True
+                )
+                reconciled_transaction.starting_balance_flag = 1  # to tell is a starting balance
+                imported_transactions.append(reconciled_transaction)
+        # Consume transactions in the ascending order
+        for transaction in new_transactions[::-1]:
             if not transaction.booked:
                 continue
-            payee = transaction.imported_payee or "" if sync_method == "goCardless" else transaction.notes
+            payee = transaction.payee_name or "" if sync_method == "goCardless" else transaction.notes
             reconciled = reconcile_transaction(
                 self.session,
                 transaction.date,
@@ -506,20 +525,6 @@ class Actual(ActualServer):
             )
             if reconciled.changed():
                 imported_transactions.append(reconciled)
-        if is_first_sync:
-            current_balance = acct.balance
-            # actual uses 'startingBalance', that already comes in cents and should be enough for our purposes
-            # https://github.com/actualbudget/actual/blob/f09f4af667ddd57e031dcdb0d428ae935aa2afad/packages/loot-core/src/server/accounts/sync.ts#L740-L752
-            expected_balance = new_transactions_data.data.balance
-            balance_to_use = expected_balance - current_balance
-            if balance_to_use:
-                payee = None if acct.offbudget else get_or_create_payee(self.session, "Starting Balance")
-                # get date from the oldest transaction. There seems to be a bug here, and it gets the youngest transaction.
-                oldest_date = new_transactions[-1].date if new_transactions else datetime.date.today()
-                reconciled_transaction = create_transaction(
-                    self.session, oldest_date, acct, payee, amount=balance_to_use
-                )
-                imported_transactions.insert(0, reconciled_transaction)
         return imported_transactions
 
     def run_bank_sync(
diff --git a/actual/api/__init__.py b/actual/api/__init__.py
index c67f410..74ffa97 100644
--- a/actual/api/__init__.py
+++ b/actual/api/__init__.py
@@ -2,14 +2,16 @@ from __future__ import annotations
 
 import datetime
 import json
-from typing import List, Literal
+from typing import List, Literal, Union
 
 import requests
 
 from actual.api.models import (
     BankSyncAccountResponseDTO,
+    BankSyncErrorDTO,
     BankSyncResponseDTO,
     BankSyncStatusDTO,
+    BankSyncTransactionResponseDTO,
     BootstrapInfoDTO,
     Endpoints,
     GetUserFileInfoDTO,
@@ -286,7 +288,7 @@ class ActualServer:
         account_id: str,
         start_date: datetime.date,
         requisition_id: str = None,
-    ) -> BankSyncResponseDTO:
+    ) -> Union[BankSyncErrorDTO, BankSyncTransactionResponseDTO]:
         if bank_sync == "gocardless" and requisition_id is None:
             raise ActualInvalidOperationError("Retrieving transactions with goCardless requires `requisition_id`")
         endpoint = Endpoints.BANK_SYNC_TRANSACTIONS.value.format(bank_sync=bank_sync)
diff --git a/actual/api/bank_sync.py b/actual/api/bank_sync.py
index 37e3c31..4e879ef 100644
--- a/actual/api/bank_sync.py
+++ b/actual/api/bank_sync.py
@@ -44,6 +44,10 @@ class BankSyncAmount(BaseModel):
 class DebtorAccount(BaseModel):
     iban: str
 
+    @property
+    def masked_iban(self):
+        return f"({self.iban[:4]} XXX {self.iban[-4:]})"
+
 
 class BalanceType(enum.Enum):
     # See https://developer.gocardless.com/bank-account-data/balance#balance_type for full documentation
@@ -75,43 +79,39 @@ class TransactionItem(BaseModel):
     transaction_id: Optional[str] = Field(None, alias="transactionId")
     booked: Optional[bool] = False
     transaction_amount: BankSyncAmount = Field(..., alias="transactionAmount")
-    # this field will come as either debtorName or creditorName, depending on if it's a debt or credit
-    # The field might not exist, therefore is set as optional with a default
+    # these fields are generated on the server itself, so we can trust them as being correct
+    payee_name: Optional[str] = Field(None, alias="payeeName")
+    date: datetime.date = Field(..., alias="date")
+    notes: Optional[str] = Field(None, alias="notes")
+    # goCardless optional fields
     payee: Optional[str] = Field(None, validation_alias=AliasChoices("debtorName", "creditorName"))
     payee_account: Optional[DebtorAccount] = Field(
         None, validation_alias=AliasChoices("debtorAccount", "creditorAccount")
     )
-    # 'bookingDate' and 'valueDate' can be null or not existing in some goCardless data. Actual does not use them.
-    # Therefore, we just use them as fallbacks for the date, that should theoretically always exist.
-    # if the 'bookingDate' does not exist, the 'valueDate' will according to the docs
-    date: datetime.date = Field(..., validation_alias=AliasChoices("date", "bookingDate", "valueDate"))
+    booking_date: Optional[datetime.date] = Field(None, alias="bookingDate")
+    value_date: Optional[datetime.date] = Field(None, alias="valueDate")
     remittance_information_unstructured: str = Field(None, alias="remittanceInformationUnstructured")
     remittance_information_unstructured_array: List[str] = Field(
         default_factory=list, alias="remittanceInformationUnstructuredArray"
     )
     additional_information: Optional[str] = Field(None, alias="additionalInformation")
+    # simpleFin optional fields
+    posted_date: Optional[datetime.date] = Field(None, alias="postedDate")
 
     @property
     def imported_payee(self):
+        """Deprecated method to convert the payee name. Use the payee_name instead."""
         name_parts = []
         name = self.payee or self.notes or self.additional_information
         if name:
             name_parts.append(title(name))
         if self.payee_account and self.payee_account.iban:
-            iban = self.payee_account.iban
-            name_parts.append(f"({iban[:4]} XXX {iban[-4:]})")
+            name_parts.append(self.payee_account.masked_iban)
         return " ".join(name_parts).strip()
 
-    @property
-    def notes(self):
-        notes = self.remittance_information_unstructured or ", ".join(
-            self.remittance_information_unstructured_array or []
-        )
-        return notes.strip().replace("#", "##")
-
 
 class Transactions(BaseModel):
-    all: List[TransactionItem]
+    all: List[TransactionItem] = Field(..., description="List of all transactions, from newest to oldest.")
     booked: List[TransactionItem]
     pending: List[TransactionItem]
 
@@ -130,6 +130,11 @@ class BankSyncTransactionData(BaseModel):
 
     @property
     def balance(self) -> decimal.Decimal:
+        """Starting balance of the account integration, converted to a decimal amount.
+
+        For `simpleFin`, this will represent the current amount on the account, while for `goCardless` it will
+        represent the actual initial amount before all transactions.
+        """
         return decimal.Decimal(self.starting_balance) / 100
 
 
diff --git a/actual/database.py b/actual/database.py
index 2fc98d5..9f51e28 100644
--- a/actual/database.py
+++ b/actual/database.py
@@ -234,6 +234,7 @@ class Accounts(BaseModel, table=True):
     sort_order: Optional[float] = Field(default=None, sa_column=Column("sort_order", Float))
     type: Optional[str] = Field(default=None, sa_column=Column("type", Text))
     account_sync_source: Optional[str] = Field(default=None, sa_column=Column("account_sync_source", Text))
+    last_sync: Optional[str] = Field(default=None, sa_column=Column("last_sync", Text))
 
     payee: "Payees" = Relationship(back_populates="account", sa_relationship_kwargs={"uselist": False})
     transactions: List["Transactions"] = Relationship(
@@ -647,6 +648,7 @@ class Transactions(BaseModel, table=True):
     parent_id: Optional[str] = Field(default=None, sa_column=Column("parent_id", Text, ForeignKey("transactions.id")))
     schedule_id: Optional[str] = Field(default=None, sa_column=Column("schedule", Text, ForeignKey("schedules.id")))
     reconciled: Optional[int] = Field(default=None, sa_column=Column("reconciled", Integer, server_default=text("0")))
+    raw_synced_data: Optional[str] = Field(default=None, sa_column=Column("raw_synced_data", Text))
 
     account: "Accounts" = Relationship(back_populates="transactions")
     category: Optional["Categories"] = Relationship(
diff --git a/actual/queries.py b/actual/queries.py
index a01bc5b..fcf2726 100644
--- a/actual/queries.py
+++ b/actual/queries.py
@@ -204,7 +204,7 @@ def create_transaction_from_ids(
         notes=notes,
         reconciled=0,
         cleared=int(cleared),
-        sort_order=int(datetime.datetime.utcnow().timestamp()),
+        sort_order=int(datetime.datetime.now(datetime.timezone.utc).timestamp() * 1000),
         financial_id=imported_id,
         imported_description=imported_payee,
     )
diff --git a/actual/rules.py b/actual/rules.py
index 83f7ced..eafaab1 100644
--- a/actual/rules.py
+++ b/actual/rules.py
@@ -498,9 +498,7 @@ class Rule(pydantic.BaseModel):
 
     def set_split_amount(self, transaction: Transactions) -> typing.List[Transactions]:
         """Run the rules from setting split amounts."""
-        from actual.queries import (
-            create_split,  # lazy import to prevert circular issues
-        )
+        from actual.queries import create_split  # lazy import to prevert circular issues
 
         # get actions that split the transaction
         split_amount_actions = [action for action in self.actions if action.op == ActionType.SET_SPLIT_AMOUNT]
diff --git a/docker/docker-compose.yml b/docker/docker-compose.yml
index 07af25a..768eaee 100644
--- a/docker/docker-compose.yml
+++ b/docker/docker-compose.yml
@@ -2,7 +2,7 @@
 services:
   actual:
     container_name: actual
-    image: docker.io/actualbudget/actual-server:25.2.0
+    image: docker.io/actualbudget/actual-server:25.3.0
     ports:
       - '5006:5006'
     volumes:
diff --git a/pyproject.toml b/pyproject.toml
index 06150f0..e31ff3e 100644
--- a/pyproject.toml
+++ b/pyproject.toml
@@ -65,3 +65,4 @@ max-complexity = 18
 
 [tool.isort]
 profile = "black"
+line_length = 120


Test patch:
diff --git a/tests/test_bank_sync.py b/tests/test_bank_sync.py
index 23597b3..67b6cd4 100644
--- a/tests/test_bank_sync.py
+++ b/tests/test_bank_sync.py
@@ -6,6 +6,7 @@ import pytest
 from requests import Session
 
 from actual import Actual, ActualBankSyncError
+from actual.api.bank_sync import TransactionItem
 from actual.database import Banks
 from actual.queries import create_account
 from tests.conftest import RequestsMock
@@ -30,7 +31,9 @@ response = {
                 "valueDate": "2024-06-13",
                 "date": "2024-06-13",
                 "transactionAmount": {"amount": "9.26", "currency": "EUR"},
+                "payeeName": "John Doe",
                 "debtorName": "John Doe",
+                "notes": "Transferring Money",
                 "remittanceInformationUnstructured": "Transferring Money",
                 "booked": True,
             },
@@ -43,6 +46,8 @@ response = {
                 "valueDate": "2024-06-13",
                 "date": "2024-06-13",
                 "transactionAmount": {"amount": "-7.77", "currency": "EUR"},
+                "payeeName": "Institution Gmbh (DE12 XXX 6789)",
+                "notes": "Payment",
                 "creditorName": "Institution GmbH",
                 "creditorAccount": {"iban": "DE123456789"},
                 "remittanceInformationUnstructured": "Payment",
@@ -54,7 +59,7 @@ response = {
             # ignored since booked is set to false, but all required fields are also missing
             {
                 "date": "2024-06-13",
-                "transactionAmount": {"amount": "-7.77", "currency": "EUR"},
+                "transactionAmount": {"amount": "0.00", "currency": "EUR"},
                 "booked": False,
             },
         ],
@@ -116,12 +121,13 @@ def test_full_bank_sync_go_cardless(session, bank_sync_data_match):
         # now try to run the bank sync
         imported_transactions = actual.run_bank_sync()
         session.commit()
-        assert len(imported_transactions) == 2
-        assert imported_transactions[0].financial_id == "208584e9-343f-4831-8095-7b9f4a34a77e"
+        assert len(imported_transactions) == 3
+        assert imported_transactions[0].financial_id is None
         assert imported_transactions[0].get_date() == datetime.date(2024, 6, 13)
-        assert imported_transactions[0].get_amount() == decimal.Decimal("9.26")
-        assert imported_transactions[0].payee.name == "John Doe"
-        assert imported_transactions[0].notes == "Transferring Money"
+        # goCardless provides the correct starting balance
+        assert imported_transactions[0].get_amount() == decimal.Decimal("1.49")
+        assert imported_transactions[0].payee.name == "Starting Balance"
+        assert imported_transactions[0].notes is None
 
         assert imported_transactions[1].financial_id == "a2c2fafe-334a-46a6-8d05-200c2e41397b"
         assert imported_transactions[1].get_date() == datetime.date(2024, 6, 13)
@@ -129,6 +135,15 @@ def test_full_bank_sync_go_cardless(session, bank_sync_data_match):
         # the name of the payee was normalized (from GmbH to Gmbh) and the masked iban is included
         assert imported_transactions[1].payee.name == "Institution Gmbh (DE12 XXX 6789)"
         assert imported_transactions[1].notes == "Payment"
+        # also test the iban generation functions
+        loaded_transaction = TransactionItem.model_validate(response["transactions"]["all"][1])
+        assert imported_transactions[1].payee.name == loaded_transaction.imported_payee
+
+        assert imported_transactions[2].financial_id == "208584e9-343f-4831-8095-7b9f4a34a77e"
+        assert imported_transactions[2].get_date() == datetime.date(2024, 6, 13)
+        assert imported_transactions[2].get_amount() == decimal.Decimal("9.26")
+        assert imported_transactions[2].payee.name == "John Doe"
+        assert imported_transactions[2].notes == "Transferring Money"
 
         # the next call should do nothing
         new_imported_transactions = actual.run_bank_sync()
@@ -140,28 +155,28 @@ def test_full_bank_sync_go_cardless(session, bank_sync_data_match):
 def test_full_bank_sync_go_simplefin(session, bank_sync_data_match):
     with Actual(token="foo") as actual:
         actual._session = session
-        create_accounts(session, "simplefin")
+        create_accounts(session, "simpleFin")
 
         # now try to run the bank sync
         imported_transactions = actual.run_bank_sync("Bank")
         assert len(imported_transactions) == 2
-        assert imported_transactions[0].financial_id == "208584e9-343f-4831-8095-7b9f4a34a77e"
+        assert imported_transactions[0].financial_id == "a2c2fafe-334a-46a6-8d05-200c2e41397b"
         assert imported_transactions[0].get_date() == datetime.date(2024, 6, 13)
-        assert imported_transactions[0].get_amount() == decimal.Decimal("9.26")
-        assert imported_transactions[0].payee.name == "Transferring Money"  # simplefin uses the wrong field
-        assert imported_transactions[0].notes == "Transferring Money"
+        assert imported_transactions[0].get_amount() == decimal.Decimal("-7.77")
+        assert imported_transactions[0].payee.name == "Payment"  # simplefin uses the wrong field
+        assert imported_transactions[0].notes == "Payment"
 
-        assert imported_transactions[1].financial_id == "a2c2fafe-334a-46a6-8d05-200c2e41397b"
+        assert imported_transactions[1].financial_id == "208584e9-343f-4831-8095-7b9f4a34a77e"
         assert imported_transactions[1].get_date() == datetime.date(2024, 6, 13)
-        assert imported_transactions[1].get_amount() == decimal.Decimal("-7.77")
-        assert imported_transactions[1].payee.name == "Payment"  # simplefin uses the wrong field
-        assert imported_transactions[1].notes == "Payment"
+        assert imported_transactions[1].get_amount() == decimal.Decimal("9.26")
+        assert imported_transactions[1].payee.name == "Transferring Money"  # simplefin uses the wrong field
+        assert imported_transactions[1].notes == "Transferring Money"
 
 
 def test_bank_sync_with_starting_balance(session, bank_sync_data_no_match):
     with Actual(token="foo") as actual:
         actual._session = session
-        create_accounts(session, "simplefin")
+        create_accounts(session, "simpleFin")
         # now try to run the bank sync
         imported_transactions = actual.run_bank_sync("Bank")
         assert len(imported_transactions) == 3
diff --git a/tests/test_cli.py b/tests/test_cli.py
index 139a95a..f98ae99 100644
--- a/tests/test_cli.py
+++ b/tests/test_cli.py
@@ -14,7 +14,7 @@ from actual.cli.config import Config, default_config_path
 from actual.queries import create_account, create_transaction
 
 runner = CliRunner()
-server_version = "25.2.0"
+server_version = "25.3.0"
 
 
 def base_dataset(actual: Actual, budget_name: str = "Test", encryption_password: str = None):
diff --git a/tests/test_integration.py b/tests/test_integration.py
index f8984ef..e87a7f8 100644
--- a/tests/test_integration.py
+++ b/tests/test_integration.py
@@ -22,7 +22,7 @@ from actual.queries import (
     get_transactions,
 )
 
-VERSIONS = ["25.2.0"]
+VERSIONS = ["25.3.0"]
 
 
 @pytest.fixture(params=VERSIONS)  # todo: support multiple versions at once