Fixed the charts

app/Generator/Report/Category/MonthReportGenerator.php

@@ -38,6 +38,68 @@ class MonthReportGenerator implements ReportGeneratorInterface
     /** @var  Carbon */
     private $start;
 
+    /**
+     * @param Collection $collection
+     * @param array      $accounts
+     *
+     * @return Collection
+     */
+    public static function filterExpenses(Collection $collection, array $accounts): Collection
+    {
+        $result = $collection->filter(
+            function (Transaction $transaction) use ($accounts) {
+                $opposing = $transaction->opposing_account_id;
+                // remove internal transfer
+                if (in_array($opposing, $accounts)) {
+                    Log::debug(sprintf('Filtered #%d because its opposite is in accounts.', $transaction->id));
+
+                    return null;
+                }
+                // remove positive amount
+                if (bccomp($transaction->transaction_amount, '0') === 1) {
+                    Log::debug(sprintf('Filtered #%d because amount is %f.', $transaction->id, $transaction->transaction_amount));
+
+                    return null;
+                }
+
+                return $transaction;
+            }
+        );
+
+        return $result;
+    }
+
+    /**
+     * @param Collection $collection
+     * @param array      $accounts
+     *
+     * @return Collection
+     */
+    public static function filterIncome(Collection $collection, array $accounts): Collection
+    {
+        $result = $collection->filter(
+            function (Transaction $transaction) use ($accounts) {
+                $opposing = $transaction->opposing_account_id;
+                // remove internal transfer
+                if (in_array($opposing, $accounts)) {
+                    Log::debug(sprintf('Filtered #%d because its opposite is in accounts.', $transaction->id));
+
+                    return null;
+                }
+                // remove positive amount
+                if (bccomp($transaction->transaction_amount, '0') === -1) {
+                    Log::debug(sprintf('Filtered #%d because amount is %f.', $transaction->id, $transaction->transaction_amount));
+
+                    return null;
+                }
+
+                return $transaction;
+            }
+        );
+
+        return $result;
+    }
+
     /**
      * @return string
      */
