diff --git a/app/Api/V1/Requests/AvailableBudgetRequest.php b/app/Api/V1/Requests/AvailableBudgetRequest.php index 9ded5fdefa..dd6daf4e65 100644 --- a/app/Api/V1/Requests/AvailableBudgetRequest.php +++ b/app/Api/V1/Requests/AvailableBudgetRequest.php @@ -55,7 +55,6 @@ 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/JournalLinkRequest.php b/app/Api/V1/Requests/JournalLinkRequest.php index b6a9d9b9af..19e5f8f430 100644 --- a/app/Api/V1/Requests/JournalLinkRequest.php +++ b/app/Api/V1/Requests/JournalLinkRequest.php @@ -57,8 +57,6 @@ 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. * diff --git a/app/Helpers/Collector/JournalCollector.php b/app/Helpers/Collector/JournalCollector.php index 2d07df89c2..26f481bdce 100644 --- a/app/Helpers/Collector/JournalCollector.php +++ b/app/Helpers/Collector/JournalCollector.php @@ -51,7 +51,6 @@ use Illuminate\Support\Collection; use Log; /** - * TODO rename references to journals to transactions * Maybe this is a good idea after all... * * Class JournalCollector diff --git a/app/Helpers/Collector/JournalCollectorInterface.php b/app/Helpers/Collector/JournalCollectorInterface.php index bb42ea69de..f3d3dbbb7b 100644 --- a/app/Helpers/Collector/JournalCollectorInterface.php +++ b/app/Helpers/Collector/JournalCollectorInterface.php @@ -82,7 +82,6 @@ interface JournalCollectorInterface /** * Get all journals. - * TODO rename me. * * @return Collection */ diff --git a/app/Http/Controllers/Budget/IndexController.php b/app/Http/Controllers/Budget/IndexController.php index 15b9f9bba2..e79ea0b59a 100644 --- a/app/Http/Controllers/Budget/IndexController.php +++ b/app/Http/Controllers/Budget/IndexController.php @@ -68,8 +68,6 @@ class IndexController extends Controller /** * Show all budgets. * - * TODO remove moment routine. - * * @param Request $request * @param string|null $moment * diff --git a/app/Http/Controllers/Chart/BudgetController.php b/app/Http/Controllers/Chart/BudgetController.php index 8ca96aa5d9..801be0814c 100644 --- a/app/Http/Controllers/Chart/BudgetController.php +++ b/app/Http/Controllers/Chart/BudgetController.php @@ -518,8 +518,6 @@ class BudgetController extends Controller /** * Small helper function for some of the charts. Extracts category names from a bunch of ID's. * - * TODO this method is duplicated and should be in a trait. - * * @param array $categoryIds * * @return array diff --git a/app/Http/Controllers/Controller.php b/app/Http/Controllers/Controller.php index b3730d5de9..a90a31dc8f 100644 --- a/app/Http/Controllers/Controller.php +++ b/app/Http/Controllers/Controller.php @@ -121,8 +121,6 @@ class Controller extends BaseController /** * Is transaction opening balance? * - * TODO move to trait. - * * @param TransactionJournal $journal * * @return bool @@ -136,8 +134,6 @@ class Controller extends BaseController /** * Redirect to asset account that transaction belongs to. * - * TODO move to trait. - * * @param TransactionJournal $journal * * @return \Illuminate\Http\RedirectResponse|\Illuminate\Routing\Redirector @@ -172,9 +168,6 @@ class Controller extends BaseController /** * Get user's language. - * - * TODO pretty sure nobody uses this. - * * @return string */ private function getLanguage(): string @@ -196,8 +189,6 @@ class Controller extends BaseController /** * Get the specific name of a page for intro. * - * TODO move to trait. - * * @return string */ private function getSpecificPageName(): string @@ -208,8 +199,6 @@ class Controller extends BaseController /** * Returns if user has seen demo. * - * TODO move to trait. - * * @return bool */ private function hasSeenDemo(): bool diff --git a/app/Http/Controllers/CurrencyController.php b/app/Http/Controllers/CurrencyController.php index b2b7ae7fb2..927bc5880c 100644 --- a/app/Http/Controllers/CurrencyController.php +++ b/app/Http/Controllers/CurrencyController.php @@ -107,8 +107,6 @@ class CurrencyController extends Controller app('preferences')->mark(); $request->session()->flash('success', (string)trans('firefly.new_default_currency', ['name' => $currency->name])); - Cache::forget('FFCURRENCYSYMBOL'); // todo are these even used? - Cache::forget('FFCURRENCYCODE'); return redirect(route('currencies.index')); } diff --git a/app/Http/Controllers/HelpController.php b/app/Http/Controllers/HelpController.php index 35555c2318..92323afb4c 100644 --- a/app/Http/Controllers/HelpController.php +++ b/app/Http/Controllers/HelpController.php @@ -68,8 +68,6 @@ class HelpController extends Controller /** * Gets the help text. * - * TODO move to repos or trait. - * * @param string $route * @param string $language * diff --git a/app/Http/Controllers/Recurring/EditController.php b/app/Http/Controllers/Recurring/EditController.php index b5f88eb645..400bd07a5b 100644 --- a/app/Http/Controllers/Recurring/EditController.php +++ b/app/Http/Controllers/Recurring/EditController.php @@ -70,9 +70,6 @@ class EditController extends Controller /** * Edit a recurring transaction. * - * todo move to repository - * todo handle old repetition type as well. - * * @param Request $request * @param Recurrence $recurrence * diff --git a/app/Http/Controllers/Recurring/IndexController.php b/app/Http/Controllers/Recurring/IndexController.php index 0321dd34e3..dd29bb353e 100644 --- a/app/Http/Controllers/Recurring/IndexController.php +++ b/app/Http/Controllers/Recurring/IndexController.php @@ -65,8 +65,6 @@ class IndexController extends Controller /** * Show all recurring transactions. * - * TODO: split collection into pages - * * @param Request $request * * @return \Illuminate\Contracts\View\Factory|\Illuminate\View\View diff --git a/app/Http/Controllers/Report/BudgetController.php b/app/Http/Controllers/Report/BudgetController.php index 494759beea..420fd485c7 100644 --- a/app/Http/Controllers/Report/BudgetController.php +++ b/app/Http/Controllers/Report/BudgetController.php @@ -38,8 +38,6 @@ class BudgetController extends Controller /** * Show partial overview of budgets. * - * TODO replace all \Throwable by catch/error and return simple string. - * * @param Collection $accounts * @param Carbon $start * @param Carbon $end diff --git a/app/Http/Controllers/Rule/CreateController.php b/app/Http/Controllers/Rule/CreateController.php index ce11445340..e35b243640 100644 --- a/app/Http/Controllers/Rule/CreateController.php +++ b/app/Http/Controllers/Rule/CreateController.php @@ -71,8 +71,6 @@ class CreateController extends Controller /** * Create a new rule. It will be stored under the given $ruleGroup. * - * TODO remove bill from this method, move to separate routine. - * * @param Request $request * @param RuleGroup $ruleGroup * diff --git a/app/Http/Controllers/TagController.php b/app/Http/Controllers/TagController.php index 3183ee4228..102f1d7bff 100644 --- a/app/Http/Controllers/TagController.php +++ b/app/Http/Controllers/TagController.php @@ -172,8 +172,6 @@ class TagController extends Controller * @param Tag $tag * @param string|null $moment * - * TODO will be cleaned up and separated - * * @return \Illuminate\Contracts\View\Factory|\Illuminate\View\View * * @SuppressWarnings(PHPMD.ExcessiveMethodLength) diff --git a/app/Http/Requests/BillFormRequest.php b/app/Http/Requests/BillFormRequest.php index db13b29bb8..59d7eaae1a 100644 --- a/app/Http/Requests/BillFormRequest.php +++ b/app/Http/Requests/BillFormRequest.php @@ -67,7 +67,6 @@ class BillFormRequest extends Request { $nameRule = 'required|between:1,255|uniqueObjectForUser:bills,name'; if ($this->integer('id') > 0) { - // todo is a fix to do this better. $nameRule .= ',' . $this->integer('id'); } // is OK diff --git a/app/Providers/EventServiceProvider.php b/app/Providers/EventServiceProvider.php index 06207a3106..fd80ce452f 100644 --- a/app/Providers/EventServiceProvider.php +++ b/app/Providers/EventServiceProvider.php @@ -117,7 +117,6 @@ class EventServiceProvider extends ServiceProvider */ protected function registerCreateEvents(): void { - // todo move this routine to a filter // in case of repeated piggy banks and/or other problems. PiggyBank::created( function (PiggyBank $piggyBank) { diff --git a/app/Repositories/Account/AccountRepository.php b/app/Repositories/Account/AccountRepository.php index 705f32f7a2..e865b10dbc 100644 --- a/app/Repositories/Account/AccountRepository.php +++ b/app/Repositories/Account/AccountRepository.php @@ -38,6 +38,7 @@ use Log; /** * Class AccountRepository. + * @SuppressWarnings(PHPMD.CouplingBetweenObjects) */ class AccountRepository implements AccountRepositoryInterface { @@ -392,7 +393,6 @@ class AccountRepository implements AccountRepositoryInterface /** * Returns the date of the very first transaction in this account. - * TODO refactor to nullable. * * @param Account $account * diff --git a/app/Repositories/Bill/BillRepository.php b/app/Repositories/Bill/BillRepository.php index a855d305ab..81c08e85e2 100644 --- a/app/Repositories/Bill/BillRepository.php +++ b/app/Repositories/Bill/BillRepository.php @@ -41,6 +41,7 @@ use Log; /** * Class BillRepository. + * @SuppressWarnings(PHPMD.CouplingBetweenObjects) */ class BillRepository implements BillRepositoryInterface { diff --git a/app/Repositories/Budget/BudgetRepository.php b/app/Repositories/Budget/BudgetRepository.php index 6556b7a24e..2bbb0fd169 100644 --- a/app/Repositories/Budget/BudgetRepository.php +++ b/app/Repositories/Budget/BudgetRepository.php @@ -34,7 +34,6 @@ use FireflyIII\Models\RuleAction; use FireflyIII\Models\RuleTrigger; use FireflyIII\Models\Transaction; use FireflyIII\Models\TransactionCurrency; -use FireflyIII\Models\TransactionJournal; use FireflyIII\Models\TransactionType; use FireflyIII\Repositories\Account\AccountRepositoryInterface; use FireflyIII\User; @@ -42,10 +41,13 @@ use Illuminate\Database\Eloquent\Builder; use Illuminate\Support\Collection; use Log; use Navigation; -use stdClass; /** * Class BudgetRepository. + * + * @SuppressWarnings(PHPMD.CouplingBetweenObjects) + * @SuppressWarnings(PHPMD.TooManyPublicMethods) + * @SuppressWarnings(PHPMD.ExcessiveClassComplexity) */ class BudgetRepository implements BudgetRepositoryInterface { @@ -139,8 +141,8 @@ class BudgetRepository implements BudgetRepositoryInterface foreach ($budgets as $budget) { $budgetId = $budget->id; $return[$budgetId] = [ - 'spent' => $this->spentInPeriod(new Collection([$budget]), $accounts, $start, $end), - 'budgeted' => '0', + 'spent' => $this->spentInPeriod(new Collection([$budget]), $accounts, $start, $end), + 'budgeted' => '0', ]; $budgetLimits = $this->getBudgetLimits($budget, $start, $end); $otherLimits = new Collection; @@ -246,7 +248,10 @@ class BudgetRepository implements BudgetRepositoryInterface * Will cache result. * * @param Budget $budget + * * @return Carbon + * @SuppressWarnings(PHPMD.CyclomaticComplexity) + * @SuppressWarnings(PHPMD.ExcessiveMethodLength) */ public function firstUseDate(Budget $budget): ?Carbon { @@ -290,6 +295,8 @@ class BudgetRepository implements BudgetRepositoryInterface * @param Carbon $end * * @return Collection + * @SuppressWarnings(PHPMD.CyclomaticComplexity) + * @SuppressWarnings(PHPMD.ExcessiveMethodLength) */ public function getAllBudgetLimits(Carbon $start = null, Carbon $end = null): Collection { @@ -421,6 +428,8 @@ class BudgetRepository implements BudgetRepositoryInterface * @param Carbon $end * * @return Collection + * @SuppressWarnings(PHPMD.CyclomaticComplexity) + * @SuppressWarnings(PHPMD.ExcessiveMethodLength) */ public function getBudgetLimits(Budget $budget, Carbon $start = null, Carbon $end = null): Collection { @@ -763,47 +772,12 @@ class BudgetRepository implements BudgetRepositoryInterface */ public function update(Budget $budget, array $data): Budget { - $oldName = $budget->name; - // update the budget: + $oldName = $budget->name; $budget->name = $data['name']; $budget->active = $data['active']; $budget->save(); - - // find any rule triggers related to budgets, with this budget name, and update them accordingly. - $types = [ - 'budget_is', - ]; - $triggers = RuleTrigger::leftJoin('rules', 'rules.id', '=', 'rule_triggers.rule_id') - ->where('rules.user_id', $this->user->id) - ->whereIn('rule_triggers.trigger_type', $types) - ->where('rule_triggers.trigger_value', $oldName) - ->get(['rule_triggers.*']); - Log::debug(sprintf('Found %d triggers to update.', $triggers->count())); - /** @var RuleTrigger $trigger */ - foreach ($triggers as $trigger) { - $trigger->trigger_value = $data['name']; - $trigger->save(); - Log::debug(sprintf('Updated trigger %d: %s', $trigger->id, $trigger->trigger_value)); - } - - - // find any rule actions related to budgets, with this budget name, and update them accordingly. - $types = [ - 'set_budget', - ]; - $actions = RuleAction::leftJoin('rules', 'rules.id', '=', 'rule_actions.rule_id') - ->where('rules.user_id', $this->user->id) - ->whereIn('rule_actions.action_type', $types) - ->where('rule_actions.action_value', $oldName) - ->get(['rule_actions.*']); - Log::debug(sprintf('Found %d actions to update.', $actions->count())); - /** @var RuleAction $action */ - foreach ($actions as $action) { - $action->action_value = $data['name']; - $action->save(); - Log::debug(sprintf('Updated action %d: %s', $action->id, $action->action_value)); - } - + $this->updateRuleTriggers($oldName, $data['name']); + $this->updateRuleActions($oldName, $data['name']); app('preferences')->mark(); return $budget; @@ -869,6 +843,8 @@ class BudgetRepository implements BudgetRepositoryInterface * @param string $amount * * @return BudgetLimit|null + * @SuppressWarnings(PHPMD.CyclomaticComplexity) + * @SuppressWarnings(PHPMD.ExcessiveMethodLength) */ public function updateLimitAmount(Budget $budget, Carbon $start, Carbon $end, string $amount): ?BudgetLimit { @@ -879,6 +855,7 @@ class BudgetRepository implements BudgetRepositoryInterface ->where('budget_limits.end_date', $end->format('Y-m-d 00:00:00')) ->get(['budget_limits.*'])->count(); Log::debug(sprintf('Found %d budget limits.', $limits)); + // there might be a budget limit for these dates: /** @var BudgetLimit $limit */ $limit = $budget->budgetlimits() @@ -929,4 +906,46 @@ class BudgetRepository implements BudgetRepositoryInterface return $limit; } + + /** + * @param string $oldName + * @param string $newName + */ + private function updateRuleActions(string $oldName, string $newName): void + { + $types = ['set_budget',]; + $actions = RuleAction::leftJoin('rules', 'rules.id', '=', 'rule_actions.rule_id') + ->where('rules.user_id', $this->user->id) + ->whereIn('rule_actions.action_type', $types) + ->where('rule_actions.action_value', $oldName) + ->get(['rule_actions.*']); + Log::debug(sprintf('Found %d actions to update.', $actions->count())); + /** @var RuleAction $action */ + foreach ($actions as $action) { + $action->action_value = $newName; + $action->save(); + Log::debug(sprintf('Updated action %d: %s', $action->id, $action->action_value)); + } + } + + /** + * @param string $oldName + * @param string $newName + */ + private function updateRuleTriggers(string $oldName, string $newName): void + { + $types = ['budget_is',]; + $triggers = RuleTrigger::leftJoin('rules', 'rules.id', '=', 'rule_triggers.rule_id') + ->where('rules.user_id', $this->user->id) + ->whereIn('rule_triggers.trigger_type', $types) + ->where('rule_triggers.trigger_value', $oldName) + ->get(['rule_triggers.*']); + Log::debug(sprintf('Found %d triggers to update.', $triggers->count())); + /** @var RuleTrigger $trigger */ + foreach ($triggers as $trigger) { + $trigger->trigger_value = $newName; + $trigger->save(); + Log::debug(sprintf('Updated trigger %d: %s', $trigger->id, $trigger->trigger_value)); + } + } } diff --git a/app/Repositories/Category/CategoryRepository.php b/app/Repositories/Category/CategoryRepository.php index b134f74fef..f35b77f0e4 100644 --- a/app/Repositories/Category/CategoryRepository.php +++ b/app/Repositories/Category/CategoryRepository.php @@ -37,6 +37,10 @@ use Navigation; /** * Class CategoryRepository. + * + * @SuppressWarnings(PHPMD.CouplingBetweenObjects) + * @SuppressWarnings(PHPMD.TooManyPublicMethods) + * @SuppressWarnings(PHPMD.ExcessiveClassComplexity) */ class CategoryRepository implements CategoryRepositoryInterface { @@ -114,6 +118,7 @@ class CategoryRepository implements CategoryRepositoryInterface * @param Category $category * * @return Carbon|null + * @SuppressWarnings(PHPMD.CyclomaticComplexity) */ public function firstUseDate(Category $category): ?Carbon { @@ -172,6 +177,8 @@ class CategoryRepository implements CategoryRepositoryInterface * @param Collection $accounts * * @return Carbon|null + * + * @SuppressWarnings(PHPMD.CyclomaticComplexity) */ public function lastUseDate(Category $category, Collection $accounts): ?Carbon { diff --git a/app/Repositories/ImportJob/ImportJobRepository.php b/app/Repositories/ImportJob/ImportJobRepository.php index 24b74bdb9e..ba6626e271 100644 --- a/app/Repositories/ImportJob/ImportJobRepository.php +++ b/app/Repositories/ImportJob/ImportJobRepository.php @@ -37,6 +37,8 @@ use Symfony\Component\HttpFoundation\File\UploadedFile; /** * Class ImportJobRepository. + * + * @SuppressWarnings(PHPMD.CouplingBetweenObjects) */ class ImportJobRepository implements ImportJobRepositoryInterface { @@ -270,7 +272,6 @@ class ImportJobRepository implements ImportJobRepositoryInterface public function storeCLIUpload(ImportJob $job, string $name, string $fileName): MessageBag { $messages = new MessageBag; - if (!file_exists($fileName)) { $messages->add('notfound', sprintf('File not found: %s', $fileName)); @@ -283,8 +284,7 @@ class ImportJobRepository implements ImportJobRepositoryInterface } )->count(); - if ($count > 0) { - // don't upload, but also don't complain about it. + if ($count > 0) {// don't upload, but also don't complain about it. Log::error(sprintf('Detected duplicate upload. Will ignore second "%s" file.', $name)); return new MessageBag; @@ -301,12 +301,10 @@ class ImportJobRepository implements ImportJobRepositoryInterface $attachment->save(); $encrypted = Crypt::encrypt($content); - // store it: $this->uploadDisk->put($attachment->fileName(), $encrypted); $attachment->uploaded = true; // update attachment $attachment->save(); - // return it. return new MessageBag; } @@ -333,9 +331,7 @@ class ImportJobRepository implements ImportJobRepositoryInterface return $att->filename === $name; } )->count(); - - if ($count > 0) { - // don't upload, but also don't complain about it. + if ($count > 0) { // don't upload, but also don't complain about it. Log::error(sprintf('Detected duplicate upload. Will ignore second "%s" file.', $name)); return new MessageBag; @@ -354,13 +350,10 @@ class ImportJobRepository implements ImportJobRepositoryInterface $fileObject->rewind(); $content = $fileObject->fread($file->getSize()); $encrypted = Crypt::encrypt($content); - - // store it: $this->uploadDisk->put($attachment->fileName(), $encrypted); $attachment->uploaded = true; // update attachment $attachment->save(); - // return it. return new MessageBag; } diff --git a/app/Repositories/Journal/JournalRepository.php b/app/Repositories/Journal/JournalRepository.php index 657b5faf61..68ca7ca9b4 100644 --- a/app/Repositories/Journal/JournalRepository.php +++ b/app/Repositories/Journal/JournalRepository.php @@ -44,6 +44,10 @@ use Log; /** * Class JournalRepository. + * + * @SuppressWarnings(PHPMD.CouplingBetweenObjects) + * @SuppressWarnings(PHPMD.TooManyPublicMethods) + * @SuppressWarnings(PHPMD.ExcessiveClassComplexity) */ class JournalRepository implements JournalRepositoryInterface { @@ -310,6 +314,8 @@ class JournalRepository implements JournalRepositoryInterface * @param null|string $field * * @return string + * @SuppressWarnings(PHPMD.CyclomaticComplexity) + * @SuppressWarnings(PHPMD.ExcessiveMethodLength) */ public function getJournalDate(TransactionJournal $journal, ?string $field): string { @@ -451,6 +457,7 @@ class JournalRepository implements JournalRepositoryInterface * @param string $field * * @return null|string + * @SuppressWarnings(PHPMD.CyclomaticComplexity) */ public function getMetaField(TransactionJournal $journal, string $field): ?string { diff --git a/app/Repositories/LinkType/LinkTypeRepository.php b/app/Repositories/LinkType/LinkTypeRepository.php index 5fe1ad3db1..10d53f37af 100644 --- a/app/Repositories/LinkType/LinkTypeRepository.php +++ b/app/Repositories/LinkType/LinkTypeRepository.php @@ -22,8 +22,6 @@ declare(strict_types=1); namespace FireflyIII\Repositories\LinkType; -use Exception; -use FireflyIII\Exceptions\FireflyException; use FireflyIII\Models\LinkType; use FireflyIII\Models\Note; use FireflyIII\Models\TransactionJournal; @@ -34,6 +32,8 @@ use Log; /** * Class LinkTypeRepository. + * + * @SuppressWarnings(PHPMD.TooManyPublicMethods) */ class LinkTypeRepository implements LinkTypeRepositoryInterface { @@ -111,13 +111,13 @@ class LinkTypeRepository implements LinkTypeRepositoryInterface } /** - * @param int $id + * @param int $linkTypeId * * @return LinkType|null */ - public function findNull(int $id): ?LinkType + public function findNull(int $linkTypeId): ?LinkType { - return LinkType::find($id); + return LinkType::find($linkTypeId); } /** @@ -224,14 +224,14 @@ class LinkTypeRepository implements LinkTypeRepositoryInterface * @param TransactionJournal $inward * @param TransactionJournal $outward * - * @return mixed - * @throws FireflyException + * @return TransactionJournalLink|null + * @SuppressWarnings(PHPMD.CyclomaticComplexity) */ - public function storeLink(array $information, TransactionJournal $inward, TransactionJournal $outward): TransactionJournalLink + public function storeLink(array $information, TransactionJournal $inward, TransactionJournal $outward): ?TransactionJournalLink { $linkType = $this->findNull((int)($information['link_type_id'] ?? 0)); if (null === $linkType) { - throw new FireflyException(sprintf('Link type #%d cannot be resolved to an actual link type', $information['link_type_id'] ?? 0)); + return null; } // might exist already: @@ -256,15 +256,7 @@ class LinkTypeRepository implements LinkTypeRepositoryInterface $link->save(); // make note in noteable: - if (\strlen((string)$information['notes']) > 0) { - $dbNote = $link->notes()->first(); - if (null === $dbNote) { - $dbNote = new Note(); - $dbNote->noteable()->associate($link); - } - $dbNote->text = trim($information['notes']); - $dbNote->save(); - } + $this->setNoteText($link, (string)$information['notes']); return $link; } @@ -314,27 +306,33 @@ class LinkTypeRepository implements LinkTypeRepositoryInterface $journalLink->destination_id = $data['outward']->id; $journalLink->link_type_id = $data['link_type_id']; $journalLink->save(); - /** @var Note $note */ - $note = $journalLink->notes()->first(); - // delete note: - if (null !== $note && '' === $data['notes']) { - try { - $note->delete(); - } catch (Exception $e) { - Log::debug(sprintf('Could not delete note for journal link: %s', $e->getMessage())); - } - } - // create note: - if (null === $note && '' !== $data['notes']) { - $note = new Note; - $note->noteable()->associate($journalLink); - } - // update note - if ('' !== $data['notes']) { - $note->text = $data['notes']; - $note->save(); - } + $this->setNoteText($journalLink, $data['notes']); return $journalLink; } + + /** + * @param TransactionJournalLink $link + * @param string $text + * + * @SuppressWarnings(PHPMD.CyclomaticComplexity) + */ + private function setNoteText(TransactionJournalLink $link, string $text): void + { + $dbNote = $link->notes()->first(); + if ('' !== $text) { + if (null === $dbNote) { + $dbNote = new Note(); + $dbNote->noteable()->associate($link); + } + $dbNote->text = trim($text); + $dbNote->save(); + + return; + } + if (null !== $dbNote && '' === $text) { + $dbNote->delete(); + } + + } } diff --git a/app/Repositories/LinkType/LinkTypeRepositoryInterface.php b/app/Repositories/LinkType/LinkTypeRepositoryInterface.php index 5d4a250614..02080dcc0e 100644 --- a/app/Repositories/LinkType/LinkTypeRepositoryInterface.php +++ b/app/Repositories/LinkType/LinkTypeRepositoryInterface.php @@ -75,11 +75,11 @@ interface LinkTypeRepositoryInterface public function findLink(TransactionJournal $one, TransactionJournal $two): bool; /** - * @param int $id + * @param int $linkTypeId * * @return LinkType|null */ - public function findNull(int $id): ?LinkType; + public function findNull(int $linkTypeId): ?LinkType; /** * See if such a link already exists (and get it). @@ -134,9 +134,9 @@ interface LinkTypeRepositoryInterface * @param TransactionJournal $inward * @param TransactionJournal $outward * - * @return mixed + * @return TransactionJournalLink|null */ - public function storeLink(array $information, TransactionJournal $inward, TransactionJournal $outward): TransactionJournalLink; + public function storeLink(array $information, TransactionJournal $inward, TransactionJournal $outward): ?TransactionJournalLink; /** * @param TransactionJournalLink $link diff --git a/app/Repositories/PiggyBank/PiggyBankRepository.php b/app/Repositories/PiggyBank/PiggyBankRepository.php index 377b9d6331..ad4c3b51a0 100644 --- a/app/Repositories/PiggyBank/PiggyBankRepository.php +++ b/app/Repositories/PiggyBank/PiggyBankRepository.php @@ -35,6 +35,9 @@ use Log; /** * Class PiggyBankRepository. + * + * @SuppressWarnings(PHPMD.TooManyPublicMethods) + * @SuppressWarnings(PHPMD.ExcessiveClassComplexity) */ class PiggyBankRepository implements PiggyBankRepositoryInterface { @@ -245,6 +248,7 @@ class PiggyBankRepository implements PiggyBankRepositoryInterface * @param TransactionJournal $journal * * @return string + * @SuppressWarnings(PHPMD.CyclomaticComplexity) */ public function getExactAmount(PiggyBank $piggyBank, PiggyBankRepetition $repetition, TransactionJournal $journal): string { @@ -256,7 +260,6 @@ class PiggyBankRepository implements PiggyBankRepositoryInterface $sources = $repos->getJournalSourceAccounts($journal)->pluck('id')->toArray(); $room = bcsub((string)$piggyBank->targetamount, (string)$repetition->currentamount); $compare = bcmul($repetition->currentamount, '-1'); - Log::debug(sprintf('Will add/remove %f to piggy bank #%d ("%s")', $amount, $piggyBank->id, $piggyBank->name)); // if piggy account matches source account, the amount is positive @@ -448,7 +451,7 @@ class PiggyBankRepository implements PiggyBankRepositoryInterface /** @var PiggyBank $piggyBank */ $piggyBank = PiggyBank::create($data); - $this->updateNote($piggyBank, $data['note']); // todo rename to 'notes' + $this->updateNote($piggyBank, $data['note']); // repetition is auto created. $repetition = $this->getRepetition($piggyBank); diff --git a/app/Repositories/Recurring/RecurringRepository.php b/app/Repositories/Recurring/RecurringRepository.php index c67f1cc36b..38df49b788 100644 --- a/app/Repositories/Recurring/RecurringRepository.php +++ b/app/Repositories/Recurring/RecurringRepository.php @@ -47,6 +47,9 @@ use Log; /** * * Class RecurringRepository + * + * @SuppressWarnings(PHPMD.CouplingBetweenObjects) + * @SuppressWarnings(PHPMD.ExcessiveClassComplexity) */ class RecurringRepository implements RecurringRepositoryInterface { @@ -98,15 +101,15 @@ class RecurringRepository implements RecurringRepositoryInterface /** * Get the budget ID from a recurring transaction transaction. * - * @param RecurrenceTransaction $recurrenceTransaction + * @param RecurrenceTransaction $recTransaction * * @return null|int */ - public function getBudget(RecurrenceTransaction $recurrenceTransaction): ?int + public function getBudget(RecurrenceTransaction $recTransaction): ?int { $return = 0; /** @var RecurrenceTransactionMeta $meta */ - foreach ($recurrenceTransaction->recurrenceTransactionMeta as $meta) { + foreach ($recTransaction->recurrenceTransactionMeta as $meta) { if ('budget_id' === $meta->name) { $return = (int)$meta->value; } @@ -118,15 +121,15 @@ class RecurringRepository implements RecurringRepositoryInterface /** * Get the category from a recurring transaction transaction. * - * @param RecurrenceTransaction $recurrenceTransaction + * @param RecurrenceTransaction $recTransaction * * @return null|string */ - public function getCategory(RecurrenceTransaction $recurrenceTransaction): ?string + public function getCategory(RecurrenceTransaction $recTransaction): ?string { $return = ''; /** @var RecurrenceTransactionMeta $meta */ - foreach ($recurrenceTransaction->recurrenceTransactionMeta as $meta) { + foreach ($recTransaction->recurrenceTransactionMeta as $meta) { if ('category_name' === $meta->name) { $return = (string)$meta->value; } @@ -189,6 +192,7 @@ class RecurringRepository implements RecurringRepositoryInterface * * * @return array + * @SuppressWarnings(PHPMD.CyclomaticComplexity) */ public function getOccurrencesInRange(RecurrenceRepetition $repetition, Carbon $start, Carbon $end): array { @@ -310,6 +314,7 @@ class RecurringRepository implements RecurringRepositoryInterface * @param int $count * * @return array + * @SuppressWarnings(PHPMD.CyclomaticComplexity) */ public function getXOccurrences(RecurrenceRepetition $repetition, Carbon $date, int $count): array { @@ -343,48 +348,43 @@ class RecurringRepository implements RecurringRepositoryInterface * @param RecurrenceRepetition $repetition * * @return string - * @throws FireflyException + * @SuppressWarnings(PHPMD.CyclomaticComplexity) */ public function repetitionDescription(RecurrenceRepetition $repetition): string { /** @var Preference $pref */ $pref = app('preferences')->getForUser($this->user, 'language', config('firefly.default_language', 'en_US')); $language = $pref->data; - switch ($repetition->repetition_type) { - default: - throw new FireflyException(sprintf('Cannot translate recurring transaction repetition type "%s"', $repetition->repetition_type)); - break; - case 'daily': - return (string)trans('firefly.recurring_daily', [], $language); - break; - case 'weekly': - $dayOfWeek = trans(sprintf('config.dow_%s', $repetition->repetition_moment), [], $language); - - return (string)trans('firefly.recurring_weekly', ['weekday' => $dayOfWeek], $language); - break; - case 'monthly': - // format a date: - return (string)trans('firefly.recurring_monthly', ['dayOfMonth' => $repetition->repetition_moment], $language); - break; - case 'ndom': - $parts = explode(',', $repetition->repetition_moment); - // first part is number of week, second is weekday. - $dayOfWeek = trans(sprintf('config.dow_%s', $parts[1]), [], $language); - - return (string)trans('firefly.recurring_ndom', ['weekday' => $dayOfWeek, 'dayOfMonth' => $parts[0]], $language); - break; - case 'yearly': - // - $today = Carbon::create()->endOfYear(); - $repDate = Carbon::createFromFormat('Y-m-d', $repetition->repetition_moment); - $diffInYears = $today->diffInYears($repDate); - $repDate->addYears($diffInYears); // technically not necessary. - $string = $repDate->formatLocalized(trans('config.month_and_day_no_year')); - - return (string)trans('firefly.recurring_yearly', ['date' => $string], $language); - break; - + if ('daily' === $repetition->repetition_type) { + return (string)trans('firefly.recurring_daily', [], $language); } + if ('weekly' === $repetition->repetition_type) { + $dayOfWeek = trans(sprintf('config.dow_%s', $repetition->repetition_moment), [], $language); + + return (string)trans('firefly.recurring_weekly', ['weekday' => $dayOfWeek], $language); + } + if ('monthly' === $repetition->repetition_type) { + return (string)trans('firefly.recurring_monthly', ['dayOfMonth' => $repetition->repetition_moment], $language); + } + if ('ndom' === $repetition->repetition_type) { + $parts = explode(',', $repetition->repetition_moment); + // first part is number of week, second is weekday. + $dayOfWeek = trans(sprintf('config.dow_%s', $parts[1]), [], $language); + + return (string)trans('firefly.recurring_ndom', ['weekday' => $dayOfWeek, 'dayOfMonth' => $parts[0]], $language); + } + if ('yearly' === $repetition->repetition_type) { + // + $today = Carbon::create()->endOfYear(); + $repDate = Carbon::createFromFormat('Y-m-d', $repetition->repetition_moment); + $diffInYears = $today->diffInYears($repDate); + $repDate->addYears($diffInYears); // technically not necessary. + $string = $repDate->formatLocalized(trans('config.month_and_day_no_year')); + + return (string)trans('firefly.recurring_yearly', ['date' => $string], $language); + } + + return ''; } @@ -435,8 +435,10 @@ class RecurringRepository implements RecurringRepositoryInterface * @param array $dates * * @return array + * @SuppressWarnings(PHPMD.CyclomaticComplexity) + * @SuppressWarnings(PHPMD.ExcessiveMethodLength) */ - protected function filterWeekends(RecurrenceRepetition $repetition, array $dates): array + private function filterWeekends(RecurrenceRepetition $repetition, array $dates): array { if ((int)$repetition->weekend === RecurrenceRepetition::WEEKEND_DO_NOTHING) { Log::debug('Repetition will not be filtered on weekend days.'); @@ -525,6 +527,7 @@ class RecurringRepository implements RecurringRepositoryInterface * @param string $moment * * @return array + * @SuppressWarnings(PHPMD.CyclomaticComplexity) */ private function getMonthlyInRange(Carbon $start, Carbon $end, int $skipMod, string $moment): array { @@ -604,6 +607,7 @@ class RecurringRepository implements RecurringRepositoryInterface * @param string $moment * * @return array + * @SuppressWarnings(PHPMD.CyclomaticComplexity) */ private function getWeeklyInRange(Carbon $start, Carbon $end, int $skipMod, string $moment): array { @@ -834,6 +838,7 @@ class RecurringRepository implements RecurringRepositoryInterface * @param string $moment * * @return array + * @SuppressWarnings(PHPMD.CyclomaticComplexity) */ private function getYearlyInRange(Carbon $start, Carbon $end, int $skipMod, string $moment): array { diff --git a/app/Repositories/Recurring/RecurringRepositoryInterface.php b/app/Repositories/Recurring/RecurringRepositoryInterface.php index d34d8ed8ab..73e3e28d0c 100644 --- a/app/Repositories/Recurring/RecurringRepositoryInterface.php +++ b/app/Repositories/Recurring/RecurringRepositoryInterface.php @@ -63,24 +63,23 @@ interface RecurringRepositoryInterface /** * Get the budget ID from a recurring transaction transaction. * - * @param RecurrenceTransaction $recurrenceTransaction + * @param RecurrenceTransaction $recTransaction * * @return null|int */ - public function getBudget(RecurrenceTransaction $recurrenceTransaction): ?int; + public function getBudget(RecurrenceTransaction $recTransaction): ?int; /** * Get the category from a recurring transaction transaction. * - * @param RecurrenceTransaction $recurrenceTransaction + * @param RecurrenceTransaction $recTransaction * * @return null|string */ - public function getCategory(RecurrenceTransaction $recurrenceTransaction): ?string; + public function getCategory(RecurrenceTransaction $recTransaction): ?string; /** * Returns the journals created for this recurrence, possibly limited by time. - * TODO make consistent with getTransactions * * @param Recurrence $recurrence * @param Carbon|null $start diff --git a/app/Repositories/Rule/RuleRepository.php b/app/Repositories/Rule/RuleRepository.php index c143a2e2ce..a0b4ae4b3b 100644 --- a/app/Repositories/Rule/RuleRepository.php +++ b/app/Repositories/Rule/RuleRepository.php @@ -32,6 +32,8 @@ use Illuminate\Support\Collection; /** * Class RuleRepository. + * + * @SuppressWarnings(PHPMD.TooManyPublicMethods) */ class RuleRepository implements RuleRepositoryInterface { @@ -386,7 +388,7 @@ class RuleRepository implements RuleRepositoryInterface private function storeActions(Rule $rule, array $data): bool { $order = 1; - foreach ($data['rule-actions'] as $index => $action) { + foreach ($data['rule-actions'] as $action) { $value = $action['value'] ?? ''; $stopProcessing = $action['stop-processing'] ?? false; @@ -422,7 +424,7 @@ class RuleRepository implements RuleRepositoryInterface ]; $this->storeTrigger($rule, $triggerValues); - foreach ($data['rule-triggers'] as $index => $trigger) { + foreach ($data['rule-triggers'] as $trigger) { $value = $trigger['value'] ?? ''; $stopProcessing = $trigger['stop-processing'] ?? false; diff --git a/app/Repositories/Tag/TagRepository.php b/app/Repositories/Tag/TagRepository.php index fc5526f74f..60b2989fcd 100644 --- a/app/Repositories/Tag/TagRepository.php +++ b/app/Repositories/Tag/TagRepository.php @@ -35,6 +35,8 @@ use Log; /** * Class TagRepository. + * + * @SuppressWarnings(PHPMD.TooManyPublicMethods) */ class TagRepository implements TagRepositoryInterface { @@ -225,6 +227,7 @@ class TagRepository implements TagRepositoryInterface * @param Carbon|null $end * * @return array + * @SuppressWarnings(PHPMD.CyclomaticComplexity) */ public function sumsOfTag(Tag $tag, ?Carbon $start, ?Carbon $end): array { @@ -267,26 +270,22 @@ class TagRepository implements TagRepositoryInterface public function tagCloud(?int $year): array { // Some vars - $tags = $this->getTagsInYear($year); - $max = $this->getMaxAmount($tags); - $min = $this->getMinAmount($tags); - $diff = bcsub($max, $min); - $return = []; - Log::debug(sprintf('Minimum is %s, maximum is %s, difference is %s', $min, $max, $diff)); - - // default scale is from 12 to 24, so 12 points. - $minimumFont = '12'; + $tags = $this->getTagsInYear($year); + $max = $this->getMaxAmount($tags); + $min = $this->getMinAmount($tags); + $diff = bcsub($max, $min); + $return = []; + $minimumFont = '12'; // default scale is from 12 to 24, so 12 points. $maxPoints = '12'; $pointsPerCoin = '0'; - // for each full coin in tag, add so many points: - if (0 !== bccomp($diff, '0')) { + Log::debug(sprintf('Minimum is %s, maximum is %s, difference is %s', $min, $max, $diff)); + + if (0 !== bccomp($diff, '0')) {// for each full coin in tag, add so many points $pointsPerCoin = bcdiv($maxPoints, $diff); } - Log::debug(sprintf('Each coin in a tag earns it %s points', $pointsPerCoin)); - /** @var Tag $tag */ foreach ($tags as $tag) { $amount = (string)$tag->amount_sum; @@ -349,6 +348,7 @@ class TagRepository implements TagRepositoryInterface * @param Collection $tags * * @return string + * @SuppressWarnings(PHPMD.CyclomaticComplexity) */ private function getMinAmount(Collection $tags): string { diff --git a/app/Repositories/User/UserRepository.php b/app/Repositories/User/UserRepository.php index 39508717cd..c9d3ef52ff 100644 --- a/app/Repositories/User/UserRepository.php +++ b/app/Repositories/User/UserRepository.php @@ -31,6 +31,8 @@ use Log; /** * Class UserRepository. + * + * @SuppressWarnings(PHPMD.TooManyPublicMethods) */ class UserRepository implements UserRepositoryInterface { diff --git a/app/Support/Import/Information/GetSpectreCustomerTrait.php b/app/Support/Import/Information/GetSpectreCustomerTrait.php index cc0664a7d9..e638d7ce53 100644 --- a/app/Support/Import/Information/GetSpectreCustomerTrait.php +++ b/app/Support/Import/Information/GetSpectreCustomerTrait.php @@ -53,7 +53,6 @@ trait GetSpectreCustomerTrait $request->setUser($importJob->user); $request->call(); - // todo what if customer is still null? $customer = $request->getCustomer(); } diff --git a/app/Support/Import/JobConfiguration/File/ConfigureRolesHandler.php b/app/Support/Import/JobConfiguration/File/ConfigureRolesHandler.php index 0c73e691df..fd633abd55 100644 --- a/app/Support/Import/JobConfiguration/File/ConfigureRolesHandler.php +++ b/app/Support/Import/JobConfiguration/File/ConfigureRolesHandler.php @@ -386,7 +386,6 @@ class ConfigureRolesHandler implements FileConfigurationInterface /** * Save the column count in the job. It's used in a later stage. - * TODO move config out of this method (make it a parameter). * * @return void */ diff --git a/app/Support/Import/JobConfiguration/Spectre/ChooseAccountsHandler.php b/app/Support/Import/JobConfiguration/Spectre/ChooseAccountsHandler.php index 478041a396..f1ccd1b25b 100644 --- a/app/Support/Import/JobConfiguration/Spectre/ChooseAccountsHandler.php +++ b/app/Support/Import/JobConfiguration/Spectre/ChooseAccountsHandler.php @@ -65,7 +65,6 @@ class ChooseAccountsHandler implements SpectreJobConfigurationInterface $importAccounts = $config['account_mapping'] ?? []; $complete = \count($importAccounts) > 0 && $importAccounts !== [0 => 0]; if ($complete) { - // todo also actually validate content. Log::debug('Looks like user has mapped import accounts to Firefly III accounts', $importAccounts); $this->repository->setStage($this->importJob, 'go-for-import'); } diff --git a/app/Support/Twig/Extension/Transaction.php b/app/Support/Twig/Extension/Transaction.php index cd5b488956..2eda973b52 100644 --- a/app/Support/Twig/Extension/Transaction.php +++ b/app/Support/Twig/Extension/Transaction.php @@ -214,8 +214,6 @@ class Transaction extends Twig_Extension } /** - * TODO improve code - * * @param TransactionModel $transaction * * @return string diff --git a/app/Validation/FireflyValidator.php b/app/Validation/FireflyValidator.php index db51337bd4..1034b1ef63 100644 --- a/app/Validation/FireflyValidator.php +++ b/app/Validation/FireflyValidator.php @@ -242,8 +242,6 @@ class FireflyValidator extends Validator } /** - * TODO lots of if-else because of API calls. - * * @param $attribute * * @return bool @@ -305,8 +303,6 @@ class FireflyValidator extends Validator } /** - * TODO This method uses a lot of if-then to handle the API calls as well. Fix. - * * @param $attribute * * @return bool @@ -449,19 +445,6 @@ class FireflyValidator extends Validator return false; } - /** - * TODO fill me. - * - * @param $attribute - * @param $value - * @param $parameters - * - * @return bool - */ - public function validateRepetitionMoment($attribute, $value, $parameters): bool { - - } - /** * @SuppressWarnings(PHPMD.UnusedFormalParameter) * @@ -578,8 +561,6 @@ class FireflyValidator extends Validator } /** - * TODO this method needs a lot of logic to be able to handle API calls. Fix that. - * * @param int $index * * @return string @@ -595,8 +576,6 @@ class FireflyValidator extends Validator } /** - * TODO this method needs a lot of logic to be able to handle API calls. Fix that. - * * @param int $index * * @return string @@ -612,8 +591,6 @@ class FireflyValidator extends Validator } /** - * TODO this method needs a lot of logic to be able to handle API calls. Fix that. - * * @param int $index * * @return string @@ -629,8 +606,6 @@ class FireflyValidator extends Validator } /** - * TODO this method needs a lot of logic to be able to handle API calls. Fix that. - * * @param int $index * * @return string