diff --git a/mod/quiz/report/attemptsreport_table.php b/mod/quiz/report/attemptsreport_table.php index ee660b9..ab4ecb5 100644 --- a/mod/quiz/report/attemptsreport_table.php +++ b/mod/quiz/report/attemptsreport_table.php @@ -387,13 +387,10 @@ abstract class quiz_attempts_report_table extends table_sql { public function base_sql($reportstudents) { global $DB; - $params = array(); $fields = $DB->sql_concat('u.id', "'#'", 'COALESCE(quiza.attempt, 0)') . ' AS uniqueid,'; if ($this->qmsubselect) { - $fields .= "\n(CASE WHEN quiza.state = :gradedattemptstate AND {$this->qmsubselect} - THEN 1 ELSE 0 END) AS gradedattempt,"; - $params['gradedattemptstate'] = quiz_attempt::FINISHED; + $fields .= "\n(CASE WHEN $this->qmsubselect THEN 1 ELSE 0 END) AS gradedattempt,"; } $extrafields = get_extra_user_fields_sql($this->context, 'u', '', @@ -425,7 +422,12 @@ abstract class quiz_attempts_report_table extends table_sql { $from = "\n{user} u"; $from .= "\nLEFT JOIN {quiz_attempts} quiza ON quiza.userid = u.id AND quiza.quiz = :quizid"; - $params['quizid'] = $this->quiz->id; + $params = array('quizid' => $this->quiz->id); + + if ($this->qmsubselect && $this->options->onlygraded) { + $from .= " AND (quiza.state <> :finishedstate OR $this->qmsubselect)"; + $params['finishedstate'] = quiz_attempt::FINISHED; + } switch ($this->options->attempts) { case quiz_attempts_report::ALL_WITH: @@ -462,11 +464,6 @@ abstract class quiz_attempts_report_table extends table_sql { $where .= " AND (quiza.state $statesql OR quiza.state IS NULL)"; } - if ($this->qmsubselect && $this->options->onlygraded) { - $from .= " AND (quiza.state <> :finishedstate OR $this->qmsubselect)"; - $params['finishedstate'] = quiz_attempt::FINISHED; - } - return array($fields, $from, $where, $params); } diff --git a/mod/quiz/report/overview/lang/en/quiz_overview.php b/mod/quiz/report/overview/lang/en/quiz_overview.php index e8bb6dc..dc188c2 100644 --- a/mod/quiz/report/overview/lang/en/quiz_overview.php +++ b/mod/quiz/report/overview/lang/en/quiz_overview.php @@ -41,7 +41,6 @@ $string['optallattempts'] = 'all attempts'; $string['optallstudents'] = 'all {$a} who have or have not attempted the quiz'; $string['optattemptsonly'] = '{$a} who have attempted the quiz'; $string['optnoattemptsonly'] = '{$a} who have not attempted the quiz'; -$string['optonlygradedattempts'] = 'that are graded for each user ({$a})'; $string['optonlyregradedattempts'] = 'that have been regraded / are marked as needing regrading'; $string['overview'] = 'Grades'; $string['overviewdownload'] = 'Overview download'; diff --git a/mod/quiz/report/reportlib.php b/mod/quiz/report/reportlib.php index 16c241f..0bb3b5b 100644 --- a/mod/quiz/report/reportlib.php +++ b/mod/quiz/report/reportlib.php @@ -156,30 +156,33 @@ function quiz_report_qm_filter_select($quiz, $quizattemptsalias = 'quiza') { function quiz_report_grade_method_sql($grademethod, $quizattemptsalias = 'quiza') { switch ($grademethod) { case QUIZ_GRADEHIGHEST : - return "NOT EXISTS (SELECT 1 FROM {quiz_attempts} qa2 + return "($quizattemptsalias.state = 'finished' AND NOT EXISTS ( + SELECT 1 FROM {quiz_attempts} qa2 WHERE qa2.quiz = $quizattemptsalias.quiz AND qa2.userid = $quizattemptsalias.userid AND qa2.state = 'finished' AND ( COALESCE(qa2.sumgrades, 0) > COALESCE($quizattemptsalias.sumgrades, 0) OR (COALESCE(qa2.sumgrades, 0) = COALESCE($quizattemptsalias.sumgrades, 0) AND qa2.attempt < $quizattemptsalias.attempt) - ))"; + )))"; case QUIZ_GRADEAVERAGE : return ''; case QUIZ_ATTEMPTFIRST : - return "NOT EXISTS (SELECT 1 FROM {quiz_attempts} qa2 + return "($quizattemptsalias.state = 'finished' AND NOT EXISTS ( + SELECT 1 FROM {quiz_attempts} qa2 WHERE qa2.quiz = $quizattemptsalias.quiz AND qa2.userid = $quizattemptsalias.userid AND qa2.state = 'finished' AND - qa2.attempt < $quizattemptsalias.attempt)"; + qa2.attempt < $quizattemptsalias.attempt))"; case QUIZ_ATTEMPTLAST : - return "NOT EXISTS (SELECT 1 FROM {quiz_attempts} qa2 + return "($quizattemptsalias.state = 'finished' AND NOT EXISTS ( + SELECT 1 FROM {quiz_attempts} qa2 WHERE qa2.quiz = $quizattemptsalias.quiz AND qa2.userid = $quizattemptsalias.userid AND qa2.state = 'finished' AND - qa2.attempt > $quizattemptsalias.attempt)"; + qa2.attempt > $quizattemptsalias.attempt))"; } } diff --git a/mod/quiz/tests/reportlib_test.php b/mod/quiz/tests/reportlib_test.php index de4b6a6..9986d30 100644 --- a/mod/quiz/tests/reportlib_test.php +++ b/mod/quiz/tests/reportlib_test.php @@ -95,16 +95,17 @@ class mod_quiz_reportlib_testcase extends advanced_testcase { $fakeattempt->userid = 123; $fakeattempt->quiz = 456; $fakeattempt->layout = '1,2,0,3,4,0,5'; + $fakeattempt->state = quiz_attempt::FINISHED; // We intentionally insert these in a funny order, to test the SQL better. // The test data is: - // id | quizid | user | attempt | sumgrades - // ---------------------------------------- - // 4 | 456 | 123 | 1 | 30 - // 2 | 456 | 123 | 2 | 50 - // 1 | 456 | 123 | 3 | 50 - // 3 | 456 | 123 | 4 | null - // 5 | 456 | 1 | 1 | 100 + // id | quizid | user | attempt | sumgrades | state + // --------------------------------------------------- + // 4 | 456 | 123 | 1 | 30 | finished + // 2 | 456 | 123 | 2 | 50 | finished + // 1 | 456 | 123 | 3 | 50 | finished + // 3 | 456 | 123 | 4 | null | inprogress + // 5 | 456 | 1 | 1 | 100 | finished // layout is only given because it has a not-null constraint. // uniqueid values are meaningless, but that column has a unique constraint. @@ -121,11 +122,13 @@ class mod_quiz_reportlib_testcase extends advanced_testcase { $fakeattempt->attempt = 4; $fakeattempt->sumgrades = null; $fakeattempt->uniqueid = 39; + $fakeattempt->state = quiz_attempt::IN_PROGRESS; $DB->insert_record('quiz_attempts', $fakeattempt); $fakeattempt->attempt = 1; $fakeattempt->sumgrades = 30; $fakeattempt->uniqueid = 52; + $fakeattempt->state = quiz_attempt::FINISHED; $DB->insert_record('quiz_attempts', $fakeattempt); $fakeattempt->attempt = 1; @@ -151,7 +154,7 @@ class mod_quiz_reportlib_testcase extends advanced_testcase { . quiz_report_qm_filter_select($quiz), array(123, 456)); $this->assertEquals(1, count($lastattempt)); $lastattempt = reset($lastattempt); - $this->assertEquals(4, $lastattempt->attempt); + $this->assertEquals(3, $lastattempt->attempt); $quiz->attempts = 0; $quiz->grademethod = QUIZ_GRADEHIGHEST;