From 8b73e0f6fee967e893d7b6d30e953de8c9284a11 Mon Sep 17 00:00:00 2001 From: Tim Hunt Date: Tue, 20 Jan 2015 18:14:29 +0000 Subject: [PATCH] MDL-63403 filter_glossary: $GLOSSARY_EXCLUDEENTRY shouldn't stop caching Also, greatly improved unit tests, to test more cases of how the filter should work. --- filter/glossary/filter.php | 58 ++++++---- filter/glossary/tests/filter_test.php | 156 +++++++++++++++++++++++++- 2 files changed, 191 insertions(+), 23 deletions(-) diff --git a/filter/glossary/filter.php b/filter/glossary/filter.php index 5c4ba448122..46c5f917bed 100644 --- a/filter/glossary/filter.php +++ b/filter/glossary/filter.php @@ -50,8 +50,13 @@ class filter_glossary extends moodle_text_filter { } } - public function filter($text, array $options = array()) { - global $CFG, $USER, $GLOSSARY_EXCLUDEENTRY; + /** + * Get all the concepts for this context. + * @return filterobject[] the concepts, and filterobjects, with an added + * ->conceptid field. + */ + protected function get_all_concepts() { + global $CFG, $USER; // Try to get current course. $coursectx = $this->context->get_course_context(false); @@ -67,11 +72,8 @@ class filter_glossary extends moodle_text_filter { $this->cacheconceptlist = null; } - if (is_array($this->cacheconceptlist) and empty($GLOSSARY_EXCLUDEENTRY)) { - if (empty($this->cacheconceptlist)) { - return $text; - } - return filter_phrases($text, $this->cacheconceptlist); + if (is_array($this->cacheconceptlist)) { + return $this->cacheconceptlist; } list($glossaries, $allconcepts) = \mod_glossary\local\concept_cache::get_concepts($courseid); @@ -80,20 +82,15 @@ class filter_glossary extends moodle_text_filter { $this->cacheuserid = $USER->id; $this->cachecourseid = $courseid; $this->cacheconcepts = array(); - return $text; + return $this->cacheconceptlist; } $strcategory = get_string('category', 'glossary'); $conceptlist = array(); - $excluded = false; foreach ($allconcepts as $concepts) { foreach ($concepts as $concept) { - if (!empty($GLOSSARY_EXCLUDEENTRY) and $concept->id == $GLOSSARY_EXCLUDEENTRY) { - $excluded = true; - continue; - } if ($concept->category) { // Link to a category. // TODO: Fix this string usage. $title = $glossaries[$concept->glossaryid] . ': ' . $strcategory . ' ' . $concept->concept; @@ -102,6 +99,7 @@ class filter_glossary extends moodle_text_filter { 'href' => $link, 'title' => $title, 'class' => 'glossary autolink category glossaryid' . $concept->glossaryid); + $conceptid = 0; } else { // Link to entry or alias $title = $glossaries[$concept->glossaryid] . ': ' . $concept->concept; @@ -114,6 +112,7 @@ class filter_glossary extends moodle_text_filter { 'href' => $link, 'title' => str_replace('&', '&', $title), // Undo the s() mangling. 'class' => 'glossary autolink concept glossaryid' . $concept->glossaryid); + $conceptid = $concept->id; } // This flag is optionally set by resource_pluginfile() // if processing an embedded file use target to prevent getting nested Moodles. @@ -122,23 +121,42 @@ class filter_glossary extends moodle_text_filter { } $href_tag_begin = html_writer::start_tag('a', $attributes); - $conceptlist[] = new filterobject($concept->concept, $href_tag_begin, '', - $concept->casesensitive, $concept->fullmatch); + $filterobj = new filterobject($concept->concept, $href_tag_begin, '', + $concept->casesensitive, $concept->fullmatch);; + $filterobj->conceptid = $conceptid; + $conceptlist[] = $filterobj; } } usort($conceptlist, 'filter_glossary::sort_entries_by_length'); - if (!$excluded) { - // Do not cache the excluded list here, it is used once per page only. - $this->cacheuserid = $USER->id; - $this->cachecourseid = $courseid; - $this->cacheconceptlist = $conceptlist; + $this->cacheuserid = $USER->id; + $this->cachecourseid = $courseid; + $this->cacheconceptlist = $conceptlist; + return $this->cacheconceptlist; + } + + public function filter($text, array $options = array()) { + global $GLOSSARY_EXCLUDEENTRY; + + $conceptlist = $this->get_all_concepts(); + + if (empty($conceptlist)) { + return $text; + } + + if (!empty($GLOSSARY_EXCLUDEENTRY)) { + foreach ($conceptlist as $key => $filterobj) { + if ($filterobj->conceptid == $GLOSSARY_EXCLUDEENTRY) { + unset($conceptlist[$key]); + } + } } if (empty($conceptlist)) { return $text; } + return filter_phrases($text, $conceptlist); // Actually search for concepts! } diff --git a/filter/glossary/tests/filter_test.php b/filter/glossary/tests/filter_test.php index ccf54eda2ad..99e1b0935e7 100644 --- a/filter/glossary/tests/filter_test.php +++ b/filter/glossary/tests/filter_test.php @@ -33,6 +33,82 @@ require_once($CFG->dirroot . '/filter/glossary/filter.php'); // Include the code */ class filter_glossary_filter_testcase extends advanced_testcase { + public function test_link_to_entry_with_alias() { + global $CFG; + $this->resetAfterTest(true); + + // Enable glossary filter at top level. + filter_set_global_state('glossary', TEXTFILTER_ON); + $CFG->glossary_linkentries = 1; + + // Create a test course. + $course = $this->getDataGenerator()->create_course(); + $context = context_course::instance($course->id); + + // Create a glossary. + $glossary = $this->getDataGenerator()->create_module('glossary', + array('course' => $course->id, 'mainglossary' => 1)); + + // Create two entries with ampersands and one normal entry. + $generator = $this->getDataGenerator()->get_plugin_generator('mod_glossary'); + $normal = $generator->create_content($glossary, array('concept' => 'entry name'), + array('first alias', 'second alias')); + + // Format text with all three entries in HTML. + $html = '

