From 8fcdb91ba313acc6c11cd013004b22dbec9c7368 Mon Sep 17 00:00:00 2001 From: James Cole Date: Sun, 30 Jul 2017 10:22:14 +0200 Subject: [PATCH] Will no longer support extended tag modes. --- app/Http/Controllers/TagController.php | 37 +-- app/Http/Requests/TagFormRequest.php | 2 - app/Repositories/Tag/TagRepository.php | 261 +----------------- .../Tag/TagRepositoryInterface.php | 14 - resources/lang/en_US/firefly.php | 3 - resources/views/tags/create.twig | 1 - resources/views/tags/edit.twig | 1 - resources/views/tags/index.twig | 84 +++--- 8 files changed, 47 insertions(+), 356 deletions(-) diff --git a/app/Http/Controllers/TagController.php b/app/Http/Controllers/TagController.php index bea865e276..df4bf6e8e1 100644 --- a/app/Http/Controllers/TagController.php +++ b/app/Http/Controllers/TagController.php @@ -89,12 +89,6 @@ class TagController extends Controller $subTitleIcon = 'fa-tag'; $apiKey = env('GOOGLE_MAPS_API_KEY', ''); - $preFilled = [ - 'tagMode' => 'nothing', - ]; - if (!$request->old('tagMode')) { - Session::flash('preFilled', $preFilled); - } // put previous url in session if not redirect from store (not "create another"). if (session('tags.create.fromStore') !== true) { $this->rememberPreviousUri('tags.create.uri'); @@ -153,26 +147,6 @@ class TagController extends Controller $subTitleIcon = 'fa-tag'; $apiKey = env('GOOGLE_MAPS_API_KEY', ''); - /* - * Default tag options (again) - */ - $tagOptions = $this->tagOptions; - - /* - * Can this tag become another type? - */ - $allowAdvance = $repository->tagAllowAdvance($tag); - $allowToBalancingAct = $repository->tagAllowBalancing($tag); - - // edit tag options: - if ($allowAdvance === false) { - unset($tagOptions['advancePayment']); - } - if ($allowToBalancingAct === false) { - unset($tagOptions['balancingAct']); - } - - // put previous url in session if not redirect from store (not "return_to_edit"). if (session('tags.edit.fromUpdate') !== true) { $this->rememberPreviousUri('tags.edit.uri'); @@ -194,6 +168,8 @@ class TagController extends Controller $title = 'Tags'; $mainTitleIcon = 'fa-tags'; $types = ['nothing', 'balancingAct', 'advancePayment']; + $hasTypes = 0; // which types of tag the user actually has. + $counts = []; // how many of each type? $count = $repository->count(); // loop each types and get the tags, group them by year. @@ -209,10 +185,13 @@ class TagController extends Controller return strtolower($date . $tag->tag); } ); + if ($tags->count() > 0) { + $hasTypes++; + } + $counts[$type] = $tags->count(); /** @var Tag $tag */ foreach ($tags as $tag) { - $year = is_null($tag->date) ? trans('firefly.no_year') : $tag->date->year; $monthFormatted = is_null($tag->date) ? trans('firefly.no_month') : $tag->date->formatLocalized($this->monthFormat); @@ -220,7 +199,7 @@ class TagController extends Controller } } - return view('tags.index', compact('title', 'mainTitleIcon', 'types', 'collection', 'count')); + return view('tags.index', compact('title', 'mainTitleIcon', 'counts', 'hasTypes', 'types', 'collection', 'count')); } /** @@ -255,7 +234,7 @@ class TagController extends Controller $start = $repository->firstUseDate($tag); $end = new Carbon; $sum = $repository->sumOfTag($tag, null, null); - $path = route('tags.show', [$tag->id,'all']); + $path = route('tags.show', [$tag->id, 'all']); } // prep for "specific date" view. diff --git a/app/Http/Requests/TagFormRequest.php b/app/Http/Requests/TagFormRequest.php index 1348a0d767..00808beb34 100644 --- a/app/Http/Requests/TagFormRequest.php +++ b/app/Http/Requests/TagFormRequest.php @@ -54,7 +54,6 @@ class TagFormRequest extends Request 'latitude' => $latitude, 'longitude' => $longitude, 'zoomLevel' => $zoomLevel, - 'tagMode' => $this->string('tagMode'), ]; return $data; @@ -84,7 +83,6 @@ class TagFormRequest extends Request 'latitude' => 'numeric|min:-90|max:90', 'longitude' => 'numeric|min:-90|max:90', 'zoomLevel' => 'numeric|min:0|max:80', - 'tagMode' => 'required|in:nothing,balancingAct,advancePayment', ]; } } diff --git a/app/Repositories/Tag/TagRepository.php b/app/Repositories/Tag/TagRepository.php index 559eefcdbf..e81201ef0b 100644 --- a/app/Repositories/Tag/TagRepository.php +++ b/app/Repositories/Tag/TagRepository.php @@ -51,21 +51,11 @@ class TagRepository implements TagRepositoryInterface return false; } + Log::debug(sprintf('Tag #%d connected', $tag->id)); + $journal->tags()->save($tag); + $journal->save(); - switch ($tag->tagMode) { - case 'nothing': - Log::debug(sprintf('Tag #%d connected', $tag->id)); - $journal->tags()->save($tag); - $journal->save(); - - return true; - case 'balancingAct': - return $this->connectBalancingAct($journal, $tag); - case 'advancePayment': - return $this->connectAdvancePayment($journal, $tag); - } - - return false; + return true; } /** @@ -237,7 +227,7 @@ class TagRepository implements TagRepositoryInterface $tag->latitude = $data['latitude']; $tag->longitude = $data['longitude']; $tag->zoomLevel = $data['zoomLevel']; - $tag->tagMode = $data['tagMode']; + $tag->tagMode = 'nothing'; $tag->user()->associate($this->user); $tag->save(); @@ -268,73 +258,6 @@ class TagRepository implements TagRepositoryInterface return strval($sum); } - /** - * 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 - * changed 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 @@ -349,183 +272,9 @@ class TagRepository implements TagRepositoryInterface $tag->latitude = $data['latitude']; $tag->longitude = $data['longitude']; $tag->zoomLevel = $data['zoomLevel']; - $tag->tagMode = $data['tagMode']; $tag->save(); return $tag; } - /** - * @param TransactionJournal $journal - * @param Tag $tag - * - * @return bool - */ - protected function connectAdvancePayment(TransactionJournal $journal, Tag $tag): bool - { - $type = $journal->transactionType->type; - $withdrawals = $tag->transactionJournals() - ->leftJoin('transaction_types', 'transaction_types.id', 'transaction_journals.transaction_type_id') - ->where('transaction_types.type', TransactionType::WITHDRAWAL)->count(); - $deposits = $tag->transactionJournals() - ->leftJoin('transaction_types', 'transaction_types.id', 'transaction_journals.transaction_type_id') - ->where('transaction_types.type', TransactionType::DEPOSIT)->count(); - - if ($type === TransactionType::TRANSFER) { // advance payments cannot accept transfers: - Log::error(sprintf('Journal #%d is a transfer and cannot connect to tag #%d', $journal->id, $tag->id)); - - return false; - } - - // the first transaction to be attached to this tag is attached just like that: - if ($withdrawals < 1 && $deposits < 1) { - Log::debug(sprintf('Tag #%d has 0 withdrawals and 0 deposits so its fine.', $tag->id)); - $journal->tags()->save($tag); - $journal->save(); - - return true; - } - - // if withdrawal and already has a withdrawal, return false: - if ($type === TransactionType::WITHDRAWAL && $withdrawals > 0) { - Log::error(sprintf('Journal #%d is a withdrawal but tag already has %d withdrawal(s).', $journal->id, $withdrawals)); - - return false; - } - - // if already has transaction journals, must match ALL asset account id's: - if ($deposits > 0 || $withdrawals === 1) { - Log::debug('Need to match all asset accounts.'); - - return $this->matchAll($journal, $tag); - } - - // this statement is unreachable. - return false; - - } - - /** - * @param TransactionJournal $journal - * @param Tag $tag - * - * @return bool - */ - protected function connectBalancingAct(TransactionJournal $journal, Tag $tag): bool - { - $type = $journal->transactionType->type; - $withdrawals = $tag->transactionJournals() - ->leftJoin('transaction_types', 'transaction_types.id', 'transaction_journals.transaction_type_id') - ->where('transaction_types.type', TransactionType::WITHDRAWAL)->count(); - $transfers = $tag->transactionJournals() - ->leftJoin('transaction_types', 'transaction_types.id', 'transaction_journals.transaction_type_id') - ->where('transaction_types.type', TransactionType::TRANSFER)->count(); - - - Log::debug(sprintf('Journal #%d is a %s', $journal->id, $type)); - - // only if this is the only withdrawal. - if ($type === TransactionType::WITHDRAWAL && $withdrawals < 1) { - Log::debug('Will connect this journal because it is the only withdrawal in this tag.'); - $journal->tags()->save($tag); - $journal->save(); - - return true; - } - // and only if this is the only transfer - if ($type === TransactionType::TRANSFER && $transfers < 1) { - Log::debug('Will connect this journal because it is the only transfer in this tag.'); - $journal->tags()->save($tag); - $journal->save(); - - return true; - } - Log::error( - sprintf( - 'Tag #%d has %d withdrawals and %d transfers and cannot contain %s #%d', - $tag->id, $withdrawals, $transfers, $type, $journal->id - ) - ); - - // ignore expense - return false; - - } - - /** - * The incoming journal ($journal)'s accounts (source accounts for a withdrawal, destination accounts for a deposit) - * must match the already existing transaction's accounts exactly. - * - * @param TransactionJournal $journal - * @param Tag $tag - * - * - * @return bool - */ - private function matchAll(TransactionJournal $journal, Tag $tag): bool - { - $journalSources = join(',', array_unique($journal->sourceAccountList()->pluck('id')->toArray())); - $journalDestinations = join(',', array_unique($journal->destinationAccountList()->pluck('id')->toArray())); - $match = true; - $journals = $tag->transactionJournals()->get(['transaction_journals.*']); - - Log::debug(sprintf('Tag #%d has %d journals to verify:', $tag->id, $journals->count())); - - /** @var TransactionJournal $existing */ - foreach ($journals as $existing) { - Log::debug(sprintf('Now existingcomparing new journal #%d to existing journal #%d', $journal->id, $existing->id)); - // $checkAccount is the source_account for a withdrawal - // $checkAccount is the destination_account for a deposit - $existingSources = join(',', array_unique($existing->sourceAccountList()->pluck('id')->toArray())); - $existingDestinations = join(',', array_unique($existing->destinationAccountList()->pluck('id')->toArray())); - - if ($existing->isWithdrawal() && $existingSources !== $journalDestinations) { - /* - * There can only be one withdrawal. And the source account(s) of the withdrawal - * must be the same as the destination of the deposit. Because any transaction that arrives - * here ($journal) must be a deposit. - */ - Log::debug(sprintf('Existing journal #%d is a withdrawal.', $existing->id)); - Log::debug(sprintf('New journal #%d must have these destination accounts: %s', $journal->id, $existingSources)); - Log::debug(sprintf('New journal #%d actually these destination accounts: %s', $journal->id, $journalDestinations)); - Log::debug('So match is FALSE'); - - $match = false; - } - if ($existing->isDeposit() && $journal->isDeposit() && $existingDestinations !== $journalDestinations) { - /* - * There can be multiple deposits. - * They must have the destination the same as the other deposits. - */ - Log::debug(sprintf('Existing journal #%d is a deposit.', $existing->id)); - Log::debug(sprintf('Journal #%d must have these destination accounts: %s', $journal->id, $existingDestinations)); - Log::debug(sprintf('Journal #%d actually these destination accounts: %s', $journal->id, $journalDestinations)); - Log::debug('So match is FALSE'); - - $match = false; - } - - if ($existing->isDeposit() && $journal->isWithdrawal() && $existingDestinations !== $journalSources) { - /* - * There can be one new withdrawal only. It must have the same source as the existing has destination. - */ - Log::debug(sprintf('Existing journal #%d is a deposit.', $existing->id)); - Log::debug(sprintf('Journal #%d must have these source accounts: %s', $journal->id, $existingDestinations)); - Log::debug(sprintf('Journal #%d actually these source accounts: %s', $journal->id, $journalSources)); - Log::debug('So match is FALSE'); - - $match = false; - } - - } - if ($match) { - Log::debug(sprintf('Match is true, connect journal #%d with tag #%d.', $journal->id, $tag->id)); - $journal->tags()->save($tag); - $journal->save(); - - return true; - } - - return false; - } } diff --git a/app/Repositories/Tag/TagRepositoryInterface.php b/app/Repositories/Tag/TagRepositoryInterface.php index c23875c588..8cc71547ba 100644 --- a/app/Repositories/Tag/TagRepositoryInterface.php +++ b/app/Repositories/Tag/TagRepositoryInterface.php @@ -135,20 +135,6 @@ interface TagRepositoryInterface */ public function sumOfTag(Tag $tag, ?Carbon $start, ?Carbon $end): string; - /** - * @param Tag $tag - * - * @return bool - */ - public function tagAllowAdvance(Tag $tag): bool; - - /** - * @param Tag $tag - * - * @return bool - */ - public function tagAllowBalancing(Tag $tag): bool; - /** * Update a tag. * diff --git a/resources/lang/en_US/firefly.php b/resources/lang/en_US/firefly.php index 816159cb07..dea2d1c6df 100644 --- a/resources/lang/en_US/firefly.php +++ b/resources/lang/en_US/firefly.php @@ -887,9 +887,6 @@ return [ 'tag_title_nothing' => 'Default tags', 'tag_title_balancingAct' => 'Balancing act tags', 'tag_title_advancePayment' => 'Advance payment tags', - 'tags_introduction' => 'Usually tags are singular words, designed to quickly band items together using things like expensive, bill or for-party. In Firefly III, tags can have more properties such as a date, description and location. This allows you to join transactions together in a more meaningful way. For example, you could make a tag called Christmas dinner with friends and add information about the restaurant. Such tags are "singular", you would only use them for a single occasion, perhaps with multiple transactions.', - 'tags_group' => 'Tags group transactions together, which makes it possible to store reimbursements (in case you front money for others) and other "balancing acts" where expenses are summed up (the payments on your new TV) or where expenses and deposits are cancelling each other out (buying something with saved money). It\'s all up to you. Using tags the old-fashioned way is of course always possible.', - 'tags_start' => 'Create a tag to get started or enter tags when creating new transactions.', 'transaction_journal_information' => 'Transaction information', 'transaction_journal_meta' => 'Meta information', diff --git a/resources/views/tags/create.twig b/resources/views/tags/create.twig index bf78d6c3c6..3c0aa3c447 100644 --- a/resources/views/tags/create.twig +++ b/resources/views/tags/create.twig @@ -15,7 +15,6 @@
{{ ExpandedForm.text('tag') }} - {{ ExpandedForm.multiRadio('tagMode',tagOptions) }}
diff --git a/resources/views/tags/edit.twig b/resources/views/tags/edit.twig index dcf7557d85..0a6bfdff15 100644 --- a/resources/views/tags/edit.twig +++ b/resources/views/tags/edit.twig @@ -17,7 +17,6 @@
{{ ExpandedForm.text('tag') }} - {{ ExpandedForm.multiRadio('tagMode',tagOptions) }}
diff --git a/resources/views/tags/index.twig b/resources/views/tags/index.twig index b92d2e51e9..6d1531cd7a 100644 --- a/resources/views/tags/index.twig +++ b/resources/views/tags/index.twig @@ -5,71 +5,55 @@ {% endblock %} {% block content %} -
-
-
-
-

