Repository: bvanelli/actualpy

Issue:
Assigning transactions as transfers between two accounts
### Description

I was able to create rules, but I'm not sure how to assign transactions as transfers between two accounts. Can you provide guidance on how to do this?

I tried to set `Action(field="description", value=account_id)`, but it would show the payee as `(deleted)` and thus cannot be applied to matching transaction. Would it be possible to set `Action(field="payee", value=account_id)`?

Reference: [Make a transfer from two existing transactions](https://actualbudget.org/docs/transactions/transfers/#make-a-transfer-from-two-existing-transactions)

Discussion:
@bvanelli: Hello,

Each transfer is an individual payee, and this payee can be taken using the function `get_payees`. The field on the payee is called `transfer_acct` for indicating which account it represents. You can get them with:

```python
account = get_account(session, "My Account")
transfer_payee_id = accout.payee.id
```

Or you can get a list of them:

```python
transfer_payees = [payee for payee in get_payees(actual.session) if payee.account]
```

Problem is, my actions currently do not support this transfers, because a transfer *is not* just setting the payee, but you need to create a corresponding transaction on the other account.

I believe I can implement it at some point, but not as of now, as it is a bit more complicated than it seems, as:

- The two transactions might not have the same date, and you have to correct it
- Second transaction might be missing, so you have to create it
- Second transaction might be unrelated, even though it has the same value

I'm not sure how actual handles all edge cases, and would have to check that to create an implementation. Do you want to run a "transfer rule" on your entire budget transactions?@bvanelli: Ok, so I went through the source code from Actual to figure out how this is handled by them. First of all, the transfers are processed under `batchUpdateTransactions`:

https://github.com/actualbudget/actual/blob/f09f4af667ddd57e031dcdb0d428ae935aa2afad/packages/loot-core/src/server/accounts/transactions.ts#L136-L147

It will then go through the transfer rules and update the transaction based on that:

https://github.com/actualbudget/actual/blob/f09f4af667ddd57e031dcdb0d428ae935aa2afad/packages/loot-core/src/server/accounts/transfer.ts#L81-L90

Currently, the user can assign any payee id on current actualpy, and it would not create the second transaction, causing the balances to be off. I would say that we do the following:

- Add a hook when modifying the payee field
- If a payee from an account is removed, the remote transaction will be too
- If a payee from an account is added, the remote transaction will be added too
- This would make the rule work as expected, but also prevent the user from breaking the local database.
- One more thing, if the transfer account is offbudget, the [category is cleared](https://github.com/actualbudget/actual/blob/f09f4af667ddd57e031dcdb0d428ae935aa2afad/packages/loot-core/src/server/accounts/transfer.ts#L19-L42)

This is not a simple bug fix but I'll tackle this at some point.@JonnyTran: Thanks for looking over this!

I found another reference in case it's useful for you. [This SQL command](https://actualbudget.org/docs/advanced/scripts/modify-transfers) doesn't create rules, but it works well to identify and apply transfers to historical transactions retroactively.

Even though currently on actualpy I cannot set a Transfer Payee on a Rule, there's workaround by going to the UI and selecting payee accounts manually at the Rules page.@bvanelli: I actually played around with it a bit, and push https://github.com/bvanelli/actualpy/pull/110 together, do you mind giving it a try? 

As far as I tried it could not corrupt one of my demo budgets, but you never know, have a backup. The worst that can happen was having unbalanced transactions.

It still cannot resolve the payee fully, as I does not look for a probable transaction on the other side. I'd have to implement this next.



Solution patch:
diff --git a/actual/database.py b/actual/database.py
index bad19b1..22fd998 100644
--- a/actual/database.py
+++ b/actual/database.py
@@ -635,7 +635,9 @@ class Transactions(BaseModel, table=True):
         default=None,
         sa_column=Column("starting_balance_flag", Integer, server_default=text("0")),
     )
-    transferred_id: Optional[str] = Field(default=None, sa_column=Column("transferred_id", Text))
+    transferred_id: Optional[str] = Field(
+        default=None, sa_column=Column("transferred_id", Text, ForeignKey("transactions.id"))
+    )
     sort_order: Optional[float] = Field(default=None, sa_column=Column("sort_order", Float))
     tombstone: Optional[int] = Field(default=None, sa_column=Column("tombstone", Integer, server_default=text("0")))
     cleared: Optional[int] = Field(default=None, sa_column=Column("cleared", Integer, server_default=text("1")))
@@ -662,7 +664,8 @@ class Transactions(BaseModel, table=True):
         },
     )
     parent: Optional["Transactions"] = Relationship(
-        back_populates="splits", sa_relationship_kwargs={"remote_side": "Transactions.id"}
+        back_populates="splits",
+        sa_relationship_kwargs={"remote_side": "Transactions.id", "foreign_keys": "Transactions.parent_id"},
     )
     splits: List["Transactions"] = Relationship(
         back_populates="parent",
@@ -671,6 +674,12 @@ class Transactions(BaseModel, table=True):
             "order_by": "remote(Transactions.sort_order.desc())",
         },
     )
