Pagination feature to item history results

[Reimar Bauer, 2011-06-23 08:35:00]

please try to avoid dups in code

some suggestions for cleaning up

http://codereview.appspot.com/4626068/diff/1/MoinMoin/apps/frontend/views.py
File MoinMoin/apps/frontend/views.py (right):

http://codereview.appspot.com/4626068/diff/1/MoinMoin/apps/frontend/views.py#...
MoinMoin/apps/frontend/views.py:543: 
In general: Try to avoid to copy paste nearly the same code from global history.
It should become something we can reuse. At least it should become a function

http://codereview.appspot.com/4626068/diff/1/MoinMoin/apps/frontend/views.py#...
MoinMoin/apps/frontend/views.py:544: offset = request.values.get('offset', 0)
ToDo: may be we should extend get to set the default also for values which is
not in the valid range.

http://codereview.appspot.com/4626068/diff/1/MoinMoin/apps/frontend/views.py#...
MoinMoin/apps/frontend/views.py:551: count = flaskg.user.getCount()
this getCount should get an additional argument to set the default, saves these
condition of the next line.

http://codereview.appspot.com/4626068/diff/1/MoinMoin/apps/frontend/views.py#...
MoinMoin/apps/frontend/views.py:568: if  not nextPage:
PEP8

http://codereview.appspot.com/4626068/diff/1/MoinMoin/apps/frontend/views.py#...
MoinMoin/apps/frontend/views.py:572: previous_offset = offset - count
previous_offset = max(offset - count, 0) ?

[sinha, 2011-06-24 06:24:53]

http://codereview.appspot.com/4626068/diff/1/MoinMoin/apps/frontend/views.py
File MoinMoin/apps/frontend/views.py (right):

http://codereview.appspot.com/4626068/diff/1/MoinMoin/apps/frontend/views.py#...
MoinMoin/apps/frontend/views.py:543: 
On 2011/06/23 08:35:01, Reimar Bauer wrote:
> In general: Try to avoid to copy paste nearly the same code from global
history.
> It should become something we can reuse. At least it should become a function

this paging for item history will be a generic paging, but the one done in
global_history is bit typical, there we are also doing grouping, finding
previous offset etc. Nonetheless we can use this paging as function, but again
in global_history function we may have to do lot of other things to use this
function.Thus leaving the global_history case if i shall have to do similar
thing elsewhere i will definitely use this as a function.

http://codereview.appspot.com/4626068/diff/1/MoinMoin/apps/frontend/views.py#...
MoinMoin/apps/frontend/views.py:544: offset = request.values.get('offset', 0)
On 2011/06/23 08:35:01, Reimar Bauer wrote:
> ToDo: may be we should extend get to set the default also for values which is
> not in the valid range.

How ? i mean where is its implementation ?

http://codereview.appspot.com/4626068/diff/1/MoinMoin/apps/frontend/views.py#...
MoinMoin/apps/frontend/views.py:551: count = flaskg.user.getCount()
On 2011/06/23 08:35:01, Reimar Bauer wrote:
> this getCount should get an additional argument to set the default, saves
these
> condition of the next line.

this function has been altered, you can confirm it here
http://codereview.appspot.com/4635066

[Reimar Bauer, 2011-06-24 08:33:30]

answered

http://codereview.appspot.com/4626068/diff/1/MoinMoin/apps/frontend/views.py
File MoinMoin/apps/frontend/views.py (right):

http://codereview.appspot.com/4626068/diff/1/MoinMoin/apps/frontend/views.py#...
MoinMoin/apps/frontend/views.py:544: offset = request.values.get('offset', 0)
On 2011/06/24 06:24:53, sinha wrote:
> On 2011/06/23 08:35:01, Reimar Bauer wrote:
> > ToDo: may be we should extend get to set the default also for values which
is
> > not in the valid range.
> 
> How ? i mean where is its implementation ?

Well it is in werkzeug datstructure, you can add a feature request to the issue
tracker.

http://codereview.appspot.com/4626068/diff/1/MoinMoin/apps/frontend/views.py#...
MoinMoin/apps/frontend/views.py:551: count = flaskg.user.getCount()
On 2011/06/24 06:24:53, sinha wrote:
> On 2011/06/23 08:35:01, Reimar Bauer wrote:
> > this getCount should get an additional argument to set the default, saves
> these
> > condition of the next line.
> 
> this function has been altered, you can confirm it here
> http://codereview.appspot.com/4635066

I reviewed it