Fix: Optimize course times display

#237

Author: rm-yakovenko

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;
@@ -254,10 +255,10 @@ public function saveCourseTimes($newCourseTimes)
                     ->where('end', Carbon::parse($courseTime['end'])->toTimeString())
                     ->count() == 0) {
                 $this->times()->create([
-                    'day' => $courseTime['day'],
-                    'start' => Carbon::parse($courseTime['start'])->toTimeString(),
-                    'end' => Carbon::parse($courseTime['end'])->toTimeString(),
-                ]);
+                                           'day' => $courseTime['day'],
+                                           'start' => Carbon::parse($courseTime['start'])->toTimeString(),
+                                           'end' => Carbon::parse($courseTime['end'])->toTimeString(),
+                                       ]);
             }
         }
     }
@@ -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()

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()
     {