From 0c7b652a7098e86ad669c48f39db09fbb17968d8 Mon Sep 17 00:00:00 2001 From: James Cole Date: Sat, 28 Jul 2018 10:45:16 +0200 Subject: [PATCH] Last code optimization before release. --- app/Rules/UniqueIban.php | 2 + app/Services/Github/Request/UpdateRequest.php | 11 +- app/Services/IP/IpifyOrg.php | 11 +- app/Services/Internal/File/EncryptService.php | 10 +- .../Support/RecurringTransactionTrait.php | 2 +- app/Services/Password/PwndVerifierV2.php | 8 +- .../Spectre/Request/SpectreRequest.php | 17 +- app/Support/Amount.php | 14 +- app/Support/Binder/AccountList.php | 16 +- app/Support/Binder/BudgetList.php | 8 +- app/Support/Binder/CategoryList.php | 8 +- app/Support/Binder/Date.php | 41 ++-- app/Support/Binder/ImportProvider.php | 2 + app/Support/Binder/JournalList.php | 7 +- app/Support/Binder/SimpleJournalList.php | 11 +- app/Support/Binder/TagList.php | 7 +- app/Support/ExpandedForm.php | 192 ++++++------------ .../File/ConfigureMappingHandler.php | 1 + .../File/ConfigureRolesHandler.php | 1 + .../Spectre/ChooseAccountsHandler.php | 1 + .../Import/Placeholder/ImportTransaction.php | 1 + .../Routine/File/AssetAccountMapper.php | 1 + .../Routine/File/MappedValuesValidator.php | 1 + .../Import/Routine/File/MappingConverger.php | 62 +++--- .../Routine/File/OpposingAccountMapper.php | 1 + .../Spectre/StageImportDataHandler.php | 1 + app/Support/Navigation.php | 1 + app/Support/Search/Search.php | 1 + app/Support/Twig/Extension/Transaction.php | 2 +- app/TransactionRules/Actions/ClearBudget.php | 4 - .../Actions/ClearCategory.php | 4 - app/TransactionRules/Actions/ClearNotes.php | 4 - .../Actions/RemoveAllTags.php | 4 - app/TransactionRules/Actions/SetCategory.php | 4 + .../Actions/SetSourceAccount.php | 9 +- .../Factory/TriggerFactory.php | 4 +- app/TransactionRules/Processor.php | 17 +- .../Triggers/AbstractTrigger.php | 4 +- .../Triggers/NotesContain.php | 2 +- app/TransactionRules/Triggers/NotesEmpty.php | 2 +- config/twigbridge.php | 2 +- resources/views/form/multiRadio.twig | 17 -- 42 files changed, 217 insertions(+), 301 deletions(-) delete mode 100644 resources/views/form/multiRadio.twig diff --git a/app/Rules/UniqueIban.php b/app/Rules/UniqueIban.php index 292746f40d..28922f3cf2 100644 --- a/app/Rules/UniqueIban.php +++ b/app/Rules/UniqueIban.php @@ -26,6 +26,7 @@ namespace FireflyIII\Rules; use FireflyIII\Models\Account; use FireflyIII\Models\AccountType; use Illuminate\Contracts\Validation\Rule; +use Illuminate\Support\Collection; use Log; /** @@ -116,6 +117,7 @@ class UniqueIban implements Rule if (null !== $this->account) { $query->where('accounts.id', '!=', $this->account->id); } + /** @var Collection $result */ $result = $query->get(['accounts.*']); foreach ($result as $account) { if ($account->iban === $iban) { diff --git a/app/Services/Github/Request/UpdateRequest.php b/app/Services/Github/Request/UpdateRequest.php index d208e8c06c..44a0bc17b7 100644 --- a/app/Services/Github/Request/UpdateRequest.php +++ b/app/Services/Github/Request/UpdateRequest.php @@ -29,6 +29,7 @@ use FireflyIII\Services\Github\Object\Release; use GuzzleHttp\Client; use GuzzleHttp\Exception\GuzzleException; use Log; +use RuntimeException; use SimpleXMLElement; /** @@ -55,10 +56,14 @@ class UpdateRequest implements GithubRequest } if (200 !== $res->getStatusCode()) { - throw new FireflyException(sprintf('Returned code %d, error: %s', $res->getStatusCode(), $res->getBody()->getContents())); + throw new FireflyException(sprintf('Returned code %d.', $res->getStatusCode())); + } + try { + $releaseXml = new SimpleXMLElement($res->getBody()->getContents(), LIBXML_NOCDATA); + } catch (RunTimeException $e) { + Log::error(sprintf('Could not get body from github updat result: %s', $e->getMessage())); + $releaseXml = new SimpleXMLElement(''); } - - $releaseXml = new SimpleXMLElement($res->getBody()->getContents(), LIBXML_NOCDATA); //fetch the products for each category if (isset($releaseXml->entry)) { diff --git a/app/Services/IP/IpifyOrg.php b/app/Services/IP/IpifyOrg.php index 2349f65dce..a171b3f8eb 100644 --- a/app/Services/IP/IpifyOrg.php +++ b/app/Services/IP/IpifyOrg.php @@ -27,6 +27,7 @@ use Exception; use GuzzleHttp\Client; use GuzzleHttp\Exception\GuzzleException; use Log; +use RunTimeException; /** * Class IpifyOrg @@ -51,11 +52,17 @@ class IpifyOrg implements IPRetrievalInterface return null; } if (200 !== $res->getStatusCode()) { - Log::warning(sprintf('Could not retrieve external IP: %d %s', $res->getStatusCode(), $res->getBody()->getContents())); + Log::warning(sprintf('Could not retrieve external IP: %d', $res->getStatusCode())); return null; } + try { + $body = (string)$res->getBody()->getContents(); + } catch (RunTimeException $e) { + Log::error(sprintf('Could not get body from ipify.org result: %s', $e->getMessage())); + $body = null; + } - return (string)$res->getBody()->getContents(); + return $body; } } diff --git a/app/Services/Internal/File/EncryptService.php b/app/Services/Internal/File/EncryptService.php index 4426e77100..6b97fca04f 100644 --- a/app/Services/Internal/File/EncryptService.php +++ b/app/Services/Internal/File/EncryptService.php @@ -25,6 +25,8 @@ namespace FireflyIII\Services\Internal\File; use Crypt; use FireflyIII\Exceptions\FireflyException; +use Illuminate\Contracts\Encryption\EncryptException; +use Log; /** * Class EncryptService @@ -43,7 +45,13 @@ class EncryptService throw new FireflyException(sprintf('File "%s" does not seem to exist.', $file)); } $content = file_get_contents($file); - $content = Crypt::encrypt($content); + try { + $content = Crypt::encrypt($content); + } catch (EncryptException $e) { + $message = sprintf('Could not encrypt file: %s', $e->getMessage()); + Log::error($message); + throw new FireflyException($message); + } $newName = sprintf('%s.upload', $key); $path = storage_path('upload') . '/' . $newName; diff --git a/app/Services/Internal/Support/RecurringTransactionTrait.php b/app/Services/Internal/Support/RecurringTransactionTrait.php index b9c03074d9..03e2487fe9 100644 --- a/app/Services/Internal/Support/RecurringTransactionTrait.php +++ b/app/Services/Internal/Support/RecurringTransactionTrait.php @@ -228,7 +228,7 @@ trait RecurringTransactionTrait /** * @param Recurrence $recurrence - * @param array $tagst + * @param array $tags */ protected function updateTags(Recurrence $recurrence, array $tags): void { diff --git a/app/Services/Password/PwndVerifierV2.php b/app/Services/Password/PwndVerifierV2.php index de2180c784..87e936b846 100644 --- a/app/Services/Password/PwndVerifierV2.php +++ b/app/Services/Password/PwndVerifierV2.php @@ -26,6 +26,7 @@ use Exception; use GuzzleHttp\Client; use GuzzleHttp\Exception\GuzzleException; use Log; +use RuntimeException; /** * Class PwndVerifierV2. @@ -64,7 +65,12 @@ class PwndVerifierV2 implements Verifier if (404 === $res->getStatusCode()) { return true; } - $strpos = stripos($res->getBody()->getContents(), $rest); + try { + $strpos = stripos($res->getBody()->getContents(), $rest); + } catch (RunTimeException $e) { + Log::error(sprintf('Could not get body from Pwnd result: %s', $e->getMessage())); + $strpos = false; + } if (false === $strpos) { Log::debug(sprintf('%s was not found in result body. Return true.', $rest)); diff --git a/app/Services/Spectre/Request/SpectreRequest.php b/app/Services/Spectre/Request/SpectreRequest.php index a215373a91..cbf62ebe71 100644 --- a/app/Services/Spectre/Request/SpectreRequest.php +++ b/app/Services/Spectre/Request/SpectreRequest.php @@ -28,6 +28,7 @@ use FireflyIII\User; use GuzzleHttp\Client; use GuzzleHttp\Exception\GuzzleException; use Log; +use RuntimeException; /** * Class SpectreRequest @@ -208,7 +209,12 @@ abstract class SpectreRequest throw new FireflyException(sprintf('Guzzle Exception: %s', $e->getMessage())); } $statusCode = $res->getStatusCode(); - $returnBody = $res->getBody()->getContents(); + try { + $returnBody = $res->getBody()->getContents(); + } catch (RunTimeException $e) { + Log::error(sprintf('Could not get body from SpectreRequest::GET result: %s', $e->getMessage())); + $returnBody = ''; + } $this->detectError($returnBody, $statusCode); $array = json_decode($returnBody, true); @@ -252,7 +258,14 @@ abstract class SpectreRequest } catch (GuzzleException|Exception $e) { throw new FireflyException(sprintf('Guzzle Exception: %s', $e->getMessage())); } - $body = $res->getBody()->getContents(); + + try { + $body = $res->getBody()->getContents(); + } catch (RunTimeException $e) { + Log::error(sprintf('Could not get body from SpectreRequest::POST result: %s', $e->getMessage())); + $body = ''; + } + $statusCode = $res->getStatusCode(); $this->detectError($body, $statusCode); diff --git a/app/Support/Amount.php b/app/Support/Amount.php index 17dff88a69..3f25d0f567 100644 --- a/app/Support/Amount.php +++ b/app/Support/Amount.php @@ -46,6 +46,9 @@ class Amount * @param bool $csPrecedes * * @return string + * + * @SuppressWarnings(PHPMD.CyclomaticComplexity) + * @SuppressWarnings(PHPMD.ExcessiveMethodLength) */ public static function getAmountJsConfig(bool $sepBySpace, int $signPosn, string $sign, bool $csPrecedes): string { @@ -53,7 +56,7 @@ class Amount $space = ' '; // require space between symbol and amount? - if (!$sepBySpace) { + if ($sepBySpace === false) { $space = ''; // no } @@ -116,6 +119,7 @@ class Amount * @param bool $coloured * * @return string + * @SuppressWarnings(PHPMD.CyclomaticComplexity) */ public function formatAnything(TransactionCurrency $format, string $amount, bool $coloured = null): string { @@ -130,12 +134,8 @@ class Amount // some complicated switches to format the amount correctly: $precedes = $amount < 0 ? $info['n_cs_precedes'] : $info['p_cs_precedes']; $separated = $amount < 0 ? $info['n_sep_by_space'] : $info['p_sep_by_space']; - $space = $separated ? ' ' : ''; - $result = $format->symbol . $space . $formatted; - - if (!$precedes) { - $result = $formatted . $space . $format->symbol; - } + $space = $separated === true ? ' ' : ''; + $result = $precedes === true ? $format->symbol . $space . $formatted : $formatted . $space . $format->symbol; if (true === $coloured) { if ($amount > 0) { diff --git a/app/Support/Binder/AccountList.php b/app/Support/Binder/AccountList.php index b63b8f2c5d..f2b6497213 100644 --- a/app/Support/Binder/AccountList.php +++ b/app/Support/Binder/AccountList.php @@ -41,11 +41,11 @@ class AccountList implements BinderInterface * * @return Collection * @throws \Symfony\Component\HttpKernel\Exception\NotFoundHttpException + * @SuppressWarnings(PHPMD.CyclomaticComplexity) */ public static function routeBinder(string $value, Route $route): Collection { if (auth()->check()) { - $collection = new Collection; if ('allAssetAccounts' === $value) { /** @var \Illuminate\Support\Collection $collection */ @@ -55,18 +55,8 @@ class AccountList implements BinderInterface ->get(['accounts.*']); } if ('allAssetAccounts' !== $value) { - - $list = []; - $incoming = explode(',', $value); - foreach ($incoming as $entry) { - $list[] = (int)$entry; - } - $list = array_unique($list); - if (0 === \count($list)) { - Log::error('Account list is empty.'); - throw new NotFoundHttpException; // @codeCoverageIgnore - } - + $incoming = array_map('\intval', explode(',', $value)); + $list = array_merge(array_unique($incoming), [0]); /** @var \Illuminate\Support\Collection $collection */ $collection = auth()->user()->accounts() ->leftJoin('account_types', 'account_types.id', '=', 'accounts.account_type_id') diff --git a/app/Support/Binder/BudgetList.php b/app/Support/Binder/BudgetList.php index 3c6da0ce92..93921d7c53 100644 --- a/app/Support/Binder/BudgetList.php +++ b/app/Support/Binder/BudgetList.php @@ -38,16 +38,12 @@ class BudgetList implements BinderInterface * * @return Collection * @throws \Symfony\Component\HttpKernel\Exception\NotFoundHttpException + * @SuppressWarnings(PHPMD.CyclomaticComplexity) */ public static function routeBinder(string $value, Route $route): Collection { if (auth()->check()) { - $list = []; - $incoming = explode(',', $value); - foreach ($incoming as $entry) { - $list[] = (int)$entry; - } - $list = array_unique($list); + $list = array_unique(array_map('\intval', explode(',', $value))); if (0 === \count($list)) { throw new NotFoundHttpException; // @codeCoverageIgnore } diff --git a/app/Support/Binder/CategoryList.php b/app/Support/Binder/CategoryList.php index bd6997423d..37e392b9ab 100644 --- a/app/Support/Binder/CategoryList.php +++ b/app/Support/Binder/CategoryList.php @@ -38,16 +38,12 @@ class CategoryList implements BinderInterface * * @return Collection * @throws \Symfony\Component\HttpKernel\Exception\NotFoundHttpException + * @SuppressWarnings(PHPMD.CyclomaticComplexity) */ public static function routeBinder(string $value, Route $route): Collection { if (auth()->check()) { - $list = []; - $incoming = explode(',', $value); - foreach ($incoming as $entry) { - $list[] = (int)$entry; - } - $list = array_unique($list); + $list = array_unique(array_map('\intval', explode(',', $value))); if (0 === \count($list)) { throw new NotFoundHttpException; // @codeCoverageIgnore } diff --git a/app/Support/Binder/Date.php b/app/Support/Binder/Date.php index e075179b61..5105c208ce 100644 --- a/app/Support/Binder/Date.php +++ b/app/Support/Binder/Date.php @@ -46,28 +46,25 @@ class Date implements BinderInterface /** @var FiscalHelperInterface $fiscalHelper */ $fiscalHelper = app(FiscalHelperInterface::class); - switch ($value) { - default: - try { - $date = new Carbon($value); - } catch (Exception $e) { - Log::error(sprintf('Could not parse date "%s" for user #%d: %s', $value, auth()->user()->id, $e->getMessage())); - throw new NotFoundHttpException; - } - - return $date; - case 'currentMonthStart': - return Carbon::now()->startOfMonth(); - case 'currentMonthEnd': - return Carbon::now()->endOfMonth(); - case 'currentYearStart': - return Carbon::now()->startOfYear(); - case 'currentYearEnd': - return Carbon::now()->endOfYear(); - case 'currentFiscalYearStart': - return $fiscalHelper->startOfFiscalYear(Carbon::now()); - case 'currentFiscalYearEnd': - return $fiscalHelper->endOfFiscalYear(Carbon::now()); + $magicWords = [ + 'currentMonthStart' => Carbon::now()->startOfMonth(), + 'currentMonthEnd' => Carbon::now()->endOfMonth(), + 'currentYearStart' => Carbon::now()->startOfYear(), + 'currentYearEnd' => Carbon::now()->endOfYear(), + 'currentFiscalYearStart' => $fiscalHelper->startOfFiscalYear(Carbon::now()), + 'currentFiscalYearEnd' => $fiscalHelper->endOfFiscalYear(Carbon::now()), + ]; + if (isset($magicWords[$value])) { + return $magicWords[$value]; } + + try { + $result = new Carbon($value); + } catch (Exception $e) { + Log::error(sprintf('Could not parse date "%s" for user #%d: %s', $value, auth()->user()->id, $e->getMessage())); + throw new NotFoundHttpException; + } + + return $result; } } diff --git a/app/Support/Binder/ImportProvider.php b/app/Support/Binder/ImportProvider.php index 7e5917f50c..989a616936 100644 --- a/app/Support/Binder/ImportProvider.php +++ b/app/Support/Binder/ImportProvider.php @@ -37,6 +37,8 @@ class ImportProvider implements BinderInterface { /** * @return array + * @SuppressWarnings(PHPMD.CyclomaticComplexity) + * @SuppressWarnings(PHPMD.ExcessiveMethodLength) */ public static function getProviders(): array { diff --git a/app/Support/Binder/JournalList.php b/app/Support/Binder/JournalList.php index 5c034c242c..7562d9346c 100644 --- a/app/Support/Binder/JournalList.php +++ b/app/Support/Binder/JournalList.php @@ -41,12 +41,7 @@ class JournalList implements BinderInterface public static function routeBinder(string $value, Route $route): Collection { if (auth()->check()) { - $list = []; - $incoming = explode(',', $value); - foreach ($incoming as $entry) { - $list[] = (int)$entry; - } - $list = array_unique($list); + $list = array_unique(array_map('\intval', explode(',', $value))); if (0 === \count($list)) { throw new NotFoundHttpException; // @codeCoverageIgnore } diff --git a/app/Support/Binder/SimpleJournalList.php b/app/Support/Binder/SimpleJournalList.php index b33ac2fc9a..35708338ed 100644 --- a/app/Support/Binder/SimpleJournalList.php +++ b/app/Support/Binder/SimpleJournalList.php @@ -41,16 +41,14 @@ class SimpleJournalList implements BinderInterface * * @return mixed * @throws \Symfony\Component\HttpKernel\Exception\NotFoundHttpException + * @SuppressWarnings(PHPMD.CyclomaticComplexity) + * @SuppressWarnings(PHPMD.ExcessiveMethodLength) + * @SuppressWarnings(PHPMD.NPathComplexity) */ public static function routeBinder(string $value, Route $route): Collection { if (auth()->check()) { - $list = []; - $incoming = explode(',', $value); - foreach ($incoming as $entry) { - $list[] = (int)$entry; - } - $list = array_unique($list); + $list = array_unique(array_map('\intval', explode(',', $value))); if (0 === \count($list)) { throw new NotFoundHttpException; // @codeCoverageIgnore } @@ -97,7 +95,6 @@ class SimpleJournalList implements BinderInterface } if ($final->count() > 0) { - if (\count($messages) > 0) { session()->flash('info', $messages); } diff --git a/app/Support/Binder/TagList.php b/app/Support/Binder/TagList.php index b4131e0eea..2201eb09b2 100644 --- a/app/Support/Binder/TagList.php +++ b/app/Support/Binder/TagList.php @@ -44,12 +44,7 @@ class TagList implements BinderInterface public static function routeBinder(string $value, Route $route): Collection { if (auth()->check()) { - $list = []; - $incoming = explode(',', $value); - foreach ($incoming as $entry) { - $list[] = strtolower(trim($entry)); - } - $list = array_unique($list); + $list = array_unique(array_map('\strtolower', explode(',', $value))); if (0 === \count($list)) { Log::error('Tag list is empty.'); throw new NotFoundHttpException; // @codeCoverageIgnore diff --git a/app/Support/ExpandedForm.php b/app/Support/ExpandedForm.php index e7edc728f2..3f7e706f9e 100644 --- a/app/Support/ExpandedForm.php +++ b/app/Support/ExpandedForm.php @@ -25,6 +25,7 @@ namespace FireflyIII\Support; use Amount as Amt; use Carbon\Carbon; use Eloquent; +use FireflyIII\Exceptions\FireflyException; use FireflyIII\Models\Account; use FireflyIII\Models\AccountType; use FireflyIII\Models\PiggyBank; @@ -46,6 +47,8 @@ use Throwable; /** * Class ExpandedForm. * + * @SuppressWarnings(PHPMD.TooManyMethods) + * @SuppressWarnings(PHPMD.TooManyPublicMethods) */ class ExpandedForm { @@ -94,6 +97,7 @@ class ExpandedForm * @param array $options * * @return string + * @throws FireflyException */ public function amount(string $name, $value = null, array $options = null): string { @@ -132,19 +136,6 @@ class ExpandedForm return $html; } - /** - * @param string $name - * @param mixed $value - * @param array $options - * - * @return string - * @throws \FireflyIII\Exceptions\FireflyException - */ - public function amountSmall(string $name, $value = null, array $options = null): string - { - return $this->currencyField($name, 'amount-small', $value, $options); - } - /** * @param string $name * @param array $options @@ -232,6 +223,7 @@ class ExpandedForm * @param array $options * * @return string + * @throws FireflyException */ public function balance(string $name, $value = null, array $options = null): string { @@ -428,6 +420,7 @@ class ExpandedForm * @param \Illuminate\Support\Collection $set * * @return array + * @SuppressWarnings(PHPMD.CyclomaticComplexity) */ public function makeSelectList(Collection $set): array { @@ -453,6 +446,7 @@ class ExpandedForm * @param \Illuminate\Support\Collection $set * * @return array + * @SuppressWarnings(PHPMD.CyclomaticComplexity) */ public function makeSelectListWithEmpty(Collection $set): array { @@ -475,36 +469,6 @@ class ExpandedForm return $selectList; } - /** @noinspection MoreThanThreeArgumentsInspection */ - - /** - * @param string $name - * @param array $list - * @param mixed $selected - * @param array $options - * - * @return string - * - */ - public function multiRadio(string $name, array $list = null, $selected = null, array $options = null): string - { - $list = $list ?? []; - $label = $this->label($name, $options); - $options = $this->expandOptionArray($name, $label, $options); - $classes = $this->getHolderClasses($name); - $selected = $this->fillFieldValue($name, $selected); - - unset($options['class']); - try { - $html = view('form.multiRadio', compact('classes', 'name', 'label', 'selected', 'options', 'list'))->render(); - } catch (Throwable $e) { - Log::debug(sprintf('Could not render multiRadio(): %s', $e->getMessage())); - $html = 'Could not render multiRadio.'; - } - - return $html; - } - /** * @param string $name * @param mixed $value @@ -538,40 +502,6 @@ class ExpandedForm return $html; } - /** - * @param string $name - * @param mixed $value - * @param array $options - * - * @return string - * @throws \FireflyIII\Exceptions\FireflyException - * - */ - public function nonSelectableBalance(string $name, $value = null, array $options = null): string - { - $label = $this->label($name, $options); - $options = $this->expandOptionArray($name, $label, $options); - $classes = $this->getHolderClasses($name); - $value = $this->fillFieldValue($name, $value); - $options['step'] = 'any'; - $selectedCurrency = $options['currency'] ?? Amt::getDefaultCurrency(); - unset($options['currency'], $options['placeholder']); - - // make sure value is formatted nicely: - if (null !== $value && '' !== $value) { - $decimals = $selectedCurrency->decimal_places ?? 2; - $value = round($value, $decimals); - } - try { - $html = view('form.non-selectable-amount', compact('selectedCurrency', 'classes', 'name', 'label', 'value', 'options'))->render(); - } catch (Throwable $e) { - Log::debug(sprintf('Could not render nonSelectableBalance(): %s', $e->getMessage())); - $html = 'Could not render nonSelectableBalance.'; - } - - return $html; - } - /** * @param string $name * @param mixed $value @@ -841,6 +771,59 @@ class ExpandedForm return $html; } + /** + * @param string $name + * @param string $view + * @param mixed $value + * @param array $options + * + * @return string + * @throws FireflyException + * + * @SuppressWarnings(PHPMD.CyclomaticComplexity) + */ + protected function currencyField(string $name, string $view, $value = null, array $options = null): string + { + $label = $this->label($name, $options); + $options = $this->expandOptionArray($name, $label, $options); + $classes = $this->getHolderClasses($name); + $value = $this->fillFieldValue($name, $value); + $options['step'] = 'any'; + $defaultCurrency = $options['currency'] ?? Amt::getDefaultCurrency(); + /** @var Collection $currencies */ + $currencies = app('amount')->getAllCurrencies(); + unset($options['currency'], $options['placeholder']); + + // perhaps the currency has been sent to us in the field $amount_currency_id_$name (amount_currency_id_amount) + $preFilled = session('preFilled'); + $key = 'amount_currency_id_' . $name; + $sentCurrencyId = isset($preFilled[$key]) ? (int)$preFilled[$key] : $defaultCurrency->id; + + Log::debug(sprintf('Sent currency ID is %d', $sentCurrencyId)); + + // find this currency in set of currencies: + foreach ($currencies as $currency) { + if ($currency->id === $sentCurrencyId) { + $defaultCurrency = $currency; + Log::debug(sprintf('default currency is now %s', $defaultCurrency->code)); + break; + } + } + + // make sure value is formatted nicely: + if (null !== $value && '' !== $value) { + $value = round($value, $defaultCurrency->decimal_places); + } + try { + $html = view('form.' . $view, compact('defaultCurrency', 'currencies', 'classes', 'name', 'label', 'value', 'options'))->render(); + } catch (Throwable $e) { + Log::debug(sprintf('Could not render currencyField(): %s', $e->getMessage())); + $html = 'Could not render currencyField.'; + } + + return $html; + } + /** * @param $name * @param $label @@ -865,6 +848,7 @@ class ExpandedForm * @param $value * * @return mixed + * @SuppressWarnings(PHPMD.CyclomaticComplexity) */ protected function fillFieldValue(string $name, $value) { @@ -906,6 +890,8 @@ class ExpandedForm return $classes; } + /** @noinspection MoreThanThreeArgumentsInspection */ + /** * @param $name * @param $options @@ -922,56 +908,4 @@ class ExpandedForm return (string)trans('form.' . $name); } - - /** @noinspection MoreThanThreeArgumentsInspection */ - /** - * @param string $name - * @param string $view - * @param mixed $value - * @param array $options - * - * @return string - * @throws \FireflyIII\Exceptions\FireflyException - * @throws \FireflyIII\Exceptions\FireflyException - */ - private function currencyField(string $name, string $view, $value = null, array $options = null): string - { - $label = $this->label($name, $options); - $options = $this->expandOptionArray($name, $label, $options); - $classes = $this->getHolderClasses($name); - $value = $this->fillFieldValue($name, $value); - $options['step'] = 'any'; - $defaultCurrency = $options['currency'] ?? Amt::getDefaultCurrency(); - $currencies = app('amount')->getAllCurrencies(); - unset($options['currency'], $options['placeholder']); - - // perhaps the currency has been sent to us in the field $amount_currency_id_$name (amount_currency_id_amount) - $preFilled = session('preFilled'); - $key = 'amount_currency_id_' . $name; - $sentCurrencyId = isset($preFilled[$key]) ? (int)$preFilled[$key] : $defaultCurrency->id; - - Log::debug(sprintf('Sent currency ID is %d', $sentCurrencyId)); - - // find this currency in set of currencies: - foreach ($currencies as $currency) { - if ($currency->id === $sentCurrencyId) { - $defaultCurrency = $currency; - Log::debug(sprintf('default currency is now %s', $defaultCurrency->code)); - break; - } - } - - // make sure value is formatted nicely: - if (null !== $value && '' !== $value) { - $value = round($value, $defaultCurrency->decimal_places); - } - try { - $html = view('form.' . $view, compact('defaultCurrency', 'currencies', 'classes', 'name', 'label', 'value', 'options'))->render(); - } catch (Throwable $e) { - Log::debug(sprintf('Could not render currencyField(): %s', $e->getMessage())); - $html = 'Could not render currencyField.'; - } - - return $html; - } } diff --git a/app/Support/Import/JobConfiguration/File/ConfigureMappingHandler.php b/app/Support/Import/JobConfiguration/File/ConfigureMappingHandler.php index 4d16537a19..568ff9d384 100644 --- a/app/Support/Import/JobConfiguration/File/ConfigureMappingHandler.php +++ b/app/Support/Import/JobConfiguration/File/ConfigureMappingHandler.php @@ -271,6 +271,7 @@ class ConfigureMappingHandler implements FileConfigurationInterface * * @return array * @throws FireflyException + * @SuppressWarnings(PHPMD.CyclomaticComplexity) */ public function getValuesForMapping(Reader $reader, array $config, array $columnConfig): array { diff --git a/app/Support/Import/JobConfiguration/File/ConfigureRolesHandler.php b/app/Support/Import/JobConfiguration/File/ConfigureRolesHandler.php index 16a45137f9..ce8413b313 100644 --- a/app/Support/Import/JobConfiguration/File/ConfigureRolesHandler.php +++ b/app/Support/Import/JobConfiguration/File/ConfigureRolesHandler.php @@ -58,6 +58,7 @@ class ConfigureRolesHandler implements FileConfigurationInterface * @param array $config * * @return MessageBag + * @SuppressWarnings(PHPMD.CyclomaticComplexity) */ public function configurationComplete(array $config): MessageBag { diff --git a/app/Support/Import/JobConfiguration/Spectre/ChooseAccountsHandler.php b/app/Support/Import/JobConfiguration/Spectre/ChooseAccountsHandler.php index 8f42ac6ea0..7c7c402cf1 100644 --- a/app/Support/Import/JobConfiguration/Spectre/ChooseAccountsHandler.php +++ b/app/Support/Import/JobConfiguration/Spectre/ChooseAccountsHandler.php @@ -110,6 +110,7 @@ class ChooseAccountsHandler implements SpectreJobConfigurationInterface * * @return array * @throws FireflyException + * @SuppressWarnings(PHPMD.CyclomaticComplexity) */ public function getNextData(): array { diff --git a/app/Support/Import/Placeholder/ImportTransaction.php b/app/Support/Import/Placeholder/ImportTransaction.php index c97f3f8f7e..3847c7553c 100644 --- a/app/Support/Import/Placeholder/ImportTransaction.php +++ b/app/Support/Import/Placeholder/ImportTransaction.php @@ -128,6 +128,7 @@ class ImportTransaction * @param ColumnValue $columnValue * * @throws FireflyException + * @SuppressWarnings(PHPMD.CyclomaticComplexity) */ public function addColumnValue(ColumnValue $columnValue): void { diff --git a/app/Support/Import/Routine/File/AssetAccountMapper.php b/app/Support/Import/Routine/File/AssetAccountMapper.php index 10535e1ad4..800ee89ebd 100644 --- a/app/Support/Import/Routine/File/AssetAccountMapper.php +++ b/app/Support/Import/Routine/File/AssetAccountMapper.php @@ -48,6 +48,7 @@ class AssetAccountMapper * @param array $data * * @return Account + * @SuppressWarnings(PHPMD.CyclomaticComplexity) */ public function map(?int $accountId, array $data): Account { diff --git a/app/Support/Import/Routine/File/MappedValuesValidator.php b/app/Support/Import/Routine/File/MappedValuesValidator.php index ddd2434871..af3d35249d 100644 --- a/app/Support/Import/Routine/File/MappedValuesValidator.php +++ b/app/Support/Import/Routine/File/MappedValuesValidator.php @@ -81,6 +81,7 @@ class MappedValuesValidator * * @return array * @throws FireflyException + * @SuppressWarnings(PHPMD.CyclomaticComplexity) */ public function validate(array $mappings): array { diff --git a/app/Support/Import/Routine/File/MappingConverger.php b/app/Support/Import/Routine/File/MappingConverger.php index 57d70522ad..50fa6352ad 100644 --- a/app/Support/Import/Routine/File/MappingConverger.php +++ b/app/Support/Import/Routine/File/MappingConverger.php @@ -135,44 +135,32 @@ class MappingConverger return $role; } - switch ($role) { - default: - throw new FireflyException(sprintf('Cannot indicate new role for mapped role "%s"', $role)); // @codeCoverageIgnore - case 'account-id': - case 'account-name': - case 'account-iban': - case 'account-number': - $newRole = 'account-id'; - break; - case 'bill-id': - case 'bill-name': - $newRole = 'bill-id'; - break; - case 'budget-id': - case 'budget-name': - $newRole = 'budget-id'; - break; - case 'currency-id': - case 'currency-name': - case 'currency-code': - case 'currency-symbol': - $newRole = 'currency-id'; - break; - case 'category-id': - case 'category-name': - $newRole = 'category-id'; - break; - case 'foreign-currency-id': - case 'foreign-currency-code': - $newRole = 'foreign-currency-id'; - break; - case 'opposing-id': - case 'opposing-name': - case 'opposing-iban': - case 'opposing-number': - $newRole = 'opposing-id'; - break; + $roleMapping = [ + 'account-id' => 'account-id', + 'account-name' => 'account-id', + 'account-iban' => 'account-id', + 'account-number' => 'account-id', + 'bill-id' => 'bill-id', + 'bill-name' => 'bill-id', + 'budget-id' => 'budget-id', + 'budget-name' => 'budget-id', + 'currency-id' => 'currency-id', + 'currency-name' => 'currency-id', + 'currency-code' => 'currency-id', + 'currency-symbol' => 'currency-id', + 'category-id' => 'category-id', + 'category-name' => 'category-id', + 'foreign-currency-id' => 'foreign-currency-id', + 'foreign-currency-code' => 'foreign-currency-id', + 'opposing-id' => 'opposing-id', + 'opposing-name' => 'opposing-id', + 'opposing-iban' => 'opposing-id', + 'opposing-number' => 'opposing-id', + ]; + if (!isset($roleMapping[$role])) { + throw new FireflyException(sprintf('Cannot indicate new role for mapped role "%s"', $role)); // @codeCoverageIgnore } + $newRole = $roleMapping[$role]; Log::debug(sprintf('Role was "%s", but because of mapping (mapped to #%d), role becomes "%s"', $role, $mapped, $newRole)); // also store the $mapped values in a "mappedValues" array. diff --git a/app/Support/Import/Routine/File/OpposingAccountMapper.php b/app/Support/Import/Routine/File/OpposingAccountMapper.php index 5f67428d7e..a6541785f3 100644 --- a/app/Support/Import/Routine/File/OpposingAccountMapper.php +++ b/app/Support/Import/Routine/File/OpposingAccountMapper.php @@ -45,6 +45,7 @@ class OpposingAccountMapper * @param array $data * * @return Account + * @SuppressWarnings(PHPMD.CyclomaticComplexity) */ public function map(?int $accountId, string $amount, array $data): Account { diff --git a/app/Support/Import/Routine/Spectre/StageImportDataHandler.php b/app/Support/Import/Routine/Spectre/StageImportDataHandler.php index 789aa6c664..bf1c29157c 100644 --- a/app/Support/Import/Routine/Spectre/StageImportDataHandler.php +++ b/app/Support/Import/Routine/Spectre/StageImportDataHandler.php @@ -107,6 +107,7 @@ class StageImportDataHandler * @param LocalAccount $originalSource * * @return array + * @SuppressWarnings(PHPMD.CyclomaticComplexity) */ private function convertToArray(array $transactions, SpectreAccount $spectreAccount, LocalAccount $originalSource): array { diff --git a/app/Support/Navigation.php b/app/Support/Navigation.php index a539793d17..4317c34b45 100644 --- a/app/Support/Navigation.php +++ b/app/Support/Navigation.php @@ -87,6 +87,7 @@ class Navigation * * @return array * @throws \FireflyIII\Exceptions\FireflyException + * @SuppressWarnings(PHPMD.CyclomaticComplexity) */ public function blockPeriods(\Carbon\Carbon $start, \Carbon\Carbon $end, string $range): array { diff --git a/app/Support/Search/Search.php b/app/Support/Search/Search.php index ff47407573..cc1e884954 100644 --- a/app/Support/Search/Search.php +++ b/app/Support/Search/Search.php @@ -188,6 +188,7 @@ class Search implements SearchInterface * @param JournalCollectorInterface $collector * * @return JournalCollectorInterface + * @SuppressWarnings(PHPMD.CyclomaticComplexity) */ private function applyModifiers(JournalCollectorInterface $collector): JournalCollectorInterface { diff --git a/app/Support/Twig/Extension/Transaction.php b/app/Support/Twig/Extension/Transaction.php index 327c7af1d6..7e544001ab 100644 --- a/app/Support/Twig/Extension/Transaction.php +++ b/app/Support/Twig/Extension/Transaction.php @@ -46,11 +46,11 @@ class Transaction extends Twig_Extension */ public function amount(TransactionModel $transaction): string { + // at this point amount is always negative. $amount = bcmul(app('steam')->positive((string)$transaction->transaction_amount), '-1'); $format = '%s'; $coloured = true; - // at this point amount is always negative. if (TransactionType::RECONCILIATION === $transaction->transaction_type_type && 1 === bccomp((string)$transaction->transaction_amount, '0')) { $amount = bcmul($amount, '-1'); } diff --git a/app/TransactionRules/Actions/ClearBudget.php b/app/TransactionRules/Actions/ClearBudget.php index e2366b7397..0047f0903f 100644 --- a/app/TransactionRules/Actions/ClearBudget.php +++ b/app/TransactionRules/Actions/ClearBudget.php @@ -32,9 +32,6 @@ use Log; */ class ClearBudget implements ActionInterface { - /** @var RuleAction The rule action */ - private $action; - /** * TriggerInterface constructor. * @@ -42,7 +39,6 @@ class ClearBudget implements ActionInterface */ public function __construct(RuleAction $action) { - $this->action = $action; } /** diff --git a/app/TransactionRules/Actions/ClearCategory.php b/app/TransactionRules/Actions/ClearCategory.php index df9637c448..90ee273feb 100644 --- a/app/TransactionRules/Actions/ClearCategory.php +++ b/app/TransactionRules/Actions/ClearCategory.php @@ -32,9 +32,6 @@ use Log; */ class ClearCategory implements ActionInterface { - /** @var RuleAction The rule action */ - private $action; - /** * TriggerInterface constructor. * @@ -42,7 +39,6 @@ class ClearCategory implements ActionInterface */ public function __construct(RuleAction $action) { - $this->action = $action; } /** diff --git a/app/TransactionRules/Actions/ClearNotes.php b/app/TransactionRules/Actions/ClearNotes.php index ff823ffbeb..7b83617770 100644 --- a/app/TransactionRules/Actions/ClearNotes.php +++ b/app/TransactionRules/Actions/ClearNotes.php @@ -32,9 +32,6 @@ use Log; */ class ClearNotes implements ActionInterface { - /** @var RuleAction The rule action */ - private $action; - /** * TriggerInterface constructor. * @@ -42,7 +39,6 @@ class ClearNotes implements ActionInterface */ public function __construct(RuleAction $action) { - $this->action = $action; } /** diff --git a/app/TransactionRules/Actions/RemoveAllTags.php b/app/TransactionRules/Actions/RemoveAllTags.php index 33535117b8..85ab97668d 100644 --- a/app/TransactionRules/Actions/RemoveAllTags.php +++ b/app/TransactionRules/Actions/RemoveAllTags.php @@ -31,9 +31,6 @@ use Log; */ class RemoveAllTags implements ActionInterface { - /** @var RuleAction The rule action */ - private $action; - /** * TriggerInterface constructor. * @@ -41,7 +38,6 @@ class RemoveAllTags implements ActionInterface */ public function __construct(RuleAction $action) { - $this->action = $action; } /** diff --git a/app/TransactionRules/Actions/SetCategory.php b/app/TransactionRules/Actions/SetCategory.php index 94a776d67a..754f663e15 100644 --- a/app/TransactionRules/Actions/SetCategory.php +++ b/app/TransactionRules/Actions/SetCategory.php @@ -61,7 +61,11 @@ class SetCategory implements ActionInterface $factory = app(CategoryFactory::class); $factory->setUser($journal->user); $category = $factory->findOrCreate(null, $name); + if (null === $category) { + Log::error(sprintf('Action SetCategory did not fire because "%s" did not result in a valid category.', $name)); + return true; + } $journal->categories()->detach(); // set category on transactions: /** @var Transaction $transaction */ diff --git a/app/TransactionRules/Actions/SetSourceAccount.php b/app/TransactionRules/Actions/SetSourceAccount.php index 570d697bbd..ff2f0e870f 100644 --- a/app/TransactionRules/Actions/SetSourceAccount.php +++ b/app/TransactionRules/Actions/SetSourceAccount.php @@ -100,7 +100,12 @@ class SetSourceAccount implements ActionInterface // update source transaction with new source account: // get source transaction: - $transaction = $journal->transactions()->where('amount', '<', 0)->first(); + $transaction = $journal->transactions()->where('amount', '<', 0)->first(); + if (null === $transaction) { + Log::error(sprintf('Cannot change source account of journal #%d because no source transaction exists.', $journal->id)); + + return false; + } $transaction->account_id = $this->newSourceAccount->id; $transaction->save(); $journal->touch(); @@ -130,7 +135,7 @@ class SetSourceAccount implements ActionInterface /** * */ - private function findRevenueAccount() + private function findRevenueAccount(): void { $account = $this->repository->findByName($this->action->action_value, [AccountType::REVENUE]); if (null === $account) { diff --git a/app/TransactionRules/Factory/TriggerFactory.php b/app/TransactionRules/Factory/TriggerFactory.php index e117de55bc..96caf8d78e 100644 --- a/app/TransactionRules/Factory/TriggerFactory.php +++ b/app/TransactionRules/Factory/TriggerFactory.php @@ -52,7 +52,7 @@ class TriggerFactory * * @throws FireflyException */ - public static function getTrigger(RuleTrigger $trigger) + public static function getTrigger(RuleTrigger $trigger): AbstractTrigger { $triggerType = $trigger->trigger_type; @@ -84,7 +84,7 @@ class TriggerFactory * * @throws FireflyException */ - public static function makeTriggerFromStrings(string $triggerType, string $triggerValue, bool $stopProcessing) + public static function makeTriggerFromStrings(string $triggerType, string $triggerValue, bool $stopProcessing): AbstractTrigger { /** @var AbstractTrigger $class */ $class = self::getTriggerClass($triggerType); diff --git a/app/TransactionRules/Processor.php b/app/TransactionRules/Processor.php index ca7298dcdf..14d44d0fc7 100644 --- a/app/TransactionRules/Processor.php +++ b/app/TransactionRules/Processor.php @@ -72,8 +72,9 @@ final class Processor * @return Processor * @throws \FireflyIII\Exceptions\FireflyException */ - public static function make(Rule $rule, $includeActions = true) + public static function make(Rule $rule, bool $includeActions = null): Processor { + $includeActions = $includeActions ?? true; Log::debug(sprintf('Making new rule from Rule %d', $rule->id)); Log::debug(sprintf('Rule is strict: %s', var_export($rule->strict, true))); $self = new self; @@ -85,7 +86,7 @@ final class Processor Log::debug(sprintf('Push trigger %d', $trigger->id)); $self->triggers->push(TriggerFactory::getTrigger($trigger)); } - if ($includeActions) { + if ($includeActions === true) { $self->actions = $rule->ruleActions()->orderBy('order', 'ASC')->get(); } @@ -104,7 +105,7 @@ final class Processor * * @throws \FireflyIII\Exceptions\FireflyException */ - public static function makeFromString(string $triggerName, string $triggerValue) + public static function makeFromString(string $triggerName, string $triggerValue): Processor { Log::debug(sprintf('Processor::makeFromString("%s", "%s")', $triggerName, $triggerValue)); $self = new self; @@ -129,7 +130,7 @@ final class Processor * * @throws \FireflyIII\Exceptions\FireflyException */ - public static function makeFromStringArray(array $triggers) + public static function makeFromStringArray(array $triggers): Processor { $self = new self; foreach ($triggers as $entry) { @@ -156,7 +157,7 @@ final class Processor * * @param int $foundTriggers */ - public function setFoundTriggers(int $foundTriggers) + public function setFoundTriggers(int $foundTriggers): void { $this->foundTriggers = $foundTriggers; } @@ -236,10 +237,10 @@ final class Processor /** * Run the actions * - * @return bool + * @return void * @throws \FireflyIII\Exceptions\FireflyException */ - private function actions() + private function actions(): void { /** * @var int @@ -255,8 +256,6 @@ final class Processor break; } } - - return true; } /** diff --git a/app/TransactionRules/Triggers/AbstractTrigger.php b/app/TransactionRules/Triggers/AbstractTrigger.php index fcf626618b..f4c2afab91 100644 --- a/app/TransactionRules/Triggers/AbstractTrigger.php +++ b/app/TransactionRules/Triggers/AbstractTrigger.php @@ -72,7 +72,7 @@ class AbstractTrigger * * @return AbstractTrigger */ - public static function makeFromTrigger(RuleTrigger $trigger) + public static function makeFromTrigger(RuleTrigger $trigger): AbstractTrigger { $self = new static; $self->trigger = $trigger; @@ -91,7 +91,7 @@ class AbstractTrigger * * @return AbstractTrigger */ - public static function makeFromTriggerValue(string $triggerValue) + public static function makeFromTriggerValue(string $triggerValue): AbstractTrigger { $self = new static; $self->triggerValue = $triggerValue; diff --git a/app/TransactionRules/Triggers/NotesContain.php b/app/TransactionRules/Triggers/NotesContain.php index 54901597b3..4c3eef2ff7 100644 --- a/app/TransactionRules/Triggers/NotesContain.php +++ b/app/TransactionRules/Triggers/NotesContain.php @@ -74,7 +74,7 @@ final class NotesContain extends AbstractTrigger implements TriggerInterface { $search = strtolower(trim($this->triggerValue)); - if (0 === \strlen($search)) { + if ('' === $search) { Log::debug(sprintf('RuleTrigger NotesContain for journal #%d: "%s" is empty, return false.', $journal->id, $search)); return false; diff --git a/app/TransactionRules/Triggers/NotesEmpty.php b/app/TransactionRules/Triggers/NotesEmpty.php index bd42f18ab6..d1d18e3544 100644 --- a/app/TransactionRules/Triggers/NotesEmpty.php +++ b/app/TransactionRules/Triggers/NotesEmpty.php @@ -68,7 +68,7 @@ final class NotesEmpty extends AbstractTrigger implements TriggerInterface $text = $note->text; } - if (0 === \strlen($text)) { + if ('' === $text) { Log::debug(sprintf('RuleTrigger NotesEmpty for journal #%d: strlen is 0, return true.', $journal->id)); return true; diff --git a/config/twigbridge.php b/config/twigbridge.php index f6a6e22ae3..fa0557b7c9 100644 --- a/config/twigbridge.php +++ b/config/twigbridge.php @@ -187,7 +187,7 @@ return [ 'ExpandedForm' => [ 'is_safe' => [ 'date', 'text', 'select', 'balance', 'optionsList', 'checkbox', 'amount', 'tags', 'integer', 'textarea', 'location', - 'multiRadio', 'file', 'multiCheckbox', 'staticText', 'amountSmall', 'password', 'nonSelectableBalance', 'nonSelectableAmount', + 'file', 'staticText', 'password', 'nonSelectableAmount', 'number', 'assetAccountList','amountNoCurrency','currencyList','ruleGroupList','assetAccountCheckList','ruleGroupListWithEmpty', 'piggyBankList','currencyListEmpty','activeAssetAccountList' ], diff --git a/resources/views/form/multiRadio.twig b/resources/views/form/multiRadio.twig deleted file mode 100644 index f321eda908..0000000000 --- a/resources/views/form/multiRadio.twig +++ /dev/null @@ -1,17 +0,0 @@ -
- - -
- {% for value,description in list %} -
- -
- {% endfor %} - {% include 'form/help' %} - {% include 'form/feedback' %} - -
-