+    transfer: Optional["Transactions"] = Relationship(
+        sa_relationship_kwargs={
+            "primaryjoin": "and_(Transactions.transferred_id == remote(Transactions.id), remote(Transactions.tombstone)==0)",
+            "foreign_keys": "Transactions.transferred_id",
+        }
+    )
 
     def get_date(self) -> datetime.date:
         """Returns the transaction date as a datetime.date object, instead of as a string."""
diff --git a/actual/queries.py b/actual/queries.py
index a4843f8..a01bc5b 100644
--- a/actual/queries.py
+++ b/actual/queries.py
@@ -191,6 +191,7 @@ def create_transaction_from_ids(
     imported_id: str = None,
     cleared: bool = False,
     imported_payee: str = None,
+    process_payee: bool = True,
 ) -> Transactions:
     """Internal method to generate a transaction from ids instead of objects."""
     date_int = int(datetime.date.strftime(date, "%Y%m%d"))
@@ -200,7 +201,6 @@ def create_transaction_from_ids(
         date=date_int,
         amount=int(round(amount * 100)),
         category_id=category_id,
-        payee_id=payee_id,
         notes=notes,
         reconciled=0,
         cleared=int(cleared),
@@ -209,6 +209,8 @@ def create_transaction_from_ids(
         imported_description=imported_payee,
     )
     s.add(t)
+    if process_payee:
+        set_transaction_payee(s, t, payee_id)
     return t
 
 
@@ -264,6 +266,49 @@ def create_transaction(
     )
 
 
+def set_transaction_payee(s: Session, transaction: Transactions, payee: typing.Union[Payees, str, None]) -> None:
+    """
+    Sets a payee safely by checking if this payee is a transfer. If it is, then the transfer will be created.
+
+    This is necessary since the payee can be set to a "transfer id", which references and account. When this happens,
+    the transaction will be marked as a transfer between the two accounts, and new transaction will need to be created
+    on the other account, with the negative amount.
+
+    :param s: session from Actual local database.
+    :param transaction: transaction to exchange the payee.
+    :param payee: object or unique id of the payee to be set. Must be existing
+    """
+    current_payee: typing.Optional[Payees] = None
+    if isinstance(payee, str):
+        payee = s.scalar(select(Payees).where(Payees.id == payee))
+    if transaction.payee_id:  # resolve based on the id, in case the relationship did not load
+        current_payee = s.scalar(select(Payees).where(Payees.id == transaction.payee_id))
+    # if old payee was a transfer, we delete that transfer based on the `transferred_id`
+    if current_payee and current_payee.transfer_acct:
+        old_tr = s.scalar(select(Transactions).where(Transactions.id == transaction.transferred_id))
+        if old_tr:  # should exist, but check for safety
+            old_tr.delete()
+            transaction.transferred_id = None
+    # if setting a transfer payee, we create a transfer
+    if payee and payee.transfer_acct:
+        transfer = create_transaction_from_ids(
+            s,
+            transaction.get_date(),
+            payee.account.id,
+            transaction.account.payee.id,
+            transaction.notes,
+            None,
+            cleared=bool(transaction.cleared),
+            amount=-transaction.get_amount(),
+            process_payee=False,
+        )
+        transaction.category_id = None
+        transfer.transferred_id, transaction.transferred_id = transaction.id, transfer.id
+
+    # finally set the payee
+    transaction.payee_id = payee.id if payee else None
+
+
 def normalize_payee(payee_name: str | None, raw_payee_name: bool = False) -> str:
     """
     Normalizes the payees according to the source code found at the [official source code](
@@ -685,14 +730,18 @@ def create_transfer(
     :param amount: amount, as a positive decimal, to be transferred.
     :param notes: additional description for the transfer.
     :return: tuple containing both transactions, as one is created per account. The transactions would be
-    cross-referenced by their `transferred_id`.
+             cross-referenced by their `transferred_id`.
     """
     if amount <= 0:
         raise ActualError("Amount must be a positive value.")
     source: Accounts = get_account(s, source_account)
     dest: Accounts = get_account(s, dest_account)
-    source_transaction = create_transaction_from_ids(s, date, source.id, dest.payee.id, notes, None, -amount)
-    dest_transaction = create_transaction_from_ids(s, date, dest.id, source.payee.id, notes, None, amount)
+    source_transaction = create_transaction_from_ids(
+        s, date, source.id, dest.payee.id, notes, None, -amount, process_payee=False
+    )
+    dest_transaction = create_transaction_from_ids(
+        s, date, dest.id, source.payee.id, notes, None, amount, process_payee=False
+    )
     # swap the transferred ids
     source_transaction.transferred_id = dest_transaction.id
     dest_transaction.transferred_id = source_transaction.id
diff --git a/actual/rules.py b/actual/rules.py
index 20f9110..83f7ced 100644
--- a/actual/rules.py
+++ b/actual/rules.py
@@ -432,6 +432,14 @@ def run(self, transaction: Transactions) -> None:
             # set the value
             if self.type == ValueType.DATE:
                 transaction.set_date(value)
+            elif attr == "payee_id":
+                # this has to be handled separately since, when setting a transfer payee, a transfer transaction needs
+                # to be created
+                from actual.queries import set_transaction_payee
+
+                # get inner session from object
+                session = transaction._sa_instance_state.session  # noqa
+                set_transaction_payee(session, transaction, value)
             else:
                 setattr(transaction, attr, value)
         elif self.op == ActionType.LINK_SCHEDULE:


Test patch:
diff --git a/tests/conftest.py b/tests/conftest.py
index 3e932d3..8326bf0 100644
--- a/tests/conftest.py
+++ b/tests/conftest.py
@@ -32,5 +32,5 @@ def session():
         sqlite_url = f"sqlite:///{f.name}"
         engine = create_engine(sqlite_url, connect_args={"check_same_thread": False})
         SQLModel.metadata.create_all(engine)
-        with Session(engine) as session:
+        with Session(engine, autoflush=True) as session:
             yield strong_reference_session(session)
diff --git a/tests/test_database.py b/tests/test_database.py
index 0ce8405..f3072d5 100644
--- a/tests/test_database.py
+++ b/tests/test_database.py
@@ -25,6 +25,7 @@
     get_transactions,
     normalize_payee,
     reconcile_transaction,
+    set_transaction_payee,
 )
 from actual.rules import Action, Condition, ConditionType, Rule
 
@@ -323,3 +324,28 @@ def test_get_preferences(session):
     new_preferences = get_preferences(session)
     assert len(new_preferences) == 1
     assert new_preferences[0].value == "foobar"
+
+
+def test_set_payee_to_transfer(session):
+    wallet = create_account(session, "Wallet")
+    bank = create_account(session, "Bank")
+    session.commit()
+    # Create a transaction setting the payee
+    t = create_transaction(session, date.today(), bank, wallet.payee, amount=-50)
+    session.commit()
+    transactions = get_transactions(session)
+    assert len(transactions) == 2
+    assert transactions[0].get_amount() == -transactions[1].get_amount()
+    assert transactions[0].transferred_id == transactions[1].id
+    assert transactions[1].transferred_id == transactions[0].id
+    # Set this payee to something else, transaction should be deleted
+    set_transaction_payee(session, t, None)
+    session.commit()
+    assert len(get_transactions(session)) == 1
+    assert t.payee_id is None
+    assert t.transferred_id is None
+    # Set payee_id back, transaction should be recreated
+    set_transaction_payee(session, t, wallet.payee.id)
+    session.commit()
+    assert t.payee_id == wallet.payee.id
+    assert t.transfer.transfer == t
diff --git a/tests/test_rules.py b/tests/test_rules.py
index 273fcde..58754ec 100644
--- a/tests/test_rules.py
+++ b/tests/test_rules.py
@@ -1,6 +1,5 @@
 import datetime
 import uuid
