-
-
Notifications
You must be signed in to change notification settings - Fork 222
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
BUGFIX: Force direct access on setting node properties in node data similarize #5281
base: 8.3
Are you sure you want to change the base?
BUGFIX: Force direct access on setting node properties in node data similarize #5281
Conversation
e350a91
to
95f164c
Compare
95f164c
to
04763af
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
+1 by reading
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. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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. |
Now it works as the code suggests. BUT if you do any change to materialize the node, this Due to this fix, the WDYT? |
I would maybe look in how Neos 9 does it? |
I changed the bugfix, so we just copy the creationDateTime. I assume we don't need to copy the |
The
creationDateTime
has no setters in AbstractNodeData, so the NodeData::similarize can't set them in the target node propery. We need to allow theObjectAccess::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