diff --git a/ckan/controllers/group.py b/ckan/controllers/group.py index d6bffcc8be0..1867079529b 100644 --- a/ckan/controllers/group.py +++ b/ckan/controllers/group.py @@ -342,7 +342,7 @@ def _force_reindex(self, grp): appearing on the read page for the group (as they're connected via the group name)''' group = model.Group.get(grp['name']) - for dataset in group.active_packages().all(): + for dataset in group.packages(): search.rebuild(dataset.name) def _save_edit(self, id, context): diff --git a/ckan/lib/dictization/model_dictize.py b/ckan/lib/dictization/model_dictize.py index 8a08ff50b03..6926e4a3395 100644 --- a/ckan/lib/dictization/model_dictize.py +++ b/ckan/lib/dictization/model_dictize.py @@ -32,7 +32,7 @@ def group_list_dictize(obj_list, context, group_dict['display_name'] = obj.display_name group_dict['packages'] = \ - len(obj.active_packages(with_private=with_private).all()) + len(obj.packages(with_private=with_private)) if context.get('for_view'): for item in plugins.PluginImplementations( diff --git a/ckan/logic/action/get.py b/ckan/logic/action/get.py index 146cd899b86..c85e3a7ece9 100644 --- a/ckan/logic/action/get.py +++ b/ckan/logic/action/get.py @@ -750,34 +750,22 @@ def group_package_show(context, data_dict): :rtype: list of dictionaries ''' - model = context["model"] - user = context["user"] - id = _get_or_bust(data_dict, 'id') - limit = data_dict.get("limit") + model = context['model'] + group_id = _get_or_bust(data_dict, 'id') - group = model.Group.get(id) + # FIXME: What if limit is not an int? Schema and validation needed. + limit = data_dict.get('limit') + + group = model.Group.get(group_id) context['group'] = group if group is None: raise NotFound _check_access('group_show', context, data_dict) - query = model.Session.query(model.PackageRevision)\ - .filter(model.PackageRevision.state=='active')\ - .filter(model.PackageRevision.current==True)\ - .join(model.Member, model.Member.table_id==model.PackageRevision.id)\ - .join(model.Group, model.Group.id==model.Member.group_id)\ - .filter_by(id=group.id)\ - .order_by(model.PackageRevision.name) - - if limit: - query = query.limit(limit) - - if context.get('return_query'): - return query - result = [] - for pkg_rev in query.all(): + for pkg_rev in group.packages(limit=limit, + return_query=context.get('return_query')): result.append(model_dictize.package_dictize(pkg_rev, context)) return result @@ -1779,30 +1767,7 @@ def group_activity_list(context, data_dict): group_show = logic.get_action('group_show') group_id = group_show(context, {'id': group_id})['id'] - # FIXME: The SQLAlchemy below should be moved into ckan/model/activity.py - # (to encapsulate SQLALchemy in the model and avoid using it from the - # logic) but it can't be because it requires the list of dataset_ids which - # comes from logic.group_package_show() (and I don't want to access the - # logic from the model). Need to change it to get the dataset_ids from the - # model instead. There seems to be multiple methods for getting a group's - # datasets, some in the logic and some in the model. - - # Get a list of the IDs of the group's datasets. - group_package_show = logic.get_action('group_package_show') - datasets = group_package_show(context, {'id': group_id}) - dataset_ids = [dataset['id'] for dataset in datasets] - - # Get the group's activities. - query = model.Session.query(model.Activity) - if dataset_ids: - query = query.filter(_or_(model.Activity.object_id == group_id, - model.Activity.object_id.in_(dataset_ids))) - else: - query = query.filter(model.Activity.object_id == group_id) - query = query.order_by(_desc(model.Activity.timestamp)) - query = query.limit(15) - activity_objects = query.all() - + activity_objects = model.activity.group_activity_list(group_id) return model_dictize.activity_list_dictize(activity_objects, context) def recently_changed_packages_activity_list(context, data_dict): diff --git a/ckan/model/activity.py b/ckan/model/activity.py index 9181acaa20d..6d9970e09ce 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,44 @@ 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) + 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 +187,7 @@ def _activites_from_users_followed_by_user_query(user_id): def _activities_from_datasets_followed_by_user_query(user_id): + '''Return a query for all activities from datasets that user_id follows.''' import ckan.model as model q = model.Session.query(model.Activity) q = q.join(model.UserFollowingDataset, @@ -151,9 +196,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 +238,7 @@ def activities_from_everything_followed_by_user(user_id, limit=15): def _dashboard_activity_query(user_id): + '''Return an SQLAlchemy query for user_id's dashboard activity stream.''' q = _user_activity_query(user_id) q = q.union(_activities_from_everything_followed_by_user_query(user_id)) return q @@ -188,7 +258,13 @@ def dashboard_activity_list(user_id, limit=15): return _most_recent_activities(q, limit) -def _recently_changed_packages_activity_query(): +def _changed_packages_activity_query(): + '''Return an SQLAlchemyu query for all changed package activities. + + Return a query for all activities with activity_type '*package', e.g. + 'new_package', 'changed_package', 'deleted_package'. + + ''' import ckan.model as model q = model.Session.query(model.Activity) q = q.filter(model.Activity.activity_type.endswith('package')) @@ -196,5 +272,11 @@ def _recently_changed_packages_activity_query(): def recently_changed_packages_activity_list(limit=15): - q = _recently_changed_packages_activity_query() + '''Return the site-wide stream of recently changed package activities. + + This activity stream includes recent 'new package', 'changed package' and + 'deleted package' activities for the whole site. + + ''' + q = _changed_packages_activity_query() return _most_recent_activities(q, limit) diff --git a/ckan/model/group.py b/ckan/model/group.py index 62767671538..35dfb73b461 100644 --- a/ckan/model/group.py +++ b/ckan/model/group.py @@ -143,35 +143,6 @@ def set_approval_status(self, status): if status == "denied": pass - def members_of_type(self, object_type, capacity=None): - from ckan import model - object_type_string = object_type.__name__.lower() - query = meta.Session.query(object_type).\ - filter(model.Group.id == self.id).\ - filter(model.Member.state == 'active').\ - filter(model.Member.table_name == object_type_string) - - if hasattr(object_type, 'state'): - query = query.filter(object_type.state == 'active') - - if capacity: - query = query.filter(model.Member.capacity == capacity) - - query = query.join(model.Member, member_table.c.table_id == - getattr(object_type, 'id')).\ - join(model.Group, group_table.c.id == member_table.c.group_id) - - return query - - def add_child(self, object_instance): - object_type_string = object_instance.__class__.__name__.lower() - if not object_instance in self.members_of_type( - object_instance.__class__).all(): - member = Member(group=self, - table_id=getattr(object_instance, 'id'), - table_name=object_type_string) - meta.Session.add(member) - def get_children_groups(self, type='group'): # Returns a list of dicts where each dict contains "id", "name", # and "title" When querying with a CTE specifying a model in the @@ -184,20 +155,48 @@ def get_children_groups(self, type='group'): return [{"id":idf, "name": name, "title": title} for idf, name, title in results] - def active_packages(self, load_eager=True, with_private=False): - query = meta.Session.query(_package.Package).\ - filter_by(state=vdm.sqlalchemy.State.ACTIVE).\ - filter(group_table.c.id == self.id).\ - filter(member_table.c.state == 'active') + def packages(self, with_private=False, limit=None, + return_query=False): + '''Return this group's active and pending packages. + + 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 + + ''' + query = meta.Session.query(_package.Package) + query = query.filter( + or_(_package.Package.state == vdm.sqlalchemy.State.ACTIVE, + _package.Package.state == vdm.sqlalchemy.State.PENDING)) + query = query.filter(group_table.c.id == self.id) if not with_private: query = query.filter(member_table.c.capacity == 'public') - query = query.join(member_table, member_table.c.table_id == - _package.Package.id).\ - 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) + + if limit is not None: + query = query.limit(limit) - return query + if return_query: + return query + else: + return query.all() @classmethod def search_by_name_or_title(cls, text_query, group_type=None): @@ -209,23 +208,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/tests/functional/api/test_activity.py b/ckan/tests/functional/api/test_activity.py index 4f7eff2a380..dfc14f9f6c4 100644 --- a/ckan/tests/functional/api/test_activity.py +++ b/ckan/tests/functional/api/test_activity.py @@ -1,3 +1,13 @@ +'''Functional tests for the public activity streams API. + +This module tests the contents of the various public activity streams: +use activity streams, dataset activity streams, group activity streams, etc. + +This module _does not_ test the private user dashboard activity stream (which +is different because the contents depend on what the user is following), that +is tested in test_dashboard.py. + +''' import datetime import logging logger = logging.getLogger(__name__) @@ -163,12 +173,6 @@ def setup_class(self): 'apikey': normal_user.apikey, 'name': normal_user.name, } - follower = model.User.get('tester') - self.follower = { - 'id': follower.id, - 'apikey': follower.apikey, - 'name': follower.name, - } warandpeace = model.Package.get('warandpeace') self.warandpeace = { 'id': warandpeace.id, @@ -177,54 +181,14 @@ def setup_class(self): self.annakarenina = { 'id': annakarenina.id, } - self.users = [self.sysadmin_user, self.normal_user, self.follower] + self.users = [self.sysadmin_user, self.normal_user] self.app = paste.fixture.TestApp(pylons.test.pylonsapp) - # Make follower follow everything else. - params = {'id': 'testsysadmin'} - extra_environ = {'Authorization': str(self.follower['apikey'])} - response = self.app.post('/api/action/follow_user', - params=json.dumps(params), extra_environ=extra_environ).json - assert response['success'] is True - params = {'id': 'annafan'} - extra_environ = {'Authorization': str(self.follower['apikey'])} - response = self.app.post('/api/action/follow_user', - params=json.dumps(params), extra_environ=extra_environ).json - assert response['success'] is True - params = {'id': 'warandpeace'} - extra_environ = {'Authorization': str(self.follower['apikey'])} - response = self.app.post('/api/action/follow_dataset', - params=json.dumps(params), extra_environ=extra_environ).json - assert response['success'] is True - params = {'id': 'annakarenina'} - extra_environ = {'Authorization': str(self.follower['apikey'])} - response = self.app.post('/api/action/follow_dataset', - params=json.dumps(params), extra_environ=extra_environ).json - assert response['success'] is True - - self.followees = \ - [ - self.sysadmin_user['id'], - self.normal_user['id'], - self.follower['id'], - self.warandpeace['id'], - self.annakarenina['id'] - ] - @classmethod def teardown_class(self): import ckan.model as model model.repo.rebuild_db() - def dashboard_activity_stream(self, apikey): - - response = self.app.get("/api/action/dashboard_activity_list", - json.dumps({}), - extra_environ={'Authorization': str(apikey)}) - response_dict = json.loads(response.body) - assert response_dict['success'] is True - return response_dict['result'] - def user_activity_stream(self, user_id, apikey=None): if apikey: extra_environ = {'Authorization': str(apikey)} @@ -290,28 +254,9 @@ def record_details(self, user_id, package_id=None, group_ids=None, details['recently changed datasets stream'] = \ self.recently_changed_datasets_stream(apikey) - details['user dashboard activity stream'] = ( - self.dashboard_activity_stream(apikey)) - - details['follower dashboard activity stream'] = ( - self.dashboard_activity_stream(self.follower['apikey'])) - details['time'] = datetime.datetime.now() return details - def check_dashboards(self, before, after, activity): - new_activities = [activity_ for activity_ in - after['user dashboard activity stream'] - if activity_ not in before['user dashboard activity stream']] - assert [activity['id'] for activity in new_activities] == [ - activity['id']] - - new_activities = [activity_ for activity_ in - after['follower dashboard activity stream'] - if activity_ not in before['follower dashboard activity stream']] - assert [activity['id'] for activity in new_activities] == [ - activity['id']] - def _create_package(self, user, name=None): if user: user_id = user['id'] @@ -358,31 +303,6 @@ def _create_package(self, user, name=None): after['recently changed datasets stream']) assert new_rcd_activities == [activity] - # The new activity should appear in the user's dashboard activity - # stream. - new_activities = [activity_ for activity_ in - after['user dashboard activity stream'] - if activity_ not in before['user dashboard activity stream']] - # There will be other new activities besides the 'follow dataset' one - # because all the dataset's old activities appear in the user's - # dashboard when she starts to follow the dataset. - assert activity['id'] in [ - activity['id'] for activity in new_activities] - - # The new activity should appear in the user "follower"'s dashboard - # activity stream because she follows all the other users and datasets. - new_activities = [activity_ for activity_ in - after['follower dashboard activity stream'] - if activity_ not in before['follower dashboard activity stream']] - # There will be other new activities besides the 'follow dataset' one - # because all the dataset's old activities appear in the user's - # dashboard when she starts to follow the dataset. - assert [activity['id'] for activity in new_activities] == [ - activity['id']] - - # The same new activity should appear on the dashboard's of the user's - # followers. - # The same new activity should appear in the activity streams of the # package's groups. for group_dict in package_created['groups']: @@ -497,8 +417,6 @@ def _add_resource(self, package, user): after['group activity streams'][group_dict['name']]) assert grp_new_activities == [activity] - self.check_dashboards(before, after, activity) - # Check that the new activity has the right attributes. assert activity['object_id'] == updated_package['id'], \ str(activity['object_id']) @@ -589,8 +507,6 @@ def _delete_extra(self, package_dict, user): after['group activity streams'][group_dict['name']]) assert grp_new_activities == [activity] - self.check_dashboards(before, after, activity) - # Check that the new activity has the right attributes. assert activity['object_id'] == updated_package['id'], \ str(activity['object_id']) @@ -679,8 +595,6 @@ def _update_extra(self, package_dict, user): after['recently changed datasets stream']) \ == user_new_activities - self.check_dashboards(before, after, activity) - # If the package has any groups, the same new activity should appear # in the activity stream of each group. for group_dict in package_dict['groups']: @@ -783,8 +697,6 @@ def _add_extra(self, package_dict, user, key=None): after['group activity streams'][group_dict['name']]) assert grp_new_activities == [activity] - self.check_dashboards(before, after, activity) - # Check that the new activity has the right attributes. assert activity['object_id'] == updated_package['id'], \ str(activity['object_id']) @@ -845,8 +757,6 @@ def _create_activity(self, user, package, params): after['package activity stream'])) assert pkg_new_activities == user_new_activities - self.check_dashboards(before, after, activity) - # Check that the new activity has the right attributes. assert activity['object_id'] == params['object_id'], ( str(activity['object_id'])) @@ -892,8 +802,6 @@ def _delete_group(self, group, user): new_activities, ("The same activity should also " "appear in the group's activity stream.") - self.check_dashboards(before, after, activity) - # Check that the new activity has the right attributes. assert activity['object_id'] == group['id'], str(activity['object_id']) assert activity['user_id'] == user['id'], str(activity['user_id']) @@ -937,8 +845,6 @@ def _update_group(self, group, user): new_activities, ("The same activity should also " "appear in the group's activity stream.") - self.check_dashboards(before, after, activity) - # Check that the new activity has the right attributes. assert activity['object_id'] == group['id'], str(activity['object_id']) assert activity['user_id'] == user['id'], str(activity['user_id']) @@ -986,8 +892,6 @@ def _update_user(self, user): "the user's activity stream, but found %i" % len(new_activities)) activity = new_activities[0] - self.check_dashboards(before, after, activity) - # Check that the new activity has the right attributes. assert activity['object_id'] == user_dict['id'], ( str(activity['object_id'])) @@ -1054,8 +958,6 @@ def _delete_resources(self, package): after['group activity streams'][group_dict['name']]) assert grp_new_activities == [activity] - self.check_dashboards(before, after, activity) - # Check that the new activity has the right attributes. assert activity['object_id'] == package['id'], ( str(activity['object_id'])) @@ -1132,8 +1034,6 @@ def _update_package(self, package, user): after['recently changed datasets stream']) \ == user_new_activities - self.check_dashboards(before, after, activity) - # If the package has any groups, the same new activity should appear # in the activity stream of each group. for group_dict in package['groups']: @@ -1211,8 +1111,6 @@ def _update_resource(self, package, resource, user): after['recently changed datasets stream']) \ == user_new_activities - self.check_dashboards(before, after, activity) - # If the package has any groups, the same new activity should appear # in the activity stream of each group. for group_dict in package['groups']: @@ -1289,8 +1187,6 @@ def _delete_package(self, package): after['recently changed datasets stream']) \ == user_new_activities - self.check_dashboards(before, after, activity) - # If the package has any groups, the same new activity should appear # in the activity stream of each group. for group_dict in package['groups']: @@ -1406,8 +1302,6 @@ def test_01_remove_tag(self): after['recently changed datasets stream']) \ == user_new_activities - self.check_dashboards(before, after, activity) - # If the package has any groups, the same new activity should appear # in the activity stream of each group. for group_dict in pkg_dict['groups']: @@ -1618,8 +1512,6 @@ def test_create_group(self): new_activities, ("The same activity should also appear in " "the group's activity stream.") - self.check_dashboards(before, after, activity) - # Check that the new activity has the right attributes. assert activity['object_id'] == group_created['id'], \ str(activity['object_id']) @@ -1692,8 +1584,6 @@ def test_add_tag(self): after['recently changed datasets stream']) \ == user_new_activities - self.check_dashboards(before, after, activity) - # If the package has any groups, the same new activity should appear # in the activity stream of each group. for group_dict in pkg_dict['groups']: @@ -2119,28 +2009,6 @@ def test_follow_dataset(self): for activity in user_new_activities: assert activity in pkg_new_activities - # The new activity should appear in the user's dashboard activity - # stream. - new_activities = [activity_ for activity_ in - after['user dashboard activity stream'] - if activity_ not in before['user dashboard activity stream']] - # There will be other new activities besides the 'follow dataset' one - # because all the dataset's old activities appear in the user's - # dashboard when she starts to follow the dataset. - assert activity['id'] in [ - activity['id'] for activity in new_activities] - - # The new activity should appear in the user "follower"'s dashboard - # activity stream because she follows all the other users and datasets. - new_activities = [activity_ for activity_ in - after['follower dashboard activity stream'] - if activity_ not in before['follower dashboard activity stream']] - # There will be other new activities besides the 'follow dataset' one - # because all the dataset's old activities appear in the user's - # dashboard when she starts to follow the dataset. - assert [activity['id'] for activity in new_activities] == [ - activity['id']] - # Check that the new activity has the right attributes. assert activity['object_id'] == self.warandpeace['id'], \ str(activity['object_id']) @@ -2182,16 +2050,6 @@ def test_follow_user(self): len(user_new_activities)) activity = user_new_activities[0] - # Check that the new activity appears in the user's private activity - # stream. - user_new_activities = (find_new_activities( - before['follower dashboard activity stream'], - after['follower dashboard activity stream'])) - assert len(user_new_activities) == 1, ("There should be 1 new " - " activity in the user's activity stream, but found %i" % - len(user_new_activities)) - assert user_new_activities[0]['id'] == activity['id'] - # Check that the new activity has the right attributes. assert activity['object_id'] == self.sysadmin_user['id'], \ str(activity['object_id']) diff --git a/ckan/tests/functional/api/test_dashboard.py b/ckan/tests/functional/api/test_dashboard.py index 1d7c291c200..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 a1b4e2bb0cc..60ee6c5e6c9 100644 --- a/ckan/tests/functional/api/test_follow.py +++ b/ckan/tests/functional/api/test_follow.py @@ -1,3 +1,15 @@ +'''Test for the follower API. + +This module tests following, unfollowing, getting a list of what you're +following or the number of things you're following, getting a list of who's +following you or the number of followers you have, testing whether or not +you're following something, etc. + +This module _does not_ test the user dashboard activity stream (which shows +activities from everything you're following), that is tested in +test_dashboard.py. + +''' import datetime import paste import pylons.test diff --git a/ckan/tests/functional/test_group.py b/ckan/tests/functional/test_group.py index 7bfc04aaadd..57e2c62574c 100644 --- a/ckan/tests/functional/test_group.py +++ b/ckan/tests/functional/test_group.py @@ -187,7 +187,7 @@ def test_index(self): groupname = 'david' group = model.Group.by_name(unicode(groupname)) group_title = group.title - group_packages_count = len(group.active_packages().all()) + group_packages_count = len(group.packages()) group_description = group.description self.check_named_element(res, 'tr', group_title, group_packages_count, @@ -321,7 +321,7 @@ def test_2_edit(self): assert group.description == newdesc, group # now look at datasets - assert len(group.active_packages().all()) == 3 + assert len(group.packages()) == 3 def test_3_edit_form_has_new_package(self): # check for dataset in autocomplete @@ -369,7 +369,7 @@ def test_4_new_duplicate_package(self): # check package only added to the group once group = model.Group.by_name(group_name) - pkg_names = [pkg.name for pkg in group.active_packages().all()] + pkg_names = [pkg.name for pkg in group.packages()] assert_equal(pkg_names, [self.packagename]) def test_edit_plugin_hook(self): @@ -435,7 +435,7 @@ def update_group(res, name, with_pkg=True): # We have the datasets in the DB, but we should also see that many # on the group read page. - assert len(group.active_packages().all()) == 3 + assert len(group.packages()) == 3 offset = url_for(controller='group', action='read', id='newname') res = self.app.get(offset, status=200, @@ -543,9 +543,9 @@ def test_2_new(self): group = model.Group.by_name(group_name) assert group.title == group_title, group assert group.description == group_description, group - assert len(group.active_packages().all()) == 1 + assert len(group.packages()) == 1 pkg = model.Package.by_name(self.packagename) - assert group.active_packages().all() == [pkg] + assert group.packages() == [pkg] def test_3_new_duplicate_group(self): prefix = '' @@ -702,7 +702,7 @@ def test_index(self): groupname = 'david' group = model.Group.by_name(unicode(groupname)) group_title = group.title - group_packages_count = len(group.active_packages().all()) + group_packages_count = len(group.packages()) group_description = group.description self.check_named_element(res, 'tr', group_title, group_packages_count, @@ -820,7 +820,7 @@ def test_2_edit(self): assert group.description == newdesc, group # now look at datasets - assert len(group.active_packages().all()) == 3 + assert len(group.packages()) == 3 def test_3_edit_form_has_new_package(self): # check for dataset in autocomplete @@ -868,7 +868,7 @@ def test_4_new_duplicate_package(self): # check package only added to the group once group = model.Group.by_name(group_name) - pkg_names = [pkg.name for pkg in group.active_packages().all()] + pkg_names = [pkg.name for pkg in group.packages()] assert_equal(pkg_names, [self.packagename]) def test_edit_plugin_hook(self): diff --git a/ckan/tests/lib/test_dictization_schema.py b/ckan/tests/lib/test_dictization_schema.py index ad9fa290cf7..ef1391b98b8 100644 --- a/ckan/tests/lib/test_dictization_schema.py +++ b/ckan/tests/lib/test_dictization_schema.py @@ -148,7 +148,7 @@ def test_2_group_schema(self): del data['extras'] converted_data, errors = validate(data, default_group_schema(), self.context) - group_pack = sorted(group.active_packages().all(), key=lambda x:x.id) + group_pack = sorted(group.packages(), key=lambda x:x.id) converted_data["packages"] = sorted(converted_data["packages"], key=lambda x:x["id"]) diff --git a/ckan/tests/models/test_group.py b/ckan/tests/models/test_group.py index 703b63778f8..424e8f3c198 100644 --- a/ckan/tests/models/test_group.py +++ b/ckan/tests/models/test_group.py @@ -25,7 +25,7 @@ def test_1_basic(self): grp = model.Group.by_name(u'group1') assert grp.title == u'Test Group' assert grp.description == u'This is a test group' - assert grp.active_packages().all() == [] + assert grp.packages() == [] def test_2_add_packages(self): model.repo.new_revision() @@ -50,7 +50,7 @@ def test_2_add_packages(self): assert grp.title == u'Russian Group' anna = model.Package.by_name(u'annakarenina') war = model.Package.by_name(u'warandpeace') - assert set(grp.active_packages().all()) == set((anna, war)), grp.active_packages().all() + assert set(grp.packages()) == set((anna, war)), grp.packages() assert grp in anna.get_groups() def test_3_search(self):