From d313f5fdf53df93452d0e89b668c7ac01bec7fef Mon Sep 17 00:00:00 2001 From: James Cole Date: Thu, 26 Dec 2024 08:53:16 +0100 Subject: [PATCH] Fix chart balance. --- .../Controllers/Account/ShowController.php | 5 +- .../Controllers/Chart/AccountController.php | 171 +++++------------- app/Support/Steam.php | 25 ++- 3 files changed, 65 insertions(+), 136 deletions(-) diff --git a/app/Http/Controllers/Account/ShowController.php b/app/Http/Controllers/Account/ShowController.php index 471237d3e5..075f759323 100644 --- a/app/Http/Controllers/Account/ShowController.php +++ b/app/Http/Controllers/Account/ShowController.php @@ -173,6 +173,7 @@ class ShowController extends Controller $objectType = config(sprintf('firefly.shortNamesByFullName.%s', $account->accountType->type)); $end = today(config('app.timezone')); $today = today(config('app.timezone')); + $accountCurrency = $this->repository->getAccountCurrency($account); $start = $this->repository->oldestJournalDate($account) ?? today(config('app.timezone'))->startOfMonth(); $subTitleIcon = config('firefly.subIconsByIdentifier.'.$account->accountType->type); $page = (int) $request->get('page'); @@ -193,7 +194,7 @@ class ShowController extends Controller $groups->setPath(route('accounts.show.all', [$account->id])); $chartUrl = route('chart.account.period', [$account->id, $start->format('Y-m-d'), $end->format('Y-m-d')]); $showAll = true; - $balance = Steam::finalAccountBalance($account, $end)['balance']; + $balances = Steam::filterAccountBalance(Steam::finalAccountBalance($account, $end), $account, $this->convertToNative, $accountCurrency); return view( 'accounts.show', @@ -213,7 +214,7 @@ class ShowController extends Controller 'subTitle', 'start', 'end', - 'balance' + 'balances' ) ); } diff --git a/app/Http/Controllers/Chart/AccountController.php b/app/Http/Controllers/Chart/AccountController.php index 5e21d6a0b3..dc835a5bb8 100644 --- a/app/Http/Controllers/Chart/AccountController.php +++ b/app/Http/Controllers/Chart/AccountController.php @@ -430,9 +430,9 @@ class AccountController extends Controller } // collect and filter balances for the entire period. - $step = $this->calculateStep($start, $end); + $step = $this->calculateStep($start, $end); Log::debug(sprintf('Step is %s', $step)); - $locale = app('steam')->getLocale(); + $locale = app('steam')->getLocale(); $return = []; // fix for issue https://github.com/firefly-iii/firefly-iii/issues/8041 // have to make sure this chart is always based on the balance at the END of the period. @@ -445,45 +445,71 @@ class AccountController extends Controller Log::debug('One'); $range = Steam::finalAccountBalanceInRange($account, $start, $end, $this->convertToNative); Log::debug('Two'); - $range = Steam::filterAccountBalances($range, $account, $this->convertToNative, $accountCurrency); + $range = Steam::filterAccountBalances($range, $account, $this->convertToNative, $accountCurrency); Log::debug('Three'); - $previous = array_values($range)[0]; + + // temp, get end balance. + Log::debug('temp get end balance'); + Steam::finalAccountBalance($account, $end); + Log::debug('END temp get end balance done'); + + $previous = array_values($range)[0]; $accountCurrency = $accountCurrency ?? $this->defaultCurrency; // do this AFTER getting the balances. - while ($end >= $current) { + Log::debug('Start chart loop.'); + + $newRange = []; + $expectedIndex = 0; + Log::debug('Balances exist at:'); + foreach ($range as $key => $value) { + $newRange[] = ['date' => $key, 'info' => $value]; + Log::debug(sprintf(' - %s', $key)); + } + $carbon = Carbon::createFromFormat('Y-m-d', $newRange[0]['date']); + while ($end->gte($current)) { + $momentBalance = $previous; $theDate = $current->format('Y-m-d'); - // each day contains multiple balances, and this may even be different over time. - $momentBalance = $range[$theDate] ?? $previous; - $return = $this->updateChartKeys($return, $momentBalance); + while ($carbon->lte($current) && array_key_exists($expectedIndex, $newRange)) { + $momentBalance = $newRange[$expectedIndex]['info']; + Log::debug(sprintf('Expected index is %d!, date is %s, current is %s', $expectedIndex, $carbon->format('Y-m-d'), $current->format('Y-m-d'))); + $carbon = Carbon::createFromFormat('Y-m-d', $newRange[$expectedIndex]['date']); + $expectedIndex++; + } + + $return = $this->updateChartKeys($return, $momentBalance); + $previous = $momentBalance; + + Log::debug(sprintf('Now at %s', $theDate), $momentBalance); // process each balance thing. - foreach($momentBalance as $key => $amount) { - $label = $current->isoFormat($format); + foreach ($momentBalance as $key => $amount) { + $label = $current->isoFormat($format); $return[$key]['entries'][$label] = $amount; } - $current = app('navigation')->addPeriod($current, $step, 0); + $current = app('navigation')->addPeriod($current, $step, 0); // here too, to fix #8041, the data is corrected to the end of the period. $current = app('navigation')->endOfX($current, $step, null); - $previous = $momentBalance; + Log::debug(sprintf('Next moment is %s', $current->format('Y-m-d'))); } + Log::debug('End of chart loop.'); // second loop (yes) to create nice array with info! Yay! $chartData = []; - foreach($return as $key => $info) { - if(3 === strlen($key)) { + foreach ($return as $key => $info) { + if (3 === strlen($key)) { // assume it's a currency: - $setCurrency = $this->currencyRepository->findByCode($key); + $setCurrency = $this->currencyRepository->findByCode($key); $info['currency_symbol'] = $setCurrency->symbol; $info['currency_code'] = $setCurrency->code; $info['label'] = sprintf('%s (%s)', $account->name, $setCurrency->symbol); } - if('balance' === $key) { + if ('balance' === $key) { $info['currency_symbol'] = $accountCurrency->symbol; $info['currency_code'] = $accountCurrency->code; $info['label'] = sprintf('%s (%s)', $account->name, $accountCurrency->symbol); } - if('native_balance' === $key) { + if ('native_balance' === $key) { $info['currency_symbol'] = $this->defaultCurrency->symbol; $info['currency_code'] = $this->defaultCurrency->code; - $info['label'] = sprintf('%s (%s) (%s)', $account->name, (string)trans('firefly.sum'), $this->defaultCurrency->symbol); + $info['label'] = sprintf('%s (%s) (%s)', $account->name, (string) trans('firefly.sum'), $this->defaultCurrency->symbol); } $chartData[] = $info; } @@ -492,115 +518,6 @@ class AccountController extends Controller $cache->store($data); return response()->json($data); - - - - var_dump($chartData);exit; - - - - - $result = [ - 'label' => sprintf('%s (%s)', $account->name, $currency->symbol), - 'currency_symbol' => $currency->symbol, - 'currency_code' => $currency->code, - ]; - $entries = []; - $current = clone $start; - - - Log::debug(sprintf('$current date is %s', $current->format('Y-m-d'))); - if ('1D' === $step) { - // per day the entire period, balance for every day. - $format = (string) trans('config.month_and_day_js', [], $locale); - $range = app('steam')->finalAccountBalanceInRange($account, $start, $end, $this->convertToNative); - $previous = array_values($range)[0]; - while ($end >= $current) { - $theDate = $current->format('Y-m-d'); - $balance = $range[$theDate]['balance'] ?? $previous; - $label = $current->isoFormat($format); - $entries[$label] = (float) $balance; - $previous = $balance; - $current->addDay(); - } - } - if ('1W' === $step || '1M' === $step || '1Y' === $step) { - while ($end >= $current) { - Log::debug(sprintf('Current is: %s', $current->format('Y-m-d'))); - $balance = Steam::finalAccountBalance($account, $current)[$currency->code] ?? '0'; - $label = app('navigation')->periodShow($current, $step); - $entries[$label] = $balance; - $current = app('navigation')->addPeriod($current, $step, 0); - // here too, to fix #8041, the data is corrected to the end of the period. - $current = app('navigation')->endOfX($current, $step, null); - } - } - $result['entries'] = $entries; - - return $result; - - - /** @var TransactionCurrency $currency */ - foreach ($currencies as $currency) { - $chartData[] = $this->periodByCurrency($start, $end, $account, $currency); - } - - $data = $this->generator->multiSet($chartData); - $cache->store($data); - - return response()->json($data); - } - - /** - * @throws FireflyException - */ - private function periodByCurrency(Carbon $start, Carbon $end, Account $account, TransactionCurrency $currency): array - { - Log::debug(sprintf('Now in periodByCurrency("%s", "%s", %s, "%s")', $start->format('Y-m-d'), $end->format('Y-m-d'), $account->id, $currency->code)); - $locale = app('steam')->getLocale(); - $step = $this->calculateStep($start, $end); - $result = [ - 'label' => sprintf('%s (%s)', $account->name, $currency->symbol), - 'currency_symbol' => $currency->symbol, - 'currency_code' => $currency->code, - ]; - $entries = []; - $current = clone $start; - Log::debug(sprintf('Step is %s', $step)); - - // fix for issue https://github.com/firefly-iii/firefly-iii/issues/8041 - // have to make sure this chart is always based on the balance at the END of the period. - // This period depends on the size of the chart - $current = app('navigation')->endOfX($current, $step, null); - Log::debug(sprintf('$current date is %s', $current->format('Y-m-d'))); - if ('1D' === $step) { - // per day the entire period, balance for every day. - $format = (string) trans('config.month_and_day_js', [], $locale); - $range = app('steam')->finalAccountBalanceInRange($account, $start, $end, $this->convertToNative); - $previous = array_values($range)[0]; - while ($end >= $current) { - $theDate = $current->format('Y-m-d'); - $balance = $range[$theDate]['balance'] ?? $previous; - $label = $current->isoFormat($format); - $entries[$label] = (float) $balance; - $previous = $balance; - $current->addDay(); - } - } - if ('1W' === $step || '1M' === $step || '1Y' === $step) { - while ($end >= $current) { - Log::debug(sprintf('Current is: %s', $current->format('Y-m-d'))); - $balance = Steam::finalAccountBalance($account, $current)[$currency->code] ?? '0'; - $label = app('navigation')->periodShow($current, $step); - $entries[$label] = $balance; - $current = app('navigation')->addPeriod($current, $step, 0); - // here too, to fix #8041, the data is corrected to the end of the period. - $current = app('navigation')->endOfX($current, $step, null); - } - } - $result['entries'] = $entries; - - return $result; } /** diff --git a/app/Support/Steam.php b/app/Support/Steam.php index 09bce4d50a..25c7c20766 100644 --- a/app/Support/Steam.php +++ b/app/Support/Steam.php @@ -148,9 +148,19 @@ class Steam // if convert to native, if NOT convert to native. if($convertToNative) { - Log::debug(sprintf('Amount is %s %s, foreign amount is %s, native amount is %s', $entryCurrency->code, $modified, $foreignModified, $nativeModified)); + Log::debug(sprintf('Amount is %s %s, foreign amount is %s, native amount is %s', $entryCurrency->code, $this->bcround($modified,2), $this->bcround($foreignModified,2), $this->bcround($nativeModified,2))); + // if the currency is the default currency add to native balance + currency balance + if($entry->transaction_currency_id === $defaultCurrency->id) { + Log::debug('Add amount to native.'); + $currentBalance['native_balance'] = bcadd($currentBalance['native_balance'], $modified); + } + // add to native balance. - $currentBalance['native_balance'] = bcadd($currentBalance['native_balance'], $nativeModified); + if($entry->foreign_currency_id !== $defaultCurrency->id) { + // this check is not necessary, because if the foreign currency is the same as the default currency, the native amount is zero. + // so adding this would mean nothing. + $currentBalance['native_balance'] = bcadd($currentBalance['native_balance'], $nativeModified); + } if($entry->foreign_currency_id === $defaultCurrency->id) { $currentBalance['native_balance'] = bcadd($currentBalance['native_balance'], $foreignModified); } @@ -159,14 +169,14 @@ class Steam $currentBalance['balance'] = bcadd($currentBalance['balance'], $modified); } // add currency balance - $currentBalance[$entryCurrency->code] = bcadd($currentBalance[$entryCurrency->code], $modified); + $currentBalance[$entryCurrency->code] = bcadd($currentBalance[$entryCurrency->code] ?? '0', $modified); } if(!$convertToNative) { Log::debug(sprintf('Amount is %s %s, foreign amount is %s, native amount is %s', $entryCurrency->code, $modified, $foreignModified, $nativeModified)); // add to balance, as expected. $currentBalance['balance'] = bcadd($currentBalance['balance'] ?? '0', $modified); // add to GBP, as expected. - $currentBalance[$entryCurrency->code] = bcadd($currentBalance[$entryCurrency->code], $modified); + $currentBalance[$entryCurrency->code] = bcadd($currentBalance[$entryCurrency->code] ?? '0', $modified); } // // add "modified" to amount if the currency id matches the account currency id. @@ -190,6 +200,7 @@ class Steam Log::debug('Updated entry',$currentBalance); } $cache->store($balances); + Log::debug('End of method'); return $balances; } @@ -334,7 +345,7 @@ class Steam if ($native->id === $accountCurrency?->id) { $return['balance'] = bcadd('' === (string) $account->virtual_balance ? '0' : $account->virtual_balance, $return['balance']); } - Log::debug(sprintf('balance is (%s only) %s (with virtual balance)', $native->code, $return['balance'])); + Log::debug(sprintf('balance is (%s only) %s (with virtual balance)', $native->code, $this->bcround($return['balance'],2))); // native balance $return['native_balance'] = (string) $account->transactions() @@ -344,7 +355,7 @@ class Steam ->sum('transactions.native_amount'); // plus native virtual balance. $return['native_balance'] = bcadd('' === (string) $account->native_virtual_balance ? '0' : $account->native_virtual_balance, $return['native_balance']); - Log::debug(sprintf('native_balance is (all transactions to %s) %s (with virtual balance)', $native->code, $return['native_balance'])); + Log::debug(sprintf('native_balance is (all transactions to %s) %s (with virtual balance)', $native->code,$this->bcround( $return['native_balance']))); // plus foreign transactions in THIS currency. $sum = (string) $account->transactions() @@ -355,7 +366,7 @@ class Steam ->sum('transactions.foreign_amount'); $return['native_balance'] = bcadd($return['native_balance'], $sum); - Log::debug(sprintf('Foreign amount transactions add (%s only) %s, total native_balance is now %s', $native->code, $sum, $return['native_balance'])); + Log::debug(sprintf('Foreign amount transactions add (%s only) %s, total native_balance is now %s', $native->code, $this->bcround($sum), $this->bcround($return['native_balance']))); } // balance(s) in other (all) currencies.