!!! TASK: Rename `_hiddenInIndex` to `hiddenInMenu` #4921

kitsunet · 2024-03-06T18:44:35Z

Renames _hiddenInIndex to hiddenInMenu.

This was done and decided in order to minimise confusion regarding magic _* underscore properties.
The property _hiddenInIndex was such underscore property in 8.3 but as with Neos 9 Node::isHiddenInIndex has been removed. As non breaking temporary replacement it was reimplemented as actual node property except with an underscore prefix: _hiddenInIndex. This is possible as the core doesn't work with these underscore properties anymore.
In Neos 9 there will simply be no magic meaning for underscore properties anymore, any property will now be allowed to start with an underscore. To better communicate this - but not enforce the use of such - Neos.Neos will refrain from declaring Node properties with underscores as this is an overloaded feature of the past and can easily cause confusion.

Fixes partially: #4208

Upgrade instructions

Following Fusion prototypes no longer accept an renderHiddenInIndex option but only consider renderHiddenInMenu. In the case you overwrote the default behaviour for any of these prototypes you should rename the option to renderHiddenInMenu.

Neos.Neos:BreadcrumbMenu
Neos.Neos:BreadcrumbMenuItems
Neos.Neos:DimensionsMenu
Neos.Neos:DimensionsMenuItems
Neos.Neos:Menu
Neos.Neos:MenuItems

Accessing the property has to be adjusted as well.

Upgrade from 8.3

- $node->isHiddenInIndex();
+ $node->getProperty('hiddenInMenu');

upgrade from 9.x-dev

- $node->getProperty('_hiddenInIndex');
+ $node->getProperty('hiddenInMenu');

Review instructions

kitsunet · 2024-03-07T09:26:08Z

unrelated errors 🤷

bwaidelich · 2024-03-10T11:51:08Z

unrelated errors 🤷

@kitsunet Fixed with #4924 and rebased

bwaidelich

Just one comment for now. Apart from that, the PR looks great!
You mentioned that there are follow-up todos for Neos UI?

Neos.Neos/Classes/Fusion/Helper/NodeHelper.php

ahaeslich · 2024-03-10T12:15:19Z

You mentioned that there are follow-up todos for Neos UI?

Looking at this PR I noticed Neos.Seo may also need adjustments. So checking our codebase I see that:

the legacy migration should rewrite hiddenInIndex to hiddenInMenu
translations must use the new name
the UI uses _hiddenInIndex
there are comments referencing the prop
our rector package needs adjustments
Neos.Demo needs adjustments in fusion and with the exported content

The internal property `hiddenInIndex` was renamed to `hiddenInMenu`. To match this renaming the fusion property `renderHiddenInIndex` is renamed to `renderHiddenInMenu`. Needs: neos/neos-development-collection#4921

kitsunet · 2024-03-10T17:57:27Z

Rector btw. is here neos/rector#45

kitsunet · 2024-03-11T11:55:25Z

the legacy migration should rewrite hiddenInIndex to hiddenInMenu
translations must use the new name

kitsunet · 2024-03-11T12:16:38Z

These testfailures seem again unreltaed?

kitsunet · 2024-03-11T12:26:07Z

ok interesting, those tests are from a recently merged PR in 9.0, but why do they show up here given that they are not in my branch?

Note this is because we get the "merge" PR branch in tests that is actually a representation of 9.0 HEAD + this PR branch and not just plainly this PR branch.

kitsunet · 2024-03-11T13:27:53Z

FYI: Karsten said just renaming the labels should be fine.

kitsunet · 2024-03-11T17:47:21Z

Neos.Demo: neos/Neos.Demo#192

nezaniel · 2024-04-16T21:32:57Z

I'd like to help here but can't get PHPStorm to detect the merge conflicts...

kitsunet · 2024-04-17T07:51:09Z

I can try to get it back on track, somehow I thought iut was long merged 🙈

kitsunet · 2024-04-19T05:23:11Z

neos-ui change: neos/neos-ui#3735
this fixes disabled tag export as well

nezaniel · 2024-04-21T10:16:15Z

what parts are still missing here? All that I can see looks very nice already

kitsunet · 2024-04-22T07:26:51Z

what parts are still missing here? All that I can see looks very nice already

Nothing IMHO, it should all be merged please 😆

mhsdesign

Thanks for taking care.
To solve the issue #4208 fully we have to get rid of ObjectAccess in the property operation as well and possibly also in other places.

