Steps to replicate:

Install Views 3
Install CaseTracker

Try to edit via Views UI a view created from CaseTracker. Or access to a www.example.com/casetracker

I got this error:

Call to a member function init_pager() on a non-object in views/includes/view.inc on line 119

This is probably a bug in Views and not in CaseTracker, because the views should be editable even from the Views UI.

Also users cannot access to the view, they get the same fatal error.

CommentFileSizeAuthor
#24 views-698270-D6.patch4.17 KBdagmar
#24 views-698270-D7.patch721 bytesdagmar
#23 views-698270_1.patch3.67 KBinfojunkie
#19 views-698270.patch3.69 KBdagmar
#14 698270-ease-pager-override-settings.patch3.69 KBdawehner
#11 698270-ease-pager-override-settings.patch3.7 KBdawehner
#10 698270-ease-pager-override-settings.patch5.8 KBmerlinofchaos
#4 views-698270-1.patch624 bytesdagmar
#2 views-698270.patch709 bytesdagmar
Support from Acquia helps fund testing for Drupal Acquia logo

Comments

dawehner’s picture

dawehner
Primary language German
CreditAttribution: dawehner commented

You can also reproduce the error by using

<?php
$view = views_get_view('frontpage');
$view->set_display('default');
// thats the bad line
$view->set_items_per_page(1);
print $view->preview();
?>
dagmar’s picture

dagmar
Primary language Spanish
Location Argentina
CreditAttribution: dagmar commented
Status: Active » Needs review
FileSize
views-698270.patch709 bytes

This issue is a consequence of several changes in the pager system for views 3:

See this links to trace how have been modified the set_items_per_page() function

http://drupalcode.org/viewvc/drupal/contributions/modules/views/includes...
http://drupalcode.org/viewvc/drupal/contributions/modules/views/includes...
http://drupalcode.org/viewvc/drupal/contributions/modules/views/plugins/...

In views 2, this function could be used to define if a views should use a pager or display all items. With the implementation of pluggable pagers, this behaviour was delegated in the plugins.

Case Tracker is calling to set_items_per_page()

egrep -unr 'set_items_per_page' .
./casetracker.module:879:    $view->set_items_per_page(0);
./casetracker.module:896:    $view->set_items_per_page(0);

And dereine php script also call directly to this function.

The problem is not really a bug of views, using Views UI, all is configured correctly to call set_items_per_page after $this->query is initialized.

Probably the best solution is to not call directly to set_items_per_page(), instead of that, modules should define 'none' as the pager plugin.

However this introduces a unnecessary backward compatibly between views 2 and 3.

So, I'm attaching a patch. What do you think about this solution?

dawehner’s picture

dawehner
Primary language German
CreditAttribution: dawehner commented

One example:

$view = views_get_view('frontpage');
$view->set_display('default');
// thats the bad line
$view->set_items_per_page(1);
$view->set_items_per_page(0);
print $view->preview();

Doesn't work as expected. I think we have to update $view->query->pager, too

dagmar’s picture

dagmar
Primary language Spanish
Location Argentina
CreditAttribution: dagmar commented
FileSize
views-698270-1.patch624 bytes

New patch

dawehner’s picture

dawehner
Primary language German
CreditAttribution: dawehner commented

I think we would need also it in $view->get_items_per_page $view->get_offset etc.

I think we should move up the initialization of the pager to $view::init_display. Then its definitive ready when it has to be there.

Additional this would later allow to attach settings to the query plugin, which is cool :)

merlinofchaos’s picture

merlinofchaos CreditAttribution: merlinofchaos commented

I think I didn't do that because I wanted to retain the ability for the style plugin to change pagers. But maybe that's not really that big of a deal.

infojunkie’s picture

infojunkie
Location Vancouver
CreditAttribution: infojunkie commented

