There has been an ongoing issue with Views PHP and Search API where the value field never, ever, returns anything to the output box when using a Search Index as the base entity for a view using a PHP Field.

Recently, a post regarding the Search API Views contributed module https://drupal.org/node/2096275 added a couple of calls into their query.inc file that will call the Views PHP pre/post execute functions:


try {
      // Patching per: https://drupal.org/node/2096275
      // Trigger pager pre_execute().
      $this->pager->pre_execute($this->query);
      $start = microtime(TRUE);

      // Execute the search.
      $results = $this->query->execute();
      $this->search_api_results = $results;

      // Store the results.
      $this->pager->total_items = $view->total_rows = $results['result count'];
      if (!empty($this->pager->options['offset'])) {
        $this->pager->total_items -= $this->pager->options['offset'];
      }
      $this->pager->update_page_info();
      $view->result = array();
      if (!empty($results['results'])) {
        $this->addResults($results['results'], $view);
      }
      // We shouldn't use $results['performance']['complete'] here, since
      // extracting the results probably takes considerable time as well.
      $view->execute_time = microtime(TRUE) - $start;
      // Patching per: https://drupal.org/node/2096275
      // Trigger pager post_execute().
      $this->pager->post_execute($view->results);
    }

this actually DOES partially resolve the issue with the value field not being connected to the output field. However, it also causes the pager to break.

When there is a view php field in the view, the pager goes away. When there is not a view php field in the view, the pager works fine. if I remove the patch above (the pre_execute and post_execute calls) then the pager immediately begins working but the value field no longer passes information onto the output box.

I've spent several hours trying to backtrace data through views php and I'm not seeing anything immediately obvious.

The biggest thing I notice is that in the $view object that is being passed around in the views_php.module file is that the $view->current_page and $view->items_per_page aren't being set properly after the post_execute function runs. I have tried manually setting this to match what is in $view->query->pager but that doesn't seem to make a difference.

Views PHP saves lives! However it's been very difficult trying to integrate more and more complex features into Search API powered views. This could well be a Search API issue, but if anyone could help me a bit to understand how Views PHP actually renders the pager with the data, that would be incredibly helpful.

Note: This may be related to: https://drupal.org/node/2024689

Thanks !

Members fund testing for the Drupal project. Drupal Association Learn more

Comments

mikemadison’s picture

Issue summary: View changes

correctly wrapping php code

mikemadison’s picture

After digging into this, I believe I have the reason.

In views_php/plugins/views/views_php_plugin_pager.inc ~line 40

$this->wrapped->total_items = count($this->wrapped->view->result);

The module is setting the total number of items to the count of the view's result. This is fine when the view isn't paged. However, if the view IS paged, the view->result object is only going to contain the number of rows specific in the 'per page' setting. So, if I have 500 items, paged with 10 items per page, my view->result is always going to be 10, no matter what page I'm on or how many items are there.

As far as I can tell,

$this->wrapped->total_items

is actually set when coming into this function from the view. So, I have tried this minor change in the code and as far as I can tell, all is well. The paging starts working again, the $value field continues to work, and all is well in the world.

if (!isset($this->wrapped->total_items)) {
  $this->wrapped->total_items = count($this->wrapped->view->result);
}
mikemadison’s picture

Assigned: Unassigned » mikemadison
Issue summary: View changes
Status: Active » Needs review
FileSize
775 bytes
Colin @ PCMarket’s picture

Hi lalweil

Thank you for your continued efforts working on this issue.

When trying to apply the patch i receive the following error:
patch: **** malformed patch at line 18:

I did however just apply the patch manually to give it a go.

For me, it did make the pager appear again, however when i click on the link to page 2 it returns no results found. i experimented with decreasing the number of items per page on the view settings and this resulted in every page other than page 1 of the results from the pager still returning no results

mikemadison’s picture

I am able to reproduce the problem. In digging a bit, it appears that the pager is redundantly setting the $view->result object. So, I'll apply the same logic as in the other patch to this. Basically, turning:

$this->wrapped->view->result = array_slice($this->wrapped->view->result, $offset, $item_per_page);

into:

if (!isset($this->wrapped->view->result)) {
        $this->wrapped->view->result = array_slice($this->wrapped->view->result, $offset, $item_per_page);
      }

I'll get a new patch in a moment

mikemadison’s picture

Colin @ PCMarket’s picture

Patch applied properly and has restored a working pager on my views when a view_php field is enabled

Thank you for the great work!

sassafrass’s picture

My pagers worked with Views PHP before updating to Search API 7.x-1.9. After updating, the pager block disappeared. I applied the patch in #52123315-Views-PHP-Search-API-Paging.patch, and it solved the pager problem for me. Thank-you!

gthing’s picture

Woo! This one threw me for a loop. I investigated possible issues with practically every other module involved before realizing it was the presence of the php field causing the pager to disappear. I can confirm the patch in #5 fixed the issue for me. Thanks!

johntang’s picture

