diff --git a/lib/accesslib.php b/lib/accesslib.php index ea059c5d99e..8f2d0abb010 100644 --- a/lib/accesslib.php +++ b/lib/accesslib.php @@ -7279,10 +7279,12 @@ function get_suspended_userids(context $context, $usecache = false) { } /** - * Gets sql for finding users with a capability in the given context + * Gets sql for finding users with capability in the given context * * @param context $context - * @param string $capability + * @param string|array $capability Capability name or array of names. + * If an array is provided then this is the equivalent of a logical 'OR', + * i.e. the user needs to have one of these capabilities. * @return array($sql, $params) */ function get_with_capability_sql(context $context, $capability) { @@ -7301,11 +7303,13 @@ function get_with_capability_sql(context $context, $capability) { } /** - * Gets sql joins for finding users with a capability in the given context + * Gets sql joins for finding users with capability in the given context * - * @param context $context - * @param string $capability - * @param string $useridcolumn e.g. u.id + * @param context $context Context for the join + * @param string|array $capability Capability name or array of names. + * If an array is provided then this is the equivalent of a logical 'OR', + * i.e. the user needs to have one of these capabilities. + * @param string $useridcolumn e.g. 'u.id' * @return \core\dml\sql_join Contains joins, wheres, params */ function get_with_capability_join(context $context, $capability, $useridcolumn) { @@ -7328,14 +7332,15 @@ function get_with_capability_join(context $context, $capability, $useridcolumn) list($contextids, $contextpaths) = get_context_info_list($context); list($incontexts, $cparams) = $DB->get_in_or_equal($contextids, SQL_PARAMS_NAMED, 'ctx'); - $cparams['cap'] = $capability; + + list($incaps, $capsparams) = $DB->get_in_or_equal($capability, SQL_PARAMS_NAMED, 'cap'); $defs = array(); $sql = "SELECT rc.id, rc.roleid, rc.permission, ctx.path FROM {role_capabilities} rc JOIN {context} ctx on rc.contextid = ctx.id - WHERE rc.contextid $incontexts AND rc.capability = :cap"; - $rcs = $DB->get_records_sql($sql, $cparams); + WHERE rc.contextid $incontexts AND rc.capability $incaps"; + $rcs = $DB->get_records_sql($sql, array_merge($cparams, $capsparams)); foreach ($rcs as $rc) { $defs[$rc->path][$rc->roleid] = $rc->permission; } diff --git a/lib/enrollib.php b/lib/enrollib.php index 5c326da91e9..c8ce48edb4f 100644 --- a/lib/enrollib.php +++ b/lib/enrollib.php @@ -1159,7 +1159,9 @@ function is_enrolled(context $context, $user = null, $withcapability = '', $only * * @param context $context * @param string $prefix optional, a prefix to the user id column - * @param string $capability optional, may include a capability name + * @param string|array $capability optional, may include a capability name, or array of names. + * If an array is provided then this is the equivalent of a logical 'OR', + * i.e. the user needs to have one of these capabilities. * @param int $group optional, 0 indicates no current group, otherwise the group id * @param bool $onlyactive consider only active enrolments in enabled plugins and time restrictions * @param bool $onlysuspended inverse of onlyactive, consider only suspended enrolments diff --git a/mod/quiz/report/attemptsreport.php b/mod/quiz/report/attemptsreport.php index 891a12ecd37..4545a7099f5 100644 --- a/mod/quiz/report/attemptsreport.php +++ b/mod/quiz/report/attemptsreport.php @@ -72,21 +72,27 @@ abstract class quiz_attempts_report extends quiz_default_report { * @param object $quiz * @param object $cm * @param object $course + * @return array with four elements: + * 0 => integer the current group id (0 for none). + * 1 => \core\dml\sql_join Contains joins, wheres, params for all the students in this course. + * 2 => \core\dml\sql_join Contains joins, wheres, params for all the students in the current group. + * 3 => \core\dml\sql_join Contains joins, wheres, params for all the students to show in the report. + * Will be the same as either element 1 or 2. */ protected function init($mode, $formclass, $quiz, $cm, $course) { $this->mode = $mode; $this->context = context_module::instance($cm->id); - list($currentgroup, $students, $groupstudents, $allowed) = - $this->load_relevant_students($cm, $course); + list($currentgroup, $studentsjoins, $groupstudentsjoins, $allowedjoins) = $this->get_students_joins( + $cm, $course); $this->qmsubselect = quiz_report_qm_filter_select($quiz); $this->form = new $formclass($this->get_base_url(), array('quiz' => $quiz, 'currentgroup' => $currentgroup, 'context' => $this->context)); - return array($currentgroup, $students, $groupstudents, $allowed); + return array($currentgroup, $studentsjoins, $groupstudentsjoins, $allowedjoins); } /** @@ -99,45 +105,37 @@ abstract class quiz_attempts_report extends quiz_default_report { } /** - * Get information about which students to show in the report. - * @param object $cm the coures module. + * Get sql fragments (joins) which can be used to build queries that + * will select an appropriate set of students to show in the reports. + * + * @param object $cm the course module. * @param object $course the course settings. * @return array with four elements: * 0 => integer the current group id (0 for none). - * 1 => array ids of all the students in this course. - * 2 => array ids of all the students in the current group. - * 3 => array ids of all the students to show in the report. Will be the - * same as either element 1 or 2. + * 1 => \core\dml\sql_join Contains joins, wheres, params for all the students in this course. + * 2 => \core\dml\sql_join Contains joins, wheres, params for all the students in the current group. + * 3 => \core\dml\sql_join Contains joins, wheres, params for all the students to show in the report. + * Will be the same as either element 1 or 2. */ - protected function load_relevant_students($cm, $course = null) { + protected function get_students_joins($cm, $course = null) { $currentgroup = $this->get_current_group($cm, $course, $this->context); + $empty = new \core\dml\sql_join(); if ($currentgroup == self::NO_GROUPS_ALLOWED) { - return array($currentgroup, array(), array(), array()); + return array($currentgroup, $empty, $empty, $empty); } - if (!$students = get_users_by_capability($this->context, - array('mod/quiz:reviewmyattempts', 'mod/quiz:attempt'), - 'u.id, 1', '', '', '', '', '', false)) { - $students = array(); - } else { - $students = array_keys($students); - } + $studentsjoins = get_enrolled_with_capabilities_join($this->context); if (empty($currentgroup)) { - return array($currentgroup, $students, array(), $students); + return array($currentgroup, $studentsjoins, $empty, $studentsjoins); } // We have a currently selected group. - if (!$groupstudents = get_users_by_capability($this->context, - array('mod/quiz:reviewmyattempts', 'mod/quiz:attempt'), - 'u.id, 1', '', '', '', $currentgroup, '', false)) { - $groupstudents = array(); - } else { - $groupstudents = array_keys($groupstudents); - } + $groupstudentsjoins = get_enrolled_with_capabilities_join($this->context, '', + array('mod/quiz:attempt', 'mod/quiz:reviewmyattempts'), $currentgroup); - return array($currentgroup, $students, $groupstudents, $groupstudents); + return array($currentgroup, $studentsjoins, $groupstudentsjoins, $groupstudentsjoins); } /** @@ -281,16 +279,17 @@ abstract class quiz_attempts_report extends quiz_default_report { * @param object $quiz the quiz settings. * @param object $cm the cm object for the quiz. * @param int $currentgroup the currently selected group. - * @param array $groupstudents the students in the current group. - * @param array $allowed the users whose attempt this user is allowed to modify. + * @param \core\dml\sql_join $groupstudentsjoins (joins, wheres, params) the students in the current group. + * @param \core\dml\sql_join $allowedjoins (joins, wheres, params) the users whose attempt this user is allowed to modify. * @param moodle_url $redirecturl where to redircet to after a successful action. */ - protected function process_actions($quiz, $cm, $currentgroup, $groupstudents, $allowed, $redirecturl) { - if (empty($currentgroup) || $groupstudents) { + protected function process_actions($quiz, $cm, $currentgroup, \core\dml\sql_join $groupstudentsjoins, + \core\dml\sql_join $allowedjoins, $redirecturl) { + if (empty($currentgroup) || $this->hasgroupstudents) { if (optional_param('delete', 0, PARAM_BOOL) && confirm_sesskey()) { if ($attemptids = optional_param_array('attemptid', array(), PARAM_INT)) { require_capability('mod/quiz:deleteattempts', $this->context); - $this->delete_selected_attempts($quiz, $cm, $attemptids, $allowed); + $this->delete_selected_attempts($quiz, $cm, $attemptids, $allowedjoins); redirect($redirecturl); } } @@ -303,21 +302,31 @@ abstract class quiz_attempts_report extends quiz_default_report { * this quiz are not deleted. * @param object $cm the course_module object. * @param array $attemptids the list of attempt ids to delete. - * @param array $allowed This list of userids that are visible in the report. + * @param \core\dml\sql_join $allowedjoins (joins, wheres, params) This list of userids that are visible in the report. * Users can only delete attempts that they are allowed to see in the report. * Empty means all users. */ - protected function delete_selected_attempts($quiz, $cm, $attemptids, $allowed) { + protected function delete_selected_attempts($quiz, $cm, $attemptids, \core\dml\sql_join $allowedjoins) { global $DB; foreach ($attemptids as $attemptid) { - $attempt = $DB->get_record('quiz_attempts', array('id' => $attemptid)); - if (!$attempt || $attempt->quiz != $quiz->id || $attempt->preview != 0) { - // Ensure the attempt exists, and belongs to this quiz. If not skip. - continue; + if (empty($allowedjoins->joins)) { + $sql = "SELECT quiza.* + FROM {quiz_attempts} quiza + JOIN {user} u ON u.id = quiza.userid + WHERE quiza.id = :attemptid"; + } else { + $sql = "SELECT quiza.* + FROM {quiz_attempts} quiza + JOIN {user} u ON u.id = quiza.userid + {$allowedjoins->joins} + WHERE {$allowedjoins->wheres} AND quiza.id = :attemptid"; } - if ($allowed && !in_array($attempt->userid, $allowed)) { - // Ensure the attempt belongs to a student included in the report. If not skip. + $params = $allowedjoins->params + array('attemptid' => $attemptid); + $attempt = $DB->get_record_sql($sql, $params); + if (!$attempt || $attempt->quiz != $quiz->id || $attempt->preview != 0) { + // Ensure the attempt exists, belongs to this quiz and belongs to + // a student included in the report. If not skip. continue; } @@ -326,4 +335,21 @@ abstract class quiz_attempts_report extends quiz_default_report { quiz_delete_attempt($attempt, $quiz); } } + + /** + * Get information about which students to show in the report. + * @param object $cm the coures module. + * @param object $course the course settings. + * @return array with four elements: + * 0 => integer the current group id (0 for none). + * 1 => array ids of all the students in this course. + * 2 => array ids of all the students in the current group. + * 3 => array ids of all the students to show in the report. Will be the + * same as either element 1 or 2. + * @deprecated since Moodle 3.2 Please use get_students_joins() instead. + */ + protected function load_relevant_students($cm, $course = null) { + $msg = 'The function load_relevant_students() is deprecated. Please use get_students_joins() instead.'; + throw new coding_exception($msg); + } } diff --git a/mod/quiz/report/attemptsreport_table.php b/mod/quiz/report/attemptsreport_table.php index ab4ecb55292..146f4dae21c 100644 --- a/mod/quiz/report/attemptsreport_table.php +++ b/mod/quiz/report/attemptsreport_table.php @@ -61,11 +61,13 @@ abstract class quiz_attempts_report_table extends table_sql { /** @var object mod_quiz_attempts_report_options the options affecting this report. */ protected $options; - /** @var object the ids of the students in the currently selected group, if applicable. */ - protected $groupstudents; + /** @var \core\dml\sql_join Contains joins, wheres, params to find students + * in the currently selected group, if applicable. + */ + protected $groupstudentsjoins; - /** @var object the ids of the students in the course. */ - protected $students; + /** @var \core\dml\sql_join Contains joins, wheres, params to find the students in the course. */ + protected $studentsjoins; /** @var object the questions that comprise this quiz.. */ protected $questions; @@ -80,20 +82,20 @@ abstract class quiz_attempts_report_table extends table_sql { * @param context $context * @param string $qmsubselect * @param mod_quiz_attempts_report_options $options - * @param array $groupstudents - * @param array $students + * @param \core\dml\sql_join $groupstudentsjoins Contains joins, wheres, params + * @param \core\dml\sql_join $studentsjoins Contains joins, wheres, params * @param array $questions * @param moodle_url $reporturl */ public function __construct($uniqueid, $quiz, $context, $qmsubselect, - mod_quiz_attempts_report_options $options, $groupstudents, $students, + mod_quiz_attempts_report_options $options, \core\dml\sql_join $groupstudentsjoins, \core\dml\sql_join $studentsjoins, $questions, $reporturl) { parent::__construct($uniqueid); $this->quiz = $quiz; $this->context = $context; $this->qmsubselect = $qmsubselect; - $this->groupstudents = $groupstudents; - $this->students = $students; + $this->groupstudentsjoins = $groupstudentsjoins; + $this->studentsjoins = $studentsjoins; $this->questions = $questions; $this->includecheckboxes = $options->checkboxcolumn; $this->reporturl = $reporturl; @@ -380,11 +382,11 @@ abstract class quiz_attempts_report_table extends table_sql { /** * Contruct all the parts of the main database query. - * @param array $reportstudents list if userids of users to include in the report. + * @param \core\dml\sql_join $allowedstudentsjoins (joins, wheres, params) defines allowed users for the report. * @return array with 4 elements ($fields, $from, $where, $params) that can be used to - * build the actual database query. + * build the actual database query. */ - public function base_sql($reportstudents) { + public function base_sql(\core\dml\sql_join $allowedstudentsjoins) { global $DB; $fields = $DB->sql_concat('u.id', "'#'", 'COALESCE(quiza.attempt, 0)') . ' AS uniqueid,'; @@ -419,7 +421,7 @@ abstract class quiz_attempts_report_table extends table_sql { // badly synchronised clocks, and a student does a really quick attempt. // This part is the same for all cases. Join the users and quiz_attempts tables. - $from = "\n{user} u"; + $from = " {user} u"; $from .= "\nLEFT JOIN {quiz_attempts} quiza ON quiza.userid = u.id AND quiza.quiz = :quizid"; $params = array('quizid' => $this->quiz->id); @@ -436,24 +438,21 @@ abstract class quiz_attempts_report_table extends table_sql { break; case quiz_attempts_report::ENROLLED_WITH: // Show only students with attempts. - list($usql, $uparams) = $DB->get_in_or_equal( - $reportstudents, SQL_PARAMS_NAMED, 'u'); - $params += $uparams; - $where = "u.id $usql AND quiza.preview = 0 AND quiza.id IS NOT NULL"; + $from .= "\n" . $allowedstudentsjoins->joins; + $where = "quiza.preview = 0 AND quiza.id IS NOT NULL AND " . $allowedstudentsjoins->wheres; + $params = array_merge($params, $allowedstudentsjoins->params); break; case quiz_attempts_report::ENROLLED_WITHOUT: // Show only students without attempts. - list($usql, $uparams) = $DB->get_in_or_equal( - $reportstudents, SQL_PARAMS_NAMED, 'u'); - $params += $uparams; - $where = "u.id $usql AND quiza.id IS NULL"; + $from .= "\n" . $allowedstudentsjoins->joins; + $where = "quiza.id IS NULL AND " . $allowedstudentsjoins->wheres; + $params = array_merge($params, $allowedstudentsjoins->params); break; case quiz_attempts_report::ENROLLED_ALL: // Show all students with or without attempts. - list($usql, $uparams) = $DB->get_in_or_equal( - $reportstudents, SQL_PARAMS_NAMED, 'u'); - $params += $uparams; - $where = "u.id $usql AND (quiza.preview = 0 OR quiza.preview IS NULL)"; + $from .= "\n" . $allowedstudentsjoins->joins; + $where = "(quiza.preview = 0 OR quiza.preview IS NULL) AND " . $allowedstudentsjoins->wheres; + $params = array_merge($params, $allowedstudentsjoins->params); break; } diff --git a/mod/quiz/report/grading/report.php b/mod/quiz/report/grading/report.php index 3b78821c943..4232ecc1fd6 100644 --- a/mod/quiz/report/grading/report.php +++ b/mod/quiz/report/grading/report.php @@ -49,7 +49,6 @@ class quiz_grading_report extends quiz_default_report { protected $context; public function display($quiz, $cm, $course) { - global $CFG, $DB, $PAGE; $this->quiz = $quiz; $this->cm = $cm; @@ -116,11 +115,10 @@ class quiz_grading_report extends quiz_default_report { // Get the group, and the list of significant users. $this->currentgroup = $this->get_current_group($cm, $course, $this->context); if ($this->currentgroup == self::NO_GROUPS_ALLOWED) { - $this->users = array(); + $this->userssql = array(); } else { - $this->users = get_users_by_capability($this->context, - array('mod/quiz:reviewmyattempts', 'mod/quiz:attempt'), '', '', '', '', - $this->currentgroup, '', false); + $this->userssql = get_enrolled_sql($this->context, + array('mod/quiz:reviewmyattempts', 'mod/quiz:attempt'), $this->currentgroup); } $hasquestions = quiz_has_questions($quiz->id); @@ -158,29 +156,28 @@ class quiz_grading_report extends quiz_default_report { } protected function get_qubaids_condition() { - global $DB; $where = "quiza.quiz = :mangrquizid AND quiza.preview = 0 AND quiza.state = :statefinished"; $params = array('mangrquizid' => $this->cm->instance, 'statefinished' => quiz_attempt::FINISHED); + $usersjoin = ''; $currentgroup = groups_get_activity_group($this->cm, true); + $enrolleduserscount = count_enrolled_users($this->context, + array('mod/quiz:reviewmyattempts', 'mod/quiz:attempt'), $currentgroup); if ($currentgroup) { - $users = get_users_by_capability($this->context, - array('mod/quiz:reviewmyattempts', 'mod/quiz:attempt'), 'u.id, u.id', '', '', '', - $currentgroup, '', false); - if (empty($users)) { + $userssql = get_enrolled_sql($this->context, + array('mod/quiz:reviewmyattempts', 'mod/quiz:attempt'), $currentgroup); + if ($enrolleduserscount < 1) { $where .= ' AND quiza.userid = 0'; } else { - list($usql, $uparam) = $DB->get_in_or_equal(array_keys($users), - SQL_PARAMS_NAMED, 'mangru'); - $where .= ' AND quiza.userid ' . $usql; - $params += $uparam; + $usersjoin = "JOIN ({$userssql[0]}) AS enr ON quiza.userid = enr.id"; + $params += $userssql[1]; } } - return new qubaid_join('{quiz_attempts} quiza', 'quiza.uniqueid', $where, $params); + return new qubaid_join("{quiz_attempts} quiza $usersjoin ", 'quiza.uniqueid', $where, $params); } protected function load_attempts_by_usage_ids($qubaids) { diff --git a/mod/quiz/report/grading/tests/behat/grading.feature b/mod/quiz/report/grading/tests/behat/grading.feature new file mode 100644 index 00000000000..de52a44f6f3 --- /dev/null +++ b/mod/quiz/report/grading/tests/behat/grading.feature @@ -0,0 +1,77 @@ +@mod @mod_quiz @mod_quiz_grading_basic +Feature: Basic use of the Manual grading report + In order to easily find students attempts that need manual grading + As a teacher + I need to use the manual grading report + + @javascript + Scenario: Use the Manual grading report + Given the following "users" exist: + | username | firstname | lastname | email | idnumber | + | teacher1 | T1 | Teacher1 | teacher1@example.com | T1000 | + | student1 | S1 | Student1 | student1@example.com | S1000 ! + And the following "courses" exist: + | fullname | shortname | category | + | Course 1 | C1 | 0 | + And the following "course enrolments" exist: + | user | course | role | + | teacher1 | C1 | editingteacher | + | student1 | C1 | student | + And I log in as "teacher1" + And I am on site homepage + And I follow "Course 1" + And I turn editing mode on + And I add a "Quiz" to section "1" and I fill the form with: + | Name | Quiz 1 | + | Description | Quiz 1 description | + And I add a "Short answer" question to the "Quiz 1" quiz with: + | Question name | Short answer 001 | + | Question text | Where is the capital city of France? | + | Answer 1 | Paris | + | Grade | 100% | + + # Check report shows nothing when there are no attempts. + And I follow "Course 1" + And I follow "Quiz 1" + When I navigate to "Manual grading" node in "Quiz administration > Results" + Then I should see "Manual grading" + And I should see "Quiz 1" + And I should see "Nothing to display" + And I follow "Also show questions that have been graded automatically" + And I should see "Nothing to display" + And I log out + + # Create an attempt. + When I log in as "student1" + And I follow "Course 1" + And I follow "Quiz 1" + And I press "Attempt quiz now" + Then I should see "Question 1" + And I should see "Not yet answered" + And I should see "Where is the capital city of France?" + When I set the field "Answer:" to "Paris" + And I press "Finish attempt ..." + Then I should see "Answer saved" + When I press "Submit all and finish" + And I click on "Submit all and finish" "button" in the "Confirmation" "dialogue" + And I log out + + # Use the manual grading report. + When I log in as "teacher1" + And I follow "Course 1" + And I follow "Quiz 1" + And I navigate to "Manual grading" node in "Quiz administration > Results" + Then I should see "Manual grading" + When I follow "Also show questions that have been graded automatically" + Then I should see "Short answer 001" + And I should see "0" in the ".cell.c2" "css_element" + And I should see "0" in the ".cell.c3" "css_element" + + # Adjust the mark for Student1. + When I click on "update grades" "link" in the "Short answer 001" "table_row" + And I set the field "Comment" to "I have adjusted your mark to 0.6" + And I set the field "Mark" to "0.6" + And I press "Save and go to next page" + Then I should see "All selected attempts have been graded. Returning to the list of questions." + And I should see "0" in the ".cell.c2" "css_element" + And I should see "1" in the ".cell.c3" "css_element" diff --git a/mod/quiz/report/overview/overview_table.php b/mod/quiz/report/overview/overview_table.php index 5f8f0e3eef3..7ff73719a9d 100644 --- a/mod/quiz/report/overview/overview_table.php +++ b/mod/quiz/report/overview/overview_table.php @@ -44,15 +44,16 @@ class quiz_overview_table extends quiz_attempts_report_table { * @param context $context * @param string $qmsubselect * @param quiz_overview_options $options - * @param array $groupstudents - * @param array $students + * @param \core\dml\sql_join $groupstudentsjoins + * @param \core\dml\sql_join $studentsjoins * @param array $questions * @param moodle_url $reporturl */ public function __construct($quiz, $context, $qmsubselect, - quiz_overview_options $options, $groupstudents, $students, $questions, $reporturl) { + quiz_overview_options $options, \core\dml\sql_join $groupstudentsjoins, + \core\dml\sql_join $studentsjoins, $questions, $reporturl) { parent::__construct('mod-quiz-report-overview-report', $quiz , $context, - $qmsubselect, $options, $groupstudents, $students, $questions, $reporturl); + $qmsubselect, $options, $groupstudentsjoins, $studentsjoins, $questions, $reporturl); } public function build_table() { @@ -68,24 +69,39 @@ class quiz_overview_table extends quiz_attempts_report_table { // End of adding the data from attempts. Now add averages at bottom. $this->add_separator(); - if ($this->groupstudents) { - $this->add_average_row(get_string('groupavg', 'grades'), $this->groupstudents); + if (!empty($this->groupstudentsjoins->joins)) { + $sql = "SELECT DISTINCT u.id + FROM {user} u + {$this->groupstudentsjoins->joins} + WHERE {$this->groupstudentsjoins->wheres}"; + $groupstudents = $DB->get_records_sql($sql, $this->groupstudentsjoins->params); + if ($groupstudents) { + $this->add_average_row(get_string('groupavg', 'grades'), $this->groupstudentsjoins); + } } - if ($this->students) { - $this->add_average_row(get_string('overallaverage', 'grades'), $this->students); + if (!empty($this->studentsjoins->joins)) { + $sql = "SELECT DISTINCT u.id + FROM {user} u + {$this->studentsjoins->joins} + WHERE {$this->studentsjoins->wheres}"; + $students = $DB->get_records_sql($sql, $this->studentsjoins->params); + if ($students) { + $this->add_average_row(get_string('overallaverage', 'grades'), $this->studentsjoins); + } } } + /** * Add an average grade over the attempts of a set of users. * @param string $label the title ot use for this row. - * @param array $users the users to average over. + * @param \core\dml\sql_join $usersjoins (joins, wheres, params) for the users to average over. */ - protected function add_average_row($label, $users) { + protected function add_average_row($label, \core\dml\sql_join $usersjoins) { global $DB; - list($fields, $from, $where, $params) = $this->base_sql($users); + list($fields, $from, $where, $params) = $this->base_sql($usersjoins); $record = $DB->get_record_sql(" SELECT AVG(quiza.sumgrades) AS grade, COUNT(quiza.sumgrades) AS numaveraged FROM $from diff --git a/mod/quiz/report/overview/report.php b/mod/quiz/report/overview/report.php index f14a5d1ce78..63d2ae26494 100644 --- a/mod/quiz/report/overview/report.php +++ b/mod/quiz/report/overview/report.php @@ -40,10 +40,11 @@ require_once($CFG->dirroot . '/mod/quiz/report/overview/overview_table.php'); class quiz_overview_report extends quiz_attempts_report { public function display($quiz, $cm, $course) { - global $CFG, $DB, $OUTPUT, $PAGE; + global $DB, $OUTPUT, $PAGE; + + list($currentgroup, $studentsjoins, $groupstudentsjoins, $allowedjoins) = $this->init( + 'overview', 'quiz_overview_settings_form', $quiz, $cm, $course); - list($currentgroup, $students, $groupstudents, $allowed) = - $this->init('overview', 'quiz_overview_settings_form', $quiz, $cm, $course); $options = new quiz_overview_options('overview', $quiz, $cm, $course); if ($fromform = $this->form->get_data()) { @@ -55,13 +56,6 @@ class quiz_overview_report extends quiz_attempts_report { $this->form->set_data($options->get_initial_form_data()); - if ($options->attempts == self::ALL_WITH) { - // This option is only available to users who can access all groups in - // groups mode, so setting allowed to empty (which means all quiz attempts - // are accessible, is not a security porblem. - $allowed = array(); - } - // Load the required questions. $questions = quiz_report_get_significant_questions($quiz); @@ -69,7 +63,7 @@ class quiz_overview_report extends quiz_attempts_report { $courseshortname = format_string($course->shortname, true, array('context' => context_course::instance($course->id))); $table = new quiz_overview_table($quiz, $this->context, $this->qmsubselect, - $options, $groupstudents, $students, $questions, $options->get_url()); + $options, $groupstudentsjoins, $studentsjoins, $questions, $options->get_url()); $filename = quiz_report_download_filename(get_string('overviewfilename', 'quiz_overview'), $courseshortname, $quiz->name); $table->is_downloading($options->download, $filename, @@ -78,8 +72,31 @@ class quiz_overview_report extends quiz_attempts_report { raise_memory_limit(MEMORY_EXTRA); } + $this->hasgroupstudents = false; + if (!empty($groupstudentsjoins->joins)) { + $sql = "SELECT DISTINCT u.id + FROM {user} u + $groupstudentsjoins->joins + WHERE $groupstudentsjoins->wheres"; + $this->hasgroupstudents = $DB->record_exists_sql($sql, $groupstudentsjoins->params); + } + $hasstudents = false; + if (!empty($studentsjoins->joins)) { + $sql = "SELECT DISTINCT u.id + FROM {user} u + $studentsjoins->joins + WHERE $studentsjoins->wheres"; + $hasstudents = $DB->record_exists_sql($sql, $studentsjoins->params); + } + if ($options->attempts == self::ALL_WITH) { + // This option is only available to users who can access all groups in + // groups mode, so setting allowed to empty (which means all quiz attempts + // are accessible, is not a security porblem. + $allowedjoins = new \core\dml\sql_join(); + } + $this->course = $course; // Hack to make this available in process_actions. - $this->process_actions($quiz, $cm, $currentgroup, $groupstudents, $allowed, $options->get_url()); + $this->process_actions($quiz, $cm, $currentgroup, $groupstudentsjoins, $allowedjoins, $options->get_url()); // Start output. if (!$table->is_downloading()) { @@ -106,9 +123,9 @@ class quiz_overview_report extends quiz_attempts_report { if (!$table->is_downloading()) { if (!$hasquestions) { echo quiz_no_questions_message($quiz, $cm, $this->context); - } else if (!$students) { + } else if (!$hasstudents) { echo $OUTPUT->notification(get_string('nostudentsyet')); - } else if ($currentgroup && !$groupstudents) { + } else if ($currentgroup && !$this->hasgroupstudents) { echo $OUTPUT->notification(get_string('nostudentsingroup')); } @@ -116,13 +133,10 @@ class quiz_overview_report extends quiz_attempts_report { $this->form->display(); } - $hasstudents = $students && (!$currentgroup || $groupstudents); + $hasstudents = $hasstudents && (!$currentgroup || $this->hasgroupstudents); if ($hasquestions && ($hasstudents || $options->attempts == self::ALL_WITH)) { // Construct the SQL. - $fields = $DB->sql_concat('u.id', "'#'", 'COALESCE(quiza.attempt, 0)') . - ' AS uniqueid, '; - - list($fields, $from, $where, $params) = $table->base_sql($allowed); + list($fields, $from, $where, $params) = $table->base_sql($allowedjoins); $table->set_count_sql("SELECT COUNT(1) FROM $from WHERE $where", $params); @@ -145,7 +159,7 @@ class quiz_overview_report extends quiz_attempts_report { // Output the regrade buttons. if (has_capability('mod/quiz:regrade', $this->context)) { $regradesneeded = $this->count_question_attempts_needing_regrade( - $quiz, $groupstudents); + $quiz, $groupstudentsjoins); if ($currentgroup) { $a= new stdClass(); $a->groupname = groups_get_group_name($currentgroup); @@ -235,11 +249,14 @@ class quiz_overview_report extends quiz_attempts_report { list($bands, $bandwidth) = self::get_bands_count_and_width($quiz); $labels = self::get_bands_labels($bands, $bandwidth, $quiz); - if ($currentgroup && $groupstudents) { - list($usql, $params) = $DB->get_in_or_equal($groupstudents); - $params[] = $quiz->id; - if ($DB->record_exists_select('quiz_grades', "userid $usql AND quiz = ?", $params)) { - $data = quiz_report_grade_bands($bandwidth, $bands, $quiz->id, $groupstudents); + if ($currentgroup && $this->hasgroupstudents) { + $sql = "SELECT qg.id + FROM {quiz_grades} qg + JOIN {user} u on u.id = qg.userid + {$groupstudentsjoins->joins} + WHERE qg.quiz = $quiz->id AND {$groupstudentsjoins->wheres}"; + if ($DB->record_exists_sql($sql, $groupstudentsjoins->params)) { + $data = quiz_report_grade_bands($bandwidth, $bands, $quiz->id, $groupstudentsjoins); $chart = self::get_chart($labels, $data); $graphname = get_string('overviewreportgraphgroup', 'quiz_overview', groups_get_group_name($currentgroup)); echo $output->chart($chart, $graphname); @@ -247,7 +264,7 @@ class quiz_overview_report extends quiz_attempts_report { } if ($DB->record_exists('quiz_grades', array('quiz'=> $quiz->id))) { - $data = quiz_report_grade_bands($bandwidth, $bands, $quiz->id, []); + $data = quiz_report_grade_bands($bandwidth, $bands, $quiz->id, new \core\dml\sql_join()); $chart = self::get_chart($labels, $data); $graphname = get_string('overviewreportgraph', 'quiz_overview'); echo $output->chart($chart, $graphname); @@ -256,14 +273,25 @@ class quiz_overview_report extends quiz_attempts_report { return true; } - protected function process_actions($quiz, $cm, $currentgroup, $groupstudents, $allowed, $redirecturl) { - parent::process_actions($quiz, $cm, $currentgroup, $groupstudents, $allowed, $redirecturl); + /** + * Extends parent function processing any submitted actions. + * + * @param object $quiz + * @param object $cm + * @param int $currentgroup + * @param \core\dml\sql_join $groupstudentsjoins (joins, wheres, params) + * @param \core\dml\sql_join $allowedjoins (joins, wheres, params) + * @param moodle_url $redirecturl + */ + protected function process_actions($quiz, $cm, $currentgroup, \core\dml\sql_join $groupstudentsjoins, + \core\dml\sql_join $allowedjoins, $redirecturl) { + parent::process_actions($quiz, $cm, $currentgroup, $groupstudentsjoins, $allowedjoins, $redirecturl); - if (empty($currentgroup) || $groupstudents) { + if (empty($currentgroup) || $this->hasgroupstudents) { if (optional_param('regrade', 0, PARAM_BOOL) && confirm_sesskey()) { if ($attemptids = optional_param_array('attemptid', array(), PARAM_INT)) { $this->start_regrade($quiz, $cm); - $this->regrade_attempts($quiz, false, $groupstudents, $attemptids); + $this->regrade_attempts($quiz, false, $groupstudentsjoins, $attemptids); $this->finish_regrade($redirecturl); } } @@ -271,17 +299,17 @@ class quiz_overview_report extends quiz_attempts_report { if (optional_param('regradeall', 0, PARAM_BOOL) && confirm_sesskey()) { $this->start_regrade($quiz, $cm); - $this->regrade_attempts($quiz, false, $groupstudents); + $this->regrade_attempts($quiz, false, $groupstudentsjoins); $this->finish_regrade($redirecturl); } else if (optional_param('regradealldry', 0, PARAM_BOOL) && confirm_sesskey()) { $this->start_regrade($quiz, $cm); - $this->regrade_attempts($quiz, true, $groupstudents); + $this->regrade_attempts($quiz, true, $groupstudentsjoins); $this->finish_regrade($redirecturl); } else if (optional_param('regradealldrydo', 0, PARAM_BOOL) && confirm_sesskey()) { $this->start_regrade($quiz, $cm); - $this->regrade_attempts_needing_it($quiz, $groupstudents); + $this->regrade_attempts_needing_it($quiz, $groupstudentsjoins); $this->finish_regrade($redirecturl); } } @@ -292,7 +320,6 @@ class quiz_overview_report extends quiz_attempts_report { * @param object $cm the cm object for the quiz. */ protected function start_regrade($quiz, $cm) { - global $OUTPUT, $PAGE; require_capability('mod/quiz:regrade', $this->context); $this->print_header_and_tabs($cm, $this->course, $quiz, $this->mode); } @@ -379,37 +406,40 @@ class quiz_overview_report extends quiz_attempts_report { * controlled by the parameters. * @param object $quiz the quiz settings. * @param bool $dryrun if true, do a pretend regrade, otherwise do it for real. - * @param array $groupstudents blank for all attempts, otherwise regrade attempts + * @param \core\dml\sql_join|array $groupstudentsjoins empty for all attempts, otherwise regrade attempts * for these users. * @param array $attemptids blank for all attempts, otherwise only regrade * attempts whose id is in this list. */ protected function regrade_attempts($quiz, $dryrun = false, - $groupstudents = array(), $attemptids = array()) { + \core\dml\sql_join$groupstudentsjoins = null, $attemptids = array()) { global $DB; $this->unlock_session(); - $where = "quiz = ? AND preview = 0"; - $params = array($quiz->id); + $sql = "SELECT quiza.* + FROM {quiz_attempts} quiza"; + $where = "quiz = :qid AND preview = 0"; + $params = array('qid' => $quiz->id); - if ($groupstudents) { - list($usql, $uparams) = $DB->get_in_or_equal($groupstudents); - $where .= " AND userid $usql"; - $params = array_merge($params, $uparams); + if ($this->hasgroupstudents && !empty($groupstudentsjoins->joins)) { + $sql .= "\nJOIN {user} u ON u.id = quiza.userid + {$groupstudentsjoins->joins}"; + $where .= " AND {$groupstudentsjoins->wheres}"; + $params += $groupstudentsjoins->params; } if ($attemptids) { - list($asql, $aparams) = $DB->get_in_or_equal($attemptids); - $where .= " AND id $asql"; - $params = array_merge($params, $aparams); + $aids = join(',', $attemptids); + $where .= " AND quiza.id IN ({$aids})"; } - $attempts = $DB->get_records_select('quiz_attempts', $where, $params); + $sql .= "\nWHERE {$where}"; + $attempts = $DB->get_records_sql($sql, $params); if (!$attempts) { return; } - $this->clear_regrade_table($quiz, $groupstudents); + $this->clear_regrade_table($quiz, $groupstudentsjoins); $progressbar = new progress_bar('quiz_overview_regrade', 500, true); $a = array( @@ -432,28 +462,30 @@ class quiz_overview_report extends quiz_attempts_report { * Regrade those questions in those attempts that are marked as needing regrading * in the quiz_overview_regrades table. * @param object $quiz the quiz settings. - * @param array $groupstudents blank for all attempts, otherwise regrade attempts + * @param \core\dml\sql_join $groupstudentsjoins empty for all attempts, otherwise regrade attempts * for these users. */ - protected function regrade_attempts_needing_it($quiz, $groupstudents) { + protected function regrade_attempts_needing_it($quiz, \core\dml\sql_join $groupstudentsjoins) { global $DB; $this->unlock_session(); - $where = "quiza.quiz = ? AND quiza.preview = 0 AND qqr.regraded = 0"; - $params = array($quiz->id); + $join = '{quiz_overview_regrades} qqr ON qqr.questionusageid = quiza.uniqueid'; + $where = "quiza.quiz = :qid AND quiza.preview = 0 AND qqr.regraded = 0"; + $params = array('qid' => $quiz->id); // Fetch all attempts that need regrading. - if ($groupstudents) { - list($usql, $uparams) = $DB->get_in_or_equal($groupstudents); - $where .= " AND quiza.userid $usql"; - $params = array_merge($params, $uparams); + if ($this->hasgroupstudents && !empty($groupstudentsjoins->joins)) { + $join .= "\nJOIN {user} u ON u.id = quiza.userid + {$groupstudentsjoins->joins}"; + $where .= " AND {$groupstudentsjoins->wheres}"; + $params += $groupstudentsjoins->params; } $toregrade = $DB->get_recordset_sql(" SELECT quiza.uniqueid, qqr.slot - FROM {quiz_attempts} quiza - JOIN {quiz_overview_regrades} qqr ON qqr.questionusageid = quiza.uniqueid - WHERE $where", $params); + FROM {quiz_attempts} quiza + JOIN $join + WHERE $where", $params); $attemptquestions = array(); foreach ($toregrade as $row) { @@ -468,7 +500,7 @@ class quiz_overview_report extends quiz_attempts_report { $attempts = $DB->get_records_list('quiz_attempts', 'uniqueid', array_keys($attemptquestions)); - $this->clear_regrade_table($quiz, $groupstudents); + $this->clear_regrade_table($quiz, $groupstudentsjoins); $progressbar = new progress_bar('quiz_overview_regrade', 500, true); $a = array( @@ -488,28 +520,32 @@ class quiz_overview_report extends quiz_attempts_report { /** * Count the number of attempts in need of a regrade. * @param object $quiz the quiz settings. - * @param array $groupstudents user ids. If this is given, only data relating + * @param \core\dml\sql_join $groupstudentsjoins (joins, wheres, params) If this is given, only data relating * to these users is cleared. */ - protected function count_question_attempts_needing_regrade($quiz, $groupstudents) { + protected function count_question_attempts_needing_regrade($quiz, \core\dml\sql_join $groupstudentsjoins) { global $DB; + $userjoin = ''; $usertest = ''; $params = array(); - if ($groupstudents) { - list($usql, $params) = $DB->get_in_or_equal($groupstudents); - $usertest = "quiza.userid $usql AND "; + if ($this->hasgroupstudents) { + $userjoin = "JOIN {user} u ON u.id = quiza.userid + {$groupstudentsjoins->joins}"; + $usertest = "{$groupstudentsjoins->wheres} AND u.id = quiza.userid AND "; + $params = $groupstudentsjoins->params; } - $params[] = $quiz->id; + $params['cquiz'] = $quiz->id; $sql = "SELECT COUNT(DISTINCT quiza.id) - FROM {quiz_attempts} quiza - JOIN {quiz_overview_regrades} qqr ON quiza.uniqueid = qqr.questionusageid - WHERE - $usertest - quiza.quiz = ? AND - quiza.preview = 0 AND - qqr.regraded = 0"; + FROM {quiz_attempts} quiza + JOIN {quiz_overview_regrades} qqr ON quiza.uniqueid = qqr.questionusageid + $userjoin + WHERE + $usertest + quiza.quiz = :cquiz AND + quiza.preview = 0 AND + qqr.regraded = 0"; return $DB->count_records_sql($sql, $params); } @@ -532,27 +568,27 @@ class quiz_overview_report extends quiz_attempts_report { /** * Remove all information about pending/complete regrades from the database. * @param object $quiz the quiz settings. - * @param array $groupstudents user ids. If this is given, only data relating + * @param \core\dml\sql_join $groupstudentsjoins (joins, wheres, params). If this is given, only data relating * to these users is cleared. */ - protected function clear_regrade_table($quiz, $groupstudents) { + protected function clear_regrade_table($quiz, \core\dml\sql_join $groupstudentsjoins) { global $DB; // Fetch all attempts that need regrading. - $where = ''; - $params = array(); - if ($groupstudents) { - list($usql, $params) = $DB->get_in_or_equal($groupstudents); - $where = "userid $usql AND "; - } - - $params[] = $quiz->id; - $DB->delete_records_select('quiz_overview_regrades', - "questionusageid IN ( + $select = "questionusageid IN ( SELECT uniqueid - FROM {quiz_attempts} - WHERE $where quiz = ? - )", $params); + FROM {quiz_attempts} quiza"; + $where = "WHERE quiza.quiz = :qid"; + $params = array('qid' => $quiz->id); + if ($this->hasgroupstudents && !empty($groupstudentsjoins->joins)) { + $select .= "\nJOIN {user} u ON u.id = quiza.userid + {$groupstudentsjoins->joins}"; + $where .= " AND {$groupstudentsjoins->wheres}"; + $params += $groupstudentsjoins->params; + } + $select .= "\n$where)"; + + $DB->delete_records_select('quiz_overview_regrades', $select, $params); } /** diff --git a/mod/quiz/report/overview/tests/behat/basic.feature b/mod/quiz/report/overview/tests/behat/basic.feature new file mode 100644 index 00000000000..929cc8fdb11 --- /dev/null +++ b/mod/quiz/report/overview/tests/behat/basic.feature @@ -0,0 +1,105 @@ +@mod @mod_quiz @mod_quiz_overview_basic +Feature: Basic use of the Grades report + In order to easily get an overview of quiz attempts + As a teacher + I need to use the Grades report + + @javascript + Scenario: Using the Grades report + Given the following "users" exist: + | username | firstname | lastname | email | idnumber | + | teacher1 | T1 | Teacher1 | teacher1@example.com | T1000 | + | student1 | S1 | Student1 | student1@example.com | S1000 ! + | student2 | S2 | Student2 | student2@example.com | S2000 ! + | student3 | S3 | Student3 | student3@example.com | S3000 ! + And the following "courses" exist: + | fullname | shortname | category | + | Course 1 | C1 | 0 | + And the following "course enrolments" exist: + | user | course | role | + | teacher1 | C1 | editingteacher | + | student1 | C1 | student | + | student2 | C1 | student | + | student3 | C1 | student | + And the following "question categories" exist: + | contextlevel | reference | name | + | Course | C1 | Test questions | + And the following "activities" exist: + | activity | name | intro | course | idnumber | + | quiz | Quiz 1 | Quiz 1 description | C1 | quiz1 | + And the following "questions" exist: + | questioncategory | qtype | name | questiontext | + | Test questions | truefalse | TF1 | First question | + | Test questions | truefalse | TF2 | Second question | + And quiz "Quiz 1" contains the following questions: + | question | page | maxmark | + | TF1 | 1 | | + | TF2 | 1 | 3.0 | + + # Add some attempts + And I log in as "student1" + And I follow "Course 1" + And I follow "Quiz 1" + And I press "Attempt quiz now" + And I click on "True" "radio" in the "First question" "question" + And I click on "False" "radio" in the "Second question" "question" + And I press "Finish attempt ..." + And I press "Submit all and finish" + And I click on "Submit all and finish" "button" in the "Confirmation" "dialogue" + And I log out + + And I log in as "student2" + And I follow "Course 1" + And I follow "Quiz 1" + And I press "Attempt quiz now" + And I click on "True" "radio" in the "First question" "question" + And I click on "True" "radio" in the "Second question" "question" + And I press "Finish attempt ..." + And I press "Submit all and finish" + And I click on "Submit all and finish" "button" in the "Confirmation" "dialogue" + And I log out + + # Basic check of the Grades report + When I log in as "teacher1" + And I follow "Course 1" + And I follow "Quiz 1" + And I navigate to "Grades" node in "Quiz administration > Results" + Then I should see "Attempts: 2" + # Check student1's grade + And I should see "25.00" in the "#mod-quiz-report-overview-report_r0_c8" "css_element" + # And student2's grade + And I should see "100.00" in the "#mod-quiz-report-overview-report_r1_c8" "css_element" + + # Check changing the form parameters + When I set the field "Attempts from" to "enrolled users who have not attempted the quiz" + And I press "Show report" + # Check teacher1's grade + Then I should see "-" in the "#mod-quiz-report-overview-report_r0_c8" "css_element" + # Check student3's grade + And I should see "-" in the "#mod-quiz-report-overview-report_r1_c8" "css_element" + + When I set the field "Attempts from" to "enrolled users who have, or have not, attempted the quiz" + And I press "Show report" + # Check student1's grade + Then I should see "25.00" in the "#mod-quiz-report-overview-report_r0_c8" "css_element" + # Check student2's grade + And I should see "100.00" in the "#mod-quiz-report-overview-report_r1_c8" "css_element" + # Check teacher1's grade + And I should see "-" in the "#mod-quiz-report-overview-report_r2_c8" "css_element" + + When I set the field "Attempts from" to "all users who have attempted the quiz" + And I press "Show report" + # Check student1's grade + Then I should see "25.00" in the "#mod-quiz-report-overview-report_r0_c8" "css_element" + # Check student2's grade + And I should see "100.00" in the "#mod-quiz-report-overview-report_r1_c8" "css_element" + + # Check regrade and delete attempts. + And I set the field with xpath "//tr[contains(normalize-space(.), 'student1@example.com')]//input[@type='checkbox']" to "1" + When I press "Regrade selected attempts" + And I press "Continue" + Then I should see "student1@example.com" + And I set the field with xpath "//tr[contains(normalize-space(.), 'student1@example.com')]//input[@type='checkbox']" to "1" + When I press "Delete selected attempts" + And I press "Yes" + Then I should not see "student1@example.com" diff --git a/mod/quiz/report/overview/tests/report_test.php b/mod/quiz/report/overview/tests/report_test.php index 1713d95b1c6..46cdafcfd35 100644 --- a/mod/quiz/report/overview/tests/report_test.php +++ b/mod/quiz/report/overview/tests/report_test.php @@ -40,20 +40,23 @@ require_once($CFG->dirroot . '/mod/quiz/report/overview/report.php'); class quiz_overview_report_testcase extends advanced_testcase { public function test_report_sql() { - global $DB, $SITE; + global $DB; $this->resetAfterTest(true); $generator = $this->getDataGenerator(); + $course = $generator->create_course(); $quizgenerator = $generator->get_plugin_generator('mod_quiz'); - $quiz = $quizgenerator->create_instance(array('course' => $SITE->id, + $quiz = $quizgenerator->create_instance(array('course' => $course->id, 'grademethod' => QUIZ_GRADEHIGHEST, 'grade' => 100.0, 'sumgrades' => 10.0, 'attempts' => 10)); $student1 = $generator->create_user(); $student2 = $generator->create_user(); $student3 = $generator->create_user(); + $generator->enrol_user($student1->id, $course->id); + $generator->enrol_user($student2->id, $course->id); + $generator->enrol_user($student3->id, $course->id); - $quizid = 123; $timestamp = 1234567890; // The test data. @@ -87,12 +90,13 @@ class quiz_overview_report_testcase extends advanced_testcase { $DB->insert_record('quiz_attempts', $data); } - // Actually getting the SQL to run is quit hard. Do a minimal set up of + // Actually getting the SQL to run is quite hard. Do a minimal set up of // some objects. $context = context_module::instance($quiz->cmid); $cm = get_coursemodule_from_id('quiz', $quiz->cmid); $qmsubselect = quiz_report_qm_filter_select($quiz); - $reportstudents = array($student1->id, $student2->id, $student3->id); + $studentsjoins = get_enrolled_with_capabilities_join($context); + $empty = new \core\dml\sql_join(); // Set the options. $reportoptions = new quiz_overview_options('overview', $quiz, $cm, null); @@ -102,19 +106,19 @@ class quiz_overview_report_testcase extends advanced_testcase { // Now do a minimal set-up of the table class. $table = new quiz_overview_table($quiz, $context, $qmsubselect, $reportoptions, - array(), $reportstudents, array(1), null); + $empty, $studentsjoins, array(1), null); $table->define_columns(array('attempt')); $table->sortable(true, 'uniqueid'); $table->define_baseurl(new moodle_url('/mod/quiz/report.php')); $table->setup(); // Run the query. - list($fields, $from, $where, $params) = $table->base_sql($reportstudents); + list($fields, $from, $where, $params) = $table->base_sql($studentsjoins); $table->set_sql($fields, $from, $where, $params); $table->query_db(30, false); // Verify what was returned: Student 1's best and in progress attempts. - // Stuent 2's finshed attempt, and Student 3 with no attempt. + // Student 2's finshed attempt, and Student 3 with no attempt. // The array key is {student id}#{attempt number}. $this->assertEquals(4, count($table->rawdata)); $this->assertArrayHasKey($student1->id . '#3', $table->rawdata); diff --git a/mod/quiz/report/reportlib.php b/mod/quiz/report/reportlib.php index 0bb3b5baa59..30f8261beef 100644 --- a/mod/quiz/report/reportlib.php +++ b/mod/quiz/report/reportlib.php @@ -191,10 +191,10 @@ function quiz_report_grade_method_sql($grademethod, $quizattemptsalias = 'quiza' * @param number $bandwidth the width of each band. * @param int $bands the number of bands * @param int $quizid the quiz id. - * @param array $userids list of user ids. + * @param \core\dml\sql_join $usersjoins (joins, wheres, params) to get enrolled users * @return array band number => number of users with scores in that band. */ -function quiz_report_grade_bands($bandwidth, $bands, $quizid, $userids = array()) { +function quiz_report_grade_bands($bandwidth, $bands, $quizid, \core\dml\sql_join $usersjoins = null) { global $DB; if (!is_int($bands)) { debugging('$bands passed to quiz_report_grade_bands must be an integer. (' . @@ -202,11 +202,14 @@ function quiz_report_grade_bands($bandwidth, $bands, $quizid, $userids = array() $bands = (int) $bands; } - if ($userids) { - list($usql, $params) = $DB->get_in_or_equal($userids, SQL_PARAMS_NAMED, 'u'); - $usql = "qg.userid $usql AND"; + if ($usersjoins && !empty($usersjoins->joins)) { + $userjoin = "JOIN {user} u ON u.id = qg.userid + {$usersjoins->joins}"; + $usertest = $usersjoins->wheres; + $params = $usersjoins->params; } else { - $usql = ''; + $userjoin = ''; + $usertest = '1=1'; $params = array(); } $sql = " @@ -215,7 +218,8 @@ SELECT band, COUNT(1) FROM ( SELECT FLOOR(qg.grade / :bandwidth) AS band FROM {quiz_grades} qg - WHERE $usql qg.quiz = :quizid + $userjoin + WHERE $usertest AND qg.quiz = :quizid ) subquery GROUP BY diff --git a/mod/quiz/report/responses/last_responses_table.php b/mod/quiz/report/responses/last_responses_table.php index ef63fade580..009c17ee539 100644 --- a/mod/quiz/report/responses/last_responses_table.php +++ b/mod/quiz/report/responses/last_responses_table.php @@ -42,15 +42,15 @@ class quiz_last_responses_table extends quiz_attempts_report_table { * @param context $context * @param string $qmsubselect * @param quiz_responses_options $options - * @param array $groupstudents - * @param array $students + * @param \core\dml\sql_join $groupstudentsjoins + * @param \core\dml\sql_join $studentsjoins * @param array $questions * @param moodle_url $reporturl */ public function __construct($quiz, $context, $qmsubselect, quiz_responses_options $options, - $groupstudents, $students, $questions, $reporturl) { + \core\dml\sql_join $groupstudentsjoins, \core\dml\sql_join $studentsjoins, $questions, $reporturl) { parent::__construct('mod-quiz-report-responses-report', $quiz, $context, - $qmsubselect, $options, $groupstudents, $students, $questions, $reporturl); + $qmsubselect, $options, $groupstudentsjoins, $studentsjoins, $questions, $reporturl); } public function build_table() { diff --git a/mod/quiz/report/responses/report.php b/mod/quiz/report/responses/report.php index dab6fa654e7..41f9608a44f 100644 --- a/mod/quiz/report/responses/report.php +++ b/mod/quiz/report/responses/report.php @@ -49,10 +49,11 @@ require_once($CFG->dirroot . '/mod/quiz/report/responses/first_or_all_responses_ class quiz_responses_report extends quiz_attempts_report { public function display($quiz, $cm, $course) { - global $OUTPUT; + global $OUTPUT, $DB; + + list($currentgroup, $studentsjoins, $groupstudentsjoins, $allowedjoins) = $this->init( + 'responses', 'quiz_responses_settings_form', $quiz, $cm, $course); - list($currentgroup, $students, $groupstudents, $allowed) = - $this->init('responses', 'quiz_responses_settings_form', $quiz, $cm, $course); $options = new quiz_responses_options('responses', $quiz, $cm, $course); if ($fromform = $this->form->get_data()) { @@ -64,13 +65,6 @@ class quiz_responses_report extends quiz_attempts_report { $this->form->set_data($options->get_initial_form_data()); - if ($options->attempts == self::ALL_WITH) { - // This option is only available to users who can access all groups in - // groups mode, so setting allowed to empty (which means all quiz attempts - // are accessible, is not a security porblem. - $allowed = array(); - } - // Load the required questions. $questions = quiz_report_get_significant_questions($quiz); @@ -83,7 +77,7 @@ class quiz_responses_report extends quiz_attempts_report { $tableclassname = 'quiz_first_or_all_responses_table'; } $table = new $tableclassname($quiz, $this->context, $this->qmsubselect, - $options, $groupstudents, $students, $questions, $options->get_url()); + $options, $groupstudentsjoins, $studentsjoins, $questions, $options->get_url()); $filename = quiz_report_download_filename(get_string('responsesfilename', 'quiz_responses'), $courseshortname, $quiz->name); $table->is_downloading($options->download, $filename, @@ -92,7 +86,30 @@ class quiz_responses_report extends quiz_attempts_report { raise_memory_limit(MEMORY_EXTRA); } - $this->process_actions($quiz, $cm, $currentgroup, $groupstudents, $allowed, $options->get_url()); + $this->hasgroupstudents = false; + if (!empty($groupstudentsjoins->joins)) { + $sql = "SELECT DISTINCT u.id + FROM {user} u + $groupstudentsjoins->joins + WHERE $groupstudentsjoins->wheres"; + $this->hasgroupstudents = $DB->record_exists_sql($sql, $groupstudentsjoins->params); + } + $hasstudents = false; + if (!empty($studentsjoins->joins)) { + $sql = "SELECT DISTINCT u.id + FROM {user} u + $studentsjoins->joins + WHERE $studentsjoins->wheres"; + $hasstudents = $DB->record_exists_sql($sql, $studentsjoins->params); + } + if ($options->attempts == self::ALL_WITH) { + // This option is only available to users who can access all groups in + // groups mode, so setting allowed to empty (which means all quiz attempts + // are accessible, is not a security problem. + $allowedjoins = array(); + } + + $this->process_actions($quiz, $cm, $currentgroup, $groupstudentsjoins, $allowedjoins, $options->get_url()); // Start output. if (!$table->is_downloading()) { @@ -119,9 +136,9 @@ class quiz_responses_report extends quiz_attempts_report { if (!$table->is_downloading()) { if (!$hasquestions) { echo quiz_no_questions_message($quiz, $cm, $this->context); - } else if (!$students) { + } else if (!$hasstudents) { echo $OUTPUT->notification(get_string('nostudentsyet')); - } else if ($currentgroup && !$groupstudents) { + } else if ($currentgroup && !$this->hasgroupstudents) { echo $OUTPUT->notification(get_string('nostudentsingroup')); } @@ -129,10 +146,10 @@ class quiz_responses_report extends quiz_attempts_report { $this->form->display(); } - $hasstudents = $students && (!$currentgroup || $groupstudents); + $hasstudents = $hasstudents && (!$currentgroup || $this->hasgroupstudents); if ($hasquestions && ($hasstudents || $options->attempts == self::ALL_WITH)) { - list($fields, $from, $where, $params) = $table->base_sql($allowed); + list($fields, $from, $where, $params) = $table->base_sql($allowedjoins); $table->set_count_sql("SELECT COUNT(1) FROM $from WHERE $where", $params); diff --git a/mod/quiz/report/statistics/classes/calculator.php b/mod/quiz/report/statistics/classes/calculator.php index ada45a7d9ee..e6ff9567a69 100644 --- a/mod/quiz/report/statistics/classes/calculator.php +++ b/mod/quiz/report/statistics/classes/calculator.php @@ -51,18 +51,18 @@ class calculator { * $quiz->grademethod ie. * QUIZ_GRADEAVERAGE, QUIZ_GRADEHIGHEST, QUIZ_ATTEMPTLAST or QUIZ_ATTEMPTFIRST * we calculate stats based on which attempts would affect the grade for each student. - * @param array $groupstudents students in this group. + * @param \core\dml\sql_join $groupstudentsjoins Contains joins, wheres, params for students in this group. * @param int $p number of positions (slots). * @param float $sumofmarkvariance sum of mark variance, calculated as part of question statistics * @return calculated $quizstats The statistics for overall attempt scores. */ - public function calculate($quizid, $whichattempts, $groupstudents, $p, $sumofmarkvariance) { + public function calculate($quizid, $whichattempts, \core\dml\sql_join $groupstudentsjoins, $p, $sumofmarkvariance) { $this->progress->start_progress('', 3); $quizstats = new calculated($whichattempts); - $countsandaverages = $this->attempt_counts_and_averages($quizid, $groupstudents); + $countsandaverages = $this->attempt_counts_and_averages($quizid, $groupstudentsjoins); $this->progress->progress(1); foreach ($countsandaverages as $propertyname => $value) { @@ -74,7 +74,7 @@ class calculator { // Recalculate sql again this time possibly including test for first attempt. list($fromqa, $whereqa, $qaparams) = - quiz_statistics_attempts_sql($quizid, $groupstudents, $whichattempts); + quiz_statistics_attempts_sql($quizid, $groupstudentsjoins, $whichattempts); $quizstats->median = $this->median($s, $fromqa, $whereqa, $qaparams); $this->progress->progress(2); @@ -115,7 +115,7 @@ class calculator { } } - $quizstats->cache(quiz_statistics_qubaids_condition($quizid, $groupstudents, $whichattempts)); + $quizstats->cache(quiz_statistics_qubaids_condition($quizid, $groupstudentsjoins, $whichattempts)); } $this->progress->end_progress(); return $quizstats; @@ -203,16 +203,16 @@ class calculator { * See : http://docs.moodle.org/dev/Quiz_item_analysis_calculations_in_practise * #Calculating_MEAN_of_grades_for_all_attempts_by_students * @param int $quizid - * @param array $groupstudents + * @param \core\dml\sql_join $groupstudentsjoins Contains joins, wheres, params for students in this group. * @return \stdClass with properties with count and avg with prefixes firstattempts, highestattempts, etc. */ - protected function attempt_counts_and_averages($quizid, $groupstudents) { + protected function attempt_counts_and_averages($quizid, \core\dml\sql_join $groupstudentsjoins) { global $DB; $attempttotals = new \stdClass(); foreach (array_keys(quiz_get_grading_options()) as $which) { - list($fromqa, $whereqa, $qaparams) = quiz_statistics_attempts_sql($quizid, $groupstudents, $which); + list($fromqa, $whereqa, $qaparams) = quiz_statistics_attempts_sql($quizid, $groupstudentsjoins, $which); $fromdb = $DB->get_record_sql("SELECT COUNT(*) AS rcount, AVG(sumgrades) AS average FROM $fromqa WHERE $whereqa", $qaparams); @@ -245,10 +245,10 @@ class calculator { $limitoffset = floor($s / 2); $limit = 1; } - $sql = "SELECT id, sumgrades - FROM $fromqa - WHERE $whereqa - ORDER BY sumgrades"; + $sql = "SELECT quiza.id, quiza.sumgrades + FROM $fromqa + WHERE $whereqa + ORDER BY sumgrades"; $medianmarks = $DB->get_records_sql_menu($sql, $qaparams, $limitoffset, $limit); @@ -274,8 +274,8 @@ class calculator { SUM(POWER((quiza.sumgrades - $mean), 2)) AS power2, SUM(POWER((quiza.sumgrades - $mean), 3)) AS power3, SUM(POWER((quiza.sumgrades - $mean), 4)) AS power4 - FROM $fromqa - WHERE $whereqa"; + FROM $fromqa + WHERE $whereqa"; $params = array('mean1' => $mean, 'mean2' => $mean, 'mean3' => $mean) + $qaparams; return $DB->get_record_sql($sql, $params, MUST_EXIST); diff --git a/mod/quiz/report/statistics/db/upgrade.php b/mod/quiz/report/statistics/db/upgrade.php index 1169acef51c..920fe14b28c 100644 --- a/mod/quiz/report/statistics/db/upgrade.php +++ b/mod/quiz/report/statistics/db/upgrade.php @@ -28,7 +28,7 @@ defined('MOODLE_INTERNAL') || die(); * Quiz statistics report upgrade code. */ function xmldb_quiz_statistics_upgrade($oldversion) { - global $CFG; + global $DB; // Moodle v2.8.0 release upgrade line. // Put any upgrade step following this. @@ -42,5 +42,13 @@ function xmldb_quiz_statistics_upgrade($oldversion) { // Moodle v3.1.0 release upgrade line. // Put any upgrade step following this. + // Moodle v3.2.0 release upgrade line. + // Clear the quiz_statistics table - it is only a cache table anyway. + // This will force re-calculation. + if ($oldversion < 2016100500) { + $DB->delete_records('quiz_statistics'); + upgrade_plugin_savepoint(true, 2016100500, 'quiz', 'statistics'); + } + return true; } diff --git a/mod/quiz/report/statistics/report.php b/mod/quiz/report/statistics/report.php index 8fdb48dd936..b7bea7223cf 100644 --- a/mod/quiz/report/statistics/report.php +++ b/mod/quiz/report/statistics/report.php @@ -52,7 +52,7 @@ class quiz_statistics_report extends quiz_default_report { * Display the report. */ public function display($quiz, $cm, $course) { - global $OUTPUT; + global $OUTPUT, $DB; raise_memory_limit(MEMORY_HUGE); @@ -98,23 +98,28 @@ class quiz_statistics_report extends quiz_default_report { $nostudentsingroup = false; // True if a group is selected and there is no one in it. if (empty($currentgroup)) { $currentgroup = 0; - $groupstudents = array(); + $groupstudentsjoins = new \core\dml\sql_join(); } else if ($currentgroup == self::NO_GROUPS_ALLOWED) { - $groupstudents = array(); + $groupstudentsjoins = new \core\dml\sql_join(); $nostudentsingroup = true; } else { // All users who can attempt quizzes and who are in the currently selected group. - $groupstudents = get_users_by_capability($this->context, - array('mod/quiz:reviewmyattempts', 'mod/quiz:attempt'), - '', '', '', '', $currentgroup, '', false); - if (!$groupstudents) { - $nostudentsingroup = true; + $groupstudentsjoins = get_enrolled_with_capabilities_join($this->context, '', + array('mod/quiz:reviewmyattempts', 'mod/quiz:attempt'), $currentgroup); + if (!empty($groupstudentsjoins->joins)) { + $sql = "SELECT DISTINCT u.id + FROM {user} u + {$groupstudentsjoins->joins} + WHERE {$groupstudentsjoins->wheres}"; + if (!$DB->record_exists_sql($sql, $groupstudentsjoins->params)) { + $nostudentsingroup = true; + } } } - $qubaids = quiz_statistics_qubaids_condition($quiz->id, $groupstudents, $whichattempts); + $qubaids = quiz_statistics_qubaids_condition($quiz->id, $groupstudentsjoins, $whichattempts); // If recalculate was requested, handle that. if ($recalculate && confirm_sesskey()) { @@ -145,24 +150,22 @@ class quiz_statistics_report extends quiz_default_report { // Get the data to be displayed. $progress = $this->get_progress_trace_instance(); list($quizstats, $questionstats) = - $this->get_all_stats_and_analysis($quiz, $whichattempts, $whichtries, $groupstudents, $questions, $progress); + $this->get_all_stats_and_analysis($quiz, $whichattempts, $whichtries, $groupstudentsjoins, $questions, $progress); } else { // Or create empty stats containers. $quizstats = new \quiz_statistics\calculated($whichattempts); $questionstats = new \core_question\statistics\questions\all_calculated_for_qubaid_condition(); } - // Set up the table, if there is data. - if ($quizstats->s()) { - $this->table->statistics_setup($quiz, $cm->id, $reporturl, $quizstats->s()); - } + // Set up the table. + $this->table->statistics_setup($quiz, $cm->id, $reporturl, $quizstats->s()); // Print the rest of the page header stuff (if not downloading. if (!$this->table->is_downloading()) { if (groups_get_activity_groupmode($cm)) { groups_print_activity_menu($cm, $reporturl->out()); - if ($currentgroup && !$groupstudents) { + if ($currentgroup && $nostudentsingroup) { $OUTPUT->notification(get_string('nostudentsingroup', 'quiz_statistics')); } } @@ -188,7 +191,7 @@ class quiz_statistics_report extends quiz_default_report { $this->output_quiz_structure_analysis_table($questionstats); if ($this->table->is_downloading() == 'xhtml' && $quizstats->s() != 0) { - $this->output_statistics_graph($quiz->id, $currentgroup, $whichattempts); + $this->output_statistics_graph($quiz->id, $qubaids); } $this->output_all_question_response_analysis($qubaids, $questions, $questionstats, $reporturl, $whichtries); @@ -258,14 +261,14 @@ class quiz_statistics_report extends quiz_default_report { } else { // On-screen display of overview report. echo $OUTPUT->heading(get_string('quizinformation', 'quiz_statistics'), 3); - echo $this->output_caching_info($quizstats->timemodified, $quiz->id, $groupstudents, $whichattempts, $reporturl); + echo $this->output_caching_info($quizstats->timemodified, $quiz->id, $groupstudentsjoins, $whichattempts, $reporturl); echo $this->everything_download_options($reporturl); $quizinfo = $quizstats->get_formatted_quiz_info_data($course, $cm, $quiz); echo $this->output_quiz_info_table($quizinfo); if ($quizstats->s()) { echo $OUTPUT->heading(get_string('quizstructureanalysis', 'quiz_statistics'), 3); $this->output_quiz_structure_analysis_table($questionstats); - $this->output_statistics_graph($quiz, $currentgroup, $whichattempts); + $this->output_statistics_graph($quiz, $qubaids); } } @@ -511,26 +514,16 @@ class quiz_statistics_report extends quiz_default_report { * Output the HTML needed to show the statistics graph. * * @param int|object $quizorid The quiz, or its ID. - * @param int $currentgroup The current group. + * @param qubaid_condition $qubaids the question usages whose responses to analyse. * @param string $whichattempts Which attempts constant. */ - protected function output_statistics_graph($quizorid, $currentgroup, $whichattempts) { + protected function output_statistics_graph($quizorid, $qubaids) { global $DB, $PAGE; $quiz = $quizorid; if (!is_object($quiz)) { $quiz = $DB->get_record('quiz', array('id' => $quizorid), '*', MUST_EXIST); } - $quizid = $quiz->id; - - if (empty($currentgroup)) { - $groupstudents = []; - } else { - $groupstudents = get_users_by_capability($this->context, array('mod/quiz:reviewmyattempts', 'mod/quiz:attempt'), - '', '', '', '', $currentgroup, '', false); - } - - $qubaids = quiz_statistics_qubaids_condition($quizid, $groupstudents, $whichattempts); // Load the rest of the required data. $questions = quiz_report_get_significant_questions($quiz); @@ -606,19 +599,20 @@ class quiz_statistics_report extends quiz_default_report { * we calculate stats based on which attempts would affect the grade for each student. * @param string $whichtries which tries to analyse for response analysis. Will be one of * question_attempt::FIRST_TRY, LAST_TRY or ALL_TRIES. - * @param array $groupstudents students in this group. + * @param \core\dml\sql_join $groupstudentsjoins Contains joins, wheres, params for students in this group. * @param array $questions full question data. * @param \core\progress\base|null $progress * @return array with 2 elements: - $quizstats The statistics for overall attempt scores. * - $questionstats \core_question\statistics\questions\all_calculated_for_qubaid_condition */ - public function get_all_stats_and_analysis($quiz, $whichattempts, $whichtries, $groupstudents, $questions, $progress = null) { + public function get_all_stats_and_analysis( + $quiz, $whichattempts, $whichtries, \core\dml\sql_join $groupstudentsjoins, $questions, $progress = null) { if ($progress === null) { $progress = new \core\progress\none(); } - $qubaids = quiz_statistics_qubaids_condition($quiz->id, $groupstudents, $whichattempts); + $qubaids = quiz_statistics_qubaids_condition($quiz->id, $groupstudentsjoins, $whichattempts); $qcalc = new \core_question\statistics\questions\calculator($questions, $progress); @@ -631,7 +625,7 @@ class quiz_statistics_report extends quiz_default_report { $questionstats = $qcalc->calculate($qubaids); $progress->progress(1); - $quizstats = $quizcalc->calculate($quiz->id, $whichattempts, $groupstudents, count($questions), + $quizstats = $quizcalc->calculate($quiz->id, $whichattempts, $groupstudentsjoins, count($questions), $qcalc->get_sum_of_mark_variance()); $progress->progress(2); } else { @@ -749,7 +743,7 @@ class quiz_statistics_report extends quiz_default_report { * * @param int $lastcachetime the time the stats were last cached. * @param int $quizid the quiz id. - * @param array $groupstudents ids of students in the group or empty array if groups not used. + * @param array $groupstudentsjoins (joins, wheres, params) for students in the group or empty array if groups not used. * @param string $whichattempts which attempts to use, represented internally as one of the constants as used in * $quiz->grademethod ie. * QUIZ_GRADEAVERAGE, QUIZ_GRADEHIGHEST, QUIZ_ATTEMPTLAST or QUIZ_ATTEMPTFIRST @@ -757,7 +751,7 @@ class quiz_statistics_report extends quiz_default_report { * @param moodle_url $reporturl url for this report * @return string HTML. */ - protected function output_caching_info($lastcachetime, $quizid, $groupstudents, $whichattempts, $reporturl) { + protected function output_caching_info($lastcachetime, $quizid, $groupstudentsjoins, $whichattempts, $reporturl) { global $DB, $OUTPUT; if (empty($lastcachetime)) { @@ -765,7 +759,7 @@ class quiz_statistics_report extends quiz_default_report { } // Find the number of attempts since the cached statistics were computed. - list($fromqa, $whereqa, $qaparams) = quiz_statistics_attempts_sql($quizid, $groupstudents, $whichattempts, true); + list($fromqa, $whereqa, $qaparams) = quiz_statistics_attempts_sql($quizid, $groupstudentsjoins, $whichattempts, true); $count = $DB->count_records_sql(" SELECT COUNT(1) FROM $fromqa diff --git a/mod/quiz/report/statistics/statisticslib.php b/mod/quiz/report/statistics/statisticslib.php index 723ac62f12d..832e14be64b 100644 --- a/mod/quiz/report/statistics/statisticslib.php +++ b/mod/quiz/report/statistics/statisticslib.php @@ -23,11 +23,13 @@ * @license http://www.gnu.org/copyleft/gpl.html GNU GPL v3 or later */ +defined('MOODLE_INTERNAL') || die; + /** * SQL to fetch relevant 'quiz_attempts' records. * * @param int $quizid quiz id to get attempts for - * @param array $groupstudents empty array if not using groups or array of students in current group. + * @param \core\dml\sql_join $groupstudentsjoins Contains joins, wheres, params, empty if not using groups * @param string $whichattempts which attempts to use, represented internally as one of the constants as used in * $quiz->grademethod ie. * QUIZ_GRADEAVERAGE, QUIZ_GRADEHIGHEST, QUIZ_ATTEMPTLAST or QUIZ_ATTEMPTFIRST @@ -35,27 +37,22 @@ * @param bool $includeungraded whether to fetch ungraded attempts too * @return array FROM and WHERE sql fragments and sql params */ -function quiz_statistics_attempts_sql($quizid, $groupstudents, $whichattempts = QUIZ_GRADEAVERAGE, $includeungraded = false) { - global $DB; - - $fromqa = '{quiz_attempts} quiza '; - +function quiz_statistics_attempts_sql($quizid, \core\dml\sql_join $groupstudentsjoins, + $whichattempts = QUIZ_GRADEAVERAGE, $includeungraded = false) { + $fromqa = "{quiz_attempts} quiza "; $whereqa = 'quiza.quiz = :quizid AND quiza.preview = 0 AND quiza.state = :quizstatefinished'; $qaparams = array('quizid' => (int)$quizid, 'quizstatefinished' => quiz_attempt::FINISHED); - if ($groupstudents) { - ksort($groupstudents); - list($grpsql, $grpparams) = $DB->get_in_or_equal(array_keys($groupstudents), - SQL_PARAMS_NAMED, 'statsuser'); - list($grpsql, $grpparams) = quiz_statistics_renumber_placeholders( - $grpsql, $grpparams, 'statsuser'); - $whereqa .= " AND quiza.userid $grpsql"; - $qaparams += $grpparams; + if (!empty($groupstudentsjoins->joins)) { + $fromqa .= "\nJOIN {user} u ON u.id = quiza.userid + {$groupstudentsjoins->joins} "; + $whereqa .= " AND {$groupstudentsjoins->wheres}"; + $qaparams += $groupstudentsjoins->params; } $whichattemptsql = quiz_report_grade_method_sql($whichattempts); if ($whichattemptsql) { - $whereqa .= ' AND '.$whichattemptsql; + $whereqa .= ' AND ' . $whichattemptsql; } if (!$includeungraded) { @@ -65,36 +62,11 @@ function quiz_statistics_attempts_sql($quizid, $groupstudents, $whichattempts = return array($fromqa, $whereqa, $qaparams); } -/** - * Re-number all the params beginning with $paramprefix in a fragment of SQL. - * - * @param string $sql the SQL. - * @param array $params the params. - * @param string $paramprefix the parameter prefix. - * @return array with two elements, the modified SQL, and the modified params. - */ -function quiz_statistics_renumber_placeholders($sql, $params, $paramprefix) { - $basenumber = null; - $newparams = array(); - $newsql = preg_replace_callback('~:' . preg_quote($paramprefix, '~') . '(\d+)\b~', - function($match) use ($paramprefix, $params, &$newparams, &$basenumber) { - if ($basenumber === null) { - $basenumber = $match[1] - 1; - } - $oldname = $paramprefix . $match[1]; - $newname = $paramprefix . ($match[1] - $basenumber); - $newparams[$newname] = $params[$oldname]; - return ':' . $newname; - }, $sql); - - return array($newsql, $newparams); -} - /** * Return a {@link qubaid_condition} from the values returned by {@link quiz_statistics_attempts_sql}. * * @param int $quizid - * @param array $groupstudents + * @param \core\dml\sql_join $groupstudentsjoins Contains joins, wheres, params * @param string $whichattempts which attempts to use, represented internally as one of the constants as used in * $quiz->grademethod ie. * QUIZ_GRADEAVERAGE, QUIZ_GRADEHIGHEST, QUIZ_ATTEMPTLAST or QUIZ_ATTEMPTFIRST @@ -102,8 +74,10 @@ function quiz_statistics_renumber_placeholders($sql, $params, $paramprefix) { * @param bool $includeungraded * @return \qubaid_join */ -function quiz_statistics_qubaids_condition($quizid, $groupstudents, $whichattempts = QUIZ_GRADEAVERAGE, $includeungraded = false) { - list($fromqa, $whereqa, $qaparams) = quiz_statistics_attempts_sql($quizid, $groupstudents, $whichattempts, $includeungraded); +function quiz_statistics_qubaids_condition($quizid, \core\dml\sql_join $groupstudentsjoins, + $whichattempts = QUIZ_GRADEAVERAGE, $includeungraded = false) { + list($fromqa, $whereqa, $qaparams) = quiz_statistics_attempts_sql( + $quizid, $groupstudentsjoins, $whichattempts, $includeungraded); return new qubaid_join($fromqa, 'quiza.uniqueid', $whereqa, $qaparams); } diff --git a/mod/quiz/report/statistics/tests/statisticslib_test.php b/mod/quiz/report/statistics/tests/statisticslib_test.php deleted file mode 100644 index edfb33bf205..00000000000 --- a/mod/quiz/report/statistics/tests/statisticslib_test.php +++ /dev/null @@ -1,52 +0,0 @@ -. - -/** - * Unit tests for (some of) statisticslib.php. - * - * @package quiz_statistics - * @category test - * @copyright 2014 The Open University - * @license http://www.gnu.org/copyleft/gpl.html GNU GPL v3 or later - */ - -defined('MOODLE_INTERNAL') || die(); - -global $CFG; -require_once($CFG->dirroot . '/mod/quiz/report/statistics/statisticslib.php'); - -/** - * Unit tests for (some of) statisticslib.php. - * - * @copyright 2014 The Open University - * @license http://www.gnu.org/copyleft/gpl.html GNU GPL v3 or later - */ -class quiz_statistics_statisticslib_testcase extends basic_testcase { - - public function test_quiz_statistics_renumber_placeholders_no_op() { - list($sql, $params) = quiz_statistics_renumber_placeholders( - ' IN (:u1, :u2)', array('u1' => 1, 'u2' => 2), 'u'); - $this->assertEquals(' IN (:u1, :u2)', $sql); - $this->assertEquals(array('u1' => 1, 'u2' => 2), $params); - } - - public function test_quiz_statistics_renumber_placeholders_work_to_do() { - list($sql, $params) = quiz_statistics_renumber_placeholders( - 'frog1 IN (:frog100 , :frog101)', array('frog100' => 1, 'frog101' => 2), 'frog'); - $this->assertEquals('frog1 IN (:frog1 , :frog2)', $sql); - $this->assertEquals(array('frog1' => 1, 'frog2' => 2), $params); - } -} diff --git a/mod/quiz/report/statistics/tests/stats_from_steps_walkthrough_test.php b/mod/quiz/report/statistics/tests/stats_from_steps_walkthrough_test.php index b79cf6485cc..9d5f3bb3a29 100644 --- a/mod/quiz/report/statistics/tests/stats_from_steps_walkthrough_test.php +++ b/mod/quiz/report/statistics/tests/stats_from_steps_walkthrough_test.php @@ -76,9 +76,10 @@ class quiz_report_statistics_from_steps_testcase extends mod_quiz_attempt_walkth $whichattempts = QUIZ_GRADEAVERAGE; // All attempts. $whichtries = question_attempt::ALL_TRIES; - $groupstudents = array(); + $groupstudentsjoins = new \core\dml\sql_join(); list($questions, $quizstats, $questionstats, $qubaids) = - $this->check_stats_calculations_and_response_analysis($csvdata, $whichattempts, $whichtries, $groupstudents); + $this->check_stats_calculations_and_response_analysis($csvdata, + $whichattempts, $whichtries, $groupstudentsjoins); if ($quizsettings['testnumber'] === '00') { $this->check_variants_count_for_quiz_00($questions, $questionstats, $whichtries, $qubaids); $this->check_quiz_stats_for_quiz_00($quizstats); @@ -360,20 +361,21 @@ class quiz_report_statistics_from_steps_testcase extends mod_quiz_attempt_walkth * @param PHPUnit_Extensions_Database_DataSet_ITable[] $csvdata Data loaded from csv files for this test. * @param string $whichattempts * @param string $whichtries - * @param int[] $groupstudents + * @param \core\dml\sql_join $groupstudentsjoins * @return array with contents 0 => $questions, 1 => $quizstats, 2=> $questionstats, 3=> $qubaids Might be needed for further * testing. */ - protected function check_stats_calculations_and_response_analysis($csvdata, $whichattempts, $whichtries, $groupstudents) { + protected function check_stats_calculations_and_response_analysis($csvdata, $whichattempts, $whichtries, + \core\dml\sql_join $groupstudentsjoins) { $this->report = new quiz_statistics_report(); $questions = $this->report->load_and_initialise_questions_for_calculations($this->quiz); list($quizstats, $questionstats) = $this->report->get_all_stats_and_analysis($this->quiz, $whichattempts, $whichtries, - $groupstudents, + $groupstudentsjoins, $questions); - $qubaids = quiz_statistics_qubaids_condition($this->quiz->id, $groupstudents, $whichattempts); + $qubaids = quiz_statistics_qubaids_condition($this->quiz->id, $groupstudentsjoins, $whichattempts); // We will create some quiz and question stat calculator instances and some response analyser instances, just in order // to check the last analysed time then returned. diff --git a/mod/quiz/report/statistics/upgrade.txt b/mod/quiz/report/statistics/upgrade.txt index 3eb79996224..7926b3f81b6 100644 --- a/mod/quiz/report/statistics/upgrade.txt +++ b/mod/quiz/report/statistics/upgrade.txt @@ -5,3 +5,5 @@ information provided here is intended especially for developers. * The function quiz_statistics_graph_get_new_colour() is deprecated in favour of the funtionality present in the new charting library. +* The function quiz_statistics_renumber_placeholders() is removed as the changes + in MDL-31243 and MDL-27072 make this redundant. diff --git a/mod/quiz/report/statistics/version.php b/mod/quiz/report/statistics/version.php index e79beeb40d4..f251bb25a20 100644 --- a/mod/quiz/report/statistics/version.php +++ b/mod/quiz/report/statistics/version.php @@ -24,7 +24,7 @@ defined('MOODLE_INTERNAL') || die(); -$plugin->version = 2016052300; +$plugin->version = 2016100500; $plugin->requires = 2016051900; $plugin->cron = 18000; $plugin->component = 'quiz_statistics'; diff --git a/mod/quiz/report/upgrade.txt b/mod/quiz/report/upgrade.txt index 18b84fd018c..93da5f9d0f0 100644 --- a/mod/quiz/report/upgrade.txt +++ b/mod/quiz/report/upgrade.txt @@ -1,6 +1,22 @@ This files describes API changes for quiz report plugins. Overview of this plugin type at http://docs.moodle.org/dev/Quiz_reports + +=== 3.2 === + +* A code refactoring based on new sql functions in MDL-31243 and removing +get_users_by_capability from the quiz reports in MDL-27072. The class +quiz_attempts_report is now initialised to return \core\dml\sql_join (joins, +wheres, params) rather than arrays of userids. This allows the use of joins +in quiz report queries and is very important when there +are larger numbers of enrolled users. The signature of many quiz report +methods now includes '$studentsjoins', rather than '$students' and similar +for '$groupstudentsjoins', '$allowedjoins' and '$usersjoins'. For clear +examples of the use of these changes please see attemptsreport_table.php +base_sql() or almost any function in overview/report.php. The protected +function quiz_attempts_report::load_relevant_students is depreciated, +please use quiz_attempts_report::get_students_joins() instead. + === 2.6 === * Improving the display page and heading levels to have a proper nesting.