From 50005544ee4133bd46fd2ca16c09b4227b89424b Mon Sep 17 00:00:00 2001 From: Yamamoto Kadir Date: Mon, 12 Jun 2017 12:01:23 +0900 Subject: [PATCH] Fix some relationship inconsistencies and reduce dashboard queries (#209) This normalizes all relationships and adds relations to models to take advantage of laravel's eagerloading and reduce queries to the database. This might go against the 'add any kind of complexities whatsoever' rule in the contribution guide, but I think adding relations would eventually simplify the code base. The amount of queries just on the dashboard fell from ~70 to 26. There's also a lot of boilerplate code in the Contact model to retrieve relationships and to maintain accurate count of them (fields like number_of_*). They can be greatly simplified with the addition of relations. One other thing in the Contact model was the definition of the Contact-User relation. This definition uses the account_id field in the contacts table and and relates it to id in users. I'm not sure if this is the intention, since the id from accounts table is used when assigning account_id for a contact. This seems to have worked so far, because both a user and its associated account are created at the same time during registration and their ids match, but if there was any difference, this relationship would relate wrong contacts and users. I changed the relationship name to account. This makes the path to a user from a contact is $contact->account->user right now. --- app/Account.php | 79 +++ app/Activity.php | 24 + app/ActivityStatistic.php | 16 + app/ActivityType.php | 8 + app/Contact.php | 485 +++++++++--------- app/Debt.php | 31 ++ app/Entry.php | 8 + app/Event.php | 16 + app/Gift.php | 27 + app/Http/Controllers/DashboardController.php | 126 ++--- app/Kid.php | 16 + app/Note.php | 16 + app/Reminder.php | 16 + app/SignificantOther.php | 28 + app/Task.php | 19 + app/User.php | 9 +- .../views/dashboard/events/_kids.blade.php | 2 +- .../events/_significantothers.blade.php | 2 +- tests/Unit/ContactTest.php | 13 +- 19 files changed, 614 insertions(+), 327 deletions(-) diff --git a/app/Account.php b/app/Account.php index 62c123474..08ccd3fbe 100644 --- a/app/Account.php +++ b/app/Account.php @@ -6,4 +6,83 @@ use Illuminate\Database\Eloquent\Model; class Account extends Model { + /** + * Get the activity records associated with the account. + */ + public function activities() + { + return $this->hasMany('App\Activity'); + } + + /** + * Get the contact records associated with the account. + */ + public function contacts() + { + return $this->hasMany('App\Contact')->orderBy('updated_at', 'desc'); + } + + /** + * Get the debt records associated with the account. + */ + public function debt() + { + return $this->hasMany('App\Debt'); + } + + /** + * Get the gift records associated with the account. + */ + public function gifts() + { + return $this->hasMany('App\Gift'); + } + + /** + * Get the event records associated with the account. + */ + public function events() + { + return $this->hasMany('App\Event')->orderBy('created_at', 'desc'); + } + + /** + * Get the kid records associated with the account. + */ + public function kids() + { + return $this->hasMany('App\Kid'); + } + + /** + * Get the note records associated with the account. + */ + public function notes() + { + return $this->hasMany('App\Note'); + } + + /** + * Get the reminder records associated with the account. + */ + public function reminders() + { + return $this->hasMany('App\Reminder'); + } + + /** + * Get the task records associated with the account. + */ + public function tasks() + { + return $this->hasMany('App\Task'); + } + + /** + * Get the user record associated with the account. + */ + public function user() + { + return $this->hasOne('App\User'); + } } diff --git a/app/Activity.php b/app/Activity.php index 79815066d..12c5306cb 100644 --- a/app/Activity.php +++ b/app/Activity.php @@ -13,6 +13,30 @@ class Activity extends Model protected $dates = ['date_it_happened']; + /** + * Get the account record associated with the gift. + */ + public function account() + { + return $this->belongsTo('App\Account'); + } + + /** + * Get the contact record associated with the gift. + */ + public function contact() + { + return $this->belongsTo('App\Contact'); + } + + /** + * Get the activity type record associated with the activity. + */ + public function type() + { + return $this->belongsTo('App\ActivityType'); + } + /** * Get the summary for this activity. * diff --git a/app/ActivityStatistic.php b/app/ActivityStatistic.php index 69ab24afe..21b12ce4c 100644 --- a/app/ActivityStatistic.php +++ b/app/ActivityStatistic.php @@ -7,4 +7,20 @@ use Illuminate\Database\Eloquent\Model; class ActivityStatistic extends Model { protected $table = 'activity_statistics'; + + /** + * Get the account record associated with the activity statistic. + */ + public function account() + { + return $this->belongsTo('App\Account'); + } + + /** + * Get the contact record associated with the activity statistic. + */ + public function contact() + { + return $this->belongsTo('App\Contact'); + } } diff --git a/app/ActivityType.php b/app/ActivityType.php index e8ee21c7a..3929a3485 100644 --- a/app/ActivityType.php +++ b/app/ActivityType.php @@ -9,6 +9,14 @@ class ActivityType extends Model { protected $table = 'activity_types'; + /** + * Get the activity type group record associated with the activity types. + */ + public function group() + { + return $this->belongsTo('App\ActivityTypeGroup'); + } + public function getTranslationKeyAsString() { return trans('people.activity_type_'.$this->key); diff --git a/app/Contact.php b/app/Contact.php index 7d189aa6d..5efe9aa87 100644 --- a/app/Contact.php +++ b/app/Contact.php @@ -3,15 +3,9 @@ namespace App; use Auth; -use App\Note; -use App\Event; -use App\Debt; -use App\Account; -use App\Country; -use App\Reminder; use Carbon\Carbon; use App\Helpers\DateHelper; -use Gmopx\LaravelOWM\LaravelOWM; +use Illuminate\Support\Collection; use Illuminate\Support\Facades\Storage; use Illuminate\Database\Eloquent\Model; @@ -22,18 +16,116 @@ class Contact extends Model ]; /** - * Eager load user with every contact. + * Eager load account with every contact. */ protected $with = [ - 'user', + 'account', ]; /** * Get the user associated with the contact. */ - public function user() + public function account() { - return $this->belongsTo('App\User', 'account_id'); + return $this->belongsTo('App\Account'); + } + + /** + * Get the activity records associated with the contact. + */ + public function activities() + { + return $this->hasMany('App\Activity')->orderBy('date_it_happened', 'desc'); + } + + /** + * Get the activity records associated with the contact. + */ + public function activityStatistics() + { + return $this->hasMany('App\ActivityStatistic'); + } + + /** + * Get the contact records associated with the contact. + */ + public function country() + { + return $this->belongsTo('App\Country'); + } + + /** + * Get the debt records associated with the contact. + */ + public function debt() + { + return $this->hasMany('App\Debt'); + } + + /** + * Get the gift records associated with the contact. + */ + public function gifts() + { + return $this->hasMany('App\Gift'); + } + + /** + * Get the event records associated with the contact. + */ + public function events() + { + return $this->hasMany('App\Event')->orderBy('created_at', 'desc'); + } + + /** + * Get the kid records associated with the contact. + */ + public function kids() + { + return $this->hasMany('App\Kid', 'child_of_contact_id'); + } + + /** + * Get the note records associated with the contact. + */ + public function notes() + { + return $this->hasMany('App\Note'); + } + + /** + * Get the reminder records associated with the contact. + */ + public function reminders() + { + return $this->hasMany('App\Reminder')->orderBy('next_expected_date', 'asc'); + } + + /** + * Get the current significant other associated with the contact. + * + * @return SignificantOther + */ + public function significantOther() + { + return $this->hasOne('App\SignificantOther')->active(); + } + + /** + * Get the significant others associated with the contact. + */ + public function significantOthers() + { + return $this->hasMany('App\SignificantOther'); + } + + /** + * Get the task records associated with the contact. + */ + public function tasks() + { + return $this->hasMany('App\Task'); } /** @@ -45,12 +137,12 @@ class Contact extends Model { $completeName = $this->first_name; - if (! is_null($this->middle_name)) { - $completeName = $completeName.' '.$this->middle_name; + if (!is_null($this->middle_name)) { + $completeName = $completeName . ' ' . $this->middle_name; } - if (! is_null($this->last_name)) { - $completeName = $completeName.' '.$this->last_name; + if (!is_null($this->last_name)) { + $completeName = $completeName . ' ' . $this->last_name; } return $completeName; @@ -63,10 +155,6 @@ class Contact extends Model */ public function getFirstName() { - if (is_null($this->first_name)) { - return null; - } - return $this->first_name; } @@ -77,10 +165,6 @@ class Contact extends Model */ public function getMiddleName() { - if (is_null($this->middle_name)) { - return null; - } - return $this->middle_name; } @@ -91,10 +175,6 @@ class Contact extends Model */ public function getLastName() { - if (is_null($this->last_name)) { - return null; - } - return $this->last_name; } @@ -105,19 +185,9 @@ class Contact extends Model */ public function getInitials() { - $initials = $this->getFirstName()[0]; + preg_match_all('/(?<=\s|^)[a-zA-Z0-9]/i', $this->getCompleteName(), $initials); - if (! is_null($this->getMiddleName())) { - $initial = $this->getMiddleName()[0]; - $initials .= $initial; - } - - if (! is_null($this->getLastName())) { - $initial = $this->getLastName()[0]; - $initials .= $initial; - } - - return $initials; + return implode('', $initials[0]); } /** @@ -127,18 +197,16 @@ class Contact extends Model */ public function getLastActivityDate($timezone) { - $activity = Activity::where('account_id', $this->account_id) - ->where('contact_id', $this->id) - ->orderBy('date_it_happened', 'desc') - ->first(); - - if (count($activity) == 0) { + if ($this->activities->count() === 0) { return null; } - $last_activity_date = DateHelper::createDateFromFormat($activity->date_it_happened, $timezone); + $lastActivity = $this->activities->sortByDesc('date_it_happened')->first(); - return DateHelper::getShortDate($last_activity_date, 'en'); + return DateHelper::getShortDate( + DateHelper::createDateFromFormat($lastActivity->date_it_happened, $timezone), + 'en' + ); } /** @@ -152,9 +220,7 @@ class Contact extends Model return null; } - $lastTalkedTo = DateHelper::createDateFromFormat($this->last_talked_to, $timezone)->diffForHumans(); - - return $lastTalkedTo; + return DateHelper::createDateFromFormat($this->last_talked_to, $timezone)->diffForHumans(); } /** @@ -183,9 +249,7 @@ class Contact extends Model return null; } - $age = $this->birthdate->diffInYears(Carbon::now()); - - return $age; + return $this->birthdate->diffInYears(Carbon::now()); } /** @@ -209,6 +273,14 @@ class Contact extends Model */ public function isBirthdateApproximate() { + if ($this->is_birthdate_approximate === 'unknown') { + return true; + } + + if ($this->is_birthdate_approximate === 'exact') { + return false; + } + return $this->is_birthdate_approximate; } @@ -225,8 +297,8 @@ class Contact extends Model return null; } - if (! is_null($this->getProvince())) { - $address = $address.', '.$this->getProvince(); + if (!is_null($this->getProvince())) { + $address = $address . ', ' . $this->getProvince(); } return $address; @@ -277,14 +349,11 @@ class Contact extends Model */ public function getCountryName() { - $country = null; - - if (! is_null($this->country_id)) { - $country = Country::findOrFail($this->country_id); - $country = $country->country; + if ($this->country) { + return $this->country->country; } - return $country; + return null; } /** @@ -315,13 +384,11 @@ class Contact extends Model */ public function getCountryISO() { - $country = Country::find($this->country_id); - - if (count($country) == 0) { - return null; + if ($this->country) { + return $this->country->iso; } - return $country->iso; + return null; } /** @@ -344,7 +411,7 @@ class Contact extends Model */ public function getLastUpdated() { - return DateHelper::createDateFromFormat($this->updated_at, $this->user->timezone)->format('Y/m/d'); + return DateHelper::createDateFromFormat($this->updated_at, $this->account->user->timezone)->format('Y/m/d'); } /** @@ -354,7 +421,7 @@ class Contact extends Model */ public function getNumberOfReminders() { - return $this->number_of_reminders; + return $this->reminders->count(); } /** @@ -364,7 +431,7 @@ class Contact extends Model */ public function getNumberOfKids() { - return $this->number_of_kids; + return $this->kids->count(); } /** @@ -374,7 +441,7 @@ class Contact extends Model */ public function getNumberOfActivities() { - return $this->number_of_activities; + return $this->activities->count(); } /** @@ -384,7 +451,7 @@ class Contact extends Model */ public function getNumberOfGifts() { - return $this->number_of_gifts_ideas + $this->number_of_gifts_offered; + return $this->gifts->count(); } /** @@ -450,27 +517,17 @@ class Contact extends Model */ public function getCurrentSignificantOther() { - $significantOther = SignificantOther::where('account_id', $this->account_id) - ->where('contact_id', $this->id) - ->where('status', 'active') - ->first(); - - return $significantOther; + return $this->significantOther; } /** * Get the notes for this contact. Return an empty collection if no notes. * - * @return Notes + * @return Note */ public function getNotes() { - $notes = Note::where('contact_id', $this->id) - ->where('account_id', $this->account_id) - ->orderBy('id', 'desc') - ->get(); - - return $notes; + return $this->notes; } /** @@ -480,7 +537,7 @@ class Contact extends Model */ public function getNumberOfNotes() { - return $this->number_of_notes; + return $this->notes->count(); } /** @@ -490,11 +547,7 @@ class Contact extends Model */ public function getKids() { - $kids = Kid::where('account_id', $this->account_id) - ->where('child_of_contact_id', $this->id) - ->get(); - - return $kids; + return $this->kids; } /** @@ -524,39 +577,39 @@ class Contact extends Model /** * Set the default avatar color for this object. * + * @param string|null $color * @return void */ - public function setAvatarColor() + public function setAvatarColor($color = null) { $colors = [ - '#fdb660', - '#93521e', - '#bd5067', - '#b3d5fe', - '#ff9807', - '#709512', - '#5f479a', - '#e5e5cd', + '#fdb660', + '#93521e', + '#bd5067', + '#b3d5fe', + '#ff9807', + '#709512', + '#5f479a', + '#e5e5cd', ]; - $randomColorChosen = $colors[mt_rand(0, count($colors) - 1)]; - $this->default_avatar_color = $randomColorChosen; + $this->default_avatar_color = $color ?? $colors[mt_rand(0, count($colors) - 1)]; + $this->save(); } /** * Log an event in the Event table about this contact. * - * @param string $objectType Contact, Activity, Kid,... - * @param int $objectId ID of the object - * @param string $natureOfOperation 'add', 'edit', 'delete' + * @param string $objectType Contact, Activity, Kid,... + * @param int $objectId ID of the object + * @param string $natureOfOperation 'add', 'edit', 'delete' * @return int Id of the created event */ public function logEvent($objectType, $objectId, $natureOfOperation) { - $event = new Event; + $event = $this->events()->create([]); $event->account_id = $this->account_id; - $event->contact_id = $this->id; $event->object_type = $objectType; $event->object_id = $objectId; $event->nature_of_operation = $natureOfOperation; @@ -568,22 +621,24 @@ class Contact extends Model /** * Add a significant other. * - * @param string $name + * @param string $firstname * @param string $gender * @param bool $birthdate_approximate * @param string $birthdate * @param int $age + * @param string $timezone * @return int */ public function addSignificantOther($firstname, $gender, $birthdate_approximate, $birthdate, $age, $timezone) { - $significantOther = new SignificantOther; + $significantOther = $this->significantOthers()->create([]); + $significantOther->account_id = $this->account_id; - $significantOther->contact_id = $this->id; $significantOther->first_name = ucfirst($firstname); $significantOther->gender = $gender; $significantOther->is_birthdate_approximate = $birthdate_approximate; $significantOther->status = 'active'; + if ($birthdate_approximate == 'approximate') { $year = Carbon::now()->subYears($age)->year; $birthdate = Carbon::createFromDate($year, 1, 1); @@ -594,6 +649,7 @@ class Contact extends Model $birthdate = Carbon::createFromFormat('Y-m-d', $birthdate); $significantOther->birthdate = $birthdate; } + $significantOther->save(); // Event @@ -605,19 +661,20 @@ class Contact extends Model /** * Update the information about the Significant other. * - * @param int $significantOtherId + * @param SignificantOther|int $significantOther * @param string $firstname - * @param string $lastname * @param string $gender * @param string $birthdate_approximate * @param string $birthdate * @param int $age * @param string $timezone - * @return Response + * @return int */ - public function editSignificantOther($significantOtherId, $firstname, $gender, $birthdate_approximate, $birthdate, $age, $timezone) + public function editSignificantOther($significantOther, $firstname, $gender, $birthdate_approximate, $birthdate, $age, $timezone) { - $significantOther = SignificantOther::findOrFail($significantOtherId); + if (!$significantOther instanceof SignificantOther) { + $significantOther = SignificantOther::findOrFail($significantOther); + } $significantOther->first_name = ucfirst($firstname); $significantOther->gender = $gender; @@ -645,21 +702,23 @@ class Contact extends Model /** * Delete the significant other. + * + * @param SignificantOther|int $significantOther */ - public function deleteSignificantOther($significantOtherId) + public function deleteSignificantOther($significantOther) { - $significantOther = SignificantOther::findOrFail($significantOtherId); + if (!$significantOther instanceof SignificantOther) { + $significantOther = SignificantOther::findOrFail($significantOther); + } + $significantOther->delete(); - $events = Event::where('contact_id', $significantOther->contact_id) - ->where('account_id', $significantOther->account_id) - ->where('object_type', 'significantother') - ->where('object_id', $significantOther->id) - ->get(); - - foreach ($events as $event) { - $event->delete(); - } + $this->events() + ->where('object_type', 'significantother') + ->where('object_id', $significantOther->id) + ->get() + ->each + ->delete(); } /** @@ -678,13 +737,14 @@ class Contact extends Model $this->first_name = $firstName; - if (! is_null($middleName)) { + if (!is_null($middleName)) { $this->middle_name = $middleName; } - if (! is_null($lastName)) { + if (!is_null($lastName)) { $this->last_name = $lastName; } + $this->save(); return true; @@ -719,9 +779,8 @@ class Contact extends Model */ public function addKid($name, $gender, $birthdate_approximate, $birthdate, $age, $timezone) { - $kid = new Kid; + $kid = $this->kids()->create([]); $kid->account_id = $this->account_id; - $kid->child_of_contact_id = $this->id; $kid->first_name = ucfirst($name); $kid->gender = $gender; $kid->is_birthdate_approximate = $birthdate_approximate; @@ -751,16 +810,21 @@ class Contact extends Model /** * Edit a kid. * + * @param Kid|int $kid * @param string $name * @param string $gender * @param bool $birthdate_approximate * @param string $birthdate * @param int $age + * @param $timezone * @return int the Kid ID */ - public function editKid($kidId, $name, $gender, $birthdate_approximate, $birthdate, $age, $timezone) + public function editKid($kid, $name, $gender, $birthdate_approximate, $birthdate, $age, $timezone) { - $kid = Kid::findOrFail($kidId); + if (!$kid instanceof Kid) { + $kid = Kid::findOrFail($kid); + } + $kid->first_name = ucfirst($name); $kid->gender = $gender; $kid->is_birthdate_approximate = $birthdate_approximate; @@ -785,22 +849,24 @@ class Contact extends Model /** * Delete the kid. + * + * @param Kid|int $kid */ - public function deleteKid($kidId) + public function deleteKid($kid) { - $kid = Kid::findOrFail($kidId); + if (!$kid instanceof Kid) { + $kid = Kid::findOrFail($kid); + } + $kid->delete(); // Delete all events - $events = Event::where('contact_id', $kid->child_of_contact_id) - ->where('account_id', $kid->account_id) - ->where('object_type', 'kid') - ->where('object_id', $kid->id) - ->get(); - - foreach ($events as $event) { - $event->delete(); - } + $this->events() + ->where('object_type', 'kid') + ->where('object_id', $kid->id) + ->get() + ->each + ->delete(); // Decrease number of kids $this->number_of_kids = $this->number_of_kids - 1; @@ -809,19 +875,20 @@ class Contact extends Model $this->number_of_kids = 0; $this->has_kids = 'false'; } + $this->save(); } /** * Create a note. * - * @param string + * @param string $body + * @return mixed */ public function addNote($body) { - $note = new Note; + $note = $this->notes()->create([]); $note->account_id = $this->account_id; - $note->contact_id = $this->id; $note->body = $body; $note->save(); @@ -835,10 +902,15 @@ class Contact extends Model /** * Delete the note. + * + * @param Note|int $note */ - public function deleteNote($noteId) + public function deleteNote($note) { - $note = Note::findOrFail($noteId); + if (!$note instanceof Note) { + $note = Note::findOrFail($note); + } + $note->delete(); // Decrease number of notes @@ -847,18 +919,16 @@ class Contact extends Model if ($this->number_of_notes < 1) { $this->number_of_notes = 0; } + $this->save(); // Delete all events - $events = Event::where('contact_id', $note->contact_id) - ->where('account_id', $note->account_id) - ->where('object_type', 'note') - ->where('object_id', $note->id) - ->get(); - - foreach ($events as $event) { - $event->delete(); - } + $this->events() + ->where('object_type', 'note') + ->where('object_id', $note->id) + ->get() + ->each + ->delete(); } /** @@ -868,12 +938,7 @@ class Contact extends Model */ public function getActivities() { - $activities = Activity::where('account_id', $this->account_id) - ->where('contact_id', $this->id) - ->orderBy('date_it_happened', 'desc') - ->get(); - - return $activities; + return $this->activities; } /** @@ -885,56 +950,26 @@ class Contact extends Model public function calculateActivitiesStatistics() { // Delete the Activities statistics table for this contact - $activitiesStatistics = ActivityStatistic::where('account_id', $this->account_id) - ->where('contact_id', $this->id) - ->get(); - - foreach ($activitiesStatistics as $activityStatistic) { - $activityStatistic->delete(); - } + $this->activityStatistics->each->delete(); // Create the statistics again - foreach ($this->getActivities() as $activity) { - $year = $activity->date_it_happened->year; - - // Check to see if there is a year for this activity already - $activityStatistic = ActivityStatistic::where('account_id', $this->account_id) - ->where('contact_id', $this->id) - ->where('year', $year) - ->first(); - - if (count($activityStatistic) == 0) { - - // Year does not exist, create the record - $activityStatistic = new ActivityStatistic; + $this->activities->groupBy('date_it_happened.year') + ->map(function (Collection $activities, $year) { + $activityStatistic = $this->activityStatistics()->create([]); $activityStatistic->account_id = $this->account_id; - $activityStatistic->contact_id = $this->id; $activityStatistic->year = $year; - $activityStatistic->count = 1; + $activityStatistic->count = $activities->count();; $activityStatistic->save(); - - } else { - - // Year does exist, increment the record - $activityStatistic->count = $activityStatistic->count + 1; - $activityStatistic->save(); - } - } + }); } /** * Get statistics for the contact * TODO: add unit test. - * - * @param int $contactId */ public function getActivitiesStats() { - $statistics = ActivityStatistic::where('contact_id', $this->id) - ->orderBy('year', 'desc') - ->get(); - - return $statistics; + return $this->activityStatistics; } /** @@ -944,12 +979,7 @@ class Contact extends Model */ public function getReminders() { - $reminders = Reminder::where('account_id', $this->account_id) - ->where('contact_id', $this->id) - ->orderBy('next_expected_date', 'asc') - ->get(); - - return $reminders; + return $this->reminders; } /** @@ -959,9 +989,7 @@ class Contact extends Model */ public function getGifts() { - return Gift::where('account_id', $this->account_id) - ->where('contact_id', $this->id) - ->get(); + return $this->gifts; } /** @@ -969,10 +997,7 @@ class Contact extends Model */ public function getGiftsOffered() { - return Gift::where('account_id', $this->account_id) - ->where('contact_id', $this->id) - ->where('has_been_offered', 'true') - ->get(); + return $this->gifts()->offered()->get(); } /** @@ -980,10 +1005,7 @@ class Contact extends Model */ public function getGiftIdeas() { - return Gift::where('account_id', $this->account_id) - ->where('contact_id', $this->id) - ->where('is_an_idea', 'true') - ->get(); + return $this->gifts()->isIdea()->get(); } /** @@ -991,10 +1013,7 @@ class Contact extends Model */ public function getTasksInProgress() { - return Task::where('account_id', $this->account_id) - ->where('contact_id', $this->id) - ->where('status', 'inprogress') - ->get(); + return $this->tasks()->inProgress()->get(); } /** @@ -1002,9 +1021,7 @@ class Contact extends Model */ public function getTasks() { - return Task::where('account_id', $this->account_id) - ->where('contact_id', $this->id) - ->get(); + return $this->tasks; } /** @@ -1012,10 +1029,7 @@ class Contact extends Model */ public function getCompletedTasks() { - return Task::where('account_id', $this->account_id) - ->where('contact_id', $this->id) - ->where('status', 'completed') - ->get(); + return $this->tasks()->completed()->get(); } /** @@ -1028,19 +1042,19 @@ class Contact extends Model $original_avatar_url = Storage::disk('public')->url($this->avatar_file_name); $avatar_filename = pathinfo($original_avatar_url, PATHINFO_FILENAME); $avatar_extension = pathinfo($original_avatar_url, PATHINFO_EXTENSION); - $resized_avatar = 'avatars/'.$avatar_filename.'_'.$size.'.'.$avatar_extension; + $resized_avatar = 'avatars/' . $avatar_filename . '_' . $size . '.' . $avatar_extension; return Storage::disk('public')->url($resized_avatar); } public function getGravatar($size) { - if ( empty( $this->email ) ) { + if (empty($this->email)) { return false; } - $gravatar_url = "https://www.gravatar.com/avatar/" . md5( strtolower( trim( $this->email ) ) ); + $gravatar_url = "https://www.gravatar.com/avatar/" . md5(strtolower(trim($this->email))); // check if gravatar exists by appending ?d=404, returns 404 response if does not exist - $gravatarHeaders = get_headers( $gravatar_url . "?d=404" ); + $gravatarHeaders = get_headers($gravatar_url . "?d=404"); if ($gravatarHeaders[0] == "HTTP/1.1 404 Not Found") { return false; } @@ -1054,10 +1068,7 @@ class Contact extends Model */ public function hasDebt() { - return Debt::where('account_id', $this->account_id) - ->where('contact_id', $this->id) - ->where('status', 'inprogress') - ->count(); + return $this->debts !== null; } /** @@ -1065,9 +1076,7 @@ class Contact extends Model */ public function getDebts() { - return Debt::where('account_id', $this->account_id) - ->where('contact_id', $this->id) - ->get(); + return $this->debts; } } diff --git a/app/Debt.php b/app/Debt.php index c446bb784..3dbbee0d0 100644 --- a/app/Debt.php +++ b/app/Debt.php @@ -4,7 +4,38 @@ namespace App; use App\Helpers\DateHelper; use Illuminate\Database\Eloquent\Model; +use Illuminate\Database\Eloquent\Builder; class Debt extends Model { + /** + * Get the account record associated with the debt. + */ + public function account() + { + return $this->belongsTo('App\Account'); + } + + /** + * Get the contact record associated with the debt. + */ + public function contact() + { + return $this->belongsTo('App\Contact'); + } + + public function scopeInProgress(Builder $query) + { + return $query->where('status', 'inprogress'); + } + + public function scopeDue(Builder $query) + { + return $query->where('in_debt', 'yes'); + } + + public function scopeOwed(Builder $query) + { + return $query->where('in_debt', 'no'); + } } diff --git a/app/Entry.php b/app/Entry.php index 628137efb..8578ff1bc 100644 --- a/app/Entry.php +++ b/app/Entry.php @@ -8,6 +8,14 @@ class Entry extends Model { protected $table = 'entries'; + /** + * Get the account record associated with the entry. + */ + public function account() + { + return $this->belongsTo('App\Account'); + } + public function getPost() { if (is_null($this->post)) { diff --git a/app/Event.php b/app/Event.php index 3e6a4eab9..adf00d89c 100644 --- a/app/Event.php +++ b/app/Event.php @@ -9,6 +9,22 @@ use Illuminate\Database\Eloquent\Model; class Event extends Model { + /** + * Get the account record associated with the event. + */ + public function account() + { + return $this->belongsTo('App\Account'); + } + + /** + * Get the contact record associated with the event. + */ + public function contact() + { + return $this->belongsTo('App\Contact'); + } + public function getDescription() { if ($this->nature_of_operation == 'create') { diff --git a/app/Gift.php b/app/Gift.php index 83d3540ed..8b878fb67 100644 --- a/app/Gift.php +++ b/app/Gift.php @@ -5,6 +5,7 @@ namespace App; use App\Helpers\DateHelper; use App\Events\Gift\GiftCreated; use App\Events\Gift\GiftDeleted; +use Illuminate\Database\Eloquent\Builder; use Illuminate\Database\Eloquent\Model; class Gift extends Model @@ -18,6 +19,32 @@ class Gift extends Model 'deleted' => GiftDeleted::class, ]; + /** + * Get the account record associated with the gift. + */ + public function account() + { + return $this->belongsTo('App\Account'); + } + + /** + * Get the contact record associated with the gift. + */ + public function contact() + { + return $this->belongsTo('App\Contact'); + } + + public function scopeOffered(Builder $query) + { + return $query->where('has_been_offered', 'true'); + } + + public function scopeIsIdea(Builder $query) + { + return $query->where('is_an_idea', 'true'); + } + public function getName() { if (is_null($this->name)) { diff --git a/app/Http/Controllers/DashboardController.php b/app/Http/Controllers/DashboardController.php index 7a0c5d133..e2110f708 100644 --- a/app/Http/Controllers/DashboardController.php +++ b/app/Http/Controllers/DashboardController.php @@ -23,100 +23,72 @@ class DashboardController extends Controller */ public function index() { - $lastUpdatedContacts = Contact::where('account_id', Auth::user()->account_id) - ->orderBy('updated_at', 'desc') - ->limit(10) - ->get(); + $account = Auth::user()->account; + + $lastUpdatedContacts = $account->contacts()->limit(10)->get(); // Latest statistics - $contacts = Contact::where('account_id', Auth::user()->account_id) - ->get(); - - if (count($contacts) == 0) { + if ($account->contacts()->count() === 0) { return view('dashboard.blank'); } - $number_of_contacts = $contacts->count(); - $number_of_reminders = 0; - $number_of_notes = 0; - $number_of_activities = 0; - $number_of_gifts = 0; - $number_of_tasks = 0; - $number_of_kids = 0; - $debt_owed = 0; - $debt_due = 0; + $number_of_contacts = $account->contacts()->count(); + $number_of_reminders = $account->reminders()->count(); + $number_of_notes = $account->notes()->count(); + $number_of_activities = $account->activities()->count(); + $number_of_gifts = $account->gifts()->count(); + $number_of_tasks = $account->tasks()->count(); + $number_of_kids = $account->kids()->count(); - foreach ($contacts as $contact) { - $number_of_reminders = $number_of_reminders + $contact->number_of_reminders; - $number_of_notes = $number_of_notes + $contact->number_of_notes; - $number_of_activities = $number_of_activities + $contact->number_of_activities; - $number_of_gifts = $number_of_gifts + $contact->number_of_gifts_ideas; - $number_of_gifts = $number_of_gifts + $contact->number_of_gifts_received; - $number_of_gifts = $number_of_gifts + $contact->number_of_gifts_offered; - $number_of_tasks = $number_of_tasks + $contact->number_of_tasks_in_progress; - $number_of_tasks = $number_of_tasks + $contact->number_of_tasks_completed; - $number_of_kids = $number_of_kids + $contact->number_of_kids; - } + $debt = $account->debt->where('status', 'inprogress'); - $debts = Debt::where('account_id', Auth::user()->account_id) - ->where('status', 'inprogress') - ->where('in_debt', 'yes') - ->get(); + $debt_due = $debt->where('in_debt', 'yes') + ->reduce(function ($totalDueDebt, Debt $debt) { + return $totalDueDebt + $debt->amount; + }, 0); - foreach ($debts as $debt) { - $debt_due = $debt_due + $debt->amount; - } - - $debts = Debt::where('account_id', Auth::user()->account_id) - ->where('status', 'inprogress') - ->where('in_debt', 'no') - ->get(); - - foreach ($debts as $debt) { - $debt_owed = $debt_owed + $debt->amount; - } + $debt_owed = $debt->where('in_debt', 'no') + ->reduce(function ($totalOwedDebt, Debt $debt) { + return $totalOwedDebt + $debt->amount; + }, 0); // List of events - $events = Event::where('account_id', Auth::user()->account_id) - ->orderBy('created_at', 'desc') - ->limit(30) - ->get(); + $events = $account->events()->with('contact.significantOthers', 'contact.kids')->limit(30)->get() + ->reject(function (Event $event) { + return $event->contact === null; + }) + ->map(function (Event $event) use ($account) { - $eventsArray = collect(); + if ($event->object_type === 'significantother') { + $object = $event->contact->significantOthers->where('id', $event->object_id)->first(); + } elseif ($event->object_type === 'kid') { + $object = $event->contact->kids->where('id', $event->object_id)->first(); + } - foreach ($events as $event) { - $contact = Contact::findOrFail($event->contact_id); - - $eventsArray->push([ - 'id' => $event->id, - 'date' => DateHelper::createDateFromFormat($event->created_at, Auth::user()->timezone), - 'object_type' => $event->object_type, - 'object_id' => $event->object_id, - 'contact_id' => $contact->id, - 'contact_complete_name' => $contact->getCompleteName(), - 'nature_of_operation' => $event->nature_of_operation, - ]); - } + return [ + 'id' => $event->id, + 'date' => DateHelper::createDateFromFormat($event->created_at, Auth::user()->timezone), + 'object' => $object ?? null, + 'object_type' => $event->object_type, + 'object_id' => $event->object_id, + 'contact_id' => $event->contact->id, + 'contact_complete_name' => $event->contact->getCompleteName(), + 'nature_of_operation' => $event->nature_of_operation, + ]; + }); // List of upcoming reminders - $upcomingReminders = Reminder::where('account_id', Auth::user()->account_id) - ->where('next_expected_date', '>', Carbon::now()) - ->orderBy('next_expected_date', 'asc') - ->limit(10) - ->get(); + $upcomingReminders = $account->reminders() + ->where('next_expected_date', '>', Carbon::now()) + ->orderBy('next_expected_date', 'asc') + ->limit(10) + ->get(); // Active tasks - $tasks = Task::where('account_id', Auth::user()->account_id) - ->where('status', 'inprogress') - ->get(); - - // Debts - $debts = Debt::where('account_id', Auth::user()->account_id) - ->where('status', 'inprogress') - ->get(); + $tasks = $account->tasks->where('status', 'inprogress'); $data = [ - 'events' => $eventsArray, + 'events' => $events, 'lastUpdatedContacts' => $lastUpdatedContacts, 'upcomingReminders' => $upcomingReminders, 'number_of_contacts' => $number_of_contacts, @@ -129,7 +101,7 @@ class DashboardController extends Controller 'debt_due' => $debt_due, 'debt_owed' => $debt_owed, 'tasks' => $tasks, - 'debts' => $debts, + 'debts' => $debt, ]; return view('dashboard.index', $data); diff --git a/app/Kid.php b/app/Kid.php index b9a4aee23..83710efd0 100644 --- a/app/Kid.php +++ b/app/Kid.php @@ -12,6 +12,22 @@ class Kid extends Model { protected $dates = ['birthdate']; + /** + * Get the account record associated with the kid. + */ + public function account() + { + return $this->belongsTo('App\Account'); + } + + /** + * Get the contact record associated with the kid. + */ + public function parent() + { + return $this->belongsTo('App\Contact', 'child_of_contact_id'); + } + /** * Gets the age of the kid in years, or returns null if the birthdate * is not set. diff --git a/app/Note.php b/app/Note.php index 88c8978c6..497793152 100644 --- a/app/Note.php +++ b/app/Note.php @@ -7,6 +7,22 @@ use Illuminate\Database\Eloquent\Model; class Note extends Model { + /** + * Get the account record associated with the note. + */ + public function account() + { + return $this->belongsTo('App\Account'); + } + + /** + * Get the contact record associated with the note. + */ + public function contact() + { + return $this->belongsTo('App\Contact'); + } + /** * Get the description of a note. * @return string diff --git a/app/Reminder.php b/app/Reminder.php index c3f8aff85..508c239a7 100644 --- a/app/Reminder.php +++ b/app/Reminder.php @@ -12,6 +12,22 @@ class Reminder extends Model { protected $dates = ['last_triggered', 'next_expected_date']; + /** + * Get the account record associated with the reminder. + */ + public function account() + { + return $this->belongsTo('App\Account'); + } + + /** + * Get the contact record associated with the reminder. + */ + public function contact() + { + return $this->belongsTo('App\Contact'); + } + /** * Get the title of a reminder. * @return string diff --git a/app/SignificantOther.php b/app/SignificantOther.php index 3df9e8815..37f2ae765 100644 --- a/app/SignificantOther.php +++ b/app/SignificantOther.php @@ -3,6 +3,7 @@ namespace App; use Carbon\Carbon; +use Illuminate\Database\Eloquent\Builder; use Illuminate\Database\Eloquent\Model; class SignificantOther extends Model @@ -13,6 +14,33 @@ class SignificantOther extends Model 'birthdate', ]; + /** + * Get the account record associated with the significant other. + */ + public function account() + { + return $this->belongsTo('App\Account'); + } + + /** + * Get the contact record associated with the significant other. + */ + public function contact() + { + return $this->belongsTo('App\Contact'); + } + + /** + * Limit the query to active significant others + * + * @param Builder $query + * @return Builder + */ + public function scopeActive(Builder $query) + { + return $query->where('status', 'active'); + } + /** * Get the first name of the significant other. * diff --git a/app/Task.php b/app/Task.php index 0ccf1ea6b..873d4cb32 100644 --- a/app/Task.php +++ b/app/Task.php @@ -4,12 +4,31 @@ namespace App; use Auth; use App\Helpers\DateHelper; +use Illuminate\Database\Eloquent\Builder; use Illuminate\Database\Eloquent\Model; class Task extends Model { protected $dates = ['completed_at']; + /** + * Get the account record associated with the task. + */ + public function account() + { + return $this->belongsTo('App\Account'); + } + + public function scopeCompleted(Builder $query) + { + return $query->where('status', 'completed'); + } + + public function scopeInProgress(Builder $query) + { + return $query->where('status', 'inprogress'); + } + public function getTitle() { if (is_null($this->title)) { diff --git a/app/User.php b/app/User.php index 0ae1163f3..f0b784d45 100644 --- a/app/User.php +++ b/app/User.php @@ -20,6 +20,13 @@ class User extends Authenticatable 'name', 'email', 'password', ]; + /** + * Eager load account with every user. + */ + protected $with = [ + 'account', + ]; + /** * The attributes that should be hidden for arrays. * @@ -36,7 +43,7 @@ class User extends Authenticatable */ public function account() { - return $this->hasOne('App\Account'); + return $this->belongsTo('App\Account'); } /** diff --git a/resources/views/dashboard/events/_kids.blade.php b/resources/views/dashboard/events/_kids.blade.php index 7bbdc2679..340414f0c 100644 --- a/resources/views/dashboard/events/_kids.blade.php +++ b/resources/views/dashboard/events/_kids.blade.php @@ -1,5 +1,5 @@ {{-- You added Jane as a child of Jane Doe --}} -{{ App\Kid::findOrFail($event['object_id'])->getFirstName() }}, +{{ $event['object']->getFirstName() }}, {{ $event['contact_complete_name'] }} {{ trans('dashboard.event_child') }} diff --git a/resources/views/dashboard/events/_significantothers.blade.php b/resources/views/dashboard/events/_significantothers.blade.php index c281cace9..d41d63c15 100644 --- a/resources/views/dashboard/events/_significantothers.blade.php +++ b/resources/views/dashboard/events/_significantothers.blade.php @@ -1,6 +1,6 @@ {{-- You added Jane Doe as Regis's significant other --}} -{{ App\SignificantOther::find($event['object_id'])->getName() }} +{{ $event['object']->getName() }} {{ trans('dashboard.event_as') }} diff --git a/tests/Unit/ContactTest.php b/tests/Unit/ContactTest.php index 5e139dd74..58e94f33d 100644 --- a/tests/Unit/ContactTest.php +++ b/tests/Unit/ContactTest.php @@ -261,10 +261,9 @@ class ContactTest extends TestCase public function testGetNumberOfReminders() { $contact = new Contact; - $contact->number_of_reminders = 3; $this->assertEquals( - 3, + 0, $contact->getNumberOfReminders() ); } @@ -272,11 +271,9 @@ class ContactTest extends TestCase public function testGetNumberOfGifts() { $contact = new Contact; - $contact->number_of_gifts_offered = 3; - $contact->number_of_gifts_ideas = 2; $this->assertEquals( - 5, + 0, $contact->getNumberOfGifts() ); } @@ -284,10 +281,9 @@ class ContactTest extends TestCase public function testGetNumberOfActivities() { $contact = new Contact; - $contact->number_of_activities = 3; $this->assertEquals( - 3, + 0, $contact->getNumberOfActivities() ); } @@ -943,8 +939,7 @@ class ContactTest extends TestCase { $contact = new Contact; - $this->assertEquals( - 0, + $this->assertFalse( $contact->hasDebt() ); }