From eb55a659db75a21803a0a9bf66d50e6918152c87 Mon Sep 17 00:00:00 2001 From: Sean Hammond Date: Wed, 21 Nov 2012 18:32:57 +0100 Subject: [PATCH] Refactor dashboard activity stream tests - Move tests for contents of dashboard activity stream out of test_activity.py, this test module was way too long and confusing, leave it for testing the public activity streams only. Add a docstring to the module saying so. - Add a docstring to test_follow.py explaining that it tests the follower functions only (follow, unfollow, etc.) and not the contents of the dashboard activity stream that is generated from what you're folllowing. - Add new tests for the contents of the dashboard activity stream in test_dashboard.py along with other dashboard tests. Currently some of these tests are failing because activities from followed groups are not appearing in the dashboard. --- ckan/tests/functional/api/test_activity.py | 164 ++------------------ ckan/tests/functional/api/test_dashboard.py | 142 +++++++++++++++-- ckan/tests/functional/api/test_follow.py | 12 ++ 3 files changed, 148 insertions(+), 170 deletions(-) diff --git a/ckan/tests/functional/api/test_activity.py b/ckan/tests/functional/api/test_activity.py index 4f7eff2a380..dfc14f9f6c4 100644 --- a/ckan/tests/functional/api/test_activity.py +++ b/ckan/tests/functional/api/test_activity.py @@ -1,3 +1,13 @@ +'''Functional tests for the public activity streams API. + +This module tests the contents of the various public activity streams: +use activity streams, dataset activity streams, group activity streams, etc. + +This module _does not_ test the private user dashboard activity stream (which +is different because the contents depend on what the user is following), that +is tested in test_dashboard.py. + +''' import datetime import logging logger = logging.getLogger(__name__) @@ -163,12 +173,6 @@ def setup_class(self): 'apikey': normal_user.apikey, 'name': normal_user.name, } - follower = model.User.get('tester') - self.follower = { - 'id': follower.id, - 'apikey': follower.apikey, - 'name': follower.name, - } warandpeace = model.Package.get('warandpeace') self.warandpeace = { 'id': warandpeace.id, @@ -177,54 +181,14 @@ def setup_class(self): self.annakarenina = { 'id': annakarenina.id, } - self.users = [self.sysadmin_user, self.normal_user, self.follower] + self.users = [self.sysadmin_user, self.normal_user] self.app = paste.fixture.TestApp(pylons.test.pylonsapp) - # Make follower follow everything else. - params = {'id': 'testsysadmin'} - extra_environ = {'Authorization': str(self.follower['apikey'])} - response = self.app.post('/api/action/follow_user', - params=json.dumps(params), extra_environ=extra_environ).json - assert response['success'] is True - params = {'id': 'annafan'} - extra_environ = {'Authorization': str(self.follower['apikey'])} - response = self.app.post('/api/action/follow_user', - params=json.dumps(params), extra_environ=extra_environ).json - assert response['success'] is True - params = {'id': 'warandpeace'} - extra_environ = {'Authorization': str(self.follower['apikey'])} - response = self.app.post('/api/action/follow_dataset', - params=json.dumps(params), extra_environ=extra_environ).json - assert response['success'] is True - params = {'id': 'annakarenina'} - extra_environ = {'Authorization': str(self.follower['apikey'])} - response = self.app.post('/api/action/follow_dataset', - params=json.dumps(params), extra_environ=extra_environ).json - assert response['success'] is True - - self.followees = \ - [ - self.sysadmin_user['id'], - self.normal_user['id'], - self.follower['id'], - self.warandpeace['id'], - self.annakarenina['id'] - ] - @classmethod def teardown_class(self): import ckan.model as model model.repo.rebuild_db() - def dashboard_activity_stream(self, apikey): - - response = self.app.get("/api/action/dashboard_activity_list", - json.dumps({}), - extra_environ={'Authorization': str(apikey)}) - response_dict = json.loads(response.body) - assert response_dict['success'] is True - return response_dict['result'] - def user_activity_stream(self, user_id, apikey=None): if apikey: extra_environ = {'Authorization': str(apikey)} @@ -290,28 +254,9 @@ def record_details(self, user_id, package_id=None, group_ids=None, details['recently changed datasets stream'] = \ self.recently_changed_datasets_stream(apikey) - details['user dashboard activity stream'] = ( - self.dashboard_activity_stream(apikey)) - - details['follower dashboard activity stream'] = ( - self.dashboard_activity_stream(self.follower['apikey'])) - details['time'] = datetime.datetime.now() return details - def check_dashboards(self, before, after, activity): - new_activities = [activity_ for activity_ in - after['user dashboard activity stream'] - if activity_ not in before['user dashboard activity stream']] - assert [activity['id'] for activity in new_activities] == [ - activity['id']] - - new_activities = [activity_ for activity_ in - after['follower dashboard activity stream'] - if activity_ not in before['follower dashboard activity stream']] - assert [activity['id'] for activity in new_activities] == [ - activity['id']] - def _create_package(self, user, name=None): if user: user_id = user['id'] @@ -358,31 +303,6 @@ def _create_package(self, user, name=None): after['recently changed datasets stream']) assert new_rcd_activities == [activity] - # The new activity should appear in the user's dashboard activity - # stream. - new_activities = [activity_ for activity_ in - after['user dashboard activity stream'] - if activity_ not in before['user dashboard activity stream']] - # There will be other new activities besides the 'follow dataset' one - # because all the dataset's old activities appear in the user's - # dashboard when she starts to follow the dataset. - assert activity['id'] in [ - activity['id'] for activity in new_activities] - - # The new activity should appear in the user "follower"'s dashboard - # activity stream because she follows all the other users and datasets. - new_activities = [activity_ for activity_ in - after['follower dashboard activity stream'] - if activity_ not in before['follower dashboard activity stream']] - # There will be other new activities besides the 'follow dataset' one - # because all the dataset's old activities appear in the user's - # dashboard when she starts to follow the dataset. - assert [activity['id'] for activity in new_activities] == [ - activity['id']] - - # The same new activity should appear on the dashboard's of the user's - # followers. - # The same new activity should appear in the activity streams of the # package's groups. for group_dict in package_created['groups']: @@ -497,8 +417,6 @@ def _add_resource(self, package, user): after['group activity streams'][group_dict['name']]) assert grp_new_activities == [activity] - self.check_dashboards(before, after, activity) - # Check that the new activity has the right attributes. assert activity['object_id'] == updated_package['id'], \ str(activity['object_id']) @@ -589,8 +507,6 @@ def _delete_extra(self, package_dict, user): after['group activity streams'][group_dict['name']]) assert grp_new_activities == [activity] - self.check_dashboards(before, after, activity) - # Check that the new activity has the right attributes. assert activity['object_id'] == updated_package['id'], \ str(activity['object_id']) @@ -679,8 +595,6 @@ def _update_extra(self, package_dict, user): after['recently changed datasets stream']) \ == user_new_activities - self.check_dashboards(before, after, activity) - # If the package has any groups, the same new activity should appear # in the activity stream of each group. for group_dict in package_dict['groups']: @@ -783,8 +697,6 @@ def _add_extra(self, package_dict, user, key=None): after['group activity streams'][group_dict['name']]) assert grp_new_activities == [activity] - self.check_dashboards(before, after, activity) - # Check that the new activity has the right attributes. assert activity['object_id'] == updated_package['id'], \ str(activity['object_id']) @@ -845,8 +757,6 @@ def _create_activity(self, user, package, params): after['package activity stream'])) assert pkg_new_activities == user_new_activities - self.check_dashboards(before, after, activity) - # Check that the new activity has the right attributes. assert activity['object_id'] == params['object_id'], ( str(activity['object_id'])) @@ -892,8 +802,6 @@ def _delete_group(self, group, user): new_activities, ("The same activity should also " "appear in the group's activity stream.") - self.check_dashboards(before, after, activity) - # Check that the new activity has the right attributes. assert activity['object_id'] == group['id'], str(activity['object_id']) assert activity['user_id'] == user['id'], str(activity['user_id']) @@ -937,8 +845,6 @@ def _update_group(self, group, user): new_activities, ("The same activity should also " "appear in the group's activity stream.") - self.check_dashboards(before, after, activity) - # Check that the new activity has the right attributes. assert activity['object_id'] == group['id'], str(activity['object_id']) assert activity['user_id'] == user['id'], str(activity['user_id']) @@ -986,8 +892,6 @@ def _update_user(self, user): "the user's activity stream, but found %i" % len(new_activities)) activity = new_activities[0] - self.check_dashboards(before, after, activity) - # Check that the new activity has the right attributes. assert activity['object_id'] == user_dict['id'], ( str(activity['object_id'])) @@ -1054,8 +958,6 @@ def _delete_resources(self, package): after['group activity streams'][group_dict['name']]) assert grp_new_activities == [activity] - self.check_dashboards(before, after, activity) - # Check that the new activity has the right attributes. assert activity['object_id'] == package['id'], ( str(activity['object_id'])) @@ -1132,8 +1034,6 @@ def _update_package(self, package, user): after['recently changed datasets stream']) \ == user_new_activities - self.check_dashboards(before, after, activity) - # If the package has any groups, the same new activity should appear # in the activity stream of each group. for group_dict in package['groups']: @@ -1211,8 +1111,6 @@ def _update_resource(self, package, resource, user): after['recently changed datasets stream']) \ == user_new_activities - self.check_dashboards(before, after, activity) - # If the package has any groups, the same new activity should appear # in the activity stream of each group. for group_dict in package['groups']: @@ -1289,8 +1187,6 @@ def _delete_package(self, package): after['recently changed datasets stream']) \ == user_new_activities - self.check_dashboards(before, after, activity) - # If the package has any groups, the same new activity should appear # in the activity stream of each group. for group_dict in package['groups']: @@ -1406,8 +1302,6 @@ def test_01_remove_tag(self): after['recently changed datasets stream']) \ == user_new_activities - self.check_dashboards(before, after, activity) - # If the package has any groups, the same new activity should appear # in the activity stream of each group. for group_dict in pkg_dict['groups']: @@ -1618,8 +1512,6 @@ def test_create_group(self): new_activities, ("The same activity should also appear in " "the group's activity stream.") - self.check_dashboards(before, after, activity) - # Check that the new activity has the right attributes. assert activity['object_id'] == group_created['id'], \ str(activity['object_id']) @@ -1692,8 +1584,6 @@ def test_add_tag(self): after['recently changed datasets stream']) \ == user_new_activities - self.check_dashboards(before, after, activity) - # If the package has any groups, the same new activity should appear # in the activity stream of each group. for group_dict in pkg_dict['groups']: @@ -2119,28 +2009,6 @@ def test_follow_dataset(self): for activity in user_new_activities: assert activity in pkg_new_activities - # The new activity should appear in the user's dashboard activity - # stream. - new_activities = [activity_ for activity_ in - after['user dashboard activity stream'] - if activity_ not in before['user dashboard activity stream']] - # There will be other new activities besides the 'follow dataset' one - # because all the dataset's old activities appear in the user's - # dashboard when she starts to follow the dataset. - assert activity['id'] in [ - activity['id'] for activity in new_activities] - - # The new activity should appear in the user "follower"'s dashboard - # activity stream because she follows all the other users and datasets. - new_activities = [activity_ for activity_ in - after['follower dashboard activity stream'] - if activity_ not in before['follower dashboard activity stream']] - # There will be other new activities besides the 'follow dataset' one - # because all the dataset's old activities appear in the user's - # dashboard when she starts to follow the dataset. - assert [activity['id'] for activity in new_activities] == [ - activity['id']] - # Check that the new activity has the right attributes. assert activity['object_id'] == self.warandpeace['id'], \ str(activity['object_id']) @@ -2182,16 +2050,6 @@ def test_follow_user(self): len(user_new_activities)) activity = user_new_activities[0] - # Check that the new activity appears in the user's private activity - # stream. - user_new_activities = (find_new_activities( - before['follower dashboard activity stream'], - after['follower dashboard activity stream'])) - assert len(user_new_activities) == 1, ("There should be 1 new " - " activity in the user's activity stream, but found %i" % - len(user_new_activities)) - assert user_new_activities[0]['id'] == activity['id'] - # Check that the new activity has the right attributes. assert activity['object_id'] == self.sysadmin_user['id'], \ str(activity['object_id']) diff --git a/ckan/tests/functional/api/test_dashboard.py b/ckan/tests/functional/api/test_dashboard.py index 1d7c291c200..6cef63beb21 100644 --- a/ckan/tests/functional/api/test_dashboard.py +++ b/ckan/tests/functional/api/test_dashboard.py @@ -1,3 +1,10 @@ +'''Test for the dashboard API. + +This module tests the various functions of the user dashboard, such as the +contents of the dashboard activity stream and reporting the number of new +activities. + +''' import ckan from ckan.lib.helpers import json import paste @@ -92,6 +99,16 @@ def test_00_dashboard_new_activities_count_not_logged_in(self): def test_00_dashboard_mark_new_activities_not_logged_in(self): self.post('dashboard_mark_all_new_activities_as_old', status=403) + def test_01_dashboard_activity_list_for_new_user(self): + '''Test the contents of a new user's dashboard activity stream.''' + activities = self.dashboard_activity_list(self.new_user) + # We expect to find a single 'new user' activity. + assert len(activities) == 1 + activity = activities[0] + assert activity['activity_type'] == 'new user' + assert activity['user_id'] == activity['object_id'] + assert activity['user_id'] == self.new_user['id'] + def test_01_new_activities_count_for_new_user(self): '''Test that a newly registered user's new activities count is 0.''' assert self.dashboard_new_activities_count(self.new_user) == 0 @@ -151,14 +168,38 @@ def test_02_own_activities_do_not_count_as_new(self): # User's own actions should not increase her activity count. assert self.dashboard_new_activities_count(self.new_user) == 0 + def test_03_dashboard_activity_list_own_activities(self): + '''Test that a user's own activities appear in her dashboard.''' + activities = self.dashboard_activity_list(self.new_user) + + # FIXME: There should actually be 6 activities here, but when you + # follow something it's old activities (from before you followed it) + # appear in your activity stream. So here we get one more activity + # than expected. + assert len(activities) == 7 + + assert activities[0]['activity_type'] == 'changed package' + assert activities[1]['activity_type'] == 'follow group' + assert activities[2]['activity_type'] == 'follow user' + assert activities[3]['activity_type'] == 'follow dataset' + assert activities[4]['activity_type'] == 'new package' + assert activities[5]['activity_type'] == 'new user' + + # FIXME: Shouldn't need the [:6] here, it's because when you follow + # something its old activities (from before you started following it) + # appear in your dashboard. + for activity in activities[:6]: + assert activity['user_id'] == self.new_user['id'] + def test_03_own_activities_not_marked_as_new(self): '''Make a user do some activities and check that her own activities aren't marked as new in her dashboard activity stream.''' assert len(self.dashboard_new_activities(self.new_user)) == 0 - def test_04_new_activities_count(self): - '''Test that new activities from objects that a user follows increase - her new activities count.''' + def test_04_activities_from_followed_datasets(self): + '''Activities from followed datasets should show in dashboard.''' + + activities_before = self.dashboard_activity_list(self.new_user) # Make someone else who new_user is not following update a dataset that # new_user is following. @@ -167,34 +208,101 @@ def test_04_new_activities_count(self): extra_environ={'Authorization': str(self.joeadmin['apikey'])}) assert response.json['success'] is True + # Check the new activity in new_user's dashboard. + activities = self.dashboard_activity_list(self.new_user) + new_activities = [activity for activity in activities + if activity not in activities_before] + assert len(new_activities) == 1 + activity = new_activities[0] + assert activity['activity_type'] == 'changed package' + assert activity['user_id'] == self.joeadmin['id'] + assert activity['data']['package']['name'] == 'warandpeace' + + def test_04_activities_from_followed_users(self): + '''Activities from followed users should show in the dashboard.''' + + activities_before = self.dashboard_activity_list(self.new_user) + # Make someone that the user is following create a new dataset. params = json.dumps({'name': 'annas_new_dataset'}) response = self.app.post('/api/action/package_create', params=params, extra_environ={'Authorization': str(self.annafan['apikey'])}) assert response.json['success'] is True - # Make someone that the user is not following update a dataset that - # the user is not following, but that belongs to a group that the user - # is following. + # Check the new activity in new_user's dashboard. + activities = self.dashboard_activity_list(self.new_user) + new_activities = [activity for activity in activities + if activity not in activities_before] + assert len(new_activities) == 1 + activity = new_activities[0] + assert activity['activity_type'] == 'new package' + assert activity['user_id'] == self.annafan['id'] + assert activity['data']['package']['name'] == 'annas_new_dataset' + + def test_04_activities_from_followed_groups(self): + '''Activities from followed groups should show in the dashboard.''' + + activities_before = self.dashboard_activity_list(self.new_user) + + # Make someone that the user is not following update a group that the + # user is following. + params = json.dumps({'id': 'roger', 'description': 'updated'}) + response = self.app.post('/api/action/group_update', params=params, + extra_environ={'Authorization': str(self.testsysadmin['apikey'])}) + assert response.json['success'] is True + + # Check the new activity in new_user's dashboard. + activities = self.dashboard_activity_list(self.new_user) + new_activities = [activity for activity in activities + if activity not in activities_before] + assert len(new_activities) == 1 + activity = new_activities[0] + assert activity['activity_type'] == 'changed group' + assert activity['user_id'] == self.testsysadmin['id'] + assert activity['data']['group']['name'] == 'roger' + + def test_04_activities_from_datasets_of_followed_groups(self): + '''Activities from datasets of followed groups should show in the + dashboard. + + ''' + activities_before = self.dashboard_activity_list(self.new_user) + + # Make someone that the user is not following update a dataset that the + # user is not following either, but that belongs to a group that the + # user is following. params = json.dumps({'name': 'annakarenina', 'notes': 'updated'}) response = self.app.post('/api/action/package_update', params=params, extra_environ={'Authorization': str(self.testsysadmin['apikey'])}) assert response.json['success'] is True - # FIXME: The number here should be 3 but activities from followed - # groups are not appearing in dashboard. When that is fixed, fix this - # number. - assert self.dashboard_new_activities_count(self.new_user) == 2 + # Check the new activity in new_user's dashboard. + activities = self.dashboard_activity_list(self.new_user) + new_activities = [activity for activity in activities + if activity not in activities_before] + assert len(new_activities) == 1 + activity = new_activities[0] + assert activity['activity_type'] == 'changed package' + assert activity['user_id'] == self.testsysadmin['id'] + assert activity['data']['package']['name'] == 'annakarenina' + + def test_05_new_activities_count(self): + '''Test that new activities from objects that a user follows increase + her new activities count.''' + # FIXME: The number here should be 4 but activities from datasets of + # followed groups are not appearing in dashboard. When that is fixed, + # fix this number. + assert self.dashboard_new_activities_count(self.new_user) == 3 - def test_05_activities_marked_as_new(self): + def test_06_activities_marked_as_new(self): '''Test that new activities from objects that a user follows are marked as new in her dashboard activity stream.''' - # FIXME: The number here should be 3 but activities from followed - # groups are not appearing in dashboard. When that is fixed, fix this - # number. - assert len(self.dashboard_new_activities(self.new_user)) == 2 + # FIXME: The number here should be 4 but activities from datasets of + # followed groups are not appearing in dashboard. When that is fixed, + # fix this number. + assert len(self.dashboard_new_activities(self.new_user)) == 3 - def test_06_mark_new_activities_as_read(self): + def test_07_mark_new_activities_as_read(self): '''Test that a user's new activities are marked as old when she views her dashboard activity stream.''' assert self.dashboard_new_activities_count(self.new_user) > 0 @@ -203,7 +311,7 @@ def test_06_mark_new_activities_as_read(self): assert self.dashboard_new_activities_count(self.new_user) == 0 assert len(self.dashboard_new_activities(self.new_user)) == 0 - def test_07_maximum_number_of_new_activities(self): + def test_08_maximum_number_of_new_activities(self): '''Test that the new activities count does not go higher than 15, even if there are more than 15 new activities from the user's followers.''' for n in range(0, 20): diff --git a/ckan/tests/functional/api/test_follow.py b/ckan/tests/functional/api/test_follow.py index ab0df832f48..a084c930fd9 100644 --- a/ckan/tests/functional/api/test_follow.py +++ b/ckan/tests/functional/api/test_follow.py @@ -1,3 +1,15 @@ +'''Test for the follower API. + +This module tests following, unfollowing, getting a list of what you're +following or the number of things you're following, getting a list of who's +following you or the number of followers you have, testing whether or not +you're following something, etc. + +This module _does not_ test the user dashboard activity stream (which shows +activities from everything you're following), that is tested in +test_dashboard.py. + +''' import datetime import paste import pylons.test