[view:total-rows] problem in Display a 'Specified number of items' pager

Created on 22 February 2022, over 2 years ago
Updated 28 February 2023, over 1 year ago

The [view:total-rows] token is miscalculating when I use the Display a specified number of items pager.

The token I use in the header:

Pager Settings:

Results:

Note:
If there is less value than the item I have specified, this token works properly.

This fix not only solves the token but also the problem of getting total_rows with php as below.

$rows = $view->total_rows;
🐛 Bug report
Status

Postponed: needs info

Version

10.1

Component
Views 

Last updated about 3 hours ago

Created by

🇹🇷Turkey seyfettinkahveci

Live updates comments and jobs are added and updated live.
  • Needs tests

    The change is currently missing an automated test that fails when run with the original code, and succeeds when the bug has been fixed.

Sign in to follow issues

Comments & Activities

Not all content is available!

It's likely this issue predates Contrib.social: some issue and comment data are missing.

  • 🇺🇸United States Kristen Pol Santa Cruz, CA, USA

    Thanks for reporting this issue. We rely on issue reports like this one to resolve bugs and improve Drupal core.

    As part of the Bug Smash Initiative, we are triaging issues that are marked "Postponed (maintainer needs more info)". This issue was marked "Postponed (maintainer needs more info)" more than a year ago for additional information. There was a response a few days later but wasn't clear enough as then the issue was tagged for steps to reproduce and there has been no activity since that time.

    Since we need more information to move forward with this issue, I am tagging for Bug Smash Initiative and keeping the status at Postponed (maintainer needs more info). If we don't receive additional information to help with the issue, it may be closed after three months.

    Thanks!

  • 🇺🇸United States paulmckibben Atlanta, GA

    I was able to reproduce this issue as follows:

    1. Create a block view of articles, where there are in excess of 200 published articles.
    2. Use a mini pager, and display 10 articles at a time.

    The result:
    - on the first page of results, I see view.total_rows = 11.
    - on the second page of results, I see view.total_rows = 21.
    - and keep incrementing by 10 on each subsequent page.

    Expected result: view.total_rows should always reflect the total number of rows for all pages, which is in excess of 200.

  • 🇺🇸United States paulmckibben Atlanta, GA

    Also, for what it's worth, the change in MR 1865 in #3 did not fix the problem for me.

  • 🇺🇸United States paulmckibben Atlanta, GA

    Also, the problem only happens with the mini-pager. The full pager shows the correct number of rows.

  • Status changed to Needs work over 1 year ago
  • 🇳🇱Netherlands Lendude Amsterdam

    @paulmckibben that should have been covered by #2572355: Some view tokens ([view:page-count] and [view:total-rows]) are incorrect , if that is still giving issues, please open a new issue for that. This issue (and suggested fix) are about the token when used with the fixed number of results pager

    Looking the code path here, I now get what is happening here and as @catch pointed out in #2572355: Some view tokens ([view:page-count] and [view:total-rows]) are incorrect is due to the fix with the token altering the query is a bit weird.

    The logic for count query being executed happens before the query is altered by limits set by pagers see \Drupal\views\Plugin\views\query\Sql::execute. Since the 'Some' pager was never intended to be used in combination with a count query (\Drupal\views\Plugin\views\pager\Some::useCountQuery is hardcoded to FALSE), this is otherwise not a problem, but this became an issue when #2572355: Some view tokens ([view:page-count] and [view:total-rows]) are incorrect forced the count query to run if that token is used.

    And we can't do the limiting before doing the count query, otherwise the count will be wrong for all pagers beside the 'Some', so maybe the proposed fix is the right way to go here. We still need test coverage for this though.

  • Status changed to Needs review over 1 year ago
  • 🇮🇳India mohit_aghera Rajkot

    - Updated issue summary with the template.
    - Add new test case to validate the fix
    - Uploading test-only patch as well so we can be sure about the bug.
    - Nit-picks in the method
    - Interdiff is taken against changes in comment #3

  • Status changed to RTBC over 1 year ago
  • 🇺🇸United States smustgrave

    Great work @mohit_aghera!

    Tested following the issue summary
    Created 2 pages
    Updated Content view to display 1
    Added header with token descripted
    Saw 2

    Applied patch
    Saw 1

  • Status changed to Needs work over 1 year ago
  • 🇦🇺Australia larowlan 🇦🇺🏝.au GMT+10
    +++ b/core/modules/views/tests/src/Kernel/TokenReplaceTest.php
    @@ -106,6 +106,38 @@ public function testTokenReplacementWithMiniPager() {
    +    $this->assertTrue($view->get_total_rows, 'The query was set to calculate the total number of rows.');
    

    Should this instead be ::assertGreaterThan(3, count($view->get_total_rows))

    Otherwise we can't be sure this test is actually working, if there are only 3 records, it will still pass.

  • Status changed to Needs review over 1 year ago
  • 🇮🇳India mohit_aghera Rajkot

    Addressing feedback in #18

  • 🇮🇳India mohit_aghera Rajkot

    Opps, used the incorrect method.
    Cancelling the test in #19

    Re-uploading the new patch against #14
    Diff is taken from #14 only.

    Hiding patch in #19

  • Status changed to RTBC over 1 year ago
  • 🇺🇸United States smustgrave

    Point #18 appears to have been addressed and passes showing the tests is working.

  • Open in Jenkins → Open on Drupal.org →
    Environment: PHP 8.1 & MySQL 5.7
    last update over 1 year ago
    29,203 pass
  • Open in Jenkins → Open on Drupal.org →
    Environment: PHP 8.1 & MySQL 5.7
    last update over 1 year ago
    29,284 pass
  • Open in Jenkins → Open on Drupal.org →
    Environment: PHP 8.1 & MySQL 5.7
    last update over 1 year ago
    29,284 pass
    • catch committed f7c10393 on 10.1.x
      Issue #3265798 by mohit_aghera, seyfettinkahveci, paulmckibben,...
  • 🇬🇧United Kingdom catch

    Committed f7c1039 and pushed to 10.1.x. Thanks!

  • Automatically closed - issue fixed for 2 weeks with no activity.

  • 🇺🇸United States tjmoyer

    Unintended consequence here: Views using the "Display a specified number of items" pager and a more link no longer display the more link now that this patch has been committed. The more link does show if you switch to another pager, but not this one. Commenting out the lines added to core/modules/views/src/Plugin/views/pager/Some.php makes the more link display properly again.

    I think to address the original issue, the total-rows number was correct as that's the total number or rows before any pager is used. To get the total number of shown results you should use something like [view:items_per_page] instead.

  • 🇳🇱Netherlands Lendude Amsterdam

    @tjmoyer thanks for reporting the regression, could you open a new issue for that? (and maybe link it here so that people who worked on this might help out fixing the regression)

    I don't think using [view:items_per_page] would work, because that would give the wrong number if you have less results than that number. But lets discuss in the new issue

  • Status changed to Fixed about 1 year ago
  • 🇳🇱Netherlands Lendude Amsterdam

    🐛 More link is missing in pager when using the "Some" pager and there are more records than shown Needs work was opened to try and fix the regression for the 'more' link

  • 🇨🇦Canada tame4tex

    This has also broken any site that used @current_record_count of @total in a Results Summary when using a "Some" pager.

    I have opened 🌱 "Some" Pager Plugin and [view:total-rows] Regression Issue Active to address this.

Production build 0.71.5 2024