From 35fc3ed37b600570b5a294cf9621c1400bd48bf1 Mon Sep 17 00:00:00 2001 From: David Read Date: Fri, 25 May 2012 16:50:17 +0100 Subject: [PATCH 01/87] [xs] Helpful SOLR hint. --- doc/solr-setup.rst | 12 ++++++++++++ 1 file changed, 12 insertions(+) diff --git a/doc/solr-setup.rst b/doc/solr-setup.rst index 395da67e0a9..c7c2f53ee2d 100644 --- a/doc/solr-setup.rst +++ b/doc/solr-setup.rst @@ -192,6 +192,18 @@ Some problems that can be found during the install: ${dataDir} +* When running Solr it says `Unable to find a javac compiler; com.sun.tools.javac.Main is not on the classpath. Perhaps JAVA_HOME does not point to the JDK.` + + See the note above about JAVA_HOME. Alternatively you may not have installed the JDK. Check by seeing if javac is installed:: + + which javac + + If it isn't do:: + + sudo apt-get install openjdk-6-jdk + + and restart SOLR. + Handling changes in the CKAN schema ----------------------------------- From 04ca4191673010e86ac0d77991cb7967fe6c58ae Mon Sep 17 00:00:00 2001 From: David Read Date: Fri, 15 Jun 2012 13:45:46 +0100 Subject: [PATCH 02/87] Fix minor bug that caused create_users to not commit changes. --- ckan/lib/create_test_data.py | 1 + 1 file changed, 1 insertion(+) diff --git a/ckan/lib/create_test_data.py b/ckan/lib/create_test_data.py index 53743e6c2cc..1b8d3b601a2 100644 --- a/ckan/lib/create_test_data.py +++ b/ckan/lib/create_test_data.py @@ -537,6 +537,7 @@ def _create_user_without_commit(cls, name='', **user_dict): user = model.User(name=unicode(name), **user_dict) model.Session.add(user) cls.user_refs.append(user_ref) + return user @classmethod def create_user(cls, name='', **kwargs): From 6780e91351915bf0a020c93cae67d597c406c348 Mon Sep 17 00:00:00 2001 From: David Read Date: Fri, 15 Jun 2012 13:46:04 +0100 Subject: [PATCH 03/87] Improve logging in useful places. --- ckan/controllers/package.py | 2 ++ ckan/lib/authenticator.py | 6 ++++++ ckan/lib/base.py | 3 +++ ckan/lib/search/__init__.py | 13 +++++++------ 4 files changed, 18 insertions(+), 6 deletions(-) diff --git a/ckan/controllers/package.py b/ckan/controllers/package.py index 9403a0271e3..a7df2cf1849 100644 --- a/ckan/controllers/package.py +++ b/ckan/controllers/package.py @@ -592,6 +592,8 @@ def _save_new(self, context, package_type=None): def _save_edit(self, name_or_id, context): from ckan.lib.search import SearchIndexError + log.debug('Package save request name: %s POST: %r', + name_or_id, request.POST) try: data_dict = clean_dict(unflatten( tuplize_dict(parse_params(request.POST)))) diff --git a/ckan/lib/authenticator.py b/ckan/lib/authenticator.py index b56711a3427..6f061caad91 100644 --- a/ckan/lib/authenticator.py +++ b/ckan/lib/authenticator.py @@ -1,8 +1,12 @@ +import logging + from zope.interface import implements from repoze.who.interfaces import IAuthenticator from ckan.model import User, Session +log = logging.getLogger(__name__) + class OpenIDAuthenticator(object): implements(IAuthenticator) @@ -25,8 +29,10 @@ def authenticate(self, environ, identity): return None user = User.by_name(identity.get('login')) if user is None: + log.debug('Login failed - username %r not found', identity.get('login')) return None if user.validate_password(identity.get('password')): return user.name + log.debug('Login as %r failed - password not valid', identity.get('login')) return None diff --git a/ckan/lib/base.py b/ckan/lib/base.py index f2cc447fdea..52604d48e7f 100644 --- a/ckan/lib/base.py +++ b/ckan/lib/base.py @@ -29,6 +29,8 @@ from ckan.lib.helpers import json import ckan.model as model +log = logging.getLogger(__name__) + PAGINATE_ITEMS_PER_PAGE = 50 APIKEY_HEADER_NAME_KEY = 'apikey_header_name' @@ -139,6 +141,7 @@ def render_template(): response.headers["Cache-Control"] = "private" # Prevent any further rendering from being cached. request.environ['__no_cache__'] = True + log.debug('Template cache-control: %s' % response.headers["Cache-Control"]) # Render Time :) try: diff --git a/ckan/lib/search/__init__.py b/ckan/lib/search/__init__.py index abc78b798fd..b68e7850c47 100644 --- a/ckan/lib/search/__init__.py +++ b/ckan/lib/search/__init__.py @@ -128,11 +128,11 @@ def rebuild(package_id=None,only_missing=False,force=False,refresh=False): If a dataset id is provided, only this dataset will be reindexed. When reindexing all datasets, if only_missing is True, only the datasets not already indexed will be processed. If force equals - True, if an execption is found, the exception will be logged, but + True, if an exception is found, the exception will be logged, but the process will carry on. ''' from ckan import model - log.debug("Rebuilding search index...") + log.info("Rebuilding search index...") package_index = index_for(model.Package) @@ -140,21 +140,22 @@ def rebuild(package_id=None,only_missing=False,force=False,refresh=False): pkg_dict = get_action('package_show')( {'model': model, 'ignore_auth': True, 'validate': False}, {'id': package_id}) + log.info('Indexing just package %r...', pkg_dict['name']) package_index.remove_dict(pkg_dict) package_index.insert_dict(pkg_dict) else: package_ids = [r[0] for r in model.Session.query(model.Package.id).filter(model.Package.state == 'active').all()] if only_missing: - log.debug('Indexing only missing packages...') + log.info('Indexing only missing packages...') package_query = query_for(model.Package) indexed_pkg_ids = set(package_query.get_all_entity_ids(max_results=len(package_ids))) package_ids = set(package_ids) - indexed_pkg_ids # Packages not indexed if len(package_ids) == 0: - log.debug('All datasets are already indexed') + log.info('All datasets are already indexed') return else: - log.debug('Rebuilding the whole index...') + log.info('Rebuilding the whole index...') # When refreshing, the index is not previously cleared if not refresh: package_index.clear() @@ -176,7 +177,7 @@ def rebuild(package_id=None,only_missing=False,force=False,refresh=False): raise model.Session.commit() - log.debug('Finished rebuilding search index.') + log.info('Finished rebuilding search index.') def check(): from ckan import model From 75ba31027650b1ff3af3c8dc16cc11663f515350 Mon Sep 17 00:00:00 2001 From: David Read Date: Thu, 21 Jun 2012 11:22:36 +0100 Subject: [PATCH 04/87] Comment required, else Toby would just delete the method out of hand. --- ckan/lib/create_test_data.py | 1 + 1 file changed, 1 insertion(+) diff --git a/ckan/lib/create_test_data.py b/ckan/lib/create_test_data.py index 1b8d3b601a2..7b4a7815359 100644 --- a/ckan/lib/create_test_data.py +++ b/ckan/lib/create_test_data.py @@ -511,6 +511,7 @@ 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): needs_commit = False From 98888d2b5a46945a96b1d4719718c836a6a15e60 Mon Sep 17 00:00:00 2001 From: David Read Date: Mon, 22 Jul 2013 17:43:45 +0100 Subject: [PATCH 05/87] [#1038] Correct copyright sign for UTF8 file and cut/paste error. --- ckan/config/environment.py | 2 +- ckan/lib/plugins.py | 4 ++-- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/ckan/config/environment.py b/ckan/config/environment.py index 7a2dd27682d..fd8b46fa98d 100644 --- a/ckan/config/environment.py +++ b/ckan/config/environment.py @@ -237,7 +237,7 @@ def template_loaded(template): ''' This code is based on Genshi code - Copyright © 2006-2012 Edgewall Software + Copyright © 2006-2012 Edgewall Software All rights reserved. Redistribution and use in source and binary forms, with or diff --git a/ckan/lib/plugins.py b/ckan/lib/plugins.py index f60b928f434..1bf87ac29cb 100644 --- a/ckan/lib/plugins.py +++ b/ckan/lib/plugins.py @@ -104,7 +104,7 @@ def register_group_plugins(map): """ Register the various IGroupForm instances. - This method will setup the mappings between package types and the + This method will setup the mappings between group types and the registered IGroupForm instances. If it's called more than once an exception will be raised. """ @@ -149,7 +149,7 @@ def register_group_plugins(map): if group_type in _group_plugins: raise ValueError, "An existing IGroupForm is "\ - "already associated with the package type "\ + "already associated with the group type "\ "'%s'" % group_type _group_plugins[group_type] = plugin From 21cd0ac33023d13127a1e66094334de9d53b8b24 Mon Sep 17 00:00:00 2001 From: David Read Date: Mon, 22 Jul 2013 17:51:18 +0100 Subject: [PATCH 06/87] [#1038] Fix to allow sqlite testing - for until the tests are overhaulled. --- ckan/config/environment.py | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/ckan/config/environment.py b/ckan/config/environment.py index fd8b46fa98d..49348789369 100644 --- a/ckan/config/environment.py +++ b/ckan/config/environment.py @@ -353,6 +353,8 @@ def genshi_lookup_attr(cls, obj, key): # Here we create the site user if they are not already in the database try: logic.get_action('get_site_user')({'ignore_auth': True}, None) - except sqlalchemy.exc.ProgrammingError: - # The database is not initialised. This is a bit dirty. + except (sqlalchemy.exc.ProgrammingError, sqlalchemy.exc.OperationalError): + # (ProgrammingError for Postgres, OperationalError for SQLite) + # The database is not initialised. This is a bit dirty. This occurs + # when running tests. pass From 96f0a3e775e14207cd1eec3b82f0e05a1f5a48ed Mon Sep 17 00:00:00 2001 From: David Read Date: Mon, 22 Jul 2013 17:52:48 +0100 Subject: [PATCH 07/87] [#1038] Test fixtures for organization hierarchy. --- ckan/lib/cli.py | 4 +- ckan/lib/create_test_data.py | 81 +++++++++++++++++++++++++++++++++--- 2 files changed, 79 insertions(+), 6 deletions(-) diff --git a/ckan/lib/cli.py b/ckan/lib/cli.py index bf4654e2e09..0acae3550bb 100644 --- a/ckan/lib/cli.py +++ b/ckan/lib/cli.py @@ -1273,7 +1273,7 @@ class CreateTestDataCommand(CkanCommand): translations of terms create-test-data vocabs - annakerenina, warandpeace, and some test vocabularies - + create-test-data hierarchy - hierarchy of groups ''' summary = __doc__.split('\n')[0] usage = __doc__ @@ -1309,6 +1309,8 @@ def command(self): CreateTestData.create_translations_test_data() elif cmd == 'vocabs': CreateTestData.create_vocabs_test_data() + elif cmd == 'hierarchy': + CreateTestData.create_group_hierarchy_test_data() else: print 'Command %s not recognized' % cmd raise NotImplementedError diff --git a/ckan/lib/create_test_data.py b/ckan/lib/create_test_data.py index 3edf865c7c7..f464907d745 100644 --- a/ckan/lib/create_test_data.py +++ b/ckan/lib/create_test_data.py @@ -38,6 +38,12 @@ def create_family_test_data(cls, extra_users=[]): relationships=family_relationships, extra_user_names=extra_users) + @classmethod + def create_group_hierarchy_test_data(cls, extra_users=[]): + cls.create_groups(group_hierarchy_groups) + cls.create_arbitrary(group_hierarchy_datasets, + extra_user_names=group_hierarchy_users) + @classmethod def create_test_user(cls): tester = model.User.by_name(u'tester') @@ -315,10 +321,9 @@ def pkg(pkg_name): @classmethod def create_groups(cls, group_dicts, admin_user_name=None, auth_profile=""): '''A more featured interface for creating groups. - All group fields can be filled, packages added and they can - have an admin user.''' + All group fields can be filled, packages added, can have + an admin user and be a member of other groups.''' rev = model.repo.new_revision() - # same name as user we create below rev.author = cls.author if admin_user_name: admin_users = [model.User.by_name(admin_user_name)] @@ -327,7 +332,7 @@ def create_groups(cls, group_dicts, admin_user_name=None, auth_profile=""): assert isinstance(group_dicts, (list, tuple)) group_attributes = set(('name', 'title', 'description', 'parent_id')) for group_dict in group_dicts: - if model.Group.by_name(group_dict['name']): + if model.Group.by_name(unicode(group_dict['name'])): log.warning('Cannot create group "%s" as it already exists.' % \ (group_dict['name'])) continue @@ -346,7 +351,35 @@ def create_groups(cls, group_dicts, admin_user_name=None, auth_profile=""): member = model.Member(group=group, table_id=pkg.id, table_name='package') model.Session.add(member) model.Session.add(group) - model.setup_default_user_roles(group, 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=user.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', [])] + for editor in editors: + member = model.Member(group=group, table_id=user.id, + table_name='user', capacity='editor') + model.Session.add(member) + # Need to commit the current Group for two reasons: + # 1. It might have a parent, and the Member will need the Group.id + # value allocated on commit. + # 2. The next Group created may have this Group as a parent so + # creation of the Member needs to refer to this one. + model.Session.commit() + rev = model.repo.new_revision() + rev.author = cls.author + # add it to a parent's group + if 'parent' in group_dict: + parent = model.Group.by_name(unicode(group_dict['parent'])) + assert parent, group_dict['parent'] + member = model.Member(group=parent, table_id=group.id, + table_name='group', capacity='parent') + model.Session.add(member) + #model.setup_default_user_roles(group, admin_users) cls.group_names.add(group_dict['name']) model.repo.commit_and_remove() @@ -825,6 +858,44 @@ def make_some_vocab_tags(cls): } ] +group_hierarchy_groups = [ + {'name': 'department-of-health', + 'title': 'Department of Health', + 'contact-email': 'contact@doh.gov.uk'}, + {'name': 'food-standards-agency', + 'title': 'Food Standards Agency', + 'contact-email': 'contact@fsa.gov.uk', + 'parent': 'department-of-health'}, + {'name': 'national-health-service', + 'title': 'National Health Service', + 'contact-email': 'contact@nhs.gov.uk', + 'parent': 'department-of-health'}, + {'name': 'nhs-wirral-ccg', + 'title': 'NHS Wirral CCG', + 'contact-email': 'contact@wirral.nhs.gov.uk', + 'parent': 'national-health-service'}, + {'name': 'nhs-southwark-ccg', + 'title': 'NHS Southwark CCG', + 'contact-email': 'contact@southwark.nhs.gov.uk', + 'parent': 'national-health-service'}, + {'name': 'cabinet-office', + 'title': 'Cabinet Office', + 'contact-email': 'contact@cabinet-office.gov.uk'}, + ] + +group_hierarchy_datasets = [ + {'name': 'doh-spend', 'title': 'Department of Health Spend Data', + 'groups': ['department-of-health']}, + {'name': 'nhs-spend', 'title': 'NHS Spend Data', + 'groups': ['national-health-service']}, + {'name': 'wirral-spend', 'title': 'Wirral Spend Data', + 'groups': ['nhs-wirral-ccg']}, + {'name': 'southwark-spend', 'title': 'Southwark Spend Data', + 'groups': ['nhs-southwark-ccg']}, + ] + +group_hierarchy_users = ['nhsadmin', 'nhseditor', 'wirraladmin', 'wirraleditor'] + # Some test terms and translations. terms = ('A Novel By Tolstoy', 'Index of the novel', From fde50fa47123c714e8098e8f7dbf88085da67ddc Mon Sep 17 00:00:00 2001 From: David Read Date: Mon, 22 Jul 2013 17:54:56 +0100 Subject: [PATCH 08/87] [#1038] Model methods for organization hierarchy. --- ckan/model/group.py | 111 ++++++++++++++++++++++++++------ ckan/tests/models/test_group.py | 64 +++++++++++++++++- 2 files changed, 153 insertions(+), 22 deletions(-) diff --git a/ckan/model/group.py b/ckan/model/group.py index c16424affee..2cf748bb078 100644 --- a/ckan/model/group.py +++ b/ckan/model/group.py @@ -87,6 +87,17 @@ def related_packages(self): return meta.Session.query(_package.Package).filter_by( id=self.table_id).all() + 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 + elif self.table_name == 'group': + 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) + 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, @@ -145,17 +156,55 @@ def set_approval_status(self, status): pass def get_children_groups(self, type='group'): - # Returns a list of dicts where each dict contains "id", "name", - # and "title" When querying with a CTE specifying a model in the - # query parameter causes problems as it returns only the first - # level deep apparently not recursing any deeper than that. If - # we simplify and request only specific fields then if returns - # the full depth of the hierarchy. - results = meta.Session.query("id", "name", "title").\ - from_statement(HIERARCHY_CTE).params(id=self.id, type=type).all() - return [{"id":idf, "name": name, "title": title} - for idf, name, title in results] + '''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).\ + filter_by(type=type).\ + join(Member, Member.table_id == Group.id).\ + filter_by(group=self).\ + filter_by(table_name='group').\ + filter_by(state='active').\ + all() + + 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. + + :rtype: a list of tuples, each one a Group and the ID its their parent + group. + + e.g. >>> dept-health.get_children_group_hierarchy() + [(, u'8a163ba7-5146-4325-90c8-fe53b25e28d0'), + (, u'06e6dbf5-d801-40a1-9dc0-6785340b2ab4'), + (, 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() + 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.''' + return meta.Session.query(Group).\ + 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.''' + 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() def packages(self, with_private=False, limit=None, return_query=False, context=None): @@ -248,6 +297,8 @@ def add_package_by_name(self, package_name): def get_groups(self, group_type=None, capacity=None): """ Get all groups that this group is within """ import ckan.model as model + # DR: Why is this cached? Surely the members can change in the + # lifetime of this Group? if '_groups' not in self.__dict__: self._groups = meta.Session.query(model.Group).\ join(model.Member, model.Member.group_id == model.Group.id and @@ -314,15 +365,33 @@ def __repr__(self): MemberRevision.related_packages = lambda self: [self.continuity.package] -HIERARCHY_CTE = """WITH RECURSIVE subtree(id) AS ( - SELECT M.* FROM public.member AS M - WHERE M.table_name = 'group' AND M.state = 'active' - UNION - SELECT M.* FROM public.member M, subtree SG - WHERE M.table_id = SG.group_id AND M.table_name = 'group' - AND M.state = 'active') - SELECT G.* FROM subtree AS ST - INNER JOIN public.group G ON G.id = ST.table_id - WHERE group_id = :id AND G.type = :type and table_name='group' - and G.state='active'""" +HIERARCHY_DOWNWARDS_CTE = """WITH RECURSIVE child AS +( + -- non-recursive term + SELECT * FROM member + WHERE group_id = :id AND table_name = 'group' AND state = 'active' + UNION ALL + -- recursive term + SELECT m.* FROM member AS m, child AS c + WHERE m.group_id = c.table_id AND m.table_name = 'group' + AND m.state = 'active' +) +SELECT G.*, child.group_id as parent_id FROM child + INNER JOIN public.group G ON G.id = child.table_id + WHERE G.type = :type AND G.state='active';""" + +HIERARCHY_UPWARDS_CTE = """WITH RECURSIVE parenttree(id) AS ( + -- non-recursive term + SELECT M.* FROM public.member AS M + WHERE table_id = :id AND M.table_name = 'group' AND M.state = 'active' + UNION + -- recursive term + SELECT M.* FROM public.member M + JOIN parenttree as PG ON PG.group_id = M.table_id + WHERE M.table_name = 'group' AND M.state = 'active' + ) + +SELECT G.* FROM parenttree AS PT + INNER JOIN public.group G ON G.id = PT.group_id + WHERE G.type = :type AND G.state='active';""" diff --git a/ckan/tests/models/test_group.py b/ckan/tests/models/test_group.py index 88d1e63d9be..000c5c41450 100644 --- a/ckan/tests/models/test_group.py +++ b/ckan/tests/models/test_group.py @@ -1,4 +1,4 @@ -from nose.tools import assert_equal +from ckan.tests import assert_equal, assert_not_in, assert_in import ckan.model as model from ckan.tests import * @@ -92,6 +92,68 @@ def _search_results(self, 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]) +name_set_from_dicts = lambda groups: set([group['name'] for group in groups]) +name_set_from_group_tuple = lambda tuples: set([t[0].name for t in tuples]) +name_set_from_groups = lambda groups: set([group.name for group in groups]) +names_from_groups = lambda groups: [group.name for group in groups] + +class TestHierarchy: + @classmethod + def setup_class(self): + CreateTestData.create_group_hierarchy_test_data() + + def test_get_children_groups(self): + res = model.Group.by_name(u'department-of-health').\ + get_children_groups() + # check groups + assert_equal(name_set_from_dicts(res), + set(('national-health-service', + 'food-standards-agency'))) + # check each group is expressed as a small dict + assert_equal(set(res[0].keys()), set(('id', 'name', 'title'))) + assert_in(res[0]['name'], ('national-health-service', 'food-standards-agency')) + assert_in(res[0]['title'], ('National Health Service', 'Food Standards Agency')) + + def test_get_children_group_hierarchy__from_top(self): + assert_equal(name_set_from_group_tuple(model.Group.by_name(u'department-of-health').\ + get_children_group_hierarchy()), + set(('national-health-service', 'food-standards-agency', + 'nhs-wirral-ccg', 'nhs-southwark-ccg'))) + # i.e. not cabinet-office + + def test_get_children_group_hierarchy__from_tier_two(self): + assert_equal(name_set_from_group_tuple(model.Group.by_name(u'national-health-service').\ + get_children_group_hierarchy()), + set(('nhs-wirral-ccg', + 'nhs-southwark-ccg'))) + # i.e. not department-of-health or food-standards-agency + + def test_get_children_group_hierarchy__from_bottom_tier(self): + assert_equal(name_set_from_group_tuple(model.Group.by_name(u'nhs-wirral-ccg').\ + get_children_group_hierarchy()), + set()) + + def test_get_parent_groups_up_hierarchy__from_top(self): + assert_equal(names_from_groups(model.Group.by_name(u'department-of-health').\ + get_parent_group_hierarchy()), + []) + + def test_get_parent_groups_up_hierarchy__from_tier_two(self): + assert_equal(names_from_groups(model.Group.by_name(u'national-health-service').\ + get_parent_group_hierarchy()), + ['department-of-health']) + + def test_get_parent_groups_up_hierarchy__from_tier_three(self): + assert_equal(names_from_groups(model.Group.by_name(u'nhs-wirral-ccg').\ + get_parent_group_hierarchy()), + ['department-of-health', + 'national-health-service']) + + def test_get_top_level_groups(self): + assert_equal(names_from_groups(model.Group.by_name(u'nhs-wirral-ccg').\ + get_top_level_groups()), + ['cabinet-office', 'department-of-health']) + class TestGroupRevisions: @classmethod def setup_class(self): From 2c0bbafc509fc340dbdaa50c73e5ca712dc6b57a Mon Sep 17 00:00:00 2001 From: David Read Date: Tue, 23 Jul 2013 17:21:08 +0100 Subject: [PATCH 09/87] [#1038] Test data is now organizations rather than groups. --- ckan/lib/create_test_data.py | 34 +++++++++++++++++++++++++-------- ckan/tests/models/test_group.py | 18 +++++++++-------- 2 files changed, 36 insertions(+), 16 deletions(-) diff --git a/ckan/lib/create_test_data.py b/ckan/lib/create_test_data.py index f464907d745..8b4d44707e1 100644 --- a/ckan/lib/create_test_data.py +++ b/ckan/lib/create_test_data.py @@ -231,8 +231,12 @@ def create_arbitrary(cls, package_dicts, relationships=[], # session has not yet been committed at this point. # Fetch from the new_groups dict instead. group = new_groups[group_name] - member = model.Member(group=group, table_id=pkg.id, table_name='package') + capacity = 'organization' if group.is_organization \ + else 'public' + 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 elif attr == 'license': pkg.license_id = val elif attr == 'license_id': @@ -330,7 +334,8 @@ def create_groups(cls, group_dicts, admin_user_name=None, auth_profile=""): else: admin_users = [] assert isinstance(group_dicts, (list, tuple)) - group_attributes = set(('name', 'title', 'description', 'parent_id')) + group_attributes = set(('name', 'title', 'description', 'parent_id', + '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.' % \ @@ -861,26 +866,39 @@ def make_some_vocab_tags(cls): group_hierarchy_groups = [ {'name': 'department-of-health', 'title': 'Department of Health', - 'contact-email': 'contact@doh.gov.uk'}, + 'contact-email': 'contact@doh.gov.uk', + 'type': 'organization', + 'is_organization': True + }, {'name': 'food-standards-agency', 'title': 'Food Standards Agency', 'contact-email': 'contact@fsa.gov.uk', - 'parent': 'department-of-health'}, + 'parent': 'department-of-health', + 'type': 'organization', + 'is_organization': True}, {'name': 'national-health-service', 'title': 'National Health Service', 'contact-email': 'contact@nhs.gov.uk', - 'parent': 'department-of-health'}, + 'parent': 'department-of-health', + 'type': 'organization', + 'is_organization': True}, {'name': 'nhs-wirral-ccg', 'title': 'NHS Wirral CCG', 'contact-email': 'contact@wirral.nhs.gov.uk', - 'parent': 'national-health-service'}, + 'parent': 'national-health-service', + 'type': 'organization', + 'is_organization': True}, {'name': 'nhs-southwark-ccg', 'title': 'NHS Southwark CCG', 'contact-email': 'contact@southwark.nhs.gov.uk', - 'parent': 'national-health-service'}, + 'parent': 'national-health-service', + 'type': 'organization', + 'is_organization': True}, {'name': 'cabinet-office', 'title': 'Cabinet Office', - 'contact-email': 'contact@cabinet-office.gov.uk'}, + 'contact-email': 'contact@cabinet-office.gov.uk', + 'type': 'organization', + 'is_organization': True}, ] group_hierarchy_datasets = [ diff --git a/ckan/tests/models/test_group.py b/ckan/tests/models/test_group.py index 000c5c41450..13cee1907dc 100644 --- a/ckan/tests/models/test_group.py +++ b/ckan/tests/models/test_group.py @@ -97,6 +97,8 @@ def _search_results(self, query, is_org=False): name_set_from_groups = lambda groups: set([group.name for group in groups]) names_from_groups = lambda groups: [group.name for group in groups] +group_type = 'organization' + class TestHierarchy: @classmethod def setup_class(self): @@ -104,7 +106,7 @@ def setup_class(self): def test_get_children_groups(self): res = model.Group.by_name(u'department-of-health').\ - get_children_groups() + get_children_groups(type=group_type) # check groups assert_equal(name_set_from_dicts(res), set(('national-health-service', @@ -116,42 +118,42 @@ def test_get_children_groups(self): def test_get_children_group_hierarchy__from_top(self): assert_equal(name_set_from_group_tuple(model.Group.by_name(u'department-of-health').\ - get_children_group_hierarchy()), + get_children_group_hierarchy(type=group_type)), set(('national-health-service', 'food-standards-agency', 'nhs-wirral-ccg', 'nhs-southwark-ccg'))) # i.e. not cabinet-office def test_get_children_group_hierarchy__from_tier_two(self): assert_equal(name_set_from_group_tuple(model.Group.by_name(u'national-health-service').\ - get_children_group_hierarchy()), + get_children_group_hierarchy(type=group_type)), set(('nhs-wirral-ccg', 'nhs-southwark-ccg'))) # i.e. not department-of-health or food-standards-agency def test_get_children_group_hierarchy__from_bottom_tier(self): assert_equal(name_set_from_group_tuple(model.Group.by_name(u'nhs-wirral-ccg').\ - get_children_group_hierarchy()), + get_children_group_hierarchy(type=group_type)), set()) def test_get_parent_groups_up_hierarchy__from_top(self): assert_equal(names_from_groups(model.Group.by_name(u'department-of-health').\ - get_parent_group_hierarchy()), + get_parent_group_hierarchy(type=group_type)), []) def test_get_parent_groups_up_hierarchy__from_tier_two(self): assert_equal(names_from_groups(model.Group.by_name(u'national-health-service').\ - get_parent_group_hierarchy()), + get_parent_group_hierarchy(type=group_type)), ['department-of-health']) def test_get_parent_groups_up_hierarchy__from_tier_three(self): assert_equal(names_from_groups(model.Group.by_name(u'nhs-wirral-ccg').\ - get_parent_group_hierarchy()), + get_parent_group_hierarchy(type=group_type)), ['department-of-health', 'national-health-service']) def test_get_top_level_groups(self): assert_equal(names_from_groups(model.Group.by_name(u'nhs-wirral-ccg').\ - get_top_level_groups()), + get_top_level_groups(type=group_type)), ['cabinet-office', 'department-of-health']) class TestGroupRevisions: From 1229ff24d506a57f56ca4d030ab9228ff1d647c9 Mon Sep 17 00:00:00 2001 From: David Read Date: Tue, 23 Jul 2013 17:22:27 +0100 Subject: [PATCH 10/87] [#1038] Docs for Member and its capacities is most handy. Explained the sort ordering for some of the group functions. --- ckan/model/group.py | 17 ++++++++++++++++- 1 file changed, 16 insertions(+), 1 deletion(-) diff --git a/ckan/model/group.py b/ckan/model/group.py index 2cf748bb078..a7de2495233 100644 --- a/ckan/model/group.py +++ b/ckan/model/group.py @@ -54,6 +54,19 @@ class Member(vdm.sqlalchemy.RevisionedObjectMixin, vdm.sqlalchemy.StatefulObjectMixin, domain_object.DomainObject): + '''A Member object represents any other object being a 'member' of a + particular Group. + + Meanings: + * Package - the Group is a collection of Packages + - capacity is 'public', 'private' + or 'organization' if the Group is an Organization + (see ckan.logic.action.package_owner_org_update) + * User - the User is granted permissions for the Group + - capacity is 'admin', 'editor' or 'member' + * Group - the Group (Member.group_id) is a child of the Group (Member.id) + in a hierarchy. + ''' def __init__(self, group=None, table_id=None, group_id=None, table_name=None, capacity='public', state='active'): self.group = group @@ -175,6 +188,7 @@ def get_children_groups(self, type='group'): 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. :rtype: a list of tuples, each one a Group and the ID its their parent group. @@ -197,7 +211,8 @@ def get_parent_group_hierarchy(self, type='group'): @classmethod def get_top_level_groups(cls, type='group'): '''Returns a list of the groups (of the specified type) which have - no parent groups.''' + 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 \ From 68bfd7aefc272bcd94cff226ce97067ec44a91a4 Mon Sep 17 00:00:00 2001 From: David Read Date: Tue, 30 Jul 2013 16:18:07 +0100 Subject: [PATCH 11/87] [#1038] Permission cascading code with tests. --- ckan/lib/create_test_data.py | 24 ++-- ckan/logic/auth/update.py | 2 +- ckan/logic/validators.py | 3 +- ckan/new_authz.py | 34 ++++- ckan/tests/logic/test_auth.py | 235 ++++++++++++++++++++++++++++++++-- 5 files changed, 275 insertions(+), 23 deletions(-) diff --git a/ckan/lib/create_test_data.py b/ckan/lib/create_test_data.py index 8b4d44707e1..80f32a1038d 100644 --- a/ckan/lib/create_test_data.py +++ b/ckan/lib/create_test_data.py @@ -40,9 +40,9 @@ def create_family_test_data(cls, extra_users=[]): @classmethod def create_group_hierarchy_test_data(cls, extra_users=[]): + cls.create_users(group_hierarchy_users) cls.create_groups(group_hierarchy_groups) - cls.create_arbitrary(group_hierarchy_datasets, - extra_user_names=group_hierarchy_users) + cls.create_arbitrary(group_hierarchy_datasets) @classmethod def create_test_user(cls): @@ -347,7 +347,7 @@ def create_groups(cls, group_dicts, admin_user_name=None, auth_profile=""): for key in group_dict: if key in group_attributes: setattr(group, key, group_dict[key]) - else: + elif key not in ('admins', 'editors', 'parent'): group.extras[key] = group_dict[key] assert isinstance(pkg_names, (list, tuple)) for pkg_name in pkg_names: @@ -360,13 +360,13 @@ def create_groups(cls, group_dicts, admin_user_name=None, auth_profile=""): for user_name in group_dict.get('admins', [])] \ + admin_users for admin in admins: - member = model.Member(group=group, table_id=user.id, + 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', [])] for editor in editors: - member = model.Member(group=group, table_id=user.id, + member = model.Member(group=group, table_id=editor.id, table_name='user', capacity='editor') model.Session.add(member) # Need to commit the current Group for two reasons: @@ -881,13 +881,17 @@ def make_some_vocab_tags(cls): 'contact-email': 'contact@nhs.gov.uk', 'parent': 'department-of-health', 'type': 'organization', - 'is_organization': True}, + 'is_organization': True, + 'editors': ['nhseditor'], + 'admins': ['nhsadmin']}, {'name': 'nhs-wirral-ccg', 'title': 'NHS Wirral CCG', 'contact-email': 'contact@wirral.nhs.gov.uk', 'parent': 'national-health-service', 'type': 'organization', - 'is_organization': True}, + 'is_organization': True, + 'editors': ['wirraleditor'], + 'admins': ['wirraladmin']}, {'name': 'nhs-southwark-ccg', 'title': 'NHS Southwark CCG', 'contact-email': 'contact@southwark.nhs.gov.uk', @@ -912,7 +916,11 @@ def make_some_vocab_tags(cls): 'groups': ['nhs-southwark-ccg']}, ] -group_hierarchy_users = ['nhsadmin', 'nhseditor', 'wirraladmin', 'wirraleditor'] +group_hierarchy_users = [{'name': 'nhsadmin', 'password': 'pass'}, + {'name': 'nhseditor', 'password': 'pass'}, + {'name': 'wirraladmin', 'password': 'pass'}, + {'name': 'wirraleditor', 'password': 'pass'}, + ] # Some test terms and translations. terms = ('A Novel By Tolstoy', diff --git a/ckan/logic/auth/update.py b/ckan/logic/auth/update.py index ac907f487d0..7376f27afb9 100644 --- a/ckan/logic/auth/update.py +++ b/ckan/logic/auth/update.py @@ -17,7 +17,7 @@ def package_update(context, data_dict): if package.owner_org: # if there is an owner org then we must have update_dataset - # premission for that organization + # permission for that organization check1 = new_authz.has_user_permission_for_group_or_org( package.owner_org, user, 'update_dataset' ) diff --git a/ckan/logic/validators.py b/ckan/logic/validators.py index f9de157795c..0fb3887798e 100644 --- a/ckan/logic/validators.py +++ b/ckan/logic/validators.py @@ -36,7 +36,8 @@ 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 user.is_in_group(group_id)): + 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/new_authz.py b/ckan/new_authz.py index 1d391c50900..3296e795cf8 100644 --- a/ckan/new_authz.py +++ b/ckan/new_authz.py @@ -128,10 +128,14 @@ 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 permission for the group ''' + ''' Check if the user has the given permissions for the group, + allowing for sysadmin rights and permission cascading down groups. ''' if not group_id: return False - group_id = model.Group.get(group_id).id + group = model.Group.get(group_id) + if not group: + return False + group_id = group.id # Sys admins can do anything if is_sysadmin(user_name): @@ -140,12 +144,29 @@ def has_user_permission_for_group_or_org(group_id, user_name, permission): user_id = get_user_id_for_username(user_name, allow_none=True) if not user_id: return False + if _has_user_permission_for_groups(user_id, permission, [group_id]): + return True + # Handle when permissions cascade. Check the user's roles on groups higher + # 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] + 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): + ''' Check if the user has the given permissions for the particular + 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 == group_id) \ + .filter(model.Member.group_id.in_(group_ids)) \ .filter(model.Member.table_name == 'user') \ .filter(model.Member.state == 'active') \ .filter(model.Member.table_id == user_id) + if capacity: + q = q.filter(model.Member.capacity == capacity) # see if any role has the required permission # admin permission allows anything for the group for row in q.all(): @@ -281,18 +302,21 @@ def _get_auth_function(action): 'user_delete_groups': True, 'user_delete_organizations': True, 'create_user_via_api': False, + 'roles_that_cascade_to_sub_groups': ['admin'], } CONFIG_PERMISSIONS = {} def check_config_permission(permission): - ''' Returns the permission True/False based on config ''' + ''' Returns the permission configuration, usually True/False ''' # set up perms if not already done if not CONFIG_PERMISSIONS: for perm in CONFIG_PERMISSIONS_DEFAULTS: key = 'ckan.auth.' + perm default = CONFIG_PERMISSIONS_DEFAULTS[perm] - CONFIG_PERMISSIONS[perm] = asbool(config.get(key, default)) + CONFIG_PERMISSIONS[perm] = config.get(key, default) + if isinstance(perm, bool): + CONFIG_PERMISSIONS[perm] = asbool(CONFIG_PERMISSIONS[perm]) if permission in CONFIG_PERMISSIONS: return CONFIG_PERMISSIONS[permission] return False diff --git a/ckan/tests/logic/test_auth.py b/ckan/tests/logic/test_auth.py index 3ea9f2db39f..fcac3c1707b 100644 --- a/ckan/tests/logic/test_auth.py +++ b/ckan/tests/logic/test_auth.py @@ -1,7 +1,10 @@ +import paste.fixture + import ckan.tests as tests from ckan.logic import get_action import ckan.model as model import ckan.new_authz as new_authz +from ckan.lib.create_test_data import CreateTestData, group_hierarchy_groups import json INITIAL_TEST_CONFIG_PERMISSIONS = { @@ -13,6 +16,7 @@ 'user_delete_organizations': False, 'create_user_via_api': False, 'create_unowned_dataset': False, + 'roles_that_cascade_to_sub_groups': ['admin'], } @@ -33,19 +37,26 @@ def teardown_class(cls): new_authz.CONFIG_PERMISSIONS.update(cls.old_perm) model.repo.rebuild_db() - def _call_api(self, action, data, user, status=None): + @classmethod + def _call_api(cls, action, data, user, status=None): params = '%s=1' % json.dumps(data) - return self.app.post('/api/action/%s' % action, - params=params, - extra_environ={'Authorization': self.apikeys[user]}, - status=status) + res = cls.app.post('/api/action/%s' % action, + params=params, + extra_environ={'Authorization': cls.apikeys[user]}, + status=[200, 403, 409]) + if res.status != (status or 200): + error = json.loads(res.body)['error'] + raise AssertionError('Status was %s but should be %s. Error: %s' % + (res.status, status, error)) + return res - def create_user(self, name): + @classmethod + def create_user(cls, name): user = {'name': name, 'password': 'pass', 'email': 'moo@moo.com'} - res = self._call_api('user_create', user, 'sysadmin', 200) - self.apikeys[name] = str(json.loads(res.body)['result']['apikey']) + res = cls._call_api('user_create', user, 'sysadmin', 200) + cls.apikeys[name] = str(json.loads(res.body)['result']['apikey']) class TestAuthOrgs(TestAuth): @@ -190,6 +201,214 @@ def test_11_delete_org(self): self._call_api('organization_delete', org, 'org_editor', 403) self._call_api('organization_delete', org, 'org_admin', 403) +ORG_HIERARCHY_PERMISSIONS = { + 'roles_that_cascade_to_sub_groups': ['admin'], + } + +class TestAuthOrgHierarchy(TestAuth): + # Tests are in the same vein as TestAuthOrgs, testing the cases where the + # group hierarchy provices extra permissions through cascading + + @classmethod + def setup_class(cls): + TestAuth.setup_class() + CreateTestData.create_group_hierarchy_test_data() + for user in model.Session.query(model.User): + cls.apikeys[user.name] = str(user.apikey) + new_authz.CONFIG_PERMISSIONS.update(ORG_HIERARCHY_PERMISSIONS) + CreateTestData.create_arbitrary( + package_dicts= [{'name': 'adataset', + 'groups': ['national-health-service']}], + extra_user_names=['john']) + + def _reset_adatasets_owner_org(self): + rev = model.repo.new_revision() + get_action('package_owner_org_update')( + {'model': model, 'ignore_auth': True}, + {'id': 'adataset', + 'organization_id': 'national-health-service'}) + + def _undelete_package_if_needed(self, package_name): + pkg = model.Package.by_name(package_name) + if pkg and pkg.state == 'deleted': + rev = model.repo.new_revision() + pkg.state = 'active' + model.repo.commit_and_remove() + + def test_05_add_users_to_org_1(self): + member = {'username': 'john', 'role': 'admin', + 'id': 'department-of-health'} + self._call_api('organization_member_create', member, 'nhsadmin', 403) + def test_05_add_users_to_org_2(self): + member = {'username': 'john', 'role': 'editor', + 'id': 'department-of-health'} + self._call_api('organization_member_create', member, 'nhsadmin', 403) + def test_05_add_users_to_org_3(self): + member = {'username': 'john', 'role': 'admin', + 'id': 'national-health-service'} + self._call_api('organization_member_create', member, 'nhsadmin', 200) + def test_05_add_users_to_org_4(self): + member = {'username': 'john', 'role': 'editor', + 'id': 'national-health-service'} + self._call_api('organization_member_create', member, 'nhsadmin', 200) + def test_05_add_users_to_org_5(self): + member = {'username': 'john', 'role': 'admin', + 'id': 'nhs-wirral-ccg'} + self._call_api('organization_member_create', member, 'nhsadmin', 200) + def test_05_add_users_to_org_6(self): + member = {'username': 'john', 'role': 'editor', + 'id': 'nhs-wirral-ccg'} + self._call_api('organization_member_create', member, 'nhsadmin', 200) + def test_05_add_users_to_org_7(self): + member = {'username': 'john', 'role': 'editor', + 'id': 'national-health-service'} + self._call_api('organization_member_create', member, 'nhseditor', 403) + + def test_07_add_datasets_1(self): + dataset = {'name': 't1', 'owner_org': 'department-of-health'} + self._call_api('package_create', dataset, 'nhsadmin', 403) + + def test_07_add_datasets_2(self): + dataset = {'name': 't2', 'owner_org': 'national-health-service'} + self._call_api('package_create', dataset, 'nhsadmin', 200) + + def test_07_add_datasets_3(self): + dataset = {'name': 't3', 'owner_org': 'nhs-wirral-ccg'} + self._call_api('package_create', dataset, 'nhsadmin', 200) + + def test_07_add_datasets_4(self): + dataset = {'name': 't4', 'owner_org': 'department-of-health'} + self._call_api('package_create', dataset, 'nhseditor', 403) + + def test_07_add_datasets_5(self): + dataset = {'name': 't5', 'owner_org': 'national-health-service'} + self._call_api('package_create', dataset, 'nhseditor', 200) + + def test_07_add_datasets_6(self): + dataset = {'name': 't6', 'owner_org': 'nhs-wirral-ccg'} + self._call_api('package_create', dataset, 'nhseditor', 403) + + def test_08_update_datasets_1(self): + dataset = {'name': 'adataset', 'owner_org': 'department-of-health'} + self._call_api('package_update', dataset, 'nhsadmin', 409) + + def test_08_update_datasets_2(self): + dataset = {'name': 'adataset', 'owner_org': 'national-health-service'} + self._call_api('package_update', dataset, 'nhsadmin', 200) + + def test_08_update_datasets_3(self): + dataset = {'name': 'adataset', 'owner_org': 'nhs-wirral-ccg'} + try: + self._call_api('package_update', dataset, 'nhsadmin', 200) + finally: + self._reset_adatasets_owner_org() + + def test_08_update_datasets_4(self): + dataset = {'name': 'adataset', 'owner_org': 'department-of-health'} + self._call_api('package_update', dataset, 'nhseditor', 409) + + def test_08_update_datasets_5(self): + dataset = {'name': 'adataset', 'owner_org': 'national-health-service'} + try: + self._call_api('package_update', dataset, 'nhseditor', 200) + finally: + self._reset_adatasets_owner_org() + + def test_08_update_datasets_6(self): + dataset = {'name': 'adataset', 'owner_org': 'nhs-wirral-ccg'} + self._call_api('package_update', dataset, 'nhseditor', 409) + + def test_09_delete_datasets_1(self): + dataset = {'id': 'doh-spend'} + try: + self._call_api('package_delete', dataset, 'nhsadmin', 403) + finally: + self._undelete_package_if_needed(dataset['id']) + + def test_09_delete_datasets_2(self): + dataset = {'id': 'nhs-spend'} + try: + self._call_api('package_delete', dataset, 'nhsadmin', 200) + finally: + self._undelete_package_if_needed(dataset['id']) + + def test_09_delete_datasets_3(self): + dataset = {'id': 'wirral-spend'} + try: + self._call_api('package_delete', dataset, 'nhsadmin', 200) + finally: + self._undelete_package_if_needed(dataset['id']) + + def test_09_delete_datasets_4(self): + dataset = {'id': 'nhs-spend'} + try: + self._call_api('package_delete', dataset, 'nhseditor', 200) + finally: + self._undelete_package_if_needed(dataset['id']) + + def test_09_delete_datasets_5(self): + dataset = {'id': 'wirral-spend'} + try: + self._call_api('package_delete', dataset, 'nhseditor', 403) + finally: + self._undelete_package_if_needed(dataset['id']) + + def _flesh_out_organization(self, org): + # When calling organization_update, unless you include the list of + # editor and admin users and parent groups, it will remove them. So + # get the current list + existing_org = get_action('organization_show')( + {'model': model, 'ignore_auth': True}, {'id': org['id']}) + org.update(existing_org) + + def test_10_edit_org_1(self): + org = {'id': 'department-of-health', 'title': 'test'} + self._flesh_out_organization(org) + self._call_api('organization_update', org, 'nhsadmin', 403) + + def test_10_edit_org_2(self): + org = {'id': 'national-health-service', 'title': 'test'} + self._flesh_out_organization(org) + import pprint; pprint.pprint(org) + print model.Session.query(model.Member).filter_by(state='deleted').all() + self._call_api('organization_update', org, 'nhsadmin', 200) + print model.Session.query(model.Member).filter_by(state='deleted').all() + + def test_10_edit_org_3(self): + org = {'id': 'nhs-wirral-ccg', 'title': 'test'} + self._flesh_out_organization(org) + self._call_api('organization_update', org, 'nhsadmin', 200) + + def test_10_edit_org_4(self): + org = {'id': 'department-of-health', 'title': 'test'} + self._flesh_out_organization(org) + self._call_api('organization_update', org, 'nhseditor', 403) + + def test_10_edit_org_5(self): + org = {'id': 'national-health-service', 'title': 'test'} + self._flesh_out_organization(org) + self._call_api('organization_update', org, 'nhseditor', 403) + + def test_10_edit_org_6(self): + org = {'id': 'nhs-wirral-ccg', 'title': 'test'} + self._flesh_out_organization(org) + self._call_api('organization_update', org, 'nhseditor', 403) + + def test_11_delete_org_1(self): + org = {'id': 'department-of-health'} + self._call_api('organization_delete', org, 'nhsadmin', 403) + self._call_api('organization_delete', org, 'nhseditor', 403) + + def test_11_delete_org_2(self): + org = {'id': 'national-health-service'} + self._call_api('organization_delete', org, 'nhsadmin', 403) + self._call_api('organization_delete', org, 'nhseditor', 403) + + def test_11_delete_org_3(self): + org = {'id': 'nhs-wirral-ccg'} + self._call_api('organization_delete', org, 'nhsadmin', 403) + self._call_api('organization_delete', org, 'nhseditor', 403) + class TestAuthGroups(TestAuth): From 4db2d24908fc382a7548fbbcc7dc5133dcdd29ad Mon Sep 17 00:00:00 2001 From: David Read Date: Tue, 30 Jul 2013 16:44:59 +0100 Subject: [PATCH 12/87] [#1038] Fix permission checking for organizations. Corrected bad test. --- ckan/logic/auth/create.py | 2 +- ckan/tests/logic/test_auth.py | 11 +++++++---- 2 files changed, 8 insertions(+), 5 deletions(-) diff --git a/ckan/logic/auth/create.py b/ckan/logic/auth/create.py index bf9c3d17ea3..299308a9ae5 100644 --- a/ckan/logic/auth/create.py +++ b/ckan/logic/auth/create.py @@ -23,7 +23,7 @@ def package_create(context, data_dict=None): # If an organization is given are we able to add a dataset to it? data_dict = data_dict or {} - org_id = data_dict.get('organization_id') + org_id = data_dict.get('owner_org') if org_id and not new_authz.has_user_permission_for_group_or_org( org_id, user, 'create_dataset'): return {'success': False, 'msg': _('User %s not authorized to add dataset to this organization') % user} diff --git a/ckan/tests/logic/test_auth.py b/ckan/tests/logic/test_auth.py index fcac3c1707b..2dc6b5e9a5a 100644 --- a/ckan/tests/logic/test_auth.py +++ b/ckan/tests/logic/test_auth.py @@ -60,6 +60,8 @@ def create_user(cls, name): class TestAuthOrgs(TestAuth): + # NB: These tests are dependent on each other, so don't run them + # separately. def test_01_create_users(self): # actual roles assigned later @@ -90,6 +92,7 @@ def test_02_create_orgs(self): def test_03_create_dataset_no_org(self): + # no owner_org supplied dataset = {'name': 'admin_create_no_org'} self._call_api('package_create', dataset, 'sysadmin', 409) @@ -106,7 +109,7 @@ def test_04_create_dataset_with_org(self): 'owner_org': 'org_no_user'} self._call_api('package_create', dataset, 'sysadmin', 200) - dataset = {'name': 'user_create_with_org', + dataset = {'name': 'user_create_with_no_org', 'owner_org': 'org_with_user'} self._call_api('package_create', dataset, 'no_org', 403) @@ -138,7 +141,7 @@ def _add_datasets(self, user): #not able to add dataset to org admin does not belong to. dataset = {'name': user + '_dataset_bad', 'owner_org': 'org_no_user'} - self._call_api('package_create', dataset, user, 409) + self._call_api('package_create', dataset, user, 403) #admin not able to make dataset not owned by a org dataset = {'name': user + '_dataset_bad'} @@ -146,7 +149,7 @@ def _add_datasets(self, user): #not able to add org to not existant org dataset = {'name': user + '_dataset_bad', 'owner_org': 'org_not_exist'} - self._call_api('package_create', dataset, user, 409) + self._call_api('package_create', dataset, user, 403) def test_07_add_datasets(self): self._add_datasets('org_admin') @@ -317,7 +320,7 @@ def test_08_update_datasets_5(self): def test_08_update_datasets_6(self): dataset = {'name': 'adataset', 'owner_org': 'nhs-wirral-ccg'} self._call_api('package_update', dataset, 'nhseditor', 409) - + def test_09_delete_datasets_1(self): dataset = {'id': 'doh-spend'} try: From 6d97199ca314f5130f5318d23fc6a2f04e6d7ab5 Mon Sep 17 00:00:00 2001 From: David Read Date: Tue, 30 Jul 2013 16:45:52 +0100 Subject: [PATCH 13/87] [#1038] Fix unreliable ordering of upward CTE. --- ckan/model/group.py | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-) diff --git a/ckan/model/group.py b/ckan/model/group.py index a7de2495233..f21bafb3cb8 100644 --- a/ckan/model/group.py +++ b/ckan/model/group.py @@ -396,17 +396,17 @@ def __repr__(self): INNER JOIN public.group G ON G.id = child.table_id WHERE G.type = :type AND G.state='active';""" -HIERARCHY_UPWARDS_CTE = """WITH RECURSIVE parenttree(id) AS ( +HIERARCHY_UPWARDS_CTE = """WITH RECURSIVE parenttree(depth) AS ( -- non-recursive term - SELECT M.* FROM public.member AS M + SELECT 0, M.* FROM public.member AS M WHERE table_id = :id AND M.table_name = 'group' AND M.state = 'active' UNION -- recursive term - SELECT M.* FROM public.member M - JOIN parenttree as PG ON PG.group_id = M.table_id - WHERE M.table_name = 'group' AND M.state = 'active' + 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' ) -SELECT G.* FROM parenttree AS PT +SELECT G.*, PT.depth FROM parenttree AS PT INNER JOIN public.group G ON G.id = PT.group_id - WHERE G.type = :type AND G.state='active';""" + WHERE G.type = :type AND G.state='active' + ORDER BY PT.depth DESC;""" From 7ce1555ca3dc65adbb882a2373d4f99ce65fbf8c Mon Sep 17 00:00:00 2001 From: David Read Date: Fri, 6 Sep 2013 16:19:27 +0100 Subject: [PATCH 14/87] [#1038] Fix up authz config error and deleted groups * Fix - CONFIG_PERMISSIONS were not read properly following my prev changes * Fix deleted child groups being visible. * Fix up a couple of tests --- ckan/logic/auth/create.py | 7 +++++++ ckan/model/group.py | 1 + ckan/new_authz.py | 2 +- ckan/tests/functional/test_group.py | 18 +++++++++++++----- 4 files changed, 22 insertions(+), 6 deletions(-) diff --git a/ckan/logic/auth/create.py b/ckan/logic/auth/create.py index ae98e257914..6061e4eb69b 100644 --- a/ckan/logic/auth/create.py +++ b/ckan/logic/auth/create.py @@ -114,6 +114,13 @@ def user_create(context, data_dict=None): def _check_group_auth(context, data_dict): + '''Has this user got update permission for all of the given groups? + If there is a package in the context then ignore that package's groups. + :returns: False if not allowed to update one (or more) of the given groups. + True otherwise. i.e. True is the default. A blank data_dict + mentions no groups, so it returns True. + + ''' # FIXME This code is shared amoung other logic.auth files and should be # somewhere better if not data_dict: diff --git a/ckan/model/group.py b/ckan/model/group.py index f21bafb3cb8..a25cebf9927 100644 --- a/ckan/model/group.py +++ b/ckan/model/group.py @@ -177,6 +177,7 @@ def get_children_groups(self, type='group'): # 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).\ filter_by(group=self).\ filter_by(table_name='group').\ diff --git a/ckan/new_authz.py b/ckan/new_authz.py index a8f13063f60..370b19fb581 100644 --- a/ckan/new_authz.py +++ b/ckan/new_authz.py @@ -354,7 +354,7 @@ def check_config_permission(permission): key = 'ckan.auth.' + perm default = CONFIG_PERMISSIONS_DEFAULTS[perm] CONFIG_PERMISSIONS[perm] = config.get(key, default) - if isinstance(perm, bool): + if isinstance(default, bool): CONFIG_PERMISSIONS[perm] = asbool(CONFIG_PERMISSIONS[perm]) if permission in CONFIG_PERMISSIONS: return CONFIG_PERMISSIONS[permission] diff --git a/ckan/tests/functional/test_group.py b/ckan/tests/functional/test_group.py index b07bbe7ea6f..4f4fddc3e05 100644 --- a/ckan/tests/functional/test_group.py +++ b/ckan/tests/functional/test_group.py @@ -24,6 +24,10 @@ def setup_class(self): model.Session.remove() CreateTestData.create() + # reduce extraneous logging + from ckan.lib import activity_streams_session_extension + activity_streams_session_extension.logger.level = 100 + @classmethod def teardown_class(self): model.repo.rebuild_db() @@ -102,17 +106,21 @@ def test_children(self): def test_sorting(self): model.repo.rebuild_db() + testsysadmin = model.User(name=u'testsysadmin') + testsysadmin.sysadmin = True + model.Session.add(testsysadmin) + pkg1 = model.Package(name="pkg1") pkg2 = model.Package(name="pkg2") model.Session.add(pkg1) model.Session.add(pkg2) CreateTestData.create_groups([{'name': "alpha", 'packages': []}, - {'name': "beta", - 'packages': ["pkg1", "pkg2"]}, - {'name': "delta", - 'packages': ["pkg1"]}, - {'name': "gamma", 'packages': []}], + {'name': "beta", + 'packages': ["pkg1", "pkg2"]}, + {'name': "delta", + 'packages': ["pkg1"]}, + {'name': "gamma", 'packages': []}], admin_user_name='testsysadmin') context = {'model': model, 'session': model.Session, From f7f5049b0ab0a7220210722461bf56768d33df89 Mon Sep 17 00:00:00 2001 From: David Read Date: Fri, 6 Sep 2013 16:42:30 +0000 Subject: [PATCH 15/87] [#1038] PEP8 fixes only. --- ckan/lib/create_test_data.py | 47 +++++++++++++++----------- ckan/logic/validators.py | 5 +-- ckan/model/group.py | 51 ++++++++++++++++------------- ckan/new_authz.py | 14 +++++--- ckan/tests/functional/test_group.py | 14 ++++---- ckan/tests/test_coding_standards.py | 1 - 6 files changed, 75 insertions(+), 57 deletions(-) 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', From af6c039c778a78f471cb6112175aae46dfda6729 Mon Sep 17 00:00:00 2001 From: David Read Date: Thu, 12 Sep 2013 13:54:42 +0100 Subject: [PATCH 16/87] [#1038] Reverse the meaning of group member of group. Now member.table_id is the *parent* of member.group_id (not the child). This means the permissions work - a user needs permissions for the member.group_id to change its members, and it makes more sense for a group to choose its parents than its children. This is because a user with rights higher up the tree often gets given automatic rights to groups lower down. If he can also make any other group a child, then he would give himself rights over it. Added test for the group permissions to check it made sense Fixed bug in get_top_level_groups - the 'and' was ignored before. The existing test picked this up. group.get_groups meaning is expanded. Seems to only get used in _group_or_org_update and treats the result as parents. Needs further examination when doing the form work on this ticket. --- ckan/lib/create_test_data.py | 6 ++++-- ckan/model/group.py | 33 +++++++++++++++++------------ ckan/new_authz.py | 19 +++++++++++------ ckan/tests/functional/test_group.py | 2 +- ckan/tests/logic/test_member.py | 31 +++++++++++++++++++++++++++ 5 files changed, 67 insertions(+), 24 deletions(-) diff --git a/ckan/lib/create_test_data.py b/ckan/lib/create_test_data.py index 2ca59a42fcc..713e3b0f9ff 100644 --- a/ckan/lib/create_test_data.py +++ b/ckan/lib/create_test_data.py @@ -386,7 +386,7 @@ def create_groups(cls, group_dicts, admin_user_name=None, auth_profile=""): if 'parent' in group_dict: parent = model.Group.by_name(unicode(group_dict['parent'])) assert parent, group_dict['parent'] - member = model.Member(group=parent, table_id=group.id, + member = model.Member(group=group, table_id=parent.id, table_name='group', capacity='parent') model.Session.add(member) #model.setup_default_user_roles(group, admin_users) @@ -527,8 +527,10 @@ def create(cls, auth_profile="", package_type=None): roger = model.Group.by_name(u'roger') model.setup_default_user_roles(david, [russianfan]) model.setup_default_user_roles(roger, [russianfan]) - model.add_user_to_role(visitor, model.Role.ADMIN, roger) + # in new_authz you can't give a visitor permissions to a + # group it seems, so this is a bit meaningless + model.add_user_to_role(visitor, model.Role.ADMIN, roger) model.repo.commit_and_remove() # method used in DGU and all good tests elsewhere diff --git a/ckan/model/group.py b/ckan/model/group.py index ee637dde82f..b0c1c22fba5 100644 --- a/ckan/model/group.py +++ b/ckan/model/group.py @@ -1,6 +1,6 @@ import datetime -from sqlalchemy import orm, types, Column, Table, ForeignKey, or_ +from sqlalchemy import orm, types, Column, Table, ForeignKey, or_, and_ import vdm.sqlalchemy import meta @@ -64,8 +64,9 @@ class Member(vdm.sqlalchemy.RevisionedObjectMixin, (see ckan.logic.action.package_owner_org_update) * User - the User is granted permissions for the Group - capacity is 'admin', 'editor' or 'member' - * Group - the Group (Member.group_id) is a child of the Group (Member.id) + * Group - the Group (Member.group_id) is a parent of the Group (Member.id) in a hierarchy. + - capacity is 'parent' ''' def __init__(self, group=None, table_id=None, group_id=None, table_name=None, capacity='public', state='active'): @@ -182,8 +183,8 @@ def get_children_groups(self, type='group'): 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).\ - filter_by(group=self).\ + join(Member, Member.group_id == Group.id).\ + filter_by(table_id=self.id).\ filter_by(table_name='group').\ filter_by(state='active').\ all() @@ -222,9 +223,10 @@ 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').\ + outerjoin(Member, + and_(Member.group_id == Group.id, + Member.table_name == 'group', + Member.state == 'active')).\ filter(Member.id == None).\ filter(Group.type == type).\ order_by(Group.title).all() @@ -318,7 +320,10 @@ def add_package_by_name(self, package_name): meta.Session.add(member) def get_groups(self, group_type=None, capacity=None): - """ Get all groups that this group is within """ + """ Get all groups that this group is within (i.e. this group's child + groups) + + """ import ckan.model as model # DR: Why is this cached? Surely the members can change in the # lifetime of this Group? @@ -392,29 +397,29 @@ def __repr__(self): ( -- non-recursive term SELECT * FROM member - WHERE group_id = :id AND table_name = 'group' AND state = 'active' + WHERE table_id = :id AND table_name = 'group' AND state = 'active' UNION ALL -- recursive term SELECT m.* FROM member AS m, child AS c - WHERE m.group_id = c.table_id AND m.table_name = 'group' + WHERE m.table_id = c.group_id AND m.table_name = 'group' AND m.state = 'active' ) SELECT G.*, child.group_id as parent_id FROM child - INNER JOIN public.group G ON G.id = child.table_id + INNER JOIN public.group G ON G.id = child.group_id WHERE G.type = :type AND G.state='active';""" HIERARCHY_UPWARDS_CTE = """WITH RECURSIVE parenttree(depth) AS ( -- non-recursive term SELECT 0, M.* FROM public.member AS M - WHERE table_id = :id AND M.table_name = 'group' AND M.state = 'active' + WHERE group_id = :id AND M.table_name = 'group' AND M.state = 'active' 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' + WHERE PG.table_id = M.group_id AND M.table_name = 'group' AND M.state = 'active' ) SELECT G.*, PT.depth FROM parenttree AS PT - INNER JOIN public.group G ON G.id = PT.group_id + INNER JOIN public.group G ON G.id = PT.table_id WHERE G.type = :type AND G.state='active' ORDER BY PT.depth DESC;""" diff --git a/ckan/new_authz.py b/ckan/new_authz.py index 2f612c82b41..495f3806b17 100644 --- a/ckan/new_authz.py +++ b/ckan/new_authz.py @@ -103,7 +103,7 @@ def clean_action_name(action_name): def is_sysadmin(username): - ''' returns True is username is a sysadmin ''' + ''' returns True if username is a sysadmin ''' if not username: return False # see if we can authorise without touching the database @@ -212,8 +212,8 @@ 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. + ''' Check if the user has the given permissions for the group, allowing for + sysadmin rights and permission cascading down a group hierarchy. ''' if not group_id: @@ -246,9 +246,11 @@ def has_user_permission_for_group_or_org(group_id, user_name, permission): 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 (ignoring permissions cascading in a group hierarchy). + Can also be filtered by a particular capacity. ''' + if not group_ids: + return False # get any roles the user has for the group q = model.Session.query(model.Member) \ .filter(model.Member.group_id.in_(group_ids)) \ @@ -267,7 +269,10 @@ def _has_user_permission_for_groups(user_id, permission, group_ids, def users_role_for_group_or_org(group_id, user_name): - ''' Check if the user role for the group ''' + ''' Returns the user's role for the group. (Ignores privileges that cascade + in a group hierarchy.) + + ''' if not group_id: return None group_id = model.Group.get(group_id).id @@ -288,7 +293,7 @@ def users_role_for_group_or_org(group_id, user_name): def has_user_permission_for_some_org(user_name, permission): - ''' Check if the user has the given permission for the group ''' + ''' Check if the user has the given permission for any organization. ''' user_id = get_user_id_for_username(user_name, allow_none=True) if not user_id: return False diff --git a/ckan/tests/functional/test_group.py b/ckan/tests/functional/test_group.py index 5575bd6e4ba..fd47a6e6699 100644 --- a/ckan/tests/functional/test_group.py +++ b/ckan/tests/functional/test_group.py @@ -74,7 +74,7 @@ def test_children(self): rev = model.repo.new_revision() rev.author = "none" - member = model.Member(group_id=parent.id, table_id=group.id, + member = model.Member(group_id=group.id, table_id=parent.id, table_name='group', capacity='member') model.Session.add(member) model.repo.commit_and_remove() diff --git a/ckan/tests/logic/test_member.py b/ckan/tests/logic/test_member.py index 39f67edd2db..f3d0ec448a0 100644 --- a/ckan/tests/logic/test_member.py +++ b/ckan/tests/logic/test_member.py @@ -11,10 +11,18 @@ def setup_class(cls): model.repo.new_revision() create_test_data.CreateTestData.create() cls.user = model.User.get('testsysadmin') + cls.tester = model.User.get('tester') cls.group = model.Group.get('david') + cls.roger = model.Group.get('roger') cls.pkgs = [model.Package.by_name('warandpeace'), model.Package.by_name('annakarenina')] + # 'Tester' becomes an admin for the 'roger' group + model.repo.new_revision() + model.Member(group=cls.roger, table_id=cls.tester.id, + table_name='user', capacity='admin') + model.repo.commit_and_remove() + @classmethod def teardown_class(cls): model.repo.rebuild_db() @@ -37,6 +45,20 @@ def test_member_create_raises_if_user_unauthorized_to_update_group(self): assert_raises(logic.NotAuthorized, logic.get_action('member_create'), ctx, dd) + def test_member_create_with_child_group_permission(self): + # 'tester' has admin priviledge for roger, so anyone can make it + # a child group. + self._member_create_group_hierarchy(parent_group=self.group, + child_group=self.roger, + user=self.tester) + + def test_member_create_raises_when_only_have_parent_group_permission(self): + assert_raises(logic.NotAuthorized, + self._member_create_group_hierarchy, + self.roger, # parent + self.group, # child + self.tester) + def test_member_create_accepts_group_name_or_id(self): by_name = self._member_create_in_group(self.pkgs[0].id, 'package', 'public', self.group.name) @@ -139,14 +161,18 @@ def test_member_delete_raises_if_object_type_is_invalid(self): self._member_delete, 'obj_id', 'invalid_obj_type') def _member_create(self, obj, obj_type, capacity): + '''Makes the given object a member of cls.group.''' ctx, dd = self._build_context(obj, obj_type, capacity) return logic.get_action('member_create')(ctx, dd) def _member_create_in_group(self, obj, obj_type, capacity, group_id): + '''Makes the given object a member of the given group.''' ctx, dd = self._build_context(obj, obj_type, capacity, group_id) return logic.get_action('member_create')(ctx, dd) def _member_create_as_user(self, obj, obj_type, capacity, user): + '''Makes the griven object a member of cls.group using privileges of + the given user.''' ctx, dd = self._build_context(obj, obj_type, capacity, user=user) return logic.get_action('member_create')(ctx, dd) @@ -162,6 +188,11 @@ def _member_delete_in_group(self, obj, obj_type, group_id): ctx, dd = self._build_context(obj, obj_type, group_id=group_id) return logic.get_action('member_delete')(ctx, dd) + def _member_create_group_hierarchy(self, parent_group, child_group, user): + ctx, dd = self._build_context(parent_group.name, 'group', 'parent', + group_id=child_group.name, user=user.id) + return logic.get_action('member_create')(ctx, dd) + def _build_context(self, obj, obj_type, capacity='public', group_id=None, user=None): ctx = {'model': model, From 34b3fb7d882afbf024c2929dbba80e5385f2d7f6 Mon Sep 17 00:00:00 2001 From: David Read Date: Thu, 12 Sep 2013 15:05:46 +0000 Subject: [PATCH 17/87] [noticket] Fix paster commands that fail due to The synchronous_search plugin is enabled automatically in ckan/plugins/core.py so a few paster commands failed trying to load it again. Looks like this was broken since 2371c05c 2013-04-05. Affected: create-test-data, dataset delete, dataset purge. --- ckan/lib/cli.py | 3 --- 1 file changed, 3 deletions(-) diff --git a/ckan/lib/cli.py b/ckan/lib/cli.py index 0acae3550bb..a7e89d27991 100644 --- a/ckan/lib/cli.py +++ b/ckan/lib/cli.py @@ -844,7 +844,6 @@ def delete(self, dataset_ref): dataset = self._get_dataset(dataset_ref) old_state = dataset.state - plugins.load('synchronous_search') rev = model.repo.new_revision() dataset.delete() model.repo.commit_and_remove() @@ -857,7 +856,6 @@ def purge(self, dataset_ref): dataset = self._get_dataset(dataset_ref) name = dataset.name - plugins.load('synchronous_search') rev = model.repo.new_revision() dataset.purge() model.repo.commit_and_remove() @@ -1284,7 +1282,6 @@ def command(self): self._load_config() self._setup_app() from ckan import plugins - plugins.load('synchronous_search') # so packages get indexed from create_test_data import CreateTestData if self.args: From 527f3e9fba7831290be6f8ac8d67b78504a051b6 Mon Sep 17 00:00:00 2001 From: David Read Date: Thu, 12 Sep 2013 15:08:25 +0000 Subject: [PATCH 18/87] [#1038] Fix display of tree hierarchy, broken in reversal af6c039. --- ckan/model/group.py | 2 +- ckan/tests/models/test_group.py | 9 +++++++++ 2 files changed, 10 insertions(+), 1 deletion(-) diff --git a/ckan/model/group.py b/ckan/model/group.py index b0c1c22fba5..fc3c5b17b30 100644 --- a/ckan/model/group.py +++ b/ckan/model/group.py @@ -404,7 +404,7 @@ def __repr__(self): WHERE m.table_id = c.group_id AND m.table_name = 'group' AND m.state = 'active' ) -SELECT G.*, child.group_id as parent_id FROM child +SELECT G.*, child.table_id as parent_id FROM child INNER JOIN public.group G ON G.id = child.group_id WHERE G.type = :type AND G.state='active';""" diff --git a/ckan/tests/models/test_group.py b/ckan/tests/models/test_group.py index 13cee1907dc..3c6973b8b40 100644 --- a/ckan/tests/models/test_group.py +++ b/ckan/tests/models/test_group.py @@ -116,6 +116,15 @@ def test_get_children_groups(self): assert_in(res[0]['name'], ('national-health-service', 'food-standards-agency')) assert_in(res[0]['title'], ('National Health Service', 'Food Standards Agency')) + def test_get_children_group_hierarchy__from_top_2(self): + groups = model.Group.by_name(u'department-of-health').\ + get_children_group_hierarchy(type=group_type) + # the first group must be NHS or Food Standards Agency - i.e. on the + # first level down + nhs = groups[0] + assert_in(nhs[0].name, ('national-health-service', 'food-standards-agency')) + assert_equal(model.Group.get(nhs[1]).name, 'department-of-health') + def test_get_children_group_hierarchy__from_top(self): assert_equal(name_set_from_group_tuple(model.Group.by_name(u'department-of-health').\ get_children_group_hierarchy(type=group_type)), From 8babc4f90f6f053c1c509476d183aa61f383ba8f Mon Sep 17 00:00:00 2001 From: David Read Date: Tue, 17 Sep 2013 10:32:19 +0000 Subject: [PATCH 19/87] [#1038] Setting of parent group in the form is now done via group['groups'] rather than custom 'parent' Removed explicit creation of Members in the create/update logic functions - these were a hangover. Instead, the form just needs to ensure the parent group gets added to the group['groups'] and it will get set in the model_save as normal. Group.get_group no longer used anywhere (and the caching looked ropey anyway) now that the previous bit is removed. Added schema validator to ensure that no loops form in the hierarchy. Downwards CTE needed ordering as found it also couldn't be relied upon to be in order it recursed, probably due to the join. Removed duplicate 'groups' key in the group schema. --- ckan/controllers/organization.py | 6 ++-- ckan/lib/dictization/model_save.py | 6 +++- ckan/logic/action/create.py | 10 +----- ckan/logic/action/update.py | 18 ---------- ckan/logic/schema.py | 8 ++--- ckan/logic/validators.py | 19 ++++++++++ ckan/model/group.py | 56 +++++++++++++----------------- ckan/tests/models/test_group.py | 9 +++++ 8 files changed, 65 insertions(+), 67 deletions(-) diff --git a/ckan/controllers/organization.py b/ckan/controllers/organization.py index b44d95e057f..fe2be826a8a 100644 --- a/ckan/controllers/organization.py +++ b/ckan/controllers/organization.py @@ -26,8 +26,10 @@ def _db_to_form_schema(self, group_type=None): into a format suitable for the form (optional)''' pass - def _setup_template_variables(self, context, data_dict, group_type=None): - pass + # This is commented so that the Group controller method runs instead, + # allowing a group plugins to setup template variables. + #def _setup_template_variables(self, context, data_dict, group_type=None): + # pass def _new_template(self, group_type): return 'organization/new.html' diff --git a/ckan/lib/dictization/model_save.py b/ckan/lib/dictization/model_save.py index 4122172b837..88bc106bac9 100644 --- a/ckan/lib/dictization/model_save.py +++ b/ckan/lib/dictization/model_save.py @@ -342,7 +342,11 @@ def group_member_save(context, group_dict, member_table_name): entities = {} Member = model.Member - ModelClass = getattr(model, member_table_name[:-1].capitalize()) + classname = member_table_name[:-1].capitalize() + if classname == 'Organization': + # Organizations use the model.Group class + classname = 'Group' + ModelClass = getattr(model, classname) for entity_dict in entity_list: name_or_id = entity_dict.get('id') or entity_dict.get('name') diff --git a/ckan/logic/action/create.py b/ckan/logic/action/create.py index fcb55ceae64..a3a062ce44a 100644 --- a/ckan/logic/action/create.py +++ b/ckan/logic/action/create.py @@ -15,6 +15,7 @@ import ckan.lib.dictization.model_dictize as model_dictize import ckan.lib.dictization.model_save as model_save import ckan.lib.navl.dictization_functions +import ckan.lib.navl.validators as validators from ckan.common import _ @@ -472,7 +473,6 @@ def _group_or_org_create(context, data_dict, is_org=False): model = context['model'] user = context['user'] session = context['session'] - parent = context.get('parent', None) data_dict['is_organization'] = is_org @@ -512,14 +512,6 @@ def _group_or_org_create(context, data_dict, is_org=False): group = model_save.group_dict_save(data, context) - if parent: - parent_group = model.Group.get( parent ) - if parent_group: - member = model.Member(group=parent_group, table_id=group.id, table_name='group') - session.add(member) - log.debug('Group %s is made child of group %s', - group.name, parent_group.name) - if user: admins = [model.User.by_name(user.decode('utf8'))] else: diff --git a/ckan/logic/action/update.py b/ckan/logic/action/update.py index c841a9bb25c..fcc8f9b5546 100644 --- a/ckan/logic/action/update.py +++ b/ckan/logic/action/update.py @@ -408,7 +408,6 @@ def _group_or_org_update(context, data_dict, is_org=False): user = context['user'] session = context['session'] id = _get_or_bust(data_dict, 'id') - parent = context.get('parent', None) group = model.Group.get(id) context["group"] = group @@ -464,23 +463,6 @@ def _group_or_org_update(context, data_dict, is_org=False): context['prevent_packages_update'] = True group = model_save.group_dict_save(data, context) - if parent: - parent_group = model.Group.get( parent ) - if parent_group and not parent_group in group.get_groups(group.type): - # Delete all of this groups memberships - current = session.query(model.Member).\ - filter(model.Member.table_id == group.id).\ - filter(model.Member.table_name == "group").all() - if current: - log.debug('Parents of group %s deleted: %r', group.name, - [membership.group.name for membership in current]) - for c in current: - session.delete(c) - member = model.Member(group=parent_group, table_id=group.id, table_name='group') - session.add(member) - log.debug('Group %s is made child of group %s', - group.name, parent_group.name) - if is_org: plugin_type = plugins.IOrganizationController else: diff --git a/ckan/logic/schema.py b/ckan/logic/schema.py index f8fe8df7fbd..66d2934c86e 100644 --- a/ckan/logic/schema.py +++ b/ckan/logic/schema.py @@ -50,6 +50,7 @@ url_validator, datasets_with_no_organization_cannot_be_private, list_of_strings, + no_loops_in_hierarchy, ) from ckan.logic.converters import (convert_user_name_or_id_to_id, convert_package_name_or_id_to_id, @@ -274,11 +275,6 @@ def default_group_schema(): "title":[ignore_missing, unicode], "name":[ignore_missing, unicode], "__extras": [ignore] - }, - 'groups': { - "name": [not_empty, unicode], - "capacity": [ignore_missing], - "__extras": [ignore] }, 'users': { "name": [not_empty, unicode], @@ -286,7 +282,7 @@ def default_group_schema(): "__extras": [ignore] }, 'groups': { - "name": [not_empty, unicode], + "name": [not_empty, no_loops_in_hierarchy, unicode], "capacity": [ignore_missing], "__extras": [ignore] } diff --git a/ckan/logic/validators.py b/ckan/logic/validators.py index 429fcb5a774..f9889297e3b 100644 --- a/ckan/logic/validators.py +++ b/ckan/logic/validators.py @@ -649,3 +649,22 @@ def list_of_strings(key, data, errors, context): if not isinstance(x, basestring): raise Invalid('%s: %s' % (_('Not a string'), x)) +def no_loops_in_hierarchy(key, data, errors, context): + '''Checks that the parent groups specified in the data would not cause + a loop in the group hierarchy, and therefore cause the recursion up/down + the hierarchy to get into an infinite loop. + ''' + if not 'id' in data: + # Must be a new group - has no children, so no chance of loops + return + group = context['model'].Group.get(data['id']) + allowable_parents = group.\ + groups_allowed_to_be_its_parent(type=group.type) + for parent in data['groups']: + parent_name = parent['name'] + # a blank name signifies top level, which is always allowed + if parent_name and context['model'].Group.get(parent_name) \ + not in allowable_parents: + raise Invalid(_('This parent would create a loop in the ' + 'hierarchy')) + diff --git a/ckan/model/group.py b/ckan/model/group.py index fc3c5b17b30..3c1b3369cac 100644 --- a/ckan/model/group.py +++ b/ckan/model/group.py @@ -193,11 +193,11 @@ def get_children_groups(self, type='group'): 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 + '''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 + :rtype: a list of tuples, each one a Group and the ID of its parent group. e.g. >>> dept-health.get_children_group_hierarchy() @@ -211,7 +211,7 @@ def get_children_group_hierarchy(self, type='group'): return results def get_parent_group_hierarchy(self, type='group'): - '''Returns this group's parent, parent's parent, parent's parent's + '''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).\ @@ -231,6 +231,22 @@ def get_top_level_groups(cls, type='group'): filter(Group.type == type).\ order_by(Group.title).all() + def groups_allowed_to_be_its_parent(self, type='group'): + '''Returns a list of the groups (of the specified type) which are + allowed to be this group's parent. It excludes ones which would + create a loop in the hierarchy, causing the recursive CTE to + be in an infinite loop. + + :returns: A list of group objects ordered by group title + + ''' + all_groups = self.all(group_type=type) + excluded_groups = set(group.name for group, id_ in + self.get_children_group_hierarchy(type=type)) + excluded_groups.add(self.name) + return [group for group in all_groups + if group.name not in excluded_groups] + def packages(self, with_private=False, limit=None, return_query=False, context=None): '''Return this group's active and pending packages. @@ -319,28 +335,6 @@ def add_package_by_name(self, package_name): table_name='package') meta.Session.add(member) - def get_groups(self, group_type=None, capacity=None): - """ Get all groups that this group is within (i.e. this group's child - groups) - - """ - import ckan.model as model - # DR: Why is this cached? Surely the members can change in the - # lifetime of this Group? - if '_groups' not in self.__dict__: - self._groups = meta.Session.query(model.Group).\ - join(model.Member, model.Member.group_id == model.Group.id and - model.Member.table_name == 'group').\ - filter(model.Member.state == 'active').\ - filter(model.Member.table_id == self.id).all() - - groups = self._groups - if group_type: - groups = [g for g in groups if g.type == group_type] - if capacity: - groups = [g for g in groups if g.capacity == capacity] - return groups - @property def all_related_revisions(self): '''Returns chronological list of all object revisions related to @@ -368,7 +362,6 @@ def all_related_revisions(self): def __repr__(self): return '' % self.name - meta.mapper(Group, group_table, extension=[vdm.sqlalchemy.Revisioner(group_revision_table), ], ) @@ -393,20 +386,21 @@ def __repr__(self): MemberRevision.related_packages = lambda self: [self.continuity.package] -HIERARCHY_DOWNWARDS_CTE = """WITH RECURSIVE child AS +HIERARCHY_DOWNWARDS_CTE = """WITH RECURSIVE child(depth) AS ( -- non-recursive term - SELECT * FROM member + SELECT 0, * FROM member WHERE table_id = :id AND table_name = 'group' AND state = 'active' UNION ALL -- recursive term - SELECT m.* FROM member AS m, child AS c + SELECT c.depth + 1, m.* FROM member AS m, child AS c WHERE m.table_id = c.group_id AND m.table_name = 'group' AND m.state = 'active' ) -SELECT G.*, child.table_id as parent_id FROM child +SELECT G.*, child.depth, child.table_id as parent_id FROM child INNER JOIN public.group G ON G.id = child.group_id - WHERE G.type = :type AND G.state='active';""" + WHERE G.type = :type AND G.state='active' + ORDER BY child.depth ASC;""" HIERARCHY_UPWARDS_CTE = """WITH RECURSIVE parenttree(depth) AS ( -- non-recursive term diff --git a/ckan/tests/models/test_group.py b/ckan/tests/models/test_group.py index 3c6973b8b40..8e12a0fe9b2 100644 --- a/ckan/tests/models/test_group.py +++ b/ckan/tests/models/test_group.py @@ -165,6 +165,15 @@ def test_get_top_level_groups(self): get_top_level_groups(type=group_type)), ['cabinet-office', 'department-of-health']) + def test_groups_allowed_to_be_its_parent(self): + groups = model.Group.by_name(u'national-health-service').\ + groups_allowed_to_be_its_parent(type=group_type) + names = names_from_groups(groups) + assert_in('department-of-health', names) + assert_in('cabinet-office', names) + assert_not_in('natonal-health-service', names) + assert_not_in('nhs-wirral-ccg', names) + class TestGroupRevisions: @classmethod def setup_class(self): From 407cf91a69b2bb8ae9e6763048aa6241b2001e8e Mon Sep 17 00:00:00 2001 From: David Read Date: Tue, 17 Sep 2013 14:09:59 +0000 Subject: [PATCH 20/87] [#1038] Fix group deletion mid-hierarchy. --- ckan/logic/action/delete.py | 11 +++++++++++ 1 file changed, 11 insertions(+) diff --git a/ckan/logic/action/delete.py b/ckan/logic/action/delete.py index 933e7624b43..8803768db22 100644 --- a/ckan/logic/action/delete.py +++ b/ckan/logic/action/delete.py @@ -1,5 +1,7 @@ '''API functions for deleting data from CKAN.''' +from sqlalchemy import or_ + import ckan.logic import ckan.logic.action import ckan.plugins as plugins @@ -252,6 +254,15 @@ def _group_or_org_delete(context, data_dict, is_org=False): rev = model.repo.new_revision() rev.author = user rev.message = _(u'REST API: Delete %s') % revisioned_details + + # The group's Member objects are deleted + # (including hierarchy connections to parent and children groups) + for member in model.Session.query(model.Member).\ + filter(or_(model.Member.table_id == id, + model.Member.group_id == id)).\ + filter(model.Member.state == 'active').all(): + member.delete() + group.delete() if is_org: From df890fc12c9ce68959898337e48d89fdbc4b5b68 Mon Sep 17 00:00:00 2001 From: David Read Date: Tue, 17 Sep 2013 15:16:57 +0000 Subject: [PATCH 21/87] [#1038] Config option fixed and documented. And tidy typos etc. --- ckan/new_authz.py | 4 ++++ ckan/tests/logic/test_auth.py | 8 ++++---- ckan/tests/logic/test_member.py | 2 +- doc/configuration.rst | 16 ++++++++++++++++ 4 files changed, 25 insertions(+), 5 deletions(-) diff --git a/ckan/new_authz.py b/ckan/new_authz.py index 495f3806b17..d2a355234c6 100644 --- a/ckan/new_authz.py +++ b/ckan/new_authz.py @@ -367,6 +367,10 @@ def check_config_permission(permission): CONFIG_PERMISSIONS[perm] = config.get(key, default) if isinstance(default, bool): CONFIG_PERMISSIONS[perm] = asbool(CONFIG_PERMISSIONS[perm]) + elif isinstance(default, list): + CONFIG_PERMISSIONS[perm] = \ + CONFIG_PERMISSIONS[perm].split(' ') \ + if CONFIG_PERMISSIONS[perm] else [] if permission in CONFIG_PERMISSIONS: return CONFIG_PERMISSIONS[permission] return False diff --git a/ckan/tests/logic/test_auth.py b/ckan/tests/logic/test_auth.py index 2dc6b5e9a5a..0fbad3af803 100644 --- a/ckan/tests/logic/test_auth.py +++ b/ckan/tests/logic/test_auth.py @@ -210,7 +210,7 @@ def test_11_delete_org(self): class TestAuthOrgHierarchy(TestAuth): # Tests are in the same vein as TestAuthOrgs, testing the cases where the - # group hierarchy provices extra permissions through cascading + # group hierarchy provides extra permissions through cascading @classmethod def setup_class(cls): @@ -224,7 +224,7 @@ def setup_class(cls): 'groups': ['national-health-service']}], extra_user_names=['john']) - def _reset_adatasets_owner_org(self): + def _reset_a_datasets_owner_org(self): rev = model.repo.new_revision() get_action('package_owner_org_update')( {'model': model, 'ignore_auth': True}, @@ -304,7 +304,7 @@ def test_08_update_datasets_3(self): try: self._call_api('package_update', dataset, 'nhsadmin', 200) finally: - self._reset_adatasets_owner_org() + self._reset_a_datasets_owner_org() def test_08_update_datasets_4(self): dataset = {'name': 'adataset', 'owner_org': 'department-of-health'} @@ -315,7 +315,7 @@ def test_08_update_datasets_5(self): try: self._call_api('package_update', dataset, 'nhseditor', 200) finally: - self._reset_adatasets_owner_org() + self._reset_a_datasets_owner_org() def test_08_update_datasets_6(self): dataset = {'name': 'adataset', 'owner_org': 'nhs-wirral-ccg'} diff --git a/ckan/tests/logic/test_member.py b/ckan/tests/logic/test_member.py index f3d0ec448a0..fa55696624b 100644 --- a/ckan/tests/logic/test_member.py +++ b/ckan/tests/logic/test_member.py @@ -171,7 +171,7 @@ def _member_create_in_group(self, obj, obj_type, capacity, group_id): return logic.get_action('member_create')(ctx, dd) def _member_create_as_user(self, obj, obj_type, capacity, user): - '''Makes the griven object a member of cls.group using privileges of + '''Makes the given object a member of cls.group using privileges of the given user.''' ctx, dd = self._build_context(obj, obj_type, capacity, user=user) return logic.get_action('member_create')(ctx, dd) diff --git a/doc/configuration.rst b/doc/configuration.rst index 284dbbfde76..61eb0754573 100644 --- a/doc/configuration.rst +++ b/doc/configuration.rst @@ -355,6 +355,22 @@ Default value: ``False`` Allow new user accounts to be created via the API. +.. _ckan.auth.roles_that_cascade_to_sub_groups: + +ckan.auth.roles_that_cascade_to_sub_groups +^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ + +Example:: + + ckan.auth.roles_that_cascade_to_sub_groups = admin editor + +Default value: ``admin`` + + +Makes role permissions apply to all the groups down the hierarchy from the groups that the role is applied to. + +e.g. a particular user has the 'admin' role for group 'Department of Health'. If you set the value of this option to 'admin' then the user will automatically have the same admin permissions for the child groups of 'Department of Health' such as 'Cancer Research' (and its children too and so on). + .. end_config-authorization From e023b44abe2d1fa502ba8f7cd746d9a87e0500ab Mon Sep 17 00:00:00 2001 From: David Read Date: Tue, 17 Sep 2013 15:18:52 +0000 Subject: [PATCH 22/87] [#1038] Fix an apparently botched merge. --- doc/solr-setup.rst | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/doc/solr-setup.rst b/doc/solr-setup.rst index 6d5bb785559..8b5b90ae89b 100644 --- a/doc/solr-setup.rst +++ b/doc/solr-setup.rst @@ -212,17 +212,17 @@ Some problems that can be found during the install: ${dataDir} -* When running Solr it says `Unable to find a javac compiler; com.sun.tools.javac.Main is not on the classpath. Perhaps JAVA_HOME does not point to the JDK.` +* When running Solr it says ``Unable to find a javac compiler; com.sun.tools.javac.Main is not on the classpath. Perhaps JAVA_HOME does not point to the JDK.`` - See the note above about JAVA_HOME. Alternatively you may not have installed the JDK. Check by seeing if javac is installed:: + See the note above about JAVA_HOME. Alternatively you may not have installed the JDK. Check by seeing if javac is installed:: which javac - If it isn't do:: + If it isn't do:: sudo apt-get install openjdk-6-jdk - and restart SOLR. + and restart SOLR. Handling changes in the CKAN schema ----------------------------------- From 4290821c16217fedf652efb9187f43e29a28b269 Mon Sep 17 00:00:00 2001 From: David Read Date: Tue, 17 Sep 2013 15:20:26 +0000 Subject: [PATCH 23/87] [#1038] Fix an apparently botched merge (cont). --- doc/solr-setup.rst | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/doc/solr-setup.rst b/doc/solr-setup.rst index 8b5b90ae89b..bc1961dd375 100644 --- a/doc/solr-setup.rst +++ b/doc/solr-setup.rst @@ -212,7 +212,7 @@ Some problems that can be found during the install: ${dataDir} -* When running Solr it says ``Unable to find a javac compiler; com.sun.tools.javac.Main is not on the classpath. Perhaps JAVA_HOME does not point to the JDK.`` +* When running Solr it says ``Unable to find a javac compiler; com.sun.tools.javac.Main is not on the classpath. Perhaps ``JAVA_HOME`` does not point to the JDK.`` See the note above about JAVA_HOME. Alternatively you may not have installed the JDK. Check by seeing if javac is installed:: @@ -222,7 +222,7 @@ Some problems that can be found during the install: sudo apt-get install openjdk-6-jdk - and restart SOLR. + and restart Solr. Handling changes in the CKAN schema ----------------------------------- From 24548c0a401f3dd4d4f16a087278065173e60a68 Mon Sep 17 00:00:00 2001 From: David Read Date: Tue, 17 Sep 2013 15:22:07 +0000 Subject: [PATCH 24/87] [#1038] Fix an apparently botched merge (cont). --- doc/solr-setup.rst | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/doc/solr-setup.rst b/doc/solr-setup.rst index bc1961dd375..a628ad2fe3b 100644 --- a/doc/solr-setup.rst +++ b/doc/solr-setup.rst @@ -212,9 +212,9 @@ Some problems that can be found during the install: ${dataDir} -* When running Solr it says ``Unable to find a javac compiler; com.sun.tools.javac.Main is not on the classpath. Perhaps ``JAVA_HOME`` does not point to the JDK.`` +* When running Solr it says ``Unable to find a javac compiler; com.sun.tools.javac.Main is not on the classpath. Perhaps JAVA_HOME does not point to the JDK.`` - See the note above about JAVA_HOME. Alternatively you may not have installed the JDK. Check by seeing if javac is installed:: + See the note above about ``JAVA_HOME``. Alternatively you may not have installed the JDK. Check by seeing if javac is installed:: which javac From fafd9f9f32a4795bca8c2eed61001b70fdcf770a Mon Sep 17 00:00:00 2001 From: David Read Date: Tue, 17 Sep 2013 16:19:29 +0000 Subject: [PATCH 25/87] [#1038] Fix reading config. --- ckan/config/deployment.ini_tmpl | 1 + ckan/new_authz.py | 2 +- 2 files changed, 2 insertions(+), 1 deletion(-) diff --git a/ckan/config/deployment.ini_tmpl b/ckan/config/deployment.ini_tmpl index 59e0a2e789c..a19862a4f73 100644 --- a/ckan/config/deployment.ini_tmpl +++ b/ckan/config/deployment.ini_tmpl @@ -66,6 +66,7 @@ ckan.auth.user_create_organizations = true ckan.auth.user_delete_groups = true ckan.auth.user_delete_organizations = true ckan.auth.create_user_via_api = false +ckan.auth.roles_that_cascade_to_sub_groups = admin ## Search Settings diff --git a/ckan/new_authz.py b/ckan/new_authz.py index 4dc15197643..7593cc1f1f3 100644 --- a/ckan/new_authz.py +++ b/ckan/new_authz.py @@ -387,7 +387,7 @@ def check_config_permission(permission): CONFIG_PERMISSIONS[perm] = config.get(key, default) if isinstance(default, bool): CONFIG_PERMISSIONS[perm] = asbool(CONFIG_PERMISSIONS[perm]) - elif isinstance(default, list): + elif isinstance(default, list) and key in config: CONFIG_PERMISSIONS[perm] = \ CONFIG_PERMISSIONS[perm].split(' ') \ if CONFIG_PERMISSIONS[perm] else [] From b7136b8beccf3817294ce6951209e26b389a4293 Mon Sep 17 00:00:00 2001 From: David Read Date: Mon, 23 Sep 2013 12:06:54 +0100 Subject: [PATCH 26/87] [#1038] Fix top level groups including deleted ones. --- ckan/model/group.py | 1 + 1 file changed, 1 insertion(+) diff --git a/ckan/model/group.py b/ckan/model/group.py index 3c1b3369cac..2c69c11a539 100644 --- a/ckan/model/group.py +++ b/ckan/model/group.py @@ -229,6 +229,7 @@ def get_top_level_groups(cls, type='group'): Member.state == 'active')).\ filter(Member.id == None).\ filter(Group.type == type).\ + filter(Group.state == 'active').\ order_by(Group.title).all() def groups_allowed_to_be_its_parent(self, type='group'): From 1be9681cf67d5d0990aa22673d32b5d7abaa24f7 Mon Sep 17 00:00:00 2001 From: David Read Date: Tue, 24 Sep 2013 15:17:08 +0100 Subject: [PATCH 27/87] [noticket] Fix displaying About page for organizations. --- ckan/controllers/group.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/ckan/controllers/group.py b/ckan/controllers/group.py index ccef46bfe78..3381bc69abd 100644 --- a/ckan/controllers/group.py +++ b/ckan/controllers/group.py @@ -824,7 +824,7 @@ def _get_group_dict(self, id): 'user': c.user or c.author, 'for_view': True} try: - return get_action('group_show')(context, {'id': id}) + return self._action('group_show')(context, {'id': id}) except NotFound: abort(404, _('Group not found')) except NotAuthorized: From ed78fc9ed5448685413597e3871f30e69e9bc751 Mon Sep 17 00:00:00 2001 From: David Read Date: Tue, 24 Sep 2013 15:18:29 +0100 Subject: [PATCH 28/87] [#1038] Improve performance of viewing hierarchy, avoiding groups as SQLAlchemy objects. --- ckan/model/group.py | 20 +++++++++++--------- ckan/tests/models/test_group.py | 6 +++--- 2 files changed, 14 insertions(+), 12 deletions(-) diff --git a/ckan/model/group.py b/ckan/model/group.py index 2c69c11a539..6bf544bd635 100644 --- a/ckan/model/group.py +++ b/ckan/model/group.py @@ -197,15 +197,16 @@ def get_children_group_hierarchy(self, type='group'): hierarchy. The ordering is such that children always come after their parent. - :rtype: a list of tuples, each one a Group and the ID of its parent - group. + :rtype: a list of tuples, each one a Group ID, name and title and then + the ID of its parent group. - e.g. >>> dept-health.get_children_group_hierarchy() - [(, u'8a163ba7-5146-4325-90c8-fe53b25e28d0'), - (, u'06e6dbf5-d801-40a1-9dc0-6785340b2ab4'), - (, u'd2e25b41-720c-4ba7-bc8f-bb34b185b3dd')] + e.g. + >>> dept-health.get_children_group_hierarchy() + [(u'8ac0...', u'national-health-service', u'National Health Service', u'e041...'), + (u'b468...', u'nhs-wirral-ccg', u'NHS Wirral CCG', u'8ac0...')] ''' - results = meta.Session.query(Group, 'parent_id').\ + results = meta.Session.query(Group.id, Group.name, Group.title, + 'parent_id').\ from_statement(HIERARCHY_DOWNWARDS_CTE).\ params(id=self.id, type=type).all() return results @@ -242,7 +243,8 @@ def groups_allowed_to_be_its_parent(self, type='group'): ''' all_groups = self.all(group_type=type) - excluded_groups = set(group.name for group, id_ in + excluded_groups = set(group_name + for group_id, group_name, group_title, parent in self.get_children_group_hierarchy(type=type)) excluded_groups.add(self.name) return [group for group in all_groups @@ -398,7 +400,7 @@ def __repr__(self): WHERE m.table_id = c.group_id AND m.table_name = 'group' AND m.state = 'active' ) -SELECT G.*, child.depth, child.table_id as parent_id FROM child +SELECT G.id, G.name, G.title, child.depth, child.table_id as parent_id FROM child INNER JOIN public.group G ON G.id = child.group_id WHERE G.type = :type AND G.state='active' ORDER BY child.depth ASC;""" diff --git a/ckan/tests/models/test_group.py b/ckan/tests/models/test_group.py index 8e12a0fe9b2..71567481953 100644 --- a/ckan/tests/models/test_group.py +++ b/ckan/tests/models/test_group.py @@ -93,7 +93,7 @@ def _search_results(self, query, is_org=False): return set([group.name for group in results]) name_set_from_dicts = lambda groups: set([group['name'] for group in groups]) -name_set_from_group_tuple = lambda tuples: set([t[0].name for t in tuples]) +name_set_from_group_tuple = lambda tuples: set([t[1] for t in tuples]) name_set_from_groups = lambda groups: set([group.name for group in groups]) names_from_groups = lambda groups: [group.name for group in groups] @@ -122,8 +122,8 @@ def test_get_children_group_hierarchy__from_top_2(self): # the first group must be NHS or Food Standards Agency - i.e. on the # first level down nhs = groups[0] - assert_in(nhs[0].name, ('national-health-service', 'food-standards-agency')) - assert_equal(model.Group.get(nhs[1]).name, 'department-of-health') + assert_in(nhs[1], ('national-health-service', 'food-standards-agency')) + assert_equal(model.Group.get(nhs[3]).name, 'department-of-health') def test_get_children_group_hierarchy__from_top(self): assert_equal(name_set_from_group_tuple(model.Group.by_name(u'department-of-health').\ From 218f73bdbdb06ef9a192dd07816e87ac6486dfcb Mon Sep 17 00:00:00 2001 From: David Read Date: Tue, 24 Sep 2013 19:59:01 +0000 Subject: [PATCH 29/87] [#1038] get_parent_groups added * get_parent_groups is used by DGU and no doubt others that use hierarchy. * get_children_groups changed to return objects as that is more convenient for DGU. Conflicts: ckan/tests/models/test_group.py --- ckan/model/group.py | 24 ++++++++++++++++++------ ckan/tests/models/test_group.py | 27 +++++++++++++++++++++------ 2 files changed, 39 insertions(+), 12 deletions(-) diff --git a/ckan/model/group.py b/ckan/model/group.py index 6bf544bd635..df5e8124ed9 100644 --- a/ckan/model/group.py +++ b/ckan/model/group.py @@ -175,12 +175,12 @@ def set_approval_status(self, status): 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).\ + # old CTE actually did, but is now far simpler, and returns Group objects + # instead of a dict. + return meta.Session.query(Group).\ filter_by(type=type).\ filter_by(state='active').\ join(Member, Member.group_id == Group.id).\ @@ -189,9 +189,6 @@ def get_children_groups(self, type='group'): filter_by(state='active').\ all() - 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 @@ -211,6 +208,21 @@ def get_children_group_hierarchy(self, type='group'): params(id=self.id, type=type).all() return results + def get_parent_groups(self, type='group'): + '''Returns this group's parent groups. + Returns a list. Will have max 1 value for organizations. + + ''' + return meta.Session.query(Group).\ + join(Member, + and_(Member.table_id == Group.id, + Member.table_name == 'group', + Member.state == 'active')).\ + filter(Member.group_id == self.id).\ + filter(Group.type == type).\ + filter(Group.state == 'active').\ + all() + 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.''' diff --git a/ckan/tests/models/test_group.py b/ckan/tests/models/test_group.py index 71567481953..68c41864007 100644 --- a/ckan/tests/models/test_group.py +++ b/ckan/tests/models/test_group.py @@ -1,4 +1,4 @@ -from ckan.tests import assert_equal, assert_not_in, assert_in +from nose.tools import assert_equal, assert_in, assert_not_in import ckan.model as model from ckan.tests import * @@ -108,13 +108,13 @@ def test_get_children_groups(self): res = model.Group.by_name(u'department-of-health').\ get_children_groups(type=group_type) # check groups - assert_equal(name_set_from_dicts(res), + assert_equal(name_set_from_groups(res), set(('national-health-service', 'food-standards-agency'))) - # check each group is expressed as a small dict - assert_equal(set(res[0].keys()), set(('id', 'name', 'title'))) - assert_in(res[0]['name'], ('national-health-service', 'food-standards-agency')) - assert_in(res[0]['title'], ('National Health Service', 'Food Standards Agency')) + # check each group is a Group + assert isinstance(res[0], model.Group) + assert_in(res[0].name, ('national-health-service', 'food-standards-agency')) + assert_in(res[0].title, ('National Health Service', 'Food Standards Agency')) def test_get_children_group_hierarchy__from_top_2(self): groups = model.Group.by_name(u'department-of-health').\ @@ -144,6 +144,21 @@ def test_get_children_group_hierarchy__from_bottom_tier(self): get_children_group_hierarchy(type=group_type)), set()) + def test_get_parents__top(self): + assert_equal(names_from_groups(model.Group.by_name(u'department-of-health').\ + get_parent_groups(type=group_type)), + []) + + def test_get_parents__tier_two(self): + assert_equal(names_from_groups(model.Group.by_name(u'national-health-service').\ + get_parent_groups(type=group_type)), + ['department-of-health']) + + def test_get_parents__tier_three(self): + assert_equal(names_from_groups(model.Group.by_name(u'nhs-wirral-ccg').\ + get_parent_groups(type=group_type)), + ['national-health-service']) + def test_get_parent_groups_up_hierarchy__from_top(self): assert_equal(names_from_groups(model.Group.by_name(u'department-of-health').\ get_parent_group_hierarchy(type=group_type)), From 18d3c8b4a27e789f3e0b5ad00e954ee53cde5129 Mon Sep 17 00:00:00 2001 From: David Read Date: Mon, 30 Sep 2013 13:33:51 +0000 Subject: [PATCH 30/87] [noticket] Fix organization template that checked package_create auth incorrectly. --- ckan/logic/auth/create.py | 3 ++- ckan/templates/organization/read.html | 4 ++-- 2 files changed, 4 insertions(+), 3 deletions(-) diff --git a/ckan/logic/auth/create.py b/ckan/logic/auth/create.py index a46a91de500..944ba31170d 100644 --- a/ckan/logic/auth/create.py +++ b/ckan/logic/auth/create.py @@ -117,6 +117,7 @@ def user_create(context, data_dict=None): def _check_group_auth(context, data_dict): '''Has this user got update permission for all of the given groups? If there is a package in the context then ignore that package's groups. + (owner_org is checked elsewhere.) :returns: False if not allowed to update one (or more) of the given groups. True otherwise. i.e. True is the default. A blank data_dict mentions no groups, so it returns True. @@ -133,7 +134,7 @@ def _check_group_auth(context, data_dict): api_version = context.get('api_version') or '1' - group_blobs = data_dict.get("groups", []) + group_blobs = data_dict.get('groups', []) groups = set() for group_blob in group_blobs: # group_blob might be a dict or a group_ref diff --git a/ckan/templates/organization/read.html b/ckan/templates/organization/read.html index aae8f995b20..0cbd4093bbc 100644 --- a/ckan/templates/organization/read.html +++ b/ckan/templates/organization/read.html @@ -1,8 +1,8 @@ {% extends "organization/read_base.html" %} {% block page_primary_action %} - {% if h.check_access('package_create', {'organization_id': c.group_dict.id}) %} - {% link_for _('Add Dataset'), controller='package', action='new', group=c.group_dict.id, class_='btn btn-primary', icon='plus-sign-alt' %} + {% if h.check_access('package_create', {'owner_org': c.group_dict.id}) %} + {% link_for _('Add Dataset'), controller='package', action='new', group=c.group_dict.name, class_='btn btn-primary', icon='plus-sign-alt' %} {% endif %} {% endblock %} From 399a6a4ee274c102250035b7be363fbe37881f29 Mon Sep 17 00:00:00 2001 From: David Read Date: Thu, 3 Oct 2013 14:57:12 +0000 Subject: [PATCH 31/87] [#1038] Add a hard limit to recursion - just in case. Conflicts: ckan/model/group.py --- ckan/model/group.py | 14 ++++++++++---- 1 file changed, 10 insertions(+), 4 deletions(-) diff --git a/ckan/model/group.py b/ckan/model/group.py index df5e8124ed9..695a1902fbc 100644 --- a/ckan/model/group.py +++ b/ckan/model/group.py @@ -400,6 +400,11 @@ def __repr__(self): #TODO MemberRevision.related_packages = lambda self: [self.continuity.package] +# Should there arise a bug that allows loops in the group hierarchy, then it +# will lead to infinite recursion, tieing up postgres processes at 100%, and +# the server will suffer. To avoid ever failing this badly, we put in this +# limit on recursion. +MAX_RECURSES = 8 HIERARCHY_DOWNWARDS_CTE = """WITH RECURSIVE child(depth) AS ( @@ -410,12 +415,12 @@ def __repr__(self): -- recursive term SELECT c.depth + 1, m.* FROM member AS m, child AS c WHERE m.table_id = c.group_id AND m.table_name = 'group' - AND m.state = 'active' + AND m.state = 'active' AND c.depth < {max_recurses} ) SELECT G.id, G.name, G.title, child.depth, child.table_id as parent_id FROM child INNER JOIN public.group G ON G.id = child.group_id WHERE G.type = :type AND G.state='active' - ORDER BY child.depth ASC;""" + ORDER BY child.depth ASC;""".format(max_recurses=MAX_RECURSES) HIERARCHY_UPWARDS_CTE = """WITH RECURSIVE parenttree(depth) AS ( -- non-recursive term @@ -425,10 +430,11 @@ def __repr__(self): -- recursive term SELECT PG.depth + 1, M.* FROM parenttree PG, public.member M WHERE PG.table_id = M.group_id AND M.table_name = 'group' - AND M.state = 'active' + AND M.state = 'active' AND PG.depth < {max_recurses} ) SELECT G.*, PT.depth FROM parenttree AS PT INNER JOIN public.group G ON G.id = PT.table_id WHERE G.type = :type AND G.state='active' - ORDER BY PT.depth DESC;""" + ORDER BY PT.depth DESC;""".format(max_recurses=MAX_RECURSES) + From 75a96e9300a655d7a8f3123a29a66bd9d52b70cb Mon Sep 17 00:00:00 2001 From: Vitor Baptista Date: Fri, 18 Oct 2013 10:31:43 -0300 Subject: [PATCH 32/87] Upgrade dependencies --- requirements.txt | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/requirements.txt b/requirements.txt index e16309daa8a..07320c945d4 100644 --- a/requirements.txt +++ b/requirements.txt @@ -4,7 +4,7 @@ FormAlchemy==1.4.3 FormEncode==1.2.6 Genshi==0.6 Jinja2==2.6 -Mako==0.8.1 +Mako==0.9.0 MarkupSafe==0.18 Pairtree==0.7.1-T Paste==1.7.5.1 @@ -22,7 +22,6 @@ WebTest==1.4.3 apachemiddleware==0.1.1 argparse==1.2.1 decorator==3.4.0 -distribute==0.6.34 fanstatic==0.12 nose==1.3.0 ofs==0.4.1 @@ -35,7 +34,7 @@ repoze.who==1.0.19 repoze.who-friendlyform==1.0.8 repoze.who.plugins.openid==0.5.3 requests==1.1.0 -simplejson==3.3.0 +simplejson==3.3.1 solrpy==0.9.5 sqlalchemy-migrate==0.7.2 unicodecsv==0.9.4 From 3ac326584ec6c38e446193bdc226784dede35cad Mon Sep 17 00:00:00 2001 From: John Martin Date: Thu, 24 Oct 2013 13:31:40 +0100 Subject: [PATCH 33/87] [#1277] Update to jQuery UI libs --- ckan/public/base/vendor/jquery.ui.core.js | 320 ++++ ckan/public/base/vendor/jquery.ui.mouse.js | 169 +++ ckan/public/base/vendor/jquery.ui.sortable.js | 1285 +++++++++++++++++ ckan/public/base/vendor/jquery.ui.widget.js | 501 +++++-- ckan/public/base/vendor/resource.config | 8 +- 5 files changed, 2151 insertions(+), 132 deletions(-) create mode 100755 ckan/public/base/vendor/jquery.ui.core.js create mode 100755 ckan/public/base/vendor/jquery.ui.mouse.js create mode 100755 ckan/public/base/vendor/jquery.ui.sortable.js mode change 100644 => 100755 ckan/public/base/vendor/jquery.ui.widget.js diff --git a/ckan/public/base/vendor/jquery.ui.core.js b/ckan/public/base/vendor/jquery.ui.core.js new file mode 100755 index 00000000000..91ca5ff5056 --- /dev/null +++ b/ckan/public/base/vendor/jquery.ui.core.js @@ -0,0 +1,320 @@ +/*! + * jQuery UI Core 1.10.3 + * http://jqueryui.com + * + * Copyright 2013 jQuery Foundation and other contributors + * Released under the MIT license. + * http://jquery.org/license + * + * http://api.jqueryui.com/category/ui-core/ + */ +(function( $, undefined ) { + +var uuid = 0, + runiqueId = /^ui-id-\d+$/; + +// $.ui might exist from components with no dependencies, e.g., $.ui.position +$.ui = $.ui || {}; + +$.extend( $.ui, { + version: "1.10.3", + + keyCode: { + BACKSPACE: 8, + COMMA: 188, + DELETE: 46, + DOWN: 40, + END: 35, + ENTER: 13, + ESCAPE: 27, + HOME: 36, + LEFT: 37, + NUMPAD_ADD: 107, + NUMPAD_DECIMAL: 110, + NUMPAD_DIVIDE: 111, + NUMPAD_ENTER: 108, + NUMPAD_MULTIPLY: 106, + NUMPAD_SUBTRACT: 109, + PAGE_DOWN: 34, + PAGE_UP: 33, + PERIOD: 190, + RIGHT: 39, + SPACE: 32, + TAB: 9, + UP: 38 + } +}); + +// plugins +$.fn.extend({ + focus: (function( orig ) { + return function( delay, fn ) { + return typeof delay === "number" ? + this.each(function() { + var elem = this; + setTimeout(function() { + $( elem ).focus(); + if ( fn ) { + fn.call( elem ); + } + }, delay ); + }) : + orig.apply( this, arguments ); + }; + })( $.fn.focus ), + + scrollParent: function() { + var scrollParent; + if (($.ui.ie && (/(static|relative)/).test(this.css("position"))) || (/absolute/).test(this.css("position"))) { + scrollParent = this.parents().filter(function() { + return (/(relative|absolute|fixed)/).test($.css(this,"position")) && (/(auto|scroll)/).test($.css(this,"overflow")+$.css(this,"overflow-y")+$.css(this,"overflow-x")); + }).eq(0); + } else { + scrollParent = this.parents().filter(function() { + return (/(auto|scroll)/).test($.css(this,"overflow")+$.css(this,"overflow-y")+$.css(this,"overflow-x")); + }).eq(0); + } + + return (/fixed/).test(this.css("position")) || !scrollParent.length ? $(document) : scrollParent; + }, + + zIndex: function( zIndex ) { + if ( zIndex !== undefined ) { + return this.css( "zIndex", zIndex ); + } + + if ( this.length ) { + var elem = $( this[ 0 ] ), position, value; + while ( elem.length && elem[ 0 ] !== document ) { + // Ignore z-index if position is set to a value where z-index is ignored by the browser + // This makes behavior of this function consistent across browsers + // WebKit always returns auto if the element is positioned + position = elem.css( "position" ); + if ( position === "absolute" || position === "relative" || position === "fixed" ) { + // IE returns 0 when zIndex is not specified + // other browsers return a string + // we ignore the case of nested elements with an explicit value of 0 + //
+ value = parseInt( elem.css( "zIndex" ), 10 ); + if ( !isNaN( value ) && value !== 0 ) { + return value; + } + } + elem = elem.parent(); + } + } + + return 0; + }, + + uniqueId: function() { + return this.each(function() { + if ( !this.id ) { + this.id = "ui-id-" + (++uuid); + } + }); + }, + + removeUniqueId: function() { + return this.each(function() { + if ( runiqueId.test( this.id ) ) { + $( this ).removeAttr( "id" ); + } + }); + } +}); + +// selectors +function focusable( element, isTabIndexNotNaN ) { + var map, mapName, img, + nodeName = element.nodeName.toLowerCase(); + if ( "area" === nodeName ) { + map = element.parentNode; + mapName = map.name; + if ( !element.href || !mapName || map.nodeName.toLowerCase() !== "map" ) { + return false; + } + img = $( "img[usemap=#" + mapName + "]" )[0]; + return !!img && visible( img ); + } + return ( /input|select|textarea|button|object/.test( nodeName ) ? + !element.disabled : + "a" === nodeName ? + element.href || isTabIndexNotNaN : + isTabIndexNotNaN) && + // the element and all of its ancestors must be visible + visible( element ); +} + +function visible( element ) { + return $.expr.filters.visible( element ) && + !$( element ).parents().addBack().filter(function() { + return $.css( this, "visibility" ) === "hidden"; + }).length; +} + +$.extend( $.expr[ ":" ], { + data: $.expr.createPseudo ? + $.expr.createPseudo(function( dataName ) { + return function( elem ) { + return !!$.data( elem, dataName ); + }; + }) : + // support: jQuery <1.8 + function( elem, i, match ) { + return !!$.data( elem, match[ 3 ] ); + }, + + focusable: function( element ) { + return focusable( element, !isNaN( $.attr( element, "tabindex" ) ) ); + }, + + tabbable: function( element ) { + var tabIndex = $.attr( element, "tabindex" ), + isTabIndexNaN = isNaN( tabIndex ); + return ( isTabIndexNaN || tabIndex >= 0 ) && focusable( element, !isTabIndexNaN ); + } +}); + +// support: jQuery <1.8 +if ( !$( "" ).outerWidth( 1 ).jquery ) { + $.each( [ "Width", "Height" ], function( i, name ) { + var side = name === "Width" ? [ "Left", "Right" ] : [ "Top", "Bottom" ], + type = name.toLowerCase(), + orig = { + innerWidth: $.fn.innerWidth, + innerHeight: $.fn.innerHeight, + outerWidth: $.fn.outerWidth, + outerHeight: $.fn.outerHeight + }; + + function reduce( elem, size, border, margin ) { + $.each( side, function() { + size -= parseFloat( $.css( elem, "padding" + this ) ) || 0; + if ( border ) { + size -= parseFloat( $.css( elem, "border" + this + "Width" ) ) || 0; + } + if ( margin ) { + size -= parseFloat( $.css( elem, "margin" + this ) ) || 0; + } + }); + return size; + } + + $.fn[ "inner" + name ] = function( size ) { + if ( size === undefined ) { + return orig[ "inner" + name ].call( this ); + } + + return this.each(function() { + $( this ).css( type, reduce( this, size ) + "px" ); + }); + }; + + $.fn[ "outer" + name] = function( size, margin ) { + if ( typeof size !== "number" ) { + return orig[ "outer" + name ].call( this, size ); + } + + return this.each(function() { + $( this).css( type, reduce( this, size, true, margin ) + "px" ); + }); + }; + }); +} + +// support: jQuery <1.8 +if ( !$.fn.addBack ) { + $.fn.addBack = function( selector ) { + return this.add( selector == null ? + this.prevObject : this.prevObject.filter( selector ) + ); + }; +} + +// support: jQuery 1.6.1, 1.6.2 (http://bugs.jquery.com/ticket/9413) +if ( $( "" ).data( "a-b", "a" ).removeData( "a-b" ).data( "a-b" ) ) { + $.fn.removeData = (function( removeData ) { + return function( key ) { + if ( arguments.length ) { + return removeData.call( this, $.camelCase( key ) ); + } else { + return removeData.call( this ); + } + }; + })( $.fn.removeData ); +} + + + + + +// deprecated +$.ui.ie = !!/msie [\w.]+/.exec( navigator.userAgent.toLowerCase() ); + +$.support.selectstart = "onselectstart" in document.createElement( "div" ); +$.fn.extend({ + disableSelection: function() { + return this.bind( ( $.support.selectstart ? "selectstart" : "mousedown" ) + + ".ui-disableSelection", function( event ) { + event.preventDefault(); + }); + }, + + enableSelection: function() { + return this.unbind( ".ui-disableSelection" ); + } +}); + +$.extend( $.ui, { + // $.ui.plugin is deprecated. Use $.widget() extensions instead. + plugin: { + add: function( module, option, set ) { + var i, + proto = $.ui[ module ].prototype; + for ( i in set ) { + proto.plugins[ i ] = proto.plugins[ i ] || []; + proto.plugins[ i ].push( [ option, set[ i ] ] ); + } + }, + call: function( instance, name, args ) { + var i, + set = instance.plugins[ name ]; + if ( !set || !instance.element[ 0 ].parentNode || instance.element[ 0 ].parentNode.nodeType === 11 ) { + return; + } + + for ( i = 0; i < set.length; i++ ) { + if ( instance.options[ set[ i ][ 0 ] ] ) { + set[ i ][ 1 ].apply( instance.element, args ); + } + } + } + }, + + // only used by resizable + hasScroll: function( el, a ) { + + //If overflow is hidden, the element might have extra content, but the user wants to hide it + if ( $( el ).css( "overflow" ) === "hidden") { + return false; + } + + var scroll = ( a && a === "left" ) ? "scrollLeft" : "scrollTop", + has = false; + + if ( el[ scroll ] > 0 ) { + return true; + } + + // TODO: determine which cases actually cause this to happen + // if the element doesn't have the scroll set, see if it's possible to + // set the scroll + el[ scroll ] = 1; + has = ( el[ scroll ] > 0 ); + el[ scroll ] = 0; + return has; + } +}); + +})( jQuery ); diff --git a/ckan/public/base/vendor/jquery.ui.mouse.js b/ckan/public/base/vendor/jquery.ui.mouse.js new file mode 100755 index 00000000000..62022cedf02 --- /dev/null +++ b/ckan/public/base/vendor/jquery.ui.mouse.js @@ -0,0 +1,169 @@ +/*! + * jQuery UI Mouse 1.10.3 + * http://jqueryui.com + * + * Copyright 2013 jQuery Foundation and other contributors + * Released under the MIT license. + * http://jquery.org/license + * + * http://api.jqueryui.com/mouse/ + * + * Depends: + * jquery.ui.widget.js + */ +(function( $, undefined ) { + +var mouseHandled = false; +$( document ).mouseup( function() { + mouseHandled = false; +}); + +$.widget("ui.mouse", { + version: "1.10.3", + options: { + cancel: "input,textarea,button,select,option", + distance: 1, + delay: 0 + }, + _mouseInit: function() { + var that = this; + + this.element + .bind("mousedown."+this.widgetName, function(event) { + return that._mouseDown(event); + }) + .bind("click."+this.widgetName, function(event) { + if (true === $.data(event.target, that.widgetName + ".preventClickEvent")) { + $.removeData(event.target, that.widgetName + ".preventClickEvent"); + event.stopImmediatePropagation(); + return false; + } + }); + + this.started = false; + }, + + // TODO: make sure destroying one instance of mouse doesn't mess with + // other instances of mouse + _mouseDestroy: function() { + this.element.unbind("."+this.widgetName); + if ( this._mouseMoveDelegate ) { + $(document) + .unbind("mousemove."+this.widgetName, this._mouseMoveDelegate) + .unbind("mouseup."+this.widgetName, this._mouseUpDelegate); + } + }, + + _mouseDown: function(event) { + // don't let more than one widget handle mouseStart + if( mouseHandled ) { return; } + + // we may have missed mouseup (out of window) + (this._mouseStarted && this._mouseUp(event)); + + this._mouseDownEvent = event; + + var that = this, + btnIsLeft = (event.which === 1), + // event.target.nodeName works around a bug in IE 8 with + // disabled inputs (#7620) + elIsCancel = (typeof this.options.cancel === "string" && event.target.nodeName ? $(event.target).closest(this.options.cancel).length : false); + if (!btnIsLeft || elIsCancel || !this._mouseCapture(event)) { + return true; + } + + this.mouseDelayMet = !this.options.delay; + if (!this.mouseDelayMet) { + this._mouseDelayTimer = setTimeout(function() { + that.mouseDelayMet = true; + }, this.options.delay); + } + + if (this._mouseDistanceMet(event) && this._mouseDelayMet(event)) { + this._mouseStarted = (this._mouseStart(event) !== false); + if (!this._mouseStarted) { + event.preventDefault(); + return true; + } + } + + // Click event may never have fired (Gecko & Opera) + if (true === $.data(event.target, this.widgetName + ".preventClickEvent")) { + $.removeData(event.target, this.widgetName + ".preventClickEvent"); + } + + // these delegates are required to keep context + this._mouseMoveDelegate = function(event) { + return that._mouseMove(event); + }; + this._mouseUpDelegate = function(event) { + return that._mouseUp(event); + }; + $(document) + .bind("mousemove."+this.widgetName, this._mouseMoveDelegate) + .bind("mouseup."+this.widgetName, this._mouseUpDelegate); + + event.preventDefault(); + + mouseHandled = true; + return true; + }, + + _mouseMove: function(event) { + // IE mouseup check - mouseup happened when mouse was out of window + if ($.ui.ie && ( !document.documentMode || document.documentMode < 9 ) && !event.button) { + return this._mouseUp(event); + } + + if (this._mouseStarted) { + this._mouseDrag(event); + return event.preventDefault(); + } + + if (this._mouseDistanceMet(event) && this._mouseDelayMet(event)) { + this._mouseStarted = + (this._mouseStart(this._mouseDownEvent, event) !== false); + (this._mouseStarted ? this._mouseDrag(event) : this._mouseUp(event)); + } + + return !this._mouseStarted; + }, + + _mouseUp: function(event) { + $(document) + .unbind("mousemove."+this.widgetName, this._mouseMoveDelegate) + .unbind("mouseup."+this.widgetName, this._mouseUpDelegate); + + if (this._mouseStarted) { + this._mouseStarted = false; + + if (event.target === this._mouseDownEvent.target) { + $.data(event.target, this.widgetName + ".preventClickEvent", true); + } + + this._mouseStop(event); + } + + return false; + }, + + _mouseDistanceMet: function(event) { + return (Math.max( + Math.abs(this._mouseDownEvent.pageX - event.pageX), + Math.abs(this._mouseDownEvent.pageY - event.pageY) + ) >= this.options.distance + ); + }, + + _mouseDelayMet: function(/* event */) { + return this.mouseDelayMet; + }, + + // These are placeholder methods, to be overriden by extending plugin + _mouseStart: function(/* event */) {}, + _mouseDrag: function(/* event */) {}, + _mouseStop: function(/* event */) {}, + _mouseCapture: function(/* event */) { return true; } +}); + +})(jQuery); diff --git a/ckan/public/base/vendor/jquery.ui.sortable.js b/ckan/public/base/vendor/jquery.ui.sortable.js new file mode 100755 index 00000000000..c9f2c9021cb --- /dev/null +++ b/ckan/public/base/vendor/jquery.ui.sortable.js @@ -0,0 +1,1285 @@ +/*! + * jQuery UI Sortable 1.10.3 + * http://jqueryui.com + * + * Copyright 2013 jQuery Foundation and other contributors + * Released under the MIT license. + * http://jquery.org/license + * + * http://api.jqueryui.com/sortable/ + * + * Depends: + * jquery.ui.core.js + * jquery.ui.mouse.js + * jquery.ui.widget.js + */ +(function( $, undefined ) { + +/*jshint loopfunc: true */ + +function isOverAxis( x, reference, size ) { + return ( x > reference ) && ( x < ( reference + size ) ); +} + +function isFloating(item) { + return (/left|right/).test(item.css("float")) || (/inline|table-cell/).test(item.css("display")); +} + +$.widget("ui.sortable", $.ui.mouse, { + version: "1.10.3", + widgetEventPrefix: "sort", + ready: false, + options: { + appendTo: "parent", + axis: false, + connectWith: false, + containment: false, + cursor: "auto", + cursorAt: false, + dropOnEmpty: true, + forcePlaceholderSize: false, + forceHelperSize: false, + grid: false, + handle: false, + helper: "original", + items: "> *", + opacity: false, + placeholder: false, + revert: false, + scroll: true, + scrollSensitivity: 20, + scrollSpeed: 20, + scope: "default", + tolerance: "intersect", + zIndex: 1000, + + // callbacks + activate: null, + beforeStop: null, + change: null, + deactivate: null, + out: null, + over: null, + receive: null, + remove: null, + sort: null, + start: null, + stop: null, + update: null + }, + _create: function() { + + var o = this.options; + this.containerCache = {}; + this.element.addClass("ui-sortable"); + + //Get the items + this.refresh(); + + //Let's determine if the items are being displayed horizontally + this.floating = this.items.length ? o.axis === "x" || isFloating(this.items[0].item) : false; + + //Let's determine the parent's offset + this.offset = this.element.offset(); + + //Initialize mouse events for interaction + this._mouseInit(); + + //We're ready to go + this.ready = true; + + }, + + _destroy: function() { + this.element + .removeClass("ui-sortable ui-sortable-disabled"); + this._mouseDestroy(); + + for ( var i = this.items.length - 1; i >= 0; i-- ) { + this.items[i].item.removeData(this.widgetName + "-item"); + } + + return this; + }, + + _setOption: function(key, value){ + if ( key === "disabled" ) { + this.options[ key ] = value; + + this.widget().toggleClass( "ui-sortable-disabled", !!value ); + } else { + // Don't call widget base _setOption for disable as it adds ui-state-disabled class + $.Widget.prototype._setOption.apply(this, arguments); + } + }, + + _mouseCapture: function(event, overrideHandle) { + var currentItem = null, + validHandle = false, + that = this; + + if (this.reverting) { + return false; + } + + if(this.options.disabled || this.options.type === "static") { + return false; + } + + //We have to refresh the items data once first + this._refreshItems(event); + + //Find out if the clicked node (or one of its parents) is a actual item in this.items + $(event.target).parents().each(function() { + if($.data(this, that.widgetName + "-item") === that) { + currentItem = $(this); + return false; + } + }); + if($.data(event.target, that.widgetName + "-item") === that) { + currentItem = $(event.target); + } + + if(!currentItem) { + return false; + } + if(this.options.handle && !overrideHandle) { + $(this.options.handle, currentItem).find("*").addBack().each(function() { + if(this === event.target) { + validHandle = true; + } + }); + if(!validHandle) { + return false; + } + } + + this.currentItem = currentItem; + this._removeCurrentsFromItems(); + return true; + + }, + + _mouseStart: function(event, overrideHandle, noActivation) { + + var i, body, + o = this.options; + + this.currentContainer = this; + + //We only need to call refreshPositions, because the refreshItems call has been moved to mouseCapture + this.refreshPositions(); + + //Create and append the visible helper + this.helper = this._createHelper(event); + + //Cache the helper size + this._cacheHelperProportions(); + + /* + * - Position generation - + * This block generates everything position related - it's the core of draggables. + */ + + //Cache the margins of the original element + this._cacheMargins(); + + //Get the next scrolling parent + this.scrollParent = this.helper.scrollParent(); + + //The element's absolute position on the page minus margins + this.offset = this.currentItem.offset(); + this.offset = { + top: this.offset.top - this.margins.top, + left: this.offset.left - this.margins.left + }; + + $.extend(this.offset, { + click: { //Where the click happened, relative to the element + left: event.pageX - this.offset.left, + top: event.pageY - this.offset.top + }, + parent: this._getParentOffset(), + relative: this._getRelativeOffset() //This is a relative to absolute position minus the actual position calculation - only used for relative positioned helper + }); + + // Only after we got the offset, we can change the helper's position to absolute + // TODO: Still need to figure out a way to make relative sorting possible + this.helper.css("position", "absolute"); + this.cssPosition = this.helper.css("position"); + + //Generate the original position + this.originalPosition = this._generatePosition(event); + this.originalPageX = event.pageX; + this.originalPageY = event.pageY; + + //Adjust the mouse offset relative to the helper if "cursorAt" is supplied + (o.cursorAt && this._adjustOffsetFromHelper(o.cursorAt)); + + //Cache the former DOM position + this.domPosition = { prev: this.currentItem.prev()[0], parent: this.currentItem.parent()[0] }; + + //If the helper is not the original, hide the original so it's not playing any role during the drag, won't cause anything bad this way + if(this.helper[0] !== this.currentItem[0]) { + this.currentItem.hide(); + } + + //Create the placeholder + this._createPlaceholder(); + + //Set a containment if given in the options + if(o.containment) { + this._setContainment(); + } + + if( o.cursor && o.cursor !== "auto" ) { // cursor option + body = this.document.find( "body" ); + + // support: IE + this.storedCursor = body.css( "cursor" ); + body.css( "cursor", o.cursor ); + + this.storedStylesheet = $( "" ).appendTo( body ); + } + + if(o.opacity) { // opacity option + if (this.helper.css("opacity")) { + this._storedOpacity = this.helper.css("opacity"); + } + this.helper.css("opacity", o.opacity); + } + + if(o.zIndex) { // zIndex option + if (this.helper.css("zIndex")) { + this._storedZIndex = this.helper.css("zIndex"); + } + this.helper.css("zIndex", o.zIndex); + } + + //Prepare scrolling + if(this.scrollParent[0] !== document && this.scrollParent[0].tagName !== "HTML") { + this.overflowOffset = this.scrollParent.offset(); + } + + //Call callbacks + this._trigger("start", event, this._uiHash()); + + //Recache the helper size + if(!this._preserveHelperProportions) { + this._cacheHelperProportions(); + } + + + //Post "activate" events to possible containers + if( !noActivation ) { + for ( i = this.containers.length - 1; i >= 0; i-- ) { + this.containers[ i ]._trigger( "activate", event, this._uiHash( this ) ); + } + } + + //Prepare possible droppables + if($.ui.ddmanager) { + $.ui.ddmanager.current = this; + } + + if ($.ui.ddmanager && !o.dropBehaviour) { + $.ui.ddmanager.prepareOffsets(this, event); + } + + this.dragging = true; + + this.helper.addClass("ui-sortable-helper"); + this._mouseDrag(event); //Execute the drag once - this causes the helper not to be visible before getting its correct position + return true; + + }, + + _mouseDrag: function(event) { + var i, item, itemElement, intersection, + o = this.options, + scrolled = false; + + //Compute the helpers position + this.position = this._generatePosition(event); + this.positionAbs = this._convertPositionTo("absolute"); + + if (!this.lastPositionAbs) { + this.lastPositionAbs = this.positionAbs; + } + + //Do scrolling + if(this.options.scroll) { + if(this.scrollParent[0] !== document && this.scrollParent[0].tagName !== "HTML") { + + if((this.overflowOffset.top + this.scrollParent[0].offsetHeight) - event.pageY < o.scrollSensitivity) { + this.scrollParent[0].scrollTop = scrolled = this.scrollParent[0].scrollTop + o.scrollSpeed; + } else if(event.pageY - this.overflowOffset.top < o.scrollSensitivity) { + this.scrollParent[0].scrollTop = scrolled = this.scrollParent[0].scrollTop - o.scrollSpeed; + } + + if((this.overflowOffset.left + this.scrollParent[0].offsetWidth) - event.pageX < o.scrollSensitivity) { + this.scrollParent[0].scrollLeft = scrolled = this.scrollParent[0].scrollLeft + o.scrollSpeed; + } else if(event.pageX - this.overflowOffset.left < o.scrollSensitivity) { + this.scrollParent[0].scrollLeft = scrolled = this.scrollParent[0].scrollLeft - o.scrollSpeed; + } + + } else { + + if(event.pageY - $(document).scrollTop() < o.scrollSensitivity) { + scrolled = $(document).scrollTop($(document).scrollTop() - o.scrollSpeed); + } else if($(window).height() - (event.pageY - $(document).scrollTop()) < o.scrollSensitivity) { + scrolled = $(document).scrollTop($(document).scrollTop() + o.scrollSpeed); + } + + if(event.pageX - $(document).scrollLeft() < o.scrollSensitivity) { + scrolled = $(document).scrollLeft($(document).scrollLeft() - o.scrollSpeed); + } else if($(window).width() - (event.pageX - $(document).scrollLeft()) < o.scrollSensitivity) { + scrolled = $(document).scrollLeft($(document).scrollLeft() + o.scrollSpeed); + } + + } + + if(scrolled !== false && $.ui.ddmanager && !o.dropBehaviour) { + $.ui.ddmanager.prepareOffsets(this, event); + } + } + + //Regenerate the absolute position used for position checks + this.positionAbs = this._convertPositionTo("absolute"); + + //Set the helper position + if(!this.options.axis || this.options.axis !== "y") { + this.helper[0].style.left = this.position.left+"px"; + } + if(!this.options.axis || this.options.axis !== "x") { + this.helper[0].style.top = this.position.top+"px"; + } + + //Rearrange + for (i = this.items.length - 1; i >= 0; i--) { + + //Cache variables and intersection, continue if no intersection + item = this.items[i]; + itemElement = item.item[0]; + intersection = this._intersectsWithPointer(item); + if (!intersection) { + continue; + } + + // Only put the placeholder inside the current Container, skip all + // items form other containers. This works because when moving + // an item from one container to another the + // currentContainer is switched before the placeholder is moved. + // + // Without this moving items in "sub-sortables" can cause the placeholder to jitter + // beetween the outer and inner container. + if (item.instance !== this.currentContainer) { + continue; + } + + // cannot intersect with itself + // no useless actions that have been done before + // no action if the item moved is the parent of the item checked + if (itemElement !== this.currentItem[0] && + this.placeholder[intersection === 1 ? "next" : "prev"]()[0] !== itemElement && + !$.contains(this.placeholder[0], itemElement) && + (this.options.type === "semi-dynamic" ? !$.contains(this.element[0], itemElement) : true) + ) { + + this.direction = intersection === 1 ? "down" : "up"; + + if (this.options.tolerance === "pointer" || this._intersectsWithSides(item)) { + this._rearrange(event, item); + } else { + break; + } + + this._trigger("change", event, this._uiHash()); + break; + } + } + + //Post events to containers + this._contactContainers(event); + + //Interconnect with droppables + if($.ui.ddmanager) { + $.ui.ddmanager.drag(this, event); + } + + //Call callbacks + this._trigger("sort", event, this._uiHash()); + + this.lastPositionAbs = this.positionAbs; + return false; + + }, + + _mouseStop: function(event, noPropagation) { + + if(!event) { + return; + } + + //If we are using droppables, inform the manager about the drop + if ($.ui.ddmanager && !this.options.dropBehaviour) { + $.ui.ddmanager.drop(this, event); + } + + if(this.options.revert) { + var that = this, + cur = this.placeholder.offset(), + axis = this.options.axis, + animation = {}; + + if ( !axis || axis === "x" ) { + animation.left = cur.left - this.offset.parent.left - this.margins.left + (this.offsetParent[0] === document.body ? 0 : this.offsetParent[0].scrollLeft); + } + if ( !axis || axis === "y" ) { + animation.top = cur.top - this.offset.parent.top - this.margins.top + (this.offsetParent[0] === document.body ? 0 : this.offsetParent[0].scrollTop); + } + this.reverting = true; + $(this.helper).animate( animation, parseInt(this.options.revert, 10) || 500, function() { + that._clear(event); + }); + } else { + this._clear(event, noPropagation); + } + + return false; + + }, + + cancel: function() { + + if(this.dragging) { + + this._mouseUp({ target: null }); + + if(this.options.helper === "original") { + this.currentItem.css(this._storedCSS).removeClass("ui-sortable-helper"); + } else { + this.currentItem.show(); + } + + //Post deactivating events to containers + for (var i = this.containers.length - 1; i >= 0; i--){ + this.containers[i]._trigger("deactivate", null, this._uiHash(this)); + if(this.containers[i].containerCache.over) { + this.containers[i]._trigger("out", null, this._uiHash(this)); + this.containers[i].containerCache.over = 0; + } + } + + } + + if (this.placeholder) { + //$(this.placeholder[0]).remove(); would have been the jQuery way - unfortunately, it unbinds ALL events from the original node! + if(this.placeholder[0].parentNode) { + this.placeholder[0].parentNode.removeChild(this.placeholder[0]); + } + if(this.options.helper !== "original" && this.helper && this.helper[0].parentNode) { + this.helper.remove(); + } + + $.extend(this, { + helper: null, + dragging: false, + reverting: false, + _noFinalSort: null + }); + + if(this.domPosition.prev) { + $(this.domPosition.prev).after(this.currentItem); + } else { + $(this.domPosition.parent).prepend(this.currentItem); + } + } + + return this; + + }, + + serialize: function(o) { + + var items = this._getItemsAsjQuery(o && o.connected), + str = []; + o = o || {}; + + $(items).each(function() { + var res = ($(o.item || this).attr(o.attribute || "id") || "").match(o.expression || (/(.+)[\-=_](.+)/)); + if (res) { + str.push((o.key || res[1]+"[]")+"="+(o.key && o.expression ? res[1] : res[2])); + } + }); + + if(!str.length && o.key) { + str.push(o.key + "="); + } + + return str.join("&"); + + }, + + toArray: function(o) { + + var items = this._getItemsAsjQuery(o && o.connected), + ret = []; + + o = o || {}; + + items.each(function() { ret.push($(o.item || this).attr(o.attribute || "id") || ""); }); + return ret; + + }, + + /* Be careful with the following core functions */ + _intersectsWith: function(item) { + + var x1 = this.positionAbs.left, + x2 = x1 + this.helperProportions.width, + y1 = this.positionAbs.top, + y2 = y1 + this.helperProportions.height, + l = item.left, + r = l + item.width, + t = item.top, + b = t + item.height, + dyClick = this.offset.click.top, + dxClick = this.offset.click.left, + isOverElementHeight = ( this.options.axis === "x" ) || ( ( y1 + dyClick ) > t && ( y1 + dyClick ) < b ), + isOverElementWidth = ( this.options.axis === "y" ) || ( ( x1 + dxClick ) > l && ( x1 + dxClick ) < r ), + isOverElement = isOverElementHeight && isOverElementWidth; + + if ( this.options.tolerance === "pointer" || + this.options.forcePointerForContainers || + (this.options.tolerance !== "pointer" && this.helperProportions[this.floating ? "width" : "height"] > item[this.floating ? "width" : "height"]) + ) { + return isOverElement; + } else { + + return (l < x1 + (this.helperProportions.width / 2) && // Right Half + x2 - (this.helperProportions.width / 2) < r && // Left Half + t < y1 + (this.helperProportions.height / 2) && // Bottom Half + y2 - (this.helperProportions.height / 2) < b ); // Top Half + + } + }, + + _intersectsWithPointer: function(item) { + + var isOverElementHeight = (this.options.axis === "x") || isOverAxis(this.positionAbs.top + this.offset.click.top, item.top, item.height), + isOverElementWidth = (this.options.axis === "y") || isOverAxis(this.positionAbs.left + this.offset.click.left, item.left, item.width), + isOverElement = isOverElementHeight && isOverElementWidth, + verticalDirection = this._getDragVerticalDirection(), + horizontalDirection = this._getDragHorizontalDirection(); + + if (!isOverElement) { + return false; + } + + return this.floating ? + ( ((horizontalDirection && horizontalDirection === "right") || verticalDirection === "down") ? 2 : 1 ) + : ( verticalDirection && (verticalDirection === "down" ? 2 : 1) ); + + }, + + _intersectsWithSides: function(item) { + + var isOverBottomHalf = isOverAxis(this.positionAbs.top + this.offset.click.top, item.top + (item.height/2), item.height), + isOverRightHalf = isOverAxis(this.positionAbs.left + this.offset.click.left, item.left + (item.width/2), item.width), + verticalDirection = this._getDragVerticalDirection(), + horizontalDirection = this._getDragHorizontalDirection(); + + if (this.floating && horizontalDirection) { + return ((horizontalDirection === "right" && isOverRightHalf) || (horizontalDirection === "left" && !isOverRightHalf)); + } else { + return verticalDirection && ((verticalDirection === "down" && isOverBottomHalf) || (verticalDirection === "up" && !isOverBottomHalf)); + } + + }, + + _getDragVerticalDirection: function() { + var delta = this.positionAbs.top - this.lastPositionAbs.top; + return delta !== 0 && (delta > 0 ? "down" : "up"); + }, + + _getDragHorizontalDirection: function() { + var delta = this.positionAbs.left - this.lastPositionAbs.left; + return delta !== 0 && (delta > 0 ? "right" : "left"); + }, + + refresh: function(event) { + this._refreshItems(event); + this.refreshPositions(); + return this; + }, + + _connectWith: function() { + var options = this.options; + return options.connectWith.constructor === String ? [options.connectWith] : options.connectWith; + }, + + _getItemsAsjQuery: function(connected) { + + var i, j, cur, inst, + items = [], + queries = [], + connectWith = this._connectWith(); + + if(connectWith && connected) { + for (i = connectWith.length - 1; i >= 0; i--){ + cur = $(connectWith[i]); + for ( j = cur.length - 1; j >= 0; j--){ + inst = $.data(cur[j], this.widgetFullName); + if(inst && inst !== this && !inst.options.disabled) { + queries.push([$.isFunction(inst.options.items) ? inst.options.items.call(inst.element) : $(inst.options.items, inst.element).not(".ui-sortable-helper").not(".ui-sortable-placeholder"), inst]); + } + } + } + } + + queries.push([$.isFunction(this.options.items) ? this.options.items.call(this.element, null, { options: this.options, item: this.currentItem }) : $(this.options.items, this.element).not(".ui-sortable-helper").not(".ui-sortable-placeholder"), this]); + + for (i = queries.length - 1; i >= 0; i--){ + queries[i][0].each(function() { + items.push(this); + }); + } + + return $(items); + + }, + + _removeCurrentsFromItems: function() { + + var list = this.currentItem.find(":data(" + this.widgetName + "-item)"); + + this.items = $.grep(this.items, function (item) { + for (var j=0; j < list.length; j++) { + if(list[j] === item.item[0]) { + return false; + } + } + return true; + }); + + }, + + _refreshItems: function(event) { + + this.items = []; + this.containers = [this]; + + var i, j, cur, inst, targetData, _queries, item, queriesLength, + items = this.items, + queries = [[$.isFunction(this.options.items) ? this.options.items.call(this.element[0], event, { item: this.currentItem }) : $(this.options.items, this.element), this]], + connectWith = this._connectWith(); + + if(connectWith && this.ready) { //Shouldn't be run the first time through due to massive slow-down + for (i = connectWith.length - 1; i >= 0; i--){ + cur = $(connectWith[i]); + for (j = cur.length - 1; j >= 0; j--){ + inst = $.data(cur[j], this.widgetFullName); + if(inst && inst !== this && !inst.options.disabled) { + queries.push([$.isFunction(inst.options.items) ? inst.options.items.call(inst.element[0], event, { item: this.currentItem }) : $(inst.options.items, inst.element), inst]); + this.containers.push(inst); + } + } + } + } + + for (i = queries.length - 1; i >= 0; i--) { + targetData = queries[i][1]; + _queries = queries[i][0]; + + for (j=0, queriesLength = _queries.length; j < queriesLength; j++) { + item = $(_queries[j]); + + item.data(this.widgetName + "-item", targetData); // Data for target checking (mouse manager) + + items.push({ + item: item, + instance: targetData, + width: 0, height: 0, + left: 0, top: 0 + }); + } + } + + }, + + refreshPositions: function(fast) { + + //This has to be redone because due to the item being moved out/into the offsetParent, the offsetParent's position will change + if(this.offsetParent && this.helper) { + this.offset.parent = this._getParentOffset(); + } + + var i, item, t, p; + + for (i = this.items.length - 1; i >= 0; i--){ + item = this.items[i]; + + //We ignore calculating positions of all connected containers when we're not over them + if(item.instance !== this.currentContainer && this.currentContainer && item.item[0] !== this.currentItem[0]) { + continue; + } + + t = this.options.toleranceElement ? $(this.options.toleranceElement, item.item) : item.item; + + if (!fast) { + item.width = t.outerWidth(); + item.height = t.outerHeight(); + } + + p = t.offset(); + item.left = p.left; + item.top = p.top; + } + + if(this.options.custom && this.options.custom.refreshContainers) { + this.options.custom.refreshContainers.call(this); + } else { + for (i = this.containers.length - 1; i >= 0; i--){ + p = this.containers[i].element.offset(); + this.containers[i].containerCache.left = p.left; + this.containers[i].containerCache.top = p.top; + this.containers[i].containerCache.width = this.containers[i].element.outerWidth(); + this.containers[i].containerCache.height = this.containers[i].element.outerHeight(); + } + } + + return this; + }, + + _createPlaceholder: function(that) { + that = that || this; + var className, + o = that.options; + + if(!o.placeholder || o.placeholder.constructor === String) { + className = o.placeholder; + o.placeholder = { + element: function() { + + var nodeName = that.currentItem[0].nodeName.toLowerCase(), + element = $( "<" + nodeName + ">", that.document[0] ) + .addClass(className || that.currentItem[0].className+" ui-sortable-placeholder") + .removeClass("ui-sortable-helper"); + + if ( nodeName === "tr" ) { + that.currentItem.children().each(function() { + $( " ", that.document[0] ) + .attr( "colspan", $( this ).attr( "colspan" ) || 1 ) + .appendTo( element ); + }); + } else if ( nodeName === "img" ) { + element.attr( "src", that.currentItem.attr( "src" ) ); + } + + if ( !className ) { + element.css( "visibility", "hidden" ); + } + + return element; + }, + update: function(container, p) { + + // 1. If a className is set as 'placeholder option, we don't force sizes - the class is responsible for that + // 2. The option 'forcePlaceholderSize can be enabled to force it even if a class name is specified + if(className && !o.forcePlaceholderSize) { + return; + } + + //If the element doesn't have a actual height by itself (without styles coming from a stylesheet), it receives the inline height from the dragged item + if(!p.height()) { p.height(that.currentItem.innerHeight() - parseInt(that.currentItem.css("paddingTop")||0, 10) - parseInt(that.currentItem.css("paddingBottom")||0, 10)); } + if(!p.width()) { p.width(that.currentItem.innerWidth() - parseInt(that.currentItem.css("paddingLeft")||0, 10) - parseInt(that.currentItem.css("paddingRight")||0, 10)); } + } + }; + } + + //Create the placeholder + that.placeholder = $(o.placeholder.element.call(that.element, that.currentItem)); + + //Append it after the actual current item + that.currentItem.after(that.placeholder); + + //Update the size of the placeholder (TODO: Logic to fuzzy, see line 316/317) + o.placeholder.update(that, that.placeholder); + + }, + + _contactContainers: function(event) { + var i, j, dist, itemWithLeastDistance, posProperty, sizeProperty, base, cur, nearBottom, floating, + innermostContainer = null, + innermostIndex = null; + + // get innermost container that intersects with item + for (i = this.containers.length - 1; i >= 0; i--) { + + // never consider a container that's located within the item itself + if($.contains(this.currentItem[0], this.containers[i].element[0])) { + continue; + } + + if(this._intersectsWith(this.containers[i].containerCache)) { + + // if we've already found a container and it's more "inner" than this, then continue + if(innermostContainer && $.contains(this.containers[i].element[0], innermostContainer.element[0])) { + continue; + } + + innermostContainer = this.containers[i]; + innermostIndex = i; + + } else { + // container doesn't intersect. trigger "out" event if necessary + if(this.containers[i].containerCache.over) { + this.containers[i]._trigger("out", event, this._uiHash(this)); + this.containers[i].containerCache.over = 0; + } + } + + } + + // if no intersecting containers found, return + if(!innermostContainer) { + return; + } + + // move the item into the container if it's not there already + if(this.containers.length === 1) { + if (!this.containers[innermostIndex].containerCache.over) { + this.containers[innermostIndex]._trigger("over", event, this._uiHash(this)); + this.containers[innermostIndex].containerCache.over = 1; + } + } else { + + //When entering a new container, we will find the item with the least distance and append our item near it + dist = 10000; + itemWithLeastDistance = null; + floating = innermostContainer.floating || isFloating(this.currentItem); + posProperty = floating ? "left" : "top"; + sizeProperty = floating ? "width" : "height"; + base = this.positionAbs[posProperty] + this.offset.click[posProperty]; + for (j = this.items.length - 1; j >= 0; j--) { + if(!$.contains(this.containers[innermostIndex].element[0], this.items[j].item[0])) { + continue; + } + if(this.items[j].item[0] === this.currentItem[0]) { + continue; + } + if (floating && !isOverAxis(this.positionAbs.top + this.offset.click.top, this.items[j].top, this.items[j].height)) { + continue; + } + cur = this.items[j].item.offset()[posProperty]; + nearBottom = false; + if(Math.abs(cur - base) > Math.abs(cur + this.items[j][sizeProperty] - base)){ + nearBottom = true; + cur += this.items[j][sizeProperty]; + } + + if(Math.abs(cur - base) < dist) { + dist = Math.abs(cur - base); itemWithLeastDistance = this.items[j]; + this.direction = nearBottom ? "up": "down"; + } + } + + //Check if dropOnEmpty is enabled + if(!itemWithLeastDistance && !this.options.dropOnEmpty) { + return; + } + + if(this.currentContainer === this.containers[innermostIndex]) { + return; + } + + itemWithLeastDistance ? this._rearrange(event, itemWithLeastDistance, null, true) : this._rearrange(event, null, this.containers[innermostIndex].element, true); + this._trigger("change", event, this._uiHash()); + this.containers[innermostIndex]._trigger("change", event, this._uiHash(this)); + this.currentContainer = this.containers[innermostIndex]; + + //Update the placeholder + this.options.placeholder.update(this.currentContainer, this.placeholder); + + this.containers[innermostIndex]._trigger("over", event, this._uiHash(this)); + this.containers[innermostIndex].containerCache.over = 1; + } + + + }, + + _createHelper: function(event) { + + var o = this.options, + helper = $.isFunction(o.helper) ? $(o.helper.apply(this.element[0], [event, this.currentItem])) : (o.helper === "clone" ? this.currentItem.clone() : this.currentItem); + + //Add the helper to the DOM if that didn't happen already + if(!helper.parents("body").length) { + $(o.appendTo !== "parent" ? o.appendTo : this.currentItem[0].parentNode)[0].appendChild(helper[0]); + } + + if(helper[0] === this.currentItem[0]) { + this._storedCSS = { width: this.currentItem[0].style.width, height: this.currentItem[0].style.height, position: this.currentItem.css("position"), top: this.currentItem.css("top"), left: this.currentItem.css("left") }; + } + + if(!helper[0].style.width || o.forceHelperSize) { + helper.width(this.currentItem.width()); + } + if(!helper[0].style.height || o.forceHelperSize) { + helper.height(this.currentItem.height()); + } + + return helper; + + }, + + _adjustOffsetFromHelper: function(obj) { + if (typeof obj === "string") { + obj = obj.split(" "); + } + if ($.isArray(obj)) { + obj = {left: +obj[0], top: +obj[1] || 0}; + } + if ("left" in obj) { + this.offset.click.left = obj.left + this.margins.left; + } + if ("right" in obj) { + this.offset.click.left = this.helperProportions.width - obj.right + this.margins.left; + } + if ("top" in obj) { + this.offset.click.top = obj.top + this.margins.top; + } + if ("bottom" in obj) { + this.offset.click.top = this.helperProportions.height - obj.bottom + this.margins.top; + } + }, + + _getParentOffset: function() { + + + //Get the offsetParent and cache its position + this.offsetParent = this.helper.offsetParent(); + var po = this.offsetParent.offset(); + + // This is a special case where we need to modify a offset calculated on start, since the following happened: + // 1. The position of the helper is absolute, so it's position is calculated based on the next positioned parent + // 2. The actual offset parent is a child of the scroll parent, and the scroll parent isn't the document, which means that + // the scroll is included in the initial calculation of the offset of the parent, and never recalculated upon drag + if(this.cssPosition === "absolute" && this.scrollParent[0] !== document && $.contains(this.scrollParent[0], this.offsetParent[0])) { + po.left += this.scrollParent.scrollLeft(); + po.top += this.scrollParent.scrollTop(); + } + + // This needs to be actually done for all browsers, since pageX/pageY includes this information + // with an ugly IE fix + if( this.offsetParent[0] === document.body || (this.offsetParent[0].tagName && this.offsetParent[0].tagName.toLowerCase() === "html" && $.ui.ie)) { + po = { top: 0, left: 0 }; + } + + return { + top: po.top + (parseInt(this.offsetParent.css("borderTopWidth"),10) || 0), + left: po.left + (parseInt(this.offsetParent.css("borderLeftWidth"),10) || 0) + }; + + }, + + _getRelativeOffset: function() { + + if(this.cssPosition === "relative") { + var p = this.currentItem.position(); + return { + top: p.top - (parseInt(this.helper.css("top"),10) || 0) + this.scrollParent.scrollTop(), + left: p.left - (parseInt(this.helper.css("left"),10) || 0) + this.scrollParent.scrollLeft() + }; + } else { + return { top: 0, left: 0 }; + } + + }, + + _cacheMargins: function() { + this.margins = { + left: (parseInt(this.currentItem.css("marginLeft"),10) || 0), + top: (parseInt(this.currentItem.css("marginTop"),10) || 0) + }; + }, + + _cacheHelperProportions: function() { + this.helperProportions = { + width: this.helper.outerWidth(), + height: this.helper.outerHeight() + }; + }, + + _setContainment: function() { + + var ce, co, over, + o = this.options; + if(o.containment === "parent") { + o.containment = this.helper[0].parentNode; + } + if(o.containment === "document" || o.containment === "window") { + this.containment = [ + 0 - this.offset.relative.left - this.offset.parent.left, + 0 - this.offset.relative.top - this.offset.parent.top, + $(o.containment === "document" ? document : window).width() - this.helperProportions.width - this.margins.left, + ($(o.containment === "document" ? document : window).height() || document.body.parentNode.scrollHeight) - this.helperProportions.height - this.margins.top + ]; + } + + if(!(/^(document|window|parent)$/).test(o.containment)) { + ce = $(o.containment)[0]; + co = $(o.containment).offset(); + over = ($(ce).css("overflow") !== "hidden"); + + this.containment = [ + co.left + (parseInt($(ce).css("borderLeftWidth"),10) || 0) + (parseInt($(ce).css("paddingLeft"),10) || 0) - this.margins.left, + co.top + (parseInt($(ce).css("borderTopWidth"),10) || 0) + (parseInt($(ce).css("paddingTop"),10) || 0) - this.margins.top, + co.left+(over ? Math.max(ce.scrollWidth,ce.offsetWidth) : ce.offsetWidth) - (parseInt($(ce).css("borderLeftWidth"),10) || 0) - (parseInt($(ce).css("paddingRight"),10) || 0) - this.helperProportions.width - this.margins.left, + co.top+(over ? Math.max(ce.scrollHeight,ce.offsetHeight) : ce.offsetHeight) - (parseInt($(ce).css("borderTopWidth"),10) || 0) - (parseInt($(ce).css("paddingBottom"),10) || 0) - this.helperProportions.height - this.margins.top + ]; + } + + }, + + _convertPositionTo: function(d, pos) { + + if(!pos) { + pos = this.position; + } + var mod = d === "absolute" ? 1 : -1, + scroll = this.cssPosition === "absolute" && !(this.scrollParent[0] !== document && $.contains(this.scrollParent[0], this.offsetParent[0])) ? this.offsetParent : this.scrollParent, + scrollIsRootNode = (/(html|body)/i).test(scroll[0].tagName); + + return { + top: ( + pos.top + // The absolute mouse position + this.offset.relative.top * mod + // Only for relative positioned nodes: Relative offset from element to offset parent + this.offset.parent.top * mod - // The offsetParent's offset without borders (offset + border) + ( ( this.cssPosition === "fixed" ? -this.scrollParent.scrollTop() : ( scrollIsRootNode ? 0 : scroll.scrollTop() ) ) * mod) + ), + left: ( + pos.left + // The absolute mouse position + this.offset.relative.left * mod + // Only for relative positioned nodes: Relative offset from element to offset parent + this.offset.parent.left * mod - // The offsetParent's offset without borders (offset + border) + ( ( this.cssPosition === "fixed" ? -this.scrollParent.scrollLeft() : scrollIsRootNode ? 0 : scroll.scrollLeft() ) * mod) + ) + }; + + }, + + _generatePosition: function(event) { + + var top, left, + o = this.options, + pageX = event.pageX, + pageY = event.pageY, + scroll = this.cssPosition === "absolute" && !(this.scrollParent[0] !== document && $.contains(this.scrollParent[0], this.offsetParent[0])) ? this.offsetParent : this.scrollParent, scrollIsRootNode = (/(html|body)/i).test(scroll[0].tagName); + + // This is another very weird special case that only happens for relative elements: + // 1. If the css position is relative + // 2. and the scroll parent is the document or similar to the offset parent + // we have to refresh the relative offset during the scroll so there are no jumps + if(this.cssPosition === "relative" && !(this.scrollParent[0] !== document && this.scrollParent[0] !== this.offsetParent[0])) { + this.offset.relative = this._getRelativeOffset(); + } + + /* + * - Position constraining - + * Constrain the position to a mix of grid, containment. + */ + + if(this.originalPosition) { //If we are not dragging yet, we won't check for options + + if(this.containment) { + if(event.pageX - this.offset.click.left < this.containment[0]) { + pageX = this.containment[0] + this.offset.click.left; + } + if(event.pageY - this.offset.click.top < this.containment[1]) { + pageY = this.containment[1] + this.offset.click.top; + } + if(event.pageX - this.offset.click.left > this.containment[2]) { + pageX = this.containment[2] + this.offset.click.left; + } + if(event.pageY - this.offset.click.top > this.containment[3]) { + pageY = this.containment[3] + this.offset.click.top; + } + } + + if(o.grid) { + top = this.originalPageY + Math.round((pageY - this.originalPageY) / o.grid[1]) * o.grid[1]; + pageY = this.containment ? ( (top - this.offset.click.top >= this.containment[1] && top - this.offset.click.top <= this.containment[3]) ? top : ((top - this.offset.click.top >= this.containment[1]) ? top - o.grid[1] : top + o.grid[1])) : top; + + left = this.originalPageX + Math.round((pageX - this.originalPageX) / o.grid[0]) * o.grid[0]; + pageX = this.containment ? ( (left - this.offset.click.left >= this.containment[0] && left - this.offset.click.left <= this.containment[2]) ? left : ((left - this.offset.click.left >= this.containment[0]) ? left - o.grid[0] : left + o.grid[0])) : left; + } + + } + + return { + top: ( + pageY - // The absolute mouse position + this.offset.click.top - // Click offset (relative to the element) + this.offset.relative.top - // Only for relative positioned nodes: Relative offset from element to offset parent + this.offset.parent.top + // The offsetParent's offset without borders (offset + border) + ( ( this.cssPosition === "fixed" ? -this.scrollParent.scrollTop() : ( scrollIsRootNode ? 0 : scroll.scrollTop() ) )) + ), + left: ( + pageX - // The absolute mouse position + this.offset.click.left - // Click offset (relative to the element) + this.offset.relative.left - // Only for relative positioned nodes: Relative offset from element to offset parent + this.offset.parent.left + // The offsetParent's offset without borders (offset + border) + ( ( this.cssPosition === "fixed" ? -this.scrollParent.scrollLeft() : scrollIsRootNode ? 0 : scroll.scrollLeft() )) + ) + }; + + }, + + _rearrange: function(event, i, a, hardRefresh) { + + a ? a[0].appendChild(this.placeholder[0]) : i.item[0].parentNode.insertBefore(this.placeholder[0], (this.direction === "down" ? i.item[0] : i.item[0].nextSibling)); + + //Various things done here to improve the performance: + // 1. we create a setTimeout, that calls refreshPositions + // 2. on the instance, we have a counter variable, that get's higher after every append + // 3. on the local scope, we copy the counter variable, and check in the timeout, if it's still the same + // 4. this lets only the last addition to the timeout stack through + this.counter = this.counter ? ++this.counter : 1; + var counter = this.counter; + + this._delay(function() { + if(counter === this.counter) { + this.refreshPositions(!hardRefresh); //Precompute after each DOM insertion, NOT on mousemove + } + }); + + }, + + _clear: function(event, noPropagation) { + + this.reverting = false; + // We delay all events that have to be triggered to after the point where the placeholder has been removed and + // everything else normalized again + var i, + delayedTriggers = []; + + // We first have to update the dom position of the actual currentItem + // Note: don't do it if the current item is already removed (by a user), or it gets reappended (see #4088) + if(!this._noFinalSort && this.currentItem.parent().length) { + this.placeholder.before(this.currentItem); + } + this._noFinalSort = null; + + if(this.helper[0] === this.currentItem[0]) { + for(i in this._storedCSS) { + if(this._storedCSS[i] === "auto" || this._storedCSS[i] === "static") { + this._storedCSS[i] = ""; + } + } + this.currentItem.css(this._storedCSS).removeClass("ui-sortable-helper"); + } else { + this.currentItem.show(); + } + + if(this.fromOutside && !noPropagation) { + delayedTriggers.push(function(event) { this._trigger("receive", event, this._uiHash(this.fromOutside)); }); + } + if((this.fromOutside || this.domPosition.prev !== this.currentItem.prev().not(".ui-sortable-helper")[0] || this.domPosition.parent !== this.currentItem.parent()[0]) && !noPropagation) { + delayedTriggers.push(function(event) { this._trigger("update", event, this._uiHash()); }); //Trigger update callback if the DOM position has changed + } + + // Check if the items Container has Changed and trigger appropriate + // events. + if (this !== this.currentContainer) { + if(!noPropagation) { + delayedTriggers.push(function(event) { this._trigger("remove", event, this._uiHash()); }); + delayedTriggers.push((function(c) { return function(event) { c._trigger("receive", event, this._uiHash(this)); }; }).call(this, this.currentContainer)); + delayedTriggers.push((function(c) { return function(event) { c._trigger("update", event, this._uiHash(this)); }; }).call(this, this.currentContainer)); + } + } + + + //Post events to containers + for (i = this.containers.length - 1; i >= 0; i--){ + if(!noPropagation) { + delayedTriggers.push((function(c) { return function(event) { c._trigger("deactivate", event, this._uiHash(this)); }; }).call(this, this.containers[i])); + } + if(this.containers[i].containerCache.over) { + delayedTriggers.push((function(c) { return function(event) { c._trigger("out", event, this._uiHash(this)); }; }).call(this, this.containers[i])); + this.containers[i].containerCache.over = 0; + } + } + + //Do what was originally in plugins + if ( this.storedCursor ) { + this.document.find( "body" ).css( "cursor", this.storedCursor ); + this.storedStylesheet.remove(); + } + if(this._storedOpacity) { + this.helper.css("opacity", this._storedOpacity); + } + if(this._storedZIndex) { + this.helper.css("zIndex", this._storedZIndex === "auto" ? "" : this._storedZIndex); + } + + this.dragging = false; + if(this.cancelHelperRemoval) { + if(!noPropagation) { + this._trigger("beforeStop", event, this._uiHash()); + for (i=0; i < delayedTriggers.length; i++) { + delayedTriggers[i].call(this, event); + } //Trigger all delayed events + this._trigger("stop", event, this._uiHash()); + } + + this.fromOutside = false; + return false; + } + + if(!noPropagation) { + this._trigger("beforeStop", event, this._uiHash()); + } + + //$(this.placeholder[0]).remove(); would have been the jQuery way - unfortunately, it unbinds ALL events from the original node! + this.placeholder[0].parentNode.removeChild(this.placeholder[0]); + + if(this.helper[0] !== this.currentItem[0]) { + this.helper.remove(); + } + this.helper = null; + + if(!noPropagation) { + for (i=0; i < delayedTriggers.length; i++) { + delayedTriggers[i].call(this, event); + } //Trigger all delayed events + this._trigger("stop", event, this._uiHash()); + } + + this.fromOutside = false; + return true; + + }, + + _trigger: function() { + if ($.Widget.prototype._trigger.apply(this, arguments) === false) { + this.cancel(); + } + }, + + _uiHash: function(_inst) { + var inst = _inst || this; + return { + helper: inst.helper, + placeholder: inst.placeholder || $([]), + position: inst.position, + originalPosition: inst.originalPosition, + offset: inst.positionAbs, + item: inst.currentItem, + sender: _inst ? _inst.element : null + }; + } + +}); + +})(jQuery); diff --git a/ckan/public/base/vendor/jquery.ui.widget.js b/ckan/public/base/vendor/jquery.ui.widget.js old mode 100644 new mode 100755 index 9da8673a58b..916a6ad73c1 --- a/ckan/public/base/vendor/jquery.ui.widget.js +++ b/ckan/public/base/vendor/jquery.ui.widget.js @@ -1,58 +1,35 @@ -/* - * jQuery UI Widget 1.8.18+amd - * https://github.com/blueimp/jQuery-File-Upload +/*! + * jQuery UI Widget 1.10.3 + * http://jqueryui.com * - * Copyright 2011, AUTHORS.txt (http://jqueryui.com/about) - * Dual licensed under the MIT or GPL Version 2 licenses. + * Copyright 2013 jQuery Foundation and other contributors + * Released under the MIT license. * http://jquery.org/license * - * http://docs.jquery.com/UI/Widget + * http://api.jqueryui.com/jQuery.widget/ */ - -(function (factory) { - if (typeof define === "function" && define.amd) { - // Register as an anonymous AMD module: - define(["jquery"], factory); - } else { - // Browser globals: - factory(jQuery); - } -}(function( $, undefined ) { - -// jQuery 1.4+ -if ( $.cleanData ) { - var _cleanData = $.cleanData; - $.cleanData = function( elems ) { - for ( var i = 0, elem; (elem = elems[i]) != null; i++ ) { - try { - $( elem ).triggerHandler( "remove" ); - // http://bugs.jquery.com/ticket/8235 - } catch( e ) {} - } - _cleanData( elems ); - }; -} else { - var _remove = $.fn.remove; - $.fn.remove = function( selector, keepData ) { - return this.each(function() { - if ( !keepData ) { - if ( !selector || $.filter( selector, [ this ] ).length ) { - $( "*", this ).add( [ this ] ).each(function() { - try { - $( this ).triggerHandler( "remove" ); - // http://bugs.jquery.com/ticket/8235 - } catch( e ) {} - }); - } - } - return _remove.call( $(this), selector, keepData ); - }); - }; -} +(function( $, undefined ) { + +var uuid = 0, + slice = Array.prototype.slice, + _cleanData = $.cleanData; +$.cleanData = function( elems ) { + for ( var i = 0, elem; (elem = elems[i]) != null; i++ ) { + try { + $( elem ).triggerHandler( "remove" ); + // http://bugs.jquery.com/ticket/8235 + } catch( e ) {} + } + _cleanData( elems ); +}; $.widget = function( name, base, prototype ) { - var namespace = name.split( "." )[ 0 ], - fullName; + var fullName, existingConstructor, constructor, basePrototype, + // proxiedPrototype allows the provided prototype to remain unmodified + // so that it can be used as a mixin for multiple widgets (#8876) + proxiedPrototype = {}, + namespace = name.split( "." )[ 0 ]; + name = name.split( "." )[ 1 ]; fullName = namespace + "-" + name; @@ -62,81 +39,167 @@ $.widget = function( name, base, prototype ) { } // create selector for plugin - $.expr[ ":" ][ fullName ] = function( elem ) { - return !!$.data( elem, name ); + $.expr[ ":" ][ fullName.toLowerCase() ] = function( elem ) { + return !!$.data( elem, fullName ); }; $[ namespace ] = $[ namespace ] || {}; - $[ namespace ][ name ] = function( options, element ) { + existingConstructor = $[ namespace ][ name ]; + constructor = $[ namespace ][ name ] = function( options, element ) { + // allow instantiation without "new" keyword + if ( !this._createWidget ) { + return new constructor( options, element ); + } + // allow instantiation without initializing for simple inheritance + // must use "new" keyword (the code above always passes args) if ( arguments.length ) { this._createWidget( options, element ); } }; - - var basePrototype = new base(); + // extend with the existing constructor to carry over any static properties + $.extend( constructor, existingConstructor, { + version: prototype.version, + // copy the object used to create the prototype in case we need to + // redefine the widget later + _proto: $.extend( {}, prototype ), + // track widgets that inherit from this widget in case this widget is + // redefined after a widget inherits from it + _childConstructors: [] + }); + + basePrototype = new base(); // we need to make the options hash a property directly on the new instance // otherwise we'll modify the options hash on the prototype that we're // inheriting from -// $.each( basePrototype, function( key, val ) { -// if ( $.isPlainObject(val) ) { -// basePrototype[ key ] = $.extend( {}, val ); -// } -// }); - basePrototype.options = $.extend( true, {}, basePrototype.options ); - $[ namespace ][ name ].prototype = $.extend( true, basePrototype, { + basePrototype.options = $.widget.extend( {}, basePrototype.options ); + $.each( prototype, function( prop, value ) { + if ( !$.isFunction( value ) ) { + proxiedPrototype[ prop ] = value; + return; + } + proxiedPrototype[ prop ] = (function() { + var _super = function() { + return base.prototype[ prop ].apply( this, arguments ); + }, + _superApply = function( args ) { + return base.prototype[ prop ].apply( this, args ); + }; + return function() { + var __super = this._super, + __superApply = this._superApply, + returnValue; + + this._super = _super; + this._superApply = _superApply; + + returnValue = value.apply( this, arguments ); + + this._super = __super; + this._superApply = __superApply; + + return returnValue; + }; + })(); + }); + constructor.prototype = $.widget.extend( basePrototype, { + // TODO: remove support for widgetEventPrefix + // always use the name + a colon as the prefix, e.g., draggable:start + // don't prefix for widgets that aren't DOM-based + widgetEventPrefix: existingConstructor ? basePrototype.widgetEventPrefix : name + }, proxiedPrototype, { + constructor: constructor, namespace: namespace, widgetName: name, - widgetEventPrefix: $[ namespace ][ name ].prototype.widgetEventPrefix || name, - widgetBaseClass: fullName - }, prototype ); + widgetFullName: fullName + }); + + // If this widget is being redefined then we need to find all widgets that + // are inheriting from it and redefine all of them so that they inherit from + // the new version of this widget. We're essentially trying to replace one + // level in the prototype chain. + if ( existingConstructor ) { + $.each( existingConstructor._childConstructors, function( i, child ) { + var childPrototype = child.prototype; + + // redefine the child widget using the same prototype that was + // originally used, but inherit from the new version of the base + $.widget( childPrototype.namespace + "." + childPrototype.widgetName, constructor, child._proto ); + }); + // remove the list of existing child constructors from the old constructor + // so the old child constructors can be garbage collected + delete existingConstructor._childConstructors; + } else { + base._childConstructors.push( constructor ); + } - $.widget.bridge( name, $[ namespace ][ name ] ); + $.widget.bridge( name, constructor ); +}; + +$.widget.extend = function( target ) { + var input = slice.call( arguments, 1 ), + inputIndex = 0, + inputLength = input.length, + key, + value; + for ( ; inputIndex < inputLength; inputIndex++ ) { + for ( key in input[ inputIndex ] ) { + value = input[ inputIndex ][ key ]; + if ( input[ inputIndex ].hasOwnProperty( key ) && value !== undefined ) { + // Clone objects + if ( $.isPlainObject( value ) ) { + target[ key ] = $.isPlainObject( target[ key ] ) ? + $.widget.extend( {}, target[ key ], value ) : + // Don't extend strings, arrays, etc. with objects + $.widget.extend( {}, value ); + // Copy everything else by reference + } else { + target[ key ] = value; + } + } + } + } + return target; }; $.widget.bridge = function( name, object ) { + var fullName = object.prototype.widgetFullName || name; $.fn[ name ] = function( options ) { var isMethodCall = typeof options === "string", - args = Array.prototype.slice.call( arguments, 1 ), + args = slice.call( arguments, 1 ), returnValue = this; // allow multiple hashes to be passed on init options = !isMethodCall && args.length ? - $.extend.apply( null, [ true, options ].concat(args) ) : + $.widget.extend.apply( null, [ options ].concat(args) ) : options; - // prevent calls to internal methods - if ( isMethodCall && options.charAt( 0 ) === "_" ) { - return returnValue; - } - if ( isMethodCall ) { this.each(function() { - var instance = $.data( this, name ), - methodValue = instance && $.isFunction( instance[options] ) ? - instance[ options ].apply( instance, args ) : - instance; - // TODO: add this back in 1.9 and use $.error() (see #5972) -// if ( !instance ) { -// throw "cannot call methods on " + name + " prior to initialization; " + -// "attempted to call method '" + options + "'"; -// } -// if ( !$.isFunction( instance[options] ) ) { -// throw "no such method '" + options + "' for " + name + " widget instance"; -// } -// var methodValue = instance[ options ].apply( instance, args ); + var methodValue, + instance = $.data( this, fullName ); + if ( !instance ) { + return $.error( "cannot call methods on " + name + " prior to initialization; " + + "attempted to call method '" + options + "'" ); + } + if ( !$.isFunction( instance[options] ) || options.charAt( 0 ) === "_" ) { + return $.error( "no such method '" + options + "' for " + name + " widget instance" ); + } + methodValue = instance[ options ].apply( instance, args ); if ( methodValue !== instance && methodValue !== undefined ) { - returnValue = methodValue; + returnValue = methodValue && methodValue.jquery ? + returnValue.pushStack( methodValue.get() ) : + methodValue; return false; } }); } else { this.each(function() { - var instance = $.data( this, name ); + var instance = $.data( this, fullName ); if ( instance ) { instance.option( options || {} )._init(); } else { - $.data( this, name, new object( options, this ) ); + $.data( this, fullName, new object( options, this ) ); } }); } @@ -145,74 +208,123 @@ $.widget.bridge = function( name, object ) { }; }; -$.Widget = function( options, element ) { - // allow instantiation without initializing for simple inheritance - if ( arguments.length ) { - this._createWidget( options, element ); - } -}; +$.Widget = function( /* options, element */ ) {}; +$.Widget._childConstructors = []; $.Widget.prototype = { widgetName: "widget", widgetEventPrefix: "", + defaultElement: "
", options: { - disabled: false + disabled: false, + + // callbacks + create: null }, _createWidget: function( options, element ) { - // $.widget.bridge stores the plugin instance, but we do it anyway - // so that it's stored even before the _create function runs - $.data( element, this.widgetName, this ); + element = $( element || this.defaultElement || this )[ 0 ]; this.element = $( element ); - this.options = $.extend( true, {}, + this.uuid = uuid++; + this.eventNamespace = "." + this.widgetName + this.uuid; + this.options = $.widget.extend( {}, this.options, this._getCreateOptions(), options ); - var self = this; - this.element.bind( "remove." + this.widgetName, function() { - self.destroy(); - }); + this.bindings = $(); + this.hoverable = $(); + this.focusable = $(); + + if ( element !== this ) { + $.data( element, this.widgetFullName, this ); + this._on( true, this.element, { + remove: function( event ) { + if ( event.target === element ) { + this.destroy(); + } + } + }); + this.document = $( element.style ? + // element within the document + element.ownerDocument : + // element is window or document + element.document || element ); + this.window = $( this.document[0].defaultView || this.document[0].parentWindow ); + } this._create(); - this._trigger( "create" ); + this._trigger( "create", null, this._getCreateEventData() ); this._init(); }, - _getCreateOptions: function() { - return $.metadata && $.metadata.get( this.element[0] )[ this.widgetName ]; - }, - _create: function() {}, - _init: function() {}, + _getCreateOptions: $.noop, + _getCreateEventData: $.noop, + _create: $.noop, + _init: $.noop, destroy: function() { + this._destroy(); + // we can probably remove the unbind calls in 2.0 + // all event bindings should go through this._on() this.element - .unbind( "." + this.widgetName ) - .removeData( this.widgetName ); + .unbind( this.eventNamespace ) + // 1.9 BC for #7810 + // TODO remove dual storage + .removeData( this.widgetName ) + .removeData( this.widgetFullName ) + // support: jquery <1.6.3 + // http://bugs.jquery.com/ticket/9413 + .removeData( $.camelCase( this.widgetFullName ) ); this.widget() - .unbind( "." + this.widgetName ) + .unbind( this.eventNamespace ) .removeAttr( "aria-disabled" ) .removeClass( - this.widgetBaseClass + "-disabled " + + this.widgetFullName + "-disabled " + "ui-state-disabled" ); + + // clean up events and states + this.bindings.unbind( this.eventNamespace ); + this.hoverable.removeClass( "ui-state-hover" ); + this.focusable.removeClass( "ui-state-focus" ); }, + _destroy: $.noop, widget: function() { return this.element; }, option: function( key, value ) { - var options = key; + var options = key, + parts, + curOption, + i; if ( arguments.length === 0 ) { // don't return a reference to the internal hash - return $.extend( {}, this.options ); + return $.widget.extend( {}, this.options ); } - if (typeof key === "string" ) { - if ( value === undefined ) { - return this.options[ key ]; - } + if ( typeof key === "string" ) { + // handle nested keys, e.g., "foo.bar" => { foo: { bar: ___ } } options = {}; - options[ key ] = value; + parts = key.split( "." ); + key = parts.shift(); + if ( parts.length ) { + curOption = options[ key ] = $.widget.extend( {}, this.options[ key ] ); + for ( i = 0; i < parts.length - 1; i++ ) { + curOption[ parts[ i ] ] = curOption[ parts[ i ] ] || {}; + curOption = curOption[ parts[ i ] ]; + } + key = parts.pop(); + if ( value === undefined ) { + return curOption[ key ] === undefined ? null : curOption[ key ]; + } + curOption[ key ] = value; + } else { + if ( value === undefined ) { + return this.options[ key ] === undefined ? null : this.options[ key ]; + } + options[ key ] = value; + } } this._setOptions( options ); @@ -220,10 +332,11 @@ $.Widget.prototype = { return this; }, _setOptions: function( options ) { - var self = this; - $.each( options, function( key, value ) { - self._setOption( key, value ); - }); + var key; + + for ( key in options ) { + this._setOption( key, options[ key ] ); + } return this; }, @@ -232,10 +345,10 @@ $.Widget.prototype = { if ( key === "disabled" ) { this.widget() - [ value ? "addClass" : "removeClass"]( - this.widgetBaseClass + "-disabled" + " " + - "ui-state-disabled" ) + .toggleClass( this.widgetFullName + "-disabled ui-state-disabled", !!value ) .attr( "aria-disabled", value ); + this.hoverable.removeClass( "ui-state-hover" ); + this.focusable.removeClass( "ui-state-focus" ); } return this; @@ -248,6 +361,97 @@ $.Widget.prototype = { return this._setOption( "disabled", true ); }, + _on: function( suppressDisabledCheck, element, handlers ) { + var delegateElement, + instance = this; + + // no suppressDisabledCheck flag, shuffle arguments + if ( typeof suppressDisabledCheck !== "boolean" ) { + handlers = element; + element = suppressDisabledCheck; + suppressDisabledCheck = false; + } + + // no element argument, shuffle and use this.element + if ( !handlers ) { + handlers = element; + element = this.element; + delegateElement = this.widget(); + } else { + // accept selectors, DOM elements + element = delegateElement = $( element ); + this.bindings = this.bindings.add( element ); + } + + $.each( handlers, function( event, handler ) { + function handlerProxy() { + // allow widgets to customize the disabled handling + // - disabled as an array instead of boolean + // - disabled class as method for disabling individual parts + if ( !suppressDisabledCheck && + ( instance.options.disabled === true || + $( this ).hasClass( "ui-state-disabled" ) ) ) { + return; + } + return ( typeof handler === "string" ? instance[ handler ] : handler ) + .apply( instance, arguments ); + } + + // copy the guid so direct unbinding works + if ( typeof handler !== "string" ) { + handlerProxy.guid = handler.guid = + handler.guid || handlerProxy.guid || $.guid++; + } + + var match = event.match( /^(\w+)\s*(.*)$/ ), + eventName = match[1] + instance.eventNamespace, + selector = match[2]; + if ( selector ) { + delegateElement.delegate( selector, eventName, handlerProxy ); + } else { + element.bind( eventName, handlerProxy ); + } + }); + }, + + _off: function( element, eventName ) { + eventName = (eventName || "").split( " " ).join( this.eventNamespace + " " ) + this.eventNamespace; + element.unbind( eventName ).undelegate( eventName ); + }, + + _delay: function( handler, delay ) { + function handlerProxy() { + return ( typeof handler === "string" ? instance[ handler ] : handler ) + .apply( instance, arguments ); + } + var instance = this; + return setTimeout( handlerProxy, delay || 0 ); + }, + + _hoverable: function( element ) { + this.hoverable = this.hoverable.add( element ); + this._on( element, { + mouseenter: function( event ) { + $( event.currentTarget ).addClass( "ui-state-hover" ); + }, + mouseleave: function( event ) { + $( event.currentTarget ).removeClass( "ui-state-hover" ); + } + }); + }, + + _focusable: function( element ) { + this.focusable = this.focusable.add( element ); + this._on( element, { + focusin: function( event ) { + $( event.currentTarget ).addClass( "ui-state-focus" ); + }, + focusout: function( event ) { + $( event.currentTarget ).removeClass( "ui-state-focus" ); + } + }); + }, + _trigger: function( type, event, data ) { var prop, orig, callback = this.options[ type ]; @@ -272,11 +476,46 @@ $.Widget.prototype = { } this.element.trigger( event, data ); - - return !( $.isFunction(callback) && - callback.call( this.element[0], event, data ) === false || + return !( $.isFunction( callback ) && + callback.apply( this.element[0], [ event ].concat( data ) ) === false || event.isDefaultPrevented() ); } }; -})); +$.each( { show: "fadeIn", hide: "fadeOut" }, function( method, defaultEffect ) { + $.Widget.prototype[ "_" + method ] = function( element, options, callback ) { + if ( typeof options === "string" ) { + options = { effect: options }; + } + var hasOptions, + effectName = !options ? + method : + options === true || typeof options === "number" ? + defaultEffect : + options.effect || defaultEffect; + options = options || {}; + if ( typeof options === "number" ) { + options = { duration: options }; + } + hasOptions = !$.isEmptyObject( options ); + options.complete = callback; + if ( options.delay ) { + element.delay( options.delay ); + } + if ( hasOptions && $.effects && $.effects.effect[ effectName ] ) { + element[ method ]( options ); + } else if ( effectName !== method && element[ effectName ] ) { + element[ effectName ]( options.duration, options.easing, callback ); + } else { + element.queue(function( next ) { + $( this )[ method ](); + if ( callback ) { + callback.call( element[ 0 ] ); + } + next(); + }); + } + }; +}); + +})( jQuery ); diff --git a/ckan/public/base/vendor/resource.config b/ckan/public/base/vendor/resource.config index 8924b72606a..56bdf2a316a 100644 --- a/ckan/public/base/vendor/resource.config +++ b/ckan/public/base/vendor/resource.config @@ -25,6 +25,7 @@ block_html5_shim = vendor = jquery.js bootstrap = jquery.js fileupload = jquery.js +reorder = jquery.js [groups] @@ -39,8 +40,13 @@ bootstrap = font-awesome/css/font-awesome.css font-awesome/css/font-awesome-ie7.css - fileupload = jquery.ui.widget.js jquery-fileupload/jquery.iframe-transport.js jquery-fileupload/jquery.fileupload.js + +reorder = + jquery.ui.core.js + jquery.ui.widget.js + jquery.ui.mouse.js + jquery.ui.sortable.js From 1b6a04b64408096922a6a8e165f82287787f8854 Mon Sep 17 00:00:00 2001 From: John Martin Date: Thu, 24 Oct 2013 13:32:53 +0100 Subject: [PATCH 34/87] [#1277] First semi working front end for re-ordering resources --- .../javascript/modules/resource-reorder.js | 116 ++++++++++++++++++ ckan/public/base/javascript/resource.config | 1 + ckan/public/base/less/dataset.less | 39 ++++++ ckan/templates/package/resources.html | 11 +- .../package/snippets/resource_item.html | 2 +- 5 files changed, 167 insertions(+), 2 deletions(-) create mode 100644 ckan/public/base/javascript/modules/resource-reorder.js diff --git a/ckan/public/base/javascript/modules/resource-reorder.js b/ckan/public/base/javascript/modules/resource-reorder.js new file mode 100644 index 00000000000..ab28eda40fc --- /dev/null +++ b/ckan/public/base/javascript/modules/resource-reorder.js @@ -0,0 +1,116 @@ +/* Module for the resource form. Handles validation and updating the form + * with external data such as from a file upload. + */ +this.ckan.module('resource-reorder', function($, _) { + return { + options: { + form: { + method: 'POST', + file: 'file', + params: [] + }, + i18n: { + label: _('Reorder resources'), + save: _('Save order'), + cancel: _('Cancel') + } + }, + template: { + title: '