On my case it's working also, But other normal views with Display a specified number of items on PAGER setting, it's show all results :(

timkang’s picture

I believe this issue was caused solely by Search API, and should be resolved with the recently committed patch for #2146435: Fix Views paging with custom pager add-ons. My comment there (#11) tries to explain a bit on how I believe Search API and Views PHP are supposed to fit together.

On lalweil's patch:
I believe Views PHP is working as designed. views_php_plugin_pager::post_execute() cannot simply leave $this->wrapped->total_items as-is, since custom PHP views filters can filter out and reduce the number of total results. $this->wrapped->view->result can't be left as-is; since the query limit and offset are removed in views_php_plugin_pager::pre_execute(), it contains all of the results, and not just those on a specific page (johntang's issue above in #9). My comment linked above explains why.

johntang’s picture

Yes, It should be fixed caused by Search API module, I have tested #10 patch. No have changes from PHP Field module.

johntang’s picture

Status: Needs review » Closed (fixed)
welly’s picture

Status: Closed (fixed) » Needs work

I'm not sure this is working correctly. Since applying the patch, when I create a custom Views PHP field, the $row data is now null. This wasn't occurring before the patch and nothing has changed in my view to make this no longer work.

johntang’s picture

@welly

Did you reverted views_php_plugin_pager.inc before try this patch?

welly’s picture

Good question, @johntang. I suspect I didn't but I will try again. I'll re-download the module and add the patch again.

Vuds’s picture

Status: Needs work » Reviewed & tested by the community

Hello there,

I'd like to put my two cents here and say that patch in #5 worked for me AND I'm not using Search API.

I have many other views in my app that are more complicated (with Infinite Scroll or Load More Pager, Geofields and IPGV&M, etc.) and they are working well with Views PHP.

But in a very simple unformatted list to display three fields and 18 itens per page limit, it didn't.

Thanks for the patch!

rclemings’s picture

My experience is the same as #16.

I'm not using Search API, but adding Views PHP fields to a couple of views caused the pager to disappear. And the number of results returned (as shown in Header: Global: Result summary) was limited to whatever number I set in the pager's "items per page" field.

Applying the patch in #5 fixed the problem. No side effects that I can see.

Is there a reason it can't be committed? I see the project is marked "maintenance fixes only" but wouldn't this qualify?

szt’s picture

Same as #16 and #17.
#5 works well.

DrCord’s picture

I am using Search API, but was experiencing the problem when it was disabled and on views not interacting with it. The #5 patch fixed it perfectly and easily. This is certainly a bug in the Views PHP module.

nimek’s picture

Very Annoying Bug pager in views simply dissappared after i added php field

#5 patch fixed it.

Please commit

cpm5280’s picture

#5 solved my issue, happily.

Jan-E’s picture

@cpm5280; could you try if the combination of these two patches also works:
https://www.drupal.org/node/2276165#comment-8835945
https://www.drupal.org/node/2146435#comment-8889247

ñull’s picture

Status: Reviewed & tested by the community » Needs work

Patch #5 works for me even without using search api to get my pager back.

However, can somebody explain the second correction? If $this->wrapped->view->result is not set, how can it be an array to be sliced?

if (!isset($this->wrapped->view->result)) {
  $this->wrapped->view->result = array_slice($this->wrapped->view->result, $offset, $item_per_page);
}

Also the patch should be relative to the module path. I can upload it correct once somebody answers the previous question.

bdombro’s picture

Patch #5 fixed my pager, and I do not use Search API.

PapaGrande’s picture

Status: Needs work » Needs review
FileSize
1.1 KB

I confirmed that #5 fixes my missing pager, but I rerolled and renamed the patch to conform with Drupal standards in that I made the module directory the base and removed the permissions change.

PapaGrande’s picture

FileSize
1.07 KB

Doh! And now this time I really did remove the permissions change.

lee20’s picture

Status: Needs review » Reviewed & tested by the community

The patch in #26 worked for me. How about that for timing! Thanks PapaGrande!

creeksideplayers’s picture

When will the patch in #26 be released? I'm seeing a problem with the views pager not showing up with fields that use Views PHP, and the patch fixed that problem.

MustangGB’s picture

I'm not using Search API, but my pager disappeared when using Views PHP fields, #26 solved it on 7.x-1.0-alpha1.

yugi’s picture

The patch in #26 worked for me, I am not using Search API as well, but the pager disappeared after adding a Views PHP filter.

aBrookland’s picture

Status: Reviewed & tested by the community » Needs work

The code that #23 asks about doesn't do anything and so the patch in #26 can't leave views_php working as intended. You can't have code that uses $this->wrapped->view->result as an array after checking that $this->wrapped->view->result is not set so I don't think this can RTBC.

As I understand it views_php is attempting to fix pagination in case views_php has been used in the filtering and thus the results from the database aren't fully filtered until views_php has done it's thing. Perhaps views_php can set a flag as to whether it's doing any filtering and needs to sort out the results otherwise just use the already set values as the patch in #26 does.

So the questionable code would become something like:

if ($this->wrapped->view->view_php_filter) {
  $this->wrapped->total_items = count($this->wrapped->view->result);
    $this->wrapped->update_page_info();

    $item_per_page = $this->wrapped->get_items_per_page();
    if ($item_per_page > 0) {
      $offset = $this->wrapped->get_current_page() * $item_per_page + $this->wrapped->get_offset();
      $this->wrapped->view->result = array_slice($this->wrapped->view->result, $offset, $item_per_page);
    }
}

Where $this->wrapped->view->view_php_filter is the flag set by views_php if there is filtering done by views_php.

One piece of information I can bring to the table is that I just came across this issue while updating a site that hadn't been updated since some time in 2012.

After updating views from 7.x-3.5 to 7.x-3.8 this issue started happening, views_php hasn't been updated so is still the same code from back in 2012 but the site didn't suffer from this issue before updating views.

So the code that is being fixed with the patch in this issue didn't affect the pagination with views 7.x-3.5, unfortunately I currently don't have the spare time to try and figure out what's changed in views to cause this code to break pagination but thought I'd add the information in case someone else can figure it out and provide a better fix.

Samfall’s picture

I just commented out the second correction mentioned in #23 and the code still seems to work fine. Maybe it's just not needed. Otherwise, this patch works great for me.

Also, I'm curious if the change in views that causes this problem is also related to sorting. Since the update to the new views, my PHP sorting doesn't work across pages. It only seems to work on the contents of each page. For example, things that should be on the first page show up on the second or third, but everything on the first page is ordered according to my sort values. I generally think this is a related issue, but can't figure out why.

Samfall’s picture

The solution to my sorting problem seems to have been addressed on this page: https://www.drupal.org/node/2276165

I'm using this combined and modified version of the patch in #26 and the one above. This partly addresses @Jan-E 's question in #22.

Since I don't use Search-API I'm curious if this also solves the problem for that.

user3077953’s picture

The patch in #26 seems to work.

The patch in #33, however, has the same problems as the patch in #2276165-6: Pager disappears when Global: PHP used in Views 3.8:
- It will not work when there are more than 999999 results.
- In my view, I have a "Global: PHP" field, which, after applying the patch, gets called for every result, including those that are not displayed on the current page. Considering I have thousands of results, this behavior is not acceptable performance-wise.

@Samfall (#32), the second correction is necessary in my case: if I don't apply it, the first page works, but all the other pages are empty. Also, I don't notice anything wrong with sorting after applying the patch in #26.

user3077953’s picture

I have to admit, though, I don't understand how the second correction

-      $this->wrapped->view->result = array_slice($this->wrapped->view->result, $offset, $item_per_page);
+      if (!isset($this->wrapped->view->result)) {
+        $this->wrapped->view->result = array_slice($this->wrapped->view->result, $offset, $item_per_page);
+      }

is any different from just removing the following line:

      $this->wrapped->view->result = array_slice($this->wrapped->view->result, $offset, $item_per_page);

If the condition (!isset($this->wrapped->view->result) is ever true, the only thing it will do is generate a warning: Warning: array_slice() expects parameter 1 to be array, null given in ...
And I don't think it will ever be true: when there are no elements, $this->wrapped->view->result is an empty array.

In fact, you could even remove the 5 following lines, and it would do the same thing:

    $item_per_page = $this->wrapped->get_items_per_page();
    if ($item_per_page > 0) {
      $offset = $this->wrapped->get_current_page() * $item_per_page + $this->wrapped->get_offset();
      $this->wrapped->view->result = array_slice($this->wrapped->view->result, $offset, $item_per_page);
    }
hotsaucedesign’s picture

I can not get the patch in #26 or #33 to work. We are running Views PHP 1.x-dev as well as Search API 1.13 and when I apply the patch in #26/#33, I get a "502 bad gateway" error.

The host (Pantheon) is not willing to change the nginx.conf file settings so I am not sure what the issue is, since no one else is reporting an error like that. We know that the issue is related to Views PHP because if we remove all PHP fields from the View, the pager returns. Even tried Views Lite Pager and that shows something but the pager is non functional.

Has anyone else experienced the 502 bad gateway error? Is that a permissions/memory thing or something else entirely? I'm assuming that the patch (probably) works but I can't confirm one way or the other. We have ~30,000 records . Any feedback/thoughts would be appreciated.

mikemadison’s picture

If you follow the link to the Search API issue linked in comment #10, that might help you hotsaucedesign.

hotsaucedesign’s picture

lalweil: Thanks for the tips and link, but I unfortunately tried that already. The patch in #10 was already committed to the version of Search API I have so it really had no effect. I'm getting one of our other devs to try to patch it on his local copy to see if its the host or just us but would still appreciate hearing from anyone else who might not be able to get the patch to work.

Anybody’s picture

I can confirm that #26 does NOT work for us. The pager simply returns "0" in combination with views_litepager.module.

As I found out in my case "$this->wrapped->total_items" is zero so the "!isset()" is never triggered for us.

Anybody’s picture

PS: The NORMAL (full) pager works fine, but my edge case shows that the result needs work, because it creates a different behaviour than expected!

Anybody’s picture

Issue tags: +views_litepager
user3077953’s picture

Unfortunately, there is a problem with patch #26: the pager is applied BEFORE any "Global: PHP" conditions listed under "Filter Criteria".

For example, if you have a view with a working pager with, say, 10 items per page, and you add a simple "Global: PHP" condition under "Filter Criteria" such as return rand()%2;, you will notice that your pages do not have 10 items anymore, but only 5, on average.

G.I.Joe’s picture

Because the "views" Module changed since 7.x-3.8.

In the "views" Module, the "views_plugin_query_default" Class replaced the query->range($offset,$limt) from the "execute()" Method to the "result()" Method.
So, In the "views_php" Module, I suppose that the logic in the "post_execute()" Method can be removed.

And I wrote the following patch.

hotsaucedesign’s picture

G.I. Joe,

Your patch came through as a garbled mess (with time stamps and some other junk in there). Could you repost please?

Thanks!

akolahi’s picture

#26 also worked for me. Many thanks!

jaylotta’s picture

#26 caused the pager to reappear when I'm using a Global PHP to filter results, but like #42, I have an issue.

My PHP filter throws out all values below a certain distance threshold so if no items are to be shown, none are returned from the view, but the pager acts like they are all they and shows all the pages that would be there if the filter were removed.

jncruces’s picture

FileSize
1.07 KB

The patch submitted in 26 solves some problems, but is incorrect, the second condition (!isset) is wrong... must be true when isset.

panthar’s picture

Hi everyone,

I also experienced this same issue with the pager going missing, but, there is a more pressing issue for me. The problem is, views php "breaks" when search API has a large index (in my case over 100k nodes).

Took me forever to track this down, but essentially, something about just the php-field's presence in the view, slows down my search API view to the point that php times out. In my example, I literally added no php code, and the view load speed all the sudden quadruples or more just by adding a views php field.

What I think is likely happening, is somewhere in the views php, there has got to be some piece of code that is either trying to do some insane parsing on an array/object from the indexed node(s) that greatly slows down the search API view, or it's executing some php code x 100k times. I can't figure out where/why though.

Anyone have any clue why this would occur? As I was saying, this happens even if I put no php code into the field.

Thanks!

panthar’s picture

Hey All,

I think I found the problem, $this->wrapped->view->query->set_limit(0); inside pre_execute() is problematic for search API and any view with a lot of results. Fixing this would improve this modules performance substantially across the board whenever there is a pager, but particularly with large data sets.

There has got to be some way to not have to query all the results to get the pager info? Seems like other views modules had to of fixed this same problem, might be worth looking at how other modules do it?

Murz’s picture

Patch from #47 shows pager back, but when I go to second page in pager, I see empty results. First page shows results normally.

But patch views_php-pager_missing-2276165-2.patch from #2276165 works well without problems.

wickwood’s picture

I have the same problem as Murz with the Patch from #47. My pager is back, first page shows results normally. I can adjust the number of items to show and the number of pages change. But going to any other page returns empty results message.

vikramy’s picture

Can you please try this patch?

Also I think logic in "post_execute()" method can be removed.

vikramy’s picture

Removing Logic in post_execute() method works for me. Thanks G.I.Joe

vikramy’s picture

skyredwang’s picture

Status: Needs work » Needs review

I tested #53 patch. It works.

skyredwang’s picture

If any patch can remove codes and not add codes and if it works, this patch should win a gold metal.

jordanrussellsmith’s picture

I applied patch #53 but I still have the same issue described in post #51. The pager shows up but any page other than page one doesn't have any content.

danharper’s picture

Is this patch only for version 1.x-dev

Cheers Dan

peterlolty’s picture

(deprecated)

peterlolty’s picture

(deprecated)

peterlolty’s picture

FileSize
774 bytes

(deprecated)

peterlolty’s picture

(deprecated)

peterlolty’s picture

Assigned: mikemadison » peterlolty
Priority: Normal » Critical
FileSize
764 bytes

I have the same problem when I upgrade the views from 7.x-3.7 to 7.x-3.8, but I didn't use Search API.

#53 is half correct, it shouldn't remove the "for(){php_execute();}" part, that's the views_php wrapped to execute the php code.
Finally, Finally , Finally , I made a solution for all (four) use-case.

1. set 'Items to display' and set 'offset' (1 and 1)
2. not set 'Items to display' and set 'offset' (0 and 1)
3. set 'Items to display' and not set 'offset' (1 and 0)
4. not set 'Items to display' and not set 'offset' (0 and 0)

All four use-case should work properly in "Full shown", "specific number of item", "full pager" and "mini pager".

I uploaded the whole /plugins/views/views_php_plugin_pager.inc (7.x-1.0-alpha1) file.
Please feel free to test and review it, or help me to make a (git) patch and share here.
Please use the views_php_plugin_pager.inc_7.x-1.0-alpha1_fix_update3.zip and ignore my previous (deprecated) comments.

rcodina’s picture

@peterlolty Please, provide a patch not just a zipped file to help module mantainer and others to see which are the changed lines. Check out this:

https://www.drupal.org/node/707484

grom358’s picture

Status: Needs review » Needs work

@peterlolty patch #63 doesn't work correctly with PHP filters. Add a filter

return rand() % 2;

and the page doesn't show 10 items. Able to goto Next page

@jncruces patch #47 clicking to goto next page shows no results.

@vikramy patch #53 PHP filters don't work.

peterlolty’s picture

@grom358 which version of "Views" did you used?

peterlolty’s picture

sorry about the zip file not git patch,,, please someone help me to make the patch.

@@ -9,9 +9,9 @@

  /**
   * Perform any needed actions just prior to the query executing.
   */
-  public function pre_execute($query) {
+  public function pre_execute(&$query) {
    $this->wrapped->pre_execute($query);

    foreach (array(/*'argument',*/ 'field', 'filter', 'sort', /*'relationship'*/) as $type) {
      foreach ($this->wrapped->view->$type as $id => $handler) {
@@ -36,16 +36,16 @@
        }
      }
    }

-     $this->wrapped->total_items = count($this->wrapped->view->result);
+/*     $this->wrapped->total_items = count($this->wrapped->view->result);
    $this->wrapped->update_page_info();

    $item_per_page = $this->wrapped->get_items_per_page();
    if ($item_per_page > 0) {
      $offset = $this->wrapped->get_current_page() * $item_per_page + $this->wrapped->get_offset();
      $this->wrapped->view->result = array_slice($this->wrapped->view->result, $offset, $item_per_page);
-    }
+    } */
    $this->wrapped->post_execute($result);
  }


https://www.diffchecker.com/dpj9gs3b

rcodina’s picture

@peterlolty To do a patch, just do this:

1) Make sure you have git installed on your pc/mac

2) Go to "Views PHP" main page: https://www.drupal.org/project/views_php

3) Go to "Version control" green tab

4) Copy "git clone" instruction. If necessary, change "7.x-1.x" for "7.x-2.x" depending on what branch you want to patch.

git clone --branch 7.x-1.x http://git.drupal.org/project/views_php.git

5) Once project is cloned, do your changes to module. Make sure not to delete hidden .git files

6) Once done, open console, go to projects root, and do:

git diff > NAME_OF_PATCH.patch

Note: To get a correct name for patch: in this issue, click on "Files" in the bottom then click on button "Patchname suggestion". Copy the suggestion.

Remember, if you google it, you have this information and even more about other topics just in drupal.org. Thanks for your contribution!.

peterlolty’s picture

FileSize
1.15 KB

@rcodina Thanks for your impressive comment.

rcodina’s picture

Status: Needs work » Needs review

@peterlolty Thanks for the patch!

rakesh.nimje84@gmail.com’s picture

#69 working for me.

klokie’s picture

Status: Needs review » Reviewed & tested by the community

#69 works for me too, thanks!

jlyon’s picture

Fixed the issue for me too

umuthan’s picture

pager showed with the #69 patch but didn't work as expected.

we have total 10 nodes.
we have php filter criteria that returns true or false
we want to show 1 node per page
pager shows 10 page but it should 9 because one node return false with the php filter
every page we saw one node but one page we didnt saw a node only blank page with only pager.

