L-electron-Rare/moodle
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

MDL-60162 quiz reports: too many rows fetched on download

Browse Source 
This happened if one user had multiple enrolments in a course, and was
quite inefficient.
This commit is contained in:
Tim Hunt
2017-10-24 18:06:44 +01:00
parent 849405177f
commit ff456b59ea
3 changed files with 113 additions and 38 deletions
Download Patch File Download Diff File Expand all files Collapse all files
mod/quiz/attemptlib.php
+12
View File
@@ -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
+6 -2
View File
@@ -515,8 +515,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/tests/report_test.php
+95 -36
View File
@@ -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,46 +66,19 @@ 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;
// The test data.
$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),
);
// 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'];
}
$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.
@@ -98,6 +87,69 @@ class quiz_overview_report_testcase extends advanced_testcase {
// 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, $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.
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;
}
}
// Actually getting the SQL to run is quite hard. Do a minimal set up of
// some objects.
$context = context_module::instance($quiz->cmid);
@@ -114,7 +166,8 @@ 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'));
@@ -126,6 +179,12 @@ class quiz_overview_report_testcase extends advanced_testcase {
$table->set_count_sql("SELECT COUNT(1) FROM (SELECT $fields FROM $from WHERE $where) temp WHERE 1 = 1", $params);
$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}.