', + button: [ + '', + '', + '', + '' + ].join('\n'), + form_actions: [ + '
', + '', + '', + '
' + ].join('\n'), + handle: [ + '', + '', + '' + ].join('\n') + }, + is_reordering: false, + + initialize: function() { + jQuery.proxyAll(this, /_on/); + + this.html_title = $(this.template.title) + .text(this.i18n('label')) + .insertBefore(this.el) + .hide(); + var button = $(this.template.button) + .on('click', this._onHandleStartReorder) + .appendTo('.page_primary_action'); + $('span', button).text(this.i18n('label')); + + this.html_form_actions = $(this.template.form_actions) + .hide() + .insertAfter(this.el); + $('.save', this.html_form_actions) + .text(this.i18n('save')) + .on('click', this._onHandleSave); + $('.cancel', this.html_form_actions) + .text(this.i18n('cancel')) + .on('click', this._onHandleCancel); + + this.html_handles = $(this.template.handle) + .hide() + .appendTo($('.resource-item', this.el)); + + this.el + .sortable() + .on('change', this._onSortOrderChanged) + .sortable('disable'); + + }, + + _onHandleStartReorder: function() { + if (!this.is_reordering) { + this.html_form_actions + .add(this.html_handles) + .add(this.html_title) + .show(); + this.el + .addClass('reordering') + .sortable('enable'); + $('.page_primary_action').hide(); + this.is_reordering = true; + } + }, + + _onHandleCancel: function() { + if (this.is_reordering) { + this.html_form_actions + .add(this.html_handles) + .add(this.html_title) + .hide(); + this.el + .removeClass('reordering') + .sortable('disable'); + $('.page_primary_action').show(); + this.is_reordering = false; + } + }, + + _onSortOrderChanged: function() { + $('.save', this.html_form_actions).removeClass('disabled') + }, + + _onHandleSave: function() { + if (!$('.save', this.html_form_actions).hasClass('disabled')) { + var order = []; + $('.resource-item', this.el).each(function() { + order.push($(this).data('id')); + }); + console.log(order); + } + } + + }; +}); diff --git a/ckan/public/base/javascript/resource.config b/ckan/public/base/javascript/resource.config index b1a7302d9e8..3eb169835c8 100644 --- a/ckan/public/base/javascript/resource.config +++ b/ckan/public/base/javascript/resource.config @@ -32,6 +32,7 @@ ckan = modules/table-selectable-rows.js modules/resource-form.js modules/resource-upload-field.js + modules/resource-reorder.js modules/follow.js modules/activity-stream.js modules/dashboard.js diff --git a/ckan/public/base/less/dataset.less b/ckan/public/base/less/dataset.less index 29b07d6b8a1..d4c5f6ae1da 100644 --- a/ckan/public/base/less/dataset.less +++ b/ckan/public/base/less/dataset.less @@ -93,6 +93,45 @@ right: 10px; } +.resource-list.reordering { + .resource-item { + border: 1px solid @moduleHeadingBorderColor; + margin-bottom: 10px; + cursor: move; + .handle { + display: block; + position: absolute; + color: @moduleHeadingActionTextColor; + left: -31px; + top: 50%; + margin-top: -15px; + width: 30px; + height: 30px; + line-height: 30px; + text-align: center; + border: 1px solid @moduleHeadingBorderColor; + border-width: 1px 0 1px 1px; + background-color: @moduleBackgroundColor; + .border-radius(20px 0 0 20px); + &:hover { + text-decoration: none; + } + } + &:hover .handle { + background-color: @layoutBackgroundColor; + } + &.ui-sortable-helper { + background-color: @layoutBackgroundColor; + border: 1px solid @layoutLinkColor; + .handle { + background-color: @layoutBackgroundColor; + border-color: @layoutLinkColor; + color: @navLinkColor; + } + } + } +} + // Dataset Forms .dataset-resource-form .dataset-form-resource-types { diff --git a/ckan/templates/package/resources.html b/ckan/templates/package/resources.html index e353d03f10b..1286db9f009 100644 --- a/ckan/templates/package/resources.html +++ b/ckan/templates/package/resources.html @@ -1,5 +1,7 @@ {% extends "package/edit_base.html" %} +{% set has_reorder = c.pkg_dict and c.pkg_dict.resources and c.pkg_dict.resources|length > 0 %} + {% block subtitle %}{{ _('Resources') }} - {{ h.dataset_display_name(pkg) }}{% endblock %} {% block page_primary_action %} @@ -8,7 +10,7 @@ {% block primary_content_inner %} {% if pkg.resources %} -
{% endif %} + + {% if data.id and h.check_access('package_delete', {'id': data.id}) and data.state != 'active' %} +
+ +
+ +
+
+ {% endif %} + {% endblock %} diff --git a/ckan/templates/package/snippets/package_form.html b/ckan/templates/package/snippets/package_form.html index d6f3b285f56..0c3d2872fbb 100644 --- a/ckan/templates/package/snippets/package_form.html +++ b/ckan/templates/package/snippets/package_form.html @@ -33,7 +33,7 @@