is there any patch for this?

peterlolty’s picture

Status: Reviewed & tested by the community » Needs work
peterlolty’s picture

#74Would you mind explain more about your use case? so I may investegate in deep, thanks

bkno’s picture

#69 patch works for me.

rcodina’s picture

Status: Needs work » Reviewed & tested by the community

@umuthan

pager showed with the #69 patch but didn't work as expected.

we have total 10 nodes.
we have php filter criteria that returns true or false
we want to show 1 node per page
pager shows 10 page but it should 9 because one node return false with the php filter
every page we saw one node but one page we didnt saw a node only blank page with only pager.

is there any patch for this?

Correct me If I'm wrong but your problem isn't related with views php or any patch, you just have to change your views pager settings: While editing a view, check out pager settings in the bottom of central column of your views settings page. Then open pager settings option and select your desired "Items per page" value. Just delete your views php filters.

@peterlolty I think your patch is fine. Given all good reviews I mark it as RTBTC.

rcodina’s picture

Status: Reviewed & tested by the community » Needs work

@peterlolty I found a bug with your patch: In a search api view where I have only a "fulltext search" exposed filter, in cases where filter is empty the paginator doesn't appear and all results are rendered.

DrCord’s picture

@rcodina - instead it should not show any results, as you haven't searched yet, right?

