Some code cleanup and fixes.

app/controllers/AccountController.php

@@ -102,12 +102,7 @@ class AccountController extends \BaseController
 
         $account = $this->_repository->store(Input::all());
 
-        if (!$account->id) {
+        if ($account->validate()) {
-            // did not save, return with error:
-            Session::flash('error', 'Could not save the new account. Please check the form.');
-
-            return Redirect::route('accounts.create')->withErrors($account->errors())->withInput();
-        } else {
             // saved! return to wherever.
             Session::flash('success', 'Account "' . $account->name . '" created!');
             if (Input::get('create') == '1') {
@@ -115,6 +110,12 @@ class AccountController extends \BaseController
             } else {
                 return Redirect::route('accounts.index');
             }
+        } else {
+            // did not save, return with error:
+            Session::flash('error', 'Could not save the new account. Please check the form.');
+
+            return Redirect::route('accounts.create')->withErrors($account->errors())->withInput();
+
         }
     }
 

app/controllers/BudgetController.php

@@ -127,7 +127,7 @@ class BudgetController extends BaseController
     {
 
         $budget = $this->_repository->store(Input::all());
-        if ($budget->id) {
+        if ($budget->validate()) {
             Event::fire('budgets.change');
             Session::flash('success', 'Budget created!');
 

app/controllers/CategoryController.php

@@ -68,8 +68,8 @@ class CategoryController extends BaseController
     public function store()
     {
         $category = $this->_repository->store(Input::all());
-        if ($category->id) {
+        if ($category->validate()) {
-            Session::flash('success', 'Category created!');
+            Session::flash('success', 'Category "' . $category->name . '" created!');
 
             if (Input::get('create') == '1') {
                 return Redirect::route('categories.create');

app/controllers/ChartController.php

@@ -68,7 +68,8 @@ class ChartController extends BaseController
         // loop and get array data.
 
         $url = count($accounts) == 1 && is_array($accounts)
-            ? '<a href="' . route('accounts.show', [$account->id]) . '">View more</a>'  :
+            ? '<a href="' . route('accounts.show', [$account->id]) . '">View more</a>'
+            :
             '<a href="' . route('accounts.index') . '">View more</a>';
         $data = [
             'chart_title' => count($accounts) == 1 ? $accounts[0]->name : 'All accounts',

app/controllers/LimitController.php

@@ -86,16 +86,19 @@ class LimitController extends BaseController
 
         // find a limit with these properties, as we might already have one:
         $limit = $this->_limits->store(Input::all());
-        if ($limit->id) {
+        if ($limit->validate()) {
+            Session::flash('success', 'Envelope created!');
             if (Input::get('from') == 'date') {
                 return Redirect::route('budgets.index');
             } else {
                 return Redirect::route('budgets.index.budget');
             }
         } else {
+            Session::flash('success', 'Could not save new envelope.');
             $budgetId = $budget ? $budget->id : null;
-
+            $parameters = [$budgetId, 'from' => Input::get('from')];
-            return Redirect::route('budgets.limits.create', [$budgetId, 'from' => Input::get('from')])->withInput();
+            return Redirect::route('budgets.limits.create', $parameters)->withInput()
+                ->withErrors($limit->errors());
         }
     }
 
@@ -106,16 +109,13 @@ class LimitController extends BaseController
      */
     public function update(\Limit $limit)
     {
+        // TODO move logic to repository.
         /** @var \Limit $limit */
         $limit->startdate = new \Carbon\Carbon(Input::get('date'));
         $limit->repeat_freq = Input::get('period');
         $limit->repeats = !is_null(Input::get('repeats')) && Input::get('repeats') == '1' ? 1 : 0;
         $limit->amount = floatval(Input::get('amount'));
-        if (!$limit->save()) {
+        if ($limit->save()) {
-            Session::flash('error', 'Could not save new limit: ' . $limit->errors()->first());
-
-            return Redirect::route('budgets.limits.edit', [$limit->id, 'from' => Input::get('from')])->withInput();
-        } else {
             Session::flash('success', 'Limit saved!');
             foreach ($limit->limitrepetitions()->get() as $rep) {
                 $rep->delete();
@@ -125,6 +125,13 @@ class LimitController extends BaseController
             } else {
                 return Redirect::route('budgets.index.budget');
             }
+
+
+        } else {
+            Session::flash('error', 'Could not save new limit: ' . $limit->errors()->first());
+
+            return Redirect::route('budgets.limits.edit', [$limit->id, 'from' => Input::get('from')])->withInput()
+                ->withErrors($limit->errors());
         }
 
     }

app/controllers/PiggybankController.php

@@ -82,19 +82,20 @@ class PiggybankController extends BaseController
     public function store()
     {
         $piggyBank = $this->_repository->store(Input::all());
-        if (!$piggyBank->id) {
+        if ($piggyBank->validate()) {
-            Session::flash('error', 'Could not save piggy bank: ' . $piggyBank->errors()->first());
-
-            return Redirect::route('piggybanks.create')->withInput();
-        } else {
             Session::flash('success', 'New piggy bank "' . $piggyBank->name . '" created!');
 
-
             if (Input::get('create') == '1') {
                 return Redirect::route('piggybanks.create')->withInput();
             }
 
             return Redirect::route('piggybanks.index');
+
+
+        } else {
+            Session::flash('error', 'Could not save piggy bank: ' . $piggyBank->errors()->first());
+
+            return Redirect::route('piggybanks.create')->withInput();
         }
 
     }
@@ -103,9 +104,17 @@ class PiggybankController extends BaseController
     {
 
         $piggyBank = $this->_repository->update(Input::all());
+        if ($piggyBank->validate()) {
             Session::flash('success', 'Piggy bank "' . $piggyBank->name . '" updated.');
 
             return Redirect::route('piggybanks.index');
+        } else {
+            Session::flash('error', 'Could not update piggy bank: ' . $piggyBank->errors()->first());
+
+            return Redirect::route('piggybanks.edit', $piggyBank->id)->withErrors($piggyBank->errors())->withInput();
+        }
+
+
     }
 
     public function updateAmount(Piggybank $piggybank)

app/controllers/RecurringController.php

@@ -60,7 +60,7 @@ class RecurringController extends BaseController
     public function store()
     {
         $recurringTransaction = $this->_repository->store(Input::all());
-        if ($recurringTransaction->id) {
+        if ($recurringTransaction->validate()) {
             Session::flash('success', 'Recurring transaction "' . $recurringTransaction->name . '" saved!');
             if (Input::get('create') == '1') {
                 return Redirect::route('recurring.create')->withInput();

app/controllers/TransactionController.php

@@ -145,19 +145,19 @@ class TransactionController extends BaseController
      */
     public function store($what)
     {
-        $transactionJournal = $this->_repository->store($what, Input::all());
+        $journal = $this->_repository->store($what, Input::all());
-        if ($transactionJournal->id) {
+        if ($journal->validate()) {
-            Session::flash('success', 'Transaction "' . $transactionJournal->description . '" saved!');
+            Session::flash('success', 'Transaction "' . $journal->description . '" saved!');
             if (Input::get('create') == '1') {
                 return Redirect::route('transactions.create', [$what])->withInput();
             } else {
                 return Redirect::route('transactions.index');
             }
         } else {
-            Session::flash('error', 'Could not save transaction: ' . $transactionJournal->errors()->first());
+            Session::flash('error', 'Could not save transaction: ' . $journal->errors()->first());
 
             return Redirect::route('transactions.create', [$what])->withInput()->withErrors(
-                $transactionJournal->errors()
+                $journal->errors()
             );
         }
 

app/lib/Firefly/Storage/Account/EloquentAccountRepository.php

@@ -96,7 +96,9 @@ class EloquentAccountRepository implements AccountRepositoryInterface
     public function findByName($name, \AccountType $type = null)
     {
         $type = is_null($type) ? \AccountType::where('description', 'Default account')->first() : $type;
-        return \Auth::user()->accounts()->where('account_type_id',$type->id)->where('name', 'like', '%' . $name . '%')->first();
+
+        return \Auth::user()->accounts()->where('account_type_id', $type->id)->where('name', 'like', '%' . $name . '%')
+            ->first();
     }
 
     /**

app/lib/Firefly/Storage/RecurringTransaction/EloquentRecurringTransactionRepository.php

@@ -7,8 +7,10 @@ use Carbon\Carbon;
 
 class EloquentRecurringTransactionRepository implements RecurringTransactionRepositoryInterface
 {
-    public function destroy(\RecurringTransaction $recurringTransaction) {
+    public function destroy(\RecurringTransaction $recurringTransaction)
+    {
         $recurringTransaction->delete();
+
         return true;
     }
 
@@ -29,6 +31,7 @@ class EloquentRecurringTransactionRepository implements RecurringTransactionRepo
         // both amounts zero:
         if ($recurringTransaction->amount_max == 0 && $recurringTransaction->amount_min == 0) {
             $recurringTransaction->errors()->add('amount_max', 'Amount max and min cannot both be zero.');
+
             return $recurringTransaction;
         }
 
@@ -41,6 +44,7 @@ class EloquentRecurringTransactionRepository implements RecurringTransactionRepo
         if ($recurringTransaction->validate()) {
             $recurringTransaction->save();
         }
+
         return $recurringTransaction;
     }
 

app/lib/Firefly/Storage/User/EloquentUserRepository.php

@@ -69,6 +69,7 @@ class EloquentUserRepository implements UserRepositoryInterface
         if (!$user->save()) {
             \Log::error('Invalid user');
             \Session::flash('error', 'Input invalid, please try again: ' . $user->errors()->first());
+
             return false;
         }
         $user->save();

app/models/TransactionJournal.php

@@ -63,6 +63,14 @@ use LaravelBook\Ardent\Ardent;
  *             'Budget[] $budgets
  * @property-read \Illuminate\Database\Eloquent\Collection|\
  *             'Category[] $categories
+ * @property-read \Illuminate\Database\Eloquent\Collection|\
+ *             'Budget[] $budgets
+ * @property-read \Illuminate\Database\Eloquent\Collection|\
+ *             'Category[] $categories
+ * @property-read \Illuminate\Database\Eloquent\Collection|\
+ *             'Budget[] $budgets
+ * @property-read \Illuminate\Database\Eloquent\Collection|\
+ *             'Category[] $categories
  */
 class TransactionJournal extends Ardent
 {

app/tests/controllers/AccountControllerTest.php

@@ -163,7 +163,7 @@ class AccountControllerTest extends TestCase
     public function testStoreFails()
     {
         $account = f::create('Account');
-        unset($account->id);
+        unset($account->name);
         $this->_repository->shouldReceive('store')->andReturn($account);
         $this->action('POST', 'AccountController@store');
         $this->assertRedirectedToRoute('accounts.create');

app/tests/controllers/BudgetControllerTest.php

@@ -5,7 +5,8 @@ use Illuminate\Database\Eloquent\Collection;
 use Mockery as m;
 use Zizaco\FactoryMuff\Facade\FactoryMuff as f;
 
-class BudgetControllerTest extends TestCase {
+class BudgetControllerTest extends TestCase
+{
     protected $_repository;
     protected $_user;
     protected $_budgets;
@@ -140,7 +141,6 @@ class BudgetControllerTest extends TestCase {
         $this->_user->shouldReceive('getAttribute')->with('email')->once()->andReturn($budget->email);
 
 
-
         $this->session(['start' => new Carbon, 'end' => new Carbon]);
 
         $this->_budgets->shouldReceive('organizeRepetitions')->once()->andReturn([]);
@@ -159,7 +159,6 @@ class BudgetControllerTest extends TestCase {
         $this->_user->shouldReceive('getAttribute')->with('email')->once()->andReturn($budget->email);
 
 
-
         $this->session(['start' => new Carbon, 'end' => new Carbon]);
 
         $this->_budgets->shouldReceive('outsideRepetitions')->once()->andReturn([]);
@@ -178,7 +177,6 @@ class BudgetControllerTest extends TestCase {
         $this->_user->shouldReceive('getAttribute')->with('email')->once()->andReturn($budget->email);
 
 
-
         $this->session(['start' => new Carbon, 'end' => new Carbon]);
 
 //        $this->_budgets->shouldReceive('show')->once()->andReturn([]);
@@ -195,6 +193,7 @@ class BudgetControllerTest extends TestCase {
         $this->action('POST', 'BudgetController@store');
         $this->assertRedirectedToRoute('budgets.index.budget');
     }
+
     public function testStoreFromDate()
     {
         $budget = f::create('Budget');
@@ -206,7 +205,7 @@ class BudgetControllerTest extends TestCase {
     public function testStoreFails()
     {
         $budget = f::create('Budget');
-        unset($budget->id);
+        unset($budget->name);
         $this->_repository->shouldReceive('store')->andReturn($budget);
         $this->action('POST', 'BudgetController@store', ['from' => 'budget']);
         $this->assertRedirectedToRoute('budgets.create');

app/tests/controllers/CategoryControllerTest.php

@@ -118,8 +118,6 @@ class CategoryControllerTest extends TestCase
         $this->session(['start' => new Carbon, 'end' => new Carbon]);
 
 
-
-
         $this->_category->shouldReceive('journalsInRange')->once()->andReturn([]);
         $this->action('GET', 'CategoryController@show', $category->id);
         $this->assertResponseOk();
@@ -136,7 +134,7 @@ class CategoryControllerTest extends TestCase
     public function testStoreFails()
     {
         $category = f::create('Category');
-        unset($category->id);
+        unset($category->name);
         $this->_repository->shouldReceive('store')->andReturn($category);
         $this->action('POST', 'CategoryController@store');
         $this->assertRedirectedToRoute('categories.create');

app/tests/controllers/JsonControllerTest.php

@@ -1,5 +1,6 @@
 <?php
 use Zizaco\FactoryMuff\Facade\FactoryMuff as f;
+
 /**
  * Class JsonControllerTest
  */

app/tests/controllers/LimitControllerTest.php

@@ -126,6 +126,7 @@ class LimitControllerTest extends TestCase
 
         $this->_limits->shouldReceive('store')->once()->andReturn($limit);
         $this->action('POST', 'LimitController@store');
+        $this->assertRedirectedToRoute('budgets.index.budget');
         $this->assertResponseStatus(302);
     }
 
@@ -137,10 +138,12 @@ class LimitControllerTest extends TestCase
         $limit->save();
         $limitrepetition = f::create('LimitRepetition');
         $limit->limitrepetitions()->save($limitrepetition);
-        unset($limit->id);
+        unset($limit->startdate);
+        unset($limit->component_id);
+
 
         $this->_limits->shouldReceive('store')->once()->andReturn($limit);
-        $this->action('POST', 'LimitController@store', [$budget->id, 'from' => 'date']);
+        $this->action('POST', 'LimitController@store', $budget->id);
         $this->assertResponseStatus(302);
     }
 

app/tests/controllers/PiggybankControllerTest.php

@@ -131,7 +131,7 @@ class PiggybankControllerTest extends TestCase
     public function testStoreFails()
     {
         $piggyBank = f::create('Piggybank');
-        unset($piggyBank->id);
+        unset($piggyBank->amount);
         $this->_piggybanks->shouldReceive('store')->andReturn($piggyBank);
         $this->action('POST', 'PiggybankController@store');
         $this->assertResponseStatus(302);
@@ -163,6 +163,25 @@ class PiggybankControllerTest extends TestCase
         $this->assertResponseStatus(302);
     }
 
+    public function testUpdateFails()
+    {
+        $piggyBank = f::create('Piggybank');
+        unset($piggyBank->amount);
+
+        $this->_piggybanks->shouldReceive('update')->andReturn($piggyBank);
+
+        // for binding
+        Auth::shouldReceive('user')->andReturn($this->_user);
+        Auth::shouldReceive('check')->andReturn(true);
+        $this->_user->shouldReceive('getAttribute')->with('id')->andReturn(
+            $piggyBank->account()->first()->user_id
+        );
+        $this->_user->shouldReceive('getAttribute')->with('email')->andReturn('some@email');
+
+        $this->action('POST', 'PiggybankController@update', $piggyBank->id);
+        $this->assertResponseStatus(302);
+    }
+
     public function testUpdateAmount()
     {
         $piggyBank = f::create('Piggybank');

app/tests/controllers/RecurringControllerTest.php

@@ -137,7 +137,8 @@ class RecurringControllerTest extends TestCase
     public function testStoreFails()
     {
         $recurringTransaction = f::create('RecurringTransaction');
-        unset($recurringTransaction->id);
+        unset($recurringTransaction->active);
+        unset($recurringTransaction->automatch);
 
         $this->_repository->shouldReceive('store')->andReturn($recurringTransaction);
         $this->action('POST', 'RecurringController@store', ['create' => '1']);

app/tests/controllers/TransactionControllerTest.php

@@ -242,7 +242,7 @@ class TransactionControllerTest extends TestCase
     public function testStoreFails()
     {
         $journal = f::create('TransactionJournal');
-        unset($journal->id);
+        unset($journal->description);
 
         $this->_repository->shouldReceive('store')->andReturn($journal);