#6352 closed enhancement (fixed)
Add parameter to differentiate filter values with bp_core_get_specific_users_count_sql
Reported by: |
|
Owned by: |
|
---|---|---|---|
Milestone: | 2.3 | Priority: | normal |
Severity: | normal | Version: | 1.2 |
Component: | Core | Keywords: | has-patch |
Cc: |
Description
Let's add a string parameter to bp_core_get_specific_users_count_sql to help differentiate which SQL statement is being passed in.
Attachments (1)
Change History (8)
#2
@
10 years ago
Aesthetically I like the simplicity of tw2113's suggestion, but yeah, we don't do this anywhere else. We need to introduce two new filters here.
#3
@
10 years ago
- Owner set to boonebgorges
- Resolution set to fixed
- Status changed from new to closed
In 9699:
#4
@
10 years ago
Curious what the difference is between passing a string as a parameter, or passing a variable? Obviously we pass tons of variables through filters hundreds of times per page load.
The patched solution maintains backwards compatibility with existing installations, using the filter API to allow manual forking of the filter logic in one place.
What does BuddyPress do in other places where these two types of filters are present?
#5
@
10 years ago
We usually have separate filter names for these sorts of things. See eg https://buddypress.trac.wordpress.org/browser/trunk/src/bp-core/classes/class-bp-core-user.php?marks=387,419#L379. I can't think of anywhere else where we use the same filter for paged/count SQL strings and differentiate in the filter with a 'type' flag of this sort. I'm assuming that tw2113 originally suggested this technique because I'd suggested it as a possibility in a Slack discussion we'd had.
Small argument in favor of different filter names: it keeps the callback signature closer to that of the parent method. function my_get_specific_users_cb( $sql, $user_ids, $limit, $page, $populate_extras )
, instead of offsetting with a $type
param.
That said, if people prefer the single filter name with a $type
param, I don't care too much. Feel free to revert [9699] with something closer to what tw2113 originally suggested.
I think just changing one or both of the filter names would be best. If we changed both, we could still run the results through the original filter name for backpat.