Attendance module shows users enrolled with disabled enrolment methods about moodle-mod_attendance HOT 9 CLOSED

comment copied from tracker:
Thanks for the report - the way the attendance module obtains the user list isn't great - it also tries to display information about suspended users as well
The core piece of the code that affects this is in attendance/locallib.php get_users()
if you aren't using groups you could modify the existing calls to get_enrolled_users and add "true" as the 8th param but this won't solve it for the cases where groups are being used.
The more I look at the get_users() function the more I see areas that could do with re-factoring.
Unfortunately my volunteer time is in very high demand so it's unlikely I'll get to this in the short-term so if you have someone that is able to work on it and provide a patch it would be really appreciated.

from moodle-mod_attendance.

We have an interesting variation on this.

We have a user that has access via multiple enrolment methods, 2 custom ones and a manual enrolment.

Our custom enrolment method suspends a user if they drop off the source list, and in this case we have 1 suspended enrolment, and two active ones:

The attendance taking screen displays this user as "Enrolment Suspend"

Which suggests that it isn't resolving all of the statuses or is taking a suspension as being higher precedence...?

from moodle-mod_attendance.

Thanks Michael, let us know if you track it down!

from moodle-mod_attendance.

I think the problem is in the get_users() function. Around about line 1103 (CONTRIB-3549) there is a select statement against the user_enrolments table.

In any case once the select is performed, the code then iterates over it's list of users, and looks for a matching $enrolments[] record (an array indexed by $user->id.

This presupposes that there is only a single enrolment for a given user, so should there be > 1 enrolment then this would be incomplete data to make the decision on.

I suspect the problem is that the get_records_sql() will be using the 1st record as a unique ID, but as this is ue.userid it isn't.

It probably needs changed to SELECT ue.id, ue.userid ..., and then within the foreach($users as $user) loop iterate the $enrolments array for each user.

Of course this wouldn't be terribly efficient, as you'd have to loop $enrolments for every $user.

Instead could loop $enrolments and "fill" $users

foreach($enrolments as $e) {
    if (isset($users[$e->userid]) {
        //Check that the user is in the class list
        if ($e->status == 0) {  //Active participant via *this* enrolment
            if(!isset($users[$e->userid]->enrolmentstatus)) { //we haven't seen this user yet
                $users[$e->userid]->enrolmentstatus = $e->status;
                $users[$e->userid]->enrolmentstart = $e->mintime;
                $users[$e->userid]->enrolmentend = $e->maxtime;
                $users[$e->userid]->type = 'standard';
            }
            else { //we *have seen this user before

            }
        }
    }
}

I guess you could end up with users that don't have enrolment information but I'd have thought it odd that $enrolments didn't hold 1 or more entry for each $user.

Not tried this in code, but purely based on my reading of this section.

from moodle-mod_attendance.

thanks - there's a lot of hard-coded sql in the attendance module that should really be changed to using core functions to return enrolments but it might be quicker to fix it using the existing sql as you mention.

from moodle-mod_attendance.

I placed a fix for this in bug #118

from moodle-mod_attendance.

Thanks Dave - I've merged that into the master branch - would be good if someone else could verify this as well and I'll cherry pick into the other branches.

from moodle-mod_attendance.

I can confirm that this fixes the problem with multiple enrollments, some suspended, some not.
+1 to cherrypick

from moodle-mod_attendance.

great - thanks, I've cherry-picked that onto the 2.8 and 2.7 branches as well.

from moodle-mod_attendance.