From 3e1d9080df2faa87819cb5bd1d042cb7b996272d Mon Sep 17 00:00:00 2001 From: Nabeel Shahzad Date: Tue, 16 Jul 2019 13:54:14 -0400 Subject: [PATCH] Backend changes separating id from pilot_id --- app/Database/factories/UserFactory.php | 1 + .../2019_07_16_141152_users_add_pilot_id.php | 67 +++++ app/Exceptions/UserPilotIdExists.php | 9 + app/Http/Controllers/Admin/UserController.php | 2 +- app/Http/Controllers/Api/AcarsController.php | 2 +- app/Http/Controllers/Auth/LoginController.php | 2 +- .../Frontend/ProfileController.php | 6 +- app/Http/Resources/PirepComment.php | 1 + app/Http/Resources/User.php | 1 + app/Listeners/NotificationEvents.php | 2 +- app/Models/Observers/UserObserver.php | 26 ++ app/Models/User.php | 23 +- app/Providers/AppServiceProvider.php | 3 + app/Services/PirepService.php | 2 +- app/Services/UserService.php | 55 ++++- composer.json | 1 + composer.lock | 233 +++++++++++++++++- resources/views/admin/pireps/edit.blade.php | 2 +- .../layouts/default/profile/index.blade.php | 2 +- .../default/widgets/latest_pilots.blade.php | 2 +- tests/UserTest.php | 46 +++- 21 files changed, 454 insertions(+), 34 deletions(-) create mode 100644 app/Database/migrations/2019_07_16_141152_users_add_pilot_id.php create mode 100644 app/Exceptions/UserPilotIdExists.php create mode 100644 app/Models/Observers/UserObserver.php diff --git a/app/Database/factories/UserFactory.php b/app/Database/factories/UserFactory.php index a6ae14f0..8d18fc15 100644 --- a/app/Database/factories/UserFactory.php +++ b/app/Database/factories/UserFactory.php @@ -8,6 +8,7 @@ $factory->define(App\Models\User::class, function (Faker $faker) { return [ 'id' => null, + 'pilot_id' => 0, 'name' => $faker->name, 'email' => $faker->safeEmail, 'password' => $password ?: $password = Hash::make('secret'), diff --git a/app/Database/migrations/2019_07_16_141152_users_add_pilot_id.php b/app/Database/migrations/2019_07_16_141152_users_add_pilot_id.php new file mode 100644 index 00000000..45500ef6 --- /dev/null +++ b/app/Database/migrations/2019_07_16_141152_users_add_pilot_id.php @@ -0,0 +1,67 @@ +unsignedBigInteger('pilot_id') + ->after('id') + ->unique() + ->nullable() + ->index('users_pilot_id'); + }); + + // Migrate the current pilot IDs + DB::update('UPDATE `users` SET `pilot_id`=`id`'); + + // Drop the old ID column and add a new one + /*Schema::table('users', function (Blueprint $table) { + $table->dropPrimary('users_id_primary'); + $table->dropColumn('id'); + $table->string('id', Model::ID_MAX_LENGTH)->primary(); + }); + + // Update the users to use the `pilot_id` (so we don't need to migrate data from other tables) + $users = DB::table('users')->get(['id']); + foreach ($users as $user) { + $user->id = $user->pilot_id; + $user->save(); + }*/ + + // role_user + // permission_user + // sessions + // pireps + // bids + // news + // user_awards + } + + /** + * Reverse the migrations. + * + * @return void + */ + public function down() + { + Schema::table('users', function (Blueprint $table) { + $table->dropColumn('pilot_id'); + }); + } +} diff --git a/app/Exceptions/UserPilotIdExists.php b/app/Exceptions/UserPilotIdExists.php new file mode 100644 index 00000000..3fd62082 --- /dev/null +++ b/app/Exceptions/UserPilotIdExists.php @@ -0,0 +1,9 @@ +pilot_id.'"'); + Log::info('Regenerating API key "'.$user->ident.'"'); $user->api_key = Utils::generateApiKey(); $user->save(); diff --git a/app/Http/Controllers/Api/AcarsController.php b/app/Http/Controllers/Api/AcarsController.php index 88d3676f..71303e90 100644 --- a/app/Http/Controllers/Api/AcarsController.php +++ b/app/Http/Controllers/Api/AcarsController.php @@ -131,7 +131,7 @@ class AcarsController extends Controller $this->checkCancelled($pirep); Log::debug( - 'Posting ACARS update (user: '.Auth::user()->pilot_id.', pirep id :'.$id.'): ', + 'Posting ACARS update (user: '.Auth::user()->ident.', pirep id :'.$id.'): ', $request->post() ); diff --git a/app/Http/Controllers/Auth/LoginController.php b/app/Http/Controllers/Auth/LoginController.php index dd685123..40c4550a 100755 --- a/app/Http/Controllers/Auth/LoginController.php +++ b/app/Http/Controllers/Auth/LoginController.php @@ -49,7 +49,7 @@ class LoginController extends Controller // TODO: How to handle ON_LEAVE? if ($user->state !== UserState::ACTIVE) { - Log::info('Trying to login '.$user->pilot_id.', state ' + Log::info('Trying to login '.$user->ident.', state ' .UserState::label($user->state)); // Log them out diff --git a/app/Http/Controllers/Frontend/ProfileController.php b/app/Http/Controllers/Frontend/ProfileController.php index 3fdc1290..59e67a04 100644 --- a/app/Http/Controllers/Frontend/ProfileController.php +++ b/app/Http/Controllers/Frontend/ProfileController.php @@ -133,7 +133,7 @@ class ProfileController extends Controller ]); if ($validator->fails()) { - Log::info('validator failed for user '.$user->pilot_id); + Log::info('validator failed for user '.$user->ident); Log::info($validator->errors()->toArray()); return redirect(route('frontend.profile.edit', $id)) @@ -153,7 +153,7 @@ class ProfileController extends Controller } if ($request->hasFile('avatar')) { $avatar = $request->file('avatar'); - $file_name = $user->pilot_id.'.'.$avatar->getClientOriginalExtension(); + $file_name = $user->ident.'.'.$avatar->getClientOriginalExtension(); $path = "avatars/{$file_name}"; // Create the avatar, resizing it and keeping the aspect ratio. @@ -189,7 +189,7 @@ class ProfileController extends Controller public function regen_apikey(Request $request) { $user = User::find(Auth::user()->id); - Log::info('Regenerating API key "'.$user->pilot_id.'"'); + Log::info('Regenerating API key "'.$user->ident.'"'); $user->api_key = Utils::generateApiKey(); $user->save(); diff --git a/app/Http/Resources/PirepComment.php b/app/Http/Resources/PirepComment.php index 58336a5b..ff0eabaa 100644 --- a/app/Http/Resources/PirepComment.php +++ b/app/Http/Resources/PirepComment.php @@ -27,6 +27,7 @@ class PirepComment extends Resource 'user' => [ 'id' => $user->id, 'pilot_id' => $user->pilot_id, + 'ident' => $user->ident, 'name' => $user->name, ], ]; diff --git a/app/Http/Resources/User.php b/app/Http/Resources/User.php index 295d1343..5e0e1dd8 100644 --- a/app/Http/Resources/User.php +++ b/app/Http/Resources/User.php @@ -11,6 +11,7 @@ class User extends Resource return [ 'id' => $this->id, 'pilot_id' => $this->pilot_id, + 'ident' => $this->ident, 'name' => $this->name, 'email' => $this->email, 'apikey' => $this->apikey, diff --git a/app/Listeners/NotificationEvents.php b/app/Listeners/NotificationEvents.php index 52affeaf..86c923d2 100644 --- a/app/Listeners/NotificationEvents.php +++ b/app/Listeners/NotificationEvents.php @@ -68,7 +68,7 @@ class NotificationEvents extends Listener public function onUserRegister(UserRegistered $event): void { Log::info('onUserRegister: ' - .$event->user->pilot_id.' is ' + .$event->user->ident.' is ' .UserState::label($event->user->state) .', sending active email'); diff --git a/app/Models/Observers/UserObserver.php b/app/Models/Observers/UserObserver.php new file mode 100644 index 00000000..7b648ebe --- /dev/null +++ b/app/Models/Observers/UserObserver.php @@ -0,0 +1,26 @@ +userSvc = $userSvc; + } + + /** + * After a user has been created, do some stuff + * + * @param User $user + */ + public function created(User $user): void + { + $this->userSvc->findAndSetPilotId($user); + } +} diff --git a/app/Models/User.php b/app/Models/User.php index bd95c870..1a63e2f1 100755 --- a/app/Models/User.php +++ b/app/Models/User.php @@ -10,14 +10,17 @@ use Illuminate\Notifications\Notifiable; use Laratrust\Traits\LaratrustUserTrait; /** - * @property int id + * @property int id + * @property int pilot_id * @property string name * @property string email * @property string password * @property string api_key - * @property mixed ident + * @property mixed timezone + * @property string ident * @property string curr_airport_id * @property string home_airport_id + * @property Airline airline * @property Flight[] flights * @property string flight_time * @property string remember_token @@ -45,9 +48,11 @@ class User extends Authenticatable public $journal_type = JournalType::USER; protected $fillable = [ + 'id', 'name', 'email', 'password', + 'pilot_id', 'airline_id', 'rank_id', 'api_key', @@ -78,6 +83,8 @@ class User extends Authenticatable ]; protected $casts = [ + 'id' => 'integer', + 'pilot_id' => 'integer', 'flights' => 'integer', 'flight_time' => 'integer', 'transfer_time' => 'integer', @@ -96,19 +103,11 @@ class User extends Authenticatable /** * @return string */ - public function getPilotIdAttribute() + public function getIdentAttribute() { $length = setting('pilots.id_length'); - return $this->airline->icao.str_pad($this->id, $length, '0', STR_PAD_LEFT); - } - - /** - * @return string - */ - public function getIdentAttribute() - { - return $this->getPilotIdAttribute(); + return $this->airline->icao . str_pad($this->pilot_id, $length, '0', STR_PAD_LEFT); } /** diff --git a/app/Providers/AppServiceProvider.php b/app/Providers/AppServiceProvider.php index 8979978a..7d718b8f 100755 --- a/app/Providers/AppServiceProvider.php +++ b/app/Providers/AppServiceProvider.php @@ -15,10 +15,12 @@ use App\Models\Observers\JournalTransactionObserver; use App\Models\Observers\SettingObserver; use App\Models\Observers\Sluggable; use App\Models\Observers\SubfleetObserver; +use App\Models\Observers\UserObserver; use App\Models\PirepField; use App\Models\PirepFieldValue; use App\Models\Setting; use App\Models\Subfleet; +use App\Models\User; use App\Repositories\SettingRepository; use App\Services\ModuleService; use Illuminate\Support\Facades\Schema; @@ -53,6 +55,7 @@ class AppServiceProvider extends ServiceProvider Setting::observe(SettingObserver::class); Subfleet::observe(SubfleetObserver::class); + User::observe(UserObserver::class); } /** diff --git a/app/Services/PirepService.php b/app/Services/PirepService.php index 739e8c7e..24f9cc87 100644 --- a/app/Services/PirepService.php +++ b/app/Services/PirepService.php @@ -425,7 +425,7 @@ class PirepService extends Service ]); if ($bid) { - Log::info('Bid for user: '.$pirep->user->pilot_id.' on flight '.$flight->ident); + Log::info('Bid for user: '.$pirep->user->ident.' on flight '.$flight->ident); $bid->delete(); } } diff --git a/app/Services/UserService.php b/app/Services/UserService.php index 74a24fe7..145fb384 100644 --- a/app/Services/UserService.php +++ b/app/Services/UserService.php @@ -6,6 +6,7 @@ use App\Contracts\Service; use App\Events\UserRegistered; use App\Events\UserStateChanged; use App\Events\UserStatsChanged; +use App\Exceptions\UserPilotIdExists; use App\Models\Enums\PirepState; use App\Models\Enums\UserState; use App\Models\Pirep; @@ -16,6 +17,7 @@ use App\Repositories\AircraftRepository; use App\Repositories\SubfleetRepository; use App\Support\Units\Time; use Illuminate\Support\Collection; +use function is_array; use Log; /** @@ -67,7 +69,7 @@ class UserService extends Service // $user->attachRole($role); // Attach any additional roles - if (!empty($groups) && \is_array($groups)) { + if (!empty($groups) && is_array($groups)) { foreach ($groups as $group) { $role = Role::where('name', $group)->first(); $user->attachRole($role); @@ -83,6 +85,55 @@ class UserService extends Service return $user; } + /** + * Find the next available pilot ID and set the current user's pilot_id to that +1 + * Called from UserObserver right now after a record is created + * + * @param User $user + * + * @return User + */ + public function findAndSetPilotId(User $user): User + { + if ($user->pilot_id !== null && $user->pilot_id > 0) { + return $user; + } + + $max = (int) User::max('pilot_id'); + $user->pilot_id = $max + 1; + $user->save(); + + Log::info('Set pilot ID for user ' . $user->id . ' to ' . $user->pilot_id); + + return $user; + } + + /** + * Change a user's pilot ID + * + * @param User $user + * @param int $pilot_id + * + * @throws UserPilotIdExists + * + * @return User + */ + public function changePilotId(User $user, int $pilot_id): User + { + if (User::where('pilot_id', '=', $pilot_id)->exists()) { + Log::error('User with id '.$pilot_id.' already exists'); + throw new UserPilotIdExists(); + } + + $old_id = $user->pilot_id; + $user->pilot_id = $pilot_id; + $user->save(); + + Log::info('Changed pilot ID for user '.$user->id.' from '.$old_id.' to '.$user->pilot_id); + + return $user; + } + /** * Return the subfleets this user is allowed access to, * based on their current rank @@ -133,7 +184,7 @@ class UserService extends Service return $user; } - Log::info('User '.$user->pilot_id.' state changing from ' + Log::info('User '.$user->ident.' state changing from ' .UserState::label($old_state).' to ' .UserState::label($user->state)); diff --git a/composer.json b/composer.json index 658d65b7..cff6ef6b 100755 --- a/composer.json +++ b/composer.json @@ -19,6 +19,7 @@ "anhskohbo/no-captcha": "3.0.*", "appstract/laravel-opcache": "^2.0", "arrilot/laravel-widgets": "3.13.*", + "doctrine/dbal": "2.9.*", "fzaninotto/faker": "^1.8", "guzzlehttp/guzzle": "6.3.*", "hashids/hashids": "2.0.*", diff --git a/composer.lock b/composer.lock index 443f8b43..118c2e99 100644 --- a/composer.lock +++ b/composer.lock @@ -4,7 +4,7 @@ "Read more about it at https://getcomposer.org/doc/01-basic-usage.md#installing-dependencies", "This file is @generated automatically" ], - "content-hash": "b4e5994cf5891923a2ed410bacad5fc4", + "content-hash": "99c96d01e78e1d4400295a887ac36667", "packages": [ { "name": "akaunting/money", @@ -857,6 +857,237 @@ ], "time": "2019-05-27T17:52:04+00:00" }, + { + "name": "doctrine/cache", + "version": "v1.8.0", + "source": { + "type": "git", + "url": "https://github.com/doctrine/cache.git", + "reference": "d768d58baee9a4862ca783840eca1b9add7a7f57" + }, + "dist": { + "type": "zip", + "url": "https://api.github.com/repos/doctrine/cache/zipball/d768d58baee9a4862ca783840eca1b9add7a7f57", + "reference": "d768d58baee9a4862ca783840eca1b9add7a7f57", + "shasum": "" + }, + "require": { + "php": "~7.1" + }, + "conflict": { + "doctrine/common": ">2.2,<2.4" + }, + "require-dev": { + "alcaeus/mongo-php-adapter": "^1.1", + "doctrine/coding-standard": "^4.0", + "mongodb/mongodb": "^1.1", + "phpunit/phpunit": "^7.0", + "predis/predis": "~1.0" + }, + "suggest": { + "alcaeus/mongo-php-adapter": "Required to use legacy MongoDB driver" + }, + "type": "library", + "extra": { + "branch-alias": { + "dev-master": "1.8.x-dev" + } + }, + "autoload": { + "psr-4": { + "Doctrine\\Common\\Cache\\": "lib/Doctrine/Common/Cache" + } + }, + "notification-url": "https://packagist.org/downloads/", + "license": [ + "MIT" + ], + "authors": [ + { + "name": "Roman Borschel", + "email": "roman@code-factory.org" + }, + { + "name": "Benjamin Eberlei", + "email": "kontakt@beberlei.de" + }, + { + "name": "Guilherme Blanco", + "email": "guilhermeblanco@gmail.com" + }, + { + "name": "Jonathan Wage", + "email": "jonwage@gmail.com" + }, + { + "name": "Johannes Schmitt", + "email": "schmittjoh@gmail.com" + } + ], + "description": "Caching library offering an object-oriented API for many cache backends", + "homepage": "https://www.doctrine-project.org", + "keywords": [ + "cache", + "caching" + ], + "time": "2018-08-21T18:01:43+00:00" + }, + { + "name": "doctrine/dbal", + "version": "v2.9.2", + "source": { + "type": "git", + "url": "https://github.com/doctrine/dbal.git", + "reference": "22800bd651c1d8d2a9719e2a3dc46d5108ebfcc9" + }, + "dist": { + "type": "zip", + "url": "https://api.github.com/repos/doctrine/dbal/zipball/22800bd651c1d8d2a9719e2a3dc46d5108ebfcc9", + "reference": "22800bd651c1d8d2a9719e2a3dc46d5108ebfcc9", + "shasum": "" + }, + "require": { + "doctrine/cache": "^1.0", + "doctrine/event-manager": "^1.0", + "ext-pdo": "*", + "php": "^7.1" + }, + "require-dev": { + "doctrine/coding-standard": "^5.0", + "jetbrains/phpstorm-stubs": "^2018.1.2", + "phpstan/phpstan": "^0.10.1", + "phpunit/phpunit": "^7.4", + "symfony/console": "^2.0.5|^3.0|^4.0", + "symfony/phpunit-bridge": "^3.4.5|^4.0.5" + }, + "suggest": { + "symfony/console": "For helpful console commands such as SQL execution and import of files." + }, + "bin": [ + "bin/doctrine-dbal" + ], + "type": "library", + "extra": { + "branch-alias": { + "dev-master": "2.9.x-dev", + "dev-develop": "3.0.x-dev" + } + }, + "autoload": { + "psr-4": { + "Doctrine\\DBAL\\": "lib/Doctrine/DBAL" + } + }, + "notification-url": "https://packagist.org/downloads/", + "license": [ + "MIT" + ], + "authors": [ + { + "name": "Roman Borschel", + "email": "roman@code-factory.org" + }, + { + "name": "Benjamin Eberlei", + "email": "kontakt@beberlei.de" + }, + { + "name": "Guilherme Blanco", + "email": "guilhermeblanco@gmail.com" + }, + { + "name": "Jonathan Wage", + "email": "jonwage@gmail.com" + } + ], + "description": "Powerful PHP database abstraction layer (DBAL) with many features for database schema introspection and management.", + "homepage": "https://www.doctrine-project.org/projects/dbal.html", + "keywords": [ + "abstraction", + "database", + "dbal", + "mysql", + "persistence", + "pgsql", + "php", + "queryobject" + ], + "time": "2018-12-31T03:27:51+00:00" + }, + { + "name": "doctrine/event-manager", + "version": "v1.0.0", + "source": { + "type": "git", + "url": "https://github.com/doctrine/event-manager.git", + "reference": "a520bc093a0170feeb6b14e9d83f3a14452e64b3" + }, + "dist": { + "type": "zip", + "url": "https://api.github.com/repos/doctrine/event-manager/zipball/a520bc093a0170feeb6b14e9d83f3a14452e64b3", + "reference": "a520bc093a0170feeb6b14e9d83f3a14452e64b3", + "shasum": "" + }, + "require": { + "php": "^7.1" + }, + "conflict": { + "doctrine/common": "<2.9@dev" + }, + "require-dev": { + "doctrine/coding-standard": "^4.0", + "phpunit/phpunit": "^7.0" + }, + "type": "library", + "extra": { + "branch-alias": { + "dev-master": "1.0.x-dev" + } + }, + "autoload": { + "psr-4": { + "Doctrine\\Common\\": "lib/Doctrine/Common" + } + }, + "notification-url": "https://packagist.org/downloads/", + "license": [ + "MIT" + ], + "authors": [ + { + "name": "Roman Borschel", + "email": "roman@code-factory.org" + }, + { + "name": "Benjamin Eberlei", + "email": "kontakt@beberlei.de" + }, + { + "name": "Guilherme Blanco", + "email": "guilhermeblanco@gmail.com" + }, + { + "name": "Jonathan Wage", + "email": "jonwage@gmail.com" + }, + { + "name": "Johannes Schmitt", + "email": "schmittjoh@gmail.com" + }, + { + "name": "Marco Pivetta", + "email": "ocramius@gmail.com" + } + ], + "description": "Doctrine Event Manager component", + "homepage": "https://www.doctrine-project.org/projects/event-manager.html", + "keywords": [ + "event", + "eventdispatcher", + "eventmanager" + ], + "time": "2018-06-11T11:59:03+00:00" + }, { "name": "doctrine/inflector", "version": "v1.3.0", diff --git a/resources/views/admin/pireps/edit.blade.php b/resources/views/admin/pireps/edit.blade.php index d10a3557..3567a128 100644 --- a/resources/views/admin/pireps/edit.blade.php +++ b/resources/views/admin/pireps/edit.blade.php @@ -11,7 +11,7 @@
Filed By: - {{ $pirep->user->pilot_id }} {{ $pirep->user->name }} + {{ $pirep->user->ident }} {{ $pirep->user->name }}
diff --git a/resources/views/layouts/default/profile/index.blade.php b/resources/views/layouts/default/profile/index.blade.php index 8a90fb8d..73ef26dd 100644 --- a/resources/views/layouts/default/profile/index.blade.php +++ b/resources/views/layouts/default/profile/index.blade.php @@ -13,7 +13,7 @@

{{ $user->name }}

-
{{ $user->pilot_id }}
+
{{ $user->ident }}
{{ $user->rank->name }}

{{ $user->airline->name }} diff --git a/resources/views/layouts/default/widgets/latest_pilots.blade.php b/resources/views/layouts/default/widgets/latest_pilots.blade.php index 7e25ea9e..4ea5bdd4 100644 --- a/resources/views/layouts/default/widgets/latest_pilots.blade.php +++ b/resources/views/layouts/default/widgets/latest_pilots.blade.php @@ -2,7 +2,7 @@ @foreach($users as $u) - {{ $u->pilot_id }} + {{ $u->ident }} {{ $u->name }} diff --git a/tests/UserTest.php b/tests/UserTest.php index 905d8d46..9e47cec1 100644 --- a/tests/UserTest.php +++ b/tests/UserTest.php @@ -1,5 +1,7 @@ id, ]); - /* - * Now we can do some actual tests - */ - - /* - * Do some sanity checks first - */ - // Make sure no flights are filtered out $this->settingsRepo->store('pilots.only_flights_from_current', false); @@ -195,4 +189,40 @@ class UserTest extends TestCase $body = $response->json()['data']; $this->assertCount(1, $body[0]['subfleets']); } + + /** + * Test the pilot ID being added when a new user is created + * @expectedException \App\Exceptions\UserPilotIdExists + */ + public function testUserPilotIdChangeAlreadyExists() + { + $user1 = factory(App\Models\User::class)->create(['id' => 1]); + $user2 = factory(App\Models\User::class)->create(['id' => 2]); + + // Now try to change the original user's pilot_id to 2 (should conflict) + $this->userSvc->changePilotId($user1, 2); + } + + /** + * Test the pilot ID being added when a new user is created + */ + public function testUserPilotIdAdded() + { + $new_user = factory(App\Models\User::class)->create(['id' => 1]); + $user = $this->userSvc->createUser($new_user); + $this->assertEquals($user->id, $user->pilot_id); + + // Add a second user + $new_user = factory(App\Models\User::class)->create(['id' => 2]); + $user2 = $this->userSvc->createUser($new_user); + $this->assertEquals($user2->id, $user2->pilot_id); + + // Now try to change the original user's pilot_id to 3 + $user = $this->userSvc->changePilotId($user, 3); + $this->assertEquals(3, $user->pilot_id); + + // Create a new user and the pilot_id should be 4 + $user3 = factory(App\Models\User::class)->create(['id' => 3]); + $this->assertEquals(4, $user3->pilot_id); + } }