From 51186bed7c7a5cb50c17fd7e0916f165f7daf205 Mon Sep 17 00:00:00 2001 From: Nabeel S Date: Mon, 17 Aug 2020 16:44:31 -0400 Subject: [PATCH] Fix for aircraft not being detected at departure airport #781 (#789) Aircraft not being checked/detected at departure airport #781 --- app/Exceptions/AircraftInvalid.php | 47 +++++++++++++++++++ .../Controllers/Frontend/PirepController.php | 21 +++++++-- app/Services/PirepService.php | 22 +++++++-- tests/AcarsTest.php | 44 +++++++++++++++++ 4 files changed, 124 insertions(+), 10 deletions(-) create mode 100644 app/Exceptions/AircraftInvalid.php diff --git a/app/Exceptions/AircraftInvalid.php b/app/Exceptions/AircraftInvalid.php new file mode 100644 index 00000000..224e1ab7 --- /dev/null +++ b/app/Exceptions/AircraftInvalid.php @@ -0,0 +1,47 @@ +aircraft = $aircraft; + parent::__construct( + 400, + static::MESSAGE + ); + } + + /** + * Return the RFC 7807 error type (without the URL root) + */ + public function getErrorType(): string + { + return 'aircraft-invalid'; + } + + /** + * Get the detailed error string + */ + public function getErrorDetails(): string + { + return $this->getMessage(); + } + + /** + * Return an array with the error details, merged with the RFC7807 response + */ + public function getErrorMetadata(): array + { + return [ + 'aircraft_id' => optional($this->aircraft)->id, + ]; + } +} diff --git a/app/Http/Controllers/Frontend/PirepController.php b/app/Http/Controllers/Frontend/PirepController.php index f364815d..d7cabd98 100644 --- a/app/Http/Controllers/Frontend/PirepController.php +++ b/app/Http/Controllers/Frontend/PirepController.php @@ -285,7 +285,7 @@ class PirepController extends Controller $user = Auth::user(); $pirep = new Pirep($request->post()); - $pirep->user_id = Auth::user()->id; + $pirep->user_id = $user->id; $attrs = $request->all(); $attrs['submit'] = strtolower($attrs['submit']); @@ -295,7 +295,7 @@ class PirepController extends Controller if (setting('pilots.only_flights_from_current') && $user->curr_airport_id !== $pirep->dpt_airport_id) { Log::info( - 'Pilot '.Auth::user()->id + 'Pilot '.$user->id .' not at departure airport (curr='.$user->curr_airport_id .', dpt='.$pirep->dpt_airport_id.')' ); @@ -309,7 +309,7 @@ class PirepController extends Controller // Can they fly this aircraft? if (setting('pireps.restrict_aircraft_to_rank', false) && !$this->userSvc->aircraftAllowed($user, $pirep->aircraft_id)) { - Log::info('Pilot '.Auth::user()->id.' not allowed to fly aircraft'); + Log::info('Pilot '.$user->id.' not allowed to fly aircraft'); return $this->flashError( 'You are not allowed to fly this aircraft!', 'frontend.pireps.create' @@ -318,9 +318,20 @@ class PirepController extends Controller // is the aircraft in the right place? /* @noinspection NotOptimalIfConditionsInspection */ + // Get the aircraft + $aircraft = $this->aircraftRepo->findWithoutFail($pirep->aircraft_id); + if ($aircraft === null) { + Log::error('Aircraft for PIREP not found, id='.$pirep->aircraft_id); + return $this->flashError( + 'The aircraft for the PIREP hasn\'t been found', + 'frontend.pireps.create' + ); + } + if (setting('pireps.only_aircraft_at_dpt_airport') - && $pirep->aircraft_id !== $pirep->dpt_airport_id) { - Log::info('Aircraft '.$pirep->aircraft_id.' not at departure airport '.$pirep->dpt_airport_id.', erroring out'); + && $aircraft->airport_id !== $pirep->dpt_airport_id + ) { + Log::info('Aircraft '.$pirep->aircraft_id.' not at departure airport (curr='.$pirep->aircraft->airport_id.', apt='.$pirep->dpt_airport_id.')'); return $this->flashError( 'This aircraft is not positioned at the departure airport!', 'frontend.pireps.create' diff --git a/app/Services/PirepService.php b/app/Services/PirepService.php index 461c034d..95d004b3 100644 --- a/app/Services/PirepService.php +++ b/app/Services/PirepService.php @@ -8,11 +8,13 @@ use App\Events\PirepCancelled; use App\Events\PirepFiled; use App\Events\PirepRejected; use App\Events\UserStatsChanged; +use App\Exceptions\AircraftInvalid; use App\Exceptions\AircraftNotAtAirport; use App\Exceptions\AircraftPermissionDenied; use App\Exceptions\PirepCancelNotAllowed; use App\Exceptions\UserNotAtAirport; use App\Models\Acars; +use App\Models\Aircraft; use App\Models\Enums\AcarsType; use App\Models\Enums\FlightType; use App\Models\Enums\PirepSource; @@ -22,6 +24,7 @@ use App\Models\Navdata; use App\Models\Pirep; use App\Models\PirepFieldValue; use App\Models\User; +use App\Repositories\AircraftRepository; use App\Repositories\PirepRepository; use Carbon\Carbon; use function count; @@ -30,20 +33,24 @@ use Illuminate\Support\Facades\Log; class PirepService extends Service { + private $aircraftRepo; private $geoSvc; private $userSvc; private $pirepRepo; /** - * @param GeoService $geoSvc - * @param PirepRepository $pirepRepo - * @param UserService $userSvc + * @param AircraftRepository $aircraftRepo + * @param GeoService $geoSvc + * @param PirepRepository $pirepRepo + * @param UserService $userSvc */ public function __construct( + AircraftRepository $aircraftRepo, GeoService $geoSvc, PirepRepository $pirepRepo, UserService $userSvc ) { + $this->aircraftRepo = $aircraftRepo; $this->geoSvc = $geoSvc; $this->userSvc = $userSvc; $this->pirepRepo = $pirepRepo; @@ -89,9 +96,14 @@ class PirepService extends Service } // See if this aircraft is at the departure airport + /** @var Aircraft $aircraft */ + $aircraft = $this->aircraftRepo->findWithoutFail($pirep->aircraft_id); + if ($aircraft === null) { + throw new AircraftInvalid($aircraft); + } + /* @noinspection NotOptimalIfConditionsInspection */ - if (setting('pireps.only_aircraft_at_dpt_airport') - && $pirep->aircraft_id !== $pirep->dpt_airport_id) { + if (setting('pireps.only_aircraft_at_dpt_airport') && $aircraft->airport_id !== $pirep->dpt_airport_id) { throw new AircraftNotAtAirport($pirep->aircraft); } diff --git a/tests/AcarsTest.php b/tests/AcarsTest.php index bf18058e..bf1fd6f5 100644 --- a/tests/AcarsTest.php +++ b/tests/AcarsTest.php @@ -107,6 +107,50 @@ class AcarsTest extends TestCase $response->assertStatus(400); } + public function testPrefileAircraftNotAtAirport() + { + $this->settingsRepo->store('pilots.only_flights_from_current', false); + $this->settingsRepo->store('pireps.restrict_aircraft_to_rank', false); + $this->settingsRepo->store('pireps.only_aircraft_at_dpt_airport', true); + + $this->user = factory(User::class)->create(); + + /** @var Airport $airport */ + $airport = factory(Airport::class)->create(); + + /** @var Airport $airport */ + $aircraft_airport = factory(Airport::class)->create(); + + /** @var Airline $airline */ + $airline = factory(Airline::class)->create(); + + /** @var Aircraft $aircraft */ + $aircraft = factory(Aircraft::class)->create(['airport_id' => $aircraft_airport->id]); + + /** + * INVALID AIRLINE_ID FIELD + */ + $uri = '/api/pireps/prefile'; + $pirep = [ + 'airline_id' => $airline->id, + 'aircraft_id' => $aircraft->id, + 'dpt_airport_id' => $airport->icao, + 'arr_airport_id' => $airport->icao, + 'flight_number' => '6000', + 'level' => 38000, + 'planned_flight_time' => 120, + 'route' => 'POINTA POINTB', + 'source_name' => 'Tests', + ]; + + $response = $this->post($uri, $pirep); + $response->assertStatus(400); + $this->assertEquals( + 'The aircraft is not at the departure airport', + $response->json('title') + ); + } + public function testBlankAirport() { $this->user = factory(User::class)->create();