rcodina’s picture

@DrCord Yes, this is an option. The other valid option is that all results are shown but with view's pager. The chosen option may vary depending on customer needs.

zmove’s picture

Status: Needs work » Reviewed & tested by the community

#69 fixed the issue.

MustangGB’s picture

Status: Reviewed & tested by the community » Needs work

Still NW as per #79.

peterlolty’s picture

Let me check!

jaydee1818’s picture

#69 doesn't work for me. I'm a little late to this thread but here is my situation.

I have a page that is outputting a View which is a list of nodes (full content display). This View should only display the first 2 items and then paginate (pagination handled by "Views Infinite Scroll" module). However it's not working.

Within the nodes that are being displayed in the View, there is a field generated by the "Viewfield" module which is generating a list of bookings in a separate View, and it's within this View that Views PHP is being used to alter data. I know for sure that Views PHP is the culprit because if I try disabling it, the infinite scroll pagination works.

peterlolty’s picture

NOT sure if the #86 patch is correct or not, need tester to give feedback.

peterlolty’s picture

Status: Needs work » Needs review
Robert_T’s picture

I tried patch #86 and it did not work, but patch #69 did the trick.

jaydee1818’s picture

Neither patch #69 nor #86 work for me.

alberto56’s picture

FileSize
3.31 KB

Here is a new patch, which combines #69 with an automated test to confirm that it works. (The patch at 86 is not working, I have hidden it.)

alberto56’s picture

Title: Paging vs. Value Field when Integrating with Search API » Pager does not appear if fields use Views PHP

A few more notes:

* I have changed the title, as this does not seem to have anything to do with Search API
* If you use the test in 90, but remove the changes to the plugins/views/views_php_plugin_pager.inc, the test will fail.

