From e552048dba78f44edd9589c0400eb0561cc0b4b0 Mon Sep 17 00:00:00 2001 From: Nigel Babu Date: Thu, 12 Dec 2013 14:36:10 +0300 Subject: [PATCH 1/6] [#1384] Make the database query in related_list * We didn't actually do the database query in related_list if we didn't pass a dataset. This commit fixes that. * Share the dictize function for both cases. --- ckan/logic/action/get.py | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/ckan/logic/action/get.py b/ckan/logic/action/get.py index 7e97d02bce9..82abfaf9f06 100644 --- a/ckan/logic/action/get.py +++ b/ckan/logic/action/get.py @@ -262,10 +262,11 @@ def related_list(context, data_dict=None): if data_dict.get('featured', False): related_list = related_list.filter(model.Related.featured == 1) + related_items = related_list.all() else: relateds = model.Related.get_for_dataset(dataset, status='active') related_items = (r.related for r in relateds) - related_list = model_dictize.related_list_dictize( related_items, context) + related_list = model_dictize.related_list_dictize( related_items, context) return related_list From d2693b834af3516d279827181bb6f0d88d005ad1 Mon Sep 17 00:00:00 2001 From: Nigel Babu Date: Tue, 17 Dec 2013 09:58:17 +0530 Subject: [PATCH 2/6] [#1384] Update related controller to use a list The controller relied on the output of related_list being a Query object. Updated them to use the list. --- ckan/controllers/related.py | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/ckan/controllers/related.py b/ckan/controllers/related.py index 8b9e88cfc1d..16de940b21e 100644 --- a/ckan/controllers/related.py +++ b/ckan/controllers/related.py @@ -40,7 +40,7 @@ def dashboard(self): base.abort(400, ('"page" parameter must be an integer')) # Update ordering in the context - query = logic.get_action('related_list')(context, data_dict) + related_list = logic.get_action('related_list')(context, data_dict) def search_url(params): url = h.url_for(controller='related', action='dashboard') @@ -55,10 +55,10 @@ def pager_url(q=None, page=None): return search_url(params) c.page = h.Page( - collection=query.all(), + collection=related_list, page=page, url=pager_url, - item_count=query.count(), + item_count=len(related_list), items_per_page=9 ) From f32277034a326867df84082a69fd3d2452a82d3a Mon Sep 17 00:00:00 2001 From: Nigel Babu Date: Tue, 17 Dec 2013 09:59:15 +0530 Subject: [PATCH 3/6] [#1384] Update tests to deal with list The tests also relied on related_list being a Query instead of a list. Updated them and added new asserts. --- ckan/tests/functional/test_related.py | 9 +++++++-- 1 file changed, 7 insertions(+), 2 deletions(-) diff --git a/ckan/tests/functional/test_related.py b/ckan/tests/functional/test_related.py index 6fa2fed4266..9e65917fb2a 100644 --- a/ckan/tests/functional/test_related.py +++ b/ckan/tests/functional/test_related.py @@ -367,8 +367,13 @@ def test_related_list_missing_id_and_name(self): usr = logic.get_action('get_site_user')({'model':model,'ignore_auth': True},{}) context = dict(model=model, user=usr['name'], session=model.Session) data_dict = {} - from sqlalchemy.orm.query import Query - assert type(logic.get_action('related_list')(context, data_dict)) == Query + related_list = logic.get_action('related_list')(context, data_dict) + assert len(related_list) == 8 + related_keys = set(['view_count', 'description', 'title', 'url', + 'created', 'featured', 'image_url', 'type', 'id', 'owner_id']) + for related in related_list: + assert set(related.keys()) == related_keys + def test_related_list(self): p = model.Package.get('warandpeace') From 546caa71b66df2cbcab34bbb50b6ebabab66cdfc Mon Sep 17 00:00:00 2001 From: Nigel Babu Date: Tue, 17 Dec 2013 10:41:27 +0530 Subject: [PATCH 4/6] [#1384] Update documentation for related_list The related_list function should not need an id or dataset parameter. We use it ourselves in the related dashboard without an id or dataset parameter. --- ckan/logic/action/get.py | 2 -- 1 file changed, 2 deletions(-) diff --git a/ckan/logic/action/get.py b/ckan/logic/action/get.py index 82abfaf9f06..3db861c2fc7 100644 --- a/ckan/logic/action/get.py +++ b/ckan/logic/action/get.py @@ -212,8 +212,6 @@ def related_show(context, data_dict=None): def related_list(context, data_dict=None): '''Return a dataset's related items. - Either the ``id`` or the ``dataset`` parameter must be given. - :param id: id or name of the dataset (optional) :type id: string :param dataset: dataset dictionary of the dataset (optional) From d621b8b2e7c8064779401343ff1460dfdb36facc Mon Sep 17 00:00:00 2001 From: Nigel Babu Date: Tue, 17 Dec 2013 10:42:30 +0530 Subject: [PATCH 5/6] [#1384] Remove sorted from model_dictize related_items is already sorted from the database query. Adding another sorting at the model_dictize level looks unnecessary and overrides the one from the query. --- ckan/lib/dictization/model_dictize.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/ckan/lib/dictization/model_dictize.py b/ckan/lib/dictization/model_dictize.py index ede1fd85c7f..515642f8da7 100644 --- a/ckan/lib/dictization/model_dictize.py +++ b/ckan/lib/dictization/model_dictize.py @@ -91,7 +91,7 @@ def related_list_dictize(related_list, context): related_dict = related_dictize(res, context) result_list.append(related_dict) - return sorted(result_list, key=lambda x: x["created"], reverse=True) + return result_list def extras_dict_dictize(extras_dict, context): From 27831b326959706f8b91b43bff4405ea9171981c Mon Sep 17 00:00:00 2001 From: Nigel Babu Date: Tue, 17 Dec 2013 11:31:45 +0530 Subject: [PATCH 6/6] [#1384] Fix test broken due to sorted change * Fix the test broken because it expects results from sorted. * Correct the PEP8 change in related controller. --- ckan/controllers/related.py | 2 +- ckan/tests/functional/test_related.py | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/ckan/controllers/related.py b/ckan/controllers/related.py index 16de940b21e..3bc6b82acb8 100644 --- a/ckan/controllers/related.py +++ b/ckan/controllers/related.py @@ -40,7 +40,7 @@ def dashboard(self): base.abort(400, ('"page" parameter must be an integer')) # Update ordering in the context - related_list = logic.get_action('related_list')(context, data_dict) + related_list = logic.get_action('related_list')(context, data_dict) def search_url(params): url = h.url_for(controller='related', action='dashboard') diff --git a/ckan/tests/functional/test_related.py b/ckan/tests/functional/test_related.py index 9e65917fb2a..78085b24a11 100644 --- a/ckan/tests/functional/test_related.py +++ b/ckan/tests/functional/test_related.py @@ -467,7 +467,7 @@ def test_api_list(self): r = json.loads(res.body) assert r['success'] == True, r assert r['result'][0]['type'] == "idea" - assert r['result'][0]['title'] == "two", r + assert r['result'][0]['title'] == "one", r p.related.remove(one) p.related.remove(two)