Commit 060e0294 authored by Tim Hunt's avatar Tim Hunt
Browse files

MDL-47494 gapselect: Fix codechecker issues in qtype_gapselect.

parent 046d8165
......@@ -62,7 +62,8 @@ class backup_qtype_gapselect_plugin extends backup_qtype_plugin {
$pluginwrapper->add_child($gapselect);
// set source to populate the data
$gapselect->set_source_table('question_gapselect', array('questionid' => backup::VAR_PARENTID));
$gapselect->set_source_table('question_gapselect',
array('questionid' => backup::VAR_PARENTID));
// don't need to annotate ids nor files
......
......@@ -47,10 +47,10 @@ class restore_qtype_gapselect_plugin extends restore_qtype_plugin {
// Add own qtype stuff
$elename = 'gapselect';
$elepath = $this->get_pathfor('/gapselect'); // we used get_recommended_name() so this works
// we used get_recommended_name() so this works
$elepath = $this->get_pathfor('/gapselect');
$paths[] = new restore_path_element($elename, $elepath);
return $paths; // And we return the interesting paths
}
......@@ -76,8 +76,6 @@ class restore_qtype_gapselect_plugin extends restore_qtype_plugin {
$newitemid = $DB->insert_record('question_gapselect', $data);
// Create mapping (needed for decoding links)
$this->set_mapping('question_gapselect', $oldid, $newitemid);
} else {
// Nothing to remap if the question already existed
}
}
......@@ -129,7 +127,8 @@ class restore_qtype_gapselect_plugin extends restore_qtype_plugin {
$contents = array();
$fields = array('correctfeedback', 'partiallycorrectfeedback', 'incorrectfeedback');
$contents[] = new restore_decode_content('question_gapselect', $fields, 'question_gapselect');
$contents[] = new restore_decode_content('question_gapselect',
$fields, 'question_gapselect');
return $contents;
}
......
......@@ -54,7 +54,7 @@ class qtype_gapselect_edit_form_base extends question_edit_form {
private $htmltclosetags = '~<\s*/\s*\w\s*.*?>|<\s*br\s*>~';
/** @var string regex to select text like [[cat]] (including the square brackets). */
private $squareBracketsRegex = '/\[\[[^]]*?\]\]/';
private $squarebracketsregex = '/\[\[[^]]*?\]\]/';
private function get_html_tags($text) {
$textarray = array();
......@@ -65,18 +65,23 @@ class qtype_gapselect_edit_form_base extends question_edit_form {
return $textarray[0];
}
}
preg_match_all($this->htmltstarttagsandattributes, $text, $textarray);
if ($textarray[0]) {
$tag = htmlspecialchars($textarray[0][0]);
$allowedtaglist = $this->get_list_of_printable_allowed_tags($this->allowedhtmltags);
return $tag . " is not allowed (only $allowedtaglist and corresponsing closing tags are allowed)";
return $tag . ' is not allowed (only ' . $allowedtaglist .
' and corresponsing closing tags are allowed)';
}
preg_match_all($this->htmltclosetags, $text, $textarray);
if ($textarray[0]) {
$tag = htmlspecialchars($textarray[0][0]);
$allowedtaglist=$this->get_list_of_printable_allowed_tags($this->allowedhtmltags);
return $tag . " is not allowed HTML tag! (only $allowedtaglist and corresponsing closing tags are allowed)";
return $tag . ' is not allowed (only ' . $allowedtaglist .
' and corresponsing closing tags are allowed)';
}
return false;
}
......@@ -120,14 +125,17 @@ class qtype_gapselect_edit_form_base extends question_edit_form {
if ($this->question->formoptions->repeatelements) {
$defaultstartnumbers = QUESTION_NUMANS_START * 2;
$repeatsatstart = max($defaultstartnumbers, QUESTION_NUMANS_START, $countanswers + QUESTION_NUMANS_ADD);
$repeatsatstart = max($defaultstartnumbers, QUESTION_NUMANS_START,
$countanswers + QUESTION_NUMANS_ADD);
} else {
$repeatsatstart = $countanswers;
}
$repeatedoptions = $this->repeated_options();
$mform->setType('answer', PARAM_RAW);
$this->repeat_elements($textboxgroup, $repeatsatstart, $repeatedoptions, 'noanswers', 'addanswers', QUESTION_NUMANS_ADD, get_string('addmorechoiceblanks', 'qtype_gapselect'));
$this->repeat_elements($textboxgroup, $repeatsatstart, $repeatedoptions,
'noanswers', 'addanswers', QUESTION_NUMANS_ADD,
get_string('addmorechoiceblanks', 'qtype_gapselect'));
}
protected function choice_group($mform) {
......@@ -136,9 +144,12 @@ class qtype_gapselect_edit_form_base extends question_edit_form {
$options[$i] = $i;
}
$grouparray = array();
$grouparray[] = $mform->createElement('text', 'answer', get_string('answer', 'qtype_gapselect'), array('size'=>30, 'class'=>'tweakcss'));
$grouparray[] = $mform->createElement('static', '', '',' '.get_string('group', 'qtype_gapselect').' ');
$grouparray[] = $mform->createElement('select', 'choicegroup', get_string('group', 'qtype_gapselect'), $options);
$grouparray[] = $mform->createElement('text', 'answer',
get_string('answer', 'qtype_gapselect'), array('size'=>30, 'class'=>'tweakcss'));
$grouparray[] = $mform->createElement('static', '', '', ' ' .
get_string('group', 'qtype_gapselect').' ');
$grouparray[] = $mform->createElement('select', 'choicegroup',
get_string('group', 'qtype_gapselect'), $options);
return $grouparray;
}
......@@ -209,7 +220,7 @@ class qtype_gapselect_edit_form_base extends question_edit_form {
}
$matches = array();
preg_match_all($this->squareBracketsRegex, $questiontext, $matches);
preg_match_all($this->squarebracketsregex, $questiontext, $matches);
$slots = $matches[0];
if (!$slots) {
......@@ -236,7 +247,9 @@ class qtype_gapselect_edit_form_base extends question_edit_form {
}
}
if (!$found) {
return $error . "<b>$slot</b> was not found in Choices! (only the choice numbers that exist in choices are allowed to be used a place holders!";
return $error . '<b>' . $slot . '</b> was not found in Choices! ' .
'(only the choice numbers that exist in choices are allowed ' .
'to be used a place holders!';
}
}
return false;
......
......@@ -37,7 +37,7 @@ require_once($CFG->dirroot . '/question/type/gapselect/questionbase.php');
* @license http://www.gnu.org/copyleft/gpl.html GNU GPL v3 or later
*/
class qtype_gapselect_question extends qtype_gapselect_question_base {
//is actually exactly the same.
// Is actually exactly the same.
}
......
......@@ -301,7 +301,8 @@ abstract class qtype_gapselect_question_base extends question_graded_automatical
return $this->check_hint_file_access($qa, $options, $args);
} else {
return parent::check_file_access($qa, $options, $component, $filearea, $args, $forcedownload);
return parent::check_file_access($qa, $options, $component, $filearea,
$args, $forcedownload);
}
}
}
......@@ -76,8 +76,10 @@ class qtype_gapselect extends qtype_gapselect_base {
foreach ($data['#']['selectoption'] as $selectoptionxml) {
$question->choices[] = array(
'answer' => $format->getpath($selectoptionxml, array('#', 'text', 0, '#'), '', true),
'choicegroup' => $format->getpath($selectoptionxml, array('#', 'group', 0, '#'), 1),
'answer' => $format->getpath($selectoptionxml,
array('#', 'text', 0, '#'), '', true),
'choicegroup' => $format->getpath($selectoptionxml,
array('#', 'group', 0, '#'), 1),
);
}
......@@ -101,7 +103,8 @@ class qtype_gapselect extends qtype_gapselect_base {
public function export_to_xml($question, $format, $extra = null) {
$output = '';
$output .= ' <shuffleanswers>' . $question->options->shuffleanswers . "</shuffleanswers>\n";
$output .= ' <shuffleanswers>' . $question->options->shuffleanswers .
"</shuffleanswers>\n";
$output .= $format->write_combined_feedback($question->options);
......@@ -114,95 +117,4 @@ class qtype_gapselect extends qtype_gapselect_base {
return $output;
}
/*
* Backup the data in the question
*
* This is used in question/backuplib.php
*/
public function backup($bf, $preferences, $question, $level = 6) {
$status = true;
$gapselects = get_records("question_gapselect", "questionid", $question, "id");
//If there are gapselect
if ($gapselects) {
//Iterate over each gapselect
foreach ($gapselects as $gapselect) {
$status = fwrite ($bf,start_tag("SDDLS",$level,true));
//Print oumultiresponse contents
fwrite ($bf,full_tag("SHUFFLEANSWERS",$level+1,false,$gapselect->shuffleanswers));
fwrite ($bf,full_tag("CORRECTFEEDBACK",$level+1,false,$gapselect->correctfeedback));
fwrite ($bf,full_tag("PARTIALLYCORRECTFEEDBACK",$level+1,false,$gapselect->partiallycorrectfeedback));
fwrite ($bf,full_tag("INCORRECTFEEDBACK",$level+1,false,$gapselect->incorrectfeedback));
fwrite ($bf,full_tag("SHOWNUMCORRECT",$level+1,false,$gapselect->shownumcorrect));
$status = fwrite ($bf,end_tag("SDDLS",$level,true));
}
//Now print question_answers
$status = question_backup_answers($bf,$preferences,$question);
}
return $status;
}
/**
* Restores the data in the question (This is used in question/restorelib.php)
*
*/
public function restore($old_question_id,$new_question_id,$info,$restore) {
$status = true;
//Get the gapselect array
$gapselects = $info['#']['SDDLS'];
//Iterate over oumultiresponses
for($i = 0; $i < sizeof($gapselects); $i++) {
$mul_info = $gapselects[$i];
//Now, build the question_gapselect record structure
$gapselect = new stdClass();
$gapselect->questionid = $new_question_id;
$gapselect->shuffleanswers = isset($mul_info['#']['SHUFFLEANSWERS']['0']['#'])?backup_todb($mul_info['#']['SHUFFLEANSWERS']['0']['#']):'';
if (array_key_exists("CORRECTFEEDBACK", $mul_info['#'])) {
$gapselect->correctfeedback = backup_todb($mul_info['#']['CORRECTFEEDBACK']['0']['#']);
} else {
$gapselect->correctfeedback = '';
}
if (array_key_exists("PARTIALLYCORRECTFEEDBACK", $mul_info['#'])) {
$gapselect->partiallycorrectfeedback = backup_todb($mul_info['#']['PARTIALLYCORRECTFEEDBACK']['0']['#']);
} else {
$gapselect->partiallycorrectfeedback = '';
}
if (array_key_exists("INCORRECTFEEDBACK", $mul_info['#'])) {
$gapselect->incorrectfeedback = backup_todb($mul_info['#']['INCORRECTFEEDBACK']['0']['#']);
} else {
$gapselect->incorrectfeedback = '';
}
if (array_key_exists('SHOWNUMCORRECT', $mul_info['#'])) {
$gapselect->shownumcorrect = backup_todb($mul_info['#']['SHOWNUMCORRECT']['0']['#']);
} else if (array_key_exists('CORRECTRESPONSESFEEDBACK', $mul_info['#'])) {
$gapselect->shownumcorrect = backup_todb($mul_info['#']['CORRECTRESPONSESFEEDBACK']['0']['#']);
} else {
$gapselect->shownumcorrect = 0;
}
$newid = insert_record ("question_gapselect",$gapselect);
//Do some output
if (($i+1) % 50 == 0) {
if (!defined('RESTORE_SILENTLY')) {
echo ".";
if (($i+1) % 1000 == 0) {
echo "<br />";
}
}
backup_flush(300);
}
if (!$newid) {
$status = false;
}
}
return $status;
}
}
......@@ -84,11 +84,12 @@ abstract class qtype_gapselect_base extends question_type {
}
// Delete old answer records
foreach($oldanswers as $oa) {
foreach ($oldanswers as $oa) {
delete_records('question_answers', 'id', $oa->id);
}
$options = $DB->get_record('question_' . $this->name(), array('questionid' => $question->id));
$options = $DB->get_record('question_' . $this->name(),
array('questionid' => $question->id));
if (!$options) {
$options = new stdClass();
$options->questionid = $question->id;
......@@ -157,7 +158,8 @@ abstract class qtype_gapselect_base extends question_type {
$question->rightchoices = array();
// Break up the question text, and store the fragments, places and right answers.
$bits = preg_split('/\[\[(\d+)]]/', $question->questiontext, null, PREG_SPLIT_DELIM_CAPTURE);
$bits = preg_split('/\[\[(\d+)]]/', $question->questiontext,
null, PREG_SPLIT_DELIM_CAPTURE);
$question->textfragments[0] = array_shift($bits);
$i = 1;
......@@ -199,7 +201,7 @@ abstract class qtype_gapselect_base extends question_type {
protected function get_array_of_choices($question) {
$subquestions = $question->options->answers;
$count = 0;
foreach ($subquestions as $key=>$subquestion) {
foreach ($subquestions as $key => $subquestion) {
$answers[$count]['id'] = $subquestion->id;
$answers[$count]['answer'] = $subquestion->answer;
$answers[$count]['fraction'] = $subquestion->fraction;
......@@ -240,9 +242,9 @@ abstract class qtype_gapselect_base extends question_type {
$arrayofplaceholdeers = $this->get_array_of_placeholders($question);
$correctplayers = array();
foreach($arrayofplaceholdeers as $ph) {
foreach($arrayofchoices as $key=>$choice) {
if(($key+1) == $ph) {
foreach ($arrayofplaceholdeers as $ph) {
foreach ($arrayofchoices as $key => $choice) {
if ($key + 1 == $ph) {
$correctplayers[]= $choice;
}
}
......@@ -253,7 +255,7 @@ abstract class qtype_gapselect_base extends question_type {
protected function get_array_of_placeholders($question) {
$qtext = $question->questiontext;
$error = '<b> ERROR</b>: Please check the form for this question. ';
if(!$qtext) {
if (!$qtext) {
echo $error . 'The question text is empty!';
return false;
}
......@@ -261,7 +263,7 @@ abstract class qtype_gapselect_base extends question_type {
//get the slots
$slots = $this->getEmbeddedTextArray($question);
if(!$slots) {
if (!$slots) {
echo $error . 'The question text is not in the correct format!';
return false;
}
......@@ -271,7 +273,7 @@ abstract class qtype_gapselect_base extends question_type {
$output[] = substr($slot, 2, strlen($slot) - 4); //2 is for '[[' and 4 is for '[[]]'.
}
return $output;
}
}
protected function get_group_of_players($question, $state, $subquestions, $group) {
$goupofanswers = array();
......
......@@ -37,7 +37,8 @@ require_once($CFG->dirroot . '/question/type/gapselect/rendererbase.php');
* @license http://www.gnu.org/copyleft/gpl.html GNU GPL v3 or later
*/
class qtype_gapselect_renderer extends qtype_elements_embedded_in_question_text_renderer {
protected function embedded_element(question_attempt $qa, $place, question_display_options $options) {
protected function embedded_element(question_attempt $qa, $place,
question_display_options $options) {
$question = $qa->get_question();
$group = $question->places[$place];
......@@ -64,13 +65,15 @@ class qtype_gapselect_renderer extends qtype_elements_embedded_in_question_text_
if ($options->correctness) {
$response = $qa->get_last_qt_data();
if (array_key_exists($fieldname, $response)) {
$fraction = (int) ($response[$fieldname] == $question->get_right_choice_for($place));
$fraction = (int) ($response[$fieldname] ==
$question->get_right_choice_for($place));
$attributes['class'] = $this->feedback_class($fraction);
$feedbackimage = $this->feedback_image($fraction);
}
}
$selecthtml = html_writer::select($selectoptions, $qa->get_qt_field_name($fieldname), $value, ' ', $attributes) . ' ' . $feedbackimage;
$selecthtml = html_writer::select($selectoptions, $qa->get_qt_field_name($fieldname),
$value, ' ', $attributes) . ' ' . $feedbackimage;
return html_writer::tag('span', $selecthtml, array('class' => 'control '.$groupclass));
}
......
......@@ -29,13 +29,14 @@ defined('MOODLE_INTERNAL') || die();
/**
* Generates the output for question types where the question includes embedded interactive elements in the
* question text.
* Generates the output for question types where the question includes embedded
* interactive elements in the question text.
*
* @copyright 2011 The Open University
* @license http://www.gnu.org/copyleft/gpl.html GNU GPL v3 or later
*/
abstract class qtype_elements_embedded_in_question_text_renderer extends qtype_with_combined_feedback_renderer {
abstract class qtype_elements_embedded_in_question_text_renderer
extends qtype_with_combined_feedback_renderer {
public function formulation_and_controls(question_attempt $qa,
question_display_options $options) {
......@@ -69,9 +70,11 @@ abstract class qtype_elements_embedded_in_question_text_renderer extends qtype_w
return 'qtext';
}
protected abstract function embedded_element(question_attempt $qa, $place, question_display_options $options);
protected abstract function embedded_element(question_attempt $qa, $place,
question_display_options $options);
protected function post_qtext_elements(question_attempt $qa, question_display_options $options) {
protected function post_qtext_elements(question_attempt $qa,
question_display_options $options) {
return '';
}
......
......@@ -41,7 +41,8 @@ class qtype_gapselect_question_test extends UnitTestCase {
public function test_get_question_summary() {
$gapselect = qtype_gapselect_test_helper::make_a_gapselect_question();
$this->assertEqual('The [[1]] brown [[2]] jumped over the [[3]] dog.; [[1]] -> {quick / slow}; [[2]] -> {fox / dog}; [[3]] -> {lazy / assiduous}',
$this->assertEqual('The [[1]] brown [[2]] jumped over the [[3]] dog.; ' .
'[[1]] -> {quick / slow}; [[2]] -> {fox / dog}; [[3]] -> {lazy / assiduous}',
$gapselect->get_question_summary());
}
......@@ -66,8 +67,8 @@ class qtype_gapselect_question_test extends UnitTestCase {
$gapselect->shufflechoices = false;
$gapselect->start_attempt(new question_attempt_step());
$this->assertEqual('{+} {-} {+} {-}',
$gapselect->summarise_response(array('p1' => '1', 'p2' => '2', 'p3' => '1', 'p4' => '2')));
$this->assertEqual('{+} {-} {+} {-}', $gapselect->summarise_response(
array('p1' => '1', 'p2' => '2', 'p3' => '1', 'p4' => '2')));
}
public function test_get_random_guess_score() {
......@@ -124,8 +125,8 @@ class qtype_gapselect_question_test extends UnitTestCase {
$gapselect->shufflechoices = false;
$gapselect->start_attempt(new question_attempt_step());
$this->assertEqual(array(2, 4),
$gapselect->get_num_parts_right(array('p1' => '1', 'p2' => '1', 'p3' => '1', 'p4' => '1')));
$this->assertEqual(array(2, 4), $gapselect->get_num_parts_right(
array('p1' => '1', 'p2' => '1', 'p3' => '1', 'p4' => '1')));
}
public function test_get_expected_data() {
......@@ -222,12 +223,12 @@ class qtype_gapselect_question_test extends UnitTestCase {
$gapselect->shufflechoices = false;
$gapselect->start_attempt(new question_attempt_step());
$this->assertEqual(array(1, question_state::$gradedright),
$gapselect->grade_response(array('p1' => '1', 'p2' => '2', 'p3' => '1', 'p4' => '2')));
$this->assertEqual(array(0.5, question_state::$gradedpartial),
$gapselect->grade_response(array('p1' => '1', 'p2' => '1', 'p3' => '1', 'p4' => '1')));
$this->assertEqual(array(0, question_state::$gradedwrong),
$gapselect->grade_response(array('p1' => '0', 'p2' => '1', 'p3' => '2', 'p4' => '1')));
$this->assertEqual(array(1, question_state::$gradedright), $gapselect->grade_response(
array('p1' => '1', 'p2' => '2', 'p3' => '1', 'p4' => '2')));
$this->assertEqual(array(0.5, question_state::$gradedpartial), $gapselect->grade_response(
array('p1' => '1', 'p2' => '1', 'p3' => '1', 'p4' => '1')));
$this->assertEqual(array(0, question_state::$gradedwrong), $gapselect->grade_response(
array('p1' => '0', 'p2' => '1', 'p3' => '2', 'p4' => '1')));
}
public function test_classify_response() {
......
......@@ -208,10 +208,14 @@ class qtype_gapselect_test extends UnitTestCase {
$expectedq->penalty = 0.3333333;
$expectedq->shuffleanswers = 1;
$expectedq->correctfeedback = array('text' => '<p>Your answer is correct.</p>', 'format' => FORMAT_MOODLE, 'files' => array());
$expectedq->partiallycorrectfeedback = array('text' => '<p>Your answer is partially correct.</p>', 'format' => FORMAT_MOODLE, 'files' => array());
$expectedq->correctfeedback = array('text' => '<p>Your answer is correct.</p>',
'format' => FORMAT_MOODLE, 'files' => array());
$expectedq->partiallycorrectfeedback = array(
'text' => '<p>Your answer is partially correct.</p>',
'format' => FORMAT_MOODLE, 'files' => array());
$expectedq->shownumcorrect = true;
$expectedq->incorrectfeedback = array('text' => '<p>Your answer is incorrect.</p>', 'format' => FORMAT_MOODLE, 'files' => array());
$expectedq->incorrectfeedback = array('text' => '<p>Your answer is incorrect.</p>',
'format' => FORMAT_MOODLE, 'files' => array());
$expectedq->choices = array(
array('answer' => 'Alpha', 'choicegroup' => 1),
......@@ -221,7 +225,8 @@ class qtype_gapselect_test extends UnitTestCase {
$expectedq->hint = array(
array('text' => 'Try again.', 'format' => FORMAT_MOODLE, 'files' => array()),
array('text' => 'These are the first three letters of the Greek alphabet.', 'format' => FORMAT_MOODLE, 'files' => array()));
array('text' => 'These are the first three letters of the Greek alphabet.',
'format' => FORMAT_MOODLE, 'files' => array()));
$expectedq->hintshownumcorrect = array(true, true);
$expectedq->hintclearwrong = array(false, true);
......@@ -261,7 +266,9 @@ class qtype_gapselect_test extends UnitTestCase {
$qdata->hints = array(
1 => new question_hint_with_parts(1, 'Try again.', FORMAT_MOODLE, true, false),
2 => new question_hint_with_parts(2, 'These are the first three letters of the Greek alphabet.', FORMAT_MOODLE, true, true),
2 => new question_hint_with_parts(2,
'These are the first three letters of the Greek alphabet.',
FORMAT_MOODLE, true, true),
);
$exporter = new qformat_xml();
......
......@@ -100,7 +100,8 @@ class qtype_gapselect_walkthrough_test extends qbehaviour_walkthrough_test_base
$this->get_contains_submit_button_expectation(false),
$this->get_contains_try_again_button_expectation(true),
$this->get_does_not_contain_correctness_expectation(),
new PatternExpectation('/' . preg_quote(get_string('notcomplete', 'qbehaviour_interactive')) . '/'),
new PatternExpectation('/' . preg_quote(
get_string('notcomplete', 'qbehaviour_interactive')) . '/'),
$this->get_contains_hint_expectation('This is the first hint'));
// Do try again.
......
Supports Markdown
0% or .
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment