Skip to content
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

WONT FIX? Inconsistency setProperty(null) on node will not be stored in some cases #4292

Closed
mhsdesign opened this issue May 24, 2023 · 1 comment

Comments

@mhsdesign
Copy link
Member

mhsdesign commented May 24, 2023

About the old CR. Neos 8.3

If you have a node with those properties defined:

properties:
  withDefault:
    defaultValue: false
    type: boolean
  withoutDefault:
    type: boolean
// will be stored (eg $node->hasProperty())
$node->setProperty("withDefault", null);

// will not be stored (eg $node->hasProperty())
$node->setProperty("withoutDefault", null);

// will also not be stored (eg $node->hasProperty())
$node->setProperty("undeclaredProperty", null);

// will be stored (eg $node->hasProperty())
$node->setProperty("anotherUndeclaredProperty", "hi");
$node->setProperty("anotherUndeclaredProperty", null);

the cause for this behavior is this guard:

if (!is_object($value) && !is_array($value) && $this->getProperty($propertyName) === $value) {
return;
}

we should also make sure that $node->hasProperty in that case.

But i dont think its worth bugfixing this minor inconsistency and this might never be a real problem.
We just wondered what was going on @Sebobo

@mhsdesign
Copy link
Member Author

mhsdesign commented Jun 6, 2023

in neos 9 the handling of null is much more consistent. As null always works as a unset. (at least it does now after #4322)

  • NOTE: if a value is set to NULL in SerializedPropertyValues, this means the key should be unset,
  • because we treat NULL and "not set" the same from an API perspective.

* NOTE: if a value is set to NULL in SerializedPropertyValues, this means the key should be unset,
* because we treat NULL and "not set" the same from an API perspective.

implementation:

the merge will also do an array_filter:

$node->properties = $node->properties->merge($event->propertyValues);

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

1 participant