[buddypress-trac] [BuddyPress Trac] #5328: Use of the WordPress HeartBeat API to check for newest activities and to let user load them

[buddypress-trac]

#5328: Use of the WordPress HeartBeat API to check for newest activities and to
let user load them
-------------------------------------------------+-----------------------
 Reporter:  imath                                |       Owner:  imath
     Type:  enhancement                          |      Status:  assigned
 Priority:  normal                               |   Milestone:  2.0
Component:  Activity                             |     Version:  1.9.1
 Severity:  normal                               |  Resolution:
 Keywords:  has-patch 2nd-opinion needs-testing  |
-------------------------------------------------+-----------------------

Comment (by DJPaul):

 PHP

 * Please can the code tidy-up bits be removed from this patch (perhaps
 just commit that tidy-up now)? Specifically the ` if ( ! empty( $pid_sql )
 )` spacing changes.
 * The introduction of `bp_is_activity_directory()` as a new helper
 function should be in its own ticket which you could commit straightaway.
 It might also make sense to introduce similar functions for all core
 components that have a directory at the same time to provide a more
 consistent API, though those shouldn't be a reason to hold up your work on
 this feature.
 * The changes to `get_last_updated()` should definitely be committed
 separately from the introduction of any heartbeat functionality. Since
 this introduces new direct SQL calls, this could also use unit tests to
 help prevent us accidentally breaking this in the future.
  * Have you EXPLAIN'd those queries to check we are hitting indexes, etc.
 ?
  * Do we not have a core function that already builds a query that does
 this for us, or is it more optimal to do this directly?
  * We should definitely object cache anything that we're proposing to hit
 every X seconds to avoid a bunch of database queries.
 * Changing the heartbeat interval with `heartbeat_settings` will affect
 any heartbeat request, not just ones from BuddyPress. There are very few
 plugins that do front-end heartbeats, but it's worth keeping in mind.
  * I think we load `bp-activity-filters.php` inside `wp-admin`, and I
 think our function here will affect the heartbeat in the various wp-admin
 screens.
 * A level of indentation can be removed from
 `bp_activity_heartbeat_last_recorded()` if we bail out at the top of the
 function, if the request doesn't contain any BP data.

 JS
 * We should namespace the JS event handlers in case other plugins choose
 to remove them via JS in the future. It's pretty easy to do: change e.g.
 `on( 'heartbeat-send', ... )` to `on( 'heartbeat-send.buddypress', ... )`.
 * In the `heartbeat-tick` JS handler, we don't do anything if the request
 doesn't contain `data['last_id_recorded']`. I think we should prefix that
 key in case another plugin implements heartbeat using that same ID. e.g.
 something like `bp_activity_last_id_recorded`.
 * Some minor indentation issues.
 * If the browser/tab is in the background or out of focus, we should not
 send any heartbeat requests to save data usage. I am not sure if this kind
 of enhancement has been added to WP Core since I last looked at the
 heartbeat code, but it's not too hard to implement manually.

 IMO if we are introducing a new feature, it should be on by default if its
 component is on. I implemented front-end heartbeat in another plugin and
 user feedback was that I needed to speed up the heartbeats -- people
 weren't on pages for longer than 15 seconds (or whatever the default ping
 period is) so never saw the effects, and no-one yet has complained that
 their crappy webhost can't handle extra requests. Something to keep in
 mind.

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