From f3b2372682ed3d629b20a480013dbcc15bddb5a6 Mon Sep 17 00:00:00 2001 From: Matt Young Date: Sat, 5 Jul 2025 01:11:26 -0500 Subject: [PATCH] School controller refactoring and testing. --- app/Http/Controllers/SchoolController.php | 90 +++------ app/Http/Requests/SchoolStoreRequest.php | 32 +++ app/Policies/SchoolPolicy.php | 4 +- .../Http/Controllers/SchoolControllerTest.php | 190 ++++++++++++++++++ 4 files changed, 248 insertions(+), 68 deletions(-) create mode 100644 app/Http/Requests/SchoolStoreRequest.php create mode 100644 tests/Feature/app/Http/Controllers/SchoolControllerTest.php diff --git a/app/Http/Controllers/SchoolController.php b/app/Http/Controllers/SchoolController.php index 3ea222a..070dada 100644 --- a/app/Http/Controllers/SchoolController.php +++ b/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'], - $validData['address'], - $validData['city'], - $validData['state'], - $validData['zip'], + $request['name'], + $request['address'], + $request['city'], + $request['state'], + $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('/schools/'.$school->id); + $headSetter->setHeadDirector(auth()->user()); + + return redirect(route('schools.show', $school)); } - 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( - Request $request - ) { + public function create(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( - Request $request, - School $school - ) { + public function edit(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( - 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'], - ]); - + public function update(SchoolStoreRequest $request, School $school) + { $school->update([ - 'name' => request('name'), - 'address' => request('address'), - 'city' => request('city'), - 'state' => request('state'), - 'zip' => request('zip'), + 'name' => $request['name'], + 'address' => $request['address'], + 'city' => $request['city'], + 'state' => $request['state'], + 'zip' => $request['zip'], ]); $message = 'Modified school #'.$school->id.' - '.$school->name.' with address
'.$school->address.'
'.$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'); diff --git a/app/Http/Requests/SchoolStoreRequest.php b/app/Http/Requests/SchoolStoreRequest.php new file mode 100644 index 0000000..e0dd2ea --- /dev/null +++ b/app/Http/Requests/SchoolStoreRequest.php @@ -0,0 +1,32 @@ + ['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; + } +} diff --git a/app/Policies/SchoolPolicy.php b/app/Policies/SchoolPolicy.php index e227f74..208bc48 100644 --- a/app/Policies/SchoolPolicy.php +++ b/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; } /** diff --git a/tests/Feature/app/Http/Controllers/SchoolControllerTest.php b/tests/Feature/app/Http/Controllers/SchoolControllerTest.php new file mode 100644 index 0000000..e9beba0 --- /dev/null +++ b/tests/Feature/app/Http/Controllers/SchoolControllerTest.php @@ -0,0 +1,190 @@ +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' => '', + '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(); + }); + +});