From 1e7e0facf1e748436970a2469d0d911f3711328d Mon Sep 17 00:00:00 2001 From: James Cole Date: Mon, 22 Jul 2019 19:23:14 +0200 Subject: [PATCH] Improve test coverage. --- app/Http/Controllers/TagController.php | 8 +- .../Http/Controllers/PeriodOverview.php | 84 +++++---- resources/views/v1/tags/show.twig | 10 +- .../Feature/Controllers/TagControllerTest.php | 167 ++++++++++-------- 4 files changed, 149 insertions(+), 120 deletions(-) diff --git a/app/Http/Controllers/TagController.php b/app/Http/Controllers/TagController.php index f56d3bdcf4..0a1ad7a732 100644 --- a/app/Http/Controllers/TagController.php +++ b/app/Http/Controllers/TagController.php @@ -192,7 +192,11 @@ class TagController extends Controller 'firefly.journals_in_period_for_tag', ['tag' => $tag->tag, 'start' => $start->formatLocalized($this->monthAndDayFormat), 'end' => $end->formatLocalized($this->monthAndDayFormat),] ); - $periods = $this->getTagPeriodOverview($tag, $start); + + $startPeriod = $this->repository->firstUseDate($tag); + $startPeriod = $startPeriod ?? new Carbon; + $endPeriod = clone $end; + $periods = $this->getTagPeriodOverview($tag, $startPeriod, $endPeriod); $path = route('tags.show', [$tag->id, $start->format('Y-m-d'), $end->format('Y-m-d')]); /** @var GroupCollectorInterface $collector */ @@ -224,7 +228,7 @@ class TagController extends Controller $subTitleIcon = 'fa-tag'; $page = (int)$request->get('page'); $pageSize = (int)app('preferences')->get('listPageSize', 50)->data; - $periods = new Collection; + $periods = []; $subTitle = (string)trans('firefly.all_journals_for_tag', ['tag' => $tag->tag]); $start = $this->repository->firstUseDate($tag) ?? new Carbon; $end = new Carbon; diff --git a/app/Support/Http/Controllers/PeriodOverview.php b/app/Support/Http/Controllers/PeriodOverview.php index fb842b02cf..af63e4dc07 100644 --- a/app/Support/Http/Controllers/PeriodOverview.php +++ b/app/Support/Http/Controllers/PeriodOverview.php @@ -30,7 +30,6 @@ use FireflyIII\Models\Category; use FireflyIII\Models\Tag; use FireflyIII\Models\Transaction; use FireflyIII\Models\TransactionType; -use FireflyIII\Repositories\Tag\TagRepositoryInterface; use FireflyIII\Support\CacheProperties; use Illuminate\Support\Collection; use Log; @@ -361,57 +360,68 @@ trait PeriodOverview * * @param Carbon $date * - * @return Collection + * @return array */ - protected function getTagPeriodOverview(Tag $tag, Carbon $date): Collection // period overview for tags. + protected function getTagPeriodOverview(Tag $tag, Carbon $start, Carbon $end): array // period overview for tags. { - die('not yet complete'); - /** @var TagRepositoryInterface $repository */ - $repository = app(TagRepositoryInterface::class); - $range = app('preferences')->get('viewRange', '1M')->data; - /** @var Carbon $end */ - $start = clone $date; - $end = $repository->firstUseDate($tag) ?? new Carbon; + + $range = app('preferences')->get('viewRange', '1M')->data; [$start, $end] = $end < $start ? [$end, $start] : [$start, $end]; - // properties for entries with their amounts. + // properties for cache $cache = new CacheProperties; $cache->addProperty($start); $cache->addProperty($end); $cache->addProperty('tag-period-entries'); $cache->addProperty($tag->id); - if ($cache->has()) { - return $cache->get(); // @codeCoverageIgnore + // return $cache->get(); // @codeCoverageIgnore } - /** @var array $dates */ $dates = app('navigation')->blockPeriods($start, $end, $range); - $entries = new Collection; - // while end larger or equal to start + $entries = []; + + // collect all expenses in this period: + /** @var GroupCollectorInterface $collector */ + $collector = app(GroupCollectorInterface::class); + $collector->setTag($tag); + $collector->setRange($start, $end); + $collector->setTypes([TransactionType::DEPOSIT]); + $earnedSet = $collector->getExtractedJournals(); + + // collect all income in this period: + /** @var GroupCollectorInterface $collector */ + $collector = app(GroupCollectorInterface::class); + $collector->setTag($tag); + $collector->setRange($start, $end); + $collector->setTypes([TransactionType::WITHDRAWAL]); + $spentSet = $collector->getExtractedJournals(); + + // collect all transfers in this period: + /** @var GroupCollectorInterface $collector */ + $collector = app(GroupCollectorInterface::class); + $collector->setTag($tag); + $collector->setRange($start, $end); + $collector->setTypes([TransactionType::TRANSFER]); + $transferSet = $collector->getExtractedJournals(); + foreach ($dates as $currentDate) { - - $spentSet = $repository->expenseInPeriod($tag, $currentDate['start'], $currentDate['end']); - $spent = $this->groupByCurrency($spentSet); - $earnedSet = $repository->incomeInPeriod($tag, $currentDate['start'], $currentDate['end']); - $earned = $this->groupByCurrency($earnedSet); - $transferredSet = $repository->transferredInPeriod($tag, $currentDate['start'], $currentDate['end']); - $transferred = $this->groupByCurrency($transferredSet); - $title = app('navigation')->periodShow($currentDate['end'], $currentDate['period']); - - $entries->push( + $spent = $this->filterJournalsByDate($spentSet, $currentDate['start'], $currentDate['end']); + $earned = $this->filterJournalsByDate($earnedSet, $currentDate['start'], $currentDate['end']); + $transferred = $this->filterJournalsByDate($transferSet, $currentDate['start'], $currentDate['end']); + $title = app('navigation')->periodShow($currentDate['end'], $currentDate['period']); + $entries[] = [ - 'transactions' => count($spentSet) + count($earnedSet) + count($transferredSet), - 'title' => $title, - 'spent' => $spent, - 'earned' => $earned, - 'transferred' => $transferred, - 'route' => route('tags.show', [$tag->id, $currentDate['start']->format('Y-m-d'), $currentDate['end']->format('Y-m-d')]), - ] - ); - + 'transactions' => 0, + 'title' => $title, + 'route' => route('tags.show', + [$tag->id, $currentDate['start']->format('Y-m-d'), $currentDate['end']->format('Y-m-d')]), + 'total_transactions' => count($spent) + count($earned) + count($transferred), + 'spent' => $this->groupByCurrency($spent), + 'earned' => $this->groupByCurrency($earned), + 'transferred' => $this->groupByCurrency($transferred), + ]; } - $cache->store($entries); return $entries; } @@ -435,7 +445,7 @@ trait PeriodOverview $cache->addProperty('transactions-period-entries'); $cache->addProperty($transactionType); if ($cache->has()) { - // return $cache->get(); // @codeCoverageIgnore + return $cache->get(); // @codeCoverageIgnore } /** @var array $dates */ $dates = app('navigation')->blockPeriods($start, $end, $range); diff --git a/resources/views/v1/tags/show.twig b/resources/views/v1/tags/show.twig index b39ccca8d9..f55ef60956 100644 --- a/resources/views/v1/tags/show.twig +++ b/resources/views/v1/tags/show.twig @@ -105,7 +105,7 @@ - {% if periods.count > 0 %} + {% if periods|length > 0 %}

