Fix issues where rule action would use old data.

app/TransactionRules/Actions/AppendDescription.php

@@ -22,8 +22,8 @@ declare(strict_types=1);
 
 namespace FireflyIII\TransactionRules\Actions;
 
-use FireflyIII\Models\RuleAction;
 use DB;
+use FireflyIII\Models\RuleAction;
 
 /**
  * Class AppendDescription.
@@ -49,6 +49,7 @@ class AppendDescription implements ActionInterface
     {
         $description = sprintf('%s%s', $journal['description'], $this->action->action_value);
         DB::table('transaction_journals')->where('id', $journal['transaction_journal_id'])->limit(1)->update(['description' => $description]);
+
         return true;
     }
 }

app/TransactionRules/Actions/AppendNotes.php

@@ -52,19 +52,20 @@ class AppendNotes implements ActionInterface
     {
         $dbNote = Note
             ::
-            where('noteable_id', (int) $journal['transaction_journal_id'])
+            where('noteable_id', (int)$journal['transaction_journal_id'])
             ->where('noteable_type', TransactionJournal::class)
             ->first(['notes.*']);
         if (null === $dbNote) {
             $dbNote                = new Note;
-            $dbNote->noteable_id   = (int) $journal['transaction_journal_id'];
+            $dbNote->noteable_id   = (int)$journal['transaction_journal_id'];
             $dbNote->noteable_type = TransactionJournal::class;
             $dbNote->text          = '';
         }
         Log::debug(sprintf('RuleAction AppendNotes appended "%s" to "%s".', $this->action->action_value, $dbNote->text));
-        $text = sprintf('%s%s', $dbNote->text, $this->action->action_value);
+        $text         = sprintf('%s%s', $dbNote->text, $this->action->action_value);
         $dbNote->text = $text;
         $dbNote->save();
+
         return true;
     }
 }

app/TransactionRules/Actions/ClearBudget.php

@@ -22,9 +22,10 @@ declare(strict_types=1);
 
 namespace FireflyIII\TransactionRules\Actions;
 
+use DB;
 use FireflyIII\Models\RuleAction;
 use Log;
-use DB;
+
 /**
  * Class ClearBudget.
  */

app/TransactionRules/Actions/ClearCategory.php

@@ -21,6 +21,7 @@
 declare(strict_types=1);
 
 namespace FireflyIII\TransactionRules\Actions;
+
 use DB;
 use FireflyIII\Models\RuleAction;
 use Log;

app/TransactionRules/Actions/ClearNotes.php

@@ -22,10 +22,11 @@ declare(strict_types=1);
 
 namespace FireflyIII\TransactionRules\Actions;
 
+use DB;
 use FireflyIII\Models\RuleAction;
 use FireflyIII\Models\TransactionJournal;
 use Log;
-use DB;
+
 /**
  * Class ClearNotes.
  */
@@ -46,10 +47,11 @@ class ClearNotes implements ActionInterface
     public function actOnArray(array $journal): bool
     {
         DB::table('notes')
-            ->where('noteable_id', $journal['transaction_journal_id'])
+          ->where('noteable_id', $journal['transaction_journal_id'])
-            ->where('noteable_type', TransactionJournal::class)
+          ->where('noteable_type', TransactionJournal::class)
-            ->delete();
+          ->delete();
         Log::debug(sprintf('RuleAction ClearNotes removed all notes.'));
+
         return true;
     }
 }

app/TransactionRules/Actions/ConvertToDeposit.php

@@ -52,46 +52,31 @@ class ConvertToDeposit implements ActionInterface
     }
 
     /**
-     * Input is a transfer from A to B.
+     * @inheritDoc
-     * Output is a deposit from C to B.
-     *
-     * @param array $journal
-     *
-     * @return bool
      * @throws FireflyException
      */
-    private function convertTransferArray(array $journal): bool
+    public function actOnArray(array $journal): bool
     {
-        $user = User::find($journal['user_id']);
+        Log::debug(sprintf('Convert journal #%d to deposit.', $journal['transaction_journal_id']));
-        // find or create revenue account.
+        $type = $journal['transaction_type_type'];
-        /** @var AccountFactory $factory */
+        if (TransactionType::DEPOSIT === $type) {
-        $factory = app(AccountFactory::class);
+            Log::error(sprintf('Journal #%d is already a deposit (rule #%d).', $journal['transaction_journal_id'], $this->action->rule_id));
-        $factory->setUser($user);
 
-        // get the action value, or use the original source name in case the action value is empty:
+            return false;
-        // this becomes a new or existing revenue account.
+        }
-        $revenueName = '' === $this->action->action_value ? $journal['source_account_name'] : $this->action->action_value;
-        $revenue     = $factory->findOrCreate($revenueName, AccountType::REVENUE);
 
-        Log::debug(sprintf('ConvertToDeposit. Action value is "%s", revenue name is "%s"', $this->action->action_value, $journal['source_account_name']));
+        if (TransactionType::WITHDRAWAL === $type) {
-        unset($source);
+            Log::debug('Going to transform a withdrawal to a deposit.');
 
-        // update source transaction(s) to be revenue account
+            return $this->convertWithdrawalArray($journal);
-        DB::table('transactions')
+        }
-          ->where('transaction_journal_id', '=', $journal['transaction_journal_id'])
+        if (TransactionType::TRANSFER === $type) {
-          ->where('amount', '<', 0)
+            Log::debug('Going to transform a transfer to a deposit.');
-          ->update(['account_id' => $revenue->id]);
 
-        // change transaction type of journal:
+            return $this->convertTransferArray($journal);
-        $newType = TransactionType::whereType(TransactionType::DEPOSIT)->first();
+        }
 
-        DB::table('transaction_journals')
+        return false;
-          ->where('id', '=', $journal['transaction_journal_id'])
-          ->update(['transaction_type_id' => $newType->id]);
-
-        Log::debug('Converted transfer to deposit.');
-
-        return true;
     }
 
     /**
@@ -143,29 +128,45 @@ class ConvertToDeposit implements ActionInterface
     }
 
     /**
-     * @inheritDoc
+     * Input is a transfer from A to B.
+     * Output is a deposit from C to B.
+     *
+     * @param array $journal
+     *
+     * @return bool
      * @throws FireflyException
      */
