From 37de568854bb0f2fe9eb816244f060b4f75b2e21 Mon Sep 17 00:00:00 2001 From: amercader Date: Thu, 29 Nov 2012 18:00:27 +0000 Subject: [PATCH 01/18] [#3023] Add new extension points to pass the package dict When creating or updating a dataset, extensions can hook to the `after_create` and `after_update` methods to receive the validated data dict. In the create case, the id is added to the dict. --- ckan/logic/action/create.py | 3 ++ ckan/logic/action/update.py | 3 ++ ckan/plugins/interfaces.py | 17 +++++++++ ckan/tests/functional/test_package.py | 55 +++++++++++++++++++++++++-- 4 files changed, 74 insertions(+), 4 deletions(-) diff --git a/ckan/logic/action/create.py b/ckan/logic/action/create.py index 2fece287b10..59659b62a68 100644 --- a/ckan/logic/action/create.py +++ b/ckan/logic/action/create.py @@ -151,10 +151,13 @@ def package_create(context, data_dict): model.setup_default_user_roles(pkg, admins) # Needed to let extensions know the package id model.Session.flush() + data['id'] = pkg.id for item in plugins.PluginImplementations(plugins.IPackageController): item.create(pkg) + item.after_create(data) + if not context.get('defer_commit'): model.repo.commit() diff --git a/ckan/logic/action/update.py b/ckan/logic/action/update.py index 5dfb365ba7a..931723cd20b 100644 --- a/ckan/logic/action/update.py +++ b/ckan/logic/action/update.py @@ -271,6 +271,9 @@ def package_update(context, data_dict): for item in plugins.PluginImplementations(plugins.IPackageController): item.edit(pkg) + + item.after_update(data) + if not context.get('defer_commit'): model.repo.commit() diff --git a/ckan/plugins/interfaces.py b/ckan/plugins/interfaces.py index bde61f934a6..26c73ad0696 100644 --- a/ckan/plugins/interfaces.py +++ b/ckan/plugins/interfaces.py @@ -266,6 +266,23 @@ def authz_remove_role(self, object_role): def delete(self, entity): pass + def after_create(self, pkg_dict): + ''' + Extensions will receive the validated data dict after the package + has been created (Note that the create method will return a package + domain object, which may not include all fields). Also the newly + created package id will be added to the dict. + ''' + pass + + def after_update(self, pkg_dict): + ''' + Extensions will receive the validated data dict after the package + has been updated (Note that the edit method will return a package + domain object, which may not include all fields). + ''' + pass + def before_search(self, search_params): ''' Extensions will receive a dictionary with the query parameters, diff --git a/ckan/tests/functional/test_package.py b/ckan/tests/functional/test_package.py index 59911a2bc06..1af57a30d4b 100644 --- a/ckan/tests/functional/test_package.py +++ b/ckan/tests/functional/test_package.py @@ -57,13 +57,23 @@ def after_search(self, search_results, search_params): self.calls['after_search'] += 1 return search_results - def before_index(self, search_params): + def before_index(self, data_dict): self.calls['before_index'] += 1 - return search_params + return data_dict - def before_view(self, search_params): + def before_view(self, data_dict): self.calls['before_view'] += 1 - return search_params + return data_dict + + def after_create(self, data_dict): + self.calls['after_create'] += 1 + self.id_in_dict = 'id' in data_dict + + return data_dict + + def after_update(self, data_dict): + self.calls['after_update'] += 1 + return data_dict existing_extra_html = ('', '') @@ -943,6 +953,26 @@ def test_edit_plugin_hook(self): finally: self._reset_data() + def test_after_update_plugin_hook(self): + # just the absolute basics + try: + plugin = MockPackageControllerPlugin() + plugins.load(plugin) + res = self.app.get(self.offset, extra_environ=self.extra_environ_admin) + new_name = u'new-name' + new_title = u'New Title' + fv = res.forms['dataset-edit'] + prefix = '' + fv[prefix + 'name'] = new_name + fv[prefix + 'title'] = new_title + res = fv.submit('save', extra_environ=self.extra_environ_admin) + # get redirected ... + assert plugin.calls['after_update'] == 1, plugin.calls + assert plugin.calls['after_create'] == 0, plugin.calls + plugins.unload(plugin) + finally: + self._reset_data() + def test_edit_700_groups_add(self): try: pkg = model.Package.by_name(u'editpkgtest') @@ -1274,6 +1304,23 @@ def test_new_plugin_hook(self): assert plugin.calls['create'] == 1, plugin.calls plugins.unload(plugin) + def test_after_create_plugin_hook(self): + plugin = MockPackageControllerPlugin() + plugins.load(plugin) + offset = url_for(controller='package', action='new') + res = self.app.get(offset, extra_environ=self.extra_environ_tester) + new_name = u'plugged2' + fv = res.forms['dataset-edit'] + prefix = '' + fv[prefix + 'name'] = new_name + res = fv.submit('save', extra_environ=self.extra_environ_tester) + # get redirected ... + assert plugin.calls['after_update'] == 0, plugin.calls + assert plugin.calls['after_create'] == 1, plugin.calls + + assert plugin.id_in_dict + + plugins.unload(plugin) def test_new_indexerror(self): bad_solr_url = 'http://127.0.0.1/badsolrurl' From 0d7cfdb2ab1389f6010dae91a255e85095aa97c8 Mon Sep 17 00:00:00 2001 From: amercader Date: Thu, 6 Dec 2012 18:19:49 +0000 Subject: [PATCH 02/18] [#3023] Pass context object to extensions on after_create and after_update --- ckan/logic/action/create.py | 2 +- ckan/logic/action/update.py | 2 +- ckan/tests/functional/test_package.py | 4 ++-- 3 files changed, 4 insertions(+), 4 deletions(-) diff --git a/ckan/logic/action/create.py b/ckan/logic/action/create.py index 59659b62a68..54bbd6876dc 100644 --- a/ckan/logic/action/create.py +++ b/ckan/logic/action/create.py @@ -156,7 +156,7 @@ def package_create(context, data_dict): for item in plugins.PluginImplementations(plugins.IPackageController): item.create(pkg) - item.after_create(data) + item.after_create(context, data) if not context.get('defer_commit'): model.repo.commit() diff --git a/ckan/logic/action/update.py b/ckan/logic/action/update.py index 931723cd20b..6ee78361542 100644 --- a/ckan/logic/action/update.py +++ b/ckan/logic/action/update.py @@ -272,7 +272,7 @@ def package_update(context, data_dict): for item in plugins.PluginImplementations(plugins.IPackageController): item.edit(pkg) - item.after_update(data) + item.after_update(context, data) if not context.get('defer_commit'): model.repo.commit() diff --git a/ckan/tests/functional/test_package.py b/ckan/tests/functional/test_package.py index 1af57a30d4b..565710cc6f2 100644 --- a/ckan/tests/functional/test_package.py +++ b/ckan/tests/functional/test_package.py @@ -65,13 +65,13 @@ def before_view(self, data_dict): self.calls['before_view'] += 1 return data_dict - def after_create(self, data_dict): + def after_create(self, context, data_dict): self.calls['after_create'] += 1 self.id_in_dict = 'id' in data_dict return data_dict - def after_update(self, data_dict): + def after_update(self, context, data_dict): self.calls['after_update'] += 1 return data_dict From 4de12f1b14d78a77dbf543951a456951b83a156a Mon Sep 17 00:00:00 2001 From: amercader Date: Fri, 7 Dec 2012 10:21:14 +0000 Subject: [PATCH 03/18] [#3023] Add new after_show extension point Gets the package dict just before being sent back to the controller. Removed an out of date comment on tests. --- ckan/logic/action/get.py | 3 +++ ckan/plugins/interfaces.py | 12 ++++++++++-- ckan/tests/functional/test_package.py | 10 +++++----- 3 files changed, 18 insertions(+), 7 deletions(-) diff --git a/ckan/logic/action/get.py b/ckan/logic/action/get.py index c85e3a7ece9..cf555d83e24 100644 --- a/ckan/logic/action/get.py +++ b/ckan/logic/action/get.py @@ -623,6 +623,9 @@ def package_show(context, data_dict): if schema and context.get('validate', True): package_dict, errors = _validate(package_dict, schema, context=context) + for item in plugins.PluginImplementations(plugins.IPackageController): + item.after_show(context, package_dict) + return package_dict def resource_show(context, data_dict): diff --git a/ckan/plugins/interfaces.py b/ckan/plugins/interfaces.py index 26c73ad0696..397138bbe29 100644 --- a/ckan/plugins/interfaces.py +++ b/ckan/plugins/interfaces.py @@ -266,7 +266,7 @@ def authz_remove_role(self, object_role): def delete(self, entity): pass - def after_create(self, pkg_dict): + def after_create(self, context, pkg_dict): ''' Extensions will receive the validated data dict after the package has been created (Note that the create method will return a package @@ -275,7 +275,7 @@ def after_create(self, pkg_dict): ''' pass - def after_update(self, pkg_dict): + def after_update(self, context, pkg_dict): ''' Extensions will receive the validated data dict after the package has been updated (Note that the edit method will return a package @@ -283,6 +283,14 @@ def after_update(self, pkg_dict): ''' pass + def after_show(self, context, pkg_dict): + ''' + Extensions will receive the validated data dict after the package + is ready for display (Note that the read method will return a + package domain object, which may not include all fields). + ''' + pass + def before_search(self, search_params): ''' Extensions will receive a dictionary with the query parameters, diff --git a/ckan/tests/functional/test_package.py b/ckan/tests/functional/test_package.py index 565710cc6f2..13e519bf176 100644 --- a/ckan/tests/functional/test_package.py +++ b/ckan/tests/functional/test_package.py @@ -75,6 +75,10 @@ def after_update(self, context, data_dict): self.calls['after_update'] += 1 return data_dict + def after_show(self, context, data_dict): + self.calls['after_show'] += 1 + return data_dict + existing_extra_html = ('', '') @@ -379,12 +383,8 @@ def test_read_plugin_hook(self): offset = url_for(controller='package', action='read', id=name) res = self.app.get(offset) - # There are now two reads of the package. The first to find out - # the package's type. And the second is the actual read that - # existed before. I don't know if this is a problem? I expect it - # can be fixed by allowing the package to be passed in to the plugin, - # either via the function argument, or adding it to the c object. assert plugin.calls['read'] == 1, plugin.calls + assert plugin.calls['after_show'] == 1, plugin.calls plugins.unload(plugin) def test_resource_list(self): From 283671a71975c701b116fa25d8592b2c29328b07 Mon Sep 17 00:00:00 2001 From: amercader Date: Mon, 17 Dec 2012 15:33:49 +0000 Subject: [PATCH 04/18] [#3023] Add new after_delete extension point Gets the context adn data_dict (tipically with just the id) after deleting a dataset. --- ckan/logic/action/delete.py | 3 +++ ckan/plugins/interfaces.py | 7 +++++ ckan/tests/functional/test_package.py | 39 +++++++++++++++++++++++++++ 3 files changed, 49 insertions(+) diff --git a/ckan/logic/action/delete.py b/ckan/logic/action/delete.py index 22246be7f5a..4e818c13fc2 100644 --- a/ckan/logic/action/delete.py +++ b/ckan/logic/action/delete.py @@ -41,6 +41,9 @@ def package_delete(context, data_dict): for item in plugins.PluginImplementations(plugins.IPackageController): item.delete(entity) + + item.after_delete(context, data_dict) + entity.delete() model.repo.commit() diff --git a/ckan/plugins/interfaces.py b/ckan/plugins/interfaces.py index 397138bbe29..b8b176922a1 100644 --- a/ckan/plugins/interfaces.py +++ b/ckan/plugins/interfaces.py @@ -283,6 +283,13 @@ def after_update(self, context, pkg_dict): ''' pass + def after_delete(self, context, pkg_dict): + ''' + Extensions will receive the data dict (tipically containing + just the package id) after the package has been deleted. + ''' + pass + def after_show(self, context, pkg_dict): ''' Extensions will receive the validated data dict after the package diff --git a/ckan/tests/functional/test_package.py b/ckan/tests/functional/test_package.py index 13e519bf176..1c6447eacf9 100644 --- a/ckan/tests/functional/test_package.py +++ b/ckan/tests/functional/test_package.py @@ -75,6 +75,10 @@ def after_update(self, context, data_dict): self.calls['after_update'] += 1 return data_dict + def after_delete(self, context, data_dict): + self.calls['after_delete'] += 1 + return data_dict + def after_show(self, context, data_dict): self.calls['after_show'] += 1 return data_dict @@ -1066,6 +1070,41 @@ def test_edit_pkg_with_relationships(self): finally: self._reset_data() +class TestDelete(TestPackageForm): + + pkg_names = [] + + @classmethod + def setup_class(self): + model.repo.init_db() + CreateTestData.create() + CreateTestData.create_test_user() + +# self.admin = model.User.by_name(u'russianfan') + +# self.extra_environ_admin = {'REMOTE_USER': self.admin.name.encode('utf8')} + self.extra_environ_tester = {'REMOTE_USER': 'tester'} + + @classmethod + def teardown_class(self): + self.purge_packages(self.pkg_names) + model.repo.rebuild_db() + + def test_delete(self): + plugin = MockPackageControllerPlugin() + plugins.load(plugin) + + offset = url_for(controller='package', action='delete', + id='warandpeace') + self.app.post(offset, extra_environ=self.extra_environ_tester) + + assert model.Package.get('warandpeace').state == u'deleted' + + assert plugin.calls['delete'] == 1 + assert plugin.calls['after_delete'] == 1 + + plugins.unload(plugin) + class TestNew(TestPackageForm): pkg_names = [] From e1c8b69512ff04148ba4fefc0292a2e4dbdba503 Mon Sep 17 00:00:00 2001 From: tobes Date: Tue, 18 Dec 2012 12:20:22 +0000 Subject: [PATCH 05/18] [#3030] Move suppress active class into _active() --- ckan/lib/helpers.py | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/ckan/lib/helpers.py b/ckan/lib/helpers.py index af6d72b20f9..4e9e493d0fa 100644 --- a/ckan/lib/helpers.py +++ b/ckan/lib/helpers.py @@ -306,8 +306,7 @@ def nav_link(text, controller, **kwargs): icon = kwargs.pop('icon', None) if icon: text = literal(' ' % icon) + text - no_active = kwargs.pop('suppress_active_class', False) - class_ = _link_class(kwargs, no_active) + class_ = _link_class(kwargs) if kwargs.pop('condition', True): link = link_to( text, @@ -327,8 +326,10 @@ def _link_active(kwargs): and c.action in highlight_actions) -def _link_class(kwargs, suppress_active_class=False): +def _link_class(kwargs): ''' creates classes for the link_to calls ''' + # suppress_active_class = kwargs.pop('suppress_active_class', False) + suppress_active_class = False if not suppress_active_class and _link_active(kwargs): active = ' active' else: From 3b4b268f17b1460c8fb6ce6993363749337b8354 Mon Sep 17 00:00:00 2001 From: tobes Date: Tue, 18 Dec 2012 12:35:25 +0000 Subject: [PATCH 06/18] [#3030] Fix suppress_activew --- ckan/lib/helpers.py | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/ckan/lib/helpers.py b/ckan/lib/helpers.py index 4e9e493d0fa..dfcddf4f484 100644 --- a/ckan/lib/helpers.py +++ b/ckan/lib/helpers.py @@ -328,8 +328,7 @@ def _link_active(kwargs): def _link_class(kwargs): ''' creates classes for the link_to calls ''' - # suppress_active_class = kwargs.pop('suppress_active_class', False) - suppress_active_class = False + suppress_active_class = kwargs.pop('suppress_active_class', False) if not suppress_active_class and _link_active(kwargs): active = ' active' else: From 4ade7982d8ad2a55bb13cc26310f04a0366088a6 Mon Sep 17 00:00:00 2001 From: tobes Date: Tue, 18 Dec 2012 12:40:59 +0000 Subject: [PATCH 07/18] [#3030] Combine code adding icons --- ckan/lib/helpers.py | 24 ++++++++++++------------ 1 file changed, 12 insertions(+), 12 deletions(-) diff --git a/ckan/lib/helpers.py b/ckan/lib/helpers.py index dfcddf4f484..44971eb4daa 100644 --- a/ckan/lib/helpers.py +++ b/ckan/lib/helpers.py @@ -301,15 +301,10 @@ def nav_link(text, controller, **kwargs): condition: if False then no link is returned ''' kwargs['controller'] = controller - if kwargs.get('inner_span'): - text = literal('') + text + literal('') - icon = kwargs.pop('icon', None) - if icon: - text = literal(' ' % icon) + text class_ = _link_class(kwargs) if kwargs.pop('condition', True): link = link_to( - text, + _create_link_text(text, **kwargs), url_for(**kwargs), class_=class_ ) @@ -318,6 +313,16 @@ def nav_link(text, controller, **kwargs): return link +def _create_link_text(text, **kwargs): + ''' Update link text to add a icon or span if specified in the kwargs ''' + if kwargs.pop('inner_span', None): + text = literal('') + text + literal('') + icon = kwargs.pop('icon', None) + if icon: + text = literal(' ' % icon) + text + return text + + def _link_active(kwargs): ''' creates classes for the link_to calls ''' highlight_actions = kwargs.get('highlight_actions', @@ -339,13 +344,8 @@ def _link_class(kwargs): def nav_named_link(text, name, **kwargs): class_ = _link_class(kwargs) - - icon = kwargs.pop('icon', None) - if icon: - text = literal(' ' % icon) + text - return link_to( - text, + _create_link_text(text, **kwargs), url_for(name, **kwargs), class_=class_ ) From 95bb34d7d370d5652383bcb6966f07e254570db8 Mon Sep 17 00:00:00 2001 From: tobes Date: Tue, 18 Dec 2012 12:53:45 +0000 Subject: [PATCH 08/18] [#3030] Deprecate subnav_named_route() as identical function nav_named_route() exits --- ckan/lib/helpers.py | 15 ++++++--------- 1 file changed, 6 insertions(+), 9 deletions(-) diff --git a/ckan/lib/helpers.py b/ckan/lib/helpers.py index 44971eb4daa..c5d01cb0ef1 100644 --- a/ckan/lib/helpers.py +++ b/ckan/lib/helpers.py @@ -38,6 +38,7 @@ import ckan.lib.fanstatic_resources as fanstatic_resources import ckan.model as model import ckan.lib.formatters as formatters +import ckan.lib.maintain as maintain get_available_locales = i18n.get_available_locales get_locales_dict = i18n.get_locales_dict @@ -361,16 +362,12 @@ def subnav_link(text, action, **kwargs): ) +@maintain.deprecated('h.subnav_named_route is deprecated please ' + 'use h.nav_named_link') def subnav_named_route(text, routename, **kwargs): - """ Generate a subnav element based on a named route """ - # FIXME this is the same as _nav_named_link - # they should be combined - class_ = _link_class(kwargs) - return link_to( - text, - url_for(str(routename), **kwargs), - class_=class_ - ) + '''Generate a subnav element based on a named route + Deprecated in ckan 2.0 ''' + return subnav_named_route(text, routename, **kwargs) def build_nav_main(*args): From 430f407621a72e337c266ed595afb25716f94089 Mon Sep 17 00:00:00 2001 From: tobes Date: Tue, 18 Dec 2012 12:55:46 +0000 Subject: [PATCH 09/18] [#3030] Remove unused code in lib.helpers - now got via routes --- ckan/lib/helpers.py | 15 --------------- 1 file changed, 15 deletions(-) diff --git a/ckan/lib/helpers.py b/ckan/lib/helpers.py index c5d01cb0ef1..c0f81203102 100644 --- a/ckan/lib/helpers.py +++ b/ckan/lib/helpers.py @@ -457,21 +457,6 @@ def default_group_type(): return str(config.get('ckan.default.group_type', 'group')) -_menu_items = { - 'add dataset': dict(controller='package', action='new'), - 'search': dict(controller='package', - action='search', - highlight_actions='index search'), - 'default_group': dict(name='%s_index' % default_group_type(), - controller='group', - highlight_actions='index search'), - 'about': dict(controller='home', action='about'), - 'login': dict(controller='user', action='login'), - 'register': dict(controller='user', action='register'), - 'organizations': dict(action='index', controller='organization'), -} - - def get_facet_items_dict(facet, limit=10, exclude_active=False): '''Return the list of unselected facet items for the given facet, sorted by count. From 244e7145d9f8def2908020ac6207a2b83fc3fdf0 Mon Sep 17 00:00:00 2001 From: tobes Date: Tue, 18 Dec 2012 13:08:56 +0000 Subject: [PATCH 10/18] [#3030] Bugfix subnav_named_route() --- ckan/lib/helpers.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/ckan/lib/helpers.py b/ckan/lib/helpers.py index c0f81203102..9b181ca8065 100644 --- a/ckan/lib/helpers.py +++ b/ckan/lib/helpers.py @@ -367,7 +367,7 @@ def subnav_link(text, action, **kwargs): def subnav_named_route(text, routename, **kwargs): '''Generate a subnav element based on a named route Deprecated in ckan 2.0 ''' - return subnav_named_route(text, routename, **kwargs) + return nav_named_link(text, routename, **kwargs) def build_nav_main(*args): From c2ef34513337e832c1c3cfb0cf4c51e876a780a6 Mon Sep 17 00:00:00 2001 From: tobes Date: Tue, 18 Dec 2012 13:10:21 +0000 Subject: [PATCH 11/18] [#3030] Increase code sharing in lib helpers --- ckan/lib/helpers.py | 51 ++++++++++++++++++++------------------------- 1 file changed, 23 insertions(+), 28 deletions(-) diff --git a/ckan/lib/helpers.py b/ckan/lib/helpers.py index 9b181ca8065..f6d75d9aa84 100644 --- a/ckan/lib/helpers.py +++ b/ckan/lib/helpers.py @@ -294,26 +294,6 @@ def are_there_flash_messages(): return flash.are_there_messages() -def nav_link(text, controller, **kwargs): - ''' - params - class_: pass extra class(s) to add to the tag - icon: name of ckan icon to use within the link - condition: if False then no link is returned - ''' - kwargs['controller'] = controller - class_ = _link_class(kwargs) - if kwargs.pop('condition', True): - link = link_to( - _create_link_text(text, **kwargs), - url_for(**kwargs), - class_=class_ - ) - else: - link = '' - return link - - def _create_link_text(text, **kwargs): ''' Update link text to add a icon or span if specified in the kwargs ''' if kwargs.pop('inner_span', None): @@ -343,23 +323,38 @@ def _link_class(kwargs): return kwargs.pop('class_', '') + active or None -def nav_named_link(text, name, **kwargs): +def _link_to(text, *args, **kwargs): + assert len(args)<2, 'Too many unnamed arguments' class_ = _link_class(kwargs) return link_to( _create_link_text(text, **kwargs), - url_for(name, **kwargs), + url_for(*args, **kwargs), class_=class_ ) +def nav_link(text, controller, **kwargs): + ''' + params + class_: pass extra class(s) to add to the tag + icon: name of ckan icon to use within the link + condition: if False then no link is returned + ''' + if kwargs.pop('condition', True): + kwargs['controller'] = controller + link = _link_to(text, **kwargs) + else: + link = '' + return link + + +def nav_named_link(text, name, **kwargs): + return _link_to(text, name, **kwargs) + + def subnav_link(text, action, **kwargs): kwargs['action'] = action - class_ = _link_class(kwargs) - return link_to( - text, - url_for(**kwargs), - class_=class_ - ) + return _link_to(text, **kwargs) @maintain.deprecated('h.subnav_named_route is deprecated please ' From 4b8a9c2c9112a4d8dd5eb05f563f84f8a8cbc65e Mon Sep 17 00:00:00 2001 From: tobes Date: Tue, 18 Dec 2012 13:24:25 +0000 Subject: [PATCH 12/18] [#3030] Further code refactor --- ckan/lib/helpers.py | 45 ++++++++++++++++++++++++--------------------- 1 file changed, 24 insertions(+), 21 deletions(-) diff --git a/ckan/lib/helpers.py b/ckan/lib/helpers.py index f6d75d9aa84..5c97d58e2c4 100644 --- a/ckan/lib/helpers.py +++ b/ckan/lib/helpers.py @@ -294,16 +294,6 @@ def are_there_flash_messages(): return flash.are_there_messages() -def _create_link_text(text, **kwargs): - ''' Update link text to add a icon or span if specified in the kwargs ''' - if kwargs.pop('inner_span', None): - text = literal('') + text + literal('') - icon = kwargs.pop('icon', None) - if icon: - text = literal(' ' % icon) + text - return text - - def _link_active(kwargs): ''' creates classes for the link_to calls ''' highlight_actions = kwargs.get('highlight_actions', @@ -312,19 +302,30 @@ def _link_active(kwargs): and c.action in highlight_actions) -def _link_class(kwargs): - ''' creates classes for the link_to calls ''' - suppress_active_class = kwargs.pop('suppress_active_class', False) - if not suppress_active_class and _link_active(kwargs): - active = ' active' - else: - active = '' - kwargs.pop('highlight_actions', '') - return kwargs.pop('class_', '') + active or None - - def _link_to(text, *args, **kwargs): + '''Common link making code for several helper functions''' assert len(args)<2, 'Too many unnamed arguments' + + def _link_class(kwargs): + ''' creates classes for the link_to calls ''' + suppress_active_class = kwargs.pop('suppress_active_class', False) + if not suppress_active_class and _link_active(kwargs): + active = ' active' + else: + active = '' + kwargs.pop('highlight_actions', '') + return kwargs.pop('class_', '') + active or None + + def _create_link_text(text, **kwargs): + ''' Update link text to add a icon or span if specified in the + kwargs ''' + if kwargs.pop('inner_span', None): + text = literal('') + text + literal('') + icon = kwargs.pop('icon', None) + if icon: + text = literal(' ' % icon) + text + return text + class_ = _link_class(kwargs) return link_to( _create_link_text(text, **kwargs), @@ -349,10 +350,12 @@ def nav_link(text, controller, **kwargs): def nav_named_link(text, name, **kwargs): + '''Create a link for a named route.''' return _link_to(text, name, **kwargs) def subnav_link(text, action, **kwargs): + '''Create a link for a named route.''' kwargs['action'] = action return _link_to(text, **kwargs) From f3c9fd69c031da7e13a78d7f4a80857ffcd242a6 Mon Sep 17 00:00:00 2001 From: Dominik Moritz Date: Fri, 21 Dec 2012 14:21:49 +0100 Subject: [PATCH 13/18] [247] fixes #247 by disallowing anonymous users to edit something that they are not allowed to create --- ckan/logic/auth/update.py | 11 +++++++---- 1 file changed, 7 insertions(+), 4 deletions(-) diff --git a/ckan/logic/auth/update.py b/ckan/logic/auth/update.py index da02b2d1009..8a8c2d8d67f 100644 --- a/ckan/logic/auth/update.py +++ b/ckan/logic/auth/update.py @@ -14,10 +14,13 @@ def package_update(context, data_dict): user = context.get('user') package = get_package_object(context, data_dict) - if package.owner_org: - check1 = new_authz.has_user_permission_for_group_or_org(package.owner_org, user, 'update_dataset') - else: - check1 = new_authz.check_config_permission('create_dataset_if_not_in_organization') + if not new_authz.auth_is_registered_user(): + check1 = new_authz.check_config_permission('anon_create_dataset') + if check1: + if package.owner_org: + check1 = new_authz.has_user_permission_for_group_or_org(package.owner_org, user, 'update_dataset') + else: + check1 = new_authz.check_config_permission('create_dataset_if_not_in_organization') if not check1: return {'success': False, 'msg': _('User %s not authorized to edit package %s') % (str(user), package.id)} else: From 85e94eb894b0ae0a2e6cd14b525acd5a450555dd Mon Sep 17 00:00:00 2001 From: tobes Date: Fri, 18 Jan 2013 05:13:16 +0000 Subject: [PATCH 14/18] [#286] Fix group autocomplete to not show orgs --- ckan/model/group.py | 10 +++++++--- 1 file changed, 7 insertions(+), 3 deletions(-) diff --git a/ckan/model/group.py b/ckan/model/group.py index 6cff2161aa5..37bcfe2b55d 100644 --- a/ckan/model/group.py +++ b/ckan/model/group.py @@ -221,13 +221,17 @@ def packages(self, with_private=False, limit=None, return query.all() @classmethod - def search_by_name_or_title(cls, text_query, group_type=None): + def search_by_name_or_title(cls, text_query, group_type=None, is_org=False): text_query = text_query.strip().lower() q = meta.Session.query(cls) \ .filter(or_(cls.name.contains(text_query), cls.title.ilike('%' + text_query + '%'))) - if group_type: - q = q.filter(cls.type == group_type) + if is_org: + q = q.filter(cls.type == 'organization') + else: + q = q.filter(cls.type != 'organization') + if group_type: + q = q.filter(cls.type == group_type) return q.order_by(cls.title) def add_package_by_name(self, package_name): From b575a8a63d03768b3b8ab9d04d695e39916e9534 Mon Sep 17 00:00:00 2001 From: tobes Date: Tue, 22 Jan 2013 03:47:31 +0000 Subject: [PATCH 15/18] [#248] Fix auth function --- ckan/logic/auth/update.py | 16 ++++++++++------ 1 file changed, 10 insertions(+), 6 deletions(-) diff --git a/ckan/logic/auth/update.py b/ckan/logic/auth/update.py index 8a8c2d8d67f..24f44634223 100644 --- a/ckan/logic/auth/update.py +++ b/ckan/logic/auth/update.py @@ -14,13 +14,17 @@ def package_update(context, data_dict): user = context.get('user') package = get_package_object(context, data_dict) - if not new_authz.auth_is_registered_user(): - check1 = new_authz.check_config_permission('anon_create_dataset') - if check1: - if package.owner_org: - check1 = new_authz.has_user_permission_for_group_or_org(package.owner_org, user, 'update_dataset') + if package.owner_org: + # if there is an owner org then we must have update_dataset + # premission for that organization + check1 = new_authz.has_user_permission_for_group_or_org(package.owner_org, user, 'update_dataset') + else: + # If dataset is not owned then we can edit if config permissions allow + if new_authz.auth_is_registered_user(): + check1 = new_authz.check_config_permission( + 'create_dataset_if_not_in_organization') else: - check1 = new_authz.check_config_permission('create_dataset_if_not_in_organization') + check1 = new_authz.check_config_permission('anon_create_dataset'))) if not check1: return {'success': False, 'msg': _('User %s not authorized to edit package %s') % (str(user), package.id)} else: From c4e26bfea4d14d4646f289af059b9ca7ed2927e0 Mon Sep 17 00:00:00 2001 From: tobes Date: Wed, 23 Jan 2013 04:48:47 +0000 Subject: [PATCH 16/18] [#247] Fix indent and trailing )) --- ckan/logic/auth/update.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/ckan/logic/auth/update.py b/ckan/logic/auth/update.py index 24f44634223..53f4ca58dfe 100644 --- a/ckan/logic/auth/update.py +++ b/ckan/logic/auth/update.py @@ -24,7 +24,7 @@ def package_update(context, data_dict): check1 = new_authz.check_config_permission( 'create_dataset_if_not_in_organization') else: - check1 = new_authz.check_config_permission('anon_create_dataset'))) + check1 = new_authz.check_config_permission('anon_create_dataset') if not check1: return {'success': False, 'msg': _('User %s not authorized to edit package %s') % (str(user), package.id)} else: From 0b6c6e9fa58fea34c0c1e19433be98faaee09865 Mon Sep 17 00:00:00 2001 From: amercader Date: Wed, 23 Jan 2013 10:26:02 +0000 Subject: [PATCH 17/18] [#3023] Fix failing test --- ckan/tests/functional/test_package.py | 11 ++++++++--- 1 file changed, 8 insertions(+), 3 deletions(-) diff --git a/ckan/tests/functional/test_package.py b/ckan/tests/functional/test_package.py index 1c6447eacf9..8cb24807aa6 100644 --- a/ckan/tests/functional/test_package.py +++ b/ckan/tests/functional/test_package.py @@ -1070,6 +1070,7 @@ def test_edit_pkg_with_relationships(self): finally: self._reset_data() + class TestDelete(TestPackageForm): pkg_names = [] @@ -1080,9 +1081,9 @@ def setup_class(self): CreateTestData.create() CreateTestData.create_test_user() -# self.admin = model.User.by_name(u'russianfan') + self.admin = model.User.by_name(u'testsysadmin') -# self.extra_environ_admin = {'REMOTE_USER': self.admin.name.encode('utf8')} + self.extra_environ_admin = {'REMOTE_USER': self.admin.name.encode('utf8')} self.extra_environ_tester = {'REMOTE_USER': 'tester'} @classmethod @@ -1096,7 +1097,10 @@ def test_delete(self): offset = url_for(controller='package', action='delete', id='warandpeace') - self.app.post(offset, extra_environ=self.extra_environ_tester) + + self.app.post(offset, extra_environ=self.extra_environ_tester, status=401) + + self.app.post(offset, extra_environ=self.extra_environ_admin) assert model.Package.get('warandpeace').state == u'deleted' @@ -1105,6 +1109,7 @@ def test_delete(self): plugins.unload(plugin) + class TestNew(TestPackageForm): pkg_names = [] From 391df0ba5b5c00ecd4e08d865d97419ce0c553c4 Mon Sep 17 00:00:00 2001 From: amercader Date: Wed, 23 Jan 2013 10:44:12 +0000 Subject: [PATCH 18/18] [#286] Add a test --- ckan/tests/models/test_group.py | 15 ++++++++++++--- 1 file changed, 12 insertions(+), 3 deletions(-) diff --git a/ckan/tests/models/test_group.py b/ckan/tests/models/test_group.py index 424e8f3c198..3296d8ae872 100644 --- a/ckan/tests/models/test_group.py +++ b/ckan/tests/models/test_group.py @@ -54,8 +54,16 @@ def test_2_add_packages(self): assert grp in anna.get_groups() def test_3_search(self): - def search_results(query): - results = model.Group.search_by_name_or_title(query) + model.repo.new_revision() + model.Session.add(model.Group(name=u'test_org', + title=u'Test org', + type=u'organization' + )) + model.repo.commit_and_remove() + + + def search_results(query, is_org=False): + results = model.Group.search_by_name_or_title(query,is_org=is_org) return set([group.name for group in results]) assert_equal(search_results('random'), set([])) assert_equal(search_results('david'), set(['david'])) @@ -66,7 +74,8 @@ def search_results(query): assert_equal(search_results('Dave\'s'), set(['david'])) assert_equal(search_results('Dave\'s books'), set(['david'])) assert_equal(search_results('Books'), set(['david', 'roger'])) - + assert_equal(search_results('Books', is_org=True), set([])) + assert_equal(search_results('Test', is_org=True), set(['test_org'])) class TestGroupRevisions: @classmethod