From aaeb68a85a88d3eaf93acec08d3bfa839c9028a8 Mon Sep 17 00:00:00 2001 From: Wilhelm Behncke Date: Mon, 22 Apr 2024 12:04:30 +0200 Subject: [PATCH 01/20] 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/20] 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/20] 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/20] 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/20] 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/20] 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/20] 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/20] 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/20] 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/20] 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/20] 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/20] 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 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 13/20] 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 14/20] 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 15/20] 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 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 16/20] 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 17/20] 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 18/20] 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 19/20] 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 20/20] 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(