Moodle
  1. Moodle
  2. MDL-25609

Frontpage combo list double linking by filters

    Details

    • Type: Bug Bug
    • Status: Closed
    • Priority: Major Major
    • Resolution: Fixed
    • Affects Version/s: 2.0
    • Fix Version/s: 2.0.1
    • Component/s: Glossary
    • Labels:
      None
    • Environment:
      Ubuntu 10.04 Moodle 2.0 Weekly
    • Database:
      MySQL
    • Affected Branches:
      MOODLE_20_STABLE
    • Fixed Branches:
      MOODLE_20_STABLE
    • Rank:
      15648

      Description

      This is probably two tickets in one...

      I migrated our server from a RedHat Enterprise 5 server running Moodle 1.9.5+ to an Ubuntu 10.04 Server, then upgraded to Moodle 2.0+ Weekly. Our glossary was on the front page with 10 or so entried and auto-linking enabled. One of the entries was "IT" and we have 2 courses in our course listing on the front page where the "IT" in the course name was auto-linked to the glossary entry and the rest of the course name was not linked to anything. In order to actually get into the IT Support Course, you had to click on the main category Davis College Department Courses and then you can click on the IT Support course.

      We simply deleted the glossary to get around it.

      I then decided to re-add the glossary activity and tried to enter a single entry in concept I put IT and filled out the rest. When I saved it, I received
      Field "LOWER(concept)" does not exist in table "glossary_entries"

      I described the mdl_glossary_entries and here is what I have...
      mysql> describe mdl_glossary_entries;
      -----------------------------------------------------------------+

      Field Type Null Key Default Extra

      -----------------------------------------------------------------+

      id bigint(10) unsigned NO PRI NULL auto_increment
      glossaryid bigint(10) unsigned NO MUL 0  
      userid bigint(10) unsigned NO MUL 0  
      concept varchar(255) NO MUL    
      definition text NO   NULL  
      definitionformat tinyint(2) unsigned NO   0  
      definitiontrust tinyint(2) unsigned NO   0  
      attachment varchar(100) NO      
      timecreated bigint(10) unsigned NO   0  
      timemodified bigint(10) unsigned NO   0  
      teacherentry tinyint(2) unsigned NO   0  
      sourceglossaryid bigint(10) unsigned NO   0  
      usedynalink tinyint(2) unsigned NO   1  
      casesensitive tinyint(2) unsigned NO   0  
      fullmatch tinyint(2) unsigned NO   1  
      approved tinyint(2) unsigned NO   1  

      -----------------------------------------------------------------+

      Is this an improper table? Should it actually reference table $prefix_glossary_entries?

        Activity

        Hide
        Eloy Lafuente (stronk7) added a comment -

        Hi Aaron,

        about the LOWER() problem, I think it was fixed some days ago by MDL-25418, so it shouldn't be a problem if you get a recent version of Moodle 2.0+

        about the filter filtering course titles... if I'm not wrong you set the glossary filter to be applied only to "contents" and not to "headings". That way course titles won't be filtered anymore (and also other "titles"). If this solution doesn't work, I only can imagine making the "IT" entry not linkable (in fact I guess you'll get a bunch of false-positives with it.

        Please, tell me if the "content only" solution worked... TIA and ciao

        Show
        Eloy Lafuente (stronk7) added a comment - Hi Aaron, about the LOWER() problem, I think it was fixed some days ago by MDL-25418 , so it shouldn't be a problem if you get a recent version of Moodle 2.0+ about the filter filtering course titles... if I'm not wrong you set the glossary filter to be applied only to "contents" and not to "headings". That way course titles won't be filtered anymore (and also other "titles"). If this solution doesn't work, I only can imagine making the "IT" entry not linkable (in fact I guess you'll get a bunch of false-positives with it . Please, tell me if the "content only" solution worked... TIA and ciao
        Hide
        Aaron Cowell added a comment -

        Hi Eloy,

        I can confirm that getting the latest CVS Dev Head resolved the LOWER() issue. Is there a reason the 2.0+ Weekly builds are not auto building from the stable moodle download page on Moodle.org?

        Regarding the filtering, I set to match case exactly so only uppercase "IT" link. I guess I'll just have to disable auto-linking for the "IT" Glossary enter itself. It would be nice if a course name is detected matching a glossary entry that a flag at the end of the course name was set. But this is not a priority or stopper by any means, we don't heavily rely on glossaries right now.

        Thanks!
        Aaron

        Show
        Aaron Cowell added a comment - Hi Eloy, I can confirm that getting the latest CVS Dev Head resolved the LOWER() issue. Is there a reason the 2.0+ Weekly builds are not auto building from the stable moodle download page on Moodle.org? Regarding the filtering, I set to match case exactly so only uppercase "IT" link. I guess I'll just have to disable auto-linking for the "IT" Glossary enter itself. It would be nice if a course name is detected matching a glossary entry that a flag at the end of the course name was set. But this is not a priority or stopper by any means, we don't heavily rely on glossaries right now. Thanks! Aaron
        Hide
        Eloy Lafuente (stronk7) added a comment -

        Hi Aaron,

        main problem when trying to match entries matching case, is that 99% of the collations available in MySQL are case-insensitive collations; basically it means that, for the DB:

        it = IT = iT = It

        unless you have your database configured to use one case-sensitive collation, that, at the time of writing this, is, exclusively, the utf8_bin collation.

        But I don't recommend you to use it, as far as it has other drawbacks, like incorrect sorting for non-english characters (user names) and so on.

        So, until MySQL decides to add more case-sensitive collations, the "matching case" feature of the glossary won't be working ok in that DB at all (note it works ok under PostgreSQL, MSSQL and Oracle).

        Ciao

        Show
        Eloy Lafuente (stronk7) added a comment - Hi Aaron, main problem when trying to match entries matching case, is that 99% of the collations available in MySQL are case-insensitive collations; basically it means that, for the DB: it = IT = iT = It unless you have your database configured to use one case-sensitive collation, that, at the time of writing this, is, exclusively, the utf8_bin collation. But I don't recommend you to use it, as far as it has other drawbacks, like incorrect sorting for non-english characters (user names) and so on. So, until MySQL decides to add more case-sensitive collations, the "matching case" feature of the glossary won't be working ok in that DB at all (note it works ok under PostgreSQL, MSSQL and Oracle). Ciao
        Hide
        Eloy Lafuente (stronk7) added a comment -

        Resolving this as fixed, thanks for feedback! Ciao

        Show
        Eloy Lafuente (stronk7) added a comment - Resolving this as fixed, thanks for feedback! Ciao
        Hide
        Aaron Cowell added a comment -

        Eloy,

        I don't know if I'm misunderstanding what you said or if you misunderstand the issue with the IT glossary entry overriding the course link in the main course listing on the front page. You can visit http://coursesites2.daviscollege.edu/ and scroll down to the bottom to the Courses list, look at "IT Support Coffee Shop" and note that "IT" in "IT Support Coffee Shop" is linked to glossary and you cannot enter the course unless you go to the category first, then to the course. Please let me know if this is the relation your mentioning above on your 3:40AM post.

        Thanks,
        Aaron

        Show
        Aaron Cowell added a comment - Eloy, I don't know if I'm misunderstanding what you said or if you misunderstand the issue with the IT glossary entry overriding the course link in the main course listing on the front page. You can visit http://coursesites2.daviscollege.edu/ and scroll down to the bottom to the Courses list, look at "IT Support Coffee Shop" and note that "IT" in "IT Support Coffee Shop" is linked to glossary and you cannot enter the course unless you go to the category first, then to the course. Please let me know if this is the relation your mentioning above on your 3:40AM post. Thanks, Aaron
        Hide
        Eloy Lafuente (stronk7) added a comment -

        Hi Aaron,

        sorry, it was me, forgetting about your initially reported problem (course list showing the IT concept linked, hence, breaking the course link itself).

        About that, can you try one temporary fix, and report here if, once applied, it works as expected? While you do that, I need to discuss with some other developers about it, but your feedback will be welcome.

        The temporary change to apply is to edit the course/renderer.php file and change the line number 133 from:

        $content .= html_writer::link(new moodle_url('/course/view.php', array('id'=>$course->id)), format_text($course->fullname, FORMAT_HTML), array('class'=>$linkclass));
        

        to:

        $content .= html_writer::link(new moodle_url('/course/view.php', array('id'=>$course->id)), format_string($course->fullname), array('class'=>$linkclass));
        

        I'm reopening this issue right now. Tell me if that fixes your problem or causes any other visible annoyance... TIA!

        Show
        Eloy Lafuente (stronk7) added a comment - Hi Aaron, sorry, it was me, forgetting about your initially reported problem (course list showing the IT concept linked, hence, breaking the course link itself). About that, can you try one temporary fix, and report here if, once applied, it works as expected? While you do that, I need to discuss with some other developers about it, but your feedback will be welcome. The temporary change to apply is to edit the course/renderer.php file and change the line number 133 from: $content .= html_writer::link( new moodle_url('/course/view.php', array('id'=>$course->id)), format_text($course->fullname, FORMAT_HTML), array('class'=>$linkclass)); to: $content .= html_writer::link( new moodle_url('/course/view.php', array('id'=>$course->id)), format_string($course->fullname), array('class'=>$linkclass)); I'm reopening this issue right now. Tell me if that fixes your problem or causes any other visible annoyance... TIA!
        Hide
        Eloy Lafuente (stronk7) added a comment -

        Aaron, I'm resolving this as fixed again. Feel free to comment / reopen if necessary.

        For reference, this is the change that will be sent to Moodle 2.0, once approved.

        https://github.com/stronk7/moodle/commit/f4c23f03dc7847ca104330a8d9f00146f5fe8d8f

        Ciao

        Show
        Eloy Lafuente (stronk7) added a comment - Aaron, I'm resolving this as fixed again. Feel free to comment / reopen if necessary. For reference, this is the change that will be sent to Moodle 2.0, once approved. https://github.com/stronk7/moodle/commit/f4c23f03dc7847ca104330a8d9f00146f5fe8d8f Ciao
        Hide
        Eloy Lafuente (stronk7) added a comment -

        Note: Have changed the title a bit to better represent the problem, FYI.

        Show
        Eloy Lafuente (stronk7) added a comment - Note: Have changed the title a bit to better represent the problem, FYI.
        Hide
        Aaron Cowell added a comment -

        Eloy, Thank you so much for your contribution, you guys are excellent!

        I've made the code change on my server and I can confirm that it does not auto-link the course name. Something I do find interesting is that I just added a forum posting with IT in the subject and IT in the message and only the IT in the message auto-linked to the glossary. I could care less about that issue, glad the course glossary auto-linking is resolved.

        Thanks again for your work on Moodle!

        Aaron

        Show
        Aaron Cowell added a comment - Eloy, Thank you so much for your contribution, you guys are excellent! I've made the code change on my server and I can confirm that it does not auto-link the course name. Something I do find interesting is that I just added a forum posting with IT in the subject and IT in the message and only the IT in the message auto-linked to the glossary. I could care less about that issue, glad the course glossary auto-linking is resolved. Thanks again for your work on Moodle! Aaron
        Hide
        Eloy Lafuente (stronk7) added a comment -

        Thanks Aaron,

        in general "simple strings" (also named "headings") (course name, post subject, activity name...) aren't processed by filters (glossary, for example), unless you have configured the filter (admin->plugins->filters) to process both contents and headings.

        Surely you will see more "IT" linked if you enabled both, but I wouldn't do it unless you've a very good reason for that (because it forces Moodle to process many, many more strings on each request => slower).

        Ciao

        PS: You were really lucky with this bug, lol (easy to fix and coincidence of me having some spare time). Please don't blame us too much if you aren't so lucky in the future, haha. Thanks!

        Show
        Eloy Lafuente (stronk7) added a comment - Thanks Aaron, in general "simple strings" (also named "headings") (course name, post subject, activity name...) aren't processed by filters (glossary, for example), unless you have configured the filter (admin->plugins->filters) to process both contents and headings. Surely you will see more "IT" linked if you enabled both, but I wouldn't do it unless you've a very good reason for that (because it forces Moodle to process many, many more strings on each request => slower). Ciao PS: You were really lucky with this bug, lol (easy to fix and coincidence of me having some spare time). Please don't blame us too much if you aren't so lucky in the future, haha. Thanks!

          People

          • Votes:
            0 Vote for this issue
            Watchers:
            0 Start watching this issue

            Dates

            • Created:
              Updated:
              Resolved: