Allow specifying a pagesize=-1 which will allow 'request/batch' to
return all results, instead of picking a "random high number" that might
achieve the same thing (unreliably).

I like the idea.
(Because I do not remember another method of doing so.)

I propose to use pagesize=0, because it is easier to type  (and maybe
even more intuitive, but I only asked my own intuition) and leaves -n
for future expansions, e.g. -20 for pagesize 20 and reverse order of
pages or whatever.

I was having a bit of a think about this on the weekend and came the the
conclusion that pagesize=0 would also probably be a better fit.

I have attached a second page that reflects this change.


I have also made a couple of changes to the Batch class. 'self.size' was
being set to the 'pagesize' specified and not being used again anywhere.
'self._size' was being set to the actual size being calculated, which
meant that the previous and next links where using a hard-coded
'pagesize=##' in the url (when the actual size may have changed in the
mean time).

I have removed 'self._size', and set 'self.size' to be assigned the
'pagesize' value. The means the the correct 'pagesize=##' will be set
(even if it is 0). One issue might be if someone is using 'batch/size'
in their template for some reason. Maybe they should use 'batch/length'
instead.

John,


Functionally, the patch works as expected. But I've the following
question and comment:

In your most recent update ( msg4190 ) to this issue, you said:

"'self._size' was being set to the actual size being calculated, which
meant that the previous and next links where using a hard-coded
'pagesize=##' in the url (when the actual size may have changed in the
mean time)."

When you say mean time, do you mean after a search query has been saved?
Could you elaborate on that point (perhaps with a use-case for such a
scenario?)

Also, there is three places in Batch.py where "_size" is referred to
(lines: 21, 22, 25). For completeness these references should be with
"size" as well. Can you upload a new patch with these changes?

Noticed the word "replaced" was missing in my last comment. The last
paragraph in msg4349 should read:

Also, there are three places in Batch.py where "_size" is still referred
to (lines: 21, 22, 25). For completeness these references should be
replaced with "size" as well. Can you upload a new patch with these changes?

It was a while ago since I looked a this, so my memory is a bit hazy...
I believe what I mean is something like the following scenario:

* assume there are 10 issues in the tracker
* I am looking at a page that contains next/previous links containing
'pagesize=10'
* While I am looking at this page, someone else creates a new issue
(meaning 11 issues in total)
* I then click on a next/previous link, the 'pagesize=10' value is
propogated to the following page that loads and any next/previous links
loaded on the 'following' page still contain 'pagesize=10' instead of
'pagesize=11' (which is the new total).

I just realised that for most people this might be confusing as there
probably won't be any next/previous links (because it is showing all
possible issues already). I have set up some custom views/reports where
the next/previous links actually mean next/previous month (not pagination).

The example of a saved query (like you mentioned) would  indeed seem to
be another instance where the inability to distinguish between 'all
issues' and '# issues' would be an issue.

I still may not be explaining myself clearly, so feel free to query me
further.


Also, thanks for picking up on the errant '_size' references. I'll sort
another patch to fix these.

3rd attempt at the patch (which fixes the errant _size references) is
attached

Thanks John for the new patch and the comments.
Satchit: Did John address all the issue you had?
Again, thanks for both of your feedback!

Bernhard: I had a brief look at John's most recent patch and response, soon after he posted it. But I couldn't look at it in detail because of some work here. I'll look at it and respond shortly.

Am Donnerstag, 11. August 2011 15:21:50 schrieb satchit:
> I'll look at it and respond shortly.

Cool! I'm looking forward to it.

The pagesize=0 component of the patch works as expected.

Regarding the replacement of the "_size" with "size", I couldn't find a way to reproduce different behaviors before and after applying the path. That said, I'll note that:

- I didn't notice any breakage because of this change and all tests pass.

- But I couldn't find a way to test the effect of this specific component in the patch.

John: 

Since there aren't any unit-tests for this code, can you provide a series of steps that I can follow in the demo instance to reproduce an issue that is fixed by the "_size" component of your patch?

If the above is not easy, then I would suggest the following: 

If the "_size" to "size" change is not needed for the "pagesize=0" component to work, break these into two separate patches, so that at least the "pagesize=0" component, which is a very useful feature, can be merged into the trunk.

Sorry this has taken so long to get back to, it has been a busy few
weeks at work.

satchit: thanks for the suggestion about breaking the patch up into two
parts, it made me realise I was probably tackling the two issues in the
reverse order (and not necessarily coming up with the best solution).

I created (and attached) a newer and much simpler patch (v4).

I have also create a separate issue for the @pagesize propagation stuff
(issue2550723) which should make that issue a bit easier to follow.

On Friday 02 September 2011 07:03:59 John Kristensen wrote:
> I created (and attached) a newer and much simpler patch (v4).
>
> I have also create a separate issue for the @pagesize propagation stuff
> (issue2550723) which should make that issue a bit easier to follow.

John, thanks for following up on this!

On Sep 2, 2011, at 3:16 AM, Bernhard Reiter wrote:

> 
> Bernhard Reiter <bernhard@intevation.de> added the comment:
> 
> On Friday 02 September 2011 07:03:59 John Kristensen wrote:
>> I created (and attached) a newer and much simpler patch (v4).
>> 
>> I have also create a separate issue for the @pagesize propagation stuff
>> (issue2550723) which should make that issue a bit easier to follow.
> 
> John, thanks for following up on this!

John: Thanks for the new patch.

Bernhard: I've reviewed the latest patch and it's good to go. 

Satchit

Thanks Satchit!

Commited with rev4645 .
Thanks again!