From efeffaa49f08999b58669fed1c900841ac5aba56 Mon Sep 17 00:00:00 2001 From: James Cole Date: Mon, 10 Sep 2018 20:24:19 +0200 Subject: [PATCH] Refactor period blocks. --- .../Controllers/Budget/ShowController.php | 2 +- .../Controllers/Category/ShowController.php | 2 +- .../Category/CategoryRepository.php | 60 +++++-- .../Category/CategoryRepositoryInterface.php | 22 +++ .../Http/Controllers/PeriodOverview.php | 153 ++++++++++-------- app/Support/Navigation.php | 2 +- resources/views/accounts/show.twig | 27 +--- resources/views/budgets/no-budget.twig | 23 +-- resources/views/categories/show.twig | 39 +---- resources/views/list/periods.twig | 44 +++++ 10 files changed, 207 insertions(+), 167 deletions(-) create mode 100644 resources/views/list/periods.twig diff --git a/app/Http/Controllers/Budget/ShowController.php b/app/Http/Controllers/Budget/ShowController.php index 7b0e62bffd..4347702490 100644 --- a/app/Http/Controllers/Budget/ShowController.php +++ b/app/Http/Controllers/Budget/ShowController.php @@ -88,7 +88,7 @@ class ShowController extends Controller 'firefly.without_budget_between', ['start' => $start->formatLocalized($this->monthAndDayFormat), 'end' => $end->formatLocalized($this->monthAndDayFormat)] ); - $periods = $this->getBudgetPeriodOverview(); + $periods = $this->getBudgetPeriodOverview($end); $page = (int)$request->get('page'); $pageSize = (int)app('preferences')->get('listPageSize', 50)->data; diff --git a/app/Http/Controllers/Category/ShowController.php b/app/Http/Controllers/Category/ShowController.php index 42826046e4..7f2c3f3a50 100644 --- a/app/Http/Controllers/Category/ShowController.php +++ b/app/Http/Controllers/Category/ShowController.php @@ -94,7 +94,7 @@ class ShowController extends Controller $subTitleIcon = 'fa-bar-chart'; $page = (int)$request->get('page'); $pageSize = (int)app('preferences')->get('listPageSize', 50)->data; - $periods = $this->getCategoryPeriodOverview($category, $start); + $periods = $this->getCategoryPeriodOverview($category, $end); $path = route('categories.show', [$category->id, $start->format('Y-m-d'), $end->format('Y-m-d')]); $subTitle = trans( 'firefly.journals_in_period_for_category', diff --git a/app/Repositories/Category/CategoryRepository.php b/app/Repositories/Category/CategoryRepository.php index da7dee7d10..317c936f7b 100644 --- a/app/Repositories/Category/CategoryRepository.php +++ b/app/Repositories/Category/CategoryRepository.php @@ -83,14 +83,37 @@ class CategoryRepository implements CategoryRepositoryInterface * @return string */ public function earnedInPeriod(Collection $categories, Collection $accounts, Carbon $start, Carbon $end): string + { + $set = $this->earnedInPeriodCollection($categories, $accounts, $start, $end); + + return (string)$set->sum('transaction_amount'); + } + + /** @noinspection MoreThanThreeArgumentsInspection */ + /** + * @param Collection $categories + * @param Collection $accounts + * @param Carbon $start + * @param Carbon $end + * + * @return Collection + */ + public function earnedInPeriodCollection(Collection $categories, Collection $accounts, Carbon $start, Carbon $end): Collection { /** @var TransactionCollectorInterface $collector */ $collector = app(TransactionCollectorInterface::class); $collector->setUser($this->user); - $collector->setRange($start, $end)->setTypes([TransactionType::DEPOSIT])->setAccounts($accounts)->setCategories($categories); - $set = $collector->getTransactions(); + if (0 !== $accounts->count()) { + $collector->setAccounts($accounts); + } - return (string)$set->sum('transaction_amount'); + if (0 === $accounts->count()) { + $collector->setAllAssetAccounts(); + } + + $collector->setRange($start, $end)->setTypes([TransactionType::DEPOSIT])->setCategories($categories); + + return $collector->getTransactions(); } /** @@ -182,6 +205,8 @@ class CategoryRepository implements CategoryRepositoryInterface return $set; } + /** @noinspection MoreThanThreeArgumentsInspection */ + /** * @param Category $category * @param Collection $accounts @@ -212,7 +237,6 @@ class CategoryRepository implements CategoryRepositoryInterface return $lastJournalDate; } - /** @noinspection MoreThanThreeArgumentsInspection */ /** * @param Collection $categories * @param Collection $accounts @@ -258,6 +282,8 @@ class CategoryRepository implements CategoryRepositoryInterface return $data; } + /** @noinspection MoreThanThreeArgumentsInspection */ + /** * @param Collection $accounts * @param Carbon $start @@ -296,7 +322,6 @@ class CategoryRepository implements CategoryRepositoryInterface return $result; } - /** @noinspection MoreThanThreeArgumentsInspection */ /** * @param Collection $categories * @param Collection $accounts @@ -383,6 +408,8 @@ class CategoryRepository implements CategoryRepositoryInterface return $result; } + /** @noinspection MoreThanThreeArgumentsInspection */ + /** * @param User $user */ @@ -401,6 +428,23 @@ class CategoryRepository implements CategoryRepositoryInterface * @return string */ public function spentInPeriod(Collection $categories, Collection $accounts, Carbon $start, Carbon $end): string + { + $set = $this->spentInPeriodCollection($categories, $accounts, $start, $end); + + + return (string)$set->sum('transaction_amount'); + } + + /** @noinspection MoreThanThreeArgumentsInspection */ + /** + * @param Collection $categories + * @param Collection $accounts + * @param Carbon $start + * @param Carbon $end + * + * @return Collection + */ + public function spentInPeriodCollection(Collection $categories, Collection $accounts, Carbon $start, Carbon $end): Collection { /** @var TransactionCollectorInterface $collector */ $collector = app(TransactionCollectorInterface::class); @@ -414,13 +458,9 @@ class CategoryRepository implements CategoryRepositoryInterface $collector->setAllAssetAccounts(); } - $set = $collector->getTransactions(); - - return (string)$set->sum('transaction_amount'); + return $collector->getTransactions(); } - /** @noinspection MoreThanThreeArgumentsInspection */ - /** * A very cryptic method name that means: * diff --git a/app/Repositories/Category/CategoryRepositoryInterface.php b/app/Repositories/Category/CategoryRepositoryInterface.php index 5431df23c7..c4944fe8ba 100644 --- a/app/Repositories/Category/CategoryRepositoryInterface.php +++ b/app/Repositories/Category/CategoryRepositoryInterface.php @@ -51,6 +51,17 @@ interface CategoryRepositoryInterface */ public function earnedInPeriod(Collection $categories, Collection $accounts, Carbon $start, Carbon $end): string; + /** @noinspection MoreThanThreeArgumentsInspection */ + /** + * @param Collection $categories + * @param Collection $accounts + * @param Carbon $start + * @param Carbon $end + * + * @return Collection + */ + public function earnedInPeriodCollection(Collection $categories, Collection $accounts, Carbon $start, Carbon $end): Collection; + /** * Find a category. * @@ -158,6 +169,17 @@ interface CategoryRepositoryInterface */ public function spentInPeriod(Collection $categories, Collection $accounts, Carbon $start, Carbon $end): string; + /** @noinspection MoreThanThreeArgumentsInspection */ + /** + * @param Collection $categories + * @param Collection $accounts + * @param Carbon $start + * @param Carbon $end + * + * @return Collection + */ + public function spentInPeriodCollection(Collection $categories, Collection $accounts, Carbon $start, Carbon $end): Collection; + /** @noinspection MoreThanThreeArgumentsInspection */ /** diff --git a/app/Support/Http/Controllers/PeriodOverview.php b/app/Support/Http/Controllers/PeriodOverview.php index c0b13e0b7b..93e302af63 100644 --- a/app/Support/Http/Controllers/PeriodOverview.php +++ b/app/Support/Http/Controllers/PeriodOverview.php @@ -27,10 +27,10 @@ use Carbon\Carbon; use FireflyIII\Helpers\Collector\TransactionCollectorInterface; use FireflyIII\Helpers\Filter\InternalTransferFilter; use FireflyIII\Models\Account; -use FireflyIII\Models\AccountType; use FireflyIII\Models\Category; use FireflyIII\Models\Tag; use FireflyIII\Models\Transaction; +use FireflyIII\Models\TransactionCurrency; use FireflyIII\Models\TransactionType; use FireflyIII\Repositories\Account\AccountRepositoryInterface; use FireflyIII\Repositories\Category\CategoryRepositoryInterface; @@ -42,18 +42,14 @@ use Illuminate\Support\Collection; /** * Trait PeriodOverview. * - * General working of an overview thing: - * - Take a start date (session or view, depends on argument). - * - Take end date. This is usually the very first object related to the period overview (first transaction in tag, etc). - * - Smart list of period, becoming larger in time: - * -- This year: months - * -- Last year: quarters - * -- Before that: years - * -- Before that: decennia - * * - Group expenses, income, etc. under this period. * - Returns collection of arrays. Possible fields are: - * - start (Carbon), end (Carbon), title (string), spent (string), earned (string), transferred (string) + * - start (string), + * end (string), + * title (string), + * spent (string), + * earned (string), + * transferred (string) * * */ @@ -64,20 +60,19 @@ trait PeriodOverview * and for each period, the amount of money spent and earned. This is a complex operation which is cached for * performance reasons. * - * TODO refactor me. - * - * @param Account $account the account involved - * @param Carbon|null $date + * @param Account $account the account involved + * @param Carbon $date * * @return Collection */ - protected function getAccountPeriodOverview(Account $account, ?Carbon $date): Collection // period overview + protected function getAccountPeriodOverview(Account $account, Carbon $date): Collection // period overview { /** @var AccountRepositoryInterface $repository */ $repository = app(AccountRepositoryInterface::class); $range = app('preferences')->get('viewRange', '1M')->data; - $start = $repository->oldestJournalDate($account) ?? Carbon::now()->startOfMonth(); - $end = $date ?? new Carbon; + $end = $repository->oldestJournalDate($account) ?? Carbon::now()->subMonth()->startOfMonth(); + $start = clone $date; + if ($end < $start) { [$start, $end] = [$end, $start]; // @codeCoverageIgnore } @@ -96,30 +91,31 @@ trait PeriodOverview $entries = new Collection; // loop dates foreach ($dates as $currentDate) { - - // try a collector for income: /** @var TransactionCollectorInterface $collector */ $collector = app(TransactionCollectorInterface::class); $collector->setAccounts(new Collection([$account]))->setRange($currentDate['start'], $currentDate['end'])->setTypes([TransactionType::DEPOSIT]) ->withOpposingAccount(); - $earned = (string)$collector->getTransactions()->sum('transaction_amount'); + $set = $collector->getTransactions(); + $earned = $this->groupByCurrency($set); - // try a collector for expenses: /** @var TransactionCollectorInterface $collector */ $collector = app(TransactionCollectorInterface::class); $collector->setAccounts(new Collection([$account]))->setRange($currentDate['start'], $currentDate['end'])->setTypes([TransactionType::WITHDRAWAL]) ->withOpposingAccount(); - $spent = (string)$collector->getTransactions()->sum('transaction_amount'); + $set = $collector->getTransactions(); + $spent = $this->groupByCurrency($set); - $dateName = app('navigation')->periodShow($currentDate['start'], $currentDate['period']); + $title = app('navigation')->periodShow($currentDate['start'], $currentDate['period']); /** @noinspection PhpUndefinedMethodInspection */ $entries->push( [ - 'name' => $dateName, - 'spent' => $spent, - 'earned' => $earned, - 'start' => $currentDate['start']->format('Y-m-d'), - 'end' => $currentDate['end']->format('Y-m-d'), + 'transactions' => 0, + 'title' => $title, + 'spent' => $spent, + 'earned' => $earned, + 'transferred' => '0', + 'route' => route('accounts.show', [$account->id, $currentDate['start']->format('Y-m-d'), $currentDate['end']->format('Y-m-d')]), + ] ); } @@ -132,19 +128,16 @@ trait PeriodOverview /** * Gets period overview used for budgets. * - * TODO refactor me. - * * @return Collection */ - protected function getBudgetPeriodOverview(): Collection + protected function getBudgetPeriodOverview(Carbon $date): Collection { /** @var JournalRepositoryInterface $repository */ $repository = app(JournalRepositoryInterface::class); $first = $repository->firstNull(); - $start = null === $first ? new Carbon : $first->date; + $end = null === $first ? new Carbon : $first->date; + $start = clone $date; $range = app('preferences')->get('viewRange', '1M')->data; - $start = app('navigation')->startOfPeriod($start, $range); - $end = app('navigation')->endOfX(new Carbon, $range, null); $entries = new Collection; $cache = new CacheProperties; $cache->addProperty($start); @@ -154,24 +147,27 @@ trait PeriodOverview if ($cache->has()) { return $cache->get(); // @codeCoverageIgnore } + + /** @var array $dates */ $dates = app('navigation')->blockPeriods($start, $end, $range); - foreach ($dates as $date) { + foreach ($dates as $currentDate) { /** @var TransactionCollectorInterface $collector */ $collector = app(TransactionCollectorInterface::class); - $collector->setAllAssetAccounts()->setRange($date['start'], $date['end'])->withoutBudget()->withOpposingAccount()->setTypes( + $collector->setAllAssetAccounts()->setRange($currentDate['start'], $currentDate['end'])->withoutBudget()->withOpposingAccount()->setTypes( [TransactionType::WITHDRAWAL] ); - $set = $collector->getTransactions(); - $sum = (string)($set->sum('transaction_amount') ?? '0'); - $journals = $set->count(); - /** @noinspection PhpUndefinedMethodInspection */ - $dateStr = $date['end']->format('Y-m-d'); - $dateName = app('navigation')->periodShow($date['end'], $date['period']); + $set = $collector->getTransactions(); + $count = $set->count(); + $spent = $this->groupByCurrency($set); + $title = app('navigation')->periodShow($currentDate['end'], $currentDate['period']); $entries->push( - ['string' => $dateStr, 'name' => $dateName, 'count' => $journals, 'sum' => $sum, 'date' => clone $date['end'], - 'start' => $date['start'], - 'end' => $date['end'], - + [ + 'route' => route('budgets.no-budget', [$currentDate['start']->format('Y-m-d'), $currentDate['end']->format('Y-m-d')]), + 'transactions' => $count, + 'title' => $title, + 'spent' => $spent, + 'earned' => '0', + 'transferred' => '0', ] ); } @@ -194,15 +190,14 @@ trait PeriodOverview { /** @var JournalRepositoryInterface $journalRepository */ $journalRepository = app(JournalRepositoryInterface::class); - /** @var AccountRepositoryInterface $accountRepository */ - $accountRepository = app(AccountRepositoryInterface::class); /** @var CategoryRepositoryInterface $categoryRepository */ $categoryRepository = app(CategoryRepositoryInterface::class); - $range = app('preferences')->get('viewRange', '1M')->data; - $first = $journalRepository->firstNull(); - $start = null === $first ? new Carbon : $first->date; - $end = $date ?? new Carbon; - $accounts = $accountRepository->getAccountsByType([AccountType::DEFAULT, AccountType::ASSET]); + + + $range = app('preferences')->get('viewRange', '1M')->data; + $first = $journalRepository->firstNull(); + $end = null === $first ? new Carbon : $first->date; + $start = clone $date; // properties for entries with their amounts. $cache = new CacheProperties(); @@ -213,37 +208,32 @@ trait PeriodOverview $cache->addProperty($category->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; foreach ($dates as $currentDate) { - $spent = $categoryRepository->spentInPeriod(new Collection([$category]), $accounts, $currentDate['start'], $currentDate['end']); - $earned = $categoryRepository->earnedInPeriod(new Collection([$category]), $accounts, $currentDate['start'], $currentDate['end']); - /** @noinspection PhpUndefinedMethodInspection */ - $dateStr = $currentDate['end']->format('Y-m-d'); - $dateName = app('navigation')->periodShow($currentDate['end'], $currentDate['period']); - + $spent = $categoryRepository->spentInPeriodCollection(new Collection([$category]), new Collection, $currentDate['start'], $currentDate['end']); + $earned = $categoryRepository->earnedInPeriodCollection(new Collection([$category]), new Collection, $currentDate['start'], $currentDate['end']); + $spent = $this->groupByCurrency($spent); + $earned = $this->groupByCurrency($earned); // amount transferred /** @var TransactionCollectorInterface $collector */ $collector = app(TransactionCollectorInterface::class); $collector->setAllAssetAccounts()->setRange($currentDate['start'], $currentDate['end'])->setCategory($category) ->withOpposingAccount()->setTypes([TransactionType::TRANSFER]); $collector->removeFilter(InternalTransferFilter::class); - $transferred = app('steam')->positive((string)$collector->getTransactions()->sum('transaction_amount')); - + $transferred = $this->groupByCurrency($collector->getTransactions()); + $title = app('navigation')->periodShow($currentDate['end'], $currentDate['period']); $entries->push( [ - 'string' => $dateStr, - 'name' => $dateName, + 'route' => route('categories.show', [$category->id, $currentDate['start']->format('Y-m-d'), $currentDate['end']->format('Y-m-d')]), + 'title' => $title, 'spent' => $spent, 'earned' => $earned, - 'sum' => bcadd($earned, $spent), 'transferred' => $transferred, - 'start' => clone $currentDate['start'], - 'end' => clone $currentDate['end'], ] ); } @@ -405,4 +395,31 @@ trait PeriodOverview return $return; } + /** + * @param Collection $transactions + * + * @return array + */ + private function groupByCurrency(Collection $transactions): array + { + $return = []; + /** @var Transaction $transaction */ + foreach ($transactions as $transaction) { + $currencyId = (int)$transaction->transaction_currency_id; + if (!isset($return[$currencyId])) { + $currency = new TransactionCurrency; + $currency->symbol = $transaction->transaction_currency_symbol; + $currency->decimal_places = $transaction->transaction_currency_dp; + $currency->name = $transaction->transaction_currency_name; + $return[$currencyId] = [ + 'amount' => '0', + 'currency' => $currency, + ]; + } + $return[$currencyId]['amount'] = bcadd($return[$currencyId]['amount'], $transaction->transaction_amount); + } + + return $return; + } + } \ No newline at end of file diff --git a/app/Support/Navigation.php b/app/Support/Navigation.php index 4317c34b45..bef0a248bf 100644 --- a/app/Support/Navigation.php +++ b/app/Support/Navigation.php @@ -96,7 +96,7 @@ class Navigation } $periods = []; /* - * Start looping per months for 1 year + the rest of the year: + * Start looping per month for 1 year + the rest of the year: */ $perMonthEnd = clone $end; $perMonthStart = clone $end; diff --git a/resources/views/accounts/show.twig b/resources/views/accounts/show.twig index 6508819793..7eef0c7a70 100644 --- a/resources/views/accounts/show.twig +++ b/resources/views/accounts/show.twig @@ -134,32 +134,7 @@ {% if periods.count > 0 %}
- {% for period in periods %} - {% if (period.spent != 0 or period.earned != 0) %} -
-
-

{{ period.name }} -

-
-
- - {% if period.spent != 0 %} - - - - - {% endif %} - {% if period.earned != 0 %} - - - - - {% endif %} -
{{ 'spent'|_ }}{{ formatAmountByCurrency(currency, period.spent) }}
{{ 'earned'|_ }}{{ formatAmountByCurrency(currency, period.earned) }}
-
-
- {% endif %} - {% endfor %} + {% include 'list.periods' %}
{% endif %} diff --git a/resources/views/budgets/no-budget.twig b/resources/views/budgets/no-budget.twig index 632f6f1368..d76dc02034 100644 --- a/resources/views/budgets/no-budget.twig +++ b/resources/views/budgets/no-budget.twig @@ -42,28 +42,7 @@ {% if periods.count > 0 %}
- {% for period in periods %} - {% if period.count > 0 %} -
-
-

{{ period.name }} -

-
-
- - - - - - - - - -
{{ 'transactions'|_ }}{{ period.count }}
{{ 'spent'|_ }}{{ period.sum|formatAmount }}
-
-
- {% endif %} - {% endfor %} + {% include 'list.periods' %}
{% endif %} diff --git a/resources/views/categories/show.twig b/resources/views/categories/show.twig index 09d6992601..568e8532c0 100644 --- a/resources/views/categories/show.twig +++ b/resources/views/categories/show.twig @@ -87,44 +87,7 @@ {% if periods.count > 0 %}
- {% for period in periods %} - {% if period.spent != 0 or period.earned != 0 or period.sum != 0 %} -
-
-

