diff --git a/app/Crud/Account/AccountCrud.php b/app/Crud/Account/AccountCrud.php index 172d3f1783..814735dae1 100644 --- a/app/Crud/Account/AccountCrud.php +++ b/app/Crud/Account/AccountCrud.php @@ -15,6 +15,7 @@ namespace FireflyIII\Crud\Account; use Carbon\Carbon; use DB; +use FireflyIII\Exceptions\FireflyException; use FireflyIII\Models\Account; use FireflyIII\Models\AccountMeta; use FireflyIII\Models\AccountType; @@ -51,6 +52,18 @@ class AccountCrud implements AccountCrudInterface $this->user = $user; } + /** + * @param array $types + * + * @return int + */ + public function count(array $types):int + { + $count = $this->user->accounts()->accountTypeIn($types)->count(); + + return $count; + } + /** * @param Account $account * @param Account $moveTo @@ -250,13 +263,14 @@ class AccountCrud implements AccountCrudInterface public function store(array $data): Account { $newAccount = $this->storeAccount($data); - if (!is_null($newAccount->id)) { - $this->storeMetadata($newAccount, $data); - } + $this->updateMetadata($newAccount, $data); - if ($data['openingBalance'] != 0) { - $this->storeInitialBalance($newAccount, $data); + if ($this->validOpeningBalanceData($data)) { + $this->updateInitialBalance($newAccount, $data); + + return $newAccount; } + $this->deleteInitialBalance($newAccount); return $newAccount; @@ -295,16 +309,66 @@ class AccountCrud implements AccountCrudInterface return $account; } + /** + * @param Account $account + */ + protected function deleteInitialBalance(Account $account) + { + $journal = $this->openingBalanceTransaction($account); + if (!is_null($journal->id)) { + $journal->delete(); + } + + } + + /** + * @param Account $account + * + * @return TransactionJournal|null + */ + protected function openingBalanceTransaction(Account $account): TransactionJournal + { + $journal = TransactionJournal + ::sortCorrectly() + ->leftJoin('transactions', 'transactions.transaction_journal_id', '=', 'transaction_journals.id') + ->where('transactions.account_id', $account->id) + ->transactionTypes([TransactionType::OPENING_BALANCE]) + ->first(['transaction_journals.*']); + if (is_null($journal)) { + Log::debug('Could not find a opening balance journal, return empty one.'); + + return new TransactionJournal; + } + Log::debug(sprintf('Found opening balance: journal #%d.', $journal->id)); + + return $journal; + } + /** * @param array $data * * @return Account + * @throws FireflyException */ protected function storeAccount(array $data): Account { - $type = config('firefly.accountTypeByIdentifier.' . $data['accountType']); - $accountType = AccountType::whereType($type)->first(); - $newAccount = new Account( + $data['accountType'] = $data['accountType'] ?? 'invalid'; + $type = config('firefly.accountTypeByIdentifier.' . $data['accountType']); + $accountType = AccountType::whereType($type)->first(); + + // verify account type + if (is_null($accountType)) { + throw new FireflyException(sprintf('Account type "%s" is invalid. Cannot create account.', $data['accountType'])); + } + + // account may exist already: + $existingAccount = $this->findByName($data['name'], [$data['accountType']]); + if (!is_null($existingAccount->id)) { + throw new FireflyException(sprintf('There already is an account named "%s" of type "%s".', $data['name'], $data['accountType'])); + } + + // create it: + $newAccount = new Account( [ 'user_id' => $data['user'], 'account_type_id' => $accountType->id, @@ -314,26 +378,14 @@ class AccountCrud implements AccountCrudInterface 'iban' => $data['iban'], ] ); - - if (!$newAccount->isValid()) { - // does the account already exist? - $searchData = [ - 'user_id' => $data['user'], - 'account_type_id' => $accountType->id, - 'virtual_balance' => $data['virtualBalance'], - 'name' => $data['name'], - 'iban' => $data['iban'], - ]; - $existingAccount = Account::firstOrNullEncrypted($searchData); - if (!$existingAccount) { - Log::error('Account create error', $newAccount->getErrors()->toArray()); - - return new Account; - } - $newAccount = $existingAccount; - - } $newAccount->save(); + // verify its creation: + if (is_null($newAccount->id)) { + Log::error( + sprintf('Could not create account "%s" (%d error(s))', $data['name'], $newAccount->getErrors()->count()), $newAccount->getErrors()->toArray() + ); + throw new FireflyException(sprintf('Tried to create account named "%s" but failed. The logs have more details.', $data['name'])); + } return $newAccount; } @@ -362,6 +414,7 @@ class AccountCrud implements AccountCrudInterface 'encrypted' => true, ] ); + Log::debug(sprintf('Created new opening balance journal: #%d', $journal->id)); $firstAccount = $account; $secondAccount = $opposing; @@ -380,27 +433,32 @@ class AccountCrud implements AccountCrudInterface $two = new Transaction(['account_id' => $secondAccount->id, 'transaction_journal_id' => $journal->id, 'amount' => $secondAmount]); $two->save(); // second transaction: to + Log::debug(sprintf('Stored two transactions, #%d and #%d', $one->id, $two->id)); + return $journal; } /** - * @param Account $account - * @param array $data + * @param float $amount + * @param int $user + * @param string $name + * + * @return Account */ - protected function storeMetadata(Account $account, array $data) + protected function storeOpposingAccount(float $amount, int $user, string $name):Account { - foreach ($this->validFields as $field) { - if (isset($data[$field])) { - $metaData = new AccountMeta( - [ - 'account_id' => $account->id, - 'name' => $field, - 'data' => $data[$field], - ] - ); - $metaData->save(); - } - } + $type = $amount < 0 ? 'expense' : 'revenue'; + $opposingData = [ + 'user' => $user, + 'accountType' => $type, + 'name' => $name . ' initial balance', + 'active' => false, + 'iban' => '', + 'virtualBalance' => 0, + ]; + Log::debug('Going to create an opening balance opposing account'); + + return $this->storeAccount($opposingData); } /** @@ -412,21 +470,24 @@ class AccountCrud implements AccountCrudInterface protected function updateInitialBalance(Account $account, array $data): bool { $openingBalance = $this->openingBalanceTransaction($account); - if ($data['openingBalance'] != 0) { - if (!is_null($openingBalance->id)) { - $date = $data['openingBalanceDate']; - $amount = $data['openingBalance']; - - return $this->updateJournal($account, $openingBalance, $date, $amount); - } + // no opening balance journal? create it: + if (is_null($openingBalance->id)) { + Log::debug('No opening balance journal yet, create journal.'); $this->storeInitialBalance($account, $data); return true; } - // else, delete it: - if ($openingBalance) { // opening balance is zero, should we delete it? - $openingBalance->delete(); // delete existing opening balance. + // opening balance data? update it! + if (!is_null($openingBalance->id)) { + $date = $data['openingBalanceDate']; + $amount = $data['openingBalance']; + + Log::debug('Opening balance journal found, update journal.'); + + $this->updateOpeningBalanceJournal($account, $openingBalance, $date, $amount); + + return true; } return true; @@ -440,71 +501,41 @@ class AccountCrud implements AccountCrudInterface protected function updateMetadata(Account $account, array $data) { foreach ($this->validFields as $field) { + /** @var AccountMeta $entry */ $entry = $account->accountMeta()->where('name', $field)->first(); - if (isset($data[$field])) { - // update if new data is present: - if (!is_null($entry)) { - $entry->data = $data[$field]; - $entry->save(); - - continue; - } - $metaData = new AccountMeta( + // if $data has field and $entry is null, create new one: + if (isset($data[$field]) && is_null($entry)) { + Log::debug( + sprintf( + 'Created meta-field "%s":"%s" for account #%d ("%s") ', + $field, $data[$field], $account->id, $account->name + ) + ); + AccountMeta::create( [ 'account_id' => $account->id, 'name' => $field, 'data' => $data[$field], ] ); - $metaData->save(); + } + + // if $data has field and $entry is not null, update $entry: + if (isset($data[$field]) && !is_null($entry)) { + $entry->data = $data[$field]; + $entry->save(); + Log::debug( + sprintf( + 'Updated meta-field "%s":"%s" for account #%d ("%s") ', + $field, $data[$field], $account->id, $account->name + ) + ); } } } - /** - * @param Account $account - * - * @return TransactionJournal|null - */ - private function openingBalanceTransaction(Account $account): TransactionJournal - { - $journal = TransactionJournal - ::sortCorrectly() - ->leftJoin('transactions', 'transactions.transaction_journal_id', '=', 'transaction_journals.id') - ->where('transactions.account_id', $account->id) - ->transactionTypes([TransactionType::OPENING_BALANCE]) - ->first(['transaction_journals.*']); - if (is_null($journal)) { - return new TransactionJournal; - } - - return $journal; - } - - /** - * @param float $amount - * @param int $user - * @param string $name - * - * @return Account - */ - private function storeOpposingAccount(float $amount, int $user, string $name):Account - { - $type = $amount < 0 ? 'expense' : 'revenue'; - $opposingData = [ - 'user' => $user, - 'accountType' => $type, - 'name' => $name . ' initial balance', - 'active' => false, - 'iban' => '', - 'virtualBalance' => 0, - ]; - - return $this->storeAccount($opposingData); - } - /** * @param Account $account * @param TransactionJournal $journal @@ -513,7 +544,7 @@ class AccountCrud implements AccountCrudInterface * * @return bool */ - private function updateJournal(Account $account, TransactionJournal $journal, Carbon $date, float $amount): bool + protected function updateOpeningBalanceJournal(Account $account, TransactionJournal $journal, Carbon $date, float $amount): bool { // update date: $journal->date = $date; @@ -530,8 +561,30 @@ class AccountCrud implements AccountCrudInterface $transaction->save(); } } + Log::debug('Updated opening balance journal.'); return true; } + + /** + * @param array $data + * + * @return bool + */ + protected function validOpeningBalanceData(array $data): bool + { + if (isset($data['openingBalance']) + && isset($data['openingBalanceDate']) + && isset($data['openingBalanceCurrency']) + && bccomp(strval($data['openingBalance']), '0') !== 0 + ) { + Log::debug('Array has valid opening balance data.'); + + return true; + } + Log::debug('Array does not have valid opening balance data.'); + + return false; + } } diff --git a/app/Crud/Account/AccountCrudInterface.php b/app/Crud/Account/AccountCrudInterface.php index 772bf6afcb..f7e995f020 100644 --- a/app/Crud/Account/AccountCrudInterface.php +++ b/app/Crud/Account/AccountCrudInterface.php @@ -24,6 +24,13 @@ use Illuminate\Support\Collection; */ interface AccountCrudInterface { + /** + * @param array $types + * + * @return int + */ + public function count(array $types):int; + /** * @param Account $account * @param Account $moveTo diff --git a/app/Http/Controllers/HomeController.php b/app/Http/Controllers/HomeController.php index 889eda4c9a..51ef56eac4 100644 --- a/app/Http/Controllers/HomeController.php +++ b/app/Http/Controllers/HomeController.php @@ -123,7 +123,7 @@ class HomeController extends Controller { $types = config('firefly.accountTypesByIdentifier.asset'); - $count = $repository->countAccounts($types); + $count = $crud->count($types); if ($count == 0) { return redirect(route('new-user.index')); diff --git a/app/Http/Controllers/NewUserController.php b/app/Http/Controllers/NewUserController.php index 145bc80230..a59a2b38ba 100644 --- a/app/Http/Controllers/NewUserController.php +++ b/app/Http/Controllers/NewUserController.php @@ -37,11 +37,11 @@ class NewUserController extends Controller /** - * @param ARI $repository + * @param AccountCrudInterface $crud * - * @@return View + * @return \Illuminate\Http\RedirectResponse|\Illuminate\Routing\Redirector|View */ - public function index(ARI $repository) + public function index(AccountCrudInterface $crud) { View::share('title', trans('firefly.welcome')); @@ -49,7 +49,7 @@ class NewUserController extends Controller $types = config('firefly.accountTypesByIdentifier.asset'); - $count = $repository->countAccounts($types); + $count = $crud->count($types); if ($count > 0) { return redirect(route('index')); diff --git a/app/Models/Account.php b/app/Models/Account.php index 22b5fadcfa..b27944737c 100644 --- a/app/Models/Account.php +++ b/app/Models/Account.php @@ -236,20 +236,6 @@ class Account extends Model return $value; } - /** - * - * @return string - */ - public function getNameForEditformAttribute(): string - { - $name = $this->name; - if ($this->accountType->type == 'Cash account') { - $name = ''; - } - - return $name; - } - /** * @return HasMany */ diff --git a/app/Repositories/Account/AccountRepository.php b/app/Repositories/Account/AccountRepository.php index fd5e695e25..ea5b4377a8 100644 --- a/app/Repositories/Account/AccountRepository.php +++ b/app/Repositories/Account/AccountRepository.php @@ -51,18 +51,6 @@ class AccountRepository implements AccountRepositoryInterface $this->user = $user; } - /** - * @param array $types - * - * @return int - */ - public function countAccounts(array $types): int - { - $count = $this->user->accounts()->accountTypeIn($types)->count(); - - return $count; - } - /** * This method is almost the same as ::earnedInPeriod, but only works for revenue accounts * instead of the implied asset accounts for ::earnedInPeriod. ::earnedInPeriod will tell you diff --git a/app/Repositories/Account/AccountRepositoryInterface.php b/app/Repositories/Account/AccountRepositoryInterface.php index 52b201fb1d..f4c628972b 100644 --- a/app/Repositories/Account/AccountRepositoryInterface.php +++ b/app/Repositories/Account/AccountRepositoryInterface.php @@ -27,13 +27,6 @@ use Illuminate\Support\Collection; interface AccountRepositoryInterface { - /** - * @param array $types - * - * @return int - */ - public function countAccounts(array $types): int; - /** * This method is almost the same as ::earnedInPeriod, but only works for revenue accounts * instead of the implied asset accounts for ::earnedInPeriod. ::earnedInPeriod will tell you