From 072b62b5b817f47529572cc6828e7535093034bc Mon Sep 17 00:00:00 2001 From: Vitor Baptista Date: Wed, 9 Apr 2014 21:51:48 -0300 Subject: [PATCH 01/12] [#1641] resource_create validates being passed an "url" parameter --- ckan/logic/action/create.py | 1 + ckan/new_tests/logic/action/test_create.py | 29 ++++++++++++++++++++++ 2 files changed, 30 insertions(+) create mode 100644 ckan/new_tests/logic/action/test_create.py diff --git a/ckan/logic/action/create.py b/ckan/logic/action/create.py index 18477c47966..6768d470702 100644 --- a/ckan/logic/action/create.py +++ b/ckan/logic/action/create.py @@ -261,6 +261,7 @@ def resource_create(context, data_dict): package_id = _get_or_bust(data_dict, 'package_id') data_dict.pop('package_id') + _get_or_bust(data_dict, 'url') pkg_dict = _get_action('package_show')(context, {'id': package_id}) diff --git a/ckan/new_tests/logic/action/test_create.py b/ckan/new_tests/logic/action/test_create.py new file mode 100644 index 00000000000..5f8538aef92 --- /dev/null +++ b/ckan/new_tests/logic/action/test_create.py @@ -0,0 +1,29 @@ +import nose.tools + +import ckan.model as model +import ckan.logic as logic +import ckan.new_tests.helpers as helpers +import ckan.new_tests.factories as factories + +eq = nose.tools.eq_ + + +class TestCreate(object): + + @classmethod + def setup_class(cls): + helpers.reset_db() + + def setup(self): + model.repo.rebuild_db() + + def test_resource_create_requires_url(self): + user = factories.User() + dataset = factories.Dataset(user=user) + data_dict = { + 'package_id': dataset['id'] + } + + nose.tools.assert_raises(logic.ValidationError, + helpers.call_action, + 'resource_create', **data_dict) From cf046dc4997367b84d0b04e1015cdfed0663cfad Mon Sep 17 00:00:00 2001 From: Vitor Baptista Date: Wed, 9 Apr 2014 21:52:44 -0300 Subject: [PATCH 02/12] [#1641] Recreate resource_create tests as "new_tests" --- ckan/new_tests/logic/action/test_create.py | 38 ++++++++++++++++++++++ ckan/tests/logic/test_action.py | 28 ---------------- 2 files changed, 38 insertions(+), 28 deletions(-) diff --git a/ckan/new_tests/logic/action/test_create.py b/ckan/new_tests/logic/action/test_create.py index 5f8538aef92..817d1bc1ba1 100644 --- a/ckan/new_tests/logic/action/test_create.py +++ b/ckan/new_tests/logic/action/test_create.py @@ -17,6 +17,30 @@ def setup_class(cls): def setup(self): model.repo.rebuild_db() + def test_resource_create(self): + user = factories.User() + dataset = factories.Dataset(user=user) + url = 'http://ckan.org/data.csv' + data_dict = { + 'package_id': dataset['id'], + 'url': url + } + + resource = helpers.call_action('resource_create', **data_dict) + + assert resource is not None + eq(resource['url'], url) + + def test_resource_create_requires_package_id(self): + url = 'http://ckan.org/data.csv' + data_dict = { + 'url': url + } + + nose.tools.assert_raises(logic.ValidationError, + helpers.call_action, + 'resource_create', **data_dict) + def test_resource_create_requires_url(self): user = factories.User() dataset = factories.Dataset(user=user) @@ -27,3 +51,17 @@ def test_resource_create_requires_url(self): nose.tools.assert_raises(logic.ValidationError, helpers.call_action, 'resource_create', **data_dict) + + def test_resource_create_validates_created_date(self): + user = factories.User() + dataset = factories.Dataset(user=user) + url = 'http://ckan.org/data.csv' + data_dict = { + 'package_id': dataset['id'], + 'url': url, + 'created': 'invalid_date' + } + + nose.tools.assert_raises(logic.ValidationError, + helpers.call_action, + 'resource_create', **data_dict) diff --git a/ckan/tests/logic/test_action.py b/ckan/tests/logic/test_action.py index a5488cdd2f0..ade0e971a2b 100644 --- a/ckan/tests/logic/test_action.py +++ b/ckan/tests/logic/test_action.py @@ -302,34 +302,6 @@ def test_18_create_package_not_authorized(self): res = self.app.post('/api/action/package_create', params=postparams, status=StatusCodes.STATUS_403_ACCESS_DENIED) - def test_41_create_resource(self): - - anna_id = model.Package.by_name(u'annakarenina').id - resource = {'package_id': anna_id, 'url': 'http://new_url'} - api_key = model.User.get('testsysadmin').apikey.encode('utf8') - postparams = '%s=1' % json.dumps(resource) - res = self.app.post('/api/action/resource_create', params=postparams, - extra_environ={'Authorization': api_key }) - - resource = json.loads(res.body)['result'] - - assert resource['url'] == 'http://new_url' - - def test_42_create_resource_with_error(self): - - anna_id = model.Package.by_name(u'annakarenina').id - resource = {'package_id': anna_id, 'url': 'new_url', 'created': 'bad_date'} - api_key = model.User.get('testsysadmin').apikey.encode('utf8') - - postparams = '%s=1' % json.dumps(resource) - res = self.app.post('/api/action/resource_create', params=postparams, - extra_environ={'Authorization': api_key}, - status=StatusCodes.STATUS_409_CONFLICT) - - assert json.loads(res.body)['error'] == {"__type": "Validation Error", "created": ["Date format incorrect"]} - - - def test_04_user_list(self): # Create deleted user to make sure he won't appear in the user_list deleted_user = CreateTestData.create_user('deleted_user') From 219368abeec2eeab9b23d68cbb91e6bc8581b4fc Mon Sep 17 00:00:00 2001 From: Vitor Baptista Date: Wed, 9 Apr 2014 21:53:27 -0300 Subject: [PATCH 03/12] Minor English typos fixes --- ckan/logic/action/create.py | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/ckan/logic/action/create.py b/ckan/logic/action/create.py index 6768d470702..425853d0c16 100644 --- a/ckan/logic/action/create.py +++ b/ckan/logic/action/create.py @@ -214,13 +214,12 @@ def package_create(context, data_dict): def resource_create(context, data_dict): '''Appends a new resource to a datasets list of resources. - :param package_id: id of package that the resource needs - should be added to. + :param package_id: id of package that the resource should be added to. :type package_id: string :param url: url of resource :type url: string :param revision_id: (optional) - :type revisiion_id: string + :type revision_id: string :param description: (optional) :type description: string :param format: (optional) From 04e6ac4c0363bb2923c27ef6bad398b3f73ac819 Mon Sep 17 00:00:00 2001 From: Ross Jones Date: Fri, 18 Apr 2014 11:27:12 +0100 Subject: [PATCH 04/12] Removed clean_action_name as it causes confusion --- ckan/logic/__init__.py | 5 ----- ckan/new_authz.py | 10 ---------- 2 files changed, 15 deletions(-) diff --git a/ckan/logic/__init__.py b/ckan/logic/__init__.py index 59be5a43b05..49fbc0d7beb 100644 --- a/ckan/logic/__init__.py +++ b/ckan/logic/__init__.py @@ -259,7 +259,6 @@ def check_access(action, context, data_dict=None): authorized to call the named action ''' - action = new_authz.clean_action_name(action) # Auth Auditing. We remove this call from the __auth_audit stack to show # we have called the auth function @@ -341,8 +340,6 @@ def get_action(action): :rtype: callable ''' - # clean the action names - action = new_authz.clean_action_name(action) if _actions: if not action in _actions: @@ -365,7 +362,6 @@ def get_action(action): if (hasattr(v, '__call__') and (v.__module__ == module_path or hasattr(v, '__replaced'))): - k = new_authz.clean_action_name(k) _actions[k] = v # Whitelist all actions defined in logic/action/get.py as @@ -380,7 +376,6 @@ def get_action(action): fetched_actions = {} for plugin in p.PluginImplementations(p.IActions): for name, auth_function in plugin.get_actions().items(): - name = new_authz.clean_action_name(name) if name in resolved_action_plugins: raise Exception( 'The action %r is already implemented in %r' % ( diff --git a/ckan/new_authz.py b/ckan/new_authz.py index 44eb09f5dc9..7d4cdc3716d 100644 --- a/ckan/new_authz.py +++ b/ckan/new_authz.py @@ -57,7 +57,6 @@ def _build(self): for key, v in module.__dict__.items(): if not key.startswith('_'): - key = clean_action_name(key) # Whitelist all auth functions defined in # logic/auth/get.py as not requiring an authorized user, # as well as ensuring that the rest do. In both cases, do @@ -75,7 +74,6 @@ def _build(self): fetched_auth_functions = {} for plugin in p.PluginImplementations(p.IAuthFunctions): for name, auth_function in plugin.get_auth_functions().items(): - name = clean_action_name(name) if name in resolved_auth_function_plugins: raise Exception( 'The auth function %r is already implemented in %r' % ( @@ -106,13 +104,6 @@ def auth_functions_list(): return _AuthFunctions.keys() -def clean_action_name(action_name): - ''' Used to convert old style action names into new style ones ''' - new_action_name = re.sub('package', 'dataset', action_name) - # CS: bad_spelling ignore - return re.sub('licence', 'license', new_action_name) - - def is_sysadmin(username): ''' Returns True is username is a sysadmin ''' user = _get_user(username) @@ -157,7 +148,6 @@ def is_authorized(action, context, data_dict=None): if context.get('ignore_auth'): return {'success': True} - action = clean_action_name(action) auth_function = _AuthFunctions.get(action) if auth_function: username = context.get('user') From 50c31142d1b5db898795ae3b19bfb36a4f01fd03 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?V=C3=ADtor=20Avelino?= Date: Wed, 23 Apr 2014 08:10:39 -0300 Subject: [PATCH 05/12] [#1538] Break long title words (module content heading on sidebar and media-item heading) --- ckan/public/base/less/layout.less | 1 + ckan/public/base/less/media.less | 1 + ckan/public/base/less/mixins.less | 13 +++++++++++++ 3 files changed, 15 insertions(+) diff --git a/ckan/public/base/less/layout.less b/ckan/public/base/less/layout.less index 9ae4d86796d..f5fefffe2ab 100644 --- a/ckan/public/base/less/layout.less +++ b/ckan/public/base/less/layout.less @@ -109,6 +109,7 @@ margin: 0 0 5px 0; font-size: 18px; line-height: 1.3; + .break-word(); } .info { margin-top: 15px; diff --git a/ckan/public/base/less/media.less b/ckan/public/base/less/media.less index f18c49d0e91..ca95fbf7357 100644 --- a/ckan/public/base/less/media.less +++ b/ckan/public/base/less/media.less @@ -90,6 +90,7 @@ font-size: 18px; line-height: 1.3; margin: 5px 0; + .break-word(); } // Overlay diff --git a/ckan/public/base/less/mixins.less b/ckan/public/base/less/mixins.less index a8bbbf9c0fc..9ceb5ad7ec5 100644 --- a/ckan/public/base/less/mixins.less +++ b/ckan/public/base/less/mixins.less @@ -1,3 +1,16 @@ +.break-word { + -ms-word-break: break-all; + word-break: break-all; + + /* Non standard for webkit */ + word-break: break-word; + + -webkit-hyphens: auto; + -moz-hyphens: auto; + -ms-hyphens: auto; + hyphens: auto; +} + .transform (@func) { -webkit-transform: @arguments; -moz-transform: @arguments; From fccfad0d4b64183ed1f5f8cf38f92e17b29f47e7 Mon Sep 17 00:00:00 2001 From: Vitor Baptista Date: Thu, 24 Apr 2014 10:50:42 -0300 Subject: [PATCH 06/12] Revert "[#1641] Recreate resource_create tests as "new_tests"" This was more complex than I expected. To avoid blocking this issue, I'm reverting it here and will work on it at #1669. This reverts commit cf046dc4997367b84d0b04e1015cdfed0663cfad. --- ckan/new_tests/logic/action/test_create.py | 38 ---------------------- ckan/tests/logic/test_action.py | 28 ++++++++++++++++ 2 files changed, 28 insertions(+), 38 deletions(-) diff --git a/ckan/new_tests/logic/action/test_create.py b/ckan/new_tests/logic/action/test_create.py index 817d1bc1ba1..5f8538aef92 100644 --- a/ckan/new_tests/logic/action/test_create.py +++ b/ckan/new_tests/logic/action/test_create.py @@ -17,30 +17,6 @@ def setup_class(cls): def setup(self): model.repo.rebuild_db() - def test_resource_create(self): - user = factories.User() - dataset = factories.Dataset(user=user) - url = 'http://ckan.org/data.csv' - data_dict = { - 'package_id': dataset['id'], - 'url': url - } - - resource = helpers.call_action('resource_create', **data_dict) - - assert resource is not None - eq(resource['url'], url) - - def test_resource_create_requires_package_id(self): - url = 'http://ckan.org/data.csv' - data_dict = { - 'url': url - } - - nose.tools.assert_raises(logic.ValidationError, - helpers.call_action, - 'resource_create', **data_dict) - def test_resource_create_requires_url(self): user = factories.User() dataset = factories.Dataset(user=user) @@ -51,17 +27,3 @@ def test_resource_create_requires_url(self): nose.tools.assert_raises(logic.ValidationError, helpers.call_action, 'resource_create', **data_dict) - - def test_resource_create_validates_created_date(self): - user = factories.User() - dataset = factories.Dataset(user=user) - url = 'http://ckan.org/data.csv' - data_dict = { - 'package_id': dataset['id'], - 'url': url, - 'created': 'invalid_date' - } - - nose.tools.assert_raises(logic.ValidationError, - helpers.call_action, - 'resource_create', **data_dict) diff --git a/ckan/tests/logic/test_action.py b/ckan/tests/logic/test_action.py index ade0e971a2b..a5488cdd2f0 100644 --- a/ckan/tests/logic/test_action.py +++ b/ckan/tests/logic/test_action.py @@ -302,6 +302,34 @@ def test_18_create_package_not_authorized(self): res = self.app.post('/api/action/package_create', params=postparams, status=StatusCodes.STATUS_403_ACCESS_DENIED) + def test_41_create_resource(self): + + anna_id = model.Package.by_name(u'annakarenina').id + resource = {'package_id': anna_id, 'url': 'http://new_url'} + api_key = model.User.get('testsysadmin').apikey.encode('utf8') + postparams = '%s=1' % json.dumps(resource) + res = self.app.post('/api/action/resource_create', params=postparams, + extra_environ={'Authorization': api_key }) + + resource = json.loads(res.body)['result'] + + assert resource['url'] == 'http://new_url' + + def test_42_create_resource_with_error(self): + + anna_id = model.Package.by_name(u'annakarenina').id + resource = {'package_id': anna_id, 'url': 'new_url', 'created': 'bad_date'} + api_key = model.User.get('testsysadmin').apikey.encode('utf8') + + postparams = '%s=1' % json.dumps(resource) + res = self.app.post('/api/action/resource_create', params=postparams, + extra_environ={'Authorization': api_key}, + status=StatusCodes.STATUS_409_CONFLICT) + + assert json.loads(res.body)['error'] == {"__type": "Validation Error", "created": ["Date format incorrect"]} + + + def test_04_user_list(self): # Create deleted user to make sure he won't appear in the user_list deleted_user = CreateTestData.create_user('deleted_user') From 114bdfd59b534ef047a993388ee541b5b4c540a8 Mon Sep 17 00:00:00 2001 From: Vitor Baptista Date: Thu, 24 Apr 2014 11:10:19 -0300 Subject: [PATCH 07/12] [#1641] Refactor renaming TestCreate -> TestResourceCreate --- ckan/new_tests/logic/action/test_create.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/ckan/new_tests/logic/action/test_create.py b/ckan/new_tests/logic/action/test_create.py index 5f8538aef92..1cbef582dd2 100644 --- a/ckan/new_tests/logic/action/test_create.py +++ b/ckan/new_tests/logic/action/test_create.py @@ -8,7 +8,7 @@ eq = nose.tools.eq_ -class TestCreate(object): +class TestResourceCreate(object): @classmethod def setup_class(cls): @@ -17,7 +17,7 @@ def setup_class(cls): def setup(self): model.repo.rebuild_db() - def test_resource_create_requires_url(self): + def test_requires_url(self): user = factories.User() dataset = factories.Dataset(user=user) data_dict = { From 71ef11f211b9c3147d4207c1a0d8ce37b553bac4 Mon Sep 17 00:00:00 2001 From: Vitor Baptista Date: Thu, 24 Apr 2014 23:12:25 -0300 Subject: [PATCH 08/12] [#1641] Rename test method so it reads more natural --- ckan/new_tests/logic/action/test_create.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/ckan/new_tests/logic/action/test_create.py b/ckan/new_tests/logic/action/test_create.py index 51d22814aa9..b30e832215f 100644 --- a/ckan/new_tests/logic/action/test_create.py +++ b/ckan/new_tests/logic/action/test_create.py @@ -98,7 +98,7 @@ def setup_class(cls): def setup(self): model.repo.rebuild_db() - def test_requires_url(self): + def test_it_requires_url(self): user = factories.User() dataset = factories.Dataset(user=user) data_dict = { From 251d0a51601b42ebd6063ca98c6ce8fc1ed18ba8 Mon Sep 17 00:00:00 2001 From: Rodrigo Parra Date: Thu, 24 Apr 2014 23:19:41 -0400 Subject: [PATCH 09/12] [#1663] Remove redundant '--ckan-migration' explanation from test documentation --- doc/contributing/test.rst | 2 -- 1 file changed, 2 deletions(-) diff --git a/doc/contributing/test.rst b/doc/contributing/test.rst index f9d2f6bc805..42d00c04de9 100644 --- a/doc/contributing/test.rst +++ b/doc/contributing/test.rst @@ -71,8 +71,6 @@ option:: nosetests --ckan --reset-db --with-pylons=test-core.ini ckan -If you are have the ``ckan-migration`` option on the tests will reset the -reset the database before the test run. .. _migrationtesting: From f49954e272e638a39b0b6baf8da11fede905097f Mon Sep 17 00:00:00 2001 From: Rodrigo Parra Date: Thu, 24 Apr 2014 23:22:02 -0400 Subject: [PATCH 10/12] Fix typo in migration testing documentation --- doc/contributing/test.rst | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/doc/contributing/test.rst b/doc/contributing/test.rst index 42d00c04de9..e0316e60968 100644 --- a/doc/contributing/test.rst +++ b/doc/contributing/test.rst @@ -82,7 +82,7 @@ Migration testing If you're a CKAN developer or extension developer and your new code requires a change to CKAN's model, you'll need to write a migration script. To ensure that the migration script itself gets tested, you should run the tests with -they ``--ckan-migration`` option, for example:: +the ``--ckan-migration`` option, for example:: nosetests --ckan --ckan-migration --with-pylons=test-core.ini ckan ckanext From 6391dda7c6900142b5d3665fd42351e2d7ac51a0 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?V=C3=ADtor=20Avelino?= Date: Fri, 25 Apr 2014 08:04:24 -0300 Subject: [PATCH 11/12] [#1538] Fix indentation --- ckan/public/base/less/mixins.less | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/ckan/public/base/less/mixins.less b/ckan/public/base/less/mixins.less index 9ceb5ad7ec5..43bfb2c408c 100644 --- a/ckan/public/base/less/mixins.less +++ b/ckan/public/base/less/mixins.less @@ -1,14 +1,14 @@ .break-word { -ms-word-break: break-all; - word-break: break-all; + word-break: break-all; - /* Non standard for webkit */ - word-break: break-word; + /* Non standard for webkit */ + word-break: break-word; -webkit-hyphens: auto; - -moz-hyphens: auto; - -ms-hyphens: auto; - hyphens: auto; + -moz-hyphens: auto; + -ms-hyphens: auto; + hyphens: auto; } .transform (@func) { From 76c7354bc98025223c6c72291c2b42281b2d3a7a Mon Sep 17 00:00:00 2001 From: Vitor Baptista Date: Fri, 25 Apr 2014 14:24:38 -0300 Subject: [PATCH 12/12] Fix link to "Testing CKAN" on "Making a pull request" doc page --- doc/contributing/pull-requests.rst | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/doc/contributing/pull-requests.rst b/doc/contributing/pull-requests.rst index 98b05c9f4dd..6909657b5c3 100644 --- a/doc/contributing/pull-requests.rst +++ b/doc/contributing/pull-requests.rst @@ -64,7 +64,7 @@ This section will walk you through the steps for making a pull request. - Your branch should contain new or changed tests for any new or changed code, and all the CKAN tests should pass on your branch, see - `Testing CKAN `_. + :doc:`test`. - Your branch should contain new or updated documentation for any new or updated code, see :doc:`documentation`.