diff --git a/app/controllers/PiggybankController.php b/app/controllers/PiggybankController.php index 9f279598c1..673a2c31f0 100644 --- a/app/controllers/PiggybankController.php +++ b/app/controllers/PiggybankController.php @@ -290,11 +290,12 @@ class PiggyBankController extends BaseController Session::flash('errors', $messages['errors']); if ($messages['errors']->count() > 0) { Session::flash('error', 'Could not store piggy bank: ' . $messages['errors']->first()); + return Redirect::route('piggy_banks.create')->withInput(); } // return to create screen: - if ($data['post_submit_action'] == 'validate_only' || $messages['errors']->count() > 0) { + if ($data['post_submit_action'] == 'validate_only') { return Redirect::route('piggy_banks.create')->withInput(); } diff --git a/app/controllers/RepeatedExpenseController.php b/app/controllers/RepeatedExpenseController.php index 32dbc7c0ce..73494b0377 100644 --- a/app/controllers/RepeatedExpenseController.php +++ b/app/controllers/RepeatedExpenseController.php @@ -178,6 +178,8 @@ class RepeatedExpenseController extends BaseController } /** + * @SuppressWarnings("CyclomaticComplexity") // It's exactly 5. So I don't mind. + * * @param PiggyBank $repeatedExpense * * @return $this @@ -194,7 +196,6 @@ class RepeatedExpenseController extends BaseController $data['remind_me'] = isset($data['remind_me']) ? 1 : 0; $data['user_id'] = Auth::user()->id; - // always validate: $messages = $this->_repository->validate($data); Session::flash('warnings', $messages['warnings']); @@ -202,10 +203,11 @@ class RepeatedExpenseController extends BaseController Session::flash('errors', $messages['errors']); if ($messages['errors']->count() > 0) { Session::flash('error', 'Could not update repeated expense: ' . $messages['errors']->first()); + return Redirect::route('repeated.edit', $repeatedExpense->id)->withInput(); } // return to update screen: - if ($data['post_submit_action'] == 'validate_only' || $messages['errors']->count() > 0) { + if ($data['post_submit_action'] == 'validate_only') { return Redirect::route('repeated.edit', $repeatedExpense->id)->withInput(); } diff --git a/app/controllers/ReportController.php b/app/controllers/ReportController.php index e88f69279c..9d2f730003 100644 --- a/app/controllers/ReportController.php +++ b/app/controllers/ReportController.php @@ -48,15 +48,15 @@ class ReportController extends BaseController } catch (Exception $e) { return View::make('error')->with('message', 'Invalid date'); } - $date = new Carbon($year . '-' . $month . '-01'); - $dayEarly = clone $date; + $date = new Carbon($year . '-' . $month . '-01'); + $dayEarly = clone $date; $subTitle = 'Budget report for ' . $date->format('F Y'); $subTitleIcon = 'fa-calendar'; - $dayEarly = $dayEarly->subDay(); - $accounts = $this->_repository->getAccountListBudgetOverview($date); - $budgets = $this->_repository->getBudgetsForMonth($date); + $dayEarly = $dayEarly->subDay(); + $accounts = $this->_repository->getAccountListBudgetOverview($date); + $budgets = $this->_repository->getBudgetsForMonth($date); - return View::make('reports.budget', compact('subTitle','subTitleIcon','date', 'accounts', 'budgets', 'dayEarly')); + return View::make('reports.budget', compact('subTitle', 'subTitleIcon', 'date', 'accounts', 'budgets', 'dayEarly')); } diff --git a/app/controllers/TransactionController.php b/app/controllers/TransactionController.php index c56d7ae08a..b987272c45 100644 --- a/app/controllers/TransactionController.php +++ b/app/controllers/TransactionController.php @@ -230,6 +230,8 @@ class TransactionController extends BaseController } /** + * @SuppressWarnings("CyclomaticComplexity") // It's exactly 5. So I don't mind. + * * @param $what * * @return $this|\Illuminate\Http\RedirectResponse @@ -245,8 +247,6 @@ class TransactionController extends BaseController $data['completed'] = 0; $data['what'] = $what; $data['currency'] = 'EUR'; - - // always validate: $messages = $this->_repository->validate($data); Session::flash('warnings', $messages['warnings']); @@ -257,17 +257,13 @@ class TransactionController extends BaseController return Redirect::route('transactions.create', $data['what'])->withInput(); } - // return to create screen: if ($data['post_submit_action'] == 'validate_only') { return Redirect::route('transactions.create', $data['what'])->withInput(); } - // store $journal = $this->_repository->store($data); Event::fire('transactionJournal.store', [$journal, Input::get('piggy_bank_id')]); // new and used. - /* - * Also trigger on both transactions. - */ + /** @var Transaction $transaction */ foreach ($journal->transactions as $transaction) { Event::fire('transaction.store', [$transaction]); @@ -303,8 +299,9 @@ class TransactionController extends BaseController Session::flash('errors', $messages['errors']); if ($messages['errors']->count() > 0) { Session::flash('error', 'Could not update transaction: ' . $messages['errors']->first()); + return Redirect::route('transactions.edit', $journal->id)->withInput(); } - if ($data['post_submit_action'] == 'validate_only' || $messages['errors']->count() > 0) { + if ($data['post_submit_action'] == 'validate_only') { return Redirect::route('transactions.edit', $journal->id)->withInput(); } $this->_repository->update($journal, $data); diff --git a/app/database/seeds/TestDataSeeder.php b/app/database/seeds/TestDataSeeder.php index 13028277d4..fa6cb958d0 100644 --- a/app/database/seeds/TestDataSeeder.php +++ b/app/database/seeds/TestDataSeeder.php @@ -184,6 +184,9 @@ class TestDataSeeder extends Seeder } /** + * @SuppressWarnings(PHPMD.ShortVariable) + * @SuppressWarnings(PHPMD.ExcessiveParameterList) + * * @param Account $from * @param Account $to * @param $amount diff --git a/app/lib/FireflyIII/Database/Account/Account.php b/app/lib/FireflyIII/Database/Account/Account.php index cb921b0baa..33a8c85ace 100644 --- a/app/lib/FireflyIII/Database/Account/Account.php +++ b/app/lib/FireflyIII/Database/Account/Account.php @@ -150,6 +150,7 @@ class Account implements CUDInterface, CommonDatabaseCallsInterface, AccountInte } /** + * @SuppressWarnings(PHPMD.ExcessiveMethodLength) // cannot make it shorter because of query. * @param Eloquent $model * * @return bool @@ -159,25 +160,25 @@ class Account implements CUDInterface, CommonDatabaseCallsInterface, AccountInte $journals = \TransactionJournal::whereIn( 'id', function (QueryBuilder $query) use ($model) { $query->select('transaction_journal_id') - ->from('transactions')->whereIn( - 'account_id', function (QueryBuilder $query) use ($model) { - $query - ->select('id') - ->from('accounts') - ->where( - function (QueryBuilder $q) use ($model) { - $q->where('id', $model->id); - $q->orWhere( - function (QueryBuilder $q) use ($model) { - $q->where('accounts.name', 'LIKE', '%' . $model->name . '%'); - $q->where('accounts.account_type_id', 3); - $q->where('accounts.active', 0); - } - ); - } - )->where('accounts.user_id', $this->getUser()->id); - } - )->get(); + ->from('transactions') + ->whereIn( + 'account_id', function (QueryBuilder $query) use ($model) { + $query + ->select('id')->from('accounts') + ->where( + function (QueryBuilder $q) use ($model) { + $q->where('id', $model->id); + $q->orWhere( + function (QueryBuilder $q) use ($model) { + $q->where('accounts.name', 'LIKE', '%' . $model->name . '%'); + $q->where('accounts.account_type_id', 3); + $q->where('accounts.active', 0); + } + ); + } + )->where('accounts.user_id', $this->getUser()->id); + } + )->get(); } )->get(); $transactions = []; @@ -218,9 +219,6 @@ class Account implements CUDInterface, CommonDatabaseCallsInterface, AccountInte public function store(array $data) { - /* - * Find account type. - */ /** @var \FireflyIII\Database\AccountType\AccountType $acctType */ $acctType = \App::make('FireflyIII\Database\AccountType\AccountType'); @@ -230,7 +228,6 @@ class Account implements CUDInterface, CommonDatabaseCallsInterface, AccountInte $data['account_type_id'] = $accountType->id; $data['active'] = isset($data['active']) && $data['active'] === '1' ? 1 : 0; - $data = array_except($data, ['_token', 'what']); $account = new \Account($data); if (!$account->isValid()) { diff --git a/app/lib/FireflyIII/Database/TransactionJournal/TransactionJournal.php b/app/lib/FireflyIII/Database/TransactionJournal/TransactionJournal.php index ba0b70bf27..ce0cfbafcc 100644 --- a/app/lib/FireflyIII/Database/TransactionJournal/TransactionJournal.php +++ b/app/lib/FireflyIII/Database/TransactionJournal/TransactionJournal.php @@ -363,11 +363,13 @@ class TransactionJournal implements TransactionJournalInterface, CUDInterface, C $errors->add('account_id', 'Invalid account.'); } break; + // often seen in deposits case (isset($model['account_id']) && isset($model['revenue_account'])): if (intval($model['account_id']) < 1) { $errors->add('account_id', 'Invalid account.'); } break; + // often seen in transfers case (isset($model['account_from_id']) && isset($model['account_to_id'])): if (intval($model['account_from_id']) < 1 || intval($model['account_from_id']) < 1) { $errors->add('account_from_id', 'Invalid account selected.'); diff --git a/app/lib/FireflyIII/Event/Piggybank.php b/app/lib/FireflyIII/Event/Piggybank.php index 1e0b62c053..5188658a2a 100644 --- a/app/lib/FireflyIII/Event/Piggybank.php +++ b/app/lib/FireflyIII/Event/Piggybank.php @@ -180,6 +180,7 @@ class PiggyBank } /** + * * Validates the presence of repetitions for all repeated expenses! */ public function validateRepeatedExpenses() @@ -189,24 +190,20 @@ class PiggyBank } /** @var \FireflyIII\Database\PiggyBank\RepeatedExpense $repository */ $repository = \App::make('FireflyIII\Database\PiggyBank\RepeatedExpense'); - $list = $repository->get(); $today = Carbon::now(); - /** @var \PiggyBank $entry */ foreach ($list as $entry) { - $start = $entry->startdate; - $target = $entry->targetdate; - $count = $entry->piggyBankrepetitions()->starts($start)->targets($target)->count(); + $count = $entry->piggyBankrepetitions()->starts($entry->startdate)->targets($entry->targetdate)->count(); if ($count == 0) { $repetition = new \PiggyBankRepetition; $repetition->piggyBank()->associate($entry); - $repetition->startdate = $start; - $repetition->targetdate = $target; + $repetition->startdate = $entry->startdate; + $repetition->targetdate = $entry->targetdate; $repetition->currentamount = 0; $repetition->save(); } - $currentTarget = clone $target; + $currentTarget = clone $entry->startdate; $currentStart = null; while ($currentTarget < $today) { $currentStart = \DateKit::subtractPeriod($currentTarget, $entry->rep_length, 0); diff --git a/app/lib/FireflyIII/Report/Report.php b/app/lib/FireflyIII/Report/Report.php index b926e6662c..70b651eb74 100644 --- a/app/lib/FireflyIII/Report/Report.php +++ b/app/lib/FireflyIII/Report/Report.php @@ -232,6 +232,8 @@ class Report implements ReportInterface * * @SuppressWarnings(PHPMD.UnusedFormalParameter) * + * TODO: This method runs two queries which are only marginally different. Try and combine these. + * * @param Carbon $date * @param bool $shared * @@ -245,7 +247,6 @@ class Report implements ReportInterface $end->endOfMonth(); $userId = $this->_accounts->getUser()->id; - $list = \TransactionJournal::leftJoin('transactions', 'transaction_journals.id', '=', 'transactions.transaction_journal_id') ->leftJoin('accounts', 'transactions.account_id', '=', 'accounts.id') ->leftJoin( diff --git a/app/lib/FireflyIII/Report/ReportQuery.php b/app/lib/FireflyIII/Report/ReportQuery.php index 7ac2cd18bd..4f8fd1a663 100644 --- a/app/lib/FireflyIII/Report/ReportQuery.php +++ b/app/lib/FireflyIII/Report/ReportQuery.php @@ -367,8 +367,6 @@ class ReportQuery implements ReportQueryInterface ); } ) - // ->where('transaction_types.type', 'Deposit') - // ->where('acm_to.data', '!=', '"sharedExpense"') ->before($end)->after($start) ->where('transaction_journals.user_id', \Auth::user()->id) ->groupBy('t_from.account_id')->orderBy('amount') diff --git a/app/lib/FireflyIII/Shared/Toolkit/Date.php b/app/lib/FireflyIII/Shared/Toolkit/Date.php index 443e343654..65d1d49ab9 100644 --- a/app/lib/FireflyIII/Shared/Toolkit/Date.php +++ b/app/lib/FireflyIII/Shared/Toolkit/Date.php @@ -183,39 +183,34 @@ class Date public function startOfPeriod(Carbon $theDate, $repeatFreq) { $date = clone $theDate; - switch ($repeatFreq) { - default: - throw new FireflyException('Cannot do startOfPeriod for $repeat_freq ' . $repeatFreq); - break; - case 'daily': - $date->startOfDay(); - break; - case 'week': - case 'weekly': - $date->startOfWeek(); - break; - case 'month': - case 'monthly': - $date->startOfMonth(); - break; - case 'quarter': - case 'quarterly': - $date->firstOfQuarter(); - break; - case 'half-year': - $month = intval($date->format('m')); - $date->startOfYear(); - if ($month >= 7) { - $date->addMonths(6); - } - break; - case 'year': - case 'yearly': - $date->startOfYear(); - break; - } - return $date; + $functionMap = [ + 'daily' => 'startOfDay', + 'week' => 'startOfWeek', + 'weekly' => 'startOfWeek', + 'month' => 'startOfMonth', + 'monthly' => 'startOfMonth', + 'quarter' => 'firstOfQuarter', + 'quartly' => 'firstOfQuarter', + 'year' => 'startOfYear', + 'yearly' => 'startOfYear', + ]; + if (isset($functionMap[$repeatFreq])) { + $function = $functionMap[$repeatFreq]; + $date->$function(); + + return $date; + } + if ($repeatFreq == 'half-year') { + $month = intval($date->format('m')); + $date->startOfYear(); + if ($month >= 7) { + $date->addMonths(6); + } + + return $date; + } + throw new FireflyException('Cannot do startOfPeriod for $repeat_freq ' . $repeatFreq); } /** @@ -229,38 +224,35 @@ class Date public function subtractPeriod(Carbon $theDate, $repeatFreq, $subtract = 1) { $date = clone $theDate; - switch ($repeatFreq) { - default: - throw new FireflyException('Cannot do subtractPeriod for $repeat_freq ' . $repeatFreq); - break; - case 'day': - case 'daily': - $date->subDays($subtract); - break; - case 'week': - case 'weekly': - $date->subWeeks($subtract); - break; - case 'month': - case 'monthly': - $date->subMonths($subtract); - break; - case 'quarter': - case 'quarterly': - $months = $subtract * 3; - $date->subMonths($months); - break; - case 'half-year': - $months = $subtract * 6; - $date->subMonths($months); - break; - case 'year': - case 'yearly': - $date->subYears($subtract); - break; + + $functionMap = [ + 'daily' => 'subDays', + 'week' => 'subWeeks', + 'weekly' => 'subWeeks', + 'month' => 'subMonths', + 'monthly' => 'subMonths', + 'year' => 'subYears', + 'yearly' => 'subYears', + ]; + $modifierMap = [ + 'quarter' => 3, + 'quarterly' => 3, + 'half-year' => 6, + ]; + if (isset($functionMap[$repeatFreq])) { + $function = $functionMap[$repeatFreq]; + $date->$function($subtract); + + return $date; + } + if (isset($modifierMap[$repeatFreq])) { + $subtract = $subtract * $modifierMap[$repeatFreq]; + $date->subMonths($subtract); + + return $date; } - return $date; + throw new FireflyException('Cannot do subtractPeriod for $repeat_freq ' . $repeatFreq); } } diff --git a/app/lib/FireflyIII/Shared/Toolkit/Filter.php b/app/lib/FireflyIII/Shared/Toolkit/Filter.php index ae04ae6062..73ca317790 100644 --- a/app/lib/FireflyIII/Shared/Toolkit/Filter.php +++ b/app/lib/FireflyIII/Shared/Toolkit/Filter.php @@ -131,6 +131,8 @@ class Filter } /** + * @SuppressWarnings("CyclomaticComplexity") // It's exactly 5. So I don't mind. + * * @param $range * @param Carbon $date *