From ac98822a55d39abd1dc1ecdeda62a2201f3a44fe Mon Sep 17 00:00:00 2001 From: James Cole Date: Fri, 9 Feb 2018 16:47:01 +0100 Subject: [PATCH] Fix for issue #1167 --- app/Http/Controllers/AccountController.php | 72 +++----- .../Controllers/Chart/AccountController.php | 167 +++++------------- app/Support/Navigation.php | 9 +- resources/views/accounts/show.twig | 56 ++---- routes/breadcrumbs.php | 11 +- routes/web.php | 6 +- 6 files changed, 89 insertions(+), 232 deletions(-) diff --git a/app/Http/Controllers/AccountController.php b/app/Http/Controllers/AccountController.php index 49e40a182e..ff5d743662 100644 --- a/app/Http/Controllers/AccountController.php +++ b/app/Http/Controllers/AccountController.php @@ -285,68 +285,46 @@ class AccountController extends Controller * * @throws FireflyException */ - public function show(Request $request, Account $account, string $moment = '') + public function show(Request $request, Account $account, Carbon $start = null, Carbon $end = null) { if (AccountType::INITIAL_BALANCE === $account->accountType->type) { return $this->redirectToOriginalAccount($account); } - $range = Preferences::get('viewRange', '1M')->data; + if ($end < $start) { + throw new FireflyException('End is after start!'); + } + $range = Preferences::get('viewRange', '1M')->data; + if (null === $start) { + $start = session('start'); + } + if (null === $end) { + $end = app('navigation')->endOfPeriod($start, $range); + } + $subTitleIcon = config('firefly.subIconsByIdentifier.' . $account->accountType->type); $page = intval($request->get('page')); $pageSize = intval(Preferences::get('listPageSize', 50)->data); - $chartUri = route('chart.account.single', [$account->id]); - $start = null; - $end = null; - $periods = new Collection; $currencyId = intval($account->getMeta('currency_id')); $currency = $this->currencyRepos->find($currencyId); if (0 === $currencyId) { $currency = app('amount')->getDefaultCurrency(); // @codeCoverageIgnore } - - // prep for "all" view. - if ('all' === $moment) { - $subTitle = trans('firefly.all_journals_for_account', ['name' => $account->name]); - $chartUri = route('chart.account.all', [$account->id]); - $first = $this->journalRepos->first(); - $start = $first->date ?? new Carbon; - $end = new Carbon; - } - - // prep for "specific date" view. - if (strlen($moment) > 0 && 'all' !== $moment) { - $start = new Carbon($moment); - $start = app('navigation')->startOfPeriod($start, $range); - $end = app('navigation')->endOfPeriod($start, $range); - $fStart = $start->formatLocalized($this->monthAndDayFormat); - $fEnd = $end->formatLocalized($this->monthAndDayFormat); - $subTitle = trans('firefly.journals_in_period_for_account', ['name' => $account->name, 'start' => $fStart, 'end' => $fEnd]); - $chartUri = route('chart.account.period', [$account->id, $start->format('Y-m-d')]); - $periods = $this->getPeriodOverview($account, $start); - } - - // prep for current period view - if (0 === strlen($moment)) { - $start = clone session('start', app('navigation')->startOfPeriod(new Carbon, $range)); - $end = clone session('end', app('navigation')->endOfPeriod(new Carbon, $range)); - $fStart = $start->formatLocalized($this->monthAndDayFormat); - $fEnd = $end->formatLocalized($this->monthAndDayFormat); - $subTitle = trans('firefly.journals_in_period_for_account', ['name' => $account->name, 'start' => $fStart, 'end' => $fEnd]); - $periods = $this->getPeriodOverview($account, null); - } - - // grab journals: + $fStart = $start->formatLocalized($this->monthAndDayFormat); + $fEnd = $end->formatLocalized($this->monthAndDayFormat); + $subTitle = trans('firefly.journals_in_period_for_account', ['name' => $account->name, 'start' => $fStart, 'end' => $fEnd]); + $chartUri = route('chart.account.period', [$account->id, $start->format('Y-m-d'), $end->format('Y-m-d')]); + $periods = $this->getPeriodOverview($account, $end); $collector = app(JournalCollectorInterface::class); $collector->setAccounts(new Collection([$account]))->setLimit($pageSize)->setPage($page); if (null !== $start) { $collector->setRange($start, $end); } $transactions = $collector->getPaginatedJournals(); - $transactions->setPath(route('accounts.show', [$account->id, $moment])); + $transactions->setPath(route('accounts.show', [$account->id, $start->format('Y-m-d'), $end->format('Y-m-d')])); return view( 'accounts.show', - compact('account', 'currency', 'moment', 'periods', 'subTitleIcon', 'transactions', 'subTitle', 'start', 'end', 'chartUri') + compact('account', 'currency', 'periods', 'subTitleIcon', 'transactions', 'subTitle', 'start', 'end', 'chartUri') ); } @@ -439,6 +417,9 @@ class AccountController extends Controller $range = Preferences::get('viewRange', '1M')->data; $start = $this->repository->oldestJournalDate($account); $end = $date ?? new Carbon; + if ($end < $start) { + list($start, $end) = [$end, $start]; + } // properties for cache $cache = new CacheProperties; @@ -449,11 +430,8 @@ class AccountController extends Controller if ($cache->has()) { return $cache->get(); // @codeCoverageIgnore } - - $dates = app('navigation')->blockPeriods($start, $end, $range); $entries = new Collection; - // loop dates foreach ($dates as $date) { @@ -471,15 +449,15 @@ class AccountController extends Controller ->withOpposingAccount(); $spent = strval($collector->getJournals()->sum('transaction_amount')); - $dateStr = $date['end']->format('Y-m-d'); $dateName = app('navigation')->periodShow($date['start'], $date['period']); $entries->push( [ - 'string' => $dateStr, 'name' => $dateName, 'spent' => $spent, 'earned' => $earned, - 'date' => clone $date['end'],] + 'start' => $date['start']->format('Y-m-d'), + 'end' => $date['end']->format('Y-m-d'), + ] ); } diff --git a/app/Http/Controllers/Chart/AccountController.php b/app/Http/Controllers/Chart/AccountController.php index d559947d87..da9bc727f0 100644 --- a/app/Http/Controllers/Chart/AccountController.php +++ b/app/Http/Controllers/Chart/AccountController.php @@ -58,69 +58,6 @@ class AccountController extends Controller $this->generator = app(GeneratorInterface::class); } - /** - * @param Account $account - * - * @return \Illuminate\Http\JsonResponse - */ - public function all(Account $account) - { - $cache = new CacheProperties; - $cache->addProperty('chart.account.all'); - $cache->addProperty($account->id); - if ($cache->has()) { - return Response::json($cache->get()); // @codeCoverageIgnore - } - - /** @var AccountRepositoryInterface $repository */ - $repository = app(AccountRepositoryInterface::class); - $start = $repository->oldestJournalDate($account); - $end = new Carbon; - - // depending on diff, do something with range of chart. - $step = '1D'; - $months = $start->diffInMonths($end); - if ($months > 3) { - $step = '1W'; - } - if ($months > 24) { - $step = '1M'; // @codeCoverageIgnore - } - if ($months > 100) { - $step = '1Y'; // @codeCoverageIgnore - } - $chartData = []; - $current = clone $start; - switch ($step) { - case '1D': - $format = (string)trans('config.month_and_day'); - $range = Steam::balanceInRange($account, $start, $end); - $previous = array_values($range)[0]; - while ($end >= $current) { - $theDate = $current->format('Y-m-d'); - $balance = $range[$theDate] ?? $previous; - $label = $current->formatLocalized($format); - $chartData[$label] = floatval($balance); - $previous = $balance; - $current->addDay(); - } - break; - case '1W': - case '1M': // @codeCoverageIgnore - case '1Y': // @codeCoverageIgnore - while ($end >= $current) { - $balance = floatval(Steam::balance($account, $current)); - $label = app('navigation')->periodShow($current, $step); - $chartData[$label] = $balance; - $current = app('navigation')->addPeriod($current, $step, 1); - } - break; - } - $data = $this->generator->singleSet($account->name, $chartData); - $cache->store($data); - - return Response::json($data); - } /** * Shows the balances for all the user's expense accounts. @@ -366,34 +303,55 @@ class AccountController extends Controller * * @return \Illuminate\Http\JsonResponse */ - public function period(Account $account, Carbon $start) + public function period(Account $account, Carbon $start, Carbon $end) { - $range = Preferences::get('viewRange', '1M')->data; - $end = app('navigation')->endOfPeriod($start, $range); - $cache = new CacheProperties(); + $cache = new CacheProperties; + $cache->addProperty('chart.account.period'); $cache->addProperty($start); $cache->addProperty($end); - $cache->addProperty('chart.account.period'); $cache->addProperty($account->id); if ($cache->has()) { return Response::json($cache->get()); // @codeCoverageIgnore } - - $format = (string)trans('config.month_and_day'); - $range = Steam::balanceInRange($account, $start, $end); - $current = clone $start; - $previous = array_values($range)[0]; - $chartData = []; - - while ($end >= $current) { - $theDate = $current->format('Y-m-d'); - $balance = $range[$theDate] ?? $previous; - $label = $current->formatLocalized($format); - $chartData[$label] = $balance; - $previous = $balance; - $current->addDay(); + // depending on diff, do something with range of chart. + $step = '1D'; + $months = $start->diffInMonths($end); + if ($months > 3) { + $step = '1W'; + } + if ($months > 24) { + $step = '1M'; // @codeCoverageIgnore + } + if ($months > 100) { + $step = '1Y'; // @codeCoverageIgnore + } + $chartData = []; + $current = clone $start; + switch ($step) { + case '1D': + $format = (string)trans('config.month_and_day'); + $range = Steam::balanceInRange($account, $start, $end); + $previous = array_values($range)[0]; + while ($end >= $current) { + $theDate = $current->format('Y-m-d'); + $balance = $range[$theDate] ?? $previous; + $label = $current->formatLocalized($format); + $chartData[$label] = floatval($balance); + $previous = $balance; + $current->addDay(); + } + break; + case '1W': + case '1M': // @codeCoverageIgnore + case '1Y': // @codeCoverageIgnore + while ($end >= $current) { + $balance = floatval(Steam::balance($account, $current)); + $label = app('navigation')->periodShow($current, $step); + $chartData[$label] = $balance; + $current = app('navigation')->addPeriod($current, $step, 1); + } + break; } - $data = $this->generator->singleSet($account->name, $chartData); $cache->store($data); @@ -457,49 +415,6 @@ class AccountController extends Controller return Response::json($data); } - /** - * Shows an account's balance for a single month. - * - * @param Account $account - * - * @return \Symfony\Component\HttpFoundation\Response - */ - public function single(Account $account) - { - $start = clone session('start', Carbon::now()->startOfMonth()); - $end = clone session('end', Carbon::now()->endOfMonth()); - - // chart properties for cache: - $cache = new CacheProperties(); - $cache->addProperty($start); - $cache->addProperty($end); - $cache->addProperty('chart.account.single'); - $cache->addProperty($account->id); - if ($cache->has()) { - return Response::json($cache->get()); // @codeCoverageIgnore - } - - $format = (string)trans('config.month_and_day'); - $range = Steam::balanceInRange($account, $start, $end); - $current = clone $start; - $previous = array_values($range)[0]; - $chartData = []; - - while ($end >= $current) { - $theDate = $current->format('Y-m-d'); - $balance = $range[$theDate] ?? $previous; - $label = $current->formatLocalized($format); - $chartData[$label] = $balance; - $previous = $balance; - $current->addDay(); - } - - $data = $this->generator->singleSet($account->name, $chartData); - $cache->store($data); - - return Response::json($data); - } - /** * @param Collection $accounts * @param Carbon $start diff --git a/app/Support/Navigation.php b/app/Support/Navigation.php index a7729c05a6..f543a706f7 100644 --- a/app/Support/Navigation.php +++ b/app/Support/Navigation.php @@ -90,8 +90,13 @@ class Navigation */ public function blockPeriods(\Carbon\Carbon $start, \Carbon\Carbon $end, string $range): array { + if ($end < $start) { + list($start, $end) = [$end, $start]; + } $periods = []; - // Start by looping per period: + /* + * Start looping per months for 1 year + the rest of the year: + */ $perMonthEnd = clone $end; $perMonthStart = clone $end; $perMonthStart->startOfyear()->subYear(); @@ -285,7 +290,7 @@ class Navigation /** * @param \Carbon\Carbon $theDate - * @param string $repeatFrequency + * @param string $repeatFrequency * * @return string * diff --git a/resources/views/accounts/show.twig b/resources/views/accounts/show.twig index 79b34d6243..c137e6e45f 100644 --- a/resources/views/accounts/show.twig +++ b/resources/views/accounts/show.twig @@ -1,7 +1,7 @@ {% extends "./layout/default" %} {% block breadcrumbs %} - {{ Breadcrumbs.render(Route.getCurrentRoute.getName, account, moment, start, end) }} + {{ Breadcrumbs.render(Route.getCurrentRoute.getName, account, start, end) }} {% endblock %} {% block content %} @@ -10,11 +10,7 @@

