Moodle
  1. Moodle
  2. MDL-33088

Glossary concepts in recent activity block are filtered

    Details

    • Type: Bug Bug
    • Status: Closed
    • Priority: Minor Minor
    • Resolution: Fixed
    • Affects Version/s: 2.3
    • Fix Version/s: 2.3
    • Component/s: Glossary
    • Labels:
      None
    • Testing Instructions:
      Hide

      0. Enable glossary auto linking in filters
      1. Create a glossary
      2. Add an entry: Concept = 'Test', Keywords = 'moodle', enable auto linking.
      3. Add an entry: Concept = 'Moodle', enable auto linking.
      4. Add a recent activity block if needed
      5. Make sure that none of the entries have been auto linked.

      Show
      0. Enable glossary auto linking in filters 1. Create a glossary 2. Add an entry: Concept = 'Test', Keywords = 'moodle', enable auto linking. 3. Add an entry: Concept = 'Moodle', enable auto linking. 4. Add a recent activity block if needed 5. Make sure that none of the entries have been auto linked.
    • Affected Branches:
      MOODLE_23_STABLE
    • Fixed Branches:
      MOODLE_23_STABLE
    • Pull from Repository:
    • Pull Master Branch:
      MDL-33088-master
    • Rank:
      40348

      Description

      When viewing the glossary entries in the recent activity block concepts can be filtered.
      Upon filtering, if the entry is 'autolinking enabled', a link will be created in place of the current one.
      Worse case scenario, the concept is used as 'autolinking' by another concept (via the concept or keywords), and in that case the link would not even be relevant to that entry.

        Activity

        Hide
        Frédéric Massart added a comment -

        I updated the heading to an h3 to add consistency with the other modules

        Show
        Frédéric Massart added a comment - I updated the heading to an h3 to add consistency with the other modules
        Hide
        Sam Hemelryk added a comment -

        Change looks good thanks Fred, feel free to put this up for integration when you are ready.

        Just as side did you know that format_text has a filter option?
        In this case a quick grep seemed to show $glossary->concept was being passed through format_string in other places within the module already so I think you've taken the right path.

        Cheers
        Sam

        Show
        Sam Hemelryk added a comment - Change looks good thanks Fred, feel free to put this up for integration when you are ready. Just as side did you know that format_text has a filter option? In this case a quick grep seemed to show $glossary->concept was being passed through format_string in other places within the module already so I think you've taken the right path. Cheers Sam
        Hide
        Frédéric Massart added a comment -

        Yes, I noticed the format_text() filter option, but after an intensive discussion with Eloy on the chat we decided to go with the lighter function.

        Going to integration.

        Thanks!

        Show
        Frédéric Massart added a comment - Yes, I noticed the format_text() filter option, but after an intensive discussion with Eloy on the chat we decided to go with the lighter function. Going to integration. Thanks!
        Hide
        Dan Poltawski added a comment -

        The main moodle.git repository has just been updated with latest weekly modifications. You may wish to rebase your PULL branches to simplify history and avoid any possible merge conflicts. This would also make integrator's life easier next week.

        TIA and ciao

        Show
        Dan Poltawski added a comment - The main moodle.git repository has just been updated with latest weekly modifications. You may wish to rebase your PULL branches to simplify history and avoid any possible merge conflicts. This would also make integrator's life easier next week. TIA and ciao
        Hide
        Frédéric Massart added a comment -

        This has been rebased! Cheers!

        Show
        Frédéric Massart added a comment - This has been rebased! Cheers!
        Hide
        Dan Poltawski added a comment -

        Hi Fred,

        Looks fine, but just one quick thing if you could fix.

        Rather than changing the heading to manually generated tag:

        	
        -    echo $OUTPUT->heading(get_string('newentries', 'glossary').':');	
        +    echo html_writer::tag('h3', get_string('newentries', 'glossary').':', array('class' => 'main'));
        

        Please could you just convert $OUTPUT->heading() to be passed the level param of 3:

        lib/outputrenderers.php:2330:    public function heading($text, $level = 2, $classes = 'main', $id = null) {
        

        Then its not such an odd diff, and actually more consistent.

        Show
        Dan Poltawski added a comment - Hi Fred, Looks fine, but just one quick thing if you could fix. Rather than changing the heading to manually generated tag: - echo $OUTPUT->heading(get_string('newentries', 'glossary').':'); + echo html_writer::tag('h3', get_string('newentries', 'glossary').':', array('class' => 'main')); Please could you just convert $OUTPUT->heading() to be passed the level param of 3: lib/outputrenderers.php:2330: public function heading($text, $level = 2, $classes = 'main', $id = null ) { Then its not such an odd diff, and actually more consistent.
        Hide
        Frédéric Massart added a comment -

        Hi Dan, my branch is updated with your remarks. Cheers!

        Show
        Frédéric Massart added a comment - Hi Dan, my branch is updated with your remarks. Cheers!
        Hide
        Dan Poltawski added a comment -

        Perfect. Integrated, thanks.

        Show
        Dan Poltawski added a comment - Perfect. Integrated, thanks.
        Hide
        Andrew Davis added a comment -

        Works as described in master. Passing.

        Show
        Andrew Davis added a comment - Works as described in master. Passing.
        Hide
        Eloy Lafuente (stronk7) added a comment -

        We could celebrate it today... but better if we perform a bigger party after releasing Moodle 2.3.

        Print this message and come to Perth that day, it's valid for one beer, wine, coke or... water, as you wish.

        Many thanks for your collaboration!

        Ciao

        Show
        Eloy Lafuente (stronk7) added a comment - We could celebrate it today... but better if we perform a bigger party after releasing Moodle 2.3. Print this message and come to Perth that day, it's valid for one beer, wine, coke or... water, as you wish. Many thanks for your collaboration! Ciao

          People

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

            Dates

            • Created:
              Updated:
              Resolved: