Pager always returning first page

Patch to follow shortly

see patch

Tested this patch on an environment running the latest Views with the recent security fix, this patch fixed the pager issue of only returning the page one results.

Note: Our environment is php 5.6.22

@joseph.olstad : I guess you're using https://www.drupal.org/project/views_ajax_get ? There's no way with core D7 views to have the AJAX exposed output use a GET, and backing out the change will cause issues for everyone using only core views with an ajax pager.

Ideally, the fix should be in Views AJAX GET.

@DeFr thanks for the tip, I cannot yet confirm the hypothesis, follow up Monday morning to check and I will report back the findings.

Either way POST works fine with or without the patch. Hope this helps.

Background information
NOTE: there is a very good reason why we WANT to use GET instead of POST.

it simplifies caching of AJAX requests, all our ajax requests are cached using "boost", without caching AJAX requests we would not be able to handle the very intense spikes in activity that we normally have to deal with.

currently POST is not a good option for us.

On monday I will verify whether or not we are using views_ajax_get which I think we are, so we could probably move this issue over to the views_ajax_get project. *EDIT* we are NOT using the views_ajax_get module

Currently on my test box a cached AJAX page takes less than 10 ms to load, compared to about 900ms for an uncached ajax request.

I am putting this back to RTBC because this issue is a deal breaker for us.

We have regularly about 500+ concurrent connections at any given moment in time with spikes taking us into 30000 concurrent connections. Without caching of anonymous ajax requests it would cost us way too much money to handle these types of loads on a daily basis. We need to provide proper anonymous caching of pages and views ajax requests. Currently the best way to do caching is with GET requests instead of POST. This is a deal breaker for us. There should be an issue for this in the "Views" queue as well as the "views_ajax_get" issue queue. I will open up an issue in views_ajax_get and mark it as a related issue.

Drupal 8 views is moving in this direction so to be forward thinking we should keep this capability possible in Drupal 7.
Here is the Drupal 8 issue for "Allow to use AJAX with GET requests"
issue #956186: Allow AJAX to use GET requests

The patch in comment #3 allows us to continue using GET which allows us to use caching. Without load it's 10ms (with boost or varnish caching) vs 900ms (without caching) , under load the difference is exacerbated and the écart widens as the load increases. I have read that it is possible to cache with POST as some people have found ways to do that however the current recommended way to use boost/varnish ajax caching is with GET requests. Up until views 7.x-3.13 GET requests cached using contrib modules boost with GET requests made possible with views_ajax_get worked fine, but broke in 7.x-3.14 because of commit in comment #27 of #1482824: Block display view ajax pager does not advance with multiple pagers on a page when the first pager > 0

Lucky #13

checked the api for drupal_get_query_parameters and the second parameter is for "REMOVING" , we need "page" otherwise it will always return page 0

new patch uses drupal_get_query_parameters as in #1482824: Block display view ajax pager does not advance with multiple pagers on a page when the first pager > 0 instead of reverting to 7.x-3.13 version of includes/ajax.inc

Looking for a review of the new patch

Removing the page parameter is the point of the patch in #1482824: Block display view ajax pager does not advance with multiple pagers on a page when the first pager > 0.

