From 5707dc7579528a14808ed2fb1763cbad26a1dba0 Mon Sep 17 00:00:00 2001 From: James Cole Date: Fri, 2 Jan 2015 12:38:13 +0100 Subject: [PATCH] Lots of cleaning up. --- .gitignore | 1 + app/controllers/BillController.php | 7 ++ app/controllers/PiggybankController.php | 4 +- app/controllers/TransactionController.php | 2 +- .../FireflyIII/Database/Account/Account.php | 7 -- app/lib/FireflyIII/Database/Bill/Bill.php | 67 +++++++++++++ .../Database/Bill/BillInterface.php | 14 +++ app/lib/FireflyIII/Shared/Toolkit/Steam.php | 1 - app/models/Bill.php | 67 ------------- app/models/Piggybank.php | 21 ---- app/models/TransactionJournal.php | 2 - app/routes.php | 4 +- app/views/bills/index.blade.php | 1 - app/views/bills/show.blade.php | 5 +- app/views/categories/show.blade.php | 1 - app/views/list/bills.blade.php | 10 +- app/views/piggy_banks/show.blade.php | 6 +- bootstrap/start.php | 2 +- .../javascript/firefly/related-manager.js | 1 - tests/functional/RelatedControllerCest.php | 99 +++++++++++++++++++ 20 files changed, 201 insertions(+), 121 deletions(-) create mode 100644 tests/functional/RelatedControllerCest.php diff --git a/.gitignore b/.gitignore index 4224919aae..90cd6c6a45 100644 --- a/.gitignore +++ b/.gitignore @@ -24,3 +24,4 @@ tests/_data/db.sqlite tests/_data/dump.sql db.sqlite_snapshot c3.php +db.sqlite-journal diff --git a/app/controllers/BillController.php b/app/controllers/BillController.php index ef25c3ffff..521c21736b 100644 --- a/app/controllers/BillController.php +++ b/app/controllers/BillController.php @@ -83,6 +83,12 @@ class BillController extends BaseController public function index() { $bills = $this->_repository->get(); + $bills->each( + function (Bill $bill) { + $bill->nextExpectedMatch = $this->_repository->nextExpectedMatch($bill); + $bill->lastFoundMatch = $this->_repository->lastFoundMatch($bill); + } + ); return View::make('bills.index', compact('bills')); } @@ -115,6 +121,7 @@ class BillController extends BaseController public function show(Bill $bill) { $journals = $bill->transactionjournals()->withRelevantData()->orderBy('date', 'DESC')->get(); + $bill->nextExpectedMatch = $this->_repository->nextExpectedMatch($bill); $hideBill = true; diff --git a/app/controllers/PiggybankController.php b/app/controllers/PiggybankController.php index 58924755ae..ce7da9f704 100644 --- a/app/controllers/PiggybankController.php +++ b/app/controllers/PiggybankController.php @@ -262,11 +262,9 @@ class PiggyBankController extends BaseController * Number of reminders: */ - $amountPerReminder = $piggyBank->amountPerReminder(); - $remindersCount = $piggyBank->countFutureReminders(); $subTitle = e($piggyBank->name); - return View::make('piggy_banks.show', compact('amountPerReminder', 'remindersCount', 'piggyBank', 'events', 'subTitle')); + return View::make('piggy_banks.show', compact('piggyBank', 'events', 'subTitle')); } diff --git a/app/controllers/TransactionController.php b/app/controllers/TransactionController.php index d62fe5a3cb..a7c9b12e71 100644 --- a/app/controllers/TransactionController.php +++ b/app/controllers/TransactionController.php @@ -249,7 +249,7 @@ class TransactionController extends BaseController $data['transaction_currency_id'] = $transactionCurrency->id; $data['completed'] = 0; $data['what'] = $what; - $data['currency'] = 'EUR'; // TODO allow custom currency + $data['currency'] = 'EUR'; // always validate: $messages = $this->_repository->validate($data); diff --git a/app/lib/FireflyIII/Database/Account/Account.php b/app/lib/FireflyIII/Database/Account/Account.php index 0cfa42dd03..661db8d40c 100644 --- a/app/lib/FireflyIII/Database/Account/Account.php +++ b/app/lib/FireflyIII/Database/Account/Account.php @@ -239,7 +239,6 @@ class Account implements CUDInterface, CommonDatabaseCallsInterface, AccountInte $q->orWhere( function (QueryBuilder $q) use ($model) { $q->where('accounts.name', 'LIKE', '%' . $model->name . '%'); - // TODO magic number! $q->where('accounts.account_type_id', 3); $q->where('accounts.active', 0); } @@ -280,7 +279,6 @@ class Account implements CUDInterface, CommonDatabaseCallsInterface, AccountInte $q->orWhere( function (EloquentBuilder $q) use ($model) { $q->where('accounts.name', 'LIKE', '%' . $model->name . '%'); - // TODO magic number! $q->where('accounts.account_type_id', 3); $q->where('accounts.active', 0); } @@ -324,7 +322,6 @@ class Account implements CUDInterface, CommonDatabaseCallsInterface, AccountInte $this->storeInitialBalance($account, $data); } - // TODO this needs cleaning up and thinking over. switch ($account->accountType->type) { case 'Asset account': case 'Default account': @@ -352,7 +349,6 @@ class Account implements CUDInterface, CommonDatabaseCallsInterface, AccountInte $model->name = $data['name']; $model->active = isset($data['active']) ? intval($data['active']) : 0; - // TODO this needs cleaning up and thinking over. switch ($model->accountType->type) { case 'Asset account': case 'Default account': @@ -365,7 +361,6 @@ class Account implements CUDInterface, CommonDatabaseCallsInterface, AccountInte if (isset($data['openingbalance']) && isset($data['openingbalancedate']) && strlen($data['openingbalancedate']) > 0) { /** @noinspection PhpParamsInspection */ $openingBalance = $this->openingBalanceTransaction($model); - // TODO this needs cleaning up and thinking over. if (is_null($openingBalance)) { $this->storeInitialBalance($model, $data); } else { @@ -483,7 +478,6 @@ class Account implements CUDInterface, CommonDatabaseCallsInterface, AccountInte */ public function findByWhat($what) { - // TODO: Implement findByWhat() method. throw new NotImplementedException; } @@ -495,7 +489,6 @@ class Account implements CUDInterface, CommonDatabaseCallsInterface, AccountInte */ public function get() { - // TODO: Implement get() method. throw new NotImplementedException; } diff --git a/app/lib/FireflyIII/Database/Bill/Bill.php b/app/lib/FireflyIII/Database/Bill/Bill.php index 65133d8441..6e17ffe827 100644 --- a/app/lib/FireflyIII/Database/Bill/Bill.php +++ b/app/lib/FireflyIII/Database/Bill/Bill.php @@ -202,6 +202,73 @@ class Bill implements CUDInterface, CommonDatabaseCallsInterface, BillInterface } + /** + * @param \Bill $bill + * + * @return Carbon|null + */ + public function lastFoundMatch(\Bill $bill) + { + $last = $bill->transactionjournals()->orderBy('date', 'DESC')->first(); + if ($last) { + return $last->date; + } + + return null; + } + + /** + * @param \Bill $bill + * + * @return Carbon|null + */ + public function nextExpectedMatch(\Bill $bill) + { + /* + * The date Firefly tries to find. If this stays null, it's "unknown". + */ + $finalDate = null; + if ($bill->active == 0) { + return $finalDate; + } + + /* + * $today is the start of the next period, to make sure FF3 won't miss anything + * when the current period has a transaction journal. + */ + $today = \DateKit::addPeriod(new Carbon, $bill->repeat_freq, 0); + + /* + * FF3 loops from the $start of the bill, and to make sure + * $skip works, it adds one (for modulo). + */ + $skip = $bill->skip + 1; + $start = \DateKit::startOfPeriod(new Carbon, $bill->repeat_freq); + /* + * go back exactly one month/week/etc because FF3 does not care about 'next' + * bills if they're too far into the past. + */ + + $counter = 0; + while ($start <= $today) { + if (($counter % $skip) == 0) { + // do something. + $end = \DateKit::endOfPeriod(clone $start, $bill->repeat_freq); + $journalCount = $bill->transactionjournals()->before($end)->after($start)->count(); + if ($journalCount == 0) { + $finalDate = clone $start; + break; + } + } + + // add period for next round! + $start = \DateKit::addPeriod($start, $bill->repeat_freq, 0); + $counter++; + } + + return $finalDate; + } + /** * @param \Bill $bill * @param \TransactionJournal $journal diff --git a/app/lib/FireflyIII/Database/Bill/BillInterface.php b/app/lib/FireflyIII/Database/Bill/BillInterface.php index 6c6a247364..f6dca6325f 100644 --- a/app/lib/FireflyIII/Database/Bill/BillInterface.php +++ b/app/lib/FireflyIII/Database/Bill/BillInterface.php @@ -23,6 +23,20 @@ interface BillInterface */ public function getJournalForBillInRange(\Bill $bill, Carbon $start, Carbon $end); + /** + * @param \Bill $bill + * + * @return Carbon|null + */ + public function lastFoundMatch(\Bill $bill); + + /** + * @param \Bill $bill + * + * @return Carbon|null + */ + public function nextExpectedMatch(\Bill $bill); + /** * @param \Bill $bill * @param \TransactionJournal $journal diff --git a/app/lib/FireflyIII/Shared/Toolkit/Steam.php b/app/lib/FireflyIII/Shared/Toolkit/Steam.php index 03f644bafc..2a2696372f 100644 --- a/app/lib/FireflyIII/Shared/Toolkit/Steam.php +++ b/app/lib/FireflyIII/Shared/Toolkit/Steam.php @@ -16,7 +16,6 @@ class Steam { /** - * TODO find a way to reliably remove cache entries for accounts. * * @param \Account $account * @param Carbon $date diff --git a/app/models/Bill.php b/app/models/Bill.php index 586d1146c1..00d7d2c1e4 100644 --- a/app/models/Bill.php +++ b/app/models/Bill.php @@ -30,21 +30,6 @@ class Bill extends Eloquent return ['created_at', 'updated_at', 'date']; } - /** - * TODO remove this method in favour of something in the FireflyIII libraries. - * - * @return null - */ - public function lastFoundMatch() - { - $last = $this->transactionjournals()->orderBy('date', 'DESC')->first(); - if ($last) { - return $last->date; - } - - return null; - } - /** * @return \Illuminate\Database\Eloquent\Relations\HasMany */ @@ -54,58 +39,6 @@ class Bill extends Eloquent } - /** - * TODO remove this method in favour of something in the FireflyIII libraries. - * - * Find the next expected match based on the set journals and the date stuff from the bill. - */ - public function nextExpectedMatch() - { - - /* - * The date Firefly tries to find. If this stays null, it's "unknown". - */ - $finalDate = null; - if ($this->active == 0) { - return $finalDate; - } - - /* - * $today is the start of the next period, to make sure FF3 won't miss anything - * when the current period has a transaction journal. - */ - $today = DateKit::addPeriod(new Carbon, $this->repeat_freq, 0); - - /* - * FF3 loops from the $start of the bill, and to make sure - * $skip works, it adds one (for modulo). - */ - $skip = $this->skip + 1; - $start = DateKit::startOfPeriod(new Carbon, $this->repeat_freq); - /* - * go back exactly one month/week/etc because FF3 does not care about 'next' - * bills if they're too far into the past. - */ - - $counter = 0; - while ($start <= $today) { - if (($counter % $skip) == 0) { - // do something. - $end = DateKit::endOfPeriod(clone $start, $this->repeat_freq); - $journalCount = $this->transactionjournals()->before($end)->after($start)->count(); - if ($journalCount == 0) { - $finalDate = clone $start; - break; - } - } - - // add period for next round! - $start = DateKit::addPeriod($start, $this->repeat_freq, 0); - $counter++; - } - - return $finalDate; - } /** * @return \Illuminate\Database\Eloquent\Relations\BelongsTo diff --git a/app/models/Piggybank.php b/app/models/Piggybank.php index c3ba7e0054..1e375b7dee 100644 --- a/app/models/Piggybank.php +++ b/app/models/Piggybank.php @@ -37,27 +37,6 @@ class PiggyBank extends Eloquent return $this->belongsTo('Account'); } - /** - * TODO remove this method in favour of something in the FireflyIII libraries. - * - * @return int - */ - public function amountPerReminder() - { - return 0; - - } - - /** - * TODO remove this method in favour of something in the FireflyIII libraries. - * - * @return int - */ - public function countFutureReminders() - { - return 0; - } - /** * TODO remove this method in favour of something in the FireflyIII libraries. * diff --git a/app/models/TransactionJournal.php b/app/models/TransactionJournal.php index e39a8a4c81..ffbd0f32e5 100644 --- a/app/models/TransactionJournal.php +++ b/app/models/TransactionJournal.php @@ -70,8 +70,6 @@ class TransactionJournal extends Eloquent return floatval($t->amount); } } - - return -0.01; } /** diff --git a/app/routes.php b/app/routes.php index 4e37cce682..9513bb3e20 100644 --- a/app/routes.php +++ b/app/routes.php @@ -258,11 +258,11 @@ Route::group( Route::get('/profile/change-password', ['uses' => 'ProfileController@changePassword', 'as' => 'change-password']); // related controller: + Route::get('/related/alreadyRelated/{tj}', ['uses' => 'RelatedController@alreadyRelated','as' => 'related.alreadyRelated']); + Route::post('/related/relate/{tj}/{tjSecond}', ['uses' => 'RelatedController@relate','as' => 'related.relate']); Route::get('/related/removeRelation/{tj}/{tjSecond}', ['uses' => 'RelatedController@removeRelation','as' => 'related.removeRelation']); Route::get('/related/related/{tj}', ['uses' => 'RelatedController@related','as' => 'related.related']); Route::post('/related/search/{tj}', ['uses' => 'RelatedController@search','as' => 'related.search']); - Route::post('/related/relate/{tj}/{tjSecond}', ['uses' => 'RelatedController@relate','as' => 'related.relate']); - Route::get('/related/alreadyRelated/{tj}', ['uses' => 'RelatedController@alreadyRelated','as' => 'related.alreadyRelated']); // bills controller Route::get('/bills', ['uses' => 'BillController@index', 'as' => 'bills.index']); diff --git a/app/views/bills/index.blade.php b/app/views/bills/index.blade.php index 3e1b231ec4..c9319ac213 100644 --- a/app/views/bills/index.blade.php +++ b/app/views/bills/index.blade.php @@ -6,7 +6,6 @@
{{{$title}}} -
@include('list.bills') diff --git a/app/views/bills/show.blade.php b/app/views/bills/show.blade.php index b4ad8d9e1e..6437e6fdcb 100644 --- a/app/views/bills/show.blade.php +++ b/app/views/bills/show.blade.php @@ -49,9 +49,8 @@ Next expected match - nextExpectedMatch();?> - @if($nextExpectedMatch) - {{$nextExpectedMatch->format('j F Y')}} + @if($bill->nextExpectedMatch) + {{$bill->nextExpectedMatch->format('j F Y')}} @else Unknown @endif diff --git a/app/views/categories/show.blade.php b/app/views/categories/show.blade.php index 37fa8d0e2d..bdfefd3e52 100644 --- a/app/views/categories/show.blade.php +++ b/app/views/categories/show.blade.php @@ -22,7 +22,6 @@
- BLa bla something here.
diff --git a/app/views/list/bills.blade.php b/app/views/list/bills.blade.php index a603bd9617..1540af42de 100644 --- a/app/views/list/bills.blade.php +++ b/app/views/list/bills.blade.php @@ -32,17 +32,15 @@ {{Amount::format($entry->amount_max)}} - lastFoundMatch();?> - @if($lastMatch) - {{$lastMatch->format('j F Y')}} + @if($entry->lastFoundMatch) + {{$entry->lastFoundMatch->format('j F Y')}} @else Unknown @endif - nextExpectedMatch();?> - @if($nextExpectedMatch) - {{$nextExpectedMatch->format('j F Y')}} + @if($entry->nextExpectedMatch) + {{$entry->nextExpectedMatch->format('j F Y')}} @else Unknown @endif diff --git a/app/views/piggy_banks/show.blade.php b/app/views/piggy_banks/show.blade.php index 47c4e86327..f11b4a74ac 100644 --- a/app/views/piggy_banks/show.blade.php +++ b/app/views/piggy_banks/show.blade.php @@ -88,16 +88,14 @@ @endif - @if($remindersCount > 0) Reminders left - {{$remindersCount}} + (in progress...) Expected amount per reminder - {{Amount::format($amountPerReminder)}} + (in progress...) - @endif diff --git a/bootstrap/start.php b/bootstrap/start.php index 1b7c257703..30dc7625b0 100644 --- a/bootstrap/start.php +++ b/bootstrap/start.php @@ -96,6 +96,6 @@ Event::subscribe('FireflyIII\Event\TransactionJournal'); // see if the various has-many-throughs actually get used. // set very tight rules on all models // create custom uniquely rules. -// TODO add "Create new X" button to any list there is: categories, accounts, piggies, etc. +// add "Create new X" button to any list there is: categories, accounts, piggies, etc. // Install PHP5 and code thing and create very small methods. return $app; diff --git a/public/assets/javascript/firefly/related-manager.js b/public/assets/javascript/firefly/related-manager.js index 923afb5147..2c3e55f8af 100644 --- a/public/assets/javascript/firefly/related-manager.js +++ b/public/assets/javascript/firefly/related-manager.js @@ -45,7 +45,6 @@ function searchRelatedTransactions(e, ID) { $.post('related/search/' + ID, {searchValue: searchValue}).success(function (data) { // post each result to some div. $('#relatedSearchResults').empty(); - // TODO this is the worst. $.each(data, function (i, row) { var tr = $(''); diff --git a/tests/functional/RelatedControllerCest.php b/tests/functional/RelatedControllerCest.php new file mode 100644 index 0000000000..629ee5e975 --- /dev/null +++ b/tests/functional/RelatedControllerCest.php @@ -0,0 +1,99 @@ +amLoggedAs(['email' => 'thegrumpydictator@gmail.com', 'password' => 'james']); + + } + + public function alreadyRelated(FunctionalTester $I) + { + $group = TransactionGroup::first(); + $journal = $group->transactionjournals()->first(); + + $I->wantTo('see already related transactions'); + $I->amOnPage('/related/alreadyRelated/' . $journal->id); + $I->see('Big expense in '); + + } + + public function alreadyRelatedNoRelations(FunctionalTester $I) + { + $journal = TransactionJournal::first(); + + $I->wantTo('see already related transactions for a journal without any'); + $I->amOnPage('/related/alreadyRelated/' . $journal->id); + $I->see('[]'); + + } + + public function relate(FunctionalTester $I) + { + $journal = TransactionJournal::leftJoin( + 'transaction_group_transaction_journal', 'transaction_journals.id', '=', 'transaction_group_transaction_journal.transaction_journal_id' + ) + ->whereNull('transaction_group_transaction_journal.transaction_group_id')->first(['transaction_journals.*']); + $otherJournal = TransactionJournal::leftJoin( + 'transaction_group_transaction_journal', 'transaction_journals.id', '=', 'transaction_group_transaction_journal.transaction_journal_id' + ) + ->whereNull('transaction_group_transaction_journal.transaction_group_id')->where( + 'transaction_journals.id', '!=', $journal->id + )->first( + ['transaction_journals.*'] + ); + $I->wantTo('relate two journals'); + $I->sendAjaxPostRequest('/related/relate/' . $journal->id . '/' . $otherJournal->id); + $I->see('true'); + } + + public function related(FunctionalTester $I) + { + $group = TransactionGroup::first(); + $journal = $group->transactionjournals()->first(); + + $I->wantTo('see the popup with already related transactions'); + $I->amOnPage('/related/related/' . $journal->id); + $I->see('Big expense in '); + } + + public function removeRelation(FunctionalTester $I) + { + $group = TransactionGroup::first(); + $one = $group->transactionjournals[0]; + $two = $group->transactionjournals[1]; + $I->wantTo('relate two journals'); + $I->amOnPage('/related/removeRelation/' . $one->id . '/' . $two->id); + $I->see('true'); + + } + + public function search(FunctionalTester $I) + { + $group = TransactionGroup::first(); + $one = $group->transactionjournals[0]; + + $I->wantTo('search for a transaction to relate'); + + $I->sendAjaxPostRequest('/related/search/' . $one->id . '?searchValue=expense'); + $I->see('Big expense in'); + } +} \ No newline at end of file