Improvements of update_running_totals management command

Author: PetrDlouhy

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"
+        )

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,30 +388,46 @@ 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]:
                     print(
                         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):

hordak/tests/models/test_core.py

@@ -60,7 +60,7 @@ def test_str_child(self):
     def test_update_running_totals_no_money(self):
         account = self.account()
         account.update_running_totals()
-        self.assertEqual(account.running_totals.get().balance, Money(0, "EUR"))
+        self.assertQuerysetEqual(account.running_totals.all(), [])
 
     def test_str_currency(self):
         account = self.account(currencies=["EUR", "GBP"])

hordak/tests/test_commands.py

@@ -108,7 +108,11 @@ def test_check_mismatch(self):
         running_total.save()
 
         ret_val = call_command("recalculate_running_totals", *["--check"])
-        self.assertEqual(ret_val, "Running totals are incorrect")
+        self.assertEqual(
+            ret_val,
+            "Running totals are INCORRECT: \n\n"
+            "Account Account 1 has faulty running total for EUR (should be € 100.00, is €200.00)\n"
+        )
 
     def test_mail_admins(self):
         """
@@ -131,8 +135,18 @@ def test_mail_admins(self):
         running_total.save()
 
         ret_val = call_command("recalculate_running_totals", *["--mail-admins"])
-        self.assertEqual(ret_val, "Running totals are incorrect")
+        self.assertEqual(
+            ret_val,
+            "Running totals are INCORRECT: \n\n"
+            "Account Account 1 has faulty running total for EUR (should be € 100.00, is €200.00)\n"
+        )
         self.assertEqual(len(mail.outbox), 1)
         self.assertEqual(
-            mail.outbox[0].subject, "[Django] Running totals are incorrect"
+            mail.outbox[0].subject,
+            "[Django] Running totals are incorrect"
+        )
+        self.assertEqual(
+            mail.outbox[0].body,
+            "Running totals are incorrect for some accounts\n\n"
+            "Account Account 1 has faulty running total for EUR (should be € 100.00, is €200.00)\n"
         )