{{ 'tags'|_ }}

-
-
-

- {{ 'tags_introduction'|_ }} -

- -

- {{ 'tags_group'|_ }} -

- -

- {{ 'tags_start'|_ }} -

-
-
- {{ 'new_tag'|_ }} -
-
-
-
{% if count == 0 %} {% include 'partials.empty' with {what: 'default', type: 'tags',route: route('tags.create')} %} {% else %}
{% for type in types %} -
-
-
-

{{ ('tag_title_'~type)|_ }}

-
-
- {% for year,months in collection[type] %} -

{{ year }}

+ {% if counts[type] > 0 %} +
+
+
+

{{ ('tag_title_'~type)|_ }}

+
+
+ {% for year,months in collection[type] %} +

{{ year }}

- {% for month,tags in months %} -
{{ month }}
-

- {% for tag in tags %} - + {% for month,tags in months %} +

{{ month }}
+

+ {% for tag in tags %} + {% if tag.tagMode == 'nothing' %} {% endif %} - {% if tag.tagMode == 'balancingAct' %} - - {% endif %} - {% if tag.tagMode == 'advancePayment' %} - - {% endif %} - {{ tag.tag }} + {% if tag.tagMode == 'balancingAct' %} + + {% endif %} + {% if tag.tagMode == 'advancePayment' %} + + {% endif %} + {{ tag.tag }} - {% endfor %} -

+ {% endfor %} +

+ {% endfor %} {% endfor %} - {% endfor %} +
+
+ {{ 'new_tag'|_ }} +
-
+ {% endif %} {% endfor %}
{% endif %}