Simbrief attachment not working #1005 (#1006)

* Refactoring for Simbrief not working #1005

* Style fixes

* Update tests for new briefing URL

* Check the OFP user

* Comment out user check temporarily

app/Http/Controllers/Api/FlightController.php

@@ -4,9 +4,11 @@ namespace App\Http\Controllers\Api;
 
 use App\Contracts\Controller;
 use App\Exceptions\AssetNotFound;
+use App\Exceptions\Unauthorized;
 use App\Http\Resources\Flight as FlightResource;
 use App\Http\Resources\Navdata as NavdataResource;
 use App\Models\SimBrief;
+use App\Models\User;
 use App\Repositories\Criteria\WhereCriteria;
 use App\Repositories\FlightRepository;
 use App\Services\FareService;
@@ -152,20 +154,25 @@ class FlightController extends Controller
      *
      * @return \Illuminate\Contracts\Routing\ResponseFactory|\Illuminate\Http\Response
      */
-    public function briefing($id)
+    public function briefing(string $id)
     {
+        /** @var User $user */
         $user = Auth::user();
         $w = [
-            'user_id'   => $user->id,
+            'id' => $id,
-            'flight_id' => $id,
         ];
 
+        /** @var SimBrief $simbrief */
         $simbrief = SimBrief::where($w)->first();
 
         if ($simbrief === null) {
             throw new AssetNotFound(new Exception('Flight briefing not found'));
         }
 
+        /*if ($simbrief->user_id !== $user->id) {
+            throw new Unauthorized(new Exception('User cannot access another user\'s simbrief'));
+        }*/
+
         return response($simbrief->acars_xml, 200, [
             'Content-Type' => 'application/xml',
         ]);

app/Http/Controllers/Api/PirepController.php

@@ -22,8 +22,6 @@ use App\Models\Acars;
 use App\Models\Enums\AcarsType;
 use App\Models\Enums\PirepFieldSource;
 use App\Models\Enums\PirepSource;
-use App\Models\Enums\PirepState;
-use App\Models\Enums\PirepStatus;
 use App\Models\Pirep;
 use App\Models\PirepComment;
 use App\Repositories\AcarsRepository;
@@ -38,9 +36,6 @@ use Illuminate\Http\Request;
 use Illuminate\Support\Facades\Auth;
 use Illuminate\Support\Facades\Log;
 
-/**
- * Class PirepController
- */
 class PirepController extends Controller
 {
     private $acarsRepo;
@@ -93,6 +88,10 @@ class PirepController extends Controller
             $attrs['created_at'] = Carbon::createFromTimeString($attrs['created_at']);
         }
 
+        if (array_key_exists('submitted_at', $attrs)) {
+            $attrs['submitted_at'] = Carbon::createFromTimeString($attrs['submitted_at']);
+        }
+
         if (array_key_exists('updated_at', $attrs)) {
             $attrs['updated_at'] = Carbon::createFromTimeString($attrs['updated_at']);
         }
@@ -306,14 +305,8 @@ class PirepController extends Controller
             }
         }
 
