From 18399a9b05cbb27b363e0441389e68d73ded64cf Mon Sep 17 00:00:00 2001 From: James Cole Date: Sun, 15 Sep 2019 06:56:20 +0200 Subject: [PATCH] Fix budget overview for #2593 --- .../Controllers/Budget/ShowController.php | 1 + .../Controllers/Chart/BudgetController.php | 92 ++++++++++++------- public/v1/js/ff/budgets/show.js | 16 ++-- public/v1/js/ff/charts.defaults.js | 1 + public/v1/js/ff/charts.js | 53 ++++++++++- resources/lang/en_US/firefly.php | 5 +- resources/views/v1/budgets/show.twig | 60 +++++++++--- 7 files changed, 169 insertions(+), 59 deletions(-) diff --git a/app/Http/Controllers/Budget/ShowController.php b/app/Http/Controllers/Budget/ShowController.php index 7c09722e6f..f878e01062 100644 --- a/app/Http/Controllers/Budget/ShowController.php +++ b/app/Http/Controllers/Budget/ShowController.php @@ -187,6 +187,7 @@ class ShowController extends Controller 'name' => $budget->name, 'start' => $budgetLimit->start_date->formatLocalized($this->monthAndDayFormat), 'end' => $budgetLimit->end_date->formatLocalized($this->monthAndDayFormat), + 'currency' => $budgetLimit->transactionCurrency->name, ] ); diff --git a/app/Http/Controllers/Chart/BudgetController.php b/app/Http/Controllers/Chart/BudgetController.php index 8e7650b943..e11463dc70 100644 --- a/app/Http/Controllers/Chart/BudgetController.php +++ b/app/Http/Controllers/Chart/BudgetController.php @@ -137,8 +137,6 @@ class BudgetController extends Controller /** * Shows the amount left in a specific budget limit. * - * TODO this chart is not multi-currency aware. - * * @param Budget $budget * @param BudgetLimit $budgetLimit * @@ -177,6 +175,8 @@ class BudgetController extends Controller $start->addDay(); } $data = $this->generator->singleSet((string)trans('firefly.left'), $entries); + // add currency symbol from budget limit: + $data['datasets'][0]['currency_symbol'] = $budgetLimit->transactionCurrency->symbol; $cache->store($data); return response()->json($data); @@ -186,14 +186,12 @@ class BudgetController extends Controller /** * Shows how much is spent per asset account. * - * TODO this chart is not multi-currency aware. - * * @param Budget $budget * @param BudgetLimit|null $budgetLimit * * @return JsonResponse */ - public function expenseAsset(Budget $budget, ?BudgetLimit $budgetLimit): JsonResponse + public function expenseAsset(Budget $budget, ?BudgetLimit $budgetLimit = null): JsonResponse { $budgetLimitId = null === $budgetLimit ? 0 : $budgetLimit->id; $cache = new CacheProperties; @@ -208,24 +206,38 @@ class BudgetController extends Controller $collector = app(GroupCollectorInterface::class); $collector->setBudget($budget); if (null !== $budgetLimit) { - $collector->setRange($budgetLimit->start_date, $budgetLimit->end_date); + $collector->setRange($budgetLimit->start_date, $budgetLimit->end_date) + ->setCurrency($budgetLimit->transactionCurrency); } + if (null === $budgetLimit) { + $collector->setRange(session()->get('start'), session()->get('end')); + } + $journals = $collector->getExtractedJournals(); $result = []; $chartData = []; + + // group by asset account ID: foreach ($journals as $journal) { - $assetId = (int)$journal['destination_account_id']; - $result[$assetId] = $result[$assetId] ?? '0'; - $result[$assetId] = bcadd($journal['amount'], $result[$assetId]); + $assetId = (int)$journal['destination_account_id']; + $result[$assetId] = $result[$assetId] ?? [ + 'amount' => '0', + 'currency_symbol' => $journal['currency_symbol'], + ]; + $result[$assetId]['amount'] = bcadd($journal['amount'], $result[$assetId]['amount']); } $names = $this->getAccountNames(array_keys($result)); - foreach ($result as $assetId => $amount) { - $chartData[$names[$assetId]] = $amount; + foreach ($result as $assetId => $info) { + $chartData[$names[$assetId]] + = [ + 'amount' => $info['amount'], + 'currency_symbol' => $info['currency_symbol'], + ]; } - $data = $this->generator->pieChart($chartData); + $data = $this->generator->multiCurrencyPieChart($chartData); $cache->store($data); return response()->json($data); @@ -235,14 +247,12 @@ class BudgetController extends Controller /** * Shows how much is spent per category. * - * TODO this chart is not multi-currency aware. - * * @param Budget $budget * @param BudgetLimit|null $budgetLimit * * @return JsonResponse */ - public function expenseCategory(Budget $budget, ?BudgetLimit $budgetLimit): JsonResponse + public function expenseCategory(Budget $budget, ?BudgetLimit $budgetLimit = null): JsonResponse { $budgetLimitId = null === $budgetLimit ? 0 : $budgetLimit->id; $cache = new CacheProperties; @@ -257,23 +267,33 @@ class BudgetController extends Controller $collector = app(GroupCollectorInterface::class); $collector->setBudget($budget)->withCategoryInformation(); if (null !== $budgetLimit) { - $collector->setRange($budgetLimit->start_date, $budgetLimit->end_date); + $collector->setRange($budgetLimit->start_date, $budgetLimit->end_date) + ->setCurrency($budgetLimit->transactionCurrency); + } + if (null === $budgetLimit) { + $collector->setRange(session()->get('start'), session()->get('end')); } $journals = $collector->getExtractedJournals(); $result = []; $chartData = []; foreach ($journals as $journal) { - $categoryId = (int)$journal['category_id']; - $result[$categoryId] = $result[$categoryId] ?? '0'; - $result[$categoryId] = bcadd($journal['amount'], $result[$categoryId]); + $categoryId = (int)$journal['category_id']; + $result[$categoryId] = $result[$categoryId] ?? [ + 'amount' => '0', + 'currency_symbol' => $journal['currency_symbol'], + ]; + $result[$categoryId]['amount'] = bcadd($journal['amount'], $result[$categoryId]['amount']); } $names = $this->getCategoryNames(array_keys($result)); - foreach ($result as $categoryId => $amount) { - $chartData[$names[$categoryId]] = $amount; + foreach ($result as $categoryId => $info) { + $chartData[$names[$categoryId]] = [ + 'amount' => $info['amount'], + 'currency_symbol' => $info['currency_symbol'], + ]; } - $data = $this->generator->pieChart($chartData); + $data = $this->generator->multiCurrencyPieChart($chartData); $cache->store($data); return response()->json($data); @@ -283,14 +303,13 @@ class BudgetController extends Controller /** * Shows how much is spent per expense account. * - * TODO this chart is not multi-currency aware. * * @param Budget $budget * @param BudgetLimit|null $budgetLimit * * @return JsonResponse */ - public function expenseExpense(Budget $budget, ?BudgetLimit $budgetLimit): JsonResponse + public function expenseExpense(Budget $budget, ?BudgetLimit $budgetLimit = null): JsonResponse { $budgetLimitId = null === $budgetLimit ? 0 : $budgetLimit->id; $cache = new CacheProperties; @@ -305,7 +324,12 @@ class BudgetController extends Controller $collector = app(GroupCollectorInterface::class); $collector->setTypes([TransactionType::WITHDRAWAL])->setBudget($budget)->withAccountInformation(); if (null !== $budgetLimit) { - $collector->setRange($budgetLimit->start_date, $budgetLimit->end_date); + $collector->setRange($budgetLimit->start_date, $budgetLimit->end_date) + ->setCurrency($budgetLimit->transactionCurrency); + } + + if (null === $budgetLimit) { + $collector->setRange(session()->get('start'), session()->get('end')); } $journals = $collector->getExtractedJournals(); @@ -313,18 +337,24 @@ class BudgetController extends Controller $chartData = []; /** @var array $journal */ foreach ($journals as $journal) { - $opposingId = (int)$journal['destination_account_id']; - $result[$opposingId] = $result[$opposingId] ?? '0'; - $result[$opposingId] = bcadd($journal['amount'], $result[$opposingId]); + $opposingId = (int)$journal['destination_account_id']; + $result[$opposingId] = $result[$opposingId] ?? [ + 'amount' => '0', + 'currency_symbol' => $journal['currency_symbol'], + ]; + $result[$opposingId]['amount'] = bcadd($journal['amount'], $result[$opposingId]['amount']); } $names = $this->getAccountNames(array_keys($result)); - foreach ($result as $opposingId => $amount) { + foreach ($result as $opposingId => $info) { $name = $names[$opposingId] ?? 'no name'; - $chartData[$name] = $amount; + $chartData[$name] = [ + 'amount' => $info['amount'], + 'currency_symbol' => $info['currency_symbol'], + ]; } - $data = $this->generator->pieChart($chartData); + $data = $this->generator->multiCurrencyPieChart($chartData); $cache->store($data); return response()->json($data); diff --git a/public/v1/js/ff/budgets/show.js b/public/v1/js/ff/budgets/show.js index 07c18bd6be..746abe3267 100644 --- a/public/v1/js/ff/budgets/show.js +++ b/public/v1/js/ff/budgets/show.js @@ -23,17 +23,15 @@ $(function () { "use strict"; if (budgetLimitID > 0) { - lineChart(budgetChartUri, 'budgetOverview'); - } - if (budgetLimitID === 0) { - columnChart(budgetChartUri, 'budgetOverview'); - } - - // other three charts: - if (budgetLimitID > 0) { + otherCurrencyLineChart(budgetChartUri, 'budgetOverview', currencySymbol); + pieChart(expenseCategoryUri, 'budget-cat-out'); + pieChart(expenseAssetUri, 'budget-asset-out'); + pieChart(expenseExpenseUri, 'budget-expense-out'); + } + if (budgetLimitID === 0) { + columnChart(budgetChartUri, 'budgetOverview'); pieChart(expenseCategoryUri, 'budget-cat-out'); pieChart(expenseAssetUri, 'budget-asset-out'); pieChart(expenseExpenseUri, 'budget-expense-out'); } - }); diff --git a/public/v1/js/ff/charts.defaults.js b/public/v1/js/ff/charts.defaults.js index 7b502f3419..edf3f637d1 100644 --- a/public/v1/js/ff/charts.defaults.js +++ b/public/v1/js/ff/charts.defaults.js @@ -97,6 +97,7 @@ var defaultChartOptions = { ticks: { callback: function (tickValue) { "use strict"; + // use first symbol or null: return accounting.formatMoney(tickValue); }, diff --git a/public/v1/js/ff/charts.js b/public/v1/js/ff/charts.js index 7013842bbe..b3dfcc5f77 100644 --- a/public/v1/js/ff/charts.js +++ b/public/v1/js/ff/charts.js @@ -92,6 +92,57 @@ function lineChart(URI, container) { drawAChart(URI, container, chartType, options, colorData); } +/** + * Overrules the currency the line chart is drawn in. + * + * @param URI + * @param container + */ +function otherCurrencyLineChart(URI, container, currencySymbol) { + "use strict"; + + var colorData = true; + + var newOpts = { + scales: { + xAxes: [ + { + gridLines: { + display: false + }, + ticks: { + // break ticks when too long. + callback: function (value, index, values) { + return formatLabel(value, 20); + } + } + } + ], + yAxes: [{ + display: true, + //hello: 'fresh', + ticks: { + callback: function (tickValue) { + "use strict"; + // use first symbol or null: + return accounting.formatMoney(tickValue); + + }, + beginAtZero: true + } + }] + }, + }; + + //var options = $.extend(true, newOpts, defaultChartOptions); + var options = $.extend(true, defaultChartOptions, newOpts); + + console.log(options); + var chartType = 'line'; + + drawAChart(URI, container, chartType, options, colorData); +} + /** * Function to draw a chart with double Y Axes and stacked columns. * @@ -305,7 +356,6 @@ function drawAChart(URI, container, chartType, options, colorData) { return; } - $.getJSON(URI).done(function (data) { containerObj.removeClass('general-chart-error'); if (data.labels.length === 0) { @@ -324,7 +374,6 @@ function drawAChart(URI, container, chartType, options, colorData) { return; } - if (colorData) { data = colorizeData(data); } diff --git a/resources/lang/en_US/firefly.php b/resources/lang/en_US/firefly.php index 74e5c9e587..2c5605e20b 100644 --- a/resources/lang/en_US/firefly.php +++ b/resources/lang/en_US/firefly.php @@ -133,8 +133,9 @@ return [ 'current_period' => 'Current period', 'show_the_current_period_and_overview' => 'Show the current period and overview', 'pref_languages_locale' => 'For a language other than English to work properly, your operating system must be equipped with the correct locale-information. If these are not present, currency data, dates and amounts may be formatted wrong.', - 'budget_in_period' => 'All transactions for budget ":name" between :start and :end', - 'chart_budget_in_period' => 'Chart for all transactions for budget ":name" between :start and :end', + 'budget_in_period' => 'All transactions for budget ":name" between :start and :end in :currency', + 'chart_budget_in_period' => 'Chart for all transactions for budget ":name" between :start and :end in :currency', + 'chart_budget_in_period_only_currency' => 'The amount you budgeted was in :currency, so this chart will only show transactions in :currency.', 'chart_account_in_period' => 'Chart for all transactions for account ":name" between :start and :end', 'chart_category_in_period' => 'Chart for all transactions for category ":name" between :start and :end', 'chart_category_all' => 'Chart for all transactions for category ":name"', diff --git a/resources/views/v1/budgets/show.twig b/resources/views/v1/budgets/show.twig index 749a165672..44100805fd 100644 --- a/resources/views/v1/budgets/show.twig +++ b/resources/views/v1/budgets/show.twig @@ -11,7 +11,7 @@

