Various code clean up.

This commit is contained in:
James Cole 2017-07-23 08:16:11 +02:00
parent 2c00a8353d
commit 8bb7d5de3f
No known key found for this signature in database
GPG Key ID: C16961E655E74B5E
26 changed files with 138 additions and 149 deletions

View File

@ -55,6 +55,7 @@ class CreateImport extends Command
/**
* @SuppressWarnings(PHPMD.ExcessiveMethodLength) // cannot be helped
* @SuppressWarnings(PHPMD.CyclomaticComplexity) // it's five exactly.
*/
public function handle()
{

View File

@ -36,6 +36,7 @@ use Steam;
/**
* Class UpgradeDatabase
* @SuppressWarnings(PHPMD.CouplingBetweenObjects) // it just touches a lot of things.
*
* @package FireflyIII\Console\Commands
*/
@ -80,6 +81,8 @@ class UpgradeDatabase extends Command
/**
* Moves the currency id info to the transaction instead of the journal.
*
* @SuppressWarnings(PHPMD.CyclomaticComplexity) // cannot be helped.
*/
private function currencyInfoToTransactions()
{
@ -96,7 +99,6 @@ class UpgradeDatabase extends Command
}
}
// read and use the foreign amounts when present.
if ($journal->hasMeta('foreign_amount')) {
$amount = Steam::positive($journal->getMeta('foreign_amount'));
@ -123,6 +125,8 @@ class UpgradeDatabase extends Command
/**
* Migrate budget repetitions to new format.
*
* @SuppressWarnings(PHPMD.CyclomaticComplexity) // it's 5.
*/
private function migrateRepetitions()
{
@ -150,6 +154,8 @@ class UpgradeDatabase extends Command
/**
* Make sure there are only transfers linked to piggy bank events.
*
* @SuppressWarnings(PHPMD.CyclomaticComplexity) // cannot be helped.
*/
private function repairPiggyBanks()
{
@ -157,6 +163,7 @@ class UpgradeDatabase extends Command
if (!Schema::hasTable('piggy_bank_events')) {
return;
}
$set = PiggyBankEvent::with(['PiggyBank', 'TransactionJournal', 'TransactionJournal.TransactionType'])->get();
/** @var PiggyBankEvent $event */
foreach ($set as $event) {
@ -208,7 +215,7 @@ class UpgradeDatabase extends Command
}
/**
*
* Make sure all accounts have proper currency info.
*/
private function updateAccountCurrencies()
{
@ -218,42 +225,38 @@ class UpgradeDatabase extends Command
/** @var Account $account */
foreach ($accounts as $account) {
// get users preference, fall back to system pref.
$defaultCurrencyCode = Preferences::getForUser($account->user, 'currencyPreference', config('firefly.default_currency', 'EUR'))->data;
$defaultCurrency = TransactionCurrency::where('code', $defaultCurrencyCode)->first();
$accountCurrency = intval($account->getMeta('currency_id'));
$openingBalance = $account->getOpeningBalance();
$openingBalanceCurrency = intval($openingBalance->transaction_currency_id);
$defaultCurrencyCode = Preferences::getForUser($account->user, 'currencyPreference', config('firefly.default_currency', 'EUR'))->data;
$defaultCurrency = TransactionCurrency::where('code', $defaultCurrencyCode)->first();
$accountCurrency = intval($account->getMeta('currency_id'));
$openingBalance = $account->getOpeningBalance();
$obCurrency = intval($openingBalance->transaction_currency_id);
// both 0? set to default currency:
if ($accountCurrency === 0 && $openingBalanceCurrency === 0) {
if ($accountCurrency === 0 && $obCurrency === 0) {
AccountMeta::create(['account_id' => $account->id, 'name' => 'currency_id', 'data' => $defaultCurrency->id]);
$this->line(sprintf('Account #%d ("%s") now has a currency setting (%s).', $account->id, $account->name, $defaultCurrencyCode));
continue;
}
// opening balance 0, account not zero? just continue:
if ($accountCurrency > 0 && $openingBalanceCurrency === 0) {
continue;
}
// account is set to 0, opening balance is not?
if ($accountCurrency === 0 && $openingBalanceCurrency > 0) {
AccountMeta::create(['account_id' => $account->id, 'name' => 'currency_id', 'data' => $openingBalanceCurrency]);
if ($accountCurrency === 0 && $obCurrency > 0) {
AccountMeta::create(['account_id' => $account->id, 'name' => 'currency_id', 'data' => $obCurrency]);
$this->line(sprintf('Account #%d ("%s") now has a currency setting (%s).', $account->id, $account->name, $defaultCurrencyCode));
continue;
}
// both are equal, just continue:
if ($accountCurrency === $openingBalanceCurrency) {
continue;
}
// do not match:
if ($accountCurrency !== $openingBalanceCurrency) {
if ($accountCurrency !== $obCurrency) {
// update opening balance:
$openingBalance->transaction_currency_id = $accountCurrency;
$openingBalance->save();
$this->line(sprintf('Account #%d ("%s") now has a correct currency for opening balance.', $account->id, $account->name));
continue;
}
// opening balance 0, account not zero? just continue:
// both are equal, just continue:
}
}

View File

@ -89,8 +89,6 @@ class JournalCollector implements JournalCollectorInterface
'account_types.type as account_type',
];
/** @var bool */
private $filterInternalTransfers;
/** @var bool */
private $filterTransfers = false;
/** @var array */
private $filters = [InternalTransferFilter::class];

View File

@ -194,7 +194,7 @@ class AccountController extends Controller
return view(
'accounts.edit', compact(
'allCurrencies', 'currencySelectList', 'account', 'currency', 'subTitle', 'subTitleIcon', 'openingBalance', 'what', 'roles'
'allCurrencies', 'currencySelectList', 'account', 'currency', 'subTitle', 'subTitleIcon', 'what', 'roles'
)
);
}

View File

@ -92,7 +92,7 @@ class ImportController extends Controller
Log::debug('Now in download()', ['job' => $job->key]);
$config = $job->configuration;
// TODO this is CSV import specific:
// This is CSV import specific:
$config['column-roles-complete'] = false;
$config['column-mapping-complete'] = false;
$config['initial-config-complete'] = false;

View File

@ -15,7 +15,6 @@ namespace FireflyIII\Http\Controllers;
use Amount;
use Carbon\Carbon;
use FireflyIII\Exceptions\FireflyException;
use FireflyIII\Helpers\Collector\JournalCollectorInterface;
use FireflyIII\Models\AccountType;
use FireflyIII\Models\TransactionType;
@ -27,7 +26,6 @@ use FireflyIII\Repositories\Journal\JournalRepositoryInterface;
use FireflyIII\Repositories\Tag\TagRepositoryInterface;
use FireflyIII\Support\CacheProperties;
use Illuminate\Http\Request;
use Preferences;
use Response;
/**

View File

@ -194,6 +194,41 @@ class RuleController extends Controller
return view('rules.rule.edit', compact('rule', 'subTitle', 'primaryTrigger', 'oldTriggers', 'oldActions', 'triggerCount', 'actionCount'));
}
/**
* Execute the given rule on a set of existing transactions
*
* @param SelectTransactionsRequest $request
* @param AccountRepositoryInterface $repository
* @param Rule $rule
*
* @return \Illuminate\Http\RedirectResponse
* @internal param RuleGroup $ruleGroup
*/
public function execute(SelectTransactionsRequest $request, AccountRepositoryInterface $repository, Rule $rule)
{
// Get parameters specified by the user
$accounts = $repository->getAccountsById($request->get('accounts'));
$startDate = new Carbon($request->get('start_date'));
$endDate = new Carbon($request->get('end_date'));
// Create a job to do the work asynchronously
$job = new ExecuteRuleOnExistingTransactions($rule);
// Apply parameters to the job
$job->setUser(auth()->user());
$job->setAccounts($accounts);
$job->setStartDate($startDate);
$job->setEndDate($endDate);
// Dispatch a new job to execute it in a queue
$this->dispatch($job);
// Tell the user that the job is queued
Session::flash('success', strval(trans('firefly.applied_rule_selection', ['title' => $rule->title])));
return redirect()->route('rules.index');
}
/**
* @param RuleGroupRepositoryInterface $repository
*
@ -243,45 +278,12 @@ class RuleController extends Controller
return Response::json('true');
}
/**
* Execute the given rule on a set of existing transactions
*
* @param SelectTransactionsRequest $request
* @param AccountRepositoryInterface $repository
* @param RuleGroup $ruleGroup
*
* @return \Illuminate\Http\RedirectResponse
*/
public function execute(SelectTransactionsRequest $request, AccountRepositoryInterface $repository, Rule $rule)
{
// Get parameters specified by the user
$accounts = $repository->getAccountsById($request->get('accounts'));
$startDate = new Carbon($request->get('start_date'));
$endDate = new Carbon($request->get('end_date'));
// Create a job to do the work asynchronously
$job = new ExecuteRuleOnExistingTransactions($rule);
// Apply parameters to the job
$job->setUser(auth()->user());
$job->setAccounts($accounts);
$job->setStartDate($startDate);
$job->setEndDate($endDate);
// Dispatch a new job to execute it in a queue
$this->dispatch($job);
// Tell the user that the job is queued
Session::flash('success', strval(trans('firefly.applied_rule_selection', ['title' => $rule->title])));
return redirect()->route('rules.index');
}
/**
* @param AccountRepositoryInterface $repository
* @param RuleGroup $ruleGroup
* @param Rule $rule
*
* @return View
* @return \Illuminate\Contracts\View\Factory|\Illuminate\View\View
*/
public function selectTransactions(AccountRepositoryInterface $repository, Rule $rule)
{
@ -323,52 +325,6 @@ class RuleController extends Controller
return redirect($this->getPreviousUri('rules.create.uri'));
}
/**
* This method allows the user to test a certain set of rule triggers. The rule triggers are grabbed from
* the rule itself.
*
* This method will parse and validate those rules and create a "TransactionMatcher" which will attempt
* to find transaction journals matching the users input. A maximum range of transactions to try (range) and
* a maximum number of transactions to return (limit) are set as well.
*
*
* @param Rule $rule
*
* @return \Illuminate\Http\JsonResponse
*/
public function testTriggersByRule(Rule $rule) {
$triggers = $rule->ruleTriggers;
if (count($triggers) === 0) {
return Response::json(['html' => '', 'warning' => trans('firefly.warning_no_valid_triggers')]);
}
$limit = config('firefly.test-triggers.limit');
$range = config('firefly.test-triggers.range');
/** @var TransactionMatcher $matcher */
$matcher = app(TransactionMatcher::class);
$matcher->setLimit($limit);
$matcher->setRange($range);
$matcher->setRule($rule);
$matchingTransactions = $matcher->findTransactionsByRule();
// Warn the user if only a subset of transactions is returned
$warning = '';
if (count($matchingTransactions) === $limit) {
$warning = trans('firefly.warning_transaction_subset', ['max_num_transactions' => $limit]);
}
if (count($matchingTransactions) === 0) {
$warning = trans('firefly.warning_no_matching_transactions', ['num_transactions' => $range]);
}
// Return json response
$view = view('list.journals-tiny', ['transactions' => $matchingTransactions])->render();
return Response::json(['html' => $view, 'warning' => $warning]);
}
/**
* This method allows the user to test a certain set of rule triggers. The rule triggers are passed along
* using the URL parameters (GET), and are usually put there using a Javascript thing.
@ -415,6 +371,53 @@ class RuleController extends Controller
return Response::json(['html' => $view, 'warning' => $warning]);
}
/**
* This method allows the user to test a certain set of rule triggers. The rule triggers are grabbed from
* the rule itself.
*
* This method will parse and validate those rules and create a "TransactionMatcher" which will attempt
* to find transaction journals matching the users input. A maximum range of transactions to try (range) and
* a maximum number of transactions to return (limit) are set as well.
*
*
* @param Rule $rule
*
* @return \Illuminate\Http\JsonResponse
*/
public function testTriggersByRule(Rule $rule)
{
$triggers = $rule->ruleTriggers;
if (count($triggers) === 0) {
return Response::json(['html' => '', 'warning' => trans('firefly.warning_no_valid_triggers')]);
}
$limit = config('firefly.test-triggers.limit');
$range = config('firefly.test-triggers.range');
/** @var TransactionMatcher $matcher */
$matcher = app(TransactionMatcher::class);
$matcher->setLimit($limit);
$matcher->setRange($range);
$matcher->setRule($rule);
$matchingTransactions = $matcher->findTransactionsByRule();
// Warn the user if only a subset of transactions is returned
$warning = '';
if (count($matchingTransactions) === $limit) {
$warning = trans('firefly.warning_transaction_subset', ['max_num_transactions' => $limit]);
}
if (count($matchingTransactions) === 0) {
$warning = trans('firefly.warning_no_matching_transactions', ['num_transactions' => $range]);
}
// Return json response
$view = view('list.journals-tiny', ['transactions' => $matchingTransactions])->render();
return Response::json(['html' => $view, 'warning' => $warning]);
}
/**
* @param RuleRepositoryInterface $repository
* @param Rule $rule

View File

@ -246,7 +246,7 @@ class RuleGroupController extends Controller
* @param RuleGroupRepositoryInterface $repository
* @param RuleGroup $ruleGroup
*
* @return \Illuminate\Http\RedirectResponse|\Illuminate\Routing\Redirector
* @return $this|\Illuminate\Http\RedirectResponse|\Illuminate\Routing\Redirector
*/
public function update(RuleGroupFormRequest $request, RuleGroupRepositoryInterface $repository, RuleGroup $ruleGroup)
{

View File

@ -15,7 +15,6 @@ namespace FireflyIII\Http\Controllers;
use FireflyIII\Support\Search\SearchInterface;
use Illuminate\Http\Request;
use Illuminate\Support\Collection;
use Response;
use View;

View File

@ -135,8 +135,8 @@ class MassController extends Controller
* @var TransactionJournal $journal
*/
foreach ($journals as $index => $journal) {
$sources = $journal->sourceAccountList($journal);
$destinations = $journal->destinationAccountList($journal);
$sources = $journal->sourceAccountList();
$destinations = $journal->destinationAccountList();
if ($sources->count() > 1) {
$messages[] = trans('firefly.cannot_edit_multiple_source', ['description' => $journal->description, 'id' => $journal->id]);
continue;

View File

@ -170,7 +170,7 @@ class SingleController extends Controller
*
* @param TransactionJournal $journal
*
* @return View
* @return \Illuminate\Http\RedirectResponse|\Illuminate\Routing\Redirector|View
*/
public function delete(TransactionJournal $journal)
{

View File

@ -173,7 +173,7 @@ class TransactionController extends Controller
* @param TransactionJournal $journal
* @param JournalTaskerInterface $tasker
*
* @return View
* @return \Illuminate\Http\RedirectResponse|\Illuminate\Routing\Redirector|View
*/
public function show(TransactionJournal $journal, JournalTaskerInterface $tasker)
{

View File

@ -176,7 +176,7 @@ class ImportStorage
*
* @return bool
*/
private function filterTransferSet(Collection $set, ImportJournal $importJournal)
private function filterTransferSet(Collection $set, ImportJournal $importJournal): bool
{
$amount = Steam::positive($importJournal->getAmount());
$asset = $importJournal->asset->getAccount();
@ -217,6 +217,8 @@ class ImportStorage
/**
* @param ImportJournal $importJournal
*
* @param Account $account
*
* @return TransactionCurrency
*/
private function getCurrency(ImportJournal $importJournal, Account $account): TransactionCurrency
@ -474,8 +476,7 @@ class ImportStorage
if ($transactionType->type === TransactionType::TRANSFER) {
$amount = Steam::positive($importJournal->getAmount());
$date = $importJournal->getDate($this->dateFormat);
$set = TransactionJournal::
leftJoin('transaction_types', 'transaction_types.id', '=', 'transaction_journals.transaction_type_id')
$set = TransactionJournal::leftJoin('transaction_types', 'transaction_types.id', '=', 'transaction_journals.transaction_type_id')
->leftJoin(
'transactions AS source', function (JoinClause $join) {
$join->on('transaction_journals.id', '=', 'source.transaction_journal_id')->where('source.amount', '<', 0);
@ -498,7 +499,7 @@ class ImportStorage
['transaction_journals.id', 'transaction_journals.encrypted', 'transaction_journals.description',
'source_accounts.name as source_name', 'destination_accounts.name as destination_name']
);
$filtered = $this->filterTransferSet($set, $importJournal);
return $this->filterTransferSet($set, $importJournal);
}

View File

@ -496,7 +496,7 @@ class AccountRepository implements AccountRepositoryInterface
}
/**
* @param string $iban
* @param null|string $iban
*
* @return null|string
*/

View File

@ -32,7 +32,14 @@ use Log;
*/
trait CreateJournalsTrait
{
/**
* @param User $user
* @param TransactionType $type
* @param array $data
*
* @return array
*/
abstract public function storeAccounts(User $user, TransactionType $type, array $data): array;
/**
*

View File

@ -18,7 +18,6 @@ use Cache;
use Carbon\Carbon;
use Illuminate\Database\Eloquent\Collection as EloquentCollection;
use Illuminate\Support\Collection;
use Log;
use Preferences as Prefs;
/**

View File

@ -7,7 +7,7 @@
*
* See the LICENSE file for details.
*/
/** global: Chart, defaultChartOptions, accounting, defaultPieOptions, noDataForChart */
/** global: Chart, defaultChartOptions, accounting, defaultPieOptions, noDataForChart, noDataForChart */
var allCharts = {};
/*
@ -240,13 +240,14 @@ function pieChart(URI, container) {
* @param colorData
*/
function drawAChart(URI, container, chartType, options, colorData) {
if ($('#' + container).length === 0) {
var containerObj = $('#' + container);
if (containerObj.length === 0) {
return;
}
$.getJSON(URI).done(function (data) {
$('#' + container).removeClass('general-chart-error');
containerObj.removeClass('general-chart-error');
if (data.labels.length === 0) {
// remove the chart container + parent
var holder = $('#' + container).parent().parent();

View File

@ -16,7 +16,7 @@ $(function () {
configAccounting(currencySymbol);
// on submit of form, disable any button in form:
$('form.form-horizontal').on('submit',function(e) {
$('form.form-horizontal').on('submit',function() {
$('button[type="submit"]').prop('disabled',true);
});

View File

@ -24,8 +24,6 @@ function showHelp(e) {
specialPage = '';
}
$('#helpBody').html('<i class="fa fa-refresh fa-spin"></i>');
$('#helpTitle').html('Please hold...');
$('#helpModal').modal('show');
$('#helpTitle').html('Help for this page');
$.getJSON('help/' + encodeURI(route)).done(function (data) {

View File

@ -60,6 +60,8 @@ function failedJobImport(jqxhr, textStatus, error) {
function reportOnJobImport(data) {
switch (data.status) {
default:
break;
case "configured":
// job is ready. Do not check again, just show the start-box. Hide the rest.
$('.statusbox').hide();
@ -173,6 +175,7 @@ function updateBar(data) {
bar.removeClass('progress-bar-success').addClass('progress-bar-info');
bar.attr('aria-valuenow', 100);
bar.css('width', '100%');
return true;
}
/**
@ -214,6 +217,4 @@ function reportOnErrors(data) {
$('#import-status-error-list').append(item);
}
}
return;
}

View File

@ -6,7 +6,7 @@
* See the LICENSE file for details.
*/
/** global: routeForTour, routeStepsUri, routeForFinishedTour */
/** global: routeForTour, routeStepsUri, routeForFinishedTour, forceDemoOff */
$(function () {
"use strict";

View File

@ -12,7 +12,6 @@
Some vars as prep for the map:
*/
var map;
var markers = [];
var setTag = false;
var mapOptions = {

View File

@ -6,7 +6,7 @@
* See the LICENSE file for details.
*/
/** global: currencyInfo, accountInfo, what,Modernizr, title, breadcrumbs, middleCrumbName, button, piggiesLength, txt, middleCrumbUrl,exchangeRateInstructions, convertForeignToNative, convertSourceToDestination, selectsForeignCurrency, accountInfo */
/** global: currencyInfo, overruleCurrency,useAccountCurrency, accountInfo, what,Modernizr, title, breadcrumbs, middleCrumbName, button, piggiesLength, txt, middleCrumbUrl,exchangeRateInstructions, convertForeignToNative, convertSourceToDestination, selectsForeignCurrency, accountInfo */
$(document).ready(function () {
"use strict";

View File

@ -99,10 +99,8 @@ class BudgetControllerTest extends TestCase
*/
public function testExpenseAsset(string $range)
{
$repository = $this->mock(BudgetRepositoryInterface::class);
$generator = $this->mock(GeneratorInterface::class);
$collector = $this->mock(JournalCollectorInterface::class);
$catRepos = $this->mock(CategoryRepositoryInterface::class);
$accountRepos = $this->mock(AccountRepositoryInterface::class);
$transactions = factory(Transaction::class, 10)->make();
$accounts = factory(Account::class, 10)->make();
@ -132,11 +130,9 @@ class BudgetControllerTest extends TestCase
*/
public function testExpenseCategory(string $range)
{
$repository = $this->mock(BudgetRepositoryInterface::class);
$generator = $this->mock(GeneratorInterface::class);
$collector = $this->mock(JournalCollectorInterface::class);
$catRepos = $this->mock(CategoryRepositoryInterface::class);
$accountRepos = $this->mock(AccountRepositoryInterface::class);
$transactions = factory(Transaction::class, 10)->make();
$categories = factory(Category::class, 10)->make();
@ -167,16 +163,8 @@ class BudgetControllerTest extends TestCase
*/
public function testExpenseExpense(string $range)
{
$repository = $this->mock(BudgetRepositoryInterface::class);
$generator = $this->mock(GeneratorInterface::class);
$collector = $this->mock(JournalCollectorInterface::class);
$catRepos = $this->mock(CategoryRepositoryInterface::class);
$accountRepos = $this->mock(AccountRepositoryInterface::class);
$repository = $this->mock(BudgetRepositoryInterface::class);
$generator = $this->mock(GeneratorInterface::class);
$collector = $this->mock(JournalCollectorInterface::class);
$catRepos = $this->mock(CategoryRepositoryInterface::class);
$accountRepos = $this->mock(AccountRepositoryInterface::class);
$transactions = factory(Transaction::class, 10)->make();
$accounts = factory(Account::class, 10)->make();

View File

@ -12,7 +12,6 @@ declare(strict_types=1);
namespace Tests\Feature\Controllers;
use FireflyIII\Support\Search\SearchInterface;
use Illuminate\Support\Collection;
use Tests\TestCase;
/**

View File

@ -63,9 +63,6 @@ class MetaPieChartTest extends TestCase
// mock all repositories:
$accountRepos = $this->mock(AccountRepositoryInterface::class);
$budgetRepos = $this->mock(BudgetRepositoryInterface::class);
$categoryRepos = $this->mock(CategoryRepositoryInterface::class);
$tagRepos = $this->mock(TagRepositoryInterface::class);
$accountRepos->shouldReceive('setUser');
$accountRepos->shouldReceive('find')->withArgs([1])->andReturn($accounts[1]);
@ -126,9 +123,6 @@ class MetaPieChartTest extends TestCase
// mock all repositories:
$accountRepos = $this->mock(AccountRepositoryInterface::class);
$budgetRepos = $this->mock(BudgetRepositoryInterface::class);
$categoryRepos = $this->mock(CategoryRepositoryInterface::class);
$tagRepos = $this->mock(TagRepositoryInterface::class);
$accountRepos->shouldReceive('setUser');
$accountRepos->shouldReceive('find')->withArgs([1])->andReturn($accounts[1]);