Uploaded image for project: 'Moodle'
  1. Moodle
  2. MDL-33088

Glossary concepts in recent activity block are filtered

    Details

    • Type: Bug
    • Status: Closed
    • Priority: 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

      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.

        Gliffy Diagrams

          Activity

          Hide
          fred Frédéric Massart added a comment -

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

          Show
          fred Frédéric Massart added a comment - I updated the heading to an h3 to add consistency with the other modules
          Hide
          samhemelryk 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
          samhemelryk 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
          fred 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
          fred 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
          poltawski 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
          poltawski 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
          fred Frédéric Massart added a comment -

          This has been rebased! Cheers!

          Show
          fred Frédéric Massart added a comment - This has been rebased! Cheers!
          Hide
          poltawski 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
          poltawski 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
          fred Frédéric Massart added a comment -

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

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

          Perfect. Integrated, thanks.

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

          Works as described in master. Passing.

          Show
          andyjdavis Andrew Davis added a comment - Works as described in master. Passing.
          Hide
          stronk7 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
          stronk7 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:
                Fix Release Date:
                25/Jun/12