{% endblock %} {% block delete_button %} - {% if h.check_access('package_delete', {'id': data.id}) %} + {% if h.check_access('package_delete', {'id': data.id}) and not data.state == 'deleted' %} {% set locale = h.dump_json({'content': _('Are you sure you want to delete this dataset?')}) %} {% block delete_button_text %}{{ _('Delete') }}{% endblock %} {% endif %} From b3f6c1b63c01b32a36c8a2350c5b1d7efd671fc0 Mon Sep 17 00:00:00 2001 From: amercader Date: Thu, 5 Dec 2013 11:45:53 +0000 Subject: [PATCH 63/87] [#1358] Tweak captions to make them clearer for users --- ckan/templates/package/resource_data.html | 2 +- ckan/templates/package/resource_edit_base.html | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/ckan/templates/package/resource_data.html b/ckan/templates/package/resource_data.html index ee01c97ef78..531d9575c44 100644 --- a/ckan/templates/package/resource_data.html +++ b/ckan/templates/package/resource_data.html @@ -9,7 +9,7 @@
diff --git a/ckan/templates/package/resource_edit_base.html b/ckan/templates/package/resource_edit_base.html index 0682cbbf014..b4478caff4e 100644 --- a/ckan/templates/package/resource_edit_base.html +++ b/ckan/templates/package/resource_edit_base.html @@ -21,7 +21,7 @@ {% block content_primary_nav %} {{ h.build_nav_icon('resource_edit', _('Edit resource'), id=pkg.name, resource_id=res.id) }} {% if 'datapusher' in g.plugins %} - {{ h.build_nav_icon('resource_data', _('Resource Data'), id=pkg.name, resource_id=res.id) }} + {{ h.build_nav_icon('resource_data', _('Resource DataStore'), id=pkg.name, resource_id=res.id) }} {% endif %} {% endblock %} From e973268cb7caa58b63cecade1c607c769a9b26e4 Mon Sep 17 00:00:00 2001 From: amercader Date: Thu, 5 Dec 2013 12:07:39 +0000 Subject: [PATCH 64/87] [#1358] Change tab name --- ckan/templates/package/resource_edit_base.html | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/ckan/templates/package/resource_edit_base.html b/ckan/templates/package/resource_edit_base.html index b4478caff4e..4ace42b6dea 100644 --- a/ckan/templates/package/resource_edit_base.html +++ b/ckan/templates/package/resource_edit_base.html @@ -21,7 +21,7 @@ {% block content_primary_nav %} {{ h.build_nav_icon('resource_edit', _('Edit resource'), id=pkg.name, resource_id=res.id) }} {% if 'datapusher' in g.plugins %} - {{ h.build_nav_icon('resource_data', _('Resource DataStore'), id=pkg.name, resource_id=res.id) }} + {{ h.build_nav_icon('resource_data', _('DataStore'), id=pkg.name, resource_id=res.id) }} {% endif %} {% endblock %} From 97ecce24b9a2d170b9899d01a391f7ebb030f587 Mon Sep 17 00:00:00 2001 From: amercader Date: Thu, 5 Dec 2013 13:19:14 +0000 Subject: [PATCH 65/87] [#1358] The joys of PEP8 --- ckanext/datapusher/helpers.py | 1 + ckanext/datapusher/plugin.py | 3 ++- 2 files changed, 3 insertions(+), 1 deletion(-) diff --git a/ckanext/datapusher/helpers.py b/ckanext/datapusher/helpers.py index 11519e2e3ef..535e6d57635 100644 --- a/ckanext/datapusher/helpers.py +++ b/ckanext/datapusher/helpers.py @@ -10,6 +10,7 @@ def datapusher_status(resource_id): 'status': 'unknown' } + def datapusher_status_description(status): _ = toolkit._ diff --git a/ckanext/datapusher/plugin.py b/ckanext/datapusher/plugin.py index 08b9a355f3e..50c9cd864f5 100644 --- a/ckanext/datapusher/plugin.py +++ b/ckanext/datapusher/plugin.py @@ -129,5 +129,6 @@ def get_auth_functions(self): def get_helpers(self): return { 'datapusher_status': helpers.datapusher_status, - 'datapusher_status_description': helpers.datapusher_status_description, + 'datapusher_status_description': + helpers.datapusher_status_description, } From 07420aef7a210ef09b93e4584e283abacc7a08a9 Mon Sep 17 00:00:00 2001 From: amercader Date: Fri, 6 Dec 2013 12:43:08 +0000 Subject: [PATCH 66/87] [#1208] Use search to get datasets on group_show Use the search rather than query the db. Instead of returning the whole package dict, pick the fields to match the ones currently returned by the API. --- ckan/lib/dictization/model_dictize.py | 36 +++++++++++++++++++++------ ckan/logic/action/get.py | 3 +++ ckan/tests/lib/test_dictization.py | 4 --- 3 files changed, 31 insertions(+), 12 deletions(-) diff --git a/ckan/lib/dictization/model_dictize.py b/ckan/lib/dictization/model_dictize.py index faf0df48f7c..d4248cfb794 100644 --- a/ckan/lib/dictization/model_dictize.py +++ b/ckan/lib/dictization/model_dictize.py @@ -350,16 +350,36 @@ def group_dictize(group, context): context['with_capacity'] = True - result_dict['packages'] = d.obj_list_dictize( - _get_members(context, group, 'packages'), - context) - - query = search.PackageSearchQuery() + # Get group packages from the search index, but adapt the output to + # maintain backwards compatibility + q = { + 'facet': 'false', + 'rows': 1000, # Only the first 1000 datasets are returned + } if group.is_organization: - q = {'q': 'owner_org:"%s" +capacity:public' % group.id, 'rows': 1} + q['fq'] = 'owner_org:"{0}"'.format(group.id) else: - q = {'q': 'groups:"%s" +capacity:public' % group.name, 'rows': 1} - result_dict['package_count'] = query.run(q)['count'] + q['fq'] = 'groups:"{0}"'.format(group.name) + + is_group_member = (context.get('user') and + new_authz.has_user_permission_for_group_or_org(group.id, context.get('user'), 'read')) + if not is_group_member: + q['fq'] += ' +capacity:public' + + search_results = logic.get_action('package_search')(context, q) + keys_to_keep = ['id', 'name', 'author', 'url', 'title', + 'notes', 'owner_org', 'private', 'maintainer_email', + 'author_email', 'state', 'version', 'license_id', + 'maintainer', 'revision_id', 'type', 'metadata_modified',] + adapted_results = [] + for pkg in search_results['results']: + new_pkg = {} + for key in keys_to_keep: + new_pkg[key] = pkg.get(key) + adapted_results.append(new_pkg) + + result_dict['packages'] = adapted_results + result_dict['package_count'] = search_results['count'] result_dict['tags'] = tag_list_dictize( _get_members(context, group, 'tags'), diff --git a/ckan/logic/action/get.py b/ckan/logic/action/get.py index 21612765a48..39d7278d89d 100644 --- a/ckan/logic/action/get.py +++ b/ckan/logic/action/get.py @@ -993,6 +993,8 @@ def group_show(context, data_dict): :rtype: dictionary + .. note:: Only its first 1000 datasets are returned + ''' return _group_or_org_show(context, data_dict) @@ -1004,6 +1006,7 @@ def organization_show(context, data_dict): :rtype: dictionary + .. note:: Only its first 1000 datasets are returned ''' return _group_or_org_show(context, data_dict, is_org=True) diff --git a/ckan/tests/lib/test_dictization.py b/ckan/tests/lib/test_dictization.py index a1a94fc2b21..498d70b3ffe 100644 --- a/ckan/tests/lib/test_dictization.py +++ b/ckan/tests/lib/test_dictization.py @@ -958,16 +958,13 @@ def test_16_group_dictized(self): 'name': u'annakarenina3', 'notes': u'Some test notes\n\n### A 3rd level heading\n\n**Some bolded text.**\n\n*Some italicized text.*\n\nForeign characters:\nu with umlaut \xfc\n66-style quote \u201c\nforeign word: th\xfcmb\n\nNeeds escaping:\nleft arrow <\n\n\n\n', 'state': u'active', - 'capacity' : 'in', 'title': u'A Novel By Tolstoy', 'private': False, - 'creator_user_id': None, 'owner_org': None, 'url': u'http://www.annakarenina.com', 'version': u'0.7a'}, {'author': None, 'author_email': None, - 'capacity' : 'public', 'title': u'A Novel By Tolstoy', 'license_id': u'other-open', 'maintainer': None, @@ -978,7 +975,6 @@ def test_16_group_dictized(self): 'state': u'active', 'title': u'A Novel By Tolstoy', 'private': False, - 'creator_user_id': None, 'owner_org': None, 'url': u'http://www.annakarenina.com', 'version': u'0.7a'}], From 53a9ff21b2f3f93dc44b3ec8ef38b94768d84f5d Mon Sep 17 00:00:00 2001 From: Samuele Santi Date: Fri, 6 Dec 2013 16:42:20 +0100 Subject: [PATCH 67/87] Prevent failure in case no record is found (empty database) (fixes #1363) --- ckan/logic/action/get.py | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/ckan/logic/action/get.py b/ckan/logic/action/get.py index 21612765a48..497d8468481 100644 --- a/ckan/logic/action/get.py +++ b/ckan/logic/action/get.py @@ -105,7 +105,9 @@ def package_list(context, data_dict): offset = data_dict.get('offset') if offset: query = query.offset(offset) - return list(zip(*query.execute())[0]) + + ## Returns the first field in each result record + return [r[0] for r in query.execute()] def current_package_list_with_resources(context, data_dict): '''Return a list of the site's datasets (packages) and their resources. From f6e5aa773cda8960788301ec051364479142a192 Mon Sep 17 00:00:00 2001 From: amercader Date: Fri, 6 Dec 2013 17:31:59 +0000 Subject: [PATCH 68/87] [#1208] Do not query datasets on group_show from the controller When dictizing the group as requested from the controller, don't query Solr for datasets as they will be ignored and get requested on the controller later on anyway --- ckan/controllers/group.py | 7 +++ ckan/lib/dictization/model_dictize.py | 63 ++++++++++++++------------- 2 files changed, 40 insertions(+), 30 deletions(-) diff --git a/ckan/controllers/group.py b/ckan/controllers/group.py index 8d4a90d4b7a..060d99b45ca 100644 --- a/ckan/controllers/group.py +++ b/ckan/controllers/group.py @@ -178,6 +178,9 @@ def read(self, id, limit=20): q = c.q = request.params.get('q', '') try: + # Do not query for the group datasets when dictizing, as they will + # be ignored and get requested on the controller anyway + context['include_group_packages'] = False c.group_dict = self._action('group_show')(context, data_dict) c.group = context['group'] except NotFound: @@ -327,6 +330,7 @@ def pager_url(q=None, page=None): items_per_page=limit ) + c.group_dict['package_count'] = query['count'] c.facets = query['facets'] maintain.deprecate_context_item('facets', 'Use `c.search_facets` instead.') @@ -369,6 +373,9 @@ def bulk_process(self, id): data_dict = {'id': id} try: + # Do not query for the group datasets when dictizing, as they will + # be ignored and get requested on the controller anyway + context['include_group_packages'] = False c.group_dict = self._action('group_show')(context, data_dict) c.group = context['group'] except NotFound: diff --git a/ckan/lib/dictization/model_dictize.py b/ckan/lib/dictization/model_dictize.py index d4248cfb794..3ef562a0543 100644 --- a/ckan/lib/dictization/model_dictize.py +++ b/ckan/lib/dictization/model_dictize.py @@ -340,7 +340,6 @@ def _get_members(context, group, member_type): def group_dictize(group, context): - model = context['model'] result_dict = d.table_dictize(group, context) result_dict['display_name'] = group.display_name @@ -350,36 +349,40 @@ def group_dictize(group, context): context['with_capacity'] = True - # Get group packages from the search index, but adapt the output to - # maintain backwards compatibility - q = { - 'facet': 'false', - 'rows': 1000, # Only the first 1000 datasets are returned - } - if group.is_organization: - q['fq'] = 'owner_org:"{0}"'.format(group.id) + if context.get('include_group_packages', True): + # Get group packages from the search index, but adapt the output to + # maintain backwards compatibility + q = { + 'facet': 'false', + 'rows': 1000, # Only the first 1000 datasets are returned + } + if group.is_organization: + q['fq'] = 'owner_org:"{0}"'.format(group.id) + else: + q['fq'] = 'groups:"{0}"'.format(group.name) + + is_group_member = (context.get('user') and + new_authz.has_user_permission_for_group_or_org(group.id, context.get('user'), 'read')) + if not is_group_member: + q['fq'] += ' +capacity:public' + + search_results = logic.get_action('package_search')(context, q) + keys_to_keep = ['id', 'name', 'author', 'url', 'title', + 'notes', 'owner_org', 'private', 'maintainer_email', + 'author_email', 'state', 'version', 'license_id', + 'maintainer', 'revision_id', 'type', 'metadata_modified',] + adapted_results = [] + for pkg in search_results['results']: + new_pkg = {} + for key in keys_to_keep: + new_pkg[key] = pkg.get(key) + adapted_results.append(new_pkg) + + result_dict['packages'] = adapted_results + result_dict['package_count'] = search_results['count'] else: - q['fq'] = 'groups:"{0}"'.format(group.name) - - is_group_member = (context.get('user') and - new_authz.has_user_permission_for_group_or_org(group.id, context.get('user'), 'read')) - if not is_group_member: - q['fq'] += ' +capacity:public' - - search_results = logic.get_action('package_search')(context, q) - keys_to_keep = ['id', 'name', 'author', 'url', 'title', - 'notes', 'owner_org', 'private', 'maintainer_email', - 'author_email', 'state', 'version', 'license_id', - 'maintainer', 'revision_id', 'type', 'metadata_modified',] - adapted_results = [] - for pkg in search_results['results']: - new_pkg = {} - for key in keys_to_keep: - new_pkg[key] = pkg.get(key) - adapted_results.append(new_pkg) - - result_dict['packages'] = adapted_results - result_dict['package_count'] = search_results['count'] + result_dict['packages'] = None + result_dict['package_count'] = None result_dict['tags'] = tag_list_dictize( _get_members(context, group, 'tags'), From 6e8859a156e0b5993db8d28b51e7c6f3c4a0c2a9 Mon Sep 17 00:00:00 2001 From: kindly Date: Mon, 9 Dec 2013 10:59:06 +0000 Subject: [PATCH 69/87] [#1373] Give datastore_search_sql own auth function. Before it was going though normal datastore_search auth function which made no sense as their was no resource to check. It was picking a random resource to check agaist! This patch also fixes a very minor unicode bug. --- ckanext/datastore/db.py | 2 ++ ckanext/datastore/logic/action.py | 2 +- ckanext/datastore/logic/auth.py | 3 +++ ckanext/datastore/plugin.py | 1 + 4 files changed, 7 insertions(+), 1 deletion(-) diff --git a/ckanext/datastore/db.py b/ckanext/datastore/db.py index 4dc41ff974d..414e0935321 100644 --- a/ckanext/datastore/db.py +++ b/ckanext/datastore/db.py @@ -292,6 +292,8 @@ def convert(data, type_name): if type_name.startswith('_'): sub_type = type_name[1:] return [convert(item, sub_type) for item in data] + if type_name == 'tsvector': + return unicode(data, 'utf-8') if isinstance(data, datetime.datetime): return data.isoformat() if isinstance(data, (int, float)): diff --git a/ckanext/datastore/logic/action.py b/ckanext/datastore/logic/action.py index 804fec1a197..1b90193a1bb 100644 --- a/ckanext/datastore/logic/action.py +++ b/ckanext/datastore/logic/action.py @@ -380,7 +380,7 @@ def datastore_search_sql(context, data_dict): '(; equals chr(59)) and string concatenation (||). ')] }) - p.toolkit.check_access('datastore_search', context, data_dict) + p.toolkit.check_access('datastore_search_sql', context, data_dict) data_dict['connection_url'] = pylons.config['ckan.datastore.read_url'] diff --git a/ckanext/datastore/logic/auth.py b/ckanext/datastore/logic/auth.py index f99d7f45ae3..5a6e3329cc0 100644 --- a/ckanext/datastore/logic/auth.py +++ b/ckanext/datastore/logic/auth.py @@ -34,6 +34,9 @@ def datastore_delete(context, data_dict): def datastore_search(context, data_dict): return datastore_auth(context, data_dict, 'resource_show') +@p.toolkit.auth_allow_anonymous_access +def datastore_search_sql(context, data_dict): + return {'success': True} def datastore_change_permissions(context, data_dict): return datastore_auth(context, data_dict) diff --git a/ckanext/datastore/plugin.py b/ckanext/datastore/plugin.py index f91310ecdec..73103e67121 100644 --- a/ckanext/datastore/plugin.py +++ b/ckanext/datastore/plugin.py @@ -221,6 +221,7 @@ def get_auth_functions(self): 'datastore_upsert': auth.datastore_upsert, 'datastore_delete': auth.datastore_delete, 'datastore_search': auth.datastore_search, + 'datastore_search_sql': auth.datastore_search_sql, 'datastore_change_permissions': auth.datastore_change_permissions} def before_map(self, m): From 933209852836e7ed1174cd91395f7020c5361ee0 Mon Sep 17 00:00:00 2001 From: kindly Date: Mon, 9 Dec 2013 13:09:36 +0000 Subject: [PATCH 70/87] [#1373] fix pep-8 issues --- ckanext/datastore/logic/auth.py | 2 ++ 1 file changed, 2 insertions(+) diff --git a/ckanext/datastore/logic/auth.py b/ckanext/datastore/logic/auth.py index 5a6e3329cc0..d002d106fda 100644 --- a/ckanext/datastore/logic/auth.py +++ b/ckanext/datastore/logic/auth.py @@ -34,9 +34,11 @@ def datastore_delete(context, data_dict): def datastore_search(context, data_dict): return datastore_auth(context, data_dict, 'resource_show') + @p.toolkit.auth_allow_anonymous_access def datastore_search_sql(context, data_dict): return {'success': True} + def datastore_change_permissions(context, data_dict): return datastore_auth(context, data_dict) From 22a3432edc3dc330da87c468a539402b234469c3 Mon Sep 17 00:00:00 2001 From: David Read Date: Mon, 9 Dec 2013 16:00:34 +0000 Subject: [PATCH 71/87] [#1038] Fix for python 2.6. --- ckan/tests/models/test_group.py | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/ckan/tests/models/test_group.py b/ckan/tests/models/test_group.py index 68c41864007..c59d6ae8048 100644 --- a/ckan/tests/models/test_group.py +++ b/ckan/tests/models/test_group.py @@ -1,7 +1,6 @@ -from nose.tools import assert_equal, assert_in, assert_not_in +from ckan.tests import assert_equal, assert_in, assert_not_in, CreateTestData import ckan.model as model -from ckan.tests import * class TestGroup(object): From eeb6e65eef7133261b92954dfcf8974d94d2ac24 Mon Sep 17 00:00:00 2001 From: amercader Date: Mon, 9 Dec 2013 16:03:42 +0000 Subject: [PATCH 72/87] [#1208] Return whole dicts on package listing for group_show The reason is that this is consistent with other package listings (tag_show, package_search) and at the same time is backwards compatible as it keeps all the existing fields. Users would be able to choose not to return the packages list. --- ckan/lib/dictization/model_dictize.py | 47 +++---- ckan/tests/lib/test_dictization.py | 169 +++++++++++++++++++++----- 2 files changed, 160 insertions(+), 56 deletions(-) diff --git a/ckan/lib/dictization/model_dictize.py b/ckan/lib/dictization/model_dictize.py index 3ef562a0543..e4ab1d24467 100644 --- a/ckan/lib/dictization/model_dictize.py +++ b/ckan/lib/dictization/model_dictize.py @@ -349,13 +349,17 @@ def group_dictize(group, context): context['with_capacity'] = True - if context.get('include_group_packages', True): - # Get group packages from the search index, but adapt the output to - # maintain backwards compatibility - q = { - 'facet': 'false', - 'rows': 1000, # Only the first 1000 datasets are returned - } + include_packages = context.get('include_group_packages', True) + + q = { + 'facet': 'false', + 'rows': 0, + } + + if include_packages: + + q['rows'] = 1000 # Only the first 1000 datasets are returned + if group.is_organization: q['fq'] = 'owner_org:"{0}"'.format(group.id) else: @@ -363,26 +367,15 @@ def group_dictize(group, context): is_group_member = (context.get('user') and new_authz.has_user_permission_for_group_or_org(group.id, context.get('user'), 'read')) - if not is_group_member: - q['fq'] += ' +capacity:public' - - search_results = logic.get_action('package_search')(context, q) - keys_to_keep = ['id', 'name', 'author', 'url', 'title', - 'notes', 'owner_org', 'private', 'maintainer_email', - 'author_email', 'state', 'version', 'license_id', - 'maintainer', 'revision_id', 'type', 'metadata_modified',] - adapted_results = [] - for pkg in search_results['results']: - new_pkg = {} - for key in keys_to_keep: - new_pkg[key] = pkg.get(key) - adapted_results.append(new_pkg) - - result_dict['packages'] = adapted_results - result_dict['package_count'] = search_results['count'] - else: - result_dict['packages'] = None - result_dict['package_count'] = None + if is_group_member: + context['ignore_capacity_check'] = True + + search_results = logic.get_action('package_search')(context, q) + + if include_packages: + result_dict['packages'] = search_results['results'] + + result_dict['package_count'] = search_results['count'] result_dict['tags'] = tag_list_dictize( _get_members(context, group, 'tags'), diff --git a/ckan/tests/lib/test_dictization.py b/ckan/tests/lib/test_dictization.py index 498d70b3ffe..9e3c4a189ec 100644 --- a/ckan/tests/lib/test_dictization.py +++ b/ckan/tests/lib/test_dictization.py @@ -949,35 +949,146 @@ def test_16_group_dictized(self): 'image_display_url': u'', 'package_count': 2, 'is_organization': False, - 'packages': [{'author': None, - 'author_email': None, - 'license_id': u'other-open', - 'maintainer': None, - 'maintainer_email': None, - 'type': u'dataset', - 'name': u'annakarenina3', - 'notes': u'Some test notes\n\n### A 3rd level heading\n\n**Some bolded text.**\n\n*Some italicized text.*\n\nForeign characters:\nu with umlaut \xfc\n66-style quote \u201c\nforeign word: th\xfcmb\n\nNeeds escaping:\nleft arrow <\n\n\n\n', - 'state': u'active', - 'title': u'A Novel By Tolstoy', - 'private': False, - 'owner_org': None, - 'url': u'http://www.annakarenina.com', - 'version': u'0.7a'}, - {'author': None, - 'author_email': None, - 'title': u'A Novel By Tolstoy', - 'license_id': u'other-open', - 'maintainer': None, - 'maintainer_email': None, - 'type': u'dataset', - 'name': u'annakarenina2', - 'notes': u'Some test notes\n\n### A 3rd level heading\n\n**Some bolded text.**\n\n*Some italicized text.*\n\nForeign characters:\nu with umlaut \xfc\n66-style quote \u201c\nforeign word: th\xfcmb\n\nNeeds escaping:\nleft arrow <\n\n\n\n', - 'state': u'active', - 'title': u'A Novel By Tolstoy', - 'private': False, - 'owner_org': None, - 'url': u'http://www.annakarenina.com', - 'version': u'0.7a'}], + 'packages': [{u'author': None, + u'author_email': None, + u'creator_user_id': None, + u'extras': [], + u'groups': [{u'approval_status': u'approved', + u'capacity': u'public', + u'description': u'', + u'image_url': u'', + u'is_organization': False, + u'name': u'help', + u'state': u'active', + u'title': u'help', + u'type': u'group'}], + u'isopen': True, + u'license_id': u'other-open', + u'license_title': u'Other (Open)', + u'maintainer': None, + u'maintainer_email': None, + u'name': u'annakarenina2', + u'notes': u'Some test notes\n\n### A 3rd level heading\n\n**Some bolded text.**\n\n*Some italicized text.*\n\nForeign characters:\nu with umlaut \xfc\n66-style quote \u201c\nforeign word: th\xfcmb\n\nNeeds escaping:\nleft arrow <\n\n\n\n', + u'num_resources': 0, + u'num_tags': 0, + u'organization': None, + u'owner_org': None, + u'private': False, + u'relationships_as_object': [], + u'relationships_as_subject': [], + u'resources': [], + u'state': u'active', + u'tags': [], + u'title': u'A Novel By Tolstoy', + u'tracking_summary': {u'recent': 0, u'total': 0}, + u'type': u'dataset', + u'url': u'http://www.annakarenina.com', + u'version': u'0.7a'}, + {u'author': None, + u'author_email': None, + u'creator_user_id': None, + u'extras': [{u'key': u'genre', + u'state': u'active', + u'value': u'romantic novel'}, + {u'key': u'original media', + u'state': u'active', + u'value': u'book'}], + u'groups': [{u'approval_status': u'approved', + u'capacity': u'in', + u'description': u'', + u'image_url': u'', + u'is_organization': False, + u'name': u'help', + u'state': u'active', + u'title': u'help', + u'type': u'group'}, + {u'approval_status': u'approved', + u'capacity': u'public', + u'description': u'These are books that David likes.', + u'image_url': u'', + u'is_organization': False, + u'name': u'david', + u'state': u'active', + u'title': u"Dave's books", + u'type': u'group'}, + {u'approval_status': u'approved', + u'capacity': u'public', + u'description': u'Roger likes these books.', + u'image_url': u'', + u'is_organization': False, + u'name': u'roger', + u'state': u'active', + u'title': u"Roger's books", + u'type': u'group'}], + u'isopen': True, + u'license_id': u'other-open', + u'license_title': u'Other (Open)', + u'maintainer': None, + u'maintainer_email': None, + u'name': u'annakarenina3', + u'notes': u'Some test notes\n\n### A 3rd level heading\n\n**Some bolded text.**\n\n*Some italicized text.*\n\nForeign characters:\nu with umlaut \xfc\n66-style quote \u201c\nforeign word: th\xfcmb\n\nNeeds escaping:\nleft arrow <\n\n\n\n', + u'num_resources': 2, + u'num_tags': 3, + u'organization': None, + u'owner_org': None, + u'private': False, + u'relationships_as_object': [], + u'relationships_as_subject': [], + u'resources': [{u'alt_url': u'alt123', + u'cache_last_updated': None, + u'cache_url': None, + u'description': u'Full text. Needs escaping: " Umlaut: \xfc', + u'format': u'plain text', + u'hash': u'abc123', + u'last_modified': None, + u'mimetype': None, + u'mimetype_inner': None, + u'name': None, + u'position': 0, + u'resource_type': None, + u'size': None, + u'size_extra': u'123', + u'state': u'active', + u'tracking_summary': {u'recent': 0, u'total': 0}, + u'url': u'http://www.annakarenina.com/download/x=1&y=2', + u'url_type': None, + u'webstore_last_updated': None, + u'webstore_url': None}, + {u'alt_url': u'alt345', + u'cache_last_updated': None, + u'cache_url': None, + u'description': u'Index of the novel', + u'format': u'JSON', + u'hash': u'def456', + u'last_modified': None, + u'mimetype': None, + u'mimetype_inner': None, + u'name': None, + u'position': 1, + u'resource_type': None, + u'size': None, + u'size_extra': u'345', + u'state': u'active', + u'tracking_summary': {u'recent': 0, u'total': 0}, + u'url': u'http://www.annakarenina.com/index.json', + u'url_type': None, + u'webstore_last_updated': None, + u'webstore_url': None}], + u'state': u'active', + u'tags': [{u'display_name': u'Flexible \u30a1', + u'name': u'Flexible \u30a1', + u'state': u'active'}, + {u'display_name': u'russian', + u'name': u'russian', + u'state': u'active'}, + {u'display_name': u'tolstoy', + u'name': u'tolstoy', + u'state': u'active'}], + u'title': u'A Novel By Tolstoy', + u'tracking_summary': {u'recent': 0, u'total': 0}, + u'type': u'dataset', + u'url': u'http://www.annakarenina.com', + u'version': u'0.7a'}], 'state': u'active', 'approval_status': u'approved', 'title': u'help', From 75394f84c0410e146b9cda434dfad0687689480d Mon Sep 17 00:00:00 2001 From: David Read Date: Mon, 9 Dec 2013 16:25:15 +0000 Subject: [PATCH 73/87] [#1038] Simplify code that reads ckan.auth config. --- ckan/new_authz.py | 10 ++++++---- ckan/tests/test_coding_standards.py | 1 - 2 files changed, 6 insertions(+), 5 deletions(-) diff --git a/ckan/new_authz.py b/ckan/new_authz.py index 92b5c63f64e..e5ac46f8042 100644 --- a/ckan/new_authz.py +++ b/ckan/new_authz.py @@ -379,7 +379,7 @@ def get_user_id_for_username(user_name, allow_none=False): 'user_delete_groups': True, 'user_delete_organizations': True, 'create_user_via_api': False, - 'roles_that_cascade_to_sub_groups': ['admin'], + 'roles_that_cascade_to_sub_groups': 'admin', } CONFIG_PERMISSIONS = {} @@ -393,12 +393,14 @@ def check_config_permission(permission): key = 'ckan.auth.' + perm default = CONFIG_PERMISSIONS_DEFAULTS[perm] CONFIG_PERMISSIONS[perm] = config.get(key, default) - if isinstance(default, bool): - CONFIG_PERMISSIONS[perm] = asbool(CONFIG_PERMISSIONS[perm]) - elif isinstance(default, list) and key in config: + if perm == 'roles_that_cascade_to_sub_groups': + # this permission is a list of strings (space separated) CONFIG_PERMISSIONS[perm] = \ CONFIG_PERMISSIONS[perm].split(' ') \ if CONFIG_PERMISSIONS[perm] else [] + else: + # most permissions are boolean + CONFIG_PERMISSIONS[perm] = asbool(CONFIG_PERMISSIONS[perm]) if permission in CONFIG_PERMISSIONS: return CONFIG_PERMISSIONS[permission] return False diff --git a/ckan/tests/test_coding_standards.py b/ckan/tests/test_coding_standards.py index e47ae715dc1..5f618e45244 100644 --- a/ckan/tests/test_coding_standards.py +++ b/ckan/tests/test_coding_standards.py @@ -472,7 +472,6 @@ class TestImportStar(object): 'ckan/tests/lib/test_tag_search.py', 'ckan/tests/misc/test_sync.py', 'ckan/tests/models/test_extras.py', - 'ckan/tests/models/test_group.py', 'ckan/tests/models/test_misc.py', 'ckan/tests/models/test_package.py', 'ckan/tests/models/test_package_relationships.py', From 804b0f9296ff36117c6b966a0969d026c709e503 Mon Sep 17 00:00:00 2001 From: amercader Date: Mon, 9 Dec 2013 16:28:03 +0000 Subject: [PATCH 74/87] [#1208] Allow users to not request the dataset listing on group_show --- ckan/controllers/group.py | 4 ++-- ckan/lib/dictization/model_dictize.py | 6 +++--- ckan/logic/action/get.py | 9 +++++++++ 3 files changed, 14 insertions(+), 5 deletions(-) diff --git a/ckan/controllers/group.py b/ckan/controllers/group.py index 060d99b45ca..f5880ff7a65 100644 --- a/ckan/controllers/group.py +++ b/ckan/controllers/group.py @@ -180,7 +180,7 @@ def read(self, id, limit=20): try: # Do not query for the group datasets when dictizing, as they will # be ignored and get requested on the controller anyway - context['include_group_packages'] = False + context['include_datasets'] = False c.group_dict = self._action('group_show')(context, data_dict) c.group = context['group'] except NotFound: @@ -375,7 +375,7 @@ def bulk_process(self, id): try: # Do not query for the group datasets when dictizing, as they will # be ignored and get requested on the controller anyway - context['include_group_packages'] = False + context['include_datasets'] = False c.group_dict = self._action('group_show')(context, data_dict) c.group = context['group'] except NotFound: diff --git a/ckan/lib/dictization/model_dictize.py b/ckan/lib/dictization/model_dictize.py index e4ab1d24467..e1603d2e52b 100644 --- a/ckan/lib/dictization/model_dictize.py +++ b/ckan/lib/dictization/model_dictize.py @@ -349,14 +349,14 @@ def group_dictize(group, context): context['with_capacity'] = True - include_packages = context.get('include_group_packages', True) + include_datasets = context.get('include_datasets', True) q = { 'facet': 'false', 'rows': 0, } - if include_packages: + if include_datasets: q['rows'] = 1000 # Only the first 1000 datasets are returned @@ -372,7 +372,7 @@ def group_dictize(group, context): search_results = logic.get_action('package_search')(context, q) - if include_packages: + if include_datasets: result_dict['packages'] = search_results['results'] result_dict['package_count'] = search_results['count'] diff --git a/ckan/logic/action/get.py b/ckan/logic/action/get.py index 39d7278d89d..71132716b68 100644 --- a/ckan/logic/action/get.py +++ b/ckan/logic/action/get.py @@ -944,6 +944,9 @@ def _group_or_org_show(context, data_dict, is_org=False): group = model.Group.get(id) context['group'] = group + context['include_datasets'] = ( + data_dict.get('include_datasets','true').lower() in ('true', '1')) + if group is None: raise NotFound if is_org and not group.is_organization: @@ -990,6 +993,9 @@ def group_show(context, data_dict): :param id: the id or name of the group :type id: string + :param include_datasets: include a list of the group's datasets + (optional, default: ``True``) + :type id: boolean :rtype: dictionary @@ -1003,6 +1009,9 @@ def organization_show(context, data_dict): :param id: the id or name of the organization :type id: string + :param include_datasets: include a list of the organization's datasets + (optional, default: ``True``) + :type id: boolean :rtype: dictionary From efdfc03bc2347acbad672486115bb384f6243ee5 Mon Sep 17 00:00:00 2001 From: amercader Date: Mon, 9 Dec 2013 17:17:04 +0000 Subject: [PATCH 75/87] [#1208] Ignore __junk field in group schema It surfaced on the tests due to the new list fields (resources, tags...) in the group's datasets --- ckan/logic/schema.py | 1 + 1 file changed, 1 insertion(+) diff --git a/ckan/logic/schema.py b/ckan/logic/schema.py index a54c243357a..7aea9853656 100644 --- a/ckan/logic/schema.py +++ b/ckan/logic/schema.py @@ -271,6 +271,7 @@ def default_group_schema(): 'approval_status': [ignore_missing, unicode], 'extras': default_extras_schema(), '__extras': [ignore], + '__junk': [ignore], 'packages': { "id": [not_empty, unicode, package_id_or_name_exists], "title":[ignore_missing, unicode], From 1de51c637aa750872d9d8a72b176a009f84df229 Mon Sep 17 00:00:00 2001 From: David Read Date: Mon, 9 Dec 2013 17:55:27 +0000 Subject: [PATCH 76/87] [#1038] Fix a bug introduced on this branch, for pre-filling the group in the new-package form. --- ckan/templates/organization/read.html | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/ckan/templates/organization/read.html b/ckan/templates/organization/read.html index 88ef246c862..aa3a0388a4d 100644 --- a/ckan/templates/organization/read.html +++ b/ckan/templates/organization/read.html @@ -2,7 +2,7 @@ {% block page_primary_action %} {% if h.check_access('package_create', {'owner_org': c.group_dict.id}) %} - {% link_for _('Add Dataset'), controller='package', action='new', group=c.group_dict.name, class_='btn btn-primary', icon='plus-sign-alt' %} + {% link_for _('Add Dataset'), controller='package', action='new', group=c.group_dict.id, class_='btn btn-primary', icon='plus-sign-alt' %} {% endif %} {% endblock %} From a691bd308199209eced83428fb3f177108f7ee1e Mon Sep 17 00:00:00 2001 From: amercader Date: Tue, 10 Dec 2013 11:16:10 +0000 Subject: [PATCH 77/87] [#1358] Revert docs theme to the version on master --- doc/_themes/sphinx-theme-okfn | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/doc/_themes/sphinx-theme-okfn b/doc/_themes/sphinx-theme-okfn index e2722286d64..4628f26abf4 160000 --- a/doc/_themes/sphinx-theme-okfn +++ b/doc/_themes/sphinx-theme-okfn @@ -1 +1 @@ -Subproject commit e2722286d647df9e79c723aa469198cace34e36d +Subproject commit 4628f26abf401fdb63ec099384bff44a27dcda4c From c68bd378f8b654ad7100d6a213175310f9ee45d1 Mon Sep 17 00:00:00 2001 From: amercader Date: Wed, 11 Dec 2013 16:25:07 +0000 Subject: [PATCH 78/87] [#1208] Fix wrong logic for search params --- ckan/lib/dictization/model_dictize.py | 21 ++++++++++----------- 1 file changed, 10 insertions(+), 11 deletions(-) diff --git a/ckan/lib/dictization/model_dictize.py b/ckan/lib/dictization/model_dictize.py index 3822257471b..1ad4a08ca14 100644 --- a/ckan/lib/dictization/model_dictize.py +++ b/ckan/lib/dictization/model_dictize.py @@ -364,19 +364,18 @@ def group_dictize(group, context): 'rows': 0, } - if include_datasets: + if group.is_organization: + q['fq'] = 'owner_org:"{0}"'.format(group.id) + else: + q['fq'] = 'groups:"{0}"'.format(group.name) - q['rows'] = 1000 # Only the first 1000 datasets are returned + is_group_member = (context.get('user') and + new_authz.has_user_permission_for_group_or_org(group.id, context.get('user'), 'read')) + if is_group_member: + context['ignore_capacity_check'] = True - if group.is_organization: - q['fq'] = 'owner_org:"{0}"'.format(group.id) - else: - q['fq'] = 'groups:"{0}"'.format(group.name) - - is_group_member = (context.get('user') and - new_authz.has_user_permission_for_group_or_org(group.id, context.get('user'), 'read')) - if is_group_member: - context['ignore_capacity_check'] = True + if include_datasets: + q['rows'] = 1000 # Only the first 1000 datasets are returned search_results = logic.get_action('package_search')(context, q) From 2028066b605ce9e64ca97447088d1b5872dc2ab3 Mon Sep 17 00:00:00 2001 From: amercader Date: Wed, 11 Dec 2013 16:25:32 +0000 Subject: [PATCH 79/87] [#1208] More robust boolean param handling --- ckan/logic/action/get.py | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/ckan/logic/action/get.py b/ckan/logic/action/get.py index d1a53dd7dab..b197b58dcc0 100644 --- a/ckan/logic/action/get.py +++ b/ckan/logic/action/get.py @@ -960,8 +960,10 @@ def _group_or_org_show(context, data_dict, is_org=False): group = model.Group.get(id) context['group'] = group - context['include_datasets'] = ( - data_dict.get('include_datasets','true').lower() in ('true', '1')) + include_datasets = data_dict.get('include_datasets', True) + if isinstance(include_datasets, basestring): + include_datasets = (include_datasets.lower() in ('true', '1')) + context['include_datasets'] = include_datasets if group is None: raise NotFound From aab84d1a006da797576cf9182cd53b9084c72304 Mon Sep 17 00:00:00 2001 From: amercader Date: Wed, 11 Dec 2013 17:47:23 +0000 Subject: [PATCH 80/87] [#1208] Add new tests for group/org list/show Added a couple of factories for groups and orgs. Not clear about the user that should be creating them. --- ckan/new_tests/factories.py | 73 ++++++++++++ ckan/new_tests/logic/action/test_get.py | 145 +++++++++++++++++++++++- 2 files changed, 217 insertions(+), 1 deletion(-) diff --git a/ckan/new_tests/factories.py b/ckan/new_tests/factories.py index 6e8385eeacf..79bbdda426f 100644 --- a/ckan/new_tests/factories.py +++ b/ckan/new_tests/factories.py @@ -99,6 +99,79 @@ def _create(cls, target_class, *args, **kwargs): return user_dict +class Group(factory.Factory): + '''A factory class for creating CKAN groups.''' + + # This is the class that GroupFactory will create and return instances + # of. + FACTORY_FOR = ckan.model.Group + + # These are the default params that will be used to create new groups. + type = 'group' + is_organization = False + + title = 'Test Group' + description = 'Just another test group.' + image_url = 'http://placekitten.com/g/200/200' + + # Generate a different group name param for each user that gets created. + name = factory.Sequence(lambda n: 'test_group_{n}'.format(n=n)) + + @classmethod + def _build(cls, target_class, *args, **kwargs): + raise NotImplementedError(".build() isn't supported in CKAN") + + @classmethod + def _create(cls, target_class, *args, **kwargs): + if args: + assert False, "Positional args aren't supported, use keyword args." + + #TODO: we will need to be able to define this when creating the instance + # perhaps passing a 'user' param? + context = { + 'user': helpers.call_action('get_site_user')['name'] + } + + group_dict = helpers.call_action('group_create', context=context, **kwargs) + return group_dict + + +class Organization(factory.Factory): + '''A factory class for creating CKAN organizations.''' + + # This is the class that OrganizationFactory will create and return instances + # of. + FACTORY_FOR = ckan.model.Group + + # These are the default params that will be used to create new organizations. + type = 'organization' + is_organization = True + + title = 'Test Organization' + description = 'Just another test organization.' + image_url = 'http://placekitten.com/g/200/100' + + # Generate a different group name param for each user that gets created. + name = factory.Sequence(lambda n: 'test_org_{n}'.format(n=n)) + + @classmethod + def _build(cls, target_class, *args, **kwargs): + raise NotImplementedError(".build() isn't supported in CKAN") + + @classmethod + def _create(cls, target_class, *args, **kwargs): + if args: + assert False, "Positional args aren't supported, use keyword args." + + #TODO: we will need to be able to define this when creating the instance + # perhaps passing a 'user' param? + context = { + 'user': helpers.call_action('get_site_user')['name'] + } + + group_dict = helpers.call_action('organization_create', context=context, **kwargs) + return group_dict + class MockUser(factory.Factory): '''A factory class for creating mock CKAN users using the mock library.''' diff --git a/ckan/new_tests/logic/action/test_get.py b/ckan/new_tests/logic/action/test_get.py index 2ef68491f9d..f2207d2a6d0 100644 --- a/ckan/new_tests/logic/action/test_get.py +++ b/ckan/new_tests/logic/action/test_get.py @@ -1,10 +1,153 @@ import nose.tools -import nose.case + import ckan.logic as logic +import ckan.lib.search as search import ckan.new_tests.helpers as helpers import ckan.new_tests.factories as factories +class TestGet(object): + + @classmethod + def setup_class(cls): + helpers.reset_db() + + def setup(self): + import ckan.model as model + + # Reset the db before each test method. + model.repo.rebuild_db() + + # Clear the search index + search.clear() + + def test_group_list(self): + + group1 = factories.Group() + group2 = factories.Group() + + group_list = helpers.call_action('group_list') + + assert (sorted(group_list) == + sorted([g['name'] for g in [group1, group2]])) + + def test_group_show(self): + + group = factories.Group() + + group_dict = helpers.call_action('group_show', id=group['id']) + + # FIXME: Should this be returned by group_create? + group_dict.pop('num_followers', None) + assert group_dict == group + + def test_group_show_packages_returned(self): + + user_name = helpers.call_action('get_site_user')['name'] + + group = factories.Group() + + datasets = [ + {'name': 'dataset_1', 'groups': [{'name': group['name']}]}, + {'name': 'dataset_2', 'groups': [{'name': group['name']}]}, + ] + + for dataset in datasets: + helpers.call_action('package_create', + context={'user': user_name}, + **dataset) + + group_dict = helpers.call_action('group_show', id=group['id']) + + assert len(group_dict['packages']) == 2 + assert group_dict['package_count'] == 2 + + def test_group_show_no_packages_returned(self): + + user_name = helpers.call_action('get_site_user')['name'] + + group = factories.Group() + + datasets = [ + {'name': 'dataset_1', 'groups': [{'name': group['name']}]}, + {'name': 'dataset_2', 'groups': [{'name': group['name']}]}, + ] + + for dataset in datasets: + helpers.call_action('package_create', + context={'user': user_name}, + **dataset) + + group_dict = helpers.call_action('group_show', id=group['id'], + include_datasets=False) + + assert not 'packages' in group_dict + assert group_dict['package_count'] == 2 + + def test_organization_list(self): + + org1 = factories.Organization() + org2 = factories.Organization() + + org_list = helpers.call_action('organization_list') + + assert (sorted(org_list) == + sorted([g['name'] for g in [org1, org2]])) + + def test_organization_show(self): + + org = factories.Organization() + + org_dict = helpers.call_action('organization_show', id=org['id']) + + # FIXME: Should this be returned by organization_create? + org_dict.pop('num_followers', None) + assert org_dict == org + + def test_organization_show_packages_returned(self): + + user_name = helpers.call_action('get_site_user')['name'] + + org = factories.Organization() + + datasets = [ + {'name': 'dataset_1', 'owner_org': org['name']}, + {'name': 'dataset_2', 'owner_org': org['name']}, + ] + + for dataset in datasets: + helpers.call_action('package_create', + context={'user': user_name}, + **dataset) + + org_dict = helpers.call_action('organization_show', id=org['id']) + + assert len(org_dict['packages']) == 2 + assert org_dict['package_count'] == 2 + + def test_organization_show_private_packages_not_returned(self): + + user_name = helpers.call_action('get_site_user')['name'] + + org = factories.Organization() + + datasets = [ + {'name': 'dataset_1', 'owner_org': org['name']}, + {'name': 'dataset_2', 'owner_org': org['name'], 'private': True}, + ] + + for dataset in datasets: + helpers.call_action('package_create', + context={'user': user_name}, + **dataset) + + org_dict = helpers.call_action('organization_show', id=org['id']) + + assert len(org_dict['packages']) == 1 + assert org_dict['packages'][0]['name'] == 'dataset_1' + assert org_dict['package_count'] == 1 + + class TestBadLimitQueryParameters(object): '''test class for #1258 non-int query parameters cause 500 errors From 589079351943eec54a011c6865072d36257e7255 Mon Sep 17 00:00:00 2001 From: amercader Date: Thu, 12 Dec 2013 10:22:04 +0000 Subject: [PATCH 81/87] [#1208] Fix tests Pep8 and horrible and unmaintaineble dictization tests --- ckan/new_tests/factories.py | 24 ++++-- ckan/tests/lib/test_dictization.py | 132 +++-------------------------- 2 files changed, 29 insertions(+), 127 deletions(-) diff --git a/ckan/new_tests/factories.py b/ckan/new_tests/factories.py index 79bbdda426f..b049038b6dc 100644 --- a/ckan/new_tests/factories.py +++ b/ckan/new_tests/factories.py @@ -126,24 +126,27 @@ def _create(cls, target_class, *args, **kwargs): if args: assert False, "Positional args aren't supported, use keyword args." - #TODO: we will need to be able to define this when creating the instance - # perhaps passing a 'user' param? + #TODO: we will need to be able to define this when creating the + # instance perhaps passing a 'user' param? context = { 'user': helpers.call_action('get_site_user')['name'] } - group_dict = helpers.call_action('group_create', context=context, **kwargs) + group_dict = helpers.call_action('group_create', + context=context, + **kwargs) return group_dict class Organization(factory.Factory): '''A factory class for creating CKAN organizations.''' - # This is the class that OrganizationFactory will create and return instances - # of. + # This is the class that OrganizationFactory will create and return + # instances of. FACTORY_FOR = ckan.model.Group - # These are the default params that will be used to create new organizations. + # These are the default params that will be used to create new + # organizations. type = 'organization' is_organization = True @@ -163,15 +166,18 @@ def _create(cls, target_class, *args, **kwargs): if args: assert False, "Positional args aren't supported, use keyword args." - #TODO: we will need to be able to define this when creating the instance - # perhaps passing a 'user' param? + #TODO: we will need to be able to define this when creating the + # instance perhaps passing a 'user' param? context = { 'user': helpers.call_action('get_site_user')['name'] } - group_dict = helpers.call_action('organization_create', context=context, **kwargs) + group_dict = helpers.call_action('organization_create', + context=context, + **kwargs) return group_dict + class MockUser(factory.Factory): '''A factory class for creating mock CKAN users using the mock library.''' diff --git a/ckan/tests/lib/test_dictization.py b/ckan/tests/lib/test_dictization.py index 815f7a80ec4..92b75db00db 100644 --- a/ckan/tests/lib/test_dictization.py +++ b/ckan/tests/lib/test_dictization.py @@ -881,8 +881,6 @@ def test_16_group_dictized(self): context = {"model": model, "session": model.Session} - pkg = model.Session.query(model.Package).filter_by(name='annakarenina3').first() - simple_group_dict = {'name': 'simple', 'title': 'simple', 'type': 'organization', @@ -900,7 +898,7 @@ def test_16_group_dictized(self): 'approval_status': 'approved', 'extras': [{'key': 'genre', 'value': u'"horror"'}, {'key': 'media', 'value': u'"dvd"'}], - 'packages':[{'name': 'annakarenina2'}, {'id': pkg.id, 'capacity': 'in'}], + 'packages':[{'name': 'annakarenina2'}], 'users':[{'name': 'annafan'}], 'groups':[{'name': 'simple'}], 'tags':[{'name': 'russian'}] @@ -951,21 +949,23 @@ def test_16_group_dictized(self): 'display_name': u'help', 'image_url': u'', 'image_display_url': u'', - 'package_count': 2, + 'package_count': 1, 'is_organization': False, 'packages': [{u'author': None, u'author_email': None, u'creator_user_id': None, u'extras': [], - u'groups': [{u'approval_status': u'approved', - u'capacity': u'public', - u'description': u'', - u'image_url': u'', - u'is_organization': False, - u'name': u'help', - u'state': u'active', - u'title': u'help', - u'type': u'group'}], + u'groups':[{u'approval_status': u'approved', + u'capacity': u'public', + u'description': u'', + u'display_name': u'help', + u'image_display_url': u'', + u'image_url': u'', + u'is_organization': False, + u'name': u'help', + u'state': u'active', + u'title': u'help', + u'type': u'group'}] , u'isopen': True, u'license_id': u'other-open', u'license_title': u'Other (Open)', @@ -988,111 +988,7 @@ def test_16_group_dictized(self): u'type': u'dataset', u'url': u'http://www.annakarenina.com', u'version': u'0.7a'}, - {u'author': None, - u'author_email': None, - u'creator_user_id': None, - u'extras': [{u'key': u'genre', - u'state': u'active', - u'value': u'romantic novel'}, - {u'key': u'original media', - u'state': u'active', - u'value': u'book'}], - u'groups': [{u'approval_status': u'approved', - u'capacity': u'in', - u'description': u'', - u'image_url': u'', - u'is_organization': False, - u'name': u'help', - u'state': u'active', - u'title': u'help', - u'type': u'group'}, - {u'approval_status': u'approved', - u'capacity': u'public', - u'description': u'These are books that David likes.', - u'image_url': u'', - u'is_organization': False, - u'name': u'david', - u'state': u'active', - u'title': u"Dave's books", - u'type': u'group'}, - {u'approval_status': u'approved', - u'capacity': u'public', - u'description': u'Roger likes these books.', - u'image_url': u'', - u'is_organization': False, - u'name': u'roger', - u'state': u'active', - u'title': u"Roger's books", - u'type': u'group'}], - u'isopen': True, - u'license_id': u'other-open', - u'license_title': u'Other (Open)', - u'maintainer': None, - u'maintainer_email': None, - u'name': u'annakarenina3', - u'notes': u'Some test notes\n\n### A 3rd level heading\n\n**Some bolded text.**\n\n*Some italicized text.*\n\nForeign characters:\nu with umlaut \xfc\n66-style quote \u201c\nforeign word: th\xfcmb\n\nNeeds escaping:\nleft arrow <\n\n\n\n', - u'num_resources': 2, - u'num_tags': 3, - u'organization': None, - u'owner_org': None, - u'private': False, - u'relationships_as_object': [], - u'relationships_as_subject': [], - u'resources': [{u'alt_url': u'alt123', - u'cache_last_updated': None, - u'cache_url': None, - u'description': u'Full text. Needs escaping: " Umlaut: \xfc', - u'format': u'plain text', - u'hash': u'abc123', - u'last_modified': None, - u'mimetype': None, - u'mimetype_inner': None, - u'name': None, - u'position': 0, - u'resource_type': None, - u'size': None, - u'size_extra': u'123', - u'state': u'active', - u'tracking_summary': {u'recent': 0, u'total': 0}, - u'url': u'http://www.annakarenina.com/download/x=1&y=2', - u'url_type': None, - u'webstore_last_updated': None, - u'webstore_url': None}, - {u'alt_url': u'alt345', - u'cache_last_updated': None, - u'cache_url': None, - u'description': u'Index of the novel', - u'format': u'JSON', - u'hash': u'def456', - u'last_modified': None, - u'mimetype': None, - u'mimetype_inner': None, - u'name': None, - u'position': 1, - u'resource_type': None, - u'size': None, - u'size_extra': u'345', - u'state': u'active', - u'tracking_summary': {u'recent': 0, u'total': 0}, - u'url': u'http://www.annakarenina.com/index.json', - u'url_type': None, - u'webstore_last_updated': None, - u'webstore_url': None}], - u'state': u'active', - u'tags': [{u'display_name': u'Flexible \u30a1', - u'name': u'Flexible \u30a1', - u'state': u'active'}, - {u'display_name': u'russian', - u'name': u'russian', - u'state': u'active'}, - {u'display_name': u'tolstoy', - u'name': u'tolstoy', - u'state': u'active'}], - u'title': u'A Novel By Tolstoy', - u'tracking_summary': {u'recent': 0, u'total': 0}, - u'type': u'dataset', - u'url': u'http://www.annakarenina.com', - u'version': u'0.7a'}], + ], 'state': u'active', 'approval_status': u'approved', 'title': u'help', From cf3a97e3f04696b74773d651a83d2888fb12d755 Mon Sep 17 00:00:00 2001 From: amercader Date: Thu, 12 Dec 2013 11:02:44 +0000 Subject: [PATCH 82/87] [#1350] Remove misleading resource format notice --- ckan/templates/package/snippets/resource_form.html | 4 ---- 1 file changed, 4 deletions(-) diff --git a/ckan/templates/package/snippets/resource_form.html b/ckan/templates/package/snippets/resource_form.html index 5d327c3c0f4..42121b422fb 100644 --- a/ckan/templates/package/snippets/resource_form.html +++ b/ckan/templates/package/snippets/resource_form.html @@ -35,10 +35,6 @@ {% block basic_fields_format %} {% set format_attrs = {'data-module': 'autocomplete', 'data-module-source': '/api/2/util/resource/format_autocomplete?incomplete=?'} %} {% call form.input('format', id='field-format', label=_('Format'), placeholder=_('eg. CSV, XML or JSON'), value=data.format, error=errors.format, classes=['control-medium'], attrs=format_attrs) %} - - - {{ _('This is generated automatically. You can edit if you wish') }} - {% endcall %} {% endblock %} From 589bb75084a6c85de4fbb04f2f99bcbd51cf966a Mon Sep 17 00:00:00 2001 From: John Martin Date: Thu, 12 Dec 2013 14:34:52 +0000 Subject: [PATCH 83/87] [#1382] Changes labels on resource upload form --- ckan/public/base/javascript/modules/image-upload.js | 6 +++--- ckan/templates/package/snippets/resource_form.html | 2 +- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/ckan/public/base/javascript/modules/image-upload.js b/ckan/public/base/javascript/modules/image-upload.js index 1872a051fbb..6f3f9d4f5d8 100644 --- a/ckan/public/base/javascript/modules/image-upload.js +++ b/ckan/public/base/javascript/modules/image-upload.js @@ -12,10 +12,10 @@ this.ckan.module('image-upload', function($, _) { field_clear: 'clear_upload', upload_label: '', i18n: { - upload: _('From computer'), - url: _('From web'), + upload: _('Upload'), + url: _('Link'), remove: _('Remove'), - upload_label: _('Upload image'), + upload_label: _('Upload'), remove_tooltip: _('Reset this') }, template: [ diff --git a/ckan/templates/package/snippets/resource_form.html b/ckan/templates/package/snippets/resource_form.html index 42121b422fb..f570aad1b8e 100644 --- a/ckan/templates/package/snippets/resource_form.html +++ b/ckan/templates/package/snippets/resource_form.html @@ -21,7 +21,7 @@ {% set is_upload = (data.url_type == 'upload') %} {{ form.image_upload(data, errors, field_url='url', field_upload='upload', field_clear='clear_upload', is_upload_enabled=h.uploads_enabled(), is_url=data.url and not is_upload, is_upload=is_upload, - upload_label=_('File Upload'), url_label=_('URL')) }} + upload_label=_('File'), url_label=_('URL')) }} {% endblock %} {% block basic_fields_name %} From 6e9f114e17565a71a35bacec7136291516bcc477 Mon Sep 17 00:00:00 2001 From: John Martin Date: Thu, 12 Dec 2013 14:57:47 +0000 Subject: [PATCH 84/87] [#1382] Adds tooltips to button --- ckan/public/base/javascript/modules/image-upload.js | 6 +++++- ckan/templates/macros/form.html | 2 +- 2 files changed, 6 insertions(+), 2 deletions(-) diff --git a/ckan/public/base/javascript/modules/image-upload.js b/ckan/public/base/javascript/modules/image-upload.js index 6f3f9d4f5d8..80eee193027 100644 --- a/ckan/public/base/javascript/modules/image-upload.js +++ b/ckan/public/base/javascript/modules/image-upload.js @@ -15,7 +15,9 @@ this.ckan.module('image-upload', function($, _) { upload: _('Upload'), url: _('Link'), remove: _('Remove'), - upload_label: _('Upload'), + upload_label: _('Image'), + upload_tooltip: _('Upload a file on your computer'), + url_tooltip: _('Link to a URL on the internet (you can also link to a API)'), remove_tooltip: _('Reset this') }, template: [ @@ -59,6 +61,7 @@ this.ckan.module('image-upload', function($, _) { // Button to set the field to be a URL this.button_url = $(' '+this.i18n('url')+'') + .prop('title', this.i18n('url_tooltip')) .on('click', this._onFromWeb) .insertAfter(this.input); @@ -86,6 +89,7 @@ this.ckan.module('image-upload', function($, _) { .on('mouseover', this._onInputMouseOver) .on('mouseout', this._onInputMouseOut) .on('change', this._onInputChange) + .prop('title', this.i18n('upload_tooltip')) .css('width', this.button_upload.outerWidth()); // Fields storage. Used in this.changeState diff --git a/ckan/templates/macros/form.html b/ckan/templates/macros/form.html index 0c0062c873b..522524350a7 100644 --- a/ckan/templates/macros/form.html +++ b/ckan/templates/macros/form.html @@ -408,7 +408,7 @@ url_label='', upload_label='') %} {% set placeholder = placeholder if placeholder else _('http://example.com/my-image.jpg') %} {% set url_label = url_label or _('Image URL') %} - {% set upload_label = upload_label or _('Upload Image') %} + {% set upload_label = upload_label or _('Image') %} {% if is_upload_enabled %}
Date: Thu, 12 Dec 2013 14:58:02 +0000 Subject: [PATCH 85/87] [#1382] Removes uneeded requiredmessage call --- ckan/templates/package/snippets/resource_form.html | 2 -- 1 file changed, 2 deletions(-) diff --git a/ckan/templates/package/snippets/resource_form.html b/ckan/templates/package/snippets/resource_form.html index f570aad1b8e..3c5cb06da28 100644 --- a/ckan/templates/package/snippets/resource_form.html +++ b/ckan/templates/package/snippets/resource_form.html @@ -38,8 +38,6 @@ {% endcall %} {% endblock %} - {{ form.required_message() }} - {% block metadata_fields %} {% if include_metadata %} From a21d109ba0340fed93433ffd1e138afe8f430a7f Mon Sep 17 00:00:00 2001 From: amercader Date: Thu, 12 Dec 2013 15:46:41 +0000 Subject: [PATCH 86/87] [#1382] Minor typo --- ckan/public/base/javascript/modules/image-upload.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/ckan/public/base/javascript/modules/image-upload.js b/ckan/public/base/javascript/modules/image-upload.js index 80eee193027..47f38fb240e 100644 --- a/ckan/public/base/javascript/modules/image-upload.js +++ b/ckan/public/base/javascript/modules/image-upload.js @@ -17,7 +17,7 @@ this.ckan.module('image-upload', function($, _) { remove: _('Remove'), upload_label: _('Image'), upload_tooltip: _('Upload a file on your computer'), - url_tooltip: _('Link to a URL on the internet (you can also link to a API)'), + url_tooltip: _('Link to a URL on the internet (you can also link to an API)'), remove_tooltip: _('Reset this') }, template: [ From 52d1c1d8086e4c3888ddc936cb109368d77c09e3 Mon Sep 17 00:00:00 2001 From: kindly Date: Thu, 12 Dec 2013 18:18:55 +0000 Subject: [PATCH 87/87] [#1255] fix dictization tests --- ckan/tests/lib/test_dictization.py | 18 +++++++----------- 1 file changed, 7 insertions(+), 11 deletions(-) diff --git a/ckan/tests/lib/test_dictization.py b/ckan/tests/lib/test_dictization.py index 92b75db00db..954e3d5d4aa 100644 --- a/ckan/tests/lib/test_dictization.py +++ b/ckan/tests/lib/test_dictization.py @@ -955,17 +955,13 @@ def test_16_group_dictized(self): u'author_email': None, u'creator_user_id': None, u'extras': [], - u'groups':[{u'approval_status': u'approved', - u'capacity': u'public', - u'description': u'', - u'display_name': u'help', - u'image_display_url': u'', - u'image_url': u'', - u'is_organization': False, - u'name': u'help', - u'state': u'active', - u'title': u'help', - u'type': u'group'}] , + u'groups':[ + {u'title': u'help', + u'display_name': u'help', + u'description': u'', + u'name': u'help', + u'image_display_url': u''} + ], u'isopen': True, u'license_id': u'other-open', u'license_title': u'Other (Open)',