From 1558da60aa8b91a59adcc2dee8d99d4da19ce9eb Mon Sep 17 00:00:00 2001 From: James Cole Date: Mon, 8 Feb 2021 15:12:04 +0100 Subject: [PATCH] Fix #4007 --- .../Controllers/Rule/SelectController.php | 17 +++--- .../Actions/ActionInterface.php | 4 +- .../Engine/RuleEngineInterface.php | 8 +++ .../Engine/SearchRuleEngine.php | 61 +++++++++++-------- resources/lang/en_US/firefly.php | 2 +- 5 files changed, 58 insertions(+), 34 deletions(-) diff --git a/app/Http/Controllers/Rule/SelectController.php b/app/Http/Controllers/Rule/SelectController.php index 08ac3bca90..ecab505702 100644 --- a/app/Http/Controllers/Rule/SelectController.php +++ b/app/Http/Controllers/Rule/SelectController.php @@ -60,7 +60,7 @@ class SelectController extends Controller $this->middleware( function ($request, $next) { - app('view')->share('title', (string) trans('firefly.rules')); + app('view')->share('title', (string)trans('firefly.rules')); app('view')->share('mainTitleIcon', 'fa-random'); return $next($request); @@ -97,9 +97,9 @@ class SelectController extends Controller // set rules: $newRuleEngine->setRules(new Collection([$rule])); $newRuleEngine->fire(); + $resultCount = $newRuleEngine->getResults(); - // Tell the user that the job is queued - session()->flash('success', (string) trans('firefly.applied_rule_selection', ['title' => $rule->title])); + session()->flash('success', (string)trans_choice('firefly.applied_rule_selection', $resultCount, ['title' => $rule->title])); return redirect()->route('rules.index'); } @@ -116,12 +116,13 @@ class SelectController extends Controller { if (false === $rule->active) { session()->flash('warning', trans('firefly.cannot_fire_inactive_rules')); + return redirect(route('rules.index')); } // does the user have shared accounts? $first = session('first', Carbon::now()->subYear())->format('Y-m-d'); $today = Carbon::now()->format('Y-m-d'); - $subTitle = (string) trans('firefly.apply_rule_selection', ['title' => $rule->title]); + $subTitle = (string)trans('firefly.apply_rule_selection', ['title' => $rule->title]); return prefixView('rules.rule.select-transactions', compact('first', 'today', 'rule', 'subTitle')); } @@ -147,7 +148,7 @@ class SelectController extends Controller // warn if nothing. if (0 === count($textTriggers)) { - return response()->json(['html' => '', 'warning' => (string) trans('firefly.warning_no_valid_triggers')]); // @codeCoverageIgnore + return response()->json(['html' => '', 'warning' => (string)trans('firefly.warning_no_valid_triggers')]); // @codeCoverageIgnore } foreach ($textTriggers as $textTrigger) { @@ -170,7 +171,7 @@ class SelectController extends Controller // Warn the user if only a subset of transactions is returned $warning = ''; if (0 === count($collection)) { - $warning = (string) trans('firefly.warning_no_matching_transactions'); // @codeCoverageIgnore + $warning = (string)trans('firefly.warning_no_matching_transactions'); // @codeCoverageIgnore } // Return json response @@ -203,7 +204,7 @@ class SelectController extends Controller $triggers = $rule->ruleTriggers; if (0 === count($triggers)) { - return response()->json(['html' => '', 'warning' => (string) trans('firefly.warning_no_valid_triggers')]); // @codeCoverageIgnore + return response()->json(['html' => '', 'warning' => (string)trans('firefly.warning_no_valid_triggers')]); // @codeCoverageIgnore } @@ -217,7 +218,7 @@ class SelectController extends Controller $warning = ''; if (0 === count($collection)) { - $warning = (string) trans('firefly.warning_no_matching_transactions'); // @codeCoverageIgnore + $warning = (string)trans('firefly.warning_no_matching_transactions'); // @codeCoverageIgnore } // Return json response diff --git a/app/TransactionRules/Actions/ActionInterface.php b/app/TransactionRules/Actions/ActionInterface.php index 7a8f7d097c..7da3388e85 100644 --- a/app/TransactionRules/Actions/ActionInterface.php +++ b/app/TransactionRules/Actions/ActionInterface.php @@ -37,9 +37,11 @@ interface ActionInterface public function __construct(RuleAction $action); /** - * Execute the action on an array. + * Execute the action on an array. Returns "true" if the action was a success and the action + * was applied. Returns false if otherwise. * * @param array $journal + * * @return bool */ public function actOnArray(array $journal): bool; diff --git a/app/TransactionRules/Engine/RuleEngineInterface.php b/app/TransactionRules/Engine/RuleEngineInterface.php index f92eb7d053..b61f175255 100644 --- a/app/TransactionRules/Engine/RuleEngineInterface.php +++ b/app/TransactionRules/Engine/RuleEngineInterface.php @@ -55,6 +55,7 @@ interface RuleEngineInterface * @param User $user */ public function setUser(User $user): void; + /** * Add rules for the engine to execute. * @@ -86,4 +87,11 @@ interface RuleEngineInterface */ public function find(): Collection; + /** + * Return the number of changed transactions from the previous "fire" action. + * + * @return int + */ + public function getResults(): int; + } diff --git a/app/TransactionRules/Engine/SearchRuleEngine.php b/app/TransactionRules/Engine/SearchRuleEngine.php index 0a95beb8c3..09d2cb2c7c 100644 --- a/app/TransactionRules/Engine/SearchRuleEngine.php +++ b/app/TransactionRules/Engine/SearchRuleEngine.php @@ -21,25 +21,6 @@ */ declare(strict_types=1); -/* - * SearchRuleEngine.php - * Copyright (c) 2020 james@firefly-iii.org - * - * This file is part of Firefly III (https://github.com/firefly-iii). - * - * This program is free software: you can redistribute it and/or modify - * it under the terms of the GNU Affero General Public License as - * published by the Free Software Foundation, either version 3 of the - * License, or (at your option) any later version. - * - * This program is distributed in the hope that it will be useful, - * but WITHOUT ANY WARRANTY; without even the implied warranty of - * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the - * GNU Affero General Public License for more details. - * - * You should have received a copy of the GNU Affero General Public License - * along with this program. If not, see . - */ namespace FireflyIII\TransactionRules\Engine; @@ -65,12 +46,14 @@ class SearchRuleEngine implements RuleEngineInterface private Collection $rules; private array $operators; private Collection $groups; + private array $resultCount; public function __construct() { - $this->rules = new Collection; - $this->groups = new Collection; - $this->operators = []; + $this->rules = new Collection; + $this->groups = new Collection; + $this->operators = []; + $this->resultCount = []; } /** @@ -125,6 +108,7 @@ class SearchRuleEngine implements RuleEngineInterface */ public function fire(): void { + $this->resultCount = []; Log::debug('SearchRuleEngine::fire()!'); // if rules and no rule groups, file each rule separately. @@ -169,6 +153,16 @@ class SearchRuleEngine implements RuleEngineInterface return $collection->unique(); } + /** + * Return the number of changed transactions from the previous "fire" action. + * + * @return int + */ + public function getResults(): int + { + return count($this->resultCount); + } + /** * Returns true if the rule has been triggered. * @@ -249,9 +243,28 @@ class SearchRuleEngine implements RuleEngineInterface { Log::debug(sprintf('Executing rule action "%s" with value "%s"', $ruleAction->action_type, $ruleAction->action_value)); $actionClass = ActionFactory::getAction($ruleAction); - $actionClass->actOnArray($transaction); + $result = $actionClass->actOnArray($transaction); + $journalId = $transaction['transaction_journal_id'] ?? 0; + if (true === $result) { + $this->resultCount[$journalId] = isset($this->resultCount[$journalId]) ? $this->resultCount[$journalId]++ : 1; + Log::debug( + sprintf( + 'Action "%s" on journal #%d was executed, so count a result. Updated transaction journal count is now %d.', + $ruleAction->action_type, + $transaction['transaction_journal_id'] ?? 0, + count($this->resultCount), + ) + ); + } + if (false === $result) { + Log::debug(sprintf('Action "%s" reports NO changes were made.', $ruleAction->action_type)); + } + + // pick up from the action if it actually acted or not: + + if ($ruleAction->stop_processing) { - Log::debug(sprintf('Rule action "%s" asks to break, so break!', $ruleAction->action_value)); + Log::debug(sprintf('Rule action "%s" asks to break, so break!', $ruleAction->action_type)); return true; } diff --git a/resources/lang/en_US/firefly.php b/resources/lang/en_US/firefly.php index bed862a9ac..e6417acee9 100644 --- a/resources/lang/en_US/firefly.php +++ b/resources/lang/en_US/firefly.php @@ -423,7 +423,7 @@ return [ '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.', + 'applied_rule_selection' => '{0} No transactions in your selection were changed by rule ":title".|[1] One transaction in your selection was changed by rule ":title".|[2,*] :count transactions in your selection were changed by rule ":title".', '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.',