From 05375b57cb80f80f4510f47964eb457bb85758b1 Mon Sep 17 00:00:00 2001 From: Louis des Landes Date: Thu, 19 May 2016 13:23:28 +1000 Subject: [PATCH 01/12] If a test app exists, reset it for each test --- ckan/tests/helpers.py | 2 ++ 1 file changed, 2 insertions(+) diff --git a/ckan/tests/helpers.py b/ckan/tests/helpers.py index 58278154d50..a1f24928739 100644 --- a/ckan/tests/helpers.py +++ b/ckan/tests/helpers.py @@ -188,6 +188,8 @@ def _apply_config_changes(cls, cfg): def setup(self): '''Reset the database and clear the search indexes.''' reset_db() + if hasattr(self, '_test_app'): + self._test_app.reset() search.clear_all() @classmethod From 991c40c48dd9d9a853e98275618872b025c485f5 Mon Sep 17 00:00:00 2001 From: Louis des Landes Date: Thu, 19 May 2016 13:23:05 +1000 Subject: [PATCH 02/12] Fix user log out on username change Fixes #2394 --- ckan/controllers/user.py | 25 ++++++-- ckan/tests/controllers/test_user.py | 94 +++++++++++++++++++++++++++++ 2 files changed, 115 insertions(+), 4 deletions(-) diff --git a/ckan/controllers/user.py b/ckan/controllers/user.py index 5f8f8086fda..d911c214bfb 100644 --- a/ckan/controllers/user.py +++ b/ckan/controllers/user.py @@ -35,6 +35,15 @@ unflatten = dictization_functions.unflatten +def set_repoze_user(user_id): + '''Set the repoze.who cookie to match a given user_id''' + if 'repoze.who.plugins' in request.environ: + rememberer = request.environ['repoze.who.plugins']['friendlyform'] + identity = {'repoze.who.userid': user_id} + response.headerlist += rememberer.remember(request.environ, + identity) + + class UserController(base.BaseController): def __before__(self, action, **env): base.BaseController.__before__(self, action, **env) @@ -245,10 +254,7 @@ def _save_new(self, context): return self.new(data_dict, errors, error_summary) if not c.user: # log the user in programatically - rememberer = request.environ['repoze.who.plugins']['friendlyform'] - identity = {'repoze.who.userid': data_dict['name']} - response.headerlist += rememberer.remember(request.environ, - identity) + set_repoze_user(data_dict['name']) h.redirect_to(controller='user', action='me', __ckan_no_root=True) else: # #1799 User has managed to register whilst logged in - warn user @@ -321,6 +327,12 @@ def edit(self, id=None, data=None, errors=None, error_summary=None): def _save_edit(self, id, context): try: + if id in (c.userobj.id, c.userobj.name): + current_user = True + else: + current_user = False + old_username = c.userobj.name + data_dict = logic.clean_dict(unflatten( logic.tuplize_dict(logic.parse_params(request.params)))) context['message'] = data_dict.get('log_message', '') @@ -343,6 +355,11 @@ def _save_edit(self, id, context): user = get_action('user_update')(context, data_dict) h.flash_success(_('Profile updated')) + + if current_user and data_dict['name'] != old_username: + # Changing currently logged in user's name. + # Update repoze.who cookie to match + set_repoze_user(data_dict['name']) h.redirect_to(controller='user', action='read', id=user['name']) except NotAuthorized: abort(403, _('Unauthorized to edit user %s') % id) diff --git a/ckan/tests/controllers/test_user.py b/ckan/tests/controllers/test_user.py index ffefa1e0011..cf78056b79c 100644 --- a/ckan/tests/controllers/test_user.py +++ b/ckan/tests/controllers/test_user.py @@ -289,6 +289,100 @@ def test_email_change_with_password(self): response = submit_and_follow(app, form, env, 'save') assert_true('Profile updated' in response) + def test_edit_user_logged_in_username_change(self): + + user_pass = 'pass' + user = factories.User(password=user_pass) + app = self._get_test_app() + + # Have to do an actual login as this test relys on repoze cookie handling. + # get the form + response = app.get('/user/login') + # ...it's the second one + login_form = response.forms[1] + # fill it in + login_form['login'] = user['name'] + login_form['password'] = user_pass + # submit it + login_form.submit() + + # Now the cookie is set, run the test + response = app.get( + url=url_for(controller='user', action='edit'), + ) + # existing values in the form + form = response.forms['user-edit-form'] + + # new values + form['name'] = 'new-name' + response = submit_and_follow(app, form, name='save') + response = helpers.webtest_maybe_follow(response) + + expected_url = url_for(controller='user', action='read', id='new-name') + assert response.request.path == expected_url + + def test_edit_user_logged_in_username_change_by_name(self): + user_pass = 'pass' + user = factories.User(password=user_pass) + app = self._get_test_app() + + # Have to do an actual login as this test relys on repoze cookie handling. + # get the form + response = app.get('/user/login') + # ...it's the second one + login_form = response.forms[1] + # fill it in + login_form['login'] = user['name'] + login_form['password'] = user_pass + # submit it + login_form.submit() + + # Now the cookie is set, run the test + response = app.get( + url=url_for(controller='user', action='edit', id=user['name']), + ) + # existing values in the form + form = response.forms['user-edit-form'] + + # new values + form['name'] = 'new-name' + response = submit_and_follow(app, form, name='save') + response = helpers.webtest_maybe_follow(response) + + expected_url = url_for(controller='user', action='read', id='new-name') + assert response.request.path == expected_url + + def test_edit_user_logged_in_username_change_by_id(self): + user_pass = 'pass' + user = factories.User(password=user_pass) + app = self._get_test_app() + + # Have to do an actual login as this test relys on repoze cookie handling. + # get the form + response = app.get('/user/login') + # ...it's the second one + login_form = response.forms[1] + # fill it in + login_form['login'] = user['name'] + login_form['password'] = user_pass + # submit it + login_form.submit() + + # Now the cookie is set, run the test + response = app.get( + url=url_for(controller='user', action='edit', id=user['id']), + ) + # existing values in the form + form = response.forms['user-edit-form'] + + # new values + form['name'] = 'new-name' + response = submit_and_follow(app, form, name='save') + response = helpers.webtest_maybe_follow(response) + + expected_url = url_for(controller='user', action='read', id='new-name') + assert response.request.path == expected_url + def test_perform_reset_for_key_change(self): password = 'password' params = {'password1': password, 'password2': password} From 41039d48188fd058736eec03d10e0f6b2606ac35 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Knut=20H=C3=BChne?= Date: Fri, 20 May 2016 10:01:32 +0200 Subject: [PATCH 03/12] [#2990] Create 403 on organization_index if user has no permission --- ckan/logic/action/get.py | 12 ++++++++---- ckan/tests/controllers/test_organization.py | 15 +++++++++++++++ 2 files changed, 23 insertions(+), 4 deletions(-) diff --git a/ckan/logic/action/get.py b/ckan/logic/action/get.py index e57e4a1819d..200981aeccb 100644 --- a/ckan/logic/action/get.py +++ b/ckan/logic/action/get.py @@ -26,6 +26,7 @@ import ckan.authz as authz from ckan.common import _ +from ckan.lib import base log = logging.getLogger('ckan.logic') @@ -518,10 +519,13 @@ def organization_list(context, data_dict): :rtype: list of strings ''' - _check_access('organization_list', context, data_dict) - data_dict['groups'] = data_dict.pop('organizations', []) - data_dict.setdefault('type', 'organization') - return _group_or_org_list(context, data_dict, is_org=True) + try: + _check_access('organization_list', context, data_dict) + data_dict['groups'] = data_dict.pop('organizations', []) + data_dict.setdefault('type', 'organization') + return _group_or_org_list(context, data_dict, is_org=True) + except logic.NotAuthorized: + base.abort(403, _('You are not authorized to see a list of organizations')) def group_list_authz(context, data_dict): diff --git a/ckan/tests/controllers/test_organization.py b/ckan/tests/controllers/test_organization.py index ce8a875928b..0db260cb38e 100644 --- a/ckan/tests/controllers/test_organization.py +++ b/ckan/tests/controllers/test_organization.py @@ -61,6 +61,21 @@ def test_all_fields_saved(self): assert_equal(group['description'], 'Sciencey datasets') +class TestOrganizationList(helpers.FunctionalTestBase): + def setup(self): + super(TestOrganizationList, self).setup() + self.app = helpers._get_test_app() + self.user = factories.User() + self.user_env = {'REMOTE_USER': self.user['name'].encode('ascii')} + self.organization_list_url = url_for(controller='organization', + action='index') + + def test_error_message_shown_when_no_organization_list_permission(self, mock_check_access): + response = self.app.get(url=self.organization_list_url, + extra_environ=self.user_env) + assert response.status_int == 403 + + class TestOrganizationRead(helpers.FunctionalTestBase): def setup(self): super(TestOrganizationRead, self).setup() From 0fa410b710e8d06c2d1e698772237dfa937e5d54 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Knut=20H=C3=BChne?= Date: Fri, 20 May 2016 11:57:40 +0200 Subject: [PATCH 04/12] [#2990] Add test for organization_list - The test makes sure that the response is a 403 - Mosly this is to make sure that the view renders at all and that it does not throw an exception in the backend anymore like it used to --- ckan/tests/controllers/test_organization.py | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/ckan/tests/controllers/test_organization.py b/ckan/tests/controllers/test_organization.py index 0db260cb38e..549a92aa9f1 100644 --- a/ckan/tests/controllers/test_organization.py +++ b/ckan/tests/controllers/test_organization.py @@ -1,6 +1,7 @@ from bs4 import BeautifulSoup from nose.tools import assert_equal, assert_true from routes import url_for +from mock import patch from ckan.tests import factories, helpers from ckan.tests.helpers import webtest_submit, submit_and_follow, assert_in @@ -70,10 +71,11 @@ def setup(self): self.organization_list_url = url_for(controller='organization', action='index') + @patch('ckan.logic.auth.get.organization_list', return_value={'success': False}) def test_error_message_shown_when_no_organization_list_permission(self, mock_check_access): response = self.app.get(url=self.organization_list_url, - extra_environ=self.user_env) - assert response.status_int == 403 + extra_environ=self.user_env, + status=403) class TestOrganizationRead(helpers.FunctionalTestBase): From bcc644f67dcc983a127f31ce9fc44d6b6924c711 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Knut=20H=C3=BChne?= Date: Fri, 20 May 2016 14:39:35 +0200 Subject: [PATCH 05/12] [#2990] Move check for permission to controller --- ckan/controllers/group.py | 1 + ckan/logic/action/get.py | 11 ++++------- 2 files changed, 5 insertions(+), 7 deletions(-) diff --git a/ckan/controllers/group.py b/ckan/controllers/group.py index 61f6a76b20f..b01050d37fc 100644 --- a/ckan/controllers/group.py +++ b/ckan/controllers/group.py @@ -159,6 +159,7 @@ def index(self): sort_by = c.sort_by_selected = request.params.get('sort') try: self._check_access('site_read', context) + self._check_access('group_list', context) except NotAuthorized: abort(403, _('Not authorized to see this page')) diff --git a/ckan/logic/action/get.py b/ckan/logic/action/get.py index 200981aeccb..2a65d4d49ee 100644 --- a/ckan/logic/action/get.py +++ b/ckan/logic/action/get.py @@ -519,13 +519,10 @@ def organization_list(context, data_dict): :rtype: list of strings ''' - try: - _check_access('organization_list', context, data_dict) - data_dict['groups'] = data_dict.pop('organizations', []) - data_dict.setdefault('type', 'organization') - return _group_or_org_list(context, data_dict, is_org=True) - except logic.NotAuthorized: - base.abort(403, _('You are not authorized to see a list of organizations')) + _check_access('organization_list', context, data_dict) + data_dict['groups'] = data_dict.pop('organizations', []) + data_dict.setdefault('type', 'organization') + return _group_or_org_list(context, data_dict, is_org=True) def group_list_authz(context, data_dict): From 3c802655579f2e94087626f55f3d09ed6531b20b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Knut=20H=C3=BChne?= Date: Fri, 20 May 2016 14:48:12 +0200 Subject: [PATCH 06/12] [#2990] remove stray import --- ckan/logic/action/get.py | 1 - 1 file changed, 1 deletion(-) diff --git a/ckan/logic/action/get.py b/ckan/logic/action/get.py index 2a65d4d49ee..e57e4a1819d 100644 --- a/ckan/logic/action/get.py +++ b/ckan/logic/action/get.py @@ -26,7 +26,6 @@ import ckan.authz as authz from ckan.common import _ -from ckan.lib import base log = logging.getLogger('ckan.logic') From b1f9c75fedf9beadc16ae4a10eed73ac723cb336 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Knut=20H=C3=BChne?= Date: Fri, 20 May 2016 16:12:47 +0200 Subject: [PATCH 07/12] [#2981] Preserve sort order in groups on pagination - Uses the same code that was introuced in #2153 for organizations - Changes the tests to sort descending so that they actually make sure that the sort order is not changed --- ckan/templates/group/index.html | 2 +- ckan/tests/controllers/test_group.py | 31 +++++++++++++++++++++------- 2 files changed, 25 insertions(+), 8 deletions(-) diff --git a/ckan/templates/group/index.html b/ckan/templates/group/index.html index f9c56530267..2d04c2a467d 100644 --- a/ckan/templates/group/index.html +++ b/ckan/templates/group/index.html @@ -34,7 +34,7 @@

