From 37de568854bb0f2fe9eb816244f060b4f75b2e21 Mon Sep 17 00:00:00 2001 From: amercader Date: Thu, 29 Nov 2012 18:00:27 +0000 Subject: [PATCH 01/24] [#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/24] [#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/24] [#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/24] [#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/24] [#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/24] [#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/24] [#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/24] [#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/24] [#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/24] [#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/24] [#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/24] [#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 d3edb7e4360140ab9ce4c507118fd508d61d5560 Mon Sep 17 00:00:00 2001 From: tobes Date: Wed, 19 Dec 2012 05:02:25 +0000 Subject: [PATCH 13/24] [#220] Improve imports in controller.user --- ckan/controllers/user.py | 70 ++++++++++++++++++++++++---------------- 1 file changed, 42 insertions(+), 28 deletions(-) diff --git a/ckan/controllers/user.py b/ckan/controllers/user.py index 344475c8e7e..f3184aa0fc5 100644 --- a/ckan/controllers/user.py +++ b/ckan/controllers/user.py @@ -1,28 +1,42 @@ import logging -from pylons import session - import genshi from urllib import quote import ckan.misc -import ckan.lib.i18n -from ckan.lib.base import * -from ckan.lib import mailer -import ckan.new_authz -from ckan.lib.navl.dictization_functions import DataError, unflatten -from ckan.logic import NotFound, NotAuthorized, ValidationError -from ckan.logic import check_access, get_action -from ckan.logic import tuplize_dict, clean_dict, parse_params -from ckan.logic.schema import user_new_form_schema, user_edit_form_schema -from ckan.lib.captcha import check_recaptcha, CaptchaError +from pylons import session, c, g, request, config +from pylons.i18n import _ + +import ckan.lib.i18n as i18n +import ckan.lib.base as base +import ckan.model as model +import ckan.lib.helpers as h +import ckan.new_authz as new_authz +import ckan.logic as logic +import ckan.logic.schema as schema +import ckan.lib.captcha as captcha +import ckan.lib.mailer as mailer +import ckan.lib.navl.dictization_functions as dictization_functions log = logging.getLogger(__name__) -class UserController(BaseController): +abort = base.abort +render = base.render +validate = base.validate + +check_access = logic.check_access +get_action = logic.get_action +NotFound = logic.NotFound +NotAuthorized = logic.NotAuthorized +ValidationError = logic.ValidationError + +DataError = dictization_functions.DataError +unflatten = dictization_functions.unflatten + +class UserController(base.BaseController): def __before__(self, action, **env): - BaseController.__before__(self, action, **env) + base.BaseController.__before__(self, action, **env) try: context = {'model': model, 'user': c.user or c.author} check_access('site_read', context) @@ -35,21 +49,21 @@ def __before__(self, action, **env): edit_user_form = 'user/edit_user_form.html' def _new_form_to_db_schema(self): - return user_new_form_schema() + return schema.user_new_form_schema() def _db_to_new_form_schema(self): '''This is an interface to manipulate data from the database into a format suitable for the form (optional)''' def _edit_form_to_db_schema(self): - return user_edit_form_schema() + return schema.user_edit_form_schema() def _db_to_edit_form_schema(self): '''This is an interface to manipulate data from the database into a format suitable for the form (optional)''' def _setup_template_variables(self, context, data_dict): - c.is_sysadmin = ckan.new_authz.is_sysadmin(c.user) + c.is_sysadmin = new_authz.is_sysadmin(c.user) try: user_dict = get_action('user_show')(context, data_dict) except NotFound: @@ -113,7 +127,7 @@ def read(self, id=None): # The legacy templates have the user's activity stream on the user # profile page, new templates do not. - if asbool(config.get('ckan.legacy_templates', False)): + if h.asbool(config.get('ckan.legacy_templates', False)): c.user_activity_stream = get_action('user_activity_list_html')( context, {'id': c.user_dict['id']}) @@ -156,16 +170,16 @@ def new(self, data=None, errors=None, error_summary=None): error_summary = error_summary or {} vars = {'data': data, 'errors': errors, 'error_summary': error_summary} - c.is_sysadmin = ckan.new_authz.is_sysadmin(c.user) + c.is_sysadmin = new_authz.is_sysadmin(c.user) c.form = render(self.new_user_form, extra_vars=vars) return render('user/new.html') def _save_new(self, context): try: - data_dict = clean_dict(unflatten( - tuplize_dict(parse_params(request.params)))) + data_dict = logic.clean_dict(unflatten( + logic.tuplize_dict(logic.parse_params(request.params)))) context['message'] = data_dict.get('log_message', '') - check_recaptcha(request) + captcha.check_recaptcha(request) user = get_action('user_create')(context, data_dict) except NotAuthorized: abort(401, _('Unauthorized to create user %s') % '') @@ -173,7 +187,7 @@ def _save_new(self, context): abort(404, _('User not found')) except DataError: abort(400, _(u'Integrity Error')) - except CaptchaError: + except captcha.CaptchaError: error_msg = _(u'Bad Captcha. Please try again.') h.flash_error(error_msg) return self.new(data_dict) @@ -232,7 +246,7 @@ def edit(self, id=None, data=None, errors=None, error_summary=None): user_obj = context.get('user_obj') - if not (ckan.new_authz.is_sysadmin(c.user) + if not (new_authz.is_sysadmin(c.user) or c.user == user_obj.name): abort(401, _('User %s not authorized to edit %s') % (str(c.user), id)) @@ -252,8 +266,8 @@ def edit(self, id=None, data=None, errors=None, error_summary=None): def _save_edit(self, id, context): try: - data_dict = clean_dict(unflatten( - tuplize_dict(parse_params(request.params)))) + data_dict = logic.clean_dict(unflatten( + logic.tuplize_dict(logic.parse_params(request.params)))) context['message'] = data_dict.get('log_message', '') data_dict['id'] = id user = get_action('user_update')(context, data_dict) @@ -305,7 +319,7 @@ def logged_in(self): # we need to set the language explicitly here or the flash # messages will not be translated. - ckan.lib.i18n.set_lang(lang) + i18n.set_lang(lang) if c.user: context = {'model': model, @@ -325,7 +339,7 @@ def logged_in(self): if g.openid_enabled: err += _(' (Or if using OpenID, it hasn\'t been associated ' 'with a user account.)') - if asbool(config.get('ckan.legacy_templates', 'false')): + if h.asbool(config.get('ckan.legacy_templates', 'false')): h.flash_error(err) h.redirect_to(locale=lang, controller='user', action='login', came_from=came_from) From 650ff43c4a74c8f28a3fcc70f051dabae2908108 Mon Sep 17 00:00:00 2001 From: tobes Date: Wed, 19 Dec 2012 05:02:42 +0000 Subject: [PATCH 14/24] [#220] Remove genshi dependency in controller user --- ckan/controllers/user.py | 14 +------------- 1 file changed, 1 insertion(+), 13 deletions(-) diff --git a/ckan/controllers/user.py b/ckan/controllers/user.py index f3184aa0fc5..39893153bbb 100644 --- a/ckan/controllers/user.py +++ b/ckan/controllers/user.py @@ -1,8 +1,6 @@ import logging -import genshi from urllib import quote -import ckan.misc from pylons import session, c, g, request, config from pylons.i18n import _ @@ -72,7 +70,7 @@ def _setup_template_variables(self, context, data_dict): abort(401, _('Not authorized to see this page')) c.user_dict = user_dict c.is_myself = user_dict['name'] == c.user - c.about_formatted = self._format_about(user_dict['about']) + c.about_formatted = h.render_markdown(user_dict['about']) ## end hooks @@ -460,16 +458,6 @@ def perform_reset(self, id): c.user_dict = user_dict return render('user/perform_reset.html') - def _format_about(self, about): - about_formatted = ckan.misc.MarkdownFormat().to_html(about) - try: - html = genshi.HTML(about_formatted) - except genshi.ParseError, e: - log.error('Could not print "about" field Field: %r Error: %r', - about, e) - html = _('Error: Could not parse About text') - return html - def _get_form_password(self): password1 = request.params.getone('password1') password2 = request.params.getone('password2') From f3c9fd69c031da7e13a78d7f4a80857ffcd242a6 Mon Sep 17 00:00:00 2001 From: Dominik Moritz Date: Fri, 21 Dec 2012 14:21:49 +0100 Subject: [PATCH 15/24] [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 16/24] [#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 17/24] [#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 2165bdaa214995c1f4998633382ac7ad4fac3519 Mon Sep 17 00:00:00 2001 From: tobes Date: Tue, 22 Jan 2013 09:06:01 +0000 Subject: [PATCH 18/24] [#220] Fix asbool callw --- ckan/controllers/user.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/ckan/controllers/user.py b/ckan/controllers/user.py index 9246f849059..25ca86abac6 100644 --- a/ckan/controllers/user.py +++ b/ckan/controllers/user.py @@ -254,7 +254,7 @@ def edit(self, id=None, data=None, errors=None, error_summary=None): data_dict) c.is_myself = True - c.show_email_notifications = asbool( + c.show_email_notifications = h.asbool( config.get('ckan.activity_streams_email_notifications')) c.form = render(self.edit_user_form, extra_vars=vars) From a1d4d3cad76fa66e3cadbe241f61da2e091abbf0 Mon Sep 17 00:00:00 2001 From: tobes Date: Tue, 22 Jan 2013 09:25:38 +0000 Subject: [PATCH 19/24] [#220] Fix broken tests due to markdown processing --- ckan/controllers/user.py | 14 +++++++++++++- 1 file changed, 13 insertions(+), 1 deletion(-) diff --git a/ckan/controllers/user.py b/ckan/controllers/user.py index 25ca86abac6..3f21d706ee3 100644 --- a/ckan/controllers/user.py +++ b/ckan/controllers/user.py @@ -3,9 +3,11 @@ from pylons import session, c, g, request, config from pylons.i18n import _ +import genshi import ckan.lib.i18n as i18n import ckan.lib.base as base +import ckan.misc as misc import ckan.model as model import ckan.lib.helpers as h import ckan.new_authz as new_authz @@ -70,7 +72,7 @@ def _setup_template_variables(self, context, data_dict): abort(401, _('Not authorized to see this page')) c.user_dict = user_dict c.is_myself = user_dict['name'] == c.user - c.about_formatted = h.render_markdown(user_dict['about']) + c.about_formatted = self._format_about(user_dict['about']) ## end hooks @@ -545,3 +547,13 @@ def unfollow(self, id): or e.error_dict) h.flash_error(error_message) h.redirect_to(controller='user', action='read', id=id) + + def _format_about(self, about): + about_formatted = misc.MarkdownFormat().to_html(about) + try: + html = genshi.HTML(about_formatted) + except genshi.ParseError, e: + log.error('Could not print "about" field Field: %r Error: %r', + about, e) + html = _('Error: Could not parse About text') + return html From 94e21ce3676a6a261c80c5147a8c68619f7cb030 Mon Sep 17 00:00:00 2001 From: John Glover Date: Tue, 22 Jan 2013 10:48:42 +0100 Subject: [PATCH 20/24] [#220] Some PEP8 fixes for user controller. --- ckan/controllers/user.py | 17 +++++++++-------- 1 file changed, 9 insertions(+), 8 deletions(-) diff --git a/ckan/controllers/user.py b/ckan/controllers/user.py index 3f21d706ee3..d4eea07fcc1 100644 --- a/ckan/controllers/user.py +++ b/ckan/controllers/user.py @@ -33,6 +33,7 @@ DataError = dictization_functions.DataError unflatten = dictization_functions.unflatten + class UserController(base.BaseController): def __before__(self, action, **env): @@ -72,7 +73,7 @@ def _setup_template_variables(self, context, data_dict): abort(401, _('Not authorized to see this page')) c.user_dict = user_dict c.is_myself = user_dict['name'] == c.user - c.about_formatted = self._format_about(user_dict['about']) + c.about_formatted = self._format_about(user_dict['about']) ## end hooks @@ -127,17 +128,17 @@ def read(self, id=None): # profile page, new templates do not. if h.asbool(config.get('ckan.legacy_templates', False)): c.user_activity_stream = get_action('user_activity_list_html')( - context, {'id': c.user_dict['id']}) + context, {'id': c.user_dict['id']}) return render('user/read.html') def me(self, locale=None): if not c.user: h.redirect_to(locale=locale, controller='user', action='login', - id=None) + id=None) user_ref = c.userobj.get_reference_preferred_for_uri() h.redirect_to(locale=locale, controller='user', action='dashboard', - id=user_ref) + id=user_ref) def register(self, data=None, errors=None, error_summary=None): return self.new(data, errors, error_summary) @@ -257,7 +258,7 @@ def edit(self, id=None, data=None, errors=None, error_summary=None): c.is_myself = True c.show_email_notifications = h.asbool( - config.get('ckan.activity_streams_email_notifications')) + config.get('ckan.activity_streams_email_notifications')) c.form = render(self.edit_user_form, extra_vars=vars) return render('user/edit.html') @@ -307,7 +308,7 @@ def login(self, error=None): self._get_repoze_handler('login_handler_path'), came_from=came_from) if error: - vars = {'error_summary': {'':error}} + vars = {'error_summary': {'': error}} else: vars = {} return render('user/login.html', extra_vars=vars) @@ -524,7 +525,7 @@ def follow(self, id): h.flash_success(_("You are now following {0}").format(id)) except ValidationError as e: error_message = (e.extra_msg or e.message or e.error_summary - or e.error_dict) + or e.error_dict) h.flash_error(error_message) except NotAuthorized as e: h.flash_error(e.extra_msg) @@ -544,7 +545,7 @@ def unfollow(self, id): h.flash_error(error_message) except ValidationError as e: error_message = (e.error_summary or e.message or e.extra_msg - or e.error_dict) + or e.error_dict) h.flash_error(error_message) h.redirect_to(controller='user', action='read', id=id) From c4e26bfea4d14d4646f289af059b9ca7ed2927e0 Mon Sep 17 00:00:00 2001 From: tobes Date: Wed, 23 Jan 2013 04:48:47 +0000 Subject: [PATCH 21/24] [#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 22/24] [#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 23/24] [#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 From 45c6d0e30c2d9b7749ec5bd9b10fb977b91ff745 Mon Sep 17 00:00:00 2001 From: Sean Hammond Date: Wed, 23 Jan 2013 21:06:44 +0100 Subject: [PATCH 24/24] Call setup_template_variables() in new_metadata() Fixes #297 --- ckan/controllers/package.py | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/ckan/controllers/package.py b/ckan/controllers/package.py index edab558dfaa..08a5527eece 100644 --- a/ckan/controllers/package.py +++ b/ckan/controllers/package.py @@ -722,6 +722,11 @@ def new_metadata(self, id, data=None, errors=None, error_summary=None): error_summary = error_summary or {} vars = {'data': data, 'errors': errors, 'error_summary': error_summary} vars['pkg_name'] = id + + package_type = self._get_package_type(id) + self._setup_template_variables(context, {}, + package_type=package_type) + return render('package/new_package_metadata.html', extra_vars=vars) def edit(self, id, data=None, errors=None, error_summary=None):