From 1f865d3ea4aa91426816ead076b2c69219bd42eb Mon Sep 17 00:00:00 2001 From: James Cole Date: Fri, 22 May 2015 15:05:32 +0200 Subject: [PATCH] Improved coverage. --- app/Helpers/Report/ReportQuery.php | 147 +----------------- app/Helpers/Report/ReportQueryInterface.php | 38 +---- app/Repositories/Budget/BudgetRepository.php | 64 +------- .../Budget/BudgetRepositoryInterface.php | 28 ---- .../Category/CategoryRepository.php | 83 ---------- .../Category/CategoryRepositoryInterface.php | 27 ---- tests/helpers/ReportHelperTest.php | 27 ++++ tests/repositories/BudgetRepositoryTest.php | 22 +-- tests/repositories/CategoryRepositoryTest.php | 22 +-- 9 files changed, 57 insertions(+), 401 deletions(-) diff --git a/app/Helpers/Report/ReportQuery.php b/app/Helpers/Report/ReportQuery.php index a2fd5ee077..78f4295ea2 100644 --- a/app/Helpers/Report/ReportQuery.php +++ b/app/Helpers/Report/ReportQuery.php @@ -22,61 +22,6 @@ use Steam; */ class ReportQuery implements ReportQueryInterface { - - /** - * This method returns all "expense" journals in a certain period, which are both transfers to a shared account - * and "ordinary" withdrawals. The query used is almost equal to ReportQueryInterface::journalsByRevenueAccount but it does - * not group and returns different fields. - * - * @param Carbon $start - * @param Carbon $end - * @param bool $includeShared - * - * @return Collection - * - */ - public function expenseInPeriod(Carbon $start, Carbon $end, $includeShared = false) - { - $query = $this->queryJournalsWithTransactions($start, $end); - if ($includeShared === false) { - $query->where( - function (Builder $query) { - $query->where( - function (Builder $q) { // only get withdrawals not from a shared account - $q->where('transaction_types.type', 'Withdrawal'); - $q->where('acm_from.data', '!=', '"sharedAsset"'); - } - ); - $query->orWhere( - function (Builder $q) { // and transfers from a shared account. - $q->where('transaction_types.type', 'Transfer'); - $q->where('acm_to.data', '=', '"sharedAsset"'); - } - ); - } - ); - } else { - $query->where('transaction_types.type', 'Withdrawal'); // any withdrawal is fine. - } - $query->groupBy('transaction_journals.id')->orderBy('transaction_journals.date'); - - // get everything, decrypt and return - $data = $query->get( - ['transaction_journals.id', 'transaction_journals.description', 'transaction_journals.encrypted', 'transaction_types.type', - DB::Raw('SUM(`t_from`.`amount`) as `queryAmount`'), - 'transaction_journals.date', 't_to.account_id as account_id', 'ac_to.name as name', 'ac_to.encrypted as account_encrypted'] - ); - - $data->each( - function (Model $object) { - $object->name = intval($object->account_encrypted) == 1 ? Crypt::decrypt($object->name) : $object->name; - } - ); - $data->sortByDesc('queryAmount'); - - return $data; - } - /** * See ReportQueryInterface::incomeInPeriodCorrected * @@ -184,73 +129,16 @@ class ReportQuery implements ReportQueryInterface return $set; } - /** - * This method returns all "income" journals in a certain period, which are both transfers from a shared account - * and "ordinary" deposits. The query used is almost equal to ReportQueryInterface::journalsByRevenueAccount but it does - * not group and returns different fields. - * - * @param Carbon $start - * @param Carbon $end - * @param bool $includeShared - * - * @return Collection - */ - public function incomeInPeriod(Carbon $start, Carbon $end, $includeShared = false) - { - $query = $this->queryJournalsWithTransactions($start, $end); - if ($includeShared === false) { - // only get deposits not to a shared account - // and transfers to a shared account. - $query->where( - function (Builder $query) { - $query->where( - function (Builder $q) { - $q->where('transaction_types.type', 'Deposit'); - $q->where('acm_to.data', '!=', '"sharedAsset"'); - } - ); - $query->orWhere( - function (Builder $q) { - $q->where('transaction_types.type', 'Transfer'); - $q->where('acm_from.data', '=', '"sharedAsset"'); - } - ); - } - ); - } else { - // any deposit is fine. - $query->where('transaction_types.type', 'Deposit'); - } - $query->groupBy('transaction_journals.id')->orderBy('transaction_journals.date'); - - // get everything, decrypt and return - $data = $query->get( - ['transaction_journals.id', - 'transaction_journals.description', - 'transaction_journals.encrypted', - 'transaction_types.type', - DB::Raw('SUM(`t_to`.`amount`) as `queryAmount`'), - 'transaction_journals.date', - 't_from.account_id as account_id', - 'ac_from.name as name', - 'ac_from.encrypted as account_encrypted' - ] - ); - - $data->each( - function (Model $object) { - $object->name = intval($object->account_encrypted) == 1 ? Crypt::decrypt($object->name) : $object->name; - } - ); - $data->sortByDesc('queryAmount'); - - return $data; - } /** * This method works the same way as ReportQueryInterface::incomeInPeriod does, but instead of returning results * will simply list the transaction journals only. This should allow any follow up counting to be accurate with * regards to tags. + * + * This method returns all "income" journals in a certain period, which are both transfers from a shared account + * and "ordinary" deposits. The query used is almost equal to ReportQueryInterface::journalsByRevenueAccount but it does + * not group and returns different fields. + * * @param Carbon $start * @param Carbon $end @@ -309,31 +197,6 @@ class ReportQuery implements ReportQueryInterface return $data; } - /** - * @param Account $account - * @param Budget $budget - * @param Carbon $start - * @param Carbon $end - * - * @return float - */ - public function spentInBudget(Account $account, Budget $budget, Carbon $start, Carbon $end) - { - - return floatval( - Auth::user()->transactionjournals() - ->leftJoin('transactions', 'transactions.transaction_journal_id', '=', 'transaction_journals.id') - ->leftJoin('budget_transaction_journal', 'budget_transaction_journal.transaction_journal_id', '=', 'transaction_journals.id') - ->transactionTypes(['Withdrawal']) - ->where('transactions.amount', '<', 0) - ->where('transactions.account_id', $account->id) - ->before($end) - ->after($start) - ->where('budget_transaction_journal.budget_id', $budget->id) - ->sum('transactions.amount') - ); - } - /** * Covers tags * diff --git a/app/Helpers/Report/ReportQueryInterface.php b/app/Helpers/Report/ReportQueryInterface.php index 1ecf9f9f21..fc46ab8255 100644 --- a/app/Helpers/Report/ReportQueryInterface.php +++ b/app/Helpers/Report/ReportQueryInterface.php @@ -16,6 +16,8 @@ interface ReportQueryInterface { /** + * See ReportQueryInterface::incomeInPeriodCorrected + * * This method returns all "expense" journals in a certain period, which are both transfers to a shared account * and "ordinary" withdrawals. The query used is almost equal to ReportQueryInterface::journalsByRevenueAccount but it does * not group and returns different fields. @@ -27,18 +29,6 @@ interface ReportQueryInterface * @return Collection * */ - public function expenseInPeriod(Carbon $start, Carbon $end, $includeShared = false); - - /** - * See ReportQueryInterface::incomeInPeriodCorrected - * - * @param Carbon $start - * @param Carbon $end - * @param bool $includeShared - * - * @return Collection - * - */ public function expenseInPeriodCorrected(Carbon $start, Carbon $end, $includeShared = false); /** @@ -52,20 +42,6 @@ interface ReportQueryInterface */ public function getAllAccounts(Carbon $start, Carbon $end, $includeShared = false); - /** - * This method returns all "income" journals in a certain period, which are both transfers from a shared account - * and "ordinary" deposits. The query used is almost equal to ReportQueryInterface::journalsByRevenueAccount but it does - * not group and returns different fields. - * - * @param Carbon $start - * @param Carbon $end - * @param bool $includeShared - * - * @return Collection - * - */ - public function incomeInPeriod(Carbon $start, Carbon $end, $includeShared = false); - /** * This method works the same way as ReportQueryInterface::incomeInPeriod does, but instead of returning results * will simply list the transaction journals only. This should allow any follow up counting to be accurate with @@ -79,16 +55,6 @@ interface ReportQueryInterface */ public function incomeInPeriodCorrected(Carbon $start, Carbon $end, $includeShared = false); - /** - * @param Account $account - * @param Budget $budget - * @param Carbon $start - * @param Carbon $end - * - * @return float - */ - public function spentInBudget(Account $account, Budget $budget, Carbon $start, Carbon $end); - /** * Covers tags as well. * diff --git a/app/Repositories/Budget/BudgetRepository.php b/app/Repositories/Budget/BudgetRepository.php index 3d35eff2a0..1a0b4f0823 100644 --- a/app/Repositories/Budget/BudgetRepository.php +++ b/app/Repositories/Budget/BudgetRepository.php @@ -49,9 +49,11 @@ class BudgetRepository implements BudgetRepositoryInterface * * @return float */ - public function expensesOnDay(Budget $budget, Carbon $date) + public function expensesOnDayCorrected(Budget $budget, Carbon $date) { - return floatval($budget->transactionjournals()->lessThan(0)->transactionTypes(['Withdrawal'])->onDate($date)->sum('amount')); + $sum = floatval($budget->transactionjournals()->transactionTypes(['Withdrawal'])->onDate($date)->get(['transaction_journals.*'])->sum('amount')); + + return $sum * -1; } /** @@ -255,39 +257,6 @@ class BudgetRepository implements BudgetRepositoryInterface return floatval($noBudgetSet) * -1; } - /** - * @param Budget $budget - * @param Carbon $start - * @param Carbon $end - * @param bool $shared - * - * @return float - */ - public function spentInPeriod(Budget $budget, Carbon $start, Carbon $end, $shared = true) - { - if ($shared === true) { - // get everything: - $sum = floatval($budget->transactionjournals()->before($end)->after($start)->lessThan(0)->sum('amount')) * -1; - } else { - // get all journals in this month where the asset account is NOT shared. - $sum = $budget->transactionjournals() - ->before($end) - ->after($start) - ->lessThan(0) - ->leftJoin('accounts', 'accounts.id', '=', 'transactions.account_id') - ->leftJoin( - 'account_meta', function (JoinClause $join) { - $join->on('account_meta.account_id', '=', 'accounts.id')->where('account_meta.name', '=', 'accountRole'); - } - ) - ->where('account_meta.data', '!=', '"sharedAsset"') - ->sum('amount'); - $sum = floatval($sum) * -1; - } - - return $sum; - } - /** * @param Budget $budget * @param Carbon $start @@ -340,17 +309,7 @@ class BudgetRepository implements BudgetRepositoryInterface return $newBudget; } - /** - * @param Budget $budget - * @param Carbon $start - * @param Carbon $end - * - * @return float - */ - public function sumBudgetExpensesInPeriod(Budget $budget, $start, $end) - { - return floatval($budget->transactionjournals()->before($end)->after($start)->lessThan(0)->sum('amount')) * -1; - } + /** * @param Budget $budget @@ -407,17 +366,4 @@ class BudgetRepository implements BudgetRepositoryInterface } - - /** - * @param Budget $budget - * @param Carbon $date - * - * @return float - */ - public function expensesOnDayCorrected(Budget $budget, Carbon $date) - { - $sum = floatval($budget->transactionjournals()->transactionTypes(['Withdrawal'])->onDate($date)->get(['transaction_journals.*'])->sum('amount')); - - return $sum * -1; - } } diff --git a/app/Repositories/Budget/BudgetRepositoryInterface.php b/app/Repositories/Budget/BudgetRepositoryInterface.php index 384893acef..b928ed35df 100644 --- a/app/Repositories/Budget/BudgetRepositoryInterface.php +++ b/app/Repositories/Budget/BudgetRepositoryInterface.php @@ -26,14 +26,6 @@ interface BudgetRepositoryInterface */ public function destroy(Budget $budget); - /** - * @param Budget $budget - * @param Carbon $date - * - * @return float - */ - public function expensesOnDay(Budget $budget, Carbon $date); - /** * Takes tags into account. * @@ -132,17 +124,6 @@ interface BudgetRepositoryInterface */ public function getWithoutBudgetSum(Carbon $start, Carbon $end); - - /** - * @param Budget $budget - * @param Carbon $start - * @param Carbon $end - * @param boolean $shared - * - * @return float - */ - public function spentInPeriod(Budget $budget, Carbon $start, Carbon $end, $shared = true); - /** * * Same as ::spentInPeriod but corrects journals for their amount (tags). @@ -163,15 +144,6 @@ interface BudgetRepositoryInterface */ public function store(array $data); - /** - * @param Budget $budget - * @param Carbon $start - * @param Carbon $end - * - * @return float - */ - public function sumBudgetExpensesInPeriod(Budget $budget, $start, $end); - /** * @param Budget $budget * @param array $data diff --git a/app/Repositories/Category/CategoryRepository.php b/app/Repositories/Category/CategoryRepository.php index 55963029bf..a6c7de1d3c 100644 --- a/app/Repositories/Category/CategoryRepository.php +++ b/app/Repositories/Category/CategoryRepository.php @@ -58,36 +58,6 @@ class CategoryRepository implements CategoryRepositoryInterface return $set; } - /** - * @param Carbon $start - * @param Carbon $end - * - * @return Collection - */ - public function getCategoriesAndExpenses($start, $end) - { - return TransactionJournal:: - where('transaction_journals.user_id', Auth::user()->id) - ->leftJoin( - 'transactions', - function (JoinClause $join) { - $join->on('transaction_journals.id', '=', 'transactions.transaction_journal_id')->where('amount', '>', 0); - } - ) - ->leftJoin( - 'category_transaction_journal', 'category_transaction_journal.transaction_journal_id', '=', 'transaction_journals.id' - ) - ->leftJoin('categories', 'categories.id', '=', 'category_transaction_journal.category_id') - ->leftJoin('transaction_types', 'transaction_types.id', '=', 'transaction_journals.transaction_type_id') - ->before($end) - ->where('categories.user_id', Auth::user()->id) - ->after($start) - ->where('transaction_types.type', 'Withdrawal') - ->groupBy('categories.id') - ->orderBy('sum', 'DESC') - ->get(['categories.id', 'categories.encrypted', 'categories.name', DB::Raw('SUM(`transactions`.`amount`) AS `sum`')]); - } - /** * * @param Carbon $start @@ -205,48 +175,6 @@ class CategoryRepository implements CategoryRepositoryInterface ->get(['transaction_journals.*']); } - /** - * @param Category $category - * @param Carbon $start - * @param Carbon $end - * - * @param bool $shared - * - * @return float - */ - public function spentInPeriod(Category $category, Carbon $start, Carbon $end, $shared = false) - { - if ($shared === true) { - // shared is true. - // always ignore transfers between accounts! - $sum = floatval( - $category->transactionjournals() - ->transactionTypes(['Withdrawal']) - ->before($end)->after($start)->lessThan(0)->sum('amount') - ) * -1; - - } else { - // do something else, SEE budgets. - // get all journals in this month where the asset account is NOT shared. - $sum = $category->transactionjournals() - ->before($end) - ->after($start) - ->transactionTypes(['Withdrawal']) - ->lessThan(0) - ->leftJoin('accounts', 'accounts.id', '=', 'transactions.account_id') - ->leftJoin( - 'account_meta', function (JoinClause $join) { - $join->on('account_meta.account_id', '=', 'accounts.id')->where('account_meta.name', '=', 'accountRole'); - } - ) - ->where('account_meta.data', '!=', '"sharedAsset"') - ->sum('amount'); - $sum = floatval($sum) * -1; - } - - return $sum; - } - /** * @param Category $category * @param Carbon $start @@ -289,17 +217,6 @@ class CategoryRepository implements CategoryRepositoryInterface return $sum; } - /** - * @param Category $category - * @param Carbon $date - * - * @return float - */ - public function spentOnDaySum(Category $category, Carbon $date) - { - return floatval($category->transactionjournals()->onDate($date)->lessThan(0)->sum('amount')) * -1; - } - /** * Corrected for tags * diff --git a/app/Repositories/Category/CategoryRepositoryInterface.php b/app/Repositories/Category/CategoryRepositoryInterface.php index 9f282c84fe..524b2ce443 100644 --- a/app/Repositories/Category/CategoryRepositoryInterface.php +++ b/app/Repositories/Category/CategoryRepositoryInterface.php @@ -32,14 +32,6 @@ interface CategoryRepositoryInterface */ public function getCategories(); - /** - * @param Carbon $start - * @param Carbon $end - * - * @return Collection - */ - public function getCategoriesAndExpenses($start, $end); - /** * Corrected for tags. * @@ -80,17 +72,6 @@ interface CategoryRepositoryInterface */ public function getWithoutCategory(Carbon $start, Carbon $end); - /** - * @param Category $category - * @param \Carbon\Carbon $start - * @param \Carbon\Carbon $end - * - * @param bool $shared - * - * @return float - */ - public function spentInPeriod(Category $category, Carbon $start, Carbon $end, $shared = false); - /** * Corrected for tags. * @@ -104,14 +85,6 @@ interface CategoryRepositoryInterface */ public function spentInPeriodCorrected(Category $category, Carbon $start, Carbon $end, $shared = false); - /** - * @param Category $category - * @param Carbon $date - * - * @return float - */ - public function spentOnDaySum(Category $category, Carbon $date); - /** * * Corrected for tags. diff --git a/tests/helpers/ReportHelperTest.php b/tests/helpers/ReportHelperTest.php index 8f66f50fb5..9fc169d107 100644 --- a/tests/helpers/ReportHelperTest.php +++ b/tests/helpers/ReportHelperTest.php @@ -235,6 +235,20 @@ class ReportHelperTest extends TestCase $right->account_type_id = $asset->id; $right->save(); $left->save(); + + // save meta for account: + AccountMeta::create([ + 'account_id' => $left->id, + 'name' => 'accountRole', + 'data' => 'defaultAsset' + ]); + AccountMeta::create([ + 'account_id' => $right->id, + 'name' => 'accountRole', + 'data' => 'defaultAsset' + ]); + + for ($i = 0; $i < 5; $i++) { $journal = FactoryMuffin::create('FireflyIII\Models\TransactionJournal'); $journal->date = $date; @@ -281,6 +295,19 @@ class ReportHelperTest extends TestCase $asset = FactoryMuffin::create('FireflyIII\Models\AccountType'); $left->account_type_id = $asset->id; $right->account_type_id = $asset->id; + + // save meta for account: + AccountMeta::create([ + 'account_id' => $left->id, + 'name' => 'accountRole', + 'data' => 'defaultAsset' + ]); + AccountMeta::create([ + 'account_id' => $right->id, + 'name' => 'accountRole', + 'data' => 'defaultAsset' + ]); + $right->save(); $left->save(); for ($i = 0; $i < 5; $i++) { diff --git a/tests/repositories/BudgetRepositoryTest.php b/tests/repositories/BudgetRepositoryTest.php index eddd87c644..d16ae370df 100644 --- a/tests/repositories/BudgetRepositoryTest.php +++ b/tests/repositories/BudgetRepositoryTest.php @@ -70,13 +70,13 @@ class BudgetRepositoryTest extends TestCase } /** - * @covers FireflyIII\Repositories\Budget\BudgetRepository::expensesOnDay + * @covers FireflyIII\Repositories\Budget\BudgetRepository::expensesOnDayCorrected */ - public function testExpensesOnDay() + public function testExpensesOnDayCorrected() { $budget = FactoryMuffin::create('FireflyIII\Models\Budget'); - $result = $this->object->expensesOnDay($budget, new Carbon); + $result = $this->object->expensesOnDayCorrected($budget, new Carbon); $this->assertEquals(0, $result); } @@ -289,13 +289,13 @@ class BudgetRepositoryTest extends TestCase } /** - * @covers FireflyIII\Repositories\Budget\BudgetRepository::spentInPeriod + * @covers FireflyIII\Repositories\Budget\BudgetRepository::spentInPeriodCorrected */ - public function testSpentInPeriod() + public function testSpentInPeriodCorrected() { $budget = FactoryMuffin::create('FireflyIII\Models\Budget'); - $amount = $this->object->spentInPeriod($budget, new Carbon, new Carbon); + $amount = $this->object->spentInPeriodCorrected($budget, new Carbon, new Carbon); $this->assertEquals(0, $amount); } @@ -316,16 +316,6 @@ class BudgetRepositoryTest extends TestCase $this->assertEquals($result->name, $data['name']); } - /** - * @covers FireflyIII\Repositories\Budget\BudgetRepository::sumBudgetExpensesInPeriod - */ - public function testSumBudgetExpensesInPeriod() - { - $budget = FactoryMuffin::create('FireflyIII\Models\Budget'); - $result = $this->object->sumBudgetExpensesInPeriod($budget, new Carbon, new Carbon); - $this->assertEquals(0, $result); - } - /** * @covers FireflyIII\Repositories\Budget\BudgetRepository::update */ diff --git a/tests/repositories/CategoryRepositoryTest.php b/tests/repositories/CategoryRepositoryTest.php index 10121ec76d..0e5b0d92a1 100644 --- a/tests/repositories/CategoryRepositoryTest.php +++ b/tests/repositories/CategoryRepositoryTest.php @@ -79,9 +79,9 @@ class CategoryRepositoryTest extends TestCase } /** - * @covers FireflyIII\Repositories\Category\CategoryRepository::getCategoriesAndExpenses + * @covers FireflyIII\Repositories\Category\CategoryRepository::getCategoriesAndExpensesCorrected */ - public function testGetCategoriesAndExpenses() + public function testGetCategoriesAndExpensesCorrected() { $user = FactoryMuffin::create('FireflyIII\User'); $type = FactoryMuffin::create('FireflyIII\Models\TransactionType'); @@ -100,9 +100,11 @@ class CategoryRepositoryTest extends TestCase } $this->be($user); - $set = $this->object->getCategoriesAndExpenses(new Carbon('2015-02-01'), new Carbon('2015-02-28')); + $set = $this->object->getCategoriesAndExpensesCorrected(new Carbon('2015-02-01'), new Carbon('2015-02-28')); $this->assertCount(5, $set); - $this->assertEquals(0, $set->first()->sum); + reset($set); + + $this->assertEquals(0, current($set)['sum']); } /** @@ -189,12 +191,12 @@ class CategoryRepositoryTest extends TestCase } /** - * @covers FireflyIII\Repositories\Category\CategoryRepository::spentInPeriod + * @covers FireflyIII\Repositories\Category\CategoryRepository::spentInPeriodCorrected */ - public function testSpentInPeriodSum() + public function testSpentInPeriodSumCorrected() { $category = FactoryMuffin::create('FireflyIII\Models\Category'); - $sum = $this->object->spentInPeriod($category, new Carbon, new Carbon); + $sum = $this->object->spentInPeriodCorrected($category, new Carbon, new Carbon); $this->assertEquals(0, $sum); @@ -202,12 +204,12 @@ class CategoryRepositoryTest extends TestCase } /** - * @covers FireflyIII\Repositories\Category\CategoryRepository::spentOnDaySum + * @covers FireflyIII\Repositories\Category\CategoryRepository::spentOnDaySumCorrected */ - public function testSpentOnDaySum() + public function testSpentOnDaySumCorrected() { $category = FactoryMuffin::create('FireflyIII\Models\Category'); - $sum = $this->object->spentOnDaySum($category, new Carbon); + $sum = $this->object->spentOnDaySumCorrected($category, new Carbon); $this->assertEquals(0, $sum); }