-    public function actOnArray(array $journal): bool
+    private function convertTransferArray(array $journal): bool
     {
-        Log::debug(sprintf('Convert journal #%d to deposit.', $journal['transaction_journal_id']));
+        $user = User::find($journal['user_id']);
-        $type = $journal['transaction_type_type'];
+        // find or create revenue account.
-        if (TransactionType::DEPOSIT === $type) {
+        /** @var AccountFactory $factory */
-            Log::error(sprintf('Journal #%d is already a deposit (rule #%d).', $journal['transaction_journal_id'], $this->action->rule_id));
+        $factory = app(AccountFactory::class);
+        $factory->setUser($user);
 
-            return false;
+        // get the action value, or use the original source name in case the action value is empty:
-        }
+        // this becomes a new or existing revenue account.
+        $revenueName = '' === $this->action->action_value ? $journal['source_account_name'] : $this->action->action_value;
+        $revenue     = $factory->findOrCreate($revenueName, AccountType::REVENUE);
 
-        if (TransactionType::WITHDRAWAL === $type) {
+        Log::debug(sprintf('ConvertToDeposit. Action value is "%s", revenue name is "%s"', $this->action->action_value, $journal['source_account_name']));
-            Log::debug('Going to transform a withdrawal to a deposit.');
+        unset($source);
 
-            return $this->convertWithdrawalArray($journal);
+        // update source transaction(s) to be revenue account
-        }
+        DB::table('transactions')
-        if (TransactionType::TRANSFER === $type) {
+          ->where('transaction_journal_id', '=', $journal['transaction_journal_id'])
-            Log::debug('Going to transform a transfer to a deposit.');
+          ->where('amount', '<', 0)
+          ->update(['account_id' => $revenue->id]);
 
-            return $this->convertTransferArray($journal);
+        // change transaction type of journal:
-        }
+        $newType = TransactionType::whereType(TransactionType::DEPOSIT)->first();
-        return false;
+
+        DB::table('transaction_journals')
+          ->where('id', '=', $journal['transaction_journal_id'])
+          ->update(['transaction_type_id' => $newType->id]);
+
+        Log::debug('Converted transfer to deposit.');
+
+        return true;
     }
 }

app/TransactionRules/Actions/ConvertToTransfer.php

@@ -24,14 +24,15 @@ declare(strict_types=1);
 namespace FireflyIII\TransactionRules\Actions;
 
 
+use DB;
 use FireflyIII\Models\Account;
 use FireflyIII\Models\AccountType;
 use FireflyIII\Models\RuleAction;
+use FireflyIII\Models\TransactionJournal;
 use FireflyIII\Models\TransactionType;
 use FireflyIII\Repositories\Account\AccountRepositoryInterface;
 use FireflyIII\User;
 use Log;
-use DB;
 
 /**
  *
@@ -56,10 +57,12 @@ class ConvertToTransfer implements ActionInterface
      */
     public function actOnArray(array $journal): bool
     {
-        $type = $journal['transaction_type_type'];
+        $type    = $journal['transaction_type_type'];
-        $user = User::find($journal['user_id']);
+        $user    = User::find($journal['user_id']);
         if (TransactionType::TRANSFER === $type) {
-            Log::error(sprintf('Journal #%d is already a transfer so cannot be converted (rule #%d).', $journal['transaction_journal_id'], $this->action->rule_id));
+            Log::error(
+                sprintf('Journal #%d is already a transfer so cannot be converted (rule #%d).', $journal['transaction_journal_id'], $this->action->rule_id)
+            );
 
             return false;
         }
@@ -70,7 +73,12 @@ class ConvertToTransfer implements ActionInterface
         $repository->setUser($user);
         $asset = $repository->findByName($this->action->action_value, [AccountType::ASSET, AccountType::LOAN, AccountType::DEBT, AccountType::MORTGAGE]);
         if (null === $asset) {
-            Log::error(sprintf('Journal #%d cannot be converted because no asset with name "%s" exists (rule #%d).', $journal['transaction_journal_id'], $this->action->action_value, $this->action->rule_id));
+            Log::error(
+                sprintf(
+                    'Journal #%d cannot be converted because no asset with name "%s" exists (rule #%d).', $journal['transaction_journal_id'],
+                    $this->action->action_value, $this->action->rule_id
+                )
+            );
 
             return false;
         }
@@ -88,49 +96,25 @@ class ConvertToTransfer implements ActionInterface
         return false; // @codeCoverageIgnore
     }
 
