Provide entity context and limit to Entity types which only have View Builder Class

I'm honestly confused by the issue title and proposal/patch. I guess I expected this line:

if (!$entity_type->hasViewBuilderClass()) {

to be the other way around, so

if ($entity_type->hasViewBuilderClass()) {

What am I missing?

You are right, Kristof
The title is quite cofiusing. Had time to well write it.
It had me too, maybe a messing not

If a custom entity type like a custom Dashboard entity type is only using the Layout Builder .. not the View Builder
And after 🐛 Provide entity context to DS block fields Fixed
This will provide a context to all entity types, which is bring some issues on them like
🐛 Error: Call to undefined method ::isOverriden() Closed: duplicate

Tested to list all entities which DS is playing with thier context by print_r($entity_type_ids); or kint($entity_type_ids);

I did not want to hard code a fix for a selected entity type.

Proposed a patch to only limit entity types which are not uisng the view builder, as they are using some other ways like
Dashboards with Layout Builder →

Still maybe the logic in 🐛 Provide entity context to DS block fields Fixed is good, but if a custom or a contrib module is using other ways, It may conflict with them.

Will having a DS limit context on entity types and selected view mode will help?

Maybe the following is the right fix for this. but needs a bit of coding.
If you like that, we could add a new DS settings config and config form to manage:
1. Force the DS context on all - ( current logic )
2. Limit with if (!$entity_type->hasViewBuilderClass()) { ( current MR fix )
3. Or Limit with selected entity types ( ds config + ds settings tab )
4. Maybe add more limits to selected view modes ( some view modes are using ( expermenting with a switch or to mix with ) the Layout Builder, SDC, SDC Display, UI Patterns, ... others )

I think I like option 2, but the other way around: by default we generate for everything, but the configuration allows you to exclude the entity types. That way, an upgrade doesn't break anything (even though the feature is very new anyway), and users can configure it without having to think about what they need to include?

Noted;

Option #2: Limit with selected entity types ( ds config + ds settings tab )

+ hook update
I agree to your direction. It it the better option.
WIP for that .....

Thanks, Kristof, for your time and following up :)

The current patch fixes the error from 🐛 Version 8.x-3.18 breaks front end of website when upgrading from 3.16 with Dashboards with Layout Builder installed. Active - even though it's counter-intuitive in my mind :) It shows that I need to dive in this context system even though it's already so old but I clearly don't get in touch with it much :)

So, I'm currently extremely tempted to revert the patch from 🐛 Provide entity context to DS block fields Fixed and get back to the drawing board to see if we can make the context provider smarter, as in, only provide context when it's needed for a DS block field (if that's even possible), and also making sure it doesn't crash. Does that sound like a plan?

I've reverted that patch because I'm having issues on other sites as well, let's try again :)

Hello @swentel
I want to point out that the added contexts have an effect on which block visibility plugins are available in the block UI, for _all_ blocks.

This could have nasty effects if:

• User installs an earlier version of ds which adds a bunch of contexts.
• User configures block visibility based on bundles of an entity type that would otherwise not be available for block visibility configuration. E.g. media.
• Use upgrades ds to a version which no longer adds these contexts, or adds different contexts.
• The block visibility config does not work anymore (?).

Actually I have not really tried how bad it will be, but it should be considered.