Items per page cannot be overridden without first overriding pager (regression?) [#1144740]

Also reported by lrobeson, http://drupal.org/node/1110180#comment-4375962

Related to #652712: Pager settings are not stored. In that issue it looks like pager settings were fixed on a per display basis, and I do not see any changes to the code although I haven't looked that in-depth. As an unrelated issue, tests written for pagers are broken now as well.

I'll try to look for a git commit that regressed this as it was reported working by Dereine in the issue noted above.

Steps to reproduce

Create a new view.
Create a couple of block displays.
Try to override the number of items per page for each display. It will change the option for every display instead of just for the overridden display.

Also, I tried modifying a view template with the items, and it views imports it with the same items per page regardless (although that template may be incorrectly formatted). In other words, I exported the example view, try to add those lines to the display, re-import, but that does not work either.

Attached a view to try out. I can't take screenshots well on this windows box (ugh) so I don't have any. Issue confirmed on two different installations by two different people Today as well as the note by lrobeson.

Comment	File	Size	Author
#12	1144740-pager-defaultable-sections_0.patch	3.15 KB	dawehner
#12
#10	1144740-pager-defaultable-sections.patch	1.92 KB	merlinofchaos
#10
#9	1144740-pager-defaultable-sections.patch	1.48 KB	merlinofchaos
#9
#8	1144740.patch	1.94 KB	dawehner
#8
#6	1144740-fix.patch	2.12 KB	dawehner
#6
	pager-items-view-example-clean.txt	8.01 KB	mradcliffe

Support from Acquia helps fund testing for Drupal Acquia logo

Comments

Comment #1

dawehner

German

CreditAttribution: dawehner commented 2 May 2011 at 20:22

Sadly i can reproduce this.

Comment #2

fandb CreditAttribution: fandb commented 5 May 2011 at 02:16

I can reproduce this as well.
Although References display appears to work properly.

Comment #3

mradcliffe

he/him

English

CreditAttribution: mradcliffe commented 5 May 2011 at 13:49

Title:

Pager items: Items per page cannot be overridden (regression)

» Items per page cannot be overridden without first overriding pager (regression?)

After undergoing some more manual testing I was able to override pager items only when I first overrode the pager type. (Long post incoming)

This makes sense with the following work flow:

#1

Set pager Full.
Set number of items to N.
Switch to display X.
Override pager Full for display X.
Set number of items to M for display X.

But not with this one:

#2

Set pager Full.
Set number of items to N.
Switch to display X.
Override number of items to M for display X.

Confusingly, you can override number of items without first overriding the pager if you have a cloned display where the pager is already overridden.

#3

Set pager Full.
Set number of items to N.
Switch to display X.
Override pager Full for display X.
Override number of items to M for display X.
Clone display X as display Y.
Override number of items to L for display Y.

But if you clone a display that has not already overridden the pager, it works as above in #2.

So the question becomes:
Should overriding the settings for a pager (and other plugins?) override the pager itself, or should the work flow in #1 and #3 be a part of Views use documentation?

Edit: This makes sense to me as-is, but it is confusing if you come at making views from #2. I think that stronger docs or explanation as part of overriding would help.

Comment #4

dawehner

German

CreditAttribution: dawehner commented 5 May 2011 at 16:00

You can only override the pager itself, because it's the only way to be sure that the options mean what you mean in your pager plugin and the options are attached to the plugin.

Comment #5

OldAccount CreditAttribution: OldAccount commented 9 May 2011 at 19:59

Subscribing

Comment #6

dawehner

German

CreditAttribution: dawehner commented 12 May 2011 at 21:08

Assigned:

Unassigned

» merlinofchaos

File	Size
1144740-fix.patch	2.12 KB

Some stuff:

* items_per_page, offset still exist in quite some places in the code. Here is a patch for it.

In general i have no idea how this defaultable_section etc. stuff works.
Earl please here here.

Comment #7

geek-merlin

they/omg

German

Freiburg, Germany

CreditAttribution: geek-merlin commented 13 May 2011 at 11:59

confirming and subscribing

Comment #8

dawehner

German

CreditAttribution: dawehner commented 1 June 2011 at 19:43

Status:

Active

» Needs review

File	Size
1144740.patch	1.94 KB

Update status and bump for merlinofchaos. It's really hard for me here.

Here is the new version

Comment #9

merlinofchaos CreditAttribution: merlinofchaos commented 1 June 2011 at 19:51

File	Size
1144740-pager-defaultable-sections.patch	1.48 KB

Try this patch and see if it works

Comment #10

merlinofchaos CreditAttribution: merlinofchaos commented 1 June 2011 at 20:00

File	Size
1144740-pager-defaultable-sections.patch	1.92 KB

Try this:

Comment #11

mradcliffe

he/him

English

CreditAttribution: mradcliffe commented 1 June 2011 at 23:57

I am a little crunched for time at the moment, but I will try when I can myself.

Comment #12

dawehner

German

CreditAttribution: dawehner commented 2 June 2011 at 06:49

File	Size
1144740-pager-defaultable-sections_0.patch	3.15 KB

Okay, fixed the override button by reverting another commit.

Now this patch works fine. Added the pattern to access and cache plugin and removed the items_per_page setting again.

Comment #13

steinmb CreditAttribution: steinmb commented 7 June 2011 at 15:27

The patch seems sound and safe, but I'm a bit out of my depth when it comes to Views code base. From a usability point of view (ehem...) does this now make sense, and pager override work just fine on our installations.

Thanks :)
Stein

Comment #14

dawehner

German

CreditAttribution: dawehner commented 7 June 2011 at 17:25

Status:

Needs review

» Needs work

+++ b/plugins/views_plugin_display.incundefined
@@ -427,16 +427,6 @@ class views_plugin_display extends views_plugin {
       ),
-      'items_per_page' => array(
-        'default' => 10,
-      ),
-      'offset' => array(
-        'default' => 0,
-      ),
-      'use_pager' => array(
-        'default' => FALSE,
-        'bool' => TRUE,

This shouldn't be removed, because this would bork the automatic update from views2

Powered by Dreditor.