-    /**
-     * A deposit is from Revenue to Asset.
-     * We replace the Revenue with another asset.
-     * @param array   $journal
-     * @param Account $asset
-     * @return bool
-     */
-    private function convertDepositArray(array $journal, Account $asset): bool
-    {
-        if ($journal['destination_account_id'] === $asset->id) {
-            Log::error(vsprintf('Journal #%d has already has "%s" as a destination asset. ConvertToTransfer failed. (rule #%d).', [$journal['transaction_journal_id'], $asset->name, $this->action->rule_id]));
-            return false;
-        }
-
-        // update source transaction:
-        DB::table('transactions')
-          ->where('transaction_journal_id', '=', $journal['transaction_journal_id'])
-          ->where('amount', '<', 0)
-          ->update(['account_id' => $asset->id]);
-
-        // change transaction type of journal:
-        $newType                      = TransactionType::whereType(TransactionType::TRANSFER)->first();
-
-        DB::table('transaction_journals')
-          ->where('id', '=', $journal['transaction_journal_id'])
-          ->update(['transaction_type_id' => $newType->id]);
-
-        Log::debug('Converted deposit to transfer.');
-
-        return true;
-    }
-
     /**
      * A withdrawal is from Asset to Expense.
      * We replace the Expense with another asset.
+     *
      * @param array   $journal
      * @param Account $asset
+     *
      * @return bool
      */
     private function convertWithdrawalArray(array $journal, Account $asset): bool
     {
         if ($journal['source_account_id'] === $asset->id) {
-            Log::error(vsprintf('Journal #%d has already has "%s" as a source asset. ConvertToTransfer failed. (rule #%d).', [$journal['transaction_journal_id'], $asset->name, $this->action->rule_id]));
+            Log::error(
+                vsprintf(
+                    'Journal #%d has already has "%s" as a source asset. ConvertToTransfer failed. (rule #%d).',
+                    [$journal['transaction_journal_id'], $asset->name, $this->action->rule_id]
+                )
+            );
+
             return false;
         }
 
@@ -141,7 +125,7 @@ class ConvertToTransfer implements ActionInterface
           ->update(['account_id' => $asset->id]);
 
         // change transaction type of journal:
-        $newType                      = TransactionType::whereType(TransactionType::TRANSFER)->first();
+        $newType = TransactionType::whereType(TransactionType::TRANSFER)->first();
 
         DB::table('transaction_journals')
           ->where('id', '=', $journal['transaction_journal_id'])
@@ -152,4 +136,44 @@ class ConvertToTransfer implements ActionInterface
         return true;
     }
 
+    /**
+     * A deposit is from Revenue to Asset.
+     * We replace the Revenue with another asset.
+     *
+     * @param array   $journal
+     * @param Account $asset
+     *
+     * @return bool
+     */
+    private function convertDepositArray(array $journal, Account $asset): bool
+    {
+        if ($journal['destination_account_id'] === $asset->id) {
+            Log::error(
+                vsprintf(
+                    'Journal #%d has already has "%s" as a destination asset. ConvertToTransfer failed. (rule #%d).',
+                    [$journal['transaction_journal_id'], $asset->name, $this->action->rule_id]
+                )
+            );
+
+            return false;
+        }
+
+        // update source transaction:
+        DB::table('transactions')
+          ->where('transaction_journal_id', '=', $journal['transaction_journal_id'])
+          ->where('amount', '<', 0)
+          ->update(['account_id' => $asset->id]);
+
+        // change transaction type of journal:
+        $newType = TransactionType::whereType(TransactionType::TRANSFER)->first();
+
+        DB::table('transaction_journals')
+          ->where('id', '=', $journal['transaction_journal_id'])
+          ->update(['transaction_type_id' => $newType->id]);
+
+        Log::debug('Converted deposit to transfer.');
+
+        return true;
+    }
+
 }

app/TransactionRules/Actions/ConvertToWithdrawal.php

@@ -24,6 +24,7 @@ declare(strict_types=1);
 namespace FireflyIII\TransactionRules\Actions;
 
 
+use DB;
 use FireflyIII\Exceptions\FireflyException;
 use FireflyIII\Factory\AccountFactory;
 use FireflyIII\Models\AccountType;
@@ -31,7 +32,6 @@ use FireflyIII\Models\RuleAction;
 use FireflyIII\Models\TransactionType;
 use FireflyIII\User;
 use Log;
-use DB;
 
 /**
  *
@@ -51,11 +51,76 @@ class ConvertToWithdrawal implements ActionInterface
         $this->action = $action;
     }
 
+    /**
+     * @inheritDoc
+     */
+    public function actOnArray(array $journal): bool
+    {
+        $type = $journal['transaction_type_type'];
+        if (TransactionType::WITHDRAWAL === $type) {
+            Log::error(sprintf('Journal #%d is already a withdrawal (rule #%d).', $journal['transaction_journal_id'], $this->action->rule_id));
+
+            return false;
+        }
+
+        if (TransactionType::DEPOSIT === $type) {
+            Log::debug('Going to transform a deposit to a withdrawal.');
+
+            return $this->convertDepositArray($journal);
+        }
+        if (TransactionType::TRANSFER === $type) {
+            Log::debug('Going to transform a transfer to a withdrawal.');
+
+            return $this->convertTransferArray($journal);
+        }
+
+        return false; // @codeCoverageIgnore
+    }
+
+    private function convertDepositArray(array $journal): bool
+    {
+        $user = User::find($journal['user_id']);
+        /** @var AccountFactory $factory */
+        $factory = app(AccountFactory::class);
+        $factory->setUser($user);
+
+        $expenseName   = '' === $this->action->action_value ? $journal['source_account_name'] : $this->action->action_value;
+        $expense       = $factory->findOrCreate($expenseName, AccountType::EXPENSE);
+        $destinationId = $journal['destination_account_id'];
+
+
+        Log::debug(sprintf('ConvertToWithdrawal. Action value is "%s", expense name is "%s"', $this->action->action_value, $expenseName));
+
+        // update source transaction(s) to be the original destination account
+        DB::table('transactions')
+          ->where('transaction_journal_id', '=', $journal['transaction_journal_id'])
+          ->where('amount', '<', 0)
+          ->update(['account_id' => $destinationId]);
+
+        // update destination transaction(s) to be new expense account.
+        DB::table('transactions')
+          ->where('transaction_journal_id', '=', $journal['transaction_journal_id'])
+          ->where('amount', '>', 0)
+          ->update(['account_id' => $expense->id]);
+
+        // change transaction type of journal:
+        $newType = TransactionType::whereType(TransactionType::WITHDRAWAL)->first();
+        DB::table('transaction_journals')
+          ->where('id', '=', $journal['transaction_journal_id'])
+          ->update(['transaction_type_id' => $newType->id]);
+
+        Log::debug('Converted deposit to withdrawal.');
+
+        return true;
+
+    }
+
     /**
      * Input is a transfer from A to B.
      * Output is a withdrawal from A to C.
      *
      * @param array $journal
+     *
      * @return bool
      * @throws FireflyException
      */
