Enrolment rewrite and role tweaks proposal - the fightback

Warning: This page is no longer in use. The information contained on the page should NOT be seen as relevant or reliable.

This is a response to Petr's document Enrolment rewrite and role tweaks proposal.

I personally don't like the proposal to store 'participation' separately from the roles system.

The participants problem

There are two problems with knowing who is a 'participant' (student) on a course. The official way of doing this currently is to find out users who have course/view access on the course.

1. Getting a list of users by capability is very slow. The capability system is good at checking if the current user has permission to do things, but slow at listing other users who have permission to do something.
2. This list is always wrong because it includes the administrator and other site-level roles that happen to have access to the course.

We need to get these lists all over the place for display - usually when displaying reports about students. For example, a list of all students who have access to the wiki, and whether they've contributed to it.

Why I don't like Petr's solution

Petr proposes to use a new course_participants table to define who is a participant, via enrol plugins.

• I think the existing roles system should be able to handle this information without regard to enrol plugins.

• I think the information Petr's system holds regarding enrolment is necessary for enrolment plugins, but should not relate to this particular task.

• I think permissions should always involve capability checks.

• Petr's system proposes two different ways of belonging to a course/place in Moodle, which are handled fundamentally differently internally: just having a role (which you can have at any context), and having an enrolment (which you can have only at course level). I think these should use the same fundamental system.

What I think should happen, option 1

We can solve the two above problems - performance and list including everyone who can see the course - very easily without making any such changes.

• Add a column 'participant' to the mdl_role table. This indicates whether the role is considered to be a 'participant' 1 or something else (manager, teacher, administrator, whatever) 0.

It is then possible to get a list of all participants on a course, supposing that you already have the context, as follows:

SELECT 
   ra.userid
FROM
   role_assignments ra
   INNER JOIN role r ON r.id = ra.roleid
WHERE
   ra.contextid IN (context path id list from context)
   AND ra.active = 1
   AND ra.hidden = 0
   AND r.participant = 1

As you can see this query should be very fast and it's easy to join it with other things. This still permits participants to be assigned at category level (if you want your students, with a single assignment, to be participants of all courses in a category) as at present.

You can also quickly get the list of all a user's direct courses:

SELECT c.*
FROM
   role_assignments ra
   INNER JOIN role r ON r.id = ra.roleid
   INNER JOIN context x ON x.id = ra.contextid
   INNER JOIN course c ON c.id = x.instanceid
WHERE
   ra.userid = user ID
   AND ra.active = 1
   AND ra.hidden = 0
   AND r.participant = 1
   AND x.contextlevel = 50

This is a cheat, though, because category and site assignment still makes things a little difficult. However, I don't think Petr's system allows category/site participation at all.

The best-performing way to handle category and site assignment would probably be to do this in PHP, checking first for site assignment (then just use get_records of mdl_course) and otherwise just doing the above query plus a category-specific one that handles complex details like category nesting.

Category nesting can cause minor performance issues. These could be entirely resolved via a new table that maps ancestor category to all descendant categories if required.

What I think should happen, option 1b

(This came out of some discussion in which Martin, Petr were involved.)

The 'participant' flag could actually go at role assignment level (to replace/invert the current 'hidden' flag). It would still be advantageous to have a field at role level but this would now only indicate the default value in the UI - so that when you assign users to a role, the default 'participant' value is 1 for student/teacher and 0 for examiner or something.

What I think should happen, option 2

It may be that the above queries are too complex to join for certain special cases (primarily the My Moodle page) where you actually need the 'get all my courses' query joined with other things.

In this case it would be advantageous to cache course participation information. My proposal for that would be as follows:

• Create a new table course_participants with fields userid, courseid

• Every time a role is assigned/unassigned/modified (ie active, participant/hidden change etc), refresh the cache for that user [note - for batch assign/unassign, do this after batch completes in case there are multiple assigns/unassigns for one user]

• Every time a course is moved to a different category, refresh the cache for that course.

• Every time a category is moved to a different parent category, refresh the cache for all contained courses.

• Fill the cache for a user via the logic above.

This table is simple to understand and join with, will not take a large amount of space per row (important given DB size concerns re the likely number of rows), and duplicates an absolute minimum of the data from role_assignments.

Permissions and participants

I think permission should always be checked by has_capability (+ groups if applicable) and never by a user's presence or absence on the participants list.

For example you might have permission to view a course but not be a participant on it. You might also be a participant on the course, but not have permission to view it. (E.g. temporary ban.)

This is the same as the current system.

Currently, guest user access is handled as follows:

• If a user visits a course they don't have course:view permission in, and the course allows guest access, then the user is given a choice to access it as guest (or this may happen automatically).

• User is temporarily granted the course:view permission on the course.

I don't see a big problem with this either.

Discarding old data

At present some user data is deleted when they have no more role assignments on a course. I would suggest removing the connection with role assignments and allowing enrolment plugins/similar systems to trigger a process to delete user data whenever they feel like it.

For example this could result in an option on the standard 'assign users' screen to 'Permanently delete course data related to user' when you unassign users. If ticked this would run after manually unassigning roles, checking for each deleted user that they don't have any other roles, then deleting data. If not ticked data would be retained for use if the user is added later.

The enrol plugins problem

I don't know much about enrol plugins but agree with Petr's suggestion that they may need a place which isn't role_assignments to store their data (as they may relate to multiple, or no, roles). I don't think this should be related to the above problems with lists of course participants. It can be solved independently.

I would make the following changes:

• Add an enrolid field to role_assignments so we can track which role assignments a particular enrol plugin instance owns. [UI note: If this is set, then the enrol plugin should be able to prevent manual actions to remove the user.]

• Ideally remove the course_participants table (this already doesn't include a course field, and with my proposal above, would not define participants). I agree enrolment plugins might need to store information about enrolled users but they also might not, or might be able to derive that information from the role assignments.

This saves a lot of database space - in our case role_assignments has ~700k rows, and adding another ~500k rows of basically duplicate data to a new table doesn't seem like a great plan.

• If not removing course_participants, then the 'status' field could probably be removed as this would not control course access under my scheme. Enrol plugins can instead toggle the existing 'active' field on role_assignments for their role assignments.