The glossary lib function glossary_get_entries_search does not respect the casesensitive setting correctly.

I found this bug when trying to use mod/glossary/showentry_ajax with just a $concept rather than the usual $eid in some custom code. Basically I wanted to get the definition from the concept rather than its id.

The library function glossary_get_entries_search is only used in two places showentry.php and showentry_ajax.php. In both of these cases the function is probably never called as $eid is normally provided.

To reproduce this bug:
Create a course, ensure the glossary filter is on for the course.
Add a course main glossary.
Add a concept e.g. Aardvark (note the concept has an initial capital letter), with definition 'A medium-sized, burrowing, nocturnal mammal native to Africa' or similar, ensure concept is automatically linked (not default) and is not marked as case sensitive (default).
Add a label to the course that has some text including the word aardvark in it.
Check the label now has a link around the glossary concept and that it works (popup).
Edit the glossary term Aardvark, making it case sensitive.
Check the label now does not have a link around the glossary concept.
That proves showentry/showentry_ajax are working correctly when calling glossary_get_entries_search with an $eid.
So to prove the bug try this to call the function with a $concept instead.
Create a new test script (called mod/glossary/a.php):

<?php

 require('../../config.php');

 require('lib.php');

 echo 'Start<br/>';

 $concept = 'Aardvark';

 $courseid = <course id created above>;

 echo $concept.'<br/>';

 $a = glossary_get_entries_search($concept, $courseid);

 print_object($a); 

Now changing the case sensitivity and the case of the $concept shows that the function glossary_get_entries_search does not respect the casesensitive setting correctly.
Alternatively the bug can be proven by adding this phpunit test to the end of mod/glossary/tests/lib_test.php 

    public function test_glossary_get_entries_search() {

        $this->resetAfterTest();

        $this->setAdminUser();

        // Turn on glossary autolinking (usedynalink).

        set_config('glossary_linkentries', 1);

        $glossarygenerator = $this->getDataGenerator()->get_plugin_generator('mod_glossary');

        $course = $this->getDataGenerator()->create_course();

        $glossary = $this->getDataGenerator()->create_module('glossary', array('course' => $course->id));

        // Note this entry is not case sensitive by default (casesensitive = 0).

        $entry = $glossarygenerator->create_content($glossary);

        // Check that a search for the concept return the entry.

        $concept = $entry->concept;

        $search = glossary_get_entries_search($concept, $course->id);

        $this->assertCount(1, $search);

        $foundentry = array_shift($search);

        $this->assertEquals($foundentry->concept, $entry->concept);

        // Now try the same search but with a lowercase term.

        $concept = strtolower($entry->concept);

        $search = glossary_get_entries_search($concept, $course->id);

        $this->assertCount(1, $search);

        $foundentry = array_shift($search);

        $this->assertEquals($foundentry->concept, $entry->concept);

        // Make an entry that is case sensitive (casesensitive = 1).

        set_config('glossary_casesensitive', 1);

        $entry = $glossarygenerator->create_content($glossary);

        $concept = $entry->concept;

        $search = glossary_get_entries_search($concept, $course->id);

        $this->assertCount(1, $search);

        $foundentry = array_shift($search);

        $this->assertEquals($foundentry->concept, $entry->concept);

        // Now try the same search but with a lowercase term.

        $concept = strtolower($entry->concept);

        $search = glossary_get_entries_search($concept, $course->id);

        $this->assertCount(0, $search);

    }

Of course this test fails, however the fix for this is simple:

mod/glossary/lib.php line 1037

                                         ( (e.casesensitive != 1 AND LOWER(concept) = :conceptlower) OR
                                          (e.casesensitive = 1 and concept = :concept)) AND

When applied the unit test passes, as does the visual check above.

However the question is whether this is valuable to fix at all. It is a 'bug' that has existed since 2005, and clearly has not affected anyone. So maybe it might be an idea to remove the function, and the two places it is called in showentry and showentry_ajax?

However fixing the function as above is arguably quicker and easier than removing it (I will add a fix on github shortly), and it then would allow other developers to use the function with a concept if they found a use in the future.

To fix or to remove? Any thoughts/opinions?