Split group update now works.

app/Api/V1/Requests/TransactionUpdateRequest.php

@@ -74,6 +74,7 @@ class TransactionUpdateRequest extends Request
             'order',
             'currency_id',
             'foreign_currency_id',
+            'transaction_journal_id',
             'source_id',
             'destination_id',
             'budget_id',
@@ -136,60 +137,6 @@ class TransactionUpdateRequest extends Request
         return $data;
     }
 
-    /**
-     * Get transaction data.
-     *
-     * @SuppressWarnings(PHPMD.CyclomaticComplexity)
-     * @SuppressWarnings(PHPMD.NPathComplexity)
-     * @return array
-     */
-    private function getTransactionData(): array
-    {
-        $return = [];
-        /**
-         * @var int $index
-         * @var array $transaction
-         */
-        foreach ($this->get('transactions') as $index => $transaction) {
-            // default response is to update nothing in the transaction:
-            $current = [];
-
-            // for each field, add it to the array if a reference is present in the request:
-            foreach ($this->integerFields as $fieldName) {
-                if (array_key_exists($fieldName, $transaction)) {
-                    $current[$fieldName] = $this->integerFromValue((string)$transaction[$fieldName]);
-                }
-            }
-
-            foreach ($this->stringFields as $fieldName) {
-                if (array_key_exists($fieldName, $transaction)) {
-                    $current[$fieldName] = $this->stringFromValue((string)$transaction[$fieldName]);
-                }
-            }
-
-            foreach ($this->dateFields as $fieldName) {
-                if (array_key_exists($fieldName, $transaction)) {
-                    $current[$fieldName] = $this->dateFromValue((string)$transaction[$fieldName]);
-                }
-            }
-
-            foreach ($this->booleanFields as $fieldName) {
-                if (array_key_exists($fieldName, $transaction)) {
-                    $current[$fieldName] = $this->convertBoolean((string)$transaction[$fieldName]);
-                }
-            }
-
-            foreach ($this->arrayFields as $fieldName) {
-                if (array_key_exists($fieldName, $transaction)) {
-                    $current[$fieldName] = $this->arrayFromValue($transaction[$fieldName]);
-                }
-            }
-            $return[] = $current;
-        }
-
-        return $return;
-    }
-
     /**
      * The rules that the incoming request must be matched against.
      *
@@ -298,6 +245,9 @@ class TransactionUpdateRequest extends Request
                 // TODO validate that the currency fits the source and/or destination account.
                 // TODO the currency info must match the accounts involved.
 
+                // TODO if the transaction_journal_id is empty, some fields are mandatory.
+                // TODO like the amount!
+
                 // all journals must have a description
                 //$this->validateDescriptions($validator);
 
@@ -315,4 +265,58 @@ class TransactionUpdateRequest extends Request
             }
         );
     }
+
+    /**
+     * Get transaction data.
+     *
+     * @SuppressWarnings(PHPMD.CyclomaticComplexity)
+     * @SuppressWarnings(PHPMD.NPathComplexity)
+     * @return array
+     */
+    private function getTransactionData(): array
+    {
+        $return = [];
+        /**
+         * @var int $index
+         * @var array $transaction
+         */
+        foreach ($this->get('transactions') as $index => $transaction) {
+            // default response is to update nothing in the transaction:
+            $current = [];
+
+            // for each field, add it to the array if a reference is present in the request:
+            foreach ($this->integerFields as $fieldName) {
+                if (array_key_exists($fieldName, $transaction)) {
+                    $current[$fieldName] = $this->integerFromValue((string)$transaction[$fieldName]);
+                }
+            }
+
+            foreach ($this->stringFields as $fieldName) {
+                if (array_key_exists($fieldName, $transaction)) {
+                    $current[$fieldName] = $this->stringFromValue((string)$transaction[$fieldName]);
+                }
+            }
+
+            foreach ($this->dateFields as $fieldName) {
+                if (array_key_exists($fieldName, $transaction)) {
+                    $current[$fieldName] = $this->dateFromValue((string)$transaction[$fieldName]);
+                }
+            }
+
+            foreach ($this->booleanFields as $fieldName) {
+                if (array_key_exists($fieldName, $transaction)) {
+                    $current[$fieldName] = $this->convertBoolean((string)$transaction[$fieldName]);
+                }
+            }
+
+            foreach ($this->arrayFields as $fieldName) {
+                if (array_key_exists($fieldName, $transaction)) {
+                    $current[$fieldName] = $this->arrayFromValue($transaction[$fieldName]);
+                }
+            }
+            $return[] = $current;
+        }
+
+        return $return;
+    }
 }

