Skip to content

Commit

Permalink
FEATURE: Adjust disabled feature to work with NodeMutator architecture
Browse files Browse the repository at this point in the history
- introduce $showDisabledNodesInSubgraph in tests to show disabled created nodes
- introduce `NodeMutator::setDisabled`
- updated all snapshots to include `"disabled": false,`
  • Loading branch information
mhsdesign committed Jun 22, 2024
1 parent 17800c6 commit c682516
Show file tree
Hide file tree
Showing 28 changed files with 150 additions and 33 deletions.
12 changes: 3 additions & 9 deletions Classes/Domain/NodeCreation/NodeCreationService.php
Original file line number Diff line number Diff line change
Expand Up @@ -53,12 +53,9 @@ public function createMutatorsForRootTemplate(RootTemplate $template, NodeType $
$this->referencesProcessor->processAndValidateReferences($node, $processingErrors)
);

if ($template->getDisabled() === true) {
$node->setHidden(true);
}

return NodeMutatorCollection::from(
NodeMutator::setProperties($validProperties),
NodeMutator::setDisabled($template->getDisabled()),
$this->createMutatorForUriPathSegment($template->getProperties()),
)->merge(
$this->createMutatorsForChildNodeTemplates(
Expand All @@ -71,7 +68,7 @@ public function createMutatorsForRootTemplate(RootTemplate $template, NodeType $

private function createMutatorsForChildNodeTemplates(Templates $templates, TransientNode $parentNode, ProcessingErrors $processingErrors): NodeMutatorCollection
{
$nodeMutators = NodeMutatorCollection::empty();
$nodeMutators = NodeMutatorCollection::createEmpty();

// `hasAutoCreatedChildNode` actually has a bug; it looks up the NodeName parameter against the raw configuration instead of the transliterated NodeName
// https://github.com/neos/neos-ui/issues/3527
Expand Down Expand Up @@ -157,6 +154,7 @@ private function createMutatorsForChildNodeTemplates(Templates $templates, Trans
NodeMutatorCollection::from(
NodeMutator::createAndSelectNode($template->getType(), $template->getName()),
NodeMutator::setProperties($validProperties),
NodeMutator::setDisabled($template->getDisabled()),
$this->createMutatorForUriPathSegment($template->getProperties())
)->merge($this->createMutatorsForChildNodeTemplates(
$template->getChildNodes(),
Expand All @@ -168,10 +166,6 @@ private function createMutatorsForChildNodeTemplates(Templates $templates, Trans

}

if ($template->getDisabled() === true) {
$node->setHidden(true);
}

return $nodeMutators;
}

Expand Down
13 changes: 13 additions & 0 deletions Classes/Domain/NodeCreation/NodeMutator.php
Original file line number Diff line number Diff line change
Expand Up @@ -48,6 +48,19 @@ public static function setProperties(array $properties): self
});
}

/**
* Queues to disable the current node.
*
* Preserves the current node pointer.
*/
public static function setDisabled(bool $disabled): self
{
return new self(function (NodeInterface $nodePointer) use ($disabled) {
$nodePointer->setHidden($disabled);
return null;
});
}

/**
* Queues to execute the collection {@see NodeMutatorCollection} on the current node.
* Any selections made in the collection {@see self::selectChildNode()} won't change the pointer to $this current node.
Expand Down
2 changes: 1 addition & 1 deletion Classes/Domain/NodeCreation/NodeMutatorCollection.php
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,7 @@ public static function from(NodeMutator ...$items): self
return new self(...$items);
}

public static function empty(): self
public static function createEmpty(): self
{
return new self();
}
Expand Down
2 changes: 1 addition & 1 deletion Classes/Domain/NodeCreation/PropertiesProcessor.php
Original file line number Diff line number Diff line change
Expand Up @@ -95,7 +95,7 @@ private function assertValidPropertyName($propertyName): void
if ($propertyName[0] === '_') {
$lowerPropertyName = strtolower($propertyName);
if ($lowerPropertyName === '_hidden') {
throw new PropertyIgnoredException('Using "_hidden" as property declaration was removed. Please use "disabled" on the first level instead.');
throw new PropertyIgnoredException('Using "_hidden" as property declaration was removed. Please use "disabled" on the first level instead.', 1719079679);
}
foreach ($legacyInternalProperties as $legacyInternalProperty) {
if ($lowerPropertyName === strtolower($legacyInternalProperty)) {
Expand Down
8 changes: 4 additions & 4 deletions Classes/Domain/Template/RootTemplate.php
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@
*/
class RootTemplate implements \JsonSerializable
{
private ?bool $disabled;
private bool $disabled;

/**
* @var array<string, mixed>
Expand All @@ -25,7 +25,7 @@ class RootTemplate implements \JsonSerializable
* @internal
* @param array<string, mixed> $properties
*/
public function __construct(?bool $disabled, array $properties, Templates $childNodes)
public function __construct(bool $disabled, array $properties, Templates $childNodes)
{
$this->disabled = $disabled;
$this->properties = $properties;
Expand All @@ -34,10 +34,10 @@ public function __construct(?bool $disabled, array $properties, Templates $child

public static function empty(): self
{
return new RootTemplate(null, [], Templates::empty());
return new RootTemplate(false, [], Templates::empty());
}

public function getDisabled(): ?bool
public function getDisabled(): bool
{
return $this->disabled;
}
Expand Down
6 changes: 3 additions & 3 deletions Classes/Domain/Template/Template.php
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@ class Template implements \JsonSerializable

private ?NodeName $name;

private ?bool $disabled;
private bool $disabled;

/**
* @var array<string, mixed>
Expand All @@ -31,7 +31,7 @@ class Template implements \JsonSerializable
* @internal
* @param array<string, mixed> $properties
*/
public function __construct(?NodeTypeName $type, ?NodeName $name, ?bool $disabled, array $properties, Templates $childNodes)
public function __construct(?NodeTypeName $type, ?NodeName $name, bool $disabled, array $properties, Templates $childNodes)
{
$this->type = $type;
$this->name = $name;
Expand All @@ -50,7 +50,7 @@ public function getName(): ?NodeName
return $this->name;
}

public function getDisabled(): ?bool
public function getDisabled(): bool
{
return $this->disabled;
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -139,7 +139,7 @@ private function createTemplateFromTemplatePart(TemplatePart $templatePart): Tem
return new Template(
$type !== null ? NodeTypeName::fromString($type) : null,
$name !== null ? NodeName::fromString(Utility::renderValidNodeName($name)) : null,
$templatePart->hasConfiguration('disabled') ? (bool)$templatePart->processConfiguration('disabled') : null,
(bool)$templatePart->processConfiguration('disabled'),
$processedProperties,
$childNodeTemplates
);
Expand Down
28 changes: 17 additions & 11 deletions Tests/Functional/AbstractNodeTemplateTestCase.php
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,8 @@ abstract class AbstractNodeTemplateTestCase extends FunctionalTestCase
use WithConfigurationTrait;
use FakeNodeTypeManagerTrait;

protected static bool $showDisabledNodesInSubgraph = false;

protected static $testablePersistenceEnabled = true;

private ContextFactoryInterface $contextFactory;
Expand All @@ -44,21 +46,21 @@ abstract class AbstractNodeTemplateTestCase extends FunctionalTestCase

private string $fixturesDir;

/** @deprecated please use {@see self::getObject()} instead */
/** @internal please use {@see self::getObject()} instead */
protected $objectManager;

public function setUp(): void
{
parent::setUp();

$this->nodeTypeManager = $this->objectManager->get(NodeTypeManager::class);
$this->nodeTypeManager = $this->getObject(NodeTypeManager::class);

$this->loadFakeNodeTypes();

$this->setupContentRepository();
$this->nodeTemplateDumper = $this->objectManager->get(NodeTemplateDumper::class);
$this->nodeTemplateDumper = $this->getObject(NodeTemplateDumper::class);

$templateFactory = $this->objectManager->get(TemplateConfigurationProcessor::class);
$templateFactory = $this->getObject(TemplateConfigurationProcessor::class);

$templateFactoryMock = $this->getMockBuilder(TemplateConfigurationProcessor::class)->disableOriginalConstructor()->getMock();
$templateFactoryMock->expects(self::once())->method('processTemplateConfiguration')->willReturnCallback(function (...$args) use($templateFactory) {
Expand All @@ -76,7 +78,7 @@ public function tearDown(): void
{
parent::tearDown();
$this->inject($this->contextFactory, 'contextInstances', []);
$this->objectManager->get(FeedbackCollection::class)->reset();
$this->getObject(FeedbackCollection::class)->reset();
$this->objectManager->forgetInstance(ContentDimensionRepository::class);
$this->objectManager->forgetInstance(TemplateConfigurationProcessor::class);
$this->objectManager->forgetInstance(NodeTypeManager::class);
Expand All @@ -96,20 +98,20 @@ final protected function getObject(string $className): object
private function setupContentRepository(): void
{
// Create an environment to create nodes.
$this->objectManager->get(ContentDimensionRepository::class)->setDimensionsConfiguration([]);
$this->getObject(ContentDimensionRepository::class)->setDimensionsConfiguration([]);

$liveWorkspace = new Workspace('live');
$workspaceRepository = $this->objectManager->get(WorkspaceRepository::class);
$workspaceRepository = $this->getObject(WorkspaceRepository::class);
$workspaceRepository->add($liveWorkspace);

$testSite = new Site('test-site');
$testSite->setSiteResourcesPackageKey('Test.Site');
$siteRepository = $this->objectManager->get(SiteRepository::class);
$siteRepository = $this->getObject(SiteRepository::class);
$siteRepository->add($testSite);

$this->persistenceManager->persistAll();
$this->contextFactory = $this->objectManager->get(ContextFactoryInterface::class);
$subgraph = $this->contextFactory->create(['workspaceName' => 'live']);
$this->contextFactory = $this->getObject(ContextFactoryInterface::class);
$subgraph = $this->contextFactory->create(['workspaceName' => 'live', 'invisibleContentShown' => static::$showDisabledNodesInSubgraph]);

$rootNode = $subgraph->getRootNode();

Expand Down Expand Up @@ -153,7 +155,11 @@ protected function createNodeInto(NodeInterface $targetNode, string $nodeTypeNam
assert($changeCollection instanceof ChangeCollection);
$changeCollection->apply();

return $targetNode->getNode('new-node');
$newNode = $targetNode->getNode('new-node');
if (!$newNode) {
throw new \RuntimeException('New node not found at expected location. Try settting $showDisabledNodesInSubgraph.', 1719079419);
}
return $newNode;
}

protected function createFakeNode(string $nodeAggregateId): NodeInterface
Expand Down
Original file line number Diff line number Diff line change
@@ -1,14 +1,17 @@
{
"disabled": false,
"properties": [],
"childNodes": [
{
"type": null,
"name": "content",
"disabled": false,
"properties": [],
"childNodes": [
{
"type": "Flowpack.NodeTemplates:Content.Text",
"name": null,
"disabled": false,
"properties": {
"text": "Text"
},
Expand Down
Original file line number Diff line number Diff line change
@@ -1,14 +1,17 @@
{
"disabled": false,
"properties": [],
"childNodes": [
{
"type": null,
"name": "column0",
"disabled": false,
"properties": [],
"childNodes": [
{
"type": "Flowpack.NodeTemplates:Content.Text",
"name": null,
"disabled": false,
"properties": {
"text": "<p>foo<\/p>"
},
Expand All @@ -19,11 +22,13 @@
{
"type": null,
"name": "column1",
"disabled": false,
"properties": [],
"childNodes": [
{
"type": "Flowpack.NodeTemplates:Content.Text",
"name": null,
"disabled": false,
"properties": {
"text": "<p>bar<\/p>"
},
Expand Down
Original file line number Diff line number Diff line change
@@ -1,14 +1,17 @@
{
"disabled": false,
"properties": [],
"childNodes": [
{
"type": null,
"name": "content",
"disabled": false,
"properties": [],
"childNodes": [
{
"type": "Flowpack.NodeTemplates:Content.Text",
"name": null,
"disabled": false,
"properties": {
"text": "Text"
},
Expand All @@ -19,6 +22,7 @@
{
"type": "Flowpack.NodeTemplates:Document.Page",
"name": "illegal-node-1",
"disabled": false,
"properties": [],
"childNodes": []
}
Expand Down
27 changes: 27 additions & 0 deletions Tests/Functional/Features/Disabled/DisabledTest.php
Original file line number Diff line number Diff line change
@@ -0,0 +1,27 @@
<?php

declare(strict_types=1);

namespace Flowpack\NodeTemplates\Tests\Functional\Features\Disabled;

use Flowpack\NodeTemplates\Tests\Functional\AbstractNodeTemplateTestCase;

class DisabledTest extends AbstractNodeTemplateTestCase
{
protected static bool $showDisabledNodesInSubgraph = true;

/** @test */
public function itMatchesSnapshot1(): void
{
$createdNode = $this->createNodeInto(
$this->homePageMainContentCollectionNode,
'Flowpack.NodeTemplates:Content.Disabled',
[]
);

$this->assertLastCreatedTemplateMatchesSnapshot('Disabled');

$this->assertNoExceptionsWereCaught();
$this->assertNodeDumpAndTemplateDumpMatchSnapshot('Disabled', $createdNode);
}
}
7 changes: 7 additions & 0 deletions Tests/Functional/Features/Disabled/NodeTypes.Disabled.yaml
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@

'Flowpack.NodeTemplates:Content.Disabled':
superTypes:
'Neos.Neos:Content': true
options:
template:
disabled: true
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
{
"isDisabled": true
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
{
"disabled": true,
"properties": [],
"childNodes": []
}
3 changes: 3 additions & 0 deletions Tests/Functional/Features/Disabled/Snapshots/Disabled.yaml
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
'{nodeTypeName}':
options:
template: []
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
{
"disabled": false,
"properties": [],
"childNodes": []
}
Original file line number Diff line number Diff line change
Expand Up @@ -52,7 +52,7 @@
"severity": "ERROR"
},
{
"message": "Property \"_hidden\" in NodeType \"Flowpack.NodeTemplates:Content.SomeExceptions\" | PropertyIgnoredException(Because internal legacy property \"_hidden\" not implement., 1686149513158)",
"message": "Property \"_hidden\" in NodeType \"Flowpack.NodeTemplates:Content.SomeExceptions\" | PropertyIgnoredException(Using \"_hidden\" as property declaration was removed. Please use \"disabled\" on the first level instead., 1719079679)",
"severity": "ERROR"
},
{
Expand Down
Loading

0 comments on commit c682516

Please sign in to comment.