From 00d37d8c81e6dccc660d51292f26c6e3a1f94591 Mon Sep 17 00:00:00 2001 From: Herculino Trotta Date: Wed, 11 Nov 2026 10:22:50 -0300 Subject: [PATCH] refactor(currencies): keep the exchanged amounts display rule in one place Account.displayed_exchange_currency holds the account/currency precedence for transactions, and shown_exchange_currency() expresses the same rule as a query expression for the insights and account totals, replacing the per-module checks. --- app/apps/accounts/models.py | 13 +++++ app/apps/currencies/utils/display.py | 13 +++++ app/apps/insights/utils/month_by_month.py | 11 +--- app/apps/insights/utils/overview.py | 19 +++---- app/apps/insights/utils/year_by_year.py | 11 +--- app/apps/transactions/models.py | 62 ++++++++------------- app/apps/transactions/utils/calculations.py | 9 ++- 7 files changed, 68 insertions(+), 70 deletions(-) create mode 100644 app/apps/currencies/utils/display.py diff --git a/app/apps/accounts/models.py b/app/apps/accounts/models.py index 5a4d4079..c5f8aca7 100644 --- a/app/apps/accounts/models.py +++ b/app/apps/accounts/models.py @@ -199,6 +199,19 @@ class Account(SharedObject): user = get_current_user() return self.untracked_by.filter(pk=user.pk).exists() + @property + def displayed_exchange_currency(self): + """The exchange currency to show amounts in, or None when they're hidden. + + The account's exchange currency takes precedence over its currency's, so + whichever one provides it also decides whether it's shown. + """ + if self.exchange_currency_id: + return self.exchange_currency if self.show_exchanged_amounts else None + if self.currency.show_exchanged_amounts: + return self.currency.exchange_currency + return None + def clean(self): super().clean() if self.exchange_currency == self.currency: diff --git a/app/apps/currencies/utils/display.py b/app/apps/currencies/utils/display.py new file mode 100644 index 00000000..e2102725 --- /dev/null +++ b/app/apps/currencies/utils/display.py @@ -0,0 +1,13 @@ +from django.db.models import Case, F, When + + +def shown_exchange_currency(path): + """Query expression for the exchange currency id at ``path`` (an account or a + currency lookup, such as ``"account__currency__"``), or NULL when that object + hides its exchanged amounts.""" + return Case( + When( + **{f"{path}show_exchanged_amounts": True}, + then=F(f"{path}exchange_currency"), + ) + ) diff --git a/app/apps/insights/utils/month_by_month.py b/app/apps/insights/utils/month_by_month.py index ce5d97fd..4d90984e 100644 --- a/app/apps/insights/utils/month_by_month.py +++ b/app/apps/insights/utils/month_by_month.py @@ -8,6 +8,7 @@ from django.utils import timezone from apps.currencies.models import Currency from apps.currencies.utils.convert import convert +from apps.currencies.utils.display import shown_exchange_currency from apps.transactions.models import Transaction @@ -94,8 +95,7 @@ def get_month_by_month_data(year=None, group_by="categories"): "account__currency__decimal_places", "account__currency__prefix", "account__currency__suffix", - "account__currency__exchange_currency", - "account__currency__show_exchanged_amounts", + exchange_currency_id=shown_exchange_currency("account__currency__"), ) .annotate( expense_total=Coalesce( @@ -140,12 +140,7 @@ def get_month_by_month_data(year=None, group_by="categories"): item_name = metric[name_field] month = metric["reference_date__month"] currency_id = metric["account__currency"] - # Currencies can hide their exchanged amounts - exchange_currency_id = ( - metric["account__currency__exchange_currency"] - if metric["account__currency__show_exchanged_amounts"] - else None - ) + exchange_currency_id = metric["exchange_currency_id"] # Use a consistent key for None (uncategorized/untagged/no entity) item_key = item_id if item_id is not None else "__none__" diff --git a/app/apps/insights/utils/overview.py b/app/apps/insights/utils/overview.py index 8325356c..29b9008f 100644 --- a/app/apps/insights/utils/overview.py +++ b/app/apps/insights/utils/overview.py @@ -6,6 +6,7 @@ from django.db.models.functions import Coalesce from apps.currencies.models import Currency from apps.currencies.utils.convert import convert +from apps.currencies.utils.display import shown_exchange_currency from apps.transactions.models import Transaction # Grouping levels the overview can be built from. Any ordering of these keys is @@ -25,8 +26,6 @@ CURRENCY_FIELDS = ( "account__currency__decimal_places", "account__currency__prefix", "account__currency__suffix", - "account__currency__exchange_currency", - "account__currency__show_exchanged_amounts", ) # Aggregated amounts, plus the totals derived from them. Every one of these is @@ -80,7 +79,12 @@ def _sum_of(transaction_type, is_paid): def _aggregate(transactions_queryset, group_fields, name_field): """Sum income/expense per currency, grouped by the given fields.""" return ( - transactions_queryset.values(*group_fields, name_field, *CURRENCY_FIELDS) + transactions_queryset.values( + *group_fields, + name_field, + *CURRENCY_FIELDS, + exchange_currency_id=shown_exchange_currency("account__currency__"), + ) .annotate( expense_current=_sum_of(Transaction.Type.EXPENSE, is_paid=True), expense_projected=_sum_of(Transaction.Type.EXPENSE, is_paid=False), @@ -122,14 +126,9 @@ def _build_currency_data(metric, showing): "total": currency_data[total], } - if ( - metric["account__currency__exchange_currency"] - and metric["account__currency__show_exchanged_amounts"] - ): + if metric["exchange_currency_id"]: from_currency = Currency.objects.get(id=metric["account__currency"]) - exchange_currency = Currency.objects.get( - id=metric["account__currency__exchange_currency"] - ) + exchange_currency = Currency.objects.get(id=metric["exchange_currency_id"]) exchanged = {} for field in AGGREGATED_FIELDS + TOTAL_FIELDS: diff --git a/app/apps/insights/utils/year_by_year.py b/app/apps/insights/utils/year_by_year.py index 6d5dfc1f..1bae0a86 100644 --- a/app/apps/insights/utils/year_by_year.py +++ b/app/apps/insights/utils/year_by_year.py @@ -7,6 +7,7 @@ from django.db.models.functions import Coalesce from apps.currencies.models import Currency from apps.currencies.utils.convert import convert +from apps.currencies.utils.display import shown_exchange_currency from apps.transactions.models import Transaction @@ -83,8 +84,7 @@ def get_year_by_year_data(group_by="categories"): "account__currency__decimal_places", "account__currency__prefix", "account__currency__suffix", - "account__currency__exchange_currency", - "account__currency__show_exchanged_amounts", + exchange_currency_id=shown_exchange_currency("account__currency__"), ) .annotate( expense_total=Coalesce( @@ -127,12 +127,7 @@ def get_year_by_year_data(group_by="categories"): item_name = metric[name_field] year = metric["reference_date__year"] currency_id = metric["account__currency"] - # Currencies can hide their exchanged amounts - exchange_currency_id = ( - metric["account__currency__exchange_currency"] - if metric["account__currency__show_exchanged_amounts"] - else None - ) + exchange_currency_id = metric["exchange_currency_id"] # Use a consistent key for None (uncategorized/untagged/no entity) item_key = item_id if item_id is not None else "__none__" diff --git a/app/apps/transactions/models.py b/app/apps/transactions/models.py index ff066cdc..e928b062 100644 --- a/app/apps/transactions/models.py +++ b/app/apps/transactions/models.py @@ -573,49 +573,33 @@ class Transaction(OwnedObject): super().delete(*args, **kwargs) def exchanged_amount(self): - if self.account.exchange_currency: - converted_amount, prefix, suffix, decimal_places = convert( - self.amount, - to_currency=self.account.exchange_currency, - from_currency=self.account.currency, - date=self.date, - ) - if converted_amount: - return { - "amount": converted_amount, - "prefix": prefix, - "suffix": suffix, - "decimal_places": decimal_places, - } - elif self.account.currency.exchange_currency: - converted_amount, prefix, suffix, decimal_places = convert( - self.amount, - to_currency=self.account.currency.exchange_currency, - from_currency=self.account.currency, - date=self.date, - ) - if converted_amount: - return { - "amount": converted_amount, - "prefix": prefix, - "suffix": suffix, - "decimal_places": decimal_places, - } - - return None + return self._exchanged_to( + self.account.exchange_currency or self.account.currency.exchange_currency + ) def displayed_exchanged_amount(self): - """The exchanged amount to display, or None when it's hidden. + """The exchanged amount to display, or None when it's hidden.""" + return self._exchanged_to(self.account.displayed_exchange_currency) - The account's exchange currency takes precedence over its currency's, so - whichever one provides it also decides whether it's shown. - """ - if self.account.exchange_currency_id: - show = self.account.show_exchanged_amounts - else: - show = self.account.currency.show_exchanged_amounts + def _exchanged_to(self, exchange_currency): + if not exchange_currency: + return None - return self.exchanged_amount() if show else None + converted_amount, prefix, suffix, decimal_places = convert( + self.amount, + to_currency=exchange_currency, + from_currency=self.account.currency, + date=self.date, + ) + if converted_amount: + return { + "amount": converted_amount, + "prefix": prefix, + "suffix": suffix, + "decimal_places": decimal_places, + } + + return None def __str__(self): type_display = self.get_type_display() diff --git a/app/apps/transactions/utils/calculations.py b/app/apps/transactions/utils/calculations.py index 889c63d5..fd2459ed 100644 --- a/app/apps/transactions/utils/calculations.py +++ b/app/apps/transactions/utils/calculations.py @@ -6,6 +6,7 @@ from django.db.models.functions import Coalesce from apps.transactions.models import Transaction from apps.currencies.utils.convert import convert +from apps.currencies.utils.display import shown_exchange_currency from apps.currencies.models import Currency @@ -404,8 +405,7 @@ def calculate_account_totals(transactions_queryset, ignore_empty=False): "account__currency__decimal_places", "account__currency__prefix", "account__currency__suffix", - "account__exchange_currency", - "account__show_exchanged_amounts", + exchange_currency_id=shown_exchange_currency("account__"), ).annotate( expense_current=Coalesce( Sum( @@ -472,9 +472,8 @@ def calculate_account_totals(transactions_queryset, ignore_empty=False): account_id = total["account"] currency = Currency.objects.get(id=total["account__currency__id"]) exchange_currency = ( - Currency.objects.get(id=total["account__exchange_currency"]) - if total["account__exchange_currency"] - and total["account__show_exchanged_amounts"] + Currency.objects.get(id=total["exchange_currency_id"]) + if total["exchange_currency_id"] else None )