Problem/Motivation

It is not possible to add relationships to a view based on entity reference fields, this is needed to use the referenced entity for fields/filters/sorts.

Will also become useful if we start to generalize this into a generic views data generation for entities, as we will also need it for things like the node -> author base field entity reference.

This is also functionality you currently get with the contrib entityreference module.

Proposed resolution

Add the necessary views data integration.

Remaining tasks

User interface changes

Adds new relationship options to the views UI.

API changes

Files: 
CommentFileSizeAuthor
#50 interdiff-1906806-50-2.txt2.74 KBdamiankloip
#50 interdiff-1906806-50.txt3.59 KBdamiankloip
#50 1906806-50.patch10.87 KBdamiankloip
PASSED: [[SimpleTest]]: [PHP 5.4 MySQL] 73,320 pass(es).
[ View ]
#32 interdiff-1906806-32.txt1.75 KBdamiankloip
#32 1906806-32.patch11.05 KBdamiankloip
FAILED: [[SimpleTest]]: [MySQL] Failed to run tests: @reason.
[ View ]
#30 drupal-1906806-29-interdiff.txt989 bytesBerdir
#30 drupal-1906806-29.patch10.82 KBBerdir
PASSED: [[SimpleTest]]: [PHP 5.4 MySQL] 72,615 pass(es).
[ View ]
#28 drupal-1906806-28-interdiff.txt3.71 KBBerdir
#28 drupal-1906806-28.patch10.82 KBBerdir
FAILED: [[SimpleTest]]: [PHP 5.4 MySQL] 72,586 pass(es), 1 fail(s), and 0 exception(s).
[ View ]
#26 drupal-1906806-26-interdiff.txt3.15 KBBerdir
#26 drupal-1906806-26.patch10.96 KBBerdir
FAILED: [[SimpleTest]]: [MySQL] Failed to run tests: @reason.
[ View ]
#24 drupal-1906806-23.patch10.86 KBBerdir
FAILED: [[SimpleTest]]: [MySQL] Failed to run tests: @reason.
[ View ]
#23 drupal-1906806-23-interdiff.txt12.72 KBBerdir
#16 drupal-1906806-15.patch11.18 KBSifro
FAILED: [[SimpleTest]]: [PHP 5.4 MySQL] Setup environment: Test cancelled by admin prior to completion.
[ View ]
#15 interdiff-1906806-12-15.txt2.18 KBSifro
#15 1906806-15.patch11.18 KBSifro
FAILED: [[SimpleTest]]: [PHP 5.4 MySQL] 64,713 pass(es), 1 fail(s), and 0 exception(s).
[ View ]
#12 interdiff-1906806-4-11.txt3.41 KBSifro
#12 drupal-1906806-12.patch11.19 KBSifro
FAILED: [[SimpleTest]]: [MySQL] 64,733 pass(es), 2 fail(s), and 1 exception(s).
[ View ]
#11 interdiff-1906806-4-11.txt3.41 KBSifro
#5 drupal-1906806-5.patch11.18 KBdawehner
FAILED: [[SimpleTest]]: [MySQL] 55,974 pass(es), 3 fail(s), and 0 exception(s).
[ View ]
#4 drupal-1906806-4.patch17.32 KBdawehner
FAILED: [[SimpleTest]]: [MySQL] 56,211 pass(es), 2 fail(s), and 0 exception(s).
[ View ]
#1 drupal-1906806-1.patch2.53 KBdawehner
FAILED: [[SimpleTest]]: [MySQL] Failed to run tests: @reason.
[ View ]

Comments

dawehner’s picture

StatusFileSize
new2.53 KB
FAILED: [[SimpleTest]]: [MySQL] Failed to run tests: @reason.
[ View ]

Just tracking some work, because #1906794: Using a relationship in the UI fails completly blocks this issue.

amateescu’s picture

+++ b/core/modules/entity_reference/entity_reference.views.inc
@@ -0,0 +1,60 @@
+      'field field' => $field['field_name'] . '_tid',

'_target_id' ;)

amateescu’s picture

Component:entity system» entity_reference.module

Also moving to the new component.

dawehner’s picture

StatusFileSize
new17.32 KB
FAILED: [[SimpleTest]]: [MySQL] 56,211 pass(es), 2 fail(s), and 0 exception(s).
[ View ]

