L-electron-Rare/moodle
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

MDL-20636 Fix 30 TODOs

Browse Source
This commit is contained in:
Tim Hunt
2011-02-24 17:47:51 +00:00
parent d649fb0221
commit 2daffca554
42 changed files with 139 additions and 233 deletions
Download Patch File Download Diff File Expand all files Collapse all files
lang/en/question.php
+2
View File
@@ -178,6 +178,7 @@ $string['movedquestionsandcategories'] = 'Moved questions and question categorie
$string['movelinksonly'] = 'Just change where links point to, do not move or copy files.';
$string['moveq'] = 'Move question(s)';
$string['moveqtoanothercontext'] = 'Move question to another context.';
$string['moveto'] = 'Move to >>';
$string['movingcategory'] = 'Moving category';
$string['movingcategoryandfiles'] = 'Are you sure you want to move category {$a->name} and all child categories to context for "{$a->contextto}"?<br /> We have detected {$a->urlcount} files linked from questions in {$a->fromareaname}, would you like to copy or move these to {$a->toareaname}?';
$string['movingcategorynofiles'] = 'Are you sure you want to move category "{$a->name}" and all child categories to context for "{$a->contextto}"?';
@@ -347,5 +348,6 @@ $string['submitted'] = 'Submit: {$a}';
$string['unknownquestion'] = 'Unknown question: {$a}.';
$string['unknownquestioncatregory'] = 'Unknown question category: {$a}.';
$string['whethercorrect'] = 'Whether correct';
$string['withselected'] = 'With selected';
$string['xoutofmax'] = '{$a->mark} out of {$a->max}';
$string['yougotnright'] = 'You have correctly selected {$a->num}.';
lib/questionlib.php
+8 -8
View File
@@ -777,7 +777,7 @@ function question_load_questions($questionids, $extrafields = '', $join = '') {
*/
function _tidy_question($question, $loadtags = false) {
global $CFG;
if (question_bank::is_qtype_installed($question->qtype)) {
if (!question_bank::is_qtype_installed($question->qtype)) {
$question->questiontext = html_writer::tag('p', get_string('warningmissingtype',
'qtype_missingtype')) . $question->questiontext;
}
@@ -1153,18 +1153,18 @@ function question_add_tops($categories, $pcontexts){
}
/**
* Returns a comma separated list of ids of the category and all subcategories
* @return array of question category ids of the category and all subcategories.
*/
function question_categorylist($categoryid) {
global $DB;
// returns a comma separated list of ids of the category and all subcategories
$categorylist = $categoryid;
if ($subcategories = $DB->get_records('question_categories', array('parent' => $categoryid), 'sortorder ASC', 'id, 1')) {
foreach ($subcategories as $subcategory) {
$categorylist .= ','. question_categorylist($subcategory->id);
}
$subcategories = $DB->get_records('question_categories', array('parent' => $categoryid), 'sortorder ASC', 'id, 1');
$categorylist = array($categoryid);
foreach ($subcategories as $subcategory) {
$categorylist = array_merge($categorylist, question_categorylist($subcategory->id));
}
return $categorylist;
}
mod/quiz/comment.php
-1
View File
@@ -57,7 +57,6 @@ echo $OUTPUT->heading(format_string($attemptobj->get_question_name($slot)));
// Process any data that was submitted.
if (data_submitted() && confirm_sesskey()) {
if (optional_param('submit', false, PARAM_BOOL)) {
// TODO better error handling.
$transaction = $DB->start_delegated_transaction();
$attemptobj->process_all_actions(time());
$transaction->allow_commit();
mod/quiz/lang/en/quiz.php
-2
View File
@@ -420,7 +420,6 @@ $string['modulename'] = 'Quiz';
$string['modulename_help'] = 'The quiz module enables the teacher to design and set quizzes consisting of multiple choice, true-false, matching and other question types. Each attempt is automatically marked, and the teacher can choose whether to give feedback and/or show correct answers.';
$string['modulenameplural'] = 'Quizzes';
$string['moveselectedonpage'] = 'Move selected questions to page: {$a}';
$string['moveto'] = 'Move to >>';
$string['multianswer'] = 'Embedded answers (cloze)';
$string['multichoice'] = 'Multiple choice';
$string['multipleanswers'] = 'Choose at least one answer.';
@@ -789,7 +788,6 @@ $string['warningmissingtype'] = '<b>This question is of a type that has not been
$string['wheregrade'] = 'Where\'s my grade?';
$string['wildcard'] = 'Wild card';
$string['windowclosing'] = 'This window will close shortly.';
$string['withselected'] = 'With selected';
$string['withsummary'] = 'with summary statistics';
$string['wronguse'] = 'You can not use this page like that';
$string['xhtml'] = 'XHTML';
mod/quiz/lib.php
+3 -4
View File
@@ -305,9 +305,8 @@ function quiz_update_effective_access($quiz, $userid) {
function quiz_delete_all_attempts($quiz) {
global $CFG, $DB;
require_once($CFG->libdir . '/questionlib.php');
// TODO remove $CFG->prefix
question_engine::delete_questions_usage_by_activities("{$CFG->prefix}question_usages.id IN (
SELECT uniqueid FROM {$CFG->prefix}quiz_attempts WHERE quiz = $quizid)");
question_engine::delete_questions_usage_by_activities("{question_usages}.id IN (
SELECT uniqueid FROM {quiz_attempts} WHERE quiz = :quizid)", array('quizid' => $quizid));
$DB->delete_records('quiz_attempts', array('quiz' => $quiz->id));
$DB->delete_records('quiz_grades', array('quiz' => $quiz->id));
}
@@ -1232,7 +1231,7 @@ function quiz_reset_userdata($data) {
SELECT uniqueid
FROM {quiz_attempts} quiza
JOIN {quiz} quiz ON quiza.quiz = quiz.id
WHERE quiz.course = ?)', array($data->courseid));
WHERE quiz.course = :courseid)', array('courseid' => $data->courseid));
$DB->delete_records_select('quiz_attempts',
'quiz IN (SELECT id FROM {quiz} WHERE course = ?)', array($data->courseid));
mod/quiz/locallib.php
-39
View File
@@ -217,21 +217,6 @@ function quiz_has_attempts($quizid) {
/// Functions to do with quiz layout and pages ////////////////////////////////
/**
* Returns a comma separated list of question ids for the current page
*
* @param string $layout the string representing the quiz layout. Each page is represented as a
* comma separated list of question ids and 0 indicating page breaks.
* So 5,2,0,3,0 means questions 5 and 2 on page 1 and question 3 on page 2
* @param int $page the number of the current page.
* @return string comma separated list of question ids
*/
function quiz_questions_on_page($layout, $page) {
$pages = explode(',0', $layout);
// TODO is this still used.
return trim($pages[$page], ',');
}
/**
* Returns a comma separated list of question ids for the quiz
*
@@ -267,7 +252,6 @@ function quiz_number_of_pages($layout) {
* @return int The number of questions in the quiz.
*/
function quiz_number_of_questions_in_quiz($layout) {
// TODO is this still used.
$layout = quiz_questions_in_quiz(quiz_clean_layout($layout));
$count = substr_count($layout, ',');
if ($layout !== '') {
@@ -276,29 +260,6 @@ function quiz_number_of_questions_in_quiz($layout) {
return $count;
}
/**
* Returns the first question number for the current quiz page
*
* @param string $quizlayout The string representing the layout for the whole quiz
* @param string $pagelayout The string representing the layout for the current page
* @return int the number of the first question
*/
function quiz_first_questionnumber($quizlayout, $pagelayout) {
// TODO is this still used.
// this works by finding all the questions from the quizlayout that
// come before the current page and then adding up their lengths.
global $CFG, $DB;
$start = strpos($quizlayout, ','.$pagelayout.',')-2;
if ($start > 0) {
$prevlist = substr($quizlayout, 0, $start);
list($usql, $params) = $DB->get_in_or_equal(explode(',', $prevlist));
return $DB->get_field_sql("SELECT sum(length)+1 FROM {question}
WHERE id $usql", $params);
} else {
return 1;
}
}
/**
* Re-paginates the quiz layout
*
mod/quiz/report/attemptsreport.php
+5 -3
View File
@@ -342,8 +342,10 @@ abstract class quiz_attempt_report extends quiz_default_report {
* Empty means all users.
*/
protected function delete_selected_attempts($quiz, $cm, $attemptids, $allowed) {
global $DB;
foreach ($attemptids as $attemptid) {
$attempt = get_record('quiz_attempts', 'id', $attemptid);
$attempt = $DB->get_record('quiz_attempts', array('id' => $attemptid));
if (!$attempt || $attempt->quiz != $quiz->id || $attempt->preview != 0) {
// Ensure the attempt exists, and belongs to this quiz. If not skip.
continue;
@@ -610,8 +612,8 @@ abstract class quiz_attempt_report_table extends table_sql {
$this->sql->fields .= ",\n$fields";
$this->sql->from .= "\nLEFT JOIN $inlineview ON " .
"$alias.questionusageid = quiza.uniqueid AND $alias.slot = $slot";
// TODO params
"$alias.questionusageid = quiza.uniqueid AND $alias.slot = :{$alias}slot";
$this->sql->params[$alias . 'slot'] = $slot;
}
/**
mod/quiz/report/overview/overviewsettings_form.php
+2 -3
View File
@@ -38,8 +38,7 @@ require_once($CFG->libdir . '/formslib.php');
*/
class mod_quiz_report_overview_settings extends moodleform {
function definition() {
global $COURSE; // TODO get rid of this.
public function definition() {
$mform = $this->_form;
$mform->addElement('header', 'preferencespage', get_string('preferencespage', 'quiz_overview'));
@@ -60,7 +59,7 @@ class mod_quiz_report_overview_settings extends moodleform {
if (!$this->_customdata['currentgroup']) {
$options[QUIZ_REPORT_ATTEMPTS_ALL] = get_string('optallattempts','quiz_overview');
}
if ($this->_customdata['currentgroup'] || $COURSE->id != SITEID) {
if ($this->_customdata['currentgroup'] || !is_inside_frontpage($this->_customdata['context'])) {
$options[QUIZ_REPORT_ATTEMPTS_ALL_STUDENTS] = get_string('optallstudents','quiz_overview', $studentsstring);
$options[QUIZ_REPORT_ATTEMPTS_STUDENTS_WITH] =
get_string('optattemptsonly','quiz_overview', $studentsstring);
mod/quiz/report/overview/report.php
+2 -1
View File
@@ -58,7 +58,8 @@ class quiz_overview_report extends quiz_attempt_report {
$qmsubselect = quiz_report_qm_filter_select($quiz);
$mform = new mod_quiz_report_overview_settings($reporturl,
array('qmsubselect' => $qmsubselect, 'quiz' => $quiz, 'currentgroup' => $currentgroup, 'context'=>$this->context));
array('qmsubselect' => $qmsubselect, 'quiz' => $quiz,
'currentgroup' => $currentgroup, 'context' => $this->context));
if ($fromform = $mform->get_data()) {
$regradeall = false;
mod/quiz/report/reportlib.php
+9 -7
View File
@@ -188,15 +188,15 @@ function quiz_report_grade_bands($bandwidth, $bands, $quizid, $userids = array()
global $DB;
if ($userids) {
list($usql, $params) = $DB->get_in_or_equal($userids);
list($usql, $params) = $DB->get_in_or_equal($userids, SQL_PARAMS_NAMED, 'u000000');
$usql = "qg.userid $usql AND";
} else {
$usql ='';
$usql = '';
$params = array();
}
$sql = "
SELECT
FLOOR(qg.grade/$bandwidth) AS band,
FLOOR(qg.grade / :bandwidth1) AS band,
COUNT(1) AS num
FROM {quiz_grades} qg
@@ -204,15 +204,17 @@ JOIN {quiz} q ON qg.quiz = q.id
WHERE
$usql
qg.quiz = ?
qg.quiz = :quizid
GROUP BY
FLOOR(qg.grade/$bandwidth)
FLOOR(qg.grade / :bandwidth2)
ORDER BY
band";
// TODO bandiwth with a placeholder.
$params[] = $quizid;
$params['quizid'] = $quizid;
$params['bandwidth1'] = $bandwidth;
$params['bandwidth2'] = $bandwidth;
$data = $DB->get_records_sql_menu($sql, $params);
mod/quiz/report/responses/report.php
+2 -1
View File
@@ -66,7 +66,8 @@ class quiz_responses_report extends quiz_attempt_report {
$qmsubselect = quiz_report_qm_filter_select($quiz);
$mform = new mod_quiz_report_responses_settings($reporturl,
array('qmsubselect' => $qmsubselect, 'quiz' => $quiz, 'currentgroup' => $currentgroup));
array('qmsubselect' => $qmsubselect, 'quiz' => $quiz,
'currentgroup' => $currentgroup, 'context' => $this->context));
if ($fromform = $mform->get_data()) {
$attemptsmode = $fromform->attemptsmode;
mod/quiz/report/responses/responsessettings_form.php
+2 -3
View File
@@ -38,8 +38,7 @@ require_once($CFG->libdir . '/formslib.php');
*/
class mod_quiz_report_responses_settings extends moodleform {
function definition() {
global $COURSE; // TODO get rid of this.
public function definition() {
$mform = $this->_form;
$mform->addElement('header', 'preferencespage', get_string('preferencespage', 'quiz_overview'));
@@ -60,7 +59,7 @@ class mod_quiz_report_responses_settings extends moodleform {
if (!$this->_customdata['currentgroup']) {
$options[QUIZ_REPORT_ATTEMPTS_ALL] = get_string('optallattempts','quiz_overview');
}
if ($this->_customdata['currentgroup'] || $COURSE->id != SITEID) {
if ($this->_customdata['currentgroup'] || !is_inside_frontpage($this->_customdata['context'])) {
$options[QUIZ_REPORT_ATTEMPTS_ALL_STUDENTS] = get_string('optallstudents','quiz_overview', $studentsstring);
$options[QUIZ_REPORT_ATTEMPTS_STUDENTS_WITH] =
get_string('optattemptsonly','quiz_overview', $studentsstring);
mod/quiz/reviewquestion.php
+4 -4
View File
@@ -48,21 +48,21 @@ $attemptobj = quiz_attempt::create($attemptid);
require_login($attemptobj->get_courseid(), false, $attemptobj->get_cm());
$attemptobj->check_review_capability();
echo $OUTPUT->header();
// Check permissions.
if ($attemptobj->is_own_attempt()) {
if (!$attemptobj->is_finished()) {
echo $OUTPUT->header();
echo $OUTPUT->notification(get_string('cannotreviewopen', 'quiz'));
echo $OUTPUT->close_window_button();
echo $OUTPUT->footer();
die;
die();
} else if (!$options->responses) {
$accessmanager = $attemptobj->get_access_manager(time());
echo $OUTPUT->header();
echo $OUTPUT->notification($accessmanager->cannot_review_message($attemptobj->get_review_options()));
echo $OUTPUT->close_window_button();
echo $OUTPUT->footer();
die;
die();
}
} else if (!$attemptobj->is_review_allowed()) {
mod/quiz/view.php
-1
View File
@@ -118,7 +118,6 @@ if (isguestuser()) {
}
// If they are not enrolled in this course in a good enough role, tell them to enrol.
// TODO, review this.
if (!($canattempt || $canpreview || $canreviewmine)) {
echo $OUTPUT->box('<p>' . get_string('youneedtoenrol', 'quiz') . "</p>\n\n<p>" .
$OUTPUT->continue_button($CFG->wwwroot . '/course/view.php?id=' . $course->id) .
question/behaviour/behaviourbase.php
+4 -4
View File
@@ -118,7 +118,7 @@ abstract class question_behaviour {
*/
public function render(question_display_options $options, $number,
core_question_renderer $qoutput, qtype_renderer $qtoutput) {
$behaviouroutput = $this->get_renderer();
$behaviouroutput = $this->get_renderer($qoutput->get_page());
$options = clone($options);
$this->adjust_display_options($options);
return $qoutput->question($this->qa, $behaviouroutput, $qtoutput, $options, $number);
@@ -139,11 +139,11 @@ abstract class question_behaviour {
}
/**
* @param moodle_page $page the page to render for.
* @return qbehaviour_renderer get the appropriate renderer to use for this model.
*/
public function get_renderer() {
global $PAGE;
return $PAGE->get_renderer(get_class($this));
public function get_renderer(moodle_page $page) {
return $page->get_renderer(get_class($this));
}
/**
question/behaviour/missing/simpletest/testmissingbehaviour.php
+3 -3
View File
@@ -40,21 +40,21 @@ require_once(dirname(__FILE__) . '/../behaviour.php');
*/
class qbehaviour_missing_test extends UnitTestCase {
public function test_missing_cannot_start() {
$qa = new question_attempt(test_question_maker::make_a_truefalse_question(), 0, 0);
$qa = new question_attempt(test_question_maker::make_a_truefalse_question(), 0);
$behaviour = new qbehaviour_missing($qa, 'deferredfeedback');
$this->expectException();
$behaviour->init_first_step(new question_attempt_step(array()));
}
public function test_missing_cannot_process() {
$qa = new question_attempt(test_question_maker::make_a_truefalse_question(), 0, 0);
$qa = new question_attempt(test_question_maker::make_a_truefalse_question(), 0);
$behaviour = new qbehaviour_missing($qa, 'deferredfeedback');
$this->expectException();
$behaviour->process_action(new question_attempt_pending_step(array()));
}
public function test_missing_cannot_get_min_grade() {
$qa = new question_attempt(test_question_maker::make_a_truefalse_question(), 0, 0);
$qa = new question_attempt(test_question_maker::make_a_truefalse_question(), 0);
$behaviour = new qbehaviour_missing($qa, 'deferredfeedback');
$this->expectException();
$behaviour->get_min_fraction();
question/behaviour/opaque/behaviour.php
+1 -1
View File
@@ -156,7 +156,7 @@ class qbehaviour_opaque extends question_behaviour {
$opaquestate = qtype_opaque_update_state($this->qa, $pendingstep);
} catch (SoapFault $sf) {
print_object($sf);
return question_attempt::DISCARD; // TODO
return question_attempt::DISCARD; // TODO better Opaque error handling.
}
if ($opaquestate->resultssequencenumber != $this->qa->get_num_steps()) {
question/editlib.php
+8 -12
View File
@@ -69,17 +69,16 @@ function get_questions_category( $category, $noparent=false, $recurse=true, $exp
$npsql = " and parent='0' ";
}
// get (list) of categories
// Get list of categories
if ($recurse) {
$categorylist = question_categorylist($category->id);
}
else {
$categorylist = $category->id;
} else {
$categorylist = array($category->id);
}
// get the list of questions for the category
list ($usql, $params) = $DB->get_in_or_equal(explode(',', $categorylist));
if ($questions = $DB->get_records_select("question","category $usql $npsql", $params, "qtype, name ASC")) {
list($usql, $params) = $DB->get_in_or_equal($categorylist);
if ($questions = $DB->get_records_select('question', "category $usql $npsql", $params, 'qtype, name')) {
// iterate through questions, getting stuff we need
foreach($questions as $question) {
@@ -1103,7 +1102,7 @@ class question_bank_view {
}
if ($recurse) {
$categoryids = explode(',', question_categorylist($category->id));
$categoryids = question_categorylist($category->id);
} else {
$categoryids = array($category->id);
}
@@ -1302,7 +1301,7 @@ class question_bank_view {
$cmoptions->hasattempts = !empty($this->quizhasattempts);
$strselectall = get_string('selectall');
$strselectnone = get_string('selectnone');
$strselectnone = get_string('deselectall');
$strdelete = get_string('delete');
list($categoryid, $contextid) = explode(',', $categoryandcontext);
@@ -1374,7 +1373,7 @@ class question_bank_view {
}
if ($canmoveall && count($addcontexts)) {
echo '<input type="submit" name="move" value="'.get_string('moveto')."\" />\n";
echo '<input type="submit" name="move" value="'.get_string('moveto', 'question')."\" />\n";
question_category_select_menu($addcontexts, false, 0, "$category->id,$category->contextid");
}
@@ -1561,9 +1560,6 @@ class question_bank_view {
function question_edit_setup($edittab, $baseurl, $requirecmid = false, $requirecourseid = true) {
global $DB, $PAGE;
//$thispageurl is used to construct urls for all question edit pages we link to from this page. It contains an array
//of parameters that are passed from page to page.
// $thispageurl = new moodle_url($PAGE->url); //TODO: this looks dumb, because this method is called BEFORE $PAGE->set_page() !!!!
$thispageurl = new moodle_url($baseurl);
$thispageurl->remove_all_params(); // We are going to explicity add back everything important - this avoids unwanted params from being retained.
question/engine/bank.php
+13 -13
View File
@@ -139,8 +139,9 @@ abstract class question_bank {
foreach (get_plugin_list('qtype') as $plugin => $notused) {
try {
$qtypes[$plugin] = self::get_qtype($plugin);
} catch (Exception $e) {
// TODO ingore, but review this later.
} catch (coding_exception $e) {
// Catching coding_exceptions here means that incompatible
// question types do not cause the rest of Moodle to break.
}
}
return $qtypes;
@@ -317,27 +318,26 @@ abstract class question_bank {
class question_finder {
/**
* Get the ids of all the questions in a list of categoryies.
* @param int|string|array $categoryids either a categoryid, or a comma-separated list
* @param array $categoryids either a categoryid, or a comma-separated list
* category ids, or an array of them.
* @param string $extraconditions extra conditions to AND with the rest of the where clause.
* @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.
* @return array questionid => questionid.
*/
public function get_questions_from_categories($categoryids, $extraconditions) {
public function get_questions_from_categories($categoryids, $extraconditions, $extraparams = array()) {
global $DB;
if (is_array($categoryids)) {
$categoryids = implode(',', $categoryids);
}
list($qcsql, $qcparams) = $DB->get_in_or_equal($categoryids, SQL_PARAMS_NAMED, 'qc0000');
if ($extraconditions) {
$extraconditions = ' AND (' . $extraconditions . ')';
}
// TODO switch to using $DB->in_or_equal.
$questionids = $DB->get_records_select_menu('question',
"category IN ($categoryids)
return $DB->get_records_select_menu('question',
"category $qcsql
AND parent = 0
AND hidden = 0
$extraconditions", array(), '', 'id,id AS id2');
return $questionids;
$extraconditions", $qcparams + $extraparams, '', 'id,id AS id2');
}
}
question/engine/datalib.php
+2 -2
View File
@@ -747,7 +747,8 @@ ORDER BY
* this method.
*
* @param string $qubaid SQL fragment that controls which usage is summed.
* This might be the name of a column in the outer query.
* This will normally be the name of a column in the outer query. Not that this
* SQL fragment must not contain any placeholders.
* @return string SQL code for the subquery.
*/
public function sum_usage_marks_subquery($qubaid) {
@@ -763,7 +764,6 @@ ORDER BY
JOIN {question_attempt_steps} qas ON qas.id = lateststepid.latestid
WHERE qa.questionusageid = $qubaid
HAVING COUNT(CASE WHEN qas.state = 'needsgrading' AND qa.maxmark > 0 THEN 1 ELSE NULL END) = 0";
// TODO handle $qubaid with placeholders.
}
public function question_attempt_latest_state_view($alias) {
question/engine/lib.php
+24 -19
View File
@@ -102,8 +102,11 @@ abstract class question_engine {
}
/**
* Delete a {@link question_usage_by_activity} from the database, based on its id.
* @param int $qubaid the id of the usage to delete.
* Delete {@link question_usage_by_activity}s from the database that match
* an arbitrary SQL where clause.
* @param string $where a where clause. Becuase of MySQL limitations, you
* must refer to {question_usages}.id in full like that.
* @param array $params values to substitute for placeholders in $where.
*/
public static function delete_questions_usage_by_activities($where, $params) {
$dm = new question_engine_data_mapper();
@@ -728,8 +731,7 @@ class question_usage_by_activity {
* @return int the number used to identify this question within this usage.
*/
public function add_question(question_definition $question, $maxmark = null) {
$qa = new question_attempt($question, $this->get_id(),
$this->context->id, $this->observer, $maxmark);
$qa = new question_attempt($question, $this->get_id(), $this->observer, $maxmark);
if (count($this->questionattempts) == 0) {
$this->questionattempts[1] = $qa;
} else {
@@ -1165,7 +1167,7 @@ class question_usage_by_activity {
$this->observer->notify_delete_attempt_steps($oldqa);
$newqa = new question_attempt($oldqa->get_question(), $oldqa->get_usage_id(),
$this->context->id, $this->observer, $newmaxmark);
$this->observer, $newmaxmark);
$newqa->set_database_id($oldqa->get_database_id());
$newqa->regrade($oldqa, $finished);
@@ -1321,10 +1323,6 @@ class question_attempt {
/** @var integer|string the id of the question_usage_by_activity we belong to. */
protected $usageid;
/** @var integer the id of the context this question_attempt belongs to. */
// TODO I don't think this is actually needed.
protected $owningcontextid = null;
/** @var integer the number used to identify this question_attempt within the usage. */
protected $slot = null;
@@ -1395,11 +1393,10 @@ class question_attempt {
* @param number $maxmark the maximum grade for this question_attempt. If not
* passed, $question->defaultmark is used.
*/
public function __construct(question_definition $question, $usageid, $owningcontextid,
public function __construct(question_definition $question, $usageid,
question_usage_observer $observer = null, $maxmark = null) {
$this->question = $question;
$this->usageid = $usageid;
$this->owningcontextid = $owningcontextid;
if (is_null($observer)) {
$observer = new question_usage_null_observer();
}
@@ -1830,10 +1827,13 @@ class question_attempt {
* @param string|null $number The question number to display.
* @return string HTML fragment representing the question.
*/
public function render($options, $number) {
global $PAGE;
$qoutput = $PAGE->get_renderer('core', 'question');
$qtoutput = $this->question->get_renderer();
public function render($options, $number, $page = null) {
if (is_null($page)) {
global $PAGE;
$page = $PAGE;
}
$qoutput = $page->get_renderer('core', 'question');
$qtoutput = $this->question->get_renderer($page);
return $this->behaviour->render($options, $number, $qoutput, $qtoutput);
}
@@ -1842,9 +1842,14 @@ class question_attempt {
* attempt is displayed in the body.
* @return string HTML fragment.
*/
public function render_head_html() {
return $this->question->get_renderer()->head_code($this) .
$this->behaviour->get_renderer()->head_code($this);
public function render_head_html($page = null) {
if (is_null($page)) {
global $PAGE;
$page = $PAGE;
}
// TODO go via behaviour.
return $this->question->get_renderer($page)->head_code($this) .
$this->behaviour->get_renderer($page)->head_code($this);
}
/**
@@ -2221,7 +2226,7 @@ class question_attempt {
}
$qa = new question_attempt($question, $record->questionusageid,
$record->contextid, null, $record->maxmark + 0);
null, $record->maxmark + 0);
$qa->set_database_id($record->questionattemptid);
$qa->set_number_in_usage($record->slot);
$qa->minfraction = $record->minfraction + 0;
question/engine/renderer.php
+3
View File
@@ -37,6 +37,9 @@ defined('MOODLE_INTERNAL') || die();
* @license http://www.gnu.org/copyleft/gpl.html GNU GPL v3 or later
*/
class core_question_renderer extends plugin_renderer_base {
public function get_page() {
return $this->page;
}
/**
* Generate the display of a question in a particular state, and with certain
question/engine/simpletest/helpers.php
+1 -1
View File
@@ -70,7 +70,7 @@ class test_question_maker {
* @return question_attempt the question attempt.
*/
public function get_a_qa($question, $maxmark = 3) {
return new question_attempt($question, 13, 0, null, $maxmark);
return new question_attempt($question, 13, null, $maxmark);
}
/**
question/engine/simpletest/testquestionattempt.php
+5 -5
View File
@@ -51,7 +51,7 @@ class question_attempt_test extends UnitTestCase {
$this->question = test_question_maker::make_a_description_question();
$this->question->defaultmark = 3;
$this->usageid = 13;
$this->qa = new question_attempt($this->question, $this->usageid, 0);
$this->qa = new question_attempt($this->question, $this->usageid);
}
public function tearDown() {
@@ -61,13 +61,13 @@ class question_attempt_test extends UnitTestCase {
}
public function test_constructor_sets_maxmark() {
$qa = new question_attempt($this->question, $this->usageid, 0);
$qa = new question_attempt($this->question, $this->usageid);
$this->assertIdentical($this->question, $qa->get_question());
$this->assertEqual(3, $qa->get_max_mark());
}
public function test_maxmark_beats_default_mark() {
$qa = new question_attempt($this->question, $this->usageid, 0, null, 2);
$qa = new question_attempt($this->question, $this->usageid, null, 2);
$this->assertEqual(2, $qa->get_max_mark());
}
@@ -143,7 +143,7 @@ class question_attempt_with_steps_test extends UnitTestCase {
public function setUp() {
$this->question = test_question_maker::make_a_description_question();
$this->qa = new testable_question_attempt($this->question, 0, 0, null, 2);
$this->qa = new testable_question_attempt($this->question, 0, null, 2);
for ($i = 0; $i < 3; $i++) {
$step = new question_attempt_step(array('i' => $i));
$this->qa->add_step($step);
@@ -247,7 +247,7 @@ class question_attempt_with_steps_test extends UnitTestCase {
}
public function test_cannot_get_min_fraction_before_start() {
$qa = new question_attempt($this->question, null, 0);
$qa = new question_attempt($this->question, 0);
$this->expectException();
$qa->get_min_fraction();
}
question/engine/simpletest/testquestionattemptstepiterator.php
+1 -1
View File
@@ -43,7 +43,7 @@ class question_attempt_step_iterator_test extends UnitTestCase {
public function setUp() {
$question = test_question_maker::make_a_description_question();
$this->qa = new testable_question_attempt($question, 0, 0);
$this->qa = new testable_question_attempt($question, 0);
for ($i = 0; $i < 3; $i++) {
$step = new question_attempt_step(array('i' => $i));
$this->qa->add_step($step);
question/format.php
-16
View File
@@ -838,22 +838,6 @@ class qformat_default {
return NULL;
}
/**
* get directory into which export is going
* @return string file path
*/
function question_get_export_dir() {
// TODO is this still used.
global $USER;
if ($this->canaccessbackupdata) {
$dirname = get_string('exportfilename', 'question');
$path = $this->course->id . '/backupdata/' . $dirname; // backupdata is protected directory
} else {
$path = 'temp/questionexport/' . $USER->id;
}
return $path;
}
/**
* Convert the question text to plain text, so it can safely be displayed
* during import to let the user see roughly what is going on.
question/format/qti_two/format.php
+5 -4
View File
@@ -261,14 +261,15 @@ function handle_questions_media(&$questions, $path, $courseid) {
*/
function exportprocess() {
global $CFG, $OUTPUT;
global $CFG, $OUTPUT, $USER;
$courseid = $this->course->id;
$path = 'temp/qformat_qti_two/' . $USER->id . '/' . $this->filename;
// create a directory for the exports (if not already existing)
if (!$export_dir = make_upload_directory($this->question_get_export_dir().'/'.$this->filename)) {
print_error('cannotcreatepath', 'question', '', $export_dir);
if (!make_upload_directory($path)) {
throw new moodle_exception('cannotcreatepath', 'question', '', $path);
}
$path = $CFG->dataroot.'/'.$this->question_get_export_dir().'/'.$this->filename;
$path = $CFG->dataroot . '/' . $path;
// get the questions (from database) in this category
$questions = get_questions_category( $this->category );
question/import.php
-3
View File
@@ -115,19 +115,16 @@ if ($form = $import_form->get_data()) {
// Do anything before that we need to
if (!$qformat->importpreprocess()) {
//TODO: need more detailed error info
print_error('cannotimport', '', $thispageurl->out());
}
// Process the uploaded file
if (!$qformat->importprocess($category)) {
//TODO: need more detailed error info
print_error('cannotimport', '', $thispageurl->out());
}
// In case anything needs to be done after
if (!$qformat->importpostprocess()) {
//TODO: need more detailed error info
print_error('cannotimport', '', $thispageurl->out());
}
question/type/calculated/backup/moodle2/backup_qtype_calculated_plugin.class.php
-8
View File
@@ -98,15 +98,7 @@ class backup_qtype_calculated_plugin extends backup_qtype_plugin {
* files to be processed both in backup and restore.
*/
public static function get_qtype_fileareas() {
// TODO: Discuss. Commented below are the "in theory" correct
// mappings for those fileareas. Instead we are using question for
// them, that will cause problems in the future if we want to change
// any of them to be 1..n (i.e. we should be always pointing to own id)
return array(
//'instruction' => 'question_numerical_option',
//'correctfeedback' => 'question_calculated_option',
//'partiallycorrectfeedback' => 'question_calculated_option',
//'incorrectfeedback' => 'question_calculated_option');
'instruction' => 'question_created',
'correctfeedback' => 'question_created',
'partiallycorrectfeedback' => 'question_created',
question/type/calculated/questiontype.php
+2 -3
View File
@@ -2145,15 +2145,14 @@ class question_calculated_qtype extends question_type {
}
return true;
} else if ($filearea == 'instruction') {
// TODO: should it be display all the time like questiontext?
// check if question id exists
// Displayed all the time like the question text. Check if question id exists
if ($itemid != $question->id) {
return false;
} else {
return true;
}
} else if (in_array($filearea, array('correctfeedback', 'partiallycorrectfeedback', 'incorrectfeedback'))) {
// TODO: calculated type doesn't display question feedback yet
// Note: calculated type doesn't display question feedback yet
return false;
} else {
return parent::check_file_access($question, $state, $options, $contextid, $component,
question/type/calculatedmulti/questiontype.php
-2
View File
@@ -107,8 +107,6 @@ class question_calculatedmulti_qtype extends question_calculated_qtype {
// Save the units.
$virtualqtype = $this->get_virtual_qtype($question);
// TODO: What is this?
// $result = $virtualqtype->save_numerical_units($question);
if (isset($result->error)) {
return $result;
} else {
question/type/calculatedsimple/questiontype.php
+2 -3
View File
@@ -378,7 +378,7 @@ class question_calculatedsimple_qtype extends question_calculated_qtype {
$itemid = reset($args);
if ($component == 'question' && $filearea == 'answerfeedback') {
// check if answer id exists
// Check if answer id exists
$result = $options->feedback && array_key_exists($itemid, $question->options->answers);
if (!$result) {
return false;
@@ -389,8 +389,7 @@ class question_calculatedsimple_qtype extends question_calculated_qtype {
}
return true;
} else if ($filearea == 'instruction') {
// TODO: should it be display all the time like questiontext?
// check if question id exists
// Displayed all the time like the question text. Check if question id exists
if ($itemid != $question->id) {
return false;
} else {
question/type/missingtype/simpletest/testmissingtype.php
+1 -1
View File
@@ -95,7 +95,7 @@ class qtype_missing_test extends UnitTestCase {
public function test_render_missing() {
$questiondata = $this->get_unknown_questiondata();
$q = question_bank::make_question($questiondata);
$qa = new testable_question_attempt($q, 0, 0);
$qa = new testable_question_attempt($q, 0);
$step = new question_attempt_step(array('answer' => 'frog'));
$step->set_state(question_state::$todo);
question/type/multichoice/backup/moodle2/backup_qtype_multichoice_plugin.class.php
-7
View File
@@ -76,14 +76,7 @@ class backup_qtype_multichoice_plugin extends backup_qtype_plugin {
* files to be processed both in backup and restore.
*/
public static function get_qtype_fileareas() {
// TODO: Discuss. Commented below are the "in theory" correct
// mappings for those fileareas. Instead we are using question for
// them, that will cause problems in the future if we want to change
// any of them to be 1..n (i.e. we should be always pointing to own id)
return array(
//'correctfeedback' => 'question_multichoice',
//'partiallycorrectfeedback' => 'question_multichoice',
//'incorrectfeedback' => 'question_multichoice');
'correctfeedback' => 'question_created',
'partiallycorrectfeedback' => 'question_created',
'incorrectfeedback' => 'question_created');
question/type/multichoice/question.php
+4 -6
View File
@@ -129,9 +129,8 @@ abstract class qtype_multichoice_base extends question_graded_automatically {
* @license http://www.gnu.org/copyleft/gpl.html GNU GPL v3 or later
*/
class qtype_multichoice_single_question extends qtype_multichoice_base {
public function get_renderer() {
global $PAGE; // TODO get rid of this global.
return $PAGE->get_renderer('qtype_multichoice', 'single');
public function get_renderer(moodle_page $page) {
return $page->get_renderer('qtype_multichoice', 'single');
}
public function get_min_fraction() {
@@ -228,9 +227,8 @@ class qtype_multichoice_single_question extends qtype_multichoice_base {
* @license http://www.gnu.org/copyleft/gpl.html GNU GPL v3 or later
*/
class qtype_multichoice_multi_question extends qtype_multichoice_base {
public function get_renderer() {
global $PAGE; // TODO get rid of this global.
return $PAGE->get_renderer('qtype_multichoice', 'multi');
public function get_renderer(moodle_page $page) {
return $page->get_renderer('qtype_multichoice', 'multi');
}
public function get_min_fraction() {
question/type/multichoice/styles.css
-1
View File
@@ -1,4 +1,3 @@
/* TODO */
.que.multichoice .answer .specificfeedback {
display: inline;
padding: 0 0.7em;
question/type/numerical/backup/moodle2/backup_qtype_numerical_plugin.class.php
+1 -7
View File
@@ -82,12 +82,6 @@ class backup_qtype_numerical_plugin extends backup_qtype_plugin {
* files to be processed both in backup and restore.
*/
public static function get_qtype_fileareas() {
// TODO: Discuss. Commented below are the "in theory" correct
// mappings for those fileareas. Instead we are using question for
// them, that will cause problems in the future if we want to change
// any of them to be 1..n (i.e. we should be always pointing to own id)
return array(
//'instruction' => 'question_numerical_option');
'instruction' => 'question_created');
return array('instruction' => 'question_created');
}
}
question/type/numerical/questiontype.php
+1 -1
View File
@@ -1067,7 +1067,7 @@ class question_numerical_qtype extends qtype_shortanswer {
$mform->setType('unitpenalty', PARAM_NUMBER);
$mform->setDefault('unitpenalty', 0.1);
$mform->setDefault('unitgradingtypes', 1);
$mform->addHelpButton('penaltygrp', 'unitpenalty', 'qtype_numerical'); // TODO help did not exist before MDL-21695
$mform->addHelpButton('penaltygrp', 'unitpenalty', 'qtype_numerical');
$mform->setDefault('unitsleft', 0);
$mform->setType('instructions', PARAM_RAW);
$mform->addHelpButton('instructions', 'numericalinstructions', 'qtype_numerical');
question/type/questionbase.php
+2 -3
View File
@@ -204,9 +204,8 @@ abstract class question_definition {
/**
* @return qtype_renderer the renderer to use for outputting this question.
*/
public function get_renderer() {
global $PAGE; // TODO get rid of this global.
return $PAGE->get_renderer('qtype_' . $this->qtype->name());
public function get_renderer(moodle_page $page) {
return $page->get_renderer($this->qtype->plugin_name());
}
/**
question/type/random/questiontype.php
+6 -4
View File
@@ -80,14 +80,16 @@ class qtype_random extends question_type {
if ($question->questiontext) {
$categorylist = question_categorylist($question->category);
} else {
$categorylist = $question->category;
$categorylist = array($question->category);
}
list($qcsql, $qcparams) = $DB->get_in_or_equal($categorylist);
// TODO use in_or_equal for $otherquestionsinuse and $this->manualqtypes
return $DB->record_exists_select('question',
"category IN ($categorylist)
"category $qcsql
AND parent = 0
AND hidden = 0
AND id NOT IN ($otherquestionsinuse)
AND qtype IN ($this->manualqtypes)");
AND qtype IN ($this->manualqtypes)", $qcparams);
}
/**
@@ -186,7 +188,7 @@ class qtype_random extends question_type {
if ($subcategories) {
$categoryids = question_categorylist($categoryid);
} else {
$categoryids = $categoryid;
$categoryids = array($categoryid);
}
$questionids = question_bank::get_finder()->get_questions_from_categories(
question/type/randomsamatch/questiontype.php
+7 -25
View File
@@ -109,36 +109,18 @@ class question_randomsamatch_qtype extends qtype_match {
// recurse into subcategories
$categorylist = question_categorylist($question->category);
} else {
$categorylist = $question->category;
$categorylist = array($question->category);
}
$saquestions = $this->get_sa_candidates($categorylist, $cmoptions->questionsinuse);
$count = count($saquestions);
$wanted = $question->options->choose;
$errorstr = '';
if ($count < $wanted && has_coursecontact_role($USER->id)) { //TODO: this teacher test is far from optimal
if ($count >= 2) {
$errorstr = "Error: could not get enough Short-Answer questions!
Got $count Short-Answer questions, but wanted $wanted.
Reducing number to choose from to $count!";
$wanted = $question->options->choose = $count;
} else {
$errorstr = "Error: could not get enough Short-Answer questions!
This can happen if all available Short-Answer questions are already
taken up by other Random questions or Random Short-Answer question.
Another possible cause for this error is that Short-Answer
questions were deleted after this Random Short-Answer question was
created.";
}
echo $OUTPUT->notification($errorstr);
$errorstr = '<span class="notifyproblem">' . $errorstr . '</span>';
}
if ($count < $wanted) {
$question->questiontext = "$errorstr<br /><br />Insufficient selection options are
available for this question, therefore it is not available in this
quiz. Please inform your teacher.";
$question->questiontext = "Insufficient selection options are
available for this question, therefore it is not available in this
quiz. Please inform your teacher.";
// Treat this as a description from this point on
$question->qtype = DESCRIPTION;
return true;
@@ -263,9 +245,9 @@ class question_randomsamatch_qtype extends qtype_match {
return $response;
}
function get_sa_candidates($categorylist, $questionsinuse=0) {
function get_sa_candidates($categorylist, $questionsinuse = 0) {
global $DB;
list ($usql, $params) = $DB->get_in_or_equal(explode(',', $categorylist));
list ($usql, $params) = $DB->get_in_or_equal($categorylist);
list ($ques_usql, $ques_params) = $DB->get_in_or_equal(explode(',', $questionsinuse), SQL_PARAMS_QM, null, false);
$params = array_merge($params, $ques_params);
return $DB->get_records_select('question',
@@ -310,7 +292,7 @@ class question_randomsamatch_qtype extends qtype_match {
// recurse into subcategories
$categorylist = question_categorylist($question->category);
} else {
$categorylist = $question->category;
$categorylist = array($question->category);
}
$question->options->subquestions = $this->get_sa_candidates($categorylist);
question/type/rendererbase.php
+4 -1
View File
@@ -187,7 +187,10 @@ abstract class qtype_renderer extends plugin_renderer_base {
* @return string HTML fragment.
*/
public function head_code(question_attempt $qa) {
// TODO I think we can get rid of this, but what about Opaque?
// This method is used by the Opaque question type. The remote question
// engine can send back arbitrary CSS that we have to link to in the
// page header. If it was not for that, we might be able to eliminate
// this method and load the required CSS and JS some other way.
$qa->get_question()->qtype->find_standard_scripts();
}