From 422e80530b1232fa0c12d05c76d5599e495d228f Mon Sep 17 00:00:00 2001 From: James Cole Date: Sun, 5 Aug 2018 15:34:20 +0200 Subject: [PATCH] Refactor rule creation. --- .../Controllers/Rule/CreateController.php | 51 ++--- .../Controllers/Rule/SelectController.php | 20 +- app/Http/Requests/RuleFormRequest.php | 100 +++++---- app/Repositories/Rule/RuleRepository.php | 40 ++-- .../Http/Controllers/RuleManagement.php | 86 ++++---- .../Factory/TriggerFactory.php | 2 +- app/TransactionRules/Processor.php | 4 +- app/Validation/FireflyValidator.php | 193 +++++++----------- public/js/ff/rules/create-edit.js | 139 +++++++++---- resources/views/rules/partials/action.twig | 10 +- resources/views/rules/partials/trigger.twig | 11 +- routes/breadcrumbs.php | 18 +- 12 files changed, 337 insertions(+), 337 deletions(-) diff --git a/app/Http/Controllers/Rule/CreateController.php b/app/Http/Controllers/Rule/CreateController.php index e35b243640..622440301d 100644 --- a/app/Http/Controllers/Rule/CreateController.php +++ b/app/Http/Controllers/Rule/CreateController.php @@ -30,7 +30,6 @@ use FireflyIII\Models\Bill; use FireflyIII\Models\RuleGroup; use FireflyIII\Repositories\Bill\BillRepositoryInterface; use FireflyIII\Repositories\Rule\RuleRepositoryInterface; - use FireflyIII\Support\Http\Controllers\RuleManagement; use Illuminate\Http\RedirectResponse; use Illuminate\Http\Request; @@ -71,6 +70,8 @@ class CreateController extends Controller /** * Create a new rule. It will be stored under the given $ruleGroup. * + * TODO reinstate bill specific code. + * * @param Request $request * @param RuleGroup $ruleGroup * @@ -78,51 +79,33 @@ class CreateController extends Controller * @SuppressWarnings(PHPMD.ExcessiveMethodLength) * @SuppressWarnings(PHPMD.CyclomaticComplexity) */ - public function create(Request $request, RuleGroup $ruleGroup) + public function create(Request $request, RuleGroup $ruleGroup = null) { $this->createDefaultRuleGroup(); $this->createDefaultRule(); - $bill = null; - $billId = (int)$request->get('fromBill'); - $preFilled = [ + $preFilled = [ 'strict' => true, ]; - $oldTriggers = []; - $oldActions = []; - $returnToBill = false; + $oldTriggers = []; + $oldActions = []; - if ('true' === $request->get('return')) { - $returnToBill = true; - } - - // has bill? - if ($billId > 0) { - $bill = $this->billRepos->find($billId); - } - - // has old input? + // restore actions and triggers from old input: if ($request->old()) { $oldTriggers = $this->getPreviousTriggers($request); $oldActions = $this->getPreviousActions($request); } - // has existing bill refered to in URI? - if (null !== $bill && !$request->old()) { - - // create some sensible defaults: - $preFilled['title'] = (string)trans('firefly.new_rule_for_bill_title', ['name' => $bill->name]); - $preFilled['description'] = (string)trans('firefly.new_rule_for_bill_description', ['name' => $bill->name]); - - - // get triggers and actions for bill: - $oldTriggers = $this->getTriggersForBill($bill); - $oldActions = $this->getActionsForBill($bill); - } $triggerCount = \count($oldTriggers); $actionCount = \count($oldActions); $subTitleIcon = 'fa-clone'; - $subTitle = (string)trans('firefly.make_new_rule', ['title' => $ruleGroup->title]); + // title depends on whether or not there is a rule group: + $subTitle = (string)trans('firefly.make_new_rule_no_group'); + if (null !== $ruleGroup) { + $subTitle = (string)trans('firefly.make_new_rule', ['title' => $ruleGroup->title]); + } + + // flash old data $request->session()->flash('preFilled', $preFilled); // put previous url in session if not redirect from store (not "create another"). @@ -132,11 +115,7 @@ class CreateController extends Controller session()->forget('rules.create.fromStore'); return view( - 'rules.rule.create', - compact( - 'subTitleIcon', 'oldTriggers', 'returnToBill', 'preFilled', 'bill', 'oldActions', 'triggerCount', 'actionCount', 'ruleGroup', - 'subTitle' - ) + 'rules.rule.create', compact('subTitleIcon', 'oldTriggers', 'preFilled', 'oldActions', 'triggerCount', 'actionCount', 'ruleGroup', 'subTitle') ); } diff --git a/app/Http/Controllers/Rule/SelectController.php b/app/Http/Controllers/Rule/SelectController.php index b8c30229f2..fbc3afc763 100644 --- a/app/Http/Controllers/Rule/SelectController.php +++ b/app/Http/Controllers/Rule/SelectController.php @@ -124,7 +124,6 @@ class SelectController extends Controller return view('rules.rule.select-transactions', compact('first', 'today', 'rule', 'subTitle')); } - /** * 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. @@ -267,18 +266,13 @@ class SelectController extends Controller private function getValidTriggerList(TestRuleFormRequest $request): array { $triggers = []; - $data = [ - 'rule-triggers' => $request->get('rule-trigger'), - 'rule-trigger-values' => $request->get('rule-trigger-value'), - 'rule-trigger-stop' => $request->get('rule-trigger-stop'), - ]; - if (\is_array($data['rule-triggers'])) { - foreach ($data['rule-triggers'] as $index => $triggerType) { - $data['rule-trigger-stop'][$index] = (int)($data['rule-trigger-stop'][$index] ?? 0.0); - $triggers[] = [ - 'type' => $triggerType, - 'value' => $data['rule-trigger-values'][$index], - 'stopProcessing' => 1 === (int)$data['rule-trigger-stop'][$index], + $data = $request->get('rule_triggers'); + if (\is_array($data)) { + foreach ($data as $index => $triggerInfo) { + $triggers[] = [ + 'type' => $triggerInfo['name'] ?? '', + 'value' => $triggerInfo['value'] ?? '', + 'stop_processing' => 1 === (int)($triggerInfo['stop_processing'] ?? '0'), ]; } } diff --git a/app/Http/Requests/RuleFormRequest.php b/app/Http/Requests/RuleFormRequest.php index 84a6e3fe30..de1e104185 100644 --- a/app/Http/Requests/RuleFormRequest.php +++ b/app/Http/Requests/RuleFormRequest.php @@ -50,45 +50,17 @@ class RuleFormRequest extends Request */ public function getRuleData(): array { - $data = [ + $data = [ 'title' => $this->string('title'), 'rule_group_id' => $this->integer('rule_group_id'), 'active' => $this->boolean('active'), 'trigger' => $this->string('trigger'), 'description' => $this->string('description'), - 'stop-processing' => $this->boolean('stop_processing'), + 'stop_processing' => $this->boolean('stop_processing'), 'strict' => $this->boolean('strict'), - 'rule-triggers' => [], - 'rule-actions' => [], + 'rule_triggers' => $this->getRuleTriggerData(), + 'rule_actions' => $this->getRuleActionData(), ]; - $triggers = $this->get('rule-trigger'); - $triggerValues = $this->get('rule-trigger-value'); - $triggerStop = $this->get('rule-trigger-stop'); - - $actions = $this->get('rule-action'); - $actionValues = $this->get('rule-action-value'); - $actionStop = $this->get('rule-action-stop'); - - if (\is_array($triggers)) { - foreach ($triggers as $index => $value) { - $data['rule-triggers'][] = [ - 'name' => $value, - 'value' => $triggerValues[$index] ?? '', - 'stop-processing' => 1 === (int)($triggerStop[$index] ?? 0), - ]; - } - } - - if (\is_array($actions)) { - foreach ($actions as $index => $value) { - $data['rule-actions'][] = [ - 'name' => $value, - 'value' => $actionValues[$index] ?? '', - 'stop-processing' => 1 === (int)($actionStop[$index] ?? 0), - ]; - } - } - return $data; } @@ -112,21 +84,65 @@ class RuleFormRequest extends Request $titleRule = 'required|between:1,100|uniqueObjectForUser:rules,title,' . (int)$this->get('id'); } $rules = [ - 'title' => $titleRule, - 'description' => 'between:1,5000|nullable', - 'stop_processing' => 'boolean', - 'rule_group_id' => 'required|belongsToUser:rule_groups', - 'trigger' => 'required|in:store-journal,update-journal', - 'rule-trigger.*' => 'required|in:' . implode(',', $validTriggers), - 'rule-trigger-value.*' => 'required|min:1|ruleTriggerValue', - 'rule-action.*' => 'required|in:' . implode(',', $validActions), - 'strict' => 'in:0,1', + 'title' => $titleRule, + 'description' => 'between:1,5000|nullable', + 'stop_processing' => 'boolean', + 'rule_group_id' => 'required|belongsToUser:rule_groups', + 'trigger' => 'required|in:store-journal,update-journal', + 'rule_triggers.*.name' => 'required|in:' . implode(',', $validTriggers), + 'rule_triggers.*.value' => 'required|min:1|ruleTriggerValue', + 'rule-actions.*.name' => 'required|in:' . implode(',', $validActions), + 'strict' => 'in:0,1', ]; // since Laravel does not support this stuff yet, here's a trick. for ($i = 0; $i < 10; ++$i) { - $rules['rule-action-value.' . $i] = 'required_if:rule-action.' . $i . ',' . $contextActions . '|ruleActionValue'; + $key = sprintf('rule_actions.%d.value', $i); + $rule = sprintf('required-if:rule_actions.%d.name,%s|ruleActionValue', $i, $contextActions); + $rules[$key] = $rule; } return $rules; } + + /** + * @return array + */ + private function getRuleActionData(): array + { + $return = []; + $actionData= $this->get('rule_actions'); + if (\is_array($actionData)) { + foreach ($actionData as $action) { + $stopProcessing = $action['stop_processing'] ?? '0'; + $return[] = [ + 'name' => $action['name'] ?? 'invalid', + 'value' => $action['value'] ?? '', + 'stop_processing' => 1 === (int)$stopProcessing, + ]; + } + } + + return $return; + } + + /** + * @return array + */ + private function getRuleTriggerData(): array + { + $return = []; + $triggerData = $this->get('rule_triggers'); + if (\is_array($triggerData)) { + foreach ($triggerData as $trigger) { + $stopProcessing = $trigger['stop_processing'] ?? '0'; + $return[] = [ + 'name' => $trigger['name'] ?? 'invalid', + 'value' => $trigger['value'] ?? '', + 'stop_processing' => 1 === (int)$stopProcessing, + ]; + } + } + + return $return; + } } diff --git a/app/Repositories/Rule/RuleRepository.php b/app/Repositories/Rule/RuleRepository.php index a0b4ae4b3b..a17f1ff9c9 100644 --- a/app/Repositories/Rule/RuleRepository.php +++ b/app/Repositories/Rule/RuleRepository.php @@ -292,7 +292,7 @@ class RuleRepository implements RuleRepositoryInterface $rule->order = ($order + 1); $rule->active = true; $rule->strict = $data['strict'] ?? false; - $rule->stop_processing = 1 === (int)$data['stop-processing']; + $rule->stop_processing = 1 === (int)$data['stop_processing']; $rule->title = $data['title']; $rule->description = \strlen($data['description']) > 0 ? $data['description'] : null; @@ -319,7 +319,7 @@ class RuleRepository implements RuleRepositoryInterface $ruleAction->rule()->associate($rule); $ruleAction->order = $values['order']; $ruleAction->active = true; - $ruleAction->stop_processing = $values['stopProcessing']; + $ruleAction->stop_processing = $values['stop_processing']; $ruleAction->action_type = $values['action']; $ruleAction->action_value = $values['value'] ?? ''; $ruleAction->save(); @@ -339,7 +339,7 @@ class RuleRepository implements RuleRepositoryInterface $ruleTrigger->rule()->associate($rule); $ruleTrigger->order = $values['order']; $ruleTrigger->active = true; - $ruleTrigger->stop_processing = $values['stopProcessing']; + $ruleTrigger->stop_processing = $values['stop_processing']; $ruleTrigger->trigger_type = $values['action']; $ruleTrigger->trigger_value = $values['value'] ?? ''; $ruleTrigger->save(); @@ -358,7 +358,7 @@ class RuleRepository implements RuleRepositoryInterface // update rule: $rule->rule_group_id = $data['rule_group_id']; $rule->active = $data['active']; - $rule->stop_processing = $data['stop-processing']; + $rule->stop_processing = $data['stop_processing']; $rule->title = $data['title']; $rule->strict = $data['strict'] ?? false; $rule->description = $data['description']; @@ -388,15 +388,15 @@ class RuleRepository implements RuleRepositoryInterface private function storeActions(Rule $rule, array $data): bool { $order = 1; - foreach ($data['rule-actions'] as $action) { + foreach ($data['rule_actions'] as $action) { $value = $action['value'] ?? ''; - $stopProcessing = $action['stop-processing'] ?? false; + $stopProcessing = $action['stop_processing'] ?? false; $actionValues = [ - 'action' => $action['name'], - 'value' => $value, - 'stopProcessing' => $stopProcessing, - 'order' => $order, + 'action' => $action['name'], + 'value' => $value, + 'stop_processing' => $stopProcessing, + 'order' => $order, ]; $this->storeAction($rule, $actionValues); @@ -417,22 +417,22 @@ class RuleRepository implements RuleRepositoryInterface $stopProcessing = false; $triggerValues = [ - 'action' => 'user_action', - 'value' => $data['trigger'], - 'stopProcessing' => $stopProcessing, - 'order' => $order, + 'action' => 'user_action', + 'value' => $data['trigger'], + 'stop_processing' => $stopProcessing, + 'order' => $order, ]; $this->storeTrigger($rule, $triggerValues); - foreach ($data['rule-triggers'] as $trigger) { + foreach ($data['rule_triggers'] as $trigger) { $value = $trigger['value'] ?? ''; - $stopProcessing = $trigger['stop-processing'] ?? false; + $stopProcessing = $trigger['stop_processing'] ?? false; $triggerValues = [ - 'action' => $trigger['name'], - 'value' => $value, - 'stopProcessing' => $stopProcessing, - 'order' => $order, + 'action' => $trigger['name'], + 'value' => $value, + 'stop_processing' => $stopProcessing, + 'order' => $order, ]; $this->storeTrigger($rule, $triggerValues); diff --git a/app/Support/Http/Controllers/RuleManagement.php b/app/Support/Http/Controllers/RuleManagement.php index 7033ae5e25..60d9b0bcb9 100644 --- a/app/Support/Http/Controllers/RuleManagement.php +++ b/app/Support/Http/Controllers/RuleManagement.php @@ -93,33 +93,30 @@ trait RuleManagement */ protected function getPreviousActions(Request $request): array { - $newIndex = 0; - $actions = []; - /** @var array $oldActions */ - $oldActions = \is_array($request->old('rule-action')) ? $request->old('rule-action') : []; - foreach ($oldActions as $index => $entry) { - $count = ($newIndex + 1); - $checked = isset($request->old('rule-action-stop')[$index]) ? true : false; - try { - $actions[] = view( - 'rules.partials.action', - [ - 'oldAction' => $entry, - 'oldValue' => $request->old('rule-action-value')[$index], - 'oldChecked' => $checked, - 'count' => $count, - ] - )->render(); - // @codeCoverageIgnoreStart - } catch (Throwable $e) { - Log::debug(sprintf('Throwable was thrown in getPreviousActions(): %s', $e->getMessage())); - Log::error($e->getTraceAsString()); + $index = 0; + $triggers = []; + $oldInput = $request->old('rule_actions'); + if (\is_array($oldInput)) { + foreach ($oldInput as $oldAction) { + try { + $triggers[] = view( + 'rules.partials.action', + [ + 'oldAction' => $oldAction['name'], + 'oldValue' => $oldAction['value'], + 'oldChecked' => 1 === (int)($oldAction['stop_processing'] ?? '0'), + 'count' => $index + 1, + ] + )->render(); + } catch (Throwable $e) { + Log::debug(sprintf('Throwable was thrown in getPreviousActions(): %s', $e->getMessage())); + Log::error($e->getTraceAsString()); + } + $index++; } - // @codeCoverageIgnoreEnd - ++$newIndex; } - return $actions; + return $triggers; } /** @@ -129,30 +126,27 @@ trait RuleManagement */ protected function getPreviousTriggers(Request $request): array { - $newIndex = 0; + $index = 0; $triggers = []; - /** @var array $oldTriggers */ - $oldTriggers = \is_array($request->old('rule-trigger')) ? $request->old('rule-trigger') : []; - foreach ($oldTriggers as $index => $entry) { - $count = ($newIndex + 1); - $oldChecked = isset($request->old('rule-trigger-stop')[$index]) ? true : false; - try { - $triggers[] = view( - 'rules.partials.trigger', - [ - 'oldTrigger' => $entry, - 'oldValue' => $request->old('rule-trigger-value')[$index], - 'oldChecked' => $oldChecked, - 'count' => $count, - ] - )->render(); - // @codeCoverageIgnoreStart - } catch (Throwable $e) { - Log::debug(sprintf('Throwable was thrown in getPreviousTriggers(): %s', $e->getMessage())); - Log::error($e->getTraceAsString()); + $oldInput = $request->old('rule_triggers'); + if (\is_array($oldInput)) { + foreach ($oldInput as $oldTrigger) { + try { + $triggers[] = view( + 'rules.partials.trigger', + [ + 'oldTrigger' => $oldTrigger['name'], + 'oldValue' => $oldTrigger['value'], + 'oldChecked' => 1 === (int)($oldTrigger['stop_processing'] ?? '0'), + 'count' => $index + 1, + ] + )->render(); + } catch (Throwable $e) { + Log::debug(sprintf('Throwable was thrown in getPreviousTriggers(): %s', $e->getMessage())); + Log::error($e->getTraceAsString()); + } + $index++; } - // @codeCoverageIgnoreEnd - ++$newIndex; } return $triggers; diff --git a/app/TransactionRules/Factory/TriggerFactory.php b/app/TransactionRules/Factory/TriggerFactory.php index 96caf8d78e..3bced52e35 100644 --- a/app/TransactionRules/Factory/TriggerFactory.php +++ b/app/TransactionRules/Factory/TriggerFactory.php @@ -89,7 +89,7 @@ class TriggerFactory /** @var AbstractTrigger $class */ $class = self::getTriggerClass($triggerType); $obj = $class::makeFromStrings($triggerValue, $stopProcessing); - Log::debug('Created trigger from string', ['type' => $triggerType, 'value' => $triggerValue, 'stopProcessing' => $stopProcessing, 'class' => $class]); + Log::debug('Created trigger from string', ['type' => $triggerType, 'value' => $triggerValue, 'stop_processing' => $stopProcessing, 'class' => $class]); return $obj; } diff --git a/app/TransactionRules/Processor.php b/app/TransactionRules/Processor.php index 14d44d0fc7..a3de4387f0 100644 --- a/app/TransactionRules/Processor.php +++ b/app/TransactionRules/Processor.php @@ -122,7 +122,7 @@ final class Processor * * The given triggers must be in the following format: * - * [type => xx, value => yy, stopProcessing => bool], [type => xx, value => yy, stopProcessing => bool], + * [type => xx, value => yy, stop_processing => bool], [type => xx, value => yy, stop_processing => bool], * * @param array $triggers * @@ -135,7 +135,7 @@ final class Processor $self = new self; foreach ($triggers as $entry) { $entry['value'] = $entry['value'] ?? ''; - $trigger = TriggerFactory::makeTriggerFromStrings($entry['type'], $entry['value'], $entry['stopProcessing']); + $trigger = TriggerFactory::makeTriggerFromStrings($entry['type'], $entry['value'], $entry['stop_processing']); $self->triggers->push($trigger); } diff --git a/app/Validation/FireflyValidator.php b/app/Validation/FireflyValidator.php index c44ac55640..fa14ce1b70 100644 --- a/app/Validation/FireflyValidator.php +++ b/app/Validation/FireflyValidator.php @@ -247,148 +247,99 @@ class FireflyValidator extends Validator * * @return bool */ - public function validateRuleActionValue($attribute): bool + public function validateRuleActionValue(string $attribute, string $value): bool { - // get the index from a string like "rule-action-value.2". + // first, get the index from this string: $parts = explode('.', $attribute); - $index = $parts[\count($parts) - 1]; - if ($index === 'value') { - // user is coming from API. - $index = $parts[\count($parts) - 2]; - } - $index = (int)$index; + $index = (int)($parts[1] ?? '0'); - // get actions from $this->data - $actions = []; - if (isset($this->data['rule-action']) && \is_array($this->data['rule-action'])) { - $actions = $this->data['rule-action']; - } - if (isset($this->data['rule-actions']) && \is_array($this->data['rule-actions'])) { - $actions = $this->data['rule-actions']; + // get the name of the trigger from the data array: + $actionType = $this->data['rule_actions'][$index]['name'] ?? 'invalid'; + + // if it's "invalid" return false. + if ('invalid' === $actionType) { + return false; } + // if it's set_budget, verify the budget name: + if ('set_budget' === $actionType) { + /** @var BudgetRepositoryInterface $repository */ + $repository = app(BudgetRepositoryInterface::class); + $budgets = $repository->getBudgets(); + // count budgets, should have at least one + $count = $budgets->filter( + function (Budget $budget) use ($value) { + return $budget->name === $value; + } + )->count(); - // loop all rule-actions. - // check if rule-action-value matches the thing. - if (\is_array($actions)) { - $name = $this->getRuleActionName($index); - $value = $this->getRuleActionValue($index); - switch ($name) { - default: - - return true; - case 'set_budget': - /** @var BudgetRepositoryInterface $repository */ - $repository = app(BudgetRepositoryInterface::class); - $budgets = $repository->getBudgets(); - // count budgets, should have at least one - $count = $budgets->filter( - function (Budget $budget) use ($value) { - return $budget->name === $value; - } - )->count(); - - return 1 === $count; - case 'link_to_bill': - /** @var BillRepositoryInterface $repository */ - $repository = app(BillRepositoryInterface::class); - $bill = $repository->findByName($value); - - return null !== $bill; - case 'invalid': - return false; - } + return 1 === $count; } - return false; + // if it's link to bill, verify the name of the bill. + if ('link_to_bill' === $actionType) { + /** @var BillRepositoryInterface $repository */ + $repository = app(BillRepositoryInterface::class); + $bill = $repository->findByName($value); + + return null !== $bill; + } + + // return true for the rest. + return true; } /** - * @param $attribute + * $attribute has the format rule_triggers.%d.value. + * + * @param string $attribute + * @param string $value * * @return bool */ - public function validateRuleTriggerValue($attribute): bool + public function validateRuleTriggerValue(string $attribute, string $value): bool { - // get the index from a string like "rule-trigger-value.2". + // + + // first, get the index from this string: $parts = explode('.', $attribute); - $index = $parts[\count($parts) - 1]; - // if the index is not a number, then we might be dealing with an API $attribute - // which is formatted "rule-triggers.0.value" - if ($index === 'value') { - $index = $parts[\count($parts) - 2]; - } - $index = (int)$index; + $index = (int)($parts[1] ?? '0'); - // get triggers from $this->data - $triggers = []; - if (isset($this->data['rule-trigger']) && \is_array($this->data['rule-trigger'])) { - $triggers = $this->data['rule-trigger']; - } - if (isset($this->data['rule-triggers']) && \is_array($this->data['rule-triggers'])) { - $triggers = $this->data['rule-triggers']; + // get the name of the trigger from the data array: + $triggerType = $this->data['rule_triggers'][$index]['name'] ?? 'invalid'; + + // invalid always returns false: + if ('invalid' === $triggerType) { + return false; } - // loop all rule-triggers. - // check if rule-value matches the thing. - if (\is_array($triggers)) { - $name = $this->getRuleTriggerName($index); - $value = $this->getRuleTriggerValue($index); - - // break on some easy checks: - switch ($name) { - case 'amount_less': - case 'amount_more': - case 'amount_exactly': - $result = is_numeric($value); - if (false === $result) { - return false; - } - break; - case 'from_account_starts': - case 'from_account_ends': - case 'from_account_is': - case 'from_account_contains': - case 'to_account_starts': - case 'to_account_ends': - case 'to_account_is': - case 'to_account_contains': - case 'description_starts': - case 'description_ends': - case 'description_contains': - case 'description_is': - case 'category_is': - case 'budget_is': - case 'tag_is': - case 'currency_is': - case 'notes_contain': - case 'notes_start': - case 'notes_end': - case 'notes_are': - return \strlen($value) > 0; - - break; - case 'transaction_type': - $count = TransactionType::where('type', $value)->count(); - if (!(1 === $count)) { - return false; - } - break; - case 'invalid': - return false; - } - // still a special case where the trigger is - // triggered in such a way that it would trigger ANYTHING. We can check for such things - // with function willmatcheverything - // we know which class it is so dont bother checking that. - $classes = Config::get('firefly.rule-triggers'); - /** @var TriggerInterface $class */ - $class = $classes[$name]; - - return !$class::willMatchEverything($value); + // these trigger types need a numerical check: + $numerical = ['amount_less', 'amount_more', 'amount_exactly']; + if (\in_array($triggerType, $numerical, true)) { + return is_numeric($value); } - return false; + // these trigger types need a simple strlen check: + $length = ['from_account_starts', 'from_account_ends', 'from_account_is', 'from_account_contains', 'to_account_starts', 'to_account_ends', + 'to_account_is', 'to_account_contains', 'description_starts', 'description_ends', 'description_contains', 'description_is', 'category_is', + 'budget_is', 'tag_is', 'currency_is', 'notes_contain', 'notes_start', 'notes_end', 'notes_are',]; + if (\in_array($triggerType, $length, true)) { + return '' !== $value; + } + + // check transaction type. + if ('transaction_type' === $triggerType) { + $count = TransactionType::where('type', $value)->count(); + + return 1 !== $count; + } + + // and finally a "will match everything check": + $classes = app('config')->get('firefly.rule-triggers'); + /** @var TriggerInterface $class */ + $class = $classes[$triggerType]; + + return !$class::willMatchEverything($value); } /** diff --git a/public/js/ff/rules/create-edit.js b/public/js/ff/rules/create-edit.js index a2d5b89451..1a8f1f2503 100644 --- a/public/js/ff/rules/create-edit.js +++ b/public/js/ff/rules/create-edit.js @@ -22,20 +22,26 @@ $(function () { "use strict"; - if (triggerCount === 0) { - addNewTrigger(); - } - if (actionCount === 0) { - addNewAction(); - } + if (triggerCount > 0) { + console.log('trigger count is larger than zero, call onAddNewTrigger.'); onAddNewTrigger(); } if (actionCount > 0) { + console.log('action count is larger than zero, call onAddNewAction.'); onAddNewAction(); } + if (triggerCount === 0) { + console.log('trigger count is zero, add trigger.'); + addNewTrigger(); + } + if (actionCount === 0) { + console.log('action count is zero, add action.'); + addNewAction(); + } + $('.add_rule_trigger').click(addNewTrigger); $('.add_rule_action').click(addNewAction); $('.test_rule_triggers').click(testRuleTriggers); @@ -49,7 +55,7 @@ $(function () { function addNewTrigger() { "use strict"; triggerCount++; - + console.log('In addNewTrigger(), count is now ' + triggerCount); // disable the button $('.add_rule_trigger').attr('disabled', 'disabled'); @@ -81,7 +87,7 @@ function addNewTrigger() { function addNewAction() { "use strict"; actionCount++; - + console.log('In addNewAction(), count is now ' + actionCount); // disable the button $('.add_rule_action').attr('disabled', 'disabled'); @@ -154,18 +160,24 @@ function removeAction(e) { */ function onAddNewAction() { "use strict"; + console.log('Now in onAddNewAction()'); + + var selectQuery = 'select[name^="rule_actions["][name$="][name]"]'; + var selectResult = $(selectQuery); + + console.log('Select query is "' + selectQuery + '" and the result length is ' + selectResult.length); // update all "select action type" dropdown buttons so they will respond correctly - $('select[name^="rule-action["]').unbind('change').change(function (e) { + selectResult.unbind('change').change(function (e) { var target = $(e.target); updateActionInput(target) }); - $.each($('.rule-action-holder'), function (i, v) { - var holder = $(v); - var select = holder.find('select'); - updateActionInput(select); - }); + // $.each($('.rule-action-holder'), function (i, v) { + // var holder = $(v); + // var select = holder.find('select'); + // updateActionInput(select); + // }); } /** @@ -173,18 +185,24 @@ function onAddNewAction() { */ function onAddNewTrigger() { "use strict"; + console.log('Now in onAddNewTrigger()'); - // update all "select trigger type" dropdown buttons so they will respond correctly - $('select[name^="rule-trigger["]').unbind('change').change(function (e) { + var selectQuery = 'select[name^="rule_triggers["][name$="][name]"]'; + var selectResult = $(selectQuery); + + console.log('Select query is "' + selectQuery + '" and the result length is ' + selectResult.length); + + // trigger when user changes the trigger type. + selectResult.unbind('change').change(function (e) { var target = $(e.target); updateTriggerInput(target) }); - $.each($('.rule-trigger-holder'), function (i, v) { - var holder = $(v); - var select = holder.find('select'); - updateTriggerInput(select); - }); + // $.each($('.rule-trigger-holder'), function (i, v) { + // var holder = $(v); + // var select = holder.find('select'); + // updateTriggerInput(select); + // }); } /** @@ -193,42 +211,56 @@ function onAddNewTrigger() { * @param selectList */ function updateActionInput(selectList) { + console.log('Now in updateActionInput() for a select list, currently with value "' + selectList.val() + '".'); // the actual row this select list is in: var parent = selectList.parent().parent(); // the text input we're looking for: - var input = parent.find('input[name^="rule-action-value["]'); - input.removeAttr('disabled'); + var inputQuery = 'input[name^="rule_actions["][name$="][value]"]'; + var inputResult = parent.find(inputQuery); + + console.log('Searching for children in this row with query "' + inputQuery + '" resulted in ' + inputResult.length + ' results.'); + + inputResult.removeAttr('disabled'); switch (selectList.val()) { case 'set_category': - createAutoComplete(input, 'json/categories'); + console.log('Select list value is ' + selectList.val() +', so input needs auto complete.'); + createAutoComplete(inputResult, 'json/categories'); break; case 'clear_category': case 'clear_budget': case 'clear_notes': case 'remove_all_tags': - input.attr('disabled', 'disabled'); + console.log('Select list value is ' + selectList.val() +', so input needs to be disabled.'); + inputResult.attr('disabled', 'disabled'); break; case 'set_budget': - createAutoComplete(input, 'json/budgets'); + console.log('Select list value is ' + selectList.val() +', so input needs auto complete.'); + createAutoComplete(inputResult, 'json/budgets'); break; case 'add_tag': case 'remove_tag': - createAutoComplete(input, 'json/tags'); + console.log('Select list value is ' + selectList.val() +', so input needs auto complete.'); + createAutoComplete(inputResult, 'json/tags'); break; case 'set_description': - createAutoComplete(input, 'json/transaction-journals/all'); + console.log('Select list value is ' + selectList.val() +', so input needs auto complete.'); + createAutoComplete(inputResult, 'json/transaction-journals/all'); break; case 'set_source_account': - createAutoComplete(input, 'json/all-accounts'); + console.log('Select list value is ' + selectList.val() +', so input needs auto complete.'); + createAutoComplete(inputResult, 'json/all-accounts'); break; case 'set_destination_account': - createAutoComplete(input, 'json/all-accounts'); + console.log('Select list value is ' + selectList.val() +', so input needs auto complete.'); + createAutoComplete(inputResult, 'json/all-accounts'); break; case 'link_to_bill': - createAutoComplete(input, 'json/bills'); + console.log('Select list value is ' + selectList.val() +', so input needs auto complete.'); + createAutoComplete(inputResult, 'json/bills'); break; default: - input.typeahead('destroy'); + console.log('Select list value is ' + selectList.val() +', destroy auto complete, do nothing else.'); + inputResult.typeahead('destroy'); break; } } @@ -239,11 +271,15 @@ function updateActionInput(selectList) { * @param selectList */ function updateTriggerInput(selectList) { + console.log('Now in updateTriggerInput() for a select list, currently with value "' + selectList.val() + '".'); // the actual row this select list is in: var parent = selectList.parent().parent(); // the text input we're looking for: - var input = parent.find('input[name^="rule-trigger-value["]'); - input.prop('disabled', false); + var inputQuery = 'input[name^="rule_triggers["][name$="][value]"]'; + var inputResult = parent.find(inputQuery); + + console.log('Searching for children in this row with query "' + inputQuery + '" resulted in ' + inputResult.length + ' results.'); + inputResult.prop('disabled', false); switch (selectList.val()) { case 'from_account_starts': case 'from_account_ends': @@ -253,26 +289,31 @@ function updateTriggerInput(selectList) { case 'to_account_ends': case 'to_account_is': case 'to_account_contains': - createAutoComplete(input, 'json/all-accounts'); + console.log('Select list value is ' + selectList.val() +', so input needs auto complete.'); + createAutoComplete(inputResult, 'json/all-accounts'); break; case 'tag_is': - // also make tag thing? - createAutoComplete(input, 'json/tags'); + console.log('Select list value is ' + selectList.val() +', so input needs auto complete.'); + createAutoComplete(inputResult, 'json/tags'); break; case 'budget_is': - createAutoComplete(input, 'json/budgets'); + console.log('Select list value is ' + selectList.val() +', so input needs auto complete.'); + createAutoComplete(inputResult, 'json/budgets'); break; case 'category_is': - createAutoComplete(input, 'json/categories'); + console.log('Select list value is ' + selectList.val() +', so input needs auto complete.'); + createAutoComplete(inputResult, 'json/categories'); break; case 'transaction_type': - createAutoComplete(input, 'json/transaction-types'); + console.log('Select list value is ' + selectList.val() +', so input needs auto complete.'); + createAutoComplete(inputResult, 'json/transaction-types'); break; case 'description_starts': case 'description_ends': case 'description_contains': case 'description_is': - createAutoComplete(input, 'json/transaction-journals/all'); + console.log('Select list value is ' + selectList.val() +', so input needs auto complete.'); + createAutoComplete(inputResult, 'json/transaction-journals/all'); break; case 'has_no_category': case 'has_any_category': @@ -282,14 +323,17 @@ function updateTriggerInput(selectList) { case 'no_notes': case 'any_notes': case 'has_any_tag': - input.prop('disabled', true); - input.typeahead('destroy'); + console.log('Select list value is ' + selectList.val() +', so input needs to be disabled.'); + inputResult.prop('disabled', true); + inputResult.typeahead('destroy'); break; case 'currency_is': - createAutoComplete(input, 'json/currency-names'); + console.log('Select list value is ' + selectList.val() +', so input needs auto complete.'); + createAutoComplete(inputResult, 'json/currency-names'); break; default: - input.typeahead('destroy'); + console.log('Select list value is ' + selectList.val() +', destroy auto complete, do nothing else.'); + inputResult.typeahead('destroy'); break; } } @@ -300,9 +344,13 @@ function updateTriggerInput(selectList) { * @param URI */ function createAutoComplete(input, URI) { + console.log('Now in createAutoComplete().') input.typeahead('destroy'); $.getJSON(URI).done(function (data) { + console.log('Input now has auto complete from URI ' + URI); input.typeahead({source: data, autoSelect: false}); + }).fail(function() { + console.log('Could not grab URI ' + URI + ' so autocomplete will not work'); }); } @@ -319,6 +367,7 @@ function testRuleTriggers() { // Serialize all trigger data var triggerData = $(".rule-trigger-tbody").find("input[type=text], input[type=checkbox], select").serializeArray(); + console.log('Found the following trigger data: ' + triggerData); // Find a list of existing transactions that match these triggers $.get('rules/test', triggerData).done(function (data) { diff --git a/resources/views/rules/partials/action.twig b/resources/views/rules/partials/action.twig index 4c13329f86..80b03d1474 100644 --- a/resources/views/rules/partials/action.twig +++ b/resources/views/rules/partials/action.twig @@ -3,12 +3,14 @@ + {# {% if errors.has('rule-action.'~count) %}

{{ errors.first('rule-action.'~count) }}

{% endif %} + #} - {% for key,name in allRuleActions() %} {% endfor %} @@ -16,18 +18,20 @@ - + {# {% if errors.has(('rule-action-value.'~count)) %}

{{ errors.first('rule-action-value.'~count) }}

{% endif %} + #}
diff --git a/resources/views/rules/partials/trigger.twig b/resources/views/rules/partials/trigger.twig index e8f9279426..2db8dfd5a7 100644 --- a/resources/views/rules/partials/trigger.twig +++ b/resources/views/rules/partials/trigger.twig @@ -3,12 +3,14 @@ + {# {% if errors.has('rule-trigger.'~count) %}

{{ errors.first('rule-trigger.'~count) }}

{% endif %} + #} - {% for key,name in allRuleTriggers() %}