Progress on student controller rewrite and testing.

app/Http/Controllers/Admin/StudentController.php

@@ -7,14 +7,11 @@ use App\Http\Controllers\Controller;
 use App\Http\Requests\StudentStoreRequest;
 use App\Models\Audition;
 use App\Models\AuditLogEntry;
-use App\Models\Entry;
 use App\Models\Event;
 use App\Models\NominationEnsemble;
 use App\Models\School;
 use App\Models\Student;
-use Illuminate\Support\Facades\Auth;
 
-use function abort;
 use function auth;
 use function compact;
 use function max;
@@ -81,97 +78,21 @@ class StudentController extends Controller
         $maxGrade = $this->maximumGrade();
         $schools = School::orderBy('name')->get();
         $student->loadCount('entries');
-        $entries = $student->entries()->with('audition.flags')->get();
+        $event_entries = $student->entries()->with('audition.flags')->get()->groupBy('audition.event_id');
         $events = Event::all();
-        $event_entries = [];
-        foreach ($events as $event) {
-            $event_entries[$event->id] = $entries->filter(function ($entry) use ($event) {
-                return $event->id === $entry->audition->event_id;
-            });
-            // Check if doubler status can change
-            foreach ($event_entries[$event->id] as $entry) {
-                $entry->doubler_decision_frozen = $this->isDoublerStatusFrozen($entry, $event_entries[$event->id]);
-            }
-        }
 
         return view('admin.students.edit',
             compact('student', 'schools', 'minGrade', 'maxGrade', 'events', 'event_entries'));
     }
 
-    private function isDoublerStatusFrozen(Entry $entry, $entries)
+    public function update(StudentStoreRequest $request, Student $student)
     {
-        // Can't change decision if results are published
-        if ($entry->audition->hasFlag('seats_published')) {
-            return true;
-        }
-
-        // Can't change decision if this is the only entry
-        if ($entries->count() === 1) {
-            return true;
-        }
-
-        // 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');
-        });
-        if ($unpublished->count() < 2) {
-            return true;
-        }
-
-        // Can't change decision if we've accepted another audition
-        foreach ($entries as $checkEntry) {
-            if ($checkEntry->audition->hasFlag('seats_published') && ! $checkEntry->hasFlag('declined')) {
-                return true;
-            }
-        }
-
-        return false;
-    }
-
-    public function update(Student $student)
-    {
-        if (! Auth::user()->is_admin) {
-            abort(403);
-        }
-        request()->validate([
-            'first_name' => ['required'],
-            'last_name' => ['required'],
-            'grade' => ['required', 'integer'],
-            'school_id' => ['required', 'exists:schools,id'],
-        ]);
-
-        foreach ($student->entries as $entry) {
-            if ($entry->audition->minimum_grade > request('grade') || $entry->audition->maximum_grade < request('grade')) {
-                return redirect('/admin/students/'.$student->id.'/edit')->with('error',
-                    'This student is entered in an audition that is not available to their new grade.');
-            }
-        }
-
-        if (Student::where('first_name', request('first_name'))
-            ->where('last_name', request('last_name'))
-            ->where('school_id', request('school_id'))
-            ->where('id', '!=', $student->id)
-            ->exists()) {
-            return redirect('/admin/students/'.$student->id.'/edit')->with('error',
-                'A student with that name already exists at that school');
-        }
-
         $student->update([
-            'first_name' => request('first_name'),
+            'first_name' => $request['first_name'],
-            'last_name' => request('last_name'),
+            'last_name' => $request['last_name'],
-            'grade' => request('grade'),
+            'grade' => $request['grade'],
-            'school_id' => request('school_id'),
+            'school_id' => $request['school_id'],
-        ]);
+            'optional_data' => $request->optional_data,
-
-        $message = 'Updated student #'.$student->id.'<br>Name: '.$student->full_name().'<br>Grade: '.$student->grade.'<br>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', 'Student updated');
@@ -181,7 +102,7 @@ class StudentController extends Controller
     public function destroy(Student $student)
     {
         if ($student->entries()->count() > 0) {
-            return to_route('admin.students.index')->with('error', 'You cannot delete a student with entries.');
+            return to_route('admin.students.index')->with('error', 'Student has entries and cannot be deleted');
         }
         $name = $student->full_name();
         $message = 'Deleted student #'.$student->id.'<br>Name: '.$student->full_name().'<br>Grade: '.$student->grade.'<br>School: '.$student->school->name;

app/Models/Doubler.php

@@ -33,6 +33,11 @@ class Doubler extends Model
         return Entry::whereIn('id', $this->entries)->with('student')->with('audition')->get();
     }
 
