Hello,

While coding my site, I ran across something that I'd like to propose for Heartbeat. I found this because I created a new stream to show messages created by a related user, and also messages that were addressed to the related user. (How Facebook shows a wall-to-wall message in their News Feed.) It was necessary to change the code below to get it to work.

Inside of /includes/heartbeatstream.inc, there is a block that starts on line 556:

    // Include messages where the actor is somehow connected to the viewer.
      $this->viewer->relations = heartbeat_related_uids($this->viewer->uid);
      $or->condition(db_and()
        ->where($access_expression . " >= " .HEARTBEAT_PUBLIC_TO_CONNECTED)
        ->condition('ha.uid', $this->viewer->relations, 'IN')
    );

The comment states "actor is somehow connected", so shouldn't the relations be considered on the uid_target field also? Like this:

    // Include messages where the actor is somehow connected to the viewer.
    $this->viewer->relations = heartbeat_related_uids($this->viewer->uid);
    $or->condition(db_and()
      ->where($access_expression . " >= " .HEARTBEAT_PUBLIC_TO_CONNECTED)
      ->condition(db_or()
      				->condition('ha.uid', $this->viewer->relations, 'IN')
      				->condition('ha.uid_target', $this->viewer->relations, 'IN')
      			 )
    );

This doesn't seem to affect the default "relationsactivity" stream at all, but this is where a review is needed. Making this change allows for more flexibility in custom streams, like mentioned above in a Facebook-like "wall-to-wall" message.

Thanks for your code and your consideration

Comments

Stalski’s picture

This is totally correct I think. So I think this can not cause problems we don't want to happen. If people are connected to the viewer, that's ok I guess.
Let me think of the access related worst case scenario where the query above will pull extra messages.

- User A, UserB and UserC
- UserC is related to UserB
- UserA is the actor (uid) in a message where userB is the addressee (E.g. UserA commented some content of UserB : this is an example I chose to indicate that there can be no relation at all between UserA and UserB)
- UserC is the logged-in user and is looking at the stream of userB
- UserC now sees the message like: 'UserA said "Waw, great post", written by UserB.'
- So userC sees a comment from a guy he does not know, only because he knows userB writing the article.

So this is not bad in this example, but it might not be the required result in all cases.

To compare with Facebook again: the fact that userA "can" comment on userB's posts, is the reason why this message can be seen by userC as well, regardless the relation between userB and userC. It just means that the fact that there is no relation between userA and userB, thus the post was public to begin with.
In drupal terms this post could be protected by a group where userB and userA are member of, but not userC. Here that message would be hidden by the group access I expect.

Hmm, the new feature thus is something like "do you want to see messages of friends of friends"

The conclusion for me is generally a big benefit in flexibility, but it might not be expected behavior. In other words, it should be possible to create, clone or use such a stream type.
That stream just needs to implement/override the query method and use the code you described.

Your thoughts?

KorbenDallas’s picture

Your example is the essence of "social networking" as we know it today, and is one reason why I believe Facebook got so popular. It might seem like a small thing, but without it, we're back to early 2000's style MySpace social networking which was arguably Web1.5.

Using the code I presented, I tried to override the query method inside a custom stream I made for this purpose. But even with the exact condition above, it would not work correctly. So that's when I traced back to the original query builder in /includes/heartbeatstream.inc. Once I made the changes above, I could get a custom stream to work for "friend of friends". It ended up being this:

<?php

/**
 * @file
 * User relations activity object.
 */

/**
 * Class CustomRelationsActivity
 * Concrete class to build a stream with activity messages for all
 * users that are connected to the viewed user.
 */
class CustomRelationsActivity extends HeartbeatStream {

  /**
   * Implementation of queryAlter().
   */
  protected function queryAlter() {

    // This stream shows messages the "viewing" user has in relation to 
    //  his/her friends, fans, etc. Both messages created by someone in 
    //  relation, AND messages to the person in relation will show up
    $this->query->condition('ha.access', HEARTBEAT_PUBLIC_TO_ADDRESSEE, '>=');

  }

  /**
   * Function to add a part of a sql to a query built by views.
   * @param object $view
   *   The view handler object by reference to add our part to the query
   */
  protected function viewsQueryAlter(&$view) {

    // This stream shows messages the "viewing" user has in relation to 
    //  his/her friends, fans, etc. Both messages created by someone in 
    //  relation, AND messages to the person in relation will show up
    $view->query->condition('ha.access', HEARTBEAT_PUBLIC_TO_ADDRESSEE, '>=');

  }
}

Again, with the proposed code change to /includes/heartbeatstream.inc, it does not appear to break any default streams behavior. But, I didn't do extensive testing on this, I only tested the default user_activity stream, default user_relations, and then my custom one shown above.

Thanks for your consideration

Stalski’s picture

Status: Needs review » Fixed

I think I'll commit your suggestion as I totally agree with you. And I suppose, if we forgot cases where it is too dangerous, an issue will come along.
Pushed to git :)

KorbenDallas’s picture

Cool, this is my first git contribution. Thanks for including it!

Status: Fixed » Closed (fixed)

Automatically closed -- issue fixed for 2 weeks with no activity.