From 376fb615d7f862aade6e760c70f25d3eb2ada6da Mon Sep 17 00:00:00 2001 From: xjm <xjm@65776.no-reply.drupal.org> Date: Tue, 17 Nov 2015 22:32:23 -0600 Subject: [PATCH] =?UTF-8?q?Issue=20#2458223=20by=20geertvd,=20eiriksm,=20d?= =?UTF-8?q?awehner,=20lokapujya,=20G=C3=A1bor=20Hojtsy,=20jeqq,=20plach,?= =?UTF-8?q?=20Dom.,=20jhodgdon,=20xjm,=20catch,=20alexpott,=20jibran:=20Du?= =?UTF-8?q?plicated=20field=20handlers=20in=20field=20UI=20for=20some=20ba?= =?UTF-8?q?se=20table=20fields?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- .../src/BlockContentViewsData.php | 27 +- .../Tests/Views/RevisionRelationshipsTest.php | 8 +- ...ws.view.test_block_content_revision_id.yml | 12 +- ...est_block_content_revision_revision_id.yml | 12 +- .../views.view.test_comment_rest.yml | 2 +- .../views.view.test_argument_datetime.yml | 4 +- .../views.view.test_filter_datetime.yml | 4 +- .../views.view.test_sort_datetime.yml | 4 +- .../views.view.test_view_fieldapi.yml | 4 +- core/modules/node/src/NodeViewsData.php | 2 +- .../views.view.test_nid_argument.yml | 2 +- ...iew.test_serializer_node_display_field.yml | 2 +- ...ew.test_serializer_node_exposed_filter.yml | 2 +- .../views.view.test_groupwise_term.yml | 2 +- ...s.view.test_taxonomy_term_relationship.yml | 2 +- core/modules/views/src/EntityViewsData.php | 36 ++- .../Tests/Update/FieldHandlersUpdateTest.php | 48 ++++ .../update/duplicate-field-handler.php | 11 + ...ews.view.test_duplicate_field_handlers.yml | 246 ++++++++++++++++++ .../views.view.test_row_render_cache.yml | 2 +- .../tests/src/Unit/EntityViewsDataTest.php | 72 +++-- core/modules/views/views.post_update.php | 96 +++++++ .../views_ui/src/Tests/HandlerTest.php | 42 ++- 23 files changed, 565 insertions(+), 77 deletions(-) create mode 100644 core/modules/views/src/Tests/Update/FieldHandlersUpdateTest.php create mode 100644 core/modules/views/tests/fixtures/update/duplicate-field-handler.php create mode 100644 core/modules/views/tests/modules/views_test_config/test_views/views.view.test_duplicate_field_handlers.yml diff --git a/core/modules/block_content/src/BlockContentViewsData.php b/core/modules/block_content/src/BlockContentViewsData.php index abb2cb211be8..40963ce91a8d 100644 --- a/core/modules/block_content/src/BlockContentViewsData.php +++ b/core/modules/block_content/src/BlockContentViewsData.php @@ -36,25 +36,24 @@ public function getViewsData() { ), ); // Advertise this table as a possible base table. - $data['block_content_revision']['table']['base']['help'] = $this->t('Block Content revision is a history of changes to block content.'); - $data['block_content_revision']['table']['base']['defaults']['title'] = 'info'; + $data['block_content_field_revision']['table']['base']['help'] = $this->t('Block Content revision is a history of changes to block content.'); + $data['block_content_field_revision']['table']['base']['defaults']['title'] = 'info'; // @todo EntityViewsData should add these relationships by default. // https://www.drupal.org/node/2410275 - $data['block_content_revision']['id']['relationship']['id'] = 'standard'; - $data['block_content_revision']['id']['relationship']['base'] = 'block_content'; - $data['block_content_revision']['id']['relationship']['base field'] = 'id'; - $data['block_content_revision']['id']['relationship']['title'] = $this->t('Block Content'); - $data['block_content_revision']['id']['relationship']['label'] = $this->t('Get the actual block content from a block content revision.'); - - $data['block_content_revision']['revision_id']['relationship']['id'] = 'standard'; - $data['block_content_revision']['revision_id']['relationship']['base'] = 'block_content'; - $data['block_content_revision']['revision_id']['relationship']['base field'] = 'revision_id'; - $data['block_content_revision']['revision_id']['relationship']['title'] = $this->t('Block Content'); - $data['block_content_revision']['revision_id']['relationship']['label'] = $this->t('Get the actual block content from a block content revision.'); + $data['block_content_field_revision']['id']['relationship']['id'] = 'standard'; + $data['block_content_field_revision']['id']['relationship']['base'] = 'block_content_field_data'; + $data['block_content_field_revision']['id']['relationship']['base field'] = 'id'; + $data['block_content_field_revision']['id']['relationship']['title'] = $this->t('Block Content'); + $data['block_content_field_revision']['id']['relationship']['label'] = $this->t('Get the actual block content from a block content revision.'); + + $data['block_content_field_revision']['revision_id']['relationship']['id'] = 'standard'; + $data['block_content_field_revision']['revision_id']['relationship']['base'] = 'block_content_field_data'; + $data['block_content_field_revision']['revision_id']['relationship']['base field'] = 'revision_id'; + $data['block_content_field_revision']['revision_id']['relationship']['title'] = $this->t('Block Content'); + $data['block_content_field_revision']['revision_id']['relationship']['label'] = $this->t('Get the actual block content from a block content revision.'); return $data; - } } diff --git a/core/modules/block_content/src/Tests/Views/RevisionRelationshipsTest.php b/core/modules/block_content/src/Tests/Views/RevisionRelationshipsTest.php index 828f124e6bfd..bd943eef45be 100644 --- a/core/modules/block_content/src/Tests/Views/RevisionRelationshipsTest.php +++ b/core/modules/block_content/src/Tests/Views/RevisionRelationshipsTest.php @@ -60,7 +60,7 @@ public function testBlockContentRevisionRelationship() { $column_map = array( 'revision_id' => 'revision_id', 'id_1' => 'id_1', - 'block_content_block_content_revision_id' => 'block_content_block_content_revision_id', + 'block_content_field_data_block_content_field_revision_id' => 'block_content_field_data_block_content_field_revision_id', ); // Here should be two rows. @@ -70,12 +70,12 @@ public function testBlockContentRevisionRelationship() { array( 'revision_id' => '1', 'id_1' => '1', - 'block_content_block_content_revision_id' => '1', + 'block_content_field_data_block_content_field_revision_id' => '1', ), array( 'revision_id' => '2', 'id_1' => '1', - 'block_content_block_content_revision_id' => '1', + 'block_content_field_data_block_content_field_revision_id' => '1', ), ); $this->assertIdenticalResultset($view_id, $resultset_id, $column_map); @@ -87,7 +87,7 @@ public function testBlockContentRevisionRelationship() { array( 'revision_id' => '2', 'id_1' => '1', - 'block_content_block_content_revision_id' => '1', + 'block_content_field_data_block_content_field_revision_id' => '1', ), ); $this->assertIdenticalResultset($view_revision_id, $resultset_revision_id, $column_map); diff --git a/core/modules/block_content/tests/modules/block_content_test_views/test_views/views.view.test_block_content_revision_id.yml b/core/modules/block_content/tests/modules/block_content_test_views/test_views/views.view.test_block_content_revision_id.yml index efcabc38841e..d08cdd09380b 100644 --- a/core/modules/block_content/tests/modules/block_content_test_views/test_views/views.view.test_block_content_revision_id.yml +++ b/core/modules/block_content/tests/modules/block_content_test_views/test_views/views.view.test_block_content_revision_id.yml @@ -8,7 +8,7 @@ label: null module: views description: '' tag: '' -base_table: block_content_revision +base_table: block_content_field_revision base_field: revision_id core: '8' display: @@ -17,28 +17,28 @@ display: relationships: id: id: id - table: block_content_revision + table: block_content_field_revision field: id required: true plugin_id: standard fields: revision_id: id: revision_id - table: block_content_revision + table: block_content_field_revision field: revision_id plugin_id: field entity_type: block_content entity_field: revision_id id_1: id: id_1 - table: block_content_revision + table: block_content_field_revision field: id plugin_id: field entity_type: block_content entity_field: id id: id: id - table: block_content + table: block_content_field_data field: id relationship: id plugin_id: field @@ -47,7 +47,7 @@ display: arguments: id: id: id - table: block_content_revision + table: block_content_field_revision field: id plugin_id: numeric entity_type: block_content diff --git a/core/modules/block_content/tests/modules/block_content_test_views/test_views/views.view.test_block_content_revision_revision_id.yml b/core/modules/block_content/tests/modules/block_content_test_views/test_views/views.view.test_block_content_revision_revision_id.yml index 566f2c56cc41..ba1699f2f4a1 100644 --- a/core/modules/block_content/tests/modules/block_content_test_views/test_views/views.view.test_block_content_revision_revision_id.yml +++ b/core/modules/block_content/tests/modules/block_content_test_views/test_views/views.view.test_block_content_revision_revision_id.yml @@ -8,7 +8,7 @@ label: null module: views description: '' tag: '' -base_table: block_content_revision +base_table: block_content_field_revision base_field: revision_id core: '8' display: @@ -17,7 +17,7 @@ display: relationships: revision_id: id: revision_id - table: block_content_revision + table: block_content_field_revision field: revision_id required: true entity_type: block_content @@ -26,21 +26,21 @@ display: fields: revision_id: id: revision_id - table: block_content_revision + table: block_content_field_revision field: revision_id plugin_id: field entity_type: block_content entity_field: revision_id id_1: id: id_1 - table: block_content_revision + table: block_content_field_revision field: id plugin_id: field entity_type: block_content entity_field: id id: id: id - table: block_content + table: block_content_field_data field: id relationship: revision_id plugin_id: field @@ -49,7 +49,7 @@ display: arguments: id: id: id - table: block_content_revision + table: block_content_field_revision field: id plugin_id: block_content_id entity_type: block_content diff --git a/core/modules/comment/tests/modules/comment_test_views/test_views/views.view.test_comment_rest.yml b/core/modules/comment/tests/modules/comment_test_views/test_views/views.view.test_comment_rest.yml index 2688bd60580e..fabd46c9e254 100644 --- a/core/modules/comment/tests/modules/comment_test_views/test_views/views.view.test_comment_rest.yml +++ b/core/modules/comment/tests/modules/comment_test_views/test_views/views.view.test_comment_rest.yml @@ -338,7 +338,7 @@ display: arguments: nid: id: nid - table: node + table: node_field_data field: nid relationship: node group_type: group diff --git a/core/modules/datetime/tests/modules/datetime_test/test_views/views.view.test_argument_datetime.yml b/core/modules/datetime/tests/modules/datetime_test/test_views/views.view.test_argument_datetime.yml index e22ad30c810d..7f3abec2292e 100644 --- a/core/modules/datetime/tests/modules/datetime_test/test_views/views.view.test_argument_datetime.yml +++ b/core/modules/datetime/tests/modules/datetime_test/test_views/views.view.test_argument_datetime.yml @@ -27,7 +27,7 @@ display: field: nid id: nid relationship: none - table: node + table: node_field_data plugin_id: numeric pager: options: @@ -39,7 +39,7 @@ display: id: nid order: ASC relationship: none - table: node + table: node_field_data plugin_id: numeric display_plugin: default display_title: Master diff --git a/core/modules/datetime/tests/modules/datetime_test/test_views/views.view.test_filter_datetime.yml b/core/modules/datetime/tests/modules/datetime_test/test_views/views.view.test_filter_datetime.yml index c9b23ded6fe2..abd0e4f1a758 100644 --- a/core/modules/datetime/tests/modules/datetime_test/test_views/views.view.test_filter_datetime.yml +++ b/core/modules/datetime/tests/modules/datetime_test/test_views/views.view.test_filter_datetime.yml @@ -24,7 +24,7 @@ display: nid: field: nid id: nid - table: node + table: node_field_data plugin_id: node filters: field_date_value: @@ -38,7 +38,7 @@ display: id: nid order: ASC relationship: none - table: node + table: node_field_data plugin_id: numeric pager: type: full diff --git a/core/modules/datetime/tests/modules/datetime_test/test_views/views.view.test_sort_datetime.yml b/core/modules/datetime/tests/modules/datetime_test/test_views/views.view.test_sort_datetime.yml index 8b8d052ae762..de808302fe1f 100644 --- a/core/modules/datetime/tests/modules/datetime_test/test_views/views.view.test_sort_datetime.yml +++ b/core/modules/datetime/tests/modules/datetime_test/test_views/views.view.test_sort_datetime.yml @@ -24,7 +24,7 @@ display: nid: field: nid id: nid - table: node + table: node_field_data plugin_id: node sorts: field_date_value: @@ -39,7 +39,7 @@ display: id: nid order: ASC relationship: none - table: node + table: node_field_data plugin_id: numeric pager: type: full diff --git a/core/modules/field/tests/modules/field_test_views/test_views/views.view.test_view_fieldapi.yml b/core/modules/field/tests/modules/field_test_views/test_views/views.view.test_view_fieldapi.yml index 869e44194b5f..ab39c2791110 100644 --- a/core/modules/field/tests/modules/field_test_views/test_views/views.view.test_view_fieldapi.yml +++ b/core/modules/field/tests/modules/field_test_views/test_views/views.view.test_view_fieldapi.yml @@ -9,7 +9,7 @@ label: test_view_fieldapi module: views description: '' tag: default -base_table: node +base_table: node_field_data base_field: nid core: '8' display: @@ -21,7 +21,7 @@ display: nid: field: nid id: nid - table: node + table: node_field_data plugin_id: field entity_type: node entity_field: nid diff --git a/core/modules/node/src/NodeViewsData.php b/core/modules/node/src/NodeViewsData.php index 3c0d50956268..895008ef52f0 100644 --- a/core/modules/node/src/NodeViewsData.php +++ b/core/modules/node/src/NodeViewsData.php @@ -240,7 +240,7 @@ public function getViewsData() { 'title' => t('Content'), 'label' => t('Get the actual content from a content revision.'), ), - ) + $data['node_revision']['vid']; + ) + $data['node_field_revision']['vid']; $data['node_field_revision']['langcode']['help'] = t('The language the original content is in.'); diff --git a/core/modules/node/tests/modules/node_test_views/test_views/views.view.test_nid_argument.yml b/core/modules/node/tests/modules/node_test_views/test_views/views.view.test_nid_argument.yml index b61f1ad9157a..f52109371693 100644 --- a/core/modules/node/tests/modules/node_test_views/test_views/views.view.test_nid_argument.yml +++ b/core/modules/node/tests/modules/node_test_views/test_views/views.view.test_nid_argument.yml @@ -56,7 +56,7 @@ display: fields: nid: id: nid - table: node + table: node_field_data field: nid relationship: none group_type: group diff --git a/core/modules/rest/tests/modules/rest_test_views/test_views/views.view.test_serializer_node_display_field.yml b/core/modules/rest/tests/modules/rest_test_views/test_views/views.view.test_serializer_node_display_field.yml index ab5092056828..7699fb14dce5 100644 --- a/core/modules/rest/tests/modules/rest_test_views/test_views/views.view.test_serializer_node_display_field.yml +++ b/core/modules/rest/tests/modules/rest_test_views/test_views/views.view.test_serializer_node_display_field.yml @@ -41,7 +41,7 @@ display: fields: nid: id: nid - table: node + table: node_field_data field: nid plugin_id: field entity_type: node diff --git a/core/modules/rest/tests/modules/rest_test_views/test_views/views.view.test_serializer_node_exposed_filter.yml b/core/modules/rest/tests/modules/rest_test_views/test_views/views.view.test_serializer_node_exposed_filter.yml index b5af1e96b54b..6165641ad864 100644 --- a/core/modules/rest/tests/modules/rest_test_views/test_views/views.view.test_serializer_node_exposed_filter.yml +++ b/core/modules/rest/tests/modules/rest_test_views/test_views/views.view.test_serializer_node_exposed_filter.yml @@ -41,7 +41,7 @@ display: fields: nid: id: nid - table: node + table: node_field_data field: nid plugin_id: field entity_type: node diff --git a/core/modules/taxonomy/tests/modules/taxonomy_test_views/test_views/views.view.test_groupwise_term.yml b/core/modules/taxonomy/tests/modules/taxonomy_test_views/test_views/views.view.test_groupwise_term.yml index a802762bbe81..5ba97e0faff1 100644 --- a/core/modules/taxonomy/tests/modules/taxonomy_test_views/test_views/views.view.test_groupwise_term.yml +++ b/core/modules/taxonomy/tests/modules/taxonomy_test_views/test_views/views.view.test_groupwise_term.yml @@ -59,7 +59,7 @@ display: subquery_namespace: '' subquery_order: DESC subquery_regenerate: true - subquery_sort: node.nid + subquery_sort: node_field_data.nid subquery_view: '' table: taxonomy_term_field_data plugin_id: groupwise_max diff --git a/core/modules/taxonomy/tests/modules/taxonomy_test_views/test_views/views.view.test_taxonomy_term_relationship.yml b/core/modules/taxonomy/tests/modules/taxonomy_test_views/test_views/views.view.test_taxonomy_term_relationship.yml index 96d0559dc008..80295b6ea249 100644 --- a/core/modules/taxonomy/tests/modules/taxonomy_test_views/test_views/views.view.test_taxonomy_term_relationship.yml +++ b/core/modules/taxonomy/tests/modules/taxonomy_test_views/test_views/views.view.test_taxonomy_term_relationship.yml @@ -156,7 +156,7 @@ display: plugin_id: field nid: id: nid - table: node + table: node_field_data field: nid entity_type: node entity_field: nid diff --git a/core/modules/views/src/EntityViewsData.php b/core/modules/views/src/EntityViewsData.php index 20692719b8b9..bc355f9c913e 100644 --- a/core/modules/views/src/EntityViewsData.php +++ b/core/modules/views/src/EntityViewsData.php @@ -116,11 +116,29 @@ protected function getFieldStorageDefinitions() { public function getViewsData() { $data = []; - $base_table = $this->entityType->getBaseTable(); + $base_table = $this->entityType->getBaseTable() ?: $this->entityType->id(); + $revisionable = $this->entityType->isRevisionable(); $base_field = $this->entityType->getKey('id'); - $data_table = $this->entityType->getDataTable(); - $revision_table = $this->entityType->getRevisionTable(); - $revision_data_table = $this->entityType->getRevisionDataTable(); + + $revision_table = ''; + if ($revisionable) { + $revision_table = $this->entityType->getRevisionTable() ?: $this->entityType->id() . '_revision'; + } + + $translatable = $this->entityType->isTranslatable(); + $data_table = ''; + if ($translatable) { + $data_table = $this->entityType->getDataTable() ?: $this->entityType->id() . '_field_data'; + } + + // Some entity types do not have a revision data table defined, but still + // have a revision table name set in + // \Drupal\Core\Entity\Sql\SqlContentEntityStorage::initTableLayout() so we + // apply the same kind of logic. + $revision_data_table = ''; + if ($revisionable && $translatable) { + $revision_data_table = $this->entityType->getRevisionDataTable() ?: $this->entityType->id() . '_field_revision'; + } $revision_field = $this->entityType->getKey('revision'); // Setup base information of the views data. @@ -213,6 +231,10 @@ public function getViewsData() { // the entity base, revision, data tables. $field_definitions = $this->entityManager->getBaseFieldDefinitions($this->entityType->id()); if ($table_mapping = $this->storage->getTableMapping()) { + // Fetch all fields that can appear in both the base table and the data + // table. + $entity_keys = $this->entityType->getKeys(); + $duplicate_fields = array_intersect_key($entity_keys, array_flip(['id', 'revision', 'bundle'])); // Iterate over each table we have so far and collect field data for each. // Based on whether the field is in the field_definitions provided by the // entity manager. @@ -221,6 +243,12 @@ public function getViewsData() { // @todo https://www.drupal.org/node/2337511 foreach ($table_mapping->getTableNames() as $table) { foreach ($table_mapping->getFieldNames($table) as $field_name) { + // To avoid confusing duplication in the user interface, for fields + // that are on both base and data tables, only add them on the data + // table (same for revision vs. revision data). + if ($data_table && ($table === $base_table || $table === $revision_table) && in_array($field_name, $duplicate_fields)) { + continue; + } $this->mapFieldDefinition($table, $field_name, $field_definitions[$field_name], $table_mapping, $data[$table]); } } diff --git a/core/modules/views/src/Tests/Update/FieldHandlersUpdateTest.php b/core/modules/views/src/Tests/Update/FieldHandlersUpdateTest.php new file mode 100644 index 000000000000..c5ccd93b4a82 --- /dev/null +++ b/core/modules/views/src/Tests/Update/FieldHandlersUpdateTest.php @@ -0,0 +1,48 @@ +<?php + +/** + * @file + * Contains \Drupal\views\Tests\Update\FieldHandlersUpdateTest. + */ + +namespace Drupal\views\Tests\Update; + +use Drupal\system\Tests\Update\UpdatePathTestBase; +use Drupal\views\Entity\View; + +/** + * Tests the upgrade path for views field handlers. + * + * @see views_post_update_cleanup_duplicate_views_data() + * + * @group Update + */ +class FieldHandlersUpdateTest extends UpdatePathTestBase { + + /** + * {@inheritdoc} + */ + protected function setDatabaseDumpFiles() { + $this->databaseDumpFiles = [ + __DIR__ . '/../../../../system/tests/fixtures/update/drupal-8.bare.standard.php.gz', + __DIR__ . '/../../../tests/fixtures/update/duplicate-field-handler.php', + ]; + } + + /** + * Tests that field handlers are updated properly. + */ + public function testViewsUpdate8004() { + $this->runUpdates(); + + // Load and initialize our test view. + $view = View::load('test_duplicate_field_handlers'); + $data = $view->toArray(); + // Check that the field is using the expected base table. + $this->assertEqual('node_field_data', $data['display']['default']['display_options']['fields']['nid']['table']); + $this->assertEqual('node_field_data', $data['display']['default']['display_options']['filters']['type']['table']); + $this->assertEqual('node_field_data', $data['display']['default']['display_options']['sorts']['vid']['table']); + $this->assertEqual('node_field_data', $data['display']['default']['display_options']['arguments']['nid']['table']); + } + +} diff --git a/core/modules/views/tests/fixtures/update/duplicate-field-handler.php b/core/modules/views/tests/fixtures/update/duplicate-field-handler.php new file mode 100644 index 000000000000..2b78f123b653 --- /dev/null +++ b/core/modules/views/tests/fixtures/update/duplicate-field-handler.php @@ -0,0 +1,11 @@ +<?php + +$connection = Drupal\Core\Database\Database::getConnection(); + +$connection->insert('config') + ->fields(array( + 'collection' => '', + 'name' => 'views.view.test_duplicate_field_handlers', + 'data' => serialize(\Drupal\Component\Serialization\Yaml::decode(file_get_contents('core/modules/views/tests/modules/views_test_config/test_views/views.view.test_duplicate_field_handlers.yml'))), + )) + ->execute(); diff --git a/core/modules/views/tests/modules/views_test_config/test_views/views.view.test_duplicate_field_handlers.yml b/core/modules/views/tests/modules/views_test_config/test_views/views.view.test_duplicate_field_handlers.yml new file mode 100644 index 000000000000..41a9dfa0c475 --- /dev/null +++ b/core/modules/views/tests/modules/views_test_config/test_views/views.view.test_duplicate_field_handlers.yml @@ -0,0 +1,246 @@ +langcode: en +status: true +dependencies: + config: + - node.type.article + module: + - node + - user +id: test_duplicate_field_handlers +label: 'Test Duplicate Field Handlers' +module: views +description: '' +tag: '' +base_table: node_field_data +base_field: nid +core: 8.x +display: + default: + display_plugin: default + id: default + display_title: Master + position: 0 + display_options: + access: + type: perm + options: + perm: 'access content' + cache: + type: tag + options: { } + query: + type: views_query + options: + disable_sql_rewrite: false + distinct: false + replica: false + query_comment: '' + query_tags: { } + exposed_form: + type: basic + options: + submit_button: Apply + reset_button: false + reset_button_label: Reset + exposed_sorts_label: 'Sort by' + expose_sort_order: true + sort_asc_label: Asc + sort_desc_label: Desc + pager: + type: none + options: + items_per_page: null + offset: 0 + style: + type: default + row: + type: fields + fields: + nid: + id: nid + table: node + field: nid + relationship: none + group_type: group + admin_label: '' + label: '' + exclude: false + alter: + alter_text: false + text: '' + make_link: false + path: '' + absolute: false + external: false + replace_spaces: false + path_case: none + trim_whitespace: false + alt: '' + rel: '' + link_class: '' + prefix: '' + suffix: '' + target: '' + nl2br: false + max_length: 0 + word_boundary: true + ellipsis: true + more_link: false + more_link_text: '' + more_link_path: '' + strip_tags: false + trim: false + preserve_tags: '' + html: false + element_type: '' + element_class: '' + element_label_type: '' + element_label_class: '' + element_label_colon: false + element_wrapper_type: '' + element_wrapper_class: '' + element_default_classes: true + empty: '' + hide_empty: false + empty_zero: false + hide_alter_empty: true + click_sort_column: value + type: number_integer + settings: + thousand_separator: '' + prefix_suffix: true + group_column: value + group_columns: { } + group_rows: true + delta_limit: 0 + delta_offset: 0 + delta_reversed: false + delta_first_last: false + multi_type: separator + separator: ', ' + field_api_classes: false + entity_type: node + entity_field: nid + plugin_id: field + filters: + type: + id: type + table: node + field: type + relationship: none + group_type: group + admin_label: '' + operator: in + value: + article: article + group: 1 + exposed: false + expose: + operator_id: '' + label: '' + description: '' + use_operator: false + operator: '' + identifier: '' + required: false + remember: false + multiple: false + remember_roles: + authenticated: authenticated + reduce: false + is_grouped: false + group_info: + label: '' + description: '' + identifier: '' + optional: true + widget: select + multiple: false + remember: false + default_group: All + default_group_multiple: { } + group_items: { } + entity_type: node + entity_field: type + plugin_id: bundle + sorts: + vid: + id: vid + table: node + field: vid + relationship: none + group_type: group + admin_label: '' + order: ASC + exposed: false + expose: + label: '' + entity_type: node + entity_field: vid + plugin_id: standard + title: 'Test Duplicate Field Handlers' + header: { } + footer: { } + empty: { } + relationships: { } + arguments: + nid: + id: nid + table: node + field: nid + relationship: none + group_type: group + admin_label: '' + default_action: ignore + exception: + value: all + title_enable: false + title: All + title_enable: false + title: '' + default_argument_type: fixed + default_argument_options: + argument: '' + default_argument_skip_url: false + summary_options: + base_path: '' + count: true + items_per_page: 25 + override: false + summary: + sort_order: asc + number_of_records: 0 + format: default_summary + specify_validation: false + validate: + type: none + fail: 'not found' + validate_options: { } + break_phrase: false + not: false + entity_type: node + entity_field: nid + plugin_id: numeric + display_extenders: { } + cache_metadata: + contexts: + - 'languages:language_content' + - 'languages:language_interface' + - 'user.node_grants:view' + - user.permissions + cacheable: false + page_1: + display_plugin: page + id: page_1 + display_title: Page + position: 1 + display_options: + display_extenders: { } + path: test-duplicate-field-handlers + cache_metadata: + contexts: + - 'languages:language_content' + - 'languages:language_interface' + - 'user.node_grants:view' + - user.permissions + cacheable: false diff --git a/core/modules/views/tests/modules/views_test_config/test_views/views.view.test_row_render_cache.yml b/core/modules/views/tests/modules/views_test_config/test_views/views.view.test_row_render_cache.yml index ec19b13ee39b..2a79e74b14db 100644 --- a/core/modules/views/tests/modules/views_test_config/test_views/views.view.test_row_render_cache.yml +++ b/core/modules/views/tests/modules/views_test_config/test_views/views.view.test_row_render_cache.yml @@ -130,7 +130,7 @@ display: fields: nid: id: nid - table: node + table: node_field_data field: nid relationship: none group_type: group diff --git a/core/modules/views/tests/src/Unit/EntityViewsDataTest.php b/core/modules/views/tests/src/Unit/EntityViewsDataTest.php index dfdc0bd4e46d..feefe0aeb8f0 100644 --- a/core/modules/views/tests/src/Unit/EntityViewsDataTest.php +++ b/core/modules/views/tests/src/Unit/EntityViewsDataTest.php @@ -93,7 +93,12 @@ protected function setUp() { 'base_table' => 'entity_test', 'id' => 'entity_test', 'label' => 'Entity test', - 'entity_keys' => ['id' => 'id', 'langcode' => 'langcode'], + 'entity_keys' => [ + 'id' => 'id', + 'langcode' => 'langcode', + 'bundle' => 'type', + 'revision' => 'revision_id', + ], 'provider' => 'entity_test', 'list_cache_contexts' => ['entity_test_list_cache_context'], ]); @@ -190,6 +195,7 @@ public function testDataTable() { $entity_type = $this->baseEntityType ->set('data_table', 'entity_test_mul_property_data') ->set('id', 'entity_test_mul') + ->set('translatable', TRUE) ->setKey('label', 'label'); $this->viewsData->setEntityType($entity_type); @@ -259,6 +265,7 @@ public function testRevisionTableWithRevisionDataTableAndDataTable() { ->set('revision_table', 'entity_test_mulrev_revision') ->set('revision_data_table', 'entity_test_mulrev_property_revision') ->set('id', 'entity_test_mulrev') + ->set('translatable', TRUE) ->setKey('revision', 'revision_id') ; $this->viewsData->setEntityType($entity_type); @@ -297,6 +304,7 @@ public function testRevisionTableWithRevisionDataTable() { ->set('revision_table', 'entity_test_mulrev_revision') ->set('revision_data_table', 'entity_test_mulrev_property_revision') ->set('id', 'entity_test_mulrev') + ->set('translatable', TRUE) ->setKey('revision', 'revision_id') ; $this->viewsData->setEntityType($entity_type); @@ -316,7 +324,7 @@ public function testRevisionTableWithRevisionDataTable() { $revision_data = $data['entity_test_mulrev_property_revision']; $this->assertCount(2, $revision_data['table']['join']); $this->assertEquals([ - 'entity_test' => ['left_field' => 'revision_id', 'field' => 'revision_id', 'type' => 'INNER'], + 'entity_test_mulrev_field_data' => ['left_field' => 'revision_id', 'field' => 'revision_id', 'type' => 'INNER'], 'entity_test_mulrev_revision' => ['left_field' => 'revision_id', 'field' => 'revision_id', 'type' => 'INNER'], ], $revision_data['table']['join']); $this->assertFalse(isset($data['data_table'])); @@ -526,10 +534,19 @@ public function testDataTableFields() { $table_mapping->expects($this->any()) ->method('getFieldNames') ->willReturnMap([ - ['entity_test_mul', ['id', 'uuid', 'type', 'langcode']], - ['entity_test_mul_property_data', ['id', 'langcode', 'name', 'description', 'homepage', 'user_id']], + ['entity_test_mul', ['uuid']], + ['entity_test_mul_property_data', ['id', 'type', 'langcode', 'name', 'description', 'homepage', 'user_id']], ]); + $table_mapping->expects($this->any()) + ->method('getFieldTableName') + ->willReturnCallback(function($field) { + if ($field == 'uuid') { + return 'entity_test_mul'; + } + return 'entity_test_mul_property_data'; + }); + $this->entityStorage->expects($this->once()) ->method('getTableMapping') ->willReturn($table_mapping); @@ -547,17 +564,13 @@ public function testDataTableFields() { $data = $this->viewsData->getViewsData(); // Check the base fields. - $this->assertNumericField($data['entity_test_mul']['id']); - $this->assertField($data['entity_test_mul']['id'], 'id'); + $this->assertFalse(isset($data['entity_test_mul']['id'])); + $this->assertFalse(isset($data['entity_test_mul']['type'])); $this->assertUuidField($data['entity_test_mul']['uuid']); $this->assertField($data['entity_test_mul']['uuid'], 'uuid'); - $this->assertBundleField($data['entity_test_mul']['type']); - $this->assertField($data['entity_test_mul']['type'], 'type'); $this->assertFalse(isset($data['entity_test_mul']['type']['relationship'])); - $this->assertLanguageField($data['entity_test_mul']['langcode']); - $this->assertField($data['entity_test_mul']['langcode'], 'langcode'); // Also ensure that field_data only fields don't appear on the base table. $this->assertFalse(isset($data['entity_test_mul']['name'])); $this->assertFalse(isset($data['entity_test_mul']['description'])); @@ -570,6 +583,9 @@ public function testDataTableFields() { $this->assertNumericField($data['entity_test_mul_property_data']['id']); $this->assertField($data['entity_test_mul_property_data']['id'], 'id'); + $this->assertBundleField($data['entity_test_mul_property_data']['type']); + $this->assertField($data['entity_test_mul_property_data']['type'], 'type'); + $this->assertLanguageField($data['entity_test_mul_property_data']['langcode']); $this->assertField($data['entity_test_mul_property_data']['langcode'], 'langcode'); $this->assertEquals('Translation language', $data['entity_test_mul_property_data']['langcode']['title']); @@ -600,7 +616,8 @@ public function testRevisionTableFields() { ->set('revision_table', 'entity_test_mulrev_revision') ->set('data_table', 'entity_test_mulrev_property_data') ->set('revision_data_table', 'entity_test_mulrev_property_revision') - ->set('id', 'entity_test_mulrev'); + ->set('id', 'entity_test_mulrev') + ->set('translatable', TRUE); $base_field_definitions = $this->setupBaseFields(EntityTestMulRev::baseFieldDefinitions($this->baseEntityType)); $user_base_field_definitions = [ 'uid' => BaseFieldDefinition::create('integer') @@ -645,6 +662,15 @@ public function testRevisionTableFields() { ['entity_test_mulrev_property_revision', ['id', 'revision_id', 'langcode', 'name', 'description', 'homepage', 'user_id']], ]); + $table_mapping->expects($this->any()) + ->method('getFieldTableName') + ->willReturnCallback(function($field) { + if ($field == 'uuid') { + return 'entity_test_mulrev'; + } + return 'entity_test_mulrev_property_data'; + }); + $this->entityStorage->expects($this->once()) ->method('getTableMapping') ->willReturn($table_mapping); @@ -654,14 +680,11 @@ public function testRevisionTableFields() { $data = $this->viewsData->getViewsData(); // Check the base fields. - $this->assertNumericField($data['entity_test_mulrev']['id']); - $this->assertField($data['entity_test_mulrev']['id'], 'id'); - $this->assertNumericField($data['entity_test_mulrev']['revision_id']); - $this->assertField($data['entity_test_mulrev']['revision_id'], 'revision_id'); + $this->assertFalse(isset($data['entity_test_mulrev']['id'])); + $this->assertFalse(isset($data['entity_test_mulrev']['type'])); + $this->assertFalse(isset($data['entity_test_mulrev']['revision_id'])); $this->assertUuidField($data['entity_test_mulrev']['uuid']); $this->assertField($data['entity_test_mulrev']['uuid'], 'uuid'); - $this->assertStringField($data['entity_test_mulrev']['type']); - $this->assertField($data['entity_test_mulrev']['type'], 'type'); // Also ensure that field_data only fields don't appear on the base table. $this->assertFalse(isset($data['entity_test_mulrev']['name'])); @@ -672,17 +695,12 @@ public function testRevisionTableFields() { $this->assertFalse(isset($data['entity_test_mulrev']['langcode'])); $this->assertFalse(isset($data['entity_test_mulrev']['user_id'])); - // Check the revision fields. - $this->assertNumericField($data['entity_test_mulrev_revision']['id']); - $this->assertField($data['entity_test_mulrev_revision']['id'], 'id'); - $this->assertNumericField($data['entity_test_mulrev_revision']['revision_id']); - $this->assertField($data['entity_test_mulrev_revision']['revision_id'], 'revision_id'); - - $this->assertLanguageField($data['entity_test_mulrev_revision']['langcode']); - $this->assertField($data['entity_test_mulrev_revision']['langcode'], 'langcode'); - $this->assertEquals('Original language', $data['entity_test_mulrev_revision']['langcode']['title']); + // Check the revision fields. The revision ID should only appear in the data + // table. + $this->assertFalse(isset($data['entity_test_mulrev_revision']['revision_id'])); // Also ensure that field_data only fields don't appear on the revision table. + $this->assertFalse(isset($data['entity_test_mulrev_revision']['id'])); $this->assertFalse(isset($data['entity_test_mulrev_revision']['name'])); $this->assertFalse(isset($data['entity_test_mulrev_revision']['description'])); $this->assertFalse(isset($data['entity_test_mulrev_revision']['description__value'])); @@ -693,6 +711,8 @@ public function testRevisionTableFields() { // Check the data fields. $this->assertNumericField($data['entity_test_mulrev_property_data']['id']); $this->assertField($data['entity_test_mulrev_property_data']['id'], 'id'); + $this->assertNumericField($data['entity_test_mulrev_property_data']['revision_id']); + $this->assertField($data['entity_test_mulrev_property_data']['revision_id'], 'revision_id'); $this->assertLanguageField($data['entity_test_mulrev_property_data']['langcode']); $this->assertField($data['entity_test_mulrev_property_data']['langcode'], 'langcode'); $this->assertStringField($data['entity_test_mulrev_property_data']['name']); diff --git a/core/modules/views/views.post_update.php b/core/modules/views/views.post_update.php index 5d248a5c740a..fea4f7e7df19 100644 --- a/core/modules/views/views.post_update.php +++ b/core/modules/views/views.post_update.php @@ -5,6 +5,9 @@ * Post update functions for Views. */ +use Drupal\Core\StringTranslation\TranslatableMarkup; +use Drupal\views\Views; + /** * @addtogroup updates-8.0.0-beta * @{ @@ -31,6 +34,99 @@ function views_post_update_update_cacheability_metadata() { } +/** + * Update some views fields that were previously duplicated. + */ +function views_post_update_cleanup_duplicate_views_data() { + $config_factory = \Drupal::configFactory(); + $ids = []; + $message = NULL; + $data_tables = []; + $base_tables = []; + $revision_tables = []; + $entities_by_table = []; + $duplicate_fields = []; + $handler_types = Views::getHandlerTypes(); + + /** @var \Drupal\Core\Entity\EntityTypeManagerInterface $entity_type_manager */ + $entity_type_manager = \Drupal::service('entity_type.manager'); + // This will allow us to create an index of all entity types of the site. + foreach ($entity_type_manager->getDefinitions() as $entity_type_id => $entity_type) { + // Store the entity keyed by base table. If it has a data table, use that as + // well. + if ($data_table = $entity_type->getDataTable()) { + $entities_by_table[$data_table] = $entity_type; + } + if ($base_table = $entity_type->getBaseTable()) { + $entities_by_table[$base_table] = $entity_type; + } + + // The following code basically contains the same kind of logic as + // \Drupal\Core\Entity\Sql\SqlContentEntityStorage::initTableLayout() to + // prefetch all tables (base, data, revision, and revision data). + $base_tables[$entity_type_id] = $entity_type->getBaseTable() ?: $entity_type->id(); + $revisionable = $entity_type->isRevisionable(); + + $revision_table = ''; + if ($revisionable) { + $revision_table = $entity_type->getRevisionTable() ?: $entity_type->id() . '_revision'; + } + $revision_tables[$entity_type_id] = $revision_table; + + $translatable = $entity_type->isTranslatable(); + $data_table = ''; + // For example the data table just exists, when the entity type is + // translatable. + if ($translatable) { + $data_table = $entity_type->getDataTable() ?: $entity_type->id() . '_field_data'; + } + $data_tables[$entity_type_id] = $data_table; + + $duplicate_fields[$entity_type_id] = array_intersect_key($entity_type->getKeys(), array_flip(['id', 'revision', 'bundle'])); + } + + foreach ($config_factory->listAll('views.view.') as $view_config_name) { + $changed = FALSE; + $view = $config_factory->getEditable($view_config_name); + + $displays = $view->get('display'); + if (isset($entities_by_table[$view->get('base_table')])) { + $entity_type = $entities_by_table[$view->get('base_table')]; + $entity_type_id = $entity_type->id(); + $data_table = $data_tables[$entity_type_id]; + $base_table = $base_tables[$entity_type_id]; + $revision_table = $revision_tables[$entity_type_id]; + + if ($data_table) { + foreach ($displays as $display_name => &$display) { + foreach ($handler_types as $handler_type) { + if (!empty($display['display_options'][$handler_type['plural']])) { + foreach ($display['display_options'][$handler_type['plural']] as $field_name => &$field) { + $table = $field['table']; + if (($table === $base_table || $table === $revision_table) && in_array($field_name, $duplicate_fields[$entity_type_id])) { + $field['table'] = $data_table; + $changed = TRUE; + } + } + } + } + } + } + } + + if ($changed) { + $view->set('display', $displays); + $view->save(); + $ids[] = $view->get('id'); + } + } + if (!empty($ids)) { + $message = new TranslatableMarkup('Updated tables for field handlers for views: @ids', ['@ids' => implode(', ', array_unique($ids))]); + } + + return $message; +} + /** * @} End of "addtogroup updates-8.0.0-beta". */ diff --git a/core/modules/views_ui/src/Tests/HandlerTest.php b/core/modules/views_ui/src/Tests/HandlerTest.php index 8bfc6867f8bd..214397a68dbd 100644 --- a/core/modules/views_ui/src/Tests/HandlerTest.php +++ b/core/modules/views_ui/src/Tests/HandlerTest.php @@ -10,6 +10,7 @@ use Drupal\Component\Utility\SafeMarkup; use Drupal\field\Entity\FieldConfig; use Drupal\field\Entity\FieldStorageConfig; +use Drupal\views\Tests\ViewTestData; use Drupal\views\ViewExecutable; /** @@ -20,12 +21,17 @@ */ class HandlerTest extends UITestBase { + /** + * {@inheritdoc} + */ + public static $modules = array('node_test_views'); + /** * Views used by this test. * * @var array */ - public static $testViews = array('test_view_empty', 'test_view_broken', 'node'); + public static $testViews = array('test_view_empty', 'test_view_broken', 'node', 'test_node_view'); /** * {@inheritdoc} @@ -34,6 +40,7 @@ protected function setUp() { parent::setUp(); $this->drupalPlaceBlock('page_title_block'); + ViewTestData::createTestViews(get_class($this), array('node_test_views')); } /** @@ -218,4 +225,37 @@ public function testBrokenHandlers() { } } + /** + * Ensures that neither node type or node ID appears multiple times. + * + * @see \Drupal\views\EntityViewsData + */ + public function testNoDuplicateFields() { + $handler_types = ['field', 'filter', 'sort', 'argument']; + + foreach ($handler_types as $handler_type) { + $add_handler_url = 'admin/structure/views/nojs/add-handler/test_node_view/default/' . $handler_type; + $this->drupalGet($add_handler_url); + + $this->assertNoDuplicateField('Node ID', 'Content'); + $this->assertNoDuplicateField('Node ID', 'Content revision'); + $this->assertNoDuplicateField('Type', 'Content'); + $this->assertNoDuplicateField('UUID', 'Content'); + $this->assertNoDuplicateField('Revision ID', 'Content'); + $this->assertNoDuplicateField('Revision ID', 'Content revision'); + } + } + + /** + * Asserts that fields only appear once. + * + * @param string $field_name + * The field name. + * @param string $entity_type + * The entity type to which the field belongs. + */ + public function assertNoDuplicateField($field_name, $entity_type) { + $elements = $this->xpath('//td[.=:entity_type]/preceding-sibling::td[@class="title" and .=:title]', [':title' => $field_name, ':entity_type' => $entity_type]); + $this->assertEqual(1, count($elements), $field_name . ' appears just once in ' . $entity_type . '.'); + } } -- GitLab