{{ 'showEverything'|_ }}

@@ -113,7 +113,7 @@
{% endif %}
-
+

{{ 'transactions'|_ }}

@@ -134,7 +134,7 @@
{% include 'list/groups' %} - {% if periods.count > 0 %} + {% if periods|length > 0 %}

@@ -152,13 +152,13 @@

- {% if periods.count > 0 %} + {% if periods|length > 0 %}
{% include 'list.periods' %}
{% endif %}
- {% if periods.count > 0 %} + {% if periods|length > 0 %}

{{ 'showEverything'|_ }}

diff --git a/tests/Feature/Controllers/TagControllerTest.php b/tests/Feature/Controllers/TagControllerTest.php index eb50e1caa9..02242aacfc 100644 --- a/tests/Feature/Controllers/TagControllerTest.php +++ b/tests/Feature/Controllers/TagControllerTest.php @@ -22,13 +22,12 @@ declare(strict_types=1); namespace Tests\Feature\Controllers; +use Amount; use Carbon\Carbon; - use FireflyIII\Helpers\Collector\GroupCollectorInterface; use FireflyIII\Helpers\Fiscal\FiscalHelperInterface; use FireflyIII\Models\Preference; use FireflyIII\Models\Tag; -use FireflyIII\Models\TransactionJournal; use FireflyIII\Models\TransactionType; use FireflyIII\Repositories\Journal\JournalRepositoryInterface; use FireflyIII\Repositories\Tag\TagRepositoryInterface; @@ -129,7 +128,6 @@ class TagControllerTest extends TestCase } /** - * @covers \FireflyIII\Http\Controllers\TagController * @covers \FireflyIII\Http\Controllers\TagController */ public function testIndex(): void @@ -153,7 +151,6 @@ class TagControllerTest extends TestCase } /** - * @covers \FireflyIII\Http\Controllers\TagController * @covers \FireflyIII\Http\Controllers\TagController */ public function testShow(): void @@ -171,28 +168,36 @@ class TagControllerTest extends TestCase Preferences::shouldReceive('get')->withArgs(['listPageSize', 50])->atLeast()->once()->andReturn($pref); // mock stuff - $repository = $this->mock(TagRepositoryInterface::class); - $collector = $this->mock(GroupCollectorInterface::class); - $userRepos = $this->mock(UserRepositoryInterface::class); + $group = $this->getRandomWithdrawalGroup(); + $paginator = new LengthAwarePaginator([$group], 1, 40, 1); + $repository = $this->mock(TagRepositoryInterface::class); + $collector = $this->mock(GroupCollectorInterface::class); + $userRepos = $this->mock(UserRepositoryInterface::class); $userRepos->shouldReceive('hasRole')->withArgs([Mockery::any(), 'owner'])->atLeast()->once()->andReturn(true); + //Preferences::shouldReceive('mark')->atLeast()->once(); + Preferences::shouldReceive('lastActivity')->atLeast()->once()->andReturn('md512345'); + $repository->shouldReceive('firstUseDate')->andReturn(new Carbon)->once(); $repository->shouldReceive('sumsOfTag')->andReturn($amounts)->once(); - $repository->shouldReceive('expenseInPeriod')->andReturn(new Collection)->atLeast()->times(1); - $repository->shouldReceive('incomeInPeriod')->andReturn(new Collection)->atLeast()->times(1); - $repository->shouldReceive('transferredInPeriod')->andReturn(new Collection)->atLeast()->times(1); + //$repository->shouldReceive('expenseInPeriod')->andReturn(new Collection)->atLeast()->times(1); + //$repository->shouldReceive('incomeInPeriod')->andReturn(new Collection)->atLeast()->times(1); + //$repository->shouldReceive('transferredInPeriod')->andReturn(new Collection)->atLeast()->times(1); - $collector->shouldReceive('removeFilter')->andReturnSelf()->once(); - $collector->shouldReceive('setAllAssetAccounts')->andReturnSelf()->once(); - $collector->shouldReceive('setLimit')->andReturnSelf()->once(); - $collector->shouldReceive('setPage')->andReturnSelf()->once(); - $collector->shouldReceive('setTag')->andReturnSelf()->once(); - $collector->shouldReceive('withOpposingAccount')->andReturnSelf()->once(); - $collector->shouldReceive('withBudgetInformation')->andReturnSelf()->once(); - $collector->shouldReceive('withCategoryInformation')->andReturnSelf()->once(); - $collector->shouldReceive('setRange')->andReturnSelf()->once(); - $collector->shouldReceive('getPaginatedTransactions')->andReturn(new LengthAwarePaginator([], 0, 10))->once(); + $collector->shouldReceive('setTypes')->andReturnSelf()->atLeast()->once(); + $collector->shouldReceive('setLimit')->andReturnSelf()->atLeast()->once(); + $collector->shouldReceive('setPage')->andReturnSelf()->atLeast()->once(); + $collector->shouldReceive('setTag')->andReturnSelf()->atLeast()->once(); + $collector->shouldReceive('withBudgetInformation')->andReturnSelf()->atLeast()->once(); + $collector->shouldReceive('withCategoryInformation')->andReturnSelf()->atLeast()->once(); + $collector->shouldReceive('withAccountInformation')->andReturnSelf()->atLeast()->once(); + $collector->shouldReceive('getPaginatedGroups')->andReturn($paginator)->atLeast()->once(); + + Amount::shouldReceive('formatAnything')->atLeast()->once()->andReturn('x'); + + $collector->shouldReceive('setRange')->andReturnSelf()->atLeast()->once(); + $collector->shouldReceive('getExtractedJournals')->atLeast()->once()->andReturn([]); $this->be($this->user()); $response = $this->get(route('tags.show', [1])); @@ -207,34 +212,39 @@ class TagControllerTest extends TestCase { $this->mockDefaultSession(); - // mock stuff - $repository = $this->mock(TagRepositoryInterface::class); - $collector = $this->mock(TransactionCollectorInterface::class); - $journalRepos = $this->mock(JournalRepositoryInterface::class); - $userRepos = $this->mock(UserRepositoryInterface::class); - $userRepos->shouldReceive('hasRole')->withArgs([Mockery::any(), 'owner'])->atLeast()->once()->andReturn(true); - - - $repository->shouldReceive('firstUseDate')->andReturn(new Carbon)->once(); - - $collector->shouldReceive('removeFilter')->andReturnSelf()->once(); - $collector->shouldReceive('setAllAssetAccounts')->andReturnSelf()->once(); - $collector->shouldReceive('setLimit')->andReturnSelf()->once(); - $collector->shouldReceive('setPage')->andReturnSelf()->once(); - $collector->shouldReceive('setTag')->andReturnSelf()->once(); - $collector->shouldReceive('withOpposingAccount')->andReturnSelf()->once(); - $collector->shouldReceive('withBudgetInformation')->andReturnSelf()->once(); - $collector->shouldReceive('withCategoryInformation')->andReturnSelf()->once(); - $collector->shouldReceive('setRange')->andReturnSelf()->once(); - $collector->shouldReceive('getPaginatedTransactions')->andReturn(new LengthAwarePaginator([], 0, 10))->once(); - $amounts = [ TransactionType::WITHDRAWAL => '0', TransactionType::TRANSFER => '0', TransactionType::DEPOSIT => '0', ]; + + $pref = new Preference; + $pref->data = 50; + Preferences::shouldReceive('get')->withArgs(['listPageSize', 50])->atLeast()->once()->andReturn($pref); + + // mock stuff + $group = $this->getRandomWithdrawalGroup(); + $paginator = new LengthAwarePaginator([$group], 1, 40, 1); + $repository = $this->mock(TagRepositoryInterface::class); + $collector = $this->mock(GroupCollectorInterface::class); + $userRepos = $this->mock(UserRepositoryInterface::class); + $userRepos->shouldReceive('hasRole')->withArgs([Mockery::any(), 'owner'])->atLeast()->once()->andReturn(true); + + $repository->shouldReceive('firstUseDate')->andReturn(new Carbon)->once(); $repository->shouldReceive('sumsOfTag')->andReturn($amounts)->once(); + $collector->shouldReceive('setLimit')->andReturnSelf()->atLeast()->once(); + $collector->shouldReceive('setPage')->andReturnSelf()->atLeast()->once(); + $collector->shouldReceive('setTag')->andReturnSelf()->atLeast()->once(); + $collector->shouldReceive('withBudgetInformation')->andReturnSelf()->atLeast()->once(); + $collector->shouldReceive('withCategoryInformation')->andReturnSelf()->atLeast()->once(); + $collector->shouldReceive('withAccountInformation')->andReturnSelf()->atLeast()->once(); + $collector->shouldReceive('getPaginatedGroups')->andReturn($paginator)->atLeast()->once(); + + Amount::shouldReceive('formatAnything')->atLeast()->once()->andReturn('x'); + + $collector->shouldReceive('setRange')->andReturnSelf()->atLeast()->once(); + $this->be($this->user()); $response = $this->get(route('tags.show', [1, 'all'])); $response->assertStatus(200); @@ -248,43 +258,50 @@ class TagControllerTest extends TestCase { $this->mockDefaultSession(); - // mock stuff - $repository = $this->mock(TagRepositoryInterface::class); - $collector = $this->mock(TransactionCollectorInterface::class); - $journalRepos = $this->mock(JournalRepositoryInterface::class); - $userRepos = $this->mock(UserRepositoryInterface::class); - $fiscalHelper = $this->mock(FiscalHelperInterface::class); - $date = new Carbon; - $fiscalHelper->shouldReceive('endOfFiscalYear')->atLeast()->once()->andReturn($date); - $fiscalHelper->shouldReceive('startOfFiscalYear')->atLeast()->once()->andReturn($date); - - $userRepos->shouldReceive('hasRole')->withArgs([Mockery::any(), 'owner'])->atLeast()->once()->andReturn(true); - - $repository->shouldReceive('firstUseDate')->andReturn(new Carbon)->once(); - - $repository->shouldReceive('expenseInPeriod')->andReturn(new Collection)->atLeast()->times(1); - $repository->shouldReceive('incomeInPeriod')->andReturn(new Collection)->atLeast()->times(1); - $repository->shouldReceive('transferredInPeriod')->andReturn(new Collection)->atLeast()->times(1); - - - $collector->shouldReceive('removeFilter')->andReturnSelf()->once(); - $collector->shouldReceive('setAllAssetAccounts')->andReturnSelf()->once(); - $collector->shouldReceive('setLimit')->andReturnSelf()->once(); - $collector->shouldReceive('setPage')->andReturnSelf()->once(); - $collector->shouldReceive('setTag')->andReturnSelf()->once(); - $collector->shouldReceive('withOpposingAccount')->andReturnSelf()->once(); - $collector->shouldReceive('withBudgetInformation')->andReturnSelf()->once(); - $collector->shouldReceive('withCategoryInformation')->andReturnSelf()->once(); - $collector->shouldReceive('setRange')->andReturnSelf()->once(); - $collector->shouldReceive('getPaginatedTransactions')->andReturn(new LengthAwarePaginator([], 0, 10))->once(); - $amounts = [ TransactionType::WITHDRAWAL => '0', TransactionType::TRANSFER => '0', TransactionType::DEPOSIT => '0', ]; + + $pref = new Preference; + $pref->data = 50; + Preferences::shouldReceive('get')->withArgs(['listPageSize', 50])->atLeast()->once()->andReturn($pref); + + // mock stuff + $helper = $this->mock(FiscalHelperInterface::class); + + $helper->shouldReceive('startOfFiscalYear')->atLeast()->once()->andReturn(new Carbon); + $helper->shouldReceive('endOfFiscalYear')->atLeast()->once()->andReturn(new Carbon); + + $group = $this->getRandomWithdrawalGroup(); + $paginator = new LengthAwarePaginator([$group], 1, 40, 1); + $repository = $this->mock(TagRepositoryInterface::class); + $collector = $this->mock(GroupCollectorInterface::class); + $userRepos = $this->mock(UserRepositoryInterface::class); + $userRepos->shouldReceive('hasRole')->withArgs([Mockery::any(), 'owner'])->atLeast()->once()->andReturn(true); + + //Preferences::shouldReceive('mark')->atLeast()->once(); + Preferences::shouldReceive('lastActivity')->atLeast()->once()->andReturn('md512345'); + + $repository->shouldReceive('firstUseDate')->andReturn(new Carbon)->once(); $repository->shouldReceive('sumsOfTag')->andReturn($amounts)->once(); + $collector->shouldReceive('setTypes')->andReturnSelf()->atLeast()->once(); + $collector->shouldReceive('setLimit')->andReturnSelf()->atLeast()->once(); + $collector->shouldReceive('setPage')->andReturnSelf()->atLeast()->once(); + $collector->shouldReceive('setTag')->andReturnSelf()->atLeast()->once(); + $collector->shouldReceive('withBudgetInformation')->andReturnSelf()->atLeast()->once(); + $collector->shouldReceive('withCategoryInformation')->andReturnSelf()->atLeast()->once(); + $collector->shouldReceive('withAccountInformation')->andReturnSelf()->atLeast()->once(); + $collector->shouldReceive('getPaginatedGroups')->andReturn($paginator)->atLeast()->once(); + + Amount::shouldReceive('formatAnything')->atLeast()->once()->andReturn('x'); + + $collector->shouldReceive('setRange')->andReturnSelf()->atLeast()->once(); + $collector->shouldReceive('getExtractedJournals')->atLeast()->once()->andReturn([]); + + $this->be($this->user()); $response = $this->get(route('tags.show', [1, '2016-01-01'])); $response->assertStatus(200); @@ -299,10 +316,8 @@ class TagControllerTest extends TestCase { $this->mockDefaultSession(); $repository = $this->mock(TagRepositoryInterface::class); - $journalRepos = $this->mock(JournalRepositoryInterface::class); - $userRepos = $this->mock(UserRepositoryInterface::class); + Preferences::shouldReceive('mark')->atLeast()->once(); - $repository->shouldReceive('findNull')->andReturn(null); $repository->shouldReceive('store')->andReturn(new Tag); @@ -327,9 +342,9 @@ class TagControllerTest extends TestCase { $this->mockDefaultSession(); $repository = $this->mock(TagRepositoryInterface::class); - $journalRepos = $this->mock(JournalRepositoryInterface::class); $userRepos = $this->mock(UserRepositoryInterface::class); - + Preferences::shouldReceive('mark')->atLeast()->once(); + $this->session(['tags.edit.uri' => 'http://localhost']); $data = [