From 1068dcb8a437efb243e404c750b62dd8cc2bc427 Mon Sep 17 00:00:00 2001 From: James Cole Date: Sun, 18 Jan 2015 09:49:32 +0100 Subject: [PATCH] Cleanup and refactor --- .../FireflyIII/Database/Account/Account.php | 16 +-- app/lib/FireflyIII/Database/Bill/Bill.php | 2 +- .../TransactionJournal/TransactionJournal.php | 18 ---- app/lib/FireflyIII/Event/Piggybank.php | 4 - app/lib/FireflyIII/Report/Report.php | 5 +- app/lib/FireflyIII/Shared/Toolkit/Date.php | 93 +++++++--------- app/lib/FireflyIII/Shared/Toolkit/Filter.php | 101 ++++++++---------- app/lib/FireflyIII/Shared/Toolkit/Form.php | 2 + 8 files changed, 91 insertions(+), 150 deletions(-) diff --git a/app/lib/FireflyIII/Database/Account/Account.php b/app/lib/FireflyIII/Database/Account/Account.php index 9fe77ea498..cb921b0baa 100644 --- a/app/lib/FireflyIII/Database/Account/Account.php +++ b/app/lib/FireflyIII/Database/Account/Account.php @@ -156,10 +156,7 @@ class Account implements CUDInterface, CommonDatabaseCallsInterface, AccountInte */ public function destroy(Eloquent $model) { - - // delete piggy banks - // delete journals: - $journals = \TransactionJournal::whereIn( + $journals = \TransactionJournal::whereIn( 'id', function (QueryBuilder $query) use ($model) { $query->select('transaction_journal_id') ->from('transactions')->whereIn( @@ -183,9 +180,6 @@ class Account implements CUDInterface, CommonDatabaseCallsInterface, AccountInte )->get(); } )->get(); - /* - * Get all transactions. - */ $transactions = []; /** @var \TransactionJournal $journal */ foreach ($journals as $journal) { @@ -195,18 +189,10 @@ class Account implements CUDInterface, CommonDatabaseCallsInterface, AccountInte } $journal->delete(); } - // also delete transactions. if (count($transactions) > 0) { \Transaction::whereIn('id', $transactions)->delete(); } - - - /* - * Trigger deletion: - */ \Event::fire('account.destroy', [$model]); - - // delete accounts: \Account::where( function (EloquentBuilder $q) use ($model) { $q->where('id', $model->id); diff --git a/app/lib/FireflyIII/Database/Bill/Bill.php b/app/lib/FireflyIII/Database/Bill/Bill.php index e987d4abc7..2110180426 100644 --- a/app/lib/FireflyIII/Database/Bill/Bill.php +++ b/app/lib/FireflyIII/Database/Bill/Bill.php @@ -114,7 +114,7 @@ class Bill implements CUDInterface, CommonDatabaseCallsInterface, BillInterface $warnings = new MessageBag; $successes = new MessageBag; $errors = new MessageBag; - if (isset($model['amount_min']) && isset($model['amount_max']) && floatval($model['amount_min']) > floatval($model['amount_max'])) { + if (floatval($model['amount_min']) > floatval($model['amount_max'])) { $errors->add('amount_max', 'Maximum amount can not be less than minimum amount.'); $errors->add('amount_min', 'Minimum amount can not be more than maximum amount.'); } diff --git a/app/lib/FireflyIII/Database/TransactionJournal/TransactionJournal.php b/app/lib/FireflyIII/Database/TransactionJournal/TransactionJournal.php index 10015a1afb..ba0b70bf27 100644 --- a/app/lib/FireflyIII/Database/TransactionJournal/TransactionJournal.php +++ b/app/lib/FireflyIII/Database/TransactionJournal/TransactionJournal.php @@ -158,31 +158,13 @@ class TransactionJournal implements TransactionJournalInterface, CUDInterface, C $journal->isValid(); $errors = $journal->getErrors(); - /* - * Is not in rules. - */ if (!isset($model['what'])) { $errors->add('description', 'Internal error: need to know type of transaction!'); } - /* - * Is not in rules. - */ $errors = $errors->merge($this->_validateAmount($model)); $errors = $errors->merge($this->_validateBudget($model)); $errors = $errors->merge($this->_validateAccount($model)); - - /* - * Add "OK" - */ - - /** - * else { - * $successes->add('account_from_id', 'OK'); - * $successes->add('account_to_id', 'OK'); - * } - * else { - */ $list = ['date', 'description', 'amount', 'budget_id', 'from', 'to', 'account_from_id', 'account_to_id', 'category', 'account_id', 'expense_account', 'revenue_account']; foreach ($list as $entry) { diff --git a/app/lib/FireflyIII/Event/Piggybank.php b/app/lib/FireflyIII/Event/Piggybank.php index 645f48432b..1e0b62c053 100644 --- a/app/lib/FireflyIII/Event/Piggybank.php +++ b/app/lib/FireflyIII/Event/Piggybank.php @@ -197,7 +197,6 @@ class PiggyBank foreach ($list as $entry) { $start = $entry->startdate; $target = $entry->targetdate; - // find a repetition on this date: $count = $entry->piggyBankrepetitions()->starts($start)->targets($target)->count(); if ($count == 0) { $repetition = new \PiggyBankRepetition; @@ -207,14 +206,11 @@ class PiggyBank $repetition->currentamount = 0; $repetition->save(); } - // then continue and do something in the current relevant time frame. - $currentTarget = clone $target; $currentStart = null; while ($currentTarget < $today) { $currentStart = \DateKit::subtractPeriod($currentTarget, $entry->rep_length, 0); $currentTarget = \DateKit::addPeriod($currentTarget, $entry->rep_length, 0); - // create if not exists: $count = $entry->piggyBankRepetitions()->starts($currentStart)->targets($currentTarget)->count(); if ($count == 0) { $repetition = new \PiggyBankRepetition; diff --git a/app/lib/FireflyIII/Report/Report.php b/app/lib/FireflyIII/Report/Report.php index d704cfb31a..b926e6662c 100644 --- a/app/lib/FireflyIII/Report/Report.php +++ b/app/lib/FireflyIII/Report/Report.php @@ -228,6 +228,8 @@ class Report implements ReportInterface } /** + * This method gets all incomes (journals) in a list. + * * @SuppressWarnings(PHPMD.UnusedFormalParameter) * * @param Carbon $date @@ -357,7 +359,6 @@ class Report implements ReportInterface } /** - * @SuppressWarnings(PHPMD.UnusedFormalParameter) * * @param Carbon $start * @param Carbon $end @@ -367,7 +368,7 @@ class Report implements ReportInterface */ public function revenueGroupedByAccount(Carbon $start, Carbon $end, $limit = 15) { - return $this->_queries->journalsByRevenueAccount($start, $end); + return $this->_queries->journalsByRevenueAccount($start, $end, $limit); diff --git a/app/lib/FireflyIII/Shared/Toolkit/Date.php b/app/lib/FireflyIII/Shared/Toolkit/Date.php index 7b41d9d273..443e343654 100644 --- a/app/lib/FireflyIII/Shared/Toolkit/Date.php +++ b/app/lib/FireflyIII/Shared/Toolkit/Date.php @@ -102,6 +102,8 @@ class Date } /** + * @SuppressWarnings("CyclomaticComplexity") // It's exactly 5. So I don't mind. + * * @param Carbon $theCurrentEnd * @param $repeatFreq * @param Carbon $maxDate @@ -111,37 +113,32 @@ class Date */ public function endOfX(Carbon $theCurrentEnd, $repeatFreq, Carbon $maxDate) { + $functionMap = [ + 'daily' => 'endOfDay', + 'week' => 'endOfWeek', + 'weekly' => 'endOfWeek', + 'month' => 'endOfMonth', + 'monthly' => 'endOfMonth', + 'quarter' => 'lastOfQuarter', + 'quarterly' => 'lastOfQuarter', + 'year' => 'endOfYear', + 'yearly' => 'endOfYear', + ]; + $specials = ['mont', 'monthly']; + $currentEnd = clone $theCurrentEnd; - switch ($repeatFreq) { - default: - throw new FireflyException('Cannot do endOfPeriod for $repeat_freq ' . $repeatFreq); - break; - case 'daily': - $currentEnd->endOfDay(); - break; - case 'week': - case 'weekly': - $currentEnd->endOfWeek(); - break; - case 'month': - case 'monthly': - $currentEnd->endOfMonth(); - break; - case 'quarter': - case 'quarterly': - $currentEnd->lastOfQuarter(); - break; - case 'half-year': - $month = intval($theCurrentEnd->format('m')); - $currentEnd->endOfYear(); - if ($month <= 6) { - $currentEnd->subMonths(6); - } - break; - case 'year': - case 'yearly': - $currentEnd->endOfYear(); - break; + + if (isset($functionMap[$repeatFreq])) { + $function = $functionMap[$repeatFreq]; + $currentEnd->$function(); + + } + if (isset($specials[$repeatFreq])) { + $month = intval($theCurrentEnd->format('m')); + $currentEnd->endOfYear(); + if ($month <= 6) { + $currentEnd->subMonths(6); + } } if ($currentEnd > $maxDate) { return clone $maxDate; @@ -159,29 +156,21 @@ class Date */ public function periodShow(Carbon $date, $repeatFrequency) { - switch ($repeatFrequency) { - default: - throw new FireflyException('No date formats for frequency "' . $repeatFrequency . '"!'); - break; - case 'daily': - return $date->format('j F Y'); - break; - case 'week': - case 'weekly': - return $date->format('\W\e\e\k W, Y'); - break; - case 'quarter': - return $date->format('F Y'); - break; - case 'monthly': - case 'month': - return $date->format('F Y'); - break; - case 'year': - case 'yearly': - return $date->format('Y'); - break; + $formatMap = [ + 'daily' => 'j F Y', + 'week' => '\W\e\e\k W, Y', + 'weekly' => '\W\e\e\k W, Y', + 'quarter' => 'F Y', + 'month' => 'F Y', + 'monthly' => 'F Y', + 'year' => 'Y', + 'yearly' => 'Y', + + ]; + if (isset($formatMap[$repeatFrequency])) { + return $date->format($formatMap[$repeatFrequency]); } + throw new FireflyException('No date formats for frequency "' . $repeatFrequency . '"!'); } /** diff --git a/app/lib/FireflyIII/Shared/Toolkit/Filter.php b/app/lib/FireflyIII/Shared/Toolkit/Filter.php index f8fdf42037..ae04ae6062 100644 --- a/app/lib/FireflyIII/Shared/Toolkit/Filter.php +++ b/app/lib/FireflyIII/Shared/Toolkit/Filter.php @@ -103,37 +103,31 @@ class Filter */ protected function updateEndDate($range, Carbon $start) { - $end = clone $start; - switch ($range) { - default: - throw new FireflyException('updateEndDate cannot handle $range ' . $range); - break; - case '1D': - $end->endOfDay(); - break; - case '1W': - $end->endOfWeek(); - break; - case '1M': - $end->endOfMonth(); - break; - case '3M': - $end->lastOfQuarter(); - break; - case '6M': - if (intval($start->format('m')) >= 7) { - $end->endOfYear(); - } else { - $end->startOfYear()->addMonths(6); - } - break; - case '1Y': - $end->endOfYear(); - break; + $functionMap = [ + '1D' => 'endOfDay', + '1W' => 'endOfWeek', + '1M' => 'endOfMonth', + '3M' => 'lastOfQuarter', + '1Y' => 'endOfYear', + ]; + $end = clone $start; + if (isset($functionMap[$range])) { + $function = $functionMap[$range]; + $end->$function(); + + return $end; } + if ($range == '6M') { + if (intval($start->format('m')) >= 7) { + $end->endOfYear(); + } else { + $end->startOfYear()->addMonths(6); + } - return $end; + return $end; + } + throw new FireflyException('updateEndDate cannot handle $range ' . $range); } /** @@ -145,37 +139,28 @@ class Filter */ protected function periodName($range, Carbon $date) { - switch ($range) { - default: - throw new FireflyException('No _periodName() for range "' . $range . '"'); - break; - case '1D': - return $date->format('jS F Y'); - break; - case '1W': - return 'week ' . $date->format('W, Y'); - break; - case '1M': - return $date->format('F Y'); - break; - case '3M': - $month = intval($date->format('m')); - - return 'Q' . ceil(($month / 12) * 4) . ' ' . $date->format('Y'); - break; - case '6M': - $month = intval($date->format('m')); - $half = ceil(($month / 12) * 2); - $halfName = $half == 1 ? 'first' : 'second'; - - return $halfName . ' half of ' . $date->format('d-m-Y'); - break; - case '1Y': - return $date->format('Y'); - break; - - + $formatMap = [ + '1D' => 'jS F Y', + '1W' => '\w\e\ek W, Y', + '1M' => 'F Y', + '1Y' => 'Y', + ]; + if (isset($formatMap[$range])) { + return $date->format($formatMap[$range]); } + if ($range == '3M') { + $month = intval($date->format('m')); + + return 'Q' . ceil(($month / 12) * 4) . ' ' . $date->format('Y'); + } + if ($range == '6M') { + $month = intval($date->format('m')); + $half = ceil(($month / 12) * 2); + $halfName = $half == 1 ? 'first' : 'second'; + + return $halfName . ' half of ' . $date->format('d-m-Y'); + } + throw new FireflyException('No _periodName() for range "' . $range . '"'); } /** diff --git a/app/lib/FireflyIII/Shared/Toolkit/Form.php b/app/lib/FireflyIII/Shared/Toolkit/Form.php index a5475a0456..1f51fe357a 100644 --- a/app/lib/FireflyIII/Shared/Toolkit/Form.php +++ b/app/lib/FireflyIII/Shared/Toolkit/Form.php @@ -12,6 +12,8 @@ use Illuminate\Support\Collection; class Form { /** + * @SuppressWarnings("CyclomaticComplexity") // It's exactly 5. So I don't mind. + * * Takes any collection and tries to make a sensible select list compatible array of it. * * @param Collection $set