Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Use an own paginator #646

Merged
merged 10 commits into from Jun 12, 2017
30 changes: 30 additions & 0 deletions src/wiki/core/wiki_paginate.py
@@ -0,0 +1,30 @@
from django.core.paginator import Paginator

class WikiPaginator(Paginator):

def __init__(self, *args, **kwargs):
"""
:param side_pages: How many pages should be shown before and after the current page
"""
self.side_pages = kwargs.pop('side_pages', 4)
super(WikiPaginator, self).__init__(*args, **kwargs)

def _get_page(self, *args, **kwargs):
self.curPage = super(WikiPaginator, self)._get_page(*args, **kwargs)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This assignment contract is too implicit. It means that page_range is broken if _get_page hasn't been called. Isn't there a way to just call this method directly from page_range ?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@benjaoming Calling _get_page() from page_range() unfortunately does not work.
Source from the django paginator (django/core/paginator.py):

class Paginator(object):
    def _get_page(self, *args, **kwargs):
        """
        Returns an instance of a single page. This hook can be used by
        subclasses to use an alternative to the standard :cls:`Page` object.
        """
        return Page(*args, **kwargs)

class Page(collections.Sequence):
    def __init__(self, object_list, number, paginator):
        self.object_list = object_list
        self.number = number
        self.paginator = paginator

I.e. the page number is required. And that is exactly what is needed here in the
first place. Based on that I would say that it is guaranteed that _get_page()
is called before page_range().

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I tried reordering this a bit

  1. We don't overwrite a weak internal method (the single underscore denotes to avoid this)
  2. renamed curPage to something with underscores, camelCase isn't used in Python normally :)

It's in floemker#4

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

More efficient is always good (int instead of a Page object). Committed.

return self.curPage

@property
def page_range(self):
left = max(self.curPage.number - self.side_pages, 2)
right = min(self.curPage.number + self.side_pages+1, self.num_pages)
pages = []
if self.num_pages > 0:
pages = [1]
if left > 2:
pages += [0]
pages += range(left, right)
if right < self.num_pages:
pages += [0]
if self.num_pages > 1:
pages += [self.num_pages]
return pages
2 changes: 2 additions & 0 deletions src/wiki/plugins/attachments/views.py
Expand Up @@ -12,6 +12,7 @@
from django.views.generic.edit import FormView
from django.views.generic.list import ListView
from wiki.core.http import send_file
from wiki.core.wiki_paginate import WikiPaginator
from wiki.decorators import get_article, response_forbidden
from wiki.plugins.attachments import forms, models, settings
from wiki.views.mixins import ArticleMixin
Expand Down Expand Up @@ -440,6 +441,7 @@ class AttachmentSearchView(ArticleMixin, ListView):
template_name = "wiki/plugins/attachments/search.html"
allow_empty = True
context_object_name = 'attachments'
paginator_class = WikiPaginator
paginate_by = 10

@method_decorator(get_article(can_write=True))
Expand Down
2 changes: 2 additions & 0 deletions src/wiki/plugins/globalhistory/views.py
Expand Up @@ -5,11 +5,13 @@
from django.utils.decorators import method_decorator
from django.views.generic import ListView
from wiki import models
from wiki.core.wiki_paginate import WikiPaginator


class GlobalHistory(ListView):

template_name = 'wiki/plugins/globalhistory/globalhistory.html'
paginator_class = WikiPaginator
paginate_by = 30
model = models.ArticleRevision
context_object_name = 'revisions'
Expand Down
2 changes: 2 additions & 0 deletions src/wiki/plugins/images/views.py
Expand Up @@ -11,6 +11,7 @@
from django.views.generic.edit import FormView
from django.views.generic.list import ListView
from wiki.conf import settings as wiki_settings
from wiki.core.wiki_paginate import WikiPaginator
from wiki.decorators import get_article
from wiki.models.pluginbase import RevisionPluginRevision
from wiki.plugins.images import forms, models
Expand All @@ -24,6 +25,7 @@ class ImageView(ArticleMixin, ListView):
template_name = 'wiki/plugins/images/index.html'
allow_empty = True
context_object_name = 'images'
paginator_class = WikiPaginator
paginate_by = 10

@method_decorator(get_article(can_read=True, not_locked=True))
Expand Down
18 changes: 17 additions & 1 deletion src/wiki/templates/wiki/includes/pagination.html
@@ -1,8 +1,24 @@
{% load i18n %}
{% if is_paginated %}
<ul class="pagination">
{% if page_obj.has_previous %}
<li><a class="prev btn btn-info" href="?{% if search_query %}q={{ search_query }}&{% endif %}page={{ page_obj.previous_page_number }}{% if appended_key %}&{{ appended_key }}={{ appended_value }}{% endif %}">&laquo;</a></li>
{% else %}
<li class="disabled"><span>&laquo;</span></li>
{% endif %}

{% for pc in paginator.page_range %}
<li class="{% if pc == page_obj.number %} active{% endif %}"><a href="?{% if search_query %}q={{ search_query }}&{% endif %}page={{ pc }}{% if appended_key %}&{{ appended_key }}={{ appended_value }}{% endif %}">{{ pc }}</a></li>
{% if pc == 0 %}
<li class="disabled"><span>...</span></li>
{% else %}
<li class="{% if pc == page_obj.number %} active{% endif %}"><a href="?{% if search_query %}q={{ search_query }}&{% endif %}page={{ pc }}{% if appended_key %}&{{ appended_key }}={{ appended_value }}{% endif %}">{{ pc }}</a></li>
{% endif %}
{% endfor %}

{% if page_obj.has_next %}
<li><a class="next btn btn-info" href="?{% if search_query %}q={{ search_query }}&{% endif %}page={{ page_obj.next_page_number }}{% if appended_key %}&{{ appended_key }}={{ appended_value }}{% endif %}">&raquo;</a></li>
{% else %}
<li class="disabled"><span>&raquo;</span></li>
{% endif %}
</ul>
{% endif %}
4 changes: 4 additions & 0 deletions src/wiki/views/article.py
Expand Up @@ -24,6 +24,7 @@
from wiki.core.exceptions import NoRootURL
from wiki.core.plugins import registry as plugin_registry
from wiki.core.utils import object_to_json_response
from wiki.core.wiki_paginate import WikiPaginator
from wiki.decorators import get_article
from wiki.views.mixins import ArticleMixin

Expand Down Expand Up @@ -608,6 +609,7 @@ class History(ListView, ArticleMixin):
template_name = "wiki/history.html"
allow_empty = True
context_object_name = 'revisions'
paginator_class = WikiPaginator
paginate_by = 10

def get_queryset(self):
Expand All @@ -634,6 +636,7 @@ class Dir(ListView, ArticleMixin):
allow_empty = True
context_object_name = 'directory'
model = models.URLPath
paginator_class = WikiPaginator
paginate_by = 30

@method_decorator(get_article(can_read=True))
Expand Down Expand Up @@ -678,6 +681,7 @@ def get_context_data(self, **kwargs):
class SearchView(ListView):

template_name = "wiki/search.html"
paginator_class = WikiPaginator
paginate_by = 25
context_object_name = "articles"

Expand Down
39 changes: 39 additions & 0 deletions tests/core/test_paginator.py
@@ -0,0 +1,39 @@
from django.test import TestCase
from wiki.core.wiki_paginate import WikiPaginator


class PaginatorTest(TestCase):
"""
Test the WikiPaginator and it's page_range() function
"""

def test_paginator(self):
objects = [1]
p = WikiPaginator(objects, 2, side_pages=2)
self.assertEqual(p.num_pages, 1)

p.page(1)
self.assertEqual(p.page_range, [1])

objects = [1, 2, 3, 4, 5, 6, 7, 8, 9]
p = WikiPaginator(objects, 2, side_pages=2)
self.assertEqual(p.num_pages, 5)

p.page(1)
self.assertEqual(p.page_range, [1, 2, 3, 0, 5])

p.page(3)
self.assertEqual(p.page_range, [1, 2, 3, 4, 5])

objects = [1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11, 12, 13, 14, 15, 16, 17]
p = WikiPaginator(objects, 2, side_pages=2)
self.assertEqual(p.num_pages, 9)

p.page(1)
self.assertEqual(p.page_range, [1, 2, 3, 0, 9])

p.page(5)
self.assertEqual(p.page_range, [1, 0, 3, 4, 5, 6, 7, 0, 9])

p.page(8)
self.assertEqual(p.page_range, [1, 0, 6, 7, 8, 9])