[buddypress-trac] [BuddyPress Trac] #6645: `BP_Activity_Activity::get()` args should be translated into `BP_Activity_Query`

[buddypress-trac]

#6645: `BP_Activity_Activity::get()` args should be translated into
`BP_Activity_Query`
----------------------------------+-----------------------------
 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
 SQL statement.)

 The complexity issue is more pertinent. Even something simple like
 `hide_sitewide=false` becomes

 {{{
 array(
     '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.

 Yes, absolutely.

 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/>
BuddyPress Trac