Improve test coverage and efficiency for accounts and budgets.

This commit is contained in:
James Cole
2019-06-23 11:13:36 +02:00
parent 8f25562923
commit 43d753e5bd
47 changed files with 919 additions and 985 deletions

View File

@@ -24,7 +24,7 @@ declare(strict_types=1);
namespace FireflyIII\Http\Controllers\Account;
use Carbon\Carbon;
use FireflyIII\Exceptions\FireflyException;
use Exception;
use FireflyIII\Helpers\Collector\GroupCollectorInterface;
use FireflyIII\Http\Controllers\Controller;
use FireflyIII\Models\Account;
@@ -36,7 +36,6 @@ use FireflyIII\Support\Http\Controllers\UserNavigation;
use Illuminate\Http\Request;
use Illuminate\Support\Collection;
use View;
use Exception;
/**
* Class ShowController
@@ -101,17 +100,18 @@ class ShowController extends Controller
[$start, $end] = [$end, $start]; // @codeCoverageIgnore
}
$objectType = config(sprintf('firefly.shortNamesByFullName.%s', $account->accountType->type));
$today = new Carbon;
$subTitleIcon = config(sprintf('firefly.subIconsByIdentifier.%s', $account->accountType->type));
$page = (int)$request->get('page');
$pageSize = (int)app('preferences')->get('listPageSize', 50)->data;
$currency = $this->repository->getAccountCurrency($account) ?? app('amount')->getDefaultCurrency();
$fStart = $start->formatLocalized($this->monthAndDayFormat);
$fEnd = $end->formatLocalized($this->monthAndDayFormat);
$subTitle = (string)trans('firefly.journals_in_period_for_account', ['name' => $account->name, 'start' => $fStart, 'end' => $fEnd]);
$chartUri = route('chart.account.period', [$account->id, $start->format('Y-m-d'), $end->format('Y-m-d')]);
$periods = $this->getAccountPeriodOverview($account, $end);
$objectType = config(sprintf('firefly.shortNamesByFullName.%s', $account->accountType->type));
$today = new Carbon;
$subTitleIcon = config(sprintf('firefly.subIconsByIdentifier.%s', $account->accountType->type));
$page = (int)$request->get('page');
$pageSize = (int)app('preferences')->get('listPageSize', 50)->data;
$currency = $this->repository->getAccountCurrency($account) ?? app('amount')->getDefaultCurrency();
$fStart = $start->formatLocalized($this->monthAndDayFormat);
$fEnd = $end->formatLocalized($this->monthAndDayFormat);
$subTitle = (string)trans('firefly.journals_in_period_for_account', ['name' => $account->name, 'start' => $fStart, 'end' => $fEnd]);
$chartUri = route('chart.account.period', [$account->id, $start->format('Y-m-d'), $end->format('Y-m-d')]);
$firstTransaction = $this->repository->oldestJournalDate($account) ?? $start;
$periods = $this->getAccountPeriodOverview($account, $firstTransaction, $end);
/** @var GroupCollectorInterface $collector */
$collector = app(GroupCollectorInterface::class);
@@ -138,10 +138,10 @@ class ShowController extends Controller
*
* @param Request $request
* @param Account $account
* @throws Exception
* @return \Illuminate\Http\RedirectResponse|\Illuminate\Routing\Redirector|View
*
*
* @throws Exception
*/
public function showAll(Request $request, Account $account)
{

View File

@@ -37,13 +37,14 @@ class ConfigurationController extends Controller
{
/**
* ConfigurationController constructor.
* @codeCoverageIgnore
*/
public function __construct()
{
parent::__construct();
$this->middleware(
function ($request, $next) {
static function ($request, $next) {
app('view')->share('title', (string)trans('firefly.administration'));
app('view')->share('mainTitleIcon', 'fa-hand-spock-o');

View File

@@ -37,6 +37,7 @@ class HomeController extends Controller
{
/**
* ConfigurationController constructor.
* @codeCoverageIgnore
*/
public function __construct()
{

View File

@@ -36,8 +36,13 @@ use View;
*/
class LinkController extends Controller
{
/** @var LinkTypeRepositoryInterface */
private $repository;
/**
* LinkController constructor.
* @codeCoverageIgnore
*/
public function __construct()
{
@@ -47,6 +52,7 @@ class LinkController extends Controller
function ($request, $next) {
app('view')->share('title', (string)trans('firefly.administration'));
app('view')->share('mainTitleIcon', 'fa-hand-spock-o');
$this->repository = app(LinkTypeRepositoryInterface::class);
return $next($request);
}
@@ -61,11 +67,11 @@ class LinkController extends Controller
*/
public function create()
{
Log::channel('audit')->info('User visits link index.');
$subTitle = (string)trans('firefly.create_new_link_type');
$subTitleIcon = 'fa-link';
Log::channel('audit')->info('User visits link index.');
// put previous url in session if not redirect from store (not "create another").
if (true !== session('link-types.create.fromStore')) {
$this->rememberPreviousUri('link-types.create.uri');
@@ -77,13 +83,12 @@ class LinkController extends Controller
/**
* Delete a link form.
*
* @param Request $request
* @param LinkTypeRepositoryInterface $repository
* @param LinkType $linkType
* @param Request $request
* @param LinkType $linkType
*
* @return \Illuminate\Http\RedirectResponse|\Illuminate\Routing\Redirector|View
*/
public function delete(Request $request, LinkTypeRepositoryInterface $repository, LinkType $linkType)
public function delete(Request $request, LinkType $linkType)
{
if (!$linkType->editable) {
$request->session()->flash('error', (string)trans('firefly.cannot_edit_link_type', ['name' => $linkType->name]));
@@ -92,18 +97,19 @@ class LinkController extends Controller
}
Log::channel('audit')->info(sprintf('User wants to delete link type #%d', $linkType->id));
$subTitle = (string)trans('firefly.delete_link_type', ['name' => $linkType->name]);
$otherTypes = $repository->get();
$count = $repository->countJournals($linkType);
$otherTypes = $this->repository->get();
$count = $this->repository->countJournals($linkType);
$moveTo = [];
$moveTo[0] = (string)trans('firefly.do_not_save_connection');
/** @var LinkType $otherType */
foreach ($otherTypes as $otherType) {
if ($otherType->id !== $linkType->id) {
$moveTo[$otherType->id] = sprintf('%s (%s / %s)', $otherType->name, $otherType->inward, $otherType->outward);
}
}
// put previous url in session
$this->rememberPreviousUri('link-types.delete.uri');
@@ -113,18 +119,17 @@ class LinkController extends Controller
/**
* Actually destroy the link.
*
* @param Request $request
* @param LinkTypeRepositoryInterface $repository
* @param LinkType $linkType
* @param Request $request
* @param LinkType $linkType
*
* @return \Illuminate\Http\RedirectResponse|\Illuminate\Routing\Redirector
*/
public function destroy(Request $request, LinkTypeRepositoryInterface $repository, LinkType $linkType)
public function destroy(Request $request, LinkType $linkType)
{
Log::channel('audit')->info(sprintf('User destroyed link type #%d', $linkType->id));
$name = $linkType->name;
$moveTo = $repository->findNull((int)$request->get('move_link_type_before_delete'));
$repository->destroy($linkType, $moveTo);
$moveTo = $this->repository->findNull((int)$request->get('move_link_type_before_delete'));
$this->repository->destroy($linkType, $moveTo);
$request->session()->flash('success', (string)trans('firefly.deleted_link_type', ['name' => $name]));
app('preferences')->mark();
@@ -135,7 +140,7 @@ class LinkController extends Controller
/**
* Edit a link form.
*
* @param Request $request
* @param Request $request
* @param LinkType $linkType
*
* @return \Illuminate\Contracts\View\Factory|\Illuminate\Http\RedirectResponse|\Illuminate\Routing\Redirector|\Illuminate\View\View
@@ -164,20 +169,18 @@ class LinkController extends Controller
/**
* Show index of all links.
*
* @param LinkTypeRepositoryInterface $repository
*
* @return \Illuminate\Contracts\View\Factory|\Illuminate\View\View
*/
public function index(LinkTypeRepositoryInterface $repository)
public function index()
{
$subTitle = (string)trans('firefly.journal_link_configuration');
$subTitleIcon = 'fa-link';
$linkTypes = $repository->get();
$linkTypes = $this->repository->get();
Log::channel('audit')->info('User on index of link types in admin.');
$linkTypes->each(
function (LinkType $linkType) use ($repository) {
$linkType->journalCount = $repository->countJournals($linkType);
function (LinkType $linkType) {
$linkType->journalCount = $this->repository->countJournals($linkType);
}
);
@@ -195,7 +198,7 @@ class LinkController extends Controller
{
$subTitle = (string)trans('firefly.overview_for_link', ['name' => $linkType->name]);
$subTitleIcon = 'fa-link';
$links = $linkType->transactionJournalLinks()->get();
$links = $this->repository->getJournalLinks($linkType);
Log::channel('audit')->info(sprintf('User viewing link type #%d', $linkType->id));
@@ -205,19 +208,18 @@ class LinkController extends Controller
/**
* Store the new link.
*
* @param LinkTypeFormRequest $request
* @param LinkTypeRepositoryInterface $repository
* @param LinkTypeFormRequest $request
*
* @return $this|\Illuminate\Http\RedirectResponse|\Illuminate\Routing\Redirector
*/
public function store(LinkTypeFormRequest $request, LinkTypeRepositoryInterface $repository)
public function store(LinkTypeFormRequest $request)
{
$data = [
'name' => $request->string('name'),
'inward' => $request->string('inward'),
'outward' => $request->string('outward'),
];
$linkType = $repository->store($data);
$linkType = $this->repository->store($data);
Log::channel('audit')->info('User stored new link type.', $linkType->toArray());
@@ -237,13 +239,12 @@ class LinkController extends Controller
/**
* Update an existing link.
*
* @param LinkTypeFormRequest $request
* @param LinkTypeRepositoryInterface $repository
* @param LinkType $linkType
* @param LinkTypeFormRequest $request
* @param LinkType $linkType
*
* @return $this|\Illuminate\Http\RedirectResponse|\Illuminate\Routing\Redirector
*/
public function update(LinkTypeFormRequest $request, LinkTypeRepositoryInterface $repository, LinkType $linkType)
public function update(LinkTypeFormRequest $request, LinkType $linkType)
{
if (!$linkType->editable) {
$request->session()->flash('error', (string)trans('firefly.cannot_edit_link_type', ['name' => $linkType->name]));
@@ -256,7 +257,7 @@ class LinkController extends Controller
'inward' => $request->string('inward'),
'outward' => $request->string('outward'),
];
$repository->update($linkType, $data);
$this->repository->update($linkType, $data);
Log::channel('audit')->info(sprintf('User update link type #%d.', $linkType->id), $data);

View File

@@ -35,6 +35,9 @@ use Log;
*/
class UserController extends Controller
{
/** @var UserRepositoryInterface */
private $repository;
/**
* UserController constructor.
*/
@@ -46,7 +49,7 @@ class UserController extends Controller
function ($request, $next) {
app('view')->share('title', (string)trans('firefly.administration'));
app('view')->share('mainTitleIcon', 'fa-hand-spock-o');
$this->repository = app(UserRepositoryInterface::class);
return $next($request);
}
);
@@ -72,13 +75,12 @@ class UserController extends Controller
* Destroy a user.
*
* @param User $user
* @param UserRepositoryInterface $repository
*
* @return \Illuminate\Http\RedirectResponse|\Illuminate\Routing\Redirector
*/
public function destroy(User $user, UserRepositoryInterface $repository)
public function destroy(User $user)
{
$repository->destroy($user);
$this->repository->destroy($user);
session()->flash('success', (string)trans('firefly.user_deleted'));
return redirect(route('admin.users'));
@@ -114,22 +116,20 @@ class UserController extends Controller
/**
* Show index of user manager.
*
* @param UserRepositoryInterface $repository
*
* @return \Illuminate\Contracts\View\Factory|\Illuminate\View\View
*/
public function index(UserRepositoryInterface $repository)
public function index()
{
$subTitle = (string)trans('firefly.user_administration');
$subTitleIcon = 'fa-users';
$users = $repository->all();
$users = $this->repository->all();
// add meta stuff.
$users->each(
function (User $user) use ($repository) {
function (User $user) {
$list = ['twoFactorAuthEnabled', 'twoFactorAuthSecret'];
$preferences = app('preferences')->getArrayForUser($user, $list);
$user->isAdmin = $repository->hasRole($user, 'owner');
$user->isAdmin = $this->repository->hasRole($user, 'owner');
$is2faEnabled = 1 === $preferences['twoFactorAuthEnabled'];
$has2faSecret = null !== $preferences['twoFactorAuthSecret'];
$user->has2FA = ($is2faEnabled && $has2faSecret);
@@ -143,18 +143,17 @@ class UserController extends Controller
/**
* Show single user.
*
* @param UserRepositoryInterface $repository
* @param User $user
*
* @return \Illuminate\Contracts\View\Factory|\Illuminate\View\View
*/
public function show(UserRepositoryInterface $repository, User $user)
public function show(User $user)
{
$title = (string)trans('firefly.administration');
$mainTitleIcon = 'fa-hand-spock-o';
$subTitle = (string)trans('firefly.single_user_administration', ['email' => $user->email]);
$subTitleIcon = 'fa-user';
$information = $repository->getUserData($user);
$information = $this->repository->getUserData($user);
return view(
'admin.users.show', compact(
@@ -168,22 +167,21 @@ class UserController extends Controller
*
* @param UserFormRequest $request
* @param User $user
* @param UserRepositoryInterface $repository
*
* @return $this|\Illuminate\Http\RedirectResponse|\Illuminate\Routing\Redirector
*/
public function update(UserFormRequest $request, User $user, UserRepositoryInterface $repository)
public function update(UserFormRequest $request, User $user)
{
Log::debug('Actually here');
$data = $request->getUserData();
// update password
if ('' !== $data['password']) {
$repository->changePassword($user, $data['password']);
$this->repository->changePassword($user, $data['password']);
}
$repository->changeStatus($user, $data['blocked'], $data['blocked_code']);
$repository->updateEmail($user, $data['email']);
$this->repository->changeStatus($user, $data['blocked'], $data['blocked_code']);
$this->repository->updateEmail($user, $data['email']);
session()->flash('success', (string)trans('firefly.updated_user', ['email' => $user->email]));
app('preferences')->mark();

View File

@@ -33,6 +33,7 @@ use Log;
/**
* Class ForgotPasswordController
* @codeCoverageIgnore
*/
class ForgotPasswordController extends Controller
{
@@ -72,6 +73,7 @@ class ForgotPasswordController extends Controller
$this->validateEmail($request);
// verify if the user is not a demo user. If so, we give him back an error.
/** @var User $user */
$user = User::where('email', $request->get('email'))->first();
if (null !== $user && $repository->hasRole($user, 'demo')) {

View File

@@ -69,22 +69,25 @@ class AmountController extends Controller
/**
* Set the amount for a single budget in a specific period. Shows a waring when its a lot.
* Set the amount for a single budget in a specific period.
*
* @param Request $request
* @param BudgetRepositoryInterface $repository
* @param Budget $budget
*
* @return JsonResponse
*
* @SuppressWarnings(PHPMD.ExcessiveMethodLength)
*/
public function amount(Request $request, BudgetRepositoryInterface $repository, Budget $budget): JsonResponse
public function amount(Request $request, Budget $budget): JsonResponse
{
// grab vars from URI
$amount = (string)$request->get('amount');
$start = Carbon::createFromFormat('Y-m-d', $request->get('start'));
$end = Carbon::createFromFormat('Y-m-d', $request->get('end'));
/** @var Carbon $start */
$start = Carbon::createFromFormat('Y-m-d', $request->get('start'));
/** @var Carbon $end */
$end = Carbon::createFromFormat('Y-m-d', $request->get('end'));
// grab other useful vars
$currency = app('amount')->getDefaultCurrency();
@@ -95,11 +98,11 @@ class AmountController extends Controller
$budgetLimit = $this->repository->updateLimitAmount($budget, $start, $end, $amount);
// calculate what the user has spent in current period.
$spent = $repository->spentInPeriod(new Collection([$budget]), new Collection, $start, $end);
$spent = $this->repository->spentInPeriod(new Collection([$budget]), new Collection, $start, $end);
// given the new budget, this is what they have left (and left per day?)
$left = app('amount')->formatAnything($currency, bcadd($amount, $spent), true);
$leftPerDay = null; //
$leftPerDay = null;
// If the user budgets ANY amount per day for this budget (anything but zero) Firefly III calculates how much he could spend per day.
if (1 === bccomp(bcadd($amount, $spent), '0')) {
@@ -113,7 +116,7 @@ class AmountController extends Controller
// If the difference is very large, give the user a notification.
$average = $this->repository->budgetedPerDay($budget);
$current = bcdiv($amount, (string)$periodLength);
if (bccomp(bcmul('1.1', $average), $current) === -1) {
if (bccomp(bcmul('1.3', $average), $current) === -1) {
$largeDiff = true;
$warnText = (string)trans(
'firefly.over_budget_warn',
@@ -141,68 +144,6 @@ class AmountController extends Controller
);
}
/**
* Shows some basic info about the income and the suggested budget.
*
* @param Carbon $start
* @param Carbon $end
* TODO remove this feature
*
* @return \Illuminate\Contracts\View\Factory|\Illuminate\View\View
*/
public function infoIncome(Carbon $start, Carbon $end)
{
$range = app('preferences')->get('viewRange', '1M')->data;
/** @var Carbon $searchBegin */
$searchBegin = app('navigation')->subtractPeriod($start, $range, 3);
$searchEnd = app('navigation')->addPeriod($end, $range, 3);
$daysInPeriod = $start->diffInDays($end);
$daysInSearchPeriod = $searchBegin->diffInDays($searchEnd);
$average = $this->repository->getAverageAvailable($start, $end);
$available = bcmul($average, (string)$daysInPeriod);
Log::debug(sprintf('Average is %s, so total available is %s because days is %d.', $average, $available, $daysInPeriod));
// amount earned in this period:
/** @var GroupCollectorInterface $collector */
$collector = app(GroupCollectorInterface::class);
$collector->setRange($searchBegin, $searchEnd)->setTypes([TransactionType::DEPOSIT]);
$earned = $collector->getSum();
// Total amount earned divided by the number of days in the whole search period is the average amount earned per day.
// This is multiplied by the number of days in the current period, showing you the average.
$earnedAverage = bcmul(bcdiv($earned, (string)$daysInSearchPeriod), (string)$daysInPeriod);
Log::debug(sprintf('Earned is %s, earned average is %s', $earned, $earnedAverage));
// amount spent in period
/** @var GroupCollectorInterface $collector */
$collector = app(GroupCollectorInterface::class);
$collector->setRange($searchBegin, $searchEnd)->setTypes([TransactionType::WITHDRAWAL]);
$spent = $collector->getSum();
$spentAverage = app('steam')->positive(bcmul(bcdiv($spent, (string)$daysInSearchPeriod), (string)$daysInPeriod));
Log::debug(sprintf('Spent is %s, spent average is %s', $earned, $earnedAverage));
// the default suggestion is the money the user has spent, on average, over this period.
$suggested = $spentAverage;
Log::debug(sprintf('Suggested is now %s (spent average)', $suggested));
// if the user makes less per period, suggest that amount instead.
if (1 === bccomp($spentAverage, $earnedAverage)) {
Log::debug(
sprintf('Because earned average (%s) is less than spent average (%s) will suggest earned average instead.', $earnedAverage, $spentAverage)
);
$suggested = $earnedAverage;
}
$result = ['available' => $available, 'earned' => $earnedAverage, 'spent' => $spentAverage, 'suggested' => $suggested,];
return view('budgets.info', compact('result', 'searchBegin', 'searchEnd', 'start', 'end'));
}
/**
* Store an available budget for the current period.
*
@@ -212,7 +153,9 @@ class AmountController extends Controller
*/
public function postUpdateIncome(BudgetIncomeRequest $request): RedirectResponse
{
/** @var Carbon $start */
$start = Carbon::createFromFormat('Y-m-d', $request->string('start'));
/** @var Carbon $end */
$end = Carbon::createFromFormat('Y-m-d', $request->string('end'));
$defaultCurrency = app('amount')->getDefaultCurrency();
$amount = $request->get('amount');
@@ -240,6 +183,6 @@ class AmountController extends Controller
$available = round($available, $defaultCurrency->decimal_places);
$page = (int)$request->get('page');
return view('budgets.income', compact('available', 'start', 'end', 'page'));
return view('budgets.income', compact('available', 'start', 'end', 'page','defaultCurrency'));
}
}

View File

@@ -40,6 +40,7 @@ class CreateController extends Controller
/**
* CreateController constructor.
* @codeCoverageIgnore
*/
public function __construct()
{

View File

@@ -40,6 +40,7 @@ class DeleteController extends Controller
/**
* DeleteController constructor.
* @codeCoverageIgnore
*/
public function __construct()
{

View File

@@ -42,6 +42,7 @@ class EditController extends Controller
/**
* EditController constructor.
* @codeCoverageIgnore
*/
public function __construct()
{

View File

@@ -45,6 +45,7 @@ class IndexController extends Controller
/**
* IndexController constructor.
* @codeCoverageIgnore
*/
public function __construct()
{

View File

@@ -43,20 +43,22 @@ use Illuminate\Http\Request;
class ShowController extends Controller
{
use PeriodOverview, AugumentData;
/** @var JournalRepositoryInterface */
private $journalRepos;
/**
* ShowController constructor.
*
* @codeCoverageIgnore
*/
public function __construct()
{
parent::__construct();
app('view')->share('hideBudgets', true);
$this->middleware(
function ($request, $next) {
app('view')->share('title', (string)trans('firefly.budgets'));
app('view')->share('mainTitleIcon', 'fa-tasks');
$this->journalRepos = app(JournalRepositoryInterface::class);
return $next($request);
}
@@ -82,9 +84,13 @@ class ShowController extends Controller
'firefly.without_budget_between',
['start' => $start->formatLocalized($this->monthAndDayFormat), 'end' => $end->formatLocalized($this->monthAndDayFormat)]
);
$periods = $this->getNoBudgetPeriodOverview($end);
$page = (int)$request->get('page');
$pageSize = (int)app('preferences')->get('listPageSize', 50)->data;
// get first journal ever to set off the budget period overview.
$first = $this->journalRepos->firstNull();
$firstDate = null !== $first ? $first->date : $start;
$periods = $this->getNoBudgetPeriodOverview($firstDate, $end);
$page = (int)$request->get('page');
$pageSize = (int)app('preferences')->get('listPageSize', 50)->data;
/** @var GroupCollectorInterface $collector */
$collector = app(GroupCollectorInterface::class);
@@ -100,16 +106,13 @@ class ShowController extends Controller
* Shows ALL transactions without a budget.
*
* @param Request $request
* @param JournalRepositoryInterface $repository
*
* @return \Illuminate\Contracts\View\Factory|\Illuminate\View\View
*
* @SuppressWarnings(PHPMD.UnusedLocalVariable)
*/
public function noBudgetAll(Request $request, JournalRepositoryInterface $repository)
public function noBudgetAll(Request $request)
{
$subTitle = (string)trans('firefly.all_journals_without_budget');
$first = $repository->firstNull();
$first = $this->journalRepos->firstNull();
$start = null === $first ? new Carbon : $first->date;
$end = new Carbon;
$page = (int)$request->get('page');
@@ -169,7 +172,7 @@ class ShowController extends Controller
public function showByBudgetLimit(Request $request, Budget $budget, BudgetLimit $budgetLimit)
{
if ($budgetLimit->budget->id !== $budget->id) {
throw new FireflyException('This budget limit is not part of this budget.');
throw new FireflyException('This budget limit is not part of this budget.'); // @codeCoverageIgnore
}
$page = (int)$request->get('page');

View File

@@ -28,6 +28,7 @@ use FireflyIII\Models\Budget;
* Class BudgetFormRequest.
*
* @codeCoverageIgnore
* TODO after 4.8.0, split for update/store
*/
class BudgetFormRequest extends Request
{

View File

@@ -23,7 +23,7 @@ declare(strict_types=1);
namespace FireflyIII\Http\Requests;
/**
* Class BillFormRequest.
* Class LinkTypeFormRequest.
*/
class LinkTypeFormRequest extends Request
{