Ticket #8688: 8688.2.patch

src/bp-groups/bp-groups-admin.php

@@ -354 +354 @@
                 }
 
                 if ( ! empty( $user_names ) ) {
+                        $new_members = count( $user_names );
+
+                        if ( 1 < $new_members ) {
+                                bp_groups_defer_group_members_count( $group_id, true );
+                        }
 
                         foreach( array_values( $user_names ) as $user_name ) {
                                 $un = trim( $user_name );
@@ -372 +377 @@
                                         }
                                 }
                         }
+
+                        if ( 1 < $new_members ) {
+                                bp_groups_defer_group_members_count( $group_id, false );
+                        }
                 }
 
                 // Process member role changes.

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

@@ -3678 +3678 @@
         }
 }
 add_action( 'bp_init', 'bp_init_group_extensions', 11 );
+
+/**
+ * Updates a group members count when a user joined or left the group.
+ *
+ * @since 10.3.0
+ *
+ * @param BP_Groups_Member|int $groups_member The BP_Groups_Member object or the group member ID.
+ * @param int                  $group_id      The group's ID.
+ */
+function bp_groups_update_group_members_count( $groups_member, $group_id = 0 ) {
+        if ( true === $groups_member instanceof BP_Groups_Member ) {
+                $group_id = $groups_member->group_id;
+        }
+
+        BP_Groups_Member::refresh_total_member_count_for_group( (int) $group_id );
+}
+add_action( 'groups_member_after_save', 'bp_groups_update_group_members_count' );
+add_action( 'groups_member_after_remove', 'bp_groups_update_group_members_count' );
+add_action( 'bp_groups_member_after_delete', 'bp_groups_update_group_members_count', 10, 2 );
+
+/**
+ * Defers a group's counting to avoid updating it when batch adding/removing users to this group.
+ *
+ * @since 10.3.0
+ *
+ * @param int $group_id The group's ID.
+ * @param bool $defer True to defer, false otherwise.
+ */
+function bp_groups_defer_group_members_count( $group_id = 0, $defer = true ) {
+        if ( $defer ) {
+                remove_action( 'groups_member_after_save', 'bp_groups_update_group_members_count' );
+                remove_action( 'groups_member_after_remove', 'bp_groups_update_group_members_count' );
+                remove_action( 'bp_groups_member_after_delete', 'bp_groups_update_group_members_count', 10, 2 );
+        } else {
+                add_action( 'groups_member_after_save', 'bp_groups_update_group_members_count' );
+                add_action( 'groups_member_after_remove', 'bp_groups_update_group_members_count' );
+                add_action( 'bp_groups_member_after_delete', 'bp_groups_update_group_members_count', 10, 2 );
+
+                BP_Groups_Member::refresh_total_member_count_for_group( (int) $group_id );
+        }
+}

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

@@ -49 +49 @@
          */
         protected $group_member_ids;
 
+        /**
+         * Constructor.
+         *
+         * @since 10.3.0
+         *
+         * @param string|array|null $query See {@link BP_User_Query}.
+         */
+        public function __construct( $query = null ) {
+                $qv = bp_parse_args(
+                        $query,
+                        array(
+                                'count' => false, // True to perform a count query. False otherwise.
+                        )
+                );
+
+                parent::__construct( $qv );
+        }
+
         /**
          * Set up action hooks.
          *
@@ -62 +80 @@
                         $this->query_vars_raw['type'] = 'last_joined';
                 }
 
-                // Set the sort order.
-                add_action( 'bp_pre_user_query', array( $this, 'set_orderby' ) );
+                if ( ! $this->query_vars_raw['count'] ) {
+                        // Set the sort order.
+                        add_action( 'bp_pre_user_query', array( $this, 'set_orderby' ) );
 
-                // Set up our populate_extras method.
-                add_action( 'bp_user_query_populate_extras', array( $this, 'populate_group_member_extras' ), 10, 2 );
+                        // Set up our populate_extras method.
+                        add_action( 'bp_user_query_populate_extras', array( $this, 'populate_group_member_extras' ), 10, 2 );
+                } else {
+                        $this->query_vars_raw['orderby'] = 'ID';
+                }
+        }
+
+        /**
+         * Use WP_User_Query() to pull data for the user IDs retrieved in the main query.
+         *
+         * If a `count` query is performed, the function is used to validate existing users.
+         *
+         * @since 10.3.0
+         */
+        public function do_wp_user_query() {
+                if ( ! $this->query_vars_raw['count'] ) {
+                        return parent::do_wp_user_query();
+                }
+
+                /**
+                 * Filters the WP User Query arguments before passing into the class.
+                 *
+                 * @since 10.3.0
+                 *
+                 * @param array         $value Array of arguments for the user query.
+                 * @param BP_User_Query $this  Current BP_User_Query instance.
+                 */
+                $wp_user_query = new WP_User_Query(
+                        apply_filters(
+                                'bp_group_members_count_query_args',
+                                array(
+                                        // Relevant.
+                                        'fields'      => 'ID',
+                                        'include'     => $this->user_ids,
+
+                                        // Overrides
+                                        'blog_id'     => 0,    // BP does not require blog roles.
+                                        'count_total' => false // We already have a count.
+
+                                ),
+                                $this
+                        )
+                );
+
+                // Validate existing user IDs.
+                $this->user_ids = array_map( 'intval', $wp_user_query->results );
+                $this->results  = $this->user_ids;
+
+                // Set the total existing users.
+                $this->total_users = count( $this->user_ids );
         }
 
         /**
@@ -473 +540 @@
 
                 return wp_list_pluck( $group_user_ids, 'user_id' );
         }
+
+        /**
+         * Perform a database query to populate any extra metadata we might need.
+         *
+         * If a `count` query is performed, the function is used to validate active users.
+         *
+         * @since 10.3.0
+         */
+        public function populate_extras() {
+                if ( ! $this->query_vars_raw['count'] ) {
+                        return parent::populate_extras();
+                }
+
+                // Validate active users.
+                $active_users    = array_filter( BP_Core_User::get_last_activity( $this->user_ids ) );
+                $active_user_ids = array_keys( $active_users );
+                $this->results   = array_intersect( $this->user_ids, $active_user_ids );
+
+                // Set the total active users.
+                $this->total_users = count( $this->results );
+        }
 }

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

