Re-engagement doesn't work with Group restriction

[cwarwicker]

Adding a re-engagement activity with a Restrict Access by Group does not work, as the users are not picked up by the initial scheduled task to add them to the activity.

We have noticed this on some clients, and also in several posts on the Moodle Forums: https://moodle.org/mod/forum/discuss.php?d=337782#p1552518

A pull request will be submitted under this issue with a fix.

[eric-gm-martin]

cwarwicker, by "we have noticed this on some clients" are you saying re-engagement does not work with groups under particular use cases? Or, that re-engagement does not work with groups at all? (I am interested in supporting this effort but would like more info)

[cwarwicker]

Hi,

It's a while since I posted this, so I can't really remember the details. But when I say "clients", these are separate Moodle instances.

As far as I recall, it was just a case that if an activity has the Restrict Access by Group, then the scheduled task in the reengagement plugin didn't find them.

[cedmallet]

Hi,

just ran across the same issue. I created a re-engagement to send a reminder (notify after delay), but with an access restriction: students must belong to a group. The notification email got sent to everyone, instead of only students within the group.

[cwarwicker]

Okay, I believe I've worked this out and will push up a new Pull Request shortly.

To the problem with this lies in this line of code:

$modinfo = get_fast_modinfo($reengagement->courseid); on lib.php:776

By not passing a userid through as the 2nd parameter, this stores the currently logged in user's ID in that $modinfo object.

This eventually gets passed through all the way into the availability_group\condition::is_available() method, which uses this check to get an array of the groups the user belongs to: $groups = $info->get_modinfo()->get_groups();

Because that modinfo object contains the currently logged in user, it checks against them instead of the actual user we want to check.

So, the initial solution to this would be to move this code from the lib.php file:

$modinfo = get_fast_modinfo($reengagement->courseid);
$cm = $modinfo->get_cm($reengagement->cmid);
$ainfomod = new \core_availability\info_module($cm);

To inside of the user loop, and to pass the user id into the get_fast_modinfo() function: (Unfortunately it doesn't seem to be possible to load this object once and then change the user id, as there is no method which allows that)gets

foreach ($startusers as $startcandidate) {

    $modinfo = get_fast_modinfo($reengagement->courseid, $startcandidate->id);
    $cm = $modinfo->get_cm($reengagement->cmid);
    $ainfomod = new \core_availability\info_module($cm);

    ...

}

This will potentially make it slower on large courses, but it is the only way I can see to ensure it works correctly.

This fix will probably also fix any other similiar instances of it not working for other restriction types as well, I would have thought.

[danmarsden]

yeah - that makes a lot more sense - nice work. If you get a chance to create a PR for the latest branch (MOODLE_400_STABLE) then it can be backported to teh MOODLE_311_STABLE and MOODLE_39_STABLE branches - if not I'll try to get to this and manually cherry pick this in later in the week.

[cwarwicker]

I've done one for moodle 4 here: https://github.com/catalyst/moodle-mod_reengagement/pull/121

[danmarsden]

thanks - merged that in, will try to come back to cherry pick that into 3.11 sometime and then we can merge into the 3.9 branch.

[danmarsden]

this should now be fixed in the moodle branches - closing.