Fix actions and associated tests.

app/TransactionRules/Actions/SetDescription.php

@@ -22,6 +22,7 @@ declare(strict_types=1);
 
 namespace FireflyIII\TransactionRules\Actions;
 
+use DB;
 use FireflyIII\Models\RuleAction;
 use FireflyIII\Models\TransactionJournal;
 use Log;
@@ -31,8 +32,7 @@ use Log;
  */
 class SetDescription implements ActionInterface
 {
-    /** @var RuleAction The rule action */
+    private RuleAction $action;
-    private $action;
 
     /**
      * TriggerInterface constructor.
@@ -46,10 +46,11 @@ class SetDescription implements ActionInterface
 
     /**
      * Set description to X
-     *
      * @param TransactionJournal $journal
      *
      * @return bool
+     * @deprecated
+     * @codeCoverageIgnore
      */
     public function act(TransactionJournal $journal): bool
     {
@@ -75,6 +76,18 @@ class SetDescription implements ActionInterface
      */
     public function actOnArray(array $journal): bool
     {
-        // TODO: Implement actOnArray() method.
+        DB::table('transaction_journals')
+          ->where('id', '=', $journal['transaction_journal_id'])
+          ->update(['description' => $this->action->action_value]);
+
+        Log::debug(
+            sprintf(
+                'RuleAction SetDescription changed the description of journal #%d from "%s" to "%s".',
+                $journal['transaction_journal_id'],
+                $journal['description'],
+                $this->action->action_value
+            )
+        );
+        return true;
     }
 }

app/TransactionRules/Actions/SetDestinationAccount.php

@@ -22,12 +22,13 @@ 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\Transaction;
 use FireflyIII\Models\TransactionJournal;
 use FireflyIII\Repositories\Account\AccountRepositoryInterface;
+use FireflyIII\User;
 use Log;
 
 /**
@@ -35,17 +36,8 @@ use Log;
  */
 class SetDestinationAccount implements ActionInterface
 {
-    /** @var RuleAction The rule action */
+    private RuleAction                 $action;
-    private $action;
+    private AccountRepositoryInterface $repository;
-
-    /** @var TransactionJournal The journal */
-    private $journal;
-
-    /** @var Account The new account */
-    private $newDestinationAccount;
-
-    /** @var AccountRepositoryInterface Account repository */
-    private $repository;
 
     /**
      * TriggerInterface constructor.
@@ -59,99 +51,21 @@ class SetDestinationAccount implements ActionInterface
 
     /**
      * Set destination account to X
-     *
      * @param TransactionJournal $journal
      *
      * @return bool
+     * @deprecated
+     * @codeCoverageIgnore
      */
     public function act(TransactionJournal $journal): bool
     {
-        $this->journal    = $journal;
+        return false;
-        $this->repository = app(AccountRepositoryInterface::class);
-        $this->repository->setUser($journal->user);
-        // journal type:
-        $type = $journal->transactionType->type;
-        // source and destination:
-        /** @var Transaction $source */
-        $source = $journal->transactions()->where('amount', '<', 0)->first();
-        /** @var Transaction $destination */
-        $destination = $journal->transactions()->where('amount', '>', 0)->first();
-
-        // sanity check:
-        if (null === $source || null === $destination) {
-            Log::error(sprintf('Cannot run rule on journal #%d because no source or dest.', $journal->id));
-
-            return false;
-        }
-
-        // it depends on the type what kind of destination account is expected.
-        $expectedDestinationTypes = config(sprintf('firefly.source_dests.%s.%s', $type, $source->account->accountType->type));
-
-        if (null === $expectedDestinationTypes) {
-            Log::error(
-                sprintf(
-                    'Configuration line "%s" is unexpectedly empty. Stopped.', sprintf('firefly.source_dests.%s.%s', $type, $source->account->accountType->type)
-                )
-            );
-
-            return false;
-        }
-
-        // try to find an account with the destination name and these types:
-        $newDestination = $this->findAccount($expectedDestinationTypes);
-        if (true === $newDestination) {
-            // update account.
-            $destination->account_id = $this->newDestinationAccount->id;
-            $destination->save();
-            $journal->touch();
-            Log::debug(sprintf('Updated transaction #%d and gave it new account ID.', $destination->id));
-
-            return true;
-        }
-
-        Log::info(sprintf('Expected destination account "%s" not found.', $this->action->action_value));
-        if (AccountType::EXPENSE === $expectedDestinationTypes[0]) {
-            Log::debug('Expected type is expense, lets create it.');
-            $this->findExpenseAccount();
-            // update account.
-            $destination->account_id = $this->newDestinationAccount->id;
-            $destination->save();
-            $journal->touch();
-            Log::debug(sprintf('Updated transaction #%d and gave it new account ID.', $destination->id));
-        }
-
-        return true;
     }
 
     /**
-     * @param array $types
+     * @return Account|null
-     *
-     * @return bool
      */
-    private function findAccount(array $types): bool
+    private function findExpenseAccount(): ?Account
-    {
-        $account = $this->repository->findByName($this->action->action_value, $types);
-
-        if (null === $account) {
-            Log::debug(sprintf('There is NO account called "%s" of type', $this->action->action_value), $types);
-
-            return false;
-        }
-        Log::debug(
-            sprintf(
-                'There exists an account called "%s". ID is #%d. Type is "%s"',
-                $this->action->action_value, $account->id, $account->accountType->type
-            )
-        );
-        $this->newDestinationAccount = $account;
-
-        return true;
-    }
-
-    /**
-     *
-     */
-    private function findExpenseAccount(): void
     {
         $account = $this->repository->findByName($this->action->action_value, [AccountType::EXPENSE]);
         if (null === $account) {
@@ -166,7 +80,7 @@ class SetDestinationAccount implements ActionInterface
             $account = $this->repository->store($data);
         }
         Log::debug(sprintf('Found or created expense account #%d ("%s")', $account->id, $account->name));
-        $this->newDestinationAccount = $account;
+        return $account;
     }
 
     /**
@@ -174,6 +88,58 @@ class SetDestinationAccount implements ActionInterface
      */
     public function actOnArray(array $journal): bool
     {
-        // TODO: Implement actOnArray() method.
+        $user             = User::find($journal['user_id']);
+        $type             = $journal['transaction_type_type'];
+        $this->repository = app(AccountRepositoryInterface::class);
+        $this->repository->setUser($user);
+
+        // 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'])));
+
+            return false;
+        }
+        // try to find an account with the destination name and these types:
+        $destination = $this->findAccount($expectedTypes);
+        if (null !== $destination) {
+            // update account of destination transaction.
+            DB::table('transactions')
+              ->where('transaction_journal_id', '=', $journal['transaction_journal_id'])
+              ->where('amount', '>', 0)
+              ->update(['account_id' => $destination->id]);
+            Log::debug(sprintf('Updated journal #%d and gave it new account ID.', $journal['transaction_journal_id']));
+
+            return true;
+        }
+        Log::info(sprintf('Expected destination account "%s" not found.', $this->action->action_value));
+
+        if (in_array(AccountType::EXPENSE, $expectedTypes)) {
+            // does not exist, but can be created.
+            Log::debug('Expected type is expense, lets create it.');
+            $expense = $this->findExpenseAccount();
+            if (null === $expense) {
+                Log::error('Could not create expense account.');
+                return false;
+            }
+            DB::table('transactions')
+              ->where('transaction_journal_id', '=', $journal['transaction_journal_id'])
+              ->where('amount', '>', 0)
+              ->update(['account_id' => $expense->id]);
+            Log::debug(sprintf('Updated journal #%d and gave it new account ID.', $journal['transaction_journal_id']));
+
+            return true;
+        }
+
+        return false;
+    }
+
+    /**
+     * @param array $types
+     * @return Account|null
+     */
+    private function findAccount(array $types): ?Account
+    {
+        return $this->repository->findByName($this->action->action_value, $types);
     }
 }

