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

fix copy paste across dimensions #3873

Open
wants to merge 3 commits into
base: 9.0
Choose a base branch
from
Open
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
4 changes: 3 additions & 1 deletion Classes/Domain/Model/Changes/CopyAfter.php
Original file line number Diff line number Diff line change
Expand Up @@ -70,7 +70,9 @@ public function apply(): void
),
$subject->workspaceName,
$subject,
OriginDimensionSpacePoint::fromDimensionSpacePoint($subject->dimensionSpacePoint),
// 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($previousSibling->dimensionSpacePoint),
$parentNodeOfPreviousSibling->aggregateId,
$succeedingSibling?->aggregateId
);
Expand Down
4 changes: 3 additions & 1 deletion Classes/Domain/Model/Changes/CopyBefore.php
Original file line number Diff line number Diff line change
Expand Up @@ -65,7 +65,9 @@ public function apply(): void
),
$subject->workspaceName,
$subject,
OriginDimensionSpacePoint::fromDimensionSpacePoint($subject->dimensionSpacePoint),
// 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),
$parentNodeOfSucceedingSibling->aggregateId,
$succeedingSibling->aggregateId
);
Expand Down
4 changes: 3 additions & 1 deletion Classes/Domain/Model/Changes/CopyInto.php
Original file line number Diff line number Diff line change
Expand Up @@ -74,7 +74,9 @@ public function apply(): void
),
$subject->workspaceName,
$subject,
OriginDimensionSpacePoint::fromDimensionSpacePoint($subject->dimensionSpacePoint),
// 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
);
Expand Down
58 changes: 46 additions & 12 deletions Classes/Domain/Model/Changes/MoveAfter.php
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);
}
Comment on lines +80 to +120
Copy link
Member

Choose a reason for hiding this comment

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

im not sure i understand the full implications of this ... as far as i know is move node super well tested and supports a variety of cases to distribute node aggregates and i dont know if we want to trade that in for the use of this CopyNodesRecursively plus removing the original node?

isnt this supported already by MoveNodeAggregate or do we need to extend the core?


$updateParentNodeInfo = new UpdateNodeInfo();
$updateParentNodeInfo->setNode($parentNodeOfPreviousSibling);
Expand Down
58 changes: 46 additions & 12 deletions Classes/Domain/Model/Changes/MoveBefore.php
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
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
3 changes: 3 additions & 0 deletions Tests/IntegrationTests/docker-compose.neos-dev-instance.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -14,12 +14,15 @@ services:
# Enable GD
PHP_EXTENSION_GD: 1
COMPOSER_CACHE_DIR: /home/circleci/.composer/cache
DB_HOST: db

db:
image: mariadb:10.11
environment:
MYSQL_DATABASE: neos
MYSQL_ROOT_PASSWORD: not_a_real_password
ports:
- 13309:3306
command: ['mysqld', '--character-set-server=utf8mb4', '--collation-server=utf8mb4_unicode_ci']
volumes:
composer_cache:
2 changes: 1 addition & 1 deletion Tests/IntegrationTests/pageModel.js
Original file line number Diff line number Diff line change
Expand Up @@ -59,7 +59,7 @@ export class PublishDropDown {

static publishDropdownDiscardAll = ReactSelector('PublishDropDown ContextDropDownContents').find('button').withText('Discard all');

static publishDropdownPublishAll = ReactSelector('PublishDropDown ShallowDropDownContents').find('button').withText('Publish all');
static publishDropdownPublishAll = ReactSelector('PublishDropDown ContextDropDownContents').find('button').withText('Publish all');

static async discardAll() {
const $discardAllBtn = Selector(this.publishDropdownDiscardAll);
Expand Down
9 changes: 7 additions & 2 deletions Tests/IntegrationTests/start-neos-dev-instance.sh
Original file line number Diff line number Diff line change
Expand Up @@ -41,6 +41,7 @@ dc exec -T php bash <<-'BASH'
./flow flow:cache:warmup
./flow doctrine:migrate
./flow user:create --username=admin --password=admin --first-name=John --last-name=Doe --roles=Administrator || true
./flow user:create --username=editor --password=editor --first-name=Some --last-name=FooBarEditor --roles=Editor || true
BASH

echo ""
Expand Down Expand Up @@ -68,7 +69,11 @@ dc exec -T php bash <<-BASH
if ./flow site:list | grep -q 'Node name'; then
./flow site:prune '*'
fi
./flow site:import --package-key=Neos.TestSite
./flow cr:setup
./flow cr:setup --content-repository onedimension
./flow cr:setup --content-repository twodimensions
./flow cr:import --content-repository onedimension Packages/Sites/Neos.Test.OneDimension/Resources/Private/Content
./flow site:create neos-test-onedimension Neos.Test.OneDimension Neos.TestNodeTypes:Document.HomePage
./flow resource:publish
BASH

Expand All @@ -85,5 +90,5 @@ dc exec -T php bash <<-'BASH'

# enable changes of the Neos.TestNodeTypes outside of the container to appear in the container via sym link to mounted volume
rm -rf /usr/src/app/TestDistribution/Packages/Application/Neos.TestNodeTypes
ln -s /usr/src/neos-ui/Tests/IntegrationTests/SharedNodeTypesPackage/ /usr/src/app/TestDistribution/Packages/Application/Neos.TestNodeTypes
ln -s /usr/src/neos-ui/Tests/IntegrationTests/TestDistribution/DistributionPackages/Neos.TestNodeTypes /usr/src/app/TestDistribution/Packages/Application/Neos.TestNodeTypes
BASH
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@ const searchOptions = (searchTerm, processedSelectBoxOptions) =>
}))
export default class DimensionSelector extends PureComponent {
static propTypes = {
icon: PropTypes.string.isRequired,
icon: PropTypes.string,
dimensionLabel: PropTypes.string.isRequired,
presets: PropTypes.object.isRequired,
activePreset: PropTypes.string.isRequired,
Expand Down
Loading