Skip to content

Commit

Permalink
[#1038] PEP8 fixes only.
Browse files Browse the repository at this point in the history
  • Loading branch information
David Read committed Sep 6, 2013
1 parent 7ce1555 commit f7f5049
Show file tree
Hide file tree
Showing 6 changed files with 75 additions and 57 deletions.
47 changes: 27 additions & 20 deletions ckan/lib/create_test_data.py
Expand Up @@ -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
Expand Down Expand Up @@ -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']))
Expand All @@ -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')
Expand Down Expand Up @@ -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)
Expand Down Expand Up @@ -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):
Expand All @@ -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']
Expand Down
5 changes: 3 additions & 2 deletions ckan/logic/validators.py
Expand Up @@ -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

Expand Down
51 changes: 29 additions & 22 deletions ckan/model/group.py
Expand Up @@ -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'<Member group=%s %s capacity=%s state=%s>' % \
(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):
Expand Down Expand Up @@ -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).\
Expand All @@ -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.
Expand All @@ -200,27 +205,29 @@ def get_children_group_hierarchy(self, type='group'):
(<Group newport-ccg>, 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'):
'''Returns a list of the groups (of the specified type) which have
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):
Expand Down Expand Up @@ -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
Expand All @@ -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
Expand Down
14 changes: 10 additions & 4 deletions ckan/new_authz.py
Expand Up @@ -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)
Expand All @@ -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)) \
Expand Down
14 changes: 6 additions & 8 deletions ckan/tests/functional/test_group.py
Expand Up @@ -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

Expand All @@ -15,7 +14,6 @@
from ckan.tests import is_search_supported



class TestGroup(FunctionalTestCase):

@classmethod
Expand Down Expand Up @@ -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')

Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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


Expand Down Expand Up @@ -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()
Expand Down
1 change: 0 additions & 1 deletion ckan/tests/test_coding_standards.py
Expand Up @@ -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',
Expand Down

0 comments on commit f7f5049

Please sign in to comment.