First we have entry name, then we have it twp aliases first alias and second alias.

'; + $filtered = format_text($html, FORMAT_HTML, array('context' => $context)); + + // Find all the glossary links in the result. + $matches = array(); + preg_match_all('~eid=([0-9]+).*?title="(.*?)"~', $filtered, $matches); + + // There should be 3 glossary links. + $this->assertEquals(3, count($matches[1])); + $this->assertEquals($normal->id, $matches[1][0]); + $this->assertEquals($normal->id, $matches[1][1]); + $this->assertEquals($normal->id, $matches[1][2]); + + // Check text of title attribute. + $this->assertEquals($glossary->name . ': entry name', $matches[2][0]); + $this->assertEquals($glossary->name . ': first alias', $matches[2][1]); + $this->assertEquals($glossary->name . ': second alias', $matches[2][2]); + } + + public function test_link_to_category() { + global $CFG; + $this->resetAfterTest(true); + + // Enable glossary filter at top level. + filter_set_global_state('glossary', TEXTFILTER_ON); + $CFG->glossary_linkentries = 1; + + // Create a test course. + $course = $this->getDataGenerator()->create_course(); + $context = context_course::instance($course->id); + + // Create a glossary. + $glossary = $this->getDataGenerator()->create_module('glossary', + array('course' => $course->id, 'mainglossary' => 1)); + + // Create two entries with ampersands and one normal entry. + /** @var mod_glossary_generator $generator */ + $generator = $this->getDataGenerator()->get_plugin_generator('mod_glossary'); + $category = $generator->create_category($glossary, array('name' => 'My category', 'usedynalink' => 1)); + + // Format text with all three entries in HTML. + $html = '

This is My category you know.

