diff --git a/app/Models/User.php b/app/Models/User.php index 9949d582..d6e321df 100755 --- a/app/Models/User.php +++ b/app/Models/User.php @@ -3,7 +3,6 @@ namespace App\Models; use App\Models\Enums\JournalType; -use App\Models\Enums\PirepState; use App\Models\Traits\JournalTrait; use Illuminate\Foundation\Auth\User as Authenticatable; use Illuminate\Notifications\Notifiable; @@ -35,6 +34,7 @@ use Laratrust\Traits\LaratrustUserTrait; * @property int rank_id * @property int state * @property bool opt_in + * @property Pirep[] pireps * @property string last_pirep_id * @property Pirep last_pirep * @property UserFieldValue[] fields @@ -251,8 +251,7 @@ class User extends Authenticatable public function pireps() { - return $this->hasMany(Pirep::class, 'user_id') - ->where('state', '!=', PirepState::CANCELLED); + return $this->hasMany(Pirep::class, 'user_id'); } public function rank() diff --git a/app/Services/UserService.php b/app/Services/UserService.php index 9d26dd6e..5ab2fd97 100644 --- a/app/Services/UserService.php +++ b/app/Services/UserService.php @@ -139,13 +139,6 @@ class UserService extends Service */ public function removeUser(User $user) { - $user->name = 'Deleted User'; - $user->email = Utils::generateApiKey().'@deleted-user.com'; - $user->api_key = Utils::generateApiKey(); - $user->password = Hash::make(Utils::generateApiKey()); - $user->state = UserState::DELETED; - $user->save(); - // Detach all roles from this user $user->detachRoles($user->roles); @@ -154,6 +147,18 @@ class UserService extends Service // Remove any bids Bid::where('user_id', $user->id)->delete(); + + // If this user has PIREPs, do a soft delete. Otherwise, just delete them outright + if ($user->pireps->count() > 0) { + $user->name = 'Deleted User'; + $user->email = Utils::generateApiKey().'@deleted-user.com'; + $user->api_key = Utils::generateApiKey(); + $user->password = Hash::make(Utils::generateApiKey()); + $user->state = UserState::DELETED; + $user->save(); + } else { + $user->delete(); + } } /** @@ -299,49 +304,41 @@ class UserService extends Service * currently active users. If the user doesn't have a PIREP, then the creation date * of the user record is used to determine the difference */ - public function findUsersOnLeave(): array + public function findUsersOnLeave() { $leave_days = setting('pilots.auto_leave_days'); if ($leave_days === 0) { return []; } - $return_users = []; - $date = Carbon::now('UTC'); - $users = User::with(['last_pirep'])->where('state', UserState::ACTIVE)->get(); + $users = User::where('state', UserState::ACTIVE)->get(); /** @var User $user */ - foreach ($users as $user) { - // If they haven't submitted a PIREP, use the date that the user was created - if (!$user->last_pirep) { - $diff_date = $user->created_at; - } else { - $diff_date = $user->last_pirep->submitted_at; - } - - // See if the difference is larger than what the setting calls for - if ($date->diffInDays($diff_date) <= $leave_days) { - continue; - } - - $skip = false; + return $users->filter(function ($user, $i) use ($date, $leave_days) { // If any role for this user has the "disable_activity_check" feature activated, skip this user foreach ($user->roles()->get() as $role) { /** @var Role $role */ if ($role->disable_activity_checks) { - $skip = true; - break; + return false; } } - if ($skip) { - continue; + // If they haven't submitted a PIREP, use the date that the user was created + $last_pirep = Pirep::where(['user_id' => $user->id])->latest('submitted_at')->first(); + if (!$last_pirep) { + $diff_date = $user->created_at; + } else { + $diff_date = $last_pirep->created_at; } - $return_users[] = $user; - } - return $return_users; + // See if the difference is larger than what the setting calls for + if ($date->diffInDays($diff_date) <= $leave_days) { + return false; + } + + return true; + }); } /** diff --git a/tests/TestData.php b/tests/TestData.php index fbe24c68..4c71ba92 100644 --- a/tests/TestData.php +++ b/tests/TestData.php @@ -47,12 +47,15 @@ trait TestData { $subfleet = $this->createSubfleetWithAircraft(2); $rank = $this->createRank(10, [$subfleet['subfleet']->id]); - $this->user = factory(\App\Models\User::class)->create(array_merge([ + + /** @var User user */ + $this->user = factory(User::class)->create(array_merge([ 'rank_id' => $rank->id, ], $user_attrs)); // Return a Pirep model return factory(Pirep::class)->make(array_merge([ + 'user_id' => $this->user->id, 'aircraft_id' => $subfleet['aircraft']->random()->id, ], $pirep_attrs)); } diff --git a/tests/UserTest.php b/tests/UserTest.php index dd584e20..20e1d843 100644 --- a/tests/UserTest.php +++ b/tests/UserTest.php @@ -328,6 +328,33 @@ class UserTest extends TestCase $response = $this->get('/api/user/'.$user->id, [], $admin_user); $response->assertStatus(404); + // Get from the DB + $user = User::find($user->id); + $this->assertNull($user); + } + + public function testUserPilotDeletedWithPireps() + { + $new_user = factory(User::class)->make()->toArray(); + $new_user['password'] = Hash::make('secret'); + $admin_user = $this->userSvc->createUser($new_user); + + $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); + + /** @var Pirep $pirep */ + factory(Pirep::class)->create([ + 'user_id' => $user->id, + ]); + + // Delete the user + $this->userSvc->removeUser($user); + + $response = $this->get('/api/user/'.$user->id, [], $admin_user); + $response->assertStatus(404); + // Get from the DB $user = User::find($user->id); $this->assertEquals('Deleted User', $user->name); @@ -359,53 +386,64 @@ class UserTest extends TestCase $this->createUser(['status' => UserState::ACTIVE]); $users_on_leave = $this->userSvc->findUsersOnLeave(); - $this->assertEquals(0, count($users_on_leave)); + $this->assertCount(0, $users_on_leave); $this->updateSetting('pilots.auto_leave_days', 1); $user = $this->createUser([ + 'state' => UserState::ACTIVE, 'status' => UserState::ACTIVE, 'created_at' => Carbon::now('UTC')->subDays(5), ]); $users_on_leave = $this->userSvc->findUsersOnLeave(); - $this->assertEquals(1, count($users_on_leave)); - $this->assertEquals($user->id, $users_on_leave[0]->id); + $this->assertCount(1, $users_on_leave); + $this->assertEquals($user->id, $users_on_leave->first()->id); // Give that user a new PIREP, still old - /** @var \App\Models\Pirep $pirep */ - $pirep = factory(Pirep::class)->create(['submitted_at' => Carbon::now('UTC')->subDays(5)]); + /** @var Pirep $pirep */ + $pirep = factory(Pirep::class)->create([ + 'user_id' => $user->id, + 'created_at' => Carbon::now('UTC')->subDays(5), + 'submitted_at' => Carbon::now('UTC')->subDays(5), + ]); $user->last_pirep_id = $pirep->id; $user->save(); $user->refresh(); $users_on_leave = $this->userSvc->findUsersOnLeave(); - $this->assertEquals(1, count($users_on_leave)); - $this->assertEquals($user->id, $users_on_leave[0]->id); + $this->assertCount(1, $users_on_leave); + $this->assertEquals($user->id, $users_on_leave->first()->id); // Create a new PIREP - /** @var \App\Models\Pirep $pirep */ - $pirep = factory(Pirep::class)->create(['submitted_at' => Carbon::now('UTC')]); + /** @var Pirep $pirep */ + $pirep = factory(Pirep::class)->create([ + 'user_id' => $user->id, + 'created_at' => Carbon::now('UTC'), + 'submitted_at' => Carbon::now('UTC'), + ]); $user->last_pirep_id = $pirep->id; $user->save(); $user->refresh(); $users_on_leave = $this->userSvc->findUsersOnLeave(); - $this->assertEquals(0, count($users_on_leave)); + $this->assertCount(0, $users_on_leave); // Check disable_activity_checks $user = $this->createUser([ 'status' => UserState::ACTIVE, 'created_at' => Carbon::now('UTC')->subDays(5), ]); + $role = $this->createRole([ 'disable_activity_checks' => true, ]); + $user->attachRole($role); $user->save(); $users_on_leave = $this->userSvc->findUsersOnLeave(); - $this->assertEquals(0, count($users_on_leave)); + $this->assertCount(0, $users_on_leave); } }