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

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

Renames _hiddenInIndex to hiddenInMenu.

Fixes partially: #4208

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?

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

Open

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

Open

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

Open

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

Draft

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

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

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

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

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

mhsdesign May 9, 2024

mhsdesign commented May 10, 2024

!!! TASK: Rename _hiddenInIndex to hiddenInMenu #4921

Are you sure you want to change the base?

!!! TASK: Rename _hiddenInIndex to hiddenInMenu #4921

Conversation

kitsunet commented Mar 6, 2024 • edited by mhsdesign

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

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

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

Choose a reason for hiding this comment

mhsdesign May 9, 2024

Choose a reason for hiding this comment

mhsdesign commented May 10, 2024

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

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

kitsunet commented Mar 6, 2024 •

edited by mhsdesign

kitsunet commented Mar 11, 2024 •

edited

mhsdesign left a comment •

edited