Merge pull request #4626 from ckan/activity-stream-fixes

Activity Stream deleted datasets fix

ckan/logic/action/delete.py

@@ -10,6 +10,7 @@
 import ckan.logic
 import ckan.logic.action
 import ckan.plugins as plugins
+import ckan.lib.dictization as dictization
 import ckan.lib.dictization.model_dictize as model_dictize
 from ckan import authz
 
@@ -346,7 +347,7 @@ def _group_or_org_delete(context, data_dict, is_org=False):
     else:
         _check_access('group_delete', context, data_dict)
 
-    # organization delete will not occure whilke all datasets for that org are
+    # organization delete will not occur while all datasets for that org are
     # not deleted
     if is_org:
         datasets = model.Session.query(model.Package) \
@@ -381,6 +382,28 @@ def _group_or_org_delete(context, data_dict, is_org=False):
 
     group.delete()
 
+    if is_org:
+        activity_type = 'deleted organization'
+    else:
+        activity_type = 'deleted group'
+
+    activity_dict = {
+        'user_id': model.User.by_name(user.decode('utf8')).id,
+        'object_id': group.id,
+        'activity_type': activity_type,
+        'data': {
+            'group': dictization.table_dictize(group, context)
+            }
+    }
+    activity_create_context = {
+        'model': model,
+        'user': user,
+        'defer_commit': True,
+        'ignore_auth': True,
+        'session': context['session']
+    }
+    _get_action('activity_create')(activity_create_context, activity_dict)
+
     if is_org:
         plugin_type = plugins.IOrganizationController
     else:

ckan/logic/action/get.py

@@ -2633,7 +2633,7 @@ def organization_activity_list(context, data_dict):
     org_show = logic.get_action('organization_show')
     org_id = org_show(context, {'id': org_id})['id']
 
