diff --git a/hordak/management/commands/recalculate_running_totals.py b/hordak/management/commands/recalculate_running_totals.py index c7b94834..95e1162a 100644 --- a/hordak/management/commands/recalculate_running_totals.py +++ b/hordak/management/commands/recalculate_running_totals.py @@ -24,8 +24,10 @@ def add_arguments(self, parser): ) def handle(self, *args, **options): - print("Recalculating running totals for all accounts") - all_values_are_correct = True + print( + f"{'Checking' if options['check'] else 'Recalculating'} running totals for all accounts" + ) + output_string = "" # We are using Legs subquery because it is quicker queryset = Account.objects.filter(pk__in=Leg.objects.values("account")) i = 0 @@ -34,14 +36,20 @@ def handle(self, *args, **options): i += 1 if i % 100 == 0: print(f"Processed {i} accounts") - value_correct = account.update_running_totals(check_only=options["check"]) - if not value_correct: - all_values_are_correct = False + faulty_values = account.update_running_totals(check_only=options["check"]) + if faulty_values: + for currency, rt_value, correct_value in faulty_values: + output_string += f"Account {account.name} has faulty running total for {currency}" + output_string += f" (should be {correct_value}, is {rt_value})\n" - if options["mail_admins"] and not all_values_are_correct: + if options["mail_admins"] and output_string: mail_admins( "Running totals are incorrect", - "Running totals are incorrect for some accounts", + f"Running totals are incorrect for some accounts\n\n{output_string}", ) - return 0 if all_values_are_correct else "Running totals are incorrect" + return ( + f"Running totals are INCORRECT: \n\n{output_string}" + if output_string + else "Running totals are correct" + ) diff --git a/hordak/models/core.py b/hordak/models/core.py index 1679d167..7edd02b3 100644 --- a/hordak/models/core.py +++ b/hordak/models/core.py @@ -68,10 +68,12 @@ class RunningTotal(models.Model): This is used to speed up the calculation of account balances. This field should be considered an estimated value calculated for performance reasons. - It is not guaranteed to be accurate. + It is not guaranteed to be accurate: + * Running totals are calculated through post_delete and pre_save signals on the Transaction model. + This means that they are not updated when a transaction is mass updated. + * They don't take into account children accounts - the total value of a parent + account needs to be calculated from its children manually now - Running totals are calculated through post_delete and pre_save signals on the Transaction model. - This means that they are not updated when a transaction is mass updated. """ account = models.ForeignKey( @@ -386,12 +388,22 @@ def transfer_to(self, to_account, amount, **transaction_kwargs): return transaction def update_running_totals(self, check_only=False): - """Update the running totals for this account""" - all_values_are_correct = True + """ + Update the running totals for this account by counting all transactions + Args: + check_only (bool): If true, don't actually update the running totals, + just check that they are correct. + Returns: + list: A list of currencies that have been updated or didn't pass the check + """ total = self.balance() + faulty_values = [] + for money in total.monies(): currency = money.currency.code + if total[currency].amount == 0: + continue try: running_total = self.running_totals.get(currency=currency) if running_total.balance != total[currency]: @@ -399,17 +411,23 @@ def update_running_totals(self, check_only=False): f"Running total for {self} ({currency}) is " f"{running_total.balance} but should be {total[currency]}" ) - all_values_are_correct = False + faulty_values.append( + (currency, running_total.balance, total[currency]) + ) + if not check_only: + self.running_totals.filter(currency=currency).update( + balance=total[currency] + ) except RunningTotal.DoesNotExist: print(f"No running total for {self} ({currency})") - all_values_are_correct = False - if not check_only: - RunningTotal.objects.update_or_create( - account=self, - currency=currency, - defaults={"balance": total[currency]}, - ) - return all_values_are_correct + faulty_values.append((currency, None, total[currency])) + if not check_only: + RunningTotal.objects.create( + account=self, + currency=currency, + balance=total[currency], + ) + return faulty_values class TransactionManager(models.Manager):