diff --git a/ckan/config/routing.py b/ckan/config/routing.py index b63de8463d3..b4a48179155 100644 --- a/ckan/config/routing.py +++ b/ckan/config/routing.py @@ -188,6 +188,7 @@ def make_map(): 'history', 'read_ajax', 'history_ajax', + 'activity', 'followers', 'follow', 'unfollow', @@ -279,7 +280,7 @@ def make_map(): # Note: openid users have slashes in their ids, so need the wildcard # in the route. m.connect('/user/activity/{id}', action='activity') - m.connect('/user/dashboard', action='dashboard') + m.connect('/dashboard', action='dashboard') m.connect('/user/follow/{id}', action='follow') m.connect('/user/unfollow/{id}', action='unfollow') m.connect('/user/followers/{id:.*}', action='followers') diff --git a/ckan/controllers/group.py b/ckan/controllers/group.py index e29f2f9b6af..76cea2ab25a 100644 --- a/ckan/controllers/group.py +++ b/ckan/controllers/group.py @@ -387,7 +387,7 @@ def _force_reindex(self, grp): appearing on the read page for the group (as they're connected via the group name)''' group = model.Group.get(grp['name']) - for dataset in group.active_packages().all(): + for dataset in group.packages(): search.rebuild(dataset.name) def _save_edit(self, id, context): diff --git a/ckan/controllers/package.py b/ckan/controllers/package.py index 7549424395f..8304fad2ce9 100644 --- a/ckan/controllers/package.py +++ b/ckan/controllers/package.py @@ -317,13 +317,6 @@ def read(self, id, format='html'): c.current_package_id = c.pkg.id c.related_count = c.pkg.related_count - # Add the package's activity stream (already rendered to HTML) to the - # template context for the package/read.html template to retrieve - # later. - c.package_activity_stream = \ - get_action('package_activity_list_html')( - context, {'id': c.current_package_id}) - PackageSaver().render_package(c.pkg_dict, context) template = self._read_template(package_type) @@ -1239,6 +1232,26 @@ def followers(self, id=None): return render('package/followers.html') + def activity(self, id): + '''Render this package's public activity stream page.''' + + context = {'model': model, 'session': model.Session, + 'user': c.user or c.author, 'for_view': True} + data_dict = {'id': id} + try: + c.pkg_dict = get_action('package_show')(context, data_dict) + c.pkg = context['package'] + c.package_activity_stream = get_action( + 'package_activity_list_html')(context, + {'id': c.pkg_dict['id']}) + c.related_count = c.pkg.related_count + except NotFound: + abort(404, _('Dataset not found')) + except NotAuthorized: + abort(401, _('Unauthorized to read dataset %s') % id) + + return render('package/activity.html') + def resource_embedded_dataviewer(self, id, resource_id, width=500, height=500): """ diff --git a/ckan/lib/dictization/model_dictize.py b/ckan/lib/dictization/model_dictize.py index d2384d56da6..57eac178d7e 100644 --- a/ckan/lib/dictization/model_dictize.py +++ b/ckan/lib/dictization/model_dictize.py @@ -32,8 +32,7 @@ def group_list_dictize(obj_list, context, group_dict['display_name'] = obj.display_name group_dict['packages'] = \ - len(obj.active_packages(with_private=with_private, - context=context).all()) + len(obj.packages(with_private=with_private, context=context)) if context.get('for_view'): if group_dict['is_organization']: diff --git a/ckan/logic/action/get.py b/ckan/logic/action/get.py index 0acfab1664e..74011c6c2dd 100644 --- a/ckan/logic/action/get.py +++ b/ckan/logic/action/get.py @@ -872,34 +872,22 @@ def group_package_show(context, data_dict): :rtype: list of dictionaries ''' - model = context["model"] - user = context["user"] - id = _get_or_bust(data_dict, 'id') - limit = data_dict.get("limit") + model = context['model'] + group_id = _get_or_bust(data_dict, 'id') - group = model.Group.get(id) + # FIXME: What if limit is not an int? Schema and validation needed. + limit = data_dict.get('limit') + + group = model.Group.get(group_id) context['group'] = group if group is None: raise NotFound _check_access('group_show', context, data_dict) - query = model.Session.query(model.PackageRevision)\ - .filter(model.PackageRevision.state=='active')\ - .filter(model.PackageRevision.current==True)\ - .join(model.Member, model.Member.table_id==model.PackageRevision.id)\ - .join(model.Group, model.Group.id==model.Member.group_id)\ - .filter_by(id=group.id)\ - .order_by(model.PackageRevision.name) - - if limit: - query = query.limit(limit) - - if context.get('return_query'): - return query - result = [] - for pkg_rev in query.all(): + for pkg_rev in group.packages(limit=limit, + return_query=context.get('return_query')): result.append(model_dictize.package_dictize(pkg_rev, context)) return result @@ -1902,30 +1890,7 @@ def group_activity_list(context, data_dict): group_show = logic.get_action('group_show') group_id = group_show(context, {'id': group_id})['id'] - # FIXME: The SQLAlchemy below should be moved into ckan/model/activity.py - # (to encapsulate SQLALchemy in the model and avoid using it from the - # logic) but it can't be because it requires the list of dataset_ids which - # comes from logic.group_package_show() (and I don't want to access the - # logic from the model). Need to change it to get the dataset_ids from the - # model instead. There seems to be multiple methods for getting a group's - # datasets, some in the logic and some in the model. - - # Get a list of the IDs of the group's datasets. - group_package_show = logic.get_action('group_package_show') - datasets = group_package_show(context, {'id': group_id}) - dataset_ids = [dataset['id'] for dataset in datasets] - - # Get the group's activities. - query = model.Session.query(model.Activity) - if dataset_ids: - query = query.filter(_or_(model.Activity.object_id == group_id, - model.Activity.object_id.in_(dataset_ids))) - else: - query = query.filter(model.Activity.object_id == group_id) - query = query.order_by(_desc(model.Activity.timestamp)) - query = query.limit(15) - activity_objects = query.all() - + activity_objects = model.activity.group_activity_list(group_id) return model_dictize.activity_list_dictize(activity_objects, context) def organization_activity_list(context, data_dict): diff --git a/ckan/model/activity.py b/ckan/model/activity.py index 9181acaa20d..cddf9e04e01 100644 --- a/ckan/model/activity.py +++ b/ckan/model/activity.py @@ -1,6 +1,6 @@ import datetime -from sqlalchemy import orm, types, Column, Table, ForeignKey, desc +from sqlalchemy import orm, types, Column, Table, ForeignKey, desc, or_ import meta import types as _types @@ -69,6 +69,7 @@ def __init__(self, activity_id, object_id, object_type, activity_type, def _most_recent_activities(q, limit): + '''Return the 'limit' most recent activites from activity query 'q'.''' import ckan.model as model q = q.order_by(desc(model.Activity.timestamp)) if limit: @@ -77,6 +78,7 @@ def _most_recent_activities(q, limit): def _activities_from_user_query(user_id): + '''Return an SQLAlchemy query for all activities from user_id.''' import ckan.model as model q = model.Session.query(model.Activity) q = q.filter(model.Activity.user_id == user_id) @@ -84,6 +86,7 @@ def _activities_from_user_query(user_id): def _activities_about_user_query(user_id): + '''Return an SQLAlchemy query for all activities about user_id.''' import ckan.model as model q = model.Session.query(model.Activity) q = q.filter(model.Activity.object_id == user_id) @@ -91,16 +94,17 @@ def _activities_about_user_query(user_id): def _user_activity_query(user_id): + '''Return an SQLAlchemy query for all activities from or about user_id.''' q = _activities_from_user_query(user_id) q = q.union(_activities_about_user_query(user_id)) return q def user_activity_list(user_id, limit=15): - '''Return the given user's public activity stream. + '''Return user_id's public activity stream. - Returns all activities from or about the given user, i.e. where the given - user is the subject or object of the activity, e.g.: + Return a list of all activities from or about the given user, i.e. where + the given user is the subject or object of the activity, e.g.: "{USER} created the dataset {DATASET}" "{OTHER_USER} started following {USER}" @@ -112,6 +116,9 @@ def user_activity_list(user_id, limit=15): def _package_activity_query(package_id): + '''Return an SQLAlchemy query for all activities about package_id. + + ''' import ckan.model as model q = model.Session.query(model.Activity) q = q.filter_by(object_id=package_id) @@ -133,7 +140,48 @@ def package_activity_list(package_id, limit=15): return _most_recent_activities(q, limit) +def _group_activity_query(group_id, limit=15): + '''Return an SQLAlchemy query for all activities about group_id. + + Returns a query for all activities whose object is either the group itself + or one of the group's datasets. + + ''' + import ckan.model as model + + group = model.Group.get(group_id) + if not group: + # Return a query with no results. + return model.Session.query(model.Activity).filter("0=1") + + dataset_ids = [dataset.id for dataset in group.packages()] + + q = model.Session.query(model.Activity) + if dataset_ids: + q = q.filter(or_(model.Activity.object_id == group_id, + model.Activity.object_id.in_(dataset_ids))) + else: + q = q.filter(model.Activity.object_id == group_id) + return q + + +def group_activity_list(group_id, limit=15): + '''Return the given group's public activity stream. + + Returns all activities where the given group or one of its datasets is the + object of the activity, e.g.: + + "{USER} updated the group {GROUP}" + "{USER} updated the dataset {DATASET}" + etc. + + ''' + q = _group_activity_query(group_id) + return _most_recent_activities(q, limit) + + def _activites_from_users_followed_by_user_query(user_id): + '''Return a query for all activities from users that user_id follows.''' import ckan.model as model q = model.Session.query(model.Activity) q = q.join(model.UserFollowingUser, @@ -143,6 +191,7 @@ def _activites_from_users_followed_by_user_query(user_id): def _activities_from_datasets_followed_by_user_query(user_id): + '''Return a query for all activities from datasets that user_id follows.''' import ckan.model as model q = model.Session.query(model.Activity) q = q.join(model.UserFollowingDataset, @@ -151,9 +200,33 @@ def _activities_from_datasets_followed_by_user_query(user_id): return q +def _activities_from_groups_followed_by_user_query(user_id): + '''Return a query for all activities about groups the given user follows. + + Return a query for all activities about the groups the given user follows, + or about any of the group's datasets. This is the union of + _group_activity_query(group_id) for each of the groups the user follows. + + ''' + import ckan.model as model + + # Get a list of the group's that the user is following. + follower_objects = model.UserFollowingGroup.followee_list(user_id) + if not follower_objects: + # Return a query with no results. + return model.Session.query(model.Activity).filter("0=1") + + q = _group_activity_query(follower_objects[0].object_id) + q = q.union_all(*[_group_activity_query(follower.object_id) + for follower in follower_objects[1:]]) + return q + + def _activities_from_everything_followed_by_user_query(user_id): + '''Return a query for all activities from everything user_id follows.''' q = _activites_from_users_followed_by_user_query(user_id) q = q.union(_activities_from_datasets_followed_by_user_query(user_id)) + q = q.union(_activities_from_groups_followed_by_user_query(user_id)) return q @@ -169,6 +242,7 @@ def activities_from_everything_followed_by_user(user_id, limit=15): def _dashboard_activity_query(user_id): + '''Return an SQLAlchemy query for user_id's dashboard activity stream.''' q = _user_activity_query(user_id) q = q.union(_activities_from_everything_followed_by_user_query(user_id)) return q @@ -188,7 +262,13 @@ def dashboard_activity_list(user_id, limit=15): return _most_recent_activities(q, limit) -def _recently_changed_packages_activity_query(): +def _changed_packages_activity_query(): + '''Return an SQLAlchemyu query for all changed package activities. + + Return a query for all activities with activity_type '*package', e.g. + 'new_package', 'changed_package', 'deleted_package'. + + ''' import ckan.model as model q = model.Session.query(model.Activity) q = q.filter(model.Activity.activity_type.endswith('package')) @@ -196,5 +276,11 @@ def _recently_changed_packages_activity_query(): def recently_changed_packages_activity_list(limit=15): - q = _recently_changed_packages_activity_query() + '''Return the site-wide stream of recently changed package activities. + + This activity stream includes recent 'new package', 'changed package' and + 'deleted package' activities for the whole site. + + ''' + q = _changed_packages_activity_query() return _most_recent_activities(q, limit) diff --git a/ckan/model/group.py b/ckan/model/group.py index 42dea99b7c2..713156ff361 100644 --- a/ckan/model/group.py +++ b/ckan/model/group.py @@ -144,35 +144,6 @@ def set_approval_status(self, status): if status == "denied": pass - def members_of_type(self, object_type, capacity=None): - from ckan import model - object_type_string = object_type.__name__.lower() - query = meta.Session.query(object_type).\ - filter(model.Group.id == self.id).\ - filter(model.Member.state == 'active').\ - filter(model.Member.table_name == object_type_string) - - if hasattr(object_type, 'state'): - query = query.filter(object_type.state == 'active') - - if capacity: - query = query.filter(model.Member.capacity == capacity) - - query = query.join(model.Member, member_table.c.table_id == - getattr(object_type, 'id')).\ - join(model.Group, group_table.c.id == member_table.c.group_id) - - return query - - def add_child(self, object_instance): - object_type_string = object_instance.__class__.__name__.lower() - if not object_instance in self.members_of_type( - object_instance.__class__).all(): - member = Member(group=self, - table_id=getattr(object_instance, 'id'), - table_name=object_type_string) - meta.Session.add(member) - def get_children_groups(self, type='group'): # Returns a list of dicts where each dict contains "id", "name", # and "title" When querying with a CTE specifying a model in the @@ -185,15 +156,35 @@ def get_children_groups(self, type='group'): return [{"id":idf, "name": name, "title": title} for idf, name, title in results] - def active_packages(self, load_eager=True, with_private=False, context=None): - # see if an this is an organization and we have membership or are - # sysadmin + + def packages(self, with_private=False, limit=None, + return_query=False, context=None): + '''Return this group's active and pending packages. + + Returns all packages in this group with VDM revision state ACTIVE or + PENDING. + + :param with_private: if True, include the group's private packages + :type with_private: boolean + + :param limit: the maximum number of packages to return + :type limit: int + + :param return_query: if True, return the SQLAlchemy query object + instead of the list of Packages resulting from the query + :type return_query: boolean + + :returns: a list of this group's packages + :rtype: list of ckan.model.package.Package objects + + ''' user_is_org_member = False context = context or {} user_is_admin = context.get('user_is_admin', False) user_id = context.get('user_id') if user_is_admin: user_is_org_member = True + elif self.is_organization and user_id: query = meta.Session.query(Member) \ .filter(Member.state == 'active') \ @@ -203,8 +194,11 @@ def active_packages(self, load_eager=True, with_private=False, context=None): user_is_org_member = len(query.all()) != 0 + #filter_by(state=vdm.sqlalchemy.State.ACTIVE).\ query = meta.Session.query(_package.Package).\ - filter_by(state=vdm.sqlalchemy.State.ACTIVE).\ + filter( + or_(_package.Package.state == vdm.sqlalchemy.State.ACTIVE, + _package.Package.state == vdm.sqlalchemy.State.PENDING)). \ filter(group_table.c.id == self.id).\ filter(member_table.c.state == 'active') @@ -215,11 +209,18 @@ def active_packages(self, load_eager=True, with_private=False, context=None): if not self.is_organization: query = query.filter(_package.Package.private == False) - query = query.join(member_table, member_table.c.table_id == - _package.Package.id).\ - join(group_table, group_table.c.id == member_table.c.group_id) + query = query.join(member_table, + member_table.c.table_id == _package.Package.id) + query = query.join(group_table, + group_table.c.id == member_table.c.group_id) - return query + if limit is not None: + query = query.limit(limit) + + if return_query: + return query + else: + return query.all() @classmethod def search_by_name_or_title(cls, text_query, group_type=None): @@ -231,23 +232,12 @@ def search_by_name_or_title(cls, text_query, group_type=None): q = q.filter(cls.type == group_type) return q.order_by(cls.title) - def as_dict(self, ref_package_by='name'): - _dict = domain_object.DomainObject.as_dict(self) - _dict['packages'] = [getattr(package, ref_package_by) - for package in self.packages] - _dict['extras'] = dict([(key, value) for key, value - in self.extras.items()]) - if (self.type == 'organization'): - _dict['users'] = [getattr(user, "name") - for user in self.members_of_type(_user.User)] - return _dict - def add_package_by_name(self, package_name): if not package_name: return package = _package.Package.by_name(package_name) assert package - if not package in self.members_of_type(package.__class__).all(): + if not package in self.packages(): member = Member(group=self, table_id=package.id, table_name='package') meta.Session.add(member) diff --git a/ckan/templates/package/activity.html b/ckan/templates/package/activity.html new file mode 100644 index 00000000000..ee0e2dd7fc0 --- /dev/null +++ b/ckan/templates/package/activity.html @@ -0,0 +1,10 @@ +{% extends "package/read.html" %} + +{% block subtitle %}{{ _('Activity Stream') }} - {{ c.pkg_dict.title or c.pkg_dict.name }}{% endblock %} + +{% block package_content %} +
+

