From b73884160acfe98f490a7ae30613277fc92272e9 Mon Sep 17 00:00:00 2001 From: James Cole Date: Sun, 31 Dec 2017 10:40:27 +0100 Subject: [PATCH] Fix account controller + coverage. --- app/Http/Controllers/AccountController.php | 14 +- .../Account/AccountRepository.php | 10 + .../Account/AccountRepositoryInterface.php | 7 + resources/views/transactions/bulk/edit.twig | 24 +- .../Controllers/AccountControllerTest.php | 6 + .../Transaction/BulkControllerTest.php | 217 ++++++++++++++++++ .../Transaction/MassControllerTest.php | 4 + 7 files changed, 254 insertions(+), 28 deletions(-) create mode 100644 tests/Feature/Controllers/Transaction/BulkControllerTest.php diff --git a/app/Http/Controllers/AccountController.php b/app/Http/Controllers/AccountController.php index bb9d6d0b04..8005640860 100644 --- a/app/Http/Controllers/AccountController.php +++ b/app/Http/Controllers/AccountController.php @@ -157,14 +157,14 @@ class AccountController extends Controller * @throws FireflyException * @throws FireflyException */ - public function edit(Request $request, Account $account) + public function edit(Request $request, Account $account, AccountRepositoryInterface $repository) { - /** @var CurrencyRepositoryInterface $repository */ - $repository = app(CurrencyRepositoryInterface::class); + /** @var CurrencyRepositoryInterface $currencyRepos */ + $currencyRepos = app(CurrencyRepositoryInterface::class); $what = config('firefly.shortNamesByFullName')[$account->accountType->type]; $subTitle = trans('firefly.edit_' . $what . '_account', ['name' => $account->name]); $subTitleIcon = config('firefly.subIconsByIdentifier.' . $what); - $allCurrencies = $repository->get(); + $allCurrencies = $currencyRepos->get(); $currencySelectList = ExpandedForm::makeSelectList($allCurrencies); $roles = []; foreach (config('firefly.accountRoles') as $role) { @@ -184,7 +184,7 @@ class AccountController extends Controller $openingBalanceAmount = '0' === $account->getOpeningBalanceAmount() ? '' : $openingBalanceAmount; $openingBalanceDate = $account->getOpeningBalanceDate(); $openingBalanceDate = 1900 === $openingBalanceDate->year ? null : $openingBalanceDate->format('Y-m-d'); - $currency = $repository->find(intval($account->getMeta('currency_id'))); + $currency = $currencyRepos->find(intval($account->getMeta('currency_id'))); $preFilled = [ 'accountNumber' => $account->getMeta('accountNumber'), @@ -196,10 +196,10 @@ class AccountController extends Controller 'openingBalance' => $openingBalanceAmount, 'virtualBalance' => $account->virtual_balance, 'currency_id' => $currency->id, - 'notes' => '', + 'notes' => '', ]; /** @var Note $note */ - $note = $account->notes()->first(); + $note = $repository->getNote($account); if (null !== $note) { $preFilled['notes'] = $note->text; } diff --git a/app/Repositories/Account/AccountRepository.php b/app/Repositories/Account/AccountRepository.php index 291166d511..3a6d0155bb 100644 --- a/app/Repositories/Account/AccountRepository.php +++ b/app/Repositories/Account/AccountRepository.php @@ -87,6 +87,16 @@ class AccountRepository implements AccountRepositoryInterface return true; } + /** + * @param Account $account + * + * @return Note|null + */ + public function getNote(Account $account): ?Note + { + return $account->notes()->first(); + } + /** * Returns the date of the very last transaction in this account. * diff --git a/app/Repositories/Account/AccountRepositoryInterface.php b/app/Repositories/Account/AccountRepositoryInterface.php index 985b32104c..145c7aeeb0 100644 --- a/app/Repositories/Account/AccountRepositoryInterface.php +++ b/app/Repositories/Account/AccountRepositoryInterface.php @@ -24,6 +24,7 @@ namespace FireflyIII\Repositories\Account; use Carbon\Carbon; use FireflyIII\Models\Account; +use FireflyIII\Models\Note; use FireflyIII\Models\TransactionJournal; use FireflyIII\User; use Illuminate\Support\Collection; @@ -33,6 +34,12 @@ use Illuminate\Support\Collection; */ interface AccountRepositoryInterface { + /** + * @param Account $account + * + * @return Note|null + */ + public function getNote(Account $account): ?Note; /** * Moved here from account CRUD. * diff --git a/resources/views/transactions/bulk/edit.twig b/resources/views/transactions/bulk/edit.twig index 5c7c3d0cc8..4957e9eeb8 100644 --- a/resources/views/transactions/bulk/edit.twig +++ b/resources/views/transactions/bulk/edit.twig @@ -79,7 +79,7 @@
@@ -97,7 +97,7 @@
@@ -111,7 +111,7 @@
@@ -121,24 +121,6 @@ -
{% if journals.count > 0 %} diff --git a/tests/Feature/Controllers/AccountControllerTest.php b/tests/Feature/Controllers/AccountControllerTest.php index ea3e97106e..058918f408 100644 --- a/tests/Feature/Controllers/AccountControllerTest.php +++ b/tests/Feature/Controllers/AccountControllerTest.php @@ -26,6 +26,7 @@ use Carbon\Carbon; use FireflyIII\Helpers\Collector\JournalCollectorInterface; use FireflyIII\Models\Account; use FireflyIII\Models\AccountType; +use FireflyIII\Models\Note; use FireflyIII\Models\Transaction; use FireflyIII\Models\TransactionCurrency; use FireflyIII\Models\TransactionJournal; @@ -114,12 +115,16 @@ class AccountControllerTest extends TestCase */ public function testEdit() { + $note = new Note(); + $note->text = 'This is a test'; // mock stuff $journalRepos = $this->mock(JournalRepositoryInterface::class); $repository = $this->mock(CurrencyRepositoryInterface::class); + $accountRepos = $this->mock(AccountRepositoryInterface::class); $repository->shouldReceive('get')->andReturn(new Collection); $journalRepos->shouldReceive('first')->once()->andReturn(new TransactionJournal); $repository->shouldReceive('find')->once()->andReturn(new TransactionCurrency()); + $accountRepos->shouldReceive('getNote')->andReturn($note)->once(); $this->be($this->user()); $account = $this->user()->accounts()->where('account_type_id', 3)->whereNull('deleted_at')->first(); @@ -127,6 +132,7 @@ class AccountControllerTest extends TestCase $response->assertStatus(200); // has bread crumb $response->assertSee('
    '); + $response->assertSee($note->text); } /** diff --git a/tests/Feature/Controllers/Transaction/BulkControllerTest.php b/tests/Feature/Controllers/Transaction/BulkControllerTest.php new file mode 100644 index 0000000000..50ae40119c --- /dev/null +++ b/tests/Feature/Controllers/Transaction/BulkControllerTest.php @@ -0,0 +1,217 @@ +. + */ +declare(strict_types=1); + +namespace Tests\Feature\Controllers\Transaction; + +use DB; +use FireflyIII\Models\TransactionJournal; +use FireflyIII\Repositories\Budget\BudgetRepositoryInterface; +use FireflyIII\Repositories\Journal\JournalRepositoryInterface; +use Illuminate\Support\Collection; +use Mockery; +use Tests\TestCase; + +/** + * Class BulkControllerTest + * + * @SuppressWarnings(PHPMD.TooManyPublicMethods) + * @SuppressWarnings(PHPMD.ExcessiveMethodLength) + * @SuppressWarnings(PHPMD.CouplingBetweenObjects) + */ +class BulkControllerTest extends TestCase +{ + /** + * @covers \FireflyIII\Http\Controllers\Transaction\BulkController::edit + * @covers \FireflyIII\Http\Controllers\Transaction\BulkController::__construct + */ + public function testEdit() + { + // mock stuff: + $budgetRepos = $this->mock(BudgetRepositoryInterface::class); + $budgetRepos->shouldReceive('getActiveBudgets')->andReturn(new Collection); + + $transfers = TransactionJournal::where('transaction_type_id', 3)->where('user_id', $this->user()->id)->take(4)->get()->pluck('id')->toArray(); + + $this->be($this->user()); + $response = $this->get(route('transactions.bulk.edit', $transfers)); + $response->assertStatus(200); + $response->assertSee('Bulk edit a number of transactions'); + // has bread crumb + $response->assertSee('
      '); + } + + /** + * @covers \FireflyIII\Http\Controllers\Transaction\BulkController::edit + */ + public function testEditMultiple() + { + // mock stuff: + $budgetRepos = $this->mock(BudgetRepositoryInterface::class); + $budgetRepos->shouldReceive('getActiveBudgets')->andReturn(new Collection); + + // default transactions + $collection = TransactionJournal::where('transaction_type_id', 3)->where('user_id', $this->user()->id)->take(2)->get(); + + // add deposit (with multiple sources) + $collection->push( + TransactionJournal::where('transaction_type_id', 2) + ->whereNull('transaction_journals.deleted_at') + ->leftJoin('transactions', 'transactions.transaction_journal_id', '=', 'transaction_journals.id') + ->groupBy('transaction_journals.id') + ->orderBy('ct', 'DESC') + ->where('user_id', $this->user()->id)->first(['transaction_journals.id', DB::raw('count(transactions.`id`) as ct')]) + ); + + // add withdrawal (with multiple destinations) + $collection->push( + TransactionJournal::where('transaction_type_id', 1) + ->whereNull('transaction_journals.deleted_at') + ->leftJoin('transactions', 'transactions.transaction_journal_id', '=', 'transaction_journals.id') + ->groupBy('transaction_journals.id') + ->orderBy('ct', 'DESC') + ->where('user_id', $this->user()->id)->first(['transaction_journals.id', DB::raw('count(transactions.`id`) as ct')]) + ); + + // add reconcile transaction + $collection->push( + TransactionJournal::where('transaction_type_id', 5) + ->whereNull('transaction_journals.deleted_at') + ->leftJoin('transactions', 'transactions.transaction_journal_id', '=', 'transaction_journals.id') + ->groupBy('transaction_journals.id') + ->orderBy('ct', 'DESC') + ->where('user_id', $this->user()->id)->first(['transaction_journals.id', DB::raw('count(transactions.`id`) as ct')]) + ); + + // add opening balance: + $collection->push(TransactionJournal::where('transaction_type_id', 4)->where('user_id', $this->user()->id)->first()); + $allIds = $collection->pluck('id')->toArray(); + $route = route('transactions.bulk.edit', join(',', $allIds)); + $this->be($this->user()); + $response = $this->get($route); + $response->assertStatus(200); + $response->assertSee('Bulk edit a number of transactions'); + $response->assertSessionHas('info'); + // has bread crumb + $response->assertSee('
        '); + $response->assertSee('marked as reconciled'); + $response->assertSee('multiple source accounts'); + $response->assertSee('multiple destination accounts'); + } + + /** + * @covers \FireflyIII\Http\Controllers\Transaction\BulkController::edit + */ + public function testEditMultipleNothingLeft() + { + // mock stuff: + $budgetRepos = $this->mock(BudgetRepositoryInterface::class); + $budgetRepos->shouldReceive('getActiveBudgets')->andReturn(new Collection); + + // default transactions + $collection = new Collection; + + // add deposit (with multiple sources) + $collection->push( + TransactionJournal::where('transaction_type_id', 2) + ->whereNull('transaction_journals.deleted_at') + ->leftJoin('transactions', 'transactions.transaction_journal_id', '=', 'transaction_journals.id') + ->groupBy('transaction_journals.id') + ->orderBy('ct', 'DESC') + ->where('user_id', $this->user()->id)->first(['transaction_journals.id', DB::raw('count(transactions.`id`) as ct')]) + ); + + // add withdrawal (with multiple destinations) + $collection->push( + TransactionJournal::where('transaction_type_id', 1) + ->whereNull('transaction_journals.deleted_at') + ->leftJoin('transactions', 'transactions.transaction_journal_id', '=', 'transaction_journals.id') + ->groupBy('transaction_journals.id') + ->orderBy('ct', 'DESC') + ->where('user_id', $this->user()->id)->first(['transaction_journals.id', DB::raw('count(transactions.`id`) as ct')]) + ); + + // add reconcile transaction + $collection->push( + TransactionJournal::where('transaction_type_id', 5) + ->whereNull('transaction_journals.deleted_at') + ->leftJoin('transactions', 'transactions.transaction_journal_id', '=', 'transaction_journals.id') + ->groupBy('transaction_journals.id') + ->orderBy('ct', 'DESC') + ->where('user_id', $this->user()->id)->first(['transaction_journals.id', DB::raw('count(transactions.`id`) as ct')]) + ); + + // add opening balance: + $collection->push(TransactionJournal::where('transaction_type_id', 4)->where('user_id', $this->user()->id)->first()); + $allIds = $collection->pluck('id')->toArray(); + $route = route('transactions.bulk.edit', join(',', $allIds)); + $this->be($this->user()); + $response = $this->get($route); + $response->assertStatus(200); + $response->assertSee('Bulk edit a number of transactions'); + $response->assertSessionHas('info'); + // has bread crumb + $response->assertSee('
          '); + $response->assertSessionHas('error', 'You have selected no valid transactions to edit.'); + $response->assertSee('marked as reconciled'); + $response->assertSee('multiple source accounts'); + $response->assertSee('multiple destination accounts'); + } + + /** + * @covers \FireflyIII\Http\Controllers\Transaction\BulkController::update + */ + public function testUpdate() + { + + $tags = ['a', 'b', 'c']; + $collection = TransactionJournal::where('transaction_type_id', 1)->where('user_id', $this->user()->id)->take(4)->get(); + $allIds = $collection->pluck('id')->toArray(); + + $data = [ + 'category' => 'Some new category', + 'budget_id' => 1, + 'tags' => 'a,b,c', + 'journals' => $allIds, + ]; + + $repository = $this->mock(JournalRepositoryInterface::class); + $repository->shouldReceive('first')->once()->andReturn(new TransactionJournal); + $repository->shouldReceive('find')->times(4)->andReturn(new TransactionJournal); + + $repository->shouldReceive('updateCategory')->times(4)->andReturn(new TransactionJournal()) + ->withArgs([Mockery::any(), $data['category']]); + + $repository->shouldReceive('updateBudget')->times(4)->andReturn(new TransactionJournal()) + ->withArgs([Mockery::any(), $data['budget_id']]); + + $repository->shouldReceive('updateTags')->times(4)->andReturn(true) + ->withArgs([Mockery::any(), $tags]); + + + + $route = route('transactions.bulk.update'); + $this->be($this->user()); + $response = $this->post($route, $data); + $response->assertStatus(302); + $response->assertSessionHas('success'); + } +} diff --git a/tests/Feature/Controllers/Transaction/MassControllerTest.php b/tests/Feature/Controllers/Transaction/MassControllerTest.php index b075dea02b..68077701d8 100644 --- a/tests/Feature/Controllers/Transaction/MassControllerTest.php +++ b/tests/Feature/Controllers/Transaction/MassControllerTest.php @@ -155,6 +155,9 @@ class MassControllerTest extends TestCase $response->assertSee('Edit a number of transactions'); // has bread crumb $response->assertSee('
            '); + $response->assertSee('marked as reconciled'); + $response->assertSee('multiple source accounts'); + $response->assertSee('multiple destination accounts'); } /** @@ -182,6 +185,7 @@ class MassControllerTest extends TestCase $response = $this->get(route('transactions.mass.edit', join(',', $allIds))); $response->assertStatus(200); $response->assertSee('Edit a number of transactions'); + $response->assertSessionHas('error','You have selected no valid transactions to edit.'); // has bread crumb $response->assertSee('
              '); }