From d836c8217de87ebce117ba574a69142811ca8d2e Mon Sep 17 00:00:00 2001 From: James Cole Date: Sat, 24 Aug 2019 07:56:08 +0200 Subject: [PATCH] No longer have to submit mandatory fields to account end point. Just submit the field you wish to update, the rest will be untouched. --- app/Api/V1/Controllers/AccountController.php | 4 +- app/Api/V1/Requests/AccountUpdateRequest.php | 48 +++++++- app/Http/Requests/Request.php | 31 +++++ .../Account/AccountRepository.php | 107 +++++++++--------- .../Internal/Support/AccountServiceTrait.php | 3 +- .../Internal/Update/AccountUpdateService.php | 39 ++++--- 6 files changed, 161 insertions(+), 71 deletions(-) diff --git a/app/Api/V1/Controllers/AccountController.php b/app/Api/V1/Controllers/AccountController.php index f3e048ad30..567c86a594 100644 --- a/app/Api/V1/Controllers/AccountController.php +++ b/app/Api/V1/Controllers/AccountController.php @@ -284,13 +284,13 @@ class AccountController extends Controller * Update account. * * @param AccountUpdateRequest $request - * @param Account $account + * @param Account $account * * @return JsonResponse */ public function update(AccountUpdateRequest $request, Account $account): JsonResponse { - $data = $request->getAllAccountData(); + $data = $request->getUpdateData(); $data['type'] = config('firefly.shortNamesByFullName.' . $account->accountType->type); $this->repository->update($account, $data); $manager = new Manager; diff --git a/app/Api/V1/Requests/AccountUpdateRequest.php b/app/Api/V1/Requests/AccountUpdateRequest.php index d8b9d57149..518f543cc0 100644 --- a/app/Api/V1/Requests/AccountUpdateRequest.php +++ b/app/Api/V1/Requests/AccountUpdateRequest.php @@ -33,6 +33,52 @@ use FireflyIII\Rules\IsBoolean; class AccountUpdateRequest extends Request { + /** + * @return array + */ + public function getUpdateData(): array + { + $active = null; + $includeNetWorth = null; + if (null !== $this->get('active')) { + $active = $this->boolean('active'); + } + if (null !== $this->get('include_net_worth')) { + $includeNetWorth = $this->boolean('include_net_worth'); + } + + $data = [ + 'name' => $this->nullableString('name'), + 'active' => $active, + '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'), + 'account_number' => $this->nullableString('account_number'), + 'account_role' => $this->nullableString('account_role'), + 'opening_balance' => $this->nullableString('opening_balance'), + 'opening_balance_date' => $this->date('opening_balance_date'), + 'cc_type' => $this->nullableString('credit_card_type'), + 'cc_Monthly_payment_date' => $this->nullableString('monthly_payment_date'), + 'notes' => $this->nullableString('notes'), + 'interest' => $this->nullableString('interest'), + 'interest_period' => $this->nullableString('interest_period'), + ]; + + if ('liability' === $data['account_type']) { + $data['opening_balance'] = bcmul($this->nullableString('liability_amount'), '-1'); + $data['opening_balance_date'] = $this->date('liability_start_date'); + $data['account_type'] = $this->nullableString('liability_type'); + $data['account_type_id'] = null; + } + + return $data; + } + /** * Authorize logged in users. * @@ -56,7 +102,7 @@ class AccountUpdateRequest extends Request $types = implode(',', array_keys(config('firefly.subTitlesByIdentifier'))); $ccPaymentTypes = implode(',', array_keys(config('firefly.ccTypes'))); $rules = [ - 'name' => sprintf('required|min:1|uniqueAccountForUser:%d', $account->id), + 'name' => sprintf('min:1|uniqueAccountForUser:%d', $account->id), 'type' => sprintf('in:%s', $types), 'iban' => 'iban|nullable', 'bic' => 'bic|nullable', diff --git a/app/Http/Requests/Request.php b/app/Http/Requests/Request.php index 27887cc05b..73e00eba3a 100644 --- a/app/Http/Requests/Request.php +++ b/app/Http/Requests/Request.php @@ -177,6 +177,37 @@ class Request extends FormRequest return (int)$string; } + /** + * Return integer value, or NULL when it's not set. + * + * @param string $field + * + * @return int|null + */ + public function nullableInteger(string $field): ?int + { + $value = (string)$this->get($field); + if ('' === $value) { + return null; + } + + return (int)$value; + } + + /** + * Return string value, or NULL if empty. + * + * @param string $field + * + * @return string|null + */ + public function nullableString(string $field): ?string + { + $string = app('steam')->cleanString((string)($this->get($field) ?? '')); + + return '' === $string ? null : $string; + } + /** * Return string value. * diff --git a/app/Repositories/Account/AccountRepository.php b/app/Repositories/Account/AccountRepository.php index 55d61a41b2..ee7af4f3a6 100644 --- a/app/Repositories/Account/AccountRepository.php +++ b/app/Repositories/Account/AccountRepository.php @@ -26,6 +26,7 @@ use Carbon\Carbon; use FireflyIII\Exceptions\FireflyException; use FireflyIII\Factory\AccountFactory; use FireflyIII\Models\Account; +use FireflyIII\Models\AccountMeta; use FireflyIII\Models\AccountType; use FireflyIII\Models\TransactionCurrency; use FireflyIII\Models\TransactionGroup; @@ -57,10 +58,21 @@ class AccountRepository implements AccountRepositoryInterface Log::warning(sprintf('%s should not be instantiated in the TEST environment!', get_class($this))); } } + + /** + * @param array $types + * + * @return int + */ + public function count(array $types): int + { + return $this->user->accounts()->accountTypeIn($types)->count(); + } + /** * Moved here from account CRUD. * - * @param Account $account + * @param Account $account * @param Account|null $moveTo * * @return bool @@ -76,20 +88,9 @@ class AccountRepository implements AccountRepositoryInterface return true; } - - /** - * @param array $types - * - * @return int - */ - public function count(array $types): int - { - return $this->user->accounts()->accountTypeIn($types)->count(); - } - /** * @param string $number - * @param array $types + * @param array $types * * @return Account|null */ @@ -116,7 +117,7 @@ class AccountRepository implements AccountRepositoryInterface /** * @param string $iban - * @param array $types + * @param array $types * * @return Account|null */ @@ -144,7 +145,7 @@ class AccountRepository implements AccountRepositoryInterface /** * @param string $name - * @param array $types + * @param array $types * * @return Account|null */ @@ -307,21 +308,19 @@ class AccountRepository implements AccountRepositoryInterface * Return meta value for account. Null if not found. * * @param Account $account - * @param string $field + * @param string $field * * @return null|string */ public function getMetaValue(Account $account, string $field): ?string { - // TODO no longer need to loop like this - - foreach ($account->accountMeta as $meta) { - if ($meta->name === $field) { - return (string)$meta->data; - } + /** @var AccountMeta $meta */ + $meta = $account->accountMeta()->where('name', $field)->first(); + if (null === $meta) { + return null; } - return null; + return (string)$meta->data; } /** @@ -342,6 +341,20 @@ class AccountRepository implements AccountRepositoryInterface return $note->text; } + /** + * @param Account $account + * + * @return TransactionJournal|null + */ + public function getOpeningBalance(Account $account): ?TransactionJournal + { + return TransactionJournal + ::leftJoin('transactions', 'transactions.transaction_journal_id', '=', 'transaction_journals.id') + ->where('transactions.account_id', $account->id) + ->transactionTypes([TransactionType::OPENING_BALANCE]) + ->first(['transaction_journals.*']); + } + /** * Returns the amount of the opening balance for this account. * @@ -387,6 +400,22 @@ class AccountRepository implements AccountRepositoryInterface return $journal->date->format('Y-m-d'); } + /** + * @param Account $account + * + * @return TransactionGroup|null + */ + public function getOpeningBalanceGroup(Account $account): ?TransactionGroup + { + $journal = $this->getOpeningBalance($account); + $group = null; + if (null !== $journal) { + $group = $journal->transactionGroup; + } + + return $group; + } + /** * @param Account $account * @@ -483,7 +512,7 @@ class AccountRepository implements AccountRepositoryInterface /** * @param string $query - * @param array $types + * @param array $types * * @return Collection */ @@ -527,7 +556,7 @@ class AccountRepository implements AccountRepositoryInterface /** * @param Account $account - * @param array $data + * @param array $data * * @return Account * @throws FireflyException @@ -542,32 +571,4 @@ class AccountRepository implements AccountRepositoryInterface return $account; } - - /** - * @param Account $account - * @return TransactionJournal|null - */ - public function getOpeningBalance(Account $account): ?TransactionJournal - { - return TransactionJournal - ::leftJoin('transactions', 'transactions.transaction_journal_id', '=', 'transaction_journals.id') - ->where('transactions.account_id', $account->id) - ->transactionTypes([TransactionType::OPENING_BALANCE]) - ->first(['transaction_journals.*']); - } - - /** - * @param Account $account - * @return TransactionGroup|null - */ - public function getOpeningBalanceGroup(Account $account): ?TransactionGroup - { - $journal = $this->getOpeningBalance($account); - $group = null; - if (null !== $journal) { - $group = $journal->transactionGroup; - } - - return $group; - } } diff --git a/app/Services/Internal/Support/AccountServiceTrait.php b/app/Services/Internal/Support/AccountServiceTrait.php index 04b8f596d8..7e5b438963 100644 --- a/app/Services/Internal/Support/AccountServiceTrait.php +++ b/app/Services/Internal/Support/AccountServiceTrait.php @@ -95,12 +95,13 @@ trait AccountServiceTrait if ($account->accountType->type === AccountType::ASSET) { $fields = $this->validAssetFields; } - if ($account->accountType->type === AccountType::ASSET && 'ccAsset' === $data['account_role']) { + if ($account->accountType->type === AccountType::ASSET && isset($data['account_role']) && 'ccAsset' === $data['account_role']) { $fields = $this->validCCFields; } /** @var AccountMetaFactory $factory */ $factory = app(AccountMetaFactory::class); foreach ($fields as $field) { + $factory->crud($account, $field, (string)($data[$field] ?? '')); } } diff --git a/app/Services/Internal/Update/AccountUpdateService.php b/app/Services/Internal/Update/AccountUpdateService.php index 812143b7fb..5755923d7b 100644 --- a/app/Services/Internal/Update/AccountUpdateService.php +++ b/app/Services/Internal/Update/AccountUpdateService.php @@ -23,10 +23,8 @@ declare(strict_types=1); namespace FireflyIII\Services\Internal\Update; -use FireflyIII\Factory\TransactionCurrencyFactory; use FireflyIII\Models\Account; use FireflyIII\Models\AccountType; -use FireflyIII\Models\TransactionCurrency; use FireflyIII\Repositories\Account\AccountRepositoryInterface; use FireflyIII\Services\Internal\Support\AccountServiceTrait; use FireflyIII\User; @@ -39,13 +37,13 @@ class AccountUpdateService { use AccountServiceTrait; - /** @var array */ - private $canHaveVirtual; /** @var AccountRepositoryInterface */ protected $accountRepository; - + /** @var array */ + private $canHaveVirtual; /** @var User */ private $user; + /** * Constructor. */ @@ -73,22 +71,35 @@ class AccountUpdateService $this->user = $account->user; // update the account itself: - $account->name = $data['name']; - $account->active = $data['active']; - $account->virtual_balance = '' === trim($data['virtual_balance']) ? '0' : $data['virtual_balance']; - $account->iban = $data['iban']; + + $account->name = $data['name'] ?? $account->name; + $account->active = $data['active'] ?? $account->active; + if (null !== $data['virtual_balance']) { + $account->virtual_balance = '' === trim($data['virtual_balance']) ? '0' : $data['virtual_balance']; + } + $account->iban = $data['iban'] ?? $account->iban; $account->save(); - if (isset($data['currency_id']) && 0 === $data['currency_id']) { + + if (isset($data['currency_id']) && null !== $data['currency_id'] && 0 === $data['currency_id']) { unset($data['currency_id']); } + // find currency, or use default currency instead. - $currency = $this->getCurrency((int)($data['currency_id'] ?? null), (string)($data['currency_code'] ?? null)); - unset($data['currency_code']); - $data['currency_id'] = $currency->id; + if (null !== $data['currency_id'] || null !== $data['currency_code']) { + $currency = $this->getCurrency((int)($data['currency_id'] ?? null), (string)($data['currency_code'] ?? null)); + unset($data['currency_code']); + $data['currency_id'] = $currency->id; + } + if (null === $data['currency_id']) { + $data['currency_id'] = $this->accountRepository->getMetaValue($account, 'currency_id'); + } + if (null === $data['account_role']) { + $data['account_role'] = $this->accountRepository->getMetaValue($account, 'account_role'); + } // update all meta data: $this->updateMetaData($account, $data); @@ -108,7 +119,7 @@ class AccountUpdateService } // update note: - if (isset($data['notes'])) { + if (isset($data['notes']) && null !== $data['notes']) { $this->updateNote($account, (string)$data['notes']); }