Moodle
  1. Moodle
  2. MDL-37523

MyMobile theme's include of Choice mod renderer crashes restores if Choice module is absent

    Details

    • Testing Instructions:
      Hide

      BEFORE APPLYING THE PATCH PRIOR TO TESTING

      1. Goto Site administration ► Plugins ► Activity modules ► Manage activities: Delete the Choice module
      2. Remove yourmoodle/mod/choice folder
      3. Backup a couple of resources or activities without users from a course.
      4. Restore that backup
      5. Select MyMobile theme from Theme selector or by URL
      6. You should now get the error message: Warning: include_once(moodle/mod/choice/renderer.php) [function.include-once]: failed to open stream: No such file or directory in moodle/theme/mymobile/renderers.php on line 35

      TESTING

      1. Apply the patch
      2. Follow same scenario as 1 to 6 above
      3. TEST you DO NOT get error message upon restoring course.
      Show
      BEFORE APPLYING THE PATCH PRIOR TO TESTING Goto Site administration ► Plugins ► Activity modules ► Manage activities: Delete the Choice module Remove yourmoodle/mod/choice folder Backup a couple of resources or activities without users from a course. Restore that backup Select MyMobile theme from Theme selector or by URL You should now get the error message: Warning: include_once(moodle/mod/choice/renderer.php) [function.include-once] : failed to open stream: No such file or directory in moodle/theme/mymobile/renderers.php on line 35 TESTING Apply the patch Follow same scenario as 1 to 6 above TEST you DO NOT get error message upon restoring course.
    • Affected Branches:
      MOODLE_23_STABLE, MOODLE_24_STABLE, MOODLE_25_STABLE
    • Fixed Branches:
      MOODLE_23_STABLE, MOODLE_24_STABLE
    • Pull 2.4 Branch:
      MDL-37523_M24
    • Pull Master Branch:
      MDL-37523_master
    • Rank:
      47170

      Description

      MyMobile theme's renderers.php line 35 has a call to include /mod/choice/renderer.php. This is really strange, as no other Moodle theme has such include to a module!

      On my moodle 2.4 test site, I have removed the Choice module, which I never use.
      When I backup and then restore a course (or part of course) on that test site, in the very last stage of restore I get the following error messages and a crash:

      Warning: include_once(moodle24/mod/choice/renderer.php) [function.include-once]: failed to open stream: No such file or directory in moodle24/theme/mymobile/renderers.php on line 35

      Warning: include_once() [function.include]: Failed opening 'moodle24/mod/choice/renderer.php' for inclusion (include_path='moodle24/lib/zend:moodle24/lib/pear:.:/usr/local/lib/php') in moodle24/theme/mymobile/renderers.php on line 35

      Fatal error: Class 'mod_choice_renderer' not found in moodle24/theme/mymobile/renderers.php on line 804.

      Please remove from MyMobile theme that include /mod/choice/renderer.php OR at least test that the Choice module does exist on a moodle site before trying to include it!

        Activity

        Hide
        Joseph Rézeau added a comment -

        BUMP

        Show
        Joseph Rézeau added a comment - BUMP
        Hide
        Mary Evans added a comment -

        @ Joseph Rézeau Did you add me as a watcher as I don't recall seeing this tracker ticket before?

        I think the problem is in the way the renderer has been written I think it should be written like so...

        /**
         * A custom renderer for the mymobile theme to produce snippets of content.
         *
         * @package    theme
         * @subpackage mymobile
         * @copyright  John Stabinger
         * @license    http://www.gnu.org/copyleft/gpl.html GNU GPL v3 or later
         */
        
        class theme_mymobile_renderer extends mod_choice_renderer {
        
            /**
             * Produces the settings tree
             *
             * @param settings_navigation $navigation
             * @return string
             */
            public function settings_tree(settings_navigation $navigation) {
                $content = $this->navigation_node($navigation, array('class' => 'settings'));
                if (has_capability('moodle/site:config', context_system::instance())) {
                    // TODO: Work out whether something is missing from here.
                }
                return $content;
            }
        
        

        The fact that John has written a TODO suggests to me that this is experimental or he wants to see if it can be added to, which is fair enough in the circumstances. Let's face it, renderers are pretty much the "new kid on the block" as far as theme designers are concerned, and not easy to understand.

        Show
        Mary Evans added a comment - @ Joseph Rézeau Did you add me as a watcher as I don't recall seeing this tracker ticket before? I think the problem is in the way the renderer has been written I think it should be written like so... /** * A custom renderer for the mymobile theme to produce snippets of content. * * @ package theme * @subpackage mymobile * @copyright John Stabinger * @license http: //www.gnu.org/copyleft/gpl.html GNU GPL v3 or later */ class theme_mymobile_renderer extends mod_choice_renderer { /** * Produces the settings tree * * @param settings_navigation $navigation * @ return string */ public function settings_tree(settings_navigation $navigation) { $content = $ this ->navigation_node($navigation, array('class' => 'settings')); if (has_capability('moodle/site:config', context_system::instance())) { // TODO: Work out whether something is missing from here. } return $content; } The fact that John has written a TODO suggests to me that this is experimental or he wants to see if it can be added to, which is fair enough in the circumstances. Let's face it, renderers are pretty much the "new kid on the block" as far as theme designers are concerned, and not easy to understand.
        Hide
        Joseph Rézeau added a comment -

        Thanks for the reply, Mary. Yes I had added you as watcher. Any idea why John is not responding?

        Show
        Joseph Rézeau added a comment - Thanks for the reply, Mary. Yes I had added you as watcher. Any idea why John is not responding?
        Hide
        Mary Evans added a comment - - edited

        Just looking at this and finding it difficult to locate where public function settings_tree has been derived from. If the renderer does not exist then it cannot be created in a theme. So this looks like it's flawed in some way. Although it is more likely I am reading this the wrong way.

        Show
        Mary Evans added a comment - - edited Just looking at this and finding it difficult to locate where public function settings_tree has been derived from. If the renderer does not exist then it cannot be created in a theme. So this looks like it's flawed in some way. Although it is more likely I am reading this the wrong way.
        Hide
        Mary Evans added a comment -

        Not sure where John is at the moment, so can't answer that Joseph. He might not be getting any notifications.

        Show
        Mary Evans added a comment - Not sure where John is at the moment, so can't answer that Joseph. He might not be getting any notifications.
        Hide
        Joseph Rézeau added a comment -

        It certainly is a flaw for a theme to have a renderer which is an extension of a Moodle module.
        I expect this was only a provisional attempt and really the myMobile theme should include its own complete renderer.

        Show
        Joseph Rézeau added a comment - It certainly is a flaw for a theme to have a renderer which is an extension of a Moodle module . I expect this was only a provisional attempt and really the myMobile theme should include its own complete renderer.
        Hide
        Mary Evans added a comment - - edited

        @Tim Hunt I've just added you as a watcher here to ask for some help with the renderer in question. It sort of similar to what I was asking in Dev Chat yesterday, but not the same renderer I was talking about. This one is pretty new to me. So if you have a minute to spare to take a look, I would be glad of a second opinion.

        Cheers
        Mary

        Show
        Mary Evans added a comment - - edited @ Tim Hunt I've just added you as a watcher here to ask for some help with the renderer in question. It sort of similar to what I was asking in Dev Chat yesterday, but not the same renderer I was talking about. This one is pretty new to me. So if you have a minute to spare to take a look, I would be glad of a second opinion. Cheers Mary
        Hide
        Mary Evans added a comment - - edited

        Just looking at this again and as I suspected I was reading this render wrongly. What I said earlier is wrong. I am not sure about any of this now...totally lost in fact!

        But what I did wonder is if the word include is wrong and should actually be required instead?
        So may be should look like this...

        required_once ($CFG->dirroot. '/mod/choice/renderer.php');
        Show
        Mary Evans added a comment - - edited Just looking at this again and as I suspected I was reading this render wrongly. What I said earlier is wrong. I am not sure about any of this now...totally lost in fact! But what I did wonder is if the word include is wrong and should actually be required instead? So may be should look like this... required_once ($CFG->dirroot. '/mod/choice/renderer.php');
        Hide
        Joseph Rézeau added a comment -

        @Mary, in my opinion a theme renderer should not require or include a module renderer at all.

        Show
        Joseph Rézeau added a comment - @Mary, in my opinion a theme renderer should not require or include a module renderer at all.
        Hide
        Tim Hunt added a comment - - edited

        There is no bug here.

        theme_mymobile wants to override the mod_choice renderer:

        class theme_mymobile_mod_choice_renderer extends mod_choice_renderer {

        that is a perfectly reasonable thing for a theme to do. In order to do this, it has to load the definition of the mod_choice_renderer class, hence the require once, also reasonable.

        As Joseph says, the problem comes when you want to un-install the choice module. (Although there is really no reason to do this. Just disable the module in the admin settings.) A better example would be a theme that wants to override mod_forumng_renderer, or something like that. Clearly, it should be possible for a theme to provide an override for the renderer of an add-on plugin, without causing errors if the plug-in is not installed. At least it is include not require, so it is not a fatal error.

        Acutally, it can be a bit messy putting all the renderres in one file. For the OU theme, we make a renderers folder, with separate files in. Then our renderers.php file looks like

        // Core renderers (alphabetical)
        require_once($CFG->dirroot . '/theme/ou/renderers/core_renderer.php');
        require_once($CFG->dirroot . '/theme/ou/renderers/core_calendar_renderer.php');
        require_once($CFG->dirroot . '/theme/ou/renderers/core_question_renderer.php');
        
        // Module renderers (alphabetical)
        require_once($CFG->dirroot . '/theme/ou/renderers/mod_oucontent_renderer.php');
        require_once($CFG->dirroot . '/theme/ou/renderers/mod_subpage_renderer.php');
        require_once($CFG->dirroot . '/theme/ou/renderers/mod_forumng_renderer.php');
        
        // Block renderers (alphabetical)
        require_once($CFG->dirroot . '/theme/ou/renderers/block_course_resources_renderer.php');
        require_once($CFG->dirroot . '/theme/ou/renderers/block_navigation_renderer.php');
        require_once($CFG->dirroot . '/theme/ou/renderers/block_settings_renderer.php');
        
        // Other renderers (alphabetical)
        require_once($CFG->dirroot . '/theme/ou/renderers/format_studyplan_renderer.php');
        

        If you adopt that pattern, then you could change it to

        $choice = get_plugin_dir('mod', 'choice');
        if (file_exists($choice . '/renderer.php')) {
            require_once($CFG->dirroot . '/theme/mymobile/renderers/mod_choice_renderer.php');
        }
        

        then /theme/mymobile/renderers/mod_choice_renderer.php would do the require_once of /mod/choice/renderer.php.

        Show
        Tim Hunt added a comment - - edited There is no bug here. theme_mymobile wants to override the mod_choice renderer: class theme_mymobile_mod_choice_renderer extends mod_choice_renderer { that is a perfectly reasonable thing for a theme to do. In order to do this, it has to load the definition of the mod_choice_renderer class, hence the require once, also reasonable. As Joseph says, the problem comes when you want to un-install the choice module. (Although there is really no reason to do this. Just disable the module in the admin settings.) A better example would be a theme that wants to override mod_forumng_renderer, or something like that. Clearly, it should be possible for a theme to provide an override for the renderer of an add-on plugin, without causing errors if the plug-in is not installed. At least it is include not require, so it is not a fatal error. Acutally, it can be a bit messy putting all the renderres in one file. For the OU theme, we make a renderers folder, with separate files in. Then our renderers.php file looks like // Core renderers (alphabetical) require_once($CFG->dirroot . '/theme/ou/renderers/core_renderer.php'); require_once($CFG->dirroot . '/theme/ou/renderers/core_calendar_renderer.php'); require_once($CFG->dirroot . '/theme/ou/renderers/core_question_renderer.php'); // Module renderers (alphabetical) require_once($CFG->dirroot . '/theme/ou/renderers/mod_oucontent_renderer.php'); require_once($CFG->dirroot . '/theme/ou/renderers/mod_subpage_renderer.php'); require_once($CFG->dirroot . '/theme/ou/renderers/mod_forumng_renderer.php'); // Block renderers (alphabetical) require_once($CFG->dirroot . '/theme/ou/renderers/block_course_resources_renderer.php'); require_once($CFG->dirroot . '/theme/ou/renderers/block_navigation_renderer.php'); require_once($CFG->dirroot . '/theme/ou/renderers/block_settings_renderer.php'); // Other renderers (alphabetical) require_once($CFG->dirroot . '/theme/ou/renderers/format_studyplan_renderer.php'); If you adopt that pattern, then you could change it to $choice = get_plugin_dir('mod', 'choice'); if (file_exists($choice . '/renderer.php')) { require_once($CFG->dirroot . '/theme/mymobile/renderers/mod_choice_renderer.php'); } then /theme/mymobile/renderers/mod_choice_renderer.php would do the require_once of /mod/choice/renderer.php.
        Hide
        Mary Evans added a comment -

        Thanks Tim for your input.

        Sorry for going over this all again, it's just that I want to make absolutely sure you are not assuming the code I added earlier was the actual code that was in 'mymobile/renderers.php', whereas in actual fact it was just my interpretation of what I thought it should be written as, at least the top section of that renderers.php. But looking through the whole of mymobile/renderers.php again I'm seeing a different view of things.

        If you look at theme/mymobile/renderers.php in Moodle_24_STABLE

        https://github.com/moodle/moodle/blob/MOODLE_24_STABLE/theme/mymobile/renderers.php

        you will see that it starts off with...

        /**
         * A custom renderer for the mymobile theme to produce snippets of content.
         *
         * @package    theme
         * @subpackage mymobile
         * @copyright  John Stabinger
         * @license    http://www.gnu.org/copyleft/gpl.html GNU GPL v3 or later
         */
        include_once ($CFG->dirroot. '/mod/choice/renderer.php');
        
        class theme_mymobile_renderer extends plugin_renderer_base {
        
            /**
        * Produces the settings tree
        *
        * @param settings_navigation $navigation
        * @return string
        */
            public function settings_tree(settings_navigation $navigation) {
                $content = $this->navigation_node($navigation, array('class' => 'settings'));
                if (has_capability('moodle/site:config', context_system::instance())) {
                    // TODO: Work out whether something is missing from here.
                }
                return $content;
            }
        
            /**
        * Produces the navigation tree
        *
        * @param global_navigation $navigation
        * @return string
        */
            public function navigation_tree(global_navigation $navigation) {
                return $this->navigation_node($navigation, array());
            }
        
            /**
        * Protected method to render a navigaiton node
        *
        * @param navigation_node $node
        * @param array $attrs
        * @return type
        */
            protected function navigation_node(navigation_node $node, $attrs = array()) {
                $items = $node->children;
        
                // exit if empty, we don't want an empty ul element
                if ($items->count() == 0) {
                    return '';
                }
        
                // array of nested li elements
                $lis = array();
                foreach ($items as $item) {
                    if (!$item->display) {
                        continue;
                    }
        
                    $isbranch = ($item->children->count() > 0 || $item->nodetype == navigation_node::NODETYPE_BRANCH);
                    $item->hideicon = true;
        
                    $content = $this->output->render($item);
                    $content .= $this->navigation_node($item);
        
                    if ($isbranch && !(is_string($item->action) || empty($item->action))) {
                        $content = html_writer::tag('li', $content, array('data-role' => 'list-divider', 'class' => (string)$item->key));
                    } else if($isbranch) {
                        $content = html_writer::tag('li', $content, array('data-role' => 'list-divider'));
                    } else {
                        $content = html_writer::tag('li', $content, array('class' => (string)$item->text));
                    }
                    $lis[] = $content;
                }
                if (!count($lis)) {
                    return '';
                }
                return implode("\n", $lis);
            }
        }
        

        And then continues with more renderers which extend the core renderer...

        class theme_mymobile_core_renderer extends core_renderer {
        ...
        

        And then goes on to extend the mod/choice/renderer...again!

        class theme_mymobile_mod_choice_renderer extends mod_choice_renderer {
        
            /**
        * Returns HTML to display choices of option
        * @param object $options
        * @param int $coursemoduleid
        * @param bool $vertical
        * @return string
        */
            public function display_options($options, $coursemoduleid, $vertical = false) {
                $layoutclass = 'horizontal';
                if ($vertical) {
                    $layoutclass = 'vertical';
                }
                $target = new moodle_url('/mod/choice/view.php');
                //changed below to post from target john
                $attributes = array('method'=>'POST', 'action'=>$target, 'class'=> $layoutclass);
        
                $html = html_writer::start_tag('form', $attributes);
                $html .= html_writer::start_tag('ul', array('class'=>'choices', 'data-role'=>'controlgroup' ));
        
                $availableoption = count($options['options']);
                foreach ($options['options'] as $option) {
                    $html .= html_writer::start_tag('li', array('class'=>'option'));
                    $option->attributes->name = 'answer';
                    $option->attributes->type = 'radio';
                    $option->attributes->id = 'answer'.html_writer::random_id();
        
                    $labeltext = $option->text;
                    if (!empty($option->attributes->disabled)) {
                        $labeltext .= ' ' . get_string('full', 'choice');
                        $availableoption--;
                    }
        
                    $html .= html_writer::empty_tag('input', (array)$option->attributes);
                    $html .= html_writer::tag('label', $labeltext, array('for'=>$option->attributes->id));
                    $html .= html_writer::end_tag('li');
                }
                $html .= html_writer::tag('li','', array('class'=>'clearfloat'));
                $html .= html_writer::end_tag('ul');
                $html .= html_writer::tag('div', '', array('class'=>'clearfloat'));
                $html .= html_writer::empty_tag('input', array('type'=>'hidden', 'name'=>'sesskey', 'value'=>sesskey()));
                $html .= html_writer::empty_tag('input', array('type'=>'hidden', 'name'=>'id', 'value'=>$coursemoduleid));
        
                if (!empty($options['hascapability']) && ($options['hascapability'])) {
                    if ($availableoption < 1) {
                       $html .= html_writer::tag('label', get_string('choicefull', 'choice'));
                    } else {
                        $html .= html_writer::empty_tag('input', array('type'=>'submit', 'value'=>get_string('savemychoice','choice'), 'class'=>'button'));
                    }
        
                    if (!empty($options['allowupdate']) && ($options['allowupdate'])) {
                        $url = new moodle_url('view.php', array('id'=>$coursemoduleid, 'action'=>'delchoice', 'sesskey'=>sesskey()));
                        $html .= html_writer::link($url, get_string('removemychoice','choice'));
                    }
                } else {
                    $html .= html_writer::tag('label', get_string('havetologin', 'choice'));
                }
        
                $html .= html_writer::end_tag('ul');
                $html .= html_writer::end_tag('form');
        
                return $html;
            }
        
            /**
        * Returns HTML to display choices result
        *
        * TODO: There are differences between this method and the mod choice renderers function.
        * This needs to be checked VERY careful as the minor changes look like they
        * may lead to regressions.
        *
        * @param object $choices
        * @param bool $forcepublish
        * @return string
        */
            public function display_publish_name_vertical($choices) {
                $html ='';
                $html .= html_writer::tag('h2',format_string(get_string("responses", "choice")), array('class'=>'main'));
        
                $attributes = array('method'=>'POST');
                $attributes['action'] = new moodle_url('/mod/choice/view.php');
                $attributes['id'] = 'attemptsform';
        
                if ($choices->viewresponsecapability) {
                    $html .= html_writer::start_tag('form', $attributes);
                    $html .= html_writer::empty_tag('input', array('type'=>'hidden', 'name'=>'id', 'value'=> $choices->coursemoduleid));
                    $html .= html_writer::empty_tag('input', array('type'=>'hidden', 'name'=>'sesskey', 'value'=> sesskey()));
                    $html .= html_writer::empty_tag('input', array('type'=>'hidden', 'name'=>'mode', 'value'=>'overview'));
                }
        
                $table = new html_table();
                $table->cellpadding = 0;
                $table->cellspacing = 0;
                $table->attributes['class'] = 'results names ';
                $table->tablealign = 'center';
                $table->data = array();
        
                $count = 0;
                ksort($choices->options);
        
                $columns = array();
                foreach ($choices->options as $optionid => $options) {
                    $coldata = '';
                    if ($choices->showunanswered && $optionid == 0) {
                        $coldata .= html_writer::tag('div', format_string(get_string('notanswered', 'choice')), array('class'=>'option'));
                    } else if ($optionid > 0) {
                        $coldata .= html_writer::tag('div', format_string($choices->options[$optionid]->text), array('class'=>'option'));
                    }
                    $numberofuser = 0;
                    if (!empty($options->user) && count($options->user) > 0) {
                        $numberofuser = count($options->user);
                    }
        
                    $coldata .= html_writer::tag('div', ' ('.$numberofuser. ')', array('class'=>'numberofuser', 'title' => get_string('numberofuser', 'choice')));
                    $columns[] = $coldata;
                }
        
                $table->head = $columns;
        
                $coldata = '';
                $columns = array();
                foreach ($choices->options as $optionid => $options) {
                    $coldata = '';
                    if ($choices->showunanswered || $optionid > 0) {
                        if (!empty($options->user)) {
                            foreach ($options->user as $user) {
                                $data = '';
                                if (empty($user->imagealt)){
                                    $user->imagealt = '';
                                }
        
                                if ($choices->viewresponsecapability && $choices->deleterepsonsecapability && $optionid > 0) {
                                    $attemptaction = html_writer::checkbox('attemptid[]', $user->id,'');
                                    $data .= html_writer::tag('div', $attemptaction, array('class'=>'attemptaction'));
                                }
                                $userimage = $this->output->user_picture($user, array('courseid'=>$choices->courseid));
                                $data .= html_writer::tag('div', $userimage, array('class'=>'image'));
        
                                $userlink = new moodle_url('/user/view.php', array('id'=>$user->id,'course'=>$choices->courseid));
                                $name = html_writer::tag('a', fullname($user, $choices->fullnamecapability), array('href'=>$userlink, 'class'=>'username'));
                                $data .= html_writer::tag('div', $name, array('class'=>'fullname'));
                                $data .= html_writer::tag('div','', array('class'=>'clearfloat'));
                                $coldata .= html_writer::tag('div', $data, array('class'=>'user'));
                            }
                        }
                    }
        
                    $columns[] = $coldata;
                    $count++;
                }
        
                $table->data[] = $columns;
                foreach ($columns as $d) {
                    $table->colclasses[] = 'data';
                }
                $html .= html_writer::tag('div', html_writer::table($table), array('class'=>'response'));
        
                $actiondata = '';
                if ($choices->viewresponsecapability && $choices->deleterepsonsecapability) {
                    $selecturl = new moodle_url('#');
        
                    $selectallactions = new component_action('click',"select_all_in", array('div',null,'tablecontainer'));
                    $selectall = new action_link($selecturl, get_string('selectall'), $selectallactions);
                    $actiondata .= $this->output->render($selectall) . ' / ';
        
                    $deselectallactions = new component_action('click',"deselect_all_in", array('div',null,'tablecontainer'));
                    $deselectall = new action_link($selecturl, get_string('deselectall'), $deselectallactions);
                    $actiondata .= $this->output->render($deselectall);
                    //below john fixed
                    $actiondata .= html_writer::tag('label', ' ' . get_string('withselected', 'choice') . ' ', array('for'=>'menuaction'));
        
                    $actionurl = new moodle_url('/mod/choice/view.php', array('sesskey'=>sesskey(), 'action'=>'delete_confirmation()'));
                    $select = new single_select($actionurl, 'action', array('delete'=>get_string('delete')), null, array(''=>get_string('moveselectedusersto', 'choice')), 'attemptsform');
        
                    $actiondata .= $this->output->render($select);
                }
                $html .= html_writer::tag('div', $actiondata, array('class'=>'responseaction'));
        
                if ($choices->viewresponsecapability) {
                    $html .= html_writer::end_tag('form');
                }
        
                return $html;
            }
        }
        

        So what is causing the Error that Joseph is getting?

        If it is not a 'big' is it that the renderers.php is written wrongly?

        Show
        Mary Evans added a comment - Thanks Tim for your input. Sorry for going over this all again, it's just that I want to make absolutely sure you are not assuming the code I added earlier was the actual code that was in 'mymobile/renderers.php', whereas in actual fact it was just my interpretation of what I thought it should be written as, at least the top section of that renderers.php. But looking through the whole of mymobile/renderers.php again I'm seeing a different view of things. If you look at theme/mymobile/renderers.php in Moodle_24_STABLE https://github.com/moodle/moodle/blob/MOODLE_24_STABLE/theme/mymobile/renderers.php you will see that it starts off with... /** * A custom renderer for the mymobile theme to produce snippets of content. * * @ package theme * @subpackage mymobile * @copyright John Stabinger * @license http: //www.gnu.org/copyleft/gpl.html GNU GPL v3 or later */ include_once ($CFG->dirroot. '/mod/choice/renderer.php'); class theme_mymobile_renderer extends plugin_renderer_base { /** * Produces the settings tree * * @param settings_navigation $navigation * @ return string */ public function settings_tree(settings_navigation $navigation) { $content = $ this ->navigation_node($navigation, array('class' => 'settings')); if (has_capability('moodle/site:config', context_system::instance())) { // TODO: Work out whether something is missing from here. } return $content; } /** * Produces the navigation tree * * @param global_navigation $navigation * @ return string */ public function navigation_tree(global_navigation $navigation) { return $ this ->navigation_node($navigation, array()); } /** * Protected method to render a navigaiton node * * @param navigation_node $node * @param array $attrs * @ return type */ protected function navigation_node(navigation_node $node, $attrs = array()) { $items = $node->children; // exit if empty, we don't want an empty ul element if ($items->count() == 0) { return ''; } // array of nested li elements $lis = array(); foreach ($items as $item) { if (!$item->display) { continue ; } $isbranch = ($item->children->count() > 0 || $item->nodetype == navigation_node::NODETYPE_BRANCH); $item->hideicon = true ; $content = $ this ->output->render($item); $content .= $ this ->navigation_node($item); if ($isbranch && !(is_string($item->action) || empty($item->action))) { $content = html_writer::tag('li', $content, array('data-role' => 'list-divider', 'class' => (string)$item->key)); } else if ($isbranch) { $content = html_writer::tag('li', $content, array('data-role' => 'list-divider')); } else { $content = html_writer::tag('li', $content, array('class' => (string)$item->text)); } $lis[] = $content; } if (!count($lis)) { return ''; } return implode( "\n" , $lis); } } And then continues with more renderers which extend the core renderer... class theme_mymobile_core_renderer extends core_renderer { ... And then goes on to extend the mod/choice/renderer...again! class theme_mymobile_mod_choice_renderer extends mod_choice_renderer { /** * Returns HTML to display choices of option * @param object $options * @param int $coursemoduleid * @param bool $vertical * @ return string */ public function display_options($options, $coursemoduleid, $vertical = false ) { $layoutclass = 'horizontal'; if ($vertical) { $layoutclass = 'vertical'; } $target = new moodle_url('/mod/choice/view.php'); //changed below to post from target john $attributes = array('method'=>'POST', 'action'=>$target, 'class'=> $layoutclass); $html = html_writer::start_tag('form', $attributes); $html .= html_writer::start_tag('ul', array('class'=>'choices', 'data-role'=>'controlgroup' )); $availableoption = count($options['options']); foreach ($options['options'] as $option) { $html .= html_writer::start_tag('li', array('class'=>'option')); $option->attributes->name = 'answer'; $option->attributes->type = 'radio'; $option->attributes->id = 'answer'.html_writer::random_id(); $labeltext = $option->text; if (!empty($option->attributes->disabled)) { $labeltext .= ' ' . get_string('full', 'choice'); $availableoption--; } $html .= html_writer::empty_tag('input', (array)$option->attributes); $html .= html_writer::tag('label', $labeltext, array(' for '=>$option->attributes->id)); $html .= html_writer::end_tag('li'); } $html .= html_writer::tag('li','', array('class'=>'clearfloat')); $html .= html_writer::end_tag('ul'); $html .= html_writer::tag('div', '', array('class'=>'clearfloat')); $html .= html_writer::empty_tag('input', array('type'=>'hidden', 'name'=>'sesskey', 'value'=>sesskey())); $html .= html_writer::empty_tag('input', array('type'=>'hidden', 'name'=>'id', 'value'=>$coursemoduleid)); if (!empty($options['hascapability']) && ($options['hascapability'])) { if ($availableoption < 1) { $html .= html_writer::tag('label', get_string('choicefull', 'choice')); } else { $html .= html_writer::empty_tag('input', array('type'=>'submit', 'value'=>get_string('savemychoice','choice'), 'class'=>'button')); } if (!empty($options['allowupdate']) && ($options['allowupdate'])) { $url = new moodle_url('view.php', array('id'=>$coursemoduleid, 'action'=>'delchoice', 'sesskey'=>sesskey())); $html .= html_writer::link($url, get_string('removemychoice','choice')); } } else { $html .= html_writer::tag('label', get_string('havetologin', 'choice')); } $html .= html_writer::end_tag('ul'); $html .= html_writer::end_tag('form'); return $html; } /** * Returns HTML to display choices result * * TODO: There are differences between this method and the mod choice renderers function. * This needs to be checked VERY careful as the minor changes look like they * may lead to regressions. * * @param object $choices * @param bool $forcepublish * @ return string */ public function display_publish_name_vertical($choices) { $html =''; $html .= html_writer::tag('h2',format_string(get_string( "responses" , "choice" )), array('class'=>'main')); $attributes = array('method'=>'POST'); $attributes['action'] = new moodle_url('/mod/choice/view.php'); $attributes['id'] = 'attemptsform'; if ($choices->viewresponsecapability) { $html .= html_writer::start_tag('form', $attributes); $html .= html_writer::empty_tag('input', array('type'=>'hidden', 'name'=>'id', 'value'=> $choices->coursemoduleid)); $html .= html_writer::empty_tag('input', array('type'=>'hidden', 'name'=>'sesskey', 'value'=> sesskey())); $html .= html_writer::empty_tag('input', array('type'=>'hidden', 'name'=>'mode', 'value'=>'overview')); } $table = new html_table(); $table->cellpadding = 0; $table->cellspacing = 0; $table->attributes['class'] = 'results names '; $table->tablealign = 'center'; $table->data = array(); $count = 0; ksort($choices->options); $columns = array(); foreach ($choices->options as $optionid => $options) { $coldata = ''; if ($choices->showunanswered && $optionid == 0) { $coldata .= html_writer::tag('div', format_string(get_string('notanswered', 'choice')), array('class'=>'option')); } else if ($optionid > 0) { $coldata .= html_writer::tag('div', format_string($choices->options[$optionid]->text), array('class'=>'option')); } $numberofuser = 0; if (!empty($options->user) && count($options->user) > 0) { $numberofuser = count($options->user); } $coldata .= html_writer::tag('div', ' ('.$numberofuser. ')', array('class'=>'numberofuser', 'title' => get_string('numberofuser', 'choice'))); $columns[] = $coldata; } $table->head = $columns; $coldata = ''; $columns = array(); foreach ($choices->options as $optionid => $options) { $coldata = ''; if ($choices->showunanswered || $optionid > 0) { if (!empty($options->user)) { foreach ($options->user as $user) { $data = ''; if (empty($user->imagealt)){ $user->imagealt = ''; } if ($choices->viewresponsecapability && $choices->deleterepsonsecapability && $optionid > 0) { $attemptaction = html_writer::checkbox('attemptid[]', $user->id,''); $data .= html_writer::tag('div', $attemptaction, array('class'=>'attemptaction')); } $userimage = $ this ->output->user_picture($user, array('courseid'=>$choices->courseid)); $data .= html_writer::tag('div', $userimage, array('class'=>'image')); $userlink = new moodle_url('/user/view.php', array('id'=>$user->id,'course'=>$choices->courseid)); $name = html_writer::tag('a', fullname($user, $choices->fullnamecapability), array('href'=>$userlink, 'class'=>'username')); $data .= html_writer::tag('div', $name, array('class'=>'fullname')); $data .= html_writer::tag('div','', array('class'=>'clearfloat')); $coldata .= html_writer::tag('div', $data, array('class'=>'user')); } } } $columns[] = $coldata; $count++; } $table->data[] = $columns; foreach ($columns as $d) { $table->colclasses[] = 'data'; } $html .= html_writer::tag('div', html_writer::table($table), array('class'=>'response')); $actiondata = ''; if ($choices->viewresponsecapability && $choices->deleterepsonsecapability) { $selecturl = new moodle_url('#'); $selectallactions = new component_action('click', "select_all_in" , array('div', null ,'tablecontainer')); $selectall = new action_link($selecturl, get_string('selectall'), $selectallactions); $actiondata .= $ this ->output->render($selectall) . ' / '; $deselectallactions = new component_action('click', "deselect_all_in" , array('div', null ,'tablecontainer')); $deselectall = new action_link($selecturl, get_string('deselectall'), $deselectallactions); $actiondata .= $ this ->output->render($deselectall); //below john fixed $actiondata .= html_writer::tag('label', ' ' . get_string('withselected', 'choice') . ' ', array(' for '=>'menuaction')); $actionurl = new moodle_url('/mod/choice/view.php', array('sesskey'=>sesskey(), 'action'=>'delete_confirmation()')); $select = new single_select($actionurl, 'action', array('delete'=>get_string('delete')), null , array(''=>get_string('moveselectedusersto', 'choice')), 'attemptsform'); $actiondata .= $ this ->output->render($select); } $html .= html_writer::tag('div', $actiondata, array('class'=>'responseaction')); if ($choices->viewresponsecapability) { $html .= html_writer::end_tag('form'); } return $html; } } So what is causing the Error that Joseph is getting? If it is not a 'big' is it that the renderers.php is written wrongly?
        Hide
        Tim Hunt added a comment -

        The bit class theme_mymobile_renderer extends plugin_renderer_base is nothing to do with mod/choice.

        Only the bit class theme_mymobile_mod_choice_renderer extends mod_choice_renderer { has anything to do with mod/choice. You could move the require_once down to just before that class if you wanted, but it is geenrally better to do all require_once calls at the top of the file.

        Joseph's error is caused beacuse Joseph deleted mod/choice, but theme/mymoodle assumes that it is present. We could argue about who is wrong there.

        Show
        Tim Hunt added a comment - The bit class theme_mymobile_renderer extends plugin_renderer_base is nothing to do with mod/choice. Only the bit class theme_mymobile_mod_choice_renderer extends mod_choice_renderer { has anything to do with mod/choice. You could move the require_once down to just before that class if you wanted, but it is geenrally better to do all require_once calls at the top of the file. Joseph's error is caused beacuse Joseph deleted mod/choice, but theme/mymoodle assumes that it is present. We could argue about who is wrong there.
        Hide
        Mary Evans added a comment -

        @ Joseph, it looks like I missed the vital clue to whole of this. The fact you deleted the mod/choice is why you got the ERROR. How did I miss that?

        The fog is clearing and I can see better now.

        I'm still confused about the use of 'include' though.

        Show
        Mary Evans added a comment - @ Joseph, it looks like I missed the vital clue to whole of this. The fact you deleted the mod/choice is why you got the ERROR. How did I miss that? The fog is clearing and I can see better now. I'm still confused about the use of 'include' though.
        Hide
        Mary Evans added a comment -

        Thanks for that Tim.

        OK...just so I have this correct.

        If I use require_once instead of include_once (as it is currently written) plus add the condition as suggested will this be OK?

        Show
        Mary Evans added a comment - Thanks for that Tim. OK...just so I have this correct. If I use require_once instead of include_once (as it is currently written) plus add the condition as suggested will this be OK?
        Hide
        Joseph Rézeau added a comment -

        Tim: If you adopt that pattern, then you could change it to

        $choice = get_plugin_dir('mod', 'choice');
        if (file_exists($choice . '/renderer.php')) {
            require_once($CFG->dirroot . '/theme/mymobile/renderers/mod_choice_renderer.php');
        }
        

        then /theme/mymobile/renderers/mod_choice_renderer.php would do the require_once of /mod/choice/renderer.php.

        Yes, that would make sense. Much better than rely on the existence of a module renderer outside of the /theme/ folder.
        To come back to the initial triggering of this "bug", I'm sure there are a number of Moodle sites out there which have opted for a "light" installation, removing a number of core modules that they never use. I mean actually removing those modules folders from their server, not just disabling them in admin. That is what I did.

        Show
        Joseph Rézeau added a comment - Tim: If you adopt that pattern, then you could change it to $choice = get_plugin_dir('mod', 'choice'); if (file_exists($choice . '/renderer.php')) { require_once($CFG->dirroot . '/theme/mymobile/renderers/mod_choice_renderer.php'); } then /theme/mymobile/renderers/mod_choice_renderer.php would do the require_once of /mod/choice/renderer.php. Yes, that would make sense. Much better than rely on the existence of a module renderer outside of the /theme/ folder. To come back to the initial triggering of this "bug", I'm sure there are a number of Moodle sites out there which have opted for a "light" installation, removing a number of core modules that they never use. I mean actually removing those modules folders from their server, not just disabling them in admin. That is what I did.
        Hide
        Mary Evans added a comment -

        Moodle does not understand get_plugin_dir()

        ( ! ) Fatal error: Call to undefined function get_plugin_dir() in C:\wamp\www\moodle24\theme\mymobile\renderers.php on line 803
        Call Stack
        #	Time	Memory	Function	Location
        1	0.0019	43526648	{main}( )	..\index.php:0
        2	1.6568	107431776	theme_config::load( )	..\index.php:159
        3	1.6585	107463256	theme_config->__construct( )	..\outputlib.php:362
        4	1.6622	107907856	include_once( 'C:\wamp\www\moodle24\theme\mymobile\renderers.php' )	..\outputlib.php:451 
        Show
        Mary Evans added a comment - Moodle does not understand get_plugin_dir() ( ! ) Fatal error: Call to undefined function get_plugin_dir() in C:\wamp\www\moodle24\theme\mymobile\renderers.php on line 803 Call Stack # Time Memory Function Location 1 0.0019 43526648 {main}( ) ..\index.php:0 2 1.6568 107431776 theme_config::load( ) ..\index.php:159 3 1.6585 107463256 theme_config->__construct( ) ..\outputlib.php:362 4 1.6622 107907856 include_once( 'C:\wamp\www\moodle24\theme\mymobile\renderers.php' ) ..\outputlib.php:451
        Hide
        Tim Hunt added a comment -

        Well, there is a function wiht a name like that somewhere in moodlelib.php. get_plugin_directory

        Show
        Tim Hunt added a comment - Well, there is a function wiht a name like that somewhere in moodlelib.php. get_plugin_directory
        Hide
        Mary Evans added a comment -

        That did it!
        Yipee!

        Show
        Mary Evans added a comment - That did it! Yipee!
        Hide
        Mary Evans added a comment -

        Hi Tim,
        I've just managed to find time to do this. Can you peer review the changes I have made then I can submit for integration review.
        Thanks
        Mary

        Show
        Mary Evans added a comment - Hi Tim, I've just managed to find time to do this. Can you peer review the changes I have made then I can submit for integration review. Thanks Mary
        Hide
        Joseph Rézeau added a comment -

        Hi Mary, I've just tested your changes on master and it works OK for me. Thanks!
        Joseph

        Show
        Joseph Rézeau added a comment - Hi Mary, I've just tested your changes on master and it works OK for me. Thanks! Joseph
        Hide
        Tim Hunt added a comment -

        +1 nice work. Please submit for integration when you have written testing instructions and cherry-picked to stable branches.

        Show
        Tim Hunt added a comment - +1 nice work. Please submit for integration when you have written testing instructions and cherry-picked to stable branches.
        Hide
        David Scotson added a comment -

        I hate making performance related comments without being very sure of myself, but is checking for the existence of a a bunch of files on every page access a good idea?

        I'm just making suggestions off the top of my head here but perhaps Moodle could enforce the directory layout and naming structure that the OU and myself seem to have settled on independantly? Then moodle would only call mod_forumng_renderer.php if that module was installed and switched on? (I think currently there some stuff in Moodle, like the CSS combiner that includes files even from switched off modules maybe that could be changed to check for somethin being turned on too?.)

        Or the existance could be checked once and then remembered (so you'd get errors if you removed a mod from a running site)?

        Show
        David Scotson added a comment - I hate making performance related comments without being very sure of myself, but is checking for the existence of a a bunch of files on every page access a good idea? I'm just making suggestions off the top of my head here but perhaps Moodle could enforce the directory layout and naming structure that the OU and myself seem to have settled on independantly? Then moodle would only call mod_forumng_renderer.php if that module was installed and switched on? (I think currently there some stuff in Moodle, like the CSS combiner that includes files even from switched off modules maybe that could be changed to check for somethin being turned on too?.) Or the existance could be checked once and then remembered (so you'd get errors if you removed a mod from a running site)?
        Hide
        Tim Hunt added a comment -

        David,

        1. Actually, with a PHP op-code cache, this is actually very, very fast. We do this sort of thing in lots of other places in Moodle. (Look at the code that build the navigation. It does this for every activity in a course, not just one.)

        2. Your idea of changing the core renderer factory (or just making a new one theme_overridden_renderer_factory_using_new_improved_directory_layout - that is not a serious naming suggestion) would be a good idea.

        Show
        Tim Hunt added a comment - David, 1. Actually, with a PHP op-code cache, this is actually very, very fast. We do this sort of thing in lots of other places in Moodle. (Look at the code that build the navigation. It does this for every activity in a course, not just one.) 2. Your idea of changing the core renderer factory (or just making a new one theme_overridden_renderer_factory_using_new_improved_directory_layout - that is not a serious naming suggestion) would be a good idea.
        Hide
        Mary Evans added a comment -

        Thanks Tim...will do.

        Show
        Mary Evans added a comment - Thanks Tim...will do.
        Hide
        Mary Evans added a comment -

        @ Joseph Rézeau

        Can you suggest some Test Instructions? How I might write these may not be the best way, as I was thinking of just renaming the mod/choice/renderer.php to renderer-old.php. Would that be OK. It worked for me but not sure it's the correct thing to do?

        Thanks
        Mary

        Show
        Mary Evans added a comment - @ Joseph Rézeau Can you suggest some Test Instructions? How I might write these may not be the best way, as I was thinking of just renaming the mod/choice/renderer.php to renderer-old.php. Would that be OK. It worked for me but not sure it's the correct thing to do? Thanks Mary
        Hide
        Joseph Rézeau added a comment -

        @Mary,
        What I did to test your mod is:
        A. Before applying your mod
        1.- Site administration ► Plugins ► Activity modules ► Manage activities: Delete the Choice module
        2.- Remove yourmoodle/mod/choice folder
        3.- Backup a couple of resources or activities without users from a course.
        4.- Restore that backup
        5.- You get the error message: Warning: include_once(moodle/mod/choice/renderer.php) [function.include-once]: failed to open stream: No such file or directory in moodle/theme/mymobile/renderers.php on line 35

        B.- Apply your mod
        same scenario: check you do not get error message upon restoring course.

        Hope that helps,
        Joseph

        Show
        Joseph Rézeau added a comment - @Mary, What I did to test your mod is: A. Before applying your mod 1.- Site administration ► Plugins ► Activity modules ► Manage activities: Delete the Choice module 2.- Remove yourmoodle/mod/choice folder 3.- Backup a couple of resources or activities without users from a course. 4.- Restore that backup 5.- You get the error message: Warning: include_once(moodle/mod/choice/renderer.php) [function.include-once] : failed to open stream: No such file or directory in moodle/theme/mymobile/renderers.php on line 35 B.- Apply your mod same scenario: check you do not get error message upon restoring course. Hope that helps, Joseph
        Hide
        Mary Evans added a comment -

        Many thanks Joseph

        Show
        Mary Evans added a comment - Many thanks Joseph
        Hide
        Eloy Lafuente (stronk7) added a comment -

        Integrated (23, 24 & master), thanks!

        Show
        Eloy Lafuente (stronk7) added a comment - Integrated (23, 24 & master), thanks!
        Hide
        Rajesh Taneja added a comment -

        Thanks for fixing this Mary,

        Works as mentioned.
        No error was thrown, while selecting/changing to MyMobile theme

        Show
        Rajesh Taneja added a comment - Thanks for fixing this Mary, Works as mentioned. No error was thrown, while selecting/changing to MyMobile theme
        Hide
        Mary Evans added a comment -

        I learnt something fixing this!
        Thanks Rajesh for testing it

        Show
        Mary Evans added a comment - I learnt something fixing this! Thanks Rajesh for testing it
        Hide
        Damyon Wiese added a comment -

        Congratulations! This issue has been resolved. Thanks for helping to make Moodle better for everyone!

        Regards, Damyon

        Show
        Damyon Wiese added a comment - Congratulations! This issue has been resolved. Thanks for helping to make Moodle better for everyone! Regards, Damyon

          People

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

            Dates

            • Created:
              Updated:
              Resolved: