From 0b32036ad0bf44e22ccf70f1daf14c5c9d7353cf Mon Sep 17 00:00:00 2001 From: Martin Gauk Date: Wed, 27 Sep 2023 18:26:30 +0200 Subject: [PATCH] fix: use term placeholders as specified in other docs --- classes/api/attempt_ui.php | 4 ++-- classes/question_ui_renderer.php | 20 +++++++++---------- question.php | 4 ++-- tests/question_ui_renderer_test.php | 19 ++++++------------ .../{parameters.xhtml => placeholder.xhtml} | 1 - 5 files changed, 20 insertions(+), 28 deletions(-) rename tests/question_uis/{parameters.xhtml => placeholder.xhtml} (73%) diff --git a/classes/api/attempt_ui.php b/classes/api/attempt_ui.php index aa411ffa..96440a90 100644 --- a/classes/api/attempt_ui.php +++ b/classes/api/attempt_ui.php @@ -34,8 +34,8 @@ class attempt_ui { /** @var string */ public string $content; - /** @var array mapping of parameter names to the strings they should be replaced with in the content */ - public array $parameters = []; + /** @var array string to string mapping of placeholder names to the values (to be replaced in the content) */ + public array $placeholders = []; /** @var string|null specifics TBD */ public ?string $includeinlinecss = null; diff --git a/classes/question_ui_renderer.php b/classes/question_ui_renderer.php index 157be78c..6d0ace2f 100644 --- a/classes/question_ui_renderer.php +++ b/classes/question_ui_renderer.php @@ -47,8 +47,8 @@ class question_ui_renderer { /** @var DOMDocument $question */ private DOMDocument $question; - /** @var array $parameters */ - private array $parameters; + /** @var array $placeholders */ + private array $placeholders; /** @var question_metadata|null $metadata */ private ?question_metadata $metadata = null; @@ -60,15 +60,15 @@ class question_ui_renderer { * Parses the given XML and initializes a new {@see question_ui_renderer} instance. * * @param string $xml XML as returned by the QPy Server - * @param array $parameters mapping of parameter names to the strings they should be replaced with in the content + * @param array $placeholders string to string mapping of placeholder names to the values * @param int $mtseed the seed to use ({@see mt_srand()}) to make `qpy:shuffle-contents` deterministic */ - public function __construct(string $xml, array $parameters, int $mtseed) { + public function __construct(string $xml, array $placeholders, int $mtseed) { $this->question = new DOMDocument(); $this->question->loadXML($xml); $this->question->normalizeDocument(); - $this->parameters = $parameters; + $this->placeholders = $placeholders; $this->mtseed = $mtseed; } @@ -430,9 +430,10 @@ private function clean_up(DOMXPath $xpath): void { } /** - * Replace placeholder PIs such as `` with the appropriate value from `$parameters`. + * Replace placeholder PIs such as `` with the appropriate value from `$this->placeholders`. * - * Since QPy transformations should not be applied to the content of parameters, this method should be called last. + * Since QPy transformations should not be applied to the content of the placeholders, + * this method should be called last. * * @param DOMXPath $xpath * @return void @@ -441,16 +442,15 @@ private function resolve_placeholders(DOMXPath $xpath): void { /** @var DOMProcessingInstruction $pi */ foreach ($xpath->query("//processing-instruction('p')") as $pi) { $key = trim($pi->data); - $value = utils::array_get_nested($this->parameters, $key); - if (is_null($value)) { + if (!isset($this->placeholders[$key])) { $pi->parentNode->removeChild($pi); } else { /** @var DOMDocumentFragment $frag */ $frag = $xpath->document->createDocumentFragment(); /* TODO: This always interprets the parameter value as XHTML. While supporting markup here is important, perhaps we should allow placeholders to opt out? */ - $frag->appendXML($value); + $frag->appendXML($this->placeholders[$key]); $pi->parentNode->replaceChild($frag, $pi); } } diff --git a/question.php b/question.php index 49ff985c..89e34e4b 100644 --- a/question.php +++ b/question.php @@ -89,7 +89,7 @@ public function start_attempt(question_attempt_step $step, $variant): void { $mtseed = mt_rand(); $step->set_qt_var(self::QT_VAR_MT_SEED, $mtseed); - $this->ui = new question_ui_renderer($attempt->ui->content, $attempt->ui->parameters, $mtseed); + $this->ui = new question_ui_renderer($attempt->ui->content, $attempt->ui->placeholders, $mtseed); } /** @@ -116,7 +116,7 @@ public function apply_attempt_state(question_attempt_step $step) { } $attempt = $this->api->view_attempt($this->packagehash, $this->questionstate, $attemptstate); - $this->ui = new question_ui_renderer($attempt->ui->content, $attempt->ui->parameters, $mtseed); + $this->ui = new question_ui_renderer($attempt->ui->content, $attempt->ui->placeholders, $mtseed); } /** diff --git a/tests/question_ui_renderer_test.php b/tests/question_ui_renderer_test.php index 1741c964..09d2c34e 100644 --- a/tests/question_ui_renderer_test.php +++ b/tests/question_ui_renderer_test.php @@ -273,7 +273,7 @@ public function test_should_shuffle_the_same_way_with_same_seed() { } /** - * Tests that placeholders are replaced with their parameters. + * Tests that placeholders are replaced. * * @return void * @throws DOMException @@ -281,14 +281,11 @@ public function test_should_shuffle_the_same_way_with_same_seed() { * @covers \qtype_questionpy\question_ui_renderer */ public function test_should_resolve_placeholders() { - $input = file_get_contents(__DIR__ . "/question_uis/parameters.xhtml"); + $input = file_get_contents(__DIR__ . "/question_uis/placeholder.xhtml"); $qa = $this->createStub(\question_attempt::class); $ui = new question_ui_renderer($input, [ "param" => "Value of param one", - "nested" => [ - "param" => "Value of param two", - ] ], mt_rand()); $result = $ui->render_formulation($qa, new \question_display_options()); @@ -296,33 +293,29 @@ public function test_should_resolve_placeholders() { $this->assertXmlStringEqualsXmlString(<< Parameter: Value of param one - Nested parameter: Value of param two EXPECTED, $result); } /** - * Tests that placeholders are just removed when the corresponding parameter is null or missing. + * Tests that placeholders are just removed when the corresponding value is missing. * * @return void * @throws DOMException * @throws coding_exception * @covers \qtype_questionpy\question_ui_renderer */ - public function test_should_remove_placeholders_when_no_corresponding_parameter_or_parameter_is_null() { - $input = file_get_contents(__DIR__ . "/question_uis/parameters.xhtml"); + public function test_should_remove_placeholders_when_no_corresponding_value() { + $input = file_get_contents(__DIR__ . "/question_uis/placeholder.xhtml"); $qa = $this->createStub(\question_attempt::class); - $ui = new question_ui_renderer($input, [ - "param" => null - ], mt_rand()); + $ui = new question_ui_renderer($input, [], mt_rand()); $result = $ui->render_formulation($qa, new \question_display_options()); $this->assertXmlStringEqualsXmlString(<< Parameter: - Nested parameter: EXPECTED, $result); } diff --git a/tests/question_uis/parameters.xhtml b/tests/question_uis/placeholder.xhtml similarity index 73% rename from tests/question_uis/parameters.xhtml rename to tests/question_uis/placeholder.xhtml index d043ad98..0426cf46 100644 --- a/tests/question_uis/parameters.xhtml +++ b/tests/question_uis/placeholder.xhtml @@ -1,6 +1,5 @@ Parameter: - Nested parameter: