diff --git a/app/TransactionRules/Actions/SetDescription.php b/app/TransactionRules/Actions/SetDescription.php index 5e12282d1f..8bd2a67458 100644 --- a/app/TransactionRules/Actions/SetDescription.php +++ b/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 $action; + private RuleAction $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; } } diff --git a/app/TransactionRules/Actions/SetDestinationAccount.php b/app/TransactionRules/Actions/SetDestinationAccount.php index 48727aec7b..d6ecd3eac5 100644 --- a/app/TransactionRules/Actions/SetDestinationAccount.php +++ b/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 $action; - - /** @var TransactionJournal The journal */ - private $journal; - - /** @var Account The new account */ - private $newDestinationAccount; - - /** @var AccountRepositoryInterface Account repository */ - private $repository; + private RuleAction $action; + private AccountRepositoryInterface $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; - $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; + return false; } /** - * @param array $types - * - * @return bool + * @return Account|null */ - private function findAccount(array $types): bool - { - $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 + private function findExpenseAccount(): ?Account { $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); } } diff --git a/app/TransactionRules/Actions/SetNotes.php b/app/TransactionRules/Actions/SetNotes.php index 0c1db8efec..168962cd7c 100644 --- a/app/TransactionRules/Actions/SetNotes.php +++ b/app/TransactionRules/Actions/SetNotes.php @@ -32,8 +32,7 @@ use Log; */ class SetNotes implements ActionInterface { - /** @var RuleAction The rule action */ - private $action; + private RuleACtion $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; } } diff --git a/app/TransactionRules/Actions/SetSourceAccount.php b/app/TransactionRules/Actions/SetSourceAccount.php index 9e2e4692ab..23f3aace23 100644 --- a/app/TransactionRules/Actions/SetSourceAccount.php +++ b/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 $action; - - /** @var TransactionJournal The journal */ - private $journal; - - /** @var Account The new source account */ - private $newSourceAccount; - - /** @var AccountRepositoryInterface Account repository */ - private $repository; + private RuleAction $action; + private AccountRepositoryInterface $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; - $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; + return false; } /** * @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); - - 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 $this->repository->findByName($this->action->action_value, $allowed); } /** - * + * @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; } } diff --git a/tests/Unit/TransactionRules/Actions/SetDescriptionTest.php b/tests/Unit/TransactionRules/Actions/SetDescriptionTest.php index 7076478b6c..8139621098 100644 --- a/tests/Unit/TransactionRules/Actions/SetDescriptionTest.php +++ b/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(); - $newDescription = 'new description' . $this->randomInt(); - $journal = $this->getRandomWithdrawal(); - $journal->description = $description; - $journal->save(); + $array = [ + 'description' => $oldDescription, + 'transaction_journal_id' => $withdrawal->id, + ]; // 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(); } } diff --git a/tests/Unit/TransactionRules/Actions/SetDestinationAccountTest.php b/tests/Unit/TransactionRules/Actions/SetDestinationAccountTest.php index 7c9fd50596..13af9ace8a 100644 --- a/tests/Unit/TransactionRules/Actions/SetDestinationAccountTest.php +++ b/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\TransactionJournal; -use FireflyIII\Repositories\Account\AccountRepositoryInterface; +use FireflyIII\Models\TransactionType; 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); - $deposit = $this->getRandomDeposit(); - $destinationTr = $deposit->transactions()->where('amount', '>', 0)->first(); - $destination = $destinationTr->account; - $user = $deposit->user; - $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); + // get random deposit: + $deposit = $this->getRandomDeposit(); + $destinationTr = $deposit->transactions()->where('amount', '>', 0)->first(); + $destination = $destinationTr->account; + $oldDestination = $destinationTr->account_id; - // find account? Return account: - $accountRepos->shouldReceive('setUser'); - $accountRepos->shouldReceive('findByName')->andReturn($account); + // grab unused asset account: + $user = $deposit->user; + $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(); - $newDestination = $destinationTr->account; - $this->assertNotEquals($destination->id, $newDestination->id); - $this->assertEquals($newDestination->id, $account->id); + $destinationTr->refresh(); + $updatedDestination = $destinationTr->account; + $this->assertEquals($newDestination->id, $updatedDestination->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); - $deposit = $this->getRandomDeposit(); + // get random deposit: + $deposit = $this->getRandomDeposit(); + $destinationTr = $deposit->transactions()->where('amount', '>', 0)->first(); + $oldDestination = $destinationTr->account; - // find account? Return account: - $accountRepos->shouldReceive('setUser'); - $accountRepos->shouldReceive('findByName')->andReturn(null); + // 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 = '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); - $account = $this->getRandomExpense(); - $withdrawal = $this->getRandomWithdrawal(); - - // find account? Return account: - $accountRepos->shouldReceive('setUser'); - $accountRepos->shouldReceive('findByName')->andReturn(null); - $accountRepos->shouldReceive('store')->once()->andReturn($account); + // get random withdrawal: + $withdrawal = $this->getRandomWithdrawal(); + $destinationTr = $withdrawal->transactions()->where('amount', '>', 0)->first(); + $oldDestination = $destinationTr->account; + // array with info: + $array = [ + '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 = '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); - $withdrawal = $this->getRandomWithdrawal(); - $destinationTr = $withdrawal->transactions()->where('amount', '>', 0)->first(); - $destination = $destinationTr->account; - $user = $withdrawal->user; - $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); + // get random withdrawal: + $withdrawal = $this->getRandomWithdrawal(); + $destinationTr = $withdrawal->transactions()->where('amount', '>', 0)->first(); + $oldDestination = $destinationTr->account; + $newDestination = $this->user()->accounts()->where('id', '!=', $oldDestination->id)->first(); - // find account? Return account: - $accountRepos->shouldReceive('setUser'); - $accountRepos->shouldReceive('findByName')->andReturn($account); + // array with info: + $array = [ + '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->id, $account->id); + $this->assertEquals($newDestination->name, $ruleAction->action_value); } } diff --git a/tests/Unit/TransactionRules/Actions/SetNotesTest.php b/tests/Unit/TransactionRules/Actions/SetNotesTest.php index f9af277b5a..53eb0fbd12 100644 --- a/tests/Unit/TransactionRules/Actions/SetNotesTest.php +++ b/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(); } /** diff --git a/tests/Unit/TransactionRules/Actions/SetSourceAccountTest.php b/tests/Unit/TransactionRules/Actions/SetSourceAccountTest.php index e4f45d1294..c30aab8cef 100644 --- a/tests/Unit/TransactionRules/Actions/SetSourceAccountTest.php +++ b/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\TransactionJournal; -use FireflyIII\Repositories\Account\AccountRepositoryInterface; +use FireflyIII\Models\TransactionType; 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(); - $sourceTr = $deposit->transactions()->where('amount', '<', 0)->first(); - $source = $sourceTr->account; - $user = $deposit->user; - $accountType = AccountType::whereType(AccountType::REVENUE)->first(); - $account = $user->accounts()->where('account_type_id', $accountType->id)->where('id', '!=', $source->id)->first(); + $deposit = $this->getRandomDeposit(); + $sourceTr = $deposit->transactions()->where('amount', '<', 0)->first(); + $source = $sourceTr->account; + $user = $deposit->user; + $accountType = AccountType::whereType(AccountType::REVENUE)->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: - $accountRepos->shouldReceive('setUser'); - $accountRepos->shouldReceive('findByName')->andReturn($account); + $array = [ + 'user_id' => $this->user()->id, + '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); - $account = $this->getRandomRevenue(); - $deposit = $this->getRandomDeposit(); + $deposit = $this->getRandomDeposit(); - $accountRepos->shouldReceive('setUser'); - $accountRepos->shouldReceive('findByName')->andReturn(null); - $accountRepos->shouldReceive('store')->once()->andReturn($account); + $array = [ + 'transaction_journal_id' => $deposit->id, + '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); - - $accountRepos->shouldReceive('setUser'); - $accountRepos->shouldReceive('findByName')->andReturn($account); + $array = [ + 'user_id' => $this->user()->id, + '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'); - $accountRepos->shouldReceive('findByName')->andReturn(null); + $array = [ + '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); } }