From b812c2e09f45e9cf716ba36179b09ca8fa2f44e2 Mon Sep 17 00:00:00 2001 From: James Cole Date: Mon, 5 Apr 2021 10:56:08 +0200 Subject: [PATCH] Simplify account creation. --- .../Requests/Models/Account/StoreRequest.php | 2 +- app/Factory/AccountFactory.php | 286 ++++++++++++------ app/Http/Requests/AccountFormRequest.php | 9 +- .../Account/AccountRepository.php | 2 +- .../Destroy/AccountDestroyService.php | 23 ++ config/firefly.php | 6 +- 6 files changed, 226 insertions(+), 102 deletions(-) diff --git a/app/Api/V1/Requests/Models/Account/StoreRequest.php b/app/Api/V1/Requests/Models/Account/StoreRequest.php index 0049d2b350..c73a2f0b9b 100644 --- a/app/Api/V1/Requests/Models/Account/StoreRequest.php +++ b/app/Api/V1/Requests/Models/Account/StoreRequest.php @@ -59,7 +59,7 @@ class StoreRequest extends FormRequest 'name' => $this->string('name'), 'active' => $active, 'include_net_worth' => $includeNetWorth, - 'account_type' => $this->string('type'), + 'account_type_name' => $this->string('type'), 'account_type_id' => null, 'currency_id' => $this->integer('currency_id'), 'order' => $this->integer('order'), diff --git a/app/Factory/AccountFactory.php b/app/Factory/AccountFactory.php index e755820b98..f7c8aa2b55 100644 --- a/app/Factory/AccountFactory.php +++ b/app/Factory/AccountFactory.php @@ -57,12 +57,11 @@ class AccountFactory */ public function __construct() { - $this->canHaveVirtual = [AccountType::ASSET, AccountType::DEBT, AccountType::LOAN, AccountType::MORTGAGE, AccountType::CREDITCARD]; $this->accountRepository = app(AccountRepositoryInterface::class); - $this->validAssetFields = ['account_role', 'account_number', 'currency_id', 'BIC', 'include_net_worth']; - $this->validCCFields = ['account_role', 'cc_monthly_payment_date', 'cc_type', 'account_number', 'currency_id', 'BIC', 'include_net_worth']; - $this->validFields = ['account_number', 'currency_id', 'BIC', 'interest', 'interest_period', 'include_net_worth']; - + $this->canHaveVirtual = config('firefly.can_have_virtual_amounts'); + $this->validAssetFields = config('firefly.valid_asset_fields'); + $this->validCCFields = config('firefly.valid_cc_fields'); + $this->validFields = config('firefly.valid_account_fields'); } /** @@ -74,16 +73,26 @@ class AccountFactory */ public function findOrCreate(string $accountName, string $accountType): Account { - Log::debug(sprintf('Searching for "%s" of type "%s"', $accountName, $accountType)); - /** @var AccountType $type */ - $type = AccountType::whereType($accountType)->first(); + Log::debug(sprintf('findOrCreate("%s", "%s")', $accountName, $accountType)); + + $type = $this->accountRepository->getAccountTypeByType($accountType); + if (null === $type) { + throw new FireflyException(sprintf('Cannot find account type "%s"', $accountType)); + } $return = $this->user->accounts->where('account_type_id', $type->id)->where('name', $accountName)->first(); if (null === $return) { Log::debug('Found nothing. Will create a new one.'); $return = $this->create( - ['user_id' => $this->user->id, 'name' => $accountName, 'account_type_id' => $type->id, 'account_type' => null, 'virtual_balance' => '0', - 'iban' => null, 'active' => true,] + [ + 'user_id' => $this->user->id, + 'name' => $accountName, + 'account_type_id' => $type->id, + 'account_type' => null, + 'virtual_balance' => '0', + 'iban' => null, + 'active' => true, + ] ); } @@ -98,118 +107,51 @@ class AccountFactory */ public function create(array $data): Account { - $type = $this->getAccountType($data['account_type_id'] ?? null, $data['account_type'] ?? null); - if (null === $type) { - throw new FireflyException( - sprintf('AccountFactory::create() was unable to find account type #%d ("%s").', $data['account_type_id'] ?? null, $data['account_type'] ?? null) - ); - } - + $type = $this->getAccountType($data); $data['iban'] = $this->filterIban($data['iban'] ?? null); // account may exist already: - Log::debug('Data array is as follows', $data); $return = $this->find($data['name'], $type->type); if (null === $return) { - $this->accountRepository->resetAccountOrder(); - - // create it: - $databaseData = ['user_id' => $this->user->id, - 'account_type_id' => $type->id, - 'name' => $data['name'], - 'order' => 25000, - 'virtual_balance' => $data['virtual_balance'] ?? null, 'active' => true === $data['active'], 'iban' => $data['iban'],]; - - $currency = $this->getCurrency((int)($data['currency_id'] ?? null), (string)($data['currency_code'] ?? null)); - unset($data['currency_code']); - $data['currency_id'] = $currency->id; - - // remove virtual balance when not an asset account or a liability - if (!in_array($type->type, $this->canHaveVirtual, true)) { - $databaseData['virtual_balance'] = null; - } - - // fix virtual balance when it's empty - if ('' === (string)$databaseData['virtual_balance']) { - $databaseData['virtual_balance'] = null; - } - - $return = Account::create($databaseData); - $this->updateMetaData($return, $data); - - // if it can have a virtual balance, it can also have an opening balance. - if (in_array($type->type, $this->canHaveVirtual, true)) { - if ($this->validOBData($data)) { - $this->updateOBGroup($return, $data); - } - if (!$this->validOBData($data)) { - $this->deleteOBGroup($return); - } - } - $this->updateNote($return, $data['notes'] ?? ''); - - // store location - $this->storeNewLocation($return, $data); - - // update order to be correct: - - // set new order: - $maxOrder = $this->accountRepository->maxOrder($type->type); - $order = null; - if (!array_key_exists('order', $data)) { - // take maxOrder + 1 - $order = $maxOrder + 1; - } - if (array_key_exists('order', $data)) { - // limit order - $order = (int)($data['order'] > $maxOrder ? $maxOrder + 1 : $data['order']); - $order = 0 === $order ? $maxOrder + 1 : $order; - } - $updateService = app(AccountUpdateService::class); - $updateService->setUser($return->user); - Log::debug(sprintf('Will set order to %d', $order)); - $return = $updateService->update($return, ['order' => $order]); + $return = $this->createAccount($type, $data); } return $return; } /** - * @param int|null $accountTypeId - * @param null|string $accountType + * @param array $data * * @return AccountType|null */ - protected function getAccountType(?int $accountTypeId, ?string $accountType): ?AccountType + protected function getAccountType(array $data): ?AccountType { - $accountTypeId = (int)$accountTypeId; - $result = null; + $accountTypeId = array_key_exists('account_type_id', $data) ? (int)$data['account_type_id'] : 0; + $accountTypeName = array_key_exists('account_type_name', $data) ? $data['account_type_name'] : null; + $result = null; + // find by name or ID if ($accountTypeId > 0) { $result = AccountType::find($accountTypeId); } + if (null !== $accountTypeName) { + $result = $this->accountRepository->getAccountTypeByType($accountTypeName); + } + + // try with type: if (null === $result) { - Log::debug(sprintf('No account type found by ID, continue search for "%s".', $accountType)); - /** @var array $types */ - $types = config('firefly.accountTypeByIdentifier.' . $accountType) ?? []; + $types = config(sprintf('firefly.accountTypeByIdentifier.%s', $accountTypeName)) ?? []; if (count($types) > 0) { - Log::debug(sprintf('%d accounts in list from config', count($types)), $types); $result = AccountType::whereIn('type', $types)->first(); } - if (null === $result && null !== $accountType) { - // try as full name: - $result = AccountType::whereType($accountType)->first(); - } } if (null === $result) { - Log::warning(sprintf('Found NO account type based on %d and "%s"', $accountTypeId, $accountType)); - } - if (null !== $result) { - Log::debug(sprintf('Found account type based on %d and "%s": "%s"', $accountTypeId, $accountType, $result->type)); + Log::warning(sprintf('Found NO account type based on %d and "%s"', $accountTypeId, $accountTypeName)); + throw new FireflyException(sprintf('AccountFactory::create() was unable to find account type #%d ("%s").', $accountTypeId, $accountTypeName)); } + Log::debug(sprintf('Found account type based on %d and "%s": "%s"', $accountTypeId, $accountTypeName, $result->type)); return $result; - } /** @@ -225,6 +167,160 @@ class AccountFactory return $this->user->accounts()->where('account_type_id', $type->id)->where('name', $accountName)->first(); } + /** + * @param array $data + * + * @return Account + */ + private function createAccount(AccountType $type, array $data): Account + { + $this->accountRepository->resetAccountOrder(); + + // create it: + $virtualBalance = array_key_exists('virtual_balance', $data) ? $data['virtual_balance'] : null; + $active = array_key_exists('active', $data) ? $data['active'] : true; + $databaseData = ['user_id' => $this->user->id, + 'account_type_id' => $type->id, + 'name' => $data['name'], + 'order' => 25000, + 'virtual_balance' => $virtualBalance, + 'active' => $active, + 'iban' => $data['iban'], + ]; + // fix virtual balance when it's empty + if ('' === (string)$databaseData['virtual_balance']) { + $databaseData['virtual_balance'] = null; + } + // remove virtual balance when not an asset account or a liability + if (!in_array($type->type, $this->canHaveVirtual, true)) { + $databaseData['virtual_balance'] = null; + } + // create account! + $account = Account::create($databaseData); + + // update meta data: + $data = $this->cleanMetaDataArray($account, $data); + $this->storeMetaData($account, $data); + + // create opening balance + $this->storeOpeningBalance($account, $data); + + // create notes + $notes = array_key_exists('notes', $data) ? $data['notes'] : ''; + $this->updateNote($account, $notes); + + // create location + $this->storeNewLocation($account, $data); + + // set order + $this->storeOrder($account, $data); + + // refresh and return + $account->refresh(); + + return $account; + } + + /** + * @param Account $account + * @param array $data + * + * @return array + */ + private function cleanMetaDataArray(Account $account, array $data): array + { + $currencyId = array_key_exists('currency_id', $data) ? (int)$data['currency_id'] : 0; + $currencyCode = array_key_exists('currency_code', $data) ? (string)$data['currency_code'] : ''; + $accountRole = array_key_exists('account_role', $data) ? (string)$data['account_role'] : null; + $currency = $this->getCurrency($currencyId, $currencyCode); + + // only asset account may have a role: + if ($account->accountType->type !== AccountType::ASSET) { + $accountRole = ''; + } + + $data['account_role'] = $accountRole; + $data['currency_id'] = $currency->id; + + return $data; + } + + /** + * @param Account $account + * @param array $data + */ + private function storeMetaData(Account $account, array $data): void + { + + $fields = $this->validFields; + if ($account->accountType->type === AccountType::ASSET) { + $fields = $this->validAssetFields; + } + if ($account->accountType->type === AccountType::ASSET && 'ccAsset' === $data['account_role']) { + $fields = $this->validCCFields; // @codeCoverageIgnore + } + + /** @var AccountMetaFactory $factory */ + $factory = app(AccountMetaFactory::class); + foreach ($fields as $field) { + // if the field is set but NULL, skip it. + // if the field is set but "", update it. + if (isset($data[$field]) && null !== $data[$field]) { + + // convert boolean value: + if (is_bool($data[$field]) && false === $data[$field]) { + $data[$field] = 0; // @codeCoverageIgnore + } + if (is_bool($data[$field]) && true === $data[$field]) { + $data[$field] = 1; // @codeCoverageIgnore + } + + $factory->crud($account, $field, (string)$data[$field]); + } + } + } + + /** + * @param Account $account + * @param array $data + */ + private function storeOpeningBalance(Account $account, array $data) + { + $accountType = $account->accountType->type; + + // if it can have a virtual balance, it can also have an opening balance. + if (in_array($accountType, $this->canHaveVirtual, true)) { + if ($this->validOBData($data)) { + $this->updateOBGroup($account, $data); + } + if (!$this->validOBData($data)) { + $this->deleteOBGroup($account); + } + } + } + + /** + * @param Account $account + * @param array $data + */ + private function storeOrder(Account $account, array $data): void + { + $accountType = $account->accountType->type; + $maxOrder = $this->accountRepository->maxOrder($accountType); + $order = null; + if (!array_key_exists('order', $data)) { + $order = $maxOrder + 1; + } + if (array_key_exists('order', $data)) { + $order = (int)($data['order'] > $maxOrder ? $maxOrder + 1 : $data['order']); + $order = 0 === $order ? $maxOrder + 1 : $order; + } + + $updateService = app(AccountUpdateService::class); + $updateService->setUser($account->user); + $updateService->update($account, ['order' => $order]); + } + /** * @param User $user */ @@ -232,4 +328,6 @@ class AccountFactory { $this->user = $user; } + + } diff --git a/app/Http/Requests/AccountFormRequest.php b/app/Http/Requests/AccountFormRequest.php index 939a21f41b..b050d8bbf5 100644 --- a/app/Http/Requests/AccountFormRequest.php +++ b/app/Http/Requests/AccountFormRequest.php @@ -47,8 +47,7 @@ class AccountFormRequest extends FormRequest $data = [ 'name' => $this->string('name'), 'active' => $this->boolean('active'), - 'account_type' => $this->string('objectType'), - 'account_type_id' => 0, + 'account_type_name' => $this->string('objectType'), 'currency_id' => $this->integer('currency_id'), 'virtual_balance' => $this->string('virtual_balance'), 'iban' => $this->string('iban'), @@ -72,9 +71,9 @@ class AccountFormRequest extends FormRequest // if the account type is "liabilities" there are actually four types of liability // that could have been selected. - if ('liabilities' === $data['account_type']) { - $data['account_type'] = null; - $data['account_type_id'] = $this->integer('liability_type_id'); + if ('liabilities' === $data['account_type_name']) { + $data['account_type_name'] = null; + $data['account_type_id'] = $this->integer('liability_type_id'); } return $data; diff --git a/app/Repositories/Account/AccountRepository.php b/app/Repositories/Account/AccountRepository.php index 71dbda25c4..2b17a7029e 100644 --- a/app/Repositories/Account/AccountRepository.php +++ b/app/Repositories/Account/AccountRepository.php @@ -210,7 +210,7 @@ class AccountRepository implements AccountRepositoryInterface */ public function getAccountTypeByType(string $type): ?AccountType { - return AccountType::whereType($type)->first(); + return AccountType::whereType(ucfirst($type))->first(); } /** diff --git a/app/Services/Internal/Destroy/AccountDestroyService.php b/app/Services/Internal/Destroy/AccountDestroyService.php index 27b68eb942..9e38e7e74b 100644 --- a/app/Services/Internal/Destroy/AccountDestroyService.php +++ b/app/Services/Internal/Destroy/AccountDestroyService.php @@ -117,7 +117,30 @@ class AccountDestroyService */ public function moveTransactions(Account $account, Account $moveTo): void { + Log::debug(sprintf('Move from account #%d to #%d', $account->id, $moveTo->id)); DB::table('transactions')->where('account_id', $account->id)->update(['account_id' => $moveTo->id]); + + $collection = Transaction::groupBy('transaction_journal_id', 'account_id') + ->where('account_id', $moveTo->id) + ->get(['transaction_journal_id', 'account_id', DB::raw('count(*) as the_count')]); + if (0 === $collection->count()) { + return; + } + + /** @var JournalDestroyService $service */ + $service = app(JournalDestroyService::class); + $user = $account->user; + /** @var \stdClass $row */ + foreach ($collection as $row) { + if ((int)$row->the_count > 1) { + $journalId = (int)$row->transaction_journal_id; + $journal = $user->transactionJournals()->find($journalId); + if (null !== $journal) { + Log::debug(sprintf('Deleted journal #%d because it has the same source as destination.', $journal->id)); + $service->destroy($journal); + } + } + } } /** diff --git a/config/firefly.php b/config/firefly.php index 6152fd714f..ec9a153cdd 100644 --- a/config/firefly.php +++ b/config/firefly.php @@ -123,7 +123,7 @@ return [ 'authentication_guard' => envNonEmpty('AUTHENTICATION_GUARD', 'web'), 'custom_logout_uri' => envNonEmpty('CUSTOM_LOGOUT_URI', ''), 'cer_provider' => envNonEmpty('CER_PROVIDER', 'fixer'), - 'ipinfo_token' => env('IPINFO_TOKEN',''), + 'ipinfo_token' => env('IPINFO_TOKEN', ''), 'update_endpoint' => 'https://version.firefly-iii.org/index.json', 'send_telemetry' => env('SEND_TELEMETRY', false), 'allow_webhooks' => env('ALLOW_WEBHOOKS', false), @@ -849,4 +849,8 @@ return [ Webhook::DELIVERY_JSON => 'DELIVERY_JSON', ], ], + 'can_have_virtual_amounts' => [AccountType::ASSET, AccountType::DEBT, AccountType::LOAN, AccountType::MORTGAGE, AccountType::CREDITCARD], + 'valid_asset_fields' => ['account_role', 'account_number', 'currency_id', 'BIC', 'include_net_worth'], + 'valid_cc_fields' => ['account_role', 'cc_monthly_payment_date', 'cc_type', 'account_number', 'currency_id', 'BIC', 'include_net_worth'], + 'valid_account_fields' => ['account_number', 'currency_id', 'BIC', 'interest', 'interest_period', 'include_net_worth'], ];