From 09f838089b6461eb28d71283514b8b23986b3117 Mon Sep 17 00:00:00 2001 From: James Cole Date: Sun, 16 Jul 2017 13:04:45 +0200 Subject: [PATCH] Allow rule to be applied to transactions (not just group). --- app/Http/Controllers/RuleController.php | 106 +++++++++++- app/Http/Controllers/RuleGroupController.php | 4 +- .../ExecuteRuleOnExistingTransactions.php | 161 ++++++++++++++++++ app/Repositories/Rule/RuleRepository.php | 2 +- app/Rules/Processor.php | 8 +- app/Rules/TransactionMatcher.php | 149 ++++++++++------ public/js/ff/rules/index.js | 31 ++++ resources/lang/en_US/firefly.php | 13 +- resources/views/rules/index.twig | 48 +++--- .../rules/rule-group/select-transactions.twig | 4 +- resources/views/rules/rule/edit.twig | 2 +- .../views/rules/rule/select-transactions.twig | 52 ++++++ routes/web.php | 3 + 13 files changed, 495 insertions(+), 88 deletions(-) create mode 100644 app/Jobs/ExecuteRuleOnExistingTransactions.php create mode 100644 resources/views/rules/rule/select-transactions.twig diff --git a/app/Http/Controllers/RuleController.php b/app/Http/Controllers/RuleController.php index 5ab2f5e4ae..b014acacc8 100644 --- a/app/Http/Controllers/RuleController.php +++ b/app/Http/Controllers/RuleController.php @@ -13,12 +13,18 @@ declare(strict_types=1); namespace FireflyIII\Http\Controllers; +use Carbon\Carbon; +use ExpandedForm; use FireflyIII\Http\Requests\RuleFormRequest; +use FireflyIII\Http\Requests\SelectTransactionsRequest; use FireflyIII\Http\Requests\TestRuleFormRequest; +use FireflyIII\Jobs\ExecuteRuleOnExistingTransactions; +use FireflyIII\Models\AccountType; use FireflyIII\Models\Rule; use FireflyIII\Models\RuleAction; use FireflyIII\Models\RuleGroup; use FireflyIII\Models\RuleTrigger; +use FireflyIII\Repositories\Account\AccountRepositoryInterface; use FireflyIII\Repositories\Rule\RuleRepositoryInterface; use FireflyIII\Repositories\RuleGroup\RuleGroupRepositoryInterface; use FireflyIII\Rules\TransactionMatcher; @@ -237,6 +243,58 @@ class RuleController extends Controller return Response::json('true'); } + /** + * Execute the given rule on a set of existing transactions + * + * @param SelectTransactionsRequest $request + * @param AccountRepositoryInterface $repository + * @param RuleGroup $ruleGroup + * + * @return \Illuminate\Http\RedirectResponse + */ + public function execute(SelectTransactionsRequest $request, AccountRepositoryInterface $repository, Rule $rule) + { + // Get parameters specified by the user + $accounts = $repository->getAccountsById($request->get('accounts')); + $startDate = new Carbon($request->get('start_date')); + $endDate = new Carbon($request->get('end_date')); + + // Create a job to do the work asynchronously + $job = new ExecuteRuleOnExistingTransactions($rule); + + // Apply parameters to the job + $job->setUser(auth()->user()); + $job->setAccounts($accounts); + $job->setStartDate($startDate); + $job->setEndDate($endDate); + + // Dispatch a new job to execute it in a queue + $this->dispatch($job); + + // Tell the user that the job is queued + Session::flash('success', strval(trans('firefly.applied_rule_selection', ['title' => $rule->title]))); + + return redirect()->route('rules.index'); + } + + /** + * @param AccountRepositoryInterface $repository + * @param RuleGroup $ruleGroup + * + * @return View + */ + public function selectTransactions(AccountRepositoryInterface $repository, Rule $rule) + { + // does the user have shared accounts? + $accounts = $repository->getAccountsByType([AccountType::ASSET]); + $accountList = ExpandedForm::makeSelectList($accounts); + $checkedAccounts = array_keys($accountList); + $first = session('first')->format('Y-m-d'); + $today = Carbon::create()->format('Y-m-d'); + $subTitle = (string)trans('firefly.apply_rule_selection', ['title' => $rule->title]); + + return view('rules.rule.select-transactions', compact('checkedAccounts', 'accountList', 'first', 'today', 'rule', 'subTitle')); + } /** * @param RuleFormRequest $request @@ -265,6 +323,52 @@ class RuleController extends Controller return redirect($this->getPreviousUri('rules.create.uri')); } + /** + * This method allows the user to test a certain set of rule triggers. The rule triggers are grabbed from + * the rule itself. + * + * This method will parse and validate those rules and create a "TransactionMatcher" which will attempt + * to find transaction journals matching the users input. A maximum range of transactions to try (range) and + * a maximum number of transactions to return (limit) are set as well. + * + * + * @param Rule $rule + * + * @return \Illuminate\Http\JsonResponse + */ + public function testTriggersByRule(Rule $rule) { + + $triggers = $rule->ruleTriggers; + + if (count($triggers) === 0) { + return Response::json(['html' => '', 'warning' => trans('firefly.warning_no_valid_triggers')]); + } + + $limit = config('firefly.test-triggers.limit'); + $range = config('firefly.test-triggers.range'); + + /** @var TransactionMatcher $matcher */ + $matcher = app(TransactionMatcher::class); + $matcher->setLimit($limit); + $matcher->setRange($range); + $matcher->setRule($rule); + $matchingTransactions = $matcher->findTransactionsByRule(); + + // Warn the user if only a subset of transactions is returned + $warning = ''; + if (count($matchingTransactions) === $limit) { + $warning = trans('firefly.warning_transaction_subset', ['max_num_transactions' => $limit]); + } + if (count($matchingTransactions) === 0) { + $warning = trans('firefly.warning_no_matching_transactions', ['num_transactions' => $range]); + } + + // Return json response + $view = view('list.journals-tiny', ['transactions' => $matchingTransactions])->render(); + + return Response::json(['html' => $view, 'warning' => $warning]); + } + /** * This method allows the user to test a certain set of rule triggers. The rule triggers are passed along * using the URL parameters (GET), and are usually put there using a Javascript thing. @@ -294,7 +398,7 @@ class RuleController extends Controller $matcher->setLimit($limit); $matcher->setRange($range); $matcher->setTriggers($triggers); - $matchingTransactions = $matcher->findMatchingTransactions(); + $matchingTransactions = $matcher->findTransactionsByTriggers(); // Warn the user if only a subset of transactions is returned $warning = ''; diff --git a/app/Http/Controllers/RuleGroupController.php b/app/Http/Controllers/RuleGroupController.php index d0f60c08d3..474a7eda73 100644 --- a/app/Http/Controllers/RuleGroupController.php +++ b/app/Http/Controllers/RuleGroupController.php @@ -178,7 +178,7 @@ class RuleGroupController extends Controller $this->dispatch($job); // Tell the user that the job is queued - Session::flash('success', strval(trans('firefly.executed_group_on_existing_transactions', ['title' => $ruleGroup->title]))); + Session::flash('success', strval(trans('firefly.applied_rule_group_selection', ['title' => $ruleGroup->title]))); return redirect()->route('rules.index'); } @@ -197,7 +197,7 @@ class RuleGroupController extends Controller $checkedAccounts = array_keys($accountList); $first = session('first')->format('Y-m-d'); $today = Carbon::create()->format('Y-m-d'); - $subTitle = (string)trans('firefly.execute_on_existing_transactions'); + $subTitle = (string)trans('firefly.apply_rule_group_selection', ['title' => $ruleGroup->title]); return view('rules.rule-group.select-transactions', compact('checkedAccounts', 'accountList', 'first', 'today', 'ruleGroup', 'subTitle')); } diff --git a/app/Jobs/ExecuteRuleOnExistingTransactions.php b/app/Jobs/ExecuteRuleOnExistingTransactions.php new file mode 100644 index 0000000000..b8f9c2bb6a --- /dev/null +++ b/app/Jobs/ExecuteRuleOnExistingTransactions.php @@ -0,0 +1,161 @@ +rule = $rule; + } + + /** + * @return Collection + */ + public function getAccounts(): Collection + { + return $this->accounts; + } + + /** + * + * @param Collection $accounts + */ + public function setAccounts(Collection $accounts) + { + $this->accounts = $accounts; + } + + /** + * @return \Carbon\Carbon + */ + public function getEndDate(): Carbon + { + return $this->endDate; + } + + /** + * + * @param Carbon $date + */ + public function setEndDate(Carbon $date) + { + $this->endDate = $date; + } + + /** + * @return \Carbon\Carbon + */ + public function getStartDate(): Carbon + { + return $this->startDate; + } + + /** + * + * @param Carbon $date + */ + public function setStartDate(Carbon $date) + { + $this->startDate = $date; + } + + /** + * @return User + */ + public function getUser(): User + { + return $this->user; + } + + /** + * + * @param User $user + */ + public function setUser(User $user) + { + $this->user = $user; + } + + /** + * Execute the job. + * + * @return void + */ + public function handle() + { + // Lookup all journals that match the parameters specified + $transactions = $this->collectJournals(); + $processor = Processor::make($this->rule); + + // Execute the rules for each transaction + foreach ($transactions as $transaction) { + + $processor->handleTransaction($transaction); + + // Stop processing this group if the rule specifies 'stop_processing' + if ($processor->getRule()->stop_processing) { + break; + } + } + + } + + /** + * Collect all journals that should be processed + * + * @return Collection + */ + protected function collectJournals() + { + /** @var JournalCollectorInterface $collector */ + $collector = app(JournalCollectorInterface::class); + $collector->setUser($this->user); + $collector->setAccounts($this->accounts)->setRange($this->startDate, $this->endDate); + + return $collector->getJournals(); + } + +} diff --git a/app/Repositories/Rule/RuleRepository.php b/app/Repositories/Rule/RuleRepository.php index 5d3a1f2d47..8d40076c71 100644 --- a/app/Repositories/Rule/RuleRepository.php +++ b/app/Repositories/Rule/RuleRepository.php @@ -265,7 +265,7 @@ class RuleRepository implements RuleRepositoryInterface $ruleAction->active = 1; $ruleAction->stop_processing = $values['stopProcessing']; $ruleAction->action_type = $values['action']; - $ruleAction->action_value = $values['value']; + $ruleAction->action_value = is_null($values['value']) ? '' : $values['value']; $ruleAction->save(); diff --git a/app/Rules/Processor.php b/app/Rules/Processor.php index ae0ecb1fd8..478c7bf0df 100644 --- a/app/Rules/Processor.php +++ b/app/Rules/Processor.php @@ -59,9 +59,11 @@ final class Processor * * @param Rule $rule * + * @param bool $includeActions + * * @return Processor */ - public static function make(Rule $rule) + public static function make(Rule $rule, $includeActions = true) { Log::debug(sprintf('Making new rule from Rule %d', $rule->id)); $self = new self; @@ -72,7 +74,9 @@ final class Processor Log::debug(sprintf('Push trigger %d', $trigger->id)); $self->triggers->push(TriggerFactory::getTrigger($trigger)); } - $self->actions = $rule->ruleActions()->orderBy('order', 'ASC')->get(); + if ($includeActions) { + $self->actions = $rule->ruleActions()->orderBy('order', 'ASC')->get(); + } return $self; } diff --git a/app/Rules/TransactionMatcher.php b/app/Rules/TransactionMatcher.php index 090d3553ba..b192f64d7d 100644 --- a/app/Rules/TransactionMatcher.php +++ b/app/Rules/TransactionMatcher.php @@ -14,6 +14,7 @@ declare(strict_types=1); namespace FireflyIII\Rules; use FireflyIII\Helpers\Collector\JournalCollectorInterface; +use FireflyIII\Models\Rule; use FireflyIII\Models\Transaction; use FireflyIII\Models\TransactionType; use FireflyIII\Repositories\Journal\JournalTaskerInterface; @@ -32,6 +33,8 @@ class TransactionMatcher private $limit = 10; /** @var int Maximum number of transaction to search in (for performance reasons) * */ private $range = 200; + /** @var Rule */ + private $rule; /** @var JournalTaskerInterface */ private $tasker; /** @var array */ @@ -50,6 +53,31 @@ class TransactionMatcher } + /** + * This method will search the user's transaction journal (with an upper limit of $range) for + * transaction journals matching the given rule. This is accomplished by trying to fire these + * triggers onto each transaction journal until enough matches are found ($limit). + * + * @return Collection + * + */ + public function findTransactionsByRule() + { + if (count($this->rule->ruleTriggers) === 0) { + return new Collection; + } + + // Variables used within the loop + $processor = Processor::make($this->rule, false); + $result = $this->runProcessor($processor); + + // 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 + $result = $result->slice(0, $this->limit); + + return $result; + } + /** * This method will search the user's transaction journal (with an upper limit of $range) for * transaction journals matching the given $triggers. This is accomplished by trying to fire these @@ -58,64 +86,15 @@ class TransactionMatcher * @return Collection * */ - public function findMatchingTransactions(): Collection + public function findTransactionsByTriggers(): Collection { if (count($this->triggers) === 0) { return new Collection; } - $pageSize = min($this->range / 2, $this->limit * 2); // Variables used within the loop - $processed = 0; - $page = 1; - $result = new Collection(); $processor = Processor::makeFromStringArray($this->triggers); - - // 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 - /** @var JournalCollectorInterface $collector */ - $collector = app(JournalCollectorInterface::class); - $collector->setUser(auth()->user()); - $collector->setAllAssetAccounts()->setLimit($pageSize)->setPage($page)->setTypes($this->transactionTypes); - $set = $collector->getPaginatedJournals(); - Log::debug(sprintf('Found %d journals to check. ', $set->count())); - - // Filter transactions that match the given triggers. - $filtered = $set->filter( - function (Transaction $transaction) use ($processor) { - Log::debug(sprintf('Test these triggers on journal #%d (transaction #%d)', $transaction->transaction_journal_id, $transaction->id)); - - return $processor->handleTransaction($transaction); - } - ); - - Log::debug(sprintf('Found %d journals that match.', $filtered->count())); - - // merge: - /** @var Collection $result */ - $result = $result->merge($filtered); - Log::debug(sprintf('Total count is now %d', $result->count())); - - // Update counters - $page++; - $processed += count($set); - - Log::debug(sprintf('Page is now %d, processed is %d', $page, $processed)); - - // Check for conditions to finish the loop - $reachedEndOfList = $set->count() < 1; - $foundEnough = $result->count() >= $this->limit; - $searchedEnough = ($processed >= $this->range); - - Log::debug(sprintf('reachedEndOfList: %s', var_export($reachedEndOfList, true))); - Log::debug(sprintf('foundEnough: %s', var_export($foundEnough, true))); - Log::debug(sprintf('searchedEnough: %s', var_export($searchedEnough, true))); - - } while (!$reachedEndOfList && !$foundEnough && !$searchedEnough); + $result = $this->runProcessor($processor); // 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 @@ -185,5 +164,73 @@ class TransactionMatcher return $this; } + /** + * @param Rule $rule + */ + public function setRule(Rule $rule) + { + $this->rule = $rule; + } + + /** + * @param Processor $processor + * + * @return Collection + */ + private function runProcessor(Processor $processor): Collection + { + // 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 + $pageSize = min($this->range / 2, $this->limit * 2); + $processed = 0; + $page = 1; + $result = new Collection(); + do { + // Fetch a batch of transactions from the database + /** @var JournalCollectorInterface $collector */ + $collector = app(JournalCollectorInterface::class); + $collector->setUser(auth()->user()); + $collector->setAllAssetAccounts()->setLimit($pageSize)->setPage($page)->setTypes($this->transactionTypes); + $set = $collector->getPaginatedJournals(); + Log::debug(sprintf('Found %d journals to check. ', $set->count())); + + // Filter transactions that match the given triggers. + $filtered = $set->filter( + function (Transaction $transaction) use ($processor) { + Log::debug(sprintf('Test these triggers on journal #%d (transaction #%d)', $transaction->transaction_journal_id, $transaction->id)); + + return $processor->handleTransaction($transaction); + } + ); + + Log::debug(sprintf('Found %d journals that match.', $filtered->count())); + + // merge: + /** @var Collection $result */ + $result = $result->merge($filtered); + Log::debug(sprintf('Total count is now %d', $result->count())); + + // Update counters + $page++; + $processed += count($set); + + Log::debug(sprintf('Page is now %d, processed is %d', $page, $processed)); + + // Check for conditions to finish the loop + $reachedEndOfList = $set->count() < 1; + $foundEnough = $result->count() >= $this->limit; + $searchedEnough = ($processed >= $this->range); + + Log::debug(sprintf('reachedEndOfList: %s', var_export($reachedEndOfList, true))); + Log::debug(sprintf('foundEnough: %s', var_export($foundEnough, true))); + Log::debug(sprintf('searchedEnough: %s', var_export($searchedEnough, true))); + + } while (!$reachedEndOfList && !$foundEnough && !$searchedEnough); + + return $result; + } + } diff --git a/public/js/ff/rules/index.js b/public/js/ff/rules/index.js index a64d6a6cc6..52dd0ef27e 100644 --- a/public/js/ff/rules/index.js +++ b/public/js/ff/rules/index.js @@ -37,9 +37,40 @@ $(function () { } ); + + // test rule triggers button: + $('.test_rule_triggers').click(testRuleTriggers); } ); +function testRuleTriggers(e) { + var obj = $(e.target); + var ruleId = parseInt(obj.data('id')); + + // Find a list of existing transactions that match these triggers + $.get('rules/test-rule/' + ruleId).done(function (data) { + var modal = $("#testTriggerModal"); + + // 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 + modal.modal(); + }).fail(function () { + alert('Cannot get transactions for given triggers.'); + }); + + return false; +} + function sortStop(event, ui) { "use strict"; diff --git a/resources/lang/en_US/firefly.php b/resources/lang/en_US/firefly.php index 95b3bec834..60895e3224 100644 --- a/resources/lang/en_US/firefly.php +++ b/resources/lang/en_US/firefly.php @@ -205,7 +205,6 @@ return [ // rules 'rules' => 'Rules', - 'rules_explanation' => 'Here you can manage rules. Rules are triggered when a transaction is created or updated. Then, if the transaction has certain properties (called "triggers") Firefly will execute the "actions". Combined, you can make Firefly respond in a certain way to new transactions.', 'rule_name' => 'Name of rule', 'rule_triggers' => 'Rule triggers when', 'rule_actions' => 'Rule will', @@ -255,14 +254,14 @@ return [ '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.', - 'execute_on_existing_transactions' => 'Execute for existing transactions', - 'rule_group_select_transactions' => 'Execute rule group ":title" on existing transactions', - 'execute_on_existing_transactions_intro' => 'When a rule or group has been changed or added, you can execute it for existing transactions', - 'execute_on_existing_transactions_short' => 'Existing transactions', - 'executed_group_on_existing_transactions' => 'Executed group ":title" for existing transactions', - 'execute_group_on_existing_transactions' => 'Execute group ":title" for existing transactions', + 'apply_rule_selection' => 'Apply rule ":title" to a selection of your transactions', + 'apply_rule_selection_intro' => 'Rules like ":title" are normally only applied to new or updated transactions, but you can tell Firefly III to run it on a selection of your existing transactions. This can be useful when you have updated a rule and you need the changes to be applied to all of your other transactions.', 'include_transactions_from_accounts' => 'Include transactions from these accounts', + 'applied_rule_selection' => 'Rule ":title" has been applied to your selection.', 'execute' => 'Execute', + 'apply_rule_group_selection' => 'Apply rule group ":title" to a selection of your transactions', + 'apply_rule_group_selection_intro' => 'Rule groups like ":title" are normally only applied to new or updated transactions, but you can tell Firefly III to run all the rules in this group on a selection of your existing transactions. This can be useful when you have updated a group of rules and you need the changes to be applied to all of your other transactions.', + 'applied_rule_group_selection' => 'Rule ":title" has been applied to your selection.', // actions and triggers 'rule_trigger_user_action' => 'User action is ":trigger_value"', diff --git a/resources/views/rules/index.twig b/resources/views/rules/index.twig index c552d1f56c..a66d291105 100644 --- a/resources/views/rules/index.twig +++ b/resources/views/rules/index.twig @@ -6,16 +6,9 @@
-
-
-

{{ 'rules'|_ }}

-
-
-

- {{ 'rules_explanation'|_ }} -

-
-
+

+ {{ 'new_rule_group'|_ }} +

@@ -38,12 +31,11 @@