Expose non-base fields in referenced entities in Add Fields

I have fixed the "cosmetic" problem renaming the $label. I'm attaching the patch and the interdiff.

It works fine for me and I'll test it more during these days. Great work miiimooo!

It's seems that with the patch we have lost some filters in the view:

• "Search: Fulltext search" is not there anymore
• The filter on the field that we have been able to add with this patch doesn't appear on the view either.

Anyway, in the database the information is stored properly.

Anyone can point me where should be the problem?

I have solved the issue that doesn't show the fields in the view with this patch.

But I'm getting a new error when I save the view.

The website encountered an unexpected error. Please try again later.

InvalidArgumentException: The configuration property display.default.display_options.filters.name_4.value.min doesn't exist. in Drupal\Core\Config\Schema\ArrayElement->get() (line 79 of core/lib/Drupal/Core/Config/Schema/ArrayElement.php).

Drupal\Core\Config\StorableConfigBase->castValue('display.default.display_options.filters.name_4.value.min', '') (Line: 216)
Drupal\Core\Config\StorableConfigBase->castValue('display.default.display_options.filters.name_4.value', Array) (Line: 216)
Drupal\Core\Config\StorableConfigBase->castValue('display.default.display_options.filters.name_4', Array) (Line: 216)
Drupal\Core\Config\StorableConfigBase->castValue('display.default.display_options.filters', Array) (Line: 216)
Drupal\Core\Config\StorableConfigBase->castValue('display.default.display_options', Array) (Line: 216)
Drupal\Core\Config\StorableConfigBase->castValue('display.default', Array) (Line: 216)
Drupal\Core\Config\StorableConfigBase->castValue('display', Array) (Line: 217)
Drupal\Core\Config\Config->save() (Line: 285)
Drupal\Core\Config\Entity\ConfigEntityStorage->doSave('search', Object) (Line: 397)
Drupal\Core\Entity\EntityStorageBase->save(Object) (Line: 264)
Drupal\Core\Config\Entity\ConfigEntityStorage->save(Object) (Line: 363)
Drupal\Core\Entity\Entity->save() (Line: 642)
Drupal\Core\Config\Entity\ConfigEntityBase->save() (Line: 1003)
Drupal\views_ui\ViewUI->save() (Line: 318)
Drupal\views_ui\ViewEditForm->save(Array, Object)
call_user_func_array(Array, Array) (Line: 116)
Drupal\Core\Form\FormSubmitter->executeSubmitHandlers(Array, Object) (Line: 56)
Drupal\Core\Form\FormSubmitter->doSubmitForm(Array, Object) (Line: 588)
Drupal\Core\Form\FormBuilder->processForm('view_edit_form', Array, Object) (Line: 319)
Drupal\Core\Form\FormBuilder->buildForm(Object, Object) (Line: 53)
Drupal\Core\Entity\EntityFormBuilder->getForm(Object, 'edit', Array) (Line: 232)
Drupal\views_ui\Controller\ViewsUIController->edit(Object, 'page_1')
call_user_func_array(Array, Array) (Line: 128)
Drupal\Core\EventSubscriber\EarlyRenderingControllerWrapperSubscriber->Drupal\Core\EventSubscriber\{closure}() (Line: 577)
Drupal\Core\Render\Renderer->executeInRenderContext(Object, Object) (Line: 129)
Drupal\Core\EventSubscriber\EarlyRenderingControllerWrapperSubscriber->wrapControllerExecutionInRenderContext(Array, Array) (Line: 102)
Drupal\Core\EventSubscriber\EarlyRenderingControllerWrapperSubscriber->Drupal\Core\EventSubscriber\{closure}()
call_user_func_array(Object, Array) (Line: 139)
Symfony\Component\HttpKernel\HttpKernel->handleRaw(Object, 1) (Line: 62)
Symfony\Component\HttpKernel\HttpKernel->handle(Object, 1, 1) (Line: 62)
Drupal\Core\StackMiddleware\Session->handle(Object, 1, 1) (Line: 53)
Drupal\Core\StackMiddleware\KernelPreHandle->handle(Object, 1, 1) (Line: 51)
Drupal\Core\StackMiddleware\ReverseProxyMiddleware->handle(Object, 1, 1) (Line: 55)
Drupal\Core\StackMiddleware\NegotiationMiddleware->handle(Object, 1, 1) (Line: 23)
Stack\StackedHttpKernel->handle(Object, 1, 1) (Line: 637)
Drupal\Core\DrupalKernel->handle(Object) (Line: 19)

I have seen that this error is similar to #2664286: Cannot use date filters on index but in this case it's not a date type.

If somebody knows where could be the problem I can try to solve it. Thanks in advance!

