Replace farmOS hooks with Events + EventSubscribers

We implemented some Events during the 2.x development cycle for some of core farmOS entities (asset, quantity, and data_stream) that allow us to use EventSubscribers instead of core entity hooks like _presave(), _insert(), _update(), _delete(): #3306227: Dispatch events for asset presave, insert, update, delete →

(Worth noting it looks like we didn't add that for plan entities... so maybe worth tacking on a commit for that with these changes at the same time. Are there other core farmOS entities that should have the same?)

Drupal core has an open issue that will ultimately provide the same for all entities automatically (which will allow us to remove our own Events): 📌 Add events for matching entity hooks Needs work

I took a pass through all modules included with farmOS and documented both hooks and Events that we currently provide. See the updated issue description for the full list.

This was my methodology for finding them:

$ grep -irn 'extends Event' .
./modules/core/map/src/Event/MapRenderEvent.php:14:class MapRenderEvent extends Event {
./modules/core/quantity/src/Event/QuantityEvent.php:11:class QuantityEvent extends Event {
./modules/core/data_stream/src/Event/DataStreamEvent.php:11:class DataStreamEvent extends Event {
./modules/core/asset/src/Event/AssetEvent.php:11:class AssetEvent extends Event {

$ grep -irn '@addtogroup hooks' .
./modules/core/entity/farm_entity.api.php:13: * @addtogroup hooks
./modules/core/api/farm_api.api.php:13: * @addtogroup hooks
./modules/core/ui/theme/farm_ui_theme.api.php:13: * @addtogroup hooks
./modules/core/ui/dashboard/farm_ui_dashboard.api.php:13: * @addtogroup hooks
./modules/core/update/farm_update.api.php:13: * @addtogroup hooks

$ grep -irn 'hook_farm_' .
./CHANGELOG.md:375:- [Correct hook_farm_update_exclude_config API docs #608](https://github.com/farmOS/farmOS/pull/608)
./docs/development/module/fields.md:59:`hook_farm_entity_bundle_field_info()`*
./docs/development/module/fields.md:80: * Implements hook_farm_entity_bundle_field_info().
./docs/development/module/updates.md:58:#### `hook_farm_update_exclude_config()`
./docs/development/module/updates.md:63:implementation of `hook_farm_update_exclude_config()`.
./docs/development/module/updates.md:69: * Implements hook_farm_update_exclude_config().
./modules/core/entity/tests/src/Kernel/FarmEntityFieldTest.php:118:   * Test farmOS fields defined in hook_farm_entity_bundle_field_info().
./modules/core/entity/tests/src/Kernel/FarmEntityFieldTest.php:181:    // bundle fields that were provided by hook_farm_entity_bundle_field_info().
./modules/core/entity/tests/modules/farm_entity_test/farm_entity_test.module:29: * Implements hook_farm_entity_bundle_field_info().
./modules/core/entity/tests/modules/farm_entity_contrib_test/farm_entity_contrib_test.module:11: * Implements hook_farm_entity_bundle_field_info().
./modules/core/entity/src/BundlePlugin/FarmEntityBundlePluginHandler.php:12: * Extends BundlePluginHandler to invoke hook_farm_entity_bundle_field_info().
./modules/core/entity/farm_entity.api.php:30:function hook_farm_entity_bundle_field_info(\Drupal\Core\Entity\EntityTypeInterface $entity_type, string $bundle) {
./modules/core/entity/farm_entity.module:80:  // Invoke hook_farm_entity_bundle_field_info() with each bundle.
./modules/core/api/farm_api.api.php:23:function hook_farm_api_meta_alter(array &$data) {
./modules/core/quantity/quantity.module:111: * Implements hook_farm_api_meta_alter().
./modules/core/ui/views/farm_ui_views.module:169: * Implements hook_farm_dashboard_groups().
./modules/core/ui/views/farm_ui_views.module:190: * Implements hook_farm_dashboard_panes().
./modules/core/ui/metrics/farm_ui_metrics.module:9: * Implements hook_farm_dashboard_panes().
./modules/core/ui/theme/farm_ui_theme.api.php:36:function hook_farm_ui_theme_region_items(string $entity_type) {
./modules/core/ui/theme/farm_ui_theme.module:175: * Implements hook_farm_ui_theme_region_items().
./modules/core/ui/theme/farm_ui_theme.module:241: * Implements hook_farm_update_exclude_config().
./modules/core/ui/dashboard/tests/modules/dashboard_test/farm_ui_dashboard_test.module:9: * Implements hook_farm_dashboard_panes().
./modules/core/ui/dashboard/farm_ui_dashboard.api.php:23:function hook_farm_dashboard_panes() {
./modules/core/ui/dashboard/farm_ui_dashboard.api.php:66:function hook_farm_dashboard_groups() {
./modules/core/ui/map/farm_ui_map.module:12: * Implements hook_farm_dashboard_panes().
./modules/core/update/tests/src/Kernel/FarmUpdateTest.php:60:    // Confirm that config excluded via hook_farm_update_exclude_config() does
./modules/core/update/tests/modules/farm_update_test/farm_update_test.module:9: * Implements hook_farm_update_exclude_config().
./modules/core/update/farm_update.api.php:23:function hook_farm_update_exclude_config() {
./modules/core/update/farm_update.module:16: * Implements hook_farm_update_exclude_config().
./modules/core/quick/farm_quick.module:38: * Implements hook_farm_entity_bundle_field_info().
./modules/asset/sensor/modules/listener/farm_sensor_listener.module:16: * Implements hook_farm_entity_bundle_field_info().
./modules/asset/group/farm_group.module:58: * Implements hook_farm_ui_theme_region_items().
./modules/asset/equipment/farm_equipment.module:11: * Implements hook_farm_entity_bundle_field_info().

For context, we are considering adding two new hooks to the farm_ui_theme module, which is what prompted this issue.

Relevant comments from the PR (https://github.com/farmOS/farmOS/pull/770):

• https://github.com/farmOS/farmOS/pull/770#issuecomment-1894810641
• https://github.com/farmOS/farmOS/pull/770#issuecomment-1895631832
• https://github.com/farmOS/farmOS/pull/770#issuecomment-1897575220
• https://github.com/farmOS/farmOS/pull/770#issuecomment-1898199489

Oops missed one.

• hook_farm_api_meta_alter()
• hook_farm_dashboard_panes()
• hook_farm_dashboard_groups()
• hook_farm_ui_theme_region_items()

These ones seem like the lowest-hanging fruit (easiest to convert to Events), I think.

• hook_farm_entity_bundle_field_info()

This one is modeled after Drupal core's hook_entity_base_field_info(). I'm hesitant to say we should diverge from that pattern, otherwise we introduce confusion/inconsistency in our DX and documentation (see https://farmos.org/development/module/fields/).

Notably, we're waiting for core to refactor how they handle base/bundle fields in 📌 Finalize API for creating, overriding, and altering code-defined bundle fields Needs work .

We have a farmOS tracking issue for that (postponed on that core issue) here: #3194206: Refactor bundle fields when Drupal core supports code-defined bundle fields → .

So I'm inclined to say we leave hook_farm_entity_bundle_field_info() alone, and plan to follow core's pattern. Ideally we'll be able to remove our hook entirely once Drupal gets their solution together. Our hook is really just a "shim" right now.

• hook_farm_update_exclude_config()

Maybe this one shouldn't even be a hook? Seems like it could be a setting provided by the farm_update module, that site admins could populate themselves.

Although, I guess the main reason we made it a hook is for modules to programmatically add to it.

Making it a config entity could be a nice "feature request" to expand possibilities, but I think we would still need an Event subscriber that allows modules to "alter" the config. So I guess I've come full circle: it needs an Event too. :-)

• hook_farm_entity_bundle_field_info()

This one is modeled after Drupal core's hook_entity_base_field_info(). I'm hesitant to say we should diverge from that pattern, otherwise we introduce confusion/inconsistency in our DX and documentation (see https://farmos.org/development/module/fields/).

Oh I just had an idea...

What if we abstracted BOTH of these hooks (hook_entity_base_field_info() and hook_farm_entity_bundle_field_info()) into a single farmOS EntityFields event, and recommended that modules use that instead of hooks. Internally we would still use hooks to give Drupal what it expects, but from a farmOS developer's perspective it would all be in one unified place.

I guess it's sort of a way of solving the Drupal core + Entity API issues in farmOS. Maybe that's a fool's errand. But it does seem like it would simplify the DX for our users.

Awesome.

I also like referring to this resource: https://gist.github.com/bojanz/c5fcf5cef22406096588

TLDR;

Choosing the right solution:
Are you allowing modules to react to something that has already happened, or alter a structure? Use events. Do you need to allow your class to have multiple instances created by the user? Use plugins. Otherwise, use tagged services.

Most of the time I think we'll want events. But a few of these hooks that are just collecting provided definitions and not altering (like dashboard panes & groups) could potentially use tagged services. A nice write up on them here: https://drupalsun.com/lakshminp/2016/08/17/tagged-services-drupal-8-0

What if we abstracted BOTH of these hooks (hook_entity_base_field_info() and hook_farm_entity_bundle_field_info()) into a single farmOS EntityFields event, and recommended that modules use that instead of hooks.

This makes sense. I think we would still want separate events to distinguish between base and bundle fields (modules should know which they are providing?) - but yes, we could likely have a single EntityFields event class that could be re-used for both use-cases. And the nice benefit being modules could encapsulate all of this logic into their own, single Event Subscriber class.