Moodle
  1. Moodle
  2. MDL-38442

Generating navigation by categories and courses causes no access to quiz result reports over the navigation

    Details

    • Type: Bug Bug
    • Status: Closed
    • Priority: Critical Critical
    • Resolution: Fixed
    • Affects Version/s: 2.4.2
    • Fix Version/s: 2.3.7, 2.4.4
    • Component/s: Navigation, Quiz
    • Labels:
    • Testing Instructions:
      Hide

      Repeat the test steps from MDL-33013:

      1. Log in as an admin
      2. Turn on editing
      3. Change the navigation block settings to generate only courses/categories.
      4. Browse to a course and check you only see the course + participants + reports in the nav.
      5. Browse to an activity and check you see the course, the section containing the activity and the activity itself. No other sections/activities.
      6. Edit your course settings to show single sections.
      7. Browse to a section and check you see it under the course in the navigation.

      Test this exact issue:

      1. Log in as an admin.
      2. Browse to a course.
      3. Create a quiz and add a single question.
      4. Log in as a student and attempt the quiz.
      5. Log is as an admin.
      6. Turn on editing.
      7. Change the navigation block settings to generate only courses/categories.
      8. Browse to the quiz.
      9. Check you see both info and results nodes under the quiz in the navigation block (in master you won't see the results node).
      Show
      Repeat the test steps from MDL-33013 : Log in as an admin Turn on editing Change the navigation block settings to generate only courses/categories. Browse to a course and check you only see the course + participants + reports in the nav. Browse to an activity and check you see the course, the section containing the activity and the activity itself. No other sections/activities. Edit your course settings to show single sections. Browse to a section and check you see it under the course in the navigation. Test this exact issue: Log in as an admin. Browse to a course. Create a quiz and add a single question. Log in as a student and attempt the quiz. Log is as an admin. Turn on editing. Change the navigation block settings to generate only courses/categories. Browse to the quiz. Check you see both info and results nodes under the quiz in the navigation block (in master you won't see the results node).
    • Affected Branches:
      MOODLE_24_STABLE
    • Fixed Branches:
      MOODLE_23_STABLE, MOODLE_24_STABLE
    • Pull 2.4 Branch:
      wip-MDL-38442-m24
    • Pull Master Branch:
      wip-MDL-38442-m25
    • Rank:
      48391

      Description

      Our navigation block is configured to be generated by categories and courses.

      We add a quiz to the course and some responses. Now we want to have a look at the results. In former versions of Moodle, a branch named "Results" was shown under the navigation entry of the quiz in the navigation block. But this branch is missing.

      It seems like a bug.

      I tried a small workaround and forced the navigation-entry for the results not to hide. I've added a parameter "doNeverHide" in the class navigation_node (lib/navigationlib.php) and evaluated this value in the hide-function. To set this flag, I've extended the function quiz_extend_navigation(...) in mod/quiz/lib.php. Here I set the node $reportnode the flag "doNeverHide".

      This worked for me. It seems as if the recursive call of hide() triggerd by parent nodes causes this behavior.

      Hope my information is useful to resolve this.

        Issue Links

          Activity

          Hide
          Adam Jenkins added a comment -

          > To set this flag, I've extended the function quiz_extend_navigation(...) in mod/quiz/lib.php. Here I set the node $reportnode the flag "doNeverHide".

          Would you mind sharing the code you altered so I can try applying it to my problem (MDL-38458)? It looks like you have the solution to my problem. Thanks.

          Show
          Adam Jenkins added a comment - > To set this flag, I've extended the function quiz_extend_navigation(...) in mod/quiz/lib.php. Here I set the node $reportnode the flag "doNeverHide". Would you mind sharing the code you altered so I can try applying it to my problem ( MDL-38458 )? It looks like you have the solution to my problem. Thanks.
          Hide
          Wolfgang Kimmig added a comment -

          Hi Adam,

          the hide-function in the class navigation_node looks like this:

              
              var $doNeverHide;
          
              /**
               * Hides the node and any children it has.
               *
               * @since 2.4.2
               */
              public function hide() {
              	// HSO modification start - Force node not to be hidden by parent-nodes.
              	if($this->doNeverHide){
              		return;
              	}
          	// HSO modification end
          		
                  $this->display = false;
                  if ($this->has_children()) {
                      foreach ($this->children as $child) {
                          $child->hide();
                      }
                  }
              }
              // HSO Modification start - Force node not to be hidden by parent-nodes.
              public function doNeverHide(){
                  $this->doNeverHide = true;
              }
              // HSO Modification end
          

          ... and in mod/quiz/lib.php the function quiz_extend_navigation(...) is changed to call the function doNeverHide() like this:

          /**
           * This fucntion extends the global navigation for the site.
           * It is important to note that you should not rely on PAGE objects within this
           * body of code as there is no guarantee that during an AJAX request they are
           * available
           *
           * @param navigation_node $quiznode The quiz node within the global navigation
           * @param object $course The course object returned from the DB
           * @param object $module The module object returned from the DB
           * @param object $cm The course module instance returned from the DB
           */
          function quiz_extend_navigation($quiznode, $course, $module, $cm) {
              global $CFG;
          
              $context = context_module::instance($cm->id);
          
              if (has_capability('mod/quiz:view', $context)) {
                  $url = new moodle_url('/mod/quiz/view.php', array('id'=>$cm->id));
                  $quiznode->add(get_string('info', 'quiz'), $url, navigation_node::TYPE_SETTING,
                          null, null, new pix_icon('i/info', ''));
              }
          
              if (has_any_capability(array('mod/quiz:viewreports', 'mod/quiz:grade'), $context)) {
                  require_once($CFG->dirroot . '/mod/quiz/report/reportlib.php');
                  $reportlist = quiz_report_list($context);
          
                  $url = new moodle_url('/mod/quiz/report.php',
                          array('id' => $cm->id, 'mode' => reset($reportlist)));
                  $reportnode = $quiznode->add(get_string('results', 'quiz'), $url,
                          navigation_node::TYPE_SETTING,
                          null, null, new pix_icon('i/report', ''));
          		
          	// HSO modification start - Force node not to be hidden by parent-nodes.
          	$reportnode->doNeverHide();
          	// HSO modification end
          		
                  foreach ($reportlist as $report) {
                  	
                      $url = new moodle_url('/mod/quiz/report.php',
                              array('id' => $cm->id, 'mode' => $report));
                      $reportnode->add(get_string($report, 'quiz_'.$report), $url,
                              navigation_node::TYPE_SETTING,
                              null, 'quiz_report_' . $report, new pix_icon('i/item', ''));
                  }
              }
          }
          

          Not really nice, because it was a little hack to figure out, what's going wrong in our navigation. I'm not sure, if this is the intended way. Better solutions are welcome

          Cheers,
          Wolfgang

          Show
          Wolfgang Kimmig added a comment - Hi Adam, the hide-function in the class navigation_node looks like this: var $doNeverHide; /** * Hides the node and any children it has. * * @since 2.4.2 */ public function hide() { // HSO modification start - Force node not to be hidden by parent-nodes. if ($ this ->doNeverHide){ return ; } // HSO modification end $ this ->display = false ; if ($ this ->has_children()) { foreach ($ this ->children as $child) { $child->hide(); } } } // HSO Modification start - Force node not to be hidden by parent-nodes. public function doNeverHide(){ $ this ->doNeverHide = true ; } // HSO Modification end ... and in mod/quiz/lib.php the function quiz_extend_navigation(...) is changed to call the function doNeverHide() like this: /** * This fucntion extends the global navigation for the site. * It is important to note that you should not rely on PAGE objects within this * body of code as there is no guarantee that during an AJAX request they are * available * * @param navigation_node $quiznode The quiz node within the global navigation * @param object $course The course object returned from the DB * @param object $module The module object returned from the DB * @param object $cm The course module instance returned from the DB */ function quiz_extend_navigation($quiznode, $course, $module, $cm) { global $CFG; $context = context_module::instance($cm->id); if (has_capability('mod/quiz:view', $context)) { $url = new moodle_url('/mod/quiz/view.php', array('id'=>$cm->id)); $quiznode->add(get_string('info', 'quiz'), $url, navigation_node::TYPE_SETTING, null , null , new pix_icon('i/info', '')); } if (has_any_capability(array('mod/quiz:viewreports', 'mod/quiz:grade'), $context)) { require_once($CFG->dirroot . '/mod/quiz/report/reportlib.php'); $reportlist = quiz_report_list($context); $url = new moodle_url('/mod/quiz/report.php', array('id' => $cm->id, 'mode' => reset($reportlist))); $reportnode = $quiznode->add(get_string('results', 'quiz'), $url, navigation_node::TYPE_SETTING, null , null , new pix_icon('i/report', '')); // HSO modification start - Force node not to be hidden by parent-nodes. $reportnode->doNeverHide(); // HSO modification end foreach ($reportlist as $report) { $url = new moodle_url('/mod/quiz/report.php', array('id' => $cm->id, 'mode' => $report)); $reportnode->add(get_string($report, 'quiz_'.$report), $url, navigation_node::TYPE_SETTING, null , 'quiz_report_' . $report, new pix_icon('i/item', '')); } } } Not really nice, because it was a little hack to figure out, what's going wrong in our navigation. I'm not sure, if this is the intended way. Better solutions are welcome Cheers, Wolfgang
          Hide
          Joshua Bragg added a comment -

          I just upgraded to 2.4.3 from 2.4.1 over the weekend and I'm seeing the same issue when using all of the various options for "Generate navigation for the following" except for everything.

          Show
          Joshua Bragg added a comment - I just upgraded to 2.4.3 from 2.4.1 over the weekend and I'm seeing the same issue when using all of the various options for "Generate navigation for the following" except for everything.
          Hide
          Matt Petro added a comment -

          This is regression caused by MDL-33013

          A simple fix for now is just to back-out that change: In lib/navigationlib.php function set_expansion_limit()

          Replace

          $child->hide();
          

          with

          $child->display = false;
          
          Show
          Matt Petro added a comment - This is regression caused by MDL-33013 A simple fix for now is just to back-out that change: In lib/navigationlib.php function set_expansion_limit() Replace $child->hide(); with $child->display = false ;
          Hide
          Tim Hunt added a comment -

          Bumping priority, to hopefully get Sam to notice this.

          Show
          Tim Hunt added a comment - Bumping priority, to hopefully get Sam to notice this.
          Hide
          Adam Jenkins added a comment -

          Thank you Matt Petro for that simple fix. It indeed has simply fixed the problem I was having.

          Show
          Adam Jenkins added a comment - Thank you Matt Petro for that simple fix. It indeed has simply fixed the problem I was having.
          Hide
          Don Hazelwood added a comment -

          I am seeing this behavior in a 2.3.6 site as well ...

          Show
          Don Hazelwood added a comment - I am seeing this behavior in a 2.3.6 site as well ...
          Hide
          Sam Hemelryk added a comment -

          Sorry guys, I've been off on personal matters for the past week and a bit.
          Looking at this now.

          Show
          Sam Hemelryk added a comment - Sorry guys, I've been off on personal matters for the past week and a bit. Looking at this now.
          Hide
          Sam Hemelryk added a comment -

          OK I've got a solution for this up for peer-review now.

          The solution I've chosen to go with works by limiting the types of nodes that get hidden when setting the expansion limit.
          To cater for this I've introduced an argument to the navigation_node::hide method that allows the developer to specify what types of nodes get hidden.

          The reason I chose this solution was because it was a more specific solution that didn't require conditional hacks and allowed me to remove a hack that was already in place.

          If someone has time to review + test that would be most appreciated.

          Many thanks
          Sam

          Show
          Sam Hemelryk added a comment - OK I've got a solution for this up for peer-review now. The solution I've chosen to go with works by limiting the types of nodes that get hidden when setting the expansion limit. To cater for this I've introduced an argument to the navigation_node::hide method that allows the developer to specify what types of nodes get hidden. The reason I chose this solution was because it was a more specific solution that didn't require conditional hacks and allowed me to remove a hack that was already in place. If someone has time to review + test that would be most appreciated. Many thanks Sam
          Hide
          Jason Fowler added a comment -

          [Y] Syntax
          [Y] Output
          [Y] Whitespace
          [-] Language
          [-] Databases
          [Y] Testing
          [Y] Security
          [-] Documentation
          [Y] Git
          [Y] Sanity check

          Show
          Jason Fowler added a comment - [Y] Syntax [Y] Output [Y] Whitespace [-] Language [-] Databases [Y] Testing [Y] Security [-] Documentation [Y] Git [Y] Sanity check
          Hide
          Tim Hunt added a comment -

          Before this gets integrated, could I have a few minutes to look at it?

          Show
          Tim Hunt added a comment - Before this gets integrated, could I have a few minutes to look at it?
          Hide
          Dan Poltawski added a comment -

          Yup.

          Show
          Dan Poltawski added a comment - Yup.
          Hide
          Tim Hunt added a comment -

          OK, So having looked at the patch, it makes more sense to me.

          I was worried by the description "To cater for this I've introduced an argument to the navigation_node::hide method that allows the developer to specify what types of nodes get hidden." because it seems that that required manual effort by each module, but actually, all this is handled within navigationlib.php, and the proposed solution seems like a good way to handle the implementation details.

          +1 from me.

          Show
          Tim Hunt added a comment - OK, So having looked at the patch, it makes more sense to me. I was worried by the description "To cater for this I've introduced an argument to the navigation_node::hide method that allows the developer to specify what types of nodes get hidden." because it seems that that required manual effort by each module, but actually, all this is handled within navigationlib.php, and the proposed solution seems like a good way to handle the implementation details. +1 from me.
          Hide
          Dan Poltawski added a comment -

          Integrated to master, 24 and 23 - thanks Sam

          Show
          Dan Poltawski added a comment - Integrated to master, 24 and 23 - thanks Sam
          Hide
          Rajesh Taneja added a comment -

          Thanks Sam,

          This works fine in 23 and 24, but for master it doesn't help. As "Results" have been moved to Administration block and info is merged with quiz link (in navigation block) this is not an issue in current master branch.

          As it works fine in master, looking at patch it makes sense to have it for master as well.

          Passing it.

          Show
          Rajesh Taneja added a comment - Thanks Sam, This works fine in 23 and 24, but for master it doesn't help. As "Results" have been moved to Administration block and info is merged with quiz link (in navigation block) this is not an issue in current master branch. As it works fine in master, looking at patch it makes sense to have it for master as well. Passing it.
          Hide
          Eloy Lafuente (stronk7) added a comment -

          Your awesome contributions are now part of Moodle, your fav LMS out there.

          Closing this as fixed.

          Many thanks for all the hard work, ciao

          Show
          Eloy Lafuente (stronk7) added a comment - Your awesome contributions are now part of Moodle, your fav LMS out there. Closing this as fixed. Many thanks for all the hard work, ciao

            People

            • Votes:
              6 Vote for this issue
              Watchers:
              10 Start watching this issue

              Dates

              • Created:
                Updated:
                Resolved: