From 20726078893d577374ae1aae723dc0743be39147 Mon Sep 17 00:00:00 2001 From: James Cole Date: Sat, 20 Dec 2014 22:07:21 +0100 Subject: [PATCH] Cleanup, mostly I removed the coding standard ignore instructions. --- app/controllers/BudgetController.php | 18 +- app/controllers/CategoryController.php | 3 +- app/controllers/GoogleChartController.php | 218 +++++++----------- app/controllers/ProfileController.php | 1 - app/lib/FireflyIII/Database/Budget/Budget.php | 17 +- app/lib/FireflyIII/Event/Piggybank.php | 2 - app/lib/FireflyIII/Shared/Toolkit/Form.php | 1 - app/models/Account.php | 2 - app/models/AccountMeta.php | 2 - app/models/Reminder.php | 2 - app/models/User.php | 2 - app/start/global.php | 1 - app/tests/TestCase.php | 2 - 13 files changed, 102 insertions(+), 169 deletions(-) diff --git a/app/controllers/BudgetController.php b/app/controllers/BudgetController.php index c3fc2b17cb..973f3336f8 100644 --- a/app/controllers/BudgetController.php +++ b/app/controllers/BudgetController.php @@ -7,9 +7,10 @@ use FireflyIII\Shared\Preferences\PreferencesInterface as Pref; /** * Class BudgetController * - * TODO move AJAX methods to their own BudgetAjaxController - * TODO Find out what constitutes proper camelCase - * TODO How is object coupling counted? + * @SuppressWarnings("CamelCase") // I'm fine with this. + * @SuppressWarnings("TooManyMethods") // I'm also fine with this. + * @SuppressWarnings("CyclomaticComplexity") // It's all 5. So ok. + * @SuppressWarnings("CouplingBetweenObjects") // There's only so much I can remove. * */ class BudgetController extends BaseController @@ -98,6 +99,7 @@ class BudgetController extends BaseController /** * The index of the budget controller contains all budgets and the current relevant limit repetition. + * TODO move currentRep to the repository. * * @return $this */ @@ -108,14 +110,8 @@ class BudgetController extends BaseController // loop the budgets: $budgets->each( function (Budget $budget) { - /** @noinspection PhpUndefinedFieldInspection */ - $budget->spent = $this->_repository->spentInMonth($budget, \Session::get('start', Carbon::now()->startOfMonth())); - /** @noinspection PhpUndefinedFieldInspection */ - $budget->currentRep = $budget->limitrepetitions()->where( - 'limit_repetitions.startdate', \Session::get('start', Carbon::now()->startOfMonth())->format('Y-m-d') - )->first( - ['limit_repetitions.*'] - ); + $budget->spent = $this->_repository->spentInMonth($budget, \Session::get('start', Carbon::now()->startOfMonth())); + $budget->currentRep = $this->_repository->getRepetitionByDate($budget, \Session::get('start', Carbon::now()->startOfMonth())); } ); diff --git a/app/controllers/CategoryController.php b/app/controllers/CategoryController.php index 11c68ab7e6..b6d6da5df8 100644 --- a/app/controllers/CategoryController.php +++ b/app/controllers/CategoryController.php @@ -4,7 +4,8 @@ use FireflyIII\Exception\FireflyException; /** * - * TODO Find out what constitutes proper camelCase + * @SuppressWarnings("CamelCase") // I'm fine with this. + * @SuppressWarnings("CyclomaticComplexity") // It's all 5. So ok. * * Class CategoryController */ diff --git a/app/controllers/GoogleChartController.php b/app/controllers/GoogleChartController.php index 61dfe4db66..fc7f645ded 100644 --- a/app/controllers/GoogleChartController.php +++ b/app/controllers/GoogleChartController.php @@ -1,6 +1,7 @@ = $current) { - $this->_chart->addRow(clone $current, $current > Carbon::now() ? null : Steam::balance($account, $current)); + $this->_chart->addRow(clone $current, Steam::balance($account, $current)); $current->addDay(); } @@ -85,7 +86,7 @@ class GoogleChartController extends BaseController $this->_chart->addColumn('Balance for ' . $account->name, 'number'); } $start = Session::get('start', Carbon::now()->startOfMonth()); - $end = Session::get('end'); + $end = Session::get('end', Carbon::now()->endOfMonth()); $current = clone $start; while ($end >= $current) { @@ -151,44 +152,35 @@ class GoogleChartController extends BaseController */ public function allCategoriesHomeChart() { - $data = []; + $this->_chart->addColumn('Category', 'string'); + $this->_chart->addColumn('Spent', 'number'); - /** @var \Grumpydictator\Gchart\GChart $chart */ - $chart = App::make('gchart'); - $chart->addColumn('Category', 'string'); - $chart->addColumn('Spent', 'number'); - - /** @var \FireflyIII\Database\TransactionJournal\TransactionJournal $tj */ - $tj = App::make('FireflyIII\Database\TransactionJournal\TransactionJournal'); - - /* - * Get the journals: - */ - $journals = $tj->getInDateRange(Session::get('start', Carbon::now()->startOfMonth()), Session::get('end')); - - /** @var \TransactionJournal $journal */ - foreach ($journals as $journal) { - if ($journal->transactionType->type == 'Withdrawal') { - $amount = $journal->getAmount(); - $category = $journal->categories()->first(); - if (!is_null($category)) { - if (isset($data[$category->name])) { - $data[$category->name] += $amount; - } else { - $data[$category->name] = $amount; - } - } + // query! + $set = \TransactionJournal::leftJoin( + 'transactions', + function (JoinClause $join) { + $join->on('transaction_journals.id', '=', 'transactions.transaction_journal_id')->where('amount', '>', 0); } - } - arsort($data); - foreach ($data as $key => $entry) { - $chart->addRow($key, $entry); + ) + ->leftJoin( + 'category_transaction_journal', 'category_transaction_journal.transaction_journal_id', '=', 'transaction_journals.id' + ) + ->leftJoin('categories', 'categories.id', '=', 'category_transaction_journal.category_id') + ->leftJoin('transaction_types', 'transaction_types.id', '=', 'transaction_journals.transaction_type_id') + ->before(Session::get('end', Carbon::now()->endOfMonth())) + ->after(Session::get('start', Carbon::now()->startOfMonth())) + ->where('transaction_types.type', 'Withdrawal') + ->groupBy('categories.id') + ->orderBy('sum', 'DESC') + ->get(['categories.id', 'categories.name', DB::Raw('SUM(`transactions`.`amount`) AS `sum`')]); + foreach ($set as $entry) { + $entry->name = strlen($entry->name) == 0 ? '(no category)' : $entry->name; + $this->_chart->addRow($entry->name, floatval($entry->sum)); } + $this->_chart->generate(); - $chart->generate(); - - return Response::json($chart->getData()); + return Response::json($this->_chart->getData()); } @@ -203,10 +195,8 @@ class GoogleChartController extends BaseController $start = clone $repetition->startdate; $end = $repetition->enddate; - /** @var \Grumpydictator\Gchart\GChart $chart */ - $chart = App::make('gchart'); - $chart->addColumn('Day', 'date'); - $chart->addColumn('Left', 'number'); + $this->_chart->addColumn('Day', 'date'); + $this->_chart->addColumn('Left', 'number'); $amount = $repetition->amount; @@ -217,12 +207,12 @@ class GoogleChartController extends BaseController */ $sum = floatval($budget->transactionjournals()->lessThan(0)->transactionTypes(['Withdrawal'])->onDate($start)->sum('amount')); $amount += $sum; - $chart->addRow(clone $start, $amount); + $this->_chart->addRow(clone $start, $amount); $start->addDay(); } - $chart->generate(); + $this->_chart->generate(); - return Response::json($chart->getData()); + return Response::json($this->_chart->getData()); } @@ -243,16 +233,13 @@ class GoogleChartController extends BaseController /** @var \FireflyIII\Database\Budget\Budget $repos */ $repos = App::make('FireflyIII\Database\Budget\Budget'); - /** @var \Grumpydictator\Gchart\GChart $chart */ - $chart = App::make('gchart'); - $chart->addColumn('Month', 'date'); - $chart->addColumn('Budgeted', 'number'); - $chart->addColumn('Spent', 'number'); + $this->_chart->addColumn('Month', 'date'); + $this->_chart->addColumn('Budgeted', 'number'); + $this->_chart->addColumn('Spent', 'number'); $end = clone $start; $end->endOfYear(); while ($start <= $end) { - $spent = $repos->spentInMonth($component, $start); $repetition = $repos->repetitionOnStartingOnDate($component, $start); if ($repetition) { @@ -261,15 +248,15 @@ class GoogleChartController extends BaseController $budgeted = null; } - $chart->addRow(clone $start, $budgeted, $spent); + $this->_chart->addRow(clone $start, $budgeted, $spent); $start->addMonth(); } - $chart->generate(); + $this->_chart->generate(); - return Response::json($chart->getData()); + return Response::json($this->_chart->getData()); } @@ -291,11 +278,9 @@ class GoogleChartController extends BaseController /** @var \FireflyIII\Database\Category\Category $repos */ $repos = App::make('FireflyIII\Database\Category\Category'); - /** @var \Grumpydictator\Gchart\GChart $chart */ - $chart = App::make('gchart'); - $chart->addColumn('Month', 'date'); - $chart->addColumn('Budgeted', 'number'); - $chart->addColumn('Spent', 'number'); + $this->_chart->addColumn('Month', 'date'); + $this->_chart->addColumn('Budgeted', 'number'); + $this->_chart->addColumn('Spent', 'number'); $end = clone $start; $end->endOfYear(); @@ -304,15 +289,15 @@ class GoogleChartController extends BaseController $spent = $repos->spentInMonth($component, $start); $budgeted = null; - $chart->addRow(clone $start, $budgeted, $spent); + $this->_chart->addRow(clone $start, $budgeted, $spent); $start->addMonth(); } - $chart->generate(); + $this->_chart->generate(); - return Response::json($chart->getData()); + return Response::json($this->_chart->getData()); } @@ -324,20 +309,18 @@ class GoogleChartController extends BaseController */ public function piggyBankHistory(\Piggybank $piggybank) { - /** @var \Grumpydictator\Gchart\GChart $chart */ - $chart = App::make('gchart'); - $chart->addColumn('Date', 'date'); - $chart->addColumn('Balance', 'number'); + $this->_chart->addColumn('Date', 'date'); + $this->_chart->addColumn('Balance', 'number'); $set = \DB::table('piggy_bank_events')->where('piggybank_id', $piggybank->id)->groupBy('date')->get(['date', DB::Raw('SUM(`amount`) AS `sum`')]); foreach ($set as $entry) { - $chart->addRow(new Carbon($entry->date), floatval($entry->sum)); + $this->_chart->addRow(new Carbon($entry->date), floatval($entry->sum)); } - $chart->generate(); + $this->_chart->generate(); - return Response::json($chart->getData()); + return Response::json($this->_chart->getData()); } @@ -349,12 +332,10 @@ class GoogleChartController extends BaseController public function recurringOverview(RecurringTransaction $recurring) { - /** @var \Grumpydictator\Gchart\GChart $chart */ - $chart = App::make('gchart'); - $chart->addColumn('Date', 'date'); - $chart->addColumn('Max amount', 'number'); - $chart->addColumn('Min amount', 'number'); - $chart->addColumn('Current entry', 'number'); + $this->_chart->addColumn('Date', 'date'); + $this->_chart->addColumn('Max amount', 'number'); + $this->_chart->addColumn('Min amount', 'number'); + $this->_chart->addColumn('Current entry', 'number'); // get first transaction or today for start: $first = $recurring->transactionjournals()->orderBy('date', 'ASC')->first(); @@ -372,13 +353,13 @@ class GoogleChartController extends BaseController $amount = 0; } unset($result); - $chart->addRow(clone $start, $recurring->amount_max, $recurring->amount_min, $amount); + $this->_chart->addRow(clone $start, $recurring->amount_max, $recurring->amount_min, $amount); $start = DateKit::addPeriod($start, $recurring->repeat_freq, 0); } - $chart->generate(); + $this->_chart->generate(); - return Response::json($chart->getData()); + return Response::json($this->_chart->getData()); } @@ -388,53 +369,24 @@ class GoogleChartController extends BaseController */ public function recurringTransactionsOverview() { - - /* - * Set of paid transaction journals. - * Set of unpaid recurring transactions. - */ $paid = ['items' => [], 'amount' => 0]; $unpaid = ['items' => [], 'amount' => 0]; - - /** @var \Grumpydictator\Gchart\GChart $chart */ - $chart = App::make('gchart'); - $chart->addColumn('Name', 'string'); - $chart->addColumn('Amount', 'number'); + $this->_chart->addColumn('Name', 'string'); + $this->_chart->addColumn('Amount', 'number'); /** @var \FireflyIII\Database\RecurringTransaction\RecurringTransaction $rcr */ - $rcr = App::make('FireflyIII\Database\RecurringTransaction\RecurringTransaction'); - + $rcr = App::make('FireflyIII\Database\RecurringTransaction\RecurringTransaction'); $recurring = $rcr->getActive(); - /** @var \RecurringTransaction $entry */ foreach ($recurring as $entry) { - /* - * Start another loop starting at the $date. - */ - $start = clone $entry->date; - $end = Carbon::now(); - - /* - * The jump we make depends on the $repeat_freq - */ + $start = clone $entry->date; + $end = Carbon::now(); $current = clone $start; - while ($current <= $end) { - /* - * Get end of period for $current: - */ $currentEnd = DateKit::endOfPeriod($current, $entry->repeat_freq); - - /* - * In the current session range? - */ - if (\Session::get('end') >= $current and $currentEnd >= \Session::get('start', Carbon::now()->startOfMonth())) { - /* - * Lets see if we've already spent money on this recurring transaction (it hath recurred). - */ + if (\Session::get('end', Carbon::now()->end§OfMonth()) >= $current and $currentEnd >= \Session::get('start', Carbon::now()->startOfMonth())) { /** @var TransactionJournal $set */ $journal = $rcr->getJournalForRecurringInRange($entry, $current, $currentEnd); - if (is_null($journal)) { $unpaid['items'][] = $entry->name; $unpaid['amount'] += (($entry->amount_max + $entry->amount_min) / 2); @@ -444,23 +396,15 @@ class GoogleChartController extends BaseController $paid['amount'] += $amount; } } - - /* - * Add some time for the next loop! - */ $current = DateKit::addPeriod($current, $entry->repeat_freq, intval($entry->skip)); - } - } /** @var \RecurringTransaction $entry */ - $chart->addRow('Unpaid: ' . join(', ', $unpaid['items']), $unpaid['amount']); - $chart->addRow('Paid: ' . join(', ', $paid['items']), $paid['amount']); - - $chart->generate(); - - return Response::json($chart->getData()); + $this->_chart->addRow('Unpaid: ' . join(', ', $unpaid['items']), $unpaid['amount']); + $this->_chart->addRow('Paid: ' . join(', ', $paid['items']), $paid['amount']); + $this->_chart->generate(); + return Response::json($this->_chart->getData()); } /** @@ -475,11 +419,9 @@ class GoogleChartController extends BaseController } catch (Exception $e) { App::abort(500); } - /** @var \Grumpydictator\Gchart\GChart $chart */ - $chart = App::make('gchart'); - $chart->addColumn('Month', 'date'); - $chart->addColumn('Income', 'number'); - $chart->addColumn('Expenses', 'number'); + $this->_chart->addColumn('Month', 'date'); + $this->_chart->addColumn('Income', 'number'); + $this->_chart->addColumn('Expenses', 'number'); /** @var \FireflyIII\Database\TransactionJournal\TransactionJournal $repository */ $repository = App::make('FireflyIII\Database\TransactionJournal\TransactionJournal'); @@ -492,14 +434,14 @@ class GoogleChartController extends BaseController $income = $repository->getSumOfIncomesByMonth($start); $expense = $repository->getSumOfExpensesByMonth($start); - $chart->addRow(clone $start, $income, $expense); + $this->_chart->addRow(clone $start, $income, $expense); $start->addMonth(); } - $chart->generate(); + $this->_chart->generate(); - return Response::json($chart->getData()); + return Response::json($this->_chart->getData()); } @@ -515,11 +457,9 @@ class GoogleChartController extends BaseController } catch (Exception $e) { App::abort(500); } - /** @var \Grumpydictator\Gchart\GChart $chart */ - $chart = App::make('gchart'); - $chart->addColumn('Summary', 'string'); - $chart->addColumn('Income', 'number'); - $chart->addColumn('Expenses', 'number'); + $this->_chart->addColumn('Summary', 'string'); + $this->_chart->addColumn('Income', 'number'); + $this->_chart->addColumn('Expenses', 'number'); /** @var \FireflyIII\Database\TransactionJournal\TransactionJournal $repository */ $repository = App::make('FireflyIII\Database\TransactionJournal\TransactionJournal'); @@ -538,14 +478,14 @@ class GoogleChartController extends BaseController $start->addMonth(); } - $chart->addRow('Sum', $income, $expense); + $this->_chart->addRow('Sum', $income, $expense); $count = $count > 0 ? $count : 1; - $chart->addRow('Average', ($income / $count), ($expense / $count)); + $this->_chart->addRow('Average', ($income / $count), ($expense / $count)); - $chart->generate(); + $this->_chart->generate(); - return Response::json($chart->getData()); + return Response::json($this->_chart->getData()); } } \ No newline at end of file diff --git a/app/controllers/ProfileController.php b/app/controllers/ProfileController.php index b11bd1b5e6..d5373ec0c5 100644 --- a/app/controllers/ProfileController.php +++ b/app/controllers/ProfileController.php @@ -32,7 +32,6 @@ class ProfileController extends BaseController { // old, new1, new2 - /** @noinspection PhpUndefinedFieldInspection */ if (!Hash::check(Input::get('old'), Auth::user()->password)) { Session::flash('error', 'Invalid current password!'); diff --git a/app/lib/FireflyIII/Database/Budget/Budget.php b/app/lib/FireflyIII/Database/Budget/Budget.php index 1fee659e8a..498456d46f 100644 --- a/app/lib/FireflyIII/Database/Budget/Budget.php +++ b/app/lib/FireflyIII/Database/Budget/Budget.php @@ -48,7 +48,7 @@ class Budget implements CUD, CommonDatabaseCalls, BudgetInterface */ public function store(array $data) { - $budget = new \Budget($data); + $budget = new \Budget($data); if (!$budget->isValid()) { \Log::error('Could not store budget: ' . $budget->getErrors()->toJson()); @@ -85,9 +85,9 @@ class Budget implements CUD, CommonDatabaseCalls, BudgetInterface { $warnings = new MessageBag; $successes = new MessageBag; - $budget = new \Budget($model); + $budget = new \Budget($model); $budget->isValid(); - $errors = $budget->getErrors(); + $errors = $budget->getErrors(); if (!$errors->has('name')) { $successes->add('name', 'OK'); @@ -180,6 +180,17 @@ class Budget implements CUD, CommonDatabaseCalls, BudgetInterface return \Paginator::make($items, $count, $take); } + /** + * @param \Budget $budget + * @param Carbon $date + * + * @return \LimitRepetition + */ + public function getRepetitionByDate(\Budget $budget, Carbon $date) + { + return $budget->limitrepetitions()->where('limit_repetitions.startdate', $date)->first(['limit_repetitions.*']); + } + /** * @param \Budget $budget * @param int $limit diff --git a/app/lib/FireflyIII/Event/Piggybank.php b/app/lib/FireflyIII/Event/Piggybank.php index 0190368234..bd9ee555f5 100644 --- a/app/lib/FireflyIII/Event/Piggybank.php +++ b/app/lib/FireflyIII/Event/Piggybank.php @@ -213,7 +213,6 @@ class Piggybank $currentTarget = clone $target; $currentStart = null; while ($currentTarget < $today) { - /** @noinspection PhpInternalEntityUsedInspection */ $currentStart = \DateKit::subtractPeriod($currentTarget, $entry->rep_length, 0); $currentTarget = \DateKit::addPeriod($currentTarget, $entry->rep_length, 0); // create if not exists: @@ -254,7 +253,6 @@ class Piggybank if ($journal->piggybankevents()->count() > 0) { - /** @noinspection PhpUnusedLocalVariableInspection */ $event = $journal->piggybankevents()->orderBy('date', 'DESC')->orderBy('id', 'DESC')->first(); $eventSum = floatval($journal->piggybankevents()->orderBy('date', 'DESC')->orderBy('id', 'DESC')->sum('amount')); diff --git a/app/lib/FireflyIII/Shared/Toolkit/Form.php b/app/lib/FireflyIII/Shared/Toolkit/Form.php index c610ab8657..036257fb1a 100644 --- a/app/lib/FireflyIII/Shared/Toolkit/Form.php +++ b/app/lib/FireflyIII/Shared/Toolkit/Form.php @@ -28,7 +28,6 @@ class Form $fields = ['title', 'name', 'description']; /** @var \Eloquent $entry */ foreach ($set as $entry) { - /** @noinspection PhpUndefinedFieldInspection */ $id = intval($entry->id); $title = null; diff --git a/app/models/Account.php b/app/models/Account.php index aeef80b5a4..e69680cdf1 100644 --- a/app/models/Account.php +++ b/app/models/Account.php @@ -23,10 +23,8 @@ class Account extends Eloquent 'active' => 'required|boolean' ]; - // @codingStandardsIgnoreStart protected $dates = ['deleted_at', 'created_at', 'updated_at']; protected $fillable = ['name', 'user_id', 'account_type_id', 'active']; - // @codingStandardsIgnoreEnd /** * Account type. diff --git a/app/models/AccountMeta.php b/app/models/AccountMeta.php index 3e3da606ba..40314b761a 100644 --- a/app/models/AccountMeta.php +++ b/app/models/AccountMeta.php @@ -19,10 +19,8 @@ class AccountMeta extends Eloquent /** * @var array */ - // @codingStandardsIgnoreStart protected $fillable = ['account_id', 'name', 'date']; protected $table = 'account_meta'; - // @codingStandardsIgnoreEnd /** * @return \Illuminate\Database\Eloquent\Relations\BelongsTo diff --git a/app/models/Reminder.php b/app/models/Reminder.php index f4a0ab908d..b5002501b3 100644 --- a/app/models/Reminder.php +++ b/app/models/Reminder.php @@ -10,9 +10,7 @@ class Reminder extends Eloquent { use ValidatingTrait; - // @codingStandardsIgnoreStart protected $table = 'reminders'; - // @codingStandardsIgnoreEnd /** * @param $value diff --git a/app/models/User.php b/app/models/User.php index 507a9541f7..cef29d346c 100644 --- a/app/models/User.php +++ b/app/models/User.php @@ -22,11 +22,9 @@ class User extends Eloquent implements UserInterface, RemindableInterface 'password' => 'required|between:60,60', 'reset' => 'between:32,32', ]; - // @codingStandardsIgnoreStart protected $fillable = ['email']; protected $hidden = ['remember_token']; protected $table = 'users'; - // @codingStandardsIgnoreEnd /** * @return \Illuminate\Database\Eloquent\Relations\HasMany diff --git a/app/start/global.php b/app/start/global.php index e12d44f2ee..36f6db85a9 100644 --- a/app/start/global.php +++ b/app/start/global.php @@ -127,5 +127,4 @@ App::down( | */ -/** @noinspection PhpIncludeInspection */ require app_path() . '/filters.php'; diff --git a/app/tests/TestCase.php b/app/tests/TestCase.php index 308b9a7296..e3ce3bcc0d 100644 --- a/app/tests/TestCase.php +++ b/app/tests/TestCase.php @@ -13,9 +13,7 @@ class TestCase extends Illuminate\Foundation\Testing\TestCase */ public function createApplication() { - /** @noinspection PhpUnusedLocalVariableInspection */ $unitTesting = true; - /** @noinspection PhpUnusedLocalVariableInspection */ $testEnvironment = 'testing'; return require __DIR__ . '/../../bootstrap/start.php';