From fdf03cd8e2b1d5220810c1aab8fd7003f4a0fdfb Mon Sep 17 00:00:00 2001 From: James Cole Date: Tue, 9 Sep 2014 20:01:13 +0200 Subject: [PATCH] Some comment cleanup in the libraries. --- .../Database/SingleTableInheritanceEntity.php | 2 +- .../Firefly/Helper/Controllers/Account.php | 9 +- app/lib/Firefly/Helper/Controllers/Chart.php | 7 +- .../Helper/Controllers/ChartInterface.php | 7 +- app/lib/Firefly/Queue/Import.php | 2 +- .../Reminder/EloquentReminderRepository.php | 10 ++ .../Piggybanks/EloquentPiggybankTrigger.php | 99 ++++++++++++++----- 7 files changed, 100 insertions(+), 36 deletions(-) diff --git a/app/lib/Firefly/Database/SingleTableInheritanceEntity.php b/app/lib/Firefly/Database/SingleTableInheritanceEntity.php index 40d31c755b..8f274f2461 100644 --- a/app/lib/Firefly/Database/SingleTableInheritanceEntity.php +++ b/app/lib/Firefly/Database/SingleTableInheritanceEntity.php @@ -73,7 +73,7 @@ abstract class SingleTableInheritanceEntity extends Ardent // newEloquentBuilder() was added in 4.1 $builder = $this->newEloquentBuilder($this->newBaseQueryBuilder()); - // Once we have the query builders, we will set the model instances so the + // Once Firefly has the query builders, it will set the model instances so the // builder can easily access any information it may need from the model // while it is constructing and executing various queries against it. $builder->setModel($this)->with($this->with); diff --git a/app/lib/Firefly/Helper/Controllers/Account.php b/app/lib/Firefly/Helper/Controllers/Account.php index b92f514a4b..b789bc9749 100644 --- a/app/lib/Firefly/Helper/Controllers/Account.php +++ b/app/lib/Firefly/Helper/Controllers/Account.php @@ -28,13 +28,16 @@ class Account implements AccountInterface * Since it is entirely possible the database is messed up somehow it might be that a transaction * journal has only one transaction. This is mainly caused by wrong deletions and other artefacts from the past. * - * If it is the case, we remove $item and continue like nothing ever happened. This will however, - * mess up some statisics but we can live with that. We might be needing some cleanup routine in the future. + * If it is the case, Firefly removes $item and continues like nothing ever happened. This will however, + * mess up some statisics but it's decided everybody should learn to live with that. * - * For now, we simply warn the user of this. + * Firefly might be needing some cleanup routine in the future. + * + * For now, Firefly simply warns the user of this. * * @param \Account $account * @param $perPage + * * @return array|mixed * @throws \Firefly\Exception\FireflyException * @SuppressWarnings(PHPMD.CyclomaticComplexity) diff --git a/app/lib/Firefly/Helper/Controllers/Chart.php b/app/lib/Firefly/Helper/Controllers/Chart.php index 81c9cfe737..991a76de6b 100644 --- a/app/lib/Firefly/Helper/Controllers/Chart.php +++ b/app/lib/Firefly/Helper/Controllers/Chart.php @@ -550,9 +550,10 @@ class Chart implements ChartInterface } /** - * We check how much money has been spend on the limitrepetition (aka: the current envelope) in the period denoted. - * Aka, we have a certain amount of money in an envelope and we wish to know how much we've spent between the dates - * entered. This can be a partial match with the date range of the envelope or no match at all. + * Firefly checks how much money has been spend on the limitrepetition (aka: the current envelope) in + * the period denoted. Aka, the user has a certain amount of money in an envelope and wishes to know how + * much he has spent between the dates entered. This date range can be a partial match with the date range + * of the envelope or no match at all. * * @param \LimitRepetition $repetition * @param Carbon $start diff --git a/app/lib/Firefly/Helper/Controllers/ChartInterface.php b/app/lib/Firefly/Helper/Controllers/ChartInterface.php index ebc42b16ad..4a0614598b 100644 --- a/app/lib/Firefly/Helper/Controllers/ChartInterface.php +++ b/app/lib/Firefly/Helper/Controllers/ChartInterface.php @@ -98,9 +98,10 @@ interface ChartInterface /** - * We check how much money has been spend on the limitrepetition (aka: the current envelope) in the period denoted. - * Aka, we have a certain amount of money in an envelope and we wish to know how much we've spent between the dates - * entered. This can be a partial match with the date range of the envelope or no match at all. + * Firefly checks how much money has been spend on the limitrepetition (aka: the current envelope) in + * the period denoted. Aka, the user has a certain amount of money in an envelope and wishes to know how + * much he has spent between the dates entered. This date range can be a partial match with the date range + * of the envelope or no match at all. * * @param \LimitRepetition $repetition * @param Carbon $start diff --git a/app/lib/Firefly/Queue/Import.php b/app/lib/Firefly/Queue/Import.php index 8918f2e452..ea3d8aa8e2 100644 --- a/app/lib/Firefly/Queue/Import.php +++ b/app/lib/Firefly/Queue/Import.php @@ -133,7 +133,7 @@ class Import return; } - // if we try to import a beneficiary, Firefly will "merge" already, + // if Firefly tries to import a beneficiary, Firefly will "merge" already existing ones, // so we don't care: if (isset($payload['data']['account_type']) && $payload['data']['account_type'] == 'Beneficiary account') { // store beneficiary diff --git a/app/lib/Firefly/Storage/Reminder/EloquentReminderRepository.php b/app/lib/Firefly/Storage/Reminder/EloquentReminderRepository.php index 79809c9bd6..651ca91147 100644 --- a/app/lib/Firefly/Storage/Reminder/EloquentReminderRepository.php +++ b/app/lib/Firefly/Storage/Reminder/EloquentReminderRepository.php @@ -65,6 +65,16 @@ class EloquentReminderRepository implements ReminderRepositoryInterface return $this->_user->reminders()->validOn($today)->get(); } + /** + * @return mixed + */ + public function getPiggybankReminders() + { + $today = new Carbon; + + return $this->_user->reminders()->where('class','PiggybankReminder')->validOn($today)->get(); + } + /** * */ diff --git a/app/lib/Firefly/Trigger/Piggybanks/EloquentPiggybankTrigger.php b/app/lib/Firefly/Trigger/Piggybanks/EloquentPiggybankTrigger.php index cb5442842a..11a687b02a 100644 --- a/app/lib/Firefly/Trigger/Piggybanks/EloquentPiggybankTrigger.php +++ b/app/lib/Firefly/Trigger/Piggybanks/EloquentPiggybankTrigger.php @@ -14,7 +14,9 @@ use Illuminate\Events\Dispatcher; class EloquentPiggybankTrigger { /** - * + * This method checks every repeating piggy bank the user has (these are called repeated expenses) and makes + * sure each repeated expense has a "repetition" for the current time period. For example, if the user has + * a weekly repeated expense of E 40,- this method will fire every week and create a new repetition. */ public function checkRepeatingPiggies() { @@ -25,26 +27,29 @@ class EloquentPiggybankTrigger $piggies = []; } - \Log::debug('Now in checkRepeatingPiggies with ' . count($piggies) . ' piggies'); - + \Log::debug('Now in checkRepeatingPiggies with ' . count($piggies) . ' piggies found.'); /** @var \Piggybank $piggyBank */ foreach ($piggies as $piggyBank) { \Log::debug('Now working on ' . $piggyBank->name); - // get the latest repetition, see if we need to "append" more: + /* + * Get the latest repetition, see if Firefly needs to create more. + */ /** @var \PiggybankRepetition $primer */ $primer = $piggyBank->piggybankrepetitions()->orderBy('targetdate', 'DESC')->first(); \Log::debug('Last target date is: ' . $primer->targetdate); - // for repeating piggy banks, the target date is mandatory: - $today = new Carbon; - $end = clone $primer->targetdate; - // the next repetition must be created starting at the day after the target date: + // the next repetition must be created starting at the day after the target date of the previous one. + /* + * A repeated expense runs from day 1 to day X. Since it repeats, the next repetition starts at day X+1 + * until however often the repeated expense is set to repeat: a month, a week, a year. + */ $start = clone $primer->targetdate; $start->addDay(); + while ($start <= $today) { \Log::debug('Looping! Start is: ' . $start); @@ -104,15 +109,26 @@ class EloquentPiggybankTrigger /** * 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. * - * @param \PiggybankRepetition $rep + * The numbers below refer to a small list I made in a text-file (it no longer exists) which contained the eight + * binary combinations that can be made of three properties each piggy bank has (among others): + * + * - Whether or not it has a start date. + * - Whether or not it has an end date. + * - Whether or not the piggy bank repeats itself. + * + * @SuppressWarnings(PHPMD.ExcessiveMethodLength) + * @SuppressWarnings(PHPMD.NPathComplexity) + * @SuppressWarnings(PHPMD.CyclomaticComplexity) + * + * @param \PiggybankRepetition $repetition * * @return null */ public function createdRepetition(\PiggybankRepetition $repetition) { + \Log::debug('TRIGGER on createdRepetition() for repetition #' . $repetition->id); $piggyBank = $repetition->piggybank; @@ -120,49 +136,70 @@ class EloquentPiggybankTrigger // no reminders needed (duh) if (is_null(($piggyBank->reminder))) { + \Log::debug('No reminders because no reminder needed.'); return null; } // no start, no target, no repeat (#1): if (is_null($piggyBank->startdate) && is_null($piggyBank->targetdate) && $piggyBank->repeats == 0) { + \Log::debug('No reminders because no start, no target, no repeat (#1)'); return null; } // no start, but repeats (#5): if (is_null($piggyBank->startdate) && $piggyBank->repeats == 1) { + \Log::debug('No reminders because no start, but repeats (#5)'); return null; } // no start, no end, but repeats (#6) if (is_null($piggyBank->startdate) && is_null($piggyBank->targetdate) && $piggyBank->repeats == 1) { + \Log::debug('No reminders because no start, no end, but repeats (#6)'); return null; } // no end, but repeats (#7) if (is_null($piggyBank->targetdate) && $piggyBank->repeats == 1) { + \Log::debug('No reminders because no end, but repeats (#7)'); return null; } - // #2, #3, #4 and #8 are valid combo's. + \Log::debug('Will continue...'); + /* + * #2, #3, #4 and #8 are valid combo's. + * + * We add two years to the end when the repetition has no target date; we "pretend" there is a target date. + * + */ if (is_null($repetition->targetdate)) { $end = new Carbon; $end->addYears(2); } else { $end = $repetition->targetdate; } + /* + * If there is no start date, the start dat becomes right now. + */ if (is_null($repetition->startdate)) { $start = new Carbon; } else { $start = $repetition->startdate; } - + /* + * Firefly checks every period X between $start and $end and if necessary creates a reminder. Firefly + * only creates reminders if the $current date is after today. Piggy banks may have their start in the past. + * + * This loop will jump a month when the reminder is set monthly, a week when it's set weekly, etcetera. + */ $current = $start; - $today = new Carbon; + $today = new Carbon; + $today->startOfDay(); while ($current <= $end) { - - // when do we start reminding? - // X days before $current: + \Log::debug('Looping reminder dates; now at ' . $current); + /* + * Piggy bank reminders start X days before the actual date of the event. + */ $reminderStart = clone $current; switch ($piggyBank->reminder) { case 'day': @@ -179,20 +216,32 @@ class EloquentPiggybankTrigger break; } + /* + * If the date is past today we create a reminder, otherwise we don't. The end date is the date + * the reminder is due; after that it is invalid. + */ if ($current >= $today) { $reminder = new \PiggybankReminder; $reminder->piggybank()->associate($piggyBank); $reminder->user()->associate(\Auth::user()); $reminder->startdate = $reminderStart; - $reminder->enddate = $current; + $reminder->enddate = $current; + $reminder->active = 1; + \Log::debug('Will create a reminder. Is it valid?'); + \Log::debug($reminder->validate()); try { - $reminder->save(); + $reminder->save(); } catch (QueryException $e) { + \Log::error('Could not save reminder: ' . $e->getMessage()); } + } else { + \Log::debug('Current is before today, will not make a reminder.'); } - + /* + * Here Firefly jumps ahead to the next reminder period. + */ switch ($piggyBank->reminder) { case 'day': $current->addDays($piggyBank->reminder_skip); @@ -234,12 +283,12 @@ class EloquentPiggybankTrigger */ public function modifyAmountAdd(\Piggybank $piggyBank, $amount) { - $rep = $piggyBank->currentRelevantRep(); + $rep = $piggyBank->currentRelevantRep(); $today = new Carbon; // create event: - $event = new \PiggybankEvent; - $event->date = new Carbon; + $event = new \PiggybankEvent; + $event->date = new Carbon; $event->amount = $amount; $event->piggybank()->associate($piggyBank); @@ -259,8 +308,8 @@ class EloquentPiggybankTrigger public function modifyAmountRemove(\Piggybank $piggyBank, $amount) { // create event: - $event = new \PiggybankEvent; - $event->date = new Carbon; + $event = new \PiggybankEvent; + $event->date = new Carbon; $event->amount = $amount; $event->piggybank()->associate($piggyBank); $event->save(); @@ -359,7 +408,7 @@ class EloquentPiggybankTrigger if (!is_null($rep->targetdate)) { $eventSumQuery->where('date', '<=', $rep->targetdate->format('Y-m-d')); } - $eventSum = floatval($eventSumQuery->sum('amount')); + $eventSum = floatval($eventSumQuery->sum('amount')); $rep->currentamount = floatval($sum) + $eventSum; $rep->save();