From 1c8834fffb37191c6b5d1dc5879e885174726e70 Mon Sep 17 00:00:00 2001 From: James Cole Date: Tue, 26 Apr 2016 08:09:10 +0200 Subject: [PATCH] Some code cleaning up and refactoring. --- app/Helpers/Report/BalanceReportHelper.php | 52 +++++++- app/Http/Controllers/TagController.php | 4 +- app/Models/Tag.php | 4 +- app/Repositories/Tag/TagRepository.php | 112 +----------------- .../Tag/TagRepositoryInterface.php | 33 ++---- app/Repositories/User/UserRepository.php | 2 +- .../User/UserRepositoryInterface.php | 6 + app/Support/Models/TagSupport.php | 78 ++++++++++++ 8 files changed, 148 insertions(+), 143 deletions(-) create mode 100644 app/Support/Models/TagSupport.php diff --git a/app/Helpers/Report/BalanceReportHelper.php b/app/Helpers/Report/BalanceReportHelper.php index 5933d04954..2945f2a677 100644 --- a/app/Helpers/Report/BalanceReportHelper.php +++ b/app/Helpers/Report/BalanceReportHelper.php @@ -83,6 +83,54 @@ class BalanceReportHelper implements BalanceReportHelperInterface return $balance; } + /** + * This method collects all transfers that are part of a "balancing act" tag + * and groups the amounts of those transfers by their destination account. + * + * This is used to indicate which expenses, usually outside of budgets, have been + * corrected by transfers from a savings account. + * + * @param Collection $accounts + * @param Carbon $start + * @param Carbon $end + * + * @return Collection + */ + private function allCoveredByBalancingActs(Collection $accounts, Carbon $start, Carbon $end): Collection + { + $ids = $accounts->pluck('id')->toArray(); + $set = $this->user->tags() + ->leftJoin('tag_transaction_journal', 'tag_transaction_journal.tag_id', '=', 'tags.id') + ->leftJoin('transaction_journals', 'tag_transaction_journal.transaction_journal_id', '=', 'transaction_journals.id') + ->leftJoin('transaction_types', 'transaction_journals.transaction_type_id', '=', 'transaction_types.id') + ->leftJoin( + 'transactions AS t_source', function (JoinClause $join) { + $join->on('transaction_journals.id', '=', 't_source.transaction_journal_id')->where('t_source.amount', '<', 0); + } + ) + ->leftJoin( + 'transactions AS t_to', function (JoinClause $join) { + $join->on('transaction_journals.id', '=', 't_to.transaction_journal_id')->where('t_to.amount', '>', 0); + } + ) + ->where('tags.tagMode', 'balancingAct') + ->where('transaction_types.type', TransactionType::TRANSFER) + ->where('transaction_journals.date', '>=', $start->format('Y-m-d')) + ->where('transaction_journals.date', '<=', $end->format('Y-m-d')) + ->whereNull('transaction_journals.deleted_at') + ->whereIn('t_source.account_id', $ids) + ->whereIn('t_destination.account_id', $ids) + ->groupBy('t_destination.account_id') + ->get( + [ + 't_destination.account_id', + DB::raw('SUM(`t_destination`.`amount`) as `sum`'), + ] + ); + + return $set; + } + /** * @param Budget $budget @@ -133,7 +181,7 @@ class BalanceReportHelper implements BalanceReportHelperInterface private function createDifferenceBalanceLine(Collection $accounts, Collection $spentData, Carbon $start, Carbon $end): BalanceLine { $diff = new BalanceLine; - $tagsLeft = $this->tagRepository->allCoveredByBalancingActs($accounts, $start, $end); + $tagsLeft = $this->allCoveredByBalancingActs($accounts, $start, $end); $diff->setRole(BalanceLine::ROLE_DIFFROLE); @@ -211,7 +259,7 @@ class BalanceReportHelper implements BalanceReportHelperInterface private function createTagsBalanceLine(Collection $accounts, Carbon $start, Carbon $end): BalanceLine { $tags = new BalanceLine; - $tagsLeft = $this->tagRepository->allCoveredByBalancingActs($accounts, $start, $end); + $tagsLeft = $this->allCoveredByBalancingActs($accounts, $start, $end); $tags->setRole(BalanceLine::ROLE_TAGROLE); diff --git a/app/Http/Controllers/TagController.php b/app/Http/Controllers/TagController.php index b2ef298649..a57e76429e 100644 --- a/app/Http/Controllers/TagController.php +++ b/app/Http/Controllers/TagController.php @@ -132,8 +132,8 @@ class TagController extends Controller /* * Can this tag become another type? */ - $allowAdvance = $repository->tagAllowAdvance($tag); - $allowToBalancingAct = $repository->tagAllowBalancing($tag); + $allowAdvance = Tag::tagAllowAdvance($tag); + $allowToBalancingAct = Tag::tagAllowBalancing($tag); // edit tag options: if ($allowAdvance === false) { diff --git a/app/Models/Tag.php b/app/Models/Tag.php index 9646cd855e..f08a04d520 100644 --- a/app/Models/Tag.php +++ b/app/Models/Tag.php @@ -5,7 +5,7 @@ namespace FireflyIII\Models; use Auth; use Crypt; -use Illuminate\Database\Eloquent\Model; +use FireflyIII\Support\Models\TagSupport; use Symfony\Component\HttpKernel\Exception\NotFoundHttpException; /** @@ -40,7 +40,7 @@ use Symfony\Component\HttpKernel\Exception\NotFoundHttpException; * @method static \Illuminate\Database\Query\Builder|\FireflyIII\Models\Tag whereZoomLevel($value) * @mixin \Eloquent */ -class Tag extends Model +class Tag extends TagSupport { protected $dates = ['created_at', 'updated_at', 'date']; protected $fillable = ['user_id', 'tag', 'date', 'description', 'longitude', 'latitude', 'zoomLevel', 'tagMode']; diff --git a/app/Repositories/Tag/TagRepository.php b/app/Repositories/Tag/TagRepository.php index 0ac270ae34..8722e04d42 100644 --- a/app/Repositories/Tag/TagRepository.php +++ b/app/Repositories/Tag/TagRepository.php @@ -25,7 +25,7 @@ class TagRepository implements TagRepositoryInterface private $user; /** - * BillRepository constructor. + * TagRepository constructor. * * @param User $user */ @@ -34,48 +34,6 @@ class TagRepository implements TagRepositoryInterface $this->user = $user; } - /** - * @param Collection $accounts - * @param Carbon $start - * @param Carbon $end - * - * @return Collection - */ - public function allCoveredByBalancingActs(Collection $accounts, Carbon $start, Carbon $end): Collection - { - $ids = $accounts->pluck('id')->toArray(); - $set = $this->user->tags() - ->leftJoin('tag_transaction_journal', 'tag_transaction_journal.tag_id', '=', 'tags.id') - ->leftJoin('transaction_journals', 'tag_transaction_journal.transaction_journal_id', '=', 'transaction_journals.id') - ->leftJoin('transaction_types', 'transaction_journals.transaction_type_id', '=', 'transaction_types.id') - ->leftJoin( - 'transactions AS t_from', function (JoinClause $join) { - $join->on('transaction_journals.id', '=', 't_from.transaction_journal_id')->where('t_from.amount', '<', 0); - } - ) - ->leftJoin( - 'transactions AS t_to', function (JoinClause $join) { - $join->on('transaction_journals.id', '=', 't_to.transaction_journal_id')->where('t_to.amount', '>', 0); - } - ) - ->where('tags.tagMode', 'balancingAct') - ->where('transaction_types.type', TransactionType::TRANSFER) - ->where('transaction_journals.date', '>=', $start->format('Y-m-d')) - ->where('transaction_journals.date', '<=', $end->format('Y-m-d')) - ->whereNull('transaction_journals.deleted_at') - ->whereIn('t_from.account_id', $ids) - ->whereIn('t_to.account_id', $ids) - ->groupBy('t_to.account_id') - ->get( - [ - 't_to.account_id', - DB::raw('SUM(`t_to`.`amount`) as `sum`'), - ] - ); - - return $set; - } - /** * * @param TransactionJournal $journal @@ -160,74 +118,6 @@ class TagRepository implements TagRepositoryInterface } - /** - * Can a tag become an advance payment? - * - * @param Tag $tag - * - * @return bool - */ - public function tagAllowAdvance(Tag $tag): bool - { - /* - * If this tag is a balancing act, and it contains transfers, it cannot be - * changes to an advancePayment. - */ - - if ($tag->tagMode == 'balancingAct' || $tag->tagMode == 'nothing') { - foreach ($tag->transactionjournals as $journal) { - if ($journal->isTransfer()) { - return false; - } - } - } - - /* - * If this tag contains more than one expenses, it cannot become an advance payment. - */ - $count = 0; - foreach ($tag->transactionjournals as $journal) { - if ($journal->isWithdrawal()) { - $count++; - } - } - if ($count > 1) { - return false; - } - - return true; - - } - - /** - * Can a tag become a balancing act? - * - * @param Tag $tag - * - * @return bool - */ - public function tagAllowBalancing(Tag $tag): bool - { - /* - * If has more than two transactions already, cannot become a balancing act: - */ - if ($tag->transactionjournals->count() > 2) { - return false; - } - - /* - * If any transaction is a deposit, cannot become a balancing act. - */ - foreach ($tag->transactionjournals as $journal) { - if ($journal->isDeposit()) { - return false; - } - } - - return true; - } - - /** * @param Tag $tag * @param array $data diff --git a/app/Repositories/Tag/TagRepositoryInterface.php b/app/Repositories/Tag/TagRepositoryInterface.php index 675b0bf34a..fe08065ba6 100644 --- a/app/Repositories/Tag/TagRepositoryInterface.php +++ b/app/Repositories/Tag/TagRepositoryInterface.php @@ -17,15 +17,8 @@ use Illuminate\Support\Collection; interface TagRepositoryInterface { /** - * @param Collection $accounts - * @param Carbon $start - * @param Carbon $end + * This method will connect a journal with a tag. * - * @return Collection - */ - public function allCoveredByBalancingActs(Collection $accounts, Carbon $start, Carbon $end): Collection; - - /** * @param TransactionJournal $journal * @param Tag $tag * @@ -34,6 +27,8 @@ interface TagRepositoryInterface public function connect(TransactionJournal $journal, Tag $tag): bool; /** + * This method destroys a tag. + * * @param Tag $tag * * @return bool @@ -41,11 +36,15 @@ interface TagRepositoryInterface public function destroy(Tag $tag): bool; /** + * This method returns all the user's tags. + * * @return Collection */ public function get(): Collection; /** + * This method stores a tag. + * * @param array $data * * @return Tag @@ -53,24 +52,8 @@ interface TagRepositoryInterface public function store(array $data): Tag; /** - * Can a tag become an advance payment? + * Update a tag. * - * @param Tag $tag - * - * @return bool - */ - public function tagAllowAdvance(Tag $tag): bool; - - /** - * Can a tag become a balancing act? - * - * @param Tag $tag - * - * @return bool - */ - public function tagAllowBalancing(Tag $tag): bool; - - /** * @param Tag $tag * @param array $data * diff --git a/app/Repositories/User/UserRepository.php b/app/Repositories/User/UserRepository.php index 3f4f0764cd..9a3c9fd267 100644 --- a/app/Repositories/User/UserRepository.php +++ b/app/Repositories/User/UserRepository.php @@ -51,6 +51,6 @@ class UserRepository implements UserRepositoryInterface */ public function count(): int { - return User::count(); + return $this->all()->count(); } } diff --git a/app/Repositories/User/UserRepositoryInterface.php b/app/Repositories/User/UserRepositoryInterface.php index e26a36360d..371c4306df 100644 --- a/app/Repositories/User/UserRepositoryInterface.php +++ b/app/Repositories/User/UserRepositoryInterface.php @@ -22,11 +22,15 @@ use Illuminate\Support\Collection; interface UserRepositoryInterface { /** + * Returns a collection of all users. + * * @return Collection */ public function all(): Collection; /** + * Gives a user a role. + * * @param User $user * @param string $role * @@ -35,6 +39,8 @@ interface UserRepositoryInterface public function attachRole(User $user, string $role): bool; /** + * Returns a count of all users. + * * @return int */ public function count(): int; diff --git a/app/Support/Models/TagSupport.php b/app/Support/Models/TagSupport.php new file mode 100644 index 0000000000..7ababbea04 --- /dev/null +++ b/app/Support/Models/TagSupport.php @@ -0,0 +1,78 @@ +tagMode == 'balancingAct' || $tag->tagMode == 'nothing') { + foreach ($tag->transactionjournals as $journal) { + if ($journal->isTransfer()) { + return false; + } + } + } + + /* + * If this tag contains more than one expenses, it cannot become an advance payment. + */ + $count = 0; + foreach ($tag->transactionjournals as $journal) { + if ($journal->isWithdrawal()) { + $count++; + } + } + if ($count > 1) { + return false; + } + + return true; + } + + /** + * Can a tag become a balancing act? + * + * @param Tag $tag + * + * @return bool + */ + public static function tagAllowBalancing(Tag $tag): bool + { + /* + * If has more than two transactions already, cannot become a balancing act: + */ + if ($tag->transactionjournals->count() > 2) { + return false; + } + + /* + * If any transaction is a deposit, cannot become a balancing act. + */ + foreach ($tag->transactionjournals as $journal) { + if ($journal->isDeposit()) { + return false; + } + } + + return true; + + } + +} \ No newline at end of file