Skip to content
GitLab
Commit 4cc469b2 authored by Andrew Nicols's avatar Andrew Nicols
Browse files

Merge branch 'MDL-60162' of git://github.com/timhunt/moodle

parents 4d4b8ae6 c282157f
Hide whitespace changes
Inline Side-by-side
mod/quiz/attemptlib.php
View file @ 4cc469b2
......@@ -1093,6 +1093,18 @@ class quiz_attempt {
return $activeslots;
}
/**
* Helper method for unit tests. Get the underlying question usage object.
* @return question_usage_by_activity the usage.
*/
public function get_question_usage() {
if (!PHPUNIT_TEST) {
throw new coding_exception('get_question_usage is only for use in unit tests. ' .
'For other operations, use the quiz_attempt api, or extend it properly.');
}
return $this->quba;
}
/**
* Get the question_attempt object for a particular question in this attempt.
* @param int $slot the number used to identify this question within this attempt.
......
mod/quiz/report/attemptsreport_table.php
View file @ 4cc469b2
......@@ -467,6 +467,35 @@ abstract class quiz_attempts_report_table extends table_sql {
return array($fields, $from, $where, $params);
}
/**
* A chance for subclasses to modify the SQL after the count query has been generated,
* and before the full query is constructed.
* @param string $fields SELECT list.
* @param string $from JOINs part of the SQL.
* @param string $where WHERE clauses.
* @param array $params Query params.
* @return array with 4 elements ($fields, $from, $where, $params) as from base_sql.
*/
protected function update_sql_after_count($fields, $from, $where, $params) {
return [$fields, $from, $where, $params];
}
/**
* Set up the SQL queries (count rows, and get data).
*
* @param \core\dml\sql_join $allowedjoins (joins, wheres, params) defines allowed users for the report.
*/
public function setup_sql_queries($allowedjoins) {
list($fields, $from, $where, $params) = $this->base_sql($allowedjoins);
// The WHERE clause is vital here, because some parts of tablelib.php will expect to
// add bits like ' AND x = 1' on the end, and that needs to leave to valid SQL.
$this->set_count_sql("SELECT COUNT(1) FROM (SELECT $fields FROM $from WHERE $where) temp WHERE 1 = 1", $params);
list($fields, $from, $where, $params) = $this->update_sql_after_count($fields, $from, $where, $params);
$this->set_sql($fields, $from, $where, $params);
}
/**
* Add the information about the latest state of the question with slot
* $slot to the query.
......@@ -515,8 +544,12 @@ abstract class quiz_attempts_report_table extends table_sql {
if ($this->is_downloading()) {
// We want usages for all attempts.
return new qubaid_join($this->sql->from, 'quiza.uniqueid',
$this->sql->where, $this->sql->params);
return new qubaid_join("(
SELECT DISTINCT quiza.uniqueid
FROM " . $this->sql->from . "
WHERE " . $this->sql->where . "
) quizasubquery", 'quizasubquery.uniqueid',
"1 = 1", $this->sql->params);
}
$qubaids = array();
......
mod/quiz/report/overview/overview_table.php
View file @ 4cc469b2
......@@ -295,6 +295,22 @@ class quiz_overview_table extends quiz_attempts_report_table {
}
}
protected function update_sql_after_count($fields, $from, $where, $params) {
$fields .= ", COALESCE((
SELECT MAX(qqr.regraded)
FROM {quiz_overview_regrades} qqr
WHERE qqr.questionusageid = quiza.uniqueid
), -1) AS regraded";
if ($this->options->onlyregraded) {
$where .= " AND COALESCE((
SELECT MAX(qqr.regraded)
FROM {quiz_overview_regrades} qqr
WHERE qqr.questionusageid = quiza.uniqueid
), -1) <> -1";
}
return [$fields, $from, $where, $params];
}
protected function requires_latest_steps_loaded() {
return $this->options->slotmarks;
}
......
mod/quiz/report/overview/report.php
View file @ 4cc469b2
......@@ -136,26 +136,7 @@ class quiz_overview_report extends quiz_attempts_report {
$hasstudents = $hasstudents && (!$currentgroup || $this->hasgroupstudents);
if ($hasquestions && ($hasstudents || $options->attempts == self::ALL_WITH)) {
// Construct the SQL.
list($fields, $from, $where, $params) = $table->base_sql($allowedjoins);
// The WHERE clause is vital here, because some parts of tablelib.php will expect to
// add bits like ' AND x = 1' on the end, and that needs to leave to valid SQL.
$table->set_count_sql("SELECT COUNT(1) FROM (SELECT $fields FROM $from WHERE $where) temp WHERE 1 = 1", $params);
// Test to see if there are any regraded attempts to be listed.
$fields .= ", COALESCE((
SELECT MAX(qqr.regraded)
FROM {quiz_overview_regrades} qqr
WHERE qqr.questionusageid = quiza.uniqueid
), -1) AS regraded";
if ($options->onlyregraded) {
$where .= " AND COALESCE((
SELECT MAX(qqr.regraded)
FROM {quiz_overview_regrades} qqr
WHERE qqr.questionusageid = quiza.uniqueid
), -1) <> -1";
}
$table->set_sql($fields, $from, $where, $params);
$table->setup_sql_queries($allowedjoins);
if (!$table->is_downloading()) {
// Output the regrade buttons.
......@@ -220,7 +201,7 @@ class quiz_overview_report extends quiz_attempts_report {
$this->add_grade_columns($quiz, $options->usercanseegrades, $columns, $headers, false);
if (!$table->is_downloading() && has_capability('mod/quiz:regrade', $this->context) &&
$this->has_regraded_questions($from, $where, $params)) {
$this->has_regraded_questions($table->sql->from, $table->sql->where, $table->sql->params)) {
$columns[] = 'regraded';
$headers[] = get_string('regrade', 'quiz_overview');
}
......
mod/quiz/report/overview/tests/report_test.php
View file @ 4cc469b2
......@@ -39,10 +39,26 @@ require_once($CFG->dirroot . '/mod/quiz/report/overview/report.php');
*/
class quiz_overview_report_testcase extends advanced_testcase {
public function test_report_sql() {
/**
* Data provider for test_report_sql.
*
* @return array the data for the test sub-cases.
*/
public function report_sql_cases() {
return [[null], ['csv']]; // Only need to test on or off, not all download types.
}
/**
* Test how the report queries the database.
*
* @param bool $isdownloading a download type, or null.
* @dataProvider report_sql_cases
*/
public function test_report_sql($isdownloading) {
global $DB;
$this->resetAfterTest(true);
// Create a course and a quiz.
$generator = $this->getDataGenerator();
$course = $generator->create_course();
$quizgenerator = $generator->get_plugin_generator('mod_quiz');
......@@ -50,54 +66,90 @@ class quiz_overview_report_testcase extends advanced_testcase {
'grademethod' => QUIZ_GRADEHIGHEST, 'grade' => 100.0, 'sumgrades' => 10.0,
'attempts' => 10));
// Add one question.
$questiongenerator = $this->getDataGenerator()->get_plugin_generator('core_question');
$cat = $questiongenerator->create_question_category();
$q = $questiongenerator->create_question('essay', 'plain', ['category' => $cat->id]);
quiz_add_quiz_question($q->id, $quiz, 0 , 10);
// Create some students and enrol them in the course.
$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);
$timestamp = 1234567890;
// This line is not really necessary for the test asserts below,
// but what it does is add an extra user row returned by
// get_enrolled_with_capabilities_join because of a second enrolment.
// The extra row returned used to make $table->query_db complain
// about duplicate records. So this is really a test that an extra
// student enrolment does not cause duplicate records in this query.
$generator->enrol_user($student2->id, $course->id, null, 'self');
// The test data.
$timestamp = 1234567890;
$fields = array('quiz', 'userid', 'attempt', 'sumgrades', 'state');
$attempts = array(
array($quiz->id, $student1->id, 1, 0.0, quiz_attempt::FINISHED),
array($quiz->id, $student1->id, 2, 5.0, quiz_attempt::FINISHED),
array($quiz->id, $student1->id, 3, 8.0, quiz_attempt::FINISHED),
array($quiz->id, $student1->id, 4, null, quiz_attempt::ABANDONED),
array($quiz->id, $student1->id, 5, null, quiz_attempt::IN_PROGRESS),
array($quiz->id, $student2->id, 1, null, quiz_attempt::ABANDONED),
array($quiz->id, $student2->id, 2, null, quiz_attempt::ABANDONED),
array($quiz->id, $student2->id, 3, 7.0, quiz_attempt::FINISHED),
array($quiz->id, $student2->id, 4, null, quiz_attempt::ABANDONED),
array($quiz->id, $student2->id, 5, null, quiz_attempt::ABANDONED),
array($quiz, $student1, 1, 0.0, quiz_attempt::FINISHED),
array($quiz, $student1, 2, 5.0, quiz_attempt::FINISHED),
array($quiz, $student1, 3, 8.0, quiz_attempt::FINISHED),
array($quiz, $student1, 4, null, quiz_attempt::ABANDONED),
array($quiz, $student1, 5, null, quiz_attempt::IN_PROGRESS),
array($quiz, $student2, 1, null, quiz_attempt::ABANDONED),
array($quiz, $student2, 2, null, quiz_attempt::ABANDONED),
array($quiz, $student2, 3, 7.0, quiz_attempt::FINISHED),
array($quiz, $student2, 4, null, quiz_attempt::ABANDONED),
array($quiz, $student2, 5, null, quiz_attempt::ABANDONED),
);
// Load it in to quiz attempts table.
$uniqueid = 1;
foreach ($attempts as $attempt) {
$data = array_combine($fields, $attempt);
$data['timestart'] = $timestamp + 3600 * $data['attempt'];
$data['timemodifed'] = $data['timestart'];
if ($data['state'] == quiz_attempt::FINISHED) {
$data['timefinish'] = $data['timestart'] + 600;
$data['timemodifed'] = $data['timefinish'];
foreach ($attempts as $attemptdata) {
list($quiz, $student, $attemptnumber, $sumgrades, $state) = $attemptdata;
$timestart = $timestamp + $attemptnumber * 3600;
$quizobj = quiz::create($quiz->id, $student->id);
$quba = question_engine::make_questions_usage_by_activity('mod_quiz', $quizobj->get_context());
$quba->set_preferred_behaviour($quizobj->get_quiz()->preferredbehaviour);
// Create the new attempt and initialize the question sessions.
$attempt = quiz_create_attempt($quizobj, $attemptnumber, null, $timestart, false, $student->id);
$attempt = quiz_start_new_attempt($quizobj, $quba, $attempt, $attemptnumber, $timestamp);
$attempt = quiz_attempt_save_started($quizobj, $quba, $attempt);
// Process some responses from the student.
$attemptobj = quiz_attempt::create($attempt->id);
switch ($state) {
case quiz_attempt::ABANDONED:
$attemptobj->process_abandon($timestart + 300, false);
break;
case quiz_attempt::IN_PROGRESS:
// Do nothing.
break;
case quiz_attempt::FINISHED:
// Save answer and finish attempt.
$attemptobj->process_submitted_actions($timestart + 300, false, [
1 => ['answer' => 'My essay by ' . $student->firstname, 'answerformat' => FORMAT_PLAIN]]);
$attemptobj->process_finish($timestart + 600, false);
// Manually grade it.
$quba = $attemptobj->get_question_usage();
$quba->get_question_attempt(1)->manual_grade(
'Comment', $sumgrades, FORMAT_HTML, $timestart + 1200);
question_engine::save_questions_usage_by_activity($quba);
$update = new stdClass();
$update->id = $attemptobj->get_attemptid();
$update->timemodified = $timestart + 1200;
$update->sumgrades = $quba->get_total_mark();
$DB->update_record('quiz_attempts', $update);
quiz_save_best_grade($attemptobj->get_quiz(), $student->id);
break;
}
$data['layout'] = ''; // Not used, but cannot be null.
$data['uniqueid'] = $uniqueid++;
$data['preview'] = 0;
$DB->insert_record('quiz_attempts', $data);
}
// This line is not really necessary for the test asserts below,
// but what it does is add an extra user row returned by
// get_enrolled_with_capabilities_join because of a second enrolment.
// The extra row returned used to make $table->query_db complain
// about duplicate records. So this is really a test that an extra
// student enrolment does not cause duplicate records in this query.
$generator->enrol_user($student2->id, $course->id, null, 'self');
// Actually getting the SQL to run is quite hard. Do a minimal set up of
// some objects.
$context = context_module::instance($quiz->cmid);
......@@ -114,18 +166,23 @@ 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,
$empty, $studentsjoins, array(1), null);
$empty, $studentsjoins, array(1 => $q), null);
$table->download = $isdownloading; // Cannot call the is_downloading API, because it gives errors.
$table->define_columns(array('fullname'));
$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($studentsjoins);
$table->set_sql($fields, $from, $where, $params);
$table->set_count_sql("SELECT COUNT(1) FROM (SELECT $fields FROM $from WHERE $where) temp WHERE 1 = 1", $params);
$table->setup_sql_queries($studentsjoins);
$table->query_db(30, false);
// Should be 4 rows, matching count($table->rawdata) tested below.
// The count is only done if not downloading.
if (!$isdownloading) {
$this->assertEquals(4, $table->totalrows);
}
// Verify what was returned: Student 1's best and in progress attempts.
// Student 2's finshed attempt, and Student 3 with no attempt.
// The array key is {student id}#{attempt number}.
......
mod/quiz/report/responses/report.php
View file @ 4cc469b2
......@@ -149,13 +149,7 @@ class quiz_responses_report extends quiz_attempts_report {
$hasstudents = $hasstudents && (!$currentgroup || $this->hasgroupstudents);
if ($hasquestions && ($hasstudents || $options->attempts == self::ALL_WITH)) {
list($fields, $from, $where, $params) = $table->base_sql($allowedjoins);
// The WHERE clause is vital here, because some parts of tablelib.php will expect to
// add bits like ' AND x = 1' on the end, and that needs to leave to valid SQL.
$table->set_count_sql("SELECT COUNT(1) FROM (SELECT $fields FROM $from WHERE $where) temp WHERE 1 = 1", $params);
$table->set_sql($fields, $from, $where, $params);
$table->setup_sql_queries($allowedjoins);
if (!$table->is_downloading()) {
// Print information on the grading method.
......
Supports Markdown
0% or .
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment