misc: tweaks and cleanups (#640)

* refactor: training states is now a helper * refactor: training creation added C3 in controller instead * fix: training policies and workaround * misc: setting dependency comment * refactor: training report attachments * misc: removed unused atchours() in user model It's been replaced by atcActivity model a while ago
Vatsim-Scandinavia · Oct 1, 2023 · 672e6fd · 672e6fd
1 parent d78ec8a
commit 672e6fd
Show file tree

Hide file tree

Showing 17 changed files with 70 additions and 51 deletions.
diff --git a/app/Console/Commands/UpdateQueueCalculation.php b/app/Console/Commands/UpdateQueueCalculation.php
@@ -2,6 +2,7 @@
 
 namespace App\Console\Commands;
 
+use App\Helpers\TrainingStatus;
 use App\Models\Area;
 use Carbon\Carbon;
 use Illuminate\Console\Command;
@@ -53,7 +54,7 @@ public function handle()
                 foreach ($rating->trainings->where('area_id', $area->id)->whereNotNull('created_at')->whereNull('paused_at') as $training) {
                     // Include training with GRP ratings inside
                     if ($training->ratings->count() >= 1 && $training->ratings->first()->vatsim_rating) {
-                        if ($training->status == 0) {
+                        if ($training->status == TrainingStatus::IN_QUEUE->value) {
                             $trainingCreated = $training->created_at;
 
                             // Calculate the difference in seconds with Carbon, then subtract the paused time if any.

diff --git a/app/Helpers/TrainingStatus.php b/app/Helpers/TrainingStatus.php
@@ -0,0 +1,18 @@
+<?php
+
+namespace App\Helpers;
+
+/**
+ * Constants for training status.
+ */
+enum TrainingStatus: int
+{
+    case CLOSED_BY_SYSTEM = -4;
+    case CLOSED_BY_STUDENT = -3;
+    case CLOSED_BY_STAFF = -2;
+    case COMPLETED = -1;
+    case IN_QUEUE = 0;
+    case PRE_TRAINING = 1;
+    case ACTIVE_TRAINING = 2;
+    case AWAITING_EXAM = 3;
+}
diff --git a/app/Http/Controllers/DashboardController.php b/app/Http/Controllers/DashboardController.php
@@ -3,7 +3,6 @@
 namespace App\Http\Controllers;
 
 use anlutro\LaravelSettings\Facade as Setting;
-use App\Models\AtcActivity;
 use App\Models\TrainingInterest;
 use App\Models\TrainingReport;
 use App\Models\User;
@@ -66,8 +65,7 @@ public function index()
         // Check if there's an active vote running to advertise
         $activeVote = Vote::where('closed', 0)->first();
 
-        $atcHoursDB = AtcActivity::find($user->id);
-        $atcHours = ($atcHoursDB == null) ? null : $atcHoursDB->hours;
+        $atcHours = $user->atcActivity->hours;
 
         $studentTrainings = \Auth::user()->mentoringTrainings();
 

diff --git a/app/Http/Controllers/GlobalSettingController.php b/app/Http/Controllers/GlobalSettingController.php
@@ -66,7 +66,7 @@ public function edit(Request $request, Setting $setting)
         isset($data['atcActivityNotifyInactive']) ? $atcActivityNotifyInactive = true : $atcActivityNotifyInactive = false;
         isset($data['atcActivityAllowReactivation']) ? $atcActivityAllowReactivation = true : $atcActivityAllowReactivation = false;
 
-        // The setting dependecy removes keys that are empty, so this is a workaround...
+        // The setting dependency doesn't support null values, so we need to set it to false if it's not set
         isset($data['linkMoodle']) ? $linkMoodle = $data['linkMoodle'] : $linkMoodle = false;
         isset($data['trainingExamTemplate']) ? $trainingExamTemplate = $data['trainingExamTemplate'] : $trainingExamTemplate = false;
 

diff --git a/app/Http/Controllers/TrainingController.php b/app/Http/Controllers/TrainingController.php
@@ -3,6 +3,7 @@
 namespace App\Http\Controllers;
 
 use App;
+use App\Helpers\TrainingStatus;
 use App\Models\Area;
 use App\Models\AtcActivity;
 use App\Models\Rating;
@@ -209,9 +210,13 @@ public function create(Request $request, $prefillUserId = null)
         $this->authorize('create', Training::class);
 
         $students = User::all();
-        $ratings = Area::with('ratings')->get()->toArray();
         $types = TrainingController::$types;
 
+        // Fetch all ratings and add C3 to all areas
+        $ratings = Area::with('ratings')->get()->each(function ($area) {
+            $area->ratings->push(Rating::where('name', 'C3')->first());
+        })->sortBy('name')->toArray();
+
         return view('training.create', compact('students', 'ratings', 'types', 'prefillUserId'));
     }
 
@@ -461,7 +466,7 @@ public function updateDetails(Training $training)
             }
 
             unset($attributes['mentors']);
-        } elseif (Auth::user()->isModeratorOrAbove()) { // XXX This is really hack since we don't send this attribute when mentors submit
+        } else {
             // Detach all if no passed key, as that means the list is empty
 
             foreach ($training->mentors as $mentor) {
@@ -492,7 +497,7 @@ public function updateDetails(Training $training)
 
         // If training is closed, force to unpause
         if ((int) $training->status != $oldStatus) {
-            if ((int) $training->status < 0) {
+            if ((int) $training->status < TrainingStatus::IN_QUEUE->value) {
                 $attributes['paused_at'] = null;
                 if (isset($training->paused_at)) {
                     TrainingActivityController::create($training->id, 'PAUSE', 0, null, Auth::user()->id);
@@ -510,12 +515,12 @@ public function updateDetails(Training $training)
 
         // Send e-mail and store endorsements rating (non-GRP ones), if it's a new status and it goes from active to closed
         if ((int) $training->status != $oldStatus) {
-            if ((int) $training->status < 0) {
+            if ((int) $training->status < TrainingStatus::IN_QUEUE->value) {
                 // Detach all mentors
                 $training->mentors()->detach();
 
                 // If the training was completed and double checked with a passed exam result, store the relevant endorsements
-                if ((int) $training->status == -1) {
+                if ((int) $training->status == TrainingStatus::COMPLETED->value) {
                     foreach ($training->ratings as $rating) {
                         if ($rating->vatsim_rating == null) {
                             // Revoke the old endorsement if active
@@ -546,7 +551,7 @@ public function updateDetails(Training $training)
                 }
 
                 // If training is completed with a passed exam result, let's set the user to active
-                if ((int) $training->status == -1) {
+                if ((int) $training->status == TrainingStatus::COMPLETED->value) {
                     // If training is [Refresh, Transfer or Fast-track] or [Standard and exam is passed]
                     if ($training->type <= 4) {
                         $training->user->atc_active = true;
@@ -571,7 +576,7 @@ public function updateDetails(Training $training)
                 return redirect($training->path())->withSuccess('Training successfully closed. E-mail confirmation sent to the student.');
             }
 
-            if ((int) $training->status == 1) {
+            if ((int) $training->status == TrainingStatus::PRE_TRAINING->value) {
                 $training->user->notify(new TrainingPreStatusNotification($training));
 
                 return redirect($training->path())->withSuccess('Training successfully updated. E-mail confirmation of pre-training sent to the student.');

diff --git a/app/Http/Controllers/TrainingExaminationController.php b/app/Http/Controllers/TrainingExaminationController.php
@@ -2,6 +2,7 @@
 
 namespace App\Http\Controllers;
 
+use App\Helpers\TrainingStatus;
 use App\Models\OneTimeLink;
 use App\Models\Position;
 use App\Models\Training;
@@ -27,7 +28,7 @@ class TrainingExaminationController extends Controller
     public function create(Request $request, Training $training)
     {
         $this->authorize('create', [TrainingExamination::class, $training]);
-        if ($training->status != 3) {
+        if ($training->status != TrainingStatus::AWAITING_EXAM->value) {
             return redirect(null, 400)->to($training->path())->withSuccess('Training examination cannot be created for a training not awaiting exam.');
         }
 

diff --git a/app/Http/Controllers/TrainingReportController.php b/app/Http/Controllers/TrainingReportController.php
@@ -2,6 +2,7 @@
 
 namespace App\Http\Controllers;
 
+use App\Helpers\TrainingStatus;
 use App\Models\OneTimeLink;
 use App\Models\Position;
 use App\Models\Training;
@@ -42,7 +43,7 @@ public function index(Training $training)
     public function create(Training $training)
     {
         $this->authorize('create', [TrainingReport::class, $training]);
-        if ($training->status < 1) {
+        if ($training->status < TrainingStatus::PRE_TRAINING->value) {
             return redirect(null, 400)->back()->withErrors('Training report cannot be created for a training not in progress.');
         }
 
@@ -73,10 +74,11 @@ public function store(Request $request, Training $training)
 
         (isset($data['draft'])) ? $data['draft'] = true : $data['draft'] = false;
 
-        $data2 = $data; // TODO this should be refactored to something better
-        unset($data2['files']);
-        $report = TrainingReport::create($data2);
+        // Remove attachments , they are added in next step
+        unset($data['files']);
+        $report = TrainingReport::create($data);
 
+        // Add attachments
         TrainingObjectAttachmentController::saveAttachments($request, $report);
 
         // Notify student of new training request if it's not a draft

diff --git a/app/Models/User.php b/app/Models/User.php
@@ -123,14 +123,6 @@ public function atcActivity()
         return $this->hasOne(AtcActivity::class);
     }
 
-    // TODO: decide if we should nuke me from orbit
-    public function atchours()
-    {
-        $atcHoursDB = AtcActivity::where('user_id', $this->id)->get()->first();
-
-        return ($atcHoursDB == null) ? null : $atcHoursDB->hours;
-    }
-
     public function getNameAttribute()
     {
         return $this->first_name . ' ' . $this->last_name;

diff --git a/app/Policies/OneTimeLinkPolicy.php b/app/Policies/OneTimeLinkPolicy.php
@@ -2,6 +2,7 @@
 
 namespace App\Policies;
 
+use App\Helpers\TrainingStatus;
 use App\Models\OneTimeLink;
 use App\Models\Training;
 use App\Models\User;
@@ -20,7 +21,7 @@ public function create(User $user, Training $training, string $type)
     {
         // Only allow examination link generation if the training is awaiting exam
         if ($type == OneTimeLink::TRAINING_EXAMINATION_TYPE) {
-            return $training->status == 3 && ($training->mentors->contains($user) || $user->isModeratorOrAbove($training->area));
+            return $training->status == TrainingStatus::AWAITING_EXAM->value && ($training->mentors->contains($user) || $user->isModeratorOrAbove($training->area));
         }
 
         return $training->mentors->contains($user) || $user->isModeratorOrAbove($training->area);

diff --git a/app/Policies/TrainingActivityPolicy.php b/app/Policies/TrainingActivityPolicy.php
@@ -17,7 +17,8 @@ class TrainingActivityPolicy
      */
     public function comment(User $user, Training $training)
     {
-        return $user->can('update', [Training::class, $training]);
+        return $training->mentors->contains($user) ||
+        $user->can('update', [Training::class, $training]);
     }
 
     /**

diff --git a/app/Policies/TrainingPolicy.php b/app/Policies/TrainingPolicy.php
@@ -3,6 +3,7 @@
 namespace App\Policies;
 
 use anlutro\LaravelSettings\Facade as Setting;
+use App\Helpers\TrainingStatus;
 use App\Models\Area;
 use App\Models\Training;
 use App\Models\TrainingExamination;
@@ -35,8 +36,7 @@ public function view(User $user, Training $training)
      */
     public function update(User $user, Training $training)
     {
-        return $training->mentors->contains($user) ||
-                $user->isModeratorOrAbove($training->area);
+        return $user->isModeratorOrAbove($training->area);
     }
 
     /**
@@ -56,7 +56,7 @@ public function delete(User $user, Training $training)
      */
     public function close(User $user, Training $training)
     {
-        return $user->is($training->user) && $training->status == 0;
+        return $user->is($training->user) && $training->status == TrainingStatus::IN_QUEUE->value;
     }
 
     /**

diff --git a/database/seeders/DatabaseSeeder.php b/database/seeders/DatabaseSeeder.php
@@ -3,6 +3,7 @@
 namespace Database\Seeders;
 
 use App\Helpers\FactoryHelper;
+use App\Helpers\TrainingStatus;
 use App\Models\Endorsement;
 use App\Models\Group;
 use App\Models\Position;
@@ -123,7 +124,7 @@ public function run()
             $training->ratings()->attach(Rating::where('vatsim_rating', '>', 1)->inRandomOrder()->first());
 
             // Give all non-queued trainings a mentor
-            if ($training->status > 0) {
+            if ($training->status > TrainingStatus::IN_QUEUE->value) {
                 $training->mentors()->attach(
                     User::whereHas('groups', function ($query) {
                         $query->where('id', 3);
@@ -137,7 +138,7 @@ public function run()
             }
 
             // Give all exam awaiting trainings a solo endorsement
-            if ($training->status == 3) {
+            if ($training->status == TrainingStatus::AWAITING_EXAM->value) {
                 if (! Endorsement::where('user_id', $training->user_id)->exists()) {
                     $soloEndorsement = Endorsement::factory()->create([
                         'user_id' => $training->user_id,

diff --git a/resources/views/dashboard.blade.php b/resources/views/dashboard.blade.php
@@ -168,13 +168,13 @@
                                         @endphp
                                         <span title="{{ $reportDate->toEuropeanDate() }}">
                                             @if($reportDate->isToday())
-                                            <span class="{{ ($trainingIntervalExceeded && $training->status != 3 && !$training->paused_at) ? 'text-danger' : '' }}">Today</span>
+                                            <span class="{{ ($trainingIntervalExceeded && $training->status != \App\Helpers\TrainingStatus::AWAITING_EXAM->value && !$training->paused_at) ? 'text-danger' : '' }}">Today</span>
                                             @elseif($reportDate->isYesterday())
-                                            <span class="{{ ($trainingIntervalExceeded && $training->status != 3 && !$training->paused_at) ? 'text-danger' : '' }}">Yesterday</span>
+                                            <span class="{{ ($trainingIntervalExceeded && $training->status != \App\Helpers\TrainingStatus::AWAITING_EXAM->value && !$training->paused_at) ? 'text-danger' : '' }}">Yesterday</span>
                                             @elseif($reportDate->diffInDays() <= 7)
-                                            <span class="{{ ($trainingIntervalExceeded && $training->status != 3 && !$training->paused_at) ? 'text-danger' : '' }}">{{ $reportDate->diffForHumans(['parts' => 1]) }}</span>
+                                            <span class="{{ ($trainingIntervalExceeded && $training->status != \App\Helpers\TrainingStatus::AWAITING_EXAM->value && !$training->paused_at) ? 'text-danger' : '' }}">{{ $reportDate->diffForHumans(['parts' => 1]) }}</span>
                                             @else
-                                            <span class="{{ ($trainingIntervalExceeded && $training->status != 3 && !$training->paused_at) ? 'text-danger' : '' }}">{{ $reportDate->diffForHumans(['parts' => 2]) }}</span>
+                                            <span class="{{ ($trainingIntervalExceeded && $training->status != \App\Helpers\TrainingStatus::AWAITING_EXAM->value && !$training->paused_at) ? 'text-danger' : '' }}">{{ $reportDate->diffForHumans(['parts' => 2]) }}</span>
                                             @endif
 
                                         </span>

diff --git a/resources/views/mentor/index.blade.php b/resources/views/mentor/index.blade.php
@@ -85,13 +85,13 @@
 
                                         <span title="{{ $reportDate->toEuropeanDate() }}">
                                             @if($reportDate->isToday())
-                                            <span class="{{ ($trainingIntervalExceeded && $training->status != 3 && !$training->paused_at) ? 'text-danger' : '' }}">Today</span>
+                                            <span class="{{ ($trainingIntervalExceeded && $training->status != \App\Helpers\TrainingStatus::AWAITING_EXAM->value && !$training->paused_at) ? 'text-danger' : '' }}">Today</span>
                                             @elseif($reportDate->isYesterday())
-                                            <span class="{{ ($trainingIntervalExceeded && $training->status != 3 && !$training->paused_at) ? 'text-danger' : '' }}">Yesterday</span>
+                                            <span class="{{ ($trainingIntervalExceeded && $training->status != \App\Helpers\TrainingStatus::AWAITING_EXAM->value && !$training->paused_at) ? 'text-danger' : '' }}">Yesterday</span>
                                             @elseif($reportDate->diffInDays() <= 7)
-                                            <span class="{{ ($trainingIntervalExceeded && $training->status != 3 && !$training->paused_at) ? 'text-danger' : '' }}">{{ $reportDate->diffForHumans(['parts' => 1]) }}</span>
+                                            <span class="{{ ($trainingIntervalExceeded && $training->status != \App\Helpers\TrainingStatus::AWAITING_EXAM->value && !$training->paused_at) ? 'text-danger' : '' }}">{{ $reportDate->diffForHumans(['parts' => 1]) }}</span>
                                             @else
-                                            <span class="{{ ($trainingIntervalExceeded && $training->status != 3 && !$training->paused_at) ? 'text-danger' : '' }}">{{ $reportDate->diffForHumans(['parts' => 2]) }}</span>
+                                            <span class="{{ ($trainingIntervalExceeded && $training->status != \App\Helpers\TrainingStatus::AWAITING_EXAM->value && !$training->paused_at) ? 'text-danger' : '' }}">{{ $reportDate->diffForHumans(['parts' => 2]) }}</span>
                                             @endif
 
                                         </span>

diff --git a/resources/views/reports/mentors.blade.php b/resources/views/reports/mentors.blade.php
@@ -84,13 +84,13 @@
                                                     @endphp
                                                     <span data-bs-toggle="tooltip" data-bs-placement="top" title="{{ $reportDate->toEuropeanDate() }}">
                                                         @if($reportDate->isToday())
-                                                            <span class="{{ ($trainingIntervalExceeded && $training->status != 3 && !$training->paused_at) ? 'text-danger' : '' }}">Today</span>
+                                                            <span class="{{ ($trainingIntervalExceeded && $training->status != \App\Helpers\TrainingStatus::AWAITING_EXAM->value && !$training->paused_at) ? 'text-danger' : '' }}">Today</span>
                                                         @elseif($reportDate->isYesterday())
-                                                            <span class="{{ ($trainingIntervalExceeded && $training->status != 3 && !$training->paused_at) ? 'text-danger' : '' }}">Yesterday</span>
+                                                            <span class="{{ ($trainingIntervalExceeded && $training->status != \App\Helpers\TrainingStatus::AWAITING_EXAM->value && !$training->paused_at) ? 'text-danger' : '' }}">Yesterday</span>
                                                         @elseif($reportDate->diffInDays() <= 7)
-                                                            <span class="{{ ($trainingIntervalExceeded && $training->status != 3 && !$training->paused_at) ? 'text-danger' : '' }}">{{ $reportDate->diffForHumans(['parts' => 1]) }}</span>
+                                                            <span class="{{ ($trainingIntervalExceeded && $training->status != \App\Helpers\TrainingStatus::AWAITING_EXAM->value && !$training->paused_at) ? 'text-danger' : '' }}">{{ $reportDate->diffForHumans(['parts' => 1]) }}</span>
                                                         @else
-                                                            <span class="{{ ($trainingIntervalExceeded && $training->status != 3 && !$training->paused_at) ? 'text-danger' : '' }}">{{ $reportDate->diffForHumans(['parts' => 2]) }}</span>
+                                                            <span class="{{ ($trainingIntervalExceeded && $training->status != \App\Helpers\TrainingStatus::AWAITING_EXAM->value && !$training->paused_at) ? 'text-danger' : '' }}">{{ $reportDate->diffForHumans(['parts' => 2]) }}</span>
                                                         @endif                                        
                                                     </span>
                                                 @else

diff --git a/resources/views/training/create.blade.php b/resources/views/training/create.blade.php
@@ -110,7 +110,6 @@ class="form-control @error('student') is-invalid @enderror"
                 showTrainingLevels: function(event) {
                     const selectedTrainingArea = event.srcElement.options[event.srcElement.selectedIndex];
                     this.ratings = payload[selectedTrainingArea.getAttribute('data-id')].ratings;
-                    this.ratings.push({id: 5, name: "C3"});
                 },
             },
         })