From aaeb68a85a88d3eaf93acec08d3bfa839c9028a8 Mon Sep 17 00:00:00 2001 From: Wilhelm Behncke Date: Mon, 22 Apr 2024 12:04:30 +0200 Subject: [PATCH 01/36] TASK: Move `Conflicts` concept to `Application\Shared` namespace This moves the classes `ConflictsOccurred`, `Conflict`, `Conflicts`, `ConflictsBuilder`, `ReasonForConflict`, `TypeOfChange` and `IconLabel` into the `Application\Shared` namespace and adjusts all references accordingly. These classes are going to be reused by multiple command handlers. --- Classes/Application/{SyncWorkspace => Shared}/Conflict.php | 2 +- Classes/Application/{SyncWorkspace => Shared}/Conflicts.php | 2 +- .../{SyncWorkspace => Shared}/ConflictsBuilder.php | 2 +- .../{SyncWorkspace => Shared}/ConflictsOccurred.php | 2 +- Classes/Application/{SyncWorkspace => Shared}/IconLabel.php | 2 +- .../{SyncWorkspace => Shared}/ReasonForConflict.php | 2 +- .../Application/{SyncWorkspace => Shared}/TypeOfChange.php | 2 +- .../SyncWorkspace/SyncWorkspaceCommandHandler.php | 5 +++++ Classes/Controller/BackendServiceController.php | 2 +- 9 files changed, 13 insertions(+), 8 deletions(-) rename Classes/Application/{SyncWorkspace => Shared}/Conflict.php (94%) rename Classes/Application/{SyncWorkspace => Shared}/Conflicts.php (97%) rename Classes/Application/{SyncWorkspace => Shared}/ConflictsBuilder.php (99%) rename Classes/Application/{SyncWorkspace => Shared}/ConflictsOccurred.php (93%) rename Classes/Application/{SyncWorkspace => Shared}/IconLabel.php (93%) rename Classes/Application/{SyncWorkspace => Shared}/ReasonForConflict.php (91%) rename Classes/Application/{SyncWorkspace => Shared}/TypeOfChange.php (93%) diff --git a/Classes/Application/SyncWorkspace/Conflict.php b/Classes/Application/Shared/Conflict.php similarity index 94% rename from Classes/Application/SyncWorkspace/Conflict.php rename to Classes/Application/Shared/Conflict.php index 5f96024af0..3f77f0ebeb 100644 --- a/Classes/Application/SyncWorkspace/Conflict.php +++ b/Classes/Application/Shared/Conflict.php @@ -12,7 +12,7 @@ declare(strict_types=1); -namespace Neos\Neos\Ui\Application\SyncWorkspace; +namespace Neos\Neos\Ui\Application\Shared; use Neos\Flow\Annotations as Flow; diff --git a/Classes/Application/SyncWorkspace/Conflicts.php b/Classes/Application/Shared/Conflicts.php similarity index 97% rename from Classes/Application/SyncWorkspace/Conflicts.php rename to Classes/Application/Shared/Conflicts.php index e53c5ef9e3..c1a292ffd5 100644 --- a/Classes/Application/SyncWorkspace/Conflicts.php +++ b/Classes/Application/Shared/Conflicts.php @@ -12,7 +12,7 @@ declare(strict_types=1); -namespace Neos\Neos\Ui\Application\SyncWorkspace; +namespace Neos\Neos\Ui\Application\Shared; use Neos\ContentRepository\Core\ContentRepository; use Neos\ContentRepository\Core\DimensionSpace\DimensionSpacePoint; diff --git a/Classes/Application/SyncWorkspace/ConflictsBuilder.php b/Classes/Application/Shared/ConflictsBuilder.php similarity index 99% rename from Classes/Application/SyncWorkspace/ConflictsBuilder.php rename to Classes/Application/Shared/ConflictsBuilder.php index 138ed415fc..a30267f3ca 100644 --- a/Classes/Application/SyncWorkspace/ConflictsBuilder.php +++ b/Classes/Application/Shared/ConflictsBuilder.php @@ -12,7 +12,7 @@ declare(strict_types=1); -namespace Neos\Neos\Ui\Application\SyncWorkspace; +namespace Neos\Neos\Ui\Application\Shared; use Neos\ContentRepository\Core\CommandHandler\CommandInterface; use Neos\ContentRepository\Core\ContentRepository; diff --git a/Classes/Application/SyncWorkspace/ConflictsOccurred.php b/Classes/Application/Shared/ConflictsOccurred.php similarity index 93% rename from Classes/Application/SyncWorkspace/ConflictsOccurred.php rename to Classes/Application/Shared/ConflictsOccurred.php index e62b9f9029..6917e42a9d 100644 --- a/Classes/Application/SyncWorkspace/ConflictsOccurred.php +++ b/Classes/Application/Shared/ConflictsOccurred.php @@ -12,7 +12,7 @@ declare(strict_types=1); -namespace Neos\Neos\Ui\Application\SyncWorkspace; +namespace Neos\Neos\Ui\Application\Shared; /** * @internal for communication within the Neos UI only diff --git a/Classes/Application/SyncWorkspace/IconLabel.php b/Classes/Application/Shared/IconLabel.php similarity index 93% rename from Classes/Application/SyncWorkspace/IconLabel.php rename to Classes/Application/Shared/IconLabel.php index 2e085ced07..7617421b4b 100644 --- a/Classes/Application/SyncWorkspace/IconLabel.php +++ b/Classes/Application/Shared/IconLabel.php @@ -12,7 +12,7 @@ declare(strict_types=1); -namespace Neos\Neos\Ui\Application\SyncWorkspace; +namespace Neos\Neos\Ui\Application\Shared; use Neos\Flow\Annotations as Flow; diff --git a/Classes/Application/SyncWorkspace/ReasonForConflict.php b/Classes/Application/Shared/ReasonForConflict.php similarity index 91% rename from Classes/Application/SyncWorkspace/ReasonForConflict.php rename to Classes/Application/Shared/ReasonForConflict.php index ccaf361dde..3979852e28 100644 --- a/Classes/Application/SyncWorkspace/ReasonForConflict.php +++ b/Classes/Application/Shared/ReasonForConflict.php @@ -12,7 +12,7 @@ declare(strict_types=1); -namespace Neos\Neos\Ui\Application\SyncWorkspace; +namespace Neos\Neos\Ui\Application\Shared; /** * @internal for communication within the Neos UI only diff --git a/Classes/Application/SyncWorkspace/TypeOfChange.php b/Classes/Application/Shared/TypeOfChange.php similarity index 93% rename from Classes/Application/SyncWorkspace/TypeOfChange.php rename to Classes/Application/Shared/TypeOfChange.php index 379ba5f119..cd474d30a5 100644 --- a/Classes/Application/SyncWorkspace/TypeOfChange.php +++ b/Classes/Application/Shared/TypeOfChange.php @@ -12,7 +12,7 @@ declare(strict_types=1); -namespace Neos\Neos\Ui\Application\SyncWorkspace; +namespace Neos\Neos\Ui\Application\Shared; /** * @internal for communication within the Neos UI only diff --git a/Classes/Application/SyncWorkspace/SyncWorkspaceCommandHandler.php b/Classes/Application/SyncWorkspace/SyncWorkspaceCommandHandler.php index 834837747e..2006bb0117 100644 --- a/Classes/Application/SyncWorkspace/SyncWorkspaceCommandHandler.php +++ b/Classes/Application/SyncWorkspace/SyncWorkspaceCommandHandler.php @@ -19,6 +19,8 @@ use Neos\Flow\Annotations as Flow; use Neos\Neos\Domain\NodeLabel\NodeLabelGeneratorInterface; use Neos\Neos\Domain\Workspace\WorkspaceProvider; +use Neos\Neos\Ui\Application\Shared\Conflicts; +use Neos\Neos\Ui\Application\Shared\ConflictsOccurred; /** * The application layer level command handler to for rebasing the workspace @@ -37,6 +39,9 @@ final class SyncWorkspaceCommandHandler #[Flow\Inject] protected NodeLabelGeneratorInterface $nodeLabelGenerator; + /** + * @throws ConflictsOccurred + */ public function handle(SyncWorkspaceCommand $command): void { try { diff --git a/Classes/Controller/BackendServiceController.php b/Classes/Controller/BackendServiceController.php index 4454e852eb..fdb6b56c46 100644 --- a/Classes/Controller/BackendServiceController.php +++ b/Classes/Controller/BackendServiceController.php @@ -46,7 +46,7 @@ use Neos\Neos\Ui\Application\PublishChangesInSite; use Neos\Neos\Ui\Application\ReloadNodes\ReloadNodesQuery; use Neos\Neos\Ui\Application\ReloadNodes\ReloadNodesQueryHandler; -use Neos\Neos\Ui\Application\SyncWorkspace\ConflictsOccurred; +use Neos\Neos\Ui\Application\Shared\ConflictsOccurred; use Neos\Neos\Ui\Application\SyncWorkspace\SyncWorkspaceCommand; use Neos\Neos\Ui\Application\SyncWorkspace\SyncWorkspaceCommandHandler; use Neos\Neos\Ui\ContentRepository\Service\NeosUiNodeService; From 5ff99a84306a89fb6d948f33500d4899965b1503 Mon Sep 17 00:00:00 2001 From: Wilhelm Behncke Date: Mon, 22 Apr 2024 13:01:36 +0200 Subject: [PATCH 02/36] TASK: Add command handlers for all Publish-related commands This includes the following tasks; - Move `PublishChangesInDocument` -> `PublishChangesInDocument\PublishChangesInDocumentCommand` - Create `PublishChangesInDocumentCommandHandler` - Move `PublishChangesInSite` -> `PublishChangesInSite\PublishChangesInSiteCommand` - Create `PublishChangesInSiteCommandHandler` --- .../PublishChangesInDocumentCommand.php} | 7 +- ...PublishChangesInDocumentCommandHandler.php | 68 ++++++++++++++++++ .../PublishChangesInSiteCommand.php} | 7 +- .../PublishChangesInSiteCommandHandler.php | 46 ++++++++++++ .../Application/Shared/PublishSucceeded.php | 37 ++++++++++ .../Controller/BackendServiceController.php | 70 +++++++------------ 6 files changed, 184 insertions(+), 51 deletions(-) rename Classes/Application/{PublishChangesInDocument.php => PublishChangesInDocument/PublishChangesInDocumentCommand.php} (88%) create mode 100644 Classes/Application/PublishChangesInDocument/PublishChangesInDocumentCommandHandler.php rename Classes/Application/{PublishChangesInSite.php => PublishChangesInSite/PublishChangesInSiteCommand.php} (89%) create mode 100644 Classes/Application/PublishChangesInSite/PublishChangesInSiteCommandHandler.php create mode 100644 Classes/Application/Shared/PublishSucceeded.php diff --git a/Classes/Application/PublishChangesInDocument.php b/Classes/Application/PublishChangesInDocument/PublishChangesInDocumentCommand.php similarity index 88% rename from Classes/Application/PublishChangesInDocument.php rename to Classes/Application/PublishChangesInDocument/PublishChangesInDocumentCommand.php index 85069b6606..87d9762511 100644 --- a/Classes/Application/PublishChangesInDocument.php +++ b/Classes/Application/PublishChangesInDocument/PublishChangesInDocumentCommand.php @@ -12,7 +12,7 @@ declare(strict_types=1); -namespace Neos\Neos\Ui\Application; +namespace Neos\Neos\Ui\Application\PublishChangesInDocument; use Neos\ContentRepository\Core\SharedModel\ContentRepository\ContentRepositoryId; use Neos\ContentRepository\Core\SharedModel\Node\NodeAggregateId; @@ -20,12 +20,13 @@ use Neos\Flow\Annotations as Flow; /** - * The application layer level command DTO to communicate publication of all changes recorded for a given document + * The application layer level command DTO to communicate publication of + * all changes recorded for a given document * * @internal for communication within the Neos UI only */ #[Flow\Proxy(false)] -final readonly class PublishChangesInDocument +final readonly class PublishChangesInDocumentCommand { public function __construct( public ContentRepositoryId $contentRepositoryId, diff --git a/Classes/Application/PublishChangesInDocument/PublishChangesInDocumentCommandHandler.php b/Classes/Application/PublishChangesInDocument/PublishChangesInDocumentCommandHandler.php new file mode 100644 index 0000000000..4b5f6bb267 --- /dev/null +++ b/Classes/Application/PublishChangesInDocument/PublishChangesInDocumentCommandHandler.php @@ -0,0 +1,68 @@ +workspaceProvider->provideForWorkspaceName( + $command->contentRepositoryId, + $command->workspaceName + ); + $publishingResult = $workspace->publishChangesInDocument($command->documentId); + + return new PublishSucceeded( + numberOfAffectedChanges: $publishingResult->numberOfPublishedChanges, + baseWorkspaceName: $workspace->getCurrentBaseWorkspaceName()?->value + ); + } catch (NodeAggregateCurrentlyDoesNotExist $e) { + throw new \RuntimeException( + $this->getLabel('NodeNotPublishedMissingParentNode'), + 1705053430, + $e + ); + } catch (NodeAggregateDoesCurrentlyNotCoverDimensionSpacePoint $e) { + throw new \RuntimeException( + $this->getLabel('NodeNotPublishedParentNodeNotInCurrentDimension'), + 1705053432, + $e + ); + } + } +} diff --git a/Classes/Application/PublishChangesInSite.php b/Classes/Application/PublishChangesInSite/PublishChangesInSiteCommand.php similarity index 89% rename from Classes/Application/PublishChangesInSite.php rename to Classes/Application/PublishChangesInSite/PublishChangesInSiteCommand.php index f645520cf4..0f5b82abde 100644 --- a/Classes/Application/PublishChangesInSite.php +++ b/Classes/Application/PublishChangesInSite/PublishChangesInSiteCommand.php @@ -12,7 +12,7 @@ declare(strict_types=1); -namespace Neos\Neos\Ui\Application; +namespace Neos\Neos\Ui\Application\PublishChangesInSite; use Neos\ContentRepository\Core\SharedModel\ContentRepository\ContentRepositoryId; use Neos\ContentRepository\Core\SharedModel\Node\NodeAggregateId; @@ -20,12 +20,13 @@ use Neos\Flow\Annotations as Flow; /** - * The application layer level command DTO to communicate publication of all changes recorded for a given site + * The application layer level command DTO to communicate publication of + * all changes recorded for a given site * * @internal for communication within the Neos UI only */ #[Flow\Proxy(false)] -final readonly class PublishChangesInSite +final readonly class PublishChangesInSiteCommand { public function __construct( public ContentRepositoryId $contentRepositoryId, diff --git a/Classes/Application/PublishChangesInSite/PublishChangesInSiteCommandHandler.php b/Classes/Application/PublishChangesInSite/PublishChangesInSiteCommandHandler.php new file mode 100644 index 0000000000..63710ca355 --- /dev/null +++ b/Classes/Application/PublishChangesInSite/PublishChangesInSiteCommandHandler.php @@ -0,0 +1,46 @@ +workspaceProvider->provideForWorkspaceName( + $command->contentRepositoryId, + $command->workspaceName + ); + $publishingResult = $workspace->publishChangesInSite($command->siteId); + + return new PublishSucceeded( + numberOfAffectedChanges: $publishingResult->numberOfPublishedChanges, + baseWorkspaceName: $workspace->getCurrentBaseWorkspaceName()?->value + ); + } +} diff --git a/Classes/Application/Shared/PublishSucceeded.php b/Classes/Application/Shared/PublishSucceeded.php new file mode 100644 index 0000000000..dd20220b2a --- /dev/null +++ b/Classes/Application/Shared/PublishSucceeded.php @@ -0,0 +1,37 @@ + get_object_vars($this) + ]; + } +} diff --git a/Classes/Controller/BackendServiceController.php b/Classes/Controller/BackendServiceController.php index fdb6b56c46..41bb1fbd67 100644 --- a/Classes/Controller/BackendServiceController.php +++ b/Classes/Controller/BackendServiceController.php @@ -19,8 +19,6 @@ use Neos\ContentRepository\Core\Feature\WorkspaceRebase\Dto\RebaseErrorHandlingStrategy; use Neos\ContentRepository\Core\Projection\ContentGraph\Node; use Neos\ContentRepository\Core\Projection\ContentGraph\VisibilityConstraints; -use Neos\ContentRepository\Core\SharedModel\Exception\NodeAggregateCurrentlyDoesNotExist; -use Neos\ContentRepository\Core\SharedModel\Exception\NodeAggregateDoesCurrentlyNotCoverDimensionSpacePoint; use Neos\ContentRepository\Core\SharedModel\Workspace\WorkspaceName; use Neos\ContentRepositoryRegistry\ContentRepositoryRegistry; use Neos\Eel\FlowQuery\FlowQuery; @@ -42,8 +40,10 @@ use Neos\Neos\Ui\Application\DiscardAllChanges; use Neos\Neos\Ui\Application\DiscardChangesInDocument; use Neos\Neos\Ui\Application\DiscardChangesInSite; -use Neos\Neos\Ui\Application\PublishChangesInDocument; -use Neos\Neos\Ui\Application\PublishChangesInSite; +use Neos\Neos\Ui\Application\PublishChangesInDocument\PublishChangesInDocumentCommand; +use Neos\Neos\Ui\Application\PublishChangesInDocument\PublishChangesInDocumentCommandHandler; +use Neos\Neos\Ui\Application\PublishChangesInSite\PublishChangesInSiteCommand; +use Neos\Neos\Ui\Application\PublishChangesInSite\PublishChangesInSiteCommandHandler; use Neos\Neos\Ui\Application\ReloadNodes\ReloadNodesQuery; use Neos\Neos\Ui\Application\ReloadNodes\ReloadNodesQueryHandler; use Neos\Neos\Ui\Application\Shared\ConflictsOccurred; @@ -140,6 +140,18 @@ class BackendServiceController extends ActionController */ protected $workspaceProvider; + /** + * @Flow\Inject + * @var PublishChangesInSiteCommandHandler + */ + protected $publishChangesInSiteCommandHandler; + + /** + * @Flow\Inject + * @var PublishChangesInDocumentCommandHandler + */ + protected $publishChangesInDocumentCommandHandler; + /** * @Flow\Inject * @var SyncWorkspaceCommandHandler @@ -206,20 +218,12 @@ public function publishChangesInSiteAction(array $command): void $command['siteId'], $contentRepositoryId )->nodeAggregateId->value; - $command = PublishChangesInSite::fromArray($command); - $workspace = $this->workspaceProvider->provideForWorkspaceName( - $command->contentRepositoryId, - $command->workspaceName - ); - $publishingResult = $workspace - ->publishChangesInSite($command->siteId); + $command = PublishChangesInSiteCommand::fromArray($command); - $this->view->assign('value', [ - 'success' => [ - 'numberOfAffectedChanges' => $publishingResult->numberOfPublishedChanges, - 'baseWorkspaceName' => $workspace->getCurrentBaseWorkspaceName()?->value - ] - ]); + $result = $this->publishChangesInSiteCommandHandler + ->handle($command); + + $this->view->assign('value', $result); } catch (\Exception $e) { $this->view->assign('value', [ 'error' => [ @@ -247,36 +251,12 @@ public function publishChangesInDocumentAction(array $command): void $command['documentId'], $contentRepositoryId )->nodeAggregateId->value; - $command = PublishChangesInDocument::fromArray($command); + $command = PublishChangesInDocumentCommand::fromArray($command); - $contentRepositoryId = SiteDetectionResult::fromRequest($this->request->getHttpRequest())->contentRepositoryId; + $result = $this->publishChangesInDocumentCommandHandler + ->handle($command); - try { - $workspace = $this->workspaceProvider->provideForWorkspaceName( - $command->contentRepositoryId, - $command->workspaceName - ); - $publishingResult = $workspace->publishChangesInDocument($command->documentId); - - $this->view->assign('value', [ - 'success' => [ - 'numberOfAffectedChanges' => $publishingResult->numberOfPublishedChanges, - 'baseWorkspaceName' => $workspace->getCurrentBaseWorkspaceName()?->value - ] - ]); - } catch (NodeAggregateCurrentlyDoesNotExist $e) { - throw new \RuntimeException( - $this->getLabel('NodeNotPublishedMissingParentNode'), - 1705053430, - $e - ); - } catch (NodeAggregateDoesCurrentlyNotCoverDimensionSpacePoint $e) { - throw new \RuntimeException( - $this->getLabel('NodeNotPublishedParentNodeNotInCurrentDimension'), - 1705053432, - $e - ); - } + $this->view->assign('value', $result); } catch (\Exception $e) { $this->view->assign('value', [ 'error' => [ From 10c00989de7761d8fefcb46d01219a131707a486 Mon Sep 17 00:00:00 2001 From: Wilhelm Behncke Date: Mon, 22 Apr 2024 17:13:23 +0200 Subject: [PATCH 03/36] TASK: Ensure that all Publish-related command handlers throw `ConflictsOccurred` --- .../PublishChangesInDocumentCommand.php | 7 ++- ...PublishChangesInDocumentCommandHandler.php | 23 ++++++++++ .../PublishChangesInSiteCommand.php | 7 ++- .../PublishChangesInSiteCommandHandler.php | 45 ++++++++++++++----- .../Controller/BackendServiceController.php | 4 +- .../src/Endpoints/index.ts | 8 ++-- packages/neos-ui-sagas/src/Publish/index.ts | 5 ++- 7 files changed, 79 insertions(+), 20 deletions(-) diff --git a/Classes/Application/PublishChangesInDocument/PublishChangesInDocumentCommand.php b/Classes/Application/PublishChangesInDocument/PublishChangesInDocumentCommand.php index 87d9762511..f5c06a6616 100644 --- a/Classes/Application/PublishChangesInDocument/PublishChangesInDocumentCommand.php +++ b/Classes/Application/PublishChangesInDocument/PublishChangesInDocumentCommand.php @@ -14,6 +14,7 @@ namespace Neos\Neos\Ui\Application\PublishChangesInDocument; +use Neos\ContentRepository\Core\DimensionSpace\DimensionSpacePoint; use Neos\ContentRepository\Core\SharedModel\ContentRepository\ContentRepositoryId; use Neos\ContentRepository\Core\SharedModel\Node\NodeAggregateId; use Neos\ContentRepository\Core\SharedModel\Workspace\WorkspaceName; @@ -32,11 +33,12 @@ public function __construct( public ContentRepositoryId $contentRepositoryId, public WorkspaceName $workspaceName, public NodeAggregateId $documentId, + public ?DimensionSpacePoint $preferredDimensionSpacePoint, ) { } /** - * @param array $values + * @param array{contentRepositoryId:string,workspaceName:string,documentId:string,preferredDimensionSpacePoint?:array} $values */ public static function fromArray(array $values): self { @@ -44,6 +46,9 @@ public static function fromArray(array $values): self ContentRepositoryId::fromString($values['contentRepositoryId']), WorkspaceName::fromString($values['workspaceName']), NodeAggregateId::fromString($values['documentId']), + isset($values['preferredDimensionSpacePoint']) && !empty($values['preferredDimensionSpacePoint']) + ? DimensionSpacePoint::fromLegacyDimensionArray($values['preferredDimensionSpacePoint']) + : null, ); } } diff --git a/Classes/Application/PublishChangesInDocument/PublishChangesInDocumentCommandHandler.php b/Classes/Application/PublishChangesInDocument/PublishChangesInDocumentCommandHandler.php index 4b5f6bb267..4c3555cc3b 100644 --- a/Classes/Application/PublishChangesInDocument/PublishChangesInDocumentCommandHandler.php +++ b/Classes/Application/PublishChangesInDocument/PublishChangesInDocumentCommandHandler.php @@ -14,10 +14,14 @@ namespace Neos\Neos\Ui\Application\PublishChangesInDocument; +use Neos\ContentRepository\Core\Feature\WorkspaceRebase\Exception\WorkspaceRebaseFailed; use Neos\ContentRepository\Core\SharedModel\Exception\NodeAggregateCurrentlyDoesNotExist; use Neos\ContentRepository\Core\SharedModel\Exception\NodeAggregateDoesCurrentlyNotCoverDimensionSpacePoint; +use Neos\ContentRepositoryRegistry\ContentRepositoryRegistry; use Neos\Flow\Annotations as Flow; use Neos\Neos\Domain\Workspace\WorkspaceProvider; +use Neos\Neos\Ui\Application\Shared\Conflicts; +use Neos\Neos\Ui\Application\Shared\ConflictsOccurred; use Neos\Neos\Ui\Application\Shared\PublishSucceeded; use Neos\Neos\Ui\Controller\TranslationTrait; @@ -32,6 +36,9 @@ final class PublishChangesInDocumentCommandHandler { use TranslationTrait; + #[Flow\Inject] + protected ContentRepositoryRegistry $contentRepositoryRegistry; + #[Flow\Inject] protected WorkspaceProvider $workspaceProvider; @@ -63,6 +70,22 @@ public function handle(PublishChangesInDocumentCommand $command): PublishSucceed 1705053432, $e ); + } catch (WorkspaceRebaseFailed $e) { + $conflictsBuilder = Conflicts::builder( + contentRepository: $this->contentRepositoryRegistry + ->get($command->contentRepositoryId), + workspaceName: $command->workspaceName, + preferredDimensionSpacePoint: $command->preferredDimensionSpacePoint + ); + + foreach ($e->commandsThatFailedDuringRebase as $commandThatFailedDuringRebase) { + $conflictsBuilder->addCommandThatFailedDuringRebase($commandThatFailedDuringRebase); + } + + throw new ConflictsOccurred( + $conflictsBuilder->build(), + 1712832228 + ); } } } diff --git a/Classes/Application/PublishChangesInSite/PublishChangesInSiteCommand.php b/Classes/Application/PublishChangesInSite/PublishChangesInSiteCommand.php index 0f5b82abde..f177482c41 100644 --- a/Classes/Application/PublishChangesInSite/PublishChangesInSiteCommand.php +++ b/Classes/Application/PublishChangesInSite/PublishChangesInSiteCommand.php @@ -14,6 +14,7 @@ namespace Neos\Neos\Ui\Application\PublishChangesInSite; +use Neos\ContentRepository\Core\DimensionSpace\DimensionSpacePoint; use Neos\ContentRepository\Core\SharedModel\ContentRepository\ContentRepositoryId; use Neos\ContentRepository\Core\SharedModel\Node\NodeAggregateId; use Neos\ContentRepository\Core\SharedModel\Workspace\WorkspaceName; @@ -32,11 +33,12 @@ public function __construct( public ContentRepositoryId $contentRepositoryId, public WorkspaceName $workspaceName, public NodeAggregateId $siteId, + public ?DimensionSpacePoint $preferredDimensionSpacePoint, ) { } /** - * @param array $values + * @param array{contentRepositoryId:string,workspaceName:string,siteId:string,preferredDimensionSpacePoint?:array} $values */ public static function fromArray(array $values): self { @@ -44,6 +46,9 @@ public static function fromArray(array $values): self ContentRepositoryId::fromString($values['contentRepositoryId']), WorkspaceName::fromString($values['workspaceName']), NodeAggregateId::fromString($values['siteId']), + isset($values['preferredDimensionSpacePoint']) && !empty($values['preferredDimensionSpacePoint']) + ? DimensionSpacePoint::fromLegacyDimensionArray($values['preferredDimensionSpacePoint']) + : null, ); } } diff --git a/Classes/Application/PublishChangesInSite/PublishChangesInSiteCommandHandler.php b/Classes/Application/PublishChangesInSite/PublishChangesInSiteCommandHandler.php index 63710ca355..ad83969e8c 100644 --- a/Classes/Application/PublishChangesInSite/PublishChangesInSiteCommandHandler.php +++ b/Classes/Application/PublishChangesInSite/PublishChangesInSiteCommandHandler.php @@ -14,8 +14,12 @@ namespace Neos\Neos\Ui\Application\PublishChangesInSite; +use Neos\ContentRepository\Core\Feature\WorkspaceRebase\Exception\WorkspaceRebaseFailed; +use Neos\ContentRepositoryRegistry\ContentRepositoryRegistry; use Neos\Flow\Annotations as Flow; use Neos\Neos\Domain\Workspace\WorkspaceProvider; +use Neos\Neos\Ui\Application\Shared\Conflicts; +use Neos\Neos\Ui\Application\Shared\ConflictsOccurred; use Neos\Neos\Ui\Application\Shared\PublishSucceeded; /** @@ -27,20 +31,41 @@ #[Flow\Scope("singleton")] final class PublishChangesInSiteCommandHandler { + #[Flow\Inject] + protected ContentRepositoryRegistry $contentRepositoryRegistry; + #[Flow\Inject] protected WorkspaceProvider $workspaceProvider; public function handle(PublishChangesInSiteCommand $command): PublishSucceeded { - $workspace = $this->workspaceProvider->provideForWorkspaceName( - $command->contentRepositoryId, - $command->workspaceName - ); - $publishingResult = $workspace->publishChangesInSite($command->siteId); - - return new PublishSucceeded( - numberOfAffectedChanges: $publishingResult->numberOfPublishedChanges, - baseWorkspaceName: $workspace->getCurrentBaseWorkspaceName()?->value - ); + try { + $workspace = $this->workspaceProvider->provideForWorkspaceName( + $command->contentRepositoryId, + $command->workspaceName + ); + $publishingResult = $workspace->publishChangesInSite($command->siteId); + + return new PublishSucceeded( + numberOfAffectedChanges: $publishingResult->numberOfPublishedChanges, + baseWorkspaceName: $workspace->getCurrentBaseWorkspaceName()?->value + ); + } catch (WorkspaceRebaseFailed $e) { + $conflictsBuilder = Conflicts::builder( + contentRepository: $this->contentRepositoryRegistry + ->get($command->contentRepositoryId), + workspaceName: $command->workspaceName, + preferredDimensionSpacePoint: $command->preferredDimensionSpacePoint + ); + + foreach ($e->commandsThatFailedDuringRebase as $commandThatFailedDuringRebase) { + $conflictsBuilder->addCommandThatFailedDuringRebase($commandThatFailedDuringRebase); + } + + throw new ConflictsOccurred( + $conflictsBuilder->build(), + 1712832228 + ); + } } } diff --git a/Classes/Controller/BackendServiceController.php b/Classes/Controller/BackendServiceController.php index 41bb1fbd67..37d08646da 100644 --- a/Classes/Controller/BackendServiceController.php +++ b/Classes/Controller/BackendServiceController.php @@ -206,7 +206,7 @@ public function changeAction(array $changes): void /** * Publish all changes in the current site * - * @phpstan-param array $command + * @phpstan-param array{workspaceName:string,siteId:string,preferredDimensionSpacePoint?:array} $command */ public function publishChangesInSiteAction(array $command): void { @@ -239,7 +239,7 @@ public function publishChangesInSiteAction(array $command): void /** * Publish all changes in the current document * - * @phpstan-param array $command + * @phpstan-param array{workspaceName:string,documentId:string,preferredDimensionSpacePoint?:array} $command */ public function publishChangesInDocumentAction(array $command): void { diff --git a/packages/neos-ui-backend-connector/src/Endpoints/index.ts b/packages/neos-ui-backend-connector/src/Endpoints/index.ts index 1bcfed2c08..aa2130b77d 100644 --- a/packages/neos-ui-backend-connector/src/Endpoints/index.ts +++ b/packages/neos-ui-backend-connector/src/Endpoints/index.ts @@ -69,7 +69,7 @@ export default (routes: Routes) => { })).then(response => fetchWithErrorHandling.parseJson(response)) .catch(reason => fetchWithErrorHandling.generalErrorHandler(reason)); - const publishChangesInSite = (siteId: NodeContextPath, workspaceName: WorkspaceName) => fetchWithErrorHandling.withCsrfToken(csrfToken => ({ + const publishChangesInSite = (siteId: NodeContextPath, workspaceName: WorkspaceName, preferredDimensionSpacePoint: null|DimensionCombination) => fetchWithErrorHandling.withCsrfToken(csrfToken => ({ url: routes.ui.service.publishChangesInSite, method: 'POST', credentials: 'include', @@ -78,12 +78,12 @@ export default (routes: Routes) => { 'Content-Type': 'application/json' }, body: JSON.stringify({ - command: {siteId, workspaceName} + command: {siteId, workspaceName, preferredDimensionSpacePoint} }) })).then(response => fetchWithErrorHandling.parseJson(response)) .catch(reason => fetchWithErrorHandling.generalErrorHandler(reason)); - const publishChangesInDocument = (documentId: NodeContextPath, workspaceName: WorkspaceName) => fetchWithErrorHandling.withCsrfToken(csrfToken => ({ + const publishChangesInDocument = (documentId: NodeContextPath, workspaceName: WorkspaceName, preferredDimensionSpacePoint: null|DimensionCombination) => fetchWithErrorHandling.withCsrfToken(csrfToken => ({ url: routes.ui.service.publishChangesInDocument, method: 'POST', credentials: 'include', @@ -92,7 +92,7 @@ export default (routes: Routes) => { 'Content-Type': 'application/json' }, body: JSON.stringify({ - command: {documentId, workspaceName} + command: {documentId, workspaceName, preferredDimensionSpacePoint} }) })).then(response => fetchWithErrorHandling.parseJson(response)) .catch(reason => fetchWithErrorHandling.generalErrorHandler(reason)); diff --git a/packages/neos-ui-sagas/src/Publish/index.ts b/packages/neos-ui-sagas/src/Publish/index.ts index 63220b1339..e552422347 100644 --- a/packages/neos-ui-sagas/src/Publish/index.ts +++ b/packages/neos-ui-sagas/src/Publish/index.ts @@ -10,7 +10,7 @@ import {put, call, select, takeEvery, take, race, all} from 'redux-saga/effects'; import {AnyError} from '@neos-project/neos-ui-error'; -import {NodeContextPath, WorkspaceName} from '@neos-project/neos-ts-interfaces'; +import {DimensionCombination, NodeContextPath, WorkspaceName} from '@neos-project/neos-ts-interfaces'; import {actionTypes, actions, selectors} from '@neos-project/neos-ui-redux-store'; import {GlobalState} from '@neos-project/neos-ui-redux-store/src/System'; import {FeedbackEnvelope} from '@neos-project/neos-ui-redux-store/src/ServerFeedback'; @@ -83,6 +83,7 @@ export function * watchPublishing({routes}: {routes: Routes}) { const {ancestorIdSelector} = SELECTORS_BY_SCOPE[scope]; const workspaceName: WorkspaceName = yield select(selectors.CR.Workspaces.personalWorkspaceNameSelector); + const dimensionSpacePoint: null|DimensionCombination = yield select(selectors.CR.ContentDimensions.active); const ancestorId: NodeContextPath = ancestorIdSelector ? yield select(ancestorIdSelector) : null; @@ -92,7 +93,7 @@ export function * watchPublishing({routes}: {routes: Routes}) { window.addEventListener('beforeunload', handleWindowBeforeUnload); const result: PublishingResponse = scope === PublishingScope.ALL ? yield call(endpoint as any, workspaceName) - : yield call(endpoint, ancestorId, workspaceName); + : yield call(endpoint, ancestorId, workspaceName, dimensionSpacePoint); if ('success' in result) { yield put(actions.CR.Publishing.succeed(result.success.numberOfAffectedChanges)); From 852bad1fb0dca94e47bedab03b9e89a9846689f5 Mon Sep 17 00:00:00 2001 From: Wilhelm Behncke Date: Mon, 22 Apr 2024 17:49:20 +0200 Subject: [PATCH 04/36] TASK: Turn `ConflictsOccurred` into a Result DTO rather than an exception --- ...PublishChangesInDocumentCommandHandler.php | 10 +++--- .../PublishChangesInSiteCommandHandler.php | 10 +++--- .../Application/Shared/ConflictsOccurred.php | 17 ++++++---- .../SyncWorkspaceCommandHandler.php | 15 ++++----- .../SyncWorkspace/SyncingSucceeded.php | 32 +++++++++++++++++++ .../Controller/BackendServiceController.php | 10 ++---- 6 files changed, 61 insertions(+), 33 deletions(-) create mode 100644 Classes/Application/SyncWorkspace/SyncingSucceeded.php diff --git a/Classes/Application/PublishChangesInDocument/PublishChangesInDocumentCommandHandler.php b/Classes/Application/PublishChangesInDocument/PublishChangesInDocumentCommandHandler.php index 4c3555cc3b..73511cd6b4 100644 --- a/Classes/Application/PublishChangesInDocument/PublishChangesInDocumentCommandHandler.php +++ b/Classes/Application/PublishChangesInDocument/PublishChangesInDocumentCommandHandler.php @@ -45,8 +45,9 @@ final class PublishChangesInDocumentCommandHandler /** * @throws NodeAggregateCurrentlyDoesNotExist */ - public function handle(PublishChangesInDocumentCommand $command): PublishSucceeded - { + public function handle( + PublishChangesInDocumentCommand $command + ): PublishSucceeded|ConflictsOccurred { try { $workspace = $this->workspaceProvider->provideForWorkspaceName( $command->contentRepositoryId, @@ -82,9 +83,8 @@ public function handle(PublishChangesInDocumentCommand $command): PublishSucceed $conflictsBuilder->addCommandThatFailedDuringRebase($commandThatFailedDuringRebase); } - throw new ConflictsOccurred( - $conflictsBuilder->build(), - 1712832228 + return new ConflictsOccurred( + conflicts: $conflictsBuilder->build() ); } } diff --git a/Classes/Application/PublishChangesInSite/PublishChangesInSiteCommandHandler.php b/Classes/Application/PublishChangesInSite/PublishChangesInSiteCommandHandler.php index ad83969e8c..5b333c6d30 100644 --- a/Classes/Application/PublishChangesInSite/PublishChangesInSiteCommandHandler.php +++ b/Classes/Application/PublishChangesInSite/PublishChangesInSiteCommandHandler.php @@ -37,8 +37,9 @@ final class PublishChangesInSiteCommandHandler #[Flow\Inject] protected WorkspaceProvider $workspaceProvider; - public function handle(PublishChangesInSiteCommand $command): PublishSucceeded - { + public function handle( + PublishChangesInSiteCommand $command + ): PublishSucceeded|ConflictsOccurred { try { $workspace = $this->workspaceProvider->provideForWorkspaceName( $command->contentRepositoryId, @@ -62,9 +63,8 @@ public function handle(PublishChangesInSiteCommand $command): PublishSucceeded $conflictsBuilder->addCommandThatFailedDuringRebase($commandThatFailedDuringRebase); } - throw new ConflictsOccurred( - $conflictsBuilder->build(), - 1712832228 + return new ConflictsOccurred( + conflicts: $conflictsBuilder->build() ); } } diff --git a/Classes/Application/Shared/ConflictsOccurred.php b/Classes/Application/Shared/ConflictsOccurred.php index 6917e42a9d..fcf1817f5a 100644 --- a/Classes/Application/Shared/ConflictsOccurred.php +++ b/Classes/Application/Shared/ConflictsOccurred.php @@ -14,18 +14,21 @@ namespace Neos\Neos\Ui\Application\Shared; +use Neos\Flow\Annotations as Flow; + /** * @internal for communication within the Neos UI only */ -final class ConflictsOccurred extends \Exception +#[Flow\Proxy(false)] +final readonly class ConflictsOccurred implements \JsonSerializable { public function __construct( - public readonly Conflicts $conflicts, - int $code + public readonly Conflicts $conflicts ) { - parent::__construct( - sprintf('%s conflict(s) occurred during rebase.', count($conflicts)), - $code - ); + } + + public function jsonSerialize(): mixed + { + return get_object_vars($this); } } diff --git a/Classes/Application/SyncWorkspace/SyncWorkspaceCommandHandler.php b/Classes/Application/SyncWorkspace/SyncWorkspaceCommandHandler.php index 2006bb0117..1019caa643 100644 --- a/Classes/Application/SyncWorkspace/SyncWorkspaceCommandHandler.php +++ b/Classes/Application/SyncWorkspace/SyncWorkspaceCommandHandler.php @@ -39,11 +39,9 @@ final class SyncWorkspaceCommandHandler #[Flow\Inject] protected NodeLabelGeneratorInterface $nodeLabelGenerator; - /** - * @throws ConflictsOccurred - */ - public function handle(SyncWorkspaceCommand $command): void - { + public function handle( + SyncWorkspaceCommand $command + ): SyncingSucceeded|ConflictsOccurred { try { $workspace = $this->workspaceProvider->provideForWorkspaceName( $command->contentRepositoryId, @@ -51,6 +49,8 @@ public function handle(SyncWorkspaceCommand $command): void ); $workspace->rebase($command->rebaseErrorHandlingStrategy); + + return new SyncingSucceeded(); } catch (WorkspaceRebaseFailed $e) { $conflictsBuilder = Conflicts::builder( contentRepository: $this->contentRepositoryRegistry @@ -64,9 +64,8 @@ public function handle(SyncWorkspaceCommand $command): void $conflictsBuilder->addCommandThatFailedDuringRebase($commandThatFailedDuringRebase); } - throw new ConflictsOccurred( - $conflictsBuilder->build(), - 1712832228 + return new ConflictsOccurred( + conflicts: $conflictsBuilder->build() ); } } diff --git a/Classes/Application/SyncWorkspace/SyncingSucceeded.php b/Classes/Application/SyncWorkspace/SyncingSucceeded.php new file mode 100644 index 0000000000..52f5a0b288 --- /dev/null +++ b/Classes/Application/SyncWorkspace/SyncingSucceeded.php @@ -0,0 +1,32 @@ + true]; + } +} diff --git a/Classes/Controller/BackendServiceController.php b/Classes/Controller/BackendServiceController.php index 37d08646da..2b91bdbfce 100644 --- a/Classes/Controller/BackendServiceController.php +++ b/Classes/Controller/BackendServiceController.php @@ -747,15 +747,9 @@ public function syncWorkspaceAction(string $targetWorkspaceName, bool $force, ?a : RebaseErrorHandlingStrategy::STRATEGY_FAIL ); - $this->syncWorkspaceCommandHandler->handle($command); + $result = $this->syncWorkspaceCommandHandler->handle($command); - $this->view->assign('value', [ - 'success' => true - ]); - } catch (ConflictsOccurred $e) { - $this->view->assign('value', [ - 'conflicts' => $e->conflicts - ]); + $this->view->assign('value', $result); } catch (\Exception $e) { $this->view->assign('value', [ 'error' => [ From aec75b0f92df5cfaef8c4d5f2a1b19dadd0b402b Mon Sep 17 00:00:00 2001 From: Wilhelm Behncke Date: Tue, 23 Apr 2024 12:51:52 +0200 Subject: [PATCH 05/36] TASK: Move logic to create Conflicts from WorkspaceRebaseFailed into dedicated Factory --- ...PublishChangesInDocumentCommandHandler.php | 15 +- .../PublishChangesInSiteCommandHandler.php | 15 +- Classes/Application/Shared/Conflict.php | 2 + Classes/Application/Shared/Conflicts.php | 19 +- .../Application/Shared/ConflictsBuilder.php | 257 +--------------- .../SyncWorkspaceCommandHandler.php | 10 +- .../ContentRepository/ConflictsFactory.php | 281 ++++++++++++++++++ 7 files changed, 312 insertions(+), 287 deletions(-) create mode 100644 Classes/Infrastructure/ContentRepository/ConflictsFactory.php diff --git a/Classes/Application/PublishChangesInDocument/PublishChangesInDocumentCommandHandler.php b/Classes/Application/PublishChangesInDocument/PublishChangesInDocumentCommandHandler.php index 73511cd6b4..9e89d1993b 100644 --- a/Classes/Application/PublishChangesInDocument/PublishChangesInDocumentCommandHandler.php +++ b/Classes/Application/PublishChangesInDocument/PublishChangesInDocumentCommandHandler.php @@ -19,11 +19,12 @@ use Neos\ContentRepository\Core\SharedModel\Exception\NodeAggregateDoesCurrentlyNotCoverDimensionSpacePoint; use Neos\ContentRepositoryRegistry\ContentRepositoryRegistry; use Neos\Flow\Annotations as Flow; +use Neos\Neos\Domain\NodeLabel\NodeLabelGeneratorInterface; use Neos\Neos\Domain\Workspace\WorkspaceProvider; -use Neos\Neos\Ui\Application\Shared\Conflicts; use Neos\Neos\Ui\Application\Shared\ConflictsOccurred; use Neos\Neos\Ui\Application\Shared\PublishSucceeded; use Neos\Neos\Ui\Controller\TranslationTrait; +use Neos\Neos\Ui\Infrastructure\ContentRepository\ConflictsFactory; /** * The application layer level command handler to perform publication of @@ -42,6 +43,9 @@ final class PublishChangesInDocumentCommandHandler #[Flow\Inject] protected WorkspaceProvider $workspaceProvider; + #[Flow\Inject] + protected NodeLabelGeneratorInterface $nodeLabelGenerator; + /** * @throws NodeAggregateCurrentlyDoesNotExist */ @@ -72,19 +76,16 @@ public function handle( $e ); } catch (WorkspaceRebaseFailed $e) { - $conflictsBuilder = Conflicts::builder( + $conflictsFactory = new ConflictsFactory( contentRepository: $this->contentRepositoryRegistry ->get($command->contentRepositoryId), + nodeLabelGenerator: $this->nodeLabelGenerator, workspaceName: $command->workspaceName, preferredDimensionSpacePoint: $command->preferredDimensionSpacePoint ); - foreach ($e->commandsThatFailedDuringRebase as $commandThatFailedDuringRebase) { - $conflictsBuilder->addCommandThatFailedDuringRebase($commandThatFailedDuringRebase); - } - return new ConflictsOccurred( - conflicts: $conflictsBuilder->build() + conflicts: $conflictsFactory->fromWorkspaceRebaseFailed($e) ); } } diff --git a/Classes/Application/PublishChangesInSite/PublishChangesInSiteCommandHandler.php b/Classes/Application/PublishChangesInSite/PublishChangesInSiteCommandHandler.php index 5b333c6d30..b7d8a2e792 100644 --- a/Classes/Application/PublishChangesInSite/PublishChangesInSiteCommandHandler.php +++ b/Classes/Application/PublishChangesInSite/PublishChangesInSiteCommandHandler.php @@ -17,10 +17,11 @@ use Neos\ContentRepository\Core\Feature\WorkspaceRebase\Exception\WorkspaceRebaseFailed; use Neos\ContentRepositoryRegistry\ContentRepositoryRegistry; use Neos\Flow\Annotations as Flow; +use Neos\Neos\Domain\NodeLabel\NodeLabelGeneratorInterface; use Neos\Neos\Domain\Workspace\WorkspaceProvider; -use Neos\Neos\Ui\Application\Shared\Conflicts; use Neos\Neos\Ui\Application\Shared\ConflictsOccurred; use Neos\Neos\Ui\Application\Shared\PublishSucceeded; +use Neos\Neos\Ui\Infrastructure\ContentRepository\ConflictsFactory; /** * The application layer level command handler to perform publication of @@ -37,6 +38,9 @@ final class PublishChangesInSiteCommandHandler #[Flow\Inject] protected WorkspaceProvider $workspaceProvider; + #[Flow\Inject] + protected NodeLabelGeneratorInterface $nodeLabelGenerator; + public function handle( PublishChangesInSiteCommand $command ): PublishSucceeded|ConflictsOccurred { @@ -52,19 +56,16 @@ public function handle( baseWorkspaceName: $workspace->getCurrentBaseWorkspaceName()?->value ); } catch (WorkspaceRebaseFailed $e) { - $conflictsBuilder = Conflicts::builder( + $conflictsFactory = new ConflictsFactory( contentRepository: $this->contentRepositoryRegistry ->get($command->contentRepositoryId), + nodeLabelGenerator: $this->nodeLabelGenerator, workspaceName: $command->workspaceName, preferredDimensionSpacePoint: $command->preferredDimensionSpacePoint ); - foreach ($e->commandsThatFailedDuringRebase as $commandThatFailedDuringRebase) { - $conflictsBuilder->addCommandThatFailedDuringRebase($commandThatFailedDuringRebase); - } - return new ConflictsOccurred( - conflicts: $conflictsBuilder->build() + conflicts: $conflictsFactory->fromWorkspaceRebaseFailed($e) ); } } diff --git a/Classes/Application/Shared/Conflict.php b/Classes/Application/Shared/Conflict.php index 3f77f0ebeb..faf13b690a 100644 --- a/Classes/Application/Shared/Conflict.php +++ b/Classes/Application/Shared/Conflict.php @@ -14,6 +14,7 @@ namespace Neos\Neos\Ui\Application\Shared; +use Neos\ContentRepository\Core\SharedModel\Node\NodeAggregateId; use Neos\Flow\Annotations as Flow; /** @@ -25,6 +26,7 @@ final readonly class Conflict implements \JsonSerializable { public function __construct( + public string $key, public ?IconLabel $affectedSite, public ?IconLabel $affectedDocument, public ?IconLabel $affectedNode, diff --git a/Classes/Application/Shared/Conflicts.php b/Classes/Application/Shared/Conflicts.php index c1a292ffd5..52bfa1723c 100644 --- a/Classes/Application/Shared/Conflicts.php +++ b/Classes/Application/Shared/Conflicts.php @@ -14,11 +14,7 @@ namespace Neos\Neos\Ui\Application\Shared; -use Neos\ContentRepository\Core\ContentRepository; -use Neos\ContentRepository\Core\DimensionSpace\DimensionSpacePoint; -use Neos\ContentRepository\Core\SharedModel\Workspace\WorkspaceName; use Neos\Flow\Annotations as Flow; -use Neos\Neos\Domain\NodeLabel\NodeLabelGeneratorInterface; /** * @internal for communication within the Neos UI only @@ -34,18 +30,9 @@ public function __construct(Conflict ...$items) $this->items = $items; } - public static function builder( - ContentRepository $contentRepository, - NodeLabelGeneratorInterface $nodeLabelGenerator, - WorkspaceName $workspaceName, - ?DimensionSpacePoint $preferredDimensionSpacePoint, - ): ConflictsBuilder { - return new ConflictsBuilder( - contentRepository: $contentRepository, - nodeLabelGenerator: $nodeLabelGenerator, - workspaceName: $workspaceName, - preferredDimensionSpacePoint: $preferredDimensionSpacePoint - ); + public static function builder(): ConflictsBuilder + { + return new ConflictsBuilder(); } public function jsonSerialize(): mixed diff --git a/Classes/Application/Shared/ConflictsBuilder.php b/Classes/Application/Shared/ConflictsBuilder.php index a30267f3ca..379b6a441a 100644 --- a/Classes/Application/Shared/ConflictsBuilder.php +++ b/Classes/Application/Shared/ConflictsBuilder.php @@ -14,37 +14,7 @@ namespace Neos\Neos\Ui\Application\Shared; -use Neos\ContentRepository\Core\CommandHandler\CommandInterface; -use Neos\ContentRepository\Core\ContentRepository; -use Neos\ContentRepository\Core\DimensionSpace\DimensionSpacePoint; -use Neos\ContentRepository\Core\Feature\NodeCreation\Command\CreateNodeAggregateWithNode; -use Neos\ContentRepository\Core\Feature\NodeCreation\Command\CreateNodeAggregateWithNodeAndSerializedProperties; -use Neos\ContentRepository\Core\Feature\NodeDisabling\Command\DisableNodeAggregate; -use Neos\ContentRepository\Core\Feature\NodeDisabling\Command\EnableNodeAggregate; -use Neos\ContentRepository\Core\Feature\NodeModification\Command\SetNodeProperties; -use Neos\ContentRepository\Core\Feature\NodeModification\Command\SetSerializedNodeProperties; -use Neos\ContentRepository\Core\Feature\NodeMove\Command\MoveNodeAggregate; -use Neos\ContentRepository\Core\Feature\NodeReferencing\Command\SetNodeReferences; -use Neos\ContentRepository\Core\Feature\NodeReferencing\Command\SetSerializedNodeReferences; -use Neos\ContentRepository\Core\Feature\NodeRemoval\Command\RemoveNodeAggregate; -use Neos\ContentRepository\Core\Feature\NodeTypeChange\Command\ChangeNodeAggregateType; -use Neos\ContentRepository\Core\Feature\NodeVariation\Command\CreateNodeVariant; -use Neos\ContentRepository\Core\Feature\SubtreeTagging\Command\TagSubtree; -use Neos\ContentRepository\Core\Feature\SubtreeTagging\Command\UntagSubtree; -use Neos\ContentRepository\Core\Feature\WorkspaceRebase\CommandThatFailedDuringRebase; -use Neos\ContentRepository\Core\NodeType\NodeTypeManager; -use Neos\ContentRepository\Core\Projection\ContentGraph\ContentSubgraphInterface; -use Neos\ContentRepository\Core\Projection\ContentGraph\Filter\FindClosestNodeFilter; -use Neos\ContentRepository\Core\Projection\ContentGraph\Node; -use Neos\ContentRepository\Core\Projection\ContentGraph\NodeAggregate; -use Neos\ContentRepository\Core\Projection\ContentGraph\VisibilityConstraints; -use Neos\ContentRepository\Core\SharedModel\Exception\NodeAggregateCurrentlyDoesNotExist; -use Neos\ContentRepository\Core\SharedModel\Exception\WorkspaceDoesNotExist; -use Neos\ContentRepository\Core\SharedModel\Node\NodeAggregateId; -use Neos\ContentRepository\Core\SharedModel\Workspace\WorkspaceName; use Neos\Flow\Annotations as Flow; -use Neos\Neos\Domain\NodeLabel\NodeLabelGeneratorInterface; -use Neos\Neos\Domain\Service\NodeTypeNameFactory; /** * @internal @@ -52,8 +22,6 @@ #[Flow\Proxy(false)] final class ConflictsBuilder { - private NodeTypeManager $nodeTypeManager; - /** * @var Conflict[] */ @@ -62,231 +30,20 @@ final class ConflictsBuilder /** * @var array */ - private array $itemsByAffectedNodeAggregateId = []; - - public function __construct( - private ContentRepository $contentRepository, - private NodeLabelGeneratorInterface $nodeLabelGenerator, - private WorkspaceName $workspaceName, - private ?DimensionSpacePoint $preferredDimensionSpacePoint, - ) { - $this->nodeTypeManager = $contentRepository->getNodeTypeManager(); - } + private array $itemsByKey = []; - public function addCommandThatFailedDuringRebase( - CommandThatFailedDuringRebase $commandThatFailedDuringRebase - ): void { - $nodeAggregateId = $this->extractNodeAggregateIdFromCommand( - $commandThatFailedDuringRebase->command - ); - - if ($nodeAggregateId && isset($this->itemsByAffectedNodeAggregateId[$nodeAggregateId->value])) { - return; + public function addConflict(Conflict $conflict): self + { + if (!isset($this->itemsByKey[$conflict->key])) { + $this->itemsByKey[$conflict->key] = $conflict; + $this->items[] = $conflict; } - $conflict = $this->createConflictFromCommandThatFailedDuringRebase( - $commandThatFailedDuringRebase - ); - - $this->items[] = $conflict; - - if ($nodeAggregateId) { - $this->itemsByAffectedNodeAggregateId[$nodeAggregateId->value] = $conflict; - } + return $this; } public function build(): Conflicts { return new Conflicts(...$this->items); } - - private function createConflictFromCommandThatFailedDuringRebase( - CommandThatFailedDuringRebase $commandThatFailedDuringRebase - ): Conflict { - $nodeAggregateId = $this->extractNodeAggregateIdFromCommand( - $commandThatFailedDuringRebase->command - ); - $subgraph = $this->acquireSubgraphFromCommand( - $commandThatFailedDuringRebase->command, - $nodeAggregateId - ); - $affectedSite = $nodeAggregateId - ? $subgraph?->findClosestNode( - $nodeAggregateId, - FindClosestNodeFilter::create(nodeTypes: NodeTypeNameFactory::NAME_SITE) - ) - : null; - $affectedDocument = $nodeAggregateId - ? $subgraph?->findClosestNode( - $nodeAggregateId, - FindClosestNodeFilter::create(nodeTypes: NodeTypeNameFactory::NAME_DOCUMENT) - ) - : null; - $affectedNode = $nodeAggregateId - ? $subgraph?->findNodeById($nodeAggregateId) - : null; - - return new Conflict( - affectedSite: $affectedSite - ? $this->createIconLabelForNode($affectedSite) - : null, - affectedDocument: $affectedDocument - ? $this->createIconLabelForNode($affectedDocument) - : null, - affectedNode: $affectedNode - ? $this->createIconLabelForNode($affectedNode) - : null, - typeOfChange: $this->createTypeOfChangeFromCommand( - $commandThatFailedDuringRebase->command - ), - reasonForConflict: $this->createReasonForConflictFromException( - $commandThatFailedDuringRebase->exception - ) - ); - } - - private function extractNodeAggregateIdFromCommand(CommandInterface $command): ?NodeAggregateId - { - return match (true) { - $command instanceof MoveNodeAggregate, - $command instanceof SetNodeProperties, - $command instanceof SetSerializedNodeProperties, - $command instanceof CreateNodeAggregateWithNode, - $command instanceof CreateNodeAggregateWithNodeAndSerializedProperties, - $command instanceof TagSubtree, - $command instanceof DisableNodeAggregate, - $command instanceof UntagSubtree, - $command instanceof EnableNodeAggregate, - $command instanceof RemoveNodeAggregate, - $command instanceof ChangeNodeAggregateType, - $command instanceof CreateNodeVariant => - $command->nodeAggregateId, - $command instanceof SetNodeReferences, - $command instanceof SetSerializedNodeReferences => - $command->sourceNodeAggregateId, - default => null - }; - } - - private function acquireSubgraphFromCommand( - CommandInterface $command, - ?NodeAggregateId $nodeAggregateIdForDimensionFallback - ): ?ContentSubgraphInterface { - try { - $contentGraph = $this->contentRepository->getContentGraph($this->workspaceName); - } catch (WorkspaceDoesNotExist) { - return null; - } - - $dimensionSpacePoint = match (true) { - $command instanceof MoveNodeAggregate => - $command->dimensionSpacePoint, - $command instanceof SetNodeProperties, - $command instanceof SetSerializedNodeProperties, - $command instanceof CreateNodeAggregateWithNode, - $command instanceof CreateNodeAggregateWithNodeAndSerializedProperties => - $command->originDimensionSpacePoint->toDimensionSpacePoint(), - $command instanceof SetNodeReferences, - $command instanceof SetSerializedNodeReferences => - $command->sourceOriginDimensionSpacePoint->toDimensionSpacePoint(), - $command instanceof TagSubtree, - $command instanceof DisableNodeAggregate, - $command instanceof UntagSubtree, - $command instanceof EnableNodeAggregate, - $command instanceof RemoveNodeAggregate => - $command->coveredDimensionSpacePoint, - $command instanceof ChangeNodeAggregateType => - null, - $command instanceof CreateNodeVariant => - $command->targetOrigin->toDimensionSpacePoint(), - default => null - }; - - if ($dimensionSpacePoint === null) { - if ($nodeAggregateIdForDimensionFallback === null) { - return null; - } - - $nodeAggregate = $contentGraph - ->findNodeAggregateById( - $nodeAggregateIdForDimensionFallback - ); - - if ($nodeAggregate) { - $dimensionSpacePoint = $this->extractValidDimensionSpacePointFromNodeAggregate( - $nodeAggregate - ); - } - } - - if ($dimensionSpacePoint === null) { - return null; - } - - return $contentGraph->getSubgraph( - $dimensionSpacePoint, - VisibilityConstraints::withoutRestrictions() - ); - } - - private function extractValidDimensionSpacePointFromNodeAggregate( - NodeAggregate $nodeAggregate - ): ?DimensionSpacePoint { - $result = null; - - foreach ($nodeAggregate->coveredDimensionSpacePoints as $coveredDimensionSpacePoint) { - if ($this->preferredDimensionSpacePoint?->equals($coveredDimensionSpacePoint)) { - return $coveredDimensionSpacePoint; - } - $result ??= $coveredDimensionSpacePoint; - } - - return $result; - } - - private function createIconLabelForNode(Node $node): IconLabel - { - $nodeType = $this->nodeTypeManager->getNodeType($node->nodeTypeName); - - return new IconLabel( - icon: $nodeType?->getConfiguration('ui.icon') ?? 'questionmark', - label: $this->nodeLabelGenerator->getLabel($node) - ); - } - - private function createTypeOfChangeFromCommand( - CommandInterface $command - ): ?TypeOfChange { - return match (true) { - $command instanceof CreateNodeAggregateWithNode, - $command instanceof CreateNodeAggregateWithNodeAndSerializedProperties, - $command instanceof CreateNodeVariant => - TypeOfChange::NODE_HAS_BEEN_CREATED, - $command instanceof SetNodeProperties, - $command instanceof SetSerializedNodeProperties, - $command instanceof SetNodeReferences, - $command instanceof SetSerializedNodeReferences, - $command instanceof TagSubtree, - $command instanceof DisableNodeAggregate, - $command instanceof UntagSubtree, - $command instanceof EnableNodeAggregate, - $command instanceof ChangeNodeAggregateType => - TypeOfChange::NODE_HAS_BEEN_CHANGED, - $command instanceof MoveNodeAggregate => - TypeOfChange::NODE_HAS_BEEN_MOVED, - $command instanceof RemoveNodeAggregate => - TypeOfChange::NODE_HAS_BEEN_DELETED, - default => null - }; - } - - private function createReasonForConflictFromException( - \Throwable $exception - ): ?ReasonForConflict { - return match ($exception::class) { - NodeAggregateCurrentlyDoesNotExist::class => - ReasonForConflict::NODE_HAS_BEEN_DELETED, - default => null - }; - } } diff --git a/Classes/Application/SyncWorkspace/SyncWorkspaceCommandHandler.php b/Classes/Application/SyncWorkspace/SyncWorkspaceCommandHandler.php index 1019caa643..eb6affa906 100644 --- a/Classes/Application/SyncWorkspace/SyncWorkspaceCommandHandler.php +++ b/Classes/Application/SyncWorkspace/SyncWorkspaceCommandHandler.php @@ -19,8 +19,8 @@ use Neos\Flow\Annotations as Flow; use Neos\Neos\Domain\NodeLabel\NodeLabelGeneratorInterface; use Neos\Neos\Domain\Workspace\WorkspaceProvider; -use Neos\Neos\Ui\Application\Shared\Conflicts; use Neos\Neos\Ui\Application\Shared\ConflictsOccurred; +use Neos\Neos\Ui\Infrastructure\ContentRepository\ConflictsFactory; /** * The application layer level command handler to for rebasing the workspace @@ -52,7 +52,7 @@ public function handle( return new SyncingSucceeded(); } catch (WorkspaceRebaseFailed $e) { - $conflictsBuilder = Conflicts::builder( + $conflictsFactory = new ConflictsFactory( contentRepository: $this->contentRepositoryRegistry ->get($command->contentRepositoryId), nodeLabelGenerator: $this->nodeLabelGenerator, @@ -60,12 +60,8 @@ public function handle( preferredDimensionSpacePoint: $command->preferredDimensionSpacePoint ); - foreach ($e->commandsThatFailedDuringRebase as $commandThatFailedDuringRebase) { - $conflictsBuilder->addCommandThatFailedDuringRebase($commandThatFailedDuringRebase); - } - return new ConflictsOccurred( - conflicts: $conflictsBuilder->build() + conflicts: $conflictsFactory->fromWorkspaceRebaseFailed($e) ); } } diff --git a/Classes/Infrastructure/ContentRepository/ConflictsFactory.php b/Classes/Infrastructure/ContentRepository/ConflictsFactory.php new file mode 100644 index 0000000000..b51d6b0d95 --- /dev/null +++ b/Classes/Infrastructure/ContentRepository/ConflictsFactory.php @@ -0,0 +1,281 @@ +nodeTypeManager = $contentRepository->getNodeTypeManager(); + + $this->workspace = $contentRepository->getWorkspaceFinder() + ->findOneByName($workspaceName); + } + + public function fromWorkspaceRebaseFailed( + WorkspaceRebaseFailed $workspaceRebaseFailed + ): Conflicts { + $conflictsBuilder = Conflicts::builder(); + + foreach ($workspaceRebaseFailed->commandsThatFailedDuringRebase as $commandThatFailedDuringRebase) { + $conflict = $this->createConflictFromCommandThatFailedDuringRebase($commandThatFailedDuringRebase); + $conflictsBuilder->addConflict($conflict); + } + + return $conflictsBuilder->build(); + } + + private function createConflictFromCommandThatFailedDuringRebase( + CommandThatFailedDuringRebase $commandThatFailedDuringRebase + ): Conflict { + $nodeAggregateId = $this->extractNodeAggregateIdFromCommand( + $commandThatFailedDuringRebase->command + ); + $subgraph = $this->acquireSubgraphFromCommand( + $commandThatFailedDuringRebase->command, + $nodeAggregateId + ); + $affectedSite = $nodeAggregateId + ? $subgraph?->findClosestNode( + $nodeAggregateId, + FindClosestNodeFilter::create(nodeTypes: NodeTypeNameFactory::NAME_SITE) + ) + : null; + $affectedDocument = $nodeAggregateId + ? $subgraph?->findClosestNode( + $nodeAggregateId, + FindClosestNodeFilter::create(nodeTypes: NodeTypeNameFactory::NAME_DOCUMENT) + ) + : null; + $affectedNode = $nodeAggregateId + ? $subgraph?->findNodeById($nodeAggregateId) + : null; + + return new Conflict( + key: $affectedNode + ? $affectedNode->nodeAggregateId->value + : 'command-' . $commandThatFailedDuringRebase->sequenceNumber, + affectedSite: $affectedSite + ? $this->createIconLabelForNode($affectedSite) + : null, + affectedDocument: $affectedDocument + ? $this->createIconLabelForNode($affectedDocument) + : null, + affectedNode: $affectedNode + ? $this->createIconLabelForNode($affectedNode) + : null, + typeOfChange: $this->createTypeOfChangeFromCommand( + $commandThatFailedDuringRebase->command + ), + reasonForConflict: $this->createReasonForConflictFromException( + $commandThatFailedDuringRebase->exception + ) + ); + } + + private function extractNodeAggregateIdFromCommand(CommandInterface $command): ?NodeAggregateId + { + return match (true) { + $command instanceof MoveNodeAggregate, + $command instanceof SetNodeProperties, + $command instanceof SetSerializedNodeProperties, + $command instanceof CreateNodeAggregateWithNode, + $command instanceof CreateNodeAggregateWithNodeAndSerializedProperties, + $command instanceof TagSubtree, + $command instanceof DisableNodeAggregate, + $command instanceof UntagSubtree, + $command instanceof EnableNodeAggregate, + $command instanceof RemoveNodeAggregate, + $command instanceof ChangeNodeAggregateType, + $command instanceof CreateNodeVariant => + $command->nodeAggregateId, + $command instanceof SetNodeReferences, + $command instanceof SetSerializedNodeReferences => + $command->sourceNodeAggregateId, + default => null + }; + } + + private function acquireSubgraphFromCommand( + CommandInterface $command, + ?NodeAggregateId $nodeAggregateIdForDimensionFallback + ): ?ContentSubgraphInterface { + if ($this->workspace === null) { + return null; + } + + $dimensionSpacePoint = match (true) { + $command instanceof MoveNodeAggregate => + $command->dimensionSpacePoint, + $command instanceof SetNodeProperties, + $command instanceof SetSerializedNodeProperties, + $command instanceof CreateNodeAggregateWithNode, + $command instanceof CreateNodeAggregateWithNodeAndSerializedProperties => + $command->originDimensionSpacePoint->toDimensionSpacePoint(), + $command instanceof SetNodeReferences, + $command instanceof SetSerializedNodeReferences => + $command->sourceOriginDimensionSpacePoint->toDimensionSpacePoint(), + $command instanceof TagSubtree, + $command instanceof DisableNodeAggregate, + $command instanceof UntagSubtree, + $command instanceof EnableNodeAggregate, + $command instanceof RemoveNodeAggregate => + $command->coveredDimensionSpacePoint, + $command instanceof ChangeNodeAggregateType => + null, + $command instanceof CreateNodeVariant => + $command->targetOrigin->toDimensionSpacePoint(), + default => null + }; + + if ($dimensionSpacePoint === null) { + if ($nodeAggregateIdForDimensionFallback === null) { + return null; + } + + $nodeAggregate = $this->contentRepository + ->getContentGraph($this->workspace->workspaceName) + ->findNodeAggregateById($nodeAggregateIdForDimensionFallback); + + if ($nodeAggregate) { + $dimensionSpacePoint = $this->extractValidDimensionSpacePointFromNodeAggregate( + $nodeAggregate + ); + } + } + + if ($dimensionSpacePoint === null) { + return null; + } + + return $this->contentRepository + ->getContentGraph($this->workspace->workspaceName) + ->getSubgraph( + $dimensionSpacePoint, + VisibilityConstraints::withoutRestrictions() + ); + } + + private function extractValidDimensionSpacePointFromNodeAggregate( + NodeAggregate $nodeAggregate + ): ?DimensionSpacePoint { + $result = null; + + foreach ($nodeAggregate->coveredDimensionSpacePoints as $coveredDimensionSpacePoint) { + if ($this->preferredDimensionSpacePoint?->equals($coveredDimensionSpacePoint)) { + return $coveredDimensionSpacePoint; + } + $result ??= $coveredDimensionSpacePoint; + } + + return $result; + } + + private function createIconLabelForNode(Node $node): IconLabel + { + $nodeType = $this->nodeTypeManager->getNodeType($node->nodeTypeName); + + return new IconLabel( + icon: $nodeType?->getConfiguration('ui.icon') ?? 'questionmark', + label: $this->nodeLabelGenerator->getLabel($node), + ); + } + + private function createTypeOfChangeFromCommand( + CommandInterface $command + ): ?TypeOfChange { + return match (true) { + $command instanceof CreateNodeAggregateWithNode, + $command instanceof CreateNodeAggregateWithNodeAndSerializedProperties, + $command instanceof CreateNodeVariant => + TypeOfChange::NODE_HAS_BEEN_CREATED, + $command instanceof SetNodeProperties, + $command instanceof SetSerializedNodeProperties, + $command instanceof SetNodeReferences, + $command instanceof SetSerializedNodeReferences, + $command instanceof TagSubtree, + $command instanceof DisableNodeAggregate, + $command instanceof UntagSubtree, + $command instanceof EnableNodeAggregate, + $command instanceof ChangeNodeAggregateType => + TypeOfChange::NODE_HAS_BEEN_CHANGED, + $command instanceof MoveNodeAggregate => + TypeOfChange::NODE_HAS_BEEN_MOVED, + $command instanceof RemoveNodeAggregate => + TypeOfChange::NODE_HAS_BEEN_DELETED, + default => null + }; + } + + private function createReasonForConflictFromException( + \Throwable $exception + ): ?ReasonForConflict { + return match ($exception::class) { + NodeAggregateCurrentlyDoesNotExist::class => + ReasonForConflict::NODE_HAS_BEEN_DELETED, + default => null + }; + } +} From 67468b3abfd396c68a0f8478bb91707365332914 Mon Sep 17 00:00:00 2001 From: Wilhelm Behncke Date: Tue, 23 Apr 2024 13:15:42 +0200 Subject: [PATCH 06/36] TASK: Use conflict DTO `key` property as key for iteration in ConflictList component --- packages/neos-ui-redux-store/src/CR/Syncing/index.ts | 1 + .../Containers/Modals/SyncWorkspaceDialog/ConflictList.tsx | 4 ++-- 2 files changed, 3 insertions(+), 2 deletions(-) diff --git a/packages/neos-ui-redux-store/src/CR/Syncing/index.ts b/packages/neos-ui-redux-store/src/CR/Syncing/index.ts index 2c41a72cd8..f34d3c8e2e 100644 --- a/packages/neos-ui-redux-store/src/CR/Syncing/index.ts +++ b/packages/neos-ui-redux-store/src/CR/Syncing/index.ts @@ -32,6 +32,7 @@ export enum ReasonForConflict { } export type Conflict = { + key: string; affectedNode: null | { icon: string; label: string; diff --git a/packages/neos-ui/src/Containers/Modals/SyncWorkspaceDialog/ConflictList.tsx b/packages/neos-ui/src/Containers/Modals/SyncWorkspaceDialog/ConflictList.tsx index b6a20b94e8..97a9189aa5 100644 --- a/packages/neos-ui/src/Containers/Modals/SyncWorkspaceDialog/ConflictList.tsx +++ b/packages/neos-ui/src/Containers/Modals/SyncWorkspaceDialog/ConflictList.tsx @@ -23,9 +23,9 @@ export const ConflictList: React.FC<{ }> = (props) => { return (
    - {props.conflicts.map((conflict, i) => ( + {props.conflicts.map((conflict) => ( From 24927dcd95418a2464e2ceb4cd11297411071a53 Mon Sep 17 00:00:00 2001 From: Wilhelm Behncke Date: Tue, 23 Apr 2024 15:57:19 +0200 Subject: [PATCH 07/36] TASK: Trigger conflict resolution from within `Publish` saga --- .../src/CR/Publishing/index.ts | 30 +++++++++++ .../src/CR/Syncing/index.ts | 25 +++++++++ packages/neos-ui-sagas/src/Publish/index.ts | 52 ++++++++++++++----- packages/neos-ui-sagas/src/Sync/index.ts | 39 +++++++++----- .../PublishingDialog/PublishingDialog.tsx | 3 ++ 5 files changed, 125 insertions(+), 24 deletions(-) diff --git a/packages/neos-ui-redux-store/src/CR/Publishing/index.ts b/packages/neos-ui-redux-store/src/CR/Publishing/index.ts index 21fea2cac6..7191709d60 100644 --- a/packages/neos-ui-redux-store/src/CR/Publishing/index.ts +++ b/packages/neos-ui-redux-store/src/CR/Publishing/index.ts @@ -25,6 +25,7 @@ export enum PublishingScope { export enum PublishingPhase { START, ONGOING, + CONFLICTS, SUCCESS, ERROR } @@ -35,6 +36,7 @@ export type State = null | { process: | { phase: PublishingPhase.START } | { phase: PublishingPhase.ONGOING } + | { phase: PublishingPhase.CONFLICTS } | { phase: PublishingPhase.ERROR; error: null | AnyError; @@ -51,6 +53,8 @@ export enum actionTypes { STARTED = '@neos/neos-ui/CR/Publishing/STARTED', CANCELLED = '@neos/neos-ui/CR/Publishing/CANCELLED', CONFIRMED = '@neos/neos-ui/CR/Publishing/CONFIRMED', + CONFLICTS_OCCURRED = '@neos/neos-ui/CR/Publishing/CONFLICTS_OCCURRED', + CONFLICTS_RESOLVED = '@neos/neos-ui/CR/Publishing/CONFLICTS_RESOLVED', FAILED = '@neos/neos-ui/CR/Publishing/FAILED', RETRIED = '@neos/neos-ui/CR/Publishing/RETRIED', SUCEEDED = '@neos/neos-ui/CR/Publishing/SUCEEDED', @@ -74,6 +78,16 @@ const cancel = () => createAction(actionTypes.CANCELLED); */ const confirm = () => createAction(actionTypes.CONFIRMED); +/** + * Signal that conflicts have occurred during the publish/discard operation + */ +const conflicts = () => createAction(actionTypes.CONFLICTS_OCCURRED); + +/** + * Signal that conflicts have been resolved during the publish/discard operation + */ +const resolveConflicts = () => createAction(actionTypes.CONFLICTS_RESOLVED); + /** * Signal that the ongoing publish/discard workflow has failed */ @@ -108,6 +122,8 @@ export const actions = { start, cancel, confirm, + conflicts, + resolveConflicts, fail, retry, succeed, @@ -145,6 +161,20 @@ export const reducer = (state: State = defaultState, action: Action): State => { phase: PublishingPhase.ONGOING } }; + case actionTypes.CONFLICTS_OCCURRED: + return { + ...state, + process: { + phase: PublishingPhase.CONFLICTS + } + }; + case actionTypes.CONFLICTS_RESOLVED: + return { + ...state, + process: { + phase: PublishingPhase.ONGOING + } + }; case actionTypes.FAILED: return { ...state, diff --git a/packages/neos-ui-redux-store/src/CR/Syncing/index.ts b/packages/neos-ui-redux-store/src/CR/Syncing/index.ts index f34d3c8e2e..ea6b159677 100644 --- a/packages/neos-ui-redux-store/src/CR/Syncing/index.ts +++ b/packages/neos-ui-redux-store/src/CR/Syncing/index.ts @@ -50,6 +50,7 @@ export type Conflict = { }; export type State = null | { + autoAcknowledge: boolean; process: | { phase: SyncingPhase.START } | { phase: SyncingPhase.ONGOING } @@ -178,12 +179,24 @@ export const reducer = (state: State = defaultState, action: Action): State => { if (state === null) { if (action.type === actionTypes.STARTED) { return { + autoAcknowledge: false, process: { phase: SyncingPhase.START } }; } + if (action.type === actionTypes.CONFLICTS_DETECTED) { + return { + autoAcknowledge: true, + process: { + phase: SyncingPhase.CONFLICT, + conflicts: action.payload.conflicts, + strategy: null + } + }; + } + return null; } @@ -192,12 +205,14 @@ export const reducer = (state: State = defaultState, action: Action): State => { return null; case actionTypes.CONFIRMED: return { + ...state, process: { phase: SyncingPhase.ONGOING } }; case actionTypes.CONFLICTS_DETECTED: return { + ...state, process: { phase: SyncingPhase.CONFLICT, conflicts: action.payload.conflicts, @@ -207,6 +222,7 @@ export const reducer = (state: State = defaultState, action: Action): State => { case actionTypes.RESOLUTION_STARTED: if (state.process.phase === SyncingPhase.CONFLICT) { return { + ...state, process: { ...state.process, phase: SyncingPhase.RESOLVING, @@ -218,6 +234,7 @@ export const reducer = (state: State = defaultState, action: Action): State => { case actionTypes.RESOLUTION_CANCELLED: if (state.process.phase === SyncingPhase.RESOLVING) { return { + ...state, process: { ...state.process, phase: SyncingPhase.CONFLICT @@ -227,12 +244,14 @@ export const reducer = (state: State = defaultState, action: Action): State => { return state; case actionTypes.RESOLUTION_CONFIRMED: return { + ...state, process: { phase: SyncingPhase.ONGOING } }; case actionTypes.FAILED: return { + ...state, process: { phase: SyncingPhase.ERROR, error: action.payload.error @@ -240,12 +259,18 @@ export const reducer = (state: State = defaultState, action: Action): State => { }; case actionTypes.RETRIED: return { + ...state, process: { phase: SyncingPhase.ONGOING } }; case actionTypes.SUCEEDED: + if (state.autoAcknowledge) { + return null; + } + return { + ...state, process: { phase: SyncingPhase.SUCCESS } diff --git a/packages/neos-ui-sagas/src/Publish/index.ts b/packages/neos-ui-sagas/src/Publish/index.ts index e552422347..ceb0673d5e 100644 --- a/packages/neos-ui-sagas/src/Publish/index.ts +++ b/packages/neos-ui-sagas/src/Publish/index.ts @@ -15,10 +15,12 @@ import {actionTypes, actions, selectors} from '@neos-project/neos-ui-redux-store import {GlobalState} from '@neos-project/neos-ui-redux-store/src/System'; import {FeedbackEnvelope} from '@neos-project/neos-ui-redux-store/src/ServerFeedback'; import {PublishingMode, PublishingScope} from '@neos-project/neos-ui-redux-store/src/CR/Publishing'; +import {Conflict} from '@neos-project/neos-ui-redux-store/src/CR/Syncing'; import backend, {Routes} from '@neos-project/neos-ui-backend-connector'; import {makeReloadNodes} from '../CR/NodeOperations/reloadNodes'; import {updateWorkspaceInfo} from '../CR/Workspaces'; +import {makeResolveConflicts, makeSyncPersonalWorkspace} from '../Sync'; const handleWindowBeforeUnload = (event: BeforeUnloadEvent) => { event.preventDefault(); @@ -32,6 +34,7 @@ type PublishingResponse = numberOfAffectedChanges: number; } } + | { conflicts: Conflict[] } | { error: AnyError }; export function * watchPublishing({routes}: {routes: Routes}) { @@ -67,6 +70,8 @@ export function * watchPublishing({routes}: {routes: Routes}) { }; const reloadAfterPublishing = makeReloadAfterPublishing({routes}); + const syncPersonalWorkspace = makeSyncPersonalWorkspace({routes}); + const resolveConflicts = makeResolveConflicts({syncPersonalWorkspace}); yield takeEvery(actionTypes.CR.Publishing.STARTED, function * publishingWorkflow(action: ReturnType) { const confirmed = yield * waitForConfirmation(); @@ -88,21 +93,37 @@ export function * watchPublishing({routes}: {routes: Routes}) { ? yield select(ancestorIdSelector) : null; + function * attemptToPublishOrDiscard(): Generator { + const result: PublishingResponse = scope === PublishingScope.ALL + ? yield call(endpoint as any, workspaceName) + : yield call(endpoint!, ancestorId, workspaceName, dimensionSpacePoint); + + if ('success' in result) { + yield put(actions.CR.Publishing.succeed(result.success.numberOfAffectedChanges)); + yield * reloadAfterPublishing(); + } else if ('conflicts' in result) { + yield put(actions.CR.Publishing.conflicts()); + const conflictsWereResolved: boolean = + yield * resolveConflicts(result.conflicts); + + if (conflictsWereResolved) { + yield put(actions.CR.Publishing.resolveConflicts()); + yield * attemptToPublishOrDiscard(); + } else { + yield put(actions.CR.Publishing.cancel()); + yield call(updateWorkspaceInfo); + } + } else if ('error' in result) { + yield put(actions.CR.Publishing.fail(result.error)); + } else { + yield put(actions.CR.Publishing.fail(null)); + } + } + do { try { window.addEventListener('beforeunload', handleWindowBeforeUnload); - const result: PublishingResponse = scope === PublishingScope.ALL - ? yield call(endpoint as any, workspaceName) - : yield call(endpoint, ancestorId, workspaceName, dimensionSpacePoint); - - if ('success' in result) { - yield put(actions.CR.Publishing.succeed(result.success.numberOfAffectedChanges)); - yield * reloadAfterPublishing(); - } else if ('error' in result) { - yield put(actions.CR.Publishing.fail(result.error)); - } else { - yield put(actions.CR.Publishing.fail(null)); - } + yield * attemptToPublishOrDiscard(); } catch (error) { yield put(actions.CR.Publishing.fail(error as AnyError)); } finally { @@ -127,6 +148,13 @@ function * waitForConfirmation() { } function * waitForRetry() { + const isOngoing: boolean = yield select( + (state: GlobalState) => state.cr.publishing !== null + ); + if (!isOngoing) { + return false; + } + const {retried}: { acknowledged: ReturnType; retried: ReturnType; diff --git a/packages/neos-ui-sagas/src/Sync/index.ts b/packages/neos-ui-sagas/src/Sync/index.ts index c9ad7ea86f..7f8287d39b 100644 --- a/packages/neos-ui-sagas/src/Sync/index.ts +++ b/packages/neos-ui-sagas/src/Sync/index.ts @@ -57,7 +57,7 @@ function * waitForConfirmation() { return Boolean(confirmed); } -const makeSyncPersonalWorkspace = (deps: { +export const makeSyncPersonalWorkspace = (deps: { routes: Routes }) => { const refreshAfterSyncing = makeRefreshAfterSyncing(deps); @@ -89,7 +89,7 @@ const makeSyncPersonalWorkspace = (deps: { return syncPersonalWorkspace; } -const makeResolveConflicts = (deps: { +export const makeResolveConflicts = (deps: { syncPersonalWorkspace: ReturnType }) => { const discardAll = makeDiscardAll(deps); @@ -97,18 +97,33 @@ const makeResolveConflicts = (deps: { function * resolveConflicts(conflicts: Conflict[]): any { yield put(actions.CR.Syncing.resolve(conflicts)); - yield takeEvery>( - actionTypes.CR.Syncing.RESOLUTION_STARTED, - function * resolve({payload: {strategy}}) { - if (strategy === ResolutionStrategy.FORCE) { - if (yield * waitForResolutionConfirmation()) { - yield * deps.syncPersonalWorkspace(true); - } - } else if (strategy === ResolutionStrategy.DISCARD_ALL) { - yield * discardAll(); + const {started}: { + cancelled: null | ReturnType; + started: null | ReturnType; + } = yield race({ + cancelled: take(actionTypes.CR.Syncing.CANCELLED), + started: take(actionTypes.CR.Syncing.RESOLUTION_STARTED) + }); + + if (started) { + const {payload: {strategy}} = started; + + if (strategy === ResolutionStrategy.FORCE) { + if (yield * waitForResolutionConfirmation()) { + yield * deps.syncPersonalWorkspace(true); + return true; } + + return false; } - ); + + if (strategy === ResolutionStrategy.DISCARD_ALL) { + yield * discardAll(); + return true; + } + } + + return false; } return resolveConflicts; diff --git a/packages/neos-ui/src/Containers/Modals/PublishingDialog/PublishingDialog.tsx b/packages/neos-ui/src/Containers/Modals/PublishingDialog/PublishingDialog.tsx index b8844bfd4d..7997c5916f 100644 --- a/packages/neos-ui/src/Containers/Modals/PublishingDialog/PublishingDialog.tsx +++ b/packages/neos-ui/src/Containers/Modals/PublishingDialog/PublishingDialog.tsx @@ -96,6 +96,9 @@ const PublishingDialog: React.FC = (props) => { /> ); + case PublishingPhase.CONFLICTS: + return null; + case PublishingPhase.ERROR: case PublishingPhase.SUCCESS: return ( From f23176bef43459868f8d2d9a2ee298c7fe7c3c81 Mon Sep 17 00:00:00 2001 From: Wilhelm Behncke Date: Mon, 20 May 2024 17:44:18 +0200 Subject: [PATCH 08/36] BUGFIX: Remove race condition in `watchNodeInformationChanges` It may happen that `fetchAdditionalNodeMetadata` adds metadata for a node that has since been removed from the store. The occurance of such "zombie" nodes leads to problems, because they may lack crucial properties like `children` that are treated as always- present by other mechanisms throughout the UI. This became apparent after https://github.com/neos/neos-ui/pull/3756. The CR.Nodes.MERGE action now takes care of preventing zombie nodes from entering the store. --- packages/neos-ui-redux-store/src/CR/Nodes/index.ts | 10 ++++++++-- 1 file changed, 8 insertions(+), 2 deletions(-) diff --git a/packages/neos-ui-redux-store/src/CR/Nodes/index.ts b/packages/neos-ui-redux-store/src/CR/Nodes/index.ts index 97618f7d74..759e3387f8 100644 --- a/packages/neos-ui-redux-store/src/CR/Nodes/index.ts +++ b/packages/neos-ui-redux-store/src/CR/Nodes/index.ts @@ -450,10 +450,16 @@ export const reducer = (state: State = defaultState, action: InitAction | EditPr if (!newNode) { throw new Error('This error should never be thrown, it\'s a way to fool TypeScript'); } - const mergedNode = defaultsDeep({}, newNode, draft.byContextPath[contextPath]); - // Force overwrite of children + const oldNode = state.byContextPath[contextPath]; + const mergedNode = defaultsDeep({}, newNode, oldNode); if (newNode.children !== undefined) { + // Force overwrite of children mergedNode.children = newNode.children; + } else if (!oldNode) { + // newNode only adds meta info, but oldNode is gone from the store. + // In order to avoid zombie nodes occupying the store, we'll leave + // the node alone in this case. + return; } // Force overwrite of matchesCurrentDimensions if (newNode.matchesCurrentDimensions !== undefined) { From 4205f09ff4eee7ebdb38a57b1f8af77cd9e12203 Mon Sep 17 00:00:00 2001 From: Wilhelm Behncke Date: Tue, 21 May 2024 12:34:48 +0200 Subject: [PATCH 09/36] BUGFIX: Wait and reload before syncing in E2E test This is to prevent the flakiness described in #3785. --- Tests/IntegrationTests/Fixtures/1Dimension/syncing.e2e.js | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/Tests/IntegrationTests/Fixtures/1Dimension/syncing.e2e.js b/Tests/IntegrationTests/Fixtures/1Dimension/syncing.e2e.js index 5a148b4b52..90bede3dd5 100644 --- a/Tests/IntegrationTests/Fixtures/1Dimension/syncing.e2e.js +++ b/Tests/IntegrationTests/Fixtures/1Dimension/syncing.e2e.js @@ -69,6 +69,10 @@ async function prepareConflictBetweenAdminAndEditor(t) { // // Sync changes from "admin" // + await t.wait(2000); + await t.eval(() => location.reload(true)); + await waitForReact(30000); + await Page.waitForIframeLoading(); await t.click(Selector('#neos-workspace-rebase')); await t.click(Selector('#neos-SyncWorkspace-Confirm')); await t.wait(1000); From 98d002ad21577239904874bb758689c5f0c88d80 Mon Sep 17 00:00:00 2001 From: Wilhelm Behncke Date: Mon, 27 May 2024 14:55:53 +0200 Subject: [PATCH 10/36] TASK: Stabilize E2E tests for synving feature fixes: #3785 --- Tests/IntegrationTests/Fixtures/1Dimension/syncing.e2e.js | 6 +++++- Tests/IntegrationTests/pageModel.js | 1 + 2 files changed, 6 insertions(+), 1 deletion(-) diff --git a/Tests/IntegrationTests/Fixtures/1Dimension/syncing.e2e.js b/Tests/IntegrationTests/Fixtures/1Dimension/syncing.e2e.js index 90bede3dd5..618a1c140f 100644 --- a/Tests/IntegrationTests/Fixtures/1Dimension/syncing.e2e.js +++ b/Tests/IntegrationTests/Fixtures/1Dimension/syncing.e2e.js @@ -144,6 +144,10 @@ async function prepareConflictBetweenAdminAndEditor(t) { } async function switchToRole(t, role) { + // We need to add a time buffer here, otherwise `t.useRole` might interrupt + // some long-running background process, errororing like this: + // > Error: NetworkError when attempting to fetch resource. + await t.wait(2000); await t.useRole(role); await waitForReact(30000); await Page.goToPage('Home'); @@ -167,7 +171,7 @@ async function chooseDiscardAllAndFinishSynchronization(t) { }); // For reasons unknown, we have to press the acknowledge button really // hard for testcafe to realize our intent... - await t.wait(200); + await t.wait(500); await t.click(Selector('#neos-DiscardDialog-Acknowledge')); // diff --git a/Tests/IntegrationTests/pageModel.js b/Tests/IntegrationTests/pageModel.js index d9f6139272..91c84bad75 100644 --- a/Tests/IntegrationTests/pageModel.js +++ b/Tests/IntegrationTests/pageModel.js @@ -103,6 +103,7 @@ export class PublishDropDown { timeout: 30000 }); await t.click($acknowledgeBtn); + await t.wait(2000); } } From d8c93a0488aa4400080934481f0162529822f85b Mon Sep 17 00:00:00 2001 From: Wilhelm Behncke Date: Tue, 28 May 2024 11:35:42 +0200 Subject: [PATCH 11/36] BUGFIX: Fix undefined state when cancelling sync operations This adds an e2e test case for cancelling and reselecting a resolution strategy and another e2e test case for immediate resolution cancellation. Both test cases are fixed by changes to the syncing saga. --- .../Fixtures/1Dimension/syncing.e2e.js | 97 ++++++++++++------- packages/neos-ui-sagas/src/Sync/index.ts | 48 ++++----- 2 files changed, 86 insertions(+), 59 deletions(-) diff --git a/Tests/IntegrationTests/Fixtures/1Dimension/syncing.e2e.js b/Tests/IntegrationTests/Fixtures/1Dimension/syncing.e2e.js index 618a1c140f..13aaee6fe6 100644 --- a/Tests/IntegrationTests/Fixtures/1Dimension/syncing.e2e.js +++ b/Tests/IntegrationTests/Fixtures/1Dimension/syncing.e2e.js @@ -15,13 +15,38 @@ const contentIframeSelector = Selector('[name="neos-content-main"]', {timeout: 2 test('Syncing: Create a conflict state between two editors and choose "Discard all" as a resolution strategy during rebase', async t => { await prepareConflictBetweenAdminAndEditor(t); - await chooseDiscardAllAndFinishSynchronization(t); + await chooseDiscardAllAsResolutionStrategy(t); + await confirmAndPerformDiscardAll(t); + await finishSynchronization(t); await assertThatSynchronizationWasSuccessful(t); }); test('Syncing: Create a conflict state between two editors and choose "Drop conflicting changes" as a resolution strategy during rebase', async t => { await prepareConflictBetweenAdminAndEditor(t); - await chooseDropConflictingChangesAndFinishSynchronization(t); + await chooseDropConflictingChangesAsResolutionStrategy(t); + await confirmDropConflictingChanges(t); + await finishSynchronization(t); + await assertThatSynchronizationWasSuccessful(t); +}); + +test('Syncing: Create a conflict state between two editors, start and cancel resolution, then restart and choose "Drop conflicting changes" as a resolution strategy during rebase', async t => { + await prepareConflictBetweenAdminAndEditor(t); + await cancelResolutionDuringStrategyChoice(t); + await startSynchronization(t); + await assertThatConflictResolutionHasStarted(t); + await chooseDropConflictingChangesAsResolutionStrategy(t); + await confirmDropConflictingChanges(t); + await finishSynchronization(t); + await assertThatSynchronizationWasSuccessful(t); +}); + +test('Syncing: Create a conflict state between two editors and choose "Drop conflicting changes" as a resolution strategy, then cancel and choose "Discard all" as a resolution strategy during rebase', async t => { + await prepareConflictBetweenAdminAndEditor(t); + await chooseDropConflictingChangesAsResolutionStrategy(t); + await cancelDropConflictingChanges(t); + await chooseDiscardAllAsResolutionStrategy(t); + await confirmAndPerformDiscardAll(t); + await finishSynchronization(t); await assertThatSynchronizationWasSuccessful(t); }); @@ -73,8 +98,7 @@ async function prepareConflictBetweenAdminAndEditor(t) { await t.eval(() => location.reload(true)); await waitForReact(30000); await Page.waitForIframeLoading(); - await t.click(Selector('#neos-workspace-rebase')); - await t.click(Selector('#neos-SyncWorkspace-Confirm')); + await startSynchronization(t); await t.wait(1000); // @@ -135,12 +159,8 @@ async function prepareConflictBetweenAdminAndEditor(t) { // // Sync changes from "editor" // - await t.click(Selector('#neos-workspace-rebase')); - await t.click(Selector('#neos-SyncWorkspace-Confirm')); - await t.expect(Selector('#neos-SelectResolutionStrategy-SelectBox').exists) - .ok('Select box for resolution strategy slection is not available', { - timeout: 30000 - }); + await startSynchronization(t); + await assertThatConflictResolutionHasStarted(t); } async function switchToRole(t, role) { @@ -153,17 +173,22 @@ async function switchToRole(t, role) { await Page.goToPage('Home'); } -async function chooseDiscardAllAndFinishSynchronization(t) { - // - // Choose "Discard All" as resolution strategy - // +async function startSynchronization(t) { + await t.click(Selector('#neos-workspace-rebase')); + await t.click(Selector('#neos-SyncWorkspace-Confirm')); +} + +async function cancelResolutionDuringStrategyChoice(t) { + await t.click(Selector('#neos-SelectResolutionStrategy-Cancel')); +} + +async function chooseDiscardAllAsResolutionStrategy(t) { await t.click(Selector('#neos-SelectResolutionStrategy-SelectBox')); await t.click(Selector('[role="button"]').withText('Discard workspace "user-admin"')); await t.click(Selector('#neos-SelectResolutionStrategy-Accept')); +} - // - // Go through discard workflow - // +async function confirmAndPerformDiscardAll(t) { await t.click(Selector('#neos-DiscardDialog-Confirm')); await t.expect(Selector('#neos-DiscardDialog-Acknowledge').exists) .ok('Acknowledge button for "Discard all" is not available.', { @@ -173,35 +198,35 @@ async function chooseDiscardAllAndFinishSynchronization(t) { // hard for testcafe to realize our intent... await t.wait(500); await t.click(Selector('#neos-DiscardDialog-Acknowledge')); - - // - // Synchronization should restart automatically, - // so we must wait for it to succeed - // - await t.expect(Selector('#neos-SyncWorkspace-Acknowledge').exists) - .ok('Acknowledge button for "Sync Workspace" is not available.', { - timeout: 30000 - }); - await t.click(Selector('#neos-SyncWorkspace-Acknowledge')); } -async function chooseDropConflictingChangesAndFinishSynchronization(t) { - // - // Choose "Drop conflicting changes" as resolution strategy - // +async function chooseDropConflictingChangesAsResolutionStrategy(t) { await t.click(Selector('#neos-SelectResolutionStrategy-SelectBox')); await t.click(Selector('[role="button"]').withText('Drop conflicting changes')); await t.click(Selector('#neos-SelectResolutionStrategy-Accept')); +} - // - // Confirm the strategy - // +async function confirmDropConflictingChanges(t) { await t.click(Selector('#neos-ResolutionStrategyConfirmation-Confirm')); +} + +async function cancelDropConflictingChanges(t) { + await t.click(Selector('#neos-ResolutionStrategyConfirmation-Cancel')); +} + +async function finishSynchronization(t) { await t.expect(Selector('#neos-SyncWorkspace-Acknowledge').exists) - .ok('Acknowledge button for "Sync Workspace" is not available.', { + .ok('Acknowledge button for "Sync Workspace" is not available.', { + timeout: 30000 + }); + await t.click(Selector('#neos-SyncWorkspace-Acknowledge')); +} + +async function assertThatConflictResolutionHasStarted(t) { + await t.expect(Selector('#neos-SelectResolutionStrategy-SelectBox').exists) + .ok('Select box for resolution strategy slection is not available', { timeout: 30000 }); - await t.click(Selector('#neos-SyncWorkspace-Acknowledge')); } async function assertThatSynchronizationWasSuccessful(t) { diff --git a/packages/neos-ui-sagas/src/Sync/index.ts b/packages/neos-ui-sagas/src/Sync/index.ts index 7f8287d39b..a6f2fca4ab 100644 --- a/packages/neos-ui-sagas/src/Sync/index.ts +++ b/packages/neos-ui-sagas/src/Sync/index.ts @@ -95,35 +95,37 @@ export const makeResolveConflicts = (deps: { const discardAll = makeDiscardAll(deps); function * resolveConflicts(conflicts: Conflict[]): any { - yield put(actions.CR.Syncing.resolve(conflicts)); - - const {started}: { - cancelled: null | ReturnType; - started: null | ReturnType; - } = yield race({ - cancelled: take(actionTypes.CR.Syncing.CANCELLED), - started: take(actionTypes.CR.Syncing.RESOLUTION_STARTED) - }); - - if (started) { - const {payload: {strategy}} = started; + while (true) { + yield put(actions.CR.Syncing.resolve(conflicts)); + + const {started}: { + cancelled: null | ReturnType; + started: null | ReturnType; + } = yield race({ + cancelled: take(actionTypes.CR.Syncing.CANCELLED), + started: take(actionTypes.CR.Syncing.RESOLUTION_STARTED) + }); + + if (started) { + const {payload: {strategy}} = started; + + if (strategy === ResolutionStrategy.FORCE) { + if (yield * waitForResolutionConfirmation()) { + yield * deps.syncPersonalWorkspace(true); + return true; + } + + continue; + } - if (strategy === ResolutionStrategy.FORCE) { - if (yield * waitForResolutionConfirmation()) { - yield * deps.syncPersonalWorkspace(true); + if (strategy === ResolutionStrategy.DISCARD_ALL) { + yield * discardAll(); return true; } - - return false; } - if (strategy === ResolutionStrategy.DISCARD_ALL) { - yield * discardAll(); - return true; - } + return false; } - - return false; } return resolveConflicts; From f2526fde5f79512ff3cd2beb9c5827dafc0f9323 Mon Sep 17 00:00:00 2001 From: Wilhelm Behncke Date: Tue, 28 May 2024 14:47:04 +0200 Subject: [PATCH 12/36] TASK: Handle edge case when automatic syncing during "publish document" removes the document This also adds two more E2E test cases for publishing with automatic syncing. --- .../Fixtures/1Dimension/syncing.e2e.js | 352 ++++++++++++------ packages/neos-ui-sagas/src/Publish/index.ts | 18 +- 2 files changed, 259 insertions(+), 111 deletions(-) diff --git a/Tests/IntegrationTests/Fixtures/1Dimension/syncing.e2e.js b/Tests/IntegrationTests/Fixtures/1Dimension/syncing.e2e.js index 13aaee6fe6..0a591b2b70 100644 --- a/Tests/IntegrationTests/Fixtures/1Dimension/syncing.e2e.js +++ b/Tests/IntegrationTests/Fixtures/1Dimension/syncing.e2e.js @@ -14,153 +14,240 @@ fixture`Syncing` const contentIframeSelector = Selector('[name="neos-content-main"]', {timeout: 2000}); test('Syncing: Create a conflict state between two editors and choose "Discard all" as a resolution strategy during rebase', async t => { - await prepareConflictBetweenAdminAndEditor(t); + await prepareContentElementConflictBetweenAdminAndEditor(t); await chooseDiscardAllAsResolutionStrategy(t); await confirmAndPerformDiscardAll(t); await finishSynchronization(t); - await assertThatSynchronizationWasSuccessful(t); + + await assertThatWeAreOnPage(t, 'Home'); + await assertThatWeCannotSeePageInTree(t, 'Sync Demo #1'); + await assertThatWeCannotSeePageInTree(t, 'Sync Demo #2'); + await assertThatWeCannotSeePageInTree(t, 'Sync Demo #3'); }); test('Syncing: Create a conflict state between two editors and choose "Drop conflicting changes" as a resolution strategy during rebase', async t => { - await prepareConflictBetweenAdminAndEditor(t); + await prepareContentElementConflictBetweenAdminAndEditor(t); await chooseDropConflictingChangesAsResolutionStrategy(t); await confirmDropConflictingChanges(t); await finishSynchronization(t); - await assertThatSynchronizationWasSuccessful(t); + + await assertThatWeAreOnPage(t, 'Home'); + await assertThatWeCannotSeePageInTree(t, 'Sync Demo #1'); + await assertThatWeCannotSeePageInTree(t, 'Sync Demo #2'); + await assertThatWeCannotSeePageInTree(t, 'Sync Demo #3'); }); test('Syncing: Create a conflict state between two editors, start and cancel resolution, then restart and choose "Drop conflicting changes" as a resolution strategy during rebase', async t => { - await prepareConflictBetweenAdminAndEditor(t); + await prepareContentElementConflictBetweenAdminAndEditor(t); await cancelResolutionDuringStrategyChoice(t); await startSynchronization(t); await assertThatConflictResolutionHasStarted(t); await chooseDropConflictingChangesAsResolutionStrategy(t); await confirmDropConflictingChanges(t); await finishSynchronization(t); - await assertThatSynchronizationWasSuccessful(t); + + await assertThatWeAreOnPage(t, 'Home'); + await assertThatWeCannotSeePageInTree(t, 'Sync Demo #1'); + await assertThatWeCannotSeePageInTree(t, 'Sync Demo #2'); + await assertThatWeCannotSeePageInTree(t, 'Sync Demo #3'); }); test('Syncing: Create a conflict state between two editors and choose "Drop conflicting changes" as a resolution strategy, then cancel and choose "Discard all" as a resolution strategy during rebase', async t => { - await prepareConflictBetweenAdminAndEditor(t); + await prepareContentElementConflictBetweenAdminAndEditor(t); await chooseDropConflictingChangesAsResolutionStrategy(t); await cancelDropConflictingChanges(t); await chooseDiscardAllAsResolutionStrategy(t); await confirmAndPerformDiscardAll(t); await finishSynchronization(t); - await assertThatSynchronizationWasSuccessful(t); + + await assertThatWeAreOnPage(t, 'Home'); + await assertThatWeCannotSeePageInTree(t, 'Sync Demo #1'); + await assertThatWeCannotSeePageInTree(t, 'Sync Demo #2'); + await assertThatWeCannotSeePageInTree(t, 'Sync Demo #3'); }); -async function prepareConflictBetweenAdminAndEditor(t) { - // - // Login as "editor" once, to initialize a content stream for their workspace - // in case there isn't one already - // - await switchToRole(t, editorUserOnOneDimensionTestSite); - await Page.waitForIframeLoading(); - await t.wait(2000); +test('Publish + Syncing: Create a conflict state between two editors, then try to publish and choose "Drop conflicting changes" as a resolution strategy during automatic rebase', async t => { + await prepareDocumentConflictBetweenAdminAndEditor(t); + await startPublishAll(t); + await assertThatConflictResolutionHasStarted(t); + await chooseDropConflictingChangesAsResolutionStrategy(t); + await confirmDropConflictingChanges(t); + await finishPublish(t); + + await assertThatWeAreOnPage(t, 'Home'); + await assertThatWeCannotSeePageInTree(t, 'This page will be deleted during sync'); +}); + +test('Publish + Syncing: Create a conflict state between two editors, then try to publish the document only and choose "Drop conflicting changes" as a resolution strategy during automatic rebase', async t => { + await prepareDocumentConflictBetweenAdminAndEditor(t); + await startPublishDocument(t); + await assertThatConflictResolutionHasStarted(t); + await chooseDropConflictingChangesAsResolutionStrategy(t); + await confirmDropConflictingChanges(t); + await finishPublish(t); + + await assertThatWeAreOnPage(t, 'Home'); + await assertThatWeCannotSeePageInTree(t, 'This page will be deleted during sync'); +}); + +async function prepareContentElementConflictBetweenAdminAndEditor(t) { + await loginAsEditorOnceToInitializeAContentStreamForTheirWorkspaceIfNeeded(t); // // Login as "admin" // - await switchToRole(t, adminUserOnOneDimensionTestSite); - await PublishDropDown.discardAll(); + await as(t, adminUserOnOneDimensionTestSite, async () => { + await PublishDropDown.discardAll(); - // - // Create a hierarchy of document nodes - // - async function createDocumentNode(pageTitleToCreate) { - await t - .click(Selector('#neos-PageTree-AddNode')) - .click(ReactSelector('InsertModeSelector').find('#into')) - .click(ReactSelector('NodeTypeItem').find('button>span>span').withText('Page_Test')) - .typeText(Selector('#neos-NodeCreationDialog-Body input'), pageTitleToCreate) - .click(Selector('#neos-NodeCreationDialog-CreateNew')); - await Page.waitForIframeLoading(); - } - await createDocumentNode('Sync Demo #1'); - await createDocumentNode('Sync Demo #2'); - await createDocumentNode('Sync Demo #3'); + // + // Create a hierarchy of document nodes + // + await createDocumentNode(t, 'Home', 'into', 'Sync Demo #1'); + await createDocumentNode(t, 'Sync Demo #1', 'into', 'Sync Demo #2'); + await createDocumentNode(t, 'Sync Demo #2', 'into', 'Sync Demo #3'); - // - // Publish everything - // - await PublishDropDown.publishAll(); + // + // Publish everything + // + await PublishDropDown.publishAll(); + }); // // Login as "editor" // - await switchToRole(t, editorUserOnOneDimensionTestSite); - - // - // Sync changes from "admin" - // - await t.wait(2000); - await t.eval(() => location.reload(true)); - await waitForReact(30000); - await Page.waitForIframeLoading(); - await startSynchronization(t); - await t.wait(1000); + await as(t, editorUserOnOneDimensionTestSite, async () => { + // + // Sync changes from "admin" + // + await t.wait(2000); + await t.eval(() => location.reload(true)); + await waitForReact(30000); + await Page.waitForIframeLoading(); + await startSynchronization(t); + await t.wait(1000); + + // + // Assert that all 3 documents are now visible in the document tree + // + await t.expect(Page.treeNode.withExactText('Sync Demo #1').exists) + .ok('[🗋 Sync Demo #1] cannot be found in the document tree of user "editor".'); + await t.expect(Page.treeNode.withExactText('Sync Demo #2').exists) + .ok('[🗋 Sync Demo #2] cannot be found in the document tree of user "editor".'); + await t.expect(Page.treeNode.withExactText('Sync Demo #3').exists) + .ok('[🗋 Sync Demo #3] cannot be found in the document tree of user "editor".'); + }); - // - // Assert that all 3 documents are now visible in the document tree - // - await t.expect(Page.treeNode.withExactText('Sync Demo #1').exists) - .ok('[🗋 Sync Demo #1] cannot be found in the document tree of user "editor".'); - await t.expect(Page.treeNode.withExactText('Sync Demo #2').exists) - .ok('[🗋 Sync Demo #2] cannot be found in the document tree of user "editor".'); - await t.expect(Page.treeNode.withExactText('Sync Demo #3').exists) - .ok('[🗋 Sync Demo #3] cannot be found in the document tree of user "editor".'); // // Login as "admin" again // - await switchToRole(t, adminUserOnOneDimensionTestSite); + await as(t, adminUserOnOneDimensionTestSite, async () => { + // + // Create a headline node in [🗋 Sync Demo #3] + // + await Page.goToPage('Sync Demo #3'); + await t + .switchToIframe(contentIframeSelector) + .click(Selector('.neos-contentcollection')) + .click(Selector('#neos-InlineToolbar-AddNode')) + .switchToMainWindow() + .click(Selector('button#into')) + .click(ReactSelector('NodeTypeItem').withProps({nodeType: {label: 'Headline_Test'}})) + .switchToIframe(contentIframeSelector) + .typeText(Selector('.test-headline h1'), 'Hello from Page "Sync Demo #3"!') + .wait(2000) + .switchToMainWindow(); + }); - // - // Create a headline node in [🗋 Sync Demo #3] - // - await Page.goToPage('Sync Demo #3'); - await t - .switchToIframe(contentIframeSelector) - .click(Selector('.neos-contentcollection')) - .click(Selector('#neos-InlineToolbar-AddNode')) - .switchToMainWindow() - .click(Selector('button#into')) - .click(ReactSelector('NodeTypeItem').withProps({nodeType: {label: 'Headline_Test'}})) - .switchToIframe(contentIframeSelector) - .typeText(Selector('.test-headline h1'), 'Hello from Page "Sync Demo #3"!') - .wait(2000) - .switchToMainWindow(); // // Login as "editor" again // - await switchToRole(t, editorUserOnOneDimensionTestSite); + await as(t, editorUserOnOneDimensionTestSite, async () => { + // + // Delete page [🗋 Sync Demo #1] + // + await deleteDocumentNode(t, 'Sync Demo #1'); - // - // Delete page [🗋 Sync Demo #1] - // - await Page.goToPage('Sync Demo #1'); - await t.click(Selector('#neos-PageTree-DeleteSelectedNode')); - await t.click(Selector('#neos-DeleteNodeModal-Confirm')); - await Page.waitForIframeLoading(); + // + // Publish everything + // + await PublishDropDown.publishAll(); + }); - // - // Publish everything - // - await PublishDropDown.publishAll(); // // Login as "admin" again and visit [🗋 Sync Demo #3] // + await as(t, adminUserOnOneDimensionTestSite, async () => { + await Page.goToPage('Sync Demo #3'); + + // + // Sync changes from "editor" + // + await startSynchronization(t); + await assertThatConflictResolutionHasStarted(t); + }); +} + +async function prepareDocumentConflictBetweenAdminAndEditor(t) { + await loginAsEditorOnceToInitializeAContentStreamForTheirWorkspaceIfNeeded(t); + + await as(t, adminUserOnOneDimensionTestSite, async () => { + await PublishDropDown.discardAll(); + await createDocumentNode(t, 'Home', 'into', 'This page will be deleted during sync'); + await PublishDropDown.publishAll(); + + await t + .switchToIframe(contentIframeSelector) + .click(Selector('.neos-contentcollection')) + .click(Selector('#neos-InlineToolbar-AddNode')) + .switchToMainWindow() + .click(Selector('button#into')) + .click(ReactSelector('NodeTypeItem').withProps({nodeType: {label: 'Headline_Test'}})) + .switchToIframe(contentIframeSelector) + .doubleClick(Selector('.test-headline h1')) + .typeText(Selector('.test-headline h1'), 'This change will not be published.') + .wait(2000) + .switchToMainWindow(); + }); + + await as(t, editorUserOnOneDimensionTestSite, async () => { + await t.wait(2000); + await t.eval(() => location.reload(true)); + await waitForReact(30000); + await Page.waitForIframeLoading(); + await startSynchronization(t); + await t.wait(1000); + await finishSynchronization(t); + + await t.expect(Page.treeNode.withExactText('This page will be deleted during sync').exists) + .ok('[🗋 This page will be deleted during sync] cannot be found in the document tree of user "editor".'); + + await deleteDocumentNode(t, 'This page will be deleted during sync'); + await PublishDropDown.publishAll(); + }); + await switchToRole(t, adminUserOnOneDimensionTestSite); - await Page.goToPage('Sync Demo #3'); + await Page.goToPage('This page will be deleted during sync'); +} - // - // Sync changes from "editor" - // - await startSynchronization(t); - await assertThatConflictResolutionHasStarted(t); +let editHasLoggedInAtLeastOnce = false; +async function loginAsEditorOnceToInitializeAContentStreamForTheirWorkspaceIfNeeded(t) { + if (editHasLoggedInAtLeastOnce) { + return; + } + + await as(t, editorUserOnOneDimensionTestSite, async () => { + await Page.waitForIframeLoading(); + await t.wait(2000); + editHasLoggedInAtLeastOnce = true; + }); +} + +async function as(t, role, asyncCallback) { + await switchToRole(t, role); + await asyncCallback(); } async function switchToRole(t, role) { @@ -173,6 +260,41 @@ async function switchToRole(t, role) { await Page.goToPage('Home'); } +async function createDocumentNode(t, referencePageTitle, insertMode, pageTitleToCreate) { + await Page.goToPage(referencePageTitle); + await t + .click(Selector('#neos-PageTree-AddNode')) + .click(ReactSelector('InsertModeSelector').find('#' + insertMode)) + .click(ReactSelector('NodeTypeItem').find('button>span>span').withText('Page_Test')) + .typeText(Selector('#neos-NodeCreationDialog-Body input'), pageTitleToCreate) + .click(Selector('#neos-NodeCreationDialog-CreateNew')); + await Page.waitForIframeLoading(); +} + +async function deleteDocumentNode(t, pageTitleToDelete) { + await Page.goToPage(pageTitleToDelete); + await t.click(Selector('#neos-PageTree-DeleteSelectedNode')); + await t.click(Selector('#neos-DeleteNodeModal-Confirm')); + await Page.waitForIframeLoading(); +} + +async function startPublishAll(t) { + await t.click(PublishDropDown.publishDropdown) + await t.click(PublishDropDown.publishDropdownPublishAll); + await t.click(Selector('#neos-PublishDialog-Confirm')); +} + +async function startPublishDocument(t) { + await t.click(Selector('#neos-PublishDropDown-Publish')) + await t.click(Selector('#neos-PublishDialog-Confirm')); +} + +async function finishPublish(t) { + await assertThatPublishingHasFinishedWithoutError(t); + await t.click(Selector('#neos-PublishDialog-Acknowledge')); + await t.wait(2000); +} + async function startSynchronization(t) { await t.click(Selector('#neos-workspace-rebase')); await t.click(Selector('#neos-SyncWorkspace-Confirm')); @@ -215,10 +337,7 @@ async function cancelDropConflictingChanges(t) { } async function finishSynchronization(t) { - await t.expect(Selector('#neos-SyncWorkspace-Acknowledge').exists) - .ok('Acknowledge button for "Sync Workspace" is not available.', { - timeout: 30000 - }); + await assertThatSynchronizationHasFinishedWithoutError(t); await t.click(Selector('#neos-SyncWorkspace-Acknowledge')); } @@ -229,22 +348,35 @@ async function assertThatConflictResolutionHasStarted(t) { }); } -async function assertThatSynchronizationWasSuccessful(t) { - // - // Assert that we have been redirected to the home page by checking if - // the currently focused document tree node is "Home". - // +async function assertThatSynchronizationHasFinishedWithoutError(t) { + await t.expect(Selector('#neos-SyncWorkspace-Acknowledge').exists) + .ok('Acknowledge button for "Sync Workspace" is not available.', { + timeout: 30000 + }); + await t.expect(Selector('#neos-SyncWorkspace-Retry').exists) + .notOk('An error occurred during "Sync Workspace".', { + timeout: 30000 + }); +} + +async function assertThatPublishingHasFinishedWithoutError(t) { + await t.expect(Selector('#neos-PublishDialog-Acknowledge').exists) + .ok('Acknowledge button for "Publishing" is not available.', { + timeout: 30000 + }); + await t.expect(Selector('#neos-PublishDialog-Retry').exists) + .notOk('An error occurred during "Publishing".', { + timeout: 30000 + }); +} + +async function assertThatWeAreOnPage(t, pageTitle) { await t .expect(Selector('[role="treeitem"] [role="button"][class*="isFocused"]').textContent) - .eql('Home'); + .eql(pageTitle); +} - // - // Assert that all 3 documents are not visible anymore in the document tree - // - await t.expect(Page.treeNode.withExactText('Sync Demo #1').exists) - .notOk('[🗋 Sync Demo #1] can still be found in the document tree of user "admin".'); - await t.expect(Page.treeNode.withExactText('Sync Demo #2').exists) - .notOk('[🗋 Sync Demo #2] can still be found in the document tree of user "admin".'); - await t.expect(Page.treeNode.withExactText('Sync Demo #3').exists) - .notOk('[🗋 Sync Demo #3] can still be found in the document tree of user "admin".'); +async function assertThatWeCannotSeePageInTree(t, pageTitle) { + await t.expect(Page.treeNode.withExactText(pageTitle).exists) + .notOk(`[🗋 ${pageTitle}] can still be found in the document tree of user "admin".`); } diff --git a/packages/neos-ui-sagas/src/Publish/index.ts b/packages/neos-ui-sagas/src/Publish/index.ts index ceb0673d5e..df2c098ee4 100644 --- a/packages/neos-ui-sagas/src/Publish/index.ts +++ b/packages/neos-ui-sagas/src/Publish/index.ts @@ -108,7 +108,23 @@ export function * watchPublishing({routes}: {routes: Routes}) { if (conflictsWereResolved) { yield put(actions.CR.Publishing.resolveConflicts()); - yield * attemptToPublishOrDiscard(); + + // + // It may happen that after conflicts are resolved, the + // document we're trying to publish no longer exists. + // + // We need to finish the publishing operation in this + // case, otherwise it'll lead to an error. + // + const publishingShouldContinue = scope === PublishingScope.DOCUMENT + ? Boolean(yield select(selectors.CR.Nodes.byContextPathSelector(ancestorId))) + : true; + + if (publishingShouldContinue) { + yield * attemptToPublishOrDiscard(); + } else { + yield put(actions.CR.Publishing.succeed(0)); + } } else { yield put(actions.CR.Publishing.cancel()); yield call(updateWorkspaceInfo); From b562be570d2a3f13d725536788376da3cdfec16d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Christian=20Mu=CC=88ller?= Date: Fri, 30 Aug 2024 23:59:35 +0200 Subject: [PATCH 13/36] Adjustments to new Reference DTOs --- Classes/Domain/Model/Changes/Property.php | 3 +-- .../CreationDialog/PromotedElementsCreationHandlerFactory.php | 3 +-- 2 files changed, 2 insertions(+), 4 deletions(-) diff --git a/Classes/Domain/Model/Changes/Property.php b/Classes/Domain/Model/Changes/Property.php index dbff01dffc..5a5ca3748f 100644 --- a/Classes/Domain/Model/Changes/Property.php +++ b/Classes/Domain/Model/Changes/Property.php @@ -239,8 +239,7 @@ private function handleNodeReferenceChange(Node $subject, string $propertyName): $subject->workspaceName, $subject->aggregateId, $subject->originDimensionSpacePoint, - ReferenceName::fromString($propertyName), - NodeReferencesToWrite::fromNodeAggregateIds(NodeAggregateIds::fromArray($destinationNodeAggregateIds)) + NodeReferencesToWrite::fromReferenceNameAndNodeAggregateIds(ReferenceName::fromString($propertyName), NodeAggregateIds::fromArray($destinationNodeAggregateIds)) ) ); } diff --git a/Classes/Infrastructure/ContentRepository/CreationDialog/PromotedElementsCreationHandlerFactory.php b/Classes/Infrastructure/ContentRepository/CreationDialog/PromotedElementsCreationHandlerFactory.php index 88066dab9e..f20cbf5818 100644 --- a/Classes/Infrastructure/ContentRepository/CreationDialog/PromotedElementsCreationHandlerFactory.php +++ b/Classes/Infrastructure/ContentRepository/CreationDialog/PromotedElementsCreationHandlerFactory.php @@ -64,8 +64,7 @@ public function handle(NodeCreationCommands $commands, NodeCreationElements $ele $commands->first->workspaceName, $commands->first->nodeAggregateId, $commands->first->originDimensionSpacePoint, - ReferenceName::fromString($elementName), - NodeReferencesToWrite::fromNodeAggregateIds($elementValue) + NodeReferencesToWrite::fromReferenceNameAndNodeAggregateIds(ReferenceName::fromString($elementName), $elementValue) ); } } From c7c160f56131729fa8a1c5e11b58c319526c879f Mon Sep 17 00:00:00 2001 From: mhsdesign <85400359+mhsdesign@users.noreply.github.com> Date: Mon, 30 Sep 2024 17:52:54 +0200 Subject: [PATCH 14/36] TASK: Fix php code --- Classes/Infrastructure/ContentRepository/ConflictsFactory.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Classes/Infrastructure/ContentRepository/ConflictsFactory.php b/Classes/Infrastructure/ContentRepository/ConflictsFactory.php index b51d6b0d95..46b6a4283d 100644 --- a/Classes/Infrastructure/ContentRepository/ConflictsFactory.php +++ b/Classes/Infrastructure/ContentRepository/ConflictsFactory.php @@ -115,7 +115,7 @@ private function createConflictFromCommandThatFailedDuringRebase( return new Conflict( key: $affectedNode - ? $affectedNode->nodeAggregateId->value + ? $affectedNode->aggregateId->value : 'command-' . $commandThatFailedDuringRebase->sequenceNumber, affectedSite: $affectedSite ? $this->createIconLabelForNode($affectedSite) From 64ac4bc73221f19b80ed833027312a9a01953658 Mon Sep 17 00:00:00 2001 From: mhsdesign <85400359+mhsdesign@users.noreply.github.com> Date: Thu, 17 Oct 2024 13:50:42 +0200 Subject: [PATCH 15/36] TASK: Adjust to changes in 9.0 (workspace metadata) --- .../PublishChangesInDocumentCommandHandler.php | 14 +++++++++----- .../PublishChangesInSiteCommandHandler.php | 14 +++++++++----- .../SyncWorkspace/SyncWorkspaceCommandHandler.php | 1 + .../ContentRepository/ConflictsFactory.php | 5 ++--- 4 files changed, 21 insertions(+), 13 deletions(-) diff --git a/Classes/Application/PublishChangesInDocument/PublishChangesInDocumentCommandHandler.php b/Classes/Application/PublishChangesInDocument/PublishChangesInDocumentCommandHandler.php index 9e89d1993b..d090fdb815 100644 --- a/Classes/Application/PublishChangesInDocument/PublishChangesInDocumentCommandHandler.php +++ b/Classes/Application/PublishChangesInDocument/PublishChangesInDocumentCommandHandler.php @@ -20,7 +20,7 @@ use Neos\ContentRepositoryRegistry\ContentRepositoryRegistry; use Neos\Flow\Annotations as Flow; use Neos\Neos\Domain\NodeLabel\NodeLabelGeneratorInterface; -use Neos\Neos\Domain\Workspace\WorkspaceProvider; +use Neos\Neos\Domain\Service\WorkspacePublishingService; use Neos\Neos\Ui\Application\Shared\ConflictsOccurred; use Neos\Neos\Ui\Application\Shared\PublishSucceeded; use Neos\Neos\Ui\Controller\TranslationTrait; @@ -41,7 +41,7 @@ final class PublishChangesInDocumentCommandHandler protected ContentRepositoryRegistry $contentRepositoryRegistry; #[Flow\Inject] - protected WorkspaceProvider $workspaceProvider; + protected WorkspacePublishingService $workspacePublishingService; #[Flow\Inject] protected NodeLabelGeneratorInterface $nodeLabelGenerator; @@ -53,15 +53,19 @@ public function handle( PublishChangesInDocumentCommand $command ): PublishSucceeded|ConflictsOccurred { try { - $workspace = $this->workspaceProvider->provideForWorkspaceName( + $publishingResult = $this->workspacePublishingService->publishChangesInDocument( $command->contentRepositoryId, + $command->workspaceName, + $command->documentId + ); + + $workspace = $this->contentRepositoryRegistry->get($command->contentRepositoryId)->findWorkspaceByName( $command->workspaceName ); - $publishingResult = $workspace->publishChangesInDocument($command->documentId); return new PublishSucceeded( numberOfAffectedChanges: $publishingResult->numberOfPublishedChanges, - baseWorkspaceName: $workspace->getCurrentBaseWorkspaceName()?->value + baseWorkspaceName: $workspace?->baseWorkspaceName?->value ); } catch (NodeAggregateCurrentlyDoesNotExist $e) { throw new \RuntimeException( diff --git a/Classes/Application/PublishChangesInSite/PublishChangesInSiteCommandHandler.php b/Classes/Application/PublishChangesInSite/PublishChangesInSiteCommandHandler.php index b7d8a2e792..2326a5cadb 100644 --- a/Classes/Application/PublishChangesInSite/PublishChangesInSiteCommandHandler.php +++ b/Classes/Application/PublishChangesInSite/PublishChangesInSiteCommandHandler.php @@ -18,7 +18,7 @@ use Neos\ContentRepositoryRegistry\ContentRepositoryRegistry; use Neos\Flow\Annotations as Flow; use Neos\Neos\Domain\NodeLabel\NodeLabelGeneratorInterface; -use Neos\Neos\Domain\Workspace\WorkspaceProvider; +use Neos\Neos\Domain\Service\WorkspacePublishingService; use Neos\Neos\Ui\Application\Shared\ConflictsOccurred; use Neos\Neos\Ui\Application\Shared\PublishSucceeded; use Neos\Neos\Ui\Infrastructure\ContentRepository\ConflictsFactory; @@ -36,7 +36,7 @@ final class PublishChangesInSiteCommandHandler protected ContentRepositoryRegistry $contentRepositoryRegistry; #[Flow\Inject] - protected WorkspaceProvider $workspaceProvider; + protected WorkspacePublishingService $workspacePublishingService; #[Flow\Inject] protected NodeLabelGeneratorInterface $nodeLabelGenerator; @@ -45,15 +45,19 @@ public function handle( PublishChangesInSiteCommand $command ): PublishSucceeded|ConflictsOccurred { try { - $workspace = $this->workspaceProvider->provideForWorkspaceName( + $publishingResult = $this->workspacePublishingService->publishChangesInSite( $command->contentRepositoryId, + $command->workspaceName, + $command->siteId + ); + + $workspace = $this->contentRepositoryRegistry->get($command->contentRepositoryId)->findWorkspaceByName( $command->workspaceName ); - $publishingResult = $workspace->publishChangesInSite($command->siteId); return new PublishSucceeded( numberOfAffectedChanges: $publishingResult->numberOfPublishedChanges, - baseWorkspaceName: $workspace->getCurrentBaseWorkspaceName()?->value + baseWorkspaceName: $workspace?->baseWorkspaceName?->value ); } catch (WorkspaceRebaseFailed $e) { $conflictsFactory = new ConflictsFactory( diff --git a/Classes/Application/SyncWorkspace/SyncWorkspaceCommandHandler.php b/Classes/Application/SyncWorkspace/SyncWorkspaceCommandHandler.php index 310ebdcfa6..3539ee95d7 100644 --- a/Classes/Application/SyncWorkspace/SyncWorkspaceCommandHandler.php +++ b/Classes/Application/SyncWorkspace/SyncWorkspaceCommandHandler.php @@ -20,6 +20,7 @@ use Neos\Neos\Domain\NodeLabel\NodeLabelGeneratorInterface; use Neos\Neos\Domain\Service\WorkspacePublishingService; use Neos\Neos\Ui\Application\Shared\ConflictsOccurred; +use Neos\Neos\Ui\Infrastructure\ContentRepository\ConflictsFactory; /** * The application layer level command handler to for rebasing the workspace diff --git a/Classes/Infrastructure/ContentRepository/ConflictsFactory.php b/Classes/Infrastructure/ContentRepository/ConflictsFactory.php index 46b6a4283d..fb688ef00e 100644 --- a/Classes/Infrastructure/ContentRepository/ConflictsFactory.php +++ b/Classes/Infrastructure/ContentRepository/ConflictsFactory.php @@ -39,9 +39,9 @@ use Neos\ContentRepository\Core\Projection\ContentGraph\Node; use Neos\ContentRepository\Core\Projection\ContentGraph\NodeAggregate; use Neos\ContentRepository\Core\Projection\ContentGraph\VisibilityConstraints; -use Neos\ContentRepository\Core\Projection\Workspace\Workspace; use Neos\ContentRepository\Core\SharedModel\Exception\NodeAggregateCurrentlyDoesNotExist; use Neos\ContentRepository\Core\SharedModel\Node\NodeAggregateId; +use Neos\ContentRepository\Core\SharedModel\Workspace\Workspace; use Neos\ContentRepository\Core\SharedModel\Workspace\WorkspaceName; use Neos\Flow\Annotations as Flow; use Neos\Neos\Domain\NodeLabel\NodeLabelGeneratorInterface; @@ -70,8 +70,7 @@ public function __construct( ) { $this->nodeTypeManager = $contentRepository->getNodeTypeManager(); - $this->workspace = $contentRepository->getWorkspaceFinder() - ->findOneByName($workspaceName); + $this->workspace = $contentRepository->findWorkspaceByName($workspaceName); } public function fromWorkspaceRebaseFailed( From 9c70968b656028cb2fef89d72c8031552a669728 Mon Sep 17 00:00:00 2001 From: mhsdesign <85400359+mhsdesign@users.noreply.github.com> Date: Thu, 17 Oct 2024 15:00:16 +0200 Subject: [PATCH 16/36] TASK: Dare to unskip tests again:) --- Tests/IntegrationTests/Fixtures/1Dimension/syncing.e2e.js | 2 -- 1 file changed, 2 deletions(-) diff --git a/Tests/IntegrationTests/Fixtures/1Dimension/syncing.e2e.js b/Tests/IntegrationTests/Fixtures/1Dimension/syncing.e2e.js index 429e381490..d6eacfb783 100644 --- a/Tests/IntegrationTests/Fixtures/1Dimension/syncing.e2e.js +++ b/Tests/IntegrationTests/Fixtures/1Dimension/syncing.e2e.js @@ -11,8 +11,6 @@ import { fixture`Syncing` .afterEach(() => checkPropTypes()); -fixture.skip`TODO Tests are flaky and create catchup errors rendering following tests also kaput: https://github.com/neos/neos-ui/pull/3769#pullrequestreview-2332466270`; - const contentIframeSelector = Selector('[name="neos-content-main"]', {timeout: 2000}); test('Syncing: Create a conflict state between two editors and choose "Discard all" as a resolution strategy during rebase', async t => { From 0448005971a2438e3767f87f89cbea4eac3ab60f Mon Sep 17 00:00:00 2001 From: Sebastian Kurfuerst Date: Mon, 21 Oct 2024 13:12:07 +0200 Subject: [PATCH 17/36] TASK: further fix e2e test cases for Neos 9 --- .../docker-compose.neos-dev-instance.yaml | 3 +++ Tests/IntegrationTests/pageModel.js | 2 +- Tests/IntegrationTests/start-neos-dev-instance.sh | 9 +++++++-- .../DimensionSwitcher/DimensionSelector.js | 2 +- 4 files changed, 12 insertions(+), 4 deletions(-) diff --git a/Tests/IntegrationTests/docker-compose.neos-dev-instance.yaml b/Tests/IntegrationTests/docker-compose.neos-dev-instance.yaml index b0dd656f7f..fa822f2292 100644 --- a/Tests/IntegrationTests/docker-compose.neos-dev-instance.yaml +++ b/Tests/IntegrationTests/docker-compose.neos-dev-instance.yaml @@ -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: diff --git a/Tests/IntegrationTests/pageModel.js b/Tests/IntegrationTests/pageModel.js index 1a3c31e6a5..c018ad505f 100644 --- a/Tests/IntegrationTests/pageModel.js +++ b/Tests/IntegrationTests/pageModel.js @@ -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); diff --git a/Tests/IntegrationTests/start-neos-dev-instance.sh b/Tests/IntegrationTests/start-neos-dev-instance.sh index c018e135a8..0733f3f266 100644 --- a/Tests/IntegrationTests/start-neos-dev-instance.sh +++ b/Tests/IntegrationTests/start-neos-dev-instance.sh @@ -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 "" @@ -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 @@ -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 diff --git a/packages/neos-ui/src/Containers/PrimaryToolbar/DimensionSwitcher/DimensionSelector.js b/packages/neos-ui/src/Containers/PrimaryToolbar/DimensionSwitcher/DimensionSelector.js index f2be3beb4a..6a2360ca40 100644 --- a/packages/neos-ui/src/Containers/PrimaryToolbar/DimensionSwitcher/DimensionSelector.js +++ b/packages/neos-ui/src/Containers/PrimaryToolbar/DimensionSwitcher/DimensionSelector.js @@ -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, From b6673a770b67d484717248ff2455d6635a0e8bb4 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Christian=20Mu=CC=88ller?= Date: Wed, 23 Oct 2024 12:32:22 +0200 Subject: [PATCH 18/36] Update to latest references commits --- Classes/Domain/Model/Changes/Property.php | 2 +- .../NodeCreation/NodeCreationCommands.php | 9 ++++++++ ...PromotedElementsCreationHandlerFactory.php | 23 ++++++++----------- 3 files changed, 19 insertions(+), 15 deletions(-) diff --git a/Classes/Domain/Model/Changes/Property.php b/Classes/Domain/Model/Changes/Property.php index 5a5ca3748f..93ece455dc 100644 --- a/Classes/Domain/Model/Changes/Property.php +++ b/Classes/Domain/Model/Changes/Property.php @@ -239,7 +239,7 @@ private function handleNodeReferenceChange(Node $subject, string $propertyName): $subject->workspaceName, $subject->aggregateId, $subject->originDimensionSpacePoint, - NodeReferencesToWrite::fromReferenceNameAndNodeAggregateIds(ReferenceName::fromString($propertyName), NodeAggregateIds::fromArray($destinationNodeAggregateIds)) + NodeReferencesToWrite::fromNameAndTargets(ReferenceName::fromString($propertyName), NodeAggregateIds::fromArray($destinationNodeAggregateIds)) ) ); } diff --git a/Classes/Domain/NodeCreation/NodeCreationCommands.php b/Classes/Domain/NodeCreation/NodeCreationCommands.php index ad225e5fcb..52115aad6e 100644 --- a/Classes/Domain/NodeCreation/NodeCreationCommands.php +++ b/Classes/Domain/NodeCreation/NodeCreationCommands.php @@ -21,6 +21,7 @@ use Neos\ContentRepository\Core\Feature\NodeModification\Command\SetNodeProperties; use Neos\ContentRepository\Core\Feature\NodeModification\Dto\PropertyValuesToWrite; use Neos\ContentRepository\Core\Feature\NodeReferencing\Command\SetNodeReferences; +use Neos\ContentRepository\Core\Feature\NodeReferencing\Dto\NodeReferencesToWrite; use Neos\ContentRepository\Core\NodeType\NodeTypeManager; /** @@ -107,6 +108,14 @@ public function withInitialPropertyValues(PropertyValuesToWrite $newInitialPrope ); } + public function withInitialReferences(NodeReferencesToWrite $newInitialReferences): self + { + return new self( + $this->first->withReferences($newInitialReferences), + ...$this->additionalCommands + ); + } + public function withAdditionalCommands( CreateNodeAggregateWithNode|SetNodeProperties|DisableNodeAggregate|EnableNodeAggregate|SetNodeReferences|CopyNodesRecursively ...$additionalCommands ): self { diff --git a/Classes/Infrastructure/ContentRepository/CreationDialog/PromotedElementsCreationHandlerFactory.php b/Classes/Infrastructure/ContentRepository/CreationDialog/PromotedElementsCreationHandlerFactory.php index f20cbf5818..996f4f3619 100644 --- a/Classes/Infrastructure/ContentRepository/CreationDialog/PromotedElementsCreationHandlerFactory.php +++ b/Classes/Infrastructure/ContentRepository/CreationDialog/PromotedElementsCreationHandlerFactory.php @@ -5,7 +5,6 @@ namespace Neos\Neos\Ui\Infrastructure\ContentRepository\CreationDialog; use Neos\ContentRepository\Core\ContentRepository; -use Neos\ContentRepository\Core\Feature\NodeReferencing\Command\SetNodeReferences; use Neos\ContentRepository\Core\Feature\NodeReferencing\Dto\NodeReferencesToWrite; use Neos\ContentRepository\Core\NodeType\NodeTypeManager; use Neos\ContentRepository\Core\SharedModel\Node\NodeAggregateIds; @@ -39,7 +38,7 @@ public function handle(NodeCreationCommands $commands, NodeCreationElements $ele return $commands; } $propertyValues = $commands->first->initialPropertyValues; - $setReferencesCommands = []; + $initialReferences = null; foreach ($elements as $elementName => $elementValue) { // handle properties if ($nodeType->hasProperty($elementName)) { @@ -56,23 +55,19 @@ public function handle(NodeCreationCommands $commands, NodeCreationElements $ele if ($nodeType->hasReference($elementName)) { assert($elementValue instanceof NodeAggregateIds); $referenceConfiguration = $nodeType->getReferences()[$elementName]; - if ( - ($referenceConfiguration['ui']['showInCreationDialog'] ?? false) === true - ) { - // a promoted element - $setReferencesCommands[] = SetNodeReferences::create( - $commands->first->workspaceName, - $commands->first->nodeAggregateId, - $commands->first->originDimensionSpacePoint, - NodeReferencesToWrite::fromReferenceNameAndNodeAggregateIds(ReferenceName::fromString($elementName), $elementValue) - ); + if (($referenceConfiguration['ui']['showInCreationDialog'] ?? false) === true) { + $referencesToWriteForCurrentElement = NodeReferencesToWrite::fromNameAndTargets(ReferenceName::fromString($elementName), $elementValue); + $initialReferences = $initialReferences === null ? $referencesToWriteForCurrentElement : $initialReferences->merge($referencesToWriteForCurrentElement); } } } + if ($initialReferences !== null) { + $commands = $commands->withInitialReferences($initialReferences); + } + return $commands - ->withInitialPropertyValues($propertyValues) - ->withAdditionalCommands(...$setReferencesCommands); + ->withInitialPropertyValues($propertyValues); } }; } From fc500360e4e3b170c96303134375c8ef6e4a17b2 Mon Sep 17 00:00:00 2001 From: Denny Lubitz Date: Thu, 24 Oct 2024 15:03:29 +0200 Subject: [PATCH 19/36] TASK: Show changes to nodeAggregates for occupied nodes dimensionspacepoints --- .../Service/WorkspaceService.php | 45 ++++++++++++------- 1 file changed, 29 insertions(+), 16 deletions(-) diff --git a/Classes/ContentRepository/Service/WorkspaceService.php b/Classes/ContentRepository/Service/WorkspaceService.php index 43cb89155f..b880763061 100644 --- a/Classes/ContentRepository/Service/WorkspaceService.php +++ b/Classes/ContentRepository/Service/WorkspaceService.php @@ -1,4 +1,5 @@ removalAttachmentPoint) { + if ($change->removalAttachmentPoint && $change->originDimensionSpacePoint !== null) { $nodeAddress = NodeAddress::create( $contentRepositoryId, $workspaceName, @@ -79,20 +80,32 @@ public function getPublishableNodeInfo(WorkspaceName $workspaceName, ContentRepo 'typeOfChange' => $this->getTypeOfChange($change) ]; } else { - $subgraph = $contentRepository->getContentGraph($workspaceName)->getSubgraph( - $change->originDimensionSpacePoint->toDimensionSpacePoint(), - VisibilityConstraints::withoutRestrictions() - ); - $node = $subgraph->findNodeById($change->nodeAggregateId); - - if ($node instanceof Node) { - $documentNode = $subgraph->findClosestNode($node->aggregateId, FindClosestNodeFilter::create(nodeTypes: NodeTypeNameFactory::NAME_DOCUMENT)); - if ($documentNode instanceof Node) { - $unpublishedNodes[] = [ - 'contextPath' => NodeAddress::fromNode($node)->toJson(), - 'documentContextPath' => NodeAddress::fromNode($documentNode)->toJson(), - 'typeOfChange' => $this->getTypeOfChange($change) - ]; + if ($change->originDimensionSpacePoint !== null) { + $originDimensionSpacePoints = [$change->originDimensionSpacePoint]; + } else { + // If originDimensionSpacePoint is null, we have a change to the nodeAggregate. All nodes in the + // occupied dimensionspacepoints shall be marked as changed. + $originDimensionSpacePoints = $contentRepository + ->getContentGraph($workspaceName) + ->findNodeAggregateById($change->nodeAggregateId) + ?->occupiedDimensionSpacePoints ?: []; + } + + foreach ($originDimensionSpacePoints as $originDimensionSpacePoint) { + $subgraph = $contentRepository->getContentGraph($workspaceName)->getSubgraph( + $originDimensionSpacePoint->toDimensionSpacePoint(), + VisibilityConstraints::withoutRestrictions() + ); + $node = $subgraph->findNodeById($change->nodeAggregateId); + if ($node instanceof Node) { + $documentNode = $subgraph->findClosestNode($node->aggregateId, FindClosestNodeFilter::create(nodeTypes: NodeTypeNameFactory::NAME_DOCUMENT)); + if ($documentNode instanceof Node) { + $unpublishedNodes[] = [ + 'contextPath' => NodeAddress::fromNode($node)->toJson(), + 'documentContextPath' => NodeAddress::fromNode($documentNode)->toJson(), + 'typeOfChange' => $this->getTypeOfChange($change) + ]; + } } } } From bbf744c1073f4f3a7e67125d1b9eeb08a48843cd Mon Sep 17 00:00:00 2001 From: Denny Lubitz Date: Thu, 24 Oct 2024 15:22:33 +0200 Subject: [PATCH 20/36] TASK: Load contentGraph once --- Classes/ContentRepository/Service/WorkspaceService.php | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/Classes/ContentRepository/Service/WorkspaceService.php b/Classes/ContentRepository/Service/WorkspaceService.php index b880763061..9f43d9e717 100644 --- a/Classes/ContentRepository/Service/WorkspaceService.php +++ b/Classes/ContentRepository/Service/WorkspaceService.php @@ -52,6 +52,7 @@ class WorkspaceService public function getPublishableNodeInfo(WorkspaceName $workspaceName, ContentRepositoryId $contentRepositoryId): array { $contentRepository = $this->contentRepositoryRegistry->get($contentRepositoryId); + $contentGraph = $contentRepository->getContentGraph($workspaceName); $pendingChanges = $this->workspacePublishingService->pendingWorkspaceChanges($contentRepositoryId, $workspaceName); /** @var array{contextPath:string,documentContextPath:string,typeOfChange:int}[] $unpublishedNodes */ $unpublishedNodes = []; @@ -92,7 +93,7 @@ public function getPublishableNodeInfo(WorkspaceName $workspaceName, ContentRepo } foreach ($originDimensionSpacePoints as $originDimensionSpacePoint) { - $subgraph = $contentRepository->getContentGraph($workspaceName)->getSubgraph( + $subgraph = $contentGraph->getSubgraph( $originDimensionSpacePoint->toDimensionSpacePoint(), VisibilityConstraints::withoutRestrictions() ); From b3bafccea646a1fb2227e5563c09a6b50fae3f4f Mon Sep 17 00:00:00 2001 From: Denny Lubitz Date: Thu, 24 Oct 2024 15:23:20 +0200 Subject: [PATCH 21/36] TASK: Load contentGraph once --- Classes/ContentRepository/Service/WorkspaceService.php | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/Classes/ContentRepository/Service/WorkspaceService.php b/Classes/ContentRepository/Service/WorkspaceService.php index 9f43d9e717..d1b9834a3c 100644 --- a/Classes/ContentRepository/Service/WorkspaceService.php +++ b/Classes/ContentRepository/Service/WorkspaceService.php @@ -86,8 +86,7 @@ public function getPublishableNodeInfo(WorkspaceName $workspaceName, ContentRepo } else { // If originDimensionSpacePoint is null, we have a change to the nodeAggregate. All nodes in the // occupied dimensionspacepoints shall be marked as changed. - $originDimensionSpacePoints = $contentRepository - ->getContentGraph($workspaceName) + $originDimensionSpacePoints = $contentGraph ->findNodeAggregateById($change->nodeAggregateId) ?->occupiedDimensionSpacePoints ?: []; } From c4dcecce0722748c6fe40a164ceb83c837d309d7 Mon Sep 17 00:00:00 2001 From: Denny Lubitz Date: Thu, 24 Oct 2024 18:33:37 +0200 Subject: [PATCH 22/36] BUGIFX: Show hidden state only for directly disabled nodes without inheritance --- Classes/Domain/Service/NodePropertyConverterService.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Classes/Domain/Service/NodePropertyConverterService.php b/Classes/Domain/Service/NodePropertyConverterService.php index 2688c2dbed..b8c7231084 100644 --- a/Classes/Domain/Service/NodePropertyConverterService.php +++ b/Classes/Domain/Service/NodePropertyConverterService.php @@ -135,7 +135,7 @@ private function getReference(Node $node, string $referenceName): array|string|n private function getProperty(Node $node, string $propertyName): mixed { if ($propertyName === '_hidden') { - return $node->tags->contain(SubtreeTag::fromString('disabled')); + return $node->tags->withoutInherited()->contain(SubtreeTag::fromString('disabled')); } $propertyValue = $node->getProperty($propertyName); From 6778a5aaec0c66b5dfae174b5e09db594d3c93ae Mon Sep 17 00:00:00 2001 From: mhsdesign <85400359+mhsdesign@users.noreply.github.com> Date: Sun, 27 Oct 2024 11:11:38 +0100 Subject: [PATCH 23/36] TASK: Make sure e2e test fail if the Publish Result Dialog shows an error Currently, the `#neos-PublishDialog-Acknowledge` id will be used both for the "ok" button in success, and for the "cancel" button in case of any unexpected errors. To make the e2e tests fail fast we change the id in the error case to `Acknowledge-Error` so we dont accidentally click it. see also: https://github.com/neos/neos-ui/pull/3769#pullrequestreview-2332466270 --- .../src/Containers/Modals/PublishingDialog/ResultDialog.tsx | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/neos-ui/src/Containers/Modals/PublishingDialog/ResultDialog.tsx b/packages/neos-ui/src/Containers/Modals/PublishingDialog/ResultDialog.tsx index d66e971f2d..9f1a3a5338 100644 --- a/packages/neos-ui/src/Containers/Modals/PublishingDialog/ResultDialog.tsx +++ b/packages/neos-ui/src/Containers/Modals/PublishingDialog/ResultDialog.tsx @@ -277,7 +277,7 @@ export const ResultDialog: React.FC<{ Date: Sun, 27 Oct 2024 11:19:06 +0100 Subject: [PATCH 24/36] TASK: Don't absorb unexpected exception if publishing failed solves https://github.com/neos/neos-ui/issues/3845 --- Classes/Controller/BackendServiceController.php | 14 ++++++++++++++ 1 file changed, 14 insertions(+) diff --git a/Classes/Controller/BackendServiceController.php b/Classes/Controller/BackendServiceController.php index b6c0280a24..c77cb7d799 100644 --- a/Classes/Controller/BackendServiceController.php +++ b/Classes/Controller/BackendServiceController.php @@ -24,6 +24,7 @@ use Neos\Eel\FlowQuery\FlowQuery; use Neos\Eel\FlowQuery\Operations\GetOperation; use Neos\Flow\Annotations as Flow; +use Neos\Flow\Log\ThrowableStorageInterface; use Neos\Flow\Mvc\ActionRequest; use Neos\Flow\Mvc\ActionResponse; use Neos\Flow\Mvc\Controller\ActionController; @@ -167,6 +168,14 @@ class BackendServiceController extends ActionController */ protected $reloadNodesQueryHandler; + /** + * Cant be named here $throwableStorage see https://github.com/neos/flow-development-collection/issues/2928 + * + * @Flow\Inject + * @var ThrowableStorageInterface + */ + protected $throwableStorage2; + /** * Set the controller context on the feedback collection after the controller * has been initialized @@ -227,6 +236,7 @@ public function publishChangesInSiteAction(array $command): void $this->view->assign('value', $result); } catch (\Exception $e) { + $this->throwableStorage2->logThrowable($e); $this->view->assign('value', [ 'error' => [ 'class' => $e::class, @@ -259,6 +269,7 @@ public function publishChangesInDocumentAction(array $command): void $this->view->assign('value', $result); } catch (\Exception $e) { + $this->throwableStorage2->logThrowable($e); $this->view->assign('value', [ 'error' => [ 'class' => $e::class, @@ -294,6 +305,7 @@ public function discardAllChangesAction(array $command): void ] ]); } catch (\Exception $e) { + $this->throwableStorage2->logThrowable($e); $this->view->assign('value', [ 'error' => [ 'class' => $e::class, @@ -333,6 +345,7 @@ public function discardChangesInSiteAction(array $command): void ] ]); } catch (\Exception $e) { + $this->throwableStorage2->logThrowable($e); $this->view->assign('value', [ 'error' => [ 'class' => $e::class, @@ -372,6 +385,7 @@ public function discardChangesInDocumentAction(array $command): void ] ]); } catch (\Exception $e) { + $this->throwableStorage2->logThrowable($e); $this->view->assign('value', [ 'error' => [ 'class' => $e::class, From 7468ffb851ac16713f8afda9277204a396f09a1c Mon Sep 17 00:00:00 2001 From: mhsdesign <85400359+mhsdesign@users.noreply.github.com> Date: Sun, 27 Oct 2024 11:19:40 +0100 Subject: [PATCH 25/36] TASK: Adjust to changes of Publishing V3 https://github.com/neos/neos-development-collection/pull/5301 --- Classes/Infrastructure/ContentRepository/ConflictsFactory.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Classes/Infrastructure/ContentRepository/ConflictsFactory.php b/Classes/Infrastructure/ContentRepository/ConflictsFactory.php index fb688ef00e..8bc0be74b6 100644 --- a/Classes/Infrastructure/ContentRepository/ConflictsFactory.php +++ b/Classes/Infrastructure/ContentRepository/ConflictsFactory.php @@ -115,7 +115,7 @@ private function createConflictFromCommandThatFailedDuringRebase( return new Conflict( key: $affectedNode ? $affectedNode->aggregateId->value - : 'command-' . $commandThatFailedDuringRebase->sequenceNumber, + : 'command-' . $commandThatFailedDuringRebase->sequenceNumber->value, affectedSite: $affectedSite ? $this->createIconLabelForNode($affectedSite) : null, From 15c13ab601a44b54f178304558e48a58cb7ef1b8 Mon Sep 17 00:00:00 2001 From: mhsdesign <85400359+mhsdesign@users.noreply.github.com> Date: Mon, 28 Oct 2024 13:49:51 +0100 Subject: [PATCH 26/36] TASK: Avoid use of internal `sequenceNumber` for building commands that failed instead we can just generate a uuid as the `sequenceNumber` was also unique for each command that failed see https://github.com/neos/neos-development-collection/pull/5301#discussion_r1818111183 --- Classes/Infrastructure/ContentRepository/ConflictsFactory.php | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/Classes/Infrastructure/ContentRepository/ConflictsFactory.php b/Classes/Infrastructure/ContentRepository/ConflictsFactory.php index 8bc0be74b6..acf407195c 100644 --- a/Classes/Infrastructure/ContentRepository/ConflictsFactory.php +++ b/Classes/Infrastructure/ContentRepository/ConflictsFactory.php @@ -114,8 +114,8 @@ private function createConflictFromCommandThatFailedDuringRebase( return new Conflict( key: $affectedNode - ? $affectedNode->aggregateId->value - : 'command-' . $commandThatFailedDuringRebase->sequenceNumber->value, + ? $affectedNode->aggregateId->value + : Algorithms::generateUUID(), affectedSite: $affectedSite ? $this->createIconLabelForNode($affectedSite) : null, From 36a5de71e2f1d9da88d64bb8ede316bcd47083fe Mon Sep 17 00:00:00 2001 From: mhsdesign <85400359+mhsdesign@users.noreply.github.com> Date: Mon, 28 Oct 2024 13:50:44 +0100 Subject: [PATCH 27/36] TASK: Inline `ConflictsBuilder` to better document deduplication logic --- Classes/Application/Shared/Conflicts.php | 7 +-- .../Application/Shared/ConflictsBuilder.php | 49 ------------------- .../ContentRepository/ConflictsFactory.php | 11 +++-- 3 files changed, 9 insertions(+), 58 deletions(-) delete mode 100644 Classes/Application/Shared/ConflictsBuilder.php diff --git a/Classes/Application/Shared/Conflicts.php b/Classes/Application/Shared/Conflicts.php index 52bfa1723c..6521088caf 100644 --- a/Classes/Application/Shared/Conflicts.php +++ b/Classes/Application/Shared/Conflicts.php @@ -27,12 +27,7 @@ public function __construct(Conflict ...$items) { - $this->items = $items; - } - - public static function builder(): ConflictsBuilder - { - return new ConflictsBuilder(); + $this->items = array_values($items); } public function jsonSerialize(): mixed diff --git a/Classes/Application/Shared/ConflictsBuilder.php b/Classes/Application/Shared/ConflictsBuilder.php deleted file mode 100644 index 379b6a441a..0000000000 --- a/Classes/Application/Shared/ConflictsBuilder.php +++ /dev/null @@ -1,49 +0,0 @@ - - */ - private array $itemsByKey = []; - - public function addConflict(Conflict $conflict): self - { - if (!isset($this->itemsByKey[$conflict->key])) { - $this->itemsByKey[$conflict->key] = $conflict; - $this->items[] = $conflict; - } - - return $this; - } - - public function build(): Conflicts - { - return new Conflicts(...$this->items); - } -} diff --git a/Classes/Infrastructure/ContentRepository/ConflictsFactory.php b/Classes/Infrastructure/ContentRepository/ConflictsFactory.php index acf407195c..ab3fdf31f3 100644 --- a/Classes/Infrastructure/ContentRepository/ConflictsFactory.php +++ b/Classes/Infrastructure/ContentRepository/ConflictsFactory.php @@ -44,6 +44,7 @@ use Neos\ContentRepository\Core\SharedModel\Workspace\Workspace; use Neos\ContentRepository\Core\SharedModel\Workspace\WorkspaceName; use Neos\Flow\Annotations as Flow; +use Neos\Flow\Utility\Algorithms; use Neos\Neos\Domain\NodeLabel\NodeLabelGeneratorInterface; use Neos\Neos\Domain\Service\NodeTypeNameFactory; use Neos\Neos\Ui\Application\Shared\Conflict; @@ -76,14 +77,18 @@ public function __construct( public function fromWorkspaceRebaseFailed( WorkspaceRebaseFailed $workspaceRebaseFailed ): Conflicts { - $conflictsBuilder = Conflicts::builder(); + /** @var array */ + $conflictsByKey = []; foreach ($workspaceRebaseFailed->commandsThatFailedDuringRebase as $commandThatFailedDuringRebase) { $conflict = $this->createConflictFromCommandThatFailedDuringRebase($commandThatFailedDuringRebase); - $conflictsBuilder->addConflict($conflict); + if (array_key_exists($conflict->key, $conflictsByKey)) { + // deduplicate if the conflict affects the same node + $conflictsByKey[$conflict->key] = $conflict; + } } - return $conflictsBuilder->build(); + return new Conflicts(...$conflictsByKey); } private function createConflictFromCommandThatFailedDuringRebase( From 874e187f76b5dc0c79e028b829ab8d94ed932589 Mon Sep 17 00:00:00 2001 From: mhsdesign <85400359+mhsdesign@users.noreply.github.com> Date: Wed, 30 Oct 2024 21:59:17 +0100 Subject: [PATCH 28/36] TASK: Adjust to references changes in Neos --- Classes/Domain/Model/Changes/Property.php | 8 +++++++- ...PromotedElementsCreationHandlerFactory.php | 19 ++++++++++--------- 2 files changed, 17 insertions(+), 10 deletions(-) diff --git a/Classes/Domain/Model/Changes/Property.php b/Classes/Domain/Model/Changes/Property.php index 93ece455dc..766754d0af 100644 --- a/Classes/Domain/Model/Changes/Property.php +++ b/Classes/Domain/Model/Changes/Property.php @@ -19,6 +19,7 @@ use Neos\ContentRepository\Core\Feature\NodeModification\Command\SetNodeProperties; use Neos\ContentRepository\Core\Feature\NodeModification\Dto\PropertyValuesToWrite; use Neos\ContentRepository\Core\Feature\NodeReferencing\Command\SetNodeReferences; +use Neos\ContentRepository\Core\Feature\NodeReferencing\Dto\NodeReferencesForName; use Neos\ContentRepository\Core\Feature\NodeReferencing\Dto\NodeReferencesToWrite; use Neos\ContentRepository\Core\Feature\NodeTypeChange\Command\ChangeNodeAggregateType; use Neos\ContentRepository\Core\Feature\NodeTypeChange\Dto\NodeAggregateTypeChangeChildConstraintConflictResolutionStrategy; @@ -239,7 +240,12 @@ private function handleNodeReferenceChange(Node $subject, string $propertyName): $subject->workspaceName, $subject->aggregateId, $subject->originDimensionSpacePoint, - NodeReferencesToWrite::fromNameAndTargets(ReferenceName::fromString($propertyName), NodeAggregateIds::fromArray($destinationNodeAggregateIds)) + NodeReferencesToWrite::fromReferences( + NodeReferencesForName::fromNameAndTargets( + ReferenceName::fromString($propertyName), + NodeAggregateIds::fromArray($destinationNodeAggregateIds) + ) + ) ) ); } diff --git a/Classes/Infrastructure/ContentRepository/CreationDialog/PromotedElementsCreationHandlerFactory.php b/Classes/Infrastructure/ContentRepository/CreationDialog/PromotedElementsCreationHandlerFactory.php index 996f4f3619..ba06077aa5 100644 --- a/Classes/Infrastructure/ContentRepository/CreationDialog/PromotedElementsCreationHandlerFactory.php +++ b/Classes/Infrastructure/ContentRepository/CreationDialog/PromotedElementsCreationHandlerFactory.php @@ -5,7 +5,7 @@ namespace Neos\Neos\Ui\Infrastructure\ContentRepository\CreationDialog; use Neos\ContentRepository\Core\ContentRepository; -use Neos\ContentRepository\Core\Feature\NodeReferencing\Dto\NodeReferencesToWrite; +use Neos\ContentRepository\Core\Feature\NodeReferencing\Dto\NodeReferencesForName; use Neos\ContentRepository\Core\NodeType\NodeTypeManager; use Neos\ContentRepository\Core\SharedModel\Node\NodeAggregateIds; use Neos\ContentRepository\Core\SharedModel\Node\ReferenceName; @@ -38,7 +38,7 @@ public function handle(NodeCreationCommands $commands, NodeCreationElements $ele return $commands; } $propertyValues = $commands->first->initialPropertyValues; - $initialReferences = null; + $initialReferences = $commands->first->references; foreach ($elements as $elementName => $elementValue) { // handle properties if ($nodeType->hasProperty($elementName)) { @@ -56,18 +56,19 @@ public function handle(NodeCreationCommands $commands, NodeCreationElements $ele assert($elementValue instanceof NodeAggregateIds); $referenceConfiguration = $nodeType->getReferences()[$elementName]; if (($referenceConfiguration['ui']['showInCreationDialog'] ?? false) === true) { - $referencesToWriteForCurrentElement = NodeReferencesToWrite::fromNameAndTargets(ReferenceName::fromString($elementName), $elementValue); - $initialReferences = $initialReferences === null ? $referencesToWriteForCurrentElement : $initialReferences->merge($referencesToWriteForCurrentElement); + $initialReferences = $initialReferences->withReference( + NodeReferencesForName::fromNameAndTargets( + ReferenceName::fromString($elementName), + $elementValue + ) + ); } } } - if ($initialReferences !== null) { - $commands = $commands->withInitialReferences($initialReferences); - } - return $commands - ->withInitialPropertyValues($propertyValues); + ->withInitialPropertyValues($propertyValues) + ->withInitialReferences($initialReferences); } }; } From ae089066b5ef03b4145da5a725ec21f35247b1db Mon Sep 17 00:00:00 2001 From: mhsdesign <85400359+mhsdesign@users.noreply.github.com> Date: Fri, 1 Nov 2024 14:23:07 +0100 Subject: [PATCH 29/36] TASK: Followup to #3844 --- Classes/Domain/Model/Changes/Property.php | 4 ++-- .../CreationDialog/PromotedElementsCreationHandlerFactory.php | 2 +- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/Classes/Domain/Model/Changes/Property.php b/Classes/Domain/Model/Changes/Property.php index 766754d0af..06cd3e5814 100644 --- a/Classes/Domain/Model/Changes/Property.php +++ b/Classes/Domain/Model/Changes/Property.php @@ -240,8 +240,8 @@ private function handleNodeReferenceChange(Node $subject, string $propertyName): $subject->workspaceName, $subject->aggregateId, $subject->originDimensionSpacePoint, - NodeReferencesToWrite::fromReferences( - NodeReferencesForName::fromNameAndTargets( + NodeReferencesToWrite::create( + NodeReferencesForName::fromTargets( ReferenceName::fromString($propertyName), NodeAggregateIds::fromArray($destinationNodeAggregateIds) ) diff --git a/Classes/Infrastructure/ContentRepository/CreationDialog/PromotedElementsCreationHandlerFactory.php b/Classes/Infrastructure/ContentRepository/CreationDialog/PromotedElementsCreationHandlerFactory.php index ba06077aa5..eea2b8355e 100644 --- a/Classes/Infrastructure/ContentRepository/CreationDialog/PromotedElementsCreationHandlerFactory.php +++ b/Classes/Infrastructure/ContentRepository/CreationDialog/PromotedElementsCreationHandlerFactory.php @@ -57,7 +57,7 @@ public function handle(NodeCreationCommands $commands, NodeCreationElements $ele $referenceConfiguration = $nodeType->getReferences()[$elementName]; if (($referenceConfiguration['ui']['showInCreationDialog'] ?? false) === true) { $initialReferences = $initialReferences->withReference( - NodeReferencesForName::fromNameAndTargets( + NodeReferencesForName::fromTargets( ReferenceName::fromString($elementName), $elementValue ) From 5fd8ab37feee6a92473f007061c891643ab88d75 Mon Sep 17 00:00:00 2001 From: Bastian Waidelich Date: Fri, 8 Nov 2024 16:17:01 +0100 Subject: [PATCH 30/36] TASK: Adjust to "serializable commands" Related: https://github.com/neos/neos-development-collection/issues/5353 Related: https://github.com/neos/neos-development-collection/pull/5348 --- .../Infrastructure/ContentRepository/ConflictsFactory.php | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/Classes/Infrastructure/ContentRepository/ConflictsFactory.php b/Classes/Infrastructure/ContentRepository/ConflictsFactory.php index ab3fdf31f3..b63d5a1071 100644 --- a/Classes/Infrastructure/ContentRepository/ConflictsFactory.php +++ b/Classes/Infrastructure/ContentRepository/ConflictsFactory.php @@ -14,9 +14,9 @@ namespace Neos\Neos\Ui\Infrastructure\ContentRepository; -use Neos\ContentRepository\Core\CommandHandler\CommandInterface; use Neos\ContentRepository\Core\ContentRepository; use Neos\ContentRepository\Core\DimensionSpace\DimensionSpacePoint; +use Neos\ContentRepository\Core\Feature\Common\RebasableToOtherWorkspaceInterface; use Neos\ContentRepository\Core\Feature\NodeCreation\Command\CreateNodeAggregateWithNode; use Neos\ContentRepository\Core\Feature\NodeCreation\Command\CreateNodeAggregateWithNodeAndSerializedProperties; use Neos\ContentRepository\Core\Feature\NodeDisabling\Command\DisableNodeAggregate; @@ -139,7 +139,7 @@ private function createConflictFromCommandThatFailedDuringRebase( ); } - private function extractNodeAggregateIdFromCommand(CommandInterface $command): ?NodeAggregateId + private function extractNodeAggregateIdFromCommand(RebasableToOtherWorkspaceInterface $command): ?NodeAggregateId { return match (true) { $command instanceof MoveNodeAggregate, @@ -163,7 +163,7 @@ private function extractNodeAggregateIdFromCommand(CommandInterface $command): ? } private function acquireSubgraphFromCommand( - CommandInterface $command, + RebasableToOtherWorkspaceInterface $command, ?NodeAggregateId $nodeAggregateIdForDimensionFallback ): ?ContentSubgraphInterface { if ($this->workspace === null) { @@ -248,7 +248,7 @@ private function createIconLabelForNode(Node $node): IconLabel } private function createTypeOfChangeFromCommand( - CommandInterface $command + RebasableToOtherWorkspaceInterface $command ): ?TypeOfChange { return match (true) { $command instanceof CreateNodeAggregateWithNode, From 8619c7729e815a590981840ab3e4ff60df300793 Mon Sep 17 00:00:00 2001 From: mhsdesign <85400359+mhsdesign@users.noreply.github.com> Date: Sat, 9 Nov 2024 11:27:40 +0100 Subject: [PATCH 31/36] TASK: Adjust to overhauled api of `CommandThatFailedDuringRebase` --- .../ContentRepository/ConflictsFactory.php | 90 ++++++------------- 1 file changed, 28 insertions(+), 62 deletions(-) diff --git a/Classes/Infrastructure/ContentRepository/ConflictsFactory.php b/Classes/Infrastructure/ContentRepository/ConflictsFactory.php index b63d5a1071..2af7051498 100644 --- a/Classes/Infrastructure/ContentRepository/ConflictsFactory.php +++ b/Classes/Infrastructure/ContentRepository/ConflictsFactory.php @@ -17,14 +17,11 @@ use Neos\ContentRepository\Core\ContentRepository; use Neos\ContentRepository\Core\DimensionSpace\DimensionSpacePoint; use Neos\ContentRepository\Core\Feature\Common\RebasableToOtherWorkspaceInterface; -use Neos\ContentRepository\Core\Feature\NodeCreation\Command\CreateNodeAggregateWithNode; use Neos\ContentRepository\Core\Feature\NodeCreation\Command\CreateNodeAggregateWithNodeAndSerializedProperties; use Neos\ContentRepository\Core\Feature\NodeDisabling\Command\DisableNodeAggregate; use Neos\ContentRepository\Core\Feature\NodeDisabling\Command\EnableNodeAggregate; -use Neos\ContentRepository\Core\Feature\NodeModification\Command\SetNodeProperties; use Neos\ContentRepository\Core\Feature\NodeModification\Command\SetSerializedNodeProperties; use Neos\ContentRepository\Core\Feature\NodeMove\Command\MoveNodeAggregate; -use Neos\ContentRepository\Core\Feature\NodeReferencing\Command\SetNodeReferences; use Neos\ContentRepository\Core\Feature\NodeReferencing\Command\SetSerializedNodeReferences; use Neos\ContentRepository\Core\Feature\NodeRemoval\Command\RemoveNodeAggregate; use Neos\ContentRepository\Core\Feature\NodeTypeChange\Command\ChangeNodeAggregateType; @@ -94,11 +91,9 @@ public function fromWorkspaceRebaseFailed( private function createConflictFromCommandThatFailedDuringRebase( CommandThatFailedDuringRebase $commandThatFailedDuringRebase ): Conflict { - $nodeAggregateId = $this->extractNodeAggregateIdFromCommand( - $commandThatFailedDuringRebase->command - ); + $nodeAggregateId = $commandThatFailedDuringRebase->getAffectedNodeAggregateId(); $subgraph = $this->acquireSubgraphFromCommand( - $commandThatFailedDuringRebase->command, + $commandThatFailedDuringRebase->getCommand(), $nodeAggregateId ); $affectedSite = $nodeAggregateId @@ -131,37 +126,14 @@ private function createConflictFromCommandThatFailedDuringRebase( ? $this->createIconLabelForNode($affectedNode) : null, typeOfChange: $this->createTypeOfChangeFromCommand( - $commandThatFailedDuringRebase->command + $commandThatFailedDuringRebase->getCommand() ), reasonForConflict: $this->createReasonForConflictFromException( - $commandThatFailedDuringRebase->exception + $commandThatFailedDuringRebase->getException() ) ); } - private function extractNodeAggregateIdFromCommand(RebasableToOtherWorkspaceInterface $command): ?NodeAggregateId - { - return match (true) { - $command instanceof MoveNodeAggregate, - $command instanceof SetNodeProperties, - $command instanceof SetSerializedNodeProperties, - $command instanceof CreateNodeAggregateWithNode, - $command instanceof CreateNodeAggregateWithNodeAndSerializedProperties, - $command instanceof TagSubtree, - $command instanceof DisableNodeAggregate, - $command instanceof UntagSubtree, - $command instanceof EnableNodeAggregate, - $command instanceof RemoveNodeAggregate, - $command instanceof ChangeNodeAggregateType, - $command instanceof CreateNodeVariant => - $command->nodeAggregateId, - $command instanceof SetNodeReferences, - $command instanceof SetSerializedNodeReferences => - $command->sourceNodeAggregateId, - default => null - }; - } - private function acquireSubgraphFromCommand( RebasableToOtherWorkspaceInterface $command, ?NodeAggregateId $nodeAggregateIdForDimensionFallback @@ -170,26 +142,23 @@ private function acquireSubgraphFromCommand( return null; } - $dimensionSpacePoint = match (true) { - $command instanceof MoveNodeAggregate => + $dimensionSpacePoint = match ($command::class) { + MoveNodeAggregate::class => $command->dimensionSpacePoint, - $command instanceof SetNodeProperties, - $command instanceof SetSerializedNodeProperties, - $command instanceof CreateNodeAggregateWithNode, - $command instanceof CreateNodeAggregateWithNodeAndSerializedProperties => + SetSerializedNodeProperties::class, + CreateNodeAggregateWithNodeAndSerializedProperties::class => $command->originDimensionSpacePoint->toDimensionSpacePoint(), - $command instanceof SetNodeReferences, - $command instanceof SetSerializedNodeReferences => + SetSerializedNodeReferences::class => $command->sourceOriginDimensionSpacePoint->toDimensionSpacePoint(), - $command instanceof TagSubtree, - $command instanceof DisableNodeAggregate, - $command instanceof UntagSubtree, - $command instanceof EnableNodeAggregate, - $command instanceof RemoveNodeAggregate => + TagSubtree::class, + DisableNodeAggregate::class, + UntagSubtree::class, + EnableNodeAggregate::class, + RemoveNodeAggregate::class => $command->coveredDimensionSpacePoint, - $command instanceof ChangeNodeAggregateType => + ChangeNodeAggregateType::class => null, - $command instanceof CreateNodeVariant => + CreateNodeVariant::class => $command->targetOrigin->toDimensionSpacePoint(), default => null }; @@ -250,24 +219,21 @@ private function createIconLabelForNode(Node $node): IconLabel private function createTypeOfChangeFromCommand( RebasableToOtherWorkspaceInterface $command ): ?TypeOfChange { - return match (true) { - $command instanceof CreateNodeAggregateWithNode, - $command instanceof CreateNodeAggregateWithNodeAndSerializedProperties, - $command instanceof CreateNodeVariant => + return match ($command::class) { + CreateNodeAggregateWithNodeAndSerializedProperties::class, + CreateNodeVariant::class => TypeOfChange::NODE_HAS_BEEN_CREATED, - $command instanceof SetNodeProperties, - $command instanceof SetSerializedNodeProperties, - $command instanceof SetNodeReferences, - $command instanceof SetSerializedNodeReferences, - $command instanceof TagSubtree, - $command instanceof DisableNodeAggregate, - $command instanceof UntagSubtree, - $command instanceof EnableNodeAggregate, - $command instanceof ChangeNodeAggregateType => + SetSerializedNodeProperties::class, + SetSerializedNodeReferences::class, + TagSubtree::class, + DisableNodeAggregate::class, + UntagSubtree::class, + EnableNodeAggregate::class, + ChangeNodeAggregateType::class => TypeOfChange::NODE_HAS_BEEN_CHANGED, - $command instanceof MoveNodeAggregate => + MoveNodeAggregate::class => TypeOfChange::NODE_HAS_BEEN_MOVED, - $command instanceof RemoveNodeAggregate => + RemoveNodeAggregate::class => TypeOfChange::NODE_HAS_BEEN_DELETED, default => null }; From e2ee696e30f91457f2387d42b2ec38d409f26c61 Mon Sep 17 00:00:00 2001 From: mhsdesign <85400359+mhsdesign@users.noreply.github.com> Date: Sat, 9 Nov 2024 17:49:10 +0100 Subject: [PATCH 32/36] TASK: Extract conflict information from event and not possibly internal command --- .../ContentRepository/ConflictsFactory.php | 111 +++++++++--------- 1 file changed, 55 insertions(+), 56 deletions(-) diff --git a/Classes/Infrastructure/ContentRepository/ConflictsFactory.php b/Classes/Infrastructure/ContentRepository/ConflictsFactory.php index 2af7051498..9a68b641bc 100644 --- a/Classes/Infrastructure/ContentRepository/ConflictsFactory.php +++ b/Classes/Infrastructure/ContentRepository/ConflictsFactory.php @@ -16,19 +16,18 @@ use Neos\ContentRepository\Core\ContentRepository; use Neos\ContentRepository\Core\DimensionSpace\DimensionSpacePoint; -use Neos\ContentRepository\Core\Feature\Common\RebasableToOtherWorkspaceInterface; -use Neos\ContentRepository\Core\Feature\NodeCreation\Command\CreateNodeAggregateWithNodeAndSerializedProperties; -use Neos\ContentRepository\Core\Feature\NodeDisabling\Command\DisableNodeAggregate; -use Neos\ContentRepository\Core\Feature\NodeDisabling\Command\EnableNodeAggregate; -use Neos\ContentRepository\Core\Feature\NodeModification\Command\SetSerializedNodeProperties; -use Neos\ContentRepository\Core\Feature\NodeMove\Command\MoveNodeAggregate; -use Neos\ContentRepository\Core\Feature\NodeReferencing\Command\SetSerializedNodeReferences; -use Neos\ContentRepository\Core\Feature\NodeRemoval\Command\RemoveNodeAggregate; -use Neos\ContentRepository\Core\Feature\NodeTypeChange\Command\ChangeNodeAggregateType; -use Neos\ContentRepository\Core\Feature\NodeVariation\Command\CreateNodeVariant; -use Neos\ContentRepository\Core\Feature\SubtreeTagging\Command\TagSubtree; -use Neos\ContentRepository\Core\Feature\SubtreeTagging\Command\UntagSubtree; -use Neos\ContentRepository\Core\Feature\WorkspaceRebase\CommandThatFailedDuringRebase; +use Neos\ContentRepository\Core\EventStore\EventInterface; +use Neos\ContentRepository\Core\Feature\NodeCreation\Event\NodeAggregateWithNodeWasCreated; +use Neos\ContentRepository\Core\Feature\NodeModification\Event\NodePropertiesWereSet; +use Neos\ContentRepository\Core\Feature\NodeMove\Event\NodeAggregateWasMoved; +use Neos\ContentRepository\Core\Feature\NodeReferencing\Event\NodeReferencesWereSet; +use Neos\ContentRepository\Core\Feature\NodeRemoval\Event\NodeAggregateWasRemoved; +use Neos\ContentRepository\Core\Feature\NodeTypeChange\Event\NodeAggregateTypeWasChanged; +use Neos\ContentRepository\Core\Feature\NodeVariation\Event\NodeGeneralizationVariantWasCreated; +use Neos\ContentRepository\Core\Feature\NodeVariation\Event\NodePeerVariantWasCreated; +use Neos\ContentRepository\Core\Feature\SubtreeTagging\Event\SubtreeWasTagged; +use Neos\ContentRepository\Core\Feature\SubtreeTagging\Event\SubtreeWasUntagged; +use Neos\ContentRepository\Core\Feature\WorkspaceRebase\EventThatFailedDuringRebase; use Neos\ContentRepository\Core\Feature\WorkspaceRebase\Exception\WorkspaceRebaseFailed; use Neos\ContentRepository\Core\NodeType\NodeTypeManager; use Neos\ContentRepository\Core\Projection\ContentGraph\ContentSubgraphInterface; @@ -77,8 +76,8 @@ public function fromWorkspaceRebaseFailed( /** @var array */ $conflictsByKey = []; - foreach ($workspaceRebaseFailed->commandsThatFailedDuringRebase as $commandThatFailedDuringRebase) { - $conflict = $this->createConflictFromCommandThatFailedDuringRebase($commandThatFailedDuringRebase); + foreach ($workspaceRebaseFailed->eventsThatFailedDuringRebase as $eventThatFailedDuringRebase) { + $conflict = $this->createConflictFromEventThatFailedDuringRebase($eventThatFailedDuringRebase); if (array_key_exists($conflict->key, $conflictsByKey)) { // deduplicate if the conflict affects the same node $conflictsByKey[$conflict->key] = $conflict; @@ -88,12 +87,12 @@ public function fromWorkspaceRebaseFailed( return new Conflicts(...$conflictsByKey); } - private function createConflictFromCommandThatFailedDuringRebase( - CommandThatFailedDuringRebase $commandThatFailedDuringRebase + private function createConflictFromEventThatFailedDuringRebase( + EventThatFailedDuringRebase $eventThatFailedDuringRebase ): Conflict { - $nodeAggregateId = $commandThatFailedDuringRebase->getAffectedNodeAggregateId(); - $subgraph = $this->acquireSubgraphFromCommand( - $commandThatFailedDuringRebase->getCommand(), + $nodeAggregateId = $eventThatFailedDuringRebase->getAffectedNodeAggregateId(); + $subgraph = $this->acquireSubgraph( + $eventThatFailedDuringRebase->getEvent(), $nodeAggregateId ); $affectedSite = $nodeAggregateId @@ -125,41 +124,42 @@ private function createConflictFromCommandThatFailedDuringRebase( affectedNode: $affectedNode ? $this->createIconLabelForNode($affectedNode) : null, - typeOfChange: $this->createTypeOfChangeFromCommand( - $commandThatFailedDuringRebase->getCommand() + typeOfChange: $this->createTypeOfChange( + $eventThatFailedDuringRebase->getEvent() ), reasonForConflict: $this->createReasonForConflictFromException( - $commandThatFailedDuringRebase->getException() + $eventThatFailedDuringRebase->getException() ) ); } - private function acquireSubgraphFromCommand( - RebasableToOtherWorkspaceInterface $command, + private function acquireSubgraph( + EventInterface $event, ?NodeAggregateId $nodeAggregateIdForDimensionFallback ): ?ContentSubgraphInterface { if ($this->workspace === null) { return null; } - $dimensionSpacePoint = match ($command::class) { - MoveNodeAggregate::class => - $command->dimensionSpacePoint, - SetSerializedNodeProperties::class, - CreateNodeAggregateWithNodeAndSerializedProperties::class => - $command->originDimensionSpacePoint->toDimensionSpacePoint(), - SetSerializedNodeReferences::class => - $command->sourceOriginDimensionSpacePoint->toDimensionSpacePoint(), - TagSubtree::class, - DisableNodeAggregate::class, - UntagSubtree::class, - EnableNodeAggregate::class, - RemoveNodeAggregate::class => - $command->coveredDimensionSpacePoint, - ChangeNodeAggregateType::class => + $dimensionSpacePoint = match ($event::class) { + NodeAggregateWasMoved::class => + $event->succeedingSiblingsForCoverage->toDimensionSpacePointSet()->getFirst(), + NodePropertiesWereSet::class, + NodeAggregateWithNodeWasCreated::class => + $event->originDimensionSpacePoint->toDimensionSpacePoint(), + NodeReferencesWereSet::class => + $event->affectedSourceOriginDimensionSpacePoints->toDimensionSpacePointSet()->getFirst(), + SubtreeWasTagged::class, + SubtreeWasUntagged::class => + $event->affectedDimensionSpacePoints->getFirst(), + NodeAggregateWasRemoved::class => + $event->affectedCoveredDimensionSpacePoints->getFirst(), + NodeAggregateTypeWasChanged::class => null, - CreateNodeVariant::class => - $command->targetOrigin->toDimensionSpacePoint(), + NodePeerVariantWasCreated::class => + $event->peerOrigin->toDimensionSpacePoint(), + NodeGeneralizationVariantWasCreated::class => + $event->generalizationOrigin->toDimensionSpacePoint(), default => null }; @@ -216,24 +216,23 @@ private function createIconLabelForNode(Node $node): IconLabel ); } - private function createTypeOfChangeFromCommand( - RebasableToOtherWorkspaceInterface $command + private function createTypeOfChange( + EventInterface $event ): ?TypeOfChange { - return match ($command::class) { - CreateNodeAggregateWithNodeAndSerializedProperties::class, - CreateNodeVariant::class => + return match ($event::class) { + NodeAggregateWithNodeWasCreated::class, + NodePeerVariantWasCreated::class, + NodeGeneralizationVariantWasCreated::class => TypeOfChange::NODE_HAS_BEEN_CREATED, - SetSerializedNodeProperties::class, - SetSerializedNodeReferences::class, - TagSubtree::class, - DisableNodeAggregate::class, - UntagSubtree::class, - EnableNodeAggregate::class, - ChangeNodeAggregateType::class => + NodePropertiesWereSet::class, + NodeReferencesWereSet::class, + SubtreeWasTagged::class, + SubtreeWasUntagged::class, + NodeAggregateTypeWasChanged::class => TypeOfChange::NODE_HAS_BEEN_CHANGED, - MoveNodeAggregate::class => + NodeAggregateWasMoved::class => TypeOfChange::NODE_HAS_BEEN_MOVED, - RemoveNodeAggregate::class => + NodeAggregateWasRemoved::class => TypeOfChange::NODE_HAS_BEEN_DELETED, default => null }; From c2e9f67ad2a0ce1dcbaefd37c12a97b8a589f34f Mon Sep 17 00:00:00 2001 From: mhsdesign <85400359+mhsdesign@users.noreply.github.com> Date: Sat, 9 Nov 2024 17:56:22 +0100 Subject: [PATCH 33/36] TASK: Adjust to rename `EventThatFailedDuringRebase` to `ConflictingEvent` --- .../Infrastructure/ContentRepository/ConflictsFactory.php | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/Classes/Infrastructure/ContentRepository/ConflictsFactory.php b/Classes/Infrastructure/ContentRepository/ConflictsFactory.php index 9a68b641bc..8fd34f809c 100644 --- a/Classes/Infrastructure/ContentRepository/ConflictsFactory.php +++ b/Classes/Infrastructure/ContentRepository/ConflictsFactory.php @@ -27,7 +27,7 @@ use Neos\ContentRepository\Core\Feature\NodeVariation\Event\NodePeerVariantWasCreated; use Neos\ContentRepository\Core\Feature\SubtreeTagging\Event\SubtreeWasTagged; use Neos\ContentRepository\Core\Feature\SubtreeTagging\Event\SubtreeWasUntagged; -use Neos\ContentRepository\Core\Feature\WorkspaceRebase\EventThatFailedDuringRebase; +use Neos\ContentRepository\Core\Feature\WorkspaceRebase\ConflictingEvent; use Neos\ContentRepository\Core\Feature\WorkspaceRebase\Exception\WorkspaceRebaseFailed; use Neos\ContentRepository\Core\NodeType\NodeTypeManager; use Neos\ContentRepository\Core\Projection\ContentGraph\ContentSubgraphInterface; @@ -76,7 +76,7 @@ public function fromWorkspaceRebaseFailed( /** @var array */ $conflictsByKey = []; - foreach ($workspaceRebaseFailed->eventsThatFailedDuringRebase as $eventThatFailedDuringRebase) { + foreach ($workspaceRebaseFailed->conflictingEvents as $eventThatFailedDuringRebase) { $conflict = $this->createConflictFromEventThatFailedDuringRebase($eventThatFailedDuringRebase); if (array_key_exists($conflict->key, $conflictsByKey)) { // deduplicate if the conflict affects the same node @@ -88,7 +88,7 @@ public function fromWorkspaceRebaseFailed( } private function createConflictFromEventThatFailedDuringRebase( - EventThatFailedDuringRebase $eventThatFailedDuringRebase + ConflictingEvent $eventThatFailedDuringRebase ): Conflict { $nodeAggregateId = $eventThatFailedDuringRebase->getAffectedNodeAggregateId(); $subgraph = $this->acquireSubgraph( From 9175b0ea265565441fb5ce1cbe164780bf0046cb Mon Sep 17 00:00:00 2001 From: mhsdesign <85400359+mhsdesign@users.noreply.github.com> Date: Sat, 9 Nov 2024 19:27:05 +0100 Subject: [PATCH 34/36] TASK: Adjust to rename `EventThatFailedDuringRebase` to `ConflictingEvent` 2 --- .../ContentRepository/ConflictsFactory.php | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/Classes/Infrastructure/ContentRepository/ConflictsFactory.php b/Classes/Infrastructure/ContentRepository/ConflictsFactory.php index 8fd34f809c..013215e7c1 100644 --- a/Classes/Infrastructure/ContentRepository/ConflictsFactory.php +++ b/Classes/Infrastructure/ContentRepository/ConflictsFactory.php @@ -87,12 +87,12 @@ public function fromWorkspaceRebaseFailed( return new Conflicts(...$conflictsByKey); } - private function createConflictFromEventThatFailedDuringRebase( - ConflictingEvent $eventThatFailedDuringRebase + private function createConflict( + ConflictingEvent $conflictingEvent ): Conflict { - $nodeAggregateId = $eventThatFailedDuringRebase->getAffectedNodeAggregateId(); + $nodeAggregateId = $conflictingEvent->getAffectedNodeAggregateId(); $subgraph = $this->acquireSubgraph( - $eventThatFailedDuringRebase->getEvent(), + $conflictingEvent->getEvent(), $nodeAggregateId ); $affectedSite = $nodeAggregateId @@ -125,10 +125,10 @@ private function createConflictFromEventThatFailedDuringRebase( ? $this->createIconLabelForNode($affectedNode) : null, typeOfChange: $this->createTypeOfChange( - $eventThatFailedDuringRebase->getEvent() + $conflictingEvent->getEvent() ), reasonForConflict: $this->createReasonForConflictFromException( - $eventThatFailedDuringRebase->getException() + $conflictingEvent->getException() ) ); } From 829645a1555bcbf85e33a4cf23239d8fac9fa655 Mon Sep 17 00:00:00 2001 From: mhsdesign <85400359+mhsdesign@users.noreply.github.com> Date: Sat, 9 Nov 2024 19:28:01 +0100 Subject: [PATCH 35/36] BUGFIX: No conflicts are shown Regression from 36a5de71e2f1d9da88d64bb8ede316bcd47083fe as the deduplication did not work. --- .../Infrastructure/ContentRepository/ConflictsFactory.php | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/Classes/Infrastructure/ContentRepository/ConflictsFactory.php b/Classes/Infrastructure/ContentRepository/ConflictsFactory.php index 013215e7c1..c78e75c603 100644 --- a/Classes/Infrastructure/ContentRepository/ConflictsFactory.php +++ b/Classes/Infrastructure/ContentRepository/ConflictsFactory.php @@ -76,9 +76,9 @@ public function fromWorkspaceRebaseFailed( /** @var array */ $conflictsByKey = []; - foreach ($workspaceRebaseFailed->conflictingEvents as $eventThatFailedDuringRebase) { - $conflict = $this->createConflictFromEventThatFailedDuringRebase($eventThatFailedDuringRebase); - if (array_key_exists($conflict->key, $conflictsByKey)) { + foreach ($workspaceRebaseFailed->conflictingEvents as $conflictingEvent) { + $conflict = $this->createConflict($conflictingEvent); + if (!array_key_exists($conflict->key, $conflictsByKey)) { // deduplicate if the conflict affects the same node $conflictsByKey[$conflict->key] = $conflict; } From 33b9bac130ac117de9ad9e351d3e87934f0af786 Mon Sep 17 00:00:00 2001 From: mhsdesign <85400359+mhsdesign@users.noreply.github.com> Date: Mon, 11 Nov 2024 21:02:04 +0100 Subject: [PATCH 36/36] TASK: Adjust to use `firstDimensionSpacePoint` --- .../ContentRepository/ConflictsFactory.php | 21 +++++++++++++++---- 1 file changed, 17 insertions(+), 4 deletions(-) diff --git a/Classes/Infrastructure/ContentRepository/ConflictsFactory.php b/Classes/Infrastructure/ContentRepository/ConflictsFactory.php index c78e75c603..a9268c8e8d 100644 --- a/Classes/Infrastructure/ContentRepository/ConflictsFactory.php +++ b/Classes/Infrastructure/ContentRepository/ConflictsFactory.php @@ -16,6 +16,7 @@ use Neos\ContentRepository\Core\ContentRepository; use Neos\ContentRepository\Core\DimensionSpace\DimensionSpacePoint; +use Neos\ContentRepository\Core\DimensionSpace\DimensionSpacePointSet; use Neos\ContentRepository\Core\EventStore\EventInterface; use Neos\ContentRepository\Core\Feature\NodeCreation\Event\NodeAggregateWithNodeWasCreated; use Neos\ContentRepository\Core\Feature\NodeModification\Event\NodePropertiesWereSet; @@ -143,17 +144,21 @@ private function acquireSubgraph( $dimensionSpacePoint = match ($event::class) { NodeAggregateWasMoved::class => - $event->succeedingSiblingsForCoverage->toDimensionSpacePointSet()->getFirst(), + // TODO it seems the event lost some information here from the intention + self::firstDimensionSpacePoint($event->succeedingSiblingsForCoverage->toDimensionSpacePointSet()), NodePropertiesWereSet::class, NodeAggregateWithNodeWasCreated::class => $event->originDimensionSpacePoint->toDimensionSpacePoint(), NodeReferencesWereSet::class => - $event->affectedSourceOriginDimensionSpacePoints->toDimensionSpacePointSet()->getFirst(), + // TODO it seems the event lost some information here from the intention + self::firstDimensionSpacePoint($event->affectedSourceOriginDimensionSpacePoints->toDimensionSpacePointSet()), SubtreeWasTagged::class, SubtreeWasUntagged::class => - $event->affectedDimensionSpacePoints->getFirst(), + // TODO it seems the event lost some information here from the intention + self::firstDimensionSpacePoint($event->affectedDimensionSpacePoints), NodeAggregateWasRemoved::class => - $event->affectedCoveredDimensionSpacePoints->getFirst(), + // TODO it seems the event lost some information here from the intention + self::firstDimensionSpacePoint($event->affectedCoveredDimensionSpacePoints), NodeAggregateTypeWasChanged::class => null, NodePeerVariantWasCreated::class => @@ -247,4 +252,12 @@ private function createReasonForConflictFromException( default => null }; } + + private static function firstDimensionSpacePoint(DimensionSpacePointSet $dimensionSpacePointSet): ?DimensionSpacePoint + { + foreach ($dimensionSpacePointSet->points as $point) { + return $point; + } + return null; + } }