diff --git a/app/Handlers/Events/AuditEventHandler.php b/app/Handlers/Events/AuditEventHandler.php
index e0249e12e7..601a18df6e 100644
--- a/app/Handlers/Events/AuditEventHandler.php
+++ b/app/Handlers/Events/AuditEventHandler.php
@@ -22,7 +22,6 @@
namespace FireflyIII\Handlers\Events;
use FireflyIII\Events\TriggeredAuditLog;
-use FireflyIII\Models\AuditLogEntry;
use FireflyIII\Repositories\AuditLogEntry\ALERepositoryInterface;
class AuditEventHandler
@@ -36,10 +35,10 @@ class AuditEventHandler
{
$array = [
'auditable' => $event->auditable,
- 'changer' => $event->changer,
- 'action' => $event->field,
- 'before' => $event->before,
- 'after' => $event->after,
+ 'changer' => $event->changer,
+ 'action' => $event->field,
+ 'before' => $event->before,
+ 'after' => $event->after,
];
/** @var ALERepositoryInterface $repository */
$repository = app(ALERepositoryInterface::class);
diff --git a/app/Repositories/AuditLogEntry/ALERepository.php b/app/Repositories/AuditLogEntry/ALERepository.php
index 7b39d0b342..8b57a88712 100644
--- a/app/Repositories/AuditLogEntry/ALERepository.php
+++ b/app/Repositories/AuditLogEntry/ALERepository.php
@@ -39,7 +39,7 @@ class ALERepository implements ALERepositoryInterface
$auditLogEntry->auditable()->associate($data['auditable']);
$auditLogEntry->changer()->associate($data['changer']);
- $auditLogEntry->action = $data['field'];
+ $auditLogEntry->action = $data['action'];
$auditLogEntry->before = $data['before'];
$auditLogEntry->after = $data['after'];
$auditLogEntry->save();
diff --git a/app/TransactionRules/Actions/SetDescription.php b/app/TransactionRules/Actions/SetDescription.php
index 37506e7f8f..ee18e71c97 100644
--- a/app/TransactionRules/Actions/SetDescription.php
+++ b/app/TransactionRules/Actions/SetDescription.php
@@ -66,7 +66,7 @@ class SetDescription implements ActionInterface
$this->action->action_value
)
);
-
+ $journal->refresh();
event(new TriggeredAuditLog($this->action->rule, $journal, 'update_description', $before, $this->action->action_value));
return true;
diff --git a/app/Validation/Account/ReconciliationValidation.php b/app/Validation/Account/ReconciliationValidation.php
index c0e3ddae9a..68b59ac53d 100644
--- a/app/Validation/Account/ReconciliationValidation.php
+++ b/app/Validation/Account/ReconciliationValidation.php
@@ -43,76 +43,67 @@ trait ReconciliationValidation
*/
protected function validateReconciliationDestination(array $array): bool
{
- $accountId = array_key_exists('id', $array) ? $array['id'] : null;
- Log::debug('Now in validateReconciliationDestination', $array);
- if (null === $accountId) {
- Log::debug('Return FALSE');
-
- return false;
- }
- $result = $this->accountRepository->find($accountId);
- if (null === $result) {
- $this->destError = (string) trans('validation.deposit_dest_bad_data', ['id' => $accountId, 'name' => '']);
- Log::debug('Return FALSE');
-
- return false;
- }
- // types depends on type of source:
- $types = [AccountType::ASSET, AccountType::LOAN, AccountType::DEBT, AccountType::MORTGAGE];
- // if source is reconciliation, destination can't be.
- if (null !== $this->source && AccountType::RECONCILIATION === $this->source->accountType->type) {
- $types = [AccountType::ASSET, AccountType::LOAN, AccountType::DEBT, AccountType::MORTGAGE];
- }
- // if source is not reconciliation, destination MUST be.
- if (null !== $this->source
- && in_array(
- $this->source->accountType->type, [AccountType::ASSET, AccountType::LOAN, AccountType::DEBT, AccountType::MORTGAGE], true
- )) {
- $types = [AccountType::RECONCILIATION];
- }
-
- if (in_array($result->accountType->type, $types, true)) {
- $this->destination = $result;
- Log::debug('Return TRUE');
+ $accountId = array_key_exists('id', $array) ? $array['id'] : null;
+ $accountName = array_key_exists('name', $array) ? $array['name'] : null;
+ // if both are NULL, the destination is valid because the reconciliation
+ // is expected to be "negative", i.e. the money flows towards the
+ // destination to the asset account which is the source.
+ if (null === $accountId && null === $accountName) {
return true;
}
- $this->destError = (string) trans('validation.deposit_dest_wrong_type');
- Log::debug('Return FALSE');
- return false;
+ // after that, search for it expecting an asset account or a liability.
+ Log::debug('Now in validateReconciliationDestination', $array);
+
+ // source can be any of the following types.
+ $validTypes = array_keys($this->combinations[$this->transactionType]);
+ $search = $this->findExistingAccount($validTypes, $array);
+ if (null === $search) {
+ $this->sourceError = (string) trans('validation.reconciliation_source_bad_data', ['id' => $accountId, 'name' => $accountName]);
+ Log::warning('Not a valid source. Cant find it.', $validTypes);
+
+ return false;
+ }
+ $this->source = $search;
+ Log::debug('Valid source account!');
+
+ return true;
}
/**
+ * Basically the same check
* @param array $array
*
* @return bool
*/
protected function validateReconciliationSource(array $array): bool
{
- $accountId = array_key_exists('id', $array) ? $array['id'] : null;
- Log::debug('In validateReconciliationSource', $array);
- if (null === $accountId) {
- Log::debug('Return FALSE');
-
- return false;
- }
- $result = $this->accountRepository->find($accountId);
- $types = [AccountType::ASSET, AccountType::LOAN, AccountType::DEBT, AccountType::MORTGAGE, AccountType::RECONCILIATION];
- if (null === $result) {
- Log::debug('Return FALSE');
-
- return false;
- }
- if (in_array($result->accountType->type, $types, true)) {
- $this->source = $result;
- Log::debug('Return TRUE');
-
+ $accountId = array_key_exists('id', $array) ? $array['id'] : null;
+ $accountName = array_key_exists('name', $array) ? $array['name'] : null;
+ // if both are NULL, the source is valid because the reconciliation
+ // is expected to be "positive", i.e. the money flows from the
+ // source to the asset account that is the destination.
+ if (null === $accountId && null === $accountName) {
return true;
}
- Log::debug('Return FALSE');
- return false;
+ // after that, search for it expecting an asset account or a liability.
+ Log::debug('Now in validateReconciliationSource', $array);
+
+ // source can be any of the following types.
+ $validTypes = array_keys($this->combinations[$this->transactionType]);
+ $search = $this->findExistingAccount($validTypes, $array);
+ if (null === $search) {
+ $this->sourceError = (string) trans('validation.reconciliation_source_bad_data', ['id' => $accountId, 'name' => $accountName]);
+ Log::warning('Not a valid source. Cant find it.', $validTypes);
+
+ return false;
+ }
+ $this->source = $search;
+ Log::debug('Valid source account!');
+
+ return true;
}
}
diff --git a/app/Validation/TransactionValidation.php b/app/Validation/TransactionValidation.php
index 25c7c303bd..e13afab2d7 100644
--- a/app/Validation/TransactionValidation.php
+++ b/app/Validation/TransactionValidation.php
@@ -27,6 +27,7 @@ use FireflyIII\Models\Account;
use FireflyIII\Models\Transaction;
use FireflyIII\Models\TransactionGroup;
use FireflyIII\Models\TransactionJournal;
+use FireflyIII\Models\TransactionType;
use Illuminate\Validation\Validator;
use Log;
@@ -104,13 +105,13 @@ trait TransactionValidation
$sourceName = array_key_exists('source_name', $transaction) ? (string) $transaction['source_name'] : null;
$sourceIban = array_key_exists('source_iban', $transaction) ? (string) $transaction['source_iban'] : null;
$sourceNumber = array_key_exists('source_number', $transaction) ? (string) $transaction['source_number'] : null;
- $array = [
+ $source = [
'id' => $sourceId,
'name' => $sourceName,
'iban' => $sourceIban,
'number' => $sourceNumber,
];
- $validSource = $accountValidator->validateSource($array);
+ $validSource = $accountValidator->validateSource($source);
// do something with result:
if (false === $validSource) {
@@ -124,18 +125,53 @@ trait TransactionValidation
$destinationName = array_key_exists('destination_name', $transaction) ? (string) $transaction['destination_name'] : null;
$destinationIban = array_key_exists('destination_iban', $transaction) ? (string) $transaction['destination_iban'] : null;
$destinationNumber = array_key_exists('destination_number', $transaction) ? (string) $transaction['destination_number'] : null;
- $array = [
+ $destination = [
'id' => $destinationId,
'name' => $destinationName,
'iban' => $destinationIban,
'number' => $destinationNumber,
];
- $validDestination = $accountValidator->validateDestination($array);
+ $validDestination = $accountValidator->validateDestination($destination);
// do something with result:
if (false === $validDestination) {
$validator->errors()->add(sprintf('transactions.%d.destination_id', $index), $accountValidator->destError);
$validator->errors()->add(sprintf('transactions.%d.destination_name', $index), $accountValidator->destError);
}
+
+ // sanity check for reconciliation accounts. They can't both be null.
+ $this->sanityCheckReconciliation($validator, $transactionType, $index, $source, $destination);
+ }
+
+ /**
+ * @param Validator $validator
+ * @param string $transactionType
+ * @param int $index
+ * @param array $source
+ * @param array $destination
+ * @return void
+ */
+ protected function sanityCheckReconciliation(Validator $validator, string $transactionType, int $index, array $source, array $destination): void
+ {
+ Log::debug('sanityCheckReconciliation');
+ if (TransactionType::RECONCILIATION === ucfirst($transactionType) &&
+ null === $source['id'] && null === $source['name'] && null === $destination['id'] && null === $destination['name']
+ ) {
+ Log::debug('Both are NULL, error!');
+ $validator->errors()->add(sprintf('transactions.%d.source_id', $index), trans('validation.reconciliation_either_account'));
+ $validator->errors()->add(sprintf('transactions.%d.source_name', $index), trans('validation.reconciliation_either_account'));
+ $validator->errors()->add(sprintf('transactions.%d.destination_id', $index), trans('validation.reconciliation_either_account'));
+ $validator->errors()->add(sprintf('transactions.%d.destination_name', $index), trans('validation.reconciliation_either_account'));
+ }
+
+ if (TransactionType::RECONCILIATION === $transactionType &&
+ (null !== $source['id'] || null !== $source['name']) &&
+ (null !== $destination['id'] || null !== $destination['name'])) {
+ Log::debug('Both are not NULL, error!');
+ $validator->errors()->add(sprintf('transactions.%d.source_id', $index), trans('validation.reconciliation_either_account'));
+ $validator->errors()->add(sprintf('transactions.%d.source_name', $index), trans('validation.reconciliation_either_account'));
+ $validator->errors()->add(sprintf('transactions.%d.destination_id', $index), trans('validation.reconciliation_either_account'));
+ $validator->errors()->add(sprintf('transactions.%d.destination_name', $index), trans('validation.reconciliation_either_account'));
+ }
}
/**
@@ -448,7 +484,7 @@ trait TransactionValidation
// I think I can get away with one combination being equal, as long as the rest
// of the code picks up on this as well.
// either way all fields must be blank or all equal
- // but if ID's are equal don't bother with the names.
+ // but if IDs are equal don't bother with the names.
$comparison = $this->collectComparisonData($transactions);
$result = $this->compareAccountData($type, $comparison);
if (false === $result) {
diff --git a/resources/lang/en_US/validation.php b/resources/lang/en_US/validation.php
index 0f0eb75744..191940fedc 100644
--- a/resources/lang/en_US/validation.php
+++ b/resources/lang/en_US/validation.php
@@ -208,10 +208,11 @@ return [
'transfer_dest_bad_data' => 'Could not find a valid destination account when searching for ID ":id" or name ":name".',
'need_id_in_edit' => 'Each split must have transaction_journal_id (either valid ID or 0).',
- 'ob_source_need_data' => 'Need to get a valid source account ID and/or valid source account name to continue.',
- 'lc_source_need_data' => 'Need to get a valid source account ID to continue.',
- 'ob_dest_need_data' => 'Need to get a valid destination account ID and/or valid destination account name to continue.',
- 'ob_dest_bad_data' => 'Could not find a valid destination account when searching for ID ":id" or name ":name".',
+ 'ob_source_need_data' => 'Need to get a valid source account ID and/or valid source account name to continue.',
+ 'lc_source_need_data' => 'Need to get a valid source account ID to continue.',
+ 'ob_dest_need_data' => 'Need to get a valid destination account ID and/or valid destination account name to continue.',
+ 'ob_dest_bad_data' => 'Could not find a valid destination account when searching for ID ":id" or name ":name".',
+ 'reconciliation_either_account' => 'To submit a reconciliation, you must submit either a source or a destination account. Not both, not neither.',
'generic_invalid_source' => 'You can\'t use this account as the source account.',
'generic_invalid_destination' => 'You can\'t use this account as the destination account.',
diff --git a/resources/views/list/ale.twig b/resources/views/list/ale.twig
index 6145385cee..e490295275 100644
--- a/resources/views/list/ale.twig
+++ b/resources/views/list/ale.twig
@@ -71,9 +71,9 @@
{% endif %}
{% if 'update_description' == logEntry.action %}
- {{ logEntry.before }}
+ {{ logEntry.before }}
→
- {{ logEntry.after }}
+ {{ logEntry.after }}
{% endif %}
{% if 'add_to_piggy' == logEntry.action %}
{{ trans('firefly.ale_action_log_add', {amount: formatAmountBySymbol(logEntry.after.amount, logEntry.after.currency_symbol, logEntry.after.decimal_places, true), name: logEntry.after.name})|raw }}