Offload stuff to repositories for rules and rule groups, see issue #185

app/Http/Controllers/RuleController.php

@@ -11,6 +11,7 @@ declare(strict_types = 1);
 namespace FireflyIII\Http\Controllers;
 
 use Auth;
+use Config;
 use FireflyIII\Http\Requests\RuleFormRequest;
 use FireflyIII\Models\Rule;
 use FireflyIII\Models\RuleAction;
@@ -19,14 +20,12 @@ use FireflyIII\Models\RuleTrigger;
 use FireflyIII\Repositories\Rule\RuleRepositoryInterface;
 use FireflyIII\Repositories\RuleGroup\RuleGroupRepositoryInterface;
 use FireflyIII\Rules\TransactionMatcher;
-use Illuminate\Database\Eloquent\Relations\HasMany;
 use Input;
 use Preferences;
 use Response;
 use Session;
 use URL;
 use View;
-use Config;
 
 /**
  * Class RuleController
@@ -143,7 +142,7 @@ class RuleController extends Controller
      *
      * @return View
      */
-    public function edit(Rule $rule)
+    public function edit(RuleRepositoryInterface $repository, Rule $rule)
     {
         // has old input?
         if (Input::old()) {
@@ -159,7 +158,7 @@ class RuleController extends Controller
         }
 
         // get rule trigger for update / store-journal:
-        $primaryTrigger = $rule->ruleTriggers()->where('trigger_type', 'user_action')->first()->trigger_value;
+        $primaryTrigger = $repository->getPrimaryTrigger($rule);
         $subTitle       = trans('firefly.edit_rule', ['title' => $rule->title]);
 
         // put previous url in session if not redirect from store (not "return_to_edit").
@@ -176,30 +175,11 @@ class RuleController extends Controller
     /**
      * @return View
      */
-    public function index()
+    public function index(RuleGroupRepositoryInterface $repository)
     {
         $this->createDefaultRuleGroup();
         $this->createDefaultRule();
-
-        $ruleGroups = Auth::user()
-                          ->ruleGroups()
-                          ->orderBy('active', 'DESC')
-                          ->orderBy('order', 'ASC')
-                          ->with(
-                              [
-                                  'rules'              => function (HasMany $query) {
-                                      $query->orderBy('active', 'DESC');
-                                      $query->orderBy('order', 'ASC');
-
-                                  },
-                                  'rules.ruleTriggers' => function (HasMany $query) {
-                                      $query->orderBy('order', 'ASC');
-                                  },
-                                  'rules.ruleActions'  => function (HasMany $query) {
-                                      $query->orderBy('order', 'ASC');
-                                  },
-                              ]
-                          )->get();
+        $ruleGroups = $repository->getRuleGroupsWithRules(Auth::user());
 
         return view('rules.index', compact('ruleGroups'));
     }
@@ -281,6 +261,47 @@ class RuleController extends Controller
 
     }
 
