diff --git a/app/Api/V1/Requests/TransactionUpdateRequest.php b/app/Api/V1/Requests/TransactionUpdateRequest.php index 937c588bd0..02f11a4894 100644 --- a/app/Api/V1/Requests/TransactionUpdateRequest.php +++ b/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; + } } diff --git a/app/Factory/TransactionJournalFactory.php b/app/Factory/TransactionJournalFactory.php index 797451aa69..988a2e12ef 100644 --- a/app/Factory/TransactionJournalFactory.php +++ b/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 string $field + * @param NullArrayObject $data + * @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 { diff --git a/app/Services/Internal/Update/GroupUpdateService.php b/app/Services/Internal/Update/GroupUpdateService.php index 87b4c8c00a..e71ece0689 100644 --- a/app/Services/Internal/Update/GroupUpdateService.php +++ b/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 { diff --git a/app/Validation/AccountValidator.php b/app/Validation/AccountValidator.php index 88f7f2b5df..1a92f82da9 100644 --- a/app/Validation/AccountValidator.php +++ b/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: diff --git a/app/Validation/TransactionValidation.php b/app/Validation/TransactionValidation.php index c973571cff..ec7714363f 100644 --- a/app/Validation/TransactionValidation.php +++ b/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); - $sources = []; - $dests = []; - foreach ($transactions as $transaction) { - $sources[] = sprintf('%d-%s', $transaction['source_id'] ?? 0, $transaction['source_name'] ?? ''); - $dests[] = sprintf('%d-%s', $transaction['destination_id'] ?? 0, $transaction['destination_name'] ?? ''); + $type = $transactions[0]['type'] ?? strtolower($transactionGroup->transactionJournals()->first()->transactionType->type); + + // compare source ID's, destination ID's, source names and destination names. + // 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. + + $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); - $dests = array_unique($dests); + // TODO not the best way to loop this. switch ($type) { case 'withdrawal': - if (count($sources) > 1) { - $validator->errors()->add('transactions.0.source_id', (string)trans('validation.all_accounts_equal')); + if ($this->arrayEqual($comparison['source_id'])) { + // 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) { - $validator->errors()->add('transactions.0.destination_id', (string)trans('validation.all_accounts_equal')); + 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.destination_id', (string)trans('validation.all_accounts_equal')); break; - case'transfer': - if (count($sources) > 1 || count($dests) > 1) { - $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')); + case 'transfer': + if ($this->arrayEqual($comparison['source_id'])) { + // source ID's are equal, return void. + 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)); + } } diff --git a/resources/lang/en_US/validation.php b/resources/lang/en_US/validation.php index 523734f2b4..bcecfe7bf2 100644 --- a/resources/lang/en_US/validation.php +++ b/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).', ];