Moodle
  1. Moodle
  2. MDL-40244

Did you remember to call setType() for 'groupinfo'

    Details

    • Testing Instructions:
      Hide
      1. Select Standard theme.
      2. Go to a course where students are in groups.
      3. Add a Wiki page and set to allow groups.
      4. Add a Forum and set to allow groups.
      5. Login as a student that is in a group.
      6. Access the Wiki page and TEST that no errors occur when adding a new page.
      7. Still logged in as a student post a comment in the forum and TEST that no errors occur.
      Show
      Select Standard theme. Go to a course where students are in groups. Add a Wiki page and set to allow groups. Add a Forum and set to allow groups. Login as a student that is in a group. Access the Wiki page and TEST that no errors occur when adding a new page. Still logged in as a student post a comment in the forum and TEST that no errors occur.
    • Affected Branches:
      MOODLE_24_STABLE, MOODLE_25_STABLE
    • Fixed Branches:
      MOODLE_24_STABLE, MOODLE_25_STABLE, MOODLE_26_STABLE
    • Pull Master Branch:
      MDL-40244_M26

      Description

      I created a wiki and set it to separate groups. The admin was doing this and isnt in any group. The first time I student (who is in a group) went to the wiki they also had to create a start page. The following was displayed (/mod/wiki/create.php?wid=1&group=1&uid=0&title=blamo)

      Did you remember to call setType() for 'groupinfo'? Defaulting to PARAM_RAW cleaning.
      line 1303 of /lib/formslib.php: call to debugging()
      line 281 of /lib/formslib.php: call to moodleform->detectMissingSetType()
      line 202 of /lib/formslib.php: call to moodleform->_process_submission()
      line 906 of /mod/wiki/pagelib.php: call to moodleform->moodleform()
      line 105 of /mod/wiki/create.php: call to page_wiki_create->set_action()

        Gliffy Diagrams

          Issue Links

            Activity

            Hide
            Andrew Davis added a comment - - edited

            After the student created the first page they got this.

            Did you remember to call setType() for 'groupinfo'? Defaulting to PARAM_RAW cleaning.
            line 1303 of /lib/formslib.php: call to debugging()
            line 281 of /lib/formslib.php: call to moodleform->detectMissingSetType()
            line 202 of /lib/formslib.php: call to moodleform->_process_submission()
            line 906 of /mod/wiki/pagelib.php: call to moodleform->moodleform()
            line 105 of /mod/wiki/create.php: call to page_wiki_create->set_action()

            This page did not call $PAGE->set_url(...). Using http://localhost/moodle/int/master/mod/wiki/create.php?action=create&wid=1&group=1&uid=0
            line 558 of /lib/pagelib.php: call to debugging()
            line 734 of /lib/pagelib.php: call to moodle_page->magic_get_url()
            line 496 of /mod/wiki/lib.php: call to moodle_page->__get()
            line 2036 of /lib/navigationlib.php: call to wiki_extend_navigation()
            line 1205 of /lib/navigationlib.php: call to global_navigation->load_activity()
            line 225 of /blocks/navigation/block_navigation.php: call to global_navigation->initialise()
            line 181 of /blocks/navigation/block_navigation.php: call to block_navigation->get_navigation()
            line 292 of /blocks/moodleblock.class.php: call to block_navigation->get_content()
            line 238 of /blocks/moodleblock.class.php: call to block_base->formatted_contents()
            line 951 of /lib/blocklib.php: call to block_base->get_content_for_output()
            line 1003 of /lib/blocklib.php: call to block_manager->create_block_contents()
            line 353 of /lib/blocklib.php: call to block_manager->ensure_content_created()
            line 3068 of /lib/outputrenderers.php: call to block_manager->region_has_content()
            line 3104 of /lib/outputrenderers.php: call to core_renderer->body_css_classes()
            line 26 of /theme/clean/layout/embedded.php: call to core_renderer->body_attributes()
            line 847 of /lib/outputrenderers.php: call to include()
            line 777 of /lib/outputrenderers.php: call to core_renderer->render_page_layout()
            line 720 of /lib/outputrenderers.php: call to core_renderer->header()
            line 2563 of /lib/weblib.php: call to core_renderer->redirect_message()
            line 111 of /mod/wiki/create.php: call to redirect()
            id="page-mod-wiki-create" class="format-weeks path-mod path-mod-wiki safari dir-ltr lang-en yui-skin-sam yui3-skin-sam localhost--moodle-int-master pagelayout-redirect course-2 context-46 cmid-15 category-1 has-region-side-pre used-region-side-pre has-region-side-post empty-region-side-post">

            Show
            Andrew Davis added a comment - - edited After the student created the first page they got this. Did you remember to call setType() for 'groupinfo'? Defaulting to PARAM_RAW cleaning. line 1303 of /lib/formslib.php: call to debugging() line 281 of /lib/formslib.php: call to moodleform->detectMissingSetType() line 202 of /lib/formslib.php: call to moodleform->_process_submission() line 906 of /mod/wiki/pagelib.php: call to moodleform->moodleform() line 105 of /mod/wiki/create.php: call to page_wiki_create->set_action() This page did not call $PAGE->set_url(...). Using http://localhost/moodle/int/master/mod/wiki/create.php?action=create&wid=1&group=1&uid=0 line 558 of /lib/pagelib.php: call to debugging() line 734 of /lib/pagelib.php: call to moodle_page->magic_get_url() line 496 of /mod/wiki/lib.php: call to moodle_page->__get() line 2036 of /lib/navigationlib.php: call to wiki_extend_navigation() line 1205 of /lib/navigationlib.php: call to global_navigation->load_activity() line 225 of /blocks/navigation/block_navigation.php: call to global_navigation->initialise() line 181 of /blocks/navigation/block_navigation.php: call to block_navigation->get_navigation() line 292 of /blocks/moodleblock.class.php: call to block_navigation->get_content() line 238 of /blocks/moodleblock.class.php: call to block_base->formatted_contents() line 951 of /lib/blocklib.php: call to block_base->get_content_for_output() line 1003 of /lib/blocklib.php: call to block_manager->create_block_contents() line 353 of /lib/blocklib.php: call to block_manager->ensure_content_created() line 3068 of /lib/outputrenderers.php: call to block_manager->region_has_content() line 3104 of /lib/outputrenderers.php: call to core_renderer->body_css_classes() line 26 of /theme/clean/layout/embedded.php: call to core_renderer->body_attributes() line 847 of /lib/outputrenderers.php: call to include() line 777 of /lib/outputrenderers.php: call to core_renderer->render_page_layout() line 720 of /lib/outputrenderers.php: call to core_renderer->header() line 2563 of /lib/weblib.php: call to core_renderer->redirect_message() line 111 of /mod/wiki/create.php: call to redirect() id="page-mod-wiki-create" class="format-weeks path-mod path-mod-wiki safari dir-ltr lang-en yui-skin-sam yui3-skin-sam localhost--moodle-int-master pagelayout-redirect course-2 context-46 cmid-15 category-1 has-region-side-pre used-region-side-pre has-region-side-post empty-region-side-post">
            Hide
            Mary Evans added a comment -

            I fixed something like this yesterday, and wondering if I dare to fix this in the same way. If I get this one right there will be no holding me back.

            Show
            Mary Evans added a comment - I fixed something like this yesterday, and wondering if I dare to fix this in the same way. If I get this one right there will be no holding me back.
            Hide
            Mary Evans added a comment -

            Just added Rajesh as a watcher.

            Show
            Mary Evans added a comment - Just added Rajesh as a watcher.
            Hide
            Mary Evans added a comment -

            Given this is all about 'groupinfo' and setType() in a form...then looking at mod/wiki/create_form.php I found this bit of code...
             

                    if (!empty($this->_customdata['groups']->availablegroups)) {
                        foreach ($this->_customdata['groups']->availablegroups as $groupdata) {
                            $groupinfo[$groupdata->id] = $groupdata->name;
                        }
                        if (count($groupinfo) > 1) {
                            $mform->addElement('select', 'groupinfo', get_string('group'), $groupinfo);
                            $mform->setDefault('groupinfo', $this->_customdata['groups']->currentgroup);
                        } else {
                            $groupid = key($groupinfo);
                            $groupname = $groupinfo[$groupid];
                            $mform->addElement('static', 'groupdesciption', get_string('group'), $groupname);
                            $mform->addElement('hidden', 'groupinfo', $groupid);                
                        }
                    }
            

            I also found if I added...
             

            $mform->setType('groupinfo', PARAM_TEXT);
            

            just after $mform->addElement('hidden', 'groupinfo', $groupid); it fixed the problem.

            So that the fix would look like this...
             

                    if (!empty($this->_customdata['groups']->availablegroups)) {
                        foreach ($this->_customdata['groups']->availablegroups as $groupdata) {
                            $groupinfo[$groupdata->id] = $groupdata->name;
                        }
                        if (count($groupinfo) > 1) {
                            $mform->addElement('select', 'groupinfo', get_string('group'), $groupinfo);
                            $mform->setDefault('groupinfo', $this->_customdata['groups']->currentgroup);
                        } else {
                            $groupid = key($groupinfo);
                            $groupname = $groupinfo[$groupid];
                            $mform->addElement('static', 'groupdesciption', get_string('group'), $groupname);
                            $mform->addElement('hidden', 'groupinfo', $groupid);
                            $mform->setType('groupinfo', PARAM_TEXT);
                        }
                    }
            

            If this is correct, Rajesh, is it OK if I assign this to myself and fix this one too?

            Show
            Mary Evans added a comment - Given this is all about 'groupinfo' and setType() in a form...then looking at mod/wiki/create_form.php I found this bit of code...   if (!empty($this->_customdata['groups']->availablegroups)) { foreach ($this->_customdata['groups']->availablegroups as $groupdata) { $groupinfo[$groupdata->id] = $groupdata->name; } if (count($groupinfo) > 1) { $mform->addElement('select', 'groupinfo', get_string('group'), $groupinfo); $mform->setDefault('groupinfo', $this->_customdata['groups']->currentgroup); } else { $groupid = key($groupinfo); $groupname = $groupinfo[$groupid]; $mform->addElement('static', 'groupdesciption', get_string('group'), $groupname); $mform->addElement('hidden', 'groupinfo', $groupid); } } I also found if I added...   $mform->setType('groupinfo', PARAM_TEXT); just after $mform->addElement('hidden', 'groupinfo', $groupid); it fixed the problem. So that the fix would look like this...   if (!empty($this->_customdata['groups']->availablegroups)) { foreach ($this->_customdata['groups']->availablegroups as $groupdata) { $groupinfo[$groupdata->id] = $groupdata->name; } if (count($groupinfo) > 1) { $mform->addElement('select', 'groupinfo', get_string('group'), $groupinfo); $mform->setDefault('groupinfo', $this->_customdata['groups']->currentgroup); } else { $groupid = key($groupinfo); $groupname = $groupinfo[$groupid]; $mform->addElement('static', 'groupdesciption', get_string('group'), $groupname); $mform->addElement('hidden', 'groupinfo', $groupid); $mform->setType('groupinfo', PARAM_TEXT); } } If this is correct, Rajesh, is it OK if I assign this to myself and fix this one too?
            Hide
            Rajesh Taneja added a comment -

            Thanks Mary,

            Your approach looks grt.
            Few points to consider:

            1. You might want to put setType in if statement.
              • It's not important to set type for static elements, so you can ignore the current fix added in else.
            2. Similar fix can be applied to
              • mod/forum/classes/post_form.php - Line 170
            Show
            Rajesh Taneja added a comment - Thanks Mary, Your approach looks grt. Few points to consider: You might want to put setType in if statement. It's not important to set type for static elements, so you can ignore the current fix added in else. Similar fix can be applied to mod/forum/classes/post_form.php - Line 170
            Hide
            Mary Evans added a comment -

            Great! Thanks Rajesh
            In that case I'll fix these later today.

            Show
            Mary Evans added a comment - Great! Thanks Rajesh In that case I'll fix these later today.
            Hide
            Mary Evans added a comment -

            Hi Rajesh,

            I found that in 2.5 that mod/forum/classes/post_form.php did not exist but forum/post_form.php did so would you advise me to fix that file for 2.5 branch and also Moodle 2.4 branch or forget it?

            Show
            Mary Evans added a comment - Hi Rajesh, I found that in 2.5 that mod/forum/classes/post_form.php did not exist but forum/post_form.php did so would you advise me to fix that file for 2.5 branch and also Moodle 2.4 branch or forget it?
            Hide
            Rajesh Taneja added a comment -

            Hello Mary,

            for 2.5 and 2.4 this code is in mod/forum/post_form.php

            Show
            Rajesh Taneja added a comment - Hello Mary, for 2.5 and 2.4 this code is in mod/forum/post_form.php
            Hide
            Mary Evans added a comment -

            OK...fixing this now after updating.

            Show
            Mary Evans added a comment - OK...fixing this now after updating.
            Hide
            Mary Evans added a comment -

            Thanks Rajesh, I'm happy with this now so I've set it for Integration Review.

            I hope the TEST instructions are adequate?

            Show
            Mary Evans added a comment - Thanks Rajesh, I'm happy with this now so I've set it for Integration Review. I hope the TEST instructions are adequate?
            Hide
            Rajesh Taneja added a comment -

            Thanks Mary,

            Testing instructions looks fine...

            Show
            Rajesh Taneja added a comment - Thanks Mary, Testing instructions looks fine...
            Hide
            Marina Glancy added a comment -

            it looks to me that it should be PARAM_INT and not PARAM_TEXT

            Show
            Marina Glancy added a comment - it looks to me that it should be PARAM_INT and not PARAM_TEXT
            Hide
            Rajesh Taneja added a comment -

            You are right Marina,
            It should be PARAM_INT.

            Show
            Rajesh Taneja added a comment - You are right Marina, It should be PARAM_INT.
            Hide
            Marina Glancy added a comment -

            ping Mary, can you correct please? PARAM_TEXT -> PARAM_INT

            Show
            Marina Glancy added a comment - ping Mary, can you correct please? PARAM_TEXT -> PARAM_INT
            Hide
            Mary Evans added a comment -

            Hi Marina,
            I put PARAM_TEXT because I assumed it was referring to 'groupinfo' where 'info' is normally text, rather than something like 'groupid' where 'id' would require an integer. Just my crazy logic...sorry.

            Show
            Mary Evans added a comment - Hi Marina, I put PARAM_TEXT because I assumed it was referring to 'groupinfo' where 'info' is normally text, rather than something like 'groupid' where 'id' would require an integer. Just my crazy logic...sorry.
            Hide
            Mary Evans added a comment -

            All done!

            Show
            Mary Evans added a comment - All done!
            Hide
            Marina Glancy added a comment -

            Thanks Mary, integrated in 2.4, 2.5 and 2.6

            I had to squash your commits, three commits to modify just two lines is a little too much. Sorry I did not confirm it with you but did not want to wait another day (blame timezones)

            Show
            Marina Glancy added a comment - Thanks Mary, integrated in 2.4, 2.5 and 2.6 I had to squash your commits, three commits to modify just two lines is a little too much. Sorry I did not confirm it with you but did not want to wait another day (blame timezones)
            Hide
            Mary Evans added a comment -

            Sorry about that Marina, but last time I did a merge/squash during integration Eloy told me off! So was a little weary when you asked me to make those changes.

            Show
            Mary Evans added a comment - Sorry about that Marina, but last time I did a merge/squash during integration Eloy told me off! So was a little weary when you asked me to make those changes.
            Hide
            Anthony Borrow added a comment -

            I tested in 2.5 and did not see any errors when creating the pages. Peace - Anthony

            Show
            Anthony Borrow added a comment - I tested in 2.5 and did not see any errors when creating the pages. Peace - Anthony
            Hide
            Anthony Borrow added a comment -

            no visible errors - passing

            Show
            Anthony Borrow added a comment - no visible errors - passing
            Hide
            Marina Glancy added a comment -

            hehe, it's a little confusing for developers I suppose. When your branch has already been integrated (and the issue changed the status), you can not squash new commits with those that are already integrated. But if we just in the process of reviewing your issue and have not integrated anything you are welcome to squash.

            Show
            Marina Glancy added a comment - hehe, it's a little confusing for developers I suppose. When your branch has already been integrated (and the issue changed the status), you can not squash new commits with those that are already integrated. But if we just in the process of reviewing your issue and have not integrated anything you are welcome to squash.
            Hide
            Mary Evans added a comment -

            Thanks Marina for clarifying the do's and don'ts of merge/squash, which I know how to do, it's just knowing when. Thanks also for teaching me new stuff about renderers...it's been an interesting week's work all in all, what with the PARAMS and setTYPE()stuff.

            Anyway at least this is fixed which is great, and should keep Clean theme in it's place.

            Cheers

            Show
            Mary Evans added a comment - Thanks Marina for clarifying the do's and don'ts of merge/squash, which I know how to do, it's just knowing when. Thanks also for teaching me new stuff about renderers...it's been an interesting week's work all in all, what with the PARAMS and setTYPE()stuff. Anyway at least this is fixed which is great, and should keep Clean theme in it's place. Cheers
            Hide
            Damyon Wiese added a comment -

            Here lies 52 bugs.
            All fixed or swept under a rug.
            If they come back one day,
            To our dismay,
            We all will feel quite un-smug.

            Thanks for the reporting/fixing/testing on this issue. It has been sent upstream.

            Show
            Damyon Wiese added a comment - Here lies 52 bugs. All fixed or swept under a rug. If they come back one day, To our dismay, We all will feel quite un-smug. Thanks for the reporting/fixing/testing on this issue. It has been sent upstream.

              People

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

                Dates

                • Created:
                  Updated:
                  Resolved: