Use a derivative to integrate all entities as row plugins

Looks like #1 almost works. I do not see any problems with or without caching, but installer failed to find ViewExecutable in views.module on line 832.

Right, did not try that test before and same exceptions here. Which part of the patch is causing that test to fail?

Nothing to help there but i noticed "Undefined index: in Drupal\views\Plugin\views\row\EntityRow->summaryTitle() (line 115)" in views ui when adding a view with entity:taxonomy_term. Also:

+++ b/core/modules/views/lib/Drupal/views/Plugin/Derivative/ViewsEntityRow.php
@@ -0,0 +1,78 @@
+          'id' => 'entity_row',

Views ui thinks the id should be something like entity:$entity_type.

+++ b/core/modules/views/tests/views_test_config/test_views/views.view.test_entity_row.yml
@@ -0,0 +1,36 @@
+disabled: '0'

status

+++ b/core/modules/views/tests/views_test_config/test_views/views.view.test_entity_row.yml
@@ -0,0 +1,36 @@
+name: test_entity_row

id

Looks like serializing the viewsData property in ViewsEntityRow is causing that failure.

Maybe we should to implement Serializable in things we cache, but that is probably out of scope here. For this issue:
- Moving the base class left few occurences of "Drupal\system\Plugin\views\row\Entity" in the subclasses.
- What about making EntityRow::defineOptions set the first view mode as default to fix the notice in #14?

I'm attempting to create a view of taxonomy terms, where I'm showing the full view mode.

In the views wizard, I select "taxonomy term" and check the box for "create a page". Under "display format" If I choose "unformatted list" of "taxonomy term" and click "save & edit" I get the following error:

Error message
Drupal\Component\Plugin\Exception\PluginException: Plugin (entity:taxonomy_term) instance class "Drupal\system\Plugin\views\row\EntityRow" does not exist. in Drupal\Component\Plugin\Factory\DefaultFactory::getPluginClass() (line 68 of drupal-8.x-dev/core/lib/Drupal/Component/Plugin/Factory/DefaultFactory.php).
The website has encountered an error. Please try again later.

leaving as needs review incase these errors have nothing to do with your magic :)

