From 66e37026806c21c81a585b2c6b68f5094506c028 Mon Sep 17 00:00:00 2001 From: sam marshall Date: Mon, 11 Sep 2017 11:29:26 +0100 Subject: [PATCH] MDL-55356 core_search: Change existing search areas to new API This change considers all existing search areas in Moodle and makes necessary changes. Custom change to course search, supported by helper in base.php: * course/classes/search/mycourse.php Custom change to message search: * message/classes/search/message_received.php * message/classes/search/message_sent.php Custom change to user search: * user/classes/search/user.php Custom changes to module areas, supported by helper in base_mod.php: * mod/book/classes/search/chapter.php * mod/data/classes/search/entry.php * mod/forum/classes/search/post.php * mod/glossary/classes/search/entry.php * mod/survey/classes/search/activity.php * mod/wiki/classes/search/collaborative_page.php (Note: the unit tests do not exhaustively check every context type for these, given that's mainly handled by the helper function which was already tested in the base_activity test.) Handled by block base class (no change): * blocks/html/classes/search/content.php Handled by activity base class (no change): * mod/assign/classes/search/activity.php * mod/book/classes/search/activity.php * mod/chat/classes/search/activity.php * mod/choice/classes/search/activity.php * mod/data/classes/search/activity.php * mod/feedback/classes/search/activity.php * mod/folder/classes/search/activity.php * mod/forum/classes/search/activity.php * mod/glossary/classes/search/activity.php * mod/imscp/classes/search/activity.php * mod/label/classes/search/activity.php * mod/lesson/classes/search/activity.php * mod/lti/classes/search/activity.php * mod/page/classes/search/activity.php * mod/quiz/classes/search/activity.php * mod/resource/classes/search/activity.php * mod/scorm/classes/search/activity.php * mod/url/classes/search/activity.php * mod/wiki/classes/search/activity.php * mod/workshop/classes/search/activity.php --- course/classes/search/mycourse.php | 19 ++++- course/tests/search_test.php | 79 ++++++++++++++++++ message/classes/search/base_message.php | 49 +++++++++++ message/classes/search/message_received.php | 17 ++-- message/classes/search/message_sent.php | 17 ++-- message/tests/search_received_test.php | 65 +++++++++++++++ message/tests/search_sent_test.php | 81 ++++++++++++++++++- mod/book/classes/search/chapter.php | 18 +++-- mod/book/tests/search_test.php | 20 +++++ mod/data/classes/search/entry.php | 15 +++- mod/data/tests/search_test.php | 16 ++++ mod/forum/classes/search/post.php | 18 +++-- mod/forum/tests/search_test.php | 20 +++++ mod/glossary/classes/search/entry.php | 16 +++- mod/glossary/tests/search_test.php | 15 ++++ mod/survey/classes/search/activity.php | 20 +++-- mod/survey/tests/search_test.php | 75 +++++++++++++++++ .../classes/search/collaborative_page.php | 19 +++-- mod/wiki/tests/search_test.php | 15 ++++ user/classes/search/user.php | 38 ++++++++- user/tests/search_test.php | 10 +++ 21 files changed, 584 insertions(+), 58 deletions(-) create mode 100644 mod/survey/tests/search_test.php diff --git a/course/classes/search/mycourse.php b/course/classes/search/mycourse.php index 8972162f603..9c3378c5e2c 100644 --- a/course/classes/search/mycourse.php +++ b/course/classes/search/mycourse.php @@ -45,11 +45,24 @@ class mycourse extends \core_search\base { * Returns recordset containing required data for indexing courses. * * @param int $modifiedfrom timestamp - * @return \moodle_recordset + * @param \context|null $context Restriction context + * @return \moodle_recordset|null Recordset or null if no change possible */ - public function get_recordset_by_timestamp($modifiedfrom = 0) { + public function get_document_recordset($modifiedfrom = 0, \context $context = null) { global $DB; - return $DB->get_recordset_select('course', 'timemodified >= ?', array($modifiedfrom), 'timemodified ASC'); + + list ($contextjoin, $contextparams) = $this->get_course_level_context_restriction_sql( + $context, 'c'); + if ($contextjoin === null) { + return null; + } + + return $DB->get_recordset_sql(" + SELECT c.* + FROM {course} c + $contextjoin + WHERE c.timemodified >= ? + ORDER BY c.timemodified ASC", array_merge($contextparams, [$modifiedfrom])); } /** diff --git a/course/tests/search_test.php b/course/tests/search_test.php index e8ed2fa4c32..b6092802e00 100644 --- a/course/tests/search_test.php +++ b/course/tests/search_test.php @@ -98,6 +98,85 @@ class course_search_testcase extends advanced_testcase { $recordset->close(); } + /** + * Tests course indexing support for contexts. + */ + public function test_mycourses_indexing_contexts() { + global $DB, $USER, $SITE; + + $searcharea = \core_search\manager::get_search_area($this->mycoursesareaid); + + // Create some courses in categories, and a forum. + $generator = $this->getDataGenerator(); + $cat1 = $generator->create_category(); + $course1 = $generator->create_course(['category' => $cat1->id]); + $cat2 = $generator->create_category(['parent' => $cat1->id]); + $course2 = $generator->create_course(['category' => $cat2->id]); + $cat3 = $generator->create_category(); + $course3 = $generator->create_course(['category' => $cat3->id]); + $forum = $generator->create_module('forum', ['course' => $course1->id]); + $DB->set_field('course', 'timemodified', 0, ['id' => $SITE->id]); + $DB->set_field('course', 'timemodified', 1, ['id' => $course1->id]); + $DB->set_field('course', 'timemodified', 2, ['id' => $course2->id]); + $DB->set_field('course', 'timemodified', 3, ['id' => $course3->id]); + + // Find the first block to use for a block context. + $blockid = array_values($DB->get_records('block_instances', null, 'id', 'id', 0, 1))[0]->id; + $blockcontext = context_block::instance($blockid); + + // Check with block context - should be null. + $this->assertNull($searcharea->get_document_recordset(0, $blockcontext)); + + // Check with user context - should be null. + $this->setAdminUser(); + $usercontext = context_user::instance($USER->id); + $this->assertNull($searcharea->get_document_recordset(0, $usercontext)); + + // Check with module context - should be null. + $modcontext = context_module::instance($forum->cmid); + $this->assertNull($searcharea->get_document_recordset(0, $modcontext)); + + // Check with course context - should return specified course if timestamp allows. + $coursecontext = context_course::instance($course3->id); + $results = self::recordset_to_ids($searcharea->get_document_recordset(3, $coursecontext)); + $this->assertEquals([$course3->id], $results); + $results = self::recordset_to_ids($searcharea->get_document_recordset(4, $coursecontext)); + $this->assertEquals([], $results); + + // Check with category context - should return course in categories and subcategories. + $catcontext = context_coursecat::instance($cat1->id); + $results = self::recordset_to_ids($searcharea->get_document_recordset(0, $catcontext)); + $this->assertEquals([$course1->id, $course2->id], $results); + $results = self::recordset_to_ids($searcharea->get_document_recordset(2, $catcontext)); + $this->assertEquals([$course2->id], $results); + + // Check with system context and null - should return all these courses + site course. + $systemcontext = context_system::instance(); + $results = self::recordset_to_ids($searcharea->get_document_recordset(0, $systemcontext)); + $this->assertEquals([$SITE->id, $course1->id, $course2->id, $course3->id], $results); + $results = self::recordset_to_ids($searcharea->get_document_recordset(0, null)); + $this->assertEquals([$SITE->id, $course1->id, $course2->id, $course3->id], $results); + $results = self::recordset_to_ids($searcharea->get_document_recordset(3, $systemcontext)); + $this->assertEquals([$course3->id], $results); + $results = self::recordset_to_ids($searcharea->get_document_recordset(3, null)); + $this->assertEquals([$course3->id], $results); + } + + /** + * Utility function to convert recordset to array of IDs for testing. + * + * @param moodle_recordset $rs Recordset to convert (and close) + * @return array Array of IDs from records indexed by number (0, 1, 2, ...) + */ + protected static function recordset_to_ids(moodle_recordset $rs) { + $results = []; + foreach ($rs as $rec) { + $results[] = $rec->id; + } + $rs->close(); + return $results; + } + /** * Document contents. * diff --git a/message/classes/search/base_message.php b/message/classes/search/base_message.php index f6ff548bd61..35fce7fc34f 100644 --- a/message/classes/search/base_message.php +++ b/message/classes/search/base_message.php @@ -132,4 +132,53 @@ abstract class base_message extends \core_search\base { return $users; } + /** + * Helper function to implement get_document_recordset for subclasses. + * + * @param int $modifiedfrom Modified from date + * @param \context|null $context Context or null + * @param string $userfield Name of user field (from or to) being considered + * @return \moodle_recordset|null Recordset or null if no results possible + * @throws \coding_exception If context invalid + */ + protected function get_document_recordset_helper($modifiedfrom, \context $context = null, + $userfield) { + global $DB; + + // Set up basic query. + $where = $userfield . ' != :noreplyuser AND ' . $userfield . + ' != :supportuser AND timecreated >= :modifiedfrom'; + $params = [ + 'noreplyuser' => \core_user::NOREPLY_USER, + 'supportuser' => \core_user::SUPPORT_USER, + 'modifiedfrom' => $modifiedfrom + ]; + + // Check context to see whether to add other restrictions. + if ($context === null) { + $context = \context_system::instance(); + } + switch ($context->contextlevel) { + case CONTEXT_COURSECAT: + case CONTEXT_COURSE: + case CONTEXT_MODULE: + case CONTEXT_BLOCK: + // There are no messages in any of these contexts so nothing can be found. + return null; + + case CONTEXT_USER: + // Add extra restriction to specific user context. + $where .= ' AND ' . $userfield . ' = :userid'; + $params['userid'] = $context->instanceid; + break; + + case CONTEXT_SYSTEM: + break; + + default: + throw new \coding_exception('Unexpected contextlevel: ' . $context->contextlevel); + } + + return $DB->get_recordset_select('message_read', $where, $params, 'timeread ASC'); + } } diff --git a/message/classes/search/message_received.php b/message/classes/search/message_received.php index 009451f468d..26a2561c137 100644 --- a/message/classes/search/message_received.php +++ b/message/classes/search/message_received.php @@ -36,19 +36,14 @@ defined('MOODLE_INTERNAL') || die(); class message_received extends base_message { /** - * Returns recordset containing message records. + * Returns a recordset with the messages for indexing. * - * @param int $modifiedfrom timestamp - * @return \moodle_recordset + * @param int $modifiedfrom + * @param \context|null $context Optional context to restrict scope of returned results + * @return moodle_recordset|null Recordset (or null if no results) */ - public function get_recordset_by_timestamp($modifiedfrom = 0) { - global $DB; - - // We don't want to index messages received from noreply and support users. - $params = array('modifiedfrom' => $modifiedfrom, 'noreplyuser' => \core_user::NOREPLY_USER, - 'supportuser' => \core_user::SUPPORT_USER); - return $DB->get_recordset_select('message_read', 'timeread >= :modifiedfrom AND - useridto != :noreplyuser AND useridto != :supportuser', $params, 'timeread ASC'); + public function get_document_recordset($modifiedfrom = 0, \context $context = null) { + return $this->get_document_recordset_helper($modifiedfrom, $context, 'useridto'); } /** diff --git a/message/classes/search/message_sent.php b/message/classes/search/message_sent.php index 07373d85974..9425a82b19a 100644 --- a/message/classes/search/message_sent.php +++ b/message/classes/search/message_sent.php @@ -35,19 +35,14 @@ defined('MOODLE_INTERNAL') || die(); class message_sent extends base_message { /** - * Returns recordset containing message records. + * Returns a recordset with the messages for indexing. * - * @param int $modifiedfrom timestamp - * @return \moodle_recordset + * @param int $modifiedfrom + * @param \context|null $context Optional context to restrict scope of returned results + * @return moodle_recordset|null Recordset (or null if no results) */ - public function get_recordset_by_timestamp($modifiedfrom = 0) { - global $DB; - - // We don't want to index messages sent by noreply and support users. - $params = array('modifiedfrom' => $modifiedfrom, 'noreplyuser' => \core_user::NOREPLY_USER, - 'supportuser' => \core_user::SUPPORT_USER); - return $DB->get_recordset_select('message_read', 'timeread >= :modifiedfrom AND - useridfrom != :noreplyuser AND useridfrom != :supportuser', $params, 'timeread ASC'); + public function get_document_recordset($modifiedfrom = 0, \context $context = null) { + return $this->get_document_recordset_helper($modifiedfrom, $context, 'useridfrom'); } /** diff --git a/message/tests/search_received_test.php b/message/tests/search_received_test.php index 7bcde6948e1..9eeb93e2e13 100644 --- a/message/tests/search_received_test.php +++ b/message/tests/search_received_test.php @@ -113,6 +113,71 @@ class message_received_search_testcase extends advanced_testcase { $recordset->close(); } + /** + * Indexing messages, with restricted contexts. + */ + public function test_message_received_indexing_contexts() { + global $SITE; + require_once(__DIR__ . '/search_sent_test.php'); + + $searcharea = \core_search\manager::get_search_area($this->messagereceivedareaid); + + $user1 = self::getDataGenerator()->create_user(); + $user2 = self::getDataGenerator()->create_user(); + + $this->preventResetByRollback(); + $sink = $this->redirectMessages(); + + // Send first message. + $message = new \core\message\message(); + $message->courseid = SITEID; + $message->userfrom = $user1; + $message->userto = $user2; + $message->subject = 'Test1'; + $message->smallmessage = 'Test small messsage'; + $message->fullmessage = 'Test full messsage'; + $message->fullmessageformat = 0; + $message->fullmessagehtml = null; + $message->notification = 0; + $message->component = 'moodle'; + $message->name = 'instantmessage'; + message_send($message); + + // Ensure that ordering by timestamp will return in consistent order. + $this->waitForSecond(); + + // Send second message in opposite direction. + $message = new \core\message\message(); + $message->courseid = SITEID; + $message->userfrom = $user2; + $message->userto = $user1; + $message->subject = 'Test2'; + $message->smallmessage = 'Test small messsage'; + $message->fullmessage = 'Test full messsage'; + $message->fullmessageformat = 0; + $message->fullmessagehtml = null; + $message->notification = 0; + $message->component = 'moodle'; + $message->name = 'instantmessage'; + message_send($message); + + // Test function with null context and system context (same). + $rs = $searcharea->get_document_recordset(0, null); + $this->assertEquals(['Test1', 'Test2'], message_sent_search_testcase::recordset_to_subjects($rs)); + $rs = $searcharea->get_document_recordset(0, context_system::instance()); + $this->assertEquals(['Test1', 'Test2'], message_sent_search_testcase::recordset_to_subjects($rs)); + + // Test with user context for each user. + $rs = $searcharea->get_document_recordset(0, \context_user::instance($user1->id)); + $this->assertEquals(['Test2'], message_sent_search_testcase::recordset_to_subjects($rs)); + $rs = $searcharea->get_document_recordset(0, \context_user::instance($user2->id)); + $this->assertEquals(['Test1'], message_sent_search_testcase::recordset_to_subjects($rs)); + + // Test with a course context (should return null). + $this->assertNull($searcharea->get_document_recordset(0, + context_course::instance($SITE->id))); + } + /** * Document contents. * diff --git a/message/tests/search_sent_test.php b/message/tests/search_sent_test.php index 75a3735d438..9fe45c51aad 100644 --- a/message/tests/search_sent_test.php +++ b/message/tests/search_sent_test.php @@ -113,6 +113,85 @@ class message_sent_search_testcase extends advanced_testcase { $recordset->close(); } + /** + * Indexing messages, with restricted contexts. + */ + public function test_message_sent_indexing_contexts() { + global $SITE; + + $searcharea = \core_search\manager::get_search_area($this->messagesentareaid); + + $user1 = self::getDataGenerator()->create_user(); + $user2 = self::getDataGenerator()->create_user(); + + $this->preventResetByRollback(); + $sink = $this->redirectMessages(); + + // Send first message. + $message = new \core\message\message(); + $message->courseid = SITEID; + $message->userfrom = $user1; + $message->userto = $user2; + $message->subject = 'Test1'; + $message->smallmessage = 'Test small messsage'; + $message->fullmessage = 'Test full messsage'; + $message->fullmessageformat = 0; + $message->fullmessagehtml = null; + $message->notification = 0; + $message->component = 'moodle'; + $message->name = 'instantmessage'; + message_send($message); + + // Ensure that ordering by timestamp will return in consistent order. + $this->waitForSecond(); + + // Send second message in opposite direction. + $message = new \core\message\message(); + $message->courseid = SITEID; + $message->userfrom = $user2; + $message->userto = $user1; + $message->subject = 'Test2'; + $message->smallmessage = 'Test small messsage'; + $message->fullmessage = 'Test full messsage'; + $message->fullmessageformat = 0; + $message->fullmessagehtml = null; + $message->notification = 0; + $message->component = 'moodle'; + $message->name = 'instantmessage'; + message_send($message); + + // Test function with null context and system context (same). + $rs = $searcharea->get_document_recordset(0, null); + $this->assertEquals(['Test1', 'Test2'], self::recordset_to_subjects($rs)); + $rs = $searcharea->get_document_recordset(0, context_system::instance()); + $this->assertEquals(['Test1', 'Test2'], self::recordset_to_subjects($rs)); + + // Test with user context for each user. + $rs = $searcharea->get_document_recordset(0, \context_user::instance($user1->id)); + $this->assertEquals(['Test1'], self::recordset_to_subjects($rs)); + $rs = $searcharea->get_document_recordset(0, \context_user::instance($user2->id)); + $this->assertEquals(['Test2'], self::recordset_to_subjects($rs)); + + // Test with a course context (should return null). + $this->assertNull($searcharea->get_document_recordset(0, + context_course::instance($SITE->id))); + } + + /** + * Utility function to convert recordset to array of message subjects for testing. + * + * @param moodle_recordset $rs Recordset to convert (and close) + * @return array Array of IDs from records indexed by number (0, 1, 2, ...) + */ + public static function recordset_to_subjects(moodle_recordset $rs) { + $results = []; + foreach ($rs as $rec) { + $results[] = $rec->subject; + } + $rs->close(); + return $results; + } + /** * Document contents. * @@ -272,4 +351,4 @@ class message_sent_search_testcase extends advanced_testcase { $this->assertFalse($doc); } -} \ No newline at end of file +} diff --git a/mod/book/classes/search/chapter.php b/mod/book/classes/search/chapter.php index 3acd1866539..0f17e63c080 100644 --- a/mod/book/classes/search/chapter.php +++ b/mod/book/classes/search/chapter.php @@ -43,16 +43,24 @@ class chapter extends \core_search\base_mod { * Returns a recordset with all required chapter information. * * @param int $modifiedfrom - * @return moodle_recordset + * @param \context|null $context Optional context to restrict scope of returned results + * @return moodle_recordset|null Recordset (or null if no results) */ - public function get_recordset_by_timestamp($modifiedfrom = 0) { + public function get_document_recordset($modifiedfrom = 0, \context $context = null) { global $DB; - $sql = 'SELECT c.*, b.id AS bookid, b.course AS courseid + list ($contextjoin, $contextparams) = $this->get_context_restriction_sql( + $context, 'book', 'b'); + if ($contextjoin === null) { + return null; + } + + $sql = "SELECT c.*, b.id AS bookid, b.course AS courseid FROM {book_chapters} c JOIN {book} b ON b.id = c.bookid - WHERE c.timemodified >= ? ORDER BY c.timemodified ASC'; - return $DB->get_recordset_sql($sql, array($modifiedfrom)); + $contextjoin + WHERE c.timemodified >= ? ORDER BY c.timemodified ASC"; + return $DB->get_recordset_sql($sql, array_merge($contextparams, [$modifiedfrom])); } /** diff --git a/mod/book/tests/search_test.php b/mod/book/tests/search_test.php index ba96a639043..65e15896ba3 100644 --- a/mod/book/tests/search_test.php +++ b/mod/book/tests/search_test.php @@ -118,6 +118,26 @@ class mod_book_search_testcase extends advanced_testcase { // No new records. $this->assertFalse($recordset->valid()); $recordset->close(); + + // Create another book and chapter. + $book2 = $this->getDataGenerator()->create_module('book', array('course' => $course1->id)); + $bookgenerator->create_chapter(array('bookid' => $book2->id, + 'content' => 'Chapter3', 'title' => 'Title3')); + + // Query by context, first book. + $recordset = $searcharea->get_document_recordset(0, \context_module::instance($book->cmid)); + $this->assertEquals(2, iterator_count($recordset)); + $recordset->close(); + + // Second book. + $recordset = $searcharea->get_document_recordset(0, \context_module::instance($book2->cmid)); + $this->assertEquals(1, iterator_count($recordset)); + $recordset->close(); + + // Course. + $recordset = $searcharea->get_document_recordset(0, \context_course::instance($course1->id)); + $this->assertEquals(3, iterator_count($recordset)); + $recordset->close(); } /** diff --git a/mod/data/classes/search/entry.php b/mod/data/classes/search/entry.php index 7a874a80424..e2a25807454 100644 --- a/mod/data/classes/search/entry.php +++ b/mod/data/classes/search/entry.php @@ -47,16 +47,25 @@ class entry extends \core_search\base_mod { * Returns recordset containing required data for indexing database entries. * * @param int $modifiedfrom timestamp - * @return moodle_recordset + * @param \context|null $context Optional context to restrict scope of returned results + * @return moodle_recordset|null Recordset (or null if no results) */ - public function get_recordset_by_timestamp($modifiedfrom = 0) { + public function get_document_recordset($modifiedfrom = 0, \context $context = null) { global $DB; + list ($contextjoin, $contextparams) = $this->get_context_restriction_sql( + $context, 'data', 'd', SQL_PARAMS_NAMED); + if ($contextjoin === null) { + return null; + } + $sql = "SELECT dr.*, d.course FROM {data_records} dr JOIN {data} d ON d.id = dr.dataid + $contextjoin WHERE dr.timemodified >= :timemodified"; - return $DB->get_recordset_sql($sql, array('timemodified' => $modifiedfrom)); + return $DB->get_recordset_sql($sql, + array_merge($contextparams, ['timemodified' => $modifiedfrom])); } /** diff --git a/mod/data/tests/search_test.php b/mod/data/tests/search_test.php index 105571f97e7..e4dc78fb796 100644 --- a/mod/data/tests/search_test.php +++ b/mod/data/tests/search_test.php @@ -314,6 +314,22 @@ class mod_data_search_test extends advanced_testcase { // No new records. $this->assertFalse($recordset->valid()); $recordset->close(); + + // Create a second database, also with one record. + $data2 = $this->getDataGenerator()->create_module('data', ['course' => $course1->id]); + $this->create_default_data_fields($fieldtypes, $data2); + $this->create_default_data_record($data2); + + // Test indexing with contexts. + $rs = $searcharea->get_document_recordset(0, context_module::instance($data1->cmid)); + $this->assertEquals(1, iterator_count($rs)); + $rs->close(); + $rs = $searcharea->get_document_recordset(0, context_module::instance($data2->cmid)); + $this->assertEquals(1, iterator_count($rs)); + $rs->close(); + $rs = $searcharea->get_document_recordset(0, context_course::instance($course1->id)); + $this->assertEquals(2, iterator_count($rs)); + $rs->close(); } /** diff --git a/mod/forum/classes/search/post.php b/mod/forum/classes/search/post.php index bb48a035304..698d465019e 100644 --- a/mod/forum/classes/search/post.php +++ b/mod/forum/classes/search/post.php @@ -56,17 +56,25 @@ class post extends \core_search\base_mod { * Returns recordset containing required data for indexing forum posts. * * @param int $modifiedfrom timestamp - * @return moodle_recordset + * @param \context|null $context Optional context to restrict scope of returned results + * @return moodle_recordset|null Recordset (or null if no results) */ - public function get_recordset_by_timestamp($modifiedfrom = 0) { + public function get_document_recordset($modifiedfrom = 0, \context $context = null) { global $DB; - $sql = 'SELECT fp.*, f.id AS forumid, f.course AS courseid + list ($contextjoin, $contextparams) = $this->get_context_restriction_sql( + $context, 'forum', 'f'); + if ($contextjoin === null) { + return null; + } + + $sql = "SELECT fp.*, f.id AS forumid, f.course AS courseid FROM {forum_posts} fp JOIN {forum_discussions} fd ON fd.id = fp.discussion JOIN {forum} f ON f.id = fd.forum - WHERE fp.modified >= ? ORDER BY fp.modified ASC'; - return $DB->get_recordset_sql($sql, array($modifiedfrom)); + $contextjoin + WHERE fp.modified >= ? ORDER BY fp.modified ASC"; + return $DB->get_recordset_sql($sql, array_merge($contextparams, [$modifiedfrom])); } /** diff --git a/mod/forum/tests/search_test.php b/mod/forum/tests/search_test.php index 87ac58e19dc..351d0a3bfcb 100644 --- a/mod/forum/tests/search_test.php +++ b/mod/forum/tests/search_test.php @@ -144,6 +144,26 @@ class mod_forum_search_testcase extends advanced_testcase { // No new records. $this->assertFalse($recordset->valid()); $recordset->close(); + + // Context test: create another forum with 1 post. + $forum2 = self::getDataGenerator()->create_module('forum', ['course' => $course1->id]); + $record = new stdClass(); + $record->course = $course1->id; + $record->userid = $user1->id; + $record->forum = $forum2->id; + $record->message = 'discussion'; + self::getDataGenerator()->get_plugin_generator('mod_forum')->create_discussion($record); + + // Test indexing with each forum then combined course context. + $rs = $searcharea->get_document_recordset(0, context_module::instance($forum1->cmid)); + $this->assertEquals(2, iterator_count($rs)); + $rs->close(); + $rs = $searcharea->get_document_recordset(0, context_module::instance($forum2->cmid)); + $this->assertEquals(1, iterator_count($rs)); + $rs->close(); + $rs = $searcharea->get_document_recordset(0, context_course::instance($course1->id)); + $this->assertEquals(3, iterator_count($rs)); + $rs->close(); } /** diff --git a/mod/glossary/classes/search/entry.php b/mod/glossary/classes/search/entry.php index 791e3414ddc..49d09bbfe0f 100644 --- a/mod/glossary/classes/search/entry.php +++ b/mod/glossary/classes/search/entry.php @@ -46,15 +46,23 @@ class entry extends \core_search\base_mod { * Returns recordset containing required data for indexing glossary entries. * * @param int $modifiedfrom timestamp - * @return moodle_recordset + * @param \context|null $context Optional context to restrict scope of returned results + * @return moodle_recordset|null Recordset (or null if no results) */ - public function get_recordset_by_timestamp($modifiedfrom = 0) { + public function get_document_recordset($modifiedfrom = 0, \context $context = null) { global $DB; + list ($contextjoin, $contextparams) = $this->get_context_restriction_sql( + $context, 'glossary', 'g'); + if ($contextjoin === null) { + return null; + } + $sql = "SELECT ge.*, g.course FROM {glossary_entries} ge JOIN {glossary} g ON g.id = ge.glossaryid - WHERE ge.timemodified >= ? ORDER BY ge.timemodified ASC"; - return $DB->get_recordset_sql($sql, array($modifiedfrom)); + $contextjoin + WHERE ge.timemodified >= ? ORDER BY ge.timemodified ASC"; + return $DB->get_recordset_sql($sql, array_merge($contextparams, [$modifiedfrom])); } /** diff --git a/mod/glossary/tests/search_test.php b/mod/glossary/tests/search_test.php index ff7d183dc51..cf8ddbe04bd 100644 --- a/mod/glossary/tests/search_test.php +++ b/mod/glossary/tests/search_test.php @@ -132,6 +132,21 @@ class mod_glossary_search_testcase extends advanced_testcase { // No new records. $this->assertFalse($recordset->valid()); $recordset->close(); + + // Create a second glossary with one entry. + $glossary2 = self::getDataGenerator()->create_module('glossary', ['course' => $course1->id]); + self::getDataGenerator()->get_plugin_generator('mod_glossary')->create_content($glossary2); + + // Test indexing with each activity then combined course context. + $rs = $searcharea->get_document_recordset(0, context_module::instance($glossary1->cmid)); + $this->assertEquals(2, iterator_count($rs)); + $rs->close(); + $rs = $searcharea->get_document_recordset(0, context_module::instance($glossary2->cmid)); + $this->assertEquals(1, iterator_count($rs)); + $rs->close(); + $rs = $searcharea->get_document_recordset(0, context_course::instance($course1->id)); + $this->assertEquals(3, iterator_count($rs)); + $rs->close(); } /** diff --git a/mod/survey/classes/search/activity.php b/mod/survey/classes/search/activity.php index fd7914175fd..e25520d7bab 100644 --- a/mod/survey/classes/search/activity.php +++ b/mod/survey/classes/search/activity.php @@ -47,16 +47,24 @@ class activity extends \core_search\base_activity { /** * Returns recordset containing required data for indexing activities. * - * Overwritten to discard records with courseid = 0. + * Overridden to discard records with courseid = 0. * * @param int $modifiedfrom timestamp - * @return \moodle_recordset + * @param \context|null $context Context + * @return \moodle_recordset|null Recordset, or null if no possible activities in given context */ - public function get_recordset_by_timestamp($modifiedfrom = 0) { + public function get_document_recordset($modifiedfrom = 0, \context $context = null) { global $DB; - $select = 'course != ? AND ' . static::MODIFIED_FIELD_NAME . ' >= ?'; - return $DB->get_recordset_select($this->get_module_name(), $select, array(0, $modifiedfrom), - static::MODIFIED_FIELD_NAME . ' ASC'); + list ($contextjoin, $contextparams) = $this->get_context_restriction_sql( + $context, $this->get_module_name(), 'modtable'); + if ($contextjoin === null) { + return null; + } + return $DB->get_recordset_sql('SELECT modtable.* FROM {' . $this->get_module_name() . + '} modtable ' . $contextjoin . ' WHERE modtable.' . static::MODIFIED_FIELD_NAME . + ' >= ? AND modtable.course != ? ORDER BY modtable.' . static::MODIFIED_FIELD_NAME . + ' ASC', + array_merge($contextparams, [$modifiedfrom, 0])); } } diff --git a/mod/survey/tests/search_test.php b/mod/survey/tests/search_test.php new file mode 100644 index 00000000000..4ff8e155261 --- /dev/null +++ b/mod/survey/tests/search_test.php @@ -0,0 +1,75 @@ +. + +/** + * Unit test for mod_survey searching. + * + * This is needed because the activity.php class overrides default behaviour. + * + * @package mod_survey + * @category test + * @copyright 2017 The Open University + * @license http://www.gnu.org/copyleft/gpl.html GNU GPL v3 or later + */ + +defined('MOODLE_INTERNAL') || die(); + +/** + * Unit test for mod_survey searching. + * + * This is needed because the activity.php class overrides default behaviour. + * + * @package mod_survey + * @category test + * @copyright 2017 The Open University + * @license http://www.gnu.org/copyleft/gpl.html GNU GPL v3 or later + */ +class mod_survey_search_testcase extends advanced_testcase { + + /** + * Test survey_view + * @return void + */ + public function test_survey_indexing() { + global $CFG; + + $this->resetAfterTest(); + + require_once($CFG->dirroot . '/search/tests/fixtures/testable_core_search.php'); + testable_core_search::instance(); + $area = \core_search\manager::get_search_area('mod_survey-activity'); + + // Setup test data. + $generator = $this->getDataGenerator(); + $course = $generator->create_course(); + $survey1 = $generator->create_module('survey', ['course' => $course->id]); + $survey2 = $generator->create_module('survey', ['course' => $course->id]); + + // Get all surveys for indexing - note that there are special entries in the table with + // course zero which should not be returned. + $rs = $area->get_document_recordset(); + $this->assertEquals(2, iterator_count($rs)); + $rs->close(); + + // Test specific context and course context. + $rs = $area->get_document_recordset(0, context_module::instance($survey1->cmid)); + $this->assertEquals(1, iterator_count($rs)); + $rs->close(); + $rs = $area->get_document_recordset(0, context_course::instance($course->id)); + $this->assertEquals(2, iterator_count($rs)); + $rs->close(); + } +} diff --git a/mod/wiki/classes/search/collaborative_page.php b/mod/wiki/classes/search/collaborative_page.php index be3a16a62c0..11220d2e3e1 100644 --- a/mod/wiki/classes/search/collaborative_page.php +++ b/mod/wiki/classes/search/collaborative_page.php @@ -45,19 +45,28 @@ class collaborative_page extends \core_search\base_mod { * Returns a recordset with all required page information. * * @param int $modifiedfrom - * @return moodle_recordset + * @param \context|null $context Optional context to restrict scope of returned results + * @return moodle_recordset|null Recordset (or null if no results) */ - public function get_recordset_by_timestamp($modifiedfrom = 0) { + public function get_document_recordset($modifiedfrom = 0, \context $context = null) { global $DB; - $sql = 'SELECT p.*, w.id AS wikiid, w.course AS courseid + list ($contextjoin, $contextparams) = $this->get_context_restriction_sql( + $context, 'wiki', 'w'); + if ($contextjoin === null) { + return null; + } + + $sql = "SELECT p.*, w.id AS wikiid, w.course AS courseid FROM {wiki_pages} p JOIN {wiki_subwikis} s ON s.id = p.subwikiid JOIN {wiki} w ON w.id = s.wikiid + $contextjoin WHERE p.timemodified >= ? AND w.wikimode = ? - ORDER BY p.timemodified ASC'; - return $DB->get_recordset_sql($sql, array($modifiedfrom, 'collaborative')); + ORDER BY p.timemodified ASC"; + return $DB->get_recordset_sql($sql, array_merge($contextparams, + [$modifiedfrom, 'collaborative'])); } /** diff --git a/mod/wiki/tests/search_test.php b/mod/wiki/tests/search_test.php index 87cdba29498..3daa8043ee9 100644 --- a/mod/wiki/tests/search_test.php +++ b/mod/wiki/tests/search_test.php @@ -126,6 +126,21 @@ class mod_wiki_search_testcase extends advanced_testcase { // No new records. $this->assertFalse($recordset->valid()); $recordset->close(); + + // Add another wiki with one page. + $collabwiki2 = $this->getDataGenerator()->create_module('wiki', ['course' => $course1->id]); + $wikigenerator->create_first_page($collabwiki2); + + // Test indexing contexts. + $rs = $searcharea->get_document_recordset(0, context_module::instance($collabwiki->cmid)); + $this->assertEquals(3, iterator_count($rs)); + $rs->close(); + $rs = $searcharea->get_document_recordset(0, context_module::instance($collabwiki2->cmid)); + $this->assertEquals(1, iterator_count($rs)); + $rs->close(); + $rs = $searcharea->get_document_recordset(0, context_course::instance($course1->id)); + $this->assertEquals(4, iterator_count($rs)); + $rs->close(); } /** diff --git a/user/classes/search/user.php b/user/classes/search/user.php index b4069b4b60e..5724b69532f 100644 --- a/user/classes/search/user.php +++ b/user/classes/search/user.php @@ -41,12 +41,42 @@ class user extends \core_search\base { * Returns recordset containing required data attributes for indexing. * * @param number $modifiedfrom - * @return \moodle_recordset + * @param \context|null $context Optional context to restrict scope of returned results + * @return \moodle_recordset|null Recordset (or null if no results) */ - public function get_recordset_by_timestamp($modifiedfrom = 0) { + public function get_document_recordset($modifiedfrom = 0, \context $context = null) { global $DB; - return $DB->get_recordset_select('user', 'timemodified >= ? AND deleted = ? AND - confirmed = ?', array($modifiedfrom, 0, 1)); + + // Prepare query conditions. + $where = 'timemodified >= ? AND deleted = ? AND confirmed = ?'; + $params = [$modifiedfrom, 0, 1]; + + // Handle context types. + if (!$context) { + $context = \context_system::instance(); + } + switch ($context->contextlevel) { + case CONTEXT_MODULE: + case CONTEXT_BLOCK: + case CONTEXT_COURSE: + case CONTEXT_COURSECAT: + // These contexts cannot contain any users. + return null; + + case CONTEXT_USER: + // Restrict to specific user. + $where .= ' AND id = ?'; + $params[] = $context->instanceid; + break; + + case CONTEXT_SYSTEM: + break; + + default: + throw new \coding_exception('Unexpected contextlevel: ' . $context->contextlevel); + } + + return $DB->get_recordset_select('user', $where, $params); } /** diff --git a/user/tests/search_test.php b/user/tests/search_test.php index b5175235abf..589665a44f7 100644 --- a/user/tests/search_test.php +++ b/user/tests/search_test.php @@ -57,6 +57,7 @@ class user_search_testcase extends advanced_testcase { * @return void */ public function test_users_indexing() { + global $SITE; // Returns the instance as long as the area is supported. $searcharea = \core_search\manager::get_search_area($this->userareaid); @@ -87,6 +88,15 @@ class user_search_testcase extends advanced_testcase { // No new records. $this->assertFalse($recordset->valid()); $recordset->close(); + + // Context support; first, try an unsupported context type. + $coursecontext = context_course::instance($SITE->id); + $this->assertNull($searcharea->get_document_recordset(0, $coursecontext)); + + // Try a specific user, will only return 1 record (that user). + $rs = $searcharea->get_document_recordset(0, context_user::instance($user1->id)); + $this->assertEquals(1, iterator_count($rs)); + $rs->close(); } /**