app/TransactionRules/Actions/SetNotes.php

@@ -32,8 +32,7 @@ use Log;
  */
 class SetNotes implements ActionInterface
 {
-    /** @var RuleAction The rule action */
+    private RuleACtion $action;
-    private $action;
 
     /**
      * TriggerInterface constructor.
@@ -49,8 +48,9 @@ class SetNotes implements ActionInterface
      * Set notes to X
      *
      * @param TransactionJournal $journal
-     *
      * @return bool
+     * @deprecated
+     * @codeCoverageIgnore
      */
     public function act(TransactionJournal $journal): bool
     {
@@ -74,6 +74,20 @@ class SetNotes implements ActionInterface
      */
     public function actOnArray(array $journal): bool
     {
-        // TODO: Implement actOnArray() method.
+        $dbNote = Note::where('noteable_id', $journal['transaction_journal_id'])
+                      ->where('noteable_type', TransactionJournal::class)->first();
+        if (null === $dbNote) {
+            $dbNote                = new Note;
+            $dbNote->noteable_id   = $journal['transaction_journal_id'];
+            $dbNote->noteable_type = TransactionJournal::class;
+            $dbNote->text          = '';
+        }
+        $oldNotes     = $dbNote->text;
+        $dbNote->text = $this->action->action_value;
+        $dbNote->save();
+
+        Log::debug(sprintf('RuleAction SetNotes changed the notes of journal #%d from "%s" to "%s".', $journal['transaction_journal_id'], $oldNotes, $this->action->action_value));
+
+        return true;
     }
 }

app/TransactionRules/Actions/SetSourceAccount.php

@@ -22,12 +22,13 @@ 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;
 
 /**
@@ -35,17 +36,8 @@ use Log;
  */
 class SetSourceAccount implements ActionInterface
 {
-    /** @var RuleAction The rule action */
+    private RuleAction                 $action;
-    private $action;
+    private AccountRepositoryInterface $repository;
-
-    /** @var TransactionJournal The journal */
-    private $journal;
-
-    /** @var Account The new source account */
-    private $newSourceAccount;
-
-    /** @var AccountRepositoryInterface Account repository */
-    private $repository;
 
     /**
      * TriggerInterface constructor.
@@ -61,85 +53,34 @@ class SetSourceAccount implements ActionInterface
      * Set source account to X
      *
      * @param TransactionJournal $journal
-     *
      * @return bool
+     * @deprecated
+     * @codeCoverageIgnore
      */
     public function act(TransactionJournal $journal): bool
     {
-        $this->journal    = $journal;
+        return false;
-        $this->repository = app(AccountRepositoryInterface::class);
-        $this->repository->setUser($journal->user);
-        // journal type:
-        $type = $journal->transactionType->type;
-
-        // 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:
-        if ((TransactionType::WITHDRAWAL === $type || TransactionType::TRANSFER === $type) && !$this->findAssetAccount($type)) {
-            Log::error(
-                sprintf(
-                    'Cannot change source account of journal #%d because no asset account with name "%s" exists.',
-                    $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) {
-            $this->findRevenueAccount();
-        }
-
-        Log::debug(sprintf('New source account is #%d ("%s").', $this->newSourceAccount->id, $this->newSourceAccount->name));
-
-        // update source transaction with new source account:
-        // get source transaction:
-        $transaction = $journal->transactions()->where('amount', '<', 0)->first();
-        if (null === $transaction) {
-            // @codeCoverageIgnoreStart
-            Log::error(sprintf('Cannot change source account of journal #%d because no source transaction exists.', $journal->id));
-
-            return false;
-            // @codeCoverageIgnoreEnd
-        }
-        $transaction->account_id = $this->newSourceAccount->id;
-        $transaction->save();
-        $journal->touch();
-        Log::debug(sprintf('Updated transaction #%d and gave it new account ID.', $transaction->id));
-
-        return true;
     }
 
     /**
      * @param string $type
      *
-     * @return bool
+     * @return Account|null
      */
-    private function findAssetAccount(string $type): bool
+    private function findAssetAccount(string $type): ?Account
     {
         // switch on type:
         $allowed = config(sprintf('firefly.expected_source_types.source.%s', $type));
         $allowed = is_array($allowed) ? $allowed : [];
-
         Log::debug(sprintf('Check config for expected_source_types.source.%s, result is', $type), $allowed);
 
-        $account = $this->repository->findByName($this->action->action_value, $allowed);
+        return $this->repository->findByName($this->action->action_value, $allowed);
-
-        if (null === $account) {
-            Log::debug(sprintf('There is NO valid source account called "%s".', $this->action->action_value));
-
-            return false;
-        }
-        Log::debug(sprintf('There exists a valid source account called "%s". ID is #%d', $this->action->action_value, $account->id));
-        $this->newSourceAccount = $account;
-
-        return true;
     }
 
     /**
-     *
+     * @return Account|null
      */
-    private function findRevenueAccount(): void
+    private function findRevenueAccount(): ?Account
     {
         $allowed = config('firefly.expected_source_types.source.Deposit');
         $account = $this->repository->findByName($this->action->action_value, $allowed);
@@ -156,7 +97,7 @@ class SetSourceAccount implements ActionInterface
             $account = $this->repository->store($data);
         }
         Log::debug(sprintf('Found or created revenue account #%d ("%s")', $account->id, $account->name));
-        $this->newSourceAccount = $account;
+        return $account;
     }
 
     /**
@@ -164,6 +105,39 @@ class SetSourceAccount implements ActionInterface
      */
     public function actOnArray(array $journal): bool
     {
-        // TODO: Implement actOnArray() method.
+        $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;
     }
 }

tests/Unit/TransactionRules/Actions/SetDescriptionTest.php

@@ -23,7 +23,6 @@ declare(strict_types=1);
 namespace Tests\Unit\TransactionRules\Actions;
 
 use FireflyIII\Models\RuleAction;
-use FireflyIII\Models\TransactionJournal;
 use FireflyIII\TransactionRules\Actions\SetDescription;
 use Tests\TestCase;
 
@@ -32,37 +31,34 @@ use Tests\TestCase;
  */
 class SetDescriptionTest extends TestCase
 {
-    /**
-     * Set up test
-     */
-    public function setUp(): void
-    {
-        self::markTestIncomplete('Incomplete for refactor.');
-
-        return;
-    }
-
     /**
      * @covers \FireflyIII\TransactionRules\Actions\SetDescription
      */
     public function testAct(): void
     {
+        $withdrawal     = $this->getRandomWithdrawal();
+        $newDescription = sprintf('new description #%d', $this->randomInt());
+        $oldDescription = $withdrawal->description;
+
         // get journal, give fixed description
-        $description          = 'text' . $this->randomInt();
+        $array = [
-        $newDescription       = 'new description' . $this->randomInt();
+            'description'            => $oldDescription,
-        $journal              = $this->getRandomWithdrawal();
+            'transaction_journal_id' => $withdrawal->id,
-        $journal->description = $description;
+        ];
-        $journal->save();
 
         // fire the action:
         $ruleAction               = new RuleAction;
         $ruleAction->action_value = $newDescription;
         $action                   = new SetDescription($ruleAction);
-        $result                   = $action->act($journal);
+        $result                   = $action->actOnArray($array);
         $this->assertTrue($result);
-        $journal = TransactionJournal::find($journal->id);
+
+        $withdrawal->refresh();
 
         // assert result
-        $this->assertEquals($newDescription, $journal->description);
+        $this->assertEquals($newDescription, $withdrawal->description);
+
+        $withdrawal->description = $oldDescription;
+        $withdrawal->save();
     }
 }

tests/Unit/TransactionRules/Actions/SetDestinationAccountTest.php

@@ -22,12 +22,9 @@ declare(strict_types=1);
 
 namespace Tests\Unit\TransactionRules\Actions;
 
-use DB;
 use FireflyIII\Models\AccountType;
 use FireflyIII\Models\RuleAction;
-use FireflyIII\Models\Transaction;
+use FireflyIII\Models\TransactionType;
-use FireflyIII\Models\TransactionJournal;
-use FireflyIII\Repositories\Account\AccountRepositoryInterface;
 use FireflyIII\TransactionRules\Actions\SetDestinationAccount;
 use Tests\TestCase;
 
@@ -38,15 +35,6 @@ use Tests\TestCase;
  */
 class SetDestinationAccountTest extends TestCase
 {
-    /**
-     * Set up test
-     */
-    public function setUp(): void
-    {
-        self::markTestIncomplete('Incomplete for refactor.');
-
-        return;
-    }
     /**
      * Give deposit existing asset account.
      *
@@ -54,31 +42,41 @@ class SetDestinationAccountTest extends TestCase
      */
     public function testActDepositExisting(): void
     {
-        $accountRepos  = $this->mock(AccountRepositoryInterface::class);
+        // get random deposit:
-        $deposit       = $this->getRandomDeposit();
+        $deposit        = $this->getRandomDeposit();
-        $destinationTr = $deposit->transactions()->where('amount', '>', 0)->first();
+        $destinationTr  = $deposit->transactions()->where('amount', '>', 0)->first();
-        $destination   = $destinationTr->account;
+        $destination    = $destinationTr->account;
-        $user          = $deposit->user;
+        $oldDestination = $destinationTr->account_id;
-        $accountType   = AccountType::whereType(AccountType::ASSET)->first();
-        $account       = $user->accounts()->where('account_type_id', $accountType->id)->where('id', '!=', $destination->id)->first();
-        $this->assertNotEquals($destination->id, $account->id);
 
-        // find account? Return account:
+        // grab unused asset account:
-        $accountRepos->shouldReceive('setUser');
+        $user           = $deposit->user;
-        $accountRepos->shouldReceive('findByName')->andReturn($account);
+        $accountType    = AccountType::whereType(AccountType::ASSET)->first();
+        $newDestination = $user->accounts()->where('account_type_id', $accountType->id)->where('id', '!=', $destination->id)->first();
+        $this->assertNotEquals($destination->id, $newDestination->id);
+
+        // array with info:
+        $array = [
+            'transaction_journal_id' => $deposit->id,
+            'user_id'                => $this->user()->id,
+            'transaction_type_type'  => TransactionType::DEPOSIT,
+            'source_account_type'    => AccountType::REVENUE,
+        ];
 
         // fire the action:
         $ruleAction               = new RuleAction;
-        $ruleAction->action_value = $account->name;
+        $ruleAction->action_value = $newDestination->name;
         $action                   = new SetDestinationAccount($ruleAction);
-        $result                   = $action->act($deposit);
+        $result                   = $action->actOnArray($array);
         $this->assertTrue($result);
 
         // test journal for new account
-        $destinationTr  = $deposit->transactions()->where('amount', '>', 0)->first();
+        $destinationTr->refresh();
-        $newDestination = $destinationTr->account;
+        $updatedDestination = $destinationTr->account;
-        $this->assertNotEquals($destination->id, $newDestination->id);
+        $this->assertEquals($newDestination->id, $updatedDestination->id);
-        $this->assertEquals($newDestination->id, $account->id);
+        $this->assertEquals($destinationTr->account_id, $newDestination->id);
+
+        $destinationTr->account_id = $oldDestination;
+        $destinationTr->save();
     }
 
     /**
@@ -88,19 +86,29 @@ class SetDestinationAccountTest extends TestCase
      */
     public function testActDepositNotExisting(): void
     {
-        $accountRepos = $this->mock(AccountRepositoryInterface::class);
+        // get random deposit:
-        $deposit      = $this->getRandomDeposit();
+        $deposit        = $this->getRandomDeposit();
+        $destinationTr  = $deposit->transactions()->where('amount', '>', 0)->first();
+        $oldDestination = $destinationTr->account;
 
-        // find account? Return account:
+        // array with info:
-        $accountRepos->shouldReceive('setUser');
+        $array = [
-        $accountRepos->shouldReceive('findByName')->andReturn(null);
+            'transaction_journal_id' => $deposit->id,
+            'user_id'                => $this->user()->id,
+            'transaction_type_type'  => TransactionType::DEPOSIT,
+            'source_account_type'    => AccountType::REVENUE,
+        ];
 
         // fire the action:
         $ruleAction               = new RuleAction;
-        $ruleAction->action_value = 'Not existing asset account #' . $this->randomInt();
+        $ruleAction->action_value = sprintf('Not existing asset account #%d', $this->randomInt());
         $action                   = new SetDestinationAccount($ruleAction);
-        $result                   = $action->act($deposit);
+        $result                   = $action->actOnArray($array);
         $this->assertFalse($result);
+
+        // test journal for new account
+        $destinationTr->refresh();
+        $this->assertEquals($destinationTr->account_id, $oldDestination->id);
     }
 
     /**
@@ -110,22 +118,32 @@ class SetDestinationAccountTest extends TestCase
      */
     public function testActWithDrawalNotExisting(): void
     {
-        $accountRepos = $this->mock(AccountRepositoryInterface::class);
+        // get random withdrawal:
-        $account      = $this->getRandomExpense();
+        $withdrawal     = $this->getRandomWithdrawal();
-        $withdrawal   = $this->getRandomWithdrawal();
+        $destinationTr  = $withdrawal->transactions()->where('amount', '>', 0)->first();
-
+        $oldDestination = $destinationTr->account;
-        // find account? Return account:
+        // array with info:
-        $accountRepos->shouldReceive('setUser');
+        $array = [
-        $accountRepos->shouldReceive('findByName')->andReturn(null);
+            'transaction_journal_id' => $withdrawal->id,
-        $accountRepos->shouldReceive('store')->once()->andReturn($account);
+            'user_id'                => $this->user()->id,
+            'transaction_type_type'  => TransactionType::WITHDRAWAL,
+            'source_account_type'    => AccountType::ASSET,
+        ];
 
         // fire the action:
         $ruleAction               = new RuleAction;
-        $ruleAction->action_value = 'Not existing expense account #' . $this->randomInt();
+        $ruleAction->action_value = sprintf('Not existing expense account #%d', $this->randomInt());
         $action                   = new SetDestinationAccount($ruleAction);
-        $result                   = $action->act($withdrawal);
+        $result                   = $action->actOnArray($array);
-
         $this->assertTrue($result);
+
+        // test journal for new account
+        $destinationTr->refresh();
+        $newDestination = $destinationTr->account;
+        $this->assertEquals($newDestination->name, $ruleAction->action_value);
+
+        $destinationTr->account_id = $oldDestination;
+        $destinationTr->save();
     }
 
     /**
@@ -135,31 +153,31 @@ class SetDestinationAccountTest extends TestCase
      */
     public function testActWithdrawalExisting(): void
     {
-        $accountRepos  = $this->mock(AccountRepositoryInterface::class);
+        // get random withdrawal:
-        $withdrawal    = $this->getRandomWithdrawal();
+        $withdrawal     = $this->getRandomWithdrawal();
-        $destinationTr = $withdrawal->transactions()->where('amount', '>', 0)->first();
+        $destinationTr  = $withdrawal->transactions()->where('amount', '>', 0)->first();
-        $destination   = $destinationTr->account;
+        $oldDestination = $destinationTr->account;
-        $user          = $withdrawal->user;
+        $newDestination = $this->user()->accounts()->where('id', '!=', $oldDestination->id)->first();
-        $accountType   = AccountType::whereType(AccountType::EXPENSE)->first();
-        $account       = $user->accounts()->where('account_type_id', $accountType->id)->where('id', '!=', $destination->id)->first();
-        $this->assertNotEquals($destination->id, $account->id);
 
-        // find account? Return account:
+        // array with info:
-        $accountRepos->shouldReceive('setUser');
+        $array = [
-        $accountRepos->shouldReceive('findByName')->andReturn($account);
+            'transaction_journal_id' => $withdrawal->id,
+            'user_id'                => $this->user()->id,
+            'transaction_type_type'  => TransactionType::WITHDRAWAL,
+            'source_account_type'    => AccountType::ASSET,
+        ];
 
         // fire the action:
         $ruleAction               = new RuleAction;
-        $ruleAction->action_value = $account->name;
+        $ruleAction->action_value = $newDestination->name;
         $action                   = new SetDestinationAccount($ruleAction);
-        $result                   = $action->act($withdrawal);
+        $result                   = $action->actOnArray($array);
         $this->assertTrue($result);
 
         // test journal for new account
-        $destinationTr  = $withdrawal->transactions()->where('amount', '>', 0)->first();
+        $destinationTr->refresh();
         $newDestination = $destinationTr->account;
-        $this->assertNotEquals($destination->id, $newDestination->id);
+        $this->assertEquals($newDestination->name, $ruleAction->action_value);
-        $this->assertEquals($newDestination->id, $account->id);
     }
 
 }

tests/Unit/TransactionRules/Actions/SetNotesTest.php

@@ -33,16 +33,6 @@ use Tests\TestCase;
  */
 class SetNotesTest extends TestCase
 {
-    /**
-     * Set up test
-     */
-    public function setUp(): void
-    {
-        self::markTestIncomplete('Incomplete for refactor.');
-
-        return;
-    }
-
     /**
      * @covers \FireflyIII\TransactionRules\Actions\SetNotes
      */
@@ -61,7 +51,7 @@ class SetNotesTest extends TestCase
 
         // fire the action:
         $ruleAction               = new RuleAction;
-        $ruleAction->action_value = 'These are new notes ' . $this->randomInt();
+        $ruleAction->action_value = sprintf('These are new notes #%d', $this->randomInt());
         $action                   = new SetNotes($ruleAction);
         $result                   = $action->act($journal);
         $this->assertTrue($result);
@@ -69,6 +59,7 @@ class SetNotesTest extends TestCase
         // assert result
         $this->assertEquals(1, $journal->notes()->count());
         $this->assertEquals($note->id, $journal->notes()->first()->id);
+        $journal->notes()->delete();
     }
 
     /**

tests/Unit/TransactionRules/Actions/SetSourceAccountTest.php

@@ -22,12 +22,9 @@ declare(strict_types=1);
 
 namespace Tests\Unit\TransactionRules\Actions;
 
-use DB;
 use FireflyIII\Models\AccountType;
 use FireflyIII\Models\RuleAction;
-use FireflyIII\Models\Transaction;
+use FireflyIII\Models\TransactionType;
-use FireflyIII\Models\TransactionJournal;
-use FireflyIII\Repositories\Account\AccountRepositoryInterface;
 use FireflyIII\TransactionRules\Actions\SetSourceAccount;
 use Tests\TestCase;
 
@@ -36,16 +33,6 @@ use Tests\TestCase;
  */
 class SetSourceAccountTest extends TestCase
 {
-    /**
-     * Set up test
-     */
-    public function setUp(): void
-    {
-        self::markTestIncomplete('Incomplete for refactor.');
-
-        return;
-    }
-
     /**
      * Give deposit existing revenue account.
      *
@@ -53,24 +40,25 @@ class SetSourceAccountTest extends TestCase
      */
     public function testActDepositExistingUpdated(): void
     {
-        $accountRepos = $this->mock(AccountRepositoryInterface::class);
+        $deposit     = $this->getRandomDeposit();
-        $deposit      = $this->getRandomDeposit();
+        $sourceTr    = $deposit->transactions()->where('amount', '<', 0)->first();
-        $sourceTr     = $deposit->transactions()->where('amount', '<', 0)->first();
+        $source      = $sourceTr->account;
-        $source       = $sourceTr->account;
+        $user        = $deposit->user;
-        $user         = $deposit->user;
+        $accountType = AccountType::whereType(AccountType::REVENUE)->first();
-        $accountType  = AccountType::whereType(AccountType::REVENUE)->first();
+        $account     = $user->accounts()->where('account_type_id', $accountType->id)->where('id', '!=', $source->id)->first();
-        $account      = $user->accounts()->where('account_type_id', $accountType->id)->where('id', '!=', $source->id)->first();
         $this->assertNotEquals($source->id, $account->id);
 
-        // find account? Return account:
+        $array = [
-        $accountRepos->shouldReceive('setUser');
+            'user_id'               => $this->user()->id,
-        $accountRepos->shouldReceive('findByName')->andReturn($account);
+            'transaction_journal_id' => $deposit->id,
+            'transaction_type_type' => TransactionType::DEPOSIT,
+        ];
 
         // fire the action:
         $ruleAction               = new RuleAction;
         $ruleAction->action_value = $account->name;
         $action                   = new SetSourceAccount($ruleAction);
-        $result                   = $action->act($deposit);
+        $result                   = $action->actOnArray($array);
         $this->assertTrue($result);
 
         // test journal for new account
@@ -78,6 +66,9 @@ class SetSourceAccountTest extends TestCase
         $newSource = $sourceTr->account;
         $this->assertNotEquals($source->id, $newSource->id);
         $this->assertEquals($newSource->id, $account->id);
+
+        $sourceTr->account_id = $source->id;
+        $sourceTr->save();
     }
 
     /**
@@ -87,19 +78,19 @@ class SetSourceAccountTest extends TestCase
      */
     public function testActDepositRevenue(): void
     {
-        $accountRepos = $this->mock(AccountRepositoryInterface::class);
+        $deposit = $this->getRandomDeposit();
-        $account      = $this->getRandomRevenue();
-        $deposit      = $this->getRandomDeposit();
 
-        $accountRepos->shouldReceive('setUser');
+        $array = [
-        $accountRepos->shouldReceive('findByName')->andReturn(null);
+            'transaction_journal_id' => $deposit->id,
-        $accountRepos->shouldReceive('store')->once()->andReturn($account);
+            'user_id'                => $this->user()->id,
+            'transaction_type_type' => TransactionType::DEPOSIT,
+        ];
 
         // fire the action:
         $ruleAction               = new RuleAction;
-        $ruleAction->action_value = 'Some new revenue #' . $this->randomInt();
+        $ruleAction->action_value = sprintf('Some new revenue #%d', $this->randomInt());
         $action                   = new SetSourceAccount($ruleAction);
-        $result                   = $action->act($deposit);
+        $result                   = $action->actOnArray($array);
         $this->assertTrue($result);
     }
 
@@ -110,9 +101,7 @@ class SetSourceAccountTest extends TestCase
      */
     public function testActWithdrawalExistingUpdated(): void
     {
-        $accountRepos = $this->mock(AccountRepositoryInterface::class);
+        $withdrawal  = $this->getRandomWithdrawal();
-        $withdrawal   = $this->getRandomWithdrawal();
-
         $sourceTr    = $withdrawal->transactions()->where('amount', '<', 0)->first();
         $source      = $sourceTr->account;
         $user        = $withdrawal->user;
@@ -120,15 +109,17 @@ class SetSourceAccountTest extends TestCase
         $account     = $user->accounts()->where('account_type_id', $accountType->id)->where('id', '!=', $source->id)->first();
         $this->assertNotEquals($source->id, $account->id);
 
-
+        $array = [
-        $accountRepos->shouldReceive('setUser');
+            'user_id' => $this->user()->id,
-        $accountRepos->shouldReceive('findByName')->andReturn($account);
+            'transaction_type_type' => TransactionType::WITHDRAWAL,
+            'transaction_journal_id' => $withdrawal->id,
+        ];
 
         // fire the action:
         $ruleAction               = new RuleAction;
         $ruleAction->action_value = $account->name;
         $action                   = new SetSourceAccount($ruleAction);
-        $result                   = $action->act($withdrawal);
+        $result                   = $action->actOnArray($array);
         $this->assertTrue($result);
 
         // test journal for new account
@@ -136,6 +127,9 @@ class SetSourceAccountTest extends TestCase
         $newSource = $sourceTr->account;
         $this->assertNotEquals($source->id, $newSource->id);
         $this->assertEquals($newSource->id, $account->id);
+
+        $sourceTr->account_id = $source->id;
+        $sourceTr->save();
     }
 
     /**
@@ -145,17 +139,19 @@ class SetSourceAccountTest extends TestCase
      */
     public function testActWithdrawalNotExisting(): void
     {
-        $accountRepos = $this->mock(AccountRepositoryInterface::class);
+        $withdrawal = $this->getRandomWithdrawal();
-        $withdrawal   = $this->getRandomWithdrawal();
 
-        $accountRepos->shouldReceive('setUser');
+        $array = [
-        $accountRepos->shouldReceive('findByName')->andReturn(null);
+            'user_id' => $this->user()->id,
+            'transaction_type_type' => TransactionType::WITHDRAWAL,
+            'transaction_journal_id' => $withdrawal->id,
+        ];
 
         // fire the action:
         $ruleAction               = new RuleAction;
-        $ruleAction->action_value = 'Some new account #' . $this->randomInt();
+        $ruleAction->action_value = sprintf('Some new account #%d', $this->randomInt());
         $action                   = new SetSourceAccount($ruleAction);
-        $result                   = $action->act($withdrawal);
+        $result                   = $action->actOnArray($array);
         $this->assertFalse($result);
     }
 }