Clean up balance chart.

app/Api/V2/Controllers/Chart/AccountController.php

@@ -28,14 +28,13 @@ use FireflyIII\Api\V2\Controllers\Controller;
 use FireflyIII\Api\V2\Request\Chart\ChartRequest;
 use FireflyIII\Exceptions\FireflyException;
 use FireflyIII\Models\Account;
-use FireflyIII\Models\AccountType;
 use FireflyIII\Models\TransactionCurrency;
 use FireflyIII\Repositories\UserGroups\Account\AccountRepositoryInterface;
 use FireflyIII\Support\Chart\ChartData;
 use FireflyIII\Support\Http\Api\CleansChartData;
+use FireflyIII\Support\Http\Api\CollectsAccountsFromFilter;
 use FireflyIII\Support\Http\Api\ValidatesUserGroupTrait;
 use Illuminate\Http\JsonResponse;
-use Illuminate\Support\Collection;
 
 /**
  * Class AccountController
@@ -44,6 +43,7 @@ class AccountController extends Controller
 {
     use CleansChartData;
     use ValidatesUserGroupTrait;
+    use CollectsAccountsFromFilter;
 
     private AccountRepositoryInterface $repository;
     private ChartData                  $chartData;
@@ -66,6 +66,7 @@ class AccountController extends Controller
 
     /**
      * TODO fix documentation
+     *
      * @throws FireflyException
      */
     public function dashboard(ChartRequest $request): JsonResponse
@@ -86,57 +87,11 @@ class AccountController extends Controller
         return response()->json($this->chartData->render());
     }
 
-    /**
-     * TODO Duplicate function but I think it belongs here or in a separate trait
-     *
-     */
-    private function getAccountList(array $queryParameters): Collection
-    {
-        $collection = new Collection();
-
-        // always collect from the query parameter, even when it's empty.
-        foreach ($queryParameters['accounts'] as $accountId) {
-            $account = $this->repository->find((int) $accountId);
-            if (null !== $account) {
-                $collection->push($account);
-            }
-        }
-
-        // if no "preselected", and found accounts
-        if ('empty' === $queryParameters['preselected'] && $collection->count() > 0) {
-            return $collection;
-        }
-        // if no preselected, but no accounts:
-        if ('empty' === $queryParameters['preselected'] && 0 === $collection->count()) {
-            $defaultSet = $this->repository->getAccountsByType([AccountType::ASSET, AccountType::DEFAULT])->pluck('id')->toArray();
-            $frontpage  = app('preferences')->get('frontpageAccounts', $defaultSet);
-
-            if (!(is_array($frontpage->data) && count($frontpage->data) > 0)) {
-                $frontpage->data = $defaultSet;
-                $frontpage->save();
-            }
-
-            return $this->repository->getAccountsById($frontpage->data);
-        }
-
-        // both options are overruled by "preselected"
-        if ('all' === $queryParameters['preselected']) {
-            return $this->repository->getAccountsByType([AccountType::ASSET, AccountType::DEFAULT, AccountType::LOAN, AccountType::DEBT, AccountType::MORTGAGE]);
-        }
-        if ('assets' === $queryParameters['preselected']) {
-            return $this->repository->getAccountsByType([AccountType::ASSET, AccountType::DEFAULT]);
-        }
-        if ('liabilities' === $queryParameters['preselected']) {
-            return $this->repository->getAccountsByType([AccountType::LOAN, AccountType::DEBT, AccountType::MORTGAGE]);
-        }
-
-        return $collection;
-    }
-
     /**
      * @throws FireflyException
      */
