Ticket #7614: 7614-4.diff

src/bp-groups/bp-groups-functions.php

@@ -481 +481 @@
 function groups_is_valid_status( $status ) {
         $bp = buddypress();
 
-        return in_array( $status, (array) $bp->groups->valid_status );
+        return in_array( $status, (array) $bp->groups->valid_status, true );
 }
 
 /**
@@ -515 +515 @@
 }
 
 /**
- * Get a group slug by its ID.
+ * Get slug from a group.
  *
  * @since 1.0.0
+ * @since 10.0.0 Updated to use `bp_get_group`.
  *
- * @param int $group_id The numeric ID of the group.
- * @return string The group's slug.
+ * @param int|string|BP_Groups_Group $group The Group ID, the Group Slug or the Group object.
+ * @return bool|string The group's slug. False if group doesn't exist.
  */
-function groups_get_slug( $group_id ) {
-        $group = groups_get_group( $group_id );
-        return !empty( $group->slug ) ? $group->slug : '';
+function groups_get_slug( $group ) {
+
+        $group = bp_get_group( $group );
+
+        if ( empty( $group->id ) ) {
+                return false;
+        }
+
+        return ! empty( $group->slug ) ? $group->slug : '';
 }
 
 /**
@@ -557 +564 @@
  * Remove a user from a group.
  *
  * @since 1.0.0
+ * @since 10.0.0 Updated to use `bp_get_group`.
  *
- * @param int $group_id ID of the group.
- * @param int $user_id  Optional. ID of the user. Defaults to the currently
- *                      logged-in user.
+ * @param int|string|BP_Groups_Group $group   The Group ID, the Group Slug or the Group object.
+ * @param int                        $user_id Optional. ID of the user. Defaults to the currently
+ *                                            logged-in user.
  * @return bool True on success, false on failure.
  */
-function groups_leave_group( $group_id, $user_id = 0 ) {
+function groups_leave_group( $group, $user_id = 0 ) {
 
-        if ( empty( $user_id ) )
+        $current_group = null;
+
+        if ( is_numeric( $group ) ) {
+                $current_group = groups_get_current_group();
+
+                if ( $current_group instanceof BP_Groups_Group && (int) $current_group->id === (int) $group ) {
+                        $group = $current_group;
+                }
+        }
+
+        if ( ! isset( $group->id ) ) {
+                $group = bp_get_group( $group );
+        }
+
+        if ( empty( $group->id ) ) {
+                return false;
+        }
+
+        if ( empty( $user_id ) ) {
                 $user_id = bp_loggedin_user_id();
+        }
 
         // Don't let single admins leave the group.
-        if ( count( groups_get_group_admins( $group_id ) ) < 2 ) {
-                if ( groups_is_user_admin( $user_id, $group_id ) ) {
+        if ( count( groups_get_group_admins( $group->id ) ) < 2 ) {
+                if ( groups_is_user_admin( $user_id, $group->id ) ) {
                         bp_core_add_message( __( 'As the only admin, you cannot leave the group.', 'buddypress' ), 'error' );
                         return false;
                 }
         }
 
-        if ( ! BP_Groups_Member::delete( $user_id, $group_id ) ) {
+        if ( ! BP_Groups_Member::delete( $user_id, $group->id ) ) {
                 return false;
         }
 
@@ -586 +613 @@
          * Fires after a user leaves a group.
          *
          * @since 1.0.0
+         * @since 10.0.0 Updated to add the `$group` parameter.
          *
-         * @param int $group_id ID of the group.
-         * @param int $user_id  ID of the user leaving the group.
+         * @param int             $group_id ID of the group.
+         * @param int             $user_id  ID of the user leaving the group.
+         * @param BP_Groups_Group $group    The group object.
          */
-        do_action( 'groups_leave_group', $group_id, $user_id );
+        do_action( 'groups_leave_group', $group->id, $user_id, $group );
 
         return true;
 }
@@ -599 +628 @@
  * Add a user to a group.
  *
  * @since 1.0.0
+ * @since 10.0.0 Updated to use `bp_get_group`.
  *
- * @param int $group_id ID of the group.
- * @param int $user_id  Optional. ID of the user. Defaults to the currently
- *                      logged-in user.
+ * @param int|string|BP_Groups_Group $group   The Group ID, the Group Slug or the Group object.
+ * @param int                        $user_id Optional. ID of the user. Defaults to the currently
+ *                                            logged-in user.
  * @return bool True on success, false on failure.
  */
-function groups_join_group( $group_id, $user_id = 0 ) {
+function groups_join_group( $group, $user_id = 0 ) {
 
-        if ( empty( $user_id ) )
+        $current_group = null;
+
+        if ( is_numeric( $group ) ) {
+                $current_group = groups_get_current_group();
+
+                if ( $current_group instanceof BP_Groups_Group && (int) $current_group->id === (int) $group ) {
+                        $group = $current_group;
+                }
+        }
+
+        if ( ! isset( $group->id ) ) {
+                $group = bp_get_group( $group );
+        }
+
+        if ( empty( $group->id ) ) {
+                return false;
+        }
+
+        $group_id = $group->id;
+
+        if ( empty( $user_id ) ) {
                 $user_id = bp_loggedin_user_id();
+        }
 
         // Check if the user has an outstanding invite. If so, delete it.
-        if ( groups_check_user_has_invite( $user_id, $group_id ) )
+        if ( groups_check_user_has_invite( $user_id, $group_id ) ) {
                 groups_delete_invite( $user_id, $group_id );
+        }
 
         // Check if the user has an outstanding request. If so, delete it.
-        if ( groups_check_for_membership_request( $user_id, $group_id ) )
+        if ( groups_check_for_membership_request( $user_id, $group_id ) ) {
                 groups_delete_membership_request( null, $user_id, $group_id );
+        }
 
         // User is already a member, just return true.
-        if ( groups_is_user_member( $user_id, $group_id ) )
+        if ( groups_is_user_member( $user_id, $group_id ) ) {
                 return true;
+        }
 
-        $new_member                = new BP_Groups_Member;
+        $new_member                = new BP_Groups_Member();
         $new_member->group_id      = $group_id;
         $new_member->user_id       = $user_id;
         $new_member->inviter_id    = 0;
@@ -631 +685 @@
         $new_member->date_modified = bp_core_current_time();
         $new_member->is_confirmed  = 1;
 
-        if ( !$new_member->save() )
+        if ( ! $new_member->save() ) {
                 return false;
-
-        $bp = buddypress();
-
-        if ( !isset( $bp->groups->current_group ) || !$bp->groups->current_group || $group_id != $bp->groups->current_group->id )
-                $group = groups_get_group( $group_id );
-        else
-                $group = $bp->groups->current_group;
+        }
 
         // Record this in activity streams.
         if ( bp_is_active( 'activity' ) ) {
-                groups_record_activity( array(
-                        'type'    => 'joined_group',
-                        'item_id' => $group_id,
-                        'user_id' => $user_id,
-                ) );
+                groups_record_activity(
+                        array(
+                                'type'    => 'joined_group',
+                                'item_id' => $group_id,
+                                'user_id' => $user_id,
+                        )
+                );
         }
 
         /**
          * Fires after a user joins a group.
          *
          * @since 1.0.0
+         * @since 10.0.0 Added the `$group` parameter.
          *
-         * @param int $group_id ID of the group.
-         * @param int $user_id  ID of the user joining the group.
+         * @param int             $group_id ID of the group.
+         * @param int             $user_id  ID of the user joining the group.
+         * @param BP_Groups_Group $group    The group object.
          */
-        do_action( 'groups_join_group', $group_id, $user_id );
+        do_action( 'groups_join_group', $group_id, $user_id, $group );
 
         return true;
 }
@@ -667 +719 @@
  * Update the last_activity meta value for a given group.
  *
  * @since 1.0.0
+ * @since 10.0.0 Updated to use `bp_get_group`.
  *
- * @param int $group_id Optional. The ID of the group whose last_activity is
- *                      being updated. Default: the current group's ID.
- * @return false|null False on failure.
+ * @param int|string|BP_Groups_Group $group The Group ID, the Group Slug or the Group object.
+ *                                          Default: the current group's ID.
+ * @return bool False on failure.
  */
-function groups_update_last_activity( $group_id = 0 ) {
+function groups_update_last_activity( $group = 0 ) {
 
-        if ( empty( $group_id ) ) {
-                $group_id = buddypress()->groups->current_group->id;
+        $current_group = null;
+
+        if ( is_numeric( $group ) ) {
+                $group_id      = (int) $group;
+                $current_group = groups_get_current_group();
+
+                if ( $current_group instanceof BP_Groups_Group && (int) $current_group->id === $group_id ) {
+                        $group = $current_group;
+                }
         }
 
-        if ( empty( $group_id ) ) {
+        if ( ! isset( $group->id ) ) {
+                $group = bp_get_group( $group );
+        }
+
+        if ( empty( $group->id ) ) {
                 return false;
         }
 
-        groups_update_groupmeta( $group_id, 'last_activity', bp_core_current_time() );
+        groups_update_groupmeta( $group->id, 'last_activity', bp_core_current_time() );
 }
-add_action( 'groups_join_group',           'groups_update_last_activity' );
-add_action( 'groups_leave_group',          'groups_update_last_activity' );
-add_action( 'groups_created_group',        'groups_update_last_activity' );
+add_action( 'groups_join_group', 'groups_update_last_activity' );
+add_action( 'groups_leave_group', 'groups_update_last_activity' );
+add_action( 'groups_created_group', 'groups_update_last_activity' );
 
 /** General Group Functions ***************************************************/
 
@@ -823 +887 @@
  * Get the member count for a group.
  *
  * @since 1.2.3
+ * @since 10.0.0 Updated to use `BP_Groups_Group::get_total_member_count` and `bp_get_group`.
  *
- * @param int $group_id Group ID.
- * @return int Count of confirmed members for the group.
+ * @param int|string|BP_Groups_Group $group      The Group ID, the Group Slug or the Group object.
+ * @param bool                       $skip_cache Optional. Skip grabbing from cache. Defaults to false.
+ * @return int|bool Count of confirmed members for the group. False if group doesn't exist.
  */
-function groups_get_total_member_count( $group_id ) {
-        return BP_Groups_Group::get_total_member_count( $group_id );
+function groups_get_total_member_count( $group, $skip_cache = false ) {
+
+        $group = bp_get_group( $group );
+
+        if ( empty( $group->id ) ) {
+                return false;
+        }
+
+        return (int) BP_Groups_Group::get_total_member_count( $group->id, (bool) $skip_cache );
 }
 
 /** Group Fetching, Filtering & Searching  ************************************/
@@ -918 +991 @@
  * Get the total group count for the site.
  *
  * @since 1.2.0
+ * @since 10.0.0 Added the `$skip_cache` parameter.
  *
+ * @param bool $skip_cache Optional. Skip getting count from cache. Defaults to false.
  * @return int
  */
-function groups_get_total_group_count() {
-        $count = wp_cache_get( 'bp_total_group_count', 'bp' );
-
-        if ( false === $count ) {
-                $count = BP_Groups_Group::get_total_group_count();
-                wp_cache_set( 'bp_total_group_count', $count, 'bp' );
-        }
-
-        return $count;
+function groups_get_total_group_count( $skip_cache = false ) {
+        return (int) BP_Groups_Group::get_total_group_count( $skip_cache );
 }
 
 /**
@@ -949 +1017 @@
  */
 function groups_get_user_groups( $user_id = 0, $pag_num = 0, $pag_page = 0 ) {
 
-        if ( empty( $user_id ) )
+        if ( empty( $user_id ) ) {
                 $user_id = bp_displayed_user_id();
+        }
 
         return BP_Groups_Member::get_group_ids( $user_id, $pag_num, $pag_page );
 }

src/bp-groups/classes/class-bp-groups-group.php

@@ -1723 +1723 @@
          * bp_current_user_can( 'bp_moderate' ).
          *
          * @since 1.6.0
+         * @since 10.0.0 Added the `$skip_cache` parameter.
          *
-         * @return int Group count.
+         * @global BuddyPress $bp   The one true BuddyPress instance.
+         * @global wpdb       $wpdb WordPress database object.
+         *
+         * @param bool $skip_cache Optional. Skip getting count from cache. Defaults to false.
+         * @return int
          */
-        public static function get_total_group_count() {
+        public static function get_total_group_count( $skip_cache = false ) {
                 global $wpdb;
 
-                $hidden_sql = '';
-                if ( !bp_current_user_can( 'bp_moderate' ) )
-                        $hidden_sql = "WHERE status != 'hidden'";
+                $cache_key = 'bp_total_group_count';
+                $count     = wp_cache_get( $cache_key, 'bp' );
 
-                $bp = buddypress();
+                if ( false === $count || true === $skip_cache ) {
+                        $hidden_sql = '';
+                        if ( ! bp_current_user_can( 'bp_moderate' ) ) {
+                                $hidden_sql = "WHERE status != 'hidden'";
+                        }
+
+                        $bp    = buddypress();
+                        $count = $wpdb->get_var( "SELECT COUNT(id) FROM {$bp->groups->table_name} {$hidden_sql}" );
+
+                        wp_cache_set( $cache_key, (int) $count, 'bp' );
+                }
 
-                return $wpdb->get_var( "SELECT COUNT(id) FROM {$bp->groups->table_name} {$hidden_sql}" );
+                /**
+                 * Filters the total group count.
+                 *
+                 * @since 10.0.0
+                 *
+                 * @param int $count Total group count.
+                 */
+                return (int) apply_filters( 'bp_groups_total_group_count', (int) $count );
         }
 
         /**
          * Get the member count for a group.
          *
          * @since 1.6.0
+         * @since 10.0.0 Updated to use the `groups_get_group_members`.
          *
-         * @param int $group_id Group ID.
+         * @param int  $group_id   Group ID.
+         * @param bool $skip_cache Optional. Skip getting count from cache. Defaults to false.
          * @return int Count of confirmed members for the group.
          */
-        public static function get_total_member_count( $group_id ) {
-                global $wpdb;
+        public static function get_total_member_count( $group_id, $skip_cache = false ) {
+                $cache_key = 'total_member_count';
+                $count     = groups_get_groupmeta( $group_id, $cache_key );
 
-                $bp = buddypress();
+                if ( false === $count || true === $skip_cache ) {
+                        $members = groups_get_group_members(
+                                array(
+                                        'group_id'            => $group_id,
+                                        'exclude_banned'      => true,
+                                        'exclude_admins_mods' => false,
+                                        'type'                => 'active',
+                                )
+                        );
+
+                        $count = $members['count'] ? $members['count'] : 0;
 
-                return $wpdb->get_var( $wpdb->prepare( "SELECT COUNT(id) FROM {$bp->groups->table_name_members} WHERE group_id = %d AND is_confirmed = 1 AND is_banned = 0", $group_id ) );
+                        groups_update_groupmeta( $group_id, $cache_key, (int) $count );
+                }
+
+                /**
+                 * Filters the total member count for a group.
+                 *
+                 * @since 10.0.0
+                 *
+                 * @param int $count    Total member count for group.
+                 * @param int $group_id The ID of the group.
+                 */
+                return (int) apply_filters( 'bp_groups_total_member_count', (int) $count, (int) $group_id );
         }
 
         /**

src/bp-groups/classes/class-bp-groups-list-table.php

@@ -701 +701 @@
          * Markup for the Number of Members column.
          *
          * @since 1.7.0
+         * @since 10.0.0 Updated to use `groups_get_total_member_count`.
          *
          * @param array $item Information about the current row.
          */
         public function column_members( $item = array() ) {
-                $count = groups_get_groupmeta( $item['id'], 'total_member_count' );
+                $count = groups_get_total_member_count( absint( $item['id'] ) );
 
                 /**
                  * Filters the markup for the number of Members column.

src/bp-groups/classes/class-bp-groups-member.php

@@ -465 +465 @@
         /** Static Methods ****************************************************/
 
         /**
-         * Refresh the total_group_count for a user.
+         * Refresh the `total_group_count` for a user.
          *
          * @since 1.8.0
          *
          * @param int $user_id ID of the user.
-         * @return bool True on success, false on failure.
          */
         public static function refresh_total_group_count_for_user( $user_id ) {
-                return bp_update_user_meta( $user_id, 'total_group_count', (int) self::total_group_count( $user_id ) );
+                bp_update_user_meta( $user_id, 'total_group_count', (int) self::total_group_count( $user_id ) );
         }
 
         /**
-         * Refresh the total_member_count for a group.
+         * Refresh the `total_member_count` for a group.
+         *
+         * The request skip the current cache so that we always grab the lastest total count.
          *
          * @since 1.8.0
+         * @since 10.0.0 Updated to use `BP_Groups_Group::get_total_member_count`
          *
          * @param int $group_id ID of the group.
-         * @return bool|int True on success, false on failure.
          */
         public static function refresh_total_member_count_for_group( $group_id ) {
-                return groups_update_groupmeta( $group_id, 'total_member_count', (int) BP_Groups_Group::get_total_member_count( $group_id ) );
+                BP_Groups_Group::get_total_member_count( $group_id, true );
         }
 
         /**
@@ -495 +496 @@
          *
          * @param int $user_id  ID of the user.
          * @param int $group_id ID of the group.
-         * @return True on success, false on failure.
+         * @return bool True on success, false on failure.
          */
         public static function delete( $user_id, $group_id ) {
                 global $wpdb;
@@ -510 +511 @@
                  */
                 do_action( 'bp_groups_member_before_delete', $user_id, $group_id );
 
-                $bp = buddypress();
+                $bp     = buddypress();
                 $remove = $wpdb->query( $wpdb->prepare( "DELETE FROM {$bp->groups->table_name_members} WHERE user_id = %d AND group_id = %d", $user_id, $group_id ) );
 
                 // Update the user's group count.
@@ -529 +530 @@
                  */
                 do_action( 'bp_groups_member_after_delete', $user_id, $group_id );
 
-                return $remove;
+                return (bool) $remove;
         }
 
         /**

src/bp-groups/screens/single/members.php

@@ -14 +14 @@
  */
 function groups_screen_group_members() {
 
-        if ( !bp_is_single_item() )
+        if ( ! bp_is_single_item() ) {
                 return false;
+        }
 
         $bp = buddypress();
 
-        // Refresh the group member count meta.
-        groups_update_groupmeta( $bp->groups->current_group->id, 'total_member_count', groups_get_total_member_count( $bp->groups->current_group->id ) );
-
         /**
          * Fires before the loading of a group's Members page.
          *
@@ -39 +37 @@
          * @param string $value Path to a group's Members template.
          */
         bp_core_load_template( apply_filters( 'groups_template_group_members', 'groups/single/home' ) );
-}
- No newline at end of file
+}

tests/phpunit/testcases/groups/cache.php

@@ -259 +259 @@
                 // check if function references cache or hits the DB by comparing query count
                 $this->assertEquals( $first_query_count, $wpdb->num_queries );
         }
+
+        /**
+         * @group groups_get_total_group_count
+         * @group counts
+         */
+        public function test_total_groups_count() {
+                $u1 = self::factory()->user->create();
+                $u2 = self::factory()->user->create();
+                $u3 = self::factory()->user->create();
+                self::factory()->group->create( array( 'creator_id' => $u1 ) );
+                self::factory()->group->create( array( 'creator_id' => $u2 ) );
+
+                $this->assertEquals( 2, groups_get_total_group_count() );
+                $this->assertEquals( 2, BP_Groups_Group::get_total_group_count() );
+
+                self::factory()->group->create( array( 'creator_id' => $u3 ) );
+
+                $this->assertEquals( 3, groups_get_total_group_count( true ) );
+                $this->assertEquals( 3, BP_Groups_Group::get_total_group_count() );
+        }
 }

tests/phpunit/testcases/groups/functions.php

@@ -184 +184 @@
                 $g = self::factory()->group->create( array( 'creator_id' => $u1 ) );
 
                 groups_join_group( $g, $u2 );
-                $this->assertEquals( 2, groups_get_groupmeta( $g, 'total_member_count' ) );
+                $this->assertEquals( 2, groups_get_total_member_count( $g ) );
+        }
+
+        /**
+         * @group total_member_count
+         */
+        public function test_total_member_count_with_invalid_group() {
+                $this->assertFalse( groups_get_total_member_count( 'invalid-group' ) );
+                $this->assertFalse( groups_get_total_member_count( '' ) );
+                $this->assertFalse( groups_get_total_member_count( 123456789 ) );
         }
 
         /**
@@ -193 +202 @@
          */
         public function test_total_member_count_groups_leave_group() {
                 $u1 = self::factory()->user->create();
+                $u2 = self::factory()->user->create();
                 $g1 = self::factory()->group->create( array( 'creator_id' => $u1 ) );
-                groups_join_group( $g1, $u1 );
 
-                groups_leave_group( $g1, $u1 );
-                $this->assertEquals( 1, groups_get_groupmeta( $g1, 'total_member_count' ) );
+                groups_join_group( $g1, $u2 );
+
+                $this->assertEquals( 2, groups_get_total_member_count( $g1 ) );
+
+                groups_leave_group( $g1, $u2 );
+
+                $this->assertEquals( 1, groups_get_total_member_count( $g1 ) );
         }
 
         /**
@@ -214 +228 @@
                 $this->set_current_user( $u1 );
                 buddypress()->is_item_admin = true;
 
+                $this->assertEquals( 2, groups_get_total_member_count( $g1 ) );
+
                 groups_ban_member( $u2, $g1 );
 
-                $this->assertEquals( 1, groups_get_groupmeta( $g1, 'total_member_count' ) );
+                $this->assertEquals( 1, groups_get_total_member_count( $g1 ) );
         }
 
         /**
@@ -235 +251 @@
 
                 groups_ban_member( $u2, $g1 );
 
+                $this->assertEquals( 1, groups_get_total_member_count( $g1 ) );
+
                 groups_unban_member( $u2, $g1 );
 
-                $this->assertEquals( 2, groups_get_groupmeta( $g1, 'total_member_count' ) );
+                $this->assertEquals( 2, groups_get_total_member_count( $g1 ) );
         }
 
         /**
@@ -255 +273 @@
                         'send_invite' => 1,
                 ) );
 
+                $this->assertEquals( 1, groups_get_total_member_count( $g ) );
+
                 groups_accept_invite( $u2, $g );
 
-                $this->assertEquals( 2, groups_get_groupmeta( $g, 'total_member_count' ) );
+                $this->assertEquals( 2, groups_get_total_member_count( $g ) );
         }
 
         /**
@@ -275 +295 @@
                 ) );
                 groups_accept_membership_request( 0, $u2, $g );
 
-                $this->assertEquals( 2, groups_get_groupmeta( $g, 'total_member_count' ) );
+                $this->assertEquals( 2, groups_get_total_member_count( $g ) );
         }
 
         /**
@@ -294 +314 @@
 
                 groups_remove_member( $u2, $g1 );
 
-                $this->assertEquals( 1, groups_get_groupmeta( $g1, 'total_member_count' ) );
+                $this->assertEquals( 1, groups_get_total_member_count( $g1 ));
+        }
+
+        /**
+         * @group total_member_count
+         * @group groups_remove_member
+         */
+        public function test_total_member_count_groups_delete_member() {
+                $u1 = self::factory()->user->create();
+                $u2 = self::factory()->user->create();
+                $u3 = self::factory()->user->create();
+                $g1 = self::factory()->group->create( array( 'creator_id' => $u1 ) );
+
+                groups_join_group( $g1, $u2 );
+                groups_join_group( $g1, $u3 );
+
+                $this->set_current_user( $u1 );
+
+                $this->assertEquals( 3, groups_get_total_member_count( $g1 ) );
+                $this->assertEquals( 3, BP_Groups_Group::get_total_member_count( $g1 ) );
+
+                // Delete user.
+                wp_delete_user( $u2 );
+
+                $this->assertEquals( 2, groups_get_total_member_count( $g1 ) );
+                $this->assertEquals( 2, BP_Groups_Group::get_total_member_count( $g1 ) );
         }
 
         /**
@@ -313 +358 @@
                         'date_created' => bp_core_current_time(),
                 ) );
 
-                $this->assertEquals( 1, groups_get_groupmeta( $g, 'total_member_count' ) );
+                $this->assertEquals( 1, groups_get_total_member_count( $g ) );
         }
 
         /**