Just tracking some more work.

dawehner’s picture

Status:Active» Needs review
StatusFileSize
new11.18 KB
FAILED: [[SimpleTest]]: [MySQL] 55,974 pass(es), 3 fail(s), and 0 exception(s).
[ View ]

Just a rerole.

The last submitted patch, drupal-1906806-5.patch, failed testing.

nicxvan’s picture

Shouldn't this be a beta blocker? I believe this is essential functionality for entity reference. Will this be fixed before beta?

stephenacrossri’s picture

I agree with nicxvan about this being a beta blocker. The value of having entity relationships is core is joining those relationships in views (now in core).

Berdir’s picture

This simply missing functionality that can be easily added at any point, it dosn't involve storage or API's, so there's no need for this to block beta.

If you want to see this moving forward, try it out, give feedback and try to fix those failing tests.

Sifro’s picture

Assigned:Unassigned» Sifro
Issue summary:View changes

I'll give this one a try

Sifro’s picture

Status:Needs work» Needs review
StatusFileSize
new3.41 KB

here is a small reroll.. it's my first attempt at contributing something, let's see what happens with the testbot

Sifro’s picture

StatusFileSize
new11.19 KB
FAILED: [[SimpleTest]]: [MySQL] 64,733 pass(es), 2 fail(s), and 1 exception(s).
[ View ]
new3.41 KB

Ok, uploading interdiff with patch.

Status:Needs review» Needs work

The last submitted patch, 12: drupal-1906806-12.patch, failed testing.

Sifro’s picture

Any chances that the exception is related to https://drupal.org/node/1783692 ?

Sifro’s picture

Status:Needs work» Needs review
StatusFileSize
new11.18 KB
FAILED: [[SimpleTest]]: [PHP 5.4 MySQL] 64,713 pass(es), 1 fail(s), and 0 exception(s).
[ View ]
new2.18 KB
Sifro’s picture

StatusFileSize
new11.18 KB
FAILED: [[SimpleTest]]: [PHP 5.4 MySQL] Setup environment: Test cancelled by admin prior to completion.
[ View ]

oh please delete me.

Status:Needs review» Needs work

The last submitted patch, 16: drupal-1906806-15.patch, failed testing.

The last submitted patch, 15: 1906806-15.patch, failed testing.

RoySegall’s picture

Joining this issue. I'll see how i can help this.

Sifro’s picture

I'll bit a bit busy for the next 10 days, so if in the meanwhile someone wants to help, it'll defintiely speed up the process.

I've made some progress since the last patch i posted, but they aren't really ready for posting.

IMO, the next thing to do is to convert the field_create_field() function ( called in EntityReferenceRelationshipTest::setUp() ) to use the new entity API.
After some searches, it looks like we need to use the function

entity_create('field_config', $field)->save();

RoySegall’s picture

@Sifro - if you made any progress i think you should post it any way. This still a progress you made :)

Sifro’s picture

Assigned:Sifro» Unassigned

Can't finish it in the short term because of unexpected workload... if someone wants to take this, feel free to do it.

@RoySegall: it's not worth it to post an interdiff with the latest minor changes I've made, as they don't really fix any failures.

However, this is what I was working on: in EntityReferenceRelationshipTest::setUp()....

field_create_field($field);

became

    $field = array(
      'translatable' => FALSE,
      'entity_type' => 'entity_test',
      'settings' => array(
        'target_type' => 'entity_test',
      ),
      'name' => 'field_test',
      'type' => 'entity_reference',
      'cardinality' => 'FIELD_CARDINALITY_UNLIMITED',
    );

entity_create('field_config', $field)->save();

and...

field_create_instance($instance);

became...

  $instance = array(
      'entity_type' => 'entity_test',
      'field_name' => 'field_test',
      'bundle' => 'entity_test',
      'widget' => array(
        'type' => 'options_select',
      ),
      'settings' => array(
        'handler' => 'default',
        'handler_settings' => array(),
      ),
    );

entity_create('field_instance_config', $instance)->save();

For some reasons, it still doesn't work, and couldn't figure out why :(

Berdir’s picture

Status:Needs work» Needs review
StatusFileSize
new12.72 KB

Yeah, this was pretty old :)

