Skip to content

Fix moodle:sync stacking by cutting per-user Moodle HTTP call volume #112

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
Celeo merged 1 commit into from
Jul 21, 2026
Merged

Fix moodle:sync stacking by cutting per-user Moodle HTTP call volume #112

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
80 changes: 80 additions & 0 deletions app/Classes/VATUSAMoodle.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -326,6 +326,21 @@ public function getCidFromUserId(int $uid): ?int
return $user->idnumber ?? null;
}

/**
* Build a full CID -> Moodle user id map in a single query, to replace a
* whole-table per-user HTTP existence check with an in-memory lookup.
*
* @return \Illuminate\Support\Collection
*/
public function getAllUserIdMap(): \Illuminate\Support\Collection
{
return DB::connection('moodle')->table('user')
->where('deleted', 0)
->whereNotNull('idnumber')
->where('idnumber', '!=', '')
->pluck('id', 'idnumber');
}

/**
* Create user.
*
Expand Down Expand Up @@ -441,6 +456,30 @@ public function assignCohort(int $uid, string $cnumber)
]);
}

/**
* Assign many users to Cohorts in a single request.
*
* @param array $items Each: ['uid' => int, 'cnumber' => string]
*
* @return mixed
* @throws \Exception
*/
public function assignCohortsBulk(array $items)
{
return $this->request("core_cohort_add_cohort_members", [
"members" => array_values(array_map(fn ($item) => [
"cohorttype" => [
'type' => 'idnumber',
'value' => $item['cnumber']
],
"usertype" => [
'type' => 'id',
'value' => $item['uid']
]
], $items))
]);
}

/**
* Unassign Cohort
*
Expand Down Expand Up @@ -491,6 +530,26 @@ public function assignRole(int $uid, ?int $cid, string $role, string $context)
]);
}

/**
* Assign many Roles in a single request.
*
* @param array $items Each: ['uid' => int, 'cid' => int|null, 'role' => string, 'context' => string]
*
* @return mixed
* @throws \Exception
*/
public function assignRolesBulk(array $items)
{
return $this->request("core_role_assign_roles", [
"assignments" => array_values(array_map(fn ($item) => [
"roleid" => $this->roleIds[$item['role']],
"userid" => $item['uid'],
"contextid" => $item['cid'],
"contextlevel" => $item['context']
], $items))
]);
}

/**
*
* Remove Role from User in Context
Expand Down Expand Up @@ -677,6 +736,27 @@ function enrolUser(
["enrolments" => [0 => ["roleid" => $rid, "userid" => $uid, "courseid" => $cid]]]);
}

/**
* Enrol many users in courses in a single request.
*
* @param array $items Each: ['uid' => int, 'cid' => int, 'rid' => int|null]
*
* @return mixed
* @throws \Exception
*/
public
function enrolUsersBulk(
array $items
) {
return $this->request("enrol_manual_enrol_users", [
"enrolments" => array_values(array_map(fn ($item) => [
"roleid" => $item['rid'] ?? $this->roleIds['STU'],
"userid" => $item['uid'],
"courseid" => $item['cid']
], $items))
]);
}

