From 9717ae852e4005c5e3fa5d43761ea61f90163971 Mon Sep 17 00:00:00 2001 From: Matt Young Date: Fri, 4 Jul 2025 17:20:49 -0500 Subject: [PATCH] Work on refactoring student controller and test --- app/Http/Controllers/StudentController.php | 46 ++------------- app/Http/Requests/StudentStoreRequest.php | 39 +++++++++++++ app/Observers/StudentObserver.php | 58 +++++++++++++++++++ app/Providers/AppServiceProvider.php | 3 + database/factories/StudentFactory.php | 9 +++ routes/user.php | 6 +- tests/Feature/PagesResponseTest.php | 2 +- .../app/Actions/Entries/CreateEntriesTest.php | 2 +- .../app/Actions/Entries/UpdateEntryTest.php | 2 +- .../Actions/Tabulation/EnterNoShowTest.php | 5 +- .../app/Actions/Tabulation/EnterScoreTest.php | 3 +- .../Controllers/StudentControllerTest.php | 52 +++++++++++++++++ 12 files changed, 178 insertions(+), 49 deletions(-) create mode 100644 app/Http/Requests/StudentStoreRequest.php create mode 100644 app/Observers/StudentObserver.php create mode 100644 tests/Feature/app/Http/Controllers/StudentControllerTest.php diff --git a/app/Http/Controllers/StudentController.php b/app/Http/Controllers/StudentController.php index 52628ae..25b4a37 100644 --- a/app/Http/Controllers/StudentController.php +++ b/app/Http/Controllers/StudentController.php @@ -2,10 +2,10 @@ namespace App\Http\Controllers; +use App\Http\Requests\StudentStoreRequest; use App\Models\Audition; use App\Models\AuditLogEntry; use App\Models\Student; -use App\Rules\UniqueFullNameAtSchool; use Illuminate\Http\Request; use Illuminate\Support\Facades\Auth; @@ -31,58 +31,24 @@ class StudentController extends Controller ['students' => $students, 'auditions' => $auditions, 'shirtSizes' => $shirtSizes]); } - /** - * Show the form for creating a new resource. - */ - public function create() - { - // - } - /** * Store a newly created resource in storage. */ - public function store(Request $request) + public function store(StudentStoreRequest $request) { if ($request->user()->cannot('create', Student::class)) { abort(403); } - $request->validate([ - 'first_name' => ['required'], - 'last_name' => [ - 'required', - new UniqueFullNameAtSchool(request('first_name'), request('last_name'), Auth::user()->school_id), - ], - 'grade' => ['required', 'integer'], - 'shirt_size' => [ - 'nullable', - function ($attribute, $value, $fail) { - if (! array_key_exists($value, Student::$shirtSizes)) { - $fail("The selected $attribute is invalid."); - } - }, - ], - ]); $student = Student::create([ - 'first_name' => request('first_name'), - 'last_name' => request('last_name'), - 'grade' => request('grade'), + 'first_name' => $request['first_name'], + 'last_name' => $request['last_name'], + 'grade' => $request['grade'], 'school_id' => Auth::user()->school_id, ]); - if (request('shirt_size') !== 'none') { + if ($request['shirt_size'] !== 'none') { $student->update(['optional_data->shirt_size' => $request['shirt_size']]); } - $message = 'Created student #'.$student->id.' - '.$student->full_name().'
Grade: '.$student->grade.'
School: '.$student->school->name; - AuditLogEntry::create([ - 'user' => auth()->user()->email, - 'ip_address' => request()->ip(), - 'message' => $message, - 'affected' => [ - 'students' => [$student->id], - 'schools' => [$student->school_id], - ], - ]); return redirect('/students')->with('success', 'Student Created'); } diff --git a/app/Http/Requests/StudentStoreRequest.php b/app/Http/Requests/StudentStoreRequest.php new file mode 100644 index 0000000..ab6dfff --- /dev/null +++ b/app/Http/Requests/StudentStoreRequest.php @@ -0,0 +1,39 @@ + ['required'], + 'last_name' => [ + 'required', + new UniqueFullNameAtSchool(request('first_name'), request('last_name'), Auth::user()->school_id), + ], + 'grade' => ['required', 'integer'], + 'shirt_size' => [ + 'nullable', + function ($attribute, $value, $fail) { + if (! array_key_exists($value, Student::$shirtSizes)) { + $fail("The selected $attribute is invalid."); + } + }, + ], + ]; + } + + public function authorize(): bool + { + return true; + } +} diff --git a/app/Observers/StudentObserver.php b/app/Observers/StudentObserver.php new file mode 100644 index 0000000..a5714c3 --- /dev/null +++ b/app/Observers/StudentObserver.php @@ -0,0 +1,58 @@ +id.' - '.$student->full_name().'
Grade: '.$student->grade.'
School: '.$student->school->name; + AuditLogEntry::create([ + 'user' => auth()->user()->email ?? 'none', + 'ip_address' => request()->ip(), + 'message' => $message, + 'affected' => [ + 'students' => [$student->id], + 'schools' => [$student->school_id], + ], + ]); + } + + /** + * Handle the Student "updated" event. + */ + public function updated(Student $student): void + { + // + } + + /** + * Handle the Student "deleted" event. + */ + public function deleted(Student $student): void + { + // + } + + /** + * Handle the Student "restored" event. + */ + public function restored(Student $student): void + { + // + } + + /** + * Handle the Student "force deleted" event. + */ + public function forceDeleted(Student $student): void + { + // + } +} diff --git a/app/Providers/AppServiceProvider.php b/app/Providers/AppServiceProvider.php index 4c49a28..550cb32 100644 --- a/app/Providers/AppServiceProvider.php +++ b/app/Providers/AppServiceProvider.php @@ -12,11 +12,13 @@ use App\Models\Entry; use App\Models\EntryFlag; use App\Models\SchoolEmailDomain; use App\Models\ScoreSheet; +use App\Models\Student; use App\Observers\BonusScoreObserver; use App\Observers\EntryFlagObserver; use App\Observers\EntryObserver; use App\Observers\SchoolEmailDomainObserver; use App\Observers\ScoreSheetObserver; +use App\Observers\StudentObserver; use App\Services\AuditionService; use App\Services\DoublerService; use App\Services\DrawService; @@ -55,6 +57,7 @@ class AppServiceProvider extends ServiceProvider Entry::observe(EntryObserver::class); SchoolEmailDomain::observe(SchoolEmailDomainObserver::class); ScoreSheet::observe(ScoreSheetObserver::class); + Student::observe(StudentObserver::class); EntryFlag::observe(EntryFlagObserver::class); // Model::preventLazyLoading(! app()->isProduction()); diff --git a/database/factories/StudentFactory.php b/database/factories/StudentFactory.php index 4aa2410..dbe51a3 100644 --- a/database/factories/StudentFactory.php +++ b/database/factories/StudentFactory.php @@ -26,4 +26,13 @@ class StudentFactory extends Factory 'grade' => rand(7, 12), ]; } + + public function forSchool(School $school) + { + return $this->state(function (array $attributes) use ($school) { + return [ + 'school_id' => $school->id, + ]; + }); + } } diff --git a/routes/user.php b/routes/user.php index abe392a..cdfcfcd 100644 --- a/routes/user.php +++ b/routes/user.php @@ -20,7 +20,7 @@ Route::middleware(['auth', 'verified'])->group(function () { // Entry Related Routes Route::middleware([ - 'auth', 'verified', 'can:create,App\Models\Entry', + 'auth', 'verified', ])->controller(EntryController::class)->group(function () { Route::get('/entries', 'index')->name('entries.index'); Route::get('/entries/create', 'create')->name('entries.create'); @@ -37,7 +37,7 @@ Route::middleware([ // Student Related Routes Route::middleware([ - 'auth', 'verified', 'can:create,App\Models\Student', + 'auth', 'verified', ])->controller(StudentController::class)->group(function () { Route::get('/students', 'index')->name('students.index'); Route::post('students', 'store')->name('students.store'); @@ -61,7 +61,7 @@ Route::middleware(['auth', 'verified'])->controller(SchoolController::class)->gr // Doubler Related Routes Route::middleware([ - 'auth', 'verified', 'can:viewAny,App\Models\DoublerRequest', + 'auth', 'verified', ])->controller(DoublerRequestController::class)->prefix('doubler_request')->group(function () { Route::get('/', 'index')->name('doubler_request.index'); Route::post('/', 'makeRequest')->name('doubler_request.make_request'); diff --git a/tests/Feature/PagesResponseTest.php b/tests/Feature/PagesResponseTest.php index 424a225..8a92236 100644 --- a/tests/Feature/PagesResponseTest.php +++ b/tests/Feature/PagesResponseTest.php @@ -68,7 +68,7 @@ it('shows students index only for a user with a school', function () { $user = User::factory()->create(); $this->actingAs($user); get(route('students.index')) - ->assertStatus(403); + ->assertRedirect(route('dashboard')); $school = School::factory()->create(); $user->school_id = $school->id; diff --git a/tests/Feature/app/Actions/Entries/CreateEntriesTest.php b/tests/Feature/app/Actions/Entries/CreateEntriesTest.php index 1eb0ea5..47ce7b9 100644 --- a/tests/Feature/app/Actions/Entries/CreateEntriesTest.php +++ b/tests/Feature/app/Actions/Entries/CreateEntriesTest.php @@ -146,7 +146,7 @@ it('logs the entry creation', function () { $audition = Audition::factory()->create(['minimum_grade' => 9, 'maximum_grade' => 12]); $this->scribe->createEntry($student, $audition); $thisEntry = Entry::where('student_id', $student->id)->first(); - $logEntry = AuditLogEntry::first(); + $logEntry = AuditLogEntry::orderBy('id', 'desc')->first(); expect($logEntry->message)->toEqual('Entered '.$thisEntry->student->full_name().' from '.$thisEntry->student->school->name.' in '.$audition->name.'.') ->and($logEntry->affected['entries'])->toEqual([$thisEntry->id]) ->and($logEntry->affected['students'])->toEqual([$thisEntry->student_id]) diff --git a/tests/Feature/app/Actions/Entries/UpdateEntryTest.php b/tests/Feature/app/Actions/Entries/UpdateEntryTest.php index 08fd6e3..b314387 100644 --- a/tests/Feature/app/Actions/Entries/UpdateEntryTest.php +++ b/tests/Feature/app/Actions/Entries/UpdateEntryTest.php @@ -203,7 +203,7 @@ it('logs changes', function () { $originalEntry = Entry::find($this->entry->id); $newAudition = Audition::factory()->create(['minimum_grade' => 9, 'maximum_grade' => 10, 'name' => 'Alphorn']); ($this->entryScribe)($this->entry, ['audition' => $newAudition]); - $logEntry = AuditLogEntry::latest()->first(); + $logEntry = AuditLogEntry::orderBy('id', 'desc')->first(); expect($logEntry->affected['auditions'])->toEqual([$originalEntry->audition_id, $newAudition->id]) ->and($logEntry->affected['entries'])->toEqual([$this->entry->id]) ->and($logEntry->affected['students'])->toEqual([$this->entry->student_id]) diff --git a/tests/Feature/app/Actions/Tabulation/EnterNoShowTest.php b/tests/Feature/app/Actions/Tabulation/EnterNoShowTest.php index db4dc3d..b979f80 100644 --- a/tests/Feature/app/Actions/Tabulation/EnterNoShowTest.php +++ b/tests/Feature/app/Actions/Tabulation/EnterNoShowTest.php @@ -6,6 +6,7 @@ use App\Actions\Tabulation\EnterNoShow; use App\Actions\Tabulation\EnterScore; use App\Exceptions\AuditionAdminException; use App\Models\Audition; +use App\Models\AuditLogEntry; use App\Models\Entry; use App\Models\SubscoreDefinition; use App\Models\User; @@ -29,7 +30,7 @@ it('can enter a no-show', function () { it('creates a log entry when entering a no-show', function () { ($this->rollTaker)($this->entry); - $logEntry = \App\Models\AuditLogEntry::latest()->first(); + $logEntry = AuditLogEntry::orderBy('id', 'desc')->first(); expect($logEntry->message)->toStartWith('No Show has been entered for'); }); @@ -40,7 +41,7 @@ it('can enter a failed prelim', function () { it('creates a log entry when entering a failed prelim', function () { ($this->rollTaker)($this->entry, 'failprelim'); - $logEntry = \App\Models\AuditLogEntry::latest()->first(); + $logEntry = AuditLogEntry::orderBy('id', 'desc')->first(); expect($logEntry->message)->toStartWith('Failed prelim has been entered for'); }); diff --git a/tests/Feature/app/Actions/Tabulation/EnterScoreTest.php b/tests/Feature/app/Actions/Tabulation/EnterScoreTest.php index 89b5ff6..5dcba3d 100644 --- a/tests/Feature/app/Actions/Tabulation/EnterScoreTest.php +++ b/tests/Feature/app/Actions/Tabulation/EnterScoreTest.php @@ -5,6 +5,7 @@ use App\Actions\Tabulation\EnterScore; use App\Exceptions\AuditionAdminException; use App\Models\Audition; +use App\Models\AuditLogEntry; use App\Models\Entry; use App\Models\ScoreSheet; use App\Models\SubscoreDefinition; @@ -122,6 +123,6 @@ it('removes a no-show flag from an entry', function () { it('logs score entry', function () { ($this->scribe)($this->judge1, $this->entry1, $this->possibleScoreArray); - $logEntry = \App\Models\AuditLogEntry::latest()->first(); + $logEntry = AuditLogEntry::orderBy('id', 'desc')->first(); expect($logEntry->message)->toStartWith('Entered Score for entry id '); }); diff --git a/tests/Feature/app/Http/Controllers/StudentControllerTest.php b/tests/Feature/app/Http/Controllers/StudentControllerTest.php new file mode 100644 index 0000000..a925eb9 --- /dev/null +++ b/tests/Feature/app/Http/Controllers/StudentControllerTest.php @@ -0,0 +1,52 @@ +create(); + $this->actingAs($user); + $response = $this->get(route('students.index')); + $response->assertRedirect(route('dashboard')); + actAsAdmin(); + $response = $this->get(route('students.index')); + $response->assertRedirect(route('dashboard')); + }); + it('redirects to the students index if the user has a school', function () { + $user = User::factory()->create(); + $school = School::factory()->create(); + $user->school_id = $school->id; + $user->save(); + $this->actingAs($user); + $response = $this->get(route('students.index')); + $response->assertOk(); + }); + it('returns the view students.index', function () { + $user = User::factory()->create(); + $school = School::factory()->create(); + $audition = Audition::factory()->create(); + $student = Student::factory()->forSchool($school)->create(); + $user->school_id = $school->id; + $user->save(); + $this->actingAs($user); + $response = $this->get(route('students.index')); + $response->assertViewIs('students.index') + ->assertViewHas('students', function ($students) use ($student) { + return $students->contains($student); + }) + ->assertViewHas('auditions', function ($auditions) use ($audition) { + return $auditions->contains($audition); + }) + ->assertSee(route('students.store')); + }); +}); + +describe('Test the store method of the student controller', function () { + +});