@@ -82,7 +147,7 @@ class ConvertToWithdrawal implements ActionInterface
           ->update(['account_id' => $expense->id]);
 
         // change transaction type of journal:
-        $newType                      = TransactionType::whereType(TransactionType::WITHDRAWAL)->first();
+        $newType = TransactionType::whereType(TransactionType::WITHDRAWAL)->first();
         DB::table('transaction_journals')
           ->where('id', '=', $journal['transaction_journal_id'])
           ->update(['transaction_type_id' => $newType->id]);
@@ -91,67 +156,4 @@ class ConvertToWithdrawal implements ActionInterface
 
         return true;
     }
-
-    /**
-     * @inheritDoc
-     */
-    public function actOnArray(array $journal): bool
-    {
-        $type = $journal['transaction_type_type'];
-        if (TransactionType::WITHDRAWAL === $type) {
-            Log::error(sprintf('Journal #%d is already a withdrawal (rule #%d).', $journal['transaction_journal_id'], $this->action->rule_id));
-            return false;
-        }
-
-        if (TransactionType::DEPOSIT === $type) {
-            Log::debug('Going to transform a deposit to a withdrawal.');
-
-            return $this->convertDepositArray($journal);
-        }
-        if (TransactionType::TRANSFER === $type) {
-            Log::debug('Going to transform a transfer to a withdrawal.');
-
-            return $this->convertTransferArray($journal);
-        }
-
-        return false; // @codeCoverageIgnore
-    }
-
-    private function convertDepositArray(array $journal): bool
-    {
-        $user = User::find($journal['user_id']);
-        /** @var AccountFactory $factory */
-        $factory = app(AccountFactory::class);
-        $factory->setUser($user);
-
-        $expenseName = '' === $this->action->action_value ? $journal['source_account_name'] : $this->action->action_value;
-        $expense     = $factory->findOrCreate($expenseName, AccountType::EXPENSE);
-        $destinationId = $journal['destination_account_id'];
-
-
-        Log::debug(sprintf('ConvertToWithdrawal. Action value is "%s", expense name is "%s"', $this->action->action_value, $expenseName));
-
-        // update source transaction(s) to be the original destination account
-        DB::table('transactions')
-          ->where('transaction_journal_id', '=', $journal['transaction_journal_id'])
-          ->where('amount', '<', 0)
-          ->update(['account_id' => $destinationId]);
-
-        // update destination transaction(s) to be new expense account.
-        DB::table('transactions')
-          ->where('transaction_journal_id', '=', $journal['transaction_journal_id'])
-          ->where('amount', '>', 0)
-          ->update(['account_id' => $expense->id]);
-
-        // change transaction type of journal:
-        $newType                      = TransactionType::whereType(TransactionType::WITHDRAWAL)->first();
-        DB::table('transaction_journals')
-          ->where('id', '=', $journal['transaction_journal_id'])
-          ->update(['transaction_type_id' => $newType->id]);
-
-        Log::debug('Converted deposit to withdrawal.');
-
-        return true;
-
-    }
 }

app/TransactionRules/Actions/DeleteTransaction.php

@@ -64,7 +64,9 @@ class DeleteTransaction implements ActionInterface
 
             return true;
         }
-        Log::debug(sprintf('RuleAction DeleteTransaction DELETED transaction journal #%d ("%s").', $journal['transaction_journal_id'], $journal['description']));
+        Log::debug(
+            sprintf('RuleAction DeleteTransaction DELETED transaction journal #%d ("%s").', $journal['transaction_journal_id'], $journal['description'])
+        );
 
         // trigger delete factory:
         $journal = TransactionJournal::find($journal['transaction_group_id']);

app/TransactionRules/Actions/LinkToBill.php

@@ -58,19 +58,26 @@ class LinkToBill implements ActionInterface
         /** @var BillRepositoryInterface $repository */
         $repository = app(BillRepositoryInterface::class);
         $repository->setUser($user);
-        $billName = (string) $this->action->action_value;
+        $billName = (string)$this->action->action_value;
         $bill     = $repository->findByName($billName);
 
         if (null !== $bill && $journal['transaction_type_type'] === TransactionType::WITHDRAWAL) {
             DB::table('transaction_journals')
               ->where('id', '=', $journal['transaction_journal_id'])
               ->update(['bill_id' => $bill->id]);
-            Log::debug(sprintf('RuleAction LinkToBill set the bill of journal #%d to bill #%d ("%s").', $journal['transaction_journal_id'], $bill->id, $bill->name));
+            Log::debug(
+                sprintf('RuleAction LinkToBill set the bill of journal #%d to bill #%d ("%s").', $journal['transaction_journal_id'], $bill->id, $bill->name)
+            );
 
             return true;
         }
 
-        Log::error(sprintf('RuleAction LinkToBill could not set the bill of journal #%d to bill "%s": no such bill found or not a withdrawal.', $journal['transaction_journal_id'], $billName));
+        Log::error(
+            sprintf(
+                'RuleAction LinkToBill could not set the bill of journal #%d to bill "%s": no such bill found or not a withdrawal.',
+                $journal['transaction_journal_id'], $billName
+            )
+        );
 
 
         return false;

app/TransactionRules/Actions/PrependDescription.php

@@ -22,8 +22,8 @@ declare(strict_types=1);
 
 namespace FireflyIII\TransactionRules\Actions;
 
-use FireflyIII\Models\RuleAction;
 use DB;
