bug in activity filtering for join group

[gwu123]

The activity filter has a bug when filtering “join group” activity.

Steps for recreating this scenario

1. Join a group from Group directory (not from the group home page)
2. Check the “My Groups” activity. You will see an entry for “user joined group….”
3. Now check the “All members” activity tab. This will not capture the above activity.

[DJPaul]

Confirmed. Probably a hide_sitewide issue.

[chriskeeble]

(Apologies if including code like this is not the way to make suggestions, I'm not too familiar with the process here).

When the join group button is clicked...
1) on an individual group page: the 'hide_sitewide' property for the recorded activity is set to false
2) on the Groups (list) page: the 'hide_sitewide' property for the recorded activity is set to true

In case 2 above, $bp->groups->current_group is not set (there is no current group). The function groups_join_group() handles this:

if ( !isset( $bp->groups->current_group ) || !$bp->groups->current_group || $group_id != $bp->groups->current_group->id )
    $group = new BP_Groups_Group( $group_id );
else
    $group = $bp->groups->current_group;

But then the function groups_record_activity() uses $bp->groups->current_group to determine whether to set hide_sitewide to true or false...

// If the group is not public, hide the activity sitewide.
if ( isset( $bp->groups->current_group->status ) && 'public' == $bp->groups->current_group->status )
    $hide_sitewide = false;
else
    $hide_sitewide = true;

I think the test should be using the item_id (group id) from $args, load the specified group and test its status, such as this (which works)...

function groups_record_activity( $args = '' ) {
	global $bp;

	if ( !bp_is_active( 'activity' ) )
		return false;

	// If the group being joined is specified, use it
	if ( isset( $args ) && array_key_exists( 'item_id', $args ))
		$group = new BP_Groups_Group($args['item_id']);
	else
		$group = $bp->groups->current_group;	// Otherwise it's the current group - but item_id won't be set in the activity ??

	// If the group is not public, hide the activity sitewide.
	if ( isset( $group->status ) && 'public' == $group->status )
		$hide_sitewide = false;
	else
		$hide_sitewide = true;

	$defaults = array (
		'id'                => false,
		'user_id'           => $bp->loggedin_user->id,
		'action'            => '',
		'content'           => '',
		'primary_link'      => '',
		'component'         => $bp->groups->id,
		'type'              => false,
		'item_id'           => false,
		'secondary_item_id' => false,
		'recorded_time'     => bp_core_current_time(),
		'hide_sitewide'     => $hide_sitewide
	);

	$r = wp_parse_args( $args, $defaults );
	extract( $r );

	return bp_activity_add( array( 'id' => $id, 'user_id' => $user_id, 'action' => $action, 'content' => $content, 'primary_link' => $primary_link, 'component' => $component, 'type' => $type, 'item_id' => $item_id, 'secondary_item_id' => $secondary_item_id, 'recorded_time' => $recorded_time, 'hide_sitewide' => $hide_sitewide ) );
}

BUT - I'm concerned that if item_id is ever not set explicitly in $args, then an activity will be recorded with a false (zero) item_id (although this is already the case).

Any thoughts?

[chriskeeble]

Update: I've done a check through all calls to groups_record_activity() and item_id is being set in all cases, as it should be. So may be better to always use it to grab the group for clarity & simplicity in the code.

[DJPaul]

I'd suggest that we check for $bp->groups->current_group->id == $'item_id (taking care to handle if item_id is not set, of course). If it does, we can use that group object again, which will save a database query. If you are able to make a patch, that would be helpful.

[boonebgorges]

+1 to what DJPaul said.

• Before querying for the group object, check to see whether $item_id == bp_get_current_group_id(), and if so, then use groups_get_current_group() to populate the $group object.
• Don't instantiate BP_Groups_Group directly. Use groups_get_group( array( 'group_id' => $group_id ) ) instead. (This is a new change to BP 1.6, to allow for better caching.)

[boonebgorges]

(In [6097]) Improvements to the way the hide_sitewide default is determined in groups_record_activity()

groups_record_activity() should attempt to use the item_id passed to the
function when determining the default value of 'hide_sitewide', instead of the
current group id (which will not always be populated, eg in the groups
directory).

Fixes #4134

Props chriskeeble