Skip to content

Commit df0b60e

Browse files
committed
feat: consider editors in is_complete_response
1 parent e9daeda commit df0b60e

File tree

7 files changed

+105
-32
lines changed

7 files changed

+105
-32
lines changed

classes/local/attempt_ui/custom_xhtml_element.php

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -20,7 +20,9 @@
2020
use core\exception\coding_exception;
2121
use DOMElement;
2222
use DOMNode;
23+
use DOMXPath;
2324
use file_exception;
25+
use Iterator;
2426
use moodle_exception;
2527
use question_attempt;
2628
use stored_file_creation_exception;
@@ -34,6 +36,14 @@
3436
* @license http://www.gnu.org/copyleft/gpl.html GNU GPL v3 or later
3537
*/
3638
interface custom_xhtml_element {
39+
/**
40+
* Finds all matching elements within the provided DOMXPath.
41+
*
42+
* @param DOMXPath $xpath
43+
* @return Iterator<static>
44+
*/
45+
public static function find_all_in(DOMXPath $xpath): Iterator;
46+
3747
/**
3848
* Parses the given DOMElement if possible.
3949
*

classes/local/attempt_ui/qpy_file_upload.php

Lines changed: 21 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -22,8 +22,10 @@
2222
use DOMDocument;
2323
use DOMElement;
2424
use DOMNode;
25+
use DOMXPath;
2526
use file_exception;
2627
use form_filemanager;
28+
use Iterator;
2729
use moodle_exception;
2830
use qtype_questionpy\local\files\response_file_service;
2931
use qtype_questionpy\local\files\validatable_upload_limits;
@@ -47,7 +49,7 @@ class qpy_file_upload implements custom_xhtml_element {
4749
*/
4850
private function __construct(
4951
/** @var DOMElement */
50-
private readonly DOMElement $element,
52+
public readonly DOMElement $element,
5153
/** @var string */
5254
public readonly string $name,
5355
) {
@@ -156,4 +158,22 @@ private function render_writable(question_attempt $qa, question_ui_renderer $ren
156158
$html = $filesrenderer->render($fm);
157159
return dom_utils::html_to_fragment($this->element->ownerDocument, $html);
158160
}
161+
162+
/**
163+
* Finds all matching elements within the provided DOMXPath.
164+
*
165+
* @param DOMXPath $xpath
166+
* @return Iterator<static>
167+
*/
168+
public static function find_all_in(DOMXPath $xpath): Iterator {
169+
/** @var DOMElement $element */
170+
foreach ($xpath->query('//qpy:file-upload') as $element) {
171+
$upload = self::from_element($element);
172+
if (!$upload) {
173+
continue;
174+
}
175+
176+
yield $upload;
177+
}
178+
}
159179
}

classes/local/attempt_ui/qpy_rich_text_editor.php

Lines changed: 21 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -22,7 +22,9 @@
2222
use DOMDocumentFragment;
2323
use DOMElement;
2424
use DOMNode;
25+
use DOMXPath;
2526
use file_exception;
27+
use Iterator;
2628
use moodle_exception;
2729
use moodle_url;
2830
use MoodleQuickForm_editor;
@@ -65,7 +67,7 @@ class qpy_rich_text_editor implements custom_xhtml_element {
6567
*/
6668
private function __construct(
6769
/** @var DOMElement */
68-
private readonly DOMElement $element,
70+
public readonly DOMElement $element,
6971
/** @var string */
7072
public readonly string $name,
7173
/** @var bool */
@@ -241,4 +243,22 @@ public function render_writable(question_ui_renderer $renderer, question_attempt
241243

242244
return dom_utils::html_to_fragment($this->element->ownerDocument, $meditor->toHtml());
243245
}
246+
247+
/**
248+
* Finds all matching elements within the provided DOMXPath.
249+
*
250+
* @param DOMXPath $xpath
251+
* @return Iterator<static>
252+
*/
253+
public static function find_all_in(DOMXPath $xpath): Iterator {
254+
/** @var DOMElement $element */
255+
foreach ($xpath->query('//qpy:rich-text-editor') as $element) {
256+
$editor = self::from_element($element);
257+
if (!$editor) {
258+
continue;
259+
}
260+
261+
yield $editor;
262+
}
263+
}
244264
}

classes/local/attempt_ui/question_ui_metadata_extractor.php

Lines changed: 34 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -40,18 +40,26 @@ class question_ui_metadata_extractor {
4040
private DOMXPath $xpath;
4141

4242
/**
43-
* @var array|false $correctresponse `false` if not yet extracted, empty array if not set in the XML.
43+
* @var array|null|false $correctresponse `false` if not yet extracted, null if not set in the XML.
4444
* @see \qtype_questionpy_question::get_correct_response()
4545
*/
4646
private array|null|false $correctresponse = false;
4747

4848
/**
49-
* @var string[] $requiredfields `false` if not yet extracted.
49+
* @var string[]|false $requiredfields `false` if not yet extracted.
5050
* @see \question_manually_gradable::is_complete_response()
5151
* @see \question_manually_gradable::is_gradable_response()
5252
*/
5353
private array|false $requiredfields = false;
5454

55+
/**
56+
* @var string[]|false $requirededitors `false` if not yet extracted.
57+
* @see \question_manually_gradable::is_complete_response()
58+
* @see \question_manually_gradable::is_gradable_response()
59+
*/
60+
private array|false $requirededitors = false;
61+
62+
5563
/**
5664
* Parses the given XML and initializes a new {@see question_ui_metadata_extractor} instance.
5765
*
@@ -67,11 +75,11 @@ public function __construct(string $xml) {
6775
}
6876

6977
/**
70-
* Extracts the names of required fields from the question UI XML.
78+
* Extracts the names of required main response fields from the question UI XML.
7179
*
7280
* @return string[]
7381
*/
74-
public function get_required_fields(): array {
82+
public function get_required_response_fields(): array {
7583
if ($this->requiredfields !== false) {
7684
return $this->requiredfields;
7785
}
@@ -90,11 +98,29 @@ public function get_required_fields(): array {
9098
}
9199
}
92100

93-
// TODO: Include required WYSIWYG editor fields.
94-
95101
return $this->requiredfields;
96102
}
97103

104+
/**
105+
* Extracts the names of required rich text editors from the question UI XML.
106+
*
107+
* @return string[]
108+
*/
109+
public function get_required_editors(): array {
110+
if ($this->requirededitors !== false) {
111+
return $this->requirededitors;
112+
}
113+
114+
$this->requirededitors = [];
115+
foreach (qpy_rich_text_editor::find_all_in($this->xpath) as $editor) {
116+
if ($editor->required) {
117+
$this->requirededitors[] = $editor->name;
118+
}
119+
}
120+
121+
return $this->requirededitors;
122+
}
123+
98124
/**
99125
* Returns the correct response for any fields that use the `@qpy:correct-response` attribute, or null if none do.
100126
*
@@ -132,7 +158,6 @@ public function get_correct_response(): ?array {
132158
return $this->correctresponse;
133159
}
134160

135-
136161
/**
137162
* Returns limits for all `<qpy:file-upload/>` and `<qpy:rich-text-editor/>` elements in the XML, indexed by their name.
138163
*
@@ -142,13 +167,11 @@ public function get_correct_response(): ?array {
142167
public function get_upload_limits(context $attemptcontext): array {
143168
$uploadlimits = [];
144169

145-
foreach ($this->xpath->query('//qpy:file-upload') as $element) {
146-
$upload = qpy_file_upload::from_element($element);
170+
foreach (qpy_file_upload::find_all_in($this->xpath) as $upload) {
147171
$uploadlimits[$upload->name] = $upload->get_limits_in($attemptcontext);
148172
}
149173

150-
foreach ($this->xpath->query('//qpy:rich-text-editor') as $element) {
151-
$editor = qpy_rich_text_editor::from_element($element);
174+
foreach (qpy_rich_text_editor::find_all_in($this->xpath) as $editor) {
152175
$uploadlimits[$editor->name] = $editor->get_limits_in($attemptcontext);
153176
}
154177

classes/local/attempt_ui/question_ui_renderer.php

Lines changed: 7 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -282,7 +282,7 @@ private function replace_shuffled_indices(DOMElement $container, DOMElement $ele
282282
}
283283

284284
/**
285-
* Renders all the `<qpy:file-upload/>`-elements.
285+
* Renders all the `<qpy:file-upload/>` and `<qpy:rich-text-editor/>`-elements.
286286
*
287287
* @param question_attempt $qa
288288
* @throws file_exception
@@ -291,24 +291,14 @@ private function replace_shuffled_indices(DOMElement $container, DOMElement $ele
291291
* @throws moodle_exception
292292
*/
293293
private function render_custom_elements(question_attempt $qa): void {
294-
/** @var DOMElement $element */
295-
foreach (iterator_to_array($this->xpath->query('//qpy:file-upload')) as $element) {
296-
$newnode = qpy_file_upload::from_element($element);
297-
if (!$newnode) {
298-
continue;
299-
}
300-
301-
$element->parentNode->replaceChild($newnode->render($qa, $this), $element);
294+
/** @var qpy_file_upload $upload */
295+
foreach (iterator_to_array(qpy_file_upload::find_all_in($this->xpath)) as $upload) {
296+
$upload->element->parentNode->replaceChild($upload->render($qa, $this), $upload->element);
302297
}
303298

304-
/** @var DOMElement $element */
305-
foreach (iterator_to_array($this->xpath->query('//qpy:rich-text-editor')) as $element) {
306-
$newnode = qpy_rich_text_editor::from_element($element);
307-
if (!$newnode) {
308-
continue;
309-
}
310-
311-
$element->parentNode->replaceChild($newnode->render($qa, $this), $element);
299+
/** @var qpy_rich_text_editor $editor */
300+
foreach (iterator_to_array(qpy_rich_text_editor::find_all_in($this->xpath)) as $editor) {
301+
$editor->element->parentNode->replaceChild($editor->render($qa, $this), $editor->element);
312302
}
313303
}
314304

question.php

Lines changed: 11 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -334,11 +334,21 @@ public function is_complete_response(array $response): bool {
334334
return !empty($qpyresponse);
335335
}
336336

337-
foreach ($this->metadata->get_required_fields() as $requiredfield) {
337+
foreach ($this->metadata->get_required_response_fields() as $requiredfield) {
338338
if (!isset($qpyresponse->{$requiredfield}) || $qpyresponse->{$requiredfield} === '') {
339339
return false;
340340
}
341341
}
342+
343+
$editors = utils::get_qpy_editors_data($response);
344+
foreach ($this->metadata->get_required_editors() as $requirededitorname) {
345+
if (!isset($editors[$requirededitorname]) || $editors[$requirededitorname]->text === '') {
346+
return false;
347+
}
348+
}
349+
350+
// TODO: Check if file uploads have their min-files satisfied (#220).
351+
342352
return true;
343353
}
344354

tests/local/attempt_ui/question_ui_metadata_extractor_test.php

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -42,6 +42,6 @@ public function test_should_extract_correct_metadata(): void {
4242
'my_text' => 'Lorem ipsum dolor sit amet.',
4343
], $metadata->get_correct_response());
4444

45-
$this->assertEquals(['my_number'], $metadata->get_required_fields());
45+
$this->assertEquals(['my_number'], $metadata->get_required_response_fields());
4646
}
4747
}

0 commit comments

Comments
 (0)