diff --git a/ckan/lib/create_test_data.py b/ckan/lib/create_test_data.py index 80f32a1038d..2ca59a42fcc 100644 --- a/ckan/lib/create_test_data.py +++ b/ckan/lib/create_test_data.py @@ -221,19 +221,23 @@ def create_arbitrary(cls, package_dicts, relationships=[], group = model.Group.by_name(unicode(group_name)) if not group: if not group_name in new_groups: - group = model.Group(name=unicode(group_name)) + group = model.Group(name= + unicode(group_name)) model.Session.add(group) new_group_names.add(group_name) new_groups[group_name] = group else: - # If adding multiple packages with the same group name, - # model.Group.by_name will not find the group as the - # session has not yet been committed at this point. - # Fetch from the new_groups dict instead. + # If adding multiple packages with the same + # group name, model.Group.by_name will not + # find the group as the session has not yet + # been committed at this point. Fetch from + # the new_groups dict instead. group = new_groups[group_name] - capacity = 'organization' if group.is_organization \ + capacity = 'organization' if group.is_organization\ else 'public' - member = model.Member(group=group, table_id=pkg.id, table_name='package', capacity=capacity) + member = model.Member(group=group, table_id=pkg.id, + table_name='package', + capacity=capacity) model.Session.add(member) if group.is_organization: pkg.owner_org = group.id @@ -338,8 +342,8 @@ def create_groups(cls, group_dicts, admin_user_name=None, auth_profile=""): 'type', 'is_organization')) for group_dict in group_dicts: if model.Group.by_name(unicode(group_dict['name'])): - log.warning('Cannot create group "%s" as it already exists.' % \ - (group_dict['name'])) + log.warning('Cannot create group "%s" as it already exists.' % + group_dict['name']) continue pkg_names = group_dict.pop('packages', []) group = model.Group(name=unicode(group_dict['name'])) @@ -353,18 +357,19 @@ def create_groups(cls, group_dicts, admin_user_name=None, auth_profile=""): for pkg_name in pkg_names: pkg = model.Package.by_name(unicode(pkg_name)) assert pkg, pkg_name - member = model.Member(group=group, table_id=pkg.id, table_name='package') + member = model.Member(group=group, table_id=pkg.id, + table_name='package') model.Session.add(member) model.Session.add(group) - admins = [model.User.by_name(user_name) \ - for user_name in group_dict.get('admins', [])] \ - + admin_users + admins = [model.User.by_name(user_name) + for user_name in group_dict.get('admins', [])] + \ + admin_users for admin in admins: member = model.Member(group=group, table_id=admin.id, table_name='user', capacity='admin') model.Session.add(member) - editors = [model.User.by_name(user_name) \ - for user_name in group_dict.get('editors', [])] + editors = [model.User.by_name(user_name) + for user_name in group_dict.get('editors', [])] for editor in editors: member = model.Member(group=group, table_id=editor.id, table_name='user', capacity='editor') @@ -400,7 +405,8 @@ def create(cls, auth_profile="", package_type=None): * Associated tags, etc etc ''' if auth_profile == "publisher": - organization_group = model.Group(name=u"organization_group", type="organization") + organization_group = model.Group(name=u"organization_group", + type="organization") cls.pkg_names = [u'annakarenina', u'warandpeace'] pkg1 = model.Package(name=cls.pkg_names[0], type=package_type) @@ -525,7 +531,6 @@ def create(cls, auth_profile="", package_type=None): model.repo.commit_and_remove() - # method used in DGU and all good tests elsewhere @classmethod def create_users(cls, user_dicts): @@ -539,9 +544,11 @@ def create_users(cls, user_dicts): @classmethod def _create_user_without_commit(cls, name='', **user_dict): - if model.User.by_name(name) or (user_dict.get('open_id') and model.User.by_openid(user_dict.get('openid'))): - log.warning('Cannot create user "%s" as it already exists.' % \ - (name or user_dict['name'])) + if model.User.by_name(name) or \ + (user_dict.get('open_id') and + model.User.by_openid(user_dict.get('openid'))): + log.warning('Cannot create user "%s" as it already exists.' % + name or user_dict['name']) return # User objects are not revisioned so no need to create a revision user_ref = name or user_dict['openid'] diff --git a/ckan/logic/validators.py b/ckan/logic/validators.py index 0fb3887798e..429fcb5a774 100644 --- a/ckan/logic/validators.py +++ b/ckan/logic/validators.py @@ -36,8 +36,9 @@ def owner_org_validator(key, data, errors, context): group_id = group.id user = context['user'] user = model.User.get(user) - if not(user.sysadmin or new_authz.has_user_permission_for_group_or_org( - group_id, user.name, 'create_dataset')): + if not(user.sysadmin or + new_authz.has_user_permission_for_group_or_org( + group_id, user.name, 'create_dataset')): raise Invalid(_('You cannot add a dataset to this organization')) data[key] = group_id diff --git a/ckan/model/group.py b/ckan/model/group.py index a25cebf9927..ee637dde82f 100644 --- a/ckan/model/group.py +++ b/ckan/model/group.py @@ -103,15 +103,19 @@ def related_packages(self): def __unicode__(self): # refer to objects by name, not ID, to help debugging if self.table_name == 'package': - table_info = 'package=%s' % meta.Session.query(_package.Package).get(self.table_id).name + table_info = 'package=%s' % meta.Session.query(_package.Package).\ + get(self.table_id).name elif self.table_name == 'group': - table_info = 'group=%s' % meta.Session.query(Group).get(self.table_id).name + table_info = 'group=%s' % meta.Session.query(Group).\ + get(self.table_id).name else: - table_info = 'table_name=%s table_id=%s' % (self.table_name, self.table_id) + table_info = 'table_name=%s table_id=%s' % (self.table_name, + self.table_id) return u'' % \ (self.group.name if self.group else repr(self.group), table_info, self.capacity, self.state) + class Group(vdm.sqlalchemy.RevisionedObjectMixin, vdm.sqlalchemy.StatefulObjectMixin, domain_object.DomainObject): @@ -172,10 +176,10 @@ def get_children_groups(self, type='group'): '''Returns the groups one level underneath this group in the hierarchy. Groups come in a list of dicts, each keyed by "id", "name" and "title". ''' - # The original intention of this method was to provide the full depth of - # the tree, but the CTE was incorrect. This new query does what that old CTE - # actually did, but is now far simpler. - results = meta.Session.query(Group.id, Group.name, Group.title).\ + # The original intention of this method was to provide the full depth + # of the tree, but the CTE was incorrect. This new query does what that + # old CTE actually did, but is now far simpler. + results = meta.Session.query(Group.id, Group.name, Group.title).\ filter_by(type=type).\ filter_by(state='active').\ join(Member, Member.table_id == Group.id).\ @@ -184,12 +188,13 @@ def get_children_groups(self, type='group'): filter_by(state='active').\ all() - return [{'id':id_, 'name': name, 'title': title} + return [{'id': id_, 'name': name, 'title': title} for id_, name, title in results] def get_children_group_hierarchy(self, type='group'): - '''Returns the groups in all levels underneath this group in the hierarchy. - The ordering is such that children always come after their parent. + '''Returns the groups in all levels underneath this group in the + hierarchy. The ordering is such that children always come after their + parent. :rtype: a list of tuples, each one a Group and the ID its their parent group. @@ -200,14 +205,16 @@ def get_children_group_hierarchy(self, type='group'): (, u'd2e25b41-720c-4ba7-bc8f-bb34b185b3dd')] ''' results = meta.Session.query(Group, 'parent_id').\ - from_statement(HIERARCHY_DOWNWARDS_CTE).params(id=self.id, type=type).all() + from_statement(HIERARCHY_DOWNWARDS_CTE).\ + params(id=self.id, type=type).all() return results def get_parent_group_hierarchy(self, type='group'): - '''Returns this group's parent, parent's parent, parent's parent's parent - etc.. Sorted with the top level parent first.''' + '''Returns this group's parent, parent's parent, parent's parent's + parent etc.. Sorted with the top level parent first.''' return meta.Session.query(Group).\ - from_statement(HIERARCHY_UPWARDS_CTE).params(id=self.id, type=type).all() + from_statement(HIERARCHY_UPWARDS_CTE).\ + params(id=self.id, type=type).all() @classmethod def get_top_level_groups(cls, type='group'): @@ -215,12 +222,12 @@ def get_top_level_groups(cls, type='group'): no parent groups. Groups are sorted by title. ''' return meta.Session.query(cls).\ - outerjoin(Member, Member.table_id == Group.id and \ - Member.table_name == 'group' and \ - Member.state == 'active').\ - filter(Member.id==None).\ - filter(Group.type==type).\ - order_by(Group.title).all() + outerjoin(Member, Member.table_id == Group.id and + Member.table_name == 'group' and + Member.state == 'active').\ + filter(Member.id == None).\ + filter(Group.type == type).\ + order_by(Group.title).all() def packages(self, with_private=False, limit=None, return_query=False, context=None): @@ -381,7 +388,6 @@ def __repr__(self): MemberRevision.related_packages = lambda self: [self.continuity.package] - HIERARCHY_DOWNWARDS_CTE = """WITH RECURSIVE child AS ( -- non-recursive term @@ -404,7 +410,8 @@ def __repr__(self): UNION -- recursive term SELECT PG.depth + 1, M.* FROM parenttree PG, public.member M - WHERE PG.group_id = M.table_id AND M.table_name = 'group' AND M.state = 'active' + WHERE PG.group_id = M.table_id AND M.table_name = 'group' + AND M.state = 'active' ) SELECT G.*, PT.depth FROM parenttree AS PT diff --git a/ckan/new_authz.py b/ckan/new_authz.py index 370b19fb581..2f612c82b41 100644 --- a/ckan/new_authz.py +++ b/ckan/new_authz.py @@ -213,7 +213,9 @@ def get_roles_with_permission(permission): def has_user_permission_for_group_or_org(group_id, user_name, permission): ''' Check if the user has the given permissions for the group, - allowing for sysadmin rights and permission cascading down groups. ''' + allowing for sysadmin rights and permission cascading down groups. + + ''' if not group_id: return False group = model.Group.get(group_id) @@ -234,15 +236,19 @@ def has_user_permission_for_group_or_org(group_id, user_name, permission): # in the group hierarchy for permission. for capacity in check_config_permission('roles_that_cascade_to_sub_groups'): parent_groups = group.get_parent_group_hierarchy(type=group.type) - group_ids = [group.id for group in parent_groups] + group_ids = [group_.id for group_ in parent_groups] if _has_user_permission_for_groups(user_id, permission, group_ids, capacity=capacity): return True return False -def _has_user_permission_for_groups(user_id, permission, group_ids, capacity=None): + +def _has_user_permission_for_groups(user_id, permission, group_ids, + capacity=None): ''' Check if the user has the given permissions for the particular - group. Can also be filtered by a particular capacity''' + group. Can also be filtered by a particular capacity. + + ''' # get any roles the user has for the group q = model.Session.query(model.Member) \ .filter(model.Member.group_id.in_(group_ids)) \ diff --git a/ckan/tests/functional/test_group.py b/ckan/tests/functional/test_group.py index 4f4fddc3e05..5575bd6e4ba 100644 --- a/ckan/tests/functional/test_group.py +++ b/ckan/tests/functional/test_group.py @@ -2,7 +2,6 @@ from nose.tools import assert_equal -import ckan.tests.test_plugins as test_plugins import ckan.model as model import ckan.lib.search as search @@ -15,7 +14,6 @@ from ckan.tests import is_search_supported - class TestGroup(FunctionalTestCase): @classmethod @@ -59,14 +57,14 @@ def test_atom_feed_page_negative(self): assert '"page" parameter must be a positive integer' in res, res def test_children(self): - if model.engine_is_sqlite() : + if model.engine_is_sqlite(): from nose import SkipTest raise SkipTest("Can't use CTE for sqlite") group_name = 'deletetest' CreateTestData.create_groups([{'name': group_name, 'packages': []}, - {'name': "parent_group", + {'name': "parent_group", 'packages': []}], admin_user_name='testsysadmin') @@ -155,7 +153,6 @@ def test_sorting(self): assert results[0]['name'] == u'beta', results[0]['name'] assert results[1]['name'] == u'delta', results[1]['name'] - def test_mainmenu(self): # the home page does a package search so have to skip this test if # search is not supported @@ -205,14 +202,16 @@ def test_read_and_authorized_to_edit(self): title = u'Dave\'s books' pkgname = u'warandpeace' offset = url_for(controller='group', action='read', id=name) - res = self.app.get(offset, extra_environ={'REMOTE_USER': 'testsysadmin'}) + res = self.app.get(offset, + extra_environ={'REMOTE_USER': 'testsysadmin'}) assert title in res, res assert 'edit' in res assert name in res def test_new_page(self): offset = url_for(controller='group', action='new') - res = self.app.get(offset, extra_environ={'REMOTE_USER': 'testsysadmin'}) + res = self.app.get(offset, + extra_environ={'REMOTE_USER': 'testsysadmin'}) assert 'Add A Group' in res, res @@ -265,7 +264,6 @@ def setup_class(self): model.Session.add(model.Package(name=self.packagename)) model.repo.commit_and_remove() - @classmethod def teardown_class(self): model.Session.remove() diff --git a/ckan/tests/test_coding_standards.py b/ckan/tests/test_coding_standards.py index f0b7469c3f4..4481342acd5 100644 --- a/ckan/tests/test_coding_standards.py +++ b/ckan/tests/test_coding_standards.py @@ -751,7 +751,6 @@ class TestPep8(object): 'ckan/tests/functional/test_cors.py', 'ckan/tests/functional/test_error.py', 'ckan/tests/functional/test_follow.py', - 'ckan/tests/functional/test_group.py', 'ckan/tests/functional/test_home.py', 'ckan/tests/functional/test_package.py', 'ckan/tests/functional/test_package_relationships.py',