Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add template logic for nested data #30

Merged
merged 4 commits into from
Oct 25, 2018
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion composer.json
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,7 @@
},
"extra": {
"branch-alias": {
"dev-master": "1.0.x-dev"
"dev-master": "1.2.x-dev"
},
"installer-name": "components"
},
Expand Down
30 changes: 30 additions & 0 deletions src/SilbinaryWolf/ArrayListExportable.php
Original file line number Diff line number Diff line change
@@ -0,0 +1,30 @@
<?php

/**
* Used in place of ArrayList when preparing nested arrays for export for SS templates.
*
* var_export calls '__set_state' on classes, so it produces code like:
*
* ArrayList::__set_state(array('items' => [...]))
*
* And because ArrayList doens't implement '__set_state', executing the code throws errors.
* So we work around this by using an ArrayListExportable to produce:
*
* ArrayListExportable::__set_state(array('items' => [...]))
*
* And implement '__set_state' to return a constructed ArrayList.
*/
class ArrayListExportable
{
public function __construct($array = array())
{
// need to store items for var_export to recurse
$this->items = $array;
}

public static function __set_state ($array)
{
// when executed, we naruto-style body-replace with in an ArrayList
return new ArrayList($array['items']);
}
}
30 changes: 29 additions & 1 deletion src/SilbinaryWolf/Components/ComponentService.php
Original file line number Diff line number Diff line change
Expand Up @@ -90,7 +90,8 @@ public function generateTemplateCode(array $res, $parser)
}
foreach ($jsonData as $propertyName => $value) {
if (is_array($value)) {
$value = 'new '.'ArrayList'.'('.var_export($value, true).')';
// export valid template logic for nested data
$value = self::exportNestedDataForTemplates($value);
}
$phpCodeValueParts[] = "\$_props['".$propertyName."'][] = ".$value.";";
}
Expand Down Expand Up @@ -185,6 +186,33 @@ public function generateTemplateCode(array $res, $parser)
return $result;
}

/**
* Recursively replace nonassociative arrays with ArrayListExportable and
* output with 'var_export' to produce template logic for the nested data.
*
* @param array $array The nested data to export
* @param bool $root Ignore this
* @return string Executable template logic
*/
private static function exportNestedDataForTemplates(array $array, $root = true)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is there a particular reason this is static?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It could be a method, but as a static function it's clear that it doesn't interact with $this at all, it takes nestsed data, recurses it, and spits back the output. Whilst I think this is technically better/clearer, I'm happy to change it if you prefer.

Copy link
Contributor

@nglasl nglasl Oct 23, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hmm, while I don't disagree, if we're going to make it static I would lean towards making it public so it can be accessed from outside the service class. But for consistency purposes, it should probably stay public and non-static. Happy to leave this one to your judgement, just let me know what you decide on :)

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

My vote is:

  • private: because it's too specific and niche to be externally useful. Else I would be all for making it public.
  • static: because it works independant of $this which is context I would find useful when navigating unfamiliar code, etc.

But I'm the noob here, so happy to be overridden.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

As discussed IRL, I can always see someone trying to access a private function, no matter how niche it is :) But I agree, private and static makes the most sense!

{
// depth first
foreach ($array as $prop => &$value) {
if (is_array($value)) {
$value = self::exportNestedDataForTemplates($value, false);
}
}
unset($value);

// json data expected to be keyed with ints, over the usual strings
if (isset($array[0])) {
// replace array with exportable array list
$array = new ArrayListExportable($array);
}

return $root ? var_export($array, true) : $array;
}

/**
* @return DBComponentField|SS_List|DataObject|ArrayData
*/
Expand Down
107 changes: 107 additions & 0 deletions tests/ComponentTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -528,6 +528,113 @@ public function testJSONPropertyErrorHandling()
}
}

/**
* Test iterating nested arrays in templates
*/
public function testJSONDeeplyNested()
{
$template = <<<SSTemplate
<:JSONNestedTest
_json='{
"NestedData": [
{
"Title": "Item 1.1",
"Children": [
{
"Title": "Item 2.1"
},
{
"Title": "Item 2.2"
},
{
"Title": "item 2.3",
"Children": [
{
"Title": "Item 3.1"
},
{
"Title": "Item 3.2"
}
]
}
]
},
{
"Title": "Item 1.2"
},
{
"Title": "Item 1.3",
"Children": [
{
"Title": "Item 2.4",
"Children": [
{
"Title": "Item 3.3"
},
{
"Title": "Item 3.4"
}
]
},
{
"Title": "Item 2.5"
},
{
"Title": "item 2.6"
}
]
}
]
}'
/>
SSTemplate;
$expectedHTML = <<<HTML
<h2>Item 1.1</h2>
<ul>
<li>
<h3>Item 2.1</h3>
</li>
<li>
<h3>Item 2.2</h3>
</li>
<li>
<h3>item 2.3</h3>
<ul>
<li>
<h4>Item 3.1</h4>
</li>
<li>
<h4>Item 3.2</h4>
</li>
</ul>
</li>
</ul>
<h2>Item 1.2</h2>
<h2>Item 1.3</h2>
<ul>
<li>
<h3>Item 2.4</h3>
<ul>
<li>
<h4>Item 3.3</h4>
</li>
<li>
<h4>Item 3.4</h4>
</li>
</ul>
</li>
<li>
<h3>Item 2.5</h3>
</li>
<li>
<h3>item 2.6</h3>
</li>
</ul>
HTML;
$resultHTML = SSViewer::fromString($template)->process(null);
$this->assertEqualIgnoringWhitespace($expectedHTML, $resultHTML, 'Unexpected output');
}

/**
* Taken from "framework\tests\view\SSViewerTest.php"
*/
Expand Down
23 changes: 23 additions & 0 deletions tests/templates/components/JSONNestedTest.ss
Original file line number Diff line number Diff line change
@@ -0,0 +1,23 @@
<% if $NestedData %>
<% loop $NestedData %>
<h2>$Title</h2>
<% if $Children %>
<ul>
<% loop $Children %>
<li>
<h3>$Title</h3>
<% if $Children %>
<ul>
<% loop $Children %>
<li>
<h4>$Title</h4>
</li>
<% end_loop %>
</ul>
<% end_if %>
</li>
<% end_loop %>
</ul>
<% end_if %>
<% end_loop %>
<% end_if %>