Some light refactoring. No changes.

This commit is contained in:
James Cole
2018-01-25 18:41:27 +01:00
parent 53fc4f2740
commit 7c70732247
28 changed files with 76 additions and 133 deletions

View File

@@ -178,7 +178,7 @@ class CreateImport extends Command
$cwd = getcwd();
$validTypes = config('import.options.file.import_formats');
$type = strtolower($this->option('type'));
if (null === $user->id) {
if (null === $user) {
$this->error(sprintf('There is no user with ID %d.', $this->option('user')));
return false;

View File

@@ -36,7 +36,7 @@ trait VerifiesAccessToken
/**
* Abstract method to make sure trait knows about method "option".
*
* @param null $key
* @param string|null $key
*
* @return mixed
*/
@@ -55,7 +55,7 @@ trait VerifiesAccessToken
$repository = app(UserRepositoryInterface::class);
$user = $repository->find($userId);
if (null === $user->id) {
if (null === $user) {
Log::error(sprintf('verifyAccessToken(): no such user for input "%d"', $userId));
return false;

View File

@@ -233,7 +233,7 @@ class JournalCollector implements JournalCollectorInterface
$countQuery->getQuery()->groups = null;
$countQuery->getQuery()->orders = null;
$countQuery->groupBy('accounts.user_id');
$this->count = $countQuery->count();
$this->count = intval($countQuery->count());
return $this->count;
}
@@ -252,7 +252,7 @@ class JournalCollector implements JournalCollectorInterface
$cache->addProperty($key);
if ($cache->has()) {
Log::debug(sprintf('Return cache of query with ID "%s".', $key));
//return $cache->get(); // @codeCoverageIgnore
return $cache->get(); // @codeCoverageIgnore
}
/** @var Collection $set */

View File

@@ -190,7 +190,7 @@ class CategoryController extends Controller
$end = new Carbon;
}
// prep for "specific date" view.$dates = app('navigation')->blockPeriods($start, $end, $range);
// prep for "specific date" view.
if (strlen($moment) > 0 && 'all' !== $moment) {
$start = app('navigation')->startOfPeriod(new Carbon($moment), $range);
$end = app('navigation')->endOfPeriod($start, $range);

View File

@@ -105,7 +105,7 @@ class StatusController extends Controller
}
if ($tagId === 0) {
$result['finishedText'] = trans('import.status_finished_no_tag');
$result['finishedText'] = trans('import.status_finished_no_tag'); // @codeCoverageIgnore
}
}

View File

@@ -128,7 +128,7 @@ class PreferencesController extends Controller
* @return \Illuminate\Http\RedirectResponse|\Illuminate\Routing\Redirector
* @SuppressWarnings(PHPMD.UnusedFormalParameter) // it's unused but the class does some validation.
*/
public function postCode(TokenFormRequest $request)
public function postCode(/** @scrutinizer ignore-unused */ TokenFormRequest $request)
{
Preferences::set('twoFactorAuthEnabled', 1);
Preferences::set('twoFactorAuthSecret', Session::get('two-factor-secret'));

View File

@@ -24,10 +24,8 @@ namespace FireflyIII\Http\Middleware;
use Closure;
use Illuminate\Http\Request;
use Illuminate\Support\Facades\Auth;
use Log;
use Preferences;
use Session;
/**
* Class AuthenticateTwoFactor.
@@ -45,25 +43,8 @@ class AuthenticateTwoFactor
*/
public function handle(Request $request, Closure $next, $guard = null)
{
// do the usual auth, again:
if (Auth::guard($guard)->guest()) {
if ($request->ajax()) {
return response('Unauthorized.', 401);
}
return redirect()->guest('login');
}
if (1 === intval(auth()->user()->blocked)) {
Auth::guard($guard)->logout();
Session::flash('logoutMessage', trans('firefly.block_account_logout'));
return redirect()->guest('login');
}
$is2faEnabled = Preferences::get('twoFactorAuthEnabled', false)->data;
$has2faSecret = null !== Preferences::get('twoFactorAuthSecret');
// grab 2auth information from session.
$is2faAuthed = 'true' === $request->cookie('twoFactorAuthenticated');
if ($is2faEnabled && $has2faSecret && !$is2faAuthed) {

View File

@@ -60,8 +60,8 @@ class TrustProxies extends Middleware
public function __construct(Repository $config)
{
$trustedProxies = env('TRUSTED_PROXIES', null);
if (null !== $trustedProxies && strlen($trustedProxies) > 0) {
$this->proxies = $trustedProxies;
if (false !== $trustedProxies && null !== $trustedProxies && strlen($trustedProxies) > 0) {
$this->proxies = strval($trustedProxies);
}
parent::__construct($config);

View File

@@ -249,6 +249,9 @@ class FileConfigurator implements ConfiguratorInterface
}
/**
* Shorthand method to return the extended status.
*
* @codeCoverageIgnore
* @return array
*/
private function getExtendedStatus(): array
@@ -257,8 +260,9 @@ class FileConfigurator implements ConfiguratorInterface
}
/**
* Shorthand method.
* Shorthand method to set the extended status.
*
* @codeCoverageIgnore
* @param array $extended
*/
private function setExtendedStatus(array $extended): void

View File

@@ -77,7 +77,6 @@ class SpectreConfigurator implements ConfiguratorInterface
$this->repository->setConfiguration($this->job, $config);
return true;
break;
default:
throw new FireflyException(sprintf('Cannot store configuration when job is in state "%s"', $stage));
break;
@@ -95,7 +94,9 @@ class SpectreConfigurator implements ConfiguratorInterface
if (is_null($this->job)) {
throw new FireflyException('Cannot call configureJob() without a job.');
}
$stage = $this->getConfig()['stage'] ?? 'initial';
$config = $this->getConfig();
$stage = $config['stage'] ?? 'initial';
Log::debug(sprintf('in getNextData(), for stage "%s".', $stage));
switch ($stage) {
case 'has-token':
@@ -109,9 +110,7 @@ class SpectreConfigurator implements ConfiguratorInterface
$this->repository->setStatus($this->job, $status);
return $this->repository->getConfiguration($this->job);
break;
case 'have-accounts':
// use special class:
/** @var HaveAccounts $class */
$class = app(HaveAccounts::class);
$class->setJob($this->job);
@@ -120,7 +119,6 @@ class SpectreConfigurator implements ConfiguratorInterface
return $data;
default:
return [];
break;
}
}
@@ -141,13 +139,10 @@ class SpectreConfigurator implements ConfiguratorInterface
Log::info('User is being redirected to Spectre.');
return 'import.spectre.redirect';
break;
case 'have-accounts':
return 'import.spectre.accounts';
break;
default:
return '';
break;
}
}

View File

@@ -77,7 +77,7 @@ class Amount implements ConverterInterface
Log::debug(sprintf('Searched from the left for "." in amount "%s", assume this is the decimal sign.', $value));
$decimal = '.';
}
unset($options, $res);
unset($res);
}
// if decimal is dot, replace all comma's and spaces with nothing. then parse as float (round to 4 pos)