{% if budgetLimit %} - {{ trans('firefly.chart_budget_in_period', {name: budget.name, start: budgetLimit.start_date.formatLocalized(monthAndDayFormat), end: budgetLimit.end_date.formatLocalized(monthAndDayFormat) }) }} + {{ trans('firefly.chart_budget_in_period', {name: budget.name, start: budgetLimit.start_date.formatLocalized(monthAndDayFormat), end: budgetLimit.end_date.formatLocalized(monthAndDayFormat),currency: budgetLimit.transactionCurrency.name }) }} {% else %} {{ trans('firefly.chart_all_journals_for_budget', {name:budget.name}) }} {% endif %} @@ -29,10 +29,16 @@
+ {% if budgetLimit %} +
+

+ {{ trans('firefly.chart_budget_in_period_only_currency', {currency: budgetLimit.transactionCurrency.name}) }} +

+
+ {% endif %}

-{% if budgetLimit %}
@@ -44,6 +50,13 @@
+ {% if budgetLimit %} +
+

+ {{ trans('firefly.chart_budget_in_period_only_currency', {currency: budgetLimit.transactionCurrency.name}) }} +

+
+ {% endif %}
@@ -56,6 +69,13 @@
+ {% if budgetLimit %} +
+

+ {{ trans('firefly.chart_budget_in_period_only_currency', {currency: budgetLimit.transactionCurrency.name}) }} +

