Skip to content

Commit 64b93e1

Browse files
committed
Update question browser for Moodle 5 compatibility and refactor to move code common to the question browser and the bulk tester into the util class.
1 parent c8b104e commit 64b93e1

File tree

5 files changed

+145
-73
lines changed

5 files changed

+145
-73
lines changed

bulktestindex.php

Lines changed: 9 additions & 36 deletions
Original file line numberDiff line numberDiff line change
@@ -127,21 +127,6 @@ function display_questions_for_all_contexts($availablequestionsbycontext) {
127127

128128

129129

130-
/**
131-
* Used for displaying all the questions in the Oldskool Moodle 4 setup.
132-
* $availablequestionsbycontext maps from
133-
* contextid to [name, numquestions] associative arrays.
134-
*/
135-
function display_questions_for_all_course_contexts($availablequestionsbycontext) {
136-
foreach ($availablequestionsbycontext as $contextid => $info) {
137-
$context = context::instance_by_id($contextid);
138-
if ($context->contextlevel === CONTEXT_COURSE || $context->contextlevel === CONTEXT_COURSECAT) {
139-
$name = $info['name'];
140-
$numcoderunnerquestions = $info['numquestions'];
141-
display_questions_for_context($contextid, $name, $numcoderunnerquestions);
142-
}
143-
}
144-
}
145130

146131

147132

@@ -225,30 +210,18 @@ function display_questions_for_all_course_contexts($availablequestionsbycontext)
225210
if ($oldskool) {
226211
// Moodle 4 style.
227212
echo $OUTPUT->heading(get_string('coderunnercontexts', 'qtype_coderunner'));
228-
display_questions_for_all_course_contexts($availablequestionsbycontext);
213+
qtype_coderunner_util::display_course_contexts(
214+
$availablequestionsbycontext,
215+
'qtype_coderunner\display_questions_for_context'
216+
);
229217
} else {
230218
// Deal with funky question bank madness in Moodle 5.0.
231219
echo html_writer::tag('p', "Moodle >= 5.0 detected. Listing by course then qbank.");
232-
$allcourses = bulk_tester::get_all_courses();
233-
foreach ($allcourses as $courseid => $course) {
234-
$coursecontext = context_course::instance($courseid);
235-
display_course_header_and_link($coursecontext->id, $course->name);
236-
$allbanks = bulk_tester::get_all_qbanks_for_course($courseid);
237-
if (count($allbanks) > 0) {
238-
echo html_writer::start_tag('ul');
239-
foreach ($allbanks as $bank) {
240-
$contextid = $bank->contextid;
241-
if (array_key_exists($contextid, $availablequestionsbycontext)) {
242-
$contextdata = $availablequestionsbycontext[$contextid];
243-
$name = $contextdata['name'];
244-
$numquestions = $contextdata['numquestions'];
245-
$coursenamebankname = $bank->coursenamebankname;
246-
display_questions_for_context($contextid, $name, $numquestions);
247-
}
248-
}
249-
echo html_writer::end_tag('ul');
250-
}
251-
}
220+
qtype_coderunner_util::display_course_grouped_contexts(
221+
$availablequestionsbycontext,
222+
'qtype_coderunner\display_course_header_and_link',
223+
'qtype_coderunner\display_questions_for_context'
224+
);
252225
}
253226
// Output final stuff, including link to bulktestall.
254227
echo html_writer::empty_tag('br');

classes/bulk_tester.php

Lines changed: 3 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -389,13 +389,7 @@ public static function run_all_tests_for_qbank($context) {
389389
* @return moodle_url base URL used for seeing the results of a test run.
390390
*/
391391
private function get_question_base_test_url() {
392-
if ($this->context->contextlevel == CONTEXT_COURSE) {
393-
$qparams['courseid'] = $this->context->instanceid;
394-
} else if ($this->context->contextlevel == CONTEXT_MODULE) {
395-
$qparams['cmid'] = $this->context->instanceid;
396-
} else {
397-
$qparams['courseid'] = SITEID;
398-
}
392+
$qparams = qtype_coderunner_util::get_question_bank_params($this->context->id);
399393
$questiontestsurl = new moodle_url('/question/type/coderunner/questiontestrun.php');
400394
$questiontestsurl->params($qparams);
401395
return $questiontestsurl;
@@ -823,16 +817,12 @@ public static function display_prototypes($courseid, $prototypes, $missingprotot
823817

824818
/**
825819
* Return a link to the given question in the question bank.
826-
* @param int $courseid the id of the course containing the question
820+
* @param int $courseid the id of the course containing the question (not used - kept for compatibility)
827821
* @param stdObj $question the question
828822
* @return html link to the question in the question bank
829823
*/
830824
private static function make_question_link($courseid, $question) {
831-
$qbankparams = ['qperpage' => 1000]; // Can't easily get the true value.
832-
$qbankparams['category'] = $question->category . ',' . $question->contextid;
833-
$qbankparams['lastchanged'] = $question->questionid;
834-
$qbankparams['courseid'] = $courseid;
835-
$qbankparams['showhidden'] = 1;
825+
$qbankparams = qtype_coderunner_util::make_question_bank_url_params($question);
836826
$questionbanklink = new moodle_url('/question/edit.php', $qbankparams);
837827
return html_writer::link($questionbanklink, $question->name, ['target' => '_blank']);
838828
}

classes/util.php

Lines changed: 90 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -68,6 +68,95 @@ public static function using_mod_qbank() {
6868
}
6969

7070

71+
/**
72+
* Display contexts for all course-level contexts (Moodle 4 style).
73+
* $availablequestionsbycontext maps from contextid to [name, numquestions] associative arrays.
74+
* $displaycallback is a function that takes ($contextid, $name, $numquestions).
75+
*/
76+
public static function display_course_contexts($availablequestionsbycontext, $displaycallback) {
77+
echo \html_writer::start_tag('ul');
78+
foreach ($availablequestionsbycontext as $contextid => $info) {
79+
$context = \context::instance_by_id($contextid);
80+
if ($context->contextlevel === CONTEXT_COURSE || $context->contextlevel === CONTEXT_COURSECAT) {
81+
$name = $info['name'];
82+
$numquestions = $info['numquestions'];
83+
$displaycallback($contextid, $name, $numquestions);
84+
}
85+
}
86+
echo \html_writer::end_tag('ul');
87+
}
88+
89+
90+
/**
91+
* Get URL parameters for accessing a question in the question bank.
92+
* Returns array with either 'cmid' or 'courseid' depending on context level.
93+
*
94+
* @param int $contextid The context ID where the question resides
95+
* @return array URL parameters (cmid or courseid)
96+
*/
97+
public static function get_question_bank_params($contextid) {
98+
$qcontext = \context::instance_by_id($contextid);
99+
$params = [];
100+
if ($qcontext->contextlevel == CONTEXT_COURSE) {
101+
$params['courseid'] = $qcontext->instanceid;
102+
} else if ($qcontext->contextlevel == CONTEXT_MODULE) {
103+
$params['cmid'] = $qcontext->instanceid;
104+
} else {
105+
$params['courseid'] = SITEID;
106+
}
107+
return $params;
108+
}
109+
110+
111+
/**
112+
* Build complete URL parameters for linking to a question in the question bank.
113+
*
114+
* @param object $question Question object with id, category, and contextid properties
115+
* @return array Complete URL parameters for question/edit.php
116+
*/
117+
public static function make_question_bank_url_params($question) {
118+
$params = self::get_question_bank_params($question->contextid);
119+
$params['qperpage'] = 1000;
120+
$params['category'] = $question->category . ',' . $question->contextid;
121+
$params['lastchanged'] = $question->id;
122+
$params['showhidden'] = 1;
123+
return $params;
124+
}
125+
126+
127+
/**
128+
* Display contexts grouped by course (Moodle 5 style).
129+
* $availablequestionsbycontext maps from contextid to [name, numquestions] associative arrays.
130+
* $courseheadercallback is a function that takes ($coursecontextid, $coursename).
131+
* $contextcallback is a function that takes ($contextid, $name, $numquestions).
132+
*/
133+
public static function display_course_grouped_contexts(
134+
$availablequestionsbycontext,
135+
$courseheadercallback,
136+
$contextcallback
137+
) {
138+
$allcourses = \qtype_coderunner\bulk_tester::get_all_courses();
139+
foreach ($allcourses as $courseid => $course) {
140+
$coursecontext = \context_course::instance($courseid);
141+
$courseheadercallback($coursecontext->id, $course->name);
142+
$allbanks = \qtype_coderunner\bulk_tester::get_all_qbanks_for_course($courseid);
143+
if (count($allbanks) > 0) {
144+
echo \html_writer::start_tag('ul');
145+
foreach ($allbanks as $bank) {
146+
$contextid = $bank->contextid;
147+
if (array_key_exists($contextid, $availablequestionsbycontext)) {
148+
$contextdata = $availablequestionsbycontext[$contextid];
149+
$name = $contextdata['name'];
150+
$numquestions = $contextdata['numquestions'];
151+
$contextcallback($contextid, $name, $numquestions);
152+
}
153+
}
154+
echo \html_writer::end_tag('ul');
155+
}
156+
}
157+
}
158+
159+
71160
// A utility method used for iterating over multibyte (utf-8) strings
72161
// in php. Taken from https://stackoverflow.com/questions/3666306/how-to-iterate-utf-8-string-in-php
73162
// We can't simply use mb_substr to extract the ith characters from a multibyte
@@ -175,7 +264,7 @@ public static function clean(&$s) {
175264
}
176265
return '\\x' . sprintf("%02x", ord($char));
177266
}, $output);
178-
267+
179268
if ($output !== '') {
180269
$output .= "\n";
181270
}

questionbrowser.php

Lines changed: 16 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -159,6 +159,9 @@ private function enhance_question_metadata($question) {
159159
$tags = $this->get_question_tags($question->id);
160160
$usedin = $this->usagemap[$question->id] ?? [];
161161

162+
// Get question bank URL parameters (handles Moodle 5's cmid vs courseid).
163+
$qbankparams = \qtype_coderunner_util::make_question_bank_url_params($question);
164+
162165
$enhanced = [
163166
'type' => 'coderunner',
164167
'id' => (string)$question->id,
@@ -168,10 +171,12 @@ private function enhance_question_metadata($question) {
168171
'coderunnertype' => $question->coderunnertype,
169172
'category' => bulk_tester::get_category_path($question->category),
170173
'categoryid' => (string)$question->category,
174+
'contextid' => (string)$question->contextid,
171175
'version' => (int)$question->version,
172176
'courseid' => (string)$courseid,
173177
'tags' => $tags,
174178
'usedin' => $usedin,
179+
'qbankparams' => $qbankparams,
175180
];
176181

177182
$enhanced['lines_of_code'] = $this->count_lines_of_code($answer);
@@ -938,18 +943,12 @@ function toggleDisplay(type, content, isHTML = false) {
938943
});
939944

940945
bankBtn.addEventListener('click', () => {
941-
if (q.id && q.courseid && q.categoryid) {
942-
const params = new URLSearchParams({
943-
'qperpage': '1000',
944-
'category': q.categoryid + ',' + <?php echo $contextid; ?>,
945-
'lastchanged': q.id,
946-
'courseid': q.courseid,
947-
'showhidden': '1'
948-
});
946+
if (q.qbankparams) {
947+
const params = new URLSearchParams(q.qbankparams);
949948
const bankUrl = `${moodleBaseUrl}/question/edit.php?${params.toString()}`;
950949
window.open(bankUrl, '_blank');
951950
} else {
952-
alert('Missing question data for question bank link');
951+
alert('Missing question bank parameters');
953952
}
954953
});
955954

@@ -1044,7 +1043,8 @@ function buildFilters(data){
10441043
kwField.innerHTML = '';
10451044
const keys = Array.from(new Set(data.flatMap(obj => Object.keys(obj))));
10461045
const optAny = document.createElement('option'); optAny.textContent = 'Any'; kwField.appendChild(optAny);
1047-
keys.filter(k => k !== 'version' && k !== 'timemodified' && k !== 'type' && k !== 'courseid' && k !== 'lines_of_code').forEach(k => {
1046+
const excludedKeys = ['version', 'timemodified', 'type', 'courseid', 'lines_of_code'];
1047+
keys.filter(k => !excludedKeys.includes(k)).forEach(k => {
10481048
const o = document.createElement('option'); o.textContent = k; kwField.appendChild(o);
10491049
});
10501050

@@ -1129,7 +1129,8 @@ function getNumericFilterRanges(){
11291129
// Advanced filter functions
11301130
function getAvailableFields() {
11311131
const keys = Array.from(new Set(rawData.flatMap(obj => Object.keys(obj))));
1132-
return keys.filter(k => k !== 'version' && k !== 'timemodified' && k !== 'type' && k !== 'courseid' && k !== 'lines_of_code').sort();
1132+
const excludedKeys = ['version', 'timemodified', 'type', 'courseid', 'lines_of_code'];
1133+
return keys.filter(k => !excludedKeys.includes(k)).sort();
11331134
}
11341135

11351136
function getOperatorsForField(field) {
@@ -1147,7 +1148,10 @@ function getOperatorsForField(field) {
11471148
} else if (typeof firstValue === 'number') {
11481149
return ['equals', 'does not equal', 'greater than', 'less than', 'greater or equal', 'less or equal'];
11491150
} else {
1150-
return ['contains', 'does not contain', 'equals', 'does not equal', 'starts with', 'ends with', 'is empty', 'is not empty', 'matches regex', 'does not match regex'];
1151+
return [
1152+
'contains', 'does not contain', 'equals', 'does not equal', 'starts with', 'ends with',
1153+
'is empty', 'is not empty', 'matches regex', 'does not match regex'
1154+
];
11511155
}
11521156
}
11531157

questionbrowserindex.php

Lines changed: 27 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -25,19 +25,30 @@
2525
namespace qtype_coderunner;
2626

2727
use context_system;
28+
use context;
29+
use context_course;
2830
use html_writer;
2931
use moodle_url;
32+
use qtype_coderunner_util;
3033

3134
require_once(__DIR__ . '/../../../config.php');
3235
require_once($CFG->libdir . '/questionlib.php');
3336
require_once(__DIR__ . '/classes/bulk_tester.php');
3437

38+
// We are Moodle 4 or less if don't have mod_qbank.
39+
$oldskool = !(qtype_coderunner_util::using_mod_qbank());
40+
3541
// Login and check permissions.
3642
$context = context_system::instance();
3743
require_login();
3844

3945
const BUTTONSTYLE = 'background-color: #FFFFD0; padding: 2px 2px 0px 2px;border: 4px solid white';
4046

47+
function display_course_header($coursecontextid, $coursename) {
48+
$litext = $coursecontextid . ' - ' . $coursename;
49+
echo html_writer::tag('h5', $litext);
50+
}
51+
4152
function display_questions_for_context($contextid, $name, $numcoderunnerquestions) {
4253
$browseallstr = get_string('browsequestions', 'qtype_coderunner', $name);
4354
if (!$browseallstr) {
@@ -65,15 +76,6 @@ function display_questions_for_context($contextid, $name, $numcoderunnerquestion
6576
echo html_writer::end_tag('li');
6677
}
6778

68-
function display_questions_for_all_contexts($availablequestionsbycontext) {
69-
echo html_writer::start_tag('ul');
70-
foreach ($availablequestionsbycontext as $contextid => $info) {
71-
$name = $info['name'];
72-
$numcoderunnerquestions = $info['numquestions'];
73-
display_questions_for_context($contextid, $name, $numcoderunnerquestions);
74-
}
75-
echo html_writer::end_tag('ul');
76-
}
7779

7880
// Set up page.
7981
$PAGE->set_url('/question/type/coderunner/questionbrowserindex.php');
@@ -92,13 +94,27 @@ function display_questions_for_all_contexts($availablequestionsbycontext) {
9294
if (count($availablequestionsbycontext) == 0) {
9395
echo html_writer::tag('p', 'You do not have permission to browse questions in any contexts.');
9496
} else {
95-
echo html_writer::tag('h3', 'Available Contexts (' . count($availablequestionsbycontext) . ')');
9697
echo html_writer::tag(
9798
'p',
9899
'<strong>Instructions:</strong> Click "Browse questions" to open the question browser for that context.'
99100
);
100101

101-
display_questions_for_all_contexts($availablequestionsbycontext);
102+
if ($oldskool) {
103+
// Moodle 4 style.
104+
echo html_writer::tag('h3', 'Available Contexts (' . count($availablequestionsbycontext) . ')');
105+
qtype_coderunner_util::display_course_contexts(
106+
$availablequestionsbycontext,
107+
'qtype_coderunner\display_questions_for_context'
108+
);
109+
} else {
110+
// Deal with funky question bank madness in Moodle 5.0.
111+
echo html_writer::tag('p', "Moodle >= 5.0 detected. Listing by course then question bank.");
112+
qtype_coderunner_util::display_course_grouped_contexts(
113+
$availablequestionsbycontext,
114+
'qtype_coderunner\display_course_header',
115+
'qtype_coderunner\display_questions_for_context'
116+
);
117+
}
102118
}
103119

104120
// Add some basic styling.

0 commit comments

Comments
 (0)