[buddypress-trac] [BuddyPress] #4134: bug in activity filtering for join group

[buddypress-trac at lists.automattic.com]

#4134: bug in activity filtering for join group
--------------------------+--------------------
 Reporter:  gwu123        |       Owner:
     Type:  defect (bug)  |      Status:  new
 Priority:  normal        |   Milestone:  1.6
Component:  Activity      |     Version:  1.5.5
 Severity:  normal        |  Resolution:
 Keywords:                |
--------------------------+--------------------

Comment (by 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?

-- 
Ticket URL: <https://buddypress.trac.wordpress.org/ticket/4134#comment:3>
BuddyPress <http://buddypress.org/>
BuddyPress