Skip to:
Content

BuddyPress.org

Opened 5 years ago

Closed 5 years ago

#5584 closed defect (bug) (fixed)

Pagination is removed when all groups are empty

Reported by: dansearle Owned by: boonebgorges
Milestone: 2.2 Priority: normal
Severity: normal Version: 2.0
Component: Groups Keywords: has-patch
Cc:

Description

If all the groups are empty (they have no members) then the pagination in the WP Admin groups list isn't shown. See /wp-admin/admin.php?page=bp-groups

The way the method BP_Groups_Group->get works means the two queries return inconsistent results, e.g. the query to fetch the list of groups doesn't always join to the members table; however the query to count them does.

Compare

$sqlselect? = "SELECT DISTINCT g.id, g.*, gm1.meta_value AS total_member_count, gm2.meta_value AS last_activity";
$sqlfrom? = " FROM {$bp->groups->table_name_groupmeta} gm1, {$bp->groups->table_name_groupmeta} gm2,";

if ( ! empty( $ruser_id? ) ) {

$sqlmembers_from? = " {$bp->groups->table_name_members} m,";

}

with

$total_sqlselect? = "SELECT COUNT(DISTINCT g.id) FROM {$bp->groups->table_name} g, {$bp->groups->table_name_members} gm1, {$bp->groups->table_name_groupmeta} gm2";

if ( ! empty( $ruser_id? ) ) {

$total_sqlselect? .= ", {$bp->groups->table_name_members} m";

}

Attachments (4)

5584-unit-tests.diff (1.0 KB) - added by Mamaduka 5 years ago.
5584-unit-tests.2.diff (1.2 KB) - added by boonebgorges 5 years ago.
5584.diff (1.1 KB) - added by Mamaduka 5 years ago.
5584.2.diff (1.2 KB) - added by Mamaduka 5 years ago.

Download all attachments as: .zip

Change History (15)

#1 @boonebgorges
5 years ago

  • Keywords needs-patch added
  • Milestone changed from Awaiting Review to 2.1

#2 @DJPaul
5 years ago

  • Keywords needs-unit-tests added

#3 @DJPaul
5 years ago

  • Milestone changed from 2.1 to 2.2

#4 @Mamaduka
5 years ago

Added first pass for unit test here.

Got weird behaviour here, if I specify $user_id as creator_id in group factory tests are failing, otherwise tests are passed.

Any advises here?

Thanks

#5 @boonebgorges
5 years ago

Got weird behaviour here, if I specify $user_id as creator_id in group factory tests are failing, otherwise tests are passed.

Your get_current_user_id() check is returning 0. So BP_UnitTest_Factory_For_Group is creating a user behind the scenes. That means that your group member deletions are doing nothing.

I've updated the patch so that it fails as expected. The proper fix will be to change the group count query so that it doesn't join the group_members table like this, but first I'd like a bit of research done into why this join was put there in the first place. ("Peatling did it" may end up being a sufficient answer.)

#6 @boonebgorges
5 years ago

(See #5983 r9120 for more details on the factory issue.)

#7 @Mamaduka
5 years ago

Boone,

Did some digging in git logs and looks like this was introduced by JJJ in r4064, maybe he can provide some feedback.

@Mamaduka
5 years ago

#8 @Mamaduka
5 years ago

  • Keywords has-patch added; needs-patch needs-unit-tests removed

Patch fixes the issue 5584.diff.

Removes joins for $bp->groups->table_name_members and $bp->groups->table_name_groupmeta tables from first select. Unlike global groups query those joins aren't needed here, since we are not sorting based on last activity.

#9 @boonebgorges
5 years ago

The age of r4064, and the fact that it's not linked to any ticket, lead me to guess it was basically a guess at fixing a problem. I'm OK with removing it based on that. Thanks for doing the digging, Mamaduka.

We're not sorting based on last_activity, but removing this join will mean that we start counting groups that have no last_activity at all. I don't *think* that this natually occurs in BP (unlike the parallel situation with members), but it could use a few minutes' research. I'm tempted to leave that join in there, just to be safe.

@Mamaduka
5 years ago

#10 @Mamaduka
5 years ago

Boone,

Updated patch and included last_activity checks.

#11 @boonebgorges
5 years ago

  • Owner set to boonebgorges
  • Resolution set to fixed
  • Status changed from new to closed

In 9230:

Don't join against the group members table when fetching total group counts.

The join doesn't take place in the 'paged' count, which leads to inconsistent
query results when groups have no members.

This reverts changes made in [4064].

Props Mamaduka.
Fixes #5584.

Note: See TracTickets for help on using tickets.