@@ -186,7 +248,7 @@ class MonthReportGenerator implements ReportGeneratorInterface
      */
     private function getEarnedAccountSummary(): array
     {
-        $transactions = $this->getIncomes();
+        $transactions = $this->getIncome();
         $result       = [];
         /** @var Transaction $transaction */
         foreach ($transactions as $transaction) {
@@ -203,7 +265,7 @@ class MonthReportGenerator implements ReportGeneratorInterface
      */
     private function getEarnedCategorySummary(): array
     {
-        $transactions = $this->getIncomes();
+        $transactions = $this->getIncome();
         $result       = [];
         /** @var Transaction $transaction */
         foreach ($transactions as $transaction) {
@@ -230,26 +292,8 @@ class MonthReportGenerator implements ReportGeneratorInterface
 
         $accountIds   = $this->accounts->pluck('id')->toArray();
         $transactions = $collector->getJournals();
+        $transactions = self::filterExpenses($transactions, $accountIds);
 
-        $transactions = $transactions->filter(
-            function (Transaction $transaction) use ($accountIds) {
-                $opposing = $transaction->opposing_account_id;
-                // remove internal transfer
-                if (in_array($opposing, $accountIds)) {
-                    Log::debug(sprintf('Filtered #%d because its opposite is in accounts.', $transaction->id));
-
-                    return null;
-                }
-                // remove positive amount
-                if (bccomp($transaction->transaction_amount, '0') === 1) {
-                    Log::debug(sprintf('Filtered #%d because amount is %f.', $transaction->id, $transaction->transaction_amount));
-
-                    return null;
-                }
-
-                return $transaction;
-            }
-        );
 
         return $transactions;
     }
@@ -257,7 +301,7 @@ class MonthReportGenerator implements ReportGeneratorInterface
     /**
      * @return Collection
      */
-    private function getIncomes(): Collection
+    private function getIncome(): Collection
     {
         $collector = new JournalCollector(auth()->user());
         $collector->setAccounts($this->accounts)->setRange($this->start, $this->end)
@@ -265,25 +309,7 @@ class MonthReportGenerator implements ReportGeneratorInterface
                   ->setCategories($this->categories)->getOpposingAccount();
         $accountIds   = $this->accounts->pluck('id')->toArray();
         $transactions = $collector->getJournals();
-        $transactions = $transactions->filter(
+        $transactions = self::filterIncome($transactions, $accountIds);
-            function (Transaction $transaction) use ($accountIds) {
-                $opposing = $transaction->opposing_account_id;
-                // remove internal transfer
-                if (in_array($opposing, $accountIds)) {
-                    Log::debug(sprintf('Filtered #%d because its opposite is in accounts.', $transaction->id));
-
-                    return null;
-                }
-                // remove positive amount
-                if (bccomp($transaction->transaction_amount, '0') === -1) {
-                    Log::debug(sprintf('Filtered #%d because amount is %f.', $transaction->id, $transaction->transaction_amount));
-
-                    return null;
-                }
-
-                return $transaction;
-            }
-        );
 
         return $transactions;
     }

app/Http/Controllers/Chart/CategoryController.php

@@ -16,10 +16,12 @@ namespace FireflyIII\Http\Controllers\Chart;
 
 use Carbon\Carbon;
 use FireflyIII\Generator\Chart\Category\CategoryChartGeneratorInterface;
+use FireflyIII\Generator\Report\Category\MonthReportGenerator;
 use FireflyIII\Helpers\Collector\JournalCollector;
 use FireflyIII\Http\Controllers\Controller;
 use FireflyIII\Models\AccountType;
 use FireflyIII\Models\Category;
+use FireflyIII\Models\Transaction;
 use FireflyIII\Models\TransactionType;
 use FireflyIII\Repositories\Account\AccountRepositoryInterface;
 use FireflyIII\Repositories\Category\CategoryRepositoryInterface;
@@ -124,29 +126,50 @@ class CategoryController extends Controller
     {
         /** @var CategoryRepositoryInterface $repository */
         $repository = app(CategoryRepositoryInterface::class);
-        $others     = intval($others) === 1;
+        /** @var bool $others */
-        $names      = [];
+        $others = intval($others) === 1;
-        $collector  = new JournalCollector(auth()->user());
+        $names  = [];
-        $collector->setAccounts($accounts)->setRange($start, $end)
+
-                  ->setTypes([TransactionType::WITHDRAWAL])
+        // collect journals (just like the category report does):
-                  ->setCategories($categories);
+        $collector = new JournalCollector(auth()->user());
-        $set    = $collector->getSumPerCategory();
+        $collector->setAccounts($accounts)->setRange($start, $end)->setTypes([TransactionType::WITHDRAWAL, TransactionType::TRANSFER])
+                  ->setCategories($categories)->getOpposingAccount()->disableFilter();
+        $accountIds   = $accounts->pluck('id')->toArray();
+        $transactions = $collector->getJournals();
+        $set          = MonthReportGenerator::filterExpenses($transactions, $accountIds);
+
+        // group by category ID:
+        $grouped = [];
+        /** @var Transaction $transaction */
+        foreach ($set as $transaction) {
+            $jrnlCatId  = intval($transaction->transaction_journal_category_id);
+            $transCatId = intval($transaction->transaction_category_id);
+            $categoryId = max($jrnlCatId, $transCatId);
+
+            $grouped[$categoryId] = $grouped[$categoryId] ?? '0';
+            $amount               = bcmul($transaction->transaction_amount, '-1');
+            $grouped[$categoryId] = bcadd($amount, $grouped[$categoryId]);
+        }
+
+        // loop and show the grouped results:
         $result = [];
         $total  = '0';
-        foreach ($set as $categoryId => $amount) {
+        foreach ($grouped as $categoryId => $amount) {
             if (!isset($names[$categoryId])) {
                 $category           = $repository->find(intval($categoryId));
                 $names[$categoryId] = $category->name;
             }
-            $amount   = bcmul($amount, '-1');
             $total    = bcadd($total, $amount);
             $result[] = ['name' => $names[$categoryId], 'id' => $categoryId, 'amount' => $amount];
         }
 
+        // also collect others?
+        // TODO include transfers
         if ($others) {
             $collector = new JournalCollector(auth()->user());
             $collector->setAccounts($accounts)->setRange($start, $end)->setTypes([TransactionType::WITHDRAWAL]);
-            $sum      = bcmul($collector->getSum(), '-1');
+            $journals = $collector->getJournals();
+            $sum      = bcmul(strval($journals->sum('transaction_amount')), '-1');
             $sum      = bcsub($sum, $total);
             $result[] = ['name' => trans('firefly.everything_else'), 'id' => 0, 'amount' => $sum];
         }
@@ -214,14 +237,33 @@ class CategoryController extends Controller
         /** @var CategoryRepositoryInterface $repository */
         $repository = app(CategoryRepositoryInterface::class);
         /** @var bool $others */
-        $others    = intval($others) === 1;
+        $others = intval($others) === 1;
-        $names     = [];
+        $names  = [];
+
+        // collect journals (just like the category report does):
         $collector = new JournalCollector(auth()->user());
-        $collector->setAccounts($accounts)->setRange($start, $end)->setTypes([TransactionType::DEPOSIT])->setCategories($categories);
+        $collector->setAccounts($accounts)->setRange($start, $end)->setTypes([TransactionType::DEPOSIT, TransactionType::TRANSFER])
-        $set    = $collector->getSumPerCategory();
+                  ->setCategories($categories)->getOpposingAccount();
+        $accountIds   = $accounts->pluck('id')->toArray();
+        $transactions = $collector->getJournals();
+        $set          = MonthReportGenerator::filterIncome($transactions, $accountIds);
+
+        // group by category ID:
+        $grouped = [];
+        /** @var Transaction $transaction */
+        foreach ($set as $transaction) {
+            $jrnlCatId  = intval($transaction->transaction_journal_category_id);
+            $transCatId = intval($transaction->transaction_category_id);
+            $categoryId = max($jrnlCatId, $transCatId);
+
+            $grouped[$categoryId] = $grouped[$categoryId] ?? '0';
+            $grouped[$categoryId] = bcadd($transaction->transaction_amount, $grouped[$categoryId]);
+        }
+
+        // loop and show the grouped results:
         $result = [];
         $total  = '0';
-        foreach ($set as $categoryId => $amount) {
+        foreach ($grouped as $categoryId => $amount) {
             if (!isset($names[$categoryId])) {
                 $category           = $repository->find(intval($categoryId));
                 $names[$categoryId] = $category->name;
@@ -230,10 +272,13 @@ class CategoryController extends Controller
             $result[] = ['name' => $names[$categoryId], 'id' => $categoryId, 'amount' => $amount];
         }
 
+        // also collect others?
+        // TODO include transfers
         if ($others) {
             $collector = new JournalCollector(auth()->user());
             $collector->setAccounts($accounts)->setRange($start, $end)->setTypes([TransactionType::DEPOSIT]);
-            $sum      = $collector->getSum();
+            $journals = $collector->getJournals();
+            $sum      = strval($journals->sum('transaction_amount'));
             $sum      = bcsub($sum, $total);
             $result[] = ['name' => trans('firefly.everything_else'), 'id' => 0, 'amount' => $sum];
         }