Problem/Motivation

\Drupal\Core\Entity\EntityListBuilder() has a hardcoded limit of 50 entities, at which it paginates. This is a fine default, but there's no way to override it dynamically, making it unusable for any use case that requires the entire list (such as the node/add, page, for example--see #2693485: Content types are ordered by machine name on /node/add page (+ similar issues with other entities)). It would sure be nice to have a public setLimit() method. Patch to follow.

Steps to reproduce

Get the entity list builder service to build list of entities.
Note that you'll received 50 entities in the first page load since it is hardcoded to 50. Each page will have 50 items.
It should be flexible so we can pass the number of items that we want in pager.

Proposed resolution

\Drupal\Core\Entity\EntityListBuilder() should have a public method setLimit() that allows to override the pager limit for enity list builder service

Remaining tasks

Based on feedback from #33 we need a general agreement from community about the solution approach.

User interface changes

N/A

API changes

A new method setLimit() is introduced to EntityListBuilder class.

Data model changes

N/A

Release notes snippet

EntityListBuilder now allows to override the number of items per page using setLimit() method.

Issue fork drupal-2736377

Command icon Show commands

Start within a Git clone of the project using the version control instructions.

Or, if you do not have SSH keys set up on git.drupalcode.org:

Comments

TravisCarden created an issue. See original summary.

traviscarden’s picture

Assigned: traviscarden » Unassigned
Status: Active » Needs review
StatusFileSize
new1.18 KB
traviscarden’s picture

Title: Add ability to set dynamically set limit on EntityListBuilder » Add ability to dynamically set limit on EntityListBuilder

Version: 8.2.x-dev » 8.3.x-dev

Drupal 8.2.0-beta1 was released on August 3, 2016, which means new developments and disruptive changes should now be targeted against the 8.3.x-dev branch. For more information see the Drupal 8 minor version schedule and the Allowed changes during the Drupal 8 release cycle.

Version: 8.3.x-dev » 8.4.x-dev

Drupal 8.3.0-alpha1 will be released the week of January 30, 2017, which means new developments and disruptive changes should now be targeted against the 8.4.x-dev branch. For more information see the Drupal 8 minor version schedule and the Allowed changes during the Drupal 8 release cycle.

Version: 8.4.x-dev » 8.5.x-dev

Drupal 8.4.0-alpha1 will be released the week of July 31, 2017, which means new developments and disruptive changes should now be targeted against the 8.5.x-dev branch. For more information see the Drupal 8 minor version schedule and the Allowed changes during the Drupal 8 release cycle.

Version: 8.5.x-dev » 8.6.x-dev

Drupal 8.5.0-alpha1 will be released the week of January 17, 2018, which means new developments and disruptive changes should now be targeted against the 8.6.x-dev branch. For more information see the Drupal 8 minor version schedule and the Allowed changes during the Drupal 8 release cycle.

Version: 8.6.x-dev » 8.7.x-dev

Drupal 8.6.0-alpha1 will be released the week of July 16, 2018, which means new developments and disruptive changes should now be targeted against the 8.7.x-dev branch. For more information see the Drupal 8 minor version schedule and the Allowed changes during the Drupal 8 release cycle.

Version: 8.7.x-dev » 8.8.x-dev

Drupal 8.7.0-alpha1 will be released the week of March 11, 2019, which means new developments and disruptive changes should now be targeted against the 8.8.x-dev branch. For more information see the Drupal 8 minor version schedule and the Allowed changes during the Drupal 8 release cycle.

Version: 8.8.x-dev » 8.9.x-dev

Drupal 8.8.0-alpha1 will be released the week of October 14th, 2019, which means new developments and disruptive changes should now be targeted against the 8.9.x-dev branch. (Any changes to 8.9.x will also be committed to 9.0.x in preparation for Drupal 9’s release, but some changes like significant feature additions will be deferred to 9.1.x.). For more information see the Drupal 8 and 9 minor version schedule and the Allowed changes during the Drupal 8 and 9 release cycles.

Version: 8.9.x-dev » 9.1.x-dev

