From cfa00fc565b8c2afb39200d67d9cd673ad9927bf Mon Sep 17 00:00:00 2001 From: sam marshall Date: Wed, 22 Nov 2017 14:31:32 +0000 Subject: [PATCH 1/2] MDL-60880 core_search: Allow search of specific context (back-end) Adds back-end support for restricting searches to specified context ids (for example so it is possible to search only a specific forum). --- search/classes/manager.php | 83 +++++++++++++---- search/engine/solr/classes/engine.php | 9 +- search/engine/solr/tests/engine_test.php | 89 ++++++++++++++++++- search/tests/base_test.php | 1 + search/tests/fixtures/mock_search_area.php | 2 +- .../tests/fixtures/testable_core_search.php | 4 +- search/tests/generator/lib.php | 2 +- search/tests/manager_test.php | 58 +++++++++++- 8 files changed, 221 insertions(+), 27 deletions(-) diff --git a/search/classes/manager.php b/search/classes/manager.php index fc5a7188e10..e8f93a55796 100644 --- a/search/classes/manager.php +++ b/search/classes/manager.php @@ -350,14 +350,22 @@ class manager { * information and there will be a performance benefit on passing only some contexts * instead of the whole context array set. * + * The areas can be limited by course id and context id. If specifying context ids, results + * are limited to the exact context ids specified and not their children (for example, giving + * the course context id would result in including search items with the course context id, and + * not anything from a context inside the course). For performance, you should also specify + * course id(s) when using context ids. + * * @param array|false $limitcourseids An array of course ids to limit the search to. False for no limiting. + * @param array|false $limitcontextids An array of context ids to limit the search to. False for no limiting. * @return bool|array Indexed by area identifier (component + area name). Returns true if the user can see everything. */ - protected function get_areas_user_accesses($limitcourseids = false) { + protected function get_areas_user_accesses($limitcourseids = false, $limitcontextids = false) { global $DB, $USER; - // All results for admins. Eventually we could add a new capability for managers. - if (is_siteadmin()) { + // All results for admins (unless they have chosen to limit results). Eventually we could + // add a new capability for managers. + if (is_siteadmin() && !$limitcourseids && !$limitcontextids) { return true; } @@ -382,23 +390,42 @@ class manager { // want to allow guests to retrieve data from them. $systemcontextid = \context_system::instance()->id; - foreach ($areasbylevel[CONTEXT_SYSTEM] as $areaid => $searchclass) { - $areascontexts[$areaid][$systemcontextid] = $systemcontextid; + if (!$limitcontextids || in_array($systemcontextid, $limitcontextids)) { + foreach ($areasbylevel[CONTEXT_SYSTEM] as $areaid => $searchclass) { + $areascontexts[$areaid][$systemcontextid] = $systemcontextid; + } } } if (!empty($areasbylevel[CONTEXT_USER])) { if ($usercontext = \context_user::instance($USER->id, IGNORE_MISSING)) { - // Extra checking although only logged users should reach this point, guest users have a valid context id. - foreach ($areasbylevel[CONTEXT_USER] as $areaid => $searchclass) { - $areascontexts[$areaid][$usercontext->id] = $usercontext->id; + if (!$limitcontextids || in_array($usercontext->id, $limitcontextids)) { + // Extra checking although only logged users should reach this point, guest users have a valid context id. + foreach ($areasbylevel[CONTEXT_USER] as $areaid => $searchclass) { + $areascontexts[$areaid][$usercontext->id] = $usercontext->id; + } } } } - // Get the courses where the current user has access. - $courses = enrol_get_my_courses(array('id', 'cacherev'), 'id', 0, [], - (bool)get_config('core', 'searchallavailablecourses')); + 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'); + } 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')); + } if (empty($limitcourseids) || in_array(SITEID, $limitcourseids)) { $courses[SITEID] = get_course(SITEID); @@ -419,7 +446,8 @@ class manager { // Info about the course modules. $modinfo = get_fast_modinfo($course); - if (!empty($areasbylevel[CONTEXT_COURSE])) { + if (!empty($areasbylevel[CONTEXT_COURSE]) && + (!$limitcontextids || in_array($coursecontext->id, $limitcontextids))) { // Add the course contexts the user can view. foreach ($areasbylevel[CONTEXT_COURSE] as $areaid => $searchclass) { if ($course->visible || has_capability('moodle/course:viewhiddencourses', $coursecontext)) { @@ -438,6 +466,10 @@ class manager { $modinstances = $modinfo->get_instances_of($modulename); foreach ($modinstances as $modinstance) { + // Skip module context if not included in list of context ids. + if ($limitcontextids && !in_array($modinstance->context->id, $limitcontextids)) { + continue; + } if ($modinstance->uservisible) { $areascontexts[$areaid][$modinstance->context->id] = $modinstance->context->id; } @@ -458,6 +490,14 @@ class manager { // Get list of course contexts. list ($contextsql, $contextparams) = $DB->get_in_or_equal($coursecontextids); + // Get list of block context (if limited). + $blockcontextwhere = ''; + $blockcontextparams = []; + if ($limitcontextids) { + list ($blockcontextsql, $blockcontextparams) = $DB->get_in_or_equal($limitcontextids); + $blockcontextwhere = 'AND x.id ' . $blockcontextsql; + } + // Query all blocks that are within an included course, and are set to be visible, and // in a supported page type (basically just course view). This query could be // extended (or a second query added) to support blocks that are within a module @@ -472,6 +512,7 @@ class manager { AND bp.subpage = '' AND bp.visible = 0 WHERE bi.parentcontextid $contextsql + $blockcontextwhere AND bi.blockname $blocknamesql AND bi.subpagepattern IS NULL AND (bi.pagetypepattern = 'site-index' @@ -479,7 +520,7 @@ class manager { OR bi.pagetypepattern = 'course-*' OR bi.pagetypepattern = '*') AND bp.id IS NULL", - array_merge([CONTEXT_BLOCK], $contextparams, $blocknameparams)); + array_merge([CONTEXT_BLOCK], $contextparams, $blockcontextparams, $blocknameparams)); $blockcontextsbyname = []; foreach ($blockrecs as $blockrec) { if (empty($blockcontextsbyname[$blockrec->blockname])) { @@ -569,8 +610,13 @@ class manager { * * It might return the results from the cache instead. * - * @param stdClass $formdata - * @param int $limit The maximum number of documents to return + * Valid formdata options include: + * - q (query text) + * - courseids (optional list of course ids to restrict) + * - contextids (optional list of context ids to restrict) + * + * @param \stdClass $formdata Query input data (usually from search form) + * @param int $limit The maximum number of documents to return * @return \core_search\document[] */ public function search(\stdClass $formdata, $limit = 0) { @@ -613,10 +659,15 @@ class manager { $limitcourseids = $formdata->courseids; } + $limitcontextids = false; + if (!empty($formdata->contextids)) { + $limitcontextids = $formdata->contextids; + } + // Clears previous query errors. $this->engine->clear_query_error(); - $areascontexts = $this->get_areas_user_accesses($limitcourseids); + $areascontexts = $this->get_areas_user_accesses($limitcourseids, $limitcontextids); if (!$areascontexts) { // User can not access any context. $docs = array(); diff --git a/search/engine/solr/classes/engine.php b/search/engine/solr/classes/engine.php index 1f0a1be635b..1df982f1292 100644 --- a/search/engine/solr/classes/engine.php +++ b/search/engine/solr/classes/engine.php @@ -137,6 +137,11 @@ class engine extends \core_search\engine { // Create the query object. $query = $this->create_user_query($filters, $usercontexts); + // If the query cannot have results, return none. + if (!$query) { + return []; + } + // We expect good match rates, so for our first get, we will get a small number of records. // This significantly speeds solr response time for first few pages. $query->setRows(min($limit * 3, static::QUERY_SIZE)); @@ -242,7 +247,7 @@ class engine extends \core_search\engine { * * @param stdClass $filters Containing query and filters. * @param array $usercontexts Contexts where the user has access. True if the user can access all contexts. - * @return SolrDisMaxQuery + * @return \SolrDisMaxQuery|null Query object or null if they can't get any results */ protected function create_user_query($filters, $usercontexts) { global $USER; @@ -305,7 +310,7 @@ class engine extends \core_search\engine { } if (empty($allcontexts)) { // This means there are no valid contexts for them, so they get no results. - return array(); + return null; } $query->addFilterQuery('contextid:(' . implode(' OR ', $allcontexts) . ')'); } diff --git a/search/engine/solr/tests/engine_test.php b/search/engine/solr/tests/engine_test.php index f2720555aa0..6d3d1179423 100644 --- a/search/engine/solr/tests/engine_test.php +++ b/search/engine/solr/tests/engine_test.php @@ -209,7 +209,7 @@ class search_solr_engine_testcase extends advanced_testcase { // Based on core_mocksearch\search\indexer. $this->assertEquals($USER->id, $results[0]->get('userid')); - $this->assertEquals(\context_system::instance()->id, $results[0]->get('contextid')); + $this->assertEquals(\context_course::instance(SITEID)->id, $results[0]->get('contextid')); // Do a test to make sure we aren't searching non-query fields, like areaid. $querydata->q = \core_search\manager::generate_areaid('core_mocksearch', 'mock_search_area'); @@ -758,4 +758,91 @@ class search_solr_engine_testcase extends advanced_testcase { $this->assertCount(10, $results->results); $this->assertEquals(1, $results->actualpage); } + + /** + * Tests searching for results restricted to context id. + */ + public function test_context_restriction() { + // Use real search areas. + $this->search->clear_static(); + $this->search->add_core_search_areas(); + + // Create 2 courses and some forums. + $generator = $this->getDataGenerator(); + $course1 = $generator->create_course(['fullname' => 'Course 1', 'summary' => 'xyzzy']); + $contextc1 = \context_course::instance($course1->id); + $course1forum1 = $generator->create_module('forum', ['course' => $course1, + 'name' => 'C1F1', 'intro' => 'xyzzy']); + $contextc1f1 = \context_module::instance($course1forum1->cmid); + $course1forum2 = $generator->create_module('forum', ['course' => $course1, + 'name' => 'C1F2', 'intro' => 'xyzzy']); + $contextc1f2 = \context_module::instance($course1forum2->cmid); + $course2 = $generator->create_course(['fullname' => 'Course 2', 'summary' => 'xyzzy']); + $contextc2 = \context_course::instance($course1->id); + $course2forum = $generator->create_module('forum', ['course' => $course2, + 'name' => 'C2F', 'intro' => 'xyzzy']); + $contextc2f = \context_module::instance($course2forum->cmid); + + // Index the courses and forums. + $this->search->index(); + + // Search as admin user should find everything. + $querydata = new stdClass(); + $querydata->q = 'xyzzy'; + $results = $this->search->search($querydata); + $this->assert_result_titles( + ['Course 1', 'Course 2', 'C1F1', 'C1F2', 'C2F'], $results); + + // Admin user manually restricts results by context id to include one course and one forum. + $querydata->contextids = [$contextc2f->id, $contextc1->id]; + $results = $this->search->search($querydata); + $this->assert_result_titles(['Course 1', 'C2F'], $results); + + // Student enrolled in only one course, same restriction, only has the available results. + $student2 = $generator->create_user(); + $generator->enrol_user($student2->id, $course2->id, 'student'); + $this->setUser($student2); + $results = $this->search->search($querydata); + $this->assert_result_titles(['C2F'], $results); + + // Student enrolled in both courses, same restriction, same results as admin. + $student1 = $generator->create_user(); + $generator->enrol_user($student1->id, $course1->id, 'student'); + $generator->enrol_user($student1->id, $course2->id, 'student'); + $this->setUser($student1); + $results = $this->search->search($querydata); + $this->assert_result_titles(['Course 1', 'C2F'], $results); + + // Restrict both course and context. + $querydata->courseids = [$course2->id]; + $results = $this->search->search($querydata); + $this->assert_result_titles(['C2F'], $results); + unset($querydata->courseids); + + // Restrict both area and context. + $querydata->areaids = ['core_course-mycourse']; + $results = $this->search->search($querydata); + $this->assert_result_titles(['Course 1'], $results); + + // Restrict area and context, incompatibly - this has no results (and doesn't do a query). + $querydata->contextids = [$contextc2f->id]; + $results = $this->search->search($querydata); + $this->assert_result_titles([], $results); + } + + /** + * Asserts that the returned documents have the expected titles (regardless of order). + * + * @param string[] $expected List of expected document titles + * @param \core_search\document[] $results List of returned documents + */ + protected function assert_result_titles(array $expected, array $results) { + $titles = []; + foreach ($results as $result) { + $titles[] = $result->get('title'); + } + sort($titles); + sort($expected); + $this->assertEquals($expected, $titles); + } } diff --git a/search/tests/base_test.php b/search/tests/base_test.php index 6b5d0f006a5..5880b231cf5 100644 --- a/search/tests/base_test.php +++ b/search/tests/base_test.php @@ -106,6 +106,7 @@ class search_base_testcase extends advanced_testcase { // Construct the search document. $rec = new \stdClass(); + $rec->contextid = 1; $area = new core_mocksearch\search\mock_search_area(); $record = $this->generator->create_record($rec); $document = $area->get_document($record); diff --git a/search/tests/fixtures/mock_search_area.php b/search/tests/fixtures/mock_search_area.php index 1ed17d3402e..b8049b8338c 100644 --- a/search/tests/fixtures/mock_search_area.php +++ b/search/tests/fixtures/mock_search_area.php @@ -33,7 +33,7 @@ class mock_search_area extends \core_search\base { * Multiple context level so we can test get_areas_user_accesses. * @var int[] */ - protected static $levels = [CONTEXT_SYSTEM, CONTEXT_USER]; + protected static $levels = [CONTEXT_COURSE, CONTEXT_USER]; /** * To make things easier, base class required config stuff. diff --git a/search/tests/fixtures/testable_core_search.php b/search/tests/fixtures/testable_core_search.php index 65ae8ce3962..55bbe854746 100644 --- a/search/tests/fixtures/testable_core_search.php +++ b/search/tests/fixtures/testable_core_search.php @@ -72,8 +72,8 @@ class testable_core_search extends \core_search\manager { * * @return array */ - public function get_areas_user_accesses($limitcourseids = false) { - return parent::get_areas_user_accesses($limitcourseids); + public function get_areas_user_accesses($limitcourseids = false, $limitcontextids = false) { + return parent::get_areas_user_accesses($limitcourseids, $limitcontextids); } /** diff --git a/search/tests/generator/lib.php b/search/tests/generator/lib.php index a6e99f7d3ea..562232e1f99 100644 --- a/search/tests/generator/lib.php +++ b/search/tests/generator/lib.php @@ -118,7 +118,7 @@ class core_search_generator extends component_generator_base { } if (!isset($options->contextid)) { - $info->contextid = \context_system::instance()->id; + $info->contextid = \context_course::instance(SITEID)->id; } else { $info->contextid = $options->contextid; } diff --git a/search/tests/manager_test.php b/search/tests/manager_test.php index 95fa82c1f47..701a69440ba 100644 --- a/search/tests/manager_test.php +++ b/search/tests/manager_test.php @@ -442,6 +442,7 @@ class search_manager_testcase extends advanced_testcase { $this->resetAfterTest(); $frontpage = $DB->get_record('course', array('id' => SITEID)); + $frontpagectx = context_course::instance($frontpage->id); $course1 = $this->getDataGenerator()->create_course(); $course1ctx = context_course::instance($course1->id); $course2 = $this->getDataGenerator()->create_course(); @@ -473,14 +474,13 @@ class search_manager_testcase extends advanced_testcase { $this->assertTrue($search->get_areas_user_accesses()); $sitectx = \context_course::instance(SITEID); - $systemctxid = \context_system::instance()->id; // Can access the frontpage ones. $this->setUser($noaccess); $contexts = $search->get_areas_user_accesses(); $this->assertEquals(array($frontpageforumcontext->id => $frontpageforumcontext->id), $contexts[$this->forumpostareaid]); $this->assertEquals(array($sitectx->id => $sitectx->id), $contexts[$this->mycoursesareaid]); - $mockctxs = array($noaccessctx->id => $noaccessctx->id, $systemctxid => $systemctxid); + $mockctxs = array($noaccessctx->id => $noaccessctx->id, $frontpagectx->id => $frontpagectx->id); $this->assertEquals($mockctxs, $contexts[$mockareaid]); $this->setUser($teacher); @@ -490,7 +490,8 @@ class search_manager_testcase extends advanced_testcase { $this->assertEquals($frontpageandcourse1, $contexts[$this->forumpostareaid]); $this->assertEquals(array($sitectx->id => $sitectx->id, $course1ctx->id => $course1ctx->id), $contexts[$this->mycoursesareaid]); - $mockctxs = array($teacherctx->id => $teacherctx->id, $systemctxid => $systemctxid); + $mockctxs = array($teacherctx->id => $teacherctx->id, + $frontpagectx->id => $frontpagectx->id, $course1ctx->id => $course1ctx->id); $this->assertEquals($mockctxs, $contexts[$mockareaid]); $this->setUser($student); @@ -498,7 +499,8 @@ class search_manager_testcase extends advanced_testcase { $this->assertEquals($frontpageandcourse1, $contexts[$this->forumpostareaid]); $this->assertEquals(array($sitectx->id => $sitectx->id, $course1ctx->id => $course1ctx->id), $contexts[$this->mycoursesareaid]); - $mockctxs = array($studentctx->id => $studentctx->id, $systemctxid => $systemctxid); + $mockctxs = array($studentctx->id => $studentctx->id, + $frontpagectx->id => $frontpagectx->id, $course1ctx->id => $course1ctx->id); $this->assertEquals($mockctxs, $contexts[$mockareaid]); // Hide the activity. @@ -532,6 +534,32 @@ class search_manager_testcase extends advanced_testcase { $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]); + + // Test context limited search with no course limit. + $contexts = $search->get_areas_user_accesses(false, + [$frontpageforumcontext->id, $course2ctx->id]); + $this->assertEquals([$frontpageforumcontext->id => $frontpageforumcontext->id], + $contexts[$this->forumpostareaid]); + $this->assertEquals([$course2ctx->id => $course2ctx->id], + $contexts[$this->mycoursesareaid]); + + // Test context limited search with course limit. + $contexts = $search->get_areas_user_accesses([$course1->id, $course2->id], + [$frontpageforumcontext->id, $course2ctx->id]); + $this->assertArrayNotHasKey($this->forumpostareaid, $contexts); + $this->assertEquals([$course2ctx->id => $course2ctx->id], + $contexts[$this->mycoursesareaid]); + + // Single context and course. + $contexts = $search->get_areas_user_accesses([$course1->id], [$context1->id]); + $this->assertEquals([$context1->id => $context1->id], $contexts[$this->forumpostareaid]); + $this->assertArrayNotHasKey($this->mycoursesareaid, $contexts); + + // 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]); + $this->assertEquals([$context1->id => $context1->id], $contexts[$this->forumpostareaid]); + $this->assertArrayNotHasKey($this->mycoursesareaid, $contexts); } /** @@ -540,6 +568,8 @@ class search_manager_testcase extends advanced_testcase { * @return void */ public function test_search_user_accesses_blocks() { + global $DB; + $this->resetAfterTest(); $this->setAdminUser(); @@ -631,6 +661,26 @@ class search_manager_testcase extends advanced_testcase { $this->setUser($student1); $limitedcontexts = $search->get_areas_user_accesses([$course3->id]); $this->assertEquals($contexts['block_html-content'], $limitedcontexts['block_html-content']); + + // Get block context ids for the blocks that appear. + global $DB; + $blockcontextids = $DB->get_fieldset_sql(' + SELECT x.id + FROM {block_instances} bi + JOIN {context} x ON x.instanceid = bi.id AND x.contextlevel = ? + WHERE (parentcontextid = ? OR parentcontextid = ?) + AND blockname = ? + ORDER BY bi.id', [CONTEXT_BLOCK, $context1->id, $context3->id, 'html']); + + // Context limited search (no course). + $contexts = $search->get_areas_user_accesses(false, + [$blockcontextids[0], $blockcontextids[2]]); + $this->assertCount(2, $contexts['block_html-content']); + + // Context limited search (with course 3). + $contexts = $search->get_areas_user_accesses([$course2->id, $course3->id], + [$blockcontextids[0], $blockcontextids[2]]); + $this->assertCount(1, $contexts['block_html-content']); } /** From 7f9fb2c846d4086b18b5e4e157aa36f9a4074126 Mon Sep 17 00:00:00 2001 From: sam marshall Date: Wed, 22 Nov 2017 16:47:29 +0000 Subject: [PATCH 2/2] MDL-60880 core_search: Allow search of specific context (front-end) --- blocks/globalsearch/block_globalsearch.php | 6 +++ lang/en/search.php | 2 + lib/outputrenderers.php | 4 ++ search/classes/output/form/search.php | 16 +++++++ search/index.php | 36 +++++++++++++++- search/tests/behat/search_query.feature | 50 ++++++++++++++++++++-- 6 files changed, 109 insertions(+), 5 deletions(-) diff --git a/blocks/globalsearch/block_globalsearch.php b/blocks/globalsearch/block_globalsearch.php index 7dd3057b63c..d3d96b20662 100644 --- a/blocks/globalsearch/block_globalsearch.php +++ b/blocks/globalsearch/block_globalsearch.php @@ -78,6 +78,12 @@ class block_globalsearch extends block_base { 'type' => 'text', 'size' => '15'); $this->content->text .= html_writer::empty_tag('input', $inputoptions); + // Context id. + if ($this->page->context && $this->page->context->contextlevel !== CONTEXT_SYSTEM) { + $this->content->text .= html_writer::empty_tag('input', ['type' => 'hidden', + 'name' => 'context', 'value' => $this->page->context->id]); + } + // Search button. $this->content->text .= html_writer::tag('button', get_string('search', 'search'), array('id' => 'searchform_button', 'type' => 'submit', 'title' => 'globalsearch', 'class' => 'btn btn-secondary')); diff --git a/lang/en/search.php b/lang/en/search.php index 096722d5a91..552d9b6064f 100644 --- a/lang/en/search.php +++ b/lang/en/search.php @@ -53,6 +53,7 @@ $string['engineserverstatus'] = 'The search engine is not available. Please cont $string['enteryoursearchquery'] = 'Enter your search query'; $string['errors'] = 'Errors'; $string['errorareanotavailable'] = '{$a} search area is not available.'; +$string['everywhere'] = 'Everywhere you can access'; $string['filesinindexdirectory'] = 'Files in index directory'; $string['filterheader'] = 'Filter'; $string['fromtime'] = 'Modified after'; @@ -89,6 +90,7 @@ $string['searcharea'] = 'Search area'; $string['searching'] = 'Searching in ...'; $string['searchnotpermitted'] = 'You are not allowed to do a search'; $string['searchsetupdescription'] = 'The following steps help you to set up Moodle global search.'; +$string['searchwithin'] = 'Search within'; $string['seconds'] = 'seconds'; $string['solutions'] = 'Solutions'; $string['statistics'] = 'Statistics'; diff --git a/lib/outputrenderers.php b/lib/outputrenderers.php index 83c74a3d749..4fbdedce39f 100644 --- a/lib/outputrenderers.php +++ b/lib/outputrenderers.php @@ -3259,6 +3259,10 @@ EOD; $contents = html_writer::tag('label', get_string('enteryoursearchquery', 'search'), array('for' => 'id_q_' . $id, 'class' => 'accesshide')) . html_writer::tag('input', '', $inputattrs); + if ($this->page->context && $this->page->context->contextlevel !== CONTEXT_SYSTEM) { + $contents .= html_writer::empty_tag('input', ['type' => 'hidden', + 'name' => 'context', 'value' => $this->page->context->id]); + } $searchinput = html_writer::tag('form', $contents, $formattrs); return html_writer::tag('div', $searchicon . $searchinput, array('class' => 'search-input-wrapper nav-link', 'id' => $id)); diff --git a/search/classes/output/form/search.php b/search/classes/output/form/search.php index 8364e98eaba..3652de78bef 100644 --- a/search/classes/output/form/search.php +++ b/search/classes/output/form/search.php @@ -48,6 +48,13 @@ class search extends \moodleform { $mform->setType('q', PARAM_TEXT); $mform->addRule('q', get_string('required'), 'required', null, 'client'); + // Show the 'search within' option if the user came from a particular context. + if (!empty($this->_customdata['searchwithin'])) { + $mform->addElement('select', 'searchwithin', get_string('searchwithin', 'search'), + $this->_customdata['searchwithin']); + $mform->setDefault('searchwithin', ''); + } + $mform->addElement('header', 'filtersection', get_string('filterheader', 'search')); $mform->setExpanded('filtersection', false); @@ -79,12 +86,21 @@ class search extends \moodleform { $mform->addElement('course', 'courseids', get_string('courses', 'core'), $options); $mform->setType('courseids', PARAM_INT); + // Course options should be hidden if we choose to search within a specific location. + if (!empty($this->_customdata['searchwithin'])) { + $mform->hideIf('courseids', 'searchwithin', 'ne', ''); + } + $mform->addElement('date_time_selector', 'timestart', get_string('fromtime', 'search'), array('optional' => true)); $mform->setDefault('timestart', 0); $mform->addElement('date_time_selector', 'timeend', get_string('totime', 'search'), array('optional' => true)); $mform->setDefault('timeend', 0); + // Source context i.e. the page they came from when they clicked search. + $mform->addElement('hidden', 'context'); + $mform->setType('context', PARAM_INT); + $this->add_action_buttons(false, get_string('search', 'search')); } } diff --git a/search/index.php b/search/index.php index 9c67e3dfde4..ece070ca488 100644 --- a/search/index.php +++ b/search/index.php @@ -27,6 +27,7 @@ require_once(__DIR__ . '/../config.php'); $page = optional_param('page', 0, PARAM_INT); $q = optional_param('q', '', PARAM_NOTAGS); $title = optional_param('title', '', PARAM_NOTAGS); +$contextid = optional_param('context', 0, PARAM_INT); // Moving areaids, courseids, timestart, and timeend further down as they might come as an array if they come from the form. $context = context_system::instance(); @@ -55,8 +56,23 @@ if (\core_search\manager::is_global_search_enabled() === false) { $search = \core_search\manager::instance(); -// We first get the submitted data as we want to set it all in the page URL. -$mform = new \core_search\output\form\search(null, array('searchengine' => $search->get_engine()->get_plugin_name())); +// Set up custom data for form. +$customdata = ['searchengine' => $search->get_engine()->get_plugin_name()]; +if ($contextid) { + // When a context is supplied, check if it's within course level. If so, show dropdown. + $context = context::instance_by_id($contextid); + $coursecontext = $context->get_course_context(false); + if ($coursecontext) { + $searchwithin = []; + $searchwithin[''] = get_string('everywhere', 'search'); + $searchwithin['course'] = $coursecontext->get_context_name(); + if ($context->contextlevel !== CONTEXT_COURSE) { + $searchwithin['context'] = $context->get_context_name(); + } + $customdata['searchwithin'] = $searchwithin; + } +} +$mform = new \core_search\output\form\search(null, $customdata); $data = $mform->get_data(); if (!$data && $q) { @@ -77,9 +93,25 @@ if (!$data && $q) { } $data->timestart = optional_param('timestart', 0, PARAM_INT); $data->timeend = optional_param('timeend', 0, PARAM_INT); + + $data->context = $contextid; + $mform->set_data($data); } +// Convert the 'search within' option, if used, to course or context restrictions. +if ($data && !empty($data->searchwithin)) { + switch ($data->searchwithin) { + case 'course': + $data->courseids = [$coursecontext->instanceid]; + break; + case 'context': + $data->courseids = [$coursecontext->instanceid]; + $data->contextids = [$context->id]; + break; + } +} + // Set the page URL. $urlparams = array('page' => $page); if ($data) { diff --git a/search/tests/behat/search_query.feature b/search/tests/behat/search_query.feature index 129bbb7204b..e3d9b3502d3 100644 --- a/search/tests/behat/search_query.feature +++ b/search/tests/behat/search_query.feature @@ -7,10 +7,13 @@ Feature: Use global search interface Background: Given the following config values are set as admin: | enableglobalsearch | 1 | + And the following "courses" exist: + | shortname | fullname | + | F1 | Amphibians | And the following "activities" exist: - | activity | name | intro | course | idnumber | - | page | PageName1 | PageDesc1 | Acceptance test site | PAGE1 | - | forum | ForumName1 | ForumDesc1 | Acceptance test site | FORUM1 | + | activity | name | intro | course | idnumber | + | page | PageName1 | PageDesc1 | F1 | PAGE1 | + | forum | ForumName1 | ForumDesc1 | F1 | FORUM1 | And I log in as "admin" @javascript @@ -47,3 +50,44 @@ Feature: Use global search interface # Check the link works. And I follow "ForumName1" And I should see "ForumName1" in the ".breadcrumb" "css_element" + + @javascript + Scenario: Search starting from site context (no within option) + Given global search expects the query "frogs" and will return: + | type | idnumber | + | activity | PAGE1 | + When I search for "frogs" using the header global search box + And I expand all fieldsets + Then I should not see "Search within" + And I should see "Courses" + + @javascript + Scenario: Search starting from course context (within option lists course) + Given global search expects the query "frogs" and will return: + | type | idnumber | + | activity | PAGE1 | + When I am on "Amphibians" course homepage + And I search for "frogs" using the header global search box + And I expand all fieldsets + Then I should see "Search within" + And I select "Everywhere you can access" from the "Search within" singleselect + And I should see "Courses" + And I select "Course: Amphibians" from the "Search within" singleselect + And I should not see "Courses" + + @javascript + Scenario: Search starting from forum context (within option lists course and forum) + Given global search expects the query "frogs" and will return: + | type | idnumber | + | activity | PAGE1 | + When I am on "Amphibians" course homepage + And I follow "ForumName1" + And I search for "frogs" using the header global search box + And I expand all fieldsets + And I should see "Search within" + And I select "Everywhere you can access" from the "Search within" singleselect + And I should see "Courses" + And I select "Course: Amphibians" from the "Search within" singleselect + And I should not see "Courses" + And I select "Forum: ForumName1" from the "Search within" singleselect + And I should not see "Courses"