From f6b2102e4827da6177eb4eee0c3ce0d38eb78ce3 Mon Sep 17 00:00:00 2001 From: Nabeel Shahzad Date: Fri, 9 Feb 2018 14:26:14 -0600 Subject: [PATCH] fix bug where aircraft restrictions aren't respected in flight calls #170 --- CHANGELOG.md | 2 + app/Http/Controllers/Api/FlightController.php | 58 +++++++++++++++-- app/Http/Resources/Flight.php | 2 +- app/Repositories/SettingRepository.php | 8 +++ tests/FlightTest.php | 15 ++--- tests/UserTest.php | 65 +++++++++++++++++++ 6 files changed, 135 insertions(+), 15 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 30fa5805..cc649223 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -15,6 +15,8 @@ - PIREP fields being set when filing manually is working - Field for the rank's image changed to string input +- API: Fixed typo from `subfleet` to `subfleets` in the `/api/flights` call(s) +- API: Subfleets returned in the flight calls respect the `pireps.restrict_aircraft_to_rank` setting *** diff --git a/app/Http/Controllers/Api/FlightController.php b/app/Http/Controllers/Api/FlightController.php index 0d7a5d62..5e9865d6 100644 --- a/app/Http/Controllers/Api/FlightController.php +++ b/app/Http/Controllers/Api/FlightController.php @@ -2,20 +2,52 @@ namespace App\Http\Controllers\Api; +use Auth; use Illuminate\Http\Request; use Prettus\Repository\Criteria\RequestCriteria; - -use App\Repositories\FlightRepository; -use App\Http\Resources\Flight as FlightResource; use Prettus\Repository\Exceptions\RepositoryException; +use App\Services\UserService; +use App\Repositories\FlightRepository; +use App\Http\Resources\Flight as FlightResource; +/** + * Class FlightController + * @package App\Http\Controllers\Api + */ class FlightController extends RestController { - protected $flightRepo; + protected $flightRepo, $userSvc; - public function __construct(FlightRepository $flightRepo) { + public function __construct( + FlightRepository $flightRepo, + UserService $userSvc + ) { $this->flightRepo = $flightRepo; + $this->userSvc = $userSvc; + } + + /** + * Filter out subfleets to only include aircraft that a user has access to + * @param $user + * @param $flight + * @return mixed + */ + public function filterSubfleets($user, $flight) + { + if(setting('pireps.restrict_aircraft_to_rank', false) === false) { + return $flight; + } + + $allowed_subfleets = $this->userSvc->getAllowableSubfleets($user)->pluck('id'); + $flight->subfleets = $flight->subfleets->filter( + function($subfleet, $item) use ($allowed_subfleets) { + if ($allowed_subfleets->contains($subfleet->id)) { + return true; + } + }); + + return $flight; } /** @@ -27,12 +59,23 @@ class FlightController extends RestController ->orderBy('flight_number', 'asc') ->paginate(50); + $user = Auth::user(); + foreach($flights as $flight) { + $this->filterSubfleets($user, $flight); + } + return FlightResource::collection($flights); } + /** + * @param $id + * @return FlightResource + */ public function get($id) { $flight = $this->flightRepo->find($id); + $this->filterSubfleets(Auth::user(), $flight); + FlightResource::withoutWrapping(); return new FlightResource($flight); } @@ -51,6 +94,11 @@ class FlightController extends RestController return response($e, 503); } + $user = Auth::user(); + foreach ($flights as $flight) { + $this->filterSubfleets($user, $flight); + } + return FlightResource::collection($flights); } } diff --git a/app/Http/Resources/Flight.php b/app/Http/Resources/Flight.php index c9ef20ab..aee5d0a6 100644 --- a/app/Http/Resources/Flight.php +++ b/app/Http/Resources/Flight.php @@ -11,7 +11,7 @@ class Flight extends Resource $flight = parent::toArray($request); $flight['airline'] = new Airline($this->airline); - $flight['subfleet'] = Subfleet::collection($this->subfleets); + $flight['subfleets'] = Subfleet::collection($this->subfleets); return $flight; } diff --git a/app/Repositories/SettingRepository.php b/app/Repositories/SettingRepository.php index 5b1182e1..73e3d64d 100644 --- a/app/Repositories/SettingRepository.php +++ b/app/Repositories/SettingRepository.php @@ -67,6 +67,14 @@ class SettingRepository extends BaseRepository implements CacheableInterface } } + /** + * @alias store($key,$value) + */ + public function save($key, $value) + { + return $this->store($key, $value); + } + /** * Update an existing setting with a new value. Doesn't create * a new setting diff --git a/tests/FlightTest.php b/tests/FlightTest.php index 52f310a1..061536f5 100644 --- a/tests/FlightTest.php +++ b/tests/FlightTest.php @@ -1,18 +1,22 @@ addData('base'); $this->flightSvc = app(FlightService::class); + $this->settingsRepo = app(SettingRepository::class); } public function addFlight($user) @@ -168,7 +172,7 @@ class FlightTest extends TestCase */ public function testMultipleBidsSingleFlight() { - setting('bids.disable_flight_on_bid', true); + $this->settingsRepo->store('bids.disable_flight_on_bid', true); $user1 = factory(User::class)->create(); $user2 = factory(User::class)->create([ @@ -224,11 +228,4 @@ class FlightTest extends TestCase $body = $req->json(); $this->assertEquals(0, sizeof($body)); } - - public function testRestrictedFlights() - { - setting('bids.disable_flight_on_bid', true); - - - } } diff --git a/tests/UserTest.php b/tests/UserTest.php index 908a0ea6..25d89aa3 100644 --- a/tests/UserTest.php +++ b/tests/UserTest.php @@ -1,5 +1,6 @@ assertEquals($added_aircraft, $aircraft_from_api); } + + /** + * Flip the setting for getting all of the user's aircraft restricted + * by rank. Make sure that they're all returned. Create two subfleets, + * assign only one of them to the user's rank. When calling the api + * to retrieve the flight, only subfleetA should be showing + */ + public function testGetAircraftAllowedFromFlight() + { + # Add subfleets and aircraft, but also add another + # set of subfleets + $subfleetA = TestData::createSubfleetWithAircraft(2); + $subfleetB = TestData::createSubfleetWithAircraft(2); + + $rank = TestData::createRank(10, [$subfleetA['subfleet']->id]); + $user = factory(App\Models\User::class)->create(['rank_id' => $rank->id,]); + + $flight = factory(App\Models\Flight::class)->create(['airline_id' => $user->airline_id]); + $flight->subfleets()->syncWithoutDetaching([ + $subfleetA['subfleet']->id, + $subfleetB['subfleet']->id + ]); + + /* + * Now we can do some actual tests + */ + + /* + * Do some sanity checks first + */ + $this->settingsRepo->store('pireps.restrict_aircraft_to_rank', false); + + $response = $this->get('/api/flights/' . $flight->id, [], $user); + $response->assertStatus(200); + $this->assertCount(2, $response->json()['subfleets']); + + /* + * Now make sure it's filtered out + */ + $this->settingsRepo->store('pireps.restrict_aircraft_to_rank', true); + + /** + * Make sure it's filtered out from the single flight call + */ + $response = $this->get('/api/flights/' . $flight->id, [], $user); + $response->assertStatus(200); + $this->assertCount(1, $response->json()['subfleets']); + + /** + * Make sure it's filtered out from the flight list + */ + $response = $this->get('/api/flights', [], $user); + $response->assertStatus(200); + $body = $response->json(); + $this->assertCount(1, $body['data'][0]['subfleets']); + + /** + * Filtered from search? + */ + $response = $this->get('/api/flights/search?flight_id=' . $flight->id, [], $user); + $response->assertStatus(200); + $body = $response->json(); + $this->assertCount(1, $body['data'][0]['subfleets']); + } }