Skip to content

Commit

Permalink
Improve user enrolment event handling to be more efficient for activi…
Browse files Browse the repository at this point in the history 
…ty information - #114.
  • Loading branch information
@gjb2048
gjb2048 committed Nov 13, 2021
1 parent 47a3436 commit c6a5d3f
Show file tree
Hide file tree
Showing 4 changed files with 159 additions and 34 deletions.
4 changes: 3 additions & 1 deletion Changes.md
View file
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,9 @@

Version 3.10.1.4 - TBR
-----------------------------
1. Improve documentation for Toggle blocks location' functionality - https://moodle.org/mod/forum/discuss.php?d=428924 and #100.
1. Add 'enableadditionalmoddata' setting to turn on / off additional information at a site level. Default is 'off'!
2. Improve documentation for Toggle blocks location' functionality - https://moodle.org/mod/forum/discuss.php?d=428924 and #100.
3. Improve user enrolment event handling to be more efficient for activity information - #114.

Version 3.10.1.3 - 30/09/2021
-----------------------------
Expand Down
179 changes: 147 additions & 32 deletions classes/activity.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -363,8 +363,7 @@ protected static function forum_meta(cm_info $modinst) {
* @return string
*/
protected static function lesson_meta(cm_info $modinst) {
$meta = self::std_meta($modinst, '', '', '', '', '', 'attempted', true);
return $meta;
return self::std_meta($modinst, '', '', '', '', '', 'attempted', true);
}

/**
Expand Down Expand Up @@ -534,14 +533,13 @@ protected static function forum_num_submissions($courseid, $mod) {
$params['forumid'] = $mod->instance;
$studentscache = \cache::make('format_topcoll', 'activitystudentscache');
$students = $studentscache->get($courseid);

$userids = implode(',', $students);

$sql = "SELECT count(DISTINCT fp.userid) as total
FROM {forum_posts} fp, {forum_discussions} fd
WHERE fd.forum = :forumid
AND fp.userid IN ($userids)
AND fp.discussion = fd.id";
FROM {forum_posts} fp, {forum_discussions} fd
WHERE fd.forum = :forumid
AND fp.userid IN ($userids)
AND fp.discussion = fd.id";
$studentspostedcount = $DB->get_records_sql($sql, $params);

if (!empty($studentspostedcount)) {
Expand Down Expand Up @@ -963,16 +961,80 @@ protected static function grade_row($courseid, $mod) {
*/
protected static function course_participant_count($courseid, $mod) {
$students = self::course_get_students($courseid);

// New users?
$usercreatedcache = \cache::make('format_topcoll', 'activityusercreatedcache');
$createdusers = $usercreatedcache->get($courseid);
$lock = null;
$newstudents = array();
if (!empty($createdusers)) {
$lock = self::lockcaches($courseid);

$studentrolescache = \cache::make('format_topcoll', 'activitystudentrolescache');
$studentroles = $studentrolescache->get('roles');
$context = \context_course::instance($courseid);
$alluserroles = get_users_roles($context, $createdusers, false);

foreach ($createdusers as $userid) {
$usershortnames = array();
foreach ($alluserroles[$userid] as $userrole) {
$usershortnames[] = $userrole->shortname;
}
$isstudent = false;
foreach ($studentroles as $studentrole) {
if (in_array($studentrole, $usershortnames)) {
// User is in a role that is based on a student archetype on the course.
$isstudent = true;
break;
}
}
if (!$isstudent) {
// Don't go any further.
continue;
} else {
$newstudents[$userid] = $userid;
}
}

$usercreatedcache->set($courseid, null);

if (is_array($students)) {
foreach ($newstudents as $newstudent) {
if (!array_key_exists($newstudent, $students)) {
$students[$newstudent] = $newstudent;
}
}
$studentscache = \cache::make('format_topcoll', 'activitystudentscache');
$studentscache->set($courseid, $students);
} else if (!empty($newstudents)) {
$students = $newstudents;
$studentscache = \cache::make('format_topcoll', 'activitystudentscache');
$studentscache->set($courseid, $students);
}
}

if (is_array($students)) {
// We have students!
$modulecountcache = \cache::make('format_topcoll', 'activitymodulecountcache');
$modulecountcourse = $modulecountcache->get($courseid);
if (empty($modulecountcourse)) {
$modulecountcourse = self::calulatecoursemodules($courseid, $students);
$modulecountcache->set($courseid, $modulecountcourse);
} else if (!empty($newstudents)) {
// Update.
$modulecountcourse = self::calulatecoursemodules($courseid, $newstudents, null, $modulecountcourse);
$modulecountcache->set($courseid, $modulecountcourse);
}

return $modulecountcourse[$mod->id];
if (!is_null($lock)) {
$lock->release();
}

return $modulecountcourse[$mod->id][0];
}

if (!is_null($lock)) {
$lock->release();
}

return 0;
Expand Down Expand Up @@ -1024,7 +1086,7 @@ public static function course_get_students($courseid) {
// Don't go any further.
continue;
} else {
$students[] = $userid;
$students[$userid] = $userid;
}
}

Expand Down Expand Up @@ -1106,7 +1168,7 @@ public static function invalidatestudentscache() {
*/
public static function userenrolmentcreated($userid, $courseid, $courseformat) {
if (self::activitymetaenabled() && self::activitymetaused($courseformat)) {
self::clearcoursemodulecount($courseid);
self::userenrolmentchanged($userid, $courseid, 1);
}
}

Expand All @@ -1118,7 +1180,7 @@ public static function userenrolmentcreated($userid, $courseid, $courseformat) {
*/
public static function userenrolmentupdated($userid, $courseid, $courseformat) {
if (self::activitymetaenabled() && self::activitymetaused($courseformat)) {
self::clearcoursemodulecount($courseid);
self::userenrolmentchanged($userid, $courseid, 0);
}
}

Expand All @@ -1130,8 +1192,59 @@ public static function userenrolmentupdated($userid, $courseid, $courseformat) {
*/
public static function userenrolmentdeleted($userid, $courseid, $courseformat) {
if (self::activitymetaenabled() && self::activitymetaused($courseformat)) {
self::clearcoursemodulecount($courseid);
self::userenrolmentchanged($userid, $courseid, -1);
}
}

/**
* A user enrolment has changed.
*
* @param int $userid User id.
* @param int $courseid Course id.
* @param int $type -1 = deleted, 0 changed and 1 created.
*/
private static function userenrolmentchanged($userid, $courseid, $type) {
$lock = self::lockcaches($courseid);
if ($type == 1) {
// Created.
/* Note: At the time of the event, the DB has not been updated to know that the given user has been assigned a role
of 'student' - role_assignments table with data relating to that contained in the event itself. */
$usercreatedcache = \cache::make('format_topcoll', 'activityusercreatedcache');
$createdusers = $usercreatedcache->get($courseid);
if (empty($createdusers)) {
$createdusers = array();
}
$createdusers[] = $userid;
$usercreatedcache->set($courseid, $createdusers);
} else if ($type == -1) {
// Deleted.
$studentscache = \cache::make('format_topcoll', 'activitystudentscache');
$students = $studentscache->get($courseid);
if (!empty($students)) {
if (array_key_exists($userid, $students)) {
unset($students[$userid]);
$studentscache->set($courseid, $students);
$modulecountcache = \cache::make('format_topcoll', 'activitymodulecountcache');
$modulecountcourse = $modulecountcache->get($courseid);
if (empty($modulecountcourse)) {
if (!empty($students)) {
$modulecountcourse = self::calulatecoursemodules($courseid, $students);
$modulecountcache->set($courseid, $modulecountcourse);
}
} else {
$modulecountcoursekeys = array_keys($modulecountcourse);
foreach ($modulecountcoursekeys as $modid) {
if (in_array($userid, $modulecountcourse[$modid][1])) {
$modulecountcourse[$modid][0]--;
unset($modulecountcourse[$modid][1][$userid]);
}
}
$modulecountcache->set($courseid, $modulecountcourse);
}
}
} // Else no students no problem.
}
$lock->release();
}

/**
Expand Down Expand Up @@ -1162,7 +1275,7 @@ public static function moduleupdated($modid, $courseid, $courseformat) {
*/
private static function modulechanged($modid, $courseid, $courseformat) {
if (self::activitymetaenabled() && self::activitymetaused($courseformat)) {
$lock = self::lockmodulecountcache($courseid);
$lock = self::lockcaches($courseid);
$studentscache = \cache::make('format_topcoll', 'activitystudentscache');
$students = $studentscache->get($courseid);
if (is_array($students)) {
Expand All @@ -1186,7 +1299,7 @@ private static function modulechanged($modid, $courseid, $courseformat) {
*/
public static function moduledeleted($modid, $courseid, $courseformat) {
if (self::activitymetaenabled() && self::activitymetaused($courseformat)) {
$lock = self::lockmodulecountcache($courseid);
$lock = self::lockcaches($courseid);
$modulecountcache = \cache::make('format_topcoll', 'activitymodulecountcache');
$modulecountcourse = $modulecountcache->get($courseid);
if (!empty($modulecountcourse)) {
Expand All @@ -1204,7 +1317,7 @@ public static function moduledeleted($modid, $courseid, $courseformat) {
* @param int $courseid Course id.
*/
private static function clearcoursemodulecount($courseid) {
$lock = self::lockmodulecountcache($courseid);
$lock = self::lockcaches($courseid);
$modulecountcache = \cache::make('format_topcoll', 'activitymodulecountcache');
$modulecountcache->set($courseid, null);
$studentscache = \cache::make('format_topcoll', 'activitystudentscache');
Expand All @@ -1218,20 +1331,22 @@ private static function clearcoursemodulecount($courseid) {
* @param int $courseid Course id.
* @param array $students Array of student id's on the course.
* @param int $modid Calculate specific module id or null if calculate all.
* @param array $modulecount Existing module count if any.
*
* @return int Number of participants (students) on the modules requested on the course.
*/
private static function calulatecoursemodules($courseid, $students, $modid = null) {
$modulecount = array(); // Mod id indexed.
if (is_null($modid)) {
// Initialise to zero in case of no enrolled students on the course.
$modinfo = get_fast_modinfo($courseid, -1);
$cms = $modinfo->get_cms(); // Array of cm_info objects.
foreach ($cms as $themod) {
$modulecount[$themod->id] = 0;
private static function calulatecoursemodules($courseid, $students, $modid = null, $modulecount = null) {
if (is_null($modulecount)) {
if (is_null($modid)) {
// Initialise to zero in case of no enrolled students on the course.
$modinfo = get_fast_modinfo($courseid, -1);
$cms = $modinfo->get_cms(); // Array of cm_info objects.
foreach ($cms as $themod) {
$modulecount[$themod->id] = array(0, array());
}
} else {
$modulecount[$modid] = array(0, array());
}
} else {
$modulecount[$modid] = 0;
}
foreach ($students as $userid) {
$modinfo = get_fast_modinfo($courseid, $userid);
Expand All @@ -1241,11 +1356,11 @@ private static function calulatecoursemodules($courseid, $students, $modid = nul
continue;
}
// From course_section_cm() in M3.8 - is_visible_on_course_page for M3.9+.
if (((method_exists($usermod, 'is_visible_on_course_page')) && ($usermod->is_visible_on_course_page()))
|| ((!empty($usermod->availableinfo)) && ($usermod->url))) {
if (($usermod->is_visible_on_course_page()) || (!empty($usermod->availableinfo) && ($usermod->url))) {
// From course_section_cm_name_title().
if ($usermod->uservisible) {
$modulecount[$usermod->id]++;
$modulecount[$usermod->id][0]++;
$modulecount[$usermod->id][1][] = $userid;
}
}
}
Expand All @@ -1255,19 +1370,19 @@ private static function calulatecoursemodules($courseid, $students, $modid = nul
}

/**
* Get a lock for the module count cache on the given course.
* Get a lock for the caches on the given course.
*
* @param int $courseid Course id.
*
* @return object The lock to release when complete.
*/
private static function lockmodulecountcache($courseid) {
private static function lockcaches($courseid) {
$lockfactory = \core\lock\lock_config::get_lock_factory('format_topcoll');
if ($lock = $lockfactory->get_lock('courseid'.$courseid, 5)) {
return $lock;
}
throw new \moodle_exception('cannotgetmodulecountcachelock', 'format_topcoll', '',
get_string('cannotgetmodulecountcachelock', 'format_topcoll', $courseid));
throw new \moodle_exception('cannotgetactivitycacheslock', 'format_topcoll', '',
get_string('cannotgetactivitycacheslock', 'format_topcoll', $courseid));
}

/**
Expand Down
7 changes: 7 additions & 0 deletions db/caches.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -56,5 +56,12 @@
'simplekeys' => true,
'simpledata' => true,
'staticacceleration' => true
),
// Caches the ids of the new users on a given course.
'activityusercreatedcache' => array(
'mode' => cache_store::MODE_APPLICATION,
'simplekeys' => true,
'simpledata' => true,
'staticacceleration' => true
)
);
3 changes: 2 additions & 1 deletion lang/en/format_topcoll.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -156,7 +156,8 @@
$string['cachedef_activitystudentrolescache'] = 'Caches the student roles.';
$string['cachedef_activitymodulecountcache'] = 'Caches the number of students who can access a given module on a given course.';
$string['cachedef_activitystudentscache'] = 'Caches the ids of the students on a given course.';
$string['cannotgetmodulecountcachelock'] = 'Cannot get module count cache lock for course id {$a}.';
$string['cachedef_activityusercreatedcache'] = 'Caches the ids of the new users on a given course.';
$string['cannotgetactivitycacheslock'] = 'Cannot get activity caches lock for course id {$a}.';

// Colour enhancement - Moodle Tracker CONTRIB-3529.
$string['setcolour'] = 'Colour';
Expand Down

0 comments on commit c6a5d3f

Please sign in to comment.