From 3a22c90824427ffb90f5a6359ae0d7d7eb3299f6 Mon Sep 17 00:00:00 2001 From: John Martin Date: Tue, 20 Nov 2012 19:12:42 +0000 Subject: [PATCH 01/27] First version of the suggested front end testing document --- doc/frontend-testing.rst | 62 ++++++++++++++++++++++++++++++++++++++++ 1 file changed, 62 insertions(+) create mode 100644 doc/frontend-testing.rst diff --git a/doc/frontend-testing.rst b/doc/frontend-testing.rst new file mode 100644 index 00000000000..318a5c79177 --- /dev/null +++ b/doc/frontend-testing.rst @@ -0,0 +1,62 @@ +Front-end Testing +================= + +All CKAN features should be coded so that they work in the +following browsers: + +* Internet Explorer: 9, 8 and 7 +* Firefox: Latest +* Chrome: Latest + +These browsers are determined by whatever has >=1% share +on this page + +Install browser virtual machines +================================ + +In order to test in all the needed browsers you'll need access to +all the above browser versions. Firefox and Chrome should be easy +whatever platform you are on. Internet Explorer is a little trickier. +You'll need Virtual Machines. + +We suggest you use to get an +Internet Explorer virtual machines. + +Testing methodology +=================== + +Soon... + +Common pitfulls & their fixes +============================= + +Reserver JS keywords +-------------------- + +Since IE has a stricter language definition in JS it really doesn't +like you using JS reserved keywords method names, variables, etc... +This is a good list of keywords not to use in your JavaScript: + + + +Unclosed JS arrays / objects +---------------------------- + +Internet Explorer doesn't like it's JS to have unclosed JS objects +and arrays. For example: + +:: + + var foo = { + bar: 'Test', + }; + +Will break. However: + +:: + + var foo = { + bar: 'Test' + }; + +Will not break. \ No newline at end of file From 4b2b0f6d3171291f5dea9decabd1894430086a5f Mon Sep 17 00:00:00 2001 From: John Martin Date: Wed, 21 Nov 2012 10:52:44 +0000 Subject: [PATCH 02/27] Final tweaks to front end testing doc --- doc/frontend-testing.rst | 31 +++++++++++++++++++++++-------- 1 file changed, 23 insertions(+), 8 deletions(-) diff --git a/doc/frontend-testing.rst b/doc/frontend-testing.rst index 318a5c79177..a202d88e21d 100644 --- a/doc/frontend-testing.rst +++ b/doc/frontend-testing.rst @@ -1,15 +1,15 @@ Front-end Testing ================= -All CKAN features should be coded so that they work in the +All new CKAN features should be coded so that they work in the following browsers: * Internet Explorer: 9, 8 and 7 * Firefox: Latest * Chrome: Latest -These browsers are determined by whatever has >=1% share -on this page +These browsers are determined by whatever has >= 1% share on this +page: http://data.gov.uk/data/site-usage?month=2012-11 Install browser virtual machines ================================ @@ -19,25 +19,40 @@ all the above browser versions. Firefox and Chrome should be easy whatever platform you are on. Internet Explorer is a little trickier. You'll need Virtual Machines. -We suggest you use to get an +We suggest you use https://github.com/xdissent/ievms to get your Internet Explorer virtual machines. Testing methodology =================== -Soon... +Firstly we have a primer page. If you've touched any of the core +front-end code you'll need to check if the primer is rendering +correctly. The primer is located at: +http://localhost:5000/testing/primer and it should look like: +https://dl.dropbox.com/s/69y9ecpf3u3zkpe/ckan-primer.png + +Secondly whilst writing a new feature you should endeavour to test +in at least in your core browser and an alternative browser as often +as you can. + +Thirdly you should fully test all new features that have a front-end +element in all browsers before making your pull request into +CKAN master. Common pitfulls & their fixes ============================= -Reserver JS keywords +Here's a few of the most common front end bugs and a list of their +fixes. + +Reserved JS keywords -------------------- Since IE has a stricter language definition in JS it really doesn't like you using JS reserved keywords method names, variables, etc... This is a good list of keywords not to use in your JavaScript: - +https://developer.mozilla.org/en-US/docs/JavaScript/Reference/Reserved_Words Unclosed JS arrays / objects ---------------------------- @@ -59,4 +74,4 @@ Will break. However: bar: 'Test' }; -Will not break. \ No newline at end of file +Will not break. From f0669af5071080742ca80663fcd0e221f4ae44e6 Mon Sep 17 00:00:00 2001 From: John Martin Date: Wed, 21 Nov 2012 12:30:56 +0000 Subject: [PATCH 03/27] Clarified browser stats and added some code samples --- doc/frontend-testing.rst | 43 +++++++++++++++++++++++++--------------- 1 file changed, 27 insertions(+), 16 deletions(-) diff --git a/doc/frontend-testing.rst b/doc/frontend-testing.rst index a202d88e21d..8d814dd12b1 100644 --- a/doc/frontend-testing.rst +++ b/doc/frontend-testing.rst @@ -5,11 +5,11 @@ All new CKAN features should be coded so that they work in the following browsers: * Internet Explorer: 9, 8 and 7 -* Firefox: Latest -* Chrome: Latest +* Firefox: Latest + previous version +* Chrome: Latest + previous version -These browsers are determined by whatever has >= 1% share on this -page: http://data.gov.uk/data/site-usage?month=2012-11 +These browsers are determined by whatever has >= 1% share with the +latest months data from: http://data.gov.uk/data/site-usage Install browser virtual machines ================================ @@ -28,8 +28,7 @@ Testing methodology Firstly we have a primer page. If you've touched any of the core front-end code you'll need to check if the primer is rendering correctly. The primer is located at: -http://localhost:5000/testing/primer and it should look like: -https://dl.dropbox.com/s/69y9ecpf3u3zkpe/ckan-primer.png +http://localhost:5000/testing/primer Secondly whilst writing a new feature you should endeavour to test in at least in your core browser and an alternative browser as often @@ -54,6 +53,20 @@ This is a good list of keywords not to use in your JavaScript: https://developer.mozilla.org/en-US/docs/JavaScript/Reference/Reserved_Words +:: + + /* These are bad */ + var a = { + default: 1, + delete: function() {} + }; + + /* These are good */ + var a = { + default_value: 1, + remove: function() {} + }; + Unclosed JS arrays / objects ---------------------------- @@ -62,16 +75,14 @@ and arrays. For example: :: - var foo = { - bar: 'Test', + /* These are bad */ + var a = { + b: 'c', }; + var a = ['b', 'c', ]; -Will break. However: - -:: - - var foo = { - bar: 'Test' + /* These are good */ + var a = { + c: 'c' }; - -Will not break. + var a = ['b', 'c']; From 1625ecf2e4552e82222b9bc65e03ffd0070fd5fe Mon Sep 17 00:00:00 2001 From: tobes Date: Fri, 23 Nov 2012 12:51:57 +0000 Subject: [PATCH 04/27] Minor doc tweeks --- doc/frontend-testing.rst | 46 ++++++++++++++++++++-------------------- 1 file changed, 23 insertions(+), 23 deletions(-) diff --git a/doc/frontend-testing.rst b/doc/frontend-testing.rst index 8d814dd12b1..8387b51b051 100644 --- a/doc/frontend-testing.rst +++ b/doc/frontend-testing.rst @@ -12,7 +12,7 @@ These browsers are determined by whatever has >= 1% share with the latest months data from: http://data.gov.uk/data/site-usage Install browser virtual machines -================================ +-------------------------------- In order to test in all the needed browsers you'll need access to all the above browser versions. Firefox and Chrome should be easy @@ -23,7 +23,7 @@ We suggest you use https://github.com/xdissent/ievms to get your Internet Explorer virtual machines. Testing methodology -=================== +------------------- Firstly we have a primer page. If you've touched any of the core front-end code you'll need to check if the primer is rendering @@ -55,17 +55,17 @@ https://developer.mozilla.org/en-US/docs/JavaScript/Reference/Reserved_Words :: - /* These are bad */ - var a = { - default: 1, - delete: function() {} - }; + /* These are bad */ + var a = { + default: 1, + delete: function() {} + }; - /* These are good */ - var a = { - default_value: 1, - remove: function() {} - }; + /* These are good */ + var a = { + default_value: 1, + remove: function() {} + }; Unclosed JS arrays / objects ---------------------------- @@ -75,14 +75,14 @@ and arrays. For example: :: - /* These are bad */ - var a = { - b: 'c', - }; - var a = ['b', 'c', ]; - - /* These are good */ - var a = { - c: 'c' - }; - var a = ['b', 'c']; + /* These are bad */ + var a = { + b: 'c', + }; + var a = ['b', 'c', ]; + + /* These are good */ + var a = { + c: 'c' + }; + var a = ['b', 'c']; From 77f8224a16bf861a058ee81302f8df6f5a39c448 Mon Sep 17 00:00:00 2001 From: Sean Hammond Date: Sun, 25 Nov 2012 12:51:26 +0100 Subject: [PATCH 05/27] Clarify single-quotes rule in Python coding standards --- doc/python-coding-standards.rst | 12 ++++++++++-- 1 file changed, 10 insertions(+), 2 deletions(-) diff --git a/doc/python-coding-standards.rst b/doc/python-coding-standards.rst index 025160b9373..684cc581c08 100644 --- a/doc/python-coding-standards.rst +++ b/doc/python-coding-standards.rst @@ -23,8 +23,16 @@ avoid spurious merge conflicts, and aid in reading pull requests. Use Single Quotes ----------------- -Use the single-quote character, ``'``, rather than the double-quote character, -``"``, for string literals. +Use single-quotes for string literals, e.g. ``'my-identifier'``, *but* use +double-quotes for strings that are likely to contain single-quote characters as +part of the string itself (such as error messages, or any strings containing +natural language), e.g. ``"You've got an error!"``. + +Single-quotes are easier to read and to type, but if a string contains +single-quote characters then double-quotes are better than escaping the +single-quote characters or wrapping the string in double single-quotes. + +We also use triple single-quotes for docstrings, see `Docstrings`_. Imports From 24b94da868b702d15acfc55fbf11861387223e2f Mon Sep 17 00:00:00 2001 From: Sean Hammond Date: Sun, 25 Nov 2012 13:07:01 +0100 Subject: [PATCH 06/27] Add CHANGELOG and docs to pull review process --- doc/contributing.rst | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/doc/contributing.rst b/doc/contributing.rst index cc9faa2483d..c22a9e8a5c5 100644 --- a/doc/contributing.rst +++ b/doc/contributing.rst @@ -109,6 +109,11 @@ branch and it will become part of CKAN! see `Feature Branches`_. - Your branch should contain new or changed tests for any new or changed code, see :ref:`Testing`. + - Your branch should contain updates to the + `CHANGELOG file `_ + briefly summarising your code changes. + - Your branch should contain new or updated documentation for any new or + updated code, see :doc:`contributing-docs`. - Your branch should be up to date with the master branch of the central CKAN repo, see `Keeping Up with master`_. - All the CKAN tests should pass on your branch, see :doc:`test`. From 7f630a9c497a62e991bc237c42570abad77b171b Mon Sep 17 00:00:00 2001 From: Sean Hammond Date: Mon, 26 Nov 2012 15:10:51 +0100 Subject: [PATCH 07/27] Move URL /user/dashboard to /dashboard We decided this makes more sense --- ckan/config/routing.py | 2 +- ckan/templates/user/dashboard.html | 1 - ckan/tests/functional/test_user.py | 4 ++-- 3 files changed, 3 insertions(+), 4 deletions(-) diff --git a/ckan/config/routing.py b/ckan/config/routing.py index 44df60358c0..80b36ac86dc 100644 --- a/ckan/config/routing.py +++ b/ckan/config/routing.py @@ -265,7 +265,7 @@ def make_map(): # Note: openid users have slashes in their ids, so need the wildcard # in the route. m.connect('/user/activity/{id}', action='activity') - m.connect('/user/dashboard', action='dashboard') + m.connect('/dashboard', action='dashboard') m.connect('/user/follow/{id}', action='follow') m.connect('/user/unfollow/{id}', action='unfollow') m.connect('/user/followers/{id:.*}', action='followers') diff --git a/ckan/templates/user/dashboard.html b/ckan/templates/user/dashboard.html index d50c1cf1797..2beaaaddea9 100644 --- a/ckan/templates/user/dashboard.html +++ b/ckan/templates/user/dashboard.html @@ -5,7 +5,6 @@ {% block subtitle %}{{ _('Dashboard') }}{% endblock %} {% block breadcrumb_content %} -
  • {% link_for _('Users'), controller='user', action='index' %}
  • {{ _('Dashboard') }}
    • {% endblock %} diff --git a/ckan/tests/functional/test_user.py b/ckan/tests/functional/test_user.py index ee8db1350b0..1c4a7614012 100644 --- a/ckan/tests/functional/test_user.py +++ b/ckan/tests/functional/test_user.py @@ -187,8 +187,8 @@ def test_login(self): # then get redirected to user's dashboard res = res.follow() assert_equal(res.status, 302) - assert res.header('Location').startswith('http://localhost/user/dashboard') or \ - res.header('Location').startswith('/user/dashboard') + assert res.header('Location').startswith('http://localhost/dashboard') or \ + res.header('Location').startswith('/dashboard') res = res.follow() assert_equal(res.status, 200) assert 'testlogin is now logged in' in res.body From 9bc76a0eed3f0318a85ec8771b6224e8ddb4aeb7 Mon Sep 17 00:00:00 2001 From: Sean Hammond Date: Mon, 26 Nov 2012 22:27:56 +0100 Subject: [PATCH 08/27] Add activity stream tab to dataset page --- ckan/config/routing.py | 1 + ckan/controllers/package.py | 27 ++++++++++++++++++++------- ckan/templates/package/activity.html | 10 ++++++++++ ckan/templates/package/read.html | 3 +++ 4 files changed, 34 insertions(+), 7 deletions(-) create mode 100644 ckan/templates/package/activity.html diff --git a/ckan/config/routing.py b/ckan/config/routing.py index 80b36ac86dc..b3760c8334f 100644 --- a/ckan/config/routing.py +++ b/ckan/config/routing.py @@ -194,6 +194,7 @@ def make_map(): 'history', 'read_ajax', 'history_ajax', + 'activity', 'followers', 'follow', 'unfollow', diff --git a/ckan/controllers/package.py b/ckan/controllers/package.py index c67da39f16a..922178de975 100644 --- a/ckan/controllers/package.py +++ b/ckan/controllers/package.py @@ -319,13 +319,6 @@ def read(self, id, format='html'): c.current_package_id = c.pkg.id c.related_count = c.pkg.related_count - # Add the package's activity stream (already rendered to HTML) to the - # template context for the package/read.html template to retrieve - # later. - c.package_activity_stream = \ - get_action('package_activity_list_html')( - context, {'id': c.current_package_id}) - PackageSaver().render_package(c.pkg_dict, context) template = self._read_template(package_type) @@ -1241,6 +1234,26 @@ def followers(self, id=None): return render('package/followers.html') + def activity(self, id): + '''Render this package's public activity stream page.''' + + context = {'model': model, 'session': model.Session, + 'user': c.user or c.author, 'for_view': True} + data_dict = {'id': id} + try: + c.pkg_dict = get_action('package_show')(context, data_dict) + c.pkg = context['package'] + c.package_activity_stream = get_action( + 'package_activity_list_html')(context, + {'id': c.pkg_dict['id']}) + c.related_count = c.pkg.related_count + except NotFound: + abort(404, _('Dataset not found')) + except NotAuthorized: + abort(401, _('Unauthorized to read dataset %s') % id) + + return render('package/activity.html') + def resource_embedded_dataviewer(self, id, resource_id, width=500, height=500): """ diff --git a/ckan/templates/package/activity.html b/ckan/templates/package/activity.html new file mode 100644 index 00000000000..ee0e2dd7fc0 --- /dev/null +++ b/ckan/templates/package/activity.html @@ -0,0 +1,10 @@ +{% extends "package/read.html" %} + +{% block subtitle %}{{ _('Activity Stream') }} - {{ c.pkg_dict.title or c.pkg_dict.name }}{% endblock %} + +{% block package_content %} +
    +

    {{ _('Activity Stream') }}

    + {{ c.package_activity_stream | safe }} +
    +{% endblock %} diff --git a/ckan/templates/package/read.html b/ckan/templates/package/read.html index 886f63ef348..35fc36b5125 100644 --- a/ckan/templates/package/read.html +++ b/ckan/templates/package/read.html @@ -45,6 +45,9 @@ {% link_for _('Dataset'), controller='package', action='read', id=pkg.name, icon='sitemap' %} + + {% link_for _('Activity Stream'), controller='package', action='activity', id=pkg.name, icon='time' %} + {% link_for _('Followers'), controller='package', action='followers', id=pkg.name, icon='group' %} From 0594077936740949f9875766f14365bc0363fd6e Mon Sep 17 00:00:00 2001 From: Sean Hammond Date: Tue, 27 Nov 2012 14:51:50 +0100 Subject: [PATCH 09/27] Fix some PEP8 issues --- ckan/tests/functional/api/__init__.py | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/ckan/tests/functional/api/__init__.py b/ckan/tests/functional/api/__init__.py index 1c6e2442feb..0faff77c638 100644 --- a/ckan/tests/functional/api/__init__.py +++ b/ckan/tests/functional/api/__init__.py @@ -1,6 +1,7 @@ from nose.tools import assert_equal import copy + def change_lists_to_sets(iterable): '''recursive method to drill down into iterables to convert any list or tuples into sets. Does not work @@ -25,9 +26,10 @@ def change_lists_to_sets(iterable): else: raise NotImplementedError + def assert_dicts_equal_ignoring_ordering(dict1, dict2): '''Asserts dicts are equal, assuming that the ordering of - any lists is unimportant.''' + any lists is unimportant.''' dicts = [copy.deepcopy(dict1), copy.deepcopy(dict2)] for d in dicts: d = change_lists_to_sets(d) From b1f46c40298e741c3d6c2b01b14ef7eabc010192 Mon Sep 17 00:00:00 2001 From: Sean Hammond Date: Tue, 27 Nov 2012 14:59:49 +0100 Subject: [PATCH 10/27] Tidy a couple of docstrings --- ckan/tests/functional/api/__init__.py | 18 +++++++++++++----- 1 file changed, 13 insertions(+), 5 deletions(-) diff --git a/ckan/tests/functional/api/__init__.py b/ckan/tests/functional/api/__init__.py index 0faff77c638..7db7ba168c8 100644 --- a/ckan/tests/functional/api/__init__.py +++ b/ckan/tests/functional/api/__init__.py @@ -3,9 +3,14 @@ def change_lists_to_sets(iterable): - '''recursive method to drill down into iterables to - convert any list or tuples into sets. Does not work - though for dictionaries in lists.''' + '''Convert any lists or tuples in `iterable` into sets. + + Recursively drill down into iterable and convert any list or tuples to + sets. + + Does not work for dictionaries in lists. + + ''' if isinstance(iterable, dict): for key in iterable: if isinstance(iterable[key], (list, tuple)): @@ -28,8 +33,11 @@ def change_lists_to_sets(iterable): def assert_dicts_equal_ignoring_ordering(dict1, dict2): - '''Asserts dicts are equal, assuming that the ordering of - any lists is unimportant.''' + '''Assert that dict1 and dict2 are equal. + + Assumes that the ordering of any lists in the dicts is unimportant. + + ''' dicts = [copy.deepcopy(dict1), copy.deepcopy(dict2)] for d in dicts: d = change_lists_to_sets(d) From 3b84c92210aa3aaf761d3f03bea6a94f0f929038 Mon Sep 17 00:00:00 2001 From: Sean Hammond Date: Tue, 27 Nov 2012 15:00:20 +0100 Subject: [PATCH 11/27] Delete a commented-out set_trace() --- ckan/tests/functional/api/__init__.py | 1 - 1 file changed, 1 deletion(-) diff --git a/ckan/tests/functional/api/__init__.py b/ckan/tests/functional/api/__init__.py index 7db7ba168c8..08c9762d91d 100644 --- a/ckan/tests/functional/api/__init__.py +++ b/ckan/tests/functional/api/__init__.py @@ -41,5 +41,4 @@ def assert_dicts_equal_ignoring_ordering(dict1, dict2): dicts = [copy.deepcopy(dict1), copy.deepcopy(dict2)] for d in dicts: d = change_lists_to_sets(d) - #from nose.tools import set_trace; set_trace() assert_equal(dicts[0], dicts[1]) From eb55a659db75a21803a0a9bf66d50e6918152c87 Mon Sep 17 00:00:00 2001 From: Sean Hammond Date: Wed, 21 Nov 2012 18:32:57 +0100 Subject: [PATCH 12/27] Refactor dashboard activity stream tests - Move tests for contents of dashboard activity stream out of test_activity.py, this test module was way too long and confusing, leave it for testing the public activity streams only. Add a docstring to the module saying so. - Add a docstring to test_follow.py explaining that it tests the follower functions only (follow, unfollow, etc.) and not the contents of the dashboard activity stream that is generated from what you're folllowing. - Add new tests for the contents of the dashboard activity stream in test_dashboard.py along with other dashboard tests. Currently some of these tests are failing because activities from followed groups are not appearing in the dashboard. --- ckan/tests/functional/api/test_activity.py | 164 ++------------------ ckan/tests/functional/api/test_dashboard.py | 142 +++++++++++++++-- ckan/tests/functional/api/test_follow.py | 12 ++ 3 files changed, 148 insertions(+), 170 deletions(-) diff --git a/ckan/tests/functional/api/test_activity.py b/ckan/tests/functional/api/test_activity.py index 4f7eff2a380..dfc14f9f6c4 100644 --- a/ckan/tests/functional/api/test_activity.py +++ b/ckan/tests/functional/api/test_activity.py @@ -1,3 +1,13 @@ +'''Functional tests for the public activity streams API. + +This module tests the contents of the various public activity streams: +use activity streams, dataset activity streams, group activity streams, etc. + +This module _does not_ test the private user dashboard activity stream (which +is different because the contents depend on what the user is following), that +is tested in test_dashboard.py. + +''' import datetime import logging logger = logging.getLogger(__name__) @@ -163,12 +173,6 @@ def setup_class(self): 'apikey': normal_user.apikey, 'name': normal_user.name, } - follower = model.User.get('tester') - self.follower = { - 'id': follower.id, - 'apikey': follower.apikey, - 'name': follower.name, - } warandpeace = model.Package.get('warandpeace') self.warandpeace = { 'id': warandpeace.id, @@ -177,54 +181,14 @@ def setup_class(self): self.annakarenina = { 'id': annakarenina.id, } - self.users = [self.sysadmin_user, self.normal_user, self.follower] + self.users = [self.sysadmin_user, self.normal_user] self.app = paste.fixture.TestApp(pylons.test.pylonsapp) - # Make follower follow everything else. - params = {'id': 'testsysadmin'} - extra_environ = {'Authorization': str(self.follower['apikey'])} - response = self.app.post('/api/action/follow_user', - params=json.dumps(params), extra_environ=extra_environ).json - assert response['success'] is True - params = {'id': 'annafan'} - extra_environ = {'Authorization': str(self.follower['apikey'])} - response = self.app.post('/api/action/follow_user', - params=json.dumps(params), extra_environ=extra_environ).json - assert response['success'] is True - params = {'id': 'warandpeace'} - extra_environ = {'Authorization': str(self.follower['apikey'])} - response = self.app.post('/api/action/follow_dataset', - params=json.dumps(params), extra_environ=extra_environ).json - assert response['success'] is True - params = {'id': 'annakarenina'} - extra_environ = {'Authorization': str(self.follower['apikey'])} - response = self.app.post('/api/action/follow_dataset', - params=json.dumps(params), extra_environ=extra_environ).json - assert response['success'] is True - - self.followees = \ - [ - self.sysadmin_user['id'], - self.normal_user['id'], - self.follower['id'], - self.warandpeace['id'], - self.annakarenina['id'] - ] - @classmethod def teardown_class(self): import ckan.model as model model.repo.rebuild_db() - def dashboard_activity_stream(self, apikey): - - response = self.app.get("/api/action/dashboard_activity_list", - json.dumps({}), - extra_environ={'Authorization': str(apikey)}) - response_dict = json.loads(response.body) - assert response_dict['success'] is True - return response_dict['result'] - def user_activity_stream(self, user_id, apikey=None): if apikey: extra_environ = {'Authorization': str(apikey)} @@ -290,28 +254,9 @@ def record_details(self, user_id, package_id=None, group_ids=None, details['recently changed datasets stream'] = \ self.recently_changed_datasets_stream(apikey) - details['user dashboard activity stream'] = ( - self.dashboard_activity_stream(apikey)) - - details['follower dashboard activity stream'] = ( - self.dashboard_activity_stream(self.follower['apikey'])) - details['time'] = datetime.datetime.now() return details - def check_dashboards(self, before, after, activity): - new_activities = [activity_ for activity_ in - after['user dashboard activity stream'] - if activity_ not in before['user dashboard activity stream']] - assert [activity['id'] for activity in new_activities] == [ - activity['id']] - - new_activities = [activity_ for activity_ in - after['follower dashboard activity stream'] - if activity_ not in before['follower dashboard activity stream']] - assert [activity['id'] for activity in new_activities] == [ - activity['id']] - def _create_package(self, user, name=None): if user: user_id = user['id'] @@ -358,31 +303,6 @@ def _create_package(self, user, name=None): after['recently changed datasets stream']) assert new_rcd_activities == [activity] - # The new activity should appear in the user's dashboard activity - # stream. - new_activities = [activity_ for activity_ in - after['user dashboard activity stream'] - if activity_ not in before['user dashboard activity stream']] - # There will be other new activities besides the 'follow dataset' one - # because all the dataset's old activities appear in the user's - # dashboard when she starts to follow the dataset. - assert activity['id'] in [ - activity['id'] for activity in new_activities] - - # The new activity should appear in the user "follower"'s dashboard - # activity stream because she follows all the other users and datasets. - new_activities = [activity_ for activity_ in - after['follower dashboard activity stream'] - if activity_ not in before['follower dashboard activity stream']] - # There will be other new activities besides the 'follow dataset' one - # because all the dataset's old activities appear in the user's - # dashboard when she starts to follow the dataset. - assert [activity['id'] for activity in new_activities] == [ - activity['id']] - - # The same new activity should appear on the dashboard's of the user's - # followers. - # The same new activity should appear in the activity streams of the # package's groups. for group_dict in package_created['groups']: @@ -497,8 +417,6 @@ def _add_resource(self, package, user): after['group activity streams'][group_dict['name']]) assert grp_new_activities == [activity] - self.check_dashboards(before, after, activity) - # Check that the new activity has the right attributes. assert activity['object_id'] == updated_package['id'], \ str(activity['object_id']) @@ -589,8 +507,6 @@ def _delete_extra(self, package_dict, user): after['group activity streams'][group_dict['name']]) assert grp_new_activities == [activity] - self.check_dashboards(before, after, activity) - # Check that the new activity has the right attributes. assert activity['object_id'] == updated_package['id'], \ str(activity['object_id']) @@ -679,8 +595,6 @@ def _update_extra(self, package_dict, user): after['recently changed datasets stream']) \ == user_new_activities - self.check_dashboards(before, after, activity) - # If the package has any groups, the same new activity should appear # in the activity stream of each group. for group_dict in package_dict['groups']: @@ -783,8 +697,6 @@ def _add_extra(self, package_dict, user, key=None): after['group activity streams'][group_dict['name']]) assert grp_new_activities == [activity] - self.check_dashboards(before, after, activity) - # Check that the new activity has the right attributes. assert activity['object_id'] == updated_package['id'], \ str(activity['object_id']) @@ -845,8 +757,6 @@ def _create_activity(self, user, package, params): after['package activity stream'])) assert pkg_new_activities == user_new_activities - self.check_dashboards(before, after, activity) - # Check that the new activity has the right attributes. assert activity['object_id'] == params['object_id'], ( str(activity['object_id'])) @@ -892,8 +802,6 @@ def _delete_group(self, group, user): new_activities, ("The same activity should also " "appear in the group's activity stream.") - self.check_dashboards(before, after, activity) - # Check that the new activity has the right attributes. assert activity['object_id'] == group['id'], str(activity['object_id']) assert activity['user_id'] == user['id'], str(activity['user_id']) @@ -937,8 +845,6 @@ def _update_group(self, group, user): new_activities, ("The same activity should also " "appear in the group's activity stream.") - self.check_dashboards(before, after, activity) - # Check that the new activity has the right attributes. assert activity['object_id'] == group['id'], str(activity['object_id']) assert activity['user_id'] == user['id'], str(activity['user_id']) @@ -986,8 +892,6 @@ def _update_user(self, user): "the user's activity stream, but found %i" % len(new_activities)) activity = new_activities[0] - self.check_dashboards(before, after, activity) - # Check that the new activity has the right attributes. assert activity['object_id'] == user_dict['id'], ( str(activity['object_id'])) @@ -1054,8 +958,6 @@ def _delete_resources(self, package): after['group activity streams'][group_dict['name']]) assert grp_new_activities == [activity] - self.check_dashboards(before, after, activity) - # Check that the new activity has the right attributes. assert activity['object_id'] == package['id'], ( str(activity['object_id'])) @@ -1132,8 +1034,6 @@ def _update_package(self, package, user): after['recently changed datasets stream']) \ == user_new_activities - self.check_dashboards(before, after, activity) - # If the package has any groups, the same new activity should appear # in the activity stream of each group. for group_dict in package['groups']: @@ -1211,8 +1111,6 @@ def _update_resource(self, package, resource, user): after['recently changed datasets stream']) \ == user_new_activities - self.check_dashboards(before, after, activity) - # If the package has any groups, the same new activity should appear # in the activity stream of each group. for group_dict in package['groups']: @@ -1289,8 +1187,6 @@ def _delete_package(self, package): after['recently changed datasets stream']) \ == user_new_activities - self.check_dashboards(before, after, activity) - # If the package has any groups, the same new activity should appear # in the activity stream of each group. for group_dict in package['groups']: @@ -1406,8 +1302,6 @@ def test_01_remove_tag(self): after['recently changed datasets stream']) \ == user_new_activities - self.check_dashboards(before, after, activity) - # If the package has any groups, the same new activity should appear # in the activity stream of each group. for group_dict in pkg_dict['groups']: @@ -1618,8 +1512,6 @@ def test_create_group(self): new_activities, ("The same activity should also appear in " "the group's activity stream.") - self.check_dashboards(before, after, activity) - # Check that the new activity has the right attributes. assert activity['object_id'] == group_created['id'], \ str(activity['object_id']) @@ -1692,8 +1584,6 @@ def test_add_tag(self): after['recently changed datasets stream']) \ == user_new_activities - self.check_dashboards(before, after, activity) - # If the package has any groups, the same new activity should appear # in the activity stream of each group. for group_dict in pkg_dict['groups']: @@ -2119,28 +2009,6 @@ def test_follow_dataset(self): for activity in user_new_activities: assert activity in pkg_new_activities - # The new activity should appear in the user's dashboard activity - # stream. - new_activities = [activity_ for activity_ in - after['user dashboard activity stream'] - if activity_ not in before['user dashboard activity stream']] - # There will be other new activities besides the 'follow dataset' one - # because all the dataset's old activities appear in the user's - # dashboard when she starts to follow the dataset. - assert activity['id'] in [ - activity['id'] for activity in new_activities] - - # The new activity should appear in the user "follower"'s dashboard - # activity stream because she follows all the other users and datasets. - new_activities = [activity_ for activity_ in - after['follower dashboard activity stream'] - if activity_ not in before['follower dashboard activity stream']] - # There will be other new activities besides the 'follow dataset' one - # because all the dataset's old activities appear in the user's - # dashboard when she starts to follow the dataset. - assert [activity['id'] for activity in new_activities] == [ - activity['id']] - # Check that the new activity has the right attributes. assert activity['object_id'] == self.warandpeace['id'], \ str(activity['object_id']) @@ -2182,16 +2050,6 @@ def test_follow_user(self): len(user_new_activities)) activity = user_new_activities[0] - # Check that the new activity appears in the user's private activity - # stream. - user_new_activities = (find_new_activities( - before['follower dashboard activity stream'], - after['follower dashboard activity stream'])) - assert len(user_new_activities) == 1, ("There should be 1 new " - " activity in the user's activity stream, but found %i" % - len(user_new_activities)) - assert user_new_activities[0]['id'] == activity['id'] - # Check that the new activity has the right attributes. assert activity['object_id'] == self.sysadmin_user['id'], \ str(activity['object_id']) diff --git a/ckan/tests/functional/api/test_dashboard.py b/ckan/tests/functional/api/test_dashboard.py index 1d7c291c200..6cef63beb21 100644 --- a/ckan/tests/functional/api/test_dashboard.py +++ b/ckan/tests/functional/api/test_dashboard.py @@ -1,3 +1,10 @@ +'''Test for the dashboard API. + +This module tests the various functions of the user dashboard, such as the +contents of the dashboard activity stream and reporting the number of new +activities. + +''' import ckan from ckan.lib.helpers import json import paste @@ -92,6 +99,16 @@ def test_00_dashboard_new_activities_count_not_logged_in(self): def test_00_dashboard_mark_new_activities_not_logged_in(self): self.post('dashboard_mark_all_new_activities_as_old', status=403) + def test_01_dashboard_activity_list_for_new_user(self): + '''Test the contents of a new user's dashboard activity stream.''' + activities = self.dashboard_activity_list(self.new_user) + # We expect to find a single 'new user' activity. + assert len(activities) == 1 + activity = activities[0] + assert activity['activity_type'] == 'new user' + assert activity['user_id'] == activity['object_id'] + assert activity['user_id'] == self.new_user['id'] + def test_01_new_activities_count_for_new_user(self): '''Test that a newly registered user's new activities count is 0.''' assert self.dashboard_new_activities_count(self.new_user) == 0 @@ -151,14 +168,38 @@ def test_02_own_activities_do_not_count_as_new(self): # User's own actions should not increase her activity count. assert self.dashboard_new_activities_count(self.new_user) == 0 + def test_03_dashboard_activity_list_own_activities(self): + '''Test that a user's own activities appear in her dashboard.''' + activities = self.dashboard_activity_list(self.new_user) + + # FIXME: There should actually be 6 activities here, but when you + # follow something it's old activities (from before you followed it) + # appear in your activity stream. So here we get one more activity + # than expected. + assert len(activities) == 7 + + assert activities[0]['activity_type'] == 'changed package' + assert activities[1]['activity_type'] == 'follow group' + assert activities[2]['activity_type'] == 'follow user' + assert activities[3]['activity_type'] == 'follow dataset' + assert activities[4]['activity_type'] == 'new package' + assert activities[5]['activity_type'] == 'new user' + + # FIXME: Shouldn't need the [:6] here, it's because when you follow + # something its old activities (from before you started following it) + # appear in your dashboard. + for activity in activities[:6]: + assert activity['user_id'] == self.new_user['id'] + def test_03_own_activities_not_marked_as_new(self): '''Make a user do some activities and check that her own activities aren't marked as new in her dashboard activity stream.''' assert len(self.dashboard_new_activities(self.new_user)) == 0 - def test_04_new_activities_count(self): - '''Test that new activities from objects that a user follows increase - her new activities count.''' + def test_04_activities_from_followed_datasets(self): + '''Activities from followed datasets should show in dashboard.''' + + activities_before = self.dashboard_activity_list(self.new_user) # Make someone else who new_user is not following update a dataset that # new_user is following. @@ -167,34 +208,101 @@ def test_04_new_activities_count(self): extra_environ={'Authorization': str(self.joeadmin['apikey'])}) assert response.json['success'] is True + # Check the new activity in new_user's dashboard. + activities = self.dashboard_activity_list(self.new_user) + new_activities = [activity for activity in activities + if activity not in activities_before] + assert len(new_activities) == 1 + activity = new_activities[0] + assert activity['activity_type'] == 'changed package' + assert activity['user_id'] == self.joeadmin['id'] + assert activity['data']['package']['name'] == 'warandpeace' + + def test_04_activities_from_followed_users(self): + '''Activities from followed users should show in the dashboard.''' + + activities_before = self.dashboard_activity_list(self.new_user) + # Make someone that the user is following create a new dataset. params = json.dumps({'name': 'annas_new_dataset'}) response = self.app.post('/api/action/package_create', params=params, extra_environ={'Authorization': str(self.annafan['apikey'])}) assert response.json['success'] is True - # Make someone that the user is not following update a dataset that - # the user is not following, but that belongs to a group that the user - # is following. + # Check the new activity in new_user's dashboard. + activities = self.dashboard_activity_list(self.new_user) + new_activities = [activity for activity in activities + if activity not in activities_before] + assert len(new_activities) == 1 + activity = new_activities[0] + assert activity['activity_type'] == 'new package' + assert activity['user_id'] == self.annafan['id'] + assert activity['data']['package']['name'] == 'annas_new_dataset' + + def test_04_activities_from_followed_groups(self): + '''Activities from followed groups should show in the dashboard.''' + + activities_before = self.dashboard_activity_list(self.new_user) + + # Make someone that the user is not following update a group that the + # user is following. + params = json.dumps({'id': 'roger', 'description': 'updated'}) + response = self.app.post('/api/action/group_update', params=params, + extra_environ={'Authorization': str(self.testsysadmin['apikey'])}) + assert response.json['success'] is True + + # Check the new activity in new_user's dashboard. + activities = self.dashboard_activity_list(self.new_user) + new_activities = [activity for activity in activities + if activity not in activities_before] + assert len(new_activities) == 1 + activity = new_activities[0] + assert activity['activity_type'] == 'changed group' + assert activity['user_id'] == self.testsysadmin['id'] + assert activity['data']['group']['name'] == 'roger' + + def test_04_activities_from_datasets_of_followed_groups(self): + '''Activities from datasets of followed groups should show in the + dashboard. + + ''' + activities_before = self.dashboard_activity_list(self.new_user) + + # Make someone that the user is not following update a dataset that the + # user is not following either, but that belongs to a group that the + # user is following. params = json.dumps({'name': 'annakarenina', 'notes': 'updated'}) response = self.app.post('/api/action/package_update', params=params, extra_environ={'Authorization': str(self.testsysadmin['apikey'])}) assert response.json['success'] is True - # FIXME: The number here should be 3 but activities from followed - # groups are not appearing in dashboard. When that is fixed, fix this - # number. - assert self.dashboard_new_activities_count(self.new_user) == 2 + # Check the new activity in new_user's dashboard. + activities = self.dashboard_activity_list(self.new_user) + new_activities = [activity for activity in activities + if activity not in activities_before] + assert len(new_activities) == 1 + activity = new_activities[0] + assert activity['activity_type'] == 'changed package' + assert activity['user_id'] == self.testsysadmin['id'] + assert activity['data']['package']['name'] == 'annakarenina' + + def test_05_new_activities_count(self): + '''Test that new activities from objects that a user follows increase + her new activities count.''' + # FIXME: The number here should be 4 but activities from datasets of + # followed groups are not appearing in dashboard. When that is fixed, + # fix this number. + assert self.dashboard_new_activities_count(self.new_user) == 3 - def test_05_activities_marked_as_new(self): + def test_06_activities_marked_as_new(self): '''Test that new activities from objects that a user follows are marked as new in her dashboard activity stream.''' - # FIXME: The number here should be 3 but activities from followed - # groups are not appearing in dashboard. When that is fixed, fix this - # number. - assert len(self.dashboard_new_activities(self.new_user)) == 2 + # FIXME: The number here should be 4 but activities from datasets of + # followed groups are not appearing in dashboard. When that is fixed, + # fix this number. + assert len(self.dashboard_new_activities(self.new_user)) == 3 - def test_06_mark_new_activities_as_read(self): + def test_07_mark_new_activities_as_read(self): '''Test that a user's new activities are marked as old when she views her dashboard activity stream.''' assert self.dashboard_new_activities_count(self.new_user) > 0 @@ -203,7 +311,7 @@ def test_06_mark_new_activities_as_read(self): assert self.dashboard_new_activities_count(self.new_user) == 0 assert len(self.dashboard_new_activities(self.new_user)) == 0 - def test_07_maximum_number_of_new_activities(self): + def test_08_maximum_number_of_new_activities(self): '''Test that the new activities count does not go higher than 15, even if there are more than 15 new activities from the user's followers.''' for n in range(0, 20): diff --git a/ckan/tests/functional/api/test_follow.py b/ckan/tests/functional/api/test_follow.py index ab0df832f48..a084c930fd9 100644 --- a/ckan/tests/functional/api/test_follow.py +++ b/ckan/tests/functional/api/test_follow.py @@ -1,3 +1,15 @@ +'''Test for the follower API. + +This module tests following, unfollowing, getting a list of what you're +following or the number of things you're following, getting a list of who's +following you or the number of followers you have, testing whether or not +you're following something, etc. + +This module _does not_ test the user dashboard activity stream (which shows +activities from everything you're following), that is tested in +test_dashboard.py. + +''' import datetime import paste import pylons.test From 0235f1a812cd879c3f540807afb6a462f3ccecf3 Mon Sep 17 00:00:00 2001 From: Sean Hammond Date: Wed, 21 Nov 2012 18:34:39 +0100 Subject: [PATCH 13/27] Add activities from followed groups to dashboard activity stream Add activities from groups that a user is following (e.g. when someone updates a group) to the user's dashboard activity stream. There are still some test_dashboard.py tests failing because activities from datasets belonging to followed groups dob't appear in the dashboard yet. --- ckan/model/activity.py | 10 ++++++++++ 1 file changed, 10 insertions(+) diff --git a/ckan/model/activity.py b/ckan/model/activity.py index 9181acaa20d..f0f706a8df3 100644 --- a/ckan/model/activity.py +++ b/ckan/model/activity.py @@ -151,9 +151,19 @@ def _activities_from_datasets_followed_by_user_query(user_id): return q +def _activities_from_groups_followed_by_user_query(user_id): + import ckan.model as model + q = model.Session.query(model.Activity) + q = q.join(model.UserFollowingGroup, + model.UserFollowingGroup.object_id == model.Activity.object_id) + q = q.filter(model.UserFollowingGroup.follower_id == user_id) + return q + + def _activities_from_everything_followed_by_user_query(user_id): q = _activites_from_users_followed_by_user_query(user_id) q = q.union(_activities_from_datasets_followed_by_user_query(user_id)) + q = q.union(_activities_from_groups_followed_by_user_query(user_id)) return q From 5847bd00065b4fc92121432d57a002fc1fd688dd Mon Sep 17 00:00:00 2001 From: Sean Hammond Date: Wed, 21 Nov 2012 20:24:29 +0100 Subject: [PATCH 14/27] Move group_package_show SQLAlchemy into model Move the SQLAlchemy query that the group_package_show() action function uses into the model. I need this for architectural reasons for upcoming commits, and we're supposed to encapsulate SQLAlchemy in the model anyway. I made the new model function support the 'return_query' option but note that there are no tests covering this, all the tests pass even without this option. --- ckan/logic/action/get.py | 28 ++++++++-------------------- ckan/model/group.py | 27 +++++++++++++++++++++++++++ 2 files changed, 35 insertions(+), 20 deletions(-) diff --git a/ckan/logic/action/get.py b/ckan/logic/action/get.py index 146cd899b86..8bfb4f5095d 100644 --- a/ckan/logic/action/get.py +++ b/ckan/logic/action/get.py @@ -750,34 +750,22 @@ def group_package_show(context, data_dict): :rtype: list of dictionaries ''' - model = context["model"] - user = context["user"] - id = _get_or_bust(data_dict, 'id') - limit = data_dict.get("limit") + model = context['model'] + group_id = _get_or_bust(data_dict, 'id') - group = model.Group.get(id) + # FIXME: What if limit is not an int? Schema and validation needed. + limit = data_dict.get('limit') + + group = model.Group.get(group_id) context['group'] = group if group is None: raise NotFound _check_access('group_show', context, data_dict) - query = model.Session.query(model.PackageRevision)\ - .filter(model.PackageRevision.state=='active')\ - .filter(model.PackageRevision.current==True)\ - .join(model.Member, model.Member.table_id==model.PackageRevision.id)\ - .join(model.Group, model.Group.id==model.Member.group_id)\ - .filter_by(id=group.id)\ - .order_by(model.PackageRevision.name) - - if limit: - query = query.limit(limit) - - if context.get('return_query'): - return query - result = [] - for pkg_rev in query.all(): + for pkg_rev in group.get_package_revisions(limit=limit, + return_query=context.get('return_query')): result.append(model_dictize.package_dictize(pkg_rev, context)) return result diff --git a/ckan/model/group.py b/ckan/model/group.py index 62767671538..b0227c05203 100644 --- a/ckan/model/group.py +++ b/ckan/model/group.py @@ -199,6 +199,33 @@ def active_packages(self, load_eager=True, with_private=False): return query + def get_package_revisions(self, limit=None, return_query=False): + '''Return a group's packages. + + :param limit: the maximum number of packages to return + :type limit: int + + :returns: a list of the group's packages, sorted by name + :rtype: list of PackageRevision objects + + ''' + import ckan.model as model + q = model.Session.query(model.PackageRevision) + q = q.filter(model.PackageRevision.state == 'active') + q = q.filter(model.PackageRevision.current == True) + q = q.join(model.Member, + model.Member.table_id == model.PackageRevision.id) + q = q.join(model.Group, model.Group.id == model.Member.group_id) + q = q.filter_by(id=self.id) + q = q.order_by(model.PackageRevision.name) + if limit is not None: + q = q.limit(limit) + if return_query: + return q + else: + return q.all() + + @classmethod def search_by_name_or_title(cls, text_query, group_type=None): text_query = text_query.strip().lower() From 8f5c897c06633b1f2e7cb7d7cd78573fac5fb57d Mon Sep 17 00:00:00 2001 From: Sean Hammond Date: Wed, 21 Nov 2012 21:04:02 +0100 Subject: [PATCH 15/27] Remove group.members_of_type() Remove group.members_of_type() and related code. It isn't used anywhere except in the old publisher and organizations extensions which are to be replaced by a new organizations implementation in CKAN core. Also it duplicates functionality provided by group.active_packages(). --- ckan/model/group.py | 42 +----------------------------------------- 1 file changed, 1 insertion(+), 41 deletions(-) diff --git a/ckan/model/group.py b/ckan/model/group.py index b0227c05203..5692e48e0cc 100644 --- a/ckan/model/group.py +++ b/ckan/model/group.py @@ -143,35 +143,6 @@ def set_approval_status(self, status): if status == "denied": pass - def members_of_type(self, object_type, capacity=None): - from ckan import model - object_type_string = object_type.__name__.lower() - query = meta.Session.query(object_type).\ - filter(model.Group.id == self.id).\ - filter(model.Member.state == 'active').\ - filter(model.Member.table_name == object_type_string) - - if hasattr(object_type, 'state'): - query = query.filter(object_type.state == 'active') - - if capacity: - query = query.filter(model.Member.capacity == capacity) - - query = query.join(model.Member, member_table.c.table_id == - getattr(object_type, 'id')).\ - join(model.Group, group_table.c.id == member_table.c.group_id) - - return query - - def add_child(self, object_instance): - object_type_string = object_instance.__class__.__name__.lower() - if not object_instance in self.members_of_type( - object_instance.__class__).all(): - member = Member(group=self, - table_id=getattr(object_instance, 'id'), - table_name=object_type_string) - meta.Session.add(member) - 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 @@ -236,23 +207,12 @@ def search_by_name_or_title(cls, text_query, group_type=None): q = q.filter(cls.type == group_type) return q.order_by(cls.title) - def as_dict(self, ref_package_by='name'): - _dict = domain_object.DomainObject.as_dict(self) - _dict['packages'] = [getattr(package, ref_package_by) - for package in self.packages] - _dict['extras'] = dict([(key, value) for key, value - in self.extras.items()]) - if (self.type == 'organization'): - _dict['users'] = [getattr(user, "name") - for user in self.members_of_type(_user.User)] - return _dict - def add_package_by_name(self, package_name): if not package_name: return package = _package.Package.by_name(package_name) assert package - if not package in self.members_of_type(package.__class__).all(): + if not package in self.active_packages().all(): member = Member(group=self, table_id=package.id, table_name='package') meta.Session.add(member) From bdc72d7abd41fbdad5a5a30c8468ce6ea370ebc1 Mon Sep 17 00:00:00 2001 From: Sean Hammond Date: Wed, 21 Nov 2012 23:36:17 +0100 Subject: [PATCH 16/27] Remove duplicate methods to get a group's packages Refactor active_packages() and get_package_revisions(), both of which return a group's packages (but in slightly different ways), replace with just one method packages(). We now have just one way to get a group's packages, the packages() method of the group model. The group_package_show() action function calls it. --- ckan/controllers/group.py | 2 +- ckan/lib/dictization/model_dictize.py | 2 +- ckan/logic/action/get.py | 2 +- ckan/model/group.py | 63 ++++++++++++----------- ckan/tests/functional/test_group.py | 18 +++---- ckan/tests/lib/test_dictization_schema.py | 2 +- ckan/tests/models/test_group.py | 4 +- 7 files changed, 47 insertions(+), 46 deletions(-) diff --git a/ckan/controllers/group.py b/ckan/controllers/group.py index d6bffcc8be0..1867079529b 100644 --- a/ckan/controllers/group.py +++ b/ckan/controllers/group.py @@ -342,7 +342,7 @@ def _force_reindex(self, grp): appearing on the read page for the group (as they're connected via the group name)''' group = model.Group.get(grp['name']) - for dataset in group.active_packages().all(): + for dataset in group.packages(): search.rebuild(dataset.name) def _save_edit(self, id, context): diff --git a/ckan/lib/dictization/model_dictize.py b/ckan/lib/dictization/model_dictize.py index 8a08ff50b03..6926e4a3395 100644 --- a/ckan/lib/dictization/model_dictize.py +++ b/ckan/lib/dictization/model_dictize.py @@ -32,7 +32,7 @@ def group_list_dictize(obj_list, context, group_dict['display_name'] = obj.display_name group_dict['packages'] = \ - len(obj.active_packages(with_private=with_private).all()) + len(obj.packages(with_private=with_private)) if context.get('for_view'): for item in plugins.PluginImplementations( diff --git a/ckan/logic/action/get.py b/ckan/logic/action/get.py index 8bfb4f5095d..f1fc99b05a7 100644 --- a/ckan/logic/action/get.py +++ b/ckan/logic/action/get.py @@ -764,7 +764,7 @@ def group_package_show(context, data_dict): _check_access('group_show', context, data_dict) result = [] - for pkg_rev in group.get_package_revisions(limit=limit, + for pkg_rev in group.packages(limit=limit, return_query=context.get('return_query')): result.append(model_dictize.package_dictize(pkg_rev, context)) diff --git a/ckan/model/group.py b/ckan/model/group.py index 5692e48e0cc..35dfb73b461 100644 --- a/ckan/model/group.py +++ b/ckan/model/group.py @@ -155,47 +155,48 @@ def get_children_groups(self, type='group'): return [{"id":idf, "name": name, "title": title} for idf, name, title in results] - def active_packages(self, load_eager=True, with_private=False): - query = meta.Session.query(_package.Package).\ - filter_by(state=vdm.sqlalchemy.State.ACTIVE).\ - filter(group_table.c.id == self.id).\ - filter(member_table.c.state == 'active') + def packages(self, with_private=False, limit=None, + return_query=False): + '''Return this group's active and pending packages. - if not with_private: - query = query.filter(member_table.c.capacity == 'public') - - query = query.join(member_table, member_table.c.table_id == - _package.Package.id).\ - join(group_table, group_table.c.id == member_table.c.group_id) - - return query + Returns all packages in this group with VDM revision state ACTIVE or + PENDING. - def get_package_revisions(self, limit=None, return_query=False): - '''Return a group's packages. + :param with_private: if True, include the group's private packages + :type with_private: boolean :param limit: the maximum number of packages to return :type limit: int - :returns: a list of the group's packages, sorted by name - :rtype: list of PackageRevision objects + :param return_query: if True, return the SQLAlchemy query object + instead of the list of Packages resulting from the query + :type return_query: boolean + + :returns: a list of this group's packages + :rtype: list of ckan.model.package.Package objects ''' - import ckan.model as model - q = model.Session.query(model.PackageRevision) - q = q.filter(model.PackageRevision.state == 'active') - q = q.filter(model.PackageRevision.current == True) - q = q.join(model.Member, - model.Member.table_id == model.PackageRevision.id) - q = q.join(model.Group, model.Group.id == model.Member.group_id) - q = q.filter_by(id=self.id) - q = q.order_by(model.PackageRevision.name) + query = meta.Session.query(_package.Package) + query = query.filter( + or_(_package.Package.state == vdm.sqlalchemy.State.ACTIVE, + _package.Package.state == vdm.sqlalchemy.State.PENDING)) + query = query.filter(group_table.c.id == self.id) + + if not with_private: + query = query.filter(member_table.c.capacity == 'public') + + query = query.join(member_table, + member_table.c.table_id == _package.Package.id) + query = query.join(group_table, + group_table.c.id == member_table.c.group_id) + if limit is not None: - q = q.limit(limit) + query = query.limit(limit) + if return_query: - return q + return query else: - return q.all() - + return query.all() @classmethod def search_by_name_or_title(cls, text_query, group_type=None): @@ -212,7 +213,7 @@ def add_package_by_name(self, package_name): return package = _package.Package.by_name(package_name) assert package - if not package in self.active_packages().all(): + if not package in self.packages(): member = Member(group=self, table_id=package.id, table_name='package') meta.Session.add(member) diff --git a/ckan/tests/functional/test_group.py b/ckan/tests/functional/test_group.py index 7bfc04aaadd..57e2c62574c 100644 --- a/ckan/tests/functional/test_group.py +++ b/ckan/tests/functional/test_group.py @@ -187,7 +187,7 @@ def test_index(self): groupname = 'david' group = model.Group.by_name(unicode(groupname)) group_title = group.title - group_packages_count = len(group.active_packages().all()) + group_packages_count = len(group.packages()) group_description = group.description self.check_named_element(res, 'tr', group_title, group_packages_count, @@ -321,7 +321,7 @@ def test_2_edit(self): assert group.description == newdesc, group # now look at datasets - assert len(group.active_packages().all()) == 3 + assert len(group.packages()) == 3 def test_3_edit_form_has_new_package(self): # check for dataset in autocomplete @@ -369,7 +369,7 @@ def test_4_new_duplicate_package(self): # check package only added to the group once group = model.Group.by_name(group_name) - pkg_names = [pkg.name for pkg in group.active_packages().all()] + pkg_names = [pkg.name for pkg in group.packages()] assert_equal(pkg_names, [self.packagename]) def test_edit_plugin_hook(self): @@ -435,7 +435,7 @@ def update_group(res, name, with_pkg=True): # We have the datasets in the DB, but we should also see that many # on the group read page. - assert len(group.active_packages().all()) == 3 + assert len(group.packages()) == 3 offset = url_for(controller='group', action='read', id='newname') res = self.app.get(offset, status=200, @@ -543,9 +543,9 @@ def test_2_new(self): group = model.Group.by_name(group_name) assert group.title == group_title, group assert group.description == group_description, group - assert len(group.active_packages().all()) == 1 + assert len(group.packages()) == 1 pkg = model.Package.by_name(self.packagename) - assert group.active_packages().all() == [pkg] + assert group.packages() == [pkg] def test_3_new_duplicate_group(self): prefix = '' @@ -702,7 +702,7 @@ def test_index(self): groupname = 'david' group = model.Group.by_name(unicode(groupname)) group_title = group.title - group_packages_count = len(group.active_packages().all()) + group_packages_count = len(group.packages()) group_description = group.description self.check_named_element(res, 'tr', group_title, group_packages_count, @@ -820,7 +820,7 @@ def test_2_edit(self): assert group.description == newdesc, group # now look at datasets - assert len(group.active_packages().all()) == 3 + assert len(group.packages()) == 3 def test_3_edit_form_has_new_package(self): # check for dataset in autocomplete @@ -868,7 +868,7 @@ def test_4_new_duplicate_package(self): # check package only added to the group once group = model.Group.by_name(group_name) - pkg_names = [pkg.name for pkg in group.active_packages().all()] + pkg_names = [pkg.name for pkg in group.packages()] assert_equal(pkg_names, [self.packagename]) def test_edit_plugin_hook(self): diff --git a/ckan/tests/lib/test_dictization_schema.py b/ckan/tests/lib/test_dictization_schema.py index ad9fa290cf7..ef1391b98b8 100644 --- a/ckan/tests/lib/test_dictization_schema.py +++ b/ckan/tests/lib/test_dictization_schema.py @@ -148,7 +148,7 @@ def test_2_group_schema(self): del data['extras'] converted_data, errors = validate(data, default_group_schema(), self.context) - group_pack = sorted(group.active_packages().all(), key=lambda x:x.id) + group_pack = sorted(group.packages(), key=lambda x:x.id) converted_data["packages"] = sorted(converted_data["packages"], key=lambda x:x["id"]) diff --git a/ckan/tests/models/test_group.py b/ckan/tests/models/test_group.py index 703b63778f8..424e8f3c198 100644 --- a/ckan/tests/models/test_group.py +++ b/ckan/tests/models/test_group.py @@ -25,7 +25,7 @@ def test_1_basic(self): grp = model.Group.by_name(u'group1') assert grp.title == u'Test Group' assert grp.description == u'This is a test group' - assert grp.active_packages().all() == [] + assert grp.packages() == [] def test_2_add_packages(self): model.repo.new_revision() @@ -50,7 +50,7 @@ def test_2_add_packages(self): assert grp.title == u'Russian Group' anna = model.Package.by_name(u'annakarenina') war = model.Package.by_name(u'warandpeace') - assert set(grp.active_packages().all()) == set((anna, war)), grp.active_packages().all() + assert set(grp.packages()) == set((anna, war)), grp.packages() assert grp in anna.get_groups() def test_3_search(self): From e9a55a11c26e037da62b4230d54ee11611a431e4 Mon Sep 17 00:00:00 2001 From: Sean Hammond Date: Thu, 22 Nov 2012 16:48:34 +0100 Subject: [PATCH 17/27] Fix following of groups Add activities from the datasets of followed groups into the user's dashboard activity stream. All the test_dashboard.py tests now pass. Move SQLAlchemy from group_activity_list() into model, alongside the SQLAlchemy queries for the other types of activity stream. Change the SQLAlchemy in activities_from_groups_followed_by_user() to be the union of group_activity_list() for each of the followed groups. Update some tests. --- ckan/logic/action/get.py | 25 +-------------- ckan/model/activity.py | 34 ++++++++++++++++++--- ckan/tests/functional/api/test_dashboard.py | 16 +++------- 3 files changed, 36 insertions(+), 39 deletions(-) diff --git a/ckan/logic/action/get.py b/ckan/logic/action/get.py index f1fc99b05a7..c85e3a7ece9 100644 --- a/ckan/logic/action/get.py +++ b/ckan/logic/action/get.py @@ -1767,30 +1767,7 @@ def group_activity_list(context, data_dict): group_show = logic.get_action('group_show') group_id = group_show(context, {'id': group_id})['id'] - # FIXME: The SQLAlchemy below should be moved into ckan/model/activity.py - # (to encapsulate SQLALchemy in the model and avoid using it from the - # logic) but it can't be because it requires the list of dataset_ids which - # comes from logic.group_package_show() (and I don't want to access the - # logic from the model). Need to change it to get the dataset_ids from the - # model instead. There seems to be multiple methods for getting a group's - # datasets, some in the logic and some in the model. - - # Get a list of the IDs of the group's datasets. - group_package_show = logic.get_action('group_package_show') - datasets = group_package_show(context, {'id': group_id}) - dataset_ids = [dataset['id'] for dataset in datasets] - - # Get the group's activities. - query = model.Session.query(model.Activity) - if dataset_ids: - query = query.filter(_or_(model.Activity.object_id == group_id, - model.Activity.object_id.in_(dataset_ids))) - else: - query = query.filter(model.Activity.object_id == group_id) - query = query.order_by(_desc(model.Activity.timestamp)) - query = query.limit(15) - activity_objects = query.all() - + activity_objects = model.activity.group_activity_list(group_id) return model_dictize.activity_list_dictize(activity_objects, context) def recently_changed_packages_activity_list(context, data_dict): diff --git a/ckan/model/activity.py b/ckan/model/activity.py index f0f706a8df3..23abbce1cd6 100644 --- a/ckan/model/activity.py +++ b/ckan/model/activity.py @@ -1,6 +1,6 @@ import datetime -from sqlalchemy import orm, types, Column, Table, ForeignKey, desc +from sqlalchemy import orm, types, Column, Table, ForeignKey, desc, or_ import meta import types as _types @@ -133,6 +133,26 @@ def package_activity_list(package_id, limit=15): return _most_recent_activities(q, limit) +def _group_activity_query(group_id, limit=15): + import ckan.model as model + + group = model.Group.get(group_id) + dataset_ids = [dataset.id for dataset in group.packages()] + + q = model.Session.query(model.Activity) + if dataset_ids: + q = q.filter(or_(model.Activity.object_id == group_id, + model.Activity.object_id.in_(dataset_ids))) + else: + q = q.filter(model.Activity.object_id == group_id) + return q + + +def group_activity_list(group_id, limit=15): + q = _group_activity_query(group_id) + return _most_recent_activities(q, limit) + + def _activites_from_users_followed_by_user_query(user_id): import ckan.model as model q = model.Session.query(model.Activity) @@ -153,10 +173,16 @@ def _activities_from_datasets_followed_by_user_query(user_id): def _activities_from_groups_followed_by_user_query(user_id): import ckan.model as model + + # Get a list of the group's that the user is following. + follower_objects = model.UserFollowingGroup.followee_list(user_id) + if not follower_objects: + # Return a query with no results. + return model.Session.query(model.Activity).filter("0=1") + q = model.Session.query(model.Activity) - q = q.join(model.UserFollowingGroup, - model.UserFollowingGroup.object_id == model.Activity.object_id) - q = q.filter(model.UserFollowingGroup.follower_id == user_id) + q = q.union_all(*[_group_activity_query(follower.object_id) + for follower in follower_objects]) return q diff --git a/ckan/tests/functional/api/test_dashboard.py b/ckan/tests/functional/api/test_dashboard.py index 6cef63beb21..0a5803e7cff 100644 --- a/ckan/tests/functional/api/test_dashboard.py +++ b/ckan/tests/functional/api/test_dashboard.py @@ -174,9 +174,9 @@ def test_03_dashboard_activity_list_own_activities(self): # FIXME: There should actually be 6 activities here, but when you # follow something it's old activities (from before you followed it) - # appear in your activity stream. So here we get one more activity - # than expected. - assert len(activities) == 7 + # appear in your activity stream. So here we get more activities than + # expected. + assert len(activities) == 8 assert activities[0]['activity_type'] == 'changed package' assert activities[1]['activity_type'] == 'follow group' @@ -289,18 +289,12 @@ def test_04_activities_from_datasets_of_followed_groups(self): def test_05_new_activities_count(self): '''Test that new activities from objects that a user follows increase her new activities count.''' - # FIXME: The number here should be 4 but activities from datasets of - # followed groups are not appearing in dashboard. When that is fixed, - # fix this number. - assert self.dashboard_new_activities_count(self.new_user) == 3 + assert self.dashboard_new_activities_count(self.new_user) == 4 def test_06_activities_marked_as_new(self): '''Test that new activities from objects that a user follows are marked as new in her dashboard activity stream.''' - # FIXME: The number here should be 4 but activities from datasets of - # followed groups are not appearing in dashboard. When that is fixed, - # fix this number. - assert len(self.dashboard_new_activities(self.new_user)) == 3 + assert len(self.dashboard_new_activities(self.new_user)) == 4 def test_07_mark_new_activities_as_read(self): '''Test that a user's new activities are marked as old when she views From 8e2f2a0baa703998337e3f035ab97064341cca4d Mon Sep 17 00:00:00 2001 From: Sean Hammond Date: Thu, 22 Nov 2012 17:41:20 +0100 Subject: [PATCH 18/27] Add docstring to group_activity_list() --- ckan/model/activity.py | 10 ++++++++++ 1 file changed, 10 insertions(+) diff --git a/ckan/model/activity.py b/ckan/model/activity.py index 23abbce1cd6..47ef8b6e7bf 100644 --- a/ckan/model/activity.py +++ b/ckan/model/activity.py @@ -149,6 +149,16 @@ def _group_activity_query(group_id, limit=15): def group_activity_list(group_id, limit=15): + '''Return the given group's public activity stream. + + Returns all activities where the given group or one of its datasets is the + object of the activity, e.g.: + + "{USER} updated the group {GROUP}" + "{USER} updated the dataset {DATASET}" + etc. + + ''' q = _group_activity_query(group_id) return _most_recent_activities(q, limit) From 3a8c37c41efa548a0f59f475e01c748a3a0669d6 Mon Sep 17 00:00:00 2001 From: Sean Hammond Date: Mon, 26 Nov 2012 14:10:31 +0100 Subject: [PATCH 19/27] Add some docstrings to model/activity.py --- ckan/model/activity.py | 46 +++++++++++++++++++++++++++++++++++++----- 1 file changed, 41 insertions(+), 5 deletions(-) diff --git a/ckan/model/activity.py b/ckan/model/activity.py index 47ef8b6e7bf..92017619cdc 100644 --- a/ckan/model/activity.py +++ b/ckan/model/activity.py @@ -69,6 +69,7 @@ def __init__(self, activity_id, object_id, object_type, activity_type, def _most_recent_activities(q, limit): + '''Return the 'limit' most recent activites from activity query 'q'.''' import ckan.model as model q = q.order_by(desc(model.Activity.timestamp)) if limit: @@ -77,6 +78,7 @@ def _most_recent_activities(q, limit): def _activities_from_user_query(user_id): + '''Return an SQLAlchemy query for all activities from user_id.''' import ckan.model as model q = model.Session.query(model.Activity) q = q.filter(model.Activity.user_id == user_id) @@ -84,6 +86,7 @@ def _activities_from_user_query(user_id): def _activities_about_user_query(user_id): + '''Return an SQLAlchemy query for all activities about user_id.''' import ckan.model as model q = model.Session.query(model.Activity) q = q.filter(model.Activity.object_id == user_id) @@ -91,16 +94,17 @@ def _activities_about_user_query(user_id): def _user_activity_query(user_id): + '''Return an SQLAlchemy query for all activities from or about user_id.''' q = _activities_from_user_query(user_id) q = q.union(_activities_about_user_query(user_id)) return q def user_activity_list(user_id, limit=15): - '''Return the given user's public activity stream. + '''Return user_id's public activity stream. - Returns all activities from or about the given user, i.e. where the given - user is the subject or object of the activity, e.g.: + Return a list of all activities from or about the given user, i.e. where + the given user is the subject or object of the activity, e.g.: "{USER} created the dataset {DATASET}" "{OTHER_USER} started following {USER}" @@ -112,6 +116,9 @@ def user_activity_list(user_id, limit=15): def _package_activity_query(package_id): + '''Return an SQLAlchemy query for all activities about package_id. + + ''' import ckan.model as model q = model.Session.query(model.Activity) q = q.filter_by(object_id=package_id) @@ -134,6 +141,12 @@ def package_activity_list(package_id, limit=15): def _group_activity_query(group_id, limit=15): + '''Return an SQLAlchemy query for all activities about group_id. + + Returns a query for all activities whose object is either the group itself + or one of the group's datasets. + + ''' import ckan.model as model group = model.Group.get(group_id) @@ -164,6 +177,7 @@ def group_activity_list(group_id, limit=15): def _activites_from_users_followed_by_user_query(user_id): + '''Return a query for all activities from users that user_id follows.''' import ckan.model as model q = model.Session.query(model.Activity) q = q.join(model.UserFollowingUser, @@ -173,6 +187,7 @@ def _activites_from_users_followed_by_user_query(user_id): def _activities_from_datasets_followed_by_user_query(user_id): + '''Return a query for all activities from datasets that user_id follows.''' import ckan.model as model q = model.Session.query(model.Activity) q = q.join(model.UserFollowingDataset, @@ -182,6 +197,13 @@ def _activities_from_datasets_followed_by_user_query(user_id): def _activities_from_groups_followed_by_user_query(user_id): + '''Return a query for all activities about groups the given user follows. + + Return a query for all activities about the groups the given user follows, + or about any of the group's datasets. This is the union of + _group_activity_query(group_id) for each of the groups the user follows. + + ''' import ckan.model as model # Get a list of the group's that the user is following. @@ -197,6 +219,7 @@ def _activities_from_groups_followed_by_user_query(user_id): def _activities_from_everything_followed_by_user_query(user_id): + '''Return a query for all activities from everything user_id follows.''' q = _activites_from_users_followed_by_user_query(user_id) q = q.union(_activities_from_datasets_followed_by_user_query(user_id)) q = q.union(_activities_from_groups_followed_by_user_query(user_id)) @@ -215,6 +238,7 @@ def activities_from_everything_followed_by_user(user_id, limit=15): def _dashboard_activity_query(user_id): + '''Return an SQLAlchemy query for user_id's dashboard activity stream.''' q = _user_activity_query(user_id) q = q.union(_activities_from_everything_followed_by_user_query(user_id)) return q @@ -234,7 +258,13 @@ def dashboard_activity_list(user_id, limit=15): return _most_recent_activities(q, limit) -def _recently_changed_packages_activity_query(): +def _changed_packages_activity_query(): + '''Return an SQLAlchemyu query for all changed package activities. + + Return a query for all activities with activity_type '*package', e.g. + 'new_package', 'changed_package', 'deleted_package'. + + ''' import ckan.model as model q = model.Session.query(model.Activity) q = q.filter(model.Activity.activity_type.endswith('package')) @@ -242,5 +272,11 @@ def _recently_changed_packages_activity_query(): def recently_changed_packages_activity_list(limit=15): - q = _recently_changed_packages_activity_query() + '''Return the site-wide stream of recently changed package activities. + + This activity stream includes recent 'new package', 'changed package' and + 'deleted package' activities for the whole site. + + ''' + q = _changed_packages_activity_query() return _most_recent_activities(q, limit) From a96757e1cd082f6c449ec5cb9eb4a60c4b610d84 Mon Sep 17 00:00:00 2001 From: Sean Hammond Date: Mon, 26 Nov 2012 14:38:08 +0100 Subject: [PATCH 20/27] Fix dashboard activity stream from followed groups It was returning all activities from the entire site, fix to return activities from followed groups only. The tests didn't catch this error because they didn't test doing random activities that should _not_ appear in the user's dashboard and asserting that they don't. Add a quick test that catches this. This is probably a problem for the other activity streams tests in test_activity.py as well. --- ckan/model/activity.py | 4 ++-- ckan/tests/functional/api/test_dashboard.py | 15 +++++++++++++++ 2 files changed, 17 insertions(+), 2 deletions(-) diff --git a/ckan/model/activity.py b/ckan/model/activity.py index 92017619cdc..6d9970e09ce 100644 --- a/ckan/model/activity.py +++ b/ckan/model/activity.py @@ -212,9 +212,9 @@ def _activities_from_groups_followed_by_user_query(user_id): # Return a query with no results. return model.Session.query(model.Activity).filter("0=1") - q = model.Session.query(model.Activity) + q = _group_activity_query(follower_objects[0].object_id) q = q.union_all(*[_group_activity_query(follower.object_id) - for follower in follower_objects]) + for follower in follower_objects[1:]]) return q diff --git a/ckan/tests/functional/api/test_dashboard.py b/ckan/tests/functional/api/test_dashboard.py index 0a5803e7cff..1b285a222c3 100644 --- a/ckan/tests/functional/api/test_dashboard.py +++ b/ckan/tests/functional/api/test_dashboard.py @@ -316,3 +316,18 @@ def test_08_maximum_number_of_new_activities(self): extra_environ={'Authorization': str(self.joeadmin['apikey'])}) assert response.json['success'] is True assert self.dashboard_new_activities_count(self.new_user) == 15 + + def test_09_activities_that_should_not_show(self): + '''Test that other activities do not appear on the user's dashboard.''' + + before = self.dashboard_activity_list(self.new_user) + + # Make someone else who new_user is not following create a new dataset. + params = json.dumps({'name': 'irrelevant_dataset'}) + response = self.app.post('/api/action/package_create', params=params, + extra_environ={'Authorization': str(self.testsysadmin['apikey'])}) + assert response.json['success'] is True + + after = self.dashboard_activity_list(self.new_user) + + assert before == after From afcce35935daa0ce4136d41bfbc66c496e46cba3 Mon Sep 17 00:00:00 2001 From: amercader Date: Wed, 28 Nov 2012 10:42:55 +0000 Subject: [PATCH 21/27] Add requests to core requirements --- pip-requirements-test.txt | 1 - pip-requirements.txt | 1 + 2 files changed, 1 insertion(+), 1 deletion(-) diff --git a/pip-requirements-test.txt b/pip-requirements-test.txt index b8103e124eb..41a359c0264 100644 --- a/pip-requirements-test.txt +++ b/pip-requirements-test.txt @@ -1,6 +1,5 @@ # These are packages that required when running ckan tests nose -requests==0.6.4 -e git+https://github.com/okfn/ckanclient#egg=ckanclient diff --git a/pip-requirements.txt b/pip-requirements.txt index 9a210b663a2..52b4c084a27 100644 --- a/pip-requirements.txt +++ b/pip-requirements.txt @@ -27,3 +27,4 @@ routes==1.13 paste==1.7.5.1 Jinja2==2.6 fanstatic==0.12 +requests==0.14 From aeddf425583e58d72f3ab3259ff0c4347f2a7903 Mon Sep 17 00:00:00 2001 From: Sean Hammond Date: Wed, 28 Nov 2012 13:24:46 +0100 Subject: [PATCH 22/27] Fix an activity streams crash It seems that _group_activity_query() can sometimes be called for a group that doesn't exist (though I'm not sure exactly how this happens). A test was failing. Not sure how this crept into master. Fix it to handle this case. --- ckan/model/activity.py | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/ckan/model/activity.py b/ckan/model/activity.py index 6d9970e09ce..cddf9e04e01 100644 --- a/ckan/model/activity.py +++ b/ckan/model/activity.py @@ -150,6 +150,10 @@ def _group_activity_query(group_id, limit=15): import ckan.model as model group = model.Group.get(group_id) + if not group: + # Return a query with no results. + return model.Session.query(model.Activity).filter("0=1") + dataset_ids = [dataset.id for dataset in group.packages()] q = model.Session.query(model.Activity) From e4f38380deab1ec8711ffd8b0fe5b69091782752 Mon Sep 17 00:00:00 2001 From: Sean Hammond Date: Wed, 28 Nov 2012 15:17:59 +0100 Subject: [PATCH 23/27] Fix a crash in mock_publisher_auth mock_publisher_auth uses `log` but doesn't initialise it. Import logging and initialise log. --- ckan/tests/mock_publisher_auth.py | 3 +++ 1 file changed, 3 insertions(+) diff --git a/ckan/tests/mock_publisher_auth.py b/ckan/tests/mock_publisher_auth.py index 749588a4fd9..01a292424ea 100644 --- a/ckan/tests/mock_publisher_auth.py +++ b/ckan/tests/mock_publisher_auth.py @@ -1,5 +1,8 @@ from ckan.new_authz import is_authorized from ckan.logic import NotAuthorized +import logging + +log = logging.getLogger("mock_publisher_auth") class MockPublisherAuth(object): """ From 6aad4b8aa4549d613769ac57896773532287172f Mon Sep 17 00:00:00 2001 From: Sean Hammond Date: Wed, 28 Nov 2012 15:19:56 +0100 Subject: [PATCH 24/27] Delete an unused import --- ckan/tests/mock_publisher_auth.py | 1 - 1 file changed, 1 deletion(-) diff --git a/ckan/tests/mock_publisher_auth.py b/ckan/tests/mock_publisher_auth.py index 01a292424ea..57e136ee23a 100644 --- a/ckan/tests/mock_publisher_auth.py +++ b/ckan/tests/mock_publisher_auth.py @@ -1,4 +1,3 @@ -from ckan.new_authz import is_authorized from ckan.logic import NotAuthorized import logging From 53013843af361b7310a6418b3609c2ff1fc0378e Mon Sep 17 00:00:00 2001 From: Sean Hammond Date: Wed, 28 Nov 2012 15:21:02 +0100 Subject: [PATCH 25/27] PEP8 fixes --- ckan/tests/mock_publisher_auth.py | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-) diff --git a/ckan/tests/mock_publisher_auth.py b/ckan/tests/mock_publisher_auth.py index 57e136ee23a..475666a21eb 100644 --- a/ckan/tests/mock_publisher_auth.py +++ b/ckan/tests/mock_publisher_auth.py @@ -3,21 +3,22 @@ log = logging.getLogger("mock_publisher_auth") + class MockPublisherAuth(object): """ MockPublisherAuth """ - + def __init__(self): self.functions = {} self._load() def _load(self): - for auth_module_name in ['get', 'create', 'update','delete']: + for auth_module_name in ['get', 'create', 'update', 'delete']: module_path = 'ckan.logic.auth.publisher.%s' % (auth_module_name,) try: module = __import__(module_path) - except ImportError,e: + except ImportError: log.debug('No auth module for action "%s"' % auth_module_name) continue @@ -27,10 +28,9 @@ def _load(self): for key, v in module.__dict__.items(): if not key.startswith('_'): self.functions[key] = v - - - def check_access(self,action, context, data_dict): + + def check_access(self, action, context, data_dict): logic_authorization = self.functions[action](context, data_dict) if not logic_authorization['success']: - msg = logic_authorization.get('msg','') + msg = logic_authorization.get('msg', '') raise NotAuthorized(msg) From 3ea18168e62ae887d38a2d46948434d8b60ebe66 Mon Sep 17 00:00:00 2001 From: Sean Hammond Date: Tue, 27 Nov 2012 22:24:06 +0100 Subject: [PATCH 26/27] Add test helper function for posting to CKAN API Add ckan.tests.post(), a helper function for posting to CKAN's action API and getting the result, that handles forming the correct action API URL, converting the apikey to a string and putting it in a headers dict like TestApp expects, collecting params into a dict and dumping them to a JSON string, and checking the 'success' field of the response. This can save a lot of lines of code and make tests more readable. I've converted test_follow.py as an example of how much can be saved. Similar savings could be made in many other test modules: ckan/tests/functional/api/model/test_vocabulary.py, ckan/tests/functional/api/model/test_group.py, ckan/tests/functional/api/test_dashboard.py, ckan/tests/functional/api/test_activity.py, ckan/tests/functional/test_follow.py, ckan/tests/functional/test_tag_vocab.py, ckan/tests/functional/test_related.py, ckan/tests/logic/test_action.py ckan/tests/logic/test_tag.py. ckan/tests/functional/api/model/test_package.py could also be changed to use this function. It currently uses a similar function defined in ckan.tests.functional.api.base:ApiTestCase. --- ckan/tests/__init__.py | 58 ++ ckan/tests/functional/api/test_follow.py | 899 ++++++++--------------- 2 files changed, 352 insertions(+), 605 deletions(-) diff --git a/ckan/tests/__init__.py b/ckan/tests/__init__.py index 721c55f6f49..4058b79ad7c 100644 --- a/ckan/tests/__init__.py +++ b/ckan/tests/__init__.py @@ -400,3 +400,61 @@ class StatusCodes: STATUS_404_NOT_FOUND = 404 STATUS_409_CONFLICT = 409 + +def post(app, action, apikey=None, status=200, **kwargs): + '''Post to the CKAN API and return the result. + + Any additional keyword arguments that you pass to this function as **kwargs + are posted as params to the API. + + Usage: + + package_dict = post(app, 'package_create', apikey=apikey, + name='my_package') + assert package_dict['name'] == 'my_package' + + num_followers = post(app, 'user_follower_count', id='annafan') + + If you are expecting an error from the API and want to check the contents + of the error dict, you have to use the status param otherwise an exception + will be raised: + + error_dict = post(app, 'group_activity_list', status=403, + id='invalid_id') + assert error_dict['message'] == 'Access Denied' + + :param app: the test app to post to + :type app: paste.fixture.TestApp + + :param action: the action to post to, e.g. 'package_create' + :type action: string + + :param apikey: the API key to put in the Authorization header of the post + (optional, default: None) + :type apikey: string + + :param status: the HTTP status code expected in the response from the CKAN + API, e.g. 403, if a different status code is received an exception will + be raised (optional, default: 200) + :type status: int + + :param **kwargs: any other keyword arguments passed to this function will + be posted to the API as params + + :raises paste.fixture.AppError: if the HTTP status code of the response + from the CKAN API is different from the status param passed to this + function + + :returns: the 'result' or 'error' dictionary from the CKAN API response + :rtype: dictionary + + ''' + params = json.dumps(kwargs) + response = app.post('/api/action/{0}'.format(action), params=params, + extra_environ={'Authorization': str(apikey)}, status=status) + if status in (200,): + assert response.json['success'] is True + return response.json['result'] + else: + assert response.json['success'] is False + return response.json['error'] diff --git a/ckan/tests/functional/api/test_follow.py b/ckan/tests/functional/api/test_follow.py index a084c930fd9..60ee6c5e6c9 100644 --- a/ckan/tests/functional/api/test_follow.py +++ b/ckan/tests/functional/api/test_follow.py @@ -14,8 +14,8 @@ import paste import pylons.test import ckan -from ckan.lib.helpers import json from ckan.tests import are_foreign_keys_supported, SkipTest +import ckan.tests def datetime_from_string(s): '''Return a standard datetime.datetime object initialised from a string in @@ -36,83 +36,54 @@ def follow_user(app, follower_id, apikey, object_id, object_arg): ''' # Record the object's followers count before. - params = json.dumps({'id': object_id}) - response = app.post('/api/action/user_follower_count', - params=params).json - assert response['success'] is True - follower_count_before = response['result'] + follower_count_before = ckan.tests.post(app, 'user_follower_count', + id=object_id) # Record the follower's followees count before. - params = json.dumps({'id': follower_id}) - response = app.post('/api/action/user_followee_count', - params=params).json - assert response['success'] is True - followee_count_before = response['result'] + followee_count_before = ckan.tests.post(app, 'user_followee_count', + id=follower_id) # Check that the user is not already following the object. - params = json.dumps({'id': object_id}) - extra_environ = {'Authorization': str(apikey)} - response = app.post('/api/action/am_following_user', - params=params, extra_environ=extra_environ).json - assert response['success'] is True - assert response['result'] is False + result = ckan.tests.post(app, 'am_following_user', + id=object_id, apikey=apikey) + assert result is False # Make the user start following the object. before = datetime.datetime.now() - params = {'id': object_arg} - extra_environ = {'Authorization': str(apikey)} - response = app.post('/api/action/follow_user', - params=json.dumps(params), extra_environ=extra_environ).json + follower = ckan.tests.post(app, 'follow_user', id=object_arg, + apikey=apikey) after = datetime.datetime.now() - assert response['success'] is True - assert response['result'] - follower = response['result'] assert follower['follower_id'] == follower_id assert follower['object_id'] == object_id timestamp = datetime_from_string(follower['datetime']) assert (timestamp >= before and timestamp <= after), str(timestamp) # Check that am_following_user now returns True. - params = json.dumps({'id': object_id}) - extra_environ = {'Authorization': str(apikey)} - response = app.post('/api/action/am_following_user', - params=params, extra_environ=extra_environ).json - assert response['success'] is True - assert response['result'] is True + result = ckan.tests.post(app, 'am_following_user', + id=object_id, apikey=apikey) + assert result is True # Check that the follower appears in the object's list of followers. - params = json.dumps({'id': object_id}) - response = app.post('/api/action/user_follower_list', - params=params).json - assert response['success'] is True - assert response['result'] - followers = response['result'] + followers = ckan.tests.post(app, 'user_follower_list', + id=object_id) assert len(followers) == follower_count_before + 1 assert len([follower for follower in followers if follower['id'] == follower_id]) == 1 # Check that the object appears in the follower's list of followees. - params = json.dumps({'id': follower_id}) - response = app.post('/api/action/user_followee_list', - params=params).json - assert response['success'] is True - assert response['result'] - followees = response['result'] + followees = ckan.tests.post(app, 'user_followee_list', + id=follower_id) assert len(followees) == followee_count_before + 1 assert len([followee for followee in followees if followee['id'] == object_id]) == 1 # Check that the object's follower count has increased by 1. - params = json.dumps({'id': object_id}) - response = app.post('/api/action/user_follower_count', - params=params).json - assert response['success'] is True - assert response['result'] == follower_count_before + 1 + follower_count_after = ckan.tests.post(app, 'user_follower_count', + id=object_id) + assert follower_count_after == follower_count_before + 1 # Check that the follower's followee count has increased by 1. - params = json.dumps({'id': follower_id}) - response = app.post('/api/action/user_followee_count', - params=params).json - assert response['success'] is True - assert response['result'] == followee_count_before + 1 + followee_count_after = ckan.tests.post(app, 'user_followee_count', + id=follower_id) + assert followee_count_after == followee_count_before + 1 def follow_dataset(app, follower_id, apikey, dataset_id, dataset_arg): '''Test a user starting to follow a dataset via the API. @@ -125,83 +96,54 @@ def follow_dataset(app, follower_id, apikey, dataset_id, dataset_arg): ''' # Record the dataset's followers count before. - params = json.dumps({'id': dataset_id}) - response = app.post('/api/action/dataset_follower_count', - params=params).json - assert response['success'] is True - follower_count_before = response['result'] + follower_count_before = ckan.tests.post(app, 'dataset_follower_count', + id=dataset_id) # Record the follower's followees count before. - params = json.dumps({'id': follower_id}) - response = app.post('/api/action/dataset_followee_count', - params=params).json - assert response['success'] is True - followee_count_before = response['result'] + followee_count_before = ckan.tests.post(app, 'dataset_followee_count', + id=follower_id) # Check that the user is not already following the dataset. - params = json.dumps({'id': dataset_id}) - extra_environ = {'Authorization': str(apikey)} - response = app.post('/api/action/am_following_dataset', - params=params, extra_environ=extra_environ).json - assert response['success'] is True - assert response['result'] is False + result = ckan.tests.post(app, 'am_following_dataset', + id=dataset_id, apikey=apikey) + assert result is False # Make the user start following the dataset. before = datetime.datetime.now() - params = {'id': dataset_arg} - extra_environ = {'Authorization': str(apikey)} - response = app.post('/api/action/follow_dataset', - params=json.dumps(params), extra_environ=extra_environ).json + follower = ckan.tests.post(app, 'follow_dataset', + id=dataset_arg, apikey=apikey) after = datetime.datetime.now() - assert response['success'] is True - assert response['result'] - follower = response['result'] assert follower['follower_id'] == follower_id assert follower['object_id'] == dataset_id timestamp = datetime_from_string(follower['datetime']) assert (timestamp >= before and timestamp <= after), str(timestamp) # Check that am_following_dataset now returns True. - params = json.dumps({'id': dataset_id}) - extra_environ = {'Authorization': str(apikey)} - response = app.post('/api/action/am_following_dataset', - params=params, extra_environ=extra_environ).json - assert response['success'] is True - assert response['result'] is True + result = ckan.tests.post(app, 'am_following_dataset', + id=dataset_id, apikey=apikey) + assert result is True # Check that the follower appears in the dataset's list of followers. - params = json.dumps({'id': dataset_id}) - response = app.post('/api/action/dataset_follower_list', - params=params).json - assert response['success'] is True - assert response['result'] - followers = response['result'] + followers = ckan.tests.post(app, 'dataset_follower_list', + id=dataset_id) assert len(followers) == follower_count_before + 1 assert len([follower for follower in followers if follower['id'] == follower_id]) == 1 # Check that the dataset appears in the follower's list of followees. - params = json.dumps({'id': follower_id}) - response = app.post('/api/action/dataset_followee_list', - params=params).json - assert response['success'] is True - assert response['result'] - followees = response['result'] + followees = ckan.tests.post(app, 'dataset_followee_list', + id=follower_id) assert len(followees) == followee_count_before + 1 assert len([followee for followee in followees if followee['id'] == dataset_id]) == 1 # Check that the dataset's follower count has increased by 1. - params = json.dumps({'id': dataset_id}) - response = app.post('/api/action/dataset_follower_count', - params=params).json - assert response['success'] is True - assert response['result'] == follower_count_before + 1 + follower_count_after = ckan.tests.post(app, 'dataset_follower_count', + id=dataset_id) + assert follower_count_after == follower_count_before + 1 # Check that the follower's followee count has increased by 1. - params = json.dumps({'id': follower_id}) - response = app.post('/api/action/dataset_followee_count', - params=params).json - assert response['success'] is True - assert response['result'] == followee_count_before + 1 + followee_count_after = ckan.tests.post(app, 'dataset_followee_count', + id=follower_id) + assert followee_count_after == followee_count_before + 1 def follow_group(app, user_id, apikey, group_id, group_arg): '''Test a user starting to follow a group via the API. @@ -214,85 +156,56 @@ def follow_group(app, user_id, apikey, group_id, group_arg): ''' # Record the group's followers count before. - params = json.dumps({'id': group_id}) - response = app.post('/api/action/group_follower_count', - params=params).json - assert response['success'] is True - follower_count_before = response['result'] + follower_count_before = ckan.tests.post(app, 'group_follower_count', + id=group_id) # Record the user's followees count before. - params = json.dumps({'id': user_id}) - response = app.post('/api/action/group_followee_count', - params=params).json - assert response['success'] is True - followee_count_before = response['result'] + followee_count_before = ckan.tests.post(app, 'group_followee_count', + id=user_id) # Check that the user is not already following the group. - params = json.dumps({'id': group_id}) - extra_environ = {'Authorization': str(apikey)} - response = app.post('/api/action/am_following_group', - params=params, extra_environ=extra_environ).json - assert response['success'] is True - assert response['result'] is False + result = ckan.tests.post(app, 'am_following_group', + id=group_id, apikey=apikey) + assert result is False # Make the user start following the group. before = datetime.datetime.now() - params = {'id': group_id} - extra_environ = {'Authorization': str(apikey)} - response = app.post('/api/action/follow_group', - params=json.dumps(params), extra_environ=extra_environ).json + follower = ckan.tests.post(app, 'follow_group', id=group_id, + apikey=apikey) after = datetime.datetime.now() - assert response['success'] is True - assert response['result'] - follower = response['result'] assert follower['follower_id'] == user_id assert follower['object_id'] == group_id timestamp = datetime_from_string(follower['datetime']) assert (timestamp >= before and timestamp <= after), str(timestamp) # Check that am_following_group now returns True. - params = json.dumps({'id': group_id}) - extra_environ = {'Authorization': str(apikey)} - response = app.post('/api/action/am_following_group', - params=params, extra_environ=extra_environ).json - assert response['success'] is True - assert response['result'] is True + result = ckan.tests.post(app, 'am_following_group', + id=group_id, apikey=apikey) + assert result is True # Check that the user appears in the group's list of followers. - params = json.dumps({'id': group_id}) - response = app.post('/api/action/group_follower_list', - params=params).json - assert response['success'] is True - assert response['result'] - followers = response['result'] + followers = ckan.tests.post(app, 'group_follower_list', + id=group_id) assert len(followers) == follower_count_before + 1 assert len([follower for follower in followers if follower['id'] == user_id]) == 1 # Check that the group appears in the user's list of followees. - params = json.dumps({'id': user_id}) - response = app.post('/api/action/group_followee_list', - params=params).json - assert response['success'] is True - assert response['result'] - followees = response['result'] + followees = ckan.tests.post(app, 'group_followee_list', + id=user_id) assert len(followees) == followee_count_before + 1 assert len([followee for followee in followees if followee['id'] == group_id]) == 1 # Check that the group's follower count has increased by 1. - params = json.dumps({'id': group_id}) - response = app.post('/api/action/group_follower_count', - params=params).json - assert response['success'] is True - assert response['result'] == follower_count_before + 1 + follower_count_after = ckan.tests.post(app, 'group_follower_count', + id=group_id) + assert follower_count_after == follower_count_before + 1 # Check that the user's followee count has increased by 1. - params = json.dumps({'id': user_id}) - response = app.post('/api/action/group_followee_count', - params=params).json - assert response['success'] is True - assert response['result'] == followee_count_before + 1 + followee_count_after = ckan.tests.post(app, 'group_followee_count', + id=user_id) + assert followee_count_after == followee_count_before + 1 class TestFollow(object): @@ -345,82 +258,61 @@ def teardown_class(self): def test_01_user_follow_user_bad_apikey(self): for apikey in ('bad api key', '', ' ', 'None', '3', '35.7', 'xxx'): - params = json.dumps({'id': self.russianfan['id']}) - extra_environ = {'Authorization': apikey} - response = self.app.post('/api/action/follow_user', - params=params, extra_environ=extra_environ, status=403).json - assert response['success'] is False - assert response['error']['message'] == 'Access denied' + error = ckan.tests.post(self.app, 'follow_user', + id=self.russianfan['id'], apikey=apikey, + status=403) + assert error['message'] == 'Access denied' def test_01_user_follow_dataset_bad_apikey(self): for apikey in ('bad api key', '', ' ', 'None', '3', '35.7', 'xxx'): - params = json.dumps({'id': self.warandpeace['id']}) - extra_environ = {'Authorization': apikey} - response = self.app.post('/api/action/follow_dataset', - params=params, extra_environ=extra_environ, status=403).json - assert response['success'] is False - assert response['error']['message'] == 'Access denied' + error = ckan.tests.post(self.app, 'follow_dataset', + id=self.warandpeace['id'], apikey=apikey, + status=403) + assert error['message'] == 'Access denied' def test_01_user_follow_group_bad_apikey(self): for apikey in ('bad api key', '', ' ', 'None', '3', '35.7', 'xxx'): - params = json.dumps({'id': self.rogers_group['id']}) - extra_environ = {'Authorization': apikey} - response = self.app.post('/api/action/follow_group', - params=params, extra_environ=extra_environ, status=403).json - assert response['success'] is False - assert response['error']['message'] == 'Access denied' + error = ckan.tests.post(self.app, 'follow_group', + id=self.rogers_group['id'], apikey=apikey, + status=403) + assert error['message'] == 'Access denied' def test_01_user_follow_user_missing_apikey(self): - params = json.dumps({'id': self.russianfan['id']}) - response = self.app.post('/api/action/follow_user', - params=params, status=403).json - assert response['success'] is False - assert response['error']['message'] == 'Access denied' + error = ckan.tests.post(self.app, 'follow_user', + id=self.russianfan['id'], status=403) + assert error['message'] == 'Access denied' def test_01_user_follow_dataset_missing_apikey(self): - params = json.dumps({'id': self.warandpeace['id']}) - response = self.app.post('/api/action/follow_dataset', - params=params, status=403).json - assert response['success'] is False - assert response['error']['message'] == 'Access denied' + error = ckan.tests.post(self.app, 'follow_dataset', + id=self.warandpeace['id'], status=403) + assert error['message'] == 'Access denied' def test_01_user_follow_group_missing_apikey(self): - params = json.dumps({'id': self.rogers_group['id']}) - response = self.app.post('/api/action/follow_group', - params=params, status=403).json - assert response['success'] is False - assert response['error']['message'] == 'Access denied' + error = ckan.tests.post(self.app, 'follow_group', + id=self.rogers_group['id'], status=403) + assert error['message'] == 'Access denied' def test_01_follow_bad_object_id(self): for action in ('follow_user', 'follow_dataset', 'follow_group'): for object_id in ('bad id', ' ', 3, 35.7, 'xxx'): - params = json.dumps({'id': object_id}) - extra_environ = {'Authorization': str(self.annafan['apikey'])} - response = self.app.post('/api/action/{0}'.format(action), - params=params, extra_environ=extra_environ, - status=409).json - assert response['success'] is False - assert response['error']['id'][0].startswith('Not found') + error = ckan.tests.post(self.app, action, + id=object_id, + apikey=self.annafan['apikey'], status=409) + assert error['id'][0].startswith('Not found') def test_01_follow_empty_object_id(self): for action in ('follow_user', 'follow_dataset', 'follow_group'): for object_id in ('', None): - params = json.dumps({'id': object_id}) - extra_environ = {'Authorization': str(self.annafan['apikey'])} - response = self.app.post('/api/action/{0}'.format(action), - params=params, extra_environ=extra_environ, - status=409).json - assert response['success'] is False - assert response['error']['id'] == ['Missing value'] + error = ckan.tests.post(self.app, action, + id=object_id, + apikey=self.annafan['apikey'], status=409) + assert error['id'] == ['Missing value'] def test_01_follow_missing_object_id(self): for action in ('follow_user', 'follow_dataset', 'follow_group'): - params = json.dumps({}) - extra_environ = {'Authorization': str(self.annafan['apikey'])} - response = self.app.post('/api/action/{0}'.format(action), - params=params, extra_environ=extra_environ, status=409).json - assert response['success'] is False - assert response['error']['id'] == ['Missing value'] + error = ckan.tests.post(self.app, action, + apikey=self.annafan['apikey'], status=409) + assert error['id'] == ['Missing value'] def test_02_user_follow_user_by_id(self): follow_user(self.app, self.annafan['id'], self.annafan['apikey'], @@ -449,204 +341,141 @@ def test_02_user_follow_group_by_name(self): def test_03_user_follow_user_already_following(self): for object_id in (self.russianfan['id'], self.russianfan['name'], self.testsysadmin['id'], self.testsysadmin['name']): - params = json.dumps({'id': object_id}) - extra_environ = { - 'Authorization': str(self.annafan['apikey']), - } - response = self.app.post('/api/action/follow_user', - params=params, extra_environ=extra_environ, status=409).json - assert response['success'] == False - assert response['error']['message'].startswith( - 'You are already following ') + error = ckan.tests.post(self.app, 'follow_user', + id=object_id, apikey=self.annafan['apikey'], + status=409) + assert error['message'].startswith('You are already following ') def test_03_user_follow_dataset_already_following(self): for object_id in (self.warandpeace['id'], self.warandpeace['name']): - params = json.dumps({'id': object_id}) - extra_environ = { - 'Authorization': str(self.annafan['apikey']), - } - response = self.app.post('/api/action/follow_dataset', - params=params, extra_environ=extra_environ, status=409).json - assert response['success'] == False - assert response['error']['message'].startswith( - 'You are already following ') + error = ckan.tests.post(self.app, 'follow_dataset', + id=object_id, apikey=self.annafan['apikey'], + status=409) + assert error['message'].startswith('You are already following ') def test_03_user_follow_group_already_following(self): for group_id in (self.rogers_group['id'], self.rogers_group['name']): - params = json.dumps({'id': group_id}) - extra_environ = { - 'Authorization': str(self.annafan['apikey']), - } - response = self.app.post('/api/action/follow_group', - params=params, extra_environ=extra_environ, status=409).json - assert response['success'] is False - assert response['error']['message'].startswith( - 'You are already following ') + error = ckan.tests.post(self.app, 'follow_group', + id=group_id, apikey=self.annafan['apikey'], + status=409) + assert error['message'].startswith('You are already following ') def test_03_user_cannot_follow_herself(self): - params = json.dumps({'id': self.annafan['id']}) - extra_environ = { - 'Authorization': str(self.annafan['apikey']), - } - response = self.app.post('/api/action/follow_user', - params=params, extra_environ=extra_environ, status=409).json - assert response['success'] is False - assert response['error']['message'] == 'You cannot follow yourself' + error = ckan.tests.post(self.app, 'follow_user', + apikey=self.annafan['apikey'], status=409, + id=self.annafan['id']) + assert error['message'] == 'You cannot follow yourself' def test_04_follower_count_bad_id(self): for action in ('user_follower_count', 'dataset_follower_count', 'group_follower_count'): for object_id in ('bad id', ' ', 3, 35.7, 'xxx', ''): - params = json.dumps({'id': object_id}) - response = self.app.post('/api/action/{0}'.format(action), - params=params, status=409).json - assert response['success'] is False - assert 'id' in response['error'] + error = ckan.tests.post(self.app, action, status=409, + id=object_id) + assert 'id' in error def test_04_follower_count_missing_id(self): for action in ('user_follower_count', 'dataset_follower_count', 'group_follower_count'): - params = json.dumps({}) - response = self.app.post('/api/action/{0}'.format(action), - params=params, status=409).json - assert response['success'] is False - assert response['error']['id'] == ['Missing value'] + error = ckan.tests.post(self.app, action, status=409) + assert error['id'] == ['Missing value'] def test_04_user_follower_count_no_followers(self): - params = json.dumps({'id': self.annafan['id']}) - response = self.app.post('/api/action/user_follower_count', - params=params).json - assert response['success'] is True - assert response['result'] == 0 + follower_count = ckan.tests.post(self.app, 'user_follower_count', + id=self.annafan['id']) + assert follower_count == 0 def test_04_dataset_follower_count_no_followers(self): - params = json.dumps({'id': self.annakarenina['id']}) - response = self.app.post('/api/action/dataset_follower_count', - params=params).json - assert response['success'] is True - assert response['result'] == 0 + follower_count = ckan.tests.post(self.app, 'dataset_follower_count', + id=self.annakarenina['id']) + assert follower_count == 0 def test_04_group_follower_count_no_followers(self): - params = json.dumps({'id': self.davids_group['id']}) - response = self.app.post('/api/action/group_follower_count', - params=params).json - assert response['success'] is True - assert response['result'] == 0 + follower_count = ckan.tests.post(self.app, 'group_follower_count', + id=self.davids_group['id']) + assert follower_count == 0 def test_04_follower_list_bad_id(self): for action in ('user_follower_list', 'dataset_follower_list', 'group_follower_list'): for object_id in ('bad id', ' ', 3, 35.7, 'xxx', ''): - params = json.dumps({'id': object_id}) - response = self.app.post('/api/action/{0}'.format(action), - params=params, status=409).json - assert response['success'] is False - assert response['error']['id'] + error = ckan.tests.post(self.app, action, status=409, + id=object_id) + assert error['id'] def test_04_follower_list_missing_id(self): for action in ('user_follower_list', 'dataset_follower_list', 'group_follower_list'): - params = json.dumps({}) - response = self.app.post('/api/action/{0}'.format(action), - params=params, status=409).json - assert response['success'] is False - assert response['error']['id'] == ['Missing value'] + error = ckan.tests.post(self.app, action, status=409) + assert error['id'] == ['Missing value'] def test_04_user_follower_list_no_followers(self): - params = json.dumps({'id': self.annafan['id']}) - response = self.app.post('/api/action/user_follower_list', - params=params).json - assert response['success'] is True - assert response['result'] == [] + followers = ckan.tests.post(self.app, 'user_follower_list', + id=self.annafan['id']) + assert followers == [] def test_04_dataset_follower_list_no_followers(self): - params = json.dumps({'id': self.annakarenina['id']}) - response = self.app.post('/api/action/dataset_follower_list', - params=params).json - assert response['success'] is True - assert response['result'] == [] + followers = ckan.tests.post(self.app, 'dataset_follower_list', + id=self.annakarenina['id']) + assert followers == [] def test_04_group_follower_list_no_followers(self): - params = json.dumps({'id': self.davids_group['id']}) - response = self.app.post('/api/action/group_follower_list', - params=params).json - assert response['success'] is True - assert response['result'] == [] + followers = ckan.tests.post(self.app, 'group_follower_list', + id=self.davids_group['id']) + assert followers == [] def test_04_am_following_bad_id(self): for action in ('am_following_dataset', 'am_following_user', 'am_following_group'): for object_id in ('bad id', ' ', 3, 35.7, 'xxx'): - params = json.dumps({'id': object_id}) - extra_environ = {'Authorization': str(self.annafan['apikey'])} - response = self.app.post('/api/action/{0}'.format(action), - params=params, extra_environ=extra_environ, - status=409).json - assert response['success'] is False - assert response['error']['id'][0].startswith('Not found: ') + error = ckan.tests.post(self.app, action, + apikey=self.annafan['apikey'], status=409, id=object_id) + assert error['id'][0].startswith('Not found: ') def test_04_am_following_missing_id(self): for action in ('am_following_dataset', 'am_following_user', 'am_following_group'): for id in ('missing', None, ''): if id == 'missing': - params = json.dumps({}) + error = ckan.tests.post(self.app, action, + apikey=self.annafan['apikey'], status=409) else: - params = json.dumps({'id':id}) - extra_environ = {'Authorization': str(self.annafan['apikey'])} - response = self.app.post('/api/action/{0}'.format(action), - params=params, extra_environ=extra_environ, - status=409).json - assert response['success'] is False - assert response['error']['id'] == [u'Missing value'] + error = ckan.tests.post(self.app, action, + apikey=self.annafan['apikey'], status=409, id=id) + assert error['id'] == [u'Missing value'] def test_04_am_following_dataset_bad_apikey(self): for apikey in ('bad api key', '', ' ', 'None', '3', '35.7', 'xxx'): - params = json.dumps({'id': self.warandpeace['id']}) - extra_environ = {'Authorization': apikey} - response = self.app.post('/api/action/am_following_dataset', - params=params, extra_environ=extra_environ, status=403).json - assert response['success'] == False - assert response['error']['message'] == 'Access denied' + error = ckan.tests.post(self.app, 'am_following_dataset', + apikey=apikey, status=403, id=self.warandpeace['id']) + assert error['message'] == 'Access denied' def test_04_am_following_dataset_missing_apikey(self): - params = json.dumps({'id': self.warandpeace['id']}) - response = self.app.post('/api/action/am_following_dataset', - params=params, status=403).json - assert response['success'] == False - assert response['error']['message'] == 'Access denied' + error = ckan.tests.post(self.app, 'am_following_dataset', status=403, + id=self.warandpeace['id']) + assert error['message'] == 'Access denied' def test_04_am_following_user_bad_apikey(self): for apikey in ('bad api key', '', ' ', 'None', '3', '35.7', 'xxx'): - params = json.dumps({'id': self.annafan['id']}) - extra_environ = {'Authorization': apikey} - response = self.app.post('/api/action/am_following_user', - params=params, extra_environ=extra_environ, status=403).json - assert response['success'] == False - assert response['error']['message'] == 'Access denied' + error = ckan.tests.post(self.app, 'am_following_user', + apikey=apikey, status=403, id=self.annafan['id']) + assert error['message'] == 'Access denied' def test_04_am_following_user_missing_apikey(self): - params = json.dumps({'id': self.annafan['id']}) - response = self.app.post('/api/action/am_following_user', - params=params, status=403).json - assert response['success'] == False - assert response['error']['message'] == 'Access denied' + error = ckan.tests.post(self.app, 'am_following_user', status=403, + id=self.annafan['id']) + assert error['message'] == 'Access denied' def test_04_am_following_group_bad_apikey(self): for apikey in ('bad api key', '', ' ', 'None', '3', '35.7', 'xxx'): - params = json.dumps({'id': self.rogers_group['id']}) - extra_environ = {'Authorization': apikey} - response = self.app.post('/api/action/am_following_group', - params=params, extra_environ=extra_environ, status=403).json - assert response['success'] == False - assert response['error']['message'] == 'Access denied' + error = ckan.tests.post(self.app, 'am_following_group', + apikey=apikey, status=403, id=self.rogers_group['id']) + assert error['message'] == 'Access denied' def test_04_am_following_group_missing_apikey(self): - params = json.dumps({'id': self.rogers_group['id']}) - response = self.app.post('/api/action/am_following_group', - params=params, status=403).json - assert response['success'] == False - assert response['error']['message'] == 'Access denied' + error = ckan.tests.post(self.app, 'am_following_group', status=403, + id=self.rogers_group['id']) + assert error['message'] == 'Access denied' class TestFollowerDelete(object): @@ -729,45 +558,30 @@ def test_01_unfollow_user_not_exists(self): she is not following. ''' - params = json.dumps({'id': self.russianfan['id']}) - extra_environ = { - 'Authorization': str(self.annafan['apikey']), - } - response = self.app.post('/api/action/unfollow_user', - params=params, extra_environ=extra_environ, status=404).json - assert response['success'] == False - assert response['error']['message'].startswith( - 'Not found: You are not following ') + error = ckan.tests.post(self.app, 'unfollow_user', + apikey=self.annafan['apikey'], status=404, + id=self.russianfan['id']) + assert error['message'].startswith('Not found: You are not following ') def test_01_unfollow_dataset_not_exists(self): '''Test the error response when a user tries to unfollow a dataset that she is not following. ''' - params = json.dumps({'id': self.annakarenina['id']}) - extra_environ = { - 'Authorization': str(self.annafan['apikey']), - } - response = self.app.post('/api/action/unfollow_dataset', - params=params, extra_environ=extra_environ, status=404).json - assert response['success'] == False - assert response['error']['message'].startswith( - 'Not found: You are not following ') + error = ckan.tests.post(self.app, 'unfollow_dataset', + apikey=self.annafan['apikey'], status=404, + id=self.annakarenina['id']) + assert error['message'].startswith('Not found: You are not following') def test_01_unfollow_group_not_exists(self): '''Test the error response when a user tries to unfollow a group that she is not following. ''' - params = json.dumps({'id': self.rogers_group['id']}) - extra_environ = { - 'Authorization': str(self.annafan['apikey']), - } - response = self.app.post('/api/action/unfollow_group', - params=params, extra_environ=extra_environ, status=404).json - assert response['success'] is False - assert response['error']['message'].startswith( - 'Not found: You are not following ') + error = ckan.tests.post(self.app, 'unfollow_group', + apikey=self.annafan['apikey'], status=404, + id=self.rogers_group['id']) + assert error['message'].startswith('Not found: You are not following') def test_01_unfollow_bad_apikey(self): '''Test the error response when a user tries to unfollow something @@ -777,58 +591,36 @@ def test_01_unfollow_bad_apikey(self): for action in ('unfollow_user', 'unfollow_dataset', 'unfollow_group'): for apikey in ('bad api key', '', ' ', 'None', '3', '35.7', 'xxx'): - params = json.dumps({ - 'id': self.joeadmin['id'], - }) - extra_environ = { - 'Authorization': apikey, - } - response = self.app.post('/api/action/{0}'.format(action), - params=params, extra_environ=extra_environ, - status=(403,409)).json - assert response['success'] is False - assert response['error']['message'] == 'Access denied' + error = ckan.tests.post(self.app, action, apikey=apikey, + status=403, id=self.joeadmin['id']) + assert error['message'] == 'Access denied' def test_01_unfollow_missing_apikey(self): '''Test error response when calling unfollow_* without api key.''' for action in ('unfollow_user', 'unfollow_dataset', 'unfollow_group'): - params = json.dumps({ - 'id': self.joeadmin['id'], - }) - response = self.app.post('/api/action/{0}'.format(action), - params=params, status=403).json - assert response['success'] is False - assert response['error']['message'] == 'Access denied' + error = ckan.tests.post(self.app, action, status=403, + id=self.joeadmin['id']) + assert error['message'] == 'Access denied' def test_01_unfollow_bad_object_id(self): '''Test error response when calling unfollow_* with bad object id.''' for action in ('unfollow_user', 'unfollow_dataset', 'unfollow_group'): for object_id in ('bad id', ' ', 3, 35.7, 'xxx'): - params = json.dumps({ - 'id': object_id, - }) - extra_environ = { - 'Authorization': str(self.annafan['apikey']), - } - response = self.app.post('/api/action/{0}'.format(action), - params=params, extra_environ=extra_environ, - status=409).json - assert response['success'] is False - assert response['error']['id'][0].startswith('Not found') + error = ckan.tests.post(self.app, action, + apikey=self.annafan['apikey'], status=409, + id=object_id) + assert error['id'][0].startswith('Not found') def test_01_unfollow_missing_object_id(self): for action in ('unfollow_user', 'unfollow_dataset', 'unfollow_group'): for id in ('missing', None, ''): if id == 'missing': - params = json.dumps({}) + error = ckan.tests.post(self.app, action, + apikey=self.annafan['apikey'], status=409) else: - params = json.dumps({'id': id}) - extra_environ = {'Authorization': str(self.annafan['apikey'])} - response = self.app.post('/api/action/{0}'.format(action), - params=params, extra_environ=extra_environ, - status=409).json - assert response['success'] is False - assert response['error']['id'] == [u'Missing value'] + error = ckan.tests.post(self.app, action, + apikey=self.annafan['apikey'], status=409, id=id) + assert error['id'] == [u'Missing value'] def _unfollow_user(self, follower_id, apikey, object_id, object_arg): '''Test a user unfollowing a user via the API. @@ -841,53 +633,33 @@ def _unfollow_user(self, follower_id, apikey, object_id, object_arg): ''' # Record the user's number of followers before. - params = json.dumps({'id': object_id}) - response = self.app.post('/api/action/user_follower_count', - params=params).json - assert response['success'] is True - count_before = response['result'] + count_before = ckan.tests.post(self.app, 'user_follower_count', + id=object_id) # Check that the user is following the object. - params = json.dumps({'id': object_id}) - extra_environ = {'Authorization': str(apikey)} - response = self.app.post('/api/action/am_following_user', - params=params, extra_environ=extra_environ).json - assert response['success'] is True - assert response['result'] is True + am_following = ckan.tests.post(self.app, 'am_following_user', + apikey=apikey, id=object_id) + assert am_following is True # Make the user unfollow the object. - params = { - 'id': object_arg, - } - extra_environ = {'Authorization': str(apikey)} - response = self.app.post('/api/action/unfollow_user', - params=json.dumps(params), extra_environ=extra_environ).json - assert response['success'] is True + ckan.tests.post(self.app, 'unfollow_user', apikey=apikey, + id=object_arg) # Check that am_following_user now returns False. - params = json.dumps({'id': object_id}) - extra_environ = {'Authorization': str(apikey)} - response = self.app.post('/api/action/am_following_user', - params=params, extra_environ=extra_environ).json - assert response['success'] is True - assert response['result'] is False + am_following = ckan.tests.post(self.app, 'am_following_user', + apikey=apikey, id=object_id) + assert am_following is False # Check that the user doesn't appear in the object's list of followers. - params = json.dumps({'id': object_id}) - response = self.app.post('/api/action/user_follower_list', - params=params).json - assert response['success'] is True - assert response['result'] - followers = response['result'] + followers = ckan.tests.post(self.app, 'user_follower_list', + id=object_id) assert len([follower for follower in followers if follower['id'] == follower_id]) == 0 # Check that the object's follower count has decreased by 1. - params = json.dumps({'id': object_id}) - response = self.app.post('/api/action/user_follower_count', - params=params).json - assert response['success'] is True - assert response['result'] == count_before - 1 + count_after = ckan.tests.post(self.app, 'user_follower_count', + id=object_id) + assert count_after == count_before - 1 def _unfollow_dataset(self, user_id, apikey, dataset_id, dataset_arg): '''Test a user unfollowing a dataset via the API. @@ -900,54 +672,34 @@ def _unfollow_dataset(self, user_id, apikey, dataset_id, dataset_arg): ''' # Record the dataset's number of followers before. - params = json.dumps({'id': dataset_id}) - response = self.app.post('/api/action/dataset_follower_count', - params=params).json - assert response['success'] is True - count_before = response['result'] + count_before = ckan.tests.post(self.app, 'dataset_follower_count', + id=dataset_id) # Check that the user is following the dataset. - params = json.dumps({'id': dataset_id}) - extra_environ = {'Authorization': str(apikey)} - response = self.app.post('/api/action/am_following_dataset', - params=params, extra_environ=extra_environ).json - assert response['success'] is True - assert response['result'] is True + am_following = ckan.tests.post(self.app, 'am_following_dataset', + apikey=apikey, id=dataset_id) + assert am_following is True # Make the user unfollow the dataset. - params = { - 'id': dataset_arg, - } - extra_environ = {'Authorization': str(apikey)} - response = self.app.post('/api/action/unfollow_dataset', - params=json.dumps(params), extra_environ=extra_environ).json - assert response['success'] is True + ckan.tests.post(self.app, 'unfollow_dataset', apikey=apikey, + id=dataset_arg) # Check that am_following_dataset now returns False. - params = json.dumps({'id': dataset_id}) - extra_environ = {'Authorization': str(apikey)} - response = self.app.post('/api/action/am_following_dataset', - params=params, extra_environ=extra_environ).json - assert response['success'] is True - assert response['result'] is False + am_following = ckan.tests.post(self.app, 'am_following_dataset', + apikey=apikey, id=dataset_id) + assert am_following is False # Check that the user doesn't appear in the dataset's list of # followers. - params = json.dumps({'id': dataset_id}) - response = self.app.post('/api/action/dataset_follower_list', - params=params).json - assert response['success'] is True - assert response['result'] - followers = response['result'] + followers = ckan.tests.post(self.app, 'dataset_follower_list', + id=dataset_id) assert len([follower for follower in followers if follower['id'] == user_id]) == 0 # Check that the dataset's follower count has decreased by 1. - params = json.dumps({'id': dataset_id}) - response = self.app.post('/api/action/dataset_follower_count', - params=params).json - assert response['success'] is True - assert response['result'] == count_before - 1 + count_after = ckan.tests.post(self.app, 'dataset_follower_count', + id=dataset_id) + assert count_after == count_before - 1 def _unfollow_group(self, user_id, apikey, group_id, group_arg): '''Test a user unfollowing a group via the API. @@ -960,54 +712,34 @@ def _unfollow_group(self, user_id, apikey, group_id, group_arg): ''' # Record the group's number of followers before. - params = json.dumps({'id': group_id}) - response = self.app.post('/api/action/group_follower_count', - params=params).json - assert response['success'] is True - count_before = response['result'] + count_before = ckan.tests.post(self.app, 'group_follower_count', + id=group_id) # Check that the user is following the group. - params = json.dumps({'id': group_id}) - extra_environ = {'Authorization': str(apikey)} - response = self.app.post('/api/action/am_following_group', - params=params, extra_environ=extra_environ).json - assert response['success'] is True - assert response['result'] is True + am_following = ckan.tests.post(self.app, 'am_following_group', + apikey=apikey, id=group_id) + assert am_following is True # Make the user unfollow the group. - params = { - 'id': group_arg, - } - extra_environ = {'Authorization': str(apikey)} - response = self.app.post('/api/action/unfollow_group', - params=json.dumps(params), extra_environ=extra_environ).json - assert response['success'] is True + ckan.tests.post(self.app, 'unfollow_group', apikey=apikey, + id=group_arg) # Check that am_following_group now returns False. - params = json.dumps({'id': group_id}) - extra_environ = {'Authorization': str(apikey)} - response = self.app.post('/api/action/am_following_group', - params=params, extra_environ=extra_environ).json - assert response['success'] is True - assert response['result'] is False + am_following = ckan.tests.post(self.app, 'am_following_group', + apikey=apikey, id=group_id) + assert am_following is False # Check that the user doesn't appear in the group's list of # followers. - params = json.dumps({'id': group_id}) - response = self.app.post('/api/action/group_follower_list', - params=params).json - assert response['success'] is True - assert 'result' in response - followers = response['result'] + followers = ckan.tests.post(self.app, 'group_follower_list', + id=group_id) assert len([follower for follower in followers if follower['id'] == user_id]) == 0 # Check that the group's follower count has decreased by 1. - params = json.dumps({'id': group_id}) - response = self.app.post('/api/action/group_follower_count', - params=params).json - assert response['success'] is True - assert response['result'] == count_before - 1 + count_after = ckan.tests.post(self.app, 'group_follower_count', + id=group_id) + assert count_after == count_before - 1 def test_02_follower_delete_by_id(self): self._unfollow_user(self.annafan['id'], self.annafan['apikey'], @@ -1114,136 +846,93 @@ def test_01_on_delete_cascade_api(self): ''' # It should no longer be possible to get joeadmin's follower list. - params = json.dumps({'id': 'joeadmin'}) - response = self.app.post('/api/action/user_follower_list', - params=params, status=409).json - assert response['success'] is False - assert response['error'].has_key('id') + error = ckan.tests.post(self.app, 'user_follower_list', status=409, + id='joeadmin') + assert 'id' in error # It should no longer be possible to get warandpeace's follower list. - params = json.dumps({'id': 'warandpeace'}) - response = self.app.post('/api/action/dataset_follower_list', - params=params, status=409).json - assert response['success'] is False - assert response['error'].has_key('id') + error = ckan.tests.post(self.app, 'dataset_follower_list', status=409, + id='warandpeace') + assert 'id' in error # It should no longer be possible to get david's follower list. - params = json.dumps({'id': 'david'}) - response = self.app.post('/api/action/group_follower_list', - params=params, status=409).json - assert response['success'] is False - assert response['error'].has_key('id') + error = ckan.tests.post(self.app, 'group_follower_list', status=409, + id='david') + assert 'id' in error # It should no longer be possible to get joeadmin's follower count. - params = json.dumps({'id': 'joeadmin'}) - response = self.app.post('/api/action/user_follower_count', - params=params, status=409).json - assert response['success'] is False - assert response['error'].has_key('id') + error = ckan.tests.post(self.app, 'user_follower_count', status=409, + id='joeadmin') + assert 'id' in error # It should no longer be possible to get warandpeace's follower count. - params = json.dumps({'id': 'warandpeace'}) - response = self.app.post('/api/action/dataset_follower_count', - params=params, status=409).json - assert response['success'] is False - assert response['error'].has_key('id') + error = ckan.tests.post(self.app, 'dataset_follower_count', status=409, + id='warandpeace') + assert 'id' in error # It should no longer be possible to get david's follower count. - params = json.dumps({'id': 'david'}) - response = self.app.post('/api/action/group_follower_count', - params=params, status=409).json - assert response['success'] is False - assert response['error'].has_key('id') + error = ckan.tests.post(self.app, 'group_follower_count', status=409, + id='david') + assert 'id' in error # It should no longer be possible to get am_following for joeadmin. - params = json.dumps({'id': 'joeadmin'}) - extra_environ = {'Authorization': str(self.testsysadmin['apikey'])} - response = self.app.post('/api/action/am_following_user', - params=params, extra_environ=extra_environ, status=409).json - assert response['success'] is False - assert response['error'].has_key('id') + error = ckan.tests.post(self.app, 'am_following_user', + apikey=self.testsysadmin['apikey'], status=409, id='joeadmin') + assert 'id' in error # It should no longer be possible to get am_following for warandpeace. - params = json.dumps({'id': 'warandpeace'}) - extra_environ = {'Authorization': str(self.testsysadmin['apikey'])} - response = self.app.post('/api/action/am_following_dataset', - params=params, extra_environ=extra_environ, status=409).json - assert response['success'] is False - assert response['error'].has_key('id') + error = ckan.tests.post(self.app, 'am_following_dataset', + apikey=self.testsysadmin['apikey'], status=409, + id='warandpeace') + assert 'id' in error # It should no longer be possible to get am_following for david. - params = json.dumps({'id': 'david'}) - extra_environ = {'Authorization': str(self.testsysadmin['apikey'])} - response = self.app.post('/api/action/am_following_group', - params=params, extra_environ=extra_environ, status=409).json - assert response['success'] is False - assert response['error'].has_key('id') + error = ckan.tests.post(self.app, 'am_following_group', + apikey=self.testsysadmin['apikey'], status=409, id='david') + assert 'id' in error # It should no longer be possible to unfollow joeadmin. - params = json.dumps({'id': 'joeadmin'}) - extra_environ = {'Authorization': str(self.tester['apikey'])} - response = self.app.post('/api/action/unfollow_user', - params=params, extra_environ=extra_environ, status=409).json - assert response['success'] is False - assert response['error']['id'] == ['Not found: User'] + error = ckan.tests.post(self.app, 'unfollow_user', + apikey=self.tester['apikey'], status=409, id='joeadmin') + assert error['id'] == ['Not found: User'] # It should no longer be possible to unfollow warandpeace. - params = json.dumps({'id': 'warandpeace'}) - extra_environ = {'Authorization': str(self.testsysadmin['apikey'])} - response = self.app.post('/api/action/unfollow_dataset', - params=params, extra_environ=extra_environ, status=409).json - assert response['success'] is False - assert response['error']['id'] == ['Not found: Dataset'] + error = ckan.tests.post(self.app, 'unfollow_dataset', + apikey=self.testsysadmin['apikey'], status=409, + id='warandpeace') + assert error['id'] == ['Not found: Dataset'] # It should no longer be possible to unfollow david. - params = json.dumps({'id': 'david'}) - extra_environ = {'Authorization': str(self.testsysadmin['apikey'])} - response = self.app.post('/api/action/unfollow_group', - params=params, extra_environ=extra_environ, status=409).json - assert response['success'] is False - assert response['error']['id'] == ['Not found: Group'] + error = ckan.tests.post(self.app, 'unfollow_group', + apikey=self.testsysadmin['apikey'], status=409, id='david') + assert error['id'] == ['Not found: Group'] # It should no longer be possible to follow joeadmin. - params = json.dumps({'id': 'joeadmin'}) - extra_environ = {'Authorization': str(self.annafan['apikey'])} - response = self.app.post('/api/action/follow_user', - params=params, extra_environ=extra_environ, status=409).json - assert response['success'] is False - assert response['error'].has_key('id') + error = ckan.tests.post(self.app, 'follow_user', + apikey=self.annafan['apikey'], status=409, id='joeadmin') + assert 'id' in error # It should no longer be possible to follow warandpeace. - params = json.dumps({'id': 'warandpeace'}) - extra_environ = {'Authorization': str(self.annafan['apikey'])} - response = self.app.post('/api/action/follow_dataset', - params=params, extra_environ=extra_environ, status=409).json - assert response['success'] is False - assert response['error'].has_key('id') + error = ckan.tests.post(self.app, 'follow_dataset', + apikey=self.annafan['apikey'], status=409, id='warandpeace') + assert 'id' in error # It should no longer be possible to follow david. - params = json.dumps({'id': 'david'}) - extra_environ = {'Authorization': str(self.annafan['apikey'])} - response = self.app.post('/api/action/follow_group', - params=params, extra_environ=extra_environ, status=409).json - assert response['success'] is False - assert response['error'].has_key('id') + error = ckan.tests.post(self.app, 'follow_group', + apikey=self.annafan['apikey'], status=409, id='david') + assert 'id' in error # Users who joeadmin was following should no longer have him in their # follower list. - params = json.dumps({'id': self.testsysadmin['id']}) - response = self.app.post('/api/action/user_follower_list', - params=params).json - assert response['success'] is True - followers = [follower['name'] for follower in response['result']] - assert 'joeadmin' not in followers + followers = ckan.tests.post(self.app, 'user_follower_list', + id=self.testsysadmin['id']) + assert 'joeadmin' not in [follower['name'] for follower in followers] # Datasets who joeadmin was following should no longer have him in # their follower list. - params = json.dumps({'id': self.annakarenina['id']}) - response = self.app.post('/api/action/dataset_follower_list', - params=params).json - assert response['success'] is True - followers = [follower['name'] for follower in response['result']] - assert 'joeadmin' not in followers + followers = ckan.tests.post(self.app, 'dataset_follower_list', + id=self.annakarenina['id']) + assert 'joeadmin' not in [follower['name'] for follower in followers] def test_02_on_delete_cascade_db(self): if not are_foreign_keys_supported(): From 740a0d887021abad60324ad421f06c36ffaa6f93 Mon Sep 17 00:00:00 2001 From: Sean Hammond Date: Wed, 28 Nov 2012 17:30:40 +0100 Subject: [PATCH 27/27] Rename post -> call_action_api More descriptive name --- ckan/tests/__init__.py | 4 +- ckan/tests/functional/api/test_follow.py | 291 ++++++++++++----------- 2 files changed, 148 insertions(+), 147 deletions(-) diff --git a/ckan/tests/__init__.py b/ckan/tests/__init__.py index 4058b79ad7c..0d4be8b4e49 100644 --- a/ckan/tests/__init__.py +++ b/ckan/tests/__init__.py @@ -401,8 +401,8 @@ class StatusCodes: STATUS_409_CONFLICT = 409 -def post(app, action, apikey=None, status=200, **kwargs): - '''Post to the CKAN API and return the result. +def call_action_api(app, action, apikey=None, status=200, **kwargs): + '''POST an HTTP request to the CKAN API and return the result. Any additional keyword arguments that you pass to this function as **kwargs are posted as params to the API. diff --git a/ckan/tests/functional/api/test_follow.py b/ckan/tests/functional/api/test_follow.py index 60ee6c5e6c9..90bc4a23101 100644 --- a/ckan/tests/functional/api/test_follow.py +++ b/ckan/tests/functional/api/test_follow.py @@ -36,21 +36,21 @@ def follow_user(app, follower_id, apikey, object_id, object_arg): ''' # Record the object's followers count before. - follower_count_before = ckan.tests.post(app, 'user_follower_count', - id=object_id) + follower_count_before = ckan.tests.call_action_api(app, + 'user_follower_count', id=object_id) # Record the follower's followees count before. - followee_count_before = ckan.tests.post(app, 'user_followee_count', - id=follower_id) + followee_count_before = ckan.tests.call_action_api(app, + 'user_followee_count', id=follower_id) # Check that the user is not already following the object. - result = ckan.tests.post(app, 'am_following_user', + result = ckan.tests.call_action_api(app, 'am_following_user', id=object_id, apikey=apikey) assert result is False # Make the user start following the object. before = datetime.datetime.now() - follower = ckan.tests.post(app, 'follow_user', id=object_arg, + follower = ckan.tests.call_action_api(app, 'follow_user', id=object_arg, apikey=apikey) after = datetime.datetime.now() assert follower['follower_id'] == follower_id @@ -59,30 +59,30 @@ def follow_user(app, follower_id, apikey, object_id, object_arg): assert (timestamp >= before and timestamp <= after), str(timestamp) # Check that am_following_user now returns True. - result = ckan.tests.post(app, 'am_following_user', + result = ckan.tests.call_action_api(app, 'am_following_user', id=object_id, apikey=apikey) assert result is True # Check that the follower appears in the object's list of followers. - followers = ckan.tests.post(app, 'user_follower_list', + followers = ckan.tests.call_action_api(app, 'user_follower_list', id=object_id) assert len(followers) == follower_count_before + 1 assert len([follower for follower in followers if follower['id'] == follower_id]) == 1 # Check that the object appears in the follower's list of followees. - followees = ckan.tests.post(app, 'user_followee_list', + followees = ckan.tests.call_action_api(app, 'user_followee_list', id=follower_id) assert len(followees) == followee_count_before + 1 assert len([followee for followee in followees if followee['id'] == object_id]) == 1 # Check that the object's follower count has increased by 1. - follower_count_after = ckan.tests.post(app, 'user_follower_count', - id=object_id) + follower_count_after = ckan.tests.call_action_api(app, + 'user_follower_count', id=object_id) assert follower_count_after == follower_count_before + 1 # Check that the follower's followee count has increased by 1. - followee_count_after = ckan.tests.post(app, 'user_followee_count', - id=follower_id) + followee_count_after = ckan.tests.call_action_api(app, + 'user_followee_count', id=follower_id) assert followee_count_after == followee_count_before + 1 def follow_dataset(app, follower_id, apikey, dataset_id, dataset_arg): @@ -96,21 +96,21 @@ def follow_dataset(app, follower_id, apikey, dataset_id, dataset_arg): ''' # Record the dataset's followers count before. - follower_count_before = ckan.tests.post(app, 'dataset_follower_count', - id=dataset_id) + follower_count_before = ckan.tests.call_action_api(app, + 'dataset_follower_count', id=dataset_id) # Record the follower's followees count before. - followee_count_before = ckan.tests.post(app, 'dataset_followee_count', - id=follower_id) + followee_count_before = ckan.tests.call_action_api(app, + 'dataset_followee_count', id=follower_id) # Check that the user is not already following the dataset. - result = ckan.tests.post(app, 'am_following_dataset', + result = ckan.tests.call_action_api(app, 'am_following_dataset', id=dataset_id, apikey=apikey) assert result is False # Make the user start following the dataset. before = datetime.datetime.now() - follower = ckan.tests.post(app, 'follow_dataset', + follower = ckan.tests.call_action_api(app, 'follow_dataset', id=dataset_arg, apikey=apikey) after = datetime.datetime.now() assert follower['follower_id'] == follower_id @@ -119,30 +119,30 @@ def follow_dataset(app, follower_id, apikey, dataset_id, dataset_arg): assert (timestamp >= before and timestamp <= after), str(timestamp) # Check that am_following_dataset now returns True. - result = ckan.tests.post(app, 'am_following_dataset', + result = ckan.tests.call_action_api(app, 'am_following_dataset', id=dataset_id, apikey=apikey) assert result is True # Check that the follower appears in the dataset's list of followers. - followers = ckan.tests.post(app, 'dataset_follower_list', + followers = ckan.tests.call_action_api(app, 'dataset_follower_list', id=dataset_id) assert len(followers) == follower_count_before + 1 assert len([follower for follower in followers if follower['id'] == follower_id]) == 1 # Check that the dataset appears in the follower's list of followees. - followees = ckan.tests.post(app, 'dataset_followee_list', + followees = ckan.tests.call_action_api(app, 'dataset_followee_list', id=follower_id) assert len(followees) == followee_count_before + 1 assert len([followee for followee in followees if followee['id'] == dataset_id]) == 1 # Check that the dataset's follower count has increased by 1. - follower_count_after = ckan.tests.post(app, 'dataset_follower_count', - id=dataset_id) + follower_count_after = ckan.tests.call_action_api(app, + 'dataset_follower_count', id=dataset_id) assert follower_count_after == follower_count_before + 1 # Check that the follower's followee count has increased by 1. - followee_count_after = ckan.tests.post(app, 'dataset_followee_count', - id=follower_id) + followee_count_after = ckan.tests.call_action_api(app, + 'dataset_followee_count', id=follower_id) assert followee_count_after == followee_count_before + 1 def follow_group(app, user_id, apikey, group_id, group_arg): @@ -156,21 +156,21 @@ def follow_group(app, user_id, apikey, group_id, group_arg): ''' # Record the group's followers count before. - follower_count_before = ckan.tests.post(app, 'group_follower_count', - id=group_id) + follower_count_before = ckan.tests.call_action_api(app, + 'group_follower_count', id=group_id) # Record the user's followees count before. - followee_count_before = ckan.tests.post(app, 'group_followee_count', - id=user_id) + followee_count_before = ckan.tests.call_action_api(app, + 'group_followee_count', id=user_id) # Check that the user is not already following the group. - result = ckan.tests.post(app, 'am_following_group', + result = ckan.tests.call_action_api(app, 'am_following_group', id=group_id, apikey=apikey) assert result is False # Make the user start following the group. before = datetime.datetime.now() - follower = ckan.tests.post(app, 'follow_group', id=group_id, + follower = ckan.tests.call_action_api(app, 'follow_group', id=group_id, apikey=apikey) after = datetime.datetime.now() assert follower['follower_id'] == user_id @@ -179,32 +179,32 @@ def follow_group(app, user_id, apikey, group_id, group_arg): assert (timestamp >= before and timestamp <= after), str(timestamp) # Check that am_following_group now returns True. - result = ckan.tests.post(app, 'am_following_group', + result = ckan.tests.call_action_api(app, 'am_following_group', id=group_id, apikey=apikey) assert result is True # Check that the user appears in the group's list of followers. - followers = ckan.tests.post(app, 'group_follower_list', + followers = ckan.tests.call_action_api(app, 'group_follower_list', id=group_id) assert len(followers) == follower_count_before + 1 assert len([follower for follower in followers if follower['id'] == user_id]) == 1 # Check that the group appears in the user's list of followees. - followees = ckan.tests.post(app, 'group_followee_list', + followees = ckan.tests.call_action_api(app, 'group_followee_list', id=user_id) assert len(followees) == followee_count_before + 1 assert len([followee for followee in followees if followee['id'] == group_id]) == 1 # Check that the group's follower count has increased by 1. - follower_count_after = ckan.tests.post(app, 'group_follower_count', - id=group_id) + follower_count_after = ckan.tests.call_action_api(app, + 'group_follower_count', id=group_id) assert follower_count_after == follower_count_before + 1 # Check that the user's followee count has increased by 1. - followee_count_after = ckan.tests.post(app, 'group_followee_count', - id=user_id) + followee_count_after = ckan.tests.call_action_api(app, + 'group_followee_count', id=user_id) assert followee_count_after == followee_count_before + 1 @@ -258,44 +258,44 @@ def teardown_class(self): def test_01_user_follow_user_bad_apikey(self): for apikey in ('bad api key', '', ' ', 'None', '3', '35.7', 'xxx'): - error = ckan.tests.post(self.app, 'follow_user', + error = ckan.tests.call_action_api(self.app, 'follow_user', id=self.russianfan['id'], apikey=apikey, status=403) assert error['message'] == 'Access denied' def test_01_user_follow_dataset_bad_apikey(self): for apikey in ('bad api key', '', ' ', 'None', '3', '35.7', 'xxx'): - error = ckan.tests.post(self.app, 'follow_dataset', + error = ckan.tests.call_action_api(self.app, 'follow_dataset', id=self.warandpeace['id'], apikey=apikey, status=403) assert error['message'] == 'Access denied' def test_01_user_follow_group_bad_apikey(self): for apikey in ('bad api key', '', ' ', 'None', '3', '35.7', 'xxx'): - error = ckan.tests.post(self.app, 'follow_group', + error = ckan.tests.call_action_api(self.app, 'follow_group', id=self.rogers_group['id'], apikey=apikey, status=403) assert error['message'] == 'Access denied' def test_01_user_follow_user_missing_apikey(self): - error = ckan.tests.post(self.app, 'follow_user', + error = ckan.tests.call_action_api(self.app, 'follow_user', id=self.russianfan['id'], status=403) assert error['message'] == 'Access denied' def test_01_user_follow_dataset_missing_apikey(self): - error = ckan.tests.post(self.app, 'follow_dataset', + error = ckan.tests.call_action_api(self.app, 'follow_dataset', id=self.warandpeace['id'], status=403) assert error['message'] == 'Access denied' def test_01_user_follow_group_missing_apikey(self): - error = ckan.tests.post(self.app, 'follow_group', + error = ckan.tests.call_action_api(self.app, 'follow_group', id=self.rogers_group['id'], status=403) assert error['message'] == 'Access denied' def test_01_follow_bad_object_id(self): for action in ('follow_user', 'follow_dataset', 'follow_group'): for object_id in ('bad id', ' ', 3, 35.7, 'xxx'): - error = ckan.tests.post(self.app, action, + error = ckan.tests.call_action_api(self.app, action, id=object_id, apikey=self.annafan['apikey'], status=409) assert error['id'][0].startswith('Not found') @@ -303,14 +303,14 @@ def test_01_follow_bad_object_id(self): def test_01_follow_empty_object_id(self): for action in ('follow_user', 'follow_dataset', 'follow_group'): for object_id in ('', None): - error = ckan.tests.post(self.app, action, + error = ckan.tests.call_action_api(self.app, action, id=object_id, apikey=self.annafan['apikey'], status=409) assert error['id'] == ['Missing value'] def test_01_follow_missing_object_id(self): for action in ('follow_user', 'follow_dataset', 'follow_group'): - error = ckan.tests.post(self.app, action, + error = ckan.tests.call_action_api(self.app, action, apikey=self.annafan['apikey'], status=409) assert error['id'] == ['Missing value'] @@ -341,27 +341,27 @@ def test_02_user_follow_group_by_name(self): def test_03_user_follow_user_already_following(self): for object_id in (self.russianfan['id'], self.russianfan['name'], self.testsysadmin['id'], self.testsysadmin['name']): - error = ckan.tests.post(self.app, 'follow_user', + error = ckan.tests.call_action_api(self.app, 'follow_user', id=object_id, apikey=self.annafan['apikey'], status=409) assert error['message'].startswith('You are already following ') def test_03_user_follow_dataset_already_following(self): for object_id in (self.warandpeace['id'], self.warandpeace['name']): - error = ckan.tests.post(self.app, 'follow_dataset', + error = ckan.tests.call_action_api(self.app, 'follow_dataset', id=object_id, apikey=self.annafan['apikey'], status=409) assert error['message'].startswith('You are already following ') def test_03_user_follow_group_already_following(self): for group_id in (self.rogers_group['id'], self.rogers_group['name']): - error = ckan.tests.post(self.app, 'follow_group', + error = ckan.tests.call_action_api(self.app, 'follow_group', id=group_id, apikey=self.annafan['apikey'], status=409) assert error['message'].startswith('You are already following ') def test_03_user_cannot_follow_herself(self): - error = ckan.tests.post(self.app, 'follow_user', + error = ckan.tests.call_action_api(self.app, 'follow_user', apikey=self.annafan['apikey'], status=409, id=self.annafan['id']) assert error['message'] == 'You cannot follow yourself' @@ -370,57 +370,57 @@ def test_04_follower_count_bad_id(self): for action in ('user_follower_count', 'dataset_follower_count', 'group_follower_count'): for object_id in ('bad id', ' ', 3, 35.7, 'xxx', ''): - error = ckan.tests.post(self.app, action, status=409, - id=object_id) + error = ckan.tests.call_action_api(self.app, action, + status=409, id=object_id) assert 'id' in error def test_04_follower_count_missing_id(self): for action in ('user_follower_count', 'dataset_follower_count', 'group_follower_count'): - error = ckan.tests.post(self.app, action, status=409) + error = ckan.tests.call_action_api(self.app, action, status=409) assert error['id'] == ['Missing value'] def test_04_user_follower_count_no_followers(self): - follower_count = ckan.tests.post(self.app, 'user_follower_count', - id=self.annafan['id']) + follower_count = ckan.tests.call_action_api(self.app, + 'user_follower_count', id=self.annafan['id']) assert follower_count == 0 def test_04_dataset_follower_count_no_followers(self): - follower_count = ckan.tests.post(self.app, 'dataset_follower_count', - id=self.annakarenina['id']) + follower_count = ckan.tests.call_action_api(self.app, + 'dataset_follower_count', id=self.annakarenina['id']) assert follower_count == 0 def test_04_group_follower_count_no_followers(self): - follower_count = ckan.tests.post(self.app, 'group_follower_count', - id=self.davids_group['id']) + follower_count = ckan.tests.call_action_api(self.app, + 'group_follower_count', id=self.davids_group['id']) assert follower_count == 0 def test_04_follower_list_bad_id(self): for action in ('user_follower_list', 'dataset_follower_list', 'group_follower_list'): for object_id in ('bad id', ' ', 3, 35.7, 'xxx', ''): - error = ckan.tests.post(self.app, action, status=409, - id=object_id) + error = ckan.tests.call_action_api(self.app, action, + status=409, id=object_id) assert error['id'] def test_04_follower_list_missing_id(self): for action in ('user_follower_list', 'dataset_follower_list', 'group_follower_list'): - error = ckan.tests.post(self.app, action, status=409) + error = ckan.tests.call_action_api(self.app, action, status=409) assert error['id'] == ['Missing value'] def test_04_user_follower_list_no_followers(self): - followers = ckan.tests.post(self.app, 'user_follower_list', + followers = ckan.tests.call_action_api(self.app, 'user_follower_list', id=self.annafan['id']) assert followers == [] def test_04_dataset_follower_list_no_followers(self): - followers = ckan.tests.post(self.app, 'dataset_follower_list', - id=self.annakarenina['id']) + followers = ckan.tests.call_action_api(self.app, + 'dataset_follower_list', id=self.annakarenina['id']) assert followers == [] def test_04_group_follower_list_no_followers(self): - followers = ckan.tests.post(self.app, 'group_follower_list', + followers = ckan.tests.call_action_api(self.app, 'group_follower_list', id=self.davids_group['id']) assert followers == [] @@ -428,7 +428,7 @@ def test_04_am_following_bad_id(self): for action in ('am_following_dataset', 'am_following_user', 'am_following_group'): for object_id in ('bad id', ' ', 3, 35.7, 'xxx'): - error = ckan.tests.post(self.app, action, + error = ckan.tests.call_action_api(self.app, action, apikey=self.annafan['apikey'], status=409, id=object_id) assert error['id'][0].startswith('Not found: ') @@ -437,44 +437,45 @@ def test_04_am_following_missing_id(self): 'am_following_group'): for id in ('missing', None, ''): if id == 'missing': - error = ckan.tests.post(self.app, action, + error = ckan.tests.call_action_api(self.app, action, apikey=self.annafan['apikey'], status=409) else: - error = ckan.tests.post(self.app, action, + error = ckan.tests.call_action_api(self.app, action, apikey=self.annafan['apikey'], status=409, id=id) assert error['id'] == [u'Missing value'] def test_04_am_following_dataset_bad_apikey(self): for apikey in ('bad api key', '', ' ', 'None', '3', '35.7', 'xxx'): - error = ckan.tests.post(self.app, 'am_following_dataset', - apikey=apikey, status=403, id=self.warandpeace['id']) + error = ckan.tests.call_action_api(self.app, + 'am_following_dataset', apikey=apikey, status=403, + id=self.warandpeace['id']) assert error['message'] == 'Access denied' def test_04_am_following_dataset_missing_apikey(self): - error = ckan.tests.post(self.app, 'am_following_dataset', status=403, - id=self.warandpeace['id']) + error = ckan.tests.call_action_api(self.app, 'am_following_dataset', + status=403, id=self.warandpeace['id']) assert error['message'] == 'Access denied' def test_04_am_following_user_bad_apikey(self): for apikey in ('bad api key', '', ' ', 'None', '3', '35.7', 'xxx'): - error = ckan.tests.post(self.app, 'am_following_user', + error = ckan.tests.call_action_api(self.app, 'am_following_user', apikey=apikey, status=403, id=self.annafan['id']) assert error['message'] == 'Access denied' def test_04_am_following_user_missing_apikey(self): - error = ckan.tests.post(self.app, 'am_following_user', status=403, - id=self.annafan['id']) + error = ckan.tests.call_action_api(self.app, 'am_following_user', + status=403, id=self.annafan['id']) assert error['message'] == 'Access denied' def test_04_am_following_group_bad_apikey(self): for apikey in ('bad api key', '', ' ', 'None', '3', '35.7', 'xxx'): - error = ckan.tests.post(self.app, 'am_following_group', + error = ckan.tests.call_action_api(self.app, 'am_following_group', apikey=apikey, status=403, id=self.rogers_group['id']) assert error['message'] == 'Access denied' def test_04_am_following_group_missing_apikey(self): - error = ckan.tests.post(self.app, 'am_following_group', status=403, - id=self.rogers_group['id']) + error = ckan.tests.call_action_api(self.app, 'am_following_group', + status=403, id=self.rogers_group['id']) assert error['message'] == 'Access denied' @@ -558,7 +559,7 @@ def test_01_unfollow_user_not_exists(self): she is not following. ''' - error = ckan.tests.post(self.app, 'unfollow_user', + error = ckan.tests.call_action_api(self.app, 'unfollow_user', apikey=self.annafan['apikey'], status=404, id=self.russianfan['id']) assert error['message'].startswith('Not found: You are not following ') @@ -568,7 +569,7 @@ def test_01_unfollow_dataset_not_exists(self): she is not following. ''' - error = ckan.tests.post(self.app, 'unfollow_dataset', + error = ckan.tests.call_action_api(self.app, 'unfollow_dataset', apikey=self.annafan['apikey'], status=404, id=self.annakarenina['id']) assert error['message'].startswith('Not found: You are not following') @@ -578,7 +579,7 @@ def test_01_unfollow_group_not_exists(self): she is not following. ''' - error = ckan.tests.post(self.app, 'unfollow_group', + error = ckan.tests.call_action_api(self.app, 'unfollow_group', apikey=self.annafan['apikey'], status=404, id=self.rogers_group['id']) assert error['message'].startswith('Not found: You are not following') @@ -591,14 +592,14 @@ def test_01_unfollow_bad_apikey(self): for action in ('unfollow_user', 'unfollow_dataset', 'unfollow_group'): for apikey in ('bad api key', '', ' ', 'None', '3', '35.7', 'xxx'): - error = ckan.tests.post(self.app, action, apikey=apikey, - status=403, id=self.joeadmin['id']) + error = ckan.tests.call_action_api(self.app, action, + apikey=apikey, status=403, id=self.joeadmin['id']) assert error['message'] == 'Access denied' def test_01_unfollow_missing_apikey(self): '''Test error response when calling unfollow_* without api key.''' for action in ('unfollow_user', 'unfollow_dataset', 'unfollow_group'): - error = ckan.tests.post(self.app, action, status=403, + error = ckan.tests.call_action_api(self.app, action, status=403, id=self.joeadmin['id']) assert error['message'] == 'Access denied' @@ -606,7 +607,7 @@ def test_01_unfollow_bad_object_id(self): '''Test error response when calling unfollow_* with bad object id.''' for action in ('unfollow_user', 'unfollow_dataset', 'unfollow_group'): for object_id in ('bad id', ' ', 3, 35.7, 'xxx'): - error = ckan.tests.post(self.app, action, + error = ckan.tests.call_action_api(self.app, action, apikey=self.annafan['apikey'], status=409, id=object_id) assert error['id'][0].startswith('Not found') @@ -615,10 +616,10 @@ def test_01_unfollow_missing_object_id(self): for action in ('unfollow_user', 'unfollow_dataset', 'unfollow_group'): for id in ('missing', None, ''): if id == 'missing': - error = ckan.tests.post(self.app, action, + error = ckan.tests.call_action_api(self.app, action, apikey=self.annafan['apikey'], status=409) else: - error = ckan.tests.post(self.app, action, + error = ckan.tests.call_action_api(self.app, action, apikey=self.annafan['apikey'], status=409, id=id) assert error['id'] == [u'Missing value'] @@ -633,32 +634,32 @@ def _unfollow_user(self, follower_id, apikey, object_id, object_arg): ''' # Record the user's number of followers before. - count_before = ckan.tests.post(self.app, 'user_follower_count', - id=object_id) + count_before = ckan.tests.call_action_api(self.app, + 'user_follower_count', id=object_id) # Check that the user is following the object. - am_following = ckan.tests.post(self.app, 'am_following_user', - apikey=apikey, id=object_id) + am_following = ckan.tests.call_action_api(self.app, + 'am_following_user', apikey=apikey, id=object_id) assert am_following is True # Make the user unfollow the object. - ckan.tests.post(self.app, 'unfollow_user', apikey=apikey, + ckan.tests.call_action_api(self.app, 'unfollow_user', apikey=apikey, id=object_arg) # Check that am_following_user now returns False. - am_following = ckan.tests.post(self.app, 'am_following_user', - apikey=apikey, id=object_id) + am_following = ckan.tests.call_action_api(self.app, + 'am_following_user', apikey=apikey, id=object_id) assert am_following is False # Check that the user doesn't appear in the object's list of followers. - followers = ckan.tests.post(self.app, 'user_follower_list', + followers = ckan.tests.call_action_api(self.app, 'user_follower_list', id=object_id) assert len([follower for follower in followers if follower['id'] == follower_id]) == 0 # Check that the object's follower count has decreased by 1. - count_after = ckan.tests.post(self.app, 'user_follower_count', - id=object_id) + count_after = ckan.tests.call_action_api(self.app, + 'user_follower_count', id=object_id) assert count_after == count_before - 1 def _unfollow_dataset(self, user_id, apikey, dataset_id, dataset_arg): @@ -672,33 +673,33 @@ def _unfollow_dataset(self, user_id, apikey, dataset_id, dataset_arg): ''' # Record the dataset's number of followers before. - count_before = ckan.tests.post(self.app, 'dataset_follower_count', - id=dataset_id) + count_before = ckan.tests.call_action_api(self.app, + 'dataset_follower_count', id=dataset_id) # Check that the user is following the dataset. - am_following = ckan.tests.post(self.app, 'am_following_dataset', - apikey=apikey, id=dataset_id) + am_following = ckan.tests.call_action_api(self.app, + 'am_following_dataset', apikey=apikey, id=dataset_id) assert am_following is True # Make the user unfollow the dataset. - ckan.tests.post(self.app, 'unfollow_dataset', apikey=apikey, + ckan.tests.call_action_api(self.app, 'unfollow_dataset', apikey=apikey, id=dataset_arg) # Check that am_following_dataset now returns False. - am_following = ckan.tests.post(self.app, 'am_following_dataset', - apikey=apikey, id=dataset_id) + am_following = ckan.tests.call_action_api(self.app, + 'am_following_dataset', apikey=apikey, id=dataset_id) assert am_following is False # Check that the user doesn't appear in the dataset's list of # followers. - followers = ckan.tests.post(self.app, 'dataset_follower_list', - id=dataset_id) + followers = ckan.tests.call_action_api(self.app, + 'dataset_follower_list', id=dataset_id) assert len([follower for follower in followers if follower['id'] == user_id]) == 0 # Check that the dataset's follower count has decreased by 1. - count_after = ckan.tests.post(self.app, 'dataset_follower_count', - id=dataset_id) + count_after = ckan.tests.call_action_api(self.app, + 'dataset_follower_count', id=dataset_id) assert count_after == count_before - 1 def _unfollow_group(self, user_id, apikey, group_id, group_arg): @@ -712,33 +713,33 @@ def _unfollow_group(self, user_id, apikey, group_id, group_arg): ''' # Record the group's number of followers before. - count_before = ckan.tests.post(self.app, 'group_follower_count', - id=group_id) + count_before = ckan.tests.call_action_api(self.app, + 'group_follower_count', id=group_id) # Check that the user is following the group. - am_following = ckan.tests.post(self.app, 'am_following_group', - apikey=apikey, id=group_id) + am_following = ckan.tests.call_action_api(self.app, + 'am_following_group', apikey=apikey, id=group_id) assert am_following is True # Make the user unfollow the group. - ckan.tests.post(self.app, 'unfollow_group', apikey=apikey, + ckan.tests.call_action_api(self.app, 'unfollow_group', apikey=apikey, id=group_arg) # Check that am_following_group now returns False. - am_following = ckan.tests.post(self.app, 'am_following_group', - apikey=apikey, id=group_id) + am_following = ckan.tests.call_action_api(self.app, + 'am_following_group', apikey=apikey, id=group_id) assert am_following is False # Check that the user doesn't appear in the group's list of # followers. - followers = ckan.tests.post(self.app, 'group_follower_list', + followers = ckan.tests.call_action_api(self.app, 'group_follower_list', id=group_id) assert len([follower for follower in followers if follower['id'] == user_id]) == 0 # Check that the group's follower count has decreased by 1. - count_after = ckan.tests.post(self.app, 'group_follower_count', - id=group_id) + count_after = ckan.tests.call_action_api(self.app, + 'group_follower_count', id=group_id) assert count_after == count_before - 1 def test_02_follower_delete_by_id(self): @@ -846,92 +847,92 @@ def test_01_on_delete_cascade_api(self): ''' # It should no longer be possible to get joeadmin's follower list. - error = ckan.tests.post(self.app, 'user_follower_list', status=409, - id='joeadmin') + error = ckan.tests.call_action_api(self.app, 'user_follower_list', + status=409, id='joeadmin') assert 'id' in error # It should no longer be possible to get warandpeace's follower list. - error = ckan.tests.post(self.app, 'dataset_follower_list', status=409, - id='warandpeace') + error = ckan.tests.call_action_api(self.app, 'dataset_follower_list', + status=409, id='warandpeace') assert 'id' in error # It should no longer be possible to get david's follower list. - error = ckan.tests.post(self.app, 'group_follower_list', status=409, - id='david') + error = ckan.tests.call_action_api(self.app, 'group_follower_list', + status=409, id='david') assert 'id' in error # It should no longer be possible to get joeadmin's follower count. - error = ckan.tests.post(self.app, 'user_follower_count', status=409, - id='joeadmin') + error = ckan.tests.call_action_api(self.app, 'user_follower_count', + status=409, id='joeadmin') assert 'id' in error # It should no longer be possible to get warandpeace's follower count. - error = ckan.tests.post(self.app, 'dataset_follower_count', status=409, - id='warandpeace') + error = ckan.tests.call_action_api(self.app, 'dataset_follower_count', + status=409, id='warandpeace') assert 'id' in error # It should no longer be possible to get david's follower count. - error = ckan.tests.post(self.app, 'group_follower_count', status=409, - id='david') + error = ckan.tests.call_action_api(self.app, 'group_follower_count', + status=409, id='david') assert 'id' in error # It should no longer be possible to get am_following for joeadmin. - error = ckan.tests.post(self.app, 'am_following_user', + error = ckan.tests.call_action_api(self.app, 'am_following_user', apikey=self.testsysadmin['apikey'], status=409, id='joeadmin') assert 'id' in error # It should no longer be possible to get am_following for warandpeace. - error = ckan.tests.post(self.app, 'am_following_dataset', + error = ckan.tests.call_action_api(self.app, 'am_following_dataset', apikey=self.testsysadmin['apikey'], status=409, id='warandpeace') assert 'id' in error # It should no longer be possible to get am_following for david. - error = ckan.tests.post(self.app, 'am_following_group', + error = ckan.tests.call_action_api(self.app, 'am_following_group', apikey=self.testsysadmin['apikey'], status=409, id='david') assert 'id' in error # It should no longer be possible to unfollow joeadmin. - error = ckan.tests.post(self.app, 'unfollow_user', + error = ckan.tests.call_action_api(self.app, 'unfollow_user', apikey=self.tester['apikey'], status=409, id='joeadmin') assert error['id'] == ['Not found: User'] # It should no longer be possible to unfollow warandpeace. - error = ckan.tests.post(self.app, 'unfollow_dataset', + error = ckan.tests.call_action_api(self.app, 'unfollow_dataset', apikey=self.testsysadmin['apikey'], status=409, id='warandpeace') assert error['id'] == ['Not found: Dataset'] # It should no longer be possible to unfollow david. - error = ckan.tests.post(self.app, 'unfollow_group', + error = ckan.tests.call_action_api(self.app, 'unfollow_group', apikey=self.testsysadmin['apikey'], status=409, id='david') assert error['id'] == ['Not found: Group'] # It should no longer be possible to follow joeadmin. - error = ckan.tests.post(self.app, 'follow_user', + error = ckan.tests.call_action_api(self.app, 'follow_user', apikey=self.annafan['apikey'], status=409, id='joeadmin') assert 'id' in error # It should no longer be possible to follow warandpeace. - error = ckan.tests.post(self.app, 'follow_dataset', + error = ckan.tests.call_action_api(self.app, 'follow_dataset', apikey=self.annafan['apikey'], status=409, id='warandpeace') assert 'id' in error # It should no longer be possible to follow david. - error = ckan.tests.post(self.app, 'follow_group', + error = ckan.tests.call_action_api(self.app, 'follow_group', apikey=self.annafan['apikey'], status=409, id='david') assert 'id' in error # Users who joeadmin was following should no longer have him in their # follower list. - followers = ckan.tests.post(self.app, 'user_follower_list', + followers = ckan.tests.call_action_api(self.app, 'user_follower_list', id=self.testsysadmin['id']) assert 'joeadmin' not in [follower['name'] for follower in followers] # Datasets who joeadmin was following should no longer have him in # their follower list. - followers = ckan.tests.post(self.app, 'dataset_follower_list', - id=self.annakarenina['id']) + followers = ckan.tests.call_action_api(self.app, + 'dataset_follower_list', id=self.annakarenina['id']) assert 'joeadmin' not in [follower['name'] for follower in followers] def test_02_on_delete_cascade_db(self):