-    private function renderAccountData(array $params, Account $account): void {
+    private function renderAccountData(array $params, Account $account): void
+    {
         $currency = $this->repository->getAccountCurrency($account);
         if (null === $currency) {
             $currency = $this->default;
@@ -155,6 +110,7 @@ class AccountController extends Controller
             'native_currency_code'           => $this->default->code,
             'native_currency_symbol'         => $this->default->symbol,
             'native_currency_decimal_places' => $this->default->decimal_places,
+            'date'                           => $params['start']->toAtomString(),
             'start'                          => $params['start']->toAtomString(),
             'end'                            => $params['end']->toAtomString(),
             'period'                         => '1D',

app/Api/V2/Controllers/Chart/BalanceController.php

@@ -24,18 +24,18 @@ declare(strict_types=1);
 
 namespace FireflyIII\Api\V2\Controllers\Chart;
 
-use Carbon\Carbon;
 use FireflyIII\Api\V2\Controllers\Controller;
-use FireflyIII\Api\V2\Request\Chart\BalanceChartRequest;
+use FireflyIII\Api\V2\Request\Chart\ChartRequest;
-use FireflyIII\Enums\UserRoleEnum;
 use FireflyIII\Exceptions\FireflyException;
 use FireflyIII\Helpers\Collector\GroupCollectorInterface;
 use FireflyIII\Models\TransactionCurrency;
 use FireflyIII\Models\TransactionType;
+use FireflyIII\Repositories\UserGroups\Account\AccountRepositoryInterface;
+use FireflyIII\Support\Chart\ChartData;
 use FireflyIII\Support\Http\Api\AccountBalanceGrouped;
 use FireflyIII\Support\Http\Api\CleansChartData;
+use FireflyIII\Support\Http\Api\CollectsAccountsFromFilter;
 use Illuminate\Http\JsonResponse;
-use Illuminate\Support\Collection;
 
 /**
  * Class BalanceController
@@ -43,7 +43,30 @@ use Illuminate\Support\Collection;
 class BalanceController extends Controller
 {
     use CleansChartData;
-    protected array                    $acceptedRoles = [UserRoleEnum::READ_ONLY];
+    use CollectsAccountsFromFilter;
+
+    private AccountRepositoryInterface $repository;
+    private GroupCollectorInterface    $collector;
+    private ChartData                  $chartData;
+    private TransactionCurrency        $default;
+
+    public function __construct()
+    {
+        parent::__construct();
+        $this->middleware(
+            function ($request, $next) {
+                $this->repository = app(AccountRepositoryInterface::class);
+                $this->collector = app(GroupCollectorInterface::class);
+                $userGroup        = $this->validateUserGroup($request);
+                $this->repository->setUserGroup($userGroup);
+                $this->collector->setUserGroup($userGroup);
+                $this->chartData = new ChartData();
+                $this->default   = app('amount')->getDefaultCurrency();
+
+                return $next($request);
+            }
+        );
+    }
 
     /**
      * The code is practically a duplicate of ReportController::operations.
@@ -54,50 +77,42 @@ class BalanceController extends Controller
      * If the transaction being processed is already in native currency OR if the
      * foreign amount is in the native currency, the amount will not be converted.
      *
-     * TODO validate and set user_group_id
-     * TODO collector set group, not user
-     *
      * @throws FireflyException
      */
-    public function balance(BalanceChartRequest $request): JsonResponse
+    public function balance(ChartRequest $request): JsonResponse
     {
-        $params         = $request->getAll();
+        $queryParameters = $request->getParameters();
+        $accounts        = $this->getAccountList($queryParameters);
 
-        /** @var Carbon $start */
+        // move date to end of day
-        $start          = $this->parameters->get('start');
+        $queryParameters['start']->startOfDay();
-
+        $queryParameters['end']->endOfDay();
-        /** @var Carbon $end */
-        $end            = $this->parameters->get('end');
-        $end->endOfDay();
-
-        /** @var Collection $accounts */
-        $accounts       = $params['accounts'];
-
-        /** @var string $preferredRange */
-        $preferredRange = $params['period'];
 
         // prepare for currency conversion and data collection:
         /** @var TransactionCurrency $default */
         $default = app('amount')->getDefaultCurrency();
 
         // get journals for entire period:
-        /** @var GroupCollectorInterface $collector */
+
-        $collector      = app(GroupCollectorInterface::class);
+        $this->collector->setRange($queryParameters['start'], $queryParameters['end'])
-        $collector->setRange($start, $end)->withAccountInformation();
+                        ->withAccountInformation()
-        $collector->setXorAccounts($accounts);
+                        ->setXorAccounts($accounts)
-        $collector->setTypes([TransactionType::WITHDRAWAL, TransactionType::DEPOSIT, TransactionType::RECONCILIATION, TransactionType::TRANSFER]);
+                        ->setTypes([TransactionType::WITHDRAWAL, TransactionType::DEPOSIT, TransactionType::RECONCILIATION, TransactionType::TRANSFER]);
-        $journals       = $collector->getExtractedJournals();
+        $journals = $this->collector->getExtractedJournals();
 
         $object = new AccountBalanceGrouped();
-        $object->setPreferredRange($preferredRange);
+        $object->setPreferredRange($queryParameters['period']);
         $object->setDefault($default);
         $object->setAccounts($accounts);
         $object->setJournals($journals);
-        $object->setStart($start);
+        $object->setStart($queryParameters['start']);
-        $object->setEnd($end);
+        $object->setEnd($queryParameters['end']);
         $object->groupByCurrencyAndPeriod();
-        $chartData      = $object->convertToChartData();
+        $data = $object->convertToChartData();
+        foreach($data as $entry) {
+            $this->chartData->add($entry);
+        }
 
-        return response()->json($this->clean($chartData));
+        return response()->json($this->chartData->render());
     }
 }

app/Api/V2/Request/Chart/ChartRequest.php

@@ -55,6 +55,7 @@ class ChartRequest extends FormRequest
             'start'       => $this->dateOrToday($queryParameters, 'start'),
             'end'         => $this->dateOrToday($queryParameters, 'end'),
             'preselected' => $this->stringFromQueryParams($queryParameters, 'preselected', 'empty'),
+            'period' => $this->stringFromQueryParams($queryParameters, 'period', '1M'),
             'accounts'    => $this->arrayOfStrings($queryParameters, 'accounts'),
             // preselected heeft maar een paar toegestane waardes, dat moet ook goed gaan.
             //            'query'         => $this->arrayOfStrings($queryParameters, 'query'),

app/Support/Chart/ChartData.php

@@ -57,14 +57,11 @@ class ChartData
         if (array_key_exists('native_currency_id', $data)) {
             $data['native_currency_id'] = (string) $data['native_currency_id'];
         }
-        if (!array_key_exists('start', $data)) {
+        $required = ['start', 'date', 'end', 'entries', 'native_entries'];
-            throw new FireflyException('Data-set is missing the "start"-variable.');
+        foreach ($required as $field) {
+            if (!array_key_exists($field, $data)) {
+                throw new FireflyException(sprintf('Data-set is missing the "%s"-variable.', $field));
             }
-        if (!array_key_exists('end', $data)) {
-            throw new FireflyException('Data-set is missing the "end"-variable.');
-        }
-        if (!array_key_exists('period', $data)) {
-            throw new FireflyException('Data-set is missing the "period"-variable.');
         }
 
         $this->series[] = $data;

app/Support/Http/Api/AccountBalanceGrouped.php

@@ -44,6 +44,13 @@ class AccountBalanceGrouped
     private array                 $journals   = [];
     private string                $preferredRange;
     private Carbon                $start;
+    private ExchangeRateConverter $converter;
+
+    public function __construct()
+    {
+        $this->accountIds = [];
+        $this->converter  = app(ExchangeRateConverter::class);
+    }
 
     /**
      * Convert the given input to a chart compatible array.
@@ -66,6 +73,7 @@ class AccountBalanceGrouped
                 'native_currency_symbol'         => $currency['native_currency_symbol'],
                 'native_currency_code'           => $currency['native_currency_code'],
                 'native_currency_decimal_places' => $currency['native_currency_decimal_places'],
+                'date'                           => $this->start->toAtomString(),
                 'start'                          => $this->start->toAtomString(),
                 'end'                            => $this->end->toAtomString(),
                 'period'                         => $this->preferredRange,
@@ -82,6 +90,7 @@ class AccountBalanceGrouped
                 'native_currency_symbol'         => $currency['native_currency_symbol'],
                 'native_currency_code'           => $currency['native_currency_code'],
                 'native_currency_decimal_places' => $currency['native_currency_decimal_places'],
+                'date'                           => $this->start->toAtomString(),
                 'start'                          => $this->start->toAtomString(),
                 'end'                            => $this->end->toAtomString(),
                 'period'                         => $this->preferredRange,
@@ -124,76 +133,7 @@ class AccountBalanceGrouped
         // loop. group by currency and by period.
         /** @var array $journal */
         foreach ($this->journals as $journal) {
-            // format the date according to the period
+            $this->processJournal($journal);
-            $period                                          = $journal['date']->format($this->carbonFormat);
-            $currencyId                                      = (int)$journal['currency_id'];
-            $currency                                        = $this->currencies[$currencyId] ?? TransactionCurrency::find($currencyId);
-            $this->currencies[$currencyId]                   = $currency; // may just re-assign itself, don't mind.
-
-            // set the array with monetary info, if it does not exist.
-            $this->data[$currencyId]          ??= [
-                'currency_id'                    => (string)$currencyId,
-                'currency_symbol'                => $journal['currency_symbol'],
-                'currency_code'                  => $journal['currency_code'],
-                'currency_name'                  => $journal['currency_name'],
-                'currency_decimal_places'        => $journal['currency_decimal_places'],
-                // native currency info (could be the same)
-                'native_currency_id'             => (string)$this->default->id,
-                'native_currency_code'           => $this->default->code,
-                'native_currency_symbol'         => $this->default->symbol,
-                'native_currency_decimal_places' => $this->default->decimal_places,
-            ];
-
-            // set the array (in monetary info) with spent/earned in this $period, if it does not exist.
-            $this->data[$currencyId][$period] ??= [
-                'period'        => $period,
-                'spent'         => '0',
-                'earned'        => '0',
-                'native_spent'  => '0',
-                'native_earned' => '0',
-            ];
-            // is this journal's amount in- our outgoing?
-            $key                                             = 'spent';
-            $amount                                          = app('steam')->negative($journal['amount']);
-            // deposit = incoming
-            // transfer or reconcile or opening balance, and these accounts are the destination.
-            if (
-                TransactionType::DEPOSIT === $journal['transaction_type_type']
-
-                || (
-                    (
-                        TransactionType::TRANSFER === $journal['transaction_type_type']
-                        || TransactionType::RECONCILIATION === $journal['transaction_type_type']
-                        || TransactionType::OPENING_BALANCE === $journal['transaction_type_type']
-                    )
-                    && in_array($journal['destination_account_id'], $this->accountIds, true)
-                )
-            ) {
-                $key    = 'earned';
-                $amount = app('steam')->positive($journal['amount']);
-            }
-
-            // get conversion rate
-            try {
-                $rate = $converter->getCurrencyRate($currency, $this->default, $journal['date']);
-            } catch (FireflyException $e) {
-                app('log')->error($e->getMessage());
-                $rate = '1';
-            }
-            $amountConverted                                 = bcmul($amount, $rate);
-
-            // perhaps transaction already has the foreign amount in the native currency.
-            if ((int)$journal['foreign_currency_id'] === $this->default->id) {
-                $amountConverted = $journal['foreign_amount'] ?? '0';
-                $amountConverted = 'earned' === $key ? app('steam')->positive($amountConverted) : app('steam')->negative($amountConverted);
-            }
-
-            // add normal entry
-            $this->data[$currencyId][$period][$key]          = bcadd($this->data[$currencyId][$period][$key], $amount);
-
-            // add converted entry
-            $convertedKey                                    = sprintf('native_%s', $key);
-            $this->data[$currencyId][$period][$convertedKey] = bcadd($this->data[$currencyId][$period][$convertedKey], $amountConverted);
         }
         $converter->summarize();
     }
@@ -242,4 +182,111 @@ class AccountBalanceGrouped
     {
         $this->start = $start;
     }
+
+    private function processJournal(array $journal): void
+    {
+        // format the date according to the period
+        $period     = $journal['date']->format($this->carbonFormat);
+        $currencyId = (int) $journal['currency_id'];
+        $currency   = $this->findCurrency($currencyId);
+
+        // set the array with monetary info, if it does not exist.
+        $this->createDefaultDataEntry($journal);
+        // set the array (in monetary info) with spent/earned in this $period, if it does not exist.
+        $this->createDefaultPeriodEntry($journal);
+
+        // is this journal's amount in- our outgoing?
+        $key    = $this->getDataKey($journal);
+        $amount = 'spent' === $key ? app('steam')->negative($journal['amount']) : app('steam')->positive($journal['amount']);
+
+        // get conversion rate
+        $rate            = $this->getRate($currency, $journal['date']);
+        $amountConverted = bcmul($amount, $rate);
+
+        // perhaps transaction already has the foreign amount in the native currency.
+        if ((int) $journal['foreign_currency_id'] === $this->default->id) {
+            $amountConverted = $journal['foreign_amount'] ?? '0';
+            $amountConverted = 'earned' === $key ? app('steam')->positive($amountConverted) : app('steam')->negative($amountConverted);
+        }
+
+        // add normal entry
+        $this->data[$currencyId][$period][$key] = bcadd($this->data[$currencyId][$period][$key], $amount);
+
+        // add converted entry
+        $convertedKey                                    = sprintf('native_%s', $key);
+        $this->data[$currencyId][$period][$convertedKey] = bcadd($this->data[$currencyId][$period][$convertedKey], $amountConverted);
+    }
+
+    private function findCurrency(int $currencyId): TransactionCurrency
+    {
+        if (array_key_exists($currencyId, $this->currencies)) {
+            return $this->currencies[$currencyId];
+        }
+        $this->currencies[$currencyId] = TransactionCurrency::find($currencyId);
+        return $this->currencies[$currencyId];
+    }
+
+    private function createDefaultDataEntry(array $journal): void
+    {
+        $currencyId              = (int) $journal['currency_id'];
+        $this->data[$currencyId] ??= [
+            'currency_id'                    => (string) $currencyId,
+            'currency_symbol'                => $journal['currency_symbol'],
+            'currency_code'                  => $journal['currency_code'],
+            'currency_name'                  => $journal['currency_name'],
+            'currency_decimal_places'        => $journal['currency_decimal_places'],
+            // native currency info (could be the same)
+            'native_currency_id'             => (string) $this->default->id,
+            'native_currency_code'           => $this->default->code,
+            'native_currency_symbol'         => $this->default->symbol,
+            'native_currency_decimal_places' => $this->default->decimal_places,
+        ];
+    }
+
+    private function createDefaultPeriodEntry(array $journal): void
+    {
+        $currencyId                       = (int) $journal['currency_id'];
+        $period                           = $journal['date']->format($this->carbonFormat);
+        $this->data[$currencyId][$period] ??= [
+            'period'        => $period,
+            'spent'         => '0',
+            'earned'        => '0',
+            'native_spent'  => '0',
+            'native_earned' => '0',
+        ];
+    }
+
+    private function getDataKey(array $journal): string
+    {
+
+        $key = 'spent';
+        // deposit = incoming
+        // transfer or reconcile or opening balance, and these accounts are the destination.
+        if (
+            TransactionType::DEPOSIT === $journal['transaction_type_type']
+
+            || (
+                (
+                    TransactionType::TRANSFER === $journal['transaction_type_type']
+                    || TransactionType::RECONCILIATION === $journal['transaction_type_type']
+                    || TransactionType::OPENING_BALANCE === $journal['transaction_type_type']
+                )
+                && in_array($journal['destination_account_id'], $this->accountIds, true)
+            )
+        ) {
+            $key = 'earned';
+        }
+        return $key;
+    }
+
+    private function getRate(TransactionCurrency $currency, Carbon $date): string
+    {
+        try {
+            $rate = $this->converter->getCurrencyRate($currency, $this->default, $date);
+        } catch (FireflyException $e) {
+            app('log')->error($e->getMessage());
+            $rate = '1';
+        }
+        return $rate;
+    }
 }

app/Support/Http/Api/CollectsAccountsFromFilter.php

@@ -0,0 +1,77 @@
+<?php
+/*
+ * CollectsAccountsFromFilter.php
+ * Copyright (c) 2024 james@firefly-iii.org.
+ *
+ * This file is part of Firefly III (https://github.com/firefly-iii).
+ *
+ * This program is free software: you can redistribute it and/or modify
+ * it under the terms of the GNU Affero General Public License as
+ * published by the Free Software Foundation, either version 3 of the
+ * License, or (at your option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU Affero General Public License for more details.
+ *
+ * You should have received a copy of the GNU Affero General Public License
+ * along with this program.  If not, see https://www.gnu.org/licenses/.
+ */
+
+declare(strict_types=1);
+
+namespace FireflyIII\Support\Http\Api;
+
+use FireflyIII\Models\AccountType;
+use Illuminate\Support\Collection;
+
+trait CollectsAccountsFromFilter
+{
+    /**
+     * TODO Duplicate function but I think it belongs here or in a separate trait
+     *
+     */
+    private function getAccountList(array $queryParameters): Collection
+    {
+        $collection = new Collection();
+
+        // always collect from the query parameter, even when it's empty.
+        foreach ($queryParameters['accounts'] as $accountId) {
+            $account = $this->repository->find((int) $accountId);
+            if (null !== $account) {
+                $collection->push($account);
+            }
+        }
+
+        // if no "preselected", and found accounts
+        if ('empty' === $queryParameters['preselected'] && $collection->count() > 0) {
+            return $collection;
+        }
+        // if no preselected, but no accounts:
+        if ('empty' === $queryParameters['preselected'] && 0 === $collection->count()) {
+            $defaultSet = $this->repository->getAccountsByType([AccountType::ASSET, AccountType::DEFAULT])->pluck('id')->toArray();
+            $frontpage  = app('preferences')->get('frontpageAccounts', $defaultSet);
+
+            if (!(is_array($frontpage->data) && count($frontpage->data) > 0)) {
+                $frontpage->data = $defaultSet;
+                $frontpage->save();
+            }
+
+            return $this->repository->getAccountsById($frontpage->data);
+        }
+
+        // both options are overruled by "preselected"
+        if ('all' === $queryParameters['preselected']) {
+            return $this->repository->getAccountsByType([AccountType::ASSET, AccountType::DEFAULT, AccountType::LOAN, AccountType::DEBT, AccountType::MORTGAGE]);
+        }
+        if ('assets' === $queryParameters['preselected']) {
+            return $this->repository->getAccountsByType([AccountType::ASSET, AccountType::DEFAULT]);
+        }
+        if ('liabilities' === $queryParameters['preselected']) {
+            return $this->repository->getAccountsByType([AccountType::LOAN, AccountType::DEBT, AccountType::MORTGAGE]);
+        }
+
+        return $collection;
+    }
+}

routes/api.php

@@ -45,6 +45,24 @@ Route::group(
     }
 );
 
+// V2 API routes for charts
+Route::group(
+    [
+        'namespace' => 'FireflyIII\Api\V2\Controllers\Chart',
+        'prefix'    => 'v2/chart',
+        'as'        => 'api.v1.chart.',
+    ],
+    static function (): void {
+        Route::get('account/dashboard', ['uses' => 'AccountController@dashboard', 'as' => 'account.dashboard']);
+//        Route::get('budget/dashboard', ['uses' => 'BudgetController@dashboard', 'as' => 'budget.dashboard']);
+//        Route::get('category/dashboard', ['uses' => 'CategoryController@dashboard', 'as' => 'category.dashboard']);
+        Route::get('balance/balance', ['uses' => 'BalanceController@balance', 'as' => 'balance.balance']);
+    }
+);
+
+
+
+
 // JsonApiRoute::server('v3')
 //            ->prefix('v3')
 //            ->resources(function (ResourceRegistrar $server) {
@@ -106,20 +124,7 @@ Route::group(
     }
 );
 
-// V2 API routes for charts
+
-Route::group(
-    [
-        'namespace' => 'FireflyIII\Api\V2\Controllers\Chart',
-        'prefix'    => 'v2/chart',
-        'as'        => 'api.v1.chart.',
-    ],
-    static function (): void {
-        Route::get('account/dashboard', ['uses' => 'AccountController@dashboard', 'as' => 'account.dashboard']);
-        Route::get('budget/dashboard', ['uses' => 'BudgetController@dashboard', 'as' => 'budget.dashboard']);
-        Route::get('category/dashboard', ['uses' => 'CategoryController@dashboard', 'as' => 'category.dashboard']);
-        Route::get('balance/balance', ['uses' => 'BalanceController@balance', 'as' => 'balance.balance']);
-    }
-);
 
 // V2 API route for accounts.
 Route::group(