diff --git a/lang/en/qtype_stack.php b/lang/en/qtype_stack.php
index d1ab312b052..7305de7b12a 100644
--- a/lang/en/qtype_stack.php
+++ b/lang/en/qtype_stack.php
@@ -649,6 +649,9 @@
$string['noroots'] = 'The graph of this PRT has no roots. Does it have nodes?';
$string['structuralproblem'] = 'The PRT structure is malformed.';
$string['missingnextnode'] = 'The PRT structure is malformed. {$a->type} next node for PRT {$a->prt} node {$a->node} is invalid. It has been set to stop.';
+$string['multipleinputs'] = 'Multiple inputs have the same name: {$a}.';
+$string['multipleprts'] = 'Multiple prts have the same name: {$a}.';
+$string['multiplenodes'] = 'Multiple nodes have the name: {$a->node} in PRT: {$a->prt}.';
// Equiv input specific string.
$string['equivnocomments'] = 'You are not permitted to use comments in this input type. Please just work line by line.';
diff --git a/questiontype.php b/questiontype.php
index 6bd3531cb1a..b7fce0a4774 100644
--- a/questiontype.php
+++ b/questiontype.php
@@ -175,6 +175,15 @@ public function save_question_options($fromform) {
break;
}
+ if (!empty($fromform->structuralerror)) {
+ if ($throwexceptions) {
+ throw new stack_exception($fromform->validationerrors);
+ }
+ $result->error = html_writer::tag('h6', $fromform->name);
+ $result->error .= $fromform->validationerrors;
+ return $result;
+ }
+
$context = $fromform->context;
parent::save_question_options($fromform);
@@ -1786,6 +1795,8 @@ public function import_from_xml($xml, $fromform, qformat_xml $format, $notused =
return false;
}
+ $loaderrors = [];
+
$fromform = $format->import_headers($xml);
$fromform->qtype = $this->name();
@@ -1871,8 +1882,15 @@ public function import_from_xml($xml, $fromform, qformat_xml $format, $notused =
$structurerepairs = '';
if (isset($xml['#']['input']) && count($xml['#']['input'])) {
+ $loadedinputs = [];
foreach ($xml['#']['input'] as $inputxml) {
- $this->import_xml_input($inputxml, $fromform, $format);
+ $loadedinput = $this->import_xml_input($inputxml, $fromform, $format);
+ if (in_array($loadedinput, $loadedinputs)) {
+ $loaderrors[$loadedinput . 'input'] = stack_string('multipleinputs', $loadedinput);
+ $fromform->structuralerror = true;
+ } else {
+ $loadedinputs[] = $loadedinput;
+ }
}
} else {
if ($fromform->defaultmark) {
@@ -1883,8 +1901,17 @@ public function import_from_xml($xml, $fromform, qformat_xml $format, $notused =
}
if (isset($xml['#']['prt']) && count($xml['#']['prt'])) {
+ $loadedprts = [];
foreach ($xml['#']['prt'] as $prtxml) {
- $structurerepairs .= $this->import_xml_prt($prtxml, $fromform, $format);
+ [$currentrepairs, $loadedprt, $nodeerrors] = $this->import_xml_prt($prtxml, $fromform, $format);
+ $loaderrors = array_merge($loaderrors, $nodeerrors);
+ $structurerepairs .= $currentrepairs;
+ if (in_array($loadedprt, $loadedprts)) {
+ $loaderrors[$loadedprt . 'prt'] = stack_string('multipleprts', $loadedprt);
+ $fromform->structuralerror = true;
+ } else {
+ $loadedprts[] = $loadedprt;
+ }
}
} else {
if ($fromform->defaultmark) {
@@ -1930,6 +1957,7 @@ public function import_from_xml($xml, $fromform, qformat_xml $format, $notused =
$this->prtgraph = [];
$errors = $this->validate_fromform($formarray, []);
+ $errors = array_merge($loaderrors, $this->validate_fromform($formarray, []));
if ($structurerepairs) {
$errors['structurerepairs'] = $structurerepairs;
}
@@ -1939,11 +1967,9 @@ public function import_from_xml($xml, $fromform, qformat_xml $format, $notused =
foreach ($errors as $key => $error) {
$errortext .= $key . ': ' . $error . '
';
}
- if (isset($errors['structuralerror'])) {
+ if (isset($errors['structuralerror']) || !empty($fromform->structuralerror)) {
// Graph creation failed. If we import this question
// we won't be able to open it in the edit form.
- // TO-DO Once we have a text-based editor we could allow saving
- // of even really broken questions.
$errortext .= stack_string('importwillfail');
} else {
$errortext .= stack_string('markedasbroken');
@@ -1984,6 +2010,8 @@ protected function import_xml_text($xml, $field, qformat_xml $format, $defaultfo
* @param array $xml the bit of the XML representing one input.
* @param object $fromform the data structure we are building from the XML.
* @param qformat_xml $format the importer/exporter object.
+ *
+ * @return string|null name of the input
*/
protected function import_xml_input($xml, $fromform, qformat_xml $format) {
$name = $format->getpath($xml, ['#', 'name', 0, '#'], null, false, 'Missing input name in the XML.');
@@ -2011,6 +2039,8 @@ protected function import_xml_input($xml, $fromform, qformat_xml $format) {
$fromform->{$name . 'showvalidation'}
= $format->getpath($xml, ['#', 'showvalidation', 0, '#'], get_config('qtype_stack', 'inputshowvalidation'));
$fromform->{$name . 'options'} = $format->getpath($xml, ['#', 'options', 0, '#'], '');
+
+ return $name;
}
/**
@@ -2019,7 +2049,7 @@ protected function import_xml_input($xml, $fromform, qformat_xml $format) {
* @param object $fromform the data structure we are building from the XML.
* @param qformat_xml $format the importer/exporter object.
*
- * @return string errors
+ * @return array [errors string, prtname string, loaderrors string]
*/
protected function import_xml_prt($xml, $fromform, qformat_xml $format) {
$errors = [];
@@ -2043,9 +2073,17 @@ protected function import_xml_prt($xml, $fromform, qformat_xml $format) {
$fromform->{$name . $field} = [];
}
+ $loaderrors = [];
if (isset($xml['#']['node'])) {
+ $loadednodes = [];
foreach ($xml['#']['node'] as $nodexml) {
- $this->import_xml_prt_node($nodexml, $name, $fromform, $format);
+ $loadednode = $this->import_xml_prt_node($nodexml, $name, $fromform, $format);
+ if (in_array($loadednode, $loadednodes)) {
+ $loaderrors[$loadednode . 'node'] = stack_string('multiplenodes', ['prt' => $name, 'node' => $loadednode]);
+ $fromform->structuralerror = true;
+ } else {
+ $loadednodes[] = $loadednode;
+ }
}
}
@@ -2070,7 +2108,7 @@ protected function import_xml_prt($xml, $fromform, qformat_xml $format) {
}
}
}
- return implode(' ', $errors);
+ return [implode(' ', $errors), $name, $loaderrors];
;
}
@@ -2080,6 +2118,8 @@ protected function import_xml_prt($xml, $fromform, qformat_xml $format) {
* @param string $prtname the name of the PRT this node belongs to.
* @param object $fromform the data structure we are building from the XML.
* @param qformat_xml $format the importer/exporter object.
+ *
+ * @return string node name
*/
protected function import_xml_prt_node($xml, $prtname, $fromform, qformat_xml $format) {
$name = $format->getpath($xml, ['#', 'name', 0, '#'], null, false, 'Missing PRT node name in the XML.');
@@ -2116,6 +2156,8 @@ protected function import_xml_prt_node($xml, $prtname, $fromform, qformat_xml $f
$format,
FORMAT_HTML
);
+
+ return $name;
}
/**
diff --git a/tests/questiontype_test.php b/tests/questiontype_test.php
index 3a6e8c96231..985aebc2e6e 100644
--- a/tests/questiontype_test.php
+++ b/tests/questiontype_test.php
@@ -369,7 +369,7 @@ public function test_xml_import(): void {
- [[feedback:firsttree]]
+ [[feedback:firsttree]][[feedback:secondtree]]
@@ -413,6 +413,24 @@ public function test_xml_import(): void {
1
+
+ ans2
+ algebraic
+ 2
+ 5
+ 1
+ 0
+
+ 0
+
+
+ 1
+ 0
+ 0
+ 1
+ 1
+
+
firsttree
1
@@ -445,6 +463,62 @@ public function test_xml_import(): void {
+
+ secondtree
+ 1
+ 1
+
+
+
+
+ 0
+ EqualComAss
+ ans2
+ 2
+
+ 0
+ =
+ 1
+ 0
+ -1
+ secondtree-1-T
+
+
+
+ =
+ 0
+ 0
+ 1
+ secondtree-1-F
+
+
+
+
+
+ 1
+ EqualComAss
+ ans2
+ 2
+
+ 0
+ =
+ 1
+ 0
+ -1
+ secondtree-2-T
+
+
+
+ =
+ 0
+ 0
+ -1
+ secondtree-2-F
+
+
+
+
+
12345
1
@@ -479,7 +553,8 @@ public function test_xml_import(): void {
$expectedq = new stdClass();
$expectedq->qtype = 'stack';
$expectedq->name = 'test-0';
- $expectedq->questiontext = 'What is $1+1$? [[input:ans1]] [[validation:ans1]]';
+ $expectedq->questiontext
+ = 'What is $1+1$? [[input:ans1]] [[validation:ans1]]';
$expectedq->questiontextformat = FORMAT_HTML;
$expectedq->generalfeedback = '';
$expectedq->generalfeedbackformat = FORMAT_HTML;
@@ -488,7 +563,10 @@ public function test_xml_import(): void {
$expectedq->penalty = 0.3333333;
$expectedq->questionvariables = '';
- $expectedq->specificfeedback = ['text' => '[[feedback:firsttree]]', 'format' => FORMAT_HTML, 'files' => []];
+ $expectedq->specificfeedback = ['text' => '[[feedback:firsttree]][[feedback:secondtree]]',
+ 'format' => FORMAT_HTML,
+ 'files' => [],
+ ];
$expectedq->questionnote = ['text' => '', 'format' => FORMAT_HTML, 'files' => []];
$expectedq->questionsimplify = 1;
$expectedq->assumepositive = 0;
@@ -531,6 +609,22 @@ public function test_xml_import(): void {
$expectedq->ans1showvalidation = 1;
$expectedq->ans1options = '';
+ $expectedq->ans2type = 'algebraic';
+ $expectedq->ans2modelans = 2;
+ $expectedq->ans2boxsize = 5;
+ $expectedq->ans2strictsyntax = 1;
+ $expectedq->ans2insertstars = 0;
+ $expectedq->ans2syntaxhint = '';
+ $expectedq->ans2syntaxattribute = 0;
+ $expectedq->ans2forbidwords = '';
+ $expectedq->ans2allowwords = '';
+ $expectedq->ans2forbidfloat = 1;
+ $expectedq->ans2requirelowestterms = 0;
+ $expectedq->ans2checkanswertype = 0;
+ $expectedq->ans2mustverify = 1;
+ $expectedq->ans2showvalidation = 1;
+ $expectedq->ans2options = '';
+
$expectedq->firsttreevalue = 1;
$expectedq->firsttreeautosimplify = 1;
$expectedq->firsttreefeedbackstyle = 1;
@@ -553,6 +647,46 @@ public function test_xml_import(): void {
$expectedq->firsttreefalseanswernote[0] = 'firsttree-1-F';
$expectedq->firsttreefalsefeedback[0] = ['text' => '', 'format' => FORMAT_HTML, 'files' => []];
+ $expectedq->secondtreevalue = 1;
+ $expectedq->secondtreeautosimplify = 1;
+ $expectedq->secondtreefeedbackstyle = 1;
+ $expectedq->secondtreefeedbackvariables = '';
+ $expectedq->secondtreeanswertest[0] = 'EqualComAss';
+ $expectedq->secondtreesans[0] = 'ans2';
+ $expectedq->secondtreetans[0] = '2';
+ $expectedq->secondtreetestoptions[0] = '';
+ $expectedq->secondtreequiet[0] = 0;
+ $expectedq->secondtreetruescoremode[0] = '=';
+ $expectedq->secondtreetruescore[0] = 1;
+ $expectedq->secondtreetruepenalty[0] = 0;
+ $expectedq->secondtreetruenextnode[0] = -1;
+ $expectedq->secondtreetrueanswernote[0] = 'secondtree-1-T';
+ $expectedq->secondtreetruefeedback[0] = ['text' => '', 'format' => FORMAT_HTML, 'files' => []];
+ $expectedq->secondtreefalsescoremode[0] = '=';
+ $expectedq->secondtreefalsescore[0] = 0;
+ $expectedq->secondtreefalsepenalty[0] = 0;
+ $expectedq->secondtreefalsenextnode[0] = 1;
+ $expectedq->secondtreefalseanswernote[0] = 'secondtree-1-F';
+ $expectedq->secondtreefalsefeedback[0] = ['text' => '', 'format' => FORMAT_HTML, 'files' => []];
+
+ $expectedq->secondtreeanswertest[1] = 'EqualComAss';
+ $expectedq->secondtreesans[1] = 'ans2';
+ $expectedq->secondtreetans[1] = '2';
+ $expectedq->secondtreetestoptions[1] = '';
+ $expectedq->secondtreequiet[1] = 0;
+ $expectedq->secondtreetruescoremode[1] = '=';
+ $expectedq->secondtreetruescore[1] = 1;
+ $expectedq->secondtreetruepenalty[1] = 0;
+ $expectedq->secondtreetruenextnode[1] = -1;
+ $expectedq->secondtreetrueanswernote[1] = 'secondtree-2-T';
+ $expectedq->secondtreetruefeedback[1] = ['text' => '', 'format' => FORMAT_HTML, 'files' => []];
+ $expectedq->secondtreefalsescoremode[1] = '=';
+ $expectedq->secondtreefalsescore[1] = 0;
+ $expectedq->secondtreefalsepenalty[1] = 0;
+ $expectedq->secondtreefalsenextnode[1] = -1;
+ $expectedq->secondtreefalseanswernote[1] = 'secondtree-2-F';
+ $expectedq->secondtreefalsefeedback[1] = ['text' => '', 'format' => FORMAT_HTML, 'files' => []];
+
$expectedq->deployedseeds = ['12345'];
$qtest = new stack_question_test('', ['ans1' => '2'], 1);
@@ -569,6 +703,57 @@ public function test_xml_import(): void {
$this->assertEquals($expectedq->deployedseeds, $q->deployedseeds); // Redundant, but gives better fail messages.
$this->assertEquals($expectedq->testcases, $q->testcases); // Redundant, but gives better fail messages.
$this->assert(new question_check_specified_fields_expectation($expectedq), $q);
+
+ $this->assertEquals(null, $q->structuralerror ?? null);
+ $this->assertEquals(null, $q->validationerrors ?? null);
+
+ $xmldata['question']['#']['input'][1]['#']['name'][0]['#'] = 'ans1';
+ $q = $importer->try_importing_using_qtypes(
+ $xmldata['question'],
+ null,
+ null,
+ 'stack'
+ );
+
+ $this->assertEquals(true, $q->structuralerror);
+ $this->assertEquals(
+ 'ans1input: Multiple inputs have the same name: ans1.' .
+ '
This question cannot be saved or imported in its current state.',
+ $q->validationerrors
+ );
+
+ $xmldata['question']['#']['input'][1]['#']['name'][0]['#'] = 'ans2';
+ $xmldata['question']['#']['prt'][1]['#']['name'][0]['#'] = 'firsttree';
+ $q = $importer->try_importing_using_qtypes(
+ $xmldata['question'],
+ null,
+ null,
+ 'stack'
+ );
+
+ $this->assertEquals(true, $q->structuralerror);
+ $this->assertEquals(
+ 'firsttreeprt: Multiple prts have the same name: firsttree.
' .
+ 'secondtreevalue: This PRT must be set up before the question can be saved. ' .
+ '
This question cannot be saved or imported in its current state.',
+ $q->validationerrors
+ );
+
+ $xmldata['question']['#']['prt'][1]['#']['name'][0]['#'] = 'secondtree';
+ $xmldata['question']['#']['prt'][1]['#']['node'][1]['#']['name'][0]['#'] = '0';
+ $q = $importer->try_importing_using_qtypes(
+ $xmldata['question'],
+ null,
+ null,
+ 'stack'
+ );
+
+ $this->assertEquals(true, $q->structuralerror);
+ $this->assertEquals(
+ '0node: Multiple nodes have the name: 0 in PRT: secondtree.
' .
+ 'This question cannot be saved or imported in its current state.',
+ $q->validationerrors
+ );
}
public function test_get_input_names_from_question_text_input_only(): void {