diff --git a/app/Exceptions/UserNotFound.php b/app/Exceptions/UserNotFound.php new file mode 100644 index 00000000..41aabd5c --- /dev/null +++ b/app/Exceptions/UserNotFound.php @@ -0,0 +1,38 @@ +getMessage(); + } + + /** + * Return an array with the error details, merged with the RFC7807 response + */ + public function getErrorMetadata(): array + { + return []; + } +} diff --git a/app/Http/Controllers/Admin/UserController.php b/app/Http/Controllers/Admin/UserController.php index 4be76a7c..317bd784 100644 --- a/app/Http/Controllers/Admin/UserController.php +++ b/app/Http/Controllers/Admin/UserController.php @@ -252,15 +252,12 @@ class UserController extends Controller public function destroy($id) { $user = $this->userRepo->findWithoutFail($id); - if (empty($user)) { Flash::error('User not found'); - return redirect(route('admin.users.index')); } - $this->userRepo->delete($id); - + $this->userSvc->removeUser($user); Flash::success('User deleted successfully.'); return redirect(route('admin.users.index')); diff --git a/app/Http/Controllers/Api/UserController.php b/app/Http/Controllers/Api/UserController.php index 4fe0963a..c0812b7c 100644 --- a/app/Http/Controllers/Api/UserController.php +++ b/app/Http/Controllers/Api/UserController.php @@ -3,6 +3,7 @@ namespace App\Http\Controllers\Api; use App\Contracts\Controller; +use App\Exceptions\UserNotFound; use App\Http\Resources\Bid as BidResource; use App\Http\Resources\Pirep as PirepResource; use App\Http\Resources\Subfleet as SubfleetResource; @@ -91,6 +92,10 @@ class UserController extends Controller public function get($id) { $user = $this->userSvc->getUser($id); + if ($user === null) { + throw new UserNotFound(); + } + return new UserResource($user); } @@ -108,6 +113,9 @@ class UserController extends Controller { $user_id = $this->getUserId($request); $user = $this->userSvc->getUser($user_id); + if ($user === null) { + throw new UserNotFound(); + } // Add a bid if ($request->isMethod('PUT') || $request->isMethod('POST')) { @@ -146,6 +154,10 @@ class UserController extends Controller public function fleet(Request $request) { $user = $this->userRepo->find($this->getUserId($request)); + if ($user === null) { + throw new UserNotFound(); + } + $subfleets = $this->userSvc->getAllowableSubfleets($user); return SubfleetResource::collection($subfleets); diff --git a/app/Models/Aircraft.php b/app/Models/Aircraft.php index c4621b79..58c81c1f 100644 --- a/app/Models/Aircraft.php +++ b/app/Models/Aircraft.php @@ -24,6 +24,7 @@ use Carbon\Carbon; * @property int status * @property int state * @property Carbon landing_time + * @property float fuel_onboard */ class Aircraft extends Model { diff --git a/app/Models/Enums/UserState.php b/app/Models/Enums/UserState.php index 5f8ef478..906e9069 100644 --- a/app/Models/Enums/UserState.php +++ b/app/Models/Enums/UserState.php @@ -11,6 +11,7 @@ class UserState extends Enum public const REJECTED = 2; public const ON_LEAVE = 3; public const SUSPENDED = 4; + public const DELETED = 5; protected static $labels = [ self::PENDING => 'user.state.pending', @@ -18,5 +19,6 @@ class UserState extends Enum self::REJECTED => 'user.state.rejected', self::ON_LEAVE => 'user.state.on_leave', self::SUSPENDED => 'user.state.suspended', + self::DELETED => 'user.state.deleted', ]; } diff --git a/app/Models/User.php b/app/Models/User.php index 71406758..9949d582 100755 --- a/app/Models/User.php +++ b/app/Models/User.php @@ -38,6 +38,8 @@ use Laratrust\Traits\LaratrustUserTrait; * @property string last_pirep_id * @property Pirep last_pirep * @property UserFieldValue[] fields + * @property Role[] roles + * @property Subfleet[] subfleets * * @mixin \Illuminate\Database\Eloquent\Builder * @mixin \Illuminate\Notifications\Notifiable diff --git a/app/Notifications/EventHandler.php b/app/Notifications/EventHandler.php index ea755258..e11efcb3 100644 --- a/app/Notifications/EventHandler.php +++ b/app/Notifications/EventHandler.php @@ -46,7 +46,7 @@ class EventHandler extends Listener * * @param \App\Contracts\Notification $notification */ - protected function notifyAdmins($notification) + protected function notifyAdmins(\App\Contracts\Notification $notification) { $admin_users = User::whereRoleIs('admin')->get(); @@ -67,8 +67,12 @@ class EventHandler extends Listener * @param User $user * @param \App\Contracts\Notification $notification */ - protected function notifyUser($user, $notification) + protected function notifyUser(User $user, \App\Contracts\Notification $notification) { + if ($user->state === UserState::DELETED) { + return; + } + try { $user->notify($notification); } catch (Exception $e) { @@ -90,7 +94,7 @@ class EventHandler extends Listener } /** @var Collection $users */ - $users = User::where($where)->get(); + $users = User::where($where)->where('state', '<>', UserState::DELETED)->get(); if (empty($users) || $users->count() === 0) { return; } diff --git a/app/Services/PirepService.php b/app/Services/PirepService.php index dfe20394..3d14fd1a 100644 --- a/app/Services/PirepService.php +++ b/app/Services/PirepService.php @@ -147,6 +147,7 @@ class PirepService extends Service $dupe_pirep = $this->findDuplicate($pirep); if ($dupe_pirep !== false) { $pirep = $dupe_pirep; + Log::info('Found duplicate PIREP, id='.$dupe_pirep->id); if ($pirep->cancelled) { throw new \App\Exceptions\PirepCancelled($pirep); } @@ -293,9 +294,11 @@ class PirepService extends Service $time_limit = Carbon::now('UTC')->subMinutes($minutes)->toDateTimeString(); $where = [ - 'user_id' => $pirep->user_id, - 'airline_id' => $pirep->airline_id, - 'flight_number' => $pirep->flight_number, + 'user_id' => $pirep->user_id, + 'airline_id' => $pirep->airline_id, + 'flight_number' => $pirep->flight_number, + 'dpt_airport_id' => $pirep->dpt_airport_id, + 'arr_airport_id' => $pirep->arr_airport_id, ]; if (filled($pirep->route_code)) { diff --git a/app/Services/UserService.php b/app/Services/UserService.php index 275117cf..7da38a1e 100644 --- a/app/Services/UserService.php +++ b/app/Services/UserService.php @@ -9,12 +9,14 @@ use App\Events\UserStatsChanged; use App\Exceptions\PilotIdNotFound; use App\Exceptions\UserPilotIdExists; use App\Models\Airline; +use App\Models\Bid; use App\Models\Enums\PirepState; use App\Models\Enums\UserState; use App\Models\Pirep; use App\Models\Rank; use App\Models\Role; use App\Models\User; +use App\Models\UserFieldValue; use App\Repositories\AircraftRepository; use App\Repositories\AirlineRepository; use App\Repositories\SubfleetRepository; @@ -23,6 +25,7 @@ use App\Support\Units\Time; use App\Support\Utils; use Carbon\Carbon; use Illuminate\Support\Collection; +use Illuminate\Support\Facades\Hash; use Illuminate\Support\Facades\Log; use function is_array; @@ -60,14 +63,19 @@ class UserService extends Service * * @param $user_id * - * @return User + * @return User|null */ - public function getUser($user_id): User + public function getUser($user_id): ?User { + /** @var User $user */ $user = $this->userRepo ->with(['airline', 'bids', 'rank']) ->find($user_id); + if ($user->state === UserState::DELETED) { + return null; + } + // Load the proper subfleets to the rank $user->rank->subfleets = $this->getAllowableSubfleets($user); $user->subfleets = $user->rank->subfleets; @@ -117,6 +125,33 @@ class UserService extends Service return $user; } + /** + * Remove the user. But don't actually delete them - set the name to deleted, email to + * something random + * + * @param User $user + * + * @throws \Exception + */ + 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); + + // Delete any fields which might have personal information + UserFieldValue::where('user_id', $user->id)->delete(); + + // Remove any bids + Bid::where('user_id', $user->id)->delete(); + } + /** * Add a user to a given role * @@ -125,7 +160,7 @@ class UserService extends Service * * @return User */ - public function addUserToRole(User $user, $roleName): User + public function addUserToRole(User $user, string $roleName): User { $role = Role::where(['name' => $roleName])->first(); $user->attachRole($role); diff --git a/resources/lang/en/user.php b/resources/lang/en/user.php index 7718936a..fe234f7a 100644 --- a/resources/lang/en/user.php +++ b/resources/lang/en/user.php @@ -8,5 +8,6 @@ return [ 'rejected' => 'Rejected', 'on_leave' => 'On Leave', 'suspended' => 'Suspended', + 'deleted' => 'Deleted', ], ]; diff --git a/resources/lang/es/user.php b/resources/lang/es/user.php index fbf87170..933a2300 100644 --- a/resources/lang/es/user.php +++ b/resources/lang/es/user.php @@ -8,5 +8,6 @@ return [ 'rejected' => 'Rechazado', 'on_leave' => 'De vacaciones', 'suspended' => 'Suspendido', + 'deleted' => 'Borrar', ], ]; diff --git a/resources/lang/it/user.php b/resources/lang/it/user.php index bea70b03..a3f29684 100644 --- a/resources/lang/it/user.php +++ b/resources/lang/it/user.php @@ -8,5 +8,6 @@ return [ 'rejected' => 'Rifiutato', 'on_leave' => 'In Ferie', 'suspended' => 'Sospeso', + 'deleted' => 'Cancellato', ], ]; diff --git a/resources/lang/pt-br/user.php b/resources/lang/pt-br/user.php index 7c431acc..0c3fd973 100755 --- a/resources/lang/pt-br/user.php +++ b/resources/lang/pt-br/user.php @@ -8,5 +8,6 @@ return [ 'rejected' => 'Rejeitado', 'on_leave' => 'Em licença', 'suspended' => 'Suspensa', + 'deleted' => 'Excluído', ], ]; diff --git a/tests/AcarsTest.php b/tests/AcarsTest.php index a2e4e356..639b11d1 100644 --- a/tests/AcarsTest.php +++ b/tests/AcarsTest.php @@ -418,20 +418,26 @@ class AcarsTest extends TestCase /** * Post a PIREP into a PREFILE state and post ACARS + * + * @throws \Exception */ public function testAcarsUpdates() { $subfleet = $this->createSubfleetWithAircraft(2); $rank = $this->createRank(10, [$subfleet['subfleet']->id]); - $this->user = factory(User::class)->create( - [ - 'rank_id' => $rank->id, - ] - ); + /** @var User user */ + $this->user = factory(User::class)->create([ + 'rank_id' => $rank->id, + ]); + /** @var Airport $airport */ $airport = factory(Airport::class)->create(); + + /** @var Airline $airline */ $airline = factory(Airline::class)->create(); + + /** @var Aircraft $aircraft */ $aircraft = $subfleet['aircraft']->random(); $uri = '/api/pireps/prefile'; diff --git a/tests/UserTest.php b/tests/UserTest.php index 173783b6..dd584e20 100644 --- a/tests/UserTest.php +++ b/tests/UserTest.php @@ -311,6 +311,29 @@ class UserTest extends TestCase $this->assertEquals(4, $user3->pilot_id); } + public function testUserPilotDeleted() + { + $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); + + // 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); + $this->assertNotEquals($new_user['password'], $user->password); + } + /** * Test that a user's name is private */