+++ b/core/modules/user/user.views.incundefined
@@ -400,3 +400,14 @@ function user_views_query_substitutions($view) {
+function user_views_plugins_row_alter(&$data) {
+  unset($data['entity:user']);

Why not remove the annotation from Drupal\user\Plugin\views\row\UserRow and do

function user_views_plugins_row_alter(&$data) {
  $data['entity:user']['class'] = 'Drupal\user\Plugin\views\row\UserRow';
}

instead? This gives other entity providers a really clear way to provide an override mechanic for the entity row in core, and should reduce some of the head scratching with the unsets.

Also, you'd probably move that file out of a directory that will get parsed for annotations, but I don't think that's a requirement.

Eclipse

I'm not sure about Eclipses comment, so I will let you decide on that :) Seems a bit weird to have a plugin we are using but intentionally not parsing an annotation definition for it?

+++ b/core/modules/views/lib/Drupal/views/Plugin/Derivative/ViewsEntityRow.phpundefined
@@ -0,0 +1,64 @@
+      if (isset($entity_info['base_table']) && drupal_container()->get('views.views_data')->get($entity_info['base_table'])) {

We can also move the views_data and create a variable for it outside the foreach loop.

Well I suggested we remove the annotation for that class as well.

Eclipse

Yup, like that (hope it passes since I'm commenting before hand).

Since it's specific to entities and rows, we don't need another example. This is saying "Extend EntityRow, and write your own entity's needs, and then alter it in" which should be pretty easy to follow, and hopefully gives greater insight into the fact that you can do this with almost any plugin, which is ridiculously empowering as a developer.

Eclipse

+++ b/core/modules/comment/comment.views.incundefined
@@ -549,3 +549,14 @@ function comment_views_data_alter(&$data) {
+ * Removes the comment row plugin provided by the generic row plugin.

+++ b/core/modules/node/node.views.incundefined
@@ -692,3 +692,14 @@ function node_views_wizard() {
+ * Removes the node row plugin provided by the generic row plugin.

I think we should say Replaces or overrides instead of removes.

+++ b/core/modules/comment/lib/Drupal/comment/Plugin/views/row/CommentRow.phpundefined
@@ -2,32 +2,20 @@
- *
- * @Plugin(
- *   id = "comment",
- *   module = "comment",
- *   title = @Translation("Comment"),
- *   help = @Translation("Display the comment with standard comment view."),
- *   base = {"comment"},
- *   entity_type = "comment",
- *   type = "normal"
- * )

I still find this odd that we are not having an annotation here, but it also makes sense just to swap out the class.

+++ b/core/modules/views/lib/Drupal/views/Plugin/Derivative/ViewsEntityRow.phpundefined
@@ -0,0 +1,66 @@
+    $entity_types = entity_get_info();

Oooo, is this deprecated now? I say leave it in because I hate the proposed 'new' way.

+++ b/core/modules/views/lib/Drupal/views/Plugin/Derivative/ViewsEntityRow.phpundefined
@@ -0,0 +1,66 @@
+    $this->derivatives = array();

This is already defaulted to an array in the class definition.

+++ b/core/modules/views/lib/Drupal/views/Plugin/Derivative/ViewsEntityRow.phpundefined
@@ -0,0 +1,66 @@
+      if (isset($entity_info['base_table']) && drupal_container()->get('views.views_data')->get($entity_info['base_table'])) {

I think I mentioned this above somewhere but we can move the drupal_container()->get('views.views_data'); outside the foreach loop.

+++ b/core/modules/views/lib/Drupal/views/Tests/Plugin/RowEntityTest.phpundefined
@@ -0,0 +1,55 @@
+    // @todo The entity_test entity isn't rendered so let's use taxonomy term.

I guess this should lose the @todo or it should say 'Replace with entity_test implementation when it can be rendered.' or something.

+++ b/core/modules/comment/comment.views.incundefined
@@ -549,3 +549,14 @@ function comment_views_data_alter(&$data) {
+
+/**
+ * Implements hook_views_plugins_row_alter().
+ *
+ * Removes the comment row plugin provided by the generic row plugin.
+ *
+ * @see \Drupal\views\Plugin\views\row\EntityRow
+ */
+function comment_views_plugins_row_alter(&$data) {
+  $data['entity:comment']['class'] = 'Drupal\comment\Plugin\views\row\CommentRow';

Doc code mismatch. Nothing gets removed. Only the class gets changed. :)

+++ b/core/modules/comment/comment.views.incundefined
@@ -549,3 +549,14 @@ function comment_views_data_alter(&$data) {
+ * Replaces the generic row plugin by a custom one for nodes.

comments :)

+++ b/core/modules/views/lib/Drupal/views/Tests/Plugin/RowEntityTest.phpundefined
@@ -0,0 +1,56 @@
+    // @todo Replaces the taxonomy term used here with the entity_test entity

s/Replaced/Replace - Sorry!!

+++ b/core/modules/comment/tests/modules/comment_test_views/test_views/views.view.test_comment_user_uid.ymlundefined
@@ -39,7 +39,7 @@ display:
+        type: entity:node

This might need to be in quotes because of the ':'. I don't know how much testing there is for this sort of thing yet, so I'm not sure how to check if all is ok.

+++ b/core/modules/taxonomy/tests/modules/taxonomy_test_views/test_views/views.view.test_taxonomy_node_term_data.ymlundefined
@@ -66,7 +66,7 @@ display:
+        type: entity:node

As above, also a few more times throughout the patch.

The rest looks good to me!

Well if it didn't parse the yaml correctly without quotes you definitely wouldn't be getting a green patch :)

I guess we should do whatever would happen when the configuration system writes this normally with the yaml dumper, which I have a feeling might wrap it in quotes, but really it doesn't matter.

I'm 99% certain it wraps in single quotes.

Eclipse

erm, I think it needs a reroll. Stuff explodes now, I can't even get to views page to clear views caches.

Additional uncaught exception thrown while handling exception.

+++ b/core/modules/comment/comment.views.inc
@@ -549,3 +549,14 @@ function comment_views_data_alter(&$data) {
+function comment_views_plugins_row_alter(&$data) {
+  $data['entity:comment']['class'] = 'Drupal\comment\Plugin\views\row\CommentRow';
+}

Is it ok not to alter module key?

+++ b/core/modules/views/lib/Drupal/views/Plugin/Derivative/ViewsEntityRow.php
@@ -0,0 +1,67 @@
+      // Just add support for entity types which have a views integration.
+      if (isset($entity_info['base_table']) && $views_data->get($entity_info['base_table'])) {

Should this skip types which cannot be rendered?

+++ b/core/modules/views/lib/Drupal/views/Tests/Plugin/RowEntityTest.php
@@ -0,0 +1,56 @@
+  /**
+   * Tests the area handler.
+   */
+  public function testEntityArea() {

Area?

+++ b/core/modules/views/lib/Drupal/views/Tests/Plugin/RowEntityTest.php
@@ -0,0 +1,56 @@
+    // @todo Replace the taxonomy term used here with the entity_test entity
+    // once it can be rendered.
+    $term = entity_create('taxonomy_term', array('name' => $this->randomName()));

Can we now use entity_test_render?

+++ b/core/modules/views/views.module
@@ -824,6 +824,18 @@ function views_hook_info() {
+  // During installation the moduleHandler might call hook_hook_info without
+  // having the classloader setup properly.
+  if (class_exists('Drupal\\views\\ViewExecutable')) {

Do we have an issue for this? Until it is fixed, should we inline plugin types here or include_once the file if the class does not exist?

Is it ok not to alter module key?

That's a really interesting idea. I'll be watching the development of this concept.

+++ b/core/modules/views/lib/Drupal/views/Tests/Plugin/RowEntityTest.php
@@ -0,0 +1,65 @@
+class RowEntityTest extends ViewTestBase {

Could this use ViewUnitTestBase?

+++ b/core/modules/views/lib/Drupal/views/Tests/Plugin/RowEntityTest.php
@@ -0,0 +1,65 @@
+  protected function viewsData() {
+    $data = parent::viewsData();
+
+    $data['entity_test_render'] = $data['entity_test'];
+    $data['entity_test_render']['real table'] = 'entity_test';
+    return $data;
+  }

This would be nice. I thought about changing 'entity type' from 'entity_test' to 'entity_test_render' in entity_test_views_data().

I'm fine with using taxonomy, it works.

After applying #55 patch I got a nasty 'looking' error on clicking edit on an enabled "Taxonomy term" view.

Path:
admin/structure/views/view/taxonomy_term/edit

Error produced:
Drupal\Component\Plugin\Exception\PluginException: The plugin (node) did not specify an instance class. in Drupal\Component\Plugin\Factory\DefaultFactory::getPluginClass() (line 62 of core/lib/Drupal/Component/Plugin/Factory/DefaultFactory.php).

To make extra sure it wasn't caching I did the following:
Cleared cache + views cache cleared + truncated all cache tables + sudo rm -fr sites/default/files/php/

@dawehner, I am new to the yml files setup, is there something I need change to properly test this patch?

Bonus with this patch is that mymodule_views_plugins_[any plugin type]_alter can go into mymodule.views.inc.

Shouldn't the container be got from \Drupal::service() now?

Where did ViewsEntityRow go?

Filed #1968090: During installation the moduleHandler calls hook_hook_info without having the classloader setup properly.

You missed a place.

RTBC as per #66.

+++ b/core/modules/views/views.moduleundefined
@@ -825,6 +825,17 @@ function views_hook_info() {
+  // During installation the moduleHandler might call hook_hook_info without
+  // having the classloader setup properly.
+  if (class_exists('Drupal\\views\\ViewExecutable')) {
+    // Register a views_plugins alter hook for all plugin types.
+    foreach (ViewExecutable::getPluginTypes() as $type) {
+      $hooks['views_plugins_' . $type . '_alter'] = array(
+        'group' => 'views',
+      );
+    }
+  }

Could this be moved to a separate issue? Is there one already for the hook_hook_info() invocation without the classloader being properly available?

Looks like the test is not in the patch anymore..?

+++ b/core/modules/comment/lib/Drupal/comment/Plugin/views/row/CommentRow.php
@@ -37,7 +25,7 @@ protected function defineOptions() {
+   * Overrides \Drupal\views\Plugin\views\row\EntityRow::buildOptionsForm().

+++ b/core/modules/node/lib/Drupal/node/Plugin/views/row/NodeRow.php
@@ -46,7 +33,7 @@ protected function defineOptions() {
+   * Overrides \Drupal\views\Plugin\views\row\EntityRow::buildOptionsForm().

{@inheritdoc}

+++ b/core/modules/views/lib/Drupal/views/Plugin/Derivative/ViewsEntityRow.php
@@ -0,0 +1,67 @@
+    $entity_types = \Drupal::service('plugin.manager.entity')
+      ->getDefinitions();

Could use \Drupal::entityManager()?

+++ b/core/modules/views/lib/Drupal/views/Plugin/Derivative/ViewsEntityRow.php
@@ -0,0 +1,67 @@
+      // Just add support for entity types which have a views integration.
+      if (isset($entity_info['base_table']) && $views_data->get($entity_info['base_table'])) {

Missed this one last time.. I still think it should check for hasController($entity_type, 'render') here.

Found one bug while testing manually:

+++ b/core/modules/comment/lib/Drupal/comment/Plugin/views/row/CommentRow.php
@@ -2,32 +2,20 @@
- *   display_types = {"normal"}

+++ b/core/modules/views/lib/Drupal/views/Plugin/Derivative/ViewsEntityRow.php
@@ -0,0 +1,66 @@
+          'display_type' => array('normal'),

'display_type' -> 'display_types'

+++ b/core/modules/views/lib/Drupal/views/Plugin/views/row/EntityRow.php
@@ -52,18 +59,37 @@ class EntityRow extends RowPluginBase {
+  public function __construct(array $configuration, $plugin_id, array $plugin_definition,EntityManager $entity_manager) {

Space missing after last comma =)

+++ b/core/modules/views/lib/Drupal/views/Tests/Plugin/RowEntityTest.php
@@ -0,0 +1,98 @@
+  public static $modules = array('entity_test', 'taxonomy', 'field', 'entity', 'system');

I guess entity_test is not needed anymore.

Great! Back to RTBC.

Sorry to push this back, but I'm missing some API documentation on the new hook_views_plugin_PLUGIN_TYPE_alter(&$data) hooks.

Wow, that is one awesome set of docs!

#87: drupal-1881606-87.patch queued for re-testing.

Weird bot x-post. Still RTBC, though.

Committed bd1f226 and pushed to 8.x. Thanks!