+use FireflyIII\Models\RuleAction;
 
 /**
  * Class PrependDescription.
@@ -50,6 +50,7 @@ class PrependDescription implements ActionInterface
     {
         $description = sprintf('%s%s', $this->action->action_value, $journal['description']);
         DB::table('transaction_journals')->where('id', $journal['transaction_journal_id'])->limit(1)->update(['description' => $description]);
+
         return true;
     }
 }

app/TransactionRules/Actions/PrependNotes.php

@@ -52,19 +52,20 @@ class PrependNotes implements ActionInterface
     {
         $dbNote = Note
             ::
-            where('noteable_id', (int) $journal['transaction_journal_id'])
+            where('noteable_id', (int)$journal['transaction_journal_id'])
             ->where('noteable_type', TransactionJournal::class)
             ->first(['notes.*']);
         if (null === $dbNote) {
             $dbNote                = new Note;
-            $dbNote->noteable_id   = (int) $journal['transaction_journal_id'];
+            $dbNote->noteable_id   = (int)$journal['transaction_journal_id'];
             $dbNote->noteable_type = TransactionJournal::class;
             $dbNote->text          = '';
         }
         Log::debug(sprintf('RuleAction PrependNotes prepended "%s" to "%s".', $this->action->action_value, $dbNote->text));
-        $text = sprintf('%s%s', $this->action->action_value, $dbNote->text);
+        $text         = sprintf('%s%s', $this->action->action_value, $dbNote->text);
         $dbNote->text = $text;
         $dbNote->save();
+
         return true;
     }
 }

app/TransactionRules/Actions/RemoveAllTags.php

@@ -22,9 +22,9 @@ declare(strict_types=1);
 
 namespace FireflyIII\TransactionRules\Actions;
 
+use DB;
 use FireflyIII\Models\RuleAction;
 use Log;
-use DB;
 
 
 /**

app/TransactionRules/Actions/RemoveTag.php

@@ -22,10 +22,10 @@ declare(strict_types=1);
 
 namespace FireflyIII\TransactionRules\Actions;
 
+use DB;
 use FireflyIII\Models\RuleAction;
 use FireflyIII\User;
 use Log;
-use DB;
 
 /**
  * Class RemoveTag.

app/TransactionRules/Actions/SetBudget.php

@@ -22,11 +22,11 @@ declare(strict_types=1);
 
 namespace FireflyIII\TransactionRules\Actions;
 
+use DB;
 use FireflyIII\Models\RuleAction;
 use FireflyIII\Models\TransactionType;
 use FireflyIII\User;
 use Log;
-use DB;
 
 /**
  * Class SetBudget.
@@ -55,7 +55,12 @@ class SetBudget implements ActionInterface
 
         $budget = $user->budgets()->where('name', $search)->first();
         if (null === $budget) {
-            Log::debug(sprintf('RuleAction SetBudget could not set budget of journal #%d to "%s" because no such budget exists.', $journal['transaction_journal_id'], $search));
+            Log::debug(
+                sprintf(
+                    'RuleAction SetBudget could not set budget of journal #%d to "%s" because no such budget exists.', $journal['transaction_journal_id'],
+                    $search
+                )
+            );
 
             return false;
         }
@@ -73,7 +78,9 @@ class SetBudget implements ActionInterface
             return true;
         }
 
-        Log::debug(sprintf('RuleAction SetBudget set the budget of journal #%d to budget #%d ("%s").', $journal['transaction_journal_id'], $budget->id, $budget->name));
+        Log::debug(
+            sprintf('RuleAction SetBudget set the budget of journal #%d to budget #%d ("%s").', $journal['transaction_journal_id'], $budget->id, $budget->name)
+        );
 
         DB::table('budget_transaction_journal')->where('transaction_journal_id', '=', $journal['transaction_journal_id'])->delete();
         DB::table('budget_transaction_journal')->insert(['transaction_journal_id' => $journal['transaction_journal_id'], 'budget_id' => $budget->id]);

app/TransactionRules/Actions/SetCategory.php

@@ -22,11 +22,11 @@ declare(strict_types=1);
 
 namespace FireflyIII\TransactionRules\Actions;
 
+use DB;
 use FireflyIII\Factory\CategoryFactory;
 use FireflyIII\Models\RuleAction;
 use FireflyIII\User;
 use Log;
-use DB;
 
 /**
  * Class SetCategory.
@@ -52,8 +52,9 @@ class SetCategory implements ActionInterface
     {
         $user = User::find($journal['user_id']);
         $search = $this->action->action_value;
-        if(null === $user) {
+        if (null === $user) {
             Log::error(sprintf('Journal has no valid user ID so action SetCategory("%s") cannot be applied', $search), $journal);
+
             return false;
         }
 
@@ -62,12 +63,22 @@ class SetCategory implements ActionInterface
         $factory->setUser($user);
         $category = $factory->findOrCreate(null, $search);
         if (null === $category) {
-            Log::debug(sprintf('RuleAction SetCategory could not set category of journal #%d to "%s" because no such category exists.', $journal['transaction_journal_id'], $search));
+            Log::debug(
+                sprintf(
+                    'RuleAction SetCategory could not set category of journal #%d to "%s" because no such category exists.', $journal['transaction_journal_id'],
+                    $search
+                )
+            );
 
             return false;
         }
 
-        Log::debug(sprintf('RuleAction SetCategory set the category of journal #%d to category #%d ("%s").', $journal['transaction_journal_id'], $category->id, $category->name));
+        Log::debug(
+            sprintf(
+                'RuleAction SetCategory set the category of journal #%d to category #%d ("%s").', $journal['transaction_journal_id'], $category->id,
+                $category->name
+            )
+        );
 
         DB::table('category_transaction_journal')->where('transaction_journal_id', '=', $journal['transaction_journal_id'])->delete();
         DB::table('category_transaction_journal')->insert(['transaction_journal_id' => $journal['transaction_journal_id'], 'category_id' => $category->id]);

app/TransactionRules/Actions/SetDescription.php

@@ -60,6 +60,7 @@ class SetDescription implements ActionInterface
                 $this->action->action_value
             )
         );
+
         return true;
     }
 }

app/TransactionRules/Actions/SetDestinationAccount.php

@@ -48,27 +48,6 @@ class SetDestinationAccount implements ActionInterface
         $this->action = $action;
     }
 
-    /**
-     * @return Account|null
-     */
-    private function findExpenseAccount(): ?Account
-    {
-        $account = $this->repository->findByName($this->action->action_value, [AccountType::EXPENSE]);
-        if (null === $account) {
-            $data    = [
-                'name'            => $this->action->action_value,
-                'account_type'    => 'expense',
-                'account_type_id' => null,
-                'virtual_balance' => 0,
-                'active'          => true,
-                'iban'            => null,
-            ];
-            $account = $this->repository->store($data);
-        }
-        Log::debug(sprintf('Found or created expense account #%d ("%s")', $account->id, $account->name));
-        return $account;
-    }
-
     /**
      * @inheritDoc
      */
