From d624f20107a4067cd5665065be4a61537b91c6da Mon Sep 17 00:00:00 2001 From: James Cole Date: Sun, 14 Oct 2018 16:40:12 +0200 Subject: [PATCH] Fixes for #1747 --- app/Console/Commands/ApplyRules.php | 106 ++++++++++++------ ...ExecuteRuleGroupOnExistingTransactions.php | 15 +-- 2 files changed, 82 insertions(+), 39 deletions(-) diff --git a/app/Console/Commands/ApplyRules.php b/app/Console/Commands/ApplyRules.php index 8569c149be..72fde55db6 100644 --- a/app/Console/Commands/ApplyRules.php +++ b/app/Console/Commands/ApplyRules.php @@ -6,6 +6,7 @@ use Carbon\Carbon; use FireflyIII\Helpers\Collector\TransactionCollectorInterface; use FireflyIII\Models\AccountType; use FireflyIII\Models\Rule; +use FireflyIII\Models\RuleGroup; use FireflyIII\Models\Transaction; use FireflyIII\Repositories\Account\AccountRepositoryInterface; use FireflyIII\Repositories\Journal\JournalRepositoryInterface; @@ -49,6 +50,10 @@ class ApplyRules extends Command /** @var Carbon */ private $endDate; /** @var Collection */ + private $results; + /** @var Collection */ + private $ruleGroups; + /** @var Collection */ private $rules; /** @var Carbon */ private $startDate; @@ -61,8 +66,10 @@ class ApplyRules extends Command public function __construct() { parent::__construct(); - $this->accounts = new Collection; - $this->rules = new Collection; + $this->accounts = new Collection; + $this->rules = new Collection; + $this->ruleGroups = new Collection; + $this->results = new Collection; } /** @@ -84,6 +91,7 @@ class ApplyRules extends Command return 1; } + // get transactions from asset accounts. /** @var TransactionCollectorInterface $collector */ $collector = app(TransactionCollectorInterface::class); @@ -92,40 +100,40 @@ class ApplyRules extends Command $collector->setRange($this->startDate, $this->endDate); $transactions = $collector->getTransactions(); $count = $transactions->count(); - $bar = $this->output->createProgressBar($count * $this->rules->count()); - $results = new Collection; - $this->line(sprintf('Will apply %d rules to %d transactions', $this->rules->count(), $transactions->count())); + // first run all rule groups: + /** @var RuleGroupRepositoryInterface $ruleGroupRepos */ + $ruleGroupRepos = app(RuleGroupRepositoryInterface::class); + $ruleGroupRepos->setUser($this->getUser()); - foreach ($this->rules as $rule) { - /** @var Processor $processor */ - $processor = app(Processor::class); - $processor->make($rule, true); - - /** @var Transaction $transaction */ - foreach ($transactions as $transaction) { - /** @var Rule $rule */ - $bar->advance(); - $result = $processor->handleTransaction($transaction); - if (true === $result) { - $results->push($transaction); - } - } + /** @var RuleGroup $ruleGroup */ + foreach ($this->ruleGroups as $ruleGroup) { + $this->line(sprintf('Going to apply rule group "%s" to %d transaction(s).', $ruleGroup->title, $count)); + $rules = $ruleGroupRepos->getActiveStoreRules($ruleGroup); + $this->applyRuleSelection($rules, $transactions, true); } - $results = $results->unique( + + // then run all rules (rule groups should be empty). + if ($this->rules->count() > 0) { + + $this->line(sprintf('Will apply %d rule(s) to %d transaction(s)', $this->rules->count(), $transactions->count())); + $this->applyRuleSelection($this->rules, $transactions, false); + } + + // filter results: + $this->results = $this->results->unique( function (Transaction $transaction) { return (int)$transaction->journal_id; } ); - $bar->finish(); $this->line(''); - if (0 === \count($results)) { + if (0 === $this->results->count()) { $this->line('The rules were fired but did not influence any transactions.'); } - if (\count($results) > 0) { - $this->line(sprintf('The rules were fired, and influences %d transaction(s).', \count($results))); - foreach ($results as $result) { + if ($this->results->count() > 0) { + $this->line(sprintf('The rule(s) was/were fired, and influenced %d transaction(s).', $this->results->count())); + foreach ($this->results as $result) { $this->line( vsprintf( 'Transaction #%d: "%s" (%s %s)', @@ -143,6 +151,40 @@ class ApplyRules extends Command return 0; } + /** + * @param Collection $rules + * @param Collection $transactions + * @param bool $breakProcessing + * + * @throws \FireflyIII\Exceptions\FireflyException + */ + private function applyRuleSelection(Collection $rules, Collection $transactions, bool $breakProcessing): void + { + $bar = $this->output->createProgressBar($rules->count() * $transactions->count()); + foreach ($rules as $rule) { + /** @var Processor $processor */ + $processor = app(Processor::class); + $processor->make($rule, true); + + /** @var Transaction $transaction */ + foreach ($transactions as $transaction) { + /** @var Rule $rule */ + $bar->advance(); + $result = $processor->handleTransaction($transaction); + if (true === $result) { + $this->results->push($transaction); + } + } + if (true === $rule->stop_processing && true === $breakProcessing) { + $this->line(''); + $this->line(sprintf('Rule #%d ("%s") says to stop processing.', $rule->id, $rule->title)); + + return; + } + } + $this->line(''); + } + /** * */ @@ -153,6 +195,9 @@ class ApplyRules extends Command $ruleRepos = app(RuleRepositoryInterface::class); $ruleRepos->setUser($this->getUser()); $this->rules = $ruleRepos->getAll(); + + // reset rule groups. + $this->ruleGroups = new Collection; } } @@ -284,7 +329,6 @@ class ApplyRules extends Command // can be empty. return true; } - $finalList = new Collection; $ruleGroupList = explode(',', $ruleGroupString); if (0 === \count($ruleGroupList)) { @@ -299,12 +343,8 @@ class ApplyRules extends Command foreach ($ruleGroupList as $ruleGroupId) { $ruleGroupId = (int)$ruleGroupId; $ruleGroup = $ruleGroupRepos->find($ruleGroupId); - if (null !== $ruleGroup) { - $rules = $ruleGroupRepos->getActiveStoreRules($ruleGroup); - $finalList = $finalList->merge($rules); - } + $this->ruleGroups->push($ruleGroup); } - $this->rules = $finalList; return true; } @@ -340,7 +380,9 @@ class ApplyRules extends Command } } if ($finalList->count() > 0) { - $this->rules = $finalList; + // reset rule groups. + $this->ruleGroups = new Collection; + $this->rules = $finalList; } return true; diff --git a/app/Jobs/ExecuteRuleGroupOnExistingTransactions.php b/app/Jobs/ExecuteRuleGroupOnExistingTransactions.php index f6901a6f3f..ec1931df95 100644 --- a/app/Jobs/ExecuteRuleGroupOnExistingTransactions.php +++ b/app/Jobs/ExecuteRuleGroupOnExistingTransactions.php @@ -154,15 +154,15 @@ class ExecuteRuleGroupOnExistingTransactions extends Job implements ShouldQueue $processors = $this->collectProcessors(); // Execute the rules for each transaction - foreach ($transactions as $transaction) { - /** @var Processor $processor */ - foreach ($processors as $processor) { + foreach ($processors as $processor) { + foreach ($transactions as $transaction) { + /** @var Processor $processor */ $processor->handleTransaction($transaction); - // Stop processing this group if the rule specifies 'stop_processing' - if ($processor->getRule()->stop_processing) { - break; - } + } + // Stop processing this group if the rule specifies 'stop_processing' + if ($processor->getRule()->stop_processing) { + break; } } } @@ -203,6 +203,7 @@ class ExecuteRuleGroupOnExistingTransactions extends Job implements ShouldQueue /** @var Processor $processor */ $processor = app(Processor::class); $processor->make($rule); + return $processor; }, $rules->all()