+    public function getAcceptedEntry()
+    {
+        return Entry::find($this->accepted_entry);
+    }
+
     // Find a doubler based on both keys
     public static function findDoubler($studentId, $eventId)
     {

app/Models/Entry.php

@@ -58,6 +58,57 @@ class Entry extends Model
 
     }
 
+    public function canChangeDoublerDecision(): bool
+    {
+        // If results are published, we can't change our decision
+        if ($this->audition->hasFlag('seats_published')) {
+            return false;
+        }
+
+        $doubler = Doubler::findDoubler($this->student_id, $this->audition->event_id);
+        // Return false if we're not a doubler
+        if (is_null($doubler)) {
+            return false;
+        }
+
+        // If we're only in two auditions and the other is published, we can't change our decision
+
+        if (count($doubler->entries()) === 2) {
+            foreach ($doubler->entries() as $entry) {
+                if ($entry->audition->hasFlag('seats_published')) {
+                    return false;
+                }
+            }
+        }
+
+        // Deal with superDoublers
+        if ($doubler->entries()->count() > 2) {
+            // If the accepted entry is published, we can't change our decision
+            if ($doubler->getAcceptedEntry()) {
+                $testEntry = $doubler->getAcceptedEntry();
+                if ($testEntry->audition->hasFlag('seats_published')) {
+                    return false;
+                }
+            }
+            // If all other entries are published, we can't change our decision
+
+            foreach ($doubler->entries() as $entry) {
+                if ($entry->id == $this->id) {
+
+                    continue; // We're not checking our own entry
+                }
+                if (! $entry->audition->hasFlag('seats_published')) {
+                    return true; // If there's at least one other unpublished entry, we can change our decision
+                }
+            }
+
+            return false; // We didn't find any unpublised entries other than this one
+        }
+
+        return true;
+
+    }
+
     public function student(): BelongsTo
     {
         return $this->belongsTo(Student::class);
@@ -131,9 +182,7 @@ class Entry extends Model
     {
         $thisFlag = EntryFlag::where('flag_name', $flag)
             ->where('entry_id', $this->id)->first();
-        if ($thisFlag) {
+        $thisFlag?->delete();
-            $thisFlag->delete();
-        }
         $this->load('flags');
     }
 

app/Observers/StudentObserver.php

@@ -3,6 +3,7 @@
 namespace App\Observers;
 
 use App\Models\AuditLogEntry;
+use App\Models\School;
 use App\Models\Student;
 
 use function auth;
@@ -32,14 +33,33 @@ class StudentObserver
      */
     public function updated(Student $student): void
     {
-        $message = 'Updated student #'.$student->id.' - '.$student->full_name().'<br>Grade: '.$student->grade.'<br>School: '.$student->school->name;
+        $message = 'Updated student #'.$student->id;
+
+        $message .= '<br>Name: '.$student->getOriginal('first_name').' '.$student->getOriginal('last_name');
+        if ($student->getOriginal('first_name') !== $student->first_name || $student->getOriginal('last_name') !== $student->last_name) {
+            $message .= ' -> '.$student->first_name.' '.$student->last_name;
+        }
+
+        $message .= '<br>Grade: '.$student->getOriginal('grade');
+        if ($student->getOriginal('grade') !== $student->grade) {
+            $message .= ' -> '.$student->grade;
+        }
+
+        $originalSchool = School::find($student->getOriginal('school_id'))->name;
+        $schoolsAffected[] = $student->school_id;
+        $message .= '<br>School: '.$originalSchool.' (#'.$student->getOriginal('school_id').')';
+        if ($student->school_id !== $student->getOriginal('school_id')) {
+            $schoolsAffected[] = $student->getOriginal('school_id');
+            $message .= ' -> '.$student->school->name.' (#'.$student->school_id.')';
+        }
+
         AuditLogEntry::create([
             'user' => auth()->user()->email ?? 'none',
             'ip_address' => request()->ip(),
             'message' => $message,
             'affected' => [
                 'students' => [$student->id],
-                'schools' => [$student->school_id],
+                'schools' => $schoolsAffected,
             ],
         ]);
     }

resources/views/admin/students/edit.blade.php

@@ -52,13 +52,13 @@
                     </tr>
                 </thead>
                 <tbody>
-                    @foreach($event_entries[$event->id] as $entry)
+                    @foreach($event_entries[$event->id] ?? [] as $entry)
                         <tr>
                             <x-table.td>{{ $entry->id }}</x-table.td>
                             <x-table.td><a href="{{ route ('seating.audition',[$entry->audition_id]) }}#entry-{{ $entry->id }}">{{ $entry->audition->name }}</a></x-table.td>
                             <x-table.td>{{ $entry->draw_number }}</x-table.td>
                             <x-table.td>
-                                @if($entry->doubler_decision_frozen)
+                                @if(! $entry->canChangeDoublerDecision())
                                     <p class="text-red-600">{{ $entry->hasFlag('declined') ? 'DECLINED':'' }}</p>
                                 @else
                                     @if($entry->hasFlag('declined'))

tests/Feature/app/Http/Controllers/Admin/StudentControllerTest.php

@@ -1,6 +1,9 @@
 <?php
 
+use App\Actions\Entries\CreateEntry;
 use App\Models\Audition;
+use App\Models\Entry;
+use App\Models\Event;
 use App\Models\NominationEnsemble;
 use App\Models\School;
 use App\Models\Student;
@@ -215,6 +218,7 @@ describe('StudentController::store', function () {
 describe('StudentController::edit', function () {
     beforeEach(function () {
         $this->student = Student::factory()->create();
+        Audition::factory()->create(['minimum_grade' => 6, 'maximum_grade' => 8]);
     });
     it('denies access to a non-admin user', function () {
         $this->get(route('admin.students.edit', 1))->assertRedirect(route('home'));
@@ -223,4 +227,119 @@ describe('StudentController::edit', function () {
         actAsTab();
         $this->get(route('admin.students.edit', 1))->assertRedirect(route('dashboard'));
     });
+    it('shows a form to edit a student', function () {
+        actAsAdmin();
+        $response = $this->get(route('admin.students.edit', $this->student->id))->assertOk();
+
+        $response->assertViewIs('admin.students.edit')
+            ->assertViewHas('student', $this->student)
+            ->assertViewHas('schools')
+            ->assertViewHas('minGrade', 6)
+            ->assertViewHas('maxGrade', 8)
+            ->assertViewHas('events')
+            ->assertViewHas('event_entries');
+    });
+    it('has all schools on the edit form', function () {
+        $schools = School::factory()->count(5)->create();
+        actAsAdmin();
+        $response = $this->get(route('admin.students.edit', $this->student->id))->assertOk();
+
+        $response->assertViewIs('admin.students.edit')
+            ->assertViewHas('student', $this->student)
+            ->assertViewHas('schools');
+        $returnedSchools = $response->viewData('schools');
+        expect($returnedSchools)->toHaveCount(6);
+        foreach ($schools as $school) {
+            expect(in_array($school->id, $returnedSchools->pluck('id')->toArray()))->toBeTruthy();
+        }
+        expect(in_array($this->student->school->id, $returnedSchools->pluck('id')->toArray()))->toBeTruthy();
+    });
+    it('has all entries grouped by event on the edit form', function () {
+        $schools = School::factory()->count(5)->create();
+        $concertEvent = Event::create(['name' => 'Concert']);
+        Audition::truncate();
+        $ca = Audition::factory()->forEvent($concertEvent)->create(['name' => 'Alto Saxophone']);
+        $ct = Audition::factory()->forEvent($concertEvent)->create(['name' => 'Tenor Saxophone']);
+        $jazzEvent = Event::create(['name' => 'Jazz']);
+        $ja = Audition::factory()->forEvent($jazzEvent)->create(['name' => 'Jazz Alto']);
+        $jt = Audition::factory()->forEvent($jazzEvent)->create(['name' => 'Jazz Tenor']);
+        $scribe = app(CreateEntry::class);
+        foreach (Audition::all() as $audition) {
+            $scribe($this->student, $audition);
+        }
+        actAsAdmin();
+        $response = $this->get(route('admin.students.edit', $this->student->id))->assertOk();
+        $response->assertViewIs('admin.students.edit')
+            ->assertViewHas('student', $this->student)
+            ->assertViewHas('event_entries');
+        expect($response->viewData('event_entries'))->toHaveCount(2)
+            ->and($response->viewData('event_entries')[$concertEvent->id])->toHaveCount(2)
+            ->and($response->viewData('event_entries')[$jazzEvent->id])->toHaveCount(2)
+            ->and($response->viewData('event_entries')[$concertEvent->id][0]['audition_id'])->toEqual($ca->id)
+            ->and($response->viewData('event_entries')[$concertEvent->id][1]['audition_id'])->toEqual($ct->id)
+            ->and($response->viewData('event_entries')[$jazzEvent->id][0]['audition_id'])->toEqual($ja->id)
+            ->and($response->viewData('event_entries')[$jazzEvent->id][1]['audition_id'])->toEqual($jt->id);
+
+    });
+});
+
+describe('StudentController::update', function () {
+    beforeEach(function () {
+        $this->school = School::factory()->create();
+        $this->student = Student::create([
+            'first_name' => 'Jean Luc',
+            'last_name' => 'Picard',
+            'grade' => 12,
+            'school_id' => $this->school->id,
+        ]);
+    });
+    it('denies access to a non-admin user', function () {
+        $this->patch(route('admin.students.update', $this->student->id))->assertRedirect(route('home'));
+        actAsNormal();
+        $this->patch(route('admin.students.update', $this->student->id))->assertRedirect(route('dashboard'));
+    });
+    it('updates a student', function () {
+        actAsAdmin();
+        $newSchool = School::factory()->create();
+        $response = $this->patch(route('admin.students.update', $this->student->id), [
+            'first_name' => 'James',
+            'last_name' => 'Kirk',
+            'school_id' => $newSchool->id,
+            'grade' => 6,
+        ]);
+        $this->student->refresh();
+        $response->assertRedirect(route('admin.students.index'));
+        expect($this->student->first_name)->toEqual('James')
+            ->and($this->student->last_name)->toEqual('Kirk')
+            ->and($this->student->school_id)->toEqual($newSchool->id)
+            ->and($this->student->grade)->toEqual(6);
+    });
+});
+
+describe('StudentController::destroy', function () {
+    beforeEach(function () {
+        $this->student = Student::factory()->create();
+    });
+    it('denies access to a non-admin user', function () {
+        $this->delete(route('admin.students.destroy', $this->student->id))->assertRedirect(route('home'));
+        actAsNormal();
+        $this->delete(route('admin.students.destroy', $this->student->id))->assertRedirect(route('dashboard'));
+    });
+    it('deletes a student', function () {
+        actAsAdmin();
+        $response = $this->delete(route('admin.students.destroy', $this->student->id));
+        $response->assertRedirect(route('admin.students.index'));
+        expect(Student::where('id', $this->student->id)->exists())->toBeFalsy();
+    });
+    it('will not delete a student with entries', function () {
+        actAsAdmin();
+        $entry = Entry::create([
+            'student_id' => $this->student->id,
+            'audition_id' => Audition::factory()->create()->id,
+        ]);
+        $response = $this->delete(route('admin.students.destroy', $this->student->id));
+        $response->assertRedirect(route('admin.students.index'))
+            ->assertSessionHas('error', 'Student has entries and cannot be deleted');
+        expect(Student::where('id', $this->student->id)->exists())->toBeTruthy();
+    });
 });

tests/Feature/app/Models/EntryTest.php

@@ -2,6 +2,7 @@
 
 use App\Actions\Tabulation\RankAuditionEntries;
 use App\Models\Audition;
+use App\Models\Doubler;
 use App\Models\Ensemble;
 use App\Models\Entry;
 use App\Models\EntryTotalScore;
@@ -247,3 +248,93 @@ it('has a scope for only available entries', function () {
     expect(Entry::count())->toEqual(4)
         ->and(Entry::available()->count())->toEqual(1);
 });
+
+describe('it can tell us if it the entries doubler decision can change', function () {
+    it('returns false if we are not a doubler', function () {
+        expect($this->entry->canChangeDoublerDecision())->toBeFalse();
+    });
+    it('returns false if its audition seats are published', function () {
+        $newAudition = Audition::factory()->forEvent($this->entry->audition->event)->create();
+        $newEntry = Entry::create([
+            'audition_id' => $newAudition->id,
+            'student_id' => $this->entry->student_id,
+        ]);
+        $this->entry->audition->addFlag('seats_published');
+        expect($this->entry->canChangeDoublerDecision())->toBeFalse();
+    });
+    it('returns false when there are two entries and the other is published', function () {
+        $newAudition = Audition::factory()->forEvent($this->entry->audition->event)->create();
+        $newEntry = Entry::create([
+            'audition_id' => $newAudition->id,
+            'student_id' => $this->entry->student_id,
+        ]);
+        $newAudition->addFlag('seats_published');
+        expect($this->entry->canChangeDoublerDecision())->toBeFalse();
+    });
+    it('returns true when there are two entries and neither is published', function () {
+        $newAudition = Audition::factory()->forEvent($this->entry->audition->event)->create();
+        $newEntry = Entry::create([
+            'audition_id' => $newAudition->id,
+            'student_id' => $this->entry->student_id,
+        ]);
+        expect($this->entry->canChangeDoublerDecision())->toBeTrue();
+    });
+    it('returns false if the accepted entry is published', function () {
+        $newAudition = Audition::factory()->forEvent($this->entry->audition->event)->create();
+        $thirdAudition = Audition::factory()->forEvent($this->entry->audition->event)->create();
+        $thirdAudition->addFlag('seats_published');
+        $newEntry = Entry::create([
+            'audition_id' => $newAudition->id,
+            'student_id' => $this->entry->student_id,
+        ]);
+        $anothernewEntry = Entry::create([
+            'audition_id' => $thirdAudition->id,
+            'student_id' => $this->entry->student_id,
+        ]);
+        Doubler::first()->update(['accepted_entry' => $anothernewEntry->id]);
+        expect($this->entry->canChangeDoublerDecision())->toBeFalse();
+    });
+    it('returns false if were the only unpublished entry', function () {
+        $newAudition = Audition::factory()->forEvent($this->entry->audition->event)->create();
+        $thirdAudition = Audition::factory()->forEvent($this->entry->audition->event)->create();
+        $thirdAudition->addFlag('seats_published');
+        $newAudition->addFlag('seats_published');
+        $newEntry = Entry::create([
+            'audition_id' => $newAudition->id,
+            'student_id' => $this->entry->student_id,
+        ]);
+        $anothernewEntry = Entry::create([
+            'audition_id' => $thirdAudition->id,
+            'student_id' => $this->entry->student_id,
+        ]);
+        expect($this->entry->canChangeDoublerDecision())->toBeFalse();
+    });
+    it('returns false if were note the only unpublished entry', function () {
+        $newAudition = Audition::factory()->forEvent($this->entry->audition->event)->create();
+        $thirdAudition = Audition::factory()->forEvent($this->entry->audition->event)->create();
+        $thirdAudition->addFlag('seats_published');
+        $newAudition->addFlag('seats_published');
+        $fourthAudition = Audition::factory()->forEvent($this->entry->audition->event)->create();
+        $newEntry = Entry::create([
+            'audition_id' => $newAudition->id,
+            'student_id' => $this->entry->student_id,
+        ]);
+        $anothernewEntry = Entry::create([
+            'audition_id' => $thirdAudition->id,
+            'student_id' => $this->entry->student_id,
+        ]);
+        $fourthEntry = Entry::create([
+            'audition_id' => $fourthAudition->id,
+            'student_id' => $this->entry->student_id,
+        ]);
+        expect($this->entry->canChangeDoublerDecision())->toBeTrue();
+    });
+    it('if nothing stops it, return true', function () {
+        $newAudition = Audition::factory()->forEvent($this->entry->audition->event)->create();
+        $newEntry = Entry::create([
+            'audition_id' => $newAudition->id,
+            'student_id' => $this->entry->student_id,
+        ]);
+        expect($this->entry->canChangeDoublerDecision())->toBeTrue();
+    });
+});

tests/Pest.php

@@ -67,7 +67,7 @@ function loadSampleAudition()
 
 function saveContentLocally($content)
 {
-    file_put_contents(storage_path('app/storage/debug.html'), $content);
+    file_put_contents(storage_path('debug.html'), $content);
 }
 
 uses()->beforeEach(function () {