Various code cleanup.

2025-02-25 18:45:27 -06:00 · 2017-03-29 21:20:54 +02:00 · 2017-03-29 21:20:54 +02:00 · 5920ccee04
commit 5920ccee04
parent 569fec4610
11 changed files with 287 additions and 132 deletions
--- a/app/Http/Controllers/Popup/ReportController.php
+++ b/app/Http/Controllers/Popup/ReportController.php
@ -9,7 +9,7 @@
 * See the LICENSE file for details.
 */

-declare(strict_types = 1);
+declare(strict_types=1);

 namespace FireflyIII\Http\Controllers\Popup;

@ -18,6 +18,7 @@ use Carbon\Carbon;
 use FireflyIII\Exceptions\FireflyException;
 use FireflyIII\Helpers\Collection\BalanceLine;
 use FireflyIII\Helpers\Collector\JournalCollectorInterface;
+use FireflyIII\Helpers\Report\PopupReportInterface;
 use FireflyIII\Http\Controllers\Controller;
 use FireflyIII\Models\Transaction;
 use FireflyIII\Models\TransactionType;
@ -39,6 +40,44 @@ use View;
 class ReportController extends Controller
 {

+    /** @var AccountRepositoryInterface */
+    private $accountRepository;
+    /** @var BudgetRepositoryInterface */
+    private $budgetRepository;
+    /** @var CategoryRepositoryInterface */
+    private $categoryRepository;
+    /** @var PopupReportInterface */
+    private $popupHelper;
+
+
+    /**
+     *
+     */
+    public function __construct()
+    {
+        parent::__construct();
+        $this->middleware(
+            function ($request, $next) {
+
+                /** @var AccountRepositoryInterface $repository */
+                $this->accountRepository = app(AccountRepositoryInterface::class);
+
+                /** @var BudgetRepositoryInterface $repository */
+                $this->budgetRepository = app(BudgetRepositoryInterface::class);
+
+                /** @var CategoryRepositoryInterface categoryRepository */
+                $this->categoryRepository = app(CategoryRepositoryInterface::class);
+
+                /** @var PopupReportInterface popupHelper */
+                $this->popupHelper = app(PopupReportInterface::class);
+
+                return $next($request);
+            }
+        );
+
+    }
+
+
    /**
     * @param Request $request
     *
@ -58,7 +97,6 @@ class ReportController extends Controller
                throw new FireflyException('Firefly cannot handle "' . e($attributes['location']) . '" ');
            case 'budget-spent-amount':
                $html = $this->budgetSpentAmount($attributes);
-
                break;
            case 'expense-entry':
                $html = $this->expenseEntry($attributes);
@ -87,30 +125,19 @@ class ReportController extends Controller
     */
    private function balanceAmount(array $attributes): string
    {
-        $role = intval($attributes['role']);
-
-        /** @var BudgetRepositoryInterface $budgetRepository */
-        $budgetRepository = app(BudgetRepositoryInterface::class);
-        $budget           = $budgetRepository->find(intval($attributes['budgetId']));
-
-        /** @var AccountRepositoryInterface $repository */
-        $repository = app(AccountRepositoryInterface::class);
-
-        $account = $repository->find(intval($attributes['accountId']));
+        $role    = intval($attributes['role']);
+        $budget  = $this->budgetRepository->find(intval($attributes['budgetId']));
+        $account = $this->accountRepository->find(intval($attributes['accountId']));
        $types   = [TransactionType::WITHDRAWAL];

        switch (true) {
            case ($role === BalanceLine::ROLE_DEFAULTROLE && !is_null($budget->id)):
-                /** @var JournalCollectorInterface $collector */
-                $collector = app(JournalCollectorInterface::class);
-                $collector
-                    ->setAccounts(new Collection([$account]))
-                    ->setRange($attributes['startDate'], $attributes['endDate'])
-                    ->setBudget($budget);
-                $journals = $collector->getJournals();
-
+                // normal row with a budget:
+                $journals = $this->popupHelper->balanceForBudget($budget, $account, $attributes);
                break;
            case ($role === BalanceLine::ROLE_DEFAULTROLE && is_null($budget->id)):
+                // normal row without a budget:
+                $journals = $this->popupHelper->balanceForNoBudget($account, $attributes);
                $budget->name = strval(trans('firefly.no_budget'));
                /** @var JournalCollectorInterface $collector */
                $collector = app(JournalCollectorInterface::class);
@ -122,6 +149,7 @@ class ReportController extends Controller
                $journals = $collector->getJournals();
                break;
            case ($role === BalanceLine::ROLE_DIFFROLE):
+                // row that displays difference
                /** @var JournalCollectorInterface $collector */
                $collector = app(JournalCollectorInterface::class);
                $collector
@ -144,6 +172,7 @@ class ReportController extends Controller
                );
                break;
            case ($role === BalanceLine::ROLE_TAGROLE):
+                // row with tag info.
                throw new FireflyException('Firefly cannot handle this type of info-button (BalanceLine::TagRole)');
        }
        $view = view('popup.report.balance-amount', compact('journals', 'budget', 'account'))->render();
@ -161,27 +190,8 @@ class ReportController extends Controller
     */
    private function budgetSpentAmount(array $attributes): string
    {
-        // need to find the budget
-        // then search for expenses in the given period
-        // list them in some table format.
-        /** @var BudgetRepositoryInterface $repository */
-        $repository = app(BudgetRepositoryInterface::class);
-        $budget     = $repository->find(intval($attributes['budgetId']));
-        /** @var JournalCollectorInterface $collector */
-        $collector = app(JournalCollectorInterface::class);
-
-        $collector
-            ->setAccounts($attributes['accounts'])
-            ->setRange($attributes['startDate'], $attributes['endDate']);
-
-        if (is_null($budget->id)) {
-            $collector->setTypes([TransactionType::WITHDRAWAL])->withoutBudget();
-        }
-        if (!is_null($budget->id)) {
-            // get all expenses in budget in period:
-            $collector->setBudget($budget);
-        }
-        $journals = $collector->getJournals();
+        $budget   = $this->budgetRepository->find(intval($attributes['budgetId']));
+        $journals = $this->popupHelper->byBudget($budget, $attributes);
        $view     = view('popup.report.budget-spent-amount', compact('journals', 'budget'))->render();

        return $view;
@ -197,18 +207,9 @@ class ReportController extends Controller
     */
    private function categoryEntry(array $attributes): string
    {
-        /** @var CategoryRepositoryInterface $repository */
-        $repository = app(CategoryRepositoryInterface::class);
-        $category   = $repository->find(intval($attributes['categoryId']));
-        $types      = [TransactionType::WITHDRAWAL, TransactionType::TRANSFER];
-        /** @var JournalCollectorInterface $collector */
-        $collector = app(JournalCollectorInterface::class);
-        $collector->setAccounts($attributes['accounts'])->setTypes($types)
-                  ->setRange($attributes['startDate'], $attributes['endDate'])
-                  ->setCategory($category);
-        $journals = $collector->getJournals(); // 7193
-
-        $view = view('popup.report.category-entry', compact('journals', 'category'))->render();
+        $category = $this->categoryRepository->find(intval($attributes['categoryId']));
+        $journals = $this->popupHelper->byCategory($category, $attributes);
+        $view     = view('popup.report.category-entry', compact('journals', 'category'))->render();

        return $view;
    }
@ -223,29 +224,9 @@ class ReportController extends Controller
     */
    private function expenseEntry(array $attributes): string
    {
-        /** @var AccountRepositoryInterface $repository */
-        $repository = app(AccountRepositoryInterface::class);
-
-        $account = $repository->find(intval($attributes['accountId']));
-        $types   = [TransactionType::WITHDRAWAL, TransactionType::TRANSFER];
-        /** @var JournalCollectorInterface $collector */
-        $collector = app(JournalCollectorInterface::class);
-        $collector->setAccounts(new Collection([$account]))->setRange($attributes['startDate'], $attributes['endDate'])->setTypes($types);
-        $journals = $collector->getJournals();
-        $report   = $attributes['accounts']->pluck('id')->toArray(); // accounts used in this report
-
-        // filter for transfers and withdrawals TO the given $account
-        $journals = $journals->filter(
-            function (Transaction $transaction) use ($report) {
-                // get the destinations:
-                $sources = $transaction->transactionJournal->sourceAccountList()->pluck('id')->toArray();
-
-                // do these intersect with the current list?
-                return !empty(array_intersect($report, $sources));
-            }
-        );
-
-        $view = view('popup.report.expense-entry', compact('journals', 'account'))->render();
+        $account  = $this->accountRepository->find(intval($attributes['accountId']));
+        $journals = $this->popupHelper->byExpenses($account, $attributes);
+        $view     = view('popup.report.expense-entry', compact('journals', 'account'))->render();

        return $view;
    }
@ -260,28 +241,9 @@ class ReportController extends Controller
     */
    private function incomeEntry(array $attributes): string
    {
-        /** @var AccountRepositoryInterface $repository */
-        $repository = app(AccountRepositoryInterface::class);
-        $account    = $repository->find(intval($attributes['accountId']));
-        $types      = [TransactionType::DEPOSIT, TransactionType::TRANSFER];
-        /** @var JournalCollectorInterface $collector */
-        $collector = app(JournalCollectorInterface::class);
-        $collector->setAccounts(new Collection([$account]))->setRange($attributes['startDate'], $attributes['endDate'])->setTypes($types);
-        $journals = $collector->getJournals();
-        $report   = $attributes['accounts']->pluck('id')->toArray(); // accounts used in this report
-
-        // filter the set so the destinations outside of $attributes['accounts'] are not included.
-        $journals = $journals->filter(
-            function (Transaction $transaction) use ($report) {
-                // get the destinations:
-                $destinations = $transaction->destinationAccountList($transaction->transactionJournal)->pluck('id')->toArray();
-
-                // do these intersect with the current list?
-                return !empty(array_intersect($report, $destinations));
-            }
-        );
-
-        $view = view('popup.report.income-entry', compact('journals', 'account'))->render();
+        $account  = $this->accountRepository->find(intval($attributes['accountId']));
+        $journals = $this->popupHelper->byIncome($account, $attributes);
+        $view     = view('popup.report.income-entry', compact('journals', 'account'))->render();

        return $view;
    }
--- a/app/Http/Controllers/ReportController.php
+++ b/app/Http/Controllers/ReportController.php
@ -48,6 +48,7 @@ class ReportController extends Controller
    {
        parent::__construct();

+        $this->helper = app(ReportHelperInterface::class);

        $this->middleware(
            function ($request, $next) {
@ -55,8 +56,6 @@ class ReportController extends Controller
                View::share('mainTitleIcon', 'fa-line-chart');
                View::share('subTitleIcon', 'fa-calendar');

-                $this->helper = app(ReportHelperInterface::class);
-
                return $next($request);
            }
        );
--- a/app/Providers/FireflyServiceProvider.php
+++ b/app/Providers/FireflyServiceProvider.php
@ -9,7 +9,7 @@
 * See the LICENSE file for details.
 */

-declare(strict_types = 1);
+declare(strict_types=1);

 namespace FireflyIII\Providers;

@ -29,6 +29,8 @@ use FireflyIII\Helpers\Report\BalanceReportHelper;
 use FireflyIII\Helpers\Report\BalanceReportHelperInterface;
 use FireflyIII\Helpers\Report\BudgetReportHelper;
 use FireflyIII\Helpers\Report\BudgetReportHelperInterface;
+use FireflyIII\Helpers\Report\PopupReport;
+use FireflyIII\Helpers\Report\PopupReportInterface;
 use FireflyIII\Helpers\Report\ReportHelper;
 use FireflyIII\Helpers\Report\ReportHelperInterface;
 use FireflyIII\Import\ImportProcedure;
@ -127,6 +129,8 @@ class FireflyServiceProvider extends ServiceProvider
        $this->app->bind(UserRepositoryInterface::class, UserRepository::class);
        $this->app->bind(AttachmentHelperInterface::class, AttachmentHelper::class);

+        // more generators:
+        $this->app->bind(PopupReportInterface::class, PopupReport::class);
        $this->app->bind(HelpInterface::class, Help::class);
        $this->app->bind(ReportHelperInterface::class, ReportHelper::class);
        $this->app->bind(FiscalHelperInterface::class, FiscalHelper::class);
--- a/config/app.php
+++ b/config/app.php
@ -9,7 +9,7 @@
 * See the LICENSE file for details.
 */

-declare(strict_types = 1);
+declare(strict_types=1);

 return [
    'name'            => 'Firefly III',
@ -70,7 +70,7 @@ return [
        //Barryvdh\Debugbar\ServiceProvider::class,
        DaveJamesMiller\Breadcrumbs\ServiceProvider::class,
        TwigBridge\ServiceProvider::class,
-        'PragmaRX\Google2FA\Vendor\Laravel\ServiceProvider',
+        PragmaRX\Google2FA\Vendor\Laravel\ServiceProvider::class,

        /*
         * More service providers.
@ -125,7 +125,7 @@ return [
        'URL'           => Illuminate\Support\Facades\URL::class,
        'Validator'     => Illuminate\Support\Facades\Validator::class,
        'View'          => Illuminate\Support\Facades\View::class,
-        'Twig'          => 'TwigBridge\Facade\Twig',
+        'Twig'          => TwigBridge\Facade\Twig::class,
        'Form'          => Collective\Html\FormFacade::class,
        'Html'          => Collective\Html\HtmlFacade::class,
        'Breadcrumbs'   => 'DaveJamesMiller\Breadcrumbs\Facade',
@ -137,7 +137,7 @@ return [
        'ExpandedForm'  => 'FireflyIII\Support\Facades\ExpandedForm',
        'Entrust'       => 'Zizaco\Entrust\EntrustFacade',
        'Input'         => 'Illuminate\Support\Facades\Input',
-        'Google2FA'     => 'PragmaRX\Google2FA\Vendor\Laravel\Facade',
+        'Google2FA'     => PragmaRX\Google2FA\Vendor\Laravel\Facade::class,
    ],

 ];
--- a/database/factories/ModelFactory.php
+++ b/database/factories/ModelFactory.php
@ -116,8 +116,10 @@ $factory->define(
 $factory->define(
    FireflyIII\Models\Tag::class, function (Faker\Generator $faker) {
    return [
-        'id'  => $faker->numberBetween(1, 10),
-        'tag' => $faker->words(1, true),
+        'id'      => $faker->numberBetween(100, 150),
+        'user_id' => 1,
+        'tagMode' => 'nothing',
+        'tag'     => $faker->words(1, true),
    ];
 }
 );
--- a/resources/views/reports/partials/bills.twig
+++ b/resources/views/reports/partials/bills.twig
@ -10,7 +10,7 @@
                <th data-defaultsign="_19" class="hidden-xs" style="text-align: right;">{{ trans('form.amount_min') }}</th>
                <th data-defaultsign="_19" class="hidden-xs" style="text-align: right;">{{ trans('form.amount_max') }}</th>
                <th data-defaultsign="_19" style="text-align: right;">{{ trans('form.amount') }}</th>
-                <th data-defaultsign="_19" style="text-align: right;">{{ trans('form.under') }}</th>
+                <th data-defaultsign="_19" class="hidden-xs" style="text-align: right;">{{ trans('form.under') }}</th>
            </tr>
            </thead>
            <tbody>
@ -34,7 +34,7 @@
                    {% if not line.isActive %}
                        <td data-value="-1">&nbsp;</td>
                    {% endif %}
-                    <td data-value="{{ (line.getMax - line.getAmount) }}" style="text-align: right;">
+                    <td data-value="{{ (line.getMax - line.getAmount) }}" style="text-align: right;" class="hidden-xs">
                        {% if line.isActive %}
                            {{ (line.getMax + line.getAmount)|formatAmount }}
                        {% endif %}
--- a/resources/views/reports/partials/budgets.twig
+++ b/resources/views/reports/partials/budgets.twig
@ -3,10 +3,10 @@
    <tr>
        <th data-defaultsign="az">{{ 'budget'|_ }}</th>
        <th data-defaultsign="month" class="hidden-xs">{{ 'date'|_ }}</th>
-        <th data-defaultsign="_19" style="text-align: right;">{{ 'budgeted'|_ }}</th>
+        <th data-defaultsign="_19" style="text-align: right;" class="hidden-xs">{{ 'budgeted'|_ }}</th>
        <th data-defaultsign="_19" style="text-align: right;">{{ 'spent'|_ }}</th>
-        <th data-defaultsort="disabled">&nbsp;</th>
-        <th data-defaultsign="_19" style="text-align: right;">{{ 'left'|_ }}</th>
+        <th data-defaultsort="disabled" class="hidden-xs">&nbsp;</th>
+        <th data-defaultsign="_19" style="text-align: right;" class="hidden-xs">{{ 'left'|_ }}</th>
        <th data-defaultsign="_19" style="text-align: right;">{{ 'overspent'|_ }}</th>
    </tr>
    </thead>
@ -23,6 +23,7 @@
        {% set sum_overspent = sum_overspent + line.overspent %}

        <tr>
+            {# Budget name, always visible #}
            {% if line.type == 'no-budget' %}
                <td data-value="zzzzzzz">
                    <em>{{ 'no_budget'|_ }}</em>
@ -32,6 +33,7 @@
                    <a href="{{ route('budgets.show',line.id) }}">{{ line.name }}</a>
                </td>
            {% endif %}
+            {# date, hidden on mobile #}
            {% if line.type == 'budget-line' %}
                <td class="hidden-xs" data-value="{{ line.start.format('Y-m-d') }}">
                    <a href="{{ route('budgets.show.limit', [line.id, line.limit]) }}">
@ -41,25 +43,35 @@
                    </a>
                </td>
            {% else %}
-                <td data-value="0000-00-00">
+                <td data-value="0000-00-00" class="hidden-xs">

                </td>
            {% endif %}
-            <td data-value="{{ line.budgeted }}" style="text-align: right;">
+
+            {# budgeted, hidden on mobile #}
+            <td data-value="{{ line.budgeted }}" style="text-align: right;" class="hidden-xs">
                {{ line.budgeted|formatAmount }}
            </td>
+
+            {# spent, visible on mobile #}
            <td data-value="{{ line.spent }}" style="text-align: right;">
                {{ line.spent|formatAmount }}
            </td>
-            <td>
+
+            {# info button, not visible on mobile #}
+            <td class="hidden-xs">
                {% if line.spent != 0 %}
                    <i class="fa fa-fw text-muted fa-info-circle firefly-info-button"
                       data-location="budget-spent-amount" data-budget-id="{{ line.id }}"></i>
                {% endif %}
            </td>
-            <td data-value="{{ line.left }}" style="text-align: right;">
+
+            {# left, hidden on mobile #}
+            <td data-value="{{ line.left }}" style="text-align: right;" class="hidden-xs">
                {{ line.left|formatAmount }}
            </td>
+
+            {# overspent, visible. #}
            <td data-value="{{ line.overspent }}" style="text-align: right;">
                {{ line.overspent|formatAmount }}
            </td>
@ -68,12 +80,22 @@
    </tbody>
    <tfoot>
    <tr>
+        {# title, visible #}
        <td><em>{{ 'sum'|_ }}</em></td>
+        {# date, hidden #}
        <td class="hidden-xs">&nbsp;</td>
-        <td style="text-align: right;">{{ sum_budgeted|formatAmount }}</td>
+
+        {# sum of budgeted, hidden #}
+        <td style="text-align: right;" class="hidden-xs">{{ sum_budgeted|formatAmount }}</td>
+
+        {# spent, visible #}
        <td style="text-align: right;">{{ sum_spent|formatAmount }}</td>
-        <td>&nbsp;</td>
-        <td style="text-align: right;">{{ sum_left|formatAmount }}</td>
+
+        {# info button, hidden #}
+        <td class="hidden-xs">&nbsp;</td>
+
+        {# left, hidden #}
+        <td style="text-align: right;" class="hidden-xs">{{ sum_left|formatAmount }}</td>
        <td style="text-align: right;">{{ sum_overspent|formatAmount }}</td>
    </tr>
    </tfoot>
--- a/resources/views/reports/partials/categories.twig
+++ b/resources/views/reports/partials/categories.twig
@ -30,7 +30,7 @@
    <tfoot>
    {% if report|length > listLength %}
        <tr>
-            <td colspan="2" class="active">
+            <td colspan="3" class="active">
                <a href="#" class="listLengthTrigger">{{ trans('firefly.show_full_list',{number:incomeTopLength}) }}</a>
            </td>
        </tr>
--- a/tests/Feature/Controllers/Chart/CategoryReportControllerTest.php
+++ b/tests/Feature/Controllers/Chart/CategoryReportControllerTest.php
@ -7,7 +7,7 @@
 * See the LICENSE file for details.
 */

-declare(strict_types = 1);
+declare(strict_types=1);

 namespace Tests\Feature\Controllers\Chart;

@ -15,6 +15,7 @@ namespace Tests\Feature\Controllers\Chart;
 use FireflyIII\Generator\Chart\Basic\GeneratorInterface;
 use FireflyIII\Helpers\Chart\MetaPieChartInterface;
 use FireflyIII\Helpers\Collector\JournalCollectorInterface;
+use FireflyIII\Models\Transaction;
 use FireflyIII\Models\TransactionType;
 use Illuminate\Support\Collection;
 use Tests\TestCase;
@ -120,8 +121,9 @@ class CategoryReportControllerTest extends TestCase
     */
    public function testMainChart()
    {
-        $generator = $this->mock(GeneratorInterface::class);
-        $collector = $this->mock(JournalCollectorInterface::class);
+        $generator   = $this->mock(GeneratorInterface::class);
+        $collector   = $this->mock(JournalCollectorInterface::class);
+        $transaction = factory(Transaction::class)->make();

        $collector->shouldReceive('setAccounts')->andReturnSelf();
        $collector->shouldReceive('setRange')->andReturnSelf();
@ -130,7 +132,7 @@ class CategoryReportControllerTest extends TestCase
        $collector->shouldReceive('disableFilter')->andReturnSelf();
        $collector->shouldReceive('setCategories')->andReturnSelf();
        $collector->shouldReceive('withOpposingAccount')->andReturnSelf();
-        $collector->shouldReceive('getJournals')->andReturn(new Collection);
+        $collector->shouldReceive('getJournals')->andReturn(new Collection([$transaction]));
        $generator->shouldReceive('multiSet')->andReturn([])->once();

        $this->be($this->user());
--- a/tests/Feature/Controllers/Chart/TagReportControllerTest.php
+++ b/tests/Feature/Controllers/Chart/TagReportControllerTest.php
@ -7,7 +7,7 @@
 * See the LICENSE file for details.
 */

-declare(strict_types = 1);
+declare(strict_types=1);

 namespace Tests\Feature\Controllers\Chart;

@ -15,6 +15,8 @@ namespace Tests\Feature\Controllers\Chart;
 use FireflyIII\Generator\Chart\Basic\GeneratorInterface;
 use FireflyIII\Helpers\Chart\MetaPieChartInterface;
 use FireflyIII\Helpers\Collector\JournalCollectorInterface;
+use FireflyIII\Models\Tag;
+use FireflyIII\Models\Transaction;
 use FireflyIII\Models\TransactionType;
 use Illuminate\Support\Collection;
 use Tests\TestCase;
@ -114,11 +116,18 @@ class TagReportControllerTest extends TestCase

    /**
     * @covers \FireflyIII\Http\Controllers\Chart\TagReportController::mainChart()
+     * @covers \FireflyIII\Http\Controllers\Chart\TagReportController::getExpenses
+     * @covers \FireflyIII\Http\Controllers\Chart\TagReportController::getIncome
+     * @covers \FireflyIII\Http\Controllers\Chart\TagReportController::groupByTag
     */
    public function testMainChart()
    {
-        $generator = $this->mock(GeneratorInterface::class);
-        $collector = $this->mock(JournalCollectorInterface::class);
+        $generator   = $this->mock(GeneratorInterface::class);
+        $collector   = $this->mock(JournalCollectorInterface::class);
+        $transaction = factory(Transaction::class)->make();
+        $tag         = factory(Tag::class)->make();
+        $transaction->transactionJournal->tags()->save($tag);
+

        $collector->shouldReceive('setAccounts')->andReturnSelf();
        $collector->shouldReceive('setRange')->andReturnSelf();
@ -127,7 +136,7 @@ class TagReportControllerTest extends TestCase
        $collector->shouldReceive('disableFilter')->andReturnSelf();
        $collector->shouldReceive('setTags')->andReturnSelf();
        $collector->shouldReceive('withOpposingAccount')->andReturnSelf();
-        $collector->shouldReceive('getJournals')->andReturn(new Collection);
+        $collector->shouldReceive('getJournals')->andReturn(new Collection([$transaction]));
        $generator->shouldReceive('multiSet')->andReturn([])->once();

        $this->be($this->user());
@ -157,7 +166,7 @@ class TagReportControllerTest extends TestCase
    }

    /**
-     * @covers \FireflyIII\Http\Controllers\Chart\TagReportController::tagIncome()
+     * @covers \FireflyIII\Http\Controllers\Chart\TagReportController::tagIncome
     */
    public function testTagIncome()
    {
--- a/tests/Feature/Controllers/Popup/ReportControllerTest.php
+++ b/tests/Feature/Controllers/Popup/ReportControllerTest.php
@ -7,7 +7,7 @@
 * See the LICENSE file for details.
 */

-declare(strict_types = 1);
+declare(strict_types=1);

 namespace Tests\Feature\Controllers\Popup;

@ -17,6 +17,8 @@ use FireflyIII\Helpers\Collector\JournalCollectorInterface;
 use FireflyIII\Models\Account;
 use FireflyIII\Models\Budget;
 use FireflyIII\Models\Category;
+use FireflyIII\Models\Tag;
+use FireflyIII\Models\Transaction;
 use FireflyIII\Models\TransactionType;
 use FireflyIII\Repositories\Account\AccountRepositoryInterface;
 use FireflyIII\Repositories\Budget\BudgetRepositoryInterface;
@ -33,12 +35,98 @@ use Tests\TestCase;
 class ReportControllerTest extends TestCase
 {

+    /**
+     * @covers                   \FireflyIII\Http\Controllers\Popup\ReportController::general
+     * @covers                   \FireflyIII\Http\Controllers\Popup\ReportController::parseAttributes
+     * @expectedExceptionMessage Could not parse end date
+     */
+    public function testBadEndDate()
+    {
+        $this->be($this->user());
+        $arguments = [
+            'attributes' => [
+                'location'   => 'bla-bla',
+                'startDate'  => Carbon::now()->endOfMonth()->format('Ymd'),
+                'endDate'    => 'bla-bla',
+                'accounts'   => 1,
+                'accountId'  => 1,
+                'categoryId' => 1,
+                'budgetId'   => 1,
+            ],
+        ];
+        $uri       = route('popup.general') . '?' . http_build_query($arguments);
+        $response  = $this->get($uri);
+        $response->assertStatus(500);
+    }
+
+    /**
+     * @covers                   \FireflyIII\Http\Controllers\Popup\ReportController::general
+     * @covers                   \FireflyIII\Http\Controllers\Popup\ReportController::parseAttributes
+     * @expectedExceptionMessage Could not parse start date
+     */
+    public function testBadStartDate()
+    {
+        $this->be($this->user());
+        $arguments = [
+            'attributes' => [
+                'location'   => 'bla-bla',
+                'startDate'  => 'bla-bla',
+                'endDate'    => Carbon::now()->endOfMonth()->format('Ymd'),
+                'accounts'   => 1,
+                'accountId'  => 1,
+                'categoryId' => 1,
+                'budgetId'   => 1,
+            ],
+        ];
+        $uri       = route('popup.general') . '?' . http_build_query($arguments);
+        $response  = $this->get($uri);
+        $response->assertStatus(500);
+    }
+
    /**
     * @covers \FireflyIII\Http\Controllers\Popup\ReportController::general
     * @covers \FireflyIII\Http\Controllers\Popup\ReportController::parseAttributes
     * @covers \FireflyIII\Http\Controllers\Popup\ReportController::balanceAmount
     */
-    public function testBalanceAmount()
+    public function testBalanceAmountDefaultNoBudget()
+    {
+        $collector    = $this->mock(JournalCollectorInterface::class);
+        $accountRepos = $this->mock(AccountRepositoryInterface::class);
+        $budgetRepos  = $this->mock(BudgetRepositoryInterface::class);
+        $account      = factory(Account::class)->make();
+
+        $budgetRepos->shouldReceive('find')->andReturn(new Budget)->once()->withArgs([0]);
+        $accountRepos->shouldReceive('find')->andReturn($account)->once()->withArgs([1]);
+        $collector->shouldReceive('setAccounts')->andReturnSelf()->once();
+        $collector->shouldReceive('setRange')->once()->andReturnSelf();
+        $collector->shouldReceive('setTypes')->once()->andReturnSelf();
+        $collector->shouldReceive('withoutBudget')->once()->andReturnSelf();
+        $collector->shouldReceive('getJournals')->once()->andReturn(new Collection);
+
+        $this->be($this->user());
+        $arguments = [
+            'attributes' => [
+                'location'   => 'balance-amount',
+                'startDate'  => Carbon::now()->startOfMonth()->format('Ymd'),
+                'endDate'    => Carbon::now()->endOfMonth()->format('Ymd'),
+                'accounts'   => 1,
+                'accountId'  => 1,
+                'categoryId' => 1,
+                'budgetId'   => 0,
+                'role'       => 1, // ROLE_DEFAULTROLE
+            ],
+        ];
+        $uri       = route('popup.general') . '?' . http_build_query($arguments);
+        $response  = $this->get($uri);
+        $response->assertStatus(200);
+    }
+
+    /**
+     * @covers \FireflyIII\Http\Controllers\Popup\ReportController::general
+     * @covers \FireflyIII\Http\Controllers\Popup\ReportController::parseAttributes
+     * @covers \FireflyIII\Http\Controllers\Popup\ReportController::balanceAmount
+     */
+    public function testBalanceAmountDefaultRole()
    {
        $collector    = $this->mock(JournalCollectorInterface::class);
        $accountRepos = $this->mock(AccountRepositoryInterface::class);
@ -46,13 +134,56 @@ class ReportControllerTest extends TestCase
        $budget       = factory(Budget::class)->make();
        $account      = factory(Account::class)->make();

+        $budgetRepos->shouldReceive('find')->andReturn($budget)->once()->withArgs([1]);
+        $accountRepos->shouldReceive('find')->andReturn($account)->once()->withArgs([1]);
+        $collector->shouldReceive('setAccounts')->andReturnSelf()->once();
+        $collector->shouldReceive('setRange')->once()->andReturnSelf();
+        $collector->shouldReceive('setBudget')->once()->andReturnSelf();
+        $collector->shouldReceive('getJournals')->once()->andReturn(new Collection);
+
+        $this->be($this->user());
+        $arguments = [
+            'attributes' => [
+                'location'   => 'balance-amount',
+                'startDate'  => Carbon::now()->startOfMonth()->format('Ymd'),
+                'endDate'    => Carbon::now()->endOfMonth()->format('Ymd'),
+                'accounts'   => 1,
+                'accountId'  => 1,
+                'categoryId' => 1,
+                'budgetId'   => 1,
+                'role'       => 1, // ROLE_DEFAULTROLE
+            ],
+        ];
+        $uri       = route('popup.general') . '?' . http_build_query($arguments);
+        $response  = $this->get($uri);
+        $response->assertStatus(200);
+    }
+
+    /**
+     * @covers \FireflyIII\Http\Controllers\Popup\ReportController::general
+     * @covers \FireflyIII\Http\Controllers\Popup\ReportController::parseAttributes
+     * @covers \FireflyIII\Http\Controllers\Popup\ReportController::balanceAmount
+     */
+    public function testBalanceAmountDiffRole()
+    {
+        $collector    = $this->mock(JournalCollectorInterface::class);
+        $accountRepos = $this->mock(AccountRepositoryInterface::class);
+        $budgetRepos  = $this->mock(BudgetRepositoryInterface::class);
+        $budget       = factory(Budget::class)->make();
+        $account      = factory(Account::class)->make();
+        $one          = factory(Transaction::class)->make();
+        $two          = factory(Transaction::class)->make();
+        $tag         = factory(Tag::class)->make();
+        $tag->tagMode = 'balancingAct';
+        $two->transactionJournal->tags()->save($tag);
+
        $budgetRepos->shouldReceive('find')->andReturn($budget)->once()->withArgs([1]);
        $accountRepos->shouldReceive('find')->andReturn($account)->once()->withArgs([1]);
        $collector->shouldReceive('setAccounts')->andReturnSelf()->once();
        $collector->shouldReceive('setTypes')->withArgs([[TransactionType::WITHDRAWAL]])->andReturnSelf();
        $collector->shouldReceive('setRange')->once()->andReturnSelf();
        $collector->shouldReceive('withoutBudget')->once()->andReturnSelf();
-        $collector->shouldReceive('getJournals')->once()->andReturn(new Collection);
+        $collector->shouldReceive('getJournals')->once()->andReturn(new Collection([$one, $two]));

        $this->be($this->user());
        $arguments = [
@ -209,5 +340,29 @@ class ReportControllerTest extends TestCase
        $response->assertStatus(200);
    }

+    /**
+     * @covers                   \FireflyIII\Http\Controllers\Popup\ReportController::general
+     * @covers                   \FireflyIII\Http\Controllers\Popup\ReportController::parseAttributes
+     * @expectedExceptionMessage Firefly cannot handle
+     */
+    public function testWrongLocation()
+    {
+        $this->be($this->user());
+        $arguments = [
+            'attributes' => [
+                'location'   => 'bla-bla',
+                'startDate'  => Carbon::now()->startOfMonth()->format('Ymd'),
+                'endDate'    => Carbon::now()->endOfMonth()->format('Ymd'),
+                'accounts'   => 1,
+                'accountId'  => 1,
+                'categoryId' => 1,
+                'budgetId'   => 1,
+            ],
+        ];
+        $uri       = route('popup.general') . '?' . http_build_query($arguments);
+        $response  = $this->get($uri);
+        $response->assertStatus(500);
+    }
+

 }