Tests is now passing again for me, might still need more changes...

Berdir’s picture

StatusFileSize
new10.86 KB
FAILED: [[SimpleTest]]: [MySQL] Failed to run tests: @reason.
[ View ]

And the actual patch...

tim.plunkett’s picture

Nothing but nitpicks, this looks good.

Also, the issue summary doesn't exist.

  1. +++ b/core/modules/entity_reference/entity_reference.views.inc
    @@ -0,0 +1,62 @@
    +  $entity_manager = Drupal::entityManager();

    \Drupal

  2. +++ b/core/modules/entity_reference/entity_reference.views.inc
    @@ -0,0 +1,62 @@
    +      '@field_name' => $field->getName()

    Missing comma

  3. +++ b/core/modules/entity_reference/src/Tests/Views/EntityReferenceRelationshipTest.php
    @@ -0,0 +1,145 @@
    + * @see entity_reference_field_views_data

    Missing ()

  4. +++ b/core/modules/entity_reference/src/Tests/Views/EntityReferenceRelationshipTest.php
    @@ -0,0 +1,145 @@
    +  public static function getInfo() {
    ...
    +  protected function setUp() {

    {@inheritdoc}

Berdir’s picture

StatusFileSize
new10.96 KB
FAILED: [[SimpleTest]]: [MySQL] Failed to run tests: @reason.
[ View ]
new3.15 KB

Thanks for the review, fixed that and also made it a unit/kerneltest, 9s => 2s.

damiankloip’s picture

  1. +++ b/core/modules/entity_reference/src/Tests/Views/EntityReferenceRelationshipTest.php
    @@ -0,0 +1,152 @@
    +    $entity_0 = $entity_storage->create(array());
    ...
    +    $entity = $entity_storage->create(array());
    ...
    +    $entity = $entity_storage->create(array('field_test' => $entity->id()));

    weird that one is entity_0 and the others are entity. I know the first one is used to get referenced. Maybe we can call it $referenced_entity?

  2. +++ b/core/modules/entity_reference/src/Tests/Views/EntityReferenceRelationshipTest.php
    @@ -0,0 +1,152 @@
    +    $views_data_field_test = $this->container->get('views.views_data')->get('entity_test__field_test');
    ...
    +    $views_data_entity_test = $this->container->get('views.views_data')->get('entity_test');

    You can use Views::viewsData()->get() is you like.

  3. +++ b/core/modules/entity_reference/src/Tests/Views/EntityReferenceRelationshipTest.php
    @@ -0,0 +1,152 @@
    +    // Check an actually test view.

    'Check an actual test view' ?

  4. +++ b/core/modules/entity_reference/src/Tests/Views/EntityReferenceRelationshipTest.php
    @@ -0,0 +1,152 @@
    +      $this->assertEqual($view->style_plugin->getField($index, 'id'), $this->entities[$index + 1]->id());
    ...
    +      $this->assertEqual($view->style_plugin->getField($index, 'id_1'), $index == 0 ? 0 : 1);
    ...
    +      $this->assertEqual($view->style_plugin->getField($index, 'id'), $this->entities[$index + 1]->id());
    ...
    +       $this->assertEqual($view->style_plugin->getField($index, 'id_1'), $index == 0 ? 0 : 1);

    maybe getFieldValue() would be better here? or just checking the result data directly?

Berdir’s picture

StatusFileSize
new10.82 KB
FAILED: [[SimpleTest]]: [PHP 5.4 MySQL] 72,586 pass(es), 1 fail(s), and 0 exception(s).
[ View ]
new3.71 KB

1. Good point, found the _0 weird too when I first re-rolled this, renamed to $referenced_entity. also removed the @todo, I commented it out and it's no longer an issue but forgot to actually remove it and the @todo.
2. Updated.
3. Yeah, fixed.
4. getFieldValue() sounds nice, but it's a protected method. And not sure that I should be accessing the data result directly?

damiankloip’s picture

Oh yeah. Forgot that is a protected method.. :)

What's wrong with just looking at data directly? The tests are about the relationship data, not style plugins or field rendering.

Berdir’s picture

StatusFileSize
new10.82 KB
PASSED: [[SimpleTest]]: [PHP 5.4 MySQL] 72,615 pass(es).
[ View ]
new989 bytes

Uhm, just tried it out and then forgot to change it back..

If you want to check the result data directly, please update the patch, I'm not exactly sure how to do it properly :)

The last submitted patch, 28: drupal-1906806-28.patch, failed testing.

damiankloip’s picture

StatusFileSize
new11.05 KB
FAILED: [[SimpleTest]]: [MySQL] Failed to run tests: @reason.
[ View ]
new1.75 KB

ok :) I just mean something like this. I added a couple of comments too.

