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

Disallow disabling tethered nodes #4821

Open
mhsdesign opened this issue Jan 8, 2024 · 9 comments · May be fixed by #4938
Open

Disallow disabling tethered nodes #4821

mhsdesign opened this issue Jan 8, 2024 · 9 comments · May be fixed by #4938
Assignees
Labels

Comments

@mhsdesign
Copy link
Member

mhsdesign commented Jan 8, 2024

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 neos/neos-ui#3004 (review)
Related neos/neos-ui#2282

@mhsdesign mhsdesign added the 9.0 label Jan 8, 2024
@kitsunet
Copy link
Member

kitsunet commented Jan 8, 2024

Not sure if I should cry or laugh 🤣

@nezaniel
Copy link
Member

nezaniel commented Jan 8, 2024

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
Copy link
Member

kitsunet commented Jan 8, 2024

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

@nezaniel
Copy link
Member

nezaniel commented Jan 8, 2024

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

@mhsdesign
Copy link
Member Author

Okay a compromise would be to implement the suggested configuration from neos/neos-ui#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
Copy link
Member

nezaniel commented Jan 8, 2024

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

@mhsdesign
Copy link
Member Author

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

@mhsdesign mhsdesign closed this as not planned Won't fix, can't repro, duplicate, stale Jan 8, 2024
@mhsdesign
Copy link
Member Author

As discussed here neos/neos-ui#3004 (comment) we want to implement constraint checks in neos 9 to disallow this for once and for all.

@mhsdesign mhsdesign reopened this Jan 12, 2024
@mhsdesign mhsdesign moved this to Prioritized 🔥 in Neos 9.0 Release Board Jan 12, 2024
@nezaniel nezaniel self-assigned this Mar 13, 2024
@nezaniel nezaniel moved this from Prioritized 🔥 to In Progress 🚧 in Neos 9.0 Release Board Mar 13, 2024
@nezaniel nezaniel linked a pull request Mar 13, 2024 that will close this issue
@mhsdesign
Copy link
Member Author

@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.)

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

Successfully merging a pull request may close this issue.

3 participants