neos / neos-development-collection

The unified repository containing the Neos core packages, used for Neos development.
https://www.neos.io/
GNU General Public License v3.0
264 stars 222 forks source link

Disallow disabling tethered nodes #4821

Open mhsdesign opened 10 months ago

mhsdesign commented 10 months ago

When hiding the main content collection - via php api: (which already sounds baaaaad) an error will be thrown:

No content collection of type Neos.Neos:ContentCollection could be found in the current node (/[99f81405-8781-4f3b-a6ba-8606a8be66d6]) or at the path "main"

So i think we should enforce that this can never happen via constraint checks.

Related discussion https://github.com/neos/neos-ui/pull/3004#pullrequestreview-1808255932 Related https://github.com/neos/neos-ui/issues/2282

kitsunet commented 10 months ago

Not sure if I should cry or laugh 🤣

nezaniel commented 10 months ago

This should never have been possible and must be fixed. Tethered nodes are considered an integral part of their parent and thus must not be disabled separately

kitsunet commented 10 months ago

Many people disagree with you (and me) - see discussion in the linked ticket

nezaniel commented 10 months ago

I'm referring to 9.0 where it was decided this way iirc

mhsdesign commented 10 months ago

Okay a compromise would be to implement the suggested configuration from https://github.com/neos/neos-ui/pull/3004

'Some.Package:Some.NodeType':
  childNodes:
    'childnode1':
      type: 'Some.Package:Some.NodeType'
      hideable: true

and depending on this hideable option allow - on a cr level - to hide a tethered node.

Buuut we should rather call it canBeDisabled or the like to fit the disabled naming.

nezaniel commented 10 months ago

I think if this turns into a discussion we should by all means merge it with the other thread

mhsdesign commented 10 months ago

okay yes makes sense not to split it up to heavily ;)

mhsdesign commented 10 months ago

As discussed here https://github.com/neos/neos-ui/pull/3004#issuecomment-1881735976 we want to implement constraint checks in neos 9 to disallow this for once and for all.

mhsdesign commented 7 months ago

@nezaniel and me discussed how this change will be affected by the introduction of the subtree tags feature. As disabled is just a tag itself. We came to the conclusion that the argumentation, that a tree should never be invalid, despite which exclusion is configured via the VisibiltiyConstraints and that will include any custom tag.

So it will NOT be possible to tag a tethered node blog with a blog tag for example.

(Additionally we discussed that we have to prevent DisableNodeAggregate but also EnableNodeAggregate.)