From fea9bc4e7ebc9b4c0a2bd670db8db42c0962e769 Mon Sep 17 00:00:00 2001 From: James Cole Date: Fri, 5 Jun 2015 07:10:51 +0200 Subject: [PATCH] Increased coverage and fixed a nasty bug. --- app/Http/Controllers/Auth/AuthController.php | 1 - app/Http/Controllers/NewUserController.php | 20 +-- app/Models/Account.php | 2 +- app/Models/TransactionJournal.php | 4 +- .../Account/AccountRepository.php | 5 +- .../Category/CategoryRepository.php | 4 +- tests/factories/all.php | 20 ++- tests/repositories/AccountRepositoryTest.php | 120 ++++++++++-------- tests/repositories/CategoryRepositoryTest.php | 68 +++++----- tests/repositories/JournalRepositoryTest.php | 37 +++++- 10 files changed, 161 insertions(+), 120 deletions(-) diff --git a/app/Http/Controllers/Auth/AuthController.php b/app/Http/Controllers/Auth/AuthController.php index 7819d86383..4330203b5c 100644 --- a/app/Http/Controllers/Auth/AuthController.php +++ b/app/Http/Controllers/Auth/AuthController.php @@ -109,7 +109,6 @@ class AuthController extends Controller if (User::count() == 1) { $admin = Role::where('name', 'owner')->first(); $this->auth->user()->attachRole($admin); - // $this->auth->user()->roles()->save($admin); } diff --git a/app/Http/Controllers/NewUserController.php b/app/Http/Controllers/NewUserController.php index 569d19115b..8a14f74632 100644 --- a/app/Http/Controllers/NewUserController.php +++ b/app/Http/Controllers/NewUserController.php @@ -23,7 +23,7 @@ class NewUserController extends Controller /** * @param AccountRepositoryInterface $repository * - * @return \Illuminate\View\View + * @@return \Illuminate\Http\RedirectResponse|\Illuminate\View\View */ public function index(AccountRepositoryInterface $repository) { @@ -45,6 +45,8 @@ class NewUserController extends Controller /** * @param NewUserFormRequest $request * @param AccountRepositoryInterface $repository + * + * @return \Illuminate\Http\RedirectResponse */ public function submit(NewUserFormRequest $request, AccountRepositoryInterface $repository) { @@ -97,20 +99,8 @@ class NewUserController extends Controller $creditCard = $repository->store($creditAccount); // store meta for CC: - AccountMeta::create( - [ - 'name' => 'ccType', - 'data' => 'monthlyFull', - 'account_id' => $creditCard->id, - ] - ); - AccountMeta::create( - [ - 'name' => 'ccMonthlyPaymentDate', - 'data' => Carbon::now()->year . '-01-01', - 'account_id' => $creditCard->id, - ] - ); + AccountMeta::create(['name' => 'ccType', 'data' => 'monthlyFull', 'account_id' => $creditCard->id,]); + AccountMeta::create(['name' => 'ccMonthlyPaymentDate', 'data' => Carbon::now()->year . '-01-01', 'account_id' => $creditCard->id,]); } Session::flash('success', 'New account(s) created!'); diff --git a/app/Models/Account.php b/app/Models/Account.php index f5092a9c96..943ecfb862 100644 --- a/app/Models/Account.php +++ b/app/Models/Account.php @@ -46,7 +46,7 @@ use Watson\Validating\ValidatingTrait; * @property mixed lastActivityDate * @property mixed piggyBalance * @property mixed difference - * @propery mixed percentage + * @property mixed percentage */ class Account extends Model { diff --git a/app/Models/TransactionJournal.php b/app/Models/TransactionJournal.php index 8b00c9b9da..ae335b6ead 100644 --- a/app/Models/TransactionJournal.php +++ b/app/Models/TransactionJournal.php @@ -68,7 +68,9 @@ use Watson\Validating\ValidatingTrait; * @property mixed account_id * @property mixed name * @property mixed symbol - * @property-read mixed $correct_amount + * @property-read mixed $correct_amount + * @method static \FireflyIII\Models\TransactionJournal orderBy + * @method static \FireflyIII\Models\TransactionJournal|null first */ class TransactionJournal extends Model { diff --git a/app/Repositories/Account/AccountRepository.php b/app/Repositories/Account/AccountRepository.php index 23682dccd8..d472825600 100644 --- a/app/Repositories/Account/AccountRepository.php +++ b/app/Repositories/Account/AccountRepository.php @@ -373,8 +373,9 @@ class AccountRepository implements AccountRepositoryInterface */ public function openingBalanceTransaction(Account $account) { - return TransactionJournal::accountIs($account) - ->orderBy('transaction_journals.date', 'ASC') + return TransactionJournal + ::orderBy('transaction_journals.date', 'ASC') + ->accountIs($account) ->orderBy('created_at', 'ASC') ->first(['transaction_journals.*']); } diff --git a/app/Repositories/Category/CategoryRepository.php b/app/Repositories/Category/CategoryRepository.php index 1fbb964e42..19e633be6b 100644 --- a/app/Repositories/Category/CategoryRepository.php +++ b/app/Repositories/Category/CategoryRepository.php @@ -75,14 +75,14 @@ class CategoryRepository implements CategoryRepositoryInterface ->where('categories.user_id', Auth::user()->id) ->after($start) ->transactionTypes(['Withdrawal']) - ->groupBy('categories.id') ->get(['categories.id as category_id', 'categories.encrypted as category_encrypted', 'categories.name', 'transaction_journals.*']); $result = []; foreach ($set as $entry) { $categoryId = intval($entry->category_id); if (isset($result[$categoryId])) { - $result[$categoryId]['sum'] += floatval($entry->amount); + bcscale(2); + $result[$categoryId]['sum'] = bcadd($result[$categoryId]['sum'], $entry->amount); } else { $isEncrypted = intval($entry->category_encrypted) == 1 ? true : false; $name = strlen($entry->name) == 0 ? trans('firefly.no_category') : $entry->name; diff --git a/tests/factories/all.php b/tests/factories/all.php index f9c35f9047..6da5476d3d 100644 --- a/tests/factories/all.php +++ b/tests/factories/all.php @@ -9,6 +9,9 @@ if (!class_exists('RandomString')) { */ class RandomString { + public static $count = 0; + public static $set = []; + /** * @param int $length * @@ -18,11 +21,22 @@ if (!class_exists('RandomString')) { { $characters = '0123456789abcdefghijklmnopqrstuvwxyzABCDEFGHIJKLMNOPQRSTUVWXYZ'; $charactersLength = strlen($characters); - $randomString = ''; + + + $randomString = ''; for ($i = 0; $i < $length; $i++) { $randomString .= $characters[rand(0, $charactersLength - 1)]; } + while (in_array($randomString, self::$set)) { + // create another if its in the current $set: + $randomString = ''; + for ($i = 0; $i < $length; $i++) { + $randomString .= $characters[rand(0, $charactersLength - 1)]; + } + } + self::$set[] = $randomString; + return $randomString; } @@ -32,7 +46,7 @@ if (!class_exists('RandomString')) { FactoryMuffin::define( 'FireflyIII\Models\Role', [ - 'name' => 'word', + 'name' => 'word', ] ); @@ -201,7 +215,7 @@ FactoryMuffin::define( return RandomString::generateRandomString(3); }, 'symbol' => function () { - return RandomString::generateRandomString(2); + return RandomString::generateRandomString(8); }, 'name' => 'word' ] diff --git a/tests/repositories/AccountRepositoryTest.php b/tests/repositories/AccountRepositoryTest.php index d96994e8b0..c2bbaf16cf 100644 --- a/tests/repositories/AccountRepositoryTest.php +++ b/tests/repositories/AccountRepositoryTest.php @@ -3,7 +3,6 @@ use Carbon\Carbon; use FireflyIII\Models\Account; use FireflyIII\Models\AccountMeta; use FireflyIII\Models\Preference; -use FireflyIII\Models\Transaction; use FireflyIII\Repositories\Account\AccountRepository; use Illuminate\Pagination\LengthAwarePaginator; use League\FactoryMuffin\Facade as FactoryMuffin; @@ -113,9 +112,9 @@ class AccountRepositoryTest extends TestCase */ public function testGetFirstTransaction() { - $account = FactoryMuffin::create('FireflyIII\Models\Account'); - $journal = FactoryMuffin::create('FireflyIII\Models\TransactionJournal'); - $first = $journal->transactions()->orderBy('date', 'DESC')->first(); + $account = FactoryMuffin::create('FireflyIII\Models\Account'); + $journal = FactoryMuffin::create('FireflyIII\Models\TransactionJournal'); + $first = $journal->transactions()->orderBy('date', 'DESC')->first(); $first->account_id = $account->id; $first->save(); @@ -414,63 +413,80 @@ class AccountRepositoryTest extends TestCase public function testGetTransfersInRange() { FactoryMuffin::create('FireflyIII\Models\Account'); - - - // three transfers, two out of range: - FactoryMuffin::create('FireflyIII\Models\TransactionType'); - FactoryMuffin::create('FireflyIII\Models\TransactionType'); - $journal1 = FactoryMuffin::create('FireflyIII\Models\TransactionJournal'); - $journal2 = FactoryMuffin::create('FireflyIII\Models\TransactionJournal'); - $journal3 = FactoryMuffin::create('FireflyIII\Models\TransactionJournal'); - $account = FactoryMuffin::create('FireflyIII\Models\Account'); - $journal2->transaction_type_id = $journal1->transaction_type_id; - $journal3->transaction_type_id = $journal1->transaction_type_id; - - // transactions are already present, update them! - $journal1->transactions[0]->account_id = $account->id; - $journal1->transactions[0]->amount = 100; - $journal1->transactions[1]->account_id = $account->id; - $journal1->transactions[1]->amount = 100; - $journal2->transactions[0]->account_id = $account->id; - $journal2->transactions[0]->amount = 100; - $journal2->transactions[1]->account_id = $account->id; - $journal2->transactions[1]->amount = 100; - $journal3->transactions[0]->account_id = $account->id; - $journal3->transactions[0]->amount = 100; - $journal3->transactions[1]->account_id = $account->id; - $journal3->transactions[1]->amount = 100; - $journal1->transactions[0]->save(); - $journal1->transactions[1]->save(); - $journal2->transactions[0]->save(); - $journal2->transactions[1]->save(); - $journal3->transactions[0]->save(); - $journal3->transactions[1]->save(); - - // check date: + FactoryMuffin::create('FireflyIII\Models\TransactionType'); // withdrawal + FactoryMuffin::create('FireflyIII\Models\TransactionType'); // deposit + FactoryMuffin::create('FireflyIII\Models\AccountType'); // expense + FactoryMuffin::create('FireflyIII\Models\AccountType'); // revenue + $asset = FactoryMuffin::create('FireflyIII\Models\AccountType'); // asset + $transfer = FactoryMuffin::create('FireflyIII\Models\TransactionType'); // transfer + $user = FactoryMuffin::create('FireflyIII\User'); // user! + $accounts = []; + $opposings = []; // opposing accounts. + $journals = []; + // dates $start = new Carbon('2014-01-01'); $end = new Carbon('2014-01-31'); $inRange = new Carbon('2014-01-15'); $before = new Carbon('2013-01-15'); - $after = new Carbon('2015-01-15'); - // journal 1 will match: - $journal1->date = $inRange; - $journal1->user_id = $account->user_id; - $journal2->date = $before; - $journal2->user_id = $account->user_id; - $journal3->date = $after; - $journal3->user_id = $account->user_id; - $journal1->save(); - $journal2->save(); - $journal3->save(); - $this->be($account->user); + // create two accounts: + for ($i = 0; $i < 2; $i++) { + $account = FactoryMuffin::create('FireflyIII\Models\Account'); + $account->account_type_id = $asset->id; + $account->user_id = $user->id; + $account->save(); + $accounts[] = $account; - $set = $this->object->getTransfersInRange($account, $start, $end); + $opposing = FactoryMuffin::create('FireflyIII\Models\Account'); + $opposing->account_type_id = $asset->id; + $opposing->user_id = $user->id; + $opposing->save(); + $opposings[] = $opposing; + } + + // for each account, create ten journals + foreach ($accounts as $index => $account) { + for ($i = 0; $i < 10; $i++) { + $journal = FactoryMuffin::create('FireflyIII\Models\TransactionJournal'); + $journal->user_id = $user->id; + $journal->transaction_type_id = $transfer->id; + $journal->save(); + + // if $i < 6, transfer is in range: + if ($i < 6) { + $journal->date = $inRange; + } else { + $journal->date = $before; + } + + /* + * Transfers can go either way (see the amount) + */ + if($i < 4) { + $amount = 100; + } else { + $amount = -100; + } - $this->assertEquals(1, $set->count()); + $journal->transactions[0]->account_id = $account->id; + $journal->transactions[0]->amount = $amount; + $journal->transactions[1]->account_id = $opposings[$index]->id; + $journal->transactions[1]->amount = $amount * -1; + $journal->transactions[0]->save(); + $journal->transactions[1]->save(); + // save journal: + $journal->save(); + $journals[] = $journal; + } + } + $this->be($user); + + $set = $this->object->getTransfersInRange($accounts[0], $start, $end); + + $this->assertEquals(4, $set->count()); $this->assertEquals(100, $set->first()->amount); - $this->assertEquals($journal1->description, $set->first()->description); + $this->assertEquals($journals[0]->description, $set->first()->description); } diff --git a/tests/repositories/CategoryRepositoryTest.php b/tests/repositories/CategoryRepositoryTest.php index 9ea3727bc4..d8fcf3ed76 100644 --- a/tests/repositories/CategoryRepositoryTest.php +++ b/tests/repositories/CategoryRepositoryTest.php @@ -84,53 +84,43 @@ class CategoryRepositoryTest extends TestCase */ public function testGetCategoriesAndExpensesCorrected() { - $user = FactoryMuffin::create('FireflyIII\User'); - $type = FactoryMuffin::create('FireflyIII\Models\TransactionType'); - // some journals and categories: - for ($i = 0; $i < 5; $i++) { - $journal = FactoryMuffin::create('FireflyIII\Models\TransactionJournal'); + $user = FactoryMuffin::create('FireflyIII\User'); + $type = FactoryMuffin::create('FireflyIII\Models\TransactionType'); // withdrawal + $currency = FactoryMuffin::create('FireflyIII\Models\TransactionCurrency'); // something + + // some dates: + $start = new Carbon('2014-01-01'); + $end = new Carbon('2014-01-31'); + $inRange = new Carbon('2014-01-12'); + + // generate three categories + // with ten journals each: + for ($i = 0; $i < 3; $i++) { + // category: /** @var Category $category */ - $category = FactoryMuffin::create('FireflyIII\Models\Category'); - $journal->user_id = $user->id; - $journal->date = new Carbon('2015-02-11'); - $journal->transaction_type_id = $type->id; - $category->user_id = $user->id; - $category->transactionjournals()->save($journal); - $journal->save(); - $category->save(); - } - - for ($i = 0; $i < 5; $i++) { - $journal1 = FactoryMuffin::create('FireflyIII\Models\TransactionJournal'); - $journal2 = FactoryMuffin::create('FireflyIII\Models\TransactionJournal'); - /** @var Category $category */ - $category = FactoryMuffin::create('FireflyIII\Models\Category'); - - $journal1->user_id = $user->id; - $journal1->date = new Carbon('2015-02-11'); - $journal1->transaction_type_id = $type->id; - - $journal2->user_id = $user->id; - $journal2->date = new Carbon('2015-02-11'); - $journal2->transaction_type_id = $type->id; - + $category = FactoryMuffin::create('FireflyIII\Models\Category'); $category->user_id = $user->id; - $category->transactionjournals()->save($journal1); - $category->transactionjournals()->save($journal2); - - $journal1->save(); - $journal2->save(); $category->save(); - } + for ($j = 0; $j < 10; $j++) { + $journal = FactoryMuffin::create('FireflyIII\Models\TransactionJournal'); + $journal->user_id = $user->id; + $journal->transaction_currency_id = $currency->id; + $journal->transaction_type_id = $type->id; // make it a withdrawal + $journal->date = $inRange; + $journal->save(); + // connect to category: + $category->transactionjournals()->save($journal); + } + } $this->be($user); - $set = $this->object->getCategoriesAndExpensesCorrected(new Carbon('2015-02-01'), new Carbon('2015-02-28')); - $this->assertCount(10, $set); + $set = $this->object->getCategoriesAndExpensesCorrected($start, $end); + $this->assertCount(3, $set); reset($set); - // every journal has amount 100. - $this->assertEquals(100, current($set)['sum']); + // 10 journals of 100 each = 1000. + $this->assertEquals('1000', current($set)['sum']); } /** diff --git a/tests/repositories/JournalRepositoryTest.php b/tests/repositories/JournalRepositoryTest.php index e0bc5443b5..a5e32593a7 100644 --- a/tests/repositories/JournalRepositoryTest.php +++ b/tests/repositories/JournalRepositoryTest.php @@ -1,4 +1,5 @@ transaction_type_id = $withdrawal; + $journal->date = $today; + $journal->save(); + + // update transactions: + $journal->transactions[0]->amount = -100; + $journal->transactions[0]->account_id = $asset->id; + $journal->transactions[0]->save(); + + $journal->transactions[1]->amount = 100; + $journal->transactions[1]->account_id = $expense->id; + $journal->transactions[1]->save(); + + // connect to expense + $today->addDay(); + } - $transaction = FactoryMuffin::create('FireflyIII\Models\Transaction'); - $before = $this->object->getAmountBefore($transaction->transactionjournal, $transaction); - - $this->assertEquals(0, $before); + $before = $this->object->getAmountBefore($journal, $journal->transactions[1]); + // five transactions, we check the last one, so amount should be 400. + $this->assertEquals(400, $before); } /**