Skip to content

Commit

Permalink
fix: use term placeholders as specified in other docs
Browse files Browse the repository at this point in the history
  • Loading branch information
MartinGauk committed Sep 27, 2023
1 parent 240eaed commit 0b32036
Show file tree
Hide file tree
Showing 5 changed files with 20 additions and 28 deletions.
4 changes: 2 additions & 2 deletions classes/api/attempt_ui.php
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down
20 changes: 10 additions & 10 deletions classes/question_ui_renderer.php
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -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;
}

Expand Down Expand Up @@ -430,9 +430,10 @@ private function clean_up(DOMXPath $xpath): void {
}

/**
* Replace placeholder PIs such as `<?p my_key?>` with the appropriate value from `$parameters`.
* Replace placeholder PIs such as `<?p my_key?>` 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
Expand All @@ -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);
}
}
Expand Down
4 changes: 2 additions & 2 deletions question.php
Original file line number Diff line number Diff line change
Expand Up @@ -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);
}

/**
Expand All @@ -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);
}

/**
Expand Down
19 changes: 6 additions & 13 deletions tests/question_ui_renderer_test.php
Original file line number Diff line number Diff line change
Expand Up @@ -273,56 +273,49 @@ 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
* @throws coding_exception
* @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 <b>one</b>",
"nested" => [
"param" => "Value of param <b>two</b>",
]
], mt_rand());

$result = $ui->render_formulation($qa, new \question_display_options());

$this->assertXmlStringEqualsXmlString(<<<EXPECTED
<div xmlns="http://www.w3.org/1999/xhtml">
<span>Parameter: Value of param <b>one</b></span>
<span>Nested parameter: Value of param <b>two</b></span>
</div>
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(<<<EXPECTED
<div xmlns="http://www.w3.org/1999/xhtml">
<span>Parameter: </span>
<span>Nested parameter: </span>
</div>
EXPECTED, $result);
}
Expand Down
Original file line number Diff line number Diff line change
@@ -1,6 +1,5 @@
<qpy:question xmlns:qpy="http://questionpy.org/ns/question">
<qpy:formulation>
<span>Parameter: <?p param?></span>
<span>Nested parameter: <?p nested[param]?></span>
</qpy:formulation>
</qpy:question>

0 comments on commit 0b32036

Please sign in to comment.