kenmirrors/firefly-iii
1
0
Fork 0
mirror of https://github.com/firefly-iii/firefly-iii.git synced 2025-09-21 03:39:00 +00:00
Code Issues Projects Releases Wiki Activity

Fix recurring transactions create

Browse Source 
- If there's a lot of accounts to calculate balances for, then recurring transactions create page
  doesn't load. Partly because it has to calculate a lot of balances, but partly because the cache
  isn't being used at all because date is `new Date` rather than say, end of month.

Fix: Change Steam balance calculator to always default cache using end of month. Since cache is
'invalidated' upon any edit, there's no reason to use current datetime anywhere its not explicitly
required by user flow.
Fix: Don't calculate balances for revenue / expense accounts since those are unbounded.

Issue: #3597
This commit is contained in:
Aniruddha Maru
2020-07-25 14:45:40 -07:00
parent 4b16d7c53d
commit 9d409a7412
3 changed files with 76 additions and 166 deletions
Download Patch File Download Diff File Expand all files Collapse all files

4
app/Http/Controllers/Transaction/ConvertController.php
View File

@@ -260,7 +260,7 @@ class ConvertController extends Controller
// group accounts: // group accounts:
/** @var Account $account */ /** @var Account $account */
foreach ($accountList as $account) { foreach ($accountList as $account) {
$balance = app('steam')->balance($account, new Carbon); $balance = app('steam')->balance($account);
$currency = $repository->getAccountCurrency($account) ?? $defaultCurrency; $currency = $repository->getAccountCurrency($account) ?? $defaultCurrency;
$role = (string) $repository->getMetaValue($account, 'account_role'); $role = (string) $repository->getMetaValue($account, 'account_role');
if ('' === $role) { if ('' === $role) {
@@ -289,7 +289,7 @@ class ConvertController extends Controller
// group accounts: // group accounts:
/** @var Account $account */ /** @var Account $account */
foreach ($accountList as $account) { foreach ($accountList as $account) {
$balance = app('steam')->balance($account, new Carbon); $balance = app('steam')->balance($account);
$currency = $repository->getAccountCurrency($account) ?? $defaultCurrency; $currency = $repository->getAccountCurrency($account) ?? $defaultCurrency;
$role = 'l_' . $account->accountType->type; $role = 'l_' . $account->accountType->type;
$key = (string) trans('firefly.opt_group_' . $role); $key = (string) trans('firefly.opt_group_' . $role);

202
app/Support/Form/AccountForm.php
View File

@@ -26,6 +26,8 @@ namespace FireflyIII\Support\Form;
use FireflyIII\Models\Account; use FireflyIII\Models\Account;
use FireflyIII\Models\AccountType; use FireflyIII\Models\AccountType;
use FireflyIII\Repositories\Account\AccountRepositoryInterface;
use Illuminate\Support\Collection;
use Log; use Log;
use Throwable; use Throwable;
@@ -41,6 +43,46 @@ class AccountForm
{ {
use FormSupport; use FormSupport;
private function getAccountsGrouped(array $types, AccountRepositoryInterface $repository = null): array
{
if (null === $repository) {
$repository = $this->getAccountRepository();
}
$accountList = $repository->getActiveAccountsByType($types);
$liabilityTypes = [AccountType::MORTGAGE, AccountType::DEBT, AccountType::CREDITCARD, AccountType::LOAN,];
$balanceTypes = [AccountType::ASSET, AccountType::DEFAULT, AccountType::MORTGAGE, AccountType::DEBT, AccountType::CREDITCARD, AccountType::LOAN,];
$defaultCurrency = app('amount')->getDefaultCurrency();
$grouped = [];
/** @var Account $account */
foreach ($accountList as $account) {
$accountWithBalance = $account->name;
if (in_array($account->accountType->type, $balanceTypes, true)) {
$balance = app('steam')->balance($account);
$currency = $repository->getAccountCurrency($account) ?? $defaultCurrency;
$formatted = app('amount')->formatAnything($currency, $balance, false);
$accountWithBalance = sprintf('%s (%s)', $account->name, $formatted);
}
$role = (string)$repository->getMetaValue($account, 'account_role');
if (in_array($account->accountType->type, $liabilityTypes, true)) {
$role = sprintf('l_%s', $account->accountType->type);
} elseif ('' === $role) {
if (AccountType::EXPENSE === $account->accountType->type) {
$role = 'expense_account';
} elseif (AccountType::REVENUE === $account->accountType->type) {
$role = 'revenue_account';
} else {
$role = 'no_account_type';
}
}
$key = (string)trans(sprintf('firefly.opt_group_%s', $role));
$grouped[$key][$account->id] = $accountWithBalance;
}
return $grouped;
}
/** /**
* Shows a <select> with all active asset accounts. * Shows a <select> with all active asset accounts.
* *
@@ -52,22 +94,8 @@ class AccountForm
*/ */
public function activeAssetAccountList(string $name, $value = null, array $options = null): string public function activeAssetAccountList(string $name, $value = null, array $options = null): string
{ {
$repository = $this->getAccountRepository(); $types = [AccountType::ASSET, AccountType::DEFAULT];
$accountList = $repository->getActiveAccountsByType([AccountType::ASSET, AccountType::DEFAULT]); $grouped = $this->getAccountsGrouped($types);
$defaultCurrency = app('amount')->getDefaultCurrency();
$grouped = [];
$date = $this->getDate();
/** @var Account $account */
foreach ($accountList as $account) {
$balance = app('steam')->balance($account, $date);
$role = $repository->getMetaValue($account, 'account_role');
$currency = $repository->getAccountCurrency($account) ?? $defaultCurrency;
$role = '' === $role ? 'no_account_type' : $role;
$key = (string)trans(sprintf('firefly.opt_group_%s', $role));
$formatted = app('amount')->formatAnything($currency, $balance, false);
$grouped[$key][$account->id] = sprintf('%s (%s)', $account->name, $formatted);
}
return $this->select($name, $grouped, $value, $options); return $this->select($name, $grouped, $value, $options);
} }
@@ -85,32 +113,7 @@ class AccountForm
public function activeLongAccountList(string $name, $value = null, array $options = null): string public function activeLongAccountList(string $name, $value = null, array $options = null): string
{ {
$types = [AccountType::ASSET, AccountType::DEFAULT, AccountType::MORTGAGE, AccountType::DEBT, AccountType::CREDITCARD, AccountType::LOAN,]; $types = [AccountType::ASSET, AccountType::DEFAULT, AccountType::MORTGAGE, AccountType::DEBT, AccountType::CREDITCARD, AccountType::LOAN,];
$liabilityTypes = [AccountType::MORTGAGE, AccountType::DEBT, AccountType::CREDITCARD, AccountType::LOAN]; $grouped = $this->getAccountsGrouped($types);
$repository = $this->getAccountRepository();
$accountList = $repository->getActiveAccountsByType($types);
$defaultCurrency = app('amount')->getDefaultCurrency();
$grouped = [];
$date = $this->getDate();
/** @var Account $account */
foreach ($accountList as $account) {
$balance = app('steam')->balance($account, $date);
$currency = $repository->getAccountCurrency($account) ?? $defaultCurrency;
$role = $repository->getMetaValue($account, 'account_role');
if ('' === $role && !in_array($account->accountType->type, $liabilityTypes, true)) {
$role = 'no_account_type';
}
if (in_array($account->accountType->type, $liabilityTypes, true)) {
$role = sprintf('l_%s', $account->accountType->type);
}
$key = (string)trans(sprintf('firefly.opt_group_%s', $role));
$formatted = app('amount')->formatAnything($currency, $balance, false);
$grouped[$key][$account->id] = sprintf('%s (%s)', $account->name, $formatted);
}
return $this->select($name, $grouped, $value, $options); return $this->select($name, $grouped, $value, $options);
} }
@@ -127,39 +130,13 @@ class AccountForm
public function activeWithdrawalDestinations(string $name, $value = null, array $options = null): string public function activeWithdrawalDestinations(string $name, $value = null, array $options = null): string
{ {
$types = [AccountType::MORTGAGE, AccountType::DEBT, AccountType::CREDITCARD, AccountType::LOAN, AccountType::EXPENSE,]; $types = [AccountType::MORTGAGE, AccountType::DEBT, AccountType::CREDITCARD, AccountType::LOAN, AccountType::EXPENSE,];
$liabilityTypes = [AccountType::MORTGAGE, AccountType::DEBT, AccountType::CREDITCARD, AccountType::LOAN];
$repository = $this->getAccountRepository(); $repository = $this->getAccountRepository();
$accountList = $repository->getActiveAccountsByType($types); $grouped = $this->getAccountsGrouped($types, $repository);
$defaultCurrency = app('amount')->getDefaultCurrency();
$grouped = [];
$date = $this->getDate();
$cash = $repository->getCashAccount(); $cash = $repository->getCashAccount();
$key = (string)trans('firefly.cash_account_type'); $key = (string)trans('firefly.cash_account_type');
$grouped[$key][$cash->id] = sprintf('(%s)', (string)trans('firefly.cash')); $grouped[$key][$cash->id] = sprintf('(%s)', (string)trans('firefly.cash'));
// group accounts:
/** @var Account $account */
foreach ($accountList as $account) {
$balance = app('steam')->balance($account, $date);
$currency = $repository->getAccountCurrency($account) ?? $defaultCurrency;
$role = (string)$repository->getMetaValue($account, 'account_role');
if ('' === $role && !in_array($account->accountType->type, $liabilityTypes, true)) {
$role = 'no_account_type';
}
if ('no_account_type' === $role && AccountType::EXPENSE === $account->accountType->type) {
$role = 'expense_account';
}
if (in_array($account->accountType->type, $liabilityTypes, true)) {
$role = sprintf('l_%s', $account->accountType->type);
}
$key = (string)trans('firefly.opt_group_' . $role);
$formatted = app('amount')->formatAnything($currency, $balance, false);
$grouped[$key][$account->id] = sprintf('%s (%s)', $account->name, $formatted);
}
return $this->select($name, $grouped, $value, $options); return $this->select($name, $grouped, $value, $options);
} }
@@ -175,36 +152,13 @@ class AccountForm
public function activeDepositDestinations(string $name, $value = null, array $options = null): string public function activeDepositDestinations(string $name, $value = null, array $options = null): string
{ {
$types = [AccountType::MORTGAGE, AccountType::DEBT, AccountType::CREDITCARD, AccountType::LOAN, AccountType::REVENUE,]; $types = [AccountType::MORTGAGE, AccountType::DEBT, AccountType::CREDITCARD, AccountType::LOAN, AccountType::REVENUE,];
$liabilityTypes = [AccountType::MORTGAGE, AccountType::DEBT, AccountType::CREDITCARD, AccountType::LOAN];
$repository = $this->getAccountRepository(); $repository = $this->getAccountRepository();
$accountList = $repository->getActiveAccountsByType($types); $grouped = $this->getAccountsGrouped($types, $repository);
$defaultCurrency = app('amount')->getDefaultCurrency();
$grouped = [];
$date = $this->getDate();
$cash = $repository->getCashAccount(); $cash = $repository->getCashAccount();
$key = (string)trans('firefly.cash_account_type'); $key = (string)trans('firefly.cash_account_type');
$grouped[$key][$cash->id] = sprintf('(%s)', (string)trans('firefly.cash')); $grouped[$key][$cash->id] = sprintf('(%s)', (string)trans('firefly.cash'));
// group accounts:
/** @var Account $account */
foreach ($accountList as $account) {
$balance = app('steam')->balance($account, $date);
$currency = $repository->getAccountCurrency($account) ?? $defaultCurrency;
$role = (string)$repository->getMetaValue($account, 'account_role');
if ('' === $role && !in_array($account->accountType->type, $liabilityTypes, true)) {
$role = 'no_account_type';
}
if ('no_account_type' === $role && AccountType::REVENUE === $account->accountType->type) {
$role = 'revenue_account';
}
if (in_array($account->accountType->type, $liabilityTypes, true)) {
$role = sprintf('l_%s', $account->accountType->type); // @codeCoverageIgnore
}
$key = (string)trans(sprintf('firefly.opt_group_%s', $role));
$formatted = app('amount')->formatAnything($currency, $balance, false);
$grouped[$key][$account->id] = sprintf('%s (%s)', $account->name, $formatted);
}
return $this->select($name, $grouped, $value, $options); return $this->select($name, $grouped, $value, $options);
} }
@@ -227,24 +181,8 @@ class AccountForm
$selected = request()->old($name) ?? []; $selected = request()->old($name) ?? [];
// get all asset accounts: // get all asset accounts:
$repository = $this->getAccountRepository();
$types = [AccountType::ASSET, AccountType::DEFAULT, AccountType::LOAN, AccountType::MORTGAGE, AccountType::DEBT]; $types = [AccountType::ASSET, AccountType::DEFAULT, AccountType::LOAN, AccountType::MORTGAGE, AccountType::DEBT];
$assetAccounts = $repository->getAccountsByType($types); $grouped = $this->getAccountsGrouped($types);
$grouped = [];
// group accounts:
/** @var Account $account */
foreach ($assetAccounts as $account) {
$role = $repository->getMetaValue($account, 'account_role');
if (null === $role && in_array($account->accountType->type, [AccountType::LOAN, AccountType::MORTGAGE, AccountType::DEBT], true)) {
$role = sprintf('l_%s', $account->accountType->type);
}
if (null === $role && !in_array($account->accountType->type, [AccountType::LOAN, AccountType::MORTGAGE, AccountType::DEBT], true)) {
$role = 'no_account_type';
}
$key = (string)trans(sprintf('firefly.opt_group_%s', $role));
$grouped[$key][$account->id] = $account->name;
}
unset($options['class']); unset($options['class']);
try { try {
@@ -268,26 +206,8 @@ class AccountForm
*/ */
public function assetAccountList(string $name, $value = null, array $options = null): string public function assetAccountList(string $name, $value = null, array $options = null): string
{ {
$repository = $this->getAccountRepository();
$types = [AccountType::ASSET, AccountType::DEFAULT]; $types = [AccountType::ASSET, AccountType::DEFAULT];
$accountList = $repository->getAccountsByType($types); $grouped = $this->getAccountsGrouped($types);
$defaultCurrency = app('amount')->getDefaultCurrency();
$grouped = [];
$date = $this->getDate();
/** @var Account $account */
foreach ($accountList as $account) {
$balance = app('steam')->balance($account, $date);
$currency = $repository->getAccountCurrency($account) ?? $defaultCurrency;
$role = (string)$repository->getMetaValue($account, 'account_role');
if ('' === $role) {
$role = 'no_account_type';
}
$key = (string)trans(sprintf('firefly.opt_group_%s', $role));
$formatted = app('amount')->formatAnything($currency, $balance, false);
$grouped[$key][$account->id] = sprintf('%s (%s)', $account->name, $formatted);
}
return $this->select($name, $grouped, $value, $options); return $this->select($name, $grouped, $value, $options);
} }
@@ -305,27 +225,7 @@ class AccountForm
public function longAccountList(string $name, $value = null, array $options = null): string public function longAccountList(string $name, $value = null, array $options = null): string
{ {
$types = [AccountType::ASSET, AccountType::DEFAULT, AccountType::MORTGAGE, AccountType::DEBT, AccountType::CREDITCARD, AccountType::LOAN,]; $types = [AccountType::ASSET, AccountType::DEFAULT, AccountType::MORTGAGE, AccountType::DEBT, AccountType::CREDITCARD, AccountType::LOAN,];
$liabilityTypes = [AccountType::MORTGAGE, AccountType::DEBT, AccountType::CREDITCARD, AccountType::LOAN]; $grouped = $this->getAccountsGrouped($types);
$repository = $this->getAccountRepository();
$accountList = $repository->getAccountsByType($types);
$defaultCurrency = app('amount')->getDefaultCurrency();
$grouped = [];
$date = $this->getDate();
/** @var Account $account */
foreach ($accountList as $account) {
$balance = app('steam')->balance($account, $date);
$currency = $repository->getAccountCurrency($account) ?? $defaultCurrency;
$role = (string)$repository->getMetaValue($account, 'account_role');
if ('' === $role) {
$role = 'no_account_type';
}
if (in_array($account->accountType->type, $liabilityTypes, true)) {
$role = sprintf('l_%s', $account->accountType->type);
}
$key = (string)trans(sprintf('firefly.opt_group_%s', $role));
$formatted = app('amount')->formatAnything($currency, $balance, false);
$grouped[$key][$account->id] = sprintf('%s (%s)', $account->name, $formatted);
}
return $this->select($name, $grouped, $value, $options); return $this->select($name, $grouped, $value, $options);
} }

22
app/Support/Steam.php
View File

@@ -40,12 +40,14 @@ class Steam
{ {
/** /**
* Gets balance at the end of current month by default
*
* @param \FireflyIII\Models\Account $account * @param \FireflyIII\Models\Account $account
* @param \Carbon\Carbon $date * @param \Carbon\Carbon $date
* *
* @return string * @return string
*/ */
public function balance(Account $account, Carbon $date, ?TransactionCurrency $currency = null): string public function balance(Account $account, Carbon $date = null, ?TransactionCurrency $currency = null): string
{ {
if ('testing' === config('app.env')) { if ('testing' === config('app.env')) {
Log::warning(sprintf('%s should NOT be called in the TEST environment!', __METHOD__)); Log::warning(sprintf('%s should NOT be called in the TEST environment!', __METHOD__));
@@ -58,6 +60,9 @@ class Steam
if ($cache->has()) { if ($cache->has()) {
return $cache->get(); // @codeCoverageIgnore return $cache->get(); // @codeCoverageIgnore
} }
if (null === $date) {
$date = Carbon::now()->endOfMonth();
}
/** @var AccountRepositoryInterface $repository */ /** @var AccountRepositoryInterface $repository */
$repository = app(AccountRepositoryInterface::class); $repository = app(AccountRepositoryInterface::class);
if (null === $currency) { if (null === $currency) {
@@ -255,12 +260,14 @@ class Steam
} }
/** /**
* Gets balance at the end of current month by default
*
* @param \FireflyIII\Models\Account $account * @param \FireflyIII\Models\Account $account
* @param \Carbon\Carbon $date * @param \Carbon\Carbon $date
* *
* @return array * @return array
*/ */
public function balancePerCurrency(Account $account, Carbon $date): array public function balancePerCurrency(Account $account, Carbon $date = null): array
{ {
if ('testing' === config('app.env')) { if ('testing' === config('app.env')) {
Log::warning(sprintf('%s should NOT be called in the TEST environment!', __METHOD__)); Log::warning(sprintf('%s should NOT be called in the TEST environment!', __METHOD__));
@@ -273,6 +280,9 @@ class Steam
if ($cache->has()) { if ($cache->has()) {
return $cache->get(); // @codeCoverageIgnore return $cache->get(); // @codeCoverageIgnore
} }
if (null === $date) {
$date = Carbon::now()->endOfMonth();
}
$query = $account->transactions() $query = $account->transactions()
->leftJoin('transaction_journals', 'transaction_journals.id', '=', 'transactions.transaction_journal_id') ->leftJoin('transaction_journals', 'transaction_journals.id', '=', 'transactions.transaction_journal_id')
->where('transaction_journals.date', '<=', $date->format('Y-m-d 23:59:59')) ->where('transaction_journals.date', '<=', $date->format('Y-m-d 23:59:59'))
@@ -289,14 +299,14 @@ class Steam
} }
/** /**
* This method always ignores the virtual balance. * This method always ignores the virtual balance. Gets balance at the end of current month by default
* *
* @param \Illuminate\Support\Collection $accounts * @param \Illuminate\Support\Collection $accounts
* @param \Carbon\Carbon $date * @param \Carbon\Carbon $date
* *
* @return array * @return array
*/ */
public function balancesByAccounts(Collection $accounts, Carbon $date): array public function balancesByAccounts(Collection $accounts, Carbon $date = null): array
{ {
if ('testing' === config('app.env')) { if ('testing' === config('app.env')) {
Log::warning(sprintf('%s should NOT be called in the TEST environment!', __METHOD__)); Log::warning(sprintf('%s should NOT be called in the TEST environment!', __METHOD__));
@@ -324,14 +334,14 @@ class Steam
} }
/** /**
* Same as above, but also groups per currency. * Same as above, but also groups per currency. Gets balance at the end of current month by default
* *
* @param \Illuminate\Support\Collection $accounts * @param \Illuminate\Support\Collection $accounts
* @param \Carbon\Carbon $date * @param \Carbon\Carbon $date
* *
* @return array * @return array
*/ */
public function balancesPerCurrencyByAccounts(Collection $accounts, Carbon $date): array public function balancesPerCurrencyByAccounts(Collection $accounts, Carbon $date = null): array
{ {
if ('testing' === config('app.env')) { if ('testing' === config('app.env')) {
Log::warning(sprintf('%s should NOT be called in the TEST environment!', __METHOD__)); Log::warning(sprintf('%s should NOT be called in the TEST environment!', __METHOD__));