Some minor refactoring.

This commit is contained in:
James Cole
2016-10-08 10:02:33 +02:00
parent d43936155c
commit 24f62b8fce
7 changed files with 170 additions and 143 deletions

View File

@@ -15,6 +15,7 @@ namespace FireflyIII\Crud\Account;
use Carbon\Carbon; use Carbon\Carbon;
use DB; use DB;
use FireflyIII\Exceptions\FireflyException;
use FireflyIII\Models\Account; use FireflyIII\Models\Account;
use FireflyIII\Models\AccountMeta; use FireflyIII\Models\AccountMeta;
use FireflyIII\Models\AccountType; use FireflyIII\Models\AccountType;
@@ -51,6 +52,18 @@ class AccountCrud implements AccountCrudInterface
$this->user = $user; $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 $account
* @param Account $moveTo * @param Account $moveTo
@@ -250,13 +263,14 @@ class AccountCrud implements AccountCrudInterface
public function store(array $data): Account public function store(array $data): Account
{ {
$newAccount = $this->storeAccount($data); $newAccount = $this->storeAccount($data);
if (!is_null($newAccount->id)) { $this->updateMetadata($newAccount, $data);
$this->storeMetadata($newAccount, $data);
}
if ($data['openingBalance'] != 0) { if ($this->validOpeningBalanceData($data)) {
$this->storeInitialBalance($newAccount, $data); $this->updateInitialBalance($newAccount, $data);
return $newAccount;
} }
$this->deleteInitialBalance($newAccount);
return $newAccount; return $newAccount;
@@ -295,16 +309,66 @@ class AccountCrud implements AccountCrudInterface
return $account; 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 * @param array $data
* *
* @return Account * @return Account
* @throws FireflyException
*/ */
protected function storeAccount(array $data): Account protected function storeAccount(array $data): Account
{ {
$type = config('firefly.accountTypeByIdentifier.' . $data['accountType']); $data['accountType'] = $data['accountType'] ?? 'invalid';
$accountType = AccountType::whereType($type)->first(); $type = config('firefly.accountTypeByIdentifier.' . $data['accountType']);
$newAccount = new Account( $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'], 'user_id' => $data['user'],
'account_type_id' => $accountType->id, 'account_type_id' => $accountType->id,
@@ -314,26 +378,14 @@ class AccountCrud implements AccountCrudInterface
'iban' => $data['iban'], '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(); $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; return $newAccount;
} }
@@ -362,6 +414,7 @@ class AccountCrud implements AccountCrudInterface
'encrypted' => true, 'encrypted' => true,
] ]
); );
Log::debug(sprintf('Created new opening balance journal: #%d', $journal->id));
$firstAccount = $account; $firstAccount = $account;
$secondAccount = $opposing; $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 = new Transaction(['account_id' => $secondAccount->id, 'transaction_journal_id' => $journal->id, 'amount' => $secondAmount]);
$two->save(); // second transaction: to $two->save(); // second transaction: to
Log::debug(sprintf('Stored two transactions, #%d and #%d', $one->id, $two->id));
return $journal; return $journal;
} }
/** /**
* @param Account $account * @param float $amount
* @param array $data * @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) { $type = $amount < 0 ? 'expense' : 'revenue';
if (isset($data[$field])) { $opposingData = [
$metaData = new AccountMeta( 'user' => $user,
[ 'accountType' => $type,
'account_id' => $account->id, 'name' => $name . ' initial balance',
'name' => $field, 'active' => false,
'data' => $data[$field], 'iban' => '',
] 'virtualBalance' => 0,
); ];
$metaData->save(); 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 protected function updateInitialBalance(Account $account, array $data): bool
{ {
$openingBalance = $this->openingBalanceTransaction($account); $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); $this->storeInitialBalance($account, $data);
return true; return true;
} }
// else, delete it: // opening balance data? update it!
if ($openingBalance) { // opening balance is zero, should we delete it? if (!is_null($openingBalance->id)) {
$openingBalance->delete(); // delete existing opening balance. $date = $data['openingBalanceDate'];
$amount = $data['openingBalance'];
Log::debug('Opening balance journal found, update journal.');
$this->updateOpeningBalanceJournal($account, $openingBalance, $date, $amount);
return true;
} }
return true; return true;
@@ -440,71 +501,41 @@ class AccountCrud implements AccountCrudInterface
protected function updateMetadata(Account $account, array $data) protected function updateMetadata(Account $account, array $data)
{ {
foreach ($this->validFields as $field) { foreach ($this->validFields as $field) {
/** @var AccountMeta $entry */
$entry = $account->accountMeta()->where('name', $field)->first(); $entry = $account->accountMeta()->where('name', $field)->first();
if (isset($data[$field])) { // if $data has field and $entry is null, create new one:
// update if new data is present: if (isset($data[$field]) && is_null($entry)) {
if (!is_null($entry)) { Log::debug(
$entry->data = $data[$field]; sprintf(
$entry->save(); 'Created meta-field "%s":"%s" for account #%d ("%s") ',
$field, $data[$field], $account->id, $account->name
continue; )
} );
$metaData = new AccountMeta( AccountMeta::create(
[ [
'account_id' => $account->id, 'account_id' => $account->id,
'name' => $field, 'name' => $field,
'data' => $data[$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 Account $account
* @param TransactionJournal $journal * @param TransactionJournal $journal
@@ -513,7 +544,7 @@ class AccountCrud implements AccountCrudInterface
* *
* @return bool * @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: // update date:
$journal->date = $date; $journal->date = $date;
@@ -530,8 +561,30 @@ class AccountCrud implements AccountCrudInterface
$transaction->save(); $transaction->save();
} }
} }
Log::debug('Updated opening balance journal.');
return true; 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;
}
} }

