Release/v1.5 r7 - #85
Open
jrchamp wants to merge 14 commits into
Open
Conversation
Rewrites import_courselibrary_search::get_searchsql() so the main query no
longer joins {course_modules}, every activity table, {tag_instance}, {tag}
or {customfield_data} on every page load. Tag, course-module-tag, activity
name/intro and custom-field matching now happen via small index-friendly
subqueries combined into c.id IN (... UNION ...). Capability and custom-
field filters are EXISTS clauses, removing the row fan-out that previously
required DISTINCT.
search() now caches the total in $totalcount_full and runs a direct
COUNT(*) against the lean WHERE; get_total_course_count() reuses the
cached value so the search SQL runs twice per request instead of three
times.
No schema, settings, template or JS changes. Functional parity preserved:
same WHERE semantics, same disableactivitydescriptionsearch honour, same
sort/filter/capability behaviour.
Moves the per-course render-time work out of the initial library page load. - export_for_template() no longer calls get_course_contents() up front. The accordion is rendered as an empty shell with a data-courseid hook; the contents are fetched via a new fragment when the user first clicks "Show contents". On large sites this eliminates ~all of the 5000+ DB reads and 10000+ filter instantiations caused by iterating every activity in every course on the page. - New fragment get_library_coursecontents renders the accordion partial (local_kickstart_pro/import_course_contents) for a single course. - Tags and custom-field instance data for the page are loaded in batch before the export loop, replacing per-course N+1 queries. - Relevance scoring is rewritten as a single batch pass (calculate_relevance_scores_batch) that loads tags, favourites and customfield data once for current course + result set, replacing the per-result get_relevance_score() that re-fetched the current course and re-ran find_all_favourites on every iteration. - AMD module toggles the accordion and lazy-fetches the fragment on first expand; subsequent clicks just flip visibility. Build produced with grunt amd.
Two follow-up reductions to the per-page render query count.
- The search SELECT now includes c.category, c.idnumber, c.startdate,
c.summary and c.summaryformat. core_course_list_element::__get()
lazy-loads any course column missing from the record via
$DB->get_field('course', ...), so on the default-config page
($courseinfo->category is read to build the category path) every
course on the page issued one extra row read. Selecting the columns
up-front removes 10-30 reads depending on which display fields are
configured.
- load_customfields_for_courses() now runs a single
SELECT FROM {customfield_data} WHERE instanceid IN (...)
for the visible page and builds data_controller instances locally via
data_controller::create($id=0, $record, $field), passing the
pre-fetched field controller so no further DB queries are issued.
Previously the handler's get_instance_data() was called once per
course, costing ~one read per course.
- Rename member variable $totalcount_full to $totalcountfull (Moodle naming convention disallows underscores in member variable names). - Drop column-alignment spaces in get_search_where() so each sql_like() argument has a single space after the comma. - Wrap the multi-line if condition in calculate_relevance_scores_batch() so the first expression sits on its own line after the opening parenthesis (PSR12.ControlStructures.ControlStructureSpacing). - Capitalise the inline comment introducing the static cache note on core_course_category::get(). - Reformat the debugging() call in load_customfields_for_courses() to split arguments across lines with the closing paren on its own line (PSR2.Methods.FunctionCallSignature). No functional change.
Adding c.startdate, c.idnumber, c.summary, c.summaryformat and c.category to the search SELECT (commit 28aad27) avoided per-course lazy loads through core_course_list_element::__get(), but as a side effect populated $course->startdate / $course->idnumber on the result row before export_for_template() decided whether to render them. On sites whose displaycourselibraryfields setting did not include "startdate" or "idnumber", the if-blocks that called userdate() / read $courseinfo->idnumber never ran, so the raw integer timestamp (or raw idnumber string) stayed on $course and the Pro template's {{#startdate}} / {{#idnumber}} truthiness check then printed it. m45 hid the bug because its display configuration included "startdate". Clear those five columns from $course immediately after constructing the core_course_list_element wrapper. The wrapper holds its own copy of the record, so $courseinfo->startdate / ->idnumber / ->category still work as before; only the public properties on $course are reset, which means the template renders idnumber/startdate strictly when the display-field branches populate them.
Add a global admin setting that controls the order of the Kickstart navigation pages (course template, student view, help, and the Pro course library / create template pages) in the tertiary navigation dropdown. The order also determines the default page: the visible page with the lowest order is shown first. A page set to order 0 is hidden. When the configured order leaves only a single visible page, the dropdown collapses to a plain page title instead of a pointless single-option selector. - lib.php: central page list + ordering/visibility/default helpers, and order-aware menu builders - settings.php: numeric "order" field per available page - format.php / list.php: default nav derived from the configured order - general_action_bar: render a title when only one page is visible - PHPUnit coverage for ordering, hiding, default and the single-item bar Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Replace the free-text order inputs with a select dropdown offering positions 1..N (one per available page) plus a "Hide" option for 0, so admins pick a valid position instead of typing an arbitrary number. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
core/modal_factory was removed in Moodle 5.2, so the formatkickstart AMD module failed to load (breaking the tertiary navigation and all Behat scenarios on 5.2). Migrate to core/modal_save_cancel, which exists since 4.3, and rebuild the compiled AMD. Bump the release name to 1.6. Verified on a Moodle 5.2 site: the previously failing Behat scenarios pass. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Incorporates #81 #82 and cherry-picks one commit from #83, adding Moodle 5.2 support without dropping 4.5 support just yet.