But instead of adding this into this pr and bloating it up i would like to suggest to keep this pr super simple and only related to _hiddenInIndex -> hiddenInMenu that way its much more structured.

All the general underscore _ logic could then be removed in one batch with another pr. What do you say? I might also be able to help with that.

Neos.ContentRepository.NodeAccess/Classes/FlowQueryOperations/FilterOperation.php

Neos.Neos/Classes/Fusion/Helper/NodeHelper.php

nezaniel

LGTM, even with the unrelated changes

Provides a helper to access hidden status as well as renaming `_hiddenInIndex` to `hiddenInMenu`. Fixes: neos#4208

mhsdesign · 2024-04-26T08:43:53Z

Extracted everything i though this pr was originally meant to be into #5015 and added the things i found missing.

This pr now is super slim and only addresses the hiddenInMenu part.
@kitsunet i think we might need a better description yet (also maybe for the WHY we did this).
Rector seems already to be adjusted: neos/rector#45

Followups:

mhsdesign

Can be merged after a nice description has been added ;)

Benjamin-K · 2024-05-09T05:19:05Z

Neos.Neos/Classes/Fusion/AbstractMenuItemsImplementation.php

-        if ($this->renderHiddenInIndex === null) {
-            $this->renderHiddenInIndex = (bool)$this->fusionValue('renderHiddenInIndex');
+        if ($this->renderHiddenInMenu === null) {
+            $this->renderHiddenInMenu = (bool)($this->fusionValue('renderHiddenInMenu') ?? $this->fusionValue('renderHiddenInIndex'));


Do we really want to support the old name, too? Would be cleaner to have a code migration for that as otherwise people will still keep on using the old name and wonder, why there is no corresponding yaml definition.

Hi ;) yes good catch ... we do have working migrations regarding eel but fusion property names cannot be that easily migrated ... thus a super slim b/c layer like this might not hurt?

But we should adjust the docs to reference this ;)

Is there a way to show an error to developers for this? Sth like:

if ($this->fusionValue('renderHiddenInIndex') !== null) { throw new Exception(...); }

I know, this has small performance drawbacks, but I would prefer a clean migration personally, as this could lead to confusion in the long run. And the warning can be removed in a later version.

Okay yes we could go down this road and additionally consider doing a string replace in all fusion files ;)
Thats a good idea i agree. Wdyt @kitsunet?

I wouldn't mind that, I think here was some opposition against throwing errors in fusion implementations as usage can be pretty conditional and you might not notice it before bringing in live.

Okay i just removed it without exceptions thrown ^^ lets see how we can migrate this 🙈 ?

mhsdesign

Oh i just found out that the docs must be adjusted as well:

neos-development-collection/Neos.Neos/Documentation/References/NeosFusionReference.rst

Line 1110 in 7faa3c8

    
           :renderHiddenInIndex: (boolean, default **true**) If TRUE, render nodes which are marked as "hidded-in-index"

And there are still a lot of references to hiddenInIndex (especially in fusion code, docs and old test code)

The test old legacy test code referencing this name will be removed via my pr here: #5036

mhsdesign · 2024-05-09T07:34:32Z

Neos.Neos/Classes/Fusion/AbstractMenuItemsImplementation.php

-        if ($this->renderHiddenInIndex === null) {
-            $this->renderHiddenInIndex = (bool)$this->fusionValue('renderHiddenInIndex');
+        if ($this->renderHiddenInMenu === null) {
+            $this->renderHiddenInMenu = (bool)($this->fusionValue('renderHiddenInMenu') ?? $this->fusionValue('renderHiddenInIndex'));


Hi ;) yes good catch ... we do have working migrations regarding eel but fusion property names cannot be that easily migrated ... thus a super slim b/c layer like this might not hurt?

But we should adjust the docs to reference this ;)

