diff --git a/app/Api/V1/Controllers/Models/Account/UpdateController.php b/app/Api/V1/Controllers/Models/Account/UpdateController.php index c7628391c3..55abbf968c 100644 --- a/app/Api/V1/Controllers/Models/Account/UpdateController.php +++ b/app/Api/V1/Controllers/Models/Account/UpdateController.php @@ -63,7 +63,7 @@ class UpdateController extends Controller * Update account. * * @param UpdateRequest $request - * @param Account $account + * @param Account $account * * @return JsonResponse */ @@ -71,8 +71,9 @@ class UpdateController extends Controller { $data = $request->getUpdateData(); $data['type'] = config('firefly.shortNamesByFullName.' . $account->accountType->type); - $this->repository->update($account, $data); - $manager = $this->getManager(); + $account = $this->repository->update($account, $data); + $manager = $this->getManager(); + $account->refresh(); /** @var AccountTransformer $transformer */ $transformer = app(AccountTransformer::class); diff --git a/app/Api/V1/Requests/Models/Account/UpdateRequest.php b/app/Api/V1/Requests/Models/Account/UpdateRequest.php index 4ccb8cd887..6ae06e6169 100644 --- a/app/Api/V1/Requests/Models/Account/UpdateRequest.php +++ b/app/Api/V1/Requests/Models/Account/UpdateRequest.php @@ -68,6 +68,7 @@ class UpdateRequest extends FormRequest 'BIC' => $this->nullableString('bic'), 'account_number' => $this->nullableString('account_number'), 'account_role' => $this->nullableString('account_role'), + 'liability_type' => $this->nullableString('liability_type'), 'opening_balance' => $this->nullableString('opening_balance'), 'opening_balance_date' => $this->date('opening_balance_date'), 'cc_type' => $this->nullableString('credit_card_type'), @@ -76,7 +77,7 @@ class UpdateRequest extends FormRequest 'interest' => $this->nullableString('interest'), 'interest_period' => $this->nullableString('interest_period'), ]; - if(null !== $this->get('order')) { + if (null !== $this->get('order')) { $data['order'] = $this->integer('order'); } @@ -122,8 +123,6 @@ class UpdateRequest extends FormRequest 'credit_card_type' => sprintf('in:%s|required_if:account_role,ccAsset', $ccPaymentTypes), 'monthly_payment_date' => 'date' . '|required_if:account_role,ccAsset|required_if:credit_card_type,monthlyFull', 'liability_type' => 'required_if:type,liability|in:loan,debt,mortgage', - 'liability_amount' => 'required_if:type,liability|min:0|numeric', - 'liability_start_date' => 'required_if:type,liability|date', 'interest' => 'required_if:type,liability|between:0,100|numeric', 'interest_period' => 'required_if:type,liability|in:daily,monthly,yearly', 'notes' => 'min:0|max:65536', diff --git a/app/Services/Internal/Support/AccountServiceTrait.php b/app/Services/Internal/Support/AccountServiceTrait.php index 55f30c5c9d..db0d4c3ee7 100644 --- a/app/Services/Internal/Support/AccountServiceTrait.php +++ b/app/Services/Internal/Support/AccountServiceTrait.php @@ -23,6 +23,7 @@ declare(strict_types=1); namespace FireflyIII\Services\Internal\Support; +use Carbon\Carbon; use Exception; use FireflyIII\Exceptions\FireflyException; use FireflyIII\Factory\AccountMetaFactory; @@ -35,6 +36,7 @@ use FireflyIII\Models\Transaction; use FireflyIII\Models\TransactionCurrency; use FireflyIII\Models\TransactionGroup; use FireflyIII\Models\TransactionJournal; +use FireflyIII\Repositories\Account\AccountRepositoryInterface; use FireflyIII\Services\Internal\Destroy\TransactionGroupDestroyService; use Log; use Validator; @@ -45,6 +47,8 @@ use Validator; */ trait AccountServiceTrait { + protected AccountRepositoryInterface $accountRepository; + /** * @param null|string $iban * @@ -84,6 +88,14 @@ trait AccountServiceTrait if ($account->accountType->type === AccountType::ASSET) { $fields = $this->validAssetFields; } + + // the account role may not be set in the data but we may have it already: + if (!array_key_exists('account_role', $data)) { + $data['account_role'] = null; + } + if (null === $data['account_role']) { + $data['account_role'] = $this->accountRepository->getMetaValue($account, 'account_role'); + } if ($account->accountType->type === AccountType::ASSET && isset($data['account_role']) && 'ccAsset' === $data['account_role']) { $fields = $this->validCCFields; // @codeCoverageIgnore } @@ -171,8 +183,8 @@ trait AccountServiceTrait */ public function isEmptyOBData(array $data): bool { - if (!array_key_exists('opening_balance', $data) && - !array_key_exists('opening_balance_date', $data) + if (!array_key_exists('opening_balance', $data) + && !array_key_exists('opening_balance_date', $data) ) { // not set, so false. return false; @@ -180,8 +192,7 @@ trait AccountServiceTrait // if isset, but is empty: if ( (array_key_exists('opening_balance', $data) && '' === $data['opening_balance']) - || - (array_key_exists('opening_balance_date', $data) && '' === $data['opening_balance_date']) + || (array_key_exists('opening_balance_date', $data) && '' === $data['opening_balance_date']) ) { return true; } @@ -266,6 +277,7 @@ trait AccountServiceTrait Log::error($e->getMessage()); Log::error($e->getTraceAsString()); } + // @codeCoverageIgnoreEnd return $group; diff --git a/app/Services/Internal/Update/AccountUpdateService.php b/app/Services/Internal/Update/AccountUpdateService.php index 955b068840..53703e8085 100644 --- a/app/Services/Internal/Update/AccountUpdateService.php +++ b/app/Services/Internal/Update/AccountUpdateService.php @@ -34,6 +34,7 @@ use Log; /** * Class AccountUpdateService + * TODO this is a mess. */ class AccountUpdateService { @@ -147,17 +148,25 @@ class AccountUpdateService } /** - * @param int $accountTypeId + * @param string $type * * @return bool */ - private function isLiabilityTypeId(int $accountTypeId): bool + private function isLiabilityType(string $type): bool { - if (0 === $accountTypeId) { + if ('' === $type) { return false; } - return 1 === AccountType::whereIn('type', [AccountType::DEBT, AccountType::LOAN, AccountType::MORTGAGE])->where('id', $accountTypeId)->count(); + return 1 === AccountType::whereIn('type', [AccountType::DEBT, AccountType::LOAN, AccountType::MORTGAGE])->where('type', ucfirst($type))->count(); + } + + /** + * @param string $type + */ + private function getAccountType(string $type): AccountType + { + return AccountType::whereType($type)->first(); } /** @@ -173,10 +182,11 @@ class AccountUpdateService $account->active = $data['active'] ?? $account->active; $account->iban = $data['iban'] ?? $account->iban; - // if account type is a liability, the liability type (account type) - // can be updated to another one. - if ($this->isLiability($account) && $this->isLiabilityTypeId((int)($data['account_type_id'] ?? 0))) { - $account->account_type_id = (int)$data['account_type_id']; + // liability stuff: + $liabilityType = $data['liability_type'] ?? ''; + if ($this->isLiability($account) && $this->isLiabilityType($liabilityType)) { + $type = $this->getAccountType($liabilityType); + $account->account_type_id = $type->id; } // update virtual balance (could be set to zero if empty string). diff --git a/app/Transformers/AccountTransformer.php b/app/Transformers/AccountTransformer.php index e27dc8334d..cad7638d42 100644 --- a/app/Transformers/AccountTransformer.php +++ b/app/Transformers/AccountTransformer.php @@ -76,12 +76,6 @@ class AccountTransformer extends AbstractTransformer [$interest, $interestPeriod] = $this->getInterest($account, $accountType); $openingBalance = number_format((float) $openingBalance, $decimalPlaces, '.', ''); - $liabilityAmount = null; - $liabilityStart = null; - if (null !== $liabilityType) { - $liabilityAmount = $openingBalance; - $liabilityStart = $openingBalanceDate; - } $includeNetWorth = '0' !== $this->repository->getMetaValue($account, 'include_net_worth'); $longitude = null; $latitude = null; @@ -117,8 +111,6 @@ class AccountTransformer extends AbstractTransformer 'opening_balance' => $openingBalance, 'opening_balance_date' => $openingBalanceDate, 'liability_type' => $liabilityType, - 'liability_amount' => $liabilityAmount, - 'liability_start_date' => $liabilityStart, 'interest' => $interest, 'interest_period' => $interestPeriod, 'include_net_worth' => $includeNetWorth,