+
+ {% endif %}
@@ -68,10 +88,16 @@
+ {% if budgetLimit %} +
+

+ {{ trans('firefly.chart_budget_in_period_only_currency', {currency: budgetLimit.transactionCurrency.name}) }} +

+
+ {% endif %} -{% endif %}
@@ -118,11 +144,15 @@ - + - + {% if limit.spent > 0 %} @@ -163,17 +193,17 @@ var budgetID = {{ budget.id }}; var budgetLimitID = 0; {% if budgetLimit.id %} - budgetLimitID = {{ budgetLimit.id }}; - var budgetChartUri = '{{ route('chart.budget.budget-limit', [budget.id, budgetLimit.id] ) }}'; - - var expenseCategoryUri = '{{ route('chart.budget.expense-category', [budget.id, budgetLimit.id]) }}'; - var expenseAssetUri = '{{ route('chart.budget.expense-asset', [budget.id, budgetLimit.id]) }}'; - var expenseExpenseUri = '{{ route('chart.budget.expense-expense', [budget.id, budgetLimit.id]) }}'; + budgetLimitID = {{ budgetLimit.id }}; + var budgetChartUri = '{{ route('chart.budget.budget-limit', [budget.id, budgetLimit.id] ) }}'; + var currencySymbol = '{{ budgetLimit.transactionCurrency.symbol }}'; + var expenseCategoryUri = '{{ route('chart.budget.expense-category', [budget.id, budgetLimit.id]) }}'; + var expenseAssetUri = '{{ route('chart.budget.expense-asset', [budget.id, budgetLimit.id]) }}'; + var expenseExpenseUri = '{{ route('chart.budget.expense-expense', [budget.id, budgetLimit.id]) }}'; {% else %} - var budgetChartUri = '{{ route('chart.budget.budget', [budget.id] ) }}'; - var expenseCategoryUri = '{{ route('chart.budget.expense-category', [budget.id]) }}'; - var expenseAssetUri = '{{ route('chart.budget.expense-asset', [budget.id]) }}'; - var expenseExpenseUri = '{{ route('chart.budget.expense-expense', [budget.id]) }}'; + var budgetChartUri = '{{ route('chart.budget.budget', [budget.id] ) }}'; + var expenseCategoryUri = '{{ route('chart.budget.expense-category', [budget.id]) }}'; + var expenseAssetUri = '{{ route('chart.budget.expense-asset', [budget.id]) }}'; + var expenseExpenseUri = '{{ route('chart.budget.expense-expense', [budget.id]) }}'; {% endif %}
{{ 'amount'|_ }}{{ limit.amount|formatAmount }} + {{ formatAmountBySymbol(limit.amount, limit.transactionCurrency.symbol, limit.transactionCurrency.decimal_places) }} +
{{ 'spent'|_ }}{{ limit.spent|formatAmount }} + {{ formatAmountBySymbol(limit.spent, limit.transactionCurrency.symbol, limit.transactionCurrency.decimal_places) }} +