…ndex`

mhsdesign · 2024-05-10T08:35:27Z

Okay i pushed a few commits adjusting fusion and documentation further. I think there are now no references left.
This is now even more breaking as there is no fallback layer in Fusion thus we need a good upgrade instruction in the pr and maybe rector?

…nal-node-properties

kdambekalns · 2024-05-14T14:26:12Z

This broke the XLIFF files, Weblate says Opening and ending tag mismatch: body line 4 and trans-unit, line 20, column 20 (<string>, line 20) for Neos.Neos/Resources/Private/Translations/*/NodeTypes/Document.xlf

dlubitz · 2024-05-14T14:34:45Z

Looks like to many </trans-unit> tags.

kdambekalns · 2024-05-14T14:38:27Z

Yep, exactly:

See neos#4921

See neos/neos-development-collection#4921

mhsdesign · 2024-05-19T13:33:04Z

Fyi we missed to include some followups in the beta10 and might also have to consider providing a core node migration or a snipped to be copied (especially looking at @ahaeslich and her project as well as the docs and @nezaniel starship?)

ahaeslich · 2024-06-05T14:22:40Z

Fyi we missed to include some followups in the beta10 and might also have to consider providing a core node migration or a snipped to be copied (especially looking at @ahaeslich and her project as well as the docs and @nezaniel starship?)

🤔 Did you discuss a core migration for this in a recent weekly? I'm undecided here ... How about all those users testing the beta releases?

github-actions bot added Task 9.0 labels Mar 6, 2024

kitsunet mentioned this pull request Mar 7, 2024

TASK: Adjust to changes in Neos removing internal node properties neos/neos-ui#3735

Merged

bwaidelich reviewed Mar 10, 2024

View reviewed changes

Neos.Neos/Classes/Fusion/Helper/NodeHelper.php Outdated Show resolved Hide resolved

ahaeslich mentioned this pull request Mar 10, 2024

TASK: Adjust usage of hiddenInIndex to use hiddenInMenu neos/neos-seo#183

Merged

mhsdesign self-requested a review April 25, 2024 08:34

mhsdesign reviewed Apr 25, 2024

View reviewed changes

Neos.ContentRepository.NodeAccess/Classes/FlowQueryOperations/FilterOperation.php Show resolved Hide resolved

Neos.Neos/Classes/Fusion/Helper/NodeHelper.php Outdated Show resolved Hide resolved

nezaniel approved these changes Apr 25, 2024

View reviewed changes

kitsunet added 3 commits April 26, 2024 10:09

TASK: Remove "internal" node properties

ebc0ddf

Provides a helper to access hidden status as well as renaming `_hiddenInIndex` to `hiddenInMenu`. Fixes: neos#4208

Behat fix for menu rendering with hiddenInMenu

f9808e2

Adjust legacy migration and labels to hiddenInMenu

565bff1

mhsdesign force-pushed the task/4208-remove-internal-node-properties branch from 05e6552 to 565bff1 Compare April 26, 2024 08:10

mhsdesign changed the title ~~TASK: Remove "internal" node properties~~ !!! TASK: Rename _hiddenInIndex to hiddenInMenu Apr 26, 2024

This was referenced Apr 26, 2024

TASK: Adjust to rewrite of _hiddenInIndex to hiddenInMenu neos/Neos.Demo#192

Merged

!!! TASK: Remove leftovers from magic underscore access for internal node properties #5015

Merged

mhsdesign approved these changes May 4, 2024

View reviewed changes

Benjamin-K reviewed May 9, 2024

View reviewed changes

mhsdesign requested changes May 9, 2024

View reviewed changes

mhsdesign and others added 8 commits May 9, 2024 09:44

TASK: Adjust comment in legacy migration

0c47965

Adjust documentation

c9a3d1a

TASK: Adjust fusion inline documentation to hiddenInMenu

8f6c36f

TASK: Adjust fusion dimension menu documentation to hiddenInMenu

019b29e

TASK: Adjust fusion default value declaration to use `renderHiddenInI…

ab7a4d7

