[buddypress-trac] [BuddyPress Trac] #6506: Should not try to redirect in bp_has_message_threads

[buddypress-trac]

#6506: Should not try to redirect in bp_has_message_threads
-----------------------------------+------------------
 Reporter:  johnjamesjacoby        |       Owner:
     Type:  defect (bug)           |      Status:  new
 Priority:  normal                 |   Milestone:  2.4
Component:  Component - Messaging  |     Version:  1.0
 Severity:  normal                 |  Resolution:
 Keywords:  has-patch              |
-----------------------------------+------------------

Comment (by johnjamesjacoby):

 Replying to [comment:4 boonebgorges]:
 > Regarding the `bp_do_404()`: This block can never be reached, because of
 the `bp_current_user_can( 'bp_moderate' )` checks in
 `BP_Messages_Component::setup_nav()`. Technically, it could be removed
 altogether, will all caps checks handled during nav setup. But swapping
 out for `bp_do_404()` will do no harm.
 Screen functions that do not hook themselves directly into `bp_screens`
 (meaning they are hooked to `bp_screens` indirectly via `setup_nav()`,
 `bp_core_new_nav_item()`, or `bp_core_new_subnav_item()`) do not require
 any additional capability checks, so I recommend we remove it here to
 avoid future confusion.

 The Messages component screens specifically needs to do a bunch of
 `bp_action_variables()` checks because of the way
 `messages_screen_conversation()` is loosely hooked in; most other
 components do not have this collision issue.

 > I agree that there should not be a redirect in
 `bp_has_messages_threads()`, but simply removing the block could
 potentially introduce security issues when the function is being called
 directly without proper cap checks. I recommend returning false, which is
 to say, `! bp_has_messages_threads()`. See [attachment:6506.02.patch].
 None of our other template-loop generating functions perform hard-stops on
 query results based on capabilities or conditions; they only generate
 smart defaults based on various conditions like current components,
 actions, action variables, displayed members, single items, etc...

 I feel as if `bp_has_message_threads()` is the closest template function
 we (currently) have to openly query for messages within any set of
 parameters. Returning anything other than the requested results seems
 counter-intuitive to me, at least until we have `bp_has_messages()` or
 something that could perhaps be more of a general handler, when we can
 then purposely limit functions like `bp_has_message_threads()` to fit
 exactly the parameters it's intended for.

 Updated patch imminent.

--
Ticket URL: <https://buddypress.trac.wordpress.org/ticket/6506#comment:5>
BuddyPress Trac <http://buddypress.org/>
BuddyPress Trac