eg, "Ezra posted a discussion ABCD into group X" but not "Groups X, Y and Z".

I'm not sure if Activity log actually supports displaying different updates or grouping to display all groups the node is posted into.
I asked for clarification at : http://drupal.org/node/1244930#comment-4893990

Support from Acquia helps fund testing for Drupal Acquia logo

Comments

ezra-g’s picture

Issue tags: -Commons release 2.0

This is not a release 2.0 blocker per batsonjay.

IceCreamYou’s picture

For reference, in activity_log.rules.inc we are currently doing something like this:

$og = og_determine_context_get_group($node, $account);

What we need to do is this:

$ogs = empty($node->og_groups) ? og_get_node_groups($node) : $node->og_groups;

Then we need to make the callbacks in activity_log.entity_groups.inc aware of this.

I think we also need to have the (single) active group available for some of the callbacks, but I'm not sure.

lightsurge’s picture

Do I understand right then that as an interim measure, a node could be logged against multiple group streams... (such that in every group a node is posted into, it would create a log entry in the /group stream in a similar form to which it currently does, but in all associated groups rather than just the first)...

But that logging a 'grouped' message such as

"Ezra posted a discussion ABCD into group X" but not "Groups X, Y and Z".

against the /user streams is more complex and would be better postponed till after the architecture change?

That sounds great to me, if the activity is in the group streams that's much better than nothing.

ezra-g’s picture

Component: Code » Activity/status streams
IceCreamYou’s picture

Actually, showing e.g. "Ezra posted a discussion into groups X, Y, and Z" isn't very hard either. We'd just have to add a token for showing multiple groups.

lightsurge’s picture

Sounds good although might we not end up with the same can of worms as #1293414: stream view paginates on total number of messages in a users log, even if a viewing user has no access to see some in terms of disclosing closed/private group names to people who don't/shouldn't know about them?

IceCreamYou’s picture

No, that's different.

lightsurge’s picture

Version: » 6.x-2.x-dev

@IceCreamYou is there any particular reason we can't use $event->target-id in _activity_log_get_active_group_members() rather than doing the work in activity_log.rules.inc?

I was starting to look at this at least in terms of logging into the streams of all users in all groups a node belongs to, and then planned to use a token to filter down the multiple groups appearing in the messages to the groups the viewing user has access to, maybe by just using og_node_groups_distinguish().

Then I realised it's going to get messy for example when:

  • Viewing a node, it's going to just display the first group as the breadcrumb currently I suppose, which is going to be a bit confusing
  • I was about to a continue a list here, but I've forgotten the others...pending!
function _activity_log_get_active_group_members($event, $admin = FALSE) {
  $node = node_load($event->target_id);
  if (empty($node)) {
    return array();
  }
  $uids = array();
  foreach ($node->og_groups as $group_nid) {
    if (!is_null($uids_to_merge)) {
      $uids = array_merge($uids, $uids_to_merge);
    }
    $uids_to_merge = array();
    $query = "SELECT uid FROM {og_uid} WHERE nid = %d";
    if ($admin) {
      $query .= " AND is_admin = 1";
    }
    $result = db_query($query, $group_nid);
    while ($account = db_fetch_object($result)) {
      $uids_to_merge[$account->uid] = $account->uid;
    }
  }
  return $uids;
}
lightsurge’s picture

Thought I might as well post as far as I got... Basically this does as above then adds a token to build a themed list of group links, just a simple quickfix to solve my problem for the interim (a lot of my users have figured out that cross-posting doesn't put items into the feeds or digests of users except in the first group... so instead they've started re-posting, meaning duplicate items in feeds on a large scale!).

This hasn't even been much tested by me, and I think it's not taking the most optimized direction, so use at your own risk! But in case it helps somebody.

lightsurge’s picture

Forgot to make it still work if a node isn't cross-posted :-D

lightsurge’s picture

Status: Active » Needs review
IceCreamYou’s picture

Status: Needs review » Needs work

Thanks for getting this rolling.

is there any particular reason we can't use $event->target-id in _activity_log_get_active_group_members() rather than doing the work in activity_log.rules.inc?

Less code. Then you only have to load the node in a single place instead of everywhere you want to use it.

Also, I think there are cases where $node->og_groups hasn't been set yet, in which case we need to explicitly find it:

$ogs = empty($node->og_groups) ? og_get_node_groups($node) : $node->og_groups;

Again, it's easier to do this in one place than to remember to do it everywhere you need it.

