From b2af9fe41b63a702b13124a02c6325664fb9b549 Mon Sep 17 00:00:00 2001 From: Rm Yakovenko Date: Tue, 29 Mar 2022 08:20:37 +0300 Subject: [PATCH 1/2] Fix: Optimize course times display https://github.com/academico-sis/academico/issues/237 --- app/Models/Course.php | 55 +++++++++++++++++++++++++++------------ tests/Unit/CourseTest.php | 52 ++++++++++++++++++++++++++++++++++++ 2 files changed, 91 insertions(+), 16 deletions(-) diff --git a/app/Models/Course.php b/app/Models/Course.php index cc9555b4b..40727f4da 100644 --- a/app/Models/Course.php +++ b/app/Models/Course.php @@ -9,6 +9,7 @@ use Carbon\Carbon; use Illuminate\Database\Eloquent\Casts\Attribute; use Illuminate\Database\Eloquent\Model; +use Illuminate\Support\Collection; use Illuminate\Support\Facades\App; use Illuminate\Support\Str; use Spatie\Activitylog\LogOptions; @@ -274,7 +275,6 @@ public function saveCourseTimes($newCourseTimes) */ public function getCourseTimesAttribute() { - $parsedCourseTimes = []; // TODO localize these $daysInitials = [ __('Sun'), @@ -293,28 +293,51 @@ public function getCourseTimesAttribute() $courseTimes = $this->children->first()->times; } - if ($courseTimes) { - foreach ($courseTimes as $courseTime) { - $initial = $daysInitials[$courseTime->day]; + foreach ($courseTimes as $courseTime) { + $timeString = sprintf( + '%s - %s', + Carbon::parse($courseTime->start)->locale(App::getLocale())->isoFormat('LT'), + Carbon::parse($courseTime->end)->locale(App::getLocale())->isoFormat('LT') + ); + $courseTime->timeString = $timeString; + $courseTime->dayString = $daysInitials[$courseTime->day]; + } - if (! isset($parsedCourseTimes[$initial])) { - $parsedCourseTimes[$initial] = []; + $courseTimes = $courseTimes->sortBy('day'); + foreach ($courseTimes->groupBy('timeString') as $groupedCourseTimes) { + $currentSeq = []; + foreach ($groupedCourseTimes as $courseTime) { + $prevCourseTime = end($currentSeq); + if ($prevCourseTime && ($courseTime->day - $prevCourseTime->day) !== 1) { + $currentSeq = []; } - - $parsedCourseTimes[$initial][] = sprintf( - '%s - %s', - Carbon::parse($courseTime->start)->locale(App::getLocale())->isoFormat('LT'), - Carbon::parse($courseTime->end)->locale(App::getLocale())->isoFormat('LT') - ); + $currentSeq[] = $courseTime; + $courseTime->firstOfSeq = reset($currentSeq); } } - $result = ''; - foreach ($parsedCourseTimes as $day => $times) { - $result .= $day.' '.implode(' / ', $times).' | '; + $groups = $courseTimes->groupBy(fn (CourseTime $courseTime) => $courseTime->firstOfSeq->id) + ->groupBy(fn (Collection $seqGroup) => $seqGroup->count() > 1 ? 'multi_days' : 'multi_times'); + + $result = []; + + // Instead of showing this: + // Mon 9:00 - 5:00 | Tue 9:00 - 5:00 | Wed 9:00 - 5:00 | Thu 9:00 - 5:00 | Fri 9:00 - 5:00 + // we could show: + // Mon - Fri 9:00 - 5:00 + foreach (collect($groups->get('multi_days', [])) as $group) { + $firstDay = $group->first(); + $lastDay = $group->last(); + $result[] = "{$firstDay->dayString} - {$lastDay->dayString} {$firstDay->timeString}"; + } + + // Mon 10:00 AM - 11:00 AM / 11:30 AM - 12:45 PM + foreach (collect($groups->get('multi_times', []))->flatten()->groupBy('day') as $group) { + $firstDay = $group->first(); + $result[] = "{$firstDay->dayString} {$group->pluck('timeString')->join(' / ')}"; } - return trim($result, ' | '); + return implode(' | ', $result); } public function getCoursePeriodNameAttribute() diff --git a/tests/Unit/CourseTest.php b/tests/Unit/CourseTest.php index aea1803df..841f676e9 100644 --- a/tests/Unit/CourseTest.php +++ b/tests/Unit/CourseTest.php @@ -207,6 +207,58 @@ public function course_with_two_schedules_on_same_day_is_correctly_parsed() $this->assertSame('Mon 10:00 AM - 11:00 AM / 11:30 AM - 12:45 PM', $courseTimeParsed); } + + /** + * @link https://github.com/academico-sis/academico/issues/237 + * + * @test + */ + public function course_with_sequential_days_is_correctly_parsed() + { + // given a course and events + $course = factory(Course::class)->create(); + + foreach (range(1, 3) as $day) { + $course->times()->create( + [ + 'course_id' => $course->id, + 'day' => $day, + 'start' => '10:00', + 'end' => '11:00', + ] + ); + } + + // day in the sequence but with different time + $course->times()->create( + [ + 'course_id' => $course->id, + 'day' => 4, + 'start' => '10:15', + 'end' => '11:00', + ] + ); + + + foreach (range(5, 6) as $day) { + $course->times()->create( + [ + 'course_id' => $course->id, + 'day' => $day, + 'start' => '10:00', + 'end' => '11:00', + ] + ); + } + + $courseTimeParsed = $course->course_times; + + $this->assertSame( + 'Mon - Wed 10:00 AM - 11:00 AM | Fri - Sat 10:00 AM - 11:00 AM | Thu 10:15 AM - 11:00 AM', + $courseTimeParsed + ); + } + /** @test */ public function course_with_two_schedules_on_different_day_is_correctly_parsed() { From 0f66bff453d904dd742b79e2400fe84dce1478bf Mon Sep 17 00:00:00 2001 From: Rm Yakovenko Date: Sat, 2 Apr 2022 14:19:22 +0300 Subject: [PATCH 2/2] Also, it's important to check if $courseTimes is null otherwise it will throw an error when at foreach() --- app/Models/Course.php | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/app/Models/Course.php b/app/Models/Course.php index 40727f4da..402040ade 100644 --- a/app/Models/Course.php +++ b/app/Models/Course.php @@ -293,6 +293,10 @@ public function getCourseTimesAttribute() $courseTimes = $this->children->first()->times; } + if (!$courseTimes) { + return ''; + } + foreach ($courseTimes as $courseTime) { $timeString = sprintf( '%s - %s',