[RFC] get_my_courses() - simpler, cheaper

[RFC] get_my_courses() - simpler, cheaper

by Martín Langhoff -
Number of replies: 8
With the new roles infrastructure, get_my_courses() turns out to be rather expensive on large sites. I am finding it a bit of a struggle to figure out a cheap way of answering 'which courses is this user related to'. It is one of our key operations so I want to find a good way of doing this we can use across the codebase.

The current my_courses has two problems:

- it gets all the fields on mdl_course
- it gets all the rows of mdl_course

the attached patch names a few fields, and selects all courses the user has an explicit relationship with. My logic is that we are interested in courses we have a _direct_ relationship -- indirect relationships (such as "I'm the admin") don't indicate a specific interest in that course. So if I am the site admin, and I am a teacher on course X, and a student in course Y, course X and Y will be listed, and nothing else.

It also discourages people from doing too much with indirect relationships which are really costly to keep track of wide eyes

One possible improvement would be to JOIN against mdl_roles as well to get the name of the role for display purposes (assuming the shortname is something we can "resolve" against the language packs).

I think the patch makes sense but I'm such an ignorant when it comes to roles wink therefore this Request For Comments.

Edit: colour coded patch here http://git.catalyst.net.nz/gitweb?p=moodle-r2.git;a=commitdiff;h=aa86af11edda901b39496aca13bbb51892752f8a;hp=758c40dbb788d015ac6a7767a9363d804805ee5b
Average of ratings: -
In reply to Martín Langhoff

Re: [RFC] get_my_courses() - simpler, cheaper

by Martin Dougiamas -
Picture of Core developers Picture of Documentation writers Picture of Moodle HQ Picture of Particularly helpful Moodlers Picture of Plugin developers Picture of Testers
The problem here is that "indirect" enrolments as you call them (where, for example, a student is enrolled in the category and thus all the courses in that category) are a perfectly valid way of being enrolled now, due to the new inheritance model of roles.

So this patch is ignoring those possibilities (which can be expensive to calculate, I know).

Perhaps there are other ways to optimise the specific situations you are calling get_my_courses from (such as caching).
Average of ratings: -
In reply to Martin Dougiamas

Re: [RFC] get_my_courses() - simpler, cheaper

by Martín Langhoff -
After a bit of a chat with MartinD the conclusion is that I can probably come up with a more convoluted bit of SQL that covers all bases and is actually correct (the one I propose above isn't sad )

"Cover all the bases" in terms of enrolment means the following contexts:

- Site
- Sitegroups (in 1.8?)
- Category
- Course

In small/medium sized moodle installs, this will probably be about the same as we have now. On large-ish installations, it will be a ton faster. Now all I need is some time to hide in my dungeon and come up with the right incantation.
Average of ratings: -
In reply to Martín Langhoff

Re: [RFC] get_my_courses() - simpler, cheaper

by Robert Brenstein -
Hmm, the above "Cover all the bases" list covers all the options that can give student access to a course, I mean allow a student to enroll. A manual enrollment by teacher/admin is covered by the last item I presume.

I recognize that most sites do want students to be automatically enrolled in all the courses they are allowed to access, it is not always the case. Or are we the only exception in the world?

What I am trying to say is that as long as there is an explicit enrollment step, we should be able to distinguish courses which students are entitled to join from courses they actually joined. In my mind, get_my_courses() should get only the latter.

Similar thing applies to list of participants as shown in the course. I would hate it to show me the students that are entitled to enroll as opposed to students that actively enrolled.
Average of ratings: -
In reply to Robert Brenstein

Re: [RFC] get_my_courses() - simpler, cheaper

by Martín Langhoff -
This is more about supporting the fact that in 1.7, you can enrol a student to a category, and he/she'll be enrolled in all the courses in that category. Or you can designate a user to be student 'sitewide' which means enrolment to all courses.

It is really useful, but it makes it costly to get those 'indirect' relationships from the DB. I am trying to figure out the cheapest way to do it wink

{Edit: so, to clarify, I am not hunting for potential students but for enrolled students. Now, the whole concept of enrolled students has become a whole lot fuzzier wide eyes }
Average of ratings: -
In reply to Martín Langhoff

Re: [RFC] get_my_courses() - simpler, cheaper

by Robert Brenstein -
The distinction will become absolutely critical when max enrollment and similar features are finally considered for inclusion (cf. for example http://moodle.org/mod/forum/discuss.php?d=59529).

Technically, the solution may be not that difficult: roles should be used when determining whether student is allowed to enroll or not (same for the automatic across-board enrollments) but student enrollments should be maintained explicitely for each course (as it used to be before roles).

I think this would eliminate the overhead you worry about since the real extensive work will be occuring only once at the enrollment time. And here, each enrollment plugin can use its smarts and shortcuts to accomplish the goal.

Note that by enrollment here I mean the act of allowing someone to access the course in student mode, whatever student mode in a given course means. I say that because roles de facto created almost a continuum in capabilities, so yes, the concept of enrolled students has become fuzzy and may need to be re-clarified.

May be each role should be classified as admin, teacher, or student role aside from its capabilities and context scope.
Average of ratings: -
In reply to Robert Brenstein

Re: [RFC] get_my_courses() - simpler, cheaper

by Martin Dougiamas -
Picture of Core developers Picture of Documentation writers Picture of Moodle HQ Picture of Particularly helpful Moodlers Picture of Plugin developers Picture of Testers
> May be each role should be classified as admin, teacher, or student role aside from its capabilities and context scope.

You can do this using the "legacy" capabilities which let you "tag" roles with one of these older labels.

But about category-level and site-level enrolments, there is no need for you to use these if it doesn't make sense on your site. The core enrolment plugins can't handle these yet anyway (they can only enrol people into courses same as always). You only need to use them when it makes perfect sense that a user needs to be a "student", say, in all the courses within a category (even if courses are added later). In such cases there is no distinction at all in the "type" of enrolment - you are an equal participant either way.
Average of ratings: -
In reply to Martin Dougiamas

Re: [RFC] get_my_courses() - simpler, cheaper

by Robert Brenstein -
Legacy roles: I was not talking about the labels or tags but real role categories that can be taken advantage of when coding. Sort of like another context of a role. I must admit that I haven't got into the roles thingie deep enough yet to follow all implications, but such a continuum we have now feels wrong in a learning setting where there are always distinct role types: somebody is a teacher/instructor/trainer and somebody is a pupil/student/trainee.

Category-level and site-level enrolments: I think that like Martin (albeit from a different angle) I am concerned about performance if such broad checks have to be done over and over for each student entering a course, and that is what I understand is occurring. The capability to extend enrollment for future so do speak is fantastic, but I still think the work should be done only upon specific request or circumstance.
Average of ratings: -