diff --git a/core/modules/system/tests/fixtures/update/drupal-8.views-entity-views-data-2846614.php b/core/modules/system/tests/fixtures/update/drupal-8.views-entity-views-data-2846614.php new file mode 100644 index 0000000..2dcacb2 --- /dev/null +++ b/core/modules/system/tests/fixtures/update/drupal-8.views-entity-views-data-2846614.php @@ -0,0 +1,32 @@ +insert('config') + ->fields([ + 'collection', + 'name', + 'data', + ]) + ->values([ + 'collection' => '', + 'name' => 'views.view.' . $views_config['id'], + 'data' => serialize($views_config), + ]) + ->execute(); +} diff --git a/core/modules/system/tests/fixtures/update/drupal-8.views-entity-views-data-2846614.yml b/core/modules/system/tests/fixtures/update/drupal-8.views-entity-views-data-2846614.yml new file mode 100644 index 0000000..2d68cbe --- /dev/null +++ b/core/modules/system/tests/fixtures/update/drupal-8.views-entity-views-data-2846614.yml @@ -0,0 +1,240 @@ +uuid: 001475a0-daec-4e8a-8ca7-97b0d24100a6 +langcode: en +status: true +dependencies: + module: + - user +id: test_user_multi_value +label: test_user_multi_value +module: views +description: '' +tag: '' +base_table: users_field_data +base_field: uid +core: 8.x +display: + default: + display_plugin: default + id: default + display_title: Master + position: 0 + display_options: + access: + type: perm + options: + perm: 'access user profiles' + 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: Filter + 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: mini + options: + items_per_page: 10 + offset: 0 + id: 0 + total_pages: null + expose: + items_per_page: false + items_per_page_label: 'Items per page' + items_per_page_options: '5, 10, 25, 50' + items_per_page_options_all: false + items_per_page_options_all_label: '- All -' + offset: false + offset_label: Offset + tags: + previous: ‹‹ + next: ›› + style: + type: default + options: + grouping: { } + row_class: '' + default_row_class: true + uses_fields: false + row: + type: fields + options: + inline: { } + separator: '' + hide_empty: false + default_field_elements: true + fields: + roles: + id: roles + table: user__roles + field: roles + 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: target_id + type: entity_reference_label + settings: + link: true + group_column: target_id + 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: user + entity_field: roles + plugin_id: field + filters: + roles: + id: roles + table: user__roles + field: roles + relationship: none + group_type: group + admin_label: '' + operator: '=' + value: '' + group: 1 + exposed: false + expose: + operator_id: '' + label: '' + description: '' + use_operator: false + operator: '' + identifier: '' + required: false + remember: false + multiple: false + remember_roles: + authenticated: authenticated + 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: user + entity_field: roles + plugin_id: string + sorts: { } + header: { } + footer: { } + empty: { } + relationships: { } + arguments: + roles: + id: roles + table: user__roles + field: roles + 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: { } + glossary: false + limit: 0 + case: none + path_case: none + transform_dash: false + break_phrase: false + entity_type: user + entity_field: roles + plugin_id: string + display_extenders: { } + cache_metadata: + max-age: -1 + contexts: + - 'languages:language_content' + - 'languages:language_interface' + - url + - url.query_args + - user.permissions + tags: { } diff --git a/core/modules/user/src/Plugin/views/filter/Roles.php b/core/modules/user/src/Plugin/views/filter/Roles.php index d84e324..83b68c4 100644 --- a/core/modules/user/src/Plugin/views/filter/Roles.php +++ b/core/modules/user/src/Plugin/views/filter/Roles.php @@ -74,7 +74,15 @@ public function operators() { */ public function calculateDependencies() { $dependencies = []; - foreach ($this->value as $role_id) { + + // Due to a bug $this->value might be a string, see + // https://www.drupal.org/node/2846614. In the empty case stop early. + // Otherwise we cast it to an array later. + if (is_string($this->value) && $this->value === '') { + return []; + } + + foreach ((array) $this->value as $role_id) { $role = $this->roleStorage->load($role_id); $dependencies[$role->getConfigDependencyKey()][] = $role->getConfigDependencyName(); } diff --git a/core/modules/user/src/UserViewsData.php b/core/modules/user/src/UserViewsData.php index 2c79114..854f679 100644 --- a/core/modules/user/src/UserViewsData.php +++ b/core/modules/user/src/UserViewsData.php @@ -220,34 +220,20 @@ public function getViewsData() { ], ]; - $data['user__roles']['table']['group'] = $this->t('User'); + // Alter the user roles target_id column. + $data['user__roles']['roles_target_id']['field']['id'] = 'user_roles'; + $data['user__roles']['roles_target_id']['field']['no group by'] = TRUE; - $data['user__roles']['table']['join'] = [ - 'users_field_data' => [ - 'left_field' => 'uid', - 'field' => 'entity_id', - ], - ]; + $data['user__roles']['roles_target_id']['filter']['id'] = 'user_roles'; + $data['user__roles']['roles_target_id']['filter']['allow empty'] = TRUE; - $data['user__roles']['roles_target_id'] = [ - 'title' => $this->t('Roles'), - 'help' => $this->t('Roles that a user belongs to.'), - 'field' => [ - 'id' => 'user_roles', - 'no group by' => TRUE, - ], - 'filter' => [ - 'id' => 'user_roles', - 'allow empty' => TRUE, - ], - 'argument' => [ - 'id' => 'user__roles_rid', - 'name table' => 'role', - 'name field' => 'name', - 'empty field name' => $this->t('No role'), - 'zero is null' => TRUE, - 'numeric' => TRUE, - ], + $data['user__roles']['roles_target_id']['argument'] = [ + 'id' => 'user__roles_rid', + 'name table' => 'role', + 'name field' => 'name', + 'empty field name' => $this->t('No role'), + 'zero is null' => TRUE, + 'numeric' => TRUE, ]; $data['user__roles']['permission'] = [ diff --git a/core/modules/views/src/EntityViewsData.php b/core/modules/views/src/EntityViewsData.php index 2ba2086..20fd1d9 100644 --- a/core/modules/views/src/EntityViewsData.php +++ b/core/modules/views/src/EntityViewsData.php @@ -371,13 +371,10 @@ protected function mapFieldDefinition($table, $field_name, FieldDefinitionInterf // @todo Introduce concept of the "main" column for a field, rather than // assuming the first one is the main column. See also what the // mapSingleFieldViewsData() method does with $first. - $multiple = (count($field_column_mapping) > 1); $first = TRUE; foreach ($field_column_mapping as $field_column_name => $schema_field_name) { - $views_field_name = ($multiple) ? $field_name . '__' . $field_column_name : $field_name; - $table_data[$views_field_name] = $this->mapSingleFieldViewsData($table, $field_name, $field_definition_type, $field_column_name, $field_schema['columns'][$field_column_name]['type'], $first, $field_definition); - - $table_data[$views_field_name]['entity field'] = $field_name; + $table_data[$schema_field_name] = $this->mapSingleFieldViewsData($table, $field_name, $field_definition_type, $field_column_name, $field_schema['columns'][$field_column_name]['type'], $first, $field_definition); + $table_data[$schema_field_name]['entity field'] = $field_name; $first = FALSE; } } diff --git a/core/modules/views/src/Tests/Update/EntityViewsMultiValueBaseFieldDataUpdateTest.php b/core/modules/views/src/Tests/Update/EntityViewsMultiValueBaseFieldDataUpdateTest.php new file mode 100644 index 0000000..642dc4d --- /dev/null +++ b/core/modules/views/src/Tests/Update/EntityViewsMultiValueBaseFieldDataUpdateTest.php @@ -0,0 +1,56 @@ +databaseDumpFiles = [ + __DIR__ . '/../../../../system/tests/fixtures/update/drupal-8.bare.standard.php.gz', + __DIR__ . '/../../../../system/tests/fixtures/update/drupal-8.views-entity-views-data-2846614.php', + ]; + } + + /** + * Tests multi-value base field views data is updated correctly. + */ + public function testUpdateMultiValueBaseFields() { + $this->runUpdates(); + + $view = Views::getView('test_user_multi_value'); + $display = $view->storage->get('display'); + + // Check each handler type present in the configuration to make sure the + // field got updated correctly. + foreach (['fields', 'filters', 'arguments'] as $type) { + $handler_config = $display['default']['display_options'][$type]['roles']; + + // The ID should remain unchanged. Otherwise the update handler could + // overwrite a separate handler config. + $this->assertEqual('roles', $handler_config['id']); + // The field should be updated from 'roles' to the correct column name. + $this->assertEqual('roles_target_id', $handler_config['field']); + // Check the table is still correct. + $this->assertEqual('user__roles', $handler_config['table']); + + + // The plugin ID should be updated as well. + $this->assertEqual($type === 'arguments' ? 'user__roles_rid' : 'user_roles', $handler_config['plugin_id']); + } + } + +} diff --git a/core/modules/views/tests/modules/views_test_config/test_views/views.view.test_entity_multivalue_basefield.yml b/core/modules/views/tests/modules/views_test_config/test_views/views.view.test_entity_multivalue_basefield.yml index 478071d..d2ecb57 100644 --- a/core/modules/views/tests/modules/views_test_config/test_views/views.view.test_entity_multivalue_basefield.yml +++ b/core/modules/views/tests/modules/views_test_config/test_views/views.view.test_entity_multivalue_basefield.yml @@ -30,7 +30,7 @@ display: name: id: name table: entity_test_multivalue_basefield__name - field: name + field: name_value plugin_id: field entity_type: entity_test_multivalue_basefield entity_field: name diff --git a/core/modules/views/tests/src/Unit/EntityViewsDataTest.php b/core/modules/views/tests/src/Unit/EntityViewsDataTest.php index 74bcc08..7c664a4 100644 --- a/core/modules/views/tests/src/Unit/EntityViewsDataTest.php +++ b/core/modules/views/tests/src/Unit/EntityViewsDataTest.php @@ -469,7 +469,7 @@ public function testBaseTableFields() { ['description', ['value' => 'description__value', 'format' => 'description__format']], ['homepage', ['value' => 'homepage']], ['user_id', ['target_id' => 'user_id']], - ['string', ['value' => 'value']], + ['string', ['value' => 'string_value']], ]); $table_mapping->expects($this->any()) ->method('getFieldNames') @@ -526,8 +526,12 @@ public function testBaseTableFields() { $this->assertEquals('users_field_data', $relationship['base']); $this->assertEquals('uid', $relationship['base field']); - $this->assertStringField($data['entity_test__string']['string']); - $this->assertField($data['entity_test__string']['string'], 'string'); + // The string field name should be used as the 'entity field' but the actual + // field should reflect what the column mapping is using for multi-value + // base fields NOT just the field name. The actual column name returned from + // mappings in the test mocks is 'value'. + $this->assertStringField($data['entity_test__string']['string_value']); + $this->assertField($data['entity_test__string']['string_value'], 'string'); $this->assertEquals([ 'left_field' => 'id', 'field' => 'entity_id', @@ -591,7 +595,7 @@ public function testDataTableFields() { ['description', ['value' => 'description__value', 'format' => 'description__format']], ['homepage', ['value' => 'homepage']], ['user_id', ['target_id' => 'user_id']], - ['string', ['value' => 'value']], + ['string', ['value' => 'string_value']], ]); $table_mapping->expects($this->any()) ->method('getFieldNames') @@ -681,8 +685,8 @@ public function testDataTableFields() { $this->assertEquals('users_field_data', $relationship['base']); $this->assertEquals('uid', $relationship['base field']); - $this->assertStringField($data['entity_test_mul__string']['string']); - $this->assertField($data['entity_test_mul__string']['string'], 'string'); + $this->assertStringField($data['entity_test_mul__string']['string_value']); + $this->assertField($data['entity_test_mul__string']['string_value'], 'string'); $this->assertEquals([ 'left_field' => 'id', 'field' => 'entity_id', @@ -740,8 +744,8 @@ public function testRevisionTableFields() { ['description', ['value' => 'description__value', 'format' => 'description__format']], ['homepage', ['value' => 'homepage']], ['user_id', ['target_id' => 'user_id']], - ['revision_id', ['value' => 'id']], - ['string', ['value' => 'value']], + ['revision_id', ['value' => 'revision_id']], + ['string', ['value' => 'string_value']], ]); $table_mapping->expects($this->any()) ->method('getFieldNames') @@ -866,8 +870,8 @@ public function testRevisionTableFields() { $this->assertEquals('users_field_data', $relationship['base']); $this->assertEquals('uid', $relationship['base field']); - $this->assertStringField($data['entity_test_mulrev__string']['string']); - $this->assertField($data['entity_test_mulrev__string']['string'], 'string'); + $this->assertStringField($data['entity_test_mulrev__string']['string_value']); + $this->assertField($data['entity_test_mulrev__string']['string_value'], 'string'); $this->assertEquals([ 'left_field' => 'id', 'field' => 'entity_id', @@ -878,8 +882,8 @@ public function testRevisionTableFields() { ]], ], $data['entity_test_mulrev__string']['table']['join']['entity_test_mulrev_property_data']); - $this->assertStringField($data['entity_test_mulrev_revision__string']['string']); - $this->assertField($data['entity_test_mulrev_revision__string']['string'], 'string'); + $this->assertStringField($data['entity_test_mulrev_revision__string']['string_value']); + $this->assertField($data['entity_test_mulrev_revision__string']['string_value'], 'string'); $this->assertEquals([ 'left_field' => 'revision_id', 'field' => 'entity_id', diff --git a/core/modules/views/views.install b/core/modules/views/views.install index f542416..e044134 100644 --- a/core/modules/views/views.install +++ b/core/modules/views/views.install @@ -4,6 +4,8 @@ * @file * Contains install and update functions for Views. */ +use Drupal\Core\Config\Schema\ArrayElement; +use Drupal\views\Views; /** * Implements hook_install(). @@ -370,3 +372,138 @@ function views_update_8200() { function views_update_8201() { // Empty update to cause a cache rebuild so that config schema get refreshed. } + +/** + * Update field names for multi-value base fields. + */ +function views_update_8300() { + // Find all multi-value base fields for content entities. + $entity_type_manager = \Drupal::entityTypeManager(); + $entity_field_manager = \Drupal::service('entity_field.manager'); + $table_update_info = []; + + foreach ($entity_type_manager->getDefinitions() as $entity_type_id => $entity_type) { + if ($entity_type->hasHandlerClass('views_data')) { + $base_field_definitions = $entity_field_manager->getBaseFieldDefinitions($entity_type_id); + + $entity_storage = $entity_type_manager->getStorage($entity_type_id); + $table_mapping = $entity_storage->getTableMapping($base_field_definitions); + + foreach ($base_field_definitions as $field_name => $base_field_definition) { + $base_field_storage_definition = $base_field_definition->getFieldStorageDefinition(); + + // Skip single value and custom storage base fields. + if (!$base_field_storage_definition->isMultiple() || $base_field_storage_definition->hasCustomStorage()) { + continue; + } + + // Get the actual table, as well as the column for the main property + // name so we can perform an update later on the views. + $table_name = $table_mapping->getFieldTableName($field_name); + $main_property_name = $base_field_storage_definition->getMainPropertyName(); + + $table_update_info[$table_name][$field_name] = $table_mapping->getFieldColumnName($base_field_storage_definition, $main_property_name); + } + } + } + + if (empty($table_update_info)) { + return; + } + + $config_factory = \Drupal::configFactory(); + /** @var \Drupal\Core\Config\TypedConfigManagerInterface $typed_config_manager */ + $typed_config_manager = \Drupal::service('config.typed'); + $views_data = Views::viewsData(); + $handler_types = ['field', 'argument', 'sort', 'relationship', 'filter']; + + $required_cleanup_handlers = []; + foreach ($config_factory->listAll('views.view.') as $id) { + $view = $config_factory->getEditable($id); + $changed = FALSE; + + foreach ($view->get('display') as $display_id => &$display) { + foreach ($handler_types as $handler_type_singular) { + $handler_type_plural = $handler_type_singular . 's'; + $handler_data = $view->get("display.$display_id.display_options.$handler_type_plural"); + + if (empty($handler_data)) { + continue; + } + + foreach ($handler_data as $key => $data) { + // If this handler has a table we're interested in, update the field + // name. + $table = $data['table']; + if (isset($table_update_info[$table])) { + $path_to_handler = "display.$display_id.display_options.$handler_type_plural.$key"; + $path_field = "{$path_to_handler}.field"; + $path_plugin_id = "{$path_to_handler}.plugin_id"; + $original_field_name = $view->get($path_field); + + // Only if the wrong field name is set do we change the field. It + // could already be using the correct field. Like + // user__roles/roles_target_id. + if (isset($table_update_info[$table][$original_field_name])) { + $required_cleanup_handlers[$id][] = $path_to_handler; + + // Set both the new table field as well as new 'plugin_id' field. + $view->set($path_field, $table_update_info[$table][$original_field_name]); + $view->set($path_plugin_id, $views_data->get($table)[$table_update_info[$table][$original_field_name]][$handler_type_singular]['id']); + + $changed = TRUE; + } + } + } + } + } + + if ($changed) { + $view->save(TRUE); + } + } + + // Beside of updating the field and plugin ID we also need to truncate orphan + // keys so he configuration applies to the config schema. + // We cannot do that inline in the other code, due to caching issues with + // typed configuration. + foreach ($required_cleanup_handlers as $id => $paths_to_handlers) { + $changed = FALSE; + $typed_view = $typed_config_manager->get($id); + $view = $config_factory->getEditable($id); + foreach ($paths_to_handlers as $path_to_handler) { + /** @var \Drupal\Core\Config\Schema\TypedConfigInterface $typed_view */ + + /** @var \Drupal\Core\Config\Schema\ArrayElement $typed_config */ + $typed_config = $typed_view->get($path_to_handler); + $config = $typed_config->getValue(); + + // Filter values we want to convert from a string to an array. + if (strpos($path_to_handler, 'filters') !== FALSE && $typed_config->get('value') instanceof ArrayElement && is_string($config['value'])) { + // An empty string casted to an array is an array with one + // element. + if ($config['value'] === '') { + $config['value'] = []; + } + else { + $config['value'] = (array) $config['value']; + } + } + + // For all the other fields we try to determine the fields using + // config schema and remove everything which is not needed. + foreach (array_keys($config) as $config_key) { + if (!isset($typed_config->getDataDefinition()['mapping'][$config_key])) { + unset($config[$config_key]); + $changed = TRUE; + } + } + $typed_config->setValue($config); + $view->set($path_to_handler, $typed_config->getValue()); + } + + if ($changed) { + $view->save(); + } + } +}