diff --git a/classes/question/bank/studentquiz_bank_view.php b/classes/question/bank/studentquiz_bank_view.php index 605b26fc..1742f14a 100755 --- a/classes/question/bank/studentquiz_bank_view.php +++ b/classes/question/bank/studentquiz_bank_view.php @@ -545,7 +545,71 @@ private function set_filter_form_fields($anonymize = true) { * but the moodle filter form can only process POST, so we need to copy them there. */ private function set_filter_post_data() { - $_POST = $_GET; + $params = []; + $paramdata = ['mform_isexpanded_id_filtertab' => optional_param('mform_isexpanded_id_filtertab', null, PARAM_TEXT), + 'cmid' => optional_param('cmid', null, PARAM_TEXT), + 'id' => optional_param('id', null, PARAM_TEXT), + 'qperpage' => optional_param('qperpage', null, PARAM_TEXT), + 'sesskey' => optional_param('sesskey', null, PARAM_TEXT), + '_qf__mod_studentquiz_question_bank_filter_form' => + optional_param('_qf__mod_studentquiz_question_bank_filter_form', null, PARAM_TEXT), + 'mform_showmore_id_filtertab' => optional_param('mform_showmore_id_filtertab', null, PARAM_TEXT), + 'onlynew' => optional_param('onlynew', null, PARAM_TEXT), + 'only_new_state' => optional_param('only_new_state', null, PARAM_TEXT), + 'only_approved_state' => optional_param('only_approved_state', null, PARAM_TEXT), + 'only_disapproved_state' => optional_param('only_disapproved_state', null, PARAM_TEXT), + 'only_changed_state' => optional_param('only_changed_state', null, PARAM_TEXT), + 'only_reviewable_state' => optional_param('only_reviewable_state', null, PARAM_TEXT), + 'onlygood' => optional_param('onlygood', null, PARAM_TEXT), + 'onlymine' => optional_param('onlymine', null, PARAM_TEXT), + 'onlydifficultforme' => optional_param('onlydifficultforme', null, PARAM_TEXT), + 'onlydifficult' => optional_param('onlydifficult', null, PARAM_TEXT), + 'tagarray_op' => optional_param('tagarray_op', null, PARAM_TEXT), + 'tagarray' => optional_param('tagarray', null, PARAM_TEXT), + 'state' => optional_param('state', null, PARAM_TEXT), + 'publiccomment_op' => optional_param('publiccomment_op', null, PARAM_TEXT), + 'publiccomment' => optional_param('publiccomment', null, PARAM_TEXT), + 'name_op' => optional_param('name_op', null, PARAM_TEXT), + 'name' => optional_param('name', null, PARAM_TEXT), + 'questiontext_op' => optional_param('questiontext_op', null, PARAM_TEXT), + 'questiontext' => optional_param('questiontext', null, PARAM_TEXT), + 'firstname_op' => optional_param('firstname_op', null, PARAM_TEXT), + 'firstname' => optional_param('firstname', null, PARAM_TEXT), + 'lastname_op' => optional_param('lastname_op', null, PARAM_TEXT), + 'lastname' => optional_param('lastname', null, PARAM_TEXT), + 'timecreated_sdt' => optional_param_array('timecreated_sdt', null, PARAM_TEXT), + 'timecreated_edt' => optional_param_array('timecreated_edt', null, PARAM_TEXT), + 'lastanswercorrect' => optional_param('lastanswercorrect', null, PARAM_TEXT), + 'myrate_op' => optional_param('myrate_op', null, PARAM_TEXT), + 'myrate' => optional_param('myrate', null, PARAM_TEXT), + 'rate' => optional_param('rate', null, PARAM_TEXT), + 'myattempts_op' => optional_param('myattempts_op', null, PARAM_TEXT), + 'myattempts' => optional_param('myattempts', null, PARAM_TEXT), + 'mydifficulty_op' => optional_param('mydifficulty_op', null, PARAM_TEXT), + 'mydifficulty' => optional_param('mydifficulty', null, PARAM_TEXT), + 'difficultylevel_op' => optional_param('difficultylevel_op', null, PARAM_TEXT), + 'difficultylevel' => optional_param('difficultylevel', null, PARAM_TEXT), + 'rate_op' => optional_param('rate_op', null, PARAM_TEXT), + 'submitbutton' => optional_param('submitbutton', null, PARAM_TEXT), + 'cat' => optional_param('cat', null, PARAM_TEXT), + 'startquiz' => optional_param('startquiz', null, PARAM_TEXT), + 'useFilter' => optional_param('useFilter', false, PARAM_BOOL), + ]; + + // Remove redundant data. + foreach ($paramdata as $key => $param) { + if (!is_null($param)) { + $params[$key] = $param; + } + } + + if (optional_param('submitbutton', null, PARAM_TEXT) === 'Filter' + && !optional_param('useFilter', false, PARAM_BOOL)) { + $params['useFilter'] = true; + $params = \mod_studentquiz\utils::flat_url_data($params); + $url = new \moodle_url('/mod/studentquiz/view.php', $params); + redirect($url); + } } /** @@ -566,7 +630,8 @@ private function initialize_filter_form($pageurl) { $this->filterform = new \mod_studentquiz_question_bank_filter_form( $this->fields, $pageurl->out(false), - array_merge(['cmid' => $this->cm->id], $this->pagevars) + array_merge(['cmid' => $this->cm->id], $this->pagevars), + 'get' ); } diff --git a/classes/utils.php b/classes/utils.php index 5b91ba7e..8bcd9089 100644 --- a/classes/utils.php +++ b/classes/utils.php @@ -406,19 +406,24 @@ public static function moodle_version_is(string $operator, string $version): boo /** * We hide 'All participants' option in group mode. It doesn't make sense to display question of all groups together, * and it makes confusing in reports. If the group = 0, NULL or an invalid group, - * we force to chose first available group by default. + * we force to choose first available group by default. * - * @param stdClass $cm Course module class. + * @param \stdClass $cm Course module class. + * @param moodle_url $url The report url. */ - public static function set_default_group($cm) { + public static function set_default_group(\stdClass $cm, moodle_url $url): void { global $USER; $allowedgroups = groups_get_activity_allowed_groups($cm, $USER->id); if ($allowedgroups && !groups_get_activity_group($cm, true, $allowedgroups)) { // Although the UI show that the first group is selected, the param 'group' is not set, // so the groups_get_activity_group() will return wrong value. We have to set it in $_GET to prevent the - // problem when user go to the student quiz in the first time. - $_GET['group'] = reset($allowedgroups)->id; + // problem when usergroups_get_activity_group go to the student quiz in the first time. + if (!optional_param('group', -1, PARAM_INT)) { + $group = reset($allowedgroups)->id; + $url->param('group', $group); + redirect($url->out()); + } } } @@ -804,4 +809,35 @@ public static function save_rate(\stdClass $data): void { $DB->update_record('studentquiz_rate', $data); } } + + /** + * Flat URL data before set to url. + * Sometimes, users may need to filter questions based on their creation date. + * The URL parameters, stored in an array as timecreated_sdt and timecreated_edt, + * contain values for year, month, day, and enabled. + * Before setting these values as URL parameters, we need to flatten it. + * e.g: + * - With these data input: + * ['timecreated_sdt' => ['day' = '11', 'month' => '11', 'year' => '2023', 'enabled' => '1]] + * - The output will be: + * ['timecreated_sdt[day]' => '11', 'timecreated_sdt[month]' => '11', timecreated_sdt[year]' => '11', 'enabled' => '1']; + * + * @param array $data URL data need to convert. + * @return array Flat parameters. + */ + public static function flat_url_data(array $data): array { + $params = []; + foreach ($data as $key => $value) { + if ($key === 'timecreated_sdt' || $key === 'timecreated_edt') { + foreach ($value as $index => $time) { + $paramname = $key . '[' . $index . ']'; + $params[$paramname] = $time; + } + } else { + $params[$key] = $value; + } + } + + return $params; + } } diff --git a/reportlib.php b/reportlib.php index 624bd11c..3af0f784 100755 --- a/reportlib.php +++ b/reportlib.php @@ -169,8 +169,9 @@ public function get_useractivitystats() { /** * Constructor assuming we already have the necessary data loaded. * @param int $cmid course_module id + * @param string $type Report type view, rank or stat. Default: view. */ - public function __construct($cmid) { + public function __construct(int $cmid, string $type = 'view') { global $DB, $USER; if (!$this->cm = get_coursemodule_from_id('studentquiz', $cmid)) { throw new mod_studentquiz_view_exception($this, 'invalidcoursemodule'); @@ -189,7 +190,19 @@ public function __construct($cmid) { $this->userid = $USER->id; $this->availablequestions = mod_studentquiz_count_questions($cmid); - \mod_studentquiz\utils::set_default_group($this->cm); + switch ($type) { + case 'view': + $url = $this->get_view_url(); + break; + case 'stat': + $url = $this->get_stat_url(); + break; + default: + $url = $this->get_rank_url(); + break; + } + + \mod_studentquiz\utils::set_default_group($this->cm, $url); $this->groupid = groups_get_activity_group($this->cm, true); // Check to see if any roles setup has been changed since we last synced the capabilities. @@ -225,7 +238,15 @@ public function get_rank_url() { * @return array */ public function get_urlview_data() { - return array('cmid' => $this->cm->id); + return ['cmid' => $this->cm->id, 'group' => optional_param('group', -1, PARAM_INT)]; + } + + /** + * Get quiz report url + * @return moodle_url + */ + public function get_view_url() { + return new moodle_url('/mod/studentquiz/view.php', $this->get_urlview_data()); } /** diff --git a/reportrank.php b/reportrank.php index 5b0175b2..be040a91 100644 --- a/reportrank.php +++ b/reportrank.php @@ -31,7 +31,7 @@ $cmid = required_param('cmid', PARAM_INT); } -$report = new mod_studentquiz_report($cmid); +$report = new mod_studentquiz_report($cmid, 'rank'); $cm = $report->get_coursemodule(); require_login($report->get_course(), false, $cm); $context = $report->get_context(); diff --git a/reportstat.php b/reportstat.php index 10903cc5..0efbde72 100644 --- a/reportstat.php +++ b/reportstat.php @@ -31,7 +31,7 @@ $cmid = required_param('cmid', PARAM_INT); } -$report = new mod_studentquiz_report($cmid); +$report = new mod_studentquiz_report($cmid, 'stat'); $cm = $report->get_coursemodule(); require_login($report->get_course(), false, $cm); diff --git a/tests/bank_view_test.php b/tests/bank_view_test.php index ead48a9e..f99a66d5 100644 --- a/tests/bank_view_test.php +++ b/tests/bank_view_test.php @@ -246,6 +246,7 @@ public function test_questionbank_filter_question_name() { $this->set_filter(QUESTION_NAME_FILTER, 'Question 1'); $this->set_filter(QUESTION_NAME_OP_FILTER, '0'); + $this->set_filter('useFilter', true); // Hard coded. $questionbank = $this->run_questionbank(); diff --git a/tests/utils_test.php b/tests/utils_test.php index a132d0f8..9f9b9e9c 100644 --- a/tests/utils_test.php +++ b/tests/utils_test.php @@ -172,4 +172,45 @@ public function test_ensure_question_version_status_is_correct(string $status, b $this->assertEquals(question_version_status::QUESTION_STATUS_READY, $newquestionversion->status); } + /** + * Test flat_url_data function. + * + * @covers \mod_studentquiz\utils::flat_url_data + */ + public function test_flat_url_data(): void { + $this->resetAfterTest(); + $params = [ + 'cmid' => '81', + 'cat' => '533,140', + 'qperpage' => '20', + 'timecreated_sdt' => [ + 'day' => '11', + 'month' => '11', + 'year' => '2022', + 'enabled' => '1', + ], + 'timecreated_edt' => [ + 'day' => '11', + 'month' => '11', + 'year' => '2023', + 'enabled' => '1', + ] + ]; + + $expectedparams = [ + 'cmid' => '81', + 'cat' => '533,140', + 'qperpage' => '20', + 'timecreated_sdt[day]' => '11', + 'timecreated_sdt[month]' => '11', + 'timecreated_sdt[year]' => '2022', + 'timecreated_sdt[enabled]' => '1', + 'timecreated_edt[day]' => '11', + 'timecreated_edt[month]' => '11', + 'timecreated_edt[year]' => '2023', + 'timecreated_edt[enabled]' => '1', + ]; + + $this->assertEquals($expectedparams, utils::flat_url_data($params)); + } } diff --git a/tests/viewlib_test.php b/tests/viewlib_test.php index e6fa404b..8d7476bd 100644 --- a/tests/viewlib_test.php +++ b/tests/viewlib_test.php @@ -55,9 +55,11 @@ protected function setUp(): void { $this->cm = get_coursemodule_from_id('studentquiz', $studentquiz->cmid); $context = \context_module::instance($this->cm->id); + $category = question_get_default_category($context->id); // Some internal moodle functions (e.g. question_edit_setup()) require the cmid to be found in $_xxx['cmid']. $_GET['cmid'] = $this->cm->id; + $_GET['cat'] = $category->id . ',' . $context->id; // Satisfy codechecker: $course $cm $studentquiz $userid. $report = new \mod_studentquiz_report($this->cm->id); diff --git a/view.php b/view.php index c6047694..f5191fe7 100755 --- a/view.php +++ b/view.php @@ -35,18 +35,18 @@ $cmid = required_param('id', PARAM_INT); // Some internal moodle functions (e.g. question_edit_setup()) require the cmid to be found in $_xxx['cmid'], // but moodle allows to view a mod page with parameter id in place of cmid. - $_GET['cmid'] = $cmid; + $url = new moodle_url('/mod/studentquiz/view.php', ['cmid' => $cmid, 'id' => $cmid]); + redirect($url); } // TODO: make course-, context- and login-check in a better starting class (not magically hidden in "report"). // And when doing that, offer course, context and studentquiz object over it, which all following actions can use. -$report = new mod_studentquiz_report($cmid); +$report = new mod_studentquiz_report($cmid, 'view'); require_login($report->get_course(), false, $report->get_coursemodule()); $course = $report->get_course(); $context = $report->get_context(); $cm = $report->get_coursemodule(); - $studentquiz = mod_studentquiz_load_studentquiz($cmid, $context->id); // If for some weired reason a studentquiz is not aggregated yet, now would be a moment to do so. @@ -56,14 +56,14 @@ // Load view. $view = new mod_studentquiz_view($course, $context, $cm, $studentquiz, $USER->id, $report); $baseurl = $view->get_questionbank()->base_url(); - // Redirect if we have received valid data. // Usually we should use submitted_data(), but since we have two forms merged and exchanging their values // using GET params, we can't use that. -if (!empty($_GET)) { + +if (!empty($_REQUEST)) { if (optional_param('startquiz', null, PARAM_BOOL)) { require_sesskey(); - if ($ids = mod_studentquiz_helper_get_ids_by_raw_submit(fix_utf8($_GET))) { + if ($ids = mod_studentquiz_helper_get_ids_by_raw_submit(fix_utf8($_REQUEST))) { if ($attempt = mod_studentquiz_generate_attempt($ids, $studentquiz, $USER->id)) { $questionusage = question_engine::load_questions_usage_by_activity($attempt->questionusageid); $baseurl->remove_params('startquiz'); diff --git a/viewlib.php b/viewlib.php index 43d3da97..6eedb61c 100755 --- a/viewlib.php +++ b/viewlib.php @@ -123,21 +123,21 @@ public function __construct($course, $context, $cm, $studentquiz, $userid, $repo * Loads the question custom bank view. */ private function load_questionbank() { - $_POST['cat'] = $this->get_category_id() . ',' . $this->get_context_id(); - $params = $_GET; + $params = $_REQUEST; + if (!optional_param('cat', null, PARAM_SEQUENCE)) { + $params['cat'] = $this->get_category_id() . ',' . $this->get_context_id(); + $params = \mod_studentquiz\utils::flat_url_data($params); + $url = new \moodle_url('/mod/studentquiz/view.php', $params); + redirect($url); + } // Get edit question link setup. - list($thispageurl, $contexts, $cmid, $cm, $module, $pagevars) + [$thispageurl, $contexts, $cmid, $cm, $module, $pagevars] = question_edit_setup('questions', '/mod/studentquiz/view.php', true); $pagevars['qperpage'] = optional_param('qperpage', \mod_studentquiz\utils::DEFAULT_QUESTIONS_PER_PAGE, PARAM_INT); $pagevars['showall'] = optional_param('showall', false, PARAM_BOOL); $pagevars['cat'] = $this->get_category_id() . ',' . $this->get_context_id(); - $this->pageurl = new moodle_url($thispageurl); - foreach ($params as $key => $value) { - if ($key == 'timecreated_sdt' || $key == 'timecreated_edt') { - $value = http_build_query($value); - } - $thispageurl->param($key, $value); - } + $urlparams = \mod_studentquiz\utils::flat_url_data($params); + $this->pageurl = new moodle_url($thispageurl, $urlparams); // Trigger notification if user got returned from the question edit form. // TODO: Shouldn't this be somewhere outside of load_questionbank(), as this is clearly not relevant for showing the // question bank?