Skip to content
Snippets Groups Projects

Adds computed bundle basefield support in views

Closed Adds computed bundle basefield support in views
issue/drupal-2981047:2981047-views-computed-bundle-fields-10-0 into 10.0.x
6 unresolved threads
Closed Ankitha Shetty requested to merge issue/drupal-2981047:2981047-views-computed-bundle-fields-10-0 into 10.0.x
6 unresolved threads

Closes #2981047

Merge request reports

Loading
Loading

Activity

Filter activity
  • Approvals
  • Assignees & reviewers
  • Comments (from bots)
  • Comments (from users)
  • Commits & branches
  • Edits
  • Labels
  • Lock status
  • Mentions
  • Merge request status
  • Tracking
core/modules/system/tests/modules/entity_test/src/Entity/EntityTestComputedField.php
59 64 return $fields;
60 65 }
61 66
67 /**
68 * {@inheritdoc}
69 */
70 public static function bundleFieldDefinitions(EntityTypeInterface $entity_type, $bundle, array $base_field_definitions) {
71 $fields = parent::bundleFieldDefinitions($entity_type, $bundle, $base_field_definitions);
72
73 $computed_field_bundles = [
74 'entity_test_comp_field_bundle',
75 'entity_test_comp_field_bundle_2',
76 ];
77
78 if (in_array($bundle, $computed_field_bundles) !== FALSE) {
  • Lee Rowlands
    Lee Rowlands @larowlan
    Maintainer
    Suggested change
    78 if (in_array($bundle, $computed_field_bundles) !== FALSE) {
    78 if (in_array($bundle, $computed_field_bundles, TRUE)) {

    We don't need the !== FALSE here, in_array only returns a bool. But we may as well add the third argument here because we only expect strings

  • Please register or sign in to reply
  • Lee Rowlands
    Lee Rowlands @larowlan started a thread on the diff
    • core/modules/system/tests/modules/entity_test/src/Entity/EntityTestComputedField.php
    23 26 * "id" = "id",
    24 27 * "uuid" = "uuid",
    25 28 * "label" = "name",
    29 * "bundle" = "type",
    • Lee Rowlands
      Lee Rowlands @larowlan
      Maintainer

      We've had to make changes to core tests to allow for this entity type now having bundles. Could this also be disruptive to contrib tests? If so, should we just add a new entity-type that has both bundles and computed fields?

    • Please register or sign in to reply
  • Lee Rowlands
    Lee Rowlands @larowlan started a thread on an old version of the diff
    • core/modules/views/src/Plugin/views/field/EntityField.php
    360 // support them. If bundles were specified as part of the definition, check
    361 // all the field definitions, else only base fields can be checked.
    361 362 // @see \Drupal\Core\Entity\EntityFieldManager::getFieldStorageDefinitions()
    362 $base_fields = $this->entityFieldManager->getBaseFieldDefinitions($entity_type_id);
    363 if (isset($this->definition['field_name']) && isset($base_fields[$this->definition['field_name']])) {
    364 return $base_fields[$this->definition['field_name']]->getFieldStorageDefinition();
    363 if (!isset($this->definition['field_name'])) {
    364 return NULL;
    365 }
    366 if (isset($this->definition['bundles'])) {
    367 foreach ($this->definition['bundles'] as $bundle) {
    368 $fields = $this->entityFieldManager->getFieldDefinitions($entity_type_id, $bundle);
    369 if (isset($fields[$this->definition['field_name']])) {
    370 return $fields[$this->definition['field_name']]->getFieldStorageDefinition();
    371 }
    372 }
  • Joachim Noreiko
    Joachim Noreiko @joachim started a thread on the diff
    • core/modules/system/tests/modules/entity_test/src/EntityTestViewsData.php
    25 25 'field_name' => 'computed_string_field',
    26 26 ],
    27 27 ];
    28 $views_data['entity_test_computed_field']['computed_bundle_field'] = [
  • Joachim Noreiko
    Joachim Noreiko @joachim started a thread on an old version of the diff
    • core/modules/views/src/Plugin/views/field/EntityField.php
    356 356 }
    357 357
    358 358 // The list of field storage definitions above does not include computed
    359 // base fields, so we need to explicitly fetch a list of all base fields in
    360 // order to support them.
    359 // fields, so we need to explicitly fetch a list of all fields in order to
    360 // support them. If bundles were specified as part of the definition, check
    361 // all the field definitions, else only base fields can be checked.
    361 362 // @see \Drupal\Core\Entity\EntityFieldManager::getFieldStorageDefinitions()
    362 $base_fields = $this->entityFieldManager->getBaseFieldDefinitions($entity_type_id);
    363 if (isset($this->definition['field_name']) && isset($base_fields[$this->definition['field_name']])) {
    364 return $base_fields[$this->definition['field_name']]->getFieldStorageDefinition();
    363 if (!isset($this->definition['field_name'])) {
    364 return NULL;
    365 }
    366 if (isset($this->definition['bundles'])) {
    • Joachim Noreiko
      Joachim Noreiko @joachim

      I don't think this is necessary.

      You can determine which bundles a computed field is on from the entity field manager, from the field map for instance.

    • Joachim Noreiko
      Joachim Noreiko @joachim

      On second thoughts, I don't think we need this at all! The only reason we have the 'bundles' in the definition is so that we can get a field definition. We loop over each bundle, and take the first one. We don't actually care WHICH bundles it's on. So, we could more easily loop over ALL bundles, and take the first one where we find the field. Or we can consult the field map, and take the first one.

      We're using the presence of 'bundles' to detect whether this is a computed field, but it would be simpler to check for base fields first, and then fall back to bundle fields.

    • Owen Bush
      Owen Bush @owenbush

      I've been working on this in DrupalCon Pittsburgh, and I've managed to resolve pretty much all the other comments - including creating a new entity type for computed bundle base fields.

      However, the field map does not appear to contain the bundle base fields as defined in my entity's "bundleFieldDefinitions()".

      They are visible if I use the EntityFieldManager::getFieldDefinitions() but not in the map.

      I'm not sure if this is expected due to the way the map is cached only on create and delete, but I just do not seem to be able to get these bundle fields to appear. Base fields appear fine, but not bundle base fields.

      I do not have access to the EntityTypeBundleInfo service within the EntityField class to determine the bundles available for the entity.

      So that leaves me with a couple of options:

      1. I do not remove the 'bundles' key on the EntityViewsData definition, as I was doing before, and load the first field definition from the EntityFieldManager.

      2. I add the EntityTypeBundleInfo service with dependency injection to the EntityField views class.

      Thoughts on the direction?

      Edited by Owen Bush
    • Joachim Noreiko changed this line in version 8 of the diff

      changed this line in version 8 of the diff

    • Please register or sign in to reply
  • Joachim Noreiko
    Joachim Noreiko @joachim started a thread on the diff
    • core/modules/views/src/Plugin/views/field/EntityField.php
    363 if (isset($this->definition['field_name']) && isset($base_fields[$this->definition['field_name']])) {
    364 return $base_fields[$this->definition['field_name']]->getFieldStorageDefinition();
    363 if (!isset($this->definition['field_name'])) {
    364 return NULL;
    365 }
    366 if (isset($this->definition['bundles'])) {
    367 foreach ($this->definition['bundles'] as $bundle) {
    368 $fields = $this->entityFieldManager->getFieldDefinitions($entity_type_id, $bundle);
    369 if (isset($fields[$this->definition['field_name']])) {
    370 return $fields[$this->definition['field_name']]->getFieldStorageDefinition();
    371 }
    372 }
    373 return NULL;
    374 }
    375
    376 $fields = $this->entityFieldManager->getBaseFieldDefinitions($entity_type_id);
  • Joachim Noreiko added 2 commits

    added 2 commits

    • 5d10123e - Fixed test class docblock.
    • 853e97d9 - Removed need to declare bundles in views data for a computed bundle field.

    Compare with previous version

  • Alex Pott closed

    closed

    • Please register or sign in to reply