app/Factory/TransactionJournalFactory.php

@@ -27,7 +27,6 @@ namespace FireflyIII\Factory;
 use Carbon\Carbon;
 use Exception;
 use FireflyIII\Exceptions\FireflyException;
-use FireflyIII\Models\TransactionCurrency;
 use FireflyIII\Models\TransactionJournal;
 use FireflyIII\Models\TransactionType;
 use FireflyIII\Repositories\Bill\BillRepositoryInterface;
@@ -129,6 +128,7 @@ class TransactionJournalFactory
         foreach ($transactions as $index => $row) {
             Log::debug(sprintf('Now creating journal %d/%d', $index + 1, count($transactions)));
 
+            Log::debug('Going to call createJournal', $row);
             $journal = $this->createJournal(new NullArrayObject($row));
             if (null !== $journal) {
                 $collection->push($journal);
@@ -159,7 +159,7 @@ class TransactionJournalFactory
      * Link a piggy bank to this journal.
      *
      * @param TransactionJournal $journal
-     * @param NullArrayObject    $data
+     * @param NullArrayObject $data
      */
     public function storePiggyEvent(TransactionJournal $journal, NullArrayObject $data): void
     {
@@ -183,8 +183,8 @@ class TransactionJournalFactory
 
     /**
      * @param TransactionJournal $journal
-     * @param NullArrayObject    $data
+     * @param NullArrayObject $data
-     * @param string             $field
+     * @param string $field
      */
     protected function storeMeta(TransactionJournal $journal, NullArrayObject $data, string $field): void
     {
@@ -305,7 +305,7 @@ class TransactionJournalFactory
 
     /**
      * @param TransactionJournal $journal
-     * @param NullArrayObject    $transaction
+     * @param NullArrayObject $transaction
      */
     private function storeMetaFields(TransactionJournal $journal, NullArrayObject $transaction): void
     {

app/Services/Internal/Update/GroupUpdateService.php

@@ -23,6 +23,8 @@ declare(strict_types=1);
 
 namespace FireflyIII\Services\Internal\Update;
 
+use FireflyIII\Exceptions\FireflyException;
+use FireflyIII\Factory\TransactionJournalFactory;
 use FireflyIII\Models\TransactionGroup;
 use FireflyIII\Models\TransactionJournal;
 use Log;
@@ -36,13 +38,14 @@ class GroupUpdateService
      * Update a transaction group.
      *
      * @param TransactionGroup $transactionGroup
-     * @param array            $data
+     * @param array $data
      *
      * @return TransactionGroup
+     * @throws FireflyException
      */
     public function update(TransactionGroup $transactionGroup, array $data): TransactionGroup
     {
-        Log::debug('Now in group update service');
+        Log::debug('Now in group update service', $data);
         $transactions = $data['transactions'] ?? [];
         // update group name.
         if (array_key_exists('group_title', $data)) {
@@ -57,19 +60,84 @@ class GroupUpdateService
             $this->updateTransactionJournal($transactionGroup, $first, reset($transactions));
             $transactionGroup->refresh();
             app('preferences')->mark();
+
             return $transactionGroup;
         }
-        die('cannot update split yet.');
+
+        Log::debug('Going to update split group.');
+
+        /**
+         * @var int $index
+         * @var array $transaction
+         */
+        foreach ($transactions as $index => $transaction) {
+            Log::debug(sprintf('Now at #%d of %d', ($index + 1), count($transactions)), $transaction);
+            $journalId = (int)($transaction['transaction_journal_id'] ?? 0);
+            /** @var TransactionJournal $journal */
+            $journal = $transactionGroup->transactionJournals()->find($journalId);
+            if (null === $journal) {
+                Log::debug('This entry has no existing journal: make a new split.');
+                // force the transaction type on the transaction data.
+                // by plucking it from another journal in the group:
+                if (!isset($transaction['type'])) {
+                    Log::debug('No transaction type is indicated.');
+                    /** @var TransactionJournal $randomJournal */
+                    $randomJournal = $transactionGroup->transactionJournals()->inRandomOrder()->with(['transactionType'])->first();
+                    if (null !== $randomJournal) {
+                        $transaction['type'] = $randomJournal->transactionType->type;
+                        Log::debug(sprintf('Transaction type set to %s.', $transaction['type']));
+                    }
+                }
+                Log::debug('Call createTransactionJournal');
+                $this->createTransactionJournal($transactionGroup, $transaction);
+                Log::debug('Done calling createTransactionJournal');
+            }
+            if (null !== $journal) {
+                Log::debug('Call updateTransactionJournal');
+                $this->updateTransactionJournal($transactionGroup, $journal, $transaction);
+                Log::debug('Done calling updateTransactionJournal');
+            }
+        }
 
         app('preferences')->mark();
+
+        return $transactionGroup;
+    }
+
+    /**
+     * @param TransactionGroup $transactionGroup
+     * @param array $data
+     * @throws FireflyException
+     */
+    private function createTransactionJournal(TransactionGroup $transactionGroup, array $data): void
+    {
+
+        $submission = [
+            'transactions' => [
+                $data,
+            ],
+        ];
+        /** @var TransactionJournalFactory $factory */
+        $factory = app(TransactionJournalFactory::class);
+        $factory->setUser($transactionGroup->user);
+        try {
+            $collection = $factory->create($submission);
+        } catch (FireflyException $e) {
+            Log::error($e->getMessage());
+            Log::error($e->getTraceAsString());
+            throw new FireflyException(sprintf('Could not create new transaction journal: %s', $e->getMessage()));
+        }
+        $collection->each(function (TransactionJournal $journal) use ($transactionGroup) {
+            $transactionGroup->transactionJournals()->save($journal);
+        });
     }
 
     /**
      * Update single journal.
      *
-     * @param TransactionGroup   $transactionGroup
+     * @param TransactionGroup $transactionGroup
      * @param TransactionJournal $journal
-     * @param array              $data
+     * @param array $data
      */
     private function updateTransactionJournal(TransactionGroup $transactionGroup, TransactionJournal $journal, array $data): void
     {

app/Validation/AccountValidator.php

@@ -141,7 +141,7 @@ class AccountValidator
         switch ($this->transactionType) {
             default:
                 $result            = false;
-                $this->sourceError = sprintf('Cannot handle type "%s"', $this->transactionType);
+                $this->sourceError = sprintf('Cannot handle type "%s" :(', $this->transactionType);
                 Log::error(sprintf('AccountValidator::validateSource cannot handle "%s", so it will always return false.', $this->transactionType));
                 break;
             case TransactionType::WITHDRAWAL:

app/Validation/TransactionValidation.php

@@ -23,6 +23,7 @@ declare(strict_types=1);
 
 namespace FireflyIII\Validation;
 
+use FireflyIII\Models\Transaction;
 use FireflyIII\Models\TransactionGroup;
 use Illuminate\Validation\Validator;
 
@@ -242,7 +243,7 @@ trait TransactionValidation
     }
 
     /**
-     * @param Validator        $validator
+     * @param Validator $validator
      * @param TransactionGroup $transactionGroup
      */
     private function validateEqualAccountsForUpdate(Validator $validator, TransactionGroup $transactionGroup): void
@@ -253,37 +254,110 @@ trait TransactionValidation
         if (count($transactions) < 2) {
             return;
         }
-        $type    = $transactions[0]['type'] ?? strtolower($transactionGroup->transactionJournals()->first()->transactionType->type);
+        $type = $transactions[0]['type'] ?? strtolower($transactionGroup->transactionJournals()->first()->transactionType->type);
-        $sources = [];
+
-        $dests   = [];
+        // compare source ID's, destination ID's, source names and destination names.
-        foreach ($transactions as $transaction) {
+        // I think I can get away with one combination being equal, as long as the rest
-            $sources[] = sprintf('%d-%s', $transaction['source_id'] ?? 0, $transaction['source_name'] ?? '');
+        // of the code picks up on this as well.
-            $dests[]   = sprintf('%d-%s', $transaction['destination_id'] ?? 0, $transaction['destination_name'] ?? '');
+        // either way all fields must be blank or all equal
+        // but if ID's are equal don't bother with the names.
+
+        $fields     = ['source_id', 'destination_id', 'source_name', 'destination_name'];
+        $comparison = [];
+        foreach ($fields as $field) {
+            $comparison[$field] = [];
+            /** @var array $transaction */
+            foreach ($transactions as $transaction) {
+                // source or destination may be omitted. If this is the case, use the original source / destination name + ID.
+                $originalData = $this->getOriginalData((int)($transaction['transaction_journal_id'] ?? 0));
+
+                // get field.
+                $comparison[$field][] = $transaction[$field] ?? $originalData[$field];
+            }
         }
-        $sources = array_unique($sources);
+        // TODO not the best way to loop this.
-        $dests   = array_unique($dests);
         switch ($type) {
             case 'withdrawal':
-                if (count($sources) > 1) {
+                if ($this->arrayEqual($comparison['source_id'])) {
-                    $validator->errors()->add('transactions.0.source_id', (string)trans('validation.all_accounts_equal'));
+                    // source ID's are equal, return void.
+                    return;
                 }
+                if ($this->arrayEqual($comparison['source_name'])) {
+                    // source names are equal, return void.
+                    return;
+                }
+                // add error:
+                $validator->errors()->add('transactions.0.source_id', (string)trans('validation.all_accounts_equal'));
                 break;
             case 'deposit':
-                if (count($dests) > 1) {
+                if ($this->arrayEqual($comparison['destination_id'])) {
-                    $validator->errors()->add('transactions.0.destination_id', (string)trans('validation.all_accounts_equal'));
+                    // destination ID's are equal, return void.
+                    return;
                 }
+                if ($this->arrayEqual($comparison['destination_name'])) {
+                    // destination names are equal, return void.
+                    return;
+                }
+                // add error:
+                $validator->errors()->add('transactions.0.destination_id', (string)trans('validation.all_accounts_equal'));
                 break;
-            case'transfer':
+            case 'transfer':
-                if (count($sources) > 1 || count($dests) > 1) {
+                if ($this->arrayEqual($comparison['source_id'])) {
-                    $validator->errors()->add('transactions.0.source_id', (string)trans('validation.all_accounts_equal'));
+                    // source ID's are equal, return void.
-                    $validator->errors()->add('transactions.0.destination_id', (string)trans('validation.all_accounts_equal'));
+                    return;
                 }
+                if ($this->arrayEqual($comparison['source_name'])) {
+                    // source names are equal, return void.
+                    return;
+                }
+                if ($this->arrayEqual($comparison['destination_id'])) {
+                    // destination ID's are equal, return void.
+                    return;
+                }
+                if ($this->arrayEqual($comparison['destination_name'])) {
+                    // destination names are equal, return void.
+                    return;
+                }
+                // add error:
+                $validator->errors()->add('transactions.0.source_id', (string)trans('validation.all_accounts_equal'));
+                $validator->errors()->add('transactions.0.destination_id', (string)trans('validation.all_accounts_equal'));
                 break;
         }
     }
 
     /**
-     * @param Validator        $validator
+     * @param int $journalId
+     * @return array
+     */
+    private function getOriginalData(int $journalId): array
+    {
+        $return = [
+            'source_id'        => 0,
+            'source_name'      => '',
+            'destination_id'   => 0,
+            'destination_name' => '',
+        ];
+        if (0 === $journalId) {
+            return $return;
+        }
+        /** @var Transaction $source */
+        $source = Transaction::where('transaction_journal_id', $journalId)->where('amount', '<', 0)->with(['account'])->first();
+        if (null !== $source) {
+            $return['source_id']   = $source->account_id;
+            $return['source_name'] = $source->account->name;
+        }
+        /** @var Transaction $destination */
+        $destination = Transaction::where('transaction_journal_id', $journalId)->where('amount', '>', 0)->with(['account'])->first();
+        if (null !== $source) {
+            $return['destination_id']   = $destination->account_id;
+            $return['destination_name'] = $destination->account->name;
+        }
+
+        return $return;
+    }
+
+    /**
+     * @param Validator $validator
      * @param TransactionGroup $transactionGroup
      */
     private function validateJournalIds(Validator $validator, TransactionGroup $transactionGroup): void
@@ -302,4 +376,13 @@ trait TransactionValidation
             }
         }
     }
+
+    /**
+     * @param array $array
+     * @return bool
+     */
+    private function arrayEqual(array $array): bool
+    {
+        return 1 === count(array_unique($array));
+    }
 }

resources/lang/en_US/validation.php

@@ -183,5 +183,5 @@ return [
     'transfer_source_bad_data'  => 'Could not find a valid source account when searching for ID ":id" or name ":name".',
     'transfer_dest_need_data'   => 'Need to get a valid destination account ID and/or valid destination account name to continue.',
     'transfer_dest_bad_data'    => 'Could not find a valid destination account when searching for ID ":id" or name ":name".',
-    'need_id_in_edit'           => 'When updating a transaction with splits, each split must have a valid transaction journal id (field "transaction_journal_id").',
+    'need_id_in_edit'           => 'Each split must have transaction_journal_id (either valid ID or 0).',
 ];