Entity Reference Views Limit not working

I don't think this is a good idea. The limit is hardcoded to 10 in \Drupal\Core\Entity\EntityAutocompleteMatcher::getMatches(), line:

      $entity_labels = $handler->getReferenceableEntities($string, $match_operator, 10);

This, I think, was the intention, to limit the matches to a decent number and preventing the flood of Ajax response with a too large list.

No matter what we do, we should ensure that some test coverage is given.

Sorry but it doesn't make sense to hardcode this value to 10.
People may have different requirements and it's such a simple thing to change.

Test coverage however is certainly necessary.

Re #8 can we make the limit 10, unless an explicit limit is set in the view (in which case use that)? This way it works out of the box no matter how many items in the view, but you can get more than 10 if you explicitly say you want that.

Entity reference views are single-purpose by their nature, so they are crafted to serve this autocomplete, and we can place a high level of responsibility on sitebuilders to configure them right - it's not like autocomplete is using a views display that is also used for some other purpose that might require a different limit.

I agree that if we give the option to set this, we should use the setting. So either we remove the ability to set the number of items for entity references or we actually use the setting. I would be for using the setting.

I agree with @claudiu.cristea that this might lead to issues when set to a high number, but there are many ways to use Views to dig yourself into a huge performance hole if you don't know what you are doing, so *shrug*

Needs work for tests.

I think we should postpone this issue until #2174633: View output is not used for entityreference options is resolved. Once that fix is in, here is a test to validate this fix (uploading as txt as it won't apply until 2174633 is committed).

