From 688ca8e37478fb76232b8f7fb8d54daabee89b5d Mon Sep 17 00:00:00 2001 From: James Cole Date: Sun, 4 Jun 2023 06:30:22 +0200 Subject: [PATCH] chore: fix various qodana issues --- .../Autocomplete/AccountController.php | 13 ++++---- .../Controllers/Chart/AccountController.php | 3 +- .../Controllers/Search/AccountController.php | 3 +- .../Autocomplete/AccountController.php | 6 ++-- .../Commands/Upgrade/AccountCurrencies.php | 15 ++------- app/Factory/AccountFactory.php | 5 +-- .../Controllers/Report/AccountController.php | 4 +-- app/Http/Middleware/AcceptHeaders.php | 4 +-- app/Models/Account.php | 7 +++-- app/Models/AccountMeta.php | 5 ++- .../Account/AccountRepository.php | 27 ++++------------ app/Repositories/Account/AccountTasker.php | 2 -- .../Internal/Support/AccountServiceTrait.php | 5 ++- .../Internal/Update/AccountUpdateService.php | 31 ------------------- app/Support/Form/AccountForm.php | 4 +-- app/Support/Search/AccountSearch.php | 10 ++++-- .../Factory/ActionFactory.php | 2 +- app/Validation/AccountValidator.php | 4 +-- .../2020_11_12_070604_changes_for_v550.php | 6 ++-- qodana.yaml | 6 ++++ 20 files changed, 56 insertions(+), 106 deletions(-) diff --git a/app/Api/V1/Controllers/Autocomplete/AccountController.php b/app/Api/V1/Controllers/Autocomplete/AccountController.php index d82110febf..07ec75799c 100644 --- a/app/Api/V1/Controllers/Autocomplete/AccountController.php +++ b/app/Api/V1/Controllers/Autocomplete/AccountController.php @@ -81,8 +81,9 @@ class AccountController extends Controller $query = $data['query']; $date = $data['date'] ?? today(config('app.timezone')); - $return = []; - $result = $this->repository->searchAccount((string)$query, $types, $data['limit']); + $return = []; + $result = $this->repository->searchAccount((string)$query, $types, $data['limit']); + // TODO this code is duplicated in the V2 Autocomplete controller, which means this code is due to be deprecated. $defaultCurrency = app('amount')->getDefaultCurrency(); /** @var Account $account */ @@ -113,12 +114,12 @@ class AccountController extends Controller } // custom order. - $order = [AccountType::ASSET, AccountType::REVENUE, AccountType::EXPENSE]; usort( $return, - function ($a, $b) use ($order) { - $posA = array_search($a['type'], $order, true); - $posB = array_search($b['type'], $order, true); + function ($a, $b) { + $order = [AccountType::ASSET, AccountType::REVENUE, AccountType::EXPENSE]; + $posA = array_search($a['type'], $order, true); + $posB = array_search($b['type'], $order, true); return $posA - $posB; } diff --git a/app/Api/V1/Controllers/Chart/AccountController.php b/app/Api/V1/Controllers/Chart/AccountController.php index 76b1173797..c14ee1843b 100644 --- a/app/Api/V1/Controllers/Chart/AccountController.php +++ b/app/Api/V1/Controllers/Chart/AccountController.php @@ -113,7 +113,7 @@ class AccountController extends Controller if (null === $currency) { $currency = $default; } - $currentSet = [ + $currentSet = [ 'label' => $account->name, 'currency_id' => (string)$currency->id, 'currency_code' => $currency->code, @@ -125,6 +125,7 @@ class AccountController extends Controller 'yAxisID' => 0, // 0, 1, 2 'entries' => [], ]; + // TODO this code is also present in the V2 chart account controller so this method is due to be deprecated. $currentStart = clone $start; $range = app('steam')->balanceInRange($account, $start, clone $end); // 2022-10-11 this method no longer converts to float. diff --git a/app/Api/V1/Controllers/Search/AccountController.php b/app/Api/V1/Controllers/Search/AccountController.php index ac9c138c3c..c4214547db 100644 --- a/app/Api/V1/Controllers/Search/AccountController.php +++ b/app/Api/V1/Controllers/Search/AccountController.php @@ -65,9 +65,8 @@ class AccountController extends Controller * @param Request $request * * @return JsonResponse|Response - * @throws JsonException */ - public function search(Request $request) + public function search(Request $request): JsonResponse|Response { Log::debug('Now in account search()'); $manager = $this->getManager(); diff --git a/app/Api/V2/Controllers/Autocomplete/AccountController.php b/app/Api/V2/Controllers/Autocomplete/AccountController.php index 33232c9754..3c4f6ef2f6 100644 --- a/app/Api/V2/Controllers/Autocomplete/AccountController.php +++ b/app/Api/V2/Controllers/Autocomplete/AccountController.php @@ -55,8 +55,6 @@ class AccountController extends Controller parent::__construct(); $this->middleware( function ($request, $next) { - /** @var User $user */ - $user = auth()->user(); $this->repository = app(AccountRepositoryInterface::class); $this->adminRepository = app(AdminAccountRepositoryInterface::class); @@ -113,10 +111,10 @@ class AccountController extends Controller } // custom order. - $order = [AccountType::ASSET, AccountType::REVENUE, AccountType::EXPENSE]; usort( $return, - function ($a, $b) use ($order) { + function ($a, $b) { + $order = [AccountType::ASSET, AccountType::REVENUE, AccountType::EXPENSE]; $pos_a = array_search($a['type'], $order, true); $pos_b = array_search($b['type'], $order, true); diff --git a/app/Console/Commands/Upgrade/AccountCurrencies.php b/app/Console/Commands/Upgrade/AccountCurrencies.php index 0b4f6ed8bf..33fbbda983 100644 --- a/app/Console/Commands/Upgrade/AccountCurrencies.php +++ b/app/Console/Commands/Upgrade/AccountCurrencies.php @@ -79,17 +79,11 @@ class AccountCurrencies extends Command /** * @return bool - * @throws ContainerExceptionInterface - * @throws NotFoundExceptionInterface */ private function isExecuted(): bool { $configVar = app('fireflyconfig')->get(self::CONFIG_NAME, false); - if (null !== $configVar) { - return (bool)$configVar->data; - } - - return false; + return (bool)$configVar?->data; } /** @@ -122,10 +116,7 @@ class AccountCurrencies extends Command $this->accountRepos->setUser($account->user); $accountCurrency = (int)$this->accountRepos->getMetaValue($account, 'currency_id'); $openingBalance = $this->accountRepos->getOpeningBalance($account); - $obCurrency = 0; - if (null !== $openingBalance) { - $obCurrency = (int)$openingBalance->transaction_currency_id; - } + $obCurrency = (int)$openingBalance?->transaction_currency_id; // both 0? set to default currency: if (0 === $accountCurrency && 0 === $obCurrency) { @@ -158,8 +149,6 @@ class AccountCurrencies extends Command ); $this->line(sprintf('Account #%d ("%s") now has a correct currency for opening balance.', $account->id, $account->name)); $this->count++; - - return; } } diff --git a/app/Factory/AccountFactory.php b/app/Factory/AccountFactory.php index 42b8a309d5..a57c8068aa 100644 --- a/app/Factory/AccountFactory.php +++ b/app/Factory/AccountFactory.php @@ -107,6 +107,7 @@ class AccountFactory Log::debug(sprintf('Now in AccountFactory::find("%s", "%s")', $accountName, $accountType)); $type = AccountType::whereType($accountType)->first(); + /** @var Account|null */ return $this->user->accounts()->where('account_type_id', $type->id)->where('name', $accountName)->first(); } @@ -294,7 +295,7 @@ class AccountFactory * * @throws FireflyException */ - private function storeCreditLiability(Account $account, array $data) + private function storeCreditLiability(Account $account, array $data): void { Log::debug('storeCreditLiability'); $account->refresh(); @@ -366,7 +367,7 @@ class AccountFactory * * @throws FireflyException */ - private function storeOpeningBalance(Account $account, array $data) + private function storeOpeningBalance(Account $account, array $data): void { $accountType = $account->accountType->type; diff --git a/app/Http/Controllers/Report/AccountController.php b/app/Http/Controllers/Report/AccountController.php index 60e240b7e2..1d7a8a5263 100644 --- a/app/Http/Controllers/Report/AccountController.php +++ b/app/Http/Controllers/Report/AccountController.php @@ -44,10 +44,10 @@ class AccountController extends Controller * @param Carbon $start * @param Carbon $end * - * @return mixed|string + * @return string * @throws FireflyException */ - public function general(Collection $accounts, Carbon $start, Carbon $end) + public function general(Collection $accounts, Carbon $start, Carbon $end): string { // chart properties for cache: $cache = new CacheProperties(); diff --git a/app/Http/Middleware/AcceptHeaders.php b/app/Http/Middleware/AcceptHeaders.php index 2e4b9b76ff..41cec7e828 100644 --- a/app/Http/Middleware/AcceptHeaders.php +++ b/app/Http/Middleware/AcceptHeaders.php @@ -42,7 +42,7 @@ class AcceptHeaders * @return Response * @throws BadHttpHeaderException */ - public function handle($request, $next): mixed + public function handle(Request $request, callable $next): mixed { $method = $request->getMethod(); $accepts = ['application/x-www-form-urlencoded', 'application/json', 'application/vnd.api+json', 'application/octet-stream', '*/*']; @@ -67,7 +67,7 @@ class AcceptHeaders } // throw bad request if trace id is not a UUID - $uuid = $request->header('X-Trace-Id', null); + $uuid = $request->header('X-Trace-Id'); if (is_string($uuid) && '' !== trim($uuid) && (preg_match('/^[a-f\d]{8}(-[a-f\d]{4}){4}[a-f\d]{8}$/i', trim($uuid)) !== 1)) { throw new BadRequestHttpException('Bad X-Trace-Id header.'); } diff --git a/app/Models/Account.php b/app/Models/Account.php index 9ce824914e..b661a1db33 100644 --- a/app/Models/Account.php +++ b/app/Models/Account.php @@ -34,6 +34,7 @@ use Illuminate\Database\Eloquent\Model; use Illuminate\Database\Eloquent\Relations\BelongsTo; use Illuminate\Database\Eloquent\Relations\HasMany; use Illuminate\Database\Eloquent\Relations\MorphMany; +use Illuminate\Database\Eloquent\Relations\MorphToMany; use Illuminate\Database\Eloquent\SoftDeletes; use Illuminate\Database\Query\Builder; use Symfony\Component\HttpKernel\Exception\NotFoundHttpException; @@ -222,9 +223,9 @@ class Account extends Model } /** - * Get all of the tags for the post. + * Get all the tags for the post. */ - public function objectGroups() + public function objectGroups(): MorphToMany { return $this->morphToMany(ObjectGroup::class, 'object_groupable'); } @@ -257,7 +258,7 @@ class Account extends Model * */ - public function setVirtualBalanceAttribute($value): void + public function setVirtualBalanceAttribute(mixed $value): void { $value = (string)$value; if ('' === $value) { diff --git a/app/Models/AccountMeta.php b/app/Models/AccountMeta.php index ea636961c6..11239faac4 100644 --- a/app/Models/AccountMeta.php +++ b/app/Models/AccountMeta.php @@ -80,11 +80,10 @@ class AccountMeta extends Model * @param mixed $value * * @return mixed - * @throws JsonException */ - public function getDataAttribute($value) + public function getDataAttribute($value): string { - return json_decode($value, true, 512, JSON_THROW_ON_ERROR); + return (string)json_decode($value, true); } /** diff --git a/app/Repositories/Account/AccountRepository.php b/app/Repositories/Account/AccountRepository.php index 92ad6f9aa5..cdcb140bca 100644 --- a/app/Repositories/Account/AccountRepository.php +++ b/app/Repositories/Account/AccountRepository.php @@ -358,11 +358,7 @@ class AccountRepository implements AccountRepositoryInterface ->where('transactions.account_id', $account->id) ->transactionTypes([TransactionType::LIABILITY_CREDIT]) ->first(['transaction_journals.*']); - if (null === $journal) { - return null; - } - - return $journal->transactionGroup; + return $journal?->transactionGroup; } /** @@ -433,13 +429,7 @@ class AccountRepository implements AccountRepositoryInterface */ public function getNoteText(Account $account): ?string { - $note = $account->notes()->first(); - - if (null === $note) { - return null; - } - - return $note->text; + return $account->notes()->first()?->text; } /** @@ -488,15 +478,10 @@ class AccountRepository implements AccountRepositoryInterface */ public function getOpeningBalanceDate(Account $account): ?string { - $journal = TransactionJournal::leftJoin('transactions', 'transactions.transaction_journal_id', '=', 'transaction_journals.id') - ->where('transactions.account_id', $account->id) - ->transactionTypes([TransactionType::OPENING_BALANCE, TransactionType::LIABILITY_CREDIT]) - ->first(['transaction_journals.*']); - if (null === $journal) { - return null; - } - - return $journal->date->format('Y-m-d H:i:s'); + return TransactionJournal::leftJoin('transactions', 'transactions.transaction_journal_id', '=', 'transaction_journals.id') + ->where('transactions.account_id', $account->id) + ->transactionTypes([TransactionType::OPENING_BALANCE, TransactionType::LIABILITY_CREDIT]) + ->first(['transaction_journals.*'])?->date->format('Y-m-d H:i:s'); } /** diff --git a/app/Repositories/Account/AccountTasker.php b/app/Repositories/Account/AccountTasker.php index 99d1614754..aa47442631 100644 --- a/app/Repositories/Account/AccountTasker.php +++ b/app/Repositories/Account/AccountTasker.php @@ -90,8 +90,6 @@ class AccountTasker implements AccountTaskerInterface 'currency_symbol' => $currency->symbol, 'currency_name' => $currency->name, 'currency_decimal_places' => $currency->decimal_places, - 'start_balance' => '0', - 'end_balance' => '0', ]; // get first journal date: diff --git a/app/Services/Internal/Support/AccountServiceTrait.php b/app/Services/Internal/Support/AccountServiceTrait.php index 8e7dc1b9f6..67d9a19c74 100644 --- a/app/Services/Internal/Support/AccountServiceTrait.php +++ b/app/Services/Internal/Support/AccountServiceTrait.php @@ -151,7 +151,7 @@ trait AccountServiceTrait if (is_bool($data[$field]) && false === $data[$field]) { $data[$field] = 0; } - if (is_bool($data[$field]) && true === $data[$field]) { + if (true === $data[$field]) { $data[$field] = 1; } if ($data[$field] instanceof Carbon) { @@ -171,15 +171,14 @@ trait AccountServiceTrait */ public function updateNote(Account $account, string $note): bool { + $dbNote = $account->notes()->first(); if ('' === $note) { - $dbNote = $account->notes()->first(); if (null !== $dbNote) { $dbNote->delete(); } return true; } - $dbNote = $account->notes()->first(); if (null === $dbNote) { $dbNote = new Note(); $dbNote->noteable()->associate($account); diff --git a/app/Services/Internal/Update/AccountUpdateService.php b/app/Services/Internal/Update/AccountUpdateService.php index c2bb6276ac..0cddf3d427 100644 --- a/app/Services/Internal/Update/AccountUpdateService.php +++ b/app/Services/Internal/Update/AccountUpdateService.php @@ -254,37 +254,6 @@ class AccountUpdateService return $account; } - /** - * @param Account $account - * @param array $data - * - * @throws FireflyException - * @deprecated In Firefly III v5.8.0 and onwards, credit transactions for liabilities are no longer created. - */ - private function updateCreditLiability(Account $account, array $data): void - { - $type = $account->accountType; - $valid = config('firefly.valid_liabilities'); - if (in_array($type->type, $valid, true)) { - $direction = array_key_exists('liability_direction', $data) ? $data['liability_direction'] : 'empty'; - // check if is submitted as empty, that makes it valid: - if ($this->validOBData($data) && !$this->isEmptyOBData($data)) { - $openingBalance = $data['opening_balance']; - $openingBalanceDate = $data['opening_balance_date']; - if ('credit' === $direction) { - $this->updateCreditTransaction($account, $direction, $openingBalance, $openingBalanceDate); - } - } - - if (!$this->validOBData($data) && $this->isEmptyOBData($data)) { - $this->deleteCreditTransaction($account); - } - if ($this->validOBData($data) && !$this->isEmptyOBData($data) && 'credit' !== $direction) { - $this->deleteCreditTransaction($account); - } - } - } - /** * @param Account $account * @param array $data diff --git a/app/Support/Form/AccountForm.php b/app/Support/Form/AccountForm.php index 6225e7b6ae..d221cdcf0a 100644 --- a/app/Support/Form/AccountForm.php +++ b/app/Support/Form/AccountForm.php @@ -51,7 +51,7 @@ class AccountForm * * @return string */ - public function activeDepositDestinations(string $name, $value = null, array $options = null): string + public function activeDepositDestinations(string $name, mixed $value = null, array $options = null): string { $types = [AccountType::MORTGAGE, AccountType::DEBT, AccountType::CREDITCARD, AccountType::LOAN, AccountType::REVENUE,]; $repository = $this->getAccountRepository(); @@ -72,7 +72,7 @@ class AccountForm * * @return string */ - public function activeWithdrawalDestinations(string $name, $value = null, array $options = null): string + public function activeWithdrawalDestinations(string $name, mixed $value = null, array $options = null): string { $types = [AccountType::MORTGAGE, AccountType::DEBT, AccountType::CREDITCARD, AccountType::LOAN, AccountType::EXPENSE,]; $repository = $this->getAccountRepository(); diff --git a/app/Support/Search/AccountSearch.php b/app/Support/Search/AccountSearch.php index 1a8cd8e002..885ec5887a 100644 --- a/app/Support/Search/AccountSearch.php +++ b/app/Support/Search/AccountSearch.php @@ -25,6 +25,7 @@ declare(strict_types=1); namespace FireflyIII\Support\Search; use FireflyIII\User; +use Illuminate\Contracts\Auth\Authenticatable; use Illuminate\Database\Eloquent\Builder; use Illuminate\Support\Collection; @@ -132,10 +133,13 @@ class AccountSearch implements GenericSearchInterface } /** - * @param User $user + * @param User|Authenticatable|null $user + * @return void */ - public function setUser(User $user): void + public function setUser(User|Authenticatable|null $user): void { - $this->user = $user; + if (null !== $user) { + $this->user = $user; + } } } diff --git a/app/TransactionRules/Factory/ActionFactory.php b/app/TransactionRules/Factory/ActionFactory.php index cfc04ecb86..8263b61275 100644 --- a/app/TransactionRules/Factory/ActionFactory.php +++ b/app/TransactionRules/Factory/ActionFactory.php @@ -37,7 +37,7 @@ use Illuminate\Support\Facades\Log; class ActionFactory { /** @var array array of action types */ - protected static $actionTypes = []; + protected static array $actionTypes = []; /** * This method returns the actual implementation (TransactionRules/Actions/[object]) for a given diff --git a/app/Validation/AccountValidator.php b/app/Validation/AccountValidator.php index 15eeacdee8..9dcee5f834 100644 --- a/app/Validation/AccountValidator.php +++ b/app/Validation/AccountValidator.php @@ -91,7 +91,7 @@ class AccountValidator Log::debug('AccountValidator source is set to NULL'); } if (null !== $account) { - Log::debug(sprintf('AccountValidator source is set to #%d: "%s" (%s)', $account->id, $account->name, $account?->accountType->type)); + Log::debug(sprintf('AccountValidator source is set to #%d: "%s" (%s)', $account->id, $account->name, $account->accountType->type)); } $this->source = $account; } @@ -105,7 +105,7 @@ class AccountValidator Log::debug('AccountValidator destination is set to NULL'); } if (null !== $account) { - Log::debug(sprintf('AccountValidator destination is set to #%d: "%s" (%s)', $account->id, $account->name, $account?->accountType->type)); + Log::debug(sprintf('AccountValidator destination is set to #%d: "%s" (%s)', $account->id, $account->name, $account->accountType->type)); } $this->destination = $account; } diff --git a/database/migrations/2020_11_12_070604_changes_for_v550.php b/database/migrations/2020_11_12_070604_changes_for_v550.php index 39d4702e7f..e72dc37b3d 100644 --- a/database/migrations/2020_11_12_070604_changes_for_v550.php +++ b/database/migrations/2020_11_12_070604_changes_for_v550.php @@ -226,9 +226,9 @@ class ChangesForV550 extends Migration $table->string('title', 255)->index(); $table->string('secret', 32)->index(); $table->boolean('active')->default(true); - $table->unsignedSmallInteger('trigger', false); - $table->unsignedSmallInteger('response', false); - $table->unsignedSmallInteger('delivery', false); + $table->unsignedSmallInteger('trigger'); + $table->unsignedSmallInteger('response'); + $table->unsignedSmallInteger('delivery'); $table->string('url', 1024); $table->foreign('user_id')->references('id')->on('users')->onDelete('cascade'); $table->unique(['user_id', 'title']); diff --git a/qodana.yaml b/qodana.yaml index e4b198f5be..3b502c2d9d 100644 --- a/qodana.yaml +++ b/qodana.yaml @@ -7,3 +7,9 @@ exclude: paths: - .ci - _ide_helper.php + - public/v1/css/jquery-ui/** + - public/v1/fonts/** + - public/v1/lib/** + - public/v1/js/lib/** + - public/v3/** + - public/v3-local/**