Better validation, can now also use notes in expression.

This commit is contained in:
James Cole 2024-03-10 08:08:26 +01:00
parent 5718d1690a
commit 0b45c1aa76
No known key found for this signature in database
GPG Key ID: B49A324B7EAD6D80
12 changed files with 171 additions and 83 deletions

View File

@ -26,13 +26,16 @@ namespace FireflyIII\TransactionRules\Actions;
use FireflyIII\Events\TriggeredAuditLog;
use FireflyIII\Models\RuleAction;
use FireflyIII\Models\TransactionJournal;
use FireflyIII\TransactionRules\Traits\RefreshNotesTrait;
/**
* Class AppendDescription.
* TODO Can be replaced (and migrated) to action "set description" with a prefilled expression
*/
class AppendDescription implements ActionInterface
{
private RuleAction $action;
use RefreshNotesTrait;
/**
* TriggerInterface constructor.
@ -44,6 +47,7 @@ class AppendDescription implements ActionInterface
public function actOnArray(array $journal): bool
{
$this->refreshNotes($journal);
$append = $this->action->getValue($journal);
$description = sprintf('%s %s', $journal['description'], $append);
\DB::table('transaction_journals')->where('id', $journal['transaction_journal_id'])->limit(1)->update(['description' => $description]);

View File

@ -29,12 +29,15 @@ use FireflyIII\Events\TriggeredAuditLog;
use FireflyIII\Models\Note;
use FireflyIII\Models\RuleAction;
use FireflyIII\Models\TransactionJournal;
use FireflyIII\TransactionRules\Traits\RefreshNotesTrait;
/**
* Class AppendDescriptionToNotes
* TODO Can be replaced (and migrated) to action "set notes" with a prefilled expression
*/
class AppendDescriptionToNotes implements ActionInterface
{
use RefreshNotesTrait;
private RuleAction $action;
/**
@ -47,6 +50,7 @@ class AppendDescriptionToNotes implements ActionInterface
public function actOnArray(array $journal): bool
{
$this->refreshNotes($journal);
/** @var null|TransactionJournal $object */
$object = TransactionJournal::where('user_id', $journal['user_id'])->find($journal['transaction_journal_id']);
if (null === $object) {

View File

@ -30,6 +30,7 @@ use FireflyIII\Models\TransactionJournal;
/**
* Class AppendNotes.
* TODO Can be replaced (and migrated) to action "set notes" with a prefilled expression
*/
class AppendNotes implements ActionInterface
{

View File

@ -33,6 +33,7 @@ use FireflyIII\Support\Request\ConvertsDataTypes;
/**
* Class AppendNotesToDescription
* TODO Can be replaced (and migrated) to action "set description" with a prefilled expression
*/
class AppendNotesToDescription implements ActionInterface
{

View File

@ -32,6 +32,7 @@ use FireflyIII\Models\TransactionJournal;
/**
* Class MoveDescriptionToNotes
* TODO Can be replaced (and migrated) to action "set notes" with a prefilled expression
*/
class MoveDescriptionToNotes implements ActionInterface
{

View File

@ -36,6 +36,7 @@ use FireflyIII\Support\Request\ConvertsDataTypes;
/**
* Class MoveNotesToDescription
* TODO Can be replaced (and migrated) to action "set notes" with a prefilled expression
*/
class MoveNotesToDescription implements ActionInterface
{

View File

@ -29,6 +29,7 @@ use FireflyIII\Models\TransactionJournal;
/**
* Class PrependDescription.
* TODO Can be replaced (and migrated) to action "set description" with a prefilled expression
*/
class PrependDescription implements ActionInterface
{

View File

@ -30,6 +30,7 @@ use FireflyIII\Models\TransactionJournal;
/**
* Class PrependNotes.
* TODO Can be replaced (and migrated) to action "set notes" with a prefilled expression
*/
class PrependNotes implements ActionInterface
{

View File

@ -26,15 +26,18 @@ namespace FireflyIII\TransactionRules\Engine;
use Carbon\Carbon;
use FireflyIII\Exceptions\FireflyException;
use FireflyIII\Models\Note;
use FireflyIII\Models\Rule;
use FireflyIII\Models\RuleAction;
use FireflyIII\Models\RuleGroup;
use FireflyIII\Models\RuleTrigger;
use FireflyIII\Models\TransactionJournal;
use FireflyIII\Repositories\Journal\JournalRepositoryInterface;
use FireflyIII\Support\Search\SearchInterface;
use FireflyIII\TransactionRules\Factory\ActionFactory;
use FireflyIII\User;
use Illuminate\Support\Collection;
use Illuminate\Support\Facades\Log;
/**
* Class SearchRuleEngine
@ -50,10 +53,10 @@ class SearchRuleEngine implements RuleEngineInterface
public function __construct()
{
$this->rules = new Collection();
$this->groups = new Collection();
$this->operators = [];
$this->resultCount = [];
$this->rules = new Collection();
$this->groups = new Collection();
$this->operators = [];
$this->resultCount = [];
// always collect the triggers from the database, unless indicated otherwise.
$this->refreshTriggers = true;
@ -70,7 +73,7 @@ class SearchRuleEngine implements RuleEngineInterface
app('log')->debug('SearchRuleEngine::find()');
$collection = new Collection();
foreach ($this->rules as $rule) {
$found = new Collection();
$found = new Collection();
if (true === $rule->strict) {
$found = $this->findStrictRule($rule);
}
@ -79,8 +82,9 @@ class SearchRuleEngine implements RuleEngineInterface
}
$collection = $collection->merge($found);
}
return $collection->unique();
$result = $collection->unique();
app('log')->debug(sprintf('SearchRuleEngine::find() returns %d unique transactions.', $result->count()));
return $result;
}
/**
@ -89,8 +93,8 @@ class SearchRuleEngine implements RuleEngineInterface
private function findStrictRule(Rule $rule): Collection
{
app('log')->debug(sprintf('Now in findStrictRule(#%d)', $rule->id ?? 0));
$searchArray = [];
$triggers = [];
$searchArray = [];
$triggers = [];
if ($this->refreshTriggers) {
$triggers = $rule->ruleTriggers()->orderBy('order', 'ASC')->get();
}
@ -121,7 +125,7 @@ class SearchRuleEngine implements RuleEngineInterface
app('log')->debug(sprintf('SearchRuleEngine:: add local added operator: %s:"%s"', $operator['type'], $operator['value']));
$searchArray[$operator['type']][] = sprintf('"%s"', $operator['value']);
}
$date = today(config('app.timezone'));
$date = today(config('app.timezone'));
if ($this->hasSpecificJournalTrigger($searchArray)) {
$date = $this->setDateFromJournalTrigger($searchArray);
}
@ -141,7 +145,7 @@ class SearchRuleEngine implements RuleEngineInterface
}
}
$result = $searchEngine->searchTransactions();
$result = $searchEngine->searchTransactions();
return $result->getCollection();
}
@ -166,7 +170,7 @@ class SearchRuleEngine implements RuleEngineInterface
$dateTrigger = true;
}
}
$result = $journalTrigger && $dateTrigger;
$result = $journalTrigger && $dateTrigger;
app('log')->debug(sprintf('Result of hasSpecificJournalTrigger is %s.', var_export($result, true)));
return $result;
@ -185,7 +189,7 @@ class SearchRuleEngine implements RuleEngineInterface
if (0 !== $journalId) {
$repository = app(JournalRepositoryInterface::class);
$repository->setUser($this->user);
$journal = $repository->find($journalId);
$journal = $repository->find($journalId);
if (null !== $journal) {
$date = $journal->date;
app('log')->debug(sprintf('Found journal #%d with date %s.', $journal->id, $journal->date->format('Y-m-d')));
@ -261,10 +265,10 @@ class SearchRuleEngine implements RuleEngineInterface
$searchEngine->parseQuery(sprintf('%s:%s', $type, $value));
}
$result = $searchEngine->searchTransactions();
$collection = $result->getCollection();
$result = $searchEngine->searchTransactions();
$collection = $result->getCollection();
app('log')->debug(sprintf('Found in this run, %d transactions', $collection->count()));
$total = $total->merge($collection);
$total = $total->merge($collection);
app('log')->debug(sprintf('Total collection is now %d transactions', $total->count()));
++$count;
// if trigger says stop processing, do so.
@ -278,7 +282,7 @@ class SearchRuleEngine implements RuleEngineInterface
app('log')->debug(sprintf('Done running %d trigger(s)', $count));
// make collection unique
$unique = $total->unique(
$unique = $total->unique(
static function (array $group) {
$str = '';
foreach ($group['transactions'] as $transaction) {
@ -369,7 +373,7 @@ class SearchRuleEngine implements RuleEngineInterface
$this->processResults($rule, $collection);
app('log')->debug(sprintf('SearchRuleEngine:: done processing strict rule #%d', $rule->id));
$result = $collection->count() > 0;
$result = $collection->count() > 0;
if (true === $result) {
app('log')->debug(sprintf('SearchRuleEngine:: rule #%d was triggered (on %d transaction(s)).', $rule->id, $collection->count()));
@ -432,6 +436,7 @@ class SearchRuleEngine implements RuleEngineInterface
private function processRuleAction(RuleAction $ruleAction, array $transaction): bool
{
app('log')->debug(sprintf('Executing rule action "%s" with value "%s"', $ruleAction->action_type, $ruleAction->action_value));
$transaction = $this->addNotes($transaction);
$actionClass = ActionFactory::getAction($ruleAction);
$result = $actionClass->actOnArray($transaction);
$journalId = $transaction['transaction_journal_id'] ?? 0;
@ -532,4 +537,15 @@ class SearchRuleEngine implements RuleEngineInterface
}
}
}
private function addNotes(array $transaction): array
{
$transaction['notes'] = '';
$dbNote = Note::where('noteable_id', (int)$transaction['transaction_journal_id'])->where('noteable_type', TransactionJournal::class)->first(['notes.*']);
if (null !== $dbNote) {
$transaction['notes'] = $dbNote->text;
}
Log::debug(sprintf('Notes of journal #%d filled in.', $transaction['transaction_journal_id']));
return $transaction;
}
}

View File

@ -79,6 +79,7 @@ class ActionExpression
'due_date',
'process_date',
'destination_transaction_id',
'notes',
];
private ExpressionLanguage $expressionLanguage;
@ -97,7 +98,7 @@ class ActionExpression
private static function isExpression(string $expr): bool
{
return str_starts_with($expr, '=');
return str_starts_with($expr, '=') && strlen($expr) > 1;
}
private function validate(): ?SyntaxError

View File

@ -0,0 +1,48 @@
<?php
/*
* RefreshNotesTrait.php
* Copyright (c) 2024 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 https://www.gnu.org/licenses/.
*/
declare(strict_types=1);
namespace FireflyIII\TransactionRules\Traits;
use FireflyIII\Models\Note;
use FireflyIII\Models\TransactionJournal;
use Illuminate\Support\Facades\Log;
trait RefreshNotesTrait
{
/**
* @param array $transaction
*
* @return array
*/
final protected function refreshNotes(array $transaction): array
{
$transaction['notes'] = '';
$dbNote = Note::where('noteable_id', (int)$transaction['transaction_journal_id'])->where('noteable_type', TransactionJournal::class)->first(['notes.*']);
if (null !== $dbNote) {
$transaction['notes'] = $dbNote->text;
}
Log::debug(sprintf('Notes of journal #%d refreshed.', $transaction['transaction_journal_id']));
return $transaction;
}
}

View File

@ -62,7 +62,7 @@ class FireflyValidator extends Validator
if (!is_string($value) || 6 !== strlen($value)) {
return false;
}
$user = auth()->user();
$user = auth()->user();
if (null === $user) {
app('log')->error('No user during validate2faCode');
@ -183,10 +183,10 @@ class FireflyValidator extends Validator
$replace = ['', '10', '11', '12', '13', '14', '15', '16', '17', '18', '19', '20', '21', '22', '23', '24', '25', '26', '27', '28', '29', '30', '31', '32', '33', '34', '35'];
// take
$first = substr($value, 0, 4);
$last = substr($value, 4);
$iban = $last . $first;
$iban = trim(str_replace($search, $replace, $iban));
$first = substr($value, 0, 4);
$last = substr($value, 4);
$iban = $last.$first;
$iban = trim(str_replace($search, $replace, $iban));
if ('' === $iban) {
return false;
}
@ -257,8 +257,8 @@ class FireflyValidator extends Validator
{
// first, get the index from this string:
$value ??= '';
$parts = explode('.', $attribute);
$index = (int)($parts[1] ?? '0');
$parts = explode('.', $attribute);
$index = (int)($parts[1] ?? '0');
// get the name of the trigger from the data array:
$actionType = $this->data['actions'][$index]['type'] ?? 'invalid';
@ -327,8 +327,8 @@ class FireflyValidator extends Validator
public function validateRuleTriggerValue(string $attribute, string $value = null): bool
{
// first, get the index from this string:
$parts = explode('.', $attribute);
$index = (int)($parts[1] ?? '0');
$parts = explode('.', $attribute);
$index = (int)($parts[1] ?? '0');
// get the name of the trigger from the data array:
$triggerType = $this->data['triggers'][$index]['type'] ?? 'invalid';
@ -339,14 +339,14 @@ class FireflyValidator extends Validator
}
// these trigger types need a numerical check:
$numerical = ['amount_less', 'amount_more', 'amount_exactly'];
$numerical = ['amount_less', 'amount_more', 'amount_exactly'];
if (in_array($triggerType, $numerical, true)) {
return is_numeric($value);
}
// these triggers need just the word "true":
// TODO create a helper to automatically return these.
$needTrue = [
$needTrue = [
'reconciled', 'has_attachments', 'has_any_category', 'has_any_budget', 'has_any_bill', 'has_any_tag', 'any_notes', 'any_external_url', 'has_no_attachments', 'has_no_category', 'has_no_budget', 'has_no_bill', 'has_no_tag', 'no_notes', 'no_external_url',
'source_is_cash',
'destination_is_cash',
@ -361,7 +361,7 @@ class FireflyValidator extends Validator
// these trigger types need a simple strlen check:
// TODO create a helper to automatically return these.
$length = [
$length = [
'source_account_starts',
'source_account_ends',
'source_account_is',
@ -496,9 +496,9 @@ class FireflyValidator extends Validator
}
/** @var User $user */
$user = User::find($this->data['user_id']);
$type = AccountType::find($this->data['account_type_id'])->first();
$value = $this->data['name'];
$user = User::find($this->data['user_id']);
$type = AccountType::find($this->data['account_type_id'])->first();
$value = $this->data['name'];
/** @var null|Account $result */
$result = $user->accounts()->where('account_type_id', $type->id)->where('name', $value)->first();
@ -509,7 +509,7 @@ class FireflyValidator extends Validator
private function validateByAccountTypeString(string $value, array $parameters, string $type): bool
{
/** @var null|array $search */
$search = \Config::get('firefly.accountTypeByIdentifier.' . $type);
$search = \Config::get('firefly.accountTypeByIdentifier.'.$type);
if (null === $search) {
return false;
@ -520,9 +520,10 @@ class FireflyValidator extends Validator
$accountTypeIds = $accountTypes->pluck('id')->toArray();
/** @var null|Account $result */
$result = auth()->user()->accounts()->whereIn('account_type_id', $accountTypeIds)->where('id', '!=', $ignore)
->where('name', $value)
->first();
$result = auth()->user()->accounts()->whereIn('account_type_id', $accountTypeIds)->where('id', '!=', $ignore)
->where('name', $value)
->first()
;
return null === $result;
}
@ -538,8 +539,9 @@ class FireflyValidator extends Validator
/** @var null|Account $result */
$result = auth()->user()->accounts()->where('account_type_id', $type->id)->where('id', '!=', $ignore)
->where('name', $value)
->first();
->where('name', $value)
->first()
;
return null === $result;
}
@ -552,12 +554,13 @@ class FireflyValidator extends Validator
/** @var Account $existingAccount */
$existingAccount = Account::find($accountId);
$type = $existingAccount->accountType;
$ignore = $existingAccount->id;
$type = $existingAccount->accountType;
$ignore = $existingAccount->id;
$entry = auth()->user()->accounts()->where('account_type_id', $type->id)->where('id', '!=', $ignore)
->where('name', $value)
->first();
$entry = auth()->user()->accounts()->where('account_type_id', $type->id)->where('id', '!=', $ignore)
->where('name', $value)
->first()
;
return null === $entry;
}
@ -570,12 +573,13 @@ class FireflyValidator extends Validator
/** @var Account $existingAccount */
$existingAccount = Account::find($this->data['id']);
$type = $existingAccount->accountType;
$ignore = $existingAccount->id;
$type = $existingAccount->accountType;
$ignore = $existingAccount->id;
$entry = auth()->user()->accounts()->where('account_type_id', $type->id)->where('id', '!=', $ignore)
->where('name', $value)
->first();
$entry = auth()->user()->accounts()->where('account_type_id', $type->id)->where('id', '!=', $ignore)
->where('name', $value)
->first()
;
return null === $entry;
}
@ -599,18 +603,19 @@ class FireflyValidator extends Validator
$accountId = (int)($parameters[0] ?? 0.0);
}
$query = AccountMeta::leftJoin('accounts', 'accounts.id', '=', 'account_meta.account_id')
->whereNull('accounts.deleted_at')
->where('accounts.user_id', auth()->user()->id)
->where('account_meta.name', 'account_number')
->where('account_meta.data', json_encode($value));
$query = AccountMeta::leftJoin('accounts', 'accounts.id', '=', 'account_meta.account_id')
->whereNull('accounts.deleted_at')
->where('accounts.user_id', auth()->user()->id)
->where('account_meta.name', 'account_number')
->where('account_meta.data', json_encode($value))
;
if ($accountId > 0) {
// exclude current account from check.
$query->where('account_meta.account_id', '!=', $accountId);
}
$set = $query->get(['account_meta.*']);
$count = $set->count();
$set = $query->get(['account_meta.*']);
$count = $set->count();
if (0 === $count) {
return true;
}
@ -618,7 +623,7 @@ class FireflyValidator extends Validator
// pretty much impossible but still.
return false;
}
$type = $this->data['objectType'] ?? 'unknown';
$type = $this->data['objectType'] ?? 'unknown';
if ('expense' !== $type && 'revenue' !== $type) {
app('log')->warning(sprintf('Account number "%s" is not unique and account type "%s" cannot share its account number.', $value, $type));
@ -688,7 +693,7 @@ class FireflyValidator extends Validator
// get existing webhook value:
if (0 !== $existingId) {
/** @var null|Webhook $webhook */
$webhook = auth()->user()->webhooks()->find($existingId);
$webhook = auth()->user()->webhooks()->find($existingId);
if (null === $webhook) {
return false;
}
@ -706,11 +711,12 @@ class FireflyValidator extends Validator
$userId = auth()->user()->id;
return 0 === Webhook::whereUserId($userId)
->where('trigger', $trigger)
->where('response', $response)
->where('delivery', $delivery)
->where('id', '!=', $existingId)
->where('url', $url)->count();
->where('trigger', $trigger)
->where('response', $response)
->where('delivery', $delivery)
->where('id', '!=', $existingId)
->where('url', $url)->count()
;
}
return false;
@ -732,21 +738,22 @@ class FireflyValidator extends Validator
public function validateUniqueObjectForUser($attribute, $value, $parameters): bool
{
[$table, $field] = $parameters;
$exclude = (int)($parameters[2] ?? 0.0);
$exclude = (int)($parameters[2] ?? 0.0);
/*
* If other data (in $this->getData()) contains
* ID field, set that field to be the $exclude.
*/
$data = $this->getData();
$data = $this->getData();
if (!array_key_exists(2, $parameters) && array_key_exists('id', $data) && (int)$data['id'] > 0) {
$exclude = (int)$data['id'];
}
// get entries from table
$result = \DB::table($table)->where('user_id', auth()->user()->id)->whereNull('deleted_at')
->where('id', '!=', $exclude)
->where($field, $value)
->first([$field]);
$result = \DB::table($table)->where('user_id', auth()->user()->id)->whereNull('deleted_at')
->where('id', '!=', $exclude)
->where($field, $value)
->first([$field])
;
if (null === $result) {
return true; // not found, so true.
}
@ -766,9 +773,10 @@ class FireflyValidator extends Validator
{
$exclude = $parameters[0] ?? null;
$query = \DB::table('object_groups')
->whereNull('object_groups.deleted_at')
->where('object_groups.user_id', auth()->user()->id)
->where('object_groups.title', $value);
->whereNull('object_groups.deleted_at')
->where('object_groups.user_id', auth()->user()->id)
->where('object_groups.title', $value)
;
if (null !== $exclude) {
$query->where('object_groups.id', '!=', (int)$exclude);
}
@ -787,7 +795,8 @@ class FireflyValidator extends Validator
{
$exclude = $parameters[0] ?? null;
$query = \DB::table('piggy_banks')->whereNull('piggy_banks.deleted_at')
->leftJoin('accounts', 'accounts.id', '=', 'piggy_banks.account_id')->where('accounts.user_id', auth()->user()->id);
->leftJoin('accounts', 'accounts.id', '=', 'piggy_banks.account_id')->where('accounts.user_id', auth()->user()->id)
;
if (null !== $exclude) {
$query->where('piggy_banks.id', '!=', (int)$exclude);
}
@ -810,17 +819,17 @@ class FireflyValidator extends Validator
$deliveries = Webhook::getDeliveriesForValidation();
// integers
$trigger = $triggers[$this->data['trigger']] ?? 0;
$response = $responses[$this->data['response']] ?? 0;
$delivery = $deliveries[$this->data['delivery']] ?? 0;
$url = $this->data['url'];
$userId = auth()->user()->id;
$trigger = $triggers[$this->data['trigger']] ?? 0;
$response = $responses[$this->data['response']] ?? 0;
$delivery = $deliveries[$this->data['delivery']] ?? 0;
$url = $this->data['url'];
$userId = auth()->user()->id;
return 0 === Webhook::whereUserId($userId)
->where('trigger', $trigger)
->where('response', $response)
->where('delivery', $delivery)
->where('url', $url)->count();
->where('trigger', $trigger)
->where('response', $response)
->where('delivery', $delivery)
->where('url', $url)->count();
}
return false;