From a47da92d8167bba38c574e1e8408fdf16c326440 Mon Sep 17 00:00:00 2001 From: James Cole Date: Sat, 12 May 2018 13:27:02 +0200 Subject: [PATCH] Test coverage for opposing account mapper. --- .../Routine/File/OpposingAccountMapper.php | 12 +- .../File/OpposingAccountMapperTest.php | 184 ++++++++++++++++++ 2 files changed, 190 insertions(+), 6 deletions(-) diff --git a/app/Support/Import/Routine/File/OpposingAccountMapper.php b/app/Support/Import/Routine/File/OpposingAccountMapper.php index f03f3f25da..42664b8c9e 100644 --- a/app/Support/Import/Routine/File/OpposingAccountMapper.php +++ b/app/Support/Import/Routine/File/OpposingAccountMapper.php @@ -79,11 +79,11 @@ class OpposingAccountMapper // if result is not null, system has found an account // but it's of the wrong type. If we dont have a name, use // the result's name, iban in the search below. - if (null !== $result && '' === (string)$data['name']) { + if (null !== $result && '' === (string)($data['name'] ?? '')) { Log::debug(sprintf('Will search for account with name "%s" instead of NULL.', $result->name)); $data['name'] = $result->name; } - if ('' !== (string)$result->iban && null !== $result && '' === $data['iban']) { + if (null !== $result && '' !== (string)$result->iban && '' === ($data['iban'] ?? '')) { Log::debug(sprintf('Will search for account with IBAN "%s" instead of NULL.', $result->iban)); $data['iban'] = $result->iban; } @@ -95,7 +95,7 @@ class OpposingAccountMapper // IBAN, accountNumber, name, $fields = ['iban' => 'findByIbanNull', 'number' => 'findByAccountNumber', 'name' => 'findByName']; foreach ($fields as $field => $function) { - $value = (string)$data[$field]; + $value = (string)($data[$field] ?? ''); if ('' === $value) { Log::debug(sprintf('Array does not contain a value for %s. Continue', $field)); continue; @@ -112,12 +112,12 @@ class OpposingAccountMapper // not found? Create it! $creation = [ 'name' => $data['name'] ?? '(no name)', - 'iban' => $data['iban'], - 'accountNumber' => $data['number'], + 'iban' => $data['iban']?? null, + 'accountNumber' => $data['number'] ?? null, 'account_type_id' => null, 'accountType' => $expectedType, 'active' => true, - 'BIC' => $data['bic'], + 'BIC' => $data['bic'] ?? null, ]; Log::debug('Will try to store a new account: ', $creation); diff --git a/tests/Unit/Support/Import/Routine/File/OpposingAccountMapperTest.php b/tests/Unit/Support/Import/Routine/File/OpposingAccountMapperTest.php index d92cd6a5ca..f41f09289c 100644 --- a/tests/Unit/Support/Import/Routine/File/OpposingAccountMapperTest.php +++ b/tests/Unit/Support/Import/Routine/File/OpposingAccountMapperTest.php @@ -24,6 +24,10 @@ declare(strict_types=1); namespace Tests\Unit\Support\Import\Routine\File; +use FireflyIII\Models\Account; +use FireflyIII\Models\AccountType; +use FireflyIII\Repositories\Account\AccountRepositoryInterface; +use FireflyIII\Support\Import\Routine\File\OpposingAccountMapper; use Tests\TestCase; /** @@ -31,5 +35,185 @@ use Tests\TestCase; */ class OpposingAccountMapperTest extends TestCase { + /** + * + * Should return account with given ID (which is of correct type). + * + * @covers \FireflyIII\Support\Import\Routine\File\OpposingAccountMapper + */ + public function testAccountId(): void + { + $expected = $this->user()->accounts()->where('account_type_id', 4)->inRandomOrder()->first(); + $amount = '-12.34'; + $repository = $this->mock(AccountRepositoryInterface::class); + $repository->shouldReceive('setUser')->once(); + $repository->shouldReceive('findNull')->andReturn($expected)->once(); + $mapper = new OpposingAccountMapper; + $mapper->setUser($this->user()); + $mapper->map(1, $amount, []); + } + + /** + * + * Should return account with given ID (which is of wrong account type). + * Will continue the search or store a revenue account with that name. + * + * @covers \FireflyIII\Support\Import\Routine\File\OpposingAccountMapper + */ + public function testAccountIdBadType(): void + { + $expected = $this->user()->accounts()->where('account_type_id', 5)->inRandomOrder()->first(); + $expected->iban = null; + $expected->save(); + $amount = '-12.34'; + $expectedArgs = [ + 'name' => $expected->name, + 'iban' => null, + 'accountNumber' => null, + 'account_type_id' => null, + 'accountType' => AccountType::EXPENSE, + 'active' => true, + 'BIC' => null, + ]; + $repository = $this->mock(AccountRepositoryInterface::class); + $repository->shouldReceive('setUser')->once(); + $repository->shouldReceive('findNull')->andReturn($expected)->once(); + $repository->shouldReceive('findByName')->withArgs([$expected->name, [AccountType::EXPENSE]])->andReturnNull(); + $repository->shouldReceive('findByName')->withArgs([$expected->name, [AccountType::ASSET]])->andReturnNull(); + $repository->shouldReceive('store')->withArgs([$expectedArgs])->once() + ->andReturn(new Account); + + + $mapper = new OpposingAccountMapper; + $mapper->setUser($this->user()); + $mapper->map(1, $amount, []); + } + + /** + * + * Should return account with given ID (which is of wrong account type). + * Will continue the search or store a revenue account with that name. + * + * @covers \FireflyIII\Support\Import\Routine\File\OpposingAccountMapper + */ + public function testAccountIdBadTypeIban(): void + { + $expected = $this->user()->accounts()->where('account_type_id', 5)->inRandomOrder()->first(); + $expected->iban = 'AD1200012030200359100100'; + $expected->save(); + + $amount = '-12.34'; + $expectedArgs = [ + 'name' => $expected->name, + 'iban' => $expected->iban, + 'accountNumber' => null, + 'account_type_id' => null, + 'accountType' => AccountType::EXPENSE, + 'active' => true, + 'BIC' => null, + ]; + $repository = $this->mock(AccountRepositoryInterface::class); + $repository->shouldReceive('setUser')->once(); + $repository->shouldReceive('findNull')->andReturn($expected)->once(); + $repository->shouldReceive('findByIbanNull')->withArgs([$expected->iban, [AccountType::EXPENSE]])->andReturnNull(); + $repository->shouldReceive('findByIbanNull')->withArgs([$expected->iban, [AccountType::ASSET]])->andReturnNull(); + $repository->shouldReceive('findByName')->withArgs([$expected->name, [AccountType::EXPENSE]])->andReturnNull(); + $repository->shouldReceive('findByName')->withArgs([$expected->name, [AccountType::ASSET]])->andReturnNull(); + $repository->shouldReceive('store')->withArgs([$expectedArgs])->once() + ->andReturn(new Account); + + + $mapper = new OpposingAccountMapper; + $mapper->setUser($this->user()); + $mapper->map(1, $amount, []); + } + + /** + * Amount = negative + * ID = null + * other data = null + * Should call store() with "(no name") for expense account + * + * @covers \FireflyIII\Support\Import\Routine\File\OpposingAccountMapper + */ + public function testBasic(): void + { + $amount = '-12.34'; + $expectedArgs = [ + 'name' => '(no name)', + 'iban' => null, + 'accountNumber' => null, + 'account_type_id' => null, + 'accountType' => AccountType::EXPENSE, + 'active' => true, + 'BIC' => null, + ]; + + $repository = $this->mock(AccountRepositoryInterface::class); + $repository->shouldReceive('setUser')->once(); + $repository->shouldReceive('store')->withArgs([$expectedArgs])->once() + ->andReturn(new Account); + + + $mapper = new OpposingAccountMapper; + $mapper->setUser($this->user()); + $mapper->map(null, $amount, []); + } + + /** + * Amount = negative + * ID = null + * other data = null + * Should call store() with "(no name") for expense account + * + * @covers \FireflyIII\Support\Import\Routine\File\OpposingAccountMapper + */ + public function testFindByAccountNumber(): void + { + $expected = $this->user()->accounts()->where('account_type_id', 4)->inRandomOrder()->first(); + $amount = '-12.34'; + $repository = $this->mock(AccountRepositoryInterface::class); + $repository->shouldReceive('setUser')->once(); + $repository->shouldReceive('findByAccountNumber')->withArgs(['12345', [AccountType::EXPENSE]]) + ->andReturn($expected)->once(); + + + $mapper = new OpposingAccountMapper; + $mapper->setUser($this->user()); + $result = $mapper->map(null, $amount, ['number' => '12345']); + $this->assertEquals($result->id, $expected->id); + } + + /** + * Amount = positive + * ID = null + * other data = null + * Should call store() with "(no name") for revenue account + * + * @covers \FireflyIII\Support\Import\Routine\File\OpposingAccountMapper + */ + public function testBasicPos(): void + { + $amount = '12.34'; + $expectedArgs = [ + 'name' => '(no name)', + 'iban' => null, + 'accountNumber' => null, + 'account_type_id' => null, + 'accountType' => AccountType::REVENUE, + 'active' => true, + 'BIC' => null, + ]; + + $repository = $this->mock(AccountRepositoryInterface::class); + $repository->shouldReceive('setUser')->once(); + $repository->shouldReceive('store')->withArgs([$expectedArgs])->once() + ->andReturn(new Account); + + + $mapper = new OpposingAccountMapper; + $mapper->setUser($this->user()); + $mapper->map(null, $amount, []); + } } \ No newline at end of file