get_admins() and get_admin() were counting on
get_users_by_capability() returning a role-assignment id to pick the
"primary" admin account. With the get_users_by_capability() rewrite,
we no longer have an RA id to clearly blame for the capability.
So, rewrite get_admins() based on the known-good SQL used in
is_siteadmin().
MDL-12452
Oracle does not accept column labels in the GROUP BY section -
get_logs_usercourse() and get_logs_userday() were computing the
day/hours column and using it in the GROUP BY. The compatible fix
is to reproduce the same computation in the GROUP BY section.
Credit for the fix goes to Dennis Rochford <Dennis.Rochford@usq.edu.au>.
make_context_subobj() was not providing a contextlevel property, and
no callers fetched the field. This comes from its humble origins where
it was only ever called for course objects. These days it's used in
many other situations, so this patch DTRT and
- fixes make_context_subobj() expect a cxtlevel and turn it into
contextlevel
- fixes all callers (accesslib, datalib) to provide it
Having no roles set as coursemanager is a valid setting.
get_courses_wmanagers() should not produce invalid SQL on it...
actually, it should not even try to get the course managers.
With this patch, get_categories() now adds a nice context
sub-object to the returned object, which means callers can
save DB traffic.
It now also supports "deep" retrievals, which means we can
rewrite the course categories display pages to avoid
costly recursion.
Don't update fields unnecessarily. Cuts 3 DB queries per category
on course/index page (45 in a 15 category setup).
fix_course_sortorder() should be fixed to avoid recursion.
For an efficient print_courses() we need to grab in a constant number
of queries...
- course data
- "course manager" role assignments
- user records for the coursemanagers' fullname()
So here we do it in 2 DB queries. The 2nd one (grabbing RAs and user
records) can be expensive if we are dealing with a large number of
courses.
Which we shouldn't - When the number of courses is large the course
listing doesn't come this way anyway...
Reworked course_parent_visible() to always return in a constant
number of db queries (2 worst case) regardless of nesting depth.
The rewritten version has a small cache, but if you are going to
walk many courses, it's still 1~2 DB queries per category seen,
so the right thing to do is to check it in the caller, as seen
in get_my_courses().
Reworked gmc to perform the course visibility checks. These are
very cheap if $CFG->allowvisiblecoursesinhiddencategories is true.
However, where we have to enforce category visibility, it adds a bit
of work. In simple terms, it adds a DB query to read all the categories,
and extra checks to make sure we are doing the right thing WRT
- course visibility vs the permission to see hidden courses
- category visibility vs the permission to see hidden categories
and still do it quickly.
This patch saves 1600 context lookups on a 1600 course category. rcache
does help a bit with small categories, but on large setups, this is
not sustainable.
And it's not needed either. We have the data right at our fingertips.
Just get it when it's cheap...
A walkthrough of course-login-as functionality shows that is
Just Works, except that get_my_courses() was showing all the
courses. So we fix it.
And cleanup load_all_capabilities() - things just work
transparently.
- Field handling moves back to get_my_courses() and now we have
almost all the fields that the old get_my_courses() did
(except for summary, which is *huge*) so get_my_courses() asks
for a lot of fields, but the get_user_courses_bycap() defaults
are _much_ leaner now.
I think this makes sense ;-)
- get_my_courses() now caches the course ids for the currently logged in
user in $USER->mycourses -- as a _string_. This is magnitudes more efficient
than having it as an array.
The cache makes a difference, but it's not very visible on
normal pageloads (with my courses block, for example).
However, over 100 iterations, for a user with 50 enrolments in a site
with 6K courses, we go from 4.3s to 0.6s. And the DB queries are *cheap*.
$tt = microtime(true);
for($n=0;$n<100;$n++) {
get_my_courses($USER->id, 'sortorder ASC');
}
error_log("took " . (microtime(true) - $tt));
get_user_courses_bycap() replaces get_courses_bycap_fromsess().
Using a combination of in-DB enrolments and in-session capability
checks, we narrow down the courses we fetch from the DB for checking.
This patch adds a small DB query, and has has a moderate impact on
the timings observable on my laptop (~300ms?), but reduces
*significantly* the bandwidth used, which in cluster environments
with frontends separate from backends has a serious impact.
get_my_courses() goes from a bazillion queries (500 in some sample
cases) to 1 for the logged-in user, and 4 for a non-logged-in user.
One of those queries brings a *lot* of data across (all rows from
mdl_course) so there is room for serious optimisation.
However, this clocks at ~300 ms on my laptop, costly, but not
the end of the world. If your PHP-DB link has bandwidth probs
it might be a problem.
A few important changes to get_my_courses()
- (Compat ALERT!) the default fields are less than before --
(will be followed by patches that fix the callers!) our defaults
had grown to quite a bit because of the crazy caching scheme it had
- the $fields parameter is to name _additional_ fields you need, and
ideally wants them passed as an array. Will cope with old-style
strings too.
- the returned courses have an extra property "context" that is
a fully valid context object, so if the caller needs to perform
further accesslib checks, we save a query per course down the road
The work is done in the newfangled get_courses_bycap_fromsess()
which is brute-force but fast. I'm sure we can optimise the common
cases a lot in it if we try. It'd be worthwhile to
- reduce the rows we grab - that's really boneheaded
- if we copy and tweak the logic of has_cap_fromsess() in it
it can be made even faster
