Pager pagination breaks when deleting content on Comment Bulk Operation

Hello @mahesh.umarane

Please attach view to display a list of unpublished comments with pagination on the issue description and provide some screenshots if possible.

Hi Team,

I have created patch for the above problem motivation. Need review.

Fixed the fails of #5

Steps to Reproduce the Issue:
1. Install Drupal 8.9.x
2. log in as an admin
3. Go to "admin/structure/views/view/content"
4. And update the "items per page" to 4 from the Pager option.
5. Now create 10+ dummy content.
6. Go to the "/admin/content" page.
7. Go to page 3 of the content list.
8. Select all the content and delete them.
8. Verify the page number. The page number is updating to 2 and no content is coming on it. While on that page we have content. (This is the issue that we fix in this ticket).

Testing Results:
I am not able to apply the patch. I tried manually as well but the patch not applied successfully.

I have manually added the changes by looking at #8. Now this patch should work

Hi dhirendra.mishra

This patch getting failed during upgrade of drupal 8 to latest release drupal 9 #3215465-11.patch. and getting the below error refer failed patch one while during update the composer.

Could you please update the patch of supporting the D9 as well to sort out this issue.

Failed Patch
Could not apply patch! Skipping. The error was: Cannot apply patch https://www.drupal.org/files/issues/2021-07-14/3215465-11.patch

Drupal 8.9.x is now open only to security issues.

Hi apaderno,

I have created the patch of latest version of Drupal core 9.2.5 and verified this patch and fuctionality working fine as expected.
Kindly check the patch of #13 comment - New Patch : 3215465-13.patch. Please verify the same.

Fixed custom command failed, added interdiff for same.

Applied patch #16 applied successfully and looks good for me

Thanks for the page ------

Steps to Reproduce the Issue:
1. Install Drupal 9.2.x-dev
2. log in as an admin
3. Go to "admin/structure/views/view/content"
4. And update the "items per page" to 5 from the Pager option.
5. Now create some approx 15 dummy content.
6. Go to the "/admin/content" page.
7. Go to last page of content list
8. Select all the content and delete them.
9. Verify the page number. The page number is updating to 2 and no content is coming on it.
10 . Now apply the patch and rebuild the cache u will and check it you will get expected result

This change looks to be a fix for a really annoying bug, so that is great. I don't like that we have a lot of changes in the tests, but it looks like that is because we were explicitly testing the presence of this bug? That sounds like a very weird thing it the design of those tests.

I'm not sure if need a change notice, but I would love if it if we could update the issue summary with the steps in #17

Patch #16 Applied successfully on Drupal 9.4.x.

Before Patch:

After Patch:

Applied patch #16 succesfully and working fine on drupal 9.4.x, refer to the screenshot.
Thank you

Looks like this still needs an issue summary update.

1. +++ b/core/modules/media_library/tests/src/FunctionalJavascript/WidgetViewsTest.php
   @@ -83,7 +83,7 @@ public function testWidgetViews() {
   -    $this->assertCount(1, $this->getCheckboxes());
   +    $this->assertCount(24, $this->getCheckboxes());
   

   I'm thrown by this quite big change to the test expectation. What's happening here??

2. +++ b/core/modules/views/src/Plugin/views/query/Sql.php
   @@ -1515,7 +1515,25 @@ public function execute(ViewExecutable $view) {
   +          $total_pages = floor($total_rows / $view->pager->options['items_per_page']);
   

   Use getItemsPerPage().

3. +++ b/core/modules/views/src/Plugin/views/query/Sql.php
   @@ -1515,7 +1515,25 @@ public function execute(ViewExecutable $view) {
   +          if (!empty($this->offset)) {
   +            if ($this->offset >= $total_rows) {
   +              $last_offset = $total_pages * $view->pager->options['items_per_page'];
   +              if ($last_offset == $total_rows) {
   +                $last_offset = $view->total_rows - $view->pager->options['items_per_page'];
   

   I'd like comments here to explain what we're calculating.

4. +++ b/core/modules/views/src/Plugin/views/query/Sql.php
   @@ -1515,7 +1515,25 @@ public function execute(ViewExecutable $view) {
   +              // It never should be negative integer, causes sql syntax error.
   

   This feels a bit hacky.

   Is there a way of doing the calculation without having to force it to be positive at the end?

Rerolled the patch for 9.5.x vesion

Patch for 9.5.x.

Tests fail in latest patch + we need to figure out why the test expectations have change so much. Back to needs work.

The problem is not only for this view, but for every view.