Skip to content

Commit

Permalink
Merge pull request #4588 from neos/bugfix/review-workspace-changes
Browse files Browse the repository at this point in the history 
BUGFIX: Show and apply all changes in workspace review
  • Loading branch information
@bwaidelich
bwaidelich authored Oct 17, 2023
2 parents f04a914 + fa38119 commit fc37b92
Show file tree
Hide file tree
Showing 3 changed files with 75 additions and 76 deletions.
131 changes: 65 additions & 66 deletions Neos.Neos/Classes/Controller/Module/Management/WorkspacesController.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,8 @@

namespace Neos\Neos\Controller\Module\Management;

use Doctrine\DBAL\DBALException;
use JsonException;
use Neos\ContentRepository\Core\ContentRepository;
use Neos\ContentRepository\Core\Feature\WorkspacePublication\Dto\NodeIdsToPublishOrDiscard;
use Neos\ContentRepository\Core\Feature\WorkspacePublication\Dto\NodeIdToPublishOrDiscard;
Expand Down Expand Up @@ -78,35 +80,20 @@ class WorkspacesController extends AbstractModuleController
#[Flow\Inject]
protected ContentRepositoryRegistry $contentRepositoryRegistry;

/**
* @Flow\Inject
* @var SiteRepository
*/
protected $siteRepository;
#[Flow\Inject]
protected SiteRepository $siteRepository;

/**
* @Flow\Inject
* @var PropertyMapper
*/
protected $propertyMapper;
#[Flow\Inject]
protected PropertyMapper $propertyMapper;

/**
* @Flow\Inject
* @var Context
*/
protected $securityContext;
#[Flow\Inject]
protected Context $securityContext;

/**
* @Flow\Inject
* @var UserService
*/
protected $domainUserService;
#[Flow\Inject]
protected UserService $domainUserService;

/**
* @var PackageManager
* @Flow\Inject
*/
protected $packageManager;
#[Flow\Inject]
protected PackageManager $packageManager;

