From be868d37f2a124b6eacc29cdf2a7238892054234 Mon Sep 17 00:00:00 2001 From: James Cole Date: Thu, 19 Jan 2017 21:54:27 +0100 Subject: [PATCH] Fixed some issues with the monthly report and missing amounts. --- app/Helpers/Collection/Account.php | 107 ------------------ .../Account/AccountRepository.php | 37 ++++-- .../Account/AccountRepositoryInterface.php | 10 ++ app/Repositories/Account/AccountTasker.php | 77 +++++++------ .../Account/AccountTaskerInterface.php | 5 +- app/Support/Steam.php | 2 +- .../views/reports/partials/accounts.twig | 14 +-- 7 files changed, 88 insertions(+), 164 deletions(-) delete mode 100644 app/Helpers/Collection/Account.php diff --git a/app/Helpers/Collection/Account.php b/app/Helpers/Collection/Account.php deleted file mode 100644 index e3fc70e702..0000000000 --- a/app/Helpers/Collection/Account.php +++ /dev/null @@ -1,107 +0,0 @@ -accounts = new Collection; - } - - /** - * @return Collection - */ - public function getAccounts(): Collection - { - return $this->accounts; - } - - /** - * @param Collection $accounts - */ - public function setAccounts(Collection $accounts) - { - $this->accounts = $accounts; - } - - /** - * @return string - */ - public function getDifference(): string - { - return $this->difference; - } - - /** - * @param string $difference - */ - public function setDifference(string $difference) - { - $this->difference = $difference; - } - - /** - * @return string - */ - public function getEnd(): string - { - return $this->end; - } - - /** - * @param string $end - */ - public function setEnd(string $end) - { - $this->end = $end; - } - - /** - * @return string - */ - public function getStart(): string - { - return $this->start; - } - - /** - * @param string $start - */ - public function setStart(string $start) - { - $this->start = $start; - } - - -} diff --git a/app/Repositories/Account/AccountRepository.php b/app/Repositories/Account/AccountRepository.php index 3a8d8a3ecc..d740291f68 100644 --- a/app/Repositories/Account/AccountRepository.php +++ b/app/Repositories/Account/AccountRepository.php @@ -283,6 +283,29 @@ class AccountRepository implements AccountRepositoryInterface return $last; } + /** + * Returns the date of the very first transaction in this account. + * + * @param Account $account + * + * @return TransactionJournal + */ + public function oldestJournal(Account $account): TransactionJournal + { + $first = $account->transactions() + ->leftJoin('transaction_journals', 'transaction_journals.id', '=', 'transactions.transaction_journal_id') + ->orderBy('transaction_journals.date', 'ASC') + ->orderBy('transaction_journals.order', 'DESC') + ->where('transaction_journals.user_id', $this->user->id) + ->orderBy('transaction_journals.id', 'ASC') + ->first(['transaction_journals.id']); + if (!is_null($first)) { + return TransactionJournal::find(intval($first->id)); + } + + return new TransactionJournal(); + } + /** * Returns the date of the very first transaction in this account. * @@ -292,18 +315,12 @@ class AccountRepository implements AccountRepositoryInterface */ public function oldestJournalDate(Account $account): Carbon { - $first = new Carbon; - $date = $account->transactions() - ->leftJoin('transaction_journals', 'transaction_journals.id', '=', 'transactions.transaction_journal_id') - ->orderBy('transaction_journals.date', 'ASC') - ->orderBy('transaction_journals.order', 'DESC') - ->orderBy('transaction_journals.id', 'ASC') - ->first(['transaction_journals.date']); - if (!is_null($date)) { - $first = new Carbon($date->date); + $journal = $this->oldestJournal($account); + if (is_null($journal->id)) { + return new Carbon; } - return $first; + return $journal->date; } /** diff --git a/app/Repositories/Account/AccountRepositoryInterface.php b/app/Repositories/Account/AccountRepositoryInterface.php index 6aca02047e..428e48144d 100644 --- a/app/Repositories/Account/AccountRepositoryInterface.php +++ b/app/Repositories/Account/AccountRepositoryInterface.php @@ -15,6 +15,7 @@ namespace FireflyIII\Repositories\Account; use Carbon\Carbon; use FireflyIII\Models\Account; +use FireflyIII\Models\TransactionJournal; use Illuminate\Support\Collection; /** @@ -105,6 +106,15 @@ interface AccountRepositoryInterface */ public function newestJournalDate(Account $account): Carbon; + /** + * Returns the date of the very first transaction in this account. + * + * @param Account $account + * + * @return TransactionJournal + */ + public function oldestJournal(Account $account): TransactionJournal; + /** * Returns the date of the very first transaction in this account. * diff --git a/app/Repositories/Account/AccountTasker.php b/app/Repositories/Account/AccountTasker.php index 5a85a93201..d1300e6f1c 100644 --- a/app/Repositories/Account/AccountTasker.php +++ b/app/Repositories/Account/AccountTasker.php @@ -14,8 +14,6 @@ declare(strict_types = 1); namespace FireflyIII\Repositories\Account; use Carbon\Carbon; -use FireflyIII\Helpers\Collection\Account as AccountCollection; -use FireflyIII\Models\Account; use FireflyIII\Models\Transaction; use FireflyIII\User; use Illuminate\Database\Query\JoinClause; @@ -106,9 +104,9 @@ class AccountTasker implements AccountTaskerInterface * @param Carbon $start * @param Carbon $end * - * @return AccountCollection + * @return array */ - public function getAccountReport(Collection $accounts, Carbon $start, Carbon $end): AccountCollection + public function getAccountReport(Collection $accounts, Carbon $start, Carbon $end): array { $startAmount = '0'; $endAmount = '0'; @@ -116,45 +114,52 @@ class AccountTasker implements AccountTaskerInterface $ids = $accounts->pluck('id')->toArray(); $yesterday = clone $start; $yesterday->subDay(); - $startSet = Steam::balancesById($ids, $yesterday); - $backupSet = Steam::balancesById($ids, $start); - $endSet = Steam::balancesById($ids, $end); + $startSet = Steam::balancesById($ids, $yesterday); + $endSet = Steam::balancesById($ids, $end); - Log::debug( - sprintf( - 'getAccountReport from %s to %s for %d accounts.', - $start->format('Y-m-d'), - $end->format('Y-m-d'), - $accounts->count() - ) - ); - $accounts->each( - function (Account $account) use ($startSet, $endSet, $backupSet) { - $account->startBalance = $startSet[$account->id] ?? '0'; - $account->endBalance = $endSet[$account->id] ?? '0'; + Log::debug('Start of accountreport'); - // check backup set just in case: - if ($account->startBalance === '0' && isset($backupSet[$account->id])) { - $account->startBalance = $backupSet[$account->id]; - } - } - ); + /** @var AccountRepositoryInterface $repository */ + $repository = app(AccountRepositoryInterface::class); + + $return = [ + 'start' => '0', + 'end' => '0', + 'difference' => '0', + 'accounts' => [], + ]; - // summarize: foreach ($accounts as $account) { - $startAmount = bcadd($startAmount, $account->startBalance); - $endAmount = bcadd($endAmount, $account->endBalance); - $diff = bcadd($diff, bcsub($account->endBalance, $account->startBalance)); + $id = $account->id; + $entry = [ + 'name' => $account->name, + 'id' => $account->id, + 'start_balance' => '0', + 'end_balance' => '0', + ]; + + // get first journal date: + $first = $repository->oldestJournal($account); + $entry['start_balance'] = $startSet[$account->id] ?? '0'; + $entry['end_balance'] = $endSet[$account->id] ?? '0'; + if (!is_null($first->id) && $yesterday < $first->date && $end >= $first->date) { + // something about balance? + $entry['start_balance'] = $first->transactions()->where('account_id', $account->id)->first()->amount; + Log::debug(sprintf('Account was opened before %s, so opening balance is %f', $yesterday->format('Y-m-d'), $entry['start_balance'])); + } + $return['start'] = bcadd($return['start'], $entry['start_balance']); + $return['end'] = bcadd($return['end'], $entry['end_balance']); + + $return['accounts'][$id] = $entry; } - $object = new AccountCollection; - $object->setStart($startAmount); - $object->setEnd($endAmount); - $object->setDifference($diff); - $object->setAccounts($accounts); + foreach ($accounts as $account) { + $id = $account->id; + $diff = bcadd($diff, bcsub($return['accounts'][$id]['end_balance'], $return['accounts'][$id]['start_balance'])); + } + $return['difference'] = bcsub($return['end'], $return['start']); - - return $object; + return $return; } /** diff --git a/app/Repositories/Account/AccountTaskerInterface.php b/app/Repositories/Account/AccountTaskerInterface.php index 67ab246ed9..57fbebaa79 100644 --- a/app/Repositories/Account/AccountTaskerInterface.php +++ b/app/Repositories/Account/AccountTaskerInterface.php @@ -14,7 +14,6 @@ declare(strict_types = 1); namespace FireflyIII\Repositories\Account; use Carbon\Carbon; -use FireflyIII\Helpers\Collection\Account as AccountCollection; use Illuminate\Support\Collection; /** @@ -54,8 +53,8 @@ interface AccountTaskerInterface * @param Carbon $start * @param Carbon $end * - * @return AccountCollection + * @return array */ - public function getAccountReport(Collection $accounts, Carbon $start, Carbon $end): AccountCollection; + public function getAccountReport(Collection $accounts, Carbon $start, Carbon $end): array; } diff --git a/app/Support/Steam.php b/app/Support/Steam.php index 0e06913bca..0185a93caf 100644 --- a/app/Support/Steam.php +++ b/app/Support/Steam.php @@ -152,7 +152,7 @@ class Steam public function balancesById(array $ids, Carbon $date): array { - // abuse chart properties: + // cache this property. $cache = new CacheProperties; $cache->addProperty($ids); $cache->addProperty('balances'); diff --git a/resources/views/reports/partials/accounts.twig b/resources/views/reports/partials/accounts.twig index 3468f4c1d0..285e2a5d06 100644 --- a/resources/views/reports/partials/accounts.twig +++ b/resources/views/reports/partials/accounts.twig @@ -8,23 +8,23 @@ - {% for account in accountReport.getAccounts %} + {% for account in accountReport.accounts %} {{ account.name }} - {{ account.startBalance|formatAmount }} - {{ account.endBalance|formatAmount }} - {{ (account.endBalance - account.startBalance)|formatAmount }} + {{ account.start_balance|formatAmount }} + {{ account.end_balance|formatAmount }} + {{ (account.end_balance - account.start_balance)|formatAmount }} {% endfor %} {{ 'sumOfSums'|_ }} - {{ accountReport.getStart|formatAmount }} - {{ accountReport.getEnd|formatAmount }} - {{ accountReport.getDifference|formatAmount }} + {{ accountReport.start|formatAmount }} + {{ accountReport.end|formatAmount }} + {{ accountReport.difference|formatAmount }}