Skip to content

Commit

Permalink
MDL-47494 gapselect: Fix codechecker issues.
Browse files Browse the repository at this point in the history
Also remove old translations that are now in AMOS, and add some unit
tests.
  • Loading branch information
timhunt committed Mar 13, 2013
1 parent 61ba43b commit 8d6fb0c
Show file tree
Hide file tree
Showing 9 changed files with 198 additions and 151 deletions.
Expand Up @@ -15,8 +15,7 @@
// along with Moodle. If not, see <http://www.gnu.org/licenses/>.

/**
* @package moodlecore
* @subpackage backup-moodle2
* @package qtype_gapselect
* @copyright 2011 The Open University
* @license http://www.gnu.org/copyleft/gpl.html GNU GPL v3 or later
*/
Expand All @@ -26,45 +25,45 @@


/**
* Provides the information to backup gapselect questions
* Provides the information to backup gapselect questions.
*
* @copyright 2011 The Open University
* @license http://www.gnu.org/copyleft/gpl.html GNU GPL v3 or later
*/
class backup_qtype_gapselect_plugin extends backup_qtype_plugin {

/**
* Returns the qtype information to attach to question element
* Returns the qtype information to attach to question element.
*/
protected function define_question_plugin_structure() {

// Define the virtual plugin element with the condition to fulfill
// Define the virtual plugin element with the condition to fulfill.
$plugin = $this->get_plugin_element(null, '../../qtype', 'gapselect');

// Create one standard named plugin element (the visible container)
// Create one standard named plugin element (the visible container).
$pluginwrapper = new backup_nested_element($this->get_recommended_name());

// connect the visible container ASAP
// Connect the visible container ASAP.
$plugin->add_child($pluginwrapper);

// This qtype uses standard question_answers, add them here
// to the tree before any other information that will use them
// to the tree before any other information that will use them.
$this->add_question_question_answers($pluginwrapper);

// Now create the qtype own structures
// Now create the qtype own structures.
$gapselect = new backup_nested_element('gapselect', array('id'), array(
'shuffleanswers', 'correctfeedback', 'correctfeedbackformat',
'partiallycorrectfeedback', 'partiallycorrectfeedbackformat',
'incorrectfeedback', 'incorrectfeedbackformat', 'shownumcorrect'));

// Now the own qtype tree
// Now the own qtype tree.
$pluginwrapper->add_child($gapselect);

// set source to populate the data
// Set source to populate the data.
$gapselect->set_source_table('question_gapselect',
array('questionid' => backup::VAR_PARENTID));

// don't need to annotate ids nor files
// Don't need to annotate ids or files.

return $plugin;
}
Expand Down
Expand Up @@ -15,8 +15,7 @@
// along with Moodle. If not, see <http://www.gnu.org/licenses/>.

/**
* @package moodlecore
* @subpackage backup-moodle2
* @package qtype_gapselect
* @copyright 2011 The Open University
* @license http://www.gnu.org/copyleft/gpl.html GNU GPL v3 or later
*/
Expand All @@ -26,31 +25,31 @@


/**
* restore plugin class that provides the necessary information
* needed to restore one gapselect qtype plugin
* Restore plugin class that provides the necessary information
* needed to restore one gapselect qtype plugin.
*
* @copyright 2011 The Open University
* @license http://www.gnu.org/copyleft/gpl.html GNU GPL v3 or later
*/
class restore_qtype_gapselect_plugin extends restore_qtype_plugin {

/**
* Returns the paths to be handled by the plugin at question level
* Returns the paths to be handled by the plugin at question level.
*/
protected function define_question_plugin_structure() {

$paths = array();

// This qtype uses question_answers, add them
// This qtype uses question_answers, add them.
$this->add_question_question_answers($paths);

// Add own qtype stuff
// Add own qtype stuff.
$elename = '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
return $paths; // And we return the interesting paths.
}

/**
Expand All @@ -62,18 +61,18 @@ public function process_gapselect($data) {
$data = (object)$data;
$oldid = $data->id;

// Detect if the question is created or mapped
// Detect if the question is created or mapped.
$oldquestionid = $this->get_old_parentid('question');
$newquestionid = $this->get_new_parentid('question');
$questioncreated = $this->get_mappingid('question_created', $oldquestionid) ? true : false;

// If the question has been created by restore, we need to create its question_gapselect too
// If the question has been created by restore, we need to create its question_gapselect too.
if ($questioncreated) {
// Adjust some columns
// Adjust some columns.
$data->questionid = $newquestionid;
// Insert record
// Insert record.
$newitemid = $DB->insert_record('question_gapselect', $data);
// Create mapping (needed for decoding links)
// Create mapping (needed for decoding links).
$this->set_mapping('question_gapselect', $oldid, $newitemid);
}
}
Expand Down
73 changes: 40 additions & 33 deletions question/type/gapselect/edit_form_base.php
Expand Up @@ -47,49 +47,59 @@ class qtype_gapselect_edit_form_base extends question_edit_form {
);

/** @var string regex to match HTML open tags. */
private $htmltstarttagsandattributes = '/<\s*\w.*?>/';
private $htmltstarttagsandattributes = '~<\s*\w+\b[^>]*>~';

/** @var string regex to match HTML close tags or br. */
private $htmltclosetags = '~<\s*/\s*\w\s*.*?>|<\s*br\s*>~';
private $htmltclosetags = '~<\s*/\s*\w+\b[^>]*>~';

/** @var string regex to select text like [[cat]] (including the square brackets). */
private $squarebracketsregex = '/\[\[[^]]*?\]\]/';

private function get_html_tags($text) {
$textarray = array();
/**
* Vaidate some input to make sure it does not contain any tags other than
* $this->allowedhtmltags.
* @param unknown_type $text the input to validate.
* @return string any validation errors.
*/
protected function get_illegal_tag_error($text) {
// Remove legal tags.
$strippedtext = $text;
foreach ($this->allowedhtmltags as $htmltag) {
$tagpair = "/<\s*\/?\s*$htmltag\s*.*?>/";
preg_match_all($tagpair, $text, $textarray);
if ($textarray[0]) {
return $textarray[0];
}
$tagpair = "~<\s*/?\s*$htmltag\b\s*[^>]*>~";
$strippedtext = preg_replace($tagpair, '', $strippedtext);
}

preg_match_all($this->htmltstarttagsandattributes, $text, $textarray);
$textarray = array();
preg_match_all($this->htmltstarttagsandattributes, $strippedtext, $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 $this->allowed_tags_message($textarray[0][0]);
}

preg_match_all($this->htmltclosetags, $text, $textarray);
preg_match_all($this->htmltclosetags, $strippedtext, $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 $this->allowed_tags_message($textarray[0][0]);
}

return false;
return '';
}

private function allowed_tags_message($badtag) {
$a = new stdClass();
$a->tag = htmlspecialchars($badtag);
$a->allowed = $this->get_list_of_printable_allowed_tags($this->allowedhtmltags);
if ($a->allowed) {
return get_string('tagsnotallowed', 'qtype_gapselect', $a);
} else {
return get_string('tagsnotallowedatall', 'qtype_gapselect', $a);
}
}

private function get_list_of_printable_allowed_tags($allowedhtmltags) {
$allowedtaglist = null;
$allowedtaglist = array();
foreach ($allowedhtmltags as $htmltag) {
$allowedtaglist .= htmlspecialchars('<'.$htmltag.'>') . ', ';
$allowedtaglist[] = htmlspecialchars('<' . $htmltag . '>');
}
return $allowedtaglist;
return implode(', ', $allowedtaglist);
}

/**
Expand All @@ -99,7 +109,7 @@ private function get_list_of_printable_allowed_tags($allowedhtmltags) {
protected function definition_inner($mform) {
global $CFG;

//add the answer (choice) fields to the form
// Add the answer (choice) fields to the form.
$this->definition_answer_choice($mform);

$this->add_combined_feedback_fields(true);
Expand Down Expand Up @@ -192,21 +202,18 @@ public function validation($data, $files) {
$questiontext = $data['questiontext'];
$choices = $data['choices'];

//check the whether the slots are valid
// Check the whether the slots are valid.
$errorsinquestiontext = $this->validate_slots($questiontext['text'], $choices);
if ($errorsinquestiontext) {
$errors['questiontext'] = $errorsinquestiontext;
}
foreach ($choices as $key => $choice) {
$answer = $choice['answer'];

//check whether the html-tags are allowed tags
$validtags = $this->get_html_tags($answer);
if (is_array($validtags)) {
continue;
}
if ($validtags) {
$errors['choices['.$key.']'] = $validtags;
// Check whether the html-tags are allowed tags.
$tagerror = $this->get_illegal_tag_error($answer);
if ($tagerror) {
$errors['choices['.$key.']'] = $tagerror;
}
}
return $errors;
Expand Down Expand Up @@ -257,4 +264,4 @@ private function validate_slots($questiontext, $choices) {
public function qtype() {
return '';
}
}
}
2 changes: 2 additions & 0 deletions question/type/gapselect/lang/en/qtype_gapselect.php
Expand Up @@ -40,3 +40,5 @@
$string['pluginnameediting'] = 'Editing a select missing words question';
$string['pluginnamesummary'] = 'Missing words in some text are filled in using dropdown menus.';
$string['shuffle'] = 'Shuffle';
$string['tagsnotallowed'] = '{$a->tag} is not allowed. (Only {$a->allowed} are permitted.)';
$string['tagsnotallowedatall'] = '{$a->tag} is not allowed. (No HTML is allowed here.)';
42 changes: 0 additions & 42 deletions question/type/gapselect/lang/fr/qtype_gapselect.php

This file was deleted.

42 changes: 0 additions & 42 deletions question/type/gapselect/lang/ru/qtype_gapselect.php

This file was deleted.

5 changes: 2 additions & 3 deletions question/type/gapselect/questionbase.php
Expand Up @@ -18,8 +18,7 @@
* Definition class for embedded element in question text question. Parent of
* gap-select, drag and drop and possibly others.
*
* @package qtype
* @subpackage gapselect
* @package qtype_gapselect
* @copyright 2011 The Open University
* @license http://www.gnu.org/copyleft/gpl.html GNU GPL v3 or later
*/
Expand All @@ -30,7 +29,7 @@

/**
* Represents embedded element in question text question. Parent of drag and drop and select from
* drop down list and ?others?
* drop down list and others.
*
* @copyright 2011 The Open University
* @license http://www.gnu.org/copyleft/gpl.html GNU GPL v3 or later
Expand Down

0 comments on commit 8d6fb0c

Please sign in to comment.