Optimizes some of the reminder routines; still have to test future repetitions although it should work fine.

app/controllers/PiggybankController.php

@@ -212,7 +212,7 @@ class PiggybankController extends BaseController
         $piggyBank = $this->_repository->store($data);
         if (!is_null($piggyBank->id)) {
             Session::flash('success', 'New piggy bank "' . $piggyBank->name . '" created!');
-            Event::fire('piggybanks.storePiggy',[$piggyBank]);
+            Event::fire('piggybanks.store',[$piggyBank]);
 
             return Redirect::route('piggybanks.index');
 
@@ -241,7 +241,7 @@ class PiggybankController extends BaseController
         $piggyBank = $this->_repository->store($data);
         if ($piggyBank->id) {
             Session::flash('success', 'New piggy bank "' . $piggyBank->name . '" created!');
-            Event::fire('piggybanks.storeRepeated',[$piggyBank]);
+            Event::fire('piggybanks.store',[$piggyBank]);
 
             return Redirect::route('piggybanks.index');
 

app/lib/Firefly/Storage/Piggybank/EloquentPiggybankRepository.php

@@ -74,6 +74,7 @@ class EloquentPiggybankRepository implements PiggybankRepositoryInterface
         foreach ($piggies as $pig) {
             $pig->leftInAccount = $this->leftOnAccount($pig->account);
         }
+
         return $piggies;
     }
 
@@ -136,20 +137,20 @@ class EloquentPiggybankRepository implements PiggybankRepositoryInterface
         $account = isset($data['account_id']) ? $accounts->find($data['account_id']) : null;
 
 
-
-
         $piggyBank = new \Piggybank($data);
 
-        if(!is_null($piggyBank->reminder) && is_null($piggyBank->startdate) && is_null($piggyBank->targetdate)) {
+        if (!is_null($piggyBank->reminder) && is_null($piggyBank->startdate) && is_null($piggyBank->targetdate)) {
+
+            $piggyBank->errors()->add('reminder', 'Cannot create reminders without start ~ AND target date.');
 
-            $piggyBank->errors()->add('reminder','Cannot create reminders without start ~ AND target date.');
             return $piggyBank;
 
         }
 
 
-        if($piggyBank->repeats && !isset($data['targetdate'])) {
+        if ($piggyBank->repeats && !isset($data['targetdate'])) {
-            $piggyBank->errors()->add('targetdate','Target date is mandatory!');
+            $piggyBank->errors()->add('targetdate', 'Target date is mandatory!');
+
             return $piggyBank;
         }
         if (!is_null($account)) {
@@ -223,6 +224,8 @@ class EloquentPiggybankRepository implements PiggybankRepositoryInterface
         $piggy->startdate
             = isset($data['startdate']) && strlen($data['startdate']) > 0 ? new Carbon($data['startdate']) : null;
 
+
+
         foreach ($piggy->piggybankrepetitions()->get() as $rep) {
             $rep->delete();
         }

app/lib/Firefly/Trigger/Piggybanks/EloquentPiggybankTrigger.php

@@ -103,29 +103,63 @@ class EloquentPiggybankTrigger
     }
 
     /**
-     * @param \Piggybank $piggyBank
+     * Whenever a repetition is made, the decision is there to make reminders for it. Or not.
+     *
+     * Some combinations are "invalid" or impossible and will never trigger reminders. Others do.
      *
-     * @return bool
-     */
-    public function destroy(\Piggybank $piggyBank)
-    {
-        return true;
-    }
-
-    /**
      * @param \PiggybankRepetition $rep
+     *
+     * @return null
      */
-    public function madeRep(\PiggybankRepetition $rep)
+    public function createdRepetition(\PiggybankRepetition $repetition)
     {
-        // do something with reminders?
+
-        $piggyBank = $rep->piggybank;
+        $piggyBank = $repetition->piggybank;
+
+        // first, exclude all combinations that will not generate (valid) reminders
+
+        // no reminders needed (duh)
         if (is_null(($piggyBank->reminder))) {
             return null;
         }
 
-        $current = clone $rep->startdate;
+        // no start, no target, no repeat (#1):
+        if (is_null($piggyBank->startdate) && is_null($piggyBank->targetdate) && $piggyBank->repeats == 0) {
+            return null;
+        }
+
+        // no start, but repeats (#5):
+        if (is_null($piggyBank->startdate) && $piggyBank->repeats == 1) {
+            return null;
+        }
+
+        // no start, no end, but repeats (#6)
+        if (is_null($piggyBank->startdate) && is_null($piggyBank->targetdate) && $piggyBank->repeats == 1) {
+            return null;
+        }
+
+        // no end, but repeats (#7)
+        if (is_null($piggyBank->targetdate) && $piggyBank->repeats == 1) {
+            return null;
+        }
+
+        // #2, #3, #4 and #8 are valid combo's.
+        if (is_null($repetition->targetdate)) {
+            $end = new Carbon;
+            $end->addYears(2);
+        } else {
+            $end = $repetition->targetdate;
+        }
+        if (is_null($repetition->startdate)) {
+            $start = new Carbon;
+        } else {
+            $start = $repetition->startdate;
+        }
+
+
+        $current = $start;
         $today = new Carbon;
-        while ($current <= $rep->targetdate) {
+        while ($current <= $end) {
 
             // when do we start reminding?
             // X days before $current:
@@ -177,6 +211,16 @@ class EloquentPiggybankTrigger
 
     }
 
+    /**
+     * @param \Piggybank $piggyBank
+     *
+     * @return bool
+     */
+    public function destroy(\Piggybank $piggyBank)
+    {
+        return true;
+    }
+
     /**
      * @param \Piggybank $piggyBank
      * @param            $amount
@@ -216,26 +260,14 @@ class EloquentPiggybankTrigger
     }
 
     /**
-     * @param \Piggybank $piggyBank
+     * This method is called when a piggy bank or repeated expense is created. It will create the first
-     */
+     * repetition which by default is equal to the PB / RE itself. After that, other triggers will take over.
-    public function storePiggy(\Piggybank $piggyBank)
-    {
-        $piggyBank->createRepetition($piggyBank->startdate, $piggyBank->targetdate);
-
-        return true;
-
-    }
-
-    /**
-     * Validates and creates all repetitions for repeating piggy banks.
-     * This routine is also called whenever Firefly runs, so new repetitions
-     * are created automatically.
      *
      * @param \Piggybank $piggyBank
      *
      * @return bool
      */
-    public function storeRepeated(\Piggybank $piggyBank)
+    public function store(\Piggybank $piggyBank)
     {
         $piggyBank->createRepetition($piggyBank->startdate, $piggyBank->targetdate);
 
@@ -248,17 +280,13 @@ class EloquentPiggybankTrigger
     public function subscribe(Dispatcher $events)
     {
         $events->listen('piggybanks.destroy', 'Firefly\Trigger\Piggybanks\EloquentPiggybankTrigger@destroy');
-
         $events->listen(
             'piggybanks.modifyAmountAdd', 'Firefly\Trigger\Piggybanks\EloquentPiggybankTrigger@modifyAmountAdd'
         );
         $events->listen(
             'piggybanks.modifyAmountRemove', 'Firefly\Trigger\Piggybanks\EloquentPiggybankTrigger@modifyAmountRemove'
         );
-        $events->listen('piggybanks.storePiggy', 'Firefly\Trigger\Piggybanks\EloquentPiggybankTrigger@storePiggy');
+        $events->listen('piggybanks.store', 'Firefly\Trigger\Piggybanks\EloquentPiggybankTrigger@store');
-        $events->listen(
-            'piggybanks.storeRepeated', 'Firefly\Trigger\Piggybanks\EloquentPiggybankTrigger@storeRepeated'
-        );
         $events->listen('piggybanks.update', 'Firefly\Trigger\Piggybanks\EloquentPiggybankTrigger@update');
         $events->listen(
             'piggybanks.createRelatedTransfer',
@@ -273,24 +301,32 @@ class EloquentPiggybankTrigger
         );
 
         $events->listen(
-            'piggybanks.repetition', 'Firefly\Trigger\Piggybanks\EloquentPiggybankTrigger@madeRep'
+            'piggybanks.repetition', 'Firefly\Trigger\Piggybanks\EloquentPiggybankTrigger@createdRepetition'
         );
     }
 
+    /**
+     * When the user updates a piggy bank the repetitions, past and now, may be wrong. The best bet
+     * would be to delete everything and start over, but that also means past repetitions will be gone.
+     *
+     * Instead, we have disabled changing the dates when the piggy bank is repeating: a repeated expense cannot
+     * have its dates changed. This will prevent many problems I don't want to deal with.
+     *
+     * @param \Piggybank $piggyBank
+     */
     public function update(\Piggybank $piggyBank)
     {
         // delete all repetitions:
         foreach ($piggyBank->piggybankrepetitions()->get() as $rep) {
+
             $rep->delete();
         }
         unset($rep);
 
         // trigger "new" piggy bank to recreate them.
-        if ($piggyBank->repeats == 1) {
+        \Event::fire('piggybanks.store', [$piggyBank]);
-            \Event::fire('piggybanks.storeRepeated', [$piggyBank]);
+
-        } else {
+
-            \Event::fire('piggybanks.storePiggy', [$piggyBank]);
-        }
         // loop the repetitions and update them according to the events and the transactions:
         foreach ($piggyBank->piggybankrepetitions()->get() as $rep) {
             // SUM for transactions
@@ -330,144 +366,4 @@ class EloquentPiggybankTrigger
     }
 
 
-
-
-
-
-//
-//    /**
-//     *
-//     */
-//    public function updatePiggybankRepetitions()
-//    {
-//        // grab all piggy banks.
-//        if (\Auth::check()) {
-//            $piggybanks = \Auth::user()->piggybanks()->with(['piggybankrepetitions'])->where('repeats', 0)->get();
-//            $today = new Carbon;
-//            /** @var \Piggybank $piggy */
-//            foreach ($piggybanks as $piggy) {
-//                if (count($piggy->piggybankrepetitions) == 0) {
-//                    $rep = new \PiggybankRepetition;
-//                    $rep->piggybank()->associate($piggy);
-//                    $rep->targetdate = $piggy->targetdate;
-//                    $rep->startdate = $piggy->startdate;
-//                    $rep->currentamount = 0;
-//                    try {
-//                        $rep->save();
-//                    } catch (QueryException $e) {
-//                    }
-//                }
-//
-//                // whatever we did here, we now have all repetitions for this
-//                // piggy bank, and we can find transactions that fall within
-//                // that repetition (to fix the "saved amount".
-//                $reps = $piggy->piggybankrepetitions()->get();
-//
-//                /** @var \PiggybankRepetition $rep */
-//                foreach ($reps as $rep) {
-//                    $query = \Transaction::where('piggybank_id', $piggy->id)->leftJoin(
-//                        'transaction_journals', 'transaction_journals.id', '=',
-//                        'transactions.transaction_journal_id'
-//                    );
-//                    if (!is_null($rep->startdate)) {
-//                        $query->where('transaction_journals.date', '>=', $rep->startdate->format('Y-m-d'));
-//                    }
-//                    if (!is_null($rep->targetdate)) {
-//                        $query->where(
-//                            'transaction_journals.date', '<=', $rep->targetdate->format('Y-m-d')
-//                        );
-//                    }
-//
-//                    // get events for piggy bank, save those as well:
-//                    $eventSumQuery = $piggy->piggybankevents();
-//                    if(!is_null($rep->startdate)) {
-//                        $eventSumQuery->where('date','>=',$rep->startdate->format('Y-m-d'));
-//                    }
-//                    if(!is_null($rep->targetdate)) {
-//                        $eventSumQuery->where('date','<=',$rep->targetdate->format('Y-m-d'));
-//                    }
-//                    $eventSum = floatval($eventSumQuery->sum('amount'));
-//
-//
-//                    $sum = $query->sum('transactions.amount');
-//                    $rep->currentamount = floatval($sum) + $eventSum;
-//                    $rep->save();
-//
-//
-//                }
-//
-//            }
-//            unset($piggy, $piggybanks, $rep);
-//
-//            // grab all repeated transactions.
-//            $repeatedExpenses = \Auth::user()->piggybanks()->with(['piggybankrepetitions'])->where('repeats', 1)->get();
-//            /** @var \Piggybank $repeated */
-//            foreach ($repeatedExpenses as $repeated) {
-//                // loop from start to today or something
-//                $rep = new \PiggybankRepetition;
-//                $rep->piggybank()->associate($repeated);
-//                $rep->startdate = $repeated->startdate;
-//                $rep->targetdate = $repeated->targetdate;
-//                $rep->currentamount = 0;
-//                try {
-//                    $rep->save();
-//                } catch (QueryException $e) {
-//                }
-//                unset($rep);
-//
-//                if ($repeated->targetdate <= $today) {
-//                    // add 1 month to startdate, or maybe X period, like 3 weeks.
-//                    $startTarget = clone $repeated->targetdate;
-//                    while ($startTarget <= $today) {
-//                        $startCurrent = clone $startTarget;
-//
-//                        // add some kind of period to start current making $endCurrent.
-//                        $endCurrent = clone $startCurrent;
-//                        switch ($repeated->rep_length) {
-//                            default:
-//                                die('No rep lengt!');
-//                                break;
-//                            case 'day':
-//                                $endCurrent->addDays($repeated->rep_every);
-//                                break;
-//                            case 'week':
-//                                $endCurrent->addWeeks($repeated->rep_every);
-//                                break;
-//                            case 'month':
-//                                $endCurrent->addMonths($repeated->rep_every);
-//                                break;
-//                            case 'year':
-//                                $endCurrent->addYears($repeated->rep_every);
-//                                break;
-//                        }
-//
-//                        $rep = new \PiggybankRepetition;
-//                        $rep->piggybank()->associate($repeated);
-//                        $rep->startdate = $startCurrent;
-//                        $rep->targetdate = $endCurrent;
-//                        $rep->currentamount = 0;
-//                        $startTarget = $endCurrent;
-//                        try {
-//                            $rep->save();
-//                        } catch (QueryException $e) {
-//
-//                        }
-//                    }
-//                }
-//                $reps = $repeated->piggybankrepetitions()->get();
-//                /** @var \PiggybankRepetition $rep */
-//                foreach ($reps as $rep) {
-//                    $sum = \Transaction::where('piggybank_id', $repeated->id)->leftJoin(
-//                        'transaction_journals', 'transaction_journals.id', '=', 'transactions.transaction_journal_id'
-//                    )->where('transaction_journals.date', '>=', $rep->startdate->format('Y-m-d'))->where(
-//                        'transaction_journals.date', '<=', $rep->targetdate->format('Y-m-d')
-//                    )->sum('transactions.amount');
-//                    $rep->currentamount = floatval($sum);
-//                    $rep->save();
-//
-//
-//                }
-//            }
-//        }
-//    }
 }

app/models/PiggybankReminder.php

@@ -8,37 +8,6 @@ class PiggybankReminder extends Reminder
 {
     protected $isSubclass = true;
 
-
-    public function amountToSave()
-    {
-        /** @var \Piggybank $piggyBank */
-        $piggyBank = $this->piggybank;
-        /** @var \PiggybankRepetition $repetition */
-        $repetition = $piggyBank->currentRelevantRep();
-
-        $today = new Carbon;
-        $diff = $today->diff($repetition->targetdate);
-        $left = $piggyBank->targetamount - $repetition->currentamount;
-        // to prevent devide by zero:
-        $piggyBank->reminder_skip = $piggyBank->reminder_skip < 1 ? 1 : $piggyBank->reminder_skip;
-        $toSave = 0;
-        switch ($piggyBank->reminder) {
-            case 'day':
-                throw new \Firefly\Exception\FireflyException('No impl day reminder/ PiggyBankReminder Render');
-                break;
-            case 'week':
-                throw new \Firefly\Exception\FireflyException('No impl week reminder/ PiggyBankReminder Render');
-                break;
-            case 'month':
-                $toSave = $left / ($diff->m / $piggyBank->reminder_skip);
-                break;
-            case 'year':
-                throw new \Firefly\Exception\FireflyException('No impl year reminder/ PiggyBankReminder Render');
-                break;
-        }
-        return floatval($toSave);
-    }
-
     /**
      * @return string
      * @throws Firefly\Exception\FireflyException
@@ -60,4 +29,51 @@ class PiggybankReminder extends Reminder
         return $fullText;
     }
 
+
+    /**
+     * @return float
+     * @throws Firefly\Exception\FireflyException
+     */
+    public function amountToSave()
+    {
+        /** @var \Piggybank $piggyBank */
+        $piggyBank = $this->piggybank;
+        /** @var \PiggybankRepetition $repetition */
+        $repetition = $piggyBank->currentRelevantRep();
+
+        // if the target date of the repetition is zero, we use the created_at date of the repetition
+        // and add two years; it's the same routine used elsewhere.
+        if (is_null($repetition->targetdate)) {
+            $targetdate = clone $repetition->created_at;
+            $targetdate->addYears(2);
+        } else {
+            $targetdate = $repetition->targetdate;
+        }
+
+
+        $today = new Carbon;
+        $diff = $today->diff($targetdate);
+        $left = $piggyBank->targetamount - $repetition->currentamount;
+        // to prevent devide by zero:
+        $piggyBank->reminder_skip = $piggyBank->reminder_skip < 1 ? 1 : $piggyBank->reminder_skip;
+        $toSave = 0;
+        switch ($piggyBank->reminder) {
+            case 'day':
+                throw new \Firefly\Exception\FireflyException('No impl day reminder/ PiggyBankReminder Render');
+                break;
+            case 'week':
+                $weeks = ceil($diff->days / 7);
+                $toSave = $left / ($weeks / $piggyBank->reminder_skip);
+                break;
+            case 'month':
+                $toSave = $left / ($diff->m / $piggyBank->reminder_skip);
+                break;
+            case 'year':
+                throw new \Firefly\Exception\FireflyException('No impl year reminder/ PiggyBankReminder Render');
+                break;
+        }
+
+        return floatval($toSave);
+    }
+
 } 

app/tests/controllers/PiggybankControllerTest.php

@@ -313,9 +313,7 @@ class PiggybankControllerTest extends TestCase
         $piggy = f::create('Piggybank');
         $piggy->repeats = 0;
         $piggy->save();
-        Event::shouldReceive('fire')->with('piggybanks.storePiggy',[$piggy])->once();
+        Event::shouldReceive('fire')->with('piggybanks.store',[$piggy])->once();
-        //Event::fire('piggybanks.storePiggy',[$piggyBank]);
-        //Event::fire('piggybanks.storeRepeated',[$piggyBank]);
 
 
         $this->_piggybanks->shouldReceive('store')->once()->andReturn($piggy);
@@ -328,7 +326,7 @@ class PiggybankControllerTest extends TestCase
         $piggy = f::create('Piggybank');
         $piggy->repeats = 1;
         $piggy->save();
-        Event::shouldReceive('fire')->with('piggybanks.storeRepeated',[$piggy])->once();
+        Event::shouldReceive('fire')->with('piggybanks.store',[$piggy])->once();
         $this->_piggybanks->shouldReceive('store')->once()->andReturn($piggy);
         $this->action('POST', 'PiggybankController@storeRepeated');
         $this->assertResponseStatus(302);

app/views/piggybanks/index.blade.php

@@ -71,11 +71,14 @@
                                     </div>
                             </td>
                             <td style="width:40%;">
-                                @if(!is_null($piggyBank->reminder))
+                                <p class="small">
-                                <small>
+                                @if(!is_null($piggyBank->targetdate))
-                                Next reminder: {{$piggyBank->nextReminderDate()->format('M jS, Y')}} ({{$piggyBank->reminder}})
+                                    Target date: {{$piggyBank->targetdate->format('M jS, Y')}}<br />
-                                </small>
                                 @endif
+                                @if(!is_null($piggyBank->reminder))
+                                    Next reminder: {{$piggyBank->nextReminderDate()->format('M jS, Y')}} ({{$piggyBank->reminder}})
+                                @endif
+                                </p>
 
                             </td>
                             <td>
@@ -132,6 +135,7 @@
                             </div>
                         </td>
                         <td style="width:40%;">
+
                             @if(!is_null($repeated->reminder))
                             <small>
                                 Next reminder: {{$repeated->nextReminderDate()->format('M jS, Y')}} ({{$piggyBank->reminder}})