Optimise code.

This commit is contained in:
James Cole
2020-10-24 17:27:36 +02:00
parent b3f1737495
commit 3979e12043
21 changed files with 199 additions and 187 deletions

View File

@@ -86,7 +86,6 @@ class FixPiggies extends Command
$event->save();
$this->line(sprintf('Piggy bank #%d was referenced by an invalid event. This has been fixed.', $event->piggy_bank_id));
$this->count++;
continue;
}
}
if (0 === $this->count) {

View File

@@ -33,11 +33,9 @@ class CronController
use CronRunner;
/**
* @param string $token
*
* @return string
*/
public function cron(string $token): string
public function cron(): string
{
$results = [];
$results[] = $this->runRecurring();

View File

@@ -69,7 +69,7 @@ class ConvertController extends Controller
function ($request, $next) {
$this->repository = app(JournalRepositoryInterface::class);
$this->accountRepository = app(AccountRepositoryInterface::class);
app('view')->share('title', (string) trans('firefly.transactions'));
app('view')->share('title', (string)trans('firefly.transactions'));
app('view')->share('mainTitleIcon', 'fa-exchange');
return $next($request);
@@ -103,7 +103,7 @@ class ConvertController extends Controller
$groupTitle = $group->title ?? $first->description;
$groupArray = $transformer->transformObject($group);
$subTitle = (string) trans('firefly.convert_to_' . $destinationType->type, ['description' => $groupTitle]);
$subTitle = (string)trans('firefly.convert_to_' . $destinationType->type, ['description' => $groupTitle]);
$subTitleIcon = 'fa-exchange';
// get a list of asset accounts and liabilities and stuff, in various combinations:
@@ -119,7 +119,7 @@ class ConvertController extends Controller
if ($sourceType->type === $destinationType->type) { // cannot convert to its own type.
Log::debug('This is already a transaction of the expected type..');
session()->flash('info', (string) trans('firefly.convert_is_already_type_' . $destinationType->type));
session()->flash('info', (string)trans('firefly.convert_is_already_type_' . $destinationType->type));
return redirect(route('transactions.show', [$group->id]));
}
@@ -174,9 +174,8 @@ class ConvertController extends Controller
// correct transfers:
$group->refresh();
$this->correctTransfer($group);
session()->flash('success', (string) trans('firefly.converted_to_' . $destinationType->type));
session()->flash('success', (string)trans('firefly.converted_to_' . $destinationType->type));
event(new UpdatedTransactionGroup($group));
return redirect(route('transactions.show', [$group->id]));
@@ -203,10 +202,10 @@ class ConvertController extends Controller
$destinationName = $data['destination_name'][$journal->id] ?? null;
// double check its not an empty string.
$sourceId = '' === $sourceId || null === $sourceId ? null : (int) $sourceId;
$sourceName = '' === $sourceName ? null : (string) $sourceName;
$destinationId = '' === $destinationId || null === $destinationId ? null : (int) $destinationId;
$destinationName = '' === $destinationName ? null : (string) $destinationName;
$sourceId = '' === $sourceId || null === $sourceId ? null : (int)$sourceId;
$sourceName = '' === $sourceName ? null : (string)$sourceName;
$destinationId = '' === $destinationId || null === $destinationId ? null : (int)$destinationId;
$destinationName = '' === $destinationName ? null : (string)$destinationName;
$validSource = $validator->validateSource($sourceId, $sourceName, null);
$validDestination = $validator->validateDestination($destinationId, $destinationName, null);
@@ -236,13 +235,6 @@ class ConvertController extends Controller
return $journal;
}
/**
* @param TransactionGroup $group
*/
private function correctTransfer(TransactionGroup $group): void
{
}
/**
* @return array
* @throws Exception
@@ -260,12 +252,12 @@ class ConvertController extends Controller
foreach ($accountList as $account) {
$balance = app('steam')->balance($account, today());
$currency = $accountRepository->getAccountCurrency($account) ?? $defaultCurrency;
$role = (string) $accountRepository->getMetaValue($account, 'account_role');
$role = (string)$accountRepository->getMetaValue($account, 'account_role');
if ('' === $role) {
$role = 'no_account_type'; // @codeCoverageIgnore
}
$key = (string) trans('firefly.opt_group_' . $role);
$key = (string)trans('firefly.opt_group_' . $role);
$grouped[$key][$account->id] = $account->name . ' (' . app('amount')->formatAnything($currency, $balance, false) . ')';
}
@@ -290,7 +282,7 @@ class ConvertController extends Controller
$balance = app('steam')->balance($account, today());
$currency = $accountRepository->getAccountCurrency($account) ?? $defaultCurrency;
$role = 'l_' . $account->accountType->type;
$key = (string) trans('firefly.opt_group_' . $role);
$key = (string)trans('firefly.opt_group_' . $role);
$grouped[$key][$account->id] = $account->name . ' (' . app('amount')->formatAnything($currency, $balance, false) . ')';
}
@@ -312,7 +304,7 @@ class ConvertController extends Controller
// group accounts:
/** @var Account $account */
foreach ($accountList as $account) {
$role = (string) $accountRepository->getMetaValue($account, 'account_role');
$role = (string)$accountRepository->getMetaValue($account, 'account_role');
$name = $account->name;
if ('' === $role) {
$role = 'no_account_type'; // @codeCoverageIgnore
@@ -332,7 +324,7 @@ class ConvertController extends Controller
$role = 'revenue_account'; // @codeCoverageIgnore
}
$key = (string) trans('firefly.opt_group_' . $role);
$key = (string)trans('firefly.opt_group_' . $role);
$grouped[$key][$account->id] = $name;
}
@@ -345,16 +337,17 @@ class ConvertController extends Controller
private function getValidWithdrawalDests(): array
{
// make repositories
/** @var AccountRepositoryInterface $repository */
$repository = app(AccountRepositoryInterface::class);
$liabilityTypes = [AccountType::MORTGAGE, AccountType::DEBT, AccountType::CREDITCARD, AccountType::LOAN];
$accountList = $repository
->getActiveAccountsByType([AccountType::EXPENSE, AccountType::CASH, AccountType::LOAN, AccountType::DEBT, AccountType::MORTGAGE]);
$grouped = [];
/** @var AccountRepositoryInterface $accountRepository */
$accountRepository = app(AccountRepositoryInterface::class);
$liabilityTypes = [AccountType::MORTGAGE, AccountType::DEBT, AccountType::CREDITCARD, AccountType::LOAN];
$accountList = $accountRepository->getActiveAccountsByType(
[AccountType::EXPENSE, AccountType::CASH, AccountType::LOAN, AccountType::DEBT, AccountType::MORTGAGE]
);
$grouped = [];
// group accounts:
/** @var Account $account */
foreach ($accountList as $account) {
$role = (string) $repository->getMetaValue($account, 'account_role');
$role = (string)$accountRepository->getMetaValue($account, 'account_role');
$name = $account->name;
if ('' === $role) {
$role = 'no_account_type'; // @codeCoverageIgnore
@@ -374,7 +367,7 @@ class ConvertController extends Controller
$role = 'expense_account'; // @codeCoverageIgnore
}
$key = (string) trans('firefly.opt_group_' . $role);
$key = (string)trans('firefly.opt_group_' . $role);
$grouped[$key][$account->id] = $name;
}

View File

@@ -41,8 +41,7 @@ class IndexController extends Controller
{
use PeriodOverview;
/** @var JournalRepositoryInterface */
private $repository;
private JournalRepositoryInterface $repository;
/**
* IndexController constructor.
@@ -132,16 +131,12 @@ class IndexController extends Controller
*/
public function indexAll(Request $request, string $objectType)
{
/** @var JournalRepositoryInterface $repository */
$repository = app(JournalRepositoryInterface::class);
$subTitleIcon = config('firefly.transactionIconsByType.' . $objectType);
$types = config('firefly.transactionTypesByType.' . $objectType);
$page = (int) $request->get('page');
$pageSize = (int) app('preferences')->get('listPageSize', 50)->data;
$path = route('transactions.index.all', [$objectType]);
$first = $repository->firstNull();
$first = $this->repository->firstNull();
$start = null === $first ? new Carbon : $first->date;
$last = $this->repository->getLast();
$end = $last ? $last->date : today(config('app.timezone'));

View File

@@ -47,8 +47,7 @@ use Log;
*/
class MassController extends Controller
{
/** @var JournalRepositoryInterface Journals and transactions overview */
private $repository;
private JournalRepositoryInterface $repository;
/**
* MassController constructor.
@@ -131,16 +130,16 @@ class MassController extends Controller
{
$subTitle = (string) trans('firefly.mass_edit_journals');
/** @var AccountRepositoryInterface $repository */
$repository = app(AccountRepositoryInterface::class);
/** @var AccountRepositoryInterface $accountRepository */
$accountRepository = app(AccountRepositoryInterface::class);
// valid withdrawal sources:
$array = array_keys(config(sprintf('firefly.source_dests.%s', TransactionType::WITHDRAWAL)));
$withdrawalSources = $repository->getAccountsByType($array);
$withdrawalSources = $accountRepository->getAccountsByType($array);
// valid deposit destinations:
$array = config(sprintf('firefly.source_dests.%s.%s', TransactionType::DEPOSIT, AccountType::REVENUE));
$depositDestinations = $repository->getAccountsByType($array);
$depositDestinations = $accountRepository->getAccountsByType($array);
/** @var BudgetRepositoryInterface $budgetRepository */
$budgetRepository = app(BudgetRepositoryInterface::class);

View File

@@ -331,8 +331,6 @@ class RecurrenceFormRequest extends FormRequest
$message = (string)trans('validation.generic_invalid_destination');
$validator->errors()->add('destination_id', $message);
$validator->errors()->add('withdrawal_destination_id', $message);
return;
}
}
}

View File

@@ -212,7 +212,6 @@ class ReportFormRequest extends FormRequest
$tag = $repository->findNull((int)$tagTag);
if (null !== $tag) {
$collection->push($tag);
continue;
}
}
}

View File

@@ -27,12 +27,10 @@ namespace FireflyIII\Jobs;
use Carbon\Carbon;
use FireflyIII\Events\RequestedReportOnJournals;
use FireflyIII\Events\StoredTransactionGroup;
use FireflyIII\Factory\PiggyBankEventFactory;
use FireflyIII\Models\Recurrence;
use FireflyIII\Models\RecurrenceRepetition;
use FireflyIII\Models\RecurrenceTransaction;
use FireflyIII\Models\TransactionGroup;
use FireflyIII\Models\TransactionJournal;
use FireflyIII\Repositories\Journal\JournalRepositoryInterface;
use FireflyIII\Repositories\Recurring\RecurringRepositoryInterface;
use FireflyIII\Repositories\TransactionGroup\TransactionGroupRepositoryInterface;
@@ -52,22 +50,14 @@ class CreateRecurringTransactions implements ShouldQueue
{
use Dispatchable, InteractsWithQueue, Queueable, SerializesModels;
/** @var int Transaction groups created */
public int $created;
/** @var int Number of recurrences actually fired */
public int $executed;
/** @var int Number of recurrences submitted */
public int $submitted;
/** @var Carbon The current date */
private Carbon $date;
/** @var bool Force the transaction to be created no matter what. */
private bool $force;
/** @var TransactionGroupRepositoryInterface */
private $groupRepository;
/** @var JournalRepositoryInterface Journal repository */
private $journalRepository;
/** @var RecurringRepositoryInterface Recurring transactions repository. */
private $repository;
public int $created;
public int $executed;
public int $submitted;
private Carbon $date;
private bool $force;
private TransactionGroupRepositoryInterface $groupRepository;
private JournalRepositoryInterface $journalRepository;
private RecurringRepositoryInterface $repository;
/**
* Create a new job instance.
@@ -121,9 +111,9 @@ class CreateRecurringTransactions implements ShouldQueue
app('preferences')->setForUser($recurrence->user, 'lastActivity', microtime());
Log::debug(sprintf('Now at recurrence #%d', $recurrence->id));
$created = $this->handleRepetitions($recurrence);
$createdReps = $this->handleRepetitions($recurrence);
Log::debug(sprintf('Done with recurrence #%d', $recurrence->id));
$result[$recurrence->user_id] = $result[$recurrence->user_id]->merge($created);
$result[$recurrence->user_id] = $result[$recurrence->user_id]->merge($createdReps);
$this->executed++;
}
@@ -218,11 +208,11 @@ class CreateRecurringTransactions implements ShouldQueue
$return = [];
/** @var RecurrenceTransaction $transaction */
foreach ($transactions as $index => $transaction) {
$single = [
$single = [
'type' => strtolower($recurrence->transactionType->type),
'date' => $date,
'user' => $recurrence->user_id,
'currency_id' => (int) $transaction->transaction_currency_id,
'currency_id' => (int)$transaction->transaction_currency_id,
'currency_code' => null,
'description' => $recurrence->recurrenceTransactions()->first()->description,
'amount' => $transaction->amount,
@@ -239,16 +229,16 @@ class CreateRecurringTransactions implements ShouldQueue
'foreign_amount' => $transaction->foreign_amount,
'reconciled' => false,
'identifier' => $index,
'recurrence_id' => (int) $recurrence->id,
'recurrence_id' => (int)$recurrence->id,
'order' => $index,
'notes' => (string) trans('firefly.created_from_recurrence', ['id' => $recurrence->id, 'title' => $recurrence->title]),
'notes' => (string)trans('firefly.created_from_recurrence', ['id' => $recurrence->id, 'title' => $recurrence->title]),
'tags' => $this->repository->getTags($transaction),
'piggy_bank_id' => $this->repository->getPiggyBank($transaction),
'piggy_bank_name' => null,
'bill_id' => null,
'bill_name' => null,
'recurrence_total' => $total,
'recurrence_count' => $count,
'recurrence_total' => $total,
'recurrence_count' => $count,
];
$return[] = $single;
}

View File

@@ -57,9 +57,9 @@ class NewIPAddressWarningMail extends Mailable
// time
$this->time = now()->formatLocalized((string)trans('config.date_time'));
$this->host = '';
$host = gethostbyaddr($this->ipAddress);
if($host !== $this->ipAddress) {
$this->host = $host;
$hostName = gethostbyaddr($this->ipAddress);
if($hostName !== $this->ipAddress) {
$this->host = $hostName;
}
return $this->view('emails.new-ip-html')->text('emails.new-ip-text')

View File

@@ -44,17 +44,7 @@ class BillUpdateService
{
use BillServiceTrait, CreatesObjectGroups;
protected $user;
/**
* Constructor.
*/
public function __construct()
{
if ('testing' === config('app.env')) {
Log::warning(sprintf('%s should not be instantiated in the TEST environment!', get_class($this)));
}
}
protected User $user;
/**
* @param Bill $bill
@@ -223,7 +213,6 @@ class BillUpdateService
Log::debug(sprintf('Updated rule trigger #%d from value "%s" to value "%s"', $trigger->id, $oldValue, $newValue));
$trigger->trigger_value = $newValue;
$trigger->save();
continue;
}
}
}
@@ -248,18 +237,14 @@ class BillUpdateService
private function updateOrder(Bill $bill, int $oldOrder, int $newOrder): void
{
if ($newOrder > $oldOrder) {
/** @var User $user */
$user = $this->user;
$user->bills()->where('order', '<=', $newOrder)->where('order', '>', $oldOrder)
$this->user->bills()->where('order', '<=', $newOrder)->where('order', '>', $oldOrder)
->where('bills.id', '!=', $bill->id)
->update(['order' => DB::raw('bills.order-1')]);
$bill->order = $newOrder;
$bill->save();
}
if ($newOrder < $oldOrder) {
/** @var User $user */
$user = $this->user;
$user->bills()->where('order', '>=', $newOrder)->where('order', '<', $oldOrder)
$this->user->bills()->where('order', '>=', $newOrder)->where('order', '<', $oldOrder)
->where('bills.id', '!=', $bill->id)
->update(['order' => DB::raw('bills.order+1')]);
$bill->order = $newOrder;

View File

@@ -54,32 +54,17 @@ class JournalUpdateService
{
use JournalServiceTrait;
/** @var BillRepositoryInterface */
private $billRepository;
/** @var CurrencyRepositoryInterface */
private $currencyRepository;
/** @var array The data to update the journal with. */
private $data;
/** @var Account The destination account. */
private $destinationAccount;
/** @var Transaction */
private $destinationTransaction;
/** @var array All meta values that are dates. */
private $metaDate;
/** @var array All meta values that are strings. */
private $metaString;
/** @var Account Source account of the journal */
private $sourceAccount;
/** @var Transaction Source transaction of the journal. */
private $sourceTransaction;
/** @var TransactionGroup The parent group. */
private $transactionGroup;
/** @var TransactionJournal The journal to update. */
private $transactionJournal;
/** @var Account If new account info is submitted, this array will hold the valid destination. */
private $validDestination;
/** @var Account If new account info is submitted, this array will hold the valid source. */
private $validSource;
private BillRepositoryInterface $billRepository;
private CurrencyRepositoryInterface $currencyRepository;
private array $data;
private Account $destinationAccount;
private Transaction $destinationTransaction;
private array $metaDate;
private array $metaString;
private Account $sourceAccount;
private Transaction $sourceTransaction;
private TransactionGroup $transactionGroup;
private TransactionJournal $transactionJournal;
/**
* JournalUpdateService constructor.
@@ -253,7 +238,7 @@ class JournalUpdateService
}
$destInfo = [
'id' => (int) ($this->data['destination_id'] ?? null),
'id' => (int)($this->data['destination_id'] ?? null),
'name' => $this->data['destination_name'] ?? null,
'iban' => $this->data['destination_iban'] ?? null,
'number' => $this->data['destination_number'] ?? null,
@@ -287,7 +272,7 @@ class JournalUpdateService
}
$sourceInfo = [
'id' => (int) ($this->data['source_id'] ?? null),
'id' => (int)($this->data['source_id'] ?? null),
'name' => $this->data['source_name'] ?? null,
'iban' => $this->data['source_iban'] ?? null,
'number' => $this->data['source_number'] ?? null,
@@ -479,8 +464,8 @@ class JournalUpdateService
)
&& TransactionType::WITHDRAWAL === $type
) {
$billId = (int) ($this->data['bill_id'] ?? 0);
$billName = (string) ($this->data['bill_name'] ?? '');
$billId = (int)($this->data['bill_id'] ?? 0);
$billName = (string)($this->data['bill_name'] ?? '');
$bill = $this->billRepository->findBill($billId, $billName);
$this->transactionJournal->bill_id = null === $bill ? null : $bill->id;
Log::debug('Updated bill ID');
@@ -551,15 +536,15 @@ class JournalUpdateService
*/
private function updateField(string $fieldName): void
{
if (array_key_exists($fieldName, $this->data) && '' !== (string) $this->data[$fieldName]) {
if (array_key_exists($fieldName, $this->data) && '' !== (string)$this->data[$fieldName]) {
$value = $this->data[$fieldName];
if ('date' === $fieldName) {
if($value instanceof Carbon) {
if ($value instanceof Carbon) {
// update timezone.
$value->setTimezone(config('app.timezone'));
}
if(!($value instanceof Carbon)) {
if (!($value instanceof Carbon)) {
$value = new Carbon($value);
}
// do some parsing.
@@ -667,7 +652,7 @@ class JournalUpdateService
foreach ($this->metaDate as $field) {
if ($this->hasFields([$field])) {
try {
$value = '' === (string) $this->data[$field] ? null : new Carbon($this->data[$field]);
$value = '' === (string)$this->data[$field] ? null : new Carbon($this->data[$field]);
} catch (Exception $e) {
Log::debug(sprintf('%s is not a valid date value: %s', $this->data[$field], $e->getMessage()));
@@ -713,7 +698,7 @@ class JournalUpdateService
{
// update notes.
if ($this->hasFields(['notes'])) {
$notes = '' === (string) $this->data['notes'] ? null : $this->data['notes'];
$notes = '' === (string)$this->data['notes'] ? null : $this->data['notes'];
$this->storeNotes($this->transactionJournal, $notes);
}
}

View File

@@ -106,8 +106,6 @@ class OAuthKeys
}
if (!self::keysInDatabase() && self::hasKeyFiles()) {
self::storeKeysInDB();
return;
}
}

View File

@@ -75,7 +75,7 @@ class AccountTransformer extends AbstractTransformer
[$currencyId, $currencyCode, $currencySymbol, $decimalPlaces] = $this->getCurrency($account);
[$creditCardType, $monthlyPaymentDate] = $this->getCCInfo($account, $accountRole, $accountType);
[$openingBalance, $openingBalanceDate] = $this->getOpeningBalance($account, $accountType, $decimalPlaces);
[$openingBalance, $openingBalanceDate] = $this->getOpeningBalance($account, $accountType);
[$interest, $interestPeriod] = $this->getInterest($account, $accountType);
$openingBalance = number_format((float) $openingBalance, $decimalPlaces, '.', '');
@@ -235,7 +235,7 @@ class AccountTransformer extends AbstractTransformer
*
* TODO refactor call to getOpeningBalanceAmount / Date because its extra queries.
*/
private function getOpeningBalance(Account $account, string $accountType, int $decimalPlaces): array
private function getOpeningBalance(Account $account, string $accountType): array
{
$openingBalance = null;
$openingBalanceDate = null;

View File

@@ -29,26 +29,19 @@ use FireflyIII\Repositories\Account\AccountRepositoryInterface;
use FireflyIII\User;
/**
* I have no idea what made me do this. I'll reverse it some day.
*
* Trait AccountValidatorProperties
*/
trait AccountValidatorProperties
{
/** @var bool */
public $createMode;
/** @var string */
public $destError;
/** @var Account */
public $destination;
/** @var Account */
public $source;
/** @var string */
public $sourceError;
/** @var AccountRepositoryInterface */
private $accountRepository;
/** @var array */
private $combinations;
/** @var string */
private $transactionType;
/** @var User */
private $user;
public bool $createMode;
public string $destError;
public Account $destination;
public Account $source;
public string $sourceError;
private AccountRepositoryInterface $accountRepository;
private array $combinations;
private string $transactionType;
private User $user;
}

View File

@@ -96,8 +96,6 @@ trait TransactionValidation
if (false === $validDestination) {
$validator->errors()->add(sprintf('transactions.%d.destination_id', $index), $accountValidator->destError);
$validator->errors()->add(sprintf('transactions.%d.destination_name', $index), $accountValidator->destError);
return;
}
}
@@ -257,8 +255,6 @@ trait TransactionValidation
$unique = array_unique($types);
if (count($unique) > 1) {
$validator->errors()->add('transactions.0.type', (string) trans('validation.transaction_types_equal'));
return;
}
}