Skip to content

Commit 3d2d2e8

Browse files
marxjohnsonmarxjohnson
committed
MDL-87848 questions: Move question count sql to question_counts
Now that we have multiple places that use question counts, it's a good idea to have the queries generated in the same place, so that we can make sure the counts treat things like versions and statuses consistently.
1 parent e2fad35 commit 3d2d2e8

5 files changed

Lines changed: 338 additions & 26 deletions

File tree

‎.upgradenotes/MDL-87848-2026030509281636.yml‎

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,9 @@
1+
issueNumber: MDL-87848
2+
notes:
3+
core_question:
4+
- message: >-
5+
`core_question\output\question_category_selector::question_count_sql`
6+
has been replaces with
7+
`core_question\local\bank\question_counts\by_category_query`, to keep
8+
all the question counting logic together in one place.
9+
type: deprecated

‎public/question/bank/managecategories/classes/output/editable_name.php‎

Lines changed: 8 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -23,6 +23,7 @@
2323
use core\url;
2424
use core_external\external_api;
2525
use core_question\category_manager;
26+
use core_question\local\bank\question_counts;
2627
use core_question\output\question_category_selector;
2728
use qbank_managecategories\helper;
2829

@@ -86,8 +87,13 @@ public static function callback(int $categoryid, string $newname): self {
8687
],
8788
);
8889
$categoryname = format_string($updatedcategory->name, true, ['context' => $context, 'escape' => false]);
89-
$questioncountsql = question_category_selector::question_count_sql(categoryparam: '?');
90-
$questioncount = $DB->get_field_sql($questioncountsql, [$categoryid]);
90+
$questioncounts = new question_counts();
91+
[$questioncountsql, $questioncountparams] = $questioncounts->by_category_query(categoryparam: ':categoryid');
92+
$params = [
93+
...$questioncountparams,
94+
'categoryid' => $categoryid,
95+
];
96+
$questioncount = $DB->get_field_sql($questioncountsql, $params);
9197