Let's recapitulate what was going on pre #1482824: Block display view ajax pager does not advance with multiple pagers on a page when the first pager > 0 for people that don't have Views AJAX GET enabled: if you reach an AJAX enabled views, with a specific page in the URL (for example, because you've clicked an edit link that have a destination parameter), then, you can't go back to the first page: in that case, there's no _POST 'page' parameter, because the first page has no 'page' query string by default. At that point, the initial GET parameter will then take effect, and you're stuck on the page you've initially loaded.

As is, this patch will regress that behavior, and people will get stuck again.

views_infinite_scroll is affected by the regression in views 7.x-3.14

Until I better understand this issue, here is a patch that keeps views_infinite_scroll working.

see this line (49 and 50) in views_infinite_scroll

See new patch for views.

We expect to be able to get page 1, 2, 3, 4 in ajax GET requests when people click our 'show more' button.

Using views 7.x-3.13 page 1 , 2 , 3 , 4 were correctly retrieved, boost would cache ajax GET requests as follows:

sites/all/files/cache/normal/localhost/views/ajax_view_name=news&view_display_id=facebook_style&view_args=6972.json
sites/all/files/cache/normal/localhost/views/ajax_view_name=news&view_display_id=facebook_style&view_args=6972&page=1.json
sites/all/files/cache/normal/localhost/views/ajax_view_name=news&view_display_id=facebook_style&view_args=6972&page=2.json
sites/all/files/cache/normal/localhost/views/ajax_view_name=news&view_display_id=facebook_style&view_args=6972&page=3.json
sites/all/files/cache/normal/localhost/views/ajax_view_name=news&view_display_id=facebook_style&view_args=6972&page=4.json
sites/all/files/cache/normal/localhost/views/ajax_view_name=news&view_display_id=facebook_style&view_args=6972&page=5.json

and so on, page X through 9999

then June 15th after upgrading to views 7.x-3.14

the 'page' was removed (due to a new second 'exclude' parameter containing array('page') to drupal_get_query_parameters() in views/includes/ajax.inc (this strips out page, but we WANT page)) and the ajax response
sites/all/files/cache/normal/localhost/views/ajax_view_name=news&view_display_id=facebook_style&view_args=6972.json
is ALWAYS returned, regardless of which page is requested (for us it's when someone clicks on 'show more' , we load ajax to get the next page )

So now in 7.x-3.14 without a patch, when someone clicks 'show more' the same results

sites/all/files/cache/normal/localhost/views/ajax_view_name=news&view_display_id=facebook_style&view_args=6972.json

are returned over, and over, and over again.

The solution I found was to back off the commit from comment #27 of #1482824: Block display view ajax pager does not advance with multiple pagers on a page when the first pager > 0

OR, to patch views with either patch #3 or patch #18

Patch #3 will also (in addition to restoring ajax GET page functionality) resolve several warnings and error messages that we noticed in the watchdog.
example:

1. MESSAGE Notice: Undefined index: q in arg() (line 2933 of 744core/includes/bootstrap.inc).
2. MESSAGE Notice: Undefined index: q in theme_pager_link() (line 638 of 744core/includes/pager.inc).

3. MESSAGE Notice: Undefined index: q in template_preprocess_views_view() (line 130 of 744core/sites/all/modules/views/theme/theme.inc).</li>
     <li>

Patch #3 seems to be the winner here, it reverts includes/ajax.inc to what it was in views 7.x-3.13

For us using views_ajax_get, the assumption that correct page number is going to be on POST and that is safe to overwrite the page parameter at GET, is wrong.
I'm not sure if fixing in views_ajax_get is feasible, it would involve doing some complex hooking on views which i'm not even sure its possible.
The issue with the original fix on 7.x-3.14 is that it assumes views is using POST, and then it is discarding the page value in GET if any.
To cover both scenarios, we're testing this patch where we remove page from GET only if it is present in POST. (sorry about the naming on the patch, in a hurry over here but wanted to share)

Hi @Sergio.Tashdjian I like your patch and it works however it introduces a whole bunch of watchdog notices.
Here is what I'm seeing:

Notice: Undefined index: q in template_preprocess_views_view() (line 130 of /core/sites/all/modules/contrib/views/theme/theme.inc).

To resolve these notices I created patch #3. ASAP, I'll try and see if I can incorporate your patch ideas with my patch.

Hi @Sergio.Tashdjian, a couple changes to your patch, using this patch there are no longer any watchdog warnings , resolves our issues, should work for you as well.

Please review.

Thanks @joseph.olstad !
I was not getting emails from this thread, so i'm seeing your finding on the watchdog warnings right now.
We've found those a couple days ago, and fixed a bit differently than you, but both ways seem to work.
What happened is that drupal_get_query_parameter() excludes the 'q' item from the array by default, so when we added
drupal_get_query_parameters($_GET); as a case, instead of drupal_get_query_parameters($_GET,array('page')); to avoid getting 'page' ignored,
we ended up removing 'q', because second parameter defaults to array('q'). So to fix that we simply sent an empty array as second parameter like this:
drupal_get_query_parameters($_GET,array()); and that way 'q' is not removed anymore.

I'm not 100% sure usage of drupal_get_query_parameters with an empty exlude array is any different from using $_GET directly, it may be worth to confirm, for peace of mind :)

Thanks again for sharing your experiences on this one.

Sounds like either way will work, we've been using our patch #23 for a while, its doing the job.

feel free to add a new patch with that change.

@Sergio.Tashdjian I got impatient waiting for you to write a new version of your patch.

This patch is your patch I believe with the changes as you described in your last comment.

I have not tested this one, but I have tested MY patch, (#23)

The patch #26 worked for me.
But the $_POST['page'] variable is not always set. So it is better to check it to avoid notices.

Hi @sergiuteaca, indeed, your new patch is better, avoids notices .

Thanks.

RTBC +1

views_infinite_scroll depends on this being patched.
see related issue
#2755395: Block pager does not properly load new items

This is also a very important fix for us.

I've attached a patch that condenses the fix a bit & updates the comment to reflect the fix explaining why this is necessary.

#31 fixes the above issue for me after updating from 7.x-3.13 to 7.x-3.14. I have also confirmed that infinite scroll views continue to work.

This seems to be a problem in Drupal 8.2.6 - can anyone else confirm?

@sittard , I agree with you, this needs to be fixed in both D7 and D8, please report your findings in this issue:

#1482824: Block display view ajax pager does not advance with multiple pagers on a page when the first pager > 0

Am planning to include this regression fix in 3.16.

Committed. Thanks!

awesome, thanks DamienMckenna!

I realise the commit in #36 was designed to restore compatibility for views_ajax_get and views_infinite_scroll, but it also breaks vanilla views in some cases.

I can't reopen this issue so I figured I'd better create a new one: #2922131: AJAX views accessed with ?page=n parameter become blank when exposed filter makes number of pages ≤ n

Does #31 cause any issues for other ajax calls ?? I want to apply this patch to a live site.

You don't need #31, just upgrade/update your views from your current version to views 7.x-3.20. It's included in 3.20

On first click on 'see more' its load the next page items (page 1). But from 2nd click its just repeat the same item. on ajax request I am getting an extra get parameter after page parameter like example.com/offers?offer_category=All&page=1&personal%2Foffers= and always getting 1 for page parameter.

But in cdn site its working perfectly ajax request url is like cdn.example.com/offers?offer_category=All&page=2

Unable to find the cause why its not working on main site.
I am using views version = "7.x-3.8"

If you need have his problem the solution is to upgrade views to the current release 7.x-3.20 or newer.

Prior to 3.16 you will likely have this problem