Skip to content

Commit

Permalink
Merge branch 'MDL-60162' of git://github.com/timhunt/moodle
Browse files Browse the repository at this point in the history
  • Loading branch information
@andrewnicols
andrewnicols committed Oct 26, 2017
2 parents 4d4b8ae + c282157 commit 4cc469b
Show file tree
Hide file tree
Showing 6 changed files with 160 additions and 67 deletions.
12 changes: 12 additions & 0 deletions mod/quiz/attemptlib.php
View file
Expand Up @@ -1093,6 +1093,18 @@ public function get_active_slots($page = 'all') {
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.
Expand Down
37 changes: 35 additions & 2 deletions mod/quiz/report/attemptsreport_table.php
View file
Expand Up @@ -467,6 +467,35 @@ public function base_sql(\core\dml\sql_join $allowedstudentsjoins) {
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.
Expand Down Expand Up @@ -515,8 +544,12 @@ protected function get_qubaids_condition() {

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();
Expand Down
16 changes: 16 additions & 0 deletions mod/quiz/report/overview/overview_table.php
View file
Expand Up @@ -295,6 +295,22 @@ public function col_regraded($attempt) {
}
}

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;
}
Expand Down
23 changes: 2 additions & 21 deletions mod/quiz/report/overview/report.php
View file
Expand Up @@ -136,26 +136,7 @@ public function display($quiz, $cm, $course) {
$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.
Expand Down Expand Up @@ -220,7 +201,7 @@ public function display($quiz, $cm, $course) {
$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');
}
Expand Down
131 changes: 94 additions & 37 deletions mod/quiz/report/overview/tests/report_test.php
View file
Expand Up @@ -39,65 +39,117 @@
*/
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');
$quiz = $quizgenerator->create_instance(array('course' => $course->id,
'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);
Expand All @@ -114,18 +166,23 @@ public function test_report_sql() {

// 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}.
Expand Down
8 changes: 1 addition & 7 deletions mod/quiz/report/responses/report.php
View file
Expand Up @@ -149,13 +149,7 @@ public function display($quiz, $cm, $course) {
$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.
Expand Down

0 comments on commit 4cc469b

Please sign in to comment.