-        $attrs['state'] = PirepState::PENDING;
-        $attrs['status'] = PirepStatus::ARRIVED;
-        $attrs['submitted_at'] = Carbon::now('UTC');
-
-        $pirep = $this->pirepRepo->update($attrs, $pirep_id);
-
         try {
-            $pirep = $this->pirepSvc->create($pirep);
+            $pirep = $this->pirepSvc->file($pirep, $attrs);
             $this->updateFields($pirep, $request);
             $this->updateFares($pirep, $request);
         } catch (\Exception $e) {

app/Http/Controllers/Frontend/SimBriefController.php

@@ -232,7 +232,7 @@ class SimBriefController
         $ofp_id = $request->input('ofp_id');
         $flight_id = $request->input('flight_id');
 
-        $simbrief = $this->simBriefSvc->checkForOfp(Auth::user()->id, $ofp_id, $flight_id);
+        $simbrief = $this->simBriefSvc->downloadOfp(Auth::user()->id, $ofp_id, $flight_id);
         if ($simbrief === null) {
             $error = new AssetNotFound(new Exception('Simbrief OFP not found'));
             return $error->getResponse();

app/Http/Resources/SimBrief.php

@@ -13,7 +13,7 @@ class SimBrief extends Resource
     {
         return [
             'id'  => $this->id,
-            'url' => url(route('api.flights.briefing', ['id' => $this->flight_id])),
+            'url' => url(route('api.flights.briefing', ['id' => $this->id])),
         ];
     }
 }

app/Services/PirepService.php

@@ -41,7 +41,7 @@ class PirepService extends Service
     private $airportSvc;
     private $geoSvc;
     private $pirepRepo;
-    private $simbriefSvc;
+    private $simBriefSvc;
     private $userSvc;
 
     /**
@@ -50,7 +50,7 @@ class PirepService extends Service
      * @param AirportRepository  $airportRepo
      * @param AirportService     $airportSvc
      * @param PirepRepository    $pirepRepo
-     * @param SimBriefService    $simbriefSvc
+     * @param SimBriefService    $simBriefSvc
      * @param UserService        $userSvc
      */
     public function __construct(
@@ -59,7 +59,7 @@ class PirepService extends Service
         AircraftRepository $aircraftRepo,
         GeoService $geoSvc,
         PirepRepository $pirepRepo,
-        SimBriefService $simbriefSvc,
+        SimBriefService $simBriefSvc,
         UserService $userSvc
     ) {
         $this->airportRepo = $airportRepo;
@@ -67,7 +67,7 @@ class PirepService extends Service
         $this->aircraftRepo = $aircraftRepo;
         $this->geoSvc = $geoSvc;
         $this->pirepRepo = $pirepRepo;
-        $this->simbriefSvc = $simbriefSvc;
+        $this->simBriefSvc = $simBriefSvc;
         $this->userSvc = $userSvc;
     }
 
@@ -151,14 +151,6 @@ class PirepService extends Service
 
         $pirep->save();
 
-        // Check of there is a simbrief_id
-        if (array_key_exists('simbrief_id', $attrs)) {
-            $simbrief = SimBrief::find($attrs['simbrief_id']);
-            if ($simbrief) {
-                $this->simbriefSvc->attachSimbriefToPirep($pirep, $simbrief);
-            }
-        }
-
         return $pirep;
     }
 
@@ -216,6 +208,72 @@ class PirepService extends Service
         return $pirep;
     }
 
+    /**
+     * Finalize a PIREP (meaning it's been filed)
+     *
+     * @param Pirep                   $pirep
+     * @param array                   $attrs
+     * @param array PirepFieldValue[] $field_values
+     *
+     * @throws \Exception
+     *
+     * @return Pirep
+     */
+    public function file(Pirep $pirep, array $attrs = [], array $field_values = []): Pirep
+    {
+        if (empty($field_values)) {
+            $field_values = [];
+        }
+
+        $attrs['state'] = PirepState::PENDING;
+        $attrs['status'] = PirepStatus::ARRIVED;
+        $attrs['submitted_at'] = Carbon::now('UTC');
+
+        $this->pirepRepo->update($attrs, $pirep->id);
+        $pirep->refresh();
+
+        // Check if there is a simbrief_id, change it to be set to the PIREP
+        // at the end of the flight when it's been filed
+        if (array_key_exists('simbrief_id', $attrs)) {
+            /** @var SimBrief $simbrief */
+            $simbrief = SimBrief::find($attrs['simbrief_id']);
+            if ($simbrief) {
+                $this->simBriefSvc->attachSimbriefToPirep($pirep, $simbrief);
+            }
+        }
+
+        // Check the block times. If a block on (arrival) time isn't
+        // specified, then use the time that it was submitted. It won't
+        // be the most accurate, but that might be OK
+        if (!$pirep->block_on_time) {
+            if ($pirep->submitted_at) {
+                $pirep->block_on_time = $pirep->submitted_at;
+            } else {
+                $pirep->block_on_time = Carbon::now('UTC');
+            }
+        }
+
+        // Check that there's a submit time
+        if (!$pirep->submitted_at) {
+            $pirep->submitted_at = Carbon::now('UTC');
+        }
+
+        // Copy some fields over from Flight if we have it
+        if ($pirep->flight) {
+            $pirep->planned_distance = $pirep->flight->distance;
+            $pirep->planned_flight_time = $pirep->flight->flight_time;
+        }
+
+        $pirep->save();
+        $pirep->refresh();
+
+        if (count($field_values) > 0) {
+            $this->updateCustomFields($pirep->id, $field_values);
+        }
+
+        return $pirep;
+    }
+
     /**
      * Find if there are duplicates to a given PIREP. Ideally, the passed
      * in PIREP hasn't been saved or gone through the create() method
@@ -316,18 +374,6 @@ class PirepService extends Service
         return $pirep;
     }
 
-    /**
-     * Alias to submit()
-     *
-     * @param \App\Models\Pirep $pirep
-     *
-     * @throws \Exception
-     */
-    public function file(Pirep $pirep)
-    {
-        return $this->submit($pirep);
-    }
-
     /**
      * Submit the PIREP. Figure out its default state
      *

app/Services/SimBriefService.php

@@ -32,7 +32,7 @@ class SimBriefService extends Service
      *
      * @return SimBrief|null
      */
