Provide views relationships for DER base fields

For ER core adds data in EntityViewsData::processViewsDataForEntityReference() see #1740492: Implement a default entity views data handler. Use of hook_views_data seems wrong because it adds new data where as we are altering existing data so maybe hook_views_data_alter. @dawehner what do you suggest?

Updated summary.

Let's not combine both the hooks. We should keep dynamic_entity_reference_field_views_data() because it is the proper way and I think we can use dynamic_entity_reference_views_data() for base fields so that other modules can change that in hook_views_data_alter(). Note that dynamic_entity_reference_views_data() is a temporary fix till we have a proper api after #2337515: Allow @FieldType to customize views data.
I have worked on this over the weekend. I used the patch from #6. I have setup a test module with basefields found some nasty edge cases. I'll write some tests later this week.

Re: #15 Patch looks good. Can you move base field relationships to hook_views_data as per #18? I can add my tests in #2542274: Write tests for DER field as basefield on an entity.
Does it work properly when a base field has a dedicated table?

+++ b/dynamic_entity_reference.views.inc
@@ -6,93 +6,236 @@
 function dynamic_entity_reference_field_views_data(FieldStorageConfigInterface $field_storage) {

Let's revert all the changes to this hook.

Can we do the changes in a different issue (out of scope)? Or is there something wrong with the changes?

Nothing wrong with the changes. I quiet like the changes. Let's move them to follow up and let's also fix them in core for ER field as well.

Added relationship data test for single field. Made some changes because of tests. Update docs.
Needs to add actual view to verify the relationship.
Unable to make the base field multivalue but is behaving as a multi field anyway.
Needs to relationship data test for multivalue field.
Needs to add actual view to verify the relationship.

+++ b/src/Tests/Views/DynamicEntityReferenceBaseFieldRelationshipTest.php
@@ -0,0 +1,324 @@
+    // Update definitions and schema.
+    $manager = \Drupal::entityDefinitionUpdateManager();
+    $storage_definition = $manager->getFieldStorageDefinition('dynamic_references', 'entity_test');
+    $storage_definition->setCardinality(FieldStorageDefinitionInterface::CARDINALITY_UNLIMITED);
+    $manager->updateFieldStorageDefinition($storage_definition);
+    $manager = \Drupal::entityDefinitionUpdateManager();
+    $storage_definition = $manager->getFieldStorageDefinition('dynamic_references', 'entity_test_mul');
+    $storage_definition->setCardinality(FieldStorageDefinitionInterface::CARDINALITY_UNLIMITED);
+    $manager->updateFieldStorageDefinition($storage_definition);

@plach can I do this in test? Or is there some easy way to update base field definition. dynamic_entity_reference_entity_test adds a base field named dynamic_references with 1 cardinality. For testSingleBaseFieldRelationship test that works fine but I have to make it an FieldStorageDefinitionInterface::CARDINALITY_UNLIMITED for second test testMultiBaseFieldRelationship. I tried above code in the test from https://www.drupal.org/node/2554097 but it gave exception.

+++ b/tests/modules/dynamic_entity_reference_entity_test/dynamic_entity_reference_entity_test.module
@@ -0,0 +1,52 @@
+      ->setCardinality(1)
+++ b/src/Tests/Views/DynamicEntityReferenceBaseFieldRelationshipTest.php
@@ -0,0 +1,324 @@
+    $entity->dynamic_references[] = $referenced_entity;
+    $entity->dynamic_references[] = $referenced_entity_mul;
...
+    $this->assertEqual($entity->dynamic_references[0]->entity->id(), $referenced_entity->id());
+    $this->assertEqual($entity->dynamic_references[1]->entity->id(), $referenced_entity_mul->id());
...
+    $entity->dynamic_references[] = $referenced_entity;
+    $entity->dynamic_references[] = $referenced_entity_mul;
...
+    $this->assertEqual($entity->dynamic_references[0]->entity->id(), $referenced_entity->id());
+    $this->assertEqual($entity->dynamic_references[1]->entity->id(), $referenced_entity_mul->id());

I set cardinality to 1 but how come I can set two values for a field and it passes? Is there something wrong with der?

Here is uncommented version of testMultiBaseFieldRelationship This patch will fail with exception.

Ok I found the way around #29.1 so unping @plach. But #29.2 is still an issue. Anyways this patch is green. Remaining thing is to check these with actual views.

+++ b/tests/modules/dynamic_entity_reference_entity_test/dynamic_entity_reference_entity_test.module
@@ -0,0 +1,61 @@
+    // @todo use \Drupal::state()->get('dynamic_entity_reference_entity_test', 0); here.
+    if (DynamicEntityReferenceBaseFieldRelationshipTest::$testRun) {

Why can't I use state here? Core tests use it all over the place. Any suggestions!

Here are the tests for basefield view tests with 1 cardinality, found a bug in views data and fixed it. Remaining tasks are doc fixes and cardinality > 1 testing with view.

As it turns out views integration for multi value basefields is broken #2477899: Multiple valued Base fields won't work in Views. We can't really write views tests for multi values der basefield not until we add relationship ourselves which seems hacky and can be moved to follow up. Only thing remaining here is write tests for entity_test_mul der basefield. I'll look at it later tonight.

1. Added views data for multi value basefield.
2. Added tests for data table entity

@todo

1. FIx views for multi value basefield.
2. Add tests for multi value basefield views.

Yay!!! found another issue in DERBaseFieldViewsData. Tests++

Added a do-not-test patch without any of the test codes.
Fixed views for multi value basefield.
@todo
Add tests for multi value basefield views.

Everything is done here. This is ready for reviews.

1. +++ b/src/Tests/Views/DynamicEntityReferenceBaseFieldRelationshipTest.php
   @@ -0,0 +1,662 @@
   +    // Check the backwards reference for test entity - data table using dynamic_references.
   

   In general, all lines of code should not be longer than 80 characters.
   https://www.drupal.org/coding-standards#linelength

2. +++ b/src/Tests/Views/DynamicEntityReferenceBaseFieldRelationshipTest.php
   @@ -0,0 +1,662 @@
   +    // Check the backwards reference for test entity - data table using dynamic_references.
   

   In general, all lines of code should not be longer than 80 characters.
   https://www.drupal.org/coding-standards#linelength

3. +++ b/src/Tests/Views/DynamicEntityReferenceBaseFieldRelationshipTest.php
   @@ -0,0 +1,662 @@
   +    // Check the backwards reference for test entity - data table using dynamic_references.
   

   In general, all lines of code should not be longer than 80 characters.
   https://www.drupal.org/coding-standards#linelength

Fixed #41, #42 and #43.

If the plan is to remove the code later. Then we must ensure the ID is identical, and core provides identical functionality.

The ID is identical that is why I was able to create views from gui.

Do you want to change the relationship ID? ($psuedo_field assignments) to prevent potential collisions with core.

Core is not providing any relationship. Core doesn't even know that relationship exists so no collision possible.

Updated setup function logic to accommodate #2542274: Write tests for DER field as basefield on an entity.

And interdiff.

Improvements from #2542274: Write tests for DER field as basefield on an entity.

This looks really good, I would say, ship it!

1. +++ b/dynamic_entity_reference.views.inc
   @@ -98,3 +102,150 @@ function dynamic_entity_reference_field_views_data(FieldStorageConfigInterface $
   +      // Unlimited (-1) or > 1 store field data in a dedicated table.
   +      $table = $field->getCardinality() == 1 ? $base_table :  $entity_type->getBaseTable() . '__' . $field_name;
   

   Afaik you could just use \Drupal\Core\Entity\Sql\TableMappingInterface::getFieldTableName here?

2. +++ b/dynamic_entity_reference.views.inc
   @@ -98,3 +102,150 @@ function dynamic_entity_reference_field_views_data(FieldStorageConfigInterface $
   +        $target_table = $target_entity_type->getDataTable() ?: $target_entity_type->getBaseTable();
   ...
   +          'base' => $target_table,
   

   I know this is probably an unsolved problem, but I just want to rise the problem that you might get translations by this join.

3. +++ b/dynamic_entity_reference.views.inc
   @@ -98,3 +102,150 @@ function dynamic_entity_reference_field_views_data(FieldStorageConfigInterface $
   +        // Relationship (Origin -> Target)
   

   I really like this docs!

4. +++ b/dynamic_entity_reference.views.inc
   @@ -98,3 +102,150 @@ function dynamic_entity_reference_field_views_data(FieldStorageConfigInterface $
   +                // Entity reference field only has one target type whereas dynamic
   +                // entity reference field can have multiple target types that is
   +                // why we need extra join condition on target types.
   +                'field' => $column_type,
   +                'value' => $target_entity_type_id,
   

   Great documentation!

When I attempted to apply provide_views-2548395-47.patch to the latest 7.x dev 2015-Aug-19, there were missing files that the patch would not create on it's own, nor could handle if a blank file of the name it needed was put in the module folder, then there were a bunch of yml files that couldn't complete either.

Notice this is with #47.

Same with #49.

patch < provide_views-2548395-49.patch
can't find file to patch at input line 5
Perhaps you should have used the -p or --strip option?
The text leading up to this was:
--------------------------
|diff --git a/dynamic_entity_reference.views.inc b/dynamic_entity_reference.views.inc
|index 7cb6de8..08133ba 100644
|--- a/dynamic_entity_reference.views.inc
|+++ b/dynamic_entity_reference.views.inc
--------------------------
File to patch: dynamic_entity_reference.views.inc
dynamic_entity_reference.views.inc: No such file or directory
Skip this patch? [y] n
File to patch:

Reroll after #2542274: Write tests for DER field as basefield on an entity.

@dawehner thank you so very much for the review. It was totally worth waiting for :).

1. I tried it but it didn't work for cardinality > 1.
2. Nice to know we'll keep in mind. Is there any core issue for this? Or any workaround for this?
3. @dpi has done that so @dpi++
4. :)

Pushed to 8.x-1.x.
Thank you @dpi for working on the initial patch. Can you please create the followup issue for #24, #25?

Thank you @dawehner for all your help in IRC as well.