From e78a59a8a841a4ccf7599036f4d7426672a2e629 Mon Sep 17 00:00:00 2001 From: James Cole Date: Sat, 7 Jul 2018 21:17:46 +0200 Subject: [PATCH] Code quality update. --- .../V1/Requests/AvailableBudgetRequest.php | 1 + app/Api/V1/Requests/CurrencyRequest.php | 2 +- app/Api/V1/Requests/JournalLinkRequest.php | 3 + app/Helpers/Attachments/AttachmentHelper.php | 148 ++++++++++-------- .../Attachments/AttachmentHelperInterface.php | 12 ++ app/Helpers/Chart/MetaPieChart.php | 44 +++--- app/Helpers/Collection/Balance.php | 6 +- app/Helpers/Collection/BalanceEntry.php | 6 +- app/Helpers/Collection/BalanceHeader.php | 2 +- app/Helpers/Collection/BalanceLine.php | 29 ++-- app/Helpers/Collection/Bill.php | 8 +- app/Helpers/Collection/BillLine.php | 14 +- app/Helpers/Collection/Category.php | 6 +- app/Helpers/Collector/JournalCollector.php | 63 ++++---- app/Helpers/Filter/InternalTransferFilter.php | 2 +- app/Helpers/Filter/OpposingAccountFilter.php | 2 +- app/Helpers/Filter/TransactionViewFilter.php | 2 +- app/Helpers/FiscalHelper.php | 12 +- app/Helpers/Help/Help.php | 21 ++- app/Helpers/Report/BalanceReportHelper.php | 5 +- app/Helpers/Report/BudgetReportHelper.php | 5 +- app/Helpers/Report/PopupReport.php | 8 +- app/Helpers/Report/ReportHelper.php | 6 +- app/Models/Category.php | 1 + app/Models/Transaction.php | 7 +- app/Transformers/LinkTypeTransformer.php | 6 +- 26 files changed, 236 insertions(+), 185 deletions(-) diff --git a/app/Api/V1/Requests/AvailableBudgetRequest.php b/app/Api/V1/Requests/AvailableBudgetRequest.php index 0a1fdb6e59..31c673e10d 100644 --- a/app/Api/V1/Requests/AvailableBudgetRequest.php +++ b/app/Api/V1/Requests/AvailableBudgetRequest.php @@ -55,6 +55,7 @@ class AvailableBudgetRequest extends Request } /** + * TODO must also accept currency code. * The rules that the incoming request must be matched against. * * @return array diff --git a/app/Api/V1/Requests/CurrencyRequest.php b/app/Api/V1/Requests/CurrencyRequest.php index 29355bcfbb..a535b8d006 100644 --- a/app/Api/V1/Requests/CurrencyRequest.php +++ b/app/Api/V1/Requests/CurrencyRequest.php @@ -68,7 +68,7 @@ class CurrencyRequest extends Request 'code' => 'required|between:3,3|unique:transaction_currencies,code', 'symbol' => 'required|between:1,5|unique:transaction_currencies,symbol', 'decimal_places' => 'required|between:0,20|numeric|min:0|max:20', - 'default' => 'in:true,false', + 'default' => 'boolean', ]; switch ($this->method()) { diff --git a/app/Api/V1/Requests/JournalLinkRequest.php b/app/Api/V1/Requests/JournalLinkRequest.php index d2609eff12..fd06410b08 100644 --- a/app/Api/V1/Requests/JournalLinkRequest.php +++ b/app/Api/V1/Requests/JournalLinkRequest.php @@ -57,6 +57,9 @@ class JournalLinkRequest extends Request } /** + * TODO include link-type name as optional parameter. + * TODO be consistent and remove notes from this object. + * * The rules that the incoming request must be matched against. * * @return array diff --git a/app/Helpers/Attachments/AttachmentHelper.php b/app/Helpers/Attachments/AttachmentHelper.php index b6d950042f..849705f2da 100644 --- a/app/Helpers/Attachments/AttachmentHelper.php +++ b/app/Helpers/Attachments/AttachmentHelper.php @@ -38,18 +38,18 @@ use Symfony\Component\HttpFoundation\File\UploadedFile; */ class AttachmentHelper implements AttachmentHelperInterface { - /** @var Collection */ + /** @var Collection All attachments */ public $attachments; - /** @var MessageBag */ + /** @var MessageBag All errors */ public $errors; - /** @var MessageBag */ + /** @var MessageBag All messages */ public $messages; - /** @var array */ + /** @var array Allowed mimes */ protected $allowedMimes = []; - /** @var int */ + /** @var int Max upload size. */ protected $maxUploadSize = 0; - /** @var \Illuminate\Contracts\Filesystem\Filesystem */ + /** @var \Illuminate\Contracts\Filesystem\Filesystem The disk where attachments are stored. */ protected $uploadDisk; @@ -67,6 +67,8 @@ class AttachmentHelper implements AttachmentHelperInterface } /** + * Returns the content of an attachment. + * * @codeCoverageIgnore * * @param Attachment $attachment @@ -75,18 +77,19 @@ class AttachmentHelper implements AttachmentHelperInterface */ public function getAttachmentContent(Attachment $attachment): string { + $content = ''; try { $content = Crypt::decrypt($this->uploadDisk->get(sprintf('at-%d.data', $attachment->id))); } catch (DecryptException|FileNotFoundException $e) { - Log::error(sprintf('Could not decrypt data of attachment #%d', $attachment->id)); - - return ''; + Log::error(sprintf('Could not decrypt data of attachment #%d: %s', $attachment->id, $e->getMessage())); } return $content; } /** + * Returns the file location for an attachment, + * * @param Attachment $attachment * * @return string @@ -99,6 +102,8 @@ class AttachmentHelper implements AttachmentHelperInterface } /** + * Get all attachments. + * * @return Collection */ public function getAttachments(): Collection @@ -107,6 +112,8 @@ class AttachmentHelper implements AttachmentHelperInterface } /** + * Get all errors. + * * @return MessageBag */ public function getErrors(): MessageBag @@ -115,6 +122,8 @@ class AttachmentHelper implements AttachmentHelperInterface } /** + * Get all messages. + * * @return MessageBag */ public function getMessages(): MessageBag @@ -122,6 +131,7 @@ class AttachmentHelper implements AttachmentHelperInterface return $this->messages; } + /** @noinspection MultipleReturnStatementsInspection */ /** * Uploads a file as a string. * @@ -163,6 +173,8 @@ class AttachmentHelper implements AttachmentHelperInterface } /** + * Save attachments that get uploaded with models, through the app. + * * @param Model $model * @param array|null $files * @@ -180,15 +192,17 @@ class AttachmentHelper implements AttachmentHelperInterface } } Log::debug('Done processing uploads.'); - - return true; } - Log::debug('Array of files is not an array. Probably nothing uploaded. Will not store attachments.'); + if (!\is_array($files) || (\is_array($files) && 0 === \count($files))) { + Log::debug('Array of files is not an array. Probably nothing uploaded. Will not store attachments.'); + } return true; } /** + * Check if a model already has this file attached. + * * @param UploadedFile $file * @param Model $model * @@ -199,67 +213,70 @@ class AttachmentHelper implements AttachmentHelperInterface $md5 = md5_file($file->getRealPath()); $name = $file->getClientOriginalName(); $class = \get_class($model); - $count = $model->user->attachments()->where('md5', $md5)->where('attachable_id', $model->id)->where('attachable_type', $class)->count(); - + /** @noinspection PhpUndefinedFieldInspection */ + $count = $model->user->attachments()->where('md5', $md5)->where('attachable_id', $model->id)->where('attachable_type', $class)->count(); + $result = false; if ($count > 0) { $msg = (string)trans('validation.file_already_attached', ['name' => $name]); $this->errors->add('attachments', $msg); Log::error($msg); - - return true; + $result = true; } - return false; + return $result; } /** + * Process the upload of a file. + * * @param UploadedFile $file * @param Model $model * - * @return Attachment + * @return Attachment|null * @throws \Illuminate\Contracts\Encryption\EncryptException */ - protected function processFile(UploadedFile $file, Model $model): Attachment + protected function processFile(UploadedFile $file, Model $model): ?Attachment { Log::debug('Now in processFile()'); $validation = $this->validateUpload($file, $model); - if (false === $validation) { - return new Attachment; + $attachment = null; + if (false !== $validation) { + $attachment = new Attachment; // create Attachment object. + /** @noinspection PhpUndefinedFieldInspection */ + $attachment->user()->associate($model->user); + $attachment->attachable()->associate($model); + $attachment->md5 = md5_file($file->getRealPath()); + $attachment->filename = $file->getClientOriginalName(); + $attachment->mime = $file->getMimeType(); + $attachment->size = $file->getSize(); + $attachment->uploaded = 0; + $attachment->save(); + Log::debug('Created attachment:', $attachment->toArray()); + + $fileObject = $file->openFile('r'); + $fileObject->rewind(); + $content = $fileObject->fread($file->getSize()); + $encrypted = Crypt::encrypt($content); + Log::debug(sprintf('Full file length is %d and upload size is %d.', \strlen($content), $file->getSize())); + Log::debug(sprintf('Encrypted content is %d', \strlen($encrypted))); + + // store it: + $this->uploadDisk->put($attachment->fileName(), $encrypted); + $attachment->uploaded = 1; // update attachment + $attachment->save(); + $this->attachments->push($attachment); + + $name = e($file->getClientOriginalName()); // add message: + $msg = (string)trans('validation.file_attached', ['name' => $name]); + $this->messages->add('attachments', $msg); } - $attachment = new Attachment; // create Attachment object. - $attachment->user()->associate($model->user); - $attachment->attachable()->associate($model); - $attachment->md5 = md5_file($file->getRealPath()); - $attachment->filename = $file->getClientOriginalName(); - $attachment->mime = $file->getMimeType(); - $attachment->size = $file->getSize(); - $attachment->uploaded = 0; - $attachment->save(); - Log::debug('Created attachment:', $attachment->toArray()); - - $fileObject = $file->openFile('r'); - $fileObject->rewind(); - $content = $fileObject->fread($file->getSize()); - $encrypted = Crypt::encrypt($content); - Log::debug(sprintf('Full file length is %d and upload size is %d.', \strlen($content), $file->getSize())); - Log::debug(sprintf('Encrypted content is %d', \strlen($encrypted))); - - // store it: - $this->uploadDisk->put($attachment->fileName(), $encrypted); - $attachment->uploaded = 1; // update attachment - $attachment->save(); - $this->attachments->push($attachment); - - $name = e($file->getClientOriginalName()); // add message: - $msg = (string)trans('validation.file_attached', ['name' => $name]); - $this->messages->add('attachments', $msg); - - // return it. return $attachment; } /** + * Verify if the mime of a file is valid. + * * @param UploadedFile $file * * @return bool @@ -271,19 +288,22 @@ class AttachmentHelper implements AttachmentHelperInterface $name = e($file->getClientOriginalName()); Log::debug(sprintf('Name is %s, and mime is %s', $name, $mime)); Log::debug('Valid mimes are', $this->allowedMimes); + $result = true; if (!\in_array($mime, $this->allowedMimes, true)) { $msg = (string)trans('validation.file_invalid_mime', ['name' => $name, 'mime' => $mime]); $this->errors->add('attachments', $msg); Log::error($msg); - return false; + $result = false; } - return true; + return $result; } /** + * Verify if the size of a file is valid. + * * @codeCoverageIgnore * * @param UploadedFile $file @@ -292,20 +312,23 @@ class AttachmentHelper implements AttachmentHelperInterface */ protected function validSize(UploadedFile $file): bool { - $size = $file->getSize(); - $name = e($file->getClientOriginalName()); + $size = $file->getSize(); + $name = e($file->getClientOriginalName()); + $result = true; if ($size > $this->maxUploadSize) { $msg = (string)trans('validation.file_too_large', ['name' => $name]); $this->errors->add('attachments', $msg); Log::error($msg); - return false; + $result = false; } - return true; + return $result; } /** + * Verify if the file was uploaded correctly. + * * @param UploadedFile $file * @param Model $model * @@ -314,16 +337,17 @@ class AttachmentHelper implements AttachmentHelperInterface protected function validateUpload(UploadedFile $file, Model $model): bool { Log::debug('Now in validateUpload()'); + $result = true; if (!$this->validMime($file)) { - return false; + $result = false; } - if (!$this->validSize($file)) { - return false; // @codeCoverageIgnore + if (true === $result && !$this->validSize($file)) { + $result = false; } - if ($this->hasFile($file, $model)) { - return false; + if (true === $result && $this->hasFile($file, $model)) { + $result = false; } - return true; + return $result; } } diff --git a/app/Helpers/Attachments/AttachmentHelperInterface.php b/app/Helpers/Attachments/AttachmentHelperInterface.php index 8f34041faa..7305202738 100644 --- a/app/Helpers/Attachments/AttachmentHelperInterface.php +++ b/app/Helpers/Attachments/AttachmentHelperInterface.php @@ -33,6 +33,8 @@ use Illuminate\Support\MessageBag; interface AttachmentHelperInterface { /** + * Get content of an attachment. + * * @param Attachment $attachment * * @return string @@ -40,6 +42,8 @@ interface AttachmentHelperInterface public function getAttachmentContent(Attachment $attachment): string; /** + * Get the location of an attachment. + * * @param Attachment $attachment * * @return string @@ -47,16 +51,22 @@ interface AttachmentHelperInterface public function getAttachmentLocation(Attachment $attachment): string; /** + * Get all attachments. + * * @return Collection */ public function getAttachments(): Collection; /** + * Get all errors. + * * @return MessageBag */ public function getErrors(): MessageBag; /** + * Get all messages/ + * * @return MessageBag */ public function getMessages(): MessageBag; @@ -72,6 +82,8 @@ interface AttachmentHelperInterface public function saveAttachmentFromApi(Attachment $attachment, string $content): bool; /** + * Save attachments that got uploaded. + * * @param Model $model * @param null|array $files * diff --git a/app/Helpers/Chart/MetaPieChart.php b/app/Helpers/Chart/MetaPieChart.php index fb60b3db80..e7326b9d35 100644 --- a/app/Helpers/Chart/MetaPieChart.php +++ b/app/Helpers/Chart/MetaPieChart.php @@ -18,6 +18,7 @@ * You should have received a copy of the GNU General Public License * along with Firefly III. If not, see . */ + declare(strict_types=1); namespace FireflyIII\Helpers\Chart; @@ -44,6 +45,14 @@ use Steam; */ class MetaPieChart implements MetaPieChartInterface { + /** @var array */ + static protected $grouping + = [ + 'account' => ['opposing_account_id'], + 'budget' => ['transaction_journal_budget_id', 'transaction_budget_id'], + 'category' => ['transaction_journal_category_id', 'transaction_category_id'], + 'tag' => [], + ]; /** @var Collection */ protected $accounts; /** @var Collection */ @@ -55,14 +64,6 @@ class MetaPieChart implements MetaPieChartInterface /** @var Carbon */ protected $end; /** @var array */ - protected $grouping - = [ - 'account' => ['opposing_account_id'], - 'budget' => ['transaction_journal_budget_id', 'transaction_budget_id'], - 'category' => ['transaction_journal_category_id', 'transaction_category_id'], - 'tag' => [], - ]; - /** @var array */ protected $repositories = [ 'account' => AccountRepositoryInterface::class, @@ -95,13 +96,12 @@ class MetaPieChart implements MetaPieChartInterface * @param string $group * * @return array - * * @SuppressWarnings(PHPMD.CyclomaticComplexity) */ public function generate(string $direction, string $group): array { $transactions = $this->getTransactions($direction); - $grouped = $this->groupByFields($transactions, $this->grouping[$group]); + $grouped = $this->groupByFields($transactions, self::$grouping[$group]); $chartData = $this->organizeByType($group, $grouped); $key = (string)trans('firefly.everything_else'); @@ -295,24 +295,28 @@ class MetaPieChart implements MetaPieChartInterface * @return array * * @SuppressWarnings(PHPMD.CyclomaticComplexity) + * */ protected function groupByFields(Collection $set, array $fields): array { + $grouped = []; if (0 === \count($fields) && $this->tags->count() > 0) { // do a special group on tags: - return $this->groupByTag($set); // @codeCoverageIgnore + $grouped = $this->groupByTag($set); // @codeCoverageIgnore } - $grouped = []; - /** @var Transaction $transaction */ - foreach ($set as $transaction) { - $values = []; - foreach ($fields as $field) { - $values[] = (int)$transaction->$field; + if (0 !== \count($fields) || $this->tags->count() <= 0) { + $grouped = []; + /** @var Transaction $transaction */ + foreach ($set as $transaction) { + $values = []; + foreach ($fields as $field) { + $values[] = (int)$transaction->$field; + } + $value = max($values); + $grouped[$value] = $grouped[$value] ?? '0'; + $grouped[$value] = bcadd($transaction->transaction_amount, $grouped[$value]); } - $value = max($values); - $grouped[$value] = $grouped[$value] ?? '0'; - $grouped[$value] = bcadd($transaction->transaction_amount, $grouped[$value]); } return $grouped; diff --git a/app/Helpers/Collection/Balance.php b/app/Helpers/Collection/Balance.php index 14e2f67f2b..c83e190ac8 100644 --- a/app/Helpers/Collection/Balance.php +++ b/app/Helpers/Collection/Balance.php @@ -46,7 +46,7 @@ class Balance /** * @param BalanceLine $line */ - public function addBalanceLine(BalanceLine $line) + public function addBalanceLine(BalanceLine $line): void { $this->balanceLines->push($line); } @@ -62,7 +62,7 @@ class Balance /** * @param BalanceHeader $balanceHeader */ - public function setBalanceHeader(BalanceHeader $balanceHeader) + public function setBalanceHeader(BalanceHeader $balanceHeader): void { $this->balanceHeader = $balanceHeader; } @@ -78,7 +78,7 @@ class Balance /** * @param Collection $balanceLines */ - public function setBalanceLines(Collection $balanceLines) + public function setBalanceLines(Collection $balanceLines): void { $this->balanceLines = $balanceLines; } diff --git a/app/Helpers/Collection/BalanceEntry.php b/app/Helpers/Collection/BalanceEntry.php index 31b7062698..75ae5419b0 100644 --- a/app/Helpers/Collection/BalanceEntry.php +++ b/app/Helpers/Collection/BalanceEntry.php @@ -47,7 +47,7 @@ class BalanceEntry /** * @param AccountModel $account */ - public function setAccount(AccountModel $account) + public function setAccount(AccountModel $account): void { $this->account = $account; } @@ -63,7 +63,7 @@ class BalanceEntry /** * @param string $left */ - public function setLeft(string $left) + public function setLeft(string $left): void { $this->left = $left; } @@ -79,7 +79,7 @@ class BalanceEntry /** * @param string $spent */ - public function setSpent(string $spent) + public function setSpent(string $spent): void { $this->spent = $spent; } diff --git a/app/Helpers/Collection/BalanceHeader.php b/app/Helpers/Collection/BalanceHeader.php index 3d5aa6476f..e3e809b532 100644 --- a/app/Helpers/Collection/BalanceHeader.php +++ b/app/Helpers/Collection/BalanceHeader.php @@ -44,7 +44,7 @@ class BalanceHeader /** * @param AccountModel $account */ - public function addAccount(AccountModel $account) + public function addAccount(AccountModel $account): void { $this->accounts->push($account); } diff --git a/app/Helpers/Collection/BalanceLine.php b/app/Helpers/Collection/BalanceLine.php index 9d62a4aa21..a679ef66ff 100644 --- a/app/Helpers/Collection/BalanceLine.php +++ b/app/Helpers/Collection/BalanceLine.php @@ -62,7 +62,7 @@ class BalanceLine /** * @param BalanceEntry $balanceEntry */ - public function addBalanceEntry(BalanceEntry $balanceEntry) + public function addBalanceEntry(BalanceEntry $balanceEntry): void { $this->balanceEntries->push($balanceEntry); } @@ -78,7 +78,7 @@ class BalanceLine /** * @param Collection $balanceEntries */ - public function setBalanceEntries(Collection $balanceEntries) + public function setBalanceEntries(Collection $balanceEntries): void { $this->balanceEntries = $balanceEntries; } @@ -94,7 +94,7 @@ class BalanceLine /** * @param BudgetModel $budget */ - public function setBudget(BudgetModel $budget) + public function setBudget(BudgetModel $budget): void { $this->budget = $budget; } @@ -110,7 +110,7 @@ class BalanceLine /** * @param BudgetLimit $budgetLimit */ - public function setBudgetLimit(BudgetLimit $budgetLimit) + public function setBudgetLimit(BudgetLimit $budgetLimit): void { $this->budgetLimit = $budgetLimit; } @@ -118,7 +118,7 @@ class BalanceLine /** * @return Carbon */ - public function getEndDate() + public function getEndDate(): Carbon { return $this->budgetLimit->end_date ?? new Carbon; } @@ -134,7 +134,7 @@ class BalanceLine /** * @param int $role */ - public function setRole(int $role) + public function setRole(int $role): void { $this->role = $role; } @@ -142,29 +142,28 @@ class BalanceLine /** * @return Carbon */ - public function getStartDate() + public function getStartDate(): Carbon { return $this->budgetLimit->start_date ?? new Carbon; } /** - * @SuppressWarnings(PHPMD.CyclomaticComplexity) - * * @return string */ public function getTitle(): string { + $title = ''; if ($this->getBudget() instanceof BudgetModel && null !== $this->getBudget()->id) { - return $this->getBudget()->name; + $title = $this->getBudget()->name; } - if (self::ROLE_DEFAULTROLE === $this->getRole()) { - return (string)trans('firefly.no_budget'); + if ('' === $title && self::ROLE_DEFAULTROLE === $this->getRole()) { + $title = (string)trans('firefly.no_budget'); } - if (self::ROLE_TAGROLE === $this->getRole()) { - return (string)trans('firefly.coveredWithTags'); + if ('' === $title && self::ROLE_TAGROLE === $this->getRole()) { + $title = (string)trans('firefly.coveredWithTags'); } - return ''; + return $title; } /** diff --git a/app/Helpers/Collection/Bill.php b/app/Helpers/Collection/Bill.php index 1cc2a7b50e..c776819d84 100644 --- a/app/Helpers/Collection/Bill.php +++ b/app/Helpers/Collection/Bill.php @@ -52,7 +52,7 @@ class Bill /** * @param BillLine $bill */ - public function addBill(BillLine $bill) + public function addBill(BillLine $bill): void { $this->bills->push($bill); } @@ -60,7 +60,7 @@ class Bill /** * */ - public function filterBills() + public function filterBills(): void { Log::debug('Now in filterBills()'); /** @var BillRepositoryInterface $repository */ @@ -114,7 +114,7 @@ class Bill /** * @param Carbon $endDate */ - public function setEndDate(Carbon $endDate) + public function setEndDate(Carbon $endDate): void { $this->endDate = $endDate; } @@ -122,7 +122,7 @@ class Bill /** * @param Carbon $startDate */ - public function setStartDate(Carbon $startDate) + public function setStartDate(Carbon $startDate): void { $this->startDate = $startDate; } diff --git a/app/Helpers/Collection/BillLine.php b/app/Helpers/Collection/BillLine.php index ed5bcbdaf1..c9341626ef 100644 --- a/app/Helpers/Collection/BillLine.php +++ b/app/Helpers/Collection/BillLine.php @@ -68,7 +68,7 @@ class BillLine /** * @param string $amount */ - public function setAmount(string $amount) + public function setAmount(string $amount): void { $this->amount = $amount; } @@ -84,7 +84,7 @@ class BillLine /** * @param BillModel $bill */ - public function setBill(BillModel $bill) + public function setBill(BillModel $bill): void { $this->bill = $bill; } @@ -116,7 +116,7 @@ class BillLine /** * @param Carbon $lastHitDate */ - public function setLastHitDate(Carbon $lastHitDate) + public function setLastHitDate(Carbon $lastHitDate): void { $this->lastHitDate = $lastHitDate; } @@ -132,7 +132,7 @@ class BillLine /** * @param string $max */ - public function setMax(string $max) + public function setMax(string $max): void { $this->max = $max; } @@ -148,7 +148,7 @@ class BillLine /** * @param string $min */ - public function setMin(string $min) + public function setMin(string $min): void { $this->min = $min; } @@ -180,7 +180,7 @@ class BillLine /** * @param int $transactionJournalId */ - public function setTransactionJournalId(int $transactionJournalId) + public function setTransactionJournalId(int $transactionJournalId): void { $this->transactionJournalId = $transactionJournalId; } @@ -204,7 +204,7 @@ class BillLine /** * @param bool $hit */ - public function setHit(bool $hit) + public function setHit(bool $hit): void { $this->hit = $hit; } diff --git a/app/Helpers/Collection/Category.php b/app/Helpers/Collection/Category.php index 12ed91b8e8..93608a7f1d 100644 --- a/app/Helpers/Collection/Category.php +++ b/app/Helpers/Collection/Category.php @@ -46,19 +46,19 @@ class Category /** * @param CategoryModel $category */ - public function addCategory(CategoryModel $category) + public function addCategory(CategoryModel $category): void { // spent is minus zero for an expense report: if ($category->spent < 0) { $this->categories->push($category); - $this->addTotal($category->spent); + $this->addTotal((string)$category->spent); } } /** * @param string $add */ - public function addTotal(string $add) + public function addTotal(string $add): void { $this->total = bcadd($this->total, $add); } diff --git a/app/Helpers/Collector/JournalCollector.php b/app/Helpers/Collector/JournalCollector.php index 33d24fcaf7..5f910f837f 100644 --- a/app/Helpers/Collector/JournalCollector.php +++ b/app/Helpers/Collector/JournalCollector.php @@ -18,6 +18,8 @@ * You should have received a copy of the GNU General Public License * along with Firefly III. If not, see . */ +/** @noinspection PhpDynamicAsStaticMethodCallInspection */ +/** @noinspection PropertyCanBeStaticInspection */ declare(strict_types=1); namespace FireflyIII\Helpers\Collector; @@ -50,14 +52,14 @@ use Log; use Steam; /** + * TODO rename references to journals to transactions * Maybe this is a good idea after all... * * Class JournalCollector - * - * + * @SuppressWarnings(PHPMD.TooManyPublicMethods) * @SuppressWarnings(PHPMD.ExcessiveClassComplexity) * @SuppressWarnings(PHPMD.CouplingBetweenObjects) - * @SuppressWarnings(PHPMD.TooManyPublicMethods) + * */ class JournalCollector implements JournalCollectorInterface { @@ -66,6 +68,7 @@ class JournalCollector implements JournalCollectorInterface private $accountIds = []; /** @var int */ private $count = 0; + /** @var array */ private $fields = [ @@ -139,7 +142,7 @@ class JournalCollector implements JournalCollectorInterface public function addFilter(string $filter): JournalCollectorInterface { $interfaces = class_implements($filter); - if (\in_array(FilterInterface::class, $interfaces) && !\in_array($filter, $this->filters)) { + if (\in_array(FilterInterface::class, $interfaces, true) && !\in_array($filter, $this->filters, true)) { Log::debug(sprintf('Enabled filter %s', $filter)); $this->filters[] = $filter; } @@ -245,8 +248,11 @@ class JournalCollector implements JournalCollectorInterface return $this->count; } + /** @noinspection MultipleReturnStatementsInspection */ /** * @return Collection + * @SuppressWarnings(PHPMD.CyclomaticComplexity) + * @SuppressWarnings(PHPMD.ExcessiveMethodLength) */ public function getJournals(): Collection { @@ -266,9 +272,6 @@ class JournalCollector implements JournalCollectorInterface return $cache->get(); // @codeCoverageIgnore } - if (true === $this->ignoreCache) { - Log::debug('Ignore cache in journal collector.'); - } /** @var Collection $set */ $set = $this->query->get(array_values($this->fields)); @@ -468,18 +471,17 @@ class JournalCollector implements JournalCollectorInterface public function setBudgets(Collection $budgets): JournalCollectorInterface { $budgetIds = $budgets->pluck('id')->toArray(); - if (0 === \count($budgetIds)) { - return $this; - } - $this->joinBudgetTables(); - Log::debug('Journal collector will filter for budgets', $budgetIds); + if (0 !== \count($budgetIds)) { + $this->joinBudgetTables(); + Log::debug('Journal collector will filter for budgets', $budgetIds); - $this->query->where( - function (EloquentBuilder $q) use ($budgetIds) { - $q->whereIn('budget_transaction.budget_id', $budgetIds); - $q->orWhereIn('budget_transaction_journal.budget_id', $budgetIds); - } - ); + $this->query->where( + function (EloquentBuilder $q) use ($budgetIds) { + $q->whereIn('budget_transaction.budget_id', $budgetIds); + $q->orWhereIn('budget_transaction_journal.budget_id', $budgetIds); + } + ); + } return $this; } @@ -492,17 +494,16 @@ class JournalCollector implements JournalCollectorInterface public function setCategories(Collection $categories): JournalCollectorInterface { $categoryIds = $categories->pluck('id')->toArray(); - if (0 === \count($categoryIds)) { - return $this; - } - $this->joinCategoryTables(); + if (0 !== \count($categoryIds)) { + $this->joinCategoryTables(); - $this->query->where( - function (EloquentBuilder $q) use ($categoryIds) { - $q->whereIn('category_transaction.category_id', $categoryIds); - $q->orWhereIn('category_transaction_journal.category_id', $categoryIds); - } - ); + $this->query->where( + function (EloquentBuilder $q) use ($categoryIds) { + $q->whereIn('category_transaction.category_id', $categoryIds); + $q->orWhereIn('category_transaction_journal.category_id', $categoryIds); + } + ); + } return $this; } @@ -606,10 +607,7 @@ class JournalCollector implements JournalCollectorInterface $this->offset = $offset; $this->query->skip($offset); Log::debug(sprintf('Changed offset to %d', $offset)); - - return $this; } - Log::debug('The limit is zero, cannot set the page.'); return $this; } @@ -688,7 +686,7 @@ class JournalCollector implements JournalCollectorInterface /** * */ - public function startQuery() + public function startQuery(): void { Log::debug('journalCollector::startQuery'); /** @var EloquentBuilder $query */ @@ -798,6 +796,7 @@ class JournalCollector implements JournalCollectorInterface foreach ($this->filters as $enabled) { if (isset($filters[$enabled])) { Log::debug(sprintf('Before filter %s: %d', $enabled, $set->count())); + /** @var Collection $set */ $set = $filters[$enabled]->filter($set); Log::debug(sprintf('After filter %s: %d', $enabled, $set->count())); } diff --git a/app/Helpers/Filter/InternalTransferFilter.php b/app/Helpers/Filter/InternalTransferFilter.php index c36e5f1353..cf163c1b57 100644 --- a/app/Helpers/Filter/InternalTransferFilter.php +++ b/app/Helpers/Filter/InternalTransferFilter.php @@ -61,7 +61,7 @@ class InternalTransferFilter implements FilterInterface return $transaction; } // both id's in $parameters? - if (\in_array($transaction->account_id, $this->accounts) && \in_array($transaction->opposing_account_id, $this->accounts)) { + if (\in_array($transaction->account_id, $this->accounts, true) && \in_array($transaction->opposing_account_id, $this->accounts, true)) { Log::debug( sprintf( 'Transaction #%d has #%d and #%d in set, so removed', diff --git a/app/Helpers/Filter/OpposingAccountFilter.php b/app/Helpers/Filter/OpposingAccountFilter.php index 7868f67556..cdafd96fca 100644 --- a/app/Helpers/Filter/OpposingAccountFilter.php +++ b/app/Helpers/Filter/OpposingAccountFilter.php @@ -58,7 +58,7 @@ class OpposingAccountFilter implements FilterInterface function (Transaction $transaction) { $opposing = $transaction->opposing_account_id; // remove internal transfer - if (\in_array($opposing, $this->accounts)) { + if (\in_array($opposing, $this->accounts, true)) { Log::debug(sprintf('Filtered #%d because its opposite is in accounts.', $transaction->id), $this->accounts); return null; diff --git a/app/Helpers/Filter/TransactionViewFilter.php b/app/Helpers/Filter/TransactionViewFilter.php index ea83a62e75..7d5bdb0b91 100644 --- a/app/Helpers/Filter/TransactionViewFilter.php +++ b/app/Helpers/Filter/TransactionViewFilter.php @@ -40,7 +40,7 @@ class TransactionViewFilter implements FilterInterface { /** * @param Collection $set - * + * @SuppressWarnings(PHPMD.CyclomaticComplexity) * @return Collection */ public function filter(Collection $set): Collection diff --git a/app/Helpers/FiscalHelper.php b/app/Helpers/FiscalHelper.php index 10130eb16f..b34de8da9f 100644 --- a/app/Helpers/FiscalHelper.php +++ b/app/Helpers/FiscalHelper.php @@ -54,10 +54,10 @@ class FiscalHelper implements FiscalHelperInterface // add 1 year and sub 1 day $endDate->addYear(); $endDate->subDay(); - - return $endDate; } - $endDate->endOfYear(); + if (false === $this->useCustomFiscalYear) { + $endDate->endOfYear(); + } return $endDate; } @@ -80,10 +80,10 @@ class FiscalHelper implements FiscalHelperInterface if ($startDate > $date) { $startDate->subYear(); } - - return $startDate; } - $startDate->startOfYear(); + if (false === $this->useCustomFiscalYear) { + $startDate->startOfYear(); + } return $startDate; } diff --git a/app/Helpers/Help/Help.php b/app/Helpers/Help/Help.php index a9fcd1869a..3b8cce631b 100644 --- a/app/Helpers/Help/Help.php +++ b/app/Helpers/Help/Help.php @@ -65,22 +65,19 @@ class Help implements HelpInterface { $uri = sprintf('https://raw.githubusercontent.com/firefly-iii/help/master/%s/%s.md', $language, $route); Log::debug(sprintf('Trying to get %s...', $uri)); - $opt = ['headers' => ['User-Agent' => $this->userAgent]]; - $content = ''; + $opt = ['headers' => ['User-Agent' => $this->userAgent]]; + $content = ''; + $statusCode = 500; + $client = new Client; try { - $client = new Client; - $res = $client->request('GET', $uri, $opt); + $res = $client->request('GET', $uri, $opt); + $statusCode = $res->getStatusCode(); + $content = trim($res->getBody()->getContents()); } catch (GuzzleException|Exception $e) { Log::error($e); - - return ''; } - Log::debug(sprintf('Status code is %d', $res->getStatusCode())); - - if (200 === $res->getStatusCode()) { - $content = trim($res->getBody()->getContents()); - } + Log::debug(sprintf('Status code is %d', $statusCode)); if (\strlen($content) > 0) { Log::debug('Content is longer than zero. Expect something.'); @@ -126,7 +123,7 @@ class Help implements HelpInterface * @param string $language * @param string $content */ - public function putInCache(string $route, string $language, string $content) + public function putInCache(string $route, string $language, string $content): void { $key = sprintf(self::CACHEKEY, $route, $language); if (\strlen($content) > 0) { diff --git a/app/Helpers/Report/BalanceReportHelper.php b/app/Helpers/Report/BalanceReportHelper.php index f44dfc15a4..09072bbb46 100644 --- a/app/Helpers/Report/BalanceReportHelper.php +++ b/app/Helpers/Report/BalanceReportHelper.php @@ -34,8 +34,6 @@ use Log; /** * Class BalanceReportHelper. - * - * @SuppressWarnings(PHPMD.CouplingBetweenObjects) // I can't really help it. */ class BalanceReportHelper implements BalanceReportHelperInterface { @@ -145,8 +143,8 @@ class BalanceReportHelper implements BalanceReportHelperInterface } /** + * @SuppressWarnings(PHPMD.CyclomaticComplexity) * @param Balance $balance - * @SuppressWarnings(PHPMD.CyclomaticComplexity) // it's exactly 5. * * @return Balance */ @@ -157,6 +155,7 @@ class BalanceReportHelper implements BalanceReportHelperInterface foreach ($set as $entry) { if (null !== $entry->getBudget()->id) { $sum = '0'; + /** @var BalanceEntry $balanceEntry */ foreach ($entry->getBalanceEntries() as $balanceEntry) { $sum = bcadd($sum, $balanceEntry->getSpent()); } diff --git a/app/Helpers/Report/BudgetReportHelper.php b/app/Helpers/Report/BudgetReportHelper.php index 7e8382d84f..66fa8bf1fa 100644 --- a/app/Helpers/Report/BudgetReportHelper.php +++ b/app/Helpers/Report/BudgetReportHelper.php @@ -47,9 +47,8 @@ class BudgetReportHelper implements BudgetReportHelperInterface } /** - * @SuppressWarnings(PHPMD.CyclomaticComplexity) // it's exactly 5. - * @SuppressWarnings(PHPMD.ExcessiveMethodLength) // all the arrays make it long. - * + * @SuppressWarnings(PHPMD.CyclomaticComplexity) + * @SuppressWarnings(PHPMD.ExcessiveMethodLength) * @param Carbon $start * @param Carbon $end * @param Collection $accounts diff --git a/app/Helpers/Report/PopupReport.php b/app/Helpers/Report/PopupReport.php index 24c1bd2d9e..e0eccc2c62 100644 --- a/app/Helpers/Report/PopupReport.php +++ b/app/Helpers/Report/PopupReport.php @@ -119,6 +119,10 @@ class PopupReport implements PopupReportInterface */ public function byExpenses(Account $account, array $attributes): Collection { + /** @var JournalRepositoryInterface $repository */ + $repository = app(JournalRepositoryInterface::class); + $repository->setUser($account->user); + /** @var JournalCollectorInterface $collector */ $collector = app(JournalCollectorInterface::class); @@ -130,9 +134,9 @@ class PopupReport implements PopupReportInterface // filter for transfers and withdrawals TO the given $account $journals = $journals->filter( - function (Transaction $transaction) use ($report) { + function (Transaction $transaction) use ($report, $repository) { // get the destinations: - $sources = $transaction->transactionJournal->sourceAccountList()->pluck('id')->toArray(); + $sources = $repository->getJournalSourceAccounts($transaction->transactionJournal)->pluck('id')->toArray(); // do these intersect with the current list? return !empty(array_intersect($report, $sources)); diff --git a/app/Helpers/Report/ReportHelper.php b/app/Helpers/Report/ReportHelper.php index 1365e1a27d..c690787630 100644 --- a/app/Helpers/Report/ReportHelper.php +++ b/app/Helpers/Report/ReportHelper.php @@ -56,10 +56,11 @@ class ReportHelper implements ReportHelperInterface * This method generates a full report for the given period on all * the users bills and their payments. * - * @SuppressWarnings(PHPMD.CyclomaticComplexity) // it's exactly 5. - * * Excludes bills which have not had a payment on the mentioned accounts. * + * @SuppressWarnings(PHPMD.CyclomaticComplexity) + * @SuppressWarnings(PHPMD.ExcessiveMethodLength) + * * @param Carbon $start * @param Carbon $end * @param Collection $accounts @@ -82,6 +83,7 @@ class ReportHelper implements ReportHelperInterface foreach ($expectedDates as $payDate) { $endOfPayPeriod = app('navigation')->endOfX($payDate, $bill->repeat_freq, null); + /** @var JournalCollectorInterface $collector */ $collector = app(JournalCollectorInterface::class); $collector->setAccounts($accounts)->setRange($payDate, $endOfPayPeriod)->setBills($bills); $journals = $collector->getJournals(); diff --git a/app/Models/Category.php b/app/Models/Category.php index 5f06f91fbe..24457e0d74 100644 --- a/app/Models/Category.php +++ b/app/Models/Category.php @@ -34,6 +34,7 @@ use Symfony\Component\HttpKernel\Exception\NotFoundHttpException; * * @property string $name * @property int $id + * @property float $spent // used in category reports */ class Category extends Model { diff --git a/app/Models/Transaction.php b/app/Models/Transaction.php index 4fd1f18d25..58b3ac5e9a 100644 --- a/app/Models/Transaction.php +++ b/app/Models/Transaction.php @@ -83,8 +83,11 @@ use Symfony\Component\HttpKernel\Exception\NotFoundHttpException; * @property TransactionCurrency $transactionCurrency * @property int $transaction_journal_id * @property TransactionCurrency $foreignCurrency - * @property string $before // used in audit reports. - * @property string $after // used in audit reports. + * @property string $before // used in audit reports. + * @property string $after // used in audit reports. + * @property int $opposing_id // ID of the opposing transaction, used in collector + * @property bool $encrypted // is the journal encrypted + * @property $bill_name_encrypted */ class Transaction extends Model { diff --git a/app/Transformers/LinkTypeTransformer.php b/app/Transformers/LinkTypeTransformer.php index 625d390c09..080fb2a52e 100644 --- a/app/Transformers/LinkTypeTransformer.php +++ b/app/Transformers/LinkTypeTransformer.php @@ -28,6 +28,10 @@ use FireflyIII\Models\LinkType; use League\Fractal\TransformerAbstract; use Symfony\Component\HttpFoundation\ParameterBag; +/** + * + * Class LinkTypeTransformer + */ class LinkTypeTransformer extends TransformerAbstract { @@ -75,7 +79,7 @@ class LinkTypeTransformer extends TransformerAbstract 'name' => $linkType->name, 'inward' => $linkType->inward, 'outward' => $linkType->outward, - 'editable' => (int)$linkType->editable, + 'editable' => 1 === (int)$linkType->editable, 'links' => [ [ 'rel' => 'self',