alberto56’s picture

FileSize
3.31 KB

Here is the correct version.

bkat’s picture

I'm trying this patch on views-php-7.x-2.x-dev and views-7.x-3.11. No pager at all.

Does there exist a version that works on 7.x-2.x-dev?

alberto56’s picture

@bkat this patch is for 7.x-1.x-dev

bkat’s picture

@alberto56 that's why I'm asking if anyone has fixed this issue on 7.x-2.x-dev. There are a couple of other issues marked for that release but they all point back to this issue.

alberto56’s picture

@bkat: Sorry, I did no mean to appear blunt! You mentioned that you tried this patch on 7.x-2.x-dev, but when I tried to apply it, it applies only partially.

alberto56’s picture

FileSize
3.34 KB

Here is a slightly different version of the same patch. In #92, I had included the test file at the very end of the .info file, which worked on 7.x-1.x-dev, but not on release versions which contain packaging information at the end of the .info file.

In the enclosed version, I have added the reference to the test file before the other includes in the .info file, so that the patch applies whether or not there is packaging information at the end of the .info file.

alberto56’s picture

In comment #10 (December 13, 2013), timkang says: "views_php_plugin_pager::post_execute() cannot simply leave $this->wrapped->total_items as-is, since custom PHP views filters can filter out and reduce the number of total results.".

However, starting with Views 7.x-3.8, released May 20, 2014, the method we were using to get the total number of row, "$this->wrapped->view->result", no longer returns the total number of rows, but rather returns the total number of rows _on the current page_.

My understanding is that views_php filtering for branch 7.x-1.x seems to be broken as of 7.x-3.8; I opened #2484077: The filter function is no longer called for rows not on current page as of Views 7.x-3.8 about that.

There is a fixed issue about filtering for branch 7.x-2.x at #1222448: Views PHP Can't Filter, but I'm not sure if it's related.

In any event, this patch probably breaks PHP filtering for views <= 3.7, but it seems that PHP filtering is broken in views >= 3.8 regardless of whether this patch is used. At least, with this patch, paging is not broken as well.

alberto56’s picture

Status: Needs review » Needs work

Confirming that php_views + this patch + views >= 7.x-3.8 breaks filtering, and will display more total items than php_views + 7.x-3.7. Setting to needs works.

bkat’s picture

I figured out how to get paging to work on 7.x-2.x-dev. It's basically just getting rid of the update_wrapped_pager() method in plugins/views/views_php_plugin_pager.inc

I'll create a separate issue for this.

bkat’s picture

https://www.drupal.org/node/2484407 is where I fixed this on 7.x-2,x-dev

andyanderso’s picture

I used the patch in#92 to patch 7.x-1.0-alpha1 and it worked to get the pager back but it failed on trying to patch the .info file.

patching file plugins/views/views_php_plugin_pager.inc
patching file views_php.info
Hunk #1 FAILED at 14.
1 out of 1 hunk FAILED -- saving rejects to file views_php.info.rej
patching file views_php.test
alberto56’s picture

@andyanderso, please try the patch at #97.

andyanderso’s picture

Patch in #97 worked great. Sorry for me being a ding-bat and getting #92 and #97 mixed up. Thanks for the work on this.

yepa’s picture

Status: Needs work » Needs review

Patch #97 work fine. Thanks

alberto56’s picture

Status: Needs review » Needs work

@yepa thanks. I will put this back to "needs work" for the following reason:

The patch works correctly if you don't have any fields which are removed from your view by php code. For example, if you only have PHP code to modify existing fields.

However, if you have, say, 10 nodes (nids 1 to 10), and 3 nodes per page, but you use PHP code in your view to remove all nodes with even nids (this is a hypothetical example):

* The unpatched version of views_php with views < 7.x-3.8 will work fine: 2 pages, the first with nids 1, 3, 5; the second with 7, 9.
* The unpatched version of views_php with views >= 7.x-3.8 will show only the first page: nodes 1, 3, 5, no pager.
* The patched version of views_php with views >= 7.x-3.8 does not calculate the pages correctly. There would be four page numbers in the pager, because there is no longer any way as of 7.x-3.8 of running the PHP filter code on items not on the current page; this is documented at #2484077: The filter function is no longer called for rows not on current page as of Views 7.x-3.8

peterlolty’s picture

Assigned: peterlolty » Unassigned

I agree with alberto56, it's the common problem for several modules: sub-module(views_php) dont get maintance / update for long time, while the main module(views) have several versions of update. The design of the current views-php miss the current views. We actually need a new group of people to build a whole new design of Views-php for the latest Views

dan.mantyla’s picture

FileSize
613 bytes

It appears that #97 is for views_php-7.x-1.0-alpha1, but I need the dev version so that another (seperate issue) patch will work.

So I tried making a patch of my own. It's attached. It works for me and my dev version of views_php. Sorry if I named it wrong, I don't understand all the numbers in the naming convesion. Also, there's no .info or .testing stuff in this patch file.

There's really only two lines I needed to change.

views_php/plugins/views/views_php-plugins-pager.inc, line #13:

    public function pre_execute(&$query) {

views_php/plugins/views/views_php-plugins-pager.inc, line #50

    //$this->update_wrapped_pager();

So clearly this is not a rear fix, it just bypasses the function that causes the pager to disappear, but this function is needed to calculate the number of pages when views_php is used to filter or sort a view. I'm justing using views_php to create some extra output in my views, so it's no skin off my rump.

Again, this is for the latest dev version. Hope this helps.

alberto56’s picture

@danmantyla thanks for sharing.

I don't understand all the numbers in the naming convesion

If you submit another patch, adding the issue number from the URL (node/2123315) and adding the comment number (for example, in your case it's 108), helps people who download the patch and use it later trace it back to where they found and the discussion around it. A good name for your patch would have been views_php-pager.dev-2123315-108.patch.

rcodina’s picture

@danmantyla On the files section of this page, look at the right side. There is a button "Patchname suggestion". Press it and see ;)

klase’s picture

Can confirm that patch in #97 works to fix issue where Views Infinite Scroll 7.x-1.1 stops working when used together with Views 7.x-3.11 and Views PHP 7.x-1.0-alpha1.

pganore1@gmail.com’s picture

@alberto56, Thanks; This fixes the issue for me.

I will just leave this here.
https://www.drupal.org/node/1016574#comment-9940241

pganore1@gmail.com’s picture

Status: Needs work » Needs review

Can someone confirm that this can be moved to close.

alberto56’s picture

This is still an issue for me, and none of the patches fully work because of a change in the Views >= 7.x-3.8 api.

NaX’s picture

Patch #97 worked for me. To make it work in more scenarios I played with the Idea of checking the Views version to add one more layer of comparability. I don't know if this is something that we should be doing but it seemed to work for me and should also be compatible with Views <= 3.7.

To this I added a function to views_php_plugin_pager.

  function get_views_version() {
    static $version = null;

    if (!is_null($version)) {
      return $version;
    }

    $path = drupal_get_path('module', 'views') . '/views.info';
    $info = drupal_parse_info_file($path);

    $version_string = $info['version'];
    $version_string = str_replace('7.x-', '', $version_string);
    $version_parts = explode('-', $version_string);

    if (empty($version_parts)) {
      $version = FALSE;
      return $version;
    }

    $version_main_parts = explode('.', $version_parts[0]);
    $version = array('major' => $version_main_parts[0], 'minor' => $version_main_parts[1]);
    return $version;
  }

Then alter post_execute()

    $version = $this->get_views_version();
    if ($version['major'] == 3 && $version['minor'] <= 7) {
      $this->wrapped->total_items = count($this->wrapped->view->result);
      $this->wrapped->update_page_info();

      $item_per_page = $this->wrapped->get_items_per_page();
      if ($item_per_page > 0) {
        $offset = $this->wrapped->get_current_page() * $item_per_page + $this->wrapped->get_offset();
        $this->wrapped->view->result = array_slice($this->wrapped->view->result, $offset, $item_per_page);
      }
    }

This does not solve the problem of vews_php fields that modify the number of pager records.

DarrellDuane’s picture

Patch in #97 worked great for me. Thanks.

joake’s picture

#100 worked for me running views_php version 7.x-2.x-dev - thanks

zmove’s picture

Status: Needs review » Reviewed & tested by the community

Patch also work on 2.x version of the module. Need to be commited to all branch.

peterlolty’s picture

Title: Pager does not appear if fields use Views PHP » Pager does not appear if fields use Views PHP (7.x-1.x)
edxxu’s picture

The patch in #97 doesn't work for me with Views PHP 7.x-1.0-alpha1, Views 7.x-3.11, Search API 7.x-1.16.

wylbur’s picture

This is the patch from comment #108, but rerolled to Drupal issue standards. This patches the 7.x-2.x dev release dated 2015-04-21.

rcodina’s picture

@wylbur Which are the differences between patches #108 and #121? Only the patch name? If not, please provide an interdiff.

peterlolty’s picture

Status: Reviewed & tested by the community » Needs review

Everyone who report RTBC please mention which patch (#num) you mentioned.

Sinan Erdem’s picture

The patch on #121 only brings back the pager. The real problem is some of the pages are empty if the result is filtered by Views_PHP. It is like the result rows are there but not displayed.

Anybody’s picture

@#124: Should we split this into separate issues or are they the same?

Patch #121 works great for me and brings back the pager correctly. I'm NOT using views_php for filtering! Just for fields.

NaX’s picture

+1 for splitting issues

peterlolty’s picture

-1 for splitting issues, not support splitting the problem, as we need a general correct solution to commit for fixing the bugs.

NaX’s picture

@peterlolty
I think they are separate issues, the one the pager disappears the other the page number is incorrect when using a filter.
The one problem we have a solution for and it is simple, the second the problem seem very complex and will take some time to fix.

peterlolty’s picture

@NaX , I see your point.
What i mean is, if we want to commit the patch, the fix should be a complete solution, not half nor a workaround. For the people who want the fix at this stage, they may consider any patches shown here which may approach their needs.

fizk’s picture

FileSize
408 bytes

With this patch, I'm able to see the pager, with and without a PHP filter.

What was happening was the range was not being set back to zero by the lines in views_php_plugin_pager::pre_execute():

$this->wrapped->view->query->set_limit(0);
$this->wrapped->view->query->set_offset(0);

To effectively set the limit and offset back to zero, this patch resets the range using hook_views_post_build().

peterlolty’s picture

Status: Needs review » Reviewed & tested by the community

#130 RTBC
with PHP filter and PHP field used inside the views.
Also Views navigation works fine.

  • fizk committed 0ecd0ed on 7.x-1.x
    Issue #2123315 by alberto56, peterlolty, lalweil, vikramy, PapaGrande,...

  • fizk committed 9802819 on 7.x-2.x
    Issue #2123315 by alberto56, peterlolty, lalweil, vikramy, PapaGrande,...
fizk’s picture

Status: Reviewed & tested by the community » Fixed

Committed. Thanks everyone for not giving up on this issue!!

DarrellDuane’s picture

Status: Fixed » Needs work

@fizk and all, thanks for your hard work on this issue.

I can confirm that I am finally getting the correct Record Count (# of records found, using the functionality from the pager) using 7.x-1.x-alpha2. However, I still get the Record Count of all of the records in the view before the views_php filter that I have in my view is applied when I use 7.x-2.x-dev as of 2015-Nov-11 (datestamp = "1447293841"). Let me know if you need more details from my site.

fizk’s picture

@DarrellDuane Can you list steps on creating a new view that would reproduce this on a fresh Drupal install?

fizk’s picture

Status: Needs work » Fixed

@DarrellDuane I think the main part of this bug has been fixed. Can you create a new issue for the specific part you'd like to work on? Thanks,

ckng’s picture

Status: Fixed » Needs work

I'm getting fatal error for code introduced in #130 for all views. Just enabling views_php only, not used in any views yet.

PHP Fatal error:  Call to a member function range() on string in /var/www/sites/drupal/modules/contrib/views_php/views_php.module on line 157
fizk’s picture

@ckng What version of Views and PHP are you using? Also, if you can, please comment out $view->build_info['query']->range(); in views_php.module on line 157 and print out the value of $view->build_info['query'].

SiaoKiax3’s picture

@fizk
The latest version fixed the pager missing issue, but it affected all other views that using " Display a specified number of items " on Pager showing all the results instead of the limited value assigned.
Views 7.x-3.11

DarrellDuane’s picture

@fizk I've created issue https://www.drupal.org/node/2613930 and added instructions about how to replicate the issue.

ckng’s picture

For my case, $view->build_info['query'] is empty.

  • fizk committed 2cd1ba8 on 7.x-1.x
    Add isset() checks for changes from Issue #2123315.
    
fizk’s picture

fizk’s picture

@ckng Let me know if the latest 7.x-1.x dev works for you. It should contain this patch:

https://www.drupal.org/commitlog/commit/17562/2cd1ba8e85e48deccc371e657d...

I'm still not sure why $view->build_info['query'] is empty for you.

  • fizk committed 9681bc9 on 7.x-2.x
    Add isset() checks for changes from Issue #2123315.
    
ckng’s picture

My views is search API solr index type, so technically there are no db query.

The patch should do.

fizk’s picture

Status: Needs work » Fixed

Status: Fixed » Closed (fixed)

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

MustangGB’s picture

This "fix" hits performance/memory so hard.

Fatal error: Allowed memory size exhausted.

To give you some idea of memory usage and load times.

Without patch (pager doesn't work):
10 MB + 8 seconds

With patch (pager works but performance sucks):
535 MB + 15 seconds

With #1937364-1: Filter does not respect pagination, it runs through all nodes in a search (pager works? and performance is great):
10 MB + 7 seconds

MustangGB’s picture

Status: Closed (fixed) » Needs work

In light of #150 I'm thinking we should re-visit this solution.

fizk’s picture

Status: Needs work » Closed (fixed)

Thanks MustangGB, can you create a new issue to fix the performance issue?

simone960’s picture

I am using 7.x-1.0-alpha3 and tested also latest 7.x-1.x-dev but the pager still not showing when views PHP added. Any idea ? Sure this is fixed ?

simone960’s picture

Sorry, it works actually. But if I embed a view from a another view into the current Views, the current view's pager doesn't show up anymore, any idea ?

Output code:

 print views_embed_view('product', 'block',$data->nid);
lanzs’s picture

I can confirm it too, in my case it doesn't work (both – when view is page and when view is block and printed with "views_embed_view").
I tried all current versions: 7.x-1.x-dev, 7.x-2.x-dev and 7.x-1.0-alpha3
I am using views 7.x-3.13

simone960’s picture

I'm using Views Field View module to embed the View at the moment.

abhishek.pareek’s picture

I previously tried this patch. Pager showed but, it produced another problem as view was querying all items irrespective of pager as limit was (0,666666).
After applying #69 patch that problem is also resolved.

peterlolty’s picture

@abhishek.pareek actually the problem has been fixed and commit already in the latest release.

ckng’s picture

@peterlolty, actually problem in #157 is real and still exist.

All items are loaded no matter what with latest dev.
Similar issue: https://www.drupal.org/node/2618504

grahamtk’s picture

So far I cannot use this fix due to heavy performance impact, and had to revert to the alpha 2 release of views php 1 and apply the patch from this thread:
#1937364: Filter does not respect pagination, it runs through all nodes in a search

MustangGB’s picture

@grahamtk: I have the same problem, as requested by fizk I started a follow-up issue for this regression: #2628014: Pager fix causes performance and memory regression