diff --git a/CHANGELOG.rst b/CHANGELOG.rst index c97cfe438de..b2ab0e077eb 100644 --- a/CHANGELOG.rst +++ b/CHANGELOG.rst @@ -7,6 +7,21 @@ Changelog --------- +v2.4.8 2017-06-29 +================= + +* Fix activity test to use utcnow (#3644) +* Changed required permission from 'update' to 'manage_group' (#3631) +* Catch invalid sort param exception (#3630) +* Choose direction of recreated package relationship depending on its type (#3626) +* Fix render_datetime for dates before year 1900 (#3611) +* Fix KeyError in 'package_create' (#3027) +* Allow slug preview to work with autocomplete fields (#2501) +* Fix filter results button not working for organization/group (#3620) +* Allow underscores in URL slug preview on create dataset (#3612) +* Create new resource view if resource format changed (#3515) +* Fixed incorrect escaping in `mail_to` + v2.4.7 2017-03-22 ================= diff --git a/ckan/controllers/group.py b/ckan/controllers/group.py index c2c31cd0f9d..a3096e06156 100644 --- a/ckan/controllers/group.py +++ b/ckan/controllers/group.py @@ -170,14 +170,24 @@ def index(self): context['user_id'] = c.userobj.id context['user_is_admin'] = c.userobj.sysadmin - data_dict_global_results = { - 'all_fields': False, - 'q': q, - 'sort': sort_by, - 'type': group_type or 'group', - } - global_results = self._action('group_list')(context, - data_dict_global_results) + try: + data_dict_global_results = { + 'all_fields': False, + 'q': q, + 'sort': sort_by, + 'type': group_type or 'group', + } + global_results = self._action('group_list')( + context, data_dict_global_results) + except ValidationError as e: + if e.error_dict and e.error_dict.get('message'): + msg = e.error_dict['message'] + else: + msg = str(e) + h.flash_error(msg) + c.page = h.Page([], 0) + return render(self._index_template(group_type), + extra_vars={'group_type': group_type}) data_dict_page_results = { 'all_fields': True, diff --git a/ckan/lib/helpers.py b/ckan/lib/helpers.py index ca55ae3a476..30a0aa022bb 100644 --- a/ckan/lib/helpers.py +++ b/ckan/lib/helpers.py @@ -17,8 +17,7 @@ from urllib import urlencode from paste.deploy.converters import asbool -from webhelpers.html import escape, HTML, literal, url_escape -from webhelpers.html.tools import mail_to +from webhelpers.html import HTML, literal, url_escape from webhelpers.html.tags import * from lib.markdown import markdown from webhelpers import paginate @@ -45,6 +44,7 @@ from ckan.common import ( _, ungettext, g, c, request, session, json, OrderedDict ) +from markupsafe import Markup, escape get_available_locales = i18n.get_available_locales get_locales_dict = i18n.get_locales_dict @@ -999,6 +999,24 @@ def render_datetime(datetime_, date_format=None, with_hours=False): return '' # if date_format was supplied we use it if date_format: + + # See http://bugs.python.org/issue1777412 + if datetime_.year < 1900: + year = str(datetime_.year) + + date_format = re.sub('(?{year}'.format(year=year[-2:]), + date_format) + date_format = re.sub('(?{year}'.format(year=year), + date_format) + + datetime_ = datetime.datetime(2016, datetime_.month, datetime_.day, + datetime_.hour, datetime_.minute, + datetime_.second) + + return datetime_.strftime(date_format) + return datetime_.strftime(date_format) # the localised date return formatters.localised_nice_date(datetime_, show_date=True, @@ -2081,6 +2099,13 @@ def license_options(existing_license_id=None): for license_id in license_ids] +def mail_to(email_address, name): + email = escape(email_address) + author = escape(name) + html = Markup(u'{1}'.format(email, author)) + return html + + # these are the functions that will end up in `h` template helpers __allowed_functions__ = [ # functions defined in ckan.lib.helpers diff --git a/ckan/logic/action/create.py b/ckan/logic/action/create.py index 3e1cb635003..5f8714402c7 100644 --- a/ckan/logic/action/create.py +++ b/ckan/logic/action/create.py @@ -306,8 +306,10 @@ def resource_create(context, data_dict): _get_action('package_update')(context, pkg_dict) context.pop('defer_commit') except ValidationError, e: - errors = e.error_dict['resources'][-1] - raise ValidationError(errors) + try: + raise ValidationError(e.error_dict['resources'][-1]) + except (KeyError, IndexError): + raise ValidationError(e.error_dict) ## Get out resource_id resource from model as it will not appear in ## package_show until after commit diff --git a/ckan/logic/action/update.py b/ckan/logic/action/update.py index 3e2929d0adf..5dd76f084d5 100644 --- a/ckan/logic/action/update.py +++ b/ckan/logic/action/update.py @@ -159,8 +159,10 @@ def resource_update(context, data_dict): updated_pkg_dict = _get_action('package_update')(context, pkg_dict) context.pop('defer_commit') except ValidationError, e: - errors = e.error_dict['resources'][n] - raise ValidationError(errors) + try: + raise ValidationError(e.error_dict['resources'][-1]) + except (KeyError, IndexError): + raise ValidationError(e.error_dict) upload.upload(id, uploader.get_max_resource_size()) model.repo.commit() diff --git a/ckan/logic/auth/create.py b/ckan/logic/auth/create.py index 581e192bc0b..4b8167e9baf 100644 --- a/ckan/logic/auth/create.py +++ b/ckan/logic/auth/create.py @@ -211,7 +211,7 @@ def _check_group_auth(context, data_dict): groups = groups - set(pkg_groups) for group in groups: - if not authz.has_user_permission_for_group_or_org(group.id, user, 'update'): + if not authz.has_user_permission_for_group_or_org(group.id, user, 'manage_group'): return False return True diff --git a/ckan/model/package.py b/ckan/model/package.py index 9bd7d695c6e..35873401b04 100644 --- a/ckan/model/package.py +++ b/ckan/model/package.py @@ -227,16 +227,18 @@ def add_relationship(self, type_, related_package, comment=u''): if type_ in package_relationship.PackageRelationship.get_forward_types(): subject = self object_ = related_package + direction = "forward" elif type_ in package_relationship.PackageRelationship.get_reverse_types(): type_ = package_relationship.PackageRelationship.reverse_to_forward_type(type_) assert type_ subject = related_package object_ = self + direction = "reverse" else: raise KeyError, 'Package relationship type: %r' % type_ rels = self.get_relationships(with_package=related_package, - type=type_, active=False, direction="forward") + type=type_, active=False, direction=direction) if rels: rel = rels[0] if comment: diff --git a/ckan/tests/controllers/test_group.py b/ckan/tests/controllers/test_group.py index 3c285c539fd..102e953f39e 100644 --- a/ckan/tests/controllers/test_group.py +++ b/ckan/tests/controllers/test_group.py @@ -34,6 +34,20 @@ def test_page_thru_list_of_orgs(self): assert orgs[0]['name'] not in response2 assert orgs[-1]['name'] in response2 + def test_invalid_sort_param_does_not_crash(self): + app = self._get_test_app() + group_url = url_for(controller='group', + action='index', + sort='title desc nope') + + app.get(url=group_url) + + group_url = url_for(controller='group', + action='index', + sort='title nope desc nope') + + app.get(url=group_url) + def _get_group_new_page(app): user = factories.User() diff --git a/ckan/tests/legacy/lib/test_helpers.py b/ckan/tests/legacy/lib/test_helpers.py index dc35fb1adea..953d095f1f6 100644 --- a/ckan/tests/legacy/lib/test_helpers.py +++ b/ckan/tests/legacy/lib/test_helpers.py @@ -34,6 +34,20 @@ def test_render_datetime_blank(self): res = h.render_datetime(None) assert_equal(res, '') + def test_render_datetime_year_before_1900(self): + res = h.render_datetime('1875-04-13T20:40:20.123456', date_format='%Y') + assert_equal(res, '1875') + + res = h.render_datetime('1875-04-13T20:40:20.123456', date_format='%y') + assert_equal(res, '75') + + def test_render_datetime_year_before_1900_escape_percent(self): + res = h.render_datetime('1875-04-13', date_format='%%%y') + assert_equal(res, '%75') + + res = h.render_datetime('1875-04-13', date_format='%%%Y') + assert_equal(res, '%1875') + def test_datetime_to_date_str(self): res = datetime.datetime(2008, 4, 13, 20, 40, 20, 123456).isoformat() assert_equal(res, '2008-04-13T20:40:20.123456') diff --git a/ckan/tests/legacy/models/test_package_relationships.py b/ckan/tests/legacy/models/test_package_relationships.py index 63ba65f2b52..3258d70649a 100644 --- a/ckan/tests/legacy/models/test_package_relationships.py +++ b/ckan/tests/legacy/models/test_package_relationships.py @@ -6,7 +6,7 @@ class TestCreation: @classmethod def teardown(self): model.repo.rebuild_db() - + def test_normal_creation(self): create = CreateTestData create.create_arbitrary([{'name':u'the-parent', 'title':u'The Parent'}, @@ -53,7 +53,7 @@ def test_reverse_creation(self): assert pr.comment == u'Some comment', pr.comment assert pr.subject == thechild assert pr.object == theparent - + def test_types(self): create = CreateTestData create.create_arbitrary([{'name':u'pkga', 'title':u'The Parent'}, @@ -71,7 +71,7 @@ def test_types(self): # pkga derives_from pkgb # pkgb child_of pkga # pkgb depends_on pkga - + pkga = model.Package.by_name(u'pkga') pkgb = model.Package.by_name(u'pkgb') assert len(pkga.relationships_as_subject) == 2, pkga.relationships_as_subject @@ -128,12 +128,12 @@ def test_usage(self): assert pkga_subject_query.count() == 2 for rel in pkga_subject_query: assert rel.subject == self.pkga - + pkgb_object_query = model.PackageRelationship.by_object(self.pkgb) assert pkgb_object_query.count() == 3, pkgb_object_query.count() for rel in pkgb_object_query: assert rel.object == self.pkgb - + class TestComplicated: @classmethod def setup_class(self): @@ -198,24 +198,26 @@ def setup_class(self): def teardown_class(self): model.repo.rebuild_db() + def _check(self, rels, subject, type, object): + for rel in rels: + if rel.subject.name == subject and rel.type == type and rel.object.name == object: + return + assert 0, 'Could not find relationship in: %r' % rels + def test_01_rels(self): "audit the simpsons family relationships" rels = model.Package.by_name(u'homer').get_relationships() assert len(rels) == 5, '%i: %s' % (len(rels), [rel for rel in rels]) - def check(rels, subject, type, object): - for rel in rels: - if rel.subject.name == subject and rel.type == type and rel.object.name == object: - return - assert 0, 'Could not find relationship in: %r' % rels - check(rels, 'homer', 'child_of', 'abraham') - check(rels, 'bart', 'child_of', 'homer') - check(rels, 'lisa', 'child_of', 'homer') - check(rels, 'homer_derived', 'derives_from', 'homer') - check(rels, 'homer', 'depends_on', 'beer') + + self._check(rels, 'homer', 'child_of', 'abraham') + self._check(rels, 'bart', 'child_of', 'homer') + self._check(rels, 'lisa', 'child_of', 'homer') + self._check(rels, 'homer_derived', 'derives_from', 'homer') + self._check(rels, 'homer', 'depends_on', 'beer') rels = model.Package.by_name(u'bart').get_relationships() assert len(rels) == 2, len(rels) - check(rels, 'bart', 'child_of', 'homer') - check(rels, 'bart', 'child_of', 'marge') + self._check(rels, 'bart', 'child_of', 'homer') + self._check(rels, 'bart', 'child_of', 'marge') def test_02_deletion(self): "delete bart is child of homer" @@ -223,12 +225,12 @@ def test_02_deletion(self): assert len(rels) == 2 assert rels[0].state == model.State.ACTIVE - + model.repo.new_revision() rels[0].delete() rels[1].delete() model.repo.commit_and_remove() - + rels = model.Package.by_name(u'bart').get_relationships() assert len(rels) == 0 @@ -277,7 +279,34 @@ def test_04_relationship_display(self): bart = model.Package.by_name(u"bart") assert len(bart.get_relationships_printable()) == 2, len(bart.get_relationships_printable()) + def test_05_revers_recreation(self): + homer = model.Package.by_name(u"homer") + homer_derived = model.Package.by_name(u"homer_derived") + rels = homer.get_relationships(with_package=homer_derived) + self._check(rels, 'homer_derived', 'derives_from', 'homer') + model.repo.new_revision() + rels[0].delete() + model.repo.commit_and_remove() + rels = homer.get_relationships(with_package=homer_derived) + assert len(homer.get_relationships(with_package=homer_derived)) == 0, \ + 'expectiong homer to have no relationships' + model.repo.new_revision() + homer.add_relationship(u"derives_from", homer_derived) + model.repo.commit_and_remove() + rels = homer.get_relationships(with_package=homer_derived) + assert len(homer.get_relationships(with_package=homer_derived)) == 1, \ + 'expectiong homer to have newly created relationship' + self._check(rels, 'homer', 'derives_from', 'homer_derived') - + model.repo.new_revision() + rels[0].delete() + model.repo.commit_and_remove() + model.repo.new_revision() + homer.add_relationship(u"has_derivation", homer_derived) + model.repo.commit_and_remove() + rels = homer.get_relationships(with_package=homer_derived) + assert len(homer.get_relationships(with_package=homer_derived)) == 1, \ + 'expectiong homer to have recreated initial relationship' + self._check(rels, 'homer_derived', 'derives_from', 'homer') diff --git a/ckan/tests/logic/action/test_update.py b/ckan/tests/logic/action/test_update.py index 43530cac732..4fb83b42df8 100644 --- a/ckan/tests/logic/action/test_update.py +++ b/ckan/tests/logic/action/test_update.py @@ -241,7 +241,7 @@ def test_user_update_activity_stream(self): '''Test that the right activity is emitted when updating a user.''' user = factories.User() - before = datetime.datetime.now() + before = datetime.datetime.utcnow() # FIXME we have to pass the email address and password to user_update # even though we're not updating those fields, otherwise validation @@ -258,7 +258,7 @@ def test_user_update_activity_stream(self): assert latest_activity['activity_type'] == 'changed user' assert latest_activity['object_id'] == user['id'] assert latest_activity['user_id'] == user['id'] - after = datetime.datetime.now() + after = datetime.datetime.utcnow() timestamp = datetime_from_string(latest_activity['timestamp']) assert timestamp >= before and timestamp <= after