diff --git a/app/Api/V1/Controllers/Models/Transaction/UpdateController.php b/app/Api/V1/Controllers/Models/Transaction/UpdateController.php index 0de76ce4fe..515ef16a72 100644 --- a/app/Api/V1/Controllers/Models/Transaction/UpdateController.php +++ b/app/Api/V1/Controllers/Models/Transaction/UpdateController.php @@ -74,7 +74,7 @@ class UpdateController extends Controller */ public function update(UpdateRequest $request, TransactionGroup $transactionGroup): JsonResponse { - Log::debug('Now in update routine.'); + Log::debug('Now in update routine for transaction group!'); $data = $request->getAll(); $transactionGroup = $this->groupRepository->update($transactionGroup, $data); $manager = $this->getManager(); diff --git a/app/Api/V1/Controllers/Models/TransactionLink/UpdateController.php b/app/Api/V1/Controllers/Models/TransactionLink/UpdateController.php index 85aa3f4ee4..a1c3caca0b 100644 --- a/app/Api/V1/Controllers/Models/TransactionLink/UpdateController.php +++ b/app/Api/V1/Controllers/Models/TransactionLink/UpdateController.php @@ -56,15 +56,9 @@ class UpdateController extends Controller */ public function update(UpdateRequest $request, TransactionJournalLink $journalLink): JsonResponse { - $manager = $this->getManager(); - $data = $request->getAll(); - $data['inward'] = $this->journalRepository->findNull($data['inward_id'] ?? 0); - $data['outward'] = $this->journalRepository->findNull($data['outward_id'] ?? 0); - if (null === $data['inward'] || null === $data['outward']) { - throw new FireflyException('200024: Source or destination does not exist.'); - } - $data['direction'] = 'inward'; - $journalLink = $this->repository->updateLink($journalLink, $data); + $manager = $this->getManager(); + $data = $request->getAll(); + $journalLink = $this->repository->updateLink($journalLink, $data); /** @var TransactionLinkTransformer $transformer */ $transformer = app(TransactionLinkTransformer::class); diff --git a/app/Api/V1/Requests/Models/Account/UpdateRequest.php b/app/Api/V1/Requests/Models/Account/UpdateRequest.php index 6ae06e6169..0a110e8d16 100644 --- a/app/Api/V1/Requests/Models/Account/UpdateRequest.php +++ b/app/Api/V1/Requests/Models/Account/UpdateRequest.php @@ -61,8 +61,6 @@ class UpdateRequest extends FormRequest 'include_net_worth' => $includeNetWorth, 'account_type' => $this->nullableString('type'), 'account_type_id' => null, - 'currency_id' => $this->nullableInteger('currency_id'), - 'currency_code' => $this->nullableString('currency_code'), 'virtual_balance' => $this->nullableString('virtual_balance'), 'iban' => $this->nullableString('iban'), 'BIC' => $this->nullableString('bic'), @@ -80,6 +78,12 @@ class UpdateRequest extends FormRequest if (null !== $this->get('order')) { $data['order'] = $this->integer('order'); } + if (null !== $this->get('currency_id')) { + $data['currency_id'] = $this->nullableInteger('currency_id'); + } + if (null !== $this->get('currency_code')) { + $data['currency_code'] = $this->nullableString('currency_code'); + } $data = $this->appendLocationData($data, null); diff --git a/app/Api/V1/Requests/Models/Transaction/UpdateRequest.php b/app/Api/V1/Requests/Models/Transaction/UpdateRequest.php index a70d13d48f..ac82f4a817 100644 --- a/app/Api/V1/Requests/Models/Transaction/UpdateRequest.php +++ b/app/Api/V1/Requests/Models/Transaction/UpdateRequest.php @@ -50,9 +50,6 @@ class UpdateRequest extends FormRequest private array $stringFields; private array $textareaFields; - - - /** * Get all data. Is pretty complex because of all the ??-statements. * @@ -125,12 +122,13 @@ class UpdateRequest extends FormRequest $this->arrayFields = [ 'tags', ]; - - - $data = [ - 'transactions' => $this->getTransactionData(), - 'apply_rules' => $this->boolean('apply_rules', true), - ]; + $data = []; + if ($this->has('transactions')) { + $data['transactions'] = $this->getTransactionData(); + } + if ($this->has('apply_rules')) { + $data['apply_rules'] = $this->boolean('apply_rules', true); + } if ($this->has('group_title')) { $data['group_title'] = $this->string('group_title'); } @@ -147,19 +145,24 @@ class UpdateRequest extends FormRequest { Log::debug('Now in getTransactionData()'); $return = []; + + if (!is_countable($this->get('transactions'))) { + return $return; + } + /** * @var int $index * @var array $transaction */ foreach ($this->get('transactions') as $transaction) { // default response is to update nothing in the transaction: - $current = []; - $current = $this->getIntegerData($current, $transaction); - $current = $this->getStringData($current, $transaction); - $current = $this->getNlStringData($current, $transaction); - $current = $this->getDateData($current, $transaction); - $current = $this->getBooleanData($current, $transaction); - $current = $this->getArrayData($current, $transaction); + $current = []; + $current = $this->getIntegerData($current, $transaction); + $current = $this->getStringData($current, $transaction); + $current = $this->getNlStringData($current, $transaction); + $current = $this->getDateData($current, $transaction); + $current = $this->getBooleanData($current, $transaction); + $current = $this->getArrayData($current, $transaction); $return[] = $current; } @@ -177,7 +180,7 @@ class UpdateRequest extends FormRequest { foreach ($this->integerFields as $fieldName) { if (array_key_exists($fieldName, $transaction)) { - $current[$fieldName] = $this->integerFromValue((string) $transaction[$fieldName]); + $current[$fieldName] = $this->integerFromValue((string)$transaction[$fieldName]); } } @@ -194,7 +197,7 @@ class UpdateRequest extends FormRequest { foreach ($this->stringFields as $fieldName) { if (array_key_exists($fieldName, $transaction)) { - $current[$fieldName] = $this->stringFromValue((string) $transaction[$fieldName]); + $current[$fieldName] = $this->stringFromValue((string)$transaction[$fieldName]); } } @@ -211,7 +214,7 @@ class UpdateRequest extends FormRequest { foreach ($this->textareaFields as $fieldName) { if (array_key_exists($fieldName, $transaction)) { - $current[$fieldName] = $this->nlStringFromValue((string) $transaction[$fieldName]); + $current[$fieldName] = $this->nlStringFromValue((string)$transaction[$fieldName]); } } @@ -229,8 +232,8 @@ class UpdateRequest extends FormRequest foreach ($this->dateFields as $fieldName) { Log::debug(sprintf('Now at date field %s', $fieldName)); if (array_key_exists($fieldName, $transaction)) { - Log::debug(sprintf('New value: "%s"', (string) $transaction[$fieldName])); - $current[$fieldName] = $this->dateFromValue((string) $transaction[$fieldName]); + Log::debug(sprintf('New value: "%s"', (string)$transaction[$fieldName])); + $current[$fieldName] = $this->dateFromValue((string)$transaction[$fieldName]); } } @@ -247,7 +250,7 @@ class UpdateRequest extends FormRequest { foreach ($this->booleanFields as $fieldName) { if (array_key_exists($fieldName, $transaction)) { - $current[$fieldName] = $this->convertBoolean((string) $transaction[$fieldName]); + $current[$fieldName] = $this->convertBoolean((string)$transaction[$fieldName]); } } @@ -362,21 +365,18 @@ class UpdateRequest extends FormRequest $transactionGroup = $this->route()->parameter('transactionGroup'); $validator->after( function (Validator $validator) use ($transactionGroup) { - // must submit at least one transaction. - $this->validateOneTransaction($validator); - // if more than one, verify that there are journal ID's present. $this->validateJournalIds($validator, $transactionGroup); // all transaction types must be equal: - $this->validateTransactionTypesForUpdate($validator); + $this->validateTransactionTypesForUpdate($validator, $transactionGroup); // validate source/destination is equal, depending on the transaction journal type. $this->validateEqualAccountsForUpdate($validator, $transactionGroup); // validate that the currency fits the source and/or destination account. // validate all account info - $this->validateAccountInformationUpdate($validator); + $this->validateAccountInformationUpdate($validator, $transactionGroup); } ); diff --git a/app/Api/V1/Requests/Models/TransactionCurrency/UpdateRequest.php b/app/Api/V1/Requests/Models/TransactionCurrency/UpdateRequest.php index a0cf19c00f..05fb87c02a 100644 --- a/app/Api/V1/Requests/Models/TransactionCurrency/UpdateRequest.php +++ b/app/Api/V1/Requests/Models/TransactionCurrency/UpdateRequest.php @@ -45,23 +45,22 @@ class UpdateRequest extends FormRequest */ public function getAll(): array { - $enabled = true; - $default = false; - if (null !== $this->get('enabled')) { - $enabled = $this->boolean('enabled'); - } - if (null !== $this->get('default')) { - $default = $this->boolean('default'); - } - - return [ - 'name' => $this->string('name'), - 'code' => $this->string('code'), - 'symbol' => $this->string('symbol'), - 'decimal_places' => $this->integer('decimal_places'), - 'default' => $default, - 'enabled' => $enabled, + // return nothing that isn't explicitely in the array: + // this is the way + $fields = [ + 'name' => ['name', 'string'], + 'code' => ['code', 'string'], + 'symbol' => ['symbol', 'string'], + 'decimal_places' => ['decimal_places', 'integer'], + 'default' => ['default', 'boolean'], + 'enabled' => ['enabled', 'boolean'], ]; + + // this is the way. + $return = $this->getAllData($fields); + + return $return; + } /** @@ -71,7 +70,8 @@ class UpdateRequest extends FormRequest */ public function rules(): array { - $currency = $this->route()->parameter('currency_code'); + $currency = $this->route()->parameter('currency_code'); + return [ 'name' => sprintf('between:1,255|unique:transaction_currencies,name,%d', $currency->id), 'code' => sprintf('between:3,3|unique:transaction_currencies,code,%d', $currency->id), diff --git a/app/Api/V1/Requests/Models/TransactionLink/UpdateRequest.php b/app/Api/V1/Requests/Models/TransactionLink/UpdateRequest.php index 041d498aa6..1c20b128de 100644 --- a/app/Api/V1/Requests/Models/TransactionLink/UpdateRequest.php +++ b/app/Api/V1/Requests/Models/TransactionLink/UpdateRequest.php @@ -23,6 +23,7 @@ declare(strict_types=1); namespace FireflyIII\Api\V1\Requests\Models\TransactionLink; +use FireflyIII\Models\TransactionJournalLink; use FireflyIII\Repositories\Journal\JournalRepositoryInterface; use FireflyIII\Repositories\LinkType\LinkTypeRepositoryInterface; use FireflyIII\Support\Request\ChecksLogin; @@ -50,7 +51,7 @@ class UpdateRequest extends FormRequest 'link_type_name' => $this->string('link_type_name'), 'inward_id' => $this->integer('inward_id'), 'outward_id' => $this->integer('outward_id'), - 'notes' => $this->nlString('notes'), + 'notes' => $this->nullableNlString('notes'), ]; } @@ -81,7 +82,7 @@ class UpdateRequest extends FormRequest { $validator->after( function (Validator $validator) { - $this->validateExistingLink($validator); + $this->validateUpdate($validator); } ); } @@ -89,42 +90,41 @@ class UpdateRequest extends FormRequest /** * @param Validator $validator */ - private function validateExistingLink(Validator $validator): void + private function validateUpdate(Validator $validator): void { - /** @var User $user */ - $user = auth()->user(); + /** @var TransactionJournalLink $existing */ + $existing = $this->route()->parameter('journalLink'); + $data = $validator->getData(); /** @var LinkTypeRepositoryInterface $repository */ $repository = app(LinkTypeRepositoryInterface::class); - $repository->setUser($user); + $repository->setUser(auth()->user()); /** @var JournalRepositoryInterface $journalRepos */ $journalRepos = app(JournalRepositoryInterface::class); - $journalRepos->setUser($user); + $journalRepos->setUser(auth()->user()); - $data = $validator->getData(); - $inwardId = (int) ($data['inward_id'] ?? 0); - $outwardId = (int) ($data['outward_id'] ?? 0); - $inward = $journalRepos->findNull($inwardId); - $outward = $journalRepos->findNull($outwardId); + $inwardId = $data['inward_id'] ?? $existing->source_id; + $outwardId = $data['outward_id'] ?? $existing->destination_id; + $inward = $journalRepos->findNull((int)$inwardId); + $outward = $journalRepos->findNull((int)$outwardId); + if($inward->id === $outward->id) { + $validator->errors()->add('inward_id', 'Inward ID must be different from outward ID.'); + $validator->errors()->add('outward_id', 'Inward ID must be different from outward ID.'); + } if (null === $inward) { - $validator->errors()->add('inward_id', 'Invalid inward ID.'); - + $validator->errors()->add('inward_id', 'This is not a valid inward journal.'); + } + if(null === $outward) { + $validator->errors()->add('inward_id', 'This is not a valid outward journal.'); + } + $inDB =$repository->findSpecificLink($existing->linkType, $inward, $outward); + if(null === $inDB) { return; } - if (null === $outward) { - $validator->errors()->add('outward_id', 'Invalid outward ID.'); - - return; - } - - if ($repository->findLink($inward, $outward)) { - // only if not updating: - $link = $this->route()->parameter('journalLink'); - if (null === $link) { - $validator->errors()->add('outward_id', 'Already have a link between inward and outward.'); - $validator->errors()->add('inward_id', 'Already have a link between inward and outward.'); - } + if($inDB->id !== $existing->id) { + $validator->errors()->add('outward_id', 'Already have a link between inward and outward.'); + $validator->errors()->add('inward_id', 'Already have a link between inward and outward.'); } } } diff --git a/app/Api/V1/Requests/Models/TransactionLinkType/UpdateRequest.php b/app/Api/V1/Requests/Models/TransactionLinkType/UpdateRequest.php index 0efbf2018f..fb50a34a7d 100644 --- a/app/Api/V1/Requests/Models/TransactionLinkType/UpdateRequest.php +++ b/app/Api/V1/Requests/Models/TransactionLinkType/UpdateRequest.php @@ -63,9 +63,9 @@ class UpdateRequest extends FormRequest { $linkType = $this->route()->parameter('linkType'); return [ - 'name' => ['required', Rule::unique('link_types', 'name')->ignore($linkType->id), 'min:1'], - 'outward' => ['required', 'different:inward', Rule::unique('link_types', 'outward')->ignore($linkType->id), 'min:1'], - 'inward' => ['required', 'different:outward', Rule::unique('link_types', 'inward')->ignore($linkType->id), 'min:1'], + 'name' => [Rule::unique('link_types', 'name')->ignore($linkType->id), 'min:1'], + 'outward' => ['different:inward', Rule::unique('link_types', 'outward')->ignore($linkType->id), 'min:1'], + 'inward' => ['different:outward', Rule::unique('link_types', 'inward')->ignore($linkType->id), 'min:1'], ]; } } diff --git a/app/Events/StoredTransactionLink.php b/app/Events/StoredTransactionLink.php index c15a335178..f97f22799b 100644 --- a/app/Events/StoredTransactionLink.php +++ b/app/Events/StoredTransactionLink.php @@ -28,6 +28,7 @@ use Illuminate\Queue\SerializesModels; /** * Class StoredTransactionLink + * TODO not used. */ class StoredTransactionLink extends Event { diff --git a/app/Events/UpdatedTransactionLink.php b/app/Events/UpdatedTransactionLink.php index 58a1f1592b..386ea2041c 100644 --- a/app/Events/UpdatedTransactionLink.php +++ b/app/Events/UpdatedTransactionLink.php @@ -28,6 +28,7 @@ use Illuminate\Queue\SerializesModels; /** * Class UpdatedTransactionLink + * TODO unused */ class UpdatedTransactionLink extends Event { diff --git a/app/Repositories/LinkType/LinkTypeRepository.php b/app/Repositories/LinkType/LinkTypeRepository.php index d1f3276ad8..1a38eee71e 100644 --- a/app/Repositories/LinkType/LinkTypeRepository.php +++ b/app/Repositories/LinkType/LinkTypeRepository.php @@ -24,7 +24,6 @@ namespace FireflyIII\Repositories\LinkType; use Exception; use FireflyIII\Events\DestroyedTransactionLink; -use FireflyIII\Events\RemovedTransactionLink; use FireflyIII\Events\StoredTransactionLink; use FireflyIII\Events\UpdatedTransactionLink; use FireflyIII\Models\LinkType; @@ -41,8 +40,7 @@ use Log; */ class LinkTypeRepository implements LinkTypeRepositoryInterface { - /** @var User */ - private $user; + private User $user; /** * @param LinkType $linkType @@ -177,7 +175,7 @@ class LinkTypeRepository implements LinkTypeRepositoryInterface public function getJournalLinks(LinkType $linkType = null): Collection { $query = TransactionJournalLink - ::with(['source','destination']) + ::with(['source', 'destination']) ->leftJoin('transaction_journals as source_journals', 'journal_links.source_id', '=', 'source_journals.id') ->leftJoin('transaction_journals as dest_journals', 'journal_links.destination_id', '=', 'dest_journals.id') ->where('source_journals.user_id', $this->user->id) @@ -188,7 +186,8 @@ class LinkTypeRepository implements LinkTypeRepositoryInterface if (null !== $linkType) { $query->where('journal_links.link_type_id', $linkType->id); } - return $query->get(['journal_links.*']); + + return $query->get(['journal_links.*']); } /** @@ -310,9 +309,15 @@ class LinkTypeRepository implements LinkTypeRepositoryInterface */ public function update(LinkType $linkType, array $data): LinkType { - $linkType->name = $data['name']; - $linkType->inward = $data['inward']; - $linkType->outward = $data['outward']; + if(array_key_exists('name', $data) && '' !== (string)$data['name']) { + $linkType->name = $data['name']; + } + if(array_key_exists('inward', $data) && '' !== (string)$data['inward']) { + $linkType->inward = $data['inward']; + } + if(array_key_exists('outward', $data) && '' !== (string)$data['outward']) { + $linkType->outward = $data['outward']; + } $linkType->save(); return $linkType; @@ -328,11 +333,25 @@ class LinkTypeRepository implements LinkTypeRepositoryInterface */ public function updateLink(TransactionJournalLink $journalLink, array $data): TransactionJournalLink { - $journalLink->source_id = $data['inward'] ? $data['inward']->id : $journalLink->source_id; - $journalLink->destination_id = $data['outward'] ? $data['outward']->id : $journalLink->destination_id; - $journalLink->link_type_id = $data['link_type_id'] ? $data['link_type_id'] : $journalLink->link_type_id; + $journalLink->source_id = $data['inward_id'] ? $data['inward_id'] : $journalLink->source_id; + $journalLink->destination_id = $data['outward_id'] ? $data['outward_id'] : $journalLink->destination_id; $journalLink->save(); - $this->setNoteText($journalLink, $data['notes']); + if (array_key_exists('link_type_name', $data)) { + $linkType = LinkType::whereName($data['link_type_name'])->first(); + if(null !== $linkType) { + $journalLink->link_type_id = $linkType->id; + $journalLink->save(); + } + $journalLink->refresh(); + } + + $journalLink->link_type_id = $data['link_type_id'] ? $data['link_type_id'] : $journalLink->link_type_id; + + + $journalLink->save(); + if (array_key_exists('notes', $data) && null !== $data['notes']) { + $this->setNoteText($journalLink, $data['notes']); + } event(new UpdatedTransactionLink($journalLink)); @@ -367,4 +386,17 @@ class LinkTypeRepository implements LinkTypeRepositoryInterface } } + + /** + * @inheritDoc + */ + public function getLink(TransactionJournal $one, TransactionJournal $two): ?TransactionJournalLink + { + $left = TransactionJournalLink::whereDestinationId($one->id)->whereSourceId($two->id)->first(); + if (null !== $left) { + return $left; + } + + return TransactionJournalLink::whereDestinationId($two->id)->whereSourceId($one->id)->first(); + } } diff --git a/app/Services/Internal/Support/AccountServiceTrait.php b/app/Services/Internal/Support/AccountServiceTrait.php index db0d4c3ee7..d427c37264 100644 --- a/app/Services/Internal/Support/AccountServiceTrait.php +++ b/app/Services/Internal/Support/AccountServiceTrait.php @@ -23,7 +23,6 @@ declare(strict_types=1); namespace FireflyIII\Services\Internal\Support; -use Carbon\Carbon; use Exception; use FireflyIII\Exceptions\FireflyException; use FireflyIII\Factory\AccountMetaFactory; @@ -234,7 +233,15 @@ trait AccountServiceTrait return null; // @codeCoverageIgnoreEnd } - $amount = app('steam')->positive($amount); + $amount = app('steam')->positive($amount); + if (!array_key_exists('currency_id', $data)) { + $currency = $this->accountRepository->getAccountCurrency($account); + if (null === $currency) { + $currency = app('default')->getDefaultCurrencyByUser($account->user); + } + $data['currency_id'] = $currency->id; + } + $submission = [ 'group_title' => null, 'user' => $account->user_id, @@ -354,6 +361,16 @@ trait AccountServiceTrait if (null === $obGroup) { return $this->createOBGroup($account, $data); } + + // $data['currency_id'] is empty so creating a new journal may break. + if (!array_key_exists('currency_id', $data)) { + $currency = $this->accountRepository->getAccountCurrency($account); + if (null === $currency) { + $currency = app('default')->getDefaultCurrencyByUser($account->user); + } + $data['currency_id'] = $currency->id; + } + /** @var TransactionJournal $journal */ $journal = $obGroup->transactionJournals()->first(); $journal->date = $data['opening_balance_date'] ?? $journal->date; diff --git a/app/Services/Internal/Update/AccountUpdateService.php b/app/Services/Internal/Update/AccountUpdateService.php index 53703e8085..9380edd624 100644 --- a/app/Services/Internal/Update/AccountUpdateService.php +++ b/app/Services/Internal/Update/AccountUpdateService.php @@ -79,9 +79,9 @@ class AccountUpdateService $account = $this->updateAccountOrder($account, $data); // find currency, or use default currency instead. - if (isset($data['currency_id']) && (null !== $data['currency_id'] || null !== $data['currency_code'])) { + if (array_key_exists('currency_id', $data) || array_key_exists('currency_code', $data)) { $currency = $this->getCurrency((int)($data['currency_id'] ?? null), (string)($data['currency_code'] ?? null)); - unset($data['currency_code']); + unset($data['currency_code'], $data['currency_id']); $data['currency_id'] = $currency->id; } diff --git a/app/Services/Internal/Update/CurrencyUpdateService.php b/app/Services/Internal/Update/CurrencyUpdateService.php index d9981e6e98..cb29b3c917 100644 --- a/app/Services/Internal/Update/CurrencyUpdateService.php +++ b/app/Services/Internal/Update/CurrencyUpdateService.php @@ -51,14 +51,26 @@ class CurrencyUpdateService */ public function update(TransactionCurrency $currency, array $data): TransactionCurrency { - $data['code'] = '' === (string)$data['code'] ? $currency->code : $data['code']; - $data['symbol'] = '' === (string)$data['symbol'] ? $currency->code : $data['symbol']; - $data['name'] = '' === (string)$data['name'] ? $currency->code : $data['name']; - $currency->code = $data['code']; - $currency->symbol = $data['symbol']; - $currency->name = $data['name']; - $currency->enabled = $data['enabled']; - $currency->decimal_places = $data['decimal_places']; + if (array_key_exists('code', $data) && '' !== (string)$data['code']) { + $currency->code = $data['code']; + } + + if (array_key_exists('symbol', $data) && '' !== (string)$data['symbol']) { + $currency->symbol = $data['symbol']; + } + + if (array_key_exists('name', $data) && '' !== (string)$data['name']) { + $currency->name = $data['name']; + } + + if (array_key_exists('enabled', $data) && is_bool($data['enabled'])) { + $currency->enabled = $data['enabled']; + } + + if (array_key_exists('decimal_places', $data) && is_int($data['decimal_places'])) { + $currency->decimal_places = $data['decimal_places']; + } + $currency->save(); return $currency; diff --git a/app/Services/Internal/Update/GroupUpdateService.php b/app/Services/Internal/Update/GroupUpdateService.php index 13f0793c10..d6ae83693e 100644 --- a/app/Services/Internal/Update/GroupUpdateService.php +++ b/app/Services/Internal/Update/GroupUpdateService.php @@ -56,6 +56,13 @@ class GroupUpdateService $transactionGroup->title = $data['group_title']; $transactionGroup->save(); } + + if (0 === count($transactions)) { + Log::debug('No transactions submitted, do nothing.'); + + return $transactionGroup; + } + if (1 === count($transactions) && 1 === $transactionGroup->transactionJournals()->count()) { /** @var TransactionJournal $first */ $first = $transactionGroup->transactionJournals()->first(); diff --git a/app/Services/Internal/Update/JournalUpdateService.php b/app/Services/Internal/Update/JournalUpdateService.php index d6e0efee8f..88ffddd045 100644 --- a/app/Services/Internal/Update/JournalUpdateService.php +++ b/app/Services/Internal/Update/JournalUpdateService.php @@ -54,28 +54,17 @@ class JournalUpdateService { use JournalServiceTrait; - /** @var BillRepositoryInterface */ - private $billRepository; - /** @var CurrencyRepositoryInterface */ - private $currencyRepository; - /** @var array The data to update the journal with. */ - private $data; - /** @var Account The destination account. */ - private $destinationAccount; - /** @var Transaction */ - private $destinationTransaction; - /** @var array All meta values that are dates. */ - private $metaDate; - /** @var array All meta values that are strings. */ - private $metaString; - /** @var Account Source account of the journal */ - private $sourceAccount; - /** @var Transaction Source transaction of the journal. */ - private $sourceTransaction; - /** @var TransactionGroup The parent group. */ - private $transactionGroup; - /** @var TransactionJournal The journal to update. */ - private $transactionJournal; + private BillRepositoryInterface $billRepository; + private CurrencyRepositoryInterface $currencyRepository; + private array $data; + private ?Account $destinationAccount; + private ?Transaction $destinationTransaction; + private array $metaDate; + private array $metaString; + private ?Account $sourceAccount; + private ?Transaction $sourceTransaction; + private TransactionGroup $transactionGroup; + private TransactionJournal $transactionJournal; /** * JournalUpdateService constructor. @@ -112,6 +101,10 @@ class JournalUpdateService $this->budgetRepository->setUser($transactionGroup->user); $this->tagFactory->setUser($transactionGroup->user); $this->accountRepository->setUser($transactionGroup->user); + $this->destinationAccount = null; + $this->destinationTransaction = null; + $this->sourceAccount = null; + $this->sourceTransaction = null; } /** @@ -130,7 +123,7 @@ class JournalUpdateService Log::debug(sprintf('Now in JournalUpdateService for journal #%d.', $this->transactionJournal->id)); // can we update account data using the new type? if ($this->hasValidAccounts()) { - Log::info('-- account info is valid, now update.'); + Log::info('Account info is valid, now update.'); // update accounts: $this->updateAccounts(); @@ -202,7 +195,7 @@ class JournalUpdateService private function getOriginalDestinationAccount(): Account { if (null === $this->destinationAccount) { - $destination = $this->getSourceTransaction(); + $destination = $this->getDestinationTransaction(); $this->destinationAccount = $destination->account; } @@ -338,6 +331,7 @@ class JournalUpdateService $destName = $this->data['destination_name'] ?? null; if (!$this->hasFields(['destination_id', 'destination_name'])) { + Log::debug('No destination info submitted, grab the original data.'); $destination = $this->getOriginalDestinationAccount(); $destId = $destination->id; $destName = $destination->name; diff --git a/app/Services/Webhook/StandardWebhookSender.php b/app/Services/Webhook/StandardWebhookSender.php index 6fdb275569..9f3e720050 100644 --- a/app/Services/Webhook/StandardWebhookSender.php +++ b/app/Services/Webhook/StandardWebhookSender.php @@ -118,7 +118,7 @@ class StandardWebhookSender implements WebhookSenderInterface ]; $client = new Client; try { - $res = $client->request('POST', $this->message->webhook->url . 'x', $options); + $res = $client->request('POST', $this->message->webhook->url, $options); $this->message->sent = true; } catch (ClientException | Exception $e) { Log::error($e->getMessage()); diff --git a/app/Transformers/TransactionGroupTransformer.php b/app/Transformers/TransactionGroupTransformer.php index cf75d05893..201f4afba8 100644 --- a/app/Transformers/TransactionGroupTransformer.php +++ b/app/Transformers/TransactionGroupTransformer.php @@ -480,19 +480,19 @@ class TransactionGroupTransformer extends AbstractTransformer } return [ - 'user' => (int)$row['user_id'], + 'user' => (string)$row['user_id'], 'transaction_journal_id' => (int)$row['transaction_journal_id'], 'type' => strtolower($type), 'date' => $row['date']->toAtomString(), 'order' => $row['order'], - 'currency_id' => (int)$row['currency_id'], + 'currency_id' => (string)$row['currency_id'], 'currency_code' => $row['currency_code'], 'currency_name' => $row['currency_name'], 'currency_symbol' => $row['currency_symbol'], 'currency_decimal_places' => (int)$row['currency_decimal_places'], - 'foreign_currency_id' => $this->integerFromArray($transaction, 'foreign_currency_id'), + 'foreign_currency_id' => $this->stringFromArray($transaction, 'foreign_currency_id', null), 'foreign_currency_code' => $row['foreign_currency_code'], 'foreign_currency_symbol' => $row['foreign_currency_symbol'], 'foreign_currency_decimal_places' => $row['foreign_currency_decimal_places'], @@ -502,23 +502,23 @@ class TransactionGroupTransformer extends AbstractTransformer 'description' => $row['description'], - 'source_id' => (int)$row['source_account_id'], + 'source_id' => (string)$row['source_account_id'], 'source_name' => $row['source_account_name'], 'source_iban' => $row['source_account_iban'], 'source_type' => $row['source_account_type'], - 'destination_id' => (int)$row['destination_account_id'], + 'destination_id' => (string)$row['destination_account_id'], 'destination_name' => $row['destination_account_name'], 'destination_iban' => $row['destination_account_iban'], 'destination_type' => $row['destination_account_type'], - 'budget_id' => $this->integerFromArray($transaction, 'budget_id'), + 'budget_id' => $this->stringFromArray($transaction, 'budget_id', null), 'budget_name' => $row['budget_name'], - 'category_id' => $this->integerFromArray($transaction, 'category_id'), + 'category_id' => $this->stringFromArray($transaction, 'category_id', null), 'category_name' => $row['category_name'], - 'bill_id' => $this->integerFromArray($transaction, 'bill_id'), + 'bill_id' => $this->stringFromArray($transaction, 'bill_id', null), 'bill_name' => $row['bill_name'], 'reconciled' => $row['reconciled'], @@ -528,7 +528,7 @@ class TransactionGroupTransformer extends AbstractTransformer 'internal_reference' => $metaFieldData['internal_reference'], 'external_id' => $metaFieldData['external_id'], 'original_source' => $metaFieldData['original_source'], - 'recurrence_id' => $this->integerFromArray($metaFieldData->getArrayCopy(), 'recurrence_id'), + 'recurrence_id' => $this->stringFromArray($metaFieldData->getArrayCopy(), 'recurrence_id', null), 'recurrence_total' => $this->integerFromArray($metaFieldData->getArrayCopy(), 'recurrence_total'), 'recurrence_count' => $this->integerFromArray($metaFieldData->getArrayCopy(), 'recurrence_count'), 'bunq_payment_id' => $metaFieldData['bunq_payment_id'], @@ -568,10 +568,10 @@ class TransactionGroupTransformer extends AbstractTransformer private function stringFromArray(array $array, string $key, ?string $default): ?string { if (array_key_exists($key, $array)) { - return $array[$key]; + return (string) $array[$key]; } if (null !== $default) { - return $default; + return (string) $default; } return null; diff --git a/app/Validation/Account/AccountValidatorProperties.php b/app/Validation/Account/AccountValidatorProperties.php index 1a1001746f..172fea5212 100644 --- a/app/Validation/Account/AccountValidatorProperties.php +++ b/app/Validation/Account/AccountValidatorProperties.php @@ -35,13 +35,5 @@ use FireflyIII\User; */ trait AccountValidatorProperties { - public bool $createMode; - public string $destError; - public Account $destination; - public Account $source; - public string $sourceError; - private AccountRepositoryInterface $accountRepository; - private array $combinations; - private string $transactionType; - private User $user; + } diff --git a/app/Validation/Account/WithdrawalValidation.php b/app/Validation/Account/WithdrawalValidation.php index 07c2e09f5b..6b41fb9c51 100644 --- a/app/Validation/Account/WithdrawalValidation.php +++ b/app/Validation/Account/WithdrawalValidation.php @@ -24,6 +24,7 @@ declare(strict_types=1); namespace FireflyIII\Validation\Account; use FireflyIII\Models\Account; +use FireflyIII\Models\AccountType; use Log; /** @@ -117,4 +118,38 @@ trait WithdrawalValidation return true; } + + /** + * @param int|null $accountId + * @param string|null $accountName + * + * @return bool + */ + protected function validateGenericSource(?int $accountId, ?string $accountName): bool + { + Log::debug(sprintf('Now in validateGenericSource(%d, "%s")', $accountId, $accountName)); + // source can be any of the following types. + $validTypes = [AccountType::ASSET, AccountType::REVENUE, AccountType::LOAN, AccountType::DEBT, AccountType::MORTGAGE]; + if (null === $accountId && null === $accountName && false === $this->canCreateTypes($validTypes)) { + // if both values are NULL we return TRUE + // because we assume the user doesnt want to submit / change anything. + $this->sourceError = (string) trans('validation.withdrawal_source_need_data'); + Log::warning('Not a valid source. Need more data.'); + + return false; + } + + // otherwise try to find the account: + $search = $this->findExistingAccount($validTypes, (int) $accountId, (string) $accountName); + if (null === $search) { + $this->sourceError = (string) trans('validation.withdrawal_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/AccountValidator.php b/app/Validation/AccountValidator.php index 4af7069303..7cbce0a91a 100644 --- a/app/Validation/AccountValidator.php +++ b/app/Validation/AccountValidator.php @@ -43,6 +43,16 @@ class AccountValidator { use AccountValidatorProperties, WithdrawalValidation, DepositValidation, TransferValidation, ReconciliationValidation, OBValidation; + public bool $createMode; + public string $destError; + public ?Account $destination; + public ?Account $source; + public string $sourceError; + private AccountRepositoryInterface $accountRepository; + private array $combinations; + private string $transactionType; + private User $user; + /** * AccountValidator constructor. */ @@ -52,6 +62,8 @@ class AccountValidator $this->destError = 'No error yet.'; $this->sourceError = 'No error yet.'; $this->combinations = config('firefly.source_dests'); + $this->source = null; + $this->destination = null; /** @var AccountRepositoryInterface accountRepository */ $this->accountRepository = app(AccountRepositoryInterface::class); @@ -66,7 +78,7 @@ class AccountValidator */ public function setTransactionType(string $transactionType): void { - Log::debug(sprintf('Transaction type for validator is now %s', ucfirst($transactionType))); + Log::debug(sprintf('Transaction type for validator is now "%s".', ucfirst($transactionType))); $this->transactionType = ucfirst($transactionType); } @@ -135,9 +147,8 @@ class AccountValidator Log::debug(sprintf('Now in AccountValidator::validateSource(%d, "%s", "%s")', $accountId, $accountName, $accountIban)); switch ($this->transactionType) { default: - $result = false; - $this->sourceError = trans('validation.invalid_account_info'); - Log::error(sprintf('AccountValidator::validateSource cannot handle "%s", so it will always return false.', $this->transactionType)); + Log::error(sprintf('AccountValidator::validateSource cannot handle "%s", so it will do a generic check.', $this->transactionType)); + $result = $this->validateGenericSource($accountId, $accountName); break; case TransactionType::WITHDRAWAL: $result = $this->validateWithdrawalSource($accountId, $accountName); @@ -197,8 +208,8 @@ class AccountValidator } /** - * @param array $validTypes - * @param int $accountId + * @param array $validTypes + * @param int $accountId * @param string $accountName * * @return Account|null @@ -221,5 +232,12 @@ class AccountValidator return null; } + /** + * @return Account|null + */ + public function getSource(): ?Account + { + return $this->source; + } } diff --git a/app/Validation/GroupValidation.php b/app/Validation/GroupValidation.php index 5cbc109b99..0b6013bd40 100644 --- a/app/Validation/GroupValidation.php +++ b/app/Validation/GroupValidation.php @@ -57,6 +57,7 @@ trait GroupValidation if (count($transactions) < 2) { // no need for validation. + Log::debug(sprintf('%d transaction(s) in submission, can skip this check.', count($transactions))); return; } // check each array: @@ -104,12 +105,15 @@ trait GroupValidation * * @SuppressWarnings(PHPMD.CyclomaticComplexity) */ - private function validateJournalid(Validator $validator, int $index, array $transaction, TransactionGroup $transactionGroup): void + private function validateJournalId(Validator $validator, int $index, array $transaction, TransactionGroup $transactionGroup): void { $journalId = $transaction['transaction_journal_id'] ?? null; + Log::debug(sprintf('Now in validateJournalId(%d, %d)', $index, $journalId)); + $journalId = null === $journalId ? null : (int) $journalId; $count = $transactionGroup->transactionJournals()->where('id', $journalId)->count(); if (null === $journalId || (null !== $journalId && 0 !== $journalId && 0 === $count)) { + Log::warning('Invalid submission: Each split must have transaction_journal_id (either valid ID or 0).'); $validator->errors()->add(sprintf('transactions.%d.source_name', $index), (string) trans('validation.need_id_in_edit')); } } diff --git a/app/Validation/TransactionValidation.php b/app/Validation/TransactionValidation.php index 6b7d3e2394..28010f45f6 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\Account; use FireflyIII\Models\Transaction; use FireflyIII\Models\TransactionGroup; use FireflyIII\Models\TransactionJournal; @@ -53,7 +54,7 @@ trait TransactionValidation * @var array $transaction */ foreach ($transactions as $index => $transaction) { - if(!is_int($index)) { + if (!is_int($index)) { continue; } $this->validateSingleAccount($validator, $index, $transactionType, $transaction); @@ -75,9 +76,9 @@ trait TransactionValidation $accountValidator->setTransactionType($transactionType); // validate source account. - $sourceId = isset($transaction['source_id']) ? (int) $transaction['source_id'] : null; - $sourceName = isset($transaction['source_name']) ? (string) $transaction['source_name'] : null; - $sourceIban = isset($transaction['source_iban']) ? (string) $transaction['source_iban'] : null; + $sourceId = isset($transaction['source_id']) ? (int)$transaction['source_id'] : null; + $sourceName = isset($transaction['source_name']) ? (string)$transaction['source_name'] : null; + $sourceIban = isset($transaction['source_iban']) ? (string)$transaction['source_iban'] : null; $validSource = $accountValidator->validateSource($sourceId, $sourceName, $sourceIban); // do something with result: @@ -88,9 +89,9 @@ trait TransactionValidation return; } // validate destination account - $destinationId = isset($transaction['destination_id']) ? (int) $transaction['destination_id'] : null; - $destinationName = isset($transaction['destination_name']) ? (string) $transaction['destination_name'] : null; - $destinationIban = isset($transaction['destination_iban']) ? (string) $transaction['destination_iban'] : null; + $destinationId = isset($transaction['destination_id']) ? (int)$transaction['destination_id'] : null; + $destinationName = isset($transaction['destination_name']) ? (string)$transaction['destination_name'] : null; + $destinationIban = isset($transaction['destination_iban']) ? (string)$transaction['destination_iban'] : null; $validDestination = $accountValidator->validateDestination($destinationId, $destinationName, $destinationIban); // do something with result: if (false === $validDestination) { @@ -102,9 +103,10 @@ trait TransactionValidation /** * Validates the given account information. Switches on given transaction type. * - * @param Validator $validator + * @param Validator $validator + * @param TransactionGroup $transactionGroup */ - public function validateAccountInformationUpdate(Validator $validator): void + public function validateAccountInformationUpdate(Validator $validator, TransactionGroup $transactionGroup): void { Log::debug('Now in validateAccountInformationUpdate()'); $transactions = $this->getTransactionsArray($validator); @@ -114,53 +116,81 @@ trait TransactionValidation * @var array $transaction */ foreach ($transactions as $index => $transaction) { - $this->validateSingleUpdate($validator, $index, $transaction); + $this->validateSingleUpdate($validator, $index, $transaction, $transactionGroup); } } /** - * @param Validator $validator - * @param int $index - * @param array $transaction + * @param Validator $validator + * @param int $index + * @param array $transaction + * @param TransactionGroup $transactionGroup */ - protected function validateSingleUpdate(Validator $validator, int $index, array $transaction): void + protected function validateSingleUpdate(Validator $validator, int $index, array $transaction, TransactionGroup $transactionGroup): void { - /** @var AccountValidator $accountValidator */ - $accountValidator = app(AccountValidator::class); - $originalType = $this->getOriginalType((int) ($transaction['transaction_journal_id'] ?? 0)); - $originalData = $this->getOriginalData((int) ($transaction['transaction_journal_id'] ?? 0)); - $transactionType = $transaction['type'] ?? $originalType; - $accountValidator->setTransactionType($transactionType); + Log::debug('Now validating single account update in validateSingleUpdate()'); // if no account types are given, just skip the check. if (!isset($transaction['source_id']) && !isset($transaction['source_name']) && !isset($transaction['destination_id']) && !isset($transaction['destination_name'])) { - return; - } - - // validate source account. - $sourceId = isset($transaction['source_id']) ? (int) $transaction['source_id'] : $originalData['source_id']; - $sourceName = $transaction['source_name'] ?? $originalData['source_name']; - $validSource = $accountValidator->validateSource($sourceId, $sourceName, null); - - // do something with result: - if (false === $validSource) { - $validator->errors()->add(sprintf('transactions.%d.source_id', $index), $accountValidator->sourceError); - $validator->errors()->add(sprintf('transactions.%d.source_name', $index), $accountValidator->sourceError); + Log::debug('No account data has been submitted so will not validating account info.'); return; } - // validate destination account - $destinationId = isset($transaction['destination_id']) ? (int) $transaction['destination_id'] : $originalData['destination_id']; - $destinationName = $transaction['destination_name'] ?? $originalData['destination_name']; - $validDestination = $accountValidator->validateDestination($destinationId, $destinationName, null); - // 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); + // create validator: + /** @var AccountValidator $accountValidator */ + $accountValidator = app(AccountValidator::class); + + // get the transaction type using the original transaction group: + $accountValidator->setTransactionType($this->getTransactionType($transactionGroup, [])); + + // validate if the submitted source and / or name are valid + if (array_key_exists('source_id', $transaction) || array_key_exists('source_name', $transaction)) { + Log::debug('Will try to validate source account information.'); + $sourceId = (int)($transaction['source_id'] ?? 0); + $sourceName = $transaction['source_name'] ?? null; + $validSource = $accountValidator->validateSource($sourceId, $sourceName, null); + + // do something with result: + if (false === $validSource) { + Log::warning('Looks like the source account is not valid so complain to the user about it.'); + $validator->errors()->add(sprintf('transactions.%d.source_id', $index), $accountValidator->sourceError); + $validator->errors()->add(sprintf('transactions.%d.source_name', $index), $accountValidator->sourceError); + + return; + } + Log::debug('Source account info is valid.'); } + + if (array_key_exists('destination_id', $transaction) || array_key_exists('destination_name', $transaction)) { + Log::debug('Will try to validate destination account information.'); + // at this point the validator may not have a source account, because it was never submitted for validation. + // must add it ourselves or the validator can never check if the destination is correct. + // the $transaction array must have a journal id or it's just one, this was validated before. + if (null === $accountValidator->source) { + Log::debug('Account validator has no source account, must find it.'); + $source = $this->getOriginalSource($transaction, $transactionGroup); + if (null !== $source) { + Log::debug('Found a source!'); + $accountValidator->source = $source; + } + } + + + $destinationId = (int)($transaction['destination_id'] ?? 0); + $destinationName = $transaction['destination_name'] ?? null; + $validDestination = $accountValidator->validateDestination($destinationId, $destinationName, null); + // do something with result: + if (false === $validDestination) { + Log::warning('Looks like the destination account is not valid so complain to the user about it.'); + $validator->errors()->add(sprintf('transactions.%d.destination_id', $index), $accountValidator->destError); + $validator->errors()->add(sprintf('transactions.%d.destination_name', $index), $accountValidator->destError); + } + Log::debug('Destination account info is valid.'); + } + Log::debug('Done with validateSingleUpdate().'); } /** @@ -175,19 +205,21 @@ trait TransactionValidation // need at least one transaction if (0 === count($transactions)) { - $validator->errors()->add('transactions', (string) trans('validation.at_least_one_transaction')); + $validator->errors()->add('transactions', (string)trans('validation.at_least_one_transaction')); } } /** * @param Validator $validator */ - public function validateTransactionArray(Validator $validator): void { + public function validateTransactionArray(Validator $validator): void + { $transactions = $this->getTransactionsArray($validator); - foreach($transactions as $key => $value) { - if(!is_int($key)) { - $validator->errors()->add('transactions.0.description', (string) trans('validation.at_least_one_transaction')); + foreach ($transactions as $key => $value) { + if (!is_int($key)) { + $validator->errors()->add('transactions.0.description', (string)trans('validation.at_least_one_transaction')); Log::debug('Added error: at_least_one_transaction.'); + return; } } @@ -204,8 +236,9 @@ trait TransactionValidation $transactions = $this->getTransactionsArray($validator); // need at least one transaction if (0 === count($transactions)) { - $validator->errors()->add('transactions.0.description', (string) trans('validation.at_least_one_transaction')); + $validator->errors()->add('transactions.0.description', (string)trans('validation.at_least_one_transaction')); Log::debug('Added error: at_least_one_transaction.'); + return; } Log::debug('Added NO errors.'); @@ -227,35 +260,40 @@ trait TransactionValidation } $unique = array_unique($types); if (count($unique) > 1) { - $validator->errors()->add('transactions.0.type', (string) trans('validation.transaction_types_equal')); + $validator->errors()->add('transactions.0.type', (string)trans('validation.transaction_types_equal')); return; } $first = $unique[0] ?? 'invalid'; if ('invalid' === $first) { - $validator->errors()->add('transactions.0.type', (string) trans('validation.invalid_transaction_type')); + $validator->errors()->add('transactions.0.type', (string)trans('validation.invalid_transaction_type')); } } /** * All types of splits must be equal. * - * @param Validator $validator + * @param Validator $validator + * @param TransactionGroup $transactionGroup */ - public function validateTransactionTypesForUpdate(Validator $validator): void + public function validateTransactionTypesForUpdate(Validator $validator, TransactionGroup $transactionGroup): void { Log::debug('Now in validateTransactionTypesForUpdate()'); $transactions = $this->getTransactionsArray($validator); $types = []; foreach ($transactions as $transaction) { - $originalType = $this->getOriginalType((int) ($transaction['transaction_journal_id'] ?? 0)); + $originalType = $this->getOriginalType((int)($transaction['transaction_journal_id'] ?? 0)); // if type is not set, fall back to the type of the journal, if one is given. $types[] = $transaction['type'] ?? $originalType; } $unique = array_unique($types); if (count($unique) > 1) { - $validator->errors()->add('transactions.0.type', (string) trans('validation.transaction_types_equal')); + Log::warning('Add error for mismatch transaction types.'); + $validator->errors()->add('transactions.0.type', (string)trans('validation.transaction_types_equal')); + + return; } + Log::debug('No errors in validateTransactionTypesForUpdate()'); } /** @@ -368,18 +406,18 @@ trait TransactionValidation default: case 'withdrawal': if (count($sources) > 1) { - $validator->errors()->add('transactions.0.source_id', (string) trans('validation.all_accounts_equal')); + $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')); + $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')); + $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; } @@ -388,6 +426,7 @@ trait TransactionValidation /** * @param TransactionGroup $group * @param array $transactions + * * @return string */ private function getTransactionType(TransactionGroup $group, array $transactions): string @@ -397,6 +436,7 @@ trait TransactionValidation /** * @param array $transactions + * * @return array */ private function collectComparisonData(array $transactions): array @@ -408,18 +448,20 @@ trait TransactionValidation /** @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)); + $originalData = $this->getOriginalData((int)($transaction['transaction_journal_id'] ?? 0)); // get field. $comparison[$field][] = $transaction[$field] ?? $originalData[$field]; } } + return $comparison; } /** * @param string $type * @param array $comparison + * * @return bool */ private function compareAccountData(string $type, array $comparison): bool @@ -437,6 +479,7 @@ trait TransactionValidation /** * @param array $comparison + * * @return bool */ private function compareAccountDataTransfer(array $comparison): bool @@ -457,11 +500,13 @@ trait TransactionValidation // destination names are equal, return void. return true; } + return false; } /** * @param array $comparison + * * @return bool */ private function compareAccountDataWithdrawal(array $comparison): bool @@ -474,11 +519,13 @@ trait TransactionValidation // source names are equal, return void. return true; } + return false; } /** * @param array $comparison + * * @return bool */ private function compareAccountDataDeposit(array $comparison): bool @@ -491,6 +538,7 @@ trait TransactionValidation // destination names are equal, return void. return true; } + return false; } @@ -504,6 +552,8 @@ trait TransactionValidation $transactions = $this->getTransactionsArray($validator); if (2 !== count($transactions)) { + Log::debug('Less than 2 transactions, do nothing.'); + return; } $type = $this->getTransactionType($transactionGroup, $transactions); @@ -517,15 +567,42 @@ trait TransactionValidation $result = $this->compareAccountData($type, $comparison); if (false === $result) { if ('withdrawal' === $type) { - $validator->errors()->add('transactions.0.source_id', (string) trans('validation.all_accounts_equal')); + $validator->errors()->add('transactions.0.source_id', (string)trans('validation.all_accounts_equal')); } if ('deposit' === $type) { - $validator->errors()->add('transactions.0.destination_id', (string) trans('validation.all_accounts_equal')); + $validator->errors()->add('transactions.0.destination_id', (string)trans('validation.all_accounts_equal')); } if ('transfer' === $type) { - $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')); + $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')); + } + Log::warning('Add error about equal accounts.'); + + return; + } + Log::debug('No errors found in validateEqualAccountsForUpdate'); + } + + /** + * @param array $transaction + * @param TransactionGroup $transactionGroup + * + * @return Account|null + */ + private function getOriginalSource(array $transaction, TransactionGroup $transactionGroup): ?Account + { + if (1 === $transactionGroup->transactionJournals->count()) { + $journal = $transactionGroup->transactionJournals->first(); + + return $journal->transactions()->where('amount', '<', 0)->first()->account; + } + /** @var TransactionJournal $journal */ + foreach ($transactionGroup->transactionJournals as $journal) { + if ((int)$journal->id === (int)$transaction['transaction_journal_id']) { + return $journal->transactions()->where('amount', '<', 0)->first()->account; } } + + return null; } } diff --git a/resources/lang/en_US/validation.php b/resources/lang/en_US/validation.php index f72b2baa44..6ed91c4a65 100644 --- a/resources/lang/en_US/validation.php +++ b/resources/lang/en_US/validation.php @@ -182,6 +182,8 @@ return [ 'withdrawal_dest_need_data' => 'Need to get a valid destination account ID and/or valid destination account name to continue.', 'withdrawal_dest_bad_data' => 'Could not find a valid destination account when searching for ID ":id" or name ":name".', + 'generic_source_bad_data' => 'Could not find a valid source account when searching for ID ":id" or name ":name".', + 'deposit_source_need_data' => 'Need to get a valid source account ID and/or valid source account name to continue.', 'deposit_source_bad_data' => 'Could not find a valid source account when searching for ID ":id" or name ":name".', 'deposit_dest_need_data' => 'Need to get a valid destination account ID and/or valid destination account name to continue.',