[buddypress-trac] [BuddyPress Trac] #7290: 2.7.0-rc2 breaks GeomyWP + Groups Locator groups list

[buddypress-trac]

#7290: 2.7.0-rc2 breaks GeomyWP + Groups Locator groups list
-------------------------------+----------------------
 Reporter:  dreadedhamish      |       Owner:
     Type:  defect (bug)       |      Status:  closed
 Priority:  highest            |   Milestone:
Component:  Groups             |     Version:
 Severity:  normal             |  Resolution:  invalid
 Keywords:  reporter-feedback  |
-------------------------------+----------------------
Changes (by boonebgorges):

 * status:  reopened => closed
 * resolution:   => invalid


Comment:

 Thank you for the detailed feedback, @ninjew! It's extremely helpful.

 > Perhaps having WHERE 1 = 1 instead of omitting the where clause would be
 helpful in such scenarios.

 This is a good idea. I've opened a ticket: #7332.

 > The new method does the proximity SQL Query only once using the
 get_groups_based_location() function instead of having it 3 times for the
 filter_group_query, filter_group_query and add_group_data filters. Which I
 believe will improve performance.

 Yup, this seems like a good technique. If I were you, I'd also look into
 implementing some persistent caching, which, if done correctly, could
 eliminate at least some of the excess queries. More specifically, if you
 did the following after your initial query:

 {{{
 $groups = $wpdb->get_results( "
                         SELECT gg.id, gg.lat, gg.lng as `long`,
 gg.address, gg.formatted_address, gg.map_icon
                         FROM `$table_name` gg"
                 );

 foreach ( $groups as $group ) {
     wp_cache_set( $group->id, 'gmw_group_geo_data', $group );
 }
 }}}

 then later on, when you need to append the data to the group objects, you
 could do something like:

 {{{
 $group_geo_data = wp_cache_get( $group_id, 'gmw_group_geo_data' );
 }}}

 This is similar to your `$this->gmw_groups` caching technique, with the
 additional advantage that sites running a persistent cache may be able to
 skip the query altogether on subsequent pageloads. (Of course, introducing
 this requires some other refactoring, and careful invalidation. So it may
 be an idea for future use.)

 I've opened #7333 to talk more about `populate()` and extending core
 objects. For the record, I think we need to support use cases like yours
 for backward compatibility reasons. But I don't think we should
 necessarily be encouraging the modification of group objects. It may be
 preferable, in the future, to do something like this (I'm guessing that
 you need your geo data in the loop):

 {{{#!php
 <?php
 function bp7290_add_geo_data_to_group_display() {
     $group_id = bp_get_group_id();

     $geo_data = custom_function_to_get_geo_data_for_a_group( $geo_data );

     // do something with $geo_data
 }
 add_action( 'bp_directory_groups_item',
 'bp7290_add_geo_data_to_group_display' );
 }}}

 `custom_function_to_get_geo_data_for_a_group()` would then fetch data from
 the WP cache (as described above), or from your own global/static object,
 or wherever you decide to store the data after querying it at the
 beginning of the loop, instead of relying on `$groups_template->group`.
 This way, you won't be dependent on any changes that BP might make to its
 own internal globals.

 Thanks again for the extended discussion! It's been extremely helpful.

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