diff --git a/app/Http/Controllers/Admin/StudentController.php b/app/Http/Controllers/Admin/StudentController.php index 88e0c15..c9d31b9 100644 --- a/app/Http/Controllers/Admin/StudentController.php +++ b/app/Http/Controllers/Admin/StudentController.php @@ -2,7 +2,9 @@ namespace App\Http\Controllers\Admin; +use App\Actions\Students\CreateStudent; use App\Http\Controllers\Controller; +use App\Http\Requests\StudentStoreRequest; use App\Models\Audition; use App\Models\AuditLogEntry; use App\Models\Entry; @@ -25,9 +27,6 @@ class StudentController extends Controller { public function index() { - if (! Auth::user()->is_admin) { - abort(403); - } $filters = session('adminStudentFilters') ?? null; $schools = School::orderBy('name')->get(); $students = Student::with(['school'])->withCount('entries')->orderBy('last_name')->orderBy('first_name'); @@ -54,62 +53,32 @@ class StudentController extends Controller public function create() { - if (! Auth::user()->is_admin) { - abort(403); - } - $minGrade = min(Audition::min('minimum_grade'), NominationEnsemble::min('minimum_grade')); - $maxGrade = max(Audition::max('maximum_grade'), NominationEnsemble::max('maximum_grade')); + $minGrade = $this->minimumGrade(); + $maxGrade = $this->maximumGrade(); + $schools = School::orderBy('name')->get(); return view('admin.students.create', ['schools' => $schools, 'minGrade' => $minGrade, 'maxGrade' => $maxGrade]); } - public function store() + public function store(StudentStoreRequest $request, CreateStudent $creator) { - if (! Auth::user()->is_admin) { - abort(403); - } - request()->validate([ - 'first_name' => ['required'], - 'last_name' => ['required'], - 'grade' => ['required', 'integer'], - 'school_id' => ['required', 'exists:schools,id'], + /** @noinspection PhpUnhandledExceptionInspection */ + $creator([ + 'first_name' => $request['first_name'], + 'last_name' => $request['last_name'], + 'grade' => $request['grade'], + 'school_id' => $request['school_id'], + 'optional_data' => $request->optional_data, ]); - if (Student::where('first_name', request('first_name')) - ->where('last_name', request('last_name')) - ->where('school_id', request('school_id')) - ->exists()) { - return redirect('/admin/students/create')->with('error', 'This student already exists.'); - } - - $student = Student::create([ - 'first_name' => request('first_name'), - 'last_name' => request('last_name'), - 'grade' => request('grade'), - 'school_id' => request('school_id'), - ]); - $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('/admin/students')->with('success', 'Created student successfully'); + return redirect(route('admin.students.index'))->with('success', 'Student created successfully'); } public function edit(Student $student) { - if (! Auth::user()->is_admin) { - abort(403); - } - $minGrade = min(Audition::min('minimum_grade'), NominationEnsemble::min('minimum_grade')); - $maxGrade = max(Audition::max('maximum_grade'), NominationEnsemble::max('maximum_grade')); + $minGrade = $this->minimumGrade(); + $maxGrade = $this->maximumGrade(); $schools = School::orderBy('name')->get(); $student->loadCount('entries'); $entries = $student->entries()->with('audition.flags')->get(); @@ -141,7 +110,7 @@ class StudentController extends Controller return true; } - // Can't change decision if this is the only entry with results not published + // Can't change the decision if this is the only entry with results not published $unpublished = $entries->reject(function ($entry) { return $entry->audition->hasFlag('seats_published'); }); @@ -230,8 +199,33 @@ class StudentController extends Controller return to_route('admin.students.index')->with('success', 'Student '.$name.' deleted successfully.'); } - public function set_filter() + private function minimumGrade(): int { - // + $nomMin = NominationEnsemble::min('minimum_grade'); + $normMin = Audition::min('minimum_grade'); + + if (is_null($nomMin)) { + $minGrade = $normMin; + } else { + $minGrade = min($nomMin, $normMin); + } + + return $minGrade; + + } + + private function maximumGrade(): int + { + $nomMax = NominationEnsemble::max('maximum_grade'); + $normMax = Audition::max('maximum_grade'); + + if (is_null($nomMax)) { + $maxGrade = $normMax; + } else { + $maxGrade = max($nomMax, $normMax); + } + + return $maxGrade; + } } diff --git a/app/Http/Requests/StudentStoreRequest.php b/app/Http/Requests/StudentStoreRequest.php index 874e86c..b66d64d 100644 --- a/app/Http/Requests/StudentStoreRequest.php +++ b/app/Http/Requests/StudentStoreRequest.php @@ -14,13 +14,21 @@ class StudentStoreRequest extends FormRequest { public function rules(): array { + $schoolId = $this->input('school_id', Auth::user()->school_id); + + // If the user is not an admin, force their school_id to be used + if (! Auth::user()->is_admin) { + $schoolId = Auth::user()->school_id; + } + return [ 'first_name' => ['required'], 'last_name' => [ 'required', - new UniqueFullNameAtSchool(request('first_name'), request('last_name'), Auth::user()->school_id), + new UniqueFullNameAtSchool(request('first_name'), request('last_name'), $schoolId), ], 'grade' => ['required', 'integer'], + 'school_id' => ['sometimes', 'exists:schools,id'], // Only validates if present 'shirt_size' => [ 'nullable', function ($attribute, $value, $fail) { diff --git a/app/Rules/UniqueFullNameAtSchool.php b/app/Rules/UniqueFullNameAtSchool.php index 9c34dbd..d97964e 100644 --- a/app/Rules/UniqueFullNameAtSchool.php +++ b/app/Rules/UniqueFullNameAtSchool.php @@ -32,7 +32,7 @@ class UniqueFullNameAtSchool implements ValidationRule public function message(): string { - return 'There is already a student with that name at the school you are trying to add them to'; + return 'There is already a student with that name at that school.'; } /** diff --git a/storage/debug.html b/storage/debug.html new file mode 100644 index 0000000..819aa6c --- /dev/null +++ b/storage/debug.html @@ -0,0 +1,12 @@ + + + + + + + Redirecting to http://auditionadmin.test/admin/students + + + Redirecting to http://auditionadmin.test/admin/students. + + \ No newline at end of file diff --git a/tests/Feature/app/Http/Controllers/Admin/StudentControllerTest.php b/tests/Feature/app/Http/Controllers/Admin/StudentControllerTest.php new file mode 100644 index 0000000..a0f2bd7 --- /dev/null +++ b/tests/Feature/app/Http/Controllers/Admin/StudentControllerTest.php @@ -0,0 +1,226 @@ +get(route('admin.students.index'))->assertRedirect(route('home')); + actAsNormal(); + $this->get(route('admin.students.index'))->assertRedirect(route('dashboard')); + actAsTab(); + $this->get(route('admin.students.index'))->assertRedirect(route('dashboard')); + }); + it('it shows an index of students', function () { + actAsAdmin(); + $students = Student::factory()->count(3)->create(); + $response = $this->get(route('admin.students.index'))->assertOk(); + $studentToView = $response->viewData('students'); + expect($studentToView)->toHaveCount(3); + + foreach ($students as $student) { + expect(in_array($student->id, $studentToView->pluck('id')->toArray())); + } + $response->assertViewIs( + 'admin.students.index'); + }); + it('can filter by first name', function () { + $names = ['name1', 'name2', 'name3']; + foreach ($names as $name) { + $students[] = Student::factory()->create(['first_name' => $name]); + } + actAsAdmin(); + $response = $this->withSession([ + 'adminStudentFilters' => [ + 'first_name' => 'name3', + 'last_name' => '', + 'school' => 'all', + 'grade' => 'all', + ], + ]) + ->get(route('admin.students.index')); + file_put_contents(storage_path('debug.html'), $response->getContent()); + $response->assertOk() + ->assertViewIs('admin.students.index'); + $returnedStudentIds = $response->viewData('students')->pluck('id')->toArray(); + expect($returnedStudentIds)->toHaveCount(1) + ->and(in_array($students[0]->id, $returnedStudentIds))->toBeFalsy() + ->and(in_array($students[1]->id, $returnedStudentIds))->toBeFalsy() + ->and(in_array($students[2]->id, $returnedStudentIds))->toBeTruthy(); + }); + + it('can filter by last name', function () { + $names = ['name1', 'name2', 'name3']; + foreach ($names as $name) { + $students[] = Student::factory()->create(['last_name' => $name]); + } + actAsAdmin(); + $response = $this->withSession([ + 'adminStudentFilters' => [ + 'first_name' => '', + 'last_name' => 'name2', + 'school' => 'all', + 'grade' => 'all', + ], + ]) + ->get(route('admin.students.index')); + file_put_contents(storage_path('debug.html'), $response->getContent()); + $response->assertOk() + ->assertViewIs('admin.students.index'); + $returnedStudentIds = $response->viewData('students')->pluck('id')->toArray(); + expect($returnedStudentIds)->toHaveCount(1) + ->and(in_array($students[0]->id, $returnedStudentIds))->toBeFalsy() + ->and(in_array($students[1]->id, $returnedStudentIds))->toBeTruthy() + ->and(in_array($students[2]->id, $returnedStudentIds))->toBeFalsy(); + }); + + it('can filter by grade', function () { + $grades = [6, 7, 8]; + foreach ($grades as $grade) { + $students[] = Student::factory()->create(['grade' => $grade]); + } + actAsAdmin(); + $response = $this->withSession([ + 'adminStudentFilters' => [ + 'first_name' => '', + 'last_name' => '', + 'school' => 'all', + 'grade' => '8', + ], + ]) + ->get(route('admin.students.index')); + file_put_contents(storage_path('debug.html'), $response->getContent()); + $response->assertOk() + ->assertViewIs('admin.students.index'); + $returnedStudentIds = $response->viewData('students')->pluck('id')->toArray(); + expect($returnedStudentIds)->toHaveCount(1) + ->and(in_array($students[0]->id, $returnedStudentIds))->toBeFalsy() + ->and(in_array($students[1]->id, $returnedStudentIds))->toBeFalsy() + ->and(in_array($students[2]->id, $returnedStudentIds))->toBeTruthy(); + }); + + it('can filter by school', function () { + $schools[1] = School::factory()->create(); + $schools[2] = School::factory()->create(); + $schools[3] = School::factory()->create(); + foreach ($schools as $school) { + $students[] = Student::factory()->create(['school_id' => $school->id]); + } + actAsAdmin(); + $response = $this->withSession([ + 'adminStudentFilters' => [ + 'first_name' => '', + 'last_name' => '', + 'school' => $schools[3]->id, + 'grade' => 'all', + ], + ]) + ->get(route('admin.students.index')); + $response->assertOk() + ->assertViewIs('admin.students.index'); + $returnedStudentIds = $response->viewData('students')->pluck('id')->toArray(); + expect($returnedStudentIds)->toHaveCount(1) + ->and(in_array($students[0]->id, $returnedStudentIds))->toBeFalsy() + ->and(in_array($students[1]->id, $returnedStudentIds))->toBeFalsy() + ->and(in_array($students[2]->id, $returnedStudentIds))->toBeTruthy(); + }); + +}); + +describe('StudentController::create', function () { + it('denies access to a non-admin user', function () { + $this->get(route('admin.students.create'))->assertRedirect(route('home')); + actAsNormal(); + $this->get(route('admin.students.create'))->assertRedirect(route('dashboard')); + actAsTab(); + $this->get(route('admin.students.create'))->assertRedirect(route('dashboard')); + }); + it('shows a form to create a student', function () { + School::factory()->count(5)->create(); + Audition::factory()->create(['minimum_grade' => 6, 'maximum_grade' => 8]); + Audition::factory()->create(['minimum_grade' => 7, 'maximum_grade' => 11]); + actAsAdmin(); + $response = $this->get(route('admin.students.create'))->assertOk(); + $response->assertViewIs('admin.students.create') + ->assertViewHas('schools') + ->assertViewHas('minGrade', 6) + ->assertViewHas('maxGrade', 11); + }); + it('still works when there are nomination ensembles', function () { + School::factory()->count(5)->create(); + Audition::factory()->create(['minimum_grade' => 6, 'maximum_grade' => 8]); + Audition::factory()->create(['minimum_grade' => 7, 'maximum_grade' => 11]); + NominationEnsemble::factory()->create(['minimum_grade' => 6, 'maximum_grade' => 8]); + actAsAdmin(); + $response = $this->get(route('admin.students.create'))->assertOk(); + $response->assertViewIs('admin.students.create') + ->assertViewHas('schools') + ->assertViewHas('minGrade', 6) + ->assertViewHas('maxGrade', 11); + }); +}); + +describe('StudentController::store', function () { + it('denies access to a non-admin user', function () { + $this->post(route('admin.students.store'))->assertRedirect(route('home')); + actAsNormal(); + $this->post(route('admin.students.store'))->assertRedirect(route('dashboard')); + actAsTab(); + $this->post(route('admin.students.store'))->assertRedirect(route('dashboard')); + }); + + it('creates a student', function () { + $school = School::factory()->create(); + actAsAdmin(); + $response = $this->post(route('admin.students.store'), [ + 'first_name' => 'John', + 'last_name' => 'Doe', + 'school_id' => $school->id, + 'grade' => 6, + ]); + file_put_contents(storage_path('debug.html'), $response->getContent()); + $response->assertRedirect(route('admin.students.index')) + ->assertSessionHas('success', 'Student created successfully'); + $student = Student::first(); + expect($student->first_name)->toEqual('John') + ->and($student->last_name)->toEqual('Doe') + ->and($student->school_id)->toEqual($school->id) + ->and($student->grade)->toEqual(6); + }); + + it('does not allow a duplicate student', function () { + $school = School::factory()->create(); + Student::create([ + 'first_name' => 'John', + 'last_name' => 'Doe', + 'school_id' => $school->id, + 'grade' => 6, + ]); + actAsAdmin(); + $response = $this->post(route('admin.students.store'), [ + 'first_name' => 'John', + 'last_name' => 'Doe', + 'school_id' => $school->id, + 'grade' => 6, + ]); + $response->assertRedirect(route('home')); + }); +}); + +describe('StudentController::edit', function () { + beforeEach(function () { + $this->student = Student::factory()->create(); + }); + it('denies access to a non-admin user', function () { + $this->get(route('admin.students.edit', 1))->assertRedirect(route('home')); + actAsNormal(); + $this->get(route('admin.students.edit', 1))->assertRedirect(route('dashboard')); + actAsTab(); + $this->get(route('admin.students.edit', 1))->assertRedirect(route('dashboard')); + }); +}); diff --git a/tests/Feature/app/Http/Controllers/Admin/UserControllerTest.php b/tests/Feature/app/Http/Controllers/Admin/UserControllerTest.php index 1f92440..e689d12 100644 --- a/tests/Feature/app/Http/Controllers/Admin/UserControllerTest.php +++ b/tests/Feature/app/Http/Controllers/Admin/UserControllerTest.php @@ -174,7 +174,7 @@ describe('UserController::store', function () { 'judging_preference' => 'light counting', 'school_id' => $school->id, ])); - //file_put_contents(storage_path('debug.html'), $response->getContent()); + $response->assertRedirect(route('admin.users.index')); $user = User::orderBy('id', 'desc')->first(); expect($user->first_name)->toBe('Jean Luc') diff --git a/tests/Feature/app/Http/Controllers/StudentControllerTest.php b/tests/Feature/app/Http/Controllers/StudentControllerTest.php index 69bcb56..8972af0 100644 --- a/tests/Feature/app/Http/Controllers/StudentControllerTest.php +++ b/tests/Feature/app/Http/Controllers/StudentControllerTest.php @@ -84,6 +84,24 @@ describe('Test the store method of the student controller', function () { ->and(Student::first()->school_id)->toEqual($this->school->id); }); + it('does not allow a user to create a student for another school', function () { + $user = User::factory()->create(); + $user->school_id = $this->school->id; + $user->save(); + $this->actingAs($user); + $otherSchool = School::factory()->create(); + $localData = $this->submitData; + $localData['school_id'] = $otherSchool->id; + + $response = $this->post(route('students.store'), $localData); + $response->assertRedirect(route('students.index')); + expect(Student::first())->toBeInstanceOf(Student::class) + ->and(Student::first()->first_name)->toEqual('John') + ->and(Student::first()->last_name)->toEqual('Doe') + ->and(Student::first()->grade)->toEqual(8) + ->and(Student::first()->school_id)->toEqual($this->school->id); + }); + it('creates a student with optional data', function () { $user = User::factory()->create(); $user->school_id = $this->school->id; diff --git a/tests/Feature/app/Rules/UniqueFullNameAtSchoolTest.php b/tests/Feature/app/Rules/UniqueFullNameAtSchoolTest.php index c92b5ba..59e2d73 100644 --- a/tests/Feature/app/Rules/UniqueFullNameAtSchoolTest.php +++ b/tests/Feature/app/Rules/UniqueFullNameAtSchoolTest.php @@ -27,7 +27,7 @@ describe('UniqueFullNameAtSchool validation rule', function () { // Assert expect($fails)->toBeTrue() - ->and($rule->message())->toBe('There is already a student with that name at the school you are trying to add them to'); + ->and($rule->message())->toBe('There is already a student with that name at that school.'); }); it('passes validation when no student with same name exists at school', function () {