From e37acc67735f9f1da8682b00b1eff2e11e9d091c Mon Sep 17 00:00:00 2001 From: Tim Hunt Date: Sat, 28 Mar 2020 22:12:30 +0000 Subject: [PATCH] MDL-62487 quiz manual grading: clean coding style and add PHPdocs --- mod/quiz/report/grading/report.php | 108 +++++++++++++++++++++-------- 1 file changed, 78 insertions(+), 30 deletions(-) diff --git a/mod/quiz/report/grading/report.php b/mod/quiz/report/grading/report.php index 39c0c407d77..eb376fc4701 100644 --- a/mod/quiz/report/grading/report.php +++ b/mod/quiz/report/grading/report.php @@ -42,14 +42,32 @@ class quiz_grading_report extends quiz_default_report { const DEFAULT_PAGE_SIZE = 5; const DEFAULT_ORDER = 'random'; - protected $viewoptions = array(); + /** @var array URL parameters for what is being displayed when grading. */ + protected $viewoptions = []; + + /** @var int the current group, 0 if none, or NO_GROUPS_ALLOWED. */ + protected $currentgroup; + + /** @var array from quiz_report_get_significant_questions. */ protected $questions; + + /** @var stdClass the course settings. */ + protected $course; + + /** @var stdClass the course_module settings. */ protected $cm; + + /** @var stdClass the quiz settings. */ protected $quiz; + + /** @var context the quiz context. */ protected $context; - /** @var renderer_base Renderer of Quiz Grading. */ - private $renderer; + /** @var quiz_grading_renderer Renderer of Quiz Grading. */ + protected $renderer; + + /** @var string fragment of SQL code to restrict to the relevant users. */ + protected $userssql; public function display($quiz, $cm, $course) { @@ -70,7 +88,7 @@ class quiz_grading_report extends quiz_default_report { $page = optional_param('page', 0, PARAM_INT); $order = optional_param('order', self::DEFAULT_ORDER, PARAM_ALPHA); - // Assemble the options requried to reload this page. + // Assemble the options required to reload this page. $optparams = array('includeauto', 'page'); foreach ($optparams as $param) { if ($$param) { @@ -85,7 +103,7 @@ class quiz_grading_report extends quiz_default_report { } // Check permissions. - $this->context = context_module::instance($cm->id); + $this->context = context_module::instance($this->cm->id); require_capability('mod/quiz:grade', $this->context); $shownames = has_capability('quiz/grading:viewstudentnames', $this->context); $showidnumbers = has_capability('quiz/grading:viewidnumber', $this->context); @@ -124,7 +142,7 @@ class quiz_grading_report extends quiz_default_report { array('mod/quiz:reviewmyattempts', 'mod/quiz:attempt'), $this->currentgroup); } - $hasquestions = quiz_has_questions($quiz->id); + $hasquestions = quiz_has_questions($this->quiz->id); $counts = null; if ($slot && $hasquestions) { // Make sure there is something to do. @@ -158,6 +176,11 @@ class quiz_grading_report extends quiz_default_report { return true; } + /** + * Get the JOIN conditions needed so we only show attempts by relevant users. + * + * @return qubaid_join + */ protected function get_qubaids_condition() { $where = "quiza.quiz = :mangrquizid AND @@ -183,6 +206,12 @@ class quiz_grading_report extends quiz_default_report { return new qubaid_join("{quiz_attempts} quiza $usersjoin ", 'quiza.uniqueid', $where, $params); } + /** + * Load the quiz_attempts rows corresponding to a list of question_usage ids. + * + * @param int[] $qubaids the question_usage ids of the quiz_attempts to load. + * @return array quiz attempts, with added user name fields. + */ protected function load_attempts_by_usage_ids($qubaids) { global $DB; @@ -208,20 +237,20 @@ class quiz_grading_report extends quiz_default_report { /** * Get the URL of the front page of the report that lists all the questions. - * @param $includeauto if not given, use the current setting, otherwise, - * force a paricular value of includeauto in the URL. - * @return string the URL. + * + * @return moodle_url the URL. */ protected function base_url() { return new moodle_url('/mod/quiz/report.php', - array('id' => $this->cm->id, 'mode' => 'grading')); + ['id' => $this->cm->id, 'mode' => 'grading']); } /** * Get the URL of the front page of the report that lists all the questions. - * @param $includeauto if not given, use the current setting, otherwise, - * force a paricular value of includeauto in the URL. - * @return string the URL. + * + * @param bool $includeauto if not given, use the current setting, otherwise, + * force a particular value of includeauto in the URL. + * @return moodle_url the URL. */ protected function list_questions_url($includeauto = null) { $url = $this->base_url(); @@ -236,18 +265,20 @@ class quiz_grading_report extends quiz_default_report { } /** + * Get the URL to grade a batch of question attempts. + * * @param int $slot * @param int $questionid * @param string $grade - * @param mixed $page = true, link to current page. false = omit page. + * @param int|bool $page = true, link to current page. false = omit page. * number = link to specific page. + * @return moodle_url */ protected function grade_question_url($slot, $questionid, $grade, $page = true) { $url = $this->base_url(); - $url->params(array('slot' => $slot, 'qid' => $questionid, 'grade' => $grade)); + $url->params(['slot' => $slot, 'qid' => $questionid, 'grade' => $grade]); $url->params($this->viewoptions); - $options = $this->viewoptions; if (!$page) { $url->remove_params('page'); } else if (is_integer($page)) { @@ -257,6 +288,14 @@ class quiz_grading_report extends quiz_default_report { return $url; } + /** + * Renders the contents of one cell of the table on the index view. + * + * @param stdClass $counts counts of different types of attempt for this slot. + * @param string $type the type of count to format. + * @param string $gradestring get_string identifier for the grading link text, if required. + * @return string HTML. + */ protected function format_count_for_table($counts, $type, $gradestring) { $result = $counts->$type; if ($counts->$type > 0) { @@ -281,7 +320,7 @@ class quiz_grading_report extends quiz_default_report { $linktext = get_string('alsoshowautomaticallygraded', 'quiz_grading'); } $output .= $this->renderer->render_display_index_heading($linktext, $this->list_questions_url(!$includeauto)); - $data = array(); + $data = []; $header = []; $header[] = get_string('qno', 'quiz_grading'); @@ -302,7 +341,7 @@ class quiz_grading_report extends quiz_default_report { continue; } - $row = array(); + $row = []; $row[] = $this->questions[$counts->slot]->number; @@ -339,13 +378,13 @@ class quiz_grading_report extends quiz_default_report { $attempts = $this->load_attempts_by_usage_ids($qubaids); // Prepare the form. - $hidden = array( + $hidden = [ 'id' => $this->cm->id, 'mode' => 'grading', 'slot' => $slot, 'qid' => $questionid, 'page' => $page, - ); + ]; if (array_key_exists('includeauto', $this->viewoptions)) { $hidden['includeauto'] = $this->viewoptions['includeauto']; } @@ -413,9 +452,15 @@ class quiz_grading_report extends quiz_default_report { $hiddeninputs, $gradequestioncontent ); + return $output; } + /** + * When saving a grading page, are all the submitted marks valid? + * + * @return bool true if all valid, else false. + */ protected function validate_submitted_marks() { $qubaids = optional_param('qubaids', null, PARAM_SEQUENCE); @@ -426,7 +471,7 @@ class quiz_grading_report extends quiz_default_report { $slots = optional_param('slots', '', PARAM_SEQUENCE); if (!$slots) { - $slots = array(); + $slots = []; } else { $slots = explode(',', $slots); } @@ -442,6 +487,9 @@ class quiz_grading_report extends quiz_default_report { return true; } + /** + * Save all submitted marks to the database. + */ protected function process_submitted_data() { global $DB; @@ -454,7 +502,7 @@ class quiz_grading_report extends quiz_default_report { $qubaids = clean_param_array(explode(',', $qubaids), PARAM_INT); $attempts = $this->load_attempts_by_usage_ids($qubaids); - $events = array(); + $events = []; $transaction = $DB->start_delegated_transaction(); foreach ($qubaids as $qubaid) { @@ -463,16 +511,16 @@ class quiz_grading_report extends quiz_default_report { $attemptobj->process_submitted_actions(time()); // Add the event we will trigger later. - $params = array( + $params = [ 'objectid' => $attemptobj->get_question_attempt($assumedslotforevents)->get_question()->id, 'courseid' => $attemptobj->get_courseid(), 'context' => context_module::instance($attemptobj->get_cmid()), - 'other' => array( + 'other' => [ 'quizid' => $attemptobj->get_quizid(), 'attemptid' => $attemptobj->get_attemptid(), - 'slot' => $assumedslotforevents - ) - ); + 'slot' => $assumedslotforevents, + ], + ]; $events[] = \mod_quiz\event\question_manually_graded::create($params); } $transaction->allow_commit(); @@ -516,10 +564,10 @@ class quiz_grading_report extends quiz_default_report { * @param int $page implements paging of the results. * Ignored if $orderby = random or $pagesize is null. * @param int $pagesize implements paging of the results. null = all. + * @return array with two elements, an array of usage ids, and a count of the total number. */ protected function get_usage_ids_where_question_in_state($summarystate, $slot, $questionid = null, $orderby = 'random', $page = 0, $pagesize = null) { - global $CFG, $DB; $dm = new question_engine_data_mapper(); if ($pagesize && $orderby != 'random') { @@ -530,7 +578,7 @@ class quiz_grading_report extends quiz_default_report { $qubaids = $this->get_qubaids_condition(); - $params = array(); + $params = []; if ($orderby == 'date') { list($statetest, $params) = $dm->in_summary_state_test( 'manuallygraded', false, 'mangrstate'); @@ -543,7 +591,7 @@ class quiz_grading_report extends quiz_default_report { } else if ($orderby == 'studentfirstname' || $orderby == 'studentlastname' || $orderby == 'idnumber') { $qubaids->from .= " JOIN {user} u ON quiza.userid = u.id "; // For name sorting, map orderby form value to - // actual column names; 'idnumber' maps naturally + // actual column names; 'idnumber' maps naturally. switch ($orderby) { case "studentlastname": $orderby = "u.lastname, u.firstname";