-from unittest.mock import MagicMock
 
 import pytest
 
@@ -24,12 +23,11 @@
 )
 
 
-def test_category_rule():
-    mock = MagicMock()
+def test_category_rule(session):
     # create basic items
-    acct = create_account(mock, "Bank")
-    cat = create_category(mock, "Food", "Expenses")
-    payee = create_payee(mock, "My payee")
+    acct = create_account(session, "Bank")
+    cat = create_category(session, "Food", "Expenses")
+    payee = create_payee(session, "My payee")
     # create rule
     condition = Condition(field="category", op="is", value=cat)
     action = Action(field="description", value=payee)
@@ -38,7 +36,7 @@ def test_category_rule():
     assert list(rs) == []
     rs.add(rule)
     # run for one transaction
-    t = create_transaction(mock, datetime.date(2024, 1, 1), acct, "", category=cat)
+    t = create_transaction(session, datetime.date(2024, 1, 1), acct, "", category=cat)
     rs.run(t)
     # evaluate if things match
     assert t.payee_id == payee.id
@@ -51,10 +49,9 @@ def test_category_rule():
     assert condition.run(t) is False
 
 
-def test_datetime_rule():
-    mock = MagicMock()
-    acct = create_account(mock, "Bank")
-    t = create_transaction(mock, datetime.date(2024, 1, 1), acct, "")
+def test_datetime_rule(session):
+    acct = create_account(session, "Bank")
+    t = create_transaction(session, datetime.date(2024, 1, 1), acct, "")
     condition = Condition(field="date", op="isapprox", value=datetime.date(2024, 1, 2))
     action = Action(field="date", value="2024-01-30")
     rs = RuleSet(rules=[Rule(conditions=[condition], actions=[action], operation="any")])
@@ -75,10 +72,9 @@ def test_datetime_rule():
     assert Condition(field="date", op="lt", value=target_date + datetime.timedelta(days=1)).run(t) is True
 
 
-def test_string_condition():
-    mock = MagicMock()
-    acct = create_account(mock, "Bank")
-    t = create_transaction(mock, datetime.date(2024, 1, 1), acct, "", "foo")
+def test_string_condition(session):
+    acct = create_account(session, "Bank")
+    t = create_transaction(session, datetime.date(2024, 1, 1), acct, "", "foo")
     assert Condition(field="notes", op="oneOf", value=["foo", "bar"]).run(t) is True
     assert Condition(field="notes", op="notOneOf", value=["foo", "bar"]).run(t) is False
     assert Condition(field="notes", op="contains", value="fo").run(t) is True
@@ -98,10 +94,9 @@ def test_string_condition():
     assert Condition(field="notes", op="doesNotContain", value="FOOBAR").run(t) is True
 
 
-def test_has_tags():
-    mock = MagicMock()
-    acct = create_account(mock, "Bank")
-    t = create_transaction(mock, datetime.date(2024, 1, 1), acct, "", "foo #bar #✨ #🙂‍↔️")
+def test_has_tags(session):
+    acct = create_account(session, "Bank")
+    t = create_transaction(session, datetime.date(2024, 1, 1), acct, "", "foo #bar #✨ #🙂‍↔️")
     assert Condition(field="notes", op="hasTags", value="#bar").run(t) is True
     assert Condition(field="notes", op="hasTags", value="#foo").run(t) is False
     # test other unicode entries
