Code quality update.

This commit is contained in:
James Cole
2018-07-07 21:17:46 +02:00
parent cbe47a9dcc
commit e78a59a8a8
26 changed files with 236 additions and 185 deletions

View File

@@ -55,6 +55,7 @@ class AvailableBudgetRequest extends Request
} }
/** /**
* TODO must also accept currency code.
* The rules that the incoming request must be matched against. * The rules that the incoming request must be matched against.
* *
* @return array * @return array

View File

@@ -68,7 +68,7 @@ class CurrencyRequest extends Request
'code' => 'required|between:3,3|unique:transaction_currencies,code', 'code' => 'required|between:3,3|unique:transaction_currencies,code',
'symbol' => 'required|between:1,5|unique:transaction_currencies,symbol', 'symbol' => 'required|between:1,5|unique:transaction_currencies,symbol',
'decimal_places' => 'required|between:0,20|numeric|min:0|max:20', 'decimal_places' => 'required|between:0,20|numeric|min:0|max:20',
'default' => 'in:true,false', 'default' => 'boolean',
]; ];
switch ($this->method()) { switch ($this->method()) {

View File

@@ -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. * The rules that the incoming request must be matched against.
* *
* @return array * @return array

View File

@@ -38,18 +38,18 @@ use Symfony\Component\HttpFoundation\File\UploadedFile;
*/ */
class AttachmentHelper implements AttachmentHelperInterface class AttachmentHelper implements AttachmentHelperInterface
{ {
/** @var Collection */ /** @var Collection All attachments */
public $attachments; public $attachments;
/** @var MessageBag */ /** @var MessageBag All errors */
public $errors; public $errors;
/** @var MessageBag */ /** @var MessageBag All messages */
public $messages; public $messages;
/** @var array */ /** @var array Allowed mimes */
protected $allowedMimes = []; protected $allowedMimes = [];
/** @var int */ /** @var int Max upload size. */
protected $maxUploadSize = 0; protected $maxUploadSize = 0;
/** @var \Illuminate\Contracts\Filesystem\Filesystem */ /** @var \Illuminate\Contracts\Filesystem\Filesystem The disk where attachments are stored. */
protected $uploadDisk; protected $uploadDisk;
@@ -67,6 +67,8 @@ class AttachmentHelper implements AttachmentHelperInterface
} }
/** /**
* Returns the content of an attachment.
*
* @codeCoverageIgnore * @codeCoverageIgnore
* *
* @param Attachment $attachment * @param Attachment $attachment
@@ -75,18 +77,19 @@ class AttachmentHelper implements AttachmentHelperInterface
*/ */
public function getAttachmentContent(Attachment $attachment): string public function getAttachmentContent(Attachment $attachment): string
{ {
$content = '';
try { try {
$content = Crypt::decrypt($this->uploadDisk->get(sprintf('at-%d.data', $attachment->id))); $content = Crypt::decrypt($this->uploadDisk->get(sprintf('at-%d.data', $attachment->id)));
} catch (DecryptException|FileNotFoundException $e) { } catch (DecryptException|FileNotFoundException $e) {
Log::error(sprintf('Could not decrypt data of attachment #%d', $attachment->id)); Log::error(sprintf('Could not decrypt data of attachment #%d: %s', $attachment->id, $e->getMessage()));
return '';
} }
return $content; return $content;
} }
/** /**
* Returns the file location for an attachment,
*
* @param Attachment $attachment * @param Attachment $attachment
* *
* @return string * @return string
@@ -99,6 +102,8 @@ class AttachmentHelper implements AttachmentHelperInterface
} }
/** /**
* Get all attachments.
*
* @return Collection * @return Collection
*/ */
public function getAttachments(): Collection public function getAttachments(): Collection
@@ -107,6 +112,8 @@ class AttachmentHelper implements AttachmentHelperInterface
} }
/** /**
* Get all errors.
*
* @return MessageBag * @return MessageBag
*/ */
public function getErrors(): MessageBag public function getErrors(): MessageBag
@@ -115,6 +122,8 @@ class AttachmentHelper implements AttachmentHelperInterface
} }
/** /**
* Get all messages.
*
* @return MessageBag * @return MessageBag
*/ */
public function getMessages(): MessageBag public function getMessages(): MessageBag
@@ -122,6 +131,7 @@ class AttachmentHelper implements AttachmentHelperInterface
return $this->messages; return $this->messages;
} }
/** @noinspection MultipleReturnStatementsInspection */
/** /**
* Uploads a file as a string. * 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 Model $model
* @param array|null $files * @param array|null $files
* *
@@ -180,15 +192,17 @@ class AttachmentHelper implements AttachmentHelperInterface
} }
} }
Log::debug('Done processing uploads.'); 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; return true;
} }
/** /**
* Check if a model already has this file attached.
*
* @param UploadedFile $file * @param UploadedFile $file
* @param Model $model * @param Model $model
* *
@@ -199,67 +213,70 @@ class AttachmentHelper implements AttachmentHelperInterface
$md5 = md5_file($file->getRealPath()); $md5 = md5_file($file->getRealPath());
$name = $file->getClientOriginalName(); $name = $file->getClientOriginalName();
$class = \get_class($model); $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) { if ($count > 0) {
$msg = (string)trans('validation.file_already_attached', ['name' => $name]); $msg = (string)trans('validation.file_already_attached', ['name' => $name]);
$this->errors->add('attachments', $msg); $this->errors->add('attachments', $msg);
Log::error($msg); Log::error($msg);
$result = true;
return true;
} }
return false; return $result;
} }
/** /**
* Process the upload of a file.
*
* @param UploadedFile $file * @param UploadedFile $file
* @param Model $model * @param Model $model
* *
* @return Attachment * @return Attachment|null
* @throws \Illuminate\Contracts\Encryption\EncryptException * @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()'); Log::debug('Now in processFile()');
$validation = $this->validateUpload($file, $model); $validation = $this->validateUpload($file, $model);
if (false === $validation) { $attachment = null;
return new Attachment; 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; return $attachment;
} }
/** /**
* Verify if the mime of a file is valid.
*
* @param UploadedFile $file * @param UploadedFile $file
* *
* @return bool * @return bool
@@ -271,19 +288,22 @@ class AttachmentHelper implements AttachmentHelperInterface
$name = e($file->getClientOriginalName()); $name = e($file->getClientOriginalName());
Log::debug(sprintf('Name is %s, and mime is %s', $name, $mime)); Log::debug(sprintf('Name is %s, and mime is %s', $name, $mime));
Log::debug('Valid mimes are', $this->allowedMimes); Log::debug('Valid mimes are', $this->allowedMimes);
$result = true;
if (!\in_array($mime, $this->allowedMimes, true)) { if (!\in_array($mime, $this->allowedMimes, true)) {
$msg = (string)trans('validation.file_invalid_mime', ['name' => $name, 'mime' => $mime]); $msg = (string)trans('validation.file_invalid_mime', ['name' => $name, 'mime' => $mime]);
$this->errors->add('attachments', $msg); $this->errors->add('attachments', $msg);
Log::error($msg); Log::error($msg);
return false; $result = false;
} }
return true; return $result;
} }
/** /**
* Verify if the size of a file is valid.
*
* @codeCoverageIgnore * @codeCoverageIgnore
* *
* @param UploadedFile $file * @param UploadedFile $file
@@ -292,20 +312,23 @@ class AttachmentHelper implements AttachmentHelperInterface
*/ */
protected function validSize(UploadedFile $file): bool protected function validSize(UploadedFile $file): bool
{ {
$size = $file->getSize(); $size = $file->getSize();
$name = e($file->getClientOriginalName()); $name = e($file->getClientOriginalName());
$result = true;
if ($size > $this->maxUploadSize) { if ($size > $this->maxUploadSize) {
$msg = (string)trans('validation.file_too_large', ['name' => $name]); $msg = (string)trans('validation.file_too_large', ['name' => $name]);
$this->errors->add('attachments', $msg); $this->errors->add('attachments', $msg);
Log::error($msg); Log::error($msg);
return false; $result = false;
} }
return true; return $result;
} }
/** /**
* Verify if the file was uploaded correctly.
*
* @param UploadedFile $file * @param UploadedFile $file
* @param Model $model * @param Model $model
* *
@@ -314,16 +337,17 @@ class AttachmentHelper implements AttachmentHelperInterface
protected function validateUpload(UploadedFile $file, Model $model): bool protected function validateUpload(UploadedFile $file, Model $model): bool
{ {
Log::debug('Now in validateUpload()'); Log::debug('Now in validateUpload()');
$result = true;
if (!$this->validMime($file)) { if (!$this->validMime($file)) {
return false; $result = false;
} }
if (!$this->validSize($file)) { if (true === $result && !$this->validSize($file)) {
return false; // @codeCoverageIgnore $result = false;
} }
if ($this->hasFile($file, $model)) { if (true === $result && $this->hasFile($file, $model)) {
return false; $result = false;
} }
return true; return $result;
} }
} }

View File

@@ -33,6 +33,8 @@ use Illuminate\Support\MessageBag;
interface AttachmentHelperInterface interface AttachmentHelperInterface
{ {
/** /**
* Get content of an attachment.
*
* @param Attachment $attachment * @param Attachment $attachment
* *
* @return string * @return string
@@ -40,6 +42,8 @@ interface AttachmentHelperInterface
public function getAttachmentContent(Attachment $attachment): string; public function getAttachmentContent(Attachment $attachment): string;
/** /**
* Get the location of an attachment.
*
* @param Attachment $attachment * @param Attachment $attachment
* *
* @return string * @return string
@@ -47,16 +51,22 @@ interface AttachmentHelperInterface
public function getAttachmentLocation(Attachment $attachment): string; public function getAttachmentLocation(Attachment $attachment): string;
/** /**
* Get all attachments.
*
* @return Collection * @return Collection
*/ */
public function getAttachments(): Collection; public function getAttachments(): Collection;
/** /**
* Get all errors.
*
* @return MessageBag * @return MessageBag
*/ */
public function getErrors(): MessageBag; public function getErrors(): MessageBag;
/** /**
* Get all messages/
*
* @return MessageBag * @return MessageBag
*/ */
public function getMessages(): MessageBag; public function getMessages(): MessageBag;
@@ -72,6 +82,8 @@ interface AttachmentHelperInterface
public function saveAttachmentFromApi(Attachment $attachment, string $content): bool; public function saveAttachmentFromApi(Attachment $attachment, string $content): bool;
/** /**
* Save attachments that got uploaded.
*
* @param Model $model * @param Model $model
* @param null|array $files * @param null|array $files
* *

View File

@@ -18,6 +18,7 @@
* You should have received a copy of the GNU General Public License * You should have received a copy of the GNU General Public License
* along with Firefly III. If not, see <http://www.gnu.org/licenses/>. * along with Firefly III. If not, see <http://www.gnu.org/licenses/>.
*/ */
declare(strict_types=1); declare(strict_types=1);
namespace FireflyIII\Helpers\Chart; namespace FireflyIII\Helpers\Chart;
@@ -44,6 +45,14 @@ use Steam;
*/ */
class MetaPieChart implements MetaPieChartInterface 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 */ /** @var Collection */
protected $accounts; protected $accounts;
/** @var Collection */ /** @var Collection */
@@ -55,14 +64,6 @@ class MetaPieChart implements MetaPieChartInterface
/** @var Carbon */ /** @var Carbon */
protected $end; protected $end;
/** @var array */ /** @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 protected $repositories
= [ = [
'account' => AccountRepositoryInterface::class, 'account' => AccountRepositoryInterface::class,
@@ -95,13 +96,12 @@ class MetaPieChart implements MetaPieChartInterface
* @param string $group * @param string $group
* *
* @return array * @return array
*
* @SuppressWarnings(PHPMD.CyclomaticComplexity) * @SuppressWarnings(PHPMD.CyclomaticComplexity)
*/ */
public function generate(string $direction, string $group): array public function generate(string $direction, string $group): array
{ {
$transactions = $this->getTransactions($direction); $transactions = $this->getTransactions($direction);
$grouped = $this->groupByFields($transactions, $this->grouping[$group]); $grouped = $this->groupByFields($transactions, self::$grouping[$group]);
$chartData = $this->organizeByType($group, $grouped); $chartData = $this->organizeByType($group, $grouped);
$key = (string)trans('firefly.everything_else'); $key = (string)trans('firefly.everything_else');
@@ -295,24 +295,28 @@ class MetaPieChart implements MetaPieChartInterface
* @return array * @return array
* *
* @SuppressWarnings(PHPMD.CyclomaticComplexity) * @SuppressWarnings(PHPMD.CyclomaticComplexity)
*
*/ */
protected function groupByFields(Collection $set, array $fields): array protected function groupByFields(Collection $set, array $fields): array
{ {
$grouped = [];
if (0 === \count($fields) && $this->tags->count() > 0) { if (0 === \count($fields) && $this->tags->count() > 0) {
// do a special group on tags: // do a special group on tags:
return $this->groupByTag($set); // @codeCoverageIgnore $grouped = $this->groupByTag($set); // @codeCoverageIgnore
} }
$grouped = []; if (0 !== \count($fields) || $this->tags->count() <= 0) {
/** @var Transaction $transaction */ $grouped = [];
foreach ($set as $transaction) { /** @var Transaction $transaction */
$values = []; foreach ($set as $transaction) {
foreach ($fields as $field) { $values = [];
$values[] = (int)$transaction->$field; 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; return $grouped;

View File

@@ -46,7 +46,7 @@ class Balance
/** /**
* @param BalanceLine $line * @param BalanceLine $line
*/ */
public function addBalanceLine(BalanceLine $line) public function addBalanceLine(BalanceLine $line): void
{ {
$this->balanceLines->push($line); $this->balanceLines->push($line);
} }
@@ -62,7 +62,7 @@ class Balance
/** /**
* @param BalanceHeader $balanceHeader * @param BalanceHeader $balanceHeader
*/ */
public function setBalanceHeader(BalanceHeader $balanceHeader) public function setBalanceHeader(BalanceHeader $balanceHeader): void
{ {
$this->balanceHeader = $balanceHeader; $this->balanceHeader = $balanceHeader;
} }
@@ -78,7 +78,7 @@ class Balance
/** /**
* @param Collection $balanceLines * @param Collection $balanceLines
*/ */
public function setBalanceLines(Collection $balanceLines) public function setBalanceLines(Collection $balanceLines): void
{ {
$this->balanceLines = $balanceLines; $this->balanceLines = $balanceLines;
} }

View File

@@ -47,7 +47,7 @@ class BalanceEntry
/** /**
* @param AccountModel $account * @param AccountModel $account
*/ */
public function setAccount(AccountModel $account) public function setAccount(AccountModel $account): void
{ {
$this->account = $account; $this->account = $account;
} }
@@ -63,7 +63,7 @@ class BalanceEntry
/** /**
* @param string $left * @param string $left
*/ */
public function setLeft(string $left) public function setLeft(string $left): void
{ {
$this->left = $left; $this->left = $left;
} }
@@ -79,7 +79,7 @@ class BalanceEntry
/** /**
* @param string $spent * @param string $spent
*/ */
public function setSpent(string $spent) public function setSpent(string $spent): void
{ {
$this->spent = $spent; $this->spent = $spent;
} }

View File

@@ -44,7 +44,7 @@ class BalanceHeader
/** /**
* @param AccountModel $account * @param AccountModel $account
*/ */
public function addAccount(AccountModel $account) public function addAccount(AccountModel $account): void
{ {
$this->accounts->push($account); $this->accounts->push($account);
} }

View File

@@ -62,7 +62,7 @@ class BalanceLine
/** /**
* @param BalanceEntry $balanceEntry * @param BalanceEntry $balanceEntry
*/ */
public function addBalanceEntry(BalanceEntry $balanceEntry) public function addBalanceEntry(BalanceEntry $balanceEntry): void
{ {
$this->balanceEntries->push($balanceEntry); $this->balanceEntries->push($balanceEntry);
} }
@@ -78,7 +78,7 @@ class BalanceLine
/** /**
* @param Collection $balanceEntries * @param Collection $balanceEntries
*/ */
public function setBalanceEntries(Collection $balanceEntries) public function setBalanceEntries(Collection $balanceEntries): void
{ {
$this->balanceEntries = $balanceEntries; $this->balanceEntries = $balanceEntries;
} }
@@ -94,7 +94,7 @@ class BalanceLine
/** /**
* @param BudgetModel $budget * @param BudgetModel $budget
*/ */
public function setBudget(BudgetModel $budget) public function setBudget(BudgetModel $budget): void
{ {
$this->budget = $budget; $this->budget = $budget;
} }
@@ -110,7 +110,7 @@ class BalanceLine
/** /**
* @param BudgetLimit $budgetLimit * @param BudgetLimit $budgetLimit
*/ */
public function setBudgetLimit(BudgetLimit $budgetLimit) public function setBudgetLimit(BudgetLimit $budgetLimit): void
{ {
$this->budgetLimit = $budgetLimit; $this->budgetLimit = $budgetLimit;
} }
@@ -118,7 +118,7 @@ class BalanceLine
/** /**
* @return Carbon * @return Carbon
*/ */
public function getEndDate() public function getEndDate(): Carbon
{ {
return $this->budgetLimit->end_date ?? new Carbon; return $this->budgetLimit->end_date ?? new Carbon;
} }
@@ -134,7 +134,7 @@ class BalanceLine
/** /**
* @param int $role * @param int $role
*/ */
public function setRole(int $role) public function setRole(int $role): void
{ {
$this->role = $role; $this->role = $role;
} }
@@ -142,29 +142,28 @@ class BalanceLine
/** /**
* @return Carbon * @return Carbon
*/ */
public function getStartDate() public function getStartDate(): Carbon
{ {
return $this->budgetLimit->start_date ?? new Carbon; return $this->budgetLimit->start_date ?? new Carbon;
} }
/** /**
* @SuppressWarnings(PHPMD.CyclomaticComplexity)
*
* @return string * @return string
*/ */
public function getTitle(): string public function getTitle(): string
{ {
$title = '';
if ($this->getBudget() instanceof BudgetModel && null !== $this->getBudget()->id) { if ($this->getBudget() instanceof BudgetModel && null !== $this->getBudget()->id) {
return $this->getBudget()->name; $title = $this->getBudget()->name;
} }
if (self::ROLE_DEFAULTROLE === $this->getRole()) { if ('' === $title && self::ROLE_DEFAULTROLE === $this->getRole()) {
return (string)trans('firefly.no_budget'); $title = (string)trans('firefly.no_budget');
} }
if (self::ROLE_TAGROLE === $this->getRole()) { if ('' === $title && self::ROLE_TAGROLE === $this->getRole()) {
return (string)trans('firefly.coveredWithTags'); $title = (string)trans('firefly.coveredWithTags');
} }
return ''; return $title;
} }
/** /**

View File

@@ -52,7 +52,7 @@ class Bill
/** /**
* @param BillLine $bill * @param BillLine $bill
*/ */
public function addBill(BillLine $bill) public function addBill(BillLine $bill): void
{ {
$this->bills->push($bill); $this->bills->push($bill);
} }
@@ -60,7 +60,7 @@ class Bill
/** /**
* *
*/ */
public function filterBills() public function filterBills(): void
{ {
Log::debug('Now in filterBills()'); Log::debug('Now in filterBills()');
/** @var BillRepositoryInterface $repository */ /** @var BillRepositoryInterface $repository */
@@ -114,7 +114,7 @@ class Bill
/** /**
* @param Carbon $endDate * @param Carbon $endDate
*/ */
public function setEndDate(Carbon $endDate) public function setEndDate(Carbon $endDate): void
{ {
$this->endDate = $endDate; $this->endDate = $endDate;
} }
@@ -122,7 +122,7 @@ class Bill
/** /**
* @param Carbon $startDate * @param Carbon $startDate
*/ */
public function setStartDate(Carbon $startDate) public function setStartDate(Carbon $startDate): void
{ {
$this->startDate = $startDate; $this->startDate = $startDate;
} }

View File

@@ -68,7 +68,7 @@ class BillLine
/** /**
* @param string $amount * @param string $amount
*/ */
public function setAmount(string $amount) public function setAmount(string $amount): void
{ {
$this->amount = $amount; $this->amount = $amount;
} }
@@ -84,7 +84,7 @@ class BillLine
/** /**
* @param BillModel $bill * @param BillModel $bill
*/ */
public function setBill(BillModel $bill) public function setBill(BillModel $bill): void
{ {
$this->bill = $bill; $this->bill = $bill;
} }
@@ -116,7 +116,7 @@ class BillLine
/** /**
* @param Carbon $lastHitDate * @param Carbon $lastHitDate
*/ */
public function setLastHitDate(Carbon $lastHitDate) public function setLastHitDate(Carbon $lastHitDate): void
{ {
$this->lastHitDate = $lastHitDate; $this->lastHitDate = $lastHitDate;
} }
@@ -132,7 +132,7 @@ class BillLine
/** /**
* @param string $max * @param string $max
*/ */
public function setMax(string $max) public function setMax(string $max): void
{ {
$this->max = $max; $this->max = $max;
} }
@@ -148,7 +148,7 @@ class BillLine
/** /**
* @param string $min * @param string $min
*/ */
public function setMin(string $min) public function setMin(string $min): void
{ {
$this->min = $min; $this->min = $min;
} }
@@ -180,7 +180,7 @@ class BillLine
/** /**
* @param int $transactionJournalId * @param int $transactionJournalId
*/ */
public function setTransactionJournalId(int $transactionJournalId) public function setTransactionJournalId(int $transactionJournalId): void
{ {
$this->transactionJournalId = $transactionJournalId; $this->transactionJournalId = $transactionJournalId;
} }
@@ -204,7 +204,7 @@ class BillLine
/** /**
* @param bool $hit * @param bool $hit
*/ */
public function setHit(bool $hit) public function setHit(bool $hit): void
{ {
$this->hit = $hit; $this->hit = $hit;
} }

View File

@@ -46,19 +46,19 @@ class Category
/** /**
* @param CategoryModel $category * @param CategoryModel $category
*/ */
public function addCategory(CategoryModel $category) public function addCategory(CategoryModel $category): void
{ {
// spent is minus zero for an expense report: // spent is minus zero for an expense report:
if ($category->spent < 0) { if ($category->spent < 0) {
$this->categories->push($category); $this->categories->push($category);
$this->addTotal($category->spent); $this->addTotal((string)$category->spent);
} }
} }
/** /**
* @param string $add * @param string $add
*/ */
public function addTotal(string $add) public function addTotal(string $add): void
{ {
$this->total = bcadd($this->total, $add); $this->total = bcadd($this->total, $add);
} }

View File

@@ -18,6 +18,8 @@
* You should have received a copy of the GNU General Public License * You should have received a copy of the GNU General Public License
* along with Firefly III. If not, see <http://www.gnu.org/licenses/>. * along with Firefly III. If not, see <http://www.gnu.org/licenses/>.
*/ */
/** @noinspection PhpDynamicAsStaticMethodCallInspection */
/** @noinspection PropertyCanBeStaticInspection */
declare(strict_types=1); declare(strict_types=1);
namespace FireflyIII\Helpers\Collector; namespace FireflyIII\Helpers\Collector;
@@ -50,14 +52,14 @@ use Log;
use Steam; use Steam;
/** /**
* TODO rename references to journals to transactions
* Maybe this is a good idea after all... * Maybe this is a good idea after all...
* *
* Class JournalCollector * Class JournalCollector
* * @SuppressWarnings(PHPMD.TooManyPublicMethods)
*
* @SuppressWarnings(PHPMD.ExcessiveClassComplexity) * @SuppressWarnings(PHPMD.ExcessiveClassComplexity)
* @SuppressWarnings(PHPMD.CouplingBetweenObjects) * @SuppressWarnings(PHPMD.CouplingBetweenObjects)
* @SuppressWarnings(PHPMD.TooManyPublicMethods) *
*/ */
class JournalCollector implements JournalCollectorInterface class JournalCollector implements JournalCollectorInterface
{ {
@@ -66,6 +68,7 @@ class JournalCollector implements JournalCollectorInterface
private $accountIds = []; private $accountIds = [];
/** @var int */ /** @var int */
private $count = 0; private $count = 0;
/** @var array */ /** @var array */
private $fields private $fields
= [ = [
@@ -139,7 +142,7 @@ class JournalCollector implements JournalCollectorInterface
public function addFilter(string $filter): JournalCollectorInterface public function addFilter(string $filter): JournalCollectorInterface
{ {
$interfaces = class_implements($filter); $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)); Log::debug(sprintf('Enabled filter %s', $filter));
$this->filters[] = $filter; $this->filters[] = $filter;
} }
@@ -245,8 +248,11 @@ class JournalCollector implements JournalCollectorInterface
return $this->count; return $this->count;
} }
/** @noinspection MultipleReturnStatementsInspection */
/** /**
* @return Collection * @return Collection
* @SuppressWarnings(PHPMD.CyclomaticComplexity)
* @SuppressWarnings(PHPMD.ExcessiveMethodLength)
*/ */
public function getJournals(): Collection public function getJournals(): Collection
{ {
@@ -266,9 +272,6 @@ class JournalCollector implements JournalCollectorInterface
return $cache->get(); // @codeCoverageIgnore return $cache->get(); // @codeCoverageIgnore
} }
if (true === $this->ignoreCache) {
Log::debug('Ignore cache in journal collector.');
}
/** @var Collection $set */ /** @var Collection $set */
$set = $this->query->get(array_values($this->fields)); $set = $this->query->get(array_values($this->fields));
@@ -468,18 +471,17 @@ class JournalCollector implements JournalCollectorInterface
public function setBudgets(Collection $budgets): JournalCollectorInterface public function setBudgets(Collection $budgets): JournalCollectorInterface
{ {
$budgetIds = $budgets->pluck('id')->toArray(); $budgetIds = $budgets->pluck('id')->toArray();
if (0 === \count($budgetIds)) { if (0 !== \count($budgetIds)) {
return $this; $this->joinBudgetTables();
} Log::debug('Journal collector will filter for budgets', $budgetIds);
$this->joinBudgetTables();
Log::debug('Journal collector will filter for budgets', $budgetIds);
$this->query->where( $this->query->where(
function (EloquentBuilder $q) use ($budgetIds) { function (EloquentBuilder $q) use ($budgetIds) {
$q->whereIn('budget_transaction.budget_id', $budgetIds); $q->whereIn('budget_transaction.budget_id', $budgetIds);
$q->orWhereIn('budget_transaction_journal.budget_id', $budgetIds); $q->orWhereIn('budget_transaction_journal.budget_id', $budgetIds);
} }
); );
}
return $this; return $this;
} }
@@ -492,17 +494,16 @@ class JournalCollector implements JournalCollectorInterface
public function setCategories(Collection $categories): JournalCollectorInterface public function setCategories(Collection $categories): JournalCollectorInterface
{ {
$categoryIds = $categories->pluck('id')->toArray(); $categoryIds = $categories->pluck('id')->toArray();
if (0 === \count($categoryIds)) { if (0 !== \count($categoryIds)) {
return $this; $this->joinCategoryTables();
}
$this->joinCategoryTables();
$this->query->where( $this->query->where(
function (EloquentBuilder $q) use ($categoryIds) { function (EloquentBuilder $q) use ($categoryIds) {
$q->whereIn('category_transaction.category_id', $categoryIds); $q->whereIn('category_transaction.category_id', $categoryIds);
$q->orWhereIn('category_transaction_journal.category_id', $categoryIds); $q->orWhereIn('category_transaction_journal.category_id', $categoryIds);
} }
); );
}
return $this; return $this;
} }
@@ -606,10 +607,7 @@ class JournalCollector implements JournalCollectorInterface
$this->offset = $offset; $this->offset = $offset;
$this->query->skip($offset); $this->query->skip($offset);
Log::debug(sprintf('Changed offset to %d', $offset)); Log::debug(sprintf('Changed offset to %d', $offset));
return $this;
} }
Log::debug('The limit is zero, cannot set the page.');
return $this; return $this;
} }
@@ -688,7 +686,7 @@ class JournalCollector implements JournalCollectorInterface
/** /**
* *
*/ */
public function startQuery() public function startQuery(): void
{ {
Log::debug('journalCollector::startQuery'); Log::debug('journalCollector::startQuery');
/** @var EloquentBuilder $query */ /** @var EloquentBuilder $query */
@@ -798,6 +796,7 @@ class JournalCollector implements JournalCollectorInterface
foreach ($this->filters as $enabled) { foreach ($this->filters as $enabled) {
if (isset($filters[$enabled])) { if (isset($filters[$enabled])) {
Log::debug(sprintf('Before filter %s: %d', $enabled, $set->count())); Log::debug(sprintf('Before filter %s: %d', $enabled, $set->count()));
/** @var Collection $set */
$set = $filters[$enabled]->filter($set); $set = $filters[$enabled]->filter($set);
Log::debug(sprintf('After filter %s: %d', $enabled, $set->count())); Log::debug(sprintf('After filter %s: %d', $enabled, $set->count()));
} }

View File

@@ -61,7 +61,7 @@ class InternalTransferFilter implements FilterInterface
return $transaction; return $transaction;
} }
// both id's in $parameters? // 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( Log::debug(
sprintf( sprintf(
'Transaction #%d has #%d and #%d in set, so removed', 'Transaction #%d has #%d and #%d in set, so removed',

View File

@@ -58,7 +58,7 @@ class OpposingAccountFilter implements FilterInterface
function (Transaction $transaction) { function (Transaction $transaction) {
$opposing = $transaction->opposing_account_id; $opposing = $transaction->opposing_account_id;
// remove internal transfer // 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); Log::debug(sprintf('Filtered #%d because its opposite is in accounts.', $transaction->id), $this->accounts);
return null; return null;

View File

@@ -40,7 +40,7 @@ class TransactionViewFilter implements FilterInterface
{ {
/** /**
* @param Collection $set * @param Collection $set
* * @SuppressWarnings(PHPMD.CyclomaticComplexity)
* @return Collection * @return Collection
*/ */
public function filter(Collection $set): Collection public function filter(Collection $set): Collection

View File

@@ -54,10 +54,10 @@ class FiscalHelper implements FiscalHelperInterface
// add 1 year and sub 1 day // add 1 year and sub 1 day
$endDate->addYear(); $endDate->addYear();
$endDate->subDay(); $endDate->subDay();
return $endDate;
} }
$endDate->endOfYear(); if (false === $this->useCustomFiscalYear) {
$endDate->endOfYear();
}
return $endDate; return $endDate;
} }
@@ -80,10 +80,10 @@ class FiscalHelper implements FiscalHelperInterface
if ($startDate > $date) { if ($startDate > $date) {
$startDate->subYear(); $startDate->subYear();
} }
return $startDate;
} }
$startDate->startOfYear(); if (false === $this->useCustomFiscalYear) {
$startDate->startOfYear();
}
return $startDate; return $startDate;
} }

View File

@@ -65,22 +65,19 @@ class Help implements HelpInterface
{ {
$uri = sprintf('https://raw.githubusercontent.com/firefly-iii/help/master/%s/%s.md', $language, $route); $uri = sprintf('https://raw.githubusercontent.com/firefly-iii/help/master/%s/%s.md', $language, $route);
Log::debug(sprintf('Trying to get %s...', $uri)); Log::debug(sprintf('Trying to get %s...', $uri));
$opt = ['headers' => ['User-Agent' => $this->userAgent]]; $opt = ['headers' => ['User-Agent' => $this->userAgent]];
$content = ''; $content = '';
$statusCode = 500;
$client = new Client;
try { 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) { } catch (GuzzleException|Exception $e) {
Log::error($e); Log::error($e);
return '';
} }
Log::debug(sprintf('Status code is %d', $res->getStatusCode())); Log::debug(sprintf('Status code is %d', $statusCode));
if (200 === $res->getStatusCode()) {
$content = trim($res->getBody()->getContents());
}
if (\strlen($content) > 0) { if (\strlen($content) > 0) {
Log::debug('Content is longer than zero. Expect something.'); Log::debug('Content is longer than zero. Expect something.');
@@ -126,7 +123,7 @@ class Help implements HelpInterface
* @param string $language * @param string $language
* @param string $content * @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); $key = sprintf(self::CACHEKEY, $route, $language);
if (\strlen($content) > 0) { if (\strlen($content) > 0) {

View File

@@ -34,8 +34,6 @@ use Log;
/** /**
* Class BalanceReportHelper. * Class BalanceReportHelper.
*
* @SuppressWarnings(PHPMD.CouplingBetweenObjects) // I can't really help it.
*/ */
class BalanceReportHelper implements BalanceReportHelperInterface class BalanceReportHelper implements BalanceReportHelperInterface
{ {
@@ -145,8 +143,8 @@ class BalanceReportHelper implements BalanceReportHelperInterface
} }
/** /**
* @SuppressWarnings(PHPMD.CyclomaticComplexity)
* @param Balance $balance * @param Balance $balance
* @SuppressWarnings(PHPMD.CyclomaticComplexity) // it's exactly 5.
* *
* @return Balance * @return Balance
*/ */
@@ -157,6 +155,7 @@ class BalanceReportHelper implements BalanceReportHelperInterface
foreach ($set as $entry) { foreach ($set as $entry) {
if (null !== $entry->getBudget()->id) { if (null !== $entry->getBudget()->id) {
$sum = '0'; $sum = '0';
/** @var BalanceEntry $balanceEntry */
foreach ($entry->getBalanceEntries() as $balanceEntry) { foreach ($entry->getBalanceEntries() as $balanceEntry) {
$sum = bcadd($sum, $balanceEntry->getSpent()); $sum = bcadd($sum, $balanceEntry->getSpent());
} }

View File

@@ -47,9 +47,8 @@ class BudgetReportHelper implements BudgetReportHelperInterface
} }
/** /**
* @SuppressWarnings(PHPMD.CyclomaticComplexity) // it's exactly 5. * @SuppressWarnings(PHPMD.CyclomaticComplexity)
* @SuppressWarnings(PHPMD.ExcessiveMethodLength) // all the arrays make it long. * @SuppressWarnings(PHPMD.ExcessiveMethodLength)
*
* @param Carbon $start * @param Carbon $start
* @param Carbon $end * @param Carbon $end
* @param Collection $accounts * @param Collection $accounts

View File

@@ -119,6 +119,10 @@ class PopupReport implements PopupReportInterface
*/ */
public function byExpenses(Account $account, array $attributes): Collection public function byExpenses(Account $account, array $attributes): Collection
{ {
/** @var JournalRepositoryInterface $repository */
$repository = app(JournalRepositoryInterface::class);
$repository->setUser($account->user);
/** @var JournalCollectorInterface $collector */ /** @var JournalCollectorInterface $collector */
$collector = app(JournalCollectorInterface::class); $collector = app(JournalCollectorInterface::class);
@@ -130,9 +134,9 @@ class PopupReport implements PopupReportInterface
// filter for transfers and withdrawals TO the given $account // filter for transfers and withdrawals TO the given $account
$journals = $journals->filter( $journals = $journals->filter(
function (Transaction $transaction) use ($report) { function (Transaction $transaction) use ($report, $repository) {
// get the destinations: // 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? // do these intersect with the current list?
return !empty(array_intersect($report, $sources)); return !empty(array_intersect($report, $sources));

View File

@@ -56,10 +56,11 @@ class ReportHelper implements ReportHelperInterface
* This method generates a full report for the given period on all * This method generates a full report for the given period on all
* the users bills and their payments. * 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. * Excludes bills which have not had a payment on the mentioned accounts.
* *
* @SuppressWarnings(PHPMD.CyclomaticComplexity)
* @SuppressWarnings(PHPMD.ExcessiveMethodLength)
*
* @param Carbon $start * @param Carbon $start
* @param Carbon $end * @param Carbon $end
* @param Collection $accounts * @param Collection $accounts
@@ -82,6 +83,7 @@ class ReportHelper implements ReportHelperInterface
foreach ($expectedDates as $payDate) { foreach ($expectedDates as $payDate) {
$endOfPayPeriod = app('navigation')->endOfX($payDate, $bill->repeat_freq, null); $endOfPayPeriod = app('navigation')->endOfX($payDate, $bill->repeat_freq, null);
/** @var JournalCollectorInterface $collector */
$collector = app(JournalCollectorInterface::class); $collector = app(JournalCollectorInterface::class);
$collector->setAccounts($accounts)->setRange($payDate, $endOfPayPeriod)->setBills($bills); $collector->setAccounts($accounts)->setRange($payDate, $endOfPayPeriod)->setBills($bills);
$journals = $collector->getJournals(); $journals = $collector->getJournals();

View File

@@ -34,6 +34,7 @@ use Symfony\Component\HttpKernel\Exception\NotFoundHttpException;
* *
* @property string $name * @property string $name
* @property int $id * @property int $id
* @property float $spent // used in category reports
*/ */
class Category extends Model class Category extends Model
{ {

View File

@@ -83,8 +83,11 @@ use Symfony\Component\HttpKernel\Exception\NotFoundHttpException;
* @property TransactionCurrency $transactionCurrency * @property TransactionCurrency $transactionCurrency
* @property int $transaction_journal_id * @property int $transaction_journal_id
* @property TransactionCurrency $foreignCurrency * @property TransactionCurrency $foreignCurrency
* @property string $before // used in audit reports. * @property string $before // used in audit reports.
* @property string $after // 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 class Transaction extends Model
{ {

View File

@@ -28,6 +28,10 @@ use FireflyIII\Models\LinkType;
use League\Fractal\TransformerAbstract; use League\Fractal\TransformerAbstract;
use Symfony\Component\HttpFoundation\ParameterBag; use Symfony\Component\HttpFoundation\ParameterBag;
/**
*
* Class LinkTypeTransformer
*/
class LinkTypeTransformer extends TransformerAbstract class LinkTypeTransformer extends TransformerAbstract
{ {
@@ -75,7 +79,7 @@ class LinkTypeTransformer extends TransformerAbstract
'name' => $linkType->name, 'name' => $linkType->name,
'inward' => $linkType->inward, 'inward' => $linkType->inward,
'outward' => $linkType->outward, 'outward' => $linkType->outward,
'editable' => (int)$linkType->editable, 'editable' => 1 === (int)$linkType->editable,
'links' => [ 'links' => [
[ [
'rel' => 'self', 'rel' => 'self',