Merge pull request #3120 from ckan/3120-get-page-number

Tidy-up BaseController pagination code

ckan/controllers/feed.py

@@ -310,7 +310,7 @@ def custom(self):
                 search_params[param] = value
                 fq += ' %s:"%s"' % (param, value)
 
-        page = self._get_page_number(request.params)
+        page = h.get_page_number(request.params)
 
         limit = ITEMS_LIMIT
         data_dict = {
@@ -455,7 +455,7 @@ def _parse_url_params(self):
         Returns the constructed search-query dict, and the valid URL
         query parameters.
         """
-        page = self._get_page_number(request.params)
+        page = h.get_page_number(request.params)
 
         limit = ITEMS_LIMIT
         data_dict = {

ckan/controllers/group.py

@@ -150,7 +150,7 @@ def add_group_type(cls, group_type):
     def index(self):
         group_type = self._guess_group_type()
 
-        page = self._get_page_number(request.params) or 1
+        page = h.get_page_number(request.params) or 1
         items_per_page = 21
 
         context = {'model': model, 'session': model.Session,
@@ -247,7 +247,7 @@ def _read(self, id, limit, group_type):
 
         context['return_query'] = True
 
-        page = self._get_page_number(request.params)
+        page = h.get_page_number(request.params)
 
         # most search operations should reset the page counter:
         params_nopage = [(k, v) for k, v in request.params.items()

ckan/controllers/package.py

@@ -146,7 +146,7 @@ def search(self):
         # unicode format (decoded from utf8)
         q = c.q = request.params.get('q', u'')
         c.query_error = False
-        page = self._get_page_number(request.params)
+        page = h.get_page_number(request.params)
 
         limit = g.datasets_per_page
 

ckan/controllers/revision.py

@@ -124,7 +124,7 @@ def list(self):
             query = model.Session.query(model.Revision)
             c.page = h.Page(
                 collection=query,
-                page=self._get_page_number(request.params),
+                page=h.get_page_number(request.params),
                 url=h.pager_url,
                 items_per_page=20
             )

ckan/controllers/tag.py

@@ -36,7 +36,7 @@ def index(self):
         data_dict = {'all_fields': True}
 
         if c.q:
-            page = self._get_page_number(request.params)
+            page = h.get_page_number(request.params)
             data_dict['q'] = c.q
             data_dict['limit'] = LIMIT
             data_dict['offset'] = (page - 1) * LIMIT

ckan/controllers/user.py

@@ -97,7 +97,7 @@ def _get_repoze_handler(self, handler_name):
                        handler_name)
 
     def index(self):
-        page = self._get_page_number(request.params)
+        page = h.get_page_number(request.params)
         c.q = request.params.get('q', '')
         c.order_by = request.params.get('order_by', 'name')
 

ckan/lib/base.py

@@ -34,13 +34,9 @@
 
 log = logging.getLogger(__name__)
 
-PAGINATE_ITEMS_PER_PAGE = 50
-
 APIKEY_HEADER_NAME_KEY = 'apikey_header_name'
 APIKEY_HEADER_NAME_DEFAULT = 'X-CKAN-API-Key'
 
-ALLOWED_FIELDSET_PARAMS = ['package_form', 'restrict']
-
 
 def abort(status_code=None, detail='', headers=None, comment=None):
     '''Abort the current request immediately by returning an HTTP exception.
@@ -376,24 +372,6 @@ def _get_user_for_apikey(self):
         user = query.filter_by(apikey=apikey).first()
         return user
 
-    def _get_page_number(self, params, key='page', default=1):
-        """
-        Returns the page number from the provided params after
-        verifies that it is an integer.
-
-        If it fails it will abort the request with a 400 error
-        """
-        p = params.get(key, default)
-
-        try:
-            p = int(p)
-            if p < 1:
-                raise ValueError("Negative number not allowed")
-        except ValueError, e:
-            abort(400, ('"page" parameter must be a positive integer'))
-
-        return p
-
 
 # Include the '_' function in the public names
 __all__ = [__name for __name in locals().keys() if not __name.startswith('_')

ckan/lib/helpers.py

@@ -1088,6 +1088,28 @@ def _range(self, regexp_match):
         return re.sub(current_page_span, current_page_link, html)
 
 
+@core_helper
+def get_page_number(params, key='page', default=1):
+    '''
+    Return the page number from the provided params after verifying that it is
+    an positive integer.
+
+    If it fails it will abort the request with a 400 error.
+    '''
+    p = params.get(key, default)
+
+    try:
+        p = int(p)
+        if p < 1:
+            raise ValueError("Negative number not allowed")
+    except ValueError:
+        import ckan.lib.base as base
+        base.abort(400, ('"{key}" parameter must be a positive integer'
+                   .format(key=key)))
+
+    return p
+
+
 @core_helper
 def get_display_timezone():
     ''' Returns a pytz timezone for the display_timezone setting in the

ckan/tests/controllers/test_feed.py

@@ -2,7 +2,6 @@
 
 from routes import url_for
 
-from ckan import model
 import ckan.tests.helpers as helpers
 import ckan.tests.factories as factories