Are you sure that error is caused by this patch? It seems rather unconnected.
In any case, thanks for creating this issue and working on it, the solution already looks quite good.

+++ b/src/Form/IndexAddFieldsForm.php
@@ -368,7 +369,16 @@ class IndexAddFieldsForm extends EntityForm {
-        $nested_properties = $property->getPropertyDefinitions();
+        if ($property instanceof EntityDataDefinitionInterface) {
+          $bundles = \Drupal::entityManager()->getBundleInfo($property->getEntityTypeId());
+          foreach ($bundles as $bundle => $bundle_label) {
+            $bundle_properties = \Drupal::entityManager()->getFieldDefinitions($property->getEntityTypeId(), $bundle);
+            $nested_properties += $bundle_properties;

Contrary to the second change, in Field, it seems here you forgot to initialize $nested_properties for complex properties.

In any case, though, instead of duplicating code, we should maybe add this to the Utility class – either just for getting the child properties of the given properties, or already in a form that allows drilling down (if that makes sense and is possible to refactor/abstract.

Also, as always when breaking things that worked previously, it would be great if we could add a check somewhere in our testing suite that makes sure this won't happen again. Probably, just a few lines in IntegrationTest would suffice.

Hi drunken monkey,

I'm not sure that the error that I'm getting because of the patch, but I get that error with the fields that now I can add after applying the patch.

Anyway, I'm agree with you that can be another issue, so I'll do that refactor and I'll try to do that test.

In my project I'm using a custom controller and I'm querying search_api from there because of the error, so I'll try to do this during the weekend.

Thanks for you help!

I haven't made the test yet, but here it's the refactor to not duplicate the code. I have created the `getNestedProperties` method in the `Utility` class. I hope that could be Ok.

is it possible to take entity_reference_revisions new composite relationship into account so it is possible to index paragraphs in the future ?

The current patch works with paragraphs but i'm not sure it works after the migration to the new entity_reference_revisions composite relationship.

@ jjcarrion: While #6 still applies for me, #9 somehow doesn't. Did you maybe make some mistake while creating it?

@ yobottehg: I'm sorry, but I can't take all contrib modules into account. If your module doesn't work with the Search API out-of-the-box, you can always override the datasource, or provide your own new one. I'll only fix problems if you can show me we're using the Entity API incorrectly, or if a change would be generally beneficial.
Also, in what way is that related to this issue?

@yobottehg, we should still be fine as entity_reference_revisions DataType extends the EntityReference (which implements the DataReferenceInterface which search_api expects) class so "instanceof DataReferenceInterface" will still return TRUE on ERR field which should make the extraction work.

Didn't test it though.

@ drunken monkey sorry for crashing this issue, but it's related. And thanks @jeroen.b for clarifing.

I have reroll the patch to make it work with the actual version, I was touching some code standards that were breaking the patch.

Here is the new patch, but nothing about the testing part yet. If somebody can point me a similar test that I can start with I can try it as well.

Thanks!

I'd probably do it in IntegrationTest: just go to an appropriately nested path and see if one of the non-base fields there is present.
Problem is, though, I don't think there's any appropriate reference on nodes currently in there, so you'd have to add one – probably best just an entity reference to a different node, that way you don't have to add an additional module (I think).
Would be great, if you could work on that, thanks! (Also thanks for the re-roll!)

(Setting to "Needs review" for the test bot, still needs tests, as said.)

Good to see all existing tests pass, at least. Setting to "Needs work" for the tests.

Thanks everyone for the previous patches, we are using this with entity reference revision and we found out that the patch #14 fixes our problem. I added some tests here, not really sure if this is enough or if we should check something else.

That's a good start, but that's just seeing the field.

I think we also want to test actually adding that field, indexing content that has such a reference and checking that it is being indexed (I guess the easiest way to do that is to just search for the text inside the reference?)

Thanks for adding the test!
See the attached for a re-roll and a few minor fixes.

Berdir is right, though, we should also test indexing of the nested fields – and, since I just made a modification there, too, also that the dependencies for these fields are computed correctly.
Indexing will have to be tested in a different test, though, unless we add some way to inspect the indexed items with the test backend (which could be handy, though).

@drunken monkey thank you for the patch, I've been looking into it but could not realize why the tests are failing. Do the tests need an extra fix?

Let's try it like this?

This looks like a good solution, it has test coverage as well. Woo!

As noted in #20 and #21, it didn't have quite enough test coverage yet.
Should be fixed with the attached patch, though. If the test bot and one of you agree, I'd say this is RTBC.

This looks good to me! Thank you so much @drunken monkey!

Excellent, good to hear!
Committed.
Thanks again, everyone!