{{ _('Activity Stream') }}

+ {{ c.package_activity_stream | safe }} +
+{% endblock %} diff --git a/ckan/templates/package/read.html b/ckan/templates/package/read.html index ec47e7293fc..3f4d4963a54 100644 --- a/ckan/templates/package/read.html +++ b/ckan/templates/package/read.html @@ -45,6 +45,9 @@ {% link_for _('Dataset'), controller='package', action='read', id=pkg.name, icon='sitemap' %} + + {% link_for _('Activity Stream'), controller='package', action='activity', id=pkg.name, icon='time' %} + {% link_for _('Followers'), controller='package', action='followers', id=pkg.name, icon='group' %} diff --git a/ckan/templates/user/dashboard.html b/ckan/templates/user/dashboard.html index d50c1cf1797..2beaaaddea9 100644 --- a/ckan/templates/user/dashboard.html +++ b/ckan/templates/user/dashboard.html @@ -5,7 +5,6 @@ {% block subtitle %}{{ _('Dashboard') }}{% endblock %} {% block breadcrumb_content %} -
  • {% link_for _('Users'), controller='user', action='index' %}
  • {{ _('Dashboard') }}
  • {% endblock %} diff --git a/ckan/tests/__init__.py b/ckan/tests/__init__.py index 721c55f6f49..0d4be8b4e49 100644 --- a/ckan/tests/__init__.py +++ b/ckan/tests/__init__.py @@ -400,3 +400,61 @@ class StatusCodes: STATUS_404_NOT_FOUND = 404 STATUS_409_CONFLICT = 409 + +def call_action_api(app, action, apikey=None, status=200, **kwargs): + '''POST an HTTP request to the CKAN API and return the result. + + Any additional keyword arguments that you pass to this function as **kwargs + are posted as params to the API. + + Usage: + + package_dict = post(app, 'package_create', apikey=apikey, + name='my_package') + assert package_dict['name'] == 'my_package' + + num_followers = post(app, 'user_follower_count', id='annafan') + + If you are expecting an error from the API and want to check the contents + of the error dict, you have to use the status param otherwise an exception + will be raised: + + error_dict = post(app, 'group_activity_list', status=403, + id='invalid_id') + assert error_dict['message'] == 'Access Denied' + + :param app: the test app to post to + :type app: paste.fixture.TestApp + + :param action: the action to post to, e.g. 'package_create' + :type action: string + + :param apikey: the API key to put in the Authorization header of the post + (optional, default: None) + :type apikey: string + + :param status: the HTTP status code expected in the response from the CKAN + API, e.g. 403, if a different status code is received an exception will + be raised (optional, default: 200) + :type status: int + + :param **kwargs: any other keyword arguments passed to this function will + be posted to the API as params + + :raises paste.fixture.AppError: if the HTTP status code of the response + from the CKAN API is different from the status param passed to this + function + + :returns: the 'result' or 'error' dictionary from the CKAN API response + :rtype: dictionary + + ''' + params = json.dumps(kwargs) + response = app.post('/api/action/{0}'.format(action), params=params, + extra_environ={'Authorization': str(apikey)}, status=status) + if status in (200,): + assert response.json['success'] is True + return response.json['result'] + else: + assert response.json['success'] is False + return response.json['error'] diff --git a/ckan/tests/functional/api/__init__.py b/ckan/tests/functional/api/__init__.py index 1c6e2442feb..08c9762d91d 100644 --- a/ckan/tests/functional/api/__init__.py +++ b/ckan/tests/functional/api/__init__.py @@ -1,10 +1,16 @@ from nose.tools import assert_equal import copy + def change_lists_to_sets(iterable): - '''recursive method to drill down into iterables to - convert any list or tuples into sets. Does not work - though for dictionaries in lists.''' + '''Convert any lists or tuples in `iterable` into sets. + + Recursively drill down into iterable and convert any list or tuples to + sets. + + Does not work for dictionaries in lists. + + ''' if isinstance(iterable, dict): for key in iterable: if isinstance(iterable[key], (list, tuple)): @@ -25,11 +31,14 @@ def change_lists_to_sets(iterable): else: raise NotImplementedError + def assert_dicts_equal_ignoring_ordering(dict1, dict2): - '''Asserts dicts are equal, assuming that the ordering of - any lists is unimportant.''' + '''Assert that dict1 and dict2 are equal. + + Assumes that the ordering of any lists in the dicts is unimportant. + + ''' dicts = [copy.deepcopy(dict1), copy.deepcopy(dict2)] for d in dicts: d = change_lists_to_sets(d) - #from nose.tools import set_trace; set_trace() assert_equal(dicts[0], dicts[1]) diff --git a/ckan/tests/functional/api/test_activity.py b/ckan/tests/functional/api/test_activity.py index 0016520fcd3..4fa1ed45f72 100644 --- a/ckan/tests/functional/api/test_activity.py +++ b/ckan/tests/functional/api/test_activity.py @@ -1,3 +1,13 @@ +'''Functional tests for the public activity streams API. + +This module tests the contents of the various public activity streams: +use activity streams, dataset activity streams, group activity streams, etc. + +This module _does not_ test the private user dashboard activity stream (which +is different because the contents depend on what the user is following), that +is tested in test_dashboard.py. + +''' import datetime import logging logger = logging.getLogger(__name__) @@ -180,12 +190,6 @@ def setup_class(self): 'apikey': normal_user.apikey, 'name': normal_user.name, } - follower = model.User.get('tester') - self.follower = { - 'id': follower.id, - 'apikey': follower.apikey, - 'name': follower.name, - } warandpeace = model.Package.get('warandpeace') self.warandpeace = { 'id': warandpeace.id, @@ -194,54 +198,14 @@ def setup_class(self): self.annakarenina = { 'id': annakarenina.id, } - self.users = [self.sysadmin_user, self.normal_user, self.follower] + self.users = [self.sysadmin_user, self.normal_user] self.app = paste.fixture.TestApp(pylons.test.pylonsapp) - # Make follower follow everything else. - params = {'id': 'testsysadmin'} - extra_environ = {'Authorization': str(self.follower['apikey'])} - response = self.app.post('/api/action/follow_user', - params=json.dumps(params), extra_environ=extra_environ).json - assert response['success'] is True - params = {'id': 'annafan'} - extra_environ = {'Authorization': str(self.follower['apikey'])} - response = self.app.post('/api/action/follow_user', - params=json.dumps(params), extra_environ=extra_environ).json - assert response['success'] is True - params = {'id': 'warandpeace'} - extra_environ = {'Authorization': str(self.follower['apikey'])} - response = self.app.post('/api/action/follow_dataset', - params=json.dumps(params), extra_environ=extra_environ).json - assert response['success'] is True - params = {'id': 'annakarenina'} - extra_environ = {'Authorization': str(self.follower['apikey'])} - response = self.app.post('/api/action/follow_dataset', - params=json.dumps(params), extra_environ=extra_environ).json - assert response['success'] is True - - self.followees = \ - [ - self.sysadmin_user['id'], - self.normal_user['id'], - self.follower['id'], - self.warandpeace['id'], - self.annakarenina['id'] - ] - @classmethod def teardown_class(self): import ckan.model as model model.repo.rebuild_db() - def dashboard_activity_stream(self, apikey): - - response = self.app.get("/api/action/dashboard_activity_list", - json.dumps({}), - extra_environ={'Authorization': str(apikey)}) - response_dict = json.loads(response.body) - assert response_dict['success'] is True - return response_dict['result'] - def user_activity_stream(self, user_id, apikey=None): if apikey: extra_environ = {'Authorization': str(apikey)} @@ -308,28 +272,9 @@ def record_details(self, user_id, package_id=None, group_ids=None, details['recently changed datasets stream'] = \ self.recently_changed_datasets_stream(apikey) - details['user dashboard activity stream'] = ( - self.dashboard_activity_stream(apikey)) - - details['follower dashboard activity stream'] = ( - self.dashboard_activity_stream(self.follower['apikey'])) - details['time'] = datetime.datetime.now() return details - def check_dashboards(self, before, after, activity): - new_activities = [activity_ for activity_ in - after['user dashboard activity stream'] - if activity_ not in before['user dashboard activity stream']] - assert [activity['id'] for activity in new_activities] == [ - activity['id']] - - new_activities = [activity_ for activity_ in - after['follower dashboard activity stream'] - if activity_ not in before['follower dashboard activity stream']] - assert [activity['id'] for activity in new_activities] == [ - activity['id']] - def _create_package(self, user, name=None): if user: user_id = user['id'] @@ -376,31 +321,6 @@ def _create_package(self, user, name=None): after['recently changed datasets stream']) assert new_rcd_activities == [activity] - # The new activity should appear in the user's dashboard activity - # stream. - new_activities = [activity_ for activity_ in - after['user dashboard activity stream'] - if activity_ not in before['user dashboard activity stream']] - # There will be other new activities besides the 'follow dataset' one - # because all the dataset's old activities appear in the user's - # dashboard when she starts to follow the dataset. - assert activity['id'] in [ - activity['id'] for activity in new_activities] - - # The new activity should appear in the user "follower"'s dashboard - # activity stream because she follows all the other users and datasets. - new_activities = [activity_ for activity_ in - after['follower dashboard activity stream'] - if activity_ not in before['follower dashboard activity stream']] - # There will be other new activities besides the 'follow dataset' one - # because all the dataset's old activities appear in the user's - # dashboard when she starts to follow the dataset. - assert [activity['id'] for activity in new_activities] == [ - activity['id']] - - # The same new activity should appear on the dashboard's of the user's - # followers. - # The same new activity should appear in the activity streams of the # package's groups. for group_dict in package_created['groups']: @@ -515,8 +435,6 @@ def _add_resource(self, package, user): after['group activity streams'][group_dict['name']]) assert grp_new_activities == [activity] - self.check_dashboards(before, after, activity) - # Check that the new activity has the right attributes. assert activity['object_id'] == updated_package['id'], \ str(activity['object_id']) @@ -607,8 +525,6 @@ def _delete_extra(self, package_dict, user): after['group activity streams'][group_dict['name']]) assert grp_new_activities == [activity] - self.check_dashboards(before, after, activity) - # Check that the new activity has the right attributes. assert activity['object_id'] == updated_package['id'], \ str(activity['object_id']) @@ -697,8 +613,6 @@ def _update_extra(self, package_dict, user): after['recently changed datasets stream']) \ == user_new_activities - self.check_dashboards(before, after, activity) - # If the package has any groups, the same new activity should appear # in the activity stream of each group. for group_dict in package_dict['groups']: @@ -801,8 +715,6 @@ def _add_extra(self, package_dict, user, key=None): after['group activity streams'][group_dict['name']]) assert grp_new_activities == [activity] - self.check_dashboards(before, after, activity) - # Check that the new activity has the right attributes. assert activity['object_id'] == updated_package['id'], \ str(activity['object_id']) @@ -863,8 +775,6 @@ def _create_activity(self, user, package, params): after['package activity stream'])) assert pkg_new_activities == user_new_activities - self.check_dashboards(before, after, activity) - # Check that the new activity has the right attributes. assert activity['object_id'] == params['object_id'], ( str(activity['object_id'])) @@ -910,8 +820,6 @@ def _delete_group(self, group, user): new_activities, ("The same activity should also " "appear in the group's activity stream.") - self.check_dashboards(before, after, activity) - # Check that the new activity has the right attributes. assert activity['object_id'] == group['id'], str(activity['object_id']) assert activity['user_id'] == user['id'], str(activity['user_id']) @@ -955,8 +863,6 @@ def _update_group(self, group, user): new_activities, ("The same activity should also " "appear in the group's activity stream.") - self.check_dashboards(before, after, activity) - # Check that the new activity has the right attributes. assert activity['object_id'] == group['id'], str(activity['object_id']) assert activity['user_id'] == user['id'], str(activity['user_id']) @@ -1004,8 +910,6 @@ def _update_user(self, user): "the user's activity stream, but found %i" % len(new_activities)) activity = new_activities[0] - self.check_dashboards(before, after, activity) - # Check that the new activity has the right attributes. assert activity['object_id'] == user_dict['id'], ( str(activity['object_id'])) @@ -1072,8 +976,6 @@ def _delete_resources(self, package): after['group activity streams'][group_dict['name']]) assert grp_new_activities == [activity] - self.check_dashboards(before, after, activity) - # Check that the new activity has the right attributes. assert activity['object_id'] == package['id'], ( str(activity['object_id'])) @@ -1150,8 +1052,6 @@ def _update_package(self, package, user): after['recently changed datasets stream']) \ == user_new_activities - self.check_dashboards(before, after, activity) - # If the package has any groups, the same new activity should appear # in the activity stream of each group. for group_dict in package['groups']: @@ -1229,8 +1129,6 @@ def _update_resource(self, package, resource, user): after['recently changed datasets stream']) \ == user_new_activities - self.check_dashboards(before, after, activity) - # If the package has any groups, the same new activity should appear # in the activity stream of each group. for group_dict in package['groups']: @@ -1307,8 +1205,6 @@ def _delete_package(self, package): after['recently changed datasets stream']) \ == user_new_activities - self.check_dashboards(before, after, activity) - # If the package has any groups, the same new activity should appear # in the activity stream of each group. for group_dict in package['groups']: @@ -1424,8 +1320,6 @@ def test_01_remove_tag(self): after['recently changed datasets stream']) \ == user_new_activities - self.check_dashboards(before, after, activity) - # If the package has any groups, the same new activity should appear # in the activity stream of each group. for group_dict in pkg_dict['groups']: @@ -1636,8 +1530,6 @@ def test_create_group(self): new_activities, ("The same activity should also appear in " "the group's activity stream.") - self.check_dashboards(before, after, activity) - # Check that the new activity has the right attributes. assert activity['object_id'] == group_created['id'], \ str(activity['object_id']) @@ -1710,8 +1602,6 @@ def test_add_tag(self): after['recently changed datasets stream']) \ == user_new_activities - self.check_dashboards(before, after, activity) - # If the package has any groups, the same new activity should appear # in the activity stream of each group. for group_dict in pkg_dict['groups']: @@ -2137,28 +2027,6 @@ def test_follow_dataset(self): for activity in user_new_activities: assert activity in pkg_new_activities - # The new activity should appear in the user's dashboard activity - # stream. - new_activities = [activity_ for activity_ in - after['user dashboard activity stream'] - if activity_ not in before['user dashboard activity stream']] - # There will be other new activities besides the 'follow dataset' one - # because all the dataset's old activities appear in the user's - # dashboard when she starts to follow the dataset. - assert activity['id'] in [ - activity['id'] for activity in new_activities] - - # The new activity should appear in the user "follower"'s dashboard - # activity stream because she follows all the other users and datasets. - new_activities = [activity_ for activity_ in - after['follower dashboard activity stream'] - if activity_ not in before['follower dashboard activity stream']] - # There will be other new activities besides the 'follow dataset' one - # because all the dataset's old activities appear in the user's - # dashboard when she starts to follow the dataset. - assert [activity['id'] for activity in new_activities] == [ - activity['id']] - # Check that the new activity has the right attributes. assert activity['object_id'] == self.warandpeace['id'], \ str(activity['object_id']) @@ -2200,16 +2068,6 @@ def test_follow_user(self): len(user_new_activities)) activity = user_new_activities[0] - # Check that the new activity appears in the user's private activity - # stream. - user_new_activities = (find_new_activities( - before['follower dashboard activity stream'], - after['follower dashboard activity stream'])) - assert len(user_new_activities) == 1, ("There should be 1 new " - " activity in the user's activity stream, but found %i" % - len(user_new_activities)) - assert user_new_activities[0]['id'] == activity['id'] - # Check that the new activity has the right attributes. assert activity['object_id'] == self.sysadmin_user['id'], \ str(activity['object_id']) diff --git a/ckan/tests/functional/api/test_dashboard.py b/ckan/tests/functional/api/test_dashboard.py index 1d7c291c200..1b285a222c3 100644 --- a/ckan/tests/functional/api/test_dashboard.py +++ b/ckan/tests/functional/api/test_dashboard.py @@ -1,3 +1,10 @@ +'''Test for the dashboard API. + +This module tests the various functions of the user dashboard, such as the +contents of the dashboard activity stream and reporting the number of new +activities. + +''' import ckan from ckan.lib.helpers import json import paste @@ -92,6 +99,16 @@ def test_00_dashboard_new_activities_count_not_logged_in(self): def test_00_dashboard_mark_new_activities_not_logged_in(self): self.post('dashboard_mark_all_new_activities_as_old', status=403) + def test_01_dashboard_activity_list_for_new_user(self): + '''Test the contents of a new user's dashboard activity stream.''' + activities = self.dashboard_activity_list(self.new_user) + # We expect to find a single 'new user' activity. + assert len(activities) == 1 + activity = activities[0] + assert activity['activity_type'] == 'new user' + assert activity['user_id'] == activity['object_id'] + assert activity['user_id'] == self.new_user['id'] + def test_01_new_activities_count_for_new_user(self): '''Test that a newly registered user's new activities count is 0.''' assert self.dashboard_new_activities_count(self.new_user) == 0 @@ -151,14 +168,38 @@ def test_02_own_activities_do_not_count_as_new(self): # User's own actions should not increase her activity count. assert self.dashboard_new_activities_count(self.new_user) == 0 + def test_03_dashboard_activity_list_own_activities(self): + '''Test that a user's own activities appear in her dashboard.''' + activities = self.dashboard_activity_list(self.new_user) + + # FIXME: There should actually be 6 activities here, but when you + # follow something it's old activities (from before you followed it) + # appear in your activity stream. So here we get more activities than + # expected. + assert len(activities) == 8 + + assert activities[0]['activity_type'] == 'changed package' + assert activities[1]['activity_type'] == 'follow group' + assert activities[2]['activity_type'] == 'follow user' + assert activities[3]['activity_type'] == 'follow dataset' + assert activities[4]['activity_type'] == 'new package' + assert activities[5]['activity_type'] == 'new user' + + # FIXME: Shouldn't need the [:6] here, it's because when you follow + # something its old activities (from before you started following it) + # appear in your dashboard. + for activity in activities[:6]: + assert activity['user_id'] == self.new_user['id'] + def test_03_own_activities_not_marked_as_new(self): '''Make a user do some activities and check that her own activities aren't marked as new in her dashboard activity stream.''' assert len(self.dashboard_new_activities(self.new_user)) == 0 - def test_04_new_activities_count(self): - '''Test that new activities from objects that a user follows increase - her new activities count.''' + def test_04_activities_from_followed_datasets(self): + '''Activities from followed datasets should show in dashboard.''' + + activities_before = self.dashboard_activity_list(self.new_user) # Make someone else who new_user is not following update a dataset that # new_user is following. @@ -167,34 +208,95 @@ def test_04_new_activities_count(self): extra_environ={'Authorization': str(self.joeadmin['apikey'])}) assert response.json['success'] is True + # Check the new activity in new_user's dashboard. + activities = self.dashboard_activity_list(self.new_user) + new_activities = [activity for activity in activities + if activity not in activities_before] + assert len(new_activities) == 1 + activity = new_activities[0] + assert activity['activity_type'] == 'changed package' + assert activity['user_id'] == self.joeadmin['id'] + assert activity['data']['package']['name'] == 'warandpeace' + + def test_04_activities_from_followed_users(self): + '''Activities from followed users should show in the dashboard.''' + + activities_before = self.dashboard_activity_list(self.new_user) + # Make someone that the user is following create a new dataset. params = json.dumps({'name': 'annas_new_dataset'}) response = self.app.post('/api/action/package_create', params=params, extra_environ={'Authorization': str(self.annafan['apikey'])}) assert response.json['success'] is True - # Make someone that the user is not following update a dataset that - # the user is not following, but that belongs to a group that the user - # is following. + # Check the new activity in new_user's dashboard. + activities = self.dashboard_activity_list(self.new_user) + new_activities = [activity for activity in activities + if activity not in activities_before] + assert len(new_activities) == 1 + activity = new_activities[0] + assert activity['activity_type'] == 'new package' + assert activity['user_id'] == self.annafan['id'] + assert activity['data']['package']['name'] == 'annas_new_dataset' + + def test_04_activities_from_followed_groups(self): + '''Activities from followed groups should show in the dashboard.''' + + activities_before = self.dashboard_activity_list(self.new_user) + + # Make someone that the user is not following update a group that the + # user is following. + params = json.dumps({'id': 'roger', 'description': 'updated'}) + response = self.app.post('/api/action/group_update', params=params, + extra_environ={'Authorization': str(self.testsysadmin['apikey'])}) + assert response.json['success'] is True + + # Check the new activity in new_user's dashboard. + activities = self.dashboard_activity_list(self.new_user) + new_activities = [activity for activity in activities + if activity not in activities_before] + assert len(new_activities) == 1 + activity = new_activities[0] + assert activity['activity_type'] == 'changed group' + assert activity['user_id'] == self.testsysadmin['id'] + assert activity['data']['group']['name'] == 'roger' + + def test_04_activities_from_datasets_of_followed_groups(self): + '''Activities from datasets of followed groups should show in the + dashboard. + + ''' + activities_before = self.dashboard_activity_list(self.new_user) + + # Make someone that the user is not following update a dataset that the + # user is not following either, but that belongs to a group that the + # user is following. params = json.dumps({'name': 'annakarenina', 'notes': 'updated'}) response = self.app.post('/api/action/package_update', params=params, extra_environ={'Authorization': str(self.testsysadmin['apikey'])}) assert response.json['success'] is True - # FIXME: The number here should be 3 but activities from followed - # groups are not appearing in dashboard. When that is fixed, fix this - # number. - assert self.dashboard_new_activities_count(self.new_user) == 2 + # Check the new activity in new_user's dashboard. + activities = self.dashboard_activity_list(self.new_user) + new_activities = [activity for activity in activities + if activity not in activities_before] + assert len(new_activities) == 1 + activity = new_activities[0] + assert activity['activity_type'] == 'changed package' + assert activity['user_id'] == self.testsysadmin['id'] + assert activity['data']['package']['name'] == 'annakarenina' + + def test_05_new_activities_count(self): + '''Test that new activities from objects that a user follows increase + her new activities count.''' + assert self.dashboard_new_activities_count(self.new_user) == 4 - def test_05_activities_marked_as_new(self): + def test_06_activities_marked_as_new(self): '''Test that new activities from objects that a user follows are marked as new in her dashboard activity stream.''' - # FIXME: The number here should be 3 but activities from followed - # groups are not appearing in dashboard. When that is fixed, fix this - # number. - assert len(self.dashboard_new_activities(self.new_user)) == 2 + assert len(self.dashboard_new_activities(self.new_user)) == 4 - def test_06_mark_new_activities_as_read(self): + def test_07_mark_new_activities_as_read(self): '''Test that a user's new activities are marked as old when she views her dashboard activity stream.''' assert self.dashboard_new_activities_count(self.new_user) > 0 @@ -203,7 +305,7 @@ def test_06_mark_new_activities_as_read(self): assert self.dashboard_new_activities_count(self.new_user) == 0 assert len(self.dashboard_new_activities(self.new_user)) == 0 - def test_07_maximum_number_of_new_activities(self): + def test_08_maximum_number_of_new_activities(self): '''Test that the new activities count does not go higher than 15, even if there are more than 15 new activities from the user's followers.''' for n in range(0, 20): @@ -214,3 +316,18 @@ def test_07_maximum_number_of_new_activities(self): extra_environ={'Authorization': str(self.joeadmin['apikey'])}) assert response.json['success'] is True assert self.dashboard_new_activities_count(self.new_user) == 15 + + def test_09_activities_that_should_not_show(self): + '''Test that other activities do not appear on the user's dashboard.''' + + before = self.dashboard_activity_list(self.new_user) + + # Make someone else who new_user is not following create a new dataset. + params = json.dumps({'name': 'irrelevant_dataset'}) + response = self.app.post('/api/action/package_create', params=params, + extra_environ={'Authorization': str(self.testsysadmin['apikey'])}) + assert response.json['success'] is True + + after = self.dashboard_activity_list(self.new_user) + + assert before == after diff --git a/ckan/tests/functional/api/test_follow.py b/ckan/tests/functional/api/test_follow.py index 8f67ffa848d..90bc4a23101 100644 --- a/ckan/tests/functional/api/test_follow.py +++ b/ckan/tests/functional/api/test_follow.py @@ -1,9 +1,21 @@ +'''Test for the follower API. + +This module tests following, unfollowing, getting a list of what you're +following or the number of things you're following, getting a list of who's +following you or the number of followers you have, testing whether or not +you're following something, etc. + +This module _does not_ test the user dashboard activity stream (which shows +activities from everything you're following), that is tested in +test_dashboard.py. + +''' import datetime import paste import pylons.test import ckan -from ckan.lib.helpers import json from ckan.tests import are_foreign_keys_supported, SkipTest +import ckan.tests def datetime_from_string(s): '''Return a standard datetime.datetime object initialised from a string in @@ -24,83 +36,54 @@ def follow_user(app, follower_id, apikey, object_id, object_arg): ''' # Record the object's followers count before. - params = json.dumps({'id': object_id}) - response = app.post('/api/action/user_follower_count', - params=params).json - assert response['success'] is True - follower_count_before = response['result'] + follower_count_before = ckan.tests.call_action_api(app, + 'user_follower_count', id=object_id) # Record the follower's followees count before. - params = json.dumps({'id': follower_id}) - response = app.post('/api/action/user_followee_count', - params=params).json - assert response['success'] is True - followee_count_before = response['result'] + followee_count_before = ckan.tests.call_action_api(app, + 'user_followee_count', id=follower_id) # Check that the user is not already following the object. - params = json.dumps({'id': object_id}) - extra_environ = {'Authorization': str(apikey)} - response = app.post('/api/action/am_following_user', - params=params, extra_environ=extra_environ).json - assert response['success'] is True - assert response['result'] is False + result = ckan.tests.call_action_api(app, 'am_following_user', + id=object_id, apikey=apikey) + assert result is False # Make the user start following the object. before = datetime.datetime.now() - params = {'id': object_arg} - extra_environ = {'Authorization': str(apikey)} - response = app.post('/api/action/follow_user', - params=json.dumps(params), extra_environ=extra_environ).json + follower = ckan.tests.call_action_api(app, 'follow_user', id=object_arg, + apikey=apikey) after = datetime.datetime.now() - assert response['success'] is True - assert response['result'] - follower = response['result'] assert follower['follower_id'] == follower_id assert follower['object_id'] == object_id timestamp = datetime_from_string(follower['datetime']) assert (timestamp >= before and timestamp <= after), str(timestamp) # Check that am_following_user now returns True. - params = json.dumps({'id': object_id}) - extra_environ = {'Authorization': str(apikey)} - response = app.post('/api/action/am_following_user', - params=params, extra_environ=extra_environ).json - assert response['success'] is True - assert response['result'] is True + result = ckan.tests.call_action_api(app, 'am_following_user', + id=object_id, apikey=apikey) + assert result is True # Check that the follower appears in the object's list of followers. - params = json.dumps({'id': object_id}) - response = app.post('/api/action/user_follower_list', - params=params).json - assert response['success'] is True - assert response['result'] - followers = response['result'] + followers = ckan.tests.call_action_api(app, 'user_follower_list', + id=object_id) assert len(followers) == follower_count_before + 1 assert len([follower for follower in followers if follower['id'] == follower_id]) == 1 # Check that the object appears in the follower's list of followees. - params = json.dumps({'id': follower_id}) - response = app.post('/api/action/user_followee_list', - params=params).json - assert response['success'] is True - assert response['result'] - followees = response['result'] + followees = ckan.tests.call_action_api(app, 'user_followee_list', + id=follower_id) assert len(followees) == followee_count_before + 1 assert len([followee for followee in followees if followee['id'] == object_id]) == 1 # Check that the object's follower count has increased by 1. - params = json.dumps({'id': object_id}) - response = app.post('/api/action/user_follower_count', - params=params).json - assert response['success'] is True - assert response['result'] == follower_count_before + 1 + follower_count_after = ckan.tests.call_action_api(app, + 'user_follower_count', id=object_id) + assert follower_count_after == follower_count_before + 1 # Check that the follower's followee count has increased by 1. - params = json.dumps({'id': follower_id}) - response = app.post('/api/action/user_followee_count', - params=params).json - assert response['success'] is True - assert response['result'] == followee_count_before + 1 + followee_count_after = ckan.tests.call_action_api(app, + 'user_followee_count', id=follower_id) + assert followee_count_after == followee_count_before + 1 def follow_dataset(app, follower_id, apikey, dataset_id, dataset_arg): '''Test a user starting to follow a dataset via the API. @@ -113,83 +96,54 @@ def follow_dataset(app, follower_id, apikey, dataset_id, dataset_arg): ''' # Record the dataset's followers count before. - params = json.dumps({'id': dataset_id}) - response = app.post('/api/action/dataset_follower_count', - params=params).json - assert response['success'] is True - follower_count_before = response['result'] + follower_count_before = ckan.tests.call_action_api(app, + 'dataset_follower_count', id=dataset_id) # Record the follower's followees count before. - params = json.dumps({'id': follower_id}) - response = app.post('/api/action/dataset_followee_count', - params=params).json - assert response['success'] is True - followee_count_before = response['result'] + followee_count_before = ckan.tests.call_action_api(app, + 'dataset_followee_count', id=follower_id) # Check that the user is not already following the dataset. - params = json.dumps({'id': dataset_id}) - extra_environ = {'Authorization': str(apikey)} - response = app.post('/api/action/am_following_dataset', - params=params, extra_environ=extra_environ).json - assert response['success'] is True - assert response['result'] is False + result = ckan.tests.call_action_api(app, 'am_following_dataset', + id=dataset_id, apikey=apikey) + assert result is False # Make the user start following the dataset. before = datetime.datetime.now() - params = {'id': dataset_arg} - extra_environ = {'Authorization': str(apikey)} - response = app.post('/api/action/follow_dataset', - params=json.dumps(params), extra_environ=extra_environ).json + follower = ckan.tests.call_action_api(app, 'follow_dataset', + id=dataset_arg, apikey=apikey) after = datetime.datetime.now() - assert response['success'] is True - assert response['result'] - follower = response['result'] assert follower['follower_id'] == follower_id assert follower['object_id'] == dataset_id timestamp = datetime_from_string(follower['datetime']) assert (timestamp >= before and timestamp <= after), str(timestamp) # Check that am_following_dataset now returns True. - params = json.dumps({'id': dataset_id}) - extra_environ = {'Authorization': str(apikey)} - response = app.post('/api/action/am_following_dataset', - params=params, extra_environ=extra_environ).json - assert response['success'] is True - assert response['result'] is True + result = ckan.tests.call_action_api(app, 'am_following_dataset', + id=dataset_id, apikey=apikey) + assert result is True # Check that the follower appears in the dataset's list of followers. - params = json.dumps({'id': dataset_id}) - response = app.post('/api/action/dataset_follower_list', - params=params).json - assert response['success'] is True - assert response['result'] - followers = response['result'] + followers = ckan.tests.call_action_api(app, 'dataset_follower_list', + id=dataset_id) assert len(followers) == follower_count_before + 1 assert len([follower for follower in followers if follower['id'] == follower_id]) == 1 # Check that the dataset appears in the follower's list of followees. - params = json.dumps({'id': follower_id}) - response = app.post('/api/action/dataset_followee_list', - params=params).json - assert response['success'] is True - assert response['result'] - followees = response['result'] + followees = ckan.tests.call_action_api(app, 'dataset_followee_list', + id=follower_id) assert len(followees) == followee_count_before + 1 assert len([followee for followee in followees if followee['id'] == dataset_id]) == 1 # Check that the dataset's follower count has increased by 1. - params = json.dumps({'id': dataset_id}) - response = app.post('/api/action/dataset_follower_count', - params=params).json - assert response['success'] is True - assert response['result'] == follower_count_before + 1 + follower_count_after = ckan.tests.call_action_api(app, + 'dataset_follower_count', id=dataset_id) + assert follower_count_after == follower_count_before + 1 # Check that the follower's followee count has increased by 1. - params = json.dumps({'id': follower_id}) - response = app.post('/api/action/dataset_followee_count', - params=params).json - assert response['success'] is True - assert response['result'] == followee_count_before + 1 + followee_count_after = ckan.tests.call_action_api(app, + 'dataset_followee_count', id=follower_id) + assert followee_count_after == followee_count_before + 1 def follow_group(app, user_id, apikey, group_id, group_arg): '''Test a user starting to follow a group via the API. @@ -202,85 +156,56 @@ def follow_group(app, user_id, apikey, group_id, group_arg): ''' # Record the group's followers count before. - params = json.dumps({'id': group_id}) - response = app.post('/api/action/group_follower_count', - params=params).json - assert response['success'] is True - follower_count_before = response['result'] + follower_count_before = ckan.tests.call_action_api(app, + 'group_follower_count', id=group_id) # Record the user's followees count before. - params = json.dumps({'id': user_id}) - response = app.post('/api/action/group_followee_count', - params=params).json - assert response['success'] is True - followee_count_before = response['result'] + followee_count_before = ckan.tests.call_action_api(app, + 'group_followee_count', id=user_id) # Check that the user is not already following the group. - params = json.dumps({'id': group_id}) - extra_environ = {'Authorization': str(apikey)} - response = app.post('/api/action/am_following_group', - params=params, extra_environ=extra_environ).json - assert response['success'] is True - assert response['result'] is False + result = ckan.tests.call_action_api(app, 'am_following_group', + id=group_id, apikey=apikey) + assert result is False # Make the user start following the group. before = datetime.datetime.now() - params = {'id': group_id} - extra_environ = {'Authorization': str(apikey)} - response = app.post('/api/action/follow_group', - params=json.dumps(params), extra_environ=extra_environ).json + follower = ckan.tests.call_action_api(app, 'follow_group', id=group_id, + apikey=apikey) after = datetime.datetime.now() - assert response['success'] is True - assert response['result'] - follower = response['result'] assert follower['follower_id'] == user_id assert follower['object_id'] == group_id timestamp = datetime_from_string(follower['datetime']) assert (timestamp >= before and timestamp <= after), str(timestamp) # Check that am_following_group now returns True. - params = json.dumps({'id': group_id}) - extra_environ = {'Authorization': str(apikey)} - response = app.post('/api/action/am_following_group', - params=params, extra_environ=extra_environ).json - assert response['success'] is True - assert response['result'] is True + result = ckan.tests.call_action_api(app, 'am_following_group', + id=group_id, apikey=apikey) + assert result is True # Check that the user appears in the group's list of followers. - params = json.dumps({'id': group_id}) - response = app.post('/api/action/group_follower_list', - params=params).json - assert response['success'] is True - assert response['result'] - followers = response['result'] + followers = ckan.tests.call_action_api(app, 'group_follower_list', + id=group_id) assert len(followers) == follower_count_before + 1 assert len([follower for follower in followers if follower['id'] == user_id]) == 1 # Check that the group appears in the user's list of followees. - params = json.dumps({'id': user_id}) - response = app.post('/api/action/group_followee_list', - params=params).json - assert response['success'] is True - assert response['result'] - followees = response['result'] + followees = ckan.tests.call_action_api(app, 'group_followee_list', + id=user_id) assert len(followees) == followee_count_before + 1 assert len([followee for followee in followees if followee['id'] == group_id]) == 1 # Check that the group's follower count has increased by 1. - params = json.dumps({'id': group_id}) - response = app.post('/api/action/group_follower_count', - params=params).json - assert response['success'] is True - assert response['result'] == follower_count_before + 1 + follower_count_after = ckan.tests.call_action_api(app, + 'group_follower_count', id=group_id) + assert follower_count_after == follower_count_before + 1 # Check that the user's followee count has increased by 1. - params = json.dumps({'id': user_id}) - response = app.post('/api/action/group_followee_count', - params=params).json - assert response['success'] is True - assert response['result'] == followee_count_before + 1 + followee_count_after = ckan.tests.call_action_api(app, + 'group_followee_count', id=user_id) + assert followee_count_after == followee_count_before + 1 class TestFollow(object): @@ -333,82 +258,61 @@ def teardown_class(self): def test_01_user_follow_user_bad_apikey(self): for apikey in ('bad api key', '', ' ', 'None', '3', '35.7', 'xxx'): - params = json.dumps({'id': self.russianfan['id']}) - extra_environ = {'Authorization': apikey} - response = self.app.post('/api/action/follow_user', - params=params, extra_environ=extra_environ, status=403).json - assert response['success'] is False - assert response['error']['message'] == 'Access denied' + error = ckan.tests.call_action_api(self.app, 'follow_user', + id=self.russianfan['id'], apikey=apikey, + status=403) + assert error['message'] == 'Access denied' def test_01_user_follow_dataset_bad_apikey(self): for apikey in ('bad api key', '', ' ', 'None', '3', '35.7', 'xxx'): - params = json.dumps({'id': self.warandpeace['id']}) - extra_environ = {'Authorization': apikey} - response = self.app.post('/api/action/follow_dataset', - params=params, extra_environ=extra_environ, status=403).json - assert response['success'] is False - assert response['error']['message'] == 'Access denied' + error = ckan.tests.call_action_api(self.app, 'follow_dataset', + id=self.warandpeace['id'], apikey=apikey, + status=403) + assert error['message'] == 'Access denied' def test_01_user_follow_group_bad_apikey(self): for apikey in ('bad api key', '', ' ', 'None', '3', '35.7', 'xxx'): - params = json.dumps({'id': self.rogers_group['id']}) - extra_environ = {'Authorization': apikey} - response = self.app.post('/api/action/follow_group', - params=params, extra_environ=extra_environ, status=403).json - assert response['success'] is False - assert response['error']['message'] == 'Access denied' + error = ckan.tests.call_action_api(self.app, 'follow_group', + id=self.rogers_group['id'], apikey=apikey, + status=403) + assert error['message'] == 'Access denied' def test_01_user_follow_user_missing_apikey(self): - params = json.dumps({'id': self.russianfan['id']}) - response = self.app.post('/api/action/follow_user', - params=params, status=403).json - assert response['success'] is False - assert response['error']['message'] == 'Access denied' + error = ckan.tests.call_action_api(self.app, 'follow_user', + id=self.russianfan['id'], status=403) + assert error['message'] == 'Access denied' def test_01_user_follow_dataset_missing_apikey(self): - params = json.dumps({'id': self.warandpeace['id']}) - response = self.app.post('/api/action/follow_dataset', - params=params, status=403).json - assert response['success'] is False - assert response['error']['message'] == 'Access denied' + error = ckan.tests.call_action_api(self.app, 'follow_dataset', + id=self.warandpeace['id'], status=403) + assert error['message'] == 'Access denied' def test_01_user_follow_group_missing_apikey(self): - params = json.dumps({'id': self.rogers_group['id']}) - response = self.app.post('/api/action/follow_group', - params=params, status=403).json - assert response['success'] is False - assert response['error']['message'] == 'Access denied' + error = ckan.tests.call_action_api(self.app, 'follow_group', + id=self.rogers_group['id'], status=403) + assert error['message'] == 'Access denied' def test_01_follow_bad_object_id(self): for action in ('follow_user', 'follow_dataset', 'follow_group'): for object_id in ('bad id', ' ', 3, 35.7, 'xxx'): - params = json.dumps({'id': object_id}) - extra_environ = {'Authorization': str(self.annafan['apikey'])} - response = self.app.post('/api/action/{0}'.format(action), - params=params, extra_environ=extra_environ, - status=409).json - assert response['success'] is False - assert response['error']['id'][0].startswith('Not found') + error = ckan.tests.call_action_api(self.app, action, + id=object_id, + apikey=self.annafan['apikey'], status=409) + assert error['id'][0].startswith('Not found') def test_01_follow_empty_object_id(self): for action in ('follow_user', 'follow_dataset', 'follow_group'): for object_id in ('', None): - params = json.dumps({'id': object_id}) - extra_environ = {'Authorization': str(self.annafan['apikey'])} - response = self.app.post('/api/action/{0}'.format(action), - params=params, extra_environ=extra_environ, - status=409).json - assert response['success'] is False - assert response['error']['id'] == ['Missing value'] + error = ckan.tests.call_action_api(self.app, action, + id=object_id, + apikey=self.annafan['apikey'], status=409) + assert error['id'] == ['Missing value'] def test_01_follow_missing_object_id(self): for action in ('follow_user', 'follow_dataset', 'follow_group'): - params = json.dumps({}) - extra_environ = {'Authorization': str(self.annafan['apikey'])} - response = self.app.post('/api/action/{0}'.format(action), - params=params, extra_environ=extra_environ, status=409).json - assert response['success'] is False - assert response['error']['id'] == ['Missing value'] + error = ckan.tests.call_action_api(self.app, action, + apikey=self.annafan['apikey'], status=409) + assert error['id'] == ['Missing value'] def test_02_user_follow_user_by_id(self): follow_user(self.app, self.annafan['id'], self.annafan['apikey'], @@ -437,204 +341,142 @@ def test_02_user_follow_group_by_name(self): def test_03_user_follow_user_already_following(self): for object_id in (self.russianfan['id'], self.russianfan['name'], self.testsysadmin['id'], self.testsysadmin['name']): - params = json.dumps({'id': object_id}) - extra_environ = { - 'Authorization': str(self.annafan['apikey']), - } - response = self.app.post('/api/action/follow_user', - params=params, extra_environ=extra_environ, status=409).json - assert response['success'] == False - assert response['error']['message'].startswith( - 'You are already following ') + error = ckan.tests.call_action_api(self.app, 'follow_user', + id=object_id, apikey=self.annafan['apikey'], + status=409) + assert error['message'].startswith('You are already following ') def test_03_user_follow_dataset_already_following(self): for object_id in (self.warandpeace['id'], self.warandpeace['name']): - params = json.dumps({'id': object_id}) - extra_environ = { - 'Authorization': str(self.annafan['apikey']), - } - response = self.app.post('/api/action/follow_dataset', - params=params, extra_environ=extra_environ, status=409).json - assert response['success'] == False - assert response['error']['message'].startswith( - 'You are already following ') + error = ckan.tests.call_action_api(self.app, 'follow_dataset', + id=object_id, apikey=self.annafan['apikey'], + status=409) + assert error['message'].startswith('You are already following ') def test_03_user_follow_group_already_following(self): for group_id in (self.rogers_group['id'], self.rogers_group['name']): - params = json.dumps({'id': group_id}) - extra_environ = { - 'Authorization': str(self.annafan['apikey']), - } - response = self.app.post('/api/action/follow_group', - params=params, extra_environ=extra_environ, status=409).json - assert response['success'] is False - assert response['error']['message'].startswith( - 'You are already following ') + error = ckan.tests.call_action_api(self.app, 'follow_group', + id=group_id, apikey=self.annafan['apikey'], + status=409) + assert error['message'].startswith('You are already following ') def test_03_user_cannot_follow_herself(self): - params = json.dumps({'id': self.annafan['id']}) - extra_environ = { - 'Authorization': str(self.annafan['apikey']), - } - response = self.app.post('/api/action/follow_user', - params=params, extra_environ=extra_environ, status=409).json - assert response['success'] is False - assert response['error']['message'] == 'You cannot follow yourself' + error = ckan.tests.call_action_api(self.app, 'follow_user', + apikey=self.annafan['apikey'], status=409, + id=self.annafan['id']) + assert error['message'] == 'You cannot follow yourself' def test_04_follower_count_bad_id(self): for action in ('user_follower_count', 'dataset_follower_count', 'group_follower_count'): for object_id in ('bad id', ' ', 3, 35.7, 'xxx', ''): - params = json.dumps({'id': object_id}) - response = self.app.post('/api/action/{0}'.format(action), - params=params, status=409).json - assert response['success'] is False - assert 'id' in response['error'] + error = ckan.tests.call_action_api(self.app, action, + status=409, id=object_id) + assert 'id' in error def test_04_follower_count_missing_id(self): for action in ('user_follower_count', 'dataset_follower_count', 'group_follower_count'): - params = json.dumps({}) - response = self.app.post('/api/action/{0}'.format(action), - params=params, status=409).json - assert response['success'] is False - assert response['error']['id'] == ['Missing value'] + error = ckan.tests.call_action_api(self.app, action, status=409) + assert error['id'] == ['Missing value'] def test_04_user_follower_count_no_followers(self): - params = json.dumps({'id': self.annafan['id']}) - response = self.app.post('/api/action/user_follower_count', - params=params).json - assert response['success'] is True - assert response['result'] == 0 + follower_count = ckan.tests.call_action_api(self.app, + 'user_follower_count', id=self.annafan['id']) + assert follower_count == 0 def test_04_dataset_follower_count_no_followers(self): - params = json.dumps({'id': self.annakarenina['id']}) - response = self.app.post('/api/action/dataset_follower_count', - params=params).json - assert response['success'] is True - assert response['result'] == 0 + follower_count = ckan.tests.call_action_api(self.app, + 'dataset_follower_count', id=self.annakarenina['id']) + assert follower_count == 0 def test_04_group_follower_count_no_followers(self): - params = json.dumps({'id': self.davids_group['id']}) - response = self.app.post('/api/action/group_follower_count', - params=params).json - assert response['success'] is True - assert response['result'] == 0 + follower_count = ckan.tests.call_action_api(self.app, + 'group_follower_count', id=self.davids_group['id']) + assert follower_count == 0 def test_04_follower_list_bad_id(self): for action in ('user_follower_list', 'dataset_follower_list', 'group_follower_list'): for object_id in ('bad id', ' ', 3, 35.7, 'xxx', ''): - params = json.dumps({'id': object_id}) - response = self.app.post('/api/action/{0}'.format(action), - params=params, status=409).json - assert response['success'] is False - assert response['error']['id'] + error = ckan.tests.call_action_api(self.app, action, + status=409, id=object_id) + assert error['id'] def test_04_follower_list_missing_id(self): for action in ('user_follower_list', 'dataset_follower_list', 'group_follower_list'): - params = json.dumps({}) - response = self.app.post('/api/action/{0}'.format(action), - params=params, status=409).json - assert response['success'] is False - assert response['error']['id'] == ['Missing value'] + error = ckan.tests.call_action_api(self.app, action, status=409) + assert error['id'] == ['Missing value'] def test_04_user_follower_list_no_followers(self): - params = json.dumps({'id': self.annafan['id']}) - response = self.app.post('/api/action/user_follower_list', - params=params).json - assert response['success'] is True - assert response['result'] == [] + followers = ckan.tests.call_action_api(self.app, 'user_follower_list', + id=self.annafan['id']) + assert followers == [] def test_04_dataset_follower_list_no_followers(self): - params = json.dumps({'id': self.annakarenina['id']}) - response = self.app.post('/api/action/dataset_follower_list', - params=params).json - assert response['success'] is True - assert response['result'] == [] + followers = ckan.tests.call_action_api(self.app, + 'dataset_follower_list', id=self.annakarenina['id']) + assert followers == [] def test_04_group_follower_list_no_followers(self): - params = json.dumps({'id': self.davids_group['id']}) - response = self.app.post('/api/action/group_follower_list', - params=params).json - assert response['success'] is True - assert response['result'] == [] + followers = ckan.tests.call_action_api(self.app, 'group_follower_list', + id=self.davids_group['id']) + assert followers == [] def test_04_am_following_bad_id(self): for action in ('am_following_dataset', 'am_following_user', 'am_following_group'): for object_id in ('bad id', ' ', 3, 35.7, 'xxx'): - params = json.dumps({'id': object_id}) - extra_environ = {'Authorization': str(self.annafan['apikey'])} - response = self.app.post('/api/action/{0}'.format(action), - params=params, extra_environ=extra_environ, - status=409).json - assert response['success'] is False - assert response['error']['id'][0].startswith('Not found: ') + error = ckan.tests.call_action_api(self.app, action, + apikey=self.annafan['apikey'], status=409, id=object_id) + assert error['id'][0].startswith('Not found: ') def test_04_am_following_missing_id(self): for action in ('am_following_dataset', 'am_following_user', 'am_following_group'): for id in ('missing', None, ''): if id == 'missing': - params = json.dumps({}) + error = ckan.tests.call_action_api(self.app, action, + apikey=self.annafan['apikey'], status=409) else: - params = json.dumps({'id':id}) - extra_environ = {'Authorization': str(self.annafan['apikey'])} - response = self.app.post('/api/action/{0}'.format(action), - params=params, extra_environ=extra_environ, - status=409).json - assert response['success'] is False - assert response['error']['id'] == [u'Missing value'] + error = ckan.tests.call_action_api(self.app, action, + apikey=self.annafan['apikey'], status=409, id=id) + assert error['id'] == [u'Missing value'] def test_04_am_following_dataset_bad_apikey(self): for apikey in ('bad api key', '', ' ', 'None', '3', '35.7', 'xxx'): - params = json.dumps({'id': self.warandpeace['id']}) - extra_environ = {'Authorization': apikey} - response = self.app.post('/api/action/am_following_dataset', - params=params, extra_environ=extra_environ, status=403).json - assert response['success'] == False - assert response['error']['message'] == 'Access denied' + error = ckan.tests.call_action_api(self.app, + 'am_following_dataset', apikey=apikey, status=403, + id=self.warandpeace['id']) + assert error['message'] == 'Access denied' def test_04_am_following_dataset_missing_apikey(self): - params = json.dumps({'id': self.warandpeace['id']}) - response = self.app.post('/api/action/am_following_dataset', - params=params, status=403).json - assert response['success'] == False - assert response['error']['message'] == 'Access denied' + error = ckan.tests.call_action_api(self.app, 'am_following_dataset', + status=403, id=self.warandpeace['id']) + assert error['message'] == 'Access denied' def test_04_am_following_user_bad_apikey(self): for apikey in ('bad api key', '', ' ', 'None', '3', '35.7', 'xxx'): - params = json.dumps({'id': self.annafan['id']}) - extra_environ = {'Authorization': apikey} - response = self.app.post('/api/action/am_following_user', - params=params, extra_environ=extra_environ, status=403).json - assert response['success'] == False - assert response['error']['message'] == 'Access denied' + error = ckan.tests.call_action_api(self.app, 'am_following_user', + apikey=apikey, status=403, id=self.annafan['id']) + assert error['message'] == 'Access denied' def test_04_am_following_user_missing_apikey(self): - params = json.dumps({'id': self.annafan['id']}) - response = self.app.post('/api/action/am_following_user', - params=params, status=403).json - assert response['success'] == False - assert response['error']['message'] == 'Access denied' + error = ckan.tests.call_action_api(self.app, 'am_following_user', + status=403, id=self.annafan['id']) + assert error['message'] == 'Access denied' def test_04_am_following_group_bad_apikey(self): for apikey in ('bad api key', '', ' ', 'None', '3', '35.7', 'xxx'): - params = json.dumps({'id': self.rogers_group['id']}) - extra_environ = {'Authorization': apikey} - response = self.app.post('/api/action/am_following_group', - params=params, extra_environ=extra_environ, status=403).json - assert response['success'] == False - assert response['error']['message'] == 'Access denied' + error = ckan.tests.call_action_api(self.app, 'am_following_group', + apikey=apikey, status=403, id=self.rogers_group['id']) + assert error['message'] == 'Access denied' def test_04_am_following_group_missing_apikey(self): - params = json.dumps({'id': self.rogers_group['id']}) - response = self.app.post('/api/action/am_following_group', - params=params, status=403).json - assert response['success'] == False - assert response['error']['message'] == 'Access denied' + error = ckan.tests.call_action_api(self.app, 'am_following_group', + status=403, id=self.rogers_group['id']) + assert error['message'] == 'Access denied' class TestFollowerDelete(object): @@ -717,45 +559,30 @@ def test_01_unfollow_user_not_exists(self): she is not following. ''' - params = json.dumps({'id': self.russianfan['id']}) - extra_environ = { - 'Authorization': str(self.annafan['apikey']), - } - response = self.app.post('/api/action/unfollow_user', - params=params, extra_environ=extra_environ, status=404).json - assert response['success'] == False - assert response['error']['message'].startswith( - 'Not found: You are not following ') + error = ckan.tests.call_action_api(self.app, 'unfollow_user', + apikey=self.annafan['apikey'], status=404, + id=self.russianfan['id']) + assert error['message'].startswith('Not found: You are not following ') def test_01_unfollow_dataset_not_exists(self): '''Test the error response when a user tries to unfollow a dataset that she is not following. ''' - params = json.dumps({'id': self.annakarenina['id']}) - extra_environ = { - 'Authorization': str(self.annafan['apikey']), - } - response = self.app.post('/api/action/unfollow_dataset', - params=params, extra_environ=extra_environ, status=404).json - assert response['success'] == False - assert response['error']['message'].startswith( - 'Not found: You are not following ') + error = ckan.tests.call_action_api(self.app, 'unfollow_dataset', + apikey=self.annafan['apikey'], status=404, + id=self.annakarenina['id']) + assert error['message'].startswith('Not found: You are not following') def test_01_unfollow_group_not_exists(self): '''Test the error response when a user tries to unfollow a group that she is not following. ''' - params = json.dumps({'id': self.rogers_group['id']}) - extra_environ = { - 'Authorization': str(self.annafan['apikey']), - } - response = self.app.post('/api/action/unfollow_group', - params=params, extra_environ=extra_environ, status=404).json - assert response['success'] is False - assert response['error']['message'].startswith( - 'Not found: You are not following ') + error = ckan.tests.call_action_api(self.app, 'unfollow_group', + apikey=self.annafan['apikey'], status=404, + id=self.rogers_group['id']) + assert error['message'].startswith('Not found: You are not following') def test_01_unfollow_bad_apikey(self): '''Test the error response when a user tries to unfollow something @@ -765,58 +592,36 @@ def test_01_unfollow_bad_apikey(self): for action in ('unfollow_user', 'unfollow_dataset', 'unfollow_group'): for apikey in ('bad api key', '', ' ', 'None', '3', '35.7', 'xxx'): - params = json.dumps({ - 'id': self.joeadmin['id'], - }) - extra_environ = { - 'Authorization': apikey, - } - response = self.app.post('/api/action/{0}'.format(action), - params=params, extra_environ=extra_environ, - status=403).json - assert response['success'] is False - assert response['error']['message'] == 'Access denied' + error = ckan.tests.call_action_api(self.app, action, + apikey=apikey, status=403, id=self.joeadmin['id']) + assert error['message'] == 'Access denied' def test_01_unfollow_missing_apikey(self): '''Test error response when calling unfollow_* without api key.''' for action in ('unfollow_user', 'unfollow_dataset', 'unfollow_group'): - params = json.dumps({ - 'id': self.joeadmin['id'], - }) - response = self.app.post('/api/action/{0}'.format(action), - params=params, status=403).json - assert response['success'] is False - assert response['error']['message'] == 'Access denied' + error = ckan.tests.call_action_api(self.app, action, status=403, + id=self.joeadmin['id']) + assert error['message'] == 'Access denied' def test_01_unfollow_bad_object_id(self): '''Test error response when calling unfollow_* with bad object id.''' for action in ('unfollow_user', 'unfollow_dataset', 'unfollow_group'): for object_id in ('bad id', ' ', 3, 35.7, 'xxx'): - params = json.dumps({ - 'id': object_id, - }) - extra_environ = { - 'Authorization': str(self.annafan['apikey']), - } - response = self.app.post('/api/action/{0}'.format(action), - params=params, extra_environ=extra_environ, - status=409).json - assert response['success'] is False - assert response['error']['id'][0].startswith('Not found') + error = ckan.tests.call_action_api(self.app, action, + apikey=self.annafan['apikey'], status=409, + id=object_id) + assert error['id'][0].startswith('Not found') def test_01_unfollow_missing_object_id(self): for action in ('unfollow_user', 'unfollow_dataset', 'unfollow_group'): for id in ('missing', None, ''): if id == 'missing': - params = json.dumps({}) + error = ckan.tests.call_action_api(self.app, action, + apikey=self.annafan['apikey'], status=409) else: - params = json.dumps({'id': id}) - extra_environ = {'Authorization': str(self.annafan['apikey'])} - response = self.app.post('/api/action/{0}'.format(action), - params=params, extra_environ=extra_environ, - status=409).json - assert response['success'] is False - assert response['error']['id'] == [u'Missing value'] + error = ckan.tests.call_action_api(self.app, action, + apikey=self.annafan['apikey'], status=409, id=id) + assert error['id'] == [u'Missing value'] def _unfollow_user(self, follower_id, apikey, object_id, object_arg): '''Test a user unfollowing a user via the API. @@ -829,53 +634,33 @@ def _unfollow_user(self, follower_id, apikey, object_id, object_arg): ''' # Record the user's number of followers before. - params = json.dumps({'id': object_id}) - response = self.app.post('/api/action/user_follower_count', - params=params).json - assert response['success'] is True - count_before = response['result'] + count_before = ckan.tests.call_action_api(self.app, + 'user_follower_count', id=object_id) # Check that the user is following the object. - params = json.dumps({'id': object_id}) - extra_environ = {'Authorization': str(apikey)} - response = self.app.post('/api/action/am_following_user', - params=params, extra_environ=extra_environ).json - assert response['success'] is True - assert response['result'] is True + am_following = ckan.tests.call_action_api(self.app, + 'am_following_user', apikey=apikey, id=object_id) + assert am_following is True # Make the user unfollow the object. - params = { - 'id': object_arg, - } - extra_environ = {'Authorization': str(apikey)} - response = self.app.post('/api/action/unfollow_user', - params=json.dumps(params), extra_environ=extra_environ).json - assert response['success'] is True + ckan.tests.call_action_api(self.app, 'unfollow_user', apikey=apikey, + id=object_arg) # Check that am_following_user now returns False. - params = json.dumps({'id': object_id}) - extra_environ = {'Authorization': str(apikey)} - response = self.app.post('/api/action/am_following_user', - params=params, extra_environ=extra_environ).json - assert response['success'] is True - assert response['result'] is False + am_following = ckan.tests.call_action_api(self.app, + 'am_following_user', apikey=apikey, id=object_id) + assert am_following is False # Check that the user doesn't appear in the object's list of followers. - params = json.dumps({'id': object_id}) - response = self.app.post('/api/action/user_follower_list', - params=params).json - assert response['success'] is True - assert response['result'] - followers = response['result'] + followers = ckan.tests.call_action_api(self.app, 'user_follower_list', + id=object_id) assert len([follower for follower in followers if follower['id'] == follower_id]) == 0 # Check that the object's follower count has decreased by 1. - params = json.dumps({'id': object_id}) - response = self.app.post('/api/action/user_follower_count', - params=params).json - assert response['success'] is True - assert response['result'] == count_before - 1 + count_after = ckan.tests.call_action_api(self.app, + 'user_follower_count', id=object_id) + assert count_after == count_before - 1 def _unfollow_dataset(self, user_id, apikey, dataset_id, dataset_arg): '''Test a user unfollowing a dataset via the API. @@ -888,54 +673,34 @@ def _unfollow_dataset(self, user_id, apikey, dataset_id, dataset_arg): ''' # Record the dataset's number of followers before. - params = json.dumps({'id': dataset_id}) - response = self.app.post('/api/action/dataset_follower_count', - params=params).json - assert response['success'] is True - count_before = response['result'] + count_before = ckan.tests.call_action_api(self.app, + 'dataset_follower_count', id=dataset_id) # Check that the user is following the dataset. - params = json.dumps({'id': dataset_id}) - extra_environ = {'Authorization': str(apikey)} - response = self.app.post('/api/action/am_following_dataset', - params=params, extra_environ=extra_environ).json - assert response['success'] is True - assert response['result'] is True + am_following = ckan.tests.call_action_api(self.app, + 'am_following_dataset', apikey=apikey, id=dataset_id) + assert am_following is True # Make the user unfollow the dataset. - params = { - 'id': dataset_arg, - } - extra_environ = {'Authorization': str(apikey)} - response = self.app.post('/api/action/unfollow_dataset', - params=json.dumps(params), extra_environ=extra_environ).json - assert response['success'] is True + ckan.tests.call_action_api(self.app, 'unfollow_dataset', apikey=apikey, + id=dataset_arg) # Check that am_following_dataset now returns False. - params = json.dumps({'id': dataset_id}) - extra_environ = {'Authorization': str(apikey)} - response = self.app.post('/api/action/am_following_dataset', - params=params, extra_environ=extra_environ).json - assert response['success'] is True - assert response['result'] is False + am_following = ckan.tests.call_action_api(self.app, + 'am_following_dataset', apikey=apikey, id=dataset_id) + assert am_following is False # Check that the user doesn't appear in the dataset's list of # followers. - params = json.dumps({'id': dataset_id}) - response = self.app.post('/api/action/dataset_follower_list', - params=params).json - assert response['success'] is True - assert response['result'] - followers = response['result'] + followers = ckan.tests.call_action_api(self.app, + 'dataset_follower_list', id=dataset_id) assert len([follower for follower in followers if follower['id'] == user_id]) == 0 # Check that the dataset's follower count has decreased by 1. - params = json.dumps({'id': dataset_id}) - response = self.app.post('/api/action/dataset_follower_count', - params=params).json - assert response['success'] is True - assert response['result'] == count_before - 1 + count_after = ckan.tests.call_action_api(self.app, + 'dataset_follower_count', id=dataset_id) + assert count_after == count_before - 1 def _unfollow_group(self, user_id, apikey, group_id, group_arg): '''Test a user unfollowing a group via the API. @@ -948,54 +713,34 @@ def _unfollow_group(self, user_id, apikey, group_id, group_arg): ''' # Record the group's number of followers before. - params = json.dumps({'id': group_id}) - response = self.app.post('/api/action/group_follower_count', - params=params).json - assert response['success'] is True - count_before = response['result'] + count_before = ckan.tests.call_action_api(self.app, + 'group_follower_count', id=group_id) # Check that the user is following the group. - params = json.dumps({'id': group_id}) - extra_environ = {'Authorization': str(apikey)} - response = self.app.post('/api/action/am_following_group', - params=params, extra_environ=extra_environ).json - assert response['success'] is True - assert response['result'] is True + am_following = ckan.tests.call_action_api(self.app, + 'am_following_group', apikey=apikey, id=group_id) + assert am_following is True # Make the user unfollow the group. - params = { - 'id': group_arg, - } - extra_environ = {'Authorization': str(apikey)} - response = self.app.post('/api/action/unfollow_group', - params=json.dumps(params), extra_environ=extra_environ).json - assert response['success'] is True + ckan.tests.call_action_api(self.app, 'unfollow_group', apikey=apikey, + id=group_arg) # Check that am_following_group now returns False. - params = json.dumps({'id': group_id}) - extra_environ = {'Authorization': str(apikey)} - response = self.app.post('/api/action/am_following_group', - params=params, extra_environ=extra_environ).json - assert response['success'] is True - assert response['result'] is False + am_following = ckan.tests.call_action_api(self.app, + 'am_following_group', apikey=apikey, id=group_id) + assert am_following is False # Check that the user doesn't appear in the group's list of # followers. - params = json.dumps({'id': group_id}) - response = self.app.post('/api/action/group_follower_list', - params=params).json - assert response['success'] is True - assert 'result' in response - followers = response['result'] + followers = ckan.tests.call_action_api(self.app, 'group_follower_list', + id=group_id) assert len([follower for follower in followers if follower['id'] == user_id]) == 0 # Check that the group's follower count has decreased by 1. - params = json.dumps({'id': group_id}) - response = self.app.post('/api/action/group_follower_count', - params=params).json - assert response['success'] is True - assert response['result'] == count_before - 1 + count_after = ckan.tests.call_action_api(self.app, + 'group_follower_count', id=group_id) + assert count_after == count_before - 1 def test_02_follower_delete_by_id(self): self._unfollow_user(self.annafan['id'], self.annafan['apikey'], @@ -1102,136 +847,93 @@ def test_01_on_delete_cascade_api(self): ''' # It should no longer be possible to get joeadmin's follower list. - params = json.dumps({'id': 'joeadmin'}) - response = self.app.post('/api/action/user_follower_list', - params=params, status=409).json - assert response['success'] is False - assert response['error'].has_key('id') + error = ckan.tests.call_action_api(self.app, 'user_follower_list', + status=409, id='joeadmin') + assert 'id' in error # It should no longer be possible to get warandpeace's follower list. - params = json.dumps({'id': 'warandpeace'}) - response = self.app.post('/api/action/dataset_follower_list', - params=params, status=409).json - assert response['success'] is False - assert response['error'].has_key('id') + error = ckan.tests.call_action_api(self.app, 'dataset_follower_list', + status=409, id='warandpeace') + assert 'id' in error # It should no longer be possible to get david's follower list. - params = json.dumps({'id': 'david'}) - response = self.app.post('/api/action/group_follower_list', - params=params, status=409).json - assert response['success'] is False - assert response['error'].has_key('id') + error = ckan.tests.call_action_api(self.app, 'group_follower_list', + status=409, id='david') + assert 'id' in error # It should no longer be possible to get joeadmin's follower count. - params = json.dumps({'id': 'joeadmin'}) - response = self.app.post('/api/action/user_follower_count', - params=params, status=409).json - assert response['success'] is False - assert response['error'].has_key('id') + error = ckan.tests.call_action_api(self.app, 'user_follower_count', + status=409, id='joeadmin') + assert 'id' in error # It should no longer be possible to get warandpeace's follower count. - params = json.dumps({'id': 'warandpeace'}) - response = self.app.post('/api/action/dataset_follower_count', - params=params, status=409).json - assert response['success'] is False - assert response['error'].has_key('id') + error = ckan.tests.call_action_api(self.app, 'dataset_follower_count', + status=409, id='warandpeace') + assert 'id' in error # It should no longer be possible to get david's follower count. - params = json.dumps({'id': 'david'}) - response = self.app.post('/api/action/group_follower_count', - params=params, status=409).json - assert response['success'] is False - assert response['error'].has_key('id') + error = ckan.tests.call_action_api(self.app, 'group_follower_count', + status=409, id='david') + assert 'id' in error # It should no longer be possible to get am_following for joeadmin. - params = json.dumps({'id': 'joeadmin'}) - extra_environ = {'Authorization': str(self.testsysadmin['apikey'])} - response = self.app.post('/api/action/am_following_user', - params=params, extra_environ=extra_environ, status=409).json - assert response['success'] is False - assert response['error'].has_key('id') + error = ckan.tests.call_action_api(self.app, 'am_following_user', + apikey=self.testsysadmin['apikey'], status=409, id='joeadmin') + assert 'id' in error # It should no longer be possible to get am_following for warandpeace. - params = json.dumps({'id': 'warandpeace'}) - extra_environ = {'Authorization': str(self.testsysadmin['apikey'])} - response = self.app.post('/api/action/am_following_dataset', - params=params, extra_environ=extra_environ, status=409).json - assert response['success'] is False - assert response['error'].has_key('id') + error = ckan.tests.call_action_api(self.app, 'am_following_dataset', + apikey=self.testsysadmin['apikey'], status=409, + id='warandpeace') + assert 'id' in error # It should no longer be possible to get am_following for david. - params = json.dumps({'id': 'david'}) - extra_environ = {'Authorization': str(self.testsysadmin['apikey'])} - response = self.app.post('/api/action/am_following_group', - params=params, extra_environ=extra_environ, status=409).json - assert response['success'] is False - assert response['error'].has_key('id') + error = ckan.tests.call_action_api(self.app, 'am_following_group', + apikey=self.testsysadmin['apikey'], status=409, id='david') + assert 'id' in error # It should no longer be possible to unfollow joeadmin. - params = json.dumps({'id': 'joeadmin'}) - extra_environ = {'Authorization': str(self.tester['apikey'])} - response = self.app.post('/api/action/unfollow_user', - params=params, extra_environ=extra_environ, status=409).json - assert response['success'] is False - assert response['error']['id'] == ['Not found: User'] + error = ckan.tests.call_action_api(self.app, 'unfollow_user', + apikey=self.tester['apikey'], status=409, id='joeadmin') + assert error['id'] == ['Not found: User'] # It should no longer be possible to unfollow warandpeace. - params = json.dumps({'id': 'warandpeace'}) - extra_environ = {'Authorization': str(self.testsysadmin['apikey'])} - response = self.app.post('/api/action/unfollow_dataset', - params=params, extra_environ=extra_environ, status=409).json - assert response['success'] is False - assert response['error']['id'] == ['Not found: Dataset'] + error = ckan.tests.call_action_api(self.app, 'unfollow_dataset', + apikey=self.testsysadmin['apikey'], status=409, + id='warandpeace') + assert error['id'] == ['Not found: Dataset'] # It should no longer be possible to unfollow david. - params = json.dumps({'id': 'david'}) - extra_environ = {'Authorization': str(self.testsysadmin['apikey'])} - response = self.app.post('/api/action/unfollow_group', - params=params, extra_environ=extra_environ, status=409).json - assert response['success'] is False - assert response['error']['id'] == ['Not found: Group'] + error = ckan.tests.call_action_api(self.app, 'unfollow_group', + apikey=self.testsysadmin['apikey'], status=409, id='david') + assert error['id'] == ['Not found: Group'] # It should no longer be possible to follow joeadmin. - params = json.dumps({'id': 'joeadmin'}) - extra_environ = {'Authorization': str(self.annafan['apikey'])} - response = self.app.post('/api/action/follow_user', - params=params, extra_environ=extra_environ, status=409).json - assert response['success'] is False - assert response['error'].has_key('id') + error = ckan.tests.call_action_api(self.app, 'follow_user', + apikey=self.annafan['apikey'], status=409, id='joeadmin') + assert 'id' in error # It should no longer be possible to follow warandpeace. - params = json.dumps({'id': 'warandpeace'}) - extra_environ = {'Authorization': str(self.annafan['apikey'])} - response = self.app.post('/api/action/follow_dataset', - params=params, extra_environ=extra_environ, status=409).json - assert response['success'] is False - assert response['error'].has_key('id') + error = ckan.tests.call_action_api(self.app, 'follow_dataset', + apikey=self.annafan['apikey'], status=409, id='warandpeace') + assert 'id' in error # It should no longer be possible to follow david. - params = json.dumps({'id': 'david'}) - extra_environ = {'Authorization': str(self.annafan['apikey'])} - response = self.app.post('/api/action/follow_group', - params=params, extra_environ=extra_environ, status=409).json - assert response['success'] is False - assert response['error'].has_key('id') + error = ckan.tests.call_action_api(self.app, 'follow_group', + apikey=self.annafan['apikey'], status=409, id='david') + assert 'id' in error # Users who joeadmin was following should no longer have him in their # follower list. - params = json.dumps({'id': self.testsysadmin['id']}) - response = self.app.post('/api/action/user_follower_list', - params=params).json - assert response['success'] is True - followers = [follower['name'] for follower in response['result']] - assert 'joeadmin' not in followers + followers = ckan.tests.call_action_api(self.app, 'user_follower_list', + id=self.testsysadmin['id']) + assert 'joeadmin' not in [follower['name'] for follower in followers] # Datasets who joeadmin was following should no longer have him in # their follower list. - params = json.dumps({'id': self.annakarenina['id']}) - response = self.app.post('/api/action/dataset_follower_list', - params=params).json - assert response['success'] is True - followers = [follower['name'] for follower in response['result']] - assert 'joeadmin' not in followers + followers = ckan.tests.call_action_api(self.app, + 'dataset_follower_list', id=self.annakarenina['id']) + assert 'joeadmin' not in [follower['name'] for follower in followers] def test_02_on_delete_cascade_db(self): if not are_foreign_keys_supported(): diff --git a/ckan/tests/functional/test_group.py b/ckan/tests/functional/test_group.py index 8a67d712457..cbe1fb52f13 100644 --- a/ckan/tests/functional/test_group.py +++ b/ckan/tests/functional/test_group.py @@ -187,7 +187,7 @@ def test_index(self): groupname = 'david' group = model.Group.by_name(unicode(groupname)) group_title = group.title - group_packages_count = len(group.active_packages().all()) + group_packages_count = len(group.packages()) group_description = group.description self.check_named_element(res, 'tr', group_title, group_packages_count, @@ -322,7 +322,7 @@ def test_2_edit(self): assert group.description == newdesc, group # now look at datasets - assert len(group.active_packages().all()) == 3 + assert len(group.packages()) == 3 def test_3_edit_form_has_new_package(self): # check for dataset in autocomplete @@ -370,7 +370,7 @@ def test_4_new_duplicate_package(self): # check package only added to the group once group = model.Group.by_name(group_name) - pkg_names = [pkg.name for pkg in group.active_packages().all()] + pkg_names = [pkg.name for pkg in group.packages()] assert_equal(pkg_names, [self.packagename]) def test_edit_plugin_hook(self): @@ -436,7 +436,7 @@ def update_group(res, name, with_pkg=True): # We have the datasets in the DB, but we should also see that many # on the group read page. - assert len(group.active_packages().all()) == 3 + assert len(group.packages()) == 3 offset = url_for(controller='group', action='read', id='newname') res = self.app.get(offset, status=200, @@ -544,9 +544,9 @@ def test_2_new(self): group = model.Group.by_name(group_name) assert group.title == group_title, group assert group.description == group_description, group - assert len(group.active_packages().all()) == 1 + assert len(group.packages()) == 1 pkg = model.Package.by_name(self.packagename) - assert group.active_packages().all() == [pkg] + assert group.packages() == [pkg] def test_3_new_duplicate_group(self): prefix = '' @@ -703,7 +703,7 @@ def test_index(self): groupname = 'david' group = model.Group.by_name(unicode(groupname)) group_title = group.title - group_packages_count = len(group.active_packages().all()) + group_packages_count = len(group.packages()) group_description = group.description self.check_named_element(res, 'tr', group_title, group_packages_count, diff --git a/ckan/tests/functional/test_user.py b/ckan/tests/functional/test_user.py index ee8db1350b0..1c4a7614012 100644 --- a/ckan/tests/functional/test_user.py +++ b/ckan/tests/functional/test_user.py @@ -187,8 +187,8 @@ def test_login(self): # then get redirected to user's dashboard res = res.follow() assert_equal(res.status, 302) - assert res.header('Location').startswith('http://localhost/user/dashboard') or \ - res.header('Location').startswith('/user/dashboard') + assert res.header('Location').startswith('http://localhost/dashboard') or \ + res.header('Location').startswith('/dashboard') res = res.follow() assert_equal(res.status, 200) assert 'testlogin is now logged in' in res.body diff --git a/ckan/tests/lib/test_dictization_schema.py b/ckan/tests/lib/test_dictization_schema.py index 712fe052e22..61bda75f2db 100644 --- a/ckan/tests/lib/test_dictization_schema.py +++ b/ckan/tests/lib/test_dictization_schema.py @@ -148,7 +148,7 @@ def test_2_group_schema(self): del data['extras'] converted_data, errors = validate(data, default_group_schema(), self.context) - group_pack = sorted(group.active_packages().all(), key=lambda x:x.id) + group_pack = sorted(group.packages(), key=lambda x:x.id) converted_data["packages"] = sorted(converted_data["packages"], key=lambda x:x["id"]) diff --git a/ckan/tests/mock_publisher_auth.py b/ckan/tests/mock_publisher_auth.py index 749588a4fd9..475666a21eb 100644 --- a/ckan/tests/mock_publisher_auth.py +++ b/ckan/tests/mock_publisher_auth.py @@ -1,21 +1,24 @@ -from ckan.new_authz import is_authorized from ckan.logic import NotAuthorized +import logging + +log = logging.getLogger("mock_publisher_auth") + class MockPublisherAuth(object): """ MockPublisherAuth """ - + def __init__(self): self.functions = {} self._load() def _load(self): - for auth_module_name in ['get', 'create', 'update','delete']: + for auth_module_name in ['get', 'create', 'update', 'delete']: module_path = 'ckan.logic.auth.publisher.%s' % (auth_module_name,) try: module = __import__(module_path) - except ImportError,e: + except ImportError: log.debug('No auth module for action "%s"' % auth_module_name) continue @@ -25,10 +28,9 @@ def _load(self): for key, v in module.__dict__.items(): if not key.startswith('_'): self.functions[key] = v - - - def check_access(self,action, context, data_dict): + + def check_access(self, action, context, data_dict): logic_authorization = self.functions[action](context, data_dict) if not logic_authorization['success']: - msg = logic_authorization.get('msg','') + msg = logic_authorization.get('msg', '') raise NotAuthorized(msg) diff --git a/ckan/tests/models/test_group.py b/ckan/tests/models/test_group.py index 703b63778f8..424e8f3c198 100644 --- a/ckan/tests/models/test_group.py +++ b/ckan/tests/models/test_group.py @@ -25,7 +25,7 @@ def test_1_basic(self): grp = model.Group.by_name(u'group1') assert grp.title == u'Test Group' assert grp.description == u'This is a test group' - assert grp.active_packages().all() == [] + assert grp.packages() == [] def test_2_add_packages(self): model.repo.new_revision() @@ -50,7 +50,7 @@ def test_2_add_packages(self): assert grp.title == u'Russian Group' anna = model.Package.by_name(u'annakarenina') war = model.Package.by_name(u'warandpeace') - assert set(grp.active_packages().all()) == set((anna, war)), grp.active_packages().all() + assert set(grp.packages()) == set((anna, war)), grp.packages() assert grp in anna.get_groups() def test_3_search(self): diff --git a/doc/contributing.rst b/doc/contributing.rst index cc9faa2483d..c22a9e8a5c5 100644 --- a/doc/contributing.rst +++ b/doc/contributing.rst @@ -109,6 +109,11 @@ branch and it will become part of CKAN! see `Feature Branches`_. - Your branch should contain new or changed tests for any new or changed code, see :ref:`Testing`. + - Your branch should contain updates to the + `CHANGELOG file `_ + briefly summarising your code changes. + - Your branch should contain new or updated documentation for any new or + updated code, see :doc:`contributing-docs`. - Your branch should be up to date with the master branch of the central CKAN repo, see `Keeping Up with master`_. - All the CKAN tests should pass on your branch, see :doc:`test`. diff --git a/doc/frontend-testing.rst b/doc/frontend-testing.rst new file mode 100644 index 00000000000..8387b51b051 --- /dev/null +++ b/doc/frontend-testing.rst @@ -0,0 +1,88 @@ +Front-end Testing +================= + +All new CKAN features should be coded so that they work in the +following browsers: + +* Internet Explorer: 9, 8 and 7 +* Firefox: Latest + previous version +* Chrome: Latest + previous version + +These browsers are determined by whatever has >= 1% share with the +latest months data from: http://data.gov.uk/data/site-usage + +Install browser virtual machines +-------------------------------- + +In order to test in all the needed browsers you'll need access to +all the above browser versions. Firefox and Chrome should be easy +whatever platform you are on. Internet Explorer is a little trickier. +You'll need Virtual Machines. + +We suggest you use https://github.com/xdissent/ievms to get your +Internet Explorer virtual machines. + +Testing methodology +------------------- + +Firstly we have a primer page. If you've touched any of the core +front-end code you'll need to check if the primer is rendering +correctly. The primer is located at: +http://localhost:5000/testing/primer + +Secondly whilst writing a new feature you should endeavour to test +in at least in your core browser and an alternative browser as often +as you can. + +Thirdly you should fully test all new features that have a front-end +element in all browsers before making your pull request into +CKAN master. + +Common pitfulls & their fixes +============================= + +Here's a few of the most common front end bugs and a list of their +fixes. + +Reserved JS keywords +-------------------- + +Since IE has a stricter language definition in JS it really doesn't +like you using JS reserved keywords method names, variables, etc... +This is a good list of keywords not to use in your JavaScript: + +https://developer.mozilla.org/en-US/docs/JavaScript/Reference/Reserved_Words + +:: + + /* These are bad */ + var a = { + default: 1, + delete: function() {} + }; + + /* These are good */ + var a = { + default_value: 1, + remove: function() {} + }; + +Unclosed JS arrays / objects +---------------------------- + +Internet Explorer doesn't like it's JS to have unclosed JS objects +and arrays. For example: + +:: + + /* These are bad */ + var a = { + b: 'c', + }; + var a = ['b', 'c', ]; + + /* These are good */ + var a = { + c: 'c' + }; + var a = ['b', 'c']; diff --git a/doc/python-coding-standards.rst b/doc/python-coding-standards.rst index 025160b9373..684cc581c08 100644 --- a/doc/python-coding-standards.rst +++ b/doc/python-coding-standards.rst @@ -23,8 +23,16 @@ avoid spurious merge conflicts, and aid in reading pull requests. Use Single Quotes ----------------- -Use the single-quote character, ``'``, rather than the double-quote character, -``"``, for string literals. +Use single-quotes for string literals, e.g. ``'my-identifier'``, *but* use +double-quotes for strings that are likely to contain single-quote characters as +part of the string itself (such as error messages, or any strings containing +natural language), e.g. ``"You've got an error!"``. + +Single-quotes are easier to read and to type, but if a string contains +single-quote characters then double-quotes are better than escaping the +single-quote characters or wrapping the string in double single-quotes. + +We also use triple single-quotes for docstrings, see `Docstrings`_. Imports diff --git a/pip-requirements-test.txt b/pip-requirements-test.txt index b8103e124eb..41a359c0264 100644 --- a/pip-requirements-test.txt +++ b/pip-requirements-test.txt @@ -1,6 +1,5 @@ # These are packages that required when running ckan tests nose -requests==0.6.4 -e git+https://github.com/okfn/ckanclient#egg=ckanclient diff --git a/pip-requirements.txt b/pip-requirements.txt index 9a210b663a2..52b4c084a27 100644 --- a/pip-requirements.txt +++ b/pip-requirements.txt @@ -27,3 +27,4 @@ routes==1.13 paste==1.7.5.1 Jinja2==2.6 fanstatic==0.12 +requests==0.14