Filter training events by event identifier, rather than by just user by redrails · Pull Request #5031 · VATSIM-UK/core

redrails · 2026-07-26T12:56:37Z

Summary of changes

Instead of just filtering the training events by CID, they are now filtered by CID and training event identifier, i.e. a user with a session on PPL_P1 should not show with a last event field when requesting mentoring for EGLL_N_APP.

Unit tests

CLC0609 · 2026-07-26T19:04:51Z

                    ->whereColumn('student_id', 'members.id')
                    ->whereNotNull('taken_date')
                    ->whereNull('cancelled_datetime')
+                    ->where('position', Session::select('position')


This will only take sessions on the same exact position. We want the last time a student had a session within a Training Group. On core, a Training Group is basically just the category on the position.

So for example if a student has requested a "OBS_PH_PT2" session, which has category="OBS To S1 Training" , then we would want to get the last time the student has had a session on any "OBS To S1 Training" positions.

Hope that makes sense!

@CLC0609 I've updated the code, the changes are slightly larger but here's my understanding of it.

Take the pending requested position (for example, EGKK_APP).

Resolve its training category from training position mappings (this seems to be in the training_positions.cts_positions column from my understanding, but my table is not fully seeded as per prod).

Build the allowed callsign set from that same category (maybe there's some refactoring possible here as the MentorPermissionService.getAllCtsCallsignsForCategory() seems to be similar).

Find the student’s latest completed session only within that set.

If none exists, show Never.

Example:

Pending EGKK_APP (S3), previous EGKK_TWR (S2) => Never.
Pending EGLL_APP (S3), previous EGKK_APP (S3) => show that previous session as Last session.

Can you confirm if A) my understanding is correct and B) If I am using the correct schemas here? I had some trouble trying to map the categories and positions from the tables I can see.

Yeah the logic above is all correct. cts_positions is just a list of callsigns so that will work. I would use getAllCtsCallsignsForCategory as it handles the pilot logic for you aswell. Seems to be some test fails aswell but looks good so far!

Thanks! I've changed the code to now use getAllCtsCallsignsForCategory with also a small refactor on that function since it was looking a bit messy. Also removed the latest_session on the Member query since it was obsolete, keeping the ordering intact as before. I'm not sure how to actually test this, would be useful to see if this works as expected - Happy to jump on discord if you have time at any point!

…t in the Gantt

…-filter

github-actions · 2026-08-07T13:13:36Z

🔍 OpenCodeReview found 5 issue(s) in this PR.

✅ Successfully posted inline: 5 comment(s)

github-actions · 2026-08-07T13:13:37Z

+        $service = app(MentorPermissionService::class);

-            return $service->getAllCtsCallsignsForCategories($user->getAvailableMentoringCategories());
+        if ($this->category) {
+            return $service->getAllCtsCallsignsForCategory($this->category);
        }

-        return $this->category ? $user->getAssignedCallsignsForCategory($this->category) : $user->getAllAssignedCallsigns();
+        return $service->getAllCtsCallsignsForCategories($user->getAvailableMentoringCategories());


[security · medium]
The viewAll permission gate that previously guarded the MentorPermissionService call has been removed. Previously, non-admin mentors would only see pending sessions matching their assigned callsigns (getAssignedCallsignsForCategory / getAllAssignedCallsigns). Now all mentors receive all callsigns for every category they can access, regardless of which specific positions they are assigned to.

While category-level access is still enforced via mount() and getAvailableMentoringCategories(), this change broadens the data visible to mentors within those categories. Confirm this is the intended behavior before deploying.

I am not sure about this one, this whole permission / can see setup is pretty sketchy on this page. I'd write a test or two and assert the permissions are still correct.

github-actions · 2026-08-07T13:13:37Z

+        $students->each(function (Member $student): void {
+            $pendingPosition = $student->pending_position;
+
+            if (! is_string($pendingPosition) || $pendingPosition === '') {
+                $student->setAttribute('last_session_date', null);
+
+                return;
+            }
+
+            $pendingCategory = TrainingPosition::query()
+                ->whereNotNull('category')
+                ->whereJsonContains('cts_positions', $pendingPosition)
+                ->orderByDesc('id')
+                ->value('category');
+
+            if (! is_string($pendingCategory) || $pendingCategory === '') {
+                $student->setAttribute('last_session_date', null);
+
+                return;
+            }
+
+            $groupCallsigns = TrainingPosition::query()
+                ->where('category', $pendingCategory)
+                ->get(['cts_positions'])
+                ->flatMap(fn (TrainingPosition $position) => is_array($position->cts_positions) ? $position->cts_positions : [])
+                ->filter(fn ($callsign) => is_string($callsign) && $callsign !== '')
+                ->unique()
+                ->values()
+                ->toArray();
+
+            if ($groupCallsigns === []) {
+                $student->setAttribute('last_session_date', null);
+
+                return;
+            }
+
+            $latestSession = Session::query()
+                ->where('student_id', $student->id)
+                ->whereNotNull('taken_date')
+                ->whereNull('cancelled_datetime')
+                ->whereIn('position', $groupCallsigns)
+                ->orderBy('taken_date', 'desc')
+                ->orderBy('taken_from', 'desc')
+                ->first(['taken_date', 'taken_from']);


[performance · high]
The each() loop performs up to 3 database queries per student: one to resolve the pending category (line 178), one to fetch all positions in that category (line 190), and one for the latest session (line 205). Since getStudentsProperty() loads ALL matching students before pagination (getPagedStudentsProperty applies forPage on the collection), 100 students would trigger ~300 extra queries.

Consider moving the category resolution and group-callsign lookup outside the loop (e.g., build a callsign-to-category map once, then a category-to-callsigns map), and batch the session queries per category group rather than per student.

Yes this query setup seems wack, I'd check if it's possible to refactor this somehow

github-actions · 2026-08-07T13:13:37Z

+        return $students
+            ->sortBy(fn (Member $student) => $student->last_session_date ?? '9999-12-31 23:59:59')
+            ->values();


[bug · medium]
The sort fallback for null last_session_date has changed from '1970-01-01 00:00:00' (old SQL COALESCE) to '9999-12-31 23:59:59'. Previously, students who had never been mentored appeared first in the list; now they appear last.

If this reversal is intentional, it should be confirmed. If unintentional, change the fallback to '0000-00-00 00:00:00' (or equivalent early date) to preserve the original ordering behavior.

Suggestion:

Suggested change

return $students

->sortBy(fn (Member $student) => $student->last_session_date ?? '9999-12-31 23:59:59')

->values();

return $students

->sortBy(fn (Member $student) => $student->last_session_date ?? '1970-01-01 00:00:00')

->values();

Apply the suggestion, do not use that ancient mysql default

github-actions · 2026-08-07T13:13:37Z

+        Livewire::actingAs($this->mentor)
+            ->test(AvailabilityGantt::class)
+            ->assertSee('Never');
+
+        Carbon::setTestNow();
+    }
+
+    #[Test]
+    public function last_session_date_uses_other_positions_in_the_same_pending_request_category(): void


[style · low]
Using assertSee('Never') is broad and could match the string anywhere on the page (e.g., labels, other student rows, HTML attributes), leading to potential false positives. Consider asserting against the student's computed attribute directly (as done in the second test), or use assertSeeInOrder / assertSeeHtml with more context to target the specific row.

Suggestion:

Suggested change

Livewire::actingAs($this->mentor)

->test(AvailabilityGantt::class)

->assertSee('Never');

Carbon::setTestNow();

}

