Code cleanup and query optimisation.

app/controllers/TransactionController.php

@@ -162,6 +162,8 @@ class TransactionController extends BaseController
     }
 
     /**
+     * @SuppressWarnings("CyclomaticComplexity") // It's 7. More than 5 but alright.
+     *
      * @param $what
      *
      * @return $this

app/lib/FireflyIII/Event/Piggybank.php

@@ -180,6 +180,7 @@ class PiggyBank
     }
 
     /**
+     * @SuppressWarnings("CyclomaticComplexity") // It's 6. More than 5 but alright.
      *
      * Validates the presence of repetitions for all repeated expenses!
      */

app/lib/FireflyIII/Report/Report.php

@@ -7,7 +7,6 @@ use FireflyIII\Database\Account\Account as AccountRepository;
 use FireflyIII\Database\SwitchUser;
 use FireflyIII\Database\TransactionJournal\TransactionJournal as JournalRepository;
 use Illuminate\Database\Query\Builder;
-use Illuminate\Database\Query\JoinClause;
 use Illuminate\Support\Collection;
 
 /**
@@ -47,6 +46,7 @@ class Report implements ReportInterface
 
     /**
      * This methods fails to take in account transfers FROM shared accounts.
+     *
      * @param Carbon $start
      * @param Carbon $end
      * @param int    $limit
@@ -117,7 +117,7 @@ class Report implements ReportInterface
         $accounts = [];
         /** @var \Account $account */
         foreach ($list as $account) {
-            $id            = intval($account->id);
+            $id = intval($account->id);
             /** @noinspection PhpParamsInspection */
             $accounts[$id] = [
                 'name'         => $account->name,
@@ -232,8 +232,6 @@ class Report implements ReportInterface
      *
      * @SuppressWarnings(PHPMD.UnusedFormalParameter)
      *
-     * TODO: This method runs two queries which are only marginally different. Try and combine these.
-     *
      * @param Carbon $date
      * @param bool   $shared
      *
@@ -247,45 +245,48 @@ class Report implements ReportInterface
         $end->endOfMonth();
         $userId = $this->_accounts->getUser()->id;
 
-        $list = \TransactionJournal::leftJoin('transactions', 'transaction_journals.id', '=', 'transactions.transaction_journal_id')
+        return $this->_queries->incomeByPeriod($start, $end);
-                                   ->leftJoin('accounts', 'transactions.account_id', '=', 'accounts.id')
-                                   ->leftJoin(
-                                       'account_meta', function (JoinClause $join) {
-                                       $join->on('account_meta.account_id', '=', 'accounts.id')->where('account_meta.name', '=', 'accountRole');
-                                   }
-                                   )
-                                   ->transactionTypes(['Deposit'])
-                                   ->where('transaction_journals.user_id', $userId)
-                                   ->where('transactions.amount', '>', 0)
-                                   ->where('transaction_journals.user_id', \Auth::user()->id)
-                                   ->where('account_meta.data', '!=', '"sharedExpense"')
-                                   ->orderBy('date', 'ASC')
-                                   ->before($end)->after($start)->get(['transaction_journals.*']);
 
-        // incoming from a shared account: it's profit (income):
+        //        $list = \TransactionJournal::leftJoin('transactions', 'transaction_journals.id', '=', 'transactions.transaction_journal_id')
-        $transfers = \TransactionJournal::withRelevantData()
+        //                                   ->leftJoin('accounts', 'transactions.account_id', '=', 'accounts.id')
-                                        ->leftJoin('transactions', 'transaction_journals.id', '=', 'transactions.transaction_journal_id')
+        //                                   ->leftJoin(
-                                        ->leftJoin('accounts', 'transactions.account_id', '=', 'accounts.id')
+        //                                       'account_meta', function (JoinClause $join) {
-                                        ->leftJoin(
+        //                                       $join->on('account_meta.account_id', '=', 'accounts.id')->where('account_meta.name', '=', 'accountRole');
-                                            'account_meta', function (JoinClause $join) {
+        //                                   }
-                                            $join->on('account_meta.account_id', '=', 'accounts.id')->where('account_meta.name', '=', 'accountRole');
+        //                                   )
-                                        }
+        //                                   ->transactionTypes(['Deposit'])
-                                        )
+        //                                   ->where('transaction_journals.user_id', $userId)
-                                        ->transactionTypes(['Transfer'])
+        //                                   ->where('transactions.amount', '>', 0)
-                                        ->where('transaction_journals.user_id', $userId)
+        //                                   ->where('transaction_journals.user_id', \Auth::user()->id)
-                                        ->where('transactions.amount', '<', 0)
+        //                                   ->where('account_meta.data', '!=', '"sharedExpense"')
-                                        ->where('account_meta.data', '=', '"sharedExpense"')
+        //                                   ->orderBy('date', 'ASC')
-                                        ->orderBy('date', 'ASC')
+        //                                   ->before($end)->after($start)->get(['transaction_journals.*']);
-                                        ->before($end)->after($start)->get(['transaction_journals.*']);
+        //
+        //        // incoming from a shared account: it's profit (income):
+        //        $transfers = \TransactionJournal::withRelevantData()
+        //                                        ->leftJoin('transactions', 'transaction_journals.id', '=', 'transactions.transaction_journal_id')
+        //                                        ->leftJoin('accounts', 'transactions.account_id', '=', 'accounts.id')
+        //                                        ->leftJoin(
+        //                                            'account_meta', function (JoinClause $join) {
+        //                                            $join->on('account_meta.account_id', '=', 'accounts.id')->where('account_meta.name', '=', 'accountRole');
+        //                                        }
+        //                                        )
+        //                                        ->transactionTypes(['Transfer'])
+        //                                        ->where('transaction_journals.user_id', $userId)
+        //                                        ->where('transactions.amount', '<', 0)
+        //                                        ->where('account_meta.data', '=', '"sharedExpense"')
+        //                                        ->orderBy('date', 'ASC')
+        //                                        ->before($end)->after($start)->get(['transaction_journals.*']);
+        //
+        //        $list = $list->merge($transfers);
+        //        $list->sort(
+        //            function (\TransactionJournal $journal) {
+        //                return $journal->date->format('U');
+        //            }
+        //        );
+        //
+        //        return $list;
 
-        $list = $list->merge($transfers);
-        $list->sort(
-            function (\TransactionJournal $journal) {
-                return $journal->date->format('U');
-            }
-        );
-
-        return $list;
     }
 
     /**
@@ -302,21 +303,21 @@ class Report implements ReportInterface
 
         \PiggyBank::
         leftJoin('accounts', 'accounts.id', '=', 'piggy_banks.account_id')
-                         ->where('accounts.user_id', \Auth::user()->id)
+                  ->where('accounts.user_id', \Auth::user()->id)
-                         ->where('repeats', 0)
+                  ->where('repeats', 0)
-                         ->where(
+                  ->where(
-                             function (Builder $query) use ($start, $end) {
+                      function (Builder $query) use ($start, $end) {
-                                 $query->whereNull('piggy_banks.deleted_at');
+                          $query->whereNull('piggy_banks.deleted_at');
-                                 $query->orWhere(
+                          $query->orWhere(
-                                     function (Builder $query) use ($start, $end) {
+                              function (Builder $query) use ($start, $end) {
-                                         $query->whereNotNull('piggy_banks.deleted_at');
+                                  $query->whereNotNull('piggy_banks.deleted_at');
-                                         $query->where('piggy_banks.deleted_at', '>=', $start->format('Y-m-d 00:00:00'));
+                                  $query->where('piggy_banks.deleted_at', '>=', $start->format('Y-m-d 00:00:00'));
-                                         $query->where('piggy_banks.deleted_at', '<=', $end->format('Y-m-d 00:00:00'));
+                                  $query->where('piggy_banks.deleted_at', '<=', $end->format('Y-m-d 00:00:00'));
-                                     }
+                              }
-                                 );
+                          );
-                             }
+                      }
-                         )
+                  )
-                         ->get(['piggy_banks.*']);
+                  ->get(['piggy_banks.*']);
 
 
     }
@@ -372,7 +373,6 @@ class Report implements ReportInterface
         return $this->_queries->journalsByRevenueAccount($start, $end, $limit);
 
 
-
     }
 
     /**

app/lib/FireflyIII/Report/ReportQuery.php

@@ -199,6 +199,70 @@ class ReportQuery implements ReportQueryInterface
 
     }
 
+    /**
+     * This method returns all "income" journals in a certain period, which are both transfers from a shared account
+     * and "ordinary" deposits. The query used is almost equal to ReportQueryInterface::journalsByRevenueAccount but it does
+     * not group and returns different fields.
+     *
+     * @param Carbon $start
+     * @param Carbon $end
+     *
+     * @return Collection
+     */
+    public function incomeByPeriod(Carbon $start, Carbon $end)
+    {
+        return \TransactionJournal::
+        leftJoin(
+            'transactions as t_from', function (JoinClause $join) {
+            $join->on('t_from.transaction_journal_id', '=', 'transaction_journals.id')->where('t_from.amount', '<', 0);
+        }
+        )
+                                  ->leftJoin('accounts as ac_from', 't_from.account_id', '=', 'ac_from.id')
+                                  ->leftJoin(
+                                      'account_meta as acm_from', function (JoinClause $join) {
+                                      $join->on('ac_from.id', '=', 'acm_from.account_id')->where('acm_from.name', '=', 'accountRole');
+                                  }
+                                  )
+                                  ->leftJoin(
+                                      'transactions as t_to', function (JoinClause $join) {
+                                      $join->on('t_to.transaction_journal_id', '=', 'transaction_journals.id')->where('t_to.amount', '>', 0);
+                                  }
+                                  )
+                                  ->leftJoin('accounts as ac_to', 't_to.account_id', '=', 'ac_to.id')
+                                  ->leftJoin(
+                                      'account_meta as acm_to', function (JoinClause $join) {
+                                      $join->on('ac_to.id', '=', 'acm_to.account_id')->where('acm_to.name', '=', 'accountRole');
+                                  }
+                                  )
+                                  ->leftJoin('transaction_types', 'transaction_types.id', '=', 'transaction_journals.transaction_type_id')
+                                  ->where(
+                                      function ($query) {
+                                          $query->where(
+                                              function ($q) {
+                                                  $q->where('transaction_types.type', 'Deposit');
+                                                  $q->where('acm_to.data', '!=', '"sharedExpense"');
+                                              }
+                                          );
+                                          $query->orWhere(
+                                              function ($q) {
+                                                  $q->where('transaction_types.type', 'Transfer');
+                                                  $q->where('acm_from.data', '=', '"sharedExpense"');
+                                              }
+                                          );
+                                      }
+                                  )
+                                  ->before($end)->after($start)
+                                  ->where('transaction_journals.user_id', \Auth::user()->id)
+                                  ->groupBy('t_from.account_id')->orderBy('transaction_journals.date')
+                                  ->get(
+                                      ['transaction_journals.id',
+                                       'transaction_journals.description',
+                                       'transaction_types.type',
+                                       't_to.amount', 'transaction_journals.date', 't_from.account_id as account_id',
+                                       'ac_from.name as name']
+                                  );
+    }
+
     /**
      * Gets a list of expenses grouped by the budget they were filed under.
      *

app/lib/FireflyIII/Report/ReportQueryInterface.php

@@ -117,6 +117,18 @@ interface ReportQueryInterface
      */
     public function journalsByRevenueAccount(Carbon $start, Carbon $end);
 
+    /**
+     * This method returns all "income" journals in a certain period, which are both transfers from a shared account
+     * and "ordinary" deposits. The query used is almost equal to ReportQueryInterface::journalsByRevenueAccount but it does
+     * not group and returns different fields.
+     *
+     * @param Carbon $start
+     * @param Carbon $end
+     *
+     * @return Collection
+     */
+    public function incomeByPeriod(Carbon $start, Carbon $end);
+
     /**
      * With an equally misleading name, this query returns are transfers to shared accounts. These are considered
      * expenses.

app/lib/FireflyIII/Shared/Toolkit/Filter.php

@@ -174,37 +174,35 @@ class Filter
      */
     public function previous($range, Carbon $date)
     {
-        switch ($range) {
+        $functionMap = [
-            default:
+            '1D' => 'Day',
-                throw new FireflyException('Cannot do _previous() on ' . $range);
+            '1W' => 'Week',
-                break;
+            '1M' => 'Month',
-            case '1D':
+            '1Y' => 'Year'
-                $date->startOfDay()->subDay();
+        ];
-                break;
-            case '1W':
-                $date->startOfWeek()->subWeek();
-                break;
-            case '1M':
-                $date->startOfMonth()->subMonth();
-                break;
-            case '3M':
-                $date->firstOfQuarter()->subMonths(3)->firstOfQuarter();
-                break;
-            case '6M':
-                $month = intval($date->format('m'));
-                if ($month <= 6) {
-                    $date->startOfYear()->subMonths(6);
-                } else {
-                    $date->startOfYear();
-                }
-                break;
-            case '1Y':
-                $date->startOfYear()->subYear();
-                break;
 
+        if (isset($functionMap[$range])) {
+            $startFunction = 'startOf' . $functionMap[$range];
+            $subFunction   = 'sub' . $functionMap[$range];
+            $date->$startFunction()->$subFunction();
+
+            return $date;
         }
+        if ($range == '3M') {
+            $date->firstOfQuarter()->subMonths(3)->firstOfQuarter();
 
-        return $date;
+            return $date;
+        }
+        if ($range == '6M') {
+            $month = intval($date->format('m'));
+            $date->startOfYear();
+            if ($month <= 6) {
+                $date->subMonths(6);
+            }
+
+            return $date;
+        }
+        throw new FireflyException('Cannot do _previous() on ' . $range);
     }
 
     /**

app/views/reports/month.blade.php

@@ -5,7 +5,41 @@
     <div class="col-lg-5 col-md-5 col-sm-12">
         <div class="panel panel-default">
             <div class="panel-heading">Income</div>
-            @include('list.journals-small',['journals' => $income])
+                <table class="table table-bordered">
+                    <?php $tableSum = 0;?>
+                    @foreach($income as $entry)
+                        <tr>
+                            <td>
+                                <a href="{{route('transactions.show',$entry->id)}}" title="{{{$entry->description}}}">{{{$entry->description}}}</a>
+                            </td>
+                            <td>
+                                <?php $tableSum += floatval($entry->amount);?>
+                                @if($entry->type == 'Withdrawal')
+                                    <span class="text-danger">{{Amount::format($entry->amount,false)}}</span>
+                                @endif
+                                @if($entry->type == 'Deposit')
+                                    <span class="text-success">{{Amount::format($entry->amount,false)}}</span>
+                                @endif
+                                @if($entry->type == 'Transfer')
+                                    <span class="text-info">{{Amount::format($entry->amount,false)}}</span>
+                                @endif
+                            </td>
+                            <td>
+                                {{$entry->date->format('j F Y')}}
+                            </td>
+                            <td>
+                                <a href="{{route('accounts.show',$entry->account_id)}}">{{{$entry->name}}}</a>
+                            </td>
+                        </tr>
+                    @endforeach
+                    @if(isset($displaySum) && $displaySum === true)
+                        <tr>
+                            <td><em>Sum</em></td>
+                            <td colspan="3">{{Amount::format($tableSum)}}</td>
+
+                        </tr>
+                    @endif
+                </table>
         </div>
     </div>
     <div class="col-lg-4 col-md-4 col-sm-12">