Improve code quality for Export directory.

This commit is contained in:
James Cole 2018-07-06 19:06:08 +02:00
parent 57345113b5
commit 8692590600
18 changed files with 170 additions and 120 deletions

View File

@ -20,10 +20,14 @@
* along with Firefly III. If not, see <http://www.gnu.org/licenses/>.
*/
/** @noinspection PhpDynamicAsStaticMethodCallInspection */
/** @noinspection PhpUndefinedMethodInspection */
declare(strict_types=1);
namespace FireflyIII\Factory;
use FireflyIII\Exceptions\FireflyException;
use FireflyIII\Models\Account;
use FireflyIII\Models\AccountType;
use FireflyIII\Services\Internal\Support\AccountServiceTrait;
@ -44,56 +48,63 @@ class AccountFactory
* @param array $data
*
* @return Account
* @throws FireflyException
* @SuppressWarnings(PHPMD.CyclomaticComplexity)
* @SuppressWarnings(PHPMD.ExcessiveMethodLength)
*/
public function create(array $data): Account
{
$type = $this->getAccountType($data['account_type_id'], $data['accountType']);
$type = $this->getAccountType($data['account_type_id'], $data['accountType']);
if (null === $type) {
throw new FireflyException(
sprintf('AccountFactory::create() was unable to find account type #%d ("%s").', $data['account_type_id'], $data['accountType'])
);
}
$data['iban'] = $this->filterIban($data['iban']);
// account may exist already:
$existingAccount = $this->find($data['name'], $type->type);
if (null !== $existingAccount) {
return $existingAccount;
$return = $this->find($data['name'], $type->type);
if (null === $return) {
// create it:
$databaseData
= [
'user_id' => $this->user->id,
'account_type_id' => $type->id,
'name' => $data['name'],
'virtual_balance' => $data['virtualBalance'] ?? '0',
'active' => true === $data['active'],
'iban' => $data['iban'],
];
// remove virtual balance when not an asset account:
if ($type->type !== AccountType::ASSET) {
$databaseData['virtual_balance'] = '0';
}
// fix virtual balance when it's empty
if ('' === $databaseData['virtual_balance']) {
$databaseData['virtual_balance'] = '0';
}
$return = Account::create($databaseData);
$this->updateMetaData($return, $data);
if ($type->type === AccountType::ASSET) {
if ($this->validIBData($data)) {
$this->updateIB($return, $data);
}
if (!$this->validIBData($data)) {
$this->deleteIB($return);
}
}
$this->updateNote($return, $data['notes'] ?? '');
}
// create it:
$databaseData
= [
'user_id' => $this->user->id,
'account_type_id' => $type->id,
'name' => $data['name'],
'virtual_balance' => $data['virtualBalance'] ?? '0',
'active' => true === $data['active'],
'iban' => $data['iban'],
];
// remove virtual balance when not an asset account:
if ($type->type !== AccountType::ASSET) {
$databaseData['virtual_balance'] = '0';
}
// fix virtual balance when it's empty
if ($databaseData['virtual_balance'] === '') {
$databaseData['virtual_balance'] = '0';
}
$newAccount = Account::create($databaseData);
$this->updateMetaData($newAccount, $data);
if ($this->validIBData($data) && $type->type === AccountType::ASSET) {
$this->updateIB($newAccount, $data);
}
if (!$this->validIBData($data) && $type->type === AccountType::ASSET) {
$this->deleteIB($newAccount);
}
// update note:
if (isset($data['notes'])) {
$this->updateNote($newAccount, $data['notes']);
}
return $newAccount;
return $return;
}
/**
@ -106,15 +117,16 @@ class AccountFactory
{
$type = AccountType::whereType($accountType)->first();
$accounts = $this->user->accounts()->where('account_type_id', $type->id)->get(['accounts.*']);
$return = null;
/** @var Account $object */
foreach ($accounts as $object) {
if ($object->name === $accountName) {
return $object;
$return = $object;
break;
}
}
return null;
return $return;
}
/**
@ -122,30 +134,35 @@ class AccountFactory
* @param string $accountType
*
* @return Account
* @throws FireflyException
*/
public function findOrCreate(string $accountName, string $accountType): Account
{
$type = AccountType::whereType($accountType)->first();
$accounts = $this->user->accounts()->where('account_type_id', $type->id)->get(['accounts.*']);
$return = null;
/** @var Account $object */
foreach ($accounts as $object) {
if ($object->name === $accountName) {
return $object;
$return = $object;
break;
}
}
if (null === $return) {
$return = $this->create(
[
'user_id' => $this->user->id,
'name' => $accountName,
'account_type_id' => $type->id,
'accountType' => null,
'virtualBalance' => '0',
'iban' => null,
'active' => true,
]
);
}
return $this->create(
[
'user_id' => $this->user->id,
'name' => $accountName,
'account_type_id' => $type->id,
'accountType' => null,
'virtualBalance' => '0',
'iban' => null,
'active' => true,
]
);
return $return;
}
/**
@ -161,18 +178,23 @@ class AccountFactory
* @param null|string $accountType
*
* @return AccountType|null
* @SuppressWarnings(PHPMD.CyclomaticComplexity)
*/
protected function getAccountType(?int $accountTypeId, ?string $accountType): ?AccountType
{
$accountTypeId = (int)$accountTypeId;
$result = null;
if ($accountTypeId > 0) {
return AccountType::find($accountTypeId);
$result = AccountType::find($accountTypeId);
}
$type = config('firefly.accountTypeByIdentifier.' . (string)$accountType);
$result = AccountType::whereType($type)->first();
if (null === $result && null !== $accountType) {
// try as full name:
$result = AccountType::whereType($accountType)->first();
if (null === $result) {
/** @var string $type */
$type = (string)config('firefly.accountTypeByIdentifier.' . (string)$accountType);
$result = AccountType::whereType($type)->first();
if (null === $result && null !== $accountType) {
// try as full name:
$result = AccountType::whereType($accountType)->first();
}
}
return $result;

View File

@ -81,25 +81,19 @@ class BillFactory
{
$billId = (int)$billId;
$billName = (string)$billName;
$bill = null;
// first find by ID:
if ($billId > 0) {
/** @var Bill $bill */
$bill = $this->user->bills()->find($billId);
if (null !== $bill) {
return $bill;
}
}
// then find by name:
if (\strlen($billName) > 0) {
if (null === $bill && \strlen($billName) > 0) {
$bill = $this->findByName($billName);
if (null !== $bill) {
return $bill;
}
}
return null;
return $bill;
}
@ -112,22 +106,24 @@ class BillFactory
{
/** @var Collection $collection */
$collection = $this->user->bills()->get();
$return = null;
/** @var Bill $bill */
foreach ($collection as $bill) {
Log::debug(sprintf('"%s" vs. "%s"', $bill->name, $name));
if ($bill->name === $name) {
return $bill;
$return = $bill;
break;
}
}
Log::debug(sprintf('Bill::Find by name returns NULL based on "%s"', $name));
Log::debug(sprintf('Bill::find("%s") by name returns null? %s', $name, var_export($return, true)));
return null;
return $return;
}
/**
* @param User $user
*/
public function setUser(User $user)
public function setUser(User $user): void
{
$this->user = $user;
}

View File

@ -1,5 +1,4 @@
<?php
/**
* BudgetFactory.php
* Copyright (c) 2018 thegrumpydictator@gmail.com
@ -19,7 +18,7 @@
* You should have received a copy of the GNU General Public License
* along with Firefly III. If not, see <http://www.gnu.org/licenses/>.
*/
/** @noinspection MultipleReturnStatementsInspection */
declare(strict_types=1);
@ -30,7 +29,7 @@ use FireflyIII\User;
use Illuminate\Support\Collection;
/**
* Class BudgetFactory
* Class BudgetFactory.
*/
class BudgetFactory
{
@ -43,13 +42,14 @@ class BudgetFactory
* @param null|string $budgetName
*
* @return Budget|null
* @SuppressWarnings(PHPMD.CyclomaticComplexity)
*/
public function find(?int $budgetId, ?string $budgetName): ?Budget
{
$budgetId = (int)$budgetId;
$budgetName = (string)$budgetName;
if (\strlen($budgetName) === 0 && $budgetId === 0) {
if (0 === $budgetId && '' === $budgetName) {
return null;
}
@ -62,7 +62,7 @@ class BudgetFactory
}
}
if (\strlen($budgetName) > 0) {
if ('' !== $budgetName) {
$budget = $this->findByName($budgetName);
if (null !== $budget) {
return $budget;
@ -94,7 +94,7 @@ class BudgetFactory
/**
* @param User $user
*/
public function setUser(User $user)
public function setUser(User $user): void
{
$this->user = $user;
}

View File

@ -1,5 +1,4 @@
<?php
/**
* CategoryFactory.php
* Copyright (c) 2018 thegrumpydictator@gmail.com
@ -19,7 +18,7 @@
* You should have received a copy of the GNU General Public License
* along with Firefly III. If not, see <http://www.gnu.org/licenses/>.
*/
/** @noinspection MultipleReturnStatementsInspection */
declare(strict_types=1);
namespace FireflyIII\Factory;
@ -64,6 +63,7 @@ class CategoryFactory
* @param null|string $categoryName
*
* @return Category|null
* @SuppressWarnings(PHPMD.CyclomaticComplexity)
*/
public function findOrCreate(?int $categoryId, ?string $categoryName): ?Category
{
@ -72,7 +72,7 @@ class CategoryFactory
Log::debug(sprintf('Going to find category with ID %d and name "%s"', $categoryId, $categoryName));
if ('' === $categoryName && $categoryId === 0) {
if ('' === $categoryName && 0 === $categoryId) {
return null;
}
// first by ID:
@ -104,7 +104,7 @@ class CategoryFactory
/**
* @param User $user
*/
public function setUser(User $user)
public function setUser(User $user): void
{
$this->user = $user;
}

View File

@ -1,5 +1,4 @@
<?php
/**
* PiggyBankEventFactory.php
* Copyright (c) 2018 thegrumpydictator@gmail.com
@ -19,7 +18,7 @@
* You should have received a copy of the GNU General Public License
* along with Firefly III. If not, see <http://www.gnu.org/licenses/>.
*/
/** @noinspection MultipleReturnStatementsInspection */
declare(strict_types=1);
namespace FireflyIII\Factory;
@ -43,6 +42,7 @@ class PiggyBankEventFactory
* @param PiggyBank|null $piggyBank
*
* @return PiggyBankEvent|null
* @SuppressWarnings(PHPMD.CyclomaticComplexity)
*/
public function create(TransactionJournal $journal, ?PiggyBank $piggyBank): ?PiggyBankEvent
{
@ -51,7 +51,6 @@ class PiggyBankEventFactory
return null;
}
// is a transfer?
if (!(TransactionType::TRANSFER === $journal->transactionType->type)) {
Log::info(sprintf('Will not connect %s #%d to a piggy bank.', $journal->transactionType->type, $journal->id));
@ -62,7 +61,6 @@ class PiggyBankEventFactory
$piggyRepos = app(PiggyBankRepositoryInterface::class);
$piggyRepos->setUser($journal->user);
// repetition exists?
$repetition = $piggyRepos->getRepetition($piggyBank);
if (null === $repetition) {
Log::error(sprintf('No piggy bank repetition on %s!', $journal->date->format('Y-m-d')));
@ -70,7 +68,6 @@ class PiggyBankEventFactory
return null;
}
// get the amount
$amount = $piggyRepos->getExactAmount($piggyBank, $repetition, $journal);
if (0 === bccomp($amount, '0')) {
Log::debug('Amount is zero, will not create event.');
@ -78,10 +75,8 @@ class PiggyBankEventFactory
return null;
}
// update amount
$piggyRepos->addAmountToRepetition($repetition, $amount);
$event = $piggyRepos->createEventWithJournal($piggyBank, $amount, $journal);
Log::debug(sprintf('Created piggy bank event #%d', $event->id));
return $event;

View File

@ -1,5 +1,4 @@
<?php
/**
* PiggyBankFactory.php
* Copyright (c) 2018 thegrumpydictator@gmail.com
@ -19,7 +18,7 @@
* You should have received a copy of the GNU General Public License
* along with Firefly III. If not, see <http://www.gnu.org/licenses/>.
*/
/** @noinspection MultipleReturnStatementsInspection */
declare(strict_types=1);
namespace FireflyIII\Factory;
@ -41,12 +40,13 @@ class PiggyBankFactory
* @param null|string $piggyBankName
*
* @return PiggyBank|null
* @SuppressWarnings(PHPMD.CyclomaticComplexity)
*/
public function find(?int $piggyBankId, ?string $piggyBankName): ?PiggyBank
{
$piggyBankId = (int)$piggyBankId;
$piggyBankName = (string)$piggyBankName;
if (\strlen($piggyBankName) === 0 && $piggyBankId === 0) {
if ('' === $piggyBankName && 0 === $piggyBankId) {
return null;
}
// first find by ID:
@ -92,7 +92,7 @@ class PiggyBankFactory
/**
* @param User $user
*/
public function setUser(User $user)
public function setUser(User $user): void
{
$this->user = $user;

View File

@ -18,12 +18,14 @@
* You should have received a copy of the GNU General Public License
* along with Firefly III. If not, see <http://www.gnu.org/licenses/>.
*/
/** @noinspection MultipleReturnStatementsInspection */
declare(strict_types=1);
namespace FireflyIII\Factory;
use Carbon\Carbon;
use FireflyIII\Exceptions\FireflyException;
use FireflyIII\Models\Recurrence;
use FireflyIII\Services\Internal\Support\RecurringTransactionTrait;
@ -56,6 +58,9 @@ class RecurrenceFactory
return null;
}
/** @var Carbon $firstDate */
$firstDate = $data['recurrence']['first_date'];
$repetitions = (int)$data['recurrence']['repetitions'];
$recurrence = new Recurrence(
[
@ -63,7 +68,7 @@ class RecurrenceFactory
'transaction_type_id' => $type->id,
'title' => $data['recurrence']['title'],
'description' => $data['recurrence']['description'],
'first_date' => $data['recurrence']['first_date']->format('Y-m-d'),
'first_date' => $firstDate->format('Y-m-d'),
'repeat_until' => $repetitions > 0 ? null : $data['recurrence']['repeat_until'],
'latest_date' => null,
'repetitions' => $data['recurrence']['repetitions'],

View File

@ -1,5 +1,4 @@
<?php
/**
* TagFactory.php
* Copyright (c) 2018 thegrumpydictator@gmail.com
@ -19,6 +18,7 @@
* You should have received a copy of the GNU General Public License
* along with Firefly III. If not, see <http://www.gnu.org/licenses/>.
*/
/** @noinspection MultipleReturnStatementsInspection */
declare(strict_types=1);

View File

@ -1,5 +1,4 @@
<?php
/**
* TransactionCurrencyFactory.php
* Copyright (c) 2018 thegrumpydictator@gmail.com
@ -20,6 +19,10 @@
* along with Firefly III. If not, see <http://www.gnu.org/licenses/>.
*/
/** @noinspection PhpDynamicAsStaticMethodCallInspection */
/** @noinspection PhpUndefinedMethodInspection */
/** @noinspection MultipleReturnStatementsInspection */
declare(strict_types=1);
namespace FireflyIII\Factory;
@ -63,13 +66,14 @@ class TransactionCurrencyFactory
* @param null|string $currencyCode
*
* @return TransactionCurrency|null
* @SuppressWarnings(PHPMD.CyclomaticComplexity)
*/
public function find(?int $currencyId, ?string $currencyCode): ?TransactionCurrency
{
$currencyCode = (string)$currencyCode;
$currencyId = (int)$currencyId;
if ('' === $currencyCode && $currencyId === 0) {
if ('' === $currencyCode && 0 === $currencyId) {
Log::warning('Cannot find anything on empty currency code and empty currency ID!');
return null;

View File

@ -50,6 +50,7 @@ class TransactionFactory
*
* @return Transaction
* @throws FireflyException
* @SuppressWarnings(PHPMD.CyclomaticComplexity)
*/
public function create(array $data): ?Transaction
{
@ -89,6 +90,9 @@ class TransactionFactory
*
* @return Collection
* @throws FireflyException
* @SuppressWarnings(PHPMD.CyclomaticComplexity)
* @SuppressWarnings(PHPMD.NPathComplexity)
* @SuppressWarnings(PHPMD.ExcessiveMethodLength)
*/
public function createPair(TransactionJournal $journal, array $data): Collection
{
@ -97,29 +101,34 @@ class TransactionFactory
$currency = $this->findCurrency($data['currency_id'], $data['currency_code']);
$description = $journal->description === $data['description'] ? null : $data['description'];
// type of source account depends on journal type:
$sourceType = $this->accountType($journal, 'source');
Log::debug(sprintf('Expect source account to be of type %s', $sourceType));
$sourceAccount = $this->findAccount($sourceType, $data['source_id'], $data['source_name']);
// same for destination account:
// type of source account and destination account depends on journal type:
$sourceType = $this->accountType($journal, 'source');
$destinationType = $this->accountType($journal, 'destination');
if (null === $sourceType || null === $destinationType) {
throw new FireflyException('Could not determine source or destination type.');
}
Log::debug(sprintf('Expect source account to be of type %s', $sourceType));
Log::debug(sprintf('Expect source destination to be of type %s', $destinationType));
// find source and destination account:
$sourceAccount = $this->findAccount($sourceType, $data['source_id'], $data['source_name']);
$destinationAccount = $this->findAccount($destinationType, $data['destination_id'], $data['destination_name']);
if (null === $sourceAccount || null === $destinationAccount) {
throw new FireflyException('Could not determine source or destination account.');
}
Log::debug(sprintf('Source type is "%s", destination type is "%s"', $sourceAccount->accountType->type, $destinationAccount->accountType->type));
// throw big fat error when source type === dest type
if ($sourceAccount->accountType->type === $destinationAccount->accountType->type
&& ($journal->transactionType->type !== TransactionType::TRANSFER
&& $journal->transactionType->type !== TransactionType::RECONCILIATION)
) {
// throw big fat error when source type === dest type and it's not a transfer or reconciliation.
if ($sourceAccount->accountType->type === $destinationAccount->accountType->type && $journal->transactionType->type !== TransactionType::TRANSFER) {
throw new FireflyException(sprintf('Source and destination account cannot be both of the type "%s"', $destinationAccount->accountType->type));
}
if ($sourceAccount->accountType->type !== AccountType::ASSET && $destinationAccount->accountType->type !== AccountType::ASSET) {
throw new FireflyException('At least one of the accounts must be an asset account.');
}
// first make a "negative" (source) transaction based on the data in the array.
$source = $this->create(
[
'description' => $description,
@ -132,8 +141,7 @@ class TransactionFactory
'identifier' => $data['identifier'],
]
);
// then make a "positive" transaction based on the data in the array.
$dest = $this->create(
$dest = $this->create(
[
'description' => $description,
'amount' => app('steam')->positive((string)$data['amount']),
@ -145,6 +153,9 @@ class TransactionFactory
'identifier' => $data['identifier'],
]
);
if (null === $source || null === $dest) {
throw new FireflyException('Could not create transactions.');
}
// set foreign currency
$foreign = $this->findCurrency($data['foreign_currency_id'], $data['foreign_currency_code']);
@ -178,7 +189,7 @@ class TransactionFactory
/**
* @param User $user
*/
public function setUser(User $user)
public function setUser(User $user): void
{
$this->user = $user;
}

View File

@ -37,14 +37,18 @@ use Log;
class TransactionJournalFactory
{
use JournalServiceTrait, TransactionTypeTrait;
/** @var User */
/** @var User The user */
private $user;
/**
* Store a new transaction journal.
*
* @param array $data
*
* @return TransactionJournal
* @throws FireflyException
* @SuppressWarnings(PHPMD.CyclomaticComplexity)
* @SuppressWarnings(PHPMD.ExcessiveMethodLength)
*/
public function create(array $data): TransactionJournal
{
@ -124,6 +128,8 @@ class TransactionJournalFactory
}
/**
* Link a piggy bank to this journal.
*
* @param TransactionJournal $journal
* @param array $data
*/

View File

@ -1,5 +1,4 @@
<?php
/**
* TransactionJournalMetaFactory.php
* Copyright (c) 2018 thegrumpydictator@gmail.com
@ -19,6 +18,7 @@
* You should have received a copy of the GNU General Public License
* along with Firefly III. If not, see <http://www.gnu.org/licenses/>.
*/
/** @noinspection MultipleReturnStatementsInspection */
declare(strict_types=1);
@ -38,6 +38,8 @@ class TransactionJournalMetaFactory
* @param array $data
*
* @return TransactionJournalMeta|null
* @SuppressWarnings(PHPMD.CyclomaticComplexity)
* @SuppressWarnings(PHPMD.ExcessiveMethodLength)
*/
public function updateOrCreate(array $data): ?TransactionJournalMeta
{
@ -57,7 +59,7 @@ class TransactionJournalMetaFactory
if ($data['data'] instanceof Carbon) {
$value = $data['data']->toW3cString();
}
if ((string)$value === '') {
if ('' === (string)$value) {
// don't store blank strings.
if (null !== $entry) {
try {

View File

@ -19,6 +19,7 @@
* You should have received a copy of the GNU General Public License
* along with Firefly III. If not, see <http://www.gnu.org/licenses/>.
*/
/** @noinspection PhpUndefinedMethodInspection */
declare(strict_types=1);

View File

@ -27,7 +27,10 @@ use Illuminate\Database\Eloquent\Relations\BelongsTo;
/**
* Class AccountMeta.
*
* @property string $data
* @property string $name
* @property int $account_id
*/
class AccountMeta extends Model
{

View File

@ -29,6 +29,8 @@ use Illuminate\Database\Eloquent\Relations\HasMany;
* Class AccountType.
*
* @property string $type
* @method whereType(string $type)
* @property int $id
*
*/
class AccountType extends Model

View File

@ -31,6 +31,7 @@ use Symfony\Component\HttpKernel\Exception\NotFoundHttpException;
*
* @property User $user
* @property string $key
* @property int $user_id
*/
class ExportJob extends Model
{

View File

@ -34,6 +34,7 @@ use Illuminate\Database\Eloquent\SoftDeletes;
* @property Carbon $updated_at
* @property string $text
* @property string $title
* @property int $noteable_id
*/
class Note extends Model
{

View File

@ -30,6 +30,7 @@ use Illuminate\Database\Eloquent\Model;
* @property PiggyBank $piggyBank
* @property int $transaction_journal_id
* @property int $piggy_bank_id
* @property int $id
*/
class PiggyBankEvent extends Model
{