- {% if moment == 'all' %} - {{ trans('firefly.chart_all_journals_for_account', {name:account.name}) }} - {% else %} - {{ trans('firefly.chart_account_in_period', {name: account.name, start: start.formatLocalized(monthAndDayFormat), end: end.formatLocalized(monthAndDayFormat) }) }} - {% endif %} + {{ trans('firefly.chart_account_in_period', {name: account.name, start: start.formatLocalized(monthAndDayFormat), end: end.formatLocalized(monthAndDayFormat) }) }}

@@ -83,14 +79,6 @@
{% endif %} - {% if periods.count > 0 %} -
-
-

{{ 'showEverything'|_ }}

-
-
- {% endif %} -
@@ -104,21 +92,12 @@ {% set showReconcile = false %} {% endif %} {% include 'list.journals' with {sorting:true, hideBills:true, hideBudgets: true, hideCategories: true, showReconcile: showReconcile} %} - {% if periods.count > 0 %} -

- - - {{ 'show_all_no_filter'|_ }} - -

- {% else %} -

- - - {{ 'show_the_current_period_and_overview'|_ }} - -

- {% endif %} +

+ + + {{ 'show_the_current_period_and_overview'|_ }} + +

@@ -128,7 +107,7 @@ {% if (period.spent != 0 or period.earned != 0) %}
-

