Skip to content

Commit

Permalink
MDL-47962 filter_glossary: $GLOSSARY_EXCLUDEENTRY shouldn't stop caching
Browse files Browse the repository at this point in the history 
Also, greatly improved unit tests, to test more cases of how the filter
should work.
  • Loading branch information
@timhunt
timhunt committed Sep 20, 2018
1 parent 43d270e commit c633345
Show file tree
Hide file tree
Showing 2 changed files with 191 additions and 23 deletions.
58 changes: 38 additions & 20 deletions filter/glossary/filter.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -50,8 +50,13 @@ public function setup($page, $context) {
}
}

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);
Expand All @@ -67,11 +72,8 @@ public function filter($text, array $options = array()) {
$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);
Expand All @@ -80,20 +82,15 @@ public function filter($text, array $options = array()) {
$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;
Expand All @@ -102,6 +99,7 @@ public function filter($text, array $options = array()) {
'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;
Expand All @@ -114,6 +112,7 @@ public function filter($text, array $options = array()) {
'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.
Expand All @@ -122,23 +121,42 @@ public function filter($text, array $options = array()) {
}
$href_tag_begin = html_writer::start_tag('a', $attributes);

$conceptlist[] = new filterobject($concept->concept, $href_tag_begin, '</a>',
$concept->casesensitive, $concept->fullmatch);
$filterobj = new filterobject($concept->concept, $href_tag_begin, '</a>',
$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!
}

Expand Down
156 changes: 153 additions & 3 deletions filter/glossary/tests/filter_test.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,82 @@
*/
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 = '<p>First we have entry name, then we have it twp aliases first alias and second alias.</p>';
$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 = '<p>This is My category you know.</p>';
$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.
*/
Expand All @@ -59,9 +135,6 @@ public function test_ampersands() {
$amp1 = $generator->create_content($glossary, array('concept' => 'A&B'));
$amp2 = $generator->create_content($glossary, array('concept' => 'C&amp;D'));

filter_manager::reset_caches();
\mod_glossary\local\concept_cache::reset_caches();

// Format text with all three entries in HTML.
$html = '<p>A&amp;B C&amp;D normal</p>';
$filtered = format_text($html, FORMAT_HTML, array('context' => $context));
Expand All @@ -81,4 +154,81 @@ public function test_ampersands() {
$this->assertEquals($glossary->name . ': C&amp;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 = '<p>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.</p>';
$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 = '<p>This is My category you know.</p>';
$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]);
}
}

0 comments on commit c633345

Please sign in to comment.