Add ability to dynamically set limit on EntityListBuilder

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.

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'm uploading patch since I don't have access to change the destination.
Test and code-commit check passing on local

Tests look good!

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

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

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

Let me work on #23.

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

Fixing feedback mentioned in #23

I have resolved the #23. Please review.

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.

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

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 →

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

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.

Let me work on #33

Fixed the issue mentioned in #33. Please review.

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.

@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.

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

- Updated the issue summary.
- Updated the CR

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