Opt-out per entity type / bundle

Thank you for your request. Yes, we see your problem. Let us discuss a proper solution here.

To make this configurable seems to be a proper proposal. This week might be a little busy, but if you are able to provide a first implementation, we would appreciate this.

I will try my best to come up with a first approach in the next few days.

Here is a first working attempt. I decided not to touch the hard-coded list of excluded entity types, and add a configurable list on top of that (used the term disabled for it).
That way we reduce the list of types that can be checked, making it easier to read.

For a start, disabled entity types will be ignored in the automatic (re-)creation of the Digital Signage view modes only. There might be other places where the list could be used as well.

I did not work on disabling bundles yet. At least for the view mode part, it's not necessary, since they get created on entity type level.

For people still on the 2.4.x branch, here's a patch that should apply to it.

There is a conceptual problem in making base field dependent on config (or any other changeable definition) because base fields get added during entity type initialization and get to create a database column. If a base field gets added dynamically later on, an update hook would be required to add that database field. On the other hand, disabling an entity type later on would also require cleaning up the database. However, that then comes with a potential data loss, if somebody disables an entity type mistakenly, that data would then be gone forever.

That's why the concept of this module was done this way: every entity type has that base field for the digital signage entity reference. The cost is so little, as it is just one integer column attached to the entity base table.

If we want to make digital signage support configurable per entity type and/or bundle, then we should consider doing this in the UI only. That would mean that the data structure remains the same as it is today, but we alter the forms in a way, that the fields are only displayed for supported entity types.

I see your point, @jurgenhaas.
That's why I didn't work on removing any base fields, for the exact reason you mentioned.
My main issue currently is, as pointed out in the IS, the automatic creation of view modes, which with the help of this new setting can now be suppressed per entity type, making the config import on another instance much less painful.

That's why I didn't work on removing any base fields, for the exact reason you mentioned.

Well, this code

    return [
      'digital_signage' => $service->fieldDefinition(),
    ];

is now only executed for all enabled entity types. And if that list changes due to a config change, an entity type could end up without that base field where it had it right before that change. And that would cause issues.

That said, we need a static list of entity type for which the base fields get created. That list should never change. In other words, there should be 2 separate lists:

1. all unsupported entity types: this is what have been the disabled entity types before
2. all disabled entity types

The unsupported entity types are of course also disabled entity types, but they can not be in the config list to disable or enable them, because they always need to remain unsupported.

Adressed feedback from #10

Sorry, I didn't get any notification about your latest commit and patch @mrshowerman. Only saw this by chance today and now assigned this issue to myself, hoping I get to the review soon.

Thanks again @mrshowerman for your work on this. The latest version of the MR looked great, and I've added the following things to it:

• Rebased to the latest version of 2.5.x
• Disabled the digital_signage field in entity forms of disabled entity types
• Add a condition to the SQL query to create schedules to exclude disabled entity types. This is important, since otherwise some previously enabled entities could sneak into the schedule even though their entity type got disabled afterwards.
• Fixed tests. This has been done such that the code will require Drupal 10.3 or later, so before merging this, we should create the 2.6.x branch and merge it there then.

Please review these changes again.

Thanks for improving the solution, @jurgenhaas. The changes look good to me!
Not sure if this single feedback is enough to set the issue to RTBC, so leaving as NR.

Thanks @mrshowerman yes, that's good to go for RTBC then. I've just created the 2.6.x branch and will merge this improvement into that.

This is now merged, and all tests are green.

Automatically closed - issue fixed for 2 weeks with no activity.