diff --git a/app/Http/Controllers/RuleController.php b/app/Http/Controllers/RuleController.php index b8feba57e4..544df0fb96 100644 --- a/app/Http/Controllers/RuleController.php +++ b/app/Http/Controllers/RuleController.php @@ -18,6 +18,7 @@ use FireflyIII\Models\RuleGroup; 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; @@ -25,6 +26,7 @@ use Response; use Session; use URL; use View; +use Config; /** * Class RuleController @@ -333,6 +335,80 @@ 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() { + $triggers = []; + $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->order = $order; + $ruleTrigger->active = 1; + $ruleTrigger->stop_processing = $stopProcessing; + $ruleTrigger->trigger_type = $trigger; + $ruleTrigger->trigger_value = $value; + + // Store in list + if( !$ruleTrigger->matchesAnything() ) { + $triggers[] = $ruleTrigger; + $order++; + } + } + + return $triggers; + } + private function createDefaultRule() { /** @var RuleRepositoryInterface $repository */ diff --git a/app/Http/routes.php b/app/Http/routes.php index 5e16686d9a..c930d03514 100644 --- a/app/Http/routes.php +++ b/app/Http/routes.php @@ -272,7 +272,8 @@ Route::group( Route::get('/rules/rules/down/{rule}', ['uses' => 'RuleController@down', 'as' => 'rules.rule.down']); Route::get('/rules/rules/edit/{rule}', ['uses' => 'RuleController@edit', 'as' => 'rules.rule.edit']); Route::get('/rules/rules/delete/{rule}', ['uses' => 'RuleController@delete', 'as' => 'rules.rule.delete']); - + Route::get('/rules/rules/test_triggers', ['uses' => 'RuleController@testTriggers', 'as' => 'rules.rule.test_triggers']); + // rules POST: Route::post('/rules/rules/trigger/reorder/{rule}', ['uses' => 'RuleController@reorderRuleTriggers']); Route::post('/rules/rules/action/reorder/{rule}', ['uses' => 'RuleController@reorderRuleActions']); diff --git a/app/Repositories/Journal/JournalRepository.php b/app/Repositories/Journal/JournalRepository.php index c11f3a4a97..c90fcb19f0 100644 --- a/app/Repositories/Journal/JournalRepository.php +++ b/app/Repositories/Journal/JournalRepository.php @@ -93,9 +93,9 @@ class JournalRepository implements JournalRepositoryInterface * * @return LengthAwarePaginator */ - public function getJournalsOfTypes(array $types, int $offset, int $page) + public function getJournalsOfTypes(array $types, int $offset, int $page, int $pagesize = 50) { - $set = Auth::user()->transactionJournals()->transactionTypes($types)->withRelevantData()->take(50)->offset($offset) + $set = Auth::user()->transactionJournals()->transactionTypes($types)->withRelevantData()->take($pagesize)->offset($offset) ->orderBy('date', 'DESC') ->orderBy('order', 'ASC') ->orderBy('id', 'DESC') @@ -103,7 +103,7 @@ class JournalRepository implements JournalRepositoryInterface ['transaction_journals.*'] ); $count = Auth::user()->transactionJournals()->transactionTypes($types)->count(); - $journals = new LengthAwarePaginator($set, $count, 50, $page); + $journals = new LengthAwarePaginator($set, $count, $pagesize, $page); return $journals; } diff --git a/app/Rules/Processor.php b/app/Rules/Processor.php index bcd81a4d86..52ece1a647 100644 --- a/app/Rules/Processor.php +++ b/app/Rules/Processor.php @@ -87,6 +87,22 @@ class Processor } + /** + * Checks whether the current transaction is triggered by the current rule + * @return boolean + */ + public function isTriggered() { + return $this->triggered(); + } + + /** + * Checks whether the current transaction is triggered by the list of given triggers + * @return boolean + */ + public function isTriggeredBy(array $triggers) { + return $this->triggeredBy($triggers); + } + /** * @return bool */ @@ -110,14 +126,15 @@ class Processor } /** + * Method to check whether the current transaction would be triggered + * by the given list of triggers * @return bool - */ - protected function triggered() - { + */ + protected function triggeredBy($triggers) { $foundTriggers = 0; $hitTriggers = 0; /** @var RuleTrigger $trigger */ - foreach ($this->rule->ruleTriggers()->orderBy('order', 'ASC')->get() as $trigger) { + foreach ($triggers as $trigger) { $foundTriggers++; /** @var TriggerInterface $triggerClass */ @@ -133,7 +150,15 @@ class Processor Log::debug('Total: ' . $foundTriggers . ' found triggers. ' . $hitTriggers . ' triggers were hit.'); return ($hitTriggers == $foundTriggers); - + + } + /** + * Checks whether the current transaction is triggered by the current rule + * @return bool + */ + protected function triggered() + { + return $this->triggeredBy($this->rule->ruleTriggers()->orderBy('order', 'ASC')->get()); } diff --git a/app/Rules/TransactionMatcher.php b/app/Rules/TransactionMatcher.php new file mode 100644 index 0000000000..629cdaa112 --- /dev/null +++ b/app/Rules/TransactionMatcher.php @@ -0,0 +1,148 @@ +setTriggers($triggers); + } + + /** + * Find matching transactions for the current set of triggers + * @param number $maxResults The maximum number of transactions returned + */ + public function findMatchingTransactions($maxResults = 50) { + /** @var JournalRepositoryInterface $repository */ + $repository = app('FireflyIII\Repositories\Journal\JournalRepositoryInterface'); + + // We don't know the number of transaction to fetch from the database, in + // order to return the proper number of matching transactions. Since we don't want + // to fetch all transactions (as the first transactions already match, or the last + // transactions are irrelevant), we will fetch data in pages. + + // The optimal pagesize is somewhere between the maximum number of results to be returned + // and the maximum number of transactions to consider. + $pagesize = min($this->maxTransactionsToSearchIn / 2, $maxResults * 2); + + // Variables used within the loop + $numTransactionsProcessed = 0; + $page = 1; + $matchingTransactions = []; + + // Flags to indicate the end of the loop + $reachedEndOfList = false; + $foundEnoughTransactions = false; + $searchedEnoughTransactions = false; + + // Start a loop to fetch batches of transactions. The loop will finish if: + // - all transactions have been fetched from the database + // - the maximum number of transactions to return has been found + // - the maximum number of transactions to search in have been searched + do { + // Fetch a batch of transactions from the database + $offset = $page > 0 ? ($page - 1) * $pagesize : 0; + $transactions = $repository->getJournalsOfTypes( $this->transactionTypes, $offset, $page, $pagesize)->getCollection()->all(); + + // Filter transactions that match the rule + $matchingTransactions += array_filter( $transactions, function($transaction) { + $processor = new Processor(new Rule, $transaction); + return $processor->isTriggeredBy($this->triggers); + }); + + // Update counters + $page++; + $numTransactionsProcessed += count($transactions); + + // Check for conditions to finish the loop + $reachedEndOfList = (count($transactions) < $pagesize); + $foundEnoughTransactions = (count($matchingTransactions) >= $maxResults); + $searchedEnoughTransactions = ($numTransactionsProcessed >= $this->maxTransactionsToSearchIn); + } while( !$reachedEndOfList && !$foundEnoughTransactions && !$searchedEnoughTransactions); + + // If the list of matchingTransactions is larger than the maximum number of results + // (e.g. if a large percentage of the transactions match), truncate the list + $matchingTransactions = array_slice($matchingTransactions, 0, $maxResults); + + return $matchingTransactions; + } + + /** + * @return array + */ + public function getTriggers() { + return $this->triggers; + } + + /** + * @param array $triggers + */ + public function setTriggers($triggers) { + $this->triggers = $triggers; + return $this; + } + + /** + * @return array + */ + public function getTransactionLimit() { + return $this->maxTransactionsToSearchIn; + } + + /** + * @param int $limit + */ + public function setTransactionLimit(int $limit) { + $this->maxTransactionsToSearchIn = $limit; + return $this; + } + + /** + * @return array + */ + public function getTransactionTypes() { + return $this->transactionTypes; + } + + /** + * @param array $transactionTypes + */ + public function setTransactionTypes(array $transactionTypes) { + $this->transactionTypes = $transactionTypes; + return $this; + } + +} diff --git a/config/firefly.php b/config/firefly.php index 48dde88d9a..911b91e25d 100644 --- a/config/firefly.php +++ b/config/firefly.php @@ -219,5 +219,11 @@ return [ 'append_description', 'prepend_description', ], - + 'test-triggers' => [ + // The maximum number of transactions shown when testing a list of triggers + 'limit' => 50, + + // The maximum number of transactions to analyse, when testing a list of triggers + 'max_transactions_to_analyse' => 1000 + ] ]; diff --git a/public/css/firefly.css b/public/css/firefly.css index 9843f2f058..4ce8b95c80 100644 --- a/public/css/firefly.css +++ b/public/css/firefly.css @@ -37,4 +37,6 @@ /* cursors */ .rule-triggers {cursor:move;} -.rule-actions {cursor:move;} \ No newline at end of file +.rule-actions {cursor:move;} + +#testTriggerModal .modal-body { max-height: 500px; overflow-y: scroll; } diff --git a/public/js/rules/create-edit.js b/public/js/rules/create-edit.js index 9310d6d8de..9a4c0c812c 100644 --- a/public/js/rules/create-edit.js +++ b/public/js/rules/create-edit.js @@ -82,4 +82,33 @@ function removeAction(e) { if($('.rule-action-tbody tr').length == 0) { addNewAction(); } +} + +function testRuleTriggers() { + "use strict"; + + // Serialize all trigger data + var triggerData = $( ".rule-trigger-tbody" ).find( "input[type=text], input[type=checkbox], select" ).serializeArray(); + + // Find a list of existing transactions that match these triggers + $.get('rules/rules/test_triggers', triggerData).done(function (data) { + var modal = $( "#testTriggerModal" ); + var numTriggers = $( ".rule-trigger-body > tr" ).length; + + // Set title and body + modal.find( ".transactions-list" ).html(data.html); + + // Show warning if appropriate + if( data.warning ) { + modal.find( ".transaction-warning .warning-contents" ).text(data.warning); + modal.find( ".transaction-warning" ).show(); + } else { + modal.find( ".transaction-warning" ).hide(); + } + + // Show the modal dialog + $( "#testTriggerModal" ).modal(); + }).fail(function () { + alert('Cannot get transactions for given triggers.'); + }); } \ No newline at end of file diff --git a/public/js/rules/create.js b/public/js/rules/create.js index 617cc284ee..4910378c74 100644 --- a/public/js/rules/create.js +++ b/public/js/rules/create.js @@ -31,4 +31,10 @@ $(function () { return false; }); + + $('.test_rule_triggers').click(function () { + testRuleTriggers(); + + return false; + }); }); diff --git a/public/js/rules/edit.js b/public/js/rules/edit.js index dc4ab331a5..39876d8c2c 100644 --- a/public/js/rules/edit.js +++ b/public/js/rules/edit.js @@ -24,6 +24,12 @@ $(function () { return false; }); + $('.test_rule_triggers').click(function () { + testRuleTriggers(); + + return false; + }); + $('.add_rule_action').click(function () { addNewAction(); diff --git a/resources/lang/en_US/firefly.php b/resources/lang/en_US/firefly.php index 63a8aab69a..e9ed71aa0c 100644 --- a/resources/lang/en_US/firefly.php +++ b/resources/lang/en_US/firefly.php @@ -126,6 +126,11 @@ return [ 'delete_rule' => 'Delete rule ":title"', 'update_rule' => 'Update rule', + 'test_rule_triggers' => 'See matching transactions', + 'warning_transaction_subset' => 'For performance reasons this list is limited to :max_num_transactions and may only show a subset of matching transactions', + 'warning_no_matching_transactions' => 'No matching transactions found. Please note that for performance reasons, only the last :num_transactions transactions have been checked.', + 'warning_no_valid_triggers' => 'No valid triggers provided.', + // actions and triggers 'rule_trigger_user_action' => 'User action is ":trigger_value"', 'rule_trigger_from_account_starts' => 'Source account starts with ":trigger_value"', diff --git a/resources/lang/nl_NL/firefly.php b/resources/lang/nl_NL/firefly.php index 3fbdfc6d6c..9288305e27 100644 --- a/resources/lang/nl_NL/firefly.php +++ b/resources/lang/nl_NL/firefly.php @@ -125,6 +125,11 @@ return [ 'edit_rule' => 'Wijzig regel ":title"', 'update_rule' => 'Werk regel bij', + 'test_rule_triggers' => 'Transacties die voldoen aan triggers', + 'warning_transaction_subset' => 'In verband met de snelheid, worden in deze lijst maximaal :max_num_transactions transacties weergegeven. Het kan daarom zijn dat transacties ontbreken in deze lijst.', + 'warning_no_matching_transactions' => 'Er zijn geen transacties gevonden die voldoen aan de triggers in de laatste :num_transactions transacties.', + 'warning_no_valid_triggers' => 'Geen geldige triggers opgegeven.', + // actions and triggers 'rule_trigger_user_action' => 'Gebruikersactie is ":trigger_value"', 'rule_trigger_from_account_starts' => 'Bronrekeningnaam begint met ":trigger_value"', diff --git a/resources/views/rules/partials/test-trigger-modal.twig b/resources/views/rules/partials/test-trigger-modal.twig new file mode 100644 index 0000000000..8239d0d88f --- /dev/null +++ b/resources/views/rules/partials/test-trigger-modal.twig @@ -0,0 +1,22 @@ + + \ No newline at end of file diff --git a/resources/views/rules/rule/create.twig b/resources/views/rules/rule/create.twig index 3361e454fe..6edc787b25 100644 --- a/resources/views/rules/rule/create.twig +++ b/resources/views/rules/rule/create.twig @@ -61,11 +61,14 @@


{{ 'add_rule_trigger'|_ }} + {{ 'test_rule_triggers'|_ }}

+ + {% include '/rules/partials/test-trigger-modal.twig' %}
diff --git a/resources/views/rules/rule/edit.twig b/resources/views/rules/rule/edit.twig index bda2c3eae9..72b2037f1f 100644 --- a/resources/views/rules/rule/edit.twig +++ b/resources/views/rules/rule/edit.twig @@ -62,11 +62,14 @@


{{ 'add_rule_trigger'|_ }} + {{ 'test_rule_triggers'|_ }}

+ + {% include '/rules/partials/test-trigger-modal.twig' %}