Total groups wrong when fetching groups with meta query

[r-a-y]

Using bp_has_groups() with the new meta_query parameter works well, however the total groups returned is wrong.

Attached is a unit test. Will have a patch up soon.

[r-a-y]

01.patch changes the way the groups total SQL statement is generated.

Before, we used two different SQL statements in BP_Groups_Group::get(). One for the paged groups and the other for total groups. This isn't so great as changes in one SQL statement will need to be mirrored in the other, which can lead to inaccuracies. The new meta query parameter outlines this problem.

Instead of fiddling around with the total groups SQL to work with the meta query parameter, the attached patch uses the paged SQL statement to generate the total groups SQL statement. See line 451 of the patch.

Because of this, the $total_sql array is now not used. However, for plugin devs using $total_sql in the 'bp_groups_get_total_groups_sql' filter, we still need to keep it for the time being. So, I've introduced a new method - get_deprecated_total_groups_sql() - to move the old code away from the get() method with the hopes of removing it later.

Patch passes the unit test attached to this ticket. However, this could use more testing.

I know this is quite a big change, but I think this is cleaner and makes more sense. Feel free to implement an alternative fix if this is deemed too radical a change as 1.8 is due to hit some time very soon.

[boonebgorges]

Good catch, r-a-y.

I agree that the way we're assembling the total count here is not ideal. Your patch seems like a good way forward in the future (though we can probably also snip LIMIT and ORDER BY clauses as well). But, as you note, it's a big change for 1.8. And since the bug is with a feature that is being introduced for the first time in 1.8 (ie, this is neither a regression nor a longtime bug), it seems fine to me to put a band-aid on it for now, as in 5099.02.patch. Then, for 1.9, we can circle back around to your more general suggestion.

Have a look at 5099.02.patch and let me know what you think.

[johnjamesjacoby]

Replying to boonebgorges:

Have a look at 5099.02.patch and let me know what you think.

Looks like a good enough work around for me. We could move Ray's larger fix to 1.8.1 when it's time, also.

[r-a-y]

02.patch works for me.

And, Boone, you're right we should remove the LIMIT and ORDER BY clauses when we look at 01.patch again later on.

[boonebgorges]

Thanks, guys.

[boonebgorges]

In 7274:

Adds unit test for #5099

This tests whether the total group count return by BP_Groups_Group::get() is
correctly calculated when using meta_query

See #5099

Props r-a-y

[boonebgorges]

In 7275:

Fixes total group count for BP_Groups_Group::get() when using meta_query

The fix contained in this changeset is intended to be temporary. A longer-term
fix will mean refactoring the way that total-group queries are composed, so
that the get() method does not have to assemble these queries twice.

See #5099

[boonebgorges]

Moving to 1.9 so we can address the larger issue of how the total group count query is assembled.

[boonebgorges]

Let's look at this alongside #5451 for 2.1.

[boonebgorges]

In 11071:

Groups: Overhaul BP_Groups_Group::get() SQL query.

The new query follows the WP and BP convention of "split" queries: one
for the IDs matching the query parameters, and a second one for the
objects corresponding to the matched IDs. This configuration allows for
improved caching and better performance, especially on installations
with large numbers of groups.

The rewrite serves as a convenient excuse to address a number of
longtime annoyances with the way group queries work:

• Previously, comma syntax was used for table joins, rather than the JOIN keyword. This required some string manipulation when using external tools for generating SQL fragments, such as WP_Tax_Query and WP_Meta_Query. See #5099, #5874. We now use the more standard JOIN syntax.
• The logic for assembling the "total" query is overwhelmingly simpler.
• Previously, group queries required three joined tables: the groups table, one groupmeta table (for last_activity), and a second groupmeta table (for total_member_count). After this changeset, these tables will only be joined when needed for sorting (orderby or type). The last_activity and total_member_count properties, when needed for display in a group loop, are lazyloaded from groupmeta - where they're precached by default (see update_meta_cache) - rather than pulled as part of the primary query, and are made available via __get() for backward compatibility.

See #5451, #5874. Fixes #5099.