View File

@@ -24,6 +24,13 @@ use Illuminate\Support\Collection;
*/ */
interface AccountCrudInterface interface AccountCrudInterface
{ {
/**
* @param array $types
*
* @return int
*/
public function count(array $types):int;
/** /**
* @param Account $account * @param Account $account
* @param Account $moveTo * @param Account $moveTo

View File

@@ -123,7 +123,7 @@ class HomeController extends Controller
{ {
$types = config('firefly.accountTypesByIdentifier.asset'); $types = config('firefly.accountTypesByIdentifier.asset');
$count = $repository->countAccounts($types); $count = $crud->count($types);
if ($count == 0) { if ($count == 0) {
return redirect(route('new-user.index')); return redirect(route('new-user.index'));

View File

@@ -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')); View::share('title', trans('firefly.welcome'));
@@ -49,7 +49,7 @@ class NewUserController extends Controller
$types = config('firefly.accountTypesByIdentifier.asset'); $types = config('firefly.accountTypesByIdentifier.asset');
$count = $repository->countAccounts($types); $count = $crud->count($types);
if ($count > 0) { if ($count > 0) {
return redirect(route('index')); return redirect(route('index'));

View File

@@ -236,20 +236,6 @@ class Account extends Model
return $value; return $value;
} }
/**
*
* @return string
*/
public function getNameForEditformAttribute(): string
{
$name = $this->name;
if ($this->accountType->type == 'Cash account') {
$name = '';
}
return $name;
}
/** /**
* @return HasMany * @return HasMany
*/ */

View File

@@ -51,18 +51,6 @@ class AccountRepository implements AccountRepositoryInterface
$this->user = $user; $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 * 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 * instead of the implied asset accounts for ::earnedInPeriod. ::earnedInPeriod will tell you

View File

@@ -27,13 +27,6 @@ use Illuminate\Support\Collection;
interface AccountRepositoryInterface 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 * 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 * instead of the implied asset accounts for ::earnedInPeriod. ::earnedInPeriod will tell you