From 2adefc21acddbbb1cbcd9983f522d46b6c06cfad Mon Sep 17 00:00:00 2001
From: Shamim Rezaie <shamim@moodle.com>
Date: Sun, 18 Feb 2018 06:36:14 +1100
Subject: [PATCH] MDL-61380 Questions: Support selecting "random" by tag in
question bank
---
.../classes/bank/random_question_loader.php | 45 +++++++----
question/engine/bank.php | 76 ++++++++++++++-----
2 files changed, 87 insertions(+), 34 deletions(-)
diff --git a/question/classes/bank/random_question_loader.php b/question/classes/bank/random_question_loader.php
index d897c81607e..f1b1578e2f9 100644
--- a/question/classes/bank/random_question_loader.php
+++ b/question/classes/bank/random_question_loader.php
@@ -77,6 +77,7 @@ class random_question_loader {
/**
* Pick a question at random from the given category, from among those with the fewest uses.
+ * If an array of tag ids are specified, then only the questions that are tagged with ALL those tags will be selected.
*
* It is up the the caller to verify that the cateogry exists. An unknown category
* behaves like an empty one.
@@ -84,12 +85,14 @@ class random_question_loader {
* @param int $categoryid the id of a category in the question bank.
* @param bool $includesubcategories wether to pick a question from exactly
* that category, or that category and subcategories.
+ * @param array $tagids An array of tag ids. A question has to be tagged with all the provided tagids (if any)
+ * in order to be eligible for being picked.
* @return int|null the id of the question picked, or null if there aren't any.
*/
- public function get_next_question_id($categoryid, $includesubcategories) {
- $this->ensure_questions_for_category_loaded($categoryid, $includesubcategories);
+ public function get_next_question_id($categoryid, $includesubcategories, $tagids = []) {
+ $this->ensure_questions_for_category_loaded($categoryid, $includesubcategories, $tagids);
- $categorykey = $this->get_category_key($categoryid, $includesubcategories);
+ $categorykey = $this->get_category_key($categoryid, $includesubcategories, $tagids);
if (empty($this->availablequestionscache[$categorykey])) {
return null;
}
@@ -107,26 +110,35 @@ class random_question_loader {
* @param int $categoryid the id of a category in the question bank.
* @param bool $includesubcategories wether to pick a question from exactly
* that category, or that category and subcategories.
+ * @param array $tagids an array of tag ids.
* @return string the cache key.
*/
- protected function get_category_key($categoryid, $includesubcategories) {
+ protected function get_category_key($categoryid, $includesubcategories, $tagids = []) {
if ($includesubcategories) {
- return $categoryid . '|1';
+ $key = $categoryid . '|1';
} else {
- return $categoryid . '|0';
+ $key = $categoryid . '|0';
}
+
+ if (!empty($tagids)) {
+ $key .= '|' . implode('|', $tagids);
+ }
+
+ return $key;
}
/**
* Populate {@link $availablequestionscache} for this combination of options.
- * @param int $categoryid the id of a category in the question bank.
- * @param bool $includesubcategories wether to pick a question from exactly
+ * @param int $categoryid The id of a category in the question bank.
+ * @param bool $includesubcategories Whether to pick a question from exactly
* that category, or that category and subcategories.
+ * @param array $tagids An array of tag ids. If an array is provided, then
+ * only the questions that are tagged with ALL the provided tagids will be loaded.
*/
- protected function ensure_questions_for_category_loaded($categoryid, $includesubcategories) {
+ protected function ensure_questions_for_category_loaded($categoryid, $includesubcategories, $tagids = []) {
global $DB;
- $categorykey = $this->get_category_key($categoryid, $includesubcategories);
+ $categorykey = $this->get_category_key($categoryid, $includesubcategories, $tagids);
if (isset($this->availablequestionscache[$categorykey])) {
// Data is already in the cache, nothing to do.
@@ -143,8 +155,8 @@ class random_question_loader {
list($extraconditions, $extraparams) = $DB->get_in_or_equal($this->excludedqtypes,
SQL_PARAMS_NAMED, 'excludedqtype', false);
- $questionidsandcounts = \question_bank::get_finder()->get_questions_from_categories_with_usage_counts(
- $categoryids, $this->qubaids, 'q.qtype ' . $extraconditions, $extraparams);
+ $questionidsandcounts = \question_bank::get_finder()->get_questions_from_categories_and_tags_with_usage_counts(
+ $categoryids, $this->qubaids, 'q.qtype ' . $extraconditions, $extraparams, $tagids);
if (!$questionidsandcounts) {
// No questions in this category.
$this->availablequestionscache[$categorykey] = array();
@@ -200,16 +212,19 @@ class random_question_loader {
/**
* Check whether a given question is available in a given category. If so, mark it used.
+ * If an optional list of tag ids are provided, then the question must be tagged with
+ * ALL of the provided tags to be considered as available.
*
* @param int $categoryid the id of a category in the question bank.
* @param bool $includesubcategories wether to pick a question from exactly
* that category, or that category and subcategories.
* @param int $questionid the question that is being used.
+ * @param array $tagids An array of tag ids. Only the questions that are tagged with all the provided tagids can be available.
* @return bool whether the question is available in the requested category.
*/
- public function is_question_available($categoryid, $includesubcategories, $questionid) {
- $this->ensure_questions_for_category_loaded($categoryid, $includesubcategories);
- $categorykey = $this->get_category_key($categoryid, $includesubcategories);
+ public function is_question_available($categoryid, $includesubcategories, $questionid, $tagids = []) {
+ $this->ensure_questions_for_category_loaded($categoryid, $includesubcategories, $tagids);
+ $categorykey = $this->get_category_key($categoryid, $includesubcategories, $tagids);
foreach ($this->availablequestionscache[$categorykey] as $questionids) {
if (isset($questionids[$questionid])) {
diff --git a/question/engine/bank.php b/question/engine/bank.php
index b5fb62c9412..6721cbfc05b 100644
--- a/question/engine/bank.php
+++ b/question/engine/bank.php
@@ -294,10 +294,6 @@ abstract class question_bank {
*/
public static function get_finder() {
return question_finder::get_instance();
- if (is_null(self::$questionfinder)) {
- self::$questionfinder = new question_finder();
- }
- return self::$questionfinder;
}
/**
@@ -539,29 +535,71 @@ class question_finder implements cache_data_source {
*/
public function get_questions_from_categories_with_usage_counts($categoryids,
qubaid_condition $qubaids, $extraconditions = '', $extraparams = array()) {
+ return $this->get_questions_from_categories_and_tags_with_usage_counts(
+ $categoryids, $qubaids, $extraconditions, $extraparams);
+ }
+
+ /**
+ * Get the ids of all the questions in a list of categories that have ALL the provided tags,
+ * with the number of times they have already been used in a given set of usages.
+ *
+ * The result array is returned in order of increasing (count previous uses).
+ *
+ * @param array $categoryids an array of question_category ids.
+ * @param qubaid_condition $qubaids which question_usages to count previous uses from.
+ * @param string $extraconditions extra conditions to AND with the rest of
+ * the where clause. Must use named parameters.
+ * @param array $extraparams any parameters used by $extraconditions.
+ * @param array $tagids an array of tag ids
+ * @return array questionid => count of number of previous uses.
+ */
+ public function get_questions_from_categories_and_tags_with_usage_counts($categoryids,
+ qubaid_condition $qubaids, $extraconditions = '', $extraparams = array(), $tagids = array()) {
global $DB;
list($qcsql, $qcparams) = $DB->get_in_or_equal($categoryids, SQL_PARAMS_NAMED, 'qc');
+ $select = "q.id, (SELECT COUNT(1)
+ FROM " . $qubaids->from_question_attempts('qa') . "
+ WHERE qa.questionid = q.id AND " . $qubaids->where() . "
+ ) AS previous_attempts";
+ $from = "{question} q";
+ $where = "q.category {$qcsql}
+ AND q.parent = 0
+ AND q.hidden = 0";
+ $params = $qcparams;
+
+ if (!empty($tagids)) {
+ // We treat each additional tag as an AND condition rather than
+ // an OR condition.
+ //
+ // For example, if the user filters by the tags "foo" and "bar" then
+ // we reduce the question list to questions that are tagged with both
+ // "foo" AND "bar". Any question that does not have ALL of the specified
+ // tags will be omitted.
+ list($tagsql, $tagparams) = $DB->get_in_or_equal($tagids, SQL_PARAMS_NAMED, 'ti');
+ $tagparams['tagcount'] = count($tagids);
+ $tagparams['questionitemtype'] = 'question';
+ $tagparams['questioncomponent'] = 'core_question';
+ $where .= " AND q.id IN (SELECT ti.itemid
+ FROM {tag_instance} ti
+ WHERE ti.itemtype = :questionitemtype
+ AND ti.component = :questioncomponent
+ AND ti.tagid {$tagsql}
+ GROUP BY ti.itemid
+ HAVING COUNT(itemid) = :tagcount)";
+ $params += $tagparams;
+ }
+
if ($extraconditions) {
$extraconditions = ' AND (' . $extraconditions . ')';
}
- return $DB->get_records_sql_menu("
- SELECT q.id, (SELECT COUNT(1)
- FROM " . $qubaids->from_question_attempts('qa') . "
- WHERE qa.questionid = q.id AND " . $qubaids->where() . "
- ) AS previous_attempts
-
- FROM {question} q
-
- WHERE q.category {$qcsql}
- AND q.parent = 0
- AND q.hidden = 0
- {$extraconditions}
-
- ORDER BY previous_attempts
- ", $qubaids->from_where_params() + $qcparams + $extraparams);
+ return $DB->get_records_sql_menu("SELECT $select
+ FROM $from
+ WHERE $where $extraconditions
+ ORDER BY previous_attempts",
+ $qubaids->from_where_params() + $params + $extraparams);
}
/* See cache_data_source::load_for_cache. */