Drupal 8.9.0-beta1 was released on March 20, 2020. 8.9.x is the final, long-term support (LTS) minor release of Drupal 8, which means new developments and disruptive changes should now be targeted against the 9.1.x-dev branch. For more information see the Drupal 8 and 9 minor version schedule and the Allowed changes during the Drupal 8 and 9 release cycles.

Version: 9.1.x-dev » 9.2.x-dev

Drupal 9.1.0-alpha1 will be released the week of October 19, 2020, which means new developments and disruptive changes should now be targeted for the 9.2.x-dev branch. For more information see the Drupal 9 minor version schedule and the Allowed changes during the Drupal 9 release cycle.

Version: 9.2.x-dev » 9.3.x-dev

Drupal 9.2.0-alpha1 will be released the week of May 3, 2021, which means new developments and disruptive changes should now be targeted for the 9.3.x-dev branch. For more information see the Drupal core minor version schedule and the Allowed changes during the Drupal core release cycle.

Version: 9.3.x-dev » 9.4.x-dev

Drupal 9.3.0-rc1 was released on November 26, 2021, which means new developments and disruptive changes should now be targeted for the 9.4.x-dev branch. For more information see the Drupal core minor version schedule and the Allowed changes during the Drupal core release cycle.

Brian C made their first commit to this issue’s fork.

Re-rolled this to satisfy PHP 7.2 deprecation of string arguments for assert().

Version: 9.4.x-dev » 9.5.x-dev

Drupal 9.4.0-alpha1 was released on May 6, 2022, which means new developments and disruptive changes should now be targeted for the 9.5.x-dev branch. For more information see the Drupal core minor version schedule and the Allowed changes during the Drupal core release cycle.

Version: 9.5.x-dev » 10.1.x-dev

Drupal 9.5.0-beta2 and Drupal 10.0.0-beta2 were released on September 29, 2022, which means new developments and disruptive changes should now be targeted for the 10.1.x-dev branch. For more information see the Drupal core minor version schedule and the Allowed changes during the Drupal core release cycle.

smustgrave’s picture

Status: Needs review » Needs work
Issue tags: +Needs Review Queue Initiative, +Needs tests

This issue is being reviewed by the kind folks in Slack, #needs-review-queue-initiative. We are working to keep the size of Needs Review queue [2700+ issues] to around 400 (1 month or less), following Review a patch or merge request as a guide.

MR will need to be updated for 10.1
New function should be typehinted

Also will need test coverage.

sourabhjain made their first commit to this issue’s fork.

mohit_aghera’s picture

Status: Needs work » Needs review
Issue tags: -Needs tests
StatusFileSize
new6.13 KB
new5.95 KB

Fixed the following things in the patch:
- Added return types and argument types in the methods.
- Added test cases to validate that limit affects listing as well as pager count.

I've specifically added functional test because Kernel test was not able to fetch the proper pager count.
Also interdiff is taken from patch #2 since almost all the patches/changes are same.

I'm uploading patch since I don't have access to change the destination.
Test and code-commit check passing on local

smustgrave’s picture

Issue tags: +Needs change record

Tests look good!

Think this will need a change record to show how users can change this.

tedfordgif’s picture

Given that the setLimit() method returns self, it probably makes sense to test that contract in the test, e.g. instead of these two lines:

+    $list_builder->setLimit(FALSE);
+    $build = $list_builder->render();

just this one

+    $build = $list_builder->setLimit(FALSE)->render();
sourabhjain’s picture

Assigned: Unassigned » sourabhjain
Status: Needs review » Needs work

Let me work on #23.

tedfordgif’s picture

My apologies for leaving out the assignment to $build, I'll edit my comment.

mohit_aghera’s picture

Status: Needs work » Needs review
StatusFileSize
new6.11 KB
new989 bytes

Fixing feedback mentioned in #23

Added change record here: https://www.drupal.org/node/3349556

sourabhjain’s picture

Assigned: sourabhjain » Unassigned
StatusFileSize
new6.11 KB
new911 bytes

