diff --git a/ckan/lib/activity.py b/ckan/lib/activity.py index 2f7622e1175..30c0beca892 100644 --- a/ckan/lib/activity.py +++ b/ckan/lib/activity.py @@ -2,9 +2,9 @@ import logging logger = logging.getLogger(__name__) -def activity_stream_item(obj, activity_type, revision_id): +def activity_stream_item(obj, activity_type, revision): try: - return obj.activity_stream_item(activity_type, revision_id) + return obj.activity_stream_item(activity_type, revision) except (AttributeError, TypeError): logger.debug("Object did not have a suitable " "activity_stream_item() method, it must not be a package.") @@ -46,7 +46,7 @@ def before_commit(self, session): logger.debug("Looking for new packages...") for obj in obj_cache['new']: logger.debug("Looking at object %s" % obj) - activity = activity_stream_item(obj, 'new', revision.id) + activity = activity_stream_item(obj, 'new', revision) if activity is None: continue # If the object returns an activity stream item we know that the @@ -86,7 +86,7 @@ def before_commit(self, session): activity = activities[package.id] else: activity = activity_stream_item(package, "changed", - revision.id) + revision) activities[package.id] = activity assert activity is not None logger.debug("activity: %s" % activity) diff --git a/ckan/model/package.py b/ckan/model/package.py index ed559ca2362..cce0f953c27 100644 --- a/ckan/model/package.py +++ b/ckan/model/package.py @@ -537,30 +537,18 @@ def get_fields(core_only=False, fields_to_ignore=None): return fields - def activity_stream_item(self, activity_type, revision_id): - try: - user_id = c.user_obj.id - except TypeError: - # Cannot access user ID through Pylons context, try to get their IP - # address instead. - try: - user_id = request.environ.get('REMOTE_ADDR', 'Unknown IP Address') - except TypeError: - # Cannot access user IP address through Pylons request, - # fallback on a default value. - user_id = 'Unknown IP Address' - logger.debug("user_id: %s" % user_id) + def activity_stream_item(self, activity_type, revision): assert activity_type in ("new", "changed", "deleted"), \ str(activity_type) + user_id = revision.user.id if activity_type == "new": - return Activity(user_id, self.id, revision_id, "new package", - None) + return Activity(user_id, self.id, revision.id, "new package", None) elif activity_type == "changed": - return Activity(user_id, self.id, revision_id, "changed package", - None) + return Activity(user_id, self.id, revision.id, "changed package", + None) elif activity_type == "deleted": - return Activity(user_id, self.id, revision_id, "deleted package", - None) + return Activity(user_id, self.id, revision.id, "deleted package", + None) def activity_stream_detail(self, activity_id, activity_type): return ActivityDetail(activity_id, self.id, u"Package", activity_type, diff --git a/ckan/tests/models/test_activity.py b/ckan/tests/models/test_activity.py index 36af160c2ed..45159c7e9f6 100644 --- a/ckan/tests/models/test_activity.py +++ b/ckan/tests/models/test_activity.py @@ -96,7 +96,8 @@ def test_create_package(self): activity = activities[-1] assert activity.object_id == package_created['id'], \ str(activity.object_id) - assert activity.user_id == "Unknown IP Address", str(activity.user_id) + assert activity.user_id == TestActivity.normal_user.id, \ + str(activity.user_id) assert activity.activity_type == 'new package', \ str(activity.activity_type) if not activity.id: @@ -170,7 +171,8 @@ def _add_resource(self, package): activity = activities[-1] assert activity.object_id == updated_package['id'], \ str(activity.object_id) - assert activity.user_id == "Unknown IP Address", str(activity.user_id) + assert activity.user_id == TestActivity.normal_user.id, \ + str(activity.user_id) assert activity.activity_type == 'changed package', \ str(activity.activity_type) if not activity.id: @@ -240,7 +242,8 @@ def _update_package(self, package): len(activities))) activity = activities[-1] assert activity.object_id == package.id, str(activity.object_id) - assert activity.user_id == "Unknown IP Address", str(activity.user_id) + assert activity.user_id == TestActivity.normal_user.id, \ + str(activity.user_id) assert activity.activity_type == 'changed package', \ str(activity.activity_type) if not activity.id: @@ -305,7 +308,8 @@ def _update_resource(self, package, resource): assert len(activities) == length_before + 1, str(activities) activity = activities[-1] assert activity.object_id == package.id, str(activity.object_id) - assert activity.user_id == "Unknown IP Address", str(activity.user_id) + assert activity.user_id == TestActivity.normal_user.id, \ + str(activity.user_id) assert activity.activity_type == 'changed package', \ str(activity.activity_type) if not activity.id: @@ -371,7 +375,8 @@ def _delete_package(self, package): len(activities))) activity = activities[-1] assert activity.object_id == package.id, str(activity.object_id) - assert activity.user_id == "Unknown IP Address", str(activity.user_id) + assert activity.user_id == TestActivity.normal_user.id, \ + str(activity.user_id) # "Deleted" packages actually show up as changed (the package's status # changes to "deleted" but the package is not expunged). assert activity.activity_type == 'changed package', \ @@ -441,7 +446,8 @@ def _delete_resources(self, package): len(activities))) activity = activities[-1] assert activity.object_id == package.id, str(activity.object_id) - assert activity.user_id == "Unknown IP Address", str(activity.user_id) + assert activity.user_id == TestActivity.normal_user.id, \ + str(activity.user_id) assert activity.activity_type == 'changed package', \ str(activity.activity_type) if not activity.id: