neos-development-collection icon indicating copy to clipboard operation
neos-development-collection copied to clipboard

BUGFIX: Force direct access on setting node properties in node data similarize

Open dlubitz opened this issue 1 year ago • 5 comments

The creationDateTime has no setters in AbstractNodeData, so the NodeData::similarize can't set them in the target node propery. We need to allow the ObjectAccess::setProperty to force direct access to the class properties.

The lastModificationDateTime was also not copied before this bugfix and shouldn't be copied anyways.

Fixes: #5280

dlubitz avatar Oct 09 '24 15:10 dlubitz

Unfortunately we can't "force direct access" to all properties, as this would skip the usage of the setters for all properties. So we need to distinguish the properties and the way we need to set them.

dlubitz avatar Oct 09 '24 16:10 dlubitz

if i would care for the old nodeData id say do it in line 769 via

$this->creationDateTime = $sourceNode->creationDateTime;

as this should(tm) work due to the scope being inside in one instance, but its time for neos 9 *g

You are absolutely right. But this is also true for all other properties. IDK why this was implemented that way. I didn't want to change that much, as this is just a bugfix.

dlubitz avatar Oct 10 '24 06:10 dlubitz

Now it works as the code suggests. BUT if you do any change to materialize the node, this lastModificationDateTime should be now, right?

Due to this fix, the lastModificationDateTime stays the same as in the original node on the first change and gets updated on every following change. From my POV, this is wrong and we need to remove the lastModificationDateTime here in the similarize.

WDYT?

dlubitz avatar Oct 10 '24 06:10 dlubitz

I would maybe look in how Neos 9 does it? timestamps->lastModified says Date and time a node was last modified in its content stream ... but i dont know how it behaves exactly.

mhsdesign avatar Oct 10 '24 06:10 mhsdesign

I changed the bugfix, so we just copy the creationDateTime.

I assume we don't need to copy the lastModificationDateTime as it always make sence to have the current time as lastModificationDateTimeafter similarize a node. Moreover the code wasn't working anyways... so not change at all 😬

dlubitz avatar Oct 11 '24 14:10 dlubitz