/**
* Unenrol User from Course
*
Expand Down
102 changes: 74 additions & 28 deletions app/Console/Commands/MoodleSync.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -62,10 +62,11 @@ public function handle()
}

//Syncronize Users
User::with('visits', 'academyCompetencies.course', 'roles')->chunk(1000, function ($users) {
$moodleIds = $this->moodle->getAllUserIdMap();
User::with('visits', 'academyCompetencies.course', 'roles')->chunk(1000, function ($users) use ($moodleIds) {
foreach ($users as $user) {
if ($this->moodle->getUserId($user->cid)) {
$this->sync($user);
if ($moodleIds->has($user->cid)) {
$this->sync($user, (int) $moodleIds[$user->cid]);
}
}
});
Expand All @@ -77,13 +78,19 @@ public function handle()
* Synchronize Roles
*
* @param \App\User $user
* @param int|null $knownId Moodle user id, if already known (bulk sync path) — skips the
* redundant existence-check lookup that the single-user CLI path
* still needs to decide create-vs-update.
*
* @throws \Exception
*/
private function sync(User $user)
private function sync(User $user, ?int $knownId = null)
{
//Update or Create
if ($id = $this->moodle->getUserId($user->cid)) {
if ($knownId !== null) {
$id = $knownId;
$this->moodle->updateUser($user, $id);
} elseif ($id = $this->moodle->getUserId($user->cid)) {
//Update Information
$this->moodle->updateUser($user, $id);
} else {
Expand All @@ -94,74 +101,113 @@ private function sync(User $user)

//Assign Cohorts
$this->moodle->clearUserCohorts($id);
$this->moodle->assignCohort($id,
Helper::ratingShortFromInt($user->rating)); //VATUSA level rating
$cohorts = [];
$cohorts[] = Helper::ratingShortFromInt($user->rating); //VATUSA level rating
if ($user->flag_homecontroller) {
$this->moodle->assignCohort($id,
"$user->facility-" . Helper::ratingShortFromInt($user->rating)); //Facility level rating
$cohorts[] = "$user->facility-" . Helper::ratingShortFromInt($user->rating); //Facility level rating
if (RoleHelper::userIsVATUSAStaff($user, false, true)
|| RoleHelper::userIsInstructor($user)
|| RoleHelper::userIsSeniorStaff($user, null, true)
|| RoleHelper::userIsMentor($user)) {
$this->moodle->assignCohort($id, "TNG"); //Training staff
$cohorts[] = "TNG"; //Training staff
}
}
$this->moodle->assignCohort($id, $user->facility); //Home Facility
$cohorts[] = $user->facility; //Home Facility

foreach ($user->visits->pluck('facility') as $facility) {
//Visiting Facilities
$this->moodle->assignCohort($id, $facility . "-V"); //Facility level visitor
$this->moodle->assignCohort($id,
"$facility-" . Helper::ratingShortFromInt($user->rating)); //Facility level rating
$cohorts[] = $facility . "-V"; //Facility level visitor
$cohorts[] = "$facility-" . Helper::ratingShortFromInt($user->rating); //Facility level rating
}
if (!empty($cohorts)) {
$this->moodle->assignCohortsBulk(array_map(fn ($cnumber) => ['uid' => $id, 'cnumber' => $cnumber],
$cohorts));
}

//Clear Roles
$this->moodle->clearUserRoles($id);

$roles = [];
//Assign Student Role
foreach ($facilities as $facility) {
$this->moodle->assignRole($id, $this->moodle->getCategoryFromShort($facility, true), "STU", "coursecat");
$roles[] = [
'uid' => $id,
'cid' => $this->moodle->getCategoryFromShort($facility, true),
'role' => "STU",
'context' => "coursecat"
];
}

//Assign Category Permissions
$enrolments = [];
if (RoleHelper::userIsVATUSAStaff($user, false, true) || RoleHelper::userIsSeniorStaff($user, $user->facility,
true)) {
$this->moodle->assignRole($id, VATUSAMoodle::CATEGORY_CONTEXT_VATUSA, "INS", "coursecat");
$this->moodle->assignRole($id, $this->moodle->getCategoryFromShort($user->facility, true), "TA",
"coursecat");
$roles[] = [
'uid' => $id, 'cid' => VATUSAMoodle::CATEGORY_CONTEXT_VATUSA, 'role' => "INS",
'context' => "coursecat"
];
$roles[] = [
'uid' => $id,
'cid' => $this->moodle->getCategoryFromShort($user->facility, true),
'role' => "TA",
'context' => "coursecat"
];
$artccCategories = $this->moodle->getAllSubcategories($this->moodle->getCategoryFromShort($user->facility),
true);
foreach ($artccCategories as $category) {
$courses = $this->moodle->getCoursesInCategory($category);
foreach ($courses as $course) {
$this->moodle->enrolUser($id, $course["id"]);
$enrolments[] = ['uid' => $id, 'cid' => $course["id"]];
}
}
}
if (RoleHelper::userIsVATUSAStaff($user, false, true) || RoleHelper::userHas($user, "ZAE", "CBT")) {
$this->moodle->assignRole($id, VATUSAMoodle::CATEGORY_CONTEXT_VATUSA, "CBT", "coursecat");
$roles[] = [
'uid' => $id, 'cid' => VATUSAMoodle::CATEGORY_CONTEXT_VATUSA, 'role' => "CBT",
'context' => "coursecat"
];
}
if (RoleHelper::userIsVATUSAStaff($user, false, true) || RoleHelper::userHas($user, $user->facility,
"FACCBT")) {
$this->moodle->assignRole($id, $this->moodle->getCategoryFromShort($user->facility, true), "FACCBT",
"coursecat");
$roles[] = [
'uid' => $id,
'cid' => $this->moodle->getCategoryFromShort($user->facility, true),
'role' => "FACCBT",
'context' => "coursecat"
];
}
if (RoleHelper::userIsVATUSAStaff($user, false, true) || RoleHelper::userIsInstructor($user,
$user->facility)) {
$this->moodle->assignRole($id, VATUSAMoodle::CATEGORY_CONTEXT_VATUSA, "INS", "coursecat");
$this->moodle->assignRole($id, $this->moodle->getCategoryFromShort($user->facility, true), "INS",
"coursecat");
$roles[] = [
'uid' => $id, 'cid' => VATUSAMoodle::CATEGORY_CONTEXT_VATUSA, 'role' => "INS",
'context' => "coursecat"
];
$roles[] = [
'uid' => $id,
'cid' => $this->moodle->getCategoryFromShort($user->facility, true),
'role' => "INS",
'context' => "coursecat"
];
}
if (RoleHelper::userIsVATUSAStaff($user, false, true) || RoleHelper::userIsMentor($user)) {
for ($i = Helper::ratingIntFromShort("S1"); $i <= $user->rating; $i++) {
$context = "EXAM_CONTEXT_" . Helper::ratingShortFromInt($i);
$this->moodle->assignRole($id, $this->moodle->getConstant($context), "MTR", "course");
$roles[] = [
'uid' => $id, 'cid' => $this->moodle->getConstant($context), 'role' => "MTR",
'context' => "course"
];
}
}

/*if (Role::where("cid", $user->cid)->where("facility", $user->facility)->where("role", "MTR")->exists()) {
$this->moodle->assignRole($id, $this->moodle->getCategoryFromShort($user->facility, true), "MTR",
"coursecat");
$roles[] = ['uid' => $id, 'cid' => $this->moodle->getCategoryFromShort($user->facility, true), 'role' => "MTR", 'context' => "coursecat"];
}*/

if (!empty($roles)) {
$this->moodle->assignRolesBulk($roles);
}
if (!empty($enrolments)) {
$this->moodle->enrolUsersBulk($enrolments);
}
}
}
46 changes: 46 additions & 0 deletions tests/Unit/MoodleUserIdMapTest.php
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,46 @@
<?php

namespace Tests\Unit;

use Illuminate\Support\Collection;
use PHPUnit\Framework\TestCase;

/**
* Pure unit test — does not boot the Laravel application or touch a database.
*
* VATUSAMoodle::getAllUserIdMap() builds its map via
* DB::table('user')->pluck('id', 'idnumber'), where the Moodle DB's `idnumber`
* column is a VARCHAR holding the VATUSA CID as a string. MoodleSync::handle()
* then looks entries up with the integer $user->cid. This test locks in the
* assumption that swap relies on: PHP normalizes numeric string array keys to
* integers, so Collection::has()/offsetGet() with an int CID still matches a
* map keyed from string column values.
*/
class MoodleUserIdMapTest extends TestCase
{
public function test_int_cid_lookup_matches_string_keyed_map(): void
{
// Simulates DB::connection('moodle')->table('user')->pluck('id', 'idnumber'),
// where idnumber is fetched as a string from a VARCHAR column.
$moodleIds = new Collection([
'1234567' => 42,
'7654321' => 99,
]);

$this->assertTrue($moodleIds->has(1234567));
$this->assertSame(42, $moodleIds[1234567]);

$this->assertFalse($moodleIds->has(9999999));
}

public function test_users_absent_from_moodle_are_skipped(): void
{
$moodleIds = new Collection(['1234567' => 42]);

$cidsToSync = collect([1234567, 2222222, 7654321])
->filter(fn ($cid) => $moodleIds->has($cid))
->values();

$this->assertSame([1234567], $cidsToSync->all());
}
}
Loading