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

BUGFIX: #3624 Node::getProperty does not always return list for references #4731

Conversation

mhsdesign
Copy link
Member

@mhsdesign mhsdesign commented Nov 12, 2023

EEL can only operate on an array of nodes if the [0] item is a node (simple duck typing)

Instead of fixing eel like proposed here #3946 with this fix we avoid returning non 0 indexed arrays here:

${q(node).property("someReference")}

Currently, this might indeed return an array with holes like [1 => NODE, 2 => NODE, 5 => NODE] if the identifiers in fields 0, 3 and 4 are not resolvable.

Thats because of the "unsafe" array_filter method in resolvePropertyReferences

protected function resolvePropertyReferences(array $value = []): array
which will leave holes in the array.

Using array_filter was introduced with Neos 2.2 so this is technically a regression of 87804e1 ^^

Fixes: #3624

…eferences

EEL can only operate on an array of nodes if the `[0]` item is a node (simple duck typing)

Instead of fixing eel like proposed here neos#3946 with this fix we avoid returning non 0 indexed arrays here:

```
${q(node).property("someReference")}
```

Currently, this might indeed return an array with holes like `[1 => NODE, 2 => NODE, 5 => NODE]` if the identifiers in fields 0, 3 and 4 are not resolvable.

Thats because of the "unsafe" `array_filter` method in `resolvePropertyReferences` https://github.com/neos/neos-development-collection/blob/378a029d0cc7ea6acb853751e7592873584a4aac/Neos.ContentRepository/Classes/Domain/Model/Node.php#L961 which will leave holes in the array.

Using `array_filter` was introduced with Neos 2.2 so this is technically a regression of neos@87804e1 ^^
@bwaidelich
Copy link
Member

This one begs for a corresponding test

Copy link
Member

@bwaidelich bwaidelich left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I took the freedom to add a unit test.
Also suggested a slightly different approach, but that's just a FYI really

@mhsdesign mhsdesign changed the base branch from 7.3 to 8.3 January 31, 2024 13:29
@github-actions github-actions bot added 8.3 and removed 7.3 labels Jan 31, 2024
@mhsdesign mhsdesign merged commit 0a30a0f into neos:8.3 Jan 31, 2024
9 checks passed
@mhsdesign mhsdesign deleted the bugfix/nodeGetPropertyReturnsArrayWithHolesForReferences branch January 31, 2024 13:42
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants