From e95273b72b4c4ee180b705cb9d90d7d87911c636 Mon Sep 17 00:00:00 2001 From: James Cole Date: Sun, 7 Jun 2015 09:09:27 +0200 Subject: [PATCH] Simplified (or tried to) some code. [skip ci] --- app/Http/Controllers/TagController.php | 1 + app/Models/Tag.php | 2 +- app/Models/TransactionJournal.php | 115 +++++++++++-------- app/Support/Twig/Journal.php | 31 ++--- app/Validation/FireflyValidator.php | 2 +- resources/twig/list/journals.twig | 2 +- tests/controllers/TagControllerTest.php | 18 +++ tests/models/TransactionJournalModelTest.php | 2 +- 8 files changed, 108 insertions(+), 65 deletions(-) diff --git a/app/Http/Controllers/TagController.php b/app/Http/Controllers/TagController.php index ad3423fb1e..6b000256b7 100644 --- a/app/Http/Controllers/TagController.php +++ b/app/Http/Controllers/TagController.php @@ -270,6 +270,7 @@ class TagController extends Controller 'tagMode' => $request->get('tagMode'), ]; + $repository->update($tag, $data); Session::flash('success', 'Tag "' . e($data['tag']) . '" updated.'); diff --git a/app/Models/Tag.php b/app/Models/Tag.php index ac566d2b8d..86e03e0ccf 100644 --- a/app/Models/Tag.php +++ b/app/Models/Tag.php @@ -44,7 +44,7 @@ class Tag extends Model protected $fillable = ['user_id', 'tag', 'date', 'description', 'longitude', 'latitude', 'zoomLevel', 'tagMode']; protected $rules = [ - 'tag' => 'required|min:1|uniqueObjectForUser:tags,tag', + 'tag' => 'required|min:1', 'description' => 'min:1', 'date' => 'date', 'latitude' => 'numeric|min:-90|max:90', diff --git a/app/Models/TransactionJournal.php b/app/Models/TransactionJournal.php index 360429b436..5b78595ec5 100644 --- a/app/Models/TransactionJournal.php +++ b/app/Models/TransactionJournal.php @@ -146,7 +146,6 @@ class TransactionJournal extends Model return $cache->get(); // @codeCoverageIgnore } - $amount = '0'; bcscale(2); /** @var Transaction $t */ @@ -155,59 +154,61 @@ class TransactionJournal extends Model $amount = $t->amount; } } + $count = $this->tags->count(); - /* - * If the journal has tags, it gets complicated. - */ - if ($this->tags->count() == 0) { - $cache->store($amount); - - return $amount; + if ($count === 1) { + // get amount for single tag: + $amount = $this->amountByTag($this->tags()->first(), $amount); } - // if journal is part of advancePayment AND journal is a withdrawal, - // then journal is being repaid by other journals, so the actual amount will lower: - /** @var Tag $advancePayment */ - $advancePayment = $this->tags()->where('tagMode', 'advancePayment')->first(); - if ($advancePayment && $this->transactionType->type == 'Withdrawal') { - // loop other deposits, remove from our amount. - $others = $advancePayment->transactionJournals()->transactionTypes(['Deposit'])->get(); - foreach ($others as $other) { - $amount = bcsub($amount, $other->actual_amount); - } - $cache->store($amount); + if ($count > 1) { + // get amount for either tag. + $amount = $this->amountByTags($amount); - return $amount; } - - // if this journal is part of an advancePayment AND the journal is a deposit, - // then the journal amount is correcting a withdrawal, and the amount is zero: - if ($advancePayment && $this->transactionType->type == 'Deposit') { - $cache->store('0'); - - return '0'; - } - - - // is balancing act? - $balancingAct = $this->tags()->where('tagMode', 'balancingAct')->first(); - - if ($balancingAct) { - // this is the expense: - if ($this->transactionType->type == 'Withdrawal') { - $transfer = $balancingAct->transactionJournals()->transactionTypes(['Transfer'])->first(); - if ($transfer) { - $amount = bcsub($amount, $transfer->actual_amount); - $cache->store($amount); - - return $amount; - } - } // @codeCoverageIgnore - } // @codeCoverageIgnore - $cache->store($amount); return $amount; + + } + + /** + * Assuming the journal has only one tag. Parameter amount is used as fallback. + * + * @param Tag $tag + * @param string $amount + * + * @return string + */ + protected function amountByTag(Tag $tag, $amount) + { + if ($tag->tagMode == 'advancePayment') { + if ($this->transactionType->type == 'Withdrawal') { + $others = $tag->transactionJournals()->transactionTypes(['Deposit'])->get(); + foreach ($others as $other) { + $amount = bcsub($amount, $other->actual_amount); + } + + return $amount; + } + if ($this->transactionType->type == 'Deposit') { + return '0'; + } + } + + if ($tag->tagMode == 'balancingAct') { + if ($this->transactionType->type == 'Withdrawal') { + $transfer = $tag->transactionJournals()->transactionTypes(['Transfer'])->first(); + if ($transfer) { + $amount = bcsub($amount, $transfer->actual_amount); + + return $amount; + } + } + } + + return $amount; + } /** @@ -219,6 +220,26 @@ class TransactionJournal extends Model return $this->belongsToMany('FireflyIII\Models\Tag'); } + /** + * @param string $amount + * + * @return string + */ + public function amountByTags($amount) + { + $firstBalancingAct = $this->tags()->where('tagMode', 'balancingAct')->first(); + if ($firstBalancingAct) { + return $this->amountByTag($firstBalancingAct, $amount); + } + + $firstAdvancePayment = $this->tags()->where('tagMode', 'advancePayment')->first(); + if ($firstAdvancePayment) { + return $this->amountByTag($firstAdvancePayment, $amount); + } + + return $amount; + } + /** * @return string */ @@ -232,7 +253,7 @@ class TransactionJournal extends Model return $this->transactions()->where('amount', '<', 0)->first()->amount; } - return '0'; + return $this->transactions()->where('amount', '>', 0)->first()->amount; } /** diff --git a/app/Support/Twig/Journal.php b/app/Support/Twig/Journal.php index 0611e1d6db..f1e327e6be 100644 --- a/app/Support/Twig/Journal.php +++ b/app/Support/Twig/Journal.php @@ -187,23 +187,26 @@ class Journal extends Twig_Extension return $string; } - if ($tag->tagMode == 'advancePayment' && $journal->transactionType->type == 'Deposit') { - $amount = App::make('amount')->formatJournal($journal, false); - $string = ' ' . $tag->tag . ''; + if ($tag->tagMode == 'advancePayment') { + if ($journal->transactionType->type == 'Deposit') { + $amount = App::make('amount')->formatJournal($journal, false); + $string = ' ' . $tag->tag . ''; - return $string; - } - /* - * AdvancePayment with a withdrawal will show the amount with a link to - * the tag. The TransactionJournal should properly calculate the amount. - */ - if ($tag->tagMode == 'advancePayment' && $journal->transactionType->type == 'Withdrawal') { - $amount = App::make('amount')->formatJournal($journal); + return $string; + } - $string = '' . $amount . ''; + /* + * AdvancePayment with a withdrawal will show the amount with a link to + * the tag. The TransactionJournal should properly calculate the amount. + */ + if ($journal->transactionType->type == 'Withdrawal') { + $amount = App::make('amount')->formatJournal($journal); - return $string; + $string = '' . $amount . ''; + + return $string; + } } diff --git a/app/Validation/FireflyValidator.php b/app/Validation/FireflyValidator.php index fbb4c8e0bc..120b757542 100644 --- a/app/Validation/FireflyValidator.php +++ b/app/Validation/FireflyValidator.php @@ -239,7 +239,7 @@ class FireflyValidator extends Validator // exclude? $table = $parameters[0]; $field = $parameters[1]; - $exclude = isset($parameters[3]) ? intval($parameters[3]) : 0; + $exclude = isset($parameters[2]) ? intval($parameters[2]) : 0; // get entries from table $set = DB::table($table)->where('user_id', Auth::user()->id)->where('id', '!=', $exclude)->get([$field]); diff --git a/resources/twig/list/journals.twig b/resources/twig/list/journals.twig index f6e737965c..4c020a16c5 100644 --- a/resources/twig/list/journals.twig +++ b/resources/twig/list/journals.twig @@ -57,7 +57,7 @@ {% if not hideTags %} {{ relevantTags(journal)|raw }} {% else %} - {{ journal.correctAmount|formatAmount }} + {{ journal.correct_amount|formatAmount }} {% endif %} diff --git a/tests/controllers/TagControllerTest.php b/tests/controllers/TagControllerTest.php index 52840a7f50..9b6a29df69 100644 --- a/tests/controllers/TagControllerTest.php +++ b/tests/controllers/TagControllerTest.php @@ -248,6 +248,24 @@ class TagControllerTest extends TestCase $this->call('POST', '/tags/update/' . $tag->id, $data); $this->assertResponseStatus(302); + $this->assertSessionHas('success'); + } + + public function testUpdateNoNameChange() + { + $tag = FactoryMuffin::create('FireflyIII\Models\Tag'); + $this->be($tag->user); + + $data = [ + '_token' => 'replaceMe', + 'tag' => $tag->tag, + 'tagMode' => 'nothing', + 'id' => $tag->id, + ]; + + $this->call('POST', '/tags/update/' . $tag->id, $data); + $this->assertResponseStatus(302); + $this->assertSessionHas('success'); } /** diff --git a/tests/models/TransactionJournalModelTest.php b/tests/models/TransactionJournalModelTest.php index 307584cb84..fb1f9256d3 100644 --- a/tests/models/TransactionJournalModelTest.php +++ b/tests/models/TransactionJournalModelTest.php @@ -329,7 +329,7 @@ class TransactionJournalModelTest extends TestCase // get asset account: $result = $journal->correct_amount; - $this->assertEquals('0', $result); + $this->assertEquals('300', $result); } /**