@@ -1789 +1789 @@
          * @return int Count of confirmed members for the group.
          */
         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 );
+                $meta_key = 'total_member_count';
+                $count    = groups_get_groupmeta( $group_id, $meta_key );
 
                 if ( false === $count || true === $skip_cache ) {
-                        $members = groups_get_group_members(
+                        $group_members = new BP_Group_Member_Query(
                                 array(
                                         'group_id'   => $group_id,
                                         'group_role' => array( 'member', 'admin', 'mod' ),
-                                        'type'       => 'active',
+                                        'count'      => true,
                                 )
                         );
 
-                        $count = $members['count'] ? $members['count'] : 0;
-
-                        groups_update_groupmeta( $group_id, $cache_key, (int) $count );
+                        $count = $group_members->total_users;
+                        groups_update_groupmeta( $group_id, $meta_key, $count );
                 }
 
                 /**
@@ -1814 +1813 @@
                  * @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 );
+                return (int) apply_filters( 'bp_groups_total_member_count', $count, (int) $group_id );
         }
 
         /**

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

@@ -309 +309 @@
                 // Update the user's group count.
                 self::refresh_total_group_count_for_user( $this->user_id );
 
-                // Update the group's member count.
-                self::refresh_total_member_count_for_group( $this->group_id );
-
                 /**
                  * Fires after the current group membership item has been saved.
                  *
@@ -447 +444 @@
                 // Update the user's group count.
                 self::refresh_total_group_count_for_user( $this->user_id );
 
-                // Update the group's member count.
-                self::refresh_total_member_count_for_group( $this->group_id );
-
                 /**
                  * Fires after a member is removed from a group.
                  *
@@ -517 +511 @@
                 // Update the user's group count.
                 self::refresh_total_group_count_for_user( $user_id );
 
-                // Update the group's member count.
-                self::refresh_total_member_count_for_group( $group_id );
-
                 /**
                  * Fires after a member is removed from a group.
                  *

tests/phpunit/testcases/groups/functions.php

@@ -7 +7 @@
 class BP_Tests_Groups_Functions extends BP_UnitTestCase {
         static public $group_ids;
         static public $user_ids;
+        protected $did_group_member_count = 0;
 
         static public function wpSetUpBeforeClass( $factory ) {
                 self::$user_ids  = $factory->user->create_many( 3 );
@@ -344 +345 @@
                 $this->assertEquals( 2, BP_Groups_Group::get_total_member_count( $g1 ) );
         }
 
+        /**
+         * @group total_member_count
+         * @ticket BP8688
+         */
+        public function test_total_member_count_groups_inactive_user() {
+                $u1 = self::factory()->user->create();
+                $u2 = wp_insert_user( array(
+                        'user_pass'  => 'foobar',
+                        'user_login' => 'foobar',
+                        'user_email' => 'foobar@buddypress.org',
+                ) );
+
+                $g1 = self::factory()->group->create( array( 'creator_id' => $u1 ) );
+
+                groups_join_group( $g1, $u2 );
+
+                $this->assertEquals( 1, groups_get_total_member_count( $g1 ) );
+        }
+
+        /**
+         * @group total_member_count
+         * @ticket BP8688
+         */
+        public function test_total_member_count_groups_spammed_user() {
+                $u1 = self::factory()->user->create();
+                $u2 = self::factory()->user->create();
+
+                $g1 = self::factory()->group->create( array( 'creator_id' => $u1 ) );
+
+                groups_join_group( $g1, $u2 );
+                bp_core_process_spammer_status( $u2, 'spam' );
+
+                $this->assertEquals( 1, groups_get_total_member_count( $g1 ) );
+        }
+
+        /**
+         * @group total_member_count
+         * @ticket BP8688
+         */
+        public function test_total_member_count_groups_deferred() {
+                $u1 = self::factory()->user->create();
+                $g1 = self::factory()->group->create( array( 'creator_id' => $u1 ) );
+                $members = array();
+                $this->did_group_member_count = 0;
+
+                add_filter( 'bp_groups_total_member_count', array( $this, 'filter_bp_groups_total_member_count' ) );
+
+                bp_groups_defer_group_members_count( $g1, true );
+                for ( $i = 1; $i < 6; $i++ ) {
+                        $members[ $i ] = self::factory()->user->create();
+                        groups_join_group( $g1, $members[ $i ] );
+                }
+                bp_groups_defer_group_members_count( $g1, false );
+
+                remove_filter( 'bp_groups_total_member_count', array( $this, 'filter_bp_groups_total_member_count' ) );
+
+                $this->assertTrue( 1 === $this->did_group_member_count );
+                $this->assertEquals( count( $members ) + 1, groups_get_total_member_count( $g1 ) );
+        }
+
+        public function filter_bp_groups_total_member_count( $count ) {
+                $this->did_group_member_count += 1;
+                return $count;
+        }
+
         /**
          * @group total_member_count
          * @group groups_create_group