From 9be68909e4225d4593b558bf37d8b8c2cca66231 Mon Sep 17 00:00:00 2001 From: amercader Date: Wed, 19 Aug 2015 11:34:06 +0100 Subject: [PATCH] [#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 | 34 ++++++++++++++++----- ckan/logic/action/get.py | 34 +++++++++++++++++++-- ckan/tests/controllers/test_group.py | 43 +++++++++++++++++++++++++++ ckan/tests/logic/action/test_get.py | 44 ++++++++++++++++++++++++++++ 4 files changed, 146 insertions(+), 9 deletions(-) diff --git a/ckan/controllers/group.py b/ckan/controllers/group.py index 3377f7bb98b..7c5706b361d 100644 --- a/ckan/controllers/group.py +++ b/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}) diff --git a/ckan/logic/action/get.py b/ckan/logic/action/get.py index e7d5965a9e9..0dd002a09e7 100644 --- a/ckan/logic/action/get.py +++ b/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) diff --git a/ckan/tests/controllers/test_group.py b/ckan/tests/controllers/test_group.py index 73dfc43da50..a1cc76ff53c 100644 --- a/ckan/tests/controllers/test_group.py +++ b/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 diff --git a/ckan/tests/logic/action/test_get.py b/ckan/tests/logic/action/test_get.py index 8d05e7d9a0a..629fe7293c1 100644 --- a/ckan/tests/logic/action/test_get.py +++ b/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):