View File

@@ -122,9 +122,9 @@ class CsvProcessor implements FileProcessorInterface
}
/**
* @codeCoverageIgnore
* Shorthand method
* Shorthand method to set the extended status.
*
* @codeCoverageIgnore
* @param array $array
*/
public function setExtendedStatus(array $array)
@@ -149,7 +149,9 @@ class CsvProcessor implements FileProcessorInterface
}
/**
* Shorthand method.
* Shorthand method to add a step.
*
* @codeCoverageIgnore
*/
private function addStep()
{
@@ -187,9 +189,9 @@ class CsvProcessor implements FileProcessorInterface
}
/**
* @codeCoverageIgnore
* Shorthand method.
* Shorthand method to return configuration.
*
* @codeCoverageIgnore
* @return array
*/
private function getConfig(): array
@@ -197,18 +199,6 @@ class CsvProcessor implements FileProcessorInterface
return $this->repository->getConfiguration($this->job);
}
/**
* @codeCoverageIgnore
* Shorthand method.
*
* @return array
*/
private function getExtendedStatus(): array
{
return $this->repository->getExtendedStatus($this->job);
}
/**
* @return Iterator
*

View File

@@ -310,7 +310,6 @@ class ImportStorage
$amount = app('steam')->positive($parameters['amount']);
$names = [$parameters['asset'], $parameters['opposing']];
$transfer = [];
sort($names);

View File

@@ -30,6 +30,7 @@ use Symfony\Component\HttpKernel\Exception\NotFoundHttpException;
* Class ExportJob.
*
* @property User $user
* @property string $key
*/
class ExportJob extends Model
{

View File

@@ -66,6 +66,7 @@ use Watson\Validating\ValidatingTrait;
* @property string $transaction_currency_symbol
* @property int $transaction_currency_dp
* @property string $transaction_currency_code
* @property string $description
*/
class Transaction extends Model
{

View File

@@ -28,6 +28,9 @@ use Symfony\Component\HttpKernel\Exception\NotFoundHttpException;
/**
* Class TransactionCurrency.
*
* @property string $code
*
*/
class TransactionCurrency extends Model
{

View File

@@ -92,8 +92,6 @@ class TransactionJournal extends Model
if (auth()->check()) {
$journalId = intval($value);
$journal = auth()->user()->transactionJournals()->where('transaction_journals.id', $journalId)
->with('transactionType')
->leftJoin('transaction_types', 'transaction_types.id', '=', 'transaction_journals.transaction_type_id')
->first(['transaction_journals.*']);
if (!is_null($journal)) {
return $journal;

View File

@@ -202,9 +202,6 @@ class LinkTypeRepository implements LinkTypeRepositoryInterface
$dbNote->save();
}
//$link->comment = $link['notes'] ?? null;
return $link;
}

View File

@@ -51,7 +51,6 @@ class HaveAccounts implements ConfigurationInterface
$accountRepository = app(AccountRepositoryInterface::class);
/** @var CurrencyRepositoryInterface $currencyRepository */
$currencyRepository = app(CurrencyRepositoryInterface::class);
$data = [];
$config = $this->job->configuration;
$collection = $accountRepository->getAccountsByType([AccountType::DEFAULT, AccountType::ASSET]);
$defaultCurrency = app('amount')->getDefaultCurrency();

View File

@@ -42,6 +42,27 @@ use Illuminate\Support\Collection;
*/
trait TransactionJournalTrait
{
/**
* @param Builder $query
* @param string $table
*
* @return bool
*/
public static function isJoined(Builder $query, string $table): bool
{
$joins = $query->getQuery()->joins;
if (null === $joins) {
return false;
}
foreach ($joins as $join) {
if ($join->table === $table) {
return true;
}
}
return false;
}
/**
* @return string
*/
@@ -211,27 +232,6 @@ trait TransactionJournalTrait
*/
abstract public function isDeposit(): bool;
/**
* @param Builder $query
* @param string $table
*
* @return bool
*/
public function isJoined(Builder $query, string $table): bool
{
$joins = $query->getQuery()->joins;
if (null === $joins) {
return false;
}
foreach ($joins as $join) {
if ($join->table === $table) {
return true;
}
}
return false;
}
/**
* @return bool
*/

View File

@@ -256,7 +256,7 @@ class Navigation
{
// define period to increment
$increment = 'addDay';
$format = self::preferredCarbonFormat($start, $end);
$format = $this->preferredCarbonFormat($start, $end);
$displayFormat = strval(trans('config.month_and_day'));
// increment by month (for year)
if ($start->diffInMonths($end) > 1) {

View File

@@ -65,7 +65,7 @@ class FireflyValidator extends Validator
*
* @return bool
*/
public function validate2faCode($attribute, $value): bool
public function validate2faCode(/** @scrutinizer ignore-unused */ $attribute, $value): bool
{
if (!is_string($value) || null === $value || 6 != strlen($value)) {
return false;
@@ -85,7 +85,7 @@ class FireflyValidator extends Validator
*
* @return bool
*/
public function validateBelongsToUser($attribute, $value, $parameters): bool
public function validateBelongsToUser(/** @scrutinizer ignore-unused */ $attribute, $value, $parameters): bool
{
$field = $parameters[1] ?? 'id';
@@ -108,7 +108,7 @@ class FireflyValidator extends Validator
*
* @return bool
*/
public function validateBic($attribute, $value): bool
public function validateBic(/** @scrutinizer ignore-unused */ $attribute, $value): bool
{
$regex = '/^[a-z]{6}[0-9a-z]{2}([0-9a-z]{3})?\z/i';
$result = preg_match($regex, $value);
@@ -130,7 +130,7 @@ class FireflyValidator extends Validator
*
* @return bool
*/
public function validateIban($attribute, $value): bool
public function validateIban(/** @scrutinizer ignore-unused */ $attribute, $value): bool
{
if (!is_string($value) || null === $value || strlen($value) < 6) {
return false;
@@ -210,7 +210,7 @@ class FireflyValidator extends Validator
*
* @return bool
*/
public function validateMore($attribute, $value, $parameters): bool
public function validateMore(/** @scrutinizer ignore-unused */ $attribute, $value, $parameters): bool
{
$compare = $parameters[0] ?? '0';
@@ -226,7 +226,7 @@ class FireflyValidator extends Validator
*
* @return bool
*/
public function validateMustExist($attribute, $value, $parameters): bool
public function validateMustExist(/** @scrutinizer ignore-unused */ $attribute, $value, $parameters): bool
{
$field = $parameters[1] ?? 'id';
@@ -335,7 +335,7 @@ class FireflyValidator extends Validator
*
* @return bool
*/
public function validateSecurePassword($attribute, $value): bool
public function validateSecurePassword(/** @scrutinizer ignore-unused */ $attribute, $value): bool
{
$verify = false;
if (isset($this->data['verify_password'])) {
@@ -360,7 +360,7 @@ class FireflyValidator extends Validator
*
* @return bool
*/
public function validateUniqueAccountForUser($attribute, $value, $parameters): bool
public function validateUniqueAccountForUser(/** @scrutinizer ignore-unused */ $attribute, $value, $parameters): bool
{
// because a user does not have to be logged in (tests and what-not).
if (!auth()->check()) {
@@ -390,7 +390,7 @@ class FireflyValidator extends Validator
*
* @return bool
*/
public function validateUniqueAccountNumberForUser($attribute, $value): bool
public function validateUniqueAccountNumberForUser(/** @scrutinizer ignore-unused */ $attribute, $value): bool
{
$accountId = $this->data['id'] ?? 0;
@@ -428,7 +428,7 @@ class FireflyValidator extends Validator
*
* @return bool
*/
public function validateUniqueObjectForUser($attribute, $value, $parameters): bool
public function validateUniqueObjectForUser(/** @scrutinizer ignore-unused */ $attribute, $value, $parameters): bool
{
$value = $this->tryDecrypt($value);
// exclude?
@@ -460,7 +460,7 @@ class FireflyValidator extends Validator
*
* @return bool
*/
public function validateUniquePiggyBankForUser($attribute, $value, $parameters): bool
public function validateUniquePiggyBankForUser(/** @scrutinizer ignore-unused */ $attribute, $value, $parameters): bool
{
$exclude = $parameters[0] ?? null;
$query = DB::table('piggy_banks')->whereNull('piggy_banks.deleted_at')

View File

@@ -36,7 +36,6 @@ $(function () {
Respond to changes in balance statements.
*/
$('input[type="number"]').on('change', function () {
console.log('On change number input.');
if (reconcileStarted) {
calculateBalanceDifference();
difference = balanceDifference - selectedAmount;
@@ -50,7 +49,6 @@ $(function () {
Respond to changes in the date range.
*/
$('input[type="date"]').on('change', function () {
console.log('On change date input.');
if (reconcileStarted) {
// hide original instructions.
$('.select_transactions_instruction').hide();
@@ -70,19 +68,16 @@ $(function () {
});
function storeReconcile() {
console.log('In storeReconcile.');
// get modal HTML:
var ids = [];
$.each($('.reconcile_checkbox:checked'), function (i, v) {
ids.push($(v).data('id'));
});
console.log('Ids is ' + ids);
var cleared = [];
$.each($('input[class="cleared"]'), function (i, v) {
var obj = $(v);
cleared.push(obj.data('id'));
});
console.log('Cleared is ' + ids);
var variables = {
startBalance: parseFloat($('input[name="start_balance"]').val()),
@@ -105,7 +100,6 @@ function storeReconcile() {
* @param e
*/
function checkReconciledBox(e) {
console.log('In checkReconciledBox.');
var el = $(e.target);
var amount = parseFloat(el.val());
// if checked, add to selected amount
@@ -125,13 +119,9 @@ function checkReconciledBox(e) {
* and put it in balanceDifference.
*/
function calculateBalanceDifference() {
console.log('In calculateBalanceDifference.');
var startBalance = parseFloat($('input[name="start_balance"]').val());
var endBalance = parseFloat($('input[name="end_balance"]').val());
balanceDifference = startBalance - endBalance;
//if (balanceDifference < 0) {
// balanceDifference = balanceDifference * -1;
//}
}
/**
@@ -139,7 +129,6 @@ function calculateBalanceDifference() {
* This more or less resets the reconciliation.
*/
function getTransactionsForRange() {
console.log('In getTransactionsForRange.');
// clear out the box:
$('#transactions_holder').empty().append($('<p>').addClass('text-center').html('<i class="fa fa-fw fa-spin fa-spinner"></i>'));
var uri = transactionsUri.replace('%start%', $('input[name="start_date"]').val()).replace('%end%', $('input[name="end_date"]').val());
@@ -154,7 +143,6 @@ function getTransactionsForRange() {
*
*/
function includeClearedTransactions() {
console.log('In includeClearedTransactions.');
$.each($('input[class="cleared"]'), function (i, v) {
var obj = $(v);
if (obj.data('younger') === false) {
@@ -168,7 +156,6 @@ function includeClearedTransactions() {
* @param data
*/
function placeTransactions(data) {
console.log('In placeTransactions.');
$('#transactions_holder').empty().html(data.html);
selectedAmount = 0;
// update start + end balance when user has not touched them.
@@ -200,7 +187,6 @@ function placeTransactions(data) {
* @returns {boolean}
*/
function startReconcile() {
console.log('In startReconcile.');
reconcileStarted = true;
// hide the start button.
@@ -222,7 +208,6 @@ function startReconcile() {
}
function updateDifference() {
console.log('In updateDifference.');
var addClass = 'text-info';
if (difference > 0) {
addClass = 'text-success';

View File

@@ -44,7 +44,6 @@ $(function () {
function startExport() {
"use strict";
console.log('startExport');
hideForm();
showLoading();
hideError();

View File

@@ -35,7 +35,6 @@ var knownErrors = 0;
$(function () {
"use strict";
console.log('in start');
timeOutId = setTimeout(checkJobStatus, startInterval);
$('.start-job').click(function () {
@@ -44,7 +43,6 @@ $(function () {
startJob();
});
if (job.configuration['auto-start']) {
console.log('Called startJob()!');
startJob();
}
});
@@ -53,7 +51,6 @@ $(function () {
* Downloads some JSON and responds to its content to see what the status is of the current import.
*/
function checkJobStatus() {
console.log('in checkJobStatus');
$.getJSON(jobStatusUri).done(reportOnJobStatus).fail(reportFailedJob);
}
@@ -61,7 +58,6 @@ function checkJobStatus() {
* This method is called when the JSON query returns an error. If possible, this error is relayed to the user.
*/
function reportFailedJob(jqxhr, textStatus, error) {
console.log('in reportFailedJob');
// hide all possible boxes:
$('.statusbox').hide();
@@ -81,7 +77,6 @@ function reportFailedJob(jqxhr, textStatus, error) {
* @param data
*/
function reportOnJobStatus(data) {
console.log('in reportOnJobStatus: ' + data.status);
switch (data.status) {
case "configured":
@@ -91,18 +86,13 @@ function reportOnJobStatus(data) {
$('.status_configured').show();
}
if (job.configuration['auto-start']) {
console.log('Job is auto start. Check status again in 500ms.');
timeOutId = setTimeout(checkJobStatus, interval);
}
if (pressedStart) {
console.log('pressedStart = true, will check extra.');
// do a time out just in case. Could be that job is running or is even done already.
timeOutId = setTimeout(checkJobStatus, 2000);
pressedStart = false;
}
if (!pressedStart) {
console.log('pressedStart = false, will do nothing.');
}
break;
case "running":
// job is running! Show the running box:
@@ -148,7 +138,6 @@ function reportOnJobStatus(data) {
break;
case "configuring":
// redirect back to configure screen.
console.log('Will now redirect to ' + jobConfigureUri);
window.location = jobConfigureUri;
break;
default:
@@ -192,9 +181,7 @@ function jobIsStalled(data) {
* Only when job is in "configured" state.
*/
function startJob() {
console.log("In startJob()");
if (job.status === "configured") {
console.log("Job started!");
// disable the button, add loading thing.
$('.start-job').prop('disabled', true).text('...');
$.post(jobStartUri, {_token: token}).fail(reportOnSubmitError);
@@ -203,7 +190,6 @@ function startJob() {
timeOutId = setTimeout(checkJobStatus, startInterval);
return;
}
console.log("Job not auto started because state is " + job.status);
}
/**

View File

@@ -51,7 +51,7 @@
</div>
<div class="box-body">
{# category always #}
{{ ExpandedForm.text('category',data['category']) }}
{{ ExpandedForm.text('category',data.category) }}
{# tags #}
{{ ExpandedForm.text('tags') }}

View File

@@ -50,13 +50,16 @@ class CategoryControllerTest extends TestCase
*/
public function testAll(string $range)
{
$repository = $this->mock(CategoryRepositoryInterface::class);
$accountRepos = $this->mock(AccountRepositoryInterface::class);
$generator = $this->mock(GeneratorInterface::class);
$firstUse = new Carbon;
$firstUse->subDays(3);
$repository->shouldReceive('spentInPeriod')->andReturn('0');
$repository->shouldReceive('earnedInPeriod')->andReturn('0');
$repository->shouldReceive('firstUseDate')->andReturn(new Carbon('1900-01-01'))->once();
$repository->shouldReceive('firstUseDate')->andReturn($firstUse)->once();
$accountRepos->shouldReceive('getAccountsByType')->withArgs([[AccountType::DEFAULT, AccountType::ASSET]])->andReturn(new Collection)->once();
$generator->shouldReceive('multiSet')->once()->andReturn([]);

View File

@@ -24,6 +24,7 @@ declare(strict_types=1);
namespace Tests\Unit\Helpers;
use FireflyIII\Http\Middleware\Sandstorm;
use FireflyIII\Models\Role;
use FireflyIII\Repositories\User\UserRepositoryInterface;
use Route;
use Symfony\Component\HttpFoundation\Response;
@@ -74,7 +75,7 @@ class SandstormTest extends TestCase
putenv('SANDSTORM=1');
$repository = $this->mock(UserRepositoryInterface::class);
$repository->shouldReceive('count')->once()->andReturn(1);
$repository->shouldReceive('count')->twice()->andReturn(1);
$response = $this->get('/_test/sandstorm');
$this->assertEquals(Response::HTTP_OK, $response->getStatusCode());
@@ -123,9 +124,10 @@ class SandstormTest extends TestCase
putenv('SANDSTORM=1');
$repository = $this->mock(UserRepositoryInterface::class);
$repository->shouldReceive('count')->once()->andReturn(0);
$repository->shouldReceive('count')->twice()->andReturn(0);
$repository->shouldReceive('store')->once()->andReturn($this->user());
$repository->shouldReceive('attachRole')->once()->andReturn(true);
$repository->shouldReceive('attachRole')->twice()->andReturn(true);
$repository->shouldReceive('getRole')->once()->andReturn(new Role);
$response = $this->get('/_test/sandstorm', ['X-Sandstorm-User-Id' => 'abcd']);
$this->assertEquals(Response::HTTP_OK, $response->getStatusCode());
@@ -152,7 +154,7 @@ class SandstormTest extends TestCase
putenv('SANDSTORM=1');
$repository = $this->mock(UserRepositoryInterface::class);
$repository->shouldReceive('count')->once()->andReturn(1);
$repository->shouldReceive('count')->twice()->andReturn(1);
$repository->shouldReceive('first')->once()->andReturn($this->user());
$response = $this->get('/_test/sandstorm', ['X-Sandstorm-User-Id' => 'abcd']);