Skip to content

Commit

Permalink
BUGFIX: Cut/Paste across dimensions should not lead to error
Browse files Browse the repository at this point in the history 
After thoroughly thinking this through, I decided
that moving nodes across dimensions is effectively a copy followed by
a delete, thus it CHANGES IDENTITIES of nodes then.

This means it will work in all cases, even if the moved node already
existed with the same ID in the target dimension.

Lateron, we additionally need to expose CreateNodeVariant (which creates
connected variants) in the UI as well.

Resolves: #4614
  • Loading branch information
@skurfuerst
skurfuerst committed Oct 21, 2024
1 parent 4b6366d commit 33df996
Show file tree
Hide file tree
Showing 3 changed files with 134 additions and 31 deletions.
58 changes: 46 additions & 12 deletions Classes/Domain/Model/Changes/MoveAfter.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -13,9 +13,13 @@
*/

use InvalidArgumentException;
use Neos\ContentRepository\Core\DimensionSpace\OriginDimensionSpacePoint;
use Neos\ContentRepository\Core\Feature\NodeDuplication\Command\CopyNodesRecursively;
use Neos\ContentRepository\Core\Feature\NodeMove\Command\MoveNodeAggregate;
use Neos\ContentRepository\Core\Feature\NodeMove\Dto\RelationDistributionStrategy;
use Neos\Neos\Ui\Domain\Model\Feedback\Operations\RemoveNode;
use Neos\ContentRepository\Core\Feature\NodeRemoval\Command\RemoveNodeAggregate;
use Neos\ContentRepository\Core\Projection\ContentGraph\VisibilityConstraints;
use Neos\ContentRepository\Core\SharedModel\Node\NodeVariantSelectionStrategy;
use Neos\Neos\Ui\Domain\Model\Feedback\Operations\UpdateNodeInfo;

/**
Expand Down Expand Up @@ -71,19 +75,49 @@ public function apply(): void
$hasEqualParentNode = $parentNode->aggregateId
->equals($parentNodeOfPreviousSibling->aggregateId);


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

$command = MoveNodeAggregate::create(
$subject->workspaceName,
$subject->dimensionSpacePoint,
$subject->aggregateId,
RelationDistributionStrategy::STRATEGY_GATHER_ALL,
$hasEqualParentNode ? null : $parentNodeOfPreviousSibling->aggregateId,
$precedingSibling->aggregateId,
$succeedingSibling?->aggregateId,
);
$contentRepository->handle($command);
if (!$precedingSibling->dimensionSpacePoint->equals($subject->dimensionSpacePoint)) {
// WORKAROUND for MOVE ACROSS DIMENSIONS:
// - we want it to work like a copy/paste, followed by an original delete.
// - This is to ensure the user can use it as expected from text editors, where context
// is not preserved during cut/paste.
// - LATERON, we need to expose CreateNodeVariant (which creates connected variants) in the UI as well.
$command = CopyNodesRecursively::createFromSubgraphAndStartNode(
$contentRepository->getContentGraph($subject->workspaceName)->getSubgraph(
$subject->dimensionSpacePoint,
VisibilityConstraints::withoutRestrictions()
),
$subject->workspaceName,
$subject,
// NOTE: in order to be able to copy/paste across dimensions, we need to use
// the TARGET NODE's DimensionSpacePoint to create the node in the target dimension.
OriginDimensionSpacePoint::fromDimensionSpacePoint($precedingSibling->dimensionSpacePoint),
$parentNodeOfPreviousSibling->aggregateId,
$succeedingSibling?->aggregateId
);

$contentRepository->handle($command);

$command = RemoveNodeAggregate::create(
$subject->workspaceName,
$subject->aggregateId,
$subject->dimensionSpacePoint,
NodeVariantSelectionStrategy::STRATEGY_ALL_SPECIALIZATIONS,
);
$contentRepository->handle($command);
} else {
$command = MoveNodeAggregate::create(
$subject->workspaceName,
$subject->dimensionSpacePoint,
$subject->aggregateId,
RelationDistributionStrategy::STRATEGY_GATHER_ALL,
$hasEqualParentNode ? null : $parentNodeOfPreviousSibling->aggregateId,
$precedingSibling->aggregateId,
$succeedingSibling?->aggregateId,
);
$contentRepository->handle($command);
}

$updateParentNodeInfo = new UpdateNodeInfo();
$updateParentNodeInfo->setNode($parentNodeOfPreviousSibling);
Expand Down
58 changes: 46 additions & 12 deletions Classes/Domain/Model/Changes/MoveBefore.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -12,9 +12,13 @@
* source code.
*/

use Neos\ContentRepository\Core\DimensionSpace\OriginDimensionSpacePoint;
use Neos\ContentRepository\Core\Feature\NodeDuplication\Command\CopyNodesRecursively;
use Neos\ContentRepository\Core\Feature\NodeMove\Command\MoveNodeAggregate;
use Neos\ContentRepository\Core\Feature\NodeMove\Dto\RelationDistributionStrategy;
use Neos\Neos\Ui\Domain\Model\Feedback\Operations\RemoveNode;
use Neos\ContentRepository\Core\Feature\NodeRemoval\Command\RemoveNodeAggregate;
use Neos\ContentRepository\Core\Projection\ContentGraph\VisibilityConstraints;
use Neos\ContentRepository\Core\SharedModel\Node\NodeVariantSelectionStrategy;
use Neos\Neos\Ui\Domain\Model\Feedback\Operations\UpdateNodeInfo;

/**
Expand Down Expand Up @@ -69,19 +73,49 @@ public function apply(): void

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

$contentRepository->handle(
MoveNodeAggregate::create(
if (!$succeedingSibling->dimensionSpacePoint->equals($subject->dimensionSpacePoint)) {
// WORKAROUND for MOVE ACROSS DIMENSIONS:
// - we want it to work like a copy/paste, followed by an original delete.
// - This is to ensure the user can use it as expected from text editors, where context
// is not preserved during cut/paste.
// - LATERON, we need to expose CreateNodeVariant (which creates connected variants) in the UI as well.
$command = CopyNodesRecursively::createFromSubgraphAndStartNode(
$contentRepository->getContentGraph($subject->workspaceName)->getSubgraph(
$subject->dimensionSpacePoint,
VisibilityConstraints::withoutRestrictions()
),
$subject->workspaceName,
$subject,
// NOTE: in order to be able to copy/paste across dimensions, we need to use
// the TARGET NODE's DimensionSpacePoint to create the node in the target dimension.
OriginDimensionSpacePoint::fromDimensionSpacePoint($succeedingSibling->dimensionSpacePoint),
$succeedingSiblingParent->aggregateId,
$succeedingSibling->aggregateId
);
$contentRepository->handle($command);

$command = RemoveNodeAggregate::create(
$subject->workspaceName,
$subject->dimensionSpacePoint,
$subject->aggregateId,
RelationDistributionStrategy::STRATEGY_GATHER_ALL,
$hasEqualParentNode
? null
: $succeedingSiblingParent->aggregateId,
$precedingSibling?->aggregateId,
$succeedingSibling->aggregateId,
)
);
$subject->dimensionSpacePoint,
NodeVariantSelectionStrategy::STRATEGY_ALL_SPECIALIZATIONS,
);
$contentRepository->handle($command);
} else {
$contentRepository->handle(
MoveNodeAggregate::create(
$subject->workspaceName,
$subject->dimensionSpacePoint,
$subject->aggregateId,
RelationDistributionStrategy::STRATEGY_GATHER_ALL,
$hasEqualParentNode
? null
: $succeedingSiblingParent->aggregateId,
$precedingSibling?->aggregateId,
$succeedingSibling->aggregateId,
)
);
}

$updateParentNodeInfo = new UpdateNodeInfo();
$updateParentNodeInfo->setNode($succeedingSiblingParent);
Expand Down
49 changes: 42 additions & 7 deletions Classes/Domain/Model/Changes/MoveInto.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -12,9 +12,14 @@
* source code.
*/

use Neos\ContentRepository\Core\DimensionSpace\OriginDimensionSpacePoint;
use Neos\ContentRepository\Core\Feature\NodeDuplication\Command\CopyNodesRecursively;
use Neos\ContentRepository\Core\Feature\NodeMove\Command\MoveNodeAggregate;
use Neos\ContentRepository\Core\Feature\NodeMove\Dto\RelationDistributionStrategy;
use Neos\ContentRepository\Core\Feature\NodeRemoval\Command\RemoveNodeAggregate;
use Neos\ContentRepository\Core\Projection\ContentGraph\Node;
use Neos\ContentRepository\Core\Projection\ContentGraph\VisibilityConstraints;
use Neos\ContentRepository\Core\SharedModel\Node\NodeVariantSelectionStrategy;
use Neos\Neos\Ui\Domain\Model\Feedback\Operations\UpdateNodeInfo;

/**
Expand Down Expand Up @@ -78,15 +83,45 @@ public function apply(): void
->equals($parentNode->aggregateId);

$contentRepository = $this->contentRepositoryRegistry->get($subject->contentRepositoryId);
$contentRepository->handle(
MoveNodeAggregate::create(
if (!$parentNode->dimensionSpacePoint->equals($subject->dimensionSpacePoint)) {
// WORKAROUND for MOVE ACROSS DIMENSIONS:
// - we want it to work like a copy/paste, followed by an original delete.
// - This is to ensure the user can use it as expected from text editors, where context
// is not preserved during cut/paste.
// - LATERON, we need to expose CreateNodeVariant (which creates connected variants) in the UI as well.
$command = CopyNodesRecursively::createFromSubgraphAndStartNode(
$contentRepository->getContentGraph($subject->workspaceName)->getSubgraph(
$subject->dimensionSpacePoint,
VisibilityConstraints::withoutRestrictions()
),
$subject->workspaceName,
$subject,
// NOTE: in order to be able to copy/paste across dimensions, we need to use
// the TARGET NODE's DimensionSpacePoint to create the node in the target dimension.
OriginDimensionSpacePoint::fromDimensionSpacePoint($parentNode->dimensionSpacePoint),
$parentNode->aggregateId,
null
);
$contentRepository->handle($command);

$command = RemoveNodeAggregate::create(
$subject->workspaceName,
$subject->dimensionSpacePoint,
$subject->aggregateId,
RelationDistributionStrategy::STRATEGY_GATHER_ALL,
$hasEqualParentNode ? null : $parentNode->aggregateId,
)
);
$subject->dimensionSpacePoint,
NodeVariantSelectionStrategy::STRATEGY_ALL_SPECIALIZATIONS,
);
$contentRepository->handle($command);
} else {
$contentRepository->handle(
MoveNodeAggregate::create(
$subject->workspaceName,
$subject->dimensionSpacePoint,
$subject->aggregateId,
RelationDistributionStrategy::STRATEGY_GATHER_ALL,
$hasEqualParentNode ? null : $parentNode->aggregateId,
)
);
}

$updateParentNodeInfo = new UpdateNodeInfo();
$updateParentNodeInfo->setNode($parentNode);
Expand Down

0 comments on commit 33df996

Please sign in to comment.