@@ -120,15 +115,17 @@ def test_has_tags():
         ("matches", "market", "hypermarket", True),
     ],
 )
-def test_imported_payee_condition(op, condition_value, value, expected_result):
-    t = create_transaction(MagicMock(), datetime.date(2024, 1, 1), "Bank", "", amount=5, imported_payee=value)
+def test_imported_payee_condition(session, op, condition_value, value, expected_result):
+    create_account(session, "Bank")
+    t = create_transaction(session, datetime.date(2024, 1, 1), "Bank", "", amount=5, imported_payee=value)
     condition = {"field": "imported_description", "type": "imported_payee", "op": op, "value": condition_value}
     cond = Condition.model_validate(condition)
     assert cond.run(t) == expected_result
 
 
-def test_numeric_condition():
-    t = create_transaction(MagicMock(), datetime.date(2024, 1, 1), "Bank", "", amount=5)
+def test_numeric_condition(session):
+    create_account(session, "Bank")
+    t = create_transaction(session, datetime.date(2024, 1, 1), "Bank", "", amount=5)
     c1 = Condition(field="amount_inflow", op="gt", value=10.0)
     assert "inflow" in c1.options
     assert c1.run(t) is False
@@ -146,13 +143,12 @@ def test_numeric_condition():
     assert str(c4) == "'amount' isbetween (500, 1000)"  # value gets converted when input as float
 
 
-def test_complex_rule():
-    mock = MagicMock()
+def test_complex_rule(session):
     # create basic items
-    acct = create_account(mock, "Bank")
-    cat = create_category(mock, "Food", "Expenses")
-    cat_extra = create_category(mock, "Restaurants", "Expenses")
-    payee = create_payee(mock, "My payee")
+    acct = create_account(session, "Bank")
+    cat = create_category(session, "Food", "Expenses")
+    cat_extra = create_category(session, "Restaurants", "Expenses")
+    payee = create_payee(session, "My payee")
     # create rule set
     rs = RuleSet(
         rules=[
@@ -175,8 +171,8 @@ def test_complex_rule():
             )
         ]
     )
-    t_true = create_transaction(mock, datetime.date(2024, 1, 1), acct, payee, category=cat)
-    t_false = create_transaction(mock, datetime.date(2024, 1, 1), acct, payee)
+    t_true = create_transaction(session, datetime.date(2024, 1, 1), acct, payee, category=cat)
+    t_false = create_transaction(session, datetime.date(2024, 1, 1), acct, payee)
     rs.run([t_true, t_false])
     assert t_true.cleared == 1
     assert t_false.cleared == 0
@@ -376,12 +372,24 @@ def test_set_split_amount_exception(session, mocker):
         ("prepend-notes", "bar", None, "bar"),
     ],
 )
-def test_preppend_append_notes(operation, value, note, expected):
-    mock = MagicMock()
-    t = create_transaction(mock, datetime.date(2024, 1, 1), "Bank", "", notes=note)
+def test_preppend_append_notes(session, operation, value, note, expected):
+    create_account(session, "Bank")
+    t = create_transaction(session, datetime.date(2024, 1, 1), "Bank", "", notes=note)
     action = Action(field="description", op=operation, value=value)
     action.run(t)
     assert t.notes == expected
     action.run(t)  # second iteration should not update the result
     assert t.notes == expected
     assert f"{operation.split('-')[0]} to notes '{value}'" in str(action)
+
+
+def test_set_transfer_payee_rule(session):
+    bank = create_account(session, "Bank")
+    t = create_transaction(session, datetime.date(2024, 1, 1), "Bank", amount=10)
+    action = Action(field="description", op="set", value=bank.payee.id)
+    action.run(t)
+    assert t.transferred_id is not None
+    assert t.transfer is not None
+    assert t.get_amount() == -t.transfer.get_amount()
+    assert t.id == t.transfer.transferred_id
+    assert t.transferred_id == t.transfer.id