Add Views EntityReference filter to support better UX for exposed filters

(fixing comment number in issue summary)

Sidenote: I recommend editing #49 to get those list bullets back in, for readability.

I found another issue when trying to use the new entity_reference filter on a user reference field.

Steps to reproduce

1. Enable the entity_reference filter for the node uid field

/**
 * Implements hook_views_data_alter().
 */
function my_module_views_data_alter(array &$data): void
{
    $data['node_field_data']['uid']['filter']['id'] = 'entity_reference';
}

2. Add the Author filter (category Node) to a node view, changing Selection type to Select list.
3. Open the view. The dropdown has no options.

The reason this happens is because target_bundles is set to an empty array. The reason target_bundles is set to an empty array is because validateExtraOptionsForm failed, namely the following line:

$subform_state->setValue([
  'settings',
], $subform_options);

And that fails because SubformState::getValues fails: $this->getParents('#parents') returns ['reference_default:user'] and the array returned by parent::getValues() does not have a reference_default:user key, it does have an options key. That's as far as I got while debugging.

@DieterHolvoet: I just happened to bump up against a similar situation. In my case it's a custom entity type that has no bundles.

I think the issue is actually in DefaultSelection, which I do not think is being touched by this patch. I suspect that only users of this patch would be trying to use DefaultSelection with a bundle-less entity, so maybe that is why it hasn't been an issue before? I'm not entirely sure.

In DefaultSelection::buildEntityQuery, there is this snippet:

    // If 'target_bundles' is NULL, all bundles are referenceable, no further
    // conditions are needed.
    if (is_array($configuration['target_bundles'])) {
      // If 'target_bundles' is an empty array, no bundle is referenceable,
      // force the query to never return anything and bail out early.
      if ($configuration['target_bundles'] === []) {
        $query->condition($entity_type->getKey('id'), NULL, '=');
        return $query;
      }
      else {
        $query->condition($entity_type->getKey('bundle'), $configuration['target_bundles'], 'IN');
      }
    }

And in DefaultSelection::buildConfigurationForm, there is this condition:

    if ($entity_type->hasKey('bundle')) {
      // ..code removed for comment..
    }
    else {
      $form['target_bundles'] = [
        '#type' => 'value',
        '#value' => [],
      ];
    }

So I think that '#value' => []needs to become '#value' => null, but I am not entirely sure that is the best solution or what other impact that change would have elsewhere. Alternatively, EntityReference::buildExtraOptionsForm could be modified to set $sub_handler_settings['target_bundles'] to null if the entity type does have have bundles.

As a workaround, I ended up writing a custom EntityReferenceSelection plugin that extends DefaultSelection and changes target_bundles to null, when the entity has no bundles, inside ::buildEntityQuery. A little messy, but gets the job done (for now).

------------------------------------------

Update: I think I found the defect. The changing of target_bundles to be null, instead of an empty array, happens in DefaultSelection::validateConfigurationForm. This method is called by EntityReference::validateExtraOptionsForm, but I am not sure the structure of $form_state is compatible anymore.

So possible solution is to check for the target_bundles empty array and set to NULL inside of EntityReference::validateExtraOptionsForm?

------------------------------------------

Update 2023-06-23: I'm reporting back that the merge request 4053 is now solving this issue.

Every instance of self::WIDGET_SELECT_LIMIT and self::ALL_VALUE should be static::.

The use of self prevents any extending classes from overriding these uses, for example via replacing the filter class via hook_views_plugins_filter_alter().

The use of static accommodates this use.

Per my update in #55, I think something like this is needed in EntityReference::validateExtraOptionsForm. This will be to handle entities with no bundles and possibly for selecting no bundles on the subform.

    // If no checkboxes were checked for 'target_bundles', store NULL ("all
    // bundles are referenceable") rather than empty array ("no bundle is
    // referenceable" - typically happens when all referenceable bundles have
    // been deleted or when an entity type has no bundles).
    if ($subform_options['target_bundles'] === []) {
      $subform_options['target_bundles'] = NULL;
    }

    // Store the sub handler options in sub_handler_settings.
    $form_state->setValue(['options', 'sub_handler_settings'], $subform_options);

-----------
Update 2023-06-23: I'm reporting back that the merge request 4053 is now solving this issue.

@smustgrave please do ...
and thanks a lot

+ i just noticed something ... for drupal.org
i tried to select new people for credits (including you) ... but it didn''t update the commit message ... hmm

@smustgrave
it seems there is an issue with with 11.x ... and 11.0.x it doesn't seem "a real one" ...

