KICK-300 Course Library performance optimisations

[stefanscholz]

Context

The Course Library page (kickstart format, ?nav=courselibrary) takes 10+ seconds to render on a customer site with 60k courses and 2.5M activities. This PR reduces that to under 1 second.

	DB reads	DB time	Total page time
main	5129	2.35 s	3.77 s
KICK-300	~830	~0.35 s	~0.7 s

This depends on a matching change in local_kickstart_pro (also KICK-300). The Pro template splits the per-section accordion into a partial so the format-side fragment can render it on demand. Both PRs must be merged together — format ships the fragment that fills the partial; Pro ships the partial.

Summary of changes

Five commits, each one a discrete optimisation. They build on each other but each is meaningful on its own.

1. 84aa7ba Rewrite the search SQL

classes/output/import_courselibrary_search.php previously built one large query that always LEFT-JOINed {course_modules} + a UNION ALL across every activity table + {tag_instance} × 2 + {tag} × 2 + {customfield_data} + {customfield_field}, then deduped with DISTINCT c.id. With an empty search box the placeholder LIKE '%%' matched everything, so the database materialised the full Cartesian product on every landing page. The same query was executed three times per request (data + two count subqueries).

After:

• The main query touches only {course}, {context} and {user_lastaccess} — no DISTINCT, no fan-out joins.
• Capability filter is an EXISTS subquery (no row multiplication).
• Search term resolves to a c.id IN (... UNION ...) clause where each branch is a small index-friendly query (course fields, course tags, course-module tags, one query per activity table). The disableactivitydescriptionsearch setting still gates the activity-intro branch.
• Custom-field filter is an EXISTS per filter.
• Count uses a direct SELECT COUNT(*) FROM ... WHERE against the lean query and is cached so the SQL runs twice per request instead of three times.

Functional parity preserved: same WHERE semantics, same sort/filter/capability behaviour, same disableactivitydescriptionsearch handling.

2. 276cacc Lazy-load course contents + batch render-path queries

classes/output/import_course_list.php, classes/output/import_courselibrary_search.php, lib.php, amd/src/formatkickstart.js

export_for_template() no longer calls get_course_contents() for every course on the page. The Pro template now emits an empty accordion shell with data-courseid/data-loaded attributes; the new fragment format_kickstart_output_fragment_get_library_coursecontents renders the accordion partial on first expand. On large sites this single change removes ~all 5000+ DB reads and ~10000+ filter instantiations the previous loop produced.

Inside export_for_template:

• Tag lookup moved to a batch loader (one WHERE itemid IN (...) query for the page instead of \core_tag_tag::get_item_tags_array() per course).
• Custom-field instance data preloaded in a single batch before the loop.

In search():

• New calculate_relevance_scores_batch() replaces the per-result get_relevance_score() call. Runs at most 4 queries for the whole page (current course, tags batch, customfield batch, favourites) instead of N×~10. Same scoring formula; only the query pattern changed. get_relevance_score() left intact in case third-party callers exist.

The AMD module wires the first click of .show-content-button to Fragment.loadFragment(...) and Templates.appendNodeContents into the empty shell. After that click, data-loaded is set so subsequent clicks only flip d-none. The .import-activity handler is rebound on the freshly loaded partial. Build artefacts produced with grunt amd.

3. 28aad27 Batch customfield data + select extra course columns

• Search SELECT extended to include c.category, c.idnumber, c.startdate, c.summary, c.summaryformat. \core_course_list_element::__get() lazy-loads any column missing from the wrapped record via $DB->get_field('course', $name, ...); with the default config (categorypath displayed) every course on the page hit this for category. Selecting the columns up-front removes the lazy load.
• load_customfields_for_courses() rewritten to run one SELECT FROM {customfield_data} WHERE instanceid IN (...) and build \core_customfield\data_controller instances locally via data_controller::create(0, $row, $field) with the pre-fetched field controller — no further DB calls per row. Previously the handler's get_instance_data() was called per course.

4. 88347c2 Fix Moodle Code Checker findings

Pure style, no behaviour change. Renames $totalcount_full → $totalcountfull (Moodle disallows underscores in member variable names), collapses column-alignment whitespace, wraps a multi-line if so the first expression sits on its own line, capitalises an inline comment, reformats a debugging() call to follow PSR2.

5. affe3ec Fix raw startdate/idnumber leaking into the library template

Regression from commit 3: now that c.startdate and c.idnumber are selected up-front, the raw integer timestamp / idnumber string stays on $course when the admin's displaycourselibraryfields setting doesn't include them. The Pro template's {{#startdate}}…{{startdate}}…{{/startdate}} block then prints the raw value (a Unix timestamp leaked into the UI as a 10-digit integer).

Fix: immediately after constructing $courseinfo = new \core_course_list_element($course), unset($course->idnumber, $course->startdate, $course->summary, $course->summaryformat, $course->category). The list-element wrapper keeps its own copy of the record, so $courseinfo->startdate etc. still work; only the public properties on $course are reset, which means the template renders these fields strictly when the display-field branches populate them.

What was deliberately NOT changed

• Default sort remains relevance. The Pro entry point (kickstart_propage.php:203) still hard-codes 'relevance'.
• get_relevance_score() is kept on the class but no longer called from this plugin. Removable in a follow-up if no external callers exist.
• The per-course format_string(\$course->fullname, ['context' => \context_course::instance(\$course->id)]) still loads its own filter set per course context. Removing it would change filter semantics for sites with per-category filter configuration — out of scope.

Test plan

• Open the library on a course as a non-admin with moodle/backup:backuptargetimport. Confirm pagination total + result set match the previous version.
• Search by course full name, short name, summary, course tag, activity name, activity intro and a course-module tag. Each should hit the corresponding branch.
• Toggle disableactivitydescriptionsearch and confirm intro matches stop returning results.
• Apply a custom-field filter and confirm results narrow.
• Switch sort: Alphabetical, Last accessed, Relevance.
• Click "Show contents" on a course → accordion populates with sections/activities. Click again to collapse, again to expand → no second fetch.
• Use the "Import activity" action inside the (lazily loaded) accordion.
• Confirm the customer-reported regression no longer happens: on an install where displaycourselibraryfields does not include startdate, the start date area no longer renders the raw Unix timestamp.
• Measure DB reads/time on a large library page; expect the order-of-magnitude reduction shown in the table above.

Deploy / merge

Merge this in lockstep with the KICK-300 PR on moodle-local_kickstart_pro. Either order is fine, but do not deploy one without the other:

• format alone → library cards render but the accordion never populates (the partial that the fragment renders doesn't exist).
• pro alone → empty accordion shells with no fragment to fill them (clicking "Show contents" no-ops because the fragment endpoint doesn't exist).

[stefanscholz]

Task linked: KICK-300 Course library performance improvements

[phakkin]

Please merge this PR to to a new Kickstart Pro release that is still Moodle 4.5 LTS compatible. We have a mid-size Moodle installation (30k courses) and Course Library is pretty much unusable with current 1.5 R6 version: loading and updating course list takes 1-2 minutes or causes database error.