{{ period.name }} -

-
-
- - {% if period.spent != 0 %} - - - - - {% endif %} - {% if period.earned != 0 %} - - - - - {% endif %} - {% if period.earned != 0 and period.spent != 0 %} - - - - - {% endif %} - {% if period.transferred != 0 %} - - - - - {% endif %} -
{{ 'spent'|_ }}{{ period.spent|formatAmount }}
{{ 'earned'|_ }}{{ period.earned|formatAmount }}
{{ 'sum'|_ }}{{ period.sum|formatAmount }}
{{ 'transferred'|_ }}{{ period.transferred|formatAmountPlain }}
-
-
- {% endif %} - {% endfor %} + {% include 'list.periods' %}
{% endif %} diff --git a/resources/views/list/periods.twig b/resources/views/list/periods.twig new file mode 100644 index 0000000000..7e8f9cd72c --- /dev/null +++ b/resources/views/list/periods.twig @@ -0,0 +1,44 @@ +{% for period in periods %} +
+
+

{{ period.title }} +

+
+
+ + {% if period.transactions > 0 %} + + + + + {% endif %} + {% for arr in period.spent %} + {% if arr.amount !=0 %} + + + + + {% endif %} + {% endfor %} + + {% for arr in period.earned %} + {% if arr.amount !=0 %} + + + + + {% endif %} + {% endfor %} + + {% for arr in period.transferred %} + {% if arr.amount !=0 %} + + + + + {% endif %} + {% endfor %} +
{{ 'transactions'|_ }}{{ period.transactions }}
{{ 'spent'|_ }}{{ formatAmountByCurrency(arr.currency, arr.amount) }}
{{ 'earned'|_ }}{{ formatAmountByCurrency(arr.currency, arr.amount) }}
{{ 'transferred'|_ }}{{ formatAmountByCurrency(arr.currency, arr.amount, false) }}
+
+
+{% endfor %} \ No newline at end of file