From b0c9745982512b846edd15188b2e1298b368e869 Mon Sep 17 00:00:00 2001 From: James Cole Date: Fri, 12 Mar 2021 06:11:34 +0100 Subject: [PATCH] Make sure rules respect active flag. --- .../V1/Requests/Models/Rule/UpdateRequest.php | 54 +++++------ .../TransactionCurrency/UpdateRequest.php | 2 - app/Repositories/Rule/RuleRepository.php | 95 +++++++++++++------ app/Transformers/RuleTransformer.php | 4 +- 4 files changed, 92 insertions(+), 63 deletions(-) diff --git a/app/Api/V1/Requests/Models/Rule/UpdateRequest.php b/app/Api/V1/Requests/Models/Rule/UpdateRequest.php index ae41e657f9..f1d3ef43be 100644 --- a/app/Api/V1/Requests/Models/Rule/UpdateRequest.php +++ b/app/Api/V1/Requests/Models/Rule/UpdateRequest.php @@ -46,31 +46,27 @@ class UpdateRequest extends FormRequest */ public function getAll(): array { - $strict = null; - $active = null; - $stopProcessing = null; - if (null !== $this->get('active')) { - $active = $this->boolean('active'); + $fields = [ + 'title' => ['title', 'string'], + 'description' => ['description', 'nlString'], + 'rule_group_id' => ['rule_group_id', 'integer'], + 'trigger' => ['trigger', 'string'], + 'strict' => ['strict', 'boolean'], + 'stop_processing' => ['stop_processing', 'boolean'], + 'active' => ['active', 'boolean'], + ]; + + $return = $this->getAllData($fields); + $triggers = $this->getRuleTriggers(); + $actions = $this->getRuleActions(); + if(null !== $triggers) { + $return['triggers'] = $triggers; } - if (null !== $this->get('strict')) { - $strict = $this->boolean('strict'); - } - if (null !== $this->get('stop_processing')) { - $stopProcessing = $this->boolean('stop_processing'); + if(null !== $actions) { + $return['actions'] = $actions; } - return [ - 'title' => $this->nullableString('title'), - 'description' => $this->nullableString('description'), - 'rule_group_id' => $this->nullableInteger('rule_group_id'), - 'rule_group_title' => $this->nullableString('rule_group_title'), - 'trigger' => $this->nullableString('trigger'), - 'strict' => $strict, - 'stop_processing' => $stopProcessing, - 'active' => $active, - 'triggers' => $this->getRuleTriggers(), - 'actions' => $this->getRuleActions(), - ]; + return $return; } /** @@ -85,11 +81,13 @@ class UpdateRequest extends FormRequest $return = []; if (is_array($triggers)) { foreach ($triggers as $trigger) { + $active = array_key_exists('active', $trigger) ? $trigger['active'] : true; + $stopProcessing= array_key_exists('stop_processing', $trigger) ? $trigger['stop_processing'] : false; $return[] = [ 'type' => $trigger['type'], 'value' => $trigger['value'], - 'active' => $this->convertBoolean((string) ($trigger['active'] ?? 'false')), - 'stop_processing' => $this->convertBoolean((string) ($trigger['stop_processing'] ?? 'false')), + 'active' => $active, + 'stop_processing' => $stopProcessing, ]; } } @@ -112,8 +110,8 @@ class UpdateRequest extends FormRequest $return[] = [ 'type' => $action['type'], 'value' => $action['value'], - 'active' => $this->convertBoolean((string) ($action['active'] ?? 'false')), - 'stop_processing' => $this->convertBoolean((string) ($action['stop_processing'] ?? 'false')), + 'active' => $this->convertBoolean((string)($action['active'] ?? 'false')), + 'stop_processing' => $this->convertBoolean((string)($action['stop_processing'] ?? 'false')), ]; } } @@ -184,7 +182,7 @@ class UpdateRequest extends FormRequest $triggers = $data['triggers'] ?? null; // need at least one trigger if (is_array($triggers) && 0 === count($triggers)) { - $validator->errors()->add('title', (string) trans('validation.at_least_one_trigger')); + $validator->errors()->add('title', (string)trans('validation.at_least_one_trigger')); } } @@ -199,7 +197,7 @@ class UpdateRequest extends FormRequest $actions = $data['actions'] ?? null; // need at least one action if (is_array($actions) && 0 === count($actions)) { - $validator->errors()->add('title', (string) trans('validation.at_least_one_action')); + $validator->errors()->add('title', (string)trans('validation.at_least_one_action')); } } } diff --git a/app/Api/V1/Requests/Models/TransactionCurrency/UpdateRequest.php b/app/Api/V1/Requests/Models/TransactionCurrency/UpdateRequest.php index 05fb87c02a..359b1533cd 100644 --- a/app/Api/V1/Requests/Models/TransactionCurrency/UpdateRequest.php +++ b/app/Api/V1/Requests/Models/TransactionCurrency/UpdateRequest.php @@ -46,7 +46,6 @@ class UpdateRequest extends FormRequest public function getAll(): array { // return nothing that isn't explicitely in the array: - // this is the way $fields = [ 'name' => ['name', 'string'], 'code' => ['code', 'string'], @@ -56,7 +55,6 @@ class UpdateRequest extends FormRequest 'enabled' => ['enabled', 'boolean'], ]; - // this is the way. $return = $this->getAllData($fields); return $return; diff --git a/app/Repositories/Rule/RuleRepository.php b/app/Repositories/Rule/RuleRepository.php index dfd44c40c7..365fbbe0d2 100644 --- a/app/Repositories/Rule/RuleRepository.php +++ b/app/Repositories/Rule/RuleRepository.php @@ -30,7 +30,6 @@ use FireflyIII\Models\RuleTrigger; use FireflyIII\Support\Search\OperatorQuerySearch; use FireflyIII\User; use Illuminate\Support\Collection; -use Log; /** * Class RuleRepository. @@ -110,7 +109,7 @@ class RuleRepository implements RuleRepositoryInterface */ public function getHighestOrderInRuleGroup(RuleGroup $ruleGroup): int { - return (int) $ruleGroup->rules()->max('order'); + return (int)$ruleGroup->rules()->max('order'); } /** @@ -319,7 +318,7 @@ class RuleRepository implements RuleRepositoryInterface $ruleAction = new RuleAction; $ruleAction->rule()->associate($rule); $ruleAction->order = $values['order']; - $ruleAction->active = true; + $ruleAction->active = $values['active']; $ruleAction->stop_processing = $values['stop_processing']; $ruleAction->action_type = $values['action']; $ruleAction->action_value = $values['value'] ?? ''; @@ -339,7 +338,7 @@ class RuleRepository implements RuleRepositoryInterface $ruleTrigger = new RuleTrigger; $ruleTrigger->rule()->associate($rule); $ruleTrigger->order = $values['order']; - $ruleTrigger->active = true; + $ruleTrigger->active = $values['active']; $ruleTrigger->stop_processing = $values['stop_processing']; $ruleTrigger->trigger_type = $values['action']; $ruleTrigger->trigger_value = $values['value'] ?? ''; @@ -357,24 +356,37 @@ class RuleRepository implements RuleRepositoryInterface public function update(Rule $rule, array $data): Rule { // update rule: - - $rule->rule_group_id = $data['rule_group_id'] ?? $rule->rule_group_id; - $rule->active = $data['active'] ?? $rule->active; - $rule->stop_processing = $data['stop_processing'] ?? $rule->stop_processing; - $rule->title = $data['title'] ?? $rule->title; - $rule->strict = $data['strict'] ?? $rule->strict; - $rule->description = $data['description'] ?? $rule->description; + $fields = [ + 'title', + 'description', + 'strict', + 'rule_group_id', + 'active', + 'stop_processing', + ]; + foreach ($fields as $field) { + if (array_key_exists($field, $data)) { + $rule->$field = $data[$field]; + } + } $rule->save(); - - if (null !== $data['triggers']) { + // update the triggers: + if (array_key_exists('trigger', $data) && 'update-journal' === $data['trigger']) { + $this->setRuleTrigger('update-journal', $rule); + } + if (array_key_exists('trigger', $data) && 'store-journal' === $data['trigger']) { + $this->setRuleTrigger('store-journal', $rule); + } + if (array_key_exists('triggers', $data)) { // delete triggers: - $rule->ruleTriggers()->delete(); + $rule->ruleTriggers()->where('trigger_type', '!=', 'user_action')->delete(); // recreate triggers: $this->storeTriggers($rule, $data); } - if (null !== $data['actions']) { - // delete actions: + + if (array_key_exists('actions', $data)) { + // delete triggers: $rule->ruleActions()->delete(); // recreate actions: @@ -384,6 +396,30 @@ class RuleRepository implements RuleRepositoryInterface return $rule; } + /** + * @param string $moment + * @param Rule $rule + */ + private function setRuleTrigger(string $moment, Rule $rule): void + { + /** @var RuleTrigger $trigger */ + $trigger = $rule->ruleTriggers()->where('trigger_type', 'user_action')->first(); + if (null !== $trigger) { + $trigger->trigger_value = $moment; + $trigger->save(); + + return; + } + $trigger = new RuleTrigger; + $trigger->order = 0; + $trigger->trigger_type = 'user_action'; + $trigger->trigger_value = $moment; + $trigger->rule_id = $rule->id; + $trigger->active = true; + $trigger->stop_processing = false; + $trigger->save(); + } + /** * @param Rule $rule * @param array $data @@ -396,16 +432,18 @@ class RuleRepository implements RuleRepositoryInterface foreach ($data['actions'] as $action) { $value = $action['value'] ?? ''; $stopProcessing = $action['stop_processing'] ?? false; - - $actionValues = [ + $active = $action['active'] ?? true; + $actionValues = [ 'action' => $action['type'], 'value' => $value, 'stop_processing' => $stopProcessing, 'order' => $order, + 'active' => $active, ]; app('telemetry')->feature('rules.actions.uses_action', $action['type']); $this->storeAction($rule, $actionValues); + ++$order; } return true; @@ -419,26 +457,18 @@ class RuleRepository implements RuleRepositoryInterface */ private function storeTriggers(Rule $rule, array $data): bool { - $order = 1; - $stopProcessing = false; - - $triggerValues = [ - 'action' => 'user_action', - 'value' => $data['trigger'], - 'stop_processing' => $stopProcessing, - 'order' => $order, - ]; - - $this->storeTrigger($rule, $triggerValues); + $order = 1; foreach ($data['triggers'] as $trigger) { $value = $trigger['value'] ?? ''; $stopProcessing = $trigger['stop_processing'] ?? false; + $active = $trigger['active'] ?? true; $triggerValues = [ 'action' => $trigger['type'], 'value' => $value, 'stop_processing' => $stopProcessing, 'order' => $order, + 'active' => $active, ]; app('telemetry')->feature('rules.triggers.uses_trigger', $trigger['type']); @@ -455,7 +485,7 @@ class RuleRepository implements RuleRepositoryInterface public function duplicate(Rule $rule): Rule { $newRule = $rule->replicate(); - $newRule->title = (string) trans('firefly.rule_copy_of', ['title' => $rule->title]); + $newRule->title = (string)trans('firefly.rule_copy_of', ['title' => $rule->title]); $newRule->save(); // replicate all triggers @@ -514,6 +544,7 @@ class RuleRepository implements RuleRepositoryInterface } } } + return $filtered; } @@ -540,6 +571,7 @@ class RuleRepository implements RuleRepositoryInterface } } } + return $filtered; } @@ -562,6 +594,7 @@ class RuleRepository implements RuleRepositoryInterface $params[] = sprintf('%s:"%s"', OperatorQuerySearch::getRootOperator($trigger->trigger_type), $trigger->trigger_value); } } + return implode(' ', $params); } @@ -578,6 +611,6 @@ class RuleRepository implements RuleRepositoryInterface $search->orderBy('rules.order', 'ASC') ->orderBy('rules.title', 'ASC'); - return $search->take($limit)->get(['id','title','description']); + return $search->take($limit)->get(['id', 'title', 'description']); } } diff --git a/app/Transformers/RuleTransformer.php b/app/Transformers/RuleTransformer.php index ce1359ba35..c715d8570f 100644 --- a/app/Transformers/RuleTransformer.php +++ b/app/Transformers/RuleTransformer.php @@ -61,10 +61,10 @@ class RuleTransformer extends AbstractTransformer $this->ruleRepository->setUser($rule->user); return [ - 'id' => (int)$rule->id, + 'id' => (string)$rule->id, 'created_at' => $rule->created_at->toAtomString(), 'updated_at' => $rule->updated_at->toAtomString(), - 'rule_group_id' => (string)$rule->rule_group_id, + 'rule_group_id' => (string) $rule->rule_group_id, 'title' => $rule->title, 'description' => $rule->description, 'order' => (int)$rule->order,