{{ _('Groups') }}

{% endif %} {% endblock %} {% block page_pagination %} - {{ c.page.pager() }} + {{ c.page.pager(q=c.q or '', sort=c.sort_by_selected or '') }} {% endblock %} {% endblock %} diff --git a/ckan/tests/controllers/test_group.py b/ckan/tests/controllers/test_group.py index bb8edaab2ab..b80724dc5b2 100644 --- a/ckan/tests/controllers/test_group.py +++ b/ckan/tests/controllers/test_group.py @@ -23,17 +23,34 @@ def test_bulk_process_throws_404_for_nonexistent_org(self): action='bulk_process', id='does-not-exist') app.get(url=bulk_process_url, status=404) - def test_page_thru_list_of_orgs(self): - orgs = [factories.Organization() for i in range(35)] + def test_page_thru_list_of_orgs_preserves_sort_order(self): + orgs = [factories.Organization() for _ in range(35)] app = self._get_test_app() - org_url = url_for(controller='organization', action='index') + org_url = url_for(controller='organization', + action='index', + sort='name desc') response = app.get(url=org_url) - assert orgs[0]['name'] in response - assert orgs[-1]['name'] not in response + assert orgs[-1]['name'] in response + assert orgs[0]['name'] not in response response2 = response.click('2') - assert orgs[0]['name'] not in response2 - assert orgs[-1]['name'] in response2 + assert orgs[-1]['name'] not in response2 + assert orgs[0]['name'] in response2 + + def test_page_thru_list_of_groups_preserves_sort_order(self): + groups = [factories.Group() for _ in range(35)] + app = self._get_test_app() + group_url = url_for(controller='group', + action='index', + sort='title desc') + + response = app.get(url=group_url) + assert groups[-1]['title'] in response + assert groups[0]['title'] not in response + + response2 = response.click(r'^2$') + assert groups[-1]['title'] not in response2 + assert groups[0]['title'] in response2 def _get_group_new_page(app): From 4d64ec6c2b9e7d62ced5d2780df2b1f4da131546 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Knut=20H=C3=BChne?= Date: Mon, 23 May 2016 16:03:54 +0200 Subject: [PATCH 08/12] [#2981] add empty params to paginated url in tests --- ckan/tests/legacy/functional/test_pagination.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/ckan/tests/legacy/functional/test_pagination.py b/ckan/tests/legacy/functional/test_pagination.py index f92fd72265b..59726aed2d7 100644 --- a/ckan/tests/legacy/functional/test_pagination.py +++ b/ckan/tests/legacy/functional/test_pagination.py @@ -100,12 +100,12 @@ def teardown_class(self): def test_group_index(self): res = self.app.get(url_for(controller='group', action='index')) - assert 'href="/group?page=2"' in res, res + assert 'href="/group?sort=&q=&page=2"' in res, res grp_numbers = scrape_search_results(res, 'group') assert_equal(['00', '01', '02', '03', '04', '05', '06', '07', '08', '09', '10', '11', '12', '13', '14', '15', '16', '17', '18', '19', '20'], grp_numbers) res = self.app.get(url_for(controller='group', action='index', page=2)) - assert 'href="/group?page=1"' in res + assert 'href="/group?sort=&q=&page=1"' in res grp_numbers = scrape_search_results(res, 'group') assert_equal(['21'], grp_numbers) From b0f0cd2c46d55b46b2502782981c1c5dcfc1f99d Mon Sep 17 00:00:00 2001 From: Louis des Landes Date: Thu, 12 May 2016 11:12:49 +1000 Subject: [PATCH 09/12] Add missing routes to IGroupForm (And fix bulk process to check 'is_organization' instead of the group type) --- ckan/controllers/group.py | 12 +-- ckan/lib/plugins.py | 23 +++-- ckanext/example_igroupform/plugin.py | 26 ++++++ .../tests/test_controllers.py | 91 +++++++++++++++++++ setup.py | 1 + 5 files changed, 140 insertions(+), 13 deletions(-) diff --git a/ckan/controllers/group.py b/ckan/controllers/group.py index 61f6a76b20f..3db017bf272 100644 --- a/ckan/controllers/group.py +++ b/ckan/controllers/group.py @@ -380,10 +380,6 @@ def bulk_process(self, id): group_type = self._ensure_controller_matches_group_type( id.split('@')[0]) - if group_type != 'organization': - # FIXME: better error - raise Exception('Must be an organization') - # check we are org admin context = {'model': model, 'session': model.Session, @@ -401,6 +397,10 @@ def bulk_process(self, id): except (NotFound, NotAuthorized): abort(404, _('Group not found')) + if not c.group_dict['is_organization']: + # FIXME: better error + raise Exception('Must be an organization') + #use different form names so that ie7 can be detected form_names = set(["bulk_action.public", "bulk_action.delete", "bulk_action.private"]) @@ -443,9 +443,7 @@ def bulk_process(self, id): get_action(action_functions[action])(context, data_dict) except NotAuthorized: abort(403, _('Not authorized to perform bulk update')) - base.redirect(h.url_for(controller='organization', - action='bulk_process', - id=id)) + self._redirect_to_this_controller(action='bulk_process', id=id) def new(self, data=None, errors=None, error_summary=None): if data and 'type' in data: diff --git a/ckan/lib/plugins.py b/ckan/lib/plugins.py index 132e5b4598f..cd22984af97 100644 --- a/ckan/lib/plugins.py +++ b/ckan/lib/plugins.py @@ -178,7 +178,7 @@ def register_group_plugins(map): '/%s/{action}/{id}' % group_type, controller=group_controller, requirements=dict(action='|'.join( - ['edit', 'authz', 'history', 'member_new', + ['edit', 'authz', 'delete', 'history', 'member_new', 'member_delete', 'followers', 'follow', 'unfollow', 'admins', 'activity']))) map.connect('%s_edit' % group_type, '/%s/edit/{id}' % group_type, @@ -193,6 +193,13 @@ def register_group_plugins(map): '/%s/activity/{id}/{offset}' % group_type, controller=group_controller, action='activity', ckan_icon='time'), + map.connect('%s_about' % group_type, '/%s/about/{id}' % group_type, + controller=group_controller, + action='about', ckan_icon='info-sign') + map.connect('%s_bulk_process' % group_type, + '/%s/bulk_process/{id}' % group_type, + controller=group_controller, + action='bulk_process', ckan_icon='sitemap') if group_type in _group_plugins: raise ValueError("An existing IGroupForm is " @@ -201,12 +208,16 @@ def register_group_plugins(map): _group_plugins[group_type] = plugin _group_controllers[group_type] = group_controller + controller_obj = None + # If using one of the default controllers, tell it that it is allowed + # to handle other group_types. + # Import them here to avoid circular imports. if group_controller == 'group': - # Tell the default group controller that it is allowed to - # handle other group_types. - # Import it here to avoid circular imports. - from ckan.controllers.group import GroupController - GroupController.add_group_type(group_type) + from ckan.controllers.group import GroupController as controller_obj + elif group_controller == 'organization': + from ckan.controllers.organization import OrganizationController as controller_obj + if controller_obj is not None: + controller_obj.add_group_type(group_type) # Setup the fallback behaviour if one hasn't been defined. if _default_group_plugin is None: diff --git a/ckanext/example_igroupform/plugin.py b/ckanext/example_igroupform/plugin.py index a95a4fd1538..5c1e76ee451 100644 --- a/ckanext/example_igroupform/plugin.py +++ b/ckanext/example_igroupform/plugin.py @@ -61,3 +61,29 @@ def is_fallback(self): def group_form(self): return 'example_igroup_form/group_form.html' + + +class ExampleIGroupFormOrganizationPlugin(plugins.SingletonPlugin, + tk.DefaultOrganizationForm): + '''An example IGroupForm Organization CKAN plugin with custom group_type. + + Doesn't do much yet. + ''' + plugins.implements(plugins.IGroupForm, inherit=False) + plugins.implements(plugins.IConfigurer) + + # IConfigurer + + def update_config(self, config_): + tk.add_template_directory(config_, 'templates') + + # IGroupForm + + def group_types(self): + return (group_type,) + + def is_fallback(self): + False + + def group_controller(self): + return 'organization' diff --git a/ckanext/example_igroupform/tests/test_controllers.py b/ckanext/example_igroupform/tests/test_controllers.py index 328eeb3453d..1a930348fd6 100644 --- a/ckanext/example_igroupform/tests/test_controllers.py +++ b/ckanext/example_igroupform/tests/test_controllers.py @@ -24,6 +24,97 @@ def _get_group_new_page(app, group_type): return env, response +class TestGroupController(helpers.FunctionalTestBase): + @classmethod + def setup_class(cls): + super(TestGroupController, cls).setup_class() + plugins.load('example_igroupform') + + @classmethod + def teardown_class(cls): + plugins.unload('example_igroupform') + super(TestGroupController, cls).teardown_class() + + def test_about(self): + app = self._get_test_app() + user = factories.User() + group = factories.Group(user=user, type=custom_group_type) + group_name = group['name'] + env = {'REMOTE_USER': user['name'].encode('ascii')} + url = url_for('%s_about' % custom_group_type, + id=group_name) + response = app.get(url=url, extra_environ=env) + response.mustcontain(group_name) + + def test_bulk_process(self): + app = self._get_test_app() + user = factories.User() + group = factories.Group(user=user, type=custom_group_type) + group_name = group['name'] + env = {'REMOTE_USER': user['name'].encode('ascii')} + url = url_for('%s_bulk_process' % custom_group_type, + id=group_name) + try: + response = app.get(url=url, extra_environ=env) + except Exception as e: + assert (e.args == ('Must be an organization', )) + else: + raise Exception("Response should have raised an exception") + + def test_delete(self): + app = self._get_test_app() + user = factories.User() + group = factories.Group(user=user, type=custom_group_type) + group_name = group['name'] + env = {'REMOTE_USER': user['name'].encode('ascii')} + url = url_for('%s_action' % custom_group_type, action='delete', + id=group_name) + response = app.get(url=url, extra_environ=env) + + +class TestOrganizationController(helpers.FunctionalTestBase): + @classmethod + def setup_class(cls): + super(TestOrganizationController, cls).setup_class() + plugins.load('example_igroupform_organization') + + @classmethod + def teardown_class(cls): + plugins.unload('example_igroupform_organization') + super(TestOrganizationController, cls).teardown_class() + + def test_about(self): + app = self._get_test_app() + user = factories.User() + group = factories.Organization(user=user, type=custom_group_type) + group_name = group['name'] + env = {'REMOTE_USER': user['name'].encode('ascii')} + url = url_for('%s_about' % custom_group_type, + id=group_name) + response = app.get(url=url, extra_environ=env) + response.mustcontain(group_name) + + def test_bulk_process(self): + app = self._get_test_app() + user = factories.User() + group = factories.Organization(user=user, type=custom_group_type) + group_name = group['name'] + env = {'REMOTE_USER': user['name'].encode('ascii')} + url = url_for('%s_bulk_process' % custom_group_type, + id=group_name) + response = app.get(url=url, extra_environ=env) + + def test_delete(self): + app = self._get_test_app() + user = factories.User() + group = factories.Organization(user=user, type=custom_group_type) + group_name = group['name'] + env = {'REMOTE_USER': user['name'].encode('ascii')} + url = url_for('%s_action' % custom_group_type, action='delete', + id=group_name) + response = app.get(url=url, extra_environ=env) + + class TestGroupControllerNew(helpers.FunctionalTestBase): @classmethod def setup_class(cls): diff --git a/setup.py b/setup.py index 8ad0809b4b9..be4edcf6a81 100644 --- a/setup.py +++ b/setup.py @@ -102,6 +102,7 @@ 'example_idatasetform_v4 = ckanext.example_idatasetform.plugin_v4:ExampleIDatasetFormPlugin', 'example_igroupform = ckanext.example_igroupform.plugin:ExampleIGroupFormPlugin', 'example_igroupform_default_group_type = ckanext.example_igroupform.plugin:ExampleIGroupFormPlugin_DefaultGroupType', + 'example_igroupform_organization = ckanext.example_igroupform.plugin:ExampleIGroupFormOrganizationPlugin', 'example_iauthfunctions_v1 = ckanext.example_iauthfunctions.plugin_v1:ExampleIAuthFunctionsPlugin', 'example_iauthfunctions_v2 = ckanext.example_iauthfunctions.plugin_v2:ExampleIAuthFunctionsPlugin', 'example_iauthfunctions_v3 = ckanext.example_iauthfunctions.plugin_v3:ExampleIAuthFunctionsPlugin', From c38b78ddfa000bb4b5ae92b31193ca19acaa9d17 Mon Sep 17 00:00:00 2001 From: Brook Elgie Date: Tue, 24 May 2016 11:26:37 +0100 Subject: [PATCH 10/12] [#2972] Overwrite path to solr core for Travis --- ckan/pastertemplates/template/bin/travis-build.bash_tmpl | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/ckan/pastertemplates/template/bin/travis-build.bash_tmpl b/ckan/pastertemplates/template/bin/travis-build.bash_tmpl index 44d0531e4d6..c58598a917f 100755 --- a/ckan/pastertemplates/template/bin/travis-build.bash_tmpl +++ b/ckan/pastertemplates/template/bin/travis-build.bash_tmpl @@ -22,6 +22,11 @@ echo "Creating the PostgreSQL user and database..." sudo -u postgres psql -c "CREATE USER ckan_default WITH PASSWORD 'pass';" sudo -u postgres psql -c 'CREATE DATABASE ckan_test WITH OWNER ckan_default;' +echo "SOLR config..." +# Solr is multicore for tests on ckan master, but it's easier to run tests on +# Travis single-core. See https://github.com/ckan/ckan/issues/2972 +sed -i -e 's/solr_url.*/solr_url = http:\/\/127.0.0.1:8983\/solr/' ckan/test-core.ini + echo "Initialising the database..." cd ckan paster db init -c test-core.ini From 2e6afc34f7b4e2b946188390c749de5ed97bd2fe Mon Sep 17 00:00:00 2001 From: amercader Date: Wed, 25 May 2016 11:19:03 +0100 Subject: [PATCH 11/12] [#2856] Create DataPusher task timestamps using UTC When an upload to the DataPusher finishes we check `resource['last_modified']` (in UTC) against `task['last_updated']` to check whether it changed during the previous upload and resubmit it. As `task['last_updated']` was created using `datetime.now()` this could lead to infinite upload loops. Discussion is here: https://github.com/ckan/ckan/issues/2856#issuecomment-220383735 --- ckanext/datapusher/logic/action.py | 10 +++++----- ckanext/datapusher/tests/test_action.py | 17 +++++++++-------- 2 files changed, 14 insertions(+), 13 deletions(-) diff --git a/ckanext/datapusher/logic/action.py b/ckanext/datapusher/logic/action.py index 8ce97c2bc8f..5afb4b0b33a 100644 --- a/ckanext/datapusher/logic/action.py +++ b/ckanext/datapusher/logic/action.py @@ -74,7 +74,7 @@ def datapusher_submit(context, data_dict): 'entity_id': res_id, 'entity_type': 'resource', 'task_type': 'datapusher', - 'last_updated': str(datetime.datetime.now()), + 'last_updated': str(datetime.datetime.utcnow()), 'state': 'submitting', 'key': 'datapusher', 'value': '{}', @@ -119,7 +119,7 @@ def datapusher_submit(context, data_dict): 'details': str(e)} task['error'] = json.dumps(error) task['state'] = 'error' - task['last_updated'] = str(datetime.datetime.now()), + task['last_updated'] = str(datetime.datetime.utcnow()), p.toolkit.get_action('task_status_update')(context, task) raise p.toolkit.ValidationError(error) @@ -134,7 +134,7 @@ def datapusher_submit(context, data_dict): 'status_code': r.status_code} task['error'] = json.dumps(error) task['state'] = 'error' - task['last_updated'] = str(datetime.datetime.now()), + task['last_updated'] = str(datetime.datetime.utcnow()), p.toolkit.get_action('task_status_update')(context, task) raise p.toolkit.ValidationError(error) @@ -143,7 +143,7 @@ def datapusher_submit(context, data_dict): task['value'] = value task['state'] = 'pending' - task['last_updated'] = str(datetime.datetime.now()), + task['last_updated'] = str(datetime.datetime.utcnow()), p.toolkit.get_action('task_status_update')(context, task) return True @@ -175,7 +175,7 @@ def datapusher_hook(context, data_dict): }) task['state'] = status - task['last_updated'] = str(datetime.datetime.now()) + task['last_updated'] = str(datetime.datetime.utcnow()) resubmit = False diff --git a/ckanext/datapusher/tests/test_action.py b/ckanext/datapusher/tests/test_action.py index 218406ccbaf..7c24612d9e6 100644 --- a/ckanext/datapusher/tests/test_action.py +++ b/ckanext/datapusher/tests/test_action.py @@ -30,7 +30,7 @@ def _pending_task(self, resource_id): 'entity_id': resource_id, 'entity_type': 'resource', 'task_type': 'datapusher', - 'last_updated': str(datetime.datetime.now()), + 'last_updated': str(datetime.datetime.utcnow()), 'state': 'pending', 'key': 'datapusher', 'value': '{}', @@ -163,13 +163,14 @@ def test_resubmits_if_upload_changes_in_the_meantime( **self._pending_task(resource['id'])) # Update the resource, set a new last_modified (changes on file upload) - helpers.call_action('resource_update', {}, - id=resource['id'], - package_id=dataset['id'], - url='http://example.com/file.csv', - format='CSV', - last_modified=datetime.datetime.now().isoformat() - ) + helpers.call_action( + 'resource_update', {}, + id=resource['id'], + package_id=dataset['id'], + url='http://example.com/file.csv', + format='CSV', + last_modified=datetime.datetime.utcnow().isoformat() + ) # Not called eq_(len(mock_datapusher_submit.mock_calls), 1) From f87cc6a2bdd4bb2154be16da66a06bb1c4ba9229 Mon Sep 17 00:00:00 2001 From: Brook Elgie Date: Thu, 26 May 2016 12:30:23 +0100 Subject: [PATCH 12/12] Move `find_flask_app` helper to test helpers. --- ckan/tests/helpers.py | 28 ++++++++++++++++++ .../tests/test_routes.py | 29 +------------------ 2 files changed, 29 insertions(+), 28 deletions(-) diff --git a/ckan/tests/helpers.py b/ckan/tests/helpers.py index a1f24928739..f41e850cf78 100644 --- a/ckan/tests/helpers.py +++ b/ckan/tests/helpers.py @@ -414,3 +414,31 @@ def wrapper(*args, **kwargs): return return_value return nose.tools.make_decorator(func)(wrapper) return decorator + + +def find_flask_app(test_app): + ''' + Helper function to recursively search the wsgi stack in `test_app` until + the flask_app is discovered. + + Relies on each layer of the stack having a reference to the app they + wrap in either a .app attribute or .apps list. + ''' + if isinstance(test_app, ckan.config.middleware.CKANFlask): + return test_app + + try: + app = test_app.apps['flask_app'].app + except (AttributeError, KeyError): + pass + else: + return find_flask_app(app) + + try: + app = test_app.app + except AttributeError: + print('No .app attribute. ' + 'Have all layers of the stack got ' + 'a reference to the app they wrap?') + else: + return find_flask_app(app) diff --git a/ckanext/example_flask_iroutes/tests/test_routes.py b/ckanext/example_flask_iroutes/tests/test_routes.py index fa8110c76c8..8e4dcc1799f 100644 --- a/ckanext/example_flask_iroutes/tests/test_routes.py +++ b/ckanext/example_flask_iroutes/tests/test_routes.py @@ -1,41 +1,14 @@ from nose.tools import eq_, ok_ -from ckan.config.middleware import CKANFlask import ckan.plugins as plugins import ckan.tests.helpers as helpers class TestFlaskIRoutes(helpers.FunctionalTestBase): - @classmethod - def _find_flask_app(cls, test_app): - '''Recursively search the wsgi stack until the flask_app is - discovered. - - Relies on each layer of the stack having a reference to the app they - wrap in either a .app attribute or .apps list. - ''' - if isinstance(test_app, CKANFlask): - return test_app - - try: - app = test_app.apps['flask_app'].app - except (AttributeError, KeyError): - pass - else: - return cls._find_flask_app(app) - - try: - app = test_app.app - except AttributeError: - print('No .app attribute. ' - 'Have all layers of the stack got ' - 'a reference to the app they wrap?') - else: - return cls._find_flask_app(app) def setup(self): self.app = helpers._get_test_app() - flask_app = self._find_flask_app(self.app) + flask_app = helpers.find_flask_app(self.app) # Blueprints can't be registered after the app has been setup. For # some reason, if debug is True, the app will have exited its initial