/**
* Display a list of unpublished content
Expand Down Expand Up @@ -190,10 +177,7 @@ public function showAction(WorkspaceName $workspace): void
]);
}

/**
* @return void
*/
public function newAction()
public function newAction(): void
{
$contentRepositoryId = SiteDetectionResult::fromRequest($this->request->getHttpRequest())
->contentRepositoryId;
Expand All @@ -218,7 +202,7 @@ public function createAction(
WorkspaceName $baseWorkspace,
string $visibility,
WorkspaceDescription $description,
) {
): void {
$contentRepositoryId = SiteDetectionResult::fromRequest($this->request->getHttpRequest())
->contentRepositoryId;
$contentRepository = $this->contentRepositoryRegistry->get($contentRepositoryId);
Expand Down Expand Up @@ -264,11 +248,8 @@ public function createAction(

/**
* Edit a workspace
*
* @param WorkspaceName $workspaceName
* @return void
*/
public function editAction(WorkspaceName $workspaceName)
public function editAction(WorkspaceName $workspaceName): void
{
$contentRepositoryId = SiteDetectionResult::fromRequest($this->request->getHttpRequest())
->contentRepositoryId;
Expand Down Expand Up @@ -301,8 +282,12 @@ public function editAction(WorkspaceName $workspaceName)
* @param string $workspaceOwner Id of the owner of the workspace
* @return void
*/
public function updateAction(WorkspaceName $workspaceName, WorkspaceTitle $title, WorkspaceDescription $description, ?string $workspaceOwner)
{
public function updateAction(
WorkspaceName $workspaceName,
WorkspaceTitle $title,
WorkspaceDescription $description,
?string $workspaceOwner
): void {
$contentRepositoryId = SiteDetectionResult::fromRequest($this->request->getHttpRequest())
->contentRepositoryId;
$contentRepository = $this->contentRepositoryRegistry->get($contentRepositoryId);
Expand Down Expand Up @@ -356,9 +341,12 @@ public function updateAction(WorkspaceName $workspaceName, WorkspaceTitle $title
* Delete a workspace
*
* @param WorkspaceName $workspaceName A workspace to delete
* @return void
* @throws IndexOutOfBoundsException
* @throws InvalidFormatPlaceholderException
* @throws StopActionException
* @throws DBALException
*/
public function deleteAction(WorkspaceName $workspaceName)
public function deleteAction(WorkspaceName $workspaceName): void
{
$contentRepositoryId = SiteDetectionResult::fromRequest($this->request->getHttpRequest())
->contentRepositoryId;
Expand Down Expand Up @@ -724,6 +712,7 @@ public function discardWorkspaceAction(WorkspaceName $workspace): void
* Computes the number of added, changed and removed nodes for the given workspace
*
* @return array<string,int>
* @throws JsonException
*/
protected function computeChangesCount(Workspace $selectedWorkspace, ContentRepository $contentRepository): array
{
Expand All @@ -749,11 +738,10 @@ protected function computeChangesCount(Workspace $selectedWorkspace, ContentRepo
/**
* Builds an array of changes for sites in the given workspace
* @return array<string,mixed>
* @throws JsonException
*/
protected function computeSiteChanges(Workspace $selectedWorkspace, ContentRepository $contentRepository): array
{
$nodeAddressFactory = NodeAddressFactory::create($contentRepository);

$siteChanges = [];
$changes = $contentRepository->projectionState(ChangeFinder::class)
->findByContentStreamId(
Expand Down Expand Up @@ -792,7 +780,11 @@ protected function computeSiteChanges(Workspace $selectedWorkspace, ContentRepos
$documentPathSegments = [];
foreach ($ancestors as $ancestor) {
$pathSegment = $ancestor->nodeName ?: NodeName::fromString($ancestor->nodeAggregateId->value);
$nodePathSegments[] = $pathSegment;
// Don't include `sites` path as they are not needed
// by the HTML/JS magic and won't be included as `$documentPathSegments`
if (!$this->getNodeType($ancestor)->isOfType(NodeTypeNameFactory::NAME_SITES)) {
$nodePathSegments[] = $pathSegment;
}
if ($this->getNodeType($ancestor)->isOfType(NodeTypeNameFactory::NAME_DOCUMENT)) {
$documentPathSegments[] = $pathSegment;
if (is_null($documentNode)) {
Expand All @@ -808,29 +800,46 @@ protected function computeSiteChanges(Workspace $selectedWorkspace, ContentRepos
// We should probably throw an exception though
if ($documentNode !== null && $siteNode !== null && $siteNode->nodeName) {
$siteNodeName = $siteNode->nodeName->value;
$documentPath = implode('/', array_slice(array_map(
// Reverse `$documentPathSegments` to start with the site node.
// The paths are used for grouping the nodes and for selecting a tree of nodes.
$documentPath = implode('/', array_reverse(array_map(
fn (NodeName $nodeName): string => $nodeName->value,
$documentPathSegments
), 2));
$relativePath = str_replace(
sprintf('//%s/%s', $siteNodeName, $documentPath),
'',
implode('/', array_map(
fn (NodeName $nodeName): string => $nodeName->value,
$nodePathSegments
))
);
)));
// Reverse `$nodePathSegments` to start with the site node.
// The paths are used for grouping the nodes and for selecting a tree of nodes.
$relativePath = implode('/', array_reverse(array_map(
fn (NodeName $nodeName): string => $nodeName->value,
$nodePathSegments
)));
if (!isset($siteChanges[$siteNodeName]['siteNode'])) {
$siteChanges[$siteNodeName]['siteNode']
= $this->siteRepository->findOneByNodeName(SiteNodeName::fromString($siteNodeName));
}

$siteChanges[$siteNodeName]['documents'][$documentPath]['documentNode'] = $documentNode;
// We need to set `isNew` and `isMoved` on document level to make our JS behave as before.
if ($documentNode->equals($node)) {
$siteChanges[$siteNodeName]['documents'][$documentPath]['isNew'] = $change->created;
$siteChanges[$siteNodeName]['documents'][$documentPath]['isMoved'] = $change->moved;
}

// As for changes of type `delete` we are using nodes from the live content stream
// we can't create `serializedNodeAddress` from the node.
// Instead, we use the original stored values.
$nodeAddress = new NodeAddress(
$change->contentStreamId,
$change->originDimensionSpacePoint->toDimensionSpacePoint(),
$change->nodeAggregateId,
$selectedWorkspace->workspaceName
);

$change = [
'node' => $node,
'serializedNodeAddress' => $nodeAddressFactory->createFromNode($node)->serializeForUri(),
'serializedNodeAddress' => $nodeAddress->serializeForUri(),
'isRemoved' => $change->deleted,
'isNew' => false,
'isNew' => $change->created,
'isMoved' => $change->moved,
'contentChanges' => $this->renderContentChanges(
$node,
$change->contentStreamId,
Expand All @@ -848,19 +857,9 @@ protected function computeSiteChanges(Workspace $selectedWorkspace, ContentRepos

ksort($siteChanges);
foreach ($siteChanges as $siteKey => $site) {
/*foreach ($site['documents'] as $documentKey => $document) {
$liveDocumentNode = $liveContext->getNodeByIdentifier($document['documentNode']->getIdentifier());
$siteChanges[$siteKey]['documents'][$documentKey]['isMoved']
= $liveDocumentNode && $document['documentNode']->getPath() !== $liveDocumentNode->getPath();
$siteChanges[$siteKey]['documents'][$documentKey]['isNew'] = $liveDocumentNode === null;
foreach ($document['changes'] as $changeKey => $change) {
$liveNode = $liveContext->getNodeByIdentifier($change['node']->getIdentifier());
$siteChanges[$siteKey]['documents'][$documentKey]['changes'][$changeKey]['isNew']
= is_null($liveNode);
$siteChanges[$siteKey]['documents'][$documentKey]['changes'][$changeKey]['isMoved']
= $liveNode && $change['node']->getPath() !== $liveNode->getPath();
}
}*/
foreach ($site['documents'] as $documentKey => $document) {
ksort($siteChanges[$siteKey]['documents'][$documentKey]['changes']);
}
ksort($siteChanges[$siteKey]['documents']);
}
return $siteChanges;
Expand Down
10 changes: 5 additions & 5 deletions ...Neos/Resources/Private/Partials/Module/Management/Workspaces/ContentChangeAttributes.html
View file
Original file line number Diff line number Diff line change
@@ -1,16 +1,16 @@
{namespace neos=Neos\Neos\ViewHelpers}
<f:if condition="{change.isRemoved}">
<f:then>class="neos-content-change legend-deleted" data-nodepath="{change.node.path}" title="{neos:backend.translate(id: 'workspaces.legend.deleted', source: 'Modules', package: 'Neos.Neos')}"</f:then>
<f:then>class="neos-content-change legend-deleted" data-nodepath="{nodepath}" title="{neos:backend.translate(id: 'workspaces.legend.deleted', source: 'Modules', package: 'Neos.Neos')}"</f:then>
<f:else>
<f:if condition="{change.isNew}">
<f:then>class="neos-content-change legend-created" data-nodepath="{change.node.path}" title="{neos:backend.translate(id: 'workspaces.legend.created', source: 'Modules', package: 'Neos.Neos')}"</f:then>
<f:then>class="neos-content-change legend-created" data-nodepath="{nodepath}" title="{neos:backend.translate(id: 'workspaces.legend.created', source: 'Modules', package: 'Neos.Neos')}"</f:then>
<f:else>
<f:if condition="{change.isMoved}">
<f:then>class="neos-content-change legend-moved" data-nodepath="{change.node.path}" title="{neos:backend.translate(id: 'workspaces.legend.moved', source: 'Modules', package: 'Neos.Neos')}"</f:then>
<f:then>class="neos-content-change legend-moved" data-nodepath="{nodepath}" title="{neos:backend.translate(id: 'workspaces.legend.moved', source: 'Modules', package: 'Neos.Neos')}"</f:then>
<f:else>
<f:if condition="{change.node.hidden}">
<f:then>class="neos-content-change legend-hidden" data-nodepath="{change.node.path}" title="{neos:backend.translate(id: 'workspaces.legend.hidden', source: 'Modules', package: 'Neos.Neos')}"</f:then>
<f:else>class="neos-content-change legend-edited" data-nodepath="{change.node.path}" title="{neos:backend.translate(id: 'workspaces.legend.edited', source: 'Modules', package: 'Neos.Neos')}"</f:else>
<f:then>class="neos-content-change legend-hidden" data-nodepath="{nodepath}" title="{neos:backend.translate(id: 'workspaces.legend.hidden', source: 'Modules', package: 'Neos.Neos')}"</f:then>
<f:else>class="neos-content-change legend-edited" data-nodepath="{nodepath}" title="{neos:backend.translate(id: 'workspaces.legend.edited', source: 'Modules', package: 'Neos.Neos')}"</f:else>
</f:if>
</f:else>
</f:if>
Expand Down
10 changes: 5 additions & 5 deletions Neos.Neos/Resources/Private/Templates/Module/Management/Workspaces/Show.html
View file
Original file line number Diff line number Diff line change
Expand Up @@ -28,11 +28,11 @@
<tbody>
<f:for each="{siteChanges}" as="site">
<f:for each="{site.documents}" key="documentPath" as="document">
<tr class="neos-document" data-nodepath="{document.documentNode.path}" data-ismoved="{f:if(condition: document.isMoved, then: 'true', else: 'false')}" data-isnew="{f:if(condition: document.isNew, then: 'true', else: 'false')}">
<tr class="neos-document" data-nodepath="{documentPath}" data-ismoved="{f:if(condition: document.isMoved, then: 'true', else: 'false')}" data-isnew="{f:if(condition: document.isNew, then: 'true', else: 'false')}">
<f:if condition="{document.changes -> f:count()} > 1">
<f:then>
<td class="check neos-priority1">
<label for="check-document-{document.documentNode.identifier}" class="neos-checkbox"><f:form.checkbox id="check-document-{document.documentNode.identifier}" class="neos-check-document" value="{document.documentNode.identifier}"/><span></span></label>
<label for="check-document-{document.documentNode.nodeAggregateId.value}" class="neos-checkbox"><f:form.checkbox id="check-document-{document.documentNode.nodeAggregateId.value}" class="neos-check-document" value="{document.documentNode.nodeAggregateId.value}"/><span></span></label>
</td>
<td class="neos-priority1 path-caption">
</f:then>
Expand All @@ -56,15 +56,15 @@
</div>
</td>
<td class="neos-action neos-folder">
<i class="fold-toggle fas fa-chevron-up icon-white" data-toggle="fold-{document.documentNode.identifier}"></i>
<i class="fold-toggle fas fa-chevron-up icon-white" data-toggle="fold-{document.documentNode.nodeAggregateId.value}"></i>
</td>
</tr>
<f:for each="{document.changes}" key="relativePath" as="change">
<tr class="neos-change fold-{document.documentNode.identifier} document-{document.documentNode.identifier}" data-nodepath="{change.node.path}" data-ismoved="{f:if(condition: change.isMoved, then: 'true', else: 'false')}" data-isnew="{f:if(condition: change.isNew, then: 'true', else: 'false')}">
<tr class="neos-change fold-{document.documentNode.nodeAggregateId.value} document-{document.documentNode.nodeAggregateId.value}" data-nodepath="{relativePath}" data-ismoved="{f:if(condition: change.isMoved, then: 'true', else: 'false')}" data-isnew="{f:if(condition: change.isNew, then: 'true', else: 'false')}">
<td class="check neos-priority1">
<label for="{change.serializedNodeAddress}" class="neos-checkbox"><f:form.checkbox name="nodes[]" value="{change.serializedNodeAddress}" id="{change.serializedNodeAddress}" /><span></span></label>
</td>
<td id="change-{change.serializedNodeAddress}" {f:render(partial: 'Module/Management/Workspaces/ContentChangeAttributes', arguments: {change: change})} data-neos-toggle="tooltip" data-placement="left" data-container="body">
<td id="change-{change.serializedNodeAddress}" {f:render(partial: 'Module/Management/Workspaces/ContentChangeAttributes', arguments: {change: change, nodepath: relativePath})} data-neos-toggle="tooltip" data-placement="left" data-container="body">
<f:render partial="Module/Management/Workspaces/ContentChangeDiff" arguments="{change: change, contentDimensions: contentDimensions}"/>
</td>
<td class="neos-action">
Expand Down

0 comments on commit fc37b92

Please sign in to comment.