From 60d61d894606345198989bfea20ecc347e116252 Mon Sep 17 00:00:00 2001 From: Sean Hammond Date: Mon, 5 Nov 2012 17:02:54 +0100 Subject: [PATCH 01/63] [#3009] Fix dashboard activity stream auth This is necessary groundwork for implementing #3009 (new activity notifications). Make dashboard_activity_list() return the dashboard activity stream of the logged in user, rather than accepting the user id as a param and letting anyone (even not logged in users) view anyone else's private activity streams! Lots of fixing up of test_activity.py to take account of this: --- ckan/logic/action/get.py | 21 +- ckan/tests/functional/api/test_activity.py | 238 ++++++++++++++------- 2 files changed, 170 insertions(+), 89 deletions(-) diff --git a/ckan/logic/action/get.py b/ckan/logic/action/get.py index 7bcca7b74a4..c60877734ee 100644 --- a/ckan/logic/action/get.py +++ b/ckan/logic/action/get.py @@ -2105,18 +2105,24 @@ def dataset_followee_list(context, data_dict): return [model_dictize.package_dictize(dataset, context) for dataset in datasets] def dashboard_activity_list(context, data_dict): - '''Return the dashboard activity stream of the given user. - - :param id: the id or name of the user - :type id: string + '''Return the authorized user's dashboard activity stream. :rtype: list of dictionaries ''' # FIXME: Filter out activities whose subject or object the user is not # authorized to read. + if 'user' not in context: + raise logic.NotAuthorized( + _("You must be logged in to see your dashboard activity stream.")) + model = context['model'] - user_id = _get_or_bust(data_dict, 'id') + + userobj = model.User.get(context['user']) + if not userobj: + raise logic.NotAuthorized( + _("You must be logged in to see your dashboard activity stream.")) + user_id = userobj.id activity_query = model.Session.query(model.Activity) user_followees_query = activity_query.join(model.UserFollowingUser, model.UserFollowingUser.object_id == model.Activity.user_id) @@ -2136,14 +2142,11 @@ def dashboard_activity_list(context, data_dict): return model_dictize.activity_list_dictize(activity_objects, context) def dashboard_activity_list_html(context, data_dict): - '''Return the dashboard activity stream of the given user as HTML. + '''Return the authorized user's dashboard activity stream as HTML. The activity stream is rendered as a snippet of HTML meant to be included in an HTML page, i.e. it doesn't have any HTML header or footer. - :param id: The id or name of the user. - :type id: string - :rtype: string ''' diff --git a/ckan/tests/functional/api/test_activity.py b/ckan/tests/functional/api/test_activity.py index 1126efd0484..6222ac48d02 100644 --- a/ckan/tests/functional/api/test_activity.py +++ b/ckan/tests/functional/api/test_activity.py @@ -216,10 +216,14 @@ def teardown_class(self): import ckan.model as model model.repo.rebuild_db() - def dashboard_activity_stream(self, user_id): - response = self.app.get( - "/api/2/rest/user/{0}/dashboard_activity".format(user_id)) - return json.loads(response.body) + 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: @@ -286,35 +290,41 @@ 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['id'])) + self.dashboard_activity_stream(self.follower['apikey'])) details['time'] = datetime.datetime.now() return details - def check_dashboard( - self, - before, after, wanted_difference, - potential_followees): - difference = find_new_activities( - before['follower dashboard activity stream'], - after['follower dashboard activity stream']) - if any(potential_followee in self.followees - for potential_followee in potential_followees): - assert difference == wanted_difference - else: - assert len(difference) == 0 + 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 new_activities == [activity] + + new_activities = [activity_ for activity_ in + after['follower dashboard activity stream'] + if activity_ not in before['follower dashboard activity stream']] + assert new_activities == [activity] def _create_package(self, user, name=None): if user: user_id = user['id'] + apikey = user['apikey'] else: user_id = 'not logged in' + apikey = None + + before = self.record_details(user_id, apikey=apikey) # Create a new package. request_data = make_package(name) before = self.record_details(user_id=user_id, - group_ids=[group['name'] for group in request_data['groups']]) + group_ids=[group['name'] for group in request_data['groups']], + apikey=apikey) extra_environ = {'Authorization': str(user['apikey'])} response = self.app.post('/api/action/package_create', json.dumps(request_data), extra_environ=extra_environ) @@ -324,7 +334,8 @@ def _create_package(self, user, name=None): after = self.record_details(user_id=user_id, package_id=package_created['id'], - group_ids=[group['name'] for group in package_created['groups']]) + group_ids=[group['name'] for group in package_created['groups']], + apikey=apikey) # Find the new activity in the user's activity stream. user_new_activities = (find_new_activities( @@ -345,7 +356,28 @@ def _create_package(self, user, name=None): after['recently changed datasets stream']) assert new_rcd_activities == [activity] - self.check_dashboard(before, after, user_new_activities, [user_id]) + # 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 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 new_activities == [activity] + + # 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. @@ -409,11 +441,13 @@ def _create_package(self, user, name=None): def _add_resource(self, package, user): if user: user_id = user['id'] + apikey = user['apikey'] else: user_id = 'not logged in' + apikey = None before = self.record_details(user_id, package['id'], - [group['id'] for group in package['groups']]) + [group['name'] for group in package['groups']], apikey=apikey) resource_ids_before = [resource['id'] for resource in package['resources']] @@ -424,7 +458,7 @@ def _add_resource(self, package, user): updated_package = package_update(self.app, package, user['apikey']) after = self.record_details(user_id, package['id'], - [group['id'] for group in package['groups']]) + [group['name'] for group in package['groups']], apikey=apikey) resource_ids_after = [resource['id'] for resource in updated_package['resources']] assert len(resource_ids_after) == len(resource_ids_before) + 1 @@ -458,8 +492,7 @@ def _add_resource(self, package, user): after['group activity streams'][group_dict['name']]) assert grp_new_activities == [activity] - self.check_dashboard(before, after, user_new_activities, - [user_id, package['id']]) + self.check_dashboards(before, after, activity) # Check that the new activity has the right attributes. assert activity['object_id'] == updated_package['id'], \ @@ -497,10 +530,14 @@ def _add_resource(self, package, user): def _delete_extra(self, package_dict, user): if user: user_id = user['id'] + apikey = user['apikey'] else: user_id = 'not logged in' + apikey = None - before = self.record_details(user_id, package_dict['id']) + before = self.record_details(user_id, package_dict['id'], + [group['name'] for group in package_dict['groups']], + apikey=apikey) extras_before = list(package_dict['extras']) assert len(extras_before) > 0, ( @@ -511,7 +548,9 @@ def _delete_extra(self, package_dict, user): updated_package = package_update(self.app, package_dict, user['apikey']) - after = self.record_details(user_id, package_dict['id']) + after = self.record_details(user_id, package_dict['id'], + [group['name'] for group in package_dict['groups']], + apikey=apikey) extras_after = updated_package['extras'] assert len(extras_after) == len(extras_before) - 1, ( "%s != %s" % (len(extras_after), len(extras_before) - 1)) @@ -545,8 +584,7 @@ def _delete_extra(self, package_dict, user): after['group activity streams'][group_dict['name']]) assert grp_new_activities == [activity] - self.check_dashboard(before, after, user_new_activities, - [user_id, package_dict['id']]) + self.check_dashboards(before, after, activity) # Check that the new activity has the right attributes. assert activity['object_id'] == updated_package['id'], \ @@ -585,11 +623,14 @@ def _delete_extra(self, package_dict, user): def _update_extra(self, package_dict, user): if user: user_id = user['id'] + apikey = user['apikey'] else: user_id = 'not logged in' + apikey=None before = self.record_details(user_id, package_dict['id'], - [group['name'] for group in package_dict['groups']]) + [group['name'] for group in package_dict['groups']], + apikey=apikey) extras_before = package_dict['extras'] assert len(extras_before) > 0, ( @@ -606,7 +647,8 @@ def _update_extra(self, package_dict, user): user['apikey']) after = self.record_details(user_id, package_dict['id'], - [group['name'] for group in package_dict['groups']]) + [group['name'] for group in package_dict['groups']], + apikey=apikey) extras_after = updated_package['extras'] assert len(extras_after) == len(extras_before), ( "%s != %s" % (len(extras_after), len(extras_before))) @@ -632,8 +674,7 @@ def _update_extra(self, package_dict, user): after['recently changed datasets stream']) \ == user_new_activities - self.check_dashboard(before, after, user_new_activities, - [user_id, package_dict['id']]) + self.check_dashboards(before, after, activity) # If the package has any groups, the same new activity should appear # in the activity stream of each group. @@ -682,10 +723,14 @@ def _add_extra(self, package_dict, user, key=None): key = 'quality' if user: user_id = user['id'] + apikey = user['apikey'] else: user_id = 'not logged in' + apikey = None - before = self.record_details(user_id, package_dict['id']) + before = self.record_details(user_id, package_dict['id'], + [group['name'] for group in package_dict['groups']], + apikey=apikey) # Make a copy of the package's extras before we add a new extra, # so we can compare the extras before and after updating the package. @@ -697,7 +742,9 @@ def _add_extra(self, package_dict, user, key=None): updated_package = package_update(self.app, package_dict, user['apikey']) - after = self.record_details(user_id, package_dict['id']) + after = self.record_details(user_id, package_dict['id'], + [group['name'] for group in package_dict['groups']], + apikey=apikey) extras_after = updated_package['extras'] assert len(extras_after) == len(extras_before) + 1, ( "%s != %s" % (len(extras_after), len(extras_before) + 1)) @@ -731,8 +778,7 @@ def _add_extra(self, package_dict, user, key=None): after['group activity streams'][group_dict['name']]) assert grp_new_activities == [activity] - self.check_dashboard(before, after, user_new_activities, - [user_id, package_dict['id']]) + self.check_dashboards(before, after, activity) # Check that the new activity has the right attributes. assert activity['object_id'] == updated_package['id'], \ @@ -769,14 +815,16 @@ def _add_extra(self, package_dict, user, key=None): str(detail['activity_type'])) def _create_activity(self, user, package, params): - before = self.record_details(user['id'], package['id']) + before = self.record_details(user['id'], package['id'], + apikey=user['apikey']) response = self.app.post('/api/action/activity_create', params=json.dumps(params), extra_environ={'Authorization': str(self.sysadmin_user['apikey'])}) assert response.json['success'] is True - after = self.record_details(user['id'], package['id']) + after = self.record_details(user['id'], package['id'], + apikey=user['apikey']) # Find the new activity in the user's activity stream. user_new_activities = (find_new_activities( @@ -792,8 +840,7 @@ def _create_activity(self, user, package, params): after['package activity stream'])) assert pkg_new_activities == user_new_activities - self.check_dashboard(before, after, user_new_activities, - [user['id'], package['id']]) + self.check_dashboards(before, after, activity) # Check that the new activity has the right attributes. assert activity['object_id'] == params['object_id'], ( @@ -840,7 +887,7 @@ def _delete_group(self, group, user): new_activities, ("The same activity should also " "appear in the group's activity stream.") - self.check_dashboard(before, after, new_activities, [user['id']]) + self.check_dashboards(before, after, activity) # Check that the new activity has the right attributes. assert activity['object_id'] == group['id'], str(activity['object_id']) @@ -862,13 +909,15 @@ def _update_group(self, group, user): item and detail are emitted. """ - before = self.record_details(user['id'], group_ids=[group['id']]) + before = self.record_details(user['id'], group_ids=[group['id']], + apikey=user['apikey']) # Update the group. group_dict = {'id': group['id'], 'title': 'edited'} group_update(self.app, group_dict, user['apikey']) - after = self.record_details(user['id'], group_ids=[group['id']]) + after = self.record_details(user['id'], group_ids=[group['id']], + apikey=user['apikey']) # Find the new activity. new_activities = find_new_activities(before['user activity stream'], @@ -883,7 +932,7 @@ def _update_group(self, group, user): new_activities, ("The same activity should also " "appear in the group's activity stream.") - self.check_dashboard(before, after, new_activities, [user['id']]) + self.check_dashboards(before, after, activity) # Check that the new activity has the right attributes. assert activity['object_id'] == group['id'], str(activity['object_id']) @@ -912,7 +961,8 @@ def _update_user(self, user): assert response_dict['success'] is True user_dict = response_dict['result'] - before = self.record_details(user_dict['id']) + before = self.record_details(user_dict['id'], + apikey=user_dict['apikey']) # Update the user. user_dict['about'] = 'edited' @@ -921,7 +971,8 @@ def _update_user(self, user): self.app.post('/api/action/user_update', json.dumps(user_dict), extra_environ={'Authorization': str(user['apikey'])}) - after = self.record_details(user_dict['id']) + after = self.record_details(user_dict['id'], + apikey=user_dict['apikey']) # Find the new activity. new_activities = find_new_activities(before['user activity stream'], @@ -930,7 +981,7 @@ def _update_user(self, user): "the user's activity stream, but found %i" % len(new_activities)) activity = new_activities[0] - self.check_dashboard(before, after, new_activities, [user_dict['id']]) + self.check_dashboards(before, after, activity) # Check that the new activity has the right attributes. assert activity['object_id'] == user_dict['id'], ( @@ -954,7 +1005,8 @@ def _delete_resources(self, package): """ before = self.record_details(self.normal_user['id'], package['id'], - [group['name'] for group in package['groups']]) + [group['name'] for group in package['groups']], + apikey=self.normal_user['apikey']) num_resources = len(package['resources']) assert num_resources > 0, \ @@ -965,7 +1017,8 @@ def _delete_resources(self, package): package_update(self.app, package, self.normal_user['apikey']) after = self.record_details(self.normal_user['id'], package['id'], - [group['name'] for group in package['groups']]) + [group['name'] for group in package['groups']], + apikey=self.normal_user['apikey']) # Find the new activity in the user's activity stream. user_new_activities = (find_new_activities( @@ -996,8 +1049,7 @@ def _delete_resources(self, package): after['group activity streams'][group_dict['name']]) assert grp_new_activities == [activity] - self.check_dashboard(before, after, user_new_activities, - [package['id']]) + self.check_dashboards(before, after, activity) # Check that the new activity has the right attributes. assert activity['object_id'] == package['id'], ( @@ -1037,10 +1089,12 @@ def _update_package(self, package, user): """ if user: user_id = user['id'] + apikey = user['apikey'] else: user_id = 'not logged in' + apikey = None - before = self.record_details(user_id, package['id']) + before = self.record_details(user_id, package['id'], apikey=apikey) # Update the package. if package['title'] != 'edited': @@ -1050,7 +1104,7 @@ def _update_package(self, package, user): package['title'] = 'edited again' package_update(self.app, package, user['apikey']) - after = self.record_details(user_id, package['id']) + after = self.record_details(user_id, package['id'], apikey=apikey) # Find the new activity in the user's activity stream. user_new_activities = (find_new_activities( @@ -1073,8 +1127,7 @@ def _update_package(self, package, user): after['recently changed datasets stream']) \ == user_new_activities - self.check_dashboard(before, after, user_new_activities, - [user_id, package['id']]) + self.check_dashboards(before, after, activity) # If the package has any groups, the same new activity should appear # in the activity stream of each group. @@ -1119,16 +1172,18 @@ def _update_resource(self, package, resource, user): """ if user: user_id = user['id'] + apikey = user['apikey'] else: user_id = 'not logged in' + apikey = None - before = self.record_details(user_id, package['id']) + before = self.record_details(user_id, package['id'], apikey=apikey) # Update the resource. resource['name'] = 'edited' package_update(self.app, package) - after = self.record_details(user_id, package['id']) + after = self.record_details(user_id, package['id'], apikey=apikey) # Find the new activity in the user's activity stream. user_new_activities = (find_new_activities( @@ -1151,8 +1206,7 @@ def _update_resource(self, package, resource, user): after['recently changed datasets stream']) \ == user_new_activities - self.check_dashboard(before, after, user_new_activities, - [user_id, package['id']]) + self.check_dashboards(before, after, activity) # If the package has any groups, the same new activity should appear # in the activity stream of each group. @@ -1230,8 +1284,7 @@ def _delete_package(self, package): after['recently changed datasets stream']) \ == user_new_activities - self.check_dashboard(before, after, user_new_activities, - [self.sysadmin_user['id'], package['id']]) + self.check_dashboards(before, after, activity) # If the package has any groups, the same new activity should appear # in the activity stream of each group. @@ -1316,14 +1369,16 @@ def test_01_remove_tag(self): assert len(pkg_dict['tags']) >= 1, ("The package has to have at least" " one tag to test removing a tag.") before = self.record_details(user['id'], pkg_dict['id'], - [group['name'] for group in pkg_dict['groups']]) + [group['name'] for group in pkg_dict['groups']], + apikey=user['apikey']) data_dict = { 'id': pkg_dict['id'], 'tags': pkg_dict['tags'][0:-1], } package_update(self.app, data_dict, user['apikey']) after = self.record_details(user['id'], pkg_dict['id'], - [group['name'] for group in pkg_dict['groups']]) + [group['name'] for group in pkg_dict['groups']], + apikey=user['apikey']) # Find the new activity in the user's activity stream. user_new_activities = (find_new_activities( @@ -1346,8 +1401,7 @@ def test_01_remove_tag(self): after['recently changed datasets stream']) \ == user_new_activities - self.check_dashboard(before, after, user_new_activities, - [user['id'], pkg_dict['id']]) + self.check_dashboards(before, after, activity) # If the package has any groups, the same new activity should appear # in the activity stream of each group. @@ -1491,7 +1545,8 @@ def test_create_user(self): assert response_dict['success'] is True user_created = response_dict['result'] - after = self.record_details(user_created['id']) + after = self.record_details(user_created['id'], + apikey=user_created['apikey']) user_activities = after['user activity stream'] assert len(user_activities) == 1, ("There should be 1 activity in " @@ -1533,7 +1588,7 @@ def test_create_group(self): user = self.normal_user - before = self.record_details(user['id']) + before = self.record_details(user['id'], apikey=user['apikey']) # Create a new group. request_data = {'name': 'a-new-group', 'title': 'A New Group'} @@ -1545,7 +1600,7 @@ def test_create_group(self): group_created = response_dict['result'] after = self.record_details(user['id'], - group_ids=[group_created['id']]) + group_ids=[group_created['id']], apikey=user['apikey']) # Find the new activity. new_activities = find_new_activities(before['user activity stream'], @@ -1558,7 +1613,7 @@ def test_create_group(self): new_activities, ("The same activity should also appear in " "the group's activity stream.") - self.check_dashboard(before, after, new_activities, [user['id']]) + self.check_dashboards(before, after, activity) # Check that the new activity has the right attributes. assert activity['object_id'] == group_created['id'], \ @@ -1601,13 +1656,15 @@ def test_add_tag(self): pkg_dict = package_show(self.app, {'id': pkg_name}) # Add one new tag to the package. - before = self.record_details(user['id'], pkg_dict['id']) + before = self.record_details(user['id'], pkg_dict['id'], + apikey=user['apikey']) new_tag_name = 'test tag' assert new_tag_name not in [tag['name'] for tag in pkg_dict['tags']] pkg_dict['tags'].append({'name': new_tag_name}) package_update(self.app, pkg_dict, user['apikey']) - after = self.record_details(user['id'], pkg_dict['id']) + after = self.record_details(user['id'], pkg_dict['id'], + apikey=user['apikey']) # Find the new activity in the user's activity stream. user_new_activities = (find_new_activities( @@ -1630,8 +1687,7 @@ def test_add_tag(self): after['recently changed datasets stream']) \ == user_new_activities - self.check_dashboard(before, after, user_new_activities, - [user['id'], pkg_dict['id']]) + self.check_dashboards(before, after, activity) # If the package has any groups, the same new activity should appear # in the activity stream of each group. @@ -2033,7 +2089,8 @@ def test_delete_extras(self): def test_follow_dataset(self): user = self.normal_user - before = self.record_details(user['id']) + before = self.record_details(user['id'], self.warandpeace['id'], + apikey=user['apikey']) data = {'id': self.warandpeace['id']} extra_environ = {'Authorization': str(user['apikey'])} response = self.app.post('/api/action/follow_dataset', @@ -2041,7 +2098,8 @@ def test_follow_dataset(self): response_dict = json.loads(response.body) assert response_dict['success'] is True - after = self.record_details(user['id'], self.warandpeace['id']) + after = self.record_details(user['id'], self.warandpeace['id'], + apikey=user['apikey']) # Find the new activity in the user's activity stream. user_new_activities = (find_new_activities( @@ -2056,7 +2114,25 @@ def test_follow_dataset(self): for activity in user_new_activities: assert activity in pkg_new_activities - self.check_dashboard(before, after, user_new_activities, [user['id']]) + # 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 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 new_activities == [activity] # Check that the new activity has the right attributes. assert activity['object_id'] == self.warandpeace['id'], \ @@ -2077,8 +2153,9 @@ def test_follow_dataset(self): def test_follow_user(self): user = self.normal_user - before = self.record_details(user['id']) - followee_before = self.record_details(self.sysadmin_user['id']) + before = self.record_details(user['id'], apikey=user['apikey']) + followee_before = self.record_details(self.sysadmin_user['id'], + apikey=self.sysadmin_user['apikey']) data = {'id': self.sysadmin_user['id']} extra_environ = {'Authorization': str(user['apikey'])} response = self.app.post('/api/action/follow_user', @@ -2086,8 +2163,9 @@ def test_follow_user(self): response_dict = json.loads(response.body) assert response_dict['success'] is True - after = self.record_details(user['id']) - followee_after = self.record_details(self.sysadmin_user['id']) + after = self.record_details(user['id'], apikey=user['apikey']) + followee_after = self.record_details(self.sysadmin_user['id'], + apikey=self.sysadmin_user['apikey']) # Find the new activity in the user's activity stream. user_new_activities = (find_new_activities( From 2398b28ba8162a88f2301249a7c439f8883e1e16 Mon Sep 17 00:00:00 2001 From: Sean Hammond Date: Tue, 6 Nov 2012 19:10:16 +0100 Subject: [PATCH 02/63] [#3009] Initial implementation of activity streams on-site notification API --- ckan/logic/action/get.py | 50 +++++++++++- ckan/model/__init__.py | 3 + ckan/model/dashboard.py | 47 +++++++++++ ckan/tests/functional/api/test_dashboard.py | 87 +++++++++++++++++++++ 4 files changed, 185 insertions(+), 2 deletions(-) create mode 100644 ckan/model/dashboard.py create mode 100644 ckan/tests/functional/api/test_dashboard.py diff --git a/ckan/logic/action/get.py b/ckan/logic/action/get.py index c60877734ee..0adc1bb4623 100644 --- a/ckan/logic/action/get.py +++ b/ckan/logic/action/get.py @@ -1,6 +1,7 @@ import uuid import logging import json +import datetime from pylons import config from pylons.i18n import _ @@ -2114,14 +2115,14 @@ def dashboard_activity_list(context, data_dict): # authorized to read. if 'user' not in context: raise logic.NotAuthorized( - _("You must be logged in to see your dashboard activity stream.")) + _("You must be logged in to access your dashboard.")) model = context['model'] userobj = model.User.get(context['user']) if not userobj: raise logic.NotAuthorized( - _("You must be logged in to see your dashboard activity stream.")) + _("You must be logged in to access your dashboard.")) user_id = userobj.id activity_query = model.Session.query(model.Activity) @@ -2141,6 +2142,7 @@ def dashboard_activity_list(context, data_dict): return model_dictize.activity_list_dictize(activity_objects, context) + def dashboard_activity_list_html(context, data_dict): '''Return the authorized user's dashboard activity stream as HTML. @@ -2154,6 +2156,50 @@ def dashboard_activity_list_html(context, data_dict): return activity_streams.activity_list_to_html(context, activity_stream) +def dashboard_new_activities_count(context, data_dict): + '''Return the number of new activities in the user's activity stream. + + Return the number of new activities in the authorized user's dashboard + activity stream. + + :rtype: int + + ''' + # We don't bother to do our own auth check in this function, because we + # assume dashboard_activity_list will do it. + activities = dashboard_activity_list(context, data_dict) + + model = context['model'] + user = model.User.get(context['user']) # The authorized user. + last_viewed = model.Dashboard.get_activity_stream_last_viewed(user.id) + + strptime = datetime.datetime.strptime + fmt = '%Y-%m-%dT%H:%M:%S.%f' + new_activities = [activity for activity in activities if + strptime(activity['timestamp'], fmt) > last_viewed] + return len(new_activities) + + +def dashboard_mark_activities_as_read(context, data_dict): + '''Mark all the authorized user's new dashboard activities as old. + + This will reset dashboard_new_activities_count to 0. + + ''' + if 'user' not in context: + raise logic.NotAuthorized( + _("You must be logged in to access your dashboard.")) + + model = context['model'] + + userobj = model.User.get(context['user']) + if not userobj: + raise logic.NotAuthorized( + _("You must be logged in to access your dashboard.")) + user_id = userobj.id + model.Dashboard.update_activity_stream_last_viewed(user_id) + + def _unpick_search(sort, allowed_fields=None, total=None): ''' This is a helper function that takes a sort string eg 'name asc, last_modified desc' and returns a list of diff --git a/ckan/model/__init__.py b/ckan/model/__init__.py index 98c647e74f4..6502b6b9a66 100644 --- a/ckan/model/__init__.py +++ b/ckan/model/__init__.py @@ -147,6 +147,9 @@ DomainObjectOperation, DomainObject, ) +from dashboard import ( + Dashboard, +) import ckan.migration diff --git a/ckan/model/dashboard.py b/ckan/model/dashboard.py new file mode 100644 index 00000000000..438fc865eca --- /dev/null +++ b/ckan/model/dashboard.py @@ -0,0 +1,47 @@ +import datetime +import sqlalchemy +import meta + +dashboard_table = sqlalchemy.Table('dashboard', meta.metadata, + sqlalchemy.Column('user_id', sqlalchemy.types.UnicodeText, + sqlalchemy.ForeignKey('user.id', onupdate='CASCADE', + ondelete='CASCADE'), + primary_key=True, nullable=False), + sqlalchemy.Column('activity_stream_last_viewed', sqlalchemy.types.DateTime, + nullable=False) +) + + +class Dashboard(object): + '''Saved data used for the user's dashboard.''' + + def __init__(self, user_id): + self.user_id = user_id + self.activity_stream_last_viewed = datetime.datetime.now() + + @classmethod + def get_activity_stream_last_viewed(cls, user_id): + query = meta.Session.query(Dashboard) + query = query.filter(Dashboard.user_id == user_id) + try: + row = query.one() + return row.activity_stream_last_viewed + except sqlalchemy.orm.exc.NoResultFound: + # No dashboard row has been created for this user so they have no + # activity_stream_last_viewed date. Return the oldest date we can + # (i.e. all activities are new to this user). + return datetime.datetime.min + + @classmethod + def update_activity_stream_last_viewed(cls, user_id): + query = meta.Session.query(Dashboard) + query = query.filter(Dashboard.user_id == user_id) + try: + row = query.one() + row.activity_stream_last_viewed = datetime.datetime.now() + except sqlalchemy.orm.exc.NoResultFound: + row = Dashboard(user_id) + meta.Session.add(row) + meta.Session.commit() + +meta.mapper(Dashboard, dashboard_table) diff --git a/ckan/tests/functional/api/test_dashboard.py b/ckan/tests/functional/api/test_dashboard.py new file mode 100644 index 00000000000..9cd330dd905 --- /dev/null +++ b/ckan/tests/functional/api/test_dashboard.py @@ -0,0 +1,87 @@ +import ckan +from ckan.lib.helpers import json +import paste +import pylons.test + + +class TestDashboard(object): + '''Tests for the logic action functions related to the user's dashboard.''' + + @classmethod + def setup_class(cls): + ckan.tests.CreateTestData.create() + cls.app = paste.fixture.TestApp(pylons.test.pylonsapp) + joeadmin = ckan.model.User.get('joeadmin') + cls.joeadmin = { + 'id': joeadmin.id, + 'apikey': joeadmin.apikey + } + + @classmethod + def teardown_class(cls): + ckan.model.repo.rebuild_db() + + def new_activities_count(self, user): + '''Return the given user's new activities count from the CKAN API.''' + + params = json.dumps({}) + response = self.app.post('/api/action/dashboard_new_activities_count', + params=params, + extra_environ={'Authorization': str(user['apikey'])}) + assert response.json['success'] is True + new_activities_count = response.json['result'] + return new_activities_count + + def mark_as_read(self, user): + params = json.dumps({}) + response = self.app.post( + '/api/action/dashboard_mark_activities_as_read', + params=params, + extra_environ={'Authorization': str(user['apikey'])}) + assert response.json['success'] is True + + def test_01_num_new_activities_new_user(self): + '''Test retreiving the number of new activities for a new user.''' + + # Create a new user. + params = json.dumps({ + 'name': 'mr_new_user', + 'email': 'mr@newuser.com', + 'password': 'iammrnew', + }) + response = self.app.post('/api/action/user_create', params=params, + extra_environ={'Authorization': str(self.joeadmin['apikey'])}) + assert response.json['success'] is True + new_user = response.json['result'] + + # We expect to find only one new activity for a newly registered user + # (A "{USER} signed up" activity). + assert self.new_activities_count(new_user) == 1 + + self.mark_as_read(new_user) + assert self.new_activities_count(new_user) == 0 + + # Create a dataset. + params = json.dumps({ + 'name': 'my_new_package', + }) + response = self.app.post('/api/action/package_create', params=params, + extra_environ={'Authorization': str(new_user['apikey'])}) + assert response.json['success'] is True + + # Now there should be a new 'user created dataset' activity. + assert self.new_activities_count(new_user) == 1 + + # Update the dataset. + params = json.dumps({ + 'name': 'my_new_package', + 'title': 'updated description', + }) + response = self.app.post('/api/action/package_update', params=params, + extra_environ={'Authorization': str(new_user['apikey'])}) + assert response.json['success'] is True + + assert self.new_activities_count(new_user) == 2 + + self.mark_as_read(new_user) + assert self.new_activities_count(new_user) == 0 From af1e3fc9c6eb974cd48146204c87fc8fe1e2a78c Mon Sep 17 00:00:00 2001 From: Sean Hammond Date: Tue, 6 Nov 2012 20:36:19 +0100 Subject: [PATCH 03/63] [#3009] Add c.new_activities to every page when logged in Not sure if this is the best way to do this --- ckan/lib/base.py | 10 ++++++++++ 1 file changed, 10 insertions(+) diff --git a/ckan/lib/base.py b/ckan/lib/base.py index edc4264e2f1..904c61a00bd 100644 --- a/ckan/lib/base.py +++ b/ckan/lib/base.py @@ -214,6 +214,16 @@ def __before__(self, action, **params): self._identify_user() i18n.handle_request(request, c) + # If the user is logged in add their number of new activities to the + # template context. + if c.userobj: + from ckan.logic import get_action + new_activities_count = get_action( + 'dashboard_new_activities_count') + context = {'model': model, 'session': model.Session, + 'user': c.user or c.author} + c.new_activities = new_activities_count(context, {}) + def _identify_user(self): ''' Identifies the user using two methods: From 7202c928175f8eb8e2b046102cd302aab1bacdbe Mon Sep 17 00:00:00 2001 From: Sean Hammond Date: Tue, 6 Nov 2012 20:36:57 +0100 Subject: [PATCH 04/63] [#3009] Add dashboard table migration script --- .../versions/061_add_dashboard_table.py | 16 ++++++++++++++++ 1 file changed, 16 insertions(+) create mode 100644 ckan/migration/versions/061_add_dashboard_table.py diff --git a/ckan/migration/versions/061_add_dashboard_table.py b/ckan/migration/versions/061_add_dashboard_table.py new file mode 100644 index 00000000000..f5f23c82138 --- /dev/null +++ b/ckan/migration/versions/061_add_dashboard_table.py @@ -0,0 +1,16 @@ +from sqlalchemy import * +from migrate import * + +def upgrade(migrate_engine): + metadata = MetaData() + metadata.bind = migrate_engine + migrate_engine.execute(''' +CREATE TABLE dashboard ( + user_id text NOT NULL, + activity_stream_last_viewed timestamp without time zone NOT NULL +); +ALTER TABLE dashboard + ADD CONSTRAINT dashboard_pkey PRIMARY KEY (user_id); +ALTER TABLE dashboard + ADD CONSTRAINT dashboard_user_id_fkey FOREIGN KEY (user_id) REFERENCES "user"(id) ON UPDATE CASCADE ON DELETE CASCADE; + ''') From a91b625202b4410b1237a4106cae382cc42af90d Mon Sep 17 00:00:00 2001 From: Sean Hammond Date: Tue, 6 Nov 2012 20:43:27 +0100 Subject: [PATCH 05/63] [#3009] Mark activities as read on loading dashboard page --- ckan/controllers/user.py | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/ckan/controllers/user.py b/ckan/controllers/user.py index a8543938e11..7166bfec33b 100644 --- a/ckan/controllers/user.py +++ b/ckan/controllers/user.py @@ -500,6 +500,11 @@ def dashboard(self, id=None): 'user': c.user or c.author, 'for_view': True} data_dict = {'id': id, 'user_obj': c.userobj} self._setup_template_variables(context, data_dict) + + # Mark the user's new activities as read whenever they view their + # dashboard page. + get_action('dashboard_mark_activities_as_read')(context, {}) + return render('user/dashboard.html') def follow(self, id): From f440d8d19a4bdbdd091c3a7b9ce96bcb65882800 Mon Sep 17 00:00:00 2001 From: John Martin Date: Wed, 7 Nov 2012 10:51:32 +0000 Subject: [PATCH 06/63] Simple dashboard notifications style --- ckan/public/base/less/masthead.less | 27 +++++++++++++++++++++++++-- ckan/templates/header.html | 11 +++++++++-- 2 files changed, 34 insertions(+), 4 deletions(-) diff --git a/ckan/public/base/less/masthead.less b/ckan/public/base/less/masthead.less index 59718a5f5e4..afe2e4abb6d 100644 --- a/ckan/public/base/less/masthead.less +++ b/ckan/public/base/less/masthead.less @@ -164,17 +164,30 @@ .border-radius(100px); .box-shadow(0 -2px 4px rgba(0, 0, 0, 0.3)) } - i { + .icon { position: absolute; top: 0; left: 30px; line-height: 25px; } + .notifications { + position: absolute; + top: -2px; + right: 8px; + width: 8px; + height: 8px; + background-color: #B55457; + border: 1px solid #E2B5B6; + overflow: hidden; + text-indent: -10000px; + .box-shadow(0 1px 2px rgba(0, 0, 0, 0.75)); + .border-radius(100px); + } &:hover { opacity: 1; + text-decoration: none; i { font-weight: normal; - text-decoration: none; } } } @@ -215,8 +228,18 @@ i { margin-right: 5px; } + .label { + float: right; + margin-left: 10px; + padding-right: 6px; + .border-radius(100px); + } &:hover { color: @mastheadLinkColor; + .label { + background-color: @mastheadLinkColor; + color: @layoutLinkColor; + } } } } diff --git a/ckan/templates/header.html b/ckan/templates/header.html index 9fd3d6a3727..3ee787b25bf 100644 --- a/ckan/templates/header.html +++ b/ckan/templates/header.html @@ -19,12 +19,19 @@

From 1b0d24c6352b46857c6f53b3e3350893ff38c82c Mon Sep 17 00:00:00 2001 From: John Martin Date: Wed, 7 Nov 2012 15:41:04 +0000 Subject: [PATCH 09/63] Final tweaks for notifications UI --- ckan/public/base/less/masthead.less | 17 +++++++++++++---- ckan/templates/header.html | 2 +- 2 files changed, 14 insertions(+), 5 deletions(-) diff --git a/ckan/public/base/less/masthead.less b/ckan/public/base/less/masthead.less index 5596a3a4cbe..764261b4dd6 100644 --- a/ckan/public/base/less/masthead.less +++ b/ckan/public/base/less/masthead.less @@ -1,6 +1,10 @@ @mastheadPadding: 5px 10px 3px; @notificationsBg: #C9403A; +@menuActiveBg: mix(@mastheadBackgroundColorStart, @mastheadLinkColor, 95%); +@menuActiveHi: mix(@mastheadBackgroundColorStart, @mastheadLinkColor, 85%); +@menuActiveLo: darken(@menuActiveBg, 10%); + .masthead() { .clearfix(); color: @mastheadTextColor; @@ -67,6 +71,11 @@ header.masthead { font-size: 12px; font-weight: bold; padding: 4px 10px; + .border-radius(3px); + &.active { + background-color: @menuActiveBg; + box-shadow: 0 -1px 0 @menuActiveLo, 0 1px 0 @menuActiveHi; + } } } } @@ -164,6 +173,10 @@ header.masthead { &.authed { margin: 0 0 0 30px; } + &.not-authed { + padding-top: 2px; + padding-bottom: 2px; + } } .dropdown-menu { @@ -178,10 +191,6 @@ header.masthead { } } } - .notifications-dropdown-menu { - margin-right: 4px; - min-width: 300px; - } .debug { position: absolute; diff --git a/ckan/templates/header.html b/ckan/templates/header.html index d365f412250..a86da127267 100644 --- a/ckan/templates/header.html +++ b/ckan/templates/header.html @@ -30,7 +30,7 @@

{% if c.userobj %}