search

backdrop-contrib / entity_plus

This module wraps in a variety of additional entity-related functionality from various sources. Partial port of D7 Entity API.
https://backdropcms.org/project/entity_plus
GNU General Public License v2.0
3 stars 11 forks source link

Merging Entity UI into Entity Plus #141

Open argiepiano opened 2 years ago

argiepiano commented 2 years ago

This is the counterpart issue to https://github.com/backdrop-contrib/entity_ui/issues/16.

There has been some conversation on Zulip about merging back Entity UI into Entity Plus. It would make sense, since these two modules are closely integrated - it's rare to see Entity UI used by itself.

Originally, these two modules were split from D7 Entity API. When some of Entity API made it into Backdrop core, the remaining parts (all the UI functionality, as well as entity metadata wrappers, etc) were ported as these two modules.

laryn commented 2 years ago

I think it makes sense to merge these two together.

docwilmot commented 2 years ago

Which is the best way?

argiepiano commented 2 years ago

Since the idea is to deprecate Entity UI, I'm leaning toward the second option:

merge the functionality of UI into Plus, change function names to entity_plus_xxx()

but keeping the ui in the function names (as in entity_plus_ui_xxx(), as done in the D7 module, to indicate the different functionality. I'd suggest we put most of the functions in entity_plus_ui.inc as done in D7. We will also need to eliminate a couple of duplications (the constants, for example, and entity_ui_access(), which is covered by entity_plus_access() until the B's core entity_access() is fixed and revamped - an issue that has been stuck for a long time).

Merging the functionality into Entity Plus will make it easier to automatically disable the deprecated Entity UI without the extra step of enabling the new Entity UI submodule. It will also avoid name collisions (produced by the first option, which keeps the function names) in case something goes wrong with the automatic disabling of the deprecated Entity UI (those name collisions can be nasty and VERY difficult to deal with, since you basically can't bootstrap and disable the offending modules - you have to disable directly into the DB).

This will also make it easier to upgrade from D7 - you only have to worry about one stub module instead of two.

argiepiano commented 2 years ago

I think we should keep the names of the Entity UI controllers (e.g. EntityDefaultUIController), to simplify the move. This way contrib modules that currently extend those classes won't have to worry about changing those.