Fixing Check-In duration when stopover line is a ring (#201)

* Add: Testcases that represent the issue. (With Potsdam 603 Bus and Berlin Ringbahn) * Fix: Negative checkin durations when a station appears more than once on a stopover line. * Revert * Fixed trips at ring lines * Readded old test from jeyemwey * Update Controller.php * Adapted HafasTripFactory to DB-Rest v5 * Adapted CheckInCollision-Test to DB-Rest v5 format * Revert "Adapted CheckInCollision-Test to DB-Rest v5 format" This reverts commit d23c85b. * Update CheckinTest.php * Codacy * Codacy * Adapted HafasTripFactory to DB-Rest v5 * Codacy * I don't know what Codacy wants from me. * Is this what Codacy wants? * Codacy is weird * These indentations look funny, but for Codacy I do everything * Added comments to database columns (old migration) Comments cannot be added later with Laravel Builder * Show correct duration * Codacy Co-authored-by: Jannik <jannik@outlook.com> Co-authored-by: Levin Herr <levin@herrlev.in>
Traewelling · Feb 5, 2021 · 8610064 · 8610064
1 parent 8dce7ba
commit 8610064
Show file tree

Hide file tree

Showing 11 changed files with 315 additions and 76 deletions.
diff --git a/app/Http/Controllers/Controller.php b/app/Http/Controllers/Controller.php
@@ -3,6 +3,7 @@
 namespace App\Http\Controllers;
 
 use App\Models\PolyLine;
+use Carbon\Carbon;
 use Illuminate\Foundation\Auth\Access\AuthorizesRequests;
 use Illuminate\Foundation\Bus\DispatchesJobs;
 use Illuminate\Foundation\Validation\ValidatesRequests;
@@ -12,12 +13,26 @@ class Controller extends BaseController
 {
     use AuthorizesRequests, DispatchesJobs, ValidatesRequests;
 
-    public static function searchForId($stationId, $array) {
+    public static function searchForId(
+        int $stationId,
+        array $array,
+        Carbon $departure = null,
+        Carbon $arrival = null
+    ): ?int {
         foreach ($array as $key => $val) {
-            if ($val['stop']['id'] === $stationId) {
+            if ($val['stop']['id'] != $stationId) {
+                continue;
+            }
+            $stopDeparture        = Carbon::parse($val['plannedDeparture'] ?? $val['departure']);
+            $stopArrival          = Carbon::parse($val['plannedArrival'] ?? $val['arrival']);
+            $departureTimeMatches = $departure == null || $departure == $stopDeparture;
+            $arrivalTimeMatches   = $arrival == null || $arrival == $stopArrival;
+
+            if ($departureTimeMatches && $arrivalTimeMatches) {
                 return $key;
             }
         }
+
         return null;
     }
 

diff --git a/app/Http/Controllers/FrontendTransportController.php b/app/Http/Controllers/FrontendTransportController.php
@@ -82,10 +82,19 @@ public function StationByCoordinates(Request $request): RedirectResponse {
     }
 
     public function TrainTrip(Request $request): Renderable|RedirectResponse {
+
+        $request->validate([
+                               'tripID'    => ['required'],
+                               'lineName'  => ['required'],
+                               'start'     => ['required', 'numeric'],
+                               'departure' => ['required', 'date']
+                           ]);
+
         $TrainTripResponse = TransportBackend::TrainTrip(
             $request->tripID,
             $request->lineName,
-            $request->start
+            $request->start,
+            Carbon::parse($request->departure)
         );
         if ($TrainTripResponse === null) {
             return redirect()->back()->with('error', __('controller.transport.not-in-stopovers'));
@@ -115,7 +124,9 @@ public function TrainCheckin(Request $request): RedirectResponse {
             'business_check' => 'max:0', // Wenn wir Businesstrips wieder einbringen, kann man das wieder auf mehr stellen.
             'tweet_check'    => 'max:2',
             'toot_check'     => 'max:2',
-            'event'          => 'integer'
+            'event'          => 'integer',
+            'departure'      => ['required', 'date'],
+            'arrival'        => ['required', 'date'],
         ]);
         try {
             $trainCheckin = TransportBackend::TrainCheckin(
@@ -127,7 +138,9 @@ public function TrainCheckin(Request $request): RedirectResponse {
                 $request->business_check,
                 $request->tweet_check,
                 $request->toot_check,
-                $request->event
+                $request->event,
+                Carbon::parse($request->departure),
+                Carbon::parse($request->arrival),
             );
 
             return redirect()->route('dashboard')->with('checkin-success', [
@@ -139,21 +152,21 @@ public function TrainCheckin(Request $request): RedirectResponse {
                 'event'                => $trainCheckin['event']
             ]);
 
-        } catch (CheckInCollisionException $e) {
+        } catch (CheckInCollisionException $exception) {
 
             return redirect()
                 ->route('dashboard')
                 ->with('error', __(
                     'controller.transport.overlapping-checkin',
                     [
-                        'url'      => url('/status/' . $e->getCollision()->status->id),
-                        'id'       => $e->getCollision()->status->id,
-                        'linename' => $e->getCollision()->HafasTrip->linename
+                        'url'      => url('/status/' . $exception->getCollision()->status->id),
+                        'id'       => $exception->getCollision()->status->id,
+                        'linename' => $exception->getCollision()->HafasTrip->linename
                     ]
                 ));
 
-        } catch (Throwable $e) {
-
+        } catch (Throwable $exception) {
+            report($exception);
             return redirect()
                 ->route('dashboard')
                 ->with('error', __('messages.exception.general'));

diff --git a/app/Http/Controllers/TransportController.php b/app/Http/Controllers/TransportController.php
@@ -173,13 +173,14 @@ public static function sortByWhenOrScheduledWhen(array $departuresList): array {
      * @param string $tripId TripID in Hafas format
      * @param string $lineName Line Name in Hafas format
      * @param $start
+     * @param Carbon|null $departure
      * @return array|null
      */
-    public static function TrainTrip(string $tripId, string $lineName, $start): ?array {
+    public static function TrainTrip(string $tripId, string $lineName, $start, Carbon $departure = null): ?array {
 
         $hafasTrip = HafasController::getHafasTrip($tripId, $lineName);
         $stopovers = json_decode($hafasTrip->stopovers, true);
-        $offset    = self::searchForId($start, $stopovers);
+        $offset    = self::searchForId($start, $stopovers, $departure);
         if ($offset === null) {
             return null;
         }
@@ -259,11 +260,14 @@ public static function TrainCheckin($tripId,
                                         $businessCheck,
                                         $tweetCheck,
                                         $tootCheck,
-                                        $eventId = 0): array {
+                                        $eventId = 0,
+                                        Carbon $departure = null,
+                                        Carbon $arrival = null): array {
+
         $hafasTrip             = HafasTrip::where('trip_id', $tripId)->first();
         $stopovers             = json_decode($hafasTrip->stopovers, true);
-        $offset1               = self::searchForId($start, $stopovers);
-        $offset2               = self::searchForId($destination, $stopovers);
+            $offset1           = self::searchForId($start, $stopovers, $departure);
+            $offset2           = self::searchForId($destination, $stopovers, null, $arrival);
         $polyline              = self::polyline($start, $destination, $hafasTrip->polyline);
         $originAttributes      = $stopovers[$offset1];
         $destinationAttributes = $stopovers[$offset2];
@@ -312,7 +316,6 @@ public static function TrainCheckin($tripId,
         $departure = Carbon::parse($stopovers[$offset1]['departure']);
         $departure->subSeconds($stopovers[$offset1]['departureDelay'] ?? 0);
 
-
         if ($stopovers[$offset2]['arrival']) {
             $arrival = Carbon::parse($stopovers[$offset2]['arrival']);
             $arrival->subSeconds($stopovers[$offset2]['arrivalDelay'] ?? 0);
@@ -332,6 +335,13 @@ public static function TrainCheckin($tripId,
                                      'business' => isset($businessCheck) && $businessCheck == 'on'
                                  ]);
 
+            $plannedDeparture = Carbon::parse(
+                $stopovers[$offset1]['plannedDeparture'] ?? $stopovers[$offset2]['plannedArrival']
+            );
+            $plannedArrival   = Carbon::parse(
+                $stopovers[$offset2]['plannedArrival'] ?? $stopovers[$offset2]['plannedDeparture']
+            );
+
         $trainCheckin = TrainCheckin::create([
                                                  'status_id'   => $status->id,
                                                  'trip_id'     => $tripId,
@@ -340,8 +350,8 @@ public static function TrainCheckin($tripId,
                                                  'distance'    => $distance,
                                                  'delay'       => $hafasTrip->delay,
                                                  'points'      => $points,
-                                                 'departure'   => $departure,
-                                                 'arrival'     => $arrival
+                                                 'departure'   => $plannedDeparture,
+                                                 'arrival'     => $plannedArrival
                                              ]);
 
         $user->load(['statuses']);

diff --git a/app/Models/TrainCheckin.php b/app/Models/TrainCheckin.php
@@ -2,6 +2,8 @@
 
 namespace App\Models;
 
+use Carbon\Carbon;
+use Exception;
 use Illuminate\Database\Eloquent\Model;
 use Illuminate\Database\Eloquent\Relations\BelongsTo;
 use Illuminate\Database\Eloquent\Relations\HasOne;
@@ -34,11 +36,15 @@ public function HafasTrip(): HasOne {
     }
 
     public function getOriginStopoverAttribute(): ?TrainStopover {
-        return $this->HafasTrip->stopoversNEW->where('train_station_id', $this->Origin->id)->first();
+        return $this->HafasTrip->stopoversNEW->where('train_station_id', $this->Origin->id)
+                                             ->where('departure_planned', $this->departure->toIso8601String())
+                                             ->first();
     }
 
     public function getDestinationStopoverAttribute(): ?TrainStopover {
-        return $this->HafasTrip->stopoversNEW->where('train_station_id', $this->Destination->id)->first();
+        return $this->HafasTrip->stopoversNEW->where('train_station_id', $this->Destination->id)
+                                             ->where('arrival_planned', $this->arrival->toIso8601String())
+                                             ->first();
     }
 
     public function getMapLines() {
@@ -89,7 +95,14 @@ public function getMapLines() {
      * @return int
      */
     public function getDurationAttribute(): int {
-        return $this->arrival->diffInMinutes($this->departure);
+        try {
+            return $this->origin_stopover->departure_planned->diffInMinutes(
+                $this->destination_stopover->arrival_planned
+            );
+        } catch (Exception) {
+            //We need the try-catch to support old checkins, where no stopovers are saved.
+            return $this->arrival->diffInMinutes($this->departure);
+        }
     }
 
     public function getSpeedAttribute(): float {

diff --git a/database/factories/HafasTripFactory.php b/database/factories/HafasTripFactory.php
@@ -64,7 +64,7 @@ public function definition() {
                 ]
             ]);
             array_push($stopOvers, [
-                'stop'              => [
+                'stop'                     => [
                     'type'     => 'stop',
                     'id'       => $stop->ibnr,
                     'name'     => $stop->name,
@@ -87,12 +87,16 @@ public function definition() {
                         'taxi'            => $this->faker->boolean(50),
                     ],
                 ],
-                'arrival'           => $time->format('c'),
-                'arrivalDelay'      => null,
-                'arrivalPlatform'   => null,
-                'departure'         => $time->format('c'),
-                'departureDelay'    => null,
-                'departurePlatform' => null,
+                'arrival'                  => $time->toIso8601String(),
+                'plannedArrival'           => $time->toIso8601String(),
+                'arrivalDelay'             => null,
+                'arrivalPlatform'          => null,
+                'plannedArrivalPlatform'   => null,
+                'departure'                => $time->toIso8601String(),
+                'plannedDeparture'         => $time->toIso8601String(),
+                'departureDelay'           => null,
+                'departurePlatform'        => null,
+                'plannedDeparturePlatform' => null,
             ]);
             $time->addMinutes(30);
         }
@@ -130,14 +134,20 @@ public function configure() {
             $add       = $interval;
 
             for ($i = 1; $i < $cnt; $i++) {
-                $stopOvers[$i]->arrival   = $startTime->clone()->addSeconds($add)->format('c');
-                $stopOvers[$i]->departure = $stopOvers[$i]->arrival;
-                $add                      += $interval;
+                $stopOvers[$i]->plannedArrival   = $startTime->clone()->addSeconds($add)->format('c');
+                $stopOvers[$i]->arrival          = $startTime->clone()->addSeconds($add)->format('c');
+                $stopOvers[$i]->plannedDeparture = $stopOvers[$i]->arrival;
+                $stopOvers[$i]->departure        = $stopOvers[$i]->arrival;
+                $add                             += $interval;
             }
-            $stopOvers[0]->arrival          = $startTime->format('c');
-            $stopOvers[0]->departure        = $startTime->format('c');
-            $stopOvers[$cnt - 1]->arrival   = $endTime->format('c');
-            $stopOvers[$cnt - 1]->departure = $endTime->format('c');
+            $stopOvers[0]->arrival                 = $startTime->format('c');
+            $stopOvers[0]->plannedArrival          = $startTime->format('c');
+            $stopOvers[0]->departure               = $startTime->format('c');
+            $stopOvers[0]->plannedDeparture        = $startTime->format('c');
+            $stopOvers[$cnt - 1]->arrival          = $endTime->format('c');
+            $stopOvers[$cnt - 1]->plannedArrival   = $endTime->format('c');
+            $stopOvers[$cnt - 1]->departure        = $endTime->format('c');
+            $stopOvers[$cnt - 1]->plannedDeparture = $endTime->format('c');
 
             $hafasTrip->update(['stopovers' => json_encode($stopOvers)]);
         });

diff --git a/database/migrations/2019_08_14_013806_create_train_checkins_table.php b/database/migrations/2019_08_14_013806_create_train_checkins_table.php
@@ -11,7 +11,7 @@ class CreateTrainCheckinsTable extends Migration
      *
      * @return void
      */
-    public function up() {
+    public function up(): void {
         Schema::create('train_checkins', function(Blueprint $table) {
             $table->bigIncrements('id');
             $table->integer('status_id')
@@ -20,8 +20,11 @@ public function up() {
             $table->string('origin');
             $table->string('destination');
             $table->integer('distance');
-            $table->timestampTz('departure');
-            $table->timestampTz('arrival')->nullable();
+            $table->timestampTz('departure')
+                  ->comment('planned departure');
+            $table->timestampTz('arrival')
+                  ->nullable()
+                  ->comment('planned arrival');
             $table->integer('points')->nullable();
             $table->integer('delay')->nullable();
             $table->timestamps();
@@ -33,7 +36,7 @@ public function up() {
      *
      * @return void
      */
-    public function down() {
+    public function down(): void {
         Schema::dropIfExists('train_checkins');
     }
 }
diff --git a/resources/js/components/stationboard.js b/resources/js/components/stationboard.js
@@ -21,15 +21,18 @@ $(document)
         var lineName = $(this).data("linename");
         var tripID = $(this).data("tripid");
         var start = $(this).data("start");
+        let departure = $(this).data("departure");
         if (touchmoved != true) {
             window.location =
                 urlTrainTrip +
                 "?tripID=" +
-                tripID +
+                encodeURIComponent(tripID) +
                 "&lineName=" +
-                lineName +
+                encodeURIComponent(lineName) +
                 "&start=" +
-                start;
+                encodeURIComponent(start) +
+                "&departure=" +
+                encodeURIComponent(departure);
         }
     })
     .on("touchmove", function (e) {
@@ -45,12 +48,9 @@ $(document)
             .parent()
             .parent()
             .data("tripid");
-        var start = $(this)
-            .parent()
-            .parent()
-            .data("start");
         var destination = $(this).data("ibnr");
         var stopname = $(this).data("stopname");
+        var arrival = $(this).data("arrival");
         var linename = $(this)
             .parent()
             .parent()
@@ -67,7 +67,7 @@ $(document)
                     );
                 modal.find("#input-tripID").val(tripID);
                 modal.find("#input-destination").val(destination);
-                modal.find("#input-start").val(start);
+                modal.find("#input-arrival").val(arrival);
             });
         }
     })

diff --git a/resources/views/includes/status.blade.php b/resources/views/includes/status.blade.php
@@ -37,7 +37,7 @@
                             @endif {{ $status->trainCheckin->HafasTrip->linename }}
                         </span>
                         <span class="pl-2"><i class="fa fa-route d-inline"></i>&nbsp;{{number($status->trainCheckin->distance, 0)}}<small>km</small></span>
-                        <span class="pl-2"><i class="fa fa-stopwatch d-inline"></i>&nbsp;{!! durationToSpan(secondsToDuration(strtotime($status->trainCheckin->arrival) - strtotime($status->trainCheckin->departure))) !!}</span>
+                        <span class="pl-2"><i class="fa fa-stopwatch d-inline"></i>&nbsp;{!! durationToSpan(secondsToDuration($status->trainCheckin->duration * 60)) !!}</span>
 
                         @if($status->event != null)
                             <br class="d-sm-none">

diff --git a/resources/views/stationboard.blade.php b/resources/views/stationboard.blade.php
@@ -65,11 +65,11 @@ class="far fa-clock fa-sm"></i>{{ $when->format('H:i (Y-m-d)') }}</small>
                                 </tr>
                                 </thead>
                                 @foreach($departures as $departure)
-
                                     <tr @if(!isset($departure->cancelled)) class="trainrow"
                                         @endif data-tripID="{{ $departure->tripId }}"
                                         data-lineName="{{ $departure->line->name != null ? $departure->line->name : $departure->line->fahrtNr }}"
-                                        data-start="{{ $departure->stop->id }}">
+                                        data-start="{{ $departure->stop->id }}"
+                                        data-departure="{{ $departure->plannedWhen }}">
                                         <td>@if (file_exists(public_path('img/'.$departure->line->product.'.svg')))
                                                 <img class="product-icon"
                                                      src="{{ asset('img/'.$departure->line->product.'.svg') }}">