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.

Sign up for GitHub

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

Jump to bottom

FEATURE: Automatically set nodemove strategy in core #3876

Merged
merged 6 commits into from
Nov 27, 2024
+29 −6
Merged

FEATURE: Automatically set nodemove strategy in core #3876

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
97a0e98
Feture: Automatically set nodemove strategy in core
pKallert Oct 23, 2024
05f87b0
Feature: Call moveNodeStrategy in UI
pKallert Nov 15, 2024
fc2b89d
Merge branch '9.0' into feature/automatically-set-nodemove-strategy
pKallert Nov 15, 2024
f1af995
TASK: Reset `Configuration/Settings.yaml`
mhsdesign Nov 19, 2024
b0fbcbd
TASK: Move `moveNodeStrategy` to Neos UI
mhsdesign Nov 19, 2024
3c03e53
TASK: Improve errors for `moveNodeStrategy` and adjust to PSR12 code …
mhsdesign Nov 19, 2024
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
12 changes: 9 additions & 3 deletions Classes/Domain/Model/Changes/MoveAfter.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -71,14 +71,20 @@ public function apply(): void
$hasEqualParentNode = $parentNode->aggregateId
->equals($parentNodeOfPreviousSibling->aggregateId);


$contentRepository = $this->contentRepositoryRegistry->get($subject->contentRepositoryId);

$rawMoveNodeStrategy = $this->getNodeType($this->subject)?->getConfiguration('options.moveNodeStrategy');
Copy link
Member

Choose a reason for hiding this comment

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

this btw. should still be part of the schema @mhsdesign in Neos.Neos as otherwise you cannot validate it....

Copy link
Member

Choose a reason for hiding this comment

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

irigh zrk who actually uses the validator 😅 but if you think so ... are the validation things merged? then it can live here in the ui ... other wise id say no its not worth it. its not something anyone is gonna use.

Copy link
Member

Choose a reason for hiding this comment

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

😂 I think we cannot merge schemas? Lets skip it for now, but IMHO would be good to have somewhere...

if (!is_string($rawMoveNodeStrategy)) {
throw new \RuntimeException(sprintf('NodeType "%s" has an invalid configuration for option "moveNodeStrategy" expected string got %s', $this->subject->nodeTypeName->value, get_debug_type($rawMoveNodeStrategy)), 1732010016);
}
$moveNodeStrategy = RelationDistributionStrategy::tryFrom($rawMoveNodeStrategy);
if ($moveNodeStrategy === null) {
throw new \RuntimeException(sprintf('NodeType "%s" has an invalid configuration for option "moveNodeStrategy" got %s', $this->subject->nodeTypeName->value, $rawMoveNodeStrategy), 1732010011);
}
$command = MoveNodeAggregate::create(
$subject->workspaceName,
$subject->dimensionSpacePoint,
$subject->aggregateId,
RelationDistributionStrategy::STRATEGY_GATHER_ALL,
$moveNodeStrategy,
$hasEqualParentNode ? null : $parentNodeOfPreviousSibling->aggregateId,
$precedingSibling->aggregateId,
$succeedingSibling?->aggregateId,
Expand Down
11 changes: 9 additions & 2 deletions Classes/Domain/Model/Changes/MoveBefore.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -68,13 +68,20 @@ public function apply(): void
->equals($succeedingSiblingParent->aggregateId);

$contentRepository = $this->contentRepositoryRegistry->get($subject->contentRepositoryId);

$rawMoveNodeStrategy = $this->getNodeType($this->subject)?->getConfiguration('options.moveNodeStrategy');
if (!is_string($rawMoveNodeStrategy)) {
throw new \RuntimeException(sprintf('NodeType "%s" has an invalid configuration for option "moveNodeStrategy" expected string got %s', $this->subject->nodeTypeName->value, get_debug_type($rawMoveNodeStrategy)), 1732010016);
}
$moveNodeStrategy = RelationDistributionStrategy::tryFrom($rawMoveNodeStrategy);
if ($moveNodeStrategy === null) {
throw new \RuntimeException(sprintf('NodeType "%s" has an invalid configuration for option "moveNodeStrategy" got %s', $this->subject->nodeTypeName->value, $rawMoveNodeStrategy), 1732010011);
}
$contentRepository->handle(
MoveNodeAggregate::create(
$subject->workspaceName,
$subject->dimensionSpacePoint,
$subject->aggregateId,
RelationDistributionStrategy::STRATEGY_GATHER_ALL,
$moveNodeStrategy,
$hasEqualParentNode
? null
: $succeedingSiblingParent->aggregateId,
Expand Down
10 changes: 9 additions & 1 deletion Classes/Domain/Model/Changes/MoveInto.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -78,12 +78,20 @@ public function apply(): void
->equals($parentNode->aggregateId);

$contentRepository = $this->contentRepositoryRegistry->get($subject->contentRepositoryId);
$rawMoveNodeStrategy = $this->getNodeType($this->subject)?->getConfiguration('options.moveNodeStrategy');
if (!is_string($rawMoveNodeStrategy)) {
throw new \RuntimeException(sprintf('NodeType "%s" has an invalid configuration for option "moveNodeStrategy" expected string got %s', $this->subject->nodeTypeName->value, get_debug_type($rawMoveNodeStrategy)), 1732010016);
}
$moveNodeStrategy = RelationDistributionStrategy::tryFrom($rawMoveNodeStrategy);
if ($moveNodeStrategy === null) {
throw new \RuntimeException(sprintf('NodeType "%s" has an invalid configuration for option "moveNodeStrategy" got %s', $this->subject->nodeTypeName->value, $rawMoveNodeStrategy), 1732010011);
}
$contentRepository->handle(
MoveNodeAggregate::create(
$subject->workspaceName,
$subject->dimensionSpacePoint,
$subject->aggregateId,
RelationDistributionStrategy::STRATEGY_GATHER_ALL,
$moveNodeStrategy,
$hasEqualParentNode ? null : $parentNode->aggregateId,
)
);
Expand Down
2 changes: 2 additions & 0 deletions Configuration/NodeTypes.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@
factoryClassName: 'Neos\Neos\Ui\Infrastructure\Neos\UriPathSegmentNodeCreationHandlerFactory'
promotedElements:
factoryClassName: 'Neos\Neos\Ui\Infrastructure\ContentRepository\CreationDialog\PromotedElementsCreationHandlerFactory'
moveNodeStrategy: gatherAll
Copy link
Member

Choose a reason for hiding this comment

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

We also discussed that it might make sense to use gatherSpecializations or scatter on document level from time to time (to create different structures for each dimension). Maybe we need to introduce some ui dialog with a little "magnet" slider which dimensions to also gather

But we should also keep in mind that non gatherAll behaviour might be broken for the Neos Ui as it relies on the information here: https://github.com/neos/neos-development-collection/blob/a14bbe701335c013df85579d1703f9dd4b018cc5/Neos.Neos/Classes/Controller/Service/NodesController.php#L334-L335


'Neos.Neos:Content':
postprocessors:
Expand All @@ -34,6 +35,7 @@
nodeCreationHandlers:
promotedElements:
factoryClassName: 'Neos\Neos\Ui\Infrastructure\ContentRepository\CreationDialog\PromotedElementsCreationHandlerFactory'
moveNodeStrategy: scatter
Copy link
Member

Choose a reason for hiding this comment

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

as discussed today for content, especially as scatter also moves virtual variants the behaviour is correct

Still, specializations pointing to the same node using the fallback mechanism will be kept gathered.


'Neos.Neos:ContentCollection':
ui:
Expand Down
Loading