Merge pull request #3229 from minrk/configurable-pagination

make pagination configurable

jupyterhub/app.py

@@ -77,6 +77,7 @@ from .user import UserDict
 from .oauth.provider import make_provider
 from ._data import DATA_FILES_PATH
 from .log import CoroutineLogFormatter, log_request
+from .pagination import Pagination
 from .proxy import Proxy, ConfigurableHTTPProxy
 from .traitlets import URLPrefix, Command, EntryPointType, Callable
 from .utils import (
@@ -279,7 +280,7 @@ class JupyterHub(Application):
 
     @default('classes')
     def _load_classes(self):
-        classes = [Spawner, Authenticator, CryptKeeper]
+        classes = [Spawner, Authenticator, CryptKeeper, Pagination]
         for name, trait in self.traits(config=True).items():
             # load entry point groups into configurable class list
             # so that they show up in config files, etc.

jupyterhub/handlers/pages.py

@@ -453,7 +453,7 @@ class AdminHandler(BaseHandler):
     @web.authenticated
     @admin_only
     async def get(self):
-        page, per_page, offset = Pagination.get_page_args(self)
+        page, per_page, offset = Pagination(config=self.config).get_page_args(self)
 
         available = {'name', 'admin', 'running', 'last_activity'}
         default_sort = ['admin', 'name']
@@ -511,7 +511,11 @@ class AdminHandler(BaseHandler):
 
         total = self.db.query(orm.User.id).count()
         pagination = Pagination(
-            url=self.request.uri, total=total, page=page, per_page=per_page,
+            url=self.request.uri,
+            total=total,
+            page=page,
+            per_page=per_page,
+            config=self.config,
         )
 
         auth_state = await self.current_user.get_auth_state()

jupyterhub/pagination.py

@@ -1,69 +1,94 @@
 """Basic class to manage pagination utils."""
 # Copyright (c) Jupyter Development Team.
 # Distributed under the terms of the Modified BSD License.
+from traitlets import Bool
+from traitlets import default
+from traitlets import Integer
+from traitlets import observe
+from traitlets import Unicode
+from traitlets import validate
+from traitlets.config import Configurable
 
 
-class Pagination:
+class Pagination(Configurable):
 
-    _page_name = 'page'
-    _per_page_name = 'per_page'
-    _default_page = 1
-    _default_per_page = 100
-    _max_per_page = 250
+    # configurable options
+    default_per_page = Integer(
+        100,
+        config=True,
+        help="Default number of entries per page for paginated results.",
+    )
 
-    def __init__(self, *args, **kwargs):
-        """Potential parameters.
-        **url**: URL in request
-        **page**: current page in use
-        **per_page**: number of records to display in the page. By default 100
-        **total**: total records considered while paginating
-        """
-        self.page = kwargs.get(self._page_name, 1)
+    max_per_page = Integer(
+        250,
+        config=True,
+        help="Maximum number of entries per page for paginated results.",
+    )
 
-        if self.per_page > self._max_per_page:
-            self.per_page = self._max_per_page
+    # state variables
+    url = Unicode("")
+    page = Integer(1)
+    per_page = Integer(1, min=1)
 
-        self.total = int(kwargs.get('total', 0))
-        self.url = kwargs.get('url') or self.get_url()
-        self.init_values()
+    @default("per_page")
+    def _default_per_page(self):
+        return self.default_per_page
 
-    def init_values(self):
-        self._cached = {}
-        self.skip = (self.page - 1) * self.per_page
-        pages = divmod(self.total, self.per_page)
-        self.total_pages = pages[0] + 1 if pages[1] else pages[0]
+    @validate("per_page")
+    def _limit_per_page(self, proposal):
+        if self.max_per_page and proposal.value > self.max_per_page:
+            return self.max_per_page
+        if proposal.value <= 1:
+            return 1
+        return proposal.value
 
-        self.has_prev = self.page > 1
-        self.has_next = self.page < self.total_pages
+    @observe("max_per_page")
+    def _apply_max(self, change):
+        if change.new:
+            self.per_page = min(change.new, self.per_page)
+
+    total = Integer(0)
+
+    total_pages = Integer(0)
+
+    @default("total_pages")
+    def _calculate_total_pages(self):
+        total_pages = self.total // self.per_page
+        if self.total % self.per_page:
+            # there's a remainder, add 1
+            total_pages += 1
+        return total_pages
+
+    @observe("per_page", "total")
+    def _update_total_pages(self, change):
+        """Update total_pages when per_page or total is changed"""
+        self.total_pages = self._calculate_total_pages()
+
+    separator = Unicode("...")
 
-    @classmethod
     def get_page_args(self, handler):
         """
         This method gets the arguments used in the webpage to configurate the pagination
         In case of no arguments, it uses the default values from this class
 
-        It returns:
-          - self.page: The page requested for paginating or the default value (1)
-          - self.per_page: The number of items to return in this page. By default 100 and no more than 250
-          - self.per_page * (self.page - 1): The offset to consider when managing pagination via the ORM
+        Returns:
+          - page: The page requested for paginating or the default value (1)
+          - per_page: The number of items to return in this page. No more than max_per_page
+          - offset: The offset to consider when managing pagination via the ORM
         """
-        self.page = handler.get_argument(self._page_name, self._default_page)
-        self.per_page = handler.get_argument(
-            self._per_page_name, self._default_per_page
-        )
+        page = handler.get_argument("page", 1)
+        per_page = handler.get_argument("per_page", self.default_per_page)
         try:
-            self.per_page = int(self.per_page)
-            if self.per_page > self._max_per_page:
-                self.per_page = self._max_per_page
-        except:
+            self.per_page = int(per_page)
+        except Exception:
             self.per_page = self._default_per_page
 
         try:
-            self.page = int(self.page)
+            self.page = int(page)
             if self.page < 1:
-                self.page = self._default_page
+                self.page = 1
         except:
-            self.page = self._default_page
+            self.page = 1
 
         return self.page, self.per_page, self.per_page * (self.page - 1)
 
@@ -81,48 +106,54 @@ class Pagination:
         return {'total': self.total, 'start': start, 'end': end}
 
     def calculate_pages_window(self):
-        """Calculates the set of pages to render later in links() method. 
-           It returns the list of pages to render via links for the pagination 
+        """Calculates the set of pages to render later in links() method.
+           It returns the list of pages to render via links for the pagination
            By default, as we've observed in other applications, we're going to render
            only a finite and predefined number of pages, avoiding visual fatigue related
            to a long list of pages. By default, we render 7 pages plus some inactive links with the characters '...'
            to point out that there are other pages that aren't explicitly rendered.
-           The primary way of work is to provide current webpage and 5 next pages, the last 2 ones 
+           The primary way of work is to provide current webpage and 5 next pages, the last 2 ones
            (in case the current page + 5 does not overflow the total lenght of pages) and the first one for reference.
            """
 
-        self.separator_character = '...'
-        default_pages_to_render = 7
-        after_page = 5
-        before_end = 2
+        before_page = 2
+        after_page = 2
+        window_size = before_page + after_page + 1
 
-        # Add 1 to self.total_pages since our default page is 1 and not 0
-        total_pages = self.total_pages + 1
+        # Add 1 to total_pages since our starting page is 1 and not 0
+        last_page = self.total_pages
 
         pages = []
 
-        if total_pages > default_pages_to_render:
-            if self.page > 1:
-                pages.extend([1, '...'])
+        # will default window + start, end fit without truncation?
+        if self.total_pages > window_size + 2:
+            if self.page - before_page > 1:
+                # before_page will not reach page 1
+                pages.append(1)
+                if self.page - before_page > 2:
+                    # before_page will not reach page 2, need separator
+                    pages.append(self.separator)
 
-            if total_pages < self.page + after_page:
-                pages.extend(list(range(self.page, total_pages)))
+            pages.extend(range(max(1, self.page - before_page), self.page))
+            # we now have up to but not including self.page
+
+            if self.page + after_page + 1 >= last_page:
+                # after_page gets us to the end
+                pages.extend(range(self.page, last_page + 1))
             else:
-                if total_pages >= self.page + after_page + before_end:
-                    pages.extend(list(range(self.page, self.page + after_page)))
-                    pages.append('...')
-                    pages.extend(list(range(total_pages - before_end, total_pages)))
-                else:
-                    pages.extend(list(range(self.page, self.page + after_page)))
-                    if self.page + after_page < total_pages:
-                        # show only last page when the after_page window left space to show it
-                        pages.append('...')
-                        pages.extend(list(range(total_pages - 1, total_pages)))
+                # add full after_page entries
+                pages.extend(range(self.page, self.page + after_page + 1))
+                # add separator *if* this doesn't get to last page - 1
+                if self.page + after_page < last_page - 1:
+                    pages.append(self.separator)
+                pages.append(last_page)
 
             return pages
 
         else:
-            return list(range(1, total_pages))
+            # everything will fit, nothing to think about
+            # always return at least one page
+            return list(range(1, last_page + 1)) or [1]
 
     @property
     def links(self):
@@ -155,9 +186,11 @@ class Pagination:
                         page=page
                     )
                 )
-            elif page == self.separator_character:
+            elif page == self.separator:
                 links.append(
-                    '<li class="disabled"><span> <span aria-hidden="true">...</span></span></li>'
+                    '<li class="disabled"><span> <span aria-hidden="true">{separator}</span></span></li>'.format(
+                        separator=self.separator
+                    )
                 )
             else:
                 links.append(

jupyterhub/tests/test_pagination.py

@@ -0,0 +1,45 @@
+"""tests for pagination"""
+from pytest import mark
+from pytest import raises
+from traitlets.config import Config
+
+from jupyterhub.pagination import Pagination
+
+
+def test_per_page_bounds():
+    cfg = Config()
+    cfg.Pagination.max_per_page = 10
+    p = Pagination(config=cfg, per_page=20, total=100)
+    assert p.per_page == 10
+    with raises(Exception):
+        p.per_page = 0
+
+
+@mark.parametrize(
+    "page, per_page, total, expected",
+    [
+        (1, 10, 99, [1, 2, 3, "...", 10]),
+        (2, 10, 99, [1, 2, 3, 4, "...", 10]),
+        (3, 10, 99, [1, 2, 3, 4, 5, "...", 10]),
+        (4, 10, 99, [1, 2, 3, 4, 5, 6, "...", 10]),
+        (5, 10, 99, [1, "...", 3, 4, 5, 6, 7, "...", 10]),
+        (6, 10, 99, [1, "...", 4, 5, 6, 7, 8, "...", 10]),
+        (7, 10, 99, [1, "...", 5, 6, 7, 8, 9, 10]),
+        (8, 10, 99, [1, "...", 6, 7, 8, 9, 10]),
+        (9, 10, 99, [1, "...", 7, 8, 9, 10]),
+        (1, 20, 99, [1, 2, 3, 4, 5]),
+        (1, 10, 0, [1]),
+        (1, 10, 1, [1]),
+        (1, 10, 10, [1]),
+        (1, 10, 11, [1, 2]),
+        (1, 10, 50, [1, 2, 3, 4, 5]),
+        (1, 10, 60, [1, 2, 3, 4, 5, 6]),
+        (1, 10, 70, [1, 2, 3, 4, 5, 6, 7]),
+        (1, 10, 80, [1, 2, 3, "...", 8]),
+    ],
+)
+def test_window(page, per_page, total, expected):
+    cfg = Config()
+    cfg.Pagination
+    pagination = Pagination(page=page, per_page=per_page, total=total)
+    assert pagination.calculate_pages_window() == expected