From 448e9c8f5c6d3f12d17fdb85950577f30435b8e5 Mon Sep 17 00:00:00 2001 From: Herculino Trotta Date: Sun, 8 Nov 2026 17:57:56 -0300 Subject: [PATCH 1/2] fix(api): save income categories, tags and entities on account create and update AccountSerializer assigned the many-to-many fields with setattr, which Django rejects, so any write that included them returned a 500. The default ModelSerializer create/update already handles them. --- app/apps/api/serializers/accounts.py | 9 ------- app/apps/api/tests/test_accounts.py | 39 +++++++++++++++++++++++++++- 2 files changed, 38 insertions(+), 10 deletions(-) diff --git a/app/apps/api/serializers/accounts.py b/app/apps/api/serializers/accounts.py index 02082ba1..3345187e 100644 --- a/app/apps/api/serializers/accounts.py +++ b/app/apps/api/serializers/accounts.py @@ -97,15 +97,6 @@ class AccountSerializer(serializers.ModelSerializer): self.fields["group_id"].queryset = AccountGroup.objects.all() self.fields["investment_class_id"].queryset = InvestmentClass.objects.all() - def create(self, validated_data): - return Account.objects.create(**validated_data) - - def update(self, instance, validated_data): - for attr, value in validated_data.items(): - setattr(instance, attr, value) - instance.save() - return instance - class AccountBalanceSerializer(serializers.Serializer): """Serializer for account balance response.""" diff --git a/app/apps/api/tests/test_accounts.py b/app/apps/api/tests/test_accounts.py index 9da60c95..929087b3 100644 --- a/app/apps/api/tests/test_accounts.py +++ b/app/apps/api/tests/test_accounts.py @@ -8,7 +8,7 @@ from rest_framework.test import APIClient from apps.accounts.models import Account, AccountGroup from apps.currencies.models import Currency -from apps.transactions.models import Transaction +from apps.transactions.models import Transaction, TransactionCategory @override_settings( @@ -97,3 +97,40 @@ class AccountBalanceAPITests(TestCase): ) self.assertEqual(response.status_code, status.HTTP_401_UNAUTHORIZED) + + def test_patch_and_create_with_income_categories(self): + category = TransactionCategory.objects.create(name="Interest", owner=self.user) + self.account.owner = self.user + self.account.save() + # Writes need Django model permissions + self.user.is_superuser = True + self.user.save() + + response = self.client.patch( + f"/api/accounts/{self.account.id}/", + {"income_categories": [category.id]}, + format="json", + ) + self.assertEqual(response.status_code, status.HTTP_200_OK, response.data) + self.assertEqual(response.data["income_categories"], [category.id]) + # The join table: the related manager is per-user and empty without one + self.assertTrue( + Account.income_categories.through.objects.filter( + account=self.account, transactioncategory=category + ).exists() + ) + + response = self.client.post( + "/api/accounts/", + { + "name": "Broker", + "group_id": None, + "currency_id": self.currency.id, + "exchange_currency_id": None, + "is_investment": True, + "income_categories": [category.id], + }, + format="json", + ) + self.assertEqual(response.status_code, status.HTTP_201_CREATED, response.data) + self.assertEqual(response.data["income_categories"], [category.id]) From fda4bade62b49cce8575855aaeda8f443584be11 Mon Sep 17 00:00:00 2001 From: Herculino Trotta Date: Sun, 8 Nov 2026 18:05:17 -0300 Subject: [PATCH 2/2] fix(investments): count income categories on unit-priced accounts and transfers at their own amount - Unit-priced accounts now honour their income categories, tags and entities: a matching transaction is a gain or loss, and any other unlinked one is money in or out at that day's market price. Before, every unlinked outflow was a loss and yields were booked as new money in. - A transfer out of an account tracked by value counts its own amount. It used the other side converted at the day's rate, so a coin bought from a savings account moved the coin's price change onto the savings account. --- ...21_alter_account_income_categories_help.py | 19 +++++ app/apps/accounts/models.py | 4 +- app/apps/investments/services.py | 73 +++++++++---------- app/apps/investments/tests/test_services.py | 36 ++++++++- .../investments/fragments/detail.html | 4 +- 5 files changed, 93 insertions(+), 43 deletions(-) create mode 100644 app/apps/accounts/migrations/0021_alter_account_income_categories_help.py diff --git a/app/apps/accounts/migrations/0021_alter_account_income_categories_help.py b/app/apps/accounts/migrations/0021_alter_account_income_categories_help.py new file mode 100644 index 00000000..30ced679 --- /dev/null +++ b/app/apps/accounts/migrations/0021_alter_account_income_categories_help.py @@ -0,0 +1,19 @@ +# Generated by Django 5.2.17 on 2026-11-08 20:59 + +from django.db import migrations, models + + +class Migration(migrations.Migration): + + dependencies = [ + ('accounts', '0020_investment_classes'), + ('transactions', '0052_transfer_templates'), + ] + + operations = [ + migrations.AlterField( + model_name='account', + name='income_categories', + field=models.ManyToManyField(blank=True, help_text="Transactions in this account with any of these categories are its result (interest, taxes, fees). Everything else that isn't a transfer counts as money coming in or going out.", related_name='investment_accounts', to='transactions.transactioncategory', verbose_name='Income categories'), + ), + ] diff --git a/app/apps/accounts/models.py b/app/apps/accounts/models.py index 79d5e383..58dd0a70 100644 --- a/app/apps/accounts/models.py +++ b/app/apps/accounts/models.py @@ -151,8 +151,8 @@ class Account(SharedObject): verbose_name=_("Income categories"), help_text=_( "Transactions in this account with any of these categories are its result " - "(interest, taxes, fees). On accounts tracked by value, everything else " - "that isn't a transfer counts as money coming in or going out." + "(interest, taxes, fees). Everything else that isn't a transfer counts as " + "money coming in or going out." ), ) income_tags = models.ManyToManyField( diff --git a/app/apps/investments/services.py b/app/apps/investments/services.py index a6371ada..1f666ccd 100644 --- a/app/apps/investments/services.py +++ b/app/apps/investments/services.py @@ -1,15 +1,14 @@ """Investment performance, derived from an account's transactions. Rules (see the account's "Investment" help text): -- A linked transaction (a transfer) moves money in or out. Its other side says - what was paid or received, which is the cost or the proceeds. -- An unlinked transaction on an account tracked by value is money coming in or - going out (a salary, a bill), unless it has one of the account's income - categories, tags or entities: then it is a gain or a loss (a reconciled balance, - interest, a tax withheld). -- On a unit-priced account an unlinked outflow is a loss, and an unlinked inflow - (a gift, an opening balance) has no known cost, so it is costed at the market - price of that day and the result is flagged as estimated. +- A linked transaction (a transfer) moves money in or out. On a unit-priced + account its other side says what was paid or received, which is the cost or + the proceeds. On an account tracked by value its own amount is. +- An unlinked transaction is money coming in or going out (a salary, a bill, a + gift), unless it has one of the account's income categories, tags or entities: + then it is a gain or a loss (a reconciled balance, interest, a tax withheld). +- On a unit-priced account, money that moves without a known other side is + valued at the market price of that day and the result is flagged as estimated. - Cost is tracked with the average cost method. """ @@ -42,7 +41,7 @@ class HoldingPerformance: contributed: Decimal = ZERO withdrawn: Decimal = ZERO estimated: bool = False - # Unlinked transactions counted as gains or losses, on accounts tracked by value + # Unlinked transactions counted as gains or losses result_ids: set = field(default_factory=set) history: list = field(default_factory=list) # (date, "buy" or "sell", unit price) for linked trades on unit-priced accounts @@ -152,8 +151,7 @@ def get_holding_performance(account, as_of=None, with_history=False): ) month_ends = _month_ends(transactions, as_of) if with_history else [] - if not perf.priced: - perf.result_ids = _result_ids(account) + perf.result_ids = _result_ids(account) for tx in transactions: while month_ends and month_ends[0] < tx.date: @@ -163,34 +161,35 @@ def get_holding_performance(account, as_of=None, with_history=False): partner = tx.linked_transaction if partner and (partner.account_id == account.id or partner.deleted): partner = None - partner_value = ( - _to(partner.amount, partner.account.currency, valuation_currency, tx.date) - if partner - else None - ) - external = not (perf.priced or partner or tx.id in perf.result_ids) + # Money in or out, as opposed to a gain or a loss + moved = partner or tx.id not in perf.result_ids + partner_value = None + if perf.priced and partner: + partner_value = _to( + partner.amount, partner.account.currency, valuation_currency, tx.date + ) + if partner_value is not None: + value, estimated = partner_value, False + elif perf.priced: + # No known other side: that day's market value + value = _to(tx.amount, account.currency, valuation_currency, tx.date) + estimated = True + else: + value, estimated = tx.amount, False + if perf.priced and partner_value is not None and tx.amount: kind = "buy" if tx.type == Transaction.Type.INCOME else "sell" perf.trades.append((tx.date, kind, partner_value / tx.amount)) - elif not (perf.priced or partner or external): + elif not (perf.priced or moved): sign = 1 if tx.type == Transaction.Type.INCOME else -1 perf.results.append((tx.date, sign * tx.amount)) if tx.type == Transaction.Type.INCOME: - if external: - cost = tx.amount - perf.contributed += cost - elif partner_value is not None: - cost = partner_value - perf.contributed += cost - elif perf.priced or partner: - # No known cost: price it at that day's market value - cost = _to(tx.amount, account.currency, valuation_currency, tx.date) - perf.estimated = True - if partner: - perf.contributed += cost or ZERO - else: - cost = ZERO # value-tracked: a gain + cost = ZERO # a gain + if moved: + cost = value + perf.contributed += cost or ZERO + perf.estimated |= estimated if cost: perf.flows.append((tx.date, -cost, "in")) perf.quantity += tx.amount @@ -200,11 +199,9 @@ def get_holding_performance(account, as_of=None, with_history=False): removed = perf.cost_basis * min(tx.amount, perf.quantity) / perf.quantity else: removed = ZERO - if partner or external: - proceeds = tx.amount if external else partner_value - if proceeds is None: - proceeds = _to(tx.amount, account.currency, valuation_currency, tx.date) or ZERO - perf.estimated = True + if moved: + proceeds = value or ZERO + perf.estimated |= estimated perf.realized += proceeds - removed perf.withdrawn += proceeds perf.flows.append((tx.date, proceeds, "out")) diff --git a/app/apps/investments/tests/test_services.py b/app/apps/investments/tests/test_services.py index 5ac80688..590c9b0d 100644 --- a/app/apps/investments/tests/test_services.py +++ b/app/apps/investments/tests/test_services.py @@ -101,7 +101,41 @@ class HoldingPerformanceTests(TestCase): self.assertEqual(perf.cost_basis, D("180")) self.assertTrue(perf.estimated) - self.assertEqual(perf.contributed, D("0")) + self.assertEqual(perf.contributed, D("180")) + + def test_priced_account_separates_money_from_yield(self): + # A foreign-currency account: pay in, interest, then money out unlinked + self.rate("100", self.d1) + result = TransactionCategory.objects.create(name="Interest") + self.etf.income_categories.set([result]) + tx(self.etf, IN, "10", self.d1) # paid in from outside + tx(self.etf, IN, "1", self.d2, category=result) + tx(self.etf, EX, "5", self.d3) # taken out, not a loss + + perf = get_holding_performance(self.etf) + + self.assertEqual(perf.contributed, D("1000")) + self.assertEqual(perf.withdrawn, D("500")) + self.assertEqual(perf.quantity, D("6")) + self.assertEqual(perf.market_value, D("600")) + # The interest is the whole return: 600 held + 500 out - 1000 in + self.assertEqual(perf.total_return, D("100")) + self.assertEqual(perf.results, []) + + def test_value_tracked_transfer_counts_its_own_amount(self): + # Paying 1000 for units now worth 900 is the units' loss, not the cash's + cash = Account.objects.create( + name="Savings", currency=self.eur, is_investment=True + ) + tx(cash, IN, "5000", self.d1) + transfer(cash, "1000", self.etf, "10", self.d2) + self.rate("90", timezone.localdate()) + + perf = get_holding_performance(cash) + + self.assertEqual(perf.withdrawn, D("1000")) + self.assertEqual(perf.total_return, D("0")) + self.assertEqual(get_holding_performance(self.etf).total_return, D("-100")) def test_value_tracked_cdb_with_tax_on_redemption(self): cdb = Account.objects.create( diff --git a/app/templates/investments/fragments/detail.html b/app/templates/investments/fragments/detail.html index 98f1ba04..10b744fb 100644 --- a/app/templates/investments/fragments/detail.html +++ b/app/templates/investments/fragments/detail.html @@ -45,7 +45,7 @@
{% if account.group %}{{ account.group.name }} · {% endif %} {% if perf.priced %}{{ a.name }} → {{ c.name }}{% else %}{% translate 'Tracked by value' %}{% endif %} - {% if perf.estimated %}{% endif %} + {% if perf.estimated %}{% endif %}
@@ -284,7 +284,7 @@ {% if tx.type == 'IN' %}{% translate 'Bought / contributed' %}{% else %}{% translate 'Sold / withdrew' %}{% endif %} · {{ p.account.name }} {% else %} - {% if not perf.priced and tx.id not in perf.result_ids %} + {% if tx.id not in perf.result_ids %} {% if tx.type == 'IN' %}{% translate 'Money in' %}{% else %}{% translate 'Money out' %}{% endif %} {% else %} {% if tx.type == 'IN' %}{% if perf.priced %}{% translate 'Gain' %}{% else %}{% translate 'Earnings' %}{% endif %}{% else %}{% translate 'Loss, tax or fee' %}{% endif %}