#[Test]

public function last_session_date_uses_other_positions_in_the_same_pending_request_category(): void

$component = Livewire::actingAs($this->mentor)

->test(AvailabilityGantt::class);

$studentResult = $component->instance()->students->firstWhere('id', $student->id);

$this->assertNotNull($studentResult);

$this->assertNull($studentResult->last_session_date);

Carbon::setTestNow();

}

#[Test]

public function last_session_date_uses_other_positions_in_the_same_pending_request_category(): void

That test does feel a bit too generic yes

github-actions · 2026-08-07T13:13:37Z

+        Livewire::actingAs($this->mentor)
+            ->test(AvailabilityGantt::class)
+            ->assertSee('Never');
+
+        Carbon::setTestNow();
+    }
+
    #[Test]
    public function last_session_date_shows_never_when_no_session_exists(): void


[style · low]
Same broad assertSee('Never') as above — consider targeting the specific student's computed attribute value for a more precise assertion.

Suggestion:

Suggested change

Livewire::actingAs($this->mentor)

->test(AvailabilityGantt::class)

->assertSee('Never');

Carbon::setTestNow();

}

#[Test]

public function last_session_date_shows_never_when_no_session_exists(): void

$component = Livewire::actingAs($this->mentor)

->test(AvailabilityGantt::class);

$studentResult = $component->instance()->students->firstWhere('id', $student->id);

$this->assertNotNull($studentResult);

$this->assertNull($studentResult->last_session_date);

Carbon::setTestNow();

}

#[Test]

public function last_session_date_shows_never_when_no_session_exists(): void

same as above

redrails · 2026-08-07T13:52:05Z

@kristiankunc could you review the suggestions above and let me know which ones require attention? I'm unsure on the process you use for copilot reviews, it can be an endless cycle.

kristiankunc · 2026-08-07T13:59:36Z

I commented on all of them, the only one requiring a bit more attention are the queries and the permissions.

Filter training events by event identifier, rather than by just user

1b6444f

kristiankunc requested a review from CLC0609 July 26, 2026 14:34

CLC0609 reviewed Jul 26, 2026

View reviewed changes

Amended logic for how the last session lookup is checked for a studen…

5d429ca

…t in the Gantt

redrails requested a review from CLC0609 July 28, 2026 00:07

redrails force-pushed the issue/5013-training-filter branch from 5d429ca to 6d3f262 Compare July 28, 2026 00:22

Merge upstream/main

c641471

redrails force-pushed the issue/5013-training-filter branch from 6d3f262 to c641471 Compare July 28, 2026 00:32

redrails added 3 commits July 29, 2026 20:55

Merge remote-tracking branch 'upstream/main' into issue/5013-training…

c637376

…-filter

Now using getAllCtsCallsigns* and refactored last session logic

fbf9cd0

Fix ordering logic that was removed in refactor

bb6fc85

CLC0609 added the area:training label Aug 1, 2026

kristiankunc added the review label Aug 7, 2026

github-actions Bot reviewed Aug 7, 2026

View reviewed changes

github-actions Bot removed the review label Aug 7, 2026

Uh oh!

Conversation

redrails commented Jul 26, 2026 • edited by CLC0609 Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary of changes

Uh oh!

Choose a reason for hiding this comment

Uh oh!

redrails Jul 28, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

github-actions Bot commented Aug 7, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

github-actions Bot Aug 7, 2026

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

github-actions Bot Aug 7, 2026

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

github-actions Bot Aug 7, 2026

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

github-actions Bot Aug 7, 2026

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

github-actions Bot Aug 7, 2026

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

redrails commented Aug 7, 2026

Uh oh!

kristiankunc commented Aug 7, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

redrails commented Jul 26, 2026 •

edited by CLC0609

Loading

redrails Jul 28, 2026 •

edited

Loading

github-actions Bot commented Aug 7, 2026 •

edited

Loading

kristiankunc commented Aug 7, 2026 •

edited

Loading