From 2085e8603aff933269d8fcb8a5f8f6fcbd4537a4 Mon Sep 17 00:00:00 2001 From: Dmitrii Metelkin Date: Mon, 7 Jan 2019 11:16:35 +1100 Subject: [PATCH] MDL-62869 search: implement searching for all courses --- admin/settings/plugins.php | 6 +- .../search/{mycourse.php => course.php} | 12 +- course/classes/search/customfield.php | 3 +- course/tests/search_test.php | 74 +++++-- lang/en/admin.php | 4 + lang/en/deprecated.txt | 4 +- lang/en/search.php | 2 + .../clean_up_deleted_search_area_task.php | 53 +++++ lib/db/upgrade.php | 17 ++ search/classes/base.php | 13 +- search/classes/manager.php | 171 +++++++++++++--- search/classes/output/form/search.php | 15 +- search/engine/solr/tests/engine_test.php | 2 +- search/index.php | 3 + search/tests/base_test.php | 9 + .../tests/fixtures/testable_core_search.php | 13 ++ search/tests/manager_test.php | 190 ++++++++++++++++-- search/upgrade.txt | 12 ++ version.php | 2 +- 19 files changed, 533 insertions(+), 72 deletions(-) rename course/classes/search/{mycourse.php => course.php} (95%) create mode 100644 lib/classes/task/clean_up_deleted_search_area_task.php diff --git a/admin/settings/plugins.php b/admin/settings/plugins.php index 616d10e5f907e..485eb012dfb61 100644 --- a/admin/settings/plugins.php +++ b/admin/settings/plugins.php @@ -576,14 +576,18 @@ $temp->add(new admin_setting_configduration('searchindextime', new lang_string('searchindextime', 'admin'), new lang_string('searchindextime_desc', 'admin'), 600)); + $temp->add(new admin_setting_heading('searchcoursesheading', new lang_string('searchablecourses', 'admin'), '')); $options = [ 0 => new lang_string('searchallavailablecourses_off', 'admin'), 1 => new lang_string('searchallavailablecourses_on', 'admin') ]; $temp->add(new admin_setting_configselect('searchallavailablecourses', new lang_string('searchallavailablecourses', 'admin'), - new lang_string('searchallavailablecourses_desc', 'admin'), + new lang_string('searchallavailablecoursesdesc', 'admin'), 0, $options)); + $temp->add(new admin_setting_configcheckbox('searchincludeallcourses', + new lang_string('searchincludeallcourses', 'admin'), new lang_string('searchincludeallcourses_desc', 'admin'), + 0)); // Search display options. $temp->add(new admin_setting_heading('searchdisplay', new lang_string('searchdisplay', 'admin'), '')); diff --git a/course/classes/search/mycourse.php b/course/classes/search/course.php similarity index 95% rename from course/classes/search/mycourse.php rename to course/classes/search/course.php index bac012796fc4d..27ba20189c90d 100644 --- a/course/classes/search/mycourse.php +++ b/course/classes/search/course.php @@ -15,7 +15,7 @@ // along with Moodle. If not, see . /** - * Search area for Moodle courses I can access. + * Search area for Moodle courses. * * @package core_course * @copyright 2016 Skylar Kelty @@ -26,13 +26,13 @@ defined('MOODLE_INTERNAL') || die(); /** - * Search area for Moodle courses I can access. + * Search area for Moodle courses. * * @package core_course * @copyright 2016 Skylar Kelty * @license http://www.gnu.org/copyleft/gpl.html GNU GPL v3 or later */ -class mycourse extends \core_search\base { +class course extends \core_search\base { /** * The context levels the search implementation is working on. @@ -112,9 +112,13 @@ public function check_access($id) { if (!$course) { return \core_search\manager::ACCESS_DELETED; } - if (can_access_course($course)) { + + $coursecontext = \context_course::instance($course->id); + + if ($course->visible || has_capability('moodle/course:viewhiddencourses', $coursecontext)) { return \core_search\manager::ACCESS_GRANTED; } + return \core_search\manager::ACCESS_DENIED; } diff --git a/course/classes/search/customfield.php b/course/classes/search/customfield.php index 8f5db60b9fc72..315281e6be55d 100644 --- a/course/classes/search/customfield.php +++ b/course/classes/search/customfield.php @@ -135,7 +135,8 @@ public function check_access($id) { if (!$course) { return \core_search\manager::ACCESS_DELETED; } - if (can_access_course($course)) { + $coursecontext = \context_course::instance($course->id); + if ($course->visible || has_capability('moodle/course:viewhiddencourses', $coursecontext)) { return \core_search\manager::ACCESS_GRANTED; } return \core_search\manager::ACCESS_DENIED; diff --git a/course/tests/search_test.php b/course/tests/search_test.php index c64ce2c83f25a..c5ada25b50dc6 100644 --- a/course/tests/search_test.php +++ b/course/tests/search_test.php @@ -41,7 +41,7 @@ class course_search_testcase extends advanced_testcase { /** * @var string Area id */ - protected $mycoursesareaid = null; + protected $coursesareaid = null; /** * @var string Area id for sections @@ -57,7 +57,7 @@ public function setUp() { $this->resetAfterTest(true); set_config('enableglobalsearch', true); - $this->mycoursesareaid = \core_search\manager::generate_areaid('core_course', 'mycourse'); + $this->coursesareaid = \core_search\manager::generate_areaid('core_course', 'course'); $this->sectionareaid = \core_search\manager::generate_areaid('core_course', 'section'); $this->customfieldareaid = \core_search\manager::generate_areaid('core_course', 'customfield'); @@ -66,15 +66,15 @@ public function setUp() { } /** - * Indexing my courses contents. + * Indexing courses contents. * * @return void */ - public function test_mycourses_indexing() { + public function test_courses_indexing() { // Returns the instance as long as the area is supported. - $searcharea = \core_search\manager::get_search_area($this->mycoursesareaid); - $this->assertInstanceOf('\core_course\search\mycourse', $searcharea); + $searcharea = \core_search\manager::get_search_area($this->coursesareaid); + $this->assertInstanceOf('\core_course\search\course', $searcharea); $user1 = self::getDataGenerator()->create_user(); $user2 = self::getDataGenerator()->create_user(); @@ -113,10 +113,10 @@ public function test_mycourses_indexing() { /** * Tests course indexing support for contexts. */ - public function test_mycourses_indexing_contexts() { + public function test_courses_indexing_contexts() { global $DB, $USER, $SITE; - $searcharea = \core_search\manager::get_search_area($this->mycoursesareaid); + $searcharea = \core_search\manager::get_search_area($this->coursesareaid); // Create some courses in categories, and a forum. $generator = $this->getDataGenerator(); @@ -194,11 +194,11 @@ protected static function recordset_to_ids(moodle_recordset $rs) { * * @return void */ - public function test_mycourses_document() { + public function test_courses_document() { // Returns the instance as long as the area is supported. - $searcharea = \core_search\manager::get_search_area($this->mycoursesareaid); - $this->assertInstanceOf('\core_course\search\mycourse', $searcharea); + $searcharea = \core_search\manager::get_search_area($this->coursesareaid); + $this->assertInstanceOf('\core_course\search\course', $searcharea); $user = self::getDataGenerator()->create_user(); $course = self::getDataGenerator()->create_course(); @@ -207,7 +207,7 @@ public function test_mycourses_document() { $doc = $searcharea->get_document($course); $this->assertInstanceOf('\core_search\document', $doc); $this->assertEquals($course->id, $doc->get('itemid')); - $this->assertEquals($this->mycoursesareaid . '-' . $course->id, $doc->get('id')); + $this->assertEquals($this->coursesareaid . '-' . $course->id, $doc->get('id')); $this->assertEquals($course->id, $doc->get('courseid')); $this->assertFalse($doc->is_set('userid')); $this->assertEquals(\core_search\manager::NO_OWNER_ID, $doc->get('owneruserid')); @@ -224,10 +224,11 @@ public function test_mycourses_document() { * * @return void */ - public function test_mycourses_access() { + public function test_courses_access() { + $this->resetAfterTest(); // Returns the instance as long as the area is supported. - $searcharea = \core_search\manager::get_search_area($this->mycoursesareaid); + $searcharea = \core_search\manager::get_search_area($this->coursesareaid); $user1 = self::getDataGenerator()->create_user(); $user2 = self::getDataGenerator()->create_user(); @@ -244,13 +245,13 @@ public function test_mycourses_access() { $this->setUser($user1); $this->assertEquals(\core_search\manager::ACCESS_GRANTED, $searcharea->check_access($course1->id)); $this->assertEquals(\core_search\manager::ACCESS_GRANTED, $searcharea->check_access($course2->id)); - $this->assertEquals(\core_search\manager::ACCESS_DENIED, $searcharea->check_access($course3->id)); + $this->assertEquals(\core_search\manager::ACCESS_GRANTED, $searcharea->check_access($course3->id)); $this->assertEquals(\core_search\manager::ACCESS_DELETED, $searcharea->check_access(-123)); $this->setUser($user2); $this->assertEquals(\core_search\manager::ACCESS_GRANTED, $searcharea->check_access($course1->id)); $this->assertEquals(\core_search\manager::ACCESS_DENIED, $searcharea->check_access($course2->id)); - $this->assertEquals(\core_search\manager::ACCESS_DENIED, $searcharea->check_access($course3->id)); + $this->assertEquals(\core_search\manager::ACCESS_GRANTED, $searcharea->check_access($course3->id)); } /** @@ -538,10 +539,43 @@ public function test_customfield_document() { } /** - * Test document icon for mycourse area. + * Document accesses for customfield area. */ - public function test_get_doc_icon_for_mycourse_area() { - $searcharea = \core_search\manager::get_search_area($this->mycoursesareaid); + public function test_customfield_access() { + $this->resetAfterTest(); + + // Returns the instance as long as the area is supported. + $searcharea = \core_search\manager::get_search_area($this->customfieldareaid); + + $user1 = self::getDataGenerator()->create_user(); + $user2 = self::getDataGenerator()->create_user(); + + $course1 = self::getDataGenerator()->create_course(); + $course2 = self::getDataGenerator()->create_course(array('visible' => 0)); + $course3 = self::getDataGenerator()->create_course(); + + $this->getDataGenerator()->enrol_user($user1->id, $course1->id, 'teacher'); + $this->getDataGenerator()->enrol_user($user2->id, $course1->id, 'student'); + $this->getDataGenerator()->enrol_user($user1->id, $course2->id, 'teacher'); + $this->getDataGenerator()->enrol_user($user2->id, $course2->id, 'student'); + + $this->setUser($user1); + $this->assertEquals(\core_search\manager::ACCESS_GRANTED, $searcharea->check_access($course1->id)); + $this->assertEquals(\core_search\manager::ACCESS_GRANTED, $searcharea->check_access($course2->id)); + $this->assertEquals(\core_search\manager::ACCESS_GRANTED, $searcharea->check_access($course3->id)); + $this->assertEquals(\core_search\manager::ACCESS_DELETED, $searcharea->check_access(-123)); + + $this->setUser($user2); + $this->assertEquals(\core_search\manager::ACCESS_GRANTED, $searcharea->check_access($course1->id)); + $this->assertEquals(\core_search\manager::ACCESS_DENIED, $searcharea->check_access($course2->id)); + $this->assertEquals(\core_search\manager::ACCESS_GRANTED, $searcharea->check_access($course3->id)); + } + + /** + * Test document icon for course area. + */ + public function test_get_doc_icon_for_course_area() { + $searcharea = \core_search\manager::get_search_area($this->coursesareaid); $document = $this->getMockBuilder('\core_search\document') ->disableOriginalConstructor() @@ -573,7 +607,7 @@ public function test_get_doc_icon_for_section_area() { * Test assigned search categories. */ public function test_get_category_names() { - $coursessearcharea = \core_search\manager::get_search_area($this->mycoursesareaid); + $coursessearcharea = \core_search\manager::get_search_area($this->coursesareaid); $sectionsearcharea = \core_search\manager::get_search_area($this->sectionareaid); $this->assertEquals(['core-courses'], $coursessearcharea->get_category_names()); diff --git a/lang/en/admin.php b/lang/en/admin.php index 257381f2d93cb..da83e5fd06982 100644 --- a/lang/en/admin.php +++ b/lang/en/admin.php @@ -1055,6 +1055,7 @@ $string['savechanges'] = 'Save changes'; $string['scssinvalid'] = 'SCSS code is not valid, fails with: {$a}'; $string['search'] = 'Search'; +$string['searchablecourses'] = 'Searchable courses'; $string['searchallavailablecourses'] = 'Searchable courses'; $string['searchallavailablecourses_off'] = 'Search within enrolled courses only'; $string['searchallavailablecourses_on'] = 'Search within all courses the user can access'; @@ -1066,6 +1067,9 @@ $string['searchhideallcategory_desc'] = 'If checked, the category with all results will be hidden on the search result screen.'; $string['searchdefaultcategory'] = 'Default search category'; $string['searchdefaultcategory_desc'] = 'Results from the selected search area category will be displayed by default.'; +$string['searchallavailablecoursesdesc'] = 'If set to search within enrolled courses only, course information (name and summary) and course content will only be searched in courses which the user is enrolled in. Otherwise, course information and course content will be searched in all courses which the user can access, such as courses with guest access enabled.'; +$string['searchincludeallcourses'] = 'Include all visible courses'; +$string['searchincludeallcourses_desc'] = 'If enabled, search results will include course information (name and summary) of courses which are visible to the user, even if they don\'t have access to the course content.'; $string['searchalldeleted'] = 'All indexed contents have been deleted'; $string['searchareaenabled'] = 'Search area enabled'; $string['searchareadisabled'] = 'Search area disabled'; diff --git a/lang/en/deprecated.txt b/lang/en/deprecated.txt index c5bba12a8973a..0d2e5023da49a 100644 --- a/lang/en/deprecated.txt +++ b/lang/en/deprecated.txt @@ -140,4 +140,6 @@ eventmessagecontactunblocked,core_message userisblockingyou,core_message userisblockingyounoncontact,core_message error:invalidbadgeurl,core_badges -nomessages,core_message \ No newline at end of file +nomessages,core_message +searchallavailablecourses_desc,core_admin +search:mycourse,core_search \ No newline at end of file diff --git a/lang/en/search.php b/lang/en/search.php index 9bf7ce88aa886..7d791e3a738b2 100644 --- a/lang/en/search.php +++ b/lang/en/search.php @@ -86,6 +86,7 @@ $string['ittook'] = 'It took'; $string['matchingfile'] = 'Matched from file {$a}'; $string['matchingfiles'] = 'Matched from files:'; +$string['mycoursesonly'] = 'My courses only'; $string['next'] = 'Next'; $string['noindexmessage'] = 'Admin: There appears to be no search index. Please'; $string['noresults'] = 'No results'; @@ -113,6 +114,7 @@ $string['search:message_received'] = 'Messages - received'; $string['search:message_sent'] = 'Messages - sent'; $string['search:mycourse'] = 'My courses'; +$string['search:course'] = 'Courses'; $string['search:section'] = 'Course sections'; $string['search:user'] = 'Users'; $string['searcharea'] = 'Search area'; diff --git a/lib/classes/task/clean_up_deleted_search_area_task.php b/lib/classes/task/clean_up_deleted_search_area_task.php new file mode 100644 index 0000000000000..ae8d73cb6f153 --- /dev/null +++ b/lib/classes/task/clean_up_deleted_search_area_task.php @@ -0,0 +1,53 @@ +. + +/** + * Adhoc task that clean up data related ro deleted search area. + * + * @package core + * @copyright 2019 Dmitrii Metelkin + * @license http://www.gnu.org/copyleft/gpl.html GNU GPL v3 or later + */ + +namespace core\task; + +defined('MOODLE_INTERNAL') || die(); + +/** + * Class that cleans up data related to deleted search area. + * + * Custom data accepted: + * - areaid -> String search area id . + * + * @package core + * @copyright 2019 Dmitrii Metelkin + * @license http://www.gnu.org/copyleft/gpl.html GNU GPL v3 or later + */ +class clean_up_deleted_search_area_task extends adhoc_task { + + /** + * Run the task to clean up deleted search are data. + */ + public function execute() { + $areaid = $this->get_custom_data(); + + try { + \core_search\manager::clean_up_non_existing_area($areaid); + } catch (\core_search\engine_exception $e) { + mtrace('Search is not configured. Skip deleting index for search area ' . $areaid); + } + } +} diff --git a/lib/db/upgrade.php b/lib/db/upgrade.php index 3aa8faa94fef5..9fff0bc77fb52 100644 --- a/lib/db/upgrade.php +++ b/lib/db/upgrade.php @@ -2736,5 +2736,22 @@ function xmldb_main_upgrade($oldversion) { upgrade_main_savepoint(true, 2019021500.02); } + if ($oldversion < 2019022500.00) { + // Create adhoc task to delete renamed My Course search area (ID core_course-mycourse). + $record = new \stdClass(); + $record->classname = '\core\task\clean_up_deleted_search_area_task'; + $record->component = 'core'; + + // Next run time based from nextruntime computation in \core\task\manager::queue_adhoc_task(). + $nextruntime = time() - 1; + $record->nextruntime = $nextruntime; + $record->customdata = json_encode('core_course-mycourse'); + + $DB->insert_record('task_adhoc', $record); + + // Main savepoint reached. + upgrade_main_savepoint(true, 2019022500.00); + } + return true; } diff --git a/search/classes/base.php b/search/classes/base.php index c2315486c3e9c..78c1fe2518694 100644 --- a/search/classes/base.php +++ b/search/classes/base.php @@ -175,8 +175,7 @@ public function get_config() { list($componentname, $varname) = $this->get_config_var_name(); $config = []; - $settingnames = array('_enabled', '_indexingstart', '_indexingend', '_lastindexrun', - '_docsignored', '_docsprocessed', '_recordsprocessed', '_partial'); + $settingnames = self::get_settingnames(); foreach ($settingnames as $name) { $config[$varname . $name] = get_config($componentname, $varname . $name); } @@ -188,6 +187,16 @@ public function get_config() { return $config; } + /** + * Return a list of all required setting names. + * + * @return array + */ + public static function get_settingnames() { + return array('_enabled', '_indexingstart', '_indexingend', '_lastindexrun', + '_docsignored', '_docsprocessed', '_recordsprocessed', '_partial'); + } + /** * Is the search component enabled by the system administrator? * diff --git a/search/classes/manager.php b/search/classes/manager.php index 01d3b65c77a35..2abe63de5d4f8 100644 --- a/search/classes/manager.php +++ b/search/classes/manager.php @@ -585,6 +585,33 @@ public static function extract_areaid_parts($areaid) { return explode('-', $areaid); } + /** + * Parse a search area id and get plugin name and config name prefix from it. + * + * @param string $areaid Search area id. + * @return array Where the first element is a plugin name and the second is config names prefix. + */ + public static function parse_areaid($areaid) { + $parts = self::extract_areaid_parts($areaid); + + if (empty($parts[1])) { + throw new \coding_exception('Trying to parse invalid search area id ' . $areaid); + } + + $component = $parts[0]; + $area = $parts[1]; + + if (strpos($component, 'core') === 0) { + $plugin = 'core_search'; + $configprefix = str_replace('-', '_', $areaid); + } else { + $plugin = $component; + $configprefix = 'search_' . $area; + } + + return [$plugin, $configprefix]; + } + /** * Returns information about the areas which the user can access. * @@ -660,45 +687,35 @@ protected function get_areas_user_accesses($limitcourseids = false, $limitcontex } if (is_siteadmin()) { - // Admins have access to all courses regardless of enrolment. - if ($limitcourseids) { - list ($coursesql, $courseparams) = $DB->get_in_or_equal($limitcourseids); - $coursesql = 'id ' . $coursesql; - } else { - $coursesql = ''; - $courseparams = []; - } - // Get courses using the same list of fields from enrol_get_my_courses. - $courses = $DB->get_records_select('course', $coursesql, $courseparams, '', - 'id, category, sortorder, shortname, fullname, idnumber, startdate, visible, ' . - 'groupmode, groupmodeforce, cacherev'); + $allcourses = $this->get_all_courses($limitcourseids); } else { - // Get the courses where the current user has access. - $courses = enrol_get_my_courses(array('id', 'cacherev'), 'id', 0, [], - (bool)get_config('core', 'searchallavailablecourses')); + $allcourses = $mycourses = $this->get_my_courses((bool)get_config('core', 'searchallavailablecourses')); + + if (self::include_all_courses()) { + $allcourses = $this->get_all_courses($limitcourseids); + } } if (empty($limitcourseids) || in_array(SITEID, $limitcourseids)) { - $courses[SITEID] = get_course(SITEID); + $allcourses[SITEID] = get_course(SITEID); + if (isset($mycourses)) { + $mycourses[SITEID] = get_course(SITEID); + } } // Keep a list of included course context ids (needed for the block calculation below). $coursecontextids = []; $modulecms = []; - foreach ($courses as $course) { + foreach ($allcourses as $course) { if (!empty($limitcourseids) && !in_array($course->id, $limitcourseids)) { // Skip non-included courses. continue; } $coursecontext = \context_course::instance($course->id); - $coursecontextids[] = $coursecontext->id; $hasgrouprestrictions = false; - // Info about the course modules. - $modinfo = get_fast_modinfo($course); - if (!empty($areasbylevel[CONTEXT_COURSE]) && (!$limitcontextids || in_array($coursecontext->id, $limitcontextids))) { // Add the course contexts the user can view. @@ -709,6 +726,16 @@ protected function get_areas_user_accesses($limitcourseids = false, $limitcontex } } + // Skip module context if a user can't access related course. + if (isset($mycourses) && !key_exists($course->id, $mycourses)) { + continue; + } + + $coursecontextids[] = $coursecontext->id; + + // Info about the course modules. + $modinfo = get_fast_modinfo($course); + if (!empty($areasbylevel[CONTEXT_MODULE])) { // Add the module contexts the user can view (cm_info->uservisible). @@ -962,10 +989,7 @@ public function search(\stdClass $formdata, $limit = 0) { } } - $limitcourseids = false; - if (!empty($formdata->courseids)) { - $limitcourseids = $formdata->courseids; - } + $limitcourseids = $this->build_limitcourseids($formdata); $limitcontextids = false; if (!empty($formdata->contextids)) { @@ -993,6 +1017,31 @@ public function search(\stdClass $formdata, $limit = 0) { return $docs; } + /** + * Build a list of course ids to limit the search based on submitted form data. + * + * @param \stdClass $formdata Submitted search form data. + * + * @return array|bool + */ + protected function build_limitcourseids(\stdClass $formdata) { + $limitcourseids = false; + + if (!empty($formdata->mycoursesonly)) { + $limitcourseids = array_keys($this->get_my_courses(false)); + } + + if (!empty($formdata->courseids)) { + if (empty($limitcourseids)) { + $limitcourseids = $formdata->courseids; + } else { + $limitcourseids = array_intersect($limitcourseids, $formdata->courseids); + } + } + + return $limitcourseids; + } + /** * Merge separate index segments into one. */ @@ -1672,4 +1721,76 @@ public static function get_default_area_category_name() { return $default; } + + /** + * Get a list of all courses limited by ids if required. + * + * @param array|false $limitcourseids An array of course ids to limit the search to. False for no limiting. + * @return array + */ + protected function get_all_courses($limitcourseids) { + global $DB; + + if ($limitcourseids) { + list ($coursesql, $courseparams) = $DB->get_in_or_equal($limitcourseids); + $coursesql = 'id ' . $coursesql; + } else { + $coursesql = ''; + $courseparams = []; + } + + // Get courses using the same list of fields from enrol_get_my_courses. + return $DB->get_records_select('course', $coursesql, $courseparams, '', + 'id, category, sortorder, shortname, fullname, idnumber, startdate, visible, ' . + 'groupmode, groupmodeforce, cacherev'); + } + + /** + * Get a list of courses as user can access. + * + * @param bool $allaccessible Include courses user is not enrolled in, but can access. + * @return array + */ + protected function get_my_courses($allaccessible) { + return enrol_get_my_courses(array('id', 'cacherev'), 'id', 0, [], $allaccessible); + } + + /** + * Check if search all courses setting is enabled. + * + * @return bool + */ + public static function include_all_courses() { + return !empty(get_config('core', 'searchincludeallcourses')); + } + + /** + * Cleans up non existing search area. + * + * 1. Remove all configs from {config_plugins} table. + * 2. Delete all related indexed documents. + * + * @param string $areaid Search area id. + */ + public static function clean_up_non_existing_area($areaid) { + global $DB; + + if (!empty(self::get_search_area($areaid))) { + throw new \coding_exception("Area $areaid exists. Please use appropriate search area class to manipulate the data."); + } + + $parts = self::parse_areaid($areaid); + + $plugin = $parts[0]; + $configprefix = $parts[1]; + + foreach (base::get_settingnames() as $settingname) { + $name = $configprefix. $settingname; + $DB->delete_records('config_plugins', ['name' => $name, 'plugin' => $plugin]); + } + + $engine = self::instance()->get_engine(); + $engine->delete($areaid); + } + } diff --git a/search/classes/output/form/search.php b/search/classes/output/form/search.php index f4c8e459c441e..1684cff3f9f83 100644 --- a/search/classes/output/form/search.php +++ b/search/classes/output/form/search.php @@ -24,6 +24,8 @@ namespace core_search\output\form; +use core_search\manager; + defined('MOODLE_INTERNAL') || die; require_once($CFG->libdir . '/formslib.php'); @@ -105,14 +107,25 @@ function definition() { ); $mform->addElement('autocomplete', 'areaids', get_string('searcharea', 'search'), $areanames, $options); + if (is_siteadmin()) { + $limittoenrolled = false; + } else { + $limittoenrolled = !manager::include_all_courses(); + } + $options = array( 'multiple' => true, - 'limittoenrolled' => !is_siteadmin(), + 'limittoenrolled' => $limittoenrolled, 'noselectionstring' => get_string('allcourses', 'search'), ); $mform->addElement('course', 'courseids', get_string('courses', 'core'), $options); $mform->setType('courseids', PARAM_INT); + if (manager::include_all_courses() || !empty(get_config('core', 'searchallavailablecourses'))) { + $mform->addElement('checkbox', 'mycoursesonly', get_string('mycoursesonly', 'search')); + $mform->setType('mycoursesonly', PARAM_INT); + } + // If the search engine can search by user, and the user is logged in (so we have // permission to call the user-listing web service) then show the user selector. if ($search->get_engine()->supports_users() && isloggedin()) { diff --git a/search/engine/solr/tests/engine_test.php b/search/engine/solr/tests/engine_test.php index 372e9600b109b..0f43ed8273351 100644 --- a/search/engine/solr/tests/engine_test.php +++ b/search/engine/solr/tests/engine_test.php @@ -821,7 +821,7 @@ public function test_context_restriction() { unset($querydata->courseids); // Restrict both area and context. - $querydata->areaids = ['core_course-mycourse']; + $querydata->areaids = ['core_course-course']; $results = $this->search->search($querydata); $this->assert_result_titles(['Course 1'], $results); diff --git a/search/index.php b/search/index.php index ef80dc333bcdc..83e840a75dcc9 100644 --- a/search/index.php +++ b/search/index.php @@ -29,6 +29,7 @@ $title = optional_param('title', '', PARAM_NOTAGS); $contextid = optional_param('context', 0, PARAM_INT); $cat = optional_param('cat', '', PARAM_NOTAGS); +$mycoursesonly = optional_param('mycoursesonly', 0, PARAM_INT); if (\core_search\manager::is_search_area_categories_enabled()) { $cat = \core_search\manager::get_search_area_category_by_name($cat); @@ -113,6 +114,7 @@ $data->timeend = optional_param('timeend', 0, PARAM_INT); $data->context = $contextid; + $data->mycoursesonly = $mycoursesonly; $mform->set_data($data); } @@ -152,6 +154,7 @@ } $urlparams['timestart'] = $data->timestart; $urlparams['timeend'] = $data->timeend; + $urlparams['mycoursesonly'] = isset($data->mycoursesonly) ? $data->mycoursesonly : 0; } if ($cat instanceof \core_search\area_category) { diff --git a/search/tests/base_test.php b/search/tests/base_test.php index d6015ee7a5e9f..f55ac434964ab 100644 --- a/search/tests/base_test.php +++ b/search/tests/base_test.php @@ -169,4 +169,13 @@ public function test_get_category_names() { $expected = ['core-other']; $this->assertEquals($expected, $stub->get_category_names()); } + + /** + * Test getting all required search area setting names. + */ + public function test_get_settingnames() { + $expected = array('_enabled', '_indexingstart', '_indexingend', '_lastindexrun', + '_docsignored', '_docsprocessed', '_recordsprocessed', '_partial'); + $this->assertEquals($expected, \core_search\base::get_settingnames()); + } } diff --git a/search/tests/fixtures/testable_core_search.php b/search/tests/fixtures/testable_core_search.php index 55bbe854746b2..62193b4ec7848 100644 --- a/search/tests/fixtures/testable_core_search.php +++ b/search/tests/fixtures/testable_core_search.php @@ -118,4 +118,17 @@ public static function is_search_area($classname) { public static function fake_current_time($faketime = 0.0) { static::$phpunitfaketime = $faketime; } + + /** + * Makes build_limitcourseids method public for testing. + * + * @param \stdClass $formdata Submitted search form data. + * + * @return array|bool + */ + public function build_limitcourseids(\stdClass $formdata) { + $limitcourseids = parent::build_limitcourseids($formdata); + + return $limitcourseids; + } } diff --git a/search/tests/manager_test.php b/search/tests/manager_test.php index d978eff42d0bb..6b04ee9dfddc0 100644 --- a/search/tests/manager_test.php +++ b/search/tests/manager_test.php @@ -38,12 +38,24 @@ */ class search_manager_testcase extends advanced_testcase { + /** + * Forum area id. + * + * @var string + */ + protected $forumpostareaid = null; - protected $mycoursesareaid = null; + + /** + * Courses area id. + * + * @var string + */ + protected $coursesareaid = null; public function setUp() { $this->forumpostareaid = \core_search\manager::generate_areaid('mod_forum', 'post'); - $this->mycoursesareaid = \core_search\manager::generate_areaid('core_course', 'mycourse'); + $this->coursesareaid = \core_search\manager::generate_areaid('core_course', 'course'); } protected function tearDown() { @@ -459,7 +471,7 @@ public function test_context_indexing() { // Confirm that some areas for different types of context were skipped. $this->assertNotFalse(strpos($log, "area: Users\n Skipping")); - $this->assertNotFalse(strpos($log, "area: My courses\n Skipping")); + $this->assertNotFalse(strpos($log, "area: Courses\n Skipping")); // Confirm that another module area had no results. $this->assertNotFalse(strpos($log, "area: Page\n No documents")); @@ -475,7 +487,7 @@ public function test_context_indexing() { $this->assertNotFalse(strpos($log, "area: Forum - posts\n Processed 3 ")); // The course area was also included this time. - $this->assertNotFalse(strpos($log, "area: My courses\n Processed 1 ")); + $this->assertNotFalse(strpos($log, "area: Courses\n Processed 1 ")); // Confirm that another module area had results too. $this->assertNotFalse(strpos($log, "area: Page\n Processed 1 ")); @@ -531,6 +543,8 @@ public function test_search_user_accesses() { $course1ctx = context_course::instance($course1->id); $course2 = $this->getDataGenerator()->create_course(); $course2ctx = context_course::instance($course2->id); + $course3 = $this->getDataGenerator()->create_course(); + $course3ctx = context_course::instance($course3->id); $teacher = $this->getDataGenerator()->create_user(); $teacherctx = context_user::instance($teacher->id); $student = $this->getDataGenerator()->create_user(); @@ -548,6 +562,8 @@ public function test_search_user_accesses() { $context1 = context_module::instance($forum1->cmid); $context2 = context_module::instance($forum2->cmid); $context3 = context_module::instance($forum3->cmid); + $forum4 = $this->getDataGenerator()->create_module('forum', array('course' => $course3->id)); + $context4 = context_module::instance($forum4->cmid); $search = testable_core_search::instance(); $mockareaid = \core_search\manager::generate_areaid('core_mocksearch', 'mock_search_area'); @@ -563,7 +579,7 @@ public function test_search_user_accesses() { $this->setUser($noaccess); $contexts = $search->get_areas_user_accesses()->usercontexts; $this->assertEquals(array($frontpageforumcontext->id => $frontpageforumcontext->id), $contexts[$this->forumpostareaid]); - $this->assertEquals(array($sitectx->id => $sitectx->id), $contexts[$this->mycoursesareaid]); + $this->assertEquals(array($sitectx->id => $sitectx->id), $contexts[$this->coursesareaid]); $mockctxs = array($noaccessctx->id => $noaccessctx->id, $frontpagectx->id => $frontpagectx->id); $this->assertEquals($mockctxs, $contexts[$mockareaid]); @@ -573,7 +589,7 @@ public function test_search_user_accesses() { $context2->id => $context2->id); $this->assertEquals($frontpageandcourse1, $contexts[$this->forumpostareaid]); $this->assertEquals(array($sitectx->id => $sitectx->id, $course1ctx->id => $course1ctx->id), - $contexts[$this->mycoursesareaid]); + $contexts[$this->coursesareaid]); $mockctxs = array($teacherctx->id => $teacherctx->id, $frontpagectx->id => $frontpagectx->id, $course1ctx->id => $course1ctx->id); $this->assertEquals($mockctxs, $contexts[$mockareaid]); @@ -582,7 +598,7 @@ public function test_search_user_accesses() { $contexts = $search->get_areas_user_accesses()->usercontexts; $this->assertEquals($frontpageandcourse1, $contexts[$this->forumpostareaid]); $this->assertEquals(array($sitectx->id => $sitectx->id, $course1ctx->id => $course1ctx->id), - $contexts[$this->mycoursesareaid]); + $contexts[$this->coursesareaid]); $mockctxs = array($studentctx->id => $studentctx->id, $frontpagectx->id => $frontpagectx->id, $course1ctx->id => $course1ctx->id); $this->assertEquals($mockctxs, $contexts[$mockareaid]); @@ -601,23 +617,23 @@ public function test_search_user_accesses() { $context2->id => $context2->id, $context3->id => $context3->id); $this->assertEquals($allcontexts, $contexts[$this->forumpostareaid]); $this->assertEquals(array($sitectx->id => $sitectx->id, $course1ctx->id => $course1ctx->id, - $course2ctx->id => $course2ctx->id), $contexts[$this->mycoursesareaid]); + $course2ctx->id => $course2ctx->id), $contexts[$this->coursesareaid]); $contexts = $search->get_areas_user_accesses(array($course1->id, $course2->id))->usercontexts; $allcontexts = array($context1->id => $context1->id, $context2->id => $context2->id, $context3->id => $context3->id); $this->assertEquals($allcontexts, $contexts[$this->forumpostareaid]); $this->assertEquals(array($course1ctx->id => $course1ctx->id, - $course2ctx->id => $course2ctx->id), $contexts[$this->mycoursesareaid]); + $course2ctx->id => $course2ctx->id), $contexts[$this->coursesareaid]); $contexts = $search->get_areas_user_accesses(array($course2->id))->usercontexts; $allcontexts = array($context3->id => $context3->id); $this->assertEquals($allcontexts, $contexts[$this->forumpostareaid]); - $this->assertEquals(array($course2ctx->id => $course2ctx->id), $contexts[$this->mycoursesareaid]); + $this->assertEquals(array($course2ctx->id => $course2ctx->id), $contexts[$this->coursesareaid]); $contexts = $search->get_areas_user_accesses(array($course1->id))->usercontexts; $allcontexts = array($context1->id => $context1->id, $context2->id => $context2->id); $this->assertEquals($allcontexts, $contexts[$this->forumpostareaid]); - $this->assertEquals(array($course1ctx->id => $course1ctx->id), $contexts[$this->mycoursesareaid]); + $this->assertEquals(array($course1ctx->id => $course1ctx->id), $contexts[$this->coursesareaid]); // Test context limited search with no course limit. $contexts = $search->get_areas_user_accesses(false, @@ -625,25 +641,39 @@ public function test_search_user_accesses() { $this->assertEquals([$frontpageforumcontext->id => $frontpageforumcontext->id], $contexts[$this->forumpostareaid]); $this->assertEquals([$course2ctx->id => $course2ctx->id], - $contexts[$this->mycoursesareaid]); + $contexts[$this->coursesareaid]); // Test context limited search with course limit. $contexts = $search->get_areas_user_accesses([$course1->id, $course2->id], [$frontpageforumcontext->id, $course2ctx->id])->usercontexts; $this->assertArrayNotHasKey($this->forumpostareaid, $contexts); $this->assertEquals([$course2ctx->id => $course2ctx->id], - $contexts[$this->mycoursesareaid]); + $contexts[$this->coursesareaid]); // Single context and course. $contexts = $search->get_areas_user_accesses([$course1->id], [$context1->id])->usercontexts; $this->assertEquals([$context1->id => $context1->id], $contexts[$this->forumpostareaid]); - $this->assertArrayNotHasKey($this->mycoursesareaid, $contexts); + $this->assertArrayNotHasKey($this->coursesareaid, $contexts); + + // Enable "Include all visible courses" feature. + set_config('searchincludeallcourses', 1); + $contexts = $search->get_areas_user_accesses()->usercontexts; + $expected = [ + $sitectx->id => $sitectx->id, + $course1ctx->id => $course1ctx->id, + $course2ctx->id => $course2ctx->id, + $course3ctx->id => $course3ctx->id + ]; + // Check that a student has assess to all courses data when "searchincludeallcourses" is enabled. + $this->assertEquals($expected, $contexts[$this->coursesareaid]); + // But at the same time doesn't have access to activities in the courses that the student can't access. + $this->assertFalse(key_exists($context4->id, $contexts[$this->forumpostareaid])); // For admins, this is still limited only if we specify the things, so it should be same. $this->setAdminUser(); $contexts = $search->get_areas_user_accesses([$course1->id], [$context1->id])->usercontexts; $this->assertEquals([$context1->id => $context1->id], $contexts[$this->forumpostareaid]); - $this->assertArrayNotHasKey($this->mycoursesareaid, $contexts); + $this->assertArrayNotHasKey($this->coursesareaid, $contexts); } /** @@ -1289,4 +1319,134 @@ public function test_get_search_area_category_by_name() { $testcategory = \core_search\manager::get_search_area_category_by_name('test_random_name'); $this->assertEquals('core-course-content', $testcategory->get_name()); } + + /** + * Test that we can check that "Include all visible courses" feature is enabled. + */ + public function test_include_all_courses_enabled() { + $this->resetAfterTest(); + $this->assertFalse(\core_search\manager::include_all_courses()); + set_config('searchincludeallcourses', 1); + $this->assertTrue(\core_search\manager::include_all_courses()); + } + + /** + * Test that we can correctly build a list of courses for a course filter for the search results. + */ + public function test_build_limitcourseids() { + global $USER; + + $this->resetAfterTest(); + $this->setAdminUser(); + + $course1 = $this->getDataGenerator()->create_course(); + $course2 = $this->getDataGenerator()->create_course(); + $course3 = $this->getDataGenerator()->create_course(); + $course4 = $this->getDataGenerator()->create_course(); + + $this->getDataGenerator()->enrol_user($USER->id, $course1->id); + $this->getDataGenerator()->enrol_user($USER->id, $course3->id); + + $search = testable_core_search::instance(); + + $formdata = new stdClass(); + $formdata->courseids = []; + $formdata->mycoursesonly = false; + $limitcourseids = $search->build_limitcourseids($formdata); + $this->assertEquals(false, $limitcourseids); + + $formdata->courseids = []; + $formdata->mycoursesonly = true; + $limitcourseids = $search->build_limitcourseids($formdata); + $this->assertEquals([$course1->id, $course3->id], $limitcourseids); + + $formdata->courseids = [$course1->id, $course2->id, $course4->id]; + $formdata->mycoursesonly = false; + $limitcourseids = $search->build_limitcourseids($formdata); + $this->assertEquals([$course1->id, $course2->id, $course4->id], $limitcourseids); + + $formdata->courseids = [$course1->id, $course2->id, $course4->id]; + $formdata->mycoursesonly = true; + $limitcourseids = $search->build_limitcourseids($formdata); + $this->assertEquals([$course1->id], $limitcourseids); + } + + /** + * Test data for test_parse_areaid test fucntion. + * + * @return array + */ + public function parse_search_area_id_data_provider() { + return [ + ['mod_book-chapter', ['mod_book', 'search_chapter']], + ['mod_customcert-activity', ['mod_customcert', 'search_activity']], + ['core_course-mycourse', ['core_search', 'core_course_mycourse']], + ]; + } + + /** + * Test that manager class can parse area id correctly. + * @dataProvider parse_search_area_id_data_provider + * + * @param string $areaid Area id to parse. + * @param array $expected Expected result of parsing. + */ + public function test_parse_search_area_id($areaid, $expected) { + $this->assertEquals($expected, \core_search\manager::parse_areaid($areaid)); + } + + /** + * Test that manager class will throw an exception when parsing an invalid area id. + */ + public function test_parse_invalid_search_area_id() { + $this->expectException('coding_exception'); + $this->expectExceptionMessage('Trying to parse invalid search area id invalid_area'); + \core_search\manager::parse_areaid('invalid_area'); + } + + /** + * Test getting a coding exception when trying to lean up existing search area. + */ + public function test_cleaning_up_existing_search_area() { + $expectedmessage = "Area mod_assign-activity exists. Please use appropriate search area class to manipulate the data."; + + $this->expectException('coding_exception'); + $this->expectExceptionMessage($expectedmessage); + + \core_search\manager::clean_up_non_existing_area('mod_assign-activity'); + } + + /** + * Test clean up of non existing search area. + */ + public function test_clean_up_non_existing_search_area() { + global $DB; + + $this->resetAfterTest(); + + $areaid = 'core_course-mycourse'; + $plugin = 'core_search'; + + // Get all settings to DB and make sure they are there. + foreach (\core_search\base::get_settingnames() as $settingname) { + $record = new stdClass(); + $record->plugin = $plugin; + $record->name = 'core_course_mycourse'. $settingname; + $record->value = 'test'; + + $DB->insert_record('config_plugins', $record); + $this->assertTrue($DB->record_exists('config_plugins', ['plugin' => $plugin, 'name' => $record->name])); + } + + // Clean up the search area. + \core_search\manager::clean_up_non_existing_area($areaid); + + // Check that records are not in DB after we ran clean up. + foreach (\core_search\base::get_settingnames() as $settingname) { + $plugin = 'core_search'; + $name = 'core_course_mycourse'. $settingname; + $this->assertFalse($DB->record_exists('config_plugins', ['plugin' => $plugin, 'name' => $name])); + } + } + } diff --git a/search/upgrade.txt b/search/upgrade.txt index b355730cd5839..7453c2ceac383 100644 --- a/search/upgrade.txt +++ b/search/upgrade.txt @@ -1,6 +1,18 @@ This files describes API changes in /search/*, information provided here is intended especially for developers. +=== 3.7 === + +* Search areas now have categories and can optionally implement get_category_names method to + display search results of the area in the required tab on the search results screen (if this + feature is enabled). +* Added a new call back search_area_categories. Plugins can implement this method in lib.php and + return a list of custom search area categories (\core_search\area_category) and associated with + them search areas. This will bring additional custom tabs to the search results screen. +* Added \core_search\manager::clean_up_non_existing_area method to clean up removed or renamed + search areas. To support that a new adhoc task core\task\clean_up_deleted_search_area_task + added. + === 3.5 === * Search areas may now optionally implement the get_contexts_to_reindex function (for modules and diff --git a/version.php b/version.php index e249b794b68d6..867466154b6cc 100644 --- a/version.php +++ b/version.php @@ -29,7 +29,7 @@ defined('MOODLE_INTERNAL') || die(); -$version = 2019030100.00; // YYYYMMDD = weekly release date of this DEV branch. +$version = 2019030100.01; // YYYYMMDD = weekly release date of this DEV branch. // RR = release increments - 00 in DEV branches. // .XX = incremental changes.