MDL-16625 Detection of whether a quiz has grades was broken in HEAD.

1. Fix this by factoring out a quiz_has_grades function.
2. Fix the quiz unit tests.
3. Fix quiz_has_feedback to return false for ungraded quizzes.
4. Fix use of get_records to do record_exists in quiz reports.
This commit is contained in:
tjhunt
2008-09-23 07:18:15 +00:00
parent 7264519e3f
commit 739b07112e
9 changed files with 126 additions and 44 deletions
+5 -2
View File
@@ -49,12 +49,15 @@ class quiz {
* @param object $quiz the row from the quiz table.
* @param object $cm the course_module object for this quiz.
* @param object $course the row from the course table for the course we belong to.
* @param boolean $getcontext intended for testing - stops the constructor getting the context.
*/
function __construct($quiz, $cm, $course) {
function __construct($quiz, $cm, $course, $getcontext = true) {
$this->quiz = $quiz;
$this->cm = $cm;
$this->course = $course;
$this->context = get_context_instance(CONTEXT_MODULE, $cm->id);
if ($getcontext && !empty($cm->id)) {
$this->context = get_context_instance(CONTEXT_MODULE, $cm->id);
}
$this->determine_layout();
}
+13 -1
View File
@@ -213,6 +213,18 @@ function quiz_user_outline($course, $user, $mod, $quiz) {
return NULL;
}
/**
* Is this a graded quiz? If this method returns true, you can assume that
* $quiz->grade and $quiz->sumgrades are non-zero (for example, if you want to
* divide by them).
*
* @param object $quiz a row from the quiz table.
* @return boolean whether this is a graded quiz.
*/
function quiz_has_grades($quiz) {
return $quiz->grade != 0 && $quiz->sumgrades != 0;
}
/**
* Get the best current grade for a particular user in a quiz.
*
@@ -239,7 +251,7 @@ function quiz_user_complete($course, $user, $mod, $quiz) {
/// a given particular instance of this module, for user activity reports.
if ($attempts = $DB->get_records_select('quiz_attempts', "userid=? AND quiz=?", array($user->id, $quiz->id), 'attempt ASC')) {
if ($quiz->grade && $quiz->sumgrades && $grade = quiz_get_best_grade($quiz, $user->id)) {
if (quiz_has_grades($quiz) && $grade = quiz_get_best_grade($quiz, $user->id)) {
echo get_string('grade') . ': ' . $grade . '/' . $quiz->grade . '<br />';
}
foreach ($attempts as $attempt) {
+7 -5
View File
@@ -420,14 +420,16 @@ function quiz_feedback_for_grade($grade, $quizid) {
* @param integer $quizid the id of the quiz object.
* @return boolean Whether this quiz has any non-blank feedback text.
*/
function quiz_has_feedback($quizid) {
function quiz_has_feedback($quiz) {
global $DB;
static $cache = array();
if (!array_key_exists($quizid, $cache)) {
$cache[$quizid] = $DB->record_exists_select('quiz_feedback',
"quizid = ? AND " . $DB->sql_isnotempty('quiz_feedback', 'feedbacktext', false, true), array($quizid));
if (!array_key_exists($quiz->id, $cache)) {
$cache[$quiz->id] = quiz_has_grades($quiz) &&
$DB->record_exists_select('quiz_feedback', "quizid = ? AND " .
$DB->sql_isnotempty('quiz_feedback', 'feedbacktext', false, true),
array($quiz->id));
}
return $cache[$quizid];
return $cache[$quiz->id];
}
/**
+4 -5
View File
@@ -23,13 +23,13 @@ class quiz_overview_report extends quiz_default_report {
$this->context = get_context_instance(CONTEXT_MODULE, $cm->id);
// Work out some display options - whether there is feedback, and whether scores should be shown.
$hasfeedback = quiz_has_feedback($quiz->id) && $quiz->grade > 1.e-7 && $quiz->sumgrades > 1.e-7;
$hasfeedback = quiz_has_feedback($quiz);
$fakeattempt = new stdClass();
$fakeattempt->preview = false;
$fakeattempt->timefinish = $quiz->timeopen;
$fakeattempt->userid = 0;
$reviewoptions = quiz_get_reviewoptions($quiz, $fakeattempt, $this->context);
$showgrades = ($quiz->grade !== 0) && ($quiz->sumgrades !== 0) && $reviewoptions->scores;
$showgrades = quiz_has_grades($quiz) && $reviewoptions->scores;
$download = optional_param('download', '', PARAM_ALPHA);
@@ -402,15 +402,14 @@ class quiz_overview_report extends quiz_default_report {
if ($currentgroup && $groupstudents){
list($usql, $params) = $DB->get_in_or_equal($groupstudents);
$params[] = $quiz->id;
//should be quicker than a COUNT to test if there is at least one record :
if ($DB->get_records_select('quiz_grades', "userid $usql AND quiz = ?", $params, '', '*', 0, 1)){
if ($DB->record_exists_select('quiz_grades', "userid $usql AND quiz = ?", $params)) {
$imageurl = "{$CFG->wwwroot}/mod/quiz/report/overview/overviewgraph.php?id={$quiz->id}&amp;groupid=$currentgroup";
$graphname = get_string('overviewreportgraphgroup', 'quiz_overview', groups_get_group_name($currentgroup));
print_heading($graphname);
echo '<div class="mdl-align"><img src="'.$imageurl.'" alt="'.$graphname.'" /></div>';
}
}
if ($DB->get_records('quiz_grades', array('quiz'=> $quiz->id), '', '*', 0, 1)){
if ($DB->record_exists('quiz_grades', array('quiz'=> $quiz->id))){
$graphname = get_string('overviewreportgraph', 'quiz_overview');
$imageurl = $CFG->wwwroot.'/mod/quiz/report/overview/overviewgraph.php?id='.$quiz->id;
print_heading($graphname);
+4 -4
View File
@@ -22,13 +22,13 @@ class quiz_responses_report extends quiz_default_report {
$context = get_context_instance(CONTEXT_MODULE, $cm->id);
// Work out some display options - whether there is feedback, and whether scores should be shown.
$hasfeedback = quiz_has_feedback($quiz->id) && $quiz->grade > 1.e-7 && $quiz->sumgrades > 1.e-7;
$hasfeedback = quiz_has_feedback($quiz);
$fakeattempt = new stdClass();
$fakeattempt->preview = false;
$fakeattempt->timefinish = $quiz->timeopen;
$fakeattempt->userid = 0;
$reviewoptions = quiz_get_reviewoptions($quiz, $fakeattempt, $context);
$showgrades = $quiz->grade && $quiz->sumgrades && $reviewoptions->scores;
$showgrades = quiz_has_grades($quiz) && $reviewoptions->scores;
$download = optional_param('download', '', PARAM_ALPHA);
@@ -171,8 +171,8 @@ class quiz_responses_report extends quiz_default_report {
}
$showgrades = $quiz->grade && $quiz->sumgrades && $reviewoptions->scores;
$hasfeedback = quiz_has_feedback($quiz->id) && $quiz->grade > 1.e-7 && $quiz->sumgrades > 1.e-7;
$showgrades = quiz_has_grades($quiz) && $reviewoptions->scores;
$hasfeedback = quiz_has_feedback($quiz);
// Construct the SQL
+1 -1
View File
@@ -166,7 +166,7 @@
/// Show scores (if the user is allowed to see scores at the moment).
$grade = quiz_rescale_grade($attempt->sumgrades, $quiz);
if ($options->scores) {
if ($quiz->grade and $quiz->sumgrades) {
if (quiz_has_grades($quiz)) {
if($overtime) {
$result->sumgrades = "0";
$result->grade = "0.0";
+59 -23
View File
@@ -18,7 +18,9 @@ class simple_rules_test extends MoodleUnitTestCase {
function test_num_attempts_access_rule() {
$quiz = new stdClass;
$quiz->attempts = 3;
$rule = new num_attempts_access_rule($quiz, 0);
$quiz->questions = '';
$quizobj = new quiz($quiz, null, null);
$rule = new num_attempts_access_rule($quizobj, 0);
$attempt = new stdClass;
$this->assertEqual($rule->description(), get_string('attemptsallowedn', 'quiz', 3));
@@ -42,7 +44,9 @@ class simple_rules_test extends MoodleUnitTestCase {
$attempt = new stdClass;
$quiz->subnet = getremoteaddr();
$rule = new ipaddress_access_rule($quiz, 0);
$quiz->questions = '';
$quizobj = new quiz($quiz, null, null);
$rule = new ipaddress_access_rule($quizobj, 0);
$this->assertFalse($rule->prevent_access());
$this->assertFalse($rule->description());
$this->assertFalse($rule->prevent_new_attempt(0, $attempt));
@@ -50,7 +54,9 @@ class simple_rules_test extends MoodleUnitTestCase {
$this->assertFalse($rule->time_left($attempt, 1));
$quiz->subnet = '0.0.0.0';
$rule = new ipaddress_access_rule($quiz, 0);
$quiz->questions = '';
$quizobj = new quiz($quiz, null, null);
$rule = new ipaddress_access_rule($quizobj, 0);
$this->assertTrue($rule->prevent_access());
$this->assertFalse($rule->description());
$this->assertFalse($rule->prevent_new_attempt(0, $attempt));
@@ -61,7 +67,9 @@ class simple_rules_test extends MoodleUnitTestCase {
function test_time_limit_access_rule() {
$quiz = new stdClass;
$quiz->timelimit = 60;
$rule = new time_limit_access_rule($quiz, 10000);
$quiz->questions = '';
$quizobj = new quiz($quiz, null, null);
$rule = new time_limit_access_rule($quizobj, 10000);
$attempt = new stdClass;
$this->assertEqual($rule->description(), get_string('quiztimelimit', 'quiz', format_time(3600)));
@@ -82,9 +90,12 @@ class open_close_date_access_rule_test extends MoodleUnitTestCase {
$quiz = new stdClass;
$quiz->timeopen = 0;
$quiz->timeclose = 0;
$quiz->questions = '';
$quizobj = new quiz($quiz, null, null);
$attempt = new stdClass;
$attempt->preview = 0;
$rule = new open_close_date_access_rule($quiz, 10000);
$rule = new open_close_date_access_rule($quizobj, 10000);
$this->assertFalse($rule->description());
$this->assertFalse($rule->prevent_access());
$this->assertFalse($rule->prevent_new_attempt(0, $attempt));
@@ -92,7 +103,7 @@ class open_close_date_access_rule_test extends MoodleUnitTestCase {
$this->assertFalse($rule->time_left($attempt, 10000));
$this->assertFalse($rule->time_left($attempt, 0));
$rule = new open_close_date_access_rule($quiz, 0);
$rule = new open_close_date_access_rule($quizobj, 0);
$this->assertFalse($rule->description());
$this->assertFalse($rule->prevent_access());
$this->assertFalse($rule->prevent_new_attempt(0, $attempt));
@@ -104,16 +115,19 @@ class open_close_date_access_rule_test extends MoodleUnitTestCase {
$quiz = new stdClass;
$quiz->timeopen = 10000;
$quiz->timeclose = 0;
$quiz->questions = '';
$quizobj = new quiz($quiz, null, null);
$attempt = new stdClass;
$attempt->preview = 0;
$rule = new open_close_date_access_rule($quiz, 9999);
$rule = new open_close_date_access_rule($quizobj, 9999);
$this->assertEqual($rule->description(), array(get_string('quiznotavailable', 'quiz', userdate(10000))));
$this->assertEqual($rule->prevent_access(), get_string('notavailable', 'quiz'));
$this->assertFalse($rule->prevent_new_attempt(0, $attempt));
$this->assertFalse($rule->is_finished(0, $attempt));
$this->assertFalse($rule->time_left($attempt, 0));
$rule = new open_close_date_access_rule($quiz, 10000);
$rule = new open_close_date_access_rule($quizobj, 10000);
$this->assertEqual($rule->description(), array(get_string('quizopenedon', 'quiz', userdate(10000))));
$this->assertFalse($rule->prevent_access());
$this->assertFalse($rule->prevent_new_attempt(0, $attempt));
@@ -125,9 +139,12 @@ class open_close_date_access_rule_test extends MoodleUnitTestCase {
$quiz = new stdClass;
$quiz->timeopen = 0;
$quiz->timeclose = 20000;
$quiz->questions = '';
$quizobj = new quiz($quiz, null, null);
$attempt = new stdClass;
$attempt->preview = 0;
$rule = new open_close_date_access_rule($quiz, 20000);
$rule = new open_close_date_access_rule($quizobj, 20000);
$this->assertEqual($rule->description(), array(get_string('quizcloseson', 'quiz', userdate(20000))));
$this->assertFalse($rule->prevent_access());
$this->assertFalse($rule->prevent_new_attempt(0, $attempt));
@@ -137,7 +154,7 @@ class open_close_date_access_rule_test extends MoodleUnitTestCase {
$this->assertEqual($rule->time_left($attempt, 20000), 0);
$this->assertEqual($rule->time_left($attempt, 20100), -100);
$rule = new open_close_date_access_rule($quiz, 20001);
$rule = new open_close_date_access_rule($quizobj, 20001);
$this->assertEqual($rule->description(), array(get_string('quizclosed', 'quiz', userdate(20000))));
$this->assertEqual($rule->prevent_access(), get_string('notavailable', 'quiz'));
$this->assertFalse($rule->prevent_new_attempt(0, $attempt));
@@ -152,29 +169,32 @@ class open_close_date_access_rule_test extends MoodleUnitTestCase {
$quiz = new stdClass;
$quiz->timeopen = 10000;
$quiz->timeclose = 20000;
$quiz->questions = '';
$quizobj = new quiz($quiz, null, null);
$attempt = new stdClass;
$attempt->preview = 0;
$rule = new open_close_date_access_rule($quiz, 9999);
$rule = new open_close_date_access_rule($quizobj, 9999);
$this->assertEqual($rule->description(), array(get_string('quiznotavailable', 'quiz', userdate(10000))));
$this->assertEqual($rule->prevent_access(), get_string('notavailable', 'quiz'));
$this->assertFalse($rule->prevent_new_attempt(0, $attempt));
$this->assertFalse($rule->is_finished(0, $attempt));
$rule = new open_close_date_access_rule($quiz, 10000);
$rule = new open_close_date_access_rule($quizobj, 10000);
$this->assertEqual($rule->description(), array(get_string('quizopenedon', 'quiz', userdate(10000)),
get_string('quizcloseson', 'quiz', userdate(20000))));
$this->assertFalse($rule->prevent_access());
$this->assertFalse($rule->prevent_new_attempt(0, $attempt));
$this->assertFalse($rule->is_finished(0, $attempt));
$rule = new open_close_date_access_rule($quiz, 20000);
$rule = new open_close_date_access_rule($quizobj, 20000);
$this->assertEqual($rule->description(), array(get_string('quizopenedon', 'quiz', userdate(10000)),
get_string('quizcloseson', 'quiz', userdate(20000))));
$this->assertFalse($rule->prevent_access());
$this->assertFalse($rule->prevent_new_attempt(0, $attempt));
$this->assertFalse($rule->is_finished(0, $attempt));
$rule = new open_close_date_access_rule($quiz, 20001);
$rule = new open_close_date_access_rule($quizobj, 20001);
$this->assertEqual($rule->description(), array(get_string('quizclosed', 'quiz', userdate(20000))));
$this->assertEqual($rule->prevent_access(), get_string('notavailable', 'quiz'));
$this->assertFalse($rule->prevent_new_attempt(0, $attempt));
@@ -194,10 +214,12 @@ class inter_attempt_delay_access_rule_test extends MoodleUnitTestCase {
$quiz->delay1 = 1000;
$quiz->delay2 = 0;
$quiz->timeclose = 0;
$quiz->questions = '';
$quizobj = new quiz($quiz, null, null);
$attempt = new stdClass;
$attempt->timefinish = 10000;
$rule = new inter_attempt_delay_access_rule($quiz, 10000);
$rule = new inter_attempt_delay_access_rule($quizobj, 10000);
$this->assertFalse($rule->description());
$this->assertFalse($rule->prevent_access());
$this->assertFalse($rule->is_finished(0, $attempt));
@@ -221,10 +243,12 @@ class inter_attempt_delay_access_rule_test extends MoodleUnitTestCase {
$quiz->delay1 = 0;
$quiz->delay2 = 1000;
$quiz->timeclose = 0;
$quiz->questions = '';
$quizobj = new quiz($quiz, null, null);
$attempt = new stdClass;
$attempt->timefinish = 10000;
$rule = new inter_attempt_delay_access_rule($quiz, 10000);
$rule = new inter_attempt_delay_access_rule($quizobj, 10000);
$this->assertFalse($rule->description());
$this->assertFalse($rule->prevent_access());
$this->assertFalse($rule->is_finished(0, $attempt));
@@ -251,10 +275,12 @@ class inter_attempt_delay_access_rule_test extends MoodleUnitTestCase {
$quiz->delay1 = 2000;
$quiz->delay2 = 1000;
$quiz->timeclose = 0;
$quiz->questions = '';
$quizobj = new quiz($quiz, null, null);
$attempt = new stdClass;
$attempt->timefinish = 10000;
$rule = new inter_attempt_delay_access_rule($quiz, 10000);
$rule = new inter_attempt_delay_access_rule($quizobj, 10000);
$this->assertFalse($rule->description());
$this->assertFalse($rule->prevent_access());
$this->assertFalse($rule->is_finished(0, $attempt));
@@ -289,10 +315,12 @@ class inter_attempt_delay_access_rule_test extends MoodleUnitTestCase {
$quiz->delay1 = 2000;
$quiz->delay2 = 1000;
$quiz->timeclose = 15000;
$quiz->questions = '';
$quizobj = new quiz($quiz, null, null);
$attempt = new stdClass;
$attempt->timefinish = 13000;
$rule = new inter_attempt_delay_access_rule($quiz, 10000);
$rule = new inter_attempt_delay_access_rule($quizobj, 10000);
$this->assertFalse($rule->description());
$this->assertFalse($rule->prevent_access());
$this->assertFalse($rule->is_finished(0, $attempt));
@@ -307,7 +335,7 @@ class inter_attempt_delay_access_rule_test extends MoodleUnitTestCase {
$attempt->timefinish = 14001;
$this->assertEqual($rule->prevent_new_attempt(2, $attempt), get_string('youcannotwait', 'quiz'));
$rule = new inter_attempt_delay_access_rule($quiz, 15000);
$rule = new inter_attempt_delay_access_rule($quizobj, 15000);
$attempt->timefinish = 13000;
$this->assertFalse($rule->prevent_new_attempt(1, $attempt));
$attempt->timefinish = 13001;
@@ -317,7 +345,7 @@ class inter_attempt_delay_access_rule_test extends MoodleUnitTestCase {
$attempt->timefinish = 14001;
$this->assertEqual($rule->prevent_new_attempt(2, $attempt), get_string('youcannotwait', 'quiz'));
$rule = new inter_attempt_delay_access_rule($quiz, 15001);
$rule = new inter_attempt_delay_access_rule($quizobj, 15001);
$attempt->timefinish = 13000;
$this->assertFalse($rule->prevent_new_attempt(1, $attempt));
$attempt->timefinish = 13001;
@@ -333,7 +361,9 @@ class password_access_rule_test extends MoodleUnitTestCase {
function test_password_access_rule() {
$quiz = new stdClass;
$quiz->password = 'frog';
$rule = new password_access_rule($quiz, 0);
$quiz->questions = '';
$quizobj = new quiz($quiz, null, null);
$rule = new password_access_rule($quizobj, 0);
$attempt = new stdClass;
$this->assertFalse($rule->prevent_access());
@@ -351,7 +381,11 @@ class password_access_rule_test extends MoodleUnitTestCase {
$quiz->id = -1; // So as not to interfere with any real quizzes.
$quiz->intro = 'SOME INTRO TEXT';
$quiz->password = 'frog';
$rule = new password_access_rule($quiz, 0);
$quiz->questions = '';
$cm = new stdClass;
$cm->id = 666;
$quizobj = new quiz($quiz, $cm, null, false);
$rule = new password_access_rule($quizobj, 0);
$rule->clear_access_allowed(-1);
$_POST['cancelpassword'] = false;
@@ -385,7 +419,9 @@ class securewindow_access_rule_test extends MoodleUnitTestCase {
function test_securewindow_access_rule() {
$quiz = new stdClass;
$quiz->popup = 1;
$rule = new securewindow_access_rule($quiz, 0);
$quiz->questions = '';
$quizobj = new quiz($quiz, null, null);
$rule = new securewindow_access_rule($quizobj, 0);
$attempt = new stdClass;
$this->assertFalse($rule->prevent_access());
+30
View File
@@ -0,0 +1,30 @@
<?php
/**
* Unit tests for (some of) mod/quiz/locallib.php.
*
* @author [email protected]
* @license http://www.gnu.org/copyleft/gpl.html GNU Public License
* @package quiz
*/
if (!defined('MOODLE_INTERNAL')) {
die('Direct access to this script is forbidden.'); /// It must be included from a Moodle page.
}
require_once($CFG->dirroot . '/mod/quiz/lib.php');
class quiz_lib_test extends MoodleUnitTestCase {
function test_quiz_has_grades() {
$quiz = new stdClass;
$quiz->grade = '100.0000';
$quiz->sumgrades = '100.0000';
$this->assertTrue(quiz_has_grades($quiz));
$quiz->sumgrades = '0.0000';
$this->assertFalse(quiz_has_grades($quiz));
$quiz->grade = '0.0000';
$this->assertFalse(quiz_has_grades($quiz));
$quiz->sumgrades = '100.0000';
$this->assertFalse(quiz_has_grades($quiz));
}
}
?>
+3 -3
View File
@@ -177,11 +177,11 @@
$attemptcolumn = $quiz->attempts != 1;
$gradecolumn = $someoptions->scores && $quiz->grade && $quiz->sumgrades;
$gradecolumn = $someoptions->scores && quiz_has_grades($quiz);
$markcolumn = $gradecolumn && ($quiz->grade != $quiz->sumgrades);
$overallstats = $alloptions->scores;
$feedbackcolumn = quiz_has_feedback($quiz->id) && $alloptions->overallfeedback;
$feedbackcolumn = quiz_has_feedback($quiz) && $alloptions->overallfeedback;
// Prepare table header
$table->class = 'generaltable quizattemptsummary';
@@ -306,7 +306,7 @@
print_heading(get_string("nomoreattempts", "quiz"));
}
if ($numattempts && $quiz->sumgrades && !is_null($mygrade)) {
if ($numattempts && $gradecolumn && !is_null($mygrade)) {
$resultinfo = '';
if ($overallstats) {