Cleanup TODOs in project

This commit is contained in:
Matt Young 2024-07-16 14:44:44 -05:00
parent 3033ef41c8
commit 7c55b35c40
9 changed files with 11 additions and 25 deletions

View File

@ -11,7 +11,6 @@ use function abort;
class EntryController extends Controller class EntryController extends Controller
{ {
// TODO authorization policies
public function index() public function index()
{ {
@ -31,7 +30,6 @@ class EntryController extends Controller
if ($request->user()->cannot('create', Entry::class)) { if ($request->user()->cannot('create', Entry::class)) {
abort(403); abort(403);
} }
// TODO write custom rule to verify the combination of student and audition is unique
$validData = $request->validate([ $validData = $request->validate([
'student_id' => ['required', 'exists:students,id'], 'student_id' => ['required', 'exists:students,id'],
'audition_id' => ['required', 'exists:auditions,id'], 'audition_id' => ['required', 'exists:auditions,id'],

View File

@ -3,7 +3,6 @@
namespace App\Http\Controllers; namespace App\Http\Controllers;
use App\Models\Audition; use App\Models\Audition;
use App\Models\School;
use App\Models\Student; use App\Models\Student;
use App\Rules\UniqueFullNameAtSchool; use App\Rules\UniqueFullNameAtSchool;
use Illuminate\Http\Request; use Illuminate\Http\Request;
@ -12,7 +11,6 @@ use Illuminate\Support\Facades\Auth;
use function abort; use function abort;
use function redirect; use function redirect;
// TODO validation rules to make sure a student name is unique at a school
class StudentController extends Controller class StudentController extends Controller
{ {
/** /**
@ -106,8 +104,6 @@ class StudentController extends Controller
'grade' => request('grade'), 'grade' => request('grade'),
]); ]);
// TODO if a students grade is changed, we need to be sure they are still eligible for the auditions in which they are entered.
return redirect('/students')->with('success', 'Student updated successfully.'); return redirect('/students')->with('success', 'Student updated successfully.');
} }

View File

@ -84,7 +84,6 @@ class UserController extends Controller
'school_id' => request('school_id'), 'school_id' => request('school_id'),
]); ]);
// TODO we probably don't want to go here if done from an admin page
return redirect('/my_school'); return redirect('/my_school');
} }
} }

View File

@ -47,7 +47,6 @@ class ScoreSheet extends Model
public function isValid() public function isValid()
{ {
// TODO move to either TabulationService or a specific service for scoreValidation
$judges = $this->audition->judges; $judges = $this->audition->judges;
return $judges->contains('id', $this->judge->id); return $judges->contains('id', $this->judge->id);

View File

@ -35,7 +35,6 @@ class ScoringGuide extends Model
*/ */
public function validateScores(array $prospective_score) public function validateScores(array $prospective_score)
{ {
// TODO move to either TabulationService or a specific service for scoreValidation
foreach ($this->subscores as $subscore) { foreach ($this->subscores as $subscore) {
if (! array_key_exists($subscore->id, $prospective_score)) { if (! array_key_exists($subscore->id, $prospective_score)) {
return 'A score must be provided for '.$subscore->name; return 'A score must be provided for '.$subscore->name;
@ -55,6 +54,5 @@ class ScoringGuide extends Model
} }
return 'success'; return 'success';
// TODO this probably needs to be rewritten as a validation rule
} }
} }

View File

@ -160,13 +160,11 @@ class User extends Authenticatable implements MustVerifyEmail
public function scoresForEntry($entry) public function scoresForEntry($entry)
{ {
// TODO Again, why is this here? Needs to go somewhere else. Maybe a Judging service
return $this->scoreSheets->where('entry_id', '=', $entry)->first()?->subscores; return $this->scoreSheets->where('entry_id', '=', $entry)->first()?->subscores;
} }
public function timeForEntryScores($entry) public function timeForEntryScores($entry)
{ {
// TODO Why is this in the User mode? Move it somewhere else
return $this->scoreSheets->where('entry_id', '=', $entry)->first()?->created_at; return $this->scoreSheets->where('entry_id', '=', $entry)->first()?->created_at;
} }
} }

View File

@ -37,7 +37,6 @@ it('grants access to tabulators', function () {
// Act & Assert // Act & Assert
get($this->r)->assertOk(); get($this->r)->assertOk();
}); });
// TODO make tests with varied information
it('returns the audition object and an array of info on each entry', function () { it('returns the audition object and an array of info on each entry', function () {
// Arrange // Arrange
$entry = Entry::factory()->create(['audition_id' => $this->audition->id]); $entry = Entry::factory()->create(['audition_id' => $this->audition->id]);

View File

@ -1,4 +1,6 @@
<?php /** @noinspection PhpUnhandledExceptionInspection */ <?php
/** @noinspection PhpUnhandledExceptionInspection */
use App\Models\Audition; use App\Models\Audition;
use App\Models\Ensemble; use App\Models\Ensemble;
@ -72,7 +74,6 @@ it('shows a delete option for ensembles with no students seated', function () {
$response = get((route('admin.ensembles.index'))); $response = get((route('admin.ensembles.index')));
$response->assertOk(); $response->assertOk();
$response->assertSee(route('admin.ensembles.destroy', $noSeatsEnsemble), false); $response->assertSee(route('admin.ensembles.destroy', $noSeatsEnsemble), false);
//$response->assertDontSee(route('admin.ensembles.destroy', $seatsEnsemble), false); // TODO figure out how to test for a delete form that does not also see an edit form
}); });
it('allows an administrator to delete an ensemble while no entries are seated', function () { it('allows an administrator to delete an ensemble while no entries are seated', function () {
// Arrange // Arrange

View File

@ -4,8 +4,8 @@
use Illuminate\Foundation\Testing\RefreshDatabase; use Illuminate\Foundation\Testing\RefreshDatabase;
use Sinnbeck\DomAssertions\Asserts\AssertForm; use Sinnbeck\DomAssertions\Asserts\AssertForm;
use Sinnbeck\DomAssertions\Asserts\AssertSelect; use Sinnbeck\DomAssertions\Asserts\AssertSelect;
use function Pest\Laravel\get; use function Pest\Laravel\get;
use function Pest\Laravel\post; use function Pest\Laravel\post;
@ -93,12 +93,10 @@ it('has a field with forms for each audition setting', function () {
->containsInput([ ->containsInput([
'name' => 'olympic_scoring', 'name' => 'olympic_scoring',
'type' => 'checkbox', 'type' => 'checkbox',
// TODO how can I test if it is checked when necessary
]) ])
->containsInput([ ->containsInput([
'name' => 'judging_enabled', 'name' => 'judging_enabled',
'type' => 'checkbox', 'type' => 'checkbox',
// TODO how can I test if it is checked when necessary
]) ])
->findSelect('#fee_structure', function (AssertSelect $select) { ->findSelect('#fee_structure', function (AssertSelect $select) {
$select->containsOption([ $select->containsOption([
@ -128,7 +126,7 @@ it('can update audition settings', function () {
'payment_city' => 'New City', 'payment_city' => 'New City',
'payment_state' => 'NS', 'payment_state' => 'NS',
'payment_zip' => 12345, 'payment_zip' => 12345,
'fee_structure' => 'oneFeePerEntry' 'fee_structure' => 'oneFeePerEntry',
]; ];
// Act // Act
$response = post(route('audition-settings-save'), $newData); $response = post(route('audition-settings-save'), $newData);