Catch various audit log things

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

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

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;

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

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) {

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.',

resources/views/list/ale.twig

@@ -71,9 +71,9 @@
                 {% endif %}
 
                 {% if 'update_description' == logEntry.action %}
-                    <code>{{ logEntry.before }}</code>
+                    <code><s>{{ logEntry.before }}</s></code>
                     &rarr;
-                    <code><s>{{ logEntry.after }}</s></code>
+                    <code>{{ logEntry.after }}</code>
                 {% 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 }}