From bb251328655617840c79e65693b2a1fd03929e6d Mon Sep 17 00:00:00 2001 From: James Cole Date: Fri, 27 Apr 2018 11:29:09 +0200 Subject: [PATCH] Fix tests --- app/Api/V1/Requests/BillRequest.php | 1 - app/Http/Controllers/RuleController.php | 4 +- .../Transaction/MassController.php | 7 +-- .../Internal/Update/AccountUpdateService.php | 2 +- app/Transformers/AccountTransformer.php | 2 +- bootstrap/app.php | 30 ++++++------ database/factories/ModelFactory.php | 4 +- phpunit.coverage.specific.xml | 3 ++ phpunit.coverage.xml | 3 ++ phpunit.xml | 3 ++ .../Api/V1/Controllers/BillControllerTest.php | 4 +- .../V1/Controllers/CurrencyControllerTest.php | 14 ++++-- .../Controllers/TransactionControllerTest.php | 4 +- .../Controllers/AccountControllerTest.php | 2 + .../Controllers/PiggyBankControllerTest.php | 6 ++- .../Transaction/BulkControllerTest.php | 49 +++---------------- .../Transaction/MassControllerTest.php | 49 +++---------------- .../Transaction/SingleControllerTest.php | 2 + .../Triggers/HasAnyCategoryTest.php | 2 + .../Triggers/ToAccountEndsTest.php | 8 ++- 20 files changed, 81 insertions(+), 118 deletions(-) diff --git a/app/Api/V1/Requests/BillRequest.php b/app/Api/V1/Requests/BillRequest.php index 64f64127d6..f2e13aaf9f 100644 --- a/app/Api/V1/Requests/BillRequest.php +++ b/app/Api/V1/Requests/BillRequest.php @@ -87,7 +87,6 @@ class BillRequest extends Request case 'PATCH': $bill = $this->route()->parameter('bill'); $rules['name'] .= ',' . $bill->id; - $rules['match'] .= ',' . $bill->id; break; } diff --git a/app/Http/Controllers/RuleController.php b/app/Http/Controllers/RuleController.php index 05cafcd305..03bffefe91 100644 --- a/app/Http/Controllers/RuleController.php +++ b/app/Http/Controllers/RuleController.php @@ -303,7 +303,7 @@ class RuleController extends Controller { $ids = $request->get('actions'); if (\is_array($ids)) { - $this->ruleGroupRepos->reorderRuleActions($rule, $ids); + $this->ruleRepos->reorderRuleActions($rule, $ids); } return response()->json('true'); @@ -320,7 +320,7 @@ class RuleController extends Controller { $ids = $request->get('triggers'); if (\is_array($ids)) { - $this->ruleGroupRepos->reorderRuleTriggers($rule, $ids); + $this->ruleRepos->reorderRuleTriggers($rule, $ids); } return response()->json('true'); diff --git a/app/Http/Controllers/Transaction/MassController.php b/app/Http/Controllers/Transaction/MassController.php index 7b9e5d6744..ede2e9b6ba 100644 --- a/app/Http/Controllers/Transaction/MassController.php +++ b/app/Http/Controllers/Transaction/MassController.php @@ -39,6 +39,7 @@ use Illuminate\Support\Collection; use Preferences; use Symfony\Component\HttpFoundation\ParameterBag; use View; +use Illuminate\View\View as IlluminateView; /** * Class MassController. @@ -69,9 +70,9 @@ class MassController extends Controller /** * @param Collection $journals * - * @return View + * @return IlluminateView */ - public function delete(Collection $journals): View + public function delete(Collection $journals): IlluminateView { $subTitle = trans('firefly.mass_delete_journals'); @@ -121,7 +122,7 @@ class MassController extends Controller * * @return View */ - public function edit(Collection $journals) + public function edit(Collection $journals): IlluminateView { $subTitle = trans('firefly.mass_edit_journals'); diff --git a/app/Services/Internal/Update/AccountUpdateService.php b/app/Services/Internal/Update/AccountUpdateService.php index e286d702d1..b66110e0d3 100644 --- a/app/Services/Internal/Update/AccountUpdateService.php +++ b/app/Services/Internal/Update/AccountUpdateService.php @@ -51,7 +51,7 @@ class AccountUpdateService $account->iban = $data['iban']; $account->save(); - if($data['currency_id'] === 0) { + if(isset($data['currency_id']) && $data['currency_id'] === 0) { unset($data['currency_id']); } diff --git a/app/Transformers/AccountTransformer.php b/app/Transformers/AccountTransformer.php index c238a28dd8..099398fd8c 100644 --- a/app/Transformers/AccountTransformer.php +++ b/app/Transformers/AccountTransformer.php @@ -153,7 +153,7 @@ class AccountTransformer extends TransformerAbstract } $currencyId = (int)$this->repository->getMetaValue($account, 'currency_id'); $currencyCode = null; - $currencySymbol = null; + $currencySymbol = 'x'; $decimalPlaces = 2; if ($currencyId > 0) { $currency = TransactionCurrency::find($currencyId); diff --git a/bootstrap/app.php b/bootstrap/app.php index 1eb38a8791..0abe08a8d8 100644 --- a/bootstrap/app.php +++ b/bootstrap/app.php @@ -36,21 +36,23 @@ declare(strict_types=1); bcscale(12); -/** - * @param string $key - * @param null $default - * - * @return mixed|null - */ -function envNonEmpty(string $key, $default = null) -{ - $result = env($key, $default); - if (is_string($result) && $result === '') { - $result = $default; - } - return $result; -} +if (!function_exists('envNonEmpty')) { + /** + * @param string $key + * @param null $default + * + * @return mixed|null + */ + function envNonEmpty(string $key, $default = null) + { + $result = env($key, $default); + if (is_string($result) && $result === '') { + $result = $default; + } + return $result; + } +} $app = new Illuminate\Foundation\Application( realpath(__DIR__ . '/../') diff --git a/database/factories/ModelFactory.php b/database/factories/ModelFactory.php index e37bd22338..04a90e6a2a 100644 --- a/database/factories/ModelFactory.php +++ b/database/factories/ModelFactory.php @@ -152,10 +152,12 @@ $factory->define( function (Faker\Generator $faker) { return [ 'id' => $faker->unique()->numberBetween(100, 10000), + 'created_at' => new Carbon, + 'updated_at' => new Carbon, 'account_id' => $faker->numberBetween(1, 10), 'name' => $faker->words(3, true), 'target_amount' => '1000.00', - 'startdate' => '2017-01-01', + 'startdate' => new Carbon('2017-01-01'), 'order' => 1, 'active' => 1, 'encrypted' => 0, diff --git a/phpunit.coverage.specific.xml b/phpunit.coverage.specific.xml index 9a25379977..da49035fc3 100644 --- a/phpunit.coverage.specific.xml +++ b/phpunit.coverage.specific.xml @@ -8,6 +8,9 @@ convertWarningsToExceptions="true" processIsolation="false" stopOnFailure="true"> + + + ./tests/Feature diff --git a/phpunit.coverage.xml b/phpunit.coverage.xml index 874f28e793..cfca4f71ee 100644 --- a/phpunit.coverage.xml +++ b/phpunit.coverage.xml @@ -8,6 +8,9 @@ convertWarningsToExceptions="true" processIsolation="false" stopOnFailure="true"> + + + ./tests/Feature diff --git a/phpunit.xml b/phpunit.xml index 1b5e9f6f27..8126efc85c 100644 --- a/phpunit.xml +++ b/phpunit.xml @@ -8,6 +8,9 @@ convertWarningsToExceptions="true" processIsolation="false" stopOnFailure="true"> + + + ./tests/Feature diff --git a/tests/Api/V1/Controllers/BillControllerTest.php b/tests/Api/V1/Controllers/BillControllerTest.php index 22a4b6cdf0..29c61739db 100644 --- a/tests/Api/V1/Controllers/BillControllerTest.php +++ b/tests/Api/V1/Controllers/BillControllerTest.php @@ -144,6 +144,7 @@ class BillControllerTest extends TestCase 'amount_min' => '66.34', 'amount_max' => '45.67', 'date' => '2018-01-01', + 'currency_id' => 1, 'repeat_freq' => 'monthly', 'skip' => 0, 'automatch' => 1, @@ -192,6 +193,7 @@ class BillControllerTest extends TestCase 'skip' => 0, 'automatch' => 1, 'active' => 1, + 'currency_id' => 1, ]; @@ -230,7 +232,7 @@ class BillControllerTest extends TestCase 'skip' => 0, 'automatch' => 1, 'active' => 1, - + 'currency_id' => 1, ]; // test API diff --git a/tests/Api/V1/Controllers/CurrencyControllerTest.php b/tests/Api/V1/Controllers/CurrencyControllerTest.php index 70c18e13a6..551a598d35 100644 --- a/tests/Api/V1/Controllers/CurrencyControllerTest.php +++ b/tests/Api/V1/Controllers/CurrencyControllerTest.php @@ -97,9 +97,17 @@ class CurrencyControllerTest extends TestCase $response = $this->get('/api/v1/currencies'); $response->assertStatus(200); $response->assertJson(['data' => [],]); - $response->assertJson( - ['meta' => ['pagination' => ['total' => $collection->count(), 'count' => $collection->count(), 'per_page' => 50, 'current_page' => 1, - 'total_pages' => 1]],] + $response->assertJson([ + 'meta' => [ + 'pagination' => [ + 'total' => $collection->count(), + 'count' => $collection->count(), + 'per_page' => 100, + 'current_page' => 1, + 'total_pages' => 1, + ], + ], + ] ); $response->assertJson( ['links' => ['self' => true, 'first' => true, 'last' => true,],] diff --git a/tests/Api/V1/Controllers/TransactionControllerTest.php b/tests/Api/V1/Controllers/TransactionControllerTest.php index f03ccf2fb1..8ecf77beb5 100644 --- a/tests/Api/V1/Controllers/TransactionControllerTest.php +++ b/tests/Api/V1/Controllers/TransactionControllerTest.php @@ -341,7 +341,7 @@ class TransactionControllerTest extends TestCase $journalRepos->shouldReceive('setUser')->once(); $accountRepos->shouldReceive('setUser'); $accountRepos->shouldReceive('getAccountsById')->andReturn(new Collection); - $accountRepos->shouldReceive('findByNameNull')->andReturn(null); + $accountRepos->shouldReceive('findByName')->andReturn(null); $data = [ 'description' => 'Some transaction #' . random_int(1, 1000), @@ -1661,7 +1661,7 @@ class TransactionControllerTest extends TestCase $journalRepos->shouldReceive('setUser')->once(); $accountRepos->shouldReceive('setUser'); $accountRepos->shouldReceive('getAccountsById')->andReturn(new Collection); - $accountRepos->shouldReceive('findByNameNull')->andReturn($account); + $accountRepos->shouldReceive('findByName')->andReturn($account); $journalRepos->shouldReceive('store')->andReturn($journal)->once(); diff --git a/tests/Feature/Controllers/AccountControllerTest.php b/tests/Feature/Controllers/AccountControllerTest.php index 4070de576b..c5784c680f 100644 --- a/tests/Feature/Controllers/AccountControllerTest.php +++ b/tests/Feature/Controllers/AccountControllerTest.php @@ -182,6 +182,8 @@ class AccountControllerTest extends TestCase Steam::shouldReceive('balancesByAccounts')->andReturn([$account->id => '100']); Steam::shouldReceive('getLastActivities')->andReturn([]); + $repository->shouldReceive('getMetaValue')->withArgs([Mockery::any(),'accountNumber'])->andReturn('123'); + $this->be($this->user()); $this->changeDateRange($this->user(), $range); $response = $this->get(route('accounts.index', ['asset'])); diff --git a/tests/Feature/Controllers/PiggyBankControllerTest.php b/tests/Feature/Controllers/PiggyBankControllerTest.php index bd78799224..81891d6d98 100644 --- a/tests/Feature/Controllers/PiggyBankControllerTest.php +++ b/tests/Feature/Controllers/PiggyBankControllerTest.php @@ -66,6 +66,8 @@ class PiggyBankControllerTest extends TestCase $journalRepos = $this->mock(JournalRepositoryInterface::class); $journalRepos->shouldReceive('first')->once()->andReturn(new TransactionJournal); $piggyRepos->shouldReceive('getCurrentAmount')->andReturn('0'); + $piggyRepos->shouldReceive('leftOnAccount')->andReturn('0'); + $this->be($this->user()); $response = $this->get(route('piggy-banks.add', [1])); $response->assertStatus(200); @@ -81,6 +83,7 @@ class PiggyBankControllerTest extends TestCase $journalRepos = $this->mock(JournalRepositoryInterface::class); $journalRepos->shouldReceive('first')->once()->andReturn(new TransactionJournal); $piggyRepos->shouldReceive('getCurrentAmount')->andReturn('0'); + $piggyRepos->shouldReceive('leftOnAccount')->andReturn('0'); $this->be($this->user()); $response = $this->get(route('piggy-banks.add-money-mobile', [1])); @@ -200,8 +203,9 @@ class PiggyBankControllerTest extends TestCase $journalRepos->shouldReceive('first')->once()->andReturn(new TransactionJournal); $repository->shouldReceive('getPiggyBanks')->andReturn(new Collection([$one, $two])); $repository->shouldReceive('getCurrentAmount')->andReturn('10'); + $repository->shouldReceive('setUser'); - Steam::shouldReceive('balanceIgnoreVirtual')->twice()->andReturn('1'); + Steam::shouldReceive('balance')->twice()->andReturn('1'); $this->be($this->user()); $response = $this->get(route('piggy-banks.index')); diff --git a/tests/Feature/Controllers/Transaction/BulkControllerTest.php b/tests/Feature/Controllers/Transaction/BulkControllerTest.php index 056d284552..009e0f8946 100644 --- a/tests/Feature/Controllers/Transaction/BulkControllerTest.php +++ b/tests/Feature/Controllers/Transaction/BulkControllerTest.php @@ -86,16 +86,16 @@ class BulkControllerTest extends TestCase $budgetRepos->shouldReceive('getActiveBudgets')->andReturn(new Collection); $journalRepos->shouldReceive('first')->andReturn(new TransactionJournal); $journalRepos->shouldReceive('getJournalSourceAccounts') - ->andReturn(new Collection([1, 2, 3]), new Collection, new Collection, new Collection); + ->andReturn(new Collection([1, 2, 3]), new Collection, new Collection, new Collection, new Collection([1])); $journalRepos->shouldReceive('getJournalDestinationAccounts') - ->andReturn(new Collection, new Collection([1, 2, 3]), new Collection, new Collection); + ->andReturn(new Collection, new Collection([1, 2, 3]), new Collection, new Collection, new Collection([1])); $journalRepos->shouldReceive('getTransactionType') - ->andReturn('Withdrawal', 'Opening balance'); + ->andReturn('Withdrawal', 'Opening balance', 'Withdrawal', 'Withdrawal', 'Withdrawal'); $journalRepos->shouldReceive('isJournalReconciled') - ->andReturn(true, false); + ->andReturn(true, false, false, false, false); // default transactions - $collection = $this->user()->transactionJournals()->take(4)->get(); + $collection = $this->user()->transactionJournals()->take(5)->get(); $allIds = $collection->pluck('id')->toArray(); $route = route('transactions.bulk.edit', implode(',', $allIds)); $this->be($this->user()); @@ -110,43 +110,6 @@ class BulkControllerTest extends TestCase $response->assertSee('multiple destination accounts'); } - /** - * @covers \FireflyIII\Http\Controllers\Transaction\BulkController::edit - */ - public function testEditMultipleNothingLeft() - { - // mock stuff: - $journalRepos = $this->mock(JournalRepositoryInterface::class); - $budgetRepos = $this->mock(BudgetRepositoryInterface::class); - $budgetRepos->shouldReceive('getActiveBudgets')->andReturn(new Collection); - $journalRepos->shouldReceive('first')->andReturn(new TransactionJournal); - $journalRepos->shouldReceive('getJournalSourceAccounts') - ->andReturn(new Collection([1, 2, 3]), new Collection, new Collection, new Collection); - $journalRepos->shouldReceive('getJournalDestinationAccounts') - ->andReturn(new Collection, new Collection([1, 2, 3]), new Collection, new Collection); - $journalRepos->shouldReceive('getTransactionType') - ->andReturn('Withdrawal', 'Opening balance'); - $journalRepos->shouldReceive('isJournalReconciled') - ->andReturn(true, true); - - - // default transactions - $collection = $this->user()->transactionJournals()->take(4)->get(); - $allIds = $collection->pluck('id')->toArray(); - $route = route('transactions.bulk.edit', implode(',', $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 * @covers \FireflyIII\Http\Requests\BulkEditJournalRequest @@ -166,7 +129,7 @@ class BulkControllerTest extends TestCase $repository = $this->mock(JournalRepositoryInterface::class); $repository->shouldReceive('first')->once()->andReturn(new TransactionJournal); - $repository->shouldReceive('find')->times(4)->andReturn(new TransactionJournal); + $repository->shouldReceive('findNull')->times(4)->andReturn(new TransactionJournal); $repository->shouldReceive('updateCategory')->times(4)->andReturn(new TransactionJournal()) ->withArgs([Mockery::any(), $data['category']]); diff --git a/tests/Feature/Controllers/Transaction/MassControllerTest.php b/tests/Feature/Controllers/Transaction/MassControllerTest.php index 260f710d9c..04657b75e2 100644 --- a/tests/Feature/Controllers/Transaction/MassControllerTest.php +++ b/tests/Feature/Controllers/Transaction/MassControllerTest.php @@ -81,7 +81,7 @@ class MassControllerTest extends TestCase // mock deletion: $repository = $this->mock(JournalRepositoryInterface::class); $repository->shouldReceive('first')->once()->andReturn(new TransactionJournal); - $repository->shouldReceive('find')->andReturnValues([$deposits[0], $deposits[1]])->times(2); + $repository->shouldReceive('findNull')->andReturnValues([$deposits[0], $deposits[1]])->times(2); $repository->shouldReceive('destroy')->times(2); $this->session(['transactions.mass-delete.uri' => 'http://localhost']); @@ -150,17 +150,17 @@ class MassControllerTest extends TestCase $journalRepos->shouldReceive('first')->andReturn(new TransactionJournal); $journalRepos->shouldReceive('getJournalSourceAccounts') - ->andReturn(new Collection([1, 2, 3]), new Collection, new Collection, new Collection); + ->andReturn(new Collection([1, 2, 3]), new Collection, new Collection, new Collection, new Collection([1])); $journalRepos->shouldReceive('getJournalDestinationAccounts') - ->andReturn(new Collection, new Collection([1, 2, 3]), new Collection, new Collection); + ->andReturn(new Collection, new Collection([1, 2, 3]), new Collection, new Collection, new Collection([1])); $journalRepos->shouldReceive('getTransactionType') - ->andReturn('Withdrawal', 'Opening balance'); + ->andReturn('Withdrawal', 'Opening balance', 'Withdrawal', 'Withdrawal', 'Withdrawal'); $journalRepos->shouldReceive('isJournalReconciled') - ->andReturn(true, false); + ->andReturn(true, false, false, false, false); // default transactions - $collection = $this->user()->transactionJournals()->take(4)->get(); + $collection = $this->user()->transactionJournals()->take(5)->get(); $allIds = $collection->pluck('id')->toArray(); $route = route('transactions.mass.edit', implode(',', $allIds)); $this->be($this->user()); @@ -174,43 +174,6 @@ class MassControllerTest extends TestCase $response->assertSee('multiple destination accounts'); } - /** - * @covers \FireflyIII\Http\Controllers\Transaction\MassController::edit - */ - public function testEditMultipleNothingLeft() - { - $budgetRepos = $this->mock(BudgetRepositoryInterface::class); - $budgetRepos->shouldReceive('getBudgets')->andReturn(new Collection); - - $journalRepos = $this->mock(JournalRepositoryInterface::class); - $journalRepos->shouldReceive('first')->once()->andReturn(new TransactionJournal); - - - // mock stuff: - $repository = $this->mock(AccountRepositoryInterface::class); - $repository->shouldReceive('getAccountsByType')->once()->withArgs([[AccountType::DEFAULT, AccountType::ASSET]])->andReturn(new Collection); - - $journalRepos->shouldReceive('getJournalSourceAccounts') - ->andReturn(new Collection([1, 2, 3]), new Collection, new Collection, new Collection); - $journalRepos->shouldReceive('getJournalDestinationAccounts') - ->andReturn(new Collection, new Collection([1, 2, 3]), new Collection, new Collection); - $journalRepos->shouldReceive('getTransactionType') - ->andReturn('Withdrawal', 'Opening balance'); - $journalRepos->shouldReceive('isJournalReconciled') - ->andReturn(true, true); - - // default transactions - $collection = $this->user()->transactionJournals()->take(4)->get(); - $allIds = $collection->pluck('id')->toArray(); - - $this->be($this->user()); - $response = $this->get(route('transactions.mass.edit', implode(',', $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('
      '); - } /** * @covers \FireflyIII\Http\Controllers\Transaction\MassController::update diff --git a/tests/Feature/Controllers/Transaction/SingleControllerTest.php b/tests/Feature/Controllers/Transaction/SingleControllerTest.php index c03cf27e5d..f856dab272 100644 --- a/tests/Feature/Controllers/Transaction/SingleControllerTest.php +++ b/tests/Feature/Controllers/Transaction/SingleControllerTest.php @@ -209,6 +209,8 @@ class SingleControllerTest extends TestCase $journalRepos->shouldReceive('first')->andReturn(new TransactionJournal); $journalRepos->shouldReceive('destroy')->once(); + $journalRepos->shouldReceive('getTransactionType')->once()->andReturn('Withdrawal'); + $this->session(['transactions.delete.uri' => 'http://localhost']); $this->be($this->user()); diff --git a/tests/Unit/TransactionRules/Triggers/HasAnyCategoryTest.php b/tests/Unit/TransactionRules/Triggers/HasAnyCategoryTest.php index a902217e79..d1b0081bd2 100644 --- a/tests/Unit/TransactionRules/Triggers/HasAnyCategoryTest.php +++ b/tests/Unit/TransactionRules/Triggers/HasAnyCategoryTest.php @@ -73,6 +73,7 @@ class HasAnyCategoryTest extends TestCase */ public function testTriggeredTransactions() { + /** @var TransactionJournal $journal */ $journal = TransactionJournal::inRandomOrder()->whereNull('deleted_at')->first(); $category = $journal->user->categories()->first(); $journal->categories()->detach(); @@ -81,6 +82,7 @@ class HasAnyCategoryTest extends TestCase // append to transaction, not to journal. foreach ($journal->transactions()->get() as $index => $transaction) { $transaction->categories()->sync([$category->id]); + $this->assertEquals(1, $transaction->categories()->count()); } $this->assertEquals(0, $journal->categories()->count()); diff --git a/tests/Unit/TransactionRules/Triggers/ToAccountEndsTest.php b/tests/Unit/TransactionRules/Triggers/ToAccountEndsTest.php index 0f8666c2a9..87738847d7 100644 --- a/tests/Unit/TransactionRules/Triggers/ToAccountEndsTest.php +++ b/tests/Unit/TransactionRules/Triggers/ToAccountEndsTest.php @@ -72,9 +72,13 @@ class ToAccountEndsTest extends TestCase */ public function testTriggeredNot() { - $journal = TransactionJournal::inRandomOrder()->whereNull('deleted_at')->first(); + $count = 0; + while ($count === 0) { + $journal = TransactionJournal::inRandomOrder()->whereNull('deleted_at')->first(); + $count = $journal->transactions()->where('amount', '>', 0)->count(); + } - $trigger = ToAccountEnds::makeFromStrings((string)random_int(1, 234), false); + $trigger = ToAccountEnds::makeFromStrings((string)random_int(1, 1234), false); $result = $trigger->triggered($journal); $this->assertFalse($result); }