+    /**
+     * @return \Illuminate\View\View
+     */
+    public function testTriggers()
+    {
+        // Create a list of triggers
+        $triggers = $this->getValidTriggerList();
+
+        if (count($triggers) == 0) {
+            return Response::json(['html' => '', 'warning' => trans('firefly.warning_no_valid_triggers')]);
+        }
+
+        // We start searching for transactions. For performance reasons, there are limits
+        // to the search: a maximum number of results and a maximum number of transactions
+        // to search in
+        $maxResults                = Config::get('firefly.test-triggers.limit');
+        $maxTransactionsToSearchIn = Config::get('firefly.test-triggers.max_transactions_to_analyse');
+
+        // Dispatch the actual work to a matched object
+        $matchingTransactions
+            = (new TransactionMatcher($triggers))
+            ->setTransactionLimit($maxTransactionsToSearchIn)
+            ->findMatchingTransactions($maxResults);
+
+        // Warn the user if only a subset of transactions is returned
+        if (count($matchingTransactions) == $maxResults) {
+            $warning = trans('firefly.warning_transaction_subset', ['max_num_transactions' => $maxResults]);
+        } else {
+            if (count($matchingTransactions) == 0) {
+                $warning = trans('firefly.warning_no_matching_transactions', ['num_transactions' => $maxTransactionsToSearchIn]);
+            } else {
+                $warning = "";
+            }
+        }
+
+        // Return json response
+        $view = view('list.journals-tiny', ['transactions' => $matchingTransactions])->render();
+
+        return Response::json(['html' => $view, 'warning' => $warning]);
+    }
+
     /**
      * @param RuleRepositoryInterface $repository
      * @param Rule                    $rule
@@ -335,80 +356,44 @@ class RuleController extends Controller
         return redirect(session('rules.rule.edit.url'));
     }
 
-    /**
-     * @return \Illuminate\View\View
-     */
-    public function testTriggers() {
-        // Create a list of triggers
-        $triggers = $this->getValidTriggerList();
-        
-        if(count($triggers) == 0) {
-            return Response::json(['html' => '', 'warning' => trans('firefly.warning_no_valid_triggers') ]);
-        }
-    
-        // We start searching for transactions. For performance reasons, there are limits
-        // to the search: a maximum number of results and a maximum number of transactions
-        // to search in
-        $maxResults = Config::get('firefly.test-triggers.limit');
-        $maxTransactionsToSearchIn = Config::get('firefly.test-triggers.max_transactions_to_analyse');
-    
-        // Dispatch the actual work to a matched object
-        $matchingTransactions = 
-            (new TransactionMatcher($triggers))
-                ->setTransactionLimit($maxTransactionsToSearchIn)
-                ->findMatchingTransactions($maxResults);
-    
-        // Warn the user if only a subset of transactions is returned
-        if(count( $matchingTransactions ) == $maxResults) {
-            $warning = trans('firefly.warning_transaction_subset', [ 'max_num_transactions' => $maxResults ] );
-        } else if(count($matchingTransactions) == 0){
-            $warning = trans('firefly.warning_no_matching_transactions', [ 'num_transactions' => $maxTransactionsToSearchIn ] );
-        } else {
-            $warning = "";
-        }
-    
-        // Return json response
-        $view = view('list.journals-tiny', [ 'transactions' => $matchingTransactions ])->render();
-    
-        return Response::json(['html' => $view, 'warning' => $warning ]);
-    }    
-    
     /**
      * Returns a list of triggers as provided in the URL.
      * Only returns triggers that will not match any transaction
+     *
      * @return array
      */
