[#2554] Add limit/offset support to group_list

So on the Organizations and Groups page we just dictize the groups on
the page (we need two calls to group_list in the controller, one
with all groups to account for the query, ordering, count, etc
and one with `all_fields` with just the ones to be displayed on
the listing).

ckan/controllers/group.py

@@ -150,15 +150,15 @@ 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
+        items_per_page = 21
+
         context = {'model': model, 'session': model.Session,
                    'user': c.user or c.author, 'for_view': True,
                    'with_private': False}
 
         q = c.q = request.params.get('q', '')
-        data_dict = {'all_fields': True, 'q': q, 'type': group_type or 'group'}
         sort_by = c.sort_by_selected = request.params.get('sort')
-        if sort_by:
-            data_dict['sort'] = sort_by
         try:
             self._check_access('site_read', context)
         except NotAuthorized:
@@ -170,14 +170,34 @@ def index(self):
             context['user_id'] = c.userobj.id
             context['user_is_admin'] = c.userobj.sysadmin
 
-        results = self._action('group_list')(context, data_dict)
+        data_dict_global_results = {
+            'all_fields': False,
+            'q': q,
+            'sort': sort_by,
+            'type': group_type or 'group',
+        }
+        global_results = self._action('group_list')(context,
+                                                    data_dict_global_results)
+
+        data_dict_page_results = {
+            'all_fields': True,
+            'q': q,
+            'sort': sort_by,
+            'type': group_type or 'group',
+            'limit': items_per_page,
+            'offset': items_per_page * (page - 1),
+        }
+        page_results = self._action('group_list')(context,
+                                                  data_dict_page_results)
 
         c.page = h.Page(
-            collection=results,
-            page = self._get_page_number(request.params),
+            collection=global_results,
+            page=page,
             url=h.pager_url,
-            items_per_page=21
+            items_per_page=items_per_page,
         )
+
+        c.page.items = page_results
         return render(self._index_template(group_type),
                       extra_vars={'group_type': group_type})
 

ckan/logic/action/get.py

@@ -368,8 +368,19 @@ def _group_or_org_list(context, data_dict, is_org=False):
     groups = data_dict.get('groups')
     group_type = data_dict.get('type', 'group')
     ref_group_by = 'id' if api == 2 else 'name'
-
-    sort = data_dict.get('sort', 'name')
+    pagination_dict = {}
+    limit = data_dict.get('limit')
+    if limit:
+        pagination_dict['limit'] = data_dict['limit']
+    offset = data_dict.get('offset')
+    if offset:
+        pagination_dict['offset'] = data_dict['offset']
+    if pagination_dict:
+        pagination_dict, errors = _validate(
+            data_dict, logic.schema.default_pagination_schema(), context)
+        if errors:
+            raise ValidationError(errors)
+    sort = data_dict.get('sort') or 'name'
     q = data_dict.get('q')
 
     all_fields = asbool(data_dict.get('all_fields', None))
@@ -436,6 +447,11 @@ def _group_or_org_list(context, data_dict, is_org=False):
         else:
             query = query.order_by(sqlalchemy.desc(sort_model_field))
 
+    if limit:
+        query = query.limit(limit)
+    if offset:
+        query = query.offset(offset)
+
     groups = query.all()
 
     if all_fields:
@@ -465,6 +481,13 @@ def group_list(context, data_dict):
         "name asc" string of field name and sort-order. The allowed fields are
         'name', 'package_count' and 'title'
     :type sort: string
+    :param limit: if given, the list of groups will be broken into pages of
+        at most ``limit`` groups per page and only one page will be returned
+        at a time (optional)
+    :type limit: int
+    :param offset: when ``limit`` is given, the offset to start
+        returning groups from
+    :type offset: int
     :param groups: a list of names of the groups to return, if given only
         groups whose names are in this list will be returned (optional)
     :type groups: list of strings
@@ -506,6 +529,13 @@ def organization_list(context, data_dict):
         "name asc" string of field name and sort-order. The allowed fields are
         'name', 'package_count' and 'title'
     :type sort: string
+    :param limit: if given, the list of organizations will be broken into pages
+        of at most ``limit`` organizations per page and only one page will be
+        returned at a time (optional)
+    :type limit: int
+    :param offset: when ``limit`` is given, the offset to start
+        returning organizations from
+    :type offset: int
     :param organizations: a list of names of the groups to return,
         if given only groups whose names are in this list will be
         returned (optional)

ckan/tests/controllers/test_group.py

@@ -254,3 +254,46 @@ def test_group_follower_list(self):
         followers_response = app.get(followers_url, extra_environ=env,
                                      status=200)
         assert_true(user_one['display_name'] in followers_response)
+
+
+class TestGroupIndex(helpers.FunctionalTestBase):
+
+    def test_group_index(self):
+        app = self._get_test_app()
+
+        for i in xrange(1, 25):
+            _i = '0' + str(i) if i < 10 else i
+            factories.Group(
+                name='test-group-{0}'.format(_i),
+                title='Test Group {0}'.format(_i))
+
+        url = url_for(controller='group',
+                      action='index')
+        response = app.get(url)
+
+        for i in xrange(1, 21):
+            _i = '0' + str(i) if i < 10 else i
+            assert_in('Test Group {0}'.format(_i), response)
+
+        assert 'Test Group 22' not in response
+
+        url = url_for(controller='group',
+                      action='index',
+                      page=1)
+        response = app.get(url)
+
+        for i in xrange(1, 21):
+            _i = '0' + str(i) if i < 10 else i
+            assert_in('Test Group {0}'.format(_i), response)
+
+        assert 'Test Group 22' not in response
+
+        url = url_for(controller='group',
+                      action='index',
+                      page=2)
+        response = app.get(url)
+
+        for i in xrange(22, 25):
+            assert_in('Test Group {0}'.format(i), response)
+
+        assert 'Test Group 21' not in response

ckan/tests/logic/action/test_get.py

@@ -8,6 +8,7 @@
 
 
 eq = nose.tools.eq_
+assert_raises = nose.tools.assert_raises
 
 
 class TestPackageShow(helpers.FunctionalTestBase):
@@ -179,6 +180,49 @@ def test_group_list_groups_returned(self):
 
         eq([g['name'] for g in child_group_returned['groups']], [expected_parent_group['name']])
 
+    def test_group_list_limit(self):
+
+        group1 = factories.Group()
+        group2 = factories.Group()
+        group3 = factories.Group()
+
+        group_list = helpers.call_action('group_list', limit=1)
+
+        eq(len(group_list), 1)
+        eq(group_list[0], group1['name'])
+
+    def test_group_list_offset(self):
+
+        group1 = factories.Group()
+        group2 = factories.Group()
+        group3 = factories.Group()
+
+        group_list = helpers.call_action('group_list', offset=2)
+
+        eq(len(group_list), 1)
+        eq(group_list[0], group3['name'])
+
+    def test_group_list_limit_and_offset(self):
+
+        group1 = factories.Group()
+        group2 = factories.Group()
+        group3 = factories.Group()
+
+        group_list = helpers.call_action('group_list', offset=1, limit=1)
+
+        eq(len(group_list), 1)
+        eq(group_list[0], group2['name'])
+
+    def test_group_list_wrong_limit(self):
+
+        assert_raises(logic.ValidationError, helpers.call_action, 'group_list',
+                      limit='a')
+
+    def test_group_list_wrong_offset(self):
+
+        assert_raises(logic.ValidationError, helpers.call_action, 'group_list',
+                      offset='-2')
+
 
 class TestGroupShow(helpers.FunctionalTestBase):