9298
$categorylink = new category_link(
9399
$categoryname,

‎public/question/classes/local/bank/question_counts.php‎

Lines changed: 35 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -60,7 +60,9 @@ public function by_course_modules(array $coursemodulecontextids): array {
6060
JOIN {question_categories} qc ON qc.contextid = c.id
6161
LEFT JOIN {question_bank_entries} qbe ON qbe.questioncategoryid = qc.id
6262
LEFT JOIN {question_versions} qv ON qv.questionbankentryid = qbe.id
63-
LEFT JOIN {question_versions} qv1 ON qv1.questionbankentryid = qbe.id AND qv.version < qv1.version
63+
LEFT JOIN {question_versions} qv1 ON qv1.questionbankentryid = qbe.id
64+
AND qv.version < qv1.version
65+
AND qv1.status != :hidden1
6466
LEFT JOIN {question} q ON q.id = qv.questionid
6567
WHERE c.id {$contextinsql}
6668
AND (qv.status != :hidden OR q.id IS NULL)
@@ -71,7 +73,39 @@ public function by_course_modules(array $coursemodulecontextids): array {
7173
$params = [
7274
...$contextinparams,
7375
'hidden' => question_version_status::QUESTION_STATUS_HIDDEN,
76+
'hidden1' => question_version_status::QUESTION_STATUS_HIDDEN,
7477
];
7578
return $db->get_records_sql_menu($sql, $params);
7679
}
80+
81+
/**
82+
* Return the SQL query for getting a count of questions in a single category.
83+
*
84+
* @param int $showallversions 1 to show all versions not only the latest.
85+
* @param string $categoryparam Category ID parameter or field. This can be a paramter that is added to the $params array
86+
* by the calling code, or field in another table where this is used as a subquery.
87+
* @return array The SQL and its parameters.
88+
*/
89+
public function by_category_query(int $showallversions = 0, string $categoryparam = 'c.id'): array {
90+
$sql = "
91+
SELECT COUNT(1)
92+
FROM {question} q
93+
JOIN {question_versions} qv ON qv.questionid = q.id
94+
JOIN {question_bank_entries} qbe ON qbe.id = qv.questionbankentryid
95+
LEFT JOIN {question_versions} qv1 ON qv1.questionbankentryid = qbe.id
96+
AND qv.version < qv1.version
97+
AND qv1.status != :hidden1
98+
WHERE q.parent = :topparent
99+
AND qv.status != :hidden
100+
AND (:showallversions = 1 OR qv1.id IS NULL)
101+
AND qbe.questioncategoryid = {$categoryparam}
102+
";
103+
$params = [
104+
'showallversions' => $showallversions,
105+
'topparent' => 0,
106+
'hidden' => question_version_status::QUESTION_STATUS_HIDDEN,
107+
'hidden1' => question_version_status::QUESTION_STATUS_HIDDEN,
108+
];
109+
return [$sql, $params];
110+
}
77111
}

‎public/question/classes/output/question_category_selector.php‎

Lines changed: 18 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -16,10 +16,13 @@
1616

1717
namespace core_question\output;
1818

19+
use core\attribute\deprecated;
1920
use core\context;
21+
use core\deprecation;
2022
use core\output\renderable;
2123
use core\output\renderer_base;
2224
use core\output\templatable;
25+
use core_question\local\bank\question_counts;
2326
use core_question\local\bank\question_version_status;
2427

2528
/**
@@ -89,19 +92,27 @@ public function get_categories_for_contexts(
8992
return [];
9093
}
9194

92-
[$insql, $inparams] = $DB->get_in_or_equal($validcontexts);
95+
[$insql, $inparams] = $DB->get_in_or_equal($validcontexts, SQL_PARAMS_NAMED);
9396

9497
$topwhere = $top ? '' : 'AND c.parent <> 0';
95-
$countsql = $this->question_count_sql($showallversions);
98+
$questioncounts = new question_counts();
99+
[$countsql, $countparams] = $questioncounts->by_category_query($showallversions);
100+
$params = array_merge($inparams, $countparams);
96101
$sql = "SELECT c.*,
97102
({$countsql}) AS questioncount
98103
FROM {question_categories} c
99104
WHERE c.contextid {$insql} {$topwhere}
100105
ORDER BY {$sortorder}";
101106

102-
return $DB->get_records_sql($sql, $inparams);
107+
return $DB->get_records_sql($sql, $params);
103108
}
104109

110+
#[deprecated(
111+
replacement: 'question_counts::by_category_query',
112+
since: 5.2,
113+
reason: 'Moved question count queries to a common class',
114+
mdl: 'MDL-87848',
115+
)]
105116
/**
106117
* Return the SQL query for getting a count of questions in a category.
107118
*
@@ -110,25 +121,10 @@ public function get_categories_for_contexts(
110121
* @return string The SQL.
111122
*/
112123
public static function question_count_sql(int $showallversions = 0, string $categoryparam = 'c.id'): string {
113-
$statuscondition = "AND (qv.status = '" . question_version_status::QUESTION_STATUS_READY . "' " .
114-
" OR qv.status = '" . question_version_status::QUESTION_STATUS_DRAFT . "' )";
115-
$substatuscondition = "AND v.status <> '" . question_version_status::QUESTION_STATUS_HIDDEN . "' ";
116-
return "
117-
SELECT COUNT(1)
118-
FROM {question} q
119-
JOIN {question_versions} qv ON qv.questionid = q.id
120-
JOIN {question_bank_entries} qbe ON qbe.id = qv.questionbankentryid
121-
WHERE q.parent = '0'
122-
$statuscondition
123-
AND ({$showallversions} = 1
124-
OR (qv.version = (SELECT MAX(v.version)
125-
FROM {question_versions} v
126-
JOIN {question_bank_entries} be ON be.id = v.questionbankentryid
127-
WHERE be.id = qbe.id $substatuscondition)
128-
)
129-
)
130-
AND qbe.questioncategoryid = {$categoryparam}
131-
";
124+
deprecation::emit_deprecation([self::class, __FUNCTION__]);
125+
$questioncounts = new question_counts();
126+
[$sql] = $questioncounts->by_category_query($showallversions, $categoryparam);
127+
return $sql;
132128
}
133129

134130
/**

0 commit comments

Comments
 (0)