Merge branch 'MDL-69206-master' of git://github.com/mihailges/moodle
authorJake Dallimore <jake@moodle.com>
Wed, 15 Jul 2020 02:29:03 +0000 (10:29 +0800)
committerJake Dallimore <jake@moodle.com>
Wed, 15 Jul 2020 02:29:03 +0000 (10:29 +0800)
15 files changed:
admin/tool/mobile/settings.php
completion/tests/behat/completion_other_courses.feature [new file with mode: 0644]
course/classes/management/helper.php
course/classes/management_renderer.php
course/completion_form.php
lib/db/upgrade.php
search/classes/engine.php
search/classes/manager.php
search/engine/solr/classes/engine.php
search/engine/solr/tests/engine_test.php
search/upgrade.txt
theme/boost/scss/moodle/course.scss
theme/boost/style/moodle.css
theme/classic/style/moodle.css
version.php

index 90de327..c2cee8c 100644 (file)
@@ -48,125 +48,146 @@ if ($hassiteconfig) {
 
     $ADMIN->add('mobileapp', $temp);
 
-    // Show only mobile settings if the mobile service is enabled.
-    if (!empty($CFG->enablemobilewebservice)) {
+    $featuresnotice = null;
+    if (empty($CFG->disablemobileappsubscription)) {
+        // General notification about limited features due to app restrictions.
+        $subscriptionurl = (new moodle_url("/$CFG->admin/tool/mobile/subscription.php"))->out(false);
+        $notify = new \core\output\notification(
+            get_string('moodleappsportalfeatureswarning', 'tool_mobile', $subscriptionurl),
+            \core\output\notification::NOTIFY_WARNING);
+        $featuresnotice = $OUTPUT->render($notify);
+    }
+
+    $hideappsubscription = empty($CFG->enablemobilewebservice);
+    $hideappsubscription = $hideappsubscription || (isset($CFG->disablemobileappsubscription) && !empty($CFG->disablemobileappsubscription));
+
+    $ADMIN->add(
+        'mobileapp',
+        new admin_externalpage(
+            'mobileappsubscription',
+            new lang_string('mobileappsubscription', 'tool_mobile'),
+            "$CFG->wwwroot/$CFG->admin/tool/mobile/subscription.php",
+            'moodle/site:config',
+            $hideappsubscription
+        )
+    );
+
+    // Type of login.
+    $temp = new admin_settingpage(
+        'mobileauthentication',
+        new lang_string('mobileauthentication', 'tool_mobile'),
+        'moodle/site:config',
+        empty($CFG->enablemobilewebservice)
+    );
+
+    $temp->add(new admin_setting_heading('tool_mobile/moodleappsportalfeaturesauth', '', $featuresnotice));
+
+    $options = array(
+        tool_mobile\api::LOGIN_VIA_APP => new lang_string('loginintheapp', 'tool_mobile'),
+        tool_mobile\api::LOGIN_VIA_BROWSER => new lang_string('logininthebrowser', 'tool_mobile'),
+        tool_mobile\api::LOGIN_VIA_EMBEDDED_BROWSER => new lang_string('loginintheembeddedbrowser', 'tool_mobile'),
+    );
+    $temp->add(new admin_setting_configselect('tool_mobile/typeoflogin',
+                new lang_string('typeoflogin', 'tool_mobile'),
+                new lang_string('typeoflogin_desc', 'tool_mobile'), 1, $options));
+
+    $options = [
+        tool_mobile\api::QR_CODE_DISABLED => new lang_string('qrcodedisabled', 'tool_mobile'),
+        tool_mobile\api::QR_CODE_URL => new lang_string('qrcodetypeurl', 'tool_mobile'),
+        tool_mobile\api::QR_CODE_LOGIN => new lang_string('qrcodetypelogin', 'tool_mobile'),
+    ];
+    $temp->add(new admin_setting_configselect('tool_mobile/qrcodetype',
+                new lang_string('qrcodetype', 'tool_mobile'),
+                new lang_string('qrcodetype_desc', 'tool_mobile'), tool_mobile\api::QR_CODE_LOGIN, $options));
+
+    $temp->add(new admin_setting_configtext('tool_mobile/forcedurlscheme',
+                new lang_string('forcedurlscheme_key', 'tool_mobile'),
+                new lang_string('forcedurlscheme', 'tool_mobile'), 'moodlemobile', PARAM_NOTAGS));
+
+    $temp->add(new admin_setting_configtext('tool_mobile/minimumversion',
+                new lang_string('minimumversion_key', 'tool_mobile'),
+                new lang_string('minimumversion', 'tool_mobile'), '', PARAM_NOTAGS));
+
+    $ADMIN->add('mobileapp', $temp);
+
+    // Appearance related settings.
+    $temp = new admin_settingpage(
+        'mobileappearance',
+        new lang_string('mobileappearance', 'tool_mobile'),
+        'moodle/site:config',
+        empty($CFG->enablemobilewebservice)
+    );
+
+    if (!empty($featuresnotice)) {
+        $temp->add(new admin_setting_heading('tool_mobile/moodleappsportalfeaturesappearance', '', $featuresnotice));
+    }
+
+    $temp->add(new admin_setting_configtext('mobilecssurl', new lang_string('mobilecssurl', 'tool_mobile'),
+                new lang_string('configmobilecssurl', 'tool_mobile'), '', PARAM_URL));
+
+    // Reference to Branded Mobile App.
+    if (empty($CFG->disableserviceads_branded)) {
+        $temp->add(new admin_setting_description('moodlebrandedappreference',
+            new lang_string('moodlebrandedapp', 'admin'),
+            new lang_string('moodlebrandedappreference', 'admin')
+        ));
+    }
+
+    $temp->add(new admin_setting_heading('tool_mobile/smartappbanners',
+                new lang_string('smartappbanners', 'tool_mobile'), ''));
 
-        $featuresnotice = null;
-        if (empty($CFG->disablemobileappsubscription)) {
-            // General notification about limited features due to app restrictions.
-            $subscriptionurl = (new moodle_url("/$CFG->admin/tool/mobile/subscription.php"))->out(false);
-            $notify = new \core\output\notification(
-                get_string('moodleappsportalfeatureswarning', 'tool_mobile', $subscriptionurl),
-                \core\output\notification::NOTIFY_WARNING);
-            $featuresnotice = $OUTPUT->render($notify);
-
-            $ADMIN->add('mobileapp', new admin_externalpage('mobileappsubscription',
-                new lang_string('mobileappsubscription', 'tool_mobile'),
-                "$CFG->wwwroot/$CFG->admin/tool/mobile/subscription.php"));
-        }
-
-        // Type of login.
-        $temp = new admin_settingpage('mobileauthentication', new lang_string('mobileauthentication', 'tool_mobile'));
-
-        $temp->add(new admin_setting_heading('tool_mobile/moodleappsportalfeaturesauth', '', $featuresnotice));
-
-        $options = array(
-            tool_mobile\api::LOGIN_VIA_APP => new lang_string('loginintheapp', 'tool_mobile'),
-            tool_mobile\api::LOGIN_VIA_BROWSER => new lang_string('logininthebrowser', 'tool_mobile'),
-            tool_mobile\api::LOGIN_VIA_EMBEDDED_BROWSER => new lang_string('loginintheembeddedbrowser', 'tool_mobile'),
-        );
-        $temp->add(new admin_setting_configselect('tool_mobile/typeoflogin',
-                    new lang_string('typeoflogin', 'tool_mobile'),
-                    new lang_string('typeoflogin_desc', 'tool_mobile'), 1, $options));
-
-        $options = [
-            tool_mobile\api::QR_CODE_DISABLED => new lang_string('qrcodedisabled', 'tool_mobile'),
-            tool_mobile\api::QR_CODE_URL => new lang_string('qrcodetypeurl', 'tool_mobile'),
-            tool_mobile\api::QR_CODE_LOGIN => new lang_string('qrcodetypelogin', 'tool_mobile'),
-        ];
-        $temp->add(new admin_setting_configselect('tool_mobile/qrcodetype',
-                    new lang_string('qrcodetype', 'tool_mobile'),
-                    new lang_string('qrcodetype_desc', 'tool_mobile'), tool_mobile\api::QR_CODE_LOGIN, $options));
-
-        $temp->add(new admin_setting_configtext('tool_mobile/forcedurlscheme',
-                    new lang_string('forcedurlscheme_key', 'tool_mobile'),
-                    new lang_string('forcedurlscheme', 'tool_mobile'), 'moodlemobile', PARAM_NOTAGS));
-
-        $temp->add(new admin_setting_configtext('tool_mobile/minimumversion',
-                    new lang_string('minimumversion_key', 'tool_mobile'),
-                    new lang_string('minimumversion', 'tool_mobile'), '', PARAM_NOTAGS));
-
-        $ADMIN->add('mobileapp', $temp);
-
-        // Appearance related settings.
-        $temp = new admin_settingpage('mobileappearance', new lang_string('mobileappearance', 'tool_mobile'));
-
-        if (!empty($featuresnotice)) {
-            $temp->add(new admin_setting_heading('tool_mobile/moodleappsportalfeaturesappearance', '', $featuresnotice));
-        }
-
-        $temp->add(new admin_setting_configtext('mobilecssurl', new lang_string('mobilecssurl', 'tool_mobile'),
-                    new lang_string('configmobilecssurl', 'tool_mobile'), '', PARAM_URL));
-
-        // Reference to Branded Mobile App.
-        if (empty($CFG->disableserviceads_branded)) {
-            $temp->add(new admin_setting_description('moodlebrandedappreference',
-                new lang_string('moodlebrandedapp', 'admin'),
-                new lang_string('moodlebrandedappreference', 'admin')
-            ));
-        }
-
-        $temp->add(new admin_setting_heading('tool_mobile/smartappbanners',
-                    new lang_string('smartappbanners', 'tool_mobile'), ''));
-
-        $temp->add(new admin_setting_configcheckbox('tool_mobile/enablesmartappbanners',
-                    new lang_string('enablesmartappbanners', 'tool_mobile'),
-                    new lang_string('enablesmartappbanners_desc', 'tool_mobile'), 0));
-
-        $temp->add(new admin_setting_configtext('tool_mobile/iosappid', new lang_string('iosappid', 'tool_mobile'),
-                    new lang_string('iosappid_desc', 'tool_mobile'), tool_mobile\api::DEFAULT_IOS_APP_ID, PARAM_ALPHANUM));
-
-        $temp->add(new admin_setting_configtext('tool_mobile/androidappid', new lang_string('androidappid', 'tool_mobile'),
-                    new lang_string('androidappid_desc', 'tool_mobile'), tool_mobile\api::DEFAULT_ANDROID_APP_ID, PARAM_NOTAGS));
-
-        $temp->add(new admin_setting_configtext('tool_mobile/setuplink', new lang_string('setuplink', 'tool_mobile'),
-            new lang_string('setuplink_desc', 'tool_mobile'), 'https://download.moodle.org/mobile', PARAM_URL));
-
-        $ADMIN->add('mobileapp', $temp);
-
-        // Features related settings.
-        $temp = new admin_settingpage('mobilefeatures', new lang_string('mobilefeatures', 'tool_mobile'));
-
-        if (!empty($featuresnotice)) {
-            $temp->add(new admin_setting_heading('tool_mobile/moodleappsportalfeatures', '', $featuresnotice));
-        }
-
-        $temp->add(new admin_setting_heading('tool_mobile/logout',
-                    new lang_string('logout'), ''));
-
-        $temp->add(new admin_setting_configcheckbox('tool_mobile/forcelogout',
-                    new lang_string('forcelogout', 'tool_mobile'),
-                    new lang_string('forcelogout_desc', 'tool_mobile'), 0));
-
-        $temp->add(new admin_setting_heading('tool_mobile/features',
-                    new lang_string('mobilefeatures', 'tool_mobile'), ''));
+    $temp->add(new admin_setting_configcheckbox('tool_mobile/enablesmartappbanners',
+                new lang_string('enablesmartappbanners', 'tool_mobile'),
+                new lang_string('enablesmartappbanners_desc', 'tool_mobile'), 0));
 
-        $options = tool_mobile\api::get_features_list();
-        $temp->add(new admin_setting_configmultiselect('tool_mobile/disabledfeatures',
-                    new lang_string('disabledfeatures', 'tool_mobile'),
-                    new lang_string('disabledfeatures_desc', 'tool_mobile'), array(), $options));
+    $temp->add(new admin_setting_configtext('tool_mobile/iosappid', new lang_string('iosappid', 'tool_mobile'),
+                new lang_string('iosappid_desc', 'tool_mobile'), tool_mobile\api::DEFAULT_IOS_APP_ID, PARAM_ALPHANUM));
 
-        $temp->add(new admin_setting_configtextarea('tool_mobile/custommenuitems',
-                    new lang_string('custommenuitems', 'tool_mobile'),
-                    new lang_string('custommenuitems_desc', 'tool_mobile'), '', PARAM_RAW, '50', '10'));
+    $temp->add(new admin_setting_configtext('tool_mobile/androidappid', new lang_string('androidappid', 'tool_mobile'),
+                new lang_string('androidappid_desc', 'tool_mobile'), tool_mobile\api::DEFAULT_ANDROID_APP_ID, PARAM_NOTAGS));
 
-        $temp->add(new admin_setting_heading('tool_mobile/language',
-                    new lang_string('language'), ''));
+    $temp->add(new admin_setting_configtext('tool_mobile/setuplink', new lang_string('setuplink', 'tool_mobile'),
+        new lang_string('setuplink_desc', 'tool_mobile'), 'https://download.moodle.org/mobile', PARAM_URL));
 
-        $temp->add(new admin_setting_configtextarea('tool_mobile/customlangstrings',
-                    new lang_string('customlangstrings', 'tool_mobile'),
-                    new lang_string('customlangstrings_desc', 'tool_mobile'), '', PARAM_RAW, '50', '10'));
+    $ADMIN->add('mobileapp', $temp);
+
+    // Features related settings.
+    $temp = new admin_settingpage(
+        'mobilefeatures',
+        new lang_string('mobilefeatures', 'tool_mobile'),
+        'moodle/site:config',
+        empty($CFG->enablemobilewebservice)
+    );
 
-        $ADMIN->add('mobileapp', $temp);
+    if (!empty($featuresnotice)) {
+        $temp->add(new admin_setting_heading('tool_mobile/moodleappsportalfeatures', '', $featuresnotice));
     }
+
+    $temp->add(new admin_setting_heading('tool_mobile/logout',
+                new lang_string('logout'), ''));
+
+    $temp->add(new admin_setting_configcheckbox('tool_mobile/forcelogout',
+                new lang_string('forcelogout', 'tool_mobile'),
+                new lang_string('forcelogout_desc', 'tool_mobile'), 0));
+
+    $temp->add(new admin_setting_heading('tool_mobile/features',
+                new lang_string('mobilefeatures', 'tool_mobile'), ''));
+
+    $options = tool_mobile\api::get_features_list();
+    $temp->add(new admin_setting_configmultiselect('tool_mobile/disabledfeatures',
+                new lang_string('disabledfeatures', 'tool_mobile'),
+                new lang_string('disabledfeatures_desc', 'tool_mobile'), array(), $options));
+
+    $temp->add(new admin_setting_configtextarea('tool_mobile/custommenuitems',
+                new lang_string('custommenuitems', 'tool_mobile'),
+                new lang_string('custommenuitems_desc', 'tool_mobile'), '', PARAM_RAW, '50', '10'));
+
+    $temp->add(new admin_setting_heading('tool_mobile/language',
+                new lang_string('language'), ''));
+
+    $temp->add(new admin_setting_configtextarea('tool_mobile/customlangstrings',
+                new lang_string('customlangstrings', 'tool_mobile'),
+                new lang_string('customlangstrings_desc', 'tool_mobile'), '', PARAM_RAW, '50', '10'));
+
+    $ADMIN->add('mobileapp', $temp);
 }
diff --git a/completion/tests/behat/completion_other_courses.feature b/completion/tests/behat/completion_other_courses.feature
new file mode 100644 (file)
index 0000000..66765b0
--- /dev/null
@@ -0,0 +1,30 @@
+@core @core_completion
+Feature: Set completion of other courses as criteria for completion of current course
+  In order to set completion of other courses as criteria for completion of current course
+  As a user
+  I want to select the prerequisite courses in completion settings
+
+  Background:
+    Given the following "courses" exist:
+      | fullname | shortname | category | enablecompletion |
+      | Course 1 | C1        | 0        | 1                |
+      | Course 2 | C2        | 0        | 1                |
+    And the following "users" exist:
+      | username | firstname | lastname | email                |
+      | student1 | Student   | One      | student1@example.com |
+    And the following "course enrolments" exist:
+      | user     | course | role    |
+      | student1 | C1     | student |
+
+  @javascript
+  Scenario: Set completion of prerequisite course as completion criteria of current course
+    When I log in as "admin"
+    And I am on "Course 1" course homepage with editing mode on
+    And I navigate to "Course completion" in current page administration
+    And I click on "Condition: Completion of other courses" "link"
+    And I set the field "Courses available" to "Course 2"
+    And I press "Save changes"
+    And I add the "Course completion status" block
+    And I click on "View course report" "link" in the "Course completion status" "block"
+    Then I should see "Course 2" in the "completion-progress" "table"
+    And I should see "Student One" in the "completion-progress" "table"
index ff1a32f..80ba0e4 100644 (file)
@@ -201,13 +201,13 @@ class helper {
         if ($category->can_change_sortorder()) {
             $actions['moveup'] = array(
                 'url' => new \moodle_url($baseurl, array('action' => 'movecategoryup')),
-                'icon' => new \pix_icon('t/up', new \lang_string('up')),
-                'string' => new \lang_string('up')
+                'icon' => new \pix_icon('t/up', new \lang_string('moveup')),
+                'string' => new \lang_string('moveup')
             );
             $actions['movedown'] = array(
                 'url' => new \moodle_url($baseurl, array('action' => 'movecategorydown')),
-                'icon' => new \pix_icon('t/down', new \lang_string('down')),
-                'string' => new \lang_string('down')
+                'icon' => new \pix_icon('t/down', new \lang_string('movedown')),
+                'string' => new \lang_string('movedown')
             );
         }
 
@@ -359,7 +359,7 @@ class helper {
      *
      * @param \core_course_category $category
      * @param \core_course_list_element $course
-     * @return string
+     * @return array
      */
     public static function get_course_listitem_actions(\core_course_category $category, \core_course_list_element $course) {
         $baseurl = new \moodle_url(
@@ -408,12 +408,12 @@ class helper {
         if ($category->can_resort_courses()) {
             $actions[] = array(
                 'url' => new \moodle_url($baseurl, array('action' => 'movecourseup')),
-                'icon' => new \pix_icon('t/up', \get_string('up')),
+                'icon' => new \pix_icon('t/up', \get_string('moveup')),
                 'attributes' => array('data-action' => 'moveup', 'class' => 'action-moveup')
             );
             $actions[] = array(
                 'url' => new \moodle_url($baseurl, array('action' => 'movecoursedown')),
-                'icon' => new \pix_icon('t/down', \get_string('down')),
+                'icon' => new \pix_icon('t/down', \get_string('movedown')),
                 'attributes' => array('data-action' => 'movedown', 'class' => 'action-movedown')
             );
         }
index ada3a69..25340f8 100644 (file)
@@ -85,6 +85,7 @@ class core_course_management_renderer extends plugin_renderer_base {
                     $categoryid = '';
                 }
                 $select = new single_select($this->page->url, 'categoryid', $categories, $categoryid, $nothing);
+                $select->attributes['aria-label'] = get_string('selectacategory');
                 $html .= $this->render($select);
             }
             $html .= html_writer::end_div();
@@ -264,8 +265,8 @@ class core_course_management_renderer extends plugin_renderer_base {
         $html .= html_writer::start_div('float-left ' . $checkboxclass);
         $html .= html_writer::start_div('custom-control custom-checkbox mr-1 ');
         $html .= html_writer::empty_tag('input', $bcatinput);
-        $html .= html_writer::tag('label', '', array(
-            'aria-label' => get_string('bulkactionselect', 'moodle', $text),
+        $labeltext = html_writer::span(get_string('bulkactionselect', 'moodle', $text), 'sr-only');
+        $html .= html_writer::tag('label', $labeltext, array(
             'class' => 'custom-control-label',
             'for' => 'categorylistitem' . $category->id));
         $html .= html_writer::end_div();
@@ -540,7 +541,7 @@ class core_course_management_renderer extends plugin_renderer_base {
         $html .= html_writer::start_div('card-body');
         $html .= $this->course_listing_actions($category, $course, $perpage);
         $html .= $this->listing_pagination($category, $page, $perpage, false, $viewmode);
-        $html .= html_writer::start_tag('ul', array('class' => 'ml course-list', 'role' => 'group'));
+        $html .= html_writer::start_tag('ul', array('class' => 'ml course-list'));
         foreach ($category->get_courses($options) as $listitem) {
             $html .= $this->course_listitem($category, $listitem, $courseid);
         }
@@ -641,8 +642,8 @@ class core_course_management_renderer extends plugin_renderer_base {
         $html .= html_writer::start_div('float-left ' . $checkboxclass);
         $html .= html_writer::start_div('custom-control custom-checkbox mr-1 ');
         $html .= html_writer::empty_tag('input', $bulkcourseinput);
-        $html .= html_writer::tag('label', '', array(
-            'aria-label' => get_string('bulkactionselect', 'moodle', $text),
+        $labeltext = html_writer::span(get_string('bulkactionselect', 'moodle', $text), 'sr-only');
+        $html .= html_writer::tag('label', $labeltext, array(
             'class' => 'custom-control-label',
             'for' => 'courselistitem' . $course->id));
         $html .= html_writer::end_div();
@@ -1215,8 +1216,8 @@ class core_course_management_renderer extends plugin_renderer_base {
         if ($bulkcourseinput) {
             $html .= html_writer::start_div('custom-control custom-checkbox mr-1');
             $html .= html_writer::empty_tag('input', $bulkcourseinput);
-            $html .= html_writer::tag('label', '', array(
-                'aria-label' => get_string('bulkactionselect', 'moodle', $text),
+            $labeltext = html_writer::span(get_string('bulkactionselect', 'moodle', $text), 'sr-only');
+            $html .= html_writer::tag('label', $labeltext, array(
                 'class' => 'custom-control-label',
                 'for' => 'coursesearchlistitem' . $course->id));
             $html .= html_writer::end_div();
@@ -1323,12 +1324,12 @@ class core_course_management_renderer extends plugin_renderer_base {
         $output .= html_writer::start_tag('form', array('class' => 'card', 'id' => $formid,
                 'action' => $searchurl, 'method' => 'get'));
         $output .= html_writer::start_tag('fieldset', array('class' => 'coursesearchbox invisiblefieldset'));
-        $output .= html_writer::tag('div', $this->output->heading($strsearchcourses.': ', 2, 'm-0'),
+        $output .= html_writer::tag('legend', $this->output->heading($strsearchcourses.': ', 2, 'm-0'),
                 array('class' => 'card-header'));
         $output .= html_writer::start_div('card-body');
         $output .= html_writer::start_div('input-group col-sm-6 col-lg-4 m-auto');
         $output .= html_writer::empty_tag('input', array('class' => 'form-control', 'type' => 'text', 'id' => $inputid,
-                'size' => $inputsize, 'name' => 'search', 'value' => s($value)));
+                'size' => $inputsize, 'name' => 'search', 'value' => s($value), 'aria-label' => get_string('searchcourses')));
         $output .= html_writer::start_tag('span', array('class' => 'input-group-btn'));
         $output .= html_writer::tag('button', get_string('go'), array('class' => 'btn btn-primary', 'type' => 'submit'));
         $output .= html_writer::end_tag('span');
index f850101..937a0ee 100644 (file)
@@ -128,14 +128,16 @@ class course_completion_form extends moodleform {
         }
 
         // Get applicable courses (prerequisites).
-        $selectedcourses = $DB->get_fieldset_sql("SELECT cc.courseinstance
-                  FROM {course_completion_criteria} cc WHERE cc.course = ?", [$course->id]);
         $hasselectablecourses = core_course_category::search_courses(['onlywithcompletion' => true], ['limit' => 2]);
         unset($hasselectablecourses[$course->id]);
         if ($hasselectablecourses) {
             // Show multiselect box.
             $mform->addElement('course', 'criteria_course', get_string('coursesavailable', 'completion'),
                 array('multiple' => 'multiple', 'onlywithcompletion' => true, 'exclude' => $course->id));
+            $mform->setType('criteria_course', PARAM_INT);
+
+            $selectedcourses = $DB->get_fieldset_select('course_completion_criteria', 'courseinstance',
+                'course = :course AND criteriatype = :type', ['course' => $course->id, 'type' => COMPLETION_CRITERIA_TYPE_COURSE]);
             $mform->setDefault('criteria_course', $selectedcourses);
 
             // Map aggregation methods to context-sensitive human readable dropdown menu.
index 977d7df..b0ccb2a 100644 (file)
@@ -2498,5 +2498,16 @@ function xmldb_main_upgrade($oldversion) {
         upgrade_main_savepoint(true, 2020062600.01);
     }
 
+    if ($oldversion < 2020071100.01) {
+        // Clean up completion criteria records referring to NULL course prerequisites.
+        $select = 'criteriatype = :type AND courseinstance IS NULL';
+        $params = ['type' => 8]; // COMPLETION_CRITERIA_TYPE_COURSE.
+
+        $DB->delete_records_select('course_completion_criteria', $select, $params);
+
+        // Main savepoint reached.
+        upgrade_main_savepoint(true, 2020071100.01);
+    }
+
     return true;
 }
index 204ddeb..55c5963 100644 (file)
@@ -218,8 +218,8 @@ abstract class engine {
      * and and have the search engine back end add them
      * to the index.
      *
-     * @param iterator $iterator the iterator of documents to index
-     * @param searcharea $searcharea the area for the documents to index
+     * @param \iterator $iterator the iterator of documents to index
+     * @param base $searcharea the area for the documents to index
      * @param array $options document indexing options
      * @return array Processed document counts
      */
@@ -227,11 +227,15 @@ abstract class engine {
         $numrecords = 0;
         $numdocs = 0;
         $numdocsignored = 0;
+        $numbatches = 0;
         $lastindexeddoc = 0;
         $firstindexeddoc = 0;
         $partial = false;
         $lastprogress = manager::get_current_time();
 
+        $batchmode = $this->supports_add_document_batch();
+        $currentbatch = [];
+
         foreach ($iterator as $document) {
             // Stop if we have exceeded the time limit (and there are still more items). Always
             // do at least one second's worth of documents otherwise it will never make progress.
@@ -255,10 +259,22 @@ abstract class engine {
                 $searcharea->attach_files($document);
             }
 
-            if ($this->add_document($document, $options['indexfiles'])) {
-                $numdocs++;
+            if ($batchmode && strlen($document->get('content')) <= $this->get_batch_max_content()) {
+                $currentbatch[] = $document;
+                if (count($currentbatch) >= $this->get_batch_max_documents()) {
+                    [$processed, $failed, $batches] = $this->add_document_batch($currentbatch, $options['indexfiles']);
+                    $numdocs += $processed;
+                    $numdocsignored += $failed;
+                    $numbatches += $batches;
+                    $currentbatch = [];
+                }
             } else {
-                $numdocsignored++;
+                if ($this->add_document($document, $options['indexfiles'])) {
+                    $numdocs++;
+                } else {
+                    $numdocsignored++;
+                }
+                $numbatches++;
             }
 
             $lastindexeddoc = $document->get('modified');
@@ -279,7 +295,15 @@ abstract class engine {
             }
         }
 
-        return array($numrecords, $numdocs, $numdocsignored, $lastindexeddoc, $partial);
+        // Add remaining documents from batch.
+        if ($batchmode && $currentbatch) {
+            [$processed, $failed, $batches] = $this->add_document_batch($currentbatch, $options['indexfiles']);
+            $numdocs += $processed;
+            $numdocsignored += $failed;
+            $numbatches += $batches;
+        }
+
+        return [$numrecords, $numdocs, $numdocsignored, $lastindexeddoc, $partial, $numbatches];
     }
 
     /**
@@ -473,6 +497,27 @@ abstract class engine {
      */
     abstract function add_document($document, $fileindexing = false);
 
+    /**
+     * Adds multiple documents to the search engine.
+     *
+     * It should return the number successfully processed, and the number of batches they were
+     * processed in (for example if you add 100 documents and there is an error processing one of
+     * those documents, and it took 4 batches, it would return [99, 1, 4]).
+     *
+     * If the engine implements this, it should return true to {@see supports_add_document_batch}.
+     *
+     * The system will only call this function with up to {@see get_batch_max_documents} documents,
+     * and each document in the batch will have content no larger than specified by
+     * {@see get_batch_max_content}.
+     *
+     * @param document[] $documents Documents to add
+     * @param bool $fileindexing True if file indexing is to be used
+     * @return int[] Array of three elements, successfully processed, failed processed, batch count
+     */
+    public function add_document_batch(array $documents, bool $fileindexing = false): array {
+        throw new \coding_exception('add_document_batch not supported by this engine');
+    }
+
     /**
      * Executes the query on the engine.
      *
@@ -653,4 +698,44 @@ abstract class engine {
     public function supports_users() {
         return false;
     }
+
+    /**
+     * Checks if the search engine supports adding documents in a batch.
+     *
+     * If it returns true to this function, the search engine must implement the add_document_batch
+     * function.
+     *
+     * @return bool True if the search engine supports adding documents in a batch
+     */
+    public function supports_add_document_batch(): bool {
+        return false;
+    }
+
+    /**
+     * Gets the maximum number of documents to send together in batch mode.
+     *
+     * Only relevant if the engine returns true to {@see supports_add_document_batch}.
+     *
+     * Can be overridden by search engine if required.
+     *
+     * @var int Number of documents to send together in batch mode, default 100.
+     */
+    public function get_batch_max_documents(): int {
+        return 100;
+    }
+
+    /**
+     * Gets the maximum size of document content to be included in a shared batch (if the
+     * document is bigger then it will be sent on its own; batching does not provide a performance
+     * improvement for big documents anyway).
+     *
+     * Only relevant if the engine returns true to {@see supports_add_document_batch}.
+     *
+     * Can be overridden by search engine if required.
+     *
+     * @return int Max size in bytes, default 1MB
+     */
+    public function get_batch_max_content(): int {
+        return 1024 * 1024;
+    }
 }
index 95e9733..458dad7 100644 (file)
@@ -1152,8 +1152,20 @@ class manager {
                     $recordset, array($searcharea, 'get_document'), $options));
             $result = $this->engine->add_documents($iterator, $searcharea, $options);
             $recordset->close();
-            if (count($result) === 5) {
-                list($numrecords, $numdocs, $numdocsignored, $lastindexeddoc, $partial) = $result;
+            $batchinfo = '';
+            if (count($result) === 6) {
+                [$numrecords, $numdocs, $numdocsignored, $lastindexeddoc, $partial, $batches] = $result;
+                // Only show the batch count if we actually batched any requests.
+                if ($batches !== $numdocs + $numdocsignored) {
+                    $batchinfo = ' (' . $batches . ' batch' . ($batches === 1 ? '' : 'es') . ')';
+                }
+            } else if (count($result) === 5) {
+                // Backward compatibility for engines that don't return a batch count.
+                [$numrecords, $numdocs, $numdocsignored, $lastindexeddoc, $partial] = $result;
+                // Deprecated since Moodle 4.0 MDL-68690.
+                // TODO: MDL-68776 This will be deleted in Moodle 4.4.
+                debugging('engine::add_documents() should return $batches (5-value return is deprecated)',
+                        DEBUG_DEVELOPER);
             } else {
                 throw new coding_exception('engine::add_documents() should return $partial (4-value return is deprecated)');
             }
@@ -1168,7 +1180,7 @@ class manager {
                 }
 
                 $progress->output('Processed ' . $numrecords . ' records containing ' . $numdocs .
-                        ' documents, in ' . $elapsed . ' seconds' . $partialtext . '.', 1);
+                        ' documents' . $batchinfo . ', in ' . $elapsed . ' seconds' . $partialtext . '.', 1);
             } else {
                 $progress->output('No new documents to index.', 1);
             }
@@ -1305,8 +1317,20 @@ class manager {
 
             // Use this iterator to add documents.
             $result = $this->engine->add_documents($iterator, $searcharea, $options);
-            if (count($result) === 5) {
-                list($numrecords, $numdocs, $numdocsignored, $lastindexeddoc, $partial) = $result;
+            $batchinfo = '';
+            if (count($result) === 6) {
+                [$numrecords, $numdocs, $numdocsignored, $lastindexeddoc, $partial, $batches] = $result;
+                // Only show the batch count if we actually batched any requests.
+                if ($batches !== $numdocs + $numdocsignored) {
+                    $batchinfo = ' (' . $batches . ' batch' . ($batches === 1 ? '' : 'es') . ')';
+                }
+            } else if (count($result) === 5) {
+                // Backward compatibility for engines that don't return a batch count.
+                [$numrecords, $numdocs, $numdocsignored, $lastindexeddoc, $partial] = $result;
+                // Deprecated since Moodle 4.0 MDL-68690.
+                // TODO: MDL-68776 This will be deleted in Moodle 4.4 (as should the below bit).
+                debugging('engine::add_documents() should return $batches (5-value return is deprecated)',
+                        DEBUG_DEVELOPER);
             } else {
                 // Backward compatibility for engines that don't support partial adding.
                 list($numrecords, $numdocs, $numdocsignored, $lastindexeddoc) = $result;
@@ -1318,7 +1342,7 @@ class manager {
             if ($numdocs > 0) {
                 $elapsed = round((self::get_current_time() - $elapsed), 3);
                 $progress->output('Processed ' . $numrecords . ' records containing ' . $numdocs .
-                        ' documents, in ' . $elapsed . ' seconds' .
+                        ' documents' . $batchinfo . ', in ' . $elapsed . ' seconds' .
                         ($partial ? ' (not complete)' : '') . '.', 1);
             } else {
                 $progress->output('No documents to index.', 1);
index d047694..a20d79e 100644 (file)
@@ -753,6 +753,32 @@ class engine extends \core_search\engine {
         return true;
     }
 
+    /**
+     * Adds a batch of documents to the engine at once.
+     *
+     * @param \core_search\document[] $documents Documents to add
+     * @param bool $fileindexing If true, indexes files (these are done one at a time)
+     * @return int[] Array of three elements: successfully processed, failed processed, batch count
+     */
+    public function add_document_batch(array $documents, bool $fileindexing = false): array {
+        $docdatabatch = [];
+        foreach ($documents as $document) {
+            $docdatabatch[] = $document->export_for_engine();
+        }
+
+        $resultcounts = $this->add_solr_documents($docdatabatch);
+
+        // Files are processed one document at a time (if there are files it's slow anyway).
+        if ($fileindexing) {
+            foreach ($documents as $document) {
+                // This will take care of updating all attached files in the index.
+                $this->process_document_files($document);
+            }
+        }
+
+        return $resultcounts;
+    }
+
     /**
      * Replaces underlines at edges of words in the content with spaces.
      *
@@ -771,12 +797,12 @@ class engine extends \core_search\engine {
     }
 
     /**
-     * Adds a text document to the search engine.
+     * Creates a Solr document object.
      *
-     * @param array $doc
-     * @return bool
+     * @param array $doc Array of document fields
+     * @return \SolrInputDocument Created document
      */
-    protected function add_solr_document($doc) {
+    protected function create_solr_document(array $doc): \SolrInputDocument {
         $solrdoc = new \SolrInputDocument();
 
         // Replace underlines in the content with spaces. The reason for this is that for italic
@@ -786,10 +812,23 @@ class engine extends \core_search\engine {
             $doc['content'] = self::replace_underlines($doc['content']);
         }
 
+        // Set all the fields.
         foreach ($doc as $field => $value) {
             $solrdoc->addField($field, $value);
         }
 
+        return $solrdoc;
+    }
+
+    /**
+     * Adds a text document to the search engine.
+     *
+     * @param array $doc
+     * @return bool
+     */
+    protected function add_solr_document($doc) {
+        $solrdoc = $this->create_solr_document($doc);
+
         try {
             $result = $this->get_search_client()->addDocument($solrdoc, true, static::AUTOCOMMIT_WITHIN);
             return true;
@@ -804,6 +843,50 @@ class engine extends \core_search\engine {
         return false;
     }
 
+    /**
+     * Adds multiple text documents to the search engine.
+     *
+     * @param array $docs Array of documents (each an array of fields) to add
+     * @return int[] Array of success, failure, batch count
+     * @throws \core_search\engine_exception
+     */
+    protected function add_solr_documents(array $docs): array {
+        $solrdocs = [];
+        foreach ($docs as $doc) {
+            $solrdocs[] = $this->create_solr_document($doc);
+        }
+
+        try {
+            // Add documents in a batch and report that they all succeeded.
+            $this->get_search_client()->addDocuments($solrdocs, true, static::AUTOCOMMIT_WITHIN);
+            return [count($solrdocs), 0, 1];
+        } catch (\SolrClientException $e) {
+            // If there is an exception, fall through...
+            $donothing = true;
+        } catch (\SolrServerException $e) {
+            // If there is an exception, fall through...
+            $donothing = true;
+        }
+
+        // When there is an error, we fall back to adding them individually so that we can report
+        // which document(s) failed. Since it overwrites, adding the successful ones multiple
+        // times won't hurt.
+        $success = 0;
+        $failure = 0;
+        $batches = 0;
+        foreach ($docs as $doc) {
+            $result = $this->add_solr_document($doc);
+            $batches++;
+            if ($result) {
+                $success++;
+            } else {
+                $failure++;
+            }
+        }
+
+        return [$success, $failure, $batches];
+    }
+
     /**
      * Index files attached to the docuemnt, ensuring the index matches the current document files.
      *
@@ -1446,6 +1529,15 @@ class engine extends \core_search\engine {
         return true;
     }
 
+    /**
+     * Solr supports adding documents in a batch.
+     *
+     * @return bool True
+     */
+    public function supports_add_document_batch(): bool {
+        return true;
+    }
+
     /**
      * Solr supports deleting the index for a context.
      *
index 7deb8d0..0bdae77 100644 (file)
@@ -1297,6 +1297,148 @@ class search_solr_engine_testcase extends advanced_testcase {
         $this->assert_raw_solr_query_result('content:xyzzy', []);
     }
 
+    /**
+     * Specific test of the add_document_batch function (also used in many other tests).
+     */
+    public function test_add_document_batch() {
+        // Get a default document.
+        $area = new core_mocksearch\search\mock_search_area();
+        $record = $this->generator->create_record();
+        $doc = $area->get_document($record);
+        $originalid = $doc->get('id');
+
+        // Now create 5 similar documents.
+        $docs = [];
+        for ($i = 1; $i <= 5; $i++) {
+            $doc = $area->get_document($record);
+            $doc->set('id', $originalid . '-' . $i);
+            $doc->set('title', 'Batch ' . $i);
+            $docs[$i] = $doc;
+        }
+
+        // Document 3 has a file attached.
+        $fs = get_file_storage();
+        $filerecord = new \stdClass();
+        $filerecord->content = 'Some FileContents';
+        $file = $this->generator->create_file($filerecord);
+        $docs[3]->add_stored_file($file);
+
+        // Add all these documents to the search engine.
+        $this->assertEquals([5, 0, 1], $this->engine->add_document_batch($docs, true));
+        $this->engine->area_index_complete($area->get_area_id());
+
+        // Check all documents were indexed.
+        $querydata = new stdClass();
+        $querydata->q = 'Batch';
+        $results = $this->search->search($querydata);
+        $this->assertCount(5, $results);
+
+        // Check it also finds based on the file.
+        $querydata->q = 'FileContents';
+        $results = $this->search->search($querydata);
+        $this->assertCount(1, $results);
+    }
+
+    /**
+     * Tests the batching logic, specifically the limit to 100 documents per
+     * batch, and not batching very large documents.
+     */
+    public function test_batching() {
+        $area = new core_mocksearch\search\mock_search_area();
+        $record = $this->generator->create_record();
+        $doc = $area->get_document($record);
+        $originalid = $doc->get('id');
+
+        // Up to 100 documents in 1 batch.
+        $docs = [];
+        for ($i = 1; $i <= 100; $i++) {
+            $doc = $area->get_document($record);
+            $doc->set('id', $originalid . '-' . $i);
+            $docs[$i] = $doc;
+        }
+        [, , , , , $batches] = $this->engine->add_documents(
+                new ArrayIterator($docs), $area, ['indexfiles' => true]);
+        $this->assertEquals(1, $batches);
+
+        // More than 100 needs 2 batches.
+        $docs = [];
+        for ($i = 1; $i <= 101; $i++) {
+            $doc = $area->get_document($record);
+            $doc->set('id', $originalid . '-' . $i);
+            $docs[$i] = $doc;
+        }
+        [, , , , , $batches] = $this->engine->add_documents(
+                new ArrayIterator($docs), $area, ['indexfiles' => true]);
+        $this->assertEquals(2, $batches);
+
+        // Small number but with some large documents that aren't batched.
+        $docs = [];
+        for ($i = 1; $i <= 10; $i++) {
+            $doc = $area->get_document($record);
+            $doc->set('id', $originalid . '-' . $i);
+            $docs[$i] = $doc;
+        }
+        // This one is just small enough to fit.
+        $docs[3]->set('content', str_pad('xyzzy ', 1024 * 1024, 'x'));
+        // These two don't fit.
+        $docs[5]->set('content', str_pad('xyzzy ', 1024 * 1024 + 1, 'x'));
+        $docs[6]->set('content', str_pad('xyzzy ', 1024 * 1024 + 1, 'x'));
+        [, , , , , $batches] = $this->engine->add_documents(
+                new ArrayIterator($docs), $area, ['indexfiles' => true]);
+        $this->assertEquals(3, $batches);
+
+        // Check that all 3 of the large documents (added as batch or not) show up in results.
+        $this->engine->area_index_complete($area->get_area_id());
+        $querydata = new stdClass();
+        $querydata->q = 'xyzzy';
+        $results = $this->search->search($querydata);
+        $this->assertCount(3, $results);
+    }
+
+    /**
+     * Tests with large documents. The point of this test is that we stop batching
+     * documents if they are bigger than 1MB, and the maximum batch count is 100,
+     * so the maximum size batch will be about 100 1MB documents.
+     */
+    public function test_add_document_batch_large() {
+        // This test is a bit slow and not that important to run every time...
+        if (!PHPUNIT_LONGTEST) {
+            $this->markTestSkipped('PHPUNIT_LONGTEST is not defined');
+        }
+
+        // Get a default document.
+        $area = new core_mocksearch\search\mock_search_area();
+        $record = $this->generator->create_record();
+        $doc = $area->get_document($record);
+        $originalid = $doc->get('id');
+
+        // Now create 100 large documents.
+        $size = 1024 * 1024;
+        $docs = [];
+        for ($i = 1; $i <= 100; $i++) {
+            $doc = $area->get_document($record);
+            $doc->set('id', $originalid . '-' . $i);
+            $doc->set('title', 'Batch ' . $i);
+            $doc->set('content', str_pad('', $size, 'Long text ' . $i . '. ', STR_PAD_RIGHT) . ' xyzzy');
+            $docs[$i] = $doc;
+        }
+
+        // Add all these documents to the search engine.
+        $this->engine->add_document_batch($docs, true);
+        $this->engine->area_index_complete($area->get_area_id());
+
+        // Check all documents were indexed, searching for text at end.
+        $querydata = new stdClass();
+        $querydata->q = 'xyzzy';
+        $results = $this->search->search($querydata);
+        $this->assertCount(100, $results);
+
+        // Search for specific text that's only in one.
+        $querydata->q = '42';
+        $results = $this->search->search($querydata);
+        $this->assertCount(1, $results);
+    }
+
     /**
      * Carries out a raw Solr query using the Solr basic query syntax.
      *
index bb0d8f3..3566bc8 100644 (file)
@@ -1,6 +1,14 @@
 This files describes API changes in /search/*,
 information provided here is intended especially for developers.
 
+=== 4.0 ===
+
+* Search indexing now supports sending multiple documents to the server in a batch. This is implemented
+  for the Solr search engine, where it significantly increases performance. For this to work, engines
+  should implement add_document_batch() function and return true to supports_add_document_batch().
+  There is also an additional parameter returned from add_documents() with the number of batches
+  sent, which is used for the log display. Existing engines should continue to work unmodified.
+
 === 3.8 ===
 
 * Search indexing supports time limits to make the scheduled task run more neatly since 3.4. In order for
index e548f33..1349003 100644 (file)
@@ -870,7 +870,7 @@ span.editinstructions {
     .listitem {
 
         &[data-selected='1'] {
-            border-left: calc(#{$list-group-border-width} + 5px) solid map-get($theme-colors, 'info');
+            border-left: calc(#{$list-group-border-width} + 5px) solid map-get($theme-colors, 'primary');
             padding-left: calc(#{$list-group-item-padding-x} - 5px);
         }
     }
index 588a0de..93f02cb 100644 (file)
@@ -13740,7 +13740,7 @@ span.editinstructions {
     #course-category-listings ul.ml ul.ml {
       margin: 0; }
   #course-category-listings .listitem[data-selected='1'] {
-    border-left: calc(1px + 5px) solid #5bc0de;
+    border-left: calc(1px + 5px) solid #1177d1;
     padding-left: calc(1.25rem - 5px); }
   #course-category-listings .item-actions {
     margin-right: 1em;
index e9b5b03..02d840a 100644 (file)
@@ -13957,7 +13957,7 @@ span.editinstructions {
     #course-category-listings ul.ml ul.ml {
       margin: 0; }
   #course-category-listings .listitem[data-selected='1'] {
-    border-left: calc(1px + 5px) solid #5bc0de;
+    border-left: calc(1px + 5px) solid #1177d1;
     padding-left: calc(1.25rem - 5px); }
   #course-category-listings .item-actions {
     margin-right: 1em;
index 7805185..10026f0 100644 (file)
@@ -29,7 +29,7 @@
 
 defined('MOODLE_INTERNAL') || die();
 
-$version  = 2020071100.00;              // YYYYMMDD      = weekly release date of this DEV branch.
+$version  = 2020071100.01;              // YYYYMMDD      = weekly release date of this DEV branch.
                                         //         RR    = release increments - 00 in DEV branches.
                                         //           .XX = incremental changes.
 $release  = '4.0dev (Build: 20200711)'; // Human-friendly version name