I have resolved the #23. Please review.

sourabhjain’s picture

Ahh! mohit_aghera has added the patch at the same time while I was uploading it so same patch added in #26 and #27 so you can ignore any of the patch.

borisson_’s picture

Issue tags: -Needs change record

Unpublished the change record, it should only be published when it is actually merged. Removing the tag as well,

smustgrave’s picture

Status: Needs review » Reviewed & tested by the community

Change record looks good and makes sense.

Will let committer decide but this may get sent back for IS update (if anyone wants to do that before it's reviewed)

The last submitted patch, 21: add_ability_to_set-2736377-21.patch, failed testing. View results

borisson_’s picture

Not sure why it was retesting #21, enabling #27 instead.

quietone’s picture

Issue summary: View changes
Status: Reviewed & tested by the community » Needs work

Triaging the RTBC queue.

Always nice to see the old issue getting resolved. Go Needs Review Queue Initiative!

As pointed out in #30 the IS is incomplete. There is very little there to help the reviewer use their time efficiently. We should assume that the Issue Summary always needs to be complete. Another committer has consistently told me they set back issues that do not have a complete proposed resolution section and I am more particular than they are!

Reviewing the comments I see reviews of the test but no comments about the actual code changes, which were first introduced in 2016. Is this the best solution? Of course, correct me if I am missing the review.

I read the code comments. Function comments are to use a third person singular present test verb (yes it is in the standard)

  1. +++ b/core/modules/system/tests/modules/entity_test/src/Controller/EntityTestController.php
    @@ -121,4 +122,20 @@ public function listEntitiesEmpty($entity_type_id) {
    +   * Render the list of all the entities with custom setLimit argument.
    

    This doesn't seem quite right. I think this is what is meant, "Renders a list of the specified number of entities."

  2. +++ b/core/modules/system/tests/src/Functional/Entity/EntityListBuilderTest.php
    @@ -57,6 +57,58 @@ public function testPager() {
    +   * Test the arguments in setLimit() method.
    

    Can be simpler. "Tests the setLimit() method."

I read the CR and it does have all the information. But it does need some work to improve the grammar and readability, including the title. There are two examples given and I think it would help to have those in a section titled 'Examples'. Remember, that not all the readers of the CR are native English speakers and we need to strive to use correct English and be concise and clear.

Setting to NW for the above items.

sourabhjain’s picture

Assigned: Unassigned » sourabhjain

Let me work on #33

sourabhjain’s picture

Assigned: sourabhjain » Unassigned
Status: Needs work » Needs review
StatusFileSize
new6.08 KB
new1.1 KB

Fixed the issue mentioned in #33. Please review.

smustgrave’s picture

Status: Needs review » Needs work

Please read the full comment and not jump straight to the suggestions. It’s making it increasingly difficult to keep up with tickets that keep doing this.

Called for an issue summary update
Verification of the solution.
Change record updates.

quietone’s picture

@sourabhjain, in cases like this, where you respond to part of a review, it is good practice to comment on exactly what you did do to address #33 and the things you did not do. This helps the next person to know what to review and what to work on.

sourabhjain’s picture

Sure @quientone. So in #35, I have resolved the point 1 and 2 suggestion which is mentioned in #33. Please review it.

mohit_aghera’s picture

Issue summary: View changes

- Updated the issue summary.
- Updated the CR

Still keeping in needs work since we need consensus around the solution.

mohit_aghera’s picture

Issue summary: View changes

Version: 10.1.x-dev » 11.x-dev

Drupal core is moving towards using a “main” branch. As an interim step, a new 11.x branch has been opened, as Drupal.org infrastructure cannot currently fully support a branch named main. New developments and disruptive changes should now be targeted for the 11.x branch, which currently accepts only minor-version allowed changes. For more information, see the Drupal core minor version schedule and the Allowed changes during the Drupal core release cycle.

Version: 11.x-dev » main

Drupal core is now using the main branch as the primary development branch. New developments and disruptive changes should now be targeted to the main branch.

Read more in the announcement.