From 75f8c246ee9b361cc8623ec85e43b61746335f1c Mon Sep 17 00:00:00 2001 From: Dragon Dionysius Date: Mon, 4 Feb 2019 02:47:17 +0100 Subject: [PATCH 1/2] Rephrased all SQL queries to follow Moodle SQL style guidelines From [MDL-62822](https://tracker.moodle.org/browse/MDL-62822?focusedCommentId=647722&page=com.atlassian.jira.plugin.system.issuetabpanels%3Acomment-tabpanel#comment-647722) > task/delete_quiz_after_migration.php > [SQL query](https://github.com/lameze/moodle/compare/cb7f6a6f99...MDL-62822-master#diff-eb39085ab85a788c6d8d534538a64140R45) does not follow moodle SQL guidelines. - this commit hasn't changed any query - Only worth mentioning: Simplified the function classes\question\bank\tag_column.php get_extra_joins(). The queries were too identical - updated all queries (which are not in tests) to follow the [Moodle SQL style guidelines](https://docs.moodle.org/dev/SQL_coding_style) - subqueries guidelines are not described on the offial developers page. Followed [this comment](https://tracker.moodle.org/browse/MDLSITE-1914?focusedCommentId=177169&page=com.atlassian.jira.plugin.system.issuetabpanels%3Acomment-tabpanel#comment-177169) as it recieved the most acknowledge - some findings: - classes\privacy\provider.php is full of $DB->execute, maybe change them, where applicable, to corresponding insert/delete/update functions - some places have LIKE, CONCAT statements and such, we should use [the compatibility functions](https://docs.moodle.org/dev/Data_manipulation_API#SQL_compatibility_functions) as much as possible - some places have have inline variable use in SQL queries (search for `" \..*\. "` with regex over the files) --- .../moodle2/backup_studentquiz_stepslib.php | 42 +- classes/privacy/provider.php | 122 ++-- classes/question/bank/approved_column.php | 5 +- classes/question/bank/comments_column.php | 8 +- .../question/bank/difficulty_level_column.php | 78 +- classes/question/bank/performances_column.php | 117 +-- classes/question/bank/rate_column.php | 23 +- .../question/bank/studentquiz_bank_view.php | 10 +- classes/question/bank/tag_column.php | 42 +- classes/task/delete_quiz_after_migration.php | 42 +- locallib.php | 666 +++++++++--------- 11 files changed, 584 insertions(+), 571 deletions(-) diff --git a/backup/moodle2/backup_studentquiz_stepslib.php b/backup/moodle2/backup_studentquiz_stepslib.php index 0a5336c7..5741d767 100644 --- a/backup/moodle2/backup_studentquiz_stepslib.php +++ b/backup/moodle2/backup_studentquiz_stepslib.php @@ -93,13 +93,13 @@ protected function define_structure() { array('id' => backup::VAR_ACTIVITYID, 'coursemodule' => backup::VAR_MODID)); // StudentQuiz Question meta of this StudentQuiz. - $questionsql = 'SELECT question.*' - .' FROM {studentquiz} sq' - .' JOIN {context} con ON( con.instanceid = sq.coursemodule )' - .' JOIN {question_categories} qc ON( qc.contextid = con.id )' - .' LEFT JOIN {question} q ON (q.category = qc.id )' - .' JOIN {studentquiz_question} question ON (question.questionid = q.id)' - .' WHERE sq.id = :studentquizid'; + $questionsql = "SELECT question.* + FROM {studentquiz} sq + JOIN {context} con ON con.instanceid = sq.coursemodule + JOIN {question_categories} qc ON qc.contextid = con.id + LEFT JOIN {question} q ON q.category = qc.id + JOIN {studentquiz_question} question ON question.questionid = q.id + WHERE sq.id = :studentquizid"; $question->set_source_sql($questionsql, array('studentquizid' => backup::VAR_PARENTID)); // Define data sources with user data. @@ -112,23 +112,23 @@ protected function define_structure() { array('studentquizid' => backup::VAR_PARENTID)); // Only select rates to questions of this StudentQuiz. - $ratesql = 'SELECT rate.*' - .' FROM {studentquiz} sq' - .' JOIN {context} con ON( con.instanceid = sq.coursemodule )' - .' JOIN {question_categories} qc ON( qc.contextid = con.id )' - .' LEFT JOIN {question} q ON (q.category = qc.id )' - .' JOIN {studentquiz_rate} rate ON (rate.questionid = q.id)' - .' WHERE sq.id = :studentquizid'; + $ratesql = "SELECT rate.* + FROM {studentquiz} sq + JOIN {context} con ON con.instanceid = sq.coursemodule + JOIN {question_categories} qc ON qc.contextid = con.id + LEFT JOIN {question} q ON q.category = qc.id + JOIN {studentquiz_rate} rate ON rate.questionid = q.id + WHERE sq.id = :studentquizid"; $rate->set_source_sql($ratesql, array('studentquizid' => backup::VAR_PARENTID)); // Only select comments to questions of this StudentQuiz. - $commentsql = 'SELECT comment.*' - .' FROM {studentquiz} sq' - .' JOIN {context} con ON( con.instanceid = sq.coursemodule )' - .' JOIN {question_categories} qc ON( qc.contextid = con.id )' - .' LEFT JOIN {question} q ON (q.category = qc.id )' - .' JOIN {studentquiz_comment} comment ON (comment.questionid = q.id)' - .' WHERE sq.id = :studentquizid'; + $commentsql = "SELECT comment.* + FROM {studentquiz} sq + JOIN {context} con ON con.instanceid = sq.coursemodule + JOIN {question_categories} qc ON qc.contextid = con.id + LEFT JOIN {question} q ON q.category = qc.id + JOIN {studentquiz_comment} comment ON comment.questionid = q.id + WHERE sq.id = :studentquizid"; $comment->set_source_sql($commentsql, array('studentquizid' => backup::VAR_PARENTID)); } diff --git a/classes/privacy/provider.php b/classes/privacy/provider.php index d22d4979..220d7399 100755 --- a/classes/privacy/provider.php +++ b/classes/privacy/provider.php @@ -100,7 +100,7 @@ public static function get_contexts_for_userid(int $userid) : contextlist { // Get activity context if user created/modified the question or their data exist in these table // base on user ID field: rate, comment, progress, practice, attempt. - $sql = 'SELECT DISTINCT ctx.id + $sql = "SELECT DISTINCT ctx.id FROM {context} ctx JOIN {studentquiz} sq ON sq.coursemodule = ctx.instanceid AND contextlevel = :contextmodule @@ -114,19 +114,23 @@ public static function get_contexts_for_userid(int $userid) : contextlist { LEFT JOIN {studentquiz_practice} practice ON practice.studentquizcoursemodule = sq.coursemodule LEFT JOIN {studentquiz_attempt} attempt ON attempt.categoryid = ca.id AND attempt.studentquizid = sq.id - WHERE (question.id IS NOT NULL - OR rate.id IS NOT NULL - OR comment.id IS NOT NULL - OR progress.questionid IS NOT NULL - OR practice.id IS NOT NULL - OR attempt.id IS NOT NULL) - AND (q.createdby = :createduser - OR q.modifiedby = :modifieduser - OR rate.userid = :rateuser - OR comment.userid = :commentuser - OR progress.userid = :progressuser - OR practice.userid = :practiceuser - OR attempt.userid = :attemptuser)'; + WHERE ( + question.id IS NOT NULL + OR rate.id IS NOT NULL + OR comment.id IS NOT NULL + OR progress.questionid IS NOT NULL + OR practice.id IS NOT NULL + OR attempt.id IS NOT NULL + ) + AND ( + q.createdby = :createduser + OR q.modifiedby = :modifieduser + OR rate.userid = :rateuser + OR comment.userid = :commentuser + OR progress.userid = :progressuser + OR practice.userid = :practiceuser + OR attempt.userid = :attemptuser + )"; $params = [ 'contextmodule' => CONTEXT_MODULE, @@ -163,19 +167,19 @@ public static function export_user_data(approved_contextlist $contextlist) { list($contextsql, $contextparam) = $DB->get_in_or_equal($contextlist->get_contextids(), SQL_PARAMS_NAMED); - $sql = "SELECT DISTINCT ctx.id as contextid, - q.id as questionid, q.name as questionname, question.approved as questionapproved, - q.createdby as questioncreatedby, q.modifiedby as questionmodifiedby, - rate.id as rateid, rate.rate as raterate, rate.questionid as ratequestionid, rate.userid as rateuserid, - comment.id as commentid, comment.comment as commentcomment, comment.questionid as commentquestionid, - comment.userid as commentuserid, comment.created as commentcreate, - progress.questionid as progressquestionid, progress.userid as progressuserid, - progress.studentquizid as progressstudentquizid, progress.lastanswercorrect as progresslastanswercorrect, - progress.attempts as progressattempts, progress.correctattempts as progresscorrectattempts, - practice.id as practiceid, practice.quizcoursemodule as practicequizcoursemodule, - practice.studentquizcoursemodule as practicestudentquizcoursemodule, practice.userid as practiceuserid, - attempt.id as attemptid, attempt.studentquizid as attempstudentquizid,attempt.userid as attemptuserid, - attempt.questionusageid as attemptquestionusageid, attempt.categoryid as attemptcategoryid + $sql = "SELECT DISTINCT ctx.id AS contextid, + q.id AS questionid, q.name AS questionname, question.approved AS questionapproved, + q.createdby AS questioncreatedby, q.modifiedby AS questionmodifiedby, + rate.id AS rateid, rate.rate AS raterate, rate.questionid AS ratequestionid, rate.userid AS rateuserid, + comment.id AS commentid, comment.comment AS commentcomment, comment.questionid AS commentquestionid, + comment.userid AS commentuserid, comment.created AS commentcreate, + progress.questionid AS progressquestionid, progress.userid AS progressuserid, + progress.studentquizid AS progressstudentquizid, progress.lastanswercorrect AS progresslastanswercorrect, + progress.attempts AS progressattempts, progress.correctattempts AS progresscorrectattempts, + practice.id AS practiceid, practice.quizcoursemodule AS practicequizcoursemodule, + practice.studentquizcoursemodule AS practicestudentquizcoursemodule, practice.userid AS practiceuserid, + attempt.id AS attemptid, attempt.studentquizid AS attempstudentquizid,attempt.userid AS attemptuserid, + attempt.questionusageid AS attemptquestionusageid, attempt.categoryid AS attemptcategoryid FROM {context} ctx JOIN {studentquiz} sq ON sq.coursemodule = ctx.instanceid AND contextlevel = :contextmodule @@ -189,19 +193,23 @@ public static function export_user_data(approved_contextlist $contextlist) { LEFT JOIN {studentquiz_practice} practice ON practice.studentquizcoursemodule = sq.coursemodule LEFT JOIN {studentquiz_attempt} attempt ON attempt.categoryid = ca.id AND attempt.studentquizid = sq.id - WHERE (question.id IS NOT NULL - OR rate.id IS NOT NULL - OR comment.id IS NOT NULL - OR progress.questionid IS NOT NULL - OR practice.id IS NOT NULL - OR attempt.id IS NOT NULL) - AND (q.createdby = :createduser - OR q.modifiedby = :modifieduser - OR rate.userid = :rateuser - OR comment.userid = :commentuser - OR progress.userid = :progressuser - OR practice.userid = :practiceuser - OR attempt.userid = :attemptuser) + WHERE ( + question.id IS NOT NULL + OR rate.id IS NOT NULL + OR comment.id IS NOT NULL + OR progress.questionid IS NOT NULL + OR practice.id IS NOT NULL + OR attempt.id IS NOT NULL + ) + AND ( + q.createdby = :createduser + OR q.modifiedby = :modifieduser + OR rate.userid = :rateuser + OR comment.userid = :commentuser + OR progress.userid = :progressuser + OR practice.userid = :practiceuser + OR attempt.userid = :attemptuser + ) AND ctx.id {$contextsql} ORDER BY ctx.id ASC"; @@ -323,11 +331,13 @@ public static function delete_data_for_all_users_in_context(\context $context) { } // Query to get all question ID belong to this module context. - $sql = 'SELECT q.id + $sql = "SELECT q.id FROM {question} q - WHERE q.category IN (SELECT id - FROM {question_categories} c - WHERE c.contextid = :contextid)'; + WHERE q.category IN ( + SELECT id + FROM {question_categories} c + WHERE c.contextid = :contextid + )"; $params = [ 'contextid' => $context->id @@ -381,9 +391,11 @@ public static function delete_data_for_all_users_in_context(\context $context) { // Delete attempts belong to this context. $DB->execute("DELETE FROM {studentquiz_attempt} - WHERE studentquizid IN (SELECT id - FROM {studentquiz} - WHERE coursemodule = :coursemodule)", [ + WHERE studentquizid IN ( + SELECT id + FROM {studentquiz} + WHERE coursemodule = :coursemodule + )", [ 'coursemodule' => $context->instanceid ]); } @@ -411,9 +423,11 @@ public static function delete_data_for_user(approved_contextlist $contextlist) { // Query to get all question ID belong to the course modules. $sql = "SELECT q.id FROM {question} q - WHERE q.category IN (SELECT id - FROM {question_categories} c - WHERE c.contextid {$contextsql})"; + WHERE q.category IN ( + SELECT id + FROM {question_categories} c + WHERE c.contextid {$contextsql} + )"; $records = $DB->get_records_sql($sql, $contextparam); @@ -471,9 +485,11 @@ public static function delete_data_for_user(approved_contextlist $contextlist) { // Delete attempts belong to user within approved context. $DB->execute("DELETE FROM {studentquiz_attempt} WHERE userid = :userid - AND studentquizid IN (SELECT id - FROM {studentquiz} - WHERE coursemodule {$studentquizsql})", [ + AND studentquizid IN ( + SELECT id + FROM {studentquiz} + WHERE coursemodule {$studentquizsql} + )", [ 'userid' => $userid ] + $sudentquizparams); } diff --git a/classes/question/bank/approved_column.php b/classes/question/bank/approved_column.php index 92a08f3f..6949598f 100644 --- a/classes/question/bank/approved_column.php +++ b/classes/question/bank/approved_column.php @@ -75,7 +75,10 @@ protected function display_content($question, $rowclasses) { * @return array modified select left join */ public function get_extra_joins() { - return array('ap' => ' LEFT JOIN (SELECT questionid, approved FROM {studentquiz_question}) ap ON ap.questionid = q.id'); + return array('ap' => " LEFT JOIN ( + SELECT questionid, approved + FROM {studentquiz_question} + ) ap ON ap.questionid = q.id"); } /** diff --git a/classes/question/bank/comments_column.php b/classes/question/bank/comments_column.php index 7a89ffba..1ffc5087 100644 --- a/classes/question/bank/comments_column.php +++ b/classes/question/bank/comments_column.php @@ -75,9 +75,11 @@ protected function display_content($question, $rowclasses) { * @return array modified select left join */ public function get_extra_joins() { - return array('co' => 'LEFT JOIN (' - . 'SELECT COUNT(comment) as comment' - . ', questionid FROM {studentquiz_comment} GROUP BY questionid) co ON co.questionid = q.id'); + return array('co' => "LEFT JOIN ( + SELECT COUNT(comment) AS comment, questionid + FROM {studentquiz_comment} + GROUP BY questionid + ) co ON co.questionid = q.id"); } /** diff --git a/classes/question/bank/difficulty_level_column.php b/classes/question/bank/difficulty_level_column.php index 8fea57a6..bfadd5be 100644 --- a/classes/question/bank/difficulty_level_column.php +++ b/classes/question/bank/difficulty_level_column.php @@ -87,41 +87,48 @@ public function get_sqlparams() { */ public function get_extra_joins() { if ($this->studentquiz->aggregated) { - return array('dl' => 'LEFT JOIN (' - . ' SELECT ROUND(1 - AVG(correctattempts / attempts), 2) AS difficultylevel, questionid ' - . ' FROM {studentquiz_progress} ' - . ' WHERE studentquizid = ' . $this->studentquizid . ' ' - . ' GROUP BY questionid) dl ON dl.questionid = q.id'); + return array('dl' => "LEFT JOIN ( + SELECT ROUND(1 - AVG(correctattempts / attempts), 2) AS difficultylevel, + questionid + FROM {studentquiz_progress} + WHERE studentquizid = " . $this->studentquizid . " + GROUP BY questionid + ) dl ON dl.questionid = q.id"); } else { - return array('dl' => 'LEFT JOIN (' - . 'SELECT ' - . ' ROUND(1-avg(case qas.state when \'gradedright\' then 1 else 0 end),2) as difficultylevel,' - . ' questionid' - . ' FROM {studentquiz_attempt} sqa ' - . ' JOIN {question_usages} qu ON qu.id = sqa.questionusageid ' - . ' JOIN {question_attempts} qa ON qa.questionusageid = qu.id' - . ' JOIN {question_attempt_steps} qas ON qas.questionattemptid = qa.id' - . ' LEFT JOIN {question_attempt_step_data} qasd ON qasd.attemptstepid = qas.id' - . ' WHERE sqa.studentquizid = ' . $this->studentquizid - . ' AND qasd.name=\'-submit\'' - . ' AND (qas.state = \'gradedright\' OR qas.state = \'gradedwrong\' OR qas.state=\'gradedpartial\')' - . ' GROUP BY qa.questionid) dl ON dl.questionid = q.id', - 'mydiffs' => 'LEFT JOIN (' - . 'SELECT ' - . ' ROUND(1-avg(case state when \'gradedright\' then 1 else 0 end),2) as mydifficulty,' - . ' sum(case state when \'gradedright\' then 1 else 0 end) as mycorrectattempts,' - . ' questionid' - . ' FROM {studentquiz_attempt} sqa ' - . ' JOIN {question_usages} qu ON qu.id = sqa.questionusageid ' - . ' JOIN {question_attempts} qa ON qa.questionusageid = qu.id' - . ' JOIN {question_attempt_steps} qas ON qas.questionattemptid = qa.id' - . ' LEFT JOIN {question_attempt_step_data} qasd ON qasd.attemptstepid = qas.id' - . ' WHERE sqa.userid = ' . $this->currentuserid - . ' AND sqa.studentquizid = ' . $this->studentquizid - . ' AND qasd.name=\'-submit\'' - . ' AND (qas.state = \'gradedright\' OR qas.state = \'gradedwrong\' OR qas.state=\'gradedpartial\')' - . ' GROUP BY qa.questionid) mydiffs ON mydiffs.questionid = q.id' - ); + return array('dl' => "LEFT JOIN ( + SELECT ROUND(1-avg(case qas.state when 'gradedright' then 1 else 0 end),2) + AS difficultylevel, + questionid + FROM {studentquiz_attempt} sqa + JOIN {question_usages} qu ON qu.id = sqa.questionusageid + JOIN {question_attempts} qa ON qa.questionusageid = qu.id + JOIN {question_attempt_steps} qas ON qas.questionattemptid = qa.id + LEFT JOIN {question_attempt_step_data} qasd ON qasd.attemptstepid = qas.id + WHERE sqa.studentquizid = " . $this->studentquizid . " + AND qasd.name='-submit' + AND (qas.state = 'gradedright' + OR qas.state = 'gradedwrong' + OR qas.state='gradedpartial') + GROUP BY qa.questionid + ) dl ON dl.questionid = q.id", + 'mydiffs' => "LEFT JOIN ( + SELECT ROUND(1-avg(case state when \'gradedright\' then 1 else 0 end),2) + AS mydifficulty, + sum(case state when \'gradedright\' then 1 else 0 end) AS mycorrectattempts, + questionid + FROM {studentquiz_attempt} sqa + JOIN {question_usages} qu ON qu.id = sqa.questionusageid + JOIN {question_attempts} qa ON qa.questionusageid = qu.id + JOIN {question_attempt_steps} qas ON qas.questionattemptid = qa.id + LEFT JOIN {question_attempt_step_data} qasd ON qasd.attemptstepid = qas.id + WHERE sqa.userid = " . $this->currentuserid . " + AND sqa.studentquizid = " . $this->studentquizid . " + AND qasd.name='-submit' + AND (qas.state = 'gradedright' + OR qas.state = 'gradedwrong' + OR qas.state = 'gradedpartial') + GROUP BY qa.questionid + ) mydiffs ON mydiffs.questionid = q.id"); } } @@ -131,7 +138,8 @@ public function get_extra_joins() { */ public function get_required_fields() { if ($this->studentquiz->aggregated) { - return array('dl.difficultylevel', 'ROUND(1 - (sp.correctattempts / sp.attempts),2) mydifficulty', '(sp.correctattempts) mycorrectattempts'); + return array('dl.difficultylevel', 'ROUND(1 - (sp.correctattempts / sp.attempts),2) AS mydifficulty', + 'sp.correctattempts AS mycorrectattempts'); } else { return array('dl.difficultylevel', 'mydiffs.mydifficulty', 'mydiffs.mycorrectattempts'); } diff --git a/classes/question/bank/performances_column.php b/classes/question/bank/performances_column.php index 8c1575a7..86b3623b 100644 --- a/classes/question/bank/performances_column.php +++ b/classes/question/bank/performances_column.php @@ -100,57 +100,62 @@ public function get_sqlparams() { */ public function get_extra_joins() { if ($this->studentquiz->aggregated) { - return array('sp' => 'LEFT JOIN {studentquiz_progress} sp ON sp.questionid = q.id AND sp.userid = '. $this->currentuserid - . ' AND sp.studentquizid = ' . $this->studentquizid); + return array('sp' => "LEFT JOIN {studentquiz_progress} sp ON sp.questionid = q.id + AND sp.userid = " . $this->currentuserid . " + AND sp.studentquizid = " . $this->studentquizid); } else { - // Add outer WHERE tests here to limit the dataset to just the module question category. - $tests = array('qa.responsesummary IS NOT NULL', 'q.parent = 0', 'q.hidden = 0', 'q.category = ' . $this->categoryid); - return array('pr' => 'LEFT JOIN (' - . 'SELECT COUNT(questionid) as practice' - . ', questionid FROM {question_attempts} qa JOIN {question} q ON qa.questionid = q.id' - . ' WHERE ' . implode(' AND ', $tests) - . ' GROUP BY qa.questionid) pr ON pr.questionid = q.id', - 'myatts' => 'LEFT JOIN (' - . 'SELECT COUNT(*) myattempts, questionid' - .' FROM {studentquiz} sq ' - .' JOIN {studentquiz_attempt} sqa on sqa.studentquizid = sq.id' - . ' JOIN {question_usages} qu ON qu.id = sqa.questionusageid' - . ' JOIN {question_attempts} qa ON qa.questionusageid = qu.id' - . ' JOIN {question_attempt_steps} qas ON qas.questionattemptid = qa.id' - . ' LEFT JOIN {question_attempt_step_data} qasd ON qasd.attemptstepid = qas.id' - . ' WHERE qasd.name = \'-submit\'' - .' AND sq.id = ' . $this->studentquizid - .' AND sqa.userid = ' . $this->currentuserid - .' AND (qas.state = \'gradedright\' OR qas.state = \'gradedwrong\' OR qas.state=\'gradedpartial\')' - . ' GROUP BY qa.questionid) myatts ON myatts.questionid = q.id', - 'mylastattempt' => 'LEFT JOIN (' - .'SELECT' - .' qa.questionid,' - .' qas.state mylastattempt' - .' FROM' - .' {studentquiz} sq ' - .' JOIN {studentquiz_attempt} sqa on sqa.studentquizid = sq.id' - .' JOIN {question_usages} qu on qu.id = sqa.questionusageid ' - .' JOIN {question_attempts} qa on qa.questionusageid = qu.id ' - .' LEFT JOIN {question_attempt_steps} qas on qas.questionattemptid = qa.id' - .' LEFT JOIN {question_attempt_step_data} qasd on qasd.attemptstepid = qas.id' - .' INNER JOIN (' - .' SELECT MAX(qasd.id) maxqasdid' - .' FROM {studentquiz} sq ' - .' JOIN {studentquiz_attempt} sqa on sqa.studentquizid = sq.id' - .' JOIN {question_usages} qu on qu.id = sqa.questionusageid ' - .' JOIN {question_attempts} qa on qa.questionusageid = qu.id ' - .' LEFT JOIN {question_attempt_steps} qas on qas.questionattemptid = qa.id' - .' LEFT JOIN {question_attempt_step_data} qasd on qasd.attemptstepid = qas.id' - .' WHERE qasd.name = \'-submit\'' - .' AND sq.id = ' . $this->studentquizid - .' AND sqa.userid = ' . $this->currentuserid - .' AND qas.fraction is not null' - .' GROUP BY qa.questionid' - .' ) qasdmax on qasd.id = qasdmax.maxqasdid' - .' WHERE qasd.name = \'-submit\'' - . ') mylatts ON mylatts.questionid = q.id' - ); + return array('pr' => "LEFT JOIN ( + SELECT COUNT(questionid) AS practice, questionid + FROM {question_attempts} qa + JOIN {question} q ON qa.questionid = q.id + WHERE qa.responsesummary IS NOT NULL + AND q.parent = 0 + AND q.hidden = 0 + AND q.category = " . $this->categoryid . " + GROUP BY qa.questionid + ) pr ON pr.questionid = q.id", + 'myatts' => "LEFT JOIN ( + SELECT COUNT(*) AS myattempts, questionid + FROM {studentquiz} sq + JOIN {studentquiz_attempt} sqa ON sqa.studentquizid = sq.id + JOIN {question_usages} qu ON qu.id = sqa.questionusageid + JOIN {question_attempts} qa ON qa.questionusageid = qu.id + JOIN {question_attempt_steps} qas ON qas.questionattemptid = qa.id + LEFT JOIN {question_attempt_step_data} qasd ON qasd.attemptstepid = qas.id + WHERE qasd.name = '-submit' + AND sq.id = " . $this->studentquizid . " + AND sqa.userid = " . $this->currentuserid . " + AND ( + qas.state = 'gradedright' + OR qas.state = 'gradedwrong' + OR qas.state='gradedpartial' + ) + GROUP BY qa.questionid + ) myatts ON myatts.questionid = q.id", + 'mylastattempt' => "LEFT JOIN ( + SELECT qa.questionid, qas.state mylastattempt + FROM {studentquiz} sq + JOIN {studentquiz_attempt} sqa ON sqa.studentquizid = sq.id + JOIN {question_usages} qu ON qu.id = sqa.questionusageid + JOIN {question_attempts} qa ON qa.questionusageid = qu.id + LEFT JOIN {question_attempt_steps} qas ON qas.questionattemptid = qa.id + LEFT JOIN {question_attempt_step_data} qasd ON qasd.attemptstepid = qas.id + INNER JOIN ( + SELECT MAX(qasd.id) maxqasdid + FROM {studentquiz} sq + JOIN {studentquiz_attempt} sqa ON sqa.studentquizid = sq.id + JOIN {question_usages} qu ON qu.id = sqa.questionusageid + JOIN {question_attempts} qa ON qa.questionusageid = qu.id + LEFT JOIN {question_attempt_steps} qas ON qas.questionattemptid = qa.id + LEFT JOIN {question_attempt_step_data} qasd ON qasd.attemptstepid = qas.id + WHERE qasd.name = '-submit' + AND sq.id = " . $this->studentquizid . " + AND sqa.userid = " . $this->currentuserid . " + AND qas.fraction is not null + GROUP BY qa.questionid + ) qasdmax ON qasd.id = qasdmax.maxqasdid + WHERE qasd.name = '-submit' + ) mylatts ON mylatts.questionid = q.id"); } } @@ -160,8 +165,16 @@ public function get_extra_joins() { */ public function get_required_fields() { if ($this->studentquiz->aggregated) { - return array('sp.attempts practice', 'sp.attempts as myattempts', '(CASE WHEN sp.attempts is null THEN \'\' ELSE - CASE WHEN sp.lastanswercorrect = 1 THEN \'gradedright\' ELSE \'gradedwrong\' END END) mylastattempt'); + return array('sp.attempts practice', 'sp.attempts AS myattempts', + "( + CASE WHEN sp.attempts is null + THEN '' + ELSE CASE WHEN sp.lastanswercorrect = 1 + THEN 'gradedright' + ELSE 'gradedwrong' + END + END + ) AS mylastattempt"); } else { return array('pr.practice', 'myatts.myattempts', 'mylatts.mylastattempt'); } diff --git a/classes/question/bank/rate_column.php b/classes/question/bank/rate_column.php index c6a15b0b..2c128b70 100644 --- a/classes/question/bank/rate_column.php +++ b/classes/question/bank/rate_column.php @@ -83,18 +83,17 @@ protected function display_content($question, $rowclasses) { * @return array modified select left join */ public function get_extra_joins() { - return array('vo' => 'LEFT JOIN (' - .'SELECT ROUND(avg(rate), 2) as rate' - .', questionid FROM {studentquiz_rate} GROUP BY questionid) vo ON vo.questionid = q.id', - 'myrate' => 'LEFT JOIN (' - . 'SELECT ' - . ' rate myrate, ' - . ' q.id questionid' - . ' FROM {question} q' - . ' LEFT JOIN {studentquiz_rate} rate on q.id = rate.questionid' - . ' AND rate.userid = ' . $this->currentuserid - . ' ) myrate ON myrate.questionid = q.id' - ); + return array('vo' => "LEFT JOIN ( + SELECT ROUND(avg(rate), 2) AS rate, questionid + FROM {studentquiz_rate} + GROUP BY questionid + ) vo ON vo.questionid = q.id", + 'myrate' => "LEFT JOIN ( + SELECT rate AS myrate, q.id AS questionid + FROM {question} q + LEFT JOIN {studentquiz_rate} rate ON q.id = rate.questionid + AND rate.userid = " . $this->currentuserid . " + ) myrate ON myrate.questionid = q.id"); } /** diff --git a/classes/question/bank/studentquiz_bank_view.php b/classes/question/bank/studentquiz_bank_view.php index 7b76bcb9..e6f2b3cd 100755 --- a/classes/question/bank/studentquiz_bank_view.php +++ b/classes/question/bank/studentquiz_bank_view.php @@ -253,11 +253,11 @@ public function process_actions() { } if ($questionids) { list($usql, $params) = $DB->get_in_or_equal($questionids); - $questions = $DB->get_records_sql(" - SELECT q.*, c.contextid - FROM {question} q - JOIN {question_categories} c ON c.id = q.category - WHERE q.id {$usql}", $params); + $sql = "SELECT q.*, c.contextid + FROM {question} q + JOIN {question_categories} c ON c.id = q.category + WHERE q.id {$usql}"; + $questions = $DB->get_records_sql($sql, $params); foreach ($questions as $question) { question_require_capability_on($question, 'move'); } diff --git a/classes/question/bank/tag_column.php b/classes/question/bank/tag_column.php index 40657fed..75167406 100644 --- a/classes/question/bank/tag_column.php +++ b/classes/question/bank/tag_column.php @@ -103,33 +103,21 @@ protected function display_content($question, $rowclasses) { * @return array sql query join additional */ public function get_extra_joins() { - if ($this->tagfilteractive) { - return array('tags' => 'LEFT JOIN (' - .' SELECT ' - .' ti.itemid questionid,' - .' COUNT(*) tags,' - .' SUM(CASE WHEN t.name LIKE :searchtag then 1 else 0 end) searchtag' - .' FROM {tag} t ' - .' JOIN {tag_instance} ti ON (t.id = ti.tagid' - .' AND ti.itemid IN (SELECT id FROM {question} q' - .' WHERE q.category = ' . $this->categoryid . '))' - .' WHERE ti.itemtype = \'question\'' - .' GROUP BY questionid' - . ') tags ON tags.questionid = q.id '); - } else { - return array('tags' => 'LEFT JOIN (' - .' SELECT ' - .' ti.itemid questionid,' - .' COUNT(*) tags,' - .' 0 searchtag' - .' FROM {tag} t ' - .' JOIN {tag_instance} ti ON (t.id = ti.tagid' - .' AND ti.itemid IN (SELECT id FROM {question} q' - .' WHERE q.category = ' . $this->categoryid . '))' - .' WHERE ti.itemtype = \'question\'' - .' GROUP BY questionid' - . ') tags ON tags.questionid = q.id '); - } + $searchtag = ($this->tagfilteractive) ? "SUM(CASE WHEN t.name LIKE :searchtag then 1 else 0 end)" : "0"; + return array('tags' => "LEFT JOIN ( + SELECT ti.itemid AS questionid, COUNT(*) AS tags, " . $searchtag . " AS searchtag + FROM {tag} t + JOIN {tag_instance} ti ON ( + t.id = ti.tagid + AND ti.itemid IN ( + SELECT id FROM {question} q + WHERE q.category = " . + $this->categoryid . " + ) + ) + WHERE ti.itemtype = 'question' + GROUP BY questionid + ) tags ON tags.questionid = q.id"); } /** diff --git a/classes/task/delete_quiz_after_migration.php b/classes/task/delete_quiz_after_migration.php index 2129ab71..a3628ce6 100644 --- a/classes/task/delete_quiz_after_migration.php +++ b/classes/task/delete_quiz_after_migration.php @@ -41,32 +41,22 @@ public function execute() { // Search if there is an orphaned quiz, which has the same name as an studentquiz and is in the same course, // but is in no section anymore. - $orphanedquiz = $DB->get_record_sql(' - select - q.id as quizid, - q.name as quizname, - cmq.course as quizcourseid, - csq.id as quizsectionid, - s.id as studentquizid, - s.name as studentquizname, - cms.course as studentquizcourseid - from {modules} ms - inner join {course_modules} cms on ms.id = cms.module - inner join {studentquiz} s on cms.instance = s.id - left join {course_modules} cmq on cms.course = cmq.course - inner join {quiz} q on cmq.instance = q.id - inner join {modules} mq on cmq.module = mq.id - left join {course_sections} csq on cmq.section = csq.id - where ms.name = \'studentquiz\' - and mq.name = \'quiz\' - and csq.id is null - and q.name like concat(s.name, \'%\') - order by - cms.course, - s.id, - q.id - limit 1 - '); + $sql = "SELECT q.id AS quizid, q.name AS quizname, cmq.course AS quizcourseid, csq.id AS quizsectionid, + s.id AS studentquizid, s.name AS studentquizname, cms.course AS studentquizcourseid + FROM {modules} ms + INNER JOIN {course_modules} cms ON ms.id = cms.module + INNER JOIN {studentquiz} s ON cms.instance = s.id + LEFT JOIN {course_modules} cmq ON cms.course = cmq.course + INNER JOIN {quiz} q ON cmq.instance = q.id + INNER JOIN {modules} mq ON cmq.module = mq.id + LEFT JOIN {course_sections} csq ON cmq.section = csq.id + WHERE ms.name = 'studentquiz' + AND mq.name = 'quiz' + AND csq.id IS NULL + AND q.name LIKE concat(s.name, '%') + ORDER BY cms.course, s.id, q.id + LIMIT 1"; + $orphanedquiz = $DB->get_record_sql($sql); // We have found a orphaned quiz, remove it. if ($orphanedquiz !== false) { diff --git a/locallib.php b/locallib.php index 30071fc9..376467b2 100644 --- a/locallib.php +++ b/locallib.php @@ -138,29 +138,29 @@ function mod_studentquiz_get_studentquiz_progress_from_question_attempts_steps($ * */ function mod_studentquiz_get_studentquiz_progress_from_question_attempts_steps_sql($studentquizid) { - $sql = <<get_records_sql($sql, array( 'questionid' => $questionid)); } @@ -564,7 +565,7 @@ function mod_studentquiz_get_comments_with_creators($questionid) { /** * Generate some HTML to render comments * - * @param array $comments from studentquiz_coments joind with user.firstname, user.lastname on comment.userid + * @param array $comments from studentquiz_coments joind with user.firstname, user.lastname ON comment.userid * ordered by comment->created ASC * @param int $userid, viewing user id * @param bool $anonymize Display or hide other author names @@ -729,56 +730,60 @@ function mod_studentquiz_user_stats($cmid, $quantifiers, $userid, $aggregated) { * TODO: Refactor: There must be a better way to do this! */ function mod_studentquiz_helper_attempt_stat_select() { - return 'SELECT ' - .' statspercategory.userid userid,' - .' statspercategory.firstname firstname,' - .' statspercategory.lastname lastname,' - // Aggregate values over all categories in cm context. - // Note: Max() of equals is faster than Sum() of groups. - // See: https://dev.mysql.com/doc/refman/5.7/en/group-by-optimization.html. - .' MAX(points) points,' - .' MAX(questions_created) questions_created,' - .' MAX(questions_created_and_rated) questions_created_and_rated,' - .' MAX(questions_approved) questions_approved,' - .' MAX(rates_received) rates_received,' - .' MAX(rates_average) rates_average,' - .' MAX(question_attempts) question_attempts,' - .' MAX(question_attempts_correct) question_attempts_correct,' - .' MAX(question_attempts_incorrect) question_attempts_incorrect,' - .' MAX(last_attempt_exists) last_attempt_exists,' - .' MAX(last_attempt_correct) last_attempt_correct,' - .' MAX(last_attempt_incorrect) last_attempt_incorrect' - // Select for each question category in context. - .' FROM ( SELECT ' - .' u.id userid,' - .' u.firstname firstname,' - .' u.lastname lastname,' - .' qc.id category, ' - // Calculate points. - .' COALESCE ( ROUND(' - .' COALESCE(creators.countq, 0) * :questionquantifier ' // Questions created. - .'+ COALESCE(approvals.countq, 0) * :approvedquantifier ' // Questions approved. - .'+ COALESCE(rates.avgv, 0) * (COALESCE(creators.countq, 0) - COALESCE(rates.not_rated_questions, 0)) * :ratequantifier ' // Rating. - .'+ COALESCE(lastattempt.last_attempt_correct, 0) * :correctanswerquantifier ' // Correct answers. - .'+ COALESCE(lastattempt.last_attempt_incorrect, 0) * :incorrectanswerquantifier ' // Incorrect answers. - .' , 1) , 0) points, ' - // Questions created. - .' COALESCE(creators.countq, 0) questions_created,' - // Questions created and rated. - .' COALESCE(COALESCE(creators.countq, 0) - COALESCE(rates.not_rated_questions, 0), 0) questions_created_and_rated,' - // Questions approved. - .' COALESCE(approvals.countq, 0) questions_approved,' - // Questions rating received. - .' COALESCE(rates.countv, 0) rates_received,' - .' COALESCE(rates.avgv, 0) rates_average,' - // Question attempts. - .' COALESCE(attempts.counta, 0) question_attempts,' - .' COALESCE(attempts.countright, 0) question_attempts_correct,' - .' COALESCE(attempts.countwrong, 0) question_attempts_incorrect,' - // Last attempt. - .' COALESCE(lastattempt.last_attempt_exists, 0) last_attempt_exists,' - .' COALESCE(lastattempt.last_attempt_correct, 0) last_attempt_correct,' - .' COALESCE(lastattempt.last_attempt_incorrect, 0) last_attempt_incorrect'; + return "SELECT statspercategory.userid AS userid, statspercategory.firstname AS firstname, + statspercategory.lastname AS lastname, + -- Aggregate values over all categories in cm context. + -- Note: Max() of equals is faster than Sum() of groups. + -- See: https://dev.mysql.com/doc/refman/5.7/en/group-by-optimization.html. + MAX(points) AS points, MAX(questions_created) AS questions_created, + MAX(questions_created_and_rated) AS questions_created_and_rated, MAX(questions_approved) AS questions_approved, + MAX(rates_received) AS rates_received, MAX(rates_average) AS rates_average, + MAX(question_attempts) AS question_attempts, MAX(question_attempts_correct) AS question_attempts_correct, + MAX(question_attempts_incorrect) AS question_attempts_incorrect, + MAX(last_attempt_exists) AS last_attempt_exists, MAX(last_attempt_correct) AS last_attempt_correct, + MAX(last_attempt_incorrect) AS last_attempt_incorrect + -- Select for each question category in context. + FROM ( + SELECT u.id AS userid, u.firstname AS firstname, u.lastname AS lastname, qc.id AS category, + -- Calculate points. + COALESCE ( + ROUND ( + -- Questions created. + COALESCE(creators.countq, 0) * :questionquantifier + + -- Questions approved. + COALESCE(approvals.countq, 0) * :approvedquantifier + + -- Rating. + COALESCE(rates.avgv, 0) * (COALESCE(creators.countq, 0) - + COALESCE(rates.not_rated_questions, 0)) * :ratequantifier + + -- Correct answers. + COALESCE(lastattempt.last_attempt_correct, 0) * :correctanswerquantifier + + -- Incorrect answers. + COALESCE(lastattempt.last_attempt_incorrect, 0) * :incorrectanswerquantifier, + 1 + ), + 0 + ) AS points, + -- Questions created. + COALESCE(creators.countq, 0) AS questions_created, + -- Questions created and rated. + COALESCE(COALESCE(creators.countq, 0) - COALESCE(rates.not_rated_questions, 0), + 0) AS questions_created_and_rated, + -- Questions approved. + COALESCE(approvals.countq, 0) AS questions_approved, + -- Questions rating received. + COALESCE(rates.countv, 0) AS rates_received, + COALESCE(rates.avgv, 0) AS rates_average, + -- Question attempts. + COALESCE(attempts.counta, 0) AS question_attempts, + COALESCE(attempts.countright, 0) AS question_attempts_correct, + COALESCE(attempts.countwrong, 0) AS question_attempts_incorrect, + -- Last attempt. + COALESCE(lastattempt.last_attempt_exists, 0) AS last_attempt_exists, + COALESCE(lastattempt.last_attempt_correct, 0) AS last_attempt_correct, + COALESCE(lastattempt.last_attempt_incorrect, 0) AS last_attempt_incorrect + -- WARNING: the trailing ) is intentionally missing, found in mod_studentquiz_user_stats var statsbycat + -- Following newline is intentional because this string is concatenated + "; } /** @@ -786,145 +791,135 @@ function mod_studentquiz_helper_attempt_stat_select() { * TODO: Refactor: There must be a better way to do this! */ function mod_studentquiz_helper_attempt_stat_joins($aggregated) { - $sql = ' FROM {studentquiz} sq' - // Get this Studentquiz Question category. - . ' JOIN {context} con ON con.instanceid = sq.coursemodule' - . ' JOIN {question_categories} qc ON qc.contextid = con.id' - // Only enrolled users. - . ' JOIN {course} c ON c.id = sq.course' - . ' JOIN {enrol} e ON e.courseid = c.id' - . ' JOIN {user_enrolments} ue ON ue.enrolid = e.id' - . ' JOIN {user} u ON ue.userid = u.id' - // Question created by user. - . ' LEFT JOIN' - . ' ( SELECT count(*) countq, q.createdby creator' - . ' FROM {studentquiz} sq' - . ' JOIN {context} con ON con.instanceid = sq.coursemodule' - . ' JOIN {question_categories} qc ON qc.contextid = con.id' - . ' JOIN {question} q on q.category = qc.id' - . ' WHERE q.hidden = 0 AND q.parent = 0 AND sq.coursemodule = :cmid4' - . ' GROUP BY creator' - . ' ) creators ON creators.creator = u.id' - // Approved questions. - . ' LEFT JOIN' - . ' ( SELECT count(*) countq, q.createdby creator' - . ' FROM {studentquiz} sq' - . ' JOIN {context} con ON con.instanceid = sq.coursemodule' - . ' JOIN {question_categories} qc ON qc.contextid = con.id' - . ' JOIN {question} q on q.category = qc.id' - . ' JOIN {studentquiz_question} sqq ON q.id = sqq.questionid' - . ' where q.hidden = 0 AND q.parent = 0 AND sqq.approved = 1 AND sq.coursemodule = :cmid5' - . ' group by creator' - . ' ) approvals ON approvals.creator = u.id' - // Average of Average Rating of own questions. - . ' LEFT JOIN' - . ' (SELECT' - . ' createdby,' - . ' AVG(avg_rate_perq) avgv,' - . ' SUM(num_rate_perq) countv,' - . ' SUM(question_not_rated) not_rated_questions' - . ' FROM (' - . ' SELECT' - . ' q.id,' - . ' q.createdby createdby,' - . ' AVG(sqv.rate) avg_rate_perq,' - . ' COUNT(sqv.rate) num_rate_perq,' - . ' MAX(CASE WHEN sqv.id is null then 1 else 0 end) question_not_rated' - . ' FROM {studentquiz} sq' - . ' JOIN {context} con on con.instanceid = sq.coursemodule' - . ' JOIN {question_categories} qc on qc.contextid = con.id' - . ' JOIN {question} q on q.category = qc.id' - . ' LEFT JOIN {studentquiz_rate} sqv on q.id = sqv.questionid' - . ' WHERE' - . ' q.hidden = 0 AND q.parent = 0' - . ' and sq.coursemodule = :cmid6' - . ' GROUP BY q.id, q.createdby' - . ' ) avgratingperquestion' - . ' GROUP BY createdby' - . ' ) rates ON rates.createdby = u.id'; + $sql = " FROM {studentquiz} sq + -- Get this Studentquiz Question category. + JOIN {context} con ON con.instanceid = sq.coursemodule + JOIN {question_categories} qc ON qc.contextid = con.id + -- Only enrolled users. + JOIN {course} c ON c.id = sq.course + JOIN {enrol} e ON e.courseid = c.id + JOIN {user_enrolments} ue ON ue.enrolid = e.id + JOIN {user} u ON ue.userid = u.id + -- Question created by user. + LEFT JOIN ( + SELECT count(*) AS countq, q.createdby AS creator + FROM {studentquiz} sq + JOIN {context} con ON con.instanceid = sq.coursemodule + JOIN {question_categories} qc ON qc.contextid = con.id + JOIN {question} q ON q.category = qc.id + WHERE q.hidden = 0 + AND q.parent = 0 + AND sq.coursemodule = :cmid4 + GROUP BY creator + ) creators ON creators.creator = u.id + -- Approved questions. + LEFT JOIN ( + SELECT count(*) AS countq, q.createdby AS creator + FROM {studentquiz} sq + JOIN {context} con ON con.instanceid = sq.coursemodule + JOIN {question_categories} qc ON qc.contextid = con.id + JOIN {question} q ON q.category = qc.id + JOIN {studentquiz_question} sqq ON q.id = sqq.questionid + WHERE q.hidden = 0 + AND q.parent = 0 + AND sqq.approved = 1 + AND sq.coursemodule = :cmid5 + GROUP BY creator + ) approvals ON approvals.creator = u.id + -- Average of Average Rating of own questions. + LEFT JOIN ( + SELECT createdby, AVG(avg_rate_perq) AS avgv, SUM(num_rate_perq) AS countv, + SUM(question_not_rated) AS not_rated_questions + FROM ( + SELECT q.id, q.createdby AS createdby, AVG(sqv.rate) AS avg_rate_perq, + COUNT(sqv.rate) AS num_rate_perq, + MAX(CASE WHEN sqv.id IS NULL THEN 1 ELSE 0 END) AS question_not_rated + FROM {studentquiz} sq + JOIN {context} con ON con.instanceid = sq.coursemodule + JOIN {question_categories} qc ON qc.contextid = con.id + JOIN {question} q ON q.category = qc.id + LEFT JOIN {studentquiz_rate} sqv ON q.id = sqv.questionid + WHERE q.hidden = 0 + AND q.parent = 0 + AND sq.coursemodule = :cmid6 + GROUP BY q.id, q.createdby + ) avgratingperquestion + GROUP BY createdby + ) rates ON rates.createdby = u.id"; if ($aggregated) { - $sql .= ' LEFT JOIN (SELECT' - . ' sp.userid,' - . ' COUNT(*) last_attempt_exists,' - . ' SUM(lastanswercorrect) last_attempt_correct,' - . ' SUM(1 - lastanswercorrect) last_attempt_incorrect' - . ' FROM' - . ' {studentquiz_progress} AS sp' - . ' JOIN {studentquiz} sq ON sq.id = sp.studentquizid' - . ' WHERE' - . ' sq.coursemodule = :cmid2' - . ' GROUP BY sp.userid) lastattempt ON lastattempt.userid = u.id' - . ' LEFT JOIN (SELECT' - . ' SUM(attempts) counta,' - . ' SUM(correctattempts) countright,' - . ' SUM(attempts - correctattempts) countwrong,' - . ' sp.userid userid' - . ' FROM' - . ' {studentquiz_progress} AS sp' - . ' JOIN {studentquiz} sq ON sq.id = sp.studentquizid' - . ' WHERE' - . ' sq.coursemodule = :cmid1' - . ' GROUP BY sp.userid) attempts ON attempts.userid = u.id'; + $sql .= " + LEFT JOIN ( + SELECT sp.userid, COUNT(*) AS last_attempt_exists, SUM(lastanswercorrect) AS last_attempt_correct, + SUM(1 - lastanswercorrect) AS last_attempt_incorrect + FROM {studentquiz_progress} sp + JOIN {studentquiz} sq ON sq.id = sp.studentquizid + WHERE sq.coursemodule = :cmid2 + GROUP BY sp.userid + ) lastattempt ON lastattempt.userid = u.id + LEFT JOIN ( + SELECT SUM(attempts) AS counta, SUM(correctattempts) AS countright, + SUM(attempts - correctattempts) AS countwrong, sp.userid AS userid + FROM {studentquiz_progress} sp + JOIN {studentquiz} sq ON sq.id = sp.studentquizid + WHERE sq.coursemodule = :cmid1 + GROUP BY sp.userid + ) attempts ON attempts.userid = u.id"; } else { - $sql .= ' LEFT JOIN' - . ' (' - . ' SELECT count(*) counta,' - . ' SUM(CASE WHEN state = \'gradedright\' THEN 1 ELSE 0 END) countright,' - . ' SUM(CASE WHEN qas.state = \'gradedwrong\' THEN 1 WHEN qas.state = \'gradedpartial\' THEN 1 ELSE 0 END) countwrong,' - . ' sqa.userid userid' - . ' FROM {studentquiz} sq' - . ' JOIN {studentquiz_attempt} sqa ON sq.id = sqa.studentquizid' - . ' JOIN {question_usages} qu ON qu.id = sqa.questionusageid' - . ' JOIN {question_attempts} qa ON qa.questionusageid = qu.id' - . ' JOIN {question_attempt_steps} qas ON qas.questionattemptid = qa.id' - . ' LEFT JOIN {question_attempt_step_data} qasd ON qasd.attemptstepid = qas.id' - . ' WHERE sq.coursemodule = :cmid7' - . ' AND qas.state in (\'gradedright\', \'gradedwrong\', \'gradedpartial\')' - // Only count grading triggered by submits. - . ' AND qasd.name = \'-submit\'' - . ' group by sqa.userid' - . ' ) attempts ON attempts.userid = u.id' - // Latest attempts. - . ' LEFT JOIN' - . ' (' - . ' SELECT' - . ' sqa.userid,' - . ' count(*) last_attempt_exists,' - . ' SUM(CASE WHEN qas.state = \'gradedright\' THEN 1 ELSE 0 END) last_attempt_correct,' - . ' SUM(CASE ' - . ' WHEN qas.state = \'gradedwrong\' THEN 1' - . ' WHEN qas.state = \'gradedpartial\' THEN 1 ELSE 0 END) last_attempt_incorrect' - . ' FROM {studentquiz} sq' - . ' JOIN {studentquiz_attempt} sqa ON sq.id = sqa.studentquizid' - . ' JOIN {question_usages} qu ON qu.id = sqa.questionusageid' - . ' JOIN {question_attempts} qa ON qa.questionusageid = qu.id' - . ' JOIN {question_attempt_steps} qas ON qas.questionattemptid = qa.id' - . ' LEFT JOIN {question_attempt_step_data} qasd ON' - . ' qasd.attemptstepid = qas.id and' - . ' qasd.id in (' - // SELECT only latest states (its a constant result). - . ' SELECT max(qasd.id) latest_grading_event' - . ' FROM {studentquiz} sq' - . ' JOIN {studentquiz_attempt} sqa ON sq.id = sqa.studentquizid' - . ' JOIN {question_usages} qu ON qu.id = sqa.questionusageid' - . ' JOIN {question_attempts} qa ON qa.questionusageid = qu.id' - . ' JOIN {question_attempt_steps} qas ON qas.questionattemptid = qa.id' - . ' JOIN {question} qq ON qq.id = qa.questionid' - . ' LEFT JOIN {question_attempt_step_data} qasd ON qasd.attemptstepid = qas.id' - . ' WHERE sq.coursemodule = :cmid1' - . ' AND qas.state in (\'gradedright\', \'gradedwrong\', \'gradedpartial\')' - . ' AND qasd.name = \'-submit\'' - . ' group by sqa.userid, questionid' - . ' )' - . ' WHERE sq.coursemodule = :cmid2' - . ' AND qas.state in (\'gradedright\', \'gradedpartial\', \'gradedwrong\')' - // Only count grading triggered by submits. - . ' AND qasd.name = \'-submit\'' - . ' group by sqa.userid' - . ' ) lastattempt ON lastattempt.userid = u.id'; + $sql .= " + LEFT JOIN ( + SELECT count(*) AS counta, SUM(CASE WHEN state = 'gradedright' THEN 1 ELSE 0 END) AS countright, + SUM(CASE WHEN qas.state = 'gradedwrong' THEN 1 WHEN qas.state = 'gradedpartial' THEN 1 ELSE 0 END) + AS countwrong, + sqa.userid AS userid + FROM {studentquiz} sq + JOIN {studentquiz_attempt} sqa ON sq.id = sqa.studentquizid + JOIN {question_usages} qu ON qu.id = sqa.questionusageid + JOIN {question_attempts} qa ON qa.questionusageid = qu.id + JOIN {question_attempt_steps} qas ON qas.questionattemptid = qa.id + LEFT JOIN {question_attempt_step_data} qasd ON qasd.attemptstepid = qas.id + WHERE sq.coursemodule = :cmid7 + AND qas.state IN ('gradedright', 'gradedwrong', 'gradedpartial') + -- Only count grading triggered by submits. + AND qasd.name = '-submit' + GROUP BY sqa.userid + ) attempts ON attempts.userid = u.id + -- Latest attempts. + LEFT JOIN ( + SELECT sqa.userid, count(*) AS last_attempt_exists, + SUM(CASE WHEN qas.state = 'gradedright' THEN 1 ELSE 0 END) AS last_attempt_correct, + SUM(CASE WHEN qas.state = 'gradedwrong' THEN 1 WHEN qas.state = 'gradedpartial' THEN 1 ELSE 0 END) + AS last_attempt_incorrect + FROM {studentquiz} sq + JOIN {studentquiz_attempt} sqa ON sq.id = sqa.studentquizid + JOIN {question_usages} qu ON qu.id = sqa.questionusageid + JOIN {question_attempts} qa ON qa.questionusageid = qu.id + JOIN {question_attempt_steps} qas ON qas.questionattemptid = qa.id + LEFT JOIN {question_attempt_step_data} qasd ON qasd.attemptstepid = qas.id + AND qasd.id IN ( + -- SELECT only latest states (its a constant result). + SELECT max(qasd.id) AS latest_grading_event + FROM {studentquiz} sq + JOIN {studentquiz_attempt} sqa ON sq.id = sqa.studentquizid + JOIN {question_usages} qu ON qu.id = sqa.questionusageid + JOIN {question_attempts} qa ON qa.questionusageid = qu.id + JOIN {question_attempt_steps} qas ON qas.questionattemptid = qa.id + JOIN {question} qq ON qq.id = qa.questionid + LEFT JOIN {question_attempt_step_data} qasd ON qasd.attemptstepid = qas.id + WHERE sq.coursemodule = :cmid1 + AND qas.state IN ('gradedright', 'gradedwrong', 'gradedpartial') + AND qasd.name = '-submit' + GROUP BY sqa.userid, questionid + ) + WHERE sq.coursemodule = :cmid2 + AND qas.state IN ('gradedright', 'gradedpartial', 'gradedwrong') + -- Only count grading triggered by submits. + AND qasd.name = '-submit' + GROUP BY sqa.userid + ) lastattempt ON lastattempt.userid = u.id"; } // Question attempts: sum of number of graded attempts per question. - $sql .= ' WHERE sq.coursemodule = :cmid3'; + $sql .= " + WHERE sq.coursemodule = :cmid3"; return $sql; } @@ -1014,28 +1009,26 @@ function mod_studentquiz_migrate_old_quiz_usage($courseorigid=null) { if (!empty($courseorigid)) { $courseids[] = $courseorigid; } else { - $courseids = $DB->get_fieldset_sql(' - select distinct cm.course - from {course_modules} cm - inner join {context} c on cm.id = c.instanceid - inner join {question_categories} qc on qc.contextid = c.id - inner join {modules} m on cm.module = m.id - where m.name = :modulename - ', array( + $sql = "SELECT DISTINCT cm.course + FROM {course_modules} cm + INNER JOIN {context} c ON cm.id = c.instanceid + INNER JOIN {question_categories} qc ON qc.contextid = c.id + INNER JOIN {modules} m ON cm.module = m.id + WHERE m.name = :modulename"; + $courseids = $DB->get_fieldset_sql($sql, array( 'modulename' => 'studentquiz' )); } // Step into each course so they operate independent from each other. foreach ($courseids as $courseid) { - // Import old Core Quiz Data (question attempts) to studentquiz. + // Import old core quiz data (question attempts) to studentquiz. // This is the case, when orphaned section(s) can be found. - $orphanedsectionids = $DB->get_fieldset_sql(' - select id - from {course_sections} - where course = :course - and name = :name - ', array( + $sql = "SELECT id + FROM {course_sections} + WHERE course = :course + AND name = :name"; + $orphanedsectionids = $DB->get_fieldset_sql($sql, array( 'course' => $courseid, 'name' => STUDENTQUIZ_COURSE_SECTION_NAME )); @@ -1045,24 +1038,24 @@ function mod_studentquiz_migrate_old_quiz_usage($courseorigid=null) { // For each course we need to find the studentquizzes. // "up" section: Only get the topmost category of that studentquiz, which isn't "top" if that one exists. - $studentquizzes = $DB->get_records_sql(' - select s.id, s.name, cm.id as cmid, c.id as contextid, qc.id as categoryid, qc.name as categoryname, qc.parent - from {studentquiz} s - inner join {course_modules} cm on s.id = cm.instance - inner join {context} c on cm.id = c.instanceid - inner join {question_categories} qc on qc.contextid = c.id - inner join {modules} m on cm.module = m.id - left join {question_categories} up on qc.contextid = up.contextid and qc.parent = up.id - where m.name = :modulename - and cm.course = :course - and ( - up.name = :topname1 - or ( - up.id is null - and qc.name <> :topname2 - ) - ) - ', array( + $sql = "SELECT s.id, s.name, cm.id AS cmid, c.id AS contextid, qc.id AS categoryid, qc.name AS categoryname, qc.parent + FROM {studentquiz} s + INNER JOIN {course_modules} cm ON s.id = cm.instance + INNER JOIN {context} c ON cm.id = c.instanceid + INNER JOIN {question_categories} qc ON qc.contextid = c.id + INNER JOIN {modules} m ON cm.module = m.id + LEFT JOIN {question_categories} up ON qc.contextid = up.contextid + AND qc.parent = up.id + WHERE m.name = :modulename + AND cm.course = :course + AND ( + up.name = :topname1 + OR ( + up.id IS NULL + AND qc.name <> :topname2 + ) + )"; + $studentquizzes = $DB->get_records_sql($sql, array( 'modulename' => 'studentquiz', 'course' => $courseid, 'topname1' => 'top', @@ -1072,17 +1065,16 @@ function mod_studentquiz_migrate_old_quiz_usage($courseorigid=null) { foreach ($studentquizzes as $studentquiz) { // Each studentquiz wants the question attempt id, which can be found inside the matching quizzes. - $oldusages = $DB->get_records_sql(' - select qu.id as qusageid, q.id as quizid, cm.id as cmid, cm.section as sectionid, c.id as contextid - from {quiz} q - inner join {course_modules} cm on q.id = cm.instance - inner join {context} c on cm.id = c.instanceid - inner join {modules} m on cm.module = m.id - inner join {question_usages} qu on c.id = qu.contextid - where m.name = :modulename - and cm.course = :course - and ' . $DB->sql_like('q.name', ':name', false) . ' - ', array( + $sql = "SELECT qu.id AS qusageid, q.id AS quizid, cm.id AS cmid, cm.section AS sectionid, c.id AS contextid + FROM {quiz} q + INNER JOIN {course_modules} cm ON q.id = cm.instance + INNER JOIN {context} c ON cm.id = c.instanceid + INNER JOIN {modules} m ON cm.module = m.id + INNER JOIN {question_usages} qu ON c.id = qu.contextid + WHERE m.name = :modulename + AND cm.course = :course + AND " . $DB->sql_like('q.name', ':name', false); + $oldusages = $DB->get_records_sql($sql, array( 'modulename' => 'quiz', 'course' => $courseid, 'name' => $studentquiz->name . '%' @@ -1101,12 +1093,11 @@ function mod_studentquiz_migrate_old_quiz_usage($courseorigid=null) { array('questionusageid' => $oldusage->qusageid)); // Now we need each user as own attempt. - $userids = $DB->get_fieldset_sql(' - select distinct qas.userid - from {question_attempt_steps} qas - inner join {question_attempts} qa on qas.questionattemptid = qa.id - where qa.questionusageid = :qusageid - ', array( + $sql = "SELECT DISTINCT qas.userid + FROM {question_attempt_steps} qas + INNER JOIN {question_attempts} qa ON qas.questionattemptid = qa.id + WHERE qa.questionusageid = :qusageid"; + $userids = $DB->get_fieldset_sql($sql, array( 'qusageid' => $oldusage->qusageid )); foreach ($userids as $userid) { @@ -1131,18 +1122,17 @@ function mod_studentquiz_migrate_old_quiz_usage($courseorigid=null) { $orphanedsectionids[] = 0; // Force multiple entries, so next command makes a IN statement in every case. list($insql, $inparams) = $DB->get_in_or_equal($orphanedsectionids, SQL_PARAMS_NAMED, 'section'); - $lastnonemptysection = $DB->get_record_sql(' - SELECT MAX(s.section) as max_section - FROM {course_sections} s - left join {course_modules} m on s.id = m.section - where s.course = :course - and s.id NOT ' . $insql . ' - and ( - m.id is not NULL - or s.name <> :sectionname - or s.summary <> :sectionsummary - ) - ', array_merge($inparams, array( + $sql = "SELECT MAX(s.section) AS max_section + FROM {course_sections} s + LEFT JOIN {course_modules} m ON s.id = m.section + WHERE s.course = :course + AND s.id NOT " . $insql . " + AND ( + m.id IS NOT NULL + OR s.name <> :sectionname + OR s.summary <> :sectionsummary + )"; + $lastnonemptysection = $DB->get_record_sql($sql, array_merge($inparams, array( 'course' => $courseid, 'sectionname' => '', 'sectionsummary' => '' @@ -1188,11 +1178,11 @@ function mod_studentquiz_get_tags_by_question_ids($ids) { list($insql, $params) = $DB->get_in_or_equal($ids); $result = array(); - $tags = $DB->get_records_sql( - 'SELECT ti.id id, t.id tagid, t.name, t.rawname, ti.itemid ' - . ' FROM {tag} t JOIN {tag_instance} ti ON ti.tagid = t.id ' - . ' WHERE ti.itemtype = \'question\' AND ti.itemid ' - . $insql, $params); + $sql = "SELECT ti.id AS id, t.id AS tagid, t.name, t.rawname, ti.itemid + FROM {tag} t + JOIN {tag_instance} ti ON ti.tagid = t.id + WHERE ti.itemtype = 'question' AND ti.itemid " . $insql; + $tags = $DB->get_records_sql($sql, $params); foreach ($tags as $tag) { if (empty($result[$tag->itemid])) { $result[$tag->itemid] = array(); @@ -1204,13 +1194,17 @@ function mod_studentquiz_get_tags_by_question_ids($ids) { function mod_studentquiz_count_questions($cmid) { global $DB; - $rs = $DB->count_records_sql('SELECT count(*) FROM {studentquiz} sq' - // Get this Studentquiz Question category. - .' JOIN {context} con ON con.instanceid = sq.coursemodule' - .' JOIN {question_categories} qc ON qc.contextid = con.id' - // Only enrolled users. - .' JOIN {question} q ON q.category = qc.id' - .' WHERE q.hidden = 0 AND q.parent = 0 AND sq.coursemodule = :cmid', array('cmid' => $cmid)); + $sql = "SELECT COUNT(*) + FROM {studentquiz} sq + -- Get this StudentQuiz question category. + JOIN {context} con ON con.instanceid = sq.coursemodule + JOIN {question_categories} qc ON qc.contextid = con.id + -- Only enrolled users. + JOIN {question} q ON q.category = qc.id + WHERE q.hidden = 0 + AND q.parent = 0 + AND sq.coursemodule = :cmid"; + $rs = $DB->count_records_sql($sql, array('cmid' => $cmid)); return $rs; } @@ -1222,36 +1216,36 @@ function mod_studentquiz_count_questions($cmid) { */ function mod_studentquiz_question_stats($cmid) { global $DB; - $sql = 'SELECT count(*) questions_available,' - .' avg(rating.avg_rating) as average_rating,' - .' sum(sqq.approved) as questions_approved' - .' FROM {studentquiz} sq' - // Get this Studentquiz Question category. - .' JOIN {context} con ON con.instanceid = sq.coursemodule' - .' JOIN {question_categories} qc ON qc.contextid = con.id' - // Only enrolled users. - .' JOIN {question} q ON q.category = qc.id' - .' LEFT JOIN {studentquiz_question} sqq on sqq.questionid = q.id' - .' LEFT JOIN (' - .' SELECT' - .' q.id questionid,' - .' coalesce(avg(sqr.rate),0) avg_rating' - .' FROM {studentquiz} sq' - .' JOIN {context} con ON con.instanceid = sq.coursemodule' - .' JOIN {question_categories} qc ON qc.contextid = con.id' - .' JOIN {question} q ON q.category = qc.id' - .' LEFT JOIN {studentquiz_rate} sqr ON sqr.questionid = q.id' - .' WHERE sq.coursemodule = :cmid2' - .' GROUP BY q.id' - .' ) rating on rating.questionid = q.id' - .' WHERE q.hidden = 0 and q.parent = 0 and sq.coursemodule = :cmid1'; + $sql = "SELECT COUNT(*) AS questions_available, + AVG(rating.avg_rating) AS average_rating, + SUM(sqq.approved) AS questions_approved + FROM {studentquiz} sq + -- Get this StudentQuiz question category. + JOIN {context} con ON con.instanceid = sq.coursemodule + JOIN {question_categories} qc ON qc.contextid = con.id + -- Only enrolled users. + JOIN {question} q ON q.category = qc.id + LEFT JOIN {studentquiz_question} sqq ON sqq.questionid = q.id + LEFT JOIN ( + SELECT q.id AS questionid, COALESCE(AVG(sqr.rate),0) AS avg_rating + FROM {studentquiz} sq + JOIN {context} con ON con.instanceid = sq.coursemodule + JOIN {question_categories} qc ON qc.contextid = con.id + JOIN {question} q ON q.category = qc.id + LEFT JOIN {studentquiz_rate} sqr ON sqr.questionid = q.id + WHERE sq.coursemodule = :cmid2 + GROUP BY q.id + ) rating ON rating.questionid = q.id + WHERE q.hidden = 0 + AND q.parent = 0 + AND sq.coursemodule = :cmid1"; $rs = $DB->get_record_sql($sql, array('cmid1' => $cmid, 'cmid2' => $cmid)); return $rs; } /** * Fix parent of question categories of StudentQuiz. - * Old Studentquiz have the parent of question categories not equalling to 0 for various reasons, but they should. + * Old StudentQuiz had the parent of question categories not equalling to 0 for various reasons, but they should. * In Moodle < 3.5 there is no "top" parent category, so the question category itself has to be corrected if it's not 0. * In Moodle >= 3.5 there is a new "top" parent category, so the question category of StudentQuiz has to have that as parent. * See https://tracker.moodle.org/browse/MDL-61132 and its diff. @@ -1261,42 +1255,42 @@ function mod_studentquiz_question_stats($cmid) { function mod_studentquiz_fix_wrong_parent_in_question_categories() { global $DB; - if (function_exists('question_get_top_category')) { // We have a moodle with "top" category feature - $categorieswithouttop = $DB->get_records_sql(' - select qc.id, qc.contextid, qc.name, qc.parent - from {question_categories} qc - inner join {context} c on qc.contextid = c.id - inner join {course_modules} cm on c.instanceid = cm.id - inner join {modules} m on cm.module = m.id - left join {question_categories} up on qc.contextid = up.contextid and qc.parent = up.id - where m.name = :modulename - and up.name is null - and qc.name <> :topname - ', array( + if (function_exists('question_get_top_category')) { // We have a moodle with "top" category feature. + $sql = "SELECT qc.id, qc.contextid, qc.name, qc.parent + FROM {question_categories} qc + INNER JOIN {context} c ON qc.contextid = c.id + INNER JOIN {course_modules} cm ON c.instanceid = cm.id + INNER JOIN {modules} m ON cm.module = m.id + LEFT JOIN {question_categories} up ON qc.contextid = up.contextid + AND qc.parent = up.id + WHERE m.name = :modulename + AND up.name IS NULL + AND qc.name <> :topname"; + $categorieswithouttop = $DB->get_records_sql($sql, array( 'modulename' => 'studentquiz', 'topname' => 'top' )); foreach ($categorieswithouttop as $currentcat) { $topcat = question_get_top_category($currentcat->contextid, true); - // now set the parent to the newly created top id + // Now set the parent to the newly created top id. $DB->set_field('question_categories', 'parent', $topcat->id, array('id' => $currentcat->id)); } } else { - $categorieswithoutparent = $DB->get_records_sql(' - select qc.id, qc.contextid, qc.name, qc.parent - from {question_categories} qc - inner join {context} c on qc.contextid = c.id - inner join {course_modules} cm on c.instanceid = cm.id - inner join {modules} m on cm.module = m.id - left join {question_categories} up on qc.contextid = up.contextid and qc.parent = up.id - where m.name = :modulename - and up.id is null - and qc.parent <> 0 - ', array( + $sql = "SELECT qc.id, qc.contextid, qc.name, qc.parent + FROM {question_categories} qc + INNER JOIN {context} c ON qc.contextid = c.id + INNER JOIN {course_modules} cm ON c.instanceid = cm.id + INNER JOIN {modules} m ON cm.module = m.id + LEFT JOIN {question_categories} up ON qc.contextid = up.contextid + AND qc.parent = up.id + WHERE m.name = :modulename + AND up.id IS NULL + AND qc.parent <> 0"; + $categorieswithoutparent = $DB->get_records_sql($sql, array( 'modulename' => 'studentquiz' )); foreach ($categorieswithoutparent as $currentcat) { - // now set the parent to 0 + // Now set the parent to 0. $DB->set_field('question_categories', 'parent', 0, array('id' => $currentcat->id)); } } From 2eb67a629170167649cb22180cd040c22513158e Mon Sep 17 00:00:00 2001 From: Dragon Dionysius Date: Mon, 4 Feb 2019 03:28:26 +0100 Subject: [PATCH 2/2] minor fixes to sql coding style --- classes/question/bank/performances_column.php | 8 ++++---- classes/question/bank/question_bank_filter.php | 4 ++-- classes/question/bank/tag_column.php | 2 +- 3 files changed, 7 insertions(+), 7 deletions(-) diff --git a/classes/question/bank/performances_column.php b/classes/question/bank/performances_column.php index 86b3623b..5ec74497 100644 --- a/classes/question/bank/performances_column.php +++ b/classes/question/bank/performances_column.php @@ -151,7 +151,7 @@ public function get_extra_joins() { WHERE qasd.name = '-submit' AND sq.id = " . $this->studentquizid . " AND sqa.userid = " . $this->currentuserid . " - AND qas.fraction is not null + AND qas.fraction IS NOT NULL GROUP BY qa.questionid ) qasdmax ON qasd.id = qasdmax.maxqasdid WHERE qasd.name = '-submit' @@ -167,13 +167,13 @@ public function get_required_fields() { if ($this->studentquiz->aggregated) { return array('sp.attempts practice', 'sp.attempts AS myattempts', "( - CASE WHEN sp.attempts is null + CASE WHEN sp.attempts IS NULL THEN '' ELSE CASE WHEN sp.lastanswercorrect = 1 THEN 'gradedright' ELSE 'gradedwrong' - END - END + END + END ) AS mylastattempt"); } else { return array('pr.practice', 'myatts.myattempts', 'mylatts.mylastattempt'); diff --git a/classes/question/bank/question_bank_filter.php b/classes/question/bank/question_bank_filter.php index a1cdf26c..12bed076 100644 --- a/classes/question/bank/question_bank_filter.php +++ b/classes/question/bank/question_bank_filter.php @@ -223,7 +223,7 @@ public function get_sql_filter($data) { $params[$name] = "%$value%"; break; case 1: // Does not contain. - $res = ' (searchtag = 0 or searchtag is null) '; + $res = ' (searchtag = 0 OR searchtag IS NULL) '; $params[$name] = "%$value%"; break; case 2: // Equal to. @@ -239,7 +239,7 @@ public function get_sql_filter($data) { $params[$name] = "%$value"; break; case 5: // Empty. - $res = ' (tags = 0 or tags is null) '; + $res = ' (tags = 0 OR tags IS NULL) '; $params[$name] = "-ignore-"; break; default: diff --git a/classes/question/bank/tag_column.php b/classes/question/bank/tag_column.php index 75167406..cf2550e2 100644 --- a/classes/question/bank/tag_column.php +++ b/classes/question/bank/tag_column.php @@ -103,7 +103,7 @@ protected function display_content($question, $rowclasses) { * @return array sql query join additional */ public function get_extra_joins() { - $searchtag = ($this->tagfilteractive) ? "SUM(CASE WHEN t.name LIKE :searchtag then 1 else 0 end)" : "0"; + $searchtag = ($this->tagfilteractive) ? "SUM(CASE WHEN t.name LIKE :searchtag THEN 1 ELSE 0 END)" : "0"; return array('tags' => "LEFT JOIN ( SELECT ti.itemid AS questionid, COUNT(*) AS tags, " . $searchtag . " AS searchtag FROM {tag} t