+++ b/activity_log.entity_groups.inc
@@ -723,18 +727,27 @@ function activity_log_eg_not_stream_owner($event, $settings) {
-  $query = "SELECT uid FROM {og_uid} WHERE is_active = 1 AND nid = %d";
-  if ($admin) {
-    $query .= " AND is_admin = 1";
-  }
-  $result = db_query($query, $node->nid);
   $uids = array();
-  while ($account = db_fetch_object($result)) {
-    $uids[] = $account->uid;
+  foreach ($node->og_groups as $group_nid) {
+    if (!is_null($uids_to_merge)) {
+      $uids = array_merge($uids, $uids_to_merge);
+    }
+    $uids_to_merge = array();
+    $query = "SELECT uid FROM {og_uid} WHERE nid = %d AND is_active = 1 AND uid != %d";
+    if ($admin) {
+      $query .= " AND is_admin = 1";
+    }
+    $result = db_query($query, $group_nid, $event->acting_uid);
+    while ($account = db_fetch_object($result)) {
+      $uids_to_merge[$account->uid] = $account->uid;
+    }
+    if (count($node->og_groups) == 1) {
+      return $uids_to_merge;
+    }

All you had to do was change the query from nid = %d to nid IN (". db_placeholders($node->og_groups) .").

+++ b/activity_log.module
@@ -492,6 +493,30 @@ function activity_log_token_values($type, $object = NULL, $options = array()) {
+    $group_links = array();
+    foreach ($gids_both as $gid => $name) {
+      $group_links[] = l($name, url('node/'. $gid, array('absolute' => TRUE)));
+    }
+    $n_groups = count($group_links);
+    $n_extra_groups = $n_groups > 4 ? $n_groups - 4 : NULL;
+    $iterate = $n_groups - $n_extra_groups;
+    for ($i = 0; $i < $iterate; $i++) {
+      $groups_themed .= $group_links[$i];
+      if ($n_groups > 2 && $i < ($iterate - 1)) {
+        $groups_themed .= ', ';
+      }
+      else if (($n_groups == 2 && $i == 0) || ($n_groups == 4 && $i == 2)) {
+        $groups_themed .= " and ";
+      }
+      else if (!is_null($n_extra_groups) && $i == 3) {
+        $groups_themed .= $n_extra_groups == 1 ? " and 1 other" : " and $n_extra_groups others";
+      }
+    }    ¶
+    $values['ogs-grouped'] = $groups_themed;

Instead of all this mess, use theme_activity_log_collapse($collection = array(), $method = 'activity_log_collapse_inline').

This patch also needs to remove the code in activity_log.rules.inc that sets $event->og, and remove all corresponding uses of it in activity_log.entity_groups.inc.

lightsurge’s picture

Thanks @IceCreamYou!

I was rattling around trying to work out how to deal with the case of private groups... then noticed the code was commented out, can this be properly removed now, or sticking around for a reason?


function activity_log_eg_acting_user_rels($event, $settings, $acting_uid) {
  if (module_exists('user_relationships_api') && $acting_uid && $rels = _activity_log_get_user_relationships($acting_uid)) {
    /*
    if (module_exists('og') && $node = $event->og) {
      if ($node->og_private) {
        $members = _activity_log_get_active_group_members($event);
        $rels = array_intersect($rels, $members);
      }
    }
     */
    if ($rels) {
      return array('user' => $rels);
    }
  }
}

lightsurge’s picture

Oh I remember now, this commit http://drupalcode.org/project/activity_log.git/commit/79ae53b

but which also causes the paging issue, which I'm guessing is why the code is still there, in case access goes back in here

IceCreamYou’s picture

Yeah access control should already be taken care of through the view filtering.

rgchi’s picture

Unfortunately the patch failed for me on a dev box. Of course I'm using Drupal Commons (Drupal core 6.26 with Drupal Commons 6.x-2.4).

$> sudo patch < activity_logs_cross_posting-1256754-10.patch

patching file activity_log.entity_groups.inc
Hunk #1 succeeded at 237 (offset -78 lines).
Hunk #2 succeeded at 544 (offset -183 lines).
patching file activity_log.module
Hunk #1 succeeded at 439 with fuzz 2 (offset -29 lines).
Hunk #2 succeeded at 461 with fuzz 1 (offset -32 lines).
patching file activity_log.rules_defaults.inc
Hunk #1 FAILED at 694.
Hunk #2 FAILED at 995.
Hunk #3 FAILED at 1168.
3 out of 3 hunks FAILED -- saving rejects to file activity_log.rules_defaults.inc.rej

I restored a back up so things are working.

(Fuller explanation of my issue, possibly related to this: http://drupal.stackexchange.com/questions/39532/in-activity-stream-how-i...

lightsurge’s picture

@richardgoodrow this patch was unfinished, so you shouldn't use it, and Commons is moving away from using Activity Log for Commons 7.x, so this seems unlikely to happen in this thread. You should post an issue in the activity log project