From 0031c5a5ad1a672fb413c1f1bfc518e1f03d54a4 Mon Sep 17 00:00:00 2001 From: James Cole Date: Wed, 5 Mar 2025 20:12:44 +0100 Subject: [PATCH] Fix #9914 --- .../Models/PiggyBank/UpdateRequest.php | 2 +- app/Factory/PiggyBankFactory.php | 65 ++++++++++--------- app/Rules/IsValidZeroOrMoreAmount.php | 11 ++++ app/Support/Request/ConvertsDataTypes.php | 12 +++- 4 files changed, 56 insertions(+), 34 deletions(-) diff --git a/app/Api/V1/Requests/Models/PiggyBank/UpdateRequest.php b/app/Api/V1/Requests/Models/PiggyBank/UpdateRequest.php index 61312e094c..6aa7e417b8 100644 --- a/app/Api/V1/Requests/Models/PiggyBank/UpdateRequest.php +++ b/app/Api/V1/Requests/Models/PiggyBank/UpdateRequest.php @@ -82,7 +82,7 @@ class UpdateRequest extends FormRequest 'accounts' => 'required', 'accounts.*' => 'array|required', 'accounts.*.account_id' => ['required', 'numeric', 'belongsToUser:accounts,id'], - 'accounts.*.current_amount' => ['numeric', new IsValidZeroOrMoreAmount()], + 'accounts.*.current_amount' => ['numeric','nullable', new IsValidZeroOrMoreAmount(true)], 'object_group_id' => 'numeric|belongsToUser:object_groups,id', 'object_group_title' => ['min:1', 'max:255'], 'transaction_currency_id' => 'exists:transaction_currencies,id|nullable', diff --git a/app/Factory/PiggyBankFactory.php b/app/Factory/PiggyBankFactory.php index fbe1b450fb..d0473beddc 100644 --- a/app/Factory/PiggyBankFactory.php +++ b/app/Factory/PiggyBankFactory.php @@ -67,7 +67,7 @@ class PiggyBankFactory public function store(array $data): PiggyBank { - $piggyBankData = $data; + $piggyBankData = $data; // unset some fields unset($piggyBankData['object_group_title'], $piggyBankData['transaction_currency_code'], $piggyBankData['transaction_currency_id'], $piggyBankData['accounts'], $piggyBankData['object_group_id'], $piggyBankData['notes']); @@ -91,11 +91,11 @@ class PiggyBankFactory throw new FireflyException('400005: Could not store new piggy bank.', 0, $e); } - $piggyBank = $this->setOrder($piggyBank, $data); + $piggyBank = $this->setOrder($piggyBank, $data); $this->linkToAccountIds($piggyBank, $data['accounts']); $this->piggyBankRepository->updateNote($piggyBank, $data['notes']); - $objectGroupTitle = $data['object_group_title'] ?? ''; + $objectGroupTitle = $data['object_group_title'] ?? ''; if ('' !== $objectGroupTitle) { $objectGroup = $this->findOrCreateObjectGroup($objectGroupTitle); if (null !== $objectGroup) { @@ -103,7 +103,7 @@ class PiggyBankFactory } } // try also with ID - $objectGroupId = (int) ($data['object_group_id'] ?? 0); + $objectGroupId = (int) ($data['object_group_id'] ?? 0); if (0 !== $objectGroupId) { $objectGroup = $this->findObjectGroupById($objectGroupId); if (null !== $objectGroup) { @@ -111,7 +111,7 @@ class PiggyBankFactory } } Log::debug('Touch piggy bank'); - $piggyBank->encrypted = false; + $piggyBank->encrypted = false; $piggyBank->save(); $piggyBank->touch(); @@ -144,11 +144,10 @@ class PiggyBankFactory // first find by ID: if ($piggyBankId > 0) { $piggyBank = PiggyBank::leftJoin('account_piggy_bank', 'account_piggy_bank.piggy_bank_id', '=', 'piggy_banks.id') - ->leftJoin('accounts', 'accounts.id', '=', 'account_piggy_bank.account_id') - ->where('accounts.user_id', $this->user->id) - ->where('piggy_banks.id', $piggyBankId) - ->first(['piggy_banks.*']) - ; + ->leftJoin('accounts', 'accounts.id', '=', 'account_piggy_bank.account_id') + ->where('accounts.user_id', $this->user->id) + ->where('piggy_banks.id', $piggyBankId) + ->first(['piggy_banks.*']); if (null !== $piggyBank) { return $piggyBank; } @@ -169,17 +168,16 @@ class PiggyBankFactory public function findByName(string $name): ?PiggyBank { return PiggyBank::leftJoin('account_piggy_bank', 'account_piggy_bank.piggy_bank_id', '=', 'piggy_banks.id') - ->leftJoin('accounts', 'accounts.id', '=', 'account_piggy_bank.account_id') - ->where('accounts.user_id', $this->user->id) - ->where('piggy_banks.name', $name) - ->first(['piggy_banks.*']) - ; + ->leftJoin('accounts', 'accounts.id', '=', 'account_piggy_bank.account_id') + ->where('accounts.user_id', $this->user->id) + ->where('piggy_banks.name', $name) + ->first(['piggy_banks.*']); } private function setOrder(PiggyBank $piggyBank, array $data): PiggyBank { $this->resetOrder(); - $order = $this->getMaxOrder() + 1; + $order = $this->getMaxOrder() + 1; if (array_key_exists('order', $data)) { $order = $data['order']; } @@ -194,15 +192,14 @@ class PiggyBankFactory { // TODO duplicate code $set = PiggyBank::leftJoin('account_piggy_bank', 'account_piggy_bank.piggy_bank_id', '=', 'piggy_banks.id') - ->leftJoin('accounts', 'accounts.id', '=', 'account_piggy_bank.account_id') - ->where('accounts.user_id', $this->user->id) - ->with( - [ - 'objectGroups', - ] - ) - ->orderBy('piggy_banks.order', 'ASC')->get(['piggy_banks.*']) - ; + ->leftJoin('accounts', 'accounts.id', '=', 'account_piggy_bank.account_id') + ->where('accounts.user_id', $this->user->id) + ->with( + [ + 'objectGroups', + ] + ) + ->orderBy('piggy_banks.order', 'ASC')->get(['piggy_banks.*']); $current = 1; foreach ($set as $piggyBank) { if ($piggyBank->order !== $current) { @@ -227,12 +224,12 @@ class PiggyBankFactory // TODO this is a tedious check. Feels like a hack. $toBeLinked = []; foreach ($piggyBank->accounts as $account) { + Log::debug(sprintf('Checking account #%d', $account->id)); foreach ($accounts as $info) { - if ($account->id === $info['account_id']) { - if (array_key_exists($account->id, $accounts)) { - $toBeLinked[$account->id] = ['current_amount' => $account->pivot->current_amount ?? '0']; - Log::debug(sprintf('Prefilled for account #%d with amount %s', $account->id, $account->pivot->current_amount ?? '0')); - } + Log::debug(sprintf(' Checking other account #%d', $info['account_id'])); + if ((int) $account->id === (int) $info['account_id']) { + $toBeLinked[$account->id] = ['current_amount' => $account->pivot->current_amount ?? '0']; + Log::debug(sprintf('Prefilled for account #%d with amount %s', $account->id, $account->pivot->current_amount ?? '0')); } } } @@ -244,9 +241,13 @@ class PiggyBankFactory if (null === $account) { continue; } - if (array_key_exists('current_amount', $info)) { + if (array_key_exists('current_amount', $info) && null !== $info['current_amount']) { $toBeLinked[$account->id] = ['current_amount' => $info['current_amount']]; - Log::debug(sprintf('Will link account #%d with amount %s', $account->id, $account->pivot->current_amount ?? '0')); + Log::debug(sprintf('[a] Will link account #%d with amount %s', $account->id, $info['current_amount'])); + } + if (array_key_exists('current_amount', $info) && null === $info['current_amount']) { + $toBeLinked[$account->id] = ['current_amount' => $toBeLinked[$account->id]['current_amount'] ?? '0']; + Log::debug(sprintf('[b] Will link account #%d with amount %s', $account->id, $toBeLinked[$account->id]['current_amount'] ?? '0')); } if (!array_key_exists('current_amount', $info)) { $toBeLinked[$account->id] ??= []; diff --git a/app/Rules/IsValidZeroOrMoreAmount.php b/app/Rules/IsValidZeroOrMoreAmount.php index da87757a6a..91f08dad04 100644 --- a/app/Rules/IsValidZeroOrMoreAmount.php +++ b/app/Rules/IsValidZeroOrMoreAmount.php @@ -31,6 +31,14 @@ use Illuminate\Support\Facades\Log; class IsValidZeroOrMoreAmount implements ValidationRule { + private bool $nullable = false; + + public function __construct(bool $nullable = false) + { + $this->nullable = $nullable; + } + + use ValidatesAmountsTrait; /** @@ -38,6 +46,9 @@ class IsValidZeroOrMoreAmount implements ValidationRule */ public function validate(string $attribute, mixed $value, \Closure $fail): void { + if (true === $this->nullable && null === $value) { + return; + } $value = (string) $value; // must not be empty: if ($this->emptyString($value)) { diff --git a/app/Support/Request/ConvertsDataTypes.php b/app/Support/Request/ConvertsDataTypes.php index 18e93a8343..95dcea0c8c 100644 --- a/app/Support/Request/ConvertsDataTypes.php +++ b/app/Support/Request/ConvertsDataTypes.php @@ -400,9 +400,19 @@ trait ConvertsDataTypes if (!is_array($entry)) { continue; } + $amount = null; + if(array_key_exists('current_amount',$entry)) { + $amount = $this->clearString((string) ($entry['current_amount'] ?? '0')); + if(null === $entry['current_amount']) { + $amount = null; + } + } + if(!array_key_exists('current_amount',$entry)) { + $amount = null; + } $return[] = [ 'account_id' => $this->integerFromValue((string) ($entry['account_id'] ?? '0')), - 'current_amount' => $this->clearString((string) ($entry['current_amount'] ?? '0')), + 'current_amount' => $amount, ]; }