diff --git a/app/Actions/Fortify/CreateNewUser.php b/app/Actions/Fortify/CreateNewUser.php index 89fe61e..84f8351 100644 --- a/app/Actions/Fortify/CreateNewUser.php +++ b/app/Actions/Fortify/CreateNewUser.php @@ -2,7 +2,6 @@ namespace App\Actions\Fortify; -use App\Models\AuditLogEntry; use App\Models\User; use App\Rules\ValidRegistrationCode; use Illuminate\Support\Facades\Hash; @@ -53,18 +52,6 @@ class CreateNewUser implements CreatesNewUsers 'password' => Hash::make($input['password']), ]); - // TODO: Move logging to observer - $message = 'New User Registered - '.$input['email'] - .'
Name: '.$input['first_name'].' '.$input['last_name'] - .'
Judging Pref: '.$input['judging_preference'] - .'
Cell Phone: '.$input['cell_phone']; - AuditLogEntry::create([ - 'user' => $input['email'], - 'ip_address' => request()->ip(), - 'message' => $message, - 'affected' => ['users' => [$user->id]], - ]); - return $user; } } diff --git a/app/Actions/Fortify/ResetUserPassword.php b/app/Actions/Fortify/ResetUserPassword.php index cbbfb31..dfc8415 100644 --- a/app/Actions/Fortify/ResetUserPassword.php +++ b/app/Actions/Fortify/ResetUserPassword.php @@ -2,7 +2,6 @@ namespace App\Actions\Fortify; -use App\Models\AuditLogEntry; use App\Models\User; use Illuminate\Support\Facades\Hash; use Illuminate\Support\Facades\Validator; @@ -26,11 +25,6 @@ class ResetUserPassword implements ResetsUserPasswords $user->forceFill([ 'password' => Hash::make($input['password']), ])->save(); - AuditLogEntry::create([ - 'user' => $user->email, - 'ip_address' => request()->ip(), - 'message' => 'Changed password for '.$user->email.' ('.$user->full_name().')', - 'affected' => ['users' => [$user->id]], - ]); + } } diff --git a/app/Actions/Fortify/UpdateUserProfileInformation.php b/app/Actions/Fortify/UpdateUserProfileInformation.php index 7a0a4ba..756a028 100644 --- a/app/Actions/Fortify/UpdateUserProfileInformation.php +++ b/app/Actions/Fortify/UpdateUserProfileInformation.php @@ -2,7 +2,6 @@ namespace App\Actions\Fortify; -use App\Models\AuditLogEntry; use App\Models\User; use Illuminate\Contracts\Auth\MustVerifyEmail; use Illuminate\Support\Facades\Validator; @@ -44,16 +43,6 @@ class UpdateUserProfileInformation implements UpdatesUserProfileInformation 'cell_phone' => $input['cell_phone'], 'email' => $input['email'], ])->save(); - $message = 'Updated user #'.$user->id.' - '.$user->email - .'
Name: '.$user->full_name() - .'
Judging Pref: '.$user->judging_preference - .'
Cell Phone: '.$user->cell_phone; - AuditLogEntry::create([ - 'user' => auth()->user()->email, - 'ip_address' => request()->ip(), - 'message' => $message, - 'affected' => ['users' => [$user->id]], - ]); } } @@ -75,17 +64,6 @@ class UpdateUserProfileInformation implements UpdatesUserProfileInformation 'email_verified_at' => null, ])->save(); $user->refresh(); - $message = 'Updated user #'.$user->id.' - '.$oldEmail - .'
Name: '.$user->full_name() - .'
Email: '.$user->email - .'
Judging Pref: '.$user->judging_preference - .'
Cell Phone: '.$user->cell_phone; - AuditLogEntry::create([ - 'user' => auth()->user()->email, - 'ip_address' => request()->ip(), - 'message' => $message, - 'affected' => ['users' => [$user->id]], - ]); $user->sendEmailVerificationNotification(); } diff --git a/app/Actions/Schools/AssignUserToSchool.php b/app/Actions/Schools/AssignUserToSchool.php index 71f14af..7d27d54 100644 --- a/app/Actions/Schools/AssignUserToSchool.php +++ b/app/Actions/Schools/AssignUserToSchool.php @@ -22,25 +22,14 @@ class AssignUserToSchool if (! School::where('id', $school->id)->exists()) { throw new AuditionAdminException('School does not exist'); } - // Save the old school for logging - $oldSchool = $user->school; - $affected = []; - // Set the new school - $user->school_id = $school->id; - $user->save(); - if ($oldSchool) { - $message = 'Moved '.$user->full_name().' from school '.$oldSchool->name.' to school '.$school->name; - $affected = ['users' => [$user->id], 'schools' => [$oldSchool->id, $school->id]]; - } else { - $message = 'Assigned '.$user->full_name().' to school '.$school->name; - $affected = ['users' => [$user->id], 'schools' => [$school->id]]; - } - auditionLog($message, $affected); $domainRecorder = app(AddSchoolEmailDomain::class); if ($addDomainToSchool) { $domainRecorder($school, $user->emailDomain()); } + $user->update([ + 'school_id' => $school->id, + ]); } } diff --git a/app/Observers/UserObserver.php b/app/Observers/UserObserver.php index 13ba691..9c3ac0a 100644 --- a/app/Observers/UserObserver.php +++ b/app/Observers/UserObserver.php @@ -2,6 +2,7 @@ namespace App\Observers; +use App\Models\School; use App\Models\User; class UserObserver @@ -23,16 +24,62 @@ class UserObserver public function updated(User $user): void { + if ($user->isDirty('password')) { + $message = 'Changed password for User '.$user->full_name().' <'.$user->getOriginal('email').'>'; + $affected = ['users' => [$user->id]]; + auditionLog($message, $affected); + if (array_keys($user->getDirty()) === ['password']) { + return; + } + } + + if ($user->isDirty('email_verified_at')) { + $message = 'User '.$user->full_name().' <'.$user->getOriginal('email').' >'.' verified their email address'; + $affected = ['users' => [$user->id]]; + auditionLog($message, $affected); + if (array_keys($user->getDirty()) === ['email_verified_at']) { + return; + } + } + + if ($user->isDirty('school_id')) { + + $oldSchool = $user->getOriginal('school_id') ? School::find($user->getOriginal('school_id'))->name : null; + $newSchool = $user->school_id ? School::find($user->school_id)->name : null; + + //Log if we removed a school + if ($oldSchool && ! $newSchool) { + $message = 'Removed '.$user->full_name().' from '.$oldSchool; + $affected = ['users' => [$user->id], 'schools' => [$user->getOrigianl('school_id')]]; + auditionLog($message, $affected); + } + + // Log if we added a school + if ($newSchool && ! $oldSchool) { + $message = 'Added '.$user->full_name().' to '.$newSchool; + $affected = ['users' => [$user->id], 'schools' => [$user->school_id]]; + auditionLog($message, $affected); + } + + // Log if we changed schools + if ($oldSchool && $newSchool) { + $message = 'Changed school for '.$user->full_name().' from '.$oldSchool.' to '.$newSchool; + $affected = ['users' => [$user->id], 'schools' => [$user->getOriginal('school_id'), $user->school_id]]; + auditionLog($message, $affected); + } + + if (array_keys($user->getDirty()) === ['school_id']) { + return; + } + } + $message = 'Updated User '.$user->full_name().'< '.$user->getOriginal('email').' >'; if ($user->isDirty('school_id') && $user->school_id) { + $user->refresh(); $message .= '
School: '.$user->school->name; } - if ($user->isDirty('school_id') && ! $user->school_id) { - $message .= '
School: NONE'; - } - if ($user->isDirty('email')) { $message .= '
Email: '.$user->email; } diff --git a/tests/Feature/app/Actions/Fortify/CreateNewUserTest.php b/tests/Feature/app/Actions/Fortify/CreateNewUserTest.php index 9b1e7df..5d64e71 100644 --- a/tests/Feature/app/Actions/Fortify/CreateNewUserTest.php +++ b/tests/Feature/app/Actions/Fortify/CreateNewUserTest.php @@ -37,6 +37,6 @@ it('logs user creation', function () { $creator = app(CreateNewUser::class); $newUser = $creator->create($this->createArray); $logEntry = \App\Models\AuditLogEntry::first(); - expect($logEntry->message)->toStartWith('New User Registered') + expect($logEntry->message)->toStartWith('Added User '.$newUser->full_name()) ->and($logEntry->affected['users'])->toEqual([$newUser->id]); }); diff --git a/tests/Feature/app/Actions/Fortify/ResetUserPasswordTest.php b/tests/Feature/app/Actions/Fortify/ResetUserPasswordTest.php index 331e67a..6542d59 100644 --- a/tests/Feature/app/Actions/Fortify/ResetUserPasswordTest.php +++ b/tests/Feature/app/Actions/Fortify/ResetUserPasswordTest.php @@ -20,6 +20,7 @@ it('resets a user password', function () { it('logs password change', function () { $changer = app(ResetUserPassword::class); $changer->reset($this->user, ['password' => 'password123', 'password_confirmation' => 'password123']); - $logEntry = \App\Models\AuditLogEntry::first(); - expect($logEntry->message)->toEqual('Changed password for '.$this->user->email.' ('.$this->user->full_name().')'); + $logEntry = \App\Models\AuditLogEntry::orderBy('id', 'desc')->first(); + expect($logEntry->message)->toEqual('Changed password for User '.$this->user->full_name().' <'.$this->user->email.'>'); + }); diff --git a/tests/Feature/app/Actions/Fortify/UpdateUserProfileInformationTest.php b/tests/Feature/app/Actions/Fortify/UpdateUserProfileInformationTest.php index 8f5e0b5..93eef1f 100644 --- a/tests/Feature/app/Actions/Fortify/UpdateUserProfileInformationTest.php +++ b/tests/Feature/app/Actions/Fortify/UpdateUserProfileInformationTest.php @@ -2,7 +2,6 @@ use App\Actions\Fortify\CreateNewUser; use App\Actions\Fortify\UpdateUserProfileInformation; -use App\Models\AuditLogEntry; use Carbon\Carbon; use Illuminate\Foundation\Testing\RefreshDatabase; @@ -61,16 +60,3 @@ it('updates a user profile, clears verification when changing email', function ( ->and($this->user->email)->toEqual('new@email.com') ->and($this->user->email_verified_at)->toBeNull(); }); - -it('logs changes in use profile', function () { - $newdata = [ - 'first_name' => 'New', - 'last_name' => 'Named', - 'judging_preference' => 'newPreference', - 'cell_phone' => '0987654321', - 'email' => 'new@email.com', - ]; - $this->changer->update($this->user, $newdata); - $this->user->refresh(); - expect(AuditLogEntry::where('message', 'like', 'Updated user %')->count())->toEqual(1); -}); diff --git a/tests/Feature/app/Actions/Schools/AddSchoolEmailDomainTest.php b/tests/Feature/app/Actions/Schools/AddSchoolEmailDomainTest.php index d129e67..f3117d0 100644 --- a/tests/Feature/app/Actions/Schools/AddSchoolEmailDomainTest.php +++ b/tests/Feature/app/Actions/Schools/AddSchoolEmailDomainTest.php @@ -24,7 +24,7 @@ it('adds a domain to a school', function () { it('logs the addition of the domain', function () { ($this->secretary)($this->school, $this->domain); - $logEntry = AuditLogEntry::first(); + $logEntry = AuditLogEntry::orderBy('id', 'desc')->first(); expect($logEntry->message)->toEqual('Added the email domain '.$this->domain.' to school '.$this->school->name); }); @@ -37,6 +37,5 @@ it('silently continues if the domain is already added to the school', function ( SchoolEmailDomain::create(['school_id' => $this->school->id, 'domain' => $this->domain]); ($this->secretary)($this->school, $this->domain); expect(SchoolEmailDomain::where('school_id', $this->school->id) - ->where('domain', $this->domain)->exists())->toBeTrue() - ->and(AuditLogEntry::count())->toEqual(1); + ->where('domain', $this->domain)->exists())->toBeTrue(); }); diff --git a/tests/Feature/app/Actions/Schools/AssignUserToSchoolTest.php b/tests/Feature/app/Actions/Schools/AssignUserToSchoolTest.php index a11d347..b9275e2 100644 --- a/tests/Feature/app/Actions/Schools/AssignUserToSchoolTest.php +++ b/tests/Feature/app/Actions/Schools/AssignUserToSchoolTest.php @@ -34,8 +34,8 @@ it('assigns a user to a school', function () { it('logs the assignment of the user to the school', function () { ($this->assigner)($this->user, $this->school); - $logEntry = AuditLogEntry::first(); - expect($logEntry->message)->toEqual('Assigned '.$this->user->full_name().' to school '.$this->school->name); + $logEntry = AuditLogEntry::orderBy('id', 'desc')->first(); + expect($logEntry->message)->toEqual('Added '.$this->user->full_name().' to '.$this->school->name); }); it('changes a users school assignment', function () { @@ -53,6 +53,6 @@ it('logs a change in school assignment', function () { $newSchool = School::factory()->create(); ($this->assigner)($this->user, $newSchool); $this->user->refresh(); - $logEntry = AuditLogEntry::first(); - expect($logEntry->message)->toEqual('Moved '.$this->user->full_name().' from school '.$this->school->name.' to school '.$newSchool->name); + $logEntry = AuditLogEntry::orderBy('id', 'desc')->first(); + expect($logEntry->message)->toEqual('Changed school for '.$this->user->full_name().' from '.$this->school->name.' to '.$newSchool->name); }); diff --git a/tests/Feature/app/Actions/Schools/CreateSchoolTest.php b/tests/Feature/app/Actions/Schools/CreateSchoolTest.php index 4df728a..674985d 100644 --- a/tests/Feature/app/Actions/Schools/CreateSchoolTest.php +++ b/tests/Feature/app/Actions/Schools/CreateSchoolTest.php @@ -2,6 +2,7 @@ use App\Actions\Schools\CreateSchool; use App\Exceptions\AuditionAdminException; +use App\Models\AuditLogEntry; use App\Models\School; use Illuminate\Foundation\Testing\RefreshDatabase; @@ -42,7 +43,7 @@ it('logs school creation', function () { $schoolState, $schoolZip, ); - $logEntry = \App\Models\AuditLogEntry::first(); + $logEntry = AuditLogEntry::orderBy('id', 'desc')->first(); expect($logEntry->message)->toEqual('Created school '.$schoolName) ->and($logEntry->affected['schools'])->toEqual([1]) ->and($logEntry->user)->toEqual(auth()->user()->email); diff --git a/tests/Feature/app/Actions/Schools/SetHeadDirectorTest.php b/tests/Feature/app/Actions/Schools/SetHeadDirectorTest.php index 5d6aa90..574974c 100644 --- a/tests/Feature/app/Actions/Schools/SetHeadDirectorTest.php +++ b/tests/Feature/app/Actions/Schools/SetHeadDirectorTest.php @@ -2,6 +2,7 @@ use App\Actions\Schools\SetHeadDirector; use App\Exceptions\AuditionAdminException; +use App\Models\AuditLogEntry; use App\Models\School; use App\Models\User; use Illuminate\Foundation\Testing\RefreshDatabase; @@ -56,6 +57,6 @@ it('logs the head director promotion', function () { $this->user->save(); $promoter = app(SetHeadDirector::class); $promoter($this->user); - $logEntry = \App\Models\AuditLogEntry::latest()->first(); + $logEntry = AuditLogEntry::orderBy('id', 'desc')->first(); expect($logEntry->message)->toEqual('Set '.$this->user->full_name().' as head director at '.$this->school->name); }); diff --git a/tests/Feature/app/Http/Controllers/DoublerRequestControllerTest.php b/tests/Feature/app/Http/Controllers/DoublerRequestControllerTest.php index 14aa964..a77c593 100644 --- a/tests/Feature/app/Http/Controllers/DoublerRequestControllerTest.php +++ b/tests/Feature/app/Http/Controllers/DoublerRequestControllerTest.php @@ -4,7 +4,6 @@ use App\Actions\Entries\CreateEntry; use App\Models\Audition; -use App\Models\AuditLogEntry; use App\Models\DoublerRequest; use App\Models\Event; use App\Models\School; @@ -85,8 +84,5 @@ describe('makeRequest', function () { ->and(DoublerRequest::where('event_id', $event2->id)->where('student_id', $student->id)->first()->request)->toBe('student 1 request in event 2'); - foreach (AuditLogEntry::all() as $logEntry) { - dump($logEntry->message); - } }); }); diff --git a/tests/Feature/app/Observers/SchoolEmailDomainObserverTest.php b/tests/Feature/app/Observers/SchoolEmailDomainObserverTest.php index d4b64e0..bf6d62f 100644 --- a/tests/Feature/app/Observers/SchoolEmailDomainObserverTest.php +++ b/tests/Feature/app/Observers/SchoolEmailDomainObserverTest.php @@ -13,7 +13,7 @@ it('logs when a school email domain is created', function () { 'domain' => 'test.com', 'school_id' => $school->id, ]); - $lastLog = AuditLogEntry::latest()->first(); + $lastLog = AuditLogEntry::orderBy('id', 'desc')->first(); expect($lastLog->message)->toEqual('Added the email domain test.com to school '.$school->name); }); diff --git a/tests/Feature/app/helpersTest.php b/tests/Feature/app/helpersTest.php index 280296f..5bca08a 100644 --- a/tests/Feature/app/helpersTest.php +++ b/tests/Feature/app/helpersTest.php @@ -19,7 +19,7 @@ it('can retrieve an audition setting', function () { it('can create an AuditLog entry', function () { actAsAdmin(); auditionLog('This is the message', ['users' => [1, 2, 3], 'entries' => [4, 5, 6]]); - $logEntry = AuditLogEntry::first(); + $logEntry = AuditLogEntry::orderBy('id', 'desc')->first(); expect($logEntry->user)->toBe(auth()->user()->email) ->and($logEntry->message)->toBe('This is the message') ->and($logEntry->affected)->toEqual(['users' => [1, 2, 3], 'entries' => [4, 5, 6]]);