'; + $filtered = format_text($html, FORMAT_HTML, array('context' => $context)); + + // Find all the glossary links in the result. + $matches = array(); + preg_match_all('~hook=([0-9]+).*?title="(.*?)"~', $filtered, $matches); + + // There should be 1 glossary link. + $this->assertEquals(1, count($matches[1])); + $this->assertEquals($category->id, $matches[1][0]); + $this->assertEquals($glossary->name . ': Category My category', $matches[2][0]); + } + /** * Test ampersands. */ @@ -59,9 +135,6 @@ class filter_glossary_filter_testcase extends advanced_testcase { $amp1 = $generator->create_content($glossary, array('concept' => 'A&B')); $amp2 = $generator->create_content($glossary, array('concept' => 'C&D')); - filter_manager::reset_caches(); - \mod_glossary\local\concept_cache::reset_caches(); - // Format text with all three entries in HTML. $html = '

A&B C&D normal

'; $filtered = format_text($html, FORMAT_HTML, array('context' => $context)); @@ -81,4 +154,81 @@ class filter_glossary_filter_testcase extends advanced_testcase { $this->assertEquals($glossary->name . ': C&D', $matches[2][1]); $this->assertEquals($glossary->name . ': normal', $matches[2][2]); } + + public function test_exclude_excludes_link_to_entry_with_alias() { + global $CFG, $GLOSSARY_EXCLUDEENTRY; + + $this->resetAfterTest(true); + + // Enable glossary filter at top level. + filter_set_global_state('glossary', TEXTFILTER_ON); + $CFG->glossary_linkentries = 1; + + // Create a test course. + $course = $this->getDataGenerator()->create_course(); + $context = context_course::instance($course->id); + + // Create a glossary. + $glossary = $this->getDataGenerator()->create_module('glossary', + array('course' => $course->id, 'mainglossary' => 1)); + + // Create two entries with ampersands and one normal entry. + $generator = $this->getDataGenerator()->get_plugin_generator('mod_glossary'); + $tobeexcluded = $generator->create_content($glossary, array('concept' => 'entry name'), + array('first alias', 'second alias')); + $normal = $generator->create_content($glossary, array('concept' => 'other entry')); + + // Format text with all three entries in HTML. + $html = '

First we have entry name, then we have it twp aliases first alias and second alias. ' . + 'In this case, those should not be linked, but this other entry should be.

'; + $GLOSSARY_EXCLUDEENTRY = $tobeexcluded->id; + $filtered = format_text($html, FORMAT_HTML, array('context' => $context)); + $GLOSSARY_EXCLUDEENTRY = null; + + // Find all the glossary links in the result. + $matches = array(); + preg_match_all('~eid=([0-9]+).*?title="(.*?)"~', $filtered, $matches); + + // There should be 1 glossary links. + $this->assertEquals(1, count($matches[1])); + $this->assertEquals($normal->id, $matches[1][0]); + $this->assertEquals($glossary->name . ': other entry', $matches[2][0]); + } + + public function test_exclude_does_not_exclude_categories() { + global $CFG, $GLOSSARY_EXCLUDEENTRY; + $this->resetAfterTest(true); + + // Enable glossary filter at top level. + filter_set_global_state('glossary', TEXTFILTER_ON); + $CFG->glossary_linkentries = 1; + + // Create a test course. + $course = $this->getDataGenerator()->create_course(); + $context = context_course::instance($course->id); + + // Create a glossary. + $glossary = $this->getDataGenerator()->create_module('glossary', + array('course' => $course->id, 'mainglossary' => 1)); + + // Create two entries with ampersands and one normal entry. + /** @var mod_glossary_generator $generator */ + $generator = $this->getDataGenerator()->get_plugin_generator('mod_glossary'); + $category = $generator->create_category($glossary, array('name' => 'My category', 'usedynalink' => 1)); + + // Format text with all three entries in HTML. + $html = '

This is My category you know.

'; + $GLOSSARY_EXCLUDEENTRY = $category->id; + $filtered = format_text($html, FORMAT_HTML, array('context' => $context)); + $GLOSSARY_EXCLUDEENTRY = null; + + // Find all the glossary links in the result. + $matches = array(); + preg_match_all('~hook=([0-9]+).*?title="(.*?)"~', $filtered, $matches); + + // There should be 1 glossary link. + $this->assertEquals(1, count($matches[1])); + $this->assertEquals($category->id, $matches[1][0]); + $this->assertEquals($glossary->name . ': Category My category', $matches[2][0]); + } }