[buddypress-trac] [BuddyPress Trac] #6645: `BP_Activity_Activity::get()` args should be translated into `BP_Activity_Query`
noreply at wordpress.org
Wed Oct 7 01:33:29 UTC 2015
#6645: `BP_Activity_Activity::get()` args should be translated into
Reporter: boonebgorges | Owner:
Type: enhancement | Status: new
Priority: normal | Milestone: Future Release
Component: Component - Activity | Version:
Severity: normal | Resolution:
Keywords: has-patch |
Comment (by boonebgorges):
> I might be overlooking what you are proposing so if I am
misunderstanding something, please let me know!
No, this is totally right. I thought about the SQL filter, but I don't
really care much about that. (If you are filtering a SQL statement, and
doing it in a way that isn't resilient, then you shouldn't be filtering a
The complexity issue is more pertinent. Even something simple like
'column' => 'hide_sitewide',
'value' => 1,
I definitely think your patch is an improvement, though it still requires
filtering `where_conditions` to drop the 'scope' clause if you want
`filter_array` to work in an unfettered way. I guess I'd argue that, while
it may be overkill to convert *all* params to `BP_Activity_Query` clauses,
it might make sense to do it in the case of the 'scope' clause. (Plus,
it's already happening internally, we're just not exposing it - we return
the SQL fragment.)
> Before it even gets to this stage, I think arguments should be filtered
through 'bp_after_has_activities_parse_args' to remove the complexity of
the query args.
And actually, here's something I just thought of, which would make the
`bp_parse_args()` filter and the scope/filter_array separation even more
useful. What if `scope` were translated into a `filter_query` somewhere
further up the stack - say, in `bp_activity_get()` or
`bp_has_activities()`? This way, the `bp_parse_args()` filter in
`BP_Activity_Activity::get()` would, in most cases, receive a more robust
and explicit set of arguments, making it easier for devs to figure out
what needs to be changed if they want to tweak the logic. (We'd continue
to accept 'scope' in `BP_Activity_Activity::get()`, but we'd never pass
the param down that far in core.)
I guess this is all coming from a strong dislike for 'scope'. It's a black
box that hides a lot of complexity and assumptions. Unraveling them
systematically so that they can be modified in a filter often means
reverse-engineering what the various values of 'scope' are intended to do.
'user_id', 'item_id', 'hide_sitewide', and even 'filter_query' can easily
be parsed programatically. 'scope' is opaque, and depends on context. I'd
be happy if it were never passed to a low-level function or filter.
I won't beat the drum for much longer :) If @r-a-y and others think it's a
good idea not to touch the flow of 'scope', that's OK with me. But I will
say this: every time I have to deal with it in a plugin, I am confused.
And if I'm getting confused, there's not much hope for most people trying
to modify activity queries in a systematic way.
@r-a-y - I'm happy to defer to your wisdom here.
Ticket URL: <https://buddypress.trac.wordpress.org/ticket/6645#comment:2>
BuddyPress Trac <http://buddypress.org/>
More information about the buddypress-trac