-    public function checkForOfp(string $user_id, string $ofp_id, string $flight_id)
+    public function downloadOfp(string $user_id, string $ofp_id, string $flight_id)
     {
         $uri = str_replace('{id}', $ofp_id, config('phpvms.simbrief_url'));
 

tests/AcarsTest.php

@@ -230,12 +230,18 @@ class AcarsTest extends TestCase
         $this->settingsRepo->store('pireps.restrict_aircraft_to_rank', false);
         $this->settingsRepo->store('pireps.restrict_aircraft_to_rank', false);
 
+        /** @var User user */
         $this->user = factory(User::class)->create([
             'curr_airport_id' => 'KJFK',
         ]);
 
+        /** @var Airport $airport */
         $airport = factory(Airport::class)->create();
+
+        /** @var Airline $airline */
         $airline = factory(Airline::class)->create();
+
+        /** @var Aircraft $aircraft */
         $aircraft = factory(Aircraft::class)->create([
             'airport_id' => 'KAUS',
         ]);

tests/PIREPTest.php

@@ -423,14 +423,16 @@ class PIREPTest extends TestCase
 
         // Submit two PIREPs
         // 1 hour flight times, but the rank should bump up because of the transfer hours
+        /** @var Pirep $pirep */
         $pirep = factory(Pirep::class)->create([
             'airline_id' => $user->airline_id,
             'user_id'    => $user->id,
         ]);
 
         $this->pirepSvc->create($pirep);
-        $this->pirepSvc->file($pirep);
+        $this->pirepSvc->submit($pirep);
 
+        /** @var User $user */
         $user = User::find($user->id);
         $this->assertEquals(UserState::ACTIVE, $user->state);
     }

tests/SimBriefTest.php

@@ -23,8 +23,9 @@ class SimBriefTest extends TestCase
      *
      * @return \App\Models\SimBrief
      */
-    protected function loadSimBrief($user): SimBrief
+    protected function loadSimBrief(User $user): SimBrief
     {
+        /** @var \App\Models\Flight $flight */
         $flight = factory(Flight::class)->create([
             'id'             => self::$simbrief_flight_id,
             'dpt_airport_id' => 'OMAA',
@@ -39,7 +40,7 @@ class SimBriefTest extends TestCase
         /** @var SimBriefService $sb */
         $sb = app(SimBriefService::class);
 
-        return $sb->checkForOfp($user->id, Utils::generateNewId(), $flight->id);
+        return $sb->downloadOfp($user->id, Utils::generateNewId(), $flight->id);
     }
 
     /**
@@ -99,14 +100,13 @@ class SimBriefTest extends TestCase
 
         $url = str_replace('http://', 'https://', $flight['simbrief']['url']);
         $this->assertEquals(
-            'https://localhost/api/flights/'.$briefing->flight_id.'/briefing',
+            'https://localhost/api/flights/'.$briefing->id.'/briefing',
             $url
         );
 
         // Retrieve the briefing via API, and then check the doctype
-        $response = $this->get('/api/flights/'.$briefing->flight_id.'/briefing');
+        $response = $this->get('/api/flights/'.$briefing->id.'/briefing', [], $this->user);
         $response->assertOk();
-        // $response->assertHeader('Content-Type', 'application/xml');
 
         $xml = simplexml_load_string($response->content());
         $this->assertNotNull($xml);