Proper recalculation of balance [skip ci]

app/Console/Commands/Correction/FixUnevenAmount.php

@@ -27,6 +27,7 @@ use FireflyIII\Console\Commands\ShowsFriendlyMessages;
 use FireflyIII\Models\Transaction;
 use FireflyIII\Models\TransactionJournal;
 use FireflyIII\Models\TransactionType;
+use FireflyIII\Support\Models\AccountBalanceCalculator;
 use Illuminate\Console\Command;
 use Illuminate\Support\Facades\Log;
 
@@ -50,6 +51,7 @@ class FixUnevenAmount extends Command
         $this->convertOldStyleTransfers();
         $this->fixUnevenAmounts();
         $this->matchCurrencies();
+        AccountBalanceCalculator::recalculateAll();
 
         return 0;
     }

app/Console/Commands/Upgrade/CorrectAccountBalance.php

@@ -44,9 +44,9 @@ class CorrectAccountBalance extends Command
 
             return 0;
         }
-        $this->correctBalanceAmounts();
+        $this->friendlyWarning('This command has been disabled.');
         $this->markAsExecuted();
-
+//        $this->correctBalanceAmounts();
         return 0;
     }
 

app/Services/Internal/Update/JournalUpdateService.php

@@ -101,7 +101,7 @@ class JournalUpdateService
             'external_url',
         ];
         $this->metaDate               = ['interest_date', 'book_date', 'process_date', 'due_date', 'payment_date',
-            'invoice_date', ];
+                                         'invoice_date',];
     }
 
     public function setData(array $data): void
@@ -111,7 +111,7 @@ class JournalUpdateService
 
     public function setTransactionGroup(TransactionGroup $transactionGroup): void
     {
-        $this->transactionGroup       = $transactionGroup;
+        $this->transactionGroup = $transactionGroup;
         $this->billRepository->setUser($transactionGroup->user);
         $this->categoryRepository->setUser($transactionGroup->user);
         $this->budgetRepository->setUser($transactionGroup->user);
@@ -168,6 +168,7 @@ class JournalUpdateService
         app('preferences')->mark();
 
         $this->transactionJournal->refresh();
+        Log::debug('Done with update journal routine');
     }
 
     private function hasValidAccounts(): bool
@@ -178,8 +179,8 @@ class JournalUpdateService
     private function hasValidSourceAccount(): bool
     {
         app('log')->debug('Now in hasValidSourceAccount().');
-        $sourceId     = $this->data['source_id'] ?? null;
+        $sourceId   = $this->data['source_id'] ?? null;
-        $sourceName   = $this->data['source_name'] ?? null;
+        $sourceName = $this->data['source_name'] ?? null;
 
         if (!$this->hasFields(['source_id', 'source_name'])) {
             $origSourceAccount = $this->getOriginalSourceAccount();
@@ -193,11 +194,11 @@ class JournalUpdateService
 
         // make a new validator.
         /** @var AccountValidator $validator */
-        $validator    = app(AccountValidator::class);
+        $validator = app(AccountValidator::class);
         $validator->setTransactionType($expectedType);
         $validator->setUser($this->transactionJournal->user);
 
-        $result       = $validator->validateSource(['id' => $sourceId]);
+        $result = $validator->validateSource(['id' => $sourceId]);
         app('log')->debug(
             sprintf('hasValidSourceAccount(%d, "%s") will return %s', $sourceId, $sourceName, var_export($result, true))
         );
@@ -263,8 +264,8 @@ class JournalUpdateService
     private function hasValidDestinationAccount(): bool
     {
         app('log')->debug('Now in hasValidDestinationAccount().');
-        $destId            = $this->data['destination_id'] ?? null;
+        $destId   = $this->data['destination_id'] ?? null;
-        $destName          = $this->data['destination_name'] ?? null;
+        $destName = $this->data['destination_name'] ?? null;
 
         if (!$this->hasFields(['destination_id', 'destination_name'])) {
             app('log')->debug('No destination info submitted, grab the original data.');
@@ -274,12 +275,12 @@ class JournalUpdateService
         }
 
         // make new account validator.
-        $expectedType      = $this->getExpectedType();
+        $expectedType = $this->getExpectedType();
         app('log')->debug(sprintf('(b) Expected type (new or unchanged) is %s', $expectedType));
 
         // make a new validator.
         /** @var AccountValidator $validator */
-        $validator         = app(AccountValidator::class);
+        $validator = app(AccountValidator::class);
         $validator->setTransactionType($expectedType);
         $validator->setUser($this->transactionJournal->user);
         $validator->source = $this->getValidSourceAccount();
@@ -332,8 +333,8 @@ class JournalUpdateService
             return $this->getOriginalSourceAccount();
         }
 
-        $sourceInfo   = [
+        $sourceInfo = [
-            'id'     => (int)($this->data['source_id'] ?? null),
+            'id'     => (int) ($this->data['source_id'] ?? null),
             'name'   => $this->data['source_name'] ?? null,
             'iban'   => $this->data['source_iban'] ?? null,
             'number' => $this->data['source_number'] ?? null,
@@ -360,8 +361,8 @@ class JournalUpdateService
      */
     private function updateAccounts(): void
     {
-        $source                = $this->getValidSourceAccount();
+        $source      = $this->getValidSourceAccount();
-        $destination           = $this->getValidDestinationAccount();
+        $destination = $this->getValidDestinationAccount();
 
         // cowardly refuse to update if both accounts are the same.
         if ($source->id === $destination->id) {
@@ -374,7 +375,7 @@ class JournalUpdateService
         $origSourceTransaction->account()->associate($source);
         $origSourceTransaction->save();
 
-        $destTransaction       = $this->getDestinationTransaction();
+        $destTransaction = $this->getDestinationTransaction();
         $destTransaction->account()->associate($destination);
         $destTransaction->save();
 
@@ -396,8 +397,8 @@ class JournalUpdateService
             return $this->getOriginalDestinationAccount();
         }
 
-        $destInfo     = [
+        $destInfo = [
-            'id'     => (int)($this->data['destination_id'] ?? null),
+            'id'     => (int) ($this->data['destination_id'] ?? null),
             'name'   => $this->data['destination_name'] ?? null,
             'iban'   => $this->data['destination_iban'] ?? null,
             'number' => $this->data['destination_number'] ?? null,
@@ -425,7 +426,7 @@ class JournalUpdateService
     {
         app('log')->debug('Now in updateType()');
         if ($this->hasFields(['type'])) {
-            $type        = 'opening-balance' === $this->data['type'] ? 'opening balance' : $this->data['type'];
+            $type = 'opening-balance' === $this->data['type'] ? 'opening balance' : $this->data['type'];
             app('log')->debug(
                 sprintf(
                     'Trying to change journal #%d from a %s to a %s.',
@@ -458,13 +459,13 @@ class JournalUpdateService
     {
         $type = $this->transactionJournal->transactionType->type;
         if ((
-            array_key_exists('bill_id', $this->data)
+                array_key_exists('bill_id', $this->data)
                 || array_key_exists('bill_name', $this->data)
-        )
+            )
             && TransactionType::WITHDRAWAL === $type
         ) {
-            $billId                            = (int)($this->data['bill_id'] ?? 0);
+            $billId                            = (int) ($this->data['bill_id'] ?? 0);
-            $billName                          = (string)($this->data['bill_name'] ?? '');
+            $billName                          = (string) ($this->data['bill_name'] ?? '');
             $bill                              = $this->billRepository->findBill($billId, $billName);
             $this->transactionJournal->bill_id = $bill?->id;
             app('log')->debug('Updated bill ID');
@@ -476,8 +477,8 @@ class JournalUpdateService
      */
     private function updateField(string $fieldName): void
     {
-        if (array_key_exists($fieldName, $this->data) && '' !== (string)$this->data[$fieldName]) {
+        if (array_key_exists($fieldName, $this->data) && '' !== (string) $this->data[$fieldName]) {
-            $value                                  = $this->data[$fieldName];
+            $value = $this->data[$fieldName];
 
             if ('date' === $fieldName) {
                 if ($value instanceof Carbon) {
@@ -548,7 +549,7 @@ class JournalUpdateService
     {
         // update notes.
         if ($this->hasFields(['notes'])) {
-            $notes = '' === (string)$this->data['notes'] ? null : $this->data['notes'];
+            $notes = '' === (string) $this->data['notes'] ? null : $this->data['notes'];
             $this->storeNotes($this->transactionJournal, $notes);
         }
     }
@@ -578,7 +579,7 @@ class JournalUpdateService
             if ($this->hasFields([$field])) {
                 $value = '' === $this->data[$field] ? null : $this->data[$field];
                 app('log')->debug(sprintf('Field "%s" is present ("%s"), try to update it.', $field, $value));
-                $set   = [
+                $set = [
                     'journal' => $this->transactionJournal,
                     'name'    => $field,
                     'data'    => $value,
@@ -596,8 +597,8 @@ class JournalUpdateService
         foreach ($this->metaDate as $field) {
             if ($this->hasFields([$field])) {
                 try {
-                    $value = '' === (string)$this->data[$field] ? null : new Carbon($this->data[$field]);
+                    $value = '' === (string) $this->data[$field] ? null : new Carbon($this->data[$field]);
-                } catch (InvalidDateException|InvalidFormatException $e) { // @phpstan-ignore-line
+                } catch (InvalidDateException | InvalidFormatException $e) { // @phpstan-ignore-line
                     app('log')->debug(sprintf('%s is not a valid date value: %s', $this->data[$field], $e->getMessage()));
 
                     return;
@@ -619,19 +620,19 @@ class JournalUpdateService
         if (!$this->hasFields(['currency_id', 'currency_code'])) {
             return;
         }
-        $currencyId                                        = $this->data['currency_id'] ?? null;
+        $currencyId   = $this->data['currency_id'] ?? null;
-        $currencyCode                                      = $this->data['currency_code'] ?? null;
+        $currencyCode = $this->data['currency_code'] ?? null;
-        $currency                                          = $this->currencyRepository->findCurrency($currencyId, $currencyCode);
+        $currency     = $this->currencyRepository->findCurrency($currencyId, $currencyCode);
         // update currency everywhere.
         $this->transactionJournal->transaction_currency_id = $currency->id;
         $this->transactionJournal->save();
 
-        $source                                            = $this->getSourceTransaction();
+        $source                          = $this->getSourceTransaction();
-        $source->transaction_currency_id                   = $currency->id;
+        $source->transaction_currency_id = $currency->id;
         $source->save();
 
-        $dest                                              = $this->getDestinationTransaction();
+        $dest                          = $this->getDestinationTransaction();
-        $dest->transaction_currency_id                     = $currency->id;
+        $dest->transaction_currency_id = $currency->id;
         $dest->save();
 
         // refresh transactions.
@@ -647,7 +648,7 @@ class JournalUpdateService
             return;
         }
 
-        $value                         = $this->data['amount'] ?? '';
+        $value = $this->data['amount'] ?? '';
         app('log')->debug(sprintf('Amount is now "%s"', $value));
 
         try {
@@ -657,11 +658,14 @@ class JournalUpdateService
 
             return;
         }
-        $origSourceTransaction         = $this->getSourceTransaction();
+
-        $origSourceTransaction->amount = app('steam')->negative($amount);
+        $origSourceTransaction                = $this->getSourceTransaction();
+        $origSourceTransaction->amount        = app('steam')->negative($amount);
+        $origSourceTransaction->balance_dirty = true;
         $origSourceTransaction->save();
-        $destTransaction               = $this->getDestinationTransaction();
+        $destTransaction                = $this->getDestinationTransaction();
-        $destTransaction->amount       = app('steam')->positive($amount);
+        $destTransaction->amount        = app('steam')->positive($amount);
+        $destTransaction->balance_dirty = true;
         $destTransaction->save();
         // refresh transactions.
         $this->sourceTransaction->refresh();
@@ -705,18 +709,17 @@ class JournalUpdateService
             // the correct fields to update in the destination transaction are NOT the foreign amount and currency
             // but rather the normal amount and currency. This is new behavior.
 
-            if(TransactionType::TRANSFER === $this->transactionJournal->transactionType->type) {
+            if (TransactionType::TRANSFER === $this->transactionJournal->transactionType->type) {
                 Log::debug('Switch amounts, store in amount and not foreign_amount');
-                $dest->transaction_currency_id   = $foreignCurrency->id;
+                $dest->transaction_currency_id = $foreignCurrency->id;
-                $dest->amount        = app('steam')->positive($foreignAmount);
+                $dest->amount                  = app('steam')->positive($foreignAmount);
             }
-            if(TransactionType::TRANSFER !== $this->transactionJournal->transactionType->type) {
+            if (TransactionType::TRANSFER !== $this->transactionJournal->transactionType->type) {
-                $dest->foreign_currency_id   = $foreignCurrency->id;
+                $dest->foreign_currency_id = $foreignCurrency->id;
-                $dest->foreign_amount        = app('steam')->positive($foreignAmount);
+                $dest->foreign_amount      = app('steam')->positive($foreignAmount);
             }
 
 
-
             $dest->save();
 
             app('log')->debug(
@@ -739,8 +742,8 @@ class JournalUpdateService
             $source->foreign_amount      = null;
             $source->save();
 
-            $dest->foreign_currency_id   = null;
+            $dest->foreign_currency_id = null;
-            $dest->foreign_amount        = null;
+            $dest->foreign_amount      = null;
             $dest->save();
             app('log')->debug(sprintf('Foreign amount is "%s" so remove foreign amount info.', $amount));
         }
@@ -751,8 +754,5 @@ class JournalUpdateService
         $this->destinationTransaction->refresh();
     }
 
-    private function collectCurrency(): TransactionCurrency
+    private function collectCurrency(): TransactionCurrency {}
-    {
-
-    }
 }

app/Support/Models/AccountBalanceCalculator.php

@@ -31,6 +31,14 @@ use FireflyIII\Models\TransactionJournal;
 use Illuminate\Support\Collection;
 use Illuminate\Support\Facades\Log;
 
+
+/**
+ * Class AccountBalanceCalculator
+ *
+ * This class started as a piece of code to create and calculate "account balance" objects, but they
+ * are at the moment unused. Instead, each transaction gets a before/after balance and an indicator if this
+ * balance is up-to-date. This class now contains some methods to recalculate those amounts.
+ */
 class AccountBalanceCalculator
 {
     private function __construct()
@@ -39,36 +47,25 @@ class AccountBalanceCalculator
     }
 
     /**
-     * Recalculate all balances for a given account.
+     * Recalculate all balances.
-     *
-     * Je moet toch altijd wel alles doen want je weet niet waar een transaction journal invloed op heeft.
-     * Dus dit aantikken per transaction journal is zinloos, beide accounts moeten gedaan worden.
      */
     public static function recalculateAll(): void
     {
         $object = new self();
-        //$object->recalculateLatest(null);
         $object->optimizedCalculation(new Collection());
-        // $object->recalculateJournals(null, null);
     }
 
     public static function recalculateForJournal(TransactionJournal $transactionJournal): void
     {
+        Log::debug(__METHOD__);
         $object = new self();
 
-        // new optimized code, currently UNUSED:
+        // recalculate the involved accounts:
-        // recalculate everything ON or AFTER the moment of this transaction.
+        $accounts = new Collection;
-//        Transaction
+        foreach($transactionJournal->transactions as $transaction) {
-//            ::leftjoin('transaction_journals','transaction_journals.id','=','transactions.transaction_journal_id')
+            $accounts->push($transaction->account);
-//            ->where('transaction_journals.user_id', $transactionJournal->user_id)
-//            ->where('transaction_journals.date', '>=', $transactionJournal->date)
-//            ->update(['transactions.balance_dirty' => true]);
-//        $object->optimizedCalculation(new Collection());
-
-        foreach ($transactionJournal->transactions as $transaction) {
-            $object->recalculateLatest($transaction->account);
-             //$object->recalculateJournals($transaction->account, $transactionJournal);
         }
+        $object->optimizedCalculation($accounts);
     }
 
     private function getAccountBalanceByAccount(int $account, int $currency): AccountBalance