-    _activity_objects = model.activity.group_activity_list(
+    _activity_objects = model.activity.organization_activity_list(
         org_id, limit=limit, offset=offset)
     activity_objects = _filter_activity_by_user(
         _activity_objects, _activity_stream_get_filtered_users())

ckan/logic/action/update.py

@@ -563,7 +563,8 @@ def _group_or_org_update(context, data_dict, is_org=False):
             activity_dict = None
         else:
             # We will emit a 'deleted group' activity.
-            activity_dict['activity_type'] = 'deleted group'
+            activity_dict['activity_type'] = \
+                'deleted organization' if is_org else 'deleted group'
     if activity_dict is not None:
         activity_dict['data'] = {
                 'group': dictization.table_dictize(group, context)

ckan/model/activity.py

@@ -204,14 +204,12 @@ def _group_activity_query(group_id):
         model.Member,
         and_(
             model.Activity.object_id == model.Member.table_id,
-            model.Member.state == 'active'
         )
     ).outerjoin(
         model.Package,
         and_(
             model.Package.id == model.Member.table_id,
             model.Package.private == False,
-            model.Package.state == 'active'
         )
     ).filter(
         # We only care about activity either on the the group itself or on
@@ -220,8 +218,55 @@ def _group_activity_query(group_id):
         # to a group but was then removed will not show up. This may not be
         # desired but is consistent with legacy behaviour.
         or_(
-            model.Member.group_id == group_id,
-            model.Activity.object_id == group_id
+            # active dataset in the group
+            and_(model.Member.group_id == group_id,
+                 model.Member.state == 'active',
+                 model.Package.state == 'active'),
+            # deleted dataset in the group
+            and_(model.Member.group_id == group_id,
+                 model.Member.state == 'deleted',
+                 model.Package.state == 'deleted'),
+                 # (we want to avoid showing changes to an active dataset that
+                 # was once in this group)
+            # activity the the group itself
+            model.Activity.object_id == group_id,
+        )
+    )
+
+    return q
+
+
+def _organization_activity_query(org_id):
+    '''Return an SQLAlchemy query for all activities about org_id.
+
+    Returns a query for all activities whose object is either the org itself
+    or one of the org's datasets.
+
+    '''
+    import ckan.model as model
+
+    org = model.Group.get(org_id)
+    if not org or not org.is_organization:
+        # Return a query with no results.
+        return model.Session.query(model.Activity).filter(text('0=1'))
+
+    q = model.Session.query(
+        model.Activity
+    ).outerjoin(
+        model.Package,
+        and_(
+            model.Package.id == model.Activity.object_id,
+            model.Package.private == False,
+        )
+    ).filter(
+        # We only care about activity either on the the org itself or on
+        # packages within that org.
+        # FIXME: This means that activity that occured while a package belonged
+        # to a org but was then removed will not show up. This may not be
+        # desired but is consistent with legacy behaviour.
+        or_(
+            model.Package.owner_org == org_id,
+            model.Activity.object_id == org_id
         )
     )
 
@@ -243,6 +288,21 @@ def group_activity_list(group_id, limit, offset):
     return _activities_at_offset(q, limit, offset)
 
 
+def organization_activity_list(group_id, limit, offset):
+    '''Return the given org's public activity stream.
+
+    Returns activities where the given org or one of its datasets is the
+    object of the activity, e.g.:
+
+    "{USER} updated the organization {ORG}"
+    "{USER} updated the dataset {DATASET}"
+    etc.
+
+    '''
+    q = _organization_activity_query(group_id)
+    return _activities_at_offset(q, limit, offset)
+
+
 def _activities_from_users_followed_by_user_query(user_id, limit):
     '''Return a query for all activities from users that user_id follows.'''
     import ckan.model as model

ckan/tests/controllers/test_group.py

@@ -758,3 +758,146 @@ def test_group_index(self):
             assert_in('Test Group {0}'.format(i), response)
 
         assert 'Test Group 20' not in response
+
+
+class TestActivity(helpers.FunctionalTestBase):
+    def test_simple(self):
+        '''Checking the template shows the activity stream.'''
+        app = self._get_test_app()
+        user = factories.User()
+        group = factories.Group(user=user)
+
+        url = url_for('group.activity',
+                      id=group['id'])
+        response = app.get(url)
+        assert_in('Mr. Test User', response)
+        assert_in('created the group', response)
+
+    def test_create_group(self):
+        app = self._get_test_app()
+        user = factories.User()
+        group = factories.Group(user=user)
+
+        url = url_for('group.activity',
+                      id=group['id'])
+        response = app.get(url)
+        assert_in('<a href="/user/{}">Mr. Test User'.format(user['name']),
+                  response)
+        assert_in('created the group', response)
+        assert_in('<a href="/group/{}">Test Group'.format(
+                  group['name']), response)
+
+    def _clear_activities(self):
+        model.Session.query(model.ActivityDetail).delete()
+        model.Session.query(model.Activity).delete()
+        model.Session.flush()
+
+    def test_change_group(self):
+        app = self._get_test_app()
+        user = factories.User()
+        group = factories.Group(user=user)
+        self._clear_activities()
+        group['title'] = 'Group with changed title'
+        helpers.call_action(
+            'group_update', context={'user': user['name']}, **group)
+
+        url = url_for('group.activity',
+                      id=group['id'])
+        response = app.get(url)
+        assert_in('<a href="/user/{}">Mr. Test User'.format(user['name']),
+                  response)
+        assert_in('updated the group', response)
+        assert_in('<a href="/group/{}">Group with changed title'
+                  .format(group['name']), response)
+
+    def test_delete_group_using_group_delete(self):
+        app = self._get_test_app()
+        user = factories.User()
+        group = factories.Group(user=user)
+        self._clear_activities()
+        helpers.call_action(
+            'group_delete', context={'user': user['name']}, **group)
+
+        url = url_for('group.activity',
+                      id=group['id'])
+        env = {'REMOTE_USER': user['name'].encode('ascii')}
+        response = app.get(url, extra_environ=env, status=404)
+        # group_delete causes the Member to state=deleted and then the user
+        # doesn't have permission to see their own deleted Group. Therefore you
+        # can't render the activity stream of that group. You'd hope that
+        # group_delete was the same as group_update state=deleted but they are
+        # not...
+
+    def test_delete_group_by_updating_state(self):
+        app = self._get_test_app()
+        user = factories.User()
+        group = factories.Group(user=user)
+        self._clear_activities()
+        group['state'] = 'deleted'
+        helpers.call_action(
+            'group_update', context={'user': user['name']}, **group)
+
+        url = url_for('group.activity',
+                      id=group['id'])
+        env = {'REMOTE_USER': user['name'].encode('ascii')}
+        response = app.get(url, extra_environ=env)
+        assert_in('<a href="/user/{}">Mr. Test User'.format(user['name']),
+                  response)
+        assert_in('deleted the group', response)
+        assert_in('<a href="/group/{}">Test Group'
+                  .format(group['name']), response)
+
+    def test_create_dataset(self):
+        app = self._get_test_app()
+        user = factories.User()
+        group = factories.Group(user=user)
+        self._clear_activities()
+        dataset = factories.Dataset(groups=[{'id': group['id']}], user=user)
+
+        url = url_for('group.activity',
+                      id=group['id'])
+        response = app.get(url)
+        assert_in('<a href="/user/{}">Mr. Test User'.format(user['name']),
+                  response)
+        assert_in('created the dataset', response)
+        assert_in('<a href="/dataset/{}">Test Dataset'.format(dataset['name']),
+                  response)
+
+    def test_change_dataset(self):
+        app = self._get_test_app()
+        user = factories.User()
+        group = factories.Group(user=user)
+        dataset = factories.Dataset(groups=[{'id': group['id']}], user=user)
+        self._clear_activities()
+        dataset['title'] = 'Dataset with changed title'
+        helpers.call_action(
+            'package_update', context={'user': user['name']}, **dataset)
+
+        url = url_for('group.activity',
+                      id=group['id'])
+        response = app.get(url)
+        assert_in('<a href="/user/{}">Mr. Test User'.format(user['name']),
+                  response)
+        assert_in('updated the dataset', response)
+        assert_in('<a href="/dataset/{}">Dataset with changed title'
+                  .format(dataset['name']),
+                  response)
+
+    def test_delete_dataset(self):
+        app = self._get_test_app()
+        user = factories.User()
+        group = factories.Group(user=user)
+        dataset = factories.Dataset(groups=[{'id': group['id']}], user=user)
+        self._clear_activities()
+        helpers.call_action(
+            'package_delete', context={'user': user['name']}, **dataset)
+
+        url = url_for('group.activity',
+                      id=group['id'])
+        response = app.get(url)
+        assert_in('<a href="/user/{}">Mr. Test User'.format(user['name']),
+                  response)
+        assert_in('deleted the dataset', response)
+        assert_in('<a href="/dataset/{}">Test Dataset'
+                  .format(dataset['name']),
+                  response)