{{ period.name }} +

{{ period.name }}

@@ -150,7 +129,6 @@
{% endif %} {% endfor %} -

{{ 'showEverything'|_ }}

{% endif %}
@@ -166,22 +144,12 @@ // uri's for charts: var chartUri = '{{ chartUri }}'; {% if start and end %} - var incomeCategoryUri = '{{ route('chart.account.income-category', [account.id, start.format('Ymd'), end.format('Ymd')]) }}'; - var expenseCategoryUri = '{{ route('chart.account.expense-category', [account.id, start.format('Ymd'), end.format('Ymd')]) }}'; - var expenseBudgetUri = '{{ route('chart.account.expense-budget', [account.id, start.format('Ymd'), end.format('Ymd')]) }}'; - {% else %} - var incomeCategoryUri = '{{ route('chart.account.income-category', [account.id, 'all', 'all']) }}'; - var expenseCategoryUri = '{{ route('chart.account.expense-category', [account.id, 'all', 'all']) }}'; - var expenseBudgetUri = '{{ route('chart.account.expense-budget', [account.id, 'all', 'all']) }}'; + var incomeCategoryUri = '{{ route('chart.account.income-category', [account.id, start.format('Ymd'), end.format('Ymd')]) }}'; + var expenseCategoryUri = '{{ route('chart.account.expense-category', [account.id, start.format('Ymd'), end.format('Ymd')]) }}'; + var expenseBudgetUri = '{{ route('chart.account.expense-budget', [account.id, start.format('Ymd'), end.format('Ymd')]) }}'; {% endif %} - - - diff --git a/routes/breadcrumbs.php b/routes/breadcrumbs.php index 68710be4eb..bb03bd83cb 100644 --- a/routes/breadcrumbs.php +++ b/routes/breadcrumbs.php @@ -76,25 +76,18 @@ Breadcrumbs::register( Breadcrumbs::register( 'accounts.show', - function (BreadCrumbsGenerator $breadcrumbs, Account $account, string $moment, Carbon $start, Carbon $end) { + function (BreadCrumbsGenerator $breadcrumbs, Account $account, Carbon $start, Carbon $end) { $what = config('firefly.shortNamesByFullName.' . $account->accountType->type); $breadcrumbs->parent('accounts.index', $what); $breadcrumbs->push($account->name, route('accounts.show', [$account->id])); - // push when is all: - if ('all' === $moment) { - $breadcrumbs->push(trans('firefly.everything'), route('accounts.show', [$account->id, 'all'])); - } - // when is specific period or when empty: - if ('all' !== $moment && '(nothing)' !== $moment) { $title = trans( 'firefly.between_dates_breadcrumb', ['start' => $start->formatLocalized(strval(trans('config.month_and_day'))), 'end' => $end->formatLocalized(strval(trans('config.month_and_day'))),] ); - $breadcrumbs->push($title, route('accounts.show', [$account->id, $moment, $start, $end])); - } + $breadcrumbs->push($title, route('accounts.show', [$account->id, $start, $end])); } ); diff --git a/routes/web.php b/routes/web.php index 3e076feff3..ab14b32774 100755 --- a/routes/web.php +++ b/routes/web.php @@ -101,7 +101,7 @@ Route::group( Route::get('create/{what}', ['uses' => 'AccountController@create', 'as' => 'create'])->where('what', 'revenue|asset|expense'); Route::get('edit/{account}', ['uses' => 'AccountController@edit', 'as' => 'edit']); Route::get('delete/{account}', ['uses' => 'AccountController@delete', 'as' => 'delete']); - Route::get('show/{account}/{moment?}', ['uses' => 'AccountController@show', 'as' => 'show']); + Route::get('show/{account}/{start_date?}/{end_date?}', ['uses' => 'AccountController@show', 'as' => 'show']); // reconcile routes: Route::get('reconcile/{account}/index/{start_date?}/{end_date?}', ['uses' => 'Account\ReconcileController@reconcile', 'as' => 'reconcile']); @@ -244,9 +244,7 @@ Route::group( Route::get('expense', ['uses' => 'AccountController@expenseAccounts', 'as' => 'expense']); Route::get('revenue', ['uses' => 'AccountController@revenueAccounts', 'as' => 'revenue']); Route::get('report/{accountList}/{start_date}/{end_date}', ['uses' => 'AccountController@report', 'as' => 'report']); - Route::get('all/{account}', ['uses' => 'AccountController@all', 'as' => 'all']); - Route::get('single/{account}', ['uses' => 'AccountController@single', 'as' => 'single']); - Route::get('period/{account}/{date}', ['uses' => 'AccountController@period', 'as' => 'period']); + Route::get('period/{account}/{start_date}/{end_date}', ['uses' => 'AccountController@period', 'as' => 'period']); Route::get('income-category/{account}/all/all', ['uses' => 'AccountController@incomeCategoryAll', 'as' => 'income-category-all']); Route::get('expense-category/{account}/all/all', ['uses' => 'AccountController@expenseCategoryAll', 'as' => 'expense-category-all']);