From bbb5f1d13c531db3ee4a9d6eb0d7ead79154f211 Mon Sep 17 00:00:00 2001 From: Nabeel S Date: Tue, 14 Jul 2020 22:02:35 -0400 Subject: [PATCH] Check for null values when checking pilot ID #760 (#768) --- app/Services/UserService.php | 20 +++++++++++++++---- tests/UserTest.php | 37 ++++++++++++++++++++++++------------ 2 files changed, 41 insertions(+), 16 deletions(-) diff --git a/app/Services/UserService.php b/app/Services/UserService.php index 185d57cd..853fa8e4 100644 --- a/app/Services/UserService.php +++ b/app/Services/UserService.php @@ -8,6 +8,7 @@ use App\Events\UserStateChanged; use App\Events\UserStatsChanged; use App\Exceptions\PilotIdNotFound; use App\Exceptions\UserPilotIdExists; +use App\Models\Airline; use App\Models\Enums\PirepState; use App\Models\Enums\UserState; use App\Models\Pirep; @@ -188,22 +189,33 @@ class UserService extends Service * Split a given pilot ID into an airline and ID portions * * @param string $pilot_id + * + * @return User */ - public function findUserByPilotId(string $pilot_id) + public function findUserByPilotId(string $pilot_id): User { + $pilot_id = trim($pilot_id); + if (empty($pilot_id)) { + throw new PilotIdNotFound(''); + } + $airlines = $this->airlineRepo->all(['id', 'icao', 'iata']); $ident_str = null; $pilot_id = strtoupper($pilot_id); + + /** @var Airline $airline */ foreach ($airlines as $airline) { if (strpos($pilot_id, $airline->icao) !== false) { $ident_str = $airline->icao; break; } - if (strpos($pilot_id, $airline->iata) !== false) { - $ident_str = $airline->iata; - break; + if (!empty($airline->iata)) { + if (strpos($pilot_id, $airline->iata) !== false) { + $ident_str = $airline->iata; + break; + } } } diff --git a/tests/UserTest.php b/tests/UserTest.php index 84d624fc..d60e7b90 100644 --- a/tests/UserTest.php +++ b/tests/UserTest.php @@ -4,6 +4,7 @@ namespace Tests; use App\Exceptions\PilotIdNotFound; use App\Exceptions\UserPilotIdExists; +use App\Models\Airline; use App\Models\User; use App\Repositories\SettingRepository; use App\Services\UserService; @@ -37,7 +38,7 @@ class UserTest extends TestCase $rank = $this->createRank(10, [$subfleetA['subfleet']->id]); - $user = factory(\App\Models\User::class)->create([ + $user = factory(User::class)->create([ 'rank_id' => $rank->id, ]); @@ -97,7 +98,7 @@ class UserTest extends TestCase $rank = $this->createRank(10, [$subfleetA['subfleet']->id]); - $user = factory(\App\Models\User::class)->create([ + $user = factory(User::class)->create([ 'rank_id' => $rank->id, ]); @@ -143,7 +144,7 @@ class UserTest extends TestCase $subfleetB = $this->createSubfleetWithAircraft(2); $rank = $this->createRank(10, [$subfleetA['subfleet']->id]); - $user = factory(\App\Models\User::class)->create([ + $user = factory(User::class)->create([ 'curr_airport_id' => $airport->id, 'rank_id' => $rank->id, ]); @@ -203,8 +204,8 @@ class UserTest extends TestCase public function testUserPilotIdChangeAlreadyExists() { $this->expectException(UserPilotIdExists::class); - $user1 = factory(\App\Models\User::class)->create(['id' => 1]); - $user2 = factory(\App\Models\User::class)->create(['id' => 2]); + $user1 = factory(User::class)->create(['id' => 1]); + $user2 = factory(User::class)->create(['id' => 2]); // Now try to change the original user's pilot_id to 2 (should conflict) $this->userSvc->changePilotId($user1, 2); @@ -215,8 +216,8 @@ class UserTest extends TestCase */ public function testUserPilotIdSplit(): void { - /** @var \App\Models\User $user */ - $user = factory(\App\Models\User::class)->create(); + /** @var User $user */ + $user = factory(User::class)->create(); $found_user = $this->userSvc->findUserByPilotId($user->ident); $this->assertEquals($user->id, $found_user->id); @@ -230,25 +231,37 @@ class UserTest extends TestCase */ public function testUserPilotIdSplitInvalidId(): void { - /** @var \App\Models\User $user */ - $user = factory(\App\Models\User::class)->create(); + /** @var User $user */ + $user = factory(User::class)->create(); $this->expectException(PilotIdNotFound::class); $this->userSvc->findUserByPilotId($user->airline->iata); } + public function testUserPilotIdInvalidIATA(): void + { + /** @var Airline $airline */ + $airline = factory(Airline::class)->create(['icao' => 'ABC', 'iata' => null]); + + /** @var User $user */ + $user = factory(User::class)->create(['airline_id' => $airline->id]); + + $this->expectException(PilotIdNotFound::class); + $this->userSvc->findUserByPilotId('123'); + } + /** * Test the pilot ID being added when a new user is created */ public function testUserPilotIdAdded() { - $new_user = factory(\App\Models\User::class)->make()->toArray(); + $new_user = factory(User::class)->make()->toArray(); $new_user['password'] = Hash::make('secret'); $user = $this->userSvc->createUser($new_user); $this->assertEquals($user->id, $user->pilot_id); // Add a second user - $new_user = factory(\App\Models\User::class)->make()->toArray(); + $new_user = factory(User::class)->make()->toArray(); $new_user['password'] = Hash::make('secret'); $user2 = $this->userSvc->createUser($new_user); $this->assertEquals($user2->id, $user2->pilot_id); @@ -258,7 +271,7 @@ class UserTest extends TestCase $this->assertEquals(3, $user->pilot_id); // Create a new user and the pilot_id should be 4 - $user3 = factory(\App\Models\User::class)->create(); + $user3 = factory(User::class)->create(); $this->assertEquals(4, $user3->pilot_id); }