diff --git a/app/controllers/PiggybankController.php b/app/controllers/PiggybankController.php
index 63922f9bfa..59a288d43d 100644
--- a/app/controllers/PiggybankController.php
+++ b/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');
diff --git a/app/lib/Firefly/Storage/Piggybank/EloquentPiggybankRepository.php b/app/lib/Firefly/Storage/Piggybank/EloquentPiggybankRepository.php
index ffb0f6bd59..3610ff532c 100644
--- a/app/lib/Firefly/Storage/Piggybank/EloquentPiggybankRepository.php
+++ b/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'])) {
- $piggyBank->errors()->add('targetdate','Target date is mandatory!');
+ if ($piggyBank->repeats && !isset($data['targetdate'])) {
+ $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();
}
diff --git a/app/lib/Firefly/Trigger/Piggybanks/EloquentPiggybankTrigger.php b/app/lib/Firefly/Trigger/Piggybanks/EloquentPiggybankTrigger.php
index ae6f1910c9..a0eef0425f 100644
--- a/app/lib/Firefly/Trigger/Piggybanks/EloquentPiggybankTrigger.php
+++ b/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
- */
- 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.
+ * 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.
*
* @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.storeRepeated', 'Firefly\Trigger\Piggybanks\EloquentPiggybankTrigger@storeRepeated'
- );
+ $events->listen('piggybanks.store', 'Firefly\Trigger\Piggybanks\EloquentPiggybankTrigger@store');
$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.storeRepeated', [$piggyBank]);
- } else {
- \Event::fire('piggybanks.storePiggy', [$piggyBank]);
- }
+ \Event::fire('piggybanks.store', [$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();
-//
-//
-// }
-// }
-// }
-// }
}
\ No newline at end of file
diff --git a/app/models/PiggybankReminder.php b/app/models/PiggybankReminder.php
index e61a0269d5..698fbaa5af 100644
--- a/app/models/PiggybankReminder.php
+++ b/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);
+ }
+
}
\ No newline at end of file
diff --git a/app/tests/controllers/PiggybankControllerTest.php b/app/tests/controllers/PiggybankControllerTest.php
index 75e2bb9071..d946cca187 100644
--- a/app/tests/controllers/PiggybankControllerTest.php
+++ b/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::fire('piggybanks.storePiggy',[$piggyBank]);
- //Event::fire('piggybanks.storeRepeated',[$piggyBank]);
+ Event::shouldReceive('fire')->with('piggybanks.store',[$piggy])->once();
$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);
diff --git a/app/views/piggybanks/index.blade.php b/app/views/piggybanks/index.blade.php
index fe111337a7..6eea7d73dc 100644
--- a/app/views/piggybanks/index.blade.php
+++ b/app/views/piggybanks/index.blade.php
@@ -71,11 +71,14 @@
|
- @if(!is_null($piggyBank->reminder))
-
- Next reminder: {{$piggyBank->nextReminderDate()->format('M jS, Y')}} ({{$piggyBank->reminder}})
-
+
+ @if(!is_null($piggyBank->targetdate))
+ Target date: {{$piggyBank->targetdate->format('M jS, Y')}}
@endif
+ @if(!is_null($piggyBank->reminder))
+ Next reminder: {{$piggyBank->nextReminderDate()->format('M jS, Y')}} ({{$piggyBank->reminder}})
+ @endif
+
|
@@ -132,6 +135,7 @@
|
+
@if(!is_null($repeated->reminder))
Next reminder: {{$repeated->nextReminderDate()->format('M jS, Y')}} ({{$piggyBank->reminder}})
|