From 4635a07e6abe17c41ef7f4c7f79323914202e2d7 Mon Sep 17 00:00:00 2001 From: Tim Hunt Date: Thu, 28 Sep 2017 15:41:41 +0100 Subject: [PATCH] MDL-60162 quiz reports: refactor duplicated code into the base class This is needed to fix the unit test in the last commit. --- mod/quiz/report/attemptsreport_table.php | 29 +++++++++++++++++++ mod/quiz/report/overview/overview_table.php | 16 ++++++++++ mod/quiz/report/overview/report.php | 23 ++------------- .../report/overview/tests/report_test.php | 4 +-- mod/quiz/report/responses/report.php | 8 +---- 5 files changed, 49 insertions(+), 31 deletions(-) diff --git a/mod/quiz/report/attemptsreport_table.php b/mod/quiz/report/attemptsreport_table.php index ed4ea6655eb..dab6e48bda9 100644 --- a/mod/quiz/report/attemptsreport_table.php +++ b/mod/quiz/report/attemptsreport_table.php @@ -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. diff --git a/mod/quiz/report/overview/overview_table.php b/mod/quiz/report/overview/overview_table.php index 616e1ff22e8..6e0e5ccf793 100644 --- a/mod/quiz/report/overview/overview_table.php +++ b/mod/quiz/report/overview/overview_table.php @@ -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; } diff --git a/mod/quiz/report/overview/report.php b/mod/quiz/report/overview/report.php index 9f6ebc2a4c1..d5d47bd07d0 100644 --- a/mod/quiz/report/overview/report.php +++ b/mod/quiz/report/overview/report.php @@ -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'); } diff --git a/mod/quiz/report/overview/tests/report_test.php b/mod/quiz/report/overview/tests/report_test.php index 6687207d730..0438155eb2a 100644 --- a/mod/quiz/report/overview/tests/report_test.php +++ b/mod/quiz/report/overview/tests/report_test.php @@ -174,9 +174,7 @@ class quiz_overview_report_testcase extends advanced_testcase { $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. diff --git a/mod/quiz/report/responses/report.php b/mod/quiz/report/responses/report.php index 45f393a507c..f72fdbbdfd9 100644 --- a/mod/quiz/report/responses/report.php +++ b/mod/quiz/report/responses/report.php @@ -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.