Error resolution

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']
-            .'<br>Name: '.$input['first_name'].' '.$input['last_name']
-            .'<br>Judging Pref: '.$input['judging_preference']
-            .'<br>Cell Phone: '.$input['cell_phone'];
-        AuditLogEntry::create([
-            'user' => $input['email'],
-            'ip_address' => request()->ip(),
-            'message' => $message,
-            'affected' => ['users' => [$user->id]],
-        ]);
-
         return $user;
     }
 }

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]],
-        ]);
     }
 }

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
-                .'<br>Name: '.$user->full_name()
-                .'<br>Judging Pref: '.$user->judging_preference
-                .'<br>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
-            .'<br>Name: '.$user->full_name()
-            .'<br>Email: '.$user->email
-            .'<br>Judging Pref: '.$user->judging_preference
-            .'<br>Cell Phone: '.$user->cell_phone;
-        AuditLogEntry::create([
-            'user' => auth()->user()->email,
-            'ip_address' => request()->ip(),
-            'message' => $message,
-            'affected' => ['users' => [$user->id]],
-        ]);
 
         $user->sendEmailVerificationNotification();
     }

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,
+        ]);
     }
 }

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 .= '<br>School: '.$user->school->name;
         }
 
-        if ($user->isDirty('school_id') && ! $user->school_id) {
-            $message .= '<br>School: NONE';
-        }
-
         if ($user->isDirty('email')) {
             $message .= '<br>Email: '.$user->email;
         }

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]);
 });

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();
+    $logEntry = \App\Models\AuditLogEntry::orderBy('id', 'desc')->first();
-    expect($logEntry->message)->toEqual('Changed password for '.$this->user->email.' ('.$this->user->full_name().')');
+    expect($logEntry->message)->toEqual('Changed password for User '.$this->user->full_name().' <'.$this->user->email.'>');
+
 });

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);
-});

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()
+        ->where('domain', $this->domain)->exists())->toBeTrue();
-        ->and(AuditLogEntry::count())->toEqual(1);
 });

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();
+    $logEntry = AuditLogEntry::orderBy('id', 'desc')->first();
-    expect($logEntry->message)->toEqual('Assigned '.$this->user->full_name().' to school '.$this->school->name);
+    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();
+    $logEntry = AuditLogEntry::orderBy('id', 'desc')->first();
-    expect($logEntry->message)->toEqual('Moved '.$this->user->full_name().' from school '.$this->school->name.' to school '.$newSchool->name);
+    expect($logEntry->message)->toEqual('Changed school for '.$this->user->full_name().' from '.$this->school->name.' to '.$newSchool->name);
 });

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);

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);
 });

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);
-        }
     });
 });

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);
 });
 

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]]);