…ndex`

TASK: Remove obsolete constants from AbstractMenuItemsImplementation

3c1f5f5

TASK: Adjust passed fusion properties to renderHiddenInMenu

36a913d

TASK: Remove fallback layer for renderHiddenInIndex in Fusion

71609dd

mhsdesign requested a review from mficzel May 10, 2024 08:40

Merge remote-tracking branch 'origin/9.0' into task/4208-remove-inter…

efcfb6b

…nal-node-properties

mhsdesign approved these changes May 14, 2024

View reviewed changes

kitsunet merged commit 8ad2c3b into neos:9.0 May 14, 2024
9 of 13 checks passed

kitsunet deleted the task/4208-remove-internal-node-properties branch May 14, 2024 13:56

kdambekalns added a commit to kdambekalns/neos-development-collection that referenced this pull request May 14, 2024

BUGFIX: Fix broken XLIFF files

a8741f9

See neos#4921

kdambekalns mentioned this pull request May 14, 2024

BUGFIX: Fix broken XLIFF files #5053

Merged

6 tasks

neos-bot pushed a commit to neos/neos that referenced this pull request May 14, 2024

BUGFIX: Fix broken XLIFF files

be09777

See neos/neos-development-collection#4921

mhsdesign mentioned this pull request Jun 12, 2024

List of (integration) Fusion, Yaml etc things that cannot be easily migrated automatically neos/rector#67

Open

mhsdesign mentioned this pull request Jun 25, 2024

BUGFIX: #65 migrate hiddenInMenu correctly neos/rector#72

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

!!! TASK: Rename `_hiddenInIndex` to `hiddenInMenu` #4921

!!! TASK: Rename `_hiddenInIndex` to `hiddenInMenu` #4921

kitsunet commented Mar 6, 2024 •

edited by mhsdesign

Loading

kitsunet commented Mar 7, 2024

bwaidelich commented Mar 10, 2024

bwaidelich left a comment

ahaeslich commented Mar 10, 2024

kitsunet commented Mar 10, 2024

kitsunet commented Mar 11, 2024

kitsunet commented Mar 11, 2024

kitsunet commented Mar 11, 2024 •

edited

Loading

kitsunet commented Mar 11, 2024

kitsunet commented Mar 11, 2024

nezaniel commented Apr 16, 2024

kitsunet commented Apr 17, 2024

kitsunet commented Apr 19, 2024

nezaniel commented Apr 21, 2024

kitsunet commented Apr 22, 2024

mhsdesign left a comment

nezaniel left a comment

mhsdesign commented Apr 26, 2024 •

edited by ahaeslich

Loading

mhsdesign left a comment

Benjamin-K May 9, 2024

mhsdesign May 9, 2024

Benjamin-K May 9, 2024

mhsdesign May 9, 2024

kitsunet May 9, 2024

mhsdesign May 10, 2024

mhsdesign left a comment •

edited

Loading

mhsdesign May 9, 2024

mhsdesign commented May 10, 2024

kdambekalns commented May 14, 2024

dlubitz commented May 14, 2024

kdambekalns commented May 14, 2024

mhsdesign commented May 19, 2024

ahaeslich commented Jun 5, 2024

!!! TASK: Rename _hiddenInIndex to hiddenInMenu #4921

!!! TASK: Rename _hiddenInIndex to hiddenInMenu #4921

Conversation

kitsunet commented Mar 6, 2024 • edited by mhsdesign Loading

kitsunet commented Mar 7, 2024

bwaidelich commented Mar 10, 2024

bwaidelich left a comment

Choose a reason for hiding this comment

ahaeslich commented Mar 10, 2024

kitsunet commented Mar 10, 2024

kitsunet commented Mar 11, 2024

kitsunet commented Mar 11, 2024

kitsunet commented Mar 11, 2024 • edited Loading

kitsunet commented Mar 11, 2024

kitsunet commented Mar 11, 2024

nezaniel commented Apr 16, 2024

kitsunet commented Apr 17, 2024

kitsunet commented Apr 19, 2024

nezaniel commented Apr 21, 2024

kitsunet commented Apr 22, 2024

mhsdesign left a comment

Choose a reason for hiding this comment

nezaniel left a comment

Choose a reason for hiding this comment

mhsdesign commented Apr 26, 2024 • edited by ahaeslich Loading

mhsdesign left a comment

Choose a reason for hiding this comment

Benjamin-K May 9, 2024

Choose a reason for hiding this comment

mhsdesign May 9, 2024

Choose a reason for hiding this comment

Benjamin-K May 9, 2024

Choose a reason for hiding this comment

mhsdesign May 9, 2024

Choose a reason for hiding this comment

kitsunet May 9, 2024

Choose a reason for hiding this comment

mhsdesign May 10, 2024

Choose a reason for hiding this comment

mhsdesign left a comment • edited Loading

Choose a reason for hiding this comment

mhsdesign May 9, 2024

Choose a reason for hiding this comment

mhsdesign commented May 10, 2024

kdambekalns commented May 14, 2024

dlubitz commented May 14, 2024

kdambekalns commented May 14, 2024

mhsdesign commented May 19, 2024

ahaeslich commented Jun 5, 2024

!!! TASK: Rename `_hiddenInIndex` to `hiddenInMenu` #4921

!!! TASK: Rename `_hiddenInIndex` to `hiddenInMenu` #4921

kitsunet commented Mar 6, 2024 •

edited by mhsdesign

Loading

kitsunet commented Mar 11, 2024 •

edited

Loading

mhsdesign commented Apr 26, 2024 •

edited by ahaeslich

Loading

mhsdesign left a comment •

edited

Loading