Clean up account taker amount inconsistencies.

app/Http/Controllers/AccountController.php

@@ -22,8 +22,8 @@ use FireflyIII\Http\Requests\AccountFormRequest;
 use FireflyIII\Models\Account;
 use FireflyIII\Models\AccountType;
 use FireflyIII\Models\Transaction;
+use FireflyIII\Models\TransactionType;
 use FireflyIII\Repositories\Account\AccountRepositoryInterface;
-use FireflyIII\Repositories\Account\AccountTaskerInterface;
 use FireflyIII\Repositories\Currency\CurrencyRepositoryInterface;
 use FireflyIII\Repositories\Journal\JournalRepositoryInterface;
 use FireflyIII\Support\CacheProperties;
@@ -326,7 +326,8 @@ class AccountController extends Controller
 
 
         return view(
-            'accounts.show', compact('account','currency', 'moment', 'accountType', 'periods', 'subTitleIcon', 'journals', 'subTitle', 'start', 'end', 'chartUri')
+            'accounts.show',
+            compact('account', 'currency', 'moment', 'accountType', 'periods', 'subTitleIcon', 'journals', 'subTitle', 'start', 'end', 'chartUri')
         );
     }
 
@@ -418,14 +419,11 @@ class AccountController extends Controller
     {
         /** @var AccountRepositoryInterface $repository */
         $repository = app(AccountRepositoryInterface::class);
-        /** @var AccountTaskerInterface $tasker */
-        $tasker = app(AccountTaskerInterface::class);
-
-        $start   = $repository->oldestJournalDate($account);
-        $range   = Preferences::get('viewRange', '1M')->data;
-        $start   = Navigation::startOfPeriod($start, $range);
-        $end     = Navigation::endOfX(new Carbon, $range);
-        $entries = new Collection;
+        $start      = $repository->oldestJournalDate($account);
+        $range      = Preferences::get('viewRange', '1M')->data;
+        $start      = Navigation::startOfPeriod($start, $range);
+        $end        = Navigation::endOfX(new Carbon, $range);
+        $entries    = new Collection;
 
         // properties for cache
         $cache = new CacheProperties;
@@ -438,19 +436,28 @@ class AccountController extends Controller
             return $cache->get(); // @codeCoverageIgnore
         }
 