so in ... a hurry ... we lost the track ... if i may say so .. :(

I had this issue as well somewhere else because the issue fork got created before the branch existed and is thus not in sync with all other branches. Let me see if I can fix it.

There we go. If anyone is wondering how I usually fix this here is the process.

1. Start gitpod with the "old" 10.1.x branch
2. Checkout 10.1.x
3. Git pull
4. Git fetch origin
5. Git fetch --all
6. Git checkout 11.x
7. Git checkout -b 3347343-11-x
8. -- do the stuff like patch applying, making changes, fixing conflicts --
9. Git commit
10. git push --set-upstream drupal-3347343 HEAD <<< this command changes with each issue fork, it's the last command from the "show commands" section.

Also @vasike

i just noticed something ... for drupal.org
i tried to select new people for credits (including you) ... but it didn''t update the commit message ... hmm

You don't have to include that in your commits. All commits will be squashed anyway when a maintainer commits this and sets the final commit message. Only maintainers of the module (or core in this case) can change the checkboxes.

@BramDriesen very thank you
p.s. this is ... anyway ... witchery ... imho

No problem, let's all be wizards 🧙‍♂️

Updates on MR:
- rebased - using 11.x
- Fix for entities with no bundle - DefaultSelection NULL value - as User
- Complete - Workaround for https://www.drupal.org/project/drupal/issues/2651418
to support sub-items (for Entities with some fieldset/tree options) - as User
- Use static for const, instead of self ... as suggested.
- Fix tests - remove duplicate (early check) + fix the check for Filter options.

Todo:
- Fix https://www.drupal.org/project/drupal/issues/2651418 - it will cleanup code in the MR.
- Entity with no bundle test - if really needed?!

Unrelated. Will hide that patch, not sure if that stops it from being retested or not.

@smustgrave

One part that was odd to me, won't hold it up, but when adding the filter it let me select the content types to filter by vs using the field settings.

The "Reference selector" it's not a "field setting" ... but a "field instance setting"
Which means a field could be used with different "Reference type" ... on different bundles.
And i think it was discussed and "decided" in the original issue.

But did notice a bug
I had 2 Articles, 1 with an entity reference value and 1 without
When I use the filter it shows duplicates of the 1 article with a reference. Imagine it would show any number of duplicates based on number of articles.

I can't replicate this. Are there other "elements" in your install? Maybe multilingual or something?

@smustgrave checked again on duplicates

the only scenario is when we have multiple valued reference field and also multiple values selected for the filter.
Which is "normal" ... and with Aggregation we could get rid of duplicates.

So i can't see / replicate the bug.

Let's try NR ... so maybe there will find out is close to RTBC or ... not.

small update - FilterEntityReferenceTest for kernel file and class name ... as it's about the filter ... not field.

@smustgrave

Tagging for subsystem maintainer as it seems this feature will require aggregation now.

This is usual ... the "same results" you'll get also for a List field with multiple values and filter options selected ...
So it's not about this filter in particular ... but filter multiple values for multiple valued field ...
just saying

@smustgrave do you mean no filter at all?
No select input?

Or no select options.

This sound critical ... but could you, please, detail ... maybe screenshots for some steps?
Thanks

In #2429699 comment #449, @Berdir answered the following when someone asked if the patch supports fields which reference configuration entities:

No, that's impossible, content and config entities can't be joined in a relationship, config entities are not stored in a way that allows an SQL JOIN to work.

That doesn't seem to be the case though. If I replace if ($target_entity_type instanceof ContentEntityTypeInterface) { with if ($target_entity_type instanceof EntityTypeInterface) { in core_field_views_data, it seems to work. My use case: I have an entity reference field on a node type, referencing webforms (a config entity type). After applying the patch and the change I just mentioned, I can add an entity reference filter listing all webforms, without issues.

Should we change this so it also works with config entities? I don't see why not.

The Needs Review Queue Bot tested this issue. It fails the Drupal core commit checks. Therefore, this issue status is now "Needs work".

This does not mean that the patch needs to be re-rolled or the MR rebased. Read the Issue Summary, the issue tags and the latest discussion here to determine what needs to be done.

Consult the Drupal Contributor Guide to find step-by-step guides for working with issues.

This problem also exists in Drupal 9.
I created a patch against 9.5.x based on 2 files from Drupal 11 commits in gitlab

1 new file added: core/modules/views/src/Plugin/views/filter/EntityReference.php
and 1 changed: core/lib/Drupal/Core/Entity/Plugin/EntityReferenceSelection/DefaultSelection.php

This patch allowed me to get working exposed filter out of entity reference field that references content (nodes).
The probleme does not exist when the field references taxonomy terms.

The views filter should be of type "field_name as a Reference filter"

Updates on MR:
- Fix the CSpell Failures
- And update to Do not limit filters available to Content Entities, use EntityTypeInterface ... as mentioned on #86
But separate filter from relationships condition, as indeed they are not "compatible".
In this way also we have aligned the filter available and Views Filter (getReferencedEntityType(): EntityTypeInterface)

let's try some review.

The to-do list in the summary could use an update as well. 🙂

attached a static patch for builds. This represents the MR 4053 at commit 1bc7bda7.

If we're adding support for config entities here, then we need the equivalent test coverage to confirm it works, so marking needs work and 'needs tests' for that. Another option would be to move that change to a follow-up, although the test coverage shouldn't be too bad to add.

Haven't done an in-depth review of the rest of the patch yet, just noticed this but added a couple of additional points from a quick look.

+++ b/core/lib/Drupal/Core/Entity/Plugin/EntityReferenceSelection/DefaultSelection.php
@@ -200,7 +200,7 @@ public function buildConfigurationForm(array $form, FormStateInterface $form_sta
-        '#value' => [],
+        '#value' => NULL,

This change is not correct, a NULL value for the target_bundles setting means "all bundles are referenceable", while an empty array (the current value in HEAD) means "no bundles are referenceable", so this is a behavior change in the default selection plugin...

@amateescu I agree the change seems odd since it will affect all entity selection handlers.
Here's an audit for the change to "core/lib/Drupal/Core/Entity/Plugin/EntityReferenceSelection/DefaultSelection.php":

• This commit introduced the change in the original MR 3629 before it was converted over to the current MR 4053- commit.
• It appears there was some discussion in #55 that suggested setting it to NULL to fix an issue with a custom entity type. Then it was committed in #73.
• This changed was not in the patch/MR on the original issue #2429699: Add Views EntityReference filter to be available for all entity reference fields.

about the change

+++ b/core/lib/Drupal/Core/Entity/Plugin/EntityReferenceSelection/DefaultSelection.php
@@ -200,7 +200,7 @@ public function buildConfigurationForm(array $form, FormStateInterface $form_sta
-        '#value' => [],
+        '#value' => NULL,

This is needed in order to get the "bundless" entities - to work.
Without it. there are no "options" for the Referenced entities.

So the question, do we still need to keep this and make it work for entities without bundles.
or this change "won't" harm.

Updated the MR with some tests for Filter of Config Entities References.

The. node type config was used for Entity Reference Testing Field.

Updates:

- Updated summary
- Change record draft available
- Remove tags: Needs issue summary update, Needs tests
- Add related issue, as the MR code contains this issue reference https://www.drupal.org/project/drupal/issues/2651418

The Needs Review Queue Bot tested this issue. It fails the Drupal core commit checks. Therefore, this issue status is now "Needs work".

This does not mean that the patch needs to be re-rolled or the MR rebased. Read the Issue Summary, the issue tags and the latest discussion here to determine what needs to be done.

Consult the Drupal Contributor Guide to find step-by-step guides for working with issues.

I'm not sure if I've missed something obvious, but this change seems to cause views to pull in the refercenced entity with an inner join. This does not happen with the default NumericFilter where it becomes a left join. I had a cursory look at the code but couldn't find out where/why this happens, but perhaps someone more versed in views can have a look.

Nevermind, I am creating a module that can copy the input value from another field to allow me to check two referenced fields with one exposed input, and am probably doing it wrong.

There was an attempt to cleanup the code about required "fields" for the selection handler options
https://git.drupalcode.org/project/drupal/-/merge_requests/4053/diffs?co...
And it "works" in the UI without issues.

But it seems the tests fails.
Something it's broken in the Views/Modal forms validations - or at least for required fields/elements.
And the extra code (setRequiredViaStatesOnChildren) helps for tests to pass.

Unfortunately ... there is no "FunctionalJavascript" tests on Filters with extra options (buildExtraOptionsForm)
or at least i couldn't see such tests.
That could help us here ... i'm also not aware on the other core issues related with those things.

So for now i just reverted the change to previous "state".

I still think the code could be cleaned up for both Filter plugin and its test.

Questions:
TaxonomyIndexTid / TaxonomyIndexTidDepth plugin filters do not have "FunctionalJavascript" tests, only "Functional" tests
Could we consider the same here. So maybe we don't need the "extra code" for this new filter?

@sthomen do you mean this change mentioned. could you please check again? as the change was reverted.
thanks

@vasike My apologies, I was just being dumb. Please ignore my comment.

There is a second attempt to clean up the MR code on both Views Filter plugin and its tests.

For Views Filter plugin the cleanup also tries to address/solve the 2 open threads
1. A todo about conditional required - removed the code, i think this "todo" will have small chances to be chased and solved soon
And, for now, it's not used (needed)
Also renamed the method for this setRequiredViaStatesOnChildren -> removeRequiredOfSubformChildrens
2. A "todo" about the extra options form validation. It seems the original issue has a related issue linked ... which it was missed when this new one was created
https://www.drupal.org/project/drupal/issues/3163740
So i cleaned up the code there and added the related issue url.
3. Re-arrange some code - to be easier to maintain/read

For tests code
I tried to cleanup - use similar code with existing code on filters ... with the same goal - - to be easier to maintain/read (toghteter with other related tests).

i hope we got closer to an end here

forgot to link the related issue

I needed this on a client site, and noticed that the entity reference filter doesn't show up for base fields.

This is easily solved with the following additional change:

diff --git a/core/modules/views/src/EntityViewsData.php b/core/modules/views/src/EntityViewsData.php
index eb964521af..19e898f12c 100644
--- a/core/modules/views/src/EntityViewsData.php
+++ b/core/modules/views/src/EntityViewsData.php
@@ -645,7 +645,7 @@ protected function processViewsDataForEntityReference($table, FieldDefinitionInt
         ];
         $views_field['field']['id'] = 'field';
         $views_field['argument']['id'] = 'numeric';
-        $views_field['filter']['id'] = 'numeric';
+        $views_field['filter']['id'] = 'entity_reference';
         $views_field['sort']['id'] = 'standard';
       }
       else {

However, changing the filter type (as opposed to adding an extra field in the views data), is not compatible with the current approach taken here.

I am wondering why the approach was changed to add an extra field, it adds a lot of noise to the UI when the 99.99% use case will be to use this filter anyway. I don't see how this can be applied to base fields without either changing the approach, or refactoring EntityViewsData to allow field types to produce multiple fields, or hard coding this reference type in there somewhere.

Also added a couple of review points to the MR.

Apart from those issues, this looks close and it would be great to close it before this continuation issue is as long as the original.

I am wondering why the approach was changed to add an extra field, it adds a lot of noise to the UI when the 99.99% use case will be to use this filter anyway. I don't see how this can be applied to base fields without either changing the approach, or refactoring EntityViewsData to allow field types to produce multiple fields, or hard coding this reference type in there somewhere.

That happened way back here after chatting with lendude at Drupal Dev Days.
https://www.drupal.org/project/drupal/issues/2429699#comment-14472189

If I remember correctly, without this MR you can set up the reference field to eg find all node IDs >= 1000, but switching to an entity reference you lose the more math related options like that. While it seems like edge case, it is possible users are using the filter as is to do such things. So a separate filter seemed the only way to not break existing sites.

Some thoughts about the change https://git.drupalcode.org/project/drupal/-/merge_requests/4053/diffs?co...

Why, imho, NULL value will not harm at all (there).

1. It's a form value element for entities that doesn't have a bundle key

    if ($entity_type->hasKey('bundle')) {
       ....
    }
    else {
      $form['target_bundles'] = [
        '#type' => 'value',
        '#value' => NULL,
      ];
    }

2. defaultConfiguration has NULL value

3. The field settings will save actually NULL value for entities without bundle
@see validateConfigurationForm
Result

uuid: 0918c28c-2a5d-49b0-8c33-ee7855dd23a9
langcode: en
status: true
dependencies:
  config:
    - field.storage.node.field_type
    - node.type.article
id: node.article.field_type
field_name: field_type
entity_type: node
bundle: article
label: Type
description: ''
required: false
translatable: false
default_value: {  }
default_value_callback: ''
settings:
  handler: 'default:node_type'
  handler_settings:
    target_bundles: null
    auto_create: false
field_type: entity_reference

4. And of course the "docs" about NULL vs [] "results"

      // For the 'target_bundles' setting, a NULL value is equivalent to "allow
      // entities from any bundle to be referenced" and an empty array value is
      // equivalent to "no entities from any bundle can be referenced".

So my conclusions are:
1. The Change to NULL should not break anything
2. DefaultSelection could be improved for "bundless" entities
3. Subforms and validations not working "great" on Views Filters.

@catch what do think?
thanks

MR updated ... trying new approach to make it work for entities with no bundle.

As part of the conclusions from #112, about DefaultSelection
I noticed that the configuration form validation "imagines" that we have a field settings form.
Which means the validations there won't work in another forms.

So i moved the validations from form validation to elements validations - for portability.
And it should work.

p.s. i still "believe" in things i shared on #112

Unfortunately ... the MR started failing ... PHPStan ... and the last one looks odd ....

If there's anyone that could help with this error?
thank you

Fixed the PHPStan errors.
However now we have the "CSpell" CI issue ...
https://www.drupal.org/project/drupal/issues/3401988

Re #111, could this be handled as a backwards compatibility break in a major release? Alternatively, I wonder about creating a small contrib module to switch the definition..

@DamienMcKenna with all my respect, I would say that, this is "out of box" issue.

2 things
- Views is in Core
- EntityReference is in core

What's not in core ... is Views and EntityReference "real integration"
This is just a small part to involve more people to help with Views ... is far from "core" ... yet ... imho

But thanks for your thoughts.

@vasike: Apologies that I wasn't clear, I was talking about to the suggestion in #110 to simply change core's field specification to use "entity_reference" instead of "numeric"; could this change be introduced as a breaking change, maybe with documentation on how to use the field as a number?

Re #111 / #116 perhaps best is to:

1. get this in using current approach of additional filter

Then in follow-up (because all the below is a lot of work in itself and big scope increase):

1. deprecate use of original filter math options (ideally specifically the ones nk longer available
2. add change record
3. add update hook to convert the additional filter from this issue back to original name

If that's at all possible / interesting perhaps that discussion can be had in a postponed issue to avoid this issue growing too big again

This issue was hit by the issue mentioned in #119, but also the fact that the branch 11.x wasn't even present in the fork (I think this is another core issue).

I brought 11.x to the fork and the MR should now run. I have no more context on the issue so not sure if it should be back to "Needs review" or still "Needs work".

@smustgrave I mean that if you expose an entity reference filter in HEAD, then you get an empty text field with no indication of what to put in it, which to me looks broken and I doubt people are using it much as-is. Given that, I think we should consider applying the new behaviour to the existing filter, not adding a separate one alongside. This would also help with applying it to base fields, which the MR currently does not.

There are ways to use it and I've done it before. For example in combination with grouped filters, which obviously doesn't scale, but is a decent workaround when you have for example a config entity reference with only a handful options.

Re #125 you could be using the filter without exposing it though, eg all node IDs >= 1000. Ie, not useful for a visitor but possibly useful for a site builder?

As per my response in #111 that's why chats with @lendude suggested that would be the most realistic way to get this in core.

I would also prefer to not have the extra filters in an ideal world and replace instead if you think we can get away with just a change record but I am concerned this then never gets in as many of us have spent chunks of time on this over the years and the goal-posts changing makes it tough of course.

Would you be the one signing off on this eventually or should we seek review from a different core committer as well?

Asking first because, if we go through route, it's probably a fair amount of work to change the MR but also then possibly further extend the MR to need update hooks (and upgrade tests?). So I think we should probably agree scope and ensure agreed scope has a realistic path to actually making it in before actioning a change of approach... Or agree to keep the current scope

@scott_euser I'll try to get a second and if possible third or more opinion from other core committers so this doesn't get turned around again.

@Berdir same question as #127 - is that exposed to site visitors, or exposed at all?

I open a "thread" on slack
https://drupal.slack.com/archives/C6SQM2J94/p1706633005193429

Also added to summary

And I'm wondering if it's not more efficient for discussions than to "fill-up" issue comments

p.s. maybe it could be a way for drupal.org to have a slack url about discussion on some issues.

just thinking aloud

sorry for the noise.

p.s.2 and i'm trying to silent (here)

Updated issue summary to reflect need to add support for base fields into this as per message from @catch in Slack.

Also moving back to Needs Work since there are other items in the list that are not yet completed as well.

Support for base fields added, but needs tests (updated issue summary).

A good example is 'uid' field in default Files view, so an easy way to try this locally:

• Before update: admin/structure/views/view/files only User ID available
• After update: admin/structure/views/view/files only User ID and User ID as a reference available

Problem at the moment is HandlerAllTest::testHandlers() fails on Comment 'Parent ID' base field, its getting allowed to use the new 'entity_reference' filter handler, but then something inside the new filter handler does not like that as a target. I ran out of time at the moment in case anyone wants to pick this up, otherwise I will try to pick it up myself again later.

I suspect sorting that issue will sort out the tests (unless there are further issues with supporting base field).

Okay tests passing now + new test added for base field reference.

So just comments from #49 left to address.

I have this merge request running in a patch on my Drupal 10.1 site. It was running fine until recent updates made to it. Now Composer wont patch it into core using "cweagans/composer-patches". Am i the only one with this issue? What should I do to get this working again the way it was? "https://git.drupalcode.org/project/drupal/-/merge_requests/4053.patch" I'm probably not the only one.

Any thoughts on how to proceed would be appreciated as its currently blocking my monthly maintenance. Thanks.

Hi @Deasly.

The merge request is meant to target Drupal 11 and it needs to continue to do so in order to eventually get in.

In the future, best to be downloading the patch file into your project and committing it, so you reference the local file, see here for details. It also talks about how you can do a git diff to make your own patch. The challenge here is you need to find the commit id of a version in the merge request that you are happy with then the commit ID eg of latest 11.x. Then its `git diff new-commit-id old-commit-id > patch-file-name-here.patch.

That only works if there is an older version that affects files that do not have a merge conflict with 10.2.x (the version I am guessing you are on). And of course you miss out on the additional stability/bug-fixing of the latest commits in the Merge Request.

Hope that helps!

Sorry for the noise here, not sure an easier way to help others out that want to use this in 10.2.x as I realise its a challenge to resolve conflicts as soon as the 11.x MR starts to become incompatible with 10.2.x like it is.

I created a branch 3347343-10-2-x--do-not-merge which people should be able to use the diff from to create a local patch BUT please bear in mind the code is just cherry-picked from the Drupal 11 Merge Request code and its therefore very possible that code is referenced that only exists in Drupal 11 and does not exist in Drupal 10 which can lead to errors, so do use with caution.

Regarding #49 I have updated the issue summary with outstanding items. Details below:

calculateDependencies is untested and should be since this is different than the Term plugin

Added tests mimicking the coverage that TaxonomyIndexTidFilterTest::testConfigDependency() provides.

The 'sort by' field is empty on initial adding of the filter (the field widget does something ajaxy when selecting bundles, the views widget does not, might be it?)

I could not spot a 'sort by' when using a field widget on an entity reference field. Is it possible to provide further steps?

Grouped autocomplete filters don't work/save (but don't work for taxonomy ref filters either), please find the related issue or open it (should be one I think)

Here is the related issue I think #2576927: Grouped exposed taxonomy filter fails validation for autocomplete widget.

Description "as a Reference filter" feels a bit vague, did we get UX feedback on this?

Perhaps this should be dealt with in the deprecation dance when the old filter gets removed. Added as a follow-up task.

Should we add this new filter for taxonomy terms too, or should we skip those since it is unclear what the difference is now when have to chose between the two different filters

We have this as a follow-up now #3339738: Convert TaxonomyIndexTid to use new EntityReference filter.

I can create a circular reference by creating an Entity reference View that uses itself as the filter, not sure what goes wrong (mostly getting an empty filter after saving), but that sounds iffy
Maybe related : Managed to click my way into "Warning: Undefined array key "reference_views" in Drupal\views\Plugin\views\filter\EntityReference->submitExtraOptionsForm() (line 440 of /app/drupal/core/modules/views/src/Plugin/views/filter/EntityReference.php)"

Needs work - added to issue summary as a task

Node titles in the select element are double escaped

Needs work (I can see this works in TaxonomyIndexTid, so its not a wider problem) - added to issue summary as a task

Exposed filters on the chosen entity reference view are also shown on the view

Needs work - added to issue summary as a task

New commit to remove a "@todo" from code ... about ajax property on Entity Reference Selection plugin.
From related issue https://www.drupal.org/project/drupal/issues/2651418

please check the latest comments there.

I'm a little confused by this thread (it's quite long and convoluted), but is there a patch/MR that works against D10? I've got a client yelping about this. :D

Under the branches listed at the top, click to show hidden branches and see '3347343-10-2-x--do-not-merge' - is has a port of the 11x merge request for 10x sites.

The Needs Review Queue Bot tested this issue. It no longer applies to Drupal core. Therefore, this issue status is now "Needs work".

This does not mean that the patch necessarily needs to be re-rolled or the MR rebased. Read the Issue Summary, the issue tags and the latest discussion here to determine what needs to be done.

Consult the Drupal Contributor Guide to find step-by-step guides for working with issues.

Solved conflicts and also new CS issues
However it seems there are other things going on for CI
As the new Validatable config job ... which, of course it fails

Probably this should be covered by a child issue
https://www.drupal.org/project/drupal/issues/2869792
... which seems ... missing
"Not ideal ... to be polite"

I put the issue for Review ... just to gain some attention
As the feeling is that it could become stalled ... as original ... for other years.

p.s. also linked those issues ... maybe there we could have more awareness

Not an input to the issue, just a patch for 10.2.x

The Needs Review Queue Bot tested this issue. It no longer applies to Drupal core. Therefore, this issue status is now "Needs work".

This does not mean that the patch necessarily needs to be re-rolled or the MR rebased. Read the Issue Summary, the issue tags and the latest discussion here to determine what needs to be done.

Consult the Drupal Contributor Guide to find step-by-step guides for working with issues.

MR green again

Also I hid the latest patch ... needs-review-queue-bot doesn't like it https://www.drupal.org/files/issues/2024-03-14/3347343-nr-bot.txt

These are the remaining tasks from the issue summary (other than getting framework manager approval):

☐ sort by empty on initial load (needs more info), see comment #49

☐ prevent circular reference: validate no selection of an entity reference that uses itself as the filter, see comment #49

☐ node titles in the select options when configuring the filter are double escaped (e.g. " shows as "), see comment #49

☐ exposed filters on the chosen entity reference view are also shown on the view, see comment #49

To get this over the line, could we consider all of these back-end minor edge cases? None of them stop the site builder from achieving the primary task and none of them affect the front-end functionality for the visitor. So perhaps we could consider them as follow-ups? They would be easier to achieve in isolation later.

For folks that use patches, the patch from #156 doesn't apply to 10.2.5 so here is one without the phpstan-baseline changes that applies to both.

The Needs Review Queue Bot tested this issue. It fails the Drupal core commit checks. Therefore, this issue status is now "Needs work".

This does not mean that the patch necessarily needs to be re-rolled or the MR rebased. Read the Issue Summary, the issue tags and the latest discussion here to determine what needs to be done.

Consult the Drupal Contributor Guide to find step-by-step guides for working with issues.

Attaching a static patch of MR 4053 for builds to use. This patch applies to 11.x and 10.2.5.

I had a custom selection plugin that extended Drupal\views\Plugin\EntityReferenceSelection\ViewsSelection. Since the plugin ID was not "views", the detection to remove the element validation (unset($sub_handler_settings['view']['#element_validate']); would not run, resulting in silent form error and the filter config values not saving.

I changed views handler detection to class based instead of plugin id to support any plugin that extends the ViewsSelection class.

Please stop uploading patches to this issue. It is causing problems with the NR bot and making this even more tedious to try and get over the line.

I completely agree with #164 - let's get this in and polish it further in follow ups if needed before we get to another 500 comment issue.

The Needs Review Queue Bot tested this issue. It no longer applies to Drupal core. Therefore, this issue status is now "Needs work".

This does not mean that the patch necessarily needs to be re-rolled or the MR rebased. Read the Issue Summary, the issue tags and the latest discussion here to determine what needs to be done.

Consult the Drupal Contributor Guide to find step-by-step guides for working with issues.

The Needs Review Queue Bot tested this issue. It fails the Drupal core commit checks. Therefore, this issue status is now "Needs work".

This does not mean that the patch necessarily needs to be re-rolled or the MR rebased. Read the Issue Summary, the issue tags and the latest discussion here to determine what needs to be done.

Consult the Drupal Contributor Guide to find step-by-step guides for working with issues.

@scotteuser - Hey I tried to update the 10.2 branch for 10.3 yesterday, but I couldn't figure out a good way to do it. First I tried to rebase, but gave up after a bunch of conflict resolutions. Then I tried merging, most of the important changes were merged no problem, and I tried to check for other relevant changes but it's a big list and hard to verify. When I completed the merge, it was about 80% right with some extra stuff.

Do you have any notes on how to do these kinds of branch updates in a sane way?

I definitely had a bit of hair-pulling as well, what I ultimately did was take the merge request diff as a patch and ran the apply patch via phpstorm which applied most files. Then I manually redid the views.views.inc as it could not manage that. So yeah, no sane way I could find yet either :)

FILE: ...pal-3347343/core/modules/views/src/Plugin/views/filter/EntityReference.php
--------------------------------------------------------------------------------
FOUND 1 ERROR AFFECTING 1 LINE
--------------------------------------------------------------------------------
 77 | ERROR | [x] Parameter $options has null default value, but is not marked
    |       |     as nullable.
    |       |     (SlevomatCodingStandard.TypeHints.NullableTypeForNullDefaultValue.NullabilityTypeMissing)

I would have thought that https://git.drupalcode.org/project/drupal/-/merge_requests/4053/diffs?co... would fix this. Maybe I did the wrong thing, would appreciate if anyone has any tips. It seems there is a giant inheritdoc chain and this MR is just surfacing a deeper issue elsewhere.

It would be lovely to get a patch or MR that works with 10.3.0

@scott_euser can you create a patch based on what you did?

For 10.3, use 3347343-10-3-x--do-not-merge Copy link from plain diff ( You will get a patch link here -> https://git.drupalcode.org/project/drupal/-/merge_requests/8428.diff )

I patched it through composer and it patched cleanly.

@abx don't use MR .diff URLs, that is opening your site up to security vulnerabilities since anyone can push to that branch (and then subsequently make changes to the module when your site installs the new patch).

Instead you should download and store the diff locally.

Thank you acbramley. I will do that.

@acbramley that's very good to know, thanks! Should the community as a whole not go back to providing patch files on d.o in that case (at least until the issue is addressed)? It seems silly to make everyone go download the same file to me.

Looks like MR 8428 applies cleanly to 10.3.0, so thanks also to @abx.

@webdrips no, because it confuses the issues and makes things much harder to manage for reviewers and commiters. This issue is already extremely long in the tooth (almost at 200 comments and we've already had to recreate the issue because the last one had too many comments to load). Uploading patches will only cause more issues.

Back to NR. We should probably squash all commits on the branch back to a single commit if this goes green too.

Thanks for sorting out my poor merge! Apologies for that!

Yes here is the documentation for the merge request diff -> patch process which goes into more detail on the how & why.

A) Resolving "exposed filters on the chosen entity reference view are also shown on the view, see comment":

1. I created an entity reference view of articles that comes with default install with an exposed filter of tags
2. I created a related articles entity reference field on the page content type
3. I created a view with exposed filter of a 'related articles' entity reference field using this MR's new filter and using the entity reference view instead of the default for the handler

Confirmed: No child exposed filters from the entity reference view are shown any more.

B) I moved the circular reference comment to a follow-up as per earlier discussions (ie, the ability to select an entity reference that uses itself as the filter).

Also resolving "node titles in the select options when configuring the filter are double escaped (e.g. " shows as "), see comment #49"

I added some special characters to the titles of my reference-able content including the " as noted and all appear fine within the exposed filter configuration.

So that leaves only "get framework manager approval" as the final issue to sort. If others agree, it would be good if someone can do a final check and RTBC this that was not so involved in the code (since its probably not appropriate for those of us that were involved in the code to RTBC it).

Spoke to @catch in slack yesterday and going to pull on the thread of #110

If we swap instead of duplicating, and don't provide a config upgrade path, what I think would happen is that existing views would continue to use the numeric plugin (because the plugin ID is stored in config), and then when they're saved again, it would update to the entity reference plugin instead.
And if this is the case it might be fine.

This is to avoid having a new field.

If we swap instead of duplicating, and don't provide a config upgrade path, what I think would happen is that existing views would continue to use the numeric plugin

Unfortunately this is not the case.

After a lot of experimenting, I finally found a base field to test this properly on - you need a base field that's a content entity reference that's not the author (because that already uses another plugin). What I used was the Comment Parent ID.

Test steps:
1. Install standard profile
2. Edit the content view and add a relationship to the Comments field on the node
3. Add an exposed filter on Comment: Parent CID using the above relationship. This will be a numeric filter
4. Add some comments to a node and test that the filter works
5. Edit EntityViewsData::processViewsDataForEntityReference and change $views_field['filter']['id'] = 'numeric'; to $views_field['filter']['id'] = 'entity_reference';
6. Clear cache
7. Test the filter works again.

You're presented with the following error:

Very surprised that the plugin is evaluated at runtime but so be it.

This is now a tricky situation, because I agree (after doing some more testing) that the current solution is not something we should commit. You currently get duplicate filters for every single entity reference filter. This includes fields like Authored By, which already have special filters (e.g for Authored by, it uses the user_name filter plugin)

But swapping out numeric filters may cause other issues for existing sites.

Not really sure where to go from here, so I think this needs to be Postponed until we come up with a viable solution.

Maybe we'll never get an upgrade path though because there are comparison options in numeric that just are not available in entity reference. This goes back to the slack thread https://drupal.slack.com/archives/C6SQM2J94/p1706799363236089?thread_ts=... and whole idea of avoiding a BC. Can we descope base fields so we continue to avoid BC and set they as a follow up issue? Ie, get back to a situation where it is truly opt-in for existing sites. What do you think? We could move back to slack to chat fundamental approach again.

I imagine most people are not using this for Comment Parent ID, and perhaps contrib can support any breaking change overrides if someone does want entity reference for cases like that, but as noted in #206 it is hard to find base fields that do not have an alternative to 'numeric' filter already provided by their respective modules in core.

So for now I reverted the support for base fields and will raise it back in the Slack thread where it was originally suggested that we should support the base fields. @catch's request for base fields was an 'ideally', but maybe given the potential problems it causes we can move that to a follow-up.

Chatted with @catch and @acbramley in slack, I get it now: we can avoid an upgrade path if the View would respect the configuration. So existing configured filters using numeric would stay numeric, and new filters added would be configured as entity_reference therefore not needing an upgrade path. So in theory if we can get Views to respect the configuration (separate issue) then we can have an easier time here. If that does not seem doable, then we continue down the route of the alternative filter, leaving both options available.

One thing that does need work though is to try to reduce extra alternatives where another module in core already provides an alternative to the numeric filter. I started on that but needs work.

1. Updated issue summary to move base fields to a follow-up to simplify the scope here as discussed with @acbramley and @catch
2. Create follow-up issue for that #3458102: Add Views EntityReference filter to be available for all entity reference base fields
3. Add pre-issue we ideally sort first to allow us to replace the existing numeric plugin in place for all non-base entity reference fields #3458099: Views handler loading should respect configuration

Postponing for the pre-issue as per issue summary

Reset the branch back to before the experimentation for removing dupes.

Hello all,

I couldn't get the 10.3.x branch to apply, so I created a separate patch that applied in my local environment just now. This is just a band aid though, and I'm happy to investigate why the 10.3.x branch won't apply if needed.

As discussed in Slack here as suggested by @catch, the change to views data & views data alter has been reverted and the EntityReference filter is made available without being used, allowing it to be used in place, rather than with the 'as a Reference' duplicate filter.

The issue summary has been updated with a code snippet of how to use and a contrib module automatically opting into this will be provided assuming the direction is accepted and ready to merge. That contrib module could also be used to provide any upgrade path away from the 'as a Reference' in case any sites are making use of the patch.

Tests have been updated to match without any functional change to the tests other then to stop referring to the _reference/'as a Reference' aspect of the filter.

The issue summary has also been updated to match this change in general.

Finally, the change record has been updated, though could use someone to update the screenshots when this is ready to merge.

This feels like it might unblock things. Short version of how we get to the original end result assuming everything currently being worked on comes together:

1. Add the new filter here, but not automatically to views data yet.

2. Do #3458099: Views handler loading should respect configuration so configuration is respected in views.

3. After #2, do #3438054: Configurable views filters to allow for different widgets to allow the filter plugin to be selected in the UI. This would be an extra UI element in views, plus some kind of API for plugins or fields to declare compatibility with each other tbd.

The for entity reference fields, there'd be a choice of numeric and entity reference, and site builders can choose which one, and we don't have an upgrade path or bc concern specific to the entity reference change - although there is still quite a lot of that in #2 and #3 overall.

Thanks! Updated issue summary post tasks as per #218

Reviewed the MR and it all looks good to me.

Retitling because the latest approach doesn't make it available for all entity reference fields yet, that bit will happen in #3438054: Configurable views filters to allow for different widgets.

Leaving the framework manager review and subsystem review tags because the latest approach was my idea and this issue has a lot of twists and turns.

Hello all,

With all the twists and turns this issue has taken I wonder if there is a current solution to getting the Settings option back in place for exposed entity reference filters?

For 10.2.7 the patch here: https://www.drupal.org/files/issues/2021-12-02/2429699-453-9.3.x.patch worked well. In 10.3.1 I've made a local patch from MR !4053 mergeable (above) and applied it. Unfortunately it does not return the Settings option. Looking at the past few comments maybe it is not supposed to by itself?

Reading through these issue chains is pretty confusing for me, (and many others I imagine,) so I thought it best to ask for some clarification. Personally my site is frozen at 10.2.7 until there is a solution for this issue.

Thanks for your time and work on this issue, it sure looks like a big can of worms that has existed since D8.

My site is also frozen at 10.2.7. Let us know how the community can help out.

Hi @jodavidson,

Essentially the problem with making a separate reference field (the 'as a Reference' that existed in some versions of the patch) would be very hard to merge back later.

Then the problem of replacing all existing Entity Reference filters on place is that the current Numeric filter offers more options (like all node IDs > 100) that would be lost and therefore it'd be a breaking change.

So the current status is to provide the Entity Reference filter but not actually make use of it (some follow up tasks to allow a content editor to use it via the UI). Splitting into follow-ups means we can actually get it to land (hopefully) but making this scope significantly less risky.

There is a code snippet in the issue summary now that shows how to make use of the latest merge request, but assuming we get framework manager approval I'll create a module in contrib that effectively does that for now until the follow-ups land (on the framework manager approval @catch noted he wants other eyes to grant that since he's the one who came up with the idea of the above).

So tiny bit of limbo state as I don't want to go make that contrib module unless we are quite confident this will be the agreed direction.

Hope that helps clarify at least (I know it doesn't solve sorry!)

Our site is also frozen at 10.2.7, but (not realizing patch #453 would work), I applied this patch https://www.drupal.org/files/issues/2024-04-17/3347343-comment156-10.2.x... As mentioned, this patch required me to rebuild my views to use the new "field_name as a reference filter". All is stable for now. Thankful to those continuing to find solutions as we are very dependent on this feature.

Thanks @scott_euser.

If I understand the summary notes. I create a basically empty custom module that really only has the hook_views_data_alter() snippet in its MODULE.module file. And the patch should take effect once the custom module is activated?

Correct if you are using latest merge request. The automated test coverage has to do the same thing in order to test the filter, so you can see that code here applying to all entity reference fields that are currently set to use string or numeric filters: https://git.drupalcode.org/project/drupal/-/merge_requests/4053/diffs#2f...

With such code applied (replacing with your own module name instead of that module which is named "views_test_entity_reference"), after clearing cache, adding a new filter with entity reference should show you the entity reference options rather than numeric options for example.

The new approach makes sense to me too, follow up steps look good and make sense.

Left some minor nitpicks on the MR but this sounds like a great way to get something in without it being very disruptive.

Thank you for the review!

• Addressed feedback from #228.
• Marked 'get framework manager approval' as done (I believe with @catch and @lendude's direction approval here this is right)
• Marked 'update change record screenshots once green light has been given for this updated direction and considered ready to merge' as done (and made the updates)

I think we are ready then!

Committed/pushed to 11.x, thanks!

Moving this back to 10.4.x to discuss whether we want to backport it there - I think it meets the criteria for a backport to the maintenance branch because it will allow contrib modules to use the new filter plugin in their views data definitions, as well as a potential contrib module applying the filter to core entity reference fields.

Amazing thank you! And thank you everyone for all the contributions to this over the years! I will set up a module now to make it easy for people to opt-in to this + migrate from older versions of this patch. Watch this space. I think we should also suggest to the contrib modules that offer similar functionality like VERF to suggest this Core plugin instead and move to minimally maintained. I don't know enough about the contrib modules to know if that is right though so good if the maintainers of those can feed back about them.

Would love it if this gets backported to 10.4 as it means we can take advantage of it sooner without patching. Given the final approach, does no harm I think, so just a policy question I guess?

Module created here: https://www.drupal.org/project/views_core_entity_reference
Feedback and contributions welcome!

change record needs update for versions

How does that work: introduced in branch 11.x for now I guess, and leave version blank until there is a release? Or do we put 11.0.0 (and again change later if back ported)

As #231 said it needs backport to 10.4.x

Okay found the doc page for it https://www.drupal.org/community/contributor-guide/task/write-a-change-r...

1. updated branch to 11.x as where it's committed
2. left version blank as per instructions 'if unsure', and unsure as per #235
3. published as per instruction that it should be published when committed
4. didn't do 10.4 note as #231 says it's for discussion still to backport or not

If I got any of this wrong apologies!

In case the decision is to backport I created branch 3347343-10-4-x-backport

The test failure is to core/tests/Drupal/Tests/Core/Command/QuickStartTest.php which passes fine locally, but does take a very long time. Seems very unrelated to this.

Okay backport tests pass @catch in case you want to backport. Thanks!

Moving to needs review. Still need to get a second opinion on backport since this would be the first bigger new thing to get backported to 10.4.

Another +1 for getting this into 10.4.x! This is essential. Thank you all, great work!

I applied the 10.4 MR on 10.3 project ... but it doesn't seem to work - broken existing handler and no new filter available.
Sorry but I didn't have time to review the MR ... changing the status ... just to gain attention ... maybe I'm missing something

Probably on your 10.3 project you were using the 'as a Reference' filter version of the patch (ie, if your patch was over roughly a month old). Core won't have updates between patch versions but I did attempt to create an update in contrib, more details here: https://www.drupal.org/project/views_core_entity_reference

Restoring status

I checked again and it "works as designed" ... sorry about my previous noise.
@scott_euser Thanks for the support
Are we good to close this beast?

No problem! Keeping it RTBC for backport to 10.4 decision essentially I think, but either way we get this in 11 at the latest.

I'm working on a site that uses a computedfield like so:

  $fields['event_experts'] = BaseFieldDefinition::create('entity_reference')
    ->setName('event_experts')
    ->setLabel(t('Event Experts'))
    ->setSetting('target_type', 'node')
    ->setComputed(TRUE)
    ->setClass(ExpertsField::class);

The ExpertsField class extends EntityReferenceFieldItemList, uses ComputedItemListTrait and then implements computeValue().

With the changes here, when I load the edit page for a view of a Search API index that has this field as an exposed field, it results in this error:

TypeError: Drupal\Core\Field\BaseFieldDefinition::createFromFieldStorageDefinition(): Argument #1 ($definition) must be of type Drupal\Core\Field\FieldStorageDefinitionInterface, null given, called in /var/www/html/web/core/modules/views/src/FieldAPIHandlerTrait.php on line 52 in Drupal\Core\Field\BaseFieldDefinition::createFromFieldStorageDefinition() (line 83 of core/lib/Drupal/Core/Field/BaseFieldDefinition.php).

Is it possible that this functionality is incompatible with computed fields based on the entity_reference base field?

Are you opting in to the entity reference filter? Ive never used computed fields (that I know of), perhaps makes sense to raise a follow-up issue?

@scott_euser: No, I've only just applied the patch on a 10.3.2 site.

I'll open another issue and will reference it here.

I opened #3477391: computedfield vs entity_reference base field for the problem I've ran into.

Can you add the support for the entity none fields (properties)? like entity owner

@damienmckenna did you apply the MR diff or the patch? The patches here are extremely out of date with the current state of the MR.

@catch I applied the MR #9226, and it's worked fine for fields, but not for entity properties.

The property is showing as an auto-complete filter, but when you select and click apply it's not giving the right results.
I checked the query the where condition is not added at all

@anas_maw the MR doesn't change anything in views by itself, were you implementing custom code to add the filter to an entity property?

@catch: I believe it was the MR.

I did add a suggestion to your issue at #3477391: computedfield vs entity_reference base field to help confirm @damienmckenna

I can't see where in the code the Select list maximum number of options of 100 gets applied. I just did a manual test with 103 options and they all appeared in the select list.

Is this an oversight? Should the number of options be limited to 100?

Switching back to Needs Work (Sorry!)

Actually NVM, sorry my bad, I see where it is being applied.

This is not an issue with this code, it is a method further along the call tree in \Drupal\views\Plugin\EntityReferenceSelection\ViewsSelection::getReferenceableEntities that is not respecting the limit. I will investigate further and open a separate issue if need be.

Switching back to Reviewed & tested by the community. Sorry for the noise!

For those interested, I have added an issue and MR to re-add the functionality that switches a select widget to an autocomplete widget when the number of options hits a predetermined limit.

See #3481590: Views EntityReference filter: Add functionality to switch from select to autocomplete widget when a max number of options reached

I've been testing this on a local site using a custom config entity and it works wonderfully, the only issue I found was that if I publish or unpublish some of the target entities they aren't removed from the select options for anonymous users until cache is cleared/rebuilt.

With that said, I approve of merging this as - is.

A possible follow-up could be to add cache tags to the select element in valueFormAddSelect like this.

  /**
   * Adds a select element to the form.
   *
   * @param array $form
   *   Associative array containing the structure of the form, passed by
   *   reference.
   * @param \Drupal\Core\Form\FormStateInterface $form_state
   *   The current state of the form.
   */
  protected function valueFormAddSelect(array &$form, FormStateInterface $form_state): void {
    $referenced_type = $this->getReferencedEntityType();
    $form['value'] = [
      '#type' => 'select',
      '#title' => $this->t('Select @entity_types', ['@entity_types' => $referenced_type->getPluralLabel()]),
      '#multiple' => TRUE,
      '#options' => $options,
      // Set a minimum size to facilitate easier selection of entities.
      '#size' => min(8, count($options)),
      '#default_value' => $default_value,
      '#cache' => [
        'tags' => $this->getValueOptionsCacheTags($referenced_type),
      ],
    ];

}

And a helper method like this.

  /**
   * Gets the cache tags for the options based on the referenced entity type.
   *
   * @param \Drupal\Core\Entity\EntityTypeInterface $referenced_type
   *   The referenced entity type.
   *
   * @return array
   *   An array of cache tags.
   */
  protected function getValueOptionsCacheTags(EntityTypeInterface $referenced_type): array {
      // Get the cache tags based on the entity type.
      $cache_tags = $referenced_type->getListCacheTags();
      
      return $cache_tags;
  }

Or however you would integrate these new cache tags into the view. calculateDependencies()?
Anyway, I think the above fix worked for me.

@trackleft2 thanks for the additional testing. If you could open a follow-up for cache tags in the selection plugin that sounds good.

Since we don't have further details about issues that people have run into, or they turned out not to be an issue with this MR, going ahead with the 10.4.x backport here.

Committed/pushed to 10.4.x, thanks!

This is only API-facing at the moment but adding to 11.1.0 release highlights just in case we want to include it.