kenmirrors/firefly-iii
1
0
Fork 0
mirror of https://github.com/firefly-iii/firefly-iii.git synced 2025-10-15 08:35:00 +00:00
Code Issues Projects Releases Wiki Activity

Refactor rule creation.

Browse Source
This commit is contained in:
James Cole
2018-08-05 15:34:20 +02:00
parent 07a8c69ba8
commit 422e80530b
12 changed files with 337 additions and 337 deletions
Download Patch File Download Diff File Expand all files Collapse all files

51
app/Http/Controllers/Rule/CreateController.php
View File

@@ -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')
);
}

20
app/Http/Controllers/Rule/SelectController.php
View File

@@ -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'),
];
}
}

100
app/Http/Requests/RuleFormRequest.php
View File

@@ -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;
}
}

40
app/Repositories/Rule/RuleRepository.php
View File

@@ -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);

86
app/Support/Http/Controllers/RuleManagement.php
View File

@@ -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;

2
app/TransactionRules/Factory/TriggerFactory.php
View File

@@ -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;
}

4
app/TransactionRules/Processor.php
View File

@@ -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);
}

193
app/Validation/FireflyValidator.php
View File

@@ -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);
}
/**