-    protected function getValidTriggerList() {
+    protected function getValidTriggerList()
+    {
         $triggers = [];
-        $order = 1;
-        $data = [
+        $order    = 1;
+        $data     = [
             'rule-triggers'       => Input::get('rule-trigger'),
             'rule-trigger-values' => Input::get('rule-trigger-value'),
             'rule-trigger-stop'   => Input::get('rule-trigger-stop'),
         ];
-    
+
         foreach ($data['rule-triggers'] as $index => $trigger) {
             $value          = $data['rule-trigger-values'][$index];
             $stopProcessing = isset($data['rule-trigger-stop'][$index]) ? true : false;
-    
+
             // Create a new trigger object
-            $ruleTrigger = new RuleTrigger;
+            $ruleTrigger                  = new RuleTrigger;
             $ruleTrigger->order           = $order;
             $ruleTrigger->active          = 1;
             $ruleTrigger->stop_processing = $stopProcessing;
             $ruleTrigger->trigger_type    = $trigger;
             $ruleTrigger->trigger_value   = $value;
-    
+
             // Store in list
-            if( !$ruleTrigger->matchesAnything() ) {
+            if (!$ruleTrigger->matchesAnything()) {
                 $triggers[] = $ruleTrigger;
                 $order++;
             }
         }
-    
+
         return $triggers;
-    }    
-    
+    }
+
     private function createDefaultRule()
     {
         /** @var RuleRepositoryInterface $repository */
@@ -451,7 +436,7 @@ class RuleController extends Controller
 
         if ($repository->count() === 0) {
             $data = [
-                'user_id'        => Auth::user()->id,
+                'user_id'     => Auth::user()->id,
                 'title'       => trans('firefly.default_rule_group_name'),
                 'description' => trans('firefly.default_rule_group_description'),
             ];

app/Repositories/Rule/RuleRepository.php

@@ -11,6 +11,7 @@ declare(strict_types = 1);
 namespace FireflyIII\Repositories\Rule;
 
 use Auth;
+use FireflyIII\Exceptions\FireflyException;
 use FireflyIII\Models\Rule;
 use FireflyIII\Models\RuleAction;
 use FireflyIII\Models\RuleGroup;
@@ -68,6 +69,22 @@ class RuleRepository implements RuleRepositoryInterface
         return intval($ruleGroup->rules()->max('order'));
     }
 
+    /**
+     * @param Rule $rule
+     *
+     * @return string
+     * @throws FireflyException
+     */
+    public function getPrimaryTrigger(Rule $rule): string
+    {
+        $count = $rule->ruleTriggers()->count();
+        if ($count === 0) {
+            throw new FireflyException('Rules should have more than zero triggers, rule #' . $rule->id . ' has none!');
+        }
+
+        return $rule->ruleTriggers()->where('trigger_type', 'user_action')->first()->trigger_value;
+    }
+
     /**
      * @param Rule $rule
      *

app/Repositories/Rule/RuleRepositoryInterface.php

@@ -22,6 +22,7 @@ use FireflyIII\Models\RuleTrigger;
  */
 interface RuleRepositoryInterface
 {
+
     /**
      * @return int
      */
@@ -46,6 +47,13 @@ interface RuleRepositoryInterface
      */
     public function getHighestOrderInRuleGroup(RuleGroup $ruleGroup);
 
+    /**
+     * @param Rule $rule
+     *
+     * @return string
+     */
+    public function getPrimaryTrigger(Rule $rule): string;
+
     /**
      * @param Rule $rule
      *

app/Repositories/RuleGroup/RuleGroupRepository.php

@@ -7,6 +7,8 @@ namespace FireflyIII\Repositories\RuleGroup;
 use Auth;
 use FireflyIII\Models\Rule;
 use FireflyIII\Models\RuleGroup;
+use FireflyIII\User;
+use Illuminate\Database\Eloquent\Relations\HasMany;
 use Illuminate\Support\Collection;
 
 /**
@@ -73,6 +75,33 @@ class RuleGroupRepository implements RuleGroupRepositoryInterface
         return intval($entry);
     }
 
+    /**
+     * @param User $user
+     *
+     * @return Collection
+     */
+    public function getRuleGroupsWithRules(User $user): Collection
+    {
+        return $user->ruleGroups()
+                    ->orderBy('active', 'DESC')
+                    ->orderBy('order', 'ASC')
+                    ->with(
+                        [
+                            'rules'              => function (HasMany $query) {
+                                $query->orderBy('active', 'DESC');
+                                $query->orderBy('order', 'ASC');
+
+                            },
+                            'rules.ruleTriggers' => function (HasMany $query) {
+                                $query->orderBy('order', 'ASC');
+                            },
+                            'rules.ruleActions'  => function (HasMany $query) {
+                                $query->orderBy('order', 'ASC');
+                            },
+                        ]
+                    )->get();
+    }
+
     /**
      * @param RuleGroup $ruleGroup
      *
@@ -203,5 +232,4 @@ class RuleGroupRepository implements RuleGroupRepositoryInterface
 
         return $ruleGroup;
     }
-
 }

app/Repositories/RuleGroup/RuleGroupRepositoryInterface.php

@@ -5,6 +5,7 @@ namespace FireflyIII\Repositories\RuleGroup;
 
 
 use FireflyIII\Models\RuleGroup;
+use FireflyIII\User;
 use Illuminate\Support\Collection;
 
 /**
@@ -14,6 +15,8 @@ use Illuminate\Support\Collection;
  */
 interface RuleGroupRepositoryInterface
 {
+
+
     /**
      * @return int
      */
@@ -37,6 +40,13 @@ interface RuleGroupRepositoryInterface
      */
     public function getHighestOrderRuleGroup();
 
+    /**
+     * @param User $user
+     *
+     * @return Collection
+     */
+    public function getRuleGroupsWithRules(User $user): Collection;
+
     /**
      * @param RuleGroup $ruleGroup
      *