FlightRouteAwards - fix if last pirep is empty/other error conditions (#886)

app/Contracts/Award.php

@@ -36,7 +36,10 @@ abstract class Award
      * You don't really need to mess with anything below here
      */
 
+    /** @var \App\Models\Award|null */
     protected $award;
+
+    /** @var \App\Models\User|null */
     protected $user;
 
     public function __construct(AwardModel $award = null, User $user = null)

app/Models/User.php

@@ -36,6 +36,7 @@ use Laratrust\Traits\LaratrustUserTrait;
  * @property int              state
  * @property bool             opt_in
  * @property string           last_pirep_id
+ * @property Pirep            last_pirep
  * @property UserFieldValue[] fields
  *
  * @mixin \Illuminate\Database\Eloquent\Builder

modules/Awards/Awards/FlightRouteAwards.php

@@ -3,6 +3,8 @@
 namespace Modules\Awards\Awards;
 
 use App\Contracts\Award;
+use ErrorException;
+use Illuminate\Support\Facades\Log;
 
 /**
  * All award classes need to extend Award and implement the check() method
@@ -44,17 +46,26 @@ class FlightRouteAwards extends Award
      */
     public function check($dptarr = null): bool
     {
-        if (!$dptarr) {
-            $dptarr = 'XXXX:YYYY';
+        if ($this->user->last_pirep_id === null) {
+            return false;
+        }
+
+        $dptarr = strtoupper(trim($dptarr));
+        if (empty($dptarr)) {
+            Log::error('FlightRouteAwards: empty departure/arrival string');
+            return false;
+        }
+
+        try {
+            [$dpt_icao, $arr_icao] = explode(':', $dptarr);
+        } catch (ErrorException $e) {
+            Log::error('FlightRouteAwards: Invalid departure/arrival, val="'.$dptarr.'\"');
+            return false;
         }
 
-        $pieces = explode(':', $dptarr);
         $dpt = $this->user->last_pirep->dpt_airport_id;
         $arr = $this->user->last_pirep->arr_airport_id;
 
-        if (strcasecmp($dpt, $pieces[0]) == 0 && strcasecmp($arr, $pieces[1]) == 0) {
-            return true;
-        }
-        return false;
+        return $dpt === $dpt_icao && $arr === $arr_icao;
     }
 }

tests/AwardsTest.php

@@ -8,6 +8,7 @@ use App\Models\User;
 use App\Models\UserAward;
 use App\Services\AwardService;
 use App\Services\PirepService;
+use Modules\Awards\Awards\FlightRouteAwards;
 use Modules\Awards\Awards\PilotFlightAwards;
 
 class AwardsTest extends TestCase
@@ -69,4 +70,63 @@ class AwardsTest extends TestCase
         $found_award = UserAward::where($w)->first();
         $this->assertNotNull($found_award);
     }
+
+    /**
+     * Test the flight route
+     */
+    public function testFlightRouteAward()
+    {
+        /** @var \App\Models\User $user */
+        $user = factory(User::class)->create([
+            'flights' => 0,
+        ]);
+
+        /** @var \App\Models\Award $award */
+        $award = factory(Award::class)->create([
+            'ref_model'        => FlightRouteAwards::class,
+            'ref_model_params' => 1,
+        ]);
+
+        /** @var Pirep $pirep */
+        $pirep = factory(Pirep::class)->create([
+            'airline_id' => $user->airline->id,
+            'user_id'    => $user->id,
+        ]);
+
+        $flightAward = new FlightRouteAwards($award, $user);
+
+        // Test no last PIREP for the user
+        $this->assertFalse($flightAward->check(''));
+
+        // Reinit award, add a last user PIREP id
+        $user->last_pirep_id = $pirep->id;
+        $user->save();
+
+        $flightAward = new FlightRouteAwards($award, $user);
+        $validStrs = [
+            $pirep->dpt_airport_id.':'.$pirep->arr_airport_id,
+            $pirep->dpt_airport_id.':'.$pirep->arr_airport_id.' ',
+            $pirep->dpt_airport_id.':'.$pirep->arr_airport_id.':',
+            strtolower($pirep->dpt_airport_id).':'.strtolower($pirep->arr_airport_id),
+        ];
+
+        foreach ($validStrs as $str) {
+            $this->assertTrue($flightAward->check($str));
+        }
+
+        // Check error conditions
+        $errStrs = [
+            '',
+            ' ',
+            ':',
+            'ABCD:EDFSDF',
+            $pirep->dpt_airport_id.':',
+            ':'.$pirep->arr_airport_id,
+            ':'.$pirep->arr_airport_id.':',
+        ];
+
+        foreach ($errStrs as $err) {
+            $this->assertFalse($flightAward->check($err));
+        }
+    }
 }