damiankloip’s picture

StatusFileSize
new11.06 KB
FAILED: [[SimpleTest]]: [MySQL] Failed to run tests: @reason.
[ View ]
new1.1 KB

Some small quibbles.

Berdir’s picture

Issue summary:View changes
Issue tags:-Needs issue summary update

Updated issue summary, not much to write I think?

damiankloip’s picture

Issue summary:View changes
tim.plunkett’s picture

Status:Needs review» Reviewed & tested by the community

Nice tests. I think the improvements since #25 are good.

tstoeckler’s picture

Status:Reviewed & tested by the community» Needs review
  1. +++ b/core/modules/entity_reference/entity_reference.views.inc
    @@ -0,0 +1,62 @@
    +    $target_type = $field->getSetting('target_type');
    +    $target_entity_type = $entity_manager->getDefinition($target_type);

    These are very bad variable names. If we do need both variables the former should be called $target_entity_type_id. Or it should be called $target_type_id and the latter should be called $target_type.

  2. +++ b/core/modules/entity_reference/entity_reference.views.inc
    @@ -0,0 +1,62 @@
    +    // Provide a relationship from field entity type to the referenced entity
    +    // type.
    ...
    +    // Provide a relationship from the referenced entity type to the field
    +    // entity type.

    As FieldConfig implements EntityInterface speaking of a "field entity type" is very, very confusing. Let's call this the "entity type with the entity reference field" vs. the "entity type that is referenced by the field".

  3. +++ b/core/modules/entity_reference/entity_reference.views.inc
    @@ -0,0 +1,62 @@
    +    $pseudo_field_name = 'reverse_' . $field->getTargetEntityTypeId() . '__' . $field->getName();

    Seems a bit inconsistent to use a single underscore and a double underscore as separators in a single string?!

  4. +++ b/core/modules/entity_reference/entity_reference.views.inc
    @@ -0,0 +1,62 @@
    +      'title' => t('@label using @field_name', array(
    +        '@label' => $target_entity_type->getLabel(),
    +        '@field_name' => $field->getName(),
    +      )),
    +      'help' => t('Relate each @label with a @field_name.', array(
    +        '@label' => $target_entity_type->getLabel(),
    +        '@field_name' => $field->getName(),
    +      )),

    $args should be used here as well.

jibran’s picture

Version:6.0» 8.x-dev

Nice try.

damiankloip’s picture

Assigned:» Unassigned
Status:Needs work» Needs review
StatusFileSize
new10.87 KB
PASSED: [[SimpleTest]]: [PHP 5.4 MySQL] 73,320 pass(es).
[ View ]
new3.59 KB
new2.74 KB

ok, I renamed the first variable to $target_entity_type_id - which I think is totally accurate. That's what it is. $target_entity_type is now fine IMO. Updated doc comments, replaced $arg usage, and used double underscore to separate the reverse pseudo table name. A couple of other small tweaks like the title and description of the tests. Since we are really just testing the data integration and not a new handler.

Two interdiffs because I forgot to include the tests changes and was too lazy to roll back into one... sorry! :p

tstoeckler’s picture

Status:Needs review» Reviewed & tested by the community

Awesome, thanks a lot. Was RTBC before, so marking RTBC again.

Status:Reviewed & tested by the community» Needs work

The last submitted patch, 50: 1906806-50.patch, failed testing.

damiankloip’s picture

50: 1906806-50.patch queued for re-testing.

damiankloip’s picture

Status:Needs work» Reviewed & tested by the community
alexpott’s picture

Status:Reviewed & tested by the community» Fixed

Committed 7ba018f and pushed to 8.x. Thanks!

Status:Fixed» Closed (fixed)

Automatically closed - issue fixed for 2 weeks with no activity.