Uploaded image for project: '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
    • Status: Closed
    • Priority: 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 Master Branch:
      wip-MDL-38442-m25

      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.

        Gliffy Diagrams

          Issue Links

            Activity

            Hide
            adamj 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
            adamj 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
            wkimmig 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
            wkimmig 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
            worfds91 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
            worfds91 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
            mpetrowi 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
            mpetrowi 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
            timhunt Tim Hunt added a comment -

            Bumping priority, to hopefully get Sam to notice this.

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

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

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

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

            Show
            dphazelwood Don Hazelwood added a comment - I am seeing this behavior in a 2.3.6 site as well ...
            Hide
            samhemelryk 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
            samhemelryk 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
            samhemelryk 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
            samhemelryk 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
            phalacee Jason Fowler added a comment -

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

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

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

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

            Yup.

            Show
            poltawski Dan Poltawski added a comment - Yup.
            Hide
            timhunt 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
            timhunt 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
            poltawski Dan Poltawski added a comment -

            Integrated to master, 24 and 23 - thanks Sam

            Show
            poltawski Dan Poltawski added a comment - Integrated to master, 24 and 23 - thanks Sam
            Hide
            rajeshtaneja 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
            rajeshtaneja 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
            stronk7 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
            stronk7 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:
                  Fix Release Date:
                  13/May/13