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

TASK: Disallow disabling tethered nodes #4938

Draft
wants to merge 1 commit into
base: 9.0
Choose a base branch
from

Conversation

nezaniel
Copy link
Member

@nezaniel nezaniel commented Mar 13, 2024

Resolves: #4821

@github-actions github-actions bot added the 9.0 label Mar 13, 2024
@mhsdesign
Copy link
Member

❤️

@mhsdesign mhsdesign marked this pull request as draft March 13, 2024 09:03
@mhsdesign
Copy link
Member

The branch is a bit broken now, i presume as it was dependant on the workspacePr it got rebased or sth?

@mhsdesign
Copy link
Member

i tried to cherry pick the changes f3614a755512c22de3c9085afda31db9ef098167 (original commit in details)

Details

From f3614a755512c22de3c9085afda31db9ef098167 Mon Sep 17 00:00:00 2001
From: Bernhard Schmitt <schmitt@sitegeist.de>
Date: Wed, 13 Mar 2024 09:43:13 +0100
Subject: [PATCH] 4821 - Disallow disabling tethered nodes

---
 ...ableNodeAggregate_ConstraintChecks.feature | 22 ++++++++++++++-----
 .../Feature/NodeDisabling/NodeDisabling.php   |  1 +
 2 files changed, 17 insertions(+), 6 deletions(-)

diff --git a/Neos.ContentRepository.BehavioralTests/Tests/Behavior/Features/06-NodeDisabling/01-DisableNodeAggregate_ConstraintChecks.feature b/Neos.ContentRepository.BehavioralTests/Tests/Behavior/Features/06-NodeDisabling/01-DisableNodeAggregate_ConstraintChecks.feature
index d41691053f..91e3033a1b 100644
--- a/Neos.ContentRepository.BehavioralTests/Tests/Behavior/Features/06-NodeDisabling/01-DisableNodeAggregate_ConstraintChecks.feature
+++ b/Neos.ContentRepository.BehavioralTests/Tests/Behavior/Features/06-NodeDisabling/01-DisableNodeAggregate_ConstraintChecks.feature
@@ -11,7 +11,11 @@ Feature: Constraint checks on node aggregate disabling
       | language   | de, gsw, en | gsw->de, en     |
     And using the following node types:
     """yaml
-    'Neos.ContentRepository.Testing:Document': []
+    'Neos.ContentRepository.Testing:Tethered': []
+    'Neos.ContentRepository.Testing:Document':
+      childNodes:
+        tethered:
+          type: 'Neos.ContentRepository.Testing:Tethered'
     """
     And using identifier "default", I define a content repository
     And I am in content repository "default"
@@ -30,8 +34,8 @@ Feature: Constraint checks on node aggregate disabling
       | nodeTypeName            | "Neos.ContentRepository:Root" |
     And the graph projection is fully up to date
     And the following CreateNodeAggregateWithNode commands are executed:
-      | nodeAggregateId | nodeTypeName                            | parentNodeAggregateId | nodeName |
-      | sir-david-nodenborough  | Neos.ContentRepository.Testing:Document | lady-eleonode-rootford        | document |
+      | nodeAggregateId | nodeTypeName                            | parentNodeAggregateId | nodeName | tetheredDescendantNodeAggregateIds |
+      | sir-david-nodenborough  | Neos.ContentRepository.Testing:Document | lady-eleonode-rootford        | document | {"tethered": "nodewyn-tetherton"}                   |
 
   Scenario: Try to disable a node aggregate in a non-existing content stream
     When the command DisableNodeAggregate is executed with payload and exceptions are caught:
@@ -58,6 +62,13 @@ Feature: Constraint checks on node aggregate disabling
       | nodeVariantSelectionStrategy | "allVariants"                          |
     Then the last command should have thrown an exception of type "NodeAggregateCurrentlyDoesNotExist"
 
+  Scenario: Try to disable a tethered node aggregate
+    When the command DisableNodeAggregate is executed with payload and exceptions are caught:
+      | Key                          | Value                                  |
+      | nodeAggregateId              | "nodewyn-tetherton"                    |
+      | nodeVariantSelectionStrategy | "allVariants"                          |
+    Then the last command should have thrown an exception of type "NodeAggregateIsTethered"
+
   Scenario: Try to disable an already disabled node aggregate
     Given the command DisableNodeAggregate is executed with payload:
       | Key                          | Value                                  |
@@ -72,14 +83,13 @@ Feature: Constraint checks on node aggregate disabling
       | nodeAggregateId      | "sir-david-nodenborough"               |
       | coveredDimensionSpacePoint   | {"language": "de"}                                     |
       | nodeVariantSelectionStrategy | "allVariants"                          |
-    Then I expect exactly 4 events to be published on stream with prefix "ContentStream:cs-identifier"
-    And event at index 3 is of type "NodeAggregateWasDisabled" with payload:
+    Then I expect exactly 5 events to be published on stream with prefix "ContentStream:cs-identifier"
+    And event at index 4 is of type "NodeAggregateWasDisabled" with payload:
       | Key                          | Expected                               |
       | contentStreamId              | "cs-identifier"                        |
       | nodeAggregateId              | "sir-david-nodenborough"               |
       | affectedDimensionSpacePoints | [{"language":"de"},{"language":"gsw"}] |
 
-
   Scenario: Try to disable a node aggregate in a non-existing dimension space point
     When the command DisableNodeAggregate is executed with payload and exceptions are caught:
       | Key                          | Value                                  |
diff --git a/Neos.ContentRepository.Core/Classes/Feature/NodeDisabling/NodeDisabling.php b/Neos.ContentRepository.Core/Classes/Feature/NodeDisabling/NodeDisabling.php
index 84bcfccd4e..9ae2372b96 100644
--- a/Neos.ContentRepository.Core/Classes/Feature/NodeDisabling/NodeDisabling.php
+++ b/Neos.ContentRepository.Core/Classes/Feature/NodeDisabling/NodeDisabling.php
@@ -60,6 +60,7 @@ private function handleDisableNodeAggregate(
             $nodeAggregate,
             $command->coveredDimensionSpacePoint
         );
+        $this->requireNodeAggregateToBeUntethered($nodeAggregate);
 
         if ($nodeAggregate->disablesDimensionSpacePoint($command->coveredDimensionSpacePoint)) {
             // already disabled, so we can return a no-operation.

but it seems to need further adjustments and conception after the subtree tags

@mhsdesign mhsdesign force-pushed the 4821-disallowDisablingTetheredNodes branch from 338369c to f8d0e08 Compare March 17, 2024 20:11
@mhsdesign mhsdesign changed the title 4821 disallow disabling tethered nodes TASK: Disallow disabling tethered nodes Apr 25, 2024
@mhsdesign
Copy link
Member

How do we want to continue here?

@github-actions github-actions bot added the Task label Apr 25, 2024
@nezaniel
Copy link
Member Author

I'd say we allow general tagging but not disabling on tethered nodes for now

@mhsdesign
Copy link
Member

Today we discussed we disallow TagSubtree(disable) DisableNodeAggregate for tethered nodes.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Disallow disabling tethered nodes
2 participants