Latest patch (#4) does not work for me as intended. I get Fatal error: Call to a member function get_plugin() on a non-object in /home/kratib/www/d6/sites/all/modules/views/plugins/views_plugin_query.inc on line 83.

My code looks like this:

<?php
$view = views_get_view($vid);
$view->set_items_per_page(0);
?>

and that's how it used to work in Views 2.x. Seems that we now need to call $view->set_display($display_id); before calling set_items_per_page.

merlinofchaos’s picture

merlinofchaos CreditAttribution: merlinofchaos commented

This may be an API change you're going to have to live with.

merlinofchaos’s picture

merlinofchaos CreditAttribution: merlinofchaos commented
FileSize
698270-ease-pager-override-settings.patch5.8 KB

I think this patch ought to cover what we need. Can I get a test?

dawehner’s picture

dawehner
Primary language German
CreditAttribution: dawehner commented
FileSize
698270-ease-pager-override-settings.patch3.7 KB

Mh

Fatal error: Call to a member function use_pager() on a non-object in /var/www/development/sites/views.dev/modules/views/includes/view.inc on line 357

I corrected this.

The tests are running fine.

Manual testing worked fine too.

dawehner’s picture

dawehner
Primary language German
CreditAttribution: dawehner commented

The problem was:

+
+      if ($this->pager->use_pager()) {

I changed this to

+
+      if ($this->display_handler->use_pager()) {
merlinofchaos’s picture

merlinofchaos CreditAttribution: merlinofchaos commented

I think that should be $this->query->use_pager()

That test is to see if the pager is actually paging. (For example, the simplest pager simply limits the number of records. It is a pager in the very strictest sense, and does not use the actual paging functionality).

Perhaps the function is misnamed?

dawehner’s picture

dawehner
Primary language German
CreditAttribution: dawehner commented
FileSize
698270-ease-pager-override-settings.patch3.69 KB

it should be $this->query->pager->use_pager

delykj’s picture

delykj CreditAttribution: delykj commented

Thanks, dereine, you saved my day. #14 works perfectly for me.

dawehner’s picture

dawehner
Primary language German
CreditAttribution: dawehner commented

Was this a rtbc?

dagmar’s picture

dagmar
Primary language Spanish
Location Argentina
CreditAttribution: dagmar commented 
+++ includes/view.inc
@@ -329,6 +345,32 @@ class view extends views_db_object {
+        $this->query->pager->set_current_page($view->current_page);

$view is visible at this point?

Or should be $this->current_page ?

dawehner’s picture

dawehner
Primary language German
CreditAttribution: dawehner commented
Status: Needs review » Needs work

Update

dagmar’s picture

dagmar
Primary language Spanish
Location Argentina
CreditAttribution: dagmar commented
Title: Pager is not correctly initialized when views are loaded from files - CaseTracker doesn't with Views 3 » Pluggable pager legacy functions: set_item_per_page() and set_offset()
Status: Needs work » Reviewed & tested by the community
FileSize
views-698270.patch3.69 KB

Ok, I tested the patch and works fine.

This file includes the fix reported in #17.

merlinofchaos’s picture

merlinofchaos CreditAttribution: merlinofchaos commented
Status: Reviewed & tested by the community » Fixed

Committed to 6.x and 7.x

Status: Fixed » Closed (fixed)

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

dagmar’s picture

dagmar
Primary language Spanish
Location Argentina
CreditAttribution: dagmar commented
Status: Closed (fixed) » Reviewed & tested by the community

I'm afraid this only was committed to DRUPAL-7--3, see http://drupal.org/cvs?file=/modules/views/plugins/views_plugin_query_def...

The patch still applies to DRUPAL-6--3.

infojunkie’s picture

infojunkie
Location Vancouver
CreditAttribution: infojunkie commented
Status: Reviewed & tested by the community » Needs review
FileSize
views-698270_1.patch3.67 KB

The latest patch has a bug in that set_offset is called on the pager object, instead of the query object, resulting in a method not found error. The attached patch fixes this.

dagmar’s picture

dagmar
Primary language Spanish
Location Argentina
CreditAttribution: dagmar commented
FileSize
views-698270-D7.patch721 bytes
views-698270-D6.patch4.17 KB

Nice catch. However, I think the bug is set_offset should be defined in views_plugin_pager in the same way that set_items_per_page is defined.

This patch is similar to #19 but includes a set_offset definition in views_plugin_pager.

I'm also attaching the D7 patch.

dawehner’s picture

dawehner
Primary language German
CreditAttribution: dawehner commented
Status: Needs review » Reviewed & tested by the community

I just review set_offset d7 patch. The rest of the d6 patch was already rtbc.

Kudos for creating a seperate d7 patch.

infojunkie’s picture

infojunkie
Location Vancouver
CreditAttribution: infojunkie commented

Latest patch works for me.

merlinofchaos’s picture

merlinofchaos CreditAttribution: merlinofchaos commented
Status: Reviewed & tested by the community » Fixed

Ok. Really, this one is committed this time.

Status: Fixed » Closed (fixed)

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

cpotter’s picture

cpotter CreditAttribution: cpotter commented
Version: 6.x-3.x-dev » 6.x-3.0-alpha2

It seems that this patch has not been applied to 3.0-alpha2