From fd37c90e614c75f1d874f38e2733586e4ca93214 Mon Sep 17 00:00:00 2001
From: Sean Hammond <seanhammond@lavabit.com>
Date: Fri, 26 Oct 2012 13:45:59 +0200
Subject: [PATCH 01/12] [#1664] Make group activity streams smarter

Make group_activity_list() return all activities where the object of the
activity is either the group or any of the group's datasets, instead of just
the group as before.

Also check that the user has 'group_show' access to the group before returning
the activity stream, this plugs an access leak where a user who did not have
'group_show' access could still get the group's activity stream.

Update some existing tests that were broken by the new permission check.

Tests for the new group activity stream content need to be added.
---
 ckan/logic/action/get.py                   | 13 ++++++++++-
 ckan/tests/functional/api/test_activity.py | 26 ++++++++++++++--------
 2 files changed, 29 insertions(+), 10 deletions(-)

diff --git a/ckan/logic/action/get.py b/ckan/logic/action/get.py
index afe32cc202c..7e9e20ca592 100644
--- a/ckan/logic/action/get.py
+++ b/ckan/logic/action/get.py
@@ -1761,11 +1761,22 @@ def group_activity_list(context, data_dict):
     '''
     model = context['model']
     group_id = _get_or_bust(data_dict, 'id')
+
+    _check_access('group_show',context, data_dict)
+
+    # 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)
-    query = query.filter_by(object_id=group_id)
+    query = query.filter(_or_(model.Activity.object_id == group_id,
+        model.Activity.object_id.in_(dataset_ids)))
     query = query.order_by(_desc(model.Activity.timestamp))
     query = query.limit(15)
     activity_objects = query.all()
+
     return model_dictize.activity_list_dictize(activity_objects, context)
 
 def recently_changed_packages_activity_list(context, data_dict):
diff --git a/ckan/tests/functional/api/test_activity.py b/ckan/tests/functional/api/test_activity.py
index da34969e60a..89f50d12c32 100644
--- a/ckan/tests/functional/api/test_activity.py
+++ b/ckan/tests/functional/api/test_activity.py
@@ -153,8 +153,13 @@ def package_activity_stream(self, package_id):
         response = self.app.get("/api/2/rest/dataset/%s/activity" % package_id)
         return json.loads(response.body)
 
-    def group_activity_stream(self, group_id):
-        response = self.app.get("/api/2/rest/group/%s/activity" % group_id)
+    def group_activity_stream(self, group_id, apikey=None):
+        if apikey:
+            extra_environ = {'Authorization': str(apikey)}
+        else:
+            extra_environ = None
+        response = self.app.get("/api/2/rest/group/%s/activity" % group_id,
+                extra_environ=extra_environ)
         return json.loads(response.body)
 
     def recently_changed_datasets_stream(self):
@@ -171,7 +176,8 @@ def activity_details(self, activity):
                 "/api/2/rest/activity/%s/details" % activity['id'])
         return json.loads(response.body)
 
-    def record_details(self, user_id, package_id=None, group_id=None):
+    def record_details(self, user_id, package_id=None, group_id=None,
+            apikey=None):
         details = {}
         details['user activity stream'] = self.user_activity_stream(user_id)
 
@@ -181,7 +187,7 @@ def record_details(self, user_id, package_id=None, group_id=None):
 
         if group_id is not None:
             details['group activity stream'] = (
-                self.group_activity_stream(group_id))
+                self.group_activity_stream(group_id, apikey))
 
         details['recently changed datasets stream'] = \
                 self.recently_changed_datasets_stream()
@@ -719,15 +725,17 @@ def _delete_group(self, group, user):
         item and detail are emitted.
 
         """
-        before = self.record_details(user.id, group_id=group.id)
+        before = self.record_details(user.id, group_id=group.id,
+                apikey=user.apikey)
 
         # Deleted the group.
-        context = {'model': model, 'session': model.Session, 'api_version':3,
+        context = {'model': model, 'session': model.Session, 'api_version': 3,
                 'user': user.name, 'allow_partial_update': True}
         group_dict = {'id': group.id, 'state': 'deleted'}
         group_update(context, group_dict)
 
-        after = self.record_details(user.id, group_id=group.id)
+        after = self.record_details(user.id, group_id=group.id,
+                apikey=user.apikey)
 
         # Find the new activity.
         new_activities = find_new_activities(before['user activity stream'],
@@ -747,10 +755,10 @@ def _delete_group(self, group, user):
         assert activity['user_id'] == user.id, str(activity['user_id'])
         assert activity['activity_type'] == 'deleted group', \
             str(activity['activity_type'])
-        if not activity.has_key('id'):
+        if 'id' not in activity:
             assert False, "activity object has no id value"
         # TODO: Test for the _correct_ revision_id value.
-        if not activity.has_key('revision_id'):
+        if 'revision_id' not in activity:
             assert False, "activity has no revision_id value"
         timestamp = datetime_from_string(activity['timestamp'])
         assert timestamp >= before['time'] and timestamp <= after['time'], \

From 9cc0f849b32301be98669d5b725ebc48b305b7f1 Mon Sep 17 00:00:00 2001
From: Sean Hammond <seanhammond@lavabit.com>
Date: Fri, 26 Oct 2012 14:36:19 +0200
Subject: [PATCH 02/12] [#1664] Add group activity stream page

You can now see a group's activity stream at /group/activity/GROUP_NAME.

This page is not integrated into the rest of the group pages yet like
the user activity stream page is with the user pages, but can be in
future if we want.

Also remove c.group_activity_stream from the group read context as it's
not used so no need to compute it.
---
 ckan/config/routing.py                    |  3 ++-
 ckan/controllers/group.py                 | 28 ++++++++++++++++++-----
 ckan/templates/group/activity_stream.html |  6 +++++
 3 files changed, 30 insertions(+), 7 deletions(-)
 create mode 100644 ckan/templates/group/activity_stream.html

diff --git a/ckan/config/routing.py b/ckan/config/routing.py
index 3558098be33..b2bcc28721e 100644
--- a/ckan/config/routing.py
+++ b/ckan/config/routing.py
@@ -237,7 +237,8 @@ def make_map():
           'edit',
           'authz',
           'delete',
-          'history'
+          'history',
+          'activity',
           ]))
           )
         m.connect('group_read', '/group/{id}', action='read')
diff --git a/ckan/controllers/group.py b/ckan/controllers/group.py
index 5a1dcdb7aa8..189381d7ec4 100644
--- a/ckan/controllers/group.py
+++ b/ckan/controllers/group.py
@@ -233,12 +233,6 @@ def pager_url(q=None, page=None):
             c.facets = {}
             c.page = h.Page(collection=[])
 
-        # Add the group's activity stream (already rendered to HTML) to the
-        # template context for the group/read.html template to retrieve later.
-        c.group_activity_stream = \
-            get_action('group_activity_list_html')(context,
-                                                   {'id': c.group_dict['id']})
-
         return render(self._read_template(c.group_dict['type']))
 
     def new(self, data=None, errors=None, error_summary=None):
@@ -498,6 +492,28 @@ def history(self, id):
             return feed.writeString('utf-8')
         return render(self._history_template(c.group_dict['type']))
 
+    def activity(self, id):
+        '''Render this group'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.group_dict = get_action('group_show')(context, data_dict)
+            c.group = context['group']
+        except NotFound:
+            abort(404, _('Group not found'))
+        except NotAuthorized:
+            abort(401, _('Unauthorized to read group %s') % id)
+
+        # Add the group's activity stream (already rendered to HTML) to the
+        # template context for the group/read.html template to retrieve later.
+        c.group_activity_stream = get_action('group_activity_list_html')(
+                context, {'id': c.group_dict['id']})
+
+        return render('group/activity_stream.html')
+
     def _render_edit_form(self, fs):
         # errors arrive in c.error and fs.errors
         c.fieldset = fs
diff --git a/ckan/templates/group/activity_stream.html b/ckan/templates/group/activity_stream.html
new file mode 100644
index 00000000000..8543d0d8a3a
--- /dev/null
+++ b/ckan/templates/group/activity_stream.html
@@ -0,0 +1,6 @@
+{% extends "page.html" %}
+
+{% block primary_content %}
+  <h2 class="hide-heading">{{ _('Activity Stream') }}</h2>
+  {{ c.group_activity_stream | safe }}
+{% endblock %}

From 8facd097f6f98cff9f0a448d19e5e29fc747a173 Mon Sep 17 00:00:00 2001
From: Sean Hammond <seanhammond@lavabit.com>
Date: Fri, 26 Oct 2012 19:38:23 +0200
Subject: [PATCH 03/12] Remove direct logic function access from
 test_activity.py

Make functional/api/test_activity.py post to the API whenever it wants
to create, update or delete users, packages, groups, etc.

Also gets rid of the need to access resource_list_dictize() directly.
---
 ckan/tests/functional/api/test_activity.py | 391 +++++++++------------
 1 file changed, 172 insertions(+), 219 deletions(-)

diff --git a/ckan/tests/functional/api/test_activity.py b/ckan/tests/functional/api/test_activity.py
index 89f50d12c32..5d6b524bc57 100644
--- a/ckan/tests/functional/api/test_activity.py
+++ b/ckan/tests/functional/api/test_activity.py
@@ -6,32 +6,64 @@
 
 import ckan
 import ckan.model as model
-from ckan.logic.action.create import package_create as _package_create
-from ckan.logic.action.create import user_create, group_create, follow_dataset
-from ckan.logic.action.create import follow_user
-from ckan.logic.action.update import package_update as _package_update, resource_update
-from ckan.logic.action.update import user_update, group_update
-from ckan.logic.action.delete import package_delete
-from ckan.logic.action.get import package_list, package_show
-from ckan.lib.dictization.model_dictize import resource_list_dictize
 from pylons.test import pylonsapp
 import paste.fixture
 from ckan.lib.helpers import json
 
 
-def package_update(context, data_dict):
-    # These tests call package_update directly which is really bad
-    # setting api_version in context make things seem like the api key
-    # is ok etc
-    context['api_version'] = 3
-    return _package_update(context, data_dict)
+def package_show(app, data_dict, apikey=None):
+    if apikey:
+        extra_environ = {'Authorization': str(apikey)}
+    else:
+        extra_environ = None
+    response = app.post('/api/action/package_show', json.dumps(data_dict),
+            extra_environ=extra_environ)
+    response_dict = json.loads(response.body)
+    assert response_dict['success'] is True
+    package = response_dict['result']
+    return package
+
+
+def package_list(app, data_dict=None, apikey=None):
+    if data_dict is None:
+        data_dict = {}
+    if apikey:
+        extra_environ = {'Authorization': str(apikey)}
+    else:
+        extra_environ = None
+    response = app.post('/api/action/package_list',
+            json.dumps(data_dict), extra_environ=extra_environ)
+    response_dict = json.loads(response.body)
+    assert response_dict['success'] is True
+    packages = response_dict['result']
+    return packages
+
+
+def package_update(app, data_dict, apikey=None):
+    if apikey:
+        extra_environ = {'Authorization': str(apikey)}
+    else:
+        extra_environ = None
+    response = app.post('/api/action/package_update',
+            json.dumps(data_dict), extra_environ=extra_environ)
+    response_dict = json.loads(response.body)
+    assert response_dict['success'] is True
+    updated_package = response_dict['result']
+    return updated_package
+
+
+def group_update(app, data_dict, apikey=None):
+    if apikey:
+        extra_environ = {'Authorization': str(apikey)}
+    else:
+        extra_environ = None
+    response = app.post('/api/action/group_update',
+            json.dumps(data_dict), extra_environ=extra_environ)
+    response_dict = json.loads(response.body)
+    assert response_dict['success'] is True
+    updated_group = response_dict['result']
+    return updated_group
 
-def package_create(context, data_dict):
-    # These tests call package_update directly which is really bad
-    # setting api_version in context make things seem like the api key
-    # is ok etc
-    context['api_version'] = 3
-    return _package_create(context, data_dict)
 
 def datetime_from_string(s):
     '''Return a standard datetime.datetime object initialised from a string in
@@ -222,14 +254,13 @@ def _create_package(self, user, name=None):
         before = self.record_details(user_id)
 
         # Create a new package.
-        context = {
-            'model': model,
-            'session': model.Session,
-            'user': user_name,
-            'allow_partial_update': True,
-            }
         request_data = make_package(name)
-        package_created = package_create(context, request_data)
+        extra_environ = {'Authorization': str(user.apikey)}
+        response = self.app.post('/api/action/package_create',
+                json.dumps(request_data), extra_environ=extra_environ)
+        response_dict = json.loads(response.body)
+        assert response_dict['success'] is True
+        package_created = response_dict['result']
 
         after = self.record_details(user_id, package_created['id'])
 
@@ -313,30 +344,17 @@ def _add_resource(self, package, user):
             user_name = '127.0.0.1'
             user_id = 'not logged in'
 
-        before = self.record_details(user_id, package.id)
+        before = self.record_details(user_id, package['id'])
 
-        # Query for the package object again, as the session that it belongs to
-        # may have been closed.
-        package = model.Session.query(model.Package).get(package.id)
-
-        resource_ids_before = [resource.id for resource in package.resources]
+        resource_ids_before = [resource['id'] for resource in
+                package['resources']]
 
         # Create a new resource.
-        context = {
-            'model': model,
-            'session': model.Session,
-            'user': user_name,
-            'allow_partial_update': True,
-            }
-        resources = resource_list_dictize(package.resources, context)
+        resources = package['resources']
         resources.append(make_resource())
-        request_data = {
-                'id': package.id,
-                'resources': resources,
-                }
-        updated_package = package_update(context, request_data)
+        updated_package = package_update(self.app, package, user.apikey)
 
-        after = self.record_details(user_id, package.id)
+        after = self.record_details(user_id, package['id'])
         resource_ids_after = [resource['id'] for resource in
                 updated_package['resources']]
         assert len(resource_ids_after) == len(resource_ids_before) + 1
@@ -362,7 +380,8 @@ def _add_resource(self, package, user):
                 after['recently changed datasets stream']) \
                         == user_new_activities
 
-        self.check_dashboard(before, after, user_new_activities, [user_id, package.id])
+        self.check_dashboard(before, after, user_new_activities,
+                [user_id, package['id']])
 
         # Check that the new activity has the right attributes.
         assert activity['object_id'] == updated_package['id'], \
@@ -406,26 +425,13 @@ def _delete_extra(self, package_dict, user):
 
         before = self.record_details(user_id, package_dict['id'])
 
-        extras_before = package_dict['extras']
+        extras_before = list(package_dict['extras'])
         assert len(extras_before) > 0, (
                 "Can't update an extra if the package doesn't have any")
 
         # Update the package's first extra.
-        context = {
-            'model': model,
-            'session': model.Session,
-            'user': user_name,
-            'extras_as_string': True,
-            }
-        extras = list(extras_before)
-        del extras[0]
-        request_data = {
-                'id': package_dict['id'],
-                'extras': extras,
-                'tags': package_dict['tags'],
-                'resources': package_dict['resources']
-                }
-        updated_package = package_update(context, request_data)
+        del package_dict['extras'][0]
+        updated_package = package_update(self.app, package_dict, user.apikey)
 
         after = self.record_details(user_id, package_dict['id'])
         extras_after = updated_package['extras']
@@ -504,24 +510,13 @@ def _update_extra(self, package_dict, user):
                 "Can't update an extra if the package doesn't have any")
 
         # Update the package's first extra.
-        context = {
-            'model': model,
-            'session': model.Session,
-            'user': user_name,
-            'allow_partial_update': True,
-            'extras_as_string': True
-            }
         extras = list(extras_before)
-        if extras[0]['value'] != 'edited':
-            extras[0]['value'] = 'edited'
+        if extras[0]['value'] != '"edited"':
+            extras[0]['value'] = '"edited"'
         else:
-            assert extras[0]['value'] != 'edited again'
-            extras[0]['value'] = 'edited again'
-        request_data = {
-                'id': package_dict['id'],
-                'extras': extras
-                }
-        updated_package = package_update(context, request_data)
+            assert extras[0]['value'] != '"edited again"'
+            extras[0]['value'] = '"edited again"'
+        updated_package = package_update(self.app, package_dict, user.apikey)
 
         after = self.record_details(user_id, package_dict['id'])
         extras_after = updated_package['extras']
@@ -597,23 +592,14 @@ def _add_extra(self, package_dict, user, key=None):
 
         before = self.record_details(user_id, package_dict['id'])
 
-        extras_before = package_dict['extras']
+        # Make a copy of the package's extras before we add a new extra,
+        # so we can compare the extras before and after updating the package.
+        extras_before = list(package_dict['extras'])
 
         # Create a new extra.
-        context = {
-            'model': model,
-            'session': model.Session,
-            'user': user_name,
-            'allow_partial_update': True,
-            'extras_as_string': True,
-            }
-        extras = list(extras_before)
+        extras = package_dict['extras']
         extras.append({'key': key, 'value': '10000'})
-        request_data = {
-                'id': package_dict['id'],
-                'extras': extras
-                }
-        updated_package = package_update(context, request_data)
+        updated_package = package_update(self.app, package_dict, user.apikey)
 
         after = self.record_details(user_id, package_dict['id'])
         extras_after = updated_package['extras']
@@ -729,10 +715,8 @@ def _delete_group(self, group, user):
                 apikey=user.apikey)
 
         # Deleted the group.
-        context = {'model': model, 'session': model.Session, 'api_version': 3,
-                'user': user.name, 'allow_partial_update': True}
         group_dict = {'id': group.id, 'state': 'deleted'}
-        group_update(context, group_dict)
+        group_update(self.app, group_dict, user.apikey)
 
         after = self.record_details(user.id, group_id=group.id,
                 apikey=user.apikey)
@@ -773,10 +757,8 @@ def _update_group(self, group, user):
         before = self.record_details(user.id, group_id=group.id)
 
         # Update the group.
-        context = {'model': model, 'session': model.Session, 'user': user.name,
-                   'allow_partial_update': True, 'api_version':3}
         group_dict = {'id': group.id, 'title': 'edited'}
-        group_updated = group_update(context, group_dict)
+        group_update(self.app, group_dict, user.apikey)
 
         after = self.record_details(user.id, group_id=group.id)
 
@@ -793,7 +775,6 @@ def _update_group(self, group, user):
 
         self.check_dashboard(before, after, new_activities, [user.id])
 
-
         # 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'])
@@ -814,24 +795,23 @@ def _update_user(self, user):
         and detail are emitted.
 
         """
-        before = self.record_details(user.id)
+        response = self.app.post('/api/action/user_show',
+                json.dumps({'id': user.id}),
+                extra_environ={'Authorization': str(user.apikey)})
+        response_dict = json.loads(response.body)
+        assert response_dict['success'] is True
+        user_dict = response_dict['result']
 
-        # Query for the user object again, as the session that it belongs to
-        # may have been closed.
-        user = model.Session.query(model.User).get(user.id)
+        before = self.record_details(user_dict['id'])
 
         # Update the user.
-        context = {'model': model, 'session': model.Session, 'user': user.name,
-                'allow_partial_update': True}
-        user_dict = {'id': user.id}
         user_dict['about'] = 'edited'
-        if user.email:
-            user_dict['email'] = user.email
-        else:
+        if not user_dict.get('email'):
             user_dict['email'] = 'there has to be a value in email or validate fails'
-        user_update(context, user_dict)
+        self.app.post('/api/action/user_update', json.dumps(user_dict),
+                extra_environ={'Authorization': str(user.apikey)})
 
-        after = self.record_details(user.id)
+        after = self.record_details(user_dict['id'])
 
         # Find the new activity.
         new_activities = find_new_activities(before['user activity stream'],
@@ -840,13 +820,14 @@ def _update_user(self, user):
             "the user's activity stream, but found %i" % len(new_activities))
         activity = new_activities[0]
 
-        self.check_dashboard(before, after, new_activities, [user.id])
+        self.check_dashboard(before, after, new_activities, [user_dict['id']])
 
         # Check that the new activity has the right attributes.
-        assert activity['object_id'] == user.id, str(activity['object_id'])
-        assert activity['user_id'] == user.id, str(activity['user_id'])
-        assert activity['activity_type'] == 'changed user', \
-            str(activity['activity_type'])
+        assert activity['object_id'] == user_dict['id'], (
+                str(activity['object_id']))
+        assert activity['user_id'] == user_dict['id'], str(activity['user_id'])
+        assert activity['activity_type'] == 'changed user', (
+            str(activity['activity_type']))
         if not activity.has_key('id'):
             assert False, "activity object has no id value"
         # TODO: Test for the _correct_ revision_id value.
@@ -876,12 +857,12 @@ def _delete_resources(self, package):
         context = {
             'model': model,
             'session': model.Session,
-            'user':self.normal_user.name,
+            'user': self.normal_user.name,
             }
         from ckan.lib.dictization.model_dictize import package_dictize
         data_dict = package_dictize(package, context)
         data_dict['resources'] = []
-        package_update(context, data_dict)
+        package_update(self.app, data_dict, self.normal_user.apikey)
 
         after = self.record_details(self.normal_user.id, package.id)
 
@@ -951,24 +932,17 @@ def _update_package(self, package, user):
             user_name = '127.0.0.1'
             user_id = 'not logged in'
 
-        before = self.record_details(user_id, package.id)
-
-        # Query for the package object again, as the session that it belongs to
-        # may have been closed.
-        package = model.Session.query(model.Package).get(package.id)
+        before = self.record_details(user_id, package['id'])
 
         # Update the package.
-        context = {'model': model, 'session': model.Session, 'user': user_name,
-                'allow_partial_update': True}
-        package_dict = {'id': package.id}
-        if package.title != 'edited':
-            package_dict['title'] = 'edited'
+        if package['title'] != 'edited':
+            package['title'] = 'edited'
         else:
-            assert package.title != 'edited again'
-            package_dict['title'] = 'edited again'
-        package_update(context, package_dict)
+            assert package['title'] != 'edited again'
+            package['title'] = 'edited again'
+        package_update(self.app, package, user.apikey)
 
-        after = self.record_details(user_id, package.id)
+        after = self.record_details(user_id, package['id'])
 
         # Find the new activity in the user's activity stream.
         user_new_activities = (find_new_activities(
@@ -991,10 +965,11 @@ def _update_package(self, package, user):
                 after['recently changed datasets stream']) \
                         == user_new_activities
 
-        self.check_dashboard(before, after, user_new_activities, [user_id, package.id])
+        self.check_dashboard(before, after, user_new_activities,
+                [user_id, package['id']])
 
         # Check that the new activity has the right attributes.
-        assert activity['object_id'] == package.id, (
+        assert activity['object_id'] == package['id'], (
             str(activity['object_id']))
         assert activity['user_id'] == user_id, str(activity['user_id'])
         assert activity['activity_type'] == 'changed package', (
@@ -1014,7 +989,7 @@ def _update_package(self, package, user):
         detail = details[0]
         assert detail['activity_id'] == activity['id'], \
             str(detail['activity_id'])
-        assert detail['object_id'] == package.id, str(detail['object_id'])
+        assert detail['object_id'] == package['id'], str(detail['object_id'])
         assert detail['object_type'] == "Package", (
             str(detail['object_type']))
         assert detail['activity_type'] == "changed", (
@@ -1033,20 +1008,13 @@ def _update_resource(self, package, resource, user):
             user_name = '127.0.0.1'
             user_id = 'not logged in'
 
-        before = self.record_details(user_id, package.id)
-
-        # Query for the Package and Resource objects again, as the session that
-        # they belong to may have been closed.
-        package = model.Session.query(model.Package).get(package.id)
-        resource = model.Session.query(model.Resource).get(resource.id)
+        before = self.record_details(user_id, package['id'])
 
         # Update the resource.
-        context = {'model': model, 'session': model.Session, 'user': user_name,
-                'allow_partial_update': True}
-        resource_dict = {'id':resource.id, 'name':'edited'}
-        resource_update(context, resource_dict)
+        resource['name'] = 'edited'
+        package_update(self.app, package)
 
-        after = self.record_details(user_id, package.id)
+        after = self.record_details(user_id, package['id'])
 
         # Find the new activity in the user's activity stream.
         user_new_activities = (find_new_activities(
@@ -1069,10 +1037,10 @@ def _update_resource(self, package, resource, user):
                 after['recently changed datasets stream']) \
                         == user_new_activities
 
-        self.check_dashboard(before, after, user_new_activities, [user_id, package.id])
+        self.check_dashboard(before, after, user_new_activities, [user_id, package['id']])
 
         # Check that the new activity has the right attributes.
-        assert activity['object_id'] == package.id, (
+        assert activity['object_id'] == package['id'], (
             str(activity['object_id']))
         assert activity['user_id'] == user_id, str(activity['user_id'])
         assert activity['activity_type'] == 'changed package', (
@@ -1111,10 +1079,12 @@ def _delete_package(self, package):
         package = model.Session.query(model.Package).get(package.id)
 
         # Delete the package.
-        context = {'model': model, 'session': model.Session,
-                'user': self.sysadmin_user.name}
-        package_dict = {'id':package.id}
-        package_delete(context, package_dict)
+        package_dict = {'id': package.id}
+        response = self.app.post('/api/action/package_delete',
+            json.dumps(package_dict),
+            extra_environ={'Authorization': str(self.sysadmin_user.apikey)})
+        response_dict = json.loads(response.body)
+        assert response_dict['success'] is True
 
         after = self.record_details(self.sysadmin_user.id, package.id)
 
@@ -1228,7 +1198,7 @@ def test_01_remove_tag(self):
             'id': pkg_dict['id'],
             'tags': pkg_dict['tags'][0:-1],
             }
-        package_update(context, data_dict)
+        package_update(self.app, data_dict, user.apikey)
         after = self.record_details(user.id, pkg_dict['id'])
 
         # Find the new activity in the user's activity stream.
@@ -1288,15 +1258,9 @@ def test_01_update_extras(self):
         when a package extra is changed.
 
         """
-        context = {
-            'model': model,
-            'session': model.Session,
-            'user': self.normal_user.name,
-            'extras_as_string': True,
-            }
         packages_with_extras = []
-        for package_name in package_list(context, {}):
-            package_dict = package_show(context, {'id': package_name})
+        for package_name in package_list(self.app):
+            package_dict = package_show(self.app, {'id': package_name})
             if len(package_dict['extras']) > 0:
                     packages_with_extras.append(package_dict)
         assert len(packages_with_extras) > 0, (
@@ -1340,8 +1304,9 @@ def test_01_update_package(self):
         when packages are updated.
 
         """
-        for package in model.Session.query(model.Package).all():
-            self._update_package(package, user=self.normal_user)
+        for package_name in package_list(self.app):
+            package_dict = package_show(self.app, {'id': package_name})
+            self._update_package(package_dict, user=self.normal_user)
 
     def test_01_update_package_not_logged_in(self):
         """
@@ -1363,13 +1328,11 @@ def test_01_update_resource(self):
         when a resource is updated.
 
         """
-        packages = model.Session.query(model.Package).all()
-        for package in packages:
-            # Query the model for the Package object again, as the session that
-            # it belongs to may have been closed.
-            pkg = model.Session.query(model.Package).get(package.id)
-            for resource in pkg.resources:
-                self._update_resource(pkg, resource, user=self.normal_user)
+        for package_name in package_list(self.app):
+            package_dict = package_show(self.app, {'id': package_name})
+            for resource in package_dict['resources']:
+                self._update_resource(package_dict, resource,
+                        user=self.normal_user)
 
     def test_01_update_resource_not_logged_in(self):
         """
@@ -1377,13 +1340,10 @@ def test_01_update_resource_not_logged_in(self):
         when a resource is updated by a user who is not logged in.
 
         """
-        packages = model.Session.query(model.Package).all()
-        for package in packages:
-            # Query the model for the Package object again, as the session that
-            # it belongs to may have been closed.
-            pkg = model.Session.query(model.Package).get(package.id)
-            for resource in pkg.resources:
-                self._update_resource(pkg, resource, user=None)
+        for package_name in package_list(self.app):
+            package_dict = package_show(self.app, {'id': package_name})
+            for resource in package_dict['resources']:
+                self._update_resource(package_dict, resource, user=None)
 
     def test_create_package(self):
         """
@@ -1416,8 +1376,9 @@ def test_add_resources(self):
         when a resource is added to a package.
 
         """
-        for package in model.Session.query(model.Package).all():
-            self._add_resource(package, user=self.normal_user)
+        for package_name in package_list(self.app):
+            package_dict = package_show(self.app, {'id': package_name})
+            self._add_resource(package_dict, user=self.normal_user)
 
     def test_add_resources_not_logged_in(self):
         """
@@ -1458,9 +1419,12 @@ def test_create_user(self):
         user_dict = {'name': 'testuser',
                 'about': 'Just a test user', 'email': 'me@test.org',
                 'password': 'testpass'}
-        context = {'model': model, 'session': model.Session,
-                'user': self.sysadmin_user.name}
-        user_created = user_create(context, user_dict)
+        response = self.app.post('/api/action/user_create',
+            json.dumps(user_dict),
+            extra_environ={'Authorization': str(self.sysadmin_user.apikey)})
+        response_dict = json.loads(response.body)
+        assert response_dict['success'] is True
+        user_created = response_dict['result']
 
         after = self.record_details(user_created['id'])
 
@@ -1507,9 +1471,13 @@ def test_create_group(self):
         before = self.record_details(user.id)
 
         # Create a new group.
-        context = {'model': model, 'session': model.Session, 'user': user.name}
         request_data = {'name': 'a-new-group', 'title': 'A New Group'}
-        group_created = group_create(context, request_data)
+        response = self.app.post('/api/action/group_create',
+            json.dumps(request_data),
+            extra_environ={'Authorization': str(user.apikey)})
+        response_dict = json.loads(response.body)
+        assert response_dict['success'] is True
+        group_created = response_dict['result']
 
         after = self.record_details(user.id, group_id=group_created['id'])
 
@@ -1576,12 +1544,9 @@ def test_add_tag(self):
         before = self.record_details(user.id, pkg_dict['id'])
         new_tag_name = 'test tag'
         assert new_tag_name not in [tag['name'] for tag in pkg_dict['tags']]
-        new_tag_list = pkg_dict['tags'] + [{'name': new_tag_name}]
-        data_dict = {
-            'id': pkg_dict['id'],
-            'tags': new_tag_list
-            }
-        package_update(context, data_dict)
+
+        pkg_dict['tags'].append({'name': new_tag_name})
+        package_update(self.app, pkg_dict, user.apikey)
         after = self.record_details(user.id, pkg_dict['id'])
 
         # Find the new activity in the user's activity stream.
@@ -1975,14 +1940,8 @@ def test_add_extras(self):
         when an extra is added to a package.
 
         """
-        context = {
-            'model': model,
-            'session': model.Session,
-            'user': self.normal_user.name,
-            'extras_as_string': True,
-            }
-        for package_name in package_list(context, {}):
-            package_dict = package_show(context, {'id': package_name})
+        for package_name in package_list(self.app):
+            package_dict = package_show(self.app, {'id': package_name})
             self._add_extra(package_dict, user=self.normal_user)
 
     def test_add_extras_not_logged_in(self):
@@ -2014,15 +1973,9 @@ def test_delete_extras(self):
         when a package extra is deleted.
 
         """
-        context = {
-            'model': model,
-            'session': model.Session,
-            'user': self.normal_user.name,
-            'extras_as_string': True,
-            }
         packages_with_extras = []
-        for package_name in package_list(context, {}):
-            package_dict = package_show(context, {'id': package_name})
+        for package_name in package_list(self.app):
+            package_dict = package_show(self.app, {'id': package_name})
             if len(package_dict['extras']) > 0:
                     packages_with_extras.append(package_dict)
         assert len(packages_with_extras) > 0, (
@@ -2061,13 +2014,13 @@ def test_delete_extras_not_logged_in(self):
     def test_follow_dataset(self):
         user = self.normal_user
         before = self.record_details(user.id)
-        context = {
-            'model': model,
-            'session': model.Session,
-            'user': user.name,
-            }
         data = {'id': self.warandpeace.id}
-        follow_dataset(context, data)
+        extra_environ = {'Authorization': str(user.apikey)}
+        response = self.app.post('/api/action/follow_dataset',
+            json.dumps(data), extra_environ=extra_environ)
+        response_dict = json.loads(response.body)
+        assert response_dict['success'] is True
+
         after = self.record_details(user.id, self.warandpeace.id)
 
         # Find the new activity in the user's activity stream.
@@ -2106,13 +2059,13 @@ def test_follow_user(self):
         user = self.normal_user
         before = self.record_details(user.id)
         followee_before = self.record_details(self.sysadmin_user.id)
-        context = {
-            'model': model,
-            'session': model.Session,
-            'user': user.name,
-            }
         data = {'id': self.sysadmin_user.id}
-        follow_user(context, data)
+        extra_environ = {'Authorization': str(user.apikey)}
+        response = self.app.post('/api/action/follow_user',
+            json.dumps(data), extra_environ=extra_environ)
+        response_dict = json.loads(response.body)
+        assert response_dict['success'] is True
+
         after = self.record_details(user.id)
         followee_after = self.record_details(self.sysadmin_user.id)
 

From c4240fbd72a8c953487ed46f951866c90fbbae7b Mon Sep 17 00:00:00 2001
From: Sean Hammond <seanhammond@lavabit.com>
Date: Fri, 26 Oct 2012 21:16:35 +0200
Subject: [PATCH 04/12] Remove direct model access from test_activity.py

Import ckan.model only where needed and don't retain model objects, in
many cases replace accessing the model with going through the API
---
 ckan/tests/functional/api/test_activity.py | 406 +++++++++++----------
 1 file changed, 205 insertions(+), 201 deletions(-)

diff --git a/ckan/tests/functional/api/test_activity.py b/ckan/tests/functional/api/test_activity.py
index 5d6b524bc57..d3d197cf542 100644
--- a/ckan/tests/functional/api/test_activity.py
+++ b/ckan/tests/functional/api/test_activity.py
@@ -4,8 +4,6 @@
 
 from nose.plugins.skip import SkipTest
 
-import ckan
-import ckan.model as model
 from pylons.test import pylonsapp
 import paste.fixture
 from ckan.lib.helpers import json
@@ -39,6 +37,23 @@ def package_list(app, data_dict=None, apikey=None):
     return packages
 
 
+def group_list(app, data_dict=None, apikey=None):
+    if data_dict is None:
+        data_dict = {}
+    if 'all_fields' not in data_dict:
+        data_dict['all_fields'] = True
+    if apikey:
+        extra_environ = {'Authorization': str(apikey)}
+    else:
+        extra_environ = None
+    response = app.post('/api/action/group_list',
+            json.dumps(data_dict), extra_environ=extra_environ)
+    response_dict = json.loads(response.body)
+    assert response_dict['success'] is True
+    groups = response_dict['result']
+    return groups
+
+
 def package_update(app, data_dict, apikey=None):
     if apikey:
         extra_environ = {'Authorization': str(apikey)}
@@ -125,52 +140,78 @@ def find_new_activities(before, after):
             new_activities.append(activity)
     return new_activities
 
+
 class TestActivity:
 
     @classmethod
     def setup_class(self):
+        import ckan
+        import ckan.model as model
         ckan.tests.CreateTestData.create()
-        self.sysadmin_user = model.User.get('testsysadmin')
-        self.normal_user = model.User.get('annafan')
-        self.follower = model.User.get('tester')
-        self.warandpeace = model.Package.get('warandpeace')
-        self.annakarenina = model.Package.get('annakarenina')
+        sysadmin_user = model.User.get('testsysadmin')
+        self.sysadmin_user = {
+                'id': sysadmin_user.id,
+                'apikey': sysadmin_user.apikey,
+                'name': sysadmin_user.name,
+                }
+        normal_user = model.User.get('annafan')
+        self.normal_user = {
+                'id': normal_user.id,
+                '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,
+                }
+        annakarenina = model.Package.get('annakarenina')
+        self.annakarenina = {
+                'id': annakarenina.id,
+                }
+        self.users = [self.sysadmin_user, self.normal_user, self.follower]
         self.app = paste.fixture.TestApp(pylonsapp)
 
         # Make follower follow everything else.
         params = {'id': 'testsysadmin'}
-        extra_environ = {'Authorization': str(self.follower.apikey)}
+        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)}
+        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)}
+        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)}
+        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
+                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, user_id):
@@ -225,7 +266,7 @@ def record_details(self, user_id, package_id=None, group_id=None,
                 self.recently_changed_datasets_stream()
 
         details['follower dashboard activity stream'] = \
-                                self.dashboard_activity_stream(self.follower.id)
+                                self.dashboard_activity_stream(self.follower['id'])
 
         details['time'] = datetime.datetime.now()
         return details
@@ -245,17 +286,15 @@ def check_dashboard(
 
     def _create_package(self, user, name=None):
         if user:
-            user_name = user.name
-            user_id = user.id
+            user_id = user['id']
         else:
-            user_name = '127.0.0.1'
             user_id = 'not logged in'
 
         before = self.record_details(user_id)
 
         # Create a new package.
         request_data = make_package(name)
-        extra_environ = {'Authorization': str(user.apikey)}
+        extra_environ = {'Authorization': str(user['apikey'])}
         response = self.app.post('/api/action/package_create',
                 json.dumps(request_data), extra_environ=extra_environ)
         response_dict = json.loads(response.body)
@@ -338,10 +377,8 @@ def _create_package(self, user, name=None):
 
     def _add_resource(self, package, user):
         if user:
-            user_name = user.name
-            user_id = user.id
+            user_id = user['id']
         else:
-            user_name = '127.0.0.1'
             user_id = 'not logged in'
 
         before = self.record_details(user_id, package['id'])
@@ -352,7 +389,7 @@ def _add_resource(self, package, user):
         # Create a new resource.
         resources = package['resources']
         resources.append(make_resource())
-        updated_package = package_update(self.app, package, user.apikey)
+        updated_package = package_update(self.app, package, user['apikey'])
 
         after = self.record_details(user_id, package['id'])
         resource_ids_after = [resource['id'] for resource in
@@ -417,10 +454,8 @@ def _add_resource(self, package, user):
 
     def _delete_extra(self, package_dict, user):
         if user:
-            user_name = user.name
-            user_id = user.id
+            user_id = user['id']
         else:
-            user_name = '127.0.0.1'
             user_id = 'not logged in'
 
         before = self.record_details(user_id, package_dict['id'])
@@ -431,7 +466,7 @@ def _delete_extra(self, package_dict, user):
 
         # Update the package's first extra.
         del package_dict['extras'][0]
-        updated_package = package_update(self.app, package_dict, user.apikey)
+        updated_package = package_update(self.app, package_dict, user['apikey'])
 
         after = self.record_details(user_id, package_dict['id'])
         extras_after = updated_package['extras']
@@ -497,10 +532,8 @@ def _delete_extra(self, package_dict, user):
 
     def _update_extra(self, package_dict, user):
         if user:
-            user_name = user.name
-            user_id = user.id
+            user_id = user['id']
         else:
-            user_name = '127.0.0.1'
             user_id = 'not logged in'
 
         before = self.record_details(user_id, package_dict['id'])
@@ -516,7 +549,8 @@ def _update_extra(self, package_dict, user):
         else:
             assert extras[0]['value'] != '"edited again"'
             extras[0]['value'] = '"edited again"'
-        updated_package = package_update(self.app, package_dict, user.apikey)
+        updated_package = package_update(self.app, package_dict,
+                user['apikey'])
 
         after = self.record_details(user_id, package_dict['id'])
         extras_after = updated_package['extras']
@@ -584,10 +618,8 @@ def _add_extra(self, package_dict, user, key=None):
         if key is None:
             key = 'quality'
         if user:
-            user_name = user.name
-            user_id = user.id
+            user_id = user['id']
         else:
-            user_name = '127.0.0.1'
             user_id = 'not logged in'
 
         before = self.record_details(user_id, package_dict['id'])
@@ -599,7 +631,8 @@ def _add_extra(self, package_dict, user, key=None):
         # Create a new extra.
         extras = package_dict['extras']
         extras.append({'key': key, 'value': '10000'})
-        updated_package = package_update(self.app, package_dict, user.apikey)
+        updated_package = package_update(self.app, package_dict,
+                user['apikey'])
 
         after = self.record_details(user_id, package_dict['id'])
         extras_after = updated_package['extras']
@@ -664,14 +697,14 @@ def _add_extra(self, package_dict, user, key=None):
             str(detail['activity_type']))
 
     def _create_activity(self, user, package, params):
-        before = self.record_details(user.id, package.id)
+        before = self.record_details(user['id'], package['id'])
 
         response = self.app.post('/api/action/activity_create',
             params=json.dumps(params),
-            extra_environ={'Authorization': str(self.sysadmin_user.apikey)})
+            extra_environ={'Authorization': str(self.sysadmin_user['apikey'])})
         assert response.json['success'] == True
 
-        after = self.record_details(user.id, package.id)
+        after = self.record_details(user['id'], package['id'])
 
         # Find the new activity in the user's activity stream.
         user_new_activities = (find_new_activities(
@@ -687,7 +720,8 @@ def _create_activity(self, user, package, params):
             after['package activity stream']))
         assert pkg_new_activities == user_new_activities
 
-        self.check_dashboard(before, after, user_new_activities, [user.id, package.id])
+        self.check_dashboard(before, after, user_new_activities,
+                [user['id'], package['id']])
 
         # Check that the new activity has the right attributes.
         assert activity['object_id'] == params['object_id'], (
@@ -711,15 +745,15 @@ def _delete_group(self, group, user):
         item and detail are emitted.
 
         """
-        before = self.record_details(user.id, group_id=group.id,
-                apikey=user.apikey)
+        before = self.record_details(user['id'], group_id=group['id'],
+                apikey=user['apikey'])
 
         # Deleted the group.
-        group_dict = {'id': group.id, 'state': 'deleted'}
-        group_update(self.app, group_dict, user.apikey)
+        group_dict = {'id': group['id'], 'state': 'deleted'}
+        group_update(self.app, group_dict, user['apikey'])
 
-        after = self.record_details(user.id, group_id=group.id,
-                apikey=user.apikey)
+        after = self.record_details(user['id'], group_id=group['id'],
+                apikey=user['apikey'])
 
         # Find the new activity.
         new_activities = find_new_activities(before['user activity stream'],
@@ -732,11 +766,11 @@ def _delete_group(self, group, user):
             after['group activity stream']) == new_activities, ("The same "
             "activity should also appear in the group's activity stream.")
 
-        self.check_dashboard(before, after, new_activities, [user.id])
+        self.check_dashboard(before, after, new_activities, [user['id']])
 
         # 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'])
+        assert activity['object_id'] == group['id'], str(activity['object_id'])
+        assert activity['user_id'] == user['id'], str(activity['user_id'])
         assert activity['activity_type'] == 'deleted group', \
             str(activity['activity_type'])
         if 'id' not in activity:
@@ -754,13 +788,13 @@ def _update_group(self, group, user):
         item and detail are emitted.
 
         """
-        before = self.record_details(user.id, group_id=group.id)
+        before = self.record_details(user['id'], group_id=group['id'])
 
         # Update the group.
-        group_dict = {'id': group.id, 'title': 'edited'}
-        group_update(self.app, group_dict, user.apikey)
+        group_dict = {'id': group['id'], 'title': 'edited'}
+        group_update(self.app, group_dict, user['apikey'])
 
-        after = self.record_details(user.id, group_id=group.id)
+        after = self.record_details(user['id'], group_id=group['id'])
 
         # Find the new activity.
         new_activities = find_new_activities(before['user activity stream'],
@@ -773,11 +807,11 @@ def _update_group(self, group, user):
             after['group activity stream']) == new_activities, ("The same "
             "activity should also appear in the group's activity stream.")
 
-        self.check_dashboard(before, after, new_activities, [user.id])
+        self.check_dashboard(before, after, new_activities, [user['id']])
 
         # 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'])
+        assert activity['object_id'] == group['id'], str(activity['object_id'])
+        assert activity['user_id'] == user['id'], str(activity['user_id'])
         assert activity['activity_type'] == 'changed group', \
             str(activity['activity_type'])
         if not activity.has_key('id'):
@@ -796,8 +830,8 @@ def _update_user(self, user):
 
         """
         response = self.app.post('/api/action/user_show',
-                json.dumps({'id': user.id}),
-                extra_environ={'Authorization': str(user.apikey)})
+                json.dumps({'id': user['id']}),
+                extra_environ={'Authorization': str(user['apikey'])})
         response_dict = json.loads(response.body)
         assert response_dict['success'] is True
         user_dict = response_dict['result']
@@ -809,7 +843,7 @@ def _update_user(self, user):
         if not user_dict.get('email'):
             user_dict['email'] = 'there has to be a value in email or validate fails'
         self.app.post('/api/action/user_update', json.dumps(user_dict),
-                extra_environ={'Authorization': str(user.apikey)})
+                extra_environ={'Authorization': str(user['apikey'])})
 
         after = self.record_details(user_dict['id'])
 
@@ -843,28 +877,17 @@ def _delete_resources(self, package):
         correct activity item and detail items are emitted.
 
         """
-        before = self.record_details(self.normal_user.id, package.id)
+        before = self.record_details(self.normal_user['id'], package['id'])
 
-        # Query the model for the Package object again, as the session that it
-        # belongs to may have been closed.
-        package = model.Session.query(model.Package).get(package.id)
-        num_resources = len(package.resources)
+        num_resources = len(package['resources'])
         assert num_resources > 0, \
                 "Cannot delete resources if there aren't any."
-        resource_ids = [resource.id for resource in package.resources]
+        resource_ids = [resource['id'] for resource in package['resources']]
 
-        # Delete the resources.
-        context = {
-            'model': model,
-            'session': model.Session,
-            'user': self.normal_user.name,
-            }
-        from ckan.lib.dictization.model_dictize import package_dictize
-        data_dict = package_dictize(package, context)
-        data_dict['resources'] = []
-        package_update(self.app, data_dict, self.normal_user.apikey)
+        package['resources'] = []
+        package_update(self.app, package, self.normal_user['apikey'])
 
-        after = self.record_details(self.normal_user.id, package.id)
+        after = self.record_details(self.normal_user['id'], package['id'])
 
         # Find the new activity in the user's activity stream.
         user_new_activities = (find_new_activities(
@@ -887,12 +910,13 @@ def _delete_resources(self, package):
                 after['recently changed datasets stream']) \
                         == user_new_activities
 
-        self.check_dashboard(before, after, user_new_activities, [package.id])
+        self.check_dashboard(before, after, user_new_activities,
+                [package['id']])
 
         # Check that the new activity has the right attributes.
-        assert activity['object_id'] == package.id, (
+        assert activity['object_id'] == package['id'], (
             str(activity['object_id']))
-        assert activity['user_id'] == self.normal_user.id, (
+        assert activity['user_id'] == self.normal_user['id'], (
             str(activity['user_id']))
         assert activity['activity_type'] == 'changed package', (
             str(activity['activity_type']))
@@ -926,10 +950,8 @@ def _update_package(self, package, user):
 
         """
         if user:
-            user_name = user.name
-            user_id = user.id
+            user_id = user['id']
         else:
-            user_name = '127.0.0.1'
             user_id = 'not logged in'
 
         before = self.record_details(user_id, package['id'])
@@ -940,7 +962,7 @@ def _update_package(self, package, user):
         else:
             assert package['title'] != 'edited again'
             package['title'] = 'edited again'
-        package_update(self.app, package, user.apikey)
+        package_update(self.app, package, user['apikey'])
 
         after = self.record_details(user_id, package['id'])
 
@@ -1002,10 +1024,8 @@ def _update_resource(self, package, resource, user):
 
         """
         if user:
-            user_name = user.name
-            user_id = user.id
+            user_id = user['id']
         else:
-            user_name = '127.0.0.1'
             user_id = 'not logged in'
 
         before = self.record_details(user_id, package['id'])
@@ -1072,21 +1092,17 @@ def _delete_package(self, package):
         item and detail are emitted.
 
         """
-        before = self.record_details(self.sysadmin_user.id, package.id)
-
-        # Query for the package object again, as the session that it belongs to
-        # may have been closed.
-        package = model.Session.query(model.Package).get(package.id)
+        before = self.record_details(self.sysadmin_user['id'], package['id'])
 
         # Delete the package.
-        package_dict = {'id': package.id}
+        package_dict = {'id': package['id']}
         response = self.app.post('/api/action/package_delete',
             json.dumps(package_dict),
-            extra_environ={'Authorization': str(self.sysadmin_user.apikey)})
+            extra_environ={'Authorization': str(self.sysadmin_user['apikey'])})
         response_dict = json.loads(response.body)
         assert response_dict['success'] is True
 
-        after = self.record_details(self.sysadmin_user.id, package.id)
+        after = self.record_details(self.sysadmin_user['id'], package['id'])
 
         # Find the new activity in the user's activity stream.
         user_new_activities = (find_new_activities(
@@ -1109,12 +1125,13 @@ def _delete_package(self, package):
                 after['recently changed datasets stream']) \
                         == user_new_activities
 
-        self.check_dashboard(before, after, user_new_activities, [self.sysadmin_user.id, package.id])
+        self.check_dashboard(before, after, user_new_activities,
+                [self.sysadmin_user['id'], package['id']])
 
         # Check that the new activity has the right attributes.
-        assert activity['object_id'] == package.id, (
+        assert activity['object_id'] == package['id'], (
                 str(activity['object_id']))
-        assert activity['user_id'] == self.sysadmin_user.id, (
+        assert activity['user_id'] == self.sysadmin_user['id'], (
             str(activity['user_id']))
         assert activity['activity_type'] == 'deleted package', (
             str(activity['activity_type']))
@@ -1133,7 +1150,7 @@ def _delete_package(self, package):
         detail = details[0]
         assert detail['activity_id'] == activity['id'], \
             str(detail['activity_id'])
-        assert detail['object_id'] == package.id, str(detail['object_id'])
+        assert detail['object_id'] == package['id'], str(detail['object_id'])
         assert detail['object_type'] == "Package", (
             str(detail['object_type']))
         assert detail['activity_type'] == "deleted", (
@@ -1148,12 +1165,10 @@ def test_01_delete_resources(self):
 
         """
         packages_with_resources = []
-        for package in model.Session.query(model.Package).all():
-            # Query for the package object again, as the session that it
-            # belongs to may have been closed.
-            package = model.Session.query(model.Package).get(package.id)
-            if len(package.resources) > 0:
-                packages_with_resources.append(package)
+        for package_name in package_list(self.app):
+            package_dict = package_show(self.app, {'id': package_name})
+            if len(package_dict['resources']) > 0:
+                packages_with_resources.append(package_dict)
         assert len(packages_with_resources) > 0, \
                 "Need some packages with resources to test deleting resources."
         for package in packages_with_resources:
@@ -1167,7 +1182,7 @@ def test_01_update_group(self):
         when groups are updated.
 
         """
-        for group in model.Session.query(model.Group).all():
+        for group in group_list(self.app):
             self._update_group(group, user=self.sysadmin_user)
 
     def test_01_remove_tag(self):
@@ -1182,24 +1197,18 @@ def test_01_remove_tag(self):
         # Get a package.
         user = self.normal_user
         pkg_name = u"warandpeace"
-        context = {
-            'model': model,
-            'session': model.Session,
-            'user': user.name,
-        }
-        pkg_dict = ckan.logic.action.get.package_show(context,
-                {'id': pkg_name})
+        pkg_dict = package_show(self.app, {'id': pkg_name}, user['apikey'])
 
         # Remove one tag from the package.
         assert len(pkg_dict['tags']) >= 1, ("The package has to have at least"
                 " one tag to test removing a tag.")
-        before = self.record_details(user.id, pkg_dict['id'])
+        before = self.record_details(user['id'], pkg_dict['id'])
         data_dict = {
             'id': pkg_dict['id'],
             'tags': pkg_dict['tags'][0:-1],
             }
-        package_update(self.app, data_dict, user.apikey)
-        after = self.record_details(user.id, pkg_dict['id'])
+        package_update(self.app, data_dict, user['apikey'])
+        after = self.record_details(user['id'], pkg_dict['id'])
 
         # Find the new activity in the user's activity stream.
         user_new_activities = (find_new_activities(
@@ -1222,12 +1231,13 @@ def test_01_remove_tag(self):
                 after['recently changed datasets stream']) \
                         == user_new_activities
 
-        self.check_dashboard(before, after, user_new_activities, [user.id, pkg_dict['id']])
+        self.check_dashboard(before, after, user_new_activities,
+                [user['id'], pkg_dict['id']])
 
         # Check that the new activity has the right attributes.
         assert activity['object_id'] == pkg_dict['id'], (
             str(activity['object_id']))
-        assert activity['user_id'] == user.id, str(activity['user_id'])
+        assert activity['user_id'] == user['id'], str(activity['user_id'])
         assert activity['activity_type'] == 'changed package', (
             str(activity['activity_type']))
         if not activity.has_key('id'):
@@ -1402,8 +1412,9 @@ def test_delete_package(self):
         when packages are deleted.
 
         """
-        for package in model.Session.query(model.Package).all():
-            self._delete_package(package)
+        for package_name in package_list(self.app):
+            package_dict = package_show(self.app, {'id': package_name})
+            self._delete_package(package_dict)
 
     def test_create_user(self):
         """
@@ -1421,7 +1432,7 @@ def test_create_user(self):
                 'password': 'testpass'}
         response = self.app.post('/api/action/user_create',
             json.dumps(user_dict),
-            extra_environ={'Authorization': str(self.sysadmin_user.apikey)})
+            extra_environ={'Authorization': str(self.sysadmin_user['apikey'])})
         response_dict = json.loads(response.body)
         assert response_dict['success'] is True
         user_created = response_dict['result']
@@ -1461,25 +1472,25 @@ def test_update_user(self):
         updated.
 
         """
-        for user in model.Session.query(model.User).all():
+        for user in self.users:
             self._update_user(user)
 
     def test_create_group(self):
 
         user = self.normal_user
 
-        before = self.record_details(user.id)
+        before = self.record_details(user['id'])
 
         # Create a new group.
         request_data = {'name': 'a-new-group', 'title': 'A New Group'}
         response = self.app.post('/api/action/group_create',
             json.dumps(request_data),
-            extra_environ={'Authorization': str(user.apikey)})
+            extra_environ={'Authorization': str(user['apikey'])})
         response_dict = json.loads(response.body)
         assert response_dict['success'] is True
         group_created = response_dict['result']
 
-        after = self.record_details(user.id, group_id=group_created['id'])
+        after = self.record_details(user['id'], group_id=group_created['id'])
 
         # Find the new activity.
         new_activities = find_new_activities(before['user activity stream'],
@@ -1491,12 +1502,12 @@ def test_create_group(self):
         assert after['group activity stream'] == new_activities, ("The same "
             "activity should also appear in the group's activity stream.")
 
-        self.check_dashboard(before, after, new_activities, [user.id])
+        self.check_dashboard(before, after, new_activities, [user['id']])
 
         # Check that the new activity has the right attributes.
         assert activity['object_id'] == group_created['id'], \
             str(activity['object_id'])
-        assert activity['user_id'] == user.id, str(activity['user_id'])
+        assert activity['user_id'] == user['id'], str(activity['user_id'])
         assert activity['activity_type'] == 'new group', \
             str(activity['activity_type'])
         if not activity.has_key('id'):
@@ -1516,7 +1527,7 @@ def test_delete_group(self):
         when groups are deleted.
 
         """
-        for group in model.Session.query(model.Group).all():
+        for group in group_list(self.app):
             self._delete_group(group, self.sysadmin_user)
 
     def test_add_tag(self):
@@ -1531,23 +1542,16 @@ def test_add_tag(self):
         # Get a package.
         user = self.normal_user
         pkg_name = u"warandpeace"
-        context = {
-            'model': model,
-            'session': model.Session,
-            'user': user.name,
-            'allow_partial_update': True
-        }
-        pkg_dict = ckan.logic.action.get.package_show(context,
-                {'id': pkg_name})
+        pkg_dict = package_show(self.app, {'id': pkg_name})
 
         # Add one new tag to the package.
-        before = self.record_details(user.id, pkg_dict['id'])
+        before = self.record_details(user['id'], pkg_dict['id'])
         new_tag_name = 'test tag'
         assert new_tag_name not in [tag['name'] for tag in pkg_dict['tags']]
 
         pkg_dict['tags'].append({'name': new_tag_name})
-        package_update(self.app, pkg_dict, user.apikey)
-        after = self.record_details(user.id, pkg_dict['id'])
+        package_update(self.app, pkg_dict, user['apikey'])
+        after = self.record_details(user['id'], pkg_dict['id'])
 
         # Find the new activity in the user's activity stream.
         user_new_activities = (find_new_activities(
@@ -1570,12 +1574,12 @@ def test_add_tag(self):
                 after['recently changed datasets stream']) \
                         == user_new_activities
 
-        self.check_dashboard(before, after, user_new_activities, [user.id, pkg_dict['id']])
+        self.check_dashboard(before, after, user_new_activities, [user['id'], pkg_dict['id']])
 
         # Check that the new activity has the right attributes.
         assert activity['object_id'] == pkg_dict['id'], (
             str(activity['object_id']))
-        assert activity['user_id'] == user.id, str(activity['user_id'])
+        assert activity['user_id'] == user['id'], str(activity['user_id'])
         assert activity['activity_type'] == 'changed package', (
             str(activity['activity_type']))
         if not activity.has_key('id'):
@@ -1604,8 +1608,8 @@ def test_activity_create_successful_no_data(self):
 
         """
         params = {
-            'user_id': self.sysadmin_user.id,
-            'object_id': self.warandpeace.id,
+            'user_id': self.sysadmin_user['id'],
+            'object_id': self.warandpeace['id'],
             'activity_type': 'changed package',
         }
         self._create_activity(self.sysadmin_user, self.warandpeace, params)
@@ -1615,8 +1619,8 @@ def test_activity_create_successful_with_data(self):
 
         """
         params = {
-            'user_id': self.sysadmin_user.id,
-            'object_id': self.annakarenina.id,
+            'user_id': self.sysadmin_user['id'],
+            'object_id': self.annakarenina['id'],
             'activity_type': 'deleted package',
             'data': {'a': 1, 'b': 2, 'c': 3}
         }
@@ -1628,8 +1632,8 @@ def test_activity_create_no_authorization(self):
 
         """
         params = {
-            'user_id': self.sysadmin_user.id,
-            'object_id': self.warandpeace.id,
+            'user_id': self.sysadmin_user['id'],
+            'object_id': self.warandpeace['id'],
             'activity_type': 'changed package',
         }
         response = self.app.post('/api/action/activity_create',
@@ -1643,13 +1647,13 @@ def test_activity_create_not_authorized(self):
 
         """
         params = {
-            'user_id': self.normal_user.id,
-            'object_id': self.warandpeace.id,
+            'user_id': self.normal_user['id'],
+            'object_id': self.warandpeace['id'],
             'activity_type': 'changed package',
         }
         response = self.app.post('/api/action/activity_create',
             params=json.dumps(params),
-            extra_environ={'Authorization': str(self.normal_user.apikey)},
+            extra_environ={'Authorization': str(self.normal_user['apikey'])},
             status=403)
         assert response.json['success'] == False
 
@@ -1660,8 +1664,8 @@ def test_activity_create_authorization_not_exists(self):
 
         """
         params = {
-            'user_id': self.normal_user.id,
-            'object_id': self.warandpeace.id,
+            'user_id': self.normal_user['id'],
+            'object_id': self.warandpeace['id'],
             'activity_type': 'changed package',
         }
         response = self.app.post('/api/action/activity_create',
@@ -1680,15 +1684,15 @@ def test_activity_create_with_id(self):
         package = self.warandpeace
         params = {
             'id': activity_id,
-            'user_id': user.id,
-            'object_id': package.id,
+            'user_id': user['id'],
+            'object_id': package['id'],
             'activity_type': 'changed package',
         }
         self._create_activity(self.sysadmin_user, self.warandpeace, params)
         assert activity_id not in [activity['id'] for activity in
-                self.user_activity_stream(user.id)]
+                self.user_activity_stream(user['id'])]
         assert activity_id not in [activity['id'] for activity in
-                self.package_activity_stream(package.id)]
+                self.package_activity_stream(package['id'])]
 
     def test_activity_create_with_timestamp(self):
         """Test that a timestamp passed to the activity_create API is ignored
@@ -1696,8 +1700,8 @@ def test_activity_create_with_timestamp(self):
 
         """
         params = {
-            'user_id': self.sysadmin_user.id,
-            'object_id': self.warandpeace.id,
+            'user_id': self.sysadmin_user['id'],
+            'object_id': self.warandpeace['id'],
             'activity_type': 'changed package',
             'timestamp': str(datetime.datetime.max),
         }
@@ -1715,15 +1719,15 @@ def test_activity_create_with_revision(self):
         package = self.warandpeace
         params = {
             'revision_id': revision_id,
-            'user_id': user.id,
-            'object_id': package.id,
+            'user_id': user['id'],
+            'object_id': package['id'],
             'activity_type': 'changed package',
         }
         self._create_activity(self.sysadmin_user, self.warandpeace, params)
         assert revision_id not in [activity['revision_id'] for activity in
-                self.user_activity_stream(user.id)]
+                self.user_activity_stream(user['id'])]
         assert revision_id not in [activity['revision_id'] for activity in
-                self.package_activity_stream(package.id)]
+                self.package_activity_stream(package['id'])]
 
     def test_activity_create_user_id_missing(self):
         """Test the error response when the activity_create API is called with
@@ -1731,12 +1735,12 @@ def test_activity_create_user_id_missing(self):
 
         """
         params = {
-            'object_id': self.warandpeace.id,
+            'object_id': self.warandpeace['id'],
             'activity_type': 'changed package',
         }
         response = self.app.post('/api/action/activity_create',
             params=json.dumps(params),
-            extra_environ={'Authorization': str(self.sysadmin_user.apikey)},
+            extra_environ={'Authorization': str(self.sysadmin_user['apikey'])},
             status=409)
         assert response.json['success'] == False
         assert response.json['error'][u'__type'] == u'Validation Error'
@@ -1750,12 +1754,12 @@ def test_activity_create_user_id_empty(self):
         """
         params = {
             'user_id': '',
-            'object_id': self.warandpeace.id,
+            'object_id': self.warandpeace['id'],
             'activity_type': 'changed package',
         }
         response = self.app.post('/api/action/activity_create',
             params=json.dumps(params),
-            extra_environ={'Authorization': str(self.sysadmin_user.apikey)},
+            extra_environ={'Authorization': str(self.sysadmin_user['apikey'])},
             status=409)
         assert response.json['success'] == False
         assert response.json['error'][u'__type'] == u'Validation Error'
@@ -1765,7 +1769,7 @@ def test_activity_create_user_id_empty(self):
         params['user_id'] = None
         response = self.app.post('/api/action/activity_create',
             params=json.dumps(params),
-            extra_environ={'Authorization': str(self.sysadmin_user.apikey)},
+            extra_environ={'Authorization': str(self.sysadmin_user['apikey'])},
             status=409)
         assert response.json['success'] == False
         assert response.json['error'][u'__type'] == u'Validation Error'
@@ -1779,12 +1783,12 @@ def test_activity_create_user_id_does_not_exist(self):
         """
         params = {
             'user_id': '1234567890abcdefghijk',
-            'object_id': self.warandpeace.id,
+            'object_id': self.warandpeace['id'],
             'activity_type': 'changed package',
         }
         response = self.app.post('/api/action/activity_create',
             params=json.dumps(params),
-            extra_environ={'Authorization': str(self.sysadmin_user.apikey)},
+            extra_environ={'Authorization': str(self.sysadmin_user['apikey'])},
             status=409)
         assert response.json['success'] == False
         assert response.json['error'][u'__type'] == u'Validation Error'
@@ -1798,12 +1802,12 @@ def test_activity_create_object_id_missing(self):
 
         """
         params = {
-            'user_id': self.sysadmin_user.id,
+            'user_id': self.sysadmin_user['id'],
             'activity_type': 'changed package',
         }
         response = self.app.post('/api/action/activity_create',
             params=json.dumps(params),
-            extra_environ={'Authorization': str(self.sysadmin_user.apikey)},
+            extra_environ={'Authorization': str(self.sysadmin_user['apikey'])},
             status=409)
         assert response.json['success'] == False
         assert response.json['error'][u'__type'] == u'Validation Error'
@@ -1818,12 +1822,12 @@ def test_activity_create_object_id_empty(self):
         """
         params = {
             'object_id': '',
-            'user_id': self.sysadmin_user.id,
+            'user_id': self.sysadmin_user['id'],
             'activity_type': 'changed package',
         }
         response = self.app.post('/api/action/activity_create',
             params=json.dumps(params),
-            extra_environ={'Authorization': str(self.sysadmin_user.apikey)},
+            extra_environ={'Authorization': str(self.sysadmin_user['apikey'])},
             status=409)
         assert response.json['success'] == False
         assert response.json['error'][u'__type'] == u'Validation Error'
@@ -1834,7 +1838,7 @@ def test_activity_create_object_id_empty(self):
         params['object_id'] = None
         response = self.app.post('/api/action/activity_create',
             params=json.dumps(params),
-            extra_environ={'Authorization': str(self.sysadmin_user.apikey)},
+            extra_environ={'Authorization': str(self.sysadmin_user['apikey'])},
             status=409)
         assert response.json['success'] == False
         assert response.json['error'][u'__type'] == u'Validation Error'
@@ -1849,12 +1853,12 @@ def test_activity_create_object_id_does_not_exist(self):
         """
         params = {
             'object_id': '1234567890qwertyuiop',
-            'user_id': self.sysadmin_user.id,
+            'user_id': self.sysadmin_user['id'],
             'activity_type': 'changed package',
         }
         response = self.app.post('/api/action/activity_create',
             params=json.dumps(params),
-            extra_environ={'Authorization': str(self.sysadmin_user.apikey)},
+            extra_environ={'Authorization': str(self.sysadmin_user['apikey'])},
             status=409)
         assert response.json['success'] == False
         assert response.json['error'][u'__type'] == u'Validation Error'
@@ -1868,12 +1872,12 @@ def test_activity_create_activity_type_missing(self):
 
         """
         params = {
-            'user_id': self.normal_user.id,
-            'object_id': self.warandpeace.id,
+            'user_id': self.normal_user['id'],
+            'object_id': self.warandpeace['id'],
         }
         response = self.app.post('/api/action/activity_create',
             params=json.dumps(params),
-            extra_environ={'Authorization': str(self.sysadmin_user.apikey)},
+            extra_environ={'Authorization': str(self.sysadmin_user['apikey'])},
             status=409)
         assert response.json['success'] == False
         assert response.json['error'][u'__type'] == u'Validation Error'
@@ -1887,13 +1891,13 @@ def test_activity_create_activity_type_empty(self):
 
         """
         params = {
-            'user_id': self.normal_user.id,
-            'object_id': self.warandpeace.id,
+            'user_id': self.normal_user['id'],
+            'object_id': self.warandpeace['id'],
             'activity_type': ''
         }
         response = self.app.post('/api/action/activity_create',
             params=json.dumps(params),
-            extra_environ={'Authorization': str(self.sysadmin_user.apikey)},
+            extra_environ={'Authorization': str(self.sysadmin_user['apikey'])},
             status=409)
         assert response.json['success'] == False
         assert response.json['error'][u'__type'] == u'Validation Error'
@@ -1904,7 +1908,7 @@ def test_activity_create_activity_type_empty(self):
         params['activity_type'] = None
         response = self.app.post('/api/action/activity_create',
             params=json.dumps(params),
-            extra_environ={'Authorization': str(self.sysadmin_user.apikey)},
+            extra_environ={'Authorization': str(self.sysadmin_user['apikey'])},
             status=409)
         assert response.json['success'] == False
         assert response.json['error'][u'__type'] == u'Validation Error'
@@ -1918,13 +1922,13 @@ def test_activity_create_activity_type_not_exists(self):
 
         """
         params = {
-            'user_id': self.normal_user.id,
-            'object_id': self.warandpeace.id,
+            'user_id': self.normal_user['id'],
+            'object_id': self.warandpeace['id'],
             'activity_type': 'foobar'
         }
         response = self.app.post('/api/action/activity_create',
             params=json.dumps(params),
-            extra_environ={'Authorization': str(self.sysadmin_user.apikey)},
+            extra_environ={'Authorization': str(self.sysadmin_user['apikey'])},
             status=409)
         assert response.json['success'] == False
         assert response.json['error'][u'__type'] == u'Validation Error'
@@ -2013,15 +2017,15 @@ def test_delete_extras_not_logged_in(self):
 
     def test_follow_dataset(self):
         user = self.normal_user
-        before = self.record_details(user.id)
-        data = {'id': self.warandpeace.id}
-        extra_environ = {'Authorization': str(user.apikey)}
+        before = self.record_details(user['id'])
+        data = {'id': self.warandpeace['id']}
+        extra_environ = {'Authorization': str(user['apikey'])}
         response = self.app.post('/api/action/follow_dataset',
             json.dumps(data), extra_environ=extra_environ)
         response_dict = json.loads(response.body)
         assert response_dict['success'] is True
 
-        after = self.record_details(user.id, self.warandpeace.id)
+        after = self.record_details(user['id'], self.warandpeace['id'])
 
         # Find the new activity in the user's activity stream.
         user_new_activities = (find_new_activities(
@@ -2036,12 +2040,12 @@ def test_follow_dataset(self):
         for activity in user_new_activities:
             assert activity in pkg_new_activities
 
-        self.check_dashboard(before, after, user_new_activities, [user.id])
+        self.check_dashboard(before, after, user_new_activities, [user['id']])
 
         # Check that the new activity has the right attributes.
-        assert activity['object_id'] == self.warandpeace.id, \
+        assert activity['object_id'] == self.warandpeace['id'], \
             str(activity['object_id'])
-        assert activity['user_id'] == user.id, str(activity['user_id'])
+        assert activity['user_id'] == user['id'], str(activity['user_id'])
         assert activity['activity_type'] == 'follow dataset', \
             str(activity['activity_type'])
         if not activity.has_key('id'):
@@ -2057,17 +2061,17 @@ def test_follow_dataset(self):
 
     def test_follow_user(self):
         user = self.normal_user
-        before = self.record_details(user.id)
-        followee_before = self.record_details(self.sysadmin_user.id)
-        data = {'id': self.sysadmin_user.id}
-        extra_environ = {'Authorization': str(user.apikey)}
+        before = self.record_details(user['id'])
+        followee_before = self.record_details(self.sysadmin_user['id'])
+        data = {'id': self.sysadmin_user['id']}
+        extra_environ = {'Authorization': str(user['apikey'])}
         response = self.app.post('/api/action/follow_user',
             json.dumps(data), extra_environ=extra_environ)
         response_dict = json.loads(response.body)
         assert response_dict['success'] is True
 
-        after = self.record_details(user.id)
-        followee_after = self.record_details(self.sysadmin_user.id)
+        after = self.record_details(user['id'])
+        followee_after = self.record_details(self.sysadmin_user['id'])
 
         # Find the new activity in the user's activity stream.
         user_new_activities = (find_new_activities(
@@ -2098,9 +2102,9 @@ def test_follow_user(self):
         assert followee_new_activities[0] == activity
 
         # Check that the new activity has the right attributes.
-        assert activity['object_id'] == self.sysadmin_user.id, \
+        assert activity['object_id'] == self.sysadmin_user['id'], \
             str(activity['object_id'])
-        assert activity['user_id'] == user.id, str(activity['user_id'])
+        assert activity['user_id'] == user['id'], str(activity['user_id'])
         assert activity['activity_type'] == 'follow user', \
             str(activity['activity_type'])
         if not activity.has_key('id'):

From 7ebf0c1e4ecc48661e39e13c18dc8125f39f016e Mon Sep 17 00:00:00 2001
From: Sean Hammond <seanhammond@lavabit.com>
Date: Fri, 26 Oct 2012 21:20:16 +0200
Subject: [PATCH 05/12] Fix an import in test_activity.py

Follow the CKAN Coding Standards for imports
---
 ckan/tests/functional/api/test_activity.py | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/ckan/tests/functional/api/test_activity.py b/ckan/tests/functional/api/test_activity.py
index d3d197cf542..afdfe01ba85 100644
--- a/ckan/tests/functional/api/test_activity.py
+++ b/ckan/tests/functional/api/test_activity.py
@@ -4,7 +4,7 @@
 
 from nose.plugins.skip import SkipTest
 
-from pylons.test import pylonsapp
+import pylons.test
 import paste.fixture
 from ckan.lib.helpers import json
 
@@ -175,7 +175,7 @@ def setup_class(self):
                 'id': annakarenina.id,
                 }
         self.users = [self.sysadmin_user, self.normal_user, self.follower]
-        self.app = paste.fixture.TestApp(pylonsapp)
+        self.app = paste.fixture.TestApp(pylons.test.pylonsapp)
 
         # Make follower follow everything else.
         params = {'id': 'testsysadmin'}

From e5039423d1ba9e34dddb9387efc645c2be9cd892 Mon Sep 17 00:00:00 2001
From: Sean Hammond <seanhammond@lavabit.com>
Date: Fri, 26 Oct 2012 21:35:37 +0200
Subject: [PATCH 06/12] test_activity.py PEP8 fixes

---
 ckan/tests/functional/api/test_activity.py | 166 +++++++++++----------
 1 file changed, 86 insertions(+), 80 deletions(-)

diff --git a/ckan/tests/functional/api/test_activity.py b/ckan/tests/functional/api/test_activity.py
index afdfe01ba85..2a6e8d1796c 100644
--- a/ckan/tests/functional/api/test_activity.py
+++ b/ckan/tests/functional/api/test_activity.py
@@ -88,6 +88,7 @@ def datetime_from_string(s):
     '''
     return datetime.datetime.strptime(s, '%Y-%m-%dT%H:%M:%S.%f')
 
+
 def make_resource():
     '''Return a test resource in dictionary form.'''
     return {
@@ -97,6 +98,7 @@ def make_resource():
             'name': 'example resource',
             }
 
+
 def make_package(name=None):
     '''Return a test package in dictionary form.'''
     if name is None:
@@ -104,14 +106,14 @@ def make_package(name=None):
 
     # A package with no resources, tags, extras or groups.
     pkg = {
-        'name' : name,
-        'title' : 'My Test Package',
-        'author' : 'test author',
-        'author_email' : 'test_author@test_author.com',
-        'maintainer' : 'test maintainer',
-        'maintainer_email' : 'test_maintainer@test_maintainer.com',
-        'notes' : 'some test notes',
-        'url' : 'www.example.com',
+        'name': name,
+        'title': 'My Test Package',
+        'author': 'test author',
+        'author_email': 'test_author@test_author.com',
+        'maintainer': 'test maintainer',
+        'maintainer_email': 'test_maintainer@test_maintainer.com',
+        'notes': 'some test notes',
+        'url': 'www.example.com',
         }
     # Add some resources to the package.
     res1 = {
@@ -128,17 +130,14 @@ def make_package(name=None):
         }
     pkg['resources'] = [res1, res2]
     # Add some tags to the package.
-    tag1 = { 'name': 'a_test_tag' }
-    tag2 = { 'name': 'another_test_tag' }
+    tag1 = {'name': 'a_test_tag'}
+    tag2 = {'name': 'another_test_tag'}
     pkg['tags'] = [tag1, tag2]
     return pkg
 
+
 def find_new_activities(before, after):
-    new_activities = []
-    for activity in after:
-        if activity not in before:
-            new_activities.append(activity)
-    return new_activities
+    return [activity for activity in after if activity not in before]
 
 
 class TestActivity:
@@ -208,14 +207,14 @@ def setup_class(self):
                 self.annakarenina['id']
             ]
 
