From 8afba50b84d28a84c224595c621d4e3ffc51dc1e Mon Sep 17 00:00:00 2001 From: Petr Skoda Date: Sun, 17 Jan 2010 10:54:13 +0000 Subject: [PATCH] MDL-21233 get_query_string() is definitely not recommended in new code, instead use moodle_url constructors with proper parameters; removed some more legacy url concatenation operations; realized out_action is not needed often, because we shoudl pass arounf moodle_url instances instead of old url strings --- lib/blocklib.php | 2 +- lib/listlib.php | 16 ++++++++++------ lib/questionlib.php | 10 +++++----- lib/weblib.php | 27 ++++----------------------- mod/quiz/editlib.php | 14 ++++++++------ mod/quiz/tabs.php | 2 +- question/category_class.php | 30 ++++++++++++------------------ question/contextmove.php | 7 ++++--- question/editlib.php | 21 +++++++++++---------- question/export.php | 2 +- question/import.php | 3 ++- question/tabs.php | 2 +- 12 files changed, 60 insertions(+), 76 deletions(-) diff --git a/lib/blocklib.php b/lib/blocklib.php index 5d1bcacd881..ce4727911f4 100644 --- a/lib/blocklib.php +++ b/lib/blocklib.php @@ -1420,7 +1420,7 @@ function block_add_block_ui($page, $output) { } asort($menu, SORT_LOCALE_STRING); - $actionurl = $page->url->out_action(); + $actionurl = new moodle_url($page->url, array('sesskey'=>sesskey())); $select = html_select::make_popup_form($actionurl, 'bui_addblock', $menu, 'add_block'); $select->nothinglabel = get_string('adddots'); diff --git a/lib/listlib.php b/lib/listlib.php index b133da203c9..7facc7b87a8 100644 --- a/lib/listlib.php +++ b/lib/listlib.php @@ -577,26 +577,30 @@ class list_item { } else { $action = $strmoveleft; } - $this->icons['left'] = $this->image_icon($action, $this->parentlist->pageurl->out_action(array('left'=>$this->id)), 'left'); + $url = new moodle_url($this->parentlist->pageurl, (array('sesskey'=>sesskey(), 'left'=>$this->id))); + $this->icons['left'] = $this->image_icon($action, $url, 'left'); } else { $this->icons['left'] = $this->image_spacer(); } if (!$first) { - $this->icons['up'] = $this->image_icon($strmoveup, $this->parentlist->pageurl->out_action(array('moveup'=>$this->id)), 'up'); + $url = new moodle_url($this->parentlist->pageurl, (array('sesskey'=>sesskey(), 'moveup'=>$this->id))); + $this->icons['up'] = $this->image_icon($strmoveup, $url, 'up'); } else { $this->icons['up'] = $this->image_spacer(); } if (!$last) { - $this->icons['down'] = $this->image_icon($strmovedown, $this->parentlist->pageurl->out_action(array('movedown'=>$this->id)), 'down'); + $url = new moodle_url($this->parentlist->pageurl, (array('sesskey'=>sesskey(), 'movedown'=>$this->id))); + $this->icons['down'] = $this->image_icon($strmovedown, $url, 'down'); } else { $this->icons['down'] = $this->image_spacer(); } if (!empty($lastitem)) { $makechildof = get_string('makechildof', 'question', $lastitem->name); - $this->icons['right'] = $this->image_icon($makechildof, $this->parentlist->pageurl->out_action(array('right'=>$this->id)), 'right'); + $url = new moodle_url($this->parentlist->pageurl, (array('sesskey'=>sesskey(), 'right'=>$this->id))); + $this->icons['right'] = $this->image_icon($makechildof, $url, 'right'); } else { $this->icons['right'] = $this->image_spacer(); } @@ -604,8 +608,8 @@ class list_item { function image_icon($action, $url, $icon) { global $OUTPUT; - return ' - ' . $action. ' '; + return ' + ' . s($action). ' '; } function image_spacer() { diff --git a/lib/questionlib.php b/lib/questionlib.php index c5103b3b4c9..a31fd8718fc 100644 --- a/lib/questionlib.php +++ b/lib/questionlib.php @@ -874,13 +874,13 @@ function question_move_questions_to_category($questionids, $newcategory) { * @param question_edit_contexts $contexts object representing contexts available from this context * @param string $querystring to append to urls * */ -function questionbank_navigation_tabs(&$row, $contexts, $querystring) { +function questionbank_navigation_tabs(&$row, $contexts, array $params) { global $CFG, $QUESTION_EDITTABCAPS; $tabs = array( - 'questions' =>array("$CFG->wwwroot/question/edit.php?$querystring", get_string('questions', 'quiz'), get_string('editquestions', 'quiz')), - 'categories' =>array("$CFG->wwwroot/question/category.php?$querystring", get_string('categories', 'quiz'), get_string('editqcats', 'quiz')), - 'import' =>array("$CFG->wwwroot/question/import.php?$querystring", get_string('import', 'quiz'), get_string('importquestions', 'quiz')), - 'export' =>array("$CFG->wwwroot/question/export.php?$querystring", get_string('export', 'quiz'), get_string('exportquestions', 'quiz'))); + 'questions' =>array(new moodle_url('/question/edit.php', $params), get_string('questions', 'quiz'), get_string('editquestions', 'quiz')), + 'categories' =>array(new moodle_url('/question/category.php', $params), get_string('categories', 'quiz'), get_string('editqcats', 'quiz')), + 'import' =>array(new moodle_url('/question/import.php', $params), get_string('import', 'quiz'), get_string('importquestions', 'quiz')), + 'export' =>array(new moodle_url('/question/export.php', $params), get_string('export', 'quiz'), get_string('exportquestions', 'quiz'))); foreach ($tabs as $tabname => $tabparams){ if ($contexts->have_one_edit_tab_cap($tabname)) { $row[] = new tabobject($tabname, $tabparams[0], $tabparams[1], $tabparams[2]); diff --git a/lib/weblib.php b/lib/weblib.php index 5b480c0fa5f..0e3a4ea5e84 100644 --- a/lib/weblib.php +++ b/lib/weblib.php @@ -249,11 +249,6 @@ function qualified_me() { * - output the url without any get params * - and output the params as hidden fields to be output within a form * - * One important usage note is that data passed to methods out, out_action, get_query_string and - * hidden_params_out affect what is returned by the function and do not change the data stored in the object. - * This is to help with typical usage of these objects where one object is used to output urls - * in many places in a page. - * * @link http://docs.moodle.org/en/Development:lib/weblib.php_moodle_url See short write up here * @license http://www.gnu.org/copyleft/gpl.html GNU GPL v3 or later * @package moodlecore @@ -467,13 +462,14 @@ class moodle_url { /** * Get the params as as a query string. + * This method should not be used outside of this method. * + * @param boolean $escaped Use & as params separator instead of plain & * @param array $overrideparams params to add to the output params, these * override existing ones with the same name. - * @param boolean $escaped Use & as params separator instead of plain & * @return string query string that can be added to a url. */ - public function get_query_string(array $overrideparams = null, $escaped = true) { + public function get_query_string($escaped = true, array $overrideparams = null) { $arr = array(); $params = $this->merge_overrideparams($overrideparams); foreach ($params as $key => $val) { @@ -511,7 +507,7 @@ class moodle_url { $uri = $this->out_omit_querystring(); - $querystring = $this->get_query_string($overrideparams, $escaped); + $querystring = $this->get_query_string($escaped, $overrideparams); if ($querystring) { $uri .= '?' . $querystring; } @@ -535,21 +531,6 @@ class moodle_url { return $uri; } - /** - * Output action url with sesskey - * - * Adds sesskey and overriderparams then calls {@link out()} - * @see out() - * - * @param array $overrideparams Allows you to override params - * @return string url - */ - public function out_action(array $overrideparams = null) { - $overrideparams = (array)$overrideparams; - $overrideparams = array('sesskey'=> sesskey()) + $overrideparams; - return $this->out(true, $overrideparams); - } - /** * Compares this moodle_url with another * See documentation of constants for an explanation of the comparison flags. diff --git a/mod/quiz/editlib.php b/mod/quiz/editlib.php index e0e58a9cafc..1c6de4ac09e 100644 --- a/mod/quiz/editlib.php +++ b/mod/quiz/editlib.php @@ -430,7 +430,7 @@ function quiz_print_question_list($quiz, $pageurl, $allowdelete = true, if ($allowdelete && !$quiz->questionsperpage) { echo '
'; echo '' . $strremove . ''; echo '
'; @@ -491,7 +491,7 @@ function quiz_print_question_list($quiz, $pageurl, $allowdelete = true, $upbuttonclass = 'upwithoutdown'; } echo "out_action(array('up' => $question->id)) . "\">out(true, array('up' => $question->id, 'sesskey'=>sesskey())) . "\">pix_url('t/up') . "\" class=\"iconsmall $upbuttonclass\" alt=\"$strmoveup\" />"; } @@ -500,7 +500,7 @@ function quiz_print_question_list($quiz, $pageurl, $allowdelete = true, if ($count < $lastindex - 1) { if (!$hasattempts) { echo "out_action(array('down' => $question->id)) . "\">out(true, array('down' => $question->id, 'sesskey'=>sesskey())) . "\">pix_url('t/down') . "\" class=\"iconsmall\"" . " alt=\"$strmovedown\" />"; } @@ -509,7 +509,7 @@ function quiz_print_question_list($quiz, $pageurl, $allowdelete = true, // remove from quiz, not question delete. if (!$hasattempts) { echo "out_action(array('remove' => $question->id)) . "\"> + $pageurl->out(true, array('remove' => $question->id, 'sesskey'=>sesskey())) . "\"> pix_url('t/delete') . "\" " . "class=\"iconsmall\" alt=\"$strremove\" />"; } @@ -1048,8 +1048,10 @@ class quiz_question_bank_view extends question_bank_view { public function add_to_quiz_url($questionid) { global $CFG; - return $CFG->wwwroot . '/mod/quiz/edit.php?' . $this->baseurl->get_query_string() . - '&addquestion=' . $questionid . '&sesskey=' . sesskey(); + $params = $this->baseurl->params(); + $params['addquestion'] = $questionid; + $params['sesskey'] = sesskey(); + return new moodle_url('/mod/quiz/edit.php', $params); } public function display($tabname, $page, $perpage, $sortorder, diff --git a/mod/quiz/tabs.php b/mod/quiz/tabs.php index b05f0b2bf4b..18cbfbfe673 100644 --- a/mod/quiz/tabs.php +++ b/mod/quiz/tabs.php @@ -86,7 +86,7 @@ if ($currenttab == 'edit' and isset($mode)) { $row[] = new tabobject('editq', "$CFG->wwwroot/mod/quiz/edit.php?cmid=$cm->id", $stredit, $streditingquiz); $row[] = new tabobject('reorder', "$CFG->wwwroot/mod/quiz/edit.php?reordertool=1&cmid=$cm->id", get_string('orderandpaging','quiz'), $streditingquiz); } - //questionbank_navigation_tabs($row, $contexts, $thispageurl->get_query_string()); + //questionbank_navigation_tabs($row, $contexts, $thispageurl->params()); $tabs[] = $row; } diff --git a/question/category_class.php b/question/category_class.php index e18ec95ec58..f217c212351 100644 --- a/question/category_class.php +++ b/question/category_class.php @@ -52,8 +52,8 @@ class question_category_list extends moodle_list { $totop = 1; } $toparent = "0,{$this->context->id}"; - redirect($CFG->wwwroot.'/question/contextmove.php?'. - $this->pageurl->get_query_string(compact('cattomove', 'totop', 'toparent'))); + $url = new moodle_url('/question/contextmove.php?', $this->pageurl->params() + compact('cattomove', 'totop', 'toparent')); + redirect($url); } } } @@ -64,23 +64,19 @@ class question_category_list_item extends list_item { public function set_icon_html($first, $last, &$lastitem){ global $CFG; $category = $this->item; - $this->icons['edit']= $this->image_icon(get_string('editthiscategory', 'question'), - "{$CFG->wwwroot}/question/category.php?".$this->parentlist->pageurl->get_query_string(array('edit'=>$category->id)), 'edit'); + $url = new moodle_url('/question/category.php', ($this->parentlist->pageurl->params() + array('edit'=>$category->id))); + $this->icons['edit']= $this->image_icon(get_string('editthiscategory', 'question'), $url, 'edit'); parent::set_icon_html($first, $last, $lastitem); $toplevel = ($this->parentlist->parentitem === null);//this is a top level item if (($this->parentlist->nextlist !== null) && $last && $toplevel && (count($this->parentlist->items)>1)){ + $url = new moodle_url($this->parentlist->pageurl, array('movedowncontext'=>$this->id, 'tocontext'=>$this->parentlist->nextlist->context->id, 'sesskey'=>sesskey())); $this->icons['down'] = $this->image_icon( - get_string('shareincontext', 'question', print_context_name($this->parentlist->nextlist->context)), - $this->parentlist->pageurl->out_action( - array('movedowncontext'=>$this->id, 'tocontext'=>$this->parentlist->nextlist->context->id) - ), 'down'); + get_string('shareincontext', 'question', print_context_name($this->parentlist->nextlist->context)), $url, 'down'); } if (($this->parentlist->lastlist !== null) && $first && $toplevel && (count($this->parentlist->items)>1)){ + $url = new moodle_url($this->parentlist->pageurl, array('moveupcontext'=>$this->id, 'tocontext'=>$this->parentlist->lastlist->context->id, 'sesskey'=>sesskey())); $this->icons['up'] = $this->image_icon( - get_string('shareincontext', 'question', print_context_name($this->parentlist->lastlist->context)), - $this->parentlist->pageurl->out_action( - array('moveupcontext'=>$this->id, 'tocontext'=>$this->parentlist->lastlist->context->id) - ), 'up'); + get_string('shareincontext', 'question', print_context_name($this->parentlist->lastlist->context)), $url, 'up'); } } public function item_html($extraargs = array()){ @@ -91,15 +87,14 @@ class question_category_list_item extends list_item { $editqestions = get_string('editquestions', 'quiz'); /// Each section adds html to be displayed as part of this list item - $questionbankurl = "{$CFG->wwwroot}/question/edit.php?". - $this->parentlist->pageurl->get_query_string(array('category'=>"$category->id,$category->contextid")); + $questionbankurl = new moodle_url("/question/edit.php", ($this->parentlist->pageurl->params() + array('category'=>"$category->id,$category->contextid"))); $catediturl = $this->parentlist->pageurl->out(true, array('edit'=>$this->id)); $item = "edit}\" href=\"$catediturl\">".$category->name ." ".'('.$category->questioncount.')'; $item .= ' '. $category->info; if (count($this->parentlist->records)!=1){ // don't allow delete if this is the last category in this context. - $item .= ' + $item .= ' ' .$str->delete. ''; } @@ -475,9 +470,8 @@ class question_category_object { if ($oldcat->contextid == $tocontextid) { // not moving contexts redirect($this->pageurl); } else { - redirect($CFG->wwwroot.'/question/contextmove.php?' . - $this->pageurl->get_query_string(array( - 'cattomove' => $updateid, 'toparent'=>$newparent))); + $url = new moodle_url('/question/contextmove.php', ($this->pageurl->params() + array('cattomove' => $updateid, 'toparent'=>$newparent))); + redirect($url); } } diff --git a/question/contextmove.php b/question/contextmove.php index f7bd51583e9..71e96b27172 100644 --- a/question/contextmove.php +++ b/question/contextmove.php @@ -22,7 +22,7 @@ - $onerrorurl = $CFG->wwwroot.'/question/category.php?'.$thispageurl->get_query_string(); + $onerrorurl = new moodle_url('/question/category.php', $thispageurl->params()); list($toparent, $contextto) = explode(',', $toparent); if (!empty($toparent)){//not top level category, make it a child of $toparent if (!$toparent = $DB->get_record('question_categories', array('id' => $toparent))){ @@ -91,7 +91,7 @@ 'fromcoursefilesid', 'tocoursefilesid')); if ($contextmoveform->is_cancelled()){ $thispageurl->remove_params('cattomove', 'toparent', 'totop'); - redirect($CFG->wwwroot."/question/category.php?".$thispageurl->get_query_string()); + redirect(new moodle_url("/question/category.php".$thispageurl->params())); }elseif ($moveformdata = $contextmoveform->get_data()) { if (isset($moveformdata->urls) && is_array($moveformdata->urls)){ check_dir_exists($CFG->dataroot."/$tocoursefilesid/", true); @@ -183,7 +183,8 @@ //finally set the new parent id $DB->update_record("question_categories", $cat); $thispageurl->remove_params('cattomove', 'toparent', 'totop'); - redirect($CFG->wwwroot."/question/category.php?".$thispageurl->get_query_string(array('cat'=>"{$cattomove->id},{$contextto->id}"))); + $url = new moodle_url('/question/category.php', ($thispageurl->params() + array('cat'=>"{$cattomove->id},{$contextto->id}"))); + redirect($url); } $streditingcategories = get_string('editcategories', 'quiz'); diff --git a/question/editlib.php b/question/editlib.php index b0c036d3310..9ffed886d2f 100644 --- a/question/editlib.php +++ b/question/editlib.php @@ -680,10 +680,11 @@ class question_bank_delete_action_column extends question_bank_action_column_bas protected function display_content($question, $rowclasses) { if (question_has_capability_on($question, 'edit')) { if ($question->hidden) { - $this->print_icon('t/restore', $this->strrestore, $this->qbank->base_url()->out_action(array('unhide' => $question->id))); + $url = new moodle_url($this->qbank->base_url(), array('unhide' => $question->id, 'sesskey'=>sesskey())); + $this->print_icon('t/restore', $this->strrestore, $url); } else { - $this->print_icon('t/delete', $this->strdelete, - $this->qbank->base_url()->out_action(array('deleteselected' => $question->id, 'q' . $question->id => 1))); + $url = new moodle_url($this->qbank->base_url(), array('deleteselected' => $question->id, 'q' . $question->id => 1, 'sesskey'=>sesskey())); + $this->print_icon('t/delete', $this->strdelete, $url); } } } @@ -1298,9 +1299,11 @@ class question_bank_view { echo $OUTPUT->paging_bar($pagingbar); if ($totalnumber > DEFAULT_QUESTIONS_PER_PAGE) { if ($perpage == DEFAULT_QUESTIONS_PER_PAGE) { - $showall = ''.get_string('showall', 'moodle', $totalnumber).''; + $url = new moodle_url('edit.php', ($pageurl->params()+array('qperpage'=>1000))); + $showall = ''.get_string('showall', 'moodle', $totalnumber).''; } else { - $showall = ''.get_string('showperpage', 'moodle', DEFAULT_QUESTIONS_PER_PAGE).''; + $url = new moodle_url('edit.php', ($pageurl->params()+array('qperpage'=>DEFAULT_QUESTIONS_PER_PAGE))); + $showall = ''.get_string('showperpage', 'moodle', DEFAULT_QUESTIONS_PER_PAGE).''; } echo "
$showall
"; } @@ -1509,12 +1512,10 @@ class question_bank_view { if ($inuse) { $questionnames .= '
'.get_string('questionsinuse', 'quiz'); } - $baseurl = new moodle_url('edit.php'); - $r = $baseurl->params($this->baseurl->params()); + $baseurl = new moodle_url('edit.php', $this->baseurl->params()); + $deleteurl = new moodle_url($baseurl, array('deleteselected'=>$questionlist, 'confirm'=>md5($questionlist), 'sesskey'=>sesskey())); - echo $OUTPUT->confirm(get_string("deletequestionscheck", "quiz", $questionnames), - $baseurl->out_action(array('deleteselected'=>$questionlist, 'confirm'=>md5($questionlist))), - $baseurl); + echo $OUTPUT->confirm(get_string("deletequestionscheck", "quiz", $questionnames), $deleteurl, $baseurl); return true; } diff --git a/question/export.php b/question/export.php index b5cc0ec26f9..9e1a65f0aba 100644 --- a/question/export.php +++ b/question/export.php @@ -113,7 +113,7 @@ $PAGE->requires->js_function_call('document.location.replace', array($efile))->after_delay(1); } - echo $OUTPUT->continue_button('edit.php?' . $thispageurl->get_query_string()); + echo $OUTPUT->continue_button(new moodle_url('edit.php', $thispageurl->params())); echo $OUTPUT->footer(); exit; } diff --git a/question/import.php b/question/import.php index 5fbc8ba2c92..90f203ec58c 100644 --- a/question/import.php +++ b/question/import.php @@ -145,7 +145,8 @@ } echo "
"; - echo $OUTPUT->continue_button("edit.php?".($thispageurl->get_query_string(array('category'=>"{$qformat->category->id},{$qformat->category->contextid}")))); + $params = $thispageurl->params() + array('category'=>"{$qformat->category->id},{$qformat->category->contextid}"); + echo $OUTPUT->continue_button(new moodle_url('edit.php', $params)); echo $OUTPUT->footer(); exit; } diff --git a/question/tabs.php b/question/tabs.php index 8941622f208..50012e0b287 100644 --- a/question/tabs.php +++ b/question/tabs.php @@ -18,7 +18,7 @@ $tabs = array(); $inactive = array(); $row = array(); - questionbank_navigation_tabs($row, $contexts, $thispageurl->get_query_string()); + questionbank_navigation_tabs($row, $contexts, $thispageurl->params()); $tabs[] = $row; print_tabs($tabs, $currenttab, array());