@@ -82,7 +61,11 @@ class SetDestinationAccount implements ActionInterface
         // it depends on the type what kind of destination account is expected.
         $expectedTypes = config(sprintf('firefly.source_dests.%s.%s', $type, $journal['source_account_type']));
         if (null === $expectedTypes) {
-            Log::error(sprintf('Configuration line "%s" is unexpectedly empty. Stopped.', sprintf('firefly.source_dests.%s.%s', $type, $journal['source_account_type'])));
+            Log::error(
+                sprintf(
+                    'Configuration line "%s" is unexpectedly empty. Stopped.', sprintf('firefly.source_dests.%s.%s', $type, $journal['source_account_type'])
+                )
+            );
 
             return false;
         }
@@ -106,6 +89,7 @@ class SetDestinationAccount implements ActionInterface
             $expense = $this->findExpenseAccount();
             if (null === $expense) {
                 Log::error('Could not create expense account.');
+
                 return false;
             }
             DB::table('transactions')
@@ -122,10 +106,33 @@ class SetDestinationAccount implements ActionInterface
 
     /**
      * @param array $types
+     *
      * @return Account|null
      */
     private function findAccount(array $types): ?Account
     {
         return $this->repository->findByName($this->action->action_value, $types);
     }
+
+    /**
+     * @return Account|null
+     */
+    private function findExpenseAccount(): ?Account
+    {
+        $account = $this->repository->findByName($this->action->action_value, [AccountType::EXPENSE]);
+        if (null === $account) {
+            $data    = [
+                'name'            => $this->action->action_value,
+                'account_type'    => 'expense',
+                'account_type_id' => null,
+                'virtual_balance' => 0,
+                'active'          => true,
+                'iban'            => null,
+            ];
+            $account = $this->repository->store($data);
+        }
+        Log::debug(sprintf('Found or created expense account #%d ("%s")', $account->id, $account->name));
+
+        return $account;
+    }
 }

app/TransactionRules/Actions/SetSourceAccount.php

@@ -25,6 +25,8 @@ namespace FireflyIII\TransactionRules\Actions;
 use DB;
 use FireflyIII\Models\Account;
 use FireflyIII\Models\RuleAction;
+use FireflyIII\Models\Transaction;
+use FireflyIII\Models\TransactionJournal;
 use FireflyIII\Models\TransactionType;
 use FireflyIII\Repositories\Account\AccountRepositoryInterface;
 use FireflyIII\User;
@@ -48,6 +50,74 @@ class SetSourceAccount implements ActionInterface
         $this->action = $action;
     }
 
+    /**
+     * @inheritDoc
+     */
+    public function actOnArray(array $journal): bool
+    {
+        $user = User::find($journal['user_id']);
+        $type = $journal['transaction_type_type'];
+        /** @var TransactionJournal $journal */
+        $journal = TransactionJournal::find((int)$journal['transaction_journal_id']);
+        /** @var AccountRepositoryInterface repository */
+        $this->repository = app(AccountRepositoryInterface::class);
+        $this->repository->setUser($user);
+
+        // if this is a transfer or a withdrawal, the new source account must be an asset account or a default account, and it MUST exist:
+        $newAccount = $this->findAssetAccount($type);
+        if ((TransactionType::WITHDRAWAL === $type || TransactionType::TRANSFER === $type) && null === $newAccount) {
+            Log::error(
+                sprintf(
+                    'Cannot change source account of journal #%d because no asset account with name "%s" exists.', $journal['transaction_journal_id'],
+                    $this->action->action_value
+                )
+            );
+
+            return false;
+        }
+        // new source account must be different from the current destination:
+        $destinationId = (int)$journal['destination_account_id'];
+        /** @var Transaction $source */
+        $source = $journal->transactions()->where('amount', '>', 0)->first();
+        if (null !== $source) {
+            $destinationId = $source->account ? (int)$source->account->id : $destinationId;
+        }
+        if (TransactionType::TRANSFER === $type && null !== $newAccount && (int)$newAccount->id === $destinationId) {
+            Log::error(
+                sprintf(
+                    'New source account ID #%d and current destination account ID #%d are the same. Do nothing.', $newAccount->id,
+                    $destinationId
+                )
+            );
+
+            return false;
+        }
+
+
+        // if this is a deposit, the new source account must be a revenue account and may be created:
+        if (TransactionType::DEPOSIT === $type) {
+            $newAccount = $this->findRevenueAccount();
+        }
+        if (null === $newAccount) {
+            Log::error('New account is NULL');
+
+            return false;
+        }
+
+        Log::debug(sprintf('New source account is #%d ("%s").', $newAccount->id, $newAccount->name));
+
+        // update source transaction with new source account:
+        // get source transaction:
+        DB::table('transactions')
+          ->where('transaction_journal_id', '=', $journal['transaction_journal_id'])
+          ->where('amount', '<', 0)
+          ->update(['account_id' => $newAccount->id]);
+
+        Log::debug(sprintf('Updated journal #%d and gave it new source account ID.', $journal['transaction_journal_id']));
+
+        return true;
+    }
+
     /**
      * @param string $type
      *
@@ -83,47 +153,7 @@ class SetSourceAccount implements ActionInterface
             $account = $this->repository->store($data);
         }
         Log::debug(sprintf('Found or created revenue account #%d ("%s")', $account->id, $account->name));
+
         return $account;
     }
-
-    /**
-     * @inheritDoc
-     */
-    public function actOnArray(array $journal): bool
-    {
-        $user             = User::find($journal['user_id']);
-        $type             = $journal['transaction_type_type'];
-        $this->repository = app(AccountRepositoryInterface::class);
-        $this->repository->setUser($user);
-
-        // if this is a transfer or a withdrawal, the new source account must be an asset account or a default account, and it MUST exist:
-        $newAccount = $this->findAssetAccount($type);
-        if ((TransactionType::WITHDRAWAL === $type || TransactionType::TRANSFER === $type) && null === $newAccount) {
-            Log::error(sprintf('Cannot change source account of journal #%d because no asset account with name "%s" exists.', $journal['transaction_journal_id'], $this->action->action_value));
-
-            return false;
-        }
-
-        // if this is a deposit, the new source account must be a revenue account and may be created:
-        if (TransactionType::DEPOSIT === $type) {
-            $newAccount = $this->findRevenueAccount();
-        }
-        if (null === $newAccount) {
-            Log::error('New account is NULL');
-            return false;
-        }
-
-        Log::debug(sprintf('New source account is #%d ("%s").', $newAccount->id, $newAccount->name));
-
-        // update source transaction with new source account:
-        // get source transaction:
-        DB::table('transactions')
-          ->where('transaction_journal_id', '=', $journal['transaction_journal_id'])
-          ->where('amount', '<', 0)
-          ->update(['account_id' => $newAccount->id]);
-
-        Log::debug(sprintf('Updated journal #%d and gave it new source account ID.', $journal['transaction_journal_id']));
-
-        return true;
-    }
 }

app/TransactionRules/Actions/UpdatePiggybank.php

@@ -54,42 +54,49 @@ class UpdatePiggybank implements ActionInterface
     }
 
     /**
-     * @param array     $journalArray
+     * @inheritDoc
-     * @param PiggyBank $piggyBank
-     * @param string    $amount
      */
-    private function addAmount(array $journalArray, PiggyBank $piggyBank, string $amount): void
+    public function actOnArray(array $journal): bool
     {
-        $user = User::find($journalArray['user_id']);
+        Log::debug(sprintf('Triggered rule action UpdatePiggybank on journal #%d', $journal['transaction_journal_id']));
-        $journal = $user->transactionJournals()->find($journalArray['transaction_journal_id']);
+        if (TransactionType::TRANSFER !== $journal['transaction_type_type']) {
-        $repository = app(PiggyBankRepositoryInterface::class);
+            Log::info(sprintf('Journal #%d is a "%s" so skip this action.', $journal['transaction_journal_id'], $journal['transaction_type_type']));
-        $repository->setUser($journal->user);
 
-        // how much can we add to the piggy bank?
+            return false;
-        $toAdd = bcsub($piggyBank->targetamount, $repository->getCurrentAmount($piggyBank));
+        }
-        Log::debug(sprintf('Max amount to add to piggy bank is %s, amount is %s', $toAdd, $amount));
+        $user = User::find($journal['user_id']);
 
-        // update amount to fit:
+        $piggyBank = $this->findPiggybank($user);
-        $amount = -1 === bccomp($amount, $toAdd) ? $amount : $toAdd;
+        if (null === $piggyBank) {
-        Log::debug(sprintf('Amount is now %s', $amount));
+            Log::info(
+                sprintf('No piggy bank names "%s", cant execute action #%d of rule #%d', $this->action->action_value, $this->action->id, $this->action->rule_id)
+            );
 
-        // if amount is zero, stop.
+            return false;
-        if (0 === bccomp('0', $amount)) {
-            Log::warning('Amount left is zero, stop.');
-
-            return;
         }
 
-        // make sure we can add amount:
+        Log::debug(sprintf('Found piggy bank #%d ("%s")', $piggyBank->id, $piggyBank->name));
-        if (false === $repository->canAddAmount($piggyBank, $amount)) {
-            Log::warning(sprintf('Cannot add %s to piggy bank.', $amount));
 
-            return;
+        /** @var Transaction $source */
+        $source = Transaction::where('transaction_journal_id', $journal['transaction_journal_id'])->where('amount', '<', 0)->first();
+        /** @var Transaction $destination */
+        $destination = Transaction::where('transaction_journal_id', $journal['transaction_journal_id'])->where('amount', '>', 0)->first();
+
+        if ((int)$source->account_id === (int)$piggyBank->account_id) {
+            Log::debug('Piggy bank account is linked to source, so remove amount.');
+            $this->removeAmount($journal, $piggyBank, $destination->amount);
+
+            return true;
         }
-        Log::debug(sprintf('Will now add %s to piggy bank.', $amount));
+        if ((int)$destination->account_id === (int)$piggyBank->account_id) {
+            Log::debug('Piggy bank account is linked to source, so add amount.');
+            $this->addAmount($journal, $piggyBank, $destination->amount);
 
-        $repository->addAmount($piggyBank, $amount);
+            return true;
-        $repository->createEventWithJournal($piggyBank, app('steam')->positive($amount), $journal);
+        }
+        Log::info('Piggy bank is not linked to source or destination, so no action will be taken.');
+
+        return true;
     }
 
     /**
@@ -109,8 +116,8 @@ class UpdatePiggybank implements ActionInterface
      */
     private function removeAmount(array $journalArray, PiggyBank $piggyBank, string $amount): void
     {
-        $user = User::find($journalArray['user_id']);
+        $user       = User::find($journalArray['user_id']);
-        $journal = $user->transactionJournals()->find($journalArray['transaction_journal_id']);
+        $journal    = $user->transactionJournals()->find($journalArray['transaction_journal_id']);
         $repository = app(PiggyBankRepositoryInterface::class);
         $repository->setUser($journal->user);
 
@@ -141,46 +148,41 @@ class UpdatePiggybank implements ActionInterface
     }
 
     /**
-     * @inheritDoc
+     * @param array     $journalArray
+     * @param PiggyBank $piggyBank
+     * @param string    $amount
      */
-    public function actOnArray(array $journal): bool
+    private function addAmount(array $journalArray, PiggyBank $piggyBank, string $amount): void
     {
-        Log::debug(sprintf('Triggered rule action UpdatePiggybank on journal #%d', $journal['transaction_journal_id']));
+        $user       = User::find($journalArray['user_id']);
-        if (TransactionType::TRANSFER !== $journal['transaction_type_type']) {
+        $journal    = $user->transactionJournals()->find($journalArray['transaction_journal_id']);
-            Log::info(sprintf('Journal #%d is a "%s" so skip this action.', $journal['transaction_journal_id'], $journal['transaction_type_type']));
+        $repository = app(PiggyBankRepositoryInterface::class);
+        $repository->setUser($journal->user);
 
-            return false;
+        // how much can we add to the piggy bank?
-        }
+        $toAdd = bcsub($piggyBank->targetamount, $repository->getCurrentAmount($piggyBank));
-        $user = User::find($journal['user_id']);
+        Log::debug(sprintf('Max amount to add to piggy bank is %s, amount is %s', $toAdd, $amount));
 
-        $piggyBank = $this->findPiggybank($user);
+        // update amount to fit:
-        if (null === $piggyBank) {
+        $amount = -1 === bccomp($amount, $toAdd) ? $amount : $toAdd;
-            Log::info(sprintf('No piggy bank names "%s", cant execute action #%d of rule #%d', $this->action->action_value, $this->action->id, $this->action->rule_id));
+        Log::debug(sprintf('Amount is now %s', $amount));
 
-            return false;
+        // if amount is zero, stop.
+        if (0 === bccomp('0', $amount)) {
+            Log::warning('Amount left is zero, stop.');
+
+            return;
         }
 
-        Log::debug(sprintf('Found piggy bank #%d ("%s")', $piggyBank->id, $piggyBank->name));
+        // make sure we can add amount:
+        if (false === $repository->canAddAmount($piggyBank, $amount)) {
+            Log::warning(sprintf('Cannot add %s to piggy bank.', $amount));
 
-        /** @var Transaction $source */
+            return;
-        $source = Transaction::where('transaction_journal_id', $journal['transaction_journal_id'])->where('amount', '<', 0)->first();
-        /** @var Transaction $destination */
-        $destination = Transaction::where('transaction_journal_id', $journal['transaction_journal_id'])->where('amount', '>', 0)->first();
-
-        if ((int) $source->account_id === (int) $piggyBank->account_id) {
-            Log::debug('Piggy bank account is linked to source, so remove amount.');
-            $this->removeAmount($journal, $piggyBank, $destination->amount);
-
-            return true;
         }
-        if ((int) $destination->account_id === (int) $piggyBank->account_id) {
+        Log::debug(sprintf('Will now add %s to piggy bank.', $amount));
-            Log::debug('Piggy bank account is linked to source, so add amount.');
-            $this->addAmount($journal, $piggyBank, $destination->amount);
 
-            return true;
+        $repository->addAmount($piggyBank, $amount);
-        }
+        $repository->createEventWithJournal($piggyBank, app('steam')->positive($amount), $journal);
-        Log::info('Piggy bank is not linked to source or destination, so no action will be taken.');
-
-        return true;
     }
 }

app/TransactionRules/Engine/SearchRuleEngine.php

@@ -223,7 +223,7 @@ class SearchRuleEngine implements RuleEngineInterface
         Log::debug(sprintf('Now in findStrictRule(#%d)', $rule->id ?? 0));
         $searchArray = [];
         /** @var RuleTrigger $ruleTrigger */
-        foreach ($rule->ruleTriggers as $ruleTrigger) {
+        foreach ($rule->ruleTriggers()->where('active',1)->get() as $ruleTrigger) {
             // if needs no context, value is different:
             $needsContext = config(sprintf('firefly.search.operators.%s.needs_context', $ruleTrigger->trigger_type)) ?? true;
             if (false === $needsContext) {
@@ -370,7 +370,7 @@ class SearchRuleEngine implements RuleEngineInterface
     {
         Log::debug(sprintf('SearchRuleEngine:: Will now execute actions on transaction journal #%d', $transaction['transaction_journal_id']));
         /** @var RuleAction $ruleAction */
-        foreach ($rule->ruleActions as $ruleAction) {
+        foreach ($rule->ruleActions()->where('active',1)->get() as $ruleAction) {
             $break = $this->processRuleAction($ruleAction, $transaction);
             if (true === $break) {
                 break;
@@ -448,7 +448,7 @@ class SearchRuleEngine implements RuleEngineInterface
         $total = new Collection;
         $count = 0;
         /** @var RuleTrigger $ruleTrigger */
-        foreach ($rule->ruleTriggers as $ruleTrigger) {
+        foreach ($rule->ruleTriggers()->where('active',1)->get() as $ruleTrigger) {
             if ('user_action' === $ruleTrigger->trigger_type) {
                 Log::debug('Skip trigger type.');
                 continue;