From e1194397efc6793617f4870ddf940775134c9eec Mon Sep 17 00:00:00 2001 From: joetsoi Date: Tue, 5 Nov 2013 10:52:13 +0000 Subject: [PATCH] [#1258] fix 500 errors on activity list action functions change tests, no need to hit the database we only need to test that the vaildators raise a ValidationError --- ckan/logic/action/get.py | 16 ++++++---- ckan/new_tests/logic/action/test_get.py | 41 +++++++++++++++++-------- 2 files changed, 38 insertions(+), 19 deletions(-) diff --git a/ckan/logic/action/get.py b/ckan/logic/action/get.py index 6c95563f0b1..902109a4b0e 100644 --- a/ckan/logic/action/get.py +++ b/ckan/logic/action/get.py @@ -2085,6 +2085,7 @@ def user_activity_list(context, data_dict): offset=offset) return model_dictize.activity_list_dictize(activity_objects, context) +@logic.validate(logic.schema.default_activity_list_schema) def package_activity_list(context, data_dict): '''Return a package's activity stream. @@ -2109,7 +2110,7 @@ def package_activity_list(context, data_dict): model = context['model'] - package_ref = _get_or_bust(data_dict, 'id') # May be name or ID. + package_ref = data_dict.get('id') # May be name or ID. package = model.Package.get(package_ref) if package is None: raise logic.NotFound @@ -2122,6 +2123,7 @@ def package_activity_list(context, data_dict): limit=limit, offset=offset) return model_dictize.activity_list_dictize(activity_objects, context) +@logic.validate(logic.schema.default_activity_list_schema) def group_activity_list(context, data_dict): '''Return a group's activity stream. @@ -2145,8 +2147,8 @@ def group_activity_list(context, data_dict): _check_access('group_show', context, data_dict) model = context['model'] - group_id = _get_or_bust(data_dict, 'id') - offset = int(data_dict.get('offset', 0)) + group_id = data_dict.get('id') + offset = data_dict.get('offset', 0) limit = int( data_dict.get('limit', config.get('ckan.activity_list_limit', 31))) @@ -2158,6 +2160,7 @@ def group_activity_list(context, data_dict): limit=limit, offset=offset) return model_dictize.activity_list_dictize(activity_objects, context) +@logic.validate(logic.schema.default_activity_list_schema) def organization_activity_list(context, data_dict): '''Return a organization's activity stream. @@ -2172,8 +2175,8 @@ def organization_activity_list(context, data_dict): _check_access('organization_show', context, data_dict) model = context['model'] - org_id = _get_or_bust(data_dict, 'id') - offset = int(data_dict.get('offset', 0)) + org_id = data_dict.get('id') + offset = data_dict.get('offset', 0) limit = int( data_dict.get('limit', config.get('ckan.activity_list_limit', 31))) @@ -2185,6 +2188,7 @@ def organization_activity_list(context, data_dict): limit=limit, offset=offset) return model_dictize.activity_list_dictize(activity_objects, context) +@logic.validate(logic.schema.default_activity_list_schema) def recently_changed_packages_activity_list(context, data_dict): '''Return the activity stream of all recently added or changed packages. @@ -2202,7 +2206,7 @@ def recently_changed_packages_activity_list(context, data_dict): # FIXME: Filter out activities whose subject or object the user is not # authorized to read. model = context['model'] - offset = int(data_dict.get('offset', 0)) + offset = data_dict.get('offset', 0) limit = int( data_dict.get('limit', config.get('ckan.activity_list_limit', 31))) diff --git a/ckan/new_tests/logic/action/test_get.py b/ckan/new_tests/logic/action/test_get.py index 1d0aae3b9bf..2de16c30999 100644 --- a/ckan/new_tests/logic/action/test_get.py +++ b/ckan/new_tests/logic/action/test_get.py @@ -5,18 +5,33 @@ import ckan.new_tests.factories as factories class TestBadLimitQueryParameters(object): - '''test class for #1258 non-int query parameters cause 500 errors''' - @classmethod - def setup_class(cls): - helpers.reset_db() + '''test class for #1258 non-int query parameters cause 500 errors + + Test that validation errors are raised when calling actions with + bad parameters. + ''' - def teardown(self): - import ckan.model as model - model.repo.rebuild_db() + def test_activity_list_actions(self): + actions = [ + 'user_activity_list', + 'package_activity_list', + 'group_activity_list', + 'organization_activity_list', + 'recently_changed_packages_activity_list', + 'user_activity_list_html', + 'package_activity_list_html', + 'group_activity_list_html', + 'organization_activity_list_html', + 'recently_changed_packages_activity_list_html', + + ] + for action in actions: + nose.tools.assert_raises(logic.ValidationError, + helpers.call_action, action, id='test_user', + limit='not_an_int', offset='not_an_int' + ) + nose.tools.assert_raises(logic.ValidationError, + helpers.call_action, action, id='test_user', + limit=-1, offset=-1 + ) - def test_user_activity_list(self): - user = factories.User() - nose.tools.assert_raises(logic.ValidationError, helpers.call_action, - 'user_activity_list', id=user['name'], limit='not_an_int', - offset='not_an_int' - )