-        // only include asset accounts when this account is an asset:
-        $assets = new Collection;
-        if (in_array($account->accountType->type, [AccountType::ASSET, AccountType::DEFAULT])) {
-            $assets = $repository->getAccountsByType([AccountType::ASSET, AccountType::DEFAULT]);
-        }
         Log::debug('Going to get period expenses and incomes.');
         while ($end >= $start) {
             $end        = Navigation::startOfPeriod($end, $range);
             $currentEnd = Navigation::endOfPeriod($end, $range);
-            $spent      = $tasker->amountOutInPeriod(new Collection([$account]), $assets, $end, $currentEnd);
-            $earned     = $tasker->amountInInPeriod(new Collection([$account]), $assets, $end, $currentEnd);
-            $dateStr    = $end->format('Y-m-d');
-            $dateName   = Navigation::periodShow($end, $range);
+
+            // try a collector for income:
+            /** @var JournalCollectorInterface $collector */
+            $collector = app(JournalCollectorInterface::class);
+            $collector->setAccounts(new Collection([$account]))->setRange($end, $currentEnd)
+                      ->setTypes([TransactionType::DEPOSIT])
+                      ->withOpposingAccount();
+            $earned = strval($collector->getJournals()->sum('transaction_amount'));
+
+            // try a collector for expenses:
+            /** @var JournalCollectorInterface $collector */
+            $collector = app(JournalCollectorInterface::class);
+            $collector->setAccounts(new Collection([$account]))->setRange($end, $currentEnd)
+                      ->setTypes([TransactionType::WITHDRAWAL])
+                      ->withOpposingAccount();
+            $spent    = strval($collector->getJournals()->sum('transaction_amount'));
+            $dateStr  = $end->format('Y-m-d');
+            $dateName = Navigation::periodShow($end, $range);
             $entries->push(
                 [
                     'string' => $dateStr,

app/Http/Controllers/Chart/ReportController.php

@@ -16,10 +16,12 @@ namespace FireflyIII\Http\Controllers\Chart;
 
 use Carbon\Carbon;
 use FireflyIII\Generator\Chart\Basic\GeneratorInterface;
+use FireflyIII\Helpers\Collector\JournalCollectorInterface;
 use FireflyIII\Http\Controllers\Controller;
-use FireflyIII\Repositories\Account\AccountTaskerInterface;
+use FireflyIII\Models\TransactionType;
 use FireflyIII\Support\CacheProperties;
 use Illuminate\Support\Collection;
+use Log;
 use Navigation;
 use Response;
 use Steam;
@@ -103,8 +105,9 @@ class ReportController extends Controller
         $cache->addProperty($accounts);
         $cache->addProperty($end);
         if ($cache->has()) {
-            return Response::json($cache->get()); // @codeCoverageIgnore
+            //return Response::json($cache->get()); // @codeCoverageIgnore
         }
+        Log::debug('Going to do operations for accounts ', $accounts->pluck('id')->toArray());
         $format    = Navigation::preferredCarbonLocalizedFormat($start, $end);
         $source    = $this->getChartData($accounts, $start, $end);
         $chartData = [
@@ -249,16 +252,31 @@ class ReportController extends Controller
             return $cache->get(); // @codeCoverageIgnore
         }
 
-
-        $tasker       = app(AccountTaskerInterface::class);
         $currentStart = clone $start;
         $spentArray   = [];
         $earnedArray  = [];
         while ($currentStart <= $end) {
-            $currentEnd          = Navigation::endOfPeriod($currentStart, '1M');
+
+            $currentEnd = Navigation::endOfPeriod($currentStart, '1M');
+
+            // try a collector for income:
+            /** @var JournalCollectorInterface $collector */
+            $collector = app(JournalCollectorInterface::class);
+            $collector->setAccounts($accounts)->setRange($currentStart, $currentEnd)
+                      ->setTypes([TransactionType::DEPOSIT, TransactionType::TRANSFER])
+                      ->withOpposingAccount();
+            $earned = strval($collector->getJournals()->sum('transaction_amount'));
+
+            // try a collector for expenses:
+            /** @var JournalCollectorInterface $collector */
+            $collector = app(JournalCollectorInterface::class);
+            $collector->setAccounts($accounts)->setRange($currentStart, $currentEnd)
+                      ->setTypes([TransactionType::WITHDRAWAL, TransactionType::TRANSFER])
+                      ->withOpposingAccount();
+            $spent = strval($collector->getJournals()->sum('transaction_amount'));
+
+
             $label               = $currentStart->format('Y-m') . '-01';
-            $spent               = $tasker->amountOutInPeriod($accounts, $accounts, $currentStart, $currentEnd);
-            $earned              = $tasker->amountInInPeriod($accounts, $accounts, $currentStart, $currentEnd);
             $spentArray[$label]  = bcmul($spent, '-1');
             $earnedArray[$label] = $earned;
             $currentStart        = Navigation::addPeriod($currentStart, '1M', 0);

app/Http/Controllers/JsonController.php

@@ -18,6 +18,7 @@ use Carbon\Carbon;
 use FireflyIII\Exceptions\FireflyException;
 use FireflyIII\Helpers\Collector\JournalCollectorInterface;
 use FireflyIII\Models\AccountType;
+use FireflyIII\Models\TransactionType;
 use FireflyIII\Repositories\Account\AccountRepositoryInterface;
 use FireflyIII\Repositories\Account\AccountTaskerInterface;
 use FireflyIII\Repositories\Bill\BillRepositoryInterface;
@@ -145,7 +146,7 @@ class JsonController extends Controller
      * @return \Illuminate\Http\JsonResponse
      *
      */
-    public function boxIn(AccountTaskerInterface $accountTasker, AccountRepositoryInterface $repository)
+    public function boxIn()
     {
         $start = session('start', Carbon::now()->startOfMonth());
         $end   = session('end', Carbon::now()->endOfMonth());
@@ -158,10 +159,16 @@ class JsonController extends Controller
         if ($cache->has()) {
             return Response::json($cache->get()); // @codeCoverageIgnore
         }
-        $accounts = $repository->getAccountsByType([AccountType::DEFAULT, AccountType::ASSET, AccountType::CASH]);
-        $assets   = $repository->getAccountsByType([AccountType::DEFAULT, AccountType::ASSET]);
-        $amount   = $accountTasker->amountInInPeriod($accounts, $assets, $start, $end);
-        $data     = ['box' => 'in', 'amount' => Amount::format($amount, false), 'amount_raw' => $amount];
+
+        // try a collector for income:
+        /** @var JournalCollectorInterface $collector */
+        $collector = app(JournalCollectorInterface::class);
+        $collector->setAllAssetAccounts()->setRange($start, $end)
+                  ->setTypes([TransactionType::DEPOSIT])
+                  ->withOpposingAccount();
+
+        $amount = strval($collector->getJournals()->sum('transaction_amount'));
+        $data   = ['box' => 'in', 'amount' => Amount::format($amount, false), 'amount_raw' => $amount];
         $cache->store($data);
 
         return Response::json($data);
@@ -173,7 +180,7 @@ class JsonController extends Controller
      *
      * @return \Symfony\Component\HttpFoundation\Response
      */
-    public function boxOut(AccountTaskerInterface $accountTasker, AccountRepositoryInterface $repository)
+    public function boxOut()
     {
         $start = session('start', Carbon::now()->startOfMonth());
         $end   = session('end', Carbon::now()->endOfMonth());
@@ -187,9 +194,13 @@ class JsonController extends Controller
             return Response::json($cache->get()); // @codeCoverageIgnore
         }
 
-        $accounts = $repository->getAccountsByType([AccountType::DEFAULT, AccountType::ASSET, AccountType::CASH]);
-        $assets   = $repository->getAccountsByType([AccountType::DEFAULT, AccountType::ASSET]);
-        $amount   = $accountTasker->amountOutInPeriod($accounts, $assets, $start, $end);
+        // try a collector for expenses:
+        /** @var JournalCollectorInterface $collector */
+        $collector = app(JournalCollectorInterface::class);
+        $collector->setAllAssetAccounts()->setRange($start, $end)
+                  ->setTypes([TransactionType::WITHDRAWAL])
+                  ->withOpposingAccount();
+        $amount = strval($collector->getJournals()->sum('transaction_amount'));
 
         $data = ['box' => 'out', 'amount' => Amount::format($amount, false), 'amount_raw' => $amount];
         $cache->store($data);

app/Repositories/Account/AccountTasker.php

@@ -31,64 +31,6 @@ class AccountTasker implements AccountTaskerInterface
     /** @var User */
     private $user;
 
-    /**
-     * @see self::amountInPeriod
-     *
-     * @param Collection $accounts
-     * @param Collection $excluded
-     * @param Carbon     $start
-     * @param Carbon     $end
-     *
-     * @return string
-     */
-    public function amountInInPeriod(Collection $accounts, Collection $excluded, Carbon $start, Carbon $end): string
-    {
-        $idList = [
-            'accounts' => $accounts->pluck('id')->toArray(),
-            'exclude'  => $excluded->pluck('id')->toArray(),
-        ];
-
-        Log::debug(
-            'Now calling amountInInPeriod.',
-            ['accounts' => $idList['accounts'], 'excluded' => $idList['exclude'],
-             'start'    => $start->format('Y-m-d'),
-             'end'      => $end->format('Y-m-d'),
-            ]
-        );
-
-        return $this->amountInPeriod($idList, $start, $end, true);
-
-    }
-
-    /**
-     * @see self::amountInPeriod
-     *
-     * @param Collection $accounts
-     * @param Collection $excluded
-     * @param Carbon     $start
-     * @param Carbon     $end
-     *
-     * @return string
-     */
-    public function amountOutInPeriod(Collection $accounts, Collection $excluded, Carbon $start, Carbon $end): string
-    {
-        $idList = [
-            'accounts' => $accounts->pluck('id')->toArray(),
-            'exclude'  => $excluded->pluck('id')->toArray(),
-        ];
-
-        Log::debug(
-            'Now calling amountOutInPeriod.',
-            ['accounts' => $idList['accounts'], 'excluded' => $idList['exclude'],
-             'start'    => $start->format('Y-m-d'),
-             'end'      => $end->format('Y-m-d'),
-            ]
-        );
-
-        return $this->amountInPeriod($idList, $start, $end, false);
-
-    }
-
     /**
      * @param Collection $accounts
      * @param Carbon     $start
@@ -155,63 +97,4 @@ class AccountTasker implements AccountTaskerInterface
         $this->user = $user;
     }
 
-    /**
-     * Will return how much money has been going out (ie. spent) by the given account(s).
-     * Alternatively, will return how much money has been coming in (ie. earned) by the given accounts.
-     *
-     * Enter $incoming=true for any money coming in (income)
-     * Enter $incoming=false  for any money going out (expenses)
-     *
-     * This means any money going out or in. You can also submit accounts to exclude,
-     * so transfers between accounts are not included.
-     *
-     * As a general rule:
-     *
-     * - Asset accounts should return both expenses and earnings. But could return 0.
-     * - Expense accounts (where money is spent) should only return earnings (the account gets money).
-     * - Revenue accounts (where money comes from) should only return expenses (they spend money).
-     *
-     *
-     *
-     * @param array  $accounts
-     * @param Carbon $start
-     * @param Carbon $end
-     * @param bool   $incoming
-     *
-     * @return string
-     */
-    protected function amountInPeriod(array $accounts, Carbon $start, Carbon $end, bool $incoming): string
-    {
-        $joinModifier = $incoming ? '<' : '>';
-        $selection    = $incoming ? '>' : '<';
-
-        $query = Transaction::distinct()
-                            ->leftJoin('transaction_journals', 'transaction_journals.id', '=', 'transactions.transaction_journal_id')
-                            ->leftJoin(
-                                'transactions as other_side', function (JoinClause $join) use ($joinModifier) {
-                                $join->on('transaction_journals.id', '=', 'other_side.transaction_journal_id')->where('other_side.amount', $joinModifier, 0);
-                            }
-                            )
-                            ->where('transaction_journals.date', '>=', $start->format('Y-m-d'))
-                            ->where('transaction_journals.date', '<=', $end->format('Y-m-d'))
-                            ->where('transaction_journals.user_id', $this->user->id)
-                            ->whereNull('transactions.deleted_at')
-                            ->whereNull('transaction_journals.deleted_at')
-                            ->whereIn('transactions.account_id', $accounts['accounts'])
-                            ->where('transactions.amount', $selection, 0);
-        if (count($accounts['exclude']) > 0) {
-            $query->whereNotIn('other_side.account_id', $accounts['exclude']);
-        }
-
-        $result = $query->get(['transactions.id', 'transactions.amount']);
-        $sum    = strval($result->sum('amount'));
-        if (strlen($sum) === 0) {
-            Log::debug('Sum is empty.');
-            $sum = '0';
-        }
-        Log::debug(sprintf('Result is %s', $sum));
-
-        return $sum;
-    }
-
 }

app/Repositories/Account/AccountTaskerInterface.php

@@ -24,30 +24,6 @@ use Illuminate\Support\Collection;
  */
 interface AccountTaskerInterface
 {
-    /**
-     * @param Collection $accounts
-     * @param Collection $excluded
-     * @param Carbon     $start
-     * @param Carbon     $end
-     *
-     * @see AccountTasker::amountInPeriod()
-     *
-     * @return string
-     */
-    public function amountInInPeriod(Collection $accounts, Collection $excluded, Carbon $start, Carbon $end): string;
-
-    /**
-     * @param Collection $accounts
-     * @param Collection $excluded
-     * @param Carbon     $start
-     * @param Carbon     $end
-     *
-     * @see AccountTasker::amountInPeriod()
-     *
-     * @return string
-     */
-    public function amountOutInPeriod(Collection $accounts, Collection $excluded, Carbon $start, Carbon $end): string;
-
     /**
      * @param Collection $accounts
      * @param Carbon     $start