School controller refactoring and testing.

app/Http/Controllers/SchoolController.php

@@ -6,6 +6,7 @@ use App\Actions\Schools\AssignUserToSchool;
 use App\Actions\Schools\CreateSchool;
 use App\Actions\Schools\SetHeadDirector;
 use App\Exceptions\AuditionAdminException;
+use App\Http\Requests\SchoolStoreRequest;
 use App\Mail\NewUserPassword;
 use App\Models\AuditLogEntry;
 use App\Models\School;
@@ -14,7 +15,6 @@ use App\Models\User;
 use Illuminate\Database\UniqueConstraintViolationException;
 use Illuminate\Http\RedirectResponse;
 use Illuminate\Http\Request;
-use Illuminate\Support\Facades\Auth;
 use Illuminate\Support\Facades\Hash;
 use Illuminate\Support\Facades\Mail;
 use Illuminate\Support\Str;
@@ -26,46 +26,30 @@ use function request;
 
 class SchoolController extends Controller
 {
-    public function store(Request $request, SetHeadDirector $headSetter): RedirectResponse
+    public function store(SchoolStoreRequest $request, SetHeadDirector $headSetter): RedirectResponse
     {
-        if ($request->user()->cannot('create', School::class)) {
-            abort(403);
-        }
-        $validData = request()->validate([
-            'name' => ['required', 'min:3', 'max:30', 'unique:schools,name'],
-            'address' => ['required'],
-            'city' => ['required'],
-            'state' => ['required', 'min:2', 'max:2'],
-            'zip' => ['required', 'min:5', 'max:10'],
-        ]);
-
         $creator = app(CreateSchool::class);
 
         $school = $creator(
-            $validData['name'],
+            $request['name'],
-            $validData['address'],
+            $request['address'],
-            $validData['city'],
+            $request['city'],
-            $validData['state'],
+            $request['state'],
-            $validData['zip'],
+            $request['zip'],
         );
 
         $assigner = app(AssignUserToSchool::class);
         $assigner(auth()->user(), $school);
 
         auth()->user()->refresh();
-        try {
+
         $headSetter->setHeadDirector(auth()->user());
-        } catch (AuditionAdminException $e) {
+
-            redirect(route('schools.show', $school))->with('error', 'Could not set as head director');
+        return redirect(route('schools.show', $school));
     }
 
-        return redirect('/schools/'.$school->id);
+    public function show(Request $request, School $school)
-    }
+    {
-
-    public function show(
-        Request $request,
-        School $school
-    ) {
         if ($request->user()->cannot('view', $school)) {
             abort(403);
         }
@@ -73,9 +57,8 @@ class SchoolController extends Controller
         return view('schools.show', ['school' => $school]);
     }
 
-    public function create(
+    public function create(Request $request)
-        Request $request
+    {
-    ) {
         if ($request->user()->cannot('create', School::class)) {
             abort(403);
         }
@@ -83,10 +66,8 @@ class SchoolController extends Controller
         return view('schools.create');
     }
 
-    public function edit(
+    public function edit(Request $request, School $school)
-        Request $request,
+    {
-        School $school
-    ) {
         if ($request->user()->cannot('update', $school)) {
             abort(403);
         }
@@ -94,27 +75,14 @@ class SchoolController extends Controller
         return view('schools.edit', ['school' => $school]);
     }
 
-    public function update(
+    public function update(SchoolStoreRequest $request, School $school)
-        Request $request,
+    {
-        School $school
-    ) {
-        if ($request->user()->cannot('update', $school)) {
-            abort(403);
-        }
-        request()->validate([
-            'name' => ['required', 'min:3', 'max:30'],
-            'address' => ['required'],
-            'city' => ['required'],
-            'state' => ['required', 'min:2', 'max:2'],
-            'zip' => ['required', 'min:5', 'max:10'],
-        ]);
-
         $school->update([
-            'name' => request('name'),
+            'name' => $request['name'],
-            'address' => request('address'),
+            'address' => $request['address'],
-            'city' => request('city'),
+            'city' => $request['city'],
-            'state' => request('state'),
+            'state' => $request['state'],
-            'zip' => request('zip'),
+            'zip' => $request['zip'],
         ]);
         $message = 'Modified school #'.$school->id.' - '.$school->name.' with address <br>'.$school->address.'<br>'.$school->city.', '.$school->state.' '.$school->zip;
         AuditLogEntry::create([
@@ -127,18 +95,8 @@ class SchoolController extends Controller
         return redirect()->route('schools.show', $school->id)->with('success', 'School details updated');
     }
 
-    public function my_school()
+    public function addDirector(School $school)
     {
-        if (Auth::user()->school) {
-            return redirect('/schools/'.Auth::user()->school->id);
-        }
-
-        return redirect('/schools/create');
-    }
-
-    public function addDirector(
-        School $school
-    ) {
 
         if (auth()->user()->school_id !== $school->id) {
             return redirect()->back()->with('error', 'No adding directors to another school');

app/Http/Requests/SchoolStoreRequest.php

@@ -0,0 +1,32 @@
+<?php
+
+namespace App\Http\Requests;
+
+use App\Models\School;
+use Illuminate\Foundation\Http\FormRequest;
+
+class SchoolStoreRequest extends FormRequest
+{
+    public function rules(): array
+    {
+        return [
+            'name' => ['required', 'min:3', 'max:30', 'unique:schools,name'],
+            'address' => ['required'],
+            'city' => ['required'],
+            'state' => ['required', 'min:2', 'max:2'],
+            'zip' => ['required', 'min:5', 'max:10'],
+        ];
+    }
+
+    public function authorize(): bool
+    {
+        if ($this->isMethod('post')) {
+            return $this->user()->can('create', School::class);
+        }
+        if ($this->isMethod('patch')) {
+            return $this->user()->can('update', $this->route('school'));
+        }
+
+        return false;
+    }
+}

app/Policies/SchoolPolicy.php

@@ -34,7 +34,7 @@ class SchoolPolicy
      */
     public function view(User $user, School $school): bool
     {
-        return $school->id == $user->school_id;
+        return $school->id === $user->school_id;
     }
 
     /**
@@ -50,7 +50,7 @@ class SchoolPolicy
      */
     public function update(User $user, School $school): bool
     {
-        return $school->id == $user->school_id;
+        return $school->id === $user->school_id;
     }
 
     /**

tests/Feature/app/Http/Controllers/SchoolControllerTest.php

@@ -0,0 +1,190 @@
+<?php
+
+use App\Models\School;
+use App\Models\User;
+use Illuminate\Foundation\Testing\RefreshDatabase;
+
+use function Pest\Laravel\actingAs;
+
+uses(RefreshDatabase::class);
+
+describe('store method', function () {
+    it('will not allow a user to create a school if they already have one', function () {
+        $user = User::factory()->create();
+        $school = School::factory()->create();
+        $user->school_id = $school->id;
+        $user->save();
+        actingAs($user);
+        $response = $this->post(route('schools.store'), [
+            'name' => 'Test School',
+            'address' => '123 Test St',
+            'city' => 'Testville',
+            'state' => 'TX',
+            'zip' => '12345',
+            'phone' => '123-456-7890',
+            'email' => '<EMAIL>',
+            'website' => 'https://test.com',
+        ]);
+        $response->assertStatus(403);
+    });
+
+    it('will allow a user with no schools to create one', function () {
+        $user = User::factory()->create();
+        actingAs($user);
+        $response = $this->post(route('schools.store'), [
+            'name' => 'Test School',
+            'address' => '123 Test St',
+            'city' => 'Testville',
+            'state' => 'TX',
+            'zip' => '12345',
+        ]);
+        $school = School::first();
+        expect($school->name)->toEqual('Test School')
+            ->and($school->address)->toEqual('123 Test St')
+            ->and($school->city)->toEqual('Testville')
+            ->and($school->state)->toEqual('TX')
+            ->and($school->zip)->toEqual('12345');
+    });
+
+    it('will redirect on success', function () {
+        $user = User::factory()->create();
+        actingAs($user);
+        $response = $this->post(route('schools.store'), [
+            'name' => 'Test School',
+            'address' => '123 Test St',
+            'city' => 'Testville',
+            'state' => 'TX',
+            'zip' => '12345',
+        ]);
+        $response->assertRedirect(route('schools.show', $user->school_id));
+    });
+
+    it('will assign the user to the new school', function () {
+        $user = User::factory()->create();
+        actingAs($user);
+        $response = $this->post(route('schools.store'), [
+            'name' => 'Test School',
+            'address' => '123 Test St',
+            'city' => 'Testville',
+            'state' => 'TX',
+            'zip' => '12345',
+        ]);
+        $user->refresh();
+        expect($user->school_id)->toEqual(School::first()->id);
+    });
+
+    it('will make the user head director', function () {
+        $user = User::factory()->create();
+        actingAs($user);
+        $response = $this->post(route('schools.store'), [
+            'name' => 'Test School',
+            'address' => '123 Test St',
+            'city' => 'Testville',
+            'state' => 'TX',
+            'zip' => '12345',
+        ]);
+        $user->refresh();
+        expect($user->hasFlag('head_director'))->toBeTrue();
+    });
+
+});
+
+describe('show method', function () {
+    it('will not allow the user to view a school they are not a member of', function () {
+        $user = User::factory()->create();
+        $school = School::factory()->create();
+        $user->school_id = $school->id;
+        $user->save();
+        actingAs($user);
+        $otherSchool = School::factory()->create();
+        $response = $this->get(route('schools.show', $otherSchool->id));
+        $response->assertStatus(403);
+    });
+
+    it('shows the view page for the users school', function () {
+        $user = User::factory()->create();
+        $school = School::factory()->create();
+        $user->school_id = $school->id;
+        $user->save();
+        actingAs($user);
+        $response = $this->get(route('schools.show', $school->id));
+        $response->assertStatus(200);
+    });
+});
+
+describe('create method', function () {
+    it('will not allow a user to create a school if they already have one', function () {
+        $user = User::factory()->create();
+        $school = School::factory()->create();
+        $user->school_id = $school->id;
+        $user->save();
+        actingAs($user);
+        $response = $this->get(route('schools.create'));
+        $response->assertStatus(403);
+    });
+
+    it('shows a school creation form', function () {
+        $user = User::factory()->create();
+        actingAs($user);
+        $response = $this->get(route('schools.create'));
+        $response->assertStatus(200)
+            ->assertViewIs('schools.create');
+    });
+});
+
+describe('edit method', function () {
+    it('will not allow a user to edit a school that is not theirs', function () {
+        $user = User::factory()->create();
+        $school = School::factory()->create();
+        $user->school_id = $school->id;
+        $user->save();
+        actingAs($user);
+        $otherSchool = School::factory()->create();
+        $response = $this->get(route('schools.edit', $otherSchool));
+        $response->assertStatus(403);
+    });
+
+    it('shows a school edit form for the users school', function () {
+        $user = User::factory()->create();
+        $school = School::factory()->create();
+        $user->school_id = $school->id;
+        $user->save();
+        actingAs($user);
+        $response = $this->get(route('schools.edit', $school));
+        $response->assertStatus(200)
+            ->assertViewIs('schools.edit');
+    });
+});
+
+describe('update method', function () {
+    it('will not allow a user to update a school that is not theirs', function () {
+        $user = User::factory()->create();
+        $school = School::factory()->create();
+        $user->school_id = $school->id;
+        $user->save();
+        actingAs($user);
+        $otherSchool = School::factory()->create();
+        $response = $this->patch(route('schools.update', $otherSchool));
+        $response->assertStatus(403);
+    });
+
+    it('will update the users school', function () {
+        $user = User::factory()->create();
+        $school = School::factory()->create();
+        $user->school_id = $school->id;
+        $user->save();
+        actingAs($user);
+        expect($school->name === 'Eastman')->toBeFalse();
+        $response = $this->patch(route('schools.update', $school), [
+            'name' => 'Eastman',
+            'address' => '26 Gibbs Street',
+            'city' => 'Rochester',
+            'state' => 'NY',
+            'zip' => '14604',
+        ]);
+        $response->assertSessionHasNoErrors()
+            ->assertRedirect(route('schools.show', $school));
+        expect($school->name === 'Eastman')->toBeFalse();
+    });
+
+});