-
     @classmethod
     def teardown_class(self):
         import ckan.model as model
         model.repo.rebuild_db()
 
     def dashboard_activity_stream(self, user_id):
-        response = self.app.get("/api/2/rest/user/%s/dashboard_activity" % user_id)
+        response = self.app.get(
+                "/api/2/rest/user/{0}/dashboard_activity".format(user_id))
         return json.loads(response.body)
 
     def user_activity_stream(self, user_id):
@@ -240,7 +239,7 @@ def recently_changed_datasets_stream(self):
                 '/api/action/recently_changed_packages_activity_list',
                 params=json.dumps({}),
                 status=200)
-        assert response.json['success'] == True
+        assert response.json['success'] is True
         activities = response.json['result']
         return activities
 
@@ -265,8 +264,8 @@ def record_details(self, user_id, package_id=None, group_id=None,
         details['recently changed datasets stream'] = \
                 self.recently_changed_datasets_stream()
 
-        details['follower dashboard activity stream'] = \
-                                self.dashboard_activity_stream(self.follower['id'])
+        details['follower dashboard activity stream'] = (
+                        self.dashboard_activity_stream(self.follower['id']))
 
         details['time'] = datetime.datetime.now()
         return details
@@ -278,12 +277,12 @@ def check_dashboard(
         difference = find_new_activities(
                     before['follower dashboard activity stream'],
                     after['follower dashboard activity stream'])
-        if any(potential_followee in self.followees for potential_followee in potential_followees):
+        if any(potential_followee in self.followees
+                for potential_followee in potential_followees):
             assert difference == wanted_difference
         else:
             assert len(difference) == 0
 
-
     def _create_package(self, user, name=None):
         if user:
             user_id = user['id']
@@ -330,10 +329,10 @@ def _create_package(self, user, name=None):
         assert activity['user_id'] == user_id, str(activity['user_id'])
         assert activity['activity_type'] == 'new package', \
             str(activity['activity_type'])
-        if not activity.has_key('id'):
+        if 'id' not in activity:
             assert False, "activity object should have an id value"
         # TODO: Test for the _correct_ revision_id value.
-        if not activity.has_key('revision_id'):
+        if 'revision_id' not in activity:
             assert False, "activity object should have a revision_id value"
         timestamp = datetime_from_string(activity['timestamp'])
         assert timestamp >= before['time'] and timestamp <= \
@@ -426,10 +425,10 @@ def _add_resource(self, package, user):
         assert activity['user_id'] == user_id, str(activity['user_id'])
         assert activity['activity_type'] == 'changed package', \
             str(activity['activity_type'])
-        if not activity.has_key('id'):
+        if 'id' not in activity:
             assert False, "activity object should have an id value"
         # TODO: Test for the _correct_ revision_id value.
-        if not activity.has_key('revision_id'):
+        if 'revision_id' not in activity:
             assert False, "activity object should have a revision_id value"
         timestamp = datetime_from_string(activity['timestamp'])
         assert (timestamp >= before['time'] and
@@ -437,7 +436,8 @@ def _add_resource(self, package, user):
 
         # Test for the presence of a correct activity detail item.
         details = self.activity_details(activity)
-        assert len(details) == 1, [(detail['activity_type'], detail['object_type']) for detail in details]
+        assert len(details) == 1, [(detail['activity_type'],
+            detail['object_type']) for detail in details]
         detail = details[0]
         assert detail['activity_id'] == activity['id'], \
             str(detail['activity_id'])
@@ -466,7 +466,8 @@ def _delete_extra(self, package_dict, user):
 
         # Update the package's first extra.
         del package_dict['extras'][0]
-        updated_package = package_update(self.app, package_dict, user['apikey'])
+        updated_package = package_update(self.app, package_dict,
+                user['apikey'])
 
         after = self.record_details(user_id, package_dict['id'])
         extras_after = updated_package['extras']
@@ -494,7 +495,8 @@ def _delete_extra(self, package_dict, user):
                 after['recently changed datasets stream']) \
                         == user_new_activities
 
-        self.check_dashboard(before, after, user_new_activities, [user_id, package_dict['id']])
+        self.check_dashboard(before, after, user_new_activities,
+                [user_id, package_dict['id']])
 
         # Check that the new activity has the right attributes.
         assert activity['object_id'] == updated_package['id'], \
@@ -502,10 +504,10 @@ def _delete_extra(self, package_dict, user):
         assert activity['user_id'] == user_id, str(activity['user_id'])
         assert activity['activity_type'] == 'changed package', \
             str(activity['activity_type'])
-        if not activity.has_key('id'):
+        if 'id' not in activity:
             assert False, "activity object should have an id value"
         # TODO: Test for the _correct_ revision_id value.
-        if not activity.has_key('revision_id'):
+        if 'revision_id' not in activity:
             assert False, "activity object should have a revision_id value"
         timestamp = datetime_from_string(activity['timestamp'])
         assert (timestamp >= before['time'] and
@@ -578,7 +580,8 @@ def _update_extra(self, package_dict, user):
                 after['recently changed datasets stream']) \
                         == user_new_activities
 
-        self.check_dashboard(before, after, user_new_activities, [user_id, package_dict['id']])
+        self.check_dashboard(before, after, user_new_activities,
+                [user_id, package_dict['id']])
 
         # Check that the new activity has the right attributes.
         assert activity['object_id'] == updated_package['id'], \
@@ -586,10 +589,10 @@ def _update_extra(self, package_dict, user):
         assert activity['user_id'] == user_id, str(activity['user_id'])
         assert activity['activity_type'] == 'changed package', \
             str(activity['activity_type'])
-        if not activity.has_key('id'):
+        if 'id' not in activity:
             assert False, "activity object should have an id value"
         # TODO: Test for the _correct_ revision_id value.
-        if not activity.has_key('revision_id'):
+        if 'revision_id' not in activity:
             assert False, "activity object should have a revision_id value"
         timestamp = datetime_from_string(activity['timestamp'])
         assert (timestamp >= before['time'] and
@@ -660,7 +663,8 @@ def _add_extra(self, package_dict, user, key=None):
                 after['recently changed datasets stream']) \
                         == user_new_activities
 
-        self.check_dashboard(before, after, user_new_activities, [user_id, package_dict['id']])
+        self.check_dashboard(before, after, user_new_activities,
+                [user_id, package_dict['id']])
 
         # Check that the new activity has the right attributes.
         assert activity['object_id'] == updated_package['id'], \
@@ -668,10 +672,10 @@ def _add_extra(self, package_dict, user, key=None):
         assert activity['user_id'] == user_id, str(activity['user_id'])
         assert activity['activity_type'] == 'changed package', \
             str(activity['activity_type'])
-        if not activity.has_key('id'):
+        if 'id' not in activity:
             assert False, "activity object should have an id value"
         # TODO: Test for the _correct_ revision_id value.
-        if not activity.has_key('revision_id'):
+        if 'revision_id' not in activity:
             assert False, "activity object should have a revision_id value"
         timestamp = datetime_from_string(activity['timestamp'])
         assert (timestamp >= before['time'] and
@@ -702,7 +706,7 @@ def _create_activity(self, user, package, params):
         response = self.app.post('/api/action/activity_create',
             params=json.dumps(params),
             extra_environ={'Authorization': str(self.sysadmin_user['apikey'])})
-        assert response.json['success'] == True
+        assert response.json['success'] is True
 
         after = self.record_details(user['id'], package['id'])
 
@@ -730,10 +734,10 @@ def _create_activity(self, user, package, params):
             str(activity['user_id']))
         assert activity['activity_type'] == params['activity_type'], (
             str(activity['activity_type']))
-        if not activity.has_key('id'):
+        if 'id' not in activity:
             assert False, "activity object has no id value"
         # TODO: Test for the _correct_ revision_id value.
-        if not activity.has_key('revision_id'):
+        if 'revision_id' not in activity:
             assert False, "activity has no revision_id value"
         timestamp = datetime_from_string(activity['timestamp'])
         assert (timestamp >= before['time'] and
@@ -814,10 +818,10 @@ def _update_group(self, group, user):
         assert activity['user_id'] == user['id'], str(activity['user_id'])
         assert activity['activity_type'] == 'changed group', \
             str(activity['activity_type'])
-        if not activity.has_key('id'):
+        if 'id' not in activity:
             assert False, "activity object has no id value"
         # TODO: Test for the _correct_ revision_id value.
-        if not activity.has_key('revision_id'):
+        if 'revision_id' not in activity:
             assert False, "activity has no revision_id value"
         timestamp = datetime_from_string(activity['timestamp'])
         assert timestamp >= before['time'] and timestamp <= after['time'], \
@@ -841,7 +845,7 @@ def _update_user(self, user):
         # Update the user.
         user_dict['about'] = 'edited'
         if not user_dict.get('email'):
-            user_dict['email'] = 'there has to be a value in email or validate fails'
+            user_dict['email'] = 'there has to be a value in email'
         self.app.post('/api/action/user_update', json.dumps(user_dict),
                 extra_environ={'Authorization': str(user['apikey'])})
 
@@ -862,10 +866,10 @@ def _update_user(self, user):
         assert activity['user_id'] == user_dict['id'], str(activity['user_id'])
         assert activity['activity_type'] == 'changed user', (
             str(activity['activity_type']))
-        if not activity.has_key('id'):
+        if 'id' not in activity:
             assert False, "activity object has no id value"
         # TODO: Test for the _correct_ revision_id value.
-        if not activity.has_key('revision_id'):
+        if 'revision_id' not in activity:
             assert False, "activity has no revision_id value"
         timestamp = datetime_from_string(activity['timestamp'])
         assert timestamp >= before['time'] and timestamp <= after['time'], \
@@ -920,10 +924,10 @@ def _delete_resources(self, package):
             str(activity['user_id']))
         assert activity['activity_type'] == 'changed package', (
             str(activity['activity_type']))
-        if not activity.has_key('id'):
+        if 'id' not in activity:
             assert False, "activity object has no id value"
         # TODO: Test for the _correct_ revision_id value.
-        if not activity.has_key('revision_id'):
+        if 'revision_id' not in activity:
             assert False, "activity has no revision_id value"
         timestamp = datetime_from_string(activity['timestamp'])
         assert timestamp >= before['time'], str(activity['timestamp'])
@@ -996,10 +1000,10 @@ def _update_package(self, package, user):
         assert activity['user_id'] == user_id, str(activity['user_id'])
         assert activity['activity_type'] == 'changed package', (
             str(activity['activity_type']))
-        if not activity.has_key('id'):
+        if 'id' not in activity:
             assert False, "activity object has no id value"
         # TODO: Test for the _correct_ revision_id value.
-        if not activity.has_key('revision_id'):
+        if 'revision_id' not in activity:
             assert False, "activity has no revision_id value"
         timestamp = datetime_from_string(activity['timestamp'])
         assert (timestamp >= before['time'] and
@@ -1057,7 +1061,8 @@ def _update_resource(self, package, resource, user):
                 after['recently changed datasets stream']) \
                         == user_new_activities
 
-        self.check_dashboard(before, after, user_new_activities, [user_id, package['id']])
+        self.check_dashboard(before, after, user_new_activities,
+                [user_id, package['id']])
 
         # Check that the new activity has the right attributes.
         assert activity['object_id'] == package['id'], (
@@ -1135,10 +1140,10 @@ def _delete_package(self, package):
             str(activity['user_id']))
         assert activity['activity_type'] == 'deleted package', (
             str(activity['activity_type']))
-        if not activity.has_key('id'):
+        if 'id' not in activity:
             assert False, "activity object has no id value"
         # TODO: Test for the _correct_ revision_id value.
-        if not activity.has_key('revision_id'):
+        if 'revision_id' not in activity:
             assert False, "activity has no revision_id value"
         timestamp = datetime_from_string(activity['timestamp'])
         assert (timestamp >= before['time'] and
@@ -1240,10 +1245,10 @@ def test_01_remove_tag(self):
         assert activity['user_id'] == user['id'], str(activity['user_id'])
         assert activity['activity_type'] == 'changed package', (
             str(activity['activity_type']))
-        if not activity.has_key('id'):
+        if 'id' not in activity:
             assert False, "activity object has no id value"
         # TODO: Test for the _correct_ revision_id value.
-        if not activity.has_key('revision_id'):
+        if 'revision_id' not in activity:
             assert False, "activity has no revision_id value"
         timestamp = datetime_from_string(activity['timestamp'])
         assert (timestamp >= before['time'] and
@@ -1451,10 +1456,10 @@ def test_create_user(self):
             str(activity['user_id'])
         assert activity['activity_type'] == 'new user', \
             str(activity['activity_type'])
-        if not activity.has_key('id'):
+        if 'id' not in activity:
             assert False, "activity object should have an id value"
         # TODO: Test for the _correct_ revision_id value.
-        if not activity.has_key('revision_id'):
+        if 'revision_id' not in activity:
             assert False, "activity object should have a revision_id value"
         timestamp = datetime_from_string(activity['timestamp'])
         assert timestamp >= before and timestamp <= after['time'], \
@@ -1510,10 +1515,10 @@ def test_create_group(self):
         assert activity['user_id'] == user['id'], str(activity['user_id'])
         assert activity['activity_type'] == 'new group', \
             str(activity['activity_type'])
-        if not activity.has_key('id'):
+        if 'id' not in activity:
             assert False, "activity object should have an id value"
         # TODO: Test for the _correct_ revision_id value.
-        if not activity.has_key('revision_id'):
+        if 'revision_id' not in activity:
             assert False, "activity object should have a revision_id value"
         timestamp = datetime_from_string(activity['timestamp'])
         assert timestamp >= before['time'] and timestamp <= after['time'], \
@@ -1574,7 +1579,8 @@ def test_add_tag(self):
                 after['recently changed datasets stream']) \
                         == user_new_activities
 
-        self.check_dashboard(before, after, user_new_activities, [user['id'], pkg_dict['id']])
+        self.check_dashboard(before, after, user_new_activities,
+                [user['id'], pkg_dict['id']])
 
         # Check that the new activity has the right attributes.
         assert activity['object_id'] == pkg_dict['id'], (
@@ -1582,10 +1588,10 @@ def test_add_tag(self):
         assert activity['user_id'] == user['id'], str(activity['user_id'])
         assert activity['activity_type'] == 'changed package', (
             str(activity['activity_type']))
-        if not activity.has_key('id'):
+        if 'id' not in activity:
             assert False, "activity object has no id value"
         # TODO: Test for the _correct_ revision_id value.
-        if not activity.has_key('revision_id'):
+        if 'revision_id' not in activity:
             assert False, "activity has no revision_id value"
         timestamp = datetime_from_string(activity['timestamp'])
         assert (timestamp >= before['time'] and
@@ -1638,7 +1644,7 @@ def test_activity_create_no_authorization(self):
         }
         response = self.app.post('/api/action/activity_create',
             params=json.dumps(params), status=403)
-        assert response.json['success'] == False
+        assert response.json['success'] is False
 
     def test_activity_create_not_authorized(self):
         """Test the error response when the activity_create API is called
@@ -1655,7 +1661,7 @@ def test_activity_create_not_authorized(self):
             params=json.dumps(params),
             extra_environ={'Authorization': str(self.normal_user['apikey'])},
             status=403)
-        assert response.json['success'] == False
+        assert response.json['success'] is False
 
     def test_activity_create_authorization_not_exists(self):
         """Test the error response when the activity_create API is called
@@ -1672,7 +1678,7 @@ def test_activity_create_authorization_not_exists(self):
             params=json.dumps(params),
             extra_environ={'Authorization': 'xxxxxxxxxx'},
             status=403)
-        assert response.json['success'] == False
+        assert response.json['success'] is False
 
     def test_activity_create_with_id(self):
         """Test that an ID passed to the activity_create API is ignored and not
@@ -1742,7 +1748,7 @@ def test_activity_create_user_id_missing(self):
             params=json.dumps(params),
             extra_environ={'Authorization': str(self.sysadmin_user['apikey'])},
             status=409)
-        assert response.json['success'] == False
+        assert response.json['success'] is False
         assert response.json['error'][u'__type'] == u'Validation Error'
         assert response.json['error'][u'user_id'] == [u'Missing value'], (
                 response.json['error'][u'user_id'])
@@ -1761,7 +1767,7 @@ def test_activity_create_user_id_empty(self):
             params=json.dumps(params),
             extra_environ={'Authorization': str(self.sysadmin_user['apikey'])},
             status=409)
-        assert response.json['success'] == False
+        assert response.json['success'] is False
         assert response.json['error'][u'__type'] == u'Validation Error'
         assert response.json['error'][u'user_id'] == [u'Missing value'], (
                 response.json['error'][u'user_id'])
@@ -1771,7 +1777,7 @@ def test_activity_create_user_id_empty(self):
             params=json.dumps(params),
             extra_environ={'Authorization': str(self.sysadmin_user['apikey'])},
             status=409)
-        assert response.json['success'] == False
+        assert response.json['success'] is False
         assert response.json['error'][u'__type'] == u'Validation Error'
         assert response.json['error'][u'user_id'] == [u'Missing value'], (
                 response.json['error'][u'user_id'])
@@ -1790,7 +1796,7 @@ def test_activity_create_user_id_does_not_exist(self):
             params=json.dumps(params),
             extra_environ={'Authorization': str(self.sysadmin_user['apikey'])},
             status=409)
-        assert response.json['success'] == False
+        assert response.json['success'] is False
         assert response.json['error'][u'__type'] == u'Validation Error'
         assert response.json['error'][u'user_id'] == [
                 u'Not found: User'], (
@@ -1809,7 +1815,7 @@ def test_activity_create_object_id_missing(self):
             params=json.dumps(params),
             extra_environ={'Authorization': str(self.sysadmin_user['apikey'])},
             status=409)
-        assert response.json['success'] == False
+        assert response.json['success'] is False
         assert response.json['error'][u'__type'] == u'Validation Error'
         assert response.json['error'][u'object_id'] == [
                 u'Missing value'], (
@@ -1829,7 +1835,7 @@ def test_activity_create_object_id_empty(self):
             params=json.dumps(params),
             extra_environ={'Authorization': str(self.sysadmin_user['apikey'])},
             status=409)
-        assert response.json['success'] == False
+        assert response.json['success'] is False
         assert response.json['error'][u'__type'] == u'Validation Error'
         assert response.json['error'][u'object_id'] == [
                 u'Missing value'], (
@@ -1840,7 +1846,7 @@ def test_activity_create_object_id_empty(self):
             params=json.dumps(params),
             extra_environ={'Authorization': str(self.sysadmin_user['apikey'])},
             status=409)
-        assert response.json['success'] == False
+        assert response.json['success'] is False
         assert response.json['error'][u'__type'] == u'Validation Error'
         assert response.json['error'][u'object_id'] == [
                 u'Missing value'], (
@@ -1860,7 +1866,7 @@ def test_activity_create_object_id_does_not_exist(self):
             params=json.dumps(params),
             extra_environ={'Authorization': str(self.sysadmin_user['apikey'])},
             status=409)
-        assert response.json['success'] == False
+        assert response.json['success'] is False
         assert response.json['error'][u'__type'] == u'Validation Error'
         assert response.json['error'][u'object_id'] == [
                 u'Not found: Dataset'], (
@@ -1879,7 +1885,7 @@ def test_activity_create_activity_type_missing(self):
             params=json.dumps(params),
             extra_environ={'Authorization': str(self.sysadmin_user['apikey'])},
             status=409)
-        assert response.json['success'] == False
+        assert response.json['success'] is False
         assert response.json['error'][u'__type'] == u'Validation Error'
         assert response.json['error'][u'object_id'] == [
                 u'Missing value'], (
@@ -1899,7 +1905,7 @@ def test_activity_create_activity_type_empty(self):
             params=json.dumps(params),
             extra_environ={'Authorization': str(self.sysadmin_user['apikey'])},
             status=409)
-        assert response.json['success'] == False
+        assert response.json['success'] is False
         assert response.json['error'][u'__type'] == u'Validation Error'
         assert response.json['error'][u'activity_type'] == [
                 u'Missing value'], (
@@ -1910,7 +1916,7 @@ def test_activity_create_activity_type_empty(self):
             params=json.dumps(params),
             extra_environ={'Authorization': str(self.sysadmin_user['apikey'])},
             status=409)
-        assert response.json['success'] == False
+        assert response.json['success'] is False
         assert response.json['error'][u'__type'] == u'Validation Error'
         assert response.json['error'][u'activity_type'] == [
                 u'Missing value'], (
@@ -1930,7 +1936,7 @@ def test_activity_create_activity_type_not_exists(self):
             params=json.dumps(params),
             extra_environ={'Authorization': str(self.sysadmin_user['apikey'])},
             status=409)
-        assert response.json['success'] == False
+        assert response.json['success'] is False
         assert response.json['error'][u'__type'] == u'Validation Error'
         assert response.json['error'][u'activity_type'] == [
             u"Not found: Activity type"], (
@@ -2048,10 +2054,10 @@ def test_follow_dataset(self):
         assert activity['user_id'] == user['id'], str(activity['user_id'])
         assert activity['activity_type'] == 'follow dataset', \
             str(activity['activity_type'])
-        if not activity.has_key('id'):
+        if 'id' not in activity:
             assert False, "activity object should have an id value"
         # TODO: Test for the _correct_ revision_id value.
-        if not activity.has_key('revision_id'):
+        if 'revision_id' not in activity:
             assert False, "activity object should have a revision_id value"
         timestamp = datetime_from_string(activity['timestamp'])
         assert timestamp >= before['time'] and timestamp <= \
@@ -2107,10 +2113,10 @@ def test_follow_user(self):
         assert activity['user_id'] == user['id'], str(activity['user_id'])
         assert activity['activity_type'] == 'follow user', \
             str(activity['activity_type'])
-        if not activity.has_key('id'):
+        if 'id' not in activity:
             assert False, "activity object should have an id value"
         # TODO: Test for the _correct_ revision_id value.
-        if not activity.has_key('revision_id'):
+        if 'revision_id' not in activity:
             assert False, "activity object should have a revision_id value"
         timestamp = datetime_from_string(activity['timestamp'])
         assert timestamp >= before['time'] and timestamp <= \

From 306d9c37d582f7b2d1e0101bb259d7971837d2db Mon Sep 17 00:00:00 2001
From: Sean Hammond <seanhammond@lavabit.com>
Date: Fri, 26 Oct 2012 21:41:47 +0200
Subject: [PATCH 07/12] Remove some commented-out tests

---
 ckan/tests/functional/api/test_activity.py | 120 ---------------------
 1 file changed, 120 deletions(-)

diff --git a/ckan/tests/functional/api/test_activity.py b/ckan/tests/functional/api/test_activity.py
index 2a6e8d1796c..e35f4bf5d67 100644
--- a/ckan/tests/functional/api/test_activity.py
+++ b/ckan/tests/functional/api/test_activity.py
@@ -2,8 +2,6 @@
 import logging
 logger = logging.getLogger(__name__)
 
-from nose.plugins.skip import SkipTest
-
 import pylons.test
 import paste.fixture
 from ckan.lib.helpers import json
@@ -1283,34 +1281,6 @@ def test_01_update_extras(self):
         for package_dict in packages_with_extras:
             self._update_extra(package_dict, user=self.normal_user)
 
-    def test_01_update_extras_not_logged_in(self):
-        """
-        Test changed package extra activity stream when no user logged in.
-
-        Test that correct activity stream item and detail items are emitted
-        when a package extra is changed by a user who is not logged in.
-
-        """
-        raise SkipTest
-
-##  TODO: remove, non logged in users can no longer edit packages
-#        context = {
-#            'model': model,
-#            'session': model.Session,
-#            'user': self.normal_user.name,
-#            'extras_as_string': True,
-#            }
-#        packages_with_extras = []
-#        for package_name in package_list(context, {}):
-#            package_dict = package_show(context, {'id': package_name})
-#            if len(package_dict['extras']) > 0:
-#                    packages_with_extras.append(package_dict)
-#        assert len(packages_with_extras) > 0, (
-#                "Need some packages with extras to test")
-#        for package_dict in packages_with_extras:
-#            self._update_extra(package_dict, None)
-#
-
     def test_01_update_package(self):
         """
         Test updated package activity stream.
@@ -1323,20 +1293,6 @@ def test_01_update_package(self):
             package_dict = package_show(self.app, {'id': package_name})
             self._update_package(package_dict, user=self.normal_user)
 
-    def test_01_update_package_not_logged_in(self):
-        """
-        Test updated package activity stream when not logged in.
-
-        Test that correct activity stream item and detail items are created
-        when packages are updated by a user who is not logged in.
-
-        """
-        raise SkipTest
-
-##  TODO: remove, non logged in users can no longer edit packages
-#       for package in model.Session.query(model.Package).all():
-#            self._update_package(package, user=None)
-
     def test_01_update_resource(self):
         """
         Test that a correct activity stream item and detail item are emitted
@@ -1370,19 +1326,6 @@ def test_create_package(self):
         """
         self._create_package(user=self.normal_user)
 
-    def test_create_package_not_logged_in(self):
-        """
-        Test new package activity stream when not logged in.
-
-        Test that correct activity stream item and detail items are emitted
-        when a new package is created by a user who is not logged in.
-
-        """
-        raise SkipTest
-
-##  TODO: remove, non logged in users can no longer edit packages
-#       self._create_package(user=None, name="not_logged_in_test_package")
-
     def test_add_resources(self):
         """
         Test new resource activity stream.
@@ -1395,20 +1338,6 @@ def test_add_resources(self):
             package_dict = package_show(self.app, {'id': package_name})
             self._add_resource(package_dict, user=self.normal_user)
 
-    def test_add_resources_not_logged_in(self):
-        """
-        Test new resource activity stream when no user logged in.
-
-        Test that correct activity stream item and detail items are emitted
-        when a resource is added to a package by a user who is not logged in.
-
-        """
-        raise SkipTest
-
-##  TODO: remove, non logged in users can no longer edit packages
-#        for package in model.Session.query(model.Package).all():
-#            self._add_resource(package, user=None)
-
     def test_delete_package(self):
         """
         Test deleted package activity stream.
@@ -1954,27 +1883,6 @@ def test_add_extras(self):
             package_dict = package_show(self.app, {'id': package_name})
             self._add_extra(package_dict, user=self.normal_user)
 
-    def test_add_extras_not_logged_in(self):
-        """
-        Test new package extra activity stream when no user logged in.
-
-        Test that correct activity stream item and detail items are emitted
-        when an extra is added to a package by a user who is not logged in.
-
-        """
-        raise SkipTest
-
-##  TODO: remove, non logged in users can no longer edit packages
-#        context = {
-#            'model': model,
-#            'session': model.Session,
-#            'user': self.normal_user.name,
-#            'extras_as_string': True,
-#            }
-#        for package_name in package_list(context, {}):
-#            package_dict = package_show(context, {'id': package_name})
-#            self._add_extra(package_dict, None, key='not_logged_in_extra_key')
-#
     def test_delete_extras(self):
         """
         Test deleted package extra activity stream.
@@ -1993,34 +1901,6 @@ def test_delete_extras(self):
         for package_dict in packages_with_extras:
             self._delete_extra(package_dict, user=self.normal_user)
 
-    def test_delete_extras_not_logged_in(self):
-        """
-        Test deleted package extra activity stream when no user logged in.
-
-        Test that correct activity stream item and detail items are emitted
-        when a package extra is deleted by a user who is not logged in.
-
-        """
-        raise SkipTest
-
-##  TODO: remove, non logged in users can no longer edit packages
-#        context = {
-#            'model': model,
-#            'session': model.Session,
-#            'user': self.normal_user.name,
-#            'extras_as_string': True,
-#            }
-#        packages_with_extras = []
-#        for package_name in package_list(context, {}):
-#            package_dict = package_show(context, {'id': package_name})
-#            if len(package_dict['extras']) > 0:
-#                    packages_with_extras.append(package_dict)
-#        assert len(packages_with_extras) > 0, (
-#                "Need some packages with extras to test")
-#        for package_dict in packages_with_extras:
-#            self._delete_extra(package_dict, None)
-#
-
     def test_follow_dataset(self):
         user = self.normal_user
         before = self.record_details(user['id'])

From 952ec8efbfb9da6cab6efdb9f43661b5776d9a42 Mon Sep 17 00:00:00 2001
From: Sean Hammond <seanhammond@lavabit.com>
Date: Mon, 29 Oct 2012 16:15:22 +0100
Subject: [PATCH 08/12] [#1664] Convert group names to ids in
 group_activity_list()

This fixes a bug where calling group_activity_list with the group's name
as the id parameter would return less results than if you called it with
the group's id.
---
 ckan/logic/action/get.py | 6 +++++-
 1 file changed, 5 insertions(+), 1 deletion(-)

diff --git a/ckan/logic/action/get.py b/ckan/logic/action/get.py
index 7e9e20ca592..969462ac4ab 100644
--- a/ckan/logic/action/get.py
+++ b/ckan/logic/action/get.py
@@ -1762,7 +1762,11 @@ def group_activity_list(context, data_dict):
     model = context['model']
     group_id = _get_or_bust(data_dict, 'id')
 
-    _check_access('group_show',context, data_dict)
+    # Convert group_id (could be id or name) into id.
+    group_show = logic.get_action('group_show')
+    group_id = group_show(context, {'id': group_id})['id']
+
+    _check_access('group_show', context, data_dict)
 
     # Get a list of the IDs of the group's datasets.
     group_package_show = logic.get_action('group_package_show')

From 5d926d91d00075ff144c56c3bb8af3308803f64d Mon Sep 17 00:00:00 2001
From: Sean Hammond <seanhammond@lavabit.com>
Date: Mon, 29 Oct 2012 17:19:27 +0100
Subject: [PATCH 09/12] [#1664] Add some tests for group activity streams

Add some tests for the new smarter group activity streams. There were
already tests that changes to the group itself (created, updated or
deleted group) appear in the group's activity stream, now add some tests
that changes to the group's packages appear in the group's activity
stream.

These tests are not ideal, test_activity.py needs refactoring it's too
complicated.
---
 ckan/tests/functional/api/test_activity.py | 176 +++++++++++++++++----
 1 file changed, 144 insertions(+), 32 deletions(-)

diff --git a/ckan/tests/functional/api/test_activity.py b/ckan/tests/functional/api/test_activity.py
index e35f4bf5d67..07829971b76 100644
--- a/ckan/tests/functional/api/test_activity.py
+++ b/ckan/tests/functional/api/test_activity.py
@@ -113,6 +113,7 @@ def make_package(name=None):
         'notes': 'some test notes',
         'url': 'www.example.com',
         }
+
     # Add some resources to the package.
     res1 = {
             'url': 'http://www.example-resource.info',
@@ -127,10 +128,15 @@ def make_package(name=None):
             'name': 'another example resource',
         }
     pkg['resources'] = [res1, res2]
+
     # Add some tags to the package.
     tag1 = {'name': 'a_test_tag'}
     tag2 = {'name': 'another_test_tag'}
     pkg['tags'] = [tag1, tag2]
+
+    # Add the package to a group.
+    pkg['groups'] = [{'name': 'roger'}]
+
     return pkg
 
 
@@ -246,7 +252,7 @@ def activity_details(self, activity):
                 "/api/2/rest/activity/%s/details" % activity['id'])
         return json.loads(response.body)
 
-    def record_details(self, user_id, package_id=None, group_id=None,
+    def record_details(self, user_id, package_id=None, group_ids=None,
             apikey=None):
         details = {}
         details['user activity stream'] = self.user_activity_stream(user_id)
@@ -255,9 +261,11 @@ def record_details(self, user_id, package_id=None, group_id=None,
             details['package activity stream'] = (
                     self.package_activity_stream(package_id))
 
-        if group_id is not None:
-            details['group activity stream'] = (
-                self.group_activity_stream(group_id, apikey))
+        if group_ids is not None:
+            details['group activity streams'] = {}
+            for group_id in group_ids:
+                details['group activity streams'][group_id] = (
+                    self.group_activity_stream(group_id, apikey))
 
         details['recently changed datasets stream'] = \
                 self.recently_changed_datasets_stream()
@@ -287,10 +295,10 @@ def _create_package(self, user, name=None):
         else:
             user_id = 'not logged in'
 
-        before = self.record_details(user_id)
-
         # Create a new package.
         request_data = make_package(name)
+        before = self.record_details(user_id=user_id,
+                group_ids=[group['name'] for group in request_data['groups']])
         extra_environ = {'Authorization': str(user['apikey'])}
         response = self.app.post('/api/action/package_create',
                 json.dumps(request_data), extra_environ=extra_environ)
@@ -298,7 +306,9 @@ def _create_package(self, user, name=None):
         assert response_dict['success'] is True
         package_created = response_dict['result']
 
-        after = self.record_details(user_id, package_created['id'])
+        after = self.record_details(user_id=user_id,
+            package_id=package_created['id'],
+            group_ids=[group['name'] for group in package_created['groups']])
 
         # Find the new activity in the user's activity stream.
         user_new_activities = (find_new_activities(
@@ -310,17 +320,25 @@ def _create_package(self, user, name=None):
 
         # The same new activity should appear in the package's activity stream.
         pkg_new_activities = after['package activity stream']
-        assert pkg_new_activities == user_new_activities
+        assert pkg_new_activities == [activity]
 
         # The same new activity should appear in the recently changed datasets
         # stream.
-        assert find_new_activities(
+        new_rcd_activities = find_new_activities(
                 before['recently changed datasets stream'],
-                after['recently changed datasets stream']) \
-                        == user_new_activities
+                after['recently changed datasets stream'])
+        assert new_rcd_activities == [activity]
 
         self.check_dashboard(before, after, user_new_activities, [user_id])
 
+        # The same new activity should appear in the activity streams of the
+        # package's groups.
+        for group_dict in package_created['groups']:
+            grp_new_activities = find_new_activities(
+                before['group activity streams'][group_dict['name']],
+                after['group activity streams'][group_dict['name']])
+            assert grp_new_activities == [activity]
+
         # Check that the new activity has the right attributes.
         assert activity['object_id'] == package_created['id'], \
             str(activity['object_id'])
@@ -378,7 +396,8 @@ def _add_resource(self, package, user):
         else:
             user_id = 'not logged in'
 
-        before = self.record_details(user_id, package['id'])
+        before = self.record_details(user_id, package['id'],
+                [group['id'] for group in package['groups']])
 
         resource_ids_before = [resource['id'] for resource in
                 package['resources']]
@@ -388,7 +407,8 @@ def _add_resource(self, package, user):
         resources.append(make_resource())
         updated_package = package_update(self.app, package, user['apikey'])
 
-        after = self.record_details(user_id, package['id'])
+        after = self.record_details(user_id, package['id'],
+                [group['id'] for group in package['groups']])
         resource_ids_after = [resource['id'] for resource in
                 updated_package['resources']]
         assert len(resource_ids_after) == len(resource_ids_before) + 1
@@ -414,6 +434,14 @@ def _add_resource(self, package, user):
                 after['recently changed datasets stream']) \
                         == user_new_activities
 
+        # 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']:
+            grp_new_activities = find_new_activities(
+                before['group activity streams'][group_dict['name']],
+                after['group activity streams'][group_dict['name']])
+            assert grp_new_activities == [activity]
+
         self.check_dashboard(before, after, user_new_activities,
                 [user_id, package['id']])
 
@@ -493,6 +521,14 @@ def _delete_extra(self, package_dict, user):
                 after['recently changed datasets stream']) \
                         == user_new_activities
 
+        # 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']:
+            grp_new_activities = find_new_activities(
+                before['group activity streams'][group_dict['name']],
+                after['group activity streams'][group_dict['name']])
+            assert grp_new_activities == [activity]
+
         self.check_dashboard(before, after, user_new_activities,
                 [user_id, package_dict['id']])
 
@@ -536,7 +572,8 @@ def _update_extra(self, package_dict, user):
         else:
             user_id = 'not logged in'
 
-        before = self.record_details(user_id, package_dict['id'])
+        before = self.record_details(user_id, package_dict['id'],
+                [group['name'] for group in package_dict['groups']])
 
         extras_before = package_dict['extras']
         assert len(extras_before) > 0, (
@@ -552,7 +589,8 @@ def _update_extra(self, package_dict, user):
         updated_package = package_update(self.app, package_dict,
                 user['apikey'])
 
-        after = self.record_details(user_id, package_dict['id'])
+        after = self.record_details(user_id, package_dict['id'],
+                [group['name'] for group in package_dict['groups']])
         extras_after = updated_package['extras']
         assert len(extras_after) == len(extras_before), (
                 "%s != %s" % (len(extras_after), len(extras_before)))
@@ -581,6 +619,14 @@ def _update_extra(self, package_dict, user):
         self.check_dashboard(before, after, user_new_activities,
                 [user_id, package_dict['id']])
 
+        # 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']:
+            grp_new_activities = find_new_activities(
+                before['group activity streams'][group_dict['name']],
+                after['group activity streams'][group_dict['name']])
+            assert grp_new_activities == [activity]
+
         # Check that the new activity has the right attributes.
         assert activity['object_id'] == updated_package['id'], \
             str(activity['object_id'])
@@ -661,6 +707,14 @@ def _add_extra(self, package_dict, user, key=None):
                 after['recently changed datasets stream']) \
                         == user_new_activities
 
+        # 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']:
+            grp_new_activities = find_new_activities(
+                before['group activity streams'][group_dict['name']],
+                after['group activity streams'][group_dict['name']])
+            assert grp_new_activities == [activity]
+
         self.check_dashboard(before, after, user_new_activities,
                 [user_id, package_dict['id']])
 
@@ -747,14 +801,14 @@ def _delete_group(self, group, user):
         item and detail are emitted.
 
         """
-        before = self.record_details(user['id'], group_id=group['id'],
+        before = self.record_details(user['id'], group_ids=[group['id']],
                 apikey=user['apikey'])
 
         # Deleted the group.
         group_dict = {'id': group['id'], 'state': 'deleted'}
         group_update(self.app, group_dict, user['apikey'])
 
-        after = self.record_details(user['id'], group_id=group['id'],
+        after = self.record_details(user['id'], group_ids=[group['id']],
                 apikey=user['apikey'])
 
         # Find the new activity.
@@ -764,9 +818,11 @@ def _delete_group(self, group, user):
             "the user's activity stream, but found %i" % len(new_activities))
         activity = new_activities[0]
 
-        assert find_new_activities(before["group activity stream"],
-            after['group activity stream']) == new_activities, ("The same "
-            "activity should also appear in the group's activity stream.")
+        assert find_new_activities(
+                before["group activity streams"][group['id']],
+                after['group activity streams'][group['id']]) == \
+                        new_activities, ("The same activity should also "
+                        "appear in the group's activity stream.")
 
         self.check_dashboard(before, after, new_activities, [user['id']])
 
@@ -790,13 +846,13 @@ def _update_group(self, group, user):
         item and detail are emitted.
 
         """
-        before = self.record_details(user['id'], group_id=group['id'])
+        before = self.record_details(user['id'], group_ids=[group['id']])
 
         # Update the group.
         group_dict = {'id': group['id'], 'title': 'edited'}
         group_update(self.app, group_dict, user['apikey'])
 
-        after = self.record_details(user['id'], group_id=group['id'])
+        after = self.record_details(user['id'], group_ids=[group['id']])
 
         # Find the new activity.
         new_activities = find_new_activities(before['user activity stream'],
@@ -805,9 +861,11 @@ def _update_group(self, group, user):
             "the user's activity stream, but found %i" % len(new_activities))
         activity = new_activities[0]
 
-        assert find_new_activities(before["group activity stream"],
-            after['group activity stream']) == new_activities, ("The same "
-            "activity should also appear in the group's activity stream.")
+        assert find_new_activities(
+                before["group activity streams"][group['id']],
+                after['group activity streams'][group['id']]) == \
+                        new_activities, ("The same activity should also "
+                        "appear in the group's activity stream.")
 
         self.check_dashboard(before, after, new_activities, [user['id']])
 
@@ -879,7 +937,8 @@ def _delete_resources(self, package):
         correct activity item and detail items are emitted.
 
         """
-        before = self.record_details(self.normal_user['id'], package['id'])
+        before = self.record_details(self.normal_user['id'], package['id'],
+                [group['name'] for group in package['groups']])
 
         num_resources = len(package['resources'])
         assert num_resources > 0, \
@@ -889,7 +948,8 @@ def _delete_resources(self, package):
         package['resources'] = []
         package_update(self.app, package, self.normal_user['apikey'])
 
-        after = self.record_details(self.normal_user['id'], package['id'])
+        after = self.record_details(self.normal_user['id'], package['id'],
+                [group['name'] for group in package['groups']])
 
         # Find the new activity in the user's activity stream.
         user_new_activities = (find_new_activities(
@@ -912,6 +972,14 @@ def _delete_resources(self, package):
                 after['recently changed datasets stream']) \
                         == user_new_activities
 
+        # 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']:
+            grp_new_activities = find_new_activities(
+                before['group activity streams'][group_dict['name']],
+                after['group activity streams'][group_dict['name']])
+            assert grp_new_activities == [activity]
+
         self.check_dashboard(before, after, user_new_activities,
                 [package['id']])
 
@@ -992,6 +1060,14 @@ def _update_package(self, package, user):
         self.check_dashboard(before, after, user_new_activities,
                 [user_id, package['id']])
 
+        # 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']:
+            grp_new_activities = find_new_activities(
+                before['group activity streams'][group_dict['name']],
+                after['group activity streams'][group_dict['name']])
+            assert grp_new_activities == [activity]
+
         # Check that the new activity has the right attributes.
         assert activity['object_id'] == package['id'], (
             str(activity['object_id']))
@@ -1062,6 +1138,14 @@ def _update_resource(self, package, resource, user):
         self.check_dashboard(before, after, user_new_activities,
                 [user_id, package['id']])
 
+        # 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']:
+            grp_new_activities = find_new_activities(
+                before['group activity streams'][group_dict['name']],
+                after['group activity streams'][group_dict['name']])
+            assert grp_new_activities == [activity]
+
         # Check that the new activity has the right attributes.
         assert activity['object_id'] == package['id'], (
             str(activity['object_id']))
@@ -1131,6 +1215,14 @@ def _delete_package(self, package):
         self.check_dashboard(before, after, user_new_activities,
                 [self.sysadmin_user['id'], package['id']])
 
+        # 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']:
+            grp_new_activities = find_new_activities(
+                before['group activity streams'][group_dict['name']],
+                after['group activity streams'][group_dict['name']])
+            assert grp_new_activities == [activity]
+
         # Check that the new activity has the right attributes.
         assert activity['object_id'] == package['id'], (
                 str(activity['object_id']))
@@ -1205,13 +1297,15 @@ def test_01_remove_tag(self):
         # Remove one tag from the package.
         assert len(pkg_dict['tags']) >= 1, ("The package has to have at least"
                 " one tag to test removing a tag.")
-        before = self.record_details(user['id'], pkg_dict['id'])
+        before = self.record_details(user['id'], pkg_dict['id'],
+                [group['name'] for group in pkg_dict['groups']])
         data_dict = {
             'id': pkg_dict['id'],
             'tags': pkg_dict['tags'][0:-1],
             }
         package_update(self.app, data_dict, user['apikey'])
-        after = self.record_details(user['id'], pkg_dict['id'])
+        after = self.record_details(user['id'], pkg_dict['id'],
+                [group['name'] for group in pkg_dict['groups']])
 
         # Find the new activity in the user's activity stream.
         user_new_activities = (find_new_activities(
@@ -1237,6 +1331,14 @@ def test_01_remove_tag(self):
         self.check_dashboard(before, after, user_new_activities,
                 [user['id'], pkg_dict['id']])
 
+        # 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']:
+            grp_new_activities = find_new_activities(
+                before['group activity streams'][group_dict['name']],
+                after['group activity streams'][group_dict['name']])
+            assert grp_new_activities == [activity]
+
         # Check that the new activity has the right attributes.
         assert activity['object_id'] == pkg_dict['id'], (
             str(activity['object_id']))
@@ -1424,7 +1526,8 @@ def test_create_group(self):
         assert response_dict['success'] is True
         group_created = response_dict['result']
 
-        after = self.record_details(user['id'], group_id=group_created['id'])
+        after = self.record_details(user['id'],
+                group_ids=[group_created['id']])
 
         # Find the new activity.
         new_activities = find_new_activities(before['user activity stream'],
@@ -1433,8 +1536,9 @@ def test_create_group(self):
             "the user's activity stream, but found %i" % len(new_activities))
         activity = new_activities[0]
 
-        assert after['group activity stream'] == new_activities, ("The same "
-            "activity should also appear in the group's activity stream.")
+        assert after['group activity streams'][group_created['id']] == \
+                new_activities, ("The same activity should also appear in "
+                "the group's activity stream.")
 
         self.check_dashboard(before, after, new_activities, [user['id']])
 
@@ -1511,6 +1615,14 @@ def test_add_tag(self):
         self.check_dashboard(before, after, user_new_activities,
                 [user['id'], pkg_dict['id']])
 
+        # 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']:
+            grp_new_activities = find_new_activities(
+                before['group activity streams'][group_dict['name']],
+                after['group activity streams'][group_dict['name']])
+            assert grp_new_activities == [activity]
+
         # Check that the new activity has the right attributes.
         assert activity['object_id'] == pkg_dict['id'], (
             str(activity['object_id']))

From 22b5ff34dbb0a0456bba5988b6e9aacba627d5e2 Mon Sep 17 00:00:00 2001
From: Sean Hammond <seanhammond@lavabit.com>
Date: Mon, 29 Oct 2012 18:12:36 +0100
Subject: [PATCH 10/12] Add auth checks to activity streams API functions

---
 ckan/logic/action/get.py                   | 25 ++++++++++++--
 ckan/tests/functional/api/test_activity.py | 38 ++++++++++++++++------
 2 files changed, 50 insertions(+), 13 deletions(-)

diff --git a/ckan/logic/action/get.py b/ckan/logic/action/get.py
index 969462ac4ab..7bcca7b74a4 100644
--- a/ckan/logic/action/get.py
+++ b/ckan/logic/action/get.py
@@ -1717,12 +1717,17 @@ def vocabulary_show(context, data_dict):
 def user_activity_list(context, data_dict):
     '''Return a user's public activity stream.
 
+    You must be authorized to view the user's profile.
+
     :param id: the id or name of the user
     :type id: string
 
     :rtype: list of dictionaries
 
     '''
+    # FIXME: Filter out activities whose subject or object the user is not
+    # authorized to read.
+    _check_access('user_show', context, data_dict)
     model = context['model']
     user_id = _get_or_bust(data_dict, 'id')
     query = model.Session.query(model.Activity)
@@ -1735,12 +1740,17 @@ def user_activity_list(context, data_dict):
 def package_activity_list(context, data_dict):
     '''Return a package's activity stream.
 
+    You must be authorized to view the package.
+
     :param id: the id or name of the package
     :type id: string
 
     :rtype: list of dictionaries
 
     '''
+    # FIXME: Filter out activities whose subject or object the user is not
+    # authorized to read.
+    _check_access('package_show', context, data_dict)
     model = context['model']
     package_id = _get_or_bust(data_dict, 'id')
     query = model.Session.query(model.Activity)
@@ -1753,12 +1763,18 @@ def package_activity_list(context, data_dict):
 def group_activity_list(context, data_dict):
     '''Return a group's activity stream.
 
+    You must be authorized to view the group.
+
     :param id: the id or name of the group
     :type id: string
 
     :rtype: list of dictionaries
 
     '''
+    # FIXME: Filter out activities whose subject or object the user is not
+    # authorized to read.
+    _check_access('group_show', context, data_dict)
+
     model = context['model']
     group_id = _get_or_bust(data_dict, 'id')
 
@@ -1766,8 +1782,6 @@ def group_activity_list(context, data_dict):
     group_show = logic.get_action('group_show')
     group_id = group_show(context, {'id': group_id})['id']
 
-    _check_access('group_show', context, data_dict)
-
     # 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})
@@ -1789,6 +1803,8 @@ def recently_changed_packages_activity_list(context, data_dict):
     :rtype: list of dictionaries
 
     '''
+    # FIXME: Filter out activities whose subject or object the user is not
+    # authorized to read.
     model = context['model']
     query = model.Session.query(model.Activity)
     query = query.filter(model.Activity.activity_type.endswith('package'))
@@ -1805,6 +1821,8 @@ def activity_detail_list(context, data_dict):
     :rtype: list of dictionaries.
 
     '''
+    # FIXME: Filter out activities whose subject or object the user is not
+    # authorized to read.
     model = context['model']
     activity_id = _get_or_bust(data_dict, 'id')
     activity_detail_objects = model.Session.query(
@@ -1812,7 +1830,6 @@ def activity_detail_list(context, data_dict):
     return model_dictize.activity_detail_list_dictize(activity_detail_objects, context)
 
 
-
 def user_activity_list_html(context, data_dict):
     '''Return a user's public activity stream as HTML.
 
@@ -2096,6 +2113,8 @@ def dashboard_activity_list(context, data_dict):
     :rtype: list of dictionaries
 
     '''
+    # FIXME: Filter out activities whose subject or object the user is not
+    # authorized to read.
     model = context['model']
     user_id = _get_or_bust(data_dict, 'id')
 
diff --git a/ckan/tests/functional/api/test_activity.py b/ckan/tests/functional/api/test_activity.py
index 07829971b76..1126efd0484 100644
--- a/ckan/tests/functional/api/test_activity.py
+++ b/ckan/tests/functional/api/test_activity.py
@@ -221,12 +221,22 @@ def dashboard_activity_stream(self, user_id):
                 "/api/2/rest/user/{0}/dashboard_activity".format(user_id))
         return json.loads(response.body)
 
-    def user_activity_stream(self, user_id):
-        response = self.app.get("/api/2/rest/user/%s/activity" % user_id)
+    def user_activity_stream(self, user_id, apikey=None):
+        if apikey:
+            extra_environ = {'Authorization': str(apikey)}
+        else:
+            extra_environ = None
+        response = self.app.get("/api/2/rest/user/%s/activity" % user_id,
+                extra_environ=extra_environ)
         return json.loads(response.body)
 
-    def package_activity_stream(self, package_id):
-        response = self.app.get("/api/2/rest/dataset/%s/activity" % package_id)
+    def package_activity_stream(self, package_id, apikey=None):
+        if apikey:
+            extra_environ = {'Authorization': str(apikey)}
+        else:
+            extra_environ = None
+        response = self.app.get("/api/2/rest/dataset/%s/activity" % package_id,
+                extra_environ=extra_environ)
         return json.loads(response.body)
 
     def group_activity_stream(self, group_id, apikey=None):
@@ -238,10 +248,15 @@ def group_activity_stream(self, group_id, apikey=None):
                 extra_environ=extra_environ)
         return json.loads(response.body)
 
-    def recently_changed_datasets_stream(self):
+    def recently_changed_datasets_stream(self, apikey=None):
+        if apikey:
+            extra_environ = {'Authorization': str(apikey)}
+        else:
+            extra_environ = None
         response = self.app.post(
                 '/api/action/recently_changed_packages_activity_list',
                 params=json.dumps({}),
+                extra_environ=extra_environ,
                 status=200)
         assert response.json['success'] is True
         activities = response.json['result']
@@ -255,11 +270,12 @@ def activity_details(self, activity):
     def record_details(self, user_id, package_id=None, group_ids=None,
             apikey=None):
         details = {}
-        details['user activity stream'] = self.user_activity_stream(user_id)
+        details['user activity stream'] = self.user_activity_stream(user_id,
+                apikey)
 
         if package_id is not None:
             details['package activity stream'] = (
-                    self.package_activity_stream(package_id))
+                    self.package_activity_stream(package_id, apikey))
 
         if group_ids is not None:
             details['group activity streams'] = {}
@@ -268,7 +284,7 @@ def record_details(self, user_id, package_id=None, group_ids=None,
                     self.group_activity_stream(group_id, apikey))
 
         details['recently changed datasets stream'] = \
-                self.recently_changed_datasets_stream()
+                self.recently_changed_datasets_stream(apikey)
 
         details['follower dashboard activity stream'] = (
                         self.dashboard_activity_stream(self.follower['id']))
@@ -1179,7 +1195,8 @@ def _delete_package(self, package):
         item and detail are emitted.
 
         """
-        before = self.record_details(self.sysadmin_user['id'], package['id'])
+        before = self.record_details(self.sysadmin_user['id'], package['id'],
+                apikey=self.sysadmin_user['apikey'])
 
         # Delete the package.
         package_dict = {'id': package['id']}
@@ -1189,7 +1206,8 @@ def _delete_package(self, package):
         response_dict = json.loads(response.body)
         assert response_dict['success'] is True
 
-        after = self.record_details(self.sysadmin_user['id'], package['id'])
+        after = self.record_details(self.sysadmin_user['id'], package['id'],
+                apikey=self.sysadmin_user['apikey'])
 
         # Find the new activity in the user's activity stream.
         user_new_activities = (find_new_activities(

From 9ebc00217997acb6505a4e18e62bd45a3a595407 Mon Sep 17 00:00:00 2001
From: Ian Murray <ian.c.murray@gmail.com>
Date: Fri, 2 Nov 2012 12:55:26 +0000
Subject: [PATCH 11/12] [#1664] Use field name in internationalized error.

---
 ckan/controllers/group.py | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/ckan/controllers/group.py b/ckan/controllers/group.py
index 189381d7ec4..ef54064749e 100644
--- a/ckan/controllers/group.py
+++ b/ckan/controllers/group.py
@@ -505,7 +505,9 @@ def activity(self, id):
         except NotFound:
             abort(404, _('Group not found'))
         except NotAuthorized:
-            abort(401, _('Unauthorized to read group %s') % id)
+            abort(401,
+                  _('Unauthorized to read group {group_id}').format(
+                      group_id=id))
 
         # Add the group's activity stream (already rendered to HTML) to the
         # template context for the group/read.html template to retrieve later.

From c59b304cb6b8a6dce09b8e9f708f9cb4d684cab4 Mon Sep 17 00:00:00 2001
From: Rufus Pollock <rufus.pollock@okfn.org>
Date: Sat, 3 Nov 2012 16:32:46 +0000
Subject: [PATCH 12/12] [doc/theme][xs]: update to latest version of the doc
 theme.

---
 doc/_themes/sphinx-theme-okfn | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/doc/_themes/sphinx-theme-okfn b/doc/_themes/sphinx-theme-okfn
index 3c912d26ad8..8d677b5d96f 160000
--- a/doc/_themes/sphinx-theme-okfn
+++ b/doc/_themes/sphinx-theme-okfn
@@ -1 +1 @@
-Subproject commit 3c912d26ad863a9cdadf89a47f63aef41d6c095b
+Subproject commit 8d677b5d96fb61e45b4e3b6fd32695d248c4449a