diff --git a/ckan/lib/helpers.py b/ckan/lib/helpers.py index bc973e89ba3..e695da925c6 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 import ckan.lib.datapreview as datapreview get_available_locales = i18n.get_available_locales @@ -294,32 +295,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 - if kwargs.get('inner_span'): - text = literal('') + text + literal('') - 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) - if kwargs.pop('condition', True): - link = link_to( - text, - url_for(**kwargs), - class_=class_ - ) - else: - link = '' - return link - - def _link_active(kwargs): ''' creates classes for the link_to calls ''' highlight_actions = kwargs.get('highlight_actions', @@ -328,50 +303,70 @@ def _link_active(kwargs): and c.action in highlight_actions) -def _link_class(kwargs, suppress_active_class=False): - ''' creates classes for the link_to calls ''' - 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 -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, - url_for(name, **kwargs), + _create_link_text(text, **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): + '''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 - 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 ' + '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 nav_named_link(text, routename, **kwargs) def build_nav_main(*args): @@ -461,21 +456,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. diff --git a/ckan/logic/action/create.py b/ckan/logic/action/create.py index 2a9570d6bf1..b9f1aca8679 100644 --- a/ckan/logic/action/create.py +++ b/ckan/logic/action/create.py @@ -148,6 +148,7 @@ 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 context_org_update = context.copy() context_org_update['ignore_auth'] = True @@ -159,6 +160,8 @@ def package_create(context, data_dict): for item in plugins.PluginImplementations(plugins.IPackageController): item.create(pkg) + item.after_create(context, data) + if not context.get('defer_commit'): model.repo.commit() diff --git a/ckan/logic/action/delete.py b/ckan/logic/action/delete.py index c104cde1a85..59e8f1a6f64 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/logic/action/get.py b/ckan/logic/action/get.py index d728a2087f8..a1314caf458 100644 --- a/ckan/logic/action/get.py +++ b/ckan/logic/action/get.py @@ -736,6 +736,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/logic/action/update.py b/ckan/logic/action/update.py index 91f375cb922..c5d8e605f46 100644 --- a/ckan/logic/action/update.py +++ b/ckan/logic/action/update.py @@ -276,6 +276,9 @@ def package_update(context, data_dict): for item in plugins.PluginImplementations(plugins.IPackageController): item.edit(pkg) + + item.after_update(context, data) + if not context.get('defer_commit'): model.repo.commit() diff --git a/ckan/logic/auth/update.py b/ckan/logic/auth/update.py index da02b2d1009..53f4ca58dfe 100644 --- a/ckan/logic/auth/update.py +++ b/ckan/logic/auth/update.py @@ -15,9 +15,16 @@ def package_update(context, data_dict): package = get_package_object(context, data_dict) 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: - check1 = new_authz.check_config_permission('create_dataset_if_not_in_organization') + # 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('anon_create_dataset') if not check1: return {'success': False, 'msg': _('User %s not authorized to edit package %s') % (str(user), package.id)} else: 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): diff --git a/ckan/plugins/interfaces.py b/ckan/plugins/interfaces.py index ce2782f63ba..91e8c71a889 100644 --- a/ckan/plugins/interfaces.py +++ b/ckan/plugins/interfaces.py @@ -335,6 +335,38 @@ def authz_remove_role(self, object_role): def delete(self, entity): pass + 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 + 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, 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 + domain object, which may not include all fields). + ''' + 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 + 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 553dd837ca5..1a5493f3f4b 100644 --- a/ckan/tests/functional/test_package.py +++ b/ckan/tests/functional/test_package.py @@ -58,13 +58,31 @@ 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, context, data_dict): + self.calls['after_create'] += 1 + self.id_in_dict = 'id' in data_dict + + return data_dict + + 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 def update_facet_titles(self, facet_titles): return facet_titles @@ -374,12 +392,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): @@ -948,6 +962,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') @@ -1041,6 +1075,46 @@ 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'testsysadmin') + + 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, status=401) + + self.app.post(offset, extra_environ=self.extra_environ_admin) + + 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 = [] @@ -1279,6 +1353,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' 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