+++ b/core/modules/field/tests/src/Functional/EntityReference/Views/SelectionTest.php
@@ -82,6 +82,41 @@ protected function setUp() {
+   * @group thisone

Ooops. Left my debug code in there. This should be removed when we reactivate this issue.

OK, let's limit to view's display limit if one is set.

Few notes:

• The test (provided in #16) is BrowserTestBase but we can achieve the same with a KernelTestBase.
• Writing the test, I discovered also a bug. When a view with no pager (pager plugin ID is none) is used, all the records are returned. But this is not OK as we need, at least, to fallback to the hard-limit of 10. This bug can be observed in the "test only" patch when the view is used without a pager. Normally, users are not noticing this bug because the entity reference display comes with pager by default. We should enforce a pager in such cases and use the hard limit of 10 items.

I don't think we need to wait that issue, since it's just the number of items.

I agree. I originally thought that fixing the above mentioned issue would include the limits imposed by the view but I was wrong. The code changes included in #19 (and earlier patches) are needed. Thank you for correcting that, @guilopes!

+++ b/core/modules/views/src/Plugin/views/display/EntityReference.php
@@ -156,6 +156,14 @@ public function query() {
+    // If the display has no pager set (plugin ID equals 'none'), set one.
+    if ($this->view->getPager()->getPluginId() === 'none') {
+      /** @var \Drupal\views\Plugin\ViewsPluginManager $pager_plugin_manager */
+      $pager_plugin_manager = \Drupal::service('plugin.manager.views.pager');
+      /** @var \Drupal\views\Plugin\views\pager\PagerPluginBase $pager */
+      $this->view->pager = $pager_plugin_manager->createInstance('some');
+      $this->view->pager->init($this->view, $this->view->getDisplay());
+    }

I disagree with this. I don't think we should be setting pagers when a site builder has specifically removed it (a default view comes with a pager of 10 items). I can't think of a good reason to allow unlimited options in an autocomplete, but that doesn't mean the reason doesn't exist.

We should be providing reasonable defaults and minimal limitations for site builders.

I totally disagree with not keeping, at least, the 10 items fallback. The rule that I'm proposing is: If a pager has been explicitly provided by the view, use it as limit. Otherwise default to 10 items. I would not allow infinite (the max count returned by the view) items -- there's a very good reason for limiting the autocomplete suggestion list.

I don't see that it matters in practice. It'd be a shame to block the patch on this point.

But because we need to decide one way or the other, I suggest we have the 10 item fallback limit if no pager as suggested by @claudiu.cristea, because a sitebuilder can always specify a pager with a 100000 item limit if that really really is what they want. In other words, if they want to live dangerously then they have to be explicit about it.

I can see more easily imagine situations in which a sitebuilder could get into trouble if there was no fallback limit, than I can situations in which they can't achieve what they want because there is a fallback limit.

@claudiu.cristea, @jonathanshaw: Good arguments and I concede the point. As @jonathanshaw points out a site builder could set the limit to 9999999 if they really wanted all results. Still, I'm worried about adding a pager to a view that doesn't have one, but I can quash that worry in order to get this into core... :)

The attached patch is revert of #22, which means is should be identical to @claudiu.cristea's patch in #19.

++ to adding the pager when none is set. People are free to set the limit on a ridiculous number, but at least they would do so knowingly. I think the solution to add a pager is more elegant then just hardcoding a limit on 10, if none is provided. With the hardcoded limit a site-builder would then have no idea where the limit is coming from and would be completely in the dark about how to circumvent this.

Updated the IS to show the current fix direction.

Not real happy with the spot where we are fixing this. Any call to \Drupal\views\Plugin\EntityReferenceSelection\ViewsSelection::initializeView will now get the limit set to the View limit.

So this call in \Drupal\views\Plugin\EntityReferenceSelection\ViewsSelection::validateReferenceableEntities

    if ($this->initializeView(NULL, 'CONTAINS', 0, $ids)) {
      // Get the results.
      $entities = $this->view->executeDisplay($display_name, $arguments);
      $result = array_keys($entities);
    }

which hardcodes its limit to 0, will now not use the passed 0, but the set limit on the View.

Moving the logic to \Drupal\views\Plugin\EntityReferenceSelection\ViewsSelection::getReferenceableEntities makes more sense to me, but that is also passed a $limit so the same would still go, just on a higher level (and it leads to some code duplication because we need to verify the View display twice before using it). So open to other ideas if somebody has them.

Added some dependency injection too.

I think it's pretty strange, though, that we are now completely throwing away the passed-in $limit as soon as the view has a pager. I think we should either check if the passed limit is 0 or maybe even do something like

min($passed_limit,
 $views_limit)

, but just (conditionally) ignoring it is strange IMO.

Is there an update of the patch in #27 for Drupal 8.6 available?

#31: !empty() should ensure it's > 0. That should be sufficient.

I am working on this..

@swatichouhan012: Did you accidentally unassign yourself, or were you only planning on working on it for 2 hours & 14 minutes? :)

This issue is still present in D9.3.0...

It's a shame this isn't fixed yet after years in core.
Any site builder CANNOT force an entity reference selection: even if his custom logic defined in a View results in 1 item, a checkbox on the entity form will list multiple options, that he cannot even see in the Views' preview!

Also, why can't this be fixed in 10.1.x instead of 11?!

I ran into this small issue using webforms to make a more visual selection of nodes. The patches are not working for 10.2.x. Would be great if this could be fixed , because showing ALL the results on a single page will result in performance issues.

I was able to fix this just by adding to
/core/modules/views/src/Plugin/EntityReferenceSelection/ViewsSelection.php

(ie. simpler than patch#27)

After the line with $this->view->setDisplay($display_name);
and before the line with $entity_reference_options = [
add these lines:

    $this->view->initPager();
    $items_per_page = $this->view->getItemsPerPage();
    if ($items_per_page > 0) {
      $limit = $items_per_page;
    }

I have uploaded the patch
Address #53 comment
Please review

#53 and patch from #54 seem to be working

edit: Patch work, but not page navigation is possible.

What do you mean?
There is no paging in entity references and if you set a limit in the View with "Display a specified number of items", you are explicitly removing the pager.
I just tested that the pager on another display (of type page) of the same View works.

The Needs Review Queue Bot tested this issue.

While you are making the above changes, we recommend that you convert this patch to a merge request. Merge requests are preferred over patches. Be sure to hide the old patch files as well. (Converting an issue to a merge request without other contributions to the issue will not receive credit.)

Create the Mr For Issue. Please Review

This will need test coverage

#54 appears to have dropped a good portion of the original patch, how come? @Prem Suthar see you opened an MR for that too so maybe you know too?

#54 appears to have dropped a good portion of the original patch

And this removal brought back the problem when the pager is not set, in which case the view returns all the results, instead of the specified limit in the widget.

I believe that in any case we should override the pager with parameters from the EntityReferenceSelection plugin.

And the implementation of what is described in the problem will cause confusion for users who want to determine the limit of results at the widget level.

I opened a new merge request that solves the problem where if a view doesn't have a pager set, all results are displayed.

Thanks @kksandr for restoring what was removed in 54 and previous MR

1) Drupal\Tests\field\Kernel\EntityReference\Views\ViewsSelectionLimitTest::testLimitedOutput
Failed asserting that actual size 20 matches expected size 10.
/builds/issue/drupal-2772523/core/modules/field/tests/src/Kernel/EntityReference/Views/ViewsSelectionLimitTest.php:84
FAILURES!
Tests: 1, Assertions: 8, Failures: 1.

Shows the test coverage so removing that tag.

Left some small comments on MR

Appears to be close!

Feedback appears to be addressed thanks!

The proposed resolution doesn't match the MR.

Also instead of dynamically changing the pager type, why not validate it when a view is selected?

Just ran into this "Entity Reference Views Limit not working". I want no pager, specific number of items 4, skipping 1, whereas I think this issue is focused on displaying greater than the default 10.

I'm trying to make it easy for a user to manage the display of a past event (image, title and metrics) by an entity reference select.

I want the user to only see the past 4 events, skipping the current one which has yet to take place.

The problem is that in Form Display, the entity reference is displaying all events. However, it is honoring skipping 1 (the current scheduled one).

The View Preview is displaying it correctly, only 4 items, skipping 1. When in use, this causes the error "This entity (node: nid) cannot be referenced." It's required to display all items under Pager.

I worked around the issue by adding a filter for schedule is between now and -18 months but the number of items that could be displayed could vary.

To me it seems the entire "Pager" section should not apply to entity reference. A separate "Entity Reference Limit" section would seem clearer with a default of 10. The user can then set to 0 or 4 and skip 1.