Remove taxonomy term reference field

[SebCorbin]

(Another issue to Unify All the things)

Entity reference is great, combined build modes and such. Taxonomy reference would be a weak duplicate

[quicksketch]

Thanks @SebCorbin! Now that Entity Reference has actually stabilized, I'd be very interested in seeing this replace term reference (and get node referencing abilities at the same time). I'd like to get an idea of how overhead it is to be generic-vs-specific though.

[snassar]

I can't contribute to the overhead question, but having a powerful reference widget like entityreference beats things like termreference or nodereference.

[rudiedirkx]

The performance 'hit' would be in the admin part, not the query part. The admin would load a heavier View or a more sophisticated query to show <option>s or autocomplete search.

I'd also include entity.module in core then, with better core entity properties support and tokens and Views fields etc. (I'm not sure how far D8 went with this.)

[alexweber]

+1, the line between when to use taxonomy terms and when to use references entities is thinner every day. I also hate that we need entity.module, token.module, etc, this should belong in core IMHO.

[jenlampton]

It sounds like we're all in favor of this (I certainly am!) Who's going to work on the pull request? ;)

[maxpearl]

What are the plans regarding ctools? At the moment entityreference requires it. (There are 4 or so ctools hooks used that I can see so far.)

[quicksketch]

Hopefully we wouldn't need a Backdrop version of CTools. Some things definitely need to moved directly into core: more AJAX commands, a dialog system, and other abilities. The piece of CTools that Entity Reference seems to depend on is CTools' plugin system. We have an issue for discussing the plugin system already at #41. @bojanz could probably speak with more accuracy on what's needed and how he'd prefer to see this implemented, since he's a maintainer of Entity Reference for D7.

[matt2000]

This is a initial pass as the port & removal of ctools dependency. Most tests pass, but I still need to debug the taxonomy index. But I want some feedback before I invest more time, to make sure this is a valid direction.

This implements a very rudimentary plugin system using the familiar hook_x_info and drupal_alter pattern, but skipping the work of altering by default, until a module declares that it needs to alter by setting a variable. Initial performance test shows this to be one or two orders of magnitude faster than always altering. I'm assuming 80% of developers will never add these kinds of plugins, because Backdrop wont convert everything is possibly can to the plugin system.

[matt2000]

If I can get some feedback on this, I hope to be able to put some more work in later this week, i.e., after Thursday.

The main questions are:

1.) Is the new plugin stuff here an acceptable replacement for CTools plugins?

2.) If so, should this be made the Core plugin API? Mainly I'm talking about the pattern embodied in this code:

<?php

/**
 * Implements hook_plugin_info
 */
function entityreference_plugin_info($type) {
  static $plugins = array();

  // We never need to run this more than once in a request, so if $plugins has data, we return it earlier.
  if (array_key_exists($type, $plugins)) return $plugins[$type];

  //The module's own plugin definitions can go here if the module author wants to let other modules modify them....

  if (in_array(__FUNCTION__, variable_get('drupal_alter_allowed', array()))) {
    drupal_alter(__FUNCTION__, $plugins);
  }

  // ... or here if not.
  $plugins['my_type']['my_plugin'] = array(
    'title' => t('My own plugin'),
    'class' => 'MyPluginClassWhichIPromiseToIncludeMyselfWhenUsingPlugins',
    'optional_property' => 'anything I want',
 );

}

[matt2000]

In case it's not obvious, if we were to adopt that as an API for core, we could move the boilerplate stuff into the core Plugin Manager, so modules offering plugins can just return an array without any of the caching and optional-altering logic. As it is, 'entityreference_plugin_info' is not really a hook (yet).

[rudiedirkx]

Is there an agreed upon plugin system, or is this your suggestion? (I don't think this is the issue to discuss that plugin system...)

Why the alter inside the plugin info implementation? The plugin system should do that at the end.

The class will be autoloaded, won't it? Manually including class files is too PHP 4.

[matt2000]

No, there is not an agreed on plugin system AFAIK. This is one possible
approach.

Yes, the alter inside the info hook is stop-gap. This was the point of my
previous comment.

It's been stated that Backdrop will not use PSR-0 for auto-loading. Some
other form of auto-loading is outside the scope of my concern right now. I
think the Drupal 7 style of having a 'file' property in the info hook data
is sufficient, but I suppose different kinds of plugins might prefer
different approaches, so I'm not taking a position on that yet.

[quicksketch]

My preferred approach for "plugins" is simply info hooks with an alter paradigm. That is, pretty much no "plugin system" at all. If we want to use classes as the target of an info hook, that's fine (like hook_views_info() and in Entity Reference as well), but in general I'm all for keeping it simple with a consistent pattern.

Classes are currently autoloaded via the D7-based class registry, but we could replace the registry with a hook as suggested in this issue: #77. That gets rid of the database table and cache, making it impossible to corrupt the registry or require both the db and cache systems for autoloading.

Overall, I think this is a good move, but I'm concerned about the apparent complexity of the system. Admittedly this is going to be both replacing term references and adding support for users and nodes, so that seems like a fair tradeoff.

Let's continue down this route for now, we'll need to do at least the following:

• Drop integration with modules (Migrate, Devel, etc.) that aren't in core.
• Drop the README.
• Drop the update hooks and replace with an upgrade path from term references.