BUGFIX: Recognize removed-state while reducing

[markusguenther]

Please read Issue #3651, to get the problem.

By adding the removed-state as 3rd priority while reducing NodeData results, we should always get the not-removed NodeData, if there are 2 (because of Node-move).

I tested this fix on our projects, and it worked. I can't find something breaking. But since this is deep inside NodeData, I've no idea, if we could get unexpected side-effects on it. Thanks to @Nikdro for the change.

Fixes: #3651

[markusguenther]

I was unable to push to @Nikdro branch; therefore I created a new PR with the changes from Niklas.
#3652

Yesterday I stumbled over a similar issue and after checking the code in the Neos-UI @kitsunet led me to the NodeData and NodeDataRepository. As @Nikdro already described, when we move e.g. a node into the own parent page, we create a shadow node that is marked as removed and we have the regular node that is moved and not marked as removed.

For some reason, the NodeDataRepository chooses the shadow node as all are filtered by identifier. The change from Niklas changes the position and so we get the moved node and not the shadow node. It would be better here to check in the UI if the target node is also the parent node and so not create a shadow node, but as this can also have implications these bugfixes seem to be a nice approach.

Behavoir before this change:

Move-node-into-same-parent-with-bug.-.01.mp4

Behavoir after this change:

Move-node-into-same-parent-with-fix.1.-.01.mp4

The only thing I could find was an issue when you move the node into the parent, publish it, and then move the node into the same parent again, without reloading the page. This does not happen when you reload the page after publishing.
So it seems that we have something wrong in the UI.

Move-node-into-same-parent-with-fix-exception-before-reload.1.-.01.mp4

I will rebase this change to Neos 8.3, and then we can hopefully continue with the discussion here. Seems that the only concern was the missing test :) With Neos 9.0, this should be different anyways.

[markusguenther]

Tested the behavior in Neos 9.0 and there we don’t have the issue. Even the last issue with the exception is not happening as the UI reloads the needed components after publishing. 🥳

[mhsdesign]

i cant say much to the implementation but only see there are no tests 🙈 shouldnt we prefer to prevent moving into the same via ui js code or even just via the move abstract change?

[kitsunet]

This makes sense