Skip to content

Commit b69d6ec

Browse files
kitsunetkitsunet
authored
Merge pull request #5508 from mhsdesign/bugfix/workspace-review-show-only-actual-changes
BUGFIX: Improve workspace module change handling
2 parents f1b23a2 + bcf61fe commit b69d6ec

File tree

4 files changed

+63
-38
lines changed

4 files changed

+63
-38
lines changed

‎Neos.Neos/Classes/Domain/Service/WorkspacePublishingService.php

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -347,12 +347,13 @@ private function resolveNodeIdsToPublishOrDiscard(
347347
NodeAggregateId $ancestorId,
348348
NodeTypeName $ancestorNodeTypeName
349349
): NodeAggregateIds {
350+
$contentGraph = $contentRepository->getContentGraph($workspaceName);
351+
350352
$nodeIdsToPublishOrDiscard = [];
351353
foreach ($this->pendingWorkspaceChangesInternal($contentRepository, $workspaceName) as $change) {
352354
if (
353355
!$this->isChangePublishableWithinAncestorScope(
354-
$contentRepository,
355-
$workspaceName,
356+
$contentGraph,
356357
$change,
357358
$ancestorNodeTypeName,
358359
$ancestorId
@@ -380,14 +381,13 @@ private function countPendingWorkspaceChangesInternal(ContentRepository $content
380381
}
381382

382383
private function isChangePublishableWithinAncestorScope(
383-
ContentRepository $contentRepository,
384-
WorkspaceName $workspaceName,
384+
ContentGraphInterface $contentGraph,
385385
Change $change,
386386
NodeTypeName $ancestorNodeTypeName,
387387
NodeAggregateId $ancestorId
388388
): bool {
389389
if ($change->originDimensionSpacePoint) {
390-
$subgraph = $contentRepository->getContentGraph($workspaceName)->getSubgraph(
390+
$subgraph = $contentGraph->getSubgraph(
391391
$change->originDimensionSpacePoint->toDimensionSpacePoint(),
392392
VisibilityConstraints::createEmpty()
393393
);
@@ -402,7 +402,7 @@ private function isChangePublishableWithinAncestorScope(
402402
return $actualAncestorNode?->aggregateId->equals($ancestorId) ?? false;
403403
} else {
404404
return $this->findAncestorAggregateIds(
405-
$contentRepository->getContentGraph($workspaceName),
405+
$contentGraph,
406406
$change->nodeAggregateId
407407
)->contain($ancestorId);
408408
}

‎Neos.Neos/Classes/PendingChangesProjection/ChangeProjection.php

Lines changed: 11 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -232,7 +232,9 @@ private function whenSubtreeWasTagged(SubtreeWasTagged $event): void
232232
}
233233
foreach ($event->affectedDimensionSpacePoints as $dimensionSpacePoint) {
234234
if ($event->tag->equals(NeosSubtreeTag::removed())) {
235-
$this->markAsDeleted($event->contentStreamId, $event->nodeAggregateId, OriginDimensionSpacePoint::fromDimensionSpacePoint($dimensionSpacePoint));
235+
$this->modifyChange($event->contentStreamId, $event->nodeAggregateId, OriginDimensionSpacePoint::fromDimensionSpacePoint($dimensionSpacePoint), static function (Change $change) {
236+
$change->deleted = true;
237+
});
236238
continue;
237239
}
238240

@@ -250,6 +252,14 @@ private function whenSubtreeWasUntagged(SubtreeWasUntagged $event): void
250252
return;
251253
}
252254
foreach ($event->affectedDimensionSpacePoints as $dimensionSpacePoint) {
255+
if ($event->tag->equals(NeosSubtreeTag::removed())) {
256+
$this->modifyChange($event->contentStreamId, $event->nodeAggregateId, OriginDimensionSpacePoint::fromDimensionSpacePoint($dimensionSpacePoint), static function (Change $change) {
257+
$change->deleted = false;
258+
$change->changed = true;
259+
});
260+
continue;
261+
}
262+
253263
$this->markAsChanged(
254264
$event->contentStreamId,
255265
$event->nodeAggregateId,
@@ -456,16 +466,6 @@ static function (Change $change) {
456466
);
457467
}
458468

459-
private function markAsDeleted(
460-
ContentStreamId $contentStreamId,
461-
NodeAggregateId $nodeAggregateId,
462-
OriginDimensionSpacePoint $originDimensionSpacePoint,
463-
): void {
464-
$this->modifyChange($contentStreamId, $nodeAggregateId, $originDimensionSpacePoint, static function (Change $change) {
465-
$change->deleted = true;
466-
});
467-
}
468-
469469
private function modifyChange(
470470
ContentStreamId $contentStreamId,
471471
NodeAggregateId $nodeAggregateId,

‎Neos.Neos/Tests/Behavior/Features/PendingChanges/NodeSoftRemoval/02-SoftRemoveNodeAggregate_WithDimensions.feature

Lines changed: 23 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -77,6 +77,29 @@ Feature: Soft remove node aggregate with node without dimensions
7777
| nody-mc-nodeface | 0 | 0 | 0 | 1 | {"language": "gsw"} |
7878
And I expect the ChangeProjection to have no changes in "cs-identifier"
7979

80+
Scenario: Soft remove and un-remove node aggregate in user-workspace
81+
Given the command TagSubtree is executed with payload:
82+
| Key | Value |
83+
| workspaceName | "user-workspace" |
84+
| nodeAggregateId | "nody-mc-nodeface" |
85+
| coveredDimensionSpacePoint | {"language": "de"} |
86+
| nodeVariantSelectionStrategy | "allSpecializations" |
87+
| tag | "removed" |
88+
89+
Given the command UntagSubtree is executed with payload:
90+
| Key | Value |
91+
| workspaceName | "user-workspace" |
92+
| nodeAggregateId | "nody-mc-nodeface" |
93+
| coveredDimensionSpacePoint | {"language": "de"} |
94+
| nodeVariantSelectionStrategy | "allSpecializations" |
95+
| tag | "removed" |
96+
97+
Then I expect the ChangeProjection to have the following changes in "user-cs-id":
98+
| nodeAggregateId | created | changed | moved | deleted | originDimensionSpacePoint |
99+
| nody-mc-nodeface | 0 | 1 | 0 | 0 | {"language": "de"} |
100+
| nody-mc-nodeface | 0 | 1 | 0 | 0 | {"language": "gsw"} |
101+
And I expect the ChangeProjection to have no changes in "cs-identifier"
102+
80103
Scenario: Soft remove node aggregate in live workspace
81104
Given the command TagSubtree is executed with payload:
82105
| Key | Value |

‎Neos.Workspace.Ui/Classes/Controller/WorkspaceController.php

Lines changed: 23 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -822,19 +822,29 @@ protected function computeSiteChanges(Workspace $selectedWorkspace, ContentRepos
822822
{
823823
$siteChanges = [];
824824
$changes = $this->getChangesFromWorkspace($selectedWorkspace, $contentRepository);
825-
826-
// TODO hack for the case $change->originDimensionSpacePoint is NULL so we can fetch a subgraph still. This is the case for changes NodeAggregateNameWasChanged or NodeAggregateTypeWasChanged
827-
$dimensionSpacePoints = iterator_to_array($contentRepository->getVariationGraph()->getDimensionSpacePoints());
828-
/** @var DimensionSpacePoint $arbitraryDimensionSpacePoint */
829-
$arbitraryDimensionSpacePoint = reset($dimensionSpacePoints);
830-
825+
$contentGraph = $contentRepository->getContentGraph($selectedWorkspace->workspaceName);
831826
foreach ($changes as $change) {
832-
$subgraph = $contentRepository->getContentGraph($selectedWorkspace->workspaceName)->getSubgraph(
833-
$change->originDimensionSpacePoint?->toDimensionSpacePoint() ?? $arbitraryDimensionSpacePoint,
834-
VisibilityConstraints::createEmpty()
835-
);
836-
837-
$node = $subgraph->findNodeById($change->nodeAggregateId);
827+
if ($change->originDimensionSpacePoint) {
828+
$subgraph = $contentGraph->getSubgraph(
829+
$change->originDimensionSpacePoint->toDimensionSpacePoint(),
830+
VisibilityConstraints::createEmpty()
831+
);
832+
$node = $subgraph->findNodeById($change->nodeAggregateId);
833+
} else {
834+
// for changes like NodeAggregateNameWasChanged or NodeAggregateTypeWasChanged, get a random occupying node:
835+
$nodeAggregate = $contentGraph->findNodeAggregateById($change->nodeAggregateId);
836+
if ($nodeAggregate === null) {
837+
continue;
838+
}
839+
$occupiedDimensionSpacePoints = $nodeAggregate->occupiedDimensionSpacePoints->getPoints();
840+
assert($occupiedDimensionSpacePoints !== []);
841+
$arbitraryDimensionSpacePoint = reset($occupiedDimensionSpacePoints);
842+
$node = $nodeAggregate->getNodeByOccupiedDimensionSpacePoint($arbitraryDimensionSpacePoint);
843+
$subgraph = $contentGraph->getSubgraph(
844+
$arbitraryDimensionSpacePoint->toDimensionSpacePoint(),
845+
VisibilityConstraints::createEmpty()
846+
);
847+
}
838848
if ($node) {
839849
$documentNode = null;
840850
$siteNode = null;
@@ -919,15 +929,7 @@ protected function computeSiteChanges(Workspace $selectedWorkspace, ContentRepos
919929
);
920930
}
921931

922-
// As for changes of type `delete` we are using nodes from the live workspace
923-
// we can't create a serialized nodeAddress from the node.
924-
// Instead, we use the original stored values.
925-
$nodeAddress = NodeAddress::create(
926-
$contentRepository->id,
927-
$selectedWorkspace->workspaceName,
928-
$change->originDimensionSpacePoint?->toDimensionSpacePoint() ?? $arbitraryDimensionSpacePoint,
929-
$change->nodeAggregateId
930-
);
932+
$nodeAddress = NodeAddress::fromNode($node);
931933
$nodeType = $contentRepository->getNodeTypeManager()->getNodeType($node->nodeTypeName);
932934
$dimensions = [